diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index f661803383..324e20cf43 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -628,7 +628,7 @@ describe("shared ACPX engine runtime behavior", () => { }); }); - it("pins the existing summary and tool-event behavior when no engine knobs are set", async () => { + it("defaults run summaries to the final output segment without thought text", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); const logs: Array<{ stream: string; text: string }> = []; @@ -705,13 +705,9 @@ describe("shared ACPX engine runtime behavior", () => { } as never); expect(result.exitCode).toBe(0); - // The summary is the full concatenation of every text delta, thought - // stream included — the engine's long-standing behavior for claude, - // codex, gemini, and custom agents. If this assertion breaks, a change - // is altering summaries for existing adapters. - expect(result.summary).toBe( - "Let me get oriented and inspect the PRs…hidden chain of thought## Update\n\n- Checked PR status\n- Continue burn-in", - ); + expect(result.summary).toBe("## Update\n\n- Checked PR status\n- Continue burn-in"); + expect(result.summary).not.toContain("Let me get oriented"); + expect(result.summary).not.toContain("hidden chain of thought"); const toolCallEvents = logs .map((entry) => { try { @@ -732,7 +728,7 @@ describe("shared ACPX engine runtime behavior", () => { ]); }); - it("summarizes only the final output segment when the adapter sets summaryStrategy", async () => { + it("does not allow configuration to include thought text in run summaries", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); const execute = createAcpxEngineExecutor({ @@ -803,7 +799,7 @@ describe("shared ACPX engine runtime behavior", () => { agent: "custom", agentCommand: "node ./fake-acp.js", stateDir, - summaryStrategy: "lastOutputSegment", + summaryStrategy: "full", }, context: {}, onLog: async () => {}, @@ -811,12 +807,74 @@ describe("shared ACPX engine runtime behavior", () => { } as never); expect(result.exitCode).toBe(0); - // Must not include intermediate narration or thought stream. expect(result.summary).toBe("## Update\n\n- Checked PR status\n- Continue burn-in"); expect(result.summary).not.toContain("Let me get oriented"); expect(result.summary).not.toContain("hidden chain of thought"); }); + it("treats a statusless initial tool call as an output-segment boundary", async () => { + const root = await makeTempRoot(); + const stateDir = path.join(root, "state"); + const execute = createAcpxEngineExecutor({ + createRuntime: () => ({ + ensureSession: async () => ({ + backendSessionId: "backend-session", + agentSessionId: "agent-session", + runtimeSessionName: "runtime-session", + }), + startTurn: () => ({ + events: (async function* () { + yield { + type: "text_delta", + text: "Intermediate setup that must not be published", + stream: "output", + tag: "agent_message_chunk", + }; + yield { + type: "tool_call", + text: "Bash", + title: "Bash", + toolCallId: "tool-without-status", + tag: "tool_call", + }; + yield { + type: "tool_call", + text: "Bash (completed)", + title: "Bash", + status: "completed", + toolCallId: "tool-without-status", + tag: "tool_call_update", + }; + yield { + type: "text_delta", + text: "## Final update\n\n- Remediation verified", + stream: "output", + tag: "agent_message_chunk", + }; + yield { type: "done", stopReason: "end_turn" }; + })(), + result: Promise.resolve({ status: "completed", stopReason: "end_turn" }), + cancel: async () => {}, + }), + close: async () => {}, + }) as never, + }); + + const result = await execute({ + runId: "run-summary-statusless-tool-call", + agent: { id: "agent-1", companyId: "company-1" }, + runtime: {}, + config: { agent: "custom", agentCommand: "node ./fake-acp.js", stateDir }, + context: {}, + onLog: async () => {}, + onMeta: async () => {}, + } as never); + + expect(result.exitCode).toBe(0); + expect(result.summary).toBe("## Final update\n\n- Remediation verified"); + expect(result.summary).not.toContain("Intermediate setup"); + }); + it("buildAcpxRunSummary prefers the last non-empty segment", () => { expect( buildAcpxRunSummary({ diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index fc3aa6ffd5..06ab95bb42 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -373,9 +373,6 @@ export interface AcpxEngineExecutorOptions { interface AcpxPreparedRuntime { acpxAgent: string; - // See the config parsing site: adapter-declared engine behavior knobs with - // behavior-preserving defaults. - summaryStrategy: "full" | "lastOutputSegment"; coalescePlaceholderToolUpdates: boolean; mode: "persistent" | "oneshot"; cwd: string; @@ -1613,13 +1610,8 @@ async function buildRuntime(input: { ); const acpxAgent = normalizeAgent(config); - // Engine behavior knobs set by the invoking adapter's acpx config builder - // (never by the engine itself): a verbose streaming backend opts into - // last-segment run summaries and placeholder tool-update coalescing here. - // The defaults preserve the engine's long-standing behavior, and the engine - // carries no knowledge of which adapters opt in. - const summaryStrategy: "full" | "lastOutputSegment" = - config.summaryStrategy === "lastOutputSegment" ? "lastOutputSegment" : "full"; + // Run summaries always fail closed to the final output segment so internal + // thought text and intermediate narration cannot become issue comments. const coalescePlaceholderToolUpdates = config.coalescePlaceholderToolUpdates === true; const mode = normalizeMode(config); const permissionMode = normalizePermissionMode(config); @@ -2175,7 +2167,6 @@ async function buildRuntime(input: { return { acpxAgent, - summaryStrategy, coalescePlaceholderToolUpdates, mode, // Remote runner-backed → the in-sandbox workspace dir; local / runner-less @@ -2464,8 +2455,7 @@ async function emitAcpxLog(ctx: AdapterExecutionContext, payload: Record => { - // Summary accumulation, per the adapter-declared strategy. "full" (the - // default) collects every text delta exactly as before. - // "lastOutputSegment" collects output text only (never thought stream), + // Summary accumulation collects output text only (never thought stream), // segmented on tool starts so multi-step narration is not glued into one - // auto-comment dump. - const textParts: string[] = []; + // automatic comment dump. const outputSegments: string[] = []; let currentOutputChunk: string[] = []; const flushOutputSegment = () => { @@ -3992,13 +3979,13 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { const turn = activeTurn as AcpRuntimeTurn; const toolTitles = new Map(); for await (const event of turn.events) { - if (event.type === "text_delta") { - if (prepared.summaryStrategy === "full") { - textParts.push(event.text); - } else if (event.stream !== "thought") { - currentOutputChunk.push(event.text); - } - } else if (event.type === "tool_call" && event.status === "pending") { + if (event.type === "text_delta" && event.stream !== "thought") { + currentOutputChunk.push(event.text); + } else if (event.type === "tool_call" && event.tag !== "tool_call_update") { + // ACP makes tool-call status optional. The normalized event tag is + // the reliable boundary between an initial call and its updates, + // so a statusless initial call must still end the preceding output + // segment while updates must not create extra boundaries. flushOutputSegment(); } if (event.type === "status" && event.tag === "usage_update") { @@ -4088,13 +4075,10 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { ? { cumulativeCostUsd: turnUsage.cumulativeCostUsd } : {}), }, - summary: - prepared.summaryStrategy === "lastOutputSegment" - ? buildAcpxRunSummary({ - outputSegments, - fallback: terminalStopReason || terminal.status, - }) - : textParts.join("").trim() || terminalStopReason || terminal.status, + summary: buildAcpxRunSummary({ + outputSegments, + fallback: terminalStopReason || terminal.status, + }), clearSession, }; // The turn phase finished. A completed, non-timed-out turn is `ok`; every diff --git a/server/src/__tests__/heartbeat-run-summary.test.ts b/server/src/__tests__/heartbeat-run-summary.test.ts index 74051ab469..593c6006e0 100644 --- a/server/src/__tests__/heartbeat-run-summary.test.ts +++ b/server/src/__tests__/heartbeat-run-summary.test.ts @@ -133,6 +133,18 @@ describe("mergeHeartbeatRunResultJson", () => { expect(buildHeartbeatRunIssueComment(merged)).toBe("## Summary\n\n1. first thing\n2. second thing"); }); + it("posts only the final adapter summary when raw output contains intermediate narration", () => { + const merged = mergeHeartbeatRunResultJson( + { stdout: "Intermediate setup that must not be published" }, + "## Final update\n\n- Remediation verified", + ); + + expect(buildHeartbeatRunIssueComment(merged)).toBe( + "## Final update\n\n- Remediation verified", + ); + expect(buildHeartbeatRunIssueComment(merged)).not.toContain("Intermediate setup"); + }); + it("creates a result payload when only a summary exists", () => { expect(mergeHeartbeatRunResultJson(null, "done")).toEqual({ summary: "done" }); });