Skip to content

feat(gateway): bind toolsets per session, and give spawn --toolsets back - #317

Merged
OmarB97 merged 1 commit into
mainfrom
feat/desktop-spawn-toolsets-per-session-fork-20260802
Aug 2, 2026
Merged

feat(gateway): bind toolsets per session, and give spawn --toolsets back#317
OmarB97 merged 1 commit into
mainfrom
feat/desktop-spawn-toolsets-per-session-fork-20260802

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Follow-up to #315. That PR removed hermes desktop spawn --toolsets because the flag had nowhere to land: session.create took no toolsets parameter, so the value travelled CLI → HTTP → renderer and died there. This adds the missing half — a per-session toolset pin on the gateway — and restores the flag on top of it.

toolsets joins model / reasoning_effort / fast as a session.create parameter, with the same posture: it steers one chat and never writes config. That distinction is the whole point. tools.configure is the global writer (it calls save_config), so it cannot express "run this chat with just file+terminal" without changing what the user's next chat gets.

Binding at create time is also what keeps it cheap. AGENTS.md rules out swapping toolsets mid-conversation because it invalidates the per-conversation prompt cache — this resolves before the agent is built and before the first turn, so there is no cached prefix to invalidate. Omitted is the unchanged path every typed chat takes.

Two things this had to get right

