From cc549691a18b2642c689ee035b46c848ada5db06 Mon Sep 17 00:00:00 2001 From: Sergio-LPA Date: Fri, 16 Oct 2026 00:01:40 +0100 Subject: [PATCH] fix(claude-local): stop sealing max-turns failures as model_not_found (#13885) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The `claude_local` adapter runs the Claude Code CLI and maps each finished process to a terminal result. The `errorCode` in that result decides which recovery path the issue takes. > - `execute.ts` resolves the error code with one long ternary chain. The order of the branches is the precedence rule. > - The model-not-found branch sits before the max-turns branch, and it is driven by `isClaudeModelNotFoundError()`, which regex-scans the run's full stdout for the phrase `model_not_found` (also `model not found`, `unknown model`, `model does not exist`). > - The agent's own tool output is part of that stdout. A run that greps adapter source, tails a log, or echoes an issue body about this defect matches the detector on text the agent printed, not on text the provider returned. > - A real `subtype=error_max_turns` failure therefore never reaches `max_turns_exhausted`. Downstream, `model_not_found` counts as a configuration-incomplete failure, so the issue goes to `blocked` with a wrong "configuration incomplete" notice instead of the normal max-turns continuation. > - This pull request gates the model-not-found branch behind the two structured signals that the `providerQuota` and `transientUpstream` branches above already apply. > - The benefit is that a structured CLI signal outranks a regex scan of untrusted output, so exhausted runs continue instead of blocking the issue. ## Linked Issues or Issue Description No public issue exists for this defect. I searched the open pull request list for `packages/adapters/claude-local` and for the `model_not_found` and `max_turns_exhausted` error codes, and found no duplicate or related pull request. The description follows the bug template. **What happened** A run that fails with `subtype=error_max_turns` is recorded with `error_code=model_not_found`. In `packages/adapters/claude-local/src/server/execute.ts`, the resolved-error-code ternary evaluates the model-not-found branch before the max-turns branch: ```ts : failed && isClaudeModelNotFoundError({ parsed, stdout: proc.stdout, ... }) ? "model_not_found" : failed && clearSessionForMaxTurns ? "max_turns_exhausted" ``` `isClaudeModelNotFoundError()` in `parse.ts` pushes `input.stdout` into its candidate list and tests it against `CLAUDE_MODEL_NOT_FOUND_RE`. The run's stdout holds the agent's own tool output, so any run that prints the phrase poisons its own classification. Because the branch is first, a genuine max-turns failure never reaches `max_turns_exhausted`. This is not cosmetic. Downstream, `model_not_found` is treated as a configuration-incomplete failure. The issue moves to `blocked` with a misleading "configuration incomplete" recovery notice instead of the normal max-turns continuation path. **Steps to reproduce** 1. Run a `claude_local` agent on a task whose tool output contains the literal string `model_not_found`. Grepping the adapter source or reading a bug report about this defect is enough. 2. Let the run exhaust its turn budget, so the CLI emits `subtype=error_max_turns`. 3. Observe that the recorded `error_code` is `model_not_found` and not `max_turns_exhausted`. **Expected behavior** A run whose parsed result carries `subtype=error_max_turns` is classified `max_turns_exhausted`, whatever phrases appear in its stdout. A structured CLI signal outranks a regex scan of untrusted output. **Paperclip version or commit** `ghcr.io/paperclipai/paperclip@sha256:23090eb6`. The branch is rebased onto current `master` (`b41ccf0`). The adapter is `@paperclipai/adapter-claude-local` on the `engine=cli` lane. **Deployment mode** Self-hosted, Docker Compose. ## What Changed - `packages/adapters/claude-local/src/server/execute.ts` - Gate the model-not-found branch with `!clearSessionForMaxTurns && !poisonedPreviousMessageId`, the same guard the `providerQuota` and `transientUpstream` branches ~30 lines above already apply. One line, no new state. - Add a comment that records why the order matters, so a later reorder is a deliberate decision. - `packages/adapters/claude-local/src/server/execute.max-turns-classification.test.ts` (new) - A parsed `error_max_turns` result whose stdout carries the literal `model_not_found` phrase still classifies as `max_turns_exhausted`. - A real provider 404 (`API Error: 404 model not found`) still classifies as `model_not_found`, so the guard does not hide the genuine case. Only the parsed-result path changes. The unparsed fallback path (`if (!parsed)`) is left alone: neither guard variable is in scope there, and a run with no parsed result has no structured max-turns signal to honour. ## Verification - `pnpm exec vitest run src/server/execute.max-turns-classification.test.ts` in `packages/adapters/claude-local`: 2/2 pass. - Negative control: with the one-line guard reverted, the first test fails with `expected "max_turns_exhausted", received "model_not_found"`. The test pins the defect and not the surrounding code. - `pnpm exec vitest run` for the whole adapter: 313 pass, 1 skipped, 3 fail. The 3 failures are in `acp.test.ts` and `acp.auth.test.ts`, all with `claude_acp_node_unsupported` because the local machine runs Node v22.23.1 and the repository requires Node >= 24.11.0. They fail the same way on `master` without this branch, and they do not touch the CLI classification path. - `tsc --noEmit -p tsconfig.json` for the adapter: no errors. ## Risks - Low risk. The change is one added condition on one branch of an existing ternary. - A real model-not-found failure still classifies correctly, because neither `clearSessionForMaxTurns` nor `poisonedPreviousMessageId` is set on that path. The second test covers this. - The narrow case that changes behaviour is a run that is both out of turns and pointed at a missing model. That run now reports `max_turns_exhausted` first. The next run reports the model error against a clean session, which is the same outcome the quota and transient-upstream branches already accept. ## Model Used - Provider: Anthropic Claude. - Exact model ID: `claude-opus-5` (Claude Opus 5). - Reasoning mode: extended thinking. - Capability details: tool use for shell, file editing and test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes (no user-facing documentation covers adapter error-code precedence) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Builder Co-authored-by: Claude Opus 5 --- .../execute.max-turns-classification.test.ts | 97 +++++++++++++++++++ .../claude-local/src/server/execute.ts | 9 +- 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 packages/adapters/claude-local/src/server/execute.max-turns-classification.test.ts diff --git a/packages/adapters/claude-local/src/server/execute.max-turns-classification.test.ts b/packages/adapters/claude-local/src/server/execute.max-turns-classification.test.ts new file mode 100644 index 00000000000..2fb496918dd --- /dev/null +++ b/packages/adapters/claude-local/src/server/execute.max-turns-classification.test.ts @@ -0,0 +1,97 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { runAdapterExecutionTargetProcess } = vi.hoisted(() => ({ + runAdapterExecutionTargetProcess: vi.fn(), +})); + +vi.mock("./acp.js", () => ({ + createClaudeAcpExecutor: () => vi.fn(), + resolveClaudeExecutionEngineForRun: async () => ({ engine: "cli", explicit: true }), +})); + +vi.mock("@paperclipai/adapter-utils/execution-target", async () => { + const actual = await vi.importActual( + "@paperclipai/adapter-utils/execution-target", + ); + return { + ...actual, + ensureAdapterExecutionTargetCommandResolvable: vi.fn(async () => undefined), + ensureAdapterExecutionTargetRuntimeCommandInstalled: vi.fn(async () => undefined), + resolveAdapterExecutionTargetCommandForLogs: vi.fn(async () => "claude"), + runAdapterExecutionTargetProcess, + }; +}); + +import { execute } from "./execute.js"; + +// Tool output the agent itself printed. It carries the literal phrase the +// model-not-found detector scans for, but the provider never returned it. +const POISONED_TOOL_OUTPUT = + 'execute.ts:1199: : failed && isClaudeModelNotFoundError({ ... ? "model_not_found"'; + +function buildProcResult(resultEvent: Record, extraStdout = "") { + return { + exitCode: 1, + signal: null, + timedOut: false, + stdout: [ + JSON.stringify({ type: "system", subtype: "init", session_id: "claude-session-1", model: "claude-sonnet" }), + ...(extraStdout + ? [ + JSON.stringify({ + type: "assistant", + session_id: "claude-session-1", + message: { content: [{ type: "text", text: extraStdout }] }, + }), + ] + : []), + JSON.stringify({ type: "result", session_id: "claude-session-1", ...resultEvent }), + ].join("\n"), + stderr: "", + pid: 321, + startedAt: new Date().toISOString(), + }; +} + +function buildContext() { + return { + runId: "run-1", + agent: { + id: "agent-1", + companyId: "company-1", + name: "Claude Coder", + adapterType: "claude_local", + adapterConfig: {}, + }, + runtime: { sessionId: null, sessionParams: null, sessionDisplayId: null, taskKey: null }, + config: { engine: "cli" }, + context: {}, + onLog: vi.fn(async () => {}), + }; +} + +describe("claude_local failure classification precedence", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("keeps max_turns_exhausted when stdout mentions model_not_found", async () => { + runAdapterExecutionTargetProcess.mockResolvedValue( + buildProcResult({ is_error: true, subtype: "error_max_turns" }, POISONED_TOOL_OUTPUT), + ); + + const result = await execute(buildContext() as never); + + expect(result.errorCode).toBe("max_turns_exhausted"); + }); + + it("still reports model_not_found when the provider returns it", async () => { + runAdapterExecutionTargetProcess.mockResolvedValue( + buildProcResult({ is_error: true, subtype: "error", result: "API Error: 404 model not found" }), + ); + + const result = await execute(buildContext() as never); + + expect(result.errorCode).toBe("model_not_found"); + }); +}); diff --git a/packages/adapters/claude-local/src/server/execute.ts b/packages/adapters/claude-local/src/server/execute.ts index de03c588c03..ac7198ebe6d 100644 --- a/packages/adapters/claude-local/src/server/execute.ts +++ b/packages/adapters/claude-local/src/server/execute.ts @@ -1224,7 +1224,14 @@ export async function execute(ctx: AdapterExecutionContext): Promise