mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
feat(desktop): on by default for dev-server runs
Gating this behind an opt-in was the wrong call. A dev server already executes arbitrary local JS — vite's module graph, every postinstall in node_modules — so a loopback debugging port does not meaningfully widen what a `npm run dev` session can already do, and `perf:serve` has opened one unconditionally all along. Requiring the variable also defeated the point: the tooling exists to be reached for mid-task, and a capability you must remember to enable before launching is one you don't have when you need it. So the port opens on 9222 — the same port scripts/eval.mjs and scripts/perf/lib/cdp.mjs already default to — for any dev-server run. HERMES_DESKTOP_CDP_PORT stops being an on-switch and becomes an override: a different port, or `off` to disable. The hard gate is unchanged and still checked first: a packaged build never opens the port, and no env value talks it into it. Neither does an unpackaged `electron .` against dist/, which is how the packaged app gets smoke tested. Refusals only log when they contradict something the developer asked for (a typo'd port, an explicit `off`). Packaged and dist runs are closed by design and stay quiet.
This commit is contained in:
parent
6254c568c8
commit
070093a318
5 changed files with 105 additions and 77 deletions
|
|
@ -8,73 +8,87 @@ import assert from 'node:assert/strict'
|
|||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { describeDevCdpDecision, resolveDevCdpPort } from './dev-cdp'
|
||||
import { DEFAULT_PORT, 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 }
|
||||
/** The ordinary `npm run dev` / `hgui` run. */
|
||||
const devRun = { env: {}, 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('a dev-server run opens the default port with no opt-in', () => {
|
||||
assert.deepEqual(resolveDevCdpPort(devRun), { port: DEFAULT_PORT, 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('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', () => {
|
||||
assert.deepEqual(resolveDevCdpPort({ ...opted, isPackaged: true }), { port: null, reason: 'packaged' })
|
||||
const decision = resolveDevCdpPort({ ...devRun, env: { HERMES_DESKTOP_CDP_PORT: '9222' }, isPackaged: true })
|
||||
|
||||
assert.deepEqual(decision, { 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
|
||||
})
|
||||
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)
|
||||
assert.equal(decision.reason, 'packaged')
|
||||
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({ ...opted, devServer: undefined }), { port: null, reason: 'no-dev-server' })
|
||||
assert.deepEqual(resolveDevCdpPort({ ...devRun, 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 } })
|
||||
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('tolerates surrounding whitespace on the requested port', () => {
|
||||
assert.equal(resolveDevCdpPort({ ...opted, env: { HERMES_DESKTOP_CDP_PORT: ' 9333 ' } }).port, 9333)
|
||||
})
|
||||
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 } })
|
||||
|
||||
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}`)
|
||||
assert.ok(describeDevCdpDecision(decision), `expected an explanation for ${JSON.stringify(value)}`)
|
||||
}
|
||||
})
|
||||
|
||||
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)
|
||||
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)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,21 +1,26 @@
|
|||
/**
|
||||
* Dev-only Chrome DevTools Protocol exposure for the desktop renderer.
|
||||
* 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. 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:
|
||||
* 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.
|
||||
*
|
||||
* 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.
|
||||
* 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
|
||||
|
|
@ -23,7 +28,7 @@
|
|||
*/
|
||||
|
||||
/** 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 ClosedReason = 'packaged' | 'no-dev-server' | 'opted-out' | 'invalid-port'
|
||||
|
||||
type DevCdpDecision = { port: number; reason: null } | { port: null; reason: ClosedReason }
|
||||
|
||||
|
|
@ -33,11 +38,15 @@ type DevCdpInput = {
|
|||
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.
|
||||
/** 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
|
||||
|
|
@ -50,17 +59,19 @@ function resolveDevCdpPort({ env, isPackaged, devServer }: DevCdpInput): DevCdpD
|
|||
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: null, reason: 'not-requested' }
|
||||
return { port: DEFAULT_PORT, reason: null }
|
||||
}
|
||||
|
||||
// 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' }
|
||||
if (OPT_OUT.has(requested.toLowerCase())) {
|
||||
return { port: null, reason: 'opted-out' }
|
||||
}
|
||||
|
||||
const port = Number(requested)
|
||||
|
|
@ -78,19 +89,20 @@ function describeDevCdpDecision(decision: DevCdpDecision): string | null {
|
|||
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':
|
||||
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 { DEFAULT_PORT, describeDevCdpDecision, resolveDevCdpPort }
|
||||
export type { DevCdpDecision }
|
||||
|
|
|
|||
|
|
@ -273,9 +273,10 @@ 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.
|
||||
// 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) {
|
||||
|
|
@ -284,8 +285,8 @@ if (DEV_CDP.port) {
|
|||
// 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.'
|
||||
`[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)
|
||||
|
|
|
|||
|
|
@ -13,7 +13,8 @@ try {
|
|||
} 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.'
|
||||
'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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -530,7 +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. |
|
||||
| `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)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue