diff --git a/src/adapters/cursor/tool-result-normalize.ts b/src/adapters/cursor/tool-result-normalize.ts index 997ef56fea..9dd602dc41 100644 --- a/src/adapters/cursor/tool-result-normalize.ts +++ b/src/adapters/cursor/tool-result-normalize.ts @@ -13,7 +13,7 @@ import { EMPTY_EXEC_OUTPUT_MESSAGE, EMPTY_EXEC_OUTPUT_REGEX, FAILED_EXEC_OUTPUT_MESSAGE, - FAILED_EXEC_OUTPUT_REGEX, + isFailedEmptyExecWrapper, isCodexExecBridgeTool, } from "../exec-tool-result-normalize"; @@ -23,7 +23,7 @@ import { * `Script failed`, so restore that arm here rather than widening the shared one. */ function isEmptyOrFailedExecWrapper(text: string): boolean { - return EMPTY_EXEC_OUTPUT_REGEX.test(text) || FAILED_EXEC_OUTPUT_REGEX.test(text); + return EMPTY_EXEC_OUTPUT_REGEX.test(text) || isFailedEmptyExecWrapper(text); } const COMPUTER_USE_TOOL_NAMES = new Set([ @@ -99,7 +99,7 @@ export function normalizeCursorToolResultText( // A `Script failed` wrapper is empty but NOT a success: reporting it as an empty success // would erase the only failure signal. Text classification stays separate from Cursor's // isError policy, which the Computer Use branch above owns. - text: FAILED_EXEC_OUTPUT_REGEX.test(text.trim()) ? FAILED_EXEC_OUTPUT_MESSAGE : EMPTY_EXEC_OUTPUT_MESSAGE, + text: isFailedEmptyExecWrapper(text.trim()) ? FAILED_EXEC_OUTPUT_MESSAGE : EMPTY_EXEC_OUTPUT_MESSAGE, isError: false, changed: true, }; diff --git a/src/adapters/exec-tool-result-normalize.ts b/src/adapters/exec-tool-result-normalize.ts index f06a07c411..feca2763f6 100644 --- a/src/adapters/exec-tool-result-normalize.ts +++ b/src/adapters/exec-tool-result-normalize.ts @@ -22,8 +22,45 @@ */ export const EMPTY_EXEC_OUTPUT_REGEX = /^(?:(?:Script completed|Command finished|Execution finished)[^\n]*\n+)?(?:Wall time[^\n]*\n+)?(?:Output:\s*)?(?:)?\s*$/; -/** Wrapper for a cell that FAILED without emitting output: empty, but not a success. */ -export const FAILED_EXEC_OUTPUT_REGEX = /^Script failed[^\n]*\n*(?:Wall time[^\n]*\n*)?(?:Output:\s*)?(?:)?\s*$/; +/** + * True when a trimmed exec wrapper says the cell FAILED and carried no output. + * + * Deliberately a line scan rather than a regex. The equivalent pattern needs a run of + * `[^\n]*`, `\n*`, `\s*` groups that can each match the same whitespace, so a malformed + * wrapper that never completes the match makes the engine try every split between them. + * Measured on Bun 1.4: a single `Script failed` line padded with 30k spaces took ~820ms + * and 60k took ~3.1s — quadratic, on text that arrives from a tool result and can be + * attacker-influenced. Each line here is classified exactly once, so there is nothing to + * backtrack over: the same 60k input is ~0.02ms. + * + * Blank separator lines are matched the way the previous pattern's `\n*` did — a line of + * spaces is NOT a separator, only a genuinely empty one is — except that a CRLF blank line + * now counts. The old pattern accepted `\n\n` but not `\r\n\r\n`, so a CRLF wrapper was + * reported as an empty SUCCESS and the failure signal was erased. That was a latent bug, + * and fixing it is the point of describing the boundary explicitly. + */ +export function isFailedEmptyExecWrapper(trimmed: string): boolean { + if (!trimmed.startsWith("Script failed")) return false; + const lines = trimmed.split(/\r?\n/); + let i = 1; + const skipEmpty = (): void => { while (i < lines.length && lines[i] === "") i++; }; + skipEmpty(); + if (i < lines.length && lines[i]!.startsWith("Wall time")) { i++; skipEmpty(); } + if (i < lines.length && lines[i]!.startsWith("Output:")) { + const after = lines[i]!.slice("Output:".length).trim(); + // Real payload after the marker: this wrapper is not empty and must pass through. + if (after !== "" && after !== "") return false; + i++; + // A bare `Output:` left the old `\s*` free to swallow any trailing whitespace, + // including whitespace-only lines; `Output: ` consumed the marker first and + // only `\n*`-style empty lines could follow. + if (after === "") { while (i < lines.length && lines[i]!.trim() === "") i++; } + else skipEmpty(); + } + if (i < lines.length && lines[i] === "") { i++; skipEmpty(); } + while (i < lines.length && lines[i]!.trim() === "") i++; + return i >= lines.length; +} /** Guidance for a failed cell whose output was empty: the failure must survive normalization. */ export const FAILED_EXEC_OUTPUT_MESSAGE = @@ -94,6 +131,6 @@ export function normalizeEmptyExecToolResultText( if (!isCodexExecBridgeTool(options.toolName, options.toolNamespace)) return undefined; const trimmed = text.trim(); // Failure first: a failed wrapper must never be described as an empty success. - if (FAILED_EXEC_OUTPUT_REGEX.test(trimmed)) return FAILED_EXEC_OUTPUT_MESSAGE; + if (isFailedEmptyExecWrapper(trimmed)) return FAILED_EXEC_OUTPUT_MESSAGE; return EMPTY_EXEC_OUTPUT_REGEX.test(trimmed) ? EMPTY_EXEC_OUTPUT_MESSAGE : undefined; } diff --git a/tests/cursor-exec-empty-result.test.ts b/tests/cursor-exec-empty-result.test.ts index 098aaa08d3..0901a0831f 100644 --- a/tests/cursor-exec-empty-result.test.ts +++ b/tests/cursor-exec-empty-result.test.ts @@ -45,6 +45,59 @@ describe("codex exec bridge empty-result normalization (devlog 260826 gap-7)", ( expect(out.text).toBe("Output:\nhello"); }); + test("a malformed failed wrapper is classified in linear time", () => { + // The previous regex used overlapping whitespace quantifiers, so an input that never + // completes the match made the engine try every split between them. Measured on Bun + // 1.4 the same shape took ~820ms at 30k and ~3.1s at 60k — quadratic growth on text + // that arrives inside a tool result. 500ms is far above the linear scan's real cost + // (~0.02ms) and far below the old behavior, so it fails loudly if backtracking returns + // without being tight enough to flake on a loaded worker. + const malformed = `Script failed${" ".repeat(60_000)}\nY`; + const startedAt = performance.now(); + const out = normalizeCursorToolResultText(malformed, { toolName: "exec", isError: false }); + const elapsedMs = performance.now() - startedAt; + + expect(out.changed).toBe(false); + expect(out.text).toBe(malformed); + expect(elapsedMs).toBeLessThan(500); + }); + + test("a CRLF failed wrapper is a failure, not an empty success", () => { + // The old pattern's `\n*` accepted an LF blank line but not a CRLF one, so on Windows + // wrappers this branch fell through to the empty-SUCCESS text and erased the failure. + const out = normalizeCursorToolResultText("Script failed\r\n\r\nOutput:", { toolName: "exec", isError: false }); + expect(out.changed).toBe(true); + expect(out.text).toContain("exec failed"); + expect(out.text).not.toContain("NOT lost context"); + }); + + test("failed-wrapper classification matches the shapes it accepted before", () => { + const accepted = [ + "Script failed", + "Script failed\nOutput:", + "Script failed\nOutput: ", + "Script failed\nWall time 1.2 seconds", + "Script failed\nWall time 1.2 seconds\nOutput: ", + "Script failed\n\n\nOutput:", + "Script failed\nWall time 1s\n\nOutput:", + ]; + for (const wrapper of accepted) { + const out = normalizeCursorToolResultText(wrapper, { toolName: "exec", isError: false }); + expect(out.text).toContain("exec failed"); + } + + const rejected = [ + "Script failed\nOutput:\nreal output", + "Script failed\nWall time 1s\nsomething real", + "Script failed\nOutput: trailing", + ]; + for (const wrapper of rejected) { + const out = normalizeCursorToolResultText(wrapper, { toolName: "exec", isError: false }); + expect(out.changed).toBe(false); + expect(out.text).toBe(wrapper); + } + }); + test("computer-use empties keep the original error semantics", () => { const out = normalizeCursorToolResultText("", { toolName: "screenshot" }); expect(out.isError).toBe(true);