Skip to content

fix(hooks): unblock verify:pr-local in web sessions; answer #248 offline half and correct #210 - #1880

Merged
BigSimmo merged 9 commits into
mainfrom
claude/task-completion-checklist-e4rfob
Aug 12, 2026
Merged

fix(hooks): unblock verify:pr-local in web sessions; answer #248 offline half and correct #210#1880
BigSimmo merged 9 commits into
mainfrom
claude/task-completion-checklist-e4rfob

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

  • fix(hooks) — the SessionStart hook broke verify:pr-local in every Claude Code web session. The hook wrote its lockfile marker to node_modules/.session-start-lock-hash immediately after npm ci, but that same npm ci had already recorded the trusted file inventory in postinstall (check-installed-lock-parity.mjs --write-stamp). The marker therefore landed one file after the stamp — 51567 files against a stamp of 51566 — so check:installed-lock-parity failed deterministically and aborted the PR gate before format, lint, typecheck and test ever ran. Its printed remedy ("Run npm ci") could not help, because a fresh npm ci re-runs the hook and recreates the same off-by-one. Moving the marker into node_modules/.cache fixes it: that directory is already in the check's VOLATILE_DIRECTORIES set, and npm ci still wipes it with the rest of node_modules, so the hook's staleness semantics are unchanged.
  • docs#248, offline half answered.check:driftdoes cover the missing-index class: all four 20260705180000 indexes are present in supabase/schema.sql and in supabase/drift-manifest.json, indexes are compared by name on table + def_hash, and supabase/drift-allowlist.json is empty, so a run would have failed with four missing_live findings. The gap is cadence and gating, not coverage — check:drift runs only from live-drift.yml (weekly cron plus manual) and blocks no PR or release. The write-up also records a genuinely uncovered sub-class: schema_drift_snapshot() builds index rows from pg_get_indexdef and never reads indisvalid/indisready, so an index left invalid by a failed CREATE INDEX CONCURRENTLY compares byte-identical while the planner refuses to use it.
  • docs#210, corrected. Half of the row is already fixed and its prescribed fix is refuted, so the row as written would have sent the next agent into a change that silently reverts. npm run typecheck has used tsconfig.typecheck.json since 450690f (excludes .next entirely; verified green with .next/dev/types/validator.ts present). Dropping the dev-types glob from tsconfig.json does not hold: Next 16 emits it deliberately (type-paths.js, "to avoid tsconfig churn when switching between dev/build modes") and writeConfigurationDefaults pushes it back into an existing include on every next dev/next build. What remains is narrower than written — scripts/run-playwright.mjs writes an isolated tsconfig that inherits the repo-root globs, and Next's own dev-types filter does not cover it because useTypeScriptCli defaults true. Recorded explicitly as not yet proven end-to-end.
  • docs — new row for the hook defect above, since the failure is subtle and the tempting repair (re-stamping) would launder a real dependency mismatch as green.

Not addressed, and why: #022 needs a deliberate hosted Supabase migration apply plus a qualified clinician's review of BMJ attestation evidence. Both are outside what this session can authorize or do; state verified unchanged (attestation_applied: false across all 11 manifest entries, migration 20260727010000 still unapplied). The live half of #248 — whether the reconcile migration partially applied, indexes were manually dropped, or schema_migrations was repaired — likewise needs the approved live/history window.

Verification

  • npm run verify:pr-local
PR-local verification summary:
- completed: check:runtime, check:installed-lock-parity, format:changed, sitemap:check,
docs:check-index, docs:check-inventory, docs:check-scripts, docs:check-links,
check:branch-review-ledger, check:outstanding-issues, lint, typecheck, test, check:rag:fixtures
- failed: (none)
- not reached: (none)
Test Files 563 passed (563)
Tests 6148 passed | 4 skipped (6152)

npm run check:installed-lock-parity before the fix: installed file inventory differs from the trusted post-install stamp (exit 1). After: 760 package locations; 51566 files. (exit 0).

UI verification not run: no UI, routing, styling, or browser-behaviour change in this diff.

Risk and rollout

  • Risk: Low. One line of a Claude Code SessionStart hook plus two documentation files. No application source, no dependency, no schema, no workflow. The hook change is confined to where a local marker file is written; if it were wrong, the visible effect is a redundant npm ci at session start, not a broken install.
  • Rollback: git revert either commit independently — the hook fix and the documentation are separate commits and share no file.
  • Provider or production effects: None. Nothing in this diff reads or mutates Supabase, OpenAI, Railway or hosted CI. The #248 findings were derived entirely from committed files (schema.sql, drift-manifest.json, drift-allowlist.json, check-drift.ts, the snapshot migration) with no live access.

Notes

  • No ## Clinical Governance Preflight section: classifyPullRequestFiles returns clinicalRisk: false, operationalRisk: false, ragRanking: false, ui: false for these three paths, and the diff touches no ingestion, answer-generation, ranking, source-rendering, document-access or privacy surface.
  • Bundling rationale: all three items are independently low-risk under both the classifier and the broader risk list in AGENTS.md, each is listed as its own bullet above, and the only code change is isolated in its own commit so it stays separately revertible while the PR is open.

Generated by Claude Code

The SessionStart hook wrote its lockfile marker to
node_modules/.session-start-lock-hash right after npm ci. That same npm ci
already ran check-installed-lock-parity.mjs --write-stamp in postinstall, so
the marker landed one file after the trusted inventory was taken: 51567 files
against a stamp of 51566.
check:installed-lock-parity therefore failed deterministically in every Claude
Code web session, aborting verify:pr-local before format, lint, typecheck and
test. Its printed remedy could not help either, since a fresh npm ci re-runs
the hook and recreates the same off-by-one.
Move the marker into node_modules/.cache, which is already in the check's
VOLATILE_DIRECTORIES set and is still wiped by npm ci, so the hook's staleness
semantics are unchanged.
Verified: check:installed-lock-parity exits 0 at 51566 files, and the full
verify:pr-local run now completes all 14 steps (6148 tests passed).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STG6AU5J4gxrFJagP4pRti
#248 — settled without touching live. check:drift does cover the missing-index
class: all four 20260705180000 indexes are in schema.sql and in the drift
manifest, indexes compare by name on table + def_hash, and the drift allowlist
is empty, so a run would have failed with four missing_live findings. The gap
is cadence, not coverage — live-drift.yml is weekly plus manual and gates
nothing. Also records a genuinely uncovered sub-class: schema_drift_snapshot()
reads pg_get_indexdef but never indisvalid/indisready, so an index left invalid
by a failed CREATE INDEX CONCURRENTLY compares byte-identical while the planner
cannot use it. The live forensic (partial apply vs manual drop vs history
repair) remains operator work behind the approved window.
#210 — half already fixed, and its prescribed fix refuted. npm run typecheck
has used tsconfig.typecheck.json since 450690f, which excludes .next entirely;
verified green with .next/dev/types/validator.ts present. Dropping the dev-types
glob from tsconfig.json does not hold: Next 16 emits it deliberately and
writeConfigurationDefaults pushes it back into an existing include on every
next dev/build. What remains is narrower — the Playwright isolated tsconfig
inherits the repo-root globs, confirmed by probe, and Next's own dev-types
filter does not apply because useTypeScriptCli defaults true. Recorded as not
yet proven end-to-end.
Also records the session-start marker defect fixed in the previous commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STG6AU5J4gxrFJagP4pRti
@supabase

supabaseBot commented Aug 12, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:30 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7c3f523a-04c9-41f0-a220-627f4c5e52d7

📥 Commits

Reviewing files that changed from the base of the PR and between ab3930d and 1520c15.

📒 Files selected for processing (2)
  • .claude/hooks/session-start.sh
  • docs/database-drift-detection.md

Comment @coderabbitai help to get the list of available commands.

@BigSimmo
BigSimmo enabled auto-merge August 12, 2026 20:27
@BigSimmo
BigSimmo requested a lite review from CopilotAugust 12, 2026 20:37

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@BigSimmo
BigSimmo disabled auto-merge August 12, 2026 21:00
@BigSimmo
BigSimmo enabled auto-merge August 12, 2026 21:24
@BigSimmo
BigSimmo disabled auto-merge August 12, 2026 21:25
@BigSimmo
BigSimmo enabled auto-merge August 12, 2026 21:41
@BigSimmo
BigSimmo merged commit 517fd00 into mainAug 12, 2026
24 checks passed
@BigSimmo
BigSimmo deleted the claude/task-completion-checklist-e4rfob branch August 12, 2026 21:47
Sign up for freeto 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.

3 participants

@BigSimmo@claude