Repository navigation
Handoff example: stricter, honestly described approval gate; timings including the policy; demo command - #297
Merged
Merged
Conversation
…timings that include the policy; working demo command From review of the handoff example's README: - [P1] "It cannot do damage" overstated the gate, and probes went through: `/bin/rm -f owner.txt`, `git restore owner.txt`, `git -C . reset --hard`, `printf changed 1>owner.txt`. confirm-dangerous.ts now parses each command (the program's base name past env/command/nohup/time/VAR=…; git's subcommand past -C/-c/--git-dir…; every truncating redirection `>`, `1>`, `2>`, `&>`, not `>>` or to /dev/null; `tee` without -a; `git checkout` / `restore` of paths, not `restore --staged`; forced and mirror pushes; `stash drop` / `clear`). Checked against 45 commands and writes. The docs call it a guardrail for selected operations, not a sandbox: edits of existing files and what a script does inside are not checked. - [P2] The per-call timings started at ToolExecutionStart, which comes after the before_tool policies, so they left out the pi policy round trip; the "0.1%" did not follow. The demo now reports "loop + policy" (model message end → tool start) and the tool apart: ~3 ms + 0.6–12 ms warm, 9–11 ms + 5.5 ms for a session's first pi call. The percentage is gone. - [P2] `cargo run --example handoff_agent` from a project directory cannot find the crate: the docs use `scripts/try-handoff.sh --demo` (and say `--manifest-path` is needed for cargo run there). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
…has tests From the review of #297: - A real tokenizer (quotes, escapes, operators, redirections incl. `>|`, `&>`, `N>`; heredoc bodies skipped) and a running cwd: `cd sub && echo x > only.txt` was checked against the wrong directory; an unfollowable `cd` makes later paths unknown. - Unknown paths ask: `$VAR`, `~`, backquotes, globs, an unclosed quote (`echo x > ~/.zshrc`, `> "my file.txt"` passed). - More covered: `bash -c` / `sh -c` scripts (read the same way), `xargs` running a deleter, `find -exec rm`, `git rm` (not `--cached`), wrappers (`timeout`, `nice`, `env -i`, `stdbuf`, a leading `\`), `dd of=`, `+ref` / `:ref` pushes, `checkout -f`, `switch -f` / `--discard-changes`. - Fewer false refusals: `sudo` / `truncate` / … match only as the program (`cargo test truncate`, a commit message, a heredoc body asked); `git clean -n` is free; files made by an allowed `tee` / `cp` / `mv` are the agent's; moving into a directory checks the file that lands there. - A refused command's files are no longer recorded as the agent's. - confirm-dangerous.test.mjs: 70 cases (the probes from both reviews, everyday commands, bookkeeping, writes and edits), run in CI with the page tests. - README: the timing column is "tool + after-tool hooks" (the pi adapter always has one), with the concurrent-calls caveat; `sed -i` listed among what the gate does not read. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
yuanhao
added a commit
that referenced
this pull request
Oct 11, 2026
…edocs as the shell does (#298) From review of #297: - [P1] `(cd scratch); rm -f owner.txt` passed: the subshell's cd was taken to apply to the rm. The tokenizer now marks `( … )`, and the checker restores the directory after one. A cd is followed only where it surely applies: a literal existing directory, not in a pipeline, the background or before `||`; otherwise later paths are unknown and ask. - [P1] `echo '<<EOF'` + a newline + `rm -f owner.txt` passed: a pre-pass regex took quoted text for a heredoc and skipped the rm. Heredocs are now recognized in the tokenizer (unquoted `<<WORD`, `<<-`, quoted delimiters), their bodies skipped from the next line to the delimiter. - Tests for both and their variants; on the previous gate they fail on exactly the two reported bypasses. - [P2] README: model and tools do not overlap (the loop awaits each); one response's tool calls run concurrently, so their durations do not add. Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Fixes from a review of the handoff example's README (#296).
[P1] The safety claim was too absolute, and the gate had holes. These four went through without approval:
/bin/rm -f owner.txtgit restore owner.txt--or.were recognizedgit -C . reset --hardprintf changed 1>owner.txt>was treated as stderr-onlyconfirm-dangerous.tsnow parses each command:env/command/nohup/time/VAR=…;-C/-c/--git-dir;>,1>,2>,&>; not>>, nor/dev/null);teewithout-a;checkout/restoreof paths (notrestore --staged);stash drop/clear.I checked 45 cases: the four probes; 22 more that must ask; 14 everyday commands that must stay free (
>>,2>/dev/null,git checkout -b,git restore --staged, a plain push…); and 5 writes and edits.The docs (example README, frontend README, CHANGELOG, the example's header) now call it approval guardrails for selected operations, not a sandbox. Edits of existing files, and what a script does inside, are not checked. For a guarantee, use a container or a disposable copy.
[P2] The timings left out the policy.
ToolExecutionStartis sent after thebefore_toolchain (agent_loop.rs:2545vs:2568), so the per-call numbers missed the pi policy round trip, and the "0.1%" didn't follow. The demo now reports two parts: loop + policy (from the model message's end to the tool's start) and tool. Release build, M2, 5 runs:The percentage is gone.
[P2] The demo command.
cargo run --example handoff_agentfrom a project directory can't find the crate. The docs now usescripts/try-handoff.sh --demo, and the example's header says--manifest-pathis needed forcargo runthere.Checks: all 41 frontend tests pass, run twice.
coding_agent's CI check (? Run \rm -rf build`?,bash ✗`) holds, and lint is clean.Second round (review of this PR)
The first rewrite still split commands with regexes, and a review found concrete bypasses. The gate now tokenizes each command (quotes, escapes, redirections incl.
>|; heredoc bodies skipped) and followscd. Before,cd sub && echo x > only.txtwas checked against the wrong folder.$VAR,~, globs, an unclosed quote.bash -c/sh -c(read recursively),xargs rm,find -exec rm,git rm, the wrapperstimeout/nice/env -i,dd of=,+ref/:refpushes,checkout -f/switch -f.cargo test truncate, a commit message mentioning sudo,git clean -n, heredoc bodies, and repeatedtee test.log/cpare free.confirm-dangerous.test.mjs: 70 cases, run in CI.🤖 Generated with Claude Code
https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG