ci(safe-paths): honor risk-tier paths instead of merging past the decline - #150
Merged
Conversation
…line topcoder1/inbox_superpilot#215 merged a tests-only PR touching web/tests/e2e/auth/signup.spec.ts twelve minutes after claude-author-automerge posted "Auto-merge blocked — risk-tier paths touched. Manual click-merge required." That comment is still on the merged PR. The two gates never disagreed in any resolvable sense. claude-author-automerge cannot block a merge — it can only decline to ARM one — and safe-paths-automerge is the only workflow that arms a docs/tests diff, and the only one that can revoke a standing arm. So the decline was an abstention, and the permissive gate won by default. The failure is not that the wrong gate won; it is that a gate announced an enforcement it never had, leaving a merged PR carrying an audit trail that asserts a human approval that never happened. Adds a TIER 2 risk-tier hold to safe-paths-automerge.yml, generalizing the docs/legal override already there (wxa_vpn#1268): safe-by-glob is not safe-by-content. The `*.spec.ts` premise "passing tests can't break runtime" is true about RUNTIME and false about DETECTION — deleting the assertion that catches a lockout is how the lockout ships in a LATER PR, laundered through a green suite. Scoping, both directions: - The hold runs ONLY in the branch where every changed file is safe-by-glob, i.e. where this workflow would otherwise arm. Hoisting it beside the tier-1 override looks equivalent and is not: it would emit a revoke-triggering reason on PRs this workflow never arms — a dependabot bump of .github/workflows/** matches the risk patterns — and the revoke step would then disarm dependabot-auto-merge's legitimate arm. Pinned structurally in test_workflow_guards.py. - The hold is released by the caller's risk_bypass_label, the same one-click label claude-author-automerge's blocked-PR comment advertises. Ignoring it would kill the advertised escape hatch. docs/legal stays absolute and is NOT releasable by the label: a label click is not evidence anyone read the clause. The pattern list is copied VERBATIM from claude-author-automerge.yml and a drift guard re-derives both and fails if they diverge — two gates that disagree about what is risky is how this reopens. Deliberately NOT narrowing the auth pattern away from test roots. That looked like the fix, but the coverage it would remove is arbitrary: matching is anchored on directory SEGMENTS, so auth/signup.spec.ts trips while auth.spec.ts, auth-flows.spec.ts and login-flow.spec.ts do not, and on #215 itself the Stripe payment spec sailed through while the auth spec tripped. Narrowing would trade a real hold for a coverage gap that is already the majority case. The gap is real but is a separate decision about who reviews auth-test edits; corpus entry added so a future "narrow the false positive" change has to confront it. Evidence: - #215's exact four files against origin/main's classify block => all_safe=1 (it auto-merged); against this branch => all_safe=0, reason=risk-tier-hold. - 17 new behavioral cases + drift guard; existing 17 override cases unchanged; 66 pytest selftests pass; BB corpus selftest 179 cases pass; shellcheck clean; both selftests pass under macOS bash 3.2.57. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Risk class: This PR touches one of the blocked path categories from Auto-merge is refused by (This is a policy notice, not a code-quality failure. The classify job itself does not fail — required CI checks remain authoritative for "is the code green.") |
|
Coverage Floor — mode:
|
|
No issues found. Tier-2 hold logic is correctly scoped to the would-arm branch, bypass-label path is wired through end-to-end, rename coverage and drift guard both check out, and the new test cases cover the key behavioral properties. |
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
1 participant
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.
What happened
topcoder1/inbox_superpilot#215merged a tests-only PR touchingweb/tests/e2e/auth/signup.spec.ts12 minutes afterclaude-author-automergeposted "Auto-merge blocked — risk-tier paths touched. Manual click-merge required." That comment is still on the merged PR.The two gates never disagreed in any resolvable sense.
claude-author-automergecannot block a merge — it can only decline to arm one.safe-paths-automergeis the only workflow that arms a docs/tests diff, and the only one that can revoke a standing arm. The decline was an abstention; the permissive gate won by default.The defect is not that the wrong gate won. It is that a gate announced an enforcement it never had, leaving a merged PR whose audit trail asserts a human approval that never happened.
The fix
A tier-2 risk-tier hold in
safe-paths-automerge.yml, generalizing thedocs/legaloverride already there (wxa_vpn#1268): safe-by-glob is not safe-by-content. The*.spec.tspremise "passing tests can't break runtime" is true about runtime and false about detection — deleting the assertion that catches a lockout is how the lockout ships in a later PR, laundered through a green suite.Scoped in both directions:
.github/workflows/**matches the risk patterns — and the revoke step would disarmdependabot-auto-merge's legitimate arm. Pinned structurally intest_workflow_guards.py.risk_bypass_label, the same one-click label the sibling's blocked-PR comment advertises. Ignoring it would kill the advertised escape hatch.docs/legalstays absolute and is not releasable by the label — a label click is not evidence anyone read the clause.The pattern list is copied verbatim from
claude-author-automerge.yml, with a drift guard that re-derives both and fails if they diverge.What this deliberately does NOT do
It does not narrow the auth pattern away from test roots. That looked like the fix — but the coverage it would remove is arbitrary. Matching is anchored on directory segments:
web/tests/e2e/auth/signup.spec.tsweb/tests/e2e/auth.spec.tsweb/tests/e2e/auth-flows.spec.tsweb/tests/e2e/login-flow.spec.tsweb/tests/e2e/session/expiry.spec.tsweb/tests/e2e/stripe/stripe-integration.spec.tsOn #215 itself the Stripe payment spec sailed through while the auth spec tripped. Narrowing would trade a real hold for a gap that is already the majority case. The gap is real, but closing it is a separate decision about who reviews auth-test edits; a corpus entry is added so a future "narrow the false positive" change has to confront it.
Evidence
origin/main's classify block →all_safe=1(it auto-merged). Against this branch →all_safe=0, reason=risk-tier-hold.Notes for the reviewer
ci-workflowsare always manual per policy, and this touches.github/workflows/**(high-risk tier). Both gates correctly decline this PR.Codex Review+Claude Reviewlanes run on the PR. Flagging since.github/workflows/**is on the pre-review list.auto-merge-approvedclick. All authors, matching the existingdocs/legaloverride's scope.🤖 Generated with Claude Code