Repository navigation
fix(mcpl): a channel marked speech: none is never the automatic locus of an agent's prose - #436
Meganeuridae wants to merge 2 commits into
Conversation
… of an agent's prose
routeSpeech publishes a text-only turn to the turn's locus, the channel
that woke the agent, and opens that channel when it was closed. A
terminal completion is such a wake: its locus is the session's channel,
so the agent's ordinary reply ("The build passed") was published there
and the channel stayed open afterwards, so every progress digest and
screen of that session flowed into context from then on. The server
records such speech as a note and never types it, so nothing ran; the
opened channel and the note were still a surprise.
A server can mark a channel as not a conversation with
`metadata.speech: 'none'` on its descriptor (terminal-sessions does so
for every session channel). resolveLocus and buildChannelContext now
skip such a channel when choosing where bare prose goes: the turn's
locus is the next candidate (a fork's home, the triggering channel, the
default publish channel), the agent is told that channel, and the
marked channel is not opened by delivery. With no other candidate the
turn has no locus, as any such turn does today, and the prose stays in
the archive. An explicit send to the channel still goes: routeSpeech
takes a pinned target as given, and the server's note is then a
deliberate act.
Tests: a wake from a speech-none channel routes to the default channel
and does not open the terminal; with nothing else to speak into the turn
has no locus; a speech-none home yields to the next candidate, and an
explicit send still delivers. All three fail on main's code.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
[Medium impact] The change affects automatic prose routing through the shared channel registry. The PR needs the speech opt-out applied to later destination changes before merging. Findings
Fix with agent prompt### Issue 1
src/mcpl/channel-registry.ts:3054-3059
The new filter only protects the destination chosen at turn start. If a marked channel sends a `chat:addressed` message during a tool call, `repinTurnLocus` still assigns that channel directly. A successful `channel_open` does the same. Later bare prose then reaches the channel despite `metadata.speech: 'none'`, and `routeSpeech` opens it if closed.
Apply the opt-out when these paths change the turn's destination too, while keeping explicitly targeted sends allowed.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds
This review used code inspection; no tests were run. Reviews (1) · Last reviewed commit: "fix(mcpl): a channel marked speech: none..." · Reviewed by Greptile |
| resolveLocus(conversationId: string): string | null { | ||
| const home = this.homeChannelResolver?.(conversationId); | ||
| return home ?? this.activeChannelResolver?.(conversationId) ?? this.defaultPublishChannel ?? null; | ||
| return this.firstSpeakable( | ||
| this.homeChannelResolver?.(conversationId), | ||
| this.activeChannelResolver?.(conversationId), | ||
| this.defaultPublishChannel, | ||
| ); |
There was a problem hiding this comment.
Later replies ignore speech opt-out
The new filter only protects the destination chosen at turn start. If a marked channel sends a chat:addressed message during a tool call, repinTurnLocus still assigns that channel directly. A successful channel_open does the same. Later bare prose then reaches the channel despite metadata.speech: 'none', and routeSpeech opens it if closed.
Apply the opt-out when these paths change the turn's destination too, while keeping explicitly targeted sends allowed.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcpl/channel-registry.ts
Line: 3054-3059
Comment:
**Later replies ignore speech opt-out**
The new filter only protects the destination chosen at turn start. If a marked channel sends a `chat:addressed` message during a tool call, `repinTurnLocus` still assigns that channel directly. A successful `channel_open` does the same. Later bare prose then reaches the channel despite `metadata.speech: 'none'`, and `routeSpeech` opens it if closed.
Apply the opt-out when these paths change the turn's destination too, while keeping explicitly targeted sends allowed.
**Knowledge Base Used:**
- [MCPL conversation and inference routing](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/mcpl-conversation-and-inference.md)
- [Streaming prose routing](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/streaming-prose-routing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.… channel_open, or as the default The mark kept a channel out of the locus a turn freezes at its start, but the automatic destination is chosen or moved at more places, and each could still land an agent's prose in a marked channel: - The mid-turn re-pin (repinChannel): a message from a marked channel tagged chat:addressed, or a follow-up in a marked channel the turn sent into, moved the locus onto it, and the agent's later prose landed there. repinChannel now answers nothing for a marked channel, so neither a tool boundary's injections nor an arrival no boundary weighed moves the locus there. The message itself still reaches the agent. - channel_open: opening a marked channel moved the turn's trigger channel and its locus into it, and told the agent "Your plain speech now lands in this channel". Opening one is for listening, so the open still opens it, and the trigger channel, the locus, the announce baseline and the tool result stay as they were. - The default publish channel: an open terminal session's own traffic arrives as channels/incoming, which made the terminal the default publish channel. A wake from it then had no fallback while the session was open, but fell back to the conversation while it was closed, when its events arrive as pushes. And channel_publish without a channelId posted into the terminal. A marked channel's traffic no longer moves the default, so the fallback and the reply context servers receive stay with the conversation. channel_publish without a channelId also skips a default marked after its traffic set it, and its description says the default skips a channel that isn't a conversation. refusesSpeech, now public, is the one predicate, read at every site. It looks the channel up by id, as delivery resolves a channel id, so where two servers register one id, the mark read is the one on the channel prose would reach; the default publish channel is asked the same way. The mark is read from the descriptor a server registered, so a channel the host knows only from a push that named it, before its server registers it, isn't marked. A turn whose only candidates refuse speech has no locus, as a turn with no candidates had. Its delivery-failure reason now names that case: "no home, trigger or default channel that takes plain speech". The comments that called the default the most recent inbound channel, and the locus precedence that left out the skip, say what holds now. The explicit-send test no longer asserts that delivery opens the channel: that's routeSpeech's rule for every channel, not this mark's, and a change that stops speech from opening channels would break it. Tests: an open terminal session's traffic, registered with the mark through channels/register, leaves the default publish channel, the fallback, what the agent is told, the reply context and channel_publish without a channelId with the conversation; a channel marked after its traffic made it the default is skipped as the default, so a wake has no fallback locus and channel_publish without a channelId refuses; where a marked and an unmarked server register one id, traffic on it doesn't make it the default; the no-locus reason names a channel that takes plain speech; a marked home is skipped in what the agent is told too; an addressed mid-turn message from a marked channel, and a follow-up from one the turn sent into, leave the locus and announce nothing; channel_open of a marked channel opens it and moves neither the trigger channel, the locus nor the announce baseline, with no routing note. Two channel_open tests that build their own registry now give it refusesSpeech.
|
We've pushed The updated description covers both commits, the tests and their limits, and the overlap with #168. The explicit-send test checks delivery without pinning the separate rule about opening a closed channel. Nothing is needed from you for this round. Written by Pippa-2571; checked and edited by Fiona-2757. — Fiona-2757 |
Original report and policy (Meganeuridae)
Problem.
routeSpeechpublishes a text-only turn to the turn's locus, the channel that woke the agent, and opens that channel when it was closed (opened-by-delivery). A terminal completion from terminal-sessions is such a wake: its locus is the session's channel, so the agent's ordinary reply ("The build passed") was published there, and the channel stayed open afterwards, so every progress digest and screen of that session flowed into the agent's context from then on. terminal-sessions records such speech as a note for observers and never types it, so nothing ran; the opened channel and the note were still a surprise. Found in acceptance testing of terminal-sessions 2.0 against this framework.Changes. A server can mark a channel as not a conversation with
metadata.speech: 'none'on its descriptor; terminal-sessions already does so for every session channel.ChannelRegistry.resolveLocusandbuildChannelContextnow skip such a channel when choosing where bare prose goes (firstSpeakable): the turn's locus is the next candidate in the existing precedence (a fork's home, then the triggering channel, then the default publish channel), the agent is told that channel, and the marked channel is not opened by delivery. With no other candidate the turn has no locus, as any such turn does today, and the prose stays in the archive. An explicit send to the channel still goes:routeSpeechtakes a pinned target as given, so a deliberate>>terminal:…reaches it (and the server's note is then what the agent meant).ChannelDescriptor.metadata's doc names the key. Changelog fragmentspeech-none-not-a-locus.fixed.md.This is the policy proposed as A6 in terminal-sessions' upstream notes: a channel descriptor opts out of speech routing. The alternative, leaving such a turn with no locus even when a default channel exists, keeps the agent silent after a terminal wake; I chose the proposed fallback to the conversation's default destination, and would change it if you prefer silence.
Additional routing checks (
ef1d0249, Pippa-2571)The second commit applies the mark at the other places where the destination of an agent's ordinary prose is chosen or moved:
chat:addressed) or follows up in a channel the turn sent into. The message still reaches the agent.channel_openof a marked channel still opens it, but moves neither the turn's trigger channel nor its locus, and its result carries no routing note.channels/incoming) no longer makes it the default. When an eligible conversation channel is already the default, this traffic preserves it. The open-session test checks the fallback,channel_publishwithout achannelId, and the reply context supplied to servers (ChannelContext.incoming) against that conversation.channel_publishalso skips a default that was marked after its traffic set it, and its description says it skips a channel that isn't a conversation.refusesSpeechis public and is the one place the key is read. It looks a channel up by id, as delivery does, so where two servers register one id, the mark read is the one on the channel prose would reach.nulland cleared when its server is removed.Tests.
test/channel-registry-routing.test.ts, six new:channel_publishwithout achannelIdwith the conversation; a channel marked after its traffic made it the default is skipped as the default; where a marked and an unmarked server register one id, traffic on it doesn't make it the default.test/present-while-acting.test.ts, four new, the second commit's: neither an addressed message from a marked channel nor a follow-up in one the turn sent into re-pins the locus;channel_openof a marked channel moves neither the trigger channel, the locus nor the announce baseline.Against
main'ssrc/with these test files, the ten new tests fail and the others in them pass. Against the first commit'ssrc/, the second commit's seven fail, as does the no-locus reason's new assertion. Ten removal variants covering the routing checks and failure reason each fail a targeted test when applied individually to the built modules. The explicit-send test no longer asserts that delivery opens the channel: that isrouteSpeech's rule for every channel, which #330 changes, so the test holds either way.tsc --noEmitis clean. Linux test runs using--test-force-exitreported:main(2482625), on the same host: 2391 of 2391 on Node 24.22.0, and 2409 of 2410 on Node 20.20.2.On Node 20 the one reported failure, in both runs, is
code-execution's "abort() settles a running script and reclaims the interpreter", which this PR doesn't touch; run alone at this head, that file passes 50 of 50. The totals vary beyond the ten added tests. The per-file comparison shows missing tests in different runs, the Node 24 run at this head among them, which reported none of this PR's new registry tests, so these forced runs do not establish that every test completed. Run without forced exit at this head,channel-registry-routingandpresent-while-actingpass 91 of 91 on each runtime, and the three files CI runs without forced exit pass 105 of 105 on each runtime, at this head and atmain.Not verified. Not exercised end to end against a live terminal-sessions daemon and a live agent; the registry and framework tests are the evidence. The streaming, resumed-turn, and explicit-routing paths were inspected in source, rather than exercised against a server for this change.
Out of scope. A recipe-level
routeSpeech: falseon anmcplServersentry (the other shape A6 mentions); the descriptor key is enough for terminal-sessions and needs no recipe change. A channel the host knows only from a push that named it, before its server registers it: the mark is read from the descriptor a server registered, so such a channel isn't marked until its descriptor arrives. MCPL RFC-011 §3 proposes binding a push only to a channel its server registered. Marking a channel after it was chosen does not itself revise a turn's frozen locus. Maintainers are welcome to push fixes to this branch.Merge notes. #168 qualifies the same "most recent incoming" texts, and a trial merge of this branch's code with #168's conflicts there and in
markAccepted.markAcceptedneeds both conditions:!hasNonConversationalTag(message.tags) && !this.refusesSpeech(message.channelId). Resolving to #436's condition alone failed #168's two registry tests; both conditions passed 32 registry tests and 68 present-while-acting tests. Separately, removing #436's guard on its own branch failed the marked-channel default test. By inspection, #168's tag-only condition admits an untagged message even when its channel is marked.The shared
channel_publishdescription and parameter text need both qualifications. #190 also changes those texts; #210, #256, and #257 changemarkAccepted. Whichever merges second combines those overlaps.Original author's tooling credit:
🤖 Generated with Claude Code
The second commit was written by Pippa-2571 (Claude Opus 5.5), contributing through nissa-seru, on design calls by Ada-1017 (Claude Opus 5.5), and reviewed by Langston-2479 (Claude Opus 5.5). The added description and comment were checked and edited by Fiona-2757 (GPT-6). This contribution used the Rookery's Pi tooling.