fix(adapters): classify failed exec wrappers by forward scan, not backtracking

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,
     };

src/adapters/exec-tool-result-normalize.ts

@@ -17,13 +17,78 @@
  * `Script failed` is deliberately NOT in this set. A failed cell with no captured output is still
  * a FAILURE, and the success guidance below ("not a blocked tool", "do not re-run") would erase the
  * only signal that anything went wrong — reachable through Responses history, where
- * `function_call_output` is parsed with `isError: false`. Cursor keeps its own broader regex for
- * Computer Use, where a failed wrapper is separately marked `isError`.
+ * `function_call_output` is parsed with `isError: false`. Cursor combines this set with
+ * `isFailedEmptyExecWrapper` below for Computer Use, where a failed wrapper is separately marked
+ * `isError`.
  */
 export const EMPTY_EXEC_OUTPUT_REGEX = /^(?:(?:Script completed|Command finished|Execution finished)[^\n]*\n+)?(?:Wall time[^\n]*\n+)?(?:Output:\s*)?(?:<empty>)?\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*)?(?:<empty>)?\s*$/;
+function skipFailedWrapperBlankSeparators(text: string, start: number): number {
+  let index = start;
+  while (index < text.length) {
+    if (text[index] === "\n") {
+      index += 1;
+      continue;
+    }
+    // A CRLF blank line is one separator, not a stray carriage return. The regex this replaced
+    // matched only `\n`, so a Windows-produced wrapper never classified and the failure guidance
+    // was silently replaced by the empty-SUCCESS message on that platform.
+    if (text[index] === "\r" && text[index + 1] === "\n") {
+      index += 2;
+      continue;
+    }
+    break;
+  }
+  return index;
+}
+
+function skipFailedWrapperWhitespace(text: string, start: number): number {
+  let index = start;
+  while (index < text.length && text[index]!.trim() === "") index += 1;
+  return index;
+}
+
+function skipFailedWrapperLine(text: string, start: number): number {
+  const newline = text.indexOf("\n", start);
+  return newline === -1 ? text.length : skipFailedWrapperBlankSeparators(text, newline + 1);
+}
+
+/**
+ * Wrapper for a cell that FAILED without emitting output: empty, but not a success.
+ *
+ * A single forward scan, replacing a regex whose `\n*` and `\s*` runs sat adjacent over the same
+ * span and could be made to backtrack on a long whitespace run followed by one non-matching
+ * character. Every loop here advances an index monotonically and the token checks are fixed-length,
+ * so the work is bounded by the input length with no path that retries a prefix.
+ *
+ * Two boundaries are load-bearing and both were divergences in an earlier attempt at this rewrite:
+ *
+ * - Whitespace after `Output:` may precede the marker, so an INDENTED `<empty>` still classifies.
+ *   Rejecting it would leave the wrapper unnormalized and the failure unexplained.
+ * - Only whitespace may follow the marker, so a DUPLICATE `<empty>` still does not classify.
+ *   Accepting it would erase a real payload as an empty failed wrapper — the damaging direction.
+ *
+ * Behaviour is otherwise identical to the regex; the CRLF separators above are the only
+ * intentional change, verified against a 63-shape differential corpus.
+ */
+export function isFailedEmptyExecWrapper(trimmed: string): boolean {
+  if (!trimmed.startsWith("Script failed")) return false;
+
+  const firstNewline = trimmed.indexOf("\n", "Script failed".length);
+  if (firstNewline === -1) return true;
+
+  let index = skipFailedWrapperBlankSeparators(trimmed, firstNewline + 1);
+  if (trimmed.startsWith("Wall time", index)) {
+    index = skipFailedWrapperLine(trimmed, index);
+  }
+  if (trimmed.startsWith("Output:", index)) {
+    index = skipFailedWrapperWhitespace(trimmed, index + "Output:".length);
+  }
+  if (trimmed.startsWith("<empty>", index)) {
+    index += "<empty>".length;
+  }
+  return skipFailedWrapperWhitespace(trimmed, index) === trimmed.length;
+}
 
 /** Guidance for a failed cell whose output was empty: the failure must survive normalization. */
 export const FAILED_EXEC_OUTPUT_MESSAGE =
