review: trim the sub-agent tool surface to Read/Grep/Bash, and window Read - #325
Conversation
…Grep/Bash, window Read, and align the eval to it LS added nothing over sandboxed Bash; Glob's find -path emulation was wrong, not just limited (* matched across /). Read gains offset/limit windowing so large files stop being silently truncated at the output cap with an unreachable tail. The eval's measured arms drop their Read/Grep/Glob restriction and run the production surface unrestricted, so the A/B measures what production ships by construction. Raised by mojadem on #305; re-anchor A/B run to follow on this PR.
🦋 Changeset detectedLatest commit: 3393ae2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Review live A/BNo reviewable delta: review.md is byte-identical in both arms (baseline |
Review GuidanceCommon patterns4 files: "Glob" (and "LS") removed from ALLOWED_TOOLS / tool-name arrays throughout the codebase as those tools are deleted from createReviewTools - const ALLOWED_TOOLS = ["Read", "Grep", "Glob"];
+ const ALLOWED_TOOLS = ["Read", "Grep"]; |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved — no blocking issues found.
Note: correctness-reviewer not assessed this run (correctness-reviewer output unavailable).
Note: test-adequacy not assessed this run (test-adequacy output unavailable).
Note: completeness not assessed this run (completeness output unavailable).
Note: holistic not assessed this run (holistic output unavailable).
Note: first-principles not assessed this run (first-principles output unavailable).
Note: conventions not assessed this run (conventions output unavailable).
1 participant
Stacked on #305 (base is its branch). Follows from mojadem's review question there (#305 (review)): with every tool subprocess sandboxed identically by srt, named tools earn their place on model ergonomics, not containment.
What changes
ls -laverbatim; Bash does that.find -pathemulation was wrong, not just limited:*matched across/, sosrc/*.tssilently returned nested files and reviewers got a wider file list than they asked for. Finding files goes through Bash, where the model owns the semantics of its own command.cat -nline numbers so findings anchor on real lines, and partial views say which lines of how many they show.bash -lc.allowedTools: ["Read", "Grep", "Glob"]pin (a surface production never ran) and takecreateReviewToolsunrestricted, so the 3-vs-5 tool mismatch class is gone by construction.Exit criterion: a green re-anchor A/B at this branch head on the new surface, since the surface is a measured variable. Dispatching it on this PR; numbers to follow.
1679 tests pass. The harness probe keeps a restricted surface (now Read/Grep) since its job is reproducing a historical configuration; its question was settled as noise on #305.