From 3ca3a6711f188096efec7724ba72e7336d70d740 Mon Sep 17 00:00:00 2001 From: Devika Verma Date: Mon, 28 Sep 2026 13:32:47 +0530 Subject: [PATCH] Sandbox: a provider change rebuilds live sessions built under the old rule (ISSUE 10) Seen 2026-09-28 on WSL: the UI reconnected three sessions 12 s before the user turned the sandbox on, so their engines were built direct; after the switch the page said "sessions are refused" while those sessions kept running commands on the host with the user's full access. - `SessionManager.apply_sandbox_setting()`: every live engine whose workspace provider differs from the one a new engine would get is dropped (its sandbox closed) so the next connection rebuilds it under the new rule, and is refused if that rule cannot be met; a running turn is marked stale and rebuilt when it ends (`_refresh_session_tools`). - `POST /v1/settings/sandbox` calls it when `provider` is in the body and returns `rebuilt_sessions`. - The app: SettingsView reports them (`onSandboxProviderChanged`); App bumps `connectNonce` when the session on screen is among them, so its socket reconnects and the server rebuilds (or refuses) it at once. Co-Authored-By: Claude Fable 5.1 --- coworker/server/app.py | 7 ++- coworker/server/manager.py | 35 ++++++++++--- surfaces/gui/src/App.tsx | 5 ++ surfaces/gui/src/api.ts | 4 +- .../components/SettingsView.sandbox.test.tsx | 7 ++- surfaces/gui/src/components/SettingsView.tsx | 9 +++- tests/test_sandbox_settings.py | 50 +++++++++++++++++++ 7 files changed, 105 insertions(+), 12 deletions(-) diff --git a/coworker/server/app.py b/coworker/server/app.py index c48b1e06..a5a365e0 100644 --- a/coworker/server/app.py +++ b/coworker/server/app.py @@ -2403,7 +2403,12 @@ def create_app(manager: SessionManager) -> FastAPI: def settings_set_sandbox(body: dict) -> dict[str, Any]: from ..sandbox import settings as sandbox_settings - return sandbox_settings.update(body or {}) + result = sandbox_settings.update(body or {}) + if result.get("ok") and "provider" in (body or {}): + # Live sessions built under the old rule are rebuilt on their next connection; + # the app reconnects the one on screen (see App.tsx, onSandboxProviderChanged). + result["rebuilt_sessions"] = manager.apply_sandbox_setting() + return result @app.get("/v1/settings/sandbox/readiness") async def settings_sandbox_readiness() -> dict[str, Any]: diff --git a/coworker/server/manager.py b/coworker/server/manager.py index a1993419..5d0e6ec5 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -698,12 +698,9 @@ class SessionManager: return record.workspace or None return self.resolve_workspace(workspace) - def pending_sandbox_build(self, session_id: str) -> str: - """The sandbox provider a NEW engine for this session would be built with, or "" - when the engine exists already or commands would run directly. Reads the config - only, no preflight: it is asked on the event loop, before the build (OPE-206).""" - if session_id in self._engines: - return "" + def _wanted_sandbox_provider(self) -> str: + """The provider a NEW engine would be built with right now, or "" for direct. + Reads the config only, no preflight: it is asked on the event loop.""" from ..sandbox.selection import is_headless from ..sandbox.workspace import DIRECT, OPENSHELL, PROVIDER_ENV @@ -712,6 +709,32 @@ class SessionManager: return "" if chosen == DIRECT else chosen return OPENSHELL if is_headless() else "" + def pending_sandbox_build(self, session_id: str) -> str: + """The sandbox provider a NEW engine for this session would be built with, or "" + when the engine exists already or commands would run directly (OPE-206).""" + if session_id in self._engines: + return "" + return self._wanted_sandbox_provider() + + def apply_sandbox_setting(self) -> list[str]: + """Settings ▸ Sandbox changed the provider: every live engine built under another + provider is dropped, so its next connection rebuilds it under the new rule (and + is refused if that rule cannot be met). A running turn keeps its engine and is + rebuilt when it ends. Returns the affected session ids. Without this, a session + opened before the switch kept running with no wall while the page said sessions + were refused (seen 2026-09-28 on WSL).""" + wanted = self._wanted_sandbox_provider() + affected: list[str] = [] + for session_id, engine in list(self._engines.items()): + workspace = getattr(engine, "sandbox_workspace", None) + describe = getattr(workspace, "describe", None) + built = str((describe() if callable(describe) else {}).get("provider") or "direct") + if built == (wanted or "direct"): + continue + self._refresh_session_tools(session_id) + affected.append(session_id) + return affected + def _engine_build_lock(self, session_id: str) -> threading.Lock: with self._engine_build_locks_guard: return self._engine_build_locks.setdefault(session_id, threading.Lock()) diff --git a/surfaces/gui/src/App.tsx b/surfaces/gui/src/App.tsx index 858b5055..3dcbd729 100644 --- a/surfaces/gui/src/App.tsx +++ b/surfaces/gui/src/App.tsx @@ -2047,6 +2047,11 @@ export function App() { key={settingsTab} initialTab={settingsTab} onBack={() => setSurface("session")} + onSandboxProviderChanged={(ids) => { + // The session on screen was built under the old sandbox rule: reconnect, so + // the server rebuilds it under the new one (or refuses it with the reason). + if (ids.includes(sessionId)) setConnectNonce((n) => n + 1); + }} onOpenPersona={(id, machineId) => openPersona(id, "settings", machineId)} onAskWorker={(machineId) => { // Memory's remote CTA: the conversation IS the edit surface — a diff --git a/surfaces/gui/src/api.ts b/surfaces/gui/src/api.ts index 8a4aa016..5f6094fe 100644 --- a/surfaces/gui/src/api.ts +++ b/surfaces/gui/src/api.ts @@ -2119,7 +2119,9 @@ export async function getSandboxSettings(machineId?: string | null): Promise>, machineId?: string | null, -): Promise<{ ok: boolean; error?: string } & Partial> { +): Promise<{ ok: boolean; error?: string; rebuilt_sessions?: string[] } & Partial> { + // `rebuilt_sessions`: after a provider change, the sessions whose engine the server + // dropped so their next connection rebuilds them under the new rule. const res = await fetch(`${engineBase(machineId)}/v1/settings/sandbox`, { method: "POST", headers: { "Content-Type": "application/json" }, diff --git a/surfaces/gui/src/components/SettingsView.sandbox.test.tsx b/surfaces/gui/src/components/SettingsView.sandbox.test.tsx index f9a5985b..447c6c9b 100644 --- a/surfaces/gui/src/components/SettingsView.sandbox.test.tsx +++ b/surfaces/gui/src/components/SettingsView.sandbox.test.tsx @@ -42,8 +42,10 @@ let setupState: any = { status: "idle", rows: [], progress: null, error: "", ela // Like the backend: each save returns the whole (updated) snapshot. const setSandboxSettings = vi.fn(async (patch: any) => { snapshot = { ...snapshot, ...patch }; - return { ok: true, ...snapshot }; + // Like the backend: a provider change names the sessions it dropped for a rebuild. + return { ok: true, ...snapshot, ...("provider" in patch ? { rebuilt_sessions: ["s-open"] } : {}) }; }); +const onSandboxProviderChanged = vi.fn(); const startSandboxSetup = vi.fn(async () => setupState); vi.mock("../api", async (importOriginal) => { @@ -89,10 +91,11 @@ describe("Settings ▸ Sandbox", () => { }); it("switching on picks the provider that needs no setup on a Mac, and off clears the key", async () => { - render(); + render(); await screen.findByTestId("sandbox-section"); fireEvent.click(screen.getByTestId("sandbox-switch")); await waitFor(() => expect(setSandboxSettings).toHaveBeenCalledWith({ provider: "seatbelt" })); + await waitFor(() => expect(onSandboxProviderChanged).toHaveBeenCalledWith(["s-open"])); // the app reconnects that session // Now on: the provider line (a dropdown on a Mac) and the sub-settings appear. expect((screen.getByTestId("sandbox-provider-select") as HTMLSelectElement).value).toBe("seatbelt"); expect(screen.getByTestId("sandbox-network-section")).toBeTruthy(); diff --git a/surfaces/gui/src/components/SettingsView.tsx b/surfaces/gui/src/components/SettingsView.tsx index 5fa793db..ffccd2dd 100644 --- a/surfaces/gui/src/components/SettingsView.tsx +++ b/surfaces/gui/src/components/SettingsView.tsx @@ -178,8 +178,12 @@ export function SettingsView({ onCreateSkill, onAskWorker, onBack, + onSandboxProviderChanged, }: { initialTab?: SetTab; + // Sandbox: the provider changed and the server dropped these sessions' engines; the + // app reconnects the one on screen so it is rebuilt (or refused) under the new rule. + onSandboxProviderChanged?: (sessionIds: string[]) => void; // "Back to app" on the rail — returns to the conversation surface. onBack?: () => void; onOpenPersona?: (id: string, machineId?: string | null) => void; @@ -402,7 +406,7 @@ export function SettingsView({ onAskWorker={(machineId) => onAskWorker?.(machineId)} /> ) : tab === "sandbox" ? ( - + ) : tab === "machines" ? ( ) : tab === "slack" || tab === "github" ? ( @@ -737,7 +741,7 @@ function PersonasSection({ // what to do. const SANDBOX_PROVIDERS = ["openshell", "seatbelt"] as const; -function SandboxSection({ machine }: { machine?: Machine | null }) { +function SandboxSection({ machine, onProviderChanged }: { machine?: Machine | null; onProviderChanged?: (sessionIds: string[]) => void }) { const { t } = useTranslation(); const mid = machine?.id ?? null; const [cfg, setCfg] = useState(null); @@ -760,6 +764,7 @@ function SandboxSection({ machine }: { machine?: Machine | null }) { } setError(""); setCfg(res as SandboxSettings); + if ("provider" in patch) onProviderChanged?.(res.rebuilt_sessions ?? []); }; // The switch reads the machine's choice; a headless machine's default rule (OpenShell diff --git a/tests/test_sandbox_settings.py b/tests/test_sandbox_settings.py index 60b6e303..b60f621b 100644 --- a/tests/test_sandbox_settings.py +++ b/tests/test_sandbox_settings.py @@ -100,3 +100,53 @@ def test_an_explicit_provider_that_cannot_be_used_shows_as_refused(config_file, assert not next(p for p in snap["providers"] if p["name"] == "openshell")["usable"] with pytest.raises(OpenShellUnavailable): selection.select("openshell") + + +def test_changing_the_provider_rebuilds_live_engines_built_under_the_old_rule(tmp_path, monkeypatch): + """Seen 2026-09-28 on WSL: sessions opened before the switch kept running direct while + the page said sessions were refused. A provider change now drops idle engines built + under another provider (a running one is rebuilt when its turn ends) and the response + names them, so the app reconnects the one on screen.""" + from fastapi.testclient import TestClient + + from coworker.sandbox import settings as sandbox_settings + from coworker.server import create_app + from tests.test_persona_connections import _mgr + + monkeypatch.setattr(sandbox_settings, "openshell_problem", lambda fresh=False: "OpenShell is not installed", raising=False) + from coworker.sandbox import selection + + monkeypatch.setattr(selection, "openshell_problem", lambda fresh=False: "OpenShell is not installed") + mgr = _mgr(tmp_path, monkeypatch) + + class Direct: + def describe(self): + return {"provider": "direct", "enforcement": "none"} + + class Boxed: + def describe(self): + return {"provider": "openshell", "enforcement": "full"} + + class Engine: + def __init__(self, ws): + self.sandbox_workspace = ws + + closed: list[str] = [] + mgr._engines.update({"direct-idle": Engine(Direct()), "direct-busy": Engine(Direct()), "boxed": Engine(Boxed())}) + monkeypatch.setattr(mgr, "_close_sandbox", lambda engine: closed.append(type(engine.sandbox_workspace).__name__)) + mgr.mark_running("direct-busy") + client = TestClient(create_app(mgr)) + res = client.post("/v1/settings/sandbox", json={"provider": "openshell"}).json() + assert res["ok"] and sorted(res["rebuilt_sessions"]) == ["direct-busy", "direct-idle"] + assert "direct-idle" not in mgr._engines # dropped now: its next connection is rebuilt (here: refused) + assert "direct-busy" in mgr._engines # mid-turn: kept until the turn ends + assert "boxed" in mgr._engines # already under the new rule + mgr.mark_idle("direct-busy") + assert "direct-busy" not in mgr._engines + # Switching off: the boxed engine is the odd one out now, and its sandbox is closed. + res = client.post("/v1/settings/sandbox", json={"provider": ""}).json() + assert res["ok"] and res["rebuilt_sessions"] == ["boxed"] and closed[-1] == "Boxed" + # A save that does not touch the provider rebuilds nothing. + mgr._engines["d"] = Engine(Direct()) + res = client.post("/v1/settings/sandbox", json={"network_profile": "strict"}).json() + assert res["ok"] and "rebuilt_sessions" not in res and "d" in mgr._engines