fix(codebuddy): bound buffered tool calls

bin/ocx.mjs

@@ -42,6 +42,7 @@ import {
   resolvePnpmGlobalOwner,
   runPnpmGlobalUpdate,
 } from "../src/update/pnpm-global-install.mjs";
+import { PNPM_READ_CWD, withPnpmCommandCwd, pnpmReadEnvironment } from "../src/update/pnpm-read-policy.mjs";
 import { checkRegistryPackageIntegrity } from "../src/update/registry-integrity.mjs";
 import { hasPendingTeardownIn } from "../src/config/pending-teardown-names.mjs";
 import {
@@ -208,6 +209,8 @@ function runPackageManagerSelfUpdate(manager) {
           encoding: "utf8",
           timeout: 20_000,
           windowsHide: true,
+          cwd: PNPM_READ_CWD,
+          env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(process.env)),
           ...invocation.options,
         });
       },
@@ -221,6 +224,20 @@ function runPackageManagerSelfUpdate(manager) {
   const managerInvocation = args => manager === "pnpm"
     ? pnpmOwnerInvocation(owner, args)
     : npmInvocation(args);
+  // Read-only pnpm probes run from the installed package directory with project pnpmfiles
+  // disabled, so an attacker-controlled cwd cannot execute hooks during the update check.
+  const readProbeOptions = invocation => ({
+    encoding: "utf8",
+    timeout: 12000,
+    windowsHide: true,
+    ...(manager === "pnpm"
+      ? {
+        cwd: PNPM_READ_CWD,
+        env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(invocation.env ?? process.env)),
+      }
+      : invocation.env ? { env: invocation.env } : {}),
+    ...invocation.options,
+  });
   const latestInvocation = managerInvocation(["view", `${PKG}@${tag}`, "version"]);
   const installArgs = manager === "pnpm"
     ? ["add", "-g", "--allow-build=bun", `${PKG}@${tag}`]
@@ -230,13 +247,7 @@ function runPackageManagerSelfUpdate(manager) {
     console.error(`opencodex: could not resolve ${manager} from a trusted absolute PATH entry; aborting before stopping the proxy.`);
     process.exit(1);
   }
-  const latestResult = spawnSync(latestInvocation.file, latestInvocation.args, {
-    encoding: "utf8",
-    timeout: 12000,
-    windowsHide: true,
-    ...(latestInvocation.env ? { env: latestInvocation.env } : {}),
-    ...latestInvocation.options,
-  });
+  const latestResult = spawnSync(latestInvocation.file, latestInvocation.args, readProbeOptions(latestInvocation));
   const latest = latestResult.status === 0 && typeof latestResult.stdout === "string" ? latestResult.stdout.trim() : "";
 
   console.log(`opencodex v${current} (installed via ${manager}, tag ${tag})`);
@@ -248,13 +259,7 @@ function runPackageManagerSelfUpdate(manager) {
   const integrity = checkRegistryPackageIntegrity(PKG, latest || null, args => {
     const invocation = managerInvocation(args);
     if (!invocation) return { status: 1 };
-    return spawnSync(invocation.file, invocation.args, {
-      encoding: "utf8",
-      timeout: 12000,
-      windowsHide: true,
-      ...(invocation.env ? { env: invocation.env } : {}),
-      ...invocation.options,
-    });
+    return spawnSync(invocation.file, invocation.args, readProbeOptions(invocation));
   });
   if (integrity.ok === false) {
     console.error(`opencodex: ${integrity.reason}; aborting before stopping the proxy.`);
@@ -768,14 +773,17 @@ function runPackageManagerSelfUpdate(manager) {
           runPnpm: (args, capture = false) => {
             const invocation = pnpmOwnerInvocation(owner, args);
             if (!invocation) return { status: 1 };
-            return spawnSync(invocation.file, invocation.args, {
+            return withPnpmCommandCwd(args, cwd => spawnSync(invocation.file, invocation.args, {
               ...invocation.options,
               stdio: capture ? "pipe" : "inherit",
               encoding: "utf8",
               timeout: 180000,
               windowsHide: true,
-              env: unprivilegedOwnershipMutationEnvironment(invocation.env ?? process.env),
-            });
+              // Reads probe from the package dir; mutations (add -g, rollback) must not
+              // keep a cwd handle inside the package Windows is replacing.
+              cwd,
+              env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(invocation.env ?? process.env)),
+            }));
           },
           log: line => console.log(line),
         });

desktop/src-tauri/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "opencodex-desktop"
-version = "2.68.0"
+version = "2.69.0"
 description = "OpenCodex desktop shell"
 authors = ["OpenCodex contributors"]
 license = "MIT"

desktop/src-tauri/src/exit.rs

@@ -146,6 +146,14 @@ pub struct Supervision {
     pub reason_set: bool,
 }
 
+/// State restored when an update's coordinated restart is abandoned.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub struct AbortedRestart {
+    pub phase: ExitPhase,
+    /// Whether the person wanted a runtime before the drain or requested one while it ran.
+    pub runtime_was_wanted: bool,
+}
+
 impl Supervision {
     /// Nothing is in flight, nobody asked for the runtime to stop, and the app is not ending.
     pub fn allowed(self) -> bool {
@@ -161,6 +169,14 @@ struct Inner {
     deferred: bool,
     /// See [`Supervision::wanted`]. Sticky: finishing a stop does not restore it.
     wanted: bool,
+    /// The intent an update temporarily replaced with `wanted=false`; consumed if it aborts.
+    restart_wanted: Option<bool>,
+}
+
+fn remember_restart_intent(inner: &mut Inner, reason: ExitReason, prior_wanted: bool) {
+    if reason == ExitReason::CoordinatedRestart {
+        inner.restart_wanted.get_or_insert(prior_wanted);
+    }
 }
 
 /// The exit sequence's state, managed by the app.
@@ -179,6 +195,7 @@ impl ExitCoordinator {
                 hides_to_tray: TrayAvailability::assumed().hides_to_tray(),
                 deferred: false,
                 wanted: true,
+                restart_wanted: None,
             }),
         }
     }
