Skip to content

Commit bdb9ced

Browse files
committed
Count shell reads and edits as evidence (CL-6937)
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).
1 parent 928447d commit bdb9ced

9 files changed

Lines changed: 328 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1515

1616
### Agent
1717

18+
- **Shell file work counts as evidence.** A worker that edited with `sed -i`, a
19+
heredoc, or `>` redirection had `editedPaths` empty and salvaged as
20+
`never-edited` — a sticky hard block that then refused the parent an identical
21+
re-dispatch; one that read with `cat`/`head` salvaged as `incomplete-report`.
22+
Both are real work classified as no work. `run_shell` commands are now scanned
23+
for file reads and writes using the same subject expansion the auto-shell
24+
policy uses, so `bash -c` and `env -S` payloads are inspected rather than
25+
trusted.
26+
1827
- **Re-read pressure no longer stops a worker.** The `reReadLimit` thrash hard
1928
stop and its soft `re-read-nudge` are removed: reading one file four times
2029
while editing another, paging a large file, or re-running a grep to verify an

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ In TUI chat mode there is no completion gate — the session stays open across t
106106
Two directors, selected by role:
107107

108108
- **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.
109-
- **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:
109+
- **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:
110110
`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.
111111
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.
112112

docs/PRODUCT.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ Corbits Code fans work out to short-lived **sub-agents** — child agents with t
159159
- **Tasks** are checklist items owned by one agent via `manage_tasks`.
160160
- **Sub-agents** are spawned with the `task` tool (wire name kept; meaning is "spawn a child agent," not "add a checklist item").
161161

162-
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.
162+
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.
163163
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.
164164

165165
## Roadmap (planned, not yet shipped)

src/shell/run-shell-authz.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ const STDIN_READERS = new Set(["cat", "tac", "nl", "rev", "head", "tail", "sort"
125125
// value-taking only for `head` and `tail`; for the other stdin readers the same
126126
// letters are boolean flags (e.g. `wc -c`, `uniq -c`, `sort -c`), so consuming a
127127
// following token there would wrongly drop a real file operand.
128-
const HEAD_TAIL_VALUE_FLAGS = new Set(["-n", "-c", "-C", "--lines", "--bytes"]);
129-
const GREP_VALUE_FLAGS = new Set(["-e", "-f", "-m", "-A", "-B", "-C", "--regexp", "--file"]);
128+
export const HEAD_TAIL_VALUE_FLAGS = new Set(["-n", "-c", "-C", "--lines", "--bytes"]);
129+
export const GREP_VALUE_FLAGS = new Set(["-e", "-f", "-m", "-A", "-B", "-C", "--regexp", "--file"]);
130130

131131
// The head of each pipeline (the stage before the first `|`) is the only stage
132132
// that reads the terminal's stdin; later stages read the pipe. A naive regex
@@ -182,7 +182,7 @@ function pipelineHeads(command: string): string[] {
182182
// classification (classifiers use other paths). A naive whitespace split
183183
// miscounts operands when a pattern or path contains spaces inside quotes
184184
// (e.g. `grep 'a b'` has one operand, not two).
185-
function tokenizeSegment(segment: string): string[] {
185+
export function tokenizeSegment(segment: string): string[] {
186186
const tokens = tokenize(segment);
187187
let i = 0;
188188
while (i < tokens.length && ENV_ASSIGNMENT.test(tokens[i]!)) i++;
@@ -316,7 +316,7 @@ function isDangerousTarget(token: string): boolean {
316316
return false;
317317
}
318318

319-
function programBasename(token: string): string {
319+
export function programBasename(token: string): string {
320320
const bare = token.replace(/['"]/g, "");
321321
const slash = bare.lastIndexOf("/");
322322
return slash >= 0 ? bare.slice(slash + 1) : bare;

src/subagent/index.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,33 @@ describe("sub-agent stop helpers", () => {
560560
).toBe("no-progress");
561561
});
562562

563+
test("shell-only work is not never-edited or incomplete-report (CL-6937)", () => {
564+
const shellState = nextThrashState(EMPTY_THRASH_STATE, [
565+
{ type: "tool_call", name: "run_shell", arguments: { command: "cat src/a.ts" } },
566+
{
567+
type: "tool_call",
568+
name: "run_shell",
569+
arguments: { command: "sed -i '' 's/a/b/' src/a.ts" },
570+
},
571+
]);
572+
const report =
573+
"## Summary\nDid it\n\n## Findings\nx\n\n## Blockers\nNone\n\n## Paths\nsrc/a.ts";
574+
expect(
575+
evaluateSubAgentStop({
576+
hasToolCalls: false,
577+
everHadToolCalls: true,
578+
turnsCompleted: 4,
579+
maxTurns: 30,
580+
consecutiveIdentical: 0,
581+
repeatLimit: 5,
582+
thrashState: shellState,
583+
requireEdit: true,
584+
requireEvidence: true,
585+
lastAssistantText: report,
586+
}),
587+
).toBe("complete");
588+
});
589+
563590
test("re-read pressure no longer stops a worker; turn-budget still does (CL-6936)", () => {
564591
let thrash = EMPTY_THRASH_STATE;
565592
thrash = nextThrashState(thrash, [
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { describe, expect, test } from "bun:test";
2+
3+
import { classifyShellFileEvidence } from "./shell-evidence.js";
4+
5+
describe("classifyShellFileEvidence (CL-6937)", () => {
6+
test("in-place editors count as writes", () => {
7+
expect(classifyShellFileEvidence("sed -i '' 's/a/b/' src/a.ts").writes).toContain("src/a.ts");
8+
expect(classifyShellFileEvidence("perl -pi -e 's/a/b/' src/b.ts").writes).toContain("src/b.ts");
9+
expect(classifyShellFileEvidence("sed -i.bak 's/a/b/' src/c.ts").writes).toContain("src/c.ts");
10+
});
11+
12+
test("sed without an in-place flag is a read, not a write", () => {
13+
const evidence = classifyShellFileEvidence("sed -n '1,20p' src/a.ts");
14+
expect(evidence.writes).toEqual([]);
15+
expect(evidence.reads).toContain("src/a.ts");
16+
});
17+
18+
test("redirection is a write regardless of program", () => {
19+
expect(classifyShellFileEvidence("echo hi > out.txt").writes).toContain("out.txt");
20+
expect(classifyShellFileEvidence("printf x >> out.txt").writes).toContain("out.txt");
21+
expect(classifyShellFileEvidence("cat <<'EOF' > gen.ts\nx\nEOF").writes).toContain("gen.ts");
22+
});
23+
24+
test("readers count as reads with their file operand", () => {
25+
expect(classifyShellFileEvidence("cat src/a.ts").reads).toContain("src/a.ts");
26+
expect(classifyShellFileEvidence("head -n 5 src/a.ts").reads).toContain("src/a.ts");
27+
expect(classifyShellFileEvidence("grep needle src/a.ts").reads).toContain("src/a.ts");
28+
});
29+
30+
test("a reader with no file operand still records evidence keyed by program", () => {
31+
expect(classifyShellFileEvidence("git status | cat").reads).toContain("shell:cat");
32+
});
33+
34+
test("wrapped payloads are inspected, not trusted", () => {
35+
expect(classifyShellFileEvidence("bash -c \"sed -i '' s/a/b/ src/a.ts\"").writes).toContain(
36+
"src/a.ts",
37+
);
38+
});
39+
40+
test("chained commands contribute both sides", () => {
41+
const evidence = classifyShellFileEvidence("cat src/a.ts && tee src/b.ts < src/a.ts");
42+
expect(evidence.reads).toContain("src/a.ts");
43+
expect(evidence.writes).toContain("src/b.ts");
44+
});
45+
46+
test("commands that touch no files yield nothing", () => {
47+
const evidence = classifyShellFileEvidence("bun run check");
48+
expect(evidence.reads).toEqual([]);
49+
expect(evidence.writes).toEqual([]);
50+
});
51+
});

0 commit comments

Comments
 (0)