feat(relay): flat in_channel continuable crons + block-formatting hints on the relay lane - #90038
Merged
benbarclay merged 20 commits intoAug 20, 2026
Merged
Conversation
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).
…wn 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.
alt-glitch
added
type/feature
…ontract §2 table test_contract_doc_conformance enforces that every CapabilityDescriptor field appears in docs/relay-connector-contract.md §2 so connector authors mirror the full surface — caught in CI (slice 2/12); the descriptor gained supports_inchannel_continuable and supports_block_formatting without the doc rows.
…ror opt-in Live regression (Alice, 2026-08-19): a continuable cron with cron_continuable_surface=in_channel delivered its brief flat, but the flat-session seed was gated on mirror_this_target = mirror_enabled AND origin-match. Without attach_to_session (and with cron.mirror_delivery defaulting False) the seed never ran; the next plain reply resolved to a blank (slack, chat, None) session and the agent had no idea about its own delivery message — not continuable in channel OR thread (in_channel mode correctly skips thread creation, so there was no thread session either). in_channel IS the continuation surface, not a mirror nicety: gate the seed on origin-match alone, and resolve origin_user_id for any origin-matching target so the seeded key still carries the scheduling user on per-user-isolated chats. attach_to_session remains the opt-in for the separate default-surface mirror behavior. Regression test drives the delivery path with attach_to_session=False and asserts the seed fires with the right user_id (fails on pre-fix code).
Seed failure was logger.debug — invisible in production while being the exact 'agent has no idea about its own brief' symptom. WARNING on: seed exception (with reason), seed returning False, and in_channel delivery to a non-origin target.
Two live failures from the Alice canary (2026-08-19, jobs 28a24afebd81 / 83b93f8be379), both leaving a continuable in_channel cron with amnesia: 1. Seed mirrored via origin heuristics and silently dropped the brief. _seed_cron_channel_session created the flat session row, then mirror_to_session RE-DISCOVERED the target via find_session_by_origin — whose multi-candidate bail-out returns None on a populated chat (flat session + N per-message thread sessions sharing one chat_id, mixed user_ids). Receipt: 'in_channel seed did NOT land on slack:D0BJTDCSR7C'. mirror_to_session now accepts an explicit session_id and both cron seeds pass the exact row they just created; origin-scan remains the fallback for callers that genuinely don't know the target. 2. The brief's OWN THREAD was never seeded. in_channel delivers flat, but a flat Slack message still invites a thread reply (the natural mobile affordance — exactly what the user did). That reply keys to (chat, thread=<brief ts>), which no seed touched. The delivery's message_id now anchors a companion _seed_cron_thread_session so BOTH reply surfaces (plain channel message AND in-thread reply) continue the job. Also: thread-seed failures upgraded debug→WARNING (silent seed failure IS the user-facing bug), and the thread seed reports landed/not-landed. Regression tests drive both against the live failure shapes: exact-session mirror asserted via session_id kwarg; thread companion asserted via the SendResult message_id anchor. Clean-fixture blind spot noted: the E2E harness used a fresh store with one row, which is why heuristic rediscovery looked fine pre-production.
The 19:53 canary run failed the in_channel seed EVEN WITH the deterministic session_id fix live, and the mirror's two failure paths (no-session bail, append exception) both logged at debug — invisible in production. WARNING both, with the explicit session id in the exception path, so the next run names the failing branch instead of another blind retest.
Continuability is explicit via attach_to_session or cron.mirror_delivery; those knobs control transcript mirroring for the ordinary thread/default surface. However, the in_channel surface must still receive its delivery text to seed the continuation session. Previously mirror_text was populated only when the optional mirror knob was enabled, so in_channel jobs created with the default false settings passed an empty string to the seed helper, which returned False. The live symptom was a delivered cron message with no continuation context; Alice reproduced it three times (latest ef7bd2869d15). Keep cleaned delivery text available for continuable surface seeding while retaining mirror_enabled for the separate _maybe_mirror_cron_delivery path. Targeted cron/in_channel regression suite: 1008 passed, 1 skipped.
… seed row never matched the DM reply's key Live incident (Alice canary 2026-08-20, job 8e21a957b77b): the continuable thread seed created its session with chat_type='thread', but a Slack DM in-thread reply arrives chat_type='dm' and build_session_key routes DM threads through the DM arm (...:dm:<chat>:<thread>). Seed row and reply row never matched — the reply had no brief in context (continuation amnesia). is_dm on _seed_cron_thread_session selects the seeded chat_type at both call sites (opened-thread and the companion in_channel thread seed); channel threads are unchanged. Sibling lane of the flat seed's is_dm fix. Tests pin the key-equality contract: seeded key == the key the reply builds.
Contributor
Author
|
Pushed 6 commits from two nights of live canary testing on the enterprise staging sandbox (all TDD red-first, cron suite 811 passed on this branch): Continuable-cron seed correctness (the feature this PR introduces, hardened against every live failure we hit):
Live-verified on the Alice canary 2026-08-20: threaded brief → in-thread reply quotes the brief back. Full B-matrix pass (surface × mirror × attach) runs on the fleet next. |
build_session_key embeds the workspace segment (scope_id) in every Slack dm/group/thread key, but both cron seed helpers built their SessionSource without it: the seeded row keyed agent:main:slack:dm:<chat>:<thread> while a real scoped reply keys agent:main:slack:dm:<team>:<chat>:<thread> — a row no reply ever resolves to. DMs were rescued only incidentally by the legacy-key claim-once migration; scoped channels/threads got continuation amnesia, and identical channel ids in two workspaces could collide. Capture HERMES_SESSION_SCOPE_ID into the cron origin (_origin_from_env — the session-context var async_delegation already snapshots), add scope_id to _seed_cron_thread_session/_seed_cron_channel_session, and pass the origin's scope at all three seed call sites. Tests: scoped dm-thread / channel-thread / flat-channel seed-vs-reply key equality through the real build_session_key, plus a two-workspace non-collision guard.
…get) The seed was decoupled from the mirror opt-in (in_channel is the continuation surface regardless of attach_to_session), but the thread-id-clearing gate above it still read mirror_this_target. With the advertised default config (attach_to_session=false, cron.mirror_delivery unset) and an origin carrying a real thread_id, the brief kept delivering INTO the origin thread while the flat (thread_id=None) session got seeded — brief and continuation surface in different places, so a plain reply never saw it. Flatten on the same gate as the seed: origin_target (with the existing live_adapter_ready guard). Fan-out/broadcast targets are unaffected. Test drives _deliver_result with a thread-carrying origin and default knobs, asserting on the routed DeliveryTarget.thread_id — RED on the old gate, GREEN now.
…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.
…atform'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).
_cron_mirror_delivery_enabled still promised 'cron deliveries live only in the cron job's own session' as the unconditional default, but the in_channel continuable surface now seeds the target session regardless of attach_to_session/cron.mirror_delivery (the seed IS the continuation feature, and in_channel is itself opt-in). State the carve-out where the guarantee is documented.
…allback nuance test_flat_key_wins_over_subblock asserted the OPPOSITE of its name (the sub-block wins, matching _relay_slack_extra). Rename to what it proves. Also note in _resolve_cron_surface_mode why its fallback differs from _relay_slack_extra's all-or-nothing sub-dict: the flat key is the legacy staging shape, and a flat knob applies to every fronted platform, gated only by the per-platform D6 capability check.
…ery metadata The seed-key fix made the SESSION scoped, but the delivery leg still dropped the scope: cron route_metadata carried only job_id (+thread), DeliveryRouter stamps scope_id only for the configured HOME channel, and the RelayAdapter's per-chat scope cache is cold after a gateway restart (learned from inbound only). A scoped Slack origin that is not the home chat therefore egressed with NO tenant discriminator, and the connector's fail-closed guard could reject the brief before delivery — the delivery-leg sibling of the seed-key scope gap. Copy origin.scope_id into the live text and media routing metadata for ORIGIN-MATCHING targets only (setdefault — never overrides router/home stamping). Fan-out/broadcast targets are excluded by the origin gate: a fan-out target's tenant is not the origin's, and a wrong scope is worse than none. Tests: restart-shaped positive (scoped non-home origin -> scope_id on routed metadata, RED before this fix) and legacy negative (scope-less origin stamps nothing).
…fallback with a real adapter shape Unspecced MagicMock/AsyncMock adapters fabricate supports_inchannel_continuable_for_platform as a truthy callable, so the scheduler's duck-typed D6 gate silently took the relay accessor branch in every in-channel test — the native scalar fallback the fixtures describe was never exercised, and setting supports_inchannel_continuable=False on a mock could not force thread mode. Pin the accessor to None on both mock fixtures (matching a real native adapter, which never defines the method), and add a fallback-boundary test with a real minimal adapter class: scalar False -> in_channel fails safe to thread, flat seed never fires.
…ually ran seed_mock.assert_not_called() alone could pass for the wrong reason — a harness failure before delivery also leaves the seed uncalled. Assert the real adapter recorded exactly one live send to the origin chat, so the test pins the D6 thread-fallback decision, not an accidental no-delivery.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two relay-lane parity gaps from an enterprise side-by-side evaluation of relay-fronted Slack against the native adapter (same box, same config, both lanes live), 2026-08-18. This PR is the gateway half of each; the connector half lands separately in gateway-gateway.
cron_continuable_surface: in_channel— brief posts flat in the DM, a plain reply continues the job — is inert on the relay lane. This is the remaining reason the customer keeps native Slack tokens on their boxes.-bullets and code-fence tables on relay. Native readsrich_blocks/markdown_blocksfrom config and renders locally; relay frames carried no formatting signal at all.Base: this branch builds on #89584 (relay transport hardening) and is stacked for merge after it.
Finding 1 — three gaps, three changes
CapabilityDescriptorgainssupports_inchannel_continuable(default False). The scheduler's existing fail-safe gate reads the adapter capability flag;RelayAdapterpreviously inherited the base-class False unconditionally, so the surface could never activate. The connector now advertises the bit per platform at handshake, andRelayAdapteradopts it in both the constructor and_apply_descriptor(renegotiation) — same treatment assupports_code_blocks._resolve_cron_surface_modereplaces the scheduler's inline config read. The scheduler resolvedplatforms.relayas the delivery config but read the surface knob only as a flat key — the relay lane's documented Slack knobs live underplatforms.relay.extra.slack.*(reply_in_thread,dm_top_level_threads_as_sessions), so operators had no working location for the knob. The helper reads the per-logical-platform sub-block first (matching_relay_slack_extraprecedence), falls back to the flat key, and scopes the sub-block so aslack:block cannot affect another fronted platform. Native configs are unchanged.RelayAdapterreceives the session store from the generic adapter boot loop, and_seed_cron_channel_sessionkeys the session off the logical platform name.Finding 2 — format hints on the wire
The connector owns the Slack API call on the relay lane, so the gateway cannot render blocks itself; it can only signal operator intent. New descriptor bit
supports_block_formatting; when the connector advertises it AND the operator enablesplatforms.relay.extra.slack.rich_blocks/markdown_blocks(same sub-block, same string-coercion semantics as the other relay Slack knobs), the gateway stampsformat_hintsinto outbound metadata on both text egress lanes —sendandedit. The edit lane matters because a streamed reply's final edit carries the finished markdown; without it, streams would seal as plain text.Compatibility: an old connector never advertises, so no hint metadata is ever sent; an old gateway never stamps, so a new connector renders plain text exactly as today. Knobs default off, matching native's opt-in posture. Both fields are additive within contract_version 1 (
from_jsonignores unknown keys in both directions).Validation
tests/relay/test_relay_inchannel_continuable.py(12 tests)tests/relay/test_relay_format_hints.py(8 tests)"false"coercion, partial knobs, edit-lane parity_format_hintsto always-None fails exactly the 3 stamping guards; restore returns greenConnector-half plan (separate gateway-gateway PR): Slack descriptor advertises both bits; the sender honors
format_hints.markdown_blocksvia Slack's nativemarkdownblock (Slack renders tables/lists/code itself, plain-text fallback kept);rich_blockshint is carried by the contract now so a connector-side rich-text renderer can land later without a wire change. Cross-repo E2E harness/driver pairs accompany the connector PR.Review-round addendum (Ben + sol-reviewer, 2026-08-20)
Three independent review rounds were run against this branch; ten commits landed on top of the original stack resolving every finding. All fixes verified per-boundary by execution (red-first where feasible), full relay+cron gates green via the canonical runner after each round.
Round 1 (head
fbf5eb8) — 6 commits8b6cf434scope_id—build_session_keyembeds the team segment in every Slack key, so seeded rows were unreachable from scoped replies (DMs rescued only incidentally by the legacy-key migration; scoped channels got continuation amnesia; identical channel ids could collide across workspaces)HERMES_SESSION_SCOPE_IDinto the cron origin;scope_idon both seed helpers and all three call sites; scoped-key + two-workspace non-collision tests through the real key builder20c56f82mirror_this_targetafter the seed was decoupled to origin-match — thread-origin briefs delivered into the thread while the flat session got seededorigin_target(same gate as the seed); test asserts the routedDeliveryTarget.thread_idis None, red-first79c39025send_for_platform— the cron delivery lane — never stamped hints at all_descriptor_for_chat,send_for_platformviadescriptor_for_platform(scalar fallback only for its own platform); knob lookup per-logical-platform; multi-platform test matrix162b23c3supports_inchannel_continuable_for_platform()resolving the destination's negotiated descriptor; scheduler prefers it when present; native adapters unchangeda78af23c_cron_mirror_delivery_enableddocstring still promised unconditional session isolation6cd1ed2etest_flat_key_wins_over_subblockasserted the opposite of its name_resolve_cron_surface_modevs_relay_slack_extraprecedence nuance documentedRound 2 (head
6cd1ed2) — 2 commits4308c453scope_idnever reached relay egress metadata: cronroute_metadatacarried onlyjob_id,DeliveryRouterstamps scope only for the configured home channel, and the relay's per-chat scope cache is cold after restart — a scoped non-home origin could be rejected by the connector's fail-closed tenant guard before deliveryorigin.scope_id(setdefault) onto origin-matching text + media routing metadata, never fan-out targets; restart-shaped positive test (red-first) + legacy-origin negative540237c9Noneon both mock fixtures; real minimal-adapter test proves scalarFalsefails safe to threadRound 3 (head
540237c) — 1 commitBoth round-2 fixes classified FIXED by execution (restart-shaped probe emitted the scoped frame through the real
DeliveryRouter/RelayAdapter); no new production defects. One test-fidelity nit fixed ascc85feea: the scalar-fallback test now also asserts the live send actually ran, so it pins the D6 decision rather than an accidental no-delivery.a1a1beemerges currentmain(conflict insend_for_platform: main's draft-seal interception composed with the format-hint stamping — hints now stamp over the interim-stripped_sfp_metadata; the seal frame itself doesn't stamp hints, which never affects cron sends and is noted for the connector PR's streamed-final work).Known follow-ups deferred to the connector-side PR: legacy flat-knob shape (
platforms.relay.extra.rich_blockswithout a sub-block) plus a per-platform descriptor miss inherits the primary descriptor's capability — same fallback convention asmax_message_length; scope note: origins persisted before this PR carry noscope_idand keep legacy scope-less seeding until the job is recreated.