Skip to content

feat(gate): let an approved migration clear the schema-drift check - #222

Merged
veksen merged 1 commit into
mainfrom
feat-schema-approval-clears-gate
Aug 5, 2026
Merged

feat(gate): let an approved migration clear the schema-drift check#222
veksen merged 1 commit into
mainfrom
feat-schema-approval-clears-gate

Conversation

@veksen

@veksen veksen commented Aug 5, 2026

Copy link
Copy Markdown
Member

Goal

A reviewer can clear the schema gate on a migration they have read. The schema-drift gate blocks a pull request that changes the schema so that a person validates the migration, but until now nothing could clear it — a repo either failed every migration or turned the condition off. Query-Doctor/Site#3870 built the half that records the validation. This is the half that acts on it.

That completes Query-Doctor/Site#3289. There is no next step in this repo.

What

Before: a pull request containing a migration failed this check, and the only way to get it green was to set schema-drift to warn or off for the whole repo.

After: a reviewer approves that pull request's migration, re-runs CI, and the check passes. A schema change nobody approved still blocks. Amending the migration withdraws the approval on the Site side, so the check blocks again on the next run.

The check's message changes too. It named the setting that disables the gate, because that was the only way out when it was written. It now names the tool that clears it.

How

Read src/gate/schema-change.ts first; the rest follows from it.

  1. SchemaChangeSignal gains approved?: boolean, and gateSchemaChange returns null when it is true. That is the line that makes an approval clear the check. It sits after the changed guard and before the policy lookup, so an approved migration passes without the repo having to soften its policy.
  2. src/main.ts — the evaluateGates roll-up counted schemaChange.changed. It now counts changed && !approved, so an approved migration is not drift in the summary either. The separate gateSchemaChange call below it already passes the whole signal, so it needed no change.
  3. src/reporters/site-api.tsapproved?: boolean on the run response type, optional in the same way changes was added.

An absent approved reads as unapproved. A Site deployment that predates the field sends no such key, and reading that as approved would drop the gate for every repo on an older API. A test pins that.

Note the field is on Site's staging and not yet on main, so this has no effect against production until Site deploys. Merging it early is safe: without the field, every run behaves exactly as it does today.

Tests

src/gate/schema-change.test.ts gains five cases: an approved migration passes, an unapproved one still fails, an absent field still fails, and the message names approve_schema_change. The existing five cases are unchanged, which is the evidence that the default path did not move.

Two of the five failed before the change. Deleting the approval guard turns "passes once someone has approved the migration" red.

Full suite 440 passed, tsc --noEmit clean, npm run build succeeds.

The src/main.ts call site is not covered. evaluateGates is tested in src/gate/evaluate.test.ts, but the wiring that feeds it lives in main.ts, which has no test harness. The gateSchemaChange path that decides the check itself is fully covered.

The gate blocks a pull request that changes the schema so a person validates
the migration. Site now records that validation (Site#3289) and reports it as
schemaChange.approved. The gate reads it: a change nobody approved still
blocks, an approved one passes.

An absent field reads as unapproved, so a Site deployment that predates it
keeps blocking rather than dropping the gate for every repo.

The check's message named the setting that disables the gate, because that was
the only way out when it was written. It now names the tool that clears it, and
says an approval does not change a check that has already reported.

Co-Authored-By: Claude <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query Doctor — 6 successful checks

 Cost regression — No query went up more than 5%
 Untested data access — No changed data-access file without a test
 New query — No new queries
 New query with index recommendation — No new query ships an index recommendation
 Schema drift — No schema changes
 High-value nudge — No index or rewrite past the threshold


More details via MCP → get_ci_run({ runId: "019fd2fe-f025-743e-afce-d88e04203454" }) · view run · docs
3 queries read against main on assumed statistics of 10,000,000 rows per table. Sync production stats for costs measured against your real data.

@veksen
veksen merged commit e2ef85e into main Aug 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant