diff --git a/src/gate/schema-change.test.ts b/src/gate/schema-change.test.ts index 0b31abe..68cc0fa 100644 --- a/src/gate/schema-change.test.ts +++ b/src/gate/schema-change.test.ts @@ -25,6 +25,28 @@ describe("gateSchemaChange", () => { expect(gateSchemaChange({ changed: false })).toBeNull(); }); + it("passes once someone has approved the migration", () => { + expect(gateSchemaChange({ changed: true, approved: true })).toBeNull(); + }); + + it("still blocks a schema change nobody has approved", () => { + expect(gateSchemaChange({ changed: true, approved: false })?.conclusion).toBe( + "failure", + ); + }); + + // An API that predates the field sends no `approved` at all. Reading that as + // approved would drop the gate for every repo on an older deployment. + it("blocks when the API sends no approval field", () => { + expect(gateSchemaChange({ changed: true })?.conclusion).toBe("failure"); + }); + + it("names the tool that clears the gate", () => { + expect(gateSchemaChange({ changed: true })!.message).toContain( + "approve_schema_change", + ); + }); + it("passes when the API returned no schema-change signal", () => { expect(gateSchemaChange(null)).toBeNull(); expect(gateSchemaChange(undefined)).toBeNull(); diff --git a/src/gate/schema-change.ts b/src/gate/schema-change.ts index 8d6de8f..213c078 100644 --- a/src/gate/schema-change.ts +++ b/src/gate/schema-change.ts @@ -11,6 +11,13 @@ import { resolveVerdict } from "./policy.ts"; /** The schema diff the API returns on a run (`runMetadata.schemaChange`). */ export interface SchemaChangeSignal { changed: boolean; + /** + * Whether someone validated this pull request's migration and the schema has + * not moved since (Site#3289). Absent on a Site deployment that predates the + * field, and read as unapproved — reading an absent field as approved would + * drop the gate for every repo on an older API. + */ + approved?: boolean; } export interface SchemaGateResult { @@ -20,20 +27,25 @@ export interface SchemaGateResult { } const MESSAGE = - "This PR changes the database schema — validate the migration before merge. " + - "To stop schema changes from blocking this repo, set the schema-drift check " + - "to warn or off in CI settings."; + "This PR changes the database schema. Review the migration, then call " + + "approve_schema_change({ runId, reason }) and re-run CI; approving does not " + + "change a check that has already reported. To stop schema changes blocking " + + "this repo, set the schema-drift check to warn or off in CI settings."; /** * Decide the schema-change gate from the API's schema diff and the repo policy. * Returns the check outcome when the PR changes the schema and the policy - * surfaces it, or `null` when there is no change or the policy is `off`. + * surfaces it, or `null` when there is no change, the migration is approved, or + * the policy is `off`. */ export function gateSchemaChange( schemaChange: SchemaChangeSignal | null | undefined, config: RepoPolicyConfig = {}, ): SchemaGateResult | null { if (!schemaChange?.changed) return null; + // An approved migration has already had the human eyeball this gate exists to + // force, so it stops blocking without the repo having to soften the policy. + if (schemaChange.approved) return null; const { conclusion, surfaced } = resolveVerdict( { condition: "schema-drift", verdictClass: "finding" }, config, diff --git a/src/main.ts b/src/main.ts index c3b9dab..04d26b6 100644 --- a/src/main.ts +++ b/src/main.ts @@ -289,8 +289,12 @@ async function runInCI( regressedCount: reportContext.comparison?.regressed.length ?? 0, newQueryCount: reportContext.comparison?.newQueries.length ?? 0, indexedNewQueryCount: eligible.length, + // An approved migration is not drift the roll-up should count: a + // person has already validated it, so the condition is satisfied + // rather than softened. schemaChanged: - reportContext.runMetadata?.schemaChange?.changed === true, + reportContext.runMetadata?.schemaChange?.changed === true && + reportContext.runMetadata.schemaChange.approved !== true, untestedDataAccessFileCount: reportContext.testPresenceVerdict?.dataAccessFiles.length ?? 0, regressionThreshold: config.regressionThreshold, diff --git a/src/reporters/site-api.ts b/src/reporters/site-api.ts index 9edb222..f567171 100644 --- a/src/reporters/site-api.ts +++ b/src/reporters/site-api.ts @@ -166,6 +166,12 @@ export interface CiRunMetadata { changed: boolean; operations: Op[]; changes?: NamedSchemaChange[]; + /** + * Whether someone validated this pull request's migration and the schema + * has not moved since (Site#3289). Absent on a Site API that predates the + * field, and read as unapproved so the gate keeps blocking. + */ + approved?: boolean; } | null; }