From 4fd8c946328387ec9ea02bdfec95f6353a932093 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 23 Aug 2026 12:21:22 -0700 Subject: [PATCH] Count shell reads and edits as evidence (CL-6937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stop policy measured whether a worker did work by counting typed tool calls only. A worker that edited with `sed -i`, a heredoc, or `>` redirection had editedPaths empty and salvaged as never-edited — a HARD_BLOCK_SALVAGES class, so the parent was then refused an identical re-dispatch for the session. One that read with cat/head salvaged as incomplete-report. Both are real work classified as no work. The prompt does prohibit shell file work, but buildGrokLeafAntiThrashNote documents grok reaching for shell first anyway, and that is the family we run. A prompt violation should produce a correction, not a verdict that the work never happened. classifyShellFileEvidence lives in run-shell-authz.ts and reuses expandShellSubjects, so bash -c / env -S / xargs payloads are inspected rather than trusted. Writes are recognized from redirection as well as from the program name, since a missed write is exactly the false salvage this prevents, while a missed read costs a worker nothing. Stacked on cl-6936 (same files). --- CHANGELOG.md | 9 ++ docs/ARCHITECTURE.md | 2 +- docs/PRODUCT.md | 2 +- src/shell/run-shell-authz.ts | 8 +- src/subagent/index.test.ts | 27 ++++ src/subagent/shell-evidence.test.ts | 51 ++++++++ src/subagent/shell-evidence.ts | 190 ++++++++++++++++++++++++++++ src/subagent/thrash.test.ts | 22 ++++ src/subagent/thrash.ts | 24 +++- 9 files changed, 328 insertions(+), 7 deletions(-) create mode 100644 src/subagent/shell-evidence.test.ts create mode 100644 src/subagent/shell-evidence.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f7e04de12..f8e70d1d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,15 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ### Agent +- **Shell file work counts as evidence.** A worker that edited with `sed -i`, a + heredoc, or `>` redirection had `editedPaths` empty and salvaged as + `never-edited` — a sticky hard block that then refused the parent an identical + re-dispatch; one that read with `cat`/`head` salvaged as `incomplete-report`. + Both are real work classified as no work. `run_shell` commands are now scanned + for file reads and writes using the same subject expansion the auto-shell + policy uses, so `bash -c` and `env -S` payloads are inspected rather than + trusted. + - **Re-read pressure no longer stops a worker.** The `reReadLimit` thrash hard stop and its soft `re-read-nudge` are removed: reading one file four times while editing another, paging a large file, or re-running a grep to verify an diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 909f3a65d..0cb3415bd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -106,7 +106,7 @@ In TUI chat mode there is no completion gate — the session stays open across t Two directors, selected by role: - **ChatDirector** (interactive, `src/agent/director.ts`) — Extends `DefaultDirector` with task list tracking, workflow nudges, LSP auto-activation, and multi-turn chat semantics. It never terminates the session: operator declines are surfaced as replies and the reactor stays alive for the next message. Auto mode is toggled by CLI flags (`--auto` / `--no-auto`); there is currently no in-session key to toggle it (default on; constrained envelope — workspace writes and unconstrained shell auto-allow; installs, recursive rm, force/uncontained worktree changes, sensitive-path and opaque-wrapper shell still ask; contained non-force `git worktree add`/`remove`/`prune` and `list` auto-allow; shell file-mutation denied). It is not a separate edit/plan mode. -- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less turn **after tools** completes only with the four-heading envelope (Summary, Findings, Blockers, Paths); a missing envelope nudges once then salvages as **incomplete-report**. A tool-less completion with **zero tool calls in the entire run** is returned as a **never-acted** salvage report (not a successful implement). When `task(intent="implement")` is set, a tool-using run that never wrote/edited/deleted a file is returned as **never-edited** instead of complete — so a pure-explore "plan" cannot look shipped to the parent (tracked via `thrashState.editedPaths` from `edit_file` / `write_file` / `delete_file`). Explore/read-only workers that used tools then replied with findings remain normal completes. Hard stops also fire after 5 consecutive identical tool-call fingerprints (**no-progress**, mirroring the director-level `IDENTICAL_REPEAT_MIN` threshold) or after the leaf turn budget (**turn-budget**, default 30, overridable via `task(maxTurns)`, agent profile `maxTurns`, or `settings.subagentMaxTurns`; floor ≥1, no hard upper cap), each returning a structured salvage report (reason, partial findings, blockers) so a looping child cannot burn tokens indefinitely. Re-read counts are **not** a stop signal: `src/subagent/thrash.ts` keeps read/edit bookkeeping only to serve the `requireEdit` / `requireEvidence` checks above, because the fingerprint period detector already catches a genuinely repeating read cycle on the evidence that it repeats, while a raw count cannot separate four reads across real progress from four reads in a loop (CL-6936). A third hard stop, **repetition**, is detected outside the director entirely: +- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less turn **after tools** completes only with the four-heading envelope (Summary, Findings, Blockers, Paths); a missing envelope nudges once then salvages as **incomplete-report**. A tool-less completion with **zero tool calls in the entire run** is returned as a **never-acted** salvage report (not a successful implement). When `task(intent="implement")` is set, a tool-using run that never wrote/edited/deleted a file is returned as **never-edited** instead of complete — so a pure-explore "plan" cannot look shipped to the parent (tracked via `thrashState.editedPaths` from `edit_file` / `write_file` / `delete_file` / `apply_patch`, **and** from file work done through `run_shell` — `sed -i`, redirection, `tee`, `cp`/`mv` — classified by `classifyShellFileEvidence` in `src/shell/run-shell-authz.ts` over the same subject expansion the auto-shell policy uses, so a worker that edits with shell is not reported as having done nothing (CL-6937)). Explore/read-only workers that used tools then replied with findings remain normal completes. Hard stops also fire after 5 consecutive identical tool-call fingerprints (**no-progress**, mirroring the director-level `IDENTICAL_REPEAT_MIN` threshold) or after the leaf turn budget (**turn-budget**, default 30, overridable via `task(maxTurns)`, agent profile `maxTurns`, or `settings.subagentMaxTurns`; floor ≥1, no hard upper cap), each returning a structured salvage report (reason, partial findings, blockers) so a looping child cannot burn tokens indefinitely. Re-read counts are **not** a stop signal: `src/subagent/thrash.ts` keeps read/edit bookkeeping only to serve the `requireEdit` / `requireEvidence` checks above, because the fingerprint period detector already catches a genuinely repeating read cycle on the evidence that it repeats, while a raw count cannot separate four reads across real progress from four reads in a loop (CL-6936). A third hard stop, **repetition**, is detected outside the director entirely: `runSubAgent`'s stream sink watches the streamed text of the in-flight cycle for degenerate token loops (`src/subagent/repetition.ts`) — format chars (ZWSP, BOM, bidi marks, soft hyphen, …) stripped then whitespace-collapsed raw text, a smallest-period KMP check over the probe tail, default window >= 16 chars repeated >= 8 times, evaluated every 256 streamed chars — and on a hit aborts the run controller mid-cycle, returning a `repetition` salvage report that leads with the looped window and warns the parent against re-dispatching the identical brief. `inference.thinking.delta` is sampled the same way on its own buffer, but with digit runs folded to one placeholder and a shorter window (>= 4 chars repeated >= 32 times), gated to periods <= 16 chars once folded: thinking is never rendered to the user, so a monotonic counter (e.g. `0/1 1/2 2/3 …`, which stays non-periodic and escapes the raw-text check) can be caught, but folding still erases real information — a healthy templated enumeration line becomes byte-identical to its neighbors once digits are erased, so the period-length cap only lets counter-shaped folded periods (a handful of chars) through and refuses the much longer periods a folded prose line produces. Because directors only see completed turns, this is the only stop that can catch a loop inside a single turn that never finishes. A one-shot **report-forced** signal fires a few turns before the cap while the leaf is still tooling — it is not a stop: the director injects a wrap-up nudge and lets the leaf finish on its own, so turn-budget stays reachable for a leaf still making progress. Operator/parent cancel after any progress likewise returns a **cancelled** salvage report (partial findings + tool activity) instead of a bare cancel string; cancel before progress still surfaces as cancelled-by-operator. Optional `task(tier=)` (`fast` | `standard` | `clever`) overrides profile inference, profile tier, and the parent provider for that spawn only, and fails closed when the tier is unconfigured. The parent `task` tool keeps a session-scoped brief-dispatch ledger (`src/subagent/brief-dispatch.ts`): fingerprints cover prompt + agent + intent + success_criteria + do_not (not maxTurns/description/tier). After thrash / no-progress / repetition / never-acted / never-edited salvage, an identical re-dispatch is hard-blocked for the rest of the parent chat; change at least one fingerprint field to force a re-run. Turn-budget salvage still invites a higher maxTurns for a few same-brief retries without a successful complete, then flips the parent hint to stop and change approach (soft — further identical dispatches are still admitted). A successful complete resets the same-brief retry budget. diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index fc306d1a9..e369e96fd 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -159,7 +159,7 @@ Corbits Code fans work out to short-lived **sub-agents** — child agents with t - **Tasks** are checklist items owned by one agent via `manage_tasks`. - **Sub-agents** are spawned with the `task` tool (wire name kept; meaning is "spawn a child agent," not "add a checklist item"). -Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip and fleet board show who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. Workers hard-stop after 2 consecutive identical tool calls, when their inference-turn budget is exhausted (default 30; parent can pass `maxTurns` per dispatch; profiles and global settings can raise the default; no hard upper cap), when they finish without ever using tools (never-acted salvage — planning/prose only is not a successful implement), or when `intent=implement` finishes after tools but without any file write/edit/delete (never-edited salvage — a pure-explore plan is not a successful implement). Re-read counts never hard-stop a worker, and look _volume_ is not a stop either — an implement may read hundreds of files before the first edit, and a repeating read cycle is caught by fingerprint detection instead. Near the turn budget a one-shot nudge asks the worker to wrap up and write its report. Each hard stop returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone. +Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip and fleet board show who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. Workers hard-stop after 2 consecutive identical tool calls, when their inference-turn budget is exhausted (default 30; parent can pass `maxTurns` per dispatch; profiles and global settings can raise the default; no hard upper cap), when they finish without ever using tools (never-acted salvage — planning/prose only is not a successful implement), or when `intent=implement` finishes after tools but without any file write/edit/delete (never-edited salvage — a pure-explore plan is not a successful implement). File work done through the shell counts as real work here even though the prompt asks for the typed tools: a prompt violation earns a correction, not a verdict that the work never happened. Re-read counts never hard-stop a worker, and look _volume_ is not a stop either — an implement may read hundreds of files before the first edit, and a repeating read cycle is caught by fingerprint detection instead. Near the turn budget a one-shot nudge asks the worker to wrap up and write its report. Each hard stop returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone. The parent tracks same-brief fingerprints for the session (`src/subagent/brief-dispatch.ts`): after no-progress / repetition / never-acted / never-edited salvage, an identical re-dispatch is refused — change prompt, agent, intent, success_criteria, and/or do_not to unlock a new run (`maxTurns` or tier alone does not). Turn-budget salvage still allows a few same-brief retries with a higher `maxTurns`, then flips the parent hint to stop and change approach; a successful complete resets the same-brief retry budget. ## Roadmap (planned, not yet shipped) diff --git a/src/shell/run-shell-authz.ts b/src/shell/run-shell-authz.ts index 963d41d45..b59aefb2a 100644 --- a/src/shell/run-shell-authz.ts +++ b/src/shell/run-shell-authz.ts @@ -125,8 +125,8 @@ const STDIN_READERS = new Set(["cat", "tac", "nl", "rev", "head", "tail", "sort" // value-taking only for `head` and `tail`; for the other stdin readers the same // letters are boolean flags (e.g. `wc -c`, `uniq -c`, `sort -c`), so consuming a // following token there would wrongly drop a real file operand. -const HEAD_TAIL_VALUE_FLAGS = new Set(["-n", "-c", "-C", "--lines", "--bytes"]); -const GREP_VALUE_FLAGS = new Set(["-e", "-f", "-m", "-A", "-B", "-C", "--regexp", "--file"]); +export const HEAD_TAIL_VALUE_FLAGS = new Set(["-n", "-c", "-C", "--lines", "--bytes"]); +export const GREP_VALUE_FLAGS = new Set(["-e", "-f", "-m", "-A", "-B", "-C", "--regexp", "--file"]); // The head of each pipeline (the stage before the first `|`) is the only stage // that reads the terminal's stdin; later stages read the pipe. A naive regex @@ -182,7 +182,7 @@ function pipelineHeads(command: string): string[] { // classification (classifiers use other paths). A naive whitespace split // miscounts operands when a pattern or path contains spaces inside quotes // (e.g. `grep 'a b'` has one operand, not two). -function tokenizeSegment(segment: string): string[] { +export function tokenizeSegment(segment: string): string[] { const tokens = tokenize(segment); let i = 0; while (i < tokens.length && ENV_ASSIGNMENT.test(tokens[i]!)) i++; @@ -316,7 +316,7 @@ function isDangerousTarget(token: string): boolean { return false; } -function programBasename(token: string): string { +export function programBasename(token: string): string { const bare = token.replace(/['"]/g, ""); const slash = bare.lastIndexOf("/"); return slash >= 0 ? bare.slice(slash + 1) : bare; diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 5ce3baab9..50a35c05f 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -560,6 +560,33 @@ describe("sub-agent stop helpers", () => { ).toBe("no-progress"); }); + test("shell-only work is not never-edited or incomplete-report (CL-6937)", () => { + const shellState = nextThrashState(EMPTY_THRASH_STATE, [ + { type: "tool_call", name: "run_shell", arguments: { command: "cat src/a.ts" } }, + { + type: "tool_call", + name: "run_shell", + arguments: { command: "sed -i '' 's/a/b/' src/a.ts" }, + }, + ]); + const report = + "## Summary\nDid it\n\n## Findings\nx\n\n## Blockers\nNone\n\n## Paths\nsrc/a.ts"; + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + everHadToolCalls: true, + turnsCompleted: 4, + maxTurns: 30, + consecutiveIdentical: 0, + repeatLimit: 5, + thrashState: shellState, + requireEdit: true, + requireEvidence: true, + lastAssistantText: report, + }), + ).toBe("complete"); + }); + test("re-read pressure no longer stops a worker; turn-budget still does (CL-6936)", () => { let thrash = EMPTY_THRASH_STATE; thrash = nextThrashState(thrash, [ diff --git a/src/subagent/shell-evidence.test.ts b/src/subagent/shell-evidence.test.ts new file mode 100644 index 000000000..8f9848331 --- /dev/null +++ b/src/subagent/shell-evidence.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test"; + +import { classifyShellFileEvidence } from "./shell-evidence.js"; + +describe("classifyShellFileEvidence (CL-6937)", () => { + test("in-place editors count as writes", () => { + expect(classifyShellFileEvidence("sed -i '' 's/a/b/' src/a.ts").writes).toContain("src/a.ts"); + expect(classifyShellFileEvidence("perl -pi -e 's/a/b/' src/b.ts").writes).toContain("src/b.ts"); + expect(classifyShellFileEvidence("sed -i.bak 's/a/b/' src/c.ts").writes).toContain("src/c.ts"); + }); + + test("sed without an in-place flag is a read, not a write", () => { + const evidence = classifyShellFileEvidence("sed -n '1,20p' src/a.ts"); + expect(evidence.writes).toEqual([]); + expect(evidence.reads).toContain("src/a.ts"); + }); + + test("redirection is a write regardless of program", () => { + expect(classifyShellFileEvidence("echo hi > out.txt").writes).toContain("out.txt"); + expect(classifyShellFileEvidence("printf x >> out.txt").writes).toContain("out.txt"); + expect(classifyShellFileEvidence("cat <<'EOF' > gen.ts\nx\nEOF").writes).toContain("gen.ts"); + }); + + test("readers count as reads with their file operand", () => { + expect(classifyShellFileEvidence("cat src/a.ts").reads).toContain("src/a.ts"); + expect(classifyShellFileEvidence("head -n 5 src/a.ts").reads).toContain("src/a.ts"); + expect(classifyShellFileEvidence("grep needle src/a.ts").reads).toContain("src/a.ts"); + }); + + test("a reader with no file operand still records evidence keyed by program", () => { + expect(classifyShellFileEvidence("git status | cat").reads).toContain("shell:cat"); + }); + + test("wrapped payloads are inspected, not trusted", () => { + expect(classifyShellFileEvidence("bash -c \"sed -i '' s/a/b/ src/a.ts\"").writes).toContain( + "src/a.ts", + ); + }); + + test("chained commands contribute both sides", () => { + const evidence = classifyShellFileEvidence("cat src/a.ts && tee src/b.ts < src/a.ts"); + expect(evidence.reads).toContain("src/a.ts"); + expect(evidence.writes).toContain("src/b.ts"); + }); + + test("commands that touch no files yield nothing", () => { + const evidence = classifyShellFileEvidence("bun run check"); + expect(evidence.reads).toEqual([]); + expect(evidence.writes).toEqual([]); + }); +}); diff --git a/src/subagent/shell-evidence.ts b/src/subagent/shell-evidence.ts new file mode 100644 index 000000000..9fa58d783 --- /dev/null +++ b/src/subagent/shell-evidence.ts @@ -0,0 +1,190 @@ +// --- Shell file evidence (CL-6937) ----------------------------------------- +// +// The stop policy measures whether a worker did real work by counting typed +// tool calls. Work done through run_shell was invisible to it, so a worker that +// edited with `sed -i` salvaged as never-edited (a sticky hard block) and one +// that read with `cat` salvaged as incomplete-report. The prompt does prohibit +// shell file work, but a prompt violation should produce a correction, not a +// verdict that the work never happened. +// +// This reuses the same subject expansion the auto-shell policy uses, so +// `bash -c`, `env -S`, and xargs payloads are inspected rather than trusted. + +import { splitChainedCommand } from "../permission/command.js"; +import { + expandShellSubjects, + GREP_VALUE_FLAGS, + HEAD_TAIL_VALUE_FLAGS, + programBasename, + tokenizeSegment, +} from "../shell/run-shell-authz.js"; + +const SHELL_READ_PROGRAMS: ReadonlySet = new Set([ + "cat", + "head", + "tail", + "grep", + "egrep", + "fgrep", + "rg", + "ag", + "ack", + "awk", + "sed", + "diff", + "find", + "fd", + "wc", + "nl", + "cut", + "sort", + "uniq", + "od", + "xxd", + "strings", + "jq", + "yq", + "file", + "stat", + "ls", +]); + +/** Programs whose ordinary use rewrites a file operand in place. */ +const SHELL_WRITE_PROGRAMS: ReadonlySet = new Set([ + "tee", + "cp", + "mv", + "install", + "touch", + "truncate", + "patch", + "ln", +]); + +/** In-place editors: only a write when the in-place flag is actually present. */ +const SHELL_IN_PLACE_PROGRAMS: ReadonlySet = new Set(["sed", "perl", "ruby", "gsed"]); + +const IN_PLACE_FLAG = /^-{1,2}(i|in-place)(=.*)?$/; +/** `sed -i.bak`, `perl -pi -e`, `sed -Ei` — the flag is fused with other letters. */ +const FUSED_IN_PLACE_FLAG = /^-[A-Za-z]*i/; + +export interface ShellFileEvidence { + /** Keys for paths (or programs) the command read. */ + reads: string[]; + /** Keys for paths (or programs) the command wrote. */ + writes: string[]; +} + +function evidenceKey(program: string, operand: string | undefined): string { + return operand !== undefined && operand.length > 0 ? operand : `shell:${program}`; +} + +/** + * Flags whose value is a separate token, so `head -n 5 f` does not read "5". + * Union of the reader flag sets above plus the common in-place/script ones. + */ +const EVIDENCE_VALUE_FLAGS: ReadonlySet = new Set([ + ...HEAD_TAIL_VALUE_FLAGS, + ...GREP_VALUE_FLAGS, + "-e", + "-E", + "-d", + "-t", + "-s", + "--expression", + "--delimiter", +]); + +/** First operand that is not a flag or a flag value, skipping `skip` of them. */ +function firstOperand(args: readonly string[], skip: number): string | undefined { + let skipped = 0; + for (let i = 0; i < args.length; i++) { + const arg = args[i]!; + if (arg === "--") continue; + if (arg.startsWith("-")) { + if (EVIDENCE_VALUE_FLAGS.has(arg)) i += 1; + continue; + } + if (skipped < skip) { + skipped += 1; + continue; + } + return arg.replace(/['"]/g, ""); + } + return undefined; +} + +/** A redirect target is one word: heredoc bodies arrive in the same string. */ +function redirectTarget(raw: string): string | undefined { + const word = raw.replace(/['"]/g, "").trim().split(/\s/)[0]; + return word !== undefined && word.length > 0 ? word : undefined; +} + +function classifySegment(segment: string, evidence: ShellFileEvidence): void { + const tokens = tokenizeSegment(segment); + if (tokens.length === 0) return; + + // Output redirection is a write regardless of the program: `echo x > f`, + // heredocs (`cat <<'EOF' > f`), `>>` appends. + for (let i = 0; i < tokens.length; i++) { + const token = tokens[i]!; + const match = /^>{1,2}$/.exec(token); + if (match !== null) { + const target = tokens[i + 1]; + const named = target === undefined ? undefined : redirectTarget(target); + if (named !== undefined) evidence.writes.push(named); + continue; + } + const fused = /^>{1,2}(?!$)(.+)$/.exec(token); + if (fused !== null) { + const named = redirectTarget(fused[1]!); + if (named !== undefined) evidence.writes.push(named); + } + } + + const program = programBasename(tokens[0]!); + const args = tokens.slice(1); + + if (SHELL_IN_PLACE_PROGRAMS.has(program)) { + const inPlace = args.some( + (arg) => IN_PLACE_FLAG.test(arg) || (arg.startsWith("-") && FUSED_IN_PLACE_FLAG.test(arg)), + ); + if (inPlace) { + // sed/perl take the script before the file operand, unless -e already + // consumed it (`perl -pi -e 's/a/b/' f`). + const scriptInFlag = args.some((arg) => arg === "-e" || arg === "--expression"); + evidence.writes.push(evidenceKey(program, firstOperand(args, scriptInFlag ? 0 : 1))); + return; + } + } + if (SHELL_WRITE_PROGRAMS.has(program)) { + evidence.writes.push(evidenceKey(program, firstOperand(args, 0))); + return; + } + if (SHELL_READ_PROGRAMS.has(program)) { + // grep-likes take the pattern first, so their file operand is the second. + const skip = program === "grep" || program === "egrep" || program === "fgrep" ? 1 : 0; + evidence.reads.push(evidenceKey(program, firstOperand(args, skip))); + } +} + +/** + * Reads and writes a run_shell command performs on files, for the stop policy's + * requireEdit / requireEvidence checks. Best effort by design: a missed read + * costs a worker nothing (the typed tools remain the primary evidence), while a + * missed write is exactly the false salvage this exists to prevent, so writes + * are recognized from redirection as well as from the program name. + */ +export function classifyShellFileEvidence(command: string): ShellFileEvidence { + const evidence: ShellFileEvidence = { reads: [], writes: [] }; + const { subjects } = expandShellSubjects(command); + for (const subject of subjects) { + for (const segment of splitChainedCommand(subject)) { + classifySegment(segment, evidence); + } + } + return { + reads: [...new Set(evidence.reads)], + writes: [...new Set(evidence.writes)], + }; +} diff --git a/src/subagent/thrash.test.ts b/src/subagent/thrash.test.ts index f0c678fe8..bb0258cc9 100644 --- a/src/subagent/thrash.test.ts +++ b/src/subagent/thrash.test.ts @@ -126,6 +126,28 @@ describe("thrash pure module", () => { expect(state.totalToolCalls).toBe(1); }); + test("run_shell file work counts as read and edit evidence (CL-6937)", () => { + const shell = (command: string): ThrashToolCallBlock => ({ + type: "tool_call", + name: "run_shell", + arguments: { command }, + }); + const edited = applyAll([shell("sed -i '' 's/a/b/' src/a.ts")]); + expect(edited.editedPaths.has("src/a.ts")).toBe(true); + + const heredoc = applyAll([shell("cat <<'EOF' > src/gen.ts\nx\nEOF")]); + expect(heredoc.editedPaths.has("src/gen.ts")).toBe(true); + + const readOnly = applyAll([shell("head -n 40 src/a.ts")]); + expect(readOnly.readCounts.get("src/a.ts")).toBe(1); + expect(readOnly.editedPaths.size).toBe(0); + + const neutral = applyAll([shell("bun run check")]); + expect(neutral.readCounts.size).toBe(0); + expect(neutral.editedPaths.size).toBe(0); + expect(neutral.totalToolCalls).toBe(1); + }); + test("chunked reads key by offset, whole-file reads key by path", () => { const state = applyAll([ read("big.ts", { offset: 0, limit: 500 }), diff --git a/src/subagent/thrash.ts b/src/subagent/thrash.ts index 2fa3a9ed0..d5f27dc00 100644 --- a/src/subagent/thrash.ts +++ b/src/subagent/thrash.ts @@ -8,10 +8,14 @@ * spread across real progress from four reads in a loop (CL-6936). * * The state this module accumulates is consumed by evaluateSubAgentStop's - * requireEdit / requireEvidence checks, not by a stop of its own. + * requireEdit / requireEvidence checks, not by a stop of its own. Reads and + * writes performed through run_shell count as evidence there (CL-6937) — the + * prompt prohibits shell file work, but a prompt violation deserves a + * correction, not a verdict that the work never happened. */ import { isProductMutationTool, productMutationPaths } from "../agent/product-mutation-tools.js"; +import { classifyShellFileEvidence } from "./shell-evidence.js"; /** Tunable thresholds for force-report detection. */ export interface ThrashConfig { @@ -51,6 +55,7 @@ export interface ThrashToolCallBlock { const READ_TOOLS = new Set(["read_file"]); const SEARCH_TOOLS = new Set(["grep", "search_files"]); +const SHELL_TOOL = "run_shell"; function parseArgs(raw: unknown): Record { let args: unknown = raw ?? {}; @@ -121,6 +126,23 @@ export function nextThrashState( if (readCounts === null) readCounts = new Map(prev.readCounts); const key = searchKey(name, args); readCounts.set(key, (readCounts.get(key) ?? 0) + 1); + } else if (name === SHELL_TOOL) { + // Shell file work is evidence too, or a worker that edits with sed -i + // salvages as never-edited and is then refused re-dispatch (CL-6937). + const command = args.command; + if (typeof command === "string" && command.length > 0) { + const evidence = classifyShellFileEvidence(command); + if (evidence.reads.length > 0) { + if (readCounts === null) readCounts = new Map(prev.readCounts); + for (const key of evidence.reads) { + readCounts.set(key, (readCounts.get(key) ?? 0) + 1); + } + } + if (evidence.writes.length > 0) { + if (editedPaths === null) editedPaths = new Set(prev.editedPaths); + for (const key of evidence.writes) editedPaths.add(key); + } + } } else if (isProductMutationTool(name)) { const paths = productMutationPaths(name, args); if (paths.length > 0) {