diff --git a/docs/deepseek-integration-plan.md b/docs/deepseek-integration-plan.md index f1c294ae..aa4a7ad2 100644 --- a/docs/deepseek-integration-plan.md +++ b/docs/deepseek-integration-plan.md @@ -48,7 +48,7 @@ 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 3080 --trusted-host ` +`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. Built entirely from parts that already exist: the server is a shell session @@ -58,6 +58,41 @@ Nothing new supervises a long-lived HTTP server, because Codeman already does. check on the request authority, and a Codeman web tab reaches it through Codeman's own origin via the webview proxy, not directly. +Three things about this shortcut are load-bearing and each came from it failing +in exactly that way against a real install: + +- **The port is chosen, never hardcoded.** `GET /api/deepseek/web-port` walks + 3080..3119 for a free loopback port. 3080 is dsh's own default, which makes it + precisely the port a DeepSeek user is most likely to already be serving on: + binding it unconditionally killed the launch with `EADDRINUSE` against the + user's own `dsh web`. +- **The tab is opened only after the server answers.** The launch polls + `POST /api/webviews/probe` until the URL responds, so a server that dies on + startup reports the failure and points at its shell tab, instead of silently + persisting a dashboard aimed at nothing. +- **The saved tab is `trusted: true`, and must be.** An untrusted webview is + sandboxed without `allow-same-origin`, which breaks this dashboard twice: the + dsh client-runtime reads `localStorage` while loading plugins and dies there, + and an opaque-origin frame sends `Origin: null`, so dsh's trust check 403s + every `/api` call regardless of what `--trusted-host` names. Passing + `location.host` only means anything once the frame actually carries that + origin. The trade is real — a trusted proxied frame is same-origin with + Codeman and can reach Codeman's API — and is defensible only because this + particular dashboard is an agent harness Codeman just started itself on + loopback, which can already run code as the user. It is not a precedent for + trusting third-party dashboards generally. + +The record is marked `managed: 'deepseek-web'`, which keeps it out of the +saved-dashboard list: the shortcut that maintains it is already a menu entry, so +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. + ## 4. Touch points (the checklist) Backend: `types/session.ts` (SessionMode + `DeepSeekConfig` + SessionState), @@ -109,9 +144,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 end to end through the webview proxy, in particular whether - `--trusted-host ` is the right authority for dsh's `/api` - fence in every deployment shape (loopback, tailscale, tunnel). +- 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. ## 6. Follow-ups diff --git a/src/types/webview.ts b/src/types/webview.ts index 246f7675..7071d071 100644 --- a/src/types/webview.ts +++ b/src/types/webview.ts @@ -32,6 +32,16 @@ export type WebviewEmbedMode = 'proxy' | 'direct'; /** A saved dashboard, persisted to `~/.codeman/webviews.json`. */ +/** + * Dashboards Codeman creates and maintains on the user's behalf. + * + * A managed record is hidden from the saved-dashboard list, because the shortcut + * that maintains it is already a menu entry of its own: listing both showed the + * same dashboard twice, once as "DeepSeek web UI..." and once as the row it had + * just written. + */ +export type WebviewManagedKind = 'deepseek-web'; + export interface Webview { id: string; /** Display name shown on the tab. */ @@ -49,6 +59,12 @@ export interface Webview { * cookies/localStorage, only for dashboards the user fully trusts. */ trusted: boolean; + /** + * Set when Codeman owns this record rather than the user (see + * `WebviewManagedKind`). Managed rows are maintained by the shortcut that + * created them, including repointing the URL when the port changes. + */ + managed?: WebviewManagedKind; /** Multi-user owner (username). Undefined in single-user mode. */ owner?: string; createdAt: number; diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 851e5c7c..08b12710 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -54,12 +54,6 @@ const BROWSER_NOTIF_RATE_LIMIT_MS = 3000; // Rate limit for browser notificati const MOBILE_RESIZE_RETRY_MS = 30000; // Small-viewport resize re-send while a desktop sizing claim is hot const AUTO_CLOSE_NOTIFICATION_MS = 8000; // Auto-close browser notifications const THROTTLE_DELAY_MS = 100; // General UI throttle delay -/** - * Port the DeepSeek Harness browser UI is started on by the run-menu shortcut. - * dsh's own default, so a hand-started `dsh web` and the shortcut land on the - * same place and share one saved tab. - */ -const DEEPSEEK_WEB_PORT = 3080; const TERMINAL_CHUNK_SIZE = 32 * 1024; // 32KB chunks for terminal buffer loading const TERMINAL_TAIL_SIZE = 1024 * 1024; // 1MB tail for initial load (more scrollback on tab switch) const SYNC_WAIT_TIMEOUT_MS = 50; // Wait timeout for terminal sync diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 3599cd93..3e28a68a 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -507,23 +507,54 @@ Object.assign(CodemanApp.prototype, { * 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. Without * passing Codeman's authority the page renders and every API call fails. + * + * The tab is saved `trusted: true`, and that is REQUIRED rather than a + * convenience: an untrusted webview is sandboxed without `allow-same-origin`, + * which breaks this dashboard twice over. The dsh client-runtime reads + * `localStorage` while loading its plugins and dies there ("the document is + * sandboxed and lacks the 'allow-same-origin' flag"), and an opaque-origin + * frame sends `Origin: null`, so dsh's own trust check 403s every `/api` call + * no matter which authority `--trusted-host` names. Passing `location.host` + * only means anything once the frame actually carries that origin. + * + * The trade this makes is real and worth stating: a trusted proxied frame is + * same-origin with Codeman and can therefore reach Codeman's own API. It is + * defensible only because of what this specific dashboard already is - an + * agent harness Codeman just started itself, on loopback, which can run code + * as the user regardless. It is not a precedent for trusting third-party + * dashboards generally, which is why it is set here rather than defaulted. */ async runDeepSeekWeb() { document.getElementById('runModeMenu')?.classList.remove('active'); const caseName = document.getElementById('quickStartCase').value || 'testcase'; - const port = DEEPSEEK_WEB_PORT; - const url = `http://127.0.0.1:${port}`; + const sessionName = `dsh-web-${caseName}`; const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting the DeepSeek web UI in ${caseName}...`); 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', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - caseName, - mode: 'shell', - sessionName: `dsh-web-${caseName}`, - }), + body: JSON.stringify({ caseName, mode: 'shell', sessionName }), }); const data = await res.json(); if (!data.success) throw new Error(data.error || 'Failed to start the shell session'); @@ -540,28 +571,83 @@ Object.assign(CodemanApp.prototype, { body: JSON.stringify({ input: `${cmd}\r` }), }); - // Reuse a saved tab for the same URL rather than stacking duplicates every - // time the server is restarted. - let webview = [...(this.webviews?.values() || [])].find((w) => w.url === url); - if (!webview) { + // 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.`); + } + + // 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; + if (webview) { + const patchRes = await fetch(`/api/webviews/${webview.id}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ url, trusted: true }), + }); + const patchData = await patchRes.json(); + if (!patchData.success) throw new Error(patchData.error || 'Failed to update the web tab'); + webview = patchData.data.webview || patchData.data; + } else { const wvRes = await fetch('/api/webviews', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ name: 'DeepSeek Harness', url, icon: '🐳' }), + body: JSON.stringify({ + name: 'DeepSeek Harness', + url, + icon: '\u{1F433}', + managed: 'deepseek-web', + trusted: true, + }), }); const wvData = await wvRes.json(); if (!wvData.success) throw new Error(wvData.error || 'Failed to save the web tab'); webview = wvData.data.webview || wvData.data; - await this.loadWebviews?.(); } + await this.loadWebviews?.(); - this._appendSessionLaunchStatus(ownsLaunchTerminal, `Serving on ${url} — opening it as a tab.`); + this._appendSessionLaunchStatus(ownsLaunchTerminal, `Serving on ${url} - opening it as a tab.`); if (webview?.id) await this.openWebview(webview.id); } catch (err) { this._reportSessionLaunchError(ownsLaunchTerminal, err.message); } }, + /** + * 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/public/webview-tabs.js b/src/web/public/webview-tabs.js index 811367b0..97c87a44 100644 --- a/src/web/public/webview-tabs.js +++ b/src/web/public/webview-tabs.js @@ -302,7 +302,10 @@ Object.assign(CodemanApp.prototype, { renderWebviewMenuItems() { const container = document.getElementById('runModeWebviews'); if (!container) return; - const list = [...(this.webviews?.values() || [])]; + // Managed records are maintained by their own menu entry (the DeepSeek web + // UI shortcut), so listing them here showed one dashboard twice: the + // shortcut that starts it, and the row it wrote on the previous click. + const list = [...(this.webviews?.values() || [])].filter((w) => !w.managed); if (list.length === 0) { container.innerHTML = '
No URLs yet
'; return; diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index 7f906f36..aa8275c3 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -12,6 +12,7 @@ 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'; @@ -68,6 +69,35 @@ 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; @@ -511,6 +541,21 @@ export function registerSystemRoutes( }; }); + // First free loopback port for a `dsh web` the UI is about to start. + // + // 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 } }; + } + 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}` + ); + }); + // Bootstrap an interactive profile so the mode becomes usable. // // This exists because DeepSeek ships NO terminal front door: `dsh` on its own diff --git a/src/web/routes/webview-routes.ts b/src/web/routes/webview-routes.ts index 6f545607..f8aed284 100644 --- a/src/web/routes/webview-routes.ts +++ b/src/web/routes/webview-routes.ts @@ -132,6 +132,7 @@ function registerCrudRoutes(app: FastifyInstance, ctx: EventPort & TabLayoutPort // dashboard on an HTTPS Codeman, which is the common case. embedMode: input.embedMode ?? 'proxy', trusted: input.trusted ?? false, + managed: input.managed, owner, createdAt: Date.now(), }; diff --git a/src/web/schemas.ts b/src/web/schemas.ts index dd6f2acd..25f6e7d7 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -1679,6 +1679,12 @@ const WebviewBaseSchema = z.object({ * and call the API that spawns agents. */ trusted: z.boolean().optional(), + /** + * Marks a record Codeman maintains itself. Declared here because a plain + * `z.object` STRIPS undeclared keys, so an undeclared marker would be dropped + * on the way in and the dedup it drives would never fire. + */ + managed: z.enum(['deepseek-web']).optional(), }); /** POST /api/webviews */