chore: sync fork with current Hermes upstream - #2
Merged
Conversation
_voice_input_callback was only set in _handle_voice_channel_join, not at adapter connect or reconnect. Voice transcription was logged but never forwarded as an inbound message without explicit /voice join. Wire the callback at both connect and reconnect paths. Fixes NousResearch#60623
…t tests PR NousResearch#61407 accesses self._handle_voice_channel_input in _platform_reconnect_watcher. The test mock runner created via _make_runner() must have this attribute.
- Wire adapter._voice_input_callback at connect and reconnect so voice transcription is forwarded without requiring /voice join (NousResearch#60623). - Add optional text_channel_id and source params to DiscordAdapter .join_voice_channel() so automatic/programmatic voice joins can establish the text-channel binding needed by _handle_voice_channel_input. - Add TestVoiceInputCallbackWiring: asserts callback wiring on startup and reconnect for Discord adapters with voice attributes.
…bility VoiceMixer duck-typed the discord.AudioSource interface (is_opus, read, cleanup) but never inherited from it. discord.py's vc.play() does an isinstance check and rejects non-AudioSource objects, causing the voice fx mixer to silently fail with: "Voice mixer failed to start: source must be an AudioSource not VoiceMixer" Added missing `import discord` and changed the class definition from `class VoiceMixer:` to `class VoiceMixer(discord.AudioSource):`.
Discord's voice socket needs a brief warm-up before receiving clients actually hear audio; the first ~100-200ms is lost, clipping the first word/syllable of TTS playback. Prepend a configurable lead of silence to speech on both playback paths: - Mixer path: new _lead_silence_bytes() helper prepends PCM silence (BYTES_PER_MS constant added to voice_mixer.py) before play_speech, on both the reply and the pre-tool ack. - Legacy FFmpegPCMAudio path: apply -af adelay=<ms>:all=1. Tunable via discord.voice_fx.lead_silence_ms (default 200, 0 disables). Fixes NousResearch#66827
…ption_tools helper Keep one owner for PATH/local-prefix ffmpeg discovery: ffmpeg_utils now delegates to tools.transcription_tools._find_ffmpeg_binary and only adds the Discord-specific FFMPEG_PATH override and Windows winget fallback on top (follow-up to PR NousResearch#60627 by @LauraGPT, fixes NousResearch#60624).
…d empty transcripts Two hand-written fixes in the voice input path: - _handle_voice_channel_input now resolves the bound text channel's channel_prompt via the adapter's _resolve_channel_prompt so voice input gets the same per-channel context as typed messages (fixes NousResearch#50149). - _enrich_message_with_transcription now guards success=True results whose transcript is empty/whitespace-only (silence, cut-off, inaudible audio): instead of emitting empty quotes the agent gets a clear sentinel note. Reimplemented against the current plain-quoted note wording; original concept and tests by @deacon-botdoctor in PR NousResearch#41603 (fixes NousResearch#41603).
… sent as documents [[audio_as_voice]] is message-global but was applied to every media file in a message. A non-audio file flagged is_voice is excluded from the embedded-photo batch and falls through to send_document, so an image in a message that also carries a voice note arrives as a file attachment instead of an inline photo. Gate the voice flag on the file extension so one message can carry an inline image AND a voice bubble. Also wrap the path append in try/except so a crafted ~\x00 path is skipped rather than aborting extraction of all attachments.
_invalidate_pending_stt_cache() clears the gateway-side transcription
cache when merge_pending_message_event() folds a follow-up message into a
still-pending event, so the next transcription picks up the merged text
and attachments. It also cleared _gateway_pending_stt_echo_sent, but that
flag is not derived state — it records that the transcript was already
delivered to the user.
Dropping it makes the re-run transcription echo the earlier notes a second
time. Both merge branches are affected, including the text-only follow-up
case where no new audio arrived at all: there the cache is invalidated,
the same voice note is transcribed again (a second paid STT call) and the
same line is echoed again.
Sequence:
1. voice note arrives, interrupt monitor transcribes it and echoes
'🎙️ "hello"'
2. user sends a follow-up while the turn is still pending, so it merges
3. drain path re-transcribes and echoes '🎙️ "hello"' a second time
Keep the ledger out of the invalidation set and track it as a count of
already-echoed transcripts instead of a single boolean. A count is what
the merge case actually needs: re-running transcription over the extended
media list returns the earlier transcripts as a prefix of the new one, so
echoing only the unsent tail suppresses the repeat while still surfacing a
newly merged voice note. A count rather than a set of seen values, so two
separate notes that transcribe identically stay two distinct deliveries —
covered by test_pending_stt_merge_echoes_two_identical_transcripts.
The guard stays within the 12-line window that
test_all_gateway_transcript_echo_sends_are_gated enforces over run.py.
…point Follow-up for salvaged NousResearch#65023/NousResearch#53020: _prepare_busy_steer_text now calls _transcribe_and_echo_pending_voice (the same helper the interrupt monitor and pending-drain paths use) instead of a private transcription+echo copy, so out-of-band voice pays one STT call per platform message and the echo respects the count-based ledger from NousResearch#67281. can_steer now accepts events whose attachments are all STT-eligible voice media, completing the steer half of NousResearch#58780. Adds extract_media gating tests for NousResearch#44826 and the contributor mapping for chefboyrdave21.
…ribe (NousResearch#67545) In continuous voice mode, pressing the record hotkey (Ctrl+B) was a silent no-op while the agent was running or voice was being transcribed. The hotkey only cleared _voice_continuous when _voice_recording was True, leaving the user trapped in an auto-restart loop that only /voice off could break. Fix: when the agent is running or transcribing, still allow the hotkey to clear _voice_continuous so the loop stops after the current turn.
…o-speech cycles When voice continuous mode detects 3 consecutive no-speech cycles it sets _voice_continuous = False and previously did an immediate eturn. The restart guard if self._voice_continuous and not submitted and not self._voice_recording came *after* the early return, so the thread could never restart. However a timing window existed: if _voice_start_recording was already queued from a prior iteration, the eturn bypassed the guard while the thread was already in flight. Replace the bare eturn with a stop_continuous_restart boolean evaluated in the same if that guards _restart_recording. This ensures both branches read a consistent no-restart signal and no recording thread is spawned after the user's session has been intentionally halted.
…havior coverage Review follow-up: the sweeper is right that the first commit only cured half the dead config — the TUI gateway builds its recorder params explicitly, so the cap never reached recordings started from the TUI. - start_continuous() grows a max_recording_seconds param (default 0.0 = disabled, so existing callers keep today's behaviour) and applies it to the shared recorder next to the silence params - tui_gateway voice.record start forwards the validated cap; corruption semantics now mirror the silence params everywhere: non-numeric/bool falls back to the documented 120 default (a hand-edited `max_recording_seconds: true` must not become a 1-second cap), while an explicit numeric <= 0 disables the cap - cli.py wiring updated to the same corruption semantics New coverage, per review: - TestMaxRecordingCap drives the mocked InputStream callback past the cap during continuous loud speech (the silence branch physically can't fire there) and asserts the one-shot callback fires exactly once; plus the disabled-cap negative - TestMaxRecordingSecondsConfigReal pins the CLI config assignment for the valid / zero / bool / garbage cases via _voice_start_recording - test_voice_record_start_forwards_max_recording_seconds pins the TUI forwarding for the same matrix
Closes NousResearch#55908. The CLI voice-mode beep amplitude is hardcoded at 0.3 inside tools.voice_mode:play_beep(), which makes the record start/stop cues too quiet on low-volume systems and headphones. Users couldn't adjust it without editing source. Move the literal into a configurable voice.beep_volume setting (clamped to 0.0-1.0, default 0.3 to preserve prior behaviour). The new _get_beep_volume() helper reads via the same load_config() pattern used by cli.py's _voice_beeps_enabled() and hermes_cli/voice.py's _beeps_enabled(), keeps bools / out-of-range / non-numeric / NaN values safely on the default, and falls back silently if config can't load so the audio cue never breaks the voice loop on a degenerate config.yaml. Covered by tests/tools/test_voice_mode.py: - TestGetBeepVolume (12 cases: missing key, custom value, boundary 0.0/1.0, out-of-range clamp, type coercion, bool guard, NaN guard, exception guard, dict-typed voice section) - TestPlayBeepVolumeWiring (guards against re-introducing a hardcoded 0.3 literal in play_beep) Docs: website/docs/user-guide/configuration.md mentions the new key. Other locale translations (zh-Hans etc.) intentionally untouched — handled by the regular i18n sync pipeline as a separate change. No change in default behaviour: existing users hear exactly the same beep.
AudioRecorder hard-codes SAMPLE_RATE (16 kHz) when opening the input stream, but some capture devices (e.g. USB microphones exposed through ALSA hw) reject 16 kHz outright — sd.InputStream fails with PaErrorCode -9997 (Invalid sample rate) and voice recording is broken. Query the default input device for its native default_samplerate at recording start and open the stream / write the WAV at that rate, falling back to the Whisper-friendly 16 kHz constant when the backend does not expose a usable rate. STT providers accept standard WAV rates, so downstream transcription is unaffected.
WSLg RDP audio has two issues causing crackling: 1. systemd-timesyncd clock adjustments jitter PulseAudio timing (microsoft/wslg#1257) — user action: stop the service 2. Cold-start RDP connection drops first ~100ms of audio packets before the virtual channel stabilises Fix (automated, WSL-only): - Detect WSL via /proc/version 'microsoft' marker - Prepend 100ms silence + apply 100ms fade-in to audio - Append 50ms silence tail for clean stream teardown - Set blocksize=4096 (default auto ~1024 is too small for RDP) - All in a single continuous sd.play() buffer Non-WSL paths unchanged. Closes NousResearch#38893
… turn-timeout Two follow-ups from the voice PR (NousResearch#70509). 1. macOS ARM64 onnx migration. Existing users who pinned openwakeword.inference_framework=onnx before the tflite fix landed kept a wake word that arms but never fires (ONNX's embedding model is broken on Apple Silicon, upstream NousResearch#336). New resolve_inference_framework() honors an explicit framework everywhere ONNX actually works, but coerces the one provably-dead combination (explicit onnx + macOS ARM64) to tflite with a one-time warning. No config mutation; empty still falls back to the platform default. Both read sites (engine init + requirements check) route through the shared resolver. 2. Voice turn-timeout leak. Each listen cycle reassigned turnTimeoutRef without clearing the prior 60s timer, so a stale timer from an earlier cycle could fire handleTurn() mid-way through a later listen — after enough idle re-listens this wedged the loop into a non-re-arming state (the 'voice chat deactivates after ~a minute' report). Clear before re-arm. Tests: 64 wake tests (added onnx-coercion / intel-kept / tflite-kept / empty-default cases; updated the stale 'explicit onnx kept on ARM64' test that encoded the old broken behavior), 39 desktop voice/wake vitest, tsc + eslint clean.
…decar-deps fixture Two independent cross-PR collisions red on main (slice 8/8): 1. fd4f756 (salvaged from stale NousResearch#54514) added an early 'drop U+FFFC placeholder' return at the top of _dispatch_inbound — written before the deferred-wait handler (6b91b50/afab7ed46e) existed further down the same function. The early return shadowed it: _pending_fffc never populated, no attachment-timeout tracking, 4 tests red. Remove the duplicate block; the deferred handler already drops the placeholder AND tracks/cancels/warns. 2. 9cf2046 tightened sidecar_deps_installed() to require node_modules/spectrum-ts, but test_runtime_record's _patch_spawn fixture still created only bare node_modules/ — 2 tests red. Mirror a real completed install. tests/plugins/platforms/photon: 164/164 after; 158/164 before.
Extracted verbatim from PR NousResearch#73636 by @ScaleLeanChris — nsec/hex key decoding, secp256k1 point math, BIP-340 Schnorr signing, and NIP-42 AUTH event construction with optional NIP-OA owner attestation tag.
…ery, poll fallback Consolidates the native-transport half of PR NousResearch#73636 by @ScaleLeanChris onto the merged adapter: persistent NIP-42-authenticated Nostr WebSocket subscription as the default inbound path (transport=auto|websocket|poll), kind-44100 membership events for live DM discovery, since-timestamp resume on reconnect with bounded exponential backoff, and automatic fallback to CLI polling when the WS can't be established. Events route through the same _handle_event() pipeline as the poll loop, so de-dupe, mention gating, p-tag DM latching, and allow-lists behave identically on both transports. Outbound stays on the CLI (one-shot sends never race a WS auth handshake — his design). E2E verified against a real in-process websockets relay: NIP-42 challenge -> signed kind-22242 AUTH (event id re-derived server-side) -> REQ subscription -> EVENT dispatch -> clean disconnect. Co-authored-by: ScaleLeanChris <chris@scalelean.com>
Sibling of NousResearch#65254 (main-slot endpoint preservation): the auxiliary scope of POST /api/model/set dropped the request's base_url/api_key on the floor, so an aux slot pinned to a custom/local endpoint silently depended on model.base_url — and broke the moment the main slot switched away and cleared it. The aux resolver already reads auxiliary.<task>.base_url/api_key (_resolve_task_provider_model); this persists them. Desktop side: setAuxiliaryToMain / applyAuxiliaryDraft now carry the user-defined provider's api_url as base_url, mirroring applyMainModel.
…n path Sibling of NousResearch#64686: _run_agent's empty-final_response early return dropped failure_reason (and NousResearch#64686 only fixed the non-empty path), so downstream consumers (TUI billing surface, transient-failure persistence) lost the structured reason exactly when a failed run produced no text. Also hardens the two BasePlatformAdapter identity checks (edit_message / delete_message) with getattr so duck-typed adapters without the attribute mean 'capability absent', not AttributeError — this was crashing the send_progress_messages path for minimal adapters and test fakes.
…72285 The multiplexed listener now rejects the default API_SERVER_KEY on /p/<profile>/ prefixes (fail-closed per-profile keys). Add the multi-profile routing section with an explicit breaking-change callout for the next release notes.
Follow-up to NousResearch#69112. That PR hardened the shared gateway dispatch path in gateway/run.py against the *caller* being cancelled. A second, self-referential cancellation specific to PhotonAdapter sits one layer underneath it and survived that fix. PhotonAdapter is the only platform adapter that awaits _notify_fatal_error() inline, on the same task that detected the fault. Both _monitor_sidecar_health and _supervise_sidecar run as self._sidecar_health_task / self._sidecar_supervisor_task, and the notification routes into GatewayRunner._handle_adapter_fatal_error_impl, which tears the adapter down via _safe_adapter_disconnect -> disconnect(). disconnect() then cancels self._sidecar_health_task and awaits it -- which, when the health task is what raised the notification, means disconnect() cancels its own caller several plain-await frames up. disconnect()'s `task is not asyncio.current_task()` guard does not catch this. The current task where that guard evaluates is the wrapper _await_adapter_cleanup_with_timeout creates around disconnect() via asyncio.ensure_future, not the health task further up the chain, so the guard passes and the cancel lands. CancelledError stopped subclassing Exception in Python 3.8, so the `except Exception` that wrapped the inline notify call never saw it. The health task died silently mid-handoff: no log line, no "exception never retrieved" warning (cancellation is normal asyncio), and no retry. The platform stayed stranded until the gateway was restarted by hand. Fix: dispatch the notification onto a new task, the same pattern DiscordAdapter._handle_bot_task_done already uses for this reason. disconnect() can then cancel the health/supervisor task freely without that cancellation reaching the code still running the handoff, so the handoff always reaches the reconnect queue. Both Photon fatal call sites are converted: the health-poll path (observed wedging) and the sidecar-crash path (same shape, not yet observed). gateway/run.py is untouched. Observed twice on a self-hosted gateway, ~4h38m and ~52min of silent inbound outage, both cleared only by a manual restart, both post-dating NousResearch#69112's merge. In each case the fatal log line appears and `queued for background reconnection` never does. Tests: new tests/plugins/platforms/photon/test_fatal_notify_self_cancel.py covers the self-cancellation (fails with CancelledError without this change), that the dispatch does not block its caller, that a failing notification warns rather than raising, and a source guard against reintroducing either inline await. Two assertions in test_overflow_recovery.py that drove these coroutines directly now drain pending tasks before asserting delivery, since the notification is deliberately no longer awaited inline. Prepared with agent assistance and reviewed before submission. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… supervisor task Fixes NousResearch#73159. When the Photon sidecar exits unexpectedly, _supervise_sidecar() (running as self._sidecar_supervisor_task) correctly detects SIDECAR_CRASHED and calls self._notify_fatal_error(). The Gateway's fatal-error handler answers that by calling adapter.disconnect(), which calls _stop_sidecar() -- from INSIDE the very task that's currently executing this whole chain. _stop_sidecar()'s cleanup unconditionally cancelled self._sidecar_supervisor_task. Cancelling the currently-running task raises CancelledError at its own next await point (inside _notify_fatal_error() or _stop_sidecar() itself). Since asyncio.CancelledError inherits from BaseException (not Exception), the Gateway's `except Exception` guards around the fatal-error handler don't catch it -- the handler aborts before ever reaching the "queue platform for background reconnection" step. Photon then stays permanently in `retrying` state until the whole process is manually restarted, even though detection worked correctly and the underlying transient upstream outage had long since recovered. Fix: in _stop_sidecar()'s cleanup, check whether self._sidecar_supervisor_task is asyncio.current_task() before cancelling it. A task cannot legally cancel itself in any useful way anyway (the cancellation only takes effect at its own next await, which is exactly the corruption described above) -- when we're inside the supervisor's own call stack, it's already in the process of finishing on its own once _notify_fatal_error() returns, so skip the cancel and just clear the reference. This mirrors an existing precedent in the same file: disconnect() already guards its OTHER task-cancellation (self._sidecar_health_task) the same way (`if task is not asyncio.current_task()`), just not the supervisor task in _stop_sidecar(). Per the issue's own note that existing tests mock out _notify_fatal_error() entirely (so this integration chain was never exercised), added two tests that drive the REAL chain: one runs _supervise_sidecar() as an actual asyncio task with a real _notify_fatal_error() that calls the real disconnect() -> _stop_sidecar(), confirming the task completes without CancelledError and that the post-disconnect reconnect-queue step actually executes; a second confirms the OTHER call path (external cleanup, a different task) still correctly cancels a running supervisor exactly as before. Reverting only the adapter.py fix (keeping the new test) reproduces the exact CancelledError from the bug report, confirming this is a genuine regression test. 2 new tests pass; 113/113 in the full tests/plugins/platforms/photon/ directory (no regression to sidecar lifecycle, overflow recovery, or health-monitoring behavior).
…isconnect() Follow-up widening of the NousResearch#73170 pattern: apply the same asyncio.current_task() guard used for the health task (and now the supervisor task in _stop_sidecar) to the inbound-task cancel path in disconnect(), so an inbound task that triggers disconnect() cannot cancel-and-await itself.
`_reap_stale_sidecar` is `async`, but it identified the processes holding the sidecar port with two blocking helpers called inline: * `_find_listener_pids` -> `subprocess.run(["lsof", ...], timeout=5.0)` * `_pid_is_sidecar` -> `subprocess.run(["ps", ...], timeout=5.0)`, once per candidate pid so the inspection can hold the shared gateway loop for 5 + 5·N seconds while nothing else on it is serviced. It only runs once the /healthz probe finds something already listening — the orphaned-sidecar recovery path — and `_reap_stale_sidecar` is awaited from `_start_sidecar`, which runs on every reconnect (`connect(is_reconnect=True)`). The stall therefore lands on a live gateway that is still serving every other platform, right when a crashed sidecar has already left an orphan behind. Move the whole inspection to one `asyncio.to_thread` hop (one hop rather than N+1 round trips). The reaping semantics are untouched: SIGTERM for verified orphans, SIGKILL escalation, and both foreign-listener RuntimeErrors behave exactly as before. Same off-the-loop class as the inbound-image decision (NousResearch#66688) and the cron-fire verifier. Adds a regression test asserting both the lsof lookup and the per-pid ps check execute on a worker thread rather than the loop thread.
`PhotonAdapter._start_sidecar` is `async`, but it ran the Spectrum
mixed-attachment patch script with a bare `subprocess.run(...)`: it spawns
node and *waits* for it, with `timeout=10`. Executed inline that holds the
shared gateway event loop for the whole window, so no other platform's
messages, heartbeats, or sessions are serviced until it returns.
The same function already establishes this exact invariant twenty lines
above, where the stale-dependency reinstall hops to a worker thread:
# Runs off the event loop so a cold install can't freeze every other
# platform's traffic.
if _sidecar_deps_stale():
await asyncio.to_thread(_reinstall_sidecar_deps)
The patch spawn never got the same treatment. It is not startup-only
either — `_start_sidecar` is called from `connect()`, which takes
`is_reconnect`, so an ordinary Photon reconnect (network blip, sidecar
death) re-runs it and stalls a live gateway that is actively serving
Discord/Telegram/Slack traffic.
Dispatch it via `asyncio.to_thread` like its sibling. Same off-the-loop
class as the inbound-image decision (NousResearch#66688) and the cron-fire verifier.
Adds a regression test asserting the spawn executes on a worker thread
rather than the loop thread.
Photon's Node sidecar intentionally hides raw handler exceptions, but the Python adapter still needs a safe failure class and retryability bit so delivery retries do not collapse into an opaque generic 500. Constraint: Sidecar responses must not leak raw stack traces or private exception text Rejected: Retry every internal sidecar error | masks permanent auth/config failures Confidence: high Scope-risk: narrow Directive: Keep sidecar error text generic; extend safe error classes instead of exposing raw SDK failures Tested: uv run --with pytest-timeout pytest tests/plugins/platforms/photon/test_overflow_recovery.py -q Tested: uv run --with pytest-timeout pytest tests/plugins/platforms/photon -q Tested: uv run ruff check plugins/platforms/photon/adapter.py tests/plugins/platforms/photon/test_overflow_recovery.py Tested: python3 -m py_compile plugins/platforms/photon/adapter.py tests/plugins/platforms/photon/test_overflow_recovery.py Tested: node --check plugins/platforms/photon/sidecar/index.mjs Tested: git diff --check Tested: python3 scripts/check-windows-footguns.py --diff origin/main Not-tested: Live Photon/Spectrum delivery against a real iMessage account Related: NousResearch#50971
…owed Maintainer follow-up to the NousResearch#51193 salvage: - _send_with_retry: permanent classes (auth_or_config, target_not_allowed) now short-circuit BEFORE the unconditional plain-text fallback resend, including when a retry attempt surfaces one — no more double-sends of permanently-failing requests. - sidecar classifySidecarError: new structured code target_not_allowed for Spectrum's 'Target not allowed for this project' AuthenticationError (shared/free-tier lines cannot initiate outbound sends to new targets). Classification applies to every handler sharing the catch-all serverError path (/send, /send-attachment, /react, /typing, ...). - _standalone_send now parses the structured error body too (it reads sidecar responses independently of _sidecar_call) and returns error_class/retryable alongside the message. - target_not_allowed maps to a canonical user-facing message in both paths; raw upstream error text never leaks through the structured code. Closes the actionable halves of NousResearch#50971, NousResearch#51897, NousResearch#52794.
Dannyxlm
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>
Dannyxlm
pushed a commit
that referenced
this pull request
Aug 15, 2026
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>
Dannyxlm
pushed a commit
that referenced
this pull request
Aug 18, 2026
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>
Dannyxlm
pushed a commit
that referenced
this pull request
Aug 21, 2026
… the relay (gateway half) (NousResearch#85796) * feat(relay): live-card ops — native draft streaming + task cards over the relay (gateway half) NS-658. Three additive ops within contract v1, emitted only when the connector's negotiated descriptor advertises them: {op: draft, chat_id, draft_id, content, final, metadata} {op: task_card, chat_id, card_id, chunks, metadata} {op: task_card_stop, chat_id, card_id, metadata} The gateway side is deliberately dumb: no platform API knowledge, no new config keys. Slack mechanics (chat.startStream/appendStream/stopStream, per-workspace feature-gate cache, send+edit fallback) live connector-side where the platform adapter lives in the relay model. Semantic bridge: base send_draft is Telegram-shaped (draft clears; final is a separate send). Slack native streaming makes the stream THE message. The adapter tracks the open draft per chat and converts the turn-final send() into draft(final=true) so the connector seals the stream instead of posting a duplicate; the stream ts returns as the message identity. A failed frame disarms interception so the edit-based fallback's real send goes through untouched. BEHAVIOR CHANGE (deliberate): relay supports_draft_streaming() now requires the descriptor flag AND the draft op. Flag-only was a latent lie — send_draft inherited NotImplementedError, so a connector setting the flag without the op would have crashed the stream consumer's draft path. supported_ops stays fail-open for legacy (pre-contract) ops; draft/task_card did not exist pre-contract and must not fail open. Task cards ride NousResearch#85476's adapter-agnostic TurnRunner seam (hasattr on send_native_task_card_progress); supports_native_task_cards() is the descriptor probe. Connector half + E2E harness pair follow in the gg repo. * fix(relay): expose native_task_cards_enabled() on the relay adapter Live-canary finding (Alice, staging): the TurnRunner's task-card lane probes adapter.native_task_cards_enabled() (the native Slack adapter's opt-in contract). The relay adapter only offered supports_native_task_cards(), so the hasattr gate failed silently and tool progress stayed on the text path — draft streaming worked, cards never rendered. Alias it to the descriptor probe. * fix(relay): match task-card methods to the TurnRunner's native keyword contract Live-canary finding #2 (Alice, staging): gateway/run.py's card lane calls send/stop_native_task_card_progress with the NATIVE Slack adapter's signature (tasks/title/reply_to/metadata/fallback_text, keyword-only) — PR 85796's relay methods took a positional card_id, so every call raised TypeError('unexpected keyword argument reply_to') in the progress task, repeatedly killing the card publisher (and the retry loop resent the final delivery 4-5x). Card id now derives per turn thread (turn:<reply_to>), thread_ts anchored like draft; title/fallback_text accepted for parity, not forwarded (plan-mode stream renders chunks). * fix(relay): one draft stream per turn for stream-is-the-message adapters Live-canary finding #4 (Alice, staging): the stream consumer bumps draft_id at every tool boundary so Telegram-shaped drafts animate each text segment as a fresh preview. On relay Slack NATIVE streaming a new draft_id opens a brand-new chat.startStream — the user saw one frozen message per segment (stuck streaming cursor ▉, never sealed: only the LAST stream gets the final=true seal) plus the real final; 5-6 cumulative snapshots per turn. Adapters that mark draft_stream_is_message keep ONE stream per turn: tool progress lives in the native task card, and the connector's suffix-delta falls back to whole-text append on prefix mismatch, so segments append cleanly. Telegram-shaped drafts keep the per-segment bump. * fix(relay): don't seal the native stream at tool boundaries — only the turn-final does Live-canary finding #5 (Alice; supersedes the incomplete #4 which was necessary but not sufficient). Root cause CONFIRMED by integration trace (test_live_cards_flow_trace.py, real consumer semantics + real adapter + stub transport): at every tool boundary the consumer calls _send_or_edit(finalize=True), which skips the draft path and issues a real send(); the relay adapter's seal-interception converts THAT into draft(final=true) — sealing the stream once per segment. Timeline showed 3 seals for a 3-segment turn: exactly the frozen cumulative ▉ snapshots seen live (the replaced stream never gets stopStream, keeping its cursor). Fix: for draft_stream_is_message adapters, a segment-break finalize (finalize=True, is_turn_final=False) stays ON the draft path as another cumulative frame; only got_done (is_turn_final=True) falls through to send() and seals. Telegram-shaped platforms unchanged. Trace test now pins the invariant: ONE user-visible message per turn. * fix(relay): strip the text cursor from native draft frames Live-canary finding #6 (Alice) — the ACTUAL duplicate-content mechanism, confirmed by full-flow scan of both sides' code + logs. The consumer appends its text cursor (▉) to every non-final display_text tick. The connector's stream sender diffs CUMULATIVE frames via prefix check: 'abc▉'.startsWith → 'abc def▉' is NEVER a prefix match (the cursor sits mid-string), so deltaFor falls back to whole-text append on EVERY tick — chat.appendStream stacks each full cumulative snapshot (cursor included) into the ONE stream message. Exactly the observed thread: repeated blocks, each ending in a frozen ▉, growing per tick. Fixes #4/#5 were real (one stream per turn now) but this was the last mechanism standing. Native streams render their own typing indicator, so the text cursor is pure noise on this path: strip it from draft frames. Prefix check now holds; every tick appends only its true suffix delta. * fix(relay): seal-interception covers EVERY egress door, not just send() Live-canary finding #7 (Alice): one duplication remained after #6 — the stream froze mid-word with the live indicator (never sealed) and the final posted as a separate message. Log receipt: 'Queued follow-up: final text delivery confirmed; delivering explicit media before continuing' — the turn's final went out via the DELIVERY RESOLVER lane (gateway/delivery.py), which calls send_for_platform() DIRECTLY, bypassing send() and its seal-interception. The open stream never absorbed the final; it arrived as a plain 'send' op → chat.postMessage. Fix: hoist the open-draft check to the top of send() (ahead of the explicit-platform branch) AND add it to send_for_platform() — an open native stream absorbs the turn-final regardless of which egress door it arrives through. The stream IS the message. * fix(relay): failed seal falls back to plain send (PR 85796 AI-review point 1) A turn-final seal that fails at the transport must never swallow the final answer: the stream consumer has already disabled the draft transport for the run, so a failed _seal_open_draft returning success=False meant the user got NOTHING. Both seal-interception sites (send + send_for_platform) now fall through to the regular plain-send path on seal failure, with a warning receipt. Also mitigates AI-review point 2 (sticky _open_draft_by_chat after an abandoned turn): a stale entry's failed seal no longer blocks the next turn's delivery. * fix(relay): arm seal-interception optimistically; never disarm on ambiguous failure (audit G-D1) Deep-audit defect G-D1 (HIGH): the outbound leg is at-most-once on the wire but its ack channel is lossy — send_outbound timeout (30s) and WS-drop 'failures' frequently mean the frame WAS delivered and the connector stream is open. send_draft popped _open_draft_by_chat on any failure, disarming seal-interception while the connector stream lived: the turn-final went out as a plain send → orphaned mid-word stream + complete duplicate final (intermittent; needs a drop/timeout inside the draft window). Fix: arm the entry BEFORE the transport call and keep it armed on failure/exception. Safe in every case: sealing a non-existent stream opens+seals a single complete message connector-side, and a truly failed seal already falls back to plain send at both interception sites. Stale-entry damage is self-healing (one warning + plain send). * fix(relay): gateway-side sealed-draft tombstone — G-D1 arming must not resurrect sealed streams Regression fix on G-D1 (live: 'worse than before' — escalating frozen prefixes). Optimistic arming had no seal-awareness: a straggler frame arriving AFTER the seal re-armed _open_draft_by_chat for the already- sealed draft_id; the next send was converted to draft(final=true) on the tombstoned connector key, which CLEARED the connector tombstone (final frame = new-turn signal), re-opened a stream with cumulative content, and left it frozen — repeating per straggler: 4-5 escalating frozen snapshots. Mirror the connector: _sealed_draft_by_chat records the sealed draft_id per chat (tombstoned BEFORE the seal's transport call); send_draft for a sealed draft_id is a success no-op (content already in the sealed message) and never arms. A new turn's fresh draft_id arms normally. * fix(relay): key stream/card state per (chat, turn anchor) — parallel turns must not collide (finding #10) Live finding #10 (Alice; three concurrent turns in one flat DM): all coordination state was keyed per CHAT on a one-active-turn assumption. Three parallel turns produced: turn B's task card merged into turn A's (both were card 'turn:root' — reply_to is None in flat DMs), B left cardless, and _open/_sealed_draft_by_chat clobbered across writers (3x duplicate finals on the last turn). Per-turn machinery was correct; the keys were not. Fix: _draft_key(chat, metadata) = chat + the turn's thread anchor (inbound stamps thread_ts = event.thread_ts or ts on every top-level message, so each turn has one even in flat DMs). draft arming, seal tombstones, both interception sites, and the task-card id all derive from the same anchor. New trace test pins two interleaved turns: distinct cards, own-stream seals, no leaked plain send, no cross-turn tombstone drops (289 tests green). * fix(gateway): preserve cumulative native stream across tools * fix(gateway): consumer-declared final — the seal carries the true final Three composed fixes for the Slack live-cards duplicate-final class: 1. finish(final_text): TurnRunner passes the completed final_response (verifier footer, completion explainer included) as the authoritative finalize payload. The native-stream seal delivers the TRUE final, so post-stream mutation no longer forks a corrective plain send (#11). 2. Interim-send contract: commentary and segment-tail sends carry a gateway-internal _interim_send marker; relay seal-interception skips them at both egress doors. A mid-turn interim send can no longer seal the live stream and orphan the real final into a duplicate. 3. Queued-follow-up lane reconciles an unconfirmed final by EDITING the consumer's delivered message in place (sealed stream = regular message, chat.update live-verified); plain send only as fallback. This was the actual duplicate lane in the parallel canaries — every duplicated turn logged 'final stream delivery not confirmed; sending first response' (subagent-completion queued inbound), not parallelism. Also: draft frames stay prefix-stable gateway-side (no fence-closing, no segment state reset, no commentary reset for stream-is-the-message adapters; MagicMock-safe 'is True' guards). * test+docs: streaming-contract coverage completeness + maintenance guidelines Coverage: two gaps closed on the consumer-declared-final contract — (1) send_for_platform (the delivery-resolver egress door) honors the _interim_send contract: no seal, marker stripped before the wire; (2) finish(final_text) on a turn that never streamed does not adopt the final (delivery ownership stays with the gateway's normal send path for non-streaming models / tool-only turns). Docs: AGENTS.md 'Known Pitfalls' gains the streaming delivery contract — the four invariants of stream-is-the-message adapters (prefix-stable frames, consumer-declared final, interim-send marker, reconcile-by-edit), each traced to its live incident, plus the live-probed Slack streaming API ground truth and the MagicMock 'is True' guard-style note. * fix(relay): seal transport failure must never silently lose the final (review B1) Two halves of one silent-loss path, live-probed on the review branch: 1. adapter: _seal_open_draft did not catch transport exceptions. A socket drop at seal time raised out of send(), skipping the fail-open plain send entirely. Now: retry the SAME idempotent final frame once (the connector's sealed-key tombstone returns the original stream ts for a repeated final — a retry can never open a second stream or duplicate), then report failure so the caller's fail-open path runs. 2. consumer: the turn-final retry (elif not _already_sent) called _send_or_edit with finalize=False, which re-entered the DRAFT-FRAME branch. Its no-op dedupe compared the adopted final against the last unsealed frame, matched, and returned True with ZERO transport calls — final_response_sent went green, delivered_final_matches reconciled, the gateway suppressed its fallback, and the user never received the answer. finalize=True keeps this retry out of the draft branch. Regression suite: tests/gateway/test_relay_seal_failure.py (3 tests). Mutation evidence in follow-up verification: reverting either half sends the suite red. * fix(relay): draft ids unique across gateway incarnations (review B3) The relay connector tombstones sealed streams by (channel, draft_id) and keeps up to 512 of them; they outlive the gateway process. Relay gateways are disposable BY DESIGN (scale-to-zero), and _draft_id_counter restarted at zero every incarnation — so the first turns after every scale-from-zero in a recently-active channel replayed already-sealed wire identities. The connector answered those frames straight out of the old tombstone: zero Slack API calls, the OLD message ts returned as the new turn's identity, the new answer silently dropped while gateway-side flags recorded success. Seed the counter from wall-clock milliseconds at process start. Ids stay plain ints within the existing contract op; incarnations cannot overlap for realistic turn counts and restart gaps. Regression: tests/gateway/test_draft_id_restart_uniqueness.py — the seed test fails on the old code (seed 0 is not epoch-scale). * fix(relay): stream/card state keyed per TURN, not per thread anchor (review B2) The thread anchor is the wrong coordination identity — simultaneously: - too coarse: two parallel turns replying INSIDE ONE Slack thread share thread_ts. Live-probed on the review branch: turn A's final sealed turn B's stream with A's content while A's own stream stayed open, and B's final degraded to a plain send. - too fragile: a flat DM with no thread metadata degraded to the bare chat id, re-creating the original finding-#10 collision the anchor was meant to fix. _draft_key now prefers the triggering inbound message id (message_id / reply_to_message_id — per-turn by construction; the gateway's Slack thread metadata and the consumer's send path both stamp it), falling back to the thread anchor, then the bare chat. The consumer stamps the same reply_to_message_id on draft frames so frames and the turn-final resolve to one key. Task-card ids share the derivation via _card_key (one helper for send AND stop, so the stop always hits the stream the send opened). Legacy resolver-lane callers with placement-only metadata still seal via _match_open_draft's fallback — but ONLY when exactly one stream is open. With several open, an identity-less send stays a plain send: a duplicate message is recoverable, sealing someone else's stream is not. Regression: tests/gateway/relay/test_relay_turn_keying.py (7 tests). * fix(relay): stream-is-the-message is a Slack semantic, gate it on the descriptor (review B4) draft_stream_is_message was hardcoded True on the relay adapter class, i.e. for EVERY relay platform. The base send_draft contract is Telegram-shaped — the draft clears client-side and the final arrives as a separate real send that becomes the history message. With the flag forced on, any non-Slack connector advertising the draft op had its turn-final intercepted into draft(final=true): probed on the review branch with a telegram descriptor, the op stream was [draft(final=false), draft(final=true)] and NO send — no history message would ever be posted. Gate the flag on the negotiated descriptor platform (slack), and skip arming seal-interception entirely when it is off. A future platform with genuine stream-is-the-message native streaming should advertise it via the descriptor rather than widening the platform check by guesswork. Regression: tests/gateway/relay/test_relay_stream_semantics_gating.py (4 tests: gating both ways, telegram final is a real send, slack final still seals). * fix(gateway): mark every mid-turn status lane interim — heartbeats must not seal the stream (review B5) Seal-interception treats the first unmarked send to an armed (chat, turn) key as the turn-final. The consumer's own interim lanes (commentary, tail flush) carry _interim_send, but four gateway-side lanes that fire DURING a streaming turn did not: - long-running heartbeat (default every 180s — probed live: at 3 minutes it sealed the live stream with '⏳ Working — 3 min', the real final posted as a duplicate, and later frames were silently swallowed by the seal tombstone) - inactivity warning - plain-text approval fallback (button lane failed) - background-review notice Add _interim_metadata() beside _non_conversational_metadata and wrap all four call sites. The marker is gateway-internal; the relay adapter strips it before the wire (existing behavior, pinned by test). Note for follow-up: the opt-out shape remains fragile — any FUTURE unmarked mid-turn send lane re-creates this bug. Inverting the contract (explicitly mark the one turn-final send) is the durable fix but touches every adapter's final-delivery path; deliberately kept out of this review-fix series. Regression: tests/gateway/test_interim_send_lanes.py (4 tests). * fix(gateway): interrupted/incomplete turns must not adopt the diagnostic as the stream final (review B6) The finish(final_text) adoption gate checked only 'not failed', but the interrupt/abort returns in agent/conversation_loop.py are {completed: False, interrupted: True, final_response: 'Operation interrupted during …'} with NO failed key. Adopting that diagnostic: 1. sealed the user's streamed partial answer over with the interrupt text (stream-is-the-message: the seal rewrites the whole message), and 2. recorded the diagnostic as the turn-final payload, so delivered_final_matches reconciled and the gateway suppressed its own error-delivery path — the diagnostic became the ONLY thing delivered. Enumerated all 27 final_response-bearing return shapes in conversation_loop.py: every non-happy-path shape carries completed: False (several with a diagnostic final_response and neither failed nor interrupted — retry exhaustion, truncation, codex-incomplete); the happy path routes through turn_finalizer.finalize_turn (completed=True). Gate is therefore: not failed AND not interrupted AND completed is not False. Results lacking the completed key entirely (older callers/test doubles) keep the previous behavior. Regression: tests/gateway/test_stream_final_adoption_gate.py (6 tests, incl. a source-level pin on the run.py call site). * fix(relay): task-card transport failures degrade to failed SendResults (review B7) send_native_task_card_progress and stop_native_task_card_progress let transport exceptions escape. The stop runs inside the progress loop's finally block on the turn-cleanup path, and the post-cancel awaits in gateway/run.py caught only CancelledError — a socket drop during a card publish/stop therefore aborted cleanup BEFORE the final-delivery bookkeeping ran. Three layers, outermost defends any adapter: - both adapter methods catch transport exceptions and return failed SendResults (progress is advisory; the TurnRunner's text fallback already handles failure results) - the progress loop's finally wraps the stop (best-effort; the connector seals orphaned card streams on its own via recycling/eviction) - the cleanup awaits log-and-continue on non-cancellation errors so final-delivery bookkeeping always runs Regression: tests/gateway/relay/test_relay_task_card_failures.py. * fix(relay): a dying turn seals its native stream instead of orphaning it (review B8) Stale-generation exits (/new, /stop mid-stream) and cancellations returned from the consumer's run() with the native stream still open: - the Slack message kept its live streaming indicator forever (the cancellation best-effort edit only runs when _message_id exists, and the native draft path deliberately keeps it None); - the adapter's armed interception state survived the turn, so the next turn on the same key could inherit it and seal a dead draft_id. New adapter op abandon_open_draft(chat, content): seals in place with the text already on screen (the consumer passes its last delivered frame) — the seal adds nothing and claims nothing; delivery flags are never set, so the gateway's normal paths still own whatever happens next. Best-effort by contract (failure reported, never raised); the connector reaps truly orphaned streams via recycling/eviction. The consumer calls it from both death paths: the stale-generation early return and the CancelledError handler. Regression: tests/gateway/test_stream_abandon_on_turn_death.py (4 tests, incl. the next-turn-inheritance hazard). * fix(relay): bound the draft/seal coordination dicts (review M1) _sealed_draft_by_chat's key embeds a per-turn identity, so every completed turn wrote a permanent entry — unbounded growth for the life of a long-running gateway process (the docstring said 'one entry per chat', which stopped being true when the key gained the turn anchor). _open_draft_by_chat could grow the same way via abandoned entries. FIFO-evict both at 512 entries — the same idiom as the sibling bounded cache (_auto_thread_by_chat, capped at 256) and the same size as the connector's own tombstone store. The straggler window the tombstone exists for is seconds long; FIFO is more than enough. Regression: tests/gateway/relay/test_relay_state_bounds.py. * fix(relay): explicit connector rejection disarms interception; exceptions stay armed (review P3) The G-D1 optimistic-arming change silently dropped disarm-on-failure entirely: after an EXPLICIT connector rejection (success=False result — not a transport ambiguity), interception stayed armed even though the stream consumer disables the draft transport on that failure and falls back to edit-based streaming. Its turn-final would then be converted into a seal on a stream the connector just told us is unusable. test_draft_failure_result_propagates claimed to cover this ('must NOT leave seal-interception armed') but passed for an unrelated reason: the stub's canned failure also failed the SEAL, whose fail-open path did the plain send. Split the two semantics and pin each honestly: - explicit rejection (result success=False): disarm — turn-final is a real send (test_draft_failure_result_propagates, now testing what its comment says) - transport exception: ambiguous, stay armed — turn-final still seals (test_draft_transport_exception_keeps_interception_armed, the G-D1 contract) Also corrects commit ba3a24a's claim ('a failed frame disarms interception so the edit-based fallback's real send goes through untouched') to hold again for the rejection case it described. * fix(relay): lost acks are ambiguous, not rejections — on the RESULT channel too (review r2, finding 1) The production ws transport does not raise on ack timeout — it returns {"success": False, "error": "relay outbound timed out"}. The round-1 ambiguity handling keyed entirely on the exception channel, so the shape production actually produces was misclassified as a definite connector rejection. Probed on the head: - lost SEAL ack: skipped the idempotent retry, fell straight to a plain send — duplicate final whenever the seal had actually applied; - lost FRAME ack: the round-1 disarm-on-rejection fired — interception disarmed, frozen native stream beside a plain final. This re-created the original G-D1 ambiguous-ack defect on the result channel. Contract now spans both channels: - transport: the ack-timeout branch tags ambiguous=True. The fail-fast branches (closing / not connected) never sent anything and stay unmarked — they are definite non-delivery. - adapter frame path: ambiguous results keep interception armed (same as exceptions); only definite rejections disarm. - adapter seal path: one shared _attempt() classifier — exception and ambiguous result both mean "unknown"; the SAME idempotent frame is retried once (connector tombstone returns the original stream ts for a repeated final). Only after both attempts stay ambiguous does the caller's fail-open plain send run: a possible duplicate after double ack loss beats a silent loss, and double ack loss on one socket almost always means the transport is down for the plain send too. Regression: tests/gateway/relay/test_relay_ack_ambiguity.py (6 tests, incl. a source-of-truth check that the transport tags the timeout branch and leaves fail-fast branches unmarked). * fix(relay): stream semantics + draft capability resolve per CHAT, not per primary (review r2, finding 2) One RelayAdapter fronts N platforms (Phase 1.5): descriptors accumulate per platform on the transport and egress is tagged per chat — but the round-1 gate keyed draft_stream_is_message and supports_draft_streaming() off the PRIMARY scalar descriptor. Probed on the head: - Slack primary + Telegram chat: the Telegram chat's turn-final was intercepted into draft(final=true) — no real Telegram history message; - Telegram primary + Slack chat: the Slack chat was denied native streaming entirely. Resolve both through _descriptor_for_chat — the same per-chat machinery max_message_length already uses (added for the identical class of bug: the primary's 39000-char cap over-sending into Discord 400s): - new stream_is_message_for_chat(chat_id) on the adapter; arming and NotImplementedError gating use it. The class attribute remains as the single-platform value and legacy-probe fallback. - supports_draft_streaming() gains an optional chat_id kwarg (base signature updated; single-platform adapters ignore it). The consumer passes chat_id with a TypeError fallback for out-of-tree adapters. - the consumer's four draft_stream_is_message reads collapse into one _stream_is_message() helper that prefers the per-chat probe (class-resolved, MagicMock-safe) over the attribute. Platform-name inference ("slack") stays deliberate: a descriptor-level semantic field is the right eventual contract but is a cross-repo wire change — noted for the gg follow-up so future platforms advertise the semantic explicitly. Regression: tests/gateway/relay/test_relay_multiplatform_semantics.py (5 tests: both starvation directions, scalar fallback, per-chat capability gate). * fix(gateway): split delivery + authoritative footer reconciles by suffix, not full resend (review r2, finding 3) The _FINAL_TEXT adoption guard refuses wholesale adoption on split turns — correct (NousResearch#78541: sealed heads would repeat inside the tail) but it was absolute: a post-split verifier footer never entered the ledger, delivered_final_matches() reported a mismatch, and the gateway resent the ENTIRE body+footer after the split chunks (the #11 duplicate class, one level up). When the authoritative final strictly prefix-extends the split ledger, the missing suffix is the only undelivered content: append it to the live tail and the ledger, so the finalize carries it and the recorded payload reconciles. Non-prefix rewrites keep the full-resend fallback — a rewrite cannot be patched onto sealed heads. Regression: tests/gateway/test_split_final_suffix_reconcile.py (3 tests: suffix rides the tail + reconciles, rewrite still mismatches, unsplit adoption unchanged). * fix(relay): cancellation mid-seal restores open state so abandon can close the stream (review r2, finding 4) _seal_open_draft pops the open entry and writes the local tombstone BEFORE awaiting transport I/O — correct ordering for the straggler race, but CancelledError is not an Exception: a cancel during the await bypassed all failure handling, leaving the remote stream live (visible streaming indicator until connector eviction) while the local state said 'nothing open'. The consumer's abandon pass — added for exactly this turn-death case — found nothing to close and no-oped. On CancelledError: restore the open entry, drop the premature tombstone (only if it is still ours), re-raise. The abandon path then seals the stream in place with the on-screen text. Regression: tests/gateway/relay/test_relay_seal_cancellation.py (2 tests: state restoration, and end-to-end cancel→abandon→remote seal). * fix(relay): thread anchors are placement, not turn identity — revive the placement-only fallback (review r2, finding 5) _match_open_draft's single-open-stream fallback was dead for its primary intended callers: metadata carrying thread_ts/thread_id (placement-only resolver lanes) was classified as having 'turn identity', so those sends never reached the fallback — probed: a plain final posted beside the still-open turn-keyed stream. Only per-turn MESSAGE ids are identity now. Thread-anchored and bare callers share the fallback: absorb into the chat's open stream when EXACTLY one is open; stay a plain send when several are (duplicate is recoverable, wrong-stream seal is not). Callers WITH a message id whose key misses never fall back — their identity is authoritative and a miss means the stream belongs to a different turn. Regression: 4 new tests in test_relay_turn_keying.py (thread-anchored seal, both ambiguous-stay-plain shapes, id-mismatch never steals). * fix(relay): random process nonce for draft-id seeding (review r2, follow-up 6) The epoch-millisecond seed (round-1 B3 fix) mitigates the restart-replay class but is not a uniqueness guarantee: two gateways starting in the same millisecond, a forked process inheriting the class state, or a clock step backwards can all mint colliding wire identities against the connector's per-(channel, draft_id) tombstone store. Seed from secrets.randbits(49) instead: collision probability negligible, no clock dependence, and ids + realistic per-process turn counts stay comfortably inside the connector's JS number range (draft_id?: number, 2^53). Regression test now spawns two real interpreters and asserts their seeds differ — the exact scale-to-zero restart shape, and both start within the same second so a clock-locked seed would fail it. * fix(relay): stamp per-turn Slack egress identity — cache is fallback only (R3-5) The connector (gateway-gateway#210) fills chat.startStream's recipient_user_id / recipient_team_id — required by Slack when streaming to a channel — from metadata.user_id / metadata.scope_id. The gateway stamped only slack_team_id per-turn and left user_id (and scope_id) to RelayAdapter._with_scope, whose per-chat caches are keyed on chat_id alone and overwritten by every inbound message: with users U1 and U2 running overlapping turns in one channel, U2's arrival overwrote the cache before U1's stream opened, and U1's stream carried U2 as recipient_user_id. _thread_metadata_for_source now stamps scope_id and user_id from the turn's OWN source (setdefault — explicit values win), so identity is turn-scoped data on the wire. _with_scope is unchanged and fill-only: the caches keep serving restart/synthetic sends that carry no per-turn identity, which is all they were ever safe for. Mutation evidence: reverting the run.py hunk sends test_thread_metadata_stamps_per_turn_user_and_scope and test_concurrent_turns_carry_their_own_identity red; restore returns green. The _with_scope fill-only tests pass on both trees (existing correct behavior, now pinned against regression). --------- Co-authored-by: Ben Barclay <ben@nousresearch.com>
Dannyxlm
pushed a commit
that referenced
this pull request
Aug 21, 2026
…er-bot Sessions browser (NousResearch#90732) Symptom: switching between bots forked a brand-new "Bot Chat" for the returned-to bot on EVERY switch, burying the user's real forever-chat (one report: a 930-message chat displaced by 7 forks in one morning). Root cause is a self-perpetuating loop between three parties: - state.db enforces UNIQUE(title): the first fork permanently squats the "Bot Chat" title; every later mint's title request is silently dropped by set_session_title (returns 0, no error to the caller). - The post-turn LLM auto-titler then names the untitled fork from its kickoff content ("Assistant introduction request #2", ...). - openBotCanonicalChat's identity check is title-string matching, so the renamed fork reads as "not plumbing" -> corrupted metadata -> clear pin -> mint again. Grandfathered pre-convention chats (real history, derived titles) hit the same branch and are forked away from immediately. Fix, two invariants: 1. Adopt-before-mint: createCanonicalChat first scans the profile via session.list include_hidden:true for an existing "Bot Chat" row and re-pins it instead of creating. The UNIQUE index makes this an exact registry lookup (at most one match), not a heuristic. Older gateways without include_hidden find nothing and fall through to mint. 2. A pin that resolves to a NON-plumbing session carrying real history is the user's conversation - keep it and open it (title drift is metadata damage, not ownership loss). Only a pin resolving to an EMPTY stray draft is treated as corrupted and replaced (which now goes through adoption first). Also removes the right-click -> Sessions per-bot stored-session browser (ProfileSessionsWorkspace and its atoms/query/rows). Bot Mode's product contract is ONE forever-chat per bot; a browser listing every hidden plumbing session contradicts that and confused users into opening dead forks. The Sessions workspace test goes with it; the include_hidden source-shape test now pins the adoption scan instead, and a new suite (canonical-chat-adopt-before-mint.test.mjs) covers both invariants plus the older-gateway fallback.
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
20 participants
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.
Summary
NousResearch/hermes-agentmaininto Danny's forkWhy
The fork had diverged: it contained one CloudSeed-critical behavior that official upstream still does not, while also being thousands of commits behind. Force-syncing would have silently deleted that behavior. This merge keeps it and absorbs current upstream before the visibility patch lands.
Cross-check
The final tree differs from current official upstream in only the five files belonging to PR #1:
agent/codex_runtime.pyagent/conversation_loop.pyrun_agent.pytests/run_agent/test_codex_app_server_ephemeral_context.pytests/run_agent/test_codex_app_server_integration.pyVerification
uv run pytest -q tests/run_agent/test_codex_app_server_ephemeral_context.py tests/run_agent/test_codex_app_server_integration.py— 33 passedgit diff --check origin/main HEAD— passedAutomated by Ava on Danny's behalf.