From 6acb39eabea7e08b571b6a5666c6ddb8f4a788ea Mon Sep 17 00:00:00 2001 From: asto Date: Thu, 15 Oct 2026 16:07:27 +0800 Subject: [PATCH 1/9] fix(tui): carry only host-declared route limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review of #108 (13ee4dfd2) left two MAJORs and one m2 on this seam. The carry in `current_runtime_route` re-stated the whole installed *effective* triple as host declarations, but that triple also contains catalog, operator-config, and Codex-roster facts: after a config reload (which intentionally does not reinstall engine limits) every engine-self-started turn pinned the stale values under host provenance until the next host turn. And because `append_embedding_host_limit_overrides` rejects out-of-range context values, an embedder- or SetModel-supplied declaration with `context_tokens` of 0 or >u32::MAX — stored verbatim by every install path — now hard-failed self-started turns as "provider route is no longer valid" where every other limit read point degrades gracefully. - `ReadyRouteCandidate::embedding_host_declared_limits()` reconstructs the host-declared subset from the candidate's recorded `EmbeddingHost` provenance; the applier records exactly one entry per declared `Some` field, so the reconstruction is lossless. - `Engine` gains a private `active_route_declared_limits` field written in lockstep at every effective-limits writer: construction (embedder declaration verbatim), both install paths (reconstructed from the candidate), `Op::SetModel` (op payload), and the turn-start apply in `handle_send_message`. - The carry now re-states only that declared subset. With no declaration the fresh resolution wins, so catalog/operator/roster facts keep refreshing; guards 1-3 and their comparison baselines are unchanged. - Declared context is sanitized to (0, u32::MAX] before the carry, and an all-unknown sanitized declaration skips the re-resolution entirely. - Pure move: the `Op::SetModel` and `Op::SyncSession` arm bodies become `apply_set_model`/`apply_synced_session` methods (token-identical to the previous arm bodies) so the lockstep contracts are unit-drivable. The #108 behavior for genuinely host-declared routes is unchanged: those install with `EmbeddingHost` provenance, the reconstruction returns the declaration, and the carry re-states it under the same three guards. Signed-off-by: asto --- crates/config/src/route/candidate.rs | 21 ++ crates/tui/src/core/engine.rs | 396 ++++++++++++++++----------- 2 files changed, 253 insertions(+), 164 deletions(-) diff --git a/crates/config/src/route/candidate.rs b/crates/config/src/route/candidate.rs index 01cdee6614..36ed481bcc 100644 --- a/crates/config/src/route/candidate.rs +++ b/crates/config/src/route/candidate.rs @@ -325,4 +325,25 @@ impl ReadyRouteCandidate { pub fn applied_limit_overrides(&self) -> &[SourcedLimitOverride] { &self.applied_limit_overrides } + + /// The subset of these limits an embedding host explicitly declared, + /// reconstructed from the recorded [`OverrideSource::EmbeddingHost`] + /// provenance. Catalog-, operator-config-, and roster-derived facts are + /// absent, so re-stating the result on a later re-derivation cannot pin + /// their stale values under host provenance. + #[must_use] + pub fn embedding_host_declared_limits(&self) -> Option { + let mut declared = RouteLimits::default(); + for item in &self.applied_limit_overrides { + if item.source != OverrideSource::EmbeddingHost { + continue; + } + match item.field { + LimitField::ContextTokens => declared.context_tokens = item.value, + LimitField::InputTokens => declared.input_tokens = item.value, + LimitField::OutputTokens => declared.output_tokens = item.value, + } + } + declared.has_known_limit().then_some(declared) + } } diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 68d4e3bf79..521f350745 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -1168,6 +1168,13 @@ pub struct Engine { /// and a budget declared for another conversation's model must not ride /// a re-derivation onto the restored one. active_route_limits_model: Option, + /// The subset of `active_route_limits` the embedding host explicitly + /// declared, as opposed to the catalog-merged effective value. Written in + /// lockstep with the effective limits so a re-derivation can re-state + /// exactly the host's own budget: catalog, operator-config, and roster + /// facts must keep refreshing from the live config on engine-self-started + /// turns instead of being pinned stale under host provenance. + active_route_declared_limits: Option, active_route_capabilities: codewhale_config::route::RouteCapabilities, rx_op: mpsc::Receiver, live_runtime_authority: Arc>, @@ -1667,6 +1674,7 @@ impl Engine { let provider_id = route.identity.exact_id; let model = route.model; let limits = crate::route_budget::known_route_limits(route.candidate.limits()); + let declared_limits = route.candidate.embedding_host_declared_limits(); let capabilities = route.candidate.capabilities(); let api_config = *route.config; let client = route.client; @@ -1676,6 +1684,7 @@ impl Engine { self.api_provider_id = provider_id; self.api_config = api_config; self.active_route_limits = limits; + self.active_route_declared_limits = declared_limits; self.active_route_limits_model = Some(model.clone()); self.active_route_capabilities = capabilities; self.api_key_env_only_recovery = Self::env_only_api_key_recovery_hint(&self.api_config); @@ -1708,6 +1717,7 @@ impl Engine { let provider_id = route.identity.exact_id; let model = route.model; let limits = crate::route_budget::known_route_limits(route.candidate.limits()); + let declared_limits = route.candidate.embedding_host_declared_limits(); let capabilities = route.candidate.capabilities(); let api_config = *route.config; let concrete_client = preflighted_client @@ -1719,6 +1729,7 @@ impl Engine { self.api_provider_id = provider_id; self.api_config = api_config; self.active_route_limits = limits; + self.active_route_declared_limits = declared_limits; self.active_route_limits_model = Some(model.clone()); self.active_route_capabilities = capabilities; self.api_key_env_only_recovery = Self::env_only_api_key_recovery_hint(&self.api_config); @@ -1753,30 +1764,226 @@ impl Engine { // wake, goal continuation) re-derive the route here, and a catalog-only // pass would drop the limits the installed route was host-declared // with — an uncatalogued model would clamp back to the compat output - // cap. Re-state the installed limits as host declarations so they - // survive the re-derivation, under three guards: the limits must - // have been resolved for exactly this model (a `SyncSession` swap - // must not carry another conversation's budget), the fresh resolution - // must land on the same wire model (a drifted resolution must resolve - // its own catalog envelope), and it must actually disagree with the + // cap. Re-state the host-declared subset so it survives the + // re-derivation, under three guards: the limits must have been + // resolved for exactly this model (a `SyncSession` swap must not + // carry another conversation's budget), the fresh resolution must + // land on the same wire model (a drifted resolution must resolve its + // own catalog envelope), and it must actually disagree with the // installed limits (an equal candidate already carries the right // facts, and re-stamping them would only relabel catalog values as - // host overrides). + // host overrides). Only the declared subset rides — the effective + // triple also contains catalog, operator-config, and roster facts + // that must keep refreshing from the live config instead of being + // pinned stale under host provenance. if route.model == self.session.model && self.active_route_limits_model.as_deref() == Some(selector) && let Some(installed) = self.active_route_limits && route.candidate.limits() != installed { + let Some(mut declared) = self.active_route_declared_limits else { + return Ok(route); + }; + // Declarations are stored verbatim from embedder and op payloads; + // degrade out-of-range values the way every other limit read + // point does instead of failing the self-started turn. + declared.context_tokens = declared + .context_tokens + .filter(|tokens| *tokens > 0 && *tokens <= u64::from(u32::MAX)); + if !declared.has_known_limit() { + return Ok(route); + } return resolve_runtime_route_for_identity_with_limits( &config, &identity, Some(selector), - Some(installed), + Some(declared), ); } Ok(route) } + /// Install a synced conversation: identity, history, prompts, workspace, + /// and mode. The route state (`active_route_limits`, + /// `active_route_limits_model`, `active_route_declared_limits`) is + /// deliberately untouched — the op carries no executable route, and a + /// budget declared for the previous conversation's model must not adopt + /// the restored one (see the guards in `current_runtime_route`). + async fn apply_synced_session( + &mut self, + session_id: Option, + messages: Vec, + system_prompt: Option, + system_prompt_override: bool, + model: String, + workspace: PathBuf, + workspace_roots: Vec, + mode: AppMode, + ) { + self.drop_all_steers().await; + // Deferred tool activations belong to one + // conversation. SyncSession installs a conversation's + // identity, history, and workspace (including the + // generated-ID new-session path), so never carry the + // previous conversation's toolbox across this edge — + // including a parked eviction note whose batch never + // recorded a result. + self.session.tool_activation_cache.clear(); + self.session.pending_toolbox_eviction_notes.clear(); + let plugin_workspace_changed = self.plugin_registry.workspace() != workspace.as_path(); + let previous_session_id = self.session.id.clone(); + let next_session_id = if let Some(session_id) = session_id { + session_id + } else if messages.is_empty() && system_prompt.is_none() { + uuid::Uuid::new_v4().to_string() + } else { + previous_session_id.clone() + }; + let closed_session_id = self.install_synced_session_id(next_session_id); + // SyncSession installs a conversation's identity; an id + // change IS a conversation boundary in this runtime — + // callers must pass their own conversation id for + // same-conversation re-syncs. A boundary in the same + // process does not rebuild the sub-agent manager, so the + // previous conversation's live children and write claims + // must be finalized here or they keep gating writers in + // the new conversation (#5372). Same-session reloads + // keep their id and are deliberately left untouched. + if let Some(closed_session_id) = closed_session_id { + let finalized = self + .subagent_manager + .write() + .await + .finalize_session_close_for_session(&closed_session_id); + if finalized > 0 { + tracing::info!( + target: "subagent", + finalized, + "finalized sub-agent fleet for closed session" + ); + } + } + let compaction_checkpoint = extract_compaction_summary_prompt(system_prompt.clone()); + let restored_messages = crate::runtime_handoff::project_messages_for_restore(&messages); + // The persisted carrier is authoritative for the one + // history checkpoint, at its saved location. Later + // turns must remain after it for Chat wire ordering. + let restored_messages = crate::compaction::restore_compaction_checkpoint( + restored_messages, + compaction_checkpoint.as_ref(), + ); + self.session.messages = restored_messages.into(); + // Direct field assignment bypasses `add_message` / + // `replace_messages`, which own the messages-revision + // bump the token-estimate cache keys on (#perf-r5). + // Without this bump the first estimate after a + // session restore is computed against whatever + // history revision was current before the sync — a + // stale number can flow into capacity checkpoints. + self.session.bump_messages_revision(); + self.session.latest_parent_input_tokens = None; + self.session.compaction_summary_prompt = compaction_checkpoint; + self.session.system_prompt = + crate::compaction::strip_compaction_summaries(system_prompt.as_ref()); + self.session.last_system_prompt_hash = + Some(system_prompt_hash(self.session.system_prompt.as_ref())); + // Prompt pins and drift baselines describe the + // conversation that was active before this sync. The + // next submitted turn must establish the installed + // conversation's own full prefix instead of comparing + // it with that stale baseline and emitting a + // `` from an empty/restored prompt. + // Host-owned overrides remain byte-stable because the + // refresh path exits early while the override is set. + self.session.pinned_prompt_context = None; + self.session.context_update_baseline = None; + // A session sync installs a new (or restored) prefix. + // Declare it so the next request re-pins the KV-cache + // prefix under a logged `resume` reason instead of + // reporting undeclared drift. + self.session.pending_prefix_change_reason = Some("resume".to_string()); + // Host-supplied prompts are persisted prefixes. Keep them + // byte-stable; mode/runtime state is projected per request. + self.session.system_prompt_override = + system_prompt_override && self.session.system_prompt.is_some(); + self.session.auto_model = model.trim().eq_ignore_ascii_case("auto"); + self.session.model = model; + self.session.workspace = workspace.clone(); + self.session.workspace_roots = + codewhale_core::normalize_workspace_roots(&workspace, &workspace_roots); + self.current_mode = mode; + self.config.model.clone_from(&self.session.model); + self.config.workspace = workspace.clone(); + self.config.workspace_roots = self.session.workspace_roots.clone(); + if plugin_workspace_changed { + self.plugin_registry = self.plugin_registry.rediscover_for_workspace(&workspace); + self.config.plugin_registry = Some(Arc::clone(&self.plugin_registry)); + // A pool may contain plugin servers and authority + // receipts from the previous workspace snapshot. + // Drop the receipts with it, or the briefing + // reconcile would re-announce the previous + // workspace's failures into the new conversation. + self.mcp_pool = None; + self.mcp_connection_errors.clear(); + // The previous workspace's in-flight connect pass + // must not finish into the freshly synced + // conversation (see the method). + self.invalidate_mcp_boot_for_workspace_change(); + } + // Base reloads the project context on every session + // sync; keep that unconditional so mid-session + // instruction edits are picked up on same-workspace + // re-syncs. The loader reads the primary root only. + let ctx = crate::project_context::load_project_context_with_parents(&workspace); + self.session.project_context = if ctx.has_instructions() { + Some(ctx) + } else { + None + }; + self.session.rebuild_working_set(); + self.reconcile_restored_work_bindings().await; + // The MCP briefing bookkeeping belongs to one + // conversation; rebuild it for the one just installed + // (see the reconcile method for why this must happen + // after the restored history lands, not before). + self.reconcile_mcp_boot_briefing_after_session_sync().await; + self.emit_session_updated().await; + let _ = self + .tx_event + .send(Event::status("Session context synced".to_string())) + .await; + } + + /// Apply an `Op::SetModel` payload: swap the model and reinstall the + /// route state the host resolved for it. The op payload is both the + /// effective and the declared value because no executable route + /// candidate arrives with the op. + async fn apply_set_model( + &mut self, + model: String, + route_limits: Option, + ) { + self.session.auto_model = model.trim().eq_ignore_ascii_case("auto"); + self.session.model = model; + self.config.model.clone_from(&self.session.model); + self.active_route_limits = route_limits; + self.active_route_declared_limits = route_limits; + self.active_route_limits_model = Some(self.session.model.clone()); + // This lightweight operation carries no executable + // route candidate, so old provider/model capability + // facts must not bleed into the new model. + self.active_route_capabilities = codewhale_config::route::RouteCapabilities::default(); + self.refresh_system_prompt_with_reason("model"); + self.emit_session_updated().await; + let _ = self + .tx_event + .send(Event::status(format!( + "Model set to: {}", + self.session.model + ))) + .await; + } + /// Create a new engine with the given configuration pub fn new(mut config: EngineConfig, api_config: &Config) -> (Self, EngineHandle) { crate::tls::ensure_rustls_crypto_provider(); @@ -2026,6 +2233,8 @@ impl Engine { }; let active_route_limits = config.active_route_limits; + // The embedder-supplied value is a declaration, not a resolver merge. + let active_route_declared_limits = active_route_limits; let active_route_limits_model = active_route_limits.map(|_| config.model.clone()); let active_turn_tool_security = config.turn_tool_security.clone(); let control_plane_restricted = active_turn_tool_security.is_some(); @@ -2073,6 +2282,7 @@ impl Engine { api_provider_id, active_route_limits, active_route_limits_model, + active_route_declared_limits, active_route_capabilities: codewhale_config::route::RouteCapabilities::default(), rx_op, live_runtime_authority: Arc::clone(&live_runtime_authority), @@ -3355,25 +3565,7 @@ impl Engine { mode: _, route_limits, } => { - self.session.auto_model = model.trim().eq_ignore_ascii_case("auto"); - self.session.model = model; - self.config.model.clone_from(&self.session.model); - self.active_route_limits = route_limits; - self.active_route_limits_model = Some(self.session.model.clone()); - // This lightweight operation carries no executable - // route candidate, so old provider/model capability - // facts must not bleed into the new model. - self.active_route_capabilities = - codewhale_config::route::RouteCapabilities::default(); - self.refresh_system_prompt_with_reason("model"); - self.emit_session_updated().await; - let _ = self - .tx_event - .send(Event::status(format!( - "Model set to: {}", - self.session.model - ))) - .await; + self.apply_set_model(model, route_limits).await; } Op::SetCompaction { config } => { let enabled = config.enabled; @@ -3455,143 +3647,17 @@ impl Engine { workspace_roots, mode, } => { - self.drop_all_steers().await; - // Deferred tool activations belong to one - // conversation. SyncSession installs a conversation's - // identity, history, and workspace (including the - // generated-ID new-session path), so never carry the - // previous conversation's toolbox across this edge — - // including a parked eviction note whose batch never - // recorded a result. - self.session.tool_activation_cache.clear(); - self.session.pending_toolbox_eviction_notes.clear(); - let plugin_workspace_changed = - self.plugin_registry.workspace() != workspace.as_path(); - let previous_session_id = self.session.id.clone(); - let next_session_id = if let Some(session_id) = session_id { - session_id - } else if messages.is_empty() && system_prompt.is_none() { - uuid::Uuid::new_v4().to_string() - } else { - previous_session_id.clone() - }; - let closed_session_id = self.install_synced_session_id(next_session_id); - // SyncSession installs a conversation's identity; an id - // change IS a conversation boundary in this runtime — - // callers must pass their own conversation id for - // same-conversation re-syncs. A boundary in the same - // process does not rebuild the sub-agent manager, so the - // previous conversation's live children and write claims - // must be finalized here or they keep gating writers in - // the new conversation (#5372). Same-session reloads - // keep their id and are deliberately left untouched. - if let Some(closed_session_id) = closed_session_id { - let finalized = self - .subagent_manager - .write() - .await - .finalize_session_close_for_session(&closed_session_id); - if finalized > 0 { - tracing::info!( - target: "subagent", - finalized, - "finalized sub-agent fleet for closed session" - ); - } - } - let compaction_checkpoint = - extract_compaction_summary_prompt(system_prompt.clone()); - let restored_messages = - crate::runtime_handoff::project_messages_for_restore(&messages); - // The persisted carrier is authoritative for the one - // history checkpoint, at its saved location. Later - // turns must remain after it for Chat wire ordering. - let restored_messages = crate::compaction::restore_compaction_checkpoint( - restored_messages, - compaction_checkpoint.as_ref(), - ); - self.session.messages = restored_messages.into(); - // Direct field assignment bypasses `add_message` / - // `replace_messages`, which own the messages-revision - // bump the token-estimate cache keys on (#perf-r5). - // Without this bump the first estimate after a - // session restore is computed against whatever - // history revision was current before the sync — a - // stale number can flow into capacity checkpoints. - self.session.bump_messages_revision(); - self.session.latest_parent_input_tokens = None; - self.session.compaction_summary_prompt = compaction_checkpoint; - self.session.system_prompt = - crate::compaction::strip_compaction_summaries(system_prompt.as_ref()); - self.session.last_system_prompt_hash = - Some(system_prompt_hash(self.session.system_prompt.as_ref())); - // Prompt pins and drift baselines describe the - // conversation that was active before this sync. The - // next submitted turn must establish the installed - // conversation's own full prefix instead of comparing - // it with that stale baseline and emitting a - // `` from an empty/restored prompt. - // Host-owned overrides remain byte-stable because the - // refresh path exits early while the override is set. - self.session.pinned_prompt_context = None; - self.session.context_update_baseline = None; - // A session sync installs a new (or restored) prefix. - // Declare it so the next request re-pins the KV-cache - // prefix under a logged `resume` reason instead of - // reporting undeclared drift. - self.session.pending_prefix_change_reason = Some("resume".to_string()); - // Host-supplied prompts are persisted prefixes. Keep them - // byte-stable; mode/runtime state is projected per request. - self.session.system_prompt_override = - system_prompt_override && self.session.system_prompt.is_some(); - self.session.auto_model = model.trim().eq_ignore_ascii_case("auto"); - self.session.model = model; - self.session.workspace = workspace.clone(); - self.session.workspace_roots = - codewhale_core::normalize_workspace_roots(&workspace, &workspace_roots); - self.current_mode = mode; - self.config.model.clone_from(&self.session.model); - self.config.workspace = workspace.clone(); - self.config.workspace_roots = self.session.workspace_roots.clone(); - if plugin_workspace_changed { - self.plugin_registry = - self.plugin_registry.rediscover_for_workspace(&workspace); - self.config.plugin_registry = Some(Arc::clone(&self.plugin_registry)); - // A pool may contain plugin servers and authority - // receipts from the previous workspace snapshot. - // Drop the receipts with it, or the briefing - // reconcile would re-announce the previous - // workspace's failures into the new conversation. - self.mcp_pool = None; - self.mcp_connection_errors.clear(); - // The previous workspace's in-flight connect pass - // must not finish into the freshly synced - // conversation (see the method). - self.invalidate_mcp_boot_for_workspace_change(); - } - // Base reloads the project context on every session - // sync; keep that unconditional so mid-session - // instruction edits are picked up on same-workspace - // re-syncs. The loader reads the primary root only. - let ctx = - crate::project_context::load_project_context_with_parents(&workspace); - self.session.project_context = if ctx.has_instructions() { - Some(ctx) - } else { - None - }; - self.session.rebuild_working_set(); - self.reconcile_restored_work_bindings().await; - // The MCP briefing bookkeeping belongs to one - // conversation; rebuild it for the one just installed - // (see the reconcile method for why this must happen - // after the restored history lands, not before). - self.reconcile_mcp_boot_briefing_after_session_sync().await; - self.emit_session_updated().await; - let _ = self - .tx_event - .send(Event::status("Session context synced".to_string())) - .await; + self.apply_synced_session( + session_id, + messages, + system_prompt, + system_prompt_override, + model, + workspace, + workspace_roots, + mode, + ) + .await; } Op::CompactContext { id, @@ -5463,6 +5529,7 @@ impl Engine { let provider_identity = route.identity.key.clone(); let model = route.model.clone(); let route_limits = crate::route_budget::known_route_limits(route.candidate.limits()); + let route_declared_limits = route.candidate.embedding_host_declared_limits(); let route_capabilities = route.candidate.capabilities(); let route_api_config = route.config.clone(); // Freeze the billing receipt here, while `route` is still the single @@ -5650,6 +5717,7 @@ impl Engine { // The model, limits, and compaction policy arrive in one operation so // no provider request can observe a partially updated route. self.active_route_limits = route_limits; + self.active_route_declared_limits = route_declared_limits; self.active_route_limits_model = Some(model.clone()); self.config.compaction = compaction; // Headless/runtime hosts supply their durable turn owner. Interactive From f036cf392d515dd251107f4bef2ff159766413c6 Mon Sep 17 00:00:00 2001 From: asto Date: Thu, 15 Oct 2026 16:07:27 +0800 Subject: [PATCH 2/9] test(tui): pin route limits lockstep contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five tests close the mutation gap the round-2 review of #108 measured: deleting any lockstep write outside `Engine::new` left the whole suite green while silently disabling the carry. Each test is verified red by a compiling targeted mutation of exactly the write it pins: - install_routes_rewrite_declared_limits_in_lockstep: a declared install records the declaration and the next re-derivation carries it; a catalog-only install clears the stale declaration and stays catalog-only. - injected_client_install_also_records_declared_limits: the injected-client branch writes the same pair. - set_model_installs_declaration_for_later_re_derivation: apply_set_model feeds both the declaration and the model spelling a later re-derivation needs. - sync_session_leaves_installed_route_limits_untouched: op-driven contract — the handler must not touch the installed route state. - current_runtime_route_sanitizes_out_of_range_declared_context: out-of-range declared context degrades to fresh instead of failing the turn, and the valid output budget still rides. The four #108 carry tests are unchanged and stay green. Signed-off-by: asto --- crates/tui/src/core/engine/tests.rs | 223 ++++++++++++++++++++++++++++ 1 file changed, 223 insertions(+) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index d8bfe86a8f..23ed536c13 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -3935,6 +3935,229 @@ fn current_runtime_route_does_not_carry_limits_across_model_drift() { ); } +#[test] +fn install_routes_rewrite_declared_limits_in_lockstep() { + // Every install must pair the effective limits with the host-declared + // subset: a declared install records the declaration (and the next + // re-derivation carries it), while a catalog-only install — a host turn + // whose route arrived without declarations — clears the stale one so + // catalog and operator facts keep refreshing instead of being pinned + // under host provenance. + let declared = codewhale_config::route::RouteLimits { + context_tokens: Some(2_000_000), + input_tokens: None, + output_tokens: Some(200_000), + }; + let config = goal_custom_route_config(); + let mut engine = engine_with_declared_local_model_limits(None); + + let declared_route = crate::route_runtime::resolve_runtime_route_with_limits( + &config, + ApiProvider::Custom, + Some("local-model"), + declared, + ) + .expect("resolve declared route"); + engine + .install_resolved_runtime_route(declared_route) + .expect("install declared route"); + assert_eq!(engine.active_route_declared_limits, Some(declared)); + + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert_eq!( + route.candidate.limits(), + declared, + "re-derivation must carry the host-declared subset the install recorded" + ); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .any(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost) + ); + + let catalog_route = crate::route_runtime::resolve_runtime_route( + &config, + ApiProvider::Custom, + Some("local-model"), + ) + .expect("resolve catalog route"); + engine + .install_resolved_runtime_route(catalog_route) + .expect("install catalog route"); + assert_eq!( + engine.active_route_declared_limits, None, + "a catalog-only install must clear the stale declaration" + ); + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .all(|item| item.source != codewhale_config::route::OverrideSource::EmbeddingHost), + "the re-derivation after a catalog-only install must stay catalog-only, got {:?}", + route.candidate.applied_limit_overrides() + ); +} + +#[test] +fn injected_client_install_also_records_declared_limits() { + // The injected-client branch of `install_resolved_runtime_route` writes + // the same lockstep pair as the validated branch; a re-derivation after + // its install must carry the declared budget too. + let declared = declared_local_model_limits(); + let config = goal_custom_route_config(); + let mut engine = engine_with_declared_local_model_limits(None); + engine.model_client_injected = true; + + let route = crate::route_runtime::resolve_runtime_route_with_limits( + &config, + ApiProvider::Custom, + Some("local-model"), + declared, + ) + .expect("resolve declared route"); + engine + .install_resolved_runtime_route(route) + .expect("install declared route"); + assert_eq!(engine.active_route_declared_limits, Some(declared)); + + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert_eq!( + route.candidate.limits(), + declared, + "the injected-branch install must feed the re-derivation's carry" + ); +} + +#[tokio::test] +async fn set_model_installs_declaration_for_later_re_derivation() { + // `Op::SetModel` carries the host's route payload without an executable + // candidate, so its lockstep writes are what let a later engine + // self-started turn still carry the declared budget. + let mut engine = engine_with_declared_local_model_limits(None); + let declared = declared_local_model_limits(); + + engine + .apply_set_model("local-model".to_string(), Some(declared)) + .await; + assert_eq!(engine.active_route_declared_limits, Some(declared)); + + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert_eq!( + route.candidate.limits(), + declared, + "the declaration installed by SetModel must survive re-derivation" + ); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .any(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost) + ); +} + +#[tokio::test] +async fn sync_session_leaves_installed_route_limits_untouched() { + // The op carries no route, so its handler must not touch the installed + // route state: a budget declared for the previous conversation's model + // must not adopt the restored one. + let declared = declared_local_model_limits(); + let mut engine = engine_with_declared_local_model_limits(Some(declared)); + let tmp = tempdir().expect("tempdir"); + + engine + .apply_synced_session( + None, + Vec::new(), + None, + false, + "restored-other-model".to_string(), + tmp.path().to_path_buf(), + Vec::new(), + AppMode::Agent, + ) + .await; + + assert_eq!(engine.session.model, "restored-other-model"); + assert_eq!( + engine.active_route_limits, + Some(declared), + "SyncSession must not touch the installed effective limits" + ); + assert_eq!( + engine.active_route_limits_model, + Some("local-model".to_string()), + "SyncSession must not retarget the installed limits' model" + ); + assert_eq!( + engine.active_route_declared_limits, + Some(declared), + "SyncSession must not touch the installed declaration" + ); + + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert_eq!(route.model, "restored-other-model"); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .all(|item| item.source != codewhale_config::route::OverrideSource::EmbeddingHost), + "the restored model's re-derivation must stay catalog-only, got {:?}", + route.candidate.applied_limit_overrides() + ); +} + +#[test] +fn current_runtime_route_sanitizes_out_of_range_declared_context() { + // Declarations are stored verbatim from embedder payloads; the carry + // must degrade out-of-range context values the way every other limit + // read point does instead of failing the self-started turn. + for context_tokens in [Some(0u64), Some(u64::from(u32::MAX) + 1)] { + let engine = + engine_with_declared_local_model_limits(Some(codewhale_config::route::RouteLimits { + context_tokens, + input_tokens: None, + output_tokens: Some(384_000), + })); + + let route = engine + .current_runtime_route() + .expect("out-of-range declared context must degrade, not fail the turn"); + assert_eq!( + route.candidate.limits().output_tokens, + Some(384_000), + "the valid declared output budget must still be carried" + ); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .all(|item| { + item.source != codewhale_config::route::OverrideSource::EmbeddingHost + || item.field != codewhale_config::route::LimitField::ContextTokens + }), + "the out-of-range declared context must not be re-stated, got {:?}", + route.candidate.applied_limit_overrides() + ); + } +} + #[tokio::test] async fn exhausted_goal_reaches_route_failure_without_budget_pause() { let config = goal_custom_route_config(); From 85be170848cf2750e3c5694f9ed28beccf83838b Mon Sep 17 00:00:00 2001 From: asto Date: Thu, 15 Oct 2026 14:52:31 +0000 Subject: [PATCH 3/9] fix(tui): preserve declarations on equal installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 review (comment 5532157364) left two MAJORs on the declared- limits carry. This closes the first: a value-equal re-install — a fresh candidate restating the exact installed triple with no EmbeddingHost provenance — silently retracted a standing declaration, so the next config drift made the carry bail and dropped the host's budget for the engine's lifetime. Under #108 the same sequence kept the values. Install paths now keep the standing declaration when the incoming effective triple and model are unchanged (declared_limits_for_install): only a value change, a model change, or a fresh declaration rewrites the pair. The turn-start restate in handle_send_message is deleted outright — it duplicated the install's values, was untestable by construction, and would have clobbered the preservation. Also: RouteLimits::MAX_WIRE_TOKENS names the (0, u32::MAX] wire rule the carry filter re-encodes; the carry and field docs now state the context-only sanitization and the ingest-as-declaration semantics honestly. Signed-off-by: asto --- crates/config/src/route/offering.rs | 5 ++ crates/tui/src/core/engine.rs | 79 ++++++++++++++++++++++------- 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/crates/config/src/route/offering.rs b/crates/config/src/route/offering.rs index 14e71ee356..ba2448b7bd 100644 --- a/crates/config/src/route/offering.rs +++ b/crates/config/src/route/offering.rs @@ -36,6 +36,11 @@ pub struct RouteLimits { } impl RouteLimits { + /// Highest token count representable on the provider wire. Wire budget + /// fields are u32, so a stored limit beyond this — or at zero — is + /// treated as unknown by the engine's range filters. + pub const MAX_WIRE_TOKENS: u64 = u32::MAX as u64; + /// Whether at least one limit fact is known. #[must_use] pub const fn has_known_limit(self) -> bool { diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 521f350745..28ec028c7b 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -290,8 +290,10 @@ impl std::fmt::Debug for ExtraTools { pub struct EngineConfig { /// Model identifier to use for responses. pub model: String, - /// Route/offering limits for the active provider+model, when the runtime - /// route resolver had concrete catalog facts. + /// Route/offering limits for the active provider+model. With no + /// executable candidate arriving alongside it, this value is stored as + /// the host's declaration for re-derivation carry: pass the budget the + /// host itself stands behind, not a resolver-merged receipt. pub active_route_limits: Option, /// Workspace root for tool execution and file operations. pub workspace: PathBuf, @@ -1169,11 +1171,15 @@ pub struct Engine { /// a re-derivation onto the restored one. active_route_limits_model: Option, /// The subset of `active_route_limits` the embedding host explicitly - /// declared, as opposed to the catalog-merged effective value. Written in - /// lockstep with the effective limits so a re-derivation can re-state - /// exactly the host's own budget: catalog, operator-config, and roster - /// facts must keep refreshing from the live config on engine-self-started - /// turns instead of being pinned stale under host provenance. + /// declared, as opposed to the catalog-merged effective value. A + /// re-derivation re-states exactly this subset so catalog, + /// operator-config, and roster facts keep refreshing from the live config + /// on engine-self-started turns instead of being pinned stale under host + /// provenance. Candidate-bearing installs derive it from the candidate's + /// `EmbeddingHost` provenance and keep a standing declaration across + /// value-equal re-installs (see `declared_limits_for_install`); where no + /// executable candidate exists — engine construction, `Op::SetModel` — + /// the payload itself is stored as the declaration. active_route_declared_limits: Option, active_route_capabilities: codewhale_config::route::RouteCapabilities, rx_op: mpsc::Receiver, @@ -1665,6 +1671,31 @@ impl Engine { format!("{message}\n\n{hint}") } + /// The declaration to install alongside an incoming effective triple. + /// + /// A value-preserving re-install does not retract a standing declaration: + /// fresh re-derivations routinely restate the exact installed triple with + /// no `EmbeddingHost` provenance, and clearing the declaration there would + /// silently drop the host's standing budget on the first such turn — the + /// stale-budget window this field exists to close. A changed effective + /// value, a different model, or an incoming candidate that does carry a + /// declaration always rewrites the pair. + fn declared_limits_for_install( + &self, + model: &str, + incoming_effective: Option, + incoming_declared: Option, + ) -> Option { + if incoming_declared.is_none() + && incoming_effective == self.active_route_limits + && self.active_route_limits_model.as_deref() == Some(model) + { + self.active_route_declared_limits + } else { + incoming_declared + } + } + /// Install a route that the host already resolved and client-preflighted. /// No identity guessing or config re-resolution is allowed at this /// boundary: the descriptor is the single authority for the turn. @@ -1679,12 +1710,13 @@ impl Engine { let api_config = *route.config; let client = route.client; + let preserved_declared = self.declared_limits_for_install(&model, limits, declared_limits); self.api_provider = provider; self.api_provider_identity = identity; self.api_provider_id = provider_id; self.api_config = api_config; self.active_route_limits = limits; - self.active_route_declared_limits = declared_limits; + self.active_route_declared_limits = preserved_declared; self.active_route_limits_model = Some(model.clone()); self.active_route_capabilities = capabilities; self.api_key_env_only_recovery = Self::env_only_api_key_recovery_hint(&self.api_config); @@ -1724,12 +1756,13 @@ impl Engine { .map(Ok) .unwrap_or_else(|| DeepSeekClient::from_candidate(&api_config, &route.candidate)); + let preserved_declared = self.declared_limits_for_install(&model, limits, declared_limits); self.api_provider = provider; self.api_provider_identity = identity; self.api_provider_id = provider_id; self.api_config = api_config; self.active_route_limits = limits; - self.active_route_declared_limits = declared_limits; + self.active_route_declared_limits = preserved_declared; self.active_route_limits_model = Some(model.clone()); self.active_route_capabilities = capabilities; self.api_key_env_only_recovery = Self::env_only_api_key_recovery_hint(&self.api_config); @@ -1784,12 +1817,16 @@ impl Engine { let Some(mut declared) = self.active_route_declared_limits else { return Ok(route); }; - // Declarations are stored verbatim from embedder and op payloads; - // degrade out-of-range values the way every other limit read - // point does instead of failing the self-started turn. - declared.context_tokens = declared - .context_tokens - .filter(|tokens| *tokens > 0 && *tokens <= u64::from(u32::MAX)); + // Declarations are stored verbatim from embedder and op payloads. + // An out-of-range context declaration would fail the re-resolution + // outright (`append_embedding_host_limit_overrides` rejects it) + // and with it the self-started turn, so degrade it to unknown the + // way every other context read point does. Input/output + // declarations cannot fail resolution and stay verbatim; the + // budget read points range-filter them. + declared.context_tokens = declared.context_tokens.filter(|tokens| { + *tokens > 0 && *tokens <= codewhale_config::route::RouteLimits::MAX_WIRE_TOKENS + }); if !declared.has_known_limit() { return Ok(route); } @@ -2233,7 +2270,10 @@ impl Engine { }; let active_route_limits = config.active_route_limits; - // The embedder-supplied value is a declaration, not a resolver merge. + // No executable candidate arrives at construction, so the + // embedder-supplied value is stored as the declaration itself; when + // the host resolved it through the resolver it may merge catalog + // facts, which this boundary cannot distinguish. let active_route_declared_limits = active_route_limits; let active_route_limits_model = active_route_limits.map(|_| config.model.clone()); let active_turn_tool_security = config.turn_tool_security.clone(); @@ -5529,7 +5569,6 @@ impl Engine { let provider_identity = route.identity.key.clone(); let model = route.model.clone(); let route_limits = crate::route_budget::known_route_limits(route.candidate.limits()); - let route_declared_limits = route.candidate.embedding_host_declared_limits(); let route_capabilities = route.candidate.capabilities(); let route_api_config = route.config.clone(); // Freeze the billing receipt here, while `route` is still the single @@ -5715,9 +5754,11 @@ impl Engine { // Apply the host-resolved route budget before building the request. // The model, limits, and compaction policy arrive in one operation so - // no provider request can observe a partially updated route. + // no provider request can observe a partially updated route. The + // declaration was already paired with these limits by the install + // above; restating it here would clobber a value-equal preservation + // (see `declared_limits_for_install`). self.active_route_limits = route_limits; - self.active_route_declared_limits = route_declared_limits; self.active_route_limits_model = Some(model.clone()); self.config.compaction = compaction; // Headless/runtime hosts supply their durable turn owner. Interactive From 21ee0990e2c2353e21077f49ca08663f17de23ce Mon Sep 17 00:00:00 2001 From: asto Date: Thu, 15 Oct 2026 14:53:02 +0000 Subject: [PATCH 4/9] test(tui): pin declared carry and fresh wins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three tests close the mutation gaps the round-1 review measured where every carry scenario had installed == declared, so reverting the carry to the whole installed triple — or resurrecting the #108 stale-pin for declaration-less routes — kept the entire suite green: - carry_restates_only_the_declared_subset: a strict-subset declaration (host output only, catalog context) carries exactly one EmbeddingHost entry; a whole-triple restatement mutant flips the provenance count. - re_derivation_without_declaration_lets_fresh_win: after a catalog-only install clears the declaration, a stale installed value must lose to the fresh resolution; the #108-fallback mutant flips both provenance and limits. - equal_install_keeps_standing_declaration: a value-equal install keeps the standing declaration and the preserved declaration still drives a later carry; the unconditional-overwrite mutant flips it. Also: direct unit tests for ReadyRouteCandidate:: embedding_host_declared_limits in its home crate (subset reconstruction and the no-host-entries gate), and assert messages on the two bare any() checks. The SetModel/SyncSession method tests now state honestly that they are method-level contracts — the op arm is a verbatim one-line delegation whose forwarding is pinned by the existing op-loop tests. Signed-off-by: asto --- crates/config/src/route/tests.rs | 61 +++++++ crates/tui/src/core/engine/tests.rs | 249 ++++++++++++++++++++++++++-- 2 files changed, 299 insertions(+), 11 deletions(-) diff --git a/crates/config/src/route/tests.rs b/crates/config/src/route/tests.rs index ad302b50a6..4366f8aec3 100644 --- a/crates/config/src/route/tests.rs +++ b/crates/config/src/route/tests.rs @@ -268,6 +268,67 @@ fn resolver_records_exact_limit_override_provenance() { assert_eq!(candidate.applied_limit_overrides(), overrides.as_slice()); } +#[test] +fn embedding_host_declared_limits_reconstructs_only_host_provenance() { + // Host entries reconstruct exactly, one per declared field; interleaved + // catalog/operator/roster provenance must not leak into the declared + // subset even when it targets the same field. + let mut request = req(Some(ProviderKind::Zai), Some("glm-5.2")); + request.limit_overrides = vec![ + SourcedLimitOverride { + field: LimitField::ContextTokens, + value: Some(262_144), + source: OverrideSource::UserContextWindow, + }, + SourcedLimitOverride { + field: LimitField::OutputTokens, + value: Some(65_536), + source: OverrideSource::EmbeddingHost, + }, + SourcedLimitOverride { + field: LimitField::ContextTokens, + value: Some(1_048_576), + source: OverrideSource::EmbeddingHost, + }, + ]; + + let candidate = models_dev_route_resolver() + .resolve(&request) + .expect("Models.dev-backed route with sourced overrides resolves"); + + // Effective keeps every override (later entries win per field). + assert_eq!(candidate.limits().context_tokens, Some(1_048_576)); + assert_eq!(candidate.limits().output_tokens, Some(65_536)); + + let declared = candidate + .embedding_host_declared_limits() + .expect("host entries exist"); + assert_eq!(declared.context_tokens, Some(1_048_576)); + assert_eq!(declared.input_tokens, None); + assert_eq!(declared.output_tokens, Some(65_536)); +} + +#[test] +fn embedding_host_declared_limits_without_host_entries_is_none() { + // A candidate whose overrides all come from other sources — or that + // carries no overrides at all — has no declared subset. + let mut request = req(Some(ProviderKind::Zai), Some("glm-5.2")); + request.limit_overrides = vec![SourcedLimitOverride { + field: LimitField::ContextTokens, + value: Some(262_144), + source: OverrideSource::UserContextWindow, + }]; + let candidate = models_dev_route_resolver() + .resolve(&request) + .expect("operator-overridden route resolves"); + assert_eq!(candidate.embedding_host_declared_limits(), None); + + let plain = models_dev_route_resolver() + .resolve(&req(Some(ProviderKind::Zai), Some("glm-5.2"))) + .expect("plain catalog route resolves"); + assert_eq!(plain.embedding_host_declared_limits(), None); +} + #[test] fn resolved_auth_source_existing_variants_keep_wire_compatibility() { let cases = [ diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 23ed536c13..0f40fa3fb4 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -3976,7 +3976,9 @@ fn install_routes_rewrite_declared_limits_in_lockstep() { .candidate .applied_limit_overrides() .iter() - .any(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost) + .any(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost), + "re-derivation must record host provenance, got {:?}", + route.candidate.applied_limit_overrides() ); let catalog_route = crate::route_runtime::resolve_runtime_route( @@ -4041,15 +4043,23 @@ fn injected_client_install_also_records_declared_limits() { #[tokio::test] async fn set_model_installs_declaration_for_later_re_derivation() { // `Op::SetModel` carries the host's route payload without an executable - // candidate, so its lockstep writes are what let a later engine - // self-started turn still carry the declared budget. + // candidate, so the apply method's lockstep writes are what let a later + // engine self-started turn still carry the declared budget. Method-level + // contract: the op arm is a verbatim one-line delegation whose + // model/mode forwarding is pinned by the op-loop `Op::SetModel` tests; + // the payload forwarding itself is not observable outside a full + // mock-client turn. let mut engine = engine_with_declared_local_model_limits(None); let declared = declared_local_model_limits(); engine .apply_set_model("local-model".to_string(), Some(declared)) .await; - assert_eq!(engine.active_route_declared_limits, Some(declared)); + assert_eq!( + engine.active_route_declared_limits, + Some(declared), + "the apply method must store the op payload as the declaration" + ); let route = engine .current_runtime_route() @@ -4064,15 +4074,20 @@ async fn set_model_installs_declaration_for_later_re_derivation() { .candidate .applied_limit_overrides() .iter() - .any(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost) + .any(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost), + "the carried budget must be re-stated as host declarations, got {:?}", + route.candidate.applied_limit_overrides() ); } #[tokio::test] async fn sync_session_leaves_installed_route_limits_untouched() { - // The op carries no route, so its handler must not touch the installed - // route state: a budget declared for the previous conversation's model - // must not adopt the restored one. + // The op carries no route, so its apply method must not touch the + // installed route state: a budget declared for the previous + // conversation's model must not adopt the restored one. Method-level + // contract: `apply_synced_session` is the verbatim body of the op arm + // (round-1 mechanical diff), and the arm's own forwarding is pinned by + // the op-loop `Op::SyncSession` tests. let declared = declared_local_model_limits(); let mut engine = engine_with_declared_local_model_limits(Some(declared)); let tmp = tempdir().expect("tempdir"); @@ -4124,9 +4139,10 @@ async fn sync_session_leaves_installed_route_limits_untouched() { #[test] fn current_runtime_route_sanitizes_out_of_range_declared_context() { - // Declarations are stored verbatim from embedder payloads; the carry - // must degrade out-of-range context values the way every other limit - // read point does instead of failing the self-started turn. + // Declarations are stored verbatim from embedder payloads; an + // out-of-range context value would fail the re-resolution outright, so + // the carry must degrade it to unknown instead of failing the + // self-started turn. for context_tokens in [Some(0u64), Some(u64::from(u32::MAX) + 1)] { let engine = engine_with_declared_local_model_limits(Some(codewhale_config::route::RouteLimits { @@ -4158,6 +4174,217 @@ fn current_runtime_route_sanitizes_out_of_range_declared_context() { } } +#[tokio::test] +async fn equal_install_keeps_standing_declaration() { + // A value-equal re-install — a fresh candidate that restates the exact + // installed triple with no host provenance — must not retract a standing + // declaration: the declared budget still describes the installed route, + // and clearing it there would drop the host's budget on the first equal + // re-derivation, before any value actually changed. The declaration only + // clears when the effective value or the model actually changes. + let config = Config { + provider: Some("deepseek".to_string()), + api_key: Some("local-test-key".to_string()), + ..Config::default() + }; + + let fresh = crate::route_runtime::resolve_runtime_route( + &config, + ApiProvider::Deepseek, + Some("deepseek-v4-flash"), + ) + .expect("resolve fresh route"); + let coincident = crate::route_budget::known_route_limits(fresh.candidate.limits()) + .expect("the catalog row carries known limits to re-declare"); + assert_ne!( + coincident.output_tokens, + Some(123_456), + "premise: the fresh resolution disagrees with the drifted value below" + ); + + let mut engine = engine_with_deepseek_flash_limits("deepseek-v4-flash", None); + // The host re-declares exactly what the catalog resolves, so the install + // below is value-equal to the standing declaration. + engine + .apply_set_model("deepseek-v4-flash".to_string(), Some(coincident)) + .await; + assert_eq!( + engine.active_route_declared_limits, + Some(coincident), + "premise: SetModel installed the declaration" + ); + + engine + .install_resolved_runtime_route(fresh) + .expect("install value-equal route"); + assert_eq!( + engine.active_route_limits, + Some(coincident), + "the install must keep the effective triple" + ); + assert_eq!( + engine.active_route_declared_limits, + Some(coincident), + "a value-equal install must keep the standing declaration, got {:?}", + engine.active_route_declared_limits + ); + + // The preserved declaration stays load-bearing: once the installed value + // drifts from a fresh resolution, the carry re-states it. + engine.active_route_limits = Some(codewhale_config::route::RouteLimits { + context_tokens: None, + input_tokens: None, + output_tokens: Some(123_456), + }); + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .any(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost), + "the preserved declaration must still drive the carry, got {:?}", + route.candidate.applied_limit_overrides() + ); +} + +#[test] +fn re_derivation_without_declaration_lets_fresh_win() { + // The stale-pin fix this PR makes: with no standing declaration (a + // catalog-only install cleared it), an installed/fresh disagreement must + // resolve fresh — catalog and operator facts keep refreshing — instead + // of re-stating the stale installed triple under host provenance. + let config = goal_custom_route_config(); + let mut engine = engine_with_declared_local_model_limits(None); + + let fresh = crate::route_runtime::resolve_runtime_route( + &config, + ApiProvider::Custom, + Some("local-model"), + ) + .expect("resolve fresh route"); + let fresh_limits = fresh.candidate.limits(); + assert_ne!( + fresh_limits.output_tokens, + Some(123_456), + "premise: the fresh resolution disagrees with the stale value below" + ); + engine + .install_resolved_runtime_route(fresh) + .expect("install catalog-only route"); + assert_eq!( + engine.active_route_declared_limits, None, + "premise: the catalog-only install cleared the declaration" + ); + + // A stale installed triple from an earlier config era disagrees with a + // fresh resolution while no declaration stands. + engine.active_route_limits = Some(codewhale_config::route::RouteLimits { + context_tokens: None, + input_tokens: None, + output_tokens: Some(123_456), + }); + + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .all(|item| item.source != codewhale_config::route::OverrideSource::EmbeddingHost), + "without a declaration the fresh resolution must win, got {:?}", + route.candidate.applied_limit_overrides() + ); + assert_eq!( + route.candidate.limits(), + fresh_limits, + "the fresh catalog triple must ride, not the stale installed value" + ); +} + +#[test] +fn carry_restates_only_the_declared_subset() { + // The carry must seed the re-derivation with the declared subset, not + // the whole installed triple: a strict-subset declaration (the host + // declares output only; the catalog supplies context) must carry exactly + // one `EmbeddingHost` entry. Re-stating the effective triple instead + // would relabel the catalog context as a host override — the + // stale-pinning behavior this PR replaces. + let config = Config { + provider: Some("deepseek".to_string()), + api_key: Some("local-test-key".to_string()), + ..Config::default() + }; + let control = crate::route_runtime::resolve_runtime_route( + &config, + ApiProvider::Deepseek, + Some("deepseek-v4-flash"), + ) + .expect("fresh route resolves"); + let catalog_limits = control.candidate.limits(); + let catalog_output = catalog_limits + .output_tokens + .expect("premise: the catalog row reports an output cap"); + assert!( + catalog_limits.context_tokens.is_some(), + "premise: the catalog row supplies context the declaration omits" + ); + let declared = codewhale_config::route::RouteLimits { + context_tokens: None, + input_tokens: None, + output_tokens: Some(catalog_output + 1_000), + }; + + let mut engine = engine_with_deepseek_flash_limits("deepseek-v4-flash", None); + let installed = crate::route_runtime::resolve_runtime_route_with_limits( + &config, + ApiProvider::Deepseek, + Some("deepseek-v4-flash"), + declared, + ) + .expect("resolve declared route"); + engine + .install_resolved_runtime_route(installed) + .expect("install declared route"); + assert_eq!(engine.active_route_declared_limits, Some(declared)); + assert_ne!( + engine.active_route_limits, + Some(declared), + "premise: the declaration is a strict subset of the installed triple" + ); + + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert_eq!(route.model, "deepseek-v4-flash"); + let host_entries: Vec<_> = route + .candidate + .applied_limit_overrides() + .iter() + .filter(|item| item.source == codewhale_config::route::OverrideSource::EmbeddingHost) + .collect(); + assert_eq!( + host_entries.len(), + 1, + "only the declared field must ride as a host override, got {:?}", + route.candidate.applied_limit_overrides() + ); + assert_eq!( + host_entries[0].field, + codewhale_config::route::LimitField::OutputTokens + ); + assert_eq!(host_entries[0].value, Some(catalog_output + 1_000)); + assert_eq!( + route.candidate.limits().context_tokens, + catalog_limits.context_tokens, + "the undeclared context must come from the fresh catalog resolution, not a host restatement" + ); +} + #[tokio::test] async fn exhausted_goal_reaches_route_failure_without_budget_pause() { let config = goal_custom_route_config(); From 07648dd3947f482f8be7d5de9e3ddbad146e89c6 Mon Sep 17 00:00:00 2001 From: asto Date: Fri, 16 Oct 2026 03:50:10 +0000 Subject: [PATCH 5/9] docs(config): state the declared-limits getter's None-value assumption Round-2 review of the declared-limits carry (comment 6207253823) noted that `SourcedLimitOverride` can express an explicit clear as a `None`-valued entry, which the reconstruction would silently report as not declared. No writer produces one today, so state that assumption on the getter instead of leaving it implicit. Signed-off-by: asto --- crates/config/src/route/candidate.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/config/src/route/candidate.rs b/crates/config/src/route/candidate.rs index 36ed481bcc..85b60a5a8b 100644 --- a/crates/config/src/route/candidate.rs +++ b/crates/config/src/route/candidate.rs @@ -331,6 +331,10 @@ impl ReadyRouteCandidate { /// provenance. Catalog-, operator-config-, and roster-derived facts are /// absent, so re-stating the result on a later re-derivation cannot pin /// their stale values under host provenance. + /// + /// A `None`-valued host entry — an explicit clear, which no writer + /// produces today — is reconstructed as not declared and cannot ride + /// the carry. #[must_use] pub fn embedding_host_declared_limits(&self) -> Option { let mut declared = RouteLimits::default(); From d2814151c7c8415f631220088c553b0d1ab75f4a Mon Sep 17 00:00:00 2001 From: asto Date: Fri, 16 Oct 2026 03:50:10 +0000 Subject: [PATCH 6/9] fix(tui): log standing route-declaration replacements on install The declared-limits carry had no observability at its two silent rewrite points: the round-2 review (comment 6207253823) could only distinguish preserve-from-clear by mutation. Log at debug when an install replaces a non-None standing declaration with a different value; preserves and declaration-less engines stay silent, so steady-state installs do not log. Signed-off-by: asto --- crates/tui/src/core/engine.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 28ec028c7b..0880503636 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -1686,14 +1686,27 @@ impl Engine { incoming_effective: Option, incoming_declared: Option, ) -> Option { - if incoming_declared.is_none() + let preserve = incoming_declared.is_none() && incoming_effective == self.active_route_limits - && self.active_route_limits_model.as_deref() == Some(model) - { + && self.active_route_limits_model.as_deref() == Some(model); + let next = if preserve { self.active_route_declared_limits } else { incoming_declared + }; + // Steady-state installs — preserves and declaration-less engines — + // stay silent; a standing declaration only actually changes here, + // and nothing else on the wire records that fact. + if self.active_route_declared_limits.is_some() && next != self.active_route_declared_limits + { + tracing::debug!( + model, + from = ?self.active_route_declared_limits, + to = ?next, + "install replaced the standing route declaration" + ); } + next } /// Install a route that the host already resolved and client-preflighted. From 57c59acc2d8a2b34298f9d01e79ae8c920f31fb2 Mon Sep 17 00:00:00 2001 From: asto Date: Fri, 16 Oct 2026 17:39:30 +0800 Subject: [PATCH 7/9] fix(tui): gate declaration preserve on route identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The declaration-preserve condition compared only the wire model and the effective limits. Two named custom endpoints can resolve the same model onto the same triple, so a value-equal install onto endpoint B adopted endpoint A's standing declaration; B's later budget change was then overlaid by A's legacy values instead of resolving fresh — the exact stale-pin window this carry exists to close (JensenChen28's round-3 review, comment 4291936291). Compare the full configured identity (provider enum, exact key, exact id) alongside value and model; the preserve now only bridges fresh re-derivations of the same route. Signed-off-by: asto --- crates/tui/src/core/engine.rs | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 0880503636..93e93c00b1 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -28,7 +28,9 @@ use tokio_util::sync::CancellationToken; use crate::approval_log::ApprovalReceiptStore; use crate::client::DeepSeekClient; use crate::compaction::{CompactionConfig, PreparedCompactionEnvelope, compact_messages_safe}; -use crate::config::{ApiProvider, Config, DEFAULT_MAX_SUBAGENTS, DEFAULT_TEXT_MODEL}; +use crate::config::{ + ApiProvider, Config, DEFAULT_MAX_SUBAGENTS, DEFAULT_TEXT_MODEL, ProviderIdentity, +}; use crate::core::model_client::SharedModelClient; use crate::error_taxonomy::{ErrorCategory, ErrorEnvelope, ErrorSeverity, StreamError}; use crate::features::{Feature, Features}; @@ -1678,17 +1680,26 @@ impl Engine { /// no `EmbeddingHost` provenance, and clearing the declaration there would /// silently drop the host's standing budget on the first such turn — the /// stale-budget window this field exists to close. A changed effective - /// value, a different model, or an incoming candidate that does carry a - /// declaration always rewrites the pair. + /// value, a different model, a different configured route, or an incoming + /// candidate that does carry a declaration always rewrites the pair. fn declared_limits_for_install( &self, + incoming_identity: &ProviderIdentity, model: &str, incoming_effective: Option, incoming_declared: Option, ) -> Option { + // The declaration describes one configured route. Two named custom + // endpoints can resolve the same wire model onto the same effective + // triple, so value and model equality alone would attribute route A's + // budget to route B — and B's later drift would be overlaid by A's + // legacy declaration instead of resolving fresh. let preserve = incoming_declared.is_none() && incoming_effective == self.active_route_limits - && self.active_route_limits_model.as_deref() == Some(model); + && self.active_route_limits_model.as_deref() == Some(model) + && self.api_provider == incoming_identity.provider + && self.api_provider_identity == incoming_identity.key + && self.api_provider_id == incoming_identity.exact_id; let next = if preserve { self.active_route_declared_limits } else { @@ -1714,16 +1725,17 @@ impl Engine { /// boundary: the descriptor is the single authority for the turn. fn install_validated_runtime_route(&mut self, route: ValidatedRuntimeRoute) { let provider = route.identity.provider; - let identity = route.identity.key; - let provider_id = route.identity.exact_id; let model = route.model; let limits = crate::route_budget::known_route_limits(route.candidate.limits()); let declared_limits = route.candidate.embedding_host_declared_limits(); let capabilities = route.candidate.capabilities(); let api_config = *route.config; let client = route.client; + let preserved_declared = + self.declared_limits_for_install(&route.identity, &model, limits, declared_limits); + let identity = route.identity.key; + let provider_id = route.identity.exact_id; - let preserved_declared = self.declared_limits_for_install(&model, limits, declared_limits); self.api_provider = provider; self.api_provider_identity = identity; self.api_provider_id = provider_id; @@ -1758,8 +1770,6 @@ impl Engine { let preflighted_client = route.take_preflighted_client(); let provider = route.identity.provider; - let identity = route.identity.key; - let provider_id = route.identity.exact_id; let model = route.model; let limits = crate::route_budget::known_route_limits(route.candidate.limits()); let declared_limits = route.candidate.embedding_host_declared_limits(); @@ -1769,7 +1779,11 @@ impl Engine { .map(Ok) .unwrap_or_else(|| DeepSeekClient::from_candidate(&api_config, &route.candidate)); - let preserved_declared = self.declared_limits_for_install(&model, limits, declared_limits); + let preserved_declared = + self.declared_limits_for_install(&route.identity, &model, limits, declared_limits); + let identity = route.identity.key; + let provider_id = route.identity.exact_id; + self.api_provider = provider; self.api_provider_identity = identity; self.api_provider_id = provider_id; From 6d011bad145acba734c4c1617d35924a75a5306b Mon Sep 17 00:00:00 2001 From: asto Date: Fri, 16 Oct 2026 17:40:07 +0800 Subject: [PATCH 8/9] test(tui): pin declaration identity boundary Two regression tests pin the route-identity guard: a value-equal install onto a second named endpoint must clear the standing declaration on the validated install path, and the subsequent operator budget drop must resolve fresh instead of being overlaid by the first endpoint's legacy declaration. The injected-client install branch pins the same boundary. Both mutations that drop the identity conjunct turn the new tests red. Also pin the carry's inclusive context bound: exactly u32::MAX is wire-representable and must ride verbatim; tightening the filter to an exclusive bound turns the new case red. Signed-off-by: asto --- crates/tui/src/core/engine/tests.rs | 175 ++++++++++++++++++++++++++++ 1 file changed, 175 insertions(+) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 0f40fa3fb4..b5d942b5f9 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -4172,6 +4172,24 @@ fn current_runtime_route_sanitizes_out_of_range_declared_context() { route.candidate.applied_limit_overrides() ); } + + // The inclusive upper bound: exactly `u32::MAX` is wire-representable + // and must ride verbatim, not be degraded by an off-by-one bound. + let engine = + engine_with_declared_local_model_limits(Some(codewhale_config::route::RouteLimits { + context_tokens: Some(u64::from(u32::MAX)), + input_tokens: None, + output_tokens: None, + })); + let route = engine + .current_runtime_route() + .expect("max representable declared context must resolve"); + assert_eq!( + route.candidate.limits().context_tokens, + Some(u64::from(u32::MAX)), + "exactly u32::MAX is in range and must be carried, got {:?}", + route.candidate.limits() + ); } #[tokio::test] @@ -4250,6 +4268,163 @@ async fn equal_install_keeps_standing_declaration() { ); } +/// Two named custom endpoints resolving the same wire model: the A→B +/// value-equal install boundary for route declarations. Both tables carry the +/// same operator context window, so the plain resolution has a known shared +/// triple. +fn shared_model_named_endpoint_config(provider: &str) -> Config { + let mut custom = HashMap::new(); + for (name, port) in [("custom-a", 18181u16), ("custom-b", 18182)] { + custom.insert( + name.to_string(), + crate::config::ProviderConfig { + kind: Some("openai-compatible".to_string()), + base_url: Some(format!("http://127.0.0.1:{port}/v1")), + model: Some("shared-model".to_string()), + api_key: Some("local-test-key".to_string()), + context_window: Some(100_000), + ..crate::config::ProviderConfig::default() + }, + ); + } + Config { + provider: Some(provider.to_string()), + providers: Some(crate::config::ProvidersConfig { + custom, + ..crate::config::ProvidersConfig::default() + }), + ..Config::default() + } +} + +fn shared_model_endpoint_effective_limits() -> codewhale_config::route::RouteLimits { + let fresh = crate::route_runtime::resolve_runtime_route( + &shared_model_named_endpoint_config("custom-b"), + ApiProvider::Custom, + Some("shared-model"), + ) + .expect("resolve the shared-model endpoint"); + crate::route_budget::known_route_limits(fresh.candidate.limits()) + .expect("the shared model resolves known limits") +} + +#[test] +fn equal_value_install_on_new_named_endpoint_does_not_inherit_declaration() { + // Two named custom endpoints can resolve the same wire model onto the + // same effective triple. A standing declaration describes endpoint A's + // route; endpoint B's value-equal declaration-less install must not + // adopt it, or B's later budget drop would be overlaid by A's legacy + // declaration instead of resolving fresh. + let shared = shared_model_endpoint_effective_limits(); + assert_eq!( + shared.context_tokens, + Some(100_000), + "premise: the operator window supplies the shared known triple" + ); + + let engine_config = EngineConfig { + model: "shared-model".to_string(), + snapshots_enabled: false, + terminal_chrome_enabled: false, + active_route_limits: Some(shared), + ..EngineConfig::default() + }; + let (mut engine, _handle) = Engine::new( + engine_config, + &shared_model_named_endpoint_config("custom-a"), + ); + assert_eq!(engine.api_provider_identity, "custom-a"); + assert_eq!( + engine.active_route_declared_limits, + Some(shared), + "premise: construction installed A's declaration" + ); + + let b_route = crate::route_runtime::resolve_runtime_route( + &shared_model_named_endpoint_config("custom-b"), + ApiProvider::Custom, + Some("shared-model"), + ) + .expect("resolve endpoint B") + .validate() + .expect("preflight endpoint B"); + engine.install_validated_runtime_route(b_route); + assert_eq!(engine.api_provider_identity, "custom-b"); + assert_eq!( + engine.active_route_declared_limits, None, + "a value-equal install on another named endpoint must clear the declaration, got {:?}", + engine.active_route_declared_limits + ); + + // Endpoint B's operator config drops the context window; the next + // self-started re-derivation must follow the fresh resolution instead of + // overlaying A's legacy declaration. + engine + .api_config + .provider_config_for_mut(ApiProvider::Custom) + .context_window = Some(10_000); + let route = engine + .current_runtime_route() + .expect("re-derived route resolves"); + assert_eq!( + route.candidate.limits().context_tokens, + Some(10_000), + "the fresh operator window must win, got {:?}", + route.candidate.limits() + ); + assert!( + route + .candidate + .applied_limit_overrides() + .iter() + .all(|item| item.source != codewhale_config::route::OverrideSource::EmbeddingHost), + "no declaration may cross the endpoint boundary, got {:?}", + route.candidate.applied_limit_overrides() + ); +} + +#[test] +fn injected_install_on_new_named_endpoint_also_clears_declaration() { + // The injected-client branch of `install_resolved_runtime_route` enforces + // the same identity boundary as the validated branch: a value-equal + // install onto another named endpoint must not adopt the standing + // declaration. + let shared = shared_model_endpoint_effective_limits(); + let engine_config = EngineConfig { + model: "shared-model".to_string(), + snapshots_enabled: false, + terminal_chrome_enabled: false, + active_route_limits: Some(shared), + ..EngineConfig::default() + }; + let (mut engine, _handle) = Engine::new( + engine_config, + &shared_model_named_endpoint_config("custom-a"), + ); + engine.model_client_injected = true; + assert_eq!( + engine.active_route_declared_limits, + Some(shared), + "premise: construction installed A's declaration" + ); + + let b_route = crate::route_runtime::resolve_runtime_route( + &shared_model_named_endpoint_config("custom-b"), + ApiProvider::Custom, + Some("shared-model"), + ) + .expect("resolve endpoint B"); + engine + .install_resolved_runtime_route(b_route) + .expect("install endpoint B route"); + assert_eq!(engine.api_provider_identity, "custom-b"); + assert_eq!( + engine.active_route_declared_limits, None, + "the injected branch must clear the declaration across the endpoint boundary, got {:?}", + engine.active_route_declared_limits + ); +} + #[test] fn re_derivation_without_declaration_lets_fresh_win() { // The stale-pin fix this PR makes: with no standing declaration (a From bb4c3b363122205aff793575c523bee31af49096 Mon Sep 17 00:00:00 2001 From: asto Date: Fri, 16 Oct 2026 17:40:09 +0800 Subject: [PATCH 9/9] test(config): make provenance pin order-sensitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reconstruction pin placed the non-host context entry before the host entry, and both the getter and the resolver are per-field last-wins, so dropping the source filter left the test green — the fixture could not catch the mutation it claimed to pin. Move the operator entry after the host entry so the filter is load-bearing and the same test proves declared differs from effective on context. Signed-off-by: asto --- crates/config/src/route/tests.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/config/src/route/tests.rs b/crates/config/src/route/tests.rs index 4366f8aec3..51e71a4e8d 100644 --- a/crates/config/src/route/tests.rs +++ b/crates/config/src/route/tests.rs @@ -272,14 +272,10 @@ fn resolver_records_exact_limit_override_provenance() { fn embedding_host_declared_limits_reconstructs_only_host_provenance() { // Host entries reconstruct exactly, one per declared field; interleaved // catalog/operator/roster provenance must not leak into the declared - // subset even when it targets the same field. + // subset even when a later operator entry for the same field would win + // last-wins. let mut request = req(Some(ProviderKind::Zai), Some("glm-5.2")); request.limit_overrides = vec![ - SourcedLimitOverride { - field: LimitField::ContextTokens, - value: Some(262_144), - source: OverrideSource::UserContextWindow, - }, SourcedLimitOverride { field: LimitField::OutputTokens, value: Some(65_536), @@ -290,14 +286,21 @@ fn embedding_host_declared_limits_reconstructs_only_host_provenance() { value: Some(1_048_576), source: OverrideSource::EmbeddingHost, }, + SourcedLimitOverride { + field: LimitField::ContextTokens, + value: Some(262_144), + source: OverrideSource::UserContextWindow, + }, ]; let candidate = models_dev_route_resolver() .resolve(&request) .expect("Models.dev-backed route with sourced overrides resolves"); - // Effective keeps every override (later entries win per field). - assert_eq!(candidate.limits().context_tokens, Some(1_048_576)); + // Effective applies every override last-wins, so the later operator + // entry owns context: the declared subset genuinely differs from the + // effective triple on that field. + assert_eq!(candidate.limits().context_tokens, Some(262_144)); assert_eq!(candidate.limits().output_tokens, Some(65_536)); let declared = candidate