fix(desktop): gate agent restart eligibility on secret availability

Automatic restart flows (post-install adapter bounce, global-config
change) derived eligibility purely from resolve_effective_agent_env +
agent_readiness. An unavailable secret tier hydrates to an empty env
that is indistinguishable from an intentionally-empty one, so those
flows could stop a live/setup process and only discover the refusal at
respawn (FailedAfterStop).

Add effective_secrets_unavailable(record, personas) — ORs the three
registry-derivable tiers (instance secrets_unavailable, definition,
harness) — and thread it through both eligibility predicates and the
under-lock recheck in each flow, refusing before any stop.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

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

@@ -405,16 +405,21 @@ enum InstallRestartOutcome {
 /// An agent qualifies iff:
 /// - it is a local backend with a live PID (`pid_alive`),
 /// - its effective command maps to `runtime_id`,
-/// - it was **spawned in setup-listener mode** (`setup_mode`), AND
-/// - its readiness **now computes `Ready`** (install fixed the blocker).
+/// - it was **spawned in setup-listener mode** (`setup_mode`),
+/// - its readiness **now computes `Ready`** (install fixed the blocker), AND
+/// - none of its secret tiers are unavailable (`!secrets_unavailable`) — an
+///   unavailable tier's empty hydrated env looks `Ready`, so without this a
+///   stop-then-respawn would stop a live setup process only to hit the spawn
+///   refusal (`FailedAfterStop`).
 fn should_restart_after_install(
     is_local: bool,
     pid_alive: bool,
     runtime_matches: bool,
     setup_mode: bool,
     now_ready: bool,
+    secrets_unavailable: bool,
 ) -> bool {
-    is_local && pid_alive && runtime_matches && setup_mode && now_ready
+    is_local && pid_alive && runtime_matches && setup_mode && now_ready && !secrets_unavailable
 }
 
 /// Restart all setup-mode agents whose runtime matches `runtime_id` and whose
@@ -426,9 +431,9 @@ async fn restart_setup_mode_agents_after_install(
     use crate::{
         app_state::AppState,
         managed_agents::{
-            agent_readiness, known_acp_runtime, load_global_agent_config, load_managed_agents,
-            load_personas, record_agent_command, resolve_effective_agent_env, AgentReadiness,
-            BackendKind,
+            agent_readiness, effective_secrets_unavailable, known_acp_runtime,
+            load_global_agent_config, load_managed_agents, load_personas, record_agent_command,
+            resolve_effective_agent_env, AgentReadiness, BackendKind,
         },
     };
     use tauri::Manager;
@@ -477,6 +482,7 @@ async fn restart_setup_mode_agents_after_install(
                     runtime_matches,
                     setup_mode,
                     now_ready,
+                    effective_secrets_unavailable(record, &personas),
                 )
             })
             .map(|r| r.pubkey.clone())
@@ -517,10 +523,11 @@ async fn restart_single_agent_after_install(
     use crate::{
         app_state::AppState,
         managed_agents::{
-            agent_readiness, current_instance_id, find_managed_agent_mut, known_acp_runtime,
-            load_global_agent_config, load_managed_agents, load_personas, record_agent_command,
-            resolve_effective_agent_env, save_managed_agents, stop_managed_agent_process,
-            sync_managed_agent_processes, AgentReadiness, BackendKind,
+            agent_readiness, current_instance_id, effective_secrets_unavailable,
+            find_managed_agent_mut, known_acp_runtime, load_global_agent_config,
+            load_managed_agents, load_personas, record_agent_command, resolve_effective_agent_env,
+            save_managed_agents, stop_managed_agent_process, sync_managed_agent_processes,
+            AgentReadiness, BackendKind,
         },
     };
     use tauri::Manager;
@@ -601,6 +608,14 @@ async fn restart_single_agent_after_install(
             ));
         }
 
