test(models): make anthropic-messages warning test hermetic - #4
Merged
Merged
Conversation
The test asserted the fallback warning contains the provider's friendly label 'Claude API Proxy', but that label only resolves when the claude-api-proxy provider plugin is registered at runtime from $HERMES_HOME/plugins/model-providers/. It is not a built-in canonical provider, so on a clean CI checkout provider_label() falls through to the raw 'claude-api-proxy' slug and the assertion fails — an environment- dependent flake that only passed on machines with the user plugin present. Seed _PROVIDER_LABELS via monkeypatch so the message-building path is exercised deterministically regardless of which providers are registered.
🔎 Lint report:
|
Kyzcreig
added a commit
that referenced
this pull request
Jun 5, 2026
The test asserted the fallback warning contains the provider's friendly label 'Claude API Proxy', but that label only resolves when the claude-api-proxy provider plugin is registered at runtime from $HERMES_HOME/plugins/model-providers/. It is not a built-in canonical provider, so on a clean CI checkout provider_label() falls through to the raw 'claude-api-proxy' slug and the assertion fails — an environment- dependent flake that only passed on machines with the user plugin present. Seed _PROVIDER_LABELS via monkeypatch so the message-building path is exercised deterministically regardless of which providers are registered. Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
Kyzcreig
pushed a commit
that referenced
this pull request
Aug 11, 2026
…on delegation callbacks (NousResearch#82592) * fix(gateway): stop frozen-preview finals and dropped idle-session delegation callbacks Two relay-plane delivery losses from the 2026-08-09 staging incident: 1. stream_consumer: the skip-redundant-finalize branch recorded _accumulated as the delivered turn-final payload even when the last ACKED edit was an earlier throttled preview snapshot, so delivered_final_matches reconciled True and the gateway suppressed the corrective final send — the user was left with a cut-off message ending in the streaming cursor. Extracted _mark_skip_redundant_finalize(): records the last acked wire payload (cursor-stripped), so a preview/final mismatch now returns False and the normal final send fires. 2. run.py: _classify_completion_target classified every ended parent session terminal unless it ended by compression. Idle/timeout session ends are the norm on scale-to-zero relay deployments and the chat route remains valid; completed async delegation results were terminally dropped. Ended parents now classify deliver unless the end was an explicit user boundary (session_reset / user_exit / session_switch). * fix(relay): drain in-flight outbound frames before transport teardown disconnect() failed every pending outbound future immediately with 'relay transport closed', so a trailing finalize edit racing turn teardown was lost even though the connector socket could still serve it. Bounded drain grace (5s) lets in-flight requests resolve; silent connectors still tear down promptly. asyncio.wait (not gather+wait_for) so a timeout doesn't cancel futures owned by the fail-remaining loop. * fix(gateway): route completion injection through the alias-aware transport resolver Third relay-plane delivery loss from the 2026-08-09 staging incidents: a delegation batch completed while the gateway was up, the watcher drained the event, and delivery vanished with no log line. _inject_watch_notification resolved its adapter with a literal p.value == platform_name scan of self.adapters — a relay-fronted gateway registers ONE adapter under Platform.RELAY fronting N logical platforms, so 'slack' never matched and the injection returned None ('no gateway route'), silently dropping the completion. The handoff path already documents this exact trap and uses resolve_delivery_transport; the injection path now does the same (native wins; relay eligible only when it fronts the logical platform), with the literal scan kept as fallback for stub runners and exotic platforms. * fix(relay): clamp disconnect drain grace to the runner's adapter-disconnect budget Review finding (JoaoMarcos44, NousResearch#82592): a fixed 5.0s drain in front of the three 1.0s sequential teardown awaits gives an 8.0s worst case inside the runner's 5.0s asyncio.wait_for(adapter.disconnect()) — tripping it cancels teardown mid-drain, skips the fail-pending loop, and leaves outbound callers blocked until _OUTBOUND_TIMEOUT_S (30s). The effective grace is now budget - 3*TEARDOWN - margin (env-aware via the same HERMES_GATEWAY_ADAPTER_DISCONNECT_TIMEOUT the runner reads), so the drain can never push teardown past its caller's budget; a budget too small for any drain disables it cleanly. * test(gateway): pin the final-send suppression contract across a behaviour matrix The gateway skips its own final send when the stream consumer claims the turn final already reached the user. Every incident in that family — NousResearch#71643 (stale finalize snapshot), NousResearch#78541 (payload-less multi-message split), NousResearch#82656 (frozen preview left with a visible cursor) — is the same failure: the consumer claimed delivery for text the platform never rendered, so the corrective send was suppressed and the answer was lost with no retry. Each was fixed with a scenario test pinned to one branch of GatewayStreamConsumer.run(). The got_done handler now has five sibling branches that each set the suppression flags and record a turn-final payload, and nothing checks them as a group: a new branch, or a new early `return True` in _send_or_edit, can reintroduce the class without failing a test. Pin the invariant instead of the branch — if the consumer offers the gateway any signal it would trust, the complete final text must have reached the wire — and assert it across {edit always / dies / never / lies} x {send always / never} x {fresh-final on / off} x {clean / interrupted stream}. The adapter records only frames that actually rendered, so an ACK the platform drops does not count as delivery. 24 honest-transport scenarios hold the invariant as a hard assertion. The 16 lying-transport scenarios are checked too; the single combination that still violates it is reported as an expected failure documenting the open exposure rather than asserting it away. Refs NousResearch#82656 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gateway,relay): prime relay egress routing for synthetic injections + cap stale completion replay Defect #4 from the 2026-08-09 staging incidents (upgrade-robustness): after every gateway restart the durable async-delegation replay injected completions correctly (post-741663cf1) but their replies bounced at the connector — 'slack egress declined: target not routed to an onboarded tenant'. The relay adapter re-attaches tenant discriminators (metadata.scope_id / metadata.user_id) from per-chat caches warmed ONLY by inbound traffic; synthetic turns race those cold caches on every deploy, scale-to-zero wake, and crash recovery. - relay adapter: prime_routing_cache() — feeds a synthetic event's session-store origin through the same _capture_scope used for real inbound (never raises). - run.py injection path: prime the resolved adapter before handle_message (duck-typed; native adapters unaffected). - async_delegation: 48h staleness cap in restore_undelivered_completions — a pending completion older than the cap is terminally dropped (payload stays queryable) instead of re-run as a fresh full-context turn; the post-restart replay of a July session burned a 102K-token context. Also carried: JoaoMarcos44's suppression behaviour-matrix harness (cherry-picked from NousResearch#82676, authorship preserved) — 39 passed + 1 xfail (the documented ACK-then-drop transport-honesty residue). * test: use recent timestamps in restored-ownership fixtures test_restore_stamps_restored_flag persisted its completion with epoch-era toy timestamps (dispatched_at=1.0), which the new 48h replay staleness cap correctly classifies as stale — the fixture then exercised the cap instead of the restored-flag contract (CI slice 4 failure). Timestamps are now now-relative; the staleness behavior itself is pinned separately in test_relay_injection_egress_priming.py. * fix(gateway,relay): close four review findings on the relay delivery fixes Review follow-ups on this branch (NousResearch#82592): 1. HIGH — classifier/resolver mismatch (falsely-acknowledged loss). _classify_completion_target now returns "deliver" for idle-ended parents, but _resolve_async_delegation_session still dropped every non-compression-ended pin: the durable row was acked at adapter acceptance, then the injection died inside the pipeline with no retry — strictly worse than the honest terminal drop on main, and the delivery leg defect #2's fix depends on did not exist. The resolver now retargets non-user-boundary ends (idle/timeout/ lifecycle) to the chat's current session — session_entry already IS the routing key's current session for the same chat — while user boundaries (session_reset / new_session / user_exit / session_switch) stay fail-closed. Both sides share one module-level _USER_BOUNDARY_END_REASONS so the verdict and the routing decision cannot drift again; a coherence test asserts deliver-verdicts resolve non-None across representative end reasons. 2. HIGH — drain clamp missed adapter-level spend. The effective drain grace budgeted drain + 3x teardown, but RelayAdapter.disconnect spends revocation-monitor teardown + go_idle time BEFORE the transport drain inside the same runner wait_for; worst case still blew the budget and cancelled teardown mid-drain (skipping the fail-pending loop). The adapter now measures its own elapsed time and threads the REMAINING budget into transport.disconnect(budget_s=...); legacy/stub transports without the keyword fall back to the no-arg signature. 3. P1 — _request_response racing disconnect() could register a future after the fail-pending loop already ran, stranding the caller for the full _OUTBOUND_TIMEOUT_S (30s). Fail fast with the same "relay transport closed" error once _closing is set. 4. P1 — _build_process_event_source's last-resort reconstruction dropped scope_id, so a scoped relay completion whose session-store origin was unavailable primed no tenant discriminator and could still bounce off the connector's fail-closed egress guard. scope_id now threads through the reconstructed SessionSource, with a warning when a scoped chat reconstructs without one. All four: RED reproduced with the fix reverted, GREEN after; relay/ delegation delivery families pass (43 + 71 + 179 across the touched suites); full tests/gateway run shows only failures already failing identically on merge base 2446c8b (env/dep issues). * fix(gateway,relay): make pending-frame failure cancellation-safe; persist completion routing origin Two remaining review findings on this branch (NousResearch#82592): 1. Cancellation could strand outbound waiters past the fail-pending loop. transport.disconnect() failed pending futures only at the END of the drain + three teardown awaits; a cancellation landing mid-drain (the runner's wait_for budget, an outer cleanup deadline) skipped the loop entirely and left registered futures unresolved — their callers blocked until _OUTBOUND_TIMEOUT_S (30s). The budget threading added earlier shrinks the window but is not a hard guarantee. The fail-pending loop (and the going_idle ack failure) now run in a `finally`, so no exit path — normal, error, or cancelled — can leave a registered future unresolved. Idempotent: done futures are skipped, a second disconnect() pass is a no-op. 2. Durable completions did not persist their routing origin, so the scope_id threading in the fallback SessionSource reconstruction had nothing to carry on the exact path it exists for (restart replay with session store + source cache gone): the async-delegation event producers never populated scope_id and the durable rows never stored it. Dispatch now snapshots the originating turn's scope_id/user_id/user_name from the session context (_capture_routing_origin — a new HERMES_SESSION_SCOPE_ID contextvar bound by the gateway at session-bind time alongside the existing vars), stores them in the existing task_json payload (no schema migration), and re-attaches them to all three completion-event shapes (live single, live batch, crash-recovery rebuild). The gateway's fallback reconstruction then primes both discriminators after a restart. Tests: cancellation mid-drain -> every pending future resolves with "relay transport closed" (mutation: moving the loop out of the finally goes RED); second-pass disconnect idempotence; end-to-end dispatch -> owner-death recovery -> event carries scope_id -> fallback SessionSource primes it (mutations: dropping the dispatch capture or the task_json persistence both go RED); live completion event carries the origin. 94 passed + 1 xfailed across the delivery/delegation suites; tests/tools delegation family 73 passed (2 collection errors pre-existing on merge base 2446c8b). --------- Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Ben Barclay <ben@nousresearch.com>
Kyzcreig
pushed a commit
that referenced
this pull request
Sep 19, 2026
…as a died link The exit-notify wrap reported every receive-loop exception at ERROR with a traceback, including the ConnectionClosedOK that follows our own CLOSE frame in disconnect(). Gate on the adapter's _running flag (published through the WS thread-local next to on_link_up): a live link's death stays ERROR, an intentional shutdown logs at DEBUG. Live pass side-effect #4 on NousResearch#113662.
This was referenced Sep 20, 2026
Merged
Kyzcreig
added a commit
that referenced
this pull request
Sep 21, 2026
…essages
Review round 1 (Argus) flagged that the user-facing block message still
advertised a state the previous commit deleted:
"pre_tool_call plugin callback timed out or is still running"
The still-running branch is gone, so "or is still running" named a condition
that can no longer occur. That exact wording is what sent this investigation
down a phantom concurrency-snowball hypothesis, and it is the observability
half of acceptance criterion #4 — the WARNING was fixed to report measured
elapsed + budget, but the string the user receives was not.
Simply dropping the clause would have been wrong. There are TWO call sites of
_pre_tool_call_timeout_block(), not one:
:5736 genuine timeout — done.wait expired; elapsed and budget known
:5683 suppression window — a LATER call refused because a PRIOR one
timed out inside the 60s cooldown
Deleting "or is still running" fixes the first and makes the second lie in the
opposite direction: it would claim THIS callback timed out when it did not.
The log lines were already de-conflated for exactly this reason; the
user-facing message now gets the same split, and both name the callback.
timeout: pre_tool_call plugin callback <name> timed out after 0.101s
(budget 0.1s)
suppression: pre_tool_call plugin callback <name> is suppressed after an
earlier timeout (retry in 60s)
Tests now assert the behavioral distinction (each path accurate, and the two
differ) instead of comparing against a shared constant — the constant-equality
shape is what let the conflation survive round 1 unnoticed.
Verified (all probes import-asserted against the worktree):
- scripts/run_tests.sh tests/hermes_cli/test_plugins.py
tests/agent/test_shell_hooks.py -q -> 128 passed, 0 failed
- mutation: re-conflate the suppression path -> FAILED on
"assert 'hung_policy' in msg2"; restored -> 128 passed
- fail-closed policy unchanged: advisory -> allow on timeout AND suppression;
enforcing -> block on both
- thread accumulation re-verified on real PR code: 40 invocations against a
hung hook -> spawned=1, thread delta=1
- ruff check -> All checks passed
findings.md also records the measurement trap that invalidated two round-1
probes: the shared venv's editable-install finder maps top-level packages to
the LIVE checkout, so bare `python probe.py` from a worktree executes live-tree
code while __file__ and inspect.getsource report the worktree path. pytest is
unaffected (rootdir wins on sys.path). Re-counted skips span 2026-08-31 to
09-21 (2,055), so the defect predates the incident by three weeks.
This was referenced Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fleet-local landing of the hermetic test fix (also submitted upstream as NousResearch#37784). Fixes the environment-dependent flake in
test_anthropic_messages_warning_clarity— seeds_PROVIDER_LABELSvia monkeypatch so the test no longer depends on the runtime claude-api-proxy user plugin. Verified passing under clean HERMES_HOME.