ab2f4602de93ca1c864275e53ca107b158da740a
83 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 kshitijk4poor
|
ab2f4602de |
refactor: MessageEvent to gateway/platforms/event.py; ElicitationHandler takes a call_context thunk
Breaks the two import cycles that forced Protocol stand-ins in the F821 sweep, so the two sites now name the real types. gateway/platforms/event.py (new leaf): MessageType, ProcessingOutcome, MessageEvent moved out of base.py verbatim. Their only dependency is gateway.session.SessionSource; base.py imported helpers.py at module level, so helpers could not name MessageEvent. Now TextBatchAggregator is typed by the real MessageEvent. 249 importers repointed (`from gateway.platforms.base import` -> `.event`, preserving each import's layout); gateway.platforms.__init__ re-exports from .event. The three revert-scheduled PLUGIN-COMPAT pointers that named these symbols (gateway.slash_commands → MessageType, dingtalk → MessageType, photon → ProcessingOutcome) and their COMPAT_MANIFEST rows now target gateway.platforms.event. Docs updated: ADDING_A_PLATFORM.md, adding-platform-adapters.md (en + zh-Hans). tools/mcp_tool_sampling.py: ElicitationHandler no longer holds a back-reference to its MCPServerTask (mcp_tool imports sampling, so the task type cannot be named there). It only ever read owner._pending_call_context, so it takes `call_context: Callable[[], Context | None]` and MCPServerTask passes `lambda: self._pending_call_context`. The consent call is one `functools.partial`, run directly or inside the captured Context. ty on the 11 touched production files vs origin/main: 0 new diagnostics, 14 resolved. (The one `source: SessionSource = None` diagnostic moves with the class; typing it Optional exposes ~60 unguarded call sites — separate follow-up.) Tests: tests/gateway + tests/plugins + tests/tools + touched files, 18,235 passed; the 31 failures reproduce identically on origin/main (macOS /private/tmp, systemd socket, long-path fixtures, live-service tests). |
||
|
Nacho Avecilla
|
a6e10e693f |
fix(gateway): stop dropping the first message after an Azure agent sleeps (#99736)
* feat(gateway): let NAS broker the scale-to-zero suspend where the guest has no lever * fix(gateway): require the going_idle ack and outlast the broker before suspending * fix(gateway): release the redial hold on every path that abandons the suspend * fix(gateway): hold the re-dial only for the brokered lever, and require it * fix(gateway): make the re-dial hold contract real and fence the in-guest suspend too * refactor(gateway): share the watcher-iteration and descriptor helpers across the sleep tests * refactor(gateway): move the sleep tests' repeated setup into their fixtures * refactor(gateway): one docstring line per sleep test, and parametrise the abort and lever paths * fix(gateway): fence the freeze gap, cool down aborts, and stop clobbering a shutdown drain * fix(gateway): slice the Fly freeze fence on the wall clock and widen it for larger machines |
||
|
Teknium
|
2776813df3 |
compat(plugins): temporary import-path shims for external plugins — ONE commit, revert on schedule
The Sep 2026 decomposition (PR #102117) makes internal import paths a non-API: names now live in the focused modules that define them. This commit is the ONLY thing keeping the old paths alive, so external plugins have time to update. It is deliberately a single, unsquashed commit: git revert <this sha> removes every shim, stub and manifest at once on the announced date. Nothing in-tree may depend on these pointers: scripts/check_compat_pointers.py (wired into lint.yml) fails CI if it does. What it adds (see COMPAT_MANIFEST.md, compat_manifest.json): - 332 facade modules get one delimited `PLUGIN-COMPAT` block appended at the end of the file - 1,172 moved names resolved lazily via a module `__getattr__` (PEP 562) — never a top-level import, so no import cycles; facades that already had `__getattr__` get a chained one - 592 third-party/stdlib names the old modules used to expose, with their original import statements - 266 public definitions that had been deleted as unused, restored byte-for-byte from the pre-decomposition tree (+40 private helpers and 16 imports pulled in only because a restored definition needs them) - 3 deleted modules recreated as re-export stubs (gateway/startup_watchdog, hermes_cli/observability/ relay_runtime, tools/environments/modal_utils) - private names (`_x`) get no pointer: they were never API (3,792 skipped) Verified: all 335 touched modules import under a fresh HERMES_HOME and every manifest name resolves; the lint reports zero in-tree uses; ruff clean; targeted suites unchanged. |
||
|
Teknium
|
e83816a4d1 |
review-fix(comments): restore lost #NNNN rationale comments across non-test source (mechanical sweep, condensed, code unchanged)
For each issue anchor present in BASE
|
||
|
Teknium
|
454ae0fbf4 | refactor(relay/adapter): one draft-frame builder for open/seal frames | ||
|
Teknium
|
155ee47a81 | refactor(relay/adapter): one op-gated best-effort egress helper (_gated_op) for media/prompt/react/thread ops; shared typing frame | ||
|
Teknium
|
bf8f95d33a | refactor(relay/adapter): single format-hint stamper, shared Slack egress prep, prompt label tables as data | ||
|
Teknium
|
6fb2921c54 | refactor(relay): AST-neutral line packing (hug/fold/pack) across relay package | ||
|
Teknium
|
55cc148069 | refactor(relay/adapter): compact docstrings/comments (keep WHYs), fold cache-capture loops, reuse _apply_descriptor in __init__ | ||
|
Teknium
|
3635aa6a32 | refactor(relay/adapter): unify task-card frame emission, SendResult projection and prompt-unavailable tail; collapse defensive ladders | ||
|
Teknium
|
659c6b9fff | refactor(gateway): simplify session, status, stream_consumer, kanban_watchers, relay adapter, hosted-room driver/discussion/replicas, shutdown/lifecycle, pairing, channel_directory, control_socket and small modules | ||
|
Teknium
|
aed6720dab |
refactor(gateway/run, slash_commands): dispatch tables, helper unification and hand-reviewed comment compaction
run.py: - built-in adapter creation: 9-branch if/elif -> _BUILTIN_ADAPTERS table - idle slash-command routing: 35 `if canonical == ...` branches -> _gateway_idle_command_handlers() - shared helpers: _send_command_ack (4 sites), _command_origin_for_source (2), _session_entry_for_manager (goal/heartbeat), _toggle_adapter_auto_tts_set (2), _load_env_or_agent_cfg_timeout (2), _float_env reuse (2), _resolve_session_key_or_none (3), _running_agent_ids (4), _schedule_rename_from_title_thread (2), _write_runtime_status_quiet (5), _AUTO_RESET_CONTEXT_NOTES/_auto_reset_reason_text - ruff SIM102/SIM103/SIM105/SIM108/SIM118 + F401 across gateway/ (semantics re-reviewed; sqlite Row `.keys()` and side-effecting assignments kept) - two hand-reviewed comment/docstring compaction passes (AST-identical, rationale kept) slash_commands.py: - /model: typed path and picker callback shared one 200-line commit block -> _perform_model_switch + _commit_model_switch - comment/docstring compaction (AST-identical) |
||
|
pierrenode
|
3245264668 |
fix(relay): carry routed profile through the passthrough-plane forward
The relay text lane stamps `SessionSource.profile` from the wire frame (#60586), but `PassthroughForward` had no profile field, so a relayed Discord slash-command/button/modal always landed in the default profile's agent:main namespace even when the connector resolved a specific profile. Add an optional `profile` to `PassthroughForward` (read off the wire in `_passthrough_from_wire`) and stamp it on the interaction's SessionSource. Absent on the wire → None → legacy routing, byte-identical for single-profile gateways. Contract doc updated. Salvaged from #61012 (pierrenode); two context conflicts resolved (delivered_via_upstream_relay / _platform_by_chat landed on main). |
||
|
Victor Kyriazakos
|
3f36c87e1e |
feat(relay): delete_message over the additive delete op — fresh-final preview cleanup
Companion to the connector's delete op (gateway-gateway 119a228). The fresh-final unfurl route re-posts the completed reply and previously left the sealed streamed preview behind (double delivery). delete_message now emits op=delete when the negotiated descriptor advertises it; without the advertisement it returns False with zero wire traffic, degrading to the old leave-the-preview behavior against older connectors. Consumer-level test drives placeholder -> stamped fresh final -> delete of the original preview id. |
||
|
Victor Kyriazakos
|
cef4c88f71 |
fix(relay): route force-on-unfurl streamed finals through fresh chat.postMessage
Slack evaluates link previews exactly once, at chat.postMessage (live probe 2026-08-28: URL at post + stamps unfurls; a chat.update that INTRODUCES the URL never does, stamped or not). Edit-based streaming posts its first frame before the model produces any URL — on flat DMs with tool_progress=accumulate that frame is the task card — so a configured unfurl_links/media: true could never surface a preview: the only post Slack evaluates carries no link. RelayAdapter now implements prefers_fresh_final_streaming(): True only when the Slack unfurl hints contain an explicit True AND the final text carries a link. The stream consumer then delivers the completed reply as one fresh send — URL and stamps present at the single moment Slack looks. False-only hints (enterprise fail-closed posture) keep the edit lane untouched: suppression rides the placeholder post and edits can never add a preview, so false inherits with zero streaming-UX cost. Consumer-level contract test drives the exact regression shape (placeholder frame -> URL-bearing final) and asserts op=send + stamps; verified RED against the unfixed adapter, GREEN with the hook. |
||
|
Ben Barclay
|
be5c75165f |
fix(relay): fall back to descriptor platform for unfurl stamping
The send and send_media lanes resolved the platform only from _platform_by_chat, which is empty until an inbound frame arrives (e.g. after a gateway restart). A proactive send to a Slack chat then missed the unfurl stamp. Mirror the streaming gate and delivery resolver: fall back to the negotiated descriptor's platform. |
||
|
Victor Kyriazakos
|
634d9c3f4c |
fix(relay): coerce string unfurl knobs and disable Slack draft streaming
Live staging (Coatue Slack): - hermes config set / Railway knobs persist "true" as a string; bots that omit unfurl_links do NOT inherit the human default, so dropping the string looked like suppression. - chat.startStream cannot carry unfurl_*. Native SlackAdapter already falls back to chat.postMessage; the relay now matches. |
||
|
Victor Kyriazakos
|
cc2fb46275 |
feat(relay): stamp slack unfurl_links/unfurl_media onto outbound frame metadata (gateway-directed)
Relay-fronted Slack reads platforms.relay.extra.slack.unfurl_links/unfurl_media and stamps explicit booleans onto the frame metadata; the connector forwards them to chat.postMessage with no config of its own (mirrors reply_in_thread). Covers send, send_for_platform (cron/scheduled), and send_media lanes. |
||
|
Ben Barclay
|
29033a3fd5 |
fix(relay): restore voice-note STT — wire media[] MIMEs, message_type "voice", and a User-Agent for CDN downloads (#95274)
* fix(relay): map wire media[] → event.media_types; accept message_type voice A relayed voice note arrived as MessageType.AUDIO with media_types=[] — the STT gate (_event_media_is_stt_input) excludes AUDIO unconditionally and its per-attachment MIME rescue was unreachable, so STT never fired and the agent fell back to the "user sent an audio file attachment" context note (live-verified on staging 2026-08-26, Discord + Telegram). Two wire-boundary fixes, both additive within contract_version 1: - "voice" parses to MessageType.VOICE: the enum already had it — pinned by test so a future refactor can't collapse the two. - media[] is now mapped into event.media_types (positional alignment with media_urls; mime-less entries keep their slot as ""). This is what run.py's per-attachment classifiers key off, so EVERY relayed attachment — image vs document, audio vs voice — now routes like its native-adapter equivalent, not just voice notes. Behaviour pinned: new-connector voice → STT-eligible; legacy audio-typed events unchanged (no STT); music uploads never STT-eligible (direct _event_media_is_stt_input assertions on real wire-parsed events, not mocks). Pairs with the gateway-gateway PR that puts "voice" on the wire. * review: pin the STT gate by test; fail safe on media/media_urls mismatch Addresses independent review of #95274. 1. The PR's acceptance criterion is STT ROUTING, but no committed test called _event_media_is_stt_input — it was only asserted ad-hoc. Adds TestSttGate: voice→eligible, voice-without-media_types→eligible (the new-connector/old-gateway shape), legacy audio-typed voice note→not eligible, music→not eligible. Mutation-verified: removing the VOICE branch from the gate turns these RED. 2. media_urls and media[] are INDEPENDENT wire fields that consumers index by the same i. Mapping MIMEs positionally without checking agreement means a disagreeing producer misassociates a MIME with the wrong URL and mis-routes that attachment — strictly worse than no MIME, which degrades safely to message-level classification. _media_types_from_wire() now maps only when the lengths agree, warns and returns [] otherwise. Note for the record: MessageType.VOICE predates this PR and the gate's VOICE branch ignores media_types, so a NEW connector against an OLD gateway ALREADY fires STT. That is desirable, but it is not "unchanged" — the PR body's rollout matrix said otherwise and is corrected. * fix(relay): send a User-Agent on relay media requests (Discord CDN 403) Discord's CDN rejects urllib's default "Python-urllib/x.y" User-Agent with HTTP 403, and RelayMediaClient never set one. Every Discord CDN pass-through download therefore failed; _localize_inbound_media then kept the raw URL (its "a public URL still has value" branch), and the consumer tried to open a URL as a FILE PATH: WARNING gateway.relay.media: relay media download failed for https://cdn.discordapp.com/...voice-message.ogg: HTTP Error 403 INFO gateway.run: Voice transcription failed for https://cdn.discord... : Audio file not found: https://cdn.discordapp.com/... This killed ALL Discord relay media inbound — voice notes, images and documents alike — not just the voice lane. Telegram/WhatsApp were unaffected because their media is connector-re-hosted (/relay/media/{id}, fetched from our own host) and localizes to real /tmp paths. Reproduced from a clean shell against a live CDN URL: curl (own UA) -> 200 urllib, no UA -> 403 Forbidden urllib + descriptive UA -> 200, 14583 bytes, OggS magic Fix: a module-level _MEDIA_USER_AGENT sent on both download() and upload(). upload() only ever targets our own connector so it was not broken, but a single client should identify itself consistently. Validated on staging: hot-patched hermes-agent-stg-test-6698, restarted the gateway service, and Ben's Discord voice note transcribed successfully — zero new 403s and zero new transcription failures after the patch (last 403 predates it). Test is mutation-verified: removing the UA from download() turns it RED while the other five media tests stay green. * fix(relay): keep url↔mime pairing through media localization Addresses a blocking review finding on my own change: mapping media[] into media_types created a POSITIONAL contract that the rest of the inbound path then broke. 1. _localize_inbound_media (adapter.py) filtered media_urls without filtering media_types. Dropping a dead connector re-host is a NORMAL best-effort path, so every surviving attachment inherited its neighbour's mime. Reproduced through the real functions: before urls [.../relay/media/dead, .../kept.png] types [application/pdf, image/png] after urls [.../kept.png] types [application/pdf, image/png] <-- PNG reads as PDF _event_media_is_image(ev, 0) -> False The loop now carries (url, mime) as PAIRS, so a dropped URL drops its mime with it. 2. _media_types_from_wire compared LENGTHS only, which is not alignment: equal-length-but-reordered wire fields were accepted and paired wrongly, and an absent media_urls skipped the check entirely while still emitting types. Resolution is now BY URL (url -> mime lookup over media_urls); an unmatched URL degrades to "" and falls back to message-level classification. Tests: 4 new cases driving the real chain (wire parse -> localization -> run.py classifier), incl. the dropped-first-attachment case the existing localization test could not catch (it builds events without media_types). The obsolete length-mismatch test now asserts the stronger by-url guarantee. Both fixes mutation-verified: reinstating the URL-only filter fails 1 test, reverting to positional resolution fails 3. Relay suite 258 passed; media/voice/stt selection 685 passed; ruff clean; cross-repo integration payload re-verified. * fix(relay): media_types is always one slot per media_url Self-review after two review rounds flagged this bug class in adjacent seams: I checked the function I edited, not every consumer of the parallel arrays I created. Grepping ALL writers found a third instance. merge_pending_message_event (gateway/platforms/base.py:2725-2735) EXTENDS media_urls and media_types together when a second media message merges into a pending one. My mapping could emit a POPULATED media_urls with an EMPTY media_types (an older connector sends media_urls but no media[]), so extend() concatenated lists of different lengths: A urls [old1.png, old2.png] types [] B urls [new.pdf] types [application/pdf] merged urls [old1.png, old2.png, new.pdf] types [application/pdf] -> old1.png reads as application/pdf; the real PDF gets '' Fix: media_types is now ALWAYS len(media_urls), padded with '' — the url-keyed lookup runs even when media[] is absent, and the localizer rewrites the list unconditionally (no short-circuit that could leave a stale/short list behind). Tests: 4 new cases — padding with no media[], the merge shift above driven through the real merge_pending_message_event, localization preserving the invariant while dropping an entry, and normalization of a short/empty media_types arriving from a non-wire source. All mutation-verified: removing the padding fails 4; restoring the guard fails 1. Relay 262 passed; media/voice/stt selection 689 passed; ruff clean; cross-repo integration payload re-verified. |
||
|
Victor Kyriazakos
|
424d07edac |
fix(relay): prompt-lifecycle acks are fire-and-forget — awaiting them ON the read loop self-deadlocked the transport
Round 2 of the approval-turn stuck-stream hunt. Round 1 (interim-marked
acks) fixed the draft-hijack-by-matching path — live logs confirm the
absorption fallback no longer fires — but the freeze persisted because
of a second, deeper defect on the same codepath:
_consume_prompt_response executes ON the transport read loop (inbound
frame -> _handle_frame -> _inbound handler). The handler awaited
self.send() for its '✅ Approved once' ack — but send() blocks on an
outbound_result future that ONLY the read loop can resolve, and the
read loop is blocked inside this very handler. Guaranteed self-deadlock
for the full outbound timeout (30s) on EVERY button tap. While wedged,
everything on the transport starved: draft appends (the frozen stream
right after approving), sibling approval-card sends (timed out into
'possibly-delivered' — the observed double-approval ambiguity), and the
turn's seal (timed out ambiguous -> plain-send fallback -> duplicate
final). Log signature was the tell: card-send timeout at tap time, no
absorption INFO, no seal-failed WARNING, no suppression line.
Fix: _send_lifecycle_ack() — acks ride a background task with strong
ref retention; the handler returns immediately and the read loop keeps
consuming, so the ack's own result frame resolves normally. Applied to
all six lifecycle sends (approval ack, slash-confirm ack + result text,
clarify acks, expiry notice). Acks are cosmetic by contract; failure
logs at debug and never breaks the reader.
Tests: new deadlock-shape test (gated transport send; handler must
return within 1s and the ack must still egress afterwards — RED on the
awaited version via TimeoutError at the exact deadlock), prior 3 tests
green with a yield for the background task. Targeted sweep 203/203.
|
||
|
Victor Kyriazakos
|
57fe01e367 |
fix(relay): prompt-lifecycle acks are interim sends — the approval ack was sealing the turn's own draft stream
Live finding (rc.4 staging, 100% reproducible on approval turns): after
resolving an exec-approval prompt_response, the adapter sends a short
ack ('Approved once'). _prompt_reply_metadata carried only placement
metadata (thread_id) — no per-turn identity, no interim marker — so
send()'s single-open-stream fallback (review B2) matched the approval
turn's OWN live draft and sealed it with the ack text. From there,
silently: every later append died on the post-seal tombstone (built for
millisecond stragglers, deliberately quiet), freezing the visible draft
mid-word; the turn-final found no open draft and fell through to a
plain send — the duplicate 'fallback' message. No suppression line, no
seal-failed warning: the log signature was pure absence.
Fix: _prompt_reply_metadata stamps _interim_send=True, which send()
already honors by bypassing draft matching. One source covers the whole
lifecycle class (approval ack, slash-confirm ack, prompt-expired
notice — all six call sites route through it).
Observability (the quiet parts, out loud):
- single-open-stream absorption now logs at INFO with the absorbed key;
- the FIRST post-seal tombstone swallow per draft key logs at WARNING
(bounded FIFO dedup) — one swallow is the normal straggler race, a
burst means a live stream was sealed mid-flight by someone else.
Tests (RED-first: both ack tests failed on the unfixed adapter at the
'draft still armed' assertion): approval ack leaves the open draft
armed and egresses as a plain send op; expiry notice same; regression
control pins the B2 contract — a real identity-less turn-final still
absorbs into its single open stream.
|
||
|
Victor Kyriazakos
|
5210dd48b8 |
fix(gateway+relay): approval prompts survive ambiguity without duplicates; streamed finals keep block formatting
Three live findings from rc.4 staging, all on the relay-fronted Slack path, all with the failure observed in live logs before the fix: 1. Approval-send timeout is AMBIGUOUS, not failed (no re-ask). send_exec_approval through the connector can time out with the card already rendered — the connector may ack after the deadline (slow platform API call, transient backpressure, event-loop stall) — and the timeout-as-failure path re-sent and produced duplicate cards. The outcome is now tri-state: sent / failed / ambiguous. Ambiguous = no re-send, no text fallback; the prompt registration stays armed so a late tap still resolves. Only a definite send error falls back to text. 2. pending_approval tool results forbid re-issuing the command. With one card correctly armed, the agent could still mint a SECOND card by re-running a rephrased variant of the gated command after reading the pending_approval tool result (observed live: same command re-issued in a different form, two cards). The tool message now instructs: do not re-run/rephrase; wait or report pending. Applied to both the terminal and execute_code arms. 3. Draft interim AND seal frames carry format_hints. format_hints are stamped on send, edit, and send_for_platform, but both draft-frame builders (send_draft interim + _seal_open_draft seal) shipped bare metadata. A streamed final therefore arrived at the connector hintless and sealed as a plain code block while non-streamed sends rendered native markdown blocks (observed live: language-tagged block on send/edit, downgrade on streamed seal). Both sites now stamp _with_format_hints_for_chat (destination-resolved, same pattern as the existing lanes). Verified live after the fix against the platform's stored message payload: rich_text_preformatted with language field on a streamed seal. Tests: tri-state outcome unit tests (5), draft/seal hint stamping + knobs- off regression control (2, RED-first), existing format-hints suite intact (14/14). Mutation-verified: reverting the adapter hunk sends test_draft_interim_and_seal_frames_carry_hints red; restore -> green. Boundary sweep (text egress lanes crossing the frame contract): send ✓ (pre-existing) edit ✓ (pre-existing) send_for_platform ✓ (pre-existing) draft-interim ✓ (this PR) draft-seal ✓ (this PR); task_card lane carries no text content — exempt. |
||
|
Ben Barclay
|
a1a1bee5c2 |
Merge branch 'main' into feat/relay-slack-parity
One conflict, gateway/relay/adapter.py send_for_platform: main added the turn-final draft-seal interception (_sfp_metadata with the _interim_send marker stripped, seal-or-fall-through); this branch added format-hint stamping on the same frame. COMPOSED: the plain-send frame now stamps _with_format_hints_for_platform over _sfp_metadata (the stripped copy), so both the seal fall-through contract and the cron-lane block hints hold. Note: the seal frame itself (op:draft final) does not stamp hints — cron sends are never open drafts, so the flagship path is unaffected; noted as a connector-PR follow-up for streamed interactive finals. |
||
|
Ben Barclay
|
162b23c3e2 |
fix(relay): D6 in_channel capability gate resolves the destination platform's descriptor
RelayAdapter.supports_inchannel_continuable is a scalar adopted from the PRIMARY identity's handshake descriptor, but one RelayAdapter fronts N platforms and the connector advertises the bit per platform. Reading the scalar for every logical platform both leaked a Slack-primary True onto other fronted platforms (activating the flat surface their descriptor never advertised) and suppressed a non-primary platform's advertised True (forcing thread mode on capable Slack behind a Discord primary). Add supports_inchannel_continuable_for_platform(platform): resolves the platform's own negotiated descriptor via descriptor_for_platform (the same Phase 1.5 seam max_message_length uses), scalar fallback only when the per-platform descriptor is unavailable. The scheduler's D6 gate prefers the query when the adapter provides it; native adapters keep the class-attribute path byte-identically. Tests: two-platform descriptor matrix (primary-True no-leak, non-primary-True honored, unknown-platform scalar fallback). |
||
|
Ben Barclay
|
79c39025c0 |
fix(relay): format hints resolve the DESTINATION platform, and stamp on send_for_platform
Two gaps in the block-formatting hint stamping: 1. Wrong descriptor: _format_hints gated on self.descriptor — the PRIMARY identity's scalar — while one RelayAdapter fronts N platforms. A Slack-primary adapter stamped Slack hints onto known Discord chats; a Discord-primary adapter suppressed hints for Slack chats whose own negotiated descriptor advertised the bit. Resolve per destination: send/edit use _descriptor_for_chat (the same seam max_message_length already uses) plus the chat's logical platform for the config sub-block; the knob lookup is now per-logical-platform (platforms.relay.extra.<platform>.*) instead of hardwired to slack. 2. Missing lane: send_for_platform — the scheduled/persisted-home lane (gateway/delivery.py), i.e. the CRON delivery path, the flagship consumer of the in_channel brief — never stamped hints at all. Stamp there too, resolving descriptor_for_platform(logical) off the transport; the scalar descriptor is used only when it belongs to that exact platform (fail closed). Tests: Slack-primary/Discord-chat no-leak, Discord-primary/Slack-chat still-stamps, send_for_platform stamps for capable platform and stays clean for incapable — all against a two-platform negotiated-descriptor transport. Existing single-platform suite unchanged and green. |
||
|
Ben Barclay
|
141af4febf |
Merge branch 'main' into relay-ws-hardening
#85796 (live-cards gateway half) landed on main and touches the same two files. Resolutions: - adapter.py __init__: union — the dedupe seen-set and the live-cards draft/seal caches are independent sibling attributes. - ws_transport.py _request_response: keep main's ambiguous-timeout contract AND this branch's raising-write catch, composed: a raise from the WRITE itself means the frame never reached the wire (definite non-delivery, no flag), while a failure surfaced after the frame was sent carries ambiguous=True like the timeout — tracked via a frame_sent marker. |
||
|
victor-kyriazakos
|
3683e70043 |
feat(relay): live-card ops — native draft streaming + task cards over the relay (gateway half) (#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 #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 (#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>
|
||
|
Ben Barclay
|
c60a525380 |
fix(relay): dedupe key platform component is spelling-invariant
The key derived its platform component with getattr(platform, 'value', ''), which handles the Platform enum the wire decoder always produces but collapses a plain-string platform — or a missing one — to the same empty string. Two DIFFERENT string platforms would then share one key component (cross-platform id collisions conflate), and enum vs string spellings of the SAME platform would produce two keys (a replay decoded differently would not dedupe). Inert on today's wire path (the decoder canonicalizes unknowns to Platform.RELAY), but alternate event constructors carry strings, so normalize at the key: enum value when present, the string itself otherwise, empty only when there is genuinely no platform. Missing platform intentionally still yields a key — fail-open on identity is reserved for missing message/chat ids. Tests pin all three properties; the spelling-invariance pair fails against the previous expression. |
||
|
Ben Barclay
|
22dfdc1c56 |
fix(relay): dedupe key reads chat identity from event.source, keyed per platform
The dedupe key read event.chat_id — a field MessageEvent does not have (chat identity lives on event.source.chat_id; see how every other read in this adapter resolves it). getattr defaulted to None, so _inbound_dedupe_key returned None for EVERY production event: the fail-open branch always taken, the seen-set permanently empty, and a replayed inbound still re-ran the whole turn. The tests passed because their SimpleNamespace events carried a top-level chat_id no production code path produces. Read the chat id from event.source, and join the underlying platform into the key: one relay adapter fronts several platforms (Phase 1.5 multiplex), and two platforms' numeric chat/message ids must not collide into one replay identity. The test event factory now builds real MessageEvent/SessionSource objects in the wire decoder's shape, so the replay and bounded-set tests fail against the broken key instead of green-lighting it. |
||
|
Victor Kyriazakos
|
31a4b8503d |
feat(relay): block-formatting hints on relay text egress (rich/markdown blocks)
Field report (enterprise side-by-side, 2026-08-18, finding 2): identical agent output renders native rich_text lists, Block Kit tables, and highlighted code on native Slack, but literal '-' bullets and code-fence tables on the relay lane. Native reads platforms.slack.extra.rich_blocks / markdown_blocks and renders Block Kit locally; relay frames carried no formatting signal, so the connector had no way to know the operator wants block rendering. Contract (additive, v1): the connector advertises supports_block_formatting in its capability descriptor. When it does AND the operator enables platforms.relay.extra.slack.rich_blocks / markdown_blocks (same per-platform sub-block and same _coerce_flag semantics as the other relay Slack knobs), the gateway stamps format_hints into outbound metadata on BOTH text egress lanes — send and edit (a streamed reply's final edit carries the finished markdown, so it must signal too or streams seal as plain text). The connector renders blocks and keeps plain text as the fallback. Old connector: never advertises -> no dead metadata ever sent. Old gateway: never stamps -> connector renders plain text as today. Knobs default OFF, matching native's opt-in posture. 8 new tests: descriptor default/from_json, hint stamping (capable+enabled), capability-absent suppression, knobs-off suppression, YAML-quoted-false coercion, partial knobs, edit-lane parity. |
||
|
Victor Kyriazakos
|
85b89451f6 |
feat(relay): flat in_channel continuable cron surface on the relay lane
Field report (enterprise side-by-side, 2026-08-18, finding 1 — the relay-only blocker): on relay-fronted Slack, cron briefs always deliver into a dedicated thread; the flat continuable surface (cron_continuable_surface: in_channel) that native Slack supports is inert, so plain DM replies never continue the job and the main conversation never sees the brief. Three gaps closed: - CapabilityDescriptor gains supports_inchannel_continuable (default False, additive within contract_version 1; from_json ignores it from old connectors, old gateways filter it as unknown). The connector advertises it per platform at handshake. - RelayAdapter maps the bit onto the adapter capability surface in both the constructor and _apply_descriptor (renegotiation), so the scheduler's D6 fail-safe gate sees it exactly like native Slack's class attribute. - _resolve_cron_surface_mode replaces the scheduler's inline flat-key read: native keeps the shipped flat shape; the relay lane reads the same per-logical-platform sub-block as the documented relay Slack knobs (platforms.relay.extra.slack.cron_continuable_surface), sub-block wins, scoped so a slack block cannot leak onto other fronted platforms. The seed path needs no changes: RelayAdapter inherits set_session_store (wired by the generic adapter boot loop) and _seed_cron_channel_session keys the flat session off the logical platform_name. 12 new tests: descriptor default/from_json/legacy-absence, adapter mapping constructor + renegotiation, and the surface-knob matrix (native flat key, relay sub-block, per-platform scoping, precedence, defaults). |
||
|
Victor Kyriazakos
|
b5f95d0890 |
fix(relay): drop replayed inbounds — bounded dedupe on (chat_id, message_id)
Live-canary finding #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. Transplanted-from: victor-fork/feat/relay-slack-live-cards@73ce04ae75 (extracted for the rc.4 relay-fixes train; tests moved to a standalone file with no live-cards dependencies) |
||
|
Ben Barclay
|
333536e7c9 |
fix(relay): stamp logical platform + relay trust on Discord interaction events (#84318)
The relay interactions passthrough lane (_discord_interaction_to_event) built its SessionSource with platform=Platform.RELAY and no delivered_via_upstream_relay marker — unlike the relay text lane (ws_transport._event_from_wire), which maps the connector's platform to the logical enum and stamps the authenticated-upstream flag. Consequences of the mismatch: - /sethome sent as a Discord slash command persisted the home channel under platforms.relay.home_channel (invisible to cron delivery, which looks up the logical platform) and mirrored it into the dead RELAY_HOME_CHANNEL env var — so cron jobs with deliver='discord' kept falling back to local-only even after the resolution/delivery fixes. The absent trust marker also meant via_relay=False, so the handler's 'Relay does not authenticate this logical home target' guard — designed to reject exactly this misfiled shape — never engaged. - Session keys forked: the connector binds the interaction's follow-up capability under buildSessionKey with platform 'discord' and chat_type 'group' (interactionSessionSource), while the gateway keyed the same interaction as relay/channel. - _capture_scope skipped recording _platform_by_chat (it ignores the generic 'relay'), losing the egress sender hint for the chat. Stamp Platform.DISCORD (the lane statically parses Discord interaction wire payloads), chat_type 'group' for guild channels (native-adapter and connector parity), and delivered_via_upstream_relay=True (parity with the text lane; set locally, never read off the wire). With this, slash-command /sethome files under platforms.discord and passes the via_relay guard legitimately, and cron delivery over relay works end to end with the #84300 resolution fixes. |
||
|
Ben Barclay
|
a189286aea |
fix(relay): stop sibling gateways answering another instance's button press (#83677)
* fix(relay): stop sibling gateways answering another instance's button press
A Discord button press arrives on the passthrough plane, and the connector
fans a passthrough forward out to EVERY live gateway session of the tenant
(relayServer.routeBusMessage delivers `passthrough` via sessionsByTenant),
unlike a message, which it narrows to the admitted instance set. The prompt
went out from exactly one instance and _pending_prompts is process-local, so
every sibling gateway saw an answer for a prompt it never minted, could not
tell that from its own prompt expiring, and fell through to chat dispatch --
where the option-shaped text ("/c1") is not a real command and run.py replied
"Unknown command `/c1`". One copy per sibling, under the single real ack.
Prompt ids are now minted as `<per-process nonce>.<8 hex>`, so an answer can
be attributed to the process that minted it. A prompt answer is always
consumed, never re-dispatched as chat: a sibling's prompt and a repeat answer
are both dropped silently, and an expired prompt of our own gets a short
"no longer waiting" notice from the owning gateway only.
Ids stay inside the connector codec's contract ([A-Za-z0-9_.-], <=32 chars,
64-byte callback budget -- verified against promptCodec.ts: 52 bytes worst
case with a full-length option id). An id with no nonce segment (a prompt in
flight across an in-place upgrade) is still treated as ours.
Tests: 4 added, each verified to fail without the fix. Full relay suite green
(160 tests).
* style(tests): ruff-format the added relay prompt tests
|
||
|
victor-kyriazakos
|
6a7cf19302 |
fix(gateway,relay): stop frozen-preview finals and dropped idle-session delegation callbacks (#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, #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 — #71643 (stale
finalize snapshot), #78541 (payload-less multi-message split), #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 #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 #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
|
||
|
Brooklyn Nicholson
|
34577fcb03 |
fix(gateway): rename a Discord thread once, after the reply lands
Titling is two-stage — a slice of the user's own words lands inline, the model's version replaces it a second later — and the platform rename lanes fired on both. That is two rate-limited calls to reach one name, and Discord allows two channel renames per ten minutes, so the throwaway could be the one that survived. The callback now carries which stage it is, and the lanes take the model's. The relay lane also asked where the reply landed at title time, which is before the model has answered: it polled the send-result cache for ten seconds and read the timeout as "never auto-threaded", so any turn with tool calls in it silently kept its raw thread name. Wait on the send itself instead — the adapter already owns that cache, so it can say when a reply arrives and, just as usefully, that one arrived carrying nothing. |
||
|
Dannyzen
|
3b0bb3b8bb |
fix(gateway): keep event loop alive during /compress and Relay drain
Offload manual /compress temporary-agent cleanup through the existing bounded off-loop helper so a slow agent.close() cannot freeze the gateway event loop, heartbeat, or platform polling. Guarantee Relay transport teardown even when the runner cancels adapter.disconnect() during go_idle: shielded finally, 2s drain-path idle ACK budget under the 5s outer disconnect budget, and bounded supervisor/reader/ws.close awaits. Original commits: - fix(gateway): offload manual /compress cleanup from the event loop - fix(gateway): tear down Relay transport even if go_idle is cancelled - fix(gateway): keep Relay disconnect budgets inside the runner window By @Dannyzen (PR #78027), salvaged onto current main. |
||
|
Ben Barclay
|
fed098bbf0 |
fix(gateway): use connector-owned no-clobber guard for relay thread rename + trace logs (#75912)
Live staging (2026-08-01): relay semantic thread rename still declined silently despite both #74482 and #75581 deployed — thread kept its initial-words name, session title generated fine. Root cause is the no-clobber guard string mismatch (see paired gateway-gateway PR): the gateway can't reproduce the thread's initial name byte-for-byte, so the connector's only_if_current_name check always failed. - relay rename lane now passes prefer_connector_created=True instead of the fragile initial-name string; the connector resolves the guard from its own created-name memory. Native-marker lane keeps the legacy only_if_current_name string (source carries the real initial name). - rename_thread: prefer_connector_created param -> only_if_connector_created on the wire, precedence over the legacy string. - INFO logs at rename dispatch (thread/lane/new_title) and result (applied=bool): the whole failure hunt needed telemetry the gateway never emitted — this makes the outcome visible in fly logs. Tests: connector-guard wire shape + precedence over legacy string; the title-turn race test updated to assert the connector-owned guard. Relay suite 149 passed; ruff + footguns clean. |
||
|
Ben Barclay
|
d26983e485 |
fix(gateway): relay TTS attachments + semantic auto-thread rename on the title turn (#74482)
Two relay-lane bugs from live Discord staging testing (2026-07-29): 1. TTS audio never attached over relay (any platform, any lane). _history_media_paths_for_session excluded only the trailing assistant entry from the persisted transcript when building the delivered-media dedup set. The agent persists rows as it produces them, so THIS turn's text_to_speech tool result (media_tag JSON) was already in the transcript at delivery time — the fresh TTS path deduped against itself and extract_media's attachment was silently stripped (response_delivery_dropped for a MEDIA-tag-only reply; fly logs show the exact signature). Fix: exclude everything from the last USER message onward (the current turn); prior-turn dedup unchanged. Affects every platform adapter (native + relay) on the non-streaming delivery path — the streaming path passes explicit history and was unaffected. 2. Connector-auto-created threads never got the LLM session-title rename. The title fires on the FIRST exchange, whose source is the PARENT channel event — the thread didn't exist at ingest, so the Phase 4 auto-thread markers can't be present and _is_discord_auto_thread_lane never matches on the relay title turn (initial titles worked; semantic renames never happened; staging telemetry shows zero thread_rename ops ever sent). Fix: consume the connector's new send-result feedback (paired gateway-gateway PR — contract §SendResult thread_id/auto_thread_name, additive): RelayAdapter.send() caches (thread_id, initial_name) per chat (bounded 256), run.py's title-callback registration + rename lane read it back and pass initial_name as only_if_current_name so the human-rename-wins guard holds on the relay lane too. Native marker path unchanged; connectors that don't stamp the fields degrade to exactly the old behavior. Tests: 3 new (send-result feedback capture, absence, bound) in test_relay_threads.py; 3 new in test_history_media_current_turn.py (current-turn TTS not deduped, prior-turn still deduped, no-user-row fallback). Relay suite 144 passed. |
||
|
Ben Barclay
|
0dc293f4f7 |
fix(relay): coerce Slack behavior flags exactly as the native adapter does
Both relay Slack knobs read their value through bool(), while the native
adapter they mirror uses str(raw).strip().lower() in {"1","true","yes","on"}.
A YAML-quoted string diverges:
dm_top_level_threads_as_sessions: "false" → relay True, native False
Non-empty strings are truthy, so the escape hatch is silently ignored in
exactly the shape an operator writes to switch it OFF. reply_in_thread has the
same defect and gates reply placement, session keying and run.py's progress
resolver, so one quoted "false" misfires three ways.
Route both through a shared _coerce_flag mirroring native's predicate. Real
booleans pass through untouched; None falls back to the default. Contract §8
documents the accepted spellings.
Tests: both knobs parametrized over the true/false spellings native accepts,
plus the absent-key default.
|
||
|
Ben Barclay
|
33833e232e |
fix(relay): apply the Slack thread anchor on the media lane too
The DM thread-anchor contract was resolved only in send(). _send_media() —
backing send_image, send_image_file, send_voice, send_video and send_document
— passed reply_to straight to the frame and never touched metadata, so
attachments egressing through the same connector-side Slack sender got both
failure shapes this branch set out to remove:
flat mode → reply_to survives, the image threads UNDER the user's DM
message (the original reported symptom)
thread mode → no metadata.thread_id, and threadTs() never reads reply_to,
so the image lands in the home channel instead of the
per-message thread
Both are reachable: gateway/run.py delivers agent artifacts through
send_voice/send_document.
Extract the three steps that must always happen together (mode gate, mirrored
reply_to_message_id strip, metadata promotion) into
_apply_slack_thread_anchor and route BOTH lanes through it, so text and media
cannot drift again. The media lane copies caller metadata rather than mutating
it — these helpers are called in loops with a shared mapping.
Also fold send_typing/stop_typing's duplicated status-anchor blocks into
_with_status_thread_anchor. They had already drifted (stop_typing omitted the
platform check) and the clear must target the thread the heartbeat set or the
status line sticks until Slack's own timeout.
Tests: media lane pinned in both modes plus the channel and
no-caller-mutation cases; verified as real by reverting the fix and watching
them fail.
|
||
|
Victor Kyriazakos
|
dd866eef34 |
Merge remote-tracking branch 'origin/main' into feat/relay-slack-blockkit-native-parity
# Conflicts: # gateway/relay/adapter.py |
||
|
Victor Kyriazakos
|
277fc97a0a |
feat(relay): dm_top_level_threads_as_sessions escape hatch — native session-keying parity
Review finding: native gates per-message DM sessions behind platforms.slack.extra.dm_top_level_threads_as_sessions; the relay lane coupled session keying to reply_in_thread alone, so 'threaded replies + one rolling session' was expressible on native but not here. Adds the same knob to the relay subset (platforms.relay.extra.slack. dm_top_level_threads_as_sessions, default true = per-message sessions, unchanged behavior). false keeps thread-per-message reply placement but skips the session stamp — one rolling DM session, legacy steer posture. TDD: opt-out + default-unchanged tests written first. |
||
|
Victor Kyriazakos
|
3e628edeb9 |
docs(relay): replace internal QA-N tracker markers with behavior descriptions
Review finding: QA-1/3/5/6/7 are internal campaign tracker ids meaning nothing to future readers of this file. Comments now describe the behavior (status thread targeting, metadata-only threading, session-keying parity) instead of citing the tracker. Comment-only change. |
||
|
Victor Kyriazakos
|
09c4a1d349 |
refactor(relay): remove dead _strip_synthetic_dm_thread; pin the run.py anchor-suppression boundary
Review finding (2026-07-28): every path through _strip_synthetic_dm_thread returned metadata unmodified — the actual strip was removed when prompts switched to trusting the run.py thread stamp, leaving a 50-line no-op and four tests that passed against it (verified by reviewer's negative control). - delete the function + its _send_prompt call site (verbatim pass-through with a pointer comment to the single mode authority) - rewrite the three pass-through tests as end-to-end placement contracts (forward run.py's stamp untouched) - NEW boundary tests pinning run.py._resolve_progress_thread_id itself: flat mode suppresses the synthetic self-anchor / preserves real threads; thread mode keeps the first-turn self-anchor. This is the cross-module coupling the review flagged as unpinned — if the upstream suppression regresses, these fail instead of prompts silently threading. |
||
|
Victor Kyriazakos
|
85a75f3155 |
refactor(relay): drop the flat_dm_status knob — liveliness is unconditional; add relay docs page
flat_dm_status was speculative config (rubric violation): no user wants 'make my agent look dead', and the only real consumer of status suppression was native's placement-contamination guard — which the relay lane handles structurally (QA-6/7 send-side anchor strip, leak-guard test), not via preference. Status now anchors whenever an inbound ts exists, in both modes. Docs: new website/docs/user-guide/messaging/relay.md — enterprise-only relay lane page documenting the platforms.relay.extra.<platform> subset shape (nested wins, flat fallback), the Slack reply_in_thread control, and always-on liveliness. Kept out of the native slack.md on purpose: relay controls are not Slack config. |
||
|
Victor Kyriazakos
|
9864e00fb4 |
feat(relay): flat-DM liveliness — status anchors to the triggering ts, replies stay flat (QA-8)
Victor's correction: flat DMs CAN have a live thinking status. setStatus on the triggering message's ts renders '… thinking'/per-tool phrases in that message's thread-footer space and clears without leaving a message artifact. Native suppresses this because ITS reply routing could inherit the activated thread; the relay lane's flat-mode sends strip their anchors explicitly (QA-6/7), so the status anchor cannot leak into reply placement — proven by the new leak-guard test. send_typing/stop_typing now anchor the status in flat mode too, gated by platforms.relay.extra.slack.flat_dm_status (default ON; false restores the fully anchorless posture). Thread mode unchanged. |
||
|
Victor Kyriazakos
|
71d5c47e21 |
fix(relay): per-message sessions for fronted Slack DMs — stamp the inbound ts as session thread (QA-3)
A 2nd top-level DM while a turn was in flight resolved to the SAME
session key and steered the running turn ('Redirected current run')
instead of starting its own. Native SlackAdapter stamps thread_ts =
event.thread_ts or ts on EVERY inbound, so build_session_key isolates
each top-level message; the connector normalizes top-level messages with
thread_id=null and the relay lane never reproduced the stamp.
_stamp_slack_session_thread applies native parity on the inbound bridge:
top-level Slack message + thread-per-message mode => source.thread_id =
its own ts (fresh session, parallel turns). Real thread replies and flat
mode untouched (flat keeps the shared rolling DM session on purpose).
Also introduces the enterprise config shape for relay-fronted Slack:
platforms.relay.extra.slack.<subset of native Slack fields> (nested
object wins; legacy flat extra.reply_in_thread still honoured). All
reply_in_thread reads (send/typing/stop_typing/run.py progress) now
route through one resolver.
|
||
|
Victor Kyriazakos
|
a51a17ebe3 |
fix(relay): promote the surviving reply_to anchor into metadata.thread_id on Slack sends (QA-7)
The connector's Slack sender threads on metadata ONLY: threadTs() reads metadata.thread_id/thread_ts and never the frame's reply_to. base.py's final-reply lane (and its stream-fallback 'first response' resend) builds metadata from source.thread_id — None for a top-level DM — so its sends carried reply_to as the sole threading signal and posted to the home channel (2026-07-27 post-approval report; the 15:17:03 frame showed meta_keys=['notify','user_id']). After the QA-6 mode gate keeps the anchor, copy it into metadata.thread_id so the wire carries the signal where the connector reads it. Flat mode unaffected (anchor already nulled); explicit thread metadata wins; non-Slack untouched. |
||
|
Victor Kyriazakos
|
467534b43e |
fix(relay): typing/status targets the per-message thread — synthesize the anchor from the inbound ts (QA-1)
Slack's thinking-status line (thread replies footer, plain chat:write — no assistant scopes needed) is thread-only: the connector's typing case no-ops without thread_ts. The typing lane's metadata has no anchor for a top-level DM (base.py builds from source.thread_id = None), so every status heartbeat was silently dropped — the trace showed typing frames with meta_keys=['user_id'] only. Cache the triggering message ts per chat on inbound (_capture_scope) and synthesize metadata.thread_id on send_typing/stop_typing in thread-per-message mode, mirroring native send_typing's _resolve_thread_ts(metadata.message_id). Flat mode unchanged (#18859); real-thread metadata wins over the cache; the clear frame targets the same synthesized thread so the status never sticks. |