From c30dfaf0e7114078b954ea101b8883c7d7f4601b Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 25 Aug 2026 03:08:15 +0200 Subject: [PATCH] fix(deepseek): run the web UI server in the background, not in a shell tab Clicking "DeepSeek web UI..." opened two tabs: the web tab asked for, and a shell tab running the server next to it. The shell was deliberate - the server lived in an ordinary session so it was visible, scrollable, killable and died with its tab, and nothing new had to supervise a long-lived HTTP server. That reasoning was sound and the result was still wrong in use: opening a dashboard should open one tab, and after the first launch the terminal is pure noise. The server moves to a background child process owned by a new `src/deepseek-web-server.ts`, behind `POST /api/deepseek/web`. What the session gave away for free is now explicit, which is most of the module: - Exactly one server. A second click reuses the running one instead of racing it for a port; the session flow could not do this at all, because two clicks were simply two sessions. - Restarted when the requested authority changes. `--trusted-host` fences dsh's own /api against the browser authority, and a Codeman reachable at both loopback and a tailnet name has two. Reusing a server fenced for the other origin renders a page whose every call 403s, which reads as a broken dashboard rather than a misconfigured one, so a mismatch restarts instead. - Killed on shutdown. The child is detached so its whole plugin tree can be signalled at once, which also means it would outlive Codeman and hold its port against the next start - the exact EADDRINUSE this feature already got wrong once. - Boot output captured and returned. With no shell tab there is nowhere else for a stack trace to land, so a failed spawn reports its own tail. The endpoint is fenced at the same bar as the profile installer and for the same reason: booting a dsh profile executes the plugin code in it, so this is a privileged action even though it reads as "open a page". `authority` comes from the client (`location.host`) because only the browser knows which origin is in play, and it is regex-confined at the schema boundary - defence in depth behind the argv-array spawn, admitting host:port in the shapes a browser authority can take and nothing readable as a second argument. `GET /api/deepseek/web-port` is gone; port selection moved into the supervisor, which is the thing that knows whether a server is already running. The two client-side probe helpers went with it, since the server now owns the wait. Verified over the tailnet authority end to end: no session is created (session count unchanged, one tab), the server runs on 3081 beside the user's own dsh web on 3080, status reports the tailnet authority, and the proxied dashboard renders with zero 4xx. Full gate green (6148 passed, +6). --- docs/deepseek-integration-plan.md | 41 +++-- src/deepseek-web-server.ts | 252 ++++++++++++++++++++++++++++++ src/web/public/session-ui.js | 106 +++---------- src/web/routes/system-routes.ts | 77 +++++---- src/web/schemas.ts | 20 +++ src/web/server.ts | 7 + test/deepseek-web-server.test.ts | 73 +++++++++ 7 files changed, 440 insertions(+), 136 deletions(-) create mode 100644 src/deepseek-web-server.ts create mode 100644 test/deepseek-web-server.test.ts diff --git a/docs/deepseek-integration-plan.md b/docs/deepseek-integration-plan.md index aa4a7ad2..21a2e864 100644 --- a/docs/deepseek-integration-plan.md +++ b/docs/deepseek-integration-plan.md @@ -49,14 +49,30 @@ in the six external CLIs before it. The browser UI is the only interactive surface DeepSeek ships itself, so it gets a **shortcut, not a run mode**: `Run ▸ DeepSeek web UI…` starts `dsh web --no-open --host 127.0.0.1 --port --trusted-host ` -in an ordinary shell session and opens the URL as an ordinary web tab. +as a background process and opens the URL as an ordinary web tab. + +The server was a **shell session** first, on the reasoning that Codeman already +supervises those (visible, scrollable, killable, dies with its tab) so nothing +new had to own a long-lived HTTP server. That version worked and was still +wrong in use: clicking "open the DeepSeek web UI" put a terminal tab on screen +next to the web tab actually asked for, every single time, and after the first +launch the terminal was pure noise. Opening a dashboard should open one tab. + +So `POST /api/deepseek/web` owns it instead (`src/deepseek-web-server.ts`), and +what the session gave away for free is now explicit: exactly one server, reused +rather than raced on a second click; restarted when the requested authority +changes; killed on Codeman shutdown (a detached child would otherwise hold its +port against the next start — the very EADDRINUSE this feature already got +wrong once); and boot output captured, since with no shell tab there is nowhere +else for a stack trace to land. It is fenced at the same bar as the profile +installer: booting a dsh profile executes the plugin code in it, so it requires +the privileged grant in multi-user mode. -Built entirely from parts that already exist: the server is a shell session -(visible, scrollable, killable, dies with its tab) and the UI is a web tab. -Nothing new supervises a long-lived HTTP server, because Codeman already does. `--trusted-host` is load-bearing — dsh fences its `/api` behind a browser-trust check on the request authority, and a Codeman web tab reaches it through -Codeman's own origin via the webview proxy, not directly. +Codeman's own origin via the webview proxy, not directly. The authority comes +from the CLIENT (`location.host`) because only the browser knows which of a +multi-homed Codeman's origins is actually in play. Three things about this shortcut are load-bearing and each came from it failing in exactly that way against a real install: @@ -88,10 +104,11 @@ listing both showed the same dashboard twice. Being managed is also what lets a relaunch repoint the existing row instead of stacking one dead dashboard per restart, since the port is now chosen per launch. -⚠️ The authority baked into `--trusted-host` is the one the launch was clicked -from. Codeman reachable at several authorities (loopback *and* a tailnet name) -therefore needs the server restarted from whichever one is in use; the reuse -path checks that the server is reachable, not that it trusts the current origin. +The authority baked into `--trusted-host` is the one the launch was clicked +from, and reuse is conditional on it: a running server fenced for a *different* +origin is stopped and restarted rather than reused, because reusing it renders a +page whose every API call 403s — which reads as a broken dashboard rather than a +misconfigured one. ## 4. Touch points (the checklist) @@ -144,9 +161,9 @@ behaviour covered by 31 new unit tests; and an isolated instance used to exercis - A Docker case with `mode: 'deepseek'` (needs a `--no-cache` agent-image rebuild — see the `--no-cache` rule in CLAUDE.md). - A remote-SSH deepseek case. -- The web-UI shortcut against a tunnel authority. Loopback is verified end to end - through the webview proxy (dashboard renders, its `/api` calls succeed); the - cross-authority case above is a known limitation rather than an open question. +- The web-UI shortcut against a tunnel authority. Loopback and a tailnet name are + both verified end to end through the webview proxy (dashboard renders, its + `/api` calls succeed, no shell session created). ## 6. Follow-ups diff --git a/src/deepseek-web-server.ts b/src/deepseek-web-server.ts new file mode 100644 index 00000000..b7ebb05b --- /dev/null +++ b/src/deepseek-web-server.ts @@ -0,0 +1,252 @@ +/** + * @fileoverview Supervises the one background `dsh web` process behind the Run + * menu's "DeepSeek web UI..." entry. + * + * The shortcut originally started the server inside an ordinary SHELL SESSION, + * on the reasoning that Codeman already knows how to supervise those: it was + * visible, scrollable, killable, and died with its tab, and nothing new had to + * own a long-lived HTTP server. That reasoning was sound and the result was + * still wrong in use — clicking "open the DeepSeek web UI" spawned a terminal + * tab the user never asked for, next to the web tab they did, and the terminal + * was noise every time after the first. + * + * So the server moves here instead: one child process, no session, no tab. + * What that buys back has to be paid for explicitly, which is what this module + * is: + * + * - **Exactly one.** A second click reuses the running server rather than + * racing it for a port. The old shell-session flow could not do this at all, + * because two clicks were simply two sessions. + * - **Restarted when the authority changes.** `--trusted-host` fences dsh's + * `/api` against the browser authority, and a Codeman reachable at both + * loopback and a tailnet name has two. Whoever asks last wins, because the + * asker is by definition the origin about to load the page. + * - **Killed on shutdown.** A detached child that outlived Codeman would hold + * its port against the next start, which is exactly the EADDRINUSE this + * feature already got wrong once. + * - **Failures reported, not swallowed.** The shell tab used to be where the + * stack trace landed. With no tab, the spawn's own output is captured and + * handed back to the caller instead. + */ + +import { spawn, type ChildProcess } from 'node:child_process'; +import { createServer } from 'node:net'; +import { join } from 'node:path'; +import { getErrorMessage } from './types.js'; + +/** + * Where the port search starts, and how far it walks. + * + * 3080 is `dsh web`'s own default, so it is the friendly first choice — and + * emphatically not a fixed port. DeepSeek's web UI is a thing users run + * themselves, which makes the default precisely the port most likely to be + * taken already; hardcoding it made this feature die with EADDRINUSE against + * the user's own server. + */ +const PORT_BASE = 3080; +const PORT_SPAN = 40; + +/** How long a freshly spawned server gets to answer before we call it failed. */ +const READY_TIMEOUT_MS = 30_000; +const READY_POLL_MS = 400; +/** Grace between SIGTERM and SIGKILL when stopping the tree. */ +const KILL_GRACE_MS = 3_000; +/** Bound on captured child output, so a chatty boot cannot grow without limit. */ +const OUTPUT_CAP = 16_384; + +export interface DeepSeekWebStatus { + running: boolean; + port: number | null; + url: string | null; + /** Browser authority this server was started to trust (`--trusted-host`). */ + authority: string | null; +} + +interface RunningServer { + child: ChildProcess; + port: number; + authority: string; + output: () => string; +} + +let current: RunningServer | null = null; + +/** + * True when nothing holds `port` on loopback. + * + * Binding is the only honest test: a connect probe cannot tell "free" from + * "listening but not answering yet", and this runs moments before `dsh web` + * binds the same port. It is inherently racy, which is why the caller still + * waits for the server to actually answer before reporting success. + */ +async function isLoopbackPortFree(port: number): Promise { + return new Promise((resolve) => { + const probe = createServer(); + probe.once('error', () => resolve(false)); + probe.once('listening', () => probe.close(() => resolve(true))); + probe.listen(port, '127.0.0.1'); + }); +} + +async function findFreePort(): Promise { + for (let port = PORT_BASE; port < PORT_BASE + PORT_SPAN; port++) { + if (await isLoopbackPortFree(port)) return port; + } + return null; +} + +/** Does the server answer HTTP yet? Any status counts: dsh may 4xx a bare GET. */ +async function answersHttp(port: number): Promise { + try { + await fetch(`http://127.0.0.1:${port}/`, { signal: AbortSignal.timeout(2_000) }); + return true; + } catch { + return false; + } +} + +/** + * Signal the whole process group. + * + * `dsh web` boots a plugin tree and fans out, so signalling only the direct + * child leaves survivors holding the port. Same negative-pid escalation as + * `runGit()` in git-clone.ts and the profile installer. + */ +function killTree(child: ChildProcess, signal: NodeJS.Signals): void { + try { + if (child.pid) process.kill(-child.pid, signal); + } catch { + try { + child.kill(signal); + } catch { + /* already gone */ + } + } +} + +export function getDeepSeekWebStatus(): DeepSeekWebStatus { + if (!current) return { running: false, port: null, url: null, authority: null }; + return { + running: true, + port: current.port, + url: `http://127.0.0.1:${current.port}`, + authority: current.authority, + }; +} + +/** Stop the background server, if one is running. Safe to call when none is. */ +export async function stopDeepSeekWeb(): Promise { + const running = current; + current = null; + if (!running) return; + + await new Promise((resolve) => { + let done = false; + const finish = () => { + if (done) return; + done = true; + clearTimeout(hard); + resolve(); + }; + running.child.once('exit', finish); + killTree(running.child, 'SIGTERM'); + const hard = setTimeout(() => { + killTree(running.child, 'SIGKILL'); + finish(); + }, KILL_GRACE_MS); + }); +} + +/** + * Start (or reuse) the background `dsh web` for `authority`. + * + * @param dshDir directory holding the resolved `dsh` binary. + * @param authority browser authority to pass as `--trusted-host`. + */ +export async function startDeepSeekWeb( + dshDir: string, + authority: string +): Promise<{ ok: true; port: number; url: string; reused: boolean } | { ok: false; error: string }> { + // Reuse only when the running server is BOTH healthy and fenced for the + // authority now asking. A server trusting the other origin renders a page + // whose every API call 403s, which looks like a broken dashboard rather than + // a misconfigured one. + if (current) { + if (current.authority === authority && (await answersHttp(current.port))) { + return { ok: true, port: current.port, url: `http://127.0.0.1:${current.port}`, reused: true }; + } + await stopDeepSeekWeb(); + } + + const port = await findFreePort(); + if (port === null) { + return { ok: false, error: `No free port for the DeepSeek web UI in ${PORT_BASE}-${PORT_BASE + PORT_SPAN - 1}` }; + } + + let child: ChildProcess; + try { + child = spawn( + join(dshDir, 'dsh'), + ['web', '--no-open', '--host', '127.0.0.1', '--port', String(port), '--trusted-host', authority], + { + stdio: ['ignore', 'pipe', 'pipe'], + // Own process group so the whole plugin tree can be signalled at once. + detached: true, + env: process.env, + } + ); + } catch (err) { + return { ok: false, error: `Failed to start dsh web: ${getErrorMessage(err)}` }; + } + + // The pipes must be drained whether or not anyone reads them: a full pipe + // blocks the child. Storage is capped; draining is not. + let output = ''; + const capture = (chunk: Buffer) => { + if (output.length < OUTPUT_CAP) output += chunk.toString('utf-8'); + }; + child.stdout?.on('data', capture); + child.stderr?.on('data', capture); + + let exited = false; + child.once('exit', () => { + exited = true; + // Only clear if this is still the current server: a restart may have + // already replaced it, and clearing then would drop the live one. + if (current?.child === child) current = null; + }); + child.once('error', () => { + exited = true; + if (current?.child === child) current = null; + }); + + const running: RunningServer = { child, port, authority, output: () => output }; + current = running; + + const deadline = Date.now() + READY_TIMEOUT_MS; + while (Date.now() < deadline) { + if (exited) { + current = null; + const tail = output.trim().slice(-800); + return { ok: false, error: tail ? `dsh web exited during startup: ${tail}` : 'dsh web exited during startup' }; + } + if (await answersHttp(port)) { + return { ok: true, port, url: `http://127.0.0.1:${port}`, reused: false }; + } + await new Promise((r) => setTimeout(r, READY_POLL_MS)); + } + + await stopDeepSeekWeb(); + const tail = output.trim().slice(-800); + return { + ok: false, + error: tail + ? `dsh web did not answer on port ${port} within ${READY_TIMEOUT_MS / 1000}s: ${tail}` + : `dsh web did not answer on port ${port} within ${READY_TIMEOUT_MS / 1000}s`, + }; +} + +/** Test seam: forget any tracked child without signalling it. */ +export function resetDeepSeekWebForTest(): void { + current = null; +} diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 3e28a68a..c718ffe7 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -497,11 +497,11 @@ Object.assign(CodemanApp.prototype, { /** * Start the DeepSeek Harness browser UI and open it as a Codeman web tab. * - * Deliberately built from parts that already exist rather than a new process - * manager: the server runs in an ordinary SHELL session, so it is visible, - * scrollable, killable and dies with its tab like anything else, and the UI - * itself is an ordinary web tab. Nothing here needs to know how to supervise a - * long-lived HTTP server, because Codeman already does. + * The server is a background child process owned by + * `deepseek-web-server.ts`, NOT a shell session. It was a shell session first, + * on the reasoning that Codeman already supervises those, and that version + * worked - it just put a terminal tab on screen beside the web tab the user + * actually asked for, on every click. Opening a dashboard should open one tab. * * `--trusted-host` is the load-bearing flag: dsh fences its `/api` behind a * browser-trust check on the request authority, and a Codeman web tab reaches @@ -526,63 +526,32 @@ Object.assign(CodemanApp.prototype, { */ async runDeepSeekWeb() { document.getElementById('runModeMenu')?.classList.remove('active'); - const caseName = document.getElementById('quickStartCase').value || 'testcase'; - const sessionName = `dsh-web-${caseName}`; - const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting the DeepSeek web UI in ${caseName}...`); + const ownsLaunchTerminal = this._beginSessionLaunchStatus('Starting the DeepSeek web UI...'); try { - // A server started by an earlier click may still be serving. Reusing it is - // what makes this entry idempotent: without the check, every click started - // a second `dsh web`, and the second one lost the port race. - const managed = [...(this.webviews?.values() || [])].find((w) => w.managed === 'deepseek-web'); - if (managed && (await this._probeUrlReachable(managed.url))) { - this._appendSessionLaunchStatus(ownsLaunchTerminal, `Already serving on ${managed.url} - opening it as a tab.`); - await this.openWebview(managed.id); - return; - } - - // Never hardcode the port. 3080 is `dsh web`'s own default, which makes it - // precisely the port a DeepSeek user is most likely to be running already; - // binding it unconditionally killed the launch with EADDRINUSE while the - // tab still opened onto nothing. - const portRes = await fetch('/api/deepseek/web-port'); - const portData = await portRes.json(); - if (!portData.success) throw new Error(portData.error || 'No free port for the DeepSeek web UI'); - const port = portData.data.port; - const url = `http://127.0.0.1:${port}`; - - const res = await fetch('/api/quick-start', { + // One request, and the server owns everything behind it: picking a free + // port, spawning, waiting for the port to answer, and reusing an already + // running server instead of racing it. This used to start the server in a + // shell SESSION, which worked but put a terminal tab on screen next to the + // web tab actually asked for, every single time. + // + // `authority` is what dsh fences its own `/api` behind (`--trusted-host`), + // so it must be the origin this page is loaded from rather than anything + // the server could guess: a Codeman reachable at both loopback and a + // tailnet name has two, and only the browser knows which one is in play. + const startRes = await fetch('/api/deepseek/web', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ caseName, mode: 'shell', sessionName }), + body: JSON.stringify({ authority: location.host }), }); - const data = await res.json(); - if (!data.success) throw new Error(data.error || 'Failed to start the shell session'); - const sessionId = data.data.sessionId; - await this._ensureCreatedSessionVisible(sessionId, data.data.session); - - // The shell needs a moment to reach its prompt before it will accept a - // command; the same settle the other shell-driven flows use. - await new Promise((r) => setTimeout(r, 1200)); - const cmd = `dsh web --no-open --host 127.0.0.1 --port ${port} --trusted-host ${location.host}`; - await fetch(`/api/sessions/${sessionId}/input`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ input: `${cmd}\r` }), - }); - - // Verify the server actually answers BEFORE persisting a tab for it. The - // tab used to open unconditionally, so a server that died on startup left - // a saved dashboard pointing at nothing and no hint as to why. - this._appendSessionLaunchStatus(ownsLaunchTerminal, `Waiting for ${url} to answer...`); - if (!(await this._waitForUrlReachable(url))) { - throw new Error(`The DeepSeek web UI never answered on ${url} - see the "${sessionName}" tab for what it printed.`); - } + const startData = await startRes.json(); + if (!startData.success) throw new Error(startData.error || 'Failed to start the DeepSeek web UI'); + const url = startData.data.url; // One managed record, repointed rather than duplicated: the port is chosen // per launch, so creating a fresh row each time would stack a dashboard // per restart, each pointing at a port nothing serves any more. - let webview = managed; + let webview = [...(this.webviews?.values() || [])].find((w) => w.managed === 'deepseek-web'); if (webview) { const patchRes = await fetch(`/api/webviews/${webview.id}`, { method: 'PATCH', @@ -617,37 +586,6 @@ Object.assign(CodemanApp.prototype, { } }, - /** - * Server-side reachability check for a URL the browser is about to embed. - * - * Goes through the existing webview probe rather than `fetch(url)` from the - * page: a loopback dashboard is cross-origin to Codeman and would fail CORS - * long before it could report whether anything is listening. - */ - async _probeUrlReachable(url) { - try { - const res = await fetch('/api/webviews/probe', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ url }), - }); - const data = await res.json(); - return !!(data.success && data.data?.reachable); - } catch { - return false; - } - }, - - /** Poll `_probeUrlReachable` until the server answers or the budget runs out. */ - async _waitForUrlReachable(url, timeoutMs = 25000, intervalMs = 1000) { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - if (await this._probeUrlReachable(url)) return true; - await new Promise((r) => setTimeout(r, intervalMs)); - } - return false; - }, - /** * Install a DeepSeek Harness terminal profile from the run menu. * diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index aa8275c3..5e0d0cbd 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -12,7 +12,6 @@ import fs from 'node:fs/promises'; import { totalmem, freemem, loadavg, cpus } from 'node:os'; import { execSync, spawn } from 'node:child_process'; import { randomBytes } from 'node:crypto'; -import { createServer } from 'node:net'; import { dataPath } from '../../config/instance.js'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type NiceConfig } from '../../types.js'; import { isUnauthenticatedNetworkAcknowledged } from '../network-auth-policy.js'; @@ -28,6 +27,7 @@ import { SubagentParentMapSchema, RevokeSessionSchema, DeepSeekInstallProfileSchema, + DeepSeekWebStartSchema, } from '../schemas.js'; import { subagentWatcher } from '../../subagent-watcher.js'; import { imageWatcher } from '../../image-watcher.js'; @@ -70,34 +70,6 @@ import { resolveTerminalHistoryConfig } from '../../config/terminal-history.js'; const DEEPSEEK_DEFAULT_TUI_PACKAGE = '@deepseek-harness-tui/dsh-tui'; const DEEPSEEK_DEFAULT_PROFILE = 'dsh-tui'; -/** - * Where `GET /api/deepseek/web-port` starts looking, and how far it walks. - * - * 3080 is `dsh web`'s own default, so it is the friendly first choice — but it - * is emphatically NOT a fixed port. DeepSeek's web UI is a thing users run - * themselves, so the default is exactly the port most likely to be taken - * already, and hardcoding it made the shortcut die with EADDRINUSE against the - * user's own server while the tab still opened onto nothing. - */ -const DEEPSEEK_WEB_PORT_BASE = 3080; -const DEEPSEEK_WEB_PORT_SPAN = 40; - -/** - * True when nothing holds `port` on the loopback interface. - * - * Binding is the only honest test: a connect probe cannot distinguish "free" - * from "listening but not answering yet", and this runs moments before `dsh web` - * binds the same port. The check is inherently racy, which is why the caller - * still verifies the server answered before it persists a tab for it. - */ -async function isLoopbackPortFree(port: number): Promise { - return new Promise((resolve) => { - const probe = createServer(); - probe.once('error', () => resolve(false)); - probe.once('listening', () => probe.close(() => resolve(true))); - probe.listen(port, '127.0.0.1'); - }); -} /** A plugin install compiles and links a dependency tree; npm-scale, not curl-scale. */ const DEEPSEEK_INSTALL_TIMEOUT_MS = 300_000; @@ -541,19 +513,44 @@ export function registerSystemRoutes( }; }); - // First free loopback port for a `dsh web` the UI is about to start. + // Start (or reuse) the background `dsh web` behind the Run menu shortcut. // - // The browser cannot answer this: it can neither bind a port nor tell a closed - // one from a filtered one. Keeping the choice server-side also keeps it next - // to the process that will inherit it. - app.get('/api/deepseek/web-port', async () => { - for (let port = DEEPSEEK_WEB_PORT_BASE; port < DEEPSEEK_WEB_PORT_BASE + DEEPSEEK_WEB_PORT_SPAN; port++) { - if (await isLoopbackPortFree(port)) return { success: true, data: { port } }; + // This runs as a plain child process rather than a shell SESSION on purpose. + // The session version worked, but it put a terminal tab on screen next to the + // web tab the user actually asked for, every single time. Nothing about a + // long-lived HTTP server needs to be a tab. + // + // Fenced at the same bar as the profile installer, and for the same reason: + // booting a dsh profile executes the plugin code in it, so this is a + // privileged action even though it reads as "open a page". + app.post('/api/deepseek/web', async (req) => { + const { authority } = parseBody(DeepSeekWebStartSchema, req.body); + if (isMultiUserMode() && !(await canUsernameRunPrivilegedCommands(getAuthUser(req).username))) { + return createErrorResponse( + ApiErrorCode.FORBIDDEN, + 'Starting the DeepSeek web UI requires the can-bypass-permissions grant' + ); } - return createErrorResponse( - ApiErrorCode.INTERNAL_ERROR, - `No free port for the DeepSeek web UI in ${DEEPSEEK_WEB_PORT_BASE}-${DEEPSEEK_WEB_PORT_BASE + DEEPSEEK_WEB_PORT_SPAN - 1}` - ); + + const { resolveDeepSeekDir, getDeepSeekNotFoundMessage } = await import('../../utils/deepseek-cli-resolver.js'); + const dir = resolveDeepSeekDir(); + if (!dir) return createErrorResponse(ApiErrorCode.OPERATION_FAILED, getDeepSeekNotFoundMessage()); + + const { startDeepSeekWeb } = await import('../../deepseek-web-server.js'); + const result = await startDeepSeekWeb(dir, authority); + if (!result.ok) return createErrorResponse(ApiErrorCode.OPERATION_FAILED, result.error); + return { success: true, data: { port: result.port, url: result.url, reused: result.reused } }; + }); + + app.get('/api/deepseek/web', async () => { + const { getDeepSeekWebStatus } = await import('../../deepseek-web-server.js'); + return { success: true, data: getDeepSeekWebStatus() }; + }); + + app.delete('/api/deepseek/web', async () => { + const { stopDeepSeekWeb } = await import('../../deepseek-web-server.js'); + await stopDeepSeekWeb(); + return { success: true, data: { stopped: true } }; }); // Bootstrap an interactive profile so the mode becomes usable. diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 25f6e7d7..826a5e4d 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -407,6 +407,26 @@ export const DeepSeekInstallProfileSchema = z }) .strict(); +/** + * POST /api/deepseek/web: start the background `dsh web` for one browser authority. + * + * `authority` becomes `--trusted-host`, which is what dsh fences its own `/api` + * behind, so it must be the origin the browser will actually load the tab from + * (`location.host`). It reaches a spawn as one element of an argv ARRAY, never a + * shell string, so this regex is defence in depth rather than the only guard: it + * admits host:port in the shapes a browser authority can take (dotted names, + * IPv4, bracketed IPv6) and nothing that could be read as a second argument. + */ +export const DeepSeekWebStartSchema = z + .object({ + authority: z + .string() + .min(1) + .max(255) + .regex(/^(?:\[[0-9a-fA-F:]+\]|[a-zA-Z0-9](?:[a-zA-Z0-9.-]*[a-zA-Z0-9])?)(?::\d{1,5})?$/), + }) + .strict(); + /** * The session that spawned the one being created — pure UI decoration, drawn as a * lineage line between the two tabs. Accepted here and, equivalently, as the diff --git a/src/web/server.ts b/src/web/server.ts index f24fee63..6227f60b 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -95,6 +95,7 @@ import { sessionWaits } from './session-wait-registry.js'; import { intentStore } from '../intent-store.js'; import { AI_CHECK_MODEL } from '../config/ai-defaults.js'; import { approvalInbox } from './approval-inbox.js'; +import { stopDeepSeekWeb } from '../deepseek-web-server.js'; import { wireRespawnListeners, setupTimedRespawn, @@ -3135,6 +3136,12 @@ export class WebServer extends EventEmitter { this._dockerBridgeServer = null; } + // The background `dsh web` is detached so its whole plugin tree can be + // signalled at once, which also means it would OUTLIVE Codeman and hold its + // port against the next start — the exact EADDRINUSE this feature already + // got wrong once. + void stopDeepSeekWeb(); + // Dispose all managed timers (intervals + resettable timeouts) this.cleanup.dispose(); diff --git a/test/deepseek-web-server.test.ts b/test/deepseek-web-server.test.ts new file mode 100644 index 00000000..ec2703b4 --- /dev/null +++ b/test/deepseek-web-server.test.ts @@ -0,0 +1,73 @@ +/** + * The background `dsh web` supervisor and the authority boundary in front of it. + * + * Two things here are worth pinning and neither is obvious from reading the + * module: + * + * 1. `authority` becomes an argv element of a spawned process (`--trusted-host + * `). The spawn is an argv ARRAY so a shell can never see it, but + * the schema is the layer that stops a value which is not a browser + * authority at all from reaching the command line, and a regex is easy to + * widen by accident. + * 2. The supervisor tracks at most ONE server. The status accessor is what every + * caller reads to decide whether to start another, so "no server" must report + * as absent rather than as a half-populated record. + */ +import { describe, expect, it, beforeEach } from 'vitest'; +import { DeepSeekWebStartSchema } from '../src/web/schemas.js'; +import { getDeepSeekWebStatus, resetDeepSeekWebForTest, stopDeepSeekWeb } from '../src/deepseek-web-server.js'; + +describe('DeepSeekWebStartSchema: the authority reaching --trusted-host', () => { + it('accepts the authority shapes a browser can actually report', () => { + for (const authority of [ + 'localhost:3000', + '127.0.0.1:5013', + 'tnode.tailf80371.ts.net:8444', + 'codeman.example.com', + '[::1]:3000', + 'host-with-dashes.local:80', + ]) { + expect(DeepSeekWebStartSchema.safeParse({ authority }).success, authority).toBe(true); + } + }); + + it('rejects values that are not an authority at all', () => { + for (const authority of [ + '', + 'http://localhost:3000', // a URL, not an authority + 'localhost:3000 --trusted-host evil', // an embedded second argument + '-oProxyCommand=evil', // leading dash, readable as a flag + 'localhost:3000/../path', + 'local host:3000', + 'user:pass@localhost:3000', + 'a'.repeat(256), + ]) { + expect(DeepSeekWebStartSchema.safeParse({ authority }).success, authority).toBe(false); + } + }); + + it('is strict, so an unexpected field cannot ride along', () => { + expect(DeepSeekWebStartSchema.safeParse({ authority: 'localhost:3000', port: 1 }).success).toBe(false); + }); + + it('requires the field rather than defaulting it', () => { + // A guessed default would silently fence dsh's /api against the wrong + // origin, which presents as a dashboard whose every call 403s. + expect(DeepSeekWebStartSchema.safeParse({}).success).toBe(false); + }); +}); + +describe('DeepSeek web supervisor: status', () => { + beforeEach(() => { + resetDeepSeekWebForTest(); + }); + + it('reports absent as fully null, not a half-filled record', () => { + expect(getDeepSeekWebStatus()).toEqual({ running: false, port: null, url: null, authority: null }); + }); + + it('stopping when nothing runs resolves rather than throwing', async () => { + await expect(stopDeepSeekWeb()).resolves.toBeUndefined(); + expect(getDeepSeekWebStatus().running).toBe(false); + }); +});