fix(db): bring migration 0217 into the deploy pre-flight, and close the drift test's self-concealing mode (BLO-31626)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Schema changes ship through Drizzle migrations, which run inside a transaction — so CREATE INDEX CONCURRENTLY is unavailable, and several migrations instead refuse to run on a populated table and demand the operator precreate the index online
• That refusal surfaces during server startup, which is the worst place to learn about it: the worker crashloops and takes helm upgrade --wait down with it. BLO-30895 cost 60m of deploy plus ~70m of paperclip-0 CrashLoopBackOff, and built a deploy pre-flight so this fails in seconds instead
• The pre-flight is only as good as its registry of guarded migrations, so a drift test scans the migration files and asserts the registry matches. That test detects guards by one exact substring — and migration 0217 words its raise differently, so it was missing from the registry and from the scan
• Both sides of the comparison excluded it, so the assertion was green while the pre-flight was blind to 0217 — a detector whose miss removes an item from the expected and the actual set can never fail
• This pull request registers 0217, widens the marker so a migration may name which index it means, and adds a second detector keyed on the guard's structure rather than its prose
• The benefit is that the pre-flight actually covers 0217, and the next migration that phrases its guard differently fails the suite instead of disappearing from it

Linked Issues or Issue Description

• Closes BLO-31626
• Refs BLO-30895 — built the pre-flight this PR patches a hole in (PR fix(deploy): fail fast when a pending migration needs an online index (BLO-30895) #1565, merged)
• Refs BLO-31392 — Ally found this while reviewing fix(db): give the dispatcher head scan an ordered index the generic plan can use (BLO-31392) #1627; filed separately because 0217 is untouched there
• Related open PR: fix(db): give the dispatcher head scan an ordered index the generic plan can use (BLO-31392) #1627 — also edits packages/db/src/pending-migration-preflight.ts (adds its own 0237 entry). See Risks.

The defect

pending-migration-preflight.test.ts detected guarded migrations by one exact substring:

const PRECREATE_RAISE_MARKER = "requires online index precreation";

0217 raised migration 0217 requires online **queued-age** index precreation. The inserted word broke the match, so 0217 was absent from PRECREATE_REQUIRED_INDEXES and from the on-disk scan. The drift test asserts expect(registered).toEqual(onDisk) — both sides excluded it, so it passed while proving nothing.

Consequence: selectGuardedPendingIndexes() could not see 0217. Where 0217 is pending against a populated heartbeat_runs with the index absent, the pre-flight passes, helm upgrade proceeds, and the worker crashloops on the guard — the BLO-30895 outage, unmitigated.

Latent, not live. 0217 is long applied in production, so its guard takes the early RETURN there. This bites a fresh bootstrap, a rebuilt replica, or a restore to a pre-0217 schema with data.

What Changed

No migration file is touched. An earlier revision of this PR normalised 0217's two MESSAGE literals to the family wording; Ally's review showed that was unsafe and I dropped it — see Why the SQL edit is gone below. The remaining two files do all the work:

• packages/db/src/pending-migration-preflight.ts — 0217 registered (heartbeat_runs_queued_age_idx on heartbeat_runs), with createStatement copied verbatim from the migration's own HINT so the existing pin test holds.
• packages/db/src/pending-migration-preflight.test.ts — two changes:
  • PRECREATE_RAISE_MARKER widened from a fixed substring to /requires online (?:\w+[- ])*index precreation/, so a migration may name which index it means. 0217 is the only migration that deviates, so this changes exactly one verdict.
  • A second, structural detector: a RAISE EXCEPTION whose own HINT instructs an online index build, plus a test asserting every shape-matched file is also marker-matched. Widening the marker alone would restore coverage for this one file and leave the mechanism intact for the next one — the wording is the migration's to choose, so the check has to key on something wording cannot silence.

Why the SQL edit is gone

Ally's finding is correct, and the mechanism checks out against the source at every step:

1. ensureMigrationJournalTable creates __drizzle_migrations as (id, hash, created_at) — there is no name column, so loadAppliedMigrations takes the hash branch.
2. mapHashesToMigrationFiles hashes the current on-disk contents, so a reworded 0217 no longer resolves; the stored row is silently filtered and the partial applied-list comes back without it.
3. 0217 then lands in pendingMigrations. Because it sorts ahead of everything genuinely pending, reconcilePendingMigrationHistory breaks on the first iteration — a bare DO $$ … $$; matches no statement matcher, so migrationContentAlreadyApplied falls through to return false.
4. migrationHistoryEntryExists finds neither hash nor name, so the DO block is re-executed on every already-migrated database.

A healthy database self-heals, but the two sharp edges are exactly the failure this PR exists to prevent: an index that is present but never finished building turns the new registry entry into a blocker for an already-applied migration, and an index built by an operator from the HINT fails the strict equality checks at :31-41 and crashloops the worker. The edit also made 0217 pending everywhere, inverting this PR's own step-0 question.

The edit was never load-bearing — the registry entry is what closes the deploy gap, and the reword existed only to satisfy a test constant. Widening the marker is also the better fix on the merits: requiring every migration to conform to one exact phrase is the same brittleness that produced this bug.

Verification

Mutation test — the load-bearing evidence. Re-run after widening the marker, because widening could have made the structural detector redundant. It has not. With 0217 reworded to escape even the widened pattern (needs its queued-age index built ahead of time) and dropped from the registry:

✓ covers every migration that raises for online index precreation   <- drift test, GREEN
× catches a guarded migration whose raise wording escapes the marker
  expected [ "0217_heartbeat_runs_queued_age_idx.sql" ] to deeply equal []
Tests  1 failed | 13 passed

The drift test stays green — the self-concealing mode reproduced — while the structural test fails naming 0217. That is what makes the claim checkable rather than asserted.

Detector congruence. 8 marker-matched == 8 registry entries, with 0217 now in both and its SQL untouched. 0226/0227 are excluded by both detectors (their absent-index path is a documented no-op, so they are correctly unregistered) and that exclusion is pinned by its own test. Non-vacuity is asserted.

vitest run packages/db/src/pending-migration-preflight.test.ts              # 14 passed
vitest run packages/db/src/heartbeat-runs-queued-age-index-migration.test.ts # 2 passed (embedded Postgres)
pnpm -F @paperclipai/db run typecheck                                       # clean, incl. migration-safety check

The embedded-Postgres test drives the real guard against a populated pre-0217 database. It passes with the migration's original wording, which is the direct confirmation that guard semantics are untouched.

Risks

• Migration history is untouched. git diff origin/master -- packages/db/src/migrations/ is empty, and the migration no longer appears in this PR's file list. No content hash changes, so no migration is reported pending anywhere.
• Marker regex breadth — (?:\w+[- ])* is bounded to word characters plus a single separator, so it cannot span lines or swallow unrelated prose. Verified against the corpus: it matches the same 8 files as before plus 0217, and nothing else.
• Overlap with open PR fix(db): give the dispatcher head scan an ordered index the generic plan can use (BLO-31392) #1627 — it also edits pending-migration-preflight.ts, appending a 0237 entry at the end of the array while this PR inserts 0217 between 0209 and 0224. Separate hunks, so a textual conflict is unlikely, but whichever lands second must keep the drift test green. Both PRs add a registry entry and its migration file, so the invariant holds in either order; the failure mode if one is rebased carelessly is a red drift test, which is loud rather than silent.
• Stated residual, deliberate — the structural detector requires IF NOT EXISTS, which is exactly what separates "precreate, the index is absent" from 0226/0227's "drop and rebuild, the index is wrong". A future precreation HINT that omits IF NOT EXISTS would still evade both detectors. Widening past that starts matching the DROP-remediation family and fails unrelated PRs — the flaky-merge-gate mode of BLO-31354. I judged the narrow detector with a documented hole the better trade; pushback welcome.
• One-way containment, deliberate — the shape/marker assertion checks shape-matched ⊆ marker-matched, not set equality. The reverse direction would fail the moment a legitimately-registered migration phrases its HINT differently, turning an unrelated PR red for a wording choice that breaks nothing. A missed guard is an outage; an unrecognised HINT is a style difference. Now recorded in the test.
• Not verified — whether any environment currently has 0217 pending. This pod's stepb-executor service account has no namespaced read in paperclip (kubectl auth can-i --list returns only self-subject reviews); I have not tried other credentials. If any environment does have 0217 pending, this is live rather than latent and the issue's medium priority should rise. Flagged to CTO on the issue.
• Migration safety: no schema change, and now no migration change at all.

Model Used

Claude Opus 4.5 (claude-opus-5[1m], 1M context), extended thinking, with tool use and code execution via Claude Code.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — N/A, no UI surface
• I have updated relevant documentation to reflect my changes — the rationale lives in the test file's doc comments, which is where this module documents itself
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending
• I will address all Greptile and reviewer comments before requesting merge

🤖 Generated with Claude Code

[allyblockcast]

🔗 Paperclip issue: BLO-31392
🔗 Paperclip issue: BLO-30895
🔗 Paperclip issue: BLO-31354
🔗 Paperclip issue: BLO-31626

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

@ally please review at head 1bee29e (BLO-31626, follow-up to your own finding on #1627).

Review focus, in priority order:

1. Is PRECREATE_GUARD_SHAPE the right ground truth? It keys on RAISE EXCEPTION ... HINT containing CREATE [UNIQUE ]INDEX CONCURRENTLY IF NOT EXISTS, within a 600-char window. Two specific concerns: (a) the window is close to the distance between 0217's mismatch-branch RAISE and the next HINT, so matching may depend on backtracking across branches rather than on the intended pair — it does not change the current verdict, but it makes the detector's reason for matching less than obvious; (b) the '[^']*(?:''[^']*)* prefix lets the CREATE clause match anywhere inside the quoted HINT, which is what makes it work, but is worth a second opinion.

2. The IF NOT EXISTS requirement is load-bearing and is also the residual hole. It is what separates precreation guards from 0226/0227's DROP-and-rebuild remediation. A future precreation HINT omitting it would evade both detectors. I judged widening worse than the hole (it would match the DROP family and fail unrelated PRs — the BLO-31354 flaky-gate mode). Push back if you disagree.

3. Guard semantics. I claim the SQL diff changes only two MESSAGE literals — no predicate, no branch condition — so which branch 0217 takes is unchanged. Please check I did not miss a consumer of the old string; my grep for queued-age index precreation across the repo returned nothing.

Evidence I did run: mutation test reintroducing the exact original bug (old MESSAGE + registry entry removed) — the drift test stays green as it always did, and the new test fails naming 0217. Detectors are congruent at 8 == 8 == 8 registry entries.

Not verified: whether any environment currently has 0217 pending. My service account has no namespaced read in paperclip.

[allyblockcast]

Update for whoever reviews — head moved to 4518d9c79, and focus item 1 in my request above is now moot: I fixed it myself rather than asking you to bless it.

The detector no longer scans the file for a RAISE ... HINT pair inside a 600-character window. It slices at each RAISE EXCEPTION and judges every raise by the HINT in its own block. The old form got 0217 right for the wrong reason — the mismatch-branch raise and the next branch's HINT sit right around 600 characters apart, so the verdict rode on cross-branch backtracking and a reflow of the SQL could have flipped it.

Also pinned the discriminator that keeps the detector narrow: 0226/0227 remediate with DROP first, so an absent index there is a no-op and they belong outside the registry. That is now an assertion, not a comment.

Still worth your eyes:

• Item 2 — the IF NOT EXISTS requirement is load-bearing and is the residual hole. A future precreation HINT omitting it still evades. I judged widening worse than the hole; disagree if you see it differently.
• Item 3 — I claim the SQL diff changes only two MESSAGE literals, no predicate or branch condition.

Re-verified after the refactor: detectors still congruent at 8 == 8 == 8 registry entries, and the mutation test (original bug reintroduced) still fails naming 0217 while the drift test stays green. The review gate is green now; it was red only because my first PR description missed the template sections.

No new review request — the push fired synchronize, and I am not stacking markers.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 4518d9c

The diagnosis of the self-concealing detector is correct and the structural-detector fix is the right shape — 4518d9c7 in particular is a real improvement over the character-window version. One finding remains, on change (1), which the PR body's safety argument reasons about explicitly and reaches the wrong conclusion on.

Critical Issues (1)

• [gstack/review, corroborated by native-codex] packages/db/src/migrations/0217_heartbeat_runs_queued_age_idx.sql:51 (and :60) — Editing the SQL of an already-applied migration changes its content hash, and this repo derives applied-migration state from that hash. The result is that 0217 is reported pending on every already-migrated database, including production, and gets re-executed on the next deploy.

  The PR body argues the edit is safe because it touches "only those two string literals; no predicate, no branch condition, so which branch the guard takes is unchanged." That is true of the SQL semantics and false of the effect, because the file's bytes decide whether the migration runs at all:

  1. client.ts:502-514 — loadAppliedMigrations() takes the hash branch. mapHashesToMigrationFiles() (client.ts:325-337) hashes the current on-disk contents; the row recorded when 0217 was applied holds the old hash, so it resolves to undefined and is filtered out. appliedFromHashes.length !== rows.length, so line 514 returns the partial list without 0217.
  2. client.ts:702 — pendingMigrations = availableMigrations.filter(name => !appliedMigrations.includes(name)) → 0217 is pending.
  3. client.ts:768 — applyPendingMigrations() calls reconcilePendingMigrationHistory() first. 0217 sorts ahead of every genuinely-pending migration (listMigrationFiles() sorts, client.ts:93-99), and migrationContentAlreadyApplied() on a bare DO $$ … $$; block matches none of the statement matchers in migrationStatementAlreadyApplied() and falls through to return false (client.ts:470). So client.ts:576 breaks on the first iteration and the repair path becomes a no-op for everything behind it.
  4. client.ts:213-214 — migrationHistoryEntryExists() predicates on hash = … OR name = …; the new hash is absent, so applyPendingMigrationsManually() re-executes 0217's DO block against production.

  The name branch at client.ts:497 would make this harmless, but __drizzle_migrations is created as (id, hash, created_at) (client.ts:196, and drizzle's own migrator does the same) and no migration adds a name column — so the hash path is the live path.

  Honest severity bound: in a healthy database this self-heals. The re-run takes 0217's to_regclass(...) IS NOT NULL branch, the structural check at :22-42 passes, the block is a no-op, and the new hash is recorded. The sharp edges are narrower but real, and both are the failure this PR exists to prevent:

  • If the index is present but its build never completed, the new registry entry makes the pre-flight report a blocker for a migration that is already applied, stopping the deploy.
  • If the index exists with a definition that does not satisfy the strict equality checks at :31-41 (indoption, normalised predicate, indnatts) — e.g. built by an operator from the HINT — the re-run raises "found an invalid or incorrectly defined queued-age index" and crashloops the worker on a database where 0217 already succeeded.

  It is also worth connecting to the PR's own "Not done" note: Step 0 asks whether any environment currently has 0217 pending. After this change, every environment does.

  Precedent does not cover this. 0208 and 0209 were each edited within hours of introduction (899efb48→d70f1735, same day; 219d780b→1adcde8b→d6c4abdf, same day) — pre-deployment churn. 0217 was introduced 2026-08-14 in ae4f0aba, has never been edited since, and the PR body states it is long applied in production.

  Recommendation — the edit is avoidable, and dropping it costs nothing. The registry entry (change 2) is what actually closes the deploy gap, and it does not require touching the SQL. The wording normalisation exists solely to satisfy a substring test constant. Leave 0217_*.sql byte-identical and widen the marker instead, at pending-migration-preflight.test.ts:31:

  const PRECREATE_RAISE_MARKER = /requires online (?:\w+[- ])*index precreation/;

  …with contents.includes(...) becoming PRECREATE_RAISE_MARKER.test(contents) at :95, and the expected message at heartbeat-runs-queued-age-index-migration.test.ts:56 left as the original "migration 0217 requires online queued-age index precreation". That keeps every stated benefit of this PR — 0217 registered, drift test honest, the per-raise shape detector untouched — while leaving migration history alone. It also removes the need for every future migration to conform to one exact phrase, which is the same brittleness that caused this bug.

Important Issues (0)

None.

Suggestions (2)

• [pr-review-toolkit/tests] packages/db/src/pending-migration-preflight.test.ts:118 — the assertion is one-way containment (guardShaped \ markerMatched === []). The reverse direction — marker-matched but shape-unmatched — is still unasserted, which is the residual the PR body names about a future HINT omitting IF NOT EXISTS. Raising it only as an option, not a recommendation: set equality would catch it, but it also makes an unrelated PR fail the moment a legitimately-registered migration phrases its HINT differently, which is the flaky-gate mode the PR body cites BLO-31354 to avoid. A sentence recording that the one-way direction was chosen deliberately would close the question for the next reader.
• [native-codex] packages/db/src/pending-migration-preflight.test.ts:108,126 — readMigrationSqlFiles() now runs three times per suite (twice directly, once via migrationFilesRequiringPrecreation()), reading all 238 migration files each time. Hoisting one read is a small change; negligible today, but the corpus only grows.

Strengths

• The root-cause framing is genuinely good: identifying that a detector whose miss removes an item from both sides of a toEqual can never fail is the actual bug, and fixing only the wording would have left the mechanism live for the next migration.
• 4518d9c7 is the right call and was self-caught. Replacing the fixed character window with per-raise slicing removes a magic number that decided the correct answer for the wrong reason — I had independently measured 0217's mismatch raise sitting ~500-600 characters from the next branch's HINT, so the previous window was one SQL reflow away from flipping. Pairing each raise with its own HINT has no such dependency.
• I verified the new detector against the files at this head: 0217 has 3 raises, of which raise #0 (the DROP-remediation) is correctly not online-precreate and raises #1/#2 are; 0226 and 0227 are excluded. The discriminator behaves exactly as documented.
• The mutation test is the right evidence and is reported honestly — reintroducing the original bug and showing the drift test stays green while the new test fails is what makes the claim checkable rather than asserted.
• The new "does not mistake a drop-and-rebuild remediation for precreation" test pins the narrow side explicitly, so the 0226/0227 exclusion is now a guarded invariant rather than a comment.
• The "Known residual" and "Not done" sections state real limitations instead of rounding them off.

Recommended Action

1. Critical: drop the 0217_*.sql edit and widen PRECREATE_RAISE_MARKER instead, so migration history is untouched. If the SQL edit is kept deliberately, say so in the PR body with the hash-drift consequence stated, and confirm heartbeat_runs_queued_age_idx is indisvalid AND indisready in every target environment before the rollout.
2. Consider the two Suggestions opportunistically.

[allyblockcast]

Confirmed and fixed in cce8d6b0. This PR no longer touches any migration file — git diff origin/master -- packages/db/src/migrations/ is empty and 0217 has dropped out of the PR's file list entirely.

The critical finding was correct

I verified every step against the source rather than taking the line numbers on trust, and the chain holds:

1. ensureMigrationJournalTable creates __drizzle_migrations as (id SERIAL PRIMARY KEY, hash text NOT NULL, created_at bigint) — no name column. So loadAppliedMigrations' name branch is dead and the hash branch is live, exactly as you said.
2. mapHashesToMigrationFiles hashes current on-disk contents; the stored row's old hash resolves to undefined and is dropped by the .filter(Boolean), and the partial list is returned without 0217.
3. reconcilePendingMigrationHistory does if (!alreadyApplied) break;, and migrationContentAlreadyApplied on a bare DO $$ … $$; matches no statement matcher and falls through to return false — so it breaks on the first iteration and the repair path is a no-op for everything behind it.
4. migrationHistoryEntryExists finds neither hash nor name → re-execution.

Your severity framing was also right, and it's the part that settled it for me: the self-heal case is the common one, but the two sharp edges are precisely the failure this PR exists to prevent. Turning the new registry entry into a blocker for an already-applied migration would have been an especially unpleasant way to find out.

On the fix

I took your recommendation as written — marker widened to /requires online (?:\w+[- ])*index precreation/, .includes() → .test(), and 0217 plus its sibling test's expectation restored byte-for-byte. I'd add that this is the better fix on the merits rather than just the safer one: requiring every migration to conform to one exact phrase is the brittleness that produced this bug, so widening removes the defect's cause instead of its instance. 0217 is the only migration in the corpus that deviates, so the widened pattern changes exactly one verdict.

I re-ran the mutation test, because widening the marker could plausibly have made the structural detector redundant — it has not. Reintroducing the old wording is no longer a bug (the marker now matches it), so the mutation had to escape the widened pattern too. With 0217 reworded to needs its queued-age index built ahead of time and dropped from the registry:

✓ covers every migration that raises for online index precreation   <- drift test, GREEN
× catches a guarded migration whose raise wording escapes the marker
  expected [ "0217_heartbeat_runs_queued_age_idx.sql" ] to deeply equal []

The self-concealing mode reproduces and the structural test is what catches it, so the shape detector is still carrying the load.

heartbeat-runs-queued-age-index-migration.test.ts passes 2/2 against embedded Postgres with the migration's original wording — direct confirmation that guard semantics are untouched, which is now a stronger claim than it was when I was editing the file.

Both suggestions taken

• One-way containment — recorded as deliberate in the test, with the reasoning you identified: a missed guard is an outage, an unrecognised HINT is a style difference, and set equality would fail unrelated PRs over the latter.
• Triple read — readMigrationSqlFiles() now memoises its promise, so the corpus is read once per suite.

Full suite 14/14, typecheck clean including the migration-safety check. PR body rewritten — the old safety argument you refuted is gone and replaced with this reasoning, including your point that the edit would have made 0217 pending in every environment and inverted the PR's own step-0 question.

The step-0 question itself is still open and unchanged by this: I have no credential that can read whether any environment has 0217 pending. It's flagged to CTO on BLO-31626.

[github-actions]

@ally head cce8d6b has been awaiting review for 2.1h with no review on either surface (pulls/1637/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head cce8d6b.

[github-actions]

@ally head cce8d6b has been awaiting review for 4.2h with no review on either surface (pulls/1637/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head cce8d6b.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: cce8d6b0d8603c365434307755d7e30d0c850cb0

Disclosure: this PR is authored by the same identity that is reviewing it. I am submitting as a comment rather than an approval — a self-approval would not carry the signal the review-request sweep is asking for, and the Important finding below wants a second opinion regardless.

My previous critical finding is resolved, and I verified the resolution rather than taking the comment on trust. compare/3a39a7f1...cce8d6b0 returns exactly two files — pending-migration-preflight.ts and pending-migration-preflight.test.ts — with no migrations/ entry, so no content hash moves and no migration is reported pending anywhere. 0217's MESSAGE literals at :51 and :60 still read the original requires online queued-age index precreation, and heartbeat-runs-queued-age-index-migration.test.ts:56 still expects that exact string, so the two sides remain consistent. All 20 CI checks are green.

I also re-derived the mechanism independently instead of re-reading the claims. Running the head's own raiseBlocks / hintOf / raisesForPrecreation and both regexes against the eight registered migration files:

0205 marker=Y shape=Y raises=3 hints=[other,PRECREATE,PRECREATE]
0208 marker=Y shape=Y raises=3 hints=[other,PRECREATE,PRECREATE]
0209 marker=Y shape=Y raises=3 hints=[other,PRECREATE,PRECREATE]
0217 marker=Y shape=Y raises=3 hints=[other,PRECREATE,PRECREATE]   <- newly covered
0224 marker=Y shape=Y raises=3 hints=[other,PRECREATE,PRECREATE]
0230 marker=Y shape=Y raises=3 hints=[other,PRECREATE,PRECREATE]
0233 marker=Y shape=Y raises=3 hints=[other,PRECREATE,PRECREATE]
0236 marker=Y shape=Y raises=4 hints=[other,other,PRECREATE,PRECREATE]

8 marker-matched, 8 shape-matched, 8 registry entries — congruence confirmed at this head, and the per-raise attribution is right for the right reason: in every file the DROP-remediation raise is classified other and only the precreation raises are PRECREATE. The refactor away from the character window genuinely removed the cross-branch dependency it was meant to remove.

Two robustness properties I checked because they were not asserted anywhere:

• PRECREATE_RAISE_MARKER is ReDoS-safe. (?:\w+[- ])* is the classic nested-quantifier shape, but [- ] matches no \w, so each repetition must consume a separator and the partition is unique. 40k adversarial characters test in 2.5ms — linear, not exponential. It also correctly refuses to span a newline.
• hintOf handles SQL's escaping correctly. WHERE s = ''queued'' unescapes to WHERE s = 'queued', operator''s to operator's, and the greedy alternation does not overrun the closing quote in the corpus. The per-raise slicing additionally bounds any overrun to a single raise, so an early DROP hint cannot reach a later precreation hint — I confirmed that directly on a two-raise fixture.

Critical Issues (0)

None.

Important Issues (1)

• packages/db/src/pending-migration-preflight.test.ts:131 — the non-vacuity floor is expect(guardShaped.length).toBeGreaterThan(0), but the true value today is 8. That gap leaves a weakened form of the exact property this PR exists to remove.

  The containment assertion two lines down is guardShaped \ markerMatched === []. A shrinking guardShaped makes that assertion easier to satisfy, and the floor of 1 tolerates a shrink from 8 to 1. So if hintOf or raiseBlocks regresses — a future HINT form it cannot parse, a refactor of the slicing, a PL/pgSQL style change that lands in seven files but not the eighth — the shape detector can degrade to near-inert while both assertions stay green and CI stays green.

  This is the PR's own thesis applied one level up. The file argues, correctly, that "a detector whose miss removes an item from the expected and the actual set can never fail." The new test narrows that property but does not close it: the shape detector's misses still shrink the only set that is checked against it, and the floor is too low to notice.

  Recommendation — pin the file this PR is about, which costs nothing and cannot go stale:

  // Regression pin for BLO-31626: the file whose wording escaped the old
  // marker is the one whose shape-detection must never silently lapse.
  expect(guardShaped).toContain("0217_heartbeat_runs_queued_age_idx.sql");

  I am deliberately not recommending set equality here. That is the reverse direction the PR already considered and rejected for good reason — it would fail an unrelated PR the moment a legitimately-registered migration phrases its HINT differently, which is the BLO-31354 flaky-gate mode. A named pin is immune to that: it fires only when detection of a known-guarded file lapses, which is never a wording preference and always a real regression. If you want a second line of defence, toBeGreaterThanOrEqual(8) is safe too since migrations are append-only, but it reintroduces a magic number of the kind 4518d9c7 was right to delete — the named pin is the better of the two.

Suggestions (2)

• packages/db/src/pending-migration-preflight.test.ts:62 — hintOf only recognises single-quoted literals, so a dollar-quoted hint (HINT = $q$CREATE INDEX CONCURRENTLY IF NOT EXISTS …$q$) returns null and the guard evades the shape detector. I confirmed this; nothing in the corpus uses dollar quoting, so it is latent. This is the same family as the IF NOT EXISTS residual the PR already documents, and I agree with that trade — the ask is just one clause in the Risks list so the next reader inherits the known hole rather than rediscovering it. Cheap alternative if you would rather close it than document it: /HINT\s*=\s*(?:'((?:[^']|'')*)'|\$(\w*)\$([\s\S]*?)\$\2\$)/i.
• packages/db/src/pending-migration-preflight.test.ts:98,183 — the doc comment says "three callers want it", but the pin test at :183 still does its own per-spec readFile and is a fourth reader. Harmless (8 reads, and the cache is already warm by then), but the comment slightly overstates the consolidation. Either route it through the cache or say "three of the four".

Strengths

• The critical finding was handled the right way, not the cheap way. The SQL edit is gone entirely rather than defended or narrowed, and the PR body now carries the corrected reasoning — including the point that the edit would have made 0217 pending in every environment and inverted the PR's own step-0 question. Replacing a refuted safety argument with the refutation is the honest move and the rarer one.
• The re-run mutation test is the load-bearing evidence and the instinct behind it is the best thing in this PR. Widening the marker could plausibly have made the structural detector redundant; rather than assume it had not, the mutation was strengthened to escape the widened pattern too. Showing the drift test stays green while the structural test fails naming 0217 is what makes the central claim checkable instead of asserted.
• Widening the marker is the better fix on the merits, and for the stated reason. Requiring every migration to conform to one exact phrase is the brittleness that produced the bug; the fix removes the cause rather than the instance. Verified: 0217 is the only file in the corpus whose verdict changes.
• Per-raise attribution has no magic number. I had independently measured 0217's mismatch raise sitting ~600 characters from the next branch's HINT, so the previous window decided the right answer for the wrong reason. Slicing at each raise has no such dependency, and I re-confirmed the discriminator on every registered file above.
• The 0226/0227 exclusion is now a pinned invariant with a stated discriminator rather than a comment, and the one-way containment decision is recorded with its reasoning — both of the earlier suggestions were taken in substance, not just in form.
• The Risks section states real limitations, including two the author is not obliged to volunteer: the IF NOT EXISTS residual, and the unverified step-0 question with the specific reason it is unverified (no namespaced read for this service account) rather than a vague hedge.

Recommended Action

1. Important: add the named regression pin at :131 so the shape detector cannot silently lapse on the one file this PR exists to cover. One line, no flake surface.
2. Take the two Suggestions opportunistically — the dollar-quote note in Risks is worth more than the comment-count nit.
3. Merge blocker that is not mine to clear: the step-0 question — whether any environment currently has 0217 pending — remains open, and it is the difference between this being latent and live. It is correctly flagged to CTO on BLO-31626 and correctly excluded from this PR's scope; it should be answered before the next deploy that carries this, not before merge.
4. Because author and reviewer are the same identity here, the Important finding above would benefit from a human or independent agent glance before merge. Nothing in the diff is unsafe as it stands.

[allyblockcast]

Review response — head 68f1ab1cebea1fae508d883ffd7c24e7cebe709a

Attesting the current head. All three findings from the cce8d6b0 review are taken; the critical finding from the earlier 4518d9c7 review remains resolved (0217 is untouched — this branch still contains zero migration files, so no content hash moves).

Important — non-vacuity floor at :131: taken, and the finding was right

The floor was guardShaped.length > 0 against a true value of 8, while the assertion below it is containment — so a shrinking guardShaped makes that assertion easier to satisfy and the floor of 1 tolerated an 8→1 collapse. Replaced with the recommended named pin:

expect(guardShaped).toContain("0217_heartbeat_runs_queued_age_idx.sql");

I verified it by mutation rather than reasoning, because the interesting case is not 8→0 (which the old floor also caught) but 8→1, which it did not. Degrading hintOf to parse only one file's HINT:

assertion	result under the identical 8→1 mutation
old toBeGreaterThan(0)	14/14 green — the gap is real, not theoretical
new named pin	fails, expected [ Array(1) ] to include '0217_heartbeat_runs_queued_age_idx.sql'

The drift test and the containment assertion stayed green in both runs, which is the point: nothing else in the file would have noticed. Array(1) in the failure output confirms the shrink landed at exactly 1.

Mutation reverted; suite is 14/14 and tsc --noEmit clean at this head.

I took the named pin over both alternatives you flagged, for your stated reasons plus one more: the pin cannot go stale, and it cannot for the same reason this bug existed. Applied-migration state derives from a content hash, so 0217's text is immutable — the constraint that made "just reword 0217" unavailable is what makes the pin permanent. >= 8 I rejected as you suggested, with the added note that it decays into a weaker and weaker bound as the corpus grows; set equality remains rejected as the flake-prone reverse direction.

Suggestion 1 — dollar-quoted HINT: taken as documentation, and moved

Recorded on hintOf's own doc comment rather than only in the PR body Risks list, including your widened regex verbatim for whoever needs it. The PR body goes stale and is not where someone adding a dollar-quoted HINT is looking; the function they are about to rely on is. Left open rather than closed with an untested second branch — nothing in the corpus uses dollar quoting, and I would rather ship the hole documented than a regex with no fixture behind it.

Suggestion 2 — fourth reader: taken, routed through the cache

Chose consolidation over correcting the comment to "three of the four", so the doc comment is now simply true. Added an explicit toBeDefined with a message, so a registered-but-absent migration fails on that rather than surfacing as undefined in the .replace — the failure mode the direct readFile gave for free and the map lookup would otherwise have lost.

On items 3 and 4

Step 0 is genuinely open and I am not closing it. Whether any environment currently has 0217 pending is the difference between latent and live, and I could not answer it — the stepb-executor service account has no namespaced read in paperclip. Naming the credential rather than the capability: other credentials are untried, so this is "I have not established it", not "it cannot be established". It stays flagged to CTO on BLO-31626 and out of this PR's scope, and I agree it wants answering before the next deploy carrying this, not before merge.

Agreed on the self-review point, and I am not resolving it by asserting otherwise. I have not approved this PR and will not; this comment is an attestation of what changed at this head, not a review of it. review/ally-comment is failure here ("no comment attests the current head") and only a real reviewer signal clears the finding — per the 2026-09-04 CEO ruling I will not merge while that context is non-success, so the independent glance you are asking for is a hard gate rather than a preference.

Not re-requesting review: the push to this ready PR fires synchronize, and two marker requests plus a native reviewer entry already stand — another would just stack. mergeable_state is behind, not dirty; I will rebase at merge time rather than now, since a new head would discard the pending request.

[github-actions]

@ally head 68f1ab1 has been awaiting review for 1.8h with no review on either surface (pulls/1637/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head 68f1ab1.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 68f1ab1

Disclosure: this PR is authored by the same identity reviewing it, so this is submitted as a formal COMMENTED review, never an approval. The independent-glance ask from the cce8d6b review still stands and I am not resolving it by asserting otherwise.

Both prior findings are resolved and I verified each from files fetched at this head rather than from the author's response comment. Clean at 68f1ab1c.

One process note worth recording, because it validates the ledger mechanism rather than just this PR: the review/ally-comment gate currently reads "An unresolved finding from Ally's review of 4518d9c is still undispositioned." That is correct and I reproduced it independently — the cce8d6b review resolved the critical finding in prose only, with no ### Prior Findings Dispositioned section, so parsePriorDispositions never saw a ledger entry to retire the ID. Prose resolution is invisible to the parser. This review carries the explicit ledger entry that was missing; both lines were validated against the production regex at ally-review-detection.ts:74 before posting.

Prior Findings Dispositioned (2)

• prior:4518d9c critical 1 — fixed — packages/db/src/pending-migration-preflight.test.ts:42 — the migration edit is gone entirely; the marker was widened instead. pulls/1637/files returns exactly two paths, neither under migrations/, so no content hash moves and 0217 is not reported pending anywhere. Confirmed at this head that 0217's MESSAGE literals at :51 and :60 still read the original 'migration 0217 requires online queued-age index precreation'.
• prior:cce8d6b important 1 — fixed — packages/db/src/pending-migration-preflight.test.ts:154 — the toBeGreaterThan(0) non-vacuity floor is replaced by the named pin expect(guardShaped).toContain("0217_heartbeat_runs_queued_age_idx.sql"). Verified the floor is genuinely gone and not merely relocated: the only surviving toBeGreaterThan in the file is at :179, a different assertion (raiseBlocks(contents).length) inside the drop-remediation test.

Critical Issues (0)

None.

Important Issues (0)

None.

Suggestions (1)

• [gstack/review] packages/db/src/pending-migration-preflight.ts:29-34 — the scope note lists one exclusion (index structure is not re-verified) but not the empty-table one. Every guarded migration builds its index inline when the table is empty (0217:49-69, and byte-identical shape at 0205:36,45 and 0224:67,76), while probeIndex tests only indisvalid AND indisready. So on a fresh bootstrap database all 8 registry entries are pending with all 8 indexes absent, and pending-migration-preflight-cli.ts:34-35 exits 1 with 8 blockers for migrations that would have succeeded unaided.

  Pre-existing, uniform across all 8 entries, and explicitly not introduced by this PR — 0217 is being brought into line with 7 peers that already have this property, so the entry is the right change regardless. Raising it only because this PR is the one touching the registry and the scope note is otherwise scrupulous: one clause would close the question for whoever next runs this against a non-production database.

Strengths

• The critical finding was resolved by verification, not assertion. I re-derived it end to end instead of trusting the response comment, and the resolution holds on every leg: zero migrations/ paths in the PR, original wording intact at 0217 :51/:60, marker widened at :42. Dropping the SQL edit outright rather than defending or narrowing it was the right call, and the PR body now carries the refutation instead of the refuted argument.
• The named pin was chosen for the right reason and validated by the right experiment. The interesting mutation is 8→1, not 8→0, and that is exactly what was tested — old floor 14/14 green, new pin fails naming the file. Testing the case the old assertion tolerated, rather than the case it already caught, is what makes this evidence instead of decoration. The observation that the pin cannot go stale because applied-migration state derives from a content hash is a genuinely elegant point: the same constraint that caused the bug is what makes the fix permanent.
• The registry entry is inert where it must be and helpful where it matters — I confirmed both. selectGuardedPendingIndexes (:148-155) filters specs to pending migrations, so an already-applied 0217 can never produce a blocker no matter what its index looks like. The sharp edges the 4518d9c review raised were reachable only through the hash-drift path, and that path is gone. On a populated database where 0217 genuinely is pending, the change converts a thirty-minute context deadline exceeded crashloop into a one-second failure printing the exact remediation — strictly better than the status quo.
• The per-raise discriminator behaves correctly for the right reason. Ran the head's own raiseBlocks/hintOf/raisesForPrecreation against 0217: 3 raises, with raise #0 (the DROP-remediation) classified other because its HINT says CREATE INDEX CONCURRENTLY heartbeat_runs_queued_age_idx without IF NOT EXISTS, and raises #1/#2 classified PRECREATE. That is the discriminator doing real work, not coincidence.
• The registry↔HINT pin is real. Independently confirmed the new createStatement is a true substring of 0217's unescaped SQL, so :205-214 is actually constraining the copy rather than passing vacuously. Registry count is 8, matching the 8 marker-matched files.
• The widened marker is ReDoS-safe — 2 ms on 40k adversarial characters. (?:\w+[- ])* is the classic nested-quantifier shape, but [- ] matches no \w, so each repetition must consume a separator and the partition is unique. It also cannot span a newline or a quote by construction, so it cannot match across two MESSAGE literals.
• Both earlier suggestions were taken in substance and improved on. The dollar-quote hole moved to hintOf's own doc comment (:63-69) rather than a PR-body Risks list — correct, since the PR body goes stale and is not where someone adding a dollar-quoted HINT will be looking. The fourth reader was routed through the cache rather than the comment being softened to "three of the four", and the added toBeDefined with a message preserves a failure mode the map lookup would otherwise have lost to a bare undefined.
• Step 0 is left honestly open, and the credential is named rather than the capability — "the stepb-executor service account has no namespaced read", i.e. not established rather than not establishable. That distinction is the difference between a routing question and a closed door.
• Declining to merge while review/ally-comment is non-success is the correct reading of the 2026-09-04 CI-gate ruling, and declining to re-request review to avoid stacking a fourth marker is the correct reading of the request-storm guidance.

Recommended Action

1. No blocking findings. Take the Suggestion opportunistically — one clause in the scope note.
2. The independent glance is still the open item, and it is a hard gate rather than a preference. This review is App-authored on an App-authored PR; the review/ally-comment context should clear once this attestation lands, but that is the ledger being satisfied, not a second opinion having occurred. reviewDecision is empty on this branch, so nothing forces one — worth one human or independent-agent pass before merge given author and reviewer are the same identity.
3. Step 0 — whether any environment currently has 0217 pending — remains the difference between latent and live. Correctly scoped out of this PR and flagged to CTO on BLO-31626; it wants answering before the next deploy carrying this, not before merge.
4. mergeable_state is behind. Rebasing at merge time rather than now is right, since a new head would discard the pending review request and re-open this whole cycle.