fix(dashboard): recycle the console executor when every worker is permanently stuck

[briandevans]

Supersedes #59240 — sibling follow-up to a6b9597d5

• a6b9597d5 ("perf(console): cache CLI-surface summaries + bound console worker pool") introduced the bounded four-worker console pool this builds on.
• What it did not add is any way for that pool to recover once its workers are permanently wedged — the code comment on main concedes the hazard ("a genuinely stuck worker keeps running to completion") but nothing acts on it.
• This adds that recovery, implementing the per-future and per-executor-generation timeout ownership plus the three regression tests requested in the review on fix(dashboard): recycle console executor when all workers are permanently stuck #59240.

#59240 is the only other open PR on this code and is abandoned: last commit 2026-07-05, no force-pushes and no author reply since, currently CONFLICTING/DIRTY, and the review landed after its last commit. This is a fresh implementation on current main rather than a rebase, because the review's first two findings are design-level rather than textual.

What does this PR do?

Dashboard console commands run on a process-global, bounded four-worker ThreadPoolExecutor (hermes_cli/web_server.py). asyncio.wait_for can only abandon the await — Python threads aren't preemptible, so a genuinely wedged command holds its worker for the life of the process.

After four such commands the pool is permanently exhausted. Every later console command, from every dashboard session and every profile, then queues behind threads that will never return, sits the full 60s, and returns:

Command timed out. Hermes Console returned to the prompt.

...forever, until Hermes is restarted. That is an availability failure, not just a slow path: the dashboard console is dead with no user-visible indication of why and no way to recover short of a restart.

This tracks which submissions actually timed out and swaps in a fresh pool once none are left.

How the review's findings are addressed

The review on #59240 named three problems. Each maps to a specific change here:

1. "The done callback decrements the global count for every non-cancelled future, not just a future that previously timed out."

There is no counter any more. A future is recorded only on the asyncio.TimeoutError path (_note_console_command_stuck), and the done-callback (_release_console_future) only ever discards that same future object from the set. A command that never timed out is never in the set, so it structurally cannot clear another worker's mark or hold the pool below its recycle threshold.

2. "_console_executor_stuck_count has no executor-generation ownership; a late completion from a retired executor can decrement timeout state accumulated by the replacement."

Marks live in Dict[generation, Set[Future]] and every callback is bound to the generation that ran it. A retired generation's entry is dropped wholesale at recycle time, so both a late completion and a late timeout report from an old pool are no-ops. The generation guard on the mark path also prevents the worse case: without it, four stale timeout reports would re-trip the threshold and retire the live pool.

3. "The diff adds no timeout/replacement regression test."

Four added, covering exactly the requested cases — four blocked workers, replacement, and late completion from the old pool. See How to Test.

Also fixed, because the recycle requires it

The atexit hook on main closes over the module global:

atexit.register(
    lambda: _console_executor
    and _console_executor.shutdown(wait=False, cancel_futures=True)
)

Once a pool is recycled that global names the replacement, so the hook would shut down the wrong pool and leave the retired one registered. Teardown is now bound to each concrete executor via a _new_console_executor() factory.

Scope

_get_console_executor() had exactly one call site, and a sweep of the root cause (grep -rn ThreadPoolExecutor over non-test sources) found no sibling site sharing it. Deliberately excluded, with reasons:

• The other run_in_executor calls in web_server.py pass None — they use the shared default loop executor, which is precisely what the dedicated console pool exists to protect. Different lifecycle, different concern.
• tools/async_delegation.py::_get_executor is also a persistent global pool, but it grows on demand rather than being fixed-cap, and has no per-call abandon-on-timeout, so it cannot reach this failure mode.
• tools/vision_tools.py::_vision_cpu_executor has no per-command timeout that abandons a worker.

Related Issue

No linked issue exists — filing this as a supersede of #59240 rather than inventing one.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

hermes_cli/web_server.py

• Added _console_executor_generation and _console_stuck_futures: Dict[int, Set[Future]] beside the existing pool globals. (Named to avoid colliding with the unrelated per-connection command_generation counter in console_ws.)
• Added _new_console_executor() — builds a pool and registers an atexit hook bound to that executor.
• Added _submit_console_command() — selects the pool, submits, and attaches the generation-bound done-callback. Pool selection and submit happen under one lock so a concurrent recycle can never hand back an executor that is already shut down by the time we submit.
• Added _release_console_future() — discards a future's own mark from its own generation.
• Added _note_console_command_stuck() — marks a timed-out future, and atomically swaps in a fresh pool once _CONSOLE_EXECUTOR_MAX_WORKERS marks accumulate, logging a warning and abandoning the old pool (shutdown(wait=False, cancel_futures=True)).
• Removed _get_console_executor(); console_ws.run_command now submits via _submit_console_command and awaits asyncio.wrap_future(cf_future), so the raw concurrent.futures.Future stays reachable after the await is abandoned.
• Added Callable and Set to the typing import.

