mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 10:36:26 +02:00
fix(claude-local): stop sealing max-turns failures as model_not_found (#13885)
<!-- Written in Simplified Technical English -->
## 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 <builder@nublo.net>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2 files changed
+105
-1
No files matched your search
@@ -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<typeof import("@paperclipai/adapter-utils/execution-target")>(
|
||||
"@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<string, unknown>, 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");
|
||||
});
|
||||
});
|
||||
@@ -1224,7 +1224,14 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
? proc.errorCode
|
||||
: loginMeta.requiresLogin
|
||||
? "claude_auth_required"
|
||||
: failed && isClaudeModelNotFoundError({
|
||||
// Structured CLI signals outrank the model-not-found detector, which
|
||||
// scans the run's full stdout for a literal "model not found" phrase.
|
||||
// An agent's own tool output (grepping runtime source, tailing logs,
|
||||
// echoing an issue body) lands in that stdout, so a genuine
|
||||
// error_max_turns run that merely *mentions* the phrase would otherwise
|
||||
// be sealed as model_not_found. Same guard the providerQuota and
|
||||
// transientUpstream branches above already apply.
|
||||
: failed && !clearSessionForMaxTurns && !poisonedPreviousMessageId && isClaudeModelNotFoundError({
|
||||
parsed,
|
||||
stdout: proc.stdout,
|
||||
stderr: proc.stderr,
|
||||
|
||||
Reference in new issue
Block a user