fix(anthropic): symmetric orphan audit for web_search_tool_result by adurham · Pull Request #1 · adurham/hermes-agent

adurham · 2026-05-13T21:00:32Z

Summary

drop_orphan_server_tool_uses_in_storage only scanned tool_search_tool_*_tool_result blocks when deciding which server_tool_use blocks were paired. web_search_tool_result was invisible to it, so healthy web_search pairs got broken: the function dropped the server_tool_use thinking it was unpaired, and every subsequent request 400'd on the now-orphaned web_search_tool_result.
Fix the audit to recognise web_search_tool_result as a paired-result type AND drop orphans in BOTH directions (use without result, result without use). Apply the same symmetric drop to the outbound wire-build twin in convert_messages_to_anthropic so a session that hits this state mid-turn still survives the next request.

Repro / how this was found

Session 20260513_093942_d374cc started 400ing every request with:

unexpected `tool_use_id` found in `web_search_tool_result` blocks:
srvtoolu_01XyDgKcEqDSm8udPKWPNBsP. Each `web_search_tool_result`
block must have a corresponding `server_tool_use` block before it.

Inspection of the persisted blocks: the offending message had two web_search_tool_result entries and zero server_tool_use entries. The matching uses had existed on disk when the response was captured (transport handler at agent/transports/anthropic.py:140-147 captures both block types), but drop_orphan_server_tool_uses_in_storage had erased them on a later persist.

Test plan

tests/agent/test_anthropic_tool_search_roundtrip.py — 54 pass (46 pre-existing + 8 new regression tests in TestDropOrphanServerToolUsesInStorage)
tests/agent/test_anthropic_adapter.py, test_apply_tool_search_modes.py — 243 pass
Full tests/agent/ — 2791 pass, 24 skipped, 0 regressions (one pre-existing xdist-ordering flake in test_vision_resolved_args.py, passes in isolation, unrelated to this change)
Sanitizer applied to the wedged session — orphan IDs verified gone, session resumable

Notes

Three pre-existing roundtrip tests fed lone *_tool_result blocks with no matching server_tool_use through _build_assistant_msg. That shape is unrealistic (Anthropic itself would 400 on it) and only passed because the old code was permissive in the wrong direction. The fixture now auto-injects a paired server_tool_use for each result block, matching what real responses always carry.
Extend SERVER_TOOL_RESULT_TYPES (in both agent/anthropic_adapter.py locations) when Anthropic adds new server-side tools that emit a paired result block.

drop_orphan_server_tool_uses_in_storage was scanning only tool_search_tool_*_tool_result blocks when collecting paired-result IDs. web_search_tool_result was invisible to it. So when an assistant message contained a healthy server_tool_use + web_search_tool_result pair, the function decided the server_tool_use was unpaired and DROPPED it, leaving the result block orphaned forever. Every subsequent API call then 400'd: unexpected `tool_use_id` found in `web_search_tool_result` blocks: <id>. Each `web_search_tool_result` block must have a corresponding `server_tool_use` block before it. Fix both audits (storage + outbound wire) to: * recognise web_search_tool_result as a paired-result type * drop orphans in BOTH directions (use without result, result without use) Verified against session 20260513_093942_d374cc, which this function had wedged exactly as described. Existing session_20260509_145003_c5e465 (the tool_search-side orphan that motivated the original sanitizer) still recovers. Three pre-existing roundtrip tests fed lone *_tool_result blocks with no matching server_tool_use through the fixture — an unrealistic shape Anthropic itself would have 400'd on. Updated the shared _build_assistant_msg fixture to auto-inject a paired server_tool_use, matching real responses. Added 8 new regression tests in TestDropOrphanServerToolUsesInStorage covering: healthy pairs survive both families, orphan use drops, orphan result drops (the new direction), mixed scenarios, splits across messages, and end-to-end wire-shape validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

adurham · 2026-05-13T21:00:52Z

Re-opened against upstream as NousResearch#25234.

github-actions · 2026-05-13T21:01:43Z

🔎 Lint report: `adam/orphan-web-search-tool-result-fix` vs `origin/main`

ruff

Total: 6 on HEAD, 6 on base (➖ 0)

🆕 New issues (4):

Rule	Count
`PLW1514`	4

First entries

scripts/hermes_token_check.py:43: [PLW1514] `pathlib.Path(...).read_text` without explicit `encoding` argument
scripts/hermes_token_check.py:54: [PLW1514] `open` in text mode without explicit `encoding` argument
tools/bridges/cc_proxy_mcp.py:244: [PLW1514] `open` in text mode without explicit `encoding` argument
scripts/hermes_usage_tracker.py:138: [PLW1514] `open` in text mode without explicit `encoding` argument

✅ Fixed issues (4):

Rule	Count
`PLW1514`	4

First entries

../../../../../tmp/lint-base/scripts/hermes_token_check.py:43: [PLW1514] `pathlib.Path(...).read_text` without explicit `encoding` argument
../../../../../tmp/lint-base/scripts/hermes_token_check.py:54: [PLW1514] `open` in text mode without explicit `encoding` argument
../../../../../tmp/lint-base/scripts/hermes_usage_tracker.py:138: [PLW1514] `open` in text mode without explicit `encoding` argument
../../../../../tmp/lint-base/tools/bridges/cc_proxy_mcp.py:244: [PLW1514] `open` in text mode without explicit `encoding` argument

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8290 on HEAD, 8289 on base (🆕 +1)

🆕 New issues (1):

Rule	Count
`invalid-argument-type`	1

First entries

tests/agent/test_anthropic_tool_search_roundtrip.py:1342: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `Overload[(key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> LiteralString, (key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> str]` cannot be called with key of type `Literal["type"]` on object of type `str`

✅ Fixed issues: none

Unchanged: 4380 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

Seven Copilot inline review comments on NousResearch#37679, four worth landing in a polish pass before merge: 1. _dispose_unused_adapter signature: 'BasePlatformAdapter' -> 'BasePlatformAdapter | None'. The function explicitly handles None and the reconnect watcher calls it with None in the except arm, so the annotation now matches the actual contract. 2. (duplicate of #1 on a different line) — same fix. 3. except Exception in _dispose_unused_adapter — the reviewer asked about asyncio.CancelledError swallowing. On Python 3.8+ (Hermes requires 3.13, see pyproject.toml), CancelledError inherits from BaseException, NOT Exception, so the existing 'except Exception' does NOT swallow task cancellation. Added an explicit comment explaining the contract so future readers don't repeat the analysis. We don't re-raise because the watcher loop intentionally treats dispose failures as best-effort: a failed dispose on an unowned adapter should not take down the watcher that's keeping the gateway alive. 4. _response_store = None after close in api_server.py — the reviewer flagged this for idempotency. Decided to keep the non-None state intentionally: setting it to None cascades to ~9 callers that access self._response_store without a None check, and 'close() is idempotent on a closed sqlite3 Connection' means the current code is already safe. The type stays stable; LSP doesn't flag a cascade of reportOptionalMemberAccess errors. (This matches the pre-existing pattern in the codebase — e.g. _mark_disconnected doesn't reset state to None either.) 5. _build_adapter_with_store: reviewer worried about disconnect() failing on the self.name property if __init__ wasn't called. Already handled: we set 'adapter.platform = Platform.API_SERVER' so the 'self.platform.value.title()' property returns 'Api_Server' without raising. The exception-swallowing branch in disconnect() does call self.name via the logger.debug format, so this is a real path that needs the platform attribute, and we have it. 6. test_disconnect_closes_response_store: bare 'pytest.raises(Exception)' -> 'pytest.raises(sqlite3.ProgrammingError)'. The bare Exception matcher would silently accept AttributeError, OperationalError, env-related issues, etc. The specific exception type ('Cannot operate on a closed database') is the actual signal we want — proves the SQLite conn is closed, not just that *something* raised. 7. test_nonretryable_failure_disposes_unowned_adapter: assertion tightened from '>= 1' to '== 1' on adapter._disconnect_calls. The docstring said 'exactly once', the assertion now matches. Catches the hypothetical 'watcher disposes the same adapter twice' regression that '>=' would have missed.

…ch#37677) Anthropic enforces two independent ceilings per image: 1. 5 MB encoded byte size 2. 8000 px longest side Hermes only guarded #1. A tall screenshot (e.g. 1200x12000 at 0.06 MB) passes every byte check but fails the pixel check, returning a non-retryable HTTP 400 that permanently bricks the conversation thread. Fixes: - error_classifier: add 'image dimensions exceed' pattern to _IMAGE_TOO_LARGE_PATTERNS so the 400 is classified as image_too_large and triggers the shrink/retry path instead of falling through to non-retryable error. - conversation_compression: check pixel dimensions (via Pillow) even when byte size is under the 4 MB target. If max(dims) > 8000, force shrink. - vision_tools._resize_image_for_vision: add optional max_dimension param. When set, images exceeding the pixel cap are downscaled even if they're under the byte budget. The resize loop now checks both byte AND pixel limits before accepting a candidate. Closes NousResearch#37677