tests/hermes_cli/test_web_server_console_ws.py

• Added a console_pool fixture (pristine pool per test, threads torn down after) and a _Wedge helper that blocks until the test releases it.
• Added four tests (below).

Thread-safety notes

• The done-callback runs on a worker thread while a swap may be happening on the event loop thread; both take _console_executor_lock.
• _note_console_command_stuck skips a future that is already done() — it finished while we were giving up on the await, so it is slow, not wedged, and must not count toward the threshold. Future.done() flips before done-callbacks run, and those callbacks take the same lock, so this check cannot race the release path in either direction.
• Marks are bounded: at most _CONSOLE_EXECUTOR_MAX_WORKERS per live generation, dropped at recycle.

How to Test

uv run --with pytest --with pytest-xdist --with pytest-asyncio \
  python3 -m pytest tests/hermes_cli/test_web_server_console_ws.py -v

Four new deterministic tests (no real 60s timeout — they drive the pool helpers directly):

Test	Covers
test_console_pool_recycles_only_when_every_worker_is_stuck	Four blocked workers replace the pool; three interleaved normal commands never clear the wedged mark, and the replacement serves new commands
test_completed_console_command_is_never_marked_stuck	A command that returns as we abandon the await is slow, not wedged
test_late_completion_from_retired_pool_leaves_replacement_untouched	Late completion and a late timeout report from the retired pool leave generation 1 untouched, and the replacement still recycles on its own fourth wedged worker
test_console_executor_atexit_hook_shuts_down_its_own_pool	A pool's exit hook tears down that pool, not whatever the global currently names

Fails before, passes after. Reverting the production hunk alone turns all four red. Each test was also verified against a targeted mutation, so the coverage binds to the logic rather than to the new symbol names — one mutation per test, no overlap:

Mutation	Test that catches it
_release_console_future clears marks it doesn't own (the original finding #1)	..._recycles_only_when_every_worker_is_stuck
Mark path drops its executor-generation guard (the original finding #2)	..._late_completion_from_retired_pool...
atexit reads the module global instead of its own pool	..._atexit_hook_shuts_down_its_own_pool
Already-done() guard removed	..._completed_console_command_is_never_marked_stuck

Adjacent suites run green (627 passed, 1 skipped): test_web_server_console_ws.py, test_console_engine.py, test_web_server.py, test_web_server_boot_handshake.py, test_web_server_pty_reconnect.py, test_web_server_pty_import.py, test_verify_console_scripts.py. ruff check is clean on both touched files.

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 console WS suite plus six adjacent tests/hermes_cli/ web_server suites (627 passed, 1 skipped), not the full tree
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS (Darwin 25.4, arm64), Python 3.12.12

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — module comments and docstrings on the touched helpers; no user-facing docs affected
• I've updated cli-config.yaml.example if I added/changed config keys — N/A, no config keys
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — stdlib threading/concurrent.futures only, no platform-specific paths or APIs
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

Contract Protected

Invariant: a console command's timeout mark is owned by exactly one (future, executor generation) pair. Only that future's own completion may clear it, and only while its generation is live.

• Known-bad inputs: a successful command completing while another worker is wedged; a completion arriving from a pool that was already retired; a timeout reported against a retired generation; a command that finishes in the instant between the timeout and the mark.
• Negative case: an ordinary command must not decrement anything — asserted directly in test_console_pool_recycles_only_when_every_worker_is_stuck, which waits for the production done-callback to have actually run (via a probe callback registered afterwards, since callbacks fire in registration order) before asserting the mark survived.
• Future inputs: the recycle threshold reads _CONSOLE_EXECUTOR_MAX_WORKERS rather than a literal, and the tests derive their loop bounds from it too, so changing the pool size cannot silently invalidate either.

[teknium1]

Thanks for the careful follow-up to #59240. The premise is real on current main: hermes_cli/web_server.py:14834-14862 creates one four-worker console pool, and console_ws only times out the await at hermes_cli/web_server.py:15129-15142.

Problems

• The replacement path in d379fd44344c abandons each retired executor's running threads with shutdown(wait=False, cancel_futures=True) and starts four more workers. Running threads cannot be stopped that way, so repeated saturation/recycle cycles can accumulate permanently stuck threads without a process-wide ceiling. This defeats the original bounded-pool intent in a6b9597d5, which explicitly capped leaked workers.

Suggested changes

• Preserve a process-wide bound, or define an explicit degraded-mode contract once a bounded number of retired workers exist; add a deterministic repeated-recycle test for that contract.

Automated hermes-sweeper review.