Skip to content

fix(messaging): bind completion delivery to the run's own destination - #746

Merged
JSap0914 merged 4 commits into
devfrom
codex/slack-request-scoped-destination
Sep 11, 2026
Merged

fix(messaging): bind completion delivery to the run's own destination#746
JSap0914 merged 4 commits into
devfrom
codex/slack-request-scoped-destination

Conversation

@JSap0914

Copy link
Copy Markdown
Collaborator

A Slack mention landed in #ceo_lead_align_input at 2026-09-11 20:10 KST while an unrelated web run was working. Three minutes later that web run's answer — an internal ReelBrain ticket summary — was posted into the mention's thread. The run had never been addressed to Slack at all.

What was wrong

A channel forwarder answered "where does this answer go?" at send time, by reading the per-channel last-active slot:

const target = opts.getLastTarget();   // getLastActiveTarget('slack')

That slot holds whoever spoke to the bot most recently. The mention overwrote it (src/slack/bot.ts:1508), the web run exited, and its agent_done — which carried origin and text but no destination — followed the slot into a conversation that had asked something else. The DB shows the split plainly: the posted body is stored under the web session b25ca0b8, not the mention session 6c6e4301.

Heartbeat jobs had the same shape one layer down. A job with no destination fell through to sendChannelOutput({channel:'active'}); a Slack destination with a channel but no thread posted to the channel root, indistinguishable from a form nobody finished; an empty-string threadId passed validation and then read as falsy at send time. #437 closed the malformed case and left the rest.

And authorizeExplicitTarget vouched for an explicitly addressed channel-root send by returning the last-active target — thread included. A caller that named its destination correctly still had it moved.

What this does

The destination travels with the run. src/messaging/run-pin.ts captures origin, requestId, scope, sessionId, remoteKey and the admitted target once at spawn, and every agent_done carries it. resolveForwarderTarget reads the destination off the event and returns null when it is absent or names another channel; null means do not send. getLastTarget/getLastChatId are removed from all three forwarder option types so the lookup cannot come back.

Heartbeat destinations are complete or held. Complete means a thread, or an explicit scope: "channel_root". Held means the tick logs a reason and sends nothing, GET /api/heartbeat surfaces it, and PUT refuses to write a new half-filled Slack destination while still inheriting an existing one. Authorization now decides whether a send is allowed without rewriting where it lands.

Behaviour this removes

Web and CLI turns are no longer mirrored into chat rooms. That was the forwarder's stated purpose, and it is the bug: "the conversation the user is watching" was only ever a guess from a global slot any concurrent conversation could move. A turn with no remote destination is not homeless — its answer is already on the surface that asked for it. Affects Classic/web answers appearing in Slack, terminal jaw results appearing in Slack, Discord forwardAll, and the Telegram getLastChatId fallback.

Live heartbeat jobs with an incomplete destination stop delivering until migrated. On this install five of nine jobs name a channel with no thread, one of them enabled (hb_daily_scrum_morning). They need scope: "channel_root" or a thread in heartbeat.json before this ships, or the next run is silent. Mention-watch jobs are unaffected — they answer the thread they find.

Testing

root,unit = 13458 tests, 13436 pass. Two failures (P13 TERM-ignoring owned handles, Pi model discovery) pass in isolation on this branch and on the base commit; they are load-sensitive under full-suite concurrency, not caused by this change. npm run gate:all — 23/23.

New: tests/unit/heartbeat-destination-binding.test.ts (HDB-001..008). Updated to the new contract: runtime-messaging-isolation (a target-less final now reaches no channel; a pinned one reaches exactly one), heartbeat-runner-modes, heartbeat-file, send-validation, and the Slack/Discord/Telegram forwarder fixtures, which now put the destination on the event instead of supplying it through a lookup.

Also repairs codex-app-multiplex-spawn, which was already failing on dev: its partial namedExports mock of runtime/events.js replaces the whole module, so projection.ts could not import recordRuntimeProjectionLoss and the file threw before its first assertion.

Not in this PR

#743's fail-closed completion matcher. matchesSlackReply still treats an absent field as agreement, and flipping it before request_settled, queued_run_started and queue_update carry the same identity would silence #655 steer tracking and #407 boot drain. This PR ships the additive half — every agent_done now carries the full pin — which is that change's precondition.

The progress-card retry storm is #744, sent separately.

Refs #742 #745 #437

suji lee added 4 commits September 11, 2026 21:29
A channel forwarder used to answer 'where does this answer go?' by reading the
per-channel last-active slot at send time. That slot belongs to whoever spoke
most recently, not to the run that is finishing. On 2026-09-11 a mention landed
in #ceo_lead_align_input while an unrelated web run was working; the mention
moved the slot, the web run exited, and its internal ticket summary was posted
into that thread (#742).

The destination now travels with the run. `runPinFields` captures origin,
requestId, scope, sessionId, remoteKey and the admitted target once at spawn,
and every agent_done terminal carries it. Slack, Discord and Telegram forwarders
resolve the destination from that payload and send nowhere when it is absent or
addressed to another channel; `getLastTarget`/`getLastChatId` are gone from
their options so the lookup cannot come back.

This removes web/CLI mirroring into chat channels. A turn with no remote
destination is not homeless - its answer is already on the surface the user is
looking at, and posting it to a room was always a guess about which room.

Refs #742 #743
…sation

A heartbeat job used to have three ways of not saying where its report goes, and
all three delivered anyway. No destination fell through to the active channel.
A Slack destination with a channel but no thread posted to the channel root,
indistinguishable from a form nobody finished. An empty-string threadId passed
validation and then read as falsy at send time. #437 closed the malformed case
and left the rest (#745).

A destination is now complete or held. Complete means a thread, or an explicit
scope:'channel_root' saying the channel itself is the audience. Held means the
tick logs a reason and sends nothing; GET /api/heartbeat surfaces it so an
operator sees the hold instead of waiting for a report that never comes. PUT
refuses to write a new half-filled Slack destination while still inheriting an
existing one, so the gap closes as jobs are edited rather than being re-saved
forever with no thread.

Also fixes a separate rewrite in the same class: authorizeExplicitTarget vouched
for an explicitly addressed channel-root send by returning the last-active
target, thread and all. Authorization decides whether a send is allowed, not
where it lands; with no configured allowlist that turned a correctly addressed
channel post into a reply inside whichever thread had spoken most recently.

Refs #745 #437
The forwarder fixtures supplied a destination through getLastTarget and left it
off the event, which is the shape that produced the misroute. They now put the
target on the agent_done payload, or on the run's spawn opts where a real
lifecycle produces the terminal.

Also repairs codex-app-multiplex-spawn, which was already failing on dev: its
partial namedExports mock of runtime/events.js replaces the whole module, so
projection.ts could not import recordRuntimeProjectionLoss and the file threw
before its first assertion.
AGENTS.md, CLAUDE.md and structure/telegram.md now state where a completion
event's destination comes from, and that a run with none reaches no channel.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 89650401-b43c-4f89-bc2a-a10a132eee7f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant