fix(compaction): keep accepted sessions_spawn results out of tool-failure summaries

nxmxbbd · nxmxbbd · commit 079397b2c385 · 2026-06-26T02:06:02.000+08:00
Accepted sessions_spawn launches can be persisted as a toolResult with isError:true while the payload carries status=accepted. The compaction safeguard collected every isError toolResult, so normal accepted subagent launches surfaced as repeated '## Tool Failures' lines in user-visible summaries. collectToolFailures now skips results whose toolName is sessions_spawn and whose payload normalizes as an accepted child spawn, mirroring the observer's existing detection; this covers legacy transcripts too. Genuinely failed spawns (status=error/forbidden) and non-spawn tools are still reported.
diff --git a/src/agents/agent-hooks/compaction-safeguard.test.ts b/src/agents/agent-hooks/compaction-safeguard.test.ts
@@ -14,6 +14,7 @@ import {
 import * as compactionModule from "../compaction.js";
 import { buildEmbeddedExtensionFactories } from "../embedded-agent-runner/extensions.js";
 import { castAgentMessage } from "../test-helpers/agent-message-fixtures.js";
+import { jsonResult } from "../tools/common.js";
 import {
   consumeCompactionSafeguardCancelReason,
   getCompactionSafeguardRuntime,
@@ -265,6 +266,104 @@ describe("compaction-safeguard tool failures", () => {
     expect(section).toContain("exec (status=failed exitCode=1): ENOENT: missing file");
   });
 
+  it("excludes accepted sessions_spawn results even when persisted with isError", () => {
+    const messages: AgentMessage[] = [
+      {
+        role: "toolResult",
+        toolCallId: "call-spawn-accepted",
+        toolName: "sessions_spawn",
+        isError: true,
+        details: {
+          status: "accepted",
+          childSessionKey: "agent:watcher:subagent:abc",
+          runId: "run-123",
+          mode: "run",
+        },
+        content: [{ type: "text", text: "accepted" }],
+        timestamp: Date.now(),
+      },
+    ];
+
+    expect(collectToolFailures(messages)).toHaveLength(0);
+  });
+
+  it("still reports sessions_spawn results that genuinely failed", () => {
+    const messages: AgentMessage[] = [
+      {
+        role: "toolResult",
+        toolCallId: "call-spawn-error",
+        toolName: "sessions_spawn",
+        isError: true,
+        details: { status: "error" },
+        content: [{ type: "text", text: "spawn rejected" }],
+        timestamp: Date.now(),
+      },
+      {
+        role: "toolResult",
+        toolCallId: "call-spawn-forbidden",
+        toolName: "sessions_spawn",
+        isError: true,
+        details: { status: "forbidden" },
+        content: [{ type: "text", text: "not allowed" }],
+        timestamp: Date.now(),
+      },
+    ];
+
+    const failures = collectToolFailures(messages);
+    expect(failures.map((failure) => failure.toolCallId)).toEqual([
+      "call-spawn-error",
+      "call-spawn-forbidden",
+    ]);
+  });
+
+  it("only excludes the accepted spawn from a mixed batch and reports look-alike non-spawn tools", () => {
+    // Build the accepted-spawn details via the production helper so the skip is
+    // proven against the real sessions_spawn result shape, not a hand-authored stub.
+    const acceptedDetails = jsonResult({
+      status: "accepted",
+      childSessionKey: "agent:watcher:subagent:abc",
+      runId: "run-123",
+      mode: "run",
+    }).details;
+    const messages: AgentMessage[] = [
+      {
+        role: "toolResult",
+        toolCallId: "call-spawn-accepted",
+        toolName: "sessions_spawn",
+        isError: true,
+        details: acceptedDetails,
+        content: [{ type: "text", text: "accepted" }],
+        timestamp: Date.now(),
+      },
+      {
+        role: "toolResult",
+        toolCallId: "call-exec-failed",
+        toolName: "exec",
+        isError: true,
+        details: { status: "failed", exitCode: 1 },
+        content: [{ type: "text", text: "boom" }],
+        timestamp: Date.now(),
+      },
+      {
+        // Same accepted-shaped details on a non-spawn tool must still be reported:
+        // the skip is gated on toolName so look-alike payloads are not suppressed.
+        role: "toolResult",
+        toolCallId: "call-other-lookalike",
+        toolName: "some_other_tool",
+        isError: true,
+        details: acceptedDetails,
+        content: [{ type: "text", text: "real failure" }],
+        timestamp: Date.now(),
+      },
+    ];
+
+    const failures = collectToolFailures(messages);
+    expect(failures.map((failure) => failure.toolCallId)).toEqual([
+      "call-exec-failed",
+      "call-other-lookalike",
+    ]);
+  });
+
   it("dedupes by toolCallId and handles empty output", () => {
     const messages: AgentMessage[] = [
       {
diff --git a/src/agents/agent-hooks/compaction-safeguard.ts b/src/agents/agent-hooks/compaction-safeguard.ts
@@ -10,6 +10,7 @@ import {
   getCompactionProvider,
   type CompactionProvider,
 } from "../../plugins/compaction-provider.js";
+import { normalizeAcceptedSessionSpawnResult } from "../accepted-session-spawn.js";
 import {
   buildHistoryPrunePlanWithWorker,
   computeAdaptiveChunkRatioWithWorker,
@@ -447,6 +448,17 @@ function collectToolFailures(messages: AgentMessage[]): ToolFailure[] {
     if (toolResult.isError !== true) {
       continue;
     }
+    // Accepted sessions_spawn launches are successes, not failures, even when a legacy
+    // transcript persisted them with isError:true. Mirror the observer's detection
+    // (toolName + accepted child-run identity, see embedded-agent-subscribe.handlers.tools)
+    // so only real failures stay in the summary and non-spawn tools are never matched by shape.
+    if (
+      typeof toolResult.toolName === "string" &&
+      toolResult.toolName.trim() === "sessions_spawn" &&
+      normalizeAcceptedSessionSpawnResult(toolResult)
+    ) {
+      continue;
+    }
     const toolCallId = typeof toolResult.toolCallId === "string" ? toolResult.toolCallId : "";
     if (!toolCallId || seen.has(toolCallId)) {
       continue;
