diff --git a/apps/desktop/electron/dev-cdp.test.ts b/apps/desktop/electron/dev-cdp.test.ts new file mode 100644 index 00000000000..f86aa9db304 --- /dev/null +++ b/apps/desktop/electron/dev-cdp.test.ts @@ -0,0 +1,94 @@ +/** + * 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 { DEFAULT_PORT, describeDevCdpDecision, resolveDevCdpPort } from './dev-cdp' + +const DEV_SERVER = 'http://127.0.0.1:5174' + +/** The ordinary `npm run dev` / `hgui` run. */ +const devRun = { env: {}, isPackaged: false, devServer: DEV_SERVER } + +test('a dev-server run opens the default port with no opt-in', () => { + assert.deepEqual(resolveDevCdpPort(devRun), { port: DEFAULT_PORT, reason: null }) +}) + +test('the default matches what the scripts/ tooling reaches for', () => { + // scripts/eval.mjs and scripts/perf/lib/cdp.mjs both default here; if this + // drifts, `node scripts/eval.mjs ...` stops finding a live renderer. + assert.equal(DEFAULT_PORT, 9222) +}) + +test('a packaged build never opens the port, however loudly the env asks', () => { + const decision = resolveDevCdpPort({ ...devRun, env: { HERMES_DESKTOP_CDP_PORT: '9222' }, isPackaged: true }) + + assert.deepEqual(decision, { port: null, reason: 'packaged' }) +}) + +test('packaged is checked before every other gate', () => { + // Belt-and-suspenders: dev server present, valid port requested, still shut. + for (const value of ['9222', '', 'off', 'garbage']) { + const decision = resolveDevCdpPort({ + env: { HERMES_DESKTOP_CDP_PORT: value }, + isPackaged: true, + devServer: DEV_SERVER + }) + + assert.equal(decision.port, null, `expected packaged to refuse ${JSON.stringify(value)}`) + 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({ ...devRun, devServer: undefined }), { port: null, reason: 'no-dev-server' }) +}) + +test('the port is overridable', () => { + assert.equal(resolveDevCdpPort({ ...devRun, env: { HERMES_DESKTOP_CDP_PORT: '9333' } }).port, 9333) +}) + +test('tolerates surrounding whitespace on the override', () => { + assert.equal(resolveDevCdpPort({ ...devRun, env: { HERMES_DESKTOP_CDP_PORT: ' 9333 ' } }).port, 9333) +}) + +test('can be switched off on a dev run', () => { + for (const value of ['0', 'off', 'OFF', 'false', 'no']) { + const decision = resolveDevCdpPort({ ...devRun, env: { HERMES_DESKTOP_CDP_PORT: value } }) + + assert.equal(decision.port, null, `expected ${JSON.stringify(value)} to close the port`) + assert.equal(decision.reason, 'opted-out') + } +}) + +test('refuses ports that are not usable integers', () => { + for (const value of ['80', '-1', '70000', 'yes', '9222.5', '92 22']) { + const decision = resolveDevCdpPort({ ...devRun, env: { HERMES_DESKTOP_CDP_PORT: value } }) + + assert.equal(decision.port, null, `expected ${JSON.stringify(value)} to be refused`) + assert.equal(decision.reason, 'invalid-port') + } +}) + +test('explains itself when an explicit setting was not honoured', () => { + // A typo'd port or a deliberate opt-out should say so — silently doing + // something other than what the env asked for is the bad failure mode. + for (const value of ['garbage', 'off']) { + const decision = resolveDevCdpPort({ ...devRun, env: { HERMES_DESKTOP_CDP_PORT: value } }) + + assert.ok(describeDevCdpDecision(decision), `expected an explanation for ${JSON.stringify(value)}`) + } +}) + +test('stays quiet when the port opened, or is closed by design', () => { + assert.equal(describeDevCdpDecision(resolveDevCdpPort(devRun)), null) + assert.equal(describeDevCdpDecision(resolveDevCdpPort({ ...devRun, isPackaged: true })), null) + assert.equal(describeDevCdpDecision(resolveDevCdpPort({ ...devRun, devServer: undefined })), null) +}) diff --git a/apps/desktop/electron/dev-cdp.ts b/apps/desktop/electron/dev-cdp.ts new file mode 100644 index 00000000000..c5c4e3ef353 --- /dev/null +++ b/apps/desktop/electron/dev-cdp.ts @@ -0,0 +1,108 @@ +/** + * Dev 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. Every one of those scripts already defaults to + * 9222, so a dev-server run opens 9222 and they just work. + * + * If you are running a dev server you are already executing arbitrary local + * JS — vite's module graph and every postinstall in node_modules — so a + * loopback debugging port does not meaningfully widen that. `perf:serve` + * already opens one unconditionally. What must never happen is a *packaged* + * app exposing it, which is the one hard gate here. + * + * - packaged build → always closed, whatever the env says. + * - no HERMES_DESKTOP_DEV_SERVER → closed (an unpackaged `electron .` against + * dist/ is how the packaged app gets smoke tested; it should behave like + * the packaged app). + * - otherwise → open on 9222, or HERMES_DESKTOP_CDP_PORT. + * + * `HERMES_DESKTOP_CDP_PORT=off` (or `0` / `false`) opts out for anyone who + * wants the port closed on a dev run. + * + * 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' | 'opted-out' | 'invalid-port' + +type DevCdpDecision = { port: number; reason: null } | { port: null; reason: ClosedReason } + +type DevCdpInput = { + env: Record + isPackaged: boolean + devServer: string | undefined +} + +/** What every script under scripts/ already reaches for. */ +const DEFAULT_PORT = 9222 + +// Below 1024 needs privileges on most platforms; 65535 is the ceiling. +const MIN_PORT = 1024 +const MAX_PORT = 65535 + +const OPT_OUT = new Set(['0', 'off', 'false', 'no']) + +/** + * 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' } + } + + // A dev server means a source-tree run (`npm run dev` / `hgui`). + if (!devServer) { + return { port: null, reason: 'no-dev-server' } + } + + const requested = (env.HERMES_DESKTOP_CDP_PORT ?? '').trim() + + if (!requested) { + return { port: DEFAULT_PORT, reason: null } + } + + if (OPT_OUT.has(requested.toLowerCase())) { + return { port: null, reason: 'opted-out' } + } + + 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 'invalid-port': + return `HERMES_DESKTOP_CDP_PORT is not a valid port (expected an integer ${MIN_PORT}-${MAX_PORT}, or "off"); renderer debugging is disabled.` + + case 'opted-out': + return 'renderer debugging disabled by HERMES_DESKTOP_CDP_PORT.' + + // Packaged and dist-run builds are closed by design — the common case, not + // worth a line of startup noise. + case 'packaged': + + case 'no-dev-server': + return null + } +} + +export { DEFAULT_PORT, describeDevCdpDecision, resolveDevCdpPort } +export type { DevCdpDecision } diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 7ac24c57f8e..aad243ad188 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,29 @@ if (REMOTE_DISPLAY_REASON) { ) } +// Renderer debugging port. On for dev-server runs (`hgui` / `npm run dev`) so +// the CDP tooling in scripts/ can attach; never for a packaged build — see +// electron/dev-cdp.ts. 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} — anything that can reach it ` + + 'can run code in the renderer. HERMES_DESKTOP_CDP_PORT=off to disable.' + ) +} 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 b7336315d29..5f9fb282ec9 100644 --- a/apps/desktop/scripts/eval.mjs +++ b/apps/desktop/scripts/eval.mjs @@ -1,6 +1,31 @@ // 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}. ` + + 'Dev-server runs (`npm run dev` / `hgui`) open one automatically — check the app is running, ' + + 'and that HERMES_DESKTOP_CDP_PORT is not set to "off" or another 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/skills/software-development/inspecting-hermes-desktop-dom/SKILL.md b/skills/software-development/inspecting-hermes-desktop-dom/SKILL.md new file mode 100644 index 00000000000..1521fd10ec3 --- /dev/null +++ b/skills/software-development/inspecting-hermes-desktop-dom/SKILL.md @@ -0,0 +1,159 @@ +--- +name: inspecting-hermes-desktop-dom +description: "Read the live Hermes desktop DOM/CSS over CDP." +version: 1.0.0 +author: Hermes Agent +license: MIT +platforms: [linux, macos, windows] +metadata: + hermes: + tags: [desktop, electron, cdp, dom, ui-verification, self-inspection] + related_skills: [node-inspect-debugger, systematic-debugging, dogfood] +--- + +# Inspecting the live Hermes desktop DOM + +## Overview + +When you are developing `apps/desktop` and the user is running that same app +(`hgui` / `npm run dev`), you can read the **live rendered DOM** of the window +they are looking at — computed styles, geometry, which CSS rule actually won, +console output — instead of inferring it from `.tsx` and being wrong. + +Dev-server runs open a Chrome DevTools Protocol port on `127.0.0.1:9222` +automatically. The renderer is a Chromium page, so everything DevTools can read, +a script can read. + +**This does not replace looking at it.** CDP answers *factual* questions ("what +is the computed padding", "did this element render", "which selector matches"). +It cannot tell you whether the result looks good. Colour balance, spacing feel, +and "is this ugly" still need the user's eyes or a screenshot. Answer facts with +CDP; hand aesthetics to the user. + +## When to Use + +- Verifying a UI change actually took effect in the running app +- "Why is this element still X?" — find the winning rule before editing anything +- Locating a stable selector for a component you're about to change +- Checking a design token's computed value on a real node +- Reading renderer console errors the user mentions but can't copy out + +**Don't use for:** perf profiling or heap work (`node-inspect-debugger`, +`debugging-hermes-desktop`), or anything where the real question is "does this +look right". + +## The port + +Open on `127.0.0.1:9222` for any dev-server run. Closed in exactly two cases +(`apps/desktop/electron/dev-cdp.ts`): + +- **packaged builds** — always, and no environment value overrides it; +- **no `HERMES_DESKTOP_DEV_SERVER`** — an unpackaged `electron .` against + `dist/` is how the packaged app gets smoke tested, so it behaves like one. + +`HERMES_DESKTOP_CDP_PORT` moves the port (`=9333`) or disables it (`=off`). + +Check before doing anything else: + +```bash +curl -s --max-time 3 http://127.0.0.1:${HERMES_DESKTOP_CDP_PORT:-9222}/json/version +``` + +Empty → no port. Do not guess another port silently. + +**Never relaunch the user's app to get a port.** That destroys their session and +their state. Launch your own isolated instance instead (below). + +## Reading the DOM + +`apps/desktop/scripts/eval.mjs` is the one-liner: + +```bash +cd apps/desktop +node scripts/eval.mjs "document.querySelectorAll('[data-slot]').length" +``` + +For multi-step work use the shared client — it has target discovery and +promise-aware eval: + +```js +import { CDP, SELECTORS } from './scripts/perf/lib/cdp.mjs' + +const cdp = await CDP.connect({ port: 9222, match: '5174' }) +const out = await cdp.eval(`JSON.stringify({ + radius: getComputedStyle(document.documentElement).getPropertyValue('--radius-scalar').trim(), + composer: !!document.querySelector('[data-slot="composer-rich-input"]') +})`) +cdp.close() +``` + +`SELECTORS` in `scripts/perf/lib/cdp.mjs` holds the stable `data-slot` hooks +(composer, thread viewport, assistant message, turn pair, profile rail). Prefer +them over inventing a `querySelector` — they are updated as a unit when +components move. + +## The question this is best at: which rule won? + +Editing every call site because a style "isn't applying" is the classic waste. +Read the real node first: + +```js +const el = document.querySelector('[data-slot="aui_assistant-message-root"] a') +JSON.stringify({ + ownClasses: el.className, + weight: getComputedStyle(el).fontWeight, + parents: (() => { + const out = [] + let n = el + while ((n = n.parentElement) && out.length < 6) out.push(n.className) + return out + })() +}) +``` + +If the node carries no class of its own, the value is **inherited** — sweeping +call sites will not fix it, and you need the ancestor rule. A plugin stylesheet +(e.g. `@tailwindcss/typography`'s `prose a { font-weight: 500 }`) routinely beats +a utility class; override on the shared class, not at each usage. + +## Your own isolated instance + +When there is no port, or you must not disturb the user's window: + +```bash +cd apps/desktop +HERMES_HOME=/tmp/cdp-probe-home \ +HERMES_DESKTOP_DEV_SERVER=http://127.0.0.1:5174 \ +HERMES_DESKTOP_CDP_PORT=9333 \ + npx electron . --user-data-dir=/tmp/cdp-probe-userdata +``` + +The separate `--user-data-dir` dodges Electron's single-instance lock, so it +cannot collide with a running `hgui`; the separate `HERMES_HOME` keeps it away +from real sessions. Pick a port other than 9222 for the same reason. Run it in +the background and kill it when done. + +`npm run perf:serve` does the same with a temp `HERMES_HOME` baked in, if you +also want the perf harness. + +## Pitfalls + +- **Never kill the user's dev server or app to "free" anything.** A mid-serve + kill nukes Chromium's socket pool, and the resulting `ERR_NETWORK_CHANGED` + gets blamed on whatever you just changed. +- **A throwaway `HERMES_HOME` has no backend.** The app logs `ECONNREFUSED` for + `hermes:api` and may exit on its own. The renderer still mounts and the DOM is + readable — read promptly, and don't mistake a self-exited probe for a broken + port. Chromium logs `DevTools listening on ws://127.0.0.1:/…` when it + binds; that line is the proof the port opened. +- **Poll, don't probe once.** A just-launched app needs a second or two before + the port answers. +- **Never dump the whole DOM.** The desktop renders hundreds of nodes and + `outerHTML` will bury your context. Project down to a small JSON object inside + the evaluated expression. +- **Pass `match` to `CDP.connect`.** Without it you may attach to the pet + overlay, quick-entry window, or a devtools target instead of the main window. +- **`cdp.eval` returns the value; raw `Runtime.evaluate` double-nests it** + (`.result.result.value`). Use the wrapper. +- **`import.meta.env.DEV` is `true` under `vite dev`** in this repo. The note in + `apps/desktop/scripts/profile-typing-lag.md` claiming otherwise is stale. diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index ec99660fd78..948a118c97f 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` | Overrides the Chrome DevTools Protocol port the renderer exposes on `127.0.0.1` for DOM/CSS inspection tooling (default `9222`). Dev-server runs (`npm run dev`, `hgui`) open it automatically; a packaged app never does, and no value here changes that. Set to `off` to disable it on a dev run. Anything that can reach the port can execute code in the renderer. | ### Microsoft Graph (Teams Meetings)