Fix cross-subtree trace leak and add an aggregate output cap

Review on #599 found two real issues:

- findAgentTraceDir walked the whole (flat, shared) subagents/ tree
  from the root workdirBase, so a Tier-2 nested orchestrator could
  read any worker's trace, not just its own descendants. Wired
  assertCanTargetAgent (authority.ts's first live call site) using the
  worker's own SubAgentSessionStore id and the store's existing
  parentSessionId chain, rather than reshaping the on-disk layout —
  the disk tree is deliberately flat across the whole fleet (shared by
  worktrees and intervention logs too), so a structural per-subtree
  root would be a much larger change. To make that id available,
  run.ts now names a worker's trace directory after its session-store
  id when one is supplied (task-tool.ts passes it), instead of always
  minting a fresh disk-only id.
- The per-entry, entry-count, and turn-window caps multiply (500 *
  4,000 = 2,000,000 chars). Added a total-output character cap that
  stops filling entries once reached and reports the remainder via the
  existing `omitted` block.

Noted but not changed: readAllTurns loads each full segment before
bounds apply. Segments are already bounded to ~256KB by the writer, so
this isn't unbounded, but avoiding the read entirely needs a line-count
index or a streaming reader — left as a follow-up rather than
expanding this fix.

Author: TheGreatAxios

src/agent/tools.ts

@@ -310,8 +310,10 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
                 }),
               ]
             : []),
-          // Tier 1: the primary session is always an orchestrator, so this is
-          // unconditional wherever task/search_agents are mounted.
+          // Tier 1: the primary session is always an orchestrator and may
+          // target any worker (assertCanTargetAgent's rule), so no authority
+          // context is passed here — omitting it is treated as unrestricted,
+          // matching Tier 1's actual authority.
           createReadAgentTraceTool(args.subAgent.getWorkdirBase),
         ]
       : []),

src/subagent/run.ts

@@ -485,8 +485,16 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<string> {
         // Every worker at every nesting depth is created under the same root
         // workdirBase (nestedDispatch.getWorkdirBase is threaded through
         // unchanged, never rebound to this worker's own dir), so the trace
-        // reader's search root is that same function.
-        createReadAgentTraceTool(nd.getWorkdirBase),
+        // reader's search root is that same function. Descendant-only
+        // scoping is enforced inside the tool via assertCanTargetAgent,
+        // reusing the fleet nodes SubAgentSessionStore already tracks and
+        // this worker's own store id (params.id) — not the disk layout,
+        // which is intentionally flat across the whole fleet.
+        createReadAgentTraceTool(nd.getWorkdirBase, {
+          actorId: params.id,
+          tier,
+          getNodes: () => nd.sessions?.list() ?? [],
+        }),
       ];
     }
 
@@ -584,7 +592,14 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<string> {
       },
     });
 
-    const workdir = join(params.workdirBase, "subagents", generateSessionId());
+    // Reuse the caller's session-store id as the on-disk directory name
+    // when it is safe as a path segment, so read_agent_trace's descendant
+    // check can walk the same parentSessionId chain SubAgentSessionStore
+    // already tracks instead of needing a second, disk-only identity
+    // scheme.
+    const safeRequestedId =
+      params.id !== undefined && /^[A-Za-z0-9_-]+$/.test(params.id) ? params.id : undefined;
+    const workdir = join(params.workdirBase, "subagents", safeRequestedId ?? generateSessionId());
     await mkdir(workdir, { recursive: true });
     // One record per stop/nudge, with its measured value beside its threshold,
     // written into this leaf's own trace dir (CL-6938).

src/subagent/task-tool.ts

@@ -804,6 +804,10 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
             ...sandbox,
             cwd: worktreeCwd ?? deps.cwd,
             workdirBase: deps.getWorkdirBase(),
+            // Same id as the SubAgentSessionStore record so read_agent_trace's
+            // descendant check (authority.ts assertCanTargetAgent) can reuse the
+            // store's parentSessionId chain instead of a second identity scheme.
+            ...(session !== undefined ? { id: session.id } : {}),
             provider,
             ...(settings !== undefined ? { settings } : {}),
             ...(catalog !== undefined ? { catalog } : {}),

src/subagent/trace-reader.test.ts

@@ -9,6 +9,7 @@ import {
   listUniqueSubdirs,
   readAgentTrace,
   MAX_TRACE_ENTRY_LIMIT,
+  MAX_TRACE_TOTAL_CHARS,
   MAX_TRACE_TURN_WINDOW,
 } from "./trace-reader.js";
 
@@ -153,6 +154,27 @@ describe("readAgentTrace", () => {
     expect(result.omitted!.hint.length).toBeGreaterThan(0);
   });
 
+  test("never exceeds the total-output character cap regardless of entry/window caps", async () => {
+    const root = tempDir();
+    const childDir = path.join(root, "subagents", "worker-1");
+    const turns = Array.from({ length: 600 }, (_, i) => ({
+      role: "assistant",
+      content: [{ type: "text", text: `turn ${i} `.repeat(1000) }], // ~5,000 chars each
+    }));
+    writeTurns(childDir, turns);
+
+    const result = await readAgentTrace(root, "worker-1", {
+      fromTurn: 0,
+      toTurn: 600,
+      limit: MAX_TRACE_ENTRY_LIMIT,
+    });
+    const totalChars = result.entries.reduce((sum, e) => sum + e.content.length, 0);
+    expect(totalChars).toBeLessThanOrEqual(MAX_TRACE_TOTAL_CHARS);
+    expect(result.entriesTruncated).toBe(true);
+    expect(result.omitted).not.toBeNull();
+    expect(result.omitted!.reason).toContain("total output cap");
+  });
+
   test("never exceeds the hard entry-limit cap regardless of requested limit", async () => {
     const root = tempDir();
     const childDir = path.join(root, "subagents", "worker-1");

src/subagent/trace-reader.ts

@@ -10,10 +10,11 @@
  * SubAgentSessionStore (which a process restart or a killed worker can
  * leave with nothing).
  *
- * Every read here is bounded: a fixed turn window, a fixed entry count, and
- * a per-entry character cap, each capped again by a hard maximum regardless
- * of what the caller asks for. No argument combination can pull an
- * unbounded blob into the parent's context.
+ * Every read here is bounded on four independent axes — turn window, entry
+ * count, per-entry characters, and total output characters (the first three
+ * multiply, so they are each capped again by a total-output ceiling) — each
+ * with a hard maximum regardless of what the caller asks for. No argument
+ * combination can pull an unbounded blob into the parent's context.
  */
 
 import fs from "node:fs";
@@ -29,6 +30,10 @@ export const MAX_TRACE_TURN_WINDOW = 200;
 export const DEFAULT_TRACE_ENTRY_LIMIT = 200;
 export const MAX_TRACE_ENTRY_LIMIT = 500;
 export const MAX_TRACE_ENTRY_CHARS = 4_000;
+// Per-entry/entry-count/turn-window caps each bound one axis, but multiply
+// together (500 entries * 4,000 chars = 2,000,000 chars in one call). This
+// caps the total regardless of how the other axes are combined.
+export const MAX_TRACE_TOTAL_CHARS = 20_000;
 
 // A pathological or runaway fleet tree should fail the search cheaply rather
 // than walk forever; a worker this deep or a fleet this large is itself a
@@ -196,6 +201,18 @@ function parseTurnsTolerant(text: string): { turns: RawTurn[]; warnings: number
   return { turns, warnings };
 }
 
+/**
+ * Reads and parses every segment before the caller's window/limit bounds
+ * apply, so a not-yet-rotated active segment is loaded whole regardless of
+ * how small a slice the caller actually wants. In practice each segment is
+ * itself bounded to ~256KB by the writer (createSegmentedJSONLWriter's
+ * DEFAULT_MAX_SEGMENT_BYTES), so this cannot grow unboundedly with a
+ * worker's total history the way reading turns.jsonl as one file could —
+ * but avoiding this read entirely (only touching the segments the requested
+ * turn range actually falls in) needs either a cheap line-count index or a
+ * streaming reader, which is a larger change than this fix; tracked as a
+ * follow-up rather than expanding this one.
+ */
 async function readAllTurns(dir: string): Promise<{ turns: RawTurn[]; warnings: number }> {
   const segments = await listSegmentFiles(dir, TURNS_FILE);
   const turns: RawTurn[] = [];
@@ -325,6 +342,8 @@ export async function readAgentTrace(
 
   const entries: TraceEntry[] = [];
   let entriesTruncated = false;
+  let totalChars = 0;
+  let stopReason: "entry-limit" | "total-chars" | null = null;
   let lastReadTurn = fromTurn;
   outer: for (let i = fromTurn; i < toTurn; i++) {
     lastReadTurn = i;
@@ -335,8 +354,15 @@ export async function readAgentTrace(
       if (kindsFilter !== null && !kindsFilter.has(entry.kind)) continue;
       if (entries.length >= limit) {
         entriesTruncated = true;
+        stopReason = "entry-limit";
+        break outer;
+      }
+      if (totalChars + entry.content.length > MAX_TRACE_TOTAL_CHARS) {
+        entriesTruncated = true;
+        stopReason = "total-chars";
         break outer;
       }
+      totalChars += entry.content.length;
       entries.push(entry);
     }
   }
@@ -349,9 +375,12 @@ export async function readAgentTrace(
   const omitted: TraceOmission | null =
     turnsBefore > 0 || turnsAfter > 0
       ? {
-          reason: entriesTruncated
-            ? "entry limit reached before the requested turn range finished reading"
-            : "turn window bounded to the default/requested range",
+          reason:
+            stopReason === "entry-limit"
+              ? "entry limit reached before the requested turn range finished reading"
+              : stopReason === "total-chars"
+                ? `total output cap (${MAX_TRACE_TOTAL_CHARS} chars) reached before the requested turn range finished reading`
+                : "turn window bounded to the default/requested range",
           turnsBefore,
           turnsAfter,
           hint:

src/subagent/trace-tool.test.ts

@@ -4,6 +4,7 @@ import os from "node:os";
 import path from "node:path";
 
 import { createReadAgentTraceTool } from "./trace-tool.js";
+import type { FleetNode } from "./authority.js";
 
 function tempDir(): string {
   return fs.mkdtempSync(path.join(os.tmpdir(), "trace-tool-"));
@@ -45,4 +46,74 @@ describe("createReadAgentTraceTool", () => {
     expect(text).toContain("worker-1");
     expect(text).toContain("hello");
   });
+
+  describe("descendant-only scoping (two sibling subtrees under one flat root)", () => {
+    // Every worker at every nesting depth lands under the same root
+    // subagents/ dir (see run.ts), so on disk workerA1 and workerY are
+    // indistinguishable siblings. Authority comes entirely from the fleet
+    // node list (parentSessionId chain), not from directory structure.
+    const nodes: FleetNode[] = [
+      { id: "orchA" },
+      { id: "workerA1", parentSessionId: "orchA" },
+      { id: "orchB" },
+      { id: "workerY", parentSessionId: "orchB" },
+    ];
+
+    function setUpRoot(): string {
+      const root = tempDir();
+      writeTurns(path.join(root, "subagents", "workerA1"), [
+        { role: "assistant", content: [{ type: "text", text: "from A1" }] },
+      ]);
+      writeTurns(path.join(root, "subagents", "workerY"), [
+        { role: "assistant", content: [{ type: "text", text: "from Y" }] },
+      ]);
+      return root;
+    }
+
+    test("orchestratorA can read its own descendant workerA1", async () => {
+      const root = setUpRoot();
+      const tool = createReadAgentTraceTool(() => root, {
+        actorId: "orchA",
+        tier: "nested-orchestrator",
+        getNodes: () => nodes,
+      });
+      if (tool.kind !== "string") throw new Error("expected string tool");
+      const text = await tool.handler({ target: "workerA1" }, new AbortController().signal);
+      expect(text).toContain("from A1");
+    });
+
+    test("orchestratorA cannot read workerY, a sibling subtree's worker", async () => {
+      const root = setUpRoot();
+      const tool = createReadAgentTraceTool(() => root, {
+        actorId: "orchA",
+        tier: "nested-orchestrator",
+        getNodes: () => nodes,
+      });
+      if (tool.kind !== "string") throw new Error("expected string tool");
+      const text = await tool.handler({ target: "workerY" }, new AbortController().signal);
+      expect(text).toContain("Error:");
+      expect(text).not.toContain("from Y");
+    });
+
+    test("an actor with no resolvable session id is denied entirely", async () => {
+      const root = setUpRoot();
+      const tool = createReadAgentTraceTool(() => root, {
+        actorId: undefined,
+        tier: "nested-orchestrator",
+        getNodes: () => nodes,
+      });
+      if (tool.kind !== "string") throw new Error("expected string tool");
+      const text = await tool.handler({ target: "workerA1" }, new AbortController().signal);
+      expect(text).toContain("Error:");
+      expect(text).not.toContain("from A1");
+    });
+
+    test("Tier 1 (no authority context) can read any worker", async () => {
+      const root = setUpRoot();
+      const tool = createReadAgentTraceTool(() => root);
+      if (tool.kind !== "string") throw new Error("expected string tool");
+      const text = await tool.handler({ target: "workerY" }, new AbortController().signal);
+      expect(text).toContain("from Y");
+    });
+  });
 });

src/subagent/trace-tool.ts

@@ -19,6 +19,27 @@ import {
   type TraceEntryKind,
   type TraceReadResult,
 } from "./trace-reader.js";
+import {
+  assertCanTargetAgent,
+  FleetAuthorityError,
+  type FleetNode,
+  type SubagentTier,
+} from "./authority.js";
+
+/**
+ * Descendant-scoping context for a Tier 2 nested orchestrator's copy of this
+ * tool. `actorId` is this worker's own SubAgentSessionStore id (the same id
+ * used as its on-disk directory name — see run.ts) and `getNodes` returns
+ * the live fleet so `assertCanTargetAgent` can walk the existing
+ * parentSessionId chain rather than trusting a per-caller check that could
+ * be forgotten at a future mount site. Omit entirely for Tier 1 (the
+ * primary orchestrator), which may target anyone.
+ */
+export interface ReadAgentTraceAuthority {
+  actorId: string | undefined;
+  tier: SubagentTier;
+  getNodes: () => readonly FleetNode[];
+}
 
 const TRACE_ENTRY_KINDS: readonly TraceEntryKind[] = [
   "text",
@@ -103,14 +124,38 @@ function formatTraceResult(result: TraceReadResult): string {
   return lines.join("\n");
 }
 
-export function createReadAgentTraceTool(getRootWorkdirBase: () => string): AgentTool {
+export function createReadAgentTraceTool(
+  getRootWorkdirBase: () => string,
+  authority?: ReadAgentTraceAuthority,
+): AgentTool {
   return stringTool({
     definition: readAgentTraceDefinition,
     handler: async (rawArgs: Record<string, unknown>): Promise<string> => {
       const parsed = ReadAgentTraceArgs(rawArgs);
       if (parsed instanceof type.errors) {
         return `Error: read_agent_trace requires target (string); ${parsed.summary}`;
       }
+      if (authority !== undefined) {
+        // Fails closed: an actor whose own store id could not be resolved
+        // (no session record for this dispatch) must never be trusted with
+        // fleet-wide read access, mirroring CL-6941's unresolved-tier rule.
+        if (authority.actorId === undefined) {
+          return (
+            "Error: read_agent_trace is unavailable for this worker (no resolvable session " +
+            "id to scope descendant access)."
+          );
+        }
+        try {
+          assertCanTargetAgent(
+            { id: authority.actorId, tier: authority.tier },
+            parsed.target,
+            authority.getNodes(),
+          );
+        } catch (cause) {
+          if (cause instanceof FleetAuthorityError) return `Error: ${cause.message}`;
+          throw cause;
+        }
+      }
       try {
         const result = await readAgentTrace(getRootWorkdirBase(), parsed.target, {
           ...(parsed.kinds !== undefined ? { kinds: parsed.kinds } : {}),

src/subagent/types.ts

@@ -76,6 +76,15 @@ export type NestedDispatchDeps = SubAgentSandboxDeps & {
 export type RunSubAgentParams = {
   cwd: string;
   workdirBase: string;
+  /**
+   * Stable id for this worker's on-disk trace directory (subagents/<id>).
+   * Callers that track a session store (task-tool.ts) pass the same id as
+   * the SubAgentSessionStore record so read_agent_trace's descendant check
+   * can reuse the store's existing parentSessionId chain instead of a
+   * second identity scheme. Falls back to a fresh generated id when unset
+   * or unsafe for a path segment.
+   */
+  id?: string;
   provider: SubAgentProvider;
   settings?: Settings;
   catalog?: readonly ProviderCatalogEntry[];