fix(gate): a denial tells the model to stop, and which no it hit - #300
Merged
Conversation
"error: tool 'x' denied by the gate" said only that something refused. The reliable next move from there was to route around it — denied write_file, reach for a shell heredoc, same effect. The string is the only signal the model gets, so it has to carry the instruction. Card decline and never-list hit both land on Decision.NEVER but mean different things: one is a decision the owner can revisit, the other can never be lifted, and an agent told the wrong one either nags about a permanent denial or abandons a retryable ask. dispatch() now remembers which path denied and picks the matching text. The card text does not claim the owner typed "no" — Approval.DENY also covers the 600s timeout, an unparseable answer, and a card refused because one was already pending. Wording moves to gate_text.py alongside announce_text (gate.py was 4 lines from the 200-line cap, and the emitted strings are their own concern).
Both denial texts contradicted the PR's own argument, in opposite directions. card_denied said "do not retry, do not re-ask" absolutely, which erases the distinction the change exists to draw: the body argues a card decline is a decision the owner can revisit, and the string forbade the retry revisiting needs. The model tells the owner, the owner says go ahead, and the model holds a standing instruction not to. Scoped to "on your own", with the owner's go-ahead named as the exception. never_denied claimed "no approval can lift it", which is false here — gate.never is config, and self-edit/SKILL.md teaches the model to edit gate.never/gate.approved and restart as routine. So the single highest-leverage route around a never-list denial was the one workaround the string did not name. It now says what is true (only the owner, in config.yaml) and forbids that route explicitly. Not an enforcement hole (editing config still cards), but a false absolute is worse than a scoped truth in a string whose whole job is to be believed. The perseveration breaker (agent/loop.py) fires on exactly this path and said "take a different approach" — the workaround both denials forbid, from a second string the model reads at the same moment. Reworded to respect a refusal while keeping the advice for the ordinary repeated-failure case. Lows: SECURITY.md documents the two texts and that they are behavioural, not enforcement; cron/tools.py dropped the same "the owner declined" overclaim on the sibling path. Rationale had been restated in four places — module docstring is now the single home. Left: the split vs #296 (gate_policy.py) — orthogonal concerns out of the same overfull file, mechanical rebase whichever lands second.
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.
Why
`error: tool 'x' denied by the gate` said only that something refused. The reliable next move from there is to route around it — deny `write_file`, reach for a `shell` heredoc, same effect achieved. The returned string is the only signal the model gets about a denial, so it has to carry the instruction, not just the fact.
What
The denial now says stop, explicitly — don't retry, don't re-ask, don't reach the same effect another way, surface it to the owner instead.
And it says which "no" it hit. A card decline and a never-list hit both land on `Decision.NEVER` and printed identical text, but they mean opposite things: one is a decision the owner can revisit, the other can never be lifted. An agent told the wrong one either nags about a permanent denial or abandons a retryable ask. `dispatch()` now remembers which path denied and picks the matching text.
The card text does not claim the owner typed "no." `Approval.DENY` also covers the 600s timeout, an unparseable answer, and a card refused because one was already pending on the thread — so it says "declined, or the card went unanswered."
Structure
Wording moves to `gate_text.py` alongside `announce_text`. `gate.py` was 4 lines from the 200-line hard cap, and the strings the gate emits are a separate concern from the logic that decides. `announce_text` had no callers outside `gate.py`, so the move is zero-churn.
Note this overlaps #296, which splits `gate.py` a different way (`gate_policy.py` for `GatePolicy` + `load_approved`/`save_approved`). The two splits are orthogonal — different concerns leaving the same overfull file — but both touch its import header, so whichever lands second will want a small rebase.
Not addressed
Distinguishing a real decline from a 600s timeout inside `Approval.DENY` — that needs a reason on the enum, and the vaguer wording is accurate today. Worth doing when the ambiguity actually costs something.
Testing
`799 passed` · `ruff check` clean · `mypy` clean.
New test pins that the two denials read differently and that both carry the stop instruction; the two existing denial assertions were updated to the new text.
🤖 Generated with Claude Code