From ee07ae93878fe8c247c738b5d422ae2976787a63 Mon Sep 17 00:00:00 2001 From: jdolle <1841898+jdolle@users.noreply.github.com> Date: Tue, 9 Dec 2025 11:32:51 -0800 Subject: [PATCH 1/7] adjust change capture and resolvers to optionally provide a full list or the simplified list of changes --- .../api/src/modules/schema/module.graphql.ts | 126 +++++++++++++++--- .../src/modules/schema/providers/inspector.ts | 13 +- .../schema/providers/registry-checks.ts | 9 +- .../schema/providers/schema-check-manager.ts | 52 +++++++- .../schema/providers/schema-version-helper.ts | 53 +++++--- .../modules/schema/resolvers/SchemaVersion.ts | 12 +- .../schema/resolvers/SuccessfulSchemaCheck.ts | 12 +- .../storage/src/schema-change-model.ts | 4 +- .../web/app/src/pages/target-proposal.tsx | 2 +- 9 files changed, 226 insertions(+), 57 deletions(-) diff --git a/packages/services/api/src/modules/schema/module.graphql.ts b/packages/services/api/src/modules/schema/module.graphql.ts index 233646e19d5..3ab65fa4037 100644 --- a/packages/services/api/src/modules/schema/module.graphql.ts +++ b/packages/services/api/src/modules/schema/module.graphql.ts @@ -879,10 +879,25 @@ export default gql` """ Schema changes that were introduced in this schema version (compared to the previous version). """ - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection """ GitHub metadata associated with the schema version. @@ -1261,9 +1276,24 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges: SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection schemaPolicyErrors: SchemaPolicyWarningConnection """ @@ -1299,10 +1329,25 @@ export default gql` schemaCompositionErrors: SchemaErrorConnection @tag(name: "public") - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection compositeSchemaSDL: String @tag(name: "public") supergraphSDL: String @tag(name: "public") @@ -1345,13 +1390,28 @@ export default gql` """ Breaking schema changes for this contract version. """ - breakingSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection """ Safe schema changes for this contract version. """ - safeSchemaChanges: SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") previousContractVersion: ContractVersion @tag(name: "public") previousDiffableContractVersion: ContractVersion @tag(name: "public") @@ -1412,12 +1472,27 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges: SchemaChangeConnection @tag(name: "public") + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") """ Breaking changes can exist in an successful schema check if the check was manually approved. """ - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection """ Schema policy errors can exist in an successful schema check if the check was manually approved. @@ -1509,9 +1584,24 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges: SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges: SchemaChangeConnection - safeSchemaChanges: SchemaChangeConnection + schemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection @tag(name: "public") + breakingSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection + safeSchemaChanges( + """ + This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. + """ + simplifyChanges: Boolean = true + ): SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection schemaPolicyErrors: SchemaPolicyWarningConnection diff --git a/packages/services/api/src/modules/schema/providers/inspector.ts b/packages/services/api/src/modules/schema/providers/inspector.ts index 34ed1bcb2fe..c9928f0b33c 100644 --- a/packages/services/api/src/modules/schema/providers/inspector.ts +++ b/packages/services/api/src/modules/schema/providers/inspector.ts @@ -10,7 +10,14 @@ import { type GraphQLSchema, } from 'graphql'; import { Injectable, Scope } from 'graphql-modules'; -import { Change, ChangeType, diff, DiffRule, TypeOfChangeType } from '@graphql-inspector/core'; +import { + Change, + ChangeType, + diff, + DiffRule, + Rule, + TypeOfChangeType, +} from '@graphql-inspector/core'; import { traceFn } from '@hive/service-common'; import { HiveSchemaChangeModel } from '@hive/storage'; import { Logger } from '../../shared/providers/logger'; @@ -31,10 +38,10 @@ export class Inspector { 'hive.diff.changes.count': result.length, }), }) - async diff(existing: GraphQLSchema, incoming: GraphQLSchema) { + async diff(existing: GraphQLSchema, incoming: GraphQLSchema, rules?: Rule[]) { this.logger.debug('Comparing Schemas'); - const changes = await diff(existing, incoming, [DiffRule.simplifyChanges]); + const changes = await diff(existing, incoming, rules); return changes .filter(dropTrimmedDescriptionChangedChange) diff --git a/packages/services/api/src/modules/schema/providers/registry-checks.ts b/packages/services/api/src/modules/schema/providers/registry-checks.ts index 2e162ac55c5..d415b4ffa18 100644 --- a/packages/services/api/src/modules/schema/providers/registry-checks.ts +++ b/packages/services/api/src/modules/schema/providers/registry-checks.ts @@ -2,7 +2,7 @@ import { URL } from 'node:url'; import { type GraphQLSchema } from 'graphql'; import { Injectable, Scope } from 'graphql-modules'; import hashObject from 'object-hash'; -import { ChangeType, CriticalityLevel, TypeOfChangeType } from '@graphql-inspector/core'; +import { ChangeType, CriticalityLevel, DiffRule, TypeOfChangeType } from '@graphql-inspector/core'; import type { CheckPolicyResponse } from '@hive/policy'; import type { CompositionFailureError, ContractsInputType } from '@hive/schema'; import { traceFn } from '@hive/service-common'; @@ -426,6 +426,7 @@ export class RegistryChecks { /** Settings for fetching conditional breaking changes. */ conditionalBreakingChangeConfig: null | ConditionalBreakingChangeDiffConfig; failDiffOnDangerousChange: null | boolean; + filterNestedChanges: boolean; }) { let existingSchema: GraphQLSchema | null = null; let incomingSchema: GraphQLSchema | null = null; @@ -463,7 +464,11 @@ export class RegistryChecks { } satisfies CheckResult; } - let inspectorChanges = await this.inspector.diff(existingSchema, incomingSchema); + let inspectorChanges = await this.inspector.diff( + existingSchema, + incomingSchema, + args.filterNestedChanges ? [DiffRule.simplifyChanges] : [], + ); // Filter out federation specific changes as they are not relevant for the schema diff and were in previous schema versions by accident. if (args.filterOutFederationChanges === true) { diff --git a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts index 9a63b89a8ec..63acb920266 100644 --- a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts +++ b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts @@ -3,6 +3,7 @@ import type { SuccessfulSchemaCheckMapper, } from '../module.graphql.mappers'; import { Injectable, Scope } from 'graphql-modules'; +import { DiffRule } from '@graphql-inspector/core'; import { Storage } from '../../shared/providers/storage'; import { formatNumber } from '../lib/number-formatting'; import { SchemaManager } from './schema-manager'; @@ -35,27 +36,70 @@ export class SchemaCheckManager { return !!(schemaCheck.breakingSchemaChanges?.length || schemaCheck.safeSchemaChanges?.length); } - getSafeSchemaChanges(schemaCheck: SchemaCheck) { + getSafeSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { if (!schemaCheck.safeSchemaChanges?.length) { return null; } + if (simplifyChanges) { + /** + * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule + * doesn't need more information than the change type and path fields. + */ + return DiffRule.simplifyChanges({ + changes: schemaCheck.safeSchemaChanges as any, + oldSchema: undefined as any, + newSchema: undefined as any, + config: undefined, + }) as unknown as typeof schemaCheck.safeSchemaChanges; + } + return schemaCheck.safeSchemaChanges; } - getAllSchemaChanges(schemaCheck: SchemaCheck) { + getAllSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { if (!schemaCheck.safeSchemaChanges?.length && !schemaCheck.breakingSchemaChanges?.length) { return null; } - return [...(schemaCheck.breakingSchemaChanges ?? []), ...(schemaCheck.safeSchemaChanges ?? [])]; + const changes = [ + ...(schemaCheck.breakingSchemaChanges ?? []), + ...(schemaCheck.safeSchemaChanges ?? []), + ]; + if (simplifyChanges) { + /** + * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule + * doesn't need more information than the change type and path fields. + */ + return DiffRule.simplifyChanges({ + changes: changes as any, + oldSchema: undefined as any, + newSchema: undefined as any, + config: undefined, + }) as unknown as typeof changes; + } + + return changes; } - getBreakingSchemaChanges(schemaCheck: SchemaCheck) { + getBreakingSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { if (!schemaCheck.breakingSchemaChanges?.length) { return null; } + if (simplifyChanges) { + /** + * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule + * doesn't need more information than the change type and path fields. + */ + return DiffRule.simplifyChanges({ + changes: schemaCheck.breakingSchemaChanges as any, + oldSchema: undefined as any, + newSchema: undefined as any, + config: undefined, + }); + } + return schemaCheck.breakingSchemaChanges; } diff --git a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts index d65d6982d2e..4e8ce4d6ea5 100644 --- a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts +++ b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts @@ -1,7 +1,7 @@ import type { SchemaVersionMapper as SchemaVersion } from '../module.graphql.mappers'; import { isTypeSystemExtensionNode, print } from 'graphql'; import { Injectable, Scope } from 'graphql-modules'; -import { CriticalityLevel } from '@graphql-inspector/core'; +import { CriticalityLevel, DiffRule } from '@graphql-inspector/core'; import { mergeTypeDefs } from '@graphql-tools/merge'; import { traceFn } from '@hive/service-common'; import type { SchemaChangeType } from '@hive/storage'; @@ -156,7 +156,7 @@ export class SchemaVersionHelper { } @traceFn('SchemaVersionHelper._getSchemaChanges', { - initAttributes: input => ({ + initAttributes: ({ version: input }) => ({ 'hive.target.id': input.targetId, 'hive.organization.id': input.organizationId, 'hive.project.id': input.projectId, @@ -167,16 +167,26 @@ export class SchemaVersionHelper { 'hive.safe-changes.count': changes?.safe?.length, }), }) - @cache(version => version.id) - private async _getSchemaChanges(schemaVersion: SchemaVersion) { + @cache<{ version: SchemaVersion; simplified: boolean }>( + ({ version, simplified = true }) => `${version.id}${simplified ? '/simple' : ''}`, + ) + private async _getSchemaChanges({ + version: schemaVersion, + simplified, + }: { + version: SchemaVersion; + simplified: boolean; + }) { if (!schemaVersion.isComposable) { return null; } if (schemaVersion.hasPersistedSchemaChanges) { - const changes = await this.storage.getSchemaChangesForVersion({ - versionId: schemaVersion.id, - }); + const changes: null | Array = await this.storage.getSchemaChangesForVersion( + { + versionId: schemaVersion.id, + }, + ); const safeChanges: Array = []; const breakingChanges: Array = []; @@ -241,6 +251,7 @@ export class SchemaVersionHelper { filterOutFederationChanges: project.type === ProjectType.FEDERATION, conditionalBreakingChangeConfig: null, failDiffOnDangerousChange, + filterNestedChanges: simplified, }); if (diffCheck.status === 'skipped') { @@ -274,23 +285,35 @@ export class SchemaVersionHelper { }); } - async getBreakingSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getBreakingSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return changes?.breaking ?? null; } - async getSafeSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getSafeSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return changes?.safe ?? null; } - async getAllSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getAllSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return changes?.all ?? null; } - async getHasSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges(schemaVersion); + async getHasSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + const changes = await this._getSchemaChanges({ + version: schemaVersion, + simplified: simplifyChanges, + }); return !!changes?.breaking?.length || !!changes?.safe?.length; } diff --git a/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts b/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts index 7e5c66421a3..566126158d5 100644 --- a/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts +++ b/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts @@ -70,14 +70,14 @@ export const SchemaVersion: SchemaVersionResolvers = { schemaCompositionErrors: async (version, _, { injector }) => { return injector.get(SchemaVersionHelper).getSchemaCompositionErrors(version); }, - breakingSchemaChanges: async (version, _, { injector }) => { - return injector.get(SchemaVersionHelper).getBreakingSchemaChanges(version); + breakingSchemaChanges: async (version, { simplifyChanges }, { injector }) => { + return injector.get(SchemaVersionHelper).getBreakingSchemaChanges(version, simplifyChanges); }, - safeSchemaChanges: async (version, _, { injector }) => { - return injector.get(SchemaVersionHelper).getSafeSchemaChanges(version); + safeSchemaChanges: async (version, { simplifyChanges }, { injector }) => { + return injector.get(SchemaVersionHelper).getSafeSchemaChanges(version, simplifyChanges); }, - schemaChanges: async (version, _, { injector }) => { - return injector.get(SchemaVersionHelper).getAllSchemaChanges(version); + schemaChanges: async (version, { simplifyChanges }, { injector }) => { + return injector.get(SchemaVersionHelper).getAllSchemaChanges(version, simplifyChanges); }, supergraph: async (version, _, { injector }) => { return injector.get(SchemaVersionHelper).getSupergraphSdl(version); diff --git a/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts b/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts index 1e4cae9781b..9715780a9e2 100644 --- a/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts +++ b/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts @@ -8,11 +8,11 @@ export const SuccessfulSchemaCheck: SuccessfulSchemaCheckResolvers = { schemaVersion: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getSchemaVersion(schemaCheck); }, - safeSchemaChanges: (schemaCheck, _, { injector }) => { - return injector.get(SchemaCheckManager).getSafeSchemaChanges(schemaCheck); + safeSchemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { + return injector.get(SchemaCheckManager).getSafeSchemaChanges(schemaCheck, simplifyChanges); }, - breakingSchemaChanges: (schemaCheck, _, { injector }) => { - return injector.get(SchemaCheckManager).getBreakingSchemaChanges(schemaCheck); + breakingSchemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { + return injector.get(SchemaCheckManager).getBreakingSchemaChanges(schemaCheck, simplifyChanges); }, hasSchemaCompositionErrors: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getHasSchemaCompositionErrors(schemaCheck); @@ -70,7 +70,7 @@ export const SuccessfulSchemaCheck: SuccessfulSchemaCheckResolvers = { conditionalBreakingChangeMetadata: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getConditionalBreakingChangeMetadata(schemaCheck); }, - schemaChanges: (schemaCheck, _, { injector }) => { - return injector.get(SchemaCheckManager).getAllSchemaChanges(schemaCheck); + schemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { + return injector.get(SchemaCheckManager).getAllSchemaChanges(schemaCheck, simplifyChanges); }, }; diff --git a/packages/services/storage/src/schema-change-model.ts b/packages/services/storage/src/schema-change-model.ts index 02e6a8ef206..7a59ecef28e 100644 --- a/packages/services/storage/src/schema-change-model.ts +++ b/packages/services/storage/src/schema-change-model.ts @@ -1316,7 +1316,7 @@ export const HiveSchemaChangeModel = z readonly criticality: CriticalityLevel; readonly reason: string | null; readonly message: string; - readonly path: string | null; + readonly path: string; readonly approvalMetadata: SchemaCheckApprovalMetadata | null; isSafeBasedOnUsage: boolean; usageStatistics: { @@ -1350,7 +1350,7 @@ export const HiveSchemaChangeModel = z criticality: change.criticality.level, message: change.message, meta: change.meta, - path: change.path ?? null, + path: change.path ?? '', isSafeBasedOnUsage: // only breaking changes can be safe based on usage (change.criticality.level === CriticalityLevel.Breaking && diff --git a/packages/web/app/src/pages/target-proposal.tsx b/packages/web/app/src/pages/target-proposal.tsx index 19fb9c9994c..67d86e5bea6 100644 --- a/packages/web/app/src/pages/target-proposal.tsx +++ b/packages/web/app/src/pages/target-proposal.tsx @@ -126,7 +126,7 @@ const ProposalChangesQuery = graphql(/* GraphQL */ ` } schemaSDL serviceName - schemaChanges { + schemaChanges(simplifyChanges: false) { edges { node { ...ProposalOverview_ChangeFragment From 5789d9ce4c15bd17a02176a1adf2e5f4402a9006 Mon Sep 17 00:00:00 2001 From: jdolle <1841898+jdolle@users.noreply.github.com> Date: Tue, 9 Dec 2025 11:35:30 -0800 Subject: [PATCH 2/7] Remove unused imports; add changeset --- .changeset/eight-views-design.md | 6 ++++++ .../api/src/modules/schema/providers/inspector.ts | 9 +-------- .../modules/schema/providers/schema-version-helper.ts | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) create mode 100644 .changeset/eight-views-design.md diff --git a/.changeset/eight-views-design.md b/.changeset/eight-views-design.md new file mode 100644 index 00000000000..3e83710356c --- /dev/null +++ b/.changeset/eight-views-design.md @@ -0,0 +1,6 @@ +--- +'hive': minor +--- + +adjust change capture and resolvers to optionally provide a full list or the simplified list of +changes diff --git a/packages/services/api/src/modules/schema/providers/inspector.ts b/packages/services/api/src/modules/schema/providers/inspector.ts index c9928f0b33c..54ac5fc543e 100644 --- a/packages/services/api/src/modules/schema/providers/inspector.ts +++ b/packages/services/api/src/modules/schema/providers/inspector.ts @@ -10,14 +10,7 @@ import { type GraphQLSchema, } from 'graphql'; import { Injectable, Scope } from 'graphql-modules'; -import { - Change, - ChangeType, - diff, - DiffRule, - Rule, - TypeOfChangeType, -} from '@graphql-inspector/core'; +import { Change, ChangeType, diff, Rule, TypeOfChangeType } from '@graphql-inspector/core'; import { traceFn } from '@hive/service-common'; import { HiveSchemaChangeModel } from '@hive/storage'; import { Logger } from '../../shared/providers/logger'; diff --git a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts index 4e8ce4d6ea5..46e8b369765 100644 --- a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts +++ b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts @@ -1,7 +1,7 @@ import type { SchemaVersionMapper as SchemaVersion } from '../module.graphql.mappers'; import { isTypeSystemExtensionNode, print } from 'graphql'; import { Injectable, Scope } from 'graphql-modules'; -import { CriticalityLevel, DiffRule } from '@graphql-inspector/core'; +import { CriticalityLevel } from '@graphql-inspector/core'; import { mergeTypeDefs } from '@graphql-tools/merge'; import { traceFn } from '@hive/service-common'; import type { SchemaChangeType } from '@hive/storage'; From e6a477cba341d23819c344c9c2168be7477d6f55 Mon Sep 17 00:00:00 2001 From: jdolle <1841898+jdolle@users.noreply.github.com> Date: Tue, 9 Dec 2025 11:51:42 -0800 Subject: [PATCH 3/7] revert change path change --- packages/services/storage/src/schema-change-model.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/services/storage/src/schema-change-model.ts b/packages/services/storage/src/schema-change-model.ts index 7a59ecef28e..02e6a8ef206 100644 --- a/packages/services/storage/src/schema-change-model.ts +++ b/packages/services/storage/src/schema-change-model.ts @@ -1316,7 +1316,7 @@ export const HiveSchemaChangeModel = z readonly criticality: CriticalityLevel; readonly reason: string | null; readonly message: string; - readonly path: string; + readonly path: string | null; readonly approvalMetadata: SchemaCheckApprovalMetadata | null; isSafeBasedOnUsage: boolean; usageStatistics: { @@ -1350,7 +1350,7 @@ export const HiveSchemaChangeModel = z criticality: change.criticality.level, message: change.message, meta: change.meta, - path: change.path ?? '', + path: change.path ?? null, isSafeBasedOnUsage: // only breaking changes can be safe based on usage (change.criticality.level === CriticalityLevel.Breaking && From a9d92bad508dcda7514b8e305d4e4dfcbbd93019 Mon Sep 17 00:00:00 2001 From: jdolle <1841898+jdolle@users.noreply.github.com> Date: Wed, 17 Dec 2025 16:50:53 -0800 Subject: [PATCH 4/7] Only enable detailed checks when a proposal id is passed --- ...1.26T00.00.01.schema-check-purging.test.ts | 13 ++ .../api/src/modules/schema/module.graphql.ts | 126 +++--------------- .../schema/providers/models/composite.ts | 6 + .../modules/schema/providers/models/single.ts | 4 + .../schema/providers/registry-checks.ts | 4 + .../schema/providers/schema-check-manager.ts | 44 +----- .../schema/providers/schema-publisher.ts | 9 +- .../schema/providers/schema-version-helper.ts | 20 ++- .../modules/schema/resolvers/SchemaVersion.ts | 12 +- .../schema/resolvers/SuccessfulSchemaCheck.ts | 12 +- .../storage/src/schema-change-model.ts | 1 + .../web/app/src/pages/target-proposal.tsx | 2 +- 12 files changed, 75 insertions(+), 178 deletions(-) diff --git a/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts b/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts index 5c4c90edf01..c672cf6e743 100644 --- a/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts +++ b/packages/migrations/test/2024.01.26T00.00.01.schema-check-purging.test.ts @@ -83,6 +83,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -166,6 +167,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -192,6 +194,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -275,6 +278,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -301,6 +305,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -394,6 +399,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -420,6 +426,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -571,6 +578,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -597,6 +605,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -745,6 +754,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let sdlStoreCount = await db.oneFirst(sql`SELECT count(*) as total FROM sdl_store`); @@ -856,6 +866,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( @@ -1016,6 +1027,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); await storage.createSchemaCheck({ @@ -1061,6 +1073,7 @@ describe('schema check purging', async () => { schemaPolicyWarnings: null, conditionalBreakingChangeMetadata: null, serviceUrl: null, + schemaProposalId: null, }); let schemaCheckCount = await db.oneFirst( diff --git a/packages/services/api/src/modules/schema/module.graphql.ts b/packages/services/api/src/modules/schema/module.graphql.ts index 3ab65fa4037..233646e19d5 100644 --- a/packages/services/api/src/modules/schema/module.graphql.ts +++ b/packages/services/api/src/modules/schema/module.graphql.ts @@ -879,25 +879,10 @@ export default gql` """ Schema changes that were introduced in this schema version (compared to the previous version). """ - schemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection @tag(name: "public") + schemaChanges: SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection - safeSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection + breakingSchemaChanges: SchemaChangeConnection + safeSchemaChanges: SchemaChangeConnection """ GitHub metadata associated with the schema version. @@ -1276,24 +1261,9 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection - safeSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection + schemaChanges: SchemaChangeConnection @tag(name: "public") + breakingSchemaChanges: SchemaChangeConnection + safeSchemaChanges: SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection schemaPolicyErrors: SchemaPolicyWarningConnection """ @@ -1329,25 +1299,10 @@ export default gql` schemaCompositionErrors: SchemaErrorConnection @tag(name: "public") - schemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection @tag(name: "public") + schemaChanges: SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection - safeSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection + breakingSchemaChanges: SchemaChangeConnection + safeSchemaChanges: SchemaChangeConnection compositeSchemaSDL: String @tag(name: "public") supergraphSDL: String @tag(name: "public") @@ -1390,28 +1345,13 @@ export default gql` """ Breaking schema changes for this contract version. """ - breakingSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection + breakingSchemaChanges: SchemaChangeConnection """ Safe schema changes for this contract version. """ - safeSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection + safeSchemaChanges: SchemaChangeConnection - schemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection @tag(name: "public") + schemaChanges: SchemaChangeConnection @tag(name: "public") previousContractVersion: ContractVersion @tag(name: "public") previousDiffableContractVersion: ContractVersion @tag(name: "public") @@ -1472,27 +1412,12 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection @tag(name: "public") + schemaChanges: SchemaChangeConnection @tag(name: "public") """ Breaking changes can exist in an successful schema check if the check was manually approved. """ - breakingSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection - safeSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection + breakingSchemaChanges: SchemaChangeConnection + safeSchemaChanges: SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection """ Schema policy errors can exist in an successful schema check if the check was manually approved. @@ -1584,24 +1509,9 @@ export default gql` """ hasSchemaChanges: Boolean! - schemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection @tag(name: "public") - breakingSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection - safeSchemaChanges( - """ - This filters out nested changes. E.g. if a type was add, the simplified changes wouldn't include information about the fields added to that new type. - """ - simplifyChanges: Boolean = true - ): SchemaChangeConnection + schemaChanges: SchemaChangeConnection @tag(name: "public") + breakingSchemaChanges: SchemaChangeConnection + safeSchemaChanges: SchemaChangeConnection schemaPolicyWarnings: SchemaPolicyWarningConnection schemaPolicyErrors: SchemaPolicyWarningConnection diff --git a/packages/services/api/src/modules/schema/providers/models/composite.ts b/packages/services/api/src/modules/schema/providers/models/composite.ts index e0acee26e1b..e84fc679207 100644 --- a/packages/services/api/src/modules/schema/providers/models/composite.ts +++ b/packages/services/api/src/modules/schema/providers/models/composite.ts @@ -78,6 +78,7 @@ export class CompositeModel { existingSdl: contract.latestValidVersion?.compositeSchemaSdl ?? null, incomingSdl: contractCompositionResult?.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange: args.failDiffOnDangerousChange, + filterNestedChanges: true, }), }; }), @@ -103,6 +104,7 @@ export class CompositeModel { conditionalBreakingChangeDiffConfig, contracts, failDiffOnDangerousChange, + filterNestedChanges, }: { input: { sdl: string; @@ -136,6 +138,7 @@ export class CompositeModel { approvedChanges: Map | null; } > | null; + filterNestedChanges: boolean; }): Promise { const incoming: PushedCompositeSchema = { kind: 'composite', @@ -228,6 +231,7 @@ export class CompositeModel { compositionCheck.result?.fullSchemaSdl ?? compositionCheck.reason?.fullSchemaSdl ?? null, conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig, failDiffOnDangerousChange, + filterNestedChanges, }), this.checks.policyCheck({ selector, @@ -487,6 +491,7 @@ export class CompositeModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges: true, // filter because publish is never associated to schema proposals in this way. }); const contractChecks = await this.getContractChecks({ @@ -652,6 +657,7 @@ export class CompositeModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges: true, // filter because deletes are never associated with schema proposals in this way. }); const contractChecks = await this.getContractChecks({ diff --git a/packages/services/api/src/modules/schema/providers/models/single.ts b/packages/services/api/src/modules/schema/providers/models/single.ts index a7f6ad6e724..a39099aafea 100644 --- a/packages/services/api/src/modules/schema/providers/models/single.ts +++ b/packages/services/api/src/modules/schema/providers/models/single.ts @@ -43,6 +43,7 @@ export class SingleModel { approvedChanges, conditionalBreakingChangeDiffConfig, failDiffOnDangerousChange, + filterNestedChanges, }: { input: { sdl: string; @@ -68,6 +69,7 @@ export class SingleModel { approvedChanges: Map; conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig; failDiffOnDangerousChange: boolean; + filterNestedChanges: boolean; }): Promise { const incoming: SingleSchema = { kind: 'single', @@ -130,6 +132,7 @@ export class SingleModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges, }), this.checks.policyCheck({ selector, @@ -267,6 +270,7 @@ export class SingleModel { existingSdl: previousVersionSdl, incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null, failDiffOnDangerousChange, + filterNestedChanges: true, // publish is never associated with schema proposals in this way. So always show the minimal changeset. }), ]); diff --git a/packages/services/api/src/modules/schema/providers/registry-checks.ts b/packages/services/api/src/modules/schema/providers/registry-checks.ts index d415b4ffa18..59e67d3b8c8 100644 --- a/packages/services/api/src/modules/schema/providers/registry-checks.ts +++ b/packages/services/api/src/modules/schema/providers/registry-checks.ts @@ -426,6 +426,10 @@ export class RegistryChecks { /** Settings for fetching conditional breaking changes. */ conditionalBreakingChangeConfig: null | ConditionalBreakingChangeDiffConfig; failDiffOnDangerousChange: null | boolean; + /** + * Set to true to reduce the number of changes to only what's relevant to the user. + * Use false for schema proposals in order to capture every single change record for the patch function. + */ filterNestedChanges: boolean; }) { let existingSchema: GraphQLSchema | null = null; diff --git a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts index 63acb920266..ffa2e41e5b9 100644 --- a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts +++ b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts @@ -36,28 +36,15 @@ export class SchemaCheckManager { return !!(schemaCheck.breakingSchemaChanges?.length || schemaCheck.safeSchemaChanges?.length); } - getSafeSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { + getSafeSchemaChanges(schemaCheck: SchemaCheck) { if (!schemaCheck.safeSchemaChanges?.length) { return null; } - if (simplifyChanges) { - /** - * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule - * doesn't need more information than the change type and path fields. - */ - return DiffRule.simplifyChanges({ - changes: schemaCheck.safeSchemaChanges as any, - oldSchema: undefined as any, - newSchema: undefined as any, - config: undefined, - }) as unknown as typeof schemaCheck.safeSchemaChanges; - } - return schemaCheck.safeSchemaChanges; } - getAllSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { + getAllSchemaChanges(schemaCheck: SchemaCheck) { if (!schemaCheck.safeSchemaChanges?.length && !schemaCheck.breakingSchemaChanges?.length) { return null; } @@ -66,40 +53,15 @@ export class SchemaCheckManager { ...(schemaCheck.breakingSchemaChanges ?? []), ...(schemaCheck.safeSchemaChanges ?? []), ]; - if (simplifyChanges) { - /** - * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule - * doesn't need more information than the change type and path fields. - */ - return DiffRule.simplifyChanges({ - changes: changes as any, - oldSchema: undefined as any, - newSchema: undefined as any, - config: undefined, - }) as unknown as typeof changes; - } return changes; } - getBreakingSchemaChanges(schemaCheck: SchemaCheck, simplifyChanges: boolean = true) { + getBreakingSchemaChanges(schemaCheck: SchemaCheck) { if (!schemaCheck.breakingSchemaChanges?.length) { return null; } - if (simplifyChanges) { - /** - * @todo modify the simplifyChanges rule to loosen the argument requirements. This rule - * doesn't need more information than the change type and path fields. - */ - return DiffRule.simplifyChanges({ - changes: schemaCheck.breakingSchemaChanges as any, - oldSchema: undefined as any, - newSchema: undefined as any, - config: undefined, - }); - } - return schemaCheck.breakingSchemaChanges; } diff --git a/packages/services/api/src/modules/schema/providers/schema-publisher.ts b/packages/services/api/src/modules/schema/providers/schema-publisher.ts index d7a343d40e8..308b8257d08 100644 --- a/packages/services/api/src/modules/schema/providers/schema-publisher.ts +++ b/packages/services/api/src/modules/schema/providers/schema-publisher.ts @@ -492,7 +492,6 @@ export class SchemaPublisher { } if (github != null) { - // @todo should proposals do anything special here? const result = await this.createGithubCheckRunStartForSchemaCheck({ organization, project, @@ -623,6 +622,7 @@ export class SchemaPublisher { conditionalBreakingChangeDiffConfig: conditionalBreakingChangeConfiguration?.conditionalBreakingChangeDiffConfig ?? null, failDiffOnDangerousChange, + filterNestedChanges: !input.schemaProposalId, }); break; case ProjectType.FEDERATION: @@ -668,6 +668,7 @@ export class SchemaPublisher { conditionalBreakingChangeDiffConfig: conditionalBreakingChangeConfiguration?.conditionalBreakingChangeDiffConfig ?? null, failDiffOnDangerousChange, + filterNestedChanges: !input.schemaProposalId, }); break; default: @@ -732,7 +733,7 @@ export class SchemaPublisher { breakingSchemaChanges: contract.schemaChanges?.breaking ?? null, safeSchemaChanges: contract.schemaChanges?.safe ?? null, })) ?? null, - schemaProposalId: input.schemaProposalId, + schemaProposalId: input.schemaProposalId ?? null, }); this.logger.info('created failed schema check. (schemaCheckId=%s)', schemaCheck.id); } else if (checkResult.conclusion === SchemaCheckConclusion.Success) { @@ -779,7 +780,7 @@ export class SchemaPublisher { breakingSchemaChanges: contract.schemaChanges?.breaking ?? null, safeSchemaChanges: contract.schemaChanges?.safe ?? null, })) ?? null, - schemaProposalId: input.schemaProposalId, + schemaProposalId: input.schemaProposalId ?? null, }); this.logger.info('created successful schema check. (schemaCheckId=%s)', schemaCheck.id); } else if (checkResult.conclusion === SchemaCheckConclusion.Skip) { @@ -858,7 +859,7 @@ export class SchemaPublisher { })), ) : null, - schemaProposalId: input.schemaProposalId, + schemaProposalId: input.schemaProposalId ?? null, }); this.logger.info('created skipped schema check. (schemaCheckId=%s)', schemaCheck.id); } diff --git a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts index 46e8b369765..20ea25360b7 100644 --- a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts +++ b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts @@ -167,15 +167,15 @@ export class SchemaVersionHelper { 'hive.safe-changes.count': changes?.safe?.length, }), }) - @cache<{ version: SchemaVersion; simplified: boolean }>( - ({ version, simplified = true }) => `${version.id}${simplified ? '/simple' : ''}`, + @cache<{ version: SchemaVersion; simplified?: boolean }>( + ({ version, simplified = true }) => `${version.id}${simplified ? '' : '/detailed'}`, ) private async _getSchemaChanges({ version: schemaVersion, simplified, }: { version: SchemaVersion; - simplified: boolean; + simplified?: boolean; }) { if (!schemaVersion.isComposable) { return null; @@ -251,7 +251,7 @@ export class SchemaVersionHelper { filterOutFederationChanges: project.type === ProjectType.FEDERATION, conditionalBreakingChangeConfig: null, failDiffOnDangerousChange, - filterNestedChanges: simplified, + filterNestedChanges: simplified ?? true, }); if (diffCheck.status === 'skipped') { @@ -285,34 +285,30 @@ export class SchemaVersionHelper { }); } - async getBreakingSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + async getBreakingSchemaChanges(schemaVersion: SchemaVersion) { const changes = await this._getSchemaChanges({ version: schemaVersion, - simplified: simplifyChanges, }); return changes?.breaking ?? null; } - async getSafeSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + async getSafeSchemaChanges(schemaVersion: SchemaVersion) { const changes = await this._getSchemaChanges({ version: schemaVersion, - simplified: simplifyChanges, }); return changes?.safe ?? null; } - async getAllSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + async getAllSchemaChanges(schemaVersion: SchemaVersion) { const changes = await this._getSchemaChanges({ version: schemaVersion, - simplified: simplifyChanges, }); return changes?.all ?? null; } - async getHasSchemaChanges(schemaVersion: SchemaVersion, simplifyChanges: boolean) { + async getHasSchemaChanges(schemaVersion: SchemaVersion) { const changes = await this._getSchemaChanges({ version: schemaVersion, - simplified: simplifyChanges, }); return !!changes?.breaking?.length || !!changes?.safe?.length; } diff --git a/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts b/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts index 566126158d5..7e5c66421a3 100644 --- a/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts +++ b/packages/services/api/src/modules/schema/resolvers/SchemaVersion.ts @@ -70,14 +70,14 @@ export const SchemaVersion: SchemaVersionResolvers = { schemaCompositionErrors: async (version, _, { injector }) => { return injector.get(SchemaVersionHelper).getSchemaCompositionErrors(version); }, - breakingSchemaChanges: async (version, { simplifyChanges }, { injector }) => { - return injector.get(SchemaVersionHelper).getBreakingSchemaChanges(version, simplifyChanges); + breakingSchemaChanges: async (version, _, { injector }) => { + return injector.get(SchemaVersionHelper).getBreakingSchemaChanges(version); }, - safeSchemaChanges: async (version, { simplifyChanges }, { injector }) => { - return injector.get(SchemaVersionHelper).getSafeSchemaChanges(version, simplifyChanges); + safeSchemaChanges: async (version, _, { injector }) => { + return injector.get(SchemaVersionHelper).getSafeSchemaChanges(version); }, - schemaChanges: async (version, { simplifyChanges }, { injector }) => { - return injector.get(SchemaVersionHelper).getAllSchemaChanges(version, simplifyChanges); + schemaChanges: async (version, _, { injector }) => { + return injector.get(SchemaVersionHelper).getAllSchemaChanges(version); }, supergraph: async (version, _, { injector }) => { return injector.get(SchemaVersionHelper).getSupergraphSdl(version); diff --git a/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts b/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts index 9715780a9e2..1e4cae9781b 100644 --- a/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts +++ b/packages/services/api/src/modules/schema/resolvers/SuccessfulSchemaCheck.ts @@ -8,11 +8,11 @@ export const SuccessfulSchemaCheck: SuccessfulSchemaCheckResolvers = { schemaVersion: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getSchemaVersion(schemaCheck); }, - safeSchemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { - return injector.get(SchemaCheckManager).getSafeSchemaChanges(schemaCheck, simplifyChanges); + safeSchemaChanges: (schemaCheck, _, { injector }) => { + return injector.get(SchemaCheckManager).getSafeSchemaChanges(schemaCheck); }, - breakingSchemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { - return injector.get(SchemaCheckManager).getBreakingSchemaChanges(schemaCheck, simplifyChanges); + breakingSchemaChanges: (schemaCheck, _, { injector }) => { + return injector.get(SchemaCheckManager).getBreakingSchemaChanges(schemaCheck); }, hasSchemaCompositionErrors: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getHasSchemaCompositionErrors(schemaCheck); @@ -70,7 +70,7 @@ export const SuccessfulSchemaCheck: SuccessfulSchemaCheckResolvers = { conditionalBreakingChangeMetadata: (schemaCheck, _, { injector }) => { return injector.get(SchemaCheckManager).getConditionalBreakingChangeMetadata(schemaCheck); }, - schemaChanges: (schemaCheck, { simplifyChanges }, { injector }) => { - return injector.get(SchemaCheckManager).getAllSchemaChanges(schemaCheck, simplifyChanges); + schemaChanges: (schemaCheck, _, { injector }) => { + return injector.get(SchemaCheckManager).getAllSchemaChanges(schemaCheck); }, }; diff --git a/packages/services/storage/src/schema-change-model.ts b/packages/services/storage/src/schema-change-model.ts index 02e6a8ef206..f1c625325ec 100644 --- a/packages/services/storage/src/schema-change-model.ts +++ b/packages/services/storage/src/schema-change-model.ts @@ -1414,6 +1414,7 @@ const SchemaCheckSharedOutputFields = { serviceUrl: z.string().nullable(), targetId: z.string(), schemaVersionId: z.string().nullable(), + schemaProposalId: z.string().nullable(), meta: z .object({ author: z.string(), diff --git a/packages/web/app/src/pages/target-proposal.tsx b/packages/web/app/src/pages/target-proposal.tsx index 67d86e5bea6..19fb9c9994c 100644 --- a/packages/web/app/src/pages/target-proposal.tsx +++ b/packages/web/app/src/pages/target-proposal.tsx @@ -126,7 +126,7 @@ const ProposalChangesQuery = graphql(/* GraphQL */ ` } schemaSDL serviceName - schemaChanges(simplifyChanges: false) { + schemaChanges { edges { node { ...ProposalOverview_ChangeFragment From 84201afa720bdebac67a98d45b26e731ca2a82ca Mon Sep 17 00:00:00 2001 From: jdolle <1841898+jdolle@users.noreply.github.com> Date: Wed, 17 Dec 2025 17:02:02 -0800 Subject: [PATCH 5/7] lint --- .../api/src/modules/schema/providers/schema-check-manager.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts index ffa2e41e5b9..7b5de099b12 100644 --- a/packages/services/api/src/modules/schema/providers/schema-check-manager.ts +++ b/packages/services/api/src/modules/schema/providers/schema-check-manager.ts @@ -3,7 +3,6 @@ import type { SuccessfulSchemaCheckMapper, } from '../module.graphql.mappers'; import { Injectable, Scope } from 'graphql-modules'; -import { DiffRule } from '@graphql-inspector/core'; import { Storage } from '../../shared/providers/storage'; import { formatNumber } from '../lib/number-formatting'; import { SchemaManager } from './schema-manager'; From caf7a36182c337143a6c153b84be3f0c4583bf9c Mon Sep 17 00:00:00 2001 From: jdolle <1841898+jdolle@users.noreply.github.com> Date: Wed, 17 Dec 2025 17:15:21 -0800 Subject: [PATCH 6/7] revert changes to SchemaVersionHelper._getSchemaChanges --- .../schema/providers/schema-version-helper.ts | 32 +++++-------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts index 20ea25360b7..249dca50f2b 100644 --- a/packages/services/api/src/modules/schema/providers/schema-version-helper.ts +++ b/packages/services/api/src/modules/schema/providers/schema-version-helper.ts @@ -156,7 +156,7 @@ export class SchemaVersionHelper { } @traceFn('SchemaVersionHelper._getSchemaChanges', { - initAttributes: ({ version: input }) => ({ + initAttributes: input => ({ 'hive.target.id': input.targetId, 'hive.organization.id': input.organizationId, 'hive.project.id': input.projectId, @@ -167,16 +167,8 @@ export class SchemaVersionHelper { 'hive.safe-changes.count': changes?.safe?.length, }), }) - @cache<{ version: SchemaVersion; simplified?: boolean }>( - ({ version, simplified = true }) => `${version.id}${simplified ? '' : '/detailed'}`, - ) - private async _getSchemaChanges({ - version: schemaVersion, - simplified, - }: { - version: SchemaVersion; - simplified?: boolean; - }) { + @cache(version => version.id) + private async _getSchemaChanges(schemaVersion: SchemaVersion) { if (!schemaVersion.isComposable) { return null; } @@ -251,7 +243,7 @@ export class SchemaVersionHelper { filterOutFederationChanges: project.type === ProjectType.FEDERATION, conditionalBreakingChangeConfig: null, failDiffOnDangerousChange, - filterNestedChanges: simplified ?? true, + filterNestedChanges: true, }); if (diffCheck.status === 'skipped') { @@ -286,30 +278,22 @@ export class SchemaVersionHelper { } async getBreakingSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges({ - version: schemaVersion, - }); + const changes = await this._getSchemaChanges(schemaVersion); return changes?.breaking ?? null; } async getSafeSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges({ - version: schemaVersion, - }); + const changes = await this._getSchemaChanges(schemaVersion); return changes?.safe ?? null; } async getAllSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges({ - version: schemaVersion, - }); + const changes = await this._getSchemaChanges(schemaVersion); return changes?.all ?? null; } async getHasSchemaChanges(schemaVersion: SchemaVersion) { - const changes = await this._getSchemaChanges({ - version: schemaVersion, - }); + const changes = await this._getSchemaChanges(schemaVersion); return !!changes?.breaking?.length || !!changes?.safe?.length; } From eb42eb93dd38d8485006321d27473dd678c7f320 Mon Sep 17 00:00:00 2001 From: jdolle <1841898+jdolle@users.noreply.github.com> Date: Wed, 17 Dec 2025 17:33:33 -0800 Subject: [PATCH 7/7] Fix root type changed models --- .../storage/src/schema-change-model.ts | 12 +++++------ pnpm-lock.yaml | 20 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/services/storage/src/schema-change-model.ts b/packages/services/storage/src/schema-change-model.ts index f1c625325ec..5ccd95fadd2 100644 --- a/packages/services/storage/src/schema-change-model.ts +++ b/packages/services/storage/src/schema-change-model.ts @@ -1012,16 +1012,16 @@ export const ObjectTypeInterfaceRemovedModel = implement().with({ type: SchemaQueryTypeChangedLiteral, meta: z.object({ - oldQueryTypeName: z.string(), - newQueryTypeName: z.string(), + oldQueryTypeName: z.string().nullable(), + newQueryTypeName: z.string().nullable(), }), }); export const SchemaMutationTypeChangedModel = implement().with({ type: SchemaMutationTypeChangedLiteral, meta: z.object({ - oldMutationTypeName: z.string(), - newMutationTypeName: z.string(), + oldMutationTypeName: z.string().nullable(), + newMutationTypeName: z.string().nullable(), }), }); @@ -1029,8 +1029,8 @@ export const SchemaSubscriptionTypeChangedModel = implement().with({ type: SchemaSubscriptionTypeChangedLiteral, meta: z.object({ - oldSubscriptionTypeName: z.string(), - newSubscriptionTypeName: z.string(), + oldSubscriptionTypeName: z.string().nullable(), + newSubscriptionTypeName: z.string().nullable(), }), }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dac413729ac..c996a011222 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -19669,8 +19669,8 @@ snapshots: dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sso-oidc': 3.596.0(@aws-sdk/client-sts@3.596.0) - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sso-oidc': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/core': 3.592.0 '@aws-sdk/credential-provider-node': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0) '@aws-sdk/middleware-host-header': 3.577.0 @@ -19822,11 +19822,11 @@ snapshots: transitivePeerDependencies: - aws-crt - '@aws-sdk/client-sso-oidc@3.596.0(@aws-sdk/client-sts@3.596.0)': + '@aws-sdk/client-sso-oidc@3.596.0': dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/core': 3.592.0 '@aws-sdk/credential-provider-node': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0) '@aws-sdk/middleware-host-header': 3.577.0 @@ -19865,7 +19865,6 @@ snapshots: '@smithy/util-utf8': 3.0.0 tslib: 2.8.1 transitivePeerDependencies: - - '@aws-sdk/client-sts' - aws-crt '@aws-sdk/client-sso-oidc@3.723.0(@aws-sdk/client-sts@3.723.0)': @@ -20085,11 +20084,11 @@ snapshots: transitivePeerDependencies: - aws-crt - '@aws-sdk/client-sts@3.596.0': + '@aws-sdk/client-sts@3.596.0(@aws-sdk/client-sso-oidc@3.596.0)': dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sso-oidc': 3.596.0(@aws-sdk/client-sts@3.596.0) + '@aws-sdk/client-sso-oidc': 3.596.0 '@aws-sdk/core': 3.592.0 '@aws-sdk/credential-provider-node': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0) '@aws-sdk/middleware-host-header': 3.577.0 @@ -20128,6 +20127,7 @@ snapshots: '@smithy/util-utf8': 3.0.0 tslib: 2.8.1 transitivePeerDependencies: + - '@aws-sdk/client-sso-oidc' - aws-crt '@aws-sdk/client-sts@3.723.0': @@ -20359,7 +20359,7 @@ snapshots: '@aws-sdk/credential-provider-ini@3.596.0(@aws-sdk/client-sso-oidc@3.596.0)(@aws-sdk/client-sts@3.596.0)': dependencies: - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/credential-provider-env': 3.587.0 '@aws-sdk/credential-provider-http': 3.596.0 '@aws-sdk/credential-provider-process': 3.587.0 @@ -20606,7 +20606,7 @@ snapshots: '@aws-sdk/credential-provider-web-identity@3.587.0(@aws-sdk/client-sts@3.596.0)': dependencies: - '@aws-sdk/client-sts': 3.596.0 + '@aws-sdk/client-sts': 3.596.0(@aws-sdk/client-sso-oidc@3.596.0) '@aws-sdk/types': 3.577.0 '@smithy/property-provider': 3.1.11 '@smithy/types': 3.7.2 @@ -20995,7 +20995,7 @@ snapshots: '@aws-sdk/token-providers@3.587.0(@aws-sdk/client-sso-oidc@3.596.0)': dependencies: - '@aws-sdk/client-sso-oidc': 3.596.0(@aws-sdk/client-sts@3.596.0) + '@aws-sdk/client-sso-oidc': 3.596.0 '@aws-sdk/types': 3.577.0 '@smithy/property-provider': 3.1.11 '@smithy/shared-ini-file-loader': 3.1.12