@@ -215,17 +232,20 @@ impl ExitCoordinator {
     /// [`ExitCoordinator::finish_stop`] is holding the phase.
     pub fn claim_drain(&self, fallback: ExitReason) -> Option<ExitReason> {
         let mut inner = self.inner();
+        let prior_wanted = inner.wanted;
         // A quit or an update is on its way, whoever ends up running the drain: the runtime it
         // stops is not one to bring back.
         inner.wanted = false;
         match inner.phase {
             ExitPhase::Idle => {
                 let reason = *inner.reason.get_or_insert(fallback);
+                remember_restart_intent(&mut inner, reason, prior_wanted);
                 inner.phase = ExitPhase::Draining;
                 Some(reason)
             }
             ExitPhase::Spawning | ExitPhase::Stopping => {
-                inner.reason.get_or_insert(fallback);
+                let reason = *inner.reason.get_or_insert(fallback);
+                remember_restart_intent(&mut inner, reason, prior_wanted);
                 inner.deferred = true;
                 None
             }
@@ -235,6 +255,7 @@ impl ExitCoordinator {
             // is the one thing a user with a runtime that would not stop cannot easily do.
             ExitPhase::DrainFailed | ExitPhase::OwnershipUnknown => {
                 let reason = *inner.reason.get_or_insert(fallback);
+                remember_restart_intent(&mut inner, reason, prior_wanted);
                 inner.phase = ExitPhase::Draining;
                 Some(reason)
             }
@@ -256,10 +277,12 @@ impl ExitCoordinator {
     /// An update drains before it installs. When the install then fails, or the drain itself did,
     /// the drain's phase used to be the end of the road: `Drained` is terminal, so no runtime could
     /// be started again and a bare window close quit the app. This returns the app to `Idle` with
-    /// no claimed reason and wants a runtime again, which is what a successful update would have
-    /// ended in too. It touches nothing unless an update's restart holds the phase: a quit is never
-    /// aborted, and a drain still running belongs to whoever runs it. Returns the phase it left.
-    pub fn abort_restart(&self) -> Option<ExitPhase> {
+    /// no claimed reason and restores the runtime intent the update temporarily suppressed. A
+    /// retry requested while the drain was in flight wins too: aborting an older update must not
+    /// overwrite newer user intent. It touches nothing unless an update's restart holds the phase:
+    /// a quit is never aborted, and a
+    /// drain still running belongs to whoever runs it. Returns the phase and restored intent.
+    pub fn abort_restart(&self) -> Option<AbortedRestart> {
         let mut inner = self.inner();
         let left = inner.phase;
         let restart = inner.reason == Some(ExitReason::CoordinatedRestart);
@@ -273,8 +296,15 @@ impl ExitCoordinator {
         inner.phase = ExitPhase::Idle;
         inner.reason = None;
         inner.deferred = false;
-        inner.wanted = true;
-        Some(left)
+        // `resume` can arrive after the update captured its original intent. Preserve that newer
+        // request as well as the older snapshot; otherwise the abort races the startup retry and
+        // can leave a runtime stopped even though the person just asked for it.
+        let runtime_was_wanted = inner.wanted || inner.restart_wanted.take().unwrap_or(false);
+        inner.wanted = runtime_was_wanted;
+        Some(AbortedRestart {
+            phase: left,
+            runtime_was_wanted,
+        })
     }
 
     /// What the runtime supervisor reads before it acts.
@@ -293,7 +323,14 @@ impl ExitCoordinator {
 
     /// A person asked for a runtime again (the startup page's retry).
     pub fn resume(&self) {
-        self.inner().wanted = true;
+        let mut inner = self.inner();
+        inner.wanted = true;
+        // A pending update snapshot must learn about the request too. A retried install that finds
+        // the drain already settled clears `wanted` again without replacing the snapshot, so a
+        // retry recorded only in `wanted` would be lost when that install fails and aborts.
+        if let Some(snapshot) = inner.restart_wanted.as_mut() {
+            *snapshot = true;
+        }
     }
 
     /// Reserve the right to start a runtime. False once something else owns the phase.
@@ -624,7 +661,7 @@ fn hide_windows(app: &AppHandle) {
 #[cfg(test)]
 mod tests {
     use super::{
-        decide, DrainVerdict, ExitCoordinator, ExitDecision, ExitPhase, ExitReason,
+        decide, AbortedRestart, DrainVerdict, ExitCoordinator, ExitDecision, ExitPhase, ExitReason,
         RestartReadiness, Supervision,
     };
     use crate::tray_availability::TrayAvailability;
@@ -707,7 +744,13 @@ mod tests {
             );
             coordinator.finish_drain(verdict);
             let left = coordinator.phase();
-            assert_eq!(coordinator.abort_restart(), Some(left));
+            assert_eq!(
+                coordinator.abort_restart(),
+                Some(AbortedRestart {
+                    phase: left,
+                    runtime_was_wanted: true,
+                })
+            );
             assert_eq!(coordinator.phase(), ExitPhase::Idle);
             // A bare close hides again instead of quitting out of a terminal phase.
             assert_eq!(coordinator.decision(), ExitDecision::Hide);
@@ -716,6 +759,85 @@ mod tests {
         }
     }
 
+    #[test]
+    fn a_failed_update_preserves_a_completed_tray_stop() {
+        let coordinator = ExitCoordinator::new();
+        coordinator.set_tray(TrayAvailability::Available);
+        assert!(coordinator.begin_stop());
+        assert_eq!(coordinator.finish_stop(), None);
+        assert!(!coordinator.supervision().wanted);
+
+        assert_eq!(
+            coordinator.claim_drain(ExitReason::CoordinatedRestart),
+            Some(ExitReason::CoordinatedRestart)
+        );
+        coordinator.finish_drain(DrainVerdict::Drained);
+        assert_eq!(
+            coordinator.abort_restart(),
+            Some(AbortedRestart {
+                phase: ExitPhase::Drained,
+                runtime_was_wanted: false,
+            })
+        );
+        assert_eq!(coordinator.phase(), ExitPhase::Idle);
+        assert_eq!(coordinator.decision(), ExitDecision::Hide);
+        assert!(!coordinator.supervision_allowed());
+    }
+
+    #[test]
+    fn a_startup_retry_during_an_update_drain_is_not_overwritten_by_abort() {
+        let coordinator = ExitCoordinator::new();
+        coordinator.set_tray(TrayAvailability::Available);
+        assert!(coordinator.begin_stop());
+        assert_eq!(coordinator.finish_stop(), None);
+        assert!(!coordinator.supervision().wanted);
+
+        assert_eq!(
+            coordinator.claim_drain(ExitReason::CoordinatedRestart),
+            Some(ExitReason::CoordinatedRestart)
+        );
+        coordinator.resume();
+        // The ending claim still prevents supervision until the failed update is handed back.
+        assert!(!coordinator.supervision_allowed());
+        coordinator.finish_drain(DrainVerdict::Drained);
+        assert_eq!(
+            coordinator.abort_restart(),
+            Some(AbortedRestart {
+                phase: ExitPhase::Drained,
+                runtime_was_wanted: true,
+            })
+        );
+        assert!(coordinator.supervision_allowed());
+    }
+
+    #[test]
+    fn a_retry_between_update_attempts_survives_the_second_claim() {
+        let coordinator = ExitCoordinator::new();
+        coordinator.set_tray(TrayAvailability::Available);
+        assert!(coordinator.begin_stop());
+        assert_eq!(coordinator.finish_stop(), None);
+
+        assert_eq!(
+            coordinator.claim_drain(ExitReason::CoordinatedRestart),
+            Some(ExitReason::CoordinatedRestart)
+        );
+        coordinator.finish_drain(DrainVerdict::Drained);
+        coordinator.resume();
+        // The next install attempt finds the drain settled and clears `wanted` again.
+        assert_eq!(
+            coordinator.claim_drain(ExitReason::CoordinatedRestart),
+            None
+        );
+        assert_eq!(
+            coordinator.abort_restart(),
+            Some(AbortedRestart {
+                phase: ExitPhase::Drained,
+                runtime_was_wanted: true,
+            })
+        );
+        assert!(coordinator.supervision_allowed());
+    }
+
     #[test]
     fn a_quit_or_a_drain_in_flight_is_never_aborted() {
         let coordinator = ExitCoordinator::new();

desktop/src-tauri/src/native_tray.rs

@@ -334,11 +334,21 @@ fn publish(app: &AppHandle, generation: u64, binding: Option<RuntimeBinding>, sn
             *state
                 .cache
                 .lock()
-                .unwrap_or_else(std::sync::PoisonError::into_inner) = (binding, snapshot);
+                .unwrap_or_else(std::sync::PoisonError::into_inner) = (binding, cached(snapshot));
         }
     });
 }
 
+/// The cached copy every later refresh starts from. `switchFailed` answers one switch, so it is
+/// delivered once and never cached: carried forward, it would settle the next switch's spinner on
+/// that switch's first loading publish.
+fn cached(mut snapshot: Value) -> Value {
+    if let Some(fields) = snapshot.as_object_mut() {
+        fields.remove("switchFailed");
+    }
+    snapshot
+}
+
 fn display_payload(snapshot: Value) -> Option<(Value, Vec<u8>)> {
     let bytes = serde_json::to_vec(&snapshot).ok()?;
     if bytes.len() <= 8 * 1024 * 1024 {
@@ -406,6 +416,11 @@ mod tests {
     }
     #[test]
     fn refresh_failure_preserves_age_and_clears_busy_state() {
+        let reported = json!({"refreshing":false,"errors":["refused"],"switchFailed":true});
+        let kept = cached(reported);
+        assert!(kept.get("switchFailed").is_none());
+        assert_eq!(kept["errors"], json!(["refused"]));
+
         let before = json!({"updatedAt":12,"refreshing":true,"today":{"totalTokens":30}});
         let after = failed(before, "Unavailable");
         assert_eq!(after["updatedAt"], 12);