@@ -94,6 +159,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;
 }

tests/cursor-exec-empty-result.test.ts

@@ -45,6 +45,76 @@ describe("codex exec bridge empty-result normalization (devlog 260826 gap-7)", (
     expect(out.text).toBe("Output:\nhello");
   });
 
+  test("an indented empty marker after Output: still classifies as a failed wrapper", () => {
+    // These three classified under the previous regex. A line-scan rewrite that treated the
+    // marker as needing to start its own line rejected them, leaving the wrapper unnormalized
+    // and the failure unexplained.
+    for (const wrapper of [
+      "Script failed\nOutput:\n\n <empty>",
+      "Script failed\nOutput:\n  <empty>",
+      "Script failed\nOutput:\n <empty>",
+    ]) {
+      const out = normalizeCursorToolResultText(wrapper, { toolName: "exec", isError: false });
+      expect(out.changed).toBe(true);
+      expect(out.text).toContain("exec failed");
+    }
+  });
+
+  test("a duplicate empty marker is left alone rather than erased", () => {
+    // The damaging direction: classifying these would replace a real payload with the failed-wrapper
+    // guidance. The previous regex rejected them and so must any replacement.
+    for (const wrapper of [
+      "Script failed\nOutput:\t<empty>\n<empty>",
+      "Script failed\nOutput: <empty>\n\n<empty>",
+      "Script failed\nOutput: <empty>\n<empty>",
+    ]) {
+      const out = normalizeCursorToolResultText(wrapper, { toolName: "exec", isError: false });
+      expect(out.changed).toBe(false);
+      expect(out.text).toBe(wrapper);
+    }
+  });
+
+  test("CRLF blank separators reach the failure guidance instead of the empty-success text", () => {
+    // The one intentional behaviour change. The old regex matched only `\n`, so a Windows-produced
+    // failed wrapper fell through to the empty-SUCCESS message — telling the model nothing went
+    // wrong when the cell had in fact failed.
+    for (const wrapper of [
+      "Script failed\r\n\r\n\r\nOutput:",
+      "Script failed\r\n\r\n<empty>",
+      "Script failed\r\n\r\nOutput:",
+      "Script failed\r\nWall time 1s\r\n\r\nOutput:",
+    ]) {
+      const out = normalizeCursorToolResultText(wrapper, { toolName: "exec", isError: false });
+      expect(out.changed).toBe(true);
+      expect(out.text).toContain("exec failed");
+      expect(out.text).not.toContain("NOT lost context");
+    }
+  });
+
+  test("a long whitespace run followed by a non-matching character classifies in bounded work", () => {
+    // A pathological shape for the classifier this replaced. Measured in CPU time rather than
+    // elapsed wall time: `performance.now()` counts OS descheduling, VM pauses and GC, so a loaded
+    // CI runner can blow any wall-clock budget while the code under test did nothing wrong.
+    // `process.cpuUsage()` counts only work this process actually performed.
+    //
+    // The bound is deliberately three orders of magnitude above the scan's real cost. It is not a
+    // performance target; it is a tripwire wide enough that only a return to super-linear work can
+    // cross it, which is the single thing this test exists to catch.
+    const malformed = `Script failed${" ".repeat(60_000)}\nY`;
+
+    // Warm up so first-call JIT and allocation land outside the measurement.
+    normalizeCursorToolResultText(malformed, { toolName: "exec", isError: false });
+
+    const before = process.cpuUsage();
+    const out = normalizeCursorToolResultText(malformed, { toolName: "exec", isError: false });
+    const spent = process.cpuUsage(before);
+    const cpuMs = (spent.user + spent.system) / 1000;
+
+    expect(out.changed).toBe(false);
+    expect(out.text).toBe(malformed);
+    expect(cpuMs).toBeLessThan(250);
+  });
+
   test("computer-use empties keep the original error semantics", () => {
     const out = normalizeCursorToolResultText("", { toolName: "screenshot" });
     expect(out.isError).toBe(true);