Skip to content

feat(browser): add authenticated extension controller - #85351

Closed
abundantbeing wants to merge 6 commits into
NousResearch:mainfrom
abundantbeing:feat/browser-extension-controller
Closed

feat(browser): add authenticated extension controller#85351
abundantbeing wants to merge 6 commits into
NousResearch:mainfrom
abundantbeing:feat/browser-extension-controller

Conversation

@abundantbeing

@abundantbeing abundantbeing commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds an opt-in browser-extension controller lane so Hermes can route existing browser_* tools to the exact authenticated browser session that opened the conversation.

The implementation has two layers:

  1. A transport-neutral broker with principal/profile/session/controller/browser-profile/transport scoping, one-shot WebSocket tickets, capability allowlisting, command lifecycle, cancellation, timeout, reconnect, and owner-scoped detach.
  2. Request-bound tool routing for the seven existing Browser Use registry schemas. Generic requests preserve the existing backend; once the gateway binds controller identity, the extension lane is authoritative and fails closed if that exact controller disappears or cannot execute the action.

Local API and authenticated dashboard/cloud transports use the same protocol and real-action allowlist:

  • browser_navigate
  • browser_snapshot
  • browser_screenshot
  • browser_click
  • browser_type
  • browser_press
  • browser_scroll
  • browser_back
  • browser_list_tabs
  • browser_activate_tab

Raw CDP, arbitrary evaluation, console, file upload, vision, and image extraction are not admitted.

Reconnect and detach semantics

Unexpected transport loss is recoverable: the controller is hidden from new dispatch while already-started commands remain pending until their original deadline. A reconnect with the same stable identity refreshes the transport and negotiated capabilities, flushes deferred cancels before new work, and can complete the original command.

An authenticated browser.controller.detach frame/RPC remains immediately terminal. A different controller id or browser profile in the same authenticated session lane is also a hard replacement: the old controller's pending work is cancelled before the successor becomes routable. Inbound heartbeat, result, cancel, and detach frames are admitted only from the current owner.

Slow but live cross-thread WebSocket writes remain in flight under the broker command deadline instead of being misclassified as failed sends. Real send failures still surface immediately.

Existing user compatibility

  • browser.extension_control.enabled defaults to false.
  • Feature off: the broker is never queried and the current Browser Use path is unchanged.
  • Feature on with no server-bound controller identity: the current backend still handles the tool.
  • Feature on with bound controller identity: missing, ambiguous, disconnected, or incapable controllers fail closed instead of switching browsers.
  • Controller tools are exposed only inside the matching request/session context; availability is never cached process-wide.
  • Once a request is bound to the controller lane, failures do not silently jump to another browser.
  • Server-bound browser-control identity variables are excluded from shared shell snapshots, preventing cross-session routing metadata persistence.

Tests

A real browser_snapshot journey covers the aiohttp route table, Bearer-authenticated registration, one-shot subprotocol ticket, controller WebSocket command/result exchange, router serialization, and zero calls to the legacy backend.

Reconnect coverage includes socket close and dashboard transport loss, same-identity capability renegotiation, deferred-cancel ordering and bounds, stale-owner rejection, explicit detach, different-identity hard replacement, timeout, late completion, and slow WebSocket send waits.

The diff is 23 files with 4,873 additions; 2,553 additions are tests and 101 are docs/config.

Resolves the Chrome-extension backend portion of #84000. This uses the existing authenticated API/dashboard transports instead of adding a second native-messaging server inside Hermes.

Related issue(s)

#84000

Checklist

  • I have performed a self-review of my changes
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated documentation as needed
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have run code quality checks locally and they pass
  • I have not committed sensitive data or credentials
  • My commit messages are clear and descriptive
  • I have credited co-authors where applicable
  • Any generated or vendored files are reproducible and justified in the PR description

Testing evidence

  • Controller/broker/API/cloud/router plus snapshot-identity and bound-route authority regressions: 95 passed, 1 POSIX-only integration skipped on Windows
  • Registry/cache/model/API compatibility: 93 passed
  • RED → GREEN: 6 strict admission failures → all green
  • RED → GREEN: real browser_snapshot WebSocket journey failed when the action was removed, then passed with 0 legacy-backend calls
  • RED → GREEN: 4 reconnect lifecycle contracts failed on the old broker, then passed
  • RED → GREEN: stale-scope cancel, completed-send TimeoutError, and zombie-controller replacement regressions
  • RED → GREEN: two bound-controller authority cases failed by invoking legacy fallback, then passed fail-closed; a real schema-build → disconnect → dispatch regression proves zero fallback calls
  • First public CI head exposed HERMES_BROWSER_CONTROL_* snapshot exclusion drift; reproduced locally and fixed in the fourth commit
  • ruff check on every touched Python module/test: clean
  • compileall on every touched Python module/test: clean
  • Docusaurus English production build: successful (only the repo's pre-existing /docs/llms.txt and /docs/llms-full.txt root-page warnings)
  • Config example parses with the feature disabled
  • GitNexus change analysis: low risk, no affected existing process flow
  • Added-line secret scan and git diff --check: clean

Additional context

The feature is deliberately opt-in and fail-closed on identity/capability mismatch. Registration validates protocol version 1 strictly (booleans are rejected), requires at least one permitted capability, and uses a 30-second one-shot ticket carried only in Sec-WebSocket-Protocol. Query-string tickets are rejected.

This branch is based on current main. The first CI head's only failing Python slice was the new browser-control ContextVars missing from shared terminal snapshot exclusions; the corrected head includes that regression fix.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) tool/browser Browser automation (CDP, Playwright) area/config Config system, migrations, profiles area/auth Authentication, OAuth, credential pools needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #84000: this PR provides an opt-in authenticated extension-controller lane for the requested shared visible-browser workflow.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

feat(browser): add authenticated extension controller

  1. Clock mismatch in the advertised ticket expiry: the broker's clock is time.monotonic by default (gateway/browser_control_broker.py BrowserControlBroker.__init__), but _handle_browser_control_register advertises ticket_expires_at = time.time() + ticket_ttl (gateway/platforms/api_server.py). Both advance at the same rate so ticket_expires_in_seconds is accurate, but the absolute ticket_expires_at is wall-clock while the broker enforces expiry on monotonic — an NTP step or manual clock change makes the advertised deadline diverge from the enforced one. Either expose the broker's own deadline (ticket.expires_at is monotonic-based too) or document that the advertised absolute time is best-effort.

  2. Ticket minting is unthrottled and each mint is O(n): every valid authenticated POST /v1/browser-control/register mints a ticket with no per-identity rate limit, and mint_ticket runs _prune_tickets, which scans all live tickets — so sustained minting from a leaked/compromised API key yields O(n²) prune work and unbounded (TTL-bounded) ticket accumulation. Consider a per-principal in-flight ticket cap (e.g. reject when a principal already holds N un-consumed tickets).

  3. browser_control_enabled re-reads config on every call: both the broker helper and the adapter's _browser_control_enabled() call load_config() per request. If load_config does a YAML merge per call, every /v1/browser-control/* request pays a config load. Consider a TTL cache (or reading the value once at adapter init, re-checked on a shorter cadence) — the feature is off by default, so this path is cold unless enabled, but it's still per-request work on the enabled path.

  4. Capability renegotiation vs. pending completion (worth a test): ControllerScope.__eq__ includes capabilities, and attach reassigns pending.scope to the new scope on reconnect. The reconnect test covers same-capability reconnects; a reconnect with a changed capability set (extension renegotiates mid-command) would make the second socket's complete(scope=…) fail its scope equality check against the reassigned pending scope and drop the result. Verify this path is intended (the identity contract says capabilities aren't identity, but completion scope-checks them) and add a test.

@abundantbeing
abundantbeing force-pushed the feat/browser-extension-controller branch 2 times, most recently from ee6bac4 to 3002493 Compare August 19, 2026 15:20
@abundantbeing

Copy link
Copy Markdown
Contributor Author

CI note for the maintainers: the failing slice on this head is unrelated to this PR's diff and reproduces on clean main (b5455fdd1):

  • tests/tools/test_image_generation.py::TestFalCatalog::test_upscale_defaults_are_all_off — reproduced locally against the branch's code, which does not touch test_image_generation.py or the FAL catalog; the assertion fails on the xai/grok-imagine-image/v2.0 catalog entry itself.
  • Clean main's own CI also reports a failing Python-tests slice (slice 12/12 on b5455fdd1).
  • Other flakes observed in the same runs: test_transcription_tools.py::TestRunCommandSttIdleTimeout (timing), test_goal_continuation_drain.py (queue key), and a GitHub 429 during checkout.

A CI re-run (maintainer-only; the fork token gets 403 on actions/rerun) should clear it. Happy to force-push a no-op refresh if that's easier.

@abundantbeing
abundantbeing force-pushed the feat/browser-extension-controller branch from 3002493 to cb2e881 Compare August 19, 2026 16:10

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact head cb2e881ccd32d3262120abb83c3845083f2f025a against its base 00e5a361b60f621ae2246dffdd0a0252895d8493 and current main 6c6c17e0fc1fdb48cd4f7b50d5e263ec86acee5d.

The controller/broker direction is strong: server-derived principals, exact session/controller/browser-profile/transport identity, one-shot WS tickets, fail-closed bound routing, owner-aware reconnect/detach, and capability filtering are the right primitives. I also checked the earlier automated reconnect/capability-renegotiation concern: attach() rebinds matching pending commands to the successor scope, so I am not repeating that advisory as a blocker.

I found two merge blockers, both in the new artifact boundary:

1. HTTP-uploaded artifacts and broker-dispatched artifacts are bound to different scope keys

artifact_scope_key() hashes (principal_id, session_id, transport_family). The API upload/download handlers store and load with _ArtifactScopeFacade(principal, transport_family=...), i.e. empty session_id. But the broker's _validate_artifact_reference() validates the same artifact with the full ControllerScope, whose session_id is necessarily non-empty because controller registration requires an existing session.

That means the intended end-to-end path cannot compose: an artifact successfully uploaded through /v1/artifacts/upload is principal/family-scoped, then browser_artifact_upload / browser_artifact_download validates it under principal/session/family and gets ArtifactScopeMismatch. The tests currently prove two separate contracts rather than the real journey: the HTTP round-trip never enters broker validation, while the broker test seeds the store directly with a session-bearing _Scope() that already matches _broker_scope().

Required: choose one canonical artifact ownership projection and use it on both sides. If artifacts are session-owned (which matches the broker identity model and the file's own docs), bind the HTTP artifact request to a server-owned session and persist that exact scope. If they are intentionally principal/family-owned, the broker must validate against that same projection and the security/docs need to say so. Please add the missing real regression: authenticated HTTP upload → registered controller scope → broker artifact dispatch (and the reverse download path).

2. The “profile-scoped” artifact store is actually first-profile-wins process state

APIServerAdapter owns a single _browser_control_artifacts. _artifact_store_for(profile) returns that singleton as soon as it exists; only the first call resolves get_profile_dir(profile) and constructs <profile>/artifacts/browser-control. The same first store is then attached process-wide to the global broker.

On a multiplex API listener, profile A touching the artifact route first therefore pins profile B to A's physical artifact root. The principal hash prevents a simple logical cross-read, but the bytes are still written into the wrong profile home and B's broker dispatch is still validating against A's store. This is the same frozen-handle class Hermes already repaired for session storage in merged #88734 (salvaging @jackulau's #88632): a context/profile-sensitive path cannot be resolved once and cached process-wide.

Required: cache stores by resolved profile/home, not once per adapter, and make broker artifact lookup select the correct store from the controller/profile scope rather than holding one global store. Add an A/B multiplex regression proving distinct physical roots and successful broker dispatch for both profiles regardless of which profile touches the endpoint first.

Topology / credit

  • #84000 by @SolshineCode is the source feature request; #85351 by @abundantbeing is a substantially broader authenticated implementation rather than a literal native-messaging backend.
  • #88203 by @abundantbeing is complementary loopback pairing/token UX on the same api_server.py surface. It is independently mergeable, but whichever lands second needs semantic composition around auth/profile admission; it does not repair the artifact ownership defects above.
  • #88734 by @teknium1, preserving @jackulau's #88632 authorship, is the directly relevant per-profile physical-storage invariant: one active profile must not inherit another profile's cached home-bound handle.

CI truth

Exact-head Docker and Nix are green. Hosted CI executed the real matrix: lint, supply-chain, E2E, Windows-only and macOS-only lanes are green. The returned Python slice 9/12 completed 2,595 passed / 1 failed / 14 skipped; the sole failure is the unrelated current image catalog assertion that xai/grok-imagine-image/v2.0/text-to-image defaults upscale=True, in tools/image_generation_tool.py, which this PR does not touch. I am not treating that as a #85351 regression.

Re-review gate: fix the canonical artifact-scope composition and per-profile store ownership, add HTTP→broker and multiplex A/B regressions, compose with #88203 if it lands first, then run a fresh exact-head matrix.

Keep extension control opt-in and preserve existing browser backends unless an exact server-bound controller is available. Centralize protocol and capability admission across API and dashboard transports, make selected-controller results authoritative, bypass stale availability caches only inside bound requests, and serialize structured results for the existing tool contract.

Add a real browser_snapshot route-table/WebSocket E2E, strict admission and ownership regressions, public configuration and protocol documentation, and tests proving feature-off/no-controller compatibility.
Treat unexpected controller transport loss as recoverable until each command's original deadline. Same-identity reconnects refresh transport and capability state, flush deferred cancels before new dispatch, and can complete already-started work.

Keep explicit detach and different controller/browser identity replacement terminal, owner-gate every inbound lifecycle frame, distinguish slow in-flight WebSocket writes from real send failures, and exclude browser-control session identity from shared shell snapshots.
Generic Hermes callers still use the existing browser backend when extension control is disabled or no server-bound controller identity exists.

Once the gateway binds a controller identity, missing scope, disconnect, or capability loss now fail closed instead of silently switching a control-this-tab request to another local or cloud browser. Covers the schema-build to dispatch disconnect race.
@abundantbeing
abundantbeing force-pushed the feat/browser-extension-controller branch from bf0d23a to 95899ab Compare August 20, 2026 07:42
@alt-glitch alt-glitch added the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Aug 20, 2026
kshitijk4poor added a commit to kshitijk4poor/hermes-agent that referenced this pull request Aug 21, 2026
… stores per profile

Addresses both merge blockers from @andrexibiza's review of NousResearch#85351:

1. HTTP-uploaded artifacts could never be consumed by broker dispatch:
   artifact_scope_key hashed (principal, session, family), the HTTP routes
   store with an EMPTY session (API-key auth has no server session) while
   broker validation carries a session-bearing ControllerScope — every
   real upload->dispatch journey died with ArtifactScopeMismatch
   (reproduced before fixing). Canonical ownership is now
   principal/transport-family (documented in the scope-key docstring);
   ids stay unguessable server-minted 32-hex and downloads one-shot.
   New composition regression: HTTP-shape upload -> registered controller
   scope -> broker artifact dispatch, mutation-checked (re-adding session
   to the key makes it fail).

2. The 'profile-scoped' artifact store was first-profile-wins process
   state: one adapter-level singleton pinned profile B to profile A's
   physical root on multiplex listeners (same frozen-handle class as
   NousResearch#88734). Stores are now cached by resolved profile, and the broker
   selects the store from the controller scope's profile_id (default-slot
   fallback preserves single-profile/test behaviour). New A/B multiplex
   regression proves distinct physical roots regardless of touch order.

Also documents the advertised ticket_expires_at as best-effort wall clock
(broker enforces expiry monotonically) per review feedback.
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Salvaged via #91535 (auto-merge armed) — all six of your commits cherry-picked onto current main with your authorship preserved (rebase merge). This is a genuinely strong feature: the WS auth core (one-shot subprotocol tickets, never-reflected, server-derived principals), the exact-identity broker contract, and the fail-closed capability allowlist all verified clean under review.

We added follow-up commits on top addressing review findings — summarizing so the dispositions are on the record here:

  1. Bind at controller registration, not transport auth — the router treated any server-stamped principal as a bound lane, so flag ON + authenticated dashboard/API session + no extension registered lost the legacy browser backend entirely. New broker.lane_registered(): a never-registered lane keeps legacy tools; a registered-but-offline lane stays fail-closed exactly as you designed.
  2. Artifact boundary composition (@andrexibiza's blocker 1) — artifact_scope_key hashed session_id, but HTTP uploads carry no session while broker dispatch always does, so the upload→dispatch journey always died with ArtifactScopeMismatch (reproduced first). Ownership is now canonically principal/transport-family; new composition regression is mutation-checked.
  3. Per-profile artifact stores (@andrexibiza's blocker 2) — the adapter singleton was first-profile-wins; stores are now cached by resolved profile and the broker selects by ControllerScope.profile_id. A/B multiplex regression added.
  4. Event-loop safety — broker lock acquisitions from loop context (WS finally/frame handling) offloaded via asyncio.to_thread so a teardown racing an in-flight command send can't stall the gateway loop.
  5. Smaller items: browser.extension_control declared in DEFAULT_CONFIG; broker's TicketInvalid renamed ControllerTicketInvalid (name collision with ws_tickets.TicketInvalid in the same auth flow); server-internal sentinel imported from its canonical definition; flag reads via load_config_readonly(); ticket_expires_at documented as best-effort wall clock vs the broker's monotonic enforcement (@Enough1122's point 1).

Closing this PR in favor of #91535. Thanks for the substantial, well-tested contribution, @abundantbeing — and thanks @andrexibiza and @Enough1122 for reviews that materially improved the merge.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Dispositions for the remaining automated-review points (1 and 3 were addressed in #91535 — expiry documented as best-effort wall clock; flag reads switched to the deepcopy-free readonly config path):

  • Unthrottled minting / O(n) prune (point 2) — real but bounded: registration requires the server's Bearer API key, tickets expire in 30s, and prune runs under the same lock as mint. A leaked API key already grants far more than ticket-table growth. A per-principal in-flight cap is a sensible hardening follow-up rather than a merge blocker.
  • Capability renegotiation vs pending completion (point 4) — traced: attach() rebinds pending.scope to the successor scope, so the new socket's complete() compares against the scope it itself registered — including its (possibly changed) capability set. A changed-capability reconnect therefore completes, not drops. Agreed it deserves an explicit test; noted as follow-up.

kshitijk4poor added a commit that referenced this pull request Aug 21, 2026
The feature flag was only documented in cli-config.yaml.example; every other
browser.* key is declared in DEFAULT_CONFIG so config tooling (dashboard
editor, hermes config get) can see it. Defaults unchanged: enabled=False,
developer_mode=False. Surfaced during review of PR #85351.
kshitijk4poor added a commit that referenced this pull request Aug 21, 2026
… transport auth

The router treated any server-stamped principal as a bound lane, so with the
flag ON every authenticated dashboard/API session lost the legacy browser
backend even when no extension controller ever registered (scope_for_session
returns None -> ControllerUnavailable, no fallback) — while check_fns still
advertised the tools via the legacy OR-gate.

New broker.lane_registered() distinguishes the two cases:
- lane never registered -> generic callers keep the legacy backend
- lane registered (controller offline/ambiguous) -> fail closed, unchanged —
  a control-this-tab session never silently jumps to another browser

Also makes the four non-allowlisted wrapped tools (cdp/console/vision/
get_images) behave correctly for never-registered lanes (legacy backend)
while staying fail-closed for registered lanes.

Surfaced during review of PR #85351.
kshitijk4poor added a commit that referenced this pull request Aug 21, 2026
attach/disconnect/detach acquire a per-controller threading.Lock that a
worker-thread dispatch can hold for up to 10s while blocking on the event
loop to transmit its command frame (run_coroutine_threadsafe +
result(timeout=10)). Acquiring that lock synchronously from loop context
(controller WS finally, frame handler, gateway WS teardown) could park the
ENTIRE gateway event loop behind the send bridge — a deterministic
multi-second global stall whenever controller teardown raced an in-flight
command. All loop-context broker calls now go through asyncio.to_thread,
matching the existing offload pattern for _close_sessions_for_transport.

Surfaced during review of PR #85351.
kshitijk4poor added a commit that referenced this pull request Aug 21, 2026
- Rename the broker's TicketInvalid to ControllerTicketInvalid: the same
  exception name already exists in hermes_cli/dashboard_auth/ws_tickets.py
  and BOTH are caught in the same WS auth flow this feature touches — two
  unrelated same-named exception types in one blast radius invited a wrong
  except clause.
- Import the 'server-internal' sentinel identity from its canonical
  definition (ws_tickets.INTERNAL_USER_ID/INTERNAL_PROVIDER) instead of
  re-declaring the strings; drift would have silently broken the
  internal-peer exclusion in _is_authenticated_identity.

Surfaced during review of PR #85351.
kshitijk4poor added a commit that referenced this pull request Aug 21, 2026
browser_control_enabled()/browser_control_developer_mode() run on every
browser tool call and inside every check_fn evaluation (uncached for bound
sessions). Both are pure reads of nested dicts; load_config()'s defensive
deepcopy (~135us/call) is wasted there. Same pattern as the other read-only
config probes.

Surfaced during review of PR #85351.
kshitijk4poor added a commit that referenced this pull request Aug 21, 2026
… stores per profile

Addresses both merge blockers from @andrexibiza's review of #85351:

1. HTTP-uploaded artifacts could never be consumed by broker dispatch:
   artifact_scope_key hashed (principal, session, family), the HTTP routes
   store with an EMPTY session (API-key auth has no server session) while
   broker validation carries a session-bearing ControllerScope — every
   real upload->dispatch journey died with ArtifactScopeMismatch
   (reproduced before fixing). Canonical ownership is now
   principal/transport-family (documented in the scope-key docstring);
   ids stay unguessable server-minted 32-hex and downloads one-shot.
   New composition regression: HTTP-shape upload -> registered controller
   scope -> broker artifact dispatch, mutation-checked (re-adding session
   to the key makes it fail).

2. The 'profile-scoped' artifact store was first-profile-wins process
   state: one adapter-level singleton pinned profile B to profile A's
   physical root on multiplex listeners (same frozen-handle class as
   #88734). Stores are now cached by resolved profile, and the broker
   selects the store from the controller scope's profile_id (default-slot
   fallback preserves single-profile/test behaviour). New A/B multiplex
   regression proves distinct physical roots regardless of touch order.

Also documents the advertised ticket_expires_at as best-effort wall clock
(broker enforces expiry monotonically) per review feedback.
swarm-agents-soxzz5 Bot added a commit to SoxZz5/hermes-agent that referenced this pull request Aug 22, 2026
* chore: AUTHOR_MAP troy.rowe@re-source.au -> troyrowe-resource

Mapping for PR NousResearch#90261 salvage (server-injected parameter 400 classifier).

* fix(classifier): retry provider-injected parameter 400s instead of aborting

The Codex OAuth backend (chatgpt.com/backend-api/codex) intermittently
injects prompt_cache_retention into its own upstream call and then rejects
it, returning HTTP 400 invalid_parameter. Hermes never sends that field on
this route (see agent/transports/codex.py::_default_prompt_cache_retention_
for_request, which only sets it for api.meta.ai and bedrock-mantle hosts).

Reproduced live: a minimal 1-message request carrying no cache parameters
at all failed 4/20 (20%) with this error, so the rejection is not
deterministic and retrying the identical request is the correct recovery.

Previously the catch-all in _classify_400 returned format_error/
retryable=False, which tripped the is_client_error abort gate in
conversation_loop and killed the turn on the first attempt - burning an
entire large-context request (~550k tokens) per failure.

Classify these as retryable server_error (should_compress=False - the
request shape was never the problem). The same guard is applied to the
sibling 5xx request-validation branch, where a fronting proxy can surface
the identical rejection.

Deliberately narrow: keyed on parameters we only send on specific routes,
and skipped when the current provider is one that legitimately sends them,
so a genuine client-side bad parameter (max_tokens on GPT-5) still fails
fast as a format_error.

* fix(agent): guard merged assistant compaction handoffs

Treat a merged assistant-role summary carrier as the driving reference handoff when it immediately follows a completed assistant stop. Its preserved prose and stale tool_calls are assistant continuity, not a fresh live user request.

Keep legitimate in-flight behavior unchanged when there is no completed stop, a real user turn follows, or a distinct later assistant tool-call row continues the loop.

Extends the NousResearch#80622 active-turn guard for the merged-carrier shape reported under NousResearch#42768.

* fix(agent): preserve live merged tool-call carriers

Identify a completed merged assistant handoff from the carrier's own stop state instead of an unrelated adjacent history row. Keep carriers with pending tool calls actionable so compaction cannot abort a live tool chain.

* fix(api): hide compaction scaffolding from clients

Project client-visible session messages through the canonical compaction classifier. Hide standalone handoffs, unwrap merged carriers to their authentic prior-tail content, strip inherited internal fields, and keep model-facing recovery history unchanged.

* fix(clients): hide compaction carriers across surfaces

* refactor(api): reuse _COMPACTION_INTERNAL_FIELDS from compaction_display

The 7-key internal-fields tuple was inlined twice (agent/compaction_display.py
and _project_client_message); a drift between the copies would silently leak
one internal field class through the API projection. Surfaced during review
of PR NousResearch#85442.

* fix(desktop): boot overlays stay opaque under window glass

The full-screen boot surfaces (connecting, onboarding, boot failure, root
crash fallback) paint their backdrop with --ui-chat-surface-background,
which the glass field turns transparent so <body> can be the one painter
(0483133). That was harmless while glass shipped off; once it shipped
on by default (be31666) every boot overlay became a window onto the
shell behind it.

These overlays mask the whole app, so they declare data-glass-opaque —
the existing contract for surfaces that paint over siblings — which pins
the token back to opaque chrome under glass and changes nothing when
glass is off.

* feat(browser): add authenticated control broker

* feat(browser): enable extension controller actions

* fix(browser): harden extension controller routing

Keep extension control opt-in and preserve existing browser backends unless an exact server-bound controller is available. Centralize protocol and capability admission across API and dashboard transports, make selected-controller results authoritative, bypass stale availability caches only inside bound requests, and serialize structured results for the existing tool contract.

Add a real browser_snapshot route-table/WebSocket E2E, strict admission and ownership regressions, public configuration and protocol documentation, and tests proving feature-off/no-controller compatibility.

* fix(browser): preserve controller work across reconnects

Treat unexpected controller transport loss as recoverable until each command's original deadline. Same-identity reconnects refresh transport and capability state, flush deferred cancels before new dispatch, and can complete already-started work.

Keep explicit detach and different controller/browser identity replacement terminal, owner-gate every inbound lifecycle frame, distinguish slow in-flight WebSocket writes from real send failures, and exclude browser-control session identity from shared shell snapshots.

* fix(browser): keep bound controller routing authoritative

Generic Hermes callers still use the existing browser backend when extension control is disabled or no server-bound controller identity exists.

Once the gateway binds a controller identity, missing scope, disconnect, or capability loss now fail closed instead of silently switching a control-this-tab request to another local or cloud browser. Covers the schema-build to dispatch disconnect race.

* feat(browser): add scoped artifact endpoints, broker permission gates, and companion journal

* chore(config): declare browser.extension_control in DEFAULT_CONFIG

The feature flag was only documented in cli-config.yaml.example; every other
browser.* key is declared in DEFAULT_CONFIG so config tooling (dashboard
editor, hermes config get) can see it. Defaults unchanged: enabled=False,
developer_mode=False. Surfaced during review of PR NousResearch#85351.

* fix(browser): bind the extension lane at controller registration, not transport auth

The router treated any server-stamped principal as a bound lane, so with the
flag ON every authenticated dashboard/API session lost the legacy browser
backend even when no extension controller ever registered (scope_for_session
returns None -> ControllerUnavailable, no fallback) — while check_fns still
advertised the tools via the legacy OR-gate.

New broker.lane_registered() distinguishes the two cases:
- lane never registered -> generic callers keep the legacy backend
- lane registered (controller offline/ambiguous) -> fail closed, unchanged —
  a control-this-tab session never silently jumps to another browser

Also makes the four non-allowlisted wrapped tools (cdp/console/vision/
get_images) behave correctly for never-registered lanes (legacy backend)
while staying fail-closed for registered lanes.

Surfaced during review of PR NousResearch#85351.

* fix(browser): offload broker lock acquisition off the event loop

attach/disconnect/detach acquire a per-controller threading.Lock that a
worker-thread dispatch can hold for up to 10s while blocking on the event
loop to transmit its command frame (run_coroutine_threadsafe +
result(timeout=10)). Acquiring that lock synchronously from loop context
(controller WS finally, frame handler, gateway WS teardown) could park the
ENTIRE gateway event loop behind the send bridge — a deterministic
multi-second global stall whenever controller teardown raced an in-flight
command. All loop-context broker calls now go through asyncio.to_thread,
matching the existing offload pattern for _close_sessions_for_transport.

Surfaced during review of PR NousResearch#85351.

* refactor(browser): dedupe auth-flow names and sentinel identity

- Rename the broker's TicketInvalid to ControllerTicketInvalid: the same
  exception name already exists in hermes_cli/dashboard_auth/ws_tickets.py
  and BOTH are caught in the same WS auth flow this feature touches — two
  unrelated same-named exception types in one blast radius invited a wrong
  except clause.
- Import the 'server-internal' sentinel identity from its canonical
  definition (ws_tickets.INTERNAL_USER_ID/INTERNAL_PROVIDER) instead of
  re-declaring the strings; drift would have silently broken the
  internal-peer exclusion in _is_authenticated_identity.

Surfaced during review of PR NousResearch#85351.

* perf(browser): read the feature flags via load_config_readonly

browser_control_enabled()/browser_control_developer_mode() run on every
browser tool call and inside every check_fn evaluation (uncached for bound
sessions). Both are pure reads of nested dicts; load_config()'s defensive
deepcopy (~135us/call) is wasted there. Same pattern as the other read-only
config probes.

Surfaced during review of PR NousResearch#85351.

* fix(browser): make the artifact boundary compose end-to-end and scope stores per profile

Addresses both merge blockers from @andrexibiza's review of NousResearch#85351:

1. HTTP-uploaded artifacts could never be consumed by broker dispatch:
   artifact_scope_key hashed (principal, session, family), the HTTP routes
   store with an EMPTY session (API-key auth has no server session) while
   broker validation carries a session-bearing ControllerScope — every
   real upload->dispatch journey died with ArtifactScopeMismatch
   (reproduced before fixing). Canonical ownership is now
   principal/transport-family (documented in the scope-key docstring);
   ids stay unguessable server-minted 32-hex and downloads one-shot.
   New composition regression: HTTP-shape upload -> registered controller
   scope -> broker artifact dispatch, mutation-checked (re-adding session
   to the key makes it fail).

2. The 'profile-scoped' artifact store was first-profile-wins process
   state: one adapter-level singleton pinned profile B to profile A's
   physical root on multiplex listeners (same frozen-handle class as
   NousResearch#88734). Stores are now cached by resolved profile, and the broker
   selects the store from the controller scope's profile_id (default-slot
   fallback preserves single-profile/test behaviour). New A/B multiplex
   regression proves distinct physical roots regardless of touch order.

Also documents the advertised ticket_expires_at as best-effort wall clock
(broker enforces expiry monotonically) per review feedback.

* fix(api): correct _handle_browser_control_frame return annotation

The frame handler returns reply dicts (heartbeat/detach acks) that the WS
reader loop sends back; the -> None annotation was the only new ty
diagnostic vs origin/main.

* fix(browser): honor live Developer Mode for privileged capability selection

The global broker snapshotted browser.extension_control.developer_mode once
at construction, so flipping it OFF in config did not revoke raw CDP/eval
from already-attached controllers until process restart — a revocation
failure at the highest-privilege browser surface (blocker 3 of
andrexibiza's NousResearch#91535 review). select() now consults the live config on
every privileged selection (explicit bool still pins for tests); off->on
also unlocks without restart. Regression test drives both directions
against an attached controller. Also drops the dead back-compat
_artifact_store property (zero readers).

* fix(browser): sweep orphan artifact files at store construction

Artifact receipts live only in memory, so files left behind by a dead
process were unreachable but persisted forever despite the advertised
300s TTL — a retention failure on the surface meant to be ephemeral
(blocker 4 of andrexibiza's NousResearch#91535 review). A fresh ArtifactStore now
removes every artifact-id-shaped file and stale *.tmp with no index entry
(at construction the index is empty, so all such files are orphans).
Non-artifact-shaped names are untouched. Regression: store -> recreate
store over same root -> orphan+tmp gone, unrelated file kept.

* perf(api): classify compaction rows once per message in run.completed transcript

_turn_transcript_messages pre-classified every message with
_is_compressed_summary_message (full content flatten + prefix scan), then
_message_response re-ran the same classifier inside its projection --
2x per non-summary row, 3x per summary row on every run.completed emit.
The outer guard was redundant: _message_response already yields
display_kind hidden for pure handoffs. One projection call per row now.
Surfaced by the post-merge simplify re-review of NousResearch#91517/NousResearch#91535.

* fix(desktop): stop tabs double-click-hiding the tab strip; body double-tap reveals it

The synthesized double-tap that hides a zone's tab strip rode every tab's
pointerdown (generic pane drag and each pane's tabDrag), so a routine
double-click on a tab (select a title, retry a click) vanished the whole
bar and stranded the zone with no tab, no close X, and no way back but a
right-click. Keep the documented hide gesture on the strip background
only, and add its inverse as recovery: double-tap a hidden zone's body
restores the strip. Regression tests pin both sides of the grammar.

* fix(desktop): scope the salvaged fix to the failure-path removal

Narrows NousResearch#86278 to exactly the defect. Tabs pass no double-tap context on any press path (generic pane drag, multi-tab selection drag, chrome.tabDrag), so a double-click on a tab can no longer hide the strip; the strip background keeps its documented hide gesture unchanged.

The body double-tap reveal from NousResearch#86278 is dropped: the zone body deliberately carries no double-click gesture (virtualized content recreates its nodes between clicks, per the standing ruling in tree-group.tsx), and recovery surfaces for a deliberately hidden header are being decided separately across NousResearch#84458 / NousResearch#81638 / NousResearch#89225. The DOUBLE_TAP_MS export is reverted since no consumer remains outside drag-session.

Test file trimmed to the two assertions that pin the grammar: a tab double-tap must not hide the strip (red on main), the strip background double-tap still hides. Taps release on window between presses so the drag-session synthesized double-tap path is the one exercised.

* refactor(desktop): make a zone's tab strip a stated mode, not a flag five paths wrote

`headerHidden` carried two meanings at once. `true` was either "the user hid
this" or "a double-tap nobody meant hid this"; `false` was either "the user
wants a strip" or "insert / tab-cycling / dock-enforce / adoption pinned one to
escape a dead end". Because the layout wrote the same field the user did, a
repair silently overwrote a preference and neither could be read back — and
since hiding also unmounted the tab, the ✕ and the menu offering "Show header",
a zone that got hidden by accident stayed that way across restarts.

Replaces it with `tabStrip?: 'always' | 'never'`, where absent is auto and only
the user ever writes it, and moves the decision into one resolver that TreeGroup
and the store both call, so the strip on screen and the toggle command cannot
disagree. Reachability moves into that resolver as an invariant that outranks an
explicit `never`: a closeable tile keeps its ✕ and a lone tool panel keeps its
chip, because "hide the chrome" is never a request to make a surface
unreachable. With that guarantee held centrally, the four repair writes are
gone. Persisted `headerHidden` is dropped rather than translated — nothing on
disk distinguishes a deliberate hide from an accidental one, and carrying the
accidents forward would re-strand exactly the people who reported being stuck.

The double-tap hide goes with it, along with the synthesized double-tap detector
it was the only consumer of. It fired from ordinary double-clicks on a tab,
nothing announced it, and its undo lived behind the chrome it had just removed.
`data-zone-no-header` goes too: it marked full-page views for a body
double-click toggle that no longer exists, and nothing has read it since.

Supersedes the tab-side half of the fix from abundantbeing and yoniebans, whose
commits this builds on.

* feat(desktop): give hiding the tab strip a command, and a way back

The strip could only be hidden by an undiscoverable double-tap, and once hidden
the zone had no chrome left to click — no tab, no ✕, no menu holding "Show".
This puts it on the same footing as the status bar, whose hide has never
stranded anyone: ⌥⌘T, a ⌘K row, the shell context menu, and the zone menu, which
now prints the keystroke on the row that takes the strip away so the way back is
stated at the moment it matters. All four resolve their target zone the same way
the other tab verbs do (hovered, else focused, else the workspace) and describe
themselves from what is on screen rather than from a stored value, so "toggle"
always means the opposite of what the user is looking at.

Adds an app-wide default alongside it, in Appearance next to Session List
Density — auto, always, or never, matching VS Code's `workbench.editor.showTabs`
and Zed's `tab_bar.show` for people who want one answer everywhere instead of a
per-zone choice they repeat. A zone that has stated its own preference still
wins, and neither value can strand a pane.

* feat(nix): wait for the backend bind target before it starts

The backend binds to `backend.host` immediately. The bind fails when the
target is not ready, because uvicorn cannot bind a name that does not
resolve, or an address that no interface holds. A unit that starts at boot
loses this race against the daemon that supplies the target, such as
tailscaled.

A bind to a Tailscale MagicDNS name shows the problem. The name is the
correct bind target, because the dashboard refuses each request with a Host
header that is different from the address that the server bound to, and a
shared machine has a different address in each tailnet. But the name does
not resolve until tailscaled is up, so the unit fails at each boot until
`Restart=on-failure` finds the moment when the name works.

A systemd user unit cannot order itself after a system unit. `After=` and
`Requires=` are silent no-ops across that boundary. Thus the wait is a poll,
and not a dependency.

This change adds three options to `services.hermes-agent.backend` on both
the NixOS module and the Home Manager module:

- `waitFor` — `null` (the default, unchanged behavior), `"hostname"`, or
  `"interface"`
- `interfaceName` — the interface to take the address from
- `waitTimeout` — the time in seconds before the unit stops

With `waitFor`, ExecStart becomes a launcher that polls for the target and
then execs hermes. `exec` keeps hermes as the MainPID, so the restart logic
of systemd sees the real process. A timeout stops the unit with an error. It
does not bind a fallback address, because a fallback can expose the backend
more widely than the user intends.

The default is not changed. Without `waitFor`, ExecStart is the same
command line as before.

* docs: Add Nix/NixOS to installation link description

* add mike@vorburger.ch to contributors

* feat(cron): bot-chat delivery target — cron output lands in a bot's canonical Bot Chat and the bot responds

deliver='bot-chat[:<profile>]' is a machine-local pseudo-platform: the
scheduler delivers job output as a real inbound turn in the target
profile's canonical Bot Chat via the chat CLI lane (--in ~ -c "Bot Chat"
--create-if-missing -Q --query-file), the same lane Bot Mode
agent-to-agent messages use. The bot reads the output, acts on it, and
responds in its chat — instead of the output only landing in Run history.

- cron/scheduler.py: token parsing, target resolution (own profile /
  named local profile / unknown -> skipped with warning), subprocess
  delivery lane with cron.bot_chat_delivery_timeout_seconds (default
  600s), preflight exemption, and bot-chat entries in
  cron_delivery_targets() for UI pickers. Excluded from 'all' by design.
- tools/cronjob_tools.py: create/update-time validation — named profiles
  must exist on this machine (fail at create, not at 3am); deliver schema
  documents the new token.
- tui_gateway/methods_tools.py: cron.manage add forwards deliver.
- hermes_cli/profiles.py: list_profile_names() cheap name-only scan.
- hermes-bots plugin: Create Cronjob dialog gains a 'Send results to'
  picker (Run history only / <bot>'s chat); bot-chat jobs send the BARE
  token on the profile-scoped create so Desktop-side aliases can never
  name a profile the backend doesn't have.
- Docs: user cron guide, automate-with-cron, cron-internals.

Machine-local by construction: names resolve only against the executing
machine's ~/.hermes/profiles/, so overlapping profile names across
multiple connected gateways are unambiguous.

* test(cron): delivery-targets test scopes platform assertions past bot-chat entries

cron_delivery_targets() now also lists machine-local bot-chat:<profile>
entries; the sibling test's exact set-equality assertion predates them.
Scope the platform assertions to gateway entries and pin that bot-chat
entries are always home_target_set.

* kanban: resume-on-retry session continuity for re-review rounds

A task that bounces review-requested-changes -> ready and gets
reclaimed by the SAME implementer profile previously got a fully cold
'hermes chat -q' session every round: full system prompt, tool
schemas, and task history re-fetched from scratch.

_default_spawn now accepts an optional conn and, when the dispatcher
passes one, calls _resolve_worker_resume_session_id() to look up the
implementer's own worker_session_id stamped on the last ENDED run
under that profile (kanban_complete/kanban_request_review already
stamp this via _stamp_worker_session_metadata). If that session still
resolves in the profile's own state.db, the child is launched with
--resume <id> --no-restore-cwd instead of a cold start. Any resolution
failure (no prior run, wrong profile, session gone) falls back to
today's cold-start behaviour -- dispatch is never blocked.

Both dispatch_once() spawn call sites (ready lane, review lane) now
pass conn through to spawn_fn via signature introspection, matching
the existing board= pattern so older test stubs are unaffected.

Card: t_e1ba67d5

---------

Co-authored-by: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>
Co-authored-by: Troy Rowe <troy.rowe@re-source.au>
Co-authored-by: abundantbeing <beingsabundant@gmail.com>
Co-authored-by: emozilla <emozilla@nousresearch.com>
Co-authored-by: yoniebans <jonny@nousresearch.com>
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
Co-authored-by: ethernet <arilotter@gmail.com>
Co-authored-by: Michael Vorburger <mike@vorburger.ch>
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
Co-authored-by: Saylent Swarm <swarm@saylent.dev>
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 comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets comp/tui Terminal UI (ui-tui/ + tui_gateway/) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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.

5 participants