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); + }); +});