feat(agents): harness-agnostic effort write path and spawn bridge

desktop/src-tauri/src/commands/agent_config.rs

@@ -14,9 +14,8 @@ use crate::{
         },
         current_instance_id, is_reserved_env_key, is_safe_to_reveal, is_well_formed_env_key,
         known_acp_runtime, load_managed_agents, load_personas, resolve_effective_agent_env,
-        save_managed_agents, sync_managed_agent_processes, AgentDefinition, BackendKind,
-        GlobalAgentConfig, KnownAcpRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey,
-        MAX_ENV_VALUE_BYTES,
+        save_managed_agents, sync_managed_agent_processes, AgentDefinition, GlobalAgentConfig,
+        KnownAcpRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey, MAX_ENV_VALUE_BYTES,
     },
 };
 
@@ -535,42 +534,15 @@ fn parse_models(raw: Option<&serde_json::Value>) -> (Vec<AcpModelEntry>, Option<
     (models, current_model)
 }
 
-/// Persist the canonical startup effort level for a local managed agent.
-///
-/// B5 (v4 direct-write): the panel's EffortPicker calls this directly to set the
-/// effort a spawn will apply at next session start. The value is stored on the
-/// record; at spawn `runtime.rs` injects it as `BUZZ_ACP_EFFORT_LEVEL` and the
-/// harness applies it via `session/set_config_option` against the adapter's
-/// advertised `thought_level` configId. Pass `None` to clear (adapter default).
-///
-/// Rejects non-local backends: remote agents receive effort through `policy_env`
-/// at deploy time (see `agents_deploy.rs`), never this local persistence path —
-/// so an effort edit against a deployed agent is a caller error, not a silent
-/// no-op that leaves the panel and the running agent disagreeing.
-#[tauri::command]
-pub fn persist_agent_effort_level(
-    pubkey: String,
+/// Atomically set the record's canonical effort column and strip every stale
+/// record-scope effort env alias. Split from the Tauri command so the invariant
+/// — no leftover alias can outrank the just-set column — is directly testable.
+pub(crate) fn apply_picker_effort_level(
+    record: &mut ManagedAgentRecord,
     effort_level: Option<String>,
-    app: AppHandle,
-    state: State<'_, AppState>,
-) -> Result<(), String> {
-    let _store_guard = state
-        .managed_agents_store_lock
-        .lock()
-        .map_err(|e| e.to_string())?;
-    let mut records = load_managed_agents(&app)?;
-    let record = records
-        .iter_mut()
-        .find(|r| r.pubkey == pubkey)
-        .ok_or_else(|| format!("agent {pubkey} not found"))?;
-    if record.backend != BackendKind::Local {
-        return Err(format!(
-            "agent {pubkey} is not a local agent; remote effort is set at deploy time"
-        ));
-    }
+) {
     record.effort_level = effort_level;
-    record.updated_at = crate::util::now_iso();
-    save_managed_agents(&app, &records)
+    crate::managed_agents::remove_record_effort_aliases(&mut record.env_vars);
 }
 
 #[cfg(test)]

desktop/src-tauri/src/commands/agent_config_tests.rs

@@ -29,7 +29,7 @@ fn with_no_goose_config<T>(body: impl FnOnce() -> T) -> T {
 }
 
 fn goose_runtime() -> &'static KnownAcpRuntime {
-    &KnownAcpRuntime {
+    static RUNTIME: KnownAcpRuntime = KnownAcpRuntime {
         id: "goose",
         label: "Goose",
         commands: &["goose"],
@@ -55,13 +55,16 @@ fn goose_runtime() -> &'static KnownAcpRuntime {
         config_file_format: Some("yaml"),
         supports_acp_native_config: true,
         thinking_env_var: Some("GOOSE_THINKING_EFFORT"),
+        effort_normalization: Some(&crate::managed_agents::GOOSE_EFFORT_NORMALIZATION),
+        effort_accepted_values: None,
         max_tokens_env_var: Some("GOOSE_MAX_TOKENS"),
         context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"),
         max_rounds_env_var: None,
         required_normalized_fields: &["model", "provider"],
         login_hint: None,
         auth_probe_args: None,
-    }
+    };
+    &RUNTIME
 }
 
 fn agent_record() -> ManagedAgentRecord {
@@ -630,6 +633,58 @@ fn baked_env_mixed_keys_correct_masking() {
     assert!(token.masked);
 }
 
+/// F1 picker direct-write invariant: a stale record-native `GOOSE_THINKING_EFFORT`
+/// (launch-projection tier 1, ABOVE the canonical column) must not survive a
+/// picker write. Setting effort `high` through the picker path both writes the
+/// column and sweeps the stale alias, so the reader and the launch projection
+/// both resolve `high` — not the stale `low`. Deleting the sweep in
+/// `apply_picker_effort_level` re-breaks this: the projection would emit `low`.
+#[test]
+fn picker_write_sweeps_stale_record_native_effort_alias() {
+    let mut record = agent_record();
+    record
+        .env_vars
+        .insert("GOOSE_THINKING_EFFORT".to_string(), "low".to_string());
+
+    super::apply_picker_effort_level(&mut record, Some("high".to_string()));
+
+    // The stale record-native alias is gone; only the column carries the value.
+    assert!(
+        !record.env_vars.contains_key("GOOSE_THINKING_EFFORT"),
+        "stale record-native effort alias must be swept by the picker write"
+    );
+    assert_eq!(record.effort_level.as_deref(), Some("high"));
+
+    // Reader: the panel resolves the just-set value, not the stale alias.
+    let surface = with_no_goose_config(|| {
+        resolve_config_surface(
+            record.clone(),
+            &[],
+            Some(goose_runtime()),
+            None,
+            &Default::default(),
+            None,
+        )
+    });
+    let effort = surface
+        .normalized
+        .thinking_effort
+        .expect("picker-set effort must resolve");
+    assert_eq!(effort.value.as_deref(), Some("high"));
+
+    // Launch projection: the spawned child receives the picker value.
+    let launch = crate::managed_agents::config_bridge::effort::effort_launch_projection(
+        &record,
+        Some(goose_runtime()),
+        &[],
+        None,
+        &std::collections::BTreeMap::new(),
+        None,
+        &std::collections::BTreeMap::new(),
+    );
+    assert_eq!(launch.value.as_deref(), Some("high"));
+}
+
 #[test]
 fn baked_env_thinking_effort_is_unmasked() {
     // BUZZ_AGENT_THINKING_EFFORT is a non-secret enum — must not be masked.

desktop/src-tauri/src/commands/agent_discovery.rs

@@ -17,7 +17,6 @@ fn active_installs() -> &'static std::sync::Mutex<std::collections::HashSet<Stri
 /// Returns the adapter install commands that `install_acp_runtime_blocking` would
 /// run for `runtime_id` given a resolved adapter binary at `adapter_path` (or `None` if not found).
 /// Returns `None` when no install is needed; `Some(cmds)` when adapter is missing or outdated.
-///
 /// For the codex **outdated** case, returns a two-step reinstall: uninstall `@zed-industries/codex-acp`
 /// then install `@agentclientprotocol/codex-acp` (npm ≥7 refuses to overwrite a bin from another pkg).
 /// For the **missing** case, catalog's `adapter_install_commands` are used as-is.
@@ -150,6 +149,7 @@ pub async fn save_custom_harness(
         model_env_var: None,
         provider_env_var: None,
         thinking_env_var: None,
+        effort_canonical_values: None,
         max_tokens_env_var: None,
         context_limit_env_var: None,
         max_rounds_env_var: None,

desktop/src-tauri/src/commands/agent_models.rs

@@ -26,7 +26,6 @@ use crate::{
         UpdateManagedAgentResponse, DEFAULT_ACP_COMMAND,
     },
     relay::{relay_ws_url_with_override, sync_managed_agent_profile},
-    util::now_iso,
 };
 
 /// Query available models from an agent via `buzz-acp models --json`.

desktop/src-tauri/src/commands/agent_models_update.rs

@@ -35,6 +35,88 @@ fn ensure_access_policy_change_supported(
     Ok(())
 }
 
+/// Reject an effort mutation for a non-local record. Remote effort is
+/// deployment-owned (set via `policy_env` at deploy time); persisting locally
+/// would make the canonical column diverge from the deployed runtime's actual
+/// effort.
+fn ensure_effort_change_supported(
+    record: &ManagedAgentRecord,
+    effort_level: &Option<Option<String>>,
+) -> Result<(), String> {
+    if effort_level.is_some() && record.backend != crate::managed_agents::BackendKind::Local {
+        return Err(format!(
+            "agent {} is not a local agent; remote effort is set at deploy time",
+            record.pubkey
+        ));
+    }
+    Ok(())
+}
+
+/// Guard/apply seam for the effort step inside `apply_record_field_updates`.
+fn apply_effort_update(
+    record: &mut ManagedAgentRecord,
+    effort_level: Option<Option<String>>,
+) -> Result<(), String> {
+    ensure_effort_change_supported(record, &effort_level)?;
+    if let Some(effort_override) = effort_level {
+        crate::commands::agent_config::apply_picker_effort_level(record, effort_override);
+    }
+    Ok(())
+}
+
+/// Proof token returned by `apply_record_field_updates`. Zero-size and
+/// `#[must_use]`; consumed by `stamp_record_updated_at`, so removing the
+/// `apply_record_field_updates` call from `update_managed_agent` leaves
+/// `applied` undefined at the timestamp site — a compile error.
+#[derive(Debug)]
+#[must_use]
+pub(crate) struct RecordFieldsApplied(());
+
+/// Apply the env-vars and effort steps of `update_managed_agent` to a record
+/// in the correct order: env_vars FIRST (so the same-request map cannot
+/// reintroduce a stale alias), then the canonical effort column write.
+///
+/// Returns a `RecordFieldsApplied` token that must be passed to
+/// `stamp_record_updated_at`. Removing this call from `update_managed_agent`
+/// leaves `applied` undefined at the timestamp site — a compile error.
+///
+/// Called by `update_managed_agent` inside its locked transaction and by tests.
+/// Any step deleted from inside this function is directly caught by the
+/// corresponding test assertion.
+///
+/// Mutation proofs (see `agent_models_update_tests.rs`):
+///   - Deleting the `apply_effort_update` call leaves `effort_level` unchanged.
+///   - Deleting `ensure_effort_change_supported` inside `apply_effort_update`
+///     lets non-local writes pass `Ok(())` without mutating the column.
+///   - Deleting `apply_picker_effort_level` inside `apply_effort_update`
+///     leaves `effort_level == None` on a local-set request.
+pub(crate) fn apply_record_field_updates(
+    record: &mut ManagedAgentRecord,
+    env_vars: Option<&std::collections::BTreeMap<String, String>>,
+    inherit_transition: bool,
+    effort_level: Option<Option<String>>,
+) -> Result<RecordFieldsApplied, String> {
+    // Order is load-bearing: env_vars before effort so a same-request
+    // env_vars map cannot reintroduce a stale alias after the column write.
+    crate::managed_agents::apply_env_vars_then_effort_transition(
+        record,
+        env_vars.cloned(),
+        inherit_transition,
+    );
+    apply_effort_update(record, effort_level)?;
+    Ok(RecordFieldsApplied(()))
+}
+
+/// Stamp `record.updated_at` with the current ISO timestamp, consuming the
+/// `RecordFieldsApplied` proof token. Removing `apply_record_field_updates`
+/// from `update_managed_agent` leaves `applied` undefined here — a compile error.
+pub(crate) fn stamp_record_updated_at(
+    record: &mut ManagedAgentRecord,
+    _applied: RecordFieldsApplied,
+) {
+    record.updated_at = crate::util::now_iso();
+}
+
 /// Flush a retained managed-agent policy, preserving any earlier profile error.
 pub(crate) async fn flush_managed_agent_policy(
     app: &AppHandle,
@@ -115,15 +197,17 @@ pub async fn update_managed_agent(
         // Harness edit: the persona's runtime is authoritative, so an explicit
         // `agent_command_override` is persisted ONLY when the user picks a
         // command that diverges from the persona, and the empty/whitespace
-        // "Inherit from persona" sentinel clears both the pin and the
-        // materialized record runtime. A name-only edit
+        // "Inherit from persona" sentinel clears the pin, the materialized
+        // record runtime, AND the per-instance effort override (column here,
+        // env aliases after `env_vars` is applied below). A name-only edit
         // (`agent_command == None`) leaves the pin intact. `harness_override`
         // threads the user's explicit intent — see `apply_agent_command_update`
         // and `update_time_agent_command_override` for the full resolution
         // rules.
+        let mut inherit_transition = false;
         if let Some(agent_command) = input.agent_command {
             let personas = load_personas(&app).unwrap_or_default();
-            crate::managed_agents::apply_agent_command_update(
+            inherit_transition = crate::managed_agents::apply_agent_command_update(
                 record,
                 &personas,
                 &agent_command,
@@ -136,9 +220,16 @@ pub async fn update_managed_agent(
         // mcp_command is intentionally not applied here — the effective MCP
         // command is always catalog-derived (known_acp_runtime at spawn time)
         // and the per-record field is never read by the runtime.
-        if let Some(env_vars) = input.env_vars {
-            crate::managed_agents::validate_user_env_keys(&env_vars)?;
-            record.env_vars = env_vars;
+        //
+        // Apply the caller-supplied `env_vars` (validated first), then — only on
+        // the pin→inherit transition — strip the record effort env aliases. The
+        // order is load-bearing: stripping AFTER the env replacement is what
+        // stops a same-request `env_vars` map from reintroducing a stale effort
+        // alias while the instance inherits its harness. The column was already
+        // cleared inside `apply_agent_command_update`. See
+        // `apply_env_vars_then_effort_transition` for the pinned invariant.
+        if let Some(ref env_vars) = input.env_vars {
+            crate::managed_agents::validate_user_env_keys(env_vars)?;
         }
 
         // Native provider/model fields are authoritative. Keep the typed marker
@@ -211,7 +302,23 @@ pub async fn update_managed_agent(
             record.respond_to_allowlist = prospective_allowlist;
         }
 
-        record.updated_at = now_iso();
+        // Effort + env_vars: applied together inside `apply_record_field_updates` to
+        // enforce the ordering invariant (env_vars before effort column write) and
+        // provide a directly-testable production seam. Effort persists inside the
+        // locked transaction so an access-policy restart above snapshots and
+        // launches the new effort value. Present+Some(v)=set; Present+None=clear;
+        // Absent=don't touch (the dialog sends it only when effortTouched).
+        // The returned token is consumed by `stamp_record_updated_at`; removing
+        // this call from `update_managed_agent` leaves `applied` undefined there
+        // — a compile error (the sole outer-seam proof for this call site).
+        let applied = apply_record_field_updates(
+            record,
+            input.env_vars.as_ref(),
+            inherit_transition,
+            input.effort_level,
+        )?;
+
+        stamp_record_updated_at(record, applied);
 
         save_managed_agents(&app, &records)?;
 
@@ -365,5 +472,6 @@ pub async fn update_managed_agent(
 }
 
 #[cfg(test)]
+#[allow(unused_must_use)]
 #[path = "agent_models_update_tests.rs"]
 mod tests;