fix(governance): consolidate reviewed-history and privacy CLI checks - #2733
fix(governance): consolidate reviewed-history and privacy CLI checks#2733BigSimmo wants to merge 5 commits into
Conversation
The shallow-clone guard added for clinical-hazard-controls and rag-plan-package-parity missed a third check with the identical defect. On a web-container clone (depth ~102) the reviewedCommit is simply absent from a truncated history, and both the spec and the CLI gate report it as "reviewedCommit does not exist: d307494...", which reads as a corrupt privacy register rather than a truncated checkout. That is the wrong conclusion to invite about a governance manifest, and it costs a diagnosis every session. - tests/privacy-readiness-contract.test.ts: same guard as its two sibling specs. Deepen the history first, and only skip the commit checks when the commit is still unreachable AND the clone is still shallow. - scripts/check-privacy-readiness.mjs: the CLI gate had the same defect, so check:production-readiness carried it too. It now warns with PRIVACY_READINESS_SHALLOW_CLONE naming exactly what was skipped, matching check-clinical-hazard-controls.mjs. Fails closed by construction: the skip needs both a shallow clone and an unreachable commit, so on a full-history checkout a genuinely missing reviewedCommit still fails. Verified directly against a manifest carrying a bogus commit: with git checks on it still reports "does not exist"; only the explicit skip path suppresses it. CI checks out at fetch-depth 0, where the checks run in full. Verification: lint and typecheck clean; full offline unit suite 1268 files passed, 0 failed. The three specs that previously failed in this container now pass, and this container's clone is no longer shallow, so the commit checks ran rather than being skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkMv1BzScPTvrzJ8F7DtzY
Two P1 findings from the Codex review of this PR, both correct. Release mode must never skip the reviewedCommit checks. The first commit applied the shallow-clone skip in every mode, including `--release`. But `check:privacy-readiness:release` backs `check:production-readiness` and `governance:release`, and the ancestry plus evidence-at-commit checks are exactly what bind the register to this repository. Once the remaining status blockers close, a release run from a truncated checkout could have printed PRIVACY_READINESS_PASS having proved none of that. The skip decision now lives in `shallowSkipDecision`, which returns `blocked` for release mode and makes the gate exit non-zero naming the full-history remedy. The spec must not reach the network. `ensureHistoryDeepened` ran `git fetch --deepen` from inside the unit suite: remote I/O and a repository mutation, not an offline probe, and an unreachable or credential-prompting remote would stall the suite rather than fail it. The spec now probes only, and a truncated checkout skips the commit checks and says so. Proven against a real depth-one clone, not argued. On the committed parent the release run printed the skip warning and reached validation, which is the reported defect. With the fix, in the same clone, release exits 1 with "reviewedCommit ... is unreachable in this shallow clone", and structural still passes with the checks skipped and stated. `shallowSkipDecision` is unit-tested across all four release/shallow/commit-present combinations. Verification: lint and typecheck clean; full offline unit suite 1268 files passed, 0 failed, 18036 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkMv1BzScPTvrzJ8F7DtzY
Both governance registers pin the commit a human reviewed, and their validators walk history to prove it is an ancestor of HEAD. A shallow checkout cannot answer that question, and on 2026-09-07 a cloud session with a 335-commit slice read the unanswerable as an answer: it reported nine clinical-safety and privacy sign-off records as pointing at commits that no longer existed. Both registers were correct and both suites pass once history is deepened to reach the commits. The privacy suite had no shallow-checkout handling at all. The hazard suite had a guard that asked only whether the commit OBJECT was present, so a session holding that object while unable to walk to it fell straight through into a false failure. Presence is not the property under test; reachability is. One shared helper now deepens first, restores the real check the moment history can answer, and skips only a shallow checkout that could not be completed, printing a sentinel when it does. On any complete clone, which is every CI run of these jobs, a genuinely wrong register still fails exactly as before, pinned by a negative control in tests/reviewed-commit-history.test.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S74EbbTkB3VRA9KkWpeMxc
|
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: Advanced 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. |
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. |
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_c1e3315a-0673-45c7-b43d-3851648a8df4) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65a7efc7a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…story automatically Two review findings on the reviewed-commit history guard. Ancestry alone was the wrong property. A checkout can hold the commit graph while omitting the historical trees, as a treeless or blobless partial clone does. There the commit is present and is an ancestor of HEAD, so the probe said "go" and the validators then failed on ls-tree, which is how 28 clinical and 33 privacy false positives were produced on a checkout whose registers were correct. The probe now asks for the property the validators actually consume: the commit is here, it is an ancestor, and its tree is readable. That case is a skip, not a failing register, and it is not a shallow-clone case, so nothing about shallowness could have caught it. The guard also ran git fetch --deepen during an ordinary unit run, which is remote I/O in an offline suite and can block on credentials. Deepening is now opt-in through REVIEWED_COMMIT_ALLOW_DEEPEN=1 and never happens by default; the default answer for a shallow checkout is to skip and name the remedy. Every remaining git call runs with GIT_NO_LAZY_FETCH and GIT_TERMINAL_PROMPT disabled, so a partial clone cannot turn a probe into a silent fetch either. Both negative controls are kept and a second added: a complete clone that cannot reach the commit, and one that does not hold it at all, both stay checkGit:true so a genuinely wrong register still goes red. Verified: tests/reviewed-commit-history.test.ts 11 passed, and the two consumers, tests/clinical-hazard-controls.test.ts and tests/privacy-readiness-contract.test.ts, 15 passed. Typecheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0142trjgjRAP2GA9wzqoYUAE
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_fa0b3bec-7d94-4140-9190-d35520896426) |
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_812adc95-dce5-4923-bbd6-5ef649deae22) |
BigSimmo
changed the title
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #16436 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
2 participants
Summary
Consolidates reviewed-commit history handling with the privacy CLI contract. Historical tests handle missing history conservatively while release privacy validation never silently skips its required evidence.
Consolidates #2733 (2d15b9d), #2720 (42fcbf9). Original heads are retained as ancestors; absorbed PRs are closed only after publication is verified.
Verification
npm run test -- tests/reviewed-commit-history.test.ts tests/clinical-hazard-controls.test.ts tests/privacy-readiness-contract.test.ts— 27 passed in 3 files.npm run format— passed; formatting committed.git diff --check— passed.Risk and rollout
npm run check:production-readinesson the shared current-base governance integration stopped at six existing privacy release blockers (five pending and one partial). The production-readiness stage did not execute. These approvals remain outstanding.Clinical Governance Preflight
docs/clinical-hazard-controls.jsonanddocs/governance/privacy-readiness.v1.jsonare unchanged — the investigation confirmed both are correct.Clinical KB Database(sjrfecxgysukkwxsowpy) — untouched.