Deduplicate CI deployment-smoke env wiring in workflow - #115
Merged
Conversation
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Two gaps, both proven before fixing and both re-proven after. guard-push's format guard ran prettier against working-tree paths, but a push sends commits. Commit `const a = 1`, run npm run format, and --check on the path passed while git show HEAD:<file> was still unformatted — the guard went green and CI failed. It now reads the pushed blob via git show <sha>:<file>, fed through --stdin-filepath so Prettier still picks the parser from the real path and still honours .prettierignore for it. Verified: an unformatted commit with a formatted working tree now exits 1, a formatted commit exits 0, and SKIP_FORMAT_GUARD=1 still overrides. Unknown verdicts (deleted blob, no parser) stay fail-open so nothing new can block a push. The band adoption gate followed every import specifier, so a route that kept its results import but stopped rendering it still counted as adoption — the exact regression the gate exists to catch. It now parses with @babel/parser and follows a static import only when the binding is mounted: JSX element, default re-export, or named re-export. import(), export-from and side-effect imports stay unconditional, since each is a mount mechanism rather than a binding that can go unused, and the lazy form is the only route to the code-split dashboard workspaces. Verified by gutting (search-app)/services/page.tsx to <div /> with its imports intact: previously green, now an orphan; restored, it passes. Pinned by temp-dir fixtures for imported-but-unrendered, default re-export, and lazy import. Closes#115. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Resolves the docs/outstanding-issues.md conflict: main re-padded the whole table and extended #109, so the conflict spanned all 58 rows while the only semantic difference was this branch closing #115. Took main's block and kept this branch's #115 row. The conflict is why no CI ran here: GitHub cannot build refs/pull/1400/merge for an unmergeable PR, so every pull_request-triggered workflow (CI, Gitleaks, Semgrep) was skipped while pull_request_target ones still ran, and CircleCI failed 3 seconds after each push. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Second conflict in docs/outstanding-issues.md in under an hour, same shape: main re-padded the table so the hunk spanned every row, while only two rows differed. Took main's #108 (completed by #1403) and kept this branch's #115 closure. #116 does not collide — main's next-id was still 116. This recurrence is the case #116 itself describes: while conflicted, the PR ran no CI at all and nothing said so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduY
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Six false greens were reported on the adoption gate in one day — an unrendered
import, `export { X }`, `export { X } from`, `export *`, a bare side-effect
import, JSX inside an unmounted helper, and a lazy import reaching every
sibling export of the same module. They are one defect: the walk asked whether
a file mentions the band, when the question is whether anything the route
mounts reaches it. Two of the six were introduced by an earlier patch to this
same walker, so this replaces the heuristic rather than adding a seventh case.
Each module is now parsed into a small graph (exported name -> local, local ->
identifiers its body references, locals that render the band) and the walk
carries, at every hop, the set of exports the importer actually mounts. The
special cases fall out of module semantics instead of being enumerated:
`dynamic(() => import("…").then((m) => m.Named))` follows only that binding;
a bare `import "…"` renders nothing so is not followed; `export { X } from`
is followed only when the importer wants X; `export *` never supplies a
default, so a page needing only a default gets no hop from it.
Verified: all five production search routes still reach the band; gutting
services/page.tsx and tools/page.tsx to <div /> each reports an orphan; the two
fixtures guarding the new mechanisms were confirmed to fail under targeted
mutation (presence-based band check, and following bare imports) after an
initial pair that did not bite was replaced.
Also from review:
- guard-push escalates to a whole-tree check for a package.json only when it
carries a prettier field, so a routine dependency bump does not. Verified:
bump passes, prettier-field narrowing blocks, .prettierrc narrowing blocks.
- AGENTS.md no longer claims the guard checks working-tree paths; that was the
defect this PR fixed, and leaving it would teach distrust of a fixed guard.
- Repairs the #115 ledger row, whose Source cell had been overwritten by the
detail text, and rewrites it for the new design.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Akwz3Sdms8uJ5AkDt3CduYBigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Both review findings are right, and both are the same failure this gate keeps producing in draft: a check that looks like it covers a case and does not. A row whose id cell is `001`, `#OO1` or empty did not match `ROW`, so it was dropped before every structural check — duplicate detection, the marker comparison, the width check — and the file reported green while carrying exactly the malformed row the gate advertises. Rows are now recognised by being a table body row at all (not a header, not a separator) and the id shape is validated rather than assumed, so an unparseable id is a reported problem instead of an invisible one. `markdown.match(MARKER)` returned only the first marker, so a conflict that kept two left a stale allocation the checker never saw. With markers 116 and 115 and a highest id of #115, nothing failed — and a later editor following the stale 115 reuses an id, which is the exact outcome this gate exists to prevent. Exactly one marker is now required. Four adversarial self-tests, all verified red against the previous logic: self-test FAILED: a dropped # on an id — expected 1 problem(s), got 0 self-test FAILED: a letter O for a zero — expected 1 problem(s), got 0 self-test FAILED: an empty id cell — expected 1 problem(s), got 0 self-test FAILED: a second next-id marker kept by a conflict — got 0 verify:cheap exit 0 — Tests 4450 passed | 4 skipped. format:check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvU8z73P6TXUXoYBqN5K1P
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
main's #1418 (ledger merge dedupe + L4 quarterly archive rotation) and #1413 both edited docs/outstanding-issues.md, so this was a real content conflict rather than staleness: git merge-tree --write-tree confirmed CONFLICT before any resolution was attempted. Resolved by taking main's version of the ledger wholesale and re-applying this branch's five-row archive move on top, so neither side's work is lost: - from main: #88 and #97 archived, new open row #126 (quarterly ledger rotation) with queue order 35, the #23 "When" update (release-browser-matrix no longer blocked by pr-required), the #86 detail update, and the issues:next-id bump to 127. - from this branch: #95, #96, #104, #109 and #115 moved from Open items to Resolved / archive. No row from either side was dropped, and no id appears in both tables. Verified: 121 rows (52 open, 69 archived), marker next-id=127 above the highest; each of #88, #97, #95, #96, #104, #109, #115 resolves to exactly one archive row and #126 to one open row; zero conflict markers remain. npm run verify:cheap -> EXIT=0; "Gate-manifest OK: all 29 verify:cheap gates are enforced in CI"; "Test Files 431 passed (431)"; "Tests 4496 passed | 4 skipped (4500)". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
* issues: archive five completed rows, delete none Triage pass over all 58 open items found five rows whose work is finished and whose next action is empty. Each moves from Open items to Resolved / archive with its fix evidence and the 2026-07-30 date: - #95 the pr-required aggregate now routes a cancelled result through a shared cancelled_error helper; guarded by seven cases that execute the extracted script, three mutation-proven. The red is deliberately retained, since GitHub counts a skipped required check as passing. - #96 every PR #1316 sub-item is dispositioned: the adoption-gate root-path gap closed on PR #1394, four findings were fixed independently, and the Therapy Compass retry-waiter finding was corrected to not-a-live-defect. - #104 a correction row with no next action - the worker's triple image read is an accepted peak-memory trade-off documented at worker/main.ts:866-869, not debt. Archived so a fourth audit does not re-file it. - #109 the branch sweep refuses on a shallow clone, an indeterminate result is its own failure, and the guard moved into the exported collector so the evidence-pack path fails closed too. - #115 the band adoption gate was redesigned to walk a real reachability graph rather than asking whether a file mentions the band. Nothing is deleted. The ledger contract is archive-only (SKILL.md:44 "Archive, never delete"; this file's own conventions: "rows are archived, not deleted, so the history stays auditable"), so no row qualifies for deletion. Row total is unchanged at 120: 58 -> 53 open, 62 -> 67 archived. Prettier widened the archive Outcome column to fit the new evidence, which repads the other archive rows; git diff --ignore-all-space is 7 insertions / 7 deletions, i.e. the five moved rows plus both separators. Verified: node scripts/check-outstanding-issues.mjs --self-test && node scripts/check-outstanding-issues.mjs -> "Outstanding-issues guard passed: 120 rows (53 open, 67 archived), unique ids, next-id=126 above the highest". npx prettier --check . -> "All matched files use Prettier code style!" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YdPS2KhKqz2buzsUgmX3c * docs: record PR 1428 review * docs: align issue 109 resolution date --------- Co-authored-by: Claude <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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
env:declarations in theDeployment boot smokeCI step that made variable wiring ambiguous/invalid at parse-time.SUPABASE_SERVICE_ROLE_KEYnow resolves via${{ secrets.SUPABASE_SERVICE_ROLE_KEY || 'placeholder-ci-service-role' }}OPENAI_API_KEYnow resolves via${{ secrets.OPENAI_API_KEY || 'placeholder-ci-openai' }}Verification
npm run verify:cheapnpm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsnpm run format:checknpm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedClinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
verify:cheapcould not be completed in this runner due engine mismatch (repo requires Node 24/npm 11; runner is Node 22/npm 10).