Commit graph

19400 commits

Author SHA1 Message Date
teknium1
4ae5efa3f4 fix: floor gate only refuses configs with an EXPLICIT below-floor _config_version
Profile clones and hand-written minimal configs carry no _config_version
key; check_config_version() coerces that to 0, which wrongly tripped the
v12 floor and left clones unstamped (caught by CI:
test_clone_config_copies_files / test_clone_from_named_profile).
Version-less configs now take the normal ladder + fresh stamp — the
historical behavior; only genuinely ancient explicit-version configs
are refused.
2026-07-29 16:44:31 -07:00
teknium1
4b33e5663b refactor: config auto-migration support floor at v12 + deprecated shim retirement 2026-07-29 16:44:31 -07:00
brooklyn!
5c07ba2f3a
Merge pull request #74422 from NousResearch/bb/platforms-changed
Desktop: platforms.changed broadcast retires the Messaging page's 6s status poll
2026-07-29 18:37:35 -05:00
brooklyn!
1d95a227f8
Merge pull request #74447 from NousResearch/bb/hover-tab-slots
Tab verbs follow the pane under the pointer
2026-07-29 18:35:59 -05:00
brooklyn!
5010b9496a
Merge pull request #74445 from NousResearch/bb/cmdk-session-tab
Stop ⌘K and notification clicks stealing the main tab
2026-07-29 18:25:46 -05:00
teknium1
011ec4513e refactor(web): extract sessions/mcp/skills/tools routes to APIRouter modules (wave 2; route-table equality verified)
- hermes_cli/web_routers/sessions.py: 14 routes across 3 routers
  (list_router, search_router, manage_router) mounted at the three original
  registration points so global route order is preserved exactly.
- hermes_cli/web_routers/mcp.py: 11 routes; OAuth flow registry
  (_mcp_oauth_flows/lock/cap) stays in web_server, reached via new
  web_deps.LateState live proxies so tests mutating web_server._mcp_oauth_flows
  keep working.
- hermes_cli/web_routers/skills.py: 12 routes across hub_router + router
  (two original registration points straddle the profiles router include).
- hermes_cli/web_routers/tools.py: 12 routes; toolset/terminal catalogs stay
  in web_server (some are defined after the mount point), reached via LateState.
- web_deps.py: add LateState — operation-time proxy for web_server-owned
  module state (getattr/item/iter/len/contains/context-manager/comparisons).
- Handler bodies byte-identical; legacy re-exports keep
  web_server.<handler> importable for tests.
- Verified: ordered route table (method, path) identical to pre-refactor app
  (291 routes); import smoke; ruff; windows-footguns clean.
- test_web_server_sessiondb_eventloop.py: structural AST scan now reads both
  web_server.py and web_routers/sessions.py (handlers moved; helpers stayed).
2026-07-29 16:21:54 -07:00
teknium1
1a3a9de630 refactor(gateway): extract run_sync onto TurnRunner (completes the TurnContext seam; AST-identical body) 2026-07-29 16:21:46 -07:00
Brooklyn Nicholson
15a55cbff1 feat(desktop): tab verbs follow the pane under the pointer
⌘1…⌘9, ⌃Tab, and the ⌘W / ⌘T family all resolved the last-interacted
zone, so switching tabs in a second pane meant clicking into it first.
They now resolve the HOVERED zone when the pointer is in one, falling
back to the focused zone otherwise — hover pane 1, ⌘2; hover pane 2, ⌘2,
and each lands in its own strip.

tabTargetGroupId() is the single resolver, keeping the number keys and
the tab verbs from disagreeing about which zone is "the" zone the way
they already couldn't. pointerover fires per boundary crossing rather
than per mouse move, and leaving the document clears the override so a
parked pointer never strands the keys on a stale zone.
2026-07-29 18:19:54 -05:00
Brooklyn Nicholson
93e54139c4 fix(desktop): stop ⌘K and notification clicks stealing the main tab
Both surfaces passed the sidebar's in-place intent, which means "load it
into main when it isn't already on screen" — right for a row you clicked
in a list you were looking at, wrong for a chat opened from outside the
workspace. Neither had a surface of its own, so they took the one you
were using.

