From 56611c6d6346cb713a2b668f68bad9f2cd4b618d Mon Sep 17 00:00:00 2001 From: "devintern-internal[bot]" <4622575+devintern-internal[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 12:49:43 +0700 Subject: [PATCH] fix: avoid false max-turns detection in Codex output Match Claude's turn-limit diagnostic as a complete line and skip harnesses that cannot pass --max-turns. Substring scans of Codex tool transcripts were classifying this repo's own source and lint dumps as max-turns exhaustion. Signed-off-by: devintern-internal[bot] <4622575+devintern-internal[bot]@users.noreply.github.com> --- .../agent-harness/src/detect-max-turns.ts | 48 ++++++++++++++--- .../agent-harness/src/detect-usage-limit.ts | 50 ++--------------- .../src/harnesses/claude-code.ts | 1 + packages/agent-harness/src/index.ts | 2 +- packages/agent-harness/src/output-lines.ts | 53 +++++++++++++++++++ packages/agent-harness/src/runners/bun.ts | 2 +- packages/agent-harness/src/runners/node.ts | 2 +- packages/agent-harness/src/types.ts | 7 +++ .../tests/detect-max-turns.test.ts | 40 +++++++++++++- .../agent-harness/tests/harnesses.test.ts | 2 + packages/code/src/index.ts | 19 ++++++- packages/code/src/lib/address-review.ts | 6 ++- packages/code/src/webhook-server.ts | 11 +++- 13 files changed, 180 insertions(+), 63 deletions(-) create mode 100644 packages/agent-harness/src/output-lines.ts diff --git a/packages/agent-harness/src/detect-max-turns.ts b/packages/agent-harness/src/detect-max-turns.ts index bc341cd..f9b40cc 100644 --- a/packages/agent-harness/src/detect-max-turns.ts +++ b/packages/agent-harness/src/detect-max-turns.ts @@ -1,23 +1,55 @@ /** * Detect max-turns exhaustion from agent CLI stdout/stderr. * - * Claude Code (`-p` / stdin) emits `Error: Reached max turns (N)` on stdout with exit code 1. - * Match known phrases across both streams since agents differ. + * Claude Code (`-p` / stdin) emits `Error: Reached max turns (N)` on stdout + * with exit code 1. Match that as a complete diagnostic line ā€” a substring + * scan of the whole transcript also sees this repository's own source, lint + * dumps, and diffs (DEV-70 Codex cron false positive). + * + * Do not treat stderr as trusted: Codex writes its tool transcript there. + * Callers should also skip detection when the harness cannot impose a CLI + * turn limit (`supportsMaxTurns`). */ +import { isSourceOrDiffLine, outputLines } from "./output-lines.js"; + const MAX_TURNS_PATTERNS = [ - /Reached max turns/i, - /max turns reached/i, - /maximum turns reached/i, + /^(?:error:\s*)?reached max turns(?:\s*\(\d+\))?[.!]?$/i, + /^(?:error:\s*)?max(?:imum)? turns reached[.!]?$/i, ] as const; +/** + * Return the diagnostic line that indicates a max-turns limit, if any. + * + * @param stdout - Captured standard output + * @param stderr - Captured standard error + */ +export function findMaxTurnsReachedLine(stdout: string, stderr: string): string | undefined { + const lines = [...outputLines(stderr), ...outputLines(stdout)]; + const matched = lines.find((line) => { + const normalized = line.normalized.trim(); + if (!normalized || isSourceOrDiffLine(normalized)) { + return false; + } + return MAX_TURNS_PATTERNS.some((pattern) => pattern.test(normalized)); + }); + return matched?.raw.trim(); +} + /** * Return whether agent output indicates the conversation hit a max-turns limit. * * @param stdout - Captured standard output * @param stderr - Captured standard error + * @param supportsMaxTurns - When false, skip scanning (harness cannot hit a CLI turn limit) */ -export function detectMaxTurnsReached(stdout: string, stderr: string): boolean { - const combined = `${stdout}\n${stderr}`; - return MAX_TURNS_PATTERNS.some((pattern) => pattern.test(combined)); +export function detectMaxTurnsReached( + stdout: string, + stderr: string, + supportsMaxTurns: boolean = true, +): boolean { + if (!supportsMaxTurns) { + return false; + } + return findMaxTurnsReachedLine(stdout, stderr) !== undefined; } diff --git a/packages/agent-harness/src/detect-usage-limit.ts b/packages/agent-harness/src/detect-usage-limit.ts index 6cba187..fe45367 100644 --- a/packages/agent-harness/src/detect-usage-limit.ts +++ b/packages/agent-harness/src/detect-usage-limit.ts @@ -37,6 +37,9 @@ * attempting text-based detection of opencode plan limits. */ +import type { OutputLine } from "./output-lines.js"; +import { isSourceOrDiffLine, outputLines } from "./output-lines.js"; + const USAGE_LIMIT_PATTERNS = [ // Keep subscription messages anchored to the whole line. Codex writes its // tool transcript to stderr, so a substring match also sees source such as @@ -59,13 +62,6 @@ const PROVIDER_LIMIT_PATTERNS = [ /\b429\b/, ] as const; -const ANSI_ESCAPE_PATTERN = new RegExp(`${String.fromCodePoint(0x1b)}\\[[0-?]*[ -/]*[@-~]`, "g"); - -interface OutputLine { - raw: string; - normalized: string; -} - const RESET_PATTERNS = [ // "resets 7:20pm (Asia/Ho_Chi_Minh)", "resets at 9am", "resets in 2 hours" /resets?\s+(?:at\s+|in\s+)?([0-9][^\n.]*?)(?:\.|\n|$)/i, @@ -101,46 +97,6 @@ function extractResetHint(text: string): string | undefined { return undefined; } -/** - * Strip terminal styling before matching, while retaining the original line - * for diagnostics. - */ -function stripAnsi(text: string): string { - return text.replace(ANSI_ESCAPE_PATTERN, ""); -} - -/** - * Split one captured stream into matchable lines. - */ -function outputLines(text: string): OutputLine[] { - return text.split(/\r?\n/).map((raw) => ({ - raw, - normalized: stripAnsi(raw), - })); -} - -/** - * Avoid treating source code, comments, quoted Markdown, search results, and - * diff hunks as provider errors. Agent transcripts can include arbitrary file - * content from tools such as `sed`, `rg`, and `git diff`, including literal - * provider diagnostics such as "429 Too Many Requests". - */ -function isSourceOrDiffLine(line: string): boolean { - const trimmed = line.trim(); - return ( - /^(?:diff --git|index\b|---\s|\+\+\+\s|@@\s)/i.test(trimmed) || - /^[+-](?![+-])/.test(trimmed) || - /^(?:\/\/|\/\*|\*|#|>|```|~~~)/.test(trimmed) || - // `rg -n`, grep, and compiler-style locations: path:line[:column]:content. - /^(?:(?:\.?\.?\/|\/)?(?:[^:\s]+\/)+[^:\s]+|[^:\s]+\.[a-z\d]+):\d+(?::\d+)?:/i.test(trimmed) || - /^(?:const|let|var|function|class|import|export|return)\b/.test(trimmed) || - /^(?:super|throw\s+new\s+Error|[\w$.]+\.(?:error|warn|log))\s*\(/.test(trimmed) || - /^(?:["'`]).*(?:["'`])[,;)]?$/.test(trimmed) || - /\b(?:includes|startsWith|endsWith|\.match|\.test)\s*\(/.test(trimmed) || - /=>/.test(trimmed) - ); -} - /** * Provider rate-limit phrases need stronger evidence than a subscription * limit phrase. In particular, a bare `429` or `Too Many Requests` in agent diff --git a/packages/agent-harness/src/harnesses/claude-code.ts b/packages/agent-harness/src/harnesses/claude-code.ts index 0424f22..2ec6d5e 100644 --- a/packages/agent-harness/src/harnesses/claude-code.ts +++ b/packages/agent-harness/src/harnesses/claude-code.ts @@ -32,6 +32,7 @@ export class ClaudeCodeHarness implements AgentHarness { readonly defaultPath = "claude"; readonly promptFlag = "-p"; readonly supportedModes = ["plan", "readonly"] as const; + readonly supportsMaxTurns = true; /** * Build `claude` CLI flags for non-interactive (`-p`) execution. diff --git a/packages/agent-harness/src/index.ts b/packages/agent-harness/src/index.ts index e93d60f..9e7fed2 100644 --- a/packages/agent-harness/src/index.ts +++ b/packages/agent-harness/src/index.ts @@ -118,7 +118,7 @@ export { } from "./spawn-agent.js"; // Max-turns detection -export { detectMaxTurnsReached } from "./detect-max-turns.js"; +export { detectMaxTurnsReached, findMaxTurnsReachedLine } from "./detect-max-turns.js"; // Usage/rate-limit detection export { detectUsageLimit, resetHintToMs, type UsageLimitResult } from "./detect-usage-limit.js"; diff --git a/packages/agent-harness/src/output-lines.ts b/packages/agent-harness/src/output-lines.ts new file mode 100644 index 0000000..6554ee6 --- /dev/null +++ b/packages/agent-harness/src/output-lines.ts @@ -0,0 +1,53 @@ +/** + * Line-level helpers for scanning agent CLI transcripts. + * + * Codex writes its full tool transcript to stderr, so callers must not treat + * either stream as trusted diagnostics. Split into lines, strip ANSI, and skip + * source / diff / search output before matching error phrases. + */ + +export interface OutputLine { + raw: string; + normalized: string; +} + +const ANSI_ESCAPE_PATTERN = new RegExp(`${String.fromCodePoint(0x1b)}\\[[0-?]*[ -/]*[@-~]`, "g"); + +/** + * Strip terminal styling before matching, while retaining the original line + * for diagnostics. + */ +export function stripAnsi(text: string): string { + return text.replace(ANSI_ESCAPE_PATTERN, ""); +} + +/** + * Split one captured stream into matchable lines. + */ +export function outputLines(text: string): OutputLine[] { + return text.split(/\r?\n/).map((raw) => ({ + raw, + normalized: stripAnsi(raw), + })); +} + +/** + * Avoid treating source code, comments, quoted Markdown, search results, and + * diff hunks as provider errors. Agent transcripts can include arbitrary file + * content from tools such as `sed`, `rg`, and `git diff`. + */ +export function isSourceOrDiffLine(line: string): boolean { + const trimmed = line.trim(); + return ( + /^(?:diff --git|index\b|---\s|\+\+\+\s|@@\s)/i.test(trimmed) || + /^[+-](?![+-])/.test(trimmed) || + /^(?:\/\/|\/\*|\*|#|>|```|~~~)/.test(trimmed) || + // `rg -n`, grep, and compiler-style locations: path:line[:column]:content. + /^(?:(?:\.?\.?\/|\/)?(?:[^:\s]+\/)+[^:\s]+|[^:\s]+\.[a-z\d]+):\d+(?::\d+)?:/i.test(trimmed) || + /^(?:const|let|var|function|class|import|export|return)\b/.test(trimmed) || + /^(?:super|throw\s+new\s+Error|[\w$.]+\.(?:error|warn|log))\s*\(/.test(trimmed) || + /^(?:["'`]).*(?:["'`])[,;)]?$/.test(trimmed) || + /\b(?:includes|startsWith|endsWith|\.match|\.test)\s*\(/.test(trimmed) || + /=>/.test(trimmed) + ); +} diff --git a/packages/agent-harness/src/runners/bun.ts b/packages/agent-harness/src/runners/bun.ts index 0e9aac5..a4d9875 100644 --- a/packages/agent-harness/src/runners/bun.ts +++ b/packages/agent-harness/src/runners/bun.ts @@ -92,6 +92,6 @@ export async function runAgentBun( stdout, stderr, exitCode, - maxTurnsReached: detectMaxTurnsReached(stdout, stderr), + maxTurnsReached: detectMaxTurnsReached(stdout, stderr, harness.supportsMaxTurns === true), }; } diff --git a/packages/agent-harness/src/runners/node.ts b/packages/agent-harness/src/runners/node.ts index c18a9cf..cd724cb 100644 --- a/packages/agent-harness/src/runners/node.ts +++ b/packages/agent-harness/src/runners/node.ts @@ -179,7 +179,7 @@ export async function runAgentNode( stdout, stderr, exitCode: code ?? 1, - maxTurnsReached: detectMaxTurnsReached(stdout, stderr), + maxTurnsReached: detectMaxTurnsReached(stdout, stderr, harness.supportsMaxTurns === true), }); } }); diff --git a/packages/agent-harness/src/types.ts b/packages/agent-harness/src/types.ts index 3460853..3dfe144 100644 --- a/packages/agent-harness/src/types.ts +++ b/packages/agent-harness/src/types.ts @@ -96,6 +96,13 @@ export interface AgentHarness { * Empty / omitted means only `"default"` is supported. */ readonly supportedModes?: readonly Exclude[]; + /** + * Whether this harness accepts a CLI turn limit (`--max-turns` or + * equivalent) and can emit a max-turns diagnostic. Omitted / false means + * callers skip transcript scanning so tool output cannot be mistaken for a + * turn-limit error. + */ + readonly supportsMaxTurns?: boolean; /** * Whether this harness's constrained modes still allow unrestricted * network and MCP tool use (web search, web fetch, MCP servers). diff --git a/packages/agent-harness/tests/detect-max-turns.test.ts b/packages/agent-harness/tests/detect-max-turns.test.ts index b675b18..9be4c59 100644 --- a/packages/agent-harness/tests/detect-max-turns.test.ts +++ b/packages/agent-harness/tests/detect-max-turns.test.ts @@ -1,10 +1,16 @@ import { describe, expect, test } from "bun:test"; -import { detectMaxTurnsReached } from "../src/detect-max-turns.js"; +import { + detectMaxTurnsReached, + findMaxTurnsReachedLine, +} from "../src/detect-max-turns.js"; describe("detectMaxTurnsReached", () => { test("detects Claude Code stdout message", () => { expect(detectMaxTurnsReached("Error: Reached max turns (1)\n", "")).toBe(true); + expect(findMaxTurnsReachedLine("Error: Reached max turns (1)\n", "")).toBe( + "Error: Reached max turns (1)", + ); }); test("detects message on stderr", () => { @@ -22,4 +28,36 @@ describe("detectMaxTurnsReached", () => { test("returns false for normal output", () => { expect(detectMaxTurnsReached("Hello! How can I help you today?\n", "")).toBe(false); }); + + test("skips scanning when the harness cannot impose a turn limit", () => { + expect(detectMaxTurnsReached("Error: Reached max turns (1)\n", "", false)).toBe(false); + }); + + test("ignores the phrase inside this repo's source and lint dumps", () => { + const transcript = [ + `@getdevintern/code lint: 3445 ā”‚ ā”‚ \`\\nšŸ”„ Moving \${taskKey} back to '\${todoStatus}' due to max turns reached...\`,`, + " /max turns reached/i,", + ' console.log("āš ļø Agent reached maximum turns limit without completing the task");', + " // Check if Agent reached max turns or had other issues", + ].join("\n"); + + expect(detectMaxTurnsReached(transcript, "")).toBe(false); + expect(detectMaxTurnsReached("", transcript)).toBe(false); + }); + + test("ignores compact diff additions", () => { + const diff = [ + "diff --git a/packages/code/src/index.ts b/packages/code/src/index.ts", + `+ \`\\nšŸ”„ Moving \${taskKey} back to '\${todoStatus}' due to max turns reached...\`,`, + ].join("\n"); + + expect(detectMaxTurnsReached("", diff)).toBe(false); + }); + + test("ignores file-location search output", () => { + const source = "packages/code/src/index.ts:3445:due to max turns reached..."; + + expect(detectMaxTurnsReached(source, "")).toBe(false); + expect(detectMaxTurnsReached("", source)).toBe(false); + }); }); diff --git a/packages/agent-harness/tests/harnesses.test.ts b/packages/agent-harness/tests/harnesses.test.ts index 1853ad5..803aa40 100644 --- a/packages/agent-harness/tests/harnesses.test.ts +++ b/packages/agent-harness/tests/harnesses.test.ts @@ -23,6 +23,7 @@ describe("ClaudeCodeHarness", () => { expect(h.displayName).toBe("Claude Code"); expect(h.defaultPath).toBe("claude"); expect(h.promptFlag).toBe("-p"); + expect(h.supportsMaxTurns).toBe(true); }); test("buildArgs empty", () => { @@ -76,6 +77,7 @@ describe("CodexHarness", () => { expect(h.displayName).toBe("Codex"); expect(h.defaultPath).toBe("codex"); expect(h.promptFlag).toBeUndefined(); + expect(h.supportsMaxTurns).toBeUndefined(); }); test("buildArgs empty", () => { diff --git a/packages/code/src/index.ts b/packages/code/src/index.ts index 965f285..845ec89 100755 --- a/packages/code/src/index.ts +++ b/packages/code/src/index.ts @@ -27,6 +27,7 @@ import { buildPromptArgs, detectIncompleteImplementation, detectMaxTurnsReached, + findMaxTurnsReachedLine, detectOpenQuestions, detectSandboxProviders, detectUsageLimit, @@ -2678,9 +2679,15 @@ async function runClarityCheck( } // Check if Agent reached max turns or had other issues - if (detectMaxTurnsReached(stdoutOutput, stderrOutput)) { + if ( + detectMaxTurnsReached(stdoutOutput, stderrOutput, harness.supportsMaxTurns === true) + ) { console.log("\nāš ļø Clarity assessment reached maximum conversation turns"); console.log(" This may indicate task complexity or insufficient details"); + const matchedLine = findMaxTurnsReachedLine(stdoutOutput, stderrOutput); + if (matchedLine) { + console.log(` Matched output: ${matchedLine}`); + } if (!skipComments) { console.log( " Will attempt to proceed with implementation but posting failure to task tracker...\n", @@ -3395,7 +3402,11 @@ async function runAgentHarness( return; } - const maxTurnsReached = detectMaxTurnsReached(stdoutOutput, stderrOutput); + const maxTurnsReached = detectMaxTurnsReached( + stdoutOutput, + stderrOutput, + harness.supportsMaxTurns === true, + ); if (maxTurnsReached) { console.log("āš ļø Agent reached maximum turns limit without completing the task"); @@ -3403,6 +3414,10 @@ async function runAgentHarness( console.log( " Consider breaking it into smaller tasks or increasing the max-turns limit", ); + const matchedLine = findMaxTurnsReachedLine(stdoutOutput, stderrOutput); + if (matchedLine) { + console.log(` Matched output: ${matchedLine}`); + } // Save incomplete implementation for analysis if (taskKey && stdoutOutput.trim()) { diff --git a/packages/code/src/lib/address-review.ts b/packages/code/src/lib/address-review.ts index 6c9d511..03bed9a 100644 --- a/packages/code/src/lib/address-review.ts +++ b/packages/code/src/lib/address-review.ts @@ -195,7 +195,11 @@ export async function runAgent( agent.on("close", (code: number | null) => { clearTimeout(timeout); sandboxCleanup().catch(() => {}); - const maxTurnsReached = detectMaxTurnsReached(stdoutOutput, stderrOutput); + const maxTurnsReached = detectMaxTurnsReached( + stdoutOutput, + stderrOutput, + harness.supportsMaxTurns === true, + ); const output = stdoutOutput + stderrOutput; resolve({ diff --git a/packages/code/src/webhook-server.ts b/packages/code/src/webhook-server.ts index 516e0af..5ae7fd3 100644 --- a/packages/code/src/webhook-server.ts +++ b/packages/code/src/webhook-server.ts @@ -14,6 +14,7 @@ import { join } from "path"; import PQueue from "p-queue"; import { detectMaxTurnsReached, + findMaxTurnsReachedLine, detectUsageLimit, resetHintToMs, resolveHarness, @@ -1324,7 +1325,11 @@ async function runAgentHarnessForReview( agent.on("close", (code: number | null) => { clearTimeout(timeout); sandboxCleanup().catch(() => {}); - const maxTurnsReached = detectMaxTurnsReached(stdoutOutput, stderrOutput); + const maxTurnsReached = detectMaxTurnsReached( + stdoutOutput, + stderrOutput, + harness.supportsMaxTurns === true, + ); const usage = detectUsageLimit(stdoutOutput, stderrOutput); const output = stdoutOutput + stderrOutput; @@ -1349,6 +1354,10 @@ async function runAgentHarnessForReview( usageResetHint: usage.resetsAt, }); } else if (maxTurnsReached) { + const matchedLine = findMaxTurnsReachedLine(stdoutOutput, stderrOutput); + if (matchedLine) { + console.log(` Matched output: ${matchedLine}`); + } resolve({ success: false, message: "Agent reached max turns limit",