+        // Fail-closed under lock: an unavailable secret tier makes the empty
+        // hydrated env look Ready, so re-check before the stop (never after).
+        if effective_secrets_unavailable(record, &personas) {
+            return Err(format!(
+                "agent {pubkey_owned} secrets unavailable from keyring under lock — not bouncing"
+            ));
+        }
+
         // Stop the process.
         let record_mut = find_managed_agent_mut(&mut records, &pubkey_owned)?;
         stop_managed_agent_process(&app_for_stop, record_mut, &mut runtimes)?;
@@ -1261,7 +1276,7 @@ mod tests {
     #[test]
     fn test_should_restart_after_install_setup_mode_now_ready_is_candidate() {
         assert!(
-            should_restart_after_install(true, true, true, true, true),
+            should_restart_after_install(true, true, true, true, true, false),
             "setup-mode codex agent that became Ready must be restarted after install"
         );
     }
@@ -1270,7 +1285,7 @@ mod tests {
     #[test]
     fn test_should_restart_after_install_still_not_ready_is_not_candidate() {
         assert!(
-            !should_restart_after_install(true, true, true, true, false),
+            !should_restart_after_install(true, true, true, true, false, false),
             "setup-mode agent still NotReady must NOT be restarted (would re-enter setup mode)"
         );
     }
@@ -1279,7 +1294,7 @@ mod tests {
     #[test]
     fn test_should_restart_after_install_healthy_agent_is_not_candidate() {
         assert!(
-            !should_restart_after_install(true, true, true, false, true),
+            !should_restart_after_install(true, true, true, false, true, false),
             "healthy in-pool agent (setup_mode=false) must NOT be bounced on install"
         );
     }
@@ -1288,7 +1303,7 @@ mod tests {
     #[test]
     fn test_should_restart_after_install_different_runtime_is_not_candidate() {
         assert!(
-            !should_restart_after_install(true, true, false, true, true),
+            !should_restart_after_install(true, true, false, true, true, false),
             "agent on a different runtime must NOT be restarted by this install"
         );
     }
@@ -1297,7 +1312,7 @@ mod tests {
     #[test]
     fn test_should_restart_after_install_non_local_is_not_candidate() {
         assert!(
-            !should_restart_after_install(false, true, true, true, true),
+            !should_restart_after_install(false, true, true, true, true, false),
             "non-local (provider-backend) agent must NOT be restarted"
         );
     }
@@ -1306,11 +1321,26 @@ mod tests {
     #[test]
     fn test_should_restart_after_install_dead_pid_is_not_candidate() {
         assert!(
-            !should_restart_after_install(true, false, true, true, true),
+            !should_restart_after_install(true, false, true, true, true, false),
             "agent whose process is no longer running must NOT be restarted"
         );
     }
 
+    /// A setup-mode agent that would otherwise be a candidate (all five prior
+    /// inputs true) but whose secrets are unavailable → NO restart. An
+    /// unavailable tier's empty hydrated env makes readiness compute `Ready`,
+    /// so this is the only input distinguishing a bounce that would succeed
+    /// from one that would stop the process and hit the spawn refusal
+    /// (`FailedAfterStop`). Mutation check: drop `&& !secrets_unavailable` from
+    /// the predicate and this test fails while the positive control stays green.
+    #[test]
+    fn test_should_restart_after_install_secrets_unavailable_is_not_candidate() {
+        assert!(
+            !should_restart_after_install(true, true, true, true, true, true),
+            "an otherwise-eligible setup-mode agent with unavailable secrets must NOT be bounced"
+        );
+    }
+
     // ── badge availability-drift (Phase 2) ───────────────────────────────────
     //
     // `availability_drift` is a pure predicate over two `Option` values —

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

@@ -16,11 +16,11 @@ use tauri::AppHandle;
 use crate::{
     app_state::AppState,
     managed_agents::{
-        agent_readiness, current_instance_id, find_managed_agent_mut, known_acp_runtime,
-        load_global_agent_config, load_managed_agents, load_personas, record_agent_command,
-        resolve_effective_agent_env, save_global_agent_config, save_managed_agents,
-        stop_managed_agent_process, sync_managed_agent_processes, validate_global_config,
-        AgentReadiness, BackendKind, GlobalAgentConfig,
+        agent_readiness, current_instance_id, effective_secrets_unavailable,
+        find_managed_agent_mut, known_acp_runtime, load_global_agent_config, load_managed_agents,
+        load_personas, record_agent_command, resolve_effective_agent_env, save_global_agent_config,
+        save_managed_agents, stop_managed_agent_process, sync_managed_agent_processes,
+        validate_global_config, AgentReadiness, BackendKind, GlobalAgentConfig,
     },
 };
 
@@ -231,7 +231,12 @@ fn collect_restart_candidates(
             // because Phase 2 will stop-then-start unconditionally.
             let env_changed = old_ready && old_effective.env != new_effective.env;
 
-            should_restart_on_config_change(old_ready, new_ready, env_changed)
+            should_restart_on_config_change(
+                old_ready,
+                new_ready,
+                env_changed,
+                effective_secrets_unavailable(record, &all_personas),
+            )
         })
         .map(|r| r.pubkey.clone())
         .collect();
@@ -334,7 +339,12 @@ async fn restart_local_agent_on_config_change(
         let new_ready = matches!(agent_readiness(&new_effective), AgentReadiness::Ready);
         // Under lock, the alive check was already done above via process_is_running.
         let env_changed = old_ready && old_effective.env != new_effective.env;
-        if !should_restart_on_config_change(old_ready, new_ready, env_changed) {
+        if !should_restart_on_config_change(
+            old_ready,
+            new_ready,
+            env_changed,
+            effective_secrets_unavailable(record, &personas_owned),
+        ) {
             return Err(format!(
                 "agent {pubkey_owned} restart condition no longer valid under lock"
             ));
@@ -429,8 +439,19 @@ fn persist_last_error(app: &AppHandle, pubkey: &str, error: &str) -> Result<(),
 /// restart would not repair the missing auth token. If the binary disappears,
 /// the process would already be dead and the PID alive-check in the candidate
 /// scan would have excluded it.
-fn should_restart_on_config_change(old_ready: bool, new_ready: bool, env_changed: bool) -> bool {
-    (!old_ready && new_ready) || (old_ready && env_changed)
+///
+/// **Fail-closed on unavailable secrets:** when any secret tier is unavailable
+/// (`secrets_unavailable`) the record's empty hydrated env can look `Ready` and
+/// its env can differ from the old config for unrelated reasons, so restart is
+/// refused — a stop-then-respawn would stop a live process only to hit the
+/// spawn refusal (`FailedAfterStop`).
+fn should_restart_on_config_change(
+    old_ready: bool,
+    new_ready: bool,
+    env_changed: bool,
+    secrets_unavailable: bool,
+) -> bool {
+    !secrets_unavailable && ((!old_ready && new_ready) || (old_ready && env_changed))
 }
 
 /// Fail closed when the CURRENTLY committed global config cannot load before an
@@ -500,7 +521,7 @@ mod tests {
     fn env_changed_running_agent_is_candidate() {
         // old_ready=true, new_ready=true, env_changed=true
         assert!(
-            should_restart_on_config_change(true, true, true),
+            should_restart_on_config_change(true, true, true, false),
             "running agent with changed env must be restarted"
         );
     }
@@ -510,7 +531,7 @@ mod tests {
     fn unchanged_running_agent_is_not_candidate() {
         // old_ready=true, new_ready=true, env_changed=false
         assert!(
-            !should_restart_on_config_change(true, true, false),
+            !should_restart_on_config_change(true, true, false, false),
             "running agent with identical env must NOT be restarted"
         );
     }
@@ -520,7 +541,7 @@ mod tests {
     fn not_ready_to_ready_is_candidate() {
         // old_ready=false, new_ready=true, env_changed=false (env_changed irrelevant)
         assert!(
-            should_restart_on_config_change(false, true, false),
+            should_restart_on_config_change(false, true, false, false),
             "NotReady → Ready must be a restart candidate"
         );
     }
@@ -531,7 +552,7 @@ mod tests {
     fn ready_to_not_ready_env_changed_is_candidate() {
         // old_ready=true (had key), new_ready=false (key removed), env_changed=true
         assert!(
-            should_restart_on_config_change(true, false, true),
+            should_restart_on_config_change(true, false, true, false),
             "Ready → NotReady with env change must be a restart candidate"
         );
     }
@@ -541,7 +562,7 @@ mod tests {
     fn both_not_ready_unchanged_is_not_candidate() {
         // old_ready=false, new_ready=false, env_changed=false
         assert!(
-            !should_restart_on_config_change(false, false, false),
+            !should_restart_on_config_change(false, false, false, false),
             "both NotReady with no env change must NOT be a candidate"
         );
     }
@@ -552,7 +573,7 @@ mod tests {
         // Changed one unrelated env var but still missing the required key.
         // old_ready=false, new_ready=false, env_changed=true
         assert!(
-            !should_restart_on_config_change(false, false, true),
+            !should_restart_on_config_change(false, false, true, false),
             "NotReady→NotReady (env changed but still broken) must NOT be a candidate"
         );
     }
@@ -566,8 +587,28 @@ mod tests {
     fn not_ready_to_ready_with_env_change_is_candidate() {
         // old_ready=false, new_ready=true, env_changed=true
         assert!(
-            should_restart_on_config_change(false, true, true),
+            should_restart_on_config_change(false, true, true, false),
             "NotReady → Ready (with env change) must be a restart candidate"
         );
     }
+
+    /// An agent that would otherwise be a restart candidate (`NotReady → Ready`,
+    /// or Ready with env changed) but whose secrets are unavailable → NO
+    /// restart. An unavailable tier's empty hydrated env can make readiness
+    /// compute `Ready` and can differ from the old config for unrelated
+    /// reasons, so this flag is the sole guard against stopping a live process
+    /// only to hit the spawn refusal (`FailedAfterStop`). Mutation check: drop
+    /// the leading `!secrets_unavailable &&` and both asserts below flip while
+    /// the healthy-restart controls above stay green.
+    #[test]
+    fn secrets_unavailable_is_never_a_candidate() {
+        assert!(
+            !should_restart_on_config_change(false, true, false, true),
+            "an unblocked (NotReady → Ready) agent with unavailable secrets must NOT be restarted"
+        );
+        assert!(
+            !should_restart_on_config_change(true, true, true, true),
+            "a running agent with changed env but unavailable secrets must NOT be restarted"
+        );
+    }
 }

desktop/src-tauri/src/managed_agents/custom_harnesses.rs

@@ -592,6 +592,32 @@ pub(crate) fn unavailable_harness_id(
         .map(|def| def.id.clone())
 }
 
+/// Whether ANY secret tier for `record` is currently unavailable — its ref is
+/// present but could not be hydrated from the keyring (missing, unreadable, or
+/// malformed generation). ORs the three registry-derivable tiers that every
+/// fail-closed spawn seam already consults individually:
+///
+/// - the instance tier (`record.secrets_unavailable`),
+/// - the definition tier ([`crate::managed_agents::unavailable_definition_id`]),
+/// - the harness tier ([`unavailable_harness_id`]).
+///
+/// This is the single predicate the restart-eligibility boundaries call before
+/// any `stop_managed_agent_process`: an unavailable record's hydrated env is
+/// empty and therefore indistinguishable from an intentionally-empty one at the
+/// `agent_readiness`/`resolve_effective_agent_env` layer, so those raw signals
+/// cannot be trusted to gate a destructive stop-then-respawn. Gating here keeps
+/// a running/setup process alive rather than stopping it and only discovering
+/// the refusal at respawn (`FailedAfterStop`). The global tier is not
+/// registry-derivable from a record and is covered by the spawn-time gate.
+pub(crate) fn effective_secrets_unavailable(
+    record: &ManagedAgentRecord,
+    personas: &[AgentDefinition],
+) -> bool {
+    record.secrets_unavailable
+        || crate::managed_agents::unavailable_definition_id(record, personas).is_some()
+        || unavailable_harness_id(record, personas).is_some()
+}
+
 /// Warm the loaded-harness registry synchronously from `custom_dir`.
 ///
 /// Must be called **before** `restore_managed_agents_on_launch` so that cold

desktop/src-tauri/src/managed_agents/mod.rs

@@ -58,7 +58,7 @@ pub(crate) fn lock_path_mutex() -> std::sync::MutexGuard<'static, ()> {
 }
 
 pub use backend::*;
-pub(crate) use custom_harnesses::unavailable_harness_id;
+pub(crate) use custom_harnesses::{effective_secrets_unavailable, unavailable_harness_id};
 pub(crate) use definition_validation::{
     validate_agent_definition_text, validate_managed_agent_definition_text,
 };

desktop/src-tauri/src/managed_agents/runtime_commands_tests.rs

@@ -473,3 +473,82 @@ fn harness_available_spawn_predicate_does_not_fire() {
         "spawn must not refuse a record whose effective harness is available"
     );
 }
+
+// ── effective_secrets_unavailable: the restart-eligibility predicate ─────────
+//
+// The restart-eligibility boundaries (post-install bounce, global-config
+// bounce) gate on this single predicate, which ORs the three
+// registry-derivable tiers. These tests prove each tier flips it independently
+// and a fully-healthy record does not, so the restart flows fail closed on ANY
+// unavailable tier — not just the harness one.
+
+use crate::managed_agents::effective_secrets_unavailable;
+
+#[test]
+fn effective_secrets_unavailable_false_for_healthy_record() {
+    // Positive control: a linked record with an available definition and no
+    // harness pin (builtin) has no unavailable tier — the predicate is false,
+    // so the negative-tier assertions below are meaningful.
+    let _lock = registry_test_lock();
+    update_loaded_harness_registry(vec![]);
+
+    let record = linked_record();
+    let def = available_definition();
+
+    let out = effective_secrets_unavailable(&record, std::slice::from_ref(&def));
+
+    assert!(
+        !out,
+        "a record with all tiers available must not be flagged unavailable"
+    );
+}
+
+#[test]
+fn effective_secrets_unavailable_true_for_instance_tier() {
+    // Instance tier: the record's own secrets_unavailable set (its env_vars_ref
+    // failed to hydrate). Mutation check: drop `record.secrets_unavailable`
+    // from the OR and this fails.
+    let _lock = registry_test_lock();
+    update_loaded_harness_registry(vec![]);
+
+    let mut record = linked_record();
+    record.secrets_unavailable = true;
+    let def = available_definition();
+
+    assert!(
+        effective_secrets_unavailable(&record, std::slice::from_ref(&def)),
+        "an instance whose own secrets are unavailable must fire the predicate"
+    );
+}
+
+#[test]
+fn effective_secrets_unavailable_true_for_definition_tier() {
+    // Definition tier: a linked persona whose secrets_unavailable is set.
+    // Mutation check: drop the `unavailable_definition_id(..).is_some()` arm.
+    let _lock = registry_test_lock();
+    update_loaded_harness_registry(vec![]);
+
+    let record = linked_record();
+    let def = unavailable_definition();
+
+    assert!(
+        effective_secrets_unavailable(&record, std::slice::from_ref(&def)),
+        "a record linked to a definition with unavailable secrets must fire the predicate"
+    );
+}
+
+#[test]
+fn effective_secrets_unavailable_true_for_harness_tier() {
+    // Harness tier: the effective harness carries an unhydratable env_ref.
+    // Mutation check: drop the `unavailable_harness_id(..).is_some()` arm.
+    let _lock = registry_test_lock();
+    update_loaded_harness_registry(vec![harness_def(true)]);
+
+    let out = effective_secrets_unavailable(&ready_harness_record(), &[]);
+
+    update_loaded_harness_registry(vec![]);
+    assert!(
+        out,
+        "a record whose effective harness env is unavailable must fire the predicate"
+    );
+}