Cut the merge gate to what can run as a required CI check - #335
Conversation
Carry forward the reviewed implementation from PR #321. The original PR preserves its review and authorship record. Assemble synthetic grouped-number probes without embedding complete identifier-shaped literals in source.
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. |
![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: 99623f78d4
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae12d1b92b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2023ab4b46
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5222cdde8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91404d7f2f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fda1e2e9e
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c73c64e1f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f958f22c3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78a3931270
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c033dd40ec
ℹ️ 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".
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
Avoid static secret-like literals in the regression fixture so external secret scanning can evaluate the final PR range without suppressing the privacy-gate coverage.
60fc50c to
57799cb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57799cb020
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f41d6341a2
ℹ️ 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".
The merge-gate tooling was 3977 lines that gated nothing automatically (a manual CLI; master's ci.yml never invoked it). Review produced 20 unresolved threads and 27 tracking issues. Per the owner's decision, keep only what can run as a real required CI check and delete the rest. Kept: - Compatibility-surface validation (read_surface_paths() and callers). - Privacy/PII scan (merge_gate_privacy.py, merge_gate_diff.py in full, plus their input pipeline: commit author/committer metadata validation and the diff-coverage scan in merge-gate.sh). - A new, minimal (38-line) check that a review exists naming the current head SHA, closing #317: PRs merged 2-8 minutes after opening, where "zero unresolved threads" meant "review had not started". Treats "no review evidence for this head" as FAIL. Cut entirely: - PR/head/base identity binding, branch-protection context derivation, and the checks rollup. - PR body/review-checklist/P4/platform-evidence validation. - Skipped-CI-job allowlist and security-reviewer-comment validation. - scripts/merge_gate_fake_gh.py (a GitHub API test double that existed only to test the cut concerns). merge-gate.sh: 1587 -> 552 lines. merge-gate.test.py shrunk to cover only the kept surface, backed by a new, much smaller fake `gh` (merge_gate_test_gh.py) replacing the deleted one. Also fixes 7 verified live defects in the kept code (#346, #358, #360, #365, #366, #373, #377), each with a regression test in merge-gate.test.py verified to fail against the pre-fix code and pass after. docs/proposed-merge-gate-ci.md proposes the CI wiring to actually make this a required check (a new workflow file, not touching .github/ per this change's scope) and explains why the review-evidence check needs pull_request_review/issue_comment triggers, not just push/synchronize. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cy findings Investigated all 11 unresolved review findings against the promoted-to- required scripts/merge_gate_privacy.py. All 11 turned out to already be fixed on fix/merge-gate-cut-336: each fix landed in an earlier commit (d16416d, 7a29762, c033dd4, 57799cb, f41d634) that is an ancestor of this branch's head, confirmed by direct behavioral testing against the scanner plus git ancestry checks. Add PrivacyScannerFindingsPR335, a direct-unit-test class against merge_gate_privacy.scan(), with one positive + one negative test per finding (22 tests total) so this behavior has explicit regression coverage -- no prior test exercised these specific sub-cases. Each was verified to fail when the specific historical defect described in its review thread is reintroduced, and pass once reverted back out (see task report for the per-finding before/after transcripts). All fixtures use fabricated values (fake UUIDs, .test-domain emails, placeholder credential strings, truncated non-decodable PEM bodies) -- no real PII or credentials. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Only the two compatibility manifests conflicted; pin sets were identical on both sides (212 entries) and no merge-gate script is pinned, so master's sealed manifest was taken and rehashed. rehash reported one changed entry and a confirming pass reports zero. The shrunk merge-gate suite runs green on the merged tree: 44 tests OK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
lamemustafa
changed the title
1 participant
Supersedes #321. Scope changed since this PR was opened: it originally hardened the merge-gate tooling; it now cuts it down and promotes the remainder into CI.
Why the change of direction
The tooling was 3,977 lines that gated nothing automatically —
scripts/merge-gate.shdoes not exist on master, and master'sci.ymlnever referenced it. It was a manual CLI tool, so a bug in it produced a bad human judgement call rather than a failed build.Meanwhile it had become the single largest source of work in the repository: 20 unresolved review threads on this PR and 27 open tracking issues, all against one script family representing under 5% of the codebase. Two of those issue pairs (#362/#380, #372/#376) were duplicate trackers filed for a single defect by successive review rounds of this same PR.
What was cut
scripts/merge_gate_fake_gh.py(930 lines)What is kept — and now actually gates
merge_gate_privacy.py,merge_gate_diff.py) — the highest-severity class in the review corpus at 68% P1.Proposed CI wiring is in
docs/proposed-merge-gate-ci.md(a separatemerge-gate.yml, because the review-evidence check needspull_request_review/issue_commenttriggers and folding those intoci.ymlwould re-run the whole native matrix on every PR comment).Size
merge-gate.shmerge-gate.test.pymerge_gate_fake_gh.pymerge_gate_privacy.pymerge_gate_diff.pymerge_gate_test_gh.py(new, smaller replacement double)44 tests pass on the merged tree.
The 20 review threads
d16416d4,7a29762e,c033dd40,57799cb0andf41d6341.No prior test exercised those 11 sub-cases, so 22 regression tests were added — one positive and one paired negative per finding. The negatives are deliberate: #328 was a PII pattern widened until it flagged ordinary date ranges, and a false positive in a required check is worse than the gap it closes.
Honest notes
merge_gate_test_gh.py(316 lines) was added beyond the literal instruction to delete the test double — the kept tests still need an offlineghstand-in. Flagged rather than absorbed.IDENTIFIER_RE(a PAN-shaped pattern can match ordinary prose) and deliberately not changed — unrelated to these findings, flagged rather than silently fixed.Once this lands, 16 of the open merge-gate tracking issues describe code that genuinely no longer exists and become closeable; the rest were duplicates or already fixed.
🤖 Generated with Claude Code