From 00a30c3a76072b2d278118e404e09d43cdc3865b Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 03:22:33 +0900 Subject: [PATCH] fix(adapters): classify failed exec wrappers by line scan, not backtracking FAILED_EXEC_OUTPUT_REGEX chained `[^\n]*`, `\n*` and `\s*` groups that can all match the same whitespace. An input that starts with the literal prefix but never completes the match makes the engine try every split between them, so cost grows quadratically with the padding length. Measured on Bun 1.4, a single `Script failed` line padded with 30k spaces took ~820ms and 60k took ~3.1s. The text comes from a tool result, so its shape is not fully under our control. Replace the pattern with a line scan that classifies each line exactly once. The same 60k input costs ~0.02ms. The rewrite also closes a latent bug it made visible: the old `\n*` accepted an LF blank line but not a CRLF one, so a Windows wrapper such as `Script failed\r\n\r\nOutput:` fell through to the empty-SUCCESS guidance and erased the only signal that the cell had failed. CRLF blank lines are now separators, as they always should have been. Every other accepted and rejected shape is unchanged; a differential run over 200k generated wrappers found no behavior difference outside the CRLF case. --- src/adapters/cursor/tool-result-normalize.ts | 6 +-- src/adapters/exec-tool-result-normalize.ts | 43 ++++++++++++++-- tests/cursor-exec-empty-result.test.ts | 53 ++++++++++++++++++++ 3 files changed, 96 insertions(+), 6 deletions(-) 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);