diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fd134e978..dc4dcd425e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ file is the human-readable summary of what changed and why. ### Desktop / Web UI +- **Deleting a sandbox no longer breaks the app** — deleting a sandbox while + one of its sessions was open could take the whole window down to the + "Something went wrong" screen, and because the app reopens whatever you were + last looking at, relaunching landed straight back on it. The app now returns + to the home screen when the thing a route points at is gone, so a deleted + sandbox (or any removed server) closes cleanly and a stale link cannot + wedge the app on startup. +- **"Waiting for another sandbox" no longer flashes when nothing is waiting** — + a create, reset, or delete with nothing queued ahead of it briefly announced + itself as waiting for another sandbox before starting. - **A sandbox can be created from an image file you already have** — creating a sandbox always downloaded its environment, which is well over a gigabyte and impossible on a machine with no route to the internet. **Add sandbox** diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 9f7bfd9e3a..41ed592ec9 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -189,6 +189,7 @@ from the squash message first, or use a plain merge. | `packages/desktop/src/main/index.ts`, `packages/desktop/src/main/windows.ts` | APP_NAMES + window title → "CoDev" (Dev/Beta variants too) | Desktop app display name ("Code" dropped once the app carried a Work section beside Code). | Keep our names. | | `packages/desktop/electron-builder.config.ts` | `productName` ×3 → "CoDev…"; artifactName → `codev-code-desktop-…` (deliberately kept when the product name dropped "Code" — the update feed's `latest*.yml` and the hand-upload flow reference these artifact names, and nothing user-visible shows them); protocol display name. Auto-update `publish` repointed off GitHub entirely: upstream's beta `anomalyco/opencode-beta` and prod `anomalyco/opencode` collapse to one shared `generic` config (`const publish`) pointing at the fork's MinIO bucket — `url` from `process.env.CODEV_UPDATE_URL` falling back to a base64 `UPDATE_URL` const (same `atob` convention as `packages/codev-brand/src/index.ts`), plus two fields that are load-bearing rather than cosmetic: `channel: "latest"` and `useMultipleRangeRequest: false`. Also a second `extraResources` entry shipping `../opencode/dist/node` → `server-bundle` (filter `node.js` + `*.wasm`, dropping the sourcemap, which outweighs the bundle) so the `@codev/sandbox` providers have a real on-disk server directory to mount into guests; `src/main/sandbox/bundle.ts` (fork-only) resolves it under `process.resourcesPath` by that name. Also the sandbox vz helper: `mac.extraResources` ships `packages/codev-sandbox/native/vzhelper/dist/codev-vmm` (built universal by `scripts/prebuild.ts`) as `Resources/codev-vmm`, and an `afterSign` hook (`signVmmHelper`) re-signs it with `vzhelper.entitlements` (`com.apple.security.virtualization` — an entitlement the Electron app itself must not carry) under the same identity that signed the app, hardened runtime on, timestamp only for non-ad-hoc, before electron-builder's built-in notarization runs. | Installer/product branding, and a packaged build must not offer upstream OpenCode releases as CoDev updates. GitHub could never have worked here: `quickbeard/codev-code` is private and `GitHubProvider.getLatestTagName` requests `/releases/latest` unauthenticated, and the repo has never carried a desktop asset. A bucket also splits nothing — one channel, one prefix. **`channel: "latest"` is required, not a default restated**: `detectUpdateChannel` is on by default, so with it absent `getResolvedPublishConfig` takes the channel from `AppInfo.channel`, which is the version's first semver prerelease component — `8` for `1.18.4-8` — and `getUpdateInfoFileName` then writes `8.yml`/`8-mac.yml`/`8-linux.yml` while the app requests `latest.yml` (`autoUpdater.channel` in `src/main/updater.ts`), i.e. a permanent `ERR_UPDATER_CHANNEL_FILE_NOT_FOUND`. **`useMultipleRangeRequest: false` is required for MinIO**, which answers a multipart range request with the whole object; electron-updater hardcodes the same `false` for its own `s3` provider citing minio/minio#5285 but defaults it to `true` for any `generic` URL that is not `s3.amazonaws.com`. Beta keeps a `publish` block despite the fork shipping no beta installers because `UPDATER_ENABLED` covers that channel and a build with no publish config gets no `app-update.yml` at all, so every check would fail with ENOENT. `${channel}` is deliberately NOT used as a URL macro — `macroExpander` expands it to that same `AppInfo.channel` (`8`), not `CODEV_CHANNEL`. appId is `ai.codev.desktop{,.beta,.dev}` and `rpm.packageName` is `codev`/`codev-beta`/`codev-dev` — see the desktop identity row for the full rename and what it costs. | Keep our names and the whole `publish` const; take upstream's other config. Never drop `channel` or `useMultipleRangeRequest` to "match upstream" — both are covered by `electron-builder.config.test.ts`. If upstream reworks its publish targets, re-point ours rather than merging theirs. Note `generic` has no publisher (`createPublisher` returns null), so installers reach the bucket by hand; build with `--publish never`. Keep the `server-bundle` extraResource and the `codev-vmm` extraResource + `afterSign` hook (all covered by `electron-builder.config.test.ts`) — renaming either strands packaged sandbox providers, and dropping `afterSign` ships a helper without the virtualization entitlement, which fails at VM creation (or fails notarization if left ad-hoc signed). | | `packages/app/src/utils/refcount.ts` (+ `.test.ts`) | `createRefCountMap` creates each item in its own root owned by the scope that created the map (disposed when the last pinning caller releases it), and callers without a reactive owner borrow the item without pinning it. | Upstream ran `create` in the *first caller's* scope, so a shared item's internal cleanups (e.g. the dir SDK context's event unsubscribe in `server-sdk.tsx`) fired when that first view closed while other holders still used the item — live updates for the directory silently died. Ownerless callers (event handlers, async continuations: `notification.tsx` session lookup, `layout.tsx` openSession) also registered cleanups Solid warns can never run and pinned items forever. | Keep ours; take upstream's surrounding changes. The two added tests ("ownerless caller borrows without pinning", "item's internal cleanups survive its first caller") pin the semantics. | +| `packages/app/src/app.tsx`, `packages/app/src/utils/session-route.ts` | `session-route.ts` gains `serverKeyFromRoute`, a non-throwing variant of `requireServerKey` (which is now defined in terms of it and kept for callers that want the throw). `app.tsx`'s `TargetServerRoute` and `LegacyTargetSessionRoute` read the route's server key through it: `TargetServerRoute` holds it in a memo, passes `undefined` rather than a bogus connection to the SDK/sync providers, and falls back to ``; `LegacyTargetSessionRoute` wraps its `SessionRouteErrorBoundary` in a `Show` on the same memo. | `requireServerKey` throws on a segment that is missing or does not round-trip, and both call sites ran it during render — so the throw escaped from a memo and killed the entire renderer with the fatal error screen. The param does go missing in normal use: removing a server navigates away (`tabs.removeServer` → `navigate("/")`) and the route's params clear before its subtree unmounts, which is how deleting a sandbox with an open session took the window down. Worse, the desktop restores the last active route on launch, so the crash repeated on every relaunch until the persisted URL changed. Rendering nothing was not enough on its own — a `/server/…` route that cannot mount its providers leaves the shell half-built and fails further down in the command registry — hence the redirect home. This is the same class as the `project-avatar-state.ts` guard below and is an upstream bug. | Keep the guard; take upstream's other changes to these files. If upstream makes the route resilient its own way, drop ours in favour of theirs. Do not "simplify" the route back to calling `requireServerKey` directly — that is the crash. | | `packages/app/src/context/server-sdk.tsx` | Event-stream failures log only after 3 consecutive failed attempts (`logStreamIssue`), with the error normalized to `name: message`; any received event resets the count. | The first failed attempt usually races a server being stopped or restarted (sandbox reset, sidecar relaunch): the stream dies before the renderer learns the server is going away and the context is disposed before a real retry, so logging attempt one produced a spurious `[global-sdk] event stream error` for normal lifecycle. A genuinely unreachable server still logs within about a second (250 ms reconnect delay). | Keep the threshold and shared `logStreamIssue`; take upstream's surrounding stream changes. | | `packages/app/src/context/command.tsx` | The DEV duplicate-command-id warning fires only for duplicates still present 1 s later (`warnIfPersistentDuplicate`); runtime keep-first behavior unchanged. | Router transitions keep the outgoing page mounted while the incoming one registers, so ids both pages provide (`command.palette`, `input.focus`) are briefly duplicated on every draft→session navigation; only a duplicate that survives the transition is a registration bug. | Keep ours; take upstream's surrounding command changes. | | `packages/desktop/src/main/logging.ts` | Added `initChromiumNoiseFilter`: an electron-log hook dropping two benign Chromium intervention messages (ResizeObserver loop, autofocus blocked) from the console transport for renderer-spied messages; file logs keep them. | The `spyRendererConsole` pipe surfaces Chromium interventions the app can neither cause nor fix as red terminal lines during `bun dev:desktop`. | Keep the hook; take upstream's surrounding logging changes. | diff --git a/packages/app/src/app.tsx b/packages/app/src/app.tsx index 433a8fbafa..670e3944f9 100644 --- a/packages/app/src/app.tsx +++ b/packages/app/src/app.tsx @@ -64,7 +64,7 @@ import LegacyLayout from "@/pages/layout" import NewLayout from "@/pages/layout-new" import { ErrorPage } from "./pages/error" import { useCheckServerHealth } from "./utils/server-health" -import { legacySessionHref, legacySessionServer, requireServerKey, sessionHref } from "./utils/session-route" +import { legacySessionHref, legacySessionServer, serverKeyFromRoute, sessionHref } from "./utils/session-route" import { createSessionLineage } from "@/pages/session/session-lineage" import { SessionPage, SessionRouteErrorBoundary, TargetSessionRouteContent } from "@/pages/session" @@ -111,16 +111,24 @@ const SessionRoute = () => { function TargetServerRoute(props: ParentProps) { const params = useParams<{ serverKey: string; id: string }>() const global = useGlobal() + // Absent while this route is being torn down: removing a server navigates + // away, and the param clears before this subtree unmounts. Rendering nothing + // is the right answer — throwing from a memo kills the whole renderer. + const key = createMemo(() => serverKeyFromRoute(params.serverKey)) const conn = createMemo(() => { - const key = requireServerKey(params.serverKey) - return global.servers.list().find((item) => ServerConnection.key(item) === key) + const current = key() + if (!current) return undefined + return global.servers.list().find((item) => ServerConnection.key(item) === current) }) return ( // Owns the server-identity remount. Session changes must NOT remount this // subtree (SessionRouteErrorBoundary resets and createSessionLineage // re-resolves reactively instead); both rely on this key for server changes. - + // An unreadable key means this route can never render: go home rather than + // sit on a route whose providers cannot mount, which leaves the shell in a + // half-built state that fails further down. + }> {props.children} @@ -136,11 +144,16 @@ const TargetSessionRoute = () => ( function LegacyTargetSessionRoute() { const params = useParams<{ serverKey: string; id: string }>() + const key = createMemo(() => serverKeyFromRoute(params.serverKey)) return ( - - - + + {(serverKey) => ( + + + + )} + ) } diff --git a/packages/app/src/utils/session-route.ts b/packages/app/src/utils/session-route.ts index 4721611d2e..85dfbbc63e 100644 --- a/packages/app/src/utils/session-route.ts +++ b/packages/app/src/utils/session-route.ts @@ -10,12 +10,22 @@ export function legacySessionHref(directory: string, sessionID: string) { return `/${base64Encode(directory)}/session/${sessionID}` } -export function requireServerKey(segment: string | undefined) { +// Undefined for a segment that is missing or does not round-trip. Route +// components must use this rather than requireServerKey: the param clears +// while a route is being torn down (removing a server navigates away), and a +// throw raised from a render or memo takes the entire renderer down with it. +export function serverKeyFromRoute(segment: string | undefined) { const key = decode64(segment) - if (!key || base64Encode(key) !== segment) throw new Error("Invalid server route") + if (!key || base64Encode(key) !== segment) return undefined return ServerConnection.Key.make(key) } +export function requireServerKey(segment: string | undefined) { + const key = serverKeyFromRoute(segment) + if (!key) throw new Error("Invalid server route") + return key +} + export function legacySessionServer( tabs: readonly { type: "session"; server: ServerConnection.Key; sessionId: string }[], sessionID: string, diff --git a/packages/codev-sandbox/src/controller.test.ts b/packages/codev-sandbox/src/controller.test.ts index 244f697c73..d2f1a5ab24 100644 --- a/packages/codev-sandbox/src/controller.test.ts +++ b/packages/codev-sandbox/src/controller.test.ts @@ -213,6 +213,22 @@ test("a job on one sandbox neither aborts nor overtakes another sandbox's job", expect(controller.getState().jobs).toEqual([]) }) +// A lone job briefly reported itself as queued, so the UI flashed "waiting for +// another sandbox" when there was no other sandbox to wait for. +test("a job with nothing ahead of it is never reported as queued", async () => { + const seen: string[] = [] + const controller = createSandboxController({ + providers: { bwrap: fakeProvider({ destroy: async () => undefined }) }, + readSandboxes: () => [config], + writeSandboxes: () => undefined, + }) + await controller.initialize() + controller.subscribe((event) => seen.push(...event.state.jobs.map((job) => job.status))) + await controller.removeSandbox(config.id) + expect(seen).not.toContain("queued") + expect(seen).toContain("running") +}) + test("cancelling a queued job returns at once and never starts it", async () => { const order: string[] = [] let resolveCreate: (() => void) | undefined diff --git a/packages/codev-sandbox/src/controller.ts b/packages/codev-sandbox/src/controller.ts index 9c77153ff3..edf0764187 100644 --- a/packages/codev-sandbox/src/controller.ts +++ b/packages/codev-sandbox/src/controller.ts @@ -113,7 +113,12 @@ export function createSandboxController(options: SandboxControllerOptions) { // A second job for the same sandbox supersedes the first: the caller asked // for the newer outcome and both would work on the same directories. queue.find((entry) => entry.job.id === id)?.abort.abort() - const job: SandboxJob = { kind, id, startedAt: Date.now(), status: "queued" } + // Only "queued" when something is genuinely ahead of it. A job that will + // start on the next microtask must not flash "waiting for another sandbox" + // at the user — and an entry already aborted (superseded just above) is on + // its way out, so it is not something to wait for. + const waiting = queue.some((entry) => !entry.abort.signal.aborted) + const job: SandboxJob = { kind, id, startedAt: Date.now(), status: waiting ? "queued" : "running" } const entry = { job, abort: new AbortController() } queue.push(entry) publishJobs()