diff --git a/.changeset/contract-changes-github-check.md b/.changeset/contract-changes-github-check.md new file mode 100644 index 00000000000..192003ba472 --- /dev/null +++ b/.changeset/contract-changes-github-check.md @@ -0,0 +1,5 @@ +--- +'hive': patch +--- + +Include contract changes in the GitHub CI schema-check summary. Previously, a `schema:check --github` run on a composite/federation project that changed only a contract (while the core composed schema was unchanged) was reported as "No changes". The summary now reports the changed contracts and their changes, and "No changes" is only shown when neither the core schema nor any contract changed. diff --git a/packages/services/api/src/modules/schema/providers/schema-publisher.spec.ts b/packages/services/api/src/modules/schema/providers/schema-publisher.spec.ts index f7bb8e1663e..8c6911fa9c5 100644 --- a/packages/services/api/src/modules/schema/providers/schema-publisher.spec.ts +++ b/packages/services/api/src/modules/schema/providers/schema-publisher.spec.ts @@ -1,6 +1,87 @@ import { describe, expect, test } from 'vitest'; import { CriticalityLevel } from '@graphql-inspector/core'; -import { changesToMarkdown, type MarkdownSchemaChange } from './schema-publisher'; +import { + buildSchemaCheckSuccessGithubOutput, + changesToMarkdown, + type MarkdownSchemaChange, +} from './schema-publisher'; + +describe('buildSchemaCheckSuccessGithubOutput', () => { + test('reports a contract-only change instead of "No changes" (#6954)', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [], + contractChanges: [ + { + contractName: 'public', + changes: [{ criticality: CriticalityLevel.NonBreaking, message: 'Field Query.b added' }], + }, + ], + }); + + expect(result.title).not.toBe('No changes'); + expect(result.title).toBe('No blocking changes'); + expect(result.summary).toContain('public'); + expect(result.summary).toContain('Field Query.b added'); + }); + + test('still reports "No changes" when neither core nor contracts changed', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [], + contractChanges: [], + }); + + expect(result.title).toBe('No changes'); + expect(result.summary).toBe('No changes detected'); + expect(result.shortSummaryFallback).toBe('No changes detected'); + }); + + test('treats null core changes and null contract changes as "No changes"', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: null, + contractChanges: null, + }); + + expect(result.title).toBe('No changes'); + }); + + test('treats contracts with empty change lists as "No changes"', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [], + contractChanges: [{ contractName: 'public', changes: [] }], + }); + + expect(result.title).toBe('No changes'); + }); + + test('preserves existing behavior for core schema changes with no contracts', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [{ criticality: CriticalityLevel.NonBreaking, message: 'Field Query.a added' }], + contractChanges: null, + }); + + expect(result.title).toBe('No blocking changes'); + expect(result.summary).toContain('Field Query.a added'); + // No contract section when there are no contract changes. + expect(result.summary).not.toContain('Contract "'); + }); + + test('lists both core and per-contract changes when both are present', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [{ criticality: CriticalityLevel.NonBreaking, message: 'Field Query.a added' }], + contractChanges: [ + { + contractName: 'public', + changes: [{ criticality: CriticalityLevel.NonBreaking, message: 'Field Query.b added' }], + }, + ], + }); + + expect(result.title).toBe('No blocking changes'); + expect(result.summary).toContain('Field Query.a added'); + expect(result.summary).toContain('## Contract "public"'); + expect(result.summary).toContain('Field Query.b added'); + }); +}); describe('changesToMarkdown', () => { test('groups changes and includes their status', () => { 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 7cdc711d658..5da1b9aa564 100644 --- a/packages/services/api/src/modules/schema/providers/schema-publisher.ts +++ b/packages/services/api/src/modules/schema/providers/schema-publisher.ts @@ -968,6 +968,13 @@ export class SchemaPublisher { organization, conclusion: checkResult.conclusion, changes: checkResult.state?.schemaChanges?.all ?? null, + contractChanges: + checkResult.state.contracts + ?.map(contract => ({ + contractName: contract.contractName, + changes: contract.schemaChanges?.all ?? [], + })) + .filter(contract => contract.changes.length > 0) ?? null, breakingChanges: checkResult.state?.schemaChanges?.breaking ?? null, warnings: checkResult.state?.schemaPolicyWarnings ?? null, compositionErrors: null, @@ -992,6 +999,7 @@ export class SchemaPublisher { ...(checkResult.state.schemaChanges?.breaking ?? []), ...(checkResult.state.schemaChanges?.safe ?? []), ], + contractChanges: null, breakingChanges: checkResult.state.schemaChanges?.breaking ?? [], compositionErrors: checkResult.state.composition.errors ?? [], warnings: checkResult.state.schemaPolicy?.warnings ?? [], @@ -1022,6 +1030,7 @@ export class SchemaPublisher { organization, conclusion: SchemaCheckConclusion.Success, changes: null, + contractChanges: null, breakingChanges: null, warnings: null, compositionErrors: null, @@ -1039,6 +1048,7 @@ export class SchemaPublisher { organization, conclusion: SchemaCheckConclusion.Failure, changes: null, + contractChanges: null, breakingChanges: null, compositionErrors: latestVersion.version.schemaCompositionErrors, warnings: null, @@ -3122,6 +3132,7 @@ export class SchemaPublisher { conclusion: SchemaCheckConclusion; warnings: SchemaCheckWarning[] | null; changes: Array | null; + contractChanges: Array<{ contractName: string; changes: Array }> | null; breakingChanges: Array | null; compositionErrors: Array<{ message: string; @@ -3138,15 +3149,10 @@ export class SchemaPublisher { let shortSummaryFallback: string; if (conclusion === SchemaCheckConclusion.Success) { - if (!changes || changes.length === 0) { - title = 'No changes'; - summary = 'No changes detected'; - shortSummaryFallback = summary; - } else { - title = 'No blocking changes'; - summary = changesToMarkdown(changes); - shortSummaryFallback = changesToMarkdown(changes, false); - } + ({ title, summary, shortSummaryFallback } = buildSchemaCheckSuccessGithubOutput({ + changes, + contractChanges: args.contractChanges, + })); } else { const total = (compositionErrors?.length ?? 0) + (breakingChanges?.length ?? 0) + (errors?.length ?? 0); @@ -3539,6 +3545,38 @@ function writeChanges( } } +export function buildSchemaCheckSuccessGithubOutput(input: { + changes: Array | null; + contractChanges: Array<{ contractName: string; changes: Array }> | null; +}): { title: string; summary: string; shortSummaryFallback: string } { + const coreChanges = input.changes ?? []; + const contractChanges = input.contractChanges?.filter(contract => contract.changes.length) ?? []; + + if (coreChanges.length === 0 && contractChanges.length === 0) { + const summary = 'No changes detected'; + return { title: 'No changes', summary, shortSummaryFallback: summary }; + } + + const buildSummary = (printListOfChanges: boolean) => + [ + coreChanges.length ? changesToMarkdown(coreChanges, printListOfChanges) : null, + ...contractChanges.map(contract => + [ + `## Contract "${contract.contractName}"`, + changesToMarkdown(contract.changes, printListOfChanges), + ].join('\n'), + ), + ] + .filter(Boolean) + .join('\n\n'); + + return { + title: 'No blocking changes', + summary: buildSummary(true), + shortSummaryFallback: buildSummary(false), + }; +} + function buildGitHubActionCheckName(input: { targetName: string; projectName: string;