You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
feat(gate): guardrailEscalation.onCleanReview — release the guardrail hold when the escalated review is clean (#9869)
* feat(gate): guardrailEscalation.onCleanReview — release the guardrail hold when the escalated review is clean
The second half of #9808, as an explicit MODE rather than a policy flip.
review -- but the disposition still held it unconditionally: guardrailHit sat
in heldForManualReview regardless of what the escalated review found. The 74
held PRs / 14 days kept landing on the maintainer, just with better reviews
attached.
gate.guardrailEscalation.onCleanReview:
hold (default) today's behavior exactly -- human-in-the-loop
proceed a clean escalated review releases the guardrail hold and the normal
approve/merge path continues -- full-autonomy mode
Because it is a manifest field it layers global -> per-repo like everything
else, so repos can be flipped one at a time and the mode is reversible without
a deploy.
Fail-closed on every axis:
- `proceed` is inert unless at least one escalation knob is actually SET: the
release is justified by extra scrutiny, so absent scrutiny nothing vouches.
- the release requires reviewGood -- gate success (which folds in the AI
verdict's blockers) AND green CI -- and the disposition re-checks that
independently of the caller's flag.
- only the guardrail term is released: migration collisions, unlinked-issue
holds, advisory-check holds, and unstable merge states all still hold.
- the manual-hold reason no longer claims "guarded path -> manual review" for
a cleared PR.
* fix(gate): stop labelling a PR for manual review in the same pass that merges it
The manual-review label announces a HOLD. Its emit condition checked
`guardrailHit` alone, so once a clean escalated review released that hold
(guardrailEscalationCleared), the planner emitted both in one pass: the merge
AND the label telling a human to come look at it. Self-contradictory, and it
leaves a manual-review label sitting on merged PRs in precisely the
full-autonomy mode the feature exists to enable.
Also closes the two patch-coverage gaps this PR still carried, both of them
the "test both sides" class:
- agent-actions: no test made guardrailHit, reviewGood and escalationConfigured
true at once, so the fourth conjunct -- `onCleanReview === "proceed"`, the
term the whole feature turns on -- was never evaluated at all. Now asserted
in both directions against identical input, plus the case where "proceed" is
set with NO escalation configured, which must NOT release (that promise is
about an escalated review; honouring it without one would leave guarded paths
weaker than before the feature).
- focus-manifest: onCleanReview was covered through parse but not through
resolveEffectiveSettings. Parsing it and dropping it during resolution would
silently restore always-hold while the manifest still read as configured for
full autonomy.
---------
Co-authored-by: loopover-orb[bot] <296761690+loopover-orb[bot]@users.noreply.github.com>
0 commit comments