Merge pull request #559 from corbitsdev/cl-6937-shell-based-reads-and-edits-count-as-zero-evidence-so-real

Count shell reads and edits as evidence (CL-6937)

Author: TheGreatAxios

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

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.
 

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)

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;

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, [

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([]);
+  });
+});