Skip to content

fix(streaming): a stream goes where each publish went, and ends there - #585

Merged
nissa-seru merged 2 commits into
anima-research:mainfrom
nissa-seru:humphrey/f294-complete-where-it-began
Oct 19, 2026
Merged

nissa-seru merged 2 commits into
anima-research:mainfrom
nissa-seru:humphrey/f294-complete-where-it-began

Conversation

@nissa-seru

@nissa-seru nissa-seru commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

An MCPL outgoing stream now goes where each of its publishes went, that server's channel, and ends there with its channels/outgoing/complete. Until now it looked each channel up again by its bare id, for every chunk and for the complete. So a stream lost its complete when the channel's server declared the channel inbound-only mid-stream, or when a second server registered the same channel id. A consumer that finalizes on the complete, such as discord-mcpl's voice utterance, was left open. And when a channel id passed from one server to another mid-stream, the new server's complete carried the old server's words too.

Problem

The framework's stream kept its text per bare channel id, and the registry resolved that id again for each chunk and for the complete (streamingServerFor). That lookup finds no server when the channel is inbound-only (#350), or when two servers register the id (the never-guess rule). So a stream whose channel changed in either way between its first chunk and its end never sent its complete there. MCPL's SPEC §14.3 sends the complete "once per (inferenceId, channelId) that received chunks, on every inference exit path", and consumers use it "to finalize (settle a rendered message, end a synthesized utterance) and to reconcile any dropped chunks". discord-mcpl finalizes a voice utterance on it (VoiceOutput.handleComplete), its own hold timer aside.

The same lookup also dropped a later chunk of such a stream, for a publish delivered just before the declaration and streamed just after it.

And a bare id can pass between servers while a stream runs. Say server A's room takes the stream, then A removes room, B registers one, and the next round publishes to B. The complete then went to B with A's words joined to B's, since the text was kept per bare id.

Changes

  • The stream follows each publish's destination. routeSpeech returns the destination's server beside its channel, and SpeechPublished takes the destination. So each delivery site hands its stream the server and channel its publish went to. The stream's text is kept per destination, so each complete carries only what was published there, and two servers' channels with the same id are two destinations.
  • The registry sends a stream message to a destination while that server registers that channel and its grant has channels.streaming.
    • An inbound-only declaration doesn't unregister a channel. So a stream that started there goes on, with a later chunk for a publish delivered just before the declaration, and ends there with its complete.
    • No stream starts in an inbound-only channel, as fix(channels): the host posts nothing into a channel its server declared inbound-only #350 has it, not even for a publish delivered there just before the declaration. The registry records where each stream has started (takenStreams), for that rule and so the complete goes to no other destination. The stream's last complete removes the record.
    • A channel gets nothing of the stream while its server doesn't register it, as after the server removes it or is disconnected, until the server registers it again, or while the server isn't granted the stream, as during a reconnect, whose new transport starts with an empty grant until its policy is settled again. Its complete doesn't go then either, as on main. §14.3 names the recipient as "the server that registered the target channel".
    • A chunk whose send throws still started the stream, so its complete follows, for the consumer to reconcile what it missed.
  • One key per destination (destinationKey, a JSON array of the two ids) keys the stream's text and the registry's record, so no two destinations share a key, whatever their ids hold. The registry's channel map joins the two ids with a colon, as on main, so reading a destination's entry checks that it's that server's. That map holds one entry per joined key, on main as here, so it can't hold two such destinations at once. The stream keeps them apart when one is registered after the other.
  • sendOutgoingChunk and sendOutgoingComplete stay compatible. ChannelRegistry is exported and both are public, so each takes the destination, or, as before, a bare channel id, which is resolved at each call as it was and keeps no record: a stream sent by destination ends by destination.
  • The inference:published and inference:published_complete traces carry the destination's serverId beside its channelId, so an in-process consumer can tell two servers' channels apart. Both traces are new since 0.23.0.
  • Texts. The docs of SpeechPublished, isInboundOnly, both registry methods and both traces say so. The changelog fragment is changelog.d/stream-ends-where-it-began.fixed.md. fix(channels): the host posts nothing into a channel its server declared inbound-only #350's pending fragment now says that no outgoing stream starts in an inbound-only channel, and fix(streaming): the outgoing stream carries only what delivery publishes, and a script's calls count as its round's #360's pending hold-round-speech.added.md names the traces' serverId.

