From 6254c568c89e4e63d83fb99b2439c7072babfb2a Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 23:23:47 -0500 Subject: [PATCH] feat(desktop): opt-in renderer debugging port for dev runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The renderer is a Chromium page, and apps/desktop already carries a whole CDP toolkit for it — scripts/eval.mjs, scripts/perf/lib/cdp.mjs with its shared SELECTORS map, and the diag-*/probe-* family. None of it can attach to `hgui` or `npm run dev`, because neither passes --remote-debugging-port. The only launcher that opens one is `npm run perf:serve`, which is a separate isolated instance rather than the app you're looking at. Add HERMES_DESKTOP_CDP_PORT. When set, the shell opens a CDP port on loopback so that existing tooling can read the live DOM: computed styles, geometry, which rule actually won. Three independent gates, all required, resolved by a pure function in electron/dev-cdp.ts so the policy is testable without an Electron app: 1. not packaged — a shipped build never opens the port, and this is checked first so no env combination can talk it into doing so; 2. HERMES_DESKTOP_DEV_SERVER present — an unpackaged `electron .` against dist/ is how the packaged app gets smoke tested, so it behaves like the packaged app here; 3. the port explicitly requested and a valid integer. Default `npm run dev` is unchanged and silent: no port, no nag. An opt-in that gets refused always logs why, so nobody loses an hour wondering what isn't listening. The address is pinned to 127.0.0.1 rather than left to Chromium's default, and is deliberately not configurable — there's no reason to expose a renderer debugger off-host and offering the knob invites someone to try. scripts/eval.mjs hardcoded :9222 and threw a raw ECONNREFUSED stack when nothing was there. It now honours the same variable and explains itself. --- apps/desktop/electron/dev-cdp.test.ts | 80 ++++++++++++++++ apps/desktop/electron/dev-cdp.ts | 96 +++++++++++++++++++ apps/desktop/electron/main.ts | 23 +++++ apps/desktop/scripts/eval.mjs | 28 +++++- .../docs/reference/environment-variables.md | 1 + 5 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 apps/desktop/electron/dev-cdp.test.ts create mode 100644 apps/desktop/electron/dev-cdp.ts diff --git a/apps/desktop/electron/dev-cdp.test.ts b/apps/desktop/electron/dev-cdp.test.ts new file mode 100644 index 000000000000..bb86a57a1287 --- /dev/null +++ b/apps/desktop/electron/dev-cdp.test.ts @@ -0,0 +1,80 @@ +/** + * Tests for electron/dev-cdp.ts. + * + * Run with: npx vitest run --project electron electron/dev-cdp.test.ts + */ + +import assert from 'node:assert/strict' + +import { test } from 'vitest' + +import { describeDevCdpDecision, resolveDevCdpPort } from './dev-cdp' + +const DEV_SERVER = 'http://127.0.0.1:5174' + +/** A dev-server run that asked for a port — the one combination that opens it. */ +const opted = { env: { HERMES_DESKTOP_CDP_PORT: '9222' }, isPackaged: false, devServer: DEV_SERVER } + +test('opens the requested port for an opted-in dev-server run', () => { + assert.deepEqual(resolveDevCdpPort(opted), { port: 9222, reason: null }) +}) + +test('stays closed unless the developer asks for it', () => { + // The default `npm run dev` / `hgui` path: dev server, no opt-in. + assert.deepEqual(resolveDevCdpPort({ ...opted, env: {} }), { port: null, reason: 'not-requested' }) +}) + +test('a packaged build never opens the port, however loudly the env asks', () => { + assert.deepEqual(resolveDevCdpPort({ ...opted, isPackaged: true }), { port: null, reason: 'packaged' }) +}) + +test('packaged wins over every other gate', () => { + // Belt-and-suspenders: even with a dev server present and a valid port + // requested, packaged is checked first and short-circuits. + const decision = resolveDevCdpPort({ + env: { HERMES_DESKTOP_CDP_PORT: '9222' }, + isPackaged: true, + devServer: DEV_SERVER + }) + + assert.equal(decision.port, null) + assert.equal(decision.reason, 'packaged') +}) + +test('an unpackaged dist run (no dev server) does not qualify', () => { + // `electron .` against dist/ is how the packaged app gets smoke tested; it + // should behave like the packaged app, not like a source-tree dev run. + assert.deepEqual(resolveDevCdpPort({ ...opted, devServer: undefined }), { port: null, reason: 'no-dev-server' }) +}) + +test('rejects ports that are not usable integers', () => { + for (const value of ['0', '80', '-1', '70000', 'yes', '9222.5', '92 22', '']) { + const decision = resolveDevCdpPort({ ...opted, env: { HERMES_DESKTOP_CDP_PORT: value } }) + + assert.equal(decision.port, null, `expected ${JSON.stringify(value)} to be refused`) + } +}) + +test('tolerates surrounding whitespace on the requested port', () => { + assert.equal(resolveDevCdpPort({ ...opted, env: { HERMES_DESKTOP_CDP_PORT: ' 9333 ' } }).port, 9333) +}) + +test('every refusal that followed an explicit request explains itself', () => { + // An opt-in that gets ignored must say why — a silent no-op is the failure + // mode where someone burns an hour wondering why nothing is listening. + const refusals = [ + resolveDevCdpPort({ ...opted, isPackaged: true }), + resolveDevCdpPort({ ...opted, devServer: undefined }), + resolveDevCdpPort({ ...opted, env: { HERMES_DESKTOP_CDP_PORT: 'nope' } }) + ] + + for (const decision of refusals) { + assert.equal(decision.port, null) + assert.ok(describeDevCdpDecision(decision), `expected an explanation for ${decision.reason}`) + } +}) + +test('says nothing when the port opened, or when it was never requested', () => { + assert.equal(describeDevCdpDecision(resolveDevCdpPort(opted)), null) + assert.equal(describeDevCdpDecision(resolveDevCdpPort({ ...opted, env: {} })), null) +}) diff --git a/apps/desktop/electron/dev-cdp.ts b/apps/desktop/electron/dev-cdp.ts new file mode 100644 index 000000000000..dc4495fcd796 --- /dev/null +++ b/apps/desktop/electron/dev-cdp.ts @@ -0,0 +1,96 @@ +/** + * Dev-only Chrome DevTools Protocol exposure for the desktop renderer. + * + * The renderer is a Chromium page, so `--remote-debugging-port` turns it into + * something the repo's existing CDP tooling (`scripts/eval.mjs`, + * `scripts/perf/lib/cdp.mjs`, the `diag-*` / `probe-*` family) can attach to + * and read the live DOM from. That is genuinely useful while iterating on the + * UI — and it is also arbitrary code execution against whatever the running + * app can reach, so it stays off unless three independent conditions all hold: + * + * 1. The build is NOT packaged. A shipped app never opens this port, whatever + * the environment says. + * 2. A dev server is wired up (`HERMES_DESKTOP_DEV_SERVER`). That is the + * signature of `npm run dev` / `hgui`; a packaged or `dist`-loading run + * has no dev server and does not qualify. + * 3. The developer opted in explicitly with a valid `HERMES_DESKTOP_CDP_PORT`. + * Absent that, a plain `npm run dev` behaves exactly as it does today — + * nobody gets a debugging port they did not ask for. + * + * The port binds to loopback (Chromium's default) and the address is + * deliberately not configurable: there is no reason to expose a renderer + * debugger off-host, and offering the knob invites someone to try. + */ + +/** Why the port is closed, for a one-line log the developer can act on. */ +type ClosedReason = 'packaged' | 'no-dev-server' | 'not-requested' | 'invalid-port' + +type DevCdpDecision = { port: number; reason: null } | { port: null; reason: ClosedReason } + +type DevCdpInput = { + env: Record + isPackaged: boolean + devServer: string | undefined +} + +// Below 1024 needs privileges; the ephemeral range is fair game but the +// well-known CDP port (9222) is what every script in scripts/ defaults to. +const MIN_PORT = 1024 +const MAX_PORT = 65535 + +/** + * Decide whether this run may expose a renderer debugging port, and on which + * port. Pure: every input is passed in, so the gate is testable without an + * Electron app or a real environment. + */ +function resolveDevCdpPort({ env, isPackaged, devServer }: DevCdpInput): DevCdpDecision { + // Packaged wins over everything. Checked first so no combination of + // environment variables can talk a shipped build into opening the port. + if (isPackaged) { + return { port: null, reason: 'packaged' } + } + + const requested = (env.HERMES_DESKTOP_CDP_PORT ?? '').trim() + + if (!requested) { + return { port: null, reason: 'not-requested' } + } + + // A dev server means a source-tree run (`npm run dev` / `hgui`). An + // unpackaged `electron .` against dist/ is how the packaged app is smoke + // tested, and it should behave like the packaged app here. + if (!devServer) { + return { port: null, reason: 'no-dev-server' } + } + + const port = Number(requested) + + if (!Number.isInteger(port) || port < MIN_PORT || port > MAX_PORT) { + return { port: null, reason: 'invalid-port' } + } + + return { port, reason: null } +} + +/** One-line explanation for a closed port, or null when it opened. */ +function describeDevCdpDecision(decision: DevCdpDecision): string | null { + switch (decision.reason) { + case null: + return null + + case 'packaged': + return 'HERMES_DESKTOP_CDP_PORT ignored: renderer debugging is dev-only and this is a packaged build.' + + case 'no-dev-server': + return 'HERMES_DESKTOP_CDP_PORT ignored: no HERMES_DESKTOP_DEV_SERVER, so this is not a dev-server run.' + + case 'invalid-port': + return `HERMES_DESKTOP_CDP_PORT ignored: not a valid port (expected an integer ${MIN_PORT}-${MAX_PORT}).` + + case 'not-requested': + return null + } +} + +export { describeDevCdpDecision, resolveDevCdpPort } +export type { DevCdpDecision } diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 7ac24c57f8ed..c0194c9437e3 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -81,6 +81,7 @@ import { shouldRemoveAppBundle, uninstallArgsForMode } from './desktop-uninstall' +import { describeDevCdpDecision, resolveDevCdpPort } from './dev-cdp' import { installEmbedReferer } from './embed-referer' import { createEventDeduper } from './event-dedupe' import { findGitBash as _findGitBash } from './find-git-bash' @@ -272,6 +273,28 @@ if (REMOTE_DISPLAY_REASON) { ) } +// Renderer debugging port for `hgui` / `npm run dev`. Opt-in, dev-server-only, +// never packaged — see electron/dev-cdp.ts for the gate. Must run before app +// `ready` like the switches above; Chromium binds it at launch. +const DEV_CDP = resolveDevCdpPort({ env: process.env, isPackaged: IS_PACKAGED, devServer: DEV_SERVER }) + +if (DEV_CDP.port) { + app.commandLine.appendSwitch('remote-debugging-port', String(DEV_CDP.port)) + // Loopback only. Chromium already defaults to 127.0.0.1, but say it out loud + // so a future edit can't widen it by omission. + app.commandLine.appendSwitch('remote-debugging-address', '127.0.0.1') + console.log( + `[hermes] renderer debugging on http://127.0.0.1:${DEV_CDP.port} (dev only; HERMES_DESKTOP_CDP_PORT). ` + + 'Anything that can reach this port can run code in the renderer.' + ) +} else { + const why = describeDevCdpDecision(DEV_CDP) + + if (why) { + console.warn(`[hermes] ${why}`) + } +} + // WSLg: Chromium blocklists the Mesa vGPU → software compositing → typing lag. // /dev/dxg means a real GPU is available; un-blocklist it. Skipped when a remote // display already forced software (SSH'd-into-WSL). diff --git a/apps/desktop/scripts/eval.mjs b/apps/desktop/scripts/eval.mjs index b7336315d292..5e1ff95f3c7b 100644 --- a/apps/desktop/scripts/eval.mjs +++ b/apps/desktop/scripts/eval.mjs @@ -1,6 +1,30 @@ // Simple eval helper — runs an expression and returns the result.value. -const targets = await (await fetch('http://127.0.0.1:9222/json')).json() -const t = targets.find((t) => t.url.includes('5174')) +// +// node scripts/eval.mjs "document.title" +// HERMES_DESKTOP_CDP_PORT=9333 node scripts/eval.mjs "document.title" +// +// Needs a renderer with a debugging port: launch `hgui` / `npm run dev` with +// HERMES_DESKTOP_CDP_PORT set (see electron/dev-cdp.ts). +const port = Number(process.env.HERMES_DESKTOP_CDP_PORT || 9222) +let targets + +try { + targets = await (await fetch(`http://127.0.0.1:${port}/json`)).json() +} catch { + console.error( + `no renderer debugging port on 127.0.0.1:${port}. ` + + 'Relaunch the app with HERMES_DESKTOP_CDP_PORT set, or point this script at the right port.' + ) + process.exit(1) +} + +const t = targets.find((t) => t.url.includes('5174')) ?? targets.find((t) => t.type === 'page') + +if (!t) { + console.error(`no page target on 127.0.0.1:${port} (found ${targets.length} target(s))`) + process.exit(1) +} + const ws = new WebSocket(t.webSocketDebuggerUrl) let id = 0 const pending = new Map() diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index ec99660fd783..1a72ca10e749 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -530,6 +530,7 @@ Three dashboard-auth providers ship in the box. For a remote Hermes Desktop conn | `HERMES_DESKTOP_CWD` | Initial project directory for Desktop chat sessions. Set by `hermes desktop --cwd`. | | `HERMES_DESKTOP_PYTHON` | Absolute path to a Python interpreter for the backend, checked before Electron auto-resolves one for the source checkout. Used by worktree dev helpers (see [TUI & Desktop from Worktrees](../developer-guide/worktree-ui-dev.md)) to reuse a shared venv. | | `HERMES_DESKTOP_DEV_SERVER` | Vite dev-server URL the Electron shell loads instead of the packaged bundle (e.g. `http://127.0.0.1:5174`). Set automatically by `npm run dev`; only relevant when hacking on the app. | +| `HERMES_DESKTOP_CDP_PORT` | Opens a Chrome DevTools Protocol port on `127.0.0.1` for the renderer, so DOM/CSS inspection tooling can attach (see [TUI & Desktop from Worktrees](../developer-guide/worktree-ui-dev.md)). Ignored unless the build is unpackaged **and** `HERMES_DESKTOP_DEV_SERVER` is set — a packaged app never opens it. Anything that can reach the port can execute code in the renderer, so leave it unset unless you're actively debugging. | ### Microsoft Graph (Teams Meetings)