Don't hand a worktree writer the LLM auto-approver - #748
Merged
Conversation
for_working_dir drops the session allowlist on purpose — a grant made against the parent checkout doesn't mean the same thing under a worktree root — but it kept approval_fn. Together those invert the trust model: the user's own "allow always" decisions vanish while the thing that can auto-allow without asking them survives. Calls they'd already settled came back as Ask and the evaluator answered on their behalf, so a background writer ran with a weaker human backstop than the session that spawned it. Drop the evaluator. A worktree writer is now strictly no more permissive than its parent, and there is a human to reach — ask_tx is threaded through build_rooted_writer_tools. Prompt deny-lists still propagate; they're terminal and path-independent.
yogthos
force-pushed
the
fix-worktree-approver-inherit
branch
from
August 3, 2026 19:25
460304b to
2c241dc
Compare
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.
Closes dirge-4wpc (PR #670 follow-up).
PermissionChecker::for_working_dirdrops the session allowlist on purpose — a grant the user made against the parent checkout doesn't mean the same thing under a worktree root — but it keptapproval_fn.Those two together invert the trust model. Concretely: you press
aoncargo testin the parent, creating a session grant. A worktree writer runscargo test; your grant is gone, so it falls through toAsk;try_auto_approvehands it to the inherited evaluator;Allowshort-circuits the human prompt (src/agent/tools/mod.rs:895-899). Your explicit decision is dropped while the LLM's authority is kept, so the background writer ends up more auto-approved than the session you're sitting in.Dropping the evaluator makes a worktree writer strictly no more permissive than its parent, and there is a human to reach —
ask_txis threaded throughbuild_rooted_writer_toolsinto these tools, so theAskprompts you rather than failing.Prompt deny-lists still propagate: terminal, un-loosenable, and path-independent, so they carry no cross-root ambiguity.
Tests: one asserting the derived checker has no
approval_fn(fails onmainwith "worktree writer inherited the auto-approver") and a companion pinning the rest of the contract — deny-list propagates, worktree root adopted, session grants stay behind.Docs:
docs/permissions.mdnow states the evaluator's scope explicitly, since "it stands in for the human on each Ask" read as unconditional.