Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/gate/schema-change.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
20 changes: 16 additions & 4 deletions src/gate/schema-change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions src/reporters/site-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down