feat(git-read): read-only git_show/git_diff/git_log as child-safe tools - #66
Merged
Conversation
…ls (closes #61) Reviewer/advisor subagents had no way to inspect git history: their (correct) bash-free tool boundary excluded diffs entirely, capping review quality at final-state reading (the review round on PR #60 hit exactly this). - new extensions/git-read: git_show, git_diff, git_log following the file-search pattern (structured schema, bounded preview, complete output persisted up to 10 MiB, process-group kill on cap) - argv construction is pure and fail-closed: revisions validated against a strict shape (no leading dash, no separators), paths are repo-relative only, and user values always sit behind a separator so they can never parse as flags - tools classified in CHILD_SAFE_PACKAGE_TOOL_NAMES; drift guard now enforces their classification - built-in roles explorer/reviewer/advisor/implementer gain the git tools (still read-only; allowlist only narrows) - search capability group now owns fileSearch + gitRead; intent regex accepts 用 git diff/log/show phrasing, with advertised-phrase tests - replay-safety allowlist extended: git tools are as side-effect-free as fd/rg, so reviewer-type workflow agents stay replay-journaled (e2e proved this gap: preflight failed before the allowlist fix) 772 node:test + 30 vitest green; drift guard passes.
Collaborator
Author
|
合并前审查已完成并推送修复:
最终基于最新 main 验证:796 Node tests + 30 Vitest 全绿;bun run check 通过(仅既存 file-search Effect warnings)。 |
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
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 #61.
Summary
Adds
extensions/git-read: three read-only git tools (git_show,git_diff,git_log) classified as child-safe, so reviewer/advisor subagents can inspect commit history and diffs that their (correct) bash-free tool boundary previously excluded entirely.The motivating failure was real: in the review round on #60, both reviewer subagents reported they could not run
git showand had to review the final tree state instead of the diff, with the parent redoing the diff review before merging.Design
sanitizeTerminalTexton delivered output.src/args.ts, pure and unit-tested): revisions validated against a strict shape (sha /HEAD~n/ name; leading-rejected so a revision can never smuggle a git flag), paths restricted to repo-relative segments (no.., no absolute), and every user value sits behind a--separator.CHILD_SAFE_PACKAGE_TOOL_NAMES; the source-scan drift guard inchild-session.test.tsnow enforces their classification. Only read subcommands are reachable — there is no code path that writes.explorer/reviewer/advisor/implementergain the git tools. Allowlists only ever narrow, so this widens nothing else.gitReadowner inOPENPI_TOOL_SURFACE; thesearchgroup now ownsfileSearch+gitRead. The intent regex accepts 「用 git diff 比较」-style phrasing, and the advertised-phrase test pins it.git_show/git_diff/git_logare as side-effect-free asfd/rg, so they joined the replay-safe allowlist with source-path verification againstextensions/git-read/index.ts. Without this, reviewer-type workflow agents silently stopped being replay-journaled — the e2e's child preflight failed on the missing tools before the fix, which is exactly the guard chain working as designed.Test plan
args.test.ts— argv purity: flag injection, path traversal, revision shapes, limit clampingindex.test.ts— real git repository: log/show/show+path/diff-worktree, error surfacing (bad revision, non-repo) asGitCommandErrorwith stderrexecute.e2e.test.ts) extended fake session exposes the git tools; preflight fail-closed was observed pre-fixreplay-safety.test.ts— git tools are replay-safe with the right source pathbun run checkgreen