feat(relay): live-card ops — native draft streaming + task cards over the relay (gateway half) - #85796
Conversation
… 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.
Live canary review — 3 findings, fixes attached (staging, Alice, 2026-08-15)Ran this PR live over the relay against gg PR #210 (stacked staging: rc4-freeze + gg#200 + #207 + #210; hermes: v2026.8.13 + this PR + staging deviations). Draft streaming and task cards both work end-to-end — receipts in NS-658/NS-660. Three integration bugs surfaced that unit tests structurally cannot catch (they all live at the TurnRunner↔adapter seam); fixes are on the canary worktree, cherry-pickable, each with tests: 1. Card lane never arms — missing
|
PR: feat(relay): live-card ops — native draft streaming + task cards over the relay (gateway half)
|
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.
…d contract Live-canary finding NousResearch#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).
…age_id) Live-canary finding NousResearch#3 (Alice, staging): the relay inbound leg is at-least-once. On WS re-handshake the connector replays its durable per-instance buffer; a long multi-tool turn (60-100s) straddling a quiet socket drop got its ORIGINAL inbound replayed after the turn finished, re-running the entire turn — the user saw the final answer posted 2-5x (each a separate execution, hence slightly different texts). Receipts: same msg text at history=0 in back-to-back sessions 121647/121840, no Slack-side retry on the connector (envelope dedupe never fired). Consumer-side idempotency: bounded FIFO seen-set (512) keyed by platform message identity; events without a message_id never dedupe (fail-open — dropping a real message is worse than rerunning one). No wire change; contract v1 untouched.
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.
…e 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.
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.
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.
…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.
…iguous 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).
…t 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.
…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).
Canary saga complete — 11 fixes pushed to this branch (
|
| # | Finding | Root cause |
|---|---|---|
| 1 | Card lane never armed | TurnRunner probes native_task_cards_enabled(); PR only had supports_native_task_cards() |
| 2 | TypeError storm, 4-5x duplicate finals | card methods took positional card_id; TurnRunner uses the native keyword contract |
| 3 | Whole-turn re-execution | relay inbound is at-least-once; replay after WS re-handshake re-ran the turn (dedupe on chat+message_id) |
| 4 | New stream per tool boundary | per-segment draft_id bump opens a new Slack stream (skip for draft_stream_is_message) |
| 5 | Stream sealed at every boundary | segment-break finalize=True fell through to send() → seal-interception; only is_turn_final seals now |
| 6 | Snapshot stacking in one message | text cursor broke the connector's prefix-delta on every tick |
| 7 | Unsealed stream + separate final | delivery resolver calls send_for_platform() directly, bypassing send()'s interception |
| — | Failed seal swallowed the final | (this PR's AI-review pt 1 — implemented: fall back to plain send) |
| G-D1 | Intermittent orphan+duplicate | lossy outbound ack: 'failed' frames often delivered; arming now optimistic |
| G-D1b | Regression: escalating frozen snapshots | optimistic arming resurrected sealed streams; gateway sealed-draft tombstone (symmetric with connector) |
| 10 | Parallel turns collide | ALL state was keyed per-chat (one-turn assumption): merged cards, clobbered seals, 3x dupes. Re-keyed per (chat, turn thread anchor) |
Deferred review items (deep-audit MED/LOW, in gg#210 comment): appendStream re-delta on timeout, live-stream eviction, restart draft_id collision, empty-frame guard, WS _read_loop not failing _pending (30s stall), stream-consumer ordering audit incomplete (2x worker timeout — the got_done/_fallback_final_send interaction deserves human eyes).
Merge requirement
The cross-repo E2E pair (connector_livecards_harness.mjs + gateway_livecards_driver.py) with fault injection — drops, ack timeouts, stragglers, parallel turns in one chat — before the next release cut. Every one of the 14 findings lived at a seam no unit test crosses; tests/gateway/relay/test_live_cards_flow_trace.py (on this branch) is the seed.
Ready for review. cc @ben
Finding #11 (final validation round) — diagnosed, NOT fixed: stale-finalize reconciliation duplicates sealed streamsLast remaining duplication, root-caused with a log receipt but deliberately left for review rather than midnight-hotfixed (design decision required): Receipt: Chain: (1) stream seals with the turn-final text; (2) a POST-SEAL payload mutation lands — here the file-mutation verifier footer, but any plugin transform triggers it too; (3) run.py's #71643 stale-finalize lane calls Fix options for review:
Current failure mode is benign (full information delivered, one extra message). All other 13 findings remain fixed and verified — parallel + sequential rounds otherwise clean. |
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).
|
Final canary verification — 2026-08-16 ✅ (closes the findings ledger, resolves #11) Two new commits since the last ledger update, live-verified on the staging sandbox before transfer:
Live result (real workspace, parallel tool-heavy turns with subagent delegation, approval prompts, and verifier-footer mutation): exactly one final per turn, footer inside the sealed message, task cards clean. Verified by the human oracle. Finding #12 is retired along with #11 — no Verification on the PR lineage: relay + consumer + trace harness 296 passed · streaming slice 538 passed / 0 failed · new contract suite Connector half: gateway-gateway#210 (raw-dialect + idempotent seals). The two halves were validated together and should ship together. |
…delines 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.
|
Coverage + docs completeness pass ( |
Summary
Gateway half of Slack live-cards over the relay (connector half: NousResearch/gateway-gateway#210). When the relay descriptor advertises
supports_draft_streaming+ thedraft/task_cardops, streamed replies ride the connector's native Slack streaming (chat.startStream/appendStream/stopStream) as stream-is-the-message: the stream IS the final message, sealed in place — no separate final post. Task cards render as plan-mode streams keyed per turn.Validated end-to-end on the staging sandbox (real Slack workspace, live canary gateway, tool-heavy turns fired in parallel — including subagent delegation, approval prompts, and post-turn payload mutation): exactly one final message per turn, live streaming across tool boundaries, task cards flipping in place. Every defect the canary surfaced is a commit on this branch with a regression test; the ledger is below.
Architecture: three delivery invariants
Stream-is-the-message. Adapters that declare
draft_stream_is_messagekeep ONE cumulative native stream per turn: tool boundaries emit further cumulative frames (no draft_id bump, no segment reset, no fence-closing mutation — frames must stay prefix-stable for the connector's append-only delta), and only the turn-final seals.Consumer-declared final.
finish(final_text)passes the completedfinal_response— including post-stream augmentation the accumulator never saw (file-mutation verifier footer, turn-completion explainer) — as the authoritative finalize payload. The seal delivers the TRUE final,delivered_final_matchesreconciles, and no corrective duplicate send ever fires. Interim sends (commentary, segment tails) carry an internal_interim_sendmarker so seal-interception can never mistake them for the final.Reconcile by edit, not by send. Every lane that previously plain-sent beside a sealed stream now edits in place: seal-interception covers both egress doors (
send()andsend_for_platform()), and the queued-follow-up lane (subagent completions arriving mid-turn) edits the consumer's delivered message instead of posting a duplicate. A sealed native stream is a regular message;chat.updateon it is live-verified.Canary findings ledger (all fixed on this branch)
is_turn_finalseals_interim_sendcontractThe dominant separate-message duplicate was the queued-follow-up lane (every duplicated live turn logged
final stream delivery not confirmed; sending first response before continuing) — parallelism was a confound: the failing turns were the ones that spawned subagents. Finding #11 and the queued-lane duplicate share the same root (post-stream payload mutation) and die together under invariant 2.Compatibility
draft_stream_is_messagedefaults false (is Trueguards keep MagicMock-based tests honest).finish()bare (interrupt/error paths, older callers/test doubles) keeps legacy behavior; the payload variant is duck-type-safe.content_delivered=Truesuppresses the complete send #71643/[Bug]: Streaming edit transport duplicates final message when Telegram rate-limits the cursor-strip edit #36965 suppression semantics preserved;test_stale_finalize_suppression.pyextended to accept the (strictly better) adopted-final shape alongside the reconcile-edit shape.Validation
tests/gateway/test_stream_final_contract.pytests/gateway/relay/,test_stream_consumer_draft.py, incident suites)tests/gateway/tests/gateway/Follow-ups (NS-658)
envelope_iddispatch logging,_read_loopfailing_pendingon dropMerge after gateway-gateway #200 → #207 → #210.