Add a stack intent for that case. It focuses the session when it's
already open, spends an unused draft tab when there is one, and only
falls back to main while main is itself a blank draft. Modifiers still
force a tab or window.
2026-07-29 18:16:40 -05:00
Brooklyn Nicholson
272d3be6c8 feat(desktop): find and reuse an open blank "New session" tab
An unused draft tab is the one a user would have typed into, so give the
tile store a way to name it (blankDraftTile) and hand it to another
session in place (reuseBlankDraftTile). A blank-but-busy tab has its
first turn in flight and an unbound tile is unknown rather than empty, so
neither is a candidate.
2026-07-29 18:16:21 -05:00
Brooklyn Nicholson
fb120f850d refactor(desktop): move the main-is-occupied check into the session door
newSessionOpensTab answers a question that isn't specific to the sidebar
"+" — is there a conversation on main that must not be discarded — and the
palette needs the same answer. Fold it into open-session as
mainChatOccupied so both callers share one definition.
2026-07-29 18:16:14 -05:00
brooklyn!
4b7f709843
Merge pull request #74436 from NousResearch/bb/update-mutex
fix(update): one updater at a time, and never roll an install backwards
2026-07-29 18:15:17 -05:00
brooklyn!
7002ed9f9d
Merge pull request #74427 from NousResearch/bb/pairing-approve-entry
fix(pairing): make the listed pending request approvable
2026-07-29 18:06:29 -05:00
brooklyn!
4a8697a9d1
Merge pull request #74438 from NousResearch/bb/thread-clearance
fix(desktop): stop an unowned publisher poisoning the thread clearance at :root
2026-07-29 18:05:41 -05:00
Brooklyn Nicholson
800c8a0458 fix(desktop): stop an unowned publisher poisoning the thread clearance at :root
The thread's bottom clearance is composer + status-stack + 2rem, and both
inputs are measured by JS onto the owning [data-chat-surface]. The surface-var
helpers fell back to document.documentElement when they couldn't resolve one —
but :root is where every surface's DEFAULTS live, so a single stale write there
becomes a global floor under every thread's clearance until reload.

An unowned publisher has nowhere to publish to, so it now publishes nowhere.
2026-07-29 17:57:49 -05:00
Brooklyn Nicholson
37519b4eeb fix(update): use the no-kill pid probe, not os.kill(pid, 0)
The Windows-footgun linter caught a real bug in the new update lock. On
Windows os.kill(pid, 0) is not a no-op: CPython routes sig=0 to
GenerateConsoleCtrlEvent, which sends Ctrl+C to the target's entire
console process group (bpo-14484). The liveness probe would have killed
the very updater it was asking about -- and any sibling sharing that
console.

Delegate to gateway.status._pid_exists, the project's existing no-kill
probe, which uses psutil (OpenProcess/GetExitCodeProcess on Windows) and
also reports zombies as dead. Any pid we cannot evaluate still counts as
dead so a corrupt marker cannot wedge the lock.
2026-07-29 17:52:06 -05:00
Brooklyn Nicholson
37d0766ba5 fix(pairing): keep GUI approvals off the code brute-force lockout
Follow-up hardening on the request-id grant path.

approve_request took the same lockout treatment as approve_code: gated by
it, and recording a miss toward it. But the two paths defend different
things. The lockout exists to stop guessing at the 8-char code space over a
messaging channel; a request id is only ever obtained by an admin already
authenticated to the store, so a miss means the row they clicked went stale.
Counting those let a handful of clicks on a stale list lock the operator out
of `hermes pairing approve` for an hour — the GUI DoSing the CLI.

