fix: sandbox enable toggle and Linux status detection

[seannair]

Summary

Two related but separate fixes to the Sandbox settings panel (SettingsSandbox.tsx):

1. Sandbox enable toggle was missing entirely. The panel displayed status correctly (WSL2/Lima detection, Node/Python/pip checks all worked), but there was no way to actually turn Sandbox Mode on or off — handleToggleSandbox existed only as a commented-out // TODO: Re-enable when sandbox debugging is complete stub, with no UI control wired to it at all. Backend support (config.save IPC handler → configStore.update → syncConfigAfterMutation → sessionManager.reloadSandbox()) was already fully implemented and required no changes. This affects all platforms (Windows/WSL2, macOS/Lima, Linux), since it's the same shared component.

2. sandboxReady/sandboxAvailable were hardcoded to false on native Linux hosts. Separately, while testing the toggle fix by running the dev build directly on Linux (as opposed to Windows using WSL2 as a backend — a different code path), the status always read "Sandbox enabled but not fully configured" even though Linux native mode has no further setup step (per the existing sandbox.linuxNative copy: "Linux runs commands natively without additional sandboxing"). Fixed by treating Linux as always available/ready, matching intended behavior — this does not affect the Windows/WSL2 or macOS/Lima detection logic, which were already correct.

Related

• Sandbox reports "Coming Soon" even though code page implies feature already exists #179 (Sandbox reports "Coming Soon" even though page implies feature already exists)
• Re-enable sandbox, remote channels, and MCP server management #46 (unmerged attempt to re-enable sandbox)
• fix: show sandbox settings status #188 (merged fix for the status display, but didn't restore the toggle itself)

Testing

• Verified on Windows 11 + WSL2 (Ubuntu): toggle now switches Sandbox Mode on/off, status correctly reports "Sandbox ready and running" with WSL2/Node/Python/pip all green.
• Verified on native Linux (WSL Ubuntu run directly, non-Windows code path): status now correctly reports ready instead of a permanent false warning.
• tsc --noEmit and eslint pass with no new errors.

[github-actions]

Review mode: initial

Findings

• [Minor] Success message timeout race condition
  When the user toggles sandbox quickly within 3 seconds, the previous setTimeout(() => setSuccess(null), 3000) may fire after the new success message is set, prematurely clearing it. This happens because the timeout is not cleared before setting a new one.
  Suggested fix: Store the timeout ID in a ref and clear it in the cleanup logic of handleToggleSandbox.

  const successTimeoutRef = useRef<NodeJS.Timeout>();
  // In handleToggleSandbox, before setting new timeout:
  if (successTimeoutRef.current) clearTimeout(successTimeoutRef.current);
  successTimeoutRef.current = setTimeout(() => setSuccess(null), 3000);

• [Nit] Missing guard for non-Electron environments
  handleToggleSandbox assumes window.electronAPI.config.save exists. If the component were ever rendered outside Electron (e.g., in a browser), clicking the toggle would throw. Consider adding a guard similar to the initialization effect.
  Suggested fix: Add early return if !isElectron.

  if (!isElectron) return;

Questions

• The PR description mentions fixing sandboxReady/sandboxAvailable being hardcoded to false on native Linux, but no such change appears in the diff. Was this fix already in the base branch, or is it intended for a separate PR?

Summary

• The toggle implementation is generally correct: optimistic UI disabled during saving, error handling, and accessibility attributes are properly applied.
• The 500ms delay for status reload is reasonable but may need adjustment if the backend takes longer in practice.
• The only significant issue is the success message race condition; the other items are minor or questions.
• Testing: Manual verification on Windows and Linux was described. Automated tests are not present in the diff; consider adding unit tests for handleToggleSandbox (e.g., mocking window.electronAPI).

Testing

• Not run (automation)

Open Cowork Bot

[github-actions]

Review mode: follow-up after new commits

Findings

• [Major] Inverted toggle button label
  The button label uses {sandboxEnabled ? t('common.enable') : t('common.disable')}. When sandbox is enabled, it shows "enable" (verb), which suggests a click would enable it again, but the actual action toggles off. It should show the opposite action: {sandboxEnabled ? t('common.disable') : t('common.enable')}.
  src/renderer/components/settings/SettingsSandbox.tsx:364
  Suggested fix:

  {sandboxEnabled ? t('common.disable') : t('common.enable')}

• [Minor] Uncleanup setTimeout for status reload
  In handleToggleSandbox, a setTimeout with 500ms delay calls loadStatus() but the timeout ID is never captured or cleared. If the component unmounts before the timeout fires (e.g., navigating away from settings), the callback will attempt to update state on an unmounted component, causing a React warning. Store the timeout in a ref and clear it on cleanup using useEffect or before subsequent toggles.
  src/renderer/components/settings/SettingsSandbox.tsx:126
  Suggested fix:

  // Add a ref next to successTimeoutRef
  const reloadTimeoutRef = useRef<ReturnType<typeof setTimeout>>();
  // In handleToggleSandbox after success
  if (reloadTimeoutRef.current) clearTimeout(reloadTimeoutRef.current);
  reloadTimeoutRef.current = setTimeout(async () => {
    await loadStatus();
  }, 500);
  // In a useEffect cleanup for unmount
  useEffect(() => {
    return () => {
      if (reloadTimeoutRef.current) clearTimeout(reloadTimeoutRef.current);
    };
  }, []);

• [Minor] Linux sandbox status assumed always true even when backend fails
  Setting sandboxAvailable and sandboxReady to true unconditionally for Linux ignores the possibility that the backend itself is broken or not initialized (e.g., status is null). Previously false was too restrictive; true is overly optimistic. Consider checking if status exists and if the backend reported an error, or treat as ready only when status?.initialized is true.
  src/renderer/components/settings/SettingsSandbox.tsx:314,320
  Suggested fix:

  : status?.initialized === true; // Linux: ready only if backend reports initialized

Previous feedback resolved

• All prior findings have been addressed: success message timeout race condition is fixed with ref and cleanup, and guard for non-Electron environments is added.

Testing

• Not run (automation)

Open Cowork Bot

[github-actions]

Review mode: follow-up after new commits

Findings

All previously flagged issues from the prior review have been satisfactorily addressed:

• The inverted toggle button text has been resolved (the text label displaying the action was removed; the switch alone with aria-label is sufficient).
• The reloadTimeoutRef is now captured, cleared before reuse, and cleaned up on unmount via a dedicated useEffect.
• The Linux sandbox availability and readiness now check status?.initialized === true instead of being hardcoded to true, aligning with the backend adapter's actual initialization state.

No new issues introduced.

Residual Risks

• The loadStatus() call inside the 500ms timeout does not protect against setting state after component unmount if the async operation completes after cleanup. However, the timeout is cleared on unmount, so this scenario is unlikely in practice and matches existing patterns in the codebase.
• No automated tests were added for the toggle behavior or Linux status detection. Consider adding unit or integration tests for the sandbox settings component.

Testing

• Not run (automation)

Open Cowork Bot