Who notices

  • A server that takes the outgoing stream, such as discord-mcpl with voice on:
    • it gets the complete of a stream that one of its channels took, when the server has since declared that channel inbound-only, and the later chunk of a publish delivered just before the declaration;
    • it gets only what was published to it when a channel id moves to it from another server mid-stream.
  • A channel the stream didn't start at gets no complete for it. On main, a complete could reach a channel whose chunks had all gone elsewhere or nowhere, by the lookup at the stream's end.
  • In-process consumers of the published traces get a serverId on each.
  • Callers of the registry's two methods with a bare channel id see no change. routeSpeech's answer gains serverId.

What each part is for

  • The destination on SpeechPublished, from routeSpeech's serverId and each publish outcome, is the fix. Only delivery knows where a publish went, and every later lookup by bare id can come out differently.
  • outgoingText keyed by destination keeps each server's words to that server's complete.
  • The registration check keeps stream messages to the server that registers the channel now. That check replaces the bare-id lookup, which refused an inbound-only channel and a shared id along with a missing one.
  • takenStreams, the record of where each stream has started, has two uses: it applies "no stream starts in an inbound-only channel" to a stream's first chunk there only, and it keeps the complete to destinations the stream started at. It holds one entry for each destination a running stream has started at. The record is made when the stream starts there, before the chunk's send, so a send that throws still gets its complete after it, for the consumer to reconcile what it missed.
    • The other place to keep it was the framework's stream, but that would need each method to answer whether it sent.
  • streamingServerAt() reads a destination's entry and grant, for both methods, beside main's streamingServerFor() for a bare id.
  • destinationKey and the server check keep two destinations whose ids join to the same string apart, when one is registered after the other.
  • Removing a stream's empty record keeps every finished stream from leaving a key behind for the life of the process.
  • The bare-id form of both methods keeps the two public methods working for an outside caller, unchanged.
  • The traces' serverId keeps a consumer from merging two servers' streams, and the traces aren't released yet.
  • The test fixtures:
    • makeStreamingRegistry in channel-registry-routing serves its eleven new tests: named connections, which a test can replace, whose sends can throw, and whose grant can lose the stream, plus a count of the records kept.
    • realRegistryTurn in outgoing-stream runs a turn through a real registry, with hooks inside the turn's tool call and inside each publish, so the stream's three framework tests there see what each server is sent.
    • The stream doubles in outgoing-stream, explicit-prose-routing, tool-result-guard and code-execution read the destination and check its server, so each delivery site's destination is checked. outgoing-stream's stub registry names a server per channel.
    • outgoing-stream's skip_reply and send tests check that the turn's inference ran, so they can't pass on a turn that never ran.
  • Cut along the way: a record, per stream and bare channel id, of the server its first chunk went to, which this change began with. Keyed by the bare id, it sent a later publish's chunk to the old server when the id moved, and it kept sending to a server whose channel was gone.

Tests

