fmt(js): npm run fix auto-fix - #2
Open
github-actions[bot] wants to merge 1 commit into
Open
Conversation
SGuibord
pushed a commit
that referenced
this pull request
Aug 13, 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 NousResearch#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>
SGuibord
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 NousResearch#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 NousResearch#5 (Alice; supersedes the incomplete NousResearch#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 NousResearch#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 NousResearch#4/NousResearch#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 NousResearch#7 (Alice): one duplication remained after NousResearch#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 NousResearch#10) Live finding NousResearch#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 (NousResearch#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-NousResearch#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 NousResearch#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>
SGuibord
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.
github-actions
Bot
force-pushed
the
bot/js-autofix
branch
from
August 23, 2026 17:26
cad5ab8 to
28e8221
Compare
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
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.
Auto-generated by the
auto-fix lint issues & formattingworkflow. Auto-merges (squash) once CI passes. If CI fails ormainmoves, the PR is auto-closed and the branch deleted — the next run re-applies on the current state.