None is a value, not an absence. all/* resolves to enabled_toolsets=None — and None is also what "no pin given" would naturally be. Conflating them would silently downgrade --toolsets all to the process default. _NO_TOOLSET_OVERRIDE is the sentinel that keeps them apart, and the deferred build threads the pin on key presence rather than truthiness.

A pin that cannot be honored fails the create. Names are validated against the same vocabulary HERMES_TUI_TOOLSETS accepts — built-ins, plugin toolsets (after a discovery pass), enabled MCP servers, and all/* — and an unresolvable name is refused (4026) rather than dropped. Handing back a session whose tools are not the ones asked for is exactly the failure the pin exists to prevent; it is also how NousResearch#38798 became a text-only agent nobody could explain. Validation runs before the active-session slot is claimed, so a typo cannot burn a lease that only session.close releases.

Notes for review

  • The resolver is extracted, not duplicated: _resolve_toolset_names now backs both HERMES_TUI_TOOLSETS and the session pin, so the two cannot drift into subtly different ideas of what a toolset name is. The extraction is behavior-preserving — the env resolver keeps its exact stderr warnings and fallback ordering, and its existing 11 tests pass untouched.
  • The pin dies at a /new boundary alongside the other session-scoped overrides. Not just symmetry: tools.configure rebuilds through _reset_session_agent, so toggling a toolset in the UI re-derives from config instead of being silently overruled by a spawn's pin the user can neither see nor clear.
  • The renderer submit gate now calls hasSessionOverrides instead of a hand-rolled model || provider || profile list. That hand-rolled list is precisely why a toolsets-only spawn would have sent no overrides at all — and it would have silently swallowed the next field someone adds too.

Related Issue

Follow-up to #315 (which removed the dead flag) and #298 (which introduced it).

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

Gateway (the missing half)

  • tui_gateway/server.py
    • _resolve_toolset_names + _ToolsetResolution: the shared resolver, extracted from _load_enabled_toolsets (which now calls it and keeps its warnings verbatim).
    • session.create accepts toolsets, validates it, and pins create_toolsets_override — before claiming the session slot.
    • _make_agent grows toolsets_override (sentinel-defaulted) and forwards it as enabled_toolsets.
    • _start_agent_build threads the pin to the deferred build; _reset_session_agent clears it.

Client chain (the leg that was broken)

  • use-spawn-bridge.ts — reads payload.toolsets into the overrides; gate switched to hasSessionOverrides.
  • session-overrides.tsSessionCreateOverrides.toolsets; hasSessionOverrides counts it (an empty array is not a pin).
  • use-session-actions/index.tsdesktopSessionCreateParams puts it on the wire, only when pinned.
  • spawn-control.ts, global.d.ts — carry the field again; an all-blank list is refused here rather than becoming a silent "inherit".
  • hermes_cli/subcommands/gui.py, hermes_cli/desktop_spawn.py--toolsets restored, split onto the wire, with a non-empty check argparse cannot express.

Tests — one per leg: CLI splitting + non-empty check, parseSpawnRequest shape, the bridge (including a toolsets-only spawn), session.create params, gateway validation/pinning/all/lease-ordering, _make_agent forwarding, and the /new clear.

How to Test

Verified end to end against a live desktop app in an isolated HERMES_HOME, driven by the real CLI over the real control channel (model qwen3.6-27b-nvfp4). Each chat was asked to name every tool it could call:

spawn tools the agent actually had prompt tokens (tool schemas included)
--toolsets file 4patch, read_file, search_files, write_file 6,090
(no flag — control) 20clarify, close_terminal, delegate_task, execute_code, patch, process, project_create, project_list, project_switch, read_file, read_terminal, search_files, session_search, skill_manage, skill_view, skills_list, terminal, web_extract, web_search, write_file 16,768

The pinned chat got exactly the file toolset and nothing else — no terminal, no web, no delegation — while the control in the same app was unaffected. The ~10.7k-token difference is the tool schemas the pin removed.

The negative case is the sharpest proof that the value reaches session.create rather than dying in the renderer as it used to:

hermes desktop spawn "..." --toolsets file,notarealtoolset
→ ✓ Sent prompt to the Hermes desktop app.
   desktop.log: [spawn-control] delivered spawn (current model)
   state.db:    no session row — the create was refused (4026)

Delivered to the renderer, and no session was created. Before this PR an unknown name was simply ignored and the chat started with the app's own toolsets. A valid pin in the same app created its session normally, so the toolset name is the only variable.

Reproduce:

  1. hermes desktop spawn "list this repo" --toolsets file,terminal — the new chat has exactly those tools; other chats are unaffected and config.yaml is untouched.

  2. --toolsets file,notarealtoolset — refused by session.create (4026, naming the bad entry), surfaced as a toast via the existing notifyError path in use-prompt-actions/submit.ts.

  3. --toolsets all means all, not "the process default".

  4. Suites (macOS 15 / Darwin 25.6.0, on 8bfff38):

    scripts/run_tests.sh tests/tui_gateway/ tests/test_tui_gateway_server.py tests/hermes_cli/test_desktop_spawn.py tests/hermes_cli/test_gui_command.py -q
    

    920 tests passed, 0 failed across 42 files.

    cd apps/desktop && npx vitest run electron/spawn-control.test.ts src/app/contrib/hooks/use-spawn-bridge.test.tsx src/app/session/session-overrides.test.ts src/app/session/hooks/use-session-actions.test.tsx
    

    82 passed. npm run typecheck and eslint on the touched files are clean.

    Note for anyone re-running the Python side in a single pytest process: tests/tui_gateway/ and tests/test_tui_gateway_server.py share module state and produce ~32 failures when run together in one process. That is pre-existing (it reproduces on main with these files untouched) and is why scripts/run_tests.sh isolates each file in its own subprocess.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — ran the full gateway + spawn surface (920 passed) plus the desktop vitest/typecheck/lint, not the whole suite
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Darwin 25.6.0) — unit suites plus a live app run in a sandboxed HERMES_HOME (see How to Test)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — docstrings + --toolsets help text; no doc referenced this flag
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (deliberately not a config key)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — argparse/JSON/dict plumbing only
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A (no model tools touched; this selects among existing ones)

Screenshots / Logs

Live run — same app, same channel, only the flag differs:

[20260802_073733_ebac94] API call #1: in=6090  out=11 total=6101   # --toolsets file
  assistant: patch,read_file,search_files,write_file

[20260802_073851_a46bba] API call #1: in=16768 out=57 total=16825  # control, no flag
  assistant: clarify,close_terminal,delegate_task,execute_code,patch,process,
             project_create,project_list,project_switch,read_file,read_terminal,
             search_files,session_search,skill_manage,skill_view,skills_list,
             terminal,web_extract,web_search,write_file

Test suites:

=== Summary: 42 files, 920 tests passed, 0 failed (100% complete) in 30.0s (4 workers) ===

… back

#315 removed `hermes desktop spawn --toolsets` because it had nowhere to
land: `session.create` took no toolsets parameter, so the value reached the
renderer and died there. This adds the missing half — a per-session toolset
pin on the gateway — and restores the flag on top of it.

`toolsets` joins model / reasoning_effort / fast as a `session.create`
parameter, and follows their posture exactly: it steers ONE chat and never
writes config. That distinction is the point. `tools.configure` is the global
writer (it calls save_config), so it cannot express "run this one chat with
just file+terminal" without changing what the user's next chat gets.

Binding at create time is also what keeps it cheap. AGENTS.md rules out
swapping toolsets mid-conversation because it invalidates the per-conversation
prompt cache; this resolves before the agent is built and before the first
turn, so there is no cached prefix to invalidate. Omitted is the unchanged
path every typed chat takes.

Two things this had to get right:

- **None is a value, not an absence.** `all`/`*` resolves to
  `enabled_toolsets=None`, which is also what "no pin given" would naturally
  be. Conflating them would silently downgrade `--toolsets all` to the process
  default. `_NO_TOOLSET_OVERRIDE` is the sentinel that keeps them apart, and
  the deferred build threads the pin on key PRESENCE rather than truthiness.

- **A pin that cannot be honored fails the create.** Names are validated
  against the same vocabulary `HERMES_TUI_TOOLSETS` accepts — built-ins,
  plugin toolsets after discovery, enabled MCP servers, `all`/`*` — and an
  unresolvable name is refused (4026) rather than dropped. A session whose
  tools are not the ones asked for is the exact failure the pin exists to
  prevent, and it is how NousResearch#38798 became a text-only agent nobody could explain.
  Validation runs BEFORE the active-session slot is claimed, so a typo cannot
  burn a lease that only session.close releases.

The resolver is extracted rather than duplicated: `_resolve_toolset_names` now
backs both `HERMES_TUI_TOOLSETS` and the session pin, so the two cannot drift
into subtly different ideas of what a toolset name is. The extraction is
behavior-preserving — the env resolver keeps its exact stderr warnings and
fallbacks, covered by its existing 11 tests.

The pin dies at a `/new` boundary alongside the other session-scoped
overrides. That is not just symmetry: `tools.configure` rebuilds through
`_reset_session_agent`, so toggling a toolset in the UI re-derives from config
instead of being silently overruled by a spawn's pin the user can neither see
nor clear.

Renderer side, the break #315 documented is closed: `useSpawnBridge` reads
`payload.toolsets` into `SessionCreateOverrides`, and
`desktopSessionCreateParams` puts it on the wire. The submit gate now calls
`hasSessionOverrides` instead of a hand-rolled `model || provider || profile`
list — that hand-rolled list is precisely what made a toolsets-only spawn send
no overrides at all, and it would have silently swallowed the next field too.

Tests cover every leg end to end: CLI splitting and its non-empty check,
`parseSpawnRequest` shape, the bridge (including a toolsets-only spawn),
`session.create` params, gateway validation/pinning/`all`/lease ordering,
`_make_agent` forwarding, and the `/new` clear.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit 41e92ea into main Aug 2, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant