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 new file mode 100644 index 00000000000..84d5d6a6397 --- /dev/null +++ b/packages/services/api/src/modules/schema/providers/schema-publisher.spec.ts @@ -0,0 +1,83 @@ +import 'reflect-metadata'; +import type { SchemaChangeType } from '@hive/storage'; +import { describe, expect, test } from 'vitest'; +import { buildSchemaCheckSuccessGithubOutput } from './schema-publisher'; + +// The helper only reads `.length`, the contract name and passes change arrays to +// the injected renderer, so a minimal fake change + a simple renderer are enough. +const change = (message: string): SchemaChangeType => ({ message }) as unknown as SchemaChangeType; +const renderChanges = (changes: ReadonlyArray): string => + changes.map(c => (c as unknown as { message: string }).message).join('\n'); + +describe('buildSchemaCheckSuccessGithubOutput', () => { + test('reports a contract-only change instead of "No changes" (#6954)', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [], + contractChanges: [{ contractName: 'public', changes: [change('Field Query.b added')] }], + renderChanges, + }); + + expect(result.title).not.toBe('No changes'); + expect(result.title).toBe('No breaking 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: [], + renderChanges, + }); + + 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, + renderChanges, + }); + + expect(result.title).toBe('No changes'); + }); + + test('treats contracts with empty change lists as "No changes"', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [], + contractChanges: [{ contractName: 'public', changes: [] }], + renderChanges, + }); + + expect(result.title).toBe('No changes'); + }); + + test('preserves existing behavior for core schema changes with no contracts', () => { + const result = buildSchemaCheckSuccessGithubOutput({ + changes: [change('Field Query.a added')], + contractChanges: null, + renderChanges, + }); + + expect(result.title).toBe('No breaking 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: [change('Field Query.a added')], + contractChanges: [{ contractName: 'public', changes: [change('Field Query.b added')] }], + renderChanges, + }); + + expect(result.title).toBe('No breaking changes'); + expect(result.summary).toContain('Field Query.a added'); + expect(result.summary).toContain('## Contract "public"'); + expect(result.summary).toContain('Field Query.b added'); + }); +}); 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 b184c1c028f..328c3c80ebf 100644 --- a/packages/services/api/src/modules/schema/providers/schema-publisher.ts +++ b/packages/services/api/src/modules/schema/providers/schema-publisher.ts @@ -952,6 +952,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, @@ -976,6 +983,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 ?? [], @@ -1006,6 +1014,7 @@ export class SchemaPublisher { organization, conclusion: SchemaCheckConclusion.Success, changes: null, + contractChanges: null, breakingChanges: null, warnings: null, compositionErrors: null, @@ -1023,6 +1032,7 @@ export class SchemaPublisher { organization, conclusion: SchemaCheckConclusion.Failure, changes: null, + contractChanges: null, breakingChanges: null, compositionErrors: latestVersion.version.schemaCompositionErrors, warnings: null, @@ -3082,6 +3092,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; @@ -3098,15 +3109,12 @@ 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 breaking changes'; - summary = this.changesToMarkdown(changes); - shortSummaryFallback = this.changesToMarkdown(changes, false); - } + ({ title, summary, shortSummaryFallback } = buildSchemaCheckSuccessGithubOutput({ + changes, + contractChanges: args.contractChanges, + renderChanges: (changesToRender, printListOfChanges) => + this.changesToMarkdown(changesToRender, printListOfChanges), + })); } else { const total = (compositionErrors?.length ?? 0) + (breakingChanges?.length ?? 0) + (errors?.length ?? 0); @@ -3489,6 +3497,54 @@ function writeChanges( } } +/** + * Builds the title and summary for a *successful* GitHub schema-check run. + * + * A successful check can still contain changes - either to the core schema or + * to one or more contracts. "No changes" must only be reported when there are + * no core changes AND no contract changes; otherwise the summary lists the core + * changes followed by a per-contract section, so a contract-only change is no + * longer misreported as "No changes" (#6954). + * + * Pure function (the markdown renderer is injected) so it can be unit-tested + * without constructing the full SchemaPublisher dependency graph. + */ +export function buildSchemaCheckSuccessGithubOutput(input: { + changes: Array | null; + contractChanges: Array<{ contractName: string; changes: Array }> | null; + renderChanges: ( + changes: ReadonlyArray, + printListOfChanges?: boolean, + ) => string; +}): { 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 ? input.renderChanges(coreChanges, printListOfChanges) : null, + ...contractChanges.map(contract => + [ + `## Contract "${contract.contractName}"`, + input.renderChanges(contract.changes, printListOfChanges), + ].join('\n'), + ), + ] + .filter(Boolean) + .join('\n\n'); + + return { + title: 'No breaking changes', + summary: buildSummary(true), + shortSummaryFallback: buildSummary(false), + }; +} + function buildGitHubActionCheckName(input: { targetName: string; projectName: string;