…bes + test-leak fix (NousResearch#40909) * fix(gateway,windows): reliability — supervisor task, JOB breakaway, status --deep Three coordinated fixes for the Windows gateway reliability story: 1. CREATE_BREAKAWAY_FROM_JOB on every detached spawn The 'hermes update' triggered from the Electron Desktop GUI ran inside Electron's job object. Without breakaway, the post-update gateway watcher spawned by update — already DETACHED_PROCESS — was still reaped when Electron's job tore down, so the gateway never came back after a GUI-initiated update. Adds CREATE_BREAKAWAY_FROM_JOB (0x01000000) to: - hermes_cli/_subprocess_compat.py::windows_detach_flags() — used by every helper that calls windows_detach_popen_kwargs(), including launch_detached_profile_gateway_restart() - The watcher subprocess's own respawn snippet in hermes_cli/gateway.py (inlined flags so the watcher's child respawn also breaks away) _spawn_detached() in gateway_windows.py already had the flag; this change brings the rest of the codebase to parity. 2. Per-minute supervisor Scheduled Task — Windows equivalent of systemd Restart=always Introduces hermes_cli/gateway_supervisor.py and registers it as a second Scheduled Task ('Hermes_Gateway_Supervisor', SC MINUTE /MO 1, LIMITED rights) alongside the existing ONLOGON task. Every minute, the supervisor uses the same gateway.status.get_running_pid() probe as 'hermes gateway status' and, if no gateway is alive, calls gateway_windows._spawn_detached() (which now includes BREAKAWAY) to bring one back. Covers every crash mode, not just 'machine rebooted': taskkill, OOM, GUI update SIGTERM, parent job teardown. Cheap — one pythonw startup per minute when down, one PID-existence check per minute when up. Wired into both the schtasks-success and Startup-folder-fallback install paths via _install_supervisor_best_effort(), and removed in uninstall(). Best-effort: a failing supervisor install logs a warning but doesn't roll back the primary install. 3. 'hermes gateway status --deep' shows per-probe PASS/FAIL Replaces the existing terse '--deep' output (which only printed paths) with an actual diagnostic table: [1] PID file present [2] Lock file held by a live process [3] get_running_pid() result [4] _pid_exists(pid) — OS-level liveness [5] gateway_state.json (state + age) [6] Last lifecycle event from gateway-exit-diag.log When the high-level summary disagrees with reality, the user can see exactly which signal is lying. Test-leak fix ------------- tests/hermes_cli/test_gateway_wsl.py::TestGatewayCommandWSLMessages monkey-patched is_linux/is_wsl/supports_systemd_services to simulate WSL but did NOT stub is_windows(). On a Windows host, the dispatcher in _gateway_command_inner takes the is_windows() branch BEFORE the WSL guidance branch, so the test invoked gateway_windows.install() for real. install() writes to %APPDATA%\...\Startup\Hermes_Gateway.cmd — the REAL user Startup folder, never sandboxed by tmp_path — pointing at the test's pytest-of-<user>/pytest-<N>/.../gateway-service/ wrapper. When pytest tore down the tmp_path, every subsequent Windows login flashed a cmd.exe window that failed to find the missing target. Stubs is_windows=False on all four affected tests: test_install_wsl_no_systemd test_start_wsl_no_systemd test_status_wsl_running_manual test_status_wsl_not_running Defense-in-depth: _build_startup_launcher() now prefixes the launcher with 'if not exist <target> exit /b 0', so any future stale Startup entry silently no-ops instead of flashing a console window. Status enhancements ------------------- - status() now reports supervisor task presence alongside the existing schtasks/Startup info, and nudges the user to reinstall if the supervisor isn't registered. - Deep mode dumps both the supervisor task name + script path. * fix(gateway,windows): drop the per-minute supervisor task — keep breakaway + deep probes Earlier in this branch we added a per-minute schtasks-based supervisor to respawn the gateway after crashes / GUI-update SIGTERMs. The implementation flashed a brief console window on every firing, which stole window focus. We tried several variants: - cmd.exe wrapper invoking pythonw -> flashes (cmd.exe is console-subsystem) - schtasks /TR pointing at pythonw -> flashes (uv venv launcher pythonw is actually subsystem=Console, not GUI; it respawns the real pythonw) - schtasks /TR pointing at base uv -> still flashes (Task Scheduler-side conhost preallocation; documented Windows quirk) - XML registration with <Hidden>true> -> still flashes (<Hidden> only hides the task in the Task Scheduler UI, not the spawned window) Researched what leading projects do: - Ollama: GUI-subsystem tray exe + Startup-folder shortcut. No supervisor. - Tailscale: real Windows Service via SCM. Session 0, no console possible. - Syncthing: --no-console flag inside the binary + Startup folder. - openclaw: VBS Run(..., 0, False) wrapper. Suppresses the *window* but Super User Q971162 confirms focus-steal still occurs in some cases. None of these use a per-minute polling scheduled task. The 'auto-restart on crash' responsibility belongs INSIDE the daemon (Tailscale's in-process recovery / Ollama's monitor+worker pair) OR is delegated to the Windows Service Control Manager — not Task Scheduler. So this commit drops the supervisor entirely. The CREATE_BREAKAWAY_FROM_JOB fix in _subprocess_compat.py (from commit c1e5fa4) survives — that is the *real* fix for problem #2 (GUI-update kills gateway): the post-update watcher in launch_detached_profile_gateway_restart() now breaks out of Electron's job object, so the gateway respawn watcher survives the GUI quit and successfully respawns the gateway. Surviving from c1e5fa4: * CREATE_BREAKAWAY_FROM_JOB in hermes_cli/_subprocess_compat.py (fixes #2) * Inlined breakaway flag in the watcher respawn snippet in gateway.py * hermes gateway status --deep PASS/FAIL probes (fixes #1 — visibility) * 'if not exist <target> exit /b 0' guard in _build_startup_launcher (fixes NousResearch#3 — silent no-op for stale Startup entries) * tests/hermes_cli/test_gateway_wsl.py is_windows=False stubs (root cause of NousResearch#3 — pytest WSL tests no longer leak Startup entries on Win hosts) Removed in this commit: * hermes_cli/gateway_supervisor.py (entire file) * Supervisor section in hermes_cli/gateway_windows.py (~180 lines): get_supervisor_task_name, get_supervisor_script_path, _build_supervisor_cmd_script, _write_supervisor_script, _install_supervisor_task, is_supervisor_task_registered, _install_supervisor_best_effort * _install_supervisor_best_effort() calls in install() (3 spots) * supervisor cleanup block in uninstall() * supervisor display lines in status() / status(deep=True) Future direction (out of scope for this PR): the right place for Windows 'Restart=always' semantics is a real Windows Service installed via pywin32's win32serviceutil.ServiceFramework — session-0 isolation, SCM auto-restart, no console window possible. That's a meaningful next-PR project, not a band-aid. Tests: 51 pass / 2 pre-existing failures in tests/hermes_cli/test_gateway_{windows,wsl}.py (the 2 failures are TestSupportsSystemdServicesWSL cases that fail on origin/main too — unrelated to this PR).

Phase 1 of the pluggable cron-scheduler refactor (Axis B — the trigger). No call-site changes; this phase only makes the abstraction exist + tested in isolation. Task 1.1: cron/scheduler_provider.py — the EXPERIMENTAL CronScheduler ABC. Required surface is name + start; is_available()/stop() carry safe defaults. is_available has a no-network invariant. Docstring marks it experimental until the Chronos provider (Phase 4) validates the shape. Task 1.2: InProcessCronScheduler wraps the historical 60s ticker loop, calling cron.scheduler.tick(sync=False) exactly as the raw ticker does. Uses stop_event.wait(interval) for responsive stop (both raw tickers already do). Tests: ABC-is-abstract, default-is_available, the InProcess loop drives tick and stops, stop() no-op, and test_abc_growth_stays_additive (the forward-compat guard: required abstractmethods must stay exactly {name, start}, so the three Phase-4 hooks land as NON-abstract additions). tick() internals in cron/scheduler.py are byte-unchanged (only new file added). Phase 0 characterization tests still green. Full tests/cron/: 445 passed.

…onnect ladder can't freeze silently (NousResearch#66377) The Telegram gateway could go silently deaf for hours: the reconnect ladder stalled mid-way (e.g. "attempt 4/10, reconnecting in 40s" then nothing) while the process stayed active(running), so Restart=always never fired. Root class: every recovery path — the ladder's re-entry (_schedule_polling_recovery), the pending-update probe (_probe_pending_updates), and PTB's error callback — gates new recovery on _polling_error_task.done(). If that single task wedges on any hung await, all recovery returns early forever and nothing retries. The heartbeat loop is a separate task, so make it an independent, cause-agnostic watchdog: if the same recovery task stays in-flight past _POLLING_ERROR_TASK_STUCK_TIMEOUT (300s — well beyond a healthy ladder attempt's bounded stop+drain+start+backoff), force a retryable-fatal so the background reconnector rebuilds the adapter instead of relying on the frozen ladder. This guarantees progress regardless of *where* the stall is (issue direction #1), tracked locally so no task-assignment site needs to change. Also salvages @koduri-mahesh-bhushan-chowdary's NousResearch#66492 (drain-await timeout), which closes the one concrete wedge vector documented in the incident (_drain_polling_connections' unbounded shutdown()/initialize() on a wedged CLOSE-WAIT pool). The watchdog covers the rest of the class. Co-authored-by: Koduri Mahesh Bhushan Chowdary <mkoduri73@gmail.com>

…reaming Two real render-cost wins found by inspection (no behavior change): 1. Sidebar re-rendered on every stream token. $sessionStates is republished on every message delta (tens/sec during a turn), and the derived ID computeds ($workingSessionIds, $attentionSessionIds, $backgroundRunningSessionIds) allocated a fresh array each time. nanostores notifies on !==, so the whole ChatSidebar + every mounted row re-rendered per token even when the working/ attention/background set was unchanged. Return the previous array reference when the contents match → nanostores skips the notify unless the set actually changes. Turns streaming from O(visible rows)/token into O(0) for the sidebar. 2. Tool rows normalized the FULL uncapped detail every render. `looksRedundant` (lowercase + whitespace-collapse over the entire read_file/terminal payload) ran twice in the ToolEntry render body, so every completed tool re-normalized its whole output on every stream tick of the running message. Memoize on the view fields so it recomputes only when the tool's content changes. Both are correctness-preserving (stable refs + memoization). The CI stream scenario drives $messages directly, not the publishSessionState path, so it won't reflect #1 — verified by inspection.

Two compounding bugs made the wander state machine completely inert despite Roam being on: 1. ROOT CAUSE: `usePetRoam`'s `schedule()` only calls `requestAnimationFrame` when its local `raf` id is falsy, but nothing ever cleared that id after the browser "spent" it -- so the rAF loop scheduled exactly ONE frame per effect mount and then went silently dead forever, no matter how long roam stayed enabled. `raf` is now cleared at the very top of `step()`, before any early-return or scheduling decision, so every code path's `schedule()` call actually queues the next frame. 2. MASKING BUG: the effect's `enabled` prop used to be `roamEnabled && active && !overlayActive && canRoam`, where `canRoam` flips false on every turn completion / clarify prompt / error / celebrate beat during a real chat session. Each flip tore down and rebuilt the WHOLE physics closure (phase, walk target, dwell timer, fall/jump integrators) via the effect's dependency array -- which incidentally re-armed bug #1's dead `raf` and let exactly one more frame through before dying again. This made the pet APPEAR to nudge forward only at the instant something forced an activity transition ("standing by/user prompt breakout" per the user's own diagnosis), never as continuous, self-driven wander -- and it's why the pane-resize and jump-bob fixes shipped in the two prior commits didn't visibly change this symptom at all. Fix: split the activity-gating signal into a new `canMove` prop, read via a ref (`canMoveRef`) inside the step loop instead of the effect's dependency array. Toggling `canMove` now freezes physics exactly where they are (still calling `schedule()` so the loop stays alive, with `last` advancing every frame so the dt clamp never has to absorb the whole frozen span at once) and resumes the SAME state on the next tick, instead of resetting. `enabled` (still an effect dependency) is now only the genuinely structural gate: roam opted in, pet loaded, not popped out -- expected to change rarely, so tearing down/rebuilding on ITS flips is correct and unchanged. Added apps/desktop/src/components/pet/use-pet-roam.test.tsx: a harness component + fake-rAF driver proving (1) the loop keeps scheduling a new frame every tick indefinitely (direct regression guard for bug #1 -- fails against the pre-fix code), (2) a `canMove` flip freezes/resumes the same closure rather than resetting it, (3) an `enabled` flip still legitimately resets everything (so the fix didn't just stop resetting anything at all). Files: apps/desktop/src/components/pet/use-pet-roam.ts, apps/desktop/src/components/pet/floating-pet.tsx (new `canMove`/`enabled` split at the usePetRoam call site), apps/desktop/src/components/pet/use-pet-roam.test.tsx (new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ch#67140) The background write guard decided ownership from `isinstance(usage_rec, dict)`, so a local skill with NO usage record passed. That successful write called bump_patch(), which created a `created_by: null` record — and the identical write was refused from then on. "Allowed exactly once, then never" is a race with our own bookkeeping, not a policy. Reproduced on main: patch #1 succeeds, patch #2 with the same arguments is refused. Option B from the issue. Option A (split `session_review` from `scheduled_curator` and let the session fork patch user-owned skills it consulted) would widen autonomous write permission onto skills the user owns with no user present to consent — wrong direction for a no-user-present actor. - skill_manager_tool: missing and explicit-null records now resolve IDENTICALLY, both fail closed. The refusal names the reason and points at `hermes curator adopt <name>`. - background_review: both review prompts told the reviewer to patch any skill consulted in the session and claimed pinned skills could be improved, while enforcement refused both. Prompts now list pinned, external, and user-owned skills as protected, and tell the reviewer to RECOMMEND adoption instead of attempting a write that will be refused. - skill_usage: document that `created_by` is a curator-management policy flag, not a provenance claim, and add `is_curator_managed()` so call sites read as the question they ask. Field name retained — it is on disk in every `.usage.json` and renaming would strand those records. - curator CLI: `hermes curator list-unmanaged` itemizes unmanaged skills with the reason each is unmanaged (completes the NousResearch#67139 spec). Foreground writes are untouched: a user-directed edit to a user-owned skill still works, including on pinned skills. Sibling tests: 9 failures in test_skill_manager_tool.py were fixtures that created record-less skills to exercise OTHER guards (consolidation-delete, read-before-write) and relied on ownership falling through. Fixed at the fixture, since the real curator only ever operates on managed sediment. One test asserted the old "manually authored" wording; rewritten to assert the behavior contract instead of the string. Validation: 274 targeted tests + all 7 background-review files (60 tests) pass. E2E on a temp HERMES_HOME (30 checks) covers the flip, foreground writes, adoption unblocking, pin semantics, prompt/enforcement parity, and the new verb. Each new test sabotage-verified: revert the fix, confirm it goes red. Fixes NousResearch#67140

…own (NousResearch#74136) Fix-up for the cherry-picked cooldown persistence: the PR's tests mocked the DB (SimpleNamespace(_db=MagicMock())), which cannot prove the cooldown survives a restart. Replace with the production shape — a real SessionDB on disk behind the real AsyncSessionDB facade — and add a restart regression: fail a hygiene compression on runner #1, tear it down, build a fresh GatewayRunner on the SAME database, and assert the cooldown is still honored (no compression agent instantiated). Also updates the timeout test to assert the DB-backed record_compression_failure_cooldown write instead of the removed in-memory dict. Sabotage-verified: reverting gateway/run.py to the in-memory dict makes the restart test fail.

Users following abbreviated links guess /docs/quickstart and /docs/installation and hit raw GitHub-Pages 404s — the real pages live under /docs/getting-started/. Add client redirects for both. Consumer-onboarding audit finding #1, Aug 2026.

…y present Per AI triage review on PR NousResearch#82095: 1. _pin_static_anthropic_token() (and its config read) was called unconditionally at the top of resolve_anthropic_token() on every invocation, even when neither ANTHROPIC_TOKEN nor CLAUDE_CODE_OAUTH_TOKEN is set and the pin can never apply. Moved the call into the #1/#2 branches where it's actually consulted, so the config read is skipped entirely for the common case. 2. Switched from load_config() (deepcopy variant) to load_config_readonly() -- this helper only reads the flag, never mutates it, and load_config_readonly() skips the ~265us deepcopy load_config() pays on every cache hit. Updated the 3 existing pin_anthropic_token tests' mock targets to load_config_readonly, and added 2 new regression tests: one asserting _pin_static_anthropic_token() is never called when no static env token is present (confirmed fails pre-fix via git stash), and one asserting the readonly loader is used instead of the mutable one.

…ion, session<->subagent, subagent<->subagent) Implements docs/design/local-agent-messaging.md (and the predecessor docs/design/cross-session-messaging.md it supersedes) end to end: two transports behind one shared model-facing tool surface, following the "define a small transport-selection interface" compatibility requirement so a real A2A HTTP transport could later be added as a drop-in without changing the tool schema the model sees. Transport A (in-process): a top-level session <-> its own delegate_task subagent tree, and a background=true subagent talking back to its parent. Reuses AIAgent.steer()/_pending_input for delivery -- no new injection mechanism. send_agent_message(recipient, body) is parent/session-only; send_to_parent(body) is subagent-only and only granted under background=true, since a synchronous child's parent is blocked inside the delegate_task polling loop and cannot act on anything until the batch returns. Transport B (cross-process): two new state.db tables (cross_session_registry, cross_session_inbox) so independently-started top-level sessions (two terminal tabs, a CLI and a gateway session, two profiles) can discover and message each other. Delivery is polling, not push -- tail-appended onto an active session's next tool-batch boundary, or injected as a fresh turn via the same _drain_process_notifications idle-poll hook already used for background-process/async-delegation completions. Inbound policy (accept/hold/refuse) is enforced at DRAIN time by the recipient against its own current config, never trusted from the sender. hop_count throttling, a per-sender-pair rate cap, and a hard one-send-per-turn ceiling are all v1-required, not follow-ups. Both transports share one invariant, enforced at a single choke point each: every delivered message is wrapped via build_agent_message_marker() before it reaches a recipient's transcript, on both the active and idle delivery paths. A bare, unwrapped body is indistinguishable from operator-authored input and would defeat the untrusted-content framing the whole design depends on -- this was the #1 finding of the design's final sign-off review and is covered directly by tests on every delivery path in both transports. send_agent_message is withheld entirely from gateway-origin sessions (gated at tool registration, not just at the recipient's inbound policy) to close the path where an untrusted external chat user could otherwise drive a gateway session to inject an instruction into a more permissive CLI session. New hermes_cli/subcommands/agents.py + hermes_cli/agents_inbox.py: `hermes agents inbox` lists/approves/denies held cross-session messages, following approvals.py's parser/handler-injection pattern but kept in its own file since the two are unrelated domains (dangerous-command mining vs. cross-agent message approval). New cross_session.inbound config.yaml key (accept/hold/refuse, per-origin defaults: hold for CLI/ACP, refuse for gateway/cron). Two pre-existing, feature-independent gaps fixed as part of building this (both real bugs the design's own verification passes surfaced): delegate_task's child lifecycle never read result["pending_steer"], silently dropping a message that arrived in the end-of-turn race window; and _push_completion_event's explicit field allowlist didn't include it either, so the fix wouldn't have reached a background=true child's actual completion-delivery path even once added to the synchronous side. Both transports were implemented independently against a shared contract module (tools/agent_messaging_contract.py) in isolated git worktrees, then merged. The merge surfaced one real integration gap neither transport's own test suite could catch on its own (they don't import each other): the tool layer only ever dispatched to Transport A's send path, so send_agent_message to a cross-process recipient was a dead end despite list_agents() advertising it as reachable. Fixed by extending the contract so each transport registers a send callable alongside its lookup callable (TransportResolution.send), rather than a hardcoded per-transport branch in the tool file -- this is also the correct shape per the design's own A2A-compatibility requirement. Independently reviewed against the actual implemented diff (not just the fix plan) before landing. Also caught and fixed, via a full tests/run_agent/ sweep diffed against a clean baseline: an unconditional call added to the turn-boundary activity hook crashed on the duck-typed test doubles several existing tests use in place of a real AIAgent instance. Verified via the repo's own full parallel test suite (24,609 tests) run against this branch and diffed file-by-file against a clean origin/main baseline: 3 files failed only on this branch, and each was individually re-run in isolation against clean main and confirmed pre-existing/ environmental (a real disk-usage-dependent health check, a timing- sensitive retry-backoff assertion, and a subprocess-timeout race that only surfaced under concurrent full-suite CPU contention) -- none are caused by this change. Design docs: docs/design/local-agent-messaging.md (2 Fable review rounds, 2 verification passes, a final sign-off pass) and its predecessor docs/design/cross-session-messaging.md (3 review rounds). Fork-only, per both docs' stated scope -- gated behind an opt-in cross_session toolset, costs zero tokens for anyone who doesn't enable it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…rst run The first-run provider picker showed Fireworks AI alongside Nous Portal before the user opened the 'Other providers' disclosure. Only Nous Portal should be visible up front; Fireworks now lives inside the expanded list but keeps its #1 position there (Nous -> Fireworks ordering preserved).

Addresses both review findings on the remote-gateway download PR: 1. Unbounded buffering (finding #1). fetchBuffer / fetchBufferViaOauthSession accumulated the entire response (then copied it again via Buffer.concat) before saveGatewayFile even opened the save dialog, so a large gateway file could exhaust the native process. Both auth paths now stream: once response headers arrive the connect timeout is cleared, the filename is derived, the save dialog is shown, and the body is piped to the chosen destination with backpressure. A read/write error tears down the stream and unlinks the partial file. The byte-moving, data-URL decoding, and filename/path helpers are extracted into gateway-file-download.ts so they're unit-testable without Electron. 2. No fallback for older gateways (finding #2). saveGatewayFile required the new /api/fs/download route. Desktop and the remote gateway update independently, so a gateway predating this PR 404s. Added a 404-only compatibility fallback to the existing capped /api/fs/read-data-url route (bounded, so it only serves smaller files — enough to keep older backends working). Tests: gateway-file-download.test.ts covers streaming, backpressure, error-cleanup (unlink on write/response error), data-URL decoding, filename derivation (incl. traversal reduction), and 404 detection; gateway-file-download-transport.test.ts asserts both transports stream (no whole-body Buffer.concat) and that the 404 fallback is wired. Both registered in the desktop platform test list. Server-side /api/fs/download tests (streaming + sensitive-file reject) already pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…-renders (NousResearch#81726) The scoped find walker wraps transcript text nodes in <mark> elements that React does not own. Assistant responses stream through markdown-text.tsx, which rebuilds the markdown DOM on every delta, and a new message is appended whenever the assistant answers — so a re-render of a changed region detaches the marks we inserted, dropping the user's highlights while the bar stays open. Watch the captured scope with a MutationObserver and re-wrap only when an unmarked occurrence of the active query actually reappears. The observer is gated behind a re-entrancy flag while the walker is mutating, coalesced to one re-apply per microtask, torn down when the bar closes or the query clears, and restores the active ordinal so a mid-stream re-render doesn't reset the user's place to match #1. An append that adds no matching text is a no-op; re-wrapping only fires when highlights genuinely went stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Two independent bugs let a deleted profile reappear / leave orphaned resources on next launch: 1. hermes_cli/profiles.py's backend-process scanner required argv[0] to resolve to an executable literally named "hermes". Electron's pool-backend spawn resolves the hermes console-script shim's path and execs it via the interpreter directly (python3 /path/to/hermes ...), so argv[0] reports as "python3" and the scanner never matched the running backend -- delete removed the profile's files but left its live backend process running (still bound to a port via uvicorn), which accumulates across repeated delete/recreate cycles. 2. The desktop sidebar's ProfileRail only refreshed its cached profile list once, on mount, so a delete/create/rename from another surface (another window, or the CLI) left a stale ghost entry until something unrelated triggered a refetch. Note: a delete via this window's own Manage-Profiles view already refreshes the shared $profiles atom ProfileRail subscribes to (confirmed by reading refreshProfiles() and handleConfirmDelete()) -- this fix only covers the cross-window/cross- process staleness gap, not a duplicate of the already-merged NousResearch#57329's Manage-Profiles rail-refresh work. Fix 1: recognize a python-interpreter argv[0] exec'ing a hermes-named console-script shim via argv[1]. Fix 2: refresh the profile list on window focus/visibilitychange, matching the existing pattern used elsewhere in the sidebar (sidebar/index.tsx, use-background-sync.ts, star-map.tsx, use-gateway-boot.ts all use the same focus+visibilitychange pattern). ## Related work already on main PR NousResearch#57329 (merged) fixed the *headline* symptom from issue NousResearch#52279 (deleted profile respawns) via a different, non-overlapping mechanism: routing profile-delete through the primary backend instead of spawning a fresh pool backend, plus a separate recreation guard in ensure_hermes_home() (NousResearch#49435, merged) that makes a backend spawned into a deleted profile's directory raise FileNotFoundError instead of silently recreating it. This PR is NOT a duplicate of that fix. Verified: even with both of those merged, a backend process that survives because of gap #1 above still holds a bound port via uvicorn -- it just can no longer resurrect the profile directory. That's real resource-hygiene, not a symptom already covered. Gap #2 touches a different file/component (ProfileRail / profile-switcher.tsx) than NousResearch#57329's rail-refresh half (which touched the Manage-Profiles view's own $profiles.ts / index.tsx) and covers a distinct staleness path (cross-window/cross-process, not same-window delete-then-refresh). Tests: tests/hermes_cli/test_profiles.py -- 156 passed (existing + regression coverage for the argv[0] python-interpreter detection case). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

adurham closed this May 13, 2026

Navigation Menu

fix(anthropic): symmetric orphan audit for web_search_tool_result - #1

fix(anthropic): symmetric orphan audit for web_search_tool_result#1
adurham wants to merge 1 commit into
mainfrom
adam/orphan-web-search-tool-result-fix

adurham commented May 13, 2026

Uh oh!

adurham commented May 13, 2026

Uh oh!

github-actions Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

adurham commented May 13, 2026

Summary

Repro / how this was found

Test plan

Notes

Uh oh!

adurham commented May 13, 2026

Uh oh!

github-actions Bot commented May 13, 2026

🔎 Lint report: adam/orphan-web-search-tool-result-fix vs origin/main

ruff

ty (type checker)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

🔎 Lint report: `adam/orphan-web-search-tool-result-fix` vs `origin/main`