Also drops the `code`/`code_hash_prefix` compat fields from list_pending.
The hash prefix is what admin surfaces mistook for an approvable code in the
first place, and re-exporting the request id under the old `code` key just
preserves the ambiguity; both consumers in the tree read `request_id` now.
The 16-hex sniffing that had been copy-pasted into the CLI and the endpoint
(where a chained conditional consulted it against the wrong field) moves to
one owner, PairingStore.looks_like_request_id.

The endpoint no longer reports a 429 on the request-id path, where lockout
can't apply — a stale id surfaced as a bogus "locked out" while the platform
sat locked for something else entirely.
2026-07-29 17:51:40 -05:00
solyanviktor-star
c352a322b0 fix(pairing): reset the failed-approval counter on a successful approval
approve_code()'s success path never cleared _failures:{platform}. The
counter is incremented on every non-matching code, persisted in
_rate_limits.json, and only ever reset to 0 when it reaches
MAX_FAILED_ATTEMPTS (firing the lockout). So it counts failures over the
gateway's entire lifetime, not consecutive ones.

An owner who mistypes a pairing code on a handful of separate occasions
— each time immediately retyping it correctly and successfully pairing —
accumulates those isolated typos. A later single fresh typo then hits
MAX_FAILED_ATTEMPTS and locks the whole platform out for an hour, at
which point _is_locked_out gates approve_code and even the *correct*
code is rejected.

Reset the counter on a successful approval, matching standard
brute-force-guard semantics (the counter tracks consecutive failures).
This does not weaken protection: an attacker cannot produce a success
without a valid code, and 5 consecutive wrong attempts still lock out.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-29 17:49:33 -05:00
Flownium
57ad222620 test: cover pairing CLI approval paths 2026-07-29 17:49:33 -05:00
Flownium
774d92fbfa fix: approve listed pairing requests 2026-07-29 17:49:33 -05:00
Brooklyn Nicholson
6a444ebec7 fix(installer): release the update marker on success, not just on drop
The updater keeps a Tauri/Cocoa event loop alive while it relaunches the
desktop, and that loop can outlive app.exit(0). Relying on Drop alone
left a *successful* update looking active -- a live pid holding a fresh
marker -- which blocked desktop startup and, now that the marker is also
the cross-process update lock, every subsequent updater until the age
ceiling expired.

Release explicitly once all install-tree mutations are done, before the
relaunch. complete() is idempotent so Drop still covers the failure and
panic paths. Arms a process-exit fallback so a wedged event loop cannot
leave a finished updater lingering as a live pid.

Co-authored-by: nateEc <nateEc@users.noreply.github.com>
2026-07-29 17:48:09 -05:00
Teknium
92856bc28a
Merge pull request #74383 from NousResearch/tests/prune-low-value
test: prune low-value tests suite-wide — 58% fewer tests, half the wall time, zero flakes
2026-07-29 15:46:36 -07:00
Brooklyn Nicholson
38d5f44df1 fix(installer): refuse a second updater instead of clobbering the marker
UpdateMarkerGuard::acquire overwrote the in-progress marker
unconditionally, so a Tauri update launched while a dashboard-spawned
"hermes update" was mid-flight simply took the marker and ran a second
updater over the same checkout. That is the race behind the reported
Windows failure: install-mode bootstrap rewound the tree while the
dashboard's updater was still running npm install against it.

acquire now returns Result and refuses when a live foreign owner holds
the marker, and Drop no longer deletes a marker this process does not
own. Liveness matches the Python and Electron readers of the same file:
dead pid or past the shared age ceiling means stale and reclaimable, so
a crashed updater cannot wedge future updates.

Adds a cfg(unix) libc dependency for the signal-0 liveness probe; the
Windows path uses OpenProcess/GetExitCodeProcess.
2026-07-29 17:46:00 -05:00
Brooklyn Nicholson
fe8e4d93da fix(update): make the in-progress marker a real cross-process lock
Three surfaces start updates against one checkout: a terminal
"hermes update", the dashboard's Update button (which spawns that same
command detached), and the desktop's, which hands off to the Tauri
updater. Only the Tauri updater published the in-progress marker, and
only Electron read it -- to gate backend startup, not to stop a second
updater. So a dashboard-spawned update and an installer-driven git
checkout could mutate the same tree concurrently, rewriting source under
a live interpreter.

Claim the same marker from cmd_update rather than adding a second
mechanism: same path, same pid+started_at payload the Rust and Electron
readers already parse. A marker only counts as live when its pid is alive
and it is inside the shared age ceiling, so a crashed updater self-heals
instead of wedging every future update. Release only removes a marker we
still own, leaving a handoff partner's claim intact.

Refusing exits 2, matching the existing concurrent-instance contract the
Tauri updater already recognizes.
2026-07-29 17:45:48 -05:00
Teknium
3913ac2ba0
test: pair load_config_readonly stubs at pruned-file sites (post-merge with main's readonly sweep)
Main's 243c9182b1/a16fd675df/7142dc4580 added load_config_readonly
sibling stubs across 38 files; our pruned versions of 11 of those files
kept only the load_config stubs. Re-applied the pairing at every
surviving site (26 patch()/setattr sites) — same return_value/
side_effect as the adjacent load_config stub. 494 tests green across
the 11 files.
2026-07-29 15:37:24 -07:00
Brooklyn Nicholson
b3daf1609c fix(installer): never let a stale --commit pin roll an install backwards
hermes-setup.exe bakes its build-time commit into the binary
(BUILD_PIN_COMMIT) and passes it as -Commit on every install-mode run,
including the retry the desktop's "Update didn't finish" screen kicks
off. The repository stage checked that SHA out unconditionally, so an
installer built months earlier rewound a current managed checkout to its
build commit -- 9,160 commits in the reported case -- leaving ancient
source against a current venv. npm then failed on workspaces that did not
exist yet at that commit, and every later update ran against the wrong
tree.

Skip the pin when its target is already an ancestor of HEAD. Fresh clones
have no such ancestry so reproducible/CI pinning is unchanged, and
--force-commit / -ForceCommit still rolls back on purpose.
2026-07-29 17:35:16 -05:00
Teknium
1cf5d3841b perf(cli): stop hermes -w stalling 30-60s on a flaky fetch in _resolve_worktree_base
The #71637 prune fix cut one stage of -w startup, but the base-ref
resolution right after it still ran an uncapped-in-practice
'git fetch origin main' (timeout=30) on every launch — and on a flaky
smart-HTTP connection that fetch intermittently stalled to the full 30s,
then cascaded into step 2's SECOND 30s fetch. Measured: back-to-back
fetches of 0.9s, 1.0s, 63.5s on the same box with healthy TLS (~185ms).

_resolve_worktree_base now:
- skips the fetch entirely when FETCH_HEAD is < 5 min old and the
  tracking ref exists (repeat launches pay zero network cost)
- caps the fetch at 5s and falls back to the locally-known tracking
  ref (labelled 'cached') on timeout/failure instead of cascading into
  a second fetch — genuine staleness stays backstopped by the pre-push
  stale-base gate
- caps 'git remote show origin' the same way

Worst case drops ~60s -> ~5s; warm path is ~0.02s (was up to 30.8s).
sync_base=False and the offline HEAD fallback are unchanged.
2026-07-29 15:34:53 -07:00
Brooklyn Nicholson
f8e07a332e feat(desktop): platforms.changed broadcast retires the Messaging page's 6s status poll
The change watcher (#73673) missed one always-on-while-mounted timer: the
Messaging page polled /api/messaging/platforms every 6s for connection
status. The gateway already persists platform connect/disconnect/health to
gateway_state.json, so watch that file's mtime and broadcast
platforms.changed (floored to 5s — the gateway also rewrites the file for
in-flight-count bookkeeping), route it through live-sync like its
siblings, and refresh the page on the tick. Older backends keep the
legacy visible-tab poll verbatim.

Finishes the always-on poll sweep for #73618.
2026-07-29 17:33:35 -05:00
teknium1
7142dc4580 test: complete the readonly-stub pairing sweep (6 more files)
Final pass: per_model_threshold_init_ordering, memory_provider_init,
plugin_context_engine_init, api_max_retries_config,
invalid_context_length_warning, tool_call_guardrail_runtime all stub
agent_init config reads that now resolve through load_config_readonly().
Verified by running the complete 59-file suspect list (every test file
that stubs load_config in string or attribute form and intersects the
swapped modules): 3,063 tests, 0 failed.
2026-07-29 15:28:15 -07:00
teknium1
a16fd675df test: pair attribute-form load_config stubs with readonly siblings
Second stub shape the first sweep missed: monkeypatch.setattr(config_mod,
"load_config", ...) — attribute-form instead of string-form. Four files
(compression_max_attempts, preflight_compression_cap_e2e,
codex_gpt55_autoraise_notice, proactive_prune_config) stub agent_init
config reads that now go through load_config_readonly(). Swept the whole
tree for the attribute form; remaining hits stub modules that still use
the mutable loader. 1,210 tests green across the 19-file re-sweep.
2026-07-29 15:28:15 -07:00
teknium1
243c9182b1 test: patch load_config_readonly alongside load_config across affected suites
The readonly swaps mean agent/ modules now read config through
load_config_readonly(); tests that stubbed only load_config stopped
intercepting those reads. Added sibling readonly stubs (same
return_value/side_effect/lambda) at every affected site across 11 test
files, found via a tree-wide sweep of load_config stubs cross-referenced
against the swapped modules. 3,915 tests green across the 38-file
sibling sweep.
2026-07-29 15:28:15 -07:00
teknium1
97aba01406 test: patch load_config_readonly alongside load_config in run_agent init tests
agent_init's config reads now go through load_config_readonly(); 8
tests that stubbed only load_config stopped intercepting them. Each of
the 10 patch sites gains a sibling readonly patch with the same
return_value. 461/461 file-local tests green.
2026-07-29 15:28:15 -07:00
teknium1
d3cd6f4158 test: cache-aliasing regression tests for provider normalization
Pins the audit findings: normalizer never mutates its input
(api_key_env + camelCase forms), providers-dict round-trips leave a
cached config byte-identical, and the normalized models mapping does
not alias the caller's dict.
2026-07-29 15:28:15 -07:00
Gabriel Stoltemberg
59ee85ed50 perf: use load_config_readonly() at read-only call sites in agent/
Salvaged from #56085 (@Stoltemberg), rebased onto current main: sites
main had already converted (credential_pool, auxiliary_client MoA
paths, model_metadata, moa_loop, agent_runtime_helpers) resolve to
main's versions; the remaining ~29 read-only sites across 16 agent/
files swap to the no-deepcopy readonly loader (~135us saved per call).

Full per-site mutation audit performed (every enclosing function read,
escapes traced): 23 SAFE, 5 ESCAPES with read-only consumers, 1 UNSAFE
path (init_agent -> get_compatible_custom_providers -> normalizer
in-place alias writes) fixed by the preceding no-mutate commits, which
make the normalizer copy-safe for ALL callers.
2026-07-29 15:28:15 -07:00
teknium1
3c6e7b1b11 fix(config): don't share the cached models mapping with normalized entries
Companion to the no-mutate fix: normalized['models'] retained a
reference to the caller's (possibly cached) models dict, and the
normalized entry escapes into long-lived runtime state
(agent._custom_providers). Shallow-copy so runtime writes can never
reach the shared config cache.
2026-07-29 15:28:15 -07:00
golldyck
bfe4cbdc2a fix(config): stop _normalize_custom_provider_entry mutating the caller's dict
The normalizer writes alias keys into the entry it is given
(entry['key_env'] = entry['api_key_env'] and entry[snake] =
entry[camel]) while building its normalized copy. Two of its three
callers — get_compatible_custom_providers and
providers_dict_to_custom_providers — pass live sub-dicts straight
from load_config_readonly()'s shared cache (only
_custom_provider_entry_to_provider_config defends with dict(entry)).

A config written with the documented camelCase / api_key_env aliases
therefore gets its cached copy polluted with injected duplicate keys,
violating the cache's explicit no-mutation contract; every later
load_config() deepcopy inherits the duplicates, and any
save_config(load_config()) flow (setup wizard, dashboard writes,
model persist) writes them back to config.yaml. The aux-client TLS
resolution runs this on every auxiliary client build, so the
mutation also happens unlocked on worker threads against a shared
object.

Shallow-copy the entry up front; the function's return value is a
separately-built dict, so behavior is otherwise unchanged.
2026-07-29 15:28:15 -07:00
Teknium
7c5a98d888
Merge remote-tracking branch 'origin/main' into tests/prune-low-value
# Conflicts:
#	tests/run_agent/test_conversation_fallback_state.py
2026-07-29 15:24:14 -07:00
Teknium
25927884e0 docs(acp): document the Buzz Desktop model picker
Buzz Desktop v0.5.1 now renders Hermes' ACP model menu in agent runtime
settings. Add a short note under the Buzz Desktop host section explaining
where the list comes from (the shared authenticated-provider inventory),
the provider:model / custom:<name>:<model> ID shapes, and that a pick is
session-scoped rather than a Hermes-wide default change.
2026-07-29 15:21:39 -07:00
Soju06
ce9f6712ff refactor(agent): remove the inter-tool delay
The 1.0s sleep between sequential tool calls has been present since the
initial commit with no documented rationale. It sleeps between local
tool executions — the next LLM request goes out only after the whole
batch — so it rate-limits nothing, and the parallel read-only path
already runs with no delay. Every multi-tool turn pays (N-1) seconds
of dead time. Remove the sleep, the internal tool_delay plumbing, and
dead test assignments. AIAgent.__init__ keeps tool_delay as a
deprecated no-op keyword for one release so existing programmatic
callers construct cleanly; passing it emits a DeprecationWarning.
2026-07-29 15:21:26 -07:00
Teknium
1a088989bc
Merge pull request #66730 from NousResearch/feat/hsp-sync-client
feat(sync): HSP/1 personal skill sync client (M1 client)
2026-07-29 15:20:35 -07:00
Teknium
fc551f9691
Merge pull request #72103 from victor-kyriazakos/feat/relay-slack-blockkit-native-parity
fix(relay): Slack DM-root prompts + flat-DM edit-streaming (native _resolve_thread_ts parity)
2026-07-29 15:13:42 -07:00
Teknium
a17ac2ca67
Merge remote-tracking branch 'origin/main' into tests/prune-low-value
# Conflicts:
#	tests/agent/test_context_compressor.py
#	tests/gateway/test_startup_restart_race.py
#	tests/hermes_cli/test_voice_wrapper.py
2026-07-29 15:13:21 -07:00
Teknium
28524adb0e fix(tests): eliminate flaky/broken tests — shadow sys.path inserts, unmocked network in compressor tests, stale-SDK feishu pin guard, quadratic redact regexes
- Remove tests/-shadowing sys.path.insert(dirname/'..') from 11 test files:
  it prepended the tests/ dir itself to sys.path, so 'import agent' /
  'import hermes_cli' resolved to the test packages and collection died
  with ModuleNotFoundError depending on import order (2 files failed in
  every full-suite run; 9 more were latent).
- Patch call_llm in 5 context-compressor tests that called compress()
  unmocked: each burned ~50s attempting live LLM traffic through the
  relay before falling back (572s file — the slowest in the suite, and
  flaky under the 300s per-file timeout). File now runs in ~5s.
- agent/redact.py: fix two catastrophically-backtracking regexes hit by
  the compressor's redaction pass on large payloads —
  _STRICT_URL_USERINFO_RE anchors on the mandatory '//' (optional-scheme
  prefix backtracked O(n^2): ~55s on a 320KB payload, now sub-ms;
  output-equivalence fuzz-verified on 20k random strings), and the
  _CFG_DOTTED_RE/_CFG_ANCHORED_RE subs gain an exact linear keyword
  pre-gate so secret-free text skips the quadratic pattern entirely.
- tests/gateway/test_feishu.py: version-guard the extra_ua_tags SDK
  signature check; the repo pins lark-oapi==1.6.8 but stale local
  installs (1.5.3) fail the assertion — skip below the pin.
- tests/tools/test_managed_browserbase_and_modal.py: stub
  agent.redact + agent.credential_persistence in the fake agent package
  (empty __path__ blocks all real agent.* imports added since the fake
  was written).
- tests/gateway/test_startup_restart_race.py: raise wait_for timeouts
  2s -> 30s; 2s wall-clock on a loaded 40-worker box flaked in the
  baseline run (passes instantly when the box is quiet).
2026-07-29 15:12:28 -07:00
kshitijk4poor
1f70ba6bca fix(mcp): propagate cancellation untouched in _connect_server orphan reap
Follow-up to the salvaged #62026 ownership fix, folding in #72054's
CancelledError rule by @adurham: start() already cancels/reaps its own
run task when the caller's connect timeout cancels start() itself, so
_connect_server() must propagate cancellation without awaiting a
redundant shutdown() inside a cancelled context. Non-cancellation
failures on the unclaimed (standalone probe) path still reap the parked
task, now with the reap failure logged instead of raising over the real
error.

Also maps mrz@mrzlab630.pw for the attribution check.

Co-authored-by: Adam Durham <amdnative@gmail.com>
2026-07-30 03:35:35 +05:30
Stepan Zadolia
c00a1d58d5 fix(mcp): retain parked startup tasks for clean shutdown 2026-07-30 03:35:35 +05:30
Seppe Gadeyne
fd5397d69a chore(release): map shady2k contributor email 2026-07-30 03:35:35 +05:30
Seppe Gadeyne
ab0d3fac3d fix(mcp): keep drain and stop on loop thread 2026-07-30 03:35:35 +05:30
Seppe Gadeyne
cac74e06c8 fix(mcp): bound loop-owned shutdown drain 2026-07-30 03:35:35 +05:30
shady
cc21c4f78f fix(mcp): drain pending tasks before closing the MCP loop
_stop_mcp_loop() stopped and closed the background loop without reaping
the tasks still on it. A task left suspended is resumed later by the GC,
whose finalizer drives its cleanup against the now-closed loop:

    Exception ignored in: <coroutine object MCPServerTask.run ...>
      File "tools/mcp_tool.py", line 2947, in run
        parked = await self._wait_for_reconnect_or_shutdown(
      File "tools/mcp_tool.py", line 2161, in _wait_for_reconnect_or_shutdown
        t.cancel()
    RuntimeError: Event loop is closed

shutdown_mcp_servers() only reaps servers held in _servers, so a server
that parked after exhausting its initial-connect budget — never inserted
there, because start() raises _error before the caller registers it — has
no owner to signal it and stays suspended until the loop is gone.

Drain the loop the way asyncio.run() does: cancel the remaining tasks and
gather them while the loop is still open, so each runs its own finally.
Cancel alone is not enough — Task.cancel() only schedules the throw.

This resolves the reported traceback, but not the ownership bug that
strands the task in the first place; that needs a follow-up. Deliberately
not using "Fixes" so #60197 stays open for it.

Addresses #60197
Addresses #66113

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-30 03:35:35 +05:30
Seppe Gadeyne
eded89ace2 test(mcp): cover parked shutdown drain path 2026-07-30 03:35:35 +05:30