Skip to content

feat: Bot Screen — per-bot Xfce desktop streamed into Hermes Desktop, take over and hand back (related #92524) - #108914

Merged
teknium1 merged 147 commits into
mainfrom
hermes/hermes-b802e898
Sep 23, 2026
Merged

teknium1 merged 147 commits into
mainfrom
hermes/hermes-b802e898

Conversation

@teknium1

@teknium1 teknium1 commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator

A bot on a headless Linux gateway gets its own Xfce desktop that Hermes Desktop streams live; you can take over to log in / solve 2FA, then hand the screen back and the bot continues with your session.

Related: #92524 (the "let me open the bot's screen, log in myself and hand it back while the agent keeps running after my PC is off" request; this PR is the Desktop + Linux-host half — a bot's own screen, takeover, hand-back and a shared browser profile. The hosted/cloud-browser leg and dashboard integration are not in it, so it does not close the issue). Also related: #108592, #97859 (earlier Bot Desktop direction), #17258 (Docker Xfce), #90380 (CUA backend seam), #90374 (agent acting on the human's seat).

What changed

  • tools/bot_desktop/ — launcher.sh starts TigerVNC Xvnc (Unix socket, 0600, -SecurityTypes None, no TCP) plus xfwm4 / xfce4-panel / xfdesktop / xfsettingsd under a private dbus-run-session, one per profile (<HERMES_HOME>/bot-desktop/). runtime.py = start/stop/status, free display allocation, published DISPLAY/XAUTHORITY/DBUS env. lease.py = who drives the screen (agent | one human viewer), per profile. rfb_filter.py = stateful RFB client-stream parser that drops KeyEvent / PointerEvent / ClientCutText / QEMU Extended KeyEvent from viewers that do not hold the lease (server-side view-only, not noVNC's viewOnly hint).
  • hermes_cli/web_routers/display.py — /api/display/ws: raw RFB over WebSocket, authenticated with a one-shot 30 s ticket minted over the already-authenticated /api/ws; lease flip closes evicted viewers with 4000 control-taken.
  • tui_gateway/methods_display.py — display.status|start|stop|observe|lease.acquire|lease.release + display.lease broadcast event.
  • tools/computer_use — every action (capture included) is refused with code: human_has_control while a human holds the lease (takeover is always human-initiated: when the bot hits a login/2FA/CAPTCHA it says so in its reply and ends its turn; the person takes over from the pane, does the step, hands back and tells it to continue; there are no agent-side handoff actions); cua-driver and headed Chromium inherit the bot's display so the agent never acts on a seat the human is sitting at. Auto-start is opt-in (bot_desktop.auto_start, default off): the Screen pane's Start button is the normal path; with the flag on, a headless host with the packages present starts the screen at the tool boundary on first use.
  • Hermes Desktop — three ways to a bot's computer: the bot's screen as the hero at the top of its Scheduled Jobs pane — a big live picture of the desktop (display.thumbnail, one JPEG grab every 4 s while visible; read-only, never touches the lease) above the title and the routines, click to expand into live access — a compact Screen row under each gateway/profile header in the Sessions sidebar (new sidebar.gatewayGroup.header contribution area; the plugin owns the box, core only exposes the slot), and Bots → right-click → Open Screen. The box shows Live · bot in control / Stopped / Not installed on host from the same display.status + lease events as the pane. The pane itself: noVNC (@novnc/novnc) pane with chip (Bot is in control / You are in control / Another viewer is in control), Take over / Hand back, red border while you drive; closing the pane hands control back. data-terminal on the canvas host so the app's type-to-focus shortcut can't steal keystrokes.
  • Install from Desktop — when the packages are missing the pane is an install card: Install on host calls display.install, which runs the distro package command (apt/dnf/pacman) on the gateway host as a supervised child and streams display.install.log / display.install.done. If sudo needs a password the host raises display.install.sudo.request on the caller's own WebSocket and the renderer shows the existing masked SudoDialog (answering on display.install.sudo.respond, redacted from the gateway trace like sudo.respond); an empty answer cancels without spawning the package manager; one install per profile at a time. Nothing installs on hermes update; the only triggers are this button and the CLI.
  • Look — tools/bot_desktop/wallpaper.png (Nous gradient) seeded as the backdrop, first dark GTK/icon theme the host ships, dark translucent panels; own panel layout with a dock of only launchers whose program exists (browser pinned to the one the bot drives, so a human who takes over lands in the bot's browser profile). Users install whatever they like on the host; it lands in the Applications menu.
  • CLI — hermes computer-use screen status|start|stop|install (apt/dnf/pacman package lists, deliberately not the xfce4 metapackage: no screensaver / power manager / polkit agent on a headless desktop).
  • Docs — website/docs/user-guide/features/bot-screen.md, registered in sidebars.ts.

Live evidence (Linux host, hermes serve headless + Hermes Desktop via CDP, real Xvnc/Xfce)

Step Before After
computer_use capture on a headless profile (no DISPLAY) 0x0, "no DISPLAY is set" screen auto-starts in 5.5 s, capture 1440x900
Desktop → Bots → Open Screen n/a noVNC canvas 1440x900 streaming the Xfce panel + hermes-bot-terminal
Take over → type echo HUMAN-TYPED-VIA-HERMES-DESKTOP ⏎ keystrokes went to the chat composer agent-side capture of the X screen shows the command and its output in the bot's terminal; composer unchanged
Second viewer acquires the lease n/a first pane flips to Another viewer is in control, its RFB socket closed 4000 control-taken, human lease second-laptop
Hand back n/a chip → Bot is in control, lease → agent, computer_use capture works again
Non-holder sends KeyEvent over RFB n/a dropped by the bridge; holder's key reaches Xvnc (unit + live)
Scheduled Jobs pane, packages present n/a hero reads Screen is off; click → Screen pane → Start screen → live stream; hero shows the live Xfce desktop picture, Live · bot in control; a window opened on the bot's display appears in the hero on the next refresh; clicking the picture opens the live pane (canvas + Take over)
Sessions sidebar, grouped by profile n/a Screen box under the default profile header, same state, click opens the pane
Packages hidden from the host, click Install on host CLI printed a sudo line to copy masked Administrator password card in Desktop → Cancel → pane shows Install cancelled: no sudo password was provided. and returns to the install card; display.install.log carries the same line; a second click while one runs is refused

Two bugs found and fixed only by the live pass: noVNC never received open when the socket was dialed before its dynamic import finished (pane stuck on "Connecting…"), and the RFB filter rejected message type 255 (QEMU Extended KeyEvent), which noVNC switches to as soon as Xvnc advertises the pseudo-encoding, so the first keypress closed the stream.

Validation

  • tests/tools/test_bot_desktop_install.py (empty sudo answer cancels without spawning; second install per profile refused, slot released), tests/tools/test_bot_desktop_lease.py (RFB filter across byte-by-byte chunking incl. QEMU key; tool refusal while human holds), tests/hermes_cli/test_display_ws_ticket.py; apps/desktop/src/lib/sibling-ws-url.test.ts.
  • tests/tools, tests/tui_gateway, tests/hermes_cli via scripts/run_tests.sh: only the 6 failures that also fail on pristine origin/main on this host (modal/parallel SDK, sort payload, update live-system guard).
  • Desktop: tsc --noEmit clean, eslint clean, vitest 969 files / 9769 tests passed.
  • ruff, check-windows-footguns --all, check_compat_pointers, check_subprocess_stdin: clean.
  • tests/tools/conftest.py pins Bot Desktop binaries to "missing" so a dev host with TigerVNC installed never launches real X servers from the suite (same class as the browser-use fixture).

Independent review round (reproduced → fixed → re-verified live)

An independent review of d9525b3 found five P1s and a P2; every one was reproduced, fixed in 3c70635, covered by an invariant test proven red without the fix, and re-verified on a real Xvnc/Xfce screen.

Finding Fix Live proof
Lease was a per-process dict: a takeover in hermes serve did not stop a gateway/CLI process driving the same display lease.json under an fcntl lock in the profile's bot-desktop/; every read hits the file; epoch per transition process A acquire → process B computer_use capture = human_has_control, no png; process C release → B captures again
Takeover during approval / backend start-up did not fence an already-admitted action re-check under the dispatch lock; a result produced after the lease epoch changed is discarded test: _dispatch that flips the lease mid-flight → result dropped, SECRET never returned
Install sudo reply went through the foreground $gateway (host B could receive host A's password) SudoRequest.origin = (connection, profile) of the request; SudoDialog answers via requestGatewayForAgent on that socket; sudo.expire / display.install.sudo.expire tear the card down tsc + 1185 vitest green
Dock Browser launched plain google-chrome → a different user-data-dir than the bot's tools/bot_desktop/browser.py: one identity (agent-browser's Chromium + bot-desktop/browser-profile) applied to BOTH the agent env (AGENT_BROWSER_EXECUTABLE_PATH/AGENT_BROWSER_PROFILE) and the dock launcher bot wrote localStorage on 127.0.0.1:8765 via agent-browser; dock click + typed URL on the screen showed BOT-WROTE-THIS in the same profile (screenshot below)
Restart reused a recorded display number and unlinked another profile's X lock/socket host-wide alloc lock; recorded number reused only if no live pid holds it; launcher never unlinks a live lock A stopped, B took :20, A restarted on :21, B kept running
Install worker ran under the default profile for a named-profile request copy_context() carries the HERMES_HOME override + transport into the thread test red without / green with

Scope honesty from the same round: Closes → Related (hosted browser leg not here), auto_start default off, request_handoff no longer claims a Telegram/Discord message was sent (the model relays the ask in its reply).

Shared browser profile: the bot wrote BOT-WROTE-THIS into localStorage through agent-browser; the dock's Browser icon, clicked on the screen, opens the same Chromium + user-data-dir and the page reads it back

Second round (@Julientalbot, on 3c70635) — 3 P1 + 2 P2 + 2 design notes, all reproduced, fixed in 7a43ce8 / d947fc8

Finding Fix Live proof
Browser tools bypassed the lease: the bot could read/act in the shared browser the human was logging into _run_browser_command brackets every local agent-browser command with the lease (refuse + epoch fence); cloud/user-CDP sessions untouched; code: human_has_control on the tool result human held from another process → browser_click and browser_snapshot refused; navigate worked after hand-back
Capture spanning a full takeover→hand-back cycle leaked epoch-only fence, human_holds() dropped test runs acquire+release inside the dispatch → result discarded
Abnormal viewer disconnect (1006) released the lease; agent resumed mid-login only 1000/1001 release; a dropped link keeps the human's exclusion; bridge lifted into _bridge() against hermes serve: transport.abort() → holder stays human; close(1000) → agent
Hero kept saying "Live" after refreshes failed 3 misses → frame dimmed + "Last seen — screen unreachable" (4 locales) vitest
Lease/install events matched by profile path only (two hosts, same ~/.hermes) isEventForBotScreen: connectionId AND profile key, one predicate for all three listeners screen-connection.test.ts
Lease reader defaulted to agent on a corrupt file present-but-unparsable → HUMAN holds (fail closed); missing → fresh agent profile test
Handoff reason vanished once the human took over acquire keeps pending_handoff as reason; pane shows it while held test

Third round (community, on d947fc8: 10 reviews, 15 comments, 3 inline threads, sibling PR #109446) — ~40 distinct findings, every legitimate one fixed in d947fc8..d65af42

Reviewers: @Julientalbot @BearHuddleston @carlotestor @iowahawkeyedave @iamlukethedev @Xipong @erosika @rahlquist @helix4u @MrD1az @Ganaderiapp @eynaudg @lEWFkRAD @dresraz @whyyagswhy @zfifteen @coe0718 @thomasbek3. Four commits by @whyyagswhy (PR #109446) are cherry-picked with authorship.

Area What was wrong Fix
Windows (P1) lease.py imported fcntl at module level; every computer_use call, display.status, screen status broke on native Windows lazy/optional fcntl (no-op lock where absent, file semantics kept); check-windows-footguns.py now flags module-level POSIX-only imports
Browser fence (P1) real-profile local Chrome (loopback cdp_url) escaped the lease fence; a dead Xvnc with a stranded human lease unfenced it fence by provenance (features.local), armed by live DISPLAY or human lease
Capture fence (P1) frame persisted + routed to aux vision before the epoch check fence right after backend.capture(), before persist/spill/vision; also capture_after
Thumbnail (P1) display.thumbnail kept grabbing while a human held refused with suppressed: human_has_control; hero captions "Hidden while someone has control"
Cross-process events (P1) request_handoff/takeover from the gateway or CLI process never reached the Desktop (on_change is in-process) hermes serve watches each served profile's lease.json (0.5 s mtime poll, epoch-deduped) and broadcasts display.lease
Viewer identity (P1) viewer_id client-chosen and disclosed via display.status: any client could co-drive or release(None) the holder server-minted ids (display.observe returns it; reuse only on the minting connection), snapshots/events carry viewer_hash never the id; bare release refused unless force
Display ticket (P1) a bot-desktop ticket logged in on /api/ws _ws_auth_reason rejects that provider
Fedora (P1 there) retired xorg-x11-server-utils/xorg-x11-utils → dnf5 aborts the whole install per-binary packages; BINARY_PACKAGES map + invariant test that every required binary maps into every distro list; apt gains x11-xkb-utils, pacman dbus xorg-xprop
Pane close codeless close (1005) never handed back close(1000) on unmount only (@whyyagswhy); 1005/1006 keep the exclusion (documented contract test)
CLI stop did not release the lease despite --help releases first (@whyyagswhy)
Portal / legacy group / hero portal matched profile path only; null-connection group matched a remote namesake; hero kept the previous bot's pixels isEventForBotScreen everywhere; ?? 'local'; hero keyed by bot (@whyyagswhy)
RFB unbounded ClientCutText length (2 GiB pin from a watcher) 256 KiB header cap → protocol close (@whyyagswhy, wrapper trimmed)
Runtime alloc lock released before Xvnc claimed :N; no per-profile start lock; pid recycled → wrong killpg; exec dbus-run-session dropped the Xvnc EXIT trap (orphan X server); log never rotated lock held across spawn+publish; start.lock; pid + create_time; launcher stays supervisor; truncate per start
Install timeout killed sudo not apt; non-atomic single-flight; CLI installer bypassed the lock with shell=True killpg; claim() before the thread; CLI routed through install_packages
computer_use cached cua backend never rebound when the screen started later; wait_for_human ran the full timeout when nobody answered display identity in the cache key; no_takeover after grace (default 60 s)
Lease reader []/null/unknown holder read as agent or raised fail closed
Bridge ex-holder kicked on the next takeover; lease.json read per input message clear on hand-back; cached decision refreshed ≤250 ms
Desktop pooled socket disposed before install/lease events; control-taken overlay unreachable; ⌘W on the canvas closed a terminal tab; sidebar row remounted every paint; older backends spun forever; older display.status rolled back a newer lease; no way to reclaim after a reload retainProfile across install/attach; close code 4000; data-remote-screen swallows ⌘W; stable render identity; -32601 → settled; epoch ordering; Hand back (force)
Dock browser human-opened dock Chromium stranded agent-browser on the profile singleton dock launches with --remote-debugging-port=0; agent attaches via --cdp to the live instance (both launch orders live-proven)
Clipboard holder's clipboard reached watchers via ServerCutText -SendCutText=0 on Xvnc
Docs "the bot never sees what you type" overreach; missing threat model; lease-file semantics; pane-close contract; recovery text rewritten; same-UID threat model stated plainly

Rulings (not changed, by design): no lease TTL/heartbeat (a vanished viewer is recovered by an explicit Hand back (force), never by the agent resuming on its own); takeover does not wait for in-flight input; SetEncodings needs no cap (16-bit count); Nous Cloud gets this with the next server release.

Live, integrated head, real Xvnc + hermes serve + headless Hermes Desktop over CDP: client-chosen viewer id ignored and a minted one returned; acquire → viewer_id: null + matching viewer_hash; display.status discloses no id; thumbnail suppressed while human holds; bare release refused (viewer_mismatch), forced release works; display ticket on /api/ws → HTTP 403; a request_handoff from a separate process arrived as display.lease in 0.24 s; in the Desktop: Take over / Hand back round-trip, reload hands back (1000), a takeover made by another process repainted the pane with its reason and offered Hand back (force), which returned control.

Human holds: red ring, "You are in control", the hero now says "Hidden while someone has control" instead of streaming their screen.

Another window took over (reason shown); this window offers "Hand back (force)".

Round 6 (independent review + design change)

  • Agent-side handoff removed. request_handoff / wait_for_human are gone from computer_use, along with the lease's pending_handoff, the "Bot needs you" badge and the per-host schema rewriter. They blocked a tool call waiting for a human who, off the Desktop pane, was never watching, and fought the 420 s tool deadline. Takeover is human-initiated only.
  • Vault tools honour the lease (P1): browser_vault_fill / enter_code / save_login reach the page over the supervisor socket and skipped the fence; they now run under the same run_fenced admission + epoch check.
  • --clone-all strips the source's screen identity (P1): launcher.pid, env, rfb.sock, lease. A clone no longer believes it owns the source's X server (screen stop on the clone killed the source's desktop).
  • Reconnecting pane re-presents its minted viewer id, so the human's lease survives blips / 4000 evictions / Reconnect; setScreenLease records a newer epoch even when nothing visible changed.
  • Refusing a bare display.stop / display.lease.release under a human is decided inside the lease transition (a racing takeover can no longer be acknowledged and silently revoked).
  • Process-group cleanup (runtime + installer) waits for the whole group, not the leader; installer also reaps on a raising output sink.
  • Install password card is app-level: survives a chat switch, expiry finds it.
  • Dock Browser entry follows the profile path after hermes profile rename without reseeding the panel layout.

Not in this PR

The hosted/cloud-browser leg of #92524 and its dashboard integration; an agent-initiated "please take over" signal (removed in round 6: it only made sense with a person watching the Desktop pane; the bot asks in its reply instead); per-bot OS users (currently per-profile displays + XDG dirs under one user), Wayland compositors, macOS/Windows hosts (they have one real seat; display.status reports supported: false and the pane says so).

Screenshots (Hermes Desktop, live against hermes serve on this Linux host)

Getting to a bot's computer

Bots → Hermes → Scheduled Jobs: the bot's screen is the hero at the very top of the pane, above the title and the routines (screen off, chip offers Start)

Screen running: the hero is a live picture of the bot's desktop (Nous wallpaper, dark theme, Chrome + terminal opened from the dock), refreshed every few seconds; caption Live · bot in control, chip Open live

Clicking the picture expands into the live Screen pane (noVNC canvas, Take over)

Sessions sidebar grouped by profile: a compact Screen row under the default profile header, so the profile's computer is reachable from its conversations

The bot's desktop itself (what display.thumbnail returns): Nous gradient wallpaper, dark top bar with task list + clock, bottom dock of only the programs present on the host (Terminal, Browser pinned to the bot's Chrome); anything the user installs shows in the Applications menu

Installing the screen packages from the Desktop side

Host without TigerVNC/Xfce: the pane becomes an install card with the exact package command and Install on host

Install on host → the gateway host asks for its sudo password through the existing masked Administrator password card (sent to that host only, redacted from the trace)

Cancel → Install cancelled: no sudo password was provided; nothing was spawned, the card returns

Take over and hand back

Watching: Bot is in control, no border, keyboard and mouse ignored by the bridge

Take over: red border, chip You are in control, human typing into the bot's terminal on the Xfce desktop

The typed command executed on the bot's screen (echo HUMAN-TYPED-VIA-HERMES-DESKTOP); the chat composer stayed untouched

A second viewer took the lease: this pane shows Another viewer is in control and its RFB socket was closed with 4000 control-taken

Hand back: chip returns to Bot is in control, lease → agent, computer_use captures work again

Infographic

Bot Screen

@teknium1
teknium1 requested a review from a team September 12, 2026 07:21
@github-actions

github-actions Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

૮ >ﻌ< ა ci review

ran on 92e2e7c — Merge remote-tracking branch 'origin/main' into hermes/herme

⚠️ Action required

package-lock.json · View job

Locked npm dependency versions changed.

package-lock.json

Package Before After
➕ @novnc/novnc — 1.7.0

How to fix:

Add the ci-reviewed label after verifying the version changes are expected.


debug info

CI timings

CI timings · View report · View job

Wall time 4m49s vs 5m54s (-18.4%). 4 job(s) slower, 12 faster, 1 unchanged.

  • Python tests / Run tests: -86.0s
  • Python lints / Windows footguns (blocking): +48.0s
  • OS-specific tests / Windows-only tests: -41.0s
  • Docs Site / docs-site-checks: +20.0s
  • OS-specific tests / macOS-only tests: -15.0s

@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/tools Tool registry, model_tools, toolsets tool/browser Browser automation (CDP, Playwright) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Sep 12, 2026
@teknium1
teknium1 force-pushed the hermes/hermes-b802e898 branch 2 times, most recently from d9525b3 to 3c70635 Compare September 12, 2026 16:14

@Julientalbot Julientalbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing this from cognitive ergonomics and human–agent interaction, with AI-assisted source inspection and controlled regression probes, on cc0814d831b61f40183228da2fa2517487e018e3.

The useful advance here is continuity: a person can intervene in the agent's actual environment, then let it continue with the resulting browser session. Watch by default, named Take over / Hand back actions, visible ownership and server-side viewer-input filtering are valuable foundations.

Recommendation: address the control guarantees before approval. When someone takes over to enter credentials or complete 2FA, the product must prevent the agent from both acting on and observing the reserved surface. The attached comments identify five reproduced cases: browser dispatch under human control; a capture result crossing takeover and hand-back; abnormal disconnection releasing control; a stale preview still labelled Live; and control state applied to the wrong host.

The interaction contract I would look for is:

  • Takeover: request → confirmed restriction of relevant agent access, with in-flight operations handled → human input enabled. Enforce this outside the model.
  • Connection loss: keep agent access blocked and provide a recoverable interrupted state. A deadline may trigger recovery or escalation; elapsed time must not grant access. A watchdog is a possible implementation choice, not the requirement itself. Closing noVNC or rebooting does not establish that secrets have been erased.
  • Hand-back: explicit transfer of control, followed by checking the resulting state. It does not certify successful authentication or completed work.
  • Status: distinguish connection health, confirmed controller and task activity. “Agent can act” does not establish “agent has resumed.”

This builds on my published criteria for exclusive control and real delegation and keeping work state outside the user's working memory. The FAA's positive exchange of controls is a useful coordination analogy: an explicit, acknowledged transfer. Confidentiality is an additional requirement here.

Two design follow-ups: retain the handoff reason while the person acts—it is currently in a hover tooltip and cleared on acquisition—and make recovery preserve the access restriction across restarts. In particular, the lease reader defaults to agent ownership on missing/invalid state; legitimate initialization should be distinguished from loss of authority state during an active session. That last point is source inspection, not a sixth executed probe.

The documented shared OS account is not a security boundary. Fixing browser coordination alone cannot establish confidentiality against other agent-accessible routes such as terminal, files or child processes. The protection claim should match the enforced boundary. This does not make hosted-browser/dashboard support or per-bot OS users requirements of this PR; those are explicitly outside its scope.

Evidence and limits: the five probes are included under the inline comments. They exercise real handlers/components with doubles at stated execution or transport boundaries. No live Linux Xfce/noVNC trial or user study was conducted. The existing targeted Python suite had 12 passes and two Linux-only skips; the existing WebSocket URL tests had two passes. Installed local dependencies were reused. Predicted confusion and recovery effort remain hypotheses for live trials with the intended users.

Comment thread tools/browser_tool.py Outdated
Comment on lines +45 to +47
# Headed Chromium opens on this profile's Bot Desktop when one is running (human can take it over).
from tools.bot_desktop.runtime import desktop_env as _bot_desktop_env
return _bot_desktop_env(env)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Apply human control to the shared browser's actions and observations.

A person may take over to enter credentials while the bot still uses its ordinary browser tools. This change binds those tools to the same desktop and persistent browser profile, but their execution path does not check the human lease applied to computer_use. “You are in control” therefore does not establish the expected exclusion.

A probe through the actual browser_click handler, real environment construction and a persisted human lease dispatched agent-browser --session review --headed --engine chrome --json click @e1 and returned success. Only the final process execution and setup boundaries were stubbed; no real browser click was performed.

Please enforce the lease at the execution boundary for actions and reads of this shared local browser, and account for operations already in progress before confirming takeover. Keep unrelated remote browser sessions appropriately scoped. Acceptance: the shared browser cannot change or expose the human intervention through these agent tools while the human holds control.

Controlled regression probe — expected to fail on this head

Save the following review-only test as tests/tools/test_review_108914_browser_control.py in a disposable checkout. The test file is not part of this PR. Run scripts/run_tests.sh tests/tools/test_review_108914_browser_control.py -j 1 --file-retries 0.

import json
from tools.bot_desktop import lease, runtime


def test_browser_click_is_fenced_while_human_controls_shared_browser(monkeypatch):
    from tools import browser_tool as browser
    from tools import browser_tool_session as session

    monkeypatch.setattr(runtime, "published_env", lambda: {"DISPLAY": ":37"})
    monkeypatch.delenv("AGENT_BROWSER_PROFILE", raising=False)
    assert browser._build_browser_env()["AGENT_BROWSER_PROFILE"].endswith("bot-desktop/browser-profile")
    monkeypatch.setattr(browser, "_is_camofox_mode", lambda: False)
    monkeypatch.setattr(browser, "_blocked_private_page_action", lambda *a: None)
    monkeypatch.setattr(session, "_browser_command_preflight", lambda: {"browser_cmd": "agent-browser"})
    monkeypatch.setattr(session, "_get_session_info", lambda *a: {"session_name": "review"})
    monkeypatch.setattr(session._cloud, "_get_browser_engine", lambda: "chrome")
    monkeypatch.setattr(session._cloud, "_is_headed_mode", lambda: True)
    commands = []

    def spawn(*args):
        commands.append(args[2])
        return {"success": True, "data": {}}

    monkeypatch.setattr(session, "_spawn_and_collect", spawn)
    lease.acquire("human-viewer")
    result = json.loads(browser.browser_click("e1", task_id="review"))
    assert commands == [], f"Human holds the lease, but the browser command was dispatched: {commands}; result={result}"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7a43ce8b57fe. _run_browser_command now brackets every local agent-browser command (no CDP url) with the lease while this profile's screen is running: refused up front with code: human_has_control, and a result whose run crossed a lease epoch change is discarded. Cloud/user-CDP sessions are untouched. Live: with a human holding the lease from another process, browser_click and browser_snapshot both returned human_has_control; navigate worked again after hand-back. Test: tests/tools/test_bot_desktop_browser_fence.py (your probe, red on the previous head).

Comment thread tools/computer_use/tool.py Outdated
except _bd_lease.HumanHasControl as e:
return _refused(e)
result = _dispatch(backend, action, args)
if _bd_lease.get().epoch != admitted.epoch and _bd_lease.human_holds():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Discard results across a completed takeover / hand-back cycle.

A capture admitted before takeover can produce a result during the human intervention and return after the human has already handed back. This condition only discards the result if the human still holds control, so a completed acquire → release cycle escapes the fence.

A controlled _dispatch seam performed acquire → synthetic capture result → release. The actual handle_computer_use returned the HUMAN_PRIVATE_FRAME marker. This proves the result-fencing gap without taking a real screenshot.

Please invalidate results crossing a relevant ownership epoch regardless of the final holder, with the admission epoch sampled at the execution boundary. Returning control authorizes subsequent observation; it should not authorize delivery of captures from the reserved human period.

Controlled regression probe — expected to fail on this head

Save the following review-only test as tests/tools/test_review_108914_capture_epoch.py in a disposable checkout. The test file is not part of this PR. Run scripts/run_tests.sh tests/tools/test_review_108914_capture_epoch.py -j 1 --file-retries 0.

import json
from tools.bot_desktop import lease, runtime


def test_capture_crossing_takeover_and_handback_discards_human_frame(monkeypatch):
    from tools.computer_use import tool

    monkeypatch.setattr(tool, "_get_backend", lambda session_id="": object())

    def capture_during_handoff(backend, action, args):
        lease.acquire("human-viewer")
        captured = json.dumps({"ok": True, "action": action, "png_b64": "HUMAN_PRIVATE_FRAME"})
        lease.release("human-viewer")
        return captured

    monkeypatch.setattr(tool, "_dispatch", capture_during_handoff)
    result = tool.handle_computer_use({"action": "capture"})
    assert "HUMAN_PRIVATE_FRAME" not in result, result

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7a43ce8b57fe: epoch-only comparison, human_holds() dropped. test_takeover_during_an_admitted_action_discards_its_result now runs the full acquire+release cycle inside the dispatch and was red before the change.

Comment thread hermes_cli/web_routers/display.py Outdated
Comment on lines +139 to +141
# Closing the viewer window hands control back; a stale holder never pins the agent out.
if _lease.viewer_may_send_input(viewer_id, profile_key=profile_home):
_lease.release(viewer_id, profile_key=profile_home)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not treat an abnormal disconnect as intentional hand-back.

The person may lose their connection halfway through entering credentials. This cleanup releases their lease regardless of the reason the stream ended. In the controlled endpoint probe below, a websocket.disconnect with code 1006 moved ownership to the agent and made wait_for_human return ok: true, without a Hand back action. Ticket consumption and the persisted lease were real; transport and the admission check were test doubles.

The tool does not literally claim the login succeeded. The problem is that connection loss grants renewed agent access to an unfinished human intervention. Closing the pane to hand back is documented, and avoiding a stranded lease is reasonable, but involuntary loss needs a distinct outcome.

Please preserve agent exclusion on connection loss and provide explicit recovery or release. A timeout can escalate recovery, but should not grant access. Acceptance: dropping the connection during a test login does not authorize new agent reads/actions; a reauthenticated person can recover and deliberately return control.

Controlled regression probe — expected to fail on this head

Save the following review-only test as tests/hermes_cli/test_review_108914_disconnect.py in a disposable checkout. The test file is not part of this PR. Run scripts/run_tests.sh tests/hermes_cli/test_review_108914_disconnect.py -j 1 --file-retries 0.

"""Review probe: connection loss must not imply an intentional hand-back.

Real endpoint, ticket consumption and persisted lease; only auth / transport are
test doubles. This is not a live noVNC or Linux desktop test.
"""

import asyncio
import json

from hermes_constants import get_hermes_home
from hermes_cli.dashboard_auth import ws_tickets
from hermes_cli.web_routers import display
from tools.bot_desktop import lease
from tools.computer_use.handoff import handle_handoff


def test_abnormal_viewer_disconnect_does_not_authorize_agent_to_resume(monkeypatch):
    profile_home = get_hermes_home()
    socket_path = profile_home / "bot-desktop" / "rfb.sock"
    socket_path.parent.mkdir(parents=True, exist_ok=True)
    socket_path.touch()
    ws_tickets._reset_for_tests()
    ticket = ws_tickets.mint_ticket(
        user_id="display:review-human", provider="bot-desktop",
        extra={"hermes_home": str(profile_home), "viewer_id": "review-human"},
    )
    lease.request_handoff("Finish login and 2FA")
    lease.acquire("review-human")

    class Viewer:
        query_params = {"display_ticket": ticket}

        async def accept(self):
            pass

        async def receive(self):
            return {"type": "websocket.disconnect", "code": 1006}

        async def close(self, **kwargs):
            pass

    class DesktopReader:
        async def read(self, count):
            await asyncio.Event().wait()

    class DesktopWriter:
        def close(self):
            pass

    async def connect(path):
        assert path == str(socket_path)
        return DesktopReader(), DesktopWriter()

    monkeypatch.setattr(display, "_ws_request_is_allowed", lambda ws: True)
    monkeypatch.setattr(display.asyncio, "open_unix_connection", connect)
    asyncio.run(display.display_ws(Viewer()))
    result = json.loads(handle_handoff("wait_for_human", {"seconds": 1}))
    assert not result["ok"], f"No Hand back action occurred; abnormal disconnect produced: {result}"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7a43ce8b57fe. The bridge records the close code from websocket.disconnect; only 1000/1001 (viewer closed the window) release the lease. Anything else keeps the human's exclusion; the Desktop reconnects into the same lease or the human presses Hand back. Live against hermes serve: transport.abort() (1006) → holder stays human; close(1000) → agent. Test: tests/hermes_cli/test_display_ws_drop_keeps_lease.py (parametrised 1006/1000; the bridge body was lifted into _bridge(ws, info) so it is testable).

Comment on lines +64 to +66
.catch(() => {
/* transient: the next tick retries; the last good frame stays up */
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Mark a retained frame as stale when refreshes fail.

A returning user may rely on “Live · bot in control” when deciding whether to keep watching. This catch retains the last image while the caption still derives from cached runtime/lease state. There is no failed-refresh or frame-age state to qualify “Live.”

The real hero component and portal-state hook were rendered with one successful thumbnail followed by rejected requests. After 60 seconds of simulated time, the button still said Screen: Live · bot in control. This was a component probe, not a real network outage; 60 seconds demonstrates persistence, not a proposed universal freshness threshold.

Please retain useful visual context but show when it was last received/captured, mark failed or stale updates, and reconcile status on recovery. Do not infer that the agent stopped merely because the viewing connection failed. Acceptance: repeated refresh failures cannot leave an old frame presented as unqualified live evidence.

Controlled regression probe — expected to fail on this head

Save the following review-only test as apps/desktop/src/plugins/hermes-bots/screen-review-108914-freshness.test.tsx in a disposable checkout. The test file is not part of this PR. Run cd apps/desktop && ../../node_modules/.bin/vitest run src/plugins/hermes-bots/screen-review-108914-freshness.test.tsx.

import { act, render } from '@testing-library/react'
import { expect, it, vi } from 'vitest'

vi.mock('@hermes/plugin-sdk', async () => {
  const { useStore } = await import('@nanostores/react')
  const { onGatewayEvent } = await import('../../contrib/events')
  return { Codicon: () => null, useValue: useStore, host: { onEvent: onGatewayEvent } }
})
vi.mock('./data', async () => {
  const { atom } = await import('nanostores')
  return { $lastRoster: atom([]), botSelectionKey: (bot: { connectionId: string; name: string }) => `${bot.connectionId}:${bot.name}` }
})
vi.mock('./i18n', () => ({ useBots: () => ({ screen: {
  portalTitle: 'Screen', portalWatching: 'Live · bot in control',
  heroOpenLive: 'Open live', heroConnecting: 'Connecting',
} }) }))
vi.mock('./routing', () => ({ resolveBotConnectionRoute: () => ({ route: null }) }))
vi.mock('./screen-connection', () => ({ VIEWER_ID: 'review', displayRequest: vi.fn() }))
vi.mock('./screen-open', () => ({ openBotScreen: vi.fn() }))

import { ScreenHero } from './screen-hero'
import { displayRequest, type DisplayStatus } from './screen-connection'
import { $screenState, setScreenStatus } from './screen-state'
import type { RosterRow } from './types'

it('does not present a retained frame as live after repeated transport failures', async () => {
  vi.useFakeTimers()
  vi.spyOn(document, 'hidden', 'get').mockReturnValue(false)
  const bot = { name: 'default', connectionId: 'host-a', sourceScoped: true } as RosterRow
  $screenState.set({})
  setScreenStatus(bot, {
    profile: 'default', profile_key: '/home/hermes/.hermes', supported: true, installed: true, running: true,
    lease: { holder: 'agent', viewer_id: null, pending_handoff: null, since: 1, reason: '' },
  } as DisplayStatus)
  const request = vi.mocked(displayRequest)
  request.mockReset()
  request.mockResolvedValueOnce({ data_url: 'data:image/jpeg;base64,LAST_GOOD_FRAME' })
    .mockRejectedValue(new Error('Gateway unavailable'))
  const view = render(<ScreenHero bot={bot} />)
  try {
    await act(async () => {})
    expect(view.container.querySelector('img')?.getAttribute('src')).toContain('LAST_GOOD_FRAME')
    expect(view.getByRole('button').getAttribute('aria-label')).toContain('Live')
    // Fifteen consecutive failed refreshes: initial success cannot justify current "Live".
    await act(async () => { await vi.advanceTimersByTimeAsync(60_000) })
    expect(request.mock.calls.length).toBeGreaterThan(2)
    expect(view.getByRole('button').getAttribute('aria-label')).not.toContain('Live')
  } finally {
    view.unmount()
    vi.restoreAllMocks()
    vi.useRealTimers()
  }
})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d947fc8: three consecutive failed refreshes dim the retained frame (grayscale, 40%) and caption it "Last seen — screen unreachable"; a success resets. Localised in all four locales.

Comment on lines +105 to +106
if (payload?.lease && payload.profile_key && payload.profile_key === profileKey) {
setScreenLease(bot, payload.lease)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Match the connection identity as well as the profile path.

Two hosts can both use /home/hermes/.hermes. The shared gateway event stream carries connectionId, but this listener matches only the path. A takeover event from host B can therefore replace host A's cached agent lease with B's human lease. Since VIEWER_ID is per desktop window, A can appear controlled by this window although its backend still permits its agent to act.

The probe below uses the actual event bus, useScreenPortalState and screen store: seed A as agent-controlled, emit B's human lease at the same path, and A's holder becomes human. It establishes false displayed ownership, not mixed video, transferred cookies or input delivered to another VM.

Please use the same connection/profile routing identity as display requests across screen event consumers, including the pane listener and installation events. Acceptance: an event from B cannot change A's control or installation state when their remote paths happen to match.

Controlled regression probe — expected to fail on this head

Save the following review-only test as apps/desktop/src/plugins/hermes-bots/screen-review-108914.test.tsx in a disposable checkout. The test file is not part of this PR. Run cd apps/desktop && ../../node_modules/.bin/vitest run src/plugins/hermes-bots/screen-review-108914.test.tsx.

import { act, renderHook } from '@testing-library/react'
import { expect, it, vi } from 'vitest'

vi.mock('@hermes/plugin-sdk', async () => {
  const { useStore } = await import('@nanostores/react')
  const { onGatewayEvent } = await import('../../contrib/events')
  return { Codicon: () => null, useValue: useStore, host: { onEvent: onGatewayEvent } }
})
vi.mock('./data', async () => {
  const { atom } = await import('nanostores')
  return { $lastRoster: atom([]), botSelectionKey: (bot: { connectionId: string; name: string }) => `${bot.connectionId}:${bot.name}` }
})
vi.mock('./i18n', () => ({ useBots: () => ({}) }))
vi.mock('./routing', () => ({ resolveBotConnectionRoute: () => ({ route: null }) }))
vi.mock('./screen-connection', () => ({ VIEWER_ID: 'same-desktop-window', displayRequest: vi.fn() }))
vi.mock('./screen-open', () => ({ openBotScreen: vi.fn() }))

import { emitGatewayEvent } from '../../contrib/events'
import { useScreenPortalState } from './screen-portal'
import { $screenState, setScreenStatus } from './screen-state'
import type { DisplayStatus } from './screen-connection'
import type { RosterRow } from './types'

it('keeps host A control state unchanged when host B has the same home path', () => {
  const botA = { name: 'default', connectionId: 'host-a', sourceScoped: true, connectionKind: 'remote' } as RosterRow
  const status = {
    profile: 'default', profile_key: '/home/hermes/.hermes', supported: true, installed: true, running: true,
    lease: { holder: 'agent', viewer_id: null, pending_handoff: null, since: 1, reason: '' }
  } as DisplayStatus
  $screenState.set({})
  setScreenStatus(botA, status)
  const { result, unmount } = renderHook(() => useScreenPortalState(botA))
  expect(result.current.lease?.holder).toBe('agent')
  act(() => emitGatewayEvent({
    type: 'display.lease', connectionId: 'host-b', profile: 'default',
    payload: { profile_key: status.profile_key, lease: { holder: 'human', viewer_id: 'same-desktop-window', pending_handoff: null, since: 2, reason: '' } }
  }))
  try {
    expect(result.current.lease?.holder).toBe('agent')
  } finally {
    unmount()
  }
})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d947fc8: one predicate isEventForBotScreen(bot, event, profileKey) in screen-connection.ts requires event.connectionId to equal the bot's route connection (untagged events only match a local route) AND the profile key; used by the pane lease listener and both install listeners. Test: screen-connection.test.ts.

@teknium1
teknium1 force-pushed the hermes/hermes-b802e898 branch from cc0814d to d947fc8 Compare September 12, 2026 19:54
@teknium1

Copy link
Copy Markdown
Collaborator Author

Thanks @Julientalbot — every finding reproduced. All five plus both design notes are addressed in 7a43ce8b57fe and d947fc8 (head d947fc8); inline replies carry the per-item evidence.

The two design notes: the lease reader now fails CLOSED on a present-but-unparsable file (only a missing file is a fresh agent-held profile; the next successful write repairs it), and a takeover after request_handoff keeps the agent's reason on the lease, which the pane shows while the human acts.

Live re-verification on a real Xvnc screen against hermes serve: browser click/snapshot refused with human_has_control while another process held the lease; 1006 drop kept human, 1000 close returned agent. Python: tests/tools + tests/tui_gateway 10327 pass (6 pre-existing host-specific failures unrelated to this PR); Desktop: tsc clean, 0 lint errors, 9803 vitest pass.

@carlotestor carlotestor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this on a headless host (no X packages, so the noVNC/takeover UI is untested — this is the Python half only).

What passed: the PR's own suite is 19/19 green. I also wrote 13 independent adversarial tests against RfbClientFilter, since server-side input filtering is the load-bearing security claim here, and it holds up well:

  • KeyEvent / PointerEvent / ClientCutText / QEMU Extended KeyEvent all dropped without the lease; FramebufferUpdateRequest still passes
  • byte-by-byte fragmentation gives byte-identical output to bulk feeding, and 50 randomized chunkings of a 120-message stream leaked zero input bytes — nothing sneaks through a split message
  • variable-length framing (SetEncodings, Fence, extended clipboard with negative length) stays correctly framed, so an input message can't hide behind a mis-parsed one
  • lease flip applies to the very next message; ClientInit shared-flag is forced to 1 even when the handshake is split across chunks

Lease under real concurrency is clean too: 6 processes × 200 acquire/release cycles on one file ended holder=agent, epoch 1970, no torn or corrupt state, no leftover .tmp files; a stale viewer's release doesn't yank control from the current holder.


One finding: unbounded memory growth in the RFB parser, reachable by a viewer with no lease.

_message_length() trusts the client-declared ClientCutText length, and feed() accumulates into self._buf until that many bytes arrive. Nothing caps _buf, and display.py:124 feeds every WebSocket frame in with no limit of its own. So a read-only watcher — someone who only ever had view access and never held the lease — can send an 8-byte header declaring 0x7FFFFFFF and then stream body bytes indefinitely. Every byte is retained in RAM, none is forwarded to Xvnc, and the connection is never closed. That's an OOM of the hermes serve process from the least-privileged role in the design.

Repro (passes on d947fc83a4):

f = RfbClientFilter(lambda: False)          # viewer does NOT hold the lease
f.feed(b"RFB 003.008\n\x01\x00")
f.feed(struct.pack("!BBBBI", 6, 0, 0, 0, 0x7FFFFFFF))   # ClientCutText, 2GB declared
for _ in range(64):
    f.feed(b"A" * 65536)
assert len(f._buf) == 8 + 64 * 65536        # 4MB held, nothing forwarded, socket still open

The fix looks like a couple of lines, and the escape hatch already exists — feed() raising ValueError is exactly the "we cannot frame this, kill the stream" path that unknown message types use, and display.py already turns it into a _CLOSE_PROTOCOL close. Capping the declared clipboard length (real ones are kilobytes; TigerVNC's own default limit is 1MB) and raising past that reuses the existing behaviour rather than adding a new one. Worth bounding _buf overall as well, so any future variable-length type inherits the ceiling instead of needing its own check.

Happy to push the cap plus a regression test if useful.

@frosty00

Copy link
Copy Markdown

^ my agent checked it based off your tweet https://x.com/Teknium/status/2098881098885595149

@helix4u

helix4u commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

@teknium1 I found a Windows regression at d947fc8 that I don't see covered in the review yet. Every nonempty computer_use action imports handoff, which imports bot_desktop.lease, and that module unconditionally imports fcntl. Running the PR's lease module on Windows produces ModuleNotFoundError: No module named 'fcntl'. This happens before backend dispatch, even with Bot Screen disabled. display.status also reaches the lease import before it can report an unsupported host. Could the Linux lease integration be gated so ordinary Windows computer use keeps working, with a regression covering that entry point?

I also reproduced @carlotestor's parser finding on this head. A watcher without control can declare a large clipboard message and keep growing the retained buffer. My bounded probe retained 4,194,312 bytes without rejection. Their proposed message-size and buffer caps fit the existing protocol-error path.

One part of @Julientalbot's takeover finding still looks incomplete: the epoch check discards results, but acquisition acknowledges human control without waiting for an already-running browser/computer-use action to stop or finish. That leaves room for a delayed click or typing operation after takeover. This last point is from source inspection, not a live Xfce reproduction. Could we cover that overlap and confirm human control only once existing actions can no longer affect the screen?

@iowahawkeyedave iowahawkeyedave left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test pass — independent probes + suite run (macOS host, Linux CI lane untouched)

Ran the PR's own bot-desktop suites and then wrote 16 independent adversarial probes (probe_108914.py) attacking the lease and the RFB filter from angles I didn't see covered in the existing tests. All 16 pass on d947fc8. No regressions, no new findings that block.

What ran

Check Result
tests/tools/test_bot_desktop_{lease,runtime,install,browser,browser_fence,launcher_seed}.py + test_display_ws_ticket.py + test_display_ws_drop_keeps_lease.py + test_display_methods.py (via scripts/run_tests.sh, Python 3.11) 17 passed, 0 failed, 2 skipped (linux_only — correct on darwin, they run on the Linux lane)
vitest: screen-connection.test.ts, sibling-ws-url.test.ts 4 passed
tsc --noEmit clean
16 independent probes (below) 16/16 pass

Probe results (the ones worth naming)

RFB filter (rfb_filter.py):

  • Byte-by-byte chunking across the handshake boundary with input gated — handshake forwards, KeyEvent fully dropped.
  • Lease flip mid-stream: the very next key event from the new holder reaches Xvnc; gated viewers get nothing.
  • QEMU Extended KeyEvent (255) gated identically to KeyEvent — the noVNC pseudo-encoding switch doesn't open a hole.
  • Split input message across two feed() calls with the gate flipping between halves: the message is dropped (gate checked at message completion). Fail-closed direction — a message can't sneak through half-authorized. I initially expected gate-at-start semantics; the implemented behavior is the safer one.
  • Unknown message type 77: ValueError, stream killed — no blind forwarding, no hiding input behind an unframed type.
  • ClientCutText with negative (TigerVNC extended clipboard) length: framed correctly and gated for non-holders.
  • FramebufferUpdateRequest / SetEncodings still flow for watchers — view-only viewers actually get a picture.

Lease (lease.py):

  • Corrupt lease.json → holder=human (fail closed). Verified, matches the docstring.
  • Deleted lease.json → holder=agent. This is the one fail-open path — defensible (deleting the file requires the same local access as everything else the agent can already do) but worth a line in the docs next to the "not a security boundary" note.
  • Stale viewer release("viewer-STALE") while viewer-A holds: lease untouched, epoch unchanged. A closing window can't yank control from the person who took over after it.
  • Cross-process fence: a fresh subprocess reading the same HERMES_HOME sees the parent's acquire — the fcntl-on-disk design holds across process boundaries, which is the whole point.
  • assert_agent_may_act() raises HumanHasControl while a human holds; epoch strictly increments per transition; wait_for_release unblocked within one poll tick of a hand-back (0.41s).

Docs honesty check: bot-screen.md states plainly that screens are "work surfaces, not security boundaries" and bots share the host account — good. That's the right place to also note the deleted-lease fail-open when you next touch the docs.

Verdict

The control guarantees survived a genuinely adversarial pass. The two prior review rounds plus this one have hit it from lease semantics, transport framing, process boundaries, and the install/sudo path. Ship it.

MrD1az commented Sep 12, 2026

Copy link
Copy Markdown

Consolidated review of d947fc83a461f098066c71ee6e28161736b87f53

Recommendation: fix-then-ship. AI-assisted review with bounded local reproductions. Five must-fix items below, including independent confirmation of the Windows regression already reported by @helix4u. The pane-close finding overlaps a second review and is included once. Findings distinguish executed checks from untested platform behavior.

F1 · P1 · Check the capture epoch before image persistence or auxiliary vision

tools/computer_use/tool.py, lines 288–294

The post-dispatch epoch check runs after _dispatch() has processed the captured frame. _capture_response() calls _capture_view(), which persists the screenshot, then may call _route_capture_through_aux_vision() and vision_analyze_tool. If the human takes over during capture, the final response reports human_has_control only after that processing has happened. Rejecting the outer result cannot retract an image already routed to auxiliary vision.

VERIFIED with probes/review_browser_repro.py: the synthetic backend acquires the real lease during capture. Actual dispatch writes the synthetic image into the isolated cache and invokes the recording auxiliary-vision stub while the human holds control. The outer result then reports human_has_control. No real image or external vision call was used.

Requested change: validate the frame's admission epoch before persistence and auxiliary-vision routing. Apply the same check to capture_after and spilled accessibility data. Keep the existing final fence as additional protection where appropriate.

F2 · P1 · Include locally launched real-profile CDP browsers in the lease fence

tools/browser_tool_session.py, lines 596–602

_shares_bot_desktop_browser() returns false for every session with cdp_url. However, local real-profile sessions also use CDP, and this PR explicitly launches their headed Chromium with the Bot Desktop environment in browser_tool_real_profile.py, lines 166–172. With real-profile mode, headed browsing, and Bot Desktop active, browser actions and observations bypass the human lease on the very screen the human is using.

VERIFIED with probes/review_browser_repro.py: an isolated human lease, published display, and local real-profile session with CDP reach _run_browser_command_unfenced() and return a synthetic snapshot. The browser execution boundary was stubbed. Session creation and launch provenance were traced in source.

Requested change: decide whether the session belongs to this display from its local launch provenance. Fence local real-profile CDP sessions while preserving the exemption for unrelated remote or user-attached browsers.

F3 · P1 · Existing report confirmed: preserve Windows computer use

tools/computer_use/tool.py, lines 251–256

This independently confirms the existing Windows report. Every valid computer_use action now imports handoff, which imports lease. The new lease module imports fcntl unconditionally. Native Windows has no fcntl, so capture, click, and the other existing actions fail before backend selection or error handling, even when Bot Desktop is disabled.

VERIFIED with probes/review_browser_repro.py: making only the fcntl import unavailable causes actual handle_computer_use({'action': 'capture'}) to raise ModuleNotFoundError. This is an import-boundary simulation on macOS. Native Windows execution is UNKNOWN.

Requested change: keep Linux-only Bot Desktop handling behind the supported-platform path, or provide suitable portable locking without breaking existing Windows dispatch.

F4 · P2 · Finish the connection-identity fix in the portal listener

apps/desktop/src/plugins/hermes-bots/screen-portal.tsx, lines 102–106

The pane and install listeners now use isEventForBotScreen, but the portal still matches only profile_key. When two hosts both use /home/hermes/.hermes, host B's lease event overwrites host A's shared screen cache. An open pane for A can then show that the user has control and change its controls although A's backend still lets its agent act. The server input filter remains separate and still rejects unauthorized viewer input.

VERIFIED with probes/desktop-portal-probe.cjs: Node executes the actual TypeScript hook with a mocked React/event bus. A host-B event writes host A's lease. This is an incomplete fix of the earlier review finding, not a claim of cross-host video or cookie transfer.

Requested change: use the existing isEventForBotScreen predicate in this listener and exercise the portal hook in the regression test.

F5 · P2 · Explicitly release control when the pane is closed

apps/desktop/src/plugins/hermes-bots/screen-pane.tsx, lines 81–85

detach() disconnects noVNC and calls WebSocket.close() without a status. The pinned noVNC 1.7.0 implementation also closes the underlying socket without a status. That intentional close is observed as 1005, while _bridge() releases only for 1000 or 1001. Closing the pane therefore leaves the human lease held and the bot paused. The current disconnect tests cover 1000 and 1006, omitting this actual client path.

VERIFIED with probes/probe_close.py: Node's native WebSocket calls close() against a temporary loopback server, which receives 1005. Passing that observed code through the PR's actual bridge with a fake framebuffer leaves holder=human. The exact dependency source was also checked at noVNC 1.7.0 websock.js. No live Electron pane was exercised.

Requested change: distinguish intentional pane teardown from connection loss explicitly, using a lease release or a deliberate clean close before noVNC starts closing the socket. Preserve exclusion on actual dropped links and avoid releasing during automatic reattachment.

Should-fix

F6 · P2 · Reserve a display until Xvnc has claimed it

tools/bot_desktop/runtime.py, lines 140–147

The host allocation lock is released when _allocate_display() returns. The selected number is not reserved until the subsequently launched Xvnc creates its X lock. Two profiles starting concurrently can therefore both choose :20, causing one screen startup to fail. The existing regression test covers a display already occupied by a live process, which misses this interval.

VERIFIED with probes/probe_allocation.py: both actual runtime.start() pipelines run concurrently, with isolated profile directories and fake launchers synchronized before readiness. Both allocate display 20. The free-display probe and Xvnc processes are mocked. Real Linux startup contention is UNKNOWN.

Requested change: keep allocation ownership until the server claims the display or atomically record pending reservations. Add a concurrent cold-start regression.

F7 · P2 · Make the documented CLI recovery release the lease

hermes_cli/subcommands/computer_use_screen.py, lines 47–50

The CLI screen stop calls only runtime.stop(), unlike the display RPC which also releases the lease. The help and troubleshooting documentation promise that CLI stop/start hands control back. A persisted human lease therefore survives the documented recovery, including on a profile whose screen is already down.

VERIFIED: acquire an isolated lease, make _launcher_pid() report no running screen, call actual _screen_stop(None), and the lease is still human. This is recoverable: a new viewer can Take over then Hand back. Please release the lease in the deliberate CLI stop path and document that UI recovery. Do not clear the lease merely on an abnormal connection loss.

F8 · P2 · Isolate thumbnail Xauthority changes

tools/bot_desktop/thumbnail.py, lines 28–37

Overlapping thumbnail requests overwrite process-wide os.environ['XAUTHORITY'] and restore each other's temporary values. The gateway dispatches requests through worker threads, so different profiles can overlap here.

VERIFIED: two actual thumbnail wrappers with a fake Xlib read produced (':20', '/isolated/21') and (':21', '/original'). After both finished, the process retained /isolated/20 instead of /original. The wrong cookie file can break X authentication and contaminate environments inherited by later work. No real Xvnc authentication failure or cross-profile image disclosure was tested.

Please avoid changing process-wide environment for concurrent profile captures, for example by using an isolated capture process.

Existing reports and remaining test requests

The unbounded RFB clipboard buffer is already covered by @carlotestor's review, and the unrelated bootstrap-installer lockfile version change is already recorded in the CI comment. Linking these avoids adding duplicate findings.

A second review also raised lease-schema hardening and the query-string display ticket. The controlled lease probe below confirms that {} and {"holder":"invalid"} permit agent admission, while [] raises AttributeError. The normal atomic writer was not shown to produce these shapes, so this is a hardening observation, not an additional reproduced production failure. The query-string ticket is single-use and short-lived; no exposure was demonstrated here. Please include dock Browser opened by the human first → Hand back → agent-browser launch in the live test matrix. That Chromium singleton concern remains UNKNOWN, not a confirmed launch failure.

Validation

The repository's canonical runner covered nine focused files: test_bot_desktop_browser.py, test_bot_desktop_browser_fence.py, test_bot_desktop_install.py, test_bot_desktop_launcher_seed.py, test_bot_desktop_lease.py, test_bot_desktop_runtime.py, test_display_ws_drop_keeps_lease.py, test_display_ws_ticket.py, and test_display_methods.py.

17 tests passed, two Linux-only tests skipped. The first pass had 15 passes and two Unix-socket binding failures caused by the sandbox. Rerunning only that file with temporary socket permission passed both tests. No upstream tests were modified. An archive checkout emitted a non-fatal git-metadata warning during bytecode preparation.

The probes below run actual code paths with the stated execution boundaries replaced. They use temporary profiles, synthetic images, and no external vision service. Native Windows, live Linux/Xvnc/Xfce, full Electron integration, and the full repository suite were not run. An exit code of zero for these probes means they reproduced the current behavior, not that the behavior is correct.

What looks good: shared on-disk leases, profile-scoped tickets, server-side RFB input filtering, epoch checks, and exclusion on abnormal disconnect all have useful focused coverage. The findings above identify gaps where those mechanisms meet existing paths.

Reproduction scripts

Use a disposable checkout at the reviewed SHA with the normal Python development dependencies and Node 22 with TypeScript stripping support. Save each block using its displayed filename. Run Python from that checkout or set PYTHONPATH to it. The socket probe binds temporary loopback and Unix sockets only. These are review probes, not proposed production changes.

review_browser_repro.py

Run: PYTHONPATH=/path/to/checkout python review_browser_repro.py

import base64, builtins, json, os, sys, tempfile
from pathlib import Path
from unittest.mock import patch

with tempfile.TemporaryDirectory(prefix='hermes-review-browser-home-') as home:
    os.environ['HERMES_HOME'] = home
    from tools.computer_use import tool
    actual_import = builtins.__import__
    def windows_import(name, *args, **kwargs):
        if name == 'fcntl':
            raise ModuleNotFoundError("No module named 'fcntl'")
        return actual_import(name, *args, **kwargs)
    with patch('builtins.__import__', side_effect=windows_import):
        try:
            tool.handle_computer_use({'action': 'capture'})
        except ModuleNotFoundError as exc:
            assert 'fcntl' in str(exc)
            print('WINDOWS: valid capture raises before backend: ' + str(exc))
        else:
            raise AssertionError('Expected unavailable fcntl failure')

    from tools.bot_desktop import lease, runtime
    from tools import browser_tool as bt
    from tools import browser_tool_session as session
    lease.acquire('human')
    info = {'session_name':'rp_test', 'cdp_url':'http://127.0.0.1:9222',
            'features': {'local':True, 'real_profile':True}}
    dispatched = []
    def fake_unfenced(*args):
        dispatched.append(args)
        return {'success':True, 'data':{'snapshot':'HUMAN-SCREEN-SENTINEL'}}
    with patch.object(runtime, 'published_env', return_value={'DISPLAY':':37'}), \
         patch.object(session, '_browser_command_preflight', return_value={'browser_cmd':'agent-browser'}), \
         patch.object(session, '_get_session_info', return_value=info), \
         patch.object(session, '_run_browser_command_unfenced', side_effect=fake_unfenced):
        result = session._run_browser_command('review','snapshot',[],timeout=1)
        assert dispatched and result['data']['snapshot'] == 'HUMAN-SCREEN-SENTINEL'
        print('REAL_PROFILE: human lease held, local real-profile CDP snapshot dispatched and returned')

    lease.release('human')
    from tools.computer_use.backend import CaptureResult
    image = base64.b64encode(b'fake-capture-only-no-credentials').decode()
    class Backend:
        def capture(self, **kwargs):
            lease.acquire('human')
            return CaptureResult(mode='vision', width=100, height=100, png_b64=image)
    vision_calls=[]
    def fake_vision(cap, summary, **kwargs):
        assert lease.human_holds()
        vision_calls.append(cap.png_b64)
        return json.dumps({'vision_analysis':'synthetic image received'})
    with patch.object(tool, '_get_backend', return_value=Backend()), \
         patch.object(runtime, 'ensure_started_for_tool'), \
         patch.object(tool, '_should_route_through_aux_vision', return_value=True), \
         patch.object(tool, '_route_capture_through_aux_vision', side_effect=fake_vision):
        result = json.loads(tool.handle_computer_use({'action':'capture','mode':'vision'},session_id='review'))
        persisted = list(Path(home).glob('cache/images/computer_use_*'))
        assert result['code'] == 'human_has_control'
        assert vision_calls == [image]
        assert len(persisted) == 1 and persisted[0].read_bytes() == base64.b64decode(image)
        print('CAPTURE: returned human_has_control, but image already reached aux-vision seam and persisted in cache')
probe_allocation.py

Run: python probe_allocation.py /path/to/checkout

"""Reproduce PR 108914's concurrent display allocation without starting Xvnc.

Usage: python3 probe_allocation.py /path/to/pinned/hermes-agent
The fake launcher synchronizes starts before creating its ready files. Both
actual runtime.start() calls must therefore allocate before either server is up.
"""
import contextvars
from concurrent.futures import ThreadPoolExecutor
from pathlib import Path
import sys
import tempfile
import threading

sys.path.insert(0, str(Path(sys.argv[1]).resolve()))
from tools.bot_desktop import browser, runtime

profile = contextvars.ContextVar("probe_profile")
with tempfile.TemporaryDirectory(prefix="hermes-alloc-proof-") as tmp:
    root = Path(tmp)
    runtime.state_dir = lambda: root / profile.get()
    runtime.is_supported_host = lambda: True
    runtime.missing_binaries = lambda: []
    runtime._launcher_pid = lambda: None
    runtime._display_in_use = lambda num: False
    runtime._ALLOC_LOCK = root / "alloc.lock"
    runtime._profile_name = lambda: profile.get()
    runtime.geometry = lambda: "1440x900"
    runtime.status = lambda: (profile.get(), (runtime.state_dir() / "display").read_text())
    browser.dock_launch = lambda: None
    barrier = threading.Barrier(2)

    class FakeLauncher:
        pid = 12345

        def __init__(self, args, *, env, **kwargs):
            barrier.wait(timeout=5)
            Path(env["HERMES_BD_ENV_FILE"]).write_text("DISPLAY=:" + env["HERMES_BD_DISPLAY_NUM"])
            Path(env["HERMES_BD_SOCKET"]).touch()

        def poll(self):
            return None

    runtime.subprocess.Popen = FakeLauncher

    def start_profile(name):
        profile.set(name)
        return runtime.start(wait_seconds=1)

    with ThreadPoolExecutor(max_workers=2) as pool:
        results = list(pool.map(start_profile, ["profile-a", "profile-b"]))
    print("Actual runtime.start() allocations:", results)
    assert results[0][1] == results[1][1], "Expected the current PR's duplicate allocation"
    print("REPRODUCED: distinct profiles received the same X display before either launcher became ready.")
    print("Scope: real Python startup/allocation flow; mocked Xvnc process and free-display probe.")
print("Temporary profile directories removed:", not root.exists())
probe_close.py

Run: python probe_close.py /path/to/checkout

"""Review-only probe: a native WebSocket close() omits a close status."""
import asyncio
import importlib.util
import os
import tempfile
from pathlib import Path
import sys

ROOT = Path(sys.argv[1]).resolve()
sys.path.insert(0, str(ROOT))

async def main():
    from websockets.asyncio.server import serve
    result = asyncio.get_running_loop().create_future()
    async def handler(ws):
        await ws.wait_closed()
        result.set_result(ws.close_code)
    async with serve(handler, '127.0.0.1', 0) as server:
        port = server.sockets[0].getsockname()[1]
        child = await asyncio.create_subprocess_exec('node', '-e',
            f"const ws = new WebSocket('ws://127.0.0.1:{port}'); ws.onopen = () => ws.close();")
        code = await asyncio.wait_for(result, 10)
        assert await child.wait() == 0
    print(f'native WebSocket.close() server close code: {code}')
    assert code == 1005
    with tempfile.TemporaryDirectory(prefix='review-close-') as home:
        os.environ['HERMES_HOME'] = home
        spec = importlib.util.spec_from_file_location('bridge_test', ROOT / 'tests/hermes_cli/test_display_ws_drop_keeps_lease.py')
        module = importlib.util.module_from_spec(spec)
        spec.loader.exec_module(module)
        module.lease.acquire('desk-1', profile_key=home)
        after = await module._bridge_once(code, home)
        print(f'actual bridge after status-less intentional close: holder={after.holder}')
        assert after.holder == module.lease.HUMAN

asyncio.run(main())
desktop-portal-probe.cjs

Run: node --disable-warning=ExperimentalWarning desktop-portal-probe.cjs /path/to/checkout

const fs = require('node:fs')
const vm = require('node:vm')
const assert = require('node:assert/strict')
const { stripTypeScriptTypes } = require('node:module')
const root = require('node:path').resolve(process.argv[2])
const source = fs.readFileSync(root + '/apps/desktop/src/plugins/hermes-bots/screen-portal.tsx', 'utf8')
  .split('export function ScreenPortal(')[0]
  .replace(/^import .*$/gm, '')
  .replace(/export /g, '')
const callbacks = []
const writes = []
const status = { supported: true, installed: true, running: true, profile_key: '/home/hermes/.hermes' }
const context = {
  host: { onEvent: (type, fn) => { callbacks.push(fn); return () => {} } },
  useEffect: fn => fn(),
  useValue: () => ({}),
  $screenState: {},
  screenStateFor: () => ({ status, lease: null }),
  setScreenLease: (bot, lease) => writes.push({ bot, lease }),
  VIEWER_ID: 'desktop-test'
}
vm.createContext(context)
vm.runInContext(stripTypeScriptTypes(source) + '; this.run = useScreenPortalState', context)
context.run({ name: 'default', sourceScoped: true, connectionId: 'host-a' })
callbacks[0]({ type: 'display.lease', connectionId: 'host-b', payload: {
  profile_key: status.profile_key,
  lease: { holder: 'human', viewer_id: 'desktop-test' }
} })
assert.equal(writes.length, 1)
assert.equal(writes[0].bot.connectionId, 'host-a')
console.log('REPRODUCED: host-b lease event wrote host-a screen cache, despite different connectionId')
probe_cli_release.py

Run: PYTHONPATH=/path/to/checkout python probe_cli_release.py

"""Review-only check: documented CLI recovery leaves an old lease behind."""
import os
import tempfile
from unittest.mock import patch

with tempfile.TemporaryDirectory(prefix='review-cli-lease-') as home:
    os.environ['HERMES_HOME'] = home
    from tools.bot_desktop import lease, runtime
    from hermes_cli.subcommands.computer_use_screen import _screen_stop
    lease.acquire('closed-window')
    with patch.object(runtime, '_launcher_pid', return_value=None):
        _screen_stop(None)
    assert lease.human_holds()
    print('REPRODUCED: CLI stop leaves human lease on a profile with no screen')
    lease.acquire('new-window')
    lease.release('new-window')
    assert not lease.human_holds()
    print('Take over then Hand back remains a recovery path')
probe_thumbnail_lease.py

Run: python probe_thumbnail_lease.py /path/to/checkout

"""Review probe: concurrent real thumbnail wrappers with simulated Xlib read.
Usage: python3 probe_thumbnail_lease.py /path/to/pinned/checkout
"""
import contextvars
from concurrent.futures import ThreadPoolExecutor
import os
from pathlib import Path
import sys
import tempfile
import threading
import types

sys.path.insert(0, sys.argv[1])
from tools.bot_desktop import runtime, thumbnail, lease
profile = contextvars.ContextVar('probe_profile')
runtime.published_env = lambda: {'DISPLAY': ':' + profile.get(), 'XAUTHORITY': '/isolated/' + profile.get()}
runtime._launcher_pid = lambda: 1234
first_entered = threading.Event()
second_entered = threading.Event()
first_finished = threading.Event()
seen = []
class Image:
    def thumbnail(self, *args): pass
    def convert(self, *args): return self
    def save(self, buf, *args, **kwargs): buf.write(b'fake-thumbnail')
def grab(*, xdisplay):
    if xdisplay == ':20':
        first_entered.set()
        assert second_entered.wait(3)
        seen.append((xdisplay, os.environ.get('XAUTHORITY')))
    else:
        second_entered.set()
        assert first_finished.wait(3)
        seen.append((xdisplay, os.environ.get('XAUTHORITY')))
    return Image()
pil = types.ModuleType('PIL')
pil.ImageGrab = types.SimpleNamespace(grab=grab)
sys.modules['PIL'] = pil
original = os.environ.get('XAUTHORITY')
os.environ['XAUTHORITY'] = '/original'
def run(name):
    profile.set(name)
    thumbnail.thumbnail_data_url()
    if name == '20': first_finished.set()
try:
    with ThreadPoolExecutor(max_workers=2) as pool:
        first = pool.submit(run, '20')
        assert first_entered.wait(3)
        second = pool.submit(run, '21')
        first.result()
        second.result()
    print('Observed (DISPLAY, XAUTHORITY):', seen)
    print('Final process XAUTHORITY:', os.environ['XAUTHORITY'])
    assert seen == [(':20', '/isolated/21'), (':21', '/original')]
    assert os.environ['XAUTHORITY'] == '/isolated/20'
finally:
    if original is None: os.environ.pop('XAUTHORITY', None)
    else: os.environ['XAUTHORITY'] = original
with tempfile.TemporaryDirectory(prefix='lease-shape-proof-') as tmp:
    path = Path(tmp) / 'bot-desktop' / 'lease.json'
    path.parent.mkdir()
    for raw in ('{}', '{"holder":"invalid"}', '[]'):
        path.write_text(raw)
        try:
            admitted = lease.assert_agent_may_act(tmp)
            print('Lease input', raw, '=> AGENT ADMITTED, holder=', admitted.holder)
        except Exception as exc:
            print('Lease input', raw, '=>', type(exc).__name__)
print('Temporary lease removed:', not Path(tmp).exists())

@alt-glitch alt-glitch added the comp/cli CLI entry point, hermes_cli/, setup wizard label Sep 12, 2026

@iamlukethedev iamlukethedev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of d947fc83a461

Thanks for the thorough prior rounds — the on-disk lease, server-side RFB gate, epoch discard, abnormal-disconnect exclusion, and connection-scoped pane/install listeners are in good shape. I’m still blocked on a few control-boundary and regression gaps that show up on this head.

Request changes

1. Windows computer use broken by unconditional fcntl (P1)
Every nonempty computer_use action imports handoff → bot_desktop.lease, which does import fcntl at module import (lease.py). That raises on native Windows before backend selection, even with Bot Screen disabled / unsupported. display.status hits the same import via _display_snapshot. Please keep Linux lease integration behind the supported-platform path (or a portable lock), and add a regression that handle_computer_use({'action': 'capture'}) still works when fcntl is unavailable.

2. Unbounded RFB ClientCutText buffer (P1) — agrees with @carlotestor
RfbClientFilter._message_length trusts the client-declared length and feed() grows _buf with no cap. A watcher who never held the lease can declare a huge clipboard message and retain megabytes (or worse) in the hermes serve process. Cap declared clipboard length (TigerVNC’s ~1MB default is a reasonable ceiling) and overall _buf; raising ValueError already maps to _CLOSE_PROTOCOL in display.py.

3. Local real-profile CDP sessions bypass the browser lease fence (P1)
_shares_bot_desktop_browser returns false whenever cdp_url is set, but real-profile local sessions always use CDP and this PR launches their headed Chromium with the Bot Desktop env. Under real-profile + headed + Bot Desktop, clicks/snapshots can still hit the screen the human is using. Please fence by local launch provenance; keep the exemption for unrelated remote / user-attached CDP.

4. Epoch check is after image persistence / aux-vision (P1)
In handle_computer_use, the post-dispatch epoch comparison runs after _dispatch → _capture_response → _capture_view, which may already persist the screenshot and route through aux vision. Returning human_has_control cannot retract that. Validate the admission epoch before persistence and vision routing (keep the final fence as belt-and-braces). Same idea for capture_after / spilled accessibility data.

5. Takeover vs in-flight input (P1 / clarify) — agrees with @helix4u
Discarding the result after an epoch change is necessary but not sufficient if an already-running click/type can still deliver to the seat after acquire. Prefer acknowledging human control only once admitted actions can no longer affect the screen, or cancel/fence at the backend boundary. Happy to treat a clear design note + test as enough if full cancel is hard.

Also please fix (P2)

  • Portal listener still matches profile_key only (screen-portal.tsx); pane/install use isEventForBotScreen. Two hosts with the same ~/.hermes path can still cross-update the portal cache — finish the connection-identity fix there and cover it in screen-connection / portal tests.
  • Closing the Screen pane calls WebSocket.close() without a status (often observed as 1005); the bridge only releases on 1000/1001, so Hand back via “close the pane” can leave holder=human and the bot paused. Explicit lease release or a deliberate clean close on intentional teardown; keep exclusion on real drops (1006).
  • CLI hermes computer-use screen stop only calls runtime.stop() while help text says it hands control back; gateway display.stop already releases. Align CLI with the RPC.
  • Display allocation: host lock is released before Xvnc creates the X lock — two cold starts can pick the same :N. Hold ownership until claimed / record pending reservations.
  • Thumbnail: mutating process-wide XAUTHORITY is racy across concurrent profile thumbnails in worker threads — isolate (e.g. subprocess or thread-local capture).
  • Lease reader hardening: {} / {"holder":"invalid"} currently admit the agent; [] raises AttributeError (not caught). Fail closed on unknown/invalid shapes.

Nits

  • package-lock @novnc/novnc@1.7.0 looks intentional; please add ci-reviewed once the bootstrap-installer lockfile bump is confirmed expected.
  • Would love regressions for: Windows import gate, RFB size caps, real-profile fence, portal connection match, pane-close release, CLI stop release.

CI

Required checks on this head look green (All required checks pass). Desktop E2E was skipped. Mergeability is still blocked pending review/labels.

Happy to re-review quickly once the P1s are in.

@Xipong Xipong left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed d947fc83a461f098066c71ee6e28161736b87f53. I recommend fixing the following before merge. Three additional defects and one residual occurrence of an existing finding; no style requests.

[P1] Ordinary Windows computer_use now imports Unix-only fcntl

Locations: tools/computer_use/tool.py::handle_computer_use, tools/computer_use/handoff.py, tools/bot_desktop/lease.py.

Every computer_use invocation now imports handoff before dispatch; handoff imports the lease, whose module-level import fcntl is unconditional. Native Windows Python does not provide fcntl. Even an ordinary Windows capture/click with Bot Desktop disabled therefore fails before reaching the CUA backend. runtime.start's Linux guard is too late.

Gate the Bot Desktop integration before importing Unix-only code, or make the lease importable on unsupported platforms without breaking the existing Windows backend. Test ordinary computer_use with fcntl unavailable and Bot Desktop disabled. Source-traced portability regression; no native Windows execution claimed.

[P2] Clean pane closure can strand the human lease

Locations: apps/desktop/src/plugins/hermes-bots/screen-pane.tsx::detach; hermes_cli/web_routers/display.py::_CLEAN_CLOSE / _bridge.

The server releases control only for 1000/1001, but pane cleanup calls rfb.disconnect() and socket.close() without a close status. The pinned noVNC 1.7.0 Websock.close also calls the underlying WebSocket.close with no argument. A clean close with no status is reported as 1005. Closing the pane while holding control can therefore leave the persisted human lease held, blocking computer_use and local browser tools.

Actually executed protocol probe (Node WebSocket -> Python websockets server): ws.close() yielded server code 1005 and client wasClean=true; ws.close(1000) yielded server code 1000. This PR's whitelist releases only the latter. This is not an Electron/noVNC/Xvnc end-to-end test.

Explicitly distinguish intentional pane closure from reconnect/transport failure and return control on the intentional-close path, or send the intended status on that path. Do not unconditionally release in detach: attach calls it too, and abnormal-disconnect exclusion must remain intact.

[P2] The allocator releases its lock before reserving the display

Location: tools/bot_desktop/runtime.py::_allocate_display / start.

The host-wide flock ends when _allocate_display returns. start only records the selection in the current profile, then spawns Xvnc. A second profile/process can enter before Xvnc creates /tmp/.X20-lock and select the same :20; the allocator does not inspect other profiles' selections or keep pending reservations. At least one desktop can fail despite other display numbers being free. Separate CLI processes can hit this regardless of per-gateway RPC serialization.

An isolated probe with the extracted allocator, real flock and temporary profile directories returned :20 for both profiles after recording A's local selection but before Xvnc startup. No live Xvnc concurrency test claimed.

Hold the allocation lock until the server claims its display, or create a host-wide pending reservation with failure cleanup. Test two starts interleaved before Xvnc readiness.

[P2, residual of the existing review] The portal still matches lease events by path alone

Location: apps/desktop/src/plugins/hermes-bots/screen-portal.tsx::useScreenPortalState, display.lease listener.

isEventForBotScreen was added to the pane/install listeners, but this listener still checks only payload.profile_key === profileKey and writes the shared screen store. A host-B event can still overwrite host A's control state when both homes have the same path. Since VIEWER_ID is window-wide, A can claim this viewer is in control while A's backend remains agent-controlled.

This is the unconverted occurrence already identified in @Julientalbot's portal review, not a new independent discovery. Apply the connection/profile predicate here too and test the actual hook with two hosts sharing a path; testing the predicate alone does not detect an unconverted consumer.

Scope: full diff and related paths inspected, with the protocol/source-isolated probes above. No full repository suite or live cross-platform desktop run claimed.

@Xipong Xipong left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review of d947fc83a461f098066c71ee6e28161736b87f53, additional to review #5188188627. Four further findings below. I did not repeat the Windows import, close-code, allocator or cross-host portal findings.

[P1] Locally launched real-profile Chrome is incorrectly exempted from the human lease

Location: tools/browser_tool_session.py::_shares_bot_desktop_browser, together with _create_local_session and tools/browser_tool_real_profile.py::_launch_real_profile_chrome.

cdp_url does not establish that a browser is external to Bot Screen. With browser.use_real_profile and browser.headed enabled, Hermes launches the local profile-copy Chrome with the Bot Desktop environment introduced by this PR. _create_local_session records that browser as features={local: true, real_profile: true} and gives it a loopback cdp_url. The new predicate immediately returns false for any CDP URL, so actions and observations against this browser skip both the pre-dispatch human check and the epoch fence, even while a person is using its window on Bot Screen.

Controlled probe: actual _create_local_session, _run_browser_command and _shares_bot_desktop_browser function excerpts, a real persisted HUMAN lease, and stubbed CDP resolution/final command execution. The local real-profile snapshot reached the execution seam and returned a synthetic private-page marker. The ordinary no-CDP local session was refused with human_has_control as a positive control. No Chrome process or real page was used.

Please classify the browser by its owned local launch/display provenance rather than treating CDP transport as proof of a remote browser. Keep genuinely unrelated cloud/user-CDP endpoints exempt. Cover this supported real-profile configuration, not only {session_name: ...} sessions.

[P2] Starting Bot Screen does not rebind an already cached CUA backend

Locations: tools/computer_use/cua_backend.py::cua_driver_child_env, tools/computer_use/tool.py::_get_backend, and tools/computer_use/cua_backend_session.py::_lifecycle_coro.

The new display environment is applied only when a driver transport is spawned. _get_backend reuses the existing backend whenever the permission mode matches; neither display.start nor the tool-boundary hook invalidates it when the profile's display changes. The MCP session is long-lived. Consequently, using computer_use before Start Screen, then starting the screen and continuing in the same session, can leave the driver bound to the original display instead of the screen being viewed. On a Linux workstation that can be the original human seat; a pre-existing headless driver can likewise remain attached to its old environment. A changed display/Xauthority after restart needs the same treatment.

Controlled cache probe using the unchanged _get_backend/_install_backend excerpts and an environment-recording backend: after changing the available environment from :0 to :20, the next call returned the identical backend with its original :0 spawn environment; only one start occurred. The real MCP code was traced to confirm that its child environment is sampled at transport creation, not each action. This is not a live CUA test.

Please include the effective profile/display identity and runtime incarnation in backend reuse, and replace the transport/clear sticky target state before dispatching on a different screen. Add a same-session before-Start/after-Start and screen-restart test.

[P2] Handoff requests from another process never reach the open screen's event listener

Locations: tools/bot_desktop/lease.py::on_change, tui_gateway/methods_display.py::_install_lease_listener, tools/computer_use/handoff.py, and the Screen pane/portal status subscriptions.

The lease is correctly shared on disk, but notification is still process-local. The display gateway broadcasts only transitions delivered to its own lease.on_change listener. A messaging gateway, CLI or isolated worker can call request_handoff in another process, update lease.json, and wait for hand-back without emitting any event in the process serving the Desktop viewer. An already-open pane only fetches status initially/reconnects and otherwise follows those events; thumbnail refreshes carry no lease state. Thus the advertised Bot needs you indicator/reason need not appear until the user manually refreshes or reopens the screen. This finding concerns the screen notification, not whether the model separately tells the user in chat.

Executed two-process probe with the byte-identical lease.py blob 2a3b84d5e5d39f57853682b4a9af741897493ae2, real files and flock. The child request was visible to get() in the parent, but the parent's subscriber received zero notifications; a same-process transition delivered one. wait_for_release remained blocked on the pending request. Only the home/key helper was shimmed to a temporary directory.

Please bridge file/epoch changes into the display server's event stream, or provide bounded status reconciliation that includes handoff requests. Test an already-open viewer with a request emitted by another process.

[P2] The final exec drops the launcher's Xvnc cleanup trap

Location: tools/bot_desktop/launcher.sh: trap ... EXIT followed by exec dbus-run-session ....

Xvnc is a background child of the shell, but the shell is then replaced with dbus-run-session. A successful exec does not run or preserve that shell EXIT trap. If the Xfce panel/private session exits independently of runtime.stop, Xvnc is therefore not cleaned up. Once the launcher PID is gone, _launcher_pid reports no running launcher and stop() returns without signalling the orphaned group. A later start can allocate another display and unlink the old RFB pathname while the old X server continues to exist. Explicit killpg-based Stop working does not cover this independent-session-exit path.

Executed the exact shell lifecycle pattern with sleep standing in for Xvnc and /bin/true standing in for the session: the session/launcher exited, while its server child remained alive. The probe cleaned that child up. This demonstrates the exec/trap issue, not a live Xfce crash.

Keep a supervisor shell/process alive to wait for and reap both children, with cleanup on session failure as well as explicit Stop. Test exit of the session child and verify the X server, socket and display lock are gone.

Validation scope: source tracing plus the controlled probes explicitly described above. No full repository suite, real Chrome/CUA/Xvnc desktop, or Electron acceptance run. A suspected noVNC disconnect/reconnect issue was investigated and excluded: its actual clean-disconnect path triggers the pane's existing reattach effect.

@erosika erosika left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review of d947fc83a461

@teknium1 requesting changes. the RFB filter itself is solid, agree with @carlotestor and @iowahawkeyedave there. the problems are around it: who the server thinks "the viewer" is, what a display ticket lets you do, and a couple of startup races. most of the below isn't in the earlier reviews yet.

what I ran

  • PR suites via scripts/run_tests.sh: 17 passed, 2 skipped (linux-only, I'm on macOS)
  • tests/tools + tests/tui_gateway + tests/hermes_cli: 8045 passed, 11 failed. 10 fail the same way on main. the other one (test_doctor_structural_corruption) is flaky under load and passes 3/3 on its own. no regressions from this PR
  • desktop: tsc clean, eslint clean, the two new vitest files pass
  • no live Xvnc/Xfce run, no linux box on hand

confirmed from earlier reviews

issue who found it note
windows computer_use breaks on import fcntl @helix4u, @MrD1az, @iamlukethedev confirmed. check-windows-footguns passed, so the guard doesn't cover this
closing the pane doesn't hand control back @MrD1az reproduced separately. close() with no code arrives as 1005, bridge only releases on 1000/1001
sidebar Screen row ignores which host an event came from @MrD1az screen-portal.tsx:105
clipboard message can grow memory without limit @carlotestor
real-profile browser skips the lease check @MrD1az
takeover drops results but doesn't stop actions already running @helix4u also: a key held down when control flips never gets its key-up, so after hand-back the agent's next type can go out with a stuck modifier
screenshot is saved / sent to vision before the takeover check @MrD1az
display number race, CLI stop not releasing, thumbnail XAUTHORITY swap, bad lease file shapes @MrD1az, @iamlukethedev agree with all

tickets and viewer ids let other clients in

a display ticket also logs you into the main gateway socket

display.observe mints a ticket meant only for /api/display/ws. but /api/ws accepts any ticket and never checks what it was minted for (web_server_chat.py:273), only the display route does (display.py:45). so a display ticket works as a gateway login, with a user id the client picked. that includes the embedded-TUI internal credential, which is otherwise not treated as a real login.

fix: reject provider == "bot-desktop" tickets at /api/ws, or keep them in a separate store.

anyone who knows the holder's viewer id can type into the screen

  • the client picks its own viewer_id (methods_display.py:105,161)
  • the bridge lets input through when that id matches the lease holder (lease.py:197-199)
  • every lease change is broadcast to all clients with the holder's id in it (methods_display.py:44-45)

so another client reads the id off the broadcast, opens a stream with it, and gets keyboard + mouse next to the real holder. nobody is evicted and the chip doesn't change. if every authenticated client is meant to be equally trusted this is a docs line. if not, the server should assign the id, tie it to the connection, and leave it out of the broadcast.

desktop startup race

starting the same profile twice at once orphans the first desktop

runtime.py:249 only counts the desktop as running once the env file exists, and the launcher writes that after Xfce is up (~5s). nothing locks around the check. a second start in that window (Start button + auto-start, or two clients):

  1. deletes env and launches another launcher.sh
  2. that launcher deletes the live rfb.sock (launcher.sh:46) and writes a new X cookie (launcher.sh:52)
  3. its Xvnc fails, but it already overwrote launcher.pid (runtime.py:280)

end state: first desktop still running with no socket, a changed cookie, no pid on record, and stop() can't find it. this is a different bug from the two-profile display race. fix: one per-profile lock across start and stop.

Screen UI in Hermes Desktop

sidebar Screen row remounts on every render

gateway-groups.tsx:346 passes a new inline function to ContribRender, which uses it as the component type. new type every render → unmount + remount, and while status is unknown each remount sends another display.status. other ContribRender callers pass a stable function.

a profile group with no connection can open another host's bot

screen-portal.tsx:166 treats connectionId === null as a match for any connection. a local group called research can open, and take over, a remote bot also called research.

older backends show a Screen UI that never settles

if display.status fails the state stays unknown, and only unsupported hides the UI. on a backend without display.*, every profile group shows an empty Screen row and the Scheduled Jobs preview says "Checking the screen…" forever.

"control-taken" message never shows

noVNC's disconnect event only has { clean }, no reason, so reason.includes('control-taken') in screen-pane.tsx:137-145 is never true. eviction lands on the clean → re-attach path by accident.

X cookie on argv, lease reads on the event loop, install + UI loose ends

  • launcher.sh:52 puts the X cookie on the xauth command line, briefly readable by other local users. xauth source - over stdin avoids it
  • display.py:100 reads lease.json synchronously on the event loop for every client message, so every mouse move
  • install timeout kills sudo, not the package manager under it
  • the install sudo card has no session id, so it attaches to whatever chat is open, marks that chat as needing input, and can go stale if you switch chats
  • the stale-preview check only counts errors, so a hung socket takes minutes to show "Last seen"
  • screen.recheck i18n key is unused. resolveSiblingWsUrl copies voice-playback.ts instead of replacing it. config comment says hermes desktop, the command is hermes computer-use screen

same-user processes can still capture the login

agree with @Julientalbot that one shared OS user isn't a security boundary. one case worth spelling out in bot-screen.md: a same-user process can swap bot-desktop/rfb.sock for its own relay, the bridge will dial it (display.py:69-77), and it captures what the human types during the login the takeover exists for. "fail closed" in the lease.py docstring reads stronger than that.

regression tests to add with the fixes

  • closing the pane releases the lease (the real close() path, not a faked 1000)
  • /api/ws rejects display tickets
  • a foreign viewer_id gets no input
  • two starts on the same profile at once
  • a group with no connection doesn't match a remote bot
  • windows: computer_use works without fcntl

@Ganaderiapp

Copy link
Copy Markdown

Independent review round — Hermes agent fleet (6 workers), native Windows + executed probes, at d947fc83a461

Six agent reviewers covered: the screen runtime core (tools/bot_desktop/), the gateway/WS surface (web_routers/display.py, ws_tickets.py, tui_gateway/methods_display.py), the agent-side fencing (computer_use, browser_tool*), native Windows execution, the Desktop half (apps/desktop/src/plugins/hermes-bots/), and install/CLI/config/docs/tests. Everything below is pinned at head d947fc83a461f098066c71ee6e28161736b87f53; probes were executed locally (Windows 11, real venv, scratch HERMES_HOME, monkeypatched backends — no live Xvnc/Xfce on this host), everything else is marked as source-reading. IDs are ours; map freely.


W — Windows: the "native execution UNKNOWN" gap is now filled (blocking)

W1 · P1 · CONFIRMED by execution (extends @helix4u's and @MrD1az's F3, and @helix4u's second claim)

On native Windows (win32, Python 3.11.0), handle_computer_use({'action':'capture'}) raises:

  tools/computer_use/tool.py:251  from tools.computer_use.handoff import HANDOFF_ACTIONS, handle_handoff
  tools/computer_use/handoff.py:16  from tools.bot_desktop import lease as _lease
  tools/bot_desktop/lease.py:17  import fcntl
ModuleNotFoundError: No module named 'fcntl'

Through the real registry: model_tools.handle_function_call('computer_use', {...}) → {"error": "[TOOL_ERROR] Tool execution failed: ModuleNotFoundError: No module named 'fcntl'"}. The import at tool.py:251 runs for every action, before any check — capture, click, and handoff actions all die. Also confirmed natively: tui_gateway.server.handle_request({'method':'display.status'}) raises the same error via methods_display.py:52 → :42 → lease.py:17 (so no {supported:false} snapshot, no error 5300 — the RPC just dies), same for display.start on first call and for hermes computer-use screen status. And the two display tests cannot even be collected on Windows (test_display_ws_drop_keeps_lease.py:13 → same import), so that lane has zero coverage of it. Minor, from the same chain: display._bridge consumes the display ticket before the failure, so a valid ticket is burned instead of a clean close.

W2 · P2 · tooling gap that let W1 ship — python scripts/check-windows-footguns.py tools/bot_desktop/lease.py prints ✓ No Windows footguns found, exit 0, over a module with a bare import fcntl. The checker's 16 rules include os.killpg, os.fork, os.setsid, ~/Desktop, shebangs — but none for POSIX-only stdlib imports. One rule closes the class:

r"^\s*(?:import|from)\s+(fcntl|pwd|grp|termios|resource|pty|tty)\b"

W3 · P2 · candidate fix, verified natively — the exact gate @helix4u asked for. One file, no new deps, same shape as the in-tree precedents (cron/jobs.py, agent/trajectory.py):

--- a/tools/bot_desktop/lease.py
+++ b/tools/bot_desktop/lease.py
@@
-import fcntl
+# Cross-process advisory locking: fcntl on POSIX (the only host Bot Desktop runs on) or msvcrt on
+# Windows, degrading to no cross-process lock if neither exists — same shape as cron/jobs.py. The
+# import must NOT be unconditional: lease is imported at module load by the computer_use dispatch,
+# the browser fence and display.status, which all run on ordinary Windows hosts where the feature is
+# simply unsupported and must stay usable.
+try:
+    import fcntl
+except ImportError:  # pragma: no cover - non-Unix
+    fcntl = None  # type: ignore[assignment]
+try:
+    import msvcrt
+except ImportError:  # pragma: no cover - non-Windows
+    msvcrt = None  # type: ignore[assignment]
@@ class _locked
         self._fh = open(self._lockfile, "a+", encoding="utf-8")
-        fcntl.flock(self._fh.fileno(), fcntl.LOCK_EX)
+        self._fh.seek(0)
+        if fcntl is not None:
+            fcntl.flock(self._fh.fileno(), fcntl.LOCK_EX)
+        elif msvcrt is not None:
+            msvcrt.locking(self._fh.fileno(), msvcrt.LK_LOCK, 1)
         return self
     def __exit__(self, *exc):
-        fcntl.flock(self._fh.fileno(), fcntl.LOCK_UN)
+        if fcntl is not None:
+            fcntl.flock(self._fh.fileno(), fcntl.LOCK_UN)
+        elif msvcrt is not None:
+            msvcrt.locking(self._fh.fileno(), msvcrt.LK_UNLCK, 1)
         self._fh.close()

Verified natively with the patch applied: registry dispatch goes from [TOOL_ERROR] ModuleNotFoundError to a real capture ({"mode":"som","width":1456,"height":782,"app":"brave.exe",...}); display.status returns supported:false; display.start returns error 5300; the lease round-trips on the msvcrt backend (acquire/release, epochs correct). POSIX path unchanged (same two flock calls, now guarded). The other three import fcntl sites added by this PR are already correctly guarded (agent/trajectory.py, one test, runtime.py inside a function behind is_supported_host) — lease.py:17 is the only one that fires on import.

W4 · P3 — methods_display.py:41 sets _lease_listener_installed before the import that installs the listener; one failure poisons the flag for the process lifetime (observed natively). Move .set() below the successful install.


S — Settlements of the two disputes

S1 · P2 · @MrD1az's F4 stands — the portal is still path-only at this head. screen-portal.tsx:100-110 matches payload.profile_key === profileKey and does not import the predicate; isEventForBotScreen is used only in screen-pane.tsx:75 and screen-install.tsx:43,52; and git show --stat d947fc83 (the commit described as fixing this) does not touch screen-portal.tsx — its only commit is 5a3e63421d. Fix is one import + one condition, plus a regression test that exercises the portal hook.

S2 · P2 · @MrD1az's F5 confirmed on both sides. Desktop: detach() (screen-pane.tsx:81-87) does socket.current?.close() with no code, and the unmount effect (:159-161, comment: "only unmount tears it down (and hands control back server-side)") never calls display.lease.release; pinned @novnc/novnc@1.7.0 also closes its socket code-less (core/websock.js:297-302). Server: uvicorn reports that as 1005; web_routers/display.py:29 has _CLEAN_CLOSE = {1000, 1001}, so the lease stays held. Probe against the real bridge function: 1005 → holder=human, 1000/1001 → agent, 1006 → human. Net effect: the documented "Closing the pane also hands control back" (bot-screen.md:68) is false at this head; the shipped test parametrizes only 1006/1000, omitting the path the renderer actually produces. Suggested: release explicitly in the unmount effect only (not inside detach(), which attach()/Reconnect also call) with VIEWER_ID, or send a clean close(1000); optionally also treat 1005 as clean server-side as defense in depth. Do not weaken the 1006 branch.


N — New findings (executed evidence unless marked)

N1 · P2 · the human lease has no expiry, so it can wedge a bot forever. Only two release paths exist (the UI call, methods_display.py:168-174; and clean viewer close, display.py:152-153). A 1005 close, a viewer that vanishes without a clean close, or screen stop all leave holder:"human" with no recovery except UI action or deleting <HERMES_HOME>/bot-desktop/lease.json. Every computer_use call and every local browser command of that profile is then refused permanently — worst on unattended bots (cron/gateway) where nobody sees the pane. Suggest a viewer heartbeat (e.g. viewer_seen refreshed while the bridge pumps) with a bounded grace before the lease falls back to AGENT.

N2 · P2 · wait_for_human is uninterruptible and burns its full budget even when nothing happened. handoff.py:19-20 sets 600s default / 1800s max; lease.py:176-190 is a synchronous poll loop with no cancel flag — /stop or an interrupt cannot end it, so a gateway worker thread or cron tick blocks for up to 30 minutes. Worse: request_handoff only sets pending_handoff without moving the holder (lease.py:167-173), and the wait's early-exit requires holder == AGENT and pending_handoff is None — so an unanswered request guarantees the full-budget failure. Probe: request_handoff ok → wait_for_human(seconds=2) → 'still_waiting holder=agent'. Suggest: cancel-aware wait; distinct short outcome when the holder never changed; document the ceiling.

N3 · P2 · the epoch is checked after the pixels and the AX text are already produced (confirms @MrD1az's F1 with ordered evidence; extends it to capture_after). Probe with a backend that flips the lease mid-capture: order ['admitted_epoch_0','takeover_during_dispatch','persist_screenshot','aux_vision_routed'], result refused — and the screenshot file stays on disk; auxiliary vision was actually invoked. Same for type+capture_after: the follow-up capture and the full spilled accessibility tree (elements_*.json, where the human's typed text lands) survive the refusal. Thread the admitted epoch into _do_capture/_maybe_follow_capture and refuse before persisting/spilling/aux-routing — or unlink artifacts when the epoch moved.

N4 · P2 · a recycled launcher.pid gets os.killpg'd and its stale DISPLAY merged into the agent env. runtime.py:108-118 trusts psutil.pid_exists alone (no create_time/cmdline check). Probe: wrote an unrelated live process's pid into launcher.pid → status() says running:True, published_env() returns its DISPLAY, and stop() would have killpgd that group (runtime.py:303). Persist a process identity at spawn (create_time) or have the launcher verify itself.

N5 · P2 · Ctrl/⌘+W while driving the screen closes the user's local terminal tab. The canvas host borrows the terminal marker (screen-pane.tsx:287-289, data-terminal=""); closeActiveTab routes anything focused under [data-terminal] to closeActiveTerminal(), and @novnc's keyboard handler never calls preventDefault (0 occurrences in core/input/keyboard.js), so the app-level mod+w binding fires while the keystroke also reaches the remote. The marker's intended half (suppressing type-to-focus) works; give the screen its own marker and add it to composerFocusBlockedBySurface().

N6 · P2 · the control-taken eviction state is unreachable with the pinned noVNC, so displacement is silent. @novnc/novnc@1.7.0 core/rfb.js:944-945 dispatches disconnect with detail:{clean} only — there is no reason field, so reason.includes('control-taken') in screen-pane.tsx is never true; the overlay and the screen.controlTaken string (all 4 locales) are dead copy. A 4000 close arrives clean:true and the pane silently re-attaches as watch. Drive the state from the raw close code or the display.lease event instead.

N7 · P3 · viewer_id is client-declared and display.status discloses the holder's id. The id is taken verbatim from display.observe and pinned into the ticket (methods_display.py:105-107), input gating and eviction compare only that string, and display.status returns the full lease to any client — so a second authenticated client can read the holder's id, mint a ticket with it, and have its input forwarded without eviction. The fallback viewer-{rid} also collides for two clients both using JSON-RPC id 1. Derive the viewer identity server-side; don't return lease.viewer_id to non-holders.

N8 · P3 · display.lease.release without viewer_id releases someone else's lease. methods_display.py:172 maps an omitted id to None, and lease.release(None) skips the holder guard entirely (lease.py:158-160) — any authenticated RPC client can hand control back out from under the human. Require a non-empty id in the RPC (the internal display.stop path can keep calling the Python function directly).

N9 · P3 · RFB clipboard framing still has no cap (re-confirms the J7/carlotestor/helix4u thread with numbers). rfb_filter.py:82-87 returns 8 + abs(n) for a signed int32, so a watcher holding no lease makes the serve process retain the declared payload before it can frame/drop it: measured retained=4,194,312 for a 4 MiB feed, zero bytes forwarded, no rejection; growth is cumulative (~2 GiB per connection bounded only by the int32) and passes under uvicorn's WS max because it happens pre-frame. Extend the same class to SetEncodings (262,144). Qualification so this isn't oversold: it is 1:1 with bytes sent (no amplification) and needs a valid ticket. Cap the declared length (TigerVNC's own MaxCutText default is 256 KiB) and close via the existing ValueError → 1003 path; add an invariant test feeding a 2 GiB header.

N10 · P3 · direct agent-browser/Chrome spawns bypass the browser fence (extends F2). The fence covers _run_browser_command only; browser_tool_real_profile.py:59,201-206 spawns agent-browser directly (session create, open about:blank, cdp lookup, close) with no assert_agent_may_act, and the lightpanda fallback navigation (browser_tool_lightpanda_fallback.py:150-183) runs inside the fenced branch but already hit the bot's display before the result is voided. Fence the session-create path; move the epoch check ahead of the side-effect stage.

N11 · P3 · hermes computer-use screen stop never releases the lease, though docs and --help say it "hands control back first". _screen_stop calls only runtime.stop(); the gateway display.stop does release first (methods_display.py:87). Fail-closed wedge, not a hole. Call lease.release() before runtime.stop() or fix the text (bot-screen.md:97,99,142-143).

Smaller confirmed items (one line each, all with repro in our reports):

  • N12 launcher.pid carrying a huge digit string ('99999999999') raises OverflowError out of status()/start()/stop() (runtime.py:113-118, psutil.pid_exists); bound the parse.
  • N13 lease._read promises fail-CLOSED but valid-JSON non-objects raise AttributeError, and a non-int epoch raises TypeError in _transition; lease files are written 0644/0666 and bot-desktop/ only gets 0700 when runtime.start() runs — chmod on write, validate the shape, widen the except.
  • N14 the RFB handshake is hardcoded to 14 bytes: an RFB 3.3 client's first message byte is rewritten (13-byte handshake), which Xvnc rejects; we could not turn this into an input bypass (the rewritten byte is always forced to 0x01), so it fails closed — but it's a 1-byte misalign for real 3.3 clients.
  • N15 no supervisor for the launcher pair: the EXIT trap is discarded by exec dbus-run-session (launcher.sh:232,248), so an Xvnc crash leaves running:True with a dead display, and a launcher death orphans Xvnc holding the display number; status() should treat a missing socket as not-running.
  • N16 display-number allocation reserves nothing between the flock release and Xvnc creating /tmp/.X{n}-lock (runtime.py:135-148); two profiles starting in that window both pick the same number and one fails to start.
  • N17 an unknown profile name escapes the display handlers' try/except and surfaces as -32603 internal error + "ws dispatch crash" instead of the handlers' 5300 (methods_display.py:49-56, server.py:514-520).
  • N18 a bot-desktop display ticket is also accepted as a general /api/ws credential (any minted ticket passes _ws_auth_reason with no purpose check, stamping provider:'bot-desktop'); bounded (the holder was already authenticated) but worth a purpose/route field.
  • N19 hermes computer-use screen install is a second implementation that bypasses install._running (a CLI install can overlap a Desktop one) and uses shell=True where install.py spawns list-form; package names are constants so no injection — route it through install.install_packages.
  • N20 tests/tools/test_bot_desktop_runtime.py drives fcntl-only code but carries no linux_only marker: fails natively on Windows (observed ModuleNotFoundError inside _allocate_display), and list_os_marked_tests.py never places it in an OS lane.
  • N21 the portal's null-connection group box can bind another connection's same-named row (screen-portal.tsx:161-171, route.connectionId === null || short-circuits); require the connection to agree.
  • N22 i18n: complete for the 4 shipped locales (verified leaf-parity), but screen.recheck is dead copy and screen-open.tsx:22's notice is a hardcoded English string.

Note (not a finding) — the ci-review-bot "Action required: package-lock.json" item checks out as benign on inspection: the @novnc/novnc 1.7.0 entry matches apps/desktop/package.json, and the apps/bootstrap-installer 0.0.1→0.21.1 row is a pre-existing stale-lock refresh (package.json already read 0.21.1 at base) — a one-line PR note should clear it for the ci-reviewed label.


Verified good (checked, no action)

  • Ticket flow: one-shot is atomic under a 20-thread race (1 winner), TTL 30s (29/30 consume, 31 rejects), provider=='bot-desktop' + hermes_home checked server-side; ticket rides the query string but no leak path found in Hermes' own logs (log_level='warning', audit drops the ticket field) — residual proxy/history exposure unmeasured.
  • RFB input gate holds for every input type across byte-at-a-time chunking; the extended-clipboard framing matches TigerVNC's reader exactly (no parser differential).
  • Install path: sudo card is confided to the installing connection (origin routing verified in prompt-overlays.tsx + transport capture); an empty password answer spawns nothing; the per-profile slot is released in a finally; docs package tables match runtime.PACKAGES element-for-element on all three distros; "nothing installs on hermes update" holds.
  • bot_desktop config section needs no _config_version bump (per repo convention) and the defaults are safe.
  • Hero staleness (J9) and the sudo/install origin fix (J10) hold at this head.

Not verified

Live Xvnc/Xfce/RFB end-to-end (no Linux host available here); the POSIX cross-process flock behavior (stubbed on Windows); proxy/LB access-log exposure of the ticket; anything explicitly marked "static" above.


Automated fleet review (6 Hermes agent workers: runtime core, gateway/WS, agent fencing, native Windows, desktop UI, install/CLI/docs/tests), run on a Windows 11 host, all probes at d947fc83a461. Reports, probe scripts and the candidate patch are local — happy to paste any detail in this thread.

@eynaudg

eynaudg commented Sep 12, 2026

Copy link
Copy Markdown

Homer review (native Windows 11 + packaged Desktop)

Tested d947fc83a461f098066c71ee6e28161736b87f53 on a real Windows 11 host
(Hermes venv Python 3.13.13 and uv CPython 3.11.15). I am not repeating the
Linux-host P1s already on this PR.

Host: Windows 11, Hermes Agent v0.21.2 (2026.9.11) packaged Desktop at
819988ac (no Bot Screen UI in this build). Scratch checkout of this PR only.

New evidence

P1 confirmed on native Windows (was UNKNOWN):
from tools.bot_desktop import lease and handle_computer_use({'action':'capture'})
both raise ModuleNotFoundError: No module named 'fcntl' before backend
dispatch, with Bot Screen unused.

Chain:

tools/computer_use/tool.py:251  from tools.computer_use.handoff import ...
tools/computer_use/handoff.py:16  from tools.bot_desktop import lease as _lease
tools/bot_desktop/lease.py:17  import fcntl
ModuleNotFoundError: No module named 'fcntl'

Same crash on:

  • Hermes venv C:\Users\guill\AppData\Local\hermes\hermes-agent\venv\Scripts\python.exe 3.13.13
  • uv Windows CPython 3.11.15 (pytest collection)

This matches @helix4u / @iamlukethedev / @Xipong / @MrD1az F3. Their probes
stubbed fcntl on macOS. This is the real interpreter.

display.status never reaches the designed unsupported payload.
runtime.status() already works on this host:

DesktopStatus(profile='homer', supported=False, installed=False,
  missing=['Xvnc', 'xfwm4', 'xfce4-panel', 'xfdesktop', 'xfsettingsd',
           'dbus-run-session', 'xauth', 'xdpyinfo', 'setxkbmap'],
  running=False, pid=None, display=None, socket=None, geometry='1440x900',
  install_command=None)

Then _display_snapshot() (tui_gateway/methods_display.py:32) imports lease
and raises the same fcntl error. The handler catches it as RPC 5300
"No module named 'fcntl'", so the pane cannot show
“No bot screen on this host” / supported: false. Gate the lease import
behind the supported-host path (or make lease.py importable without fcntl).

PR tests on native Windows (uv run --with pytest, -o addopts=):

Result Test
ERROR collect tests/tools/test_bot_desktop_lease.py (import fcntl)
ERROR collect tests/hermes_cli/test_display_ws_drop_keeps_lease.py (import fcntl)
FAILED test_recorded_display_held_by_a_live_server_is_not_reused (runtime.py:139 fcntl)
PASSED test_no_running_screen_returns_none_without_grabbing
PASSED test_empty_password_cancels_without_spawning
PASSED test_second_install_for_same_profile_is_refused
PASSED test_display_ticket_must_be_a_bot_desktop_ticket_pinned_to_a_profile_home
PASSED test_install_worker_keeps_the_requested_profile_scope

Desktop viewer (Windows packaged app → Linux home gateway):
This install is v0.21.2 / 819988ac. apps/desktop/src/plugins/hermes-bots/ has
no screen-pane.tsx and no “Open Screen” string. I did not side-load the PR
renderer into the running app (would kill the live session). Live noVNC / Take
over / pane-close 1005 / remote /api/display/ws ticket were not exercised.
After this PR ships, the viewer path looks correctly wired for remotes:

  • resolveScreenWsUrl uses resolveSiblingWsUrl({ connectionId, profile }, '/api/display/ws', { stripGatewayCredential: true }) then url.searchParams.set('display_ticket', ticket) (screen-connection.ts:84-98)
  • canvas host has data-terminal so type-to-focus should not steal keys (screen-pane.tsx:287-289)
  • detach() still calls socket.close() with no code (screen-pane.tsx:85) — already filed as 1005; not live-confirmed here

Pane close / 1005: not exercised (no Open Screen in this Desktop build).

Not re-reported

RFB clipboard cap, real-profile CDP fence, epoch-before-vision, in-flight
input cancel, portal isEventForBotScreen, alloc lock, CLI screen stop,
XAUTHORITY races — already filed.

Ask

Please gate lease/fcntl so Windows computer_use keeps working, and so
display.status can return the supported: false snapshot this host already
computes. Happy to send a tiny follow-up if useful.

@BearHuddleston BearHuddleston left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact head d947fc83a461f098066c71ee6e28161736b87f53. Read the existing top-level discussion, formal reviews and inline threads before investigating, then refreshed them after both review workers finished and validated their findings.

Three inline findings remain after deduplication: two additional Desktop lifecycle/identity defects and real execution of the previously UNKNOWN human-first dock-browser handoff. I am not re-filing the Windows import, lease/fencing, RFB buffer, portal connection, close-code, allocator, sessionless sudo, or other already-covered issues.

Verification:

  • The nine focused Bot Desktop/display Python files pass via scripts/run_tests.sh: 19 passed, 0 failed.
  • Executed React/TypeScript component, gateway-pool and event-bus probes: reproduced the missing route retention and cross-bot thumbnail reuse. Retaining the route was a passing positive control. The sessionless sudo probe also reproduced an existing report and is excluded from the inline findings.
  • Real agent-browser 0.26.0 and Chrome for Testing 151.0.7922.34 on isolated Xvfb: the human-first handoff assertion fails; agent-first and same-session recovery after closing only the human-launched browser succeed. Temporary HOME/HERMES_HOME and user/network namespaces; no external page or credentials.
  • Additional corroboration of Xipong's existing review, not new findings: real cua-driver 0.26.1 stays on the original 640×480 display after publishing an 800×600 Bot Screen, while a fresh session captures 800×600; the real launcher with extracted Xvnc and a controlled panel exit leaves a responsive X server after runtime.stop() returns false. These external behavior-contract probes intentionally fail on this head; they are not passing regressions.

Limits: no full Electron/noVNC/Xfce acceptance run or full repository suite. Desktop probes used transport/UI-boundary doubles; browser setup/discovery was controlled, but the Chromium/agent-browser execution was real. Exact-head CI reports required checks passing, with Desktop E2E skipped. No tracked source edits, commits, pushes, host package installation, or live-profile changes.

Comment thread tools/bot_desktop/runtime.py Outdated
Comment on lines +271 to +274
from tools.bot_desktop.browser import dock_launch
if (browser := dock_launch()) is not None:
# first-run / default-browser dialogs would sit between the human and the bot's tabs
child_env["HERMES_BD_BROWSER_EXEC"] = f"{browser[0]} --user-data-dir={browser[1]} --no-first-run --no-default-browser-check"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Make the human-first dock browser attachable by automation

This launches raw Chromium with the shared profile but no automation endpoint. On a fresh bot, Take over → open the dock's Browser before any agent-browser call → Hand back leaves that Chromium owning the profile singleton. The subsequent agent-browser launch exits through the singleton without obtaining CDP, so the bot cannot continue browsing while the human-opened instance remains running.

I executed this dock argv and the real _run_browser_command/environment/process path using agent-browser 0.26.0 and Chrome 151.0.7922.34 on isolated Xvfb. The result was success:false, Chrome exited early (exit code: 0) without writing DevToolsActivePort. The original browser stayed alive and had no DevTools port file. Closing only that browser made the next call in the same profile and agent session succeed; an independent agent-first control also succeeded. No live login or UI acceptance run is claimed. This supplies the real execution missing from the earlier UNKNOWN singleton concern.

Please have the dock open an automation-owned browser, or implement an explicit shared-browser attachment lifecycle. A common binary/user-data-dir alone does not establish an automation connection; cover both launch orders.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d732fad0af00 (rebased into d65af42). The dock launcher now runs Chromium with --remote-debugging-port=0; tools/bot_desktop/browser.py::running_instance_cdp_port trusts DevToolsActivePort only when the SingletonLock pid is alive and the port accepts a connect (and excludes the instance the calling agent-browser session launched itself), and the local argv gains --cdp <port> when that instance exists. Live on this host, both orders: human opens the dock browser first → browser_navigate succeeds in the same single Chromium process; agent first → dock click joins the agent instance and later navigates still succeed. Before the fix the human-first order failed exactly as you described (exit 21, no DevToolsActivePort).

Comment on lines +75 to +77

try {
await displayRequest(bot, 'display.install')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Retain the gateway route while Screen waits for events

Open an inactive local bot's Screen directly without activating its chat. This RPC uses a pooled secondary, but neither the pane nor this card holds host.retainProfile. requestGatewayForAgent releases its request lease in finally and disposes an otherwise unowned socket as soon as display.install acknowledges. The actual install runs in a background worker, so subsequent sudo/log/done events have lost their receiving transport: the card stays Installing with its button disabled. display.observe has the same lifetime problem for subsequent lease events even while the separate RFB socket remains open.

A probe through the actual install component, display routing, gateway pool and event bus (transport doubled) reproduced closure at ACK, loss of the completion event and the stuck button. Holding the existing retainGatewayForAgent('local', 'worker') as a positive control delivered completion and re-enabled the button.

Acquire host.retainProfile(botScreenRoute(bot)) before starting this event-driven operation and keep it for the appropriate Screen/install lifetime, releasing on cleanup. This is separate from the already-reported sudo prompt's chat association and the corrected worker-context propagation.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1eed9c82e27d (rebased into d65af42). retainBotScreen(bot) feature-detects host.retainProfile the way group-turns.ts does; the install card retains before display.install and releases on done/failed/unmount, and the pane holds a retention for the attach lifetime (acquired before display.observe, released on detach/unmount). Test: screen-install-retention.test.tsx asserts retain-before-request and release-on-done.

Comment on lines +29 to +35
useEffect(() => {
if (!running) {
setDataUrl(null)
setMisses(0)

return
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Clear the thumbnail when its owning bot changes

RoutinesPane renders this hero without an owner key (cron.tsx:1245), so selecting another bot reuses the hook state. If both bots have cached running status, running stays true and this branch never clears dataUrl. The newly selected bot B consequently shows bot A's pixels beneath B's current status/CTA until B's first thumbnail succeeds. A slow or failing B can keep displaying the wrong bot's screen, not merely an old frame from B.

I reran a probe rendering the actual ScreenHero: after A's thumbnail resolves, rerender with B and defer B's response. The image remains A's and the caption still says Live; resolving B finally replaces it. Thumbnail transport and portal tone were controlled; the unkeyed owner-change caller was source-traced.

Reset image/failure state on the connection+profile identity change, or key the hero by that identity. Add a switch-between-two-running-bots test with B's response pending/rejected. This is distinct from the earlier same-bot failed-refresh staleness report and the portal's wrong-host event filtering.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by @whyyagswhy's 7bc75c0 (cherry-picked with authorship, rebased into d65af42): ScreenHero is keyed by botSelectionKey(bot) so dataUrl/misses/stale reset on an owner change, and a late thumbnail from the previous bot is discarded. Test: screen-isolation.test.tsx "hero reset on bot switch" + "late-thumbnail discard".

@harsipsum

Copy link
Copy Markdown

@teknium1 trying to test this on a Nous-hosted Hermes instance, with Hermes Desktop on Windows as the viewer. What is the supported way to get the matching gateway preview onto the managed Cloud instance?

The Desktop side has been updated to this PR at d947fc83a461f098066c71ee6e28161736b87f53. I also used Update in Nous Portal, but the Cloud-side checks afterward still return:

Hermes Agent v0.21.2 (2026.9.11) · upstream 939e45c9
Install directory: /opt/hermes
Install method: docker

Running the documented hermes computer-use screen status on that Cloud host returns:

hermes computer-use: error: argument computer_use_action: invalid choice: screen (choose from install, status, doctor, permissions)

The installed host also lacks tools/bot_desktop/runtime.py, tui_gateway/methods_display.py, and hermes_cli/web_routers/display.py.

I understand the PR docs explicitly support Hermes Cloud connections, and Install on host installs the Linux desktop packages once the gateway has the new handlers. The missing part here appears to be getting that gateway code onto the hosted instance, not running Linux on my Windows viewer.

Can Nous enable a matching preview deployment, or is there a supported self-service branch/preview option I have missed? Happy to test the actual screen takeover, manual login, hand-back, and continued Cloud operation with the PC off. Related to #92524; posting here rather than opening a duplicate.

@alt-glitch alt-glitch added the sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows label Sep 12, 2026
… instead of silently ignoring them

profile_dir() accepted only an absolute AGENT_BROWSER_PROFILE; `~/pin` and `pin`
fell back to the default user-data-dir while the docs said setting the variable
pins your own. The dock's Browser icon and agent-browser both derive their
identity from profile_dir(), so the silent fallback was at least confusing and,
combined with any other consumer of the raw value, a way to end up on two jars.

`~` is expanded and a relative path is anchored at the profile's HERMES_HOME (the
same root runtime.state_dir() uses), so per-profile pins stay per-profile. The
docstring and the bot-screen docs state the resolution rule.

Contributor PR #112708 (@Rook-CodeVolt) proposed documenting "absolute only";
this takes the behavioural fix instead.

Closes #110029
…opping the viewer's stream

launcher.sh starts Xvnc with -AcceptSetDesktopSize so the viewer can fit the
screen to its pane, but rfb_filter.py had no frame for client message type 251
and raised "unknown RFB client message type 251", closing the WebSocket on the
first resize request.

SetDesktopSize (u8 type, pad, u16 width, u16 height, u8 nScreens, pad, then 16
bytes per screen) is framed like the other variable-length client messages and
forwarded intact; it carries no input, so watchers without the lease send it too.
A truncated message waits for the remaining bytes.

Closes #110039
…creen like computer_use does

bot_desktop.auto_start had one trigger, computer_use dispatch. The browser tool
built its headed child env from desktop_env(), which only reads the env the
launcher already published, so on a fresh headless profile with browser.headed
and auto_start both on the first browser use got no DISPLAY and nothing said why.

_build_browser_env() now calls the same ensure_started_for_tool() hook when a
headed browser is requested (headless browsing never brings a screen up), with
the hook's own failure handling — a failed start falls through to the tool's
usual "no display" diagnosis, exactly as for computer_use. Docs updated: "first
use" covers the first computer_use call or headed browser use.

Closes #110050
…creen; the env builder stays pure

_build_browser_env() is shared by the npx cache warmer (hermes update / doctor
--fix), the lazy Chromium auto-installer, the Lightpanda engine's env and
browser_use_cli. Hooking bot_desktop.auto_start there made every one of those
block up to 15 s spawning Xvnc+Xfce with browser.headed on, against
desktop_env()'s own "never starts anything" contract.

The hook now sits where a headed Chromium is actually spawned for a tool
action, mirroring computer_use dispatch: _spawn_and_collect (the first
agent-browser command forks the daemon; Lightpanda engine excluded), the Chrome
fallback from Lightpanda, and the real-profile Chrome launch. The regression
test asserts both halves: the env builder never starts the screen, the headed
Chromium spawn does, a headless or Lightpanda spawn does not.
… to Xvnc

Framing client message 251 (so the stream no longer dies) is kept, but the
message resizes the bot's framebuffer under a working agent — Xvnc runs
-AcceptSetDesktopSize — so forwarding it from a viewer that does not hold the
lease was a fail-open flip for a server-mutating message. The shipped pane
sets resizeSession=false anyway; SetDesktopSize now sits in _INPUT_TYPES and
only the lease holder's resize reaches the server. Test covers both legs.
… the socket it binds

_reap_orphaned_server only knew the orphan via /tmp/.X<n>-lock, so a launcher
SIGKILLed while a /tmp cleaner also took the lock (case B of #109941) — or a
failed start() that dropped <sd>/display — left one live Xvnc leaked per
occurrence; the allocator merely moved to the next number. When the lock names
nothing alive, the reaper now scans for the Xvnc whose command line binds this
profile's rfb.sock (the same ownership proof it already required), and skips the
lock unlink when no number is recorded.
…live

The janitor reaped the bot's headed Chromium after 120 s of AGENT inactivity — which is
exactly the state a human takeover (login, 2FA) puts the agent in — so the browser died
under the human mid-login. The janitor now counts a human-held lease as activity for the
browser the human shares with the bot, and the agent-browser daemon's own idle timer
(which cannot see the lease) steps back on the Bot Desktop so the lease-aware janitor
owns that browser's lifetime; a crashed hermes still leaves it to the orphan reaper.

Fixes #110064
…-literal guard

Main now rejects literal /tmp paths in favour of the scratch-dir resolver. The
X11 protocol fixes .X<n>-lock and .X11-unix/X<n> under /tmp; these lines detect
or document that location, they do not pick scratch space.
…top (opt-in)

Watching the bot drive its screen is the point of Bot Screen, but until now
you learned it had happened only afterwards. With "Open Screen when the bot
uses it" checked on a bot's row menu, the first live tool.start for a screen
tool (computer_use, browser_*) on a session the bot owns brings its Screen
tab forward.

Why opt-in and fenced: Desktop's rule is offer, don't hijack. The raise is
per bot (BotMeta.screenAutoOpen, rides profile ui_meta like pin/hide), never
moves keyboard focus (openBotScreen reveals the pane; the viewer grabs keys
only on Take over), never fires for replayed history (the reconnect replay
re-dispatches parked frames, so the wake is rate-limited to one per bot per
30 s rather than trusting seq), does nothing while the tab is open, and a
manual Close mid-run holds until the run has been quiet for a cooldown.

Live (isolated headless Electron + real serve backend, CDP): opt-in off →
tool.start opens nothing; toggled via the real menu → toast + aria-checked
true → tool.start opens "Hermes · Screen" (Screen is off state), focus stays
on the row; second call is a no-op; real Close → held at +2 s and +29 s,
raised again at +31 s.

Rule borrowed from thomasbek3/hermes-bot-kit computer-viewer's auto-connect.
…lution ratchet

Main's hermes_platform ratchet (tests/test_managed_runtime_resolution.py) flags every
bare shutil.which outside hermes_platform/. Bot Screen's four sites probe Xvnc,
xfce4-session, a system Chromium and apt/dnf/pacman on the gateway host, which is
the control host the resolver describes; none of them installs or starts anything
and the install path runs only after the user's sudo approval.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles ci-reviewed applied to manually approve dangerous changes comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/desktop Electron desktop app (apps/desktop/*) comp/tools Tool registry, model_tools, toolsets comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/browser Browser automation (CDP, Playwright) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.