mirror of
https://github.com/block/buzz.git
synced 2026-10-31 14:31:02 +00:00
fix(pool): tri-state effort capability model survives live model switches
Replace the populate-once PoolEffortCapabilities cache + capabilities_ever_discovered
bool pair with a three-state EffortCapabilityState enum:
Unknown — no session has returned capabilities yet (pre-first-session window,
or capabilities reset after a model switch)
Supported { config_id, valid_values } — current model advertises thought_level
Unsupported — current model confirmed to not have thought_level
The old model populated capabilities once (on the first session/new) and never
refreshed them. After a live model switch — which resets agent.model_capabilities to
None to discard the pre-switch snapshot — return_agent wrote Unknown/kept stale state,
so handle_set_config_option_control kept validating against the old model's option set
and session_config_captured emitted pre-switch configOptions to Desktop.
Changes:
pool.rs:
- EffortCapabilityState enum replaces PoolEffortCapabilities + capabilities_ever_discovered
- return_agent derives tri-state from the returning worker's model_capabilities on
every return (not just the first), so the pool state always reflects the current model
- AgentPool initialises with Unknown; struct fields updated accordingly
pool.rs (create_session_and_apply_model):
- Capabilities are populated from EVERY session/new response, not just when None
- After a successful apply_model_switch, agent.model_capabilities is reset to None so
return_agent writes Unknown rather than stale-Supported
pool.rs (session_config_captured):
- On the switch path, configOptions emitted as null so Desktop clears its picker
state rather than caching pre-switch capability data
lib.rs (handle_set_config_option_control):
- Four-way A/B/C/D classifier replaced with a three-way tri-state dispatch
- Supported(config_id matches) → validate + store (case A)
- Supported(config_id no-match) → synthetic ok, non-effort (case B)
- Unsupported → reject with unsupported_model status (eliminates case-C
occupancy inference: capability loss is now surfaced immediately)
- Unknown → pre-discovery trust path: category=thought_level stores + pending_session
pool.rs field comment:
- last_acked_effort_gen comment updated to name resolve_effort_report as the sole
ack authority (Thufir Pass 3 non-blocking MINOR)
Tests (lib.rs control_result_tests):
- pool_with_capabilities_via_return helper drives the PRODUCTION return_agent path,
not the test-only notify_capabilities_discovered shortcut
- test_tristate_switch_effort_to_no_effort_rejects_pick: effort→no-effort switch
writes Unsupported; subsequent pick is rejected with unsupported_model status
- test_tristate_switch_no_effort_to_effort_accepts_pick: no-effort→effort switch
writes Supported with new configId; pick validates and stores correctly
- test_tristate_switch_new_options_validates_against_new_snapshot: both models
Supported with different option sets; stale value rejected, new value accepted
- test_tristate_unknown_after_switch_uses_pre_discovery_trust_path: capabilities=None
on return writes Unknown; pre-discovery category trust path remains active
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -1256,27 +1256,24 @@ fn handle_set_config_option_control(
|
||||
// advertised in session/new (agentConfigCore.ts uses the one from the
|
||||
// session cache via deferredUntilNativeOptionsAvailable resolution).
|
||||
//
|
||||
// V-2 fix: use pool-level capability cache instead of scanning idle agents.
|
||||
// Checked-out workers leave None slots; the cache is written at return_agent
|
||||
// V-2 fix: use pool-level capability state instead of scanning idle agents.
|
||||
// Checked-out workers leave None slots; the state is written at return_agent
|
||||
// once the first worker completes a session and returns to the pool.
|
||||
//
|
||||
// Four cases:
|
||||
// A. effort_capabilities.config_id is Some and matches → full validation path
|
||||
// B. effort_capabilities.config_id is None AND capabilities not yet discovered
|
||||
// AND no category trust → unknown configId → synthetic ok (non-effort)
|
||||
// C. effort_capabilities.config_id is None AND capabilities were discovered →
|
||||
// all workers are currently busy; trust the incoming configId from the
|
||||
// Desktop's session cache and store for apply at next checkout/session.
|
||||
// D. effort_capabilities.config_id is None AND capabilities not yet discovered
|
||||
// AND category == "thought_level" → pre-first-session window; Desktop
|
||||
// sends its session-cache configId with category as the trust signal.
|
||||
// Store the value and ack pending_session — the first session creation
|
||||
// will apply it and emit the honest final ok/failure.
|
||||
let thought_level_id = pool
|
||||
.effort_capabilities
|
||||
.config_id
|
||||
.as_deref()
|
||||
.map(str::to_string);
|
||||
// Three-state classifier (replaces the former case A/B/C/D four-way):
|
||||
// Supported(config_id, valid_values):
|
||||
// incoming configId matches → full I-7 validation path (case A).
|
||||
// incoming configId doesn't match → synthetic ok (non-effort, case B).
|
||||
// Unsupported:
|
||||
// current model confirmed to not have thought_level; reject the pick
|
||||
// immediately so Desktop surfaces the capability loss rather than
|
||||
// silently queuing a value that will never apply.
|
||||
// Unknown:
|
||||
// no session has returned capabilities yet (pre-first-session window,
|
||||
// or capabilities reset after a model switch pending the next session/new).
|
||||
// Trust the Desktop's category field as the trust signal (case D):
|
||||
// if category == "thought_level", store and ack pending_session —
|
||||
// the first session creation will apply it and emit the honest result.
|
||||
|
||||
// Category from the incoming frame (Desktop sends "thought_level" for all
|
||||
// effort picks and clears, including during the pre-discovery window).
|
||||
@@ -1285,51 +1282,75 @@ fn handle_set_config_option_control(
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("");
|
||||
|
||||
// Determine if this is a thought_level pick:
|
||||
// Case A: cache has a matching configId.
|
||||
// Case C: capabilities known from a prior session, all workers busy.
|
||||
// Case D: pre-first-session, Desktop asserts category = "thought_level".
|
||||
//
|
||||
// For the clear path (empty value), the same cases apply.
|
||||
let cache_matches = thought_level_id.as_deref() == Some(config_id);
|
||||
// Case C: capabilities were discovered before (so configId is known) but
|
||||
// workers are currently checked out and the cache is temporarily None.
|
||||
let all_busy_with_known_caps =
|
||||
pool.capabilities_ever_discovered && thought_level_id.is_none() && config_id != "unknown";
|
||||
// Case D: pre-first-session; Desktop sends category as the trust signal.
|
||||
let pre_discovery_trusted = !pool.capabilities_ever_discovered
|
||||
&& frame_category == "thought_level"
|
||||
&& config_id != "unknown";
|
||||
let is_thought_level = cache_matches || all_busy_with_known_caps || pre_discovery_trusted;
|
||||
|
||||
let (status, include_category) = if is_thought_level {
|
||||
// I-7: validate the incoming value against adapter-advertised options
|
||||
// from the pool-level capability cache. Works even when all workers
|
||||
// are checked out — the cache was written at the first session creation.
|
||||
// Skip validation when cache is empty (case C — workers busy, no cache
|
||||
// to validate against; trust the Desktop's session-cache configId).
|
||||
let valid_values = &pool.effort_capabilities.valid_values;
|
||||
|
||||
if !valid_values.is_empty()
|
||||
&& !value.is_empty()
|
||||
&& !valid_values.contains(&value.to_string())
|
||||
{
|
||||
// Value is not in the adapter's advertised option set.
|
||||
// Tri-state dispatch: Supported → validate + store, Unsupported → reject,
|
||||
// Unknown → pre-discovery trust path.
|
||||
use crate::pool::EffortCapabilityState;
|
||||
let (status, include_category) = match &pool.effort_capability_state {
|
||||
EffortCapabilityState::Supported {
|
||||
config_id: known_id,
|
||||
valid_values,
|
||||
} if known_id.as_str() == config_id => {
|
||||
// Case A: configId matches the pool's known supported configId.
|
||||
// I-7: validate value against adapter-advertised options.
|
||||
if !valid_values.is_empty()
|
||||
&& !value.is_empty()
|
||||
&& !valid_values.contains(&value.to_string())
|
||||
{
|
||||
tracing::warn!(
|
||||
target: "pool::effort",
|
||||
"effort value {value:?} not in advertised options {valid_values:?} — rejecting"
|
||||
);
|
||||
let mut ack = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": config_id,
|
||||
"status": "invalid_value",
|
||||
"value": value,
|
||||
});
|
||||
ack["category"] = serde_json::json!("thought_level");
|
||||
if let Some(ref n) = nonce {
|
||||
ack["nonce"] = serde_json::json!(n);
|
||||
}
|
||||
obs.emit(
|
||||
"control_result",
|
||||
None,
|
||||
&observer::ObserverContext::default(),
|
||||
ack,
|
||||
);
|
||||
return;
|
||||
}
|
||||
// Value is valid (or no validation possible for clears).
|
||||
if value.is_empty() {
|
||||
pool.pending_effort_nonce = nonce.clone();
|
||||
pool.clear_pool_effort();
|
||||
("pending_session", true)
|
||||
} else {
|
||||
pool.pending_effort_nonce = nonce.clone();
|
||||
let result = pool.set_pool_effort(config_id, value);
|
||||
match result {
|
||||
SetPoolEffortResult::Stored { .. } => ("pending_session", true),
|
||||
}
|
||||
}
|
||||
}
|
||||
EffortCapabilityState::Supported { .. } => {
|
||||
// Case B: state is Supported but configId doesn't match the incoming
|
||||
// frame — not an effort option; emit synthetic ok (no-op).
|
||||
("ok", false)
|
||||
}
|
||||
EffortCapabilityState::Unsupported => {
|
||||
// Current model does not support effort. Reject so Desktop can surface
|
||||
// the capability loss rather than silently queuing a value that will
|
||||
// never apply.
|
||||
tracing::warn!(
|
||||
target: "pool::effort",
|
||||
"effort value {value:?} not in advertised options {valid_values:?} — rejecting"
|
||||
"effort pick configId={config_id} rejected — current model is known-unsupported"
|
||||
);
|
||||
let mut ack = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": config_id,
|
||||
"status": "invalid_value",
|
||||
"status": "unsupported_model",
|
||||
"value": value,
|
||||
});
|
||||
ack["category"] = serde_json::json!("thought_level");
|
||||
// P2-1: echo the nonce on the early-return rejection path, matching
|
||||
// the behaviour of the common ack builder below. Without this the
|
||||
// Desktop's awaitEffortOutcome nonce guard rejects the ack, causing
|
||||
// an 8 s timeout instead of an immediate "invalid_value" outcome.
|
||||
if let Some(ref n) = nonce {
|
||||
ack["nonce"] = serde_json::json!(n);
|
||||
}
|
||||
@@ -1341,22 +1362,27 @@ fn handle_set_config_option_control(
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Empty value = clear (Auto). Bypass set_pool_effort.
|
||||
if value.is_empty() {
|
||||
pool.pending_effort_nonce = nonce.clone();
|
||||
pool.clear_pool_effort();
|
||||
("pending_session", true)
|
||||
} else {
|
||||
pool.pending_effort_nonce = nonce.clone();
|
||||
let result = pool.set_pool_effort(config_id, value);
|
||||
match result {
|
||||
SetPoolEffortResult::Stored { .. } => ("pending_session", true),
|
||||
EffortCapabilityState::Unknown => {
|
||||
// Pre-first-session window (or reset after a model switch pending
|
||||
// the next session/new). Trust the Desktop's category field as the
|
||||
// signal: if category == "thought_level", store for apply and ack
|
||||
// pending_session; otherwise synthetic ok (non-effort).
|
||||
if frame_category == "thought_level" && config_id != "unknown" {
|
||||
if value.is_empty() {
|
||||
pool.pending_effort_nonce = nonce.clone();
|
||||
pool.clear_pool_effort();
|
||||
("pending_session", true)
|
||||
} else {
|
||||
pool.pending_effort_nonce = nonce.clone();
|
||||
let result = pool.set_pool_effort(config_id, value);
|
||||
match result {
|
||||
SetPoolEffortResult::Stored { .. } => ("pending_session", true),
|
||||
}
|
||||
}
|
||||
} else {
|
||||
("ok", false)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Not a thought_level option — synthetic ok (no-op behaviour unchanged).
|
||||
("ok", false)
|
||||
};
|
||||
|
||||
// B5: include "category": "thought_level" ONLY on the real-forward branch.
|
||||
@@ -8046,6 +8072,7 @@ mod observer_payload_trim_tests {
|
||||
#[cfg(test)]
|
||||
mod control_result_tests {
|
||||
use super::*;
|
||||
use crate::pool::{AgentModelCapabilities, EffortCapabilityState};
|
||||
|
||||
// ── B5 harness-level tests for handle_set_config_option_control ──────────
|
||||
//
|
||||
@@ -8584,4 +8611,289 @@ mod control_result_tests {
|
||||
"synthetic ok ack must not carry category field"
|
||||
);
|
||||
}
|
||||
|
||||
// ── Tri-state switch-path regressions ─────────────────────────────────────
|
||||
|
||||
/// Helper: build a pool with an idle agent, drive capabilities via the real
|
||||
/// `return_agent` path (not the test-only `notify_capabilities_discovered`
|
||||
/// helper), and return the pool ready for further assertions.
|
||||
///
|
||||
/// `thought_level_id` — Some means the agent supports effort; None means
|
||||
/// the model does not.
|
||||
async fn pool_with_capabilities_via_return(thought_level_id: Option<&str>) -> AgentPool {
|
||||
use crate::acp::AcpClient;
|
||||
let acp = AcpClient::spawn(
|
||||
"bash",
|
||||
&["-c".to_string(), "sleep 10".to_string()],
|
||||
&[],
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.expect("failed to spawn test agent for capability helper");
|
||||
let agent = OwnedAgent {
|
||||
index: 0,
|
||||
acp,
|
||||
state: SessionState::default(),
|
||||
model_capabilities: None,
|
||||
desired_model: None,
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
startup_effort: None,
|
||||
desired_effort_gen: None,
|
||||
pending_effort_nonce: None,
|
||||
last_effort_result: None,
|
||||
agent_name: "test".into(),
|
||||
goose_system_prompt_supported: None,
|
||||
protocol_version: 2,
|
||||
};
|
||||
let mut pool = AgentPool::from_slots(vec![Some(agent)]);
|
||||
// Claim and immediately return with populated capabilities — drives the
|
||||
// `return_agent` capability-refresh path, the PRODUCTION code path.
|
||||
let mut agent = pool.try_claim(None).unwrap();
|
||||
agent.model_capabilities = Some(AgentModelCapabilities {
|
||||
thought_level_config_id: thought_level_id.map(str::to_string),
|
||||
config_options_raw: thought_level_id
|
||||
.map(|id| {
|
||||
vec![serde_json::json!({
|
||||
"id": id,
|
||||
"options": [{"value": "low"}, {"value": "medium"}, {"value": "high"}]
|
||||
})]
|
||||
})
|
||||
.unwrap_or_default(),
|
||||
available_models_raw: None,
|
||||
});
|
||||
pool.return_agent(agent);
|
||||
pool
|
||||
}
|
||||
|
||||
/// Switch effort-supporting model → non-effort model:
|
||||
/// - `return_agent` must write `Unsupported`
|
||||
/// - a subsequent effort pick must be REJECTED with `"unsupported_model"` status
|
||||
/// - the pick must not update `pool.desired_effort`
|
||||
#[tokio::test]
|
||||
async fn test_tristate_switch_effort_to_no_effort_rejects_pick() {
|
||||
// Start Supported (effort-capable model).
|
||||
let mut pool = pool_with_capabilities_via_return(Some("effort")).await;
|
||||
assert!(
|
||||
matches!(
|
||||
pool.effort_capability_state,
|
||||
EffortCapabilityState::Supported { .. }
|
||||
),
|
||||
"setup: pool must be Supported"
|
||||
);
|
||||
|
||||
// Simulate a model switch: agent returns with capabilities but no
|
||||
// thought_level — the new model does not support effort.
|
||||
let mut agent = pool.try_claim(None).unwrap();
|
||||
agent.model_capabilities = Some(AgentModelCapabilities {
|
||||
thought_level_config_id: None,
|
||||
config_options_raw: vec![],
|
||||
available_models_raw: None,
|
||||
});
|
||||
pool.return_agent(agent);
|
||||
|
||||
// Pool must now be Unsupported.
|
||||
assert!(
|
||||
matches!(
|
||||
pool.effort_capability_state,
|
||||
EffortCapabilityState::Unsupported
|
||||
),
|
||||
"after switch to non-effort model: pool must be Unsupported"
|
||||
);
|
||||
|
||||
// A subsequent effort pick must be REJECTED.
|
||||
let obs = observer::ObserverHandle::in_process();
|
||||
let payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": "effort",
|
||||
"value": "high",
|
||||
"category": "thought_level",
|
||||
});
|
||||
handle_set_config_option_control(&payload, &mut pool, Some(&obs));
|
||||
|
||||
let events = obs.snapshot();
|
||||
assert_eq!(events.len(), 1, "exactly one ack emitted");
|
||||
assert_eq!(
|
||||
events[0].payload["status"].as_str().unwrap(),
|
||||
"unsupported_model",
|
||||
"effort pick on Unsupported state must emit unsupported_model"
|
||||
);
|
||||
// Pick must not have been stored — desired_effort unchanged.
|
||||
assert!(
|
||||
pool.desired_effort.is_none(),
|
||||
"Unsupported pick must not update pool.desired_effort"
|
||||
);
|
||||
}
|
||||
|
||||
/// Switch no-effort model → effort-supporting model:
|
||||
/// - `return_agent` must write `Supported` with the new option set
|
||||
/// - a subsequent pick with the correct configId must be STORED and acked
|
||||
/// `pending_session`
|
||||
#[tokio::test]
|
||||
async fn test_tristate_switch_no_effort_to_effort_accepts_pick() {
|
||||
// Start Unsupported (non-effort model).
|
||||
let mut pool = pool_with_capabilities_via_return(None).await;
|
||||
assert!(
|
||||
matches!(
|
||||
pool.effort_capability_state,
|
||||
EffortCapabilityState::Unsupported
|
||||
),
|
||||
"setup: pool must be Unsupported"
|
||||
);
|
||||
|
||||
// Simulate a model switch: agent returns with thought_level capability.
|
||||
let new_config_id = "effort_v2";
|
||||
let mut agent = pool.try_claim(None).unwrap();
|
||||
agent.model_capabilities = Some(AgentModelCapabilities {
|
||||
thought_level_config_id: Some(new_config_id.to_string()),
|
||||
config_options_raw: vec![serde_json::json!({
|
||||
"id": new_config_id,
|
||||
"options": [{"value": "low"}, {"value": "high"}]
|
||||
})],
|
||||
available_models_raw: None,
|
||||
});
|
||||
pool.return_agent(agent);
|
||||
|
||||
// Pool must now be Supported with the new configId.
|
||||
assert!(
|
||||
matches!(
|
||||
&pool.effort_capability_state,
|
||||
EffortCapabilityState::Supported { config_id, .. } if config_id == new_config_id
|
||||
),
|
||||
"after switch to effort-capable model: pool must be Supported with new configId"
|
||||
);
|
||||
|
||||
// A pick with the correct configId must be STORED.
|
||||
let obs = observer::ObserverHandle::in_process();
|
||||
let payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": new_config_id,
|
||||
"value": "high",
|
||||
});
|
||||
handle_set_config_option_control(&payload, &mut pool, Some(&obs));
|
||||
|
||||
let events = obs.snapshot();
|
||||
assert_eq!(events.len(), 1, "exactly one ack emitted");
|
||||
assert_eq!(
|
||||
events[0].payload["status"].as_str().unwrap(),
|
||||
"pending_session",
|
||||
"pick on newly-Supported state must emit pending_session"
|
||||
);
|
||||
assert_eq!(
|
||||
pool.desired_effort
|
||||
.as_ref()
|
||||
.map(|(id, v)| (id.as_str(), v.as_str())),
|
||||
Some((new_config_id, "high")),
|
||||
"pick must be stored in pool.desired_effort"
|
||||
);
|
||||
}
|
||||
|
||||
/// Option-set CHANGE across a switch (both models support effort, different option sets):
|
||||
/// - value valid for the OLD model but absent from the new model's options → `invalid_value`
|
||||
/// - value in the NEW model's option set → `pending_session`
|
||||
#[tokio::test]
|
||||
async fn test_tristate_switch_new_options_validates_against_new_snapshot() {
|
||||
// Start with configId="effort", options=[low, medium, high].
|
||||
let mut pool = pool_with_capabilities_via_return(Some("effort")).await;
|
||||
|
||||
// Simulate a switch to a model with configId="effort", options=[standard, extended] only.
|
||||
let mut agent = pool.try_claim(None).unwrap();
|
||||
agent.model_capabilities = Some(AgentModelCapabilities {
|
||||
thought_level_config_id: Some("effort".to_string()),
|
||||
config_options_raw: vec![serde_json::json!({
|
||||
"id": "effort",
|
||||
"options": [{"value": "standard"}, {"value": "extended"}]
|
||||
})],
|
||||
available_models_raw: None,
|
||||
});
|
||||
pool.return_agent(agent);
|
||||
|
||||
// Verify snapshot updated.
|
||||
let new_valid_values = match &pool.effort_capability_state {
|
||||
EffortCapabilityState::Supported { valid_values, .. } => valid_values.clone(),
|
||||
other => panic!("expected Supported, got {other:?}"),
|
||||
};
|
||||
assert_eq!(
|
||||
new_valid_values,
|
||||
vec!["standard", "extended"],
|
||||
"option set must reflect new model's options"
|
||||
);
|
||||
|
||||
// Old value "high" (valid for old model, absent from new) → invalid_value.
|
||||
let obs = observer::ObserverHandle::in_process();
|
||||
let stale_payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": "effort",
|
||||
"value": "high",
|
||||
});
|
||||
handle_set_config_option_control(&stale_payload, &mut pool, Some(&obs));
|
||||
let events = obs.snapshot();
|
||||
assert_eq!(events.len(), 1, "exactly one ack for stale pick");
|
||||
assert_eq!(
|
||||
events[0].payload["status"].as_str().unwrap(),
|
||||
"invalid_value",
|
||||
"stale value from old model option set must be rejected"
|
||||
);
|
||||
|
||||
// New value "extended" (valid for new model) → pending_session.
|
||||
let obs2 = observer::ObserverHandle::in_process();
|
||||
let valid_payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": "effort",
|
||||
"value": "extended",
|
||||
});
|
||||
handle_set_config_option_control(&valid_payload, &mut pool, Some(&obs2));
|
||||
let events2 = obs2.snapshot();
|
||||
assert_eq!(events2.len(), 1, "exactly one ack for new-model pick");
|
||||
assert_eq!(
|
||||
events2[0].payload["status"].as_str().unwrap(),
|
||||
"pending_session",
|
||||
"value from new model option set must be accepted"
|
||||
);
|
||||
}
|
||||
|
||||
/// After a model switch, `effort_capability_state` becomes `Unknown` while
|
||||
/// the agent is checked out (capabilities reset). Once the agent returns
|
||||
/// with `model_capabilities = None` (post-switch, pre-next-session/new),
|
||||
/// a pick with category trust must use the Unknown path (pending_session),
|
||||
/// NOT be rejected as Unsupported.
|
||||
#[tokio::test]
|
||||
async fn test_tristate_unknown_after_switch_uses_pre_discovery_trust_path() {
|
||||
// Start Supported.
|
||||
let mut pool = pool_with_capabilities_via_return(Some("effort")).await;
|
||||
|
||||
// Simulate post-switch return with capabilities=None (reset by switch).
|
||||
let mut agent = pool.try_claim(None).unwrap();
|
||||
agent.model_capabilities = None; // reset — capabilities unknown until next session/new
|
||||
pool.return_agent(agent);
|
||||
|
||||
// Pool must be Unknown.
|
||||
assert!(
|
||||
matches!(pool.effort_capability_state, EffortCapabilityState::Unknown),
|
||||
"capabilities=None on return must produce Unknown state"
|
||||
);
|
||||
|
||||
// A pick with category trust must use the pre-discovery path (pending_session).
|
||||
let obs = observer::ObserverHandle::in_process();
|
||||
let payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": "effort",
|
||||
"value": "high",
|
||||
"category": "thought_level",
|
||||
});
|
||||
handle_set_config_option_control(&payload, &mut pool, Some(&obs));
|
||||
|
||||
let events = obs.snapshot();
|
||||
assert_eq!(events.len(), 1, "exactly one ack emitted");
|
||||
assert_eq!(
|
||||
events[0].payload["status"].as_str().unwrap(),
|
||||
"pending_session",
|
||||
"Unknown state with category trust must emit pending_session (pre-discovery path)"
|
||||
);
|
||||
assert_eq!(
|
||||
pool.desired_effort.as_ref().map(|(_, v)| v.as_str()),
|
||||
Some("high"),
|
||||
"Unknown path must store pick for apply at next session"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -280,21 +280,30 @@ impl OwnedAgent {
|
||||
}
|
||||
}
|
||||
|
||||
/// Pool-level capability snapshot for the `thought_level` config option.
|
||||
/// Pool-level effort capability state derived from worker session data.
|
||||
///
|
||||
/// Written at `return_agent` when a worker with populated capabilities returns
|
||||
/// to the pool. Because checked-out agents carry their own `model_capabilities`,
|
||||
/// this cache ensures `handle_set_config_option_control` can identify and
|
||||
/// validate effort picks even when all workers are checked out (pool slots
|
||||
/// are `None`).
|
||||
/// Written at `return_agent` from the returning worker's `model_capabilities`.
|
||||
/// Drives the classifier in `handle_set_config_option_control`:
|
||||
/// - `Unknown` → no session data yet; trust the Desktop's `category` field
|
||||
/// (pre-first-session window, case D).
|
||||
/// - `Supported` → current model advertises `thought_level`; validate picks
|
||||
/// against the snapshot's option set (case A).
|
||||
/// - `Unsupported` → current model confirmed to not advertise `thought_level`;
|
||||
/// reject picks immediately without updating the pool.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct PoolEffortCapabilities {
|
||||
/// The adapter's `thought_level` configId from `session/new`.
|
||||
/// `None` until the first session has been created.
|
||||
pub config_id: Option<String>,
|
||||
/// Adapter-advertised option values for the `thought_level` config option.
|
||||
/// Empty until the first session returns options.
|
||||
pub valid_values: Vec<String>,
|
||||
pub enum EffortCapabilityState {
|
||||
/// No session has returned capabilities yet — pre-first-session window.
|
||||
#[default]
|
||||
Unknown,
|
||||
/// Current model advertises `thought_level`. Contains the configId and
|
||||
/// the adapter-advertised valid option values.
|
||||
Supported {
|
||||
config_id: String,
|
||||
valid_values: Vec<String>,
|
||||
},
|
||||
/// Current model does not advertise `thought_level` (confirmed via a
|
||||
/// session/new response that carries no `thought_level` configOption).
|
||||
Unsupported,
|
||||
}
|
||||
|
||||
/// Pre-prompt effort application result sent from a worker task to the main loop
|
||||
@@ -364,23 +373,21 @@ pub struct AgentPool {
|
||||
/// pool level — a live pick/clear is always authoritative over startup seeding,
|
||||
/// even if the pool value is `None` (i.e. the user explicitly cleared it).
|
||||
pub effort_ever_picked: bool,
|
||||
/// Pool-level capability cache for the `thought_level` config option.
|
||||
/// Pool-level effort capability state for the `thought_level` config option.
|
||||
///
|
||||
/// Written at `return_agent` (refreshed from the returned agent's
|
||||
/// capabilities). Allows `handle_set_config_option_control` to identify
|
||||
/// and validate effort picks regardless of idle occupancy.
|
||||
pub effort_capabilities: PoolEffortCapabilities,
|
||||
/// True once any worker has ever had capabilities populated and returned to
|
||||
/// the pool. Used to distinguish "pre-first-return" (capabilities unknown
|
||||
/// — case D trust path applies) from "all workers currently busy"
|
||||
/// (capabilities known but cache temporarily empty — case C trust path)
|
||||
/// in `handle_set_config_option_control`.
|
||||
pub capabilities_ever_discovered: bool,
|
||||
/// Written at `return_agent` from the returning worker's `model_capabilities`,
|
||||
/// refreshing on every return so the state always reflects the current model.
|
||||
/// Drives `handle_set_config_option_control`'s three-way classifier:
|
||||
/// `Supported` → validate and store; `Unsupported` → reject; `Unknown` →
|
||||
/// pre-discovery trust (store for apply at first session).
|
||||
pub effort_capability_state: EffortCapabilityState,
|
||||
/// Generation of the last effort pick/clear for which a terminal ack
|
||||
/// (ok/failure/cleared) has already been emitted. Used by `return_agent`
|
||||
/// to emit exactly one terminal ack per generation when parallelism > 1:
|
||||
/// the first worker whose result is processed owns the ack; subsequent
|
||||
/// workers with the same generation are silenced.
|
||||
/// workers with the same generation are silenced. The sole ack authority
|
||||
/// is `resolve_effort_report`; `return_agent` only reads this field for
|
||||
/// the stale-generation force-invalidation (F1) guard.
|
||||
///
|
||||
/// `None` means no terminal ack has been emitted yet for any generation.
|
||||
pub last_acked_effort_gen: Option<u64>,
|
||||
@@ -743,8 +750,7 @@ impl AgentPool {
|
||||
effort_generation: 0,
|
||||
pending_effort_nonce: None,
|
||||
effort_ever_picked: false,
|
||||
effort_capabilities: PoolEffortCapabilities::default(),
|
||||
capabilities_ever_discovered: false,
|
||||
effort_capability_state: EffortCapabilityState::Unknown,
|
||||
last_acked_effort_gen: None,
|
||||
}
|
||||
}
|
||||
@@ -819,9 +825,9 @@ impl AgentPool {
|
||||
/// so the next session creation applies the current pool value. This closes
|
||||
/// the window where a busy worker's surviving session runs at a stale effort.
|
||||
///
|
||||
/// **Capability refresh:** the pool-level `effort_capabilities` cache is
|
||||
/// updated from the returned agent's capabilities (if populated), ensuring
|
||||
/// the cache is available even when all other workers are checked out.
|
||||
/// **Capability refresh:** the pool-level `effort_capability_state` is
|
||||
/// re-derived from the returned agent's `model_capabilities`, so the state
|
||||
/// always reflects the current model even after live model switches.
|
||||
pub fn return_agent(&mut self, mut agent: OwnedAgent) {
|
||||
let idx = agent.index;
|
||||
|
||||
@@ -911,11 +917,18 @@ impl AgentPool {
|
||||
agent.state.invalidate_all();
|
||||
}
|
||||
|
||||
// Capability refresh: write back the agent's capability snapshot to the
|
||||
// pool-level cache so validation remains available when all workers are
|
||||
// checked out. Always overwrite — returned workers have fresh capabilities.
|
||||
if let Some(ref caps) = agent.model_capabilities {
|
||||
if caps.thought_level_config_id.is_some() {
|
||||
// Capability refresh: derive the pool-level tri-state from the returned
|
||||
// agent's model_capabilities. Refresh on every return so the pool state
|
||||
// always reflects the current model — not just the first model seen.
|
||||
//
|
||||
// `Some(caps)` with a thought_level configId → Supported
|
||||
// `Some(caps)` without a thought_level configId → Unsupported (model
|
||||
// confirmed to not advertise thought_level; future picks are rejected)
|
||||
// `None` → Unknown (capability not yet resolved; keep the pre-first-
|
||||
// session trust path active for the next session)
|
||||
match &agent.model_capabilities {
|
||||
Some(caps) if caps.thought_level_config_id.is_some() => {
|
||||
let config_id = caps.thought_level_config_id.clone().unwrap();
|
||||
let valid_values = caps
|
||||
.config_options_raw
|
||||
.iter()
|
||||
@@ -923,7 +936,7 @@ impl AgentPool {
|
||||
opt.get("id")
|
||||
.or_else(|| opt.get("configId"))
|
||||
.and_then(|v| v.as_str())
|
||||
== caps.thought_level_config_id.as_deref()
|
||||
== Some(&config_id)
|
||||
})
|
||||
.and_then(|opt| opt.get("options"))
|
||||
.and_then(|o| o.as_array())
|
||||
@@ -935,24 +948,20 @@ impl AgentPool {
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
self.effort_capabilities = PoolEffortCapabilities {
|
||||
config_id: caps.thought_level_config_id.clone(),
|
||||
self.effort_capability_state = EffortCapabilityState::Supported {
|
||||
config_id,
|
||||
valid_values,
|
||||
};
|
||||
self.capabilities_ever_discovered = true;
|
||||
} else if self.capabilities_ever_discovered {
|
||||
// P3: the returning agent has populated capabilities but no
|
||||
// thought_level configId — the model was swapped to one that
|
||||
// does not support effort. Clear the pool-level cache so
|
||||
// subsequent picks are not validated against stale options.
|
||||
//
|
||||
// capabilities_ever_discovered stays true: we have seen at
|
||||
// least one session. This activates case C in
|
||||
// handle_set_config_option_control, letting picks through
|
||||
// without value-validation while the cache is empty (correct
|
||||
// — the user can still send a pick; the adapter will reject
|
||||
// it if the value is unsupported).
|
||||
self.effort_capabilities = PoolEffortCapabilities::default();
|
||||
}
|
||||
Some(_) => {
|
||||
// Populated capabilities, no thought_level — model is confirmed unsupported.
|
||||
self.effort_capability_state = EffortCapabilityState::Unsupported;
|
||||
}
|
||||
None => {
|
||||
// Capabilities not yet resolved (e.g. reset after a model switch).
|
||||
// Leave the tri-state Unknown so the pre-discovery trust path stays
|
||||
// active; the next session/new will resolve it.
|
||||
self.effort_capability_state = EffortCapabilityState::Unknown;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1318,13 +1327,13 @@ impl AgentPool {
|
||||
|
||||
/// Notify the pool that capabilities have been discovered for a worker.
|
||||
///
|
||||
/// **Test-only helper.** In production the pool capability cache is written
|
||||
/// **Test-only helper.** In production the pool capability state is written
|
||||
/// by `return_agent` when a worker returns after completing its first session.
|
||||
/// This function lets tests seed the cache directly without going through the
|
||||
/// This function lets tests seed the state directly without going through the
|
||||
/// full spawn-session-return cycle.
|
||||
#[cfg(test)]
|
||||
pub fn notify_capabilities_discovered(&mut self, caps: &AgentModelCapabilities) {
|
||||
if caps.thought_level_config_id.is_some() && self.effort_capabilities.config_id.is_none() {
|
||||
if let Some(ref config_id) = caps.thought_level_config_id {
|
||||
let valid_values = caps
|
||||
.config_options_raw
|
||||
.iter()
|
||||
@@ -1332,7 +1341,7 @@ impl AgentPool {
|
||||
opt.get("id")
|
||||
.or_else(|| opt.get("configId"))
|
||||
.and_then(|v| v.as_str())
|
||||
== caps.thought_level_config_id.as_deref()
|
||||
== Some(config_id.as_str())
|
||||
})
|
||||
.and_then(|opt| opt.get("options"))
|
||||
.and_then(|o| o.as_array())
|
||||
@@ -1342,11 +1351,12 @@ impl AgentPool {
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
self.effort_capabilities = PoolEffortCapabilities {
|
||||
config_id: caps.thought_level_config_id.clone(),
|
||||
self.effort_capability_state = EffortCapabilityState::Supported {
|
||||
config_id: config_id.clone(),
|
||||
valid_values,
|
||||
};
|
||||
self.capabilities_ever_discovered = true;
|
||||
} else {
|
||||
self.effort_capability_state = EffortCapabilityState::Unsupported;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1536,14 +1546,15 @@ async fn create_session_and_apply_model(
|
||||
}
|
||||
}
|
||||
|
||||
// Populate model capabilities on first session creation.
|
||||
if agent.model_capabilities.is_none() {
|
||||
agent.model_capabilities = Some(AgentModelCapabilities {
|
||||
config_options_raw: extract_agent_config_options(&resp.raw),
|
||||
available_models_raw: extract_model_state(&resp.raw),
|
||||
thought_level_config_id: extract_thought_level_config_id(&resp.raw),
|
||||
});
|
||||
}
|
||||
// Populate model capabilities from every session/new response — not just the
|
||||
// first. This ensures that after a model switch (which resets capabilities to
|
||||
// None to discard the pre-switch snapshot), the next session/new supplies
|
||||
// fresh capability data for the new model.
|
||||
agent.model_capabilities = Some(AgentModelCapabilities {
|
||||
config_options_raw: extract_agent_config_options(&resp.raw),
|
||||
available_models_raw: extract_model_state(&resp.raw),
|
||||
thought_level_config_id: extract_thought_level_config_id(&resp.raw),
|
||||
});
|
||||
|
||||
// B5 startup-default: arm desired_effort from startup_effort + capabilities.
|
||||
// No-op when desired_effort already set (live pick takes precedence, via the
|
||||
@@ -1558,6 +1569,14 @@ async fn create_session_and_apply_model(
|
||||
match resolve_model_switch_method(&resp.raw, desired) {
|
||||
Some(method) => {
|
||||
apply_model_switch(&mut agent.acp, &resp.session_id, desired, &method).await?;
|
||||
// The session/new response captured above reflects the pre-switch
|
||||
// model's configOptions. Reset capabilities to None so:
|
||||
// 1. `return_agent` writes EffortCapabilityState::Unknown, keeping
|
||||
// the pre-discovery trust path active (not stale-Supported).
|
||||
// 2. The next session/new (after this session is invalidated on
|
||||
// the next model switch or rotation) re-derives capabilities
|
||||
// from the then-current model's response.
|
||||
agent.model_capabilities = None;
|
||||
true
|
||||
}
|
||||
None => {
|
||||
@@ -1680,10 +1699,23 @@ async fn create_session_and_apply_model(
|
||||
// post-switch state. modelOverridden reflects whether the switch actually
|
||||
// applied — false on the unsupported arm so the panel doesn't show a
|
||||
// stale override badge.
|
||||
//
|
||||
// When a model switch succeeded, the configOptions in resp.raw are pre-switch
|
||||
// data and must not be cached — the next session/new will supply fresh options
|
||||
// for the new model. Emit null for configOptions on the switch path so Desktop
|
||||
// clears its picker state rather than rendering stale capability data.
|
||||
let config_options_for_cache = if switch_succeeded {
|
||||
serde_json::Value::Null
|
||||
} else {
|
||||
resp.raw
|
||||
.get("configOptions")
|
||||
.cloned()
|
||||
.unwrap_or(serde_json::Value::Null)
|
||||
};
|
||||
agent.acp.observe(
|
||||
"session_config_captured",
|
||||
serde_json::json!({
|
||||
"configOptions": resp.raw.get("configOptions").cloned().unwrap_or(serde_json::Value::Null),
|
||||
"configOptions": config_options_for_cache,
|
||||
"modes": resp.raw.get("modes").cloned().unwrap_or(serde_json::Value::Null),
|
||||
"models": resp.raw.get("models").cloned().unwrap_or(serde_json::Value::Null),
|
||||
"modelOverridden": agent.model_overridden && switch_succeeded,
|
||||
@@ -8235,9 +8267,14 @@ mod effort_tests {
|
||||
available_models_raw: None,
|
||||
});
|
||||
|
||||
// All slots are None (workers checked out) but capabilities ARE known.
|
||||
assert!(pool.effort_capabilities.config_id.is_some());
|
||||
assert!(pool.capabilities_ever_discovered);
|
||||
// All slots are None (workers checked out) but capabilities ARE known (Supported).
|
||||
assert!(
|
||||
matches!(
|
||||
pool.effort_capability_state,
|
||||
EffortCapabilityState::Supported { .. }
|
||||
),
|
||||
"V-2 setup: pool must be Supported after notify_capabilities_discovered"
|
||||
);
|
||||
assert!(pool.agents_mut().iter().flatten().next().is_none());
|
||||
|
||||
// set_pool_effort should store, not return NoCatalog.
|
||||
@@ -9222,12 +9259,12 @@ mod effort_tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ── P3: stale capability cache cleared on model swap ─────────────────────
|
||||
// ── P3: capability state updated to Unsupported on model swap ─────────────────
|
||||
|
||||
/// P3: when a returning agent has populated capabilities but no
|
||||
/// thought_level configId (model swapped to non-effort), the pool-level
|
||||
/// effort_capabilities cache is cleared so stale options are not used for
|
||||
/// validation on the next pick.
|
||||
/// effort_capability_state transitions to `Unsupported` so subsequent picks
|
||||
/// are rejected immediately rather than validated against stale options.
|
||||
#[tokio::test]
|
||||
async fn test_capability_cache_cleared_when_model_loses_thought_level() {
|
||||
let acp = AcpClient::spawn(
|
||||
@@ -9260,8 +9297,13 @@ mod effort_tests {
|
||||
config_options_raw: vec![serde_json::json!({"id": "effort", "options": [{"value": "low"}, {"value": "high"}]})],
|
||||
available_models_raw: None,
|
||||
});
|
||||
assert!(pool.effort_capabilities.config_id.is_some());
|
||||
assert!(pool.capabilities_ever_discovered);
|
||||
assert!(
|
||||
matches!(
|
||||
pool.effort_capability_state,
|
||||
EffortCapabilityState::Supported { .. }
|
||||
),
|
||||
"P3 setup: must start Supported"
|
||||
);
|
||||
|
||||
// Worker returns after a model swap — no thought_level in new model.
|
||||
let mut agent = pool.try_claim(None).unwrap();
|
||||
@@ -9272,19 +9314,13 @@ mod effort_tests {
|
||||
});
|
||||
pool.return_agent(agent);
|
||||
|
||||
// P3: cache must be cleared.
|
||||
// P3: state must transition to Unsupported — not Unknown, not stale-Supported.
|
||||
assert!(
|
||||
pool.effort_capabilities.config_id.is_none(),
|
||||
"P3: effort_capabilities must be cleared when model loses thought_level"
|
||||
);
|
||||
assert!(
|
||||
pool.effort_capabilities.valid_values.is_empty(),
|
||||
"P3: valid_values must be cleared"
|
||||
);
|
||||
// capabilities_ever_discovered stays true (case C remains active).
|
||||
assert!(
|
||||
pool.capabilities_ever_discovered,
|
||||
"capabilities_ever_discovered must stay true after cache clear"
|
||||
matches!(
|
||||
pool.effort_capability_state,
|
||||
EffortCapabilityState::Unsupported
|
||||
),
|
||||
"P3: effort_capability_state must be Unsupported when model loses thought_level"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user