From 494362cd233aa27d3258fcc5e50ef920f5ef139c Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 1 Aug 2026 12:48:30 -0700 Subject: [PATCH] FN-8697: preserve redacted failed-tool diagnostics Persist safe error detail so operators can diagnose repeated executor tool failures. - Redact and retain failed-tool diagnostics even when tool output persistence is disabled. - Cover redaction, disclosure independence, inert rendering, and retry-backoff normalization. Files changed: .../__tests__/AgentLogViewer.rendering.test.tsx | 29 +++++++++++++++++ .../src/__tests__/agent-logger-diagnostics.test.ts | 38 ++++++++++++++++++++++ packages/engine/src/__tests__/agent-logger.test.ts | 4 +-- .../__tests__/executor-tool-failure-retry.test.ts | 22 ++++++++++++- packages/engine/src/agent-logger.ts | 11 +++++-- 5 files changed, 99 insertions(+), 5 deletions(-) Fusion-Task-Id: FN-8697 Fusion-Task-Lineage: 5ed900b6-a42b-4ad7-9270-47053295e6e1 Co-authored-by: Fusion (runfusion.ai) --- .../AgentLogViewer.rendering.test.tsx | 29 ++++++++++++++ .../agent-logger-diagnostics.test.ts | 38 +++++++++++++++++++ .../engine/src/__tests__/agent-logger.test.ts | 4 +- .../executor-tool-failure-retry.test.ts | 22 ++++++++++- packages/engine/src/agent-logger.ts | 11 +++++- 5 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 packages/engine/src/__tests__/agent-logger-diagnostics.test.ts diff --git a/packages/dashboard/app/components/__tests__/AgentLogViewer.rendering.test.tsx b/packages/dashboard/app/components/__tests__/AgentLogViewer.rendering.test.tsx index 2999e7ca53..771861f2ee 100644 --- a/packages/dashboard/app/components/__tests__/AgentLogViewer.rendering.test.tsx +++ b/packages/dashboard/app/components/__tests__/AgentLogViewer.rendering.test.tsx @@ -334,6 +334,35 @@ describe("AgentLogViewer", () => { expect(content.classList.contains("agent-log-tool-detail-content--collapsed")).toBe(true); }); + it("keeps identical failed-tool disclosures independent and renders error markup as inert text", () => { + Object.defineProperty(window, "innerWidth", { configurable: true, writable: true, value: 390 }); + const detail = "Error: edit failed\n\n at edit (tools.ts:12:4)"; + const entries = [ + makeEntry({ text: "edit", type: "tool_error", detail, timestamp: "2026-01-01T00:00:00Z" }), + makeEntry({ text: "edit", type: "tool_error", detail, timestamp: "2026-01-01T00:00:00Z" }), + makeEntry({ text: "edit", type: "tool_error", detail, timestamp: "2026-01-01T00:00:00Z" }), + ]; + const { container } = render(); + + const toggles = screen.getAllByTestId("tool-detail-toggle"); + expect(toggles).toHaveLength(3); + expect(toggles[0]).toHaveAccessibleName("Show output (3 lines)"); + expect(toggles[0]).toBeEnabled(); + fireEvent.click(toggles[0]); + expect(toggles[0]).toHaveAttribute("aria-expanded", "true"); + expect(toggles[1]).toHaveAttribute("aria-expanded", "false"); + fireEvent.click(toggles[1]); + expect(toggles[1]).toHaveAttribute("aria-expanded", "true"); + expect(toggles[2]).toHaveAttribute("aria-expanded", "false"); + const expandedDetails = screen.getAllByTestId("tool-detail-content").filter( + (content) => !content.classList.contains("agent-log-tool-detail-content--collapsed"), + ); + expect(expandedDetails).toHaveLength(2); + expect(expandedDetails[0]).toHaveTextContent(""); + expect(expandedDetails[1]).toHaveTextContent(""); + expect(container.querySelector("script")).toBeNull(); + }); + it("shows complete long tool output after expanding the output disclosure", () => { const longDetail = `first line\n${"output ".repeat(45)}AGENT_LOG_RESULT_SUFFIX`; render(); diff --git a/packages/engine/src/__tests__/agent-logger-diagnostics.test.ts b/packages/engine/src/__tests__/agent-logger-diagnostics.test.ts new file mode 100644 index 0000000000..612c24cac5 --- /dev/null +++ b/packages/engine/src/__tests__/agent-logger-diagnostics.test.ts @@ -0,0 +1,38 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { AgentLogger } from "../agent-logger.js"; + +vi.mock("../logger.js", () => ({ + createLogger: () => ({ log: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})); + +describe("AgentLogger failed-tool diagnostics (FN-8697)", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it("persists redacted error detail even when normal tool output persistence is disabled", async () => { + const appendLog = vi.fn().mockResolvedValue(undefined); + const logger = new AgentLogger({ + taskId: "FN-8697", + appendLog, + persistAgentToolOutput: false, + }); + const secret = "sk-live-ABCDEFG1234567890abcdef"; + // FNXC:AgentLogDiagnostics 2026-08-01-19:24: Tool diagnostics must honor every shared-redactor match, including low-entropy opaque values, before either log sink persists them. + const opaqueToken = "a".repeat(40); + const detail = `Error: edit failed\nAuthorization: Bearer ${secret}\ntoken=${opaqueToken}\n at edit (tools.ts:12:4)`; + + logger.onToolEnd("edit", true, detail); + await logger.flush(); + + expect(appendLog).toHaveBeenCalledWith(expect.objectContaining({ + type: "tool_error", + text: "edit", + detail: expect.stringContaining("Error: edit failed"), + })); + const persisted = appendLog.mock.calls[0][0].detail as string; + expect(persisted).toContain("at edit (tools.ts:12:4)"); + expect(persisted).toContain("[REDACTED]"); + expect(persisted).not.toContain(secret); + expect(persisted).not.toContain(opaqueToken); + }); +}); diff --git a/packages/engine/src/__tests__/agent-logger.test.ts b/packages/engine/src/__tests__/agent-logger.test.ts index 56aff229f1..f0b0ec1e4d 100644 --- a/packages/engine/src/__tests__/agent-logger.test.ts +++ b/packages/engine/src/__tests__/agent-logger.test.ts @@ -551,7 +551,7 @@ describe("AgentLogger", () => { persistAgentToolOutput: true, }); - const longError = "error:" + "y".repeat(1200); + const longError = "error:" + "y-".repeat(600); logger.onToolEnd("Read", true, longError); await vi.advanceTimersByTimeAsync(0); @@ -568,7 +568,7 @@ describe("AgentLogger", () => { persistAgentToolOutput: true, }); - const longResult = "x".repeat(600); + const longResult = "x-".repeat(300); logger.onToolEnd("Bash", false, longResult); await vi.advanceTimersByTimeAsync(0); diff --git a/packages/engine/src/__tests__/executor-tool-failure-retry.test.ts b/packages/engine/src/__tests__/executor-tool-failure-retry.test.ts index 528509fa98..b9565c77e6 100644 --- a/packages/engine/src/__tests__/executor-tool-failure-retry.test.ts +++ b/packages/engine/src/__tests__/executor-tool-failure-retry.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { TaskDetail } from "@fusion/core"; +import { resolveConsecutiveToolFailureRetryBackoffMs, resolveMaxConsecutiveToolFailureRetries, type TaskDetail } from "@fusion/core"; import "./executor-test-helpers.js"; import { TaskExecutor } from "../executor.js"; import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; @@ -105,6 +105,26 @@ describe("executor consecutive tool-failure retry (FN-7996)", () => { })); }); + it("normalizes the configured backoff and waits before retrying", async () => { + const { executor, task } = makeHarness({ + retries: 2.9, + entries: [{ type: "tool_error" }, { type: "tool_error" }, { type: "tool_error" }], + settings: { executorToolFailureRetryBackoffMs: 2500.9 }, + }); + const execute = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + expect(resolveMaxConsecutiveToolFailureRetries({ executorToolFailureRetryCount: 2.9 })).toBe(2); + expect(resolveMaxConsecutiveToolFailureRetries({ executorToolFailureRetryCount: -1 })).toBe(2); + expect(resolveConsecutiveToolFailureRetryBackoffMs({ executorToolFailureRetryBackoffMs: 2500.9 })).toBe(2500); + expect(resolveConsecutiveToolFailureRetryBackoffMs({ executorToolFailureRetryBackoffMs: -1 })).toBe(2000); + + await (executor as any).handleGraphFailure(task, graphFailure()); + await vi.advanceTimersByTimeAsync(2499); + expect(execute).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + expect(execute).toHaveBeenCalledWith(task); + }); + it("parks unchanged after a spent retry budget and emits one exhaustion audit", async () => { const { executor, store, task } = makeHarness({ retries: 2, diff --git a/packages/engine/src/agent-logger.ts b/packages/engine/src/agent-logger.ts index 9cc70a0935..366f0c4356 100644 --- a/packages/engine/src/agent-logger.ts +++ b/packages/engine/src/agent-logger.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import type { TaskStore, AgentLogEntry, AgentRole } from "@fusion/core"; -import { categorizeToolName } from "@fusion/core"; +import { categorizeToolName, redactSecrets } from "@fusion/core"; import { createLogger } from "./logger.js"; /** @@ -483,12 +483,19 @@ export class AgentLogger { const isToolEntry = type === "tool" || type === "tool_result" || type === "tool_error"; // FNXC:AgentLogging 2026-07-15-16:00: Failed tool detail is diagnostic signal, unlike verbose arguments/success output, and must survive default-off tool-output persistence for FN-7995 Activity diagnosis. const includeDetail = !isToolEntry || type === "tool_error" || this.persistAgentToolOutput; + /* + FNXC:AgentLogDiagnostics 2026-08-01-17:51: + FN-8697 requires useful failed-tool detail to cross the established secret-redaction boundary + before an AgentLogger batch reaches JSONL, the API, or the Activity disclosure. Redact at this + common entry point so store and callback sinks cannot persist or display credentials. + */ + const persistedDetail = detail !== undefined && includeDetail ? redactSecrets(detail) : undefined; const entry: AgentLogEntry = { timestamp: new Date().toISOString(), taskId: this.taskId, text, type, - ...(detail !== undefined && includeDetail && { detail }), + ...(persistedDetail !== undefined && { detail: persistedDetail }), ...(this.agent !== undefined && { agent: this.agent }), ...(timing?.durationMs !== undefined && { durationMs: timing.durationMs }), ...(timing?.timeToFirstTokenMs !== undefined && { timeToFirstTokenMs: timing.timeToFirstTokenMs }),