mirror of
https://github.com/andrewyng/openworker.git
synced 2026-11-15 14:48:42 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
8da4bc0ccb
commit
3ca3a6711f
@@ -2403,7 +2403,12 @@ def create_app(manager: SessionManager) -> FastAPI:
|
|||||||
def settings_set_sandbox(body: dict) -> dict[str, Any]:
|
def settings_set_sandbox(body: dict) -> dict[str, Any]:
|
||||||
from ..sandbox import settings as sandbox_settings
|
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")
|
@app.get("/v1/settings/sandbox/readiness")
|
||||||
async def settings_sandbox_readiness() -> dict[str, Any]:
|
async def settings_sandbox_readiness() -> dict[str, Any]:
|
||||||
|
|||||||
@@ -698,12 +698,9 @@ class SessionManager:
|
|||||||
return record.workspace or None
|
return record.workspace or None
|
||||||
return self.resolve_workspace(workspace)
|
return self.resolve_workspace(workspace)
|
||||||
|
|
||||||
def pending_sandbox_build(self, session_id: str) -> str:
|
def _wanted_sandbox_provider(self) -> str:
|
||||||
"""The sandbox provider a NEW engine for this session would be built with, or ""
|
"""The provider a NEW engine would be built with right now, or "" for direct.
|
||||||
when the engine exists already or commands would run directly. Reads the config
|
Reads the config only, no preflight: it is asked on the event loop."""
|
||||||
only, no preflight: it is asked on the event loop, before the build (OPE-206)."""
|
|
||||||
if session_id in self._engines:
|
|
||||||
return ""
|
|
||||||
from ..sandbox.selection import is_headless
|
from ..sandbox.selection import is_headless
|
||||||
from ..sandbox.workspace import DIRECT, OPENSHELL, PROVIDER_ENV
|
from ..sandbox.workspace import DIRECT, OPENSHELL, PROVIDER_ENV
|
||||||
|
|
||||||
@@ -712,6 +709,32 @@ class SessionManager:
|
|||||||
return "" if chosen == DIRECT else chosen
|
return "" if chosen == DIRECT else chosen
|
||||||
return OPENSHELL if is_headless() else ""
|
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:
|
def _engine_build_lock(self, session_id: str) -> threading.Lock:
|
||||||
with self._engine_build_locks_guard:
|
with self._engine_build_locks_guard:
|
||||||
return self._engine_build_locks.setdefault(session_id, threading.Lock())
|
return self._engine_build_locks.setdefault(session_id, threading.Lock())
|
||||||
|
|||||||
@@ -2047,6 +2047,11 @@ export function App() {
|
|||||||
key={settingsTab}
|
key={settingsTab}
|
||||||
initialTab={settingsTab}
|
initialTab={settingsTab}
|
||||||
onBack={() => setSurface("session")}
|
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)}
|
onOpenPersona={(id, machineId) => openPersona(id, "settings", machineId)}
|
||||||
onAskWorker={(machineId) => {
|
onAskWorker={(machineId) => {
|
||||||
// Memory's remote CTA: the conversation IS the edit surface — a
|
// Memory's remote CTA: the conversation IS the edit surface — a
|
||||||
|
|||||||
@@ -2119,7 +2119,9 @@ export async function getSandboxSettings(machineId?: string | null): Promise<San
|
|||||||
export async function setSandboxSettings(
|
export async function setSandboxSettings(
|
||||||
patch: Partial<Pick<SandboxSettings, "provider" | "network_profile" | "credentials">>,
|
patch: Partial<Pick<SandboxSettings, "provider" | "network_profile" | "credentials">>,
|
||||||
machineId?: string | null,
|
machineId?: string | null,
|
||||||
): Promise<{ ok: boolean; error?: string } & Partial<SandboxSettings>> {
|
): Promise<{ ok: boolean; error?: string; rebuilt_sessions?: string[] } & Partial<SandboxSettings>> {
|
||||||
|
// `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`, {
|
const res = await fetch(`${engineBase(machineId)}/v1/settings/sandbox`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers: { "Content-Type": "application/json" },
|
headers: { "Content-Type": "application/json" },
|
||||||
|
|||||||
@@ -42,8 +42,10 @@ let setupState: any = { status: "idle", rows: [], progress: null, error: "", ela
|
|||||||
// Like the backend: each save returns the whole (updated) snapshot.
|
// Like the backend: each save returns the whole (updated) snapshot.
|
||||||
const setSandboxSettings = vi.fn(async (patch: any) => {
|
const setSandboxSettings = vi.fn(async (patch: any) => {
|
||||||
snapshot = { ...snapshot, ...patch };
|
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);
|
const startSandboxSetup = vi.fn(async () => setupState);
|
||||||
|
|
||||||
vi.mock("../api", async (importOriginal) => {
|
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 () => {
|
it("switching on picks the provider that needs no setup on a Mac, and off clears the key", async () => {
|
||||||
render(<SettingsView initialTab="sandbox" />);
|
render(<SettingsView initialTab="sandbox" onSandboxProviderChanged={onSandboxProviderChanged} />);
|
||||||
await screen.findByTestId("sandbox-section");
|
await screen.findByTestId("sandbox-section");
|
||||||
fireEvent.click(screen.getByTestId("sandbox-switch"));
|
fireEvent.click(screen.getByTestId("sandbox-switch"));
|
||||||
await waitFor(() => expect(setSandboxSettings).toHaveBeenCalledWith({ provider: "seatbelt" }));
|
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.
|
// 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-provider-select") as HTMLSelectElement).value).toBe("seatbelt");
|
||||||
expect(screen.getByTestId("sandbox-network-section")).toBeTruthy();
|
expect(screen.getByTestId("sandbox-network-section")).toBeTruthy();
|
||||||
|
|||||||
@@ -178,8 +178,12 @@ export function SettingsView({
|
|||||||
onCreateSkill,
|
onCreateSkill,
|
||||||
onAskWorker,
|
onAskWorker,
|
||||||
onBack,
|
onBack,
|
||||||
|
onSandboxProviderChanged,
|
||||||
}: {
|
}: {
|
||||||
initialTab?: SetTab;
|
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.
|
// "Back to app" on the rail — returns to the conversation surface.
|
||||||
onBack?: () => void;
|
onBack?: () => void;
|
||||||
onOpenPersona?: (id: string, machineId?: string | null) => void;
|
onOpenPersona?: (id: string, machineId?: string | null) => void;
|
||||||
@@ -402,7 +406,7 @@ export function SettingsView({
|
|||||||
onAskWorker={(machineId) => onAskWorker?.(machineId)}
|
onAskWorker={(machineId) => onAskWorker?.(machineId)}
|
||||||
/>
|
/>
|
||||||
) : tab === "sandbox" ? (
|
) : tab === "sandbox" ? (
|
||||||
<SandboxSection key={scopeId || "local"} machine={scoped} />
|
<SandboxSection key={scopeId || "local"} machine={scoped} onProviderChanged={onSandboxProviderChanged} />
|
||||||
) : tab === "machines" ? (
|
) : tab === "machines" ? (
|
||||||
<MachinesSection />
|
<MachinesSection />
|
||||||
) : tab === "slack" || tab === "github" ? (
|
) : tab === "slack" || tab === "github" ? (
|
||||||
@@ -737,7 +741,7 @@ function PersonasSection({
|
|||||||
// what to do.
|
// what to do.
|
||||||
const SANDBOX_PROVIDERS = ["openshell", "seatbelt"] as const;
|
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 { t } = useTranslation();
|
||||||
const mid = machine?.id ?? null;
|
const mid = machine?.id ?? null;
|
||||||
const [cfg, setCfg] = useState<SandboxSettings | null>(null);
|
const [cfg, setCfg] = useState<SandboxSettings | null>(null);
|
||||||
@@ -760,6 +764,7 @@ function SandboxSection({ machine }: { machine?: Machine | null }) {
|
|||||||
}
|
}
|
||||||
setError("");
|
setError("");
|
||||||
setCfg(res as SandboxSettings);
|
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
|
// The switch reads the machine's choice; a headless machine's default rule (OpenShell
|
||||||
|
|||||||
@@ -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"]
|
assert not next(p for p in snap["providers"] if p["name"] == "openshell")["usable"]
|
||||||
with pytest.raises(OpenShellUnavailable):
|
with pytest.raises(OpenShellUnavailable):
|
||||||
selection.select("openshell")
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user