mirror of
https://github.com/instructkr/claw-code.git
synced 2026-10-29 00:00:35 +08:00
fix(runtime): keep a failed turn's tool results, and size guards from 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 c8834cb4d1a3372eb70241429f641162a420297e)
This commit is contained in:
1 parent
7c38d78759
commit
6a797de094
7 files changed
+478
-93
No files matched your search
@@ -255,6 +255,25 @@ impl ApiError {
|
||||
}
|
||||
}
|
||||
|
||||
/// True for failures whose own text cannot say what went wrong: transport, decode, and frame
|
||||
/// errors.
|
||||
///
|
||||
/// These matter because a server that answers an oversized request with a non-SSE HTTP 500
|
||||
/// body lands here rather than in [`Self::is_context_window_failure`] — reqwest reports
|
||||
/// "error decoding response body" and the real cause is lost. Callers that know how large the
|
||||
/// request was can combine the two signals; callers that do not must treat this as unknown,
|
||||
/// never as an overflow.
|
||||
#[must_use]
|
||||
pub fn is_ambiguous_transport_failure(&self) -> bool {
|
||||
match self {
|
||||
Self::Http(_) | Self::Json { .. } | Self::InvalidSseFrame(_) => true,
|
||||
Self::RetriesExhausted { last_error, .. } => {
|
||||
last_error.is_ambiguous_transport_failure()
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn is_context_window_failure(&self) -> bool {
|
||||
match self {
|
||||
|
||||
@@ -27,9 +27,10 @@ pub use providers::openai_compat::{
|
||||
OpenAiCompatClient, OpenAiCompatConfig,
|
||||
};
|
||||
pub use providers::{
|
||||
detect_provider_kind, max_tokens_for_model, max_tokens_for_model_with_override,
|
||||
model_family_identity_for, model_family_identity_for_kind, provider_diagnostics_for_model,
|
||||
resolve_model_alias, ProviderDiagnostics, ProviderKind,
|
||||
context_window_override, detect_provider_kind, estimate_message_request_input_tokens,
|
||||
max_tokens_for_model, max_tokens_for_model_with_override, model_family_identity_for,
|
||||
model_family_identity_for_kind, model_token_limit, provider_diagnostics_for_model,
|
||||
resolve_model_alias, ModelTokenLimit, ProviderDiagnostics, ProviderKind,
|
||||
};
|
||||
pub use sse::{parse_frame, SseParser};
|
||||
pub use types::{
|
||||
|
||||
@@ -619,16 +619,43 @@ fn web_passthrough_diagnostic(
|
||||
}
|
||||
}
|
||||
|
||||
/// Overrides the context window every sizing guard is computed against.
|
||||
///
|
||||
/// The model *name* is not a reliable statement of the window when the endpoint is a local
|
||||
/// server: the harness sends its compiled-in default name (a Claude alias) while the backend
|
||||
/// serves whatever `-c` it was started with. Every guard downstream of [`model_token_limit`]
|
||||
/// then sizes itself against the name's window instead of the real one, which makes the
|
||||
/// preflights unable to fire rather than merely inaccurate. This variable is the operator's
|
||||
/// statement of the truth, and it wins over the table.
|
||||
const CONTEXT_WINDOW_ENV_VAR: &str = "CLAW_CONTEXT_WINDOW";
|
||||
|
||||
/// Fraction of the window a single response may be allowed to claim when the window is
|
||||
/// declared. Output and input share one budget on a local server, so reserving a quarter for
|
||||
/// the reply leaves three quarters for prompt plus transcript.
|
||||
const MAX_OUTPUT_SHARE_OF_WINDOW: u32 = 4;
|
||||
|
||||
/// The operator-declared context window, when set and parseable as a positive token count.
|
||||
#[must_use]
|
||||
pub fn context_window_override() -> Option<u32> {
|
||||
parse_context_window_override(std::env::var(CONTEXT_WINDOW_ENV_VAR).ok().as_deref())
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
fn parse_context_window_override(value: Option<&str>) -> Option<u32> {
|
||||
value
|
||||
.and_then(|raw| raw.trim().parse::<u32>().ok())
|
||||
.filter(|window| *window > 0)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn max_tokens_for_model(model: &str) -> u32 {
|
||||
let canonical = resolve_model_alias(model);
|
||||
let heuristic = if canonical.contains("opus") {
|
||||
32_000
|
||||
} else {
|
||||
64_000
|
||||
};
|
||||
|
||||
model_token_limit(model).map_or(heuristic, |limit| heuristic.min(limit.max_output_tokens))
|
||||
// A declared window caps the reply too. Asking a 65k server for 32k of output reserves half
|
||||
// the window for a single message, which the server then silently clamps — leaving the
|
||||
// client's own accounting wrong by the difference.
|
||||
model_token_limit(model).map_or_else(
|
||||
|| max_tokens_for_model_by_name(model),
|
||||
|limit| max_tokens_for_model_by_name(model).min(limit.max_output_tokens),
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns the effective max output tokens for a model, preferring a plugin
|
||||
@@ -641,6 +668,33 @@ pub fn max_tokens_for_model_with_override(model: &str, plugin_override: Option<u
|
||||
|
||||
#[must_use]
|
||||
pub fn model_token_limit(model: &str) -> Option<ModelTokenLimit> {
|
||||
// A declared window replaces the table's, and supplies one for models the table has never
|
||||
// heard of. Returning `Some` for an unknown model is the point: `None` makes both preflights
|
||||
// return `Ok` without looking at anything, so an unrecognised local model gets no guard at all.
|
||||
if let Some(context_window_tokens) = context_window_override() {
|
||||
return Some(ModelTokenLimit {
|
||||
max_output_tokens: max_tokens_for_model_by_name(model)
|
||||
.min(context_window_tokens / MAX_OUTPUT_SHARE_OF_WINDOW),
|
||||
context_window_tokens,
|
||||
});
|
||||
}
|
||||
model_token_limit_by_name(model)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
fn max_tokens_for_model_by_name(model: &str) -> u32 {
|
||||
let canonical = resolve_model_alias(model);
|
||||
let heuristic = if canonical.contains("opus") {
|
||||
32_000
|
||||
} else {
|
||||
64_000
|
||||
};
|
||||
model_token_limit_by_name(model)
|
||||
.map_or(heuristic, |limit| heuristic.min(limit.max_output_tokens))
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
fn model_token_limit_by_name(model: &str) -> Option<ModelTokenLimit> {
|
||||
let canonical = resolve_model_alias(model);
|
||||
let base_model = canonical.rsplit('/').next().unwrap_or(canonical.as_str());
|
||||
match base_model {
|
||||
@@ -723,7 +777,7 @@ pub fn preflight_message_request(request: &MessageRequest) -> Result<(), ApiErro
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn estimate_message_request_input_tokens(request: &MessageRequest) -> u32 {
|
||||
pub fn estimate_message_request_input_tokens(request: &MessageRequest) -> u32 {
|
||||
let mut estimate = estimate_serialized_tokens(&request.messages);
|
||||
estimate = estimate.saturating_add(estimate_serialized_tokens(&request.system));
|
||||
estimate = estimate.saturating_add(estimate_serialized_tokens(&request.tools));
|
||||
|
||||
Reference in new issue
Block a user