fix: harden the agent loop against a self-hosted Anthropic-compatible backend - #3290
Open
reacechalmers-lab wants to merge 10 commits into
Open
fix: harden the agent loop against a self-hosted Anthropic-compatible backend#3290reacechalmers-lab wants to merge 10 commits into
reacechalmers-lab wants to merge 10 commits into
Conversation
An unbounded read_file returned the entire file, which on a small-context
backend overflows the model's window. The backend then rejects the whole
request (llama-server: "request (N tokens) exceeds the available context
size"), which kills the turn — so reading everything at once is precisely
what makes the session stop. Observed live: one 100 KB source file is
~25k tokens against a 32k window with a ~6k-token prefix, and a session
read the same file three times and died on six 400s.
read_file now returns a page and says so:
- absent `limit` yields DEFAULT_READ_LINES (2000) rather than EOF
- MAX_READ_CHARS (64 KB) bounds the payload even when `limit` is
explicit, so minified or generated files can't defeat the line cap
- `truncated` and `nextOffset` tell the caller it has a partial view
and exactly where to resume; the tool description explains both
MAX_READ_SIZE (10 MB) is untouched: it is a process guard against slurping
huge or binary blobs, not a context guard, and at ~2.5M tokens it never
fires on the files that actually overflow a window.
Also clears pre-existing clippy failures in trident.rs that blocked
-D warnings on every edit to this crate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q9McEEH32ZtyM62GSXVSyJ
(cherry picked from commit c796d7e)
`ApiError` is the `Err` half of nearly every `Result` in the api crate, and
its `Api` variant carried ~139 bytes inline (8 fields, four of them
`Option<String>`). That tripped clippy::result_large_err on 35 signatures
and, with -D warnings, made the whole crate unbuildable under the repo's
own documented verification command.
Move the payload into `ApiErrorDetails` behind a `Box`:
- `ApiError::Api(Box<ApiErrorDetails>)`, with `ApiError::api(..)` to
construct and `api_details()` to borrow
- `ApiErrorDetails` re-exported from the crate root
- adding a field here can no longer re-inflate every signature
This also simplifies `enrich_bearer_auth_error`, which previously
destructured and rebuilt all eight fields at five separate return points
just to append a hint; it now mutates the boxed payload in place.
Also clears the remaining pre-existing lints that blocked -D warnings:
unused test bindings, a redundant `use serde_json`, and an explicit
`.into_iter()` in claw-rag-service.
Verified: cargo test --workspace passes, 0 failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q9McEEH32ZtyM62GSXVSyJ
(cherry picked from commit e9210bd)
`cargo clippy --workspace --all-targets -- -D warnings` is this repo's own
documented verification command, and it could not run to completion. Earlier
compile failures were masking a further 48 lints; with those cleared the
workspace now passes with zero findings.
Real fixes, not suppressions:
- status_json_value took 8 positional parameters, four of them a tail of
`Option<&_>` that most callers passed as None — trivially transposable.
Grouped into `StatusJsonExtras` with a Default impl; all 7 call sites now
name only the fields they actually set.
- extract_help_metadata's 8-tuple return is now a documented `HelpMetadata`
alias.
- the auto-compact loop indexed preserve_schedule by its loop counter;
it now iterates the array directly.
- a sandbox status chain had two identical arms; merged the condition.
- a filter_map whose every arm returned Some is now a map.
- 33 unused stdout/stderr bindings in the output-format contract tests are
prefixed with `_`, and 9 `map_or(false, ..)` are now `is_some_and(..)`.
NOTE: several `--bin claw` tests (parse_args defaults) read the *real*
`~/.claw/settings.json` rather than an isolated config home, so they fail on
any machine whose user config sets `permissions.defaultMode`. With that file
moved aside the binary's suite is 207/207. That isolation gap is pre-existing
and left for a separate change.
Verified: clippy workspace/all-targets/-D warnings reports 0; cargo test
--workspace passes with the user config neutralised; scripts/fmt.sh clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q9McEEH32ZtyM62GSXVSyJ
(cherry picked from commit 03c8a8f)
The suite asserted against whatever the developer's box happened to be configured as. Two independent leaks, both surfaced by a real `~/.claw`: 1. `parse_args` resolves defaults — notably `permissions.defaultMode` — through the user-scope config, which `default_config_home()` reads from `CLAW_CONFIG_HOME` falling back to `$HOME/.claw`. Nothing overrode it, so a `defaultMode` of `dontAsk` on the host turned 12 assertions from `WorkspaceWrite` into `DangerFullAccess`. `env_lock()` now returns an `EnvGuard` that points `CLAW_CONFIG_HOME` at a fresh empty directory and restores the previous value on drop, so "no user config" is the state under test. All 42 call sites already bind the guard, so this fixes them at once; tests that want a populated config home can still set the var themselves afterwards. `removed_login_and_logout_subcommands_error_helpfully` asserted a Default-sourced permission mode without taking the guard at all, and now does. 2. `direct_resume_safe_slash_commands_route_to_local_json_actions_831` hardcoded `/sandbox` => "warn", which is only true where namespace isolation is unsupported and just the filesystem sandbox is active (ultraworkers#731). On a host with working user namespaces the correct answer is "ok". That entry now accepts either; the test is about ultraworkers#831 routing — local JSON action instead of `interactive_only` — not about the kernel under the runner. Verified with the real ~/.claw/settings.json in place (defaultMode=dontAsk): cargo test --workspace 0 failures, clippy --workspace --all-targets -D warnings 0 findings, scripts/fmt.sh clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q9McEEH32ZtyM62GSXVSyJ (cherry picked from commit 67aa453)
…nning total `maybe_auto_compact` gated on `usage_tracker.cumulative_usage().input_tokens`, which is wrong in two independent ways: 1. It ignores cached input. With prompt caching a warm call reports `input_tokens: 2` and puts the real context in `cache_read_input_tokens`, so the counter barely moved and auto-compaction never fired at all, however large the conversation grew. Backends without prompt caching report the whole prompt as `input_tokens`, so the bug was invisible there. 2. A sum across calls is not a context measurement, and it never decreases. Once it did cross the threshold, every subsequent iteration compacted again, permanently, even after compaction had shrunk the context. Gate on the latest call's total input instead — input + cache_creation + cache_read — which is the live context window size. It rises with the real context and falls back below the threshold once compaction has done its job, so no counter reset is needed. `CLAUDE_CODE_AUTO_COMPACT_INPUT_TOKENS` now means what it reads like: compact when the live context reaches N tokens. The default of 100_000 is unchanged and still lands at 50% of a 200k window. Adds `TokenUsage::total_input_tokens()` (saturating) and four tests: the cached-usage shape that never fired, non-latching across three turns, and two unit tests for the new helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bVvRYn7CQKR6hT2j1L6Cs (cherry picked from commit aebba4e)
The usage-based compaction gate is post-hoc: it can only see the context after a call returns and reports its counters. Between that measurement and the next request the session can grow enormously - a reasoning model emitting a long thinking block, then a tool result carrying a large file. Observed live: a call measured at 17,858 prompt tokens generated 10,842 more, a read_file added ~19k on top, and the next request reached the provider at 47,755 tokens and was rejected outright. No tuning of the post-hoc threshold prevents that, because nothing measures the session in between. Add a pre-send check driven by estimate_session_tokens, a local heuristic over the transcript, so it costs no API call and works before any usage has been recorded at all. Note the limit of this: compaction preserves the 4 most recent messages, and in an agentic tool-use loop those messages ARE the tool results carrying the bulk. This check bounds gradual growth; it cannot rescue a session whose last four messages already exceed the window. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bVvRYn7CQKR6hT2j1L6Cs (cherry picked from commit 26ab52d)
Two related gaps let a single agentic turn exceed a small context window, with no recovery path once it happened. 1. read_file's character ceiling was a hardcoded 64 KB (~16k tokens), sized for a 200k window. Two reads in one turn reach ~48k tokens, which a 32k backend rejects outright. Make it configurable via CLAW_MAX_READ_CHARS, defaulting to the existing 64 KB so behavior is unchanged unless set. Reads remain paged, not refused: callers still get truncated/nextOffset. 2. Compaction could not help, because it works at message granularity and an agentic turn is five messages (user / assistant+tool_use / tool_result / assistant+tool_use / tool_result). With preserve_recent_messages: 4 the only droppable message is the short user prompt, so the two oversized tool results were preserved verbatim by design. Trim preserved tool-result payloads over 16 KB to a head/tail excerpt with an explicit "[... N characters elided ...]" marker, so the model knows its view is partial and can re-read. Small payloads pass through untouched. Also apply trimming on the early-return path: a session can exceed the window while holding fewer messages than preserve_recent_messages, where should_compact is false and message-granular compaction has no move to make. compact_now no longer gates on removed_message_count alone, which would have computed the smaller session and then discarded it. Also fixes a pre-existing flaky test: repl_executes_python_code resolves the python runtime through PATH, which the PowerShell tests blank while they run. It now takes the same env_lock, so it no longer intermittently fails with "python runtime not found" on a machine that has python. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bVvRYn7CQKR6hT2j1L6Cs (cherry picked from commit b0d4d41)
The git diff snapshot is rendered into the *system prompt*, so its cost is paid again by every session whose working tree has changed -- and a diff changes on essentially every edit. The 50_000-char budget was four times the entire instruction-file budget (12_000) and unbounded in practical effect: a single modified file added ~15_000 tokens to the prompt. Measured against a self-hosted backend, one uncommitted change took the prompt from 8_302 to 23_457 tokens and a one-word reply from ~3 s to 296 s. With the new default the same session is 7_911 tokens and 36 s. That cost is absorbable against a hosted frontier model and ruinous against a local one. Default is now 4_000 chars, matching MAX_INSTRUCTION_FILE_CHARS, and is overridable with CLAW_MAX_GIT_DIFF_CHARS following the same pattern as CLAW_MAX_READ_CHARS. A budget of 0 omits the snapshot entirely rather than emitting an empty section with a truncation marker; git status and the recent-commit list are unaffected either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RNKqVB2JE9UnCkz8FC9HPy (cherry picked from commit e01c6ea)
A reply carrying no tool_use ended the turn unconditionally. Small local models routinely narrate the next tool call in prose — "Now running the EEC-V probe:" — and then emit EOS, so the announced work never happened and the user was dropped back at the prompt. The same failure wears a second hat mid-task: stopping to ask for a go-ahead on work already requested. The loop now inspects the final line of a tool-less reply. If it announces a pending action, or asks a question after tools have already run this turn, a short nudge is pushed as a user message and the loop continues instead of breaking. The budget is bounded (default 3 per turn, CLAW_MAX_STALL_NUDGES, 0 disables) so a model that really is finished still gets to stop. Prevention pairs with recovery: the "Doing tasks" system-prompt section now tells the model to act in the turn it plans in. Tests cover the auto-continue, the bound, a plain completion that must NOT be nudged, the disable switch, and last-line-only detection. The behavioural pair was mutation-checked: reverting the continue to a break fails both. (cherry picked from commit 9035fe6)
… the real context window Five defects found by auditing the agent stack, all of which let the harness act confidently on a wrong picture of its own state. 1. A turn that errored discarded the record of tools that had already run. The CLI hands run_turn a CLONE of the session; the turn appends the user message, each assistant reply, and a tool_result per executed tool to that clone alone. The error path dropped it while the side effects of those tools remained on disk, and the recovery path then replayed the same user input against a session showing no sign they had ever run — inviting a second execution of tools that are not idempotent. The clone is now adopted before teardown, and recovery resumes the turn instead of replaying it (new ConversationRuntime::resume_turn). 2. Every sizing guard was computed against the model NAME's context window. With a local server behind a cloud model alias that is 200k against a much smaller backend, which does not make the preflights inaccurate — it makes them unreachable. CLAW_CONTEXT_WINDOW now declares the real window; model_token_limit honours it and returns a limit for unrecognised models instead of None, max_tokens is capped to a quarter of it, and the auto-compaction threshold derives from it (70%) rather than defaulting to a 200k-window constant. 3. Whether to recover by discarding conversation history was decided by substring matching on rendered error prose, including strings that ordinary transport and decode failures also produce — so a dropped connection cost the session its transcript. Classification now happens in the API layer, where the error still has its type and the request is still in hand: typed overflow errors, plus ambiguous transport failures only when the request actually filled the window. The two copy-pasted detection blocks are gone. 4. The top-level agent loop had no iteration ceiling (usize::MAX) while spawned subagents were bounded at 32. It now gets the same bound, overridable with CLAW_MAX_TURN_ITERATIONS. 5. Auto-continue nudges were injected as USER turns, so harness-authored text was persisted, re-sent on every later request, and folded into compaction summaries as something the user had said. They are recorded as system turns now; the wire payload is unchanged. Also replaces the repository's own self-host section, which described a translator and a serving stack no longer in the request path. That file is folded into the system prompt verbatim, so it was briefing the model with an architecture that does not exist. (cherry picked from commit c8834cb)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ten independent fixes found by running claw-code against a self-hosted Anthropic-compatible
server (llama.cpp's native
/v1/messages) instead of the Anthropic API. Every one of them is aplace where the harness acts on a confident but wrong picture of its own state; most are latent
against the cloud API and only become visible when the backend's context window is smaller than
the model name implies, or when a request fails part-way through a tool-using turn.
No machine-specific configuration is included — this is code only.
The two that can cause real damage
A failed turn discarded the record of tools that had already run. The CLI hands
run_turnaclone of the session; the turn appends the user message, each assistant reply, and a
tool_resultper executed tool to that clone alone. The error path dropped the clone while theside effects of those tools remained on disk, and the auto-compact recovery then replayed the
same user input against a session showing no sign they had ever run — inviting a second
execution of tools that are not idempotent (
bash,write_file,edit). The clone is nowadopted before teardown, and recovery calls a new
ConversationRuntime::resume_turnthatcontinues the existing turn instead of replaying it.
Whether to discard conversation history was decided by substring-matching error prose. The
recovery gate matched on strings including
"error decoding response body"and"Failed to parse input at pos", which ordinary transport and decode failures produce just asreadily as an overflow does — so a dropped connection or a client-side timeout cost the session
its transcript. Classification now happens in the API layer, where the error still has its type
and the request is still in hand: typed overflow errors, plus ambiguous transport failures only
when the request actually filled the window (
ApiError::is_ambiguous_transport_failure+request_fills_context_window). Two copy-pasted detection blocks are replaced by one typedcheck carried on
RuntimeError.Context-window sizing
Every sizing guard was computed from the model name's context window. Against a local server
behind a cloud model alias that is 200k versus a much smaller backend, which does not make the
preflights inaccurate — it makes them unreachable.
model_token_limitalso returnedNonefor unrecognised models, and
Nonemeans both preflights returnOkwithout checking anything.CLAW_CONTEXT_WINDOWnow declares the real window and is the single source for:model_token_limit— honours it, and returns a limit for unrecognised models instead ofNonemax_tokens_for_model— capped to a quarter of the windowCLAUDE_CODE_AUTO_COMPACT_INPUT_TOKENSis unset, instead of a 200k-window constantUnset, behaviour is unchanged.
Also included
usize::MAX) while spawned subagents werebounded at 32. It now gets the same bound;
CLAW_MAX_TURN_ITERATIONSoverrides.persisted, re-sent on every later request, and folded into compaction summaries as something
the user had said. They are recorded as system turns now; the wire payload is unchanged.
read_fileinstead of whole-file reads, auto-compactionmeasuring the live context rather than a cumulative total, a pre-request compaction check,
tool-result payload bounds, a configurable git-diff snapshot budget, auto-continue for a
stalled turn,
ApiError::Apiboxed to clearresult_large_err, and test isolation from themachine's real user config.
Test plan
cargo test --workspace— all green, includes new tests for the resume path, thesalvaged tool results, system-role nudges, and threshold derivation
cargo clippy --workspace --all-targets -- -D warnings— cleanscripts/fmt.sh --check— cleantest goes red, then restored
read_filetool calland returns the correct answer, exit 0
blocked with
context_window_blockedrather than being sent🤖 Generated with Claude Code