From e3723faafbba84edc2019fcbed139d367e421f05 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Mon, 17 Aug 2026 21:53:19 +0700 Subject: [PATCH 1/3] feat(sandbox): size a new macOS sandbox's memory to the Mac MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every sandbox got 4 GiB whatever the machine — thin for the office toolchain on a 32 GiB Mac, heavy-handed on an 8 GiB one. A sandbox that sets no Memory of its own now gets half the host's RAM, rounded down to a whole GiB and capped at 8 GiB: unchanged at 4 GiB on an 8 GiB Mac, 8 GiB from 16 GiB up. That is the rule Windows applies to the WSL utility VM, and the cap is what makes it safe to borrow. WSL's ceiling is one shared budget for every distro; here each sandbox is its own VM holding its own reservation for as long as it runs, so an uncapped half would let two sandboxes claim a whole machine. The Add dialog quotes the defaults in its field placeholders, so it can no longer keep the copy it held in src/sandbox/resources.ts — a mirror of a value derived from the host's RAM is a guess. The main process resolves them and the dialog reads them over a new sandbox-defaults channel, rendering the resources block once they arrive; the query is fired when the sandbox context mounts, so it has resolved long before the dialog opens. The ranges stay mirrored, since those are genuinely fixed. vz/provider.test.ts no longer pins 4096: with no per-sandbox and no provider limit the answer depends on the machine running the test, so it asserts against sandboxDefaultMemoryMiB() and the rule itself is covered directly against hosts this one is not. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 11 +++ DIVERGENCES.md | 2 +- packages/app/src/sandbox/context.tsx | 18 +++- packages/app/src/sandbox/dialog-add.tsx | 97 ++++++++++--------- packages/app/src/sandbox/resources.ts | 4 +- packages/app/src/sandbox/types.ts | 7 ++ packages/codev-sandbox/README.md | 10 +- packages/codev-sandbox/src/policy.test.ts | 25 ++++- packages/codev-sandbox/src/provider.ts | 28 ++++-- .../codev-sandbox/src/vz/provider.test.ts | 7 +- packages/codev-sandbox/src/vz/provider.ts | 4 +- .../desktop/src/main/sandbox/controller.ts | 6 ++ packages/desktop/src/main/sandbox/ipc.ts | 1 + packages/desktop/src/preload/index.ts | 1 + packages/desktop/src/preload/types.ts | 7 +- 15 files changed, 159 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c6c290db..7ca79754a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,17 @@ file is the human-readable summary of what changed and why. ### Sandbox +- **A new sandbox on macOS sizes its memory to the Mac** — every sandbox used + to get 4 GB whatever the machine, which is thin for the office toolchain on + a 32 GB Mac and heavy-handed on an 8 GB one. A sandbox created without its + own Memory value now gets half the Mac's RAM, capped at 8 GB and rounded + down to a whole gigabyte: 4 GB on an 8 GB Mac as before, 8 GB on anything + with 16 GB or more. This is the same rule Windows applies to WSL, with one + difference worth knowing — there the 8 GB is one ceiling shared by + everything WSL runs, while here each sandbox is its own virtual machine + holding its own memory for as long as it runs, so several large sandboxes + running at once add up. The Memory field in the Add sandbox dialog still + overrides it, and now shows the number this Mac would actually use. - **Sessions in a sandbox survive stopping it** — anything the sandbox had written in the seconds before it stopped was thrown away: stop a sandbox, or add a folder to one (which restarts it), or quit the app, and the sessions diff --git a/DIVERGENCES.md b/DIVERGENCES.md index bbcfb31a18..93e2972c89 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -208,7 +208,7 @@ from the squash message first, or use a plain merge. | Async settings stores: `packages/desktop/src/main/{store,index}.ts`, `packages/desktop/package.json` | `store.ts` no longer uses electron-store: `getStore` now returns the fork-only `store-file.ts` store — in-memory state, flushed write-behind through `store-writer.ts`'s serialized latest-wins async atomic writer (temp → fsync → rename via `fs.promises`, so the fsync runs on the libuv threadpool). Adds `flushStoreWrites(timeoutMs)`; `removeStoreFile`/`removeStoreFileIfEmpty` discard/flush the writer around the delete so a late flush cannot resurrect the file. `index.ts`: `stopSidecars` awaits `flushStoreWrites(1500)`. `package.json`: the `electron-store` dependency is removed. On-disk format is unchanged (flat keys, tab-indented JSON, no extension) — existing files load as-is; a corrupt file now starts empty with a logged error instead of throwing from every accessor. | conf v15's `store.set` is a synchronous fsync on the Electron main thread, and one of those parked the whole app in uninterruptible disk sleep (kernel journal stall + lost ext4 fast-commit wakeup, observed live during a sandbox mount-apply restart): event loop dead, IPC dead, quit impossible. conf also rereads the file on every access, which makes write-behind unsound with the stock class (a set on one key reads a stale file and drops a still-queued update), so the storage layer had to be replaced rather than patched. | Keep the fork store; take upstream's surrounding changes. If upstream adds electron-store options (schema, migrations, dot-notation) or new store call sites, extend `store-file.ts` to match — its API is the superset the desktop actually uses (`get/set/has/delete/clear/store/path`). Do not reintroduce a synchronous write path on the main thread. | | Desktop sandbox wiring: `packages/desktop/src/main/{index,store-keys}.ts`, `packages/desktop/package.json` | Hooks the fork-only `src/main/sandbox/{bundle,controller,ipc}.ts` modules into startup: `index.ts` constructs the sandbox controller beside the WSL servers controller, adds `sandboxes.stopAll()` to `stopSidecars`, registers the `sandbox-*` IPC channels next to the WSL ones, and calls `sandboxes.initialize()` (all platforms) once the local server is ready; `store-keys.ts` gains `SANDBOXES_KEY = "sandboxes"`; `package.json` gains the `@codev/sandbox` and `@codev/gateway` workspace devDependencies (bundled into the main process by electron-vite, so not runtime dependencies; gateway supplies the sandbox egress broker's credentials and config fragment). | The desktop app is where sandboxes are created and run; `createSandboxController` generalizes the proven WSL-servers controller pattern to the per-platform `@codev/sandbox` providers (macOS vz / Windows wsl / Linux bwrap). | Keep the wiring lines and take upstream's surrounding changes; they are additive and order-insensitive apart from `initialize()` needing the local server ready first. Do not rename the `sandbox-*` channels or the `sandboxes` store key — preload/renderer and persisted sandbox configs depend on them. | | `packages/app/src/components/server/server-row-menu.tsx` | The trailing **Delete** item carries `style={{ "--menu-v2-fg": "var(--v2-state-fg-danger)" }}`. Nothing else changes — same `disabled={builtin}`, same `handleRemove(key)`, same position after the separator. | Delete rendered in the same colour as Edit and Set as default, so the one irreversible action in the menu carried no more weight than the rest. The session actions menu already colours its Delete this way; this brings the server menu in line. It has to be the CSS variable rather than a text utility: `menu-v2.css` colours the item **and** its `menu-v2-item-content` label span from `--menu-v2-fg`, so a class on the item never reaches the label. | Keep the style prop. On conflict take upstream's item and re-apply it. Do not swap it for a Tailwind text colour — that silently only recolours half the item. The disabled state dims by `opacity` alone, so the red survives it. | -| Desktop sandbox renderer slice: `packages/app/package.json`, `packages/app/src/{index.ts,app.tsx}`, `packages/app/src/context/{platform,server}.tsx`, `packages/app/src/components/{settings-v2/servers.tsx,dialog-select-server.tsx}`, `packages/app/src/wsl/settings.tsx`, `packages/app/src/i18n/{en,es}.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx` | Surfaces the fork-only sandbox modules (`packages/app/src/sandbox/*`, `packages/desktop/src/renderer/sandbox/*`) in the UI: app `package.json` exports `./sandbox/types` (mirror of `./wsl/types`); `index.ts` re-exports `useSandboxes` + the sandbox types; `app.tsx` mounts `SandboxesProvider` inside `WslServersProvider`; `platform.tsx` gains optional `sandboxes?: SandboxesPlatform` and, on the desktop variant, `openFilePathDialog` (a path-only file picker — the existing attachment picker reads the file's bytes, which is unusable for a multi-gigabyte guest image), implemented in `renderer/index.tsx` over the existing `open-file-picker` channel and releasing the read token immediately; `server.tsx` adds the third `ServerConnection.Sidecar` variant `{ variant: "sandbox"; name; projectDir?; mounts? }` with key `sandbox:` (`projectDir` is the host folder mounted into the guest and `mounts` the extra host folders with their resolved guest paths — the guest only ever sees its own mount points, so this is the only way the app can name a sandbox's projects after the folders it works on); `settings-v2/servers.tsx` filters sandbox rows out of the generic list and renders `SandboxServerSettings` beside the WSL section; `dialog-select-server.tsx` `handleRemove` routes `sandbox:` keys to `platform.sandboxes.remove` (like `wsl:` keys) and moves the `tabs.removeServer(key)` + `server.remove(key)` pair *above* both awaited platform removals; `wsl/settings.tsx` `AddServerMenu` shows the menu when either wslServers or sandboxes exist and gains an "Add sandbox" item; i18n gains the `sandbox.*` key family (including the optional per-sandbox CPU/memory fields, macOS-only since only the vz backend applies them; `sandbox.add.projectDirHint`, the standing explanation above the folder field; the `sandbox.mounts.*` family for the manage-folders dialog that adds/removes extra mounts on an existing sandbox; `sandbox.job.queued`, shown while a sandbox's create/reset/destroy waits its turn behind another sandbox's job; `sandbox.server.stop` for the menu entry that shuts a sandbox down until it is explicitly started again; the `sandbox.add.image*` / `sandbox.add.name*` / `sandbox.arch.*` families for the optional custom-image field and the sandbox-name checks; and `sandbox.add.resourcesHint` plus interpolated `sandbox.add.{cpus,memory,resourcesInvalid}`, which quote the vz defaults and ranges from the constants rather than restating them in prose); preload exposes `api.sandboxes` over the `sandbox-*` channels and `renderer/index.tsx` wires the platform api, loading gate, ready-connection list, and the `startupServer` fallback (fork-only wrapper that validates `sandbox:` default keys the way `availableStartupServer` validates `wsl:` ones). | Ready sandboxes must appear as first-class servers and be manageable from settings; the slice mirrors the WSL vertical exactly so upstream conflicts resolve the same way in both. Removing a server destroys a disk and takes seconds, and the connection leaves the list the moment it finishes, so the tab cleanup has to happen before the await rather than after it. | Keep every listed edit; take upstream's surrounding changes. If upstream adds a `Sidecar` variant or reworks `AddServerMenu`, re-apply the sandbox variant/menu item on top. The `sandbox:` key format is persisted as the default-server key and as `config.id` in the main-process store — never change one side alone. Keep the tab cleanup ahead of the platform removals in `handleRemove`; if upstream reorders it back, re-apply. | +| Desktop sandbox renderer slice: `packages/app/package.json`, `packages/app/src/{index.ts,app.tsx}`, `packages/app/src/context/{platform,server}.tsx`, `packages/app/src/components/{settings-v2/servers.tsx,dialog-select-server.tsx}`, `packages/app/src/wsl/settings.tsx`, `packages/app/src/i18n/{en,es}.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx` | Surfaces the fork-only sandbox modules (`packages/app/src/sandbox/*`, `packages/desktop/src/renderer/sandbox/*`) in the UI: app `package.json` exports `./sandbox/types` (mirror of `./wsl/types`); `index.ts` re-exports `useSandboxes` + the sandbox types; `app.tsx` mounts `SandboxesProvider` inside `WslServersProvider`; `platform.tsx` gains optional `sandboxes?: SandboxesPlatform` and, on the desktop variant, `openFilePathDialog` (a path-only file picker — the existing attachment picker reads the file's bytes, which is unusable for a multi-gigabyte guest image), implemented in `renderer/index.tsx` over the existing `open-file-picker` channel and releasing the read token immediately; `server.tsx` adds the third `ServerConnection.Sidecar` variant `{ variant: "sandbox"; name; projectDir?; mounts? }` with key `sandbox:` (`projectDir` is the host folder mounted into the guest and `mounts` the extra host folders with their resolved guest paths — the guest only ever sees its own mount points, so this is the only way the app can name a sandbox's projects after the folders it works on); `settings-v2/servers.tsx` filters sandbox rows out of the generic list and renders `SandboxServerSettings` beside the WSL section; `dialog-select-server.tsx` `handleRemove` routes `sandbox:` keys to `platform.sandboxes.remove` (like `wsl:` keys) and moves the `tabs.removeServer(key)` + `server.remove(key)` pair *above* both awaited platform removals; `wsl/settings.tsx` `AddServerMenu` shows the menu when either wslServers or sandboxes exist and gains an "Add sandbox" item; i18n gains the `sandbox.*` key family (including the optional per-sandbox CPU/memory fields, macOS-only since only the vz backend applies them; `sandbox.add.projectDirHint`, the standing explanation above the folder field; the `sandbox.mounts.*` family for the manage-folders dialog that adds/removes extra mounts on an existing sandbox; `sandbox.job.queued`, shown while a sandbox's create/reset/destroy waits its turn behind another sandbox's job; `sandbox.server.stop` for the menu entry that shuts a sandbox down until it is explicitly started again; the `sandbox.add.image*` / `sandbox.add.name*` / `sandbox.arch.*` families for the optional custom-image field and the sandbox-name checks; and `sandbox.add.resourcesHint` plus interpolated `sandbox.add.{cpus,memory,resourcesInvalid}`, which quote the vz defaults and ranges rather than restating them in prose — the ranges come from the mirrored constants in `src/sandbox/resources.ts`, the defaults from the `sandbox-defaults` channel, because vz memory defaults to half the host's RAM and no constant can hold that); preload exposes `api.sandboxes` over the `sandbox-*` channels and `renderer/index.tsx` wires the platform api, loading gate, ready-connection list, and the `startupServer` fallback (fork-only wrapper that validates `sandbox:` default keys the way `availableStartupServer` validates `wsl:` ones). | Ready sandboxes must appear as first-class servers and be manageable from settings; the slice mirrors the WSL vertical exactly so upstream conflicts resolve the same way in both. Removing a server destroys a disk and takes seconds, and the connection leaves the list the moment it finishes, so the tab cleanup has to happen before the await rather than after it. | Keep every listed edit; take upstream's surrounding changes. If upstream adds a `Sidecar` variant or reworks `AddServerMenu`, re-apply the sandbox variant/menu item on top. The `sandbox:` key format is persisted as the default-server key and as `config.id` in the main-process store — never change one side alone. Keep the tab cleanup ahead of the platform removals in `handleRemove`; if upstream reorders it back, re-apply. | | Sandbox guest durability: `packages/core/src/database/database.ts`, `packages/core/src/flag/flag.ts` | `PRAGMA synchronous` is no longer hardcoded to `NORMAL`: it reads `Flag.CODEV_SQLITE_SYNCHRONOUS`, validated against a whitelist (`OFF`/`NORMAL`/`FULL`/`EXTRA`) because the value lands in a PRAGMA, and falls back to `NORMAL` — so nothing changes for any process that does not set it. The sandbox sets it to `FULL` for guests (fork-only `packages/codev-sandbox/src/boot.ts`). | WAL + `synchronous = NORMAL` is durable across a crashed process but not across a machine that disappears, because those commits sit in the page cache until a checkpoint. A sandbox guest is exactly that machine: `VZVirtualMachine.stop()` is documented as pulling the power cable, and the guest gets no chance to sync — so stopping a sandbox, restarting one to apply a mount, or force quitting the app dropped every session written in the preceding seconds. Measured on a real vz guest: a file written and left unsynced is gone after a plain Stop/Start; with `FULL` a session created two seconds before `kill -9` of the whole app survives. The knob lives in core because the guest runs the host's own server bundle — there is no sandbox-only build to patch. | Keep the flag and the whitelist; take upstream's other pragmas. If upstream makes synchronous configurable its own way, drop ours for theirs. Never widen the whitelist to pass the value through unchecked. The default must stay `NORMAL` — raising it for every install would make the host pay for a guarantee only the sandbox needs. | | `packages/desktop/src/renderer/index.tsx` | Desktop notification icon `https://opencode.ai/favicon-96x96-v3.png` → the relative `./favicon-96x96-v3.png`. The renderer's `publicDir` is `packages/app/public`, whose `favicon-96x96-v3.png` symlinks to the rebranded `packages/ui/src/assets/favicon/` mark, so the file is emitted next to `index.html` and resolves against the renderer origin (`oc://renderer/` packaged, the vite dev server otherwise) — the same asset and the same relative form `src/renderer/index.html` already uses for ``. | A desktop app should not reach out to a remote host to draw its own notification icon (it fails offline and leaks a request per notification), and the remote file is upstream-branded while the bundled one is the codev mark. | Keep our relative path. If upstream changes the URL, re-point it at the bundled asset rather than taking a new remote host. `packages/app/src/entry.tsx` and `pages/layout/helpers.ts` keep their remote `opencode.ai/favicon*` sources — same bug class, not yet fixed. | diff --git a/packages/app/src/sandbox/context.tsx b/packages/app/src/sandbox/context.tsx index dd296c3c77..a7f54d6e36 100644 --- a/packages/app/src/sandbox/context.tsx +++ b/packages/app/src/sandbox/context.tsx @@ -1,11 +1,12 @@ import { createSimpleContext } from "@opencode-ai/ui/context" import { queryOptions, useQuery, useQueryClient } from "@tanstack/solid-query" import { createEffect, onCleanup } from "solid-js" -import type { SandboxAvailability, SandboxesState } from "./types" +import type { SandboxAvailability, SandboxDefaults, SandboxesState } from "./types" import { usePlatform } from "../context/platform" const sandboxesQueryKey = ["platform", "sandboxes"] as const const sandboxAvailabilityQueryKey = ["platform", "sandboxAvailability"] as const +const sandboxDefaultsQueryKey = ["platform", "sandboxDefaults"] as const export const { use: useSandboxes, provider: SandboxesProvider } = createSimpleContext({ name: "Sandboxes", @@ -33,6 +34,20 @@ export const { use: useSandboxes, provider: SandboxesProvider } = createSimpleCo }) }) + // Fetched with the rest rather than when the Add dialog opens, so the + // dialog's placeholders have their numbers the moment it appears. They + // depend on the host's RAM, which does not change while the app runs. + const defaults = useQuery(() => { + const api = platform.sandboxes + return queryOptions({ + queryKey: sandboxDefaultsQueryKey, + queryFn: () => api!.defaults(), + enabled: !!api, + staleTime: Number.POSITIVE_INFINITY, + gcTime: Number.POSITIVE_INFINITY, + }) + }) + createEffect(() => { const api = platform.sandboxes if (!api) return @@ -45,6 +60,7 @@ export const { use: useSandboxes, provider: SandboxesProvider } = createSimpleCo return { state: state as typeof state & { readonly data: SandboxesState | undefined }, availability: availability as typeof availability & { readonly data: SandboxAvailability | undefined }, + defaults: defaults as typeof defaults & { readonly data: SandboxDefaults | undefined }, } }, }) diff --git a/packages/app/src/sandbox/dialog-add.tsx b/packages/app/src/sandbox/dialog-add.tsx index 9017cc7a6a..6694c5ae68 100644 --- a/packages/app/src/sandbox/dialog-add.tsx +++ b/packages/app/src/sandbox/dialog-add.tsx @@ -20,13 +20,7 @@ import type { SandboxConfig, SandboxImageProblem } from "./types" import { useSandboxes } from "./context" import { sandboxCreatePhase } from "./create-phase" import { sandboxNameUsable, validateSandboxName } from "./name-model" -import { - SANDBOX_CPUS_RANGE, - SANDBOX_DEFAULT_CPUS, - SANDBOX_DEFAULT_MEMORY_GIB, - SANDBOX_MEMORY_MIB_RANGE, - sandboxResourcesValid, -} from "./resources" +import { SANDBOX_CPUS_RANGE, SANDBOX_MEMORY_MIB_RANGE, sandboxResourcesValid } from "./resources" // onCreated fires with the new sandbox before the dialog closes. Callers that // need to react to a creation must use it rather than watching the sandbox @@ -210,10 +204,7 @@ export function DialogAddSandbox(props: { it explains the whole dialog, not the field it happens to sit above, and DialogTitleGroup ties it to the dialog for screen readers. */} - {language.t("sandbox.server.add")}} - > + {language.t("sandbox.server.add")}}>

{language.t("sandbox.add.imageOk")}

 - - {/* Says what the numbers buy. The defaults live in the - placeholders and the ceilings in the out-of-range message, so - neither needs restating here. */} -

{language.t("sandbox.add.resourcesHint")}

- {/* Grid, not flex: the input sets its own `width: 280px` and - `flex: none`, so two of them side by side overflowed the dialog - rather than sharing the row. Grid tracks size them, and - `!w-full` is how the rest of the app overrides that width. */} -
- setStore("cpus", event.currentTarget.value)} - /> - setStore("memoryGiB", event.currentTarget.value)} - /> -
- -

- {language.t("sandbox.add.resourcesInvalid", { - minCpus: SANDBOX_CPUS_RANGE.min, - maxCpus: SANDBOX_CPUS_RANGE.max, - minMemory: SANDBOX_MEMORY_MIB_RANGE.min / 1024, - maxMemory: SANDBOX_MEMORY_MIB_RANGE.max / 1024, - })} -

- + {/* Held back until the defaults arrive: they are what the + placeholders quote, and the memory one is a property of this Mac + rather than a constant the app can hold. The query is fired when + the sandbox context mounts, so in practice it has resolved long + before the dialog opens. */} + + {(defaults) => ( + <> + {/* Says what the numbers buy. The defaults live in the + placeholders and the ceilings in the out-of-range message, + so neither needs restating here. */} +

{language.t("sandbox.add.resourcesHint")}

+ {/* Grid, not flex: the input sets its own `width: 280px` and + `flex: none`, so two of them side by side overflowed the + dialog rather than sharing the row. Grid tracks size them, + and `!w-full` is how the rest of the app overrides that + width. */} +
+ setStore("cpus", event.currentTarget.value)} + /> + setStore("memoryGiB", event.currentTarget.value)} + /> +
+ +

+ {language.t("sandbox.add.resourcesInvalid", { + minCpus: SANDBOX_CPUS_RANGE.min, + maxCpus: SANDBOX_CPUS_RANGE.max, + minMemory: SANDBOX_MEMORY_MIB_RANGE.min / 1024, + maxMemory: SANDBOX_MEMORY_MIB_RANGE.max / 1024, + })} +

+ + + )} {(current) => ( diff --git a/packages/app/src/sandbox/resources.ts b/packages/app/src/sandbox/resources.ts index 2aea456431..b810f3a675 100644 --- a/packages/app/src/sandbox/resources.ts +++ b/packages/app/src/sandbox/resources.ts @@ -3,8 +3,8 @@ import type { SandboxResources } from "./types" // Mirrors SANDBOX_*_RANGE in @codev/sandbox: the app package must not import // the node-flavored package, and the main process re-validates on the way in, // so this copy only decides when the dialog's Create button is enabled. -export const SANDBOX_DEFAULT_CPUS = 4 -export const SANDBOX_DEFAULT_MEMORY_GIB = 4 +// The *defaults* are deliberately not mirrored here — memory is derived from +// the host's RAM, so a copy would be a guess; the dialog reads them over IPC. export const SANDBOX_CPUS_RANGE = { min: 1, max: 32 } as const export const SANDBOX_MEMORY_MIB_RANGE = { min: 1024, max: 65536 } as const diff --git a/packages/app/src/sandbox/types.ts b/packages/app/src/sandbox/types.ts index b6b3b9110b..106906fbc1 100644 --- a/packages/app/src/sandbox/types.ts +++ b/packages/app/src/sandbox/types.ts @@ -40,6 +40,12 @@ export type ResolvedSandboxMount = { // offers the fields only where they take effect. export type SandboxResources = { cpus?: number; memoryMiB?: number } +// What a sandbox that sets neither gets. Memory is derived from the host's RAM +// (half of it, capped), so unlike the ranges the dialog cannot keep a copy — +// it reads these from the main process and quotes them as the field +// placeholders. +export type SandboxDefaults = { cpus: number; memoryMiB: number } + export type SandboxRuntime = | { kind: "starting" } | { kind: "ready"; url: string; username: string | null; password: string | null; mounts: ResolvedSandboxMount[] } @@ -91,6 +97,7 @@ export type SandboxesPlatform = { getState(): Promise subscribe(cb: (event: SandboxesEvent) => void): () => void availability(): Promise + defaults(): Promise // Resolves undefined when the create was cancelled mid-download. add( name: string, diff --git a/packages/codev-sandbox/README.md b/packages/codev-sandbox/README.md index 0e0ab0b142..7bf761e645 100644 --- a/packages/codev-sandbox/README.md +++ b/packages/codev-sandbox/README.md @@ -13,7 +13,15 @@ Electron main process. - `provider.ts` — the `SandboxProvider` contract every backend implements: `availability` / `create` / `start` / `destroy` / `reset`, plus the optional `applyMounts` hot path. Also the persisted `SandboxConfig` shape (project - dir, extra mounts, per-sandbox cpu/memory limits) and its validation. + dir, extra mounts, per-sandbox cpu/memory limits) and its validation. A + sandbox that sets no limits gets `SANDBOX_DEFAULT_CPUS` and + `sandboxDefaultMemoryMiB()` — half the host's RAM, whole GiB, capped at 8 + GiB, floored at the range minimum. That is WSL's rule for its utility VM, + and the cap is what makes it safe to borrow: WSL's ceiling is shared by + every distro, while each vz sandbox holds its own reservation for as long as + it runs. The Add dialog reads the resolved numbers back over the + `sandbox-defaults` IPC channel rather than mirroring them, since the memory + one is a property of the machine. - `controller.ts` — lifecycle state machine: runtime states, the job queue, stale-start guards. Create, reset and destroy run **one at a time across all sandboxes** — not for the sandboxes' sake, but because create and reset stage diff --git a/packages/codev-sandbox/src/policy.test.ts b/packages/codev-sandbox/src/policy.test.ts index cdb78486c4..7a118906ff 100644 --- a/packages/codev-sandbox/src/policy.test.ts +++ b/packages/codev-sandbox/src/policy.test.ts @@ -1,5 +1,11 @@ import { expect, test } from "bun:test" -import { SANDBOX_CPUS_RANGE, SANDBOX_MEMORY_MIB_RANGE, sandboxResourcesValid } from "./provider" +import { + SANDBOX_CPUS_RANGE, + SANDBOX_DEFAULT_MEMORY_CAP_MIB, + SANDBOX_MEMORY_MIB_RANGE, + sandboxDefaultMemoryMiB, + sandboxResourcesValid, +} from "./provider" import { MACOS_UNSUPPORTED_MESSAGE, WINDOWS_10_WARNING, @@ -51,3 +57,20 @@ test("sandbox resource limits accept sane values and reject the rest", () => { expect(sandboxResourcesValid({ memoryMiB: 131072 })).toBe(false) expect(sandboxResourcesValid({ cpus: Number.NaN })).toBe(false) }) + +test("the default memory is half the host's RAM, capped and floored", () => { + const gib = 1024 ** 3 + expect(sandboxDefaultMemoryMiB(8 * gib)).toBe(4096) + expect(sandboxDefaultMemoryMiB(16 * gib)).toBe(8192) + // Capped: a 64 GiB Mac would otherwise hand one sandbox 32 GiB, and each + // running sandbox holds its reservation for as long as it runs. + expect(sandboxDefaultMemoryMiB(64 * gib)).toBe(SANDBOX_DEFAULT_MEMORY_CAP_MIB) + // Rounded down to a whole GiB rather than landing on 6.5 GiB. + expect(sandboxDefaultMemoryMiB(13 * gib)).toBe(6144) + // Floored at the range minimum, so a small machine still gets a bootable + // guest instead of a VM the validator would reject. + expect(sandboxDefaultMemoryMiB(2 * gib)).toBe(SANDBOX_MEMORY_MIB_RANGE.min) + expect(sandboxDefaultMemoryMiB(512 * 1024 * 1024)).toBe(SANDBOX_MEMORY_MIB_RANGE.min) + // Whatever this machine has, the result is always a usable configuration. + expect(sandboxResourcesValid({ memoryMiB: sandboxDefaultMemoryMiB() })).toBe(true) +}) diff --git a/packages/codev-sandbox/src/provider.ts b/packages/codev-sandbox/src/provider.ts index fe7681aa50..5255bf29c8 100644 --- a/packages/codev-sandbox/src/provider.ts +++ b/packages/codev-sandbox/src/provider.ts @@ -1,3 +1,5 @@ +import { totalmem } from "node:os" + export type SandboxBackend = "vz" | "wsl" | "bwrap" export type SandboxArch = "x64" | "arm64" @@ -40,11 +42,20 @@ export type ResolvedSandboxMount = { guestPath: string } -// What a sandbox gets when it sets no limits of its own. Named here rather +// What a sandbox gets when it sets no limits of its own. Resolved here rather // than left as literals in the vz backend because the Add sandbox dialog -// quotes these numbers to the user; they must not drift apart. +// quotes these numbers to the user; they must not drift apart, so the dialog +// asks the main process for them rather than keeping a copy. export const SANDBOX_DEFAULT_CPUS = 4 -export const SANDBOX_DEFAULT_MEMORY_MIB = 4096 + +// Memory scales with the host instead of sitting at a fixed 4 GiB, following +// the rule WSL applies to its utility VM: half the machine's RAM, capped. +// The cap is what makes the rule safe to borrow — unlike WSL's single shared +// utility VM, each sandbox here is its own VM holding its own reservation for +// as long as it runs, so an uncapped half would let two of them claim a whole +// machine. Rounded down to a whole GiB, and never below the floor a guest +// needs to boot the office toolchain at all. +export const SANDBOX_DEFAULT_MEMORY_CAP_MIB = 8192 // Ceilings keep an unusable VM (0 cpus, 64MiB) or an oversubscribed host out // of the persisted config; the guest needs real headroom for the office @@ -52,6 +63,13 @@ export const SANDBOX_DEFAULT_MEMORY_MIB = 4096 export const SANDBOX_CPUS_RANGE = { min: 1, max: 32 } as const export const SANDBOX_MEMORY_MIB_RANGE = { min: 1024, max: 65536 } as const +// totalBytes is injectable so the rule can be tested against machines this one +// is not; production always reads the host. +export function sandboxDefaultMemoryMiB(totalBytes = totalmem()) { + const halfInGiB = Math.floor(totalBytes / 2 / 1024 ** 3) + return Math.min(Math.max(halfInGiB * 1024, SANDBOX_MEMORY_MIB_RANGE.min), SANDBOX_DEFAULT_MEMORY_CAP_MIB) +} + export function sandboxResourcesValid(config: { cpus?: number; memoryMiB?: number }) { const inRange = (value: number | undefined, range: { min: number; max: number }) => value === undefined || (Number.isInteger(value) && value >= range.min && value <= range.max) @@ -78,9 +96,7 @@ export type RunningSandbox = { onExit: (cb: (code: number | null, signal: NodeJS.Signals | null) => void) => void } -export type SandboxAvailability = - | { supported: true; warning: string | null } - | { supported: false; message: string } +export type SandboxAvailability = { supported: true; warning: string | null } | { supported: false; message: string } // Image download progress, reported while create/reset stage the guest image. export type SandboxProgress = { bytes: number; totalBytes: number } diff --git a/packages/codev-sandbox/src/vz/provider.test.ts b/packages/codev-sandbox/src/vz/provider.test.ts index 3b7b686743..ea2b52f786 100644 --- a/packages/codev-sandbox/src/vz/provider.test.ts +++ b/packages/codev-sandbox/src/vz/provider.test.ts @@ -5,6 +5,7 @@ import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises" import { tmpdir } from "node:os" import { join } from "node:path" import type { SandboxConfig } from "../provider" +import { SANDBOX_DEFAULT_CPUS, sandboxDefaultMemoryMiB } from "../provider" import { runCommand } from "../bwrap/rootfs" import { createVzProvider, vzSandboxDirs, type VzProviderOptions } from "./provider" @@ -130,8 +131,10 @@ test("starts the VM via the helper protocol and serves health through the vsock "host-to-guest", "guest-to-host", ]) - expect(vmConfig.cpus).toBe(4) - expect(vmConfig.memoryMiB).toBe(4096) + expect(vmConfig.cpus).toBe(SANDBOX_DEFAULT_CPUS) + // Not a literal: with no per-sandbox and no provider limit, memory follows + // whatever RAM the machine running this test has. + expect(vmConfig.memoryMiB).toBe(sandboxDefaultMemoryMiB()) const boot = JSON.parse(await readFile(join(prepared.dirs.guestConfig, "boot.json"), "utf8")) expect(boot.env.CODEV_SERVER_PASSWORD).toBe(sandbox.password) expect(boot.env.CODEV_SERVER_HOSTNAME).toBe("127.0.0.1") diff --git a/packages/codev-sandbox/src/vz/provider.ts b/packages/codev-sandbox/src/vz/provider.ts index 1d3f73f587..d1c4f360e5 100644 --- a/packages/codev-sandbox/src/vz/provider.ts +++ b/packages/codev-sandbox/src/vz/provider.ts @@ -10,7 +10,7 @@ import type { SandboxAsset } from "../manifest" import { resolveGuestMounts } from "../mounts" import { macosSandboxAvailability } from "../policy" import { createSandboxSocketDir } from "../socket" -import { SANDBOX_DEFAULT_CPUS, SANDBOX_DEFAULT_MEMORY_MIB } from "../provider" +import { SANDBOX_DEFAULT_CPUS, sandboxDefaultMemoryMiB } from "../provider" import type { SandboxAvailability, SandboxConfig, SandboxJobOptions, SandboxProvider } from "../provider" import { createVzHelperEventReader, type VzHelperEvent, type VzVmConfig } from "./protocol" @@ -184,7 +184,7 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { cmdline: VZ_CMDLINE, // Per-sandbox limits win over the provider-wide defaults. cpus: config.cpus ?? options.cpus ?? SANDBOX_DEFAULT_CPUS, - memoryMiB: config.memoryMiB ?? options.memoryMiB ?? SANDBOX_DEFAULT_MEMORY_MIB, + memoryMiB: config.memoryMiB ?? options.memoryMiB ?? sandboxDefaultMemoryMiB(), shares: [ { tag: "workspace", path: config.projectDir, readOnly: false }, { tag: "bundle", path: options.bundleDir, readOnly: true }, diff --git a/packages/desktop/src/main/sandbox/controller.ts b/packages/desktop/src/main/sandbox/controller.ts index db7991d874..47b99a8edd 100644 --- a/packages/desktop/src/main/sandbox/controller.ts +++ b/packages/desktop/src/main/sandbox/controller.ts @@ -2,6 +2,7 @@ import { dirname, join } from "node:path" import { fileURLToPath } from "node:url" import { app } from "electron" import { + SANDBOX_DEFAULT_CPUS, SANDBOX_MANIFEST_URL, createBwrapProvider, createSandboxController, @@ -9,6 +10,7 @@ import { createWslProvider, fetchSandboxManifest, requireImageAsset, + sandboxDefaultMemoryMiB, sandboxResourcesValid, } from "@codev/sandbox" import type { SandboxArch, SandboxAvailability, SandboxBackend, SandboxConfig, SandboxProvider } from "@codev/sandbox" @@ -41,6 +43,10 @@ export function createDesktopSandboxController(logger: ControllerLogger) { backend: provider?.backend ?? null, // What a custom image has to match to be usable here. arch: sandboxArch() ?? null, + // What a sandbox that sets no limits of its own gets. Memory depends on + // how much RAM this machine has, so the dialog cannot hold a copy of the + // number the way it does for the ranges — it has to ask. + defaults: () => ({ cpus: SANDBOX_DEFAULT_CPUS, memoryMiB: sandboxDefaultMemoryMiB() }), availability: (): Promise => { if (!provider) return Promise.resolve({ supported: false, message: `sandboxes are not supported on ${process.platform}` }) diff --git a/packages/desktop/src/main/sandbox/ipc.ts b/packages/desktop/src/main/sandbox/ipc.ts index 11e24c06c9..85a7e3be58 100644 --- a/packages/desktop/src/main/sandbox/ipc.ts +++ b/packages/desktop/src/main/sandbox/ipc.ts @@ -46,6 +46,7 @@ export function registerSandboxIpcHandlers(controller: DesktopSandboxController) ipcMain.handle("sandbox-unsubscribe", (event) => unsubscribe(event.sender.id)) ipcMain.handle("sandbox-get-state", () => controller.getState()) ipcMain.handle("sandbox-availability", () => controller.availability()) + ipcMain.handle("sandbox-defaults", () => controller.defaults()) // Inspects a candidate image so the dialog can report the problem while the // user is still filling the form. Returns the same problem shape the add // path enforces, so the two can never disagree about what is acceptable. diff --git a/packages/desktop/src/preload/index.ts b/packages/desktop/src/preload/index.ts index c3a418dc7f..6b4125f9c5 100644 --- a/packages/desktop/src/preload/index.ts +++ b/packages/desktop/src/preload/index.ts @@ -48,6 +48,7 @@ const api: ElectronAPI = { } }, availability: () => ipcRenderer.invoke("sandbox-availability"), + defaults: () => ipcRenderer.invoke("sandbox-defaults"), add: (name, projectDir, resources, imagePath) => ipcRenderer.invoke("sandbox-add", name, projectDir, resources, imagePath), inspectImage: (path) => ipcRenderer.invoke("sandbox-inspect-image", path), diff --git a/packages/desktop/src/preload/types.ts b/packages/desktop/src/preload/types.ts index 8f4f7bed7c..7fa24036b2 100644 --- a/packages/desktop/src/preload/types.ts +++ b/packages/desktop/src/preload/types.ts @@ -19,17 +19,14 @@ export type { export type { SandboxAvailability, SandboxConfig, + SandboxDefaults, SandboxItem, SandboxJob, SandboxRuntime, SandboxesEvent, SandboxesState, } from "@opencode-ai/app/sandbox/types" -export type { - GatewayCompleteResult, - GatewaySetupResult, - GatewaySsoState, -} from "@opencode-ai/app/gateway/types" +export type { GatewayCompleteResult, GatewaySetupResult, GatewaySsoState } from "@opencode-ai/app/gateway/types" export type ServerReadyData = { url: string From d6e8362fd6e77f67473957513e44ca81fe0cb978 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Mon, 17 Aug 2026 22:01:53 +0700 Subject: [PATCH 2/3] feat(sandbox): divide the memory budget across running sandboxes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit gave a sandbox half the Mac's RAM, capped at 8 GiB, and left the cap as the only protection against several of them at once. On a 16 GiB Mac that made one sandbox half the machine and two of them all of it. Sandboxes now share one budget — half the host's RAM — rather than each claiming it. A sandbox that names no memory of its own gets an even share of that budget across the sandboxes that would then be running, still capped at 8 GiB per sandbox and now floored at 2 GiB, because a guest too small to run the office toolchain is the worse outcome. On a 16 GiB Mac: 8 GiB alone, 4 GiB each for two, 2 GiB each for three. The share is resolved at start rather than at create, since what else is up changes in between, and it can only ever shape the sandbox being started: a VM's memory is fixed at boot, so a second sandbox makes room by taking less itself, never by shrinking the one already running. That puts the decision in the controller, the only thing that knows what is up — it passes the resolved number to provider.start() as defaultMemoryMiB, where a per-sandbox or provider-wide limit still outranks it, and answers defaultResources() for the Add dialog with the share the sandbox about to be created would get. initialize() now marks every sandbox it is about to start as starting before starting any of them. Otherwise a boot with two sandboxes handed the whole budget to whichever came first in the persisted file and squeezed the other, which is no basis for the decision. The renderer refetches the defaults when the set of live sandboxes changes, but not on the other state events — those include every download-progress tick. CPUs are deliberately left alone: they are time-sliced, so four each oversubscribes nothing. Memory is reserved, which is why this exists. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 25 ++++---- packages/app/src/sandbox/context.tsx | 10 +++ packages/codev-sandbox/README.md | 22 ++++--- packages/codev-sandbox/src/controller.test.ts | 62 +++++++++++++++++++ packages/codev-sandbox/src/controller.ts | 29 ++++++++- packages/codev-sandbox/src/policy.test.ts | 37 ++++++++--- packages/codev-sandbox/src/provider.ts | 45 ++++++++++---- .../codev-sandbox/src/vz/provider.test.ts | 29 +++++++-- packages/codev-sandbox/src/vz/provider.ts | 9 ++- .../desktop/src/main/sandbox/controller.ts | 6 -- packages/desktop/src/main/sandbox/ipc.ts | 4 +- 11 files changed, 220 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ca79754a1..f877ee5a0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,17 +16,20 @@ file is the human-readable summary of what changed and why. ### Sandbox -- **A new sandbox on macOS sizes its memory to the Mac** — every sandbox used - to get 4 GB whatever the machine, which is thin for the office toolchain on - a 32 GB Mac and heavy-handed on an 8 GB one. A sandbox created without its - own Memory value now gets half the Mac's RAM, capped at 8 GB and rounded - down to a whole gigabyte: 4 GB on an 8 GB Mac as before, 8 GB on anything - with 16 GB or more. This is the same rule Windows applies to WSL, with one - difference worth knowing — there the 8 GB is one ceiling shared by - everything WSL runs, while here each sandbox is its own virtual machine - holding its own memory for as long as it runs, so several large sandboxes - running at once add up. The Memory field in the Add sandbox dialog still - overrides it, and now shows the number this Mac would actually use. +- **Sandboxes on macOS size their memory to the Mac, and share it** — every + sandbox used to get 4 GB whatever the machine, which is thin for the office + toolchain on a 32 GB Mac and heavy-handed on an 8 GB one. Sandboxes may now + use half the Mac's RAM between them, and a sandbox that names no Memory of + its own takes an even share of that: on a 16 GB Mac, 8 GB on its own, 4 GB + each when two run together, 2 GB each when three do — never more than 8 GB + and never less than 2 GB, whatever the machine and however many are up. The + share is worked out when a sandbox starts, so it depends on what is running + at that moment, and it only ever applies to the sandbox being started: a + running sandbox's memory is fixed while it runs, so starting a second one + makes room by giving the newcomer less, never by shrinking the first. A + sandbox you have stopped holds nothing. The Memory field in the Add sandbox + dialog still overrides all of this, and shows the share the sandbox you are + about to create would get. - **Sessions in a sandbox survive stopping it** — anything the sandbox had written in the seconds before it stopped was thrown away: stop a sandbox, or add a folder to one (which restarts it), or quit the app, and the sessions diff --git a/packages/app/src/sandbox/context.tsx b/packages/app/src/sandbox/context.tsx index a7f54d6e36..7cfdb9292b 100644 --- a/packages/app/src/sandbox/context.tsx +++ b/packages/app/src/sandbox/context.tsx @@ -51,8 +51,18 @@ export const { use: useSandboxes, provider: SandboxesProvider } = createSimpleCo createEffect(() => { const api = platform.sandboxes if (!api) return + // The defaults are a share of one memory budget divided by the sandboxes + // that are up, so they move whenever that set does — but not on the + // other state events, which include every download-progress tick. + let active = -1 const off = api.subscribe((event) => { queryClient.setQueryData(sandboxesQueryKey, event.state) + const next = event.state.sandboxes.filter( + (item) => item.runtime.kind === "ready" || item.runtime.kind === "starting", + ).length + if (next === active) return + active = next + void queryClient.invalidateQueries({ queryKey: sandboxDefaultsQueryKey }) }) onCleanup(off) }) diff --git a/packages/codev-sandbox/README.md b/packages/codev-sandbox/README.md index 7bf761e645..94433b5d63 100644 --- a/packages/codev-sandbox/README.md +++ b/packages/codev-sandbox/README.md @@ -15,13 +15,15 @@ Electron main process. `applyMounts` hot path. Also the persisted `SandboxConfig` shape (project dir, extra mounts, per-sandbox cpu/memory limits) and its validation. A sandbox that sets no limits gets `SANDBOX_DEFAULT_CPUS` and - `sandboxDefaultMemoryMiB()` — half the host's RAM, whole GiB, capped at 8 - GiB, floored at the range minimum. That is WSL's rule for its utility VM, - and the cap is what makes it safe to borrow: WSL's ceiling is shared by - every distro, while each vz sandbox holds its own reservation for as long as - it runs. The Add dialog reads the resolved numbers back over the - `sandbox-defaults` IPC channel rather than mirroring them, since the memory - one is a property of the machine. + `sandboxDefaultMemoryMiB({ sandboxes })` — sandboxes may use half the host's + RAM between them, and each takes an even share of that budget, whole GiB, + capped at 8 GiB and floored at 2 GiB. The budget is WSL's rule for its + utility VM; the split is what makes it safe to borrow, since WSL's ceiling + covers every distro at once while each vz sandbox holds its own reservation. + CPUs are not shared out the same way — they are time-sliced, so four each + oversubscribes nothing. The Add dialog reads the resolved numbers back over + the `sandbox-defaults` IPC channel rather than mirroring them: they depend + on the machine *and* on what is running. - `controller.ts` — lifecycle state machine: runtime states, the job queue, stale-start guards. Create, reset and destroy run **one at a time across all sandboxes** — not for the sandboxes' sake, but because create and reset stage @@ -31,7 +33,11 @@ Electron main process. for the *same* sandbox supersedes the first. `initialize()` starts every persisted sandbox except the ones the user stopped — stopping is a decision, so it persists in the config and survives a relaunch, and only an explicit - start clears it. + start clears it; it marks them all starting before starting any of them, so a + boot splits the memory budget evenly rather than letting persisted order + decide who gets it. Being the only thing that knows what is up, the + controller also resolves each start's `defaultMemoryMiB` and answers + `defaultResources()` for the Add dialog. - `policy.ts` — OS version gates: hard block below macOS 12, tiered warn-never-block on Windows, always-supported on Linux. - `manifest.ts` / `image.ts` — pinned manifest parsing and content-addressed, diff --git a/packages/codev-sandbox/src/controller.test.ts b/packages/codev-sandbox/src/controller.test.ts index e3340ebe83..1a4393f9b8 100644 --- a/packages/codev-sandbox/src/controller.test.ts +++ b/packages/codev-sandbox/src/controller.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test" import { createSandboxController, type SandboxController } from "./controller" import type { RunningSandbox, SandboxConfig, SandboxProvider, SandboxRuntime } from "./provider" +import { sandboxDefaultMemoryMiB } from "./provider" type FakeRunning = RunningSandbox & { stopped: boolean @@ -750,3 +751,64 @@ test("restartRunning skips a sandbox that failed to start", async () => { await controller.restartRunning() expect(starts).toBe(1) }) + +test("sandboxes coming up together split the memory budget", async () => { + const shares: Record = {} + const controller = createSandboxController({ + providers: { + bwrap: fakeProvider({ + start: async (target, opts) => { + shares[target.name] = opts?.defaultMemoryMiB + return fakeRunning("http://127.0.0.1:4096") + }, + }), + }, + readSandboxes: () => [config, other], + writeSandboxes: () => undefined, + }) + await controller.initialize() + await waitForRuntimeKind(controller, config.id, "ready") + await waitForRuntimeKind(controller, other.id, "ready") + // Neither took the whole budget on the strength of coming first in the + // persisted list: a boot marks everything it is about to start before it + // starts any of it, so both see the same count. + expect(shares).toEqual({ + default: sandboxDefaultMemoryMiB({ sandboxes: 2 }), + other: sandboxDefaultMemoryMiB({ sandboxes: 2 }), + }) + + // A third joining two that are already up gets a third of the budget — and + // the dialog is told that same number before it is added, since the sandbox + // it is about to create is the one that makes it three. + expect(controller.defaultResources().memoryMiB).toBe(sandboxDefaultMemoryMiB({ sandboxes: 3 })) + await controller.addSandbox(third) + await waitForRuntimeKind(controller, third.id, "ready") + expect(shares.third).toBe(sandboxDefaultMemoryMiB({ sandboxes: 3 })) +}) + +test("a stopped sandbox holds no share of the memory budget", async () => { + const shares: Record = {} + const controller = createSandboxController({ + providers: { + bwrap: fakeProvider({ + start: async (target, opts) => { + shares[target.name] = opts?.defaultMemoryMiB + return fakeRunning("http://127.0.0.1:4096") + }, + }), + }, + readSandboxes: () => [config, { ...other, stopped: true }], + writeSandboxes: () => undefined, + }) + await controller.initialize() + await waitForRuntimeKind(controller, config.id, "ready") + // The stopped one is persisted but holds no memory, so the one that did + // start gets the budget a lone sandbox gets. + expect(shares).toEqual({ default: sandboxDefaultMemoryMiB({ sandboxes: 1 }) }) + expect(controller.defaultResources().memoryMiB).toBe(sandboxDefaultMemoryMiB({ sandboxes: 2 })) + + // Starting it is what takes a share, and the newcomer is the one that pays. + await controller.startSandbox(other.id) + await waitForRuntimeKind(controller, other.id, "ready") + expect(shares.other).toBe(sandboxDefaultMemoryMiB({ sandboxes: 2 })) +}) diff --git a/packages/codev-sandbox/src/controller.ts b/packages/codev-sandbox/src/controller.ts index ed4f819916..9becb72af6 100644 --- a/packages/codev-sandbox/src/controller.ts +++ b/packages/codev-sandbox/src/controller.ts @@ -8,6 +8,7 @@ import type { SandboxProvider, SandboxRuntime, } from "./provider" +import { SANDBOX_DEFAULT_CPUS, sandboxDefaultMemoryMiB } from "./provider" export type SandboxJob = { kind: "create" | "destroy" | "reset" @@ -162,6 +163,13 @@ export function createSandboxController(options: SandboxControllerOptions) { return startAttempts.get(id) === attempt && state.sandboxes.some((item) => item.config.id === id) } + // Sandboxes holding memory on the host right now. "starting" counts: a VM is + // handed its memory before it is ready, and a boot marks everything it is + // about to start before it starts any of it, so two sandboxes coming up + // together split the budget instead of racing for it. + const activeSandboxes = () => + state.sandboxes.filter((item) => item.runtime.kind === "ready" || item.runtime.kind === "starting").length + const stopSandboxInternal = (id: string) => { const existing = running.get(id) if (!existing) return @@ -182,7 +190,12 @@ export function createSandboxController(options: SandboxControllerOptions) { setRuntime(id, { kind: "starting" }) logger?.log("sandbox starting", { id, backend: item.config.backend }) try { - const sandbox = await requireProvider(item.config.backend).start(item.config) + // Resolved here, after this sandbox is marked starting, so it counts + // itself: the share is the budget split across everything that will be + // up once it boots. + const sandbox = await requireProvider(item.config.backend).start(item.config, { + defaultMemoryMiB: sandboxDefaultMemoryMiB({ sandboxes: activeSandboxes() }), + }) if (!isCurrentStartAttempt(id, attempt)) { try { sandbox.stop() @@ -218,6 +231,14 @@ export function createSandboxController(options: SandboxControllerOptions) { getState() { return state }, + + // What a sandbox added right now would get if the user names no limits of + // its own — one more than what is up, since the new one joins them. The + // Add dialog quotes these as its field placeholders, so this has to be the + // same call the start path makes rather than a second copy of the rule. + defaultResources() { + return { cpus: SANDBOX_DEFAULT_CPUS, memoryMiB: sandboxDefaultMemoryMiB({ sandboxes: activeSandboxes() + 1 }) } + }, subscribe(listener: (event: SandboxesEvent) => void) { listeners.add(listener) return () => listeners.delete(listener) @@ -225,8 +246,14 @@ export function createSandboxController(options: SandboxControllerOptions) { async initialize() { const persisted = options.readSandboxes() + // Everything about to start is marked starting up front rather than as + // each start reaches it. The memory a sandbox gets is a share of one + // budget divided by what else is up, so marking them one at a time would + // hand the first sandbox the whole budget and starve the last — and the + // order is just the persisted one, which is no basis for the decision. setState({ sandboxes: persisted.map((config) => { + if (!config.stopped) return { config, runtime: { kind: "starting" as const } } const existing = state.sandboxes.find((item) => item.config.id === config.id) return { config, runtime: existing?.runtime ?? { kind: "stopped" } } }), diff --git a/packages/codev-sandbox/src/policy.test.ts b/packages/codev-sandbox/src/policy.test.ts index 7a118906ff..b16d6e705c 100644 --- a/packages/codev-sandbox/src/policy.test.ts +++ b/packages/codev-sandbox/src/policy.test.ts @@ -2,6 +2,7 @@ import { expect, test } from "bun:test" import { SANDBOX_CPUS_RANGE, SANDBOX_DEFAULT_MEMORY_CAP_MIB, + SANDBOX_DEFAULT_MEMORY_FLOOR_MIB, SANDBOX_MEMORY_MIB_RANGE, sandboxDefaultMemoryMiB, sandboxResourcesValid, @@ -58,19 +59,37 @@ test("sandbox resource limits accept sane values and reject the rest", () => { expect(sandboxResourcesValid({ cpus: Number.NaN })).toBe(false) }) -test("the default memory is half the host's RAM, capped and floored", () => { +test("one sandbox gets half the host's RAM, capped and floored", () => { const gib = 1024 ** 3 - expect(sandboxDefaultMemoryMiB(8 * gib)).toBe(4096) - expect(sandboxDefaultMemoryMiB(16 * gib)).toBe(8192) + expect(sandboxDefaultMemoryMiB({ totalBytes: 8 * gib })).toBe(4096) + expect(sandboxDefaultMemoryMiB({ totalBytes: 16 * gib })).toBe(8192) // Capped: a 64 GiB Mac would otherwise hand one sandbox 32 GiB, and each // running sandbox holds its reservation for as long as it runs. - expect(sandboxDefaultMemoryMiB(64 * gib)).toBe(SANDBOX_DEFAULT_MEMORY_CAP_MIB) + expect(sandboxDefaultMemoryMiB({ totalBytes: 64 * gib })).toBe(SANDBOX_DEFAULT_MEMORY_CAP_MIB) // Rounded down to a whole GiB rather than landing on 6.5 GiB. - expect(sandboxDefaultMemoryMiB(13 * gib)).toBe(6144) - // Floored at the range minimum, so a small machine still gets a bootable - // guest instead of a VM the validator would reject. - expect(sandboxDefaultMemoryMiB(2 * gib)).toBe(SANDBOX_MEMORY_MIB_RANGE.min) - expect(sandboxDefaultMemoryMiB(512 * 1024 * 1024)).toBe(SANDBOX_MEMORY_MIB_RANGE.min) + expect(sandboxDefaultMemoryMiB({ totalBytes: 13 * gib })).toBe(6144) + // Floored, so a small machine still gets a guest that can do something. + expect(sandboxDefaultMemoryMiB({ totalBytes: 2 * gib })).toBe(SANDBOX_DEFAULT_MEMORY_FLOOR_MIB) + expect(sandboxDefaultMemoryMiB({ totalBytes: 512 * 1024 * 1024 })).toBe(SANDBOX_DEFAULT_MEMORY_FLOOR_MIB) // Whatever this machine has, the result is always a usable configuration. expect(sandboxResourcesValid({ memoryMiB: sandboxDefaultMemoryMiB() })).toBe(true) }) + +test("sandboxes running together split the budget instead of each taking it", () => { + const gib = 1024 ** 3 + // 16 GiB Mac: 8 GiB of budget, so the second sandbox halves the share and + // the third takes a third of it — 2.67 GiB rounded down to whole GiB. + expect(sandboxDefaultMemoryMiB({ totalBytes: 16 * gib, sandboxes: 1 })).toBe(8192) + expect(sandboxDefaultMemoryMiB({ totalBytes: 16 * gib, sandboxes: 2 })).toBe(4096) + expect(sandboxDefaultMemoryMiB({ totalBytes: 16 * gib, sandboxes: 3 })).toBe(2048) + // The per-sandbox cap still applies to the share on a large machine: half of + // 64 GiB across two sandboxes is 16 GiB each, which is more than one guest + // may take. + expect(sandboxDefaultMemoryMiB({ totalBytes: 64 * gib, sandboxes: 2 })).toBe(SANDBOX_DEFAULT_MEMORY_CAP_MIB) + expect(sandboxDefaultMemoryMiB({ totalBytes: 64 * gib, sandboxes: 5 })).toBe(6144) + // The floor beats the budget: enough sandboxes at once do exceed half the + // machine, because a guest too small to work is the worse outcome. + expect(sandboxDefaultMemoryMiB({ totalBytes: 16 * gib, sandboxes: 8 })).toBe(SANDBOX_DEFAULT_MEMORY_FLOOR_MIB) + // A nonsense count cannot produce a nonsense share. + expect(sandboxDefaultMemoryMiB({ totalBytes: 16 * gib, sandboxes: 0 })).toBe(8192) +}) diff --git a/packages/codev-sandbox/src/provider.ts b/packages/codev-sandbox/src/provider.ts index 5255bf29c8..a447ae328c 100644 --- a/packages/codev-sandbox/src/provider.ts +++ b/packages/codev-sandbox/src/provider.ts @@ -48,26 +48,42 @@ export type ResolvedSandboxMount = { // asks the main process for them rather than keeping a copy. export const SANDBOX_DEFAULT_CPUS = 4 -// Memory scales with the host instead of sitting at a fixed 4 GiB, following -// the rule WSL applies to its utility VM: half the machine's RAM, capped. -// The cap is what makes the rule safe to borrow — unlike WSL's single shared -// utility VM, each sandbox here is its own VM holding its own reservation for -// as long as it runs, so an uncapped half would let two of them claim a whole -// machine. Rounded down to a whole GiB, and never below the floor a guest -// needs to boot the office toolchain at all. +// Sandboxes may collectively use half the machine's RAM; a single one may take +// at most this much of it. Borrowed from the rule Windows applies to the WSL +// utility VM, but WSL's ceiling is one budget shared by every distro, while +// each sandbox here is its own VM holding its own reservation for as long as +// it runs — so the budget has to be divided rather than handed out whole. export const SANDBOX_DEFAULT_MEMORY_CAP_MIB = 8192 +// The share never drops below this, however many sandboxes are up: a guest too +// small to run the office toolchain is not a useful outcome, and the user can +// still type something smaller. So enough concurrent sandboxes do exceed the +// budget — the floor wins on purpose. Above the validator's floor, which is +// what a deliberately-typed value may go down to. +export const SANDBOX_DEFAULT_MEMORY_FLOOR_MIB = 2048 + // Ceilings keep an unusable VM (0 cpus, 64MiB) or an oversubscribed host out // of the persisted config; the guest needs real headroom for the office // toolchain. Both ends inclusive. export const SANDBOX_CPUS_RANGE = { min: 1, max: 32 } as const export const SANDBOX_MEMORY_MIB_RANGE = { min: 1024, max: 65536 } as const -// totalBytes is injectable so the rule can be tested against machines this one -// is not; production always reads the host. -export function sandboxDefaultMemoryMiB(totalBytes = totalmem()) { - const halfInGiB = Math.floor(totalBytes / 2 / 1024 ** 3) - return Math.min(Math.max(halfInGiB * 1024, SANDBOX_MEMORY_MIB_RANGE.min), SANDBOX_DEFAULT_MEMORY_CAP_MIB) +// An even share of the budget for each sandbox that would then be running, +// whole GiB, clamped. Resolved when a sandbox starts rather than when it is +// created, because what else is up changes between the two — and only for the +// sandbox being started: a running VM's memory is fixed at boot, so starting a +// second sandbox shrinks the newcomer, never the one already up. +// +// CPUs are deliberately not shared out the same way. They are time-sliced, so +// handing every guest four of them oversubscribes nothing; memory is reserved, +// which is the whole reason this exists. +// +// sandboxes and totalBytes are injectable so the rule can be tested against +// machines and situations this one is not. +export function sandboxDefaultMemoryMiB(opts?: { sandboxes?: number; totalBytes?: number }) { + const budgetGiB = Math.floor((opts?.totalBytes ?? totalmem()) / 2 / 1024 ** 3) + const shareGiB = Math.floor(budgetGiB / Math.max(opts?.sandboxes ?? 1, 1)) + return Math.min(Math.max(shareGiB * 1024, SANDBOX_DEFAULT_MEMORY_FLOOR_MIB), SANDBOX_DEFAULT_MEMORY_CAP_MIB) } export function sandboxResourcesValid(config: { cpus?: number; memoryMiB?: number }) { @@ -110,7 +126,10 @@ export type SandboxProvider = { backend: SandboxBackend availability(): Promise create(config: SandboxConfig, opts?: SandboxJobOptions): Promise - start(config: SandboxConfig, opts?: { signal?: AbortSignal }): Promise + // defaultMemoryMiB is the share the controller worked out for this start + // from what else is running; a config that sets its own memory ignores it, + // and so do the backends that cannot apply limits at all. + start(config: SandboxConfig, opts?: { signal?: AbortSignal; defaultMemoryMiB?: number }): Promise destroy(config: SandboxConfig, opts?: { signal?: AbortSignal }): Promise reset(config: SandboxConfig, opts?: SandboxJobOptions): Promise // Applies config.mounts to the already-running sandbox without a restart. diff --git a/packages/codev-sandbox/src/vz/provider.test.ts b/packages/codev-sandbox/src/vz/provider.test.ts index ea2b52f786..db2b72a929 100644 --- a/packages/codev-sandbox/src/vz/provider.test.ts +++ b/packages/codev-sandbox/src/vz/provider.test.ts @@ -189,12 +189,16 @@ test("per-sandbox cpus and memory win over the provider defaults", async () => { const prepared = await preparedOptions(dir, FAKE_HELPER) const provider = createVzProvider({ ...prepared.options, cpus: 2, memoryMiB: 2048 }) - const sandbox = await provider.start({ - ...config, - projectDir: prepared.projectDir, - cpus: 8, - memoryMiB: 16384, - }) + const sandbox = await provider.start( + { + ...config, + projectDir: prepared.projectDir, + cpus: 8, + memoryMiB: 16384, + }, + // Loses to both: the share is what a sandbox gets when it named nothing. + { defaultMemoryMiB: 3072 }, + ) const vmConfig = JSON.parse(await readFile(join(prepared.dirs.runtime, "vm.json"), "utf8")) expect(vmConfig.cpus).toBe(8) expect(vmConfig.memoryMiB).toBe(16384) @@ -203,6 +207,19 @@ test("per-sandbox cpus and memory win over the provider defaults", async () => { await rm(dir, { recursive: true, force: true }) }) +test("the controller's share is used when the sandbox names no memory of its own", async () => { + const dir = await mkdtemp(join(tmpdir(), "codev-vz-")) + const prepared = await preparedOptions(dir, FAKE_HELPER) + const provider = createVzProvider(prepared.options) + + const sandbox = await provider.start({ ...config, projectDir: prepared.projectDir }, { defaultMemoryMiB: 3072 }) + const vmConfig = JSON.parse(await readFile(join(prepared.dirs.runtime, "vm.json"), "utf8")) + expect(vmConfig.memoryMiB).toBe(3072) + + sandbox.stop() + await rm(dir, { recursive: true, force: true }) +}) + test("surfaces helper error events with the stderr tail", async () => { const dir = await mkdtemp(join(tmpdir(), "codev-vz-")) const prepared = await preparedOptions(dir, FAILING_HELPER) diff --git a/packages/codev-sandbox/src/vz/provider.ts b/packages/codev-sandbox/src/vz/provider.ts index d1c4f360e5..7054ae8225 100644 --- a/packages/codev-sandbox/src/vz/provider.ts +++ b/packages/codev-sandbox/src/vz/provider.ts @@ -124,7 +124,7 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { await prepare(config, opts) }, - async start(config) { + async start(config, opts) { const dirs = vzSandboxDirs(options.dataDir, config.name) const project = await stat(config.projectDir).catch(() => null) if (!project?.isDirectory()) throw new Error(`project directory does not exist: ${config.projectDir}`) @@ -182,9 +182,12 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { initrd: initrd?.isFile() ? join(dirs.image, VZ_INITRD_FILE) : "", disk: dirs.disk, cmdline: VZ_CMDLINE, - // Per-sandbox limits win over the provider-wide defaults. + // Per-sandbox limits win over the provider-wide defaults, which in + // turn win over the share the controller worked out for this start. + // The trailing fallback is for a provider driven without a controller + // (the e2e scripts, the tests): nothing else is running there. cpus: config.cpus ?? options.cpus ?? SANDBOX_DEFAULT_CPUS, - memoryMiB: config.memoryMiB ?? options.memoryMiB ?? sandboxDefaultMemoryMiB(), + memoryMiB: config.memoryMiB ?? options.memoryMiB ?? opts?.defaultMemoryMiB ?? sandboxDefaultMemoryMiB(), shares: [ { tag: "workspace", path: config.projectDir, readOnly: false }, { tag: "bundle", path: options.bundleDir, readOnly: true }, diff --git a/packages/desktop/src/main/sandbox/controller.ts b/packages/desktop/src/main/sandbox/controller.ts index 47b99a8edd..db7991d874 100644 --- a/packages/desktop/src/main/sandbox/controller.ts +++ b/packages/desktop/src/main/sandbox/controller.ts @@ -2,7 +2,6 @@ import { dirname, join } from "node:path" import { fileURLToPath } from "node:url" import { app } from "electron" import { - SANDBOX_DEFAULT_CPUS, SANDBOX_MANIFEST_URL, createBwrapProvider, createSandboxController, @@ -10,7 +9,6 @@ import { createWslProvider, fetchSandboxManifest, requireImageAsset, - sandboxDefaultMemoryMiB, sandboxResourcesValid, } from "@codev/sandbox" import type { SandboxArch, SandboxAvailability, SandboxBackend, SandboxConfig, SandboxProvider } from "@codev/sandbox" @@ -43,10 +41,6 @@ export function createDesktopSandboxController(logger: ControllerLogger) { backend: provider?.backend ?? null, // What a custom image has to match to be usable here. arch: sandboxArch() ?? null, - // What a sandbox that sets no limits of its own gets. Memory depends on - // how much RAM this machine has, so the dialog cannot hold a copy of the - // number the way it does for the ranges — it has to ask. - defaults: () => ({ cpus: SANDBOX_DEFAULT_CPUS, memoryMiB: sandboxDefaultMemoryMiB() }), availability: (): Promise => { if (!provider) return Promise.resolve({ supported: false, message: `sandboxes are not supported on ${process.platform}` }) diff --git a/packages/desktop/src/main/sandbox/ipc.ts b/packages/desktop/src/main/sandbox/ipc.ts index 85a7e3be58..bd74a78a37 100644 --- a/packages/desktop/src/main/sandbox/ipc.ts +++ b/packages/desktop/src/main/sandbox/ipc.ts @@ -46,7 +46,9 @@ export function registerSandboxIpcHandlers(controller: DesktopSandboxController) ipcMain.handle("sandbox-unsubscribe", (event) => unsubscribe(event.sender.id)) ipcMain.handle("sandbox-get-state", () => controller.getState()) ipcMain.handle("sandbox-availability", () => controller.availability()) - ipcMain.handle("sandbox-defaults", () => controller.defaults()) + // Depends on how many sandboxes are up, so it is read when the dialog needs + // it rather than cached in the renderer. + ipcMain.handle("sandbox-defaults", () => controller.defaultResources()) // Inspects a candidate image so the dialog can report the problem while the // user is still filling the form. Returns the same problem shape the add // path enforces, so the two can never disagree about what is acceptable. From 0dcf55ec9b6830eda2848f388190b415ca6c00ea Mon Sep 17 00:00:00 2001 From: minhn4 Date: Mon, 17 Aug 2026 22:20:43 +0700 Subject: [PATCH 3/3] fix(sandbox): size a sandbox against memory actually reserved, not head count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dividing the budget by the number of sandboxes assumed each one holds an even share, and none of them do. A sandbox that started alone took the whole budget and cannot give it back, so the next one was sized as though half were free: on a 16 GiB Mac, delete the small sandbox and add another and it took 4 GiB while the survivor already held all 8 — 12 GiB of VM on a 16 GiB machine. A hand-typed limit was invisible to the count for the same reason. The share is now the lesser of an even share and what is genuinely left, so both failure modes go: neither measure is right alone, because what-is-left on its own would floor the second sandbox even when the first holds very little. What is left is counted from what running sandboxes report reserving rather than from what the controller offered them: RunningSandbox gains an optional memoryMiB that the vz backend fills in with the figure the VM was really built with, whichever of its four sources won. Bubblewrap and WSL leave it absent — neither reserves anything a sandbox owns — so their sandboxes correctly count for nothing against the budget. Sandboxes still starting are absent from the running map and count for nothing either, which is what lets a boot divide the budget evenly between the sandboxes coming up together. Consequence worth stating: a second sandbox added beside one that started alone now gets the 2 GiB floor rather than 4 GiB. That is the budget being enforced rather than exceeded — restarting the app re-sizes both and splits it evenly, and an explicit Memory value overrides all of it. The renderer refetches the defaults on any change in the sandboxes' runtime kinds rather than on a change in how many are active: a sandbox reaching ready is when its memory is really taken, and that leaves the active count alone. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 21 +++++---- packages/app/src/sandbox/context.tsx | 18 ++++---- packages/codev-sandbox/README.md | 27 +++++++---- packages/codev-sandbox/src/controller.test.ts | 46 ++++++++++++++++++- packages/codev-sandbox/src/controller.ts | 23 ++++++++-- packages/codev-sandbox/src/policy.test.ts | 22 +++++++++ packages/codev-sandbox/src/provider.ts | 37 +++++++++++---- packages/codev-sandbox/src/vz/provider.ts | 4 ++ 8 files changed, 157 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f877ee5a0e..3451e8d2b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,16 +20,19 @@ file is the human-readable summary of what changed and why. sandbox used to get 4 GB whatever the machine, which is thin for the office toolchain on a 32 GB Mac and heavy-handed on an 8 GB one. Sandboxes may now use half the Mac's RAM between them, and a sandbox that names no Memory of - its own takes an even share of that: on a 16 GB Mac, 8 GB on its own, 4 GB - each when two run together, 2 GB each when three do — never more than 8 GB - and never less than 2 GB, whatever the machine and however many are up. The - share is worked out when a sandbox starts, so it depends on what is running - at that moment, and it only ever applies to the sandbox being started: a + its own gets an even share of that budget, or whatever is genuinely left of + it, whichever is smaller — never more than 8 GB and never less than 2 GB. On + a 16 GB Mac: 8 GB on its own, 4 GB each when two start together. The share + is worked out when a sandbox starts, from what the sandboxes already running + actually hold, and it only ever applies to the sandbox being started: a running sandbox's memory is fixed while it runs, so starting a second one - makes room by giving the newcomer less, never by shrinking the first. A - sandbox you have stopped holds nothing. The Memory field in the Add sandbox - dialog still overrides all of this, and shows the share the sandbox you are - about to create would get. + makes room by giving the newcomer less, never by shrinking the first. This + is why a second sandbox added alongside one that started alone gets 2 GB, + and why stopping or deleting a sandbox hands its memory straight back to the + next one — while restarting the app re-sizes every sandbox at once and + splits the budget evenly between them. The Memory field in the Add sandbox + dialog still overrides all of this, and shows what the sandbox you are about + to create would get. - **Sessions in a sandbox survive stopping it** — anything the sandbox had written in the seconds before it stopped was thrown away: stop a sandbox, or add a folder to one (which restarts it), or quit the app, and the sessions diff --git a/packages/app/src/sandbox/context.tsx b/packages/app/src/sandbox/context.tsx index 7cfdb9292b..ea02853df7 100644 --- a/packages/app/src/sandbox/context.tsx +++ b/packages/app/src/sandbox/context.tsx @@ -51,17 +51,17 @@ export const { use: useSandboxes, provider: SandboxesProvider } = createSimpleCo createEffect(() => { const api = platform.sandboxes if (!api) return - // The defaults are a share of one memory budget divided by the sandboxes - // that are up, so they move whenever that set does — but not on the - // other state events, which include every download-progress tick. - let active = -1 + // The defaults come out of one memory budget shared by the sandboxes + // that are up, so they move whenever a sandbox appears, disappears, or + // changes state — a sandbox reaching ready is when its memory is really + // taken. Refetching on the runtime kinds rather than on every event + // keeps download-progress ticks, which carry no such change, out of it. + let runtimes = "" const off = api.subscribe((event) => { queryClient.setQueryData(sandboxesQueryKey, event.state) - const next = event.state.sandboxes.filter( - (item) => item.runtime.kind === "ready" || item.runtime.kind === "starting", - ).length - if (next === active) return - active = next + const next = event.state.sandboxes.map((item) => `${item.config.id}:${item.runtime.kind}`).join() + if (next === runtimes) return + runtimes = next void queryClient.invalidateQueries({ queryKey: sandboxDefaultsQueryKey }) }) onCleanup(off) diff --git a/packages/codev-sandbox/README.md b/packages/codev-sandbox/README.md index 94433b5d63..36bc168fce 100644 --- a/packages/codev-sandbox/README.md +++ b/packages/codev-sandbox/README.md @@ -15,15 +15,19 @@ Electron main process. `applyMounts` hot path. Also the persisted `SandboxConfig` shape (project dir, extra mounts, per-sandbox cpu/memory limits) and its validation. A sandbox that sets no limits gets `SANDBOX_DEFAULT_CPUS` and - `sandboxDefaultMemoryMiB({ sandboxes })` — sandboxes may use half the host's - RAM between them, and each takes an even share of that budget, whole GiB, - capped at 8 GiB and floored at 2 GiB. The budget is WSL's rule for its - utility VM; the split is what makes it safe to borrow, since WSL's ceiling - covers every distro at once while each vz sandbox holds its own reservation. - CPUs are not shared out the same way — they are time-sliced, so four each - oversubscribes nothing. The Add dialog reads the resolved numbers back over - the `sandbox-defaults` IPC channel rather than mirroring them: they depend - on the machine *and* on what is running. + `sandboxDefaultMemoryMiB({ sandboxes, committedMiB })` — sandboxes may use + half the host's RAM between them, and each gets an even share of that budget + or whatever is genuinely left of it, whichever is smaller, in whole GiB, + capped at 8 GiB and floored at 2 GiB. Both measures are needed: an even + share alone ignores that a sandbox which started alone took the whole budget + and cannot give it back, while what-is-left alone would floor the second + sandbox even when the first holds very little. The budget is WSL's rule for + its utility VM; the sharing is what makes it safe to borrow, since WSL's + ceiling covers every distro at once while each vz sandbox holds its own + reservation. CPUs are not shared out the same way — they are time-sliced, so + four each oversubscribes nothing. The Add dialog reads the resolved numbers + back over the `sandbox-defaults` IPC channel rather than mirroring them: + they depend on the machine *and* on what is running on it. - `controller.ts` — lifecycle state machine: runtime states, the job queue, stale-start guards. Create, reset and destroy run **one at a time across all sandboxes** — not for the sandboxes' sake, but because create and reset stage @@ -37,7 +41,10 @@ Electron main process. boot splits the memory budget evenly rather than letting persisted order decide who gets it. Being the only thing that knows what is up, the controller also resolves each start's `defaultMemoryMiB` and answers - `defaultResources()` for the Add dialog. + `defaultResources()` for the Add dialog — totalling what running sandboxes + reported reserving (`RunningSandbox.memoryMiB`) rather than what it offered + them, since a sandbox may have named its own limit and two of the three + backends reserve nothing at all. - `policy.ts` — OS version gates: hard block below macOS 12, tiered warn-never-block on Windows, always-supported on Linux. - `manifest.ts` / `image.ts` — pinned manifest parsing and content-addressed, diff --git a/packages/codev-sandbox/src/controller.test.ts b/packages/codev-sandbox/src/controller.test.ts index 1a4393f9b8..7b1c7ce223 100644 --- a/packages/codev-sandbox/src/controller.test.ts +++ b/packages/codev-sandbox/src/controller.test.ts @@ -8,13 +8,16 @@ type FakeRunning = RunningSandbox & { exit: (code: number | null, signal: NodeJS.Signals | null) => void } -function fakeRunning(url: string): FakeRunning { +function fakeRunning(url: string, memoryMiB?: number): FakeRunning { let onExit: ((code: number | null, signal: NodeJS.Signals | null) => void) | undefined const sandbox: FakeRunning = { url, username: "codev", password: "secret", mounts: [], + // Left absent unless a test asks for it, matching the backends that + // reserve nothing — which is what the fake's own bwrap backend does. + ...(memoryMiB === undefined ? {} : { memoryMiB }), stopped: false, stop: () => { sandbox.stopped = true @@ -812,3 +815,44 @@ test("a stopped sandbox holds no share of the memory budget", async () => { await waitForRuntimeKind(controller, other.id, "ready") expect(shares.other).toBe(sandboxDefaultMemoryMiB({ sandboxes: 2 })) }) + +test("the next sandbox is sized against what the running ones actually reserved", async () => { + const shares: Record = {} + const controller = createSandboxController({ + providers: { + bwrap: fakeProvider({ + // Reports what it reserved, the way the vz backend does. Bubblewrap + // and WSL reserve nothing and report nothing, so their sandboxes + // never consume the budget. + start: async (target, opts) => { + shares[target.name] = opts?.defaultMemoryMiB + return fakeRunning("http://127.0.0.1:4096", opts?.defaultMemoryMiB) + }, + }), + }, + readSandboxes: () => [config], + writeSandboxes: () => undefined, + }) + await controller.initialize() + await waitForRuntimeKind(controller, config.id, "ready") + const alone = sandboxDefaultMemoryMiB({ sandboxes: 1 }) + expect(shares.default).toBe(alone) + + // The first sandbox started alone and took its share of the budget with it, + // so the second is sized against what that leaves rather than against a head + // count that cannot see it. + const second = sandboxDefaultMemoryMiB({ sandboxes: 2, committedMiB: alone }) + expect(controller.defaultResources().memoryMiB).toBe(second) + await controller.addSandbox(other) + await waitForRuntimeKind(controller, other.id, "ready") + expect(shares.other).toBe(second) + + // Deleting the first frees exactly what it held, so the next sandbox is + // sized against the one still running — not against the pair that once was. + await controller.removeSandbox(config.id) + expect(controller.defaultResources().memoryMiB).toBe(sandboxDefaultMemoryMiB({ sandboxes: 2, committedMiB: second })) + + // Stopping the survivor hands its reservation back too. + await controller.stopSandbox(other.id) + expect(controller.defaultResources().memoryMiB).toBe(alone) +}) diff --git a/packages/codev-sandbox/src/controller.ts b/packages/codev-sandbox/src/controller.ts index 9becb72af6..aa151df218 100644 --- a/packages/codev-sandbox/src/controller.ts +++ b/packages/codev-sandbox/src/controller.ts @@ -170,6 +170,14 @@ export function createSandboxController(options: SandboxControllerOptions) { const activeSandboxes = () => state.sandboxes.filter((item) => item.runtime.kind === "ready" || item.runtime.kind === "starting").length + // Memory the running sandboxes actually hold, which is not what they were + // offered: one may have named its own limit, and a backend that reserves + // nothing reports nothing. Sandboxes still starting are absent from + // `running` and so count for nothing here — deliberately, since a boot + // brings several up at once and the even share is what divides the budget + // between them. + const committedMemoryMiB = () => [...running.values()].reduce((total, sandbox) => total + (sandbox.memoryMiB ?? 0), 0) + const stopSandboxInternal = (id: string) => { const existing = running.get(id) if (!existing) return @@ -192,9 +200,12 @@ export function createSandboxController(options: SandboxControllerOptions) { try { // Resolved here, after this sandbox is marked starting, so it counts // itself: the share is the budget split across everything that will be - // up once it boots. + // up once it boots, and never more than the budget still has left. const sandbox = await requireProvider(item.config.backend).start(item.config, { - defaultMemoryMiB: sandboxDefaultMemoryMiB({ sandboxes: activeSandboxes() }), + defaultMemoryMiB: sandboxDefaultMemoryMiB({ + sandboxes: activeSandboxes(), + committedMiB: committedMemoryMiB(), + }), }) if (!isCurrentStartAttempt(id, attempt)) { try { @@ -237,7 +248,13 @@ export function createSandboxController(options: SandboxControllerOptions) { // Add dialog quotes these as its field placeholders, so this has to be the // same call the start path makes rather than a second copy of the rule. defaultResources() { - return { cpus: SANDBOX_DEFAULT_CPUS, memoryMiB: sandboxDefaultMemoryMiB({ sandboxes: activeSandboxes() + 1 }) } + return { + cpus: SANDBOX_DEFAULT_CPUS, + memoryMiB: sandboxDefaultMemoryMiB({ + sandboxes: activeSandboxes() + 1, + committedMiB: committedMemoryMiB(), + }), + } }, subscribe(listener: (event: SandboxesEvent) => void) { listeners.add(listener) diff --git a/packages/codev-sandbox/src/policy.test.ts b/packages/codev-sandbox/src/policy.test.ts index b16d6e705c..af18b2c78e 100644 --- a/packages/codev-sandbox/src/policy.test.ts +++ b/packages/codev-sandbox/src/policy.test.ts @@ -93,3 +93,25 @@ test("sandboxes running together split the budget instead of each taking it", () // A nonsense count cannot produce a nonsense share. expect(sandboxDefaultMemoryMiB({ totalBytes: 16 * gib, sandboxes: 0 })).toBe(8192) }) + +test("the share never exceeds what running sandboxes have left of the budget", () => { + const gib = 1024 ** 3 + const mac = { totalBytes: 16 * gib } + // One sandbox that started alone holds the whole 8 GiB budget, so the next + // one gets the floor rather than an even share the host cannot back. Head + // count alone would have said 4 GiB and put 12 GiB of a 16 GiB Mac in VMs. + expect(sandboxDefaultMemoryMiB({ ...mac, sandboxes: 2, committedMiB: 8192 })).toBe(SANDBOX_DEFAULT_MEMORY_FLOOR_MIB) + // A sandbox holding only its half leaves the other half for the newcomer: + // this is what a second sandbox gets after the one that took 8 GiB is + // deleted, and it lands exactly on the budget. + expect(sandboxDefaultMemoryMiB({ ...mac, sandboxes: 2, committedMiB: 4096 })).toBe(4096) + // An explicitly-oversized sandbox is seen for what it holds, not for being + // one sandbox — the count is blind to a hand-typed limit, the total is not. + expect(sandboxDefaultMemoryMiB({ ...mac, sandboxes: 2, committedMiB: 16384 })).toBe(SANDBOX_DEFAULT_MEMORY_FLOOR_MIB) + // Sandboxes coming up together have reserved nothing yet, so the even share + // is what divides the budget between them. + expect(sandboxDefaultMemoryMiB({ ...mac, sandboxes: 2, committedMiB: 0 })).toBe(4096) + // The even share still binds when plenty is free: a big machine does not + // hand the second of five sandboxes everything that is left. + expect(sandboxDefaultMemoryMiB({ totalBytes: 64 * gib, sandboxes: 5, committedMiB: 8192 })).toBe(6144) +}) diff --git a/packages/codev-sandbox/src/provider.ts b/packages/codev-sandbox/src/provider.ts index a447ae328c..fc0220c86a 100644 --- a/packages/codev-sandbox/src/provider.ts +++ b/packages/codev-sandbox/src/provider.ts @@ -68,22 +68,35 @@ export const SANDBOX_DEFAULT_MEMORY_FLOOR_MIB = 2048 export const SANDBOX_CPUS_RANGE = { min: 1, max: 32 } as const export const SANDBOX_MEMORY_MIB_RANGE = { min: 1024, max: 65536 } as const -// An even share of the budget for each sandbox that would then be running, -// whole GiB, clamped. Resolved when a sandbox starts rather than when it is -// created, because what else is up changes between the two — and only for the -// sandbox being started: a running VM's memory is fixed at boot, so starting a -// second sandbox shrinks the newcomer, never the one already up. +// The lesser of an even share of the budget and what is actually left of it, +// whole GiB, clamped. Two measures because neither alone is right: an even +// share ignores that a sandbox which started alone took the whole budget and +// cannot give any of it back, while what-is-left alone would hand the second +// sandbox the floor even when the first is only holding a modest amount. +// +// Resolved when a sandbox starts rather than when it is created, because what +// else is up changes between the two — and only for the sandbox being started: +// a running VM's memory is fixed at boot, so starting a second sandbox shrinks +// the newcomer, never the one already up. // // CPUs are deliberately not shared out the same way. They are time-sliced, so // handing every guest four of them oversubscribes nothing; memory is reserved, // which is the whole reason this exists. // -// sandboxes and totalBytes are injectable so the rule can be tested against -// machines and situations this one is not. -export function sandboxDefaultMemoryMiB(opts?: { sandboxes?: number; totalBytes?: number }) { +// Everything is injectable so the rule can be tested against machines and +// situations this one is not. committedMiB counts only what running sandboxes +// actually reserved, which is why the controller asks the backends rather than +// assuming they took what it offered. +export function sandboxDefaultMemoryMiB(opts?: { sandboxes?: number; committedMiB?: number; totalBytes?: number }) { const budgetGiB = Math.floor((opts?.totalBytes ?? totalmem()) / 2 / 1024 ** 3) const shareGiB = Math.floor(budgetGiB / Math.max(opts?.sandboxes ?? 1, 1)) - return Math.min(Math.max(shareGiB * 1024, SANDBOX_DEFAULT_MEMORY_FLOOR_MIB), SANDBOX_DEFAULT_MEMORY_CAP_MIB) + // Can go negative once the floor has overdrawn the budget; the clamp below + // is what stops that from mattering. + const freeGiB = Math.floor((budgetGiB * 1024 - (opts?.committedMiB ?? 0)) / 1024) + return Math.min( + Math.max(Math.min(shareGiB, freeGiB) * 1024, SANDBOX_DEFAULT_MEMORY_FLOOR_MIB), + SANDBOX_DEFAULT_MEMORY_CAP_MIB, + ) } export function sandboxResourcesValid(config: { cpus?: number; memoryMiB?: number }) { @@ -108,6 +121,12 @@ export type RunningSandbox = { username: string | null password: string mounts: ResolvedSandboxMount[] + // What this sandbox actually reserved, for the controller to size the next + // one against. Absent from the backends that reserve nothing — bubblewrap + // shares the host's memory like any other process, and WSL's limits belong + // to a utility VM this sandbox does not own — so their sandboxes correctly + // count for nothing against the budget. + memoryMiB?: number stop: () => void onExit: (cb: (code: number | null, signal: NodeJS.Signals | null) => void) => void } diff --git a/packages/codev-sandbox/src/vz/provider.ts b/packages/codev-sandbox/src/vz/provider.ts index 7054ae8225..9c6c360055 100644 --- a/packages/codev-sandbox/src/vz/provider.ts +++ b/packages/codev-sandbox/src/vz/provider.ts @@ -279,6 +279,10 @@ export function createVzProvider(options: VzProviderOptions): SandboxProvider { username: credentials.username, password: credentials.password, mounts: guestMounts.map((mount) => ({ path: mount.path, guestPath: mount.guestPath })), + // The figure the VM was actually built with, whichever of the four + // sources above won — the controller sizes the next sandbox against + // this rather than against what it offered. + memoryMiB: vmConfig.memoryMiB, stop: () => { // Ask the helper for a guest stop; escalate to kill if it hangs. child.stdin?.write('{"cmd":"stop"}\n')