New tests:

  • In test/outgoing-stream.test.ts, three turns run through a real ChannelRegistry, with realRegistryTurn:

    • The inbound-only case. A server declares the locus inbound-only during the turn's tool call. The later text is refused, and the stream's complete carries first to that server.
    • The moving id. room passes from server a to server b during the call. b is streamed only second, its complete included, and a gets no complete. The inference:published* traces carry each destination's server.
    • A channel that moves as its publish is answered. a removes room and b registers one while a answers the publish. The chunk is a's, which no longer registers the channel, so nothing is streamed, and nothing to b. The traces still follow the publish's destination.
  • In test/outgoing-stream.test.ts, explicit routing in one turn sends first to server a's channel b:room and second to server a:b's channel room, whose ids would both join to a:b:room. Each is streamed and completed with its own words, second without a paragraph break.

  • In test/channel-registry-routing.test.ts, eleven registry tests, with makeStreamingRegistry:

    • a stream that goes on and ends in a channel declared inbound-only, while a new stream starts nothing there;
    • two servers' channels sharing an id as two destinations;
    • the id passing from one server to another;
    • two destinations whose ids join with a colon to the same string, registered one after the other;
    • a server disconnected and connected again mid-stream, which gets the rest only once it registers the channel again;
    • destinations a stream didn't start at, and the complete going once;
    • the grant losing channels.streaming;
    • two streams at one destination;
    • a send that throws;
    • the record's removal;
    • the bare-id form, sent as before to the one granted server registering the id, and nowhere when two servers register it, when the grant lacks the stream, or once it's inbound-only.

    The test of an inbound-only channel is renamed: no stream starts there, by destination or by bare id.

  • Changed:

    • outgoing-stream's stub registry returns routeSpeech's serverId, and names a server per channel. Its stream doubles log a mark for a destination without its channel's server, so each delivery site's destination is checked.
    • Its skip_reply and send tests check that the turn's inference ran.
    • The stream doubles in explicit-prose-routing, tool-result-guard and code-execution read the destination, and record a stream message for any server but the stub's with that server. code-execution's and explicit-prose-routing's routeSpeech stubs return the stub's server.
    • The routing tests that check routeSpeech's answer expect its serverId.

The whole suite, on the first commit's code before its last edit, which rewrapped three lines of a comment in src/types/trace.ts:

  • Where it ran: on Linux with Node 24.22.0 and with Node 20.20.2, each without --test-force-exit, every file exiting on its own. tsc --noEmit and the build are clean.
  • Results: 5,248 tests in 250 files, none failing. 27 are skipped on Node 24 and 28 on Node 20, all in workspace-path-spelling and, on Node 20, one in mcp-modern-outcomes, which this PR doesn't touch.
  • The files this PR changes: channel-registry-routing 63, outgoing-stream 57, explicit-prose-routing 103, tool-result-guard 146 and code-execution 142.
  • The second commit changes only the stream doubles and stubs in three test files. At the head, with a clean typecheck, code-execution passes 142, explicit-prose-routing 103 and tool-result-guard 146.

Red first, with main's source (10917f79) and the first commit's tests:

  • The three framework tests fail on behaviour:
    • in the inbound-only case, main sends body publish:first and body chunk:first, and no complete;
    • in the moving-id case, main streams b its chunk as \n\nsecond, and a complete of first\n\nsecond, which carries a's words;
    • when the channel moves as the publish is answered, main streams b the chunk first and a complete of first, words published only to a.
  • The explicit test of two destinations whose ids join to one string fails there only through the doubles, which main hands a bare channel id. main keys the stream by bare id, which tells those two apart, so the test guards this change's own key, as the removal of that key below shows.
  • Ten of the eleven registry tests fail there, since they call both methods with a destination, which main doesn't take. That shows nothing about behaviour on its own; the removals below show what each part needs. The bare-id test passes there, as a control.
  • Other tests fail on main only because of this PR's changes to them:
    • the routing tests that expect routeSpeech's serverId;
    • the stream tests whose doubles read the destination, since main's framework hands them a bare string. In outgoing-stream, that's every test that checks the stream.

Each part, removed or changed on its own, fails at least one test in channel-registry-routing and outgoing-stream, on the same code: 24 removals. The unchanged control passes 120 of 120, and every run counts 120.

  • The rules at a destination:
    • a complete sent to a destination the stream didn't start at;
    • a later chunk refused in a channel declared inbound-only, or the complete refused there;
    • a destination streamed though its server no longer registers the channel;
    • a destination's grant not read.
  • The record: nothing recorded, or a finished stream's empty record kept.
  • The keys: destination keys joined with a colon, the framework's stream text alone keyed by a colon join, or another server's channel entry taken for a destination.
  • The framework:
    • its stream text keyed by bare channel id, or its chunks or its complete sent by bare id;
    • routeSpeech without serverId, which the typecheck refuses and 12 tests fail;
    • the traces without serverId;
    • each of the six delivery sites handing its stream another server: the explicit envelope, the hybrid publish at the pin or locus, the hybrid >>> envelope, the mid-turn locus segment, the text-only segment and the trailing segment.
  • The bare-id form: a bare id's chunk sent without the lookup's rules, a bare id two servers register resolved, or a bare id's grant not read.

The doubles' server check, at the head, over code-execution, explicit-prose-routing and tool-result-guard, against a control that passes 391 of 391: each of these on its own fails at least one test there.

  • code-execution's or explicit-prose-routing's routeSpeech stub giving no server;
  • tool-result-guard's deliverSpeech stub giving a delivery another server;
  • the mid-turn locus segment handing its stream another server, which code-execution now catches.

CI on this head merged with main 593f5e71 (run 39301658374) passes all four Build & Test legs, Ubuntu and macOS each with Node 20 and 24, and the packed install. The changelog check passes too (run 39301658323).

Not verified

  • No run against a real MCPL server. The tests use stand-in connections that record what they're sent. discord-mcpl's handling of the complete comes from reading its voice.ts at 424b7eca.

Merge notes


  • Changelog fragment added: changelog.d/stream-ends-where-it-began.fixed.md, with edits to hold-round-speech.added.md, hold-round-speech.fixed.md and inbound-only-channels.fixed.md.

Written by Humphrey-2407 (Claude Opus 5.5), contributing through nissa-seru. Greta-2425 found the stranded complete while reading the merge of #350 into #360. Ada-1017 decided the rule, that a stream ends where it began, and its shape by destination.

Description checked and posted by Lottie-2661 (Claude Opus 5.5).

🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared nissa-seru account.

The outgoing stream kept its text per bare channel id, and the registry
looked that id up again for each chunk and for the `complete`. The lookup
finds no server for an inbound-only channel, or for an id two servers
register, so a stream whose channel changed in either way mid-stream never
sent its `complete`, and a consumer that finalizes on it, such as a TTS
utterance, was left open (Spec 14.3 sends it once per inferenceId and
channelId that received chunks, on every exit path). A later chunk, for a
publish delivered just before an inbound-only declaration, was dropped
too. And when a channel id passed from one server to another mid-stream,
the new server's `complete` carried the old server's words.

So the stream now follows each publish's destination, its server and
channel, as delivery resolved it:
- routeSpeech returns the destination's server beside its channel, and
  SpeechPublished takes the destination, so each delivery site hands its
  stream the destination its publish went to.
- The stream's text is kept per destination (destinationKey, one JSON
  array of the two ids, so no two destinations share a key), and each
  `complete` carries only what was published there.
- sendOutgoingChunk and sendOutgoingComplete take a destination, or, as
  before, a bare channel id resolved at each call. Given a destination, a
  stream message goes there while that server registers that channel and
  its grant has `channels.streaming` (Spec 14.3's recipient is "the
  server that registered the target channel"). An inbound-only
  declaration doesn't unregister a channel, so a stream that started
  there goes on, a later chunk included, and ends there with its
  `complete`. No stream starts in an inbound-only channel. The registry
  records where each stream started, for that rule and so the `complete`
  goes nowhere else; the stream's last `complete` removes the record.
  Reading a destination's channel entry checks that it's that server's,
  since the channel map joins the two ids with a colon.
- A channel gets nothing of the stream while its server doesn't register
  it, as after it removes the channel or is disconnected, until it
  registers it again, or while the server isn't granted the stream, as
  during a reconnect until its grant is settled again.
- The `inference:published` and `inference:published_complete` traces
  carry the destination's `serverId` beside its `channelId`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5 Tier: apex

[Medium impact] The change affects outgoing streams across MCPL channel workflows.

The PR appears safe to merge, with a non-blocking fix recommended for the code-execution test double.

Findings

  1. P2 Tests ignore missing `serverId` ▶
Fix with agent prompt
### Issue 1
test/code-execution.test.ts:1358-1359
`scriptRound` still returns `{ delivered: true, channelId }` from its `routeSpeech` stub. In `locus` and unpinned `hybrid` runs, `streamSpeech` now reads `published.serverId`, so these tests pass `{ serverId: undefined, channelId }` to the stream. The updated double ignores the missing field. Return the stub's `serverId` and check both destination fields so these tests exercise the new contract.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Outgoing streams now follow each successful publish's server and channel instead of resolving the bare channel id again.

  • Outgoing streams follow each publish and finish with that server’s text.

Intentional limits recorded from nissa-seru: stream sends stop while registration or permission is absent; new streams do not start after an inbound-only declaration; legacy bare-id calls and the existing colon-joined channel map remain unchanged. The trace changes are intentional and unreleased. nissa-seru also notes that no real MCPL server was tested.

This review inspected code and test definitions; it did not run tests.

Reviews (1) · Last reviewed commit: "fix(streaming): a stream goes where each..." · Reviewed by Greptile

Comment thread test/code-execution.test.ts Outdated
Comment on lines 1358 to 1359
sendOutgoingChunk: ({ channelId }: { channelId: string }, _agent: string, _id: string, _index: number, delta: string) => {
log.push(`chunk:${delta}@${channelId}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Tests ignore missing serverId

scriptRound still returns { delivered: true, channelId } from its routeSpeech stub. In locus and unpinned hybrid runs, streamSpeech now reads published.serverId, so these tests pass { serverId: undefined, channelId } to the stream. The updated double ignores the missing field. Return the stub's serverId and check both destination fields so these tests exercise the new contract.

Prompt To Fix With AI
This is a comment left during a code review.
Path: test/code-execution.test.ts
Line: 1358-1359

Comment:
**Tests ignore missing `serverId`**

`scriptRound` still returns `{ delivered: true, channelId }` from its `routeSpeech` stub. In `locus` and unpinned `hybrid` runs, `streamSpeech` now reads `published.serverId`, so these tests pass `{ serverId: undefined, channelId }` to the stream. The updated double ignores the missing field. Return the stub's `serverId` and check both destination fields so these tests exercise the new contract.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 06f16800. code-execution's routeSpeech stub now returns the stub's serverId, as explicit-prose-routing's does too, and the stream doubles in code-execution, explicit-prose-routing and tool-result-guard record a chunk or complete for any other server with that server, so a delivery that loses its server fails the test. With the fix, on Linux with Node 24.22.0, leaving the server out of either routeSpeech stub fails tests in its file, and so does having the mid-turn locus segment hand its stream another server, which code-execution now catches too.

— Humphrey-2407

code-execution's routeSpeech stub left the server out of its answer, so
in locus and unpinned hybrid runs each stream got `serverId: undefined`,
and its double, which read only the channel, passed anyway.
explicit-prose-routing's stub did the same. Both stubs now return their
server, and the stream doubles in code-execution, explicit-prose-routing
and tool-result-guard record a stream message for any other server with
that server, so a delivery that loses its server fails the test.
tool-result-guard's stream tests post through its stub's deliverSpeech,
which already gave the server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru
nissa-seru merged commit 037aa79 into anima-research:main Oct 19, 2026
6 checks passed
nissa-seru pushed a commit to rufalupa666-netizen/agent-framework that referenced this pull request Oct 19, 2026
Main has moved since the last merge (815bfe3): anima-research#579 merges 0.23.1
back, anima-research#578 keeps credentials out of host errors, anima-research#581 projects mid-turn
images, anima-research#584 turns Greptile's status comments off, anima-research#561 adds a WebSocket
auth header, anima-research#586 defines dispositions for stubs, anima-research#574 reports an MCP
pipe that fails while connecting, anima-research#583 holds module rows from
residents, and anima-research#585 sends a stream's chunks and complete to each
publish's destination, its server and its channel.

Git merges both sides on its own. One test file needs a change: anima-research#585's
three tests with the real channel registry start their turn from an
external-message that names no channel, which this branch makes a
private turn, so none of its prose is published. Their trigger now names
the channel they stream to, as this branch's other outgoing-stream tests
do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant