From 2bfd5c229db16b870ea0a3db0f8f192fab79c51a Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Sun, 16 Aug 2026 13:02:27 +0000 Subject: [PATCH 1/2] feat(desktop): codex app-server transport, correcting L4a's dead daemon rung (L4b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L4a shipped `codex app-server proxy --sock ` as the daemon data path, on the strength of that subcommand's help text ("Proxy stdio bytes to the running app-server control socket"). It cannot carry the protocol, and it fails in the worst possible way: exit 0, no stdout, no stderr. Measured with a logging relay between the CLI and its own control socket: - `daemon version` — the vendor's OWN client for this socket — opens with `GET / HTTP/1.1` + `Upgrade: websocket`, takes `101 Switching Protocols`, and only then speaks JSON-RPC inside frames. - `app-server proxy --sock` forwards stdin verbatim with no upgrade. The daemon closes on the first byte of anything that is not an HTTP upgrade — valid JSON-RPC and pure garbage close identically, so it is not a parse error. So the control socket is a WebSocket server bound to a Unix domain socket (the daemon's own argv is `app-server --listen unix://`). The plan's original "WebSocket attach" was right and L4a's correction of it was wrong; what the plan actually got wrong was the auth — there is no bearer scheme, the socket is `srw-------` and filesystem permissions are the auth. Both causes L4a guessed at are disproven: not a framing question (Content-Length and half-close fail identically), and not `enable-remote-control` — the live round trip succeeds with remote control reporting `disabled` throughout, so that toggle stays off. - `CodexAttachPlan` becomes a discriminated union: a `daemon` plan carries a `socketPath` and cannot carry an argv, so the shipped bug is now unrepresentable rather than merely fixed. - `codexchannel.ts` opens either rung behind one send/onFrame surface. The spawn rung reassembles newline-delimited JSON across chunk boundaries; the WebSocket rung must NOT buffer, since a message with no trailing newline is already complete and holding it would stall the channel forever. - `perMessageDeflate: false` is load-bearing: `ws` offers permessage-deflate by default and the daemon hangs up on that handshake rather than declining the extension. Isolated by varying one option at a time against the live daemon. - A daemon that will not come up falls back to spawn and SAYS so — "shared with your codex TUI" and "dies with this window" are different promises. `close()` on a daemon channel closes our socket only; the rung exists so the session outlives us. - `codexBinary` resolves through PATH plus the well-known install dirs the way kimiweb.ts does, because the official installer writes ~/.local/bin/codex with its PATH line in .bashrc — which a GUI-launched Electron app never sources. Verified: 26 unit tests, 13 mutations all killed, and an opt-in live e2e (TERMIPOD_CODEX_ATTACH_E2E=1) that round-trips `initialize` against a real daemon. The deflate mutation is killed only by the e2e, with the real symptom — no unit test can see that handshake. Not yet reachable from main.ts, like L4a: this is the transport, and L4c is the driver that wires it in. No user-visible behaviour changes yet. Co-Authored-By: Claude Opus 5 --- .../src/localagent/codexattach.e2e.test.ts | 169 ++++++++ .../src/localagent/codexattach.test.ts | 98 ++++- .../electron/src/localagent/codexattach.ts | 179 +++++++-- .../src/localagent/codexchannel.test.ts | 267 +++++++++++++ .../electron/src/localagent/codexchannel.ts | 374 ++++++++++++++++++ docs/changelog-desktop.md | 17 + docs/plans/desktop-companion-vision-parity.md | 147 ++++--- 7 files changed, 1141 insertions(+), 110 deletions(-) create mode 100644 desktop/electron/src/localagent/codexattach.e2e.test.ts create mode 100644 desktop/electron/src/localagent/codexchannel.test.ts create mode 100644 desktop/electron/src/localagent/codexchannel.ts diff --git a/desktop/electron/src/localagent/codexattach.e2e.test.ts b/desktop/electron/src/localagent/codexattach.e2e.test.ts new file mode 100644 index 00000000..8ade37fd --- /dev/null +++ b/desktop/electron/src/localagent/codexattach.e2e.test.ts @@ -0,0 +1,169 @@ +/// End-to-end against a REAL codex app-server (vision-parity L4b). +/// +/// The unit tests fake both transports, which proves the plumbing and proves +/// nothing about the handshake — and the handshake is exactly where L4a went +/// wrong. It shipped `codex app-server proxy --sock ` as the daemon rung +/// on the strength of that subcommand's help text; against a live daemon the +/// proxy relays raw bytes without the WebSocket upgrade the control socket +/// requires, so the socket closes and the proxy exits **0 with no output**. A +/// dead channel that looks exactly like a quiet agent. +/// +/// So this file exists to ask the one question no fixture can answer: does a +/// frame we send come back answered? +/// +/// **Opt-in.** It brings up a shared background daemon on the operator's +/// machine — cheap (no model turn, no tokens) but a real side effect. So: +/// +/// TERMIPOD_CODEX_ATTACH_E2E=1 npm test +/// +/// If the daemon was not already running, the test stops it again afterwards +/// and leaves the box as it found it. Skipped without the variable, and skipped +/// with a clear reason when codex is absent or was not installed by the +/// official installer script (no managed standalone → no daemon rung at all, +/// which is the common case and not a failure). +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; + +import { + codexBinary, + managedCodexPath, + codexHome, + planCodexAttach, +} from './codexattach.ts'; +import { openCodexChannel, type CodexFrame } from './codexchannel.ts'; + +const ENABLED = process.env['TERMIPOD_CODEX_ATTACH_E2E'] === '1'; + +const HOME = os.homedir(); +const CHOME = codexHome(HOME, process.env); +const BIN = codexBinary(HOME, process.env); + +function managedInstallPresent(): boolean { + return fs.existsSync(managedCodexPath(CHOME)); +} + +/// Is the daemon up right now? `daemon version` is the vendor's own client for +/// the control socket, so this is their answer, not ours. +function daemonRunning(): boolean { + try { + const out = execFileSync(BIN, ['app-server', 'daemon', 'version'], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + timeout: 30_000, + }); + return (JSON.parse(out) as { status?: string }).status === 'running'; + } catch { + return false; + } +} + +function skipReason(): string | null { + if (!ENABLED) return 'set TERMIPOD_CODEX_ATTACH_E2E=1 to run the live codex attach test'; + if (BIN === 'codex') { + try { + execFileSync('codex', ['--version'], { stdio: 'ignore', timeout: 30_000 }); + } catch { + return 'codex is not installed'; + } + } + if (!managedInstallPresent()) { + return `no installer-managed codex at ${managedCodexPath(CHOME)} — this box has no daemon rung`; + } + return null; +} + +test('the daemon rung carries a real JSON-RPC round trip', async (t) => { + const skip = skipReason(); + if (skip !== null) { + t.skip(skip); + return; + } + + const wasRunning = daemonRunning(); + const plan = planCodexAttach(HOME, { managedInstallPresent: true, bin: BIN }, process.env); + assert.equal(plan.mode, 'daemon', 'a managed install must take the daemon rung'); + + const frames: CodexFrame[] = []; + let resolveInit: (() => void) | undefined; + const initialized = new Promise((r) => { + resolveInit = r; + }); + + const channel = await openCodexChannel( + plan, + { + onFrame: (f) => { + frames.push(f); + if (f['id'] === 1 && f['result'] !== undefined) resolveInit?.(); + }, + onClose: () => {}, + }, + { cwd: process.cwd(), connectTimeoutMs: 30_000 }, + ); + + try { + // If this reports 'spawn' we fell back, which means the socket handshake + // failed — the precise failure L4a shipped and could not see. + assert.equal(channel.mode, 'daemon', `expected the daemon rung, got ${channel.mode}: ${channel.reason}`); + + channel.send({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { clientInfo: { name: 'termipod', title: 'TermiPod', version: '0.1.0' } }, + }); + + await Promise.race([ + initialized, + new Promise((_, reject) => setTimeout(() => reject(new Error('no initialize result within 30s')), 30_000)), + ]); + + const result = frames.find((f) => f['id'] === 1)?.['result'] as Record | undefined; + assert.ok(result !== undefined, 'initialize must be answered'); + // The server's own view of where it lives — proof the answer came from a + // real app-server and not from an echo. + assert.equal(result['codexHome'], CHOME); + } finally { + channel.close(); + if (!wasRunning) { + try { + execFileSync(BIN, ['app-server', 'daemon', 'stop'], { stdio: 'ignore', timeout: 30_000 }); + } catch { + /* best effort — leaving a daemon up is untidy, not broken */ + } + } + } +}); + +test('closing our channel leaves the shared daemon running', async (t) => { + const skip = skipReason(); + if (skip !== null) { + t.skip(skip); + return; + } + + const wasRunning = daemonRunning(); + const plan = planCodexAttach(HOME, { managedInstallPresent: true, bin: BIN }, process.env); + const channel = await openCodexChannel( + plan, + { onFrame: () => {}, onClose: () => {} }, + { cwd: process.cwd(), connectTimeoutMs: 30_000 }, + ); + channel.close(); + + // The rung's entire promise is that the session outlives the app. If closing + // a client took the daemon down, every "your session survives" claim in the + // UI would be false. + assert.equal(daemonRunning(), true, 'the daemon must survive a client disconnect'); + + if (!wasRunning) { + try { + execFileSync(BIN, ['app-server', 'daemon', 'stop'], { stdio: 'ignore', timeout: 30_000 }); + } catch { + /* best effort */ + } + } +}); diff --git a/desktop/electron/src/localagent/codexattach.test.ts b/desktop/electron/src/localagent/codexattach.test.ts index a048e26a..ca8f735d 100644 --- a/desktop/electron/src/localagent/codexattach.test.ts +++ b/desktop/electron/src/localagent/codexattach.test.ts @@ -1,14 +1,16 @@ -/// L4a attach-rung checks. Every expectation here was measured against -/// codex-cli 0.147.0 on 2026-08-16 (`codex app-server --help`, `... daemon -/// --help`, `... proxy --help`, and a real `daemon start` refusal), not read -/// from the plan — whose L4 line described a WebSocket and a bearer scheme that -/// do not exist. Run with `node --test`. +/// L4a/L4b attach-rung checks. Every expectation here was measured against +/// codex-cli 0.147.0 (`codex app-server --help`, `... daemon --help`, a real +/// `daemon start`, and a logging relay placed between the CLI and its own +/// control socket), not read from the plan. Run with `node --test`. import { test } from 'node:test'; import assert from 'node:assert/strict'; import path from 'node:path'; import { + codexBinary, + codexBinDirs, codexHome, controlSocketPath, + findCodexOnPath, managedCodexPath, MAX_UNIX_SOCKET_PATH, planCodexAttach, @@ -33,25 +35,26 @@ test('no installer-managed codex: spawn, because `daemon start` would refuse', ( // no managed standalone install, and `daemon start` fails outright on it. const p = planCodexAttach(HOME, { managedInstallPresent: false }, {}); assert.equal(p.mode, 'spawn'); - assert.deepEqual(p.argv, ['codex', 'app-server']); - assert.equal(p.startArgv, undefined, 'spawn needs no preparation step'); + assert.deepEqual(p.mode === 'spawn' ? p.argv : null, ['codex', 'app-server']); assert.match(p.reason, /packages\/standalone\/current\/codex/); }); -test('installer-managed codex: attach to the shared daemon via the stdio proxy', () => { +test('installer-managed codex: the daemon rung is a SOCKET, never an argv', () => { const p = planCodexAttach(HOME, { managedInstallPresent: true }, {}); assert.equal(p.mode, 'daemon'); - // The transport is a Unix socket reached through `app-server proxy` — there - // is no WebSocket URL and no token anywhere in this argv. - assert.deepEqual(p.argv, [ - 'codex', - 'app-server', - 'proxy', - '--sock', - '/home/u/.codex/app-server-control/app-server-control.sock', - ]); + if (p.mode !== 'daemon') return; + assert.equal(p.socketPath, '/home/u/.codex/app-server-control/app-server-control.sock'); + // Bringing the daemon UP is a command; talking to it is not. assert.deepEqual(p.startArgv, ['codex', 'app-server', 'daemon', 'start']); - assert.ok(!p.argv.some((a) => /ws:|wss:|token|bearer/i.test(a)), 'no WebSocket or bearer anywhere in the argv'); + // L4a shipped `app-server proxy --sock ` as the data path. Measured + // against the live daemon, that subcommand relays raw stdin bytes without the + // WebSocket upgrade the socket requires, so the daemon closes the connection + // and the proxy exits 0 with no output at all. Nothing may reintroduce it. + assert.ok(!('argv' in p), 'a daemon plan must not carry an argv to run'); + assert.ok( + !JSON.stringify(p).includes('proxy'), + 'the `app-server proxy` subcommand cannot carry this protocol — it never upgrades', + ); }); test('a socket path over SUN_LEN disqualifies the daemon rung up front', () => { @@ -78,8 +81,61 @@ test('every rung explains itself', () => { }); test('the codex binary is overridable for a non-PATH install', () => { - // A GUI-launched Electron app does not inherit the login shell PATH — the - // problem kimiweb.ts already had to solve for kimi. const p = planCodexAttach(HOME, { managedInstallPresent: false }, { TERMIPOD_CODEX_BIN: '/opt/bin/codex' }); - assert.equal(p.argv[0], '/opt/bin/codex'); + assert.equal(p.mode === 'spawn' ? p.argv[0] : null, '/opt/bin/codex'); + // The override also reaches the daemon rung's start command, which is the + // only argv that rung has. + const d = planCodexAttach(HOME, { managedInstallPresent: true }, { TERMIPOD_CODEX_BIN: '/opt/bin/codex' }); + assert.equal(d.mode === 'daemon' ? d.startArgv[0] : null, '/opt/bin/codex'); +}); + +test('the well-known dirs cover the installer location a GUI app cannot see', () => { + // The official installer writes ~/.local/bin/codex and appends its PATH line + // to .bashrc — which an Electron app launched from a Dock icon never sources. + const dirs = codexBinDirs(HOME, {}); + assert.ok(dirs.includes('/home/u/.local/bin'), 'the installer target must be searched'); + assert.ok( + dirs.includes('/home/u/.codex/packages/standalone/current/bin'), + 'the managed standalone bin dir must be searched', + ); + // A relocated CODEX_HOME moves the managed dir with it. + assert.ok(codexBinDirs(HOME, { CODEX_HOME: '/srv/cx' }).includes('/srv/cx/packages/standalone/current/bin')); +}); + +test('findCodexOnPath returns an absolute path, and null when nothing exists', () => { + const present = new Set(['/home/u/.local/bin/codex']); + const exists = (p: string): boolean => present.has(p); + assert.equal(findCodexOnPath('/usr/bin:/bin', ['/home/u/.local/bin'], exists), '/home/u/.local/bin/codex'); + // PATH wins when it has one, since that is what the user's shell would run. + present.add('/usr/bin/codex'); + assert.equal(findCodexOnPath('/usr/bin:/bin', ['/home/u/.local/bin'], exists), '/usr/bin/codex'); + assert.equal(findCodexOnPath('/usr/bin', [], () => false), null); + // Empty PATH segments must not produce a relative candidate like "codex". + assert.equal(findCodexOnPath('', [], () => true), null); +}); + +test('codexBinary prefers the override, then a real file, then the bare name', () => { + assert.equal(codexBinary(HOME, { TERMIPOD_CODEX_BIN: '/opt/x' }, () => false), '/opt/x'); + assert.equal(codexBinary(HOME, { PATH: '/usr/bin' }, (p) => p === '/usr/bin/codex'), '/usr/bin/codex'); + // Nothing found: hand back the bare name and let the OS have its say, rather + // than failing here on a guess about what is installed. + assert.equal(codexBinary(HOME, { PATH: '/usr/bin' }, () => false), 'codex'); +}); + +test('codexBinary finds an installer-managed codex that is NOT on PATH', () => { + // The separating input, and the entire reason `codexBinDirs` exists: a GUI + // -launched Electron app inherits a PATH that never sourced .bashrc, so the + // official installer's ~/.local/bin/codex is invisible to it. A test whose + // codex is also on PATH cannot tell whether the well-known dirs are consulted + // at all — searching PATH alone would pass it just as well. + const onlyInInstallerDir = (p: string): boolean => p === '/home/u/.local/bin/codex'; + assert.equal( + codexBinary(HOME, { PATH: '/usr/bin:/bin' }, onlyInInstallerDir), + '/home/u/.local/bin/codex', + 'a codex reachable only from the installer dir must still be found', + ); + // Same for the managed standalone tree, which is where the installer's + // symlink actually points. + const onlyInManagedDir = (p: string): boolean => p === '/home/u/.codex/packages/standalone/current/bin/codex'; + assert.equal(codexBinary(HOME, { PATH: '/usr/bin' }, onlyInManagedDir), '/home/u/.codex/packages/standalone/current/bin/codex'); }); diff --git a/desktop/electron/src/localagent/codexattach.ts b/desktop/electron/src/localagent/codexattach.ts index a076ba17..ef018a2c 100644 --- a/desktop/electron/src/localagent/codexattach.ts +++ b/desktop/electron/src/localagent/codexattach.ts @@ -1,27 +1,53 @@ -/// How a local codex session reaches an app-server (vision-parity **L4a**). +/// How a local codex session reaches an app-server (vision-parity **L4a/L4b**). /// -/// codex speaks the same JSON-RPC-over-stdio app-server protocol whichever way -/// we reach it, so the only real decision is WHICH PROCESS we spawn — and that -/// decision is a pure function of what is installed. This module is that -/// function; the driver that speaks the protocol is L4b. +/// codex speaks the same JSON-RPC app-server protocol whichever way we reach +/// it, so the only real decision is WHICH TRANSPORT we open — and that decision +/// is a pure function of what is installed. This module is that function; +/// `codexchannel.ts` opens what it returns, and the driver that speaks the +/// protocol is L4c. /// /// ## The two rungs, as they actually exist (codex-cli 0.147.0, measured) /// /// - **`daemon`** — a shared, long-lived app server. `codex app-server daemon /// start` brings it up (idempotent by its own definition: "start ... if it -/// is not already running"), and `codex app-server proxy --sock ` -/// pipes our stdio to its **Unix domain control socket**. A session on the -/// daemon outlives our app and is shared with the vendor's own TUI. -/// - **`spawn`** — `codex app-server`, one child per session, dying with us. +/// is not already running") and it listens as a **WebSocket server bound to +/// a Unix domain socket** — its own argv is `codex app-server --listen +/// unix://`. A session on the daemon outlives our app and is shared with +/// the vendor's own TUI. +/// - **`spawn`** — `codex app-server`, one child per session, speaking +/// line-delimited JSON-RPC on stdio and dying with us. /// -/// ## Three corrections to the plan's L4 line, all measured +/// ## The daemon rung is a socket, NOT an argv — and that is L4a's correction /// -/// 1. **It is not a WebSocket and there is no bearer scheme.** The vendor's -/// shared-daemon transport is a Unix domain socket reached through a stdio -/// proxy. `--code-mode-host ` exists but is a different feature -/// (where *code mode* runs), not the session transport. Nothing here needs -/// a WebSocket client, which is why this module hands back argv instead. -/// 2. **We cannot "spawn it detached if absent".** `daemon start` refuses +/// L4a shipped this rung as `codex app-server proxy --sock `, on the +/// strength of that subcommand's summary ("Proxy stdio bytes to the running +/// app-server control socket"). **It does not work, and it fails silently.** +/// Measured by sitting a logging relay between the CLI and the daemon: +/// +/// - `daemon version` — the vendor's own client for this socket — opens with +/// `GET / HTTP/1.1` + `Upgrade: websocket` + `Sec-WebSocket-Version: 13`, +/// gets `101 Switching Protocols`, and only then speaks JSON-RPC inside +/// WebSocket frames. +/// - `app-server proxy --sock` sends **our stdin bytes verbatim**, with no +/// upgrade. The daemon closes the connection on the first byte of anything +/// that is not an HTTP upgrade — valid JSON-RPC and pure garbage are closed +/// identically, so this is not a parse error. The proxy then exits **0 with +/// no stdout and no stderr**, which reads exactly like "the agent had +/// nothing to say". +/// +/// So the plan's original L4 line was **right that this is a WebSocket** and +/// L4a's "it is not a WebSocket" correction was wrong; what the plan got wrong +/// was the *authentication* ("its bearer scheme") and the *address family*. +/// There is no token: the socket is created `srw-------` and **filesystem +/// permissions are the auth**. `--listen` does accept `ws://IP:PORT` for a TCP +/// WebSocket, but the daemon does not use it, and nothing here should. +/// +/// The type below encodes that: a `daemon` plan carries a `socketPath` and +/// **cannot** carry an argv. The shipped bug is now unrepresentable. +/// +/// ## Two further measured constraints, unchanged from L4a +/// +/// 1. **We cannot "spawn it detached if absent".** `daemon start` refuses /// unless codex was installed by the official installer script — it wants /// the managed binary at `/packages/standalone/current/codex` /// and says so: @@ -29,16 +55,16 @@ /// the standalone install managed by the Codex installer, because the /// daemon starts and updates app-server from that fixed path."* /// An npm / homebrew / distro codex therefore has **no daemon rung at -/// all**. -/// 3. **So the rung order inverts.** The plan called stdio spawn "the fallback -/// rung only"; for the common install it is the ONLY rung. `daemon` is an -/// opportunistic upgrade we take when the managed install is present. -/// -/// Also measured: the control socket path is subject to the platform's -/// `SUN_LEN` cap (~108 bytes). A relocated `CODEX_HOME` nested deeply enough -/// makes the daemon unreachable with *"path must be shorter than SUN_LEN"* — -/// so a long path disqualifies the daemon rung rather than failing later, at -/// connect time, with a message no one would attribute to path length. +/// all**, so per-session stdio is not "the fallback rung only" — for the +/// common install it is the only rung, and `daemon` is an opportunistic +/// upgrade. +/// 2. **`SUN_LEN`.** The control socket path is subject to the platform's cap +/// (~104–108 bytes). A deeply relocated `CODEX_HOME` fails at CONNECT time +/// with *"path must be shorter than SUN_LEN"* — a message no one would +/// attribute to path length — so a long path disqualifies the daemon rung +/// here, where we can say why. +import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; /// Conservative bound on a Unix domain socket path. The real cap is @@ -48,25 +74,36 @@ export const MAX_UNIX_SOCKET_PATH = 104; export type CodexAttachMode = 'daemon' | 'spawn'; -export interface CodexAttachPlan { - mode: CodexAttachMode; - /// Argv for the process whose stdio carries the app-server protocol. - argv: string[]; - /// The command that must succeed FIRST for `mode: 'daemon'` — bringing the - /// shared daemon up. Absent for `spawn`, which needs no preparation. - startArgv?: string[]; - /// Why this rung, in a sentence, for the session record and the UI. A user - /// whose session is not shared with their TUI should be able to find out why - /// without reading code. - reason: string; -} +/// Where the app-server protocol will flow. A discriminated union because the +/// two rungs are not the same kind of thing: one is a socket we open, the other +/// is a process we spawn. L4a modelled both as `argv` and shipped a daemon rung +/// that could never carry a byte. +export type CodexAttachPlan = + | { + mode: 'daemon'; + /// The Unix domain socket to open a **WebSocket** on. Not an argv: no + /// child process is involved in the data path at all. + socketPath: string; + /// Must succeed FIRST — brings the shared daemon up if it is not already + /// running. This one IS an argv. + startArgv: string[]; + reason: string; + } + | { + mode: 'spawn'; + /// Argv for the child whose stdio carries the protocol. + argv: string[]; + reason: string; + }; /// The daemon's control socket, given a codex home. export function controlSocketPath(codexHome: string): string { return path.join(codexHome, 'app-server-control', 'app-server-control.sock'); } -/// The managed standalone binary `daemon start` insists on. +/// The managed standalone binary `daemon start` insists on. (It is a symlink to +/// `bin/codex` in the same tree, so it is both the path the daemon reports and +/// a runnable binary.) export function managedCodexPath(codexHome: string): string { return path.join(codexHome, 'packages', 'standalone', 'current', 'codex'); } @@ -79,10 +116,70 @@ export function codexHome(home: string, env: NodeJS.ProcessEnv = process.env): s return dir !== undefined && dir !== '' ? dir : path.join(home, '.codex'); } +/// Directories a codex install lands in that a GUI-launched app will not have +/// on its PATH. The official installer writes `~/.local/bin/codex` and appends +/// its PATH line to `.bashrc` — which an Electron app started from a Dock or +/// Start-menu icon never sources, so the standalone install is *less* +/// discoverable than a distro package was. Same class of problem `kimiweb.ts` +/// solves for kimi. +export function codexBinDirs(home: string, env: NodeJS.ProcessEnv = process.env): string[] { + const dirs = [ + path.join(home, '.local', 'bin'), + path.join(codexHome(home, env), 'packages', 'standalone', 'current', 'bin'), + ]; + if (process.platform === 'win32') { + dirs.push(path.join(env['APPDATA'] ?? path.join(home, 'AppData', 'Roaming'), 'npm')); + } + return dirs; +} + +/// The launcher filenames a codex install creates, in runnable order. `.ps1` is +/// excluded for the reason kimiweb.ts documents: `cmd /C x.ps1` opens it, it +/// does not run it. +const WIN_CODEX_NAMES = ['codex.cmd', 'codex.exe', 'codex.bat']; + +/// Resolve codex to an ABSOLUTE path by scanning a PATH value plus the +/// well-known dirs. Pure (the existence check is injected) and exported so the +/// discoverability rule is testable without an install. Returns null when +/// nothing matches — callers may still fall back to the bare name and let the +/// OS try. +export function findCodexOnPath( + pathValue: string | undefined, + extraDirs: string[] = [], + exists: (p: string) => boolean = fs.existsSync, +): string | null { + const names = process.platform === 'win32' ? WIN_CODEX_NAMES : ['codex']; + const dirs = [...(pathValue ?? '').split(path.delimiter), ...extraDirs]; + for (const raw of dirs) { + const dir = raw.trim(); + if (dir === '') continue; + for (const name of names) { + const p = path.join(dir, name); + if (exists(p)) return p; + } + } + return null; +} + +/// The codex binary to run: an explicit override, else a real file found on +/// PATH or in a well-known dir, else the bare name. +export function codexBinary( + home: string = os.homedir(), + env: NodeJS.ProcessEnv = process.env, + exists: (p: string) => boolean = fs.existsSync, +): string { + const explicit = env['TERMIPOD_CODEX_BIN']; + if (explicit !== undefined && explicit !== '') return explicit; + return findCodexOnPath(env['PATH'], codexBinDirs(home, env), exists) ?? 'codex'; +} + export interface CodexAttachProbe { /// Does `/packages/standalone/current/codex` exist? The caller /// supplies this (an `fs.existsSync`) so the decision itself stays pure. managedInstallPresent: boolean; + /// Resolved codex binary. Defaults to the bare name so the decision table can + /// be tested with no install on the box. + bin?: string; } /// Choose the rung. Pure: every input is a value, so the whole decision table @@ -92,7 +189,7 @@ export function planCodexAttach( probe: CodexAttachProbe, env: NodeJS.ProcessEnv = process.env, ): CodexAttachPlan { - const codexBin = env['TERMIPOD_CODEX_BIN'] ?? 'codex'; + const codexBin = probe.bin ?? env['TERMIPOD_CODEX_BIN'] ?? 'codex'; const chome = codexHome(home, env); const sock = controlSocketPath(chome); if (!probe.managedInstallPresent) { @@ -114,7 +211,7 @@ export function planCodexAttach( } return { mode: 'daemon', - argv: [codexBin, 'app-server', 'proxy', '--sock', sock], + socketPath: sock, startArgv: [codexBin, 'app-server', 'daemon', 'start'], reason: 'attached to the shared codex app-server daemon; this session survives the app and is visible to the codex TUI', }; diff --git a/desktop/electron/src/localagent/codexchannel.test.ts b/desktop/electron/src/localagent/codexchannel.test.ts new file mode 100644 index 00000000..4697e5f3 --- /dev/null +++ b/desktop/electron/src/localagent/codexchannel.test.ts @@ -0,0 +1,267 @@ +/// L4b channel checks — the decoders are pure, and both transports are opened +/// against injected fakes so the contract is provable with no codex on the box. +/// The live round trip is `codexattach.e2e.test.ts`, which is the only place +/// that can prove the handshake actually works. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { EventEmitter } from 'node:events'; +import { PassThrough } from 'node:stream'; + +import { + CODEX_WS_OPTIONS, + codexSocketUrl, + decodeMessage, + LineBuffer, + openCodexChannel, + type ChannelDeps, + type CodexChannelHandlers, + type CodexFrame, + type WsLike, +} from './codexchannel.ts'; +import type { CodexAttachPlan } from './codexattach.ts'; + +test('decodeMessage parses one unit and keeps unparseable output as junk', () => { + const r = decodeMessage('{"a":1}\n\n \nnot json\n{"b":2}'); + assert.deepEqual(r.frames, [{ a: 1 }, { b: 2 }]); + // Blank lines are framing, not junk; a non-JSON line is junk and must not be + // silently dropped — swallowing engine output is how E3 shipped invisible. + assert.deepEqual(r.junk, ['not json']); +}); + +test('decodeMessage rejects non-object JSON', () => { + // A bare scalar or array is valid JSON but not a JSON-RPC frame; treating it + // as one would hand the driver something with no `method` and no `id`. + const r = decodeMessage('42\n["a"]\n"text"\nnull'); + assert.deepEqual(r.frames, []); + assert.equal(r.junk.length, 4); +}); + +test('LineBuffer reassembles a frame split across chunk boundaries', () => { + const b = new LineBuffer(); + assert.deepEqual(b.push('{"me').frames, [], 'a partial line yields nothing yet'); + assert.deepEqual(b.push('thod":"x"}').frames, [], 'still no newline, still nothing'); + assert.deepEqual(b.push('\n').frames, [{ method: 'x' }]); +}); + +test('LineBuffer holds the tail and flush() releases a final unterminated frame', () => { + const b = new LineBuffer(); + const first = b.push('{"a":1}\n{"b":2}'); + assert.deepEqual(first.frames, [{ a: 1 }], 'only the completed line is emitted'); + // A process that exits without a trailing newline still said something. + assert.deepEqual(b.flush().frames, [{ b: 2 }]); + assert.deepEqual(b.flush().frames, [], 'flush is idempotent'); +}); + +test('LineBuffer handles several frames in one chunk, in order', () => { + const b = new LineBuffer(); + assert.deepEqual(b.push('{"n":1}\n{"n":2}\n{"n":3}\n').frames, [{ n: 1 }, { n: 2 }, { n: 3 }]); +}); + +test('the socket URL is the ws+unix form, and deflate is OFF', () => { + assert.equal(codexSocketUrl('/home/u/.codex/x.sock'), 'ws+unix:///home/u/.codex/x.sock:/'); + // Measured against the live daemon: `ws` offers permessage-deflate by + // default and the daemon HANGS UP on that handshake instead of declining the + // extension. Isolated by varying one option at a time. This is not a + // preference; flipping it breaks the daemon rung entirely. + assert.equal(CODEX_WS_OPTIONS.perMessageDeflate, false); +}); + +class FakeWs extends EventEmitter implements WsLike { + sent: string[] = []; + closed = false; + send(data: string): void { + this.sent.push(data); + } + close(): void { + this.closed = true; + } +} + +class FakeChild extends EventEmitter { + stdout = new PassThrough(); + stderr = new PassThrough(); + stdin = new PassThrough(); + killed = false; + kill(): void { + this.killed = true; + } +} + +function collector(): { handlers: CodexChannelHandlers; frames: CodexFrame[]; junk: string[]; closes: string[] } { + const frames: CodexFrame[] = []; + const junk: string[] = []; + const closes: string[] = []; + return { + frames, + junk, + closes, + handlers: { + onFrame: (f) => frames.push(f), + onJunk: (j) => junk.push(j), + onClose: (i) => closes.push(i.reason), + }, + }; +} + +const DAEMON_PLAN: CodexAttachPlan = { + mode: 'daemon', + socketPath: '/tmp/x.sock', + startArgv: ['codex', 'app-server', 'daemon', 'start'], + reason: 'attached to the shared codex app-server daemon; it outlives this app', +}; + +const SPAWN_PLAN: CodexAttachPlan = { + mode: 'spawn', + argv: ['codex', 'app-server'], + reason: 'no installer-managed codex, so running a per-session app server', +}; + +function deps(over: Partial = {}): { deps: ChannelDeps; ws: FakeWs; child: FakeChild; started: string[][] } { + const ws = new FakeWs(); + const child = new FakeChild(); + const started: string[][] = []; + return { + ws, + child, + started, + deps: { + connect: async () => ws, + spawn: () => child as never, + startDaemon: async (argv) => { + started.push(argv); + }, + ...over, + }, + }; +} + +test('daemon rung: starts the daemon, opens the socket, relays frames both ways', async () => { + const c = collector(); + const d = deps(); + const open = openCodexChannel(DAEMON_PLAN, c.handlers, { cwd: '/w' }, d.deps); + setImmediate(() => d.ws.emit('open')); + const ch = await open; + + assert.equal(ch.mode, 'daemon'); + assert.deepEqual(d.started, [['codex', 'app-server', 'daemon', 'start']], 'the daemon must be brought up first'); + + ch.send({ jsonrpc: '2.0', id: 1, method: 'initialize' }); + assert.deepEqual(JSON.parse(d.ws.sent[0] ?? ''), { jsonrpc: '2.0', id: 1, method: 'initialize' }); + + // One WebSocket message is one complete unit — no reassembly, and a message + // with no trailing newline must still be delivered. + d.ws.emit('message', '{"id":1,"result":{"codexHome":"/h"}}'); + assert.deepEqual(c.frames, [{ id: 1, result: { codexHome: '/h' } }]); +}); + +test('daemon rung: closing our channel must NOT stop the shared daemon', async () => { + const c = collector(); + const d = deps(); + const open = openCodexChannel(DAEMON_PLAN, c.handlers, { cwd: '/w' }, d.deps); + setImmediate(() => d.ws.emit('open')); + const ch = await open; + ch.close(); + assert.ok(d.ws.closed, 'our socket closes'); + // The rung's whole purpose is that the session outlives us. Nothing in this + // path may issue `daemon stop`. + assert.deepEqual(d.started, [['codex', 'app-server', 'daemon', 'start']]); +}); + +test('daemon rung: a daemon that will not start falls back to spawn, and SAYS so', async () => { + const c = collector(); + const d = deps({ + startDaemon: async () => { + throw new Error('managed standalone Codex install not found'); + }, + }); + const ch = await openCodexChannel(DAEMON_PLAN, c.handlers, { cwd: '/w' }, d.deps); + assert.equal(ch.mode, 'spawn', 'we still get a working session'); + // "shared with your TUI" and "dies with this window" are different promises. + // A silent downgrade would leave the user believing the wrong one. + assert.match(ch.reason, /could not be reached/); + assert.match(ch.reason, /managed standalone Codex install not found/); + assert.match(ch.reason, /will not outlive/); +}); + +test('daemon rung: a socket that never finishes the handshake times out, not hangs', async () => { + const c = collector(); + const d = deps(); + // No 'open' is ever emitted — the failure mode a silent daemon produces. + const ch = await openCodexChannel(DAEMON_PLAN, c.handlers, { cwd: '/w', connectTimeoutMs: 20 }, d.deps); + assert.equal(ch.mode, 'spawn'); + assert.match(ch.reason, /timed out/); +}); + +test('daemon rung: a close after open is reported as a close, not a failed open', async () => { + const c = collector(); + const d = deps(); + const open = openCodexChannel(DAEMON_PLAN, c.handlers, { cwd: '/w' }, d.deps); + setImmediate(() => d.ws.emit('open')); + await open; + d.ws.emit('close', 1006); + assert.equal(c.closes.length, 1); + assert.match(c.closes[0] ?? '', /1006/); +}); + +test('daemon rung: a socket that fails reports close exactly once', async () => { + const c = collector(); + const d = deps(); + const open = openCodexChannel(DAEMON_PLAN, c.handlers, { cwd: '/w' }, d.deps); + setImmediate(() => d.ws.emit('open')); + await open; + // `ws` emits BOTH on a broken connection. A driver told twice that its + // channel ended would tear the same session down twice. + d.ws.emit('error', new Error('ECONNRESET')); + d.ws.emit('close', 1006); + assert.equal(c.closes.length, 1, 'error+close is one ending, not two'); + assert.match(c.closes[0] ?? '', /ECONNRESET/, 'the first, more specific reason wins'); +}); + +test('spawn rung: stdout is a byte stream, so frames reassemble across chunks', async () => { + const c = collector(); + const d = deps(); + const ch = await openCodexChannel(SPAWN_PLAN, c.handlers, { cwd: '/w' }, d.deps); + assert.equal(ch.mode, 'spawn'); + + d.child.stdout.write('{"method":"thread/'); + d.child.stdout.write('started","params":{}}\n'); + await new Promise((r) => setImmediate(r)); + assert.deepEqual(c.frames, [{ method: 'thread/started', params: {} }]); +}); + +test('spawn rung: send writes a newline-delimited frame; close ends stdin', async () => { + const c = collector(); + const d = deps(); + const ch = await openCodexChannel(SPAWN_PLAN, c.handlers, { cwd: '/w' }, d.deps); + const written: string[] = []; + d.child.stdin.on('data', (b: Buffer) => written.push(b.toString('utf8'))); + + ch.send({ id: 1, method: 'initialize' }); + await new Promise((r) => setImmediate(r)); + assert.equal(written.join(''), '{"id":1,"method":"initialize"}\n', 'the trailing newline IS the framing'); + + let stdinEnded = false; + d.child.stdin.on('finish', () => { + stdinEnded = true; + }); + ch.close(); + await new Promise((r) => setImmediate(r)); + // Ending stdin is what retires an app-server session — the same contract + // claudechild.ts documents. The kill is only the backstop, so asserting the + // kill alone would pass on a close() that never let the engine finish. + assert.ok(stdinEnded, 'close() must end stdin, not just kill'); + assert.ok(d.child.killed); +}); + +test('spawn rung: an exit flushes the unterminated tail before reporting close', async () => { + const c = collector(); + const d = deps(); + await openCodexChannel(SPAWN_PLAN, c.handlers, { cwd: '/w' }, d.deps); + d.child.stdout.write('{"method":"last"}'); + await new Promise((r) => setImmediate(r)); + assert.deepEqual(c.frames, [], 'not yet — no newline'); + d.child.emit('exit', 0); + // A final frame with no trailing newline is still something the engine said. + assert.deepEqual(c.frames, [{ method: 'last' }]); + assert.equal(c.closes.length, 1); +}); diff --git a/desktop/electron/src/localagent/codexchannel.ts b/desktop/electron/src/localagent/codexchannel.ts new file mode 100644 index 00000000..f0e671d7 --- /dev/null +++ b/desktop/electron/src/localagent/codexchannel.ts @@ -0,0 +1,374 @@ +/// The byte path to a codex app-server (vision-parity **L4b**). +/// +/// `codexattach.ts` decides WHICH rung; this module opens it and presents both +/// as one thing: a bidirectional stream of JSON-RPC frames. The driver above +/// (L4c) should not be able to tell whether it is talking to a child process or +/// to a shared daemon — that is the whole point of the abstraction, and it is +/// why `send`/`onFrame` are the entire surface. +/// +/// ## Two transports, one contract +/// +/// - **spawn** — `codex app-server` as a child. Its stdout is a *byte stream* +/// of newline-delimited JSON, so frames must be reassembled across chunk +/// boundaries (`LineBuffer`). +/// - **daemon** — a **WebSocket over a Unix domain socket**. WebSocket is a +/// message protocol, so each message arrives whole and no reassembly is +/// needed; a message may still contain more than one newline-separated +/// object, so it goes through the same decoder minus the buffering. +/// +/// ## `perMessageDeflate: false` is load-bearing +/// +/// Measured against codex-cli 0.147.0: `ws` offers `permessage-deflate` by +/// default, and the daemon **hangs up on the handshake** rather than declining +/// the extension — `socket hang up`, before `open`. Isolated against the live +/// daemon by varying one option at a time: deflate on fails with or without a +/// `Host` header, deflate off succeeds with or without it. So the option below +/// is the fix, and the `Host` header is not part of it. +/// +/// ## Closing a daemon channel must not stop the daemon +/// +/// The daemon rung exists so a session outlives us. `close()` therefore closes +/// **our socket** and nothing else; only an explicit `codex app-server daemon +/// stop` should end the shared server. The spawn rung is the opposite by +/// nature: its child dies with us, which is what `close()` does there. +import { spawn as nodeSpawn } from 'node:child_process'; +import type { CodexAttachMode, CodexAttachPlan } from './codexattach.ts'; + +/// A parsed JSON-RPC frame. Deliberately untyped beyond "an object" — the +/// app-server vocabulary is large and versioned, and the frame profile is what +/// gives it meaning (see `frameprofile/`). Typing it here would be a second, +/// staler copy of the vendor's schema. +export type CodexFrame = Record; + +/// What one decode pass produced. `junk` is carried rather than dropped: a line +/// we cannot parse is a fact about the engine, and silently swallowing engine +/// output is precisely how E3 shipped a feature nobody could see. +export interface DecodeResult { + frames: CodexFrame[]; + junk: string[]; +} + +/// Decode a COMPLETE unit of text (one WebSocket message, or one full line from +/// a stream) into frames. Pure. Empty and whitespace-only pieces are not junk — +/// they are just framing. +export function decodeMessage(text: string): DecodeResult { + const frames: CodexFrame[] = []; + const junk: string[] = []; + for (const piece of text.split('\n')) { + const trimmed = piece.trim(); + if (trimmed === '') continue; + let parsed: unknown; + try { + parsed = JSON.parse(trimmed); + } catch { + junk.push(trimmed); + continue; + } + if (parsed !== null && typeof parsed === 'object' && !Array.isArray(parsed)) { + frames.push(parsed as CodexFrame); + } else { + junk.push(trimmed); + } + } + return { frames, junk }; +} + +/// Reassemble newline-delimited JSON from a byte STREAM. A chunk may split a +/// frame anywhere, so the tail is held until its newline arrives. Pure, and +/// separate from `decodeMessage` because the WebSocket rung must NOT buffer: a +/// message with no trailing newline is complete, and holding it would stall the +/// channel forever. +export class LineBuffer { + private tail = ''; + + push(chunk: string): DecodeResult { + const combined = this.tail + chunk; + const lastBreak = combined.lastIndexOf('\n'); + if (lastBreak < 0) { + this.tail = combined; + return { frames: [], junk: [] }; + } + this.tail = combined.slice(lastBreak + 1); + return decodeMessage(combined.slice(0, lastBreak)); + } + + /// Whatever is left when the stream ends — a final frame with no trailing + /// newline is still a frame. + flush(): DecodeResult { + const rest = this.tail; + this.tail = ''; + return decodeMessage(rest); + } +} + +/// The `ws` URL for a Unix domain socket. `ws+unix://:` +/// is the form `ws` understands; the daemon serves the upgrade at `/`. +export function codexSocketUrl(socketPath: string): string { + return `ws+unix://${socketPath}:/`; +} + +/// Handshake options for the daemon rung. See the module note: the daemon hangs +/// up on a handshake that offers `permessage-deflate`. +export const CODEX_WS_OPTIONS = { perMessageDeflate: false } as const; + +export interface CodexChannel { + /// Which rung actually opened — not necessarily the one first planned, since + /// a daemon that will not come up falls back to a spawned child. + readonly mode: CodexAttachMode; + /// A sentence naming the rung, for the session record and the UI. + readonly reason: string; + send(frame: CodexFrame): void; + close(): void; +} + +export interface CodexChannelHandlers { + onFrame(frame: CodexFrame): void; + /// The channel ended. `reason` is always a sentence; `code` is the WebSocket + /// close code or the child's exit code when there is one. + onClose(info: { code: number | null; reason: string }): void; + /// Output that was not a JSON object. Optional, but offered so that engine + /// chatter has somewhere to go other than the floor. + onJunk?(line: string): void; +} + +/// The `ws` surface this module uses, structurally, so a test can supply a fake +/// without the library or a socket. +export interface WsLike { + on(event: 'open', cb: () => void): void; + on(event: 'message', cb: (data: unknown) => void): void; + on(event: 'error', cb: (err: Error) => void): void; + on(event: 'close', cb: (code: number, reason: unknown) => void): void; + send(data: string): void; + close(): void; +} + +type WsCtor = new (url: string, options: unknown) => WsLike; + +export interface SpawnedProcess { + stdout: NodeJS.ReadableStream; + stderr: NodeJS.ReadableStream | null; + stdin: NodeJS.WritableStream; + kill(signal?: NodeJS.Signals): void; + on(event: 'exit', cb: (code: number | null) => void): void; +} + +export interface ChannelDeps { + /// Open a WebSocket. Injected so the daemon rung is testable with no daemon. + /// Async because the real one loads `ws` lazily — see `defaultDeps`. + connect(url: string, options: typeof CODEX_WS_OPTIONS): Promise; + /// Spawn the app-server child. + spawn(bin: string, args: string[], opts: { cwd: string; env: NodeJS.ProcessEnv }): SpawnedProcess; + /// Bring the shared daemon up. Resolves when it is running, rejects with a + /// usable message when it will not start. + startDaemon(argv: string[], opts: { env: NodeJS.ProcessEnv }): Promise; +} + +export interface OpenOptions { + cwd: string; + env?: NodeJS.ProcessEnv; + /// How long to wait for the WebSocket to open before giving up on the daemon + /// rung. A daemon that accepts the connection but never completes the upgrade + /// would otherwise hang the session with no explanation. + connectTimeoutMs?: number; +} + +const DEFAULT_CONNECT_TIMEOUT_MS = 10_000; + +/// Open the planned rung, falling back to a spawned child when the shared +/// daemon cannot be reached. +/// +/// The fallback is **reported, never silent**: the returned channel's `mode` +/// and `reason` say which rung is live and why, because "your session is shared +/// with the codex TUI" and "your session dies with this window" are different +/// promises and the user is entitled to know which one they got. +export async function openCodexChannel( + plan: CodexAttachPlan, + handlers: CodexChannelHandlers, + opts: OpenOptions, + deps: ChannelDeps = defaultDeps(), +): Promise { + const env = opts.env ?? process.env; + if (plan.mode === 'daemon') { + try { + await deps.startDaemon(plan.startArgv, { env }); + return await openDaemonChannel(plan.socketPath, plan.reason, handlers, opts, deps); + } catch (err) { + const why = err instanceof Error ? err.message : String(err); + const fallback: CodexAttachPlan = { + mode: 'spawn', + argv: [plan.startArgv[0] ?? 'codex', 'app-server'], + reason: `the shared codex daemon could not be reached (${why}); running a per-session app server instead, so this session will not outlive the app`, + }; + return openSpawnChannel(fallback, handlers, opts, deps); + } + } + return openSpawnChannel(plan, handlers, opts, deps); +} + +async function openDaemonChannel( + socketPath: string, + reason: string, + handlers: CodexChannelHandlers, + opts: OpenOptions, + deps: ChannelDeps, +): Promise { + const ws = await deps.connect(codexSocketUrl(socketPath), CODEX_WS_OPTIONS); + return new Promise((resolve, reject) => { + let opened = false; + let settled = false; + // A socket that fails after opening emits BOTH `error` and `close`, and a + // driver told twice that its channel ended would tear the session down + // twice. First one wins. + let closeReported = false; + const reportClose = (code: number | null, why: string): void => { + if (!opened || closeReported) return; + closeReported = true; + handlers.onClose({ code, reason: why }); + }; + const timer = setTimeout(() => { + if (settled) return; + settled = true; + try { + ws.close(); + } catch { + /* the socket is already gone; nothing to unwind */ + } + reject(new Error(`timed out opening the app-server socket at ${socketPath}`)); + }, opts.connectTimeoutMs ?? DEFAULT_CONNECT_TIMEOUT_MS); + + ws.on('open', () => { + opened = true; + if (settled) return; + settled = true; + clearTimeout(timer); + resolve({ + mode: 'daemon', + reason, + send(frame) { + ws.send(JSON.stringify(frame)); + }, + close() { + // Our socket only — the daemon keeps running, which is the point. + ws.close(); + }, + }); + }); + + ws.on('message', (data) => { + const { frames, junk } = decodeMessage(textOf(data)); + for (const f of frames) handlers.onFrame(f); + for (const j of junk) handlers.onJunk?.(j); + }); + + ws.on('error', (err) => { + if (!settled) { + settled = true; + clearTimeout(timer); + reject(err); + return; + } + reportClose(null, err.message); + }); + + ws.on('close', (code) => { + clearTimeout(timer); + if (!settled) { + settled = true; + reject(new Error(`app-server socket closed during handshake (code ${String(code)})`)); + return; + } + reportClose(code, `app-server socket closed (code ${String(code)})`); + }); + }); +} + +function openSpawnChannel( + plan: Extract, + handlers: CodexChannelHandlers, + opts: OpenOptions, + deps: ChannelDeps, +): CodexChannel { + const [bin, ...args] = plan.argv; + const child = deps.spawn(bin ?? 'codex', args, { cwd: opts.cwd, env: opts.env ?? process.env }); + const buffer = new LineBuffer(); + + child.stdout.setEncoding?.('utf8'); + child.stdout.on('data', (chunk: string | Buffer) => { + const { frames, junk } = buffer.push(typeof chunk === 'string' ? chunk : chunk.toString('utf8')); + for (const f of frames) handlers.onFrame(f); + for (const j of junk) handlers.onJunk?.(j); + }); + + child.on('exit', (code) => { + const { frames, junk } = buffer.flush(); + for (const f of frames) handlers.onFrame(f); + for (const j of junk) handlers.onJunk?.(j); + handlers.onClose({ code, reason: `codex app-server exited (code ${String(code)})` }); + }); + + return { + mode: 'spawn', + reason: plan.reason, + send(frame) { + child.stdin.write(JSON.stringify(frame) + '\n'); + }, + close() { + // Ending stdin is what retires a session; the kill is the backstop. + try { + child.stdin.end(); + } catch { + /* already closed */ + } + child.kill(); + }, + }; +} + +function textOf(data: unknown): string { + if (typeof data === 'string') return data; + if (data instanceof Uint8Array) return Buffer.from(data).toString('utf8'); + if (Array.isArray(data)) return data.map((d) => textOf(d)).join(''); + return String(data); +} + +function defaultDeps(): ChannelDeps { + return { + async connect(url, options) { + // Loaded lazily: `ws` is only needed for the daemon rung, and the common + // install has no daemon rung at all. `ws` is external in esbuild.mjs, so + // this stays a REAL dynamic import and the result is node's + // cjs-module-lexer namespace rather than the raw `module.exports` — the + // interop ssh2mod.ts documents at length. `ws` exports the class both as + // `module.exports` and as `.WebSocket`, so normalize across all three + // shapes rather than betting on one. + const m = (await import('ws')) as unknown as { + WebSocket?: WsCtor; + default?: WsCtor & { WebSocket?: WsCtor }; + }; + const ctor = m.WebSocket ?? m.default?.WebSocket ?? m.default; + if (typeof ctor !== 'function') { + throw new Error('the `ws` module did not export a WebSocket constructor'); + } + return new ctor(url, options); + }, + spawn(bin, args, o) { + return nodeSpawn(bin, args, { cwd: o.cwd, env: o.env, stdio: ['pipe', 'pipe', 'pipe'] }) as SpawnedProcess; + }, + startDaemon(argv, o) { + return new Promise((resolve, reject) => { + const [bin, ...rest] = argv; + const p = nodeSpawn(bin ?? 'codex', rest, { env: o.env, stdio: ['ignore', 'pipe', 'pipe'] }); + let err = ''; + p.stderr?.setEncoding('utf8'); + p.stderr?.on('data', (d: string) => { + err += d; + }); + p.on('error', reject); + p.on('exit', (code) => { + if (code === 0) resolve(); + else reject(new Error(err.trim() || `\`codex app-server daemon start\` exited ${String(code)}`)); + }); + }); + }, + }; +} diff --git a/docs/changelog-desktop.md b/docs/changelog-desktop.md index c6ee7007..be00b8de 100644 --- a/docs/changelog-desktop.md +++ b/docs/changelog-desktop.md @@ -133,6 +133,23 @@ This complements: record, because the hub stores externalized bytes as `application/octet-stream` and no browser will paint that as an image. +- **Groundwork for driving codex locally: the transport to its app-server.** + A codex installed by the official installer script runs a shared background + app-server, and a session on it outlives this app and is visible in codex's + own TUI. The desktop can now open that channel, or spawn a private + per-session app-server when the shared one is unavailable — falling back + with a sentence saying which it got, because "shared with your TUI" and + "dies with this window" are different promises. Not yet wired to any + surface: the driver that speaks the protocol is the next wedge, so nothing + visible changes yet. (vision-parity L4b) + + **codex installed by the official script is now found even when it is not on + PATH.** The installer writes `~/.local/bin/codex` and puts its PATH line in + `.bashrc`, which an app launched from a Dock or Start-menu icon never reads + — so the officially-installed codex was *less* discoverable than a + distro-packaged one. The well-known install dirs are searched directly, with + `TERMIPOD_CODEX_BIN` as an override. + ### Changed - **The UI-sharing consent text now lists all eight gated tools.** It had said "four things" since the first Author lane and had never been updated for diff --git a/docs/plans/desktop-companion-vision-parity.md b/docs/plans/desktop-companion-vision-parity.md index 73403fb6..a5fbbe70 100644 --- a/docs/plans/desktop-companion-vision-parity.md +++ b/docs/plans/desktop-companion-vision-parity.md @@ -10,9 +10,12 @@ > 2026-08-16** (streaming command output; relay result passthrough). > **R4 shipped 2026-08-16** (live output + agent-produced media) — > **W3 complete**; L3c is deferrable. **W4 started 2026-08-16**: -> **F4 shipped** (user-level MCP reseed for claude + codex) and -> **L4a shipped** (codex attach-rung resolver; L4's WebSocket premise -> was wrong — see the wedge). Left in W4: L4b, R5, R6 +> **F4 shipped** (user-level MCP reseed for claude + codex), +> **L4a shipped** (codex attach-rung resolver) and **L4b shipped** +> (codex app-server transport — which corrected L4a: the daemon's +> control socket *is* a WebSocket, and L4a's `app-server proxy` data +> path was a dead channel). L4 split again: **L4c** is the driver. +> Left in W4: L4c, R5, R6 > **Audience:** principal · contributors · maintainers > **Last verified vs code:** 2026.730.1231-alpha (`cea267fa`) — every > anchor below re-verified against that tip by the authoring audit @@ -458,26 +461,21 @@ transport rung (lane T), never the renderer's ceiling. argv from the same N1 table L3 reads. **L4a shipped 2026-08-16** — the attach-rung resolver - (`localagent/codexattach.ts`). **This line originally specified a - WebSocket attach with a bearer scheme; no such interface exists.** - Measured against codex-cli **0.147.0** (the installed CLI had also - moved on from the 0.133.0 that E3 was written against): + (`localagent/codexattach.ts`). Measured against codex-cli **0.147.0** + (the installed CLI had also moved on from the 0.133.0 that E3 was + written against), three corrections to this line, all of which stand: - 1. **The shared-daemon transport is a Unix domain socket, not a - WebSocket, and there is no bearer scheme.** `codex app-server - daemon start` brings the daemon up and `codex app-server proxy - --sock ` pipes stdio to its control socket at - `/app-server-control/app-server-control.sock`. The + 1. **There is no bearer scheme.** `daemon start` brings the daemon up + and the control socket lives at + `/app-server-control/app-server-control.sock`, created + `srw-------` — **filesystem permissions are the auth**. The `--code-mode-host ` flag is a different feature (where - *code mode* runs), not the session transport. So the attach rung - needs **no WebSocket client and no token** — it is the same - JSON-RPC-over-stdio we already speak, pointed at another process. + *code mode* runs), not the session transport. 2. **"Spawn it detached if absent" is not available.** `daemon start` refuses unless codex was installed by the official installer script, wanting the managed binary at `/packages/standalone/current/codex`. An npm / - homebrew / distro codex — including the one on this machine — - therefore has **no daemon rung at all**. + homebrew / distro codex has **no daemon rung at all**. 3. **The rung order inverts.** Per-session stdio is not "the fallback rung only"; for the common install it is the only rung. The daemon is an opportunistic upgrade taken when the managed install exists. @@ -488,40 +486,93 @@ transport rung (lane T), never the renderer's ceiling. SUN_LEN"* — a message nobody would attribute to path length — so the resolver disqualifies the daemon rung up front and says why. - **Confirmed against a live daemon (2026-08-16).** The dev machine was - moved to the installer-managed standalone codex, so the daemon rung — - previously unreachable here — could be exercised. `codex app-server - daemon start` reports both paths this resolver computes, **exactly**: - `managedCodexPath` = `/packages/standalone/current/codex` - and `socketPath` = - `/app-server-control/app-server-control.sock`. The socket - is created `srw-------` — **filesystem permissions are the auth**, - which is the positive form of "there is no bearer scheme". + **L4b shipped 2026-08-16** — the transport + (`localagent/codexchannel.ts`), and **a correction to L4a's own + correction**. - **Open for L4b — the proxy's data path is NOT yet proven.** `codex - app-server proxy --sock ` connects and exits 0 with no stdout - and no stderr for the same `initialize` handshake that works over a - plain `codex app-server` child (E3's probe shape), with stdin held - open. Two candidate causes, neither confirmed: the daemon may require - `codex app-server daemon enable-remote-control` (its help does not say - what that exposes, and the sibling `bootstrap` frames it as - SSH-driven — a security-posture question for the director, not a - default), or the control socket may expect a framing/handshake the - bare JSON-RPC line does not supply. **L4b must not assume the attach - rung works until a round trip is observed**; the resolver deliberately - decides *which argv*, not *that it succeeds*. + ★★ **L4a claimed "the shared-daemon transport is a Unix domain + socket, NOT a WebSocket". That was wrong, and it shipped a dead + channel.** The control socket is a **WebSocket server bound to a Unix + domain socket** — the daemon's own argv is `codex app-server --listen + unix://`, and `--listen` accepts `stdio:// | unix:// | unix://PATH | + ws://IP:PORT`. So the plan's original "WebSocket attach" was right; + what it got wrong was the *authentication* and the *address family*. - **Discoverability regression to handle in L4b.** The standalone codex - lives at `~/.local/bin/codex` with its PATH entry written into - `.bashrc` — which a GUI-launched Electron app never sources. It is - therefore *less* discoverable than a `/usr/bin/codex` package was. - That is what `TERMIPOD_CODEX_BIN` exists for, and L4b should resolve - the binary the way `kimiweb.ts` already does for kimi - (`findKimiOnPath` + `mergePathDirs`) rather than trusting `PATH`. + How the error survived L4a: the evidence was a `--help` tree, and + `app-server proxy`'s summary reads "Proxy stdio bytes to the running + app-server control socket", which is exactly what an attach rung + should say. L4a therefore shipped + `[bin, 'app-server', 'proxy', '--sock', sock]` as the daemon data + path. Measured with a logging relay placed between the CLI and its + own socket: - **L4b** (still to do) is the driver itself: the app-server JSON-RPC - client over whichever argv L4a returns, delta throttling, R1 approval - cards, `turn/interrupt`, config seeding. + - `daemon version` — the vendor's OWN client for this socket — opens + with `GET / HTTP/1.1` + `Upgrade: websocket` + + `Sec-WebSocket-Version: 13`, takes `101 Switching Protocols`, and + only then speaks JSON-RPC inside frames. + - `app-server proxy --sock` forwards our stdin **verbatim, with no + upgrade**. The daemon closes the connection on the first byte of + anything that is not an HTTP upgrade — valid JSON-RPC and pure + garbage are closed identically, so it is not a parse error — and + the proxy exits **0 with no stdout and no stderr**. A dead channel + that reads exactly like a quiet agent. + + Two candidate causes L4a recorded are now both **disproven**: it is + not a framing question (`Content-Length` and half-close fail the same + way), and it is not `enable-remote-control` — the live round trip + below succeeds with `remoteControl/status` reporting `disabled` + throughout, so that security-posture toggle was never needed and + remains off. + + *As built:* `CodexAttachPlan` became a **discriminated union** — a + `daemon` plan carries a `socketPath` and *cannot* carry an argv, so + the shipped bug is now unrepresentable rather than merely fixed. + `codexchannel.ts` opens either rung behind one `send`/`onFrame` + surface: the spawn rung reassembles newline-delimited JSON across + chunk boundaries (`LineBuffer`), while the WebSocket rung must *not* + buffer, because a message with no trailing newline is already + complete and holding it would stall the channel forever. + + ★ **`perMessageDeflate: false` is load-bearing.** `ws` offers + `permessage-deflate` by default and the daemon **hangs up on the + handshake** rather than declining the extension. Isolated by varying + one option at a time against the live daemon: deflate on fails with + or without a `Host` header, deflate off succeeds with or without it. + A unit test cannot see this at all — the e2e is what kills the + mutation, and it fails with the real symptom (`socket hang up`). + + *Two decisions worth naming.* A daemon that will not come up **falls + back to spawn, and says so in a sentence** — "shared with your codex + TUI" and "dies with this window" are different promises, and a silent + downgrade would leave the user holding the wrong one. And `close()` + on a daemon channel closes **our socket only**: the rung exists so + the session outlives us, so nothing in that path may issue `daemon + stop`. + + *Not yet reachable from the app.* Like L4a, this module is not + imported by `main.ts` — it is proven by an opt-in live e2e + (`TERMIPOD_CODEX_ATTACH_E2E=1`) against a real daemon, not by use. + L4c is what wires it in, and until then no user-visible behaviour + changes. + + **Discoverability, handled.** The standalone codex lives at + `~/.local/bin/codex` with its PATH entry written into `.bashrc` — + which a GUI-launched Electron app never sources, making it *less* + discoverable than a `/usr/bin/codex` package was. `codexBinary` now + resolves through PATH plus the well-known dirs the way `kimiweb.ts` + does for kimi, with `TERMIPOD_CODEX_BIN` as the override. + + **L4c** (still to do) is the driver itself: the app-server JSON-RPC + client over L4b's channel, delta throttling, R1 approval cards, + `turn/interrupt`, config seeding, and resume argv from the N1 table. + The translation half is already done and shipped — the hub's `codex` + frame profile maps `thread/started` → `session.init`, + `item/completed` → `text`/`tool_result` and so on, and it rides to + the desktop in `agent_families.generated.json`, so L4c reads that + table rather than writing a second one. The vendor also generates the + whole protocol on demand (`codex app-server generate-ts --out DIR`, + and `generate-json-schema`), which is the authority to check request + shapes against. ### Lane E — event-vocabulary gaps (hub; verified per-driver) From 2092de6533e9831f9fe4d23cd6a6f467833d3042 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 13:43:34 +0000 Subject: [PATCH 2/2] fix(desktop): drain the spawned app-server's stderr into onJunk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spawn rung piped stderr and never read it. codex writes WARNING lines there (the PATH-aliases complaint appears on every run against a relocated CODEX_HOME), and an unread pipe blocks the engine outright once the ~64 KiB buffer fills — a mid-session hang with no symptom, in the module whose own contract says swallowing engine output is how features ship invisible. stderr lines now flow to onJunk, never to onFrame (a JSON-shaped log line is still a log line), the unterminated tail is flushed on exit, and the drain is attached even when no onJunk handler is given so the pipe can never back up. Co-Authored-By: Claude Fable 5 --- .../src/localagent/codexchannel.test.ts | 26 +++++++++++++++++++ .../electron/src/localagent/codexchannel.ts | 21 +++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/desktop/electron/src/localagent/codexchannel.test.ts b/desktop/electron/src/localagent/codexchannel.test.ts index 4697e5f3..9f24d826 100644 --- a/desktop/electron/src/localagent/codexchannel.test.ts +++ b/desktop/electron/src/localagent/codexchannel.test.ts @@ -253,6 +253,32 @@ test('spawn rung: send writes a newline-delimited frame; close ends stdin', asyn assert.ok(d.child.killed); }); +test('spawn rung: stderr is drained to onJunk, never to onFrame', async () => { + const c = collector(); + const d = deps(); + await openCodexChannel(SPAWN_PLAN, c.handlers, { cwd: '/w' }, d.deps); + + // codex writes WARNING chatter to stderr. It must be READ — the child is + // spawned with stderr piped, and an unread pipe blocks the engine once the + // ~64 KiB buffer fills, a hang with no symptom — and it must land in onJunk: + // a JSON-shaped log line on stderr is still a log line, not a frame. + d.child.stderr.write('WARNING: could not create PATH aliases\n{"looks":"like json"}\npartial'); + await new Promise((r) => setImmediate(r)); + assert.deepEqual(c.junk, ['WARNING: could not create PATH aliases', '{"looks":"like json"}']); + assert.deepEqual(c.frames, [], 'stderr must never produce frames'); + + // The unterminated tail is still something the engine said. + d.child.emit('exit', 1); + assert.deepEqual(c.junk.at(-1), 'partial'); +}); + +test('spawn rung: stderr flows even when no onJunk handler is given', async () => { + const d = deps(); + // No onJunk — the drain must still be attached, or the pipe backs up. + await openCodexChannel(SPAWN_PLAN, { onFrame: () => {}, onClose: () => {} }, { cwd: '/w' }, d.deps); + assert.ok(d.child.stderr.listenerCount('data') > 0, 'stderr must have a reader regardless of handlers'); +}); + test('spawn rung: an exit flushes the unterminated tail before reporting close', async () => { const c = collector(); const d = deps(); diff --git a/desktop/electron/src/localagent/codexchannel.ts b/desktop/electron/src/localagent/codexchannel.ts index f0e671d7..9054372a 100644 --- a/desktop/electron/src/localagent/codexchannel.ts +++ b/desktop/electron/src/localagent/codexchannel.ts @@ -299,10 +299,31 @@ function openSpawnChannel( for (const j of junk) handlers.onJunk?.(j); }); + // stderr is never protocol, but it MUST be drained: the child is spawned with + // stderr piped, and codex writes WARNING lines there — an unread pipe blocks + // the engine outright once the ~64 KiB buffer fills, a hang with no symptom. + // The lines themselves go to onJunk (never onFrame, however JSON-shaped a log + // line looks): engine complaints belong somewhere other than the floor. + let errTail = ''; + const relayErrText = (text: string): void => { + const pieces = (errTail + text).split('\n'); + errTail = pieces.pop() ?? ''; + for (const line of pieces) { + const t = line.trim(); + if (t !== '') handlers.onJunk?.(t); + } + }; + child.stderr?.setEncoding?.('utf8'); + child.stderr?.on('data', (chunk: string | Buffer) => { + relayErrText(typeof chunk === 'string' ? chunk : chunk.toString('utf8')); + }); + child.on('exit', (code) => { const { frames, junk } = buffer.flush(); for (const f of frames) handlers.onFrame(f); for (const j of junk) handlers.onJunk?.(j); + if (errTail.trim() !== '') handlers.onJunk?.(errTail.trim()); + errTail = ''; handlers.onClose({ code, reason: `codex app-server exited (code ${String(code)})` }); });