From fbfae7df6ba6a7cc3912d9ca846be9a4e24f6ece Mon Sep 17 00:00:00 2001 From: Quickbeard Date: Sat, 22 Aug 2026 16:31:17 +0700 Subject: [PATCH] test(desktop): run the desktop test suite in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The package shipped 21 test files and no `test` script, so `bun turbo test` skipped it entirely: 126 passing tests that gated nothing. Add the script (scoped to `src`, like `@opencode-ai/ui`, so the built `out/` tree is never scanned) and register the matching turbo task — CI only runs test tasks declared in turbo.json, so the script alone would not have been enough. No `^build` dependency: the workspace packages the tests import (`@codev/sandbox`, `@codev/gateway`, `@opencode-ai/app` types) all export TypeScript source. Co-Authored-By: Claude Opus 5 (1M context) --- DIVERGENCES.md | 3 ++- packages/desktop/package.json | 1 + turbo.json | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/DIVERGENCES.md b/DIVERGENCES.md index c7e98444db..9058b4d0a5 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -208,6 +208,7 @@ from the squash message first, or use a plain merge. | Startup proxy-stall gate: `packages/desktop/src/main/{index,ipc}.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx`, `packages/app/src/app.tsx`, `packages/app/src/context/platform.tsx` | Three additive pieces. (1) `main/index.ts` fires `session.defaultSession.resolveProxy(url)` for the sidecar URL as soon as the port is known (fire-and-forget, result logged). (2) A `local-server-health` IPC chain: main answers with the Node-fetch `checkHealth` against the spawned sidecar once `serverReady` resolves (`false` if initialization failed), preload exposes `localServerHealth()`, the renderer platform maps it, and `platform.tsx` gains the optional `localServerHealth?()` capability. (3) `app.tsx` `ConnectionGate`: the health-check effect is restructured around an inner `poll`; when the 10 s blocking window expires without a healthy answer and the connection is the built-in sidecar, it asks `platform.localServerHealth` — if the host process vouches for the server it keeps blocking on the splash for up to another 50 s instead of showing the unreachable error. Background mode and `type: "http"` behavior are unchanged, and without the capability (web) the gate behaves exactly as upstream. | Chromium queues every renderer request — loopback included — until proxy initialization (PAC download / WPAD discovery) resolves; the implicit loopback-bypass rules are only applied once the resolution service is READY (`net/proxy_resolution/configured_proxy_resolution_service.cc`). On corporate networks that takes tens of seconds (measured 21 s vs 64 ms with an unreachable PAC host), so users behind a proxy saw "Could not reach Local Server" at every launch while the server was healthy the whole time — main's own Node-fetch health check (loopback in `NO_PROXY`) had already passed before the window opened. The prewarm overlaps proxy init with sidecar startup; the gate fallback distinguishes "server down" from "renderer network stack not ready". | Keep all three pieces; take upstream's surrounding changes. If upstream restructures `ConnectionGate`, re-apply the vouch-then-extend fallback around whatever its blocking window becomes. Keep the `local-server-health` channel name stable. | | `packages/desktop/src/main/updater.ts` | `autoUpdater.allowDowngrade` `true` → `false`; `autoUpdater.quitAndInstall()` → `quitAndInstall(true, true)`. | Upstream's `true` is safe against immutable GitHub release tags. The fork's feed is a mutable bucket path, where re-uploading or failing to replace a stale `latest.yml` would actively downgrade every client on the next 10-minute poll. The `quitAndInstall` arguments are load-bearing against the assisted installer (see the `electron-builder.config.ts` row): the defaults are `isSilent = false, isForceRunAfter = false`, and `NsisUpdater.doInstall` appends `/S` only when `isSilent` — so with an assisted installer the default would run the full wizard on **every** update, re-asking install mode and directory for a location the user already chose. `isForceRunAfter` is honoured only when `isSilent` is set (`BaseUpdater.quitAndInstall` passes `autoRunAppAfterInstall` instead when it is not), so both arguments are needed to install quietly *and* come back up. | Keep `false`; take upstream's other updater settings. Keep `quitAndInstall(true, true)` for as long as `nsis.oneClick` is `false`; the two only make sense together. `channel = "latest"` and `allowPrerelease = false` stay as upstream has them — the first must match `publish.channel` in `electron-builder.config.ts`, the second is inert for the `generic` provider (it only steers GitHub tag resolution). | | Dual-path WSL install: `packages/desktop/src/main/wsl/{runtime,servers}.ts` | `runtime.ts`: three internals gain `export` (`runPowerShell`, `runInteractiveCommand`, `resolveSystem32Command`) — no behavior change. `servers.ts`: the `install-wsl` job calls `installWslRuntimeDualPath` and the `install-distro` job calls `installWslDistroDualPath` (both from the fork-only `install-bundle.ts`, which holds all new logic) instead of `installWslRuntimeElevated` / `installWslDistro` directly; the `install-wsl` job additionally treats exit 3010 (`WSL_INSTALL_REBOOT_REQUIRED`, re-raised by the fork's elevated script) as success with `pendingRestart: true` rather than as a failure — `wsl --version` answers from the freshly installed MSI before the just-enabled VirtualMachinePlatform is active, so no post-install probe can detect the pending reboot; otherwise both return the same `WslCommandResult` shape and the surrounding job/error handling is untouched. | The customer fleet sits behind whitelist-only proxies that cannot reach Microsoft's servers. The dual path downloads from the codev storage host — the runtime as a per-architecture MSI named by its own manifest (`/codev/installers/wsl/manifest.json`, assets under `/codev/installers/wsl//`) and staged through `stageAsset` for resume + sha256 verification, the distro from the pinned `wsl.zip` — enables VirtualMachinePlatform and Microsoft-Windows-Subsystem-Linux via offline dism + installs the MSI behind one UAC prompt (Windows 10 requires the WSL optional component even for the standalone MSI — without it every `wsl` distro command fails with Wsl/WSL_E_WSL_OPTIONAL_COMPONENT_REQUIRED; on machines that already have it dism is a no-op), and falls back to upstream's `wsl --install` / `--web-download` paths only when the download cannot be *obtained* — a completed-but-failed elevated install is returned as-is, never retried behind a second UAC prompt. | Keep the two call swaps, the 3010 handling in `install-wsl`, and the three `export` keywords; take upstream's surrounding changes. If upstream reworks `installWslRuntimeElevated` / `installWslDistro`, keep them exported from `runtime.ts` — `install-bundle.ts` calls them as the fallback path. | +| `packages/desktop/package.json` | Added the `test` script (`bun test src --only-failures`). | The package had 21 test files and no way to run them: with no `test` script, `bun turbo test` skipped it and every desktop test was dead weight. Scoped to `src` (like `@opencode-ai/ui`) so the built `out/` tree is never scanned. Paired with the `turbo.json` row — the script alone does not enrol the package in CI. | Keep ours. If upstream adds its own `test` script, take theirs only if it covers `src`. | | 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. | @@ -276,7 +277,7 @@ from the squash message first, or use a plain merge. | `packages/core/src/plugin/skill.ts`, `packages/opencode/src/skill/index.ts`, `packages/core/test/plugin/skill.test.ts` | Built-in skill `customize-opencode` → `customize-codev` in both registrations (plus `location` `/builtin/customize-codev.md`); descriptions rewritten to name CoDev Code; the markdown body wrapped in `rebrand(...)`; `Skill.get`/`Skill.require` resolve the old name through a `lookup` helper; test expectations updated | The skill's name and description are listed in the system prompt, so they are among the most visible brand strings the model sees; the body is upstream's config guide. The alias keeps existing configs and `skill` permission rules that name `customize-opencode` working. | Keep our name, the alias, and the `rebrand` wrapper; take upstream's `customize-opencode.md` body verbatim (it is deliberately not hand-edited for branding). | | `packages/opencode/src/command/template/initialize.txt` | "OpenCode" → "CoDev Code" (×2) | `/init` writes this guidance into the user's `AGENTS.md`. Edited in place rather than wrapped because the file is already fork-divergent (config-file rename row) and barely churns. | Keep our two strings; take upstream's surrounding text and rebrand any new "OpenCode" mention. | | `packages/core/package.json` | Added dependency `"@codev/brand": "workspace:*"` | The built-in skill registration rebrands its body through the fork-only `packages/codev-brand`. | Keep our dependency line; take upstream's other fields. | -| `turbo.json` | Added task `"@codev/sandbox#test"` (no deps, no outputs) | CI's `bun turbo test` only runs test tasks registered here; the fork-only sandbox package's tests must gate PRs. | Keep our task entry; take upstream's other tasks. | +| `turbo.json` | Added tasks `"@codev/sandbox#test"` and `"@opencode-ai/desktop#test"` (both no deps, no outputs) | CI's `bun turbo test` only runs test tasks registered here; the fork-only sandbox package's tests must gate PRs, and the desktop package ships 21 test files that ran nowhere until its task was registered. Neither needs `^build`: both packages' workspace dependencies export TypeScript source. | Keep our task entries; take upstream's other tasks. | | Sandbox project label fallback: `packages/app/src/context/global.tsx` | `enrich()` (the per-server project-list decorator) gains one fork line: when the merged project record has no `name` yet, it fills it from the fork-only `sandboxProjectHostName(conn, worktree, primary)` (`packages/app/src/sandbox/projects.ts`), which maps a sandbox connection's guest paths back to host folder basenames — `conn.projectDir` for the primary (matched by the synced primary directory, or the `/workspace` literal before sync loads) and `conn.mounts[].path` for extras. Non-sandbox connections and named projects are untouched (the helper returns `undefined` / the branch is skipped). | A fresh sandbox project renders `displayName()` = name-or-worktree-basename, and inside the guest the primary worktree is `/workspace`, so every new sandbox flashed the label "workspace" until `adoptFolderName`'s server-side rename round-tripped (guest reports the project → `project.update` → `project.updated` event). The fallback is display-only; the durable rename through the server is unchanged and takes precedence the moment it lands. | Keep the one `if (!base.name)` line after upstream's `base` construction and take upstream's surrounding `enrich` changes. If upstream renames or restructures `enrich`, re-apply the fallback wherever project records are decorated for the sidebar list. The helper stays in the fork-only sandbox module — do not inline it. | | Sandbox/WSL delete cleanup + confirmation: `packages/app/src/context/server.tsx`, `packages/app/src/components/dialog-select-server.tsx`, `packages/app/src/i18n/{en,es}.ts` | `server.tsx` gains `forget(key)` beside `remove(key)`: it deletes the persisted per-server `projects`/`lastProject`/`recentlyClosed` map entries for one key. `dialog-select-server.tsx` `handleRemove` (already diverged, see the renderer-slice row) additionally: for `sandbox:` and `wsl:` keys, awaits a confirmation through the fork-only `packages/app/src/sandbox/dialog-delete.tsx` / `packages/app/src/wsl/dialog-delete.tsx` (lazy-imported, pushed with `dialog.push` so it stacks on whatever dialog opened the menu) and returns without touching anything when declined; and calls `server.forget(key)` right after `server.remove(key)` for both key kinds. The controller gains a `useDialog()` binding for this. i18n gains the `sandbox.delete.*` and `wsl.delete.*` families. The two dialogs state different consequences on purpose: deleting a sandbox erases its disk (sessions, terminals, guest files — permanently; mounted host folders survive), while removing a WSL server destroys nothing — the app never unregisters distributions — it only forgets the app-side project state. | `remove()` only edits the stored HTTP server list; the project maps are keyed by server key and outlive the server, which is right for an HTTP server that still exists remotely but wrong for a deleted sandbox or removed distro — a recreated sandbox with the same name inherited the old projects (`server.projects["sandbox:"]`) and, via the leftover `recentlyClosed`, silently refused to seed folders closed in the previous sandbox's life; a re-added distro likewise inherited its previous project list. And deleting a sandbox erases guest data permanently, so it must not be a one-click menu action; the confirmations live in `handleRemove` because the settings rows and the generic server list all funnel through it. | Keep the confirm-and-forget block at the top of `handleRemove` and the `forget` calls between `server.remove(key)` and the awaited platform removals; take upstream's surrounding changes. `forget` is fork-only API — if upstream reworks the persisted `server` record shape, re-point the three deletes at the new maps. Do not gate `forget` behind the platform removal succeeding: the tab/workspace cleanup ahead of it already treats the server as gone. HTTP removals deliberately keep their maps — the server still exists remotely and re-adding it should restore its projects. If the app ever gains real distro unregistration, the WSL dialog's "nothing is deleted" wording must change with it. | | Vanished-stream retry + visibility + persisted retry parts: `packages/opencode/src/provider/error.ts`, `packages/opencode/src/session/{processor,retry,prompt,message-v2}.ts`, `packages/session-ui/src/components/message-part.tsx`, `packages/tui/src/routes/session/index.tsx`, `packages/ui/src/i18n/{en,es}.ts`, tests `packages/opencode/test/session/{retry,message-v2,processor-effect}.test.ts`, `packages/opencode/test/cli/run/run-process.test.ts` | error.ts: new `ProviderError.StreamVanishedError`. processor.ts: `ProcessorContext.produced` (set on `text-delta`/`tool-input-start`/`tool-call`, reset per attempt); after the stream drains, a turn with no output, no error, and finish `undefined`/`"unknown"` fails with `StreamVanishedError` so the existing `SessionRetry.policy` retries it (visible "retrying… attempt #N" status). retry.ts: `retryable()` gains an optional `attempt` param (policy passes `meta.attempt`); `STREAM_VANISHED_CODE` matches `metadata.code` and stops after `STREAM_VANISHED_MAX_RETRIES` (3) so a deterministic provider quirk cannot re-bill turns forever. message-v2.ts: maps `StreamVanishedError` → `APIError{isRetryable:true, metadata.code}`; new `transportInterruptCode()` walks the cause chain for `ECONNRESET`/`EPIPE`/`ETIMEDOUT`/`UND_ERR_SOCKET` so undici's mid-stream `TypeError("terminated")` (cause `SocketError "other side closed"`) becomes a retryable `APIError` instead of an unretryable `Unknown` (AbortedError when `ctx.aborted`). prompt.ts: the run-loop exit records a visible `APIError` ("the reply may be incomplete") on a message whose finish is `"unknown"` with no error, instead of going idle silently — mirrors upstream's own content-filter precedent. Persisted retry parts: the schema's dormant `RetryPart` (already in `@opencode-ai/schema/session-v1` and the generated SDKs, previously never written or rendered) is now written per retry attempt — `SessionRetry.policy`'s `set` callback gains the parsed `error`, and processor.ts persists `{type:"retry", attempt, error, time}` via `session.updatePart` before publishing the live retry status — and rendered by a new `PART_MAPPING["retry"]` component in session-ui's message-part.tsx (error-card style, message + "retried - attempt #N" line; new i18n key `ui.messagePart.retry.retried` en/es) and by a `PART_MAPPING["retry"]` entry in the TUI's routes/session/index.tsx (a muted one-liner aligned with the transcript — error-colored `Retried` label, `(attempt #N) · `, mirroring the footer's live `Retrying (attempt N) · message` wording; fork-only view test `packages/tui/test/retry-part.view.test.tsx`). Upstream's two `codev run` subprocess tests that pin the old contract ("unknown stream finish preserves partial output and exits 0" — renamed here to "…retries and preserves partial output" — and "--format json records partial output for an unknown stream finish") are updated to the retrying behaviour: each queues an explicit reply after `llm.fail(...)` so the retry consumes it instead of the test server's auto-"ok", stdout carries both turns, and the json test asserts the extra `step_start,text,step_finish` trio with the vanished step still finishing `"unknown"` and the retried one `"stop"`. What they lock in is unchanged: an SSE error item is not fatal — exit stays 0, pre-failure output survives, and the provider message never reaches stderr. | A provider (or corporate proxy) that drops the SSE mid-turn ended the run with no retry, no error card, no banner — the agent "just stopped" and only a user-typed "continue" resumed it. Empty turns are safely retried (nothing to duplicate); turns with output are not rerun but now say why they ended. | Keep our `produced` tracking, the post-drain fail, the classifier cap, the transport-code walk, the loop-exit error, and the two retry-part renderers (session-ui and TUI `PART_MAPPING["retry"]` entries); take upstream's surrounding changes. If upstream ships its own abnormal-finish handling, prefer theirs and drop ours where they overlap. Good upstream-PR candidate. The two `run-process.test.ts` expectations are ours only because the retry is: if upstream's version of that file conflicts, re-apply the queued-reply shape on top of their test body; if their behaviour is adopted instead, revert these two tests to upstream wholesale. | diff --git a/packages/desktop/package.json b/packages/desktop/package.json index dd280be21f..9396315b33 100644 --- a/packages/desktop/package.json +++ b/packages/desktop/package.json @@ -11,6 +11,7 @@ }, "scripts": { "typecheck": "tsgo -b", + "test": "bun test src --only-failures", "predev": "bun ./scripts/predev.ts", "dev": "electron-vite dev", "prebuild": "bun ./scripts/prebuild.ts", diff --git a/turbo.json b/turbo.json index be9fbf02f0..92b08e7bbe 100644 --- a/turbo.json +++ b/turbo.json @@ -29,6 +29,10 @@ "dependsOn": ["^build"], "outputs": [] }, + "@opencode-ai/desktop#test": { + "dependsOn": [], + "outputs": [] + }, "@codev/sandbox#test": { "dependsOn": [], "outputs": []