Close two stale CI-coverage rows, add browser evidence for the activation wizard, and schedule the bundle-budget measurement - #2527
Merged
Conversation
…eady exists Both rows asked for a CI job that runs the caring-contacts database and row-level-security suite. That job landed on main in PR #2474 (79a824e), so building it again would duplicate it. The job is at .github/workflows/ci.yml:1115: a digest-pinned postgres:17 service container on host port 54329 with trust auth and a pg_isready health check, running `npm run caring-contacts:db:test` at :1150. It is wired into the required pr-required aggregate at :1167, :1198, :1317 and :1319, and pinned by a contract test at tests/ci-cache-safety.test.ts:123-135 that also asserts db-reset-verify does not duplicate the same command. There is no residual coverage hole. Every caring-contacts path classifies to static_heavy_changed=true, which the job's `if:` condition covers — checked with scripts/ci-change-scope.mjs for the migrations directory, the Postgres store, the test helper and the runner script. The suite itself was re-run against a real local PostgreSQL cluster: 2 test files passed, 213 tests passed. Also corrects the one clause this makes false. The doc comment in tests/caring-contacts-domain-isolation.test.ts said no workflow runs the database suite at all. CI does run it now; what remains true, and is why the guards still belong in this file, is that a guard living in the database-only files fires solely where a database is configured and never in the default offline run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
Nothing scheduled a baseline measurement, so growth accumulated across many merged PRs stayed invisible until it crossed the 10% tolerance and failed whichever unrelated PR happened to land last. Production is currently +5.2% of its committed baseline — over halfway to that ceiling, with no owner and no trigger. The measuring machinery already existed and was simply never called: check-bundle-budget.mjs implements --refresh-baseline with git provenance validation, and prints CI integration guidance for a job nobody had built. This adds that job. Weekly on Wednesdays at 04:40 UTC plus manual dispatch, deliberately off the crowded Sunday-evening cluster where a cold build would queue, and midday in Perth so the report is fresh during the working day. The build is deliberately cold — .next is removed first — because a cached build makes the check read stale output and report byte-identical numbers, telling you the budget passes when it does not. It is report only. The measurement goes to a rolling issue and the refreshed bundle-budget.json ships as an artifact; the workflow never commits, pushes, or opens a PR. Two reasons, both recorded in the file: check-github-action-pins.mjs bans workflow-authored branch mutation because bot-authored heads leave required checks awaiting approval, and a baseline moved by a bot is a baseline nobody reviewed — the growth it absorbs becomes unattributable, which is the failure this row exists to prevent. A human applies it in a normal PR. The baseline numbers themselves are untouched here. The recorded baselineSource no longer resolves, so refreshing now would absorb growth that cannot be attributed to any reviewed change set. tests/bundle-budget-refresh-workflow.test.ts pins the schedule, the cold-build ordering, the refresh flag, the permissions split, and the absence of any push or branch mutation. It is registered in test:ci-workflows, which a repo guard requires of every workflow-reading test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
…a real referral (#JZA0XK) The activation wizard had no browser evidence at any stage. The isolated Playwright server seeds no referral and the wizard route requires one, so every browser case landed on the no-referral screen and one asserted the wizard has count 0. Three previous implementers declined to fabricate a referral id. They were right: an invented id renders `referral-not-visible`, structurally the same notice as no referral at all, so the test would claim to prove a stage while showing the same screen. So the wizard gets a second server instead, which is the resolution already recorded in the phase-2b seed report. `run-playwright.mjs` starts a second `next start` from the same isolated build, on its own port, with CARING_CONTACTS_DEMO_SEED=on, published as PLAYWRIGHT_SEEDED_BASE_URL and torn down with the primary on every exit path. Its readiness probe is the primary's. The launch-error slot moved from a module-level variable onto each child, so one server's failure can no longer be reported against the other. The primary server's environment is unchanged, deliberately. Its empty caseload, the "No referral named" notice and the count-0 assertion are real production states — a newly onboarded team has no patients on day one — and seeding that server would delete those observations rather than add one. The new spec enters from demo-seed-referral-wren, a referral written through real repository methods by real actors, and proves what only a browser can: that the wizard mounts at all (it is this workspace's only Client Component, reading its draft through useSyncExternalStore with a getServerSnapshot, so a server render that never hydrates is identical in markup to one that did); that a typed draft survives a real page reload; that the sensitive inputs and the fictional-number caution render at 320px, in dark and under forced colours, with tap targets measured from the rendered box rather than a class string; and the two-write middle state. That last one is the case the issue raises the priority for. Stage 4 creates the plan and then starts it, and created-but-not-started is a reachable, recoverable state whose screen tells a clinician the plan exists and that pressing again finishes the same plan. It is the only screen in this workspace that asks someone to press a writing control a second time. It is reached here by blocking only the activation request, so the create genuinely succeeds and the activation genuinely fails, and the assertions are that the draft is kept and still holds the plan id, that the second create body is byte-identical to the first — a replay under the same idempotency key, not a second plan for this patient — and that the draft is cleared only once both writes are confirmed. Wired into gates rather than left collectable but unrun: its own npm script, the seeded project added to test:e2e:pr so verify:ui covers it, and the spec placed on a PR UI shard with per-shard project selection, since a spec passed to a --project=chromium run would collect zero tests and exit 0. Follow-up owed: registering chromium-caring-contacts-seeded in scripts/playwright-browser-preflight.mjs would let the runner drop the project-name mapping it currently uses to satisfy that fail-closed check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…eflight `run-playwright.mjs` was translating `--project=chromium-caring-contacts-seeded` into `--project=chromium` for one purpose only: satisfying the browser preflight, which owns the project-to-browser-family table and fails closed on a name it does not hold. The translation was true — the seeded project uses `devices["Desktop Chrome"]`, so it needs exactly the binary `chromium` needs — but it meant the runner carried a second, parallel description of the same run, and a reader had to check both to know what was launched. The preflight now holds the name itself, so the translation is deleted rather than explained. `playwrightArgs` is what the caller wrote, and it is what both the preflight and `playwright test` receive. Registering the project in DEFAULT_CONFIG_PROJECTS also fixes a quieter gap: a bare run with no `--project` collects every project in the config, so the preflight's default list was previously understating what such a run would launch. Proven by running the journey with the translation gone: the four seeded cases pass under their own project name, with the primary and seeded servers on separate ports, and no "unmapped Playwright project" abort. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
…rigger Two immutable inbox requests, no canonical ledger edit. An `add` for a residual risk this PR creates: the scheduled bundle-budget workflow is never parsed by GitHub until it first runs, and a workflow that fails to parse creates zero jobs and reports a bare failure. Here that would land in a scheduled run that blocks nothing, so it could go unnoticed. One dispatch after merge closes it. An `update` to #QSHHGK recording that its missing trigger now exists, and what still keeps it open: no named refresh owner, a baselineSource that still does not resolve, and a deliberately unrefreshed baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
BigSimmo
marked this pull request as ready for review
September 2, 2026 05:56
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_870efc30-5db6-42dc-8790-93f140bf5da8) |
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e7c02d73-3a9b-4305-bcdf-d6545749381b) |
Resolves a real conflict in two files, both of which this branch also touches. package.json — `test:ci-workflows`: main added tests/browser-test-plan.test.ts, this branch added tests/bundle-budget-refresh-workflow.test.ts, and both landed in the same list. Resolved as the union. Taking either side alone would have silently dropped one workflow-reading contract from the only script that runs them, which is the exact coverage hole the guard in tests/ci-cache-safety.ts exists to prevent. Proven by running the merged script: both files appear in the gate receipt and 447 tests pass. docs/scripts-index.md — generated. Resolved by regenerating with scripts/update-docs-inventory.mjs rather than by hand-merging the conflict markers: 284 script files, 287 npm scripts, and docs:check-inventory agrees. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
BigSimmo
enabled auto-merge (squash)
September 2, 2026 09:15
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bb71cafa-f0da-4de1-91a1-f3d534865fa4) |
`Static PR checks` went red on 7ca17a9 with: docs/scripts-index.md inventory is stale. Run `npm run docs:update` (expected 284 script files and 288 npm scripts) The merge of main brought in a new npm script, and this branch's copy of the generated index still carried the count from before it existed — 287 against a real 288. Regenerated with scripts/update-docs-inventory.mjs rather than edited by hand, which is the only way the counts and the listing stay consistent. Nothing else changed: the failure was one stale generated file, not a fault in the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2f42cd4d-2bd0-4633-8ad9-2010ebc1c5c1) |
This was referenced Sep 2, 2026
8 tasks
BigSimmo
added a commit
that referenced
this pull request
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#27TWKMand#B0530Frather than reimplementing them. Both rows asked for a CI job running the Caring Contacts Postgres and row-level-security suite. That job already exists on main — it landed in PR Compact Answer source and safety support UI #2474 (79a824e) at.github/workflows/ci.yml:1115, with a digest-pinnedpostgres:17service container,CARING_CONTACTS_DATABASE_URL, membership in the requiredpr-requiredaggregate, and a contract test attests/ci-cache-safety.test.ts:123-135. Building it again would have duplicated it. Two immutabledonerequests are queued in the ledger inbox, and one now-false clause intests/caring-contacts-domain-isolation.test.ts(which claimed no workflow runs the database suite) is corrected.#JZA0XK— browser evidence for the Caring Contacts activation wizard. The isolated Playwright server seeds no referral, so every existing browser case lands on the no-referral screen and one asserts the wizard has count 0. Rather than fabricate a referral id — which renders a structurally identical notice and would prove nothing, as three previous implementers correctly judged — this adds a second test server from the same isolated build with the demo seed enabled, on its own port and its own Playwright project. The new spec drives the wizard from a genuinely seeded referral and covers hydration, draft survival across a reload, the sensitive inputs and fictional-number caution at 320px / dark / forced colours, and the two-write middle state where a created-but-not-started plan is finished by a second press of the same control. The existing unseeded server and its count-0 assertion are untouched.#QSHHGK— scheduled bundle-budget baseline measurement. Nothing scheduled a refresh, so accumulated growth stayed invisible until it crossed the tolerance and failed whichever unrelated PR landed last. The measuring machinery already existed (check-bundle-budget.mjs --refresh-baseline) and was simply never called. Adds a weekly report-only workflow that builds cold, measures, publishes to a rolling issue, and attaches the refreshed baseline as an artifact. It never commits, pushes, or opens a PR.run-playwright.mjshad been translating the seeded project name tochromiumpurely to satisfy a fail-closed check that did not know the name. The preflight now holds it, so the translation is deleted rather than explained.#QSHHGKstating what still keeps it open.Verification
npm run verify:pr-localwas re-run on the final head and completed with all 29 routed gates green —failed: (none),not reached: (none). It is the gate that mattered: on an earlier head it caught a staledocs/scripts-index.mdcount that none of the individually-selected checks below would have found.Decisive lines from the individually-run gates:
npm run check:github-actions—GitHub Actions pin check passed.npm run check:ledger-write-discipline—Ledger write discipline passed for 45a3dcacb54a..HEAD.npm run check:ci-scope—Mockup spec parity: 15 advisory specs all match mockupPatterns./CI change scope self-test passed.npm run check:gate-manifest—Gate-manifest OK: all 37 verify:cheap gates are enforced in CI (static-pr + mapped jobs), and the 34 static gates are documented consistently.npm run check:playwright-pr-shards—PR UI shard parity OK: 33 production specs across 3 groups.npm run test:ci-workflows—Test Files 17 passed (17) / Tests 392 passed (392)npm run test(full offline unit suite) —Test Files 948 passed (948) / Tests 12074 passed | 1 skipped (12075)Test Files 4 passed (4) / Tests 65 passed (65)npm run test:e2e:caring-contacts-activation—4 passed, under the project's own name, with the primary and seeded servers on separate portsnpm run format— no changes; the tree already conformsAlso run before this branch existed, to establish that the first item was already done: the Caring Contacts Postgres suite against a real local PostgreSQL cluster —
Test Files 2 passed (2) / Tests 213 passed (213).npm run test:focusedrefuses by design on any changed test path (Focused test selection is unsafe: test or configuration paths changed), so the affected files were proven directly throughscripts/run-vitest.mjsand then by the full suite it names.Verification not run:
npm run verify:releaseand every provider-backed gate (eval:*,check:supabase-project, live Supabase/OpenAI). No live service is touched by this change; the only database used was a disposable local cluster.Risk and rollout
scripts/check-github-action-pins.mjsforbids workflow-authored branch updates because bot-authored heads leave required checks awaiting approval, and a baseline moved by a bot is one nobody reviewed.Notes
The bundle baseline numbers are deliberately not refreshed here. Production sits at +5.2% of its committed baseline, but the recorded
baselineSourceis unreachable, so that growth cannot be attributed to any reviewed change set and refreshing now would silently absorb it. That unreachability was re-checked after deepening this container's clone to 3419 commits:0764fb58…still does not resolve, which corroborates the#QSHHGKledger row's finding independently. The first run produces a measurement with a source that does resolve, which is the point at which a refresh becomes reviewable.Two unit tests initially failed in this container and were not caused by this branch.
tests/rag-plan-package-parity.test.tsandtests/clinical-hazard-controls.test.tsboth validate against historical commits (f3d1a3c…,883f100…) that a shallow clone does not hold. Rather than excuse them, the clone was deepened and both pass; the full-suite result quoted above is a genuine all-green run with no excluded files.One limit worth stating, and it is now tracked. The new workflow is schedule- and dispatch-only, so this PR's CI never invokes it. It is checked by its contract test, the action-pin gate, and review — but not by GitHub's own workflow-syntax parser, which runs only when the workflow first executes. A YAML fault would therefore surface in an isolated non-blocking scheduled run rather than at merge. One
workflow_dispatchafter merge both closes that risk and produces the first measurement#QSHHGKis waiting on.The browser-preflight follow-up is done here, not deferred.
chromium-caring-contacts-seededis registered inscripts/playwright-browser-preflight.mjs, sorun-playwright.mjsno longer translates the project name tochromiumto get past that fail-closed check. Registering it also corrected a quieter gap: a bare run with no--projectcollects every project in the config, so the preflight's default list had been understating what such a run launches. Re-proven with the translation gone.🤖 Generated with Claude Code
https://claude.ai/code/session_01Cq952TVk67XZzJnjiKt5pU
Generated by Claude Code
Note
Low Risk
Changes are CI, Playwright runner infrastructure, and ledger docs—no production app logic; main operational risk is extra CI time/port use when the seeded Playwright server runs on shard 1.
Overview
Adds report-only weekly bundle-budget measurement (#QSHHGK): a new
bundle-budget-refreshworkflow cold-builds (no.nextcache), runscheck-bundle-budget.mjs --refresh-baseline, uploads artifacts, and updates a single labelled rolling issue—no commits, pushes, or PRs. Contract coverage lands intests/bundle-budget-refresh-workflow.test.tsandtest:ci-workflows.Caring Contacts activation browser evidence (#JZA0XK) without disturbing the empty-store workspace spec:
run-playwright.mjscan start a secondnext startwithCARING_CONTACTS_DEMO_SEED=on, exposed asPLAYWRIGHT_SEEDED_BASE_URLfor the newchromium-caring-contacts-seededproject andtests/ui-caring-contacts-activation.spec.ts(hydration, draft reload, 320px/dark/forced-colours, two-write recovery). PR UI shards,test:e2e:pr, and browser preflight are wired so only the shard that needs it selects the seeded project.Ledger housekeeping: marks
#27TWKMand#B0530Fdone (Postgres CI already on main), updates#QSHHGK, adds a follow-up about schedule-only workflows not hitting GitHub’s YAML parser until first run, and fixes a stale comment incaring-contacts-domain-isolation.test.ts.Reviewed by Cursor Bugbot for commit 544bf61. Configure here.