From 48a64b01d01c2f5ef5ec1bb75555f657517c5ecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ram=C3=B3n=20Mart=C3=ADnez?= Date: Fri, 2 Oct 2026 12:16:20 +0200 Subject: [PATCH 1/2] fix(mcpl): closed-channel invitation suggests a reply prefix that parses and resolves The invitation quoted the channel label verbatim as the ">>" target. The prefix grammar reads the target as the first non-whitespace run, so any label with a space broke it: a DM labelled "DM: alice" became ">>#DM: alice" (target "#DM:", body "alice ..."), the reply bounced, and the retained text later went out with a stray "alice" line; "#fable (antra's server)" would have delivered "(antra's server)" as text. ChannelRegistry.proseTargetFor(channelId) returns one whitespace-free token that resolveProseTarget() maps back to the same channel: "@name" for a DM, else "#label", "#name" without the server suffix, or the descriptor id. The invitation uses it for the prefix and keeps the readable label for naming the place; a channel the registry doesn't know falls back to "#label" only when that has no whitespace, else the channel id. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012ketk7LSSu5jzr5A2X3Qzb --- .../fix-prose-invitation-target.fixed.md | 8 +++ src/framework.ts | 7 ++- src/mcpl/channel-registry.ts | 32 +++++++++++ test/channel-registry-routing.test.ts | 36 ++++++++++++ test/trunk-channel-routing.test.ts | 56 +++++++++++++++++++ 5 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fix-prose-invitation-target.fixed.md diff --git a/changelog.d/fix-prose-invitation-target.fixed.md b/changelog.d/fix-prose-invitation-target.fixed.md new file mode 100644 index 000000000..2d62eb16e --- /dev/null +++ b/changelog.d/fix-prose-invitation-target.fixed.md @@ -0,0 +1,8 @@ +- The closed-channel invitation's "reply without joining" prefix is now always one + whitespace-free token that resolves back to the channel (`@name` for a DM, else `#label`, + `#name` without the server suffix, or the channel id), from the new + `ChannelRegistry.proseTargetFor()`. It used to quote the label verbatim, and the prefix + grammar reads the target as the first non-whitespace run: a DM labelled `DM: alice` gave + `>>#DM: alice` (target `#DM:`, body `alice …`, so the reply bounced and the retained text + went out with a stray `alice` line), and a suffixed label like `#fable (antra's server)` + delivered `(antra's server)` as text. diff --git a/src/framework.ts b/src/framework.ts index 4bb960be4..08ee5660f 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -7323,6 +7323,11 @@ export class AgentFramework { const label = opts.channelLabel ?? descriptor?.label; const channelLabel = label ? `#${label}` : `"${opts.channelId}"`; const place = opts.guildName ? `${channelLabel} in "${opts.guildName}"` : channelLabel; + // The label is for reading; the prefix must be ONE token that resolves back + // here (a DM label "DM: alice" quoted verbatim parses as target "#DM:"). + const target = + this.channelRegistry.proseTargetFor?.(opts.channelId) ?? + (label && !/\s/.test(label) ? `#${label}` : opts.channelId); const missedCount = opts.missedMessages ?? 0; const missedNote = missedCount > 0 @@ -7338,7 +7343,7 @@ export class AgentFramework { `a reply to one of your messages) reach you from it; the rest of its traffic is invisible to you. ` + missedNote + `Your options:\n` + - `1. Reply without joining — write your reply this turn prefixed with ">>${channelLabel}"; it will be delivered there. ` + + `1. Reply without joining — write your reply this turn prefixed with ">>${target}"; it will be delivered there. ` + `Note: follow-ups to your reply will NOT reach you unless they @-mention you or use the reply feature on your message.\n` + `2. Join the channel — call channel_open with channelId "${opts.channelId}" and serverId "${opts.serverId}"` + (maxBackscroll > 0 diff --git a/src/mcpl/channel-registry.ts b/src/mcpl/channel-registry.ts index 01c81ec1a..c909979ab 100644 --- a/src/mcpl/channel-registry.ts +++ b/src/mcpl/channel-registry.ts @@ -2445,6 +2445,38 @@ export class ChannelRegistry { return { error: `no channel matches "${trimmed}"`, ...(near.length ? { candidates: near } : {}) }; } + /** + * The `>>` target to show an agent for a channel: one whitespace-free token + * that `resolveProseTarget()` maps back to this same channel. The prefix + * grammar takes the target as the first non-whitespace run, so a label with + * a space can't be quoted verbatim: `>>#DM: alice` parses as target `#DM:` + * plus body `alice …`, and `>>#fable (antra's server)` delivers + * `(antra's server)` as text. Tried in order: `@name` for a DM, `#label`, + * `#name` (label without its server suffix), the descriptor id. Undefined + * when the channel isn't registered. + */ + proseTargetFor(channelId: string): string | undefined { + const entry = this.findChannelEntry(channelId); + if (!entry) return undefined; + const d = entry.descriptor; + const label = d.label ?? ''; + const meta = d.metadata as { channelType?: string; recipientName?: string } | undefined; + const isDm = meta?.channelType === 'dm' || label.toLowerCase().startsWith('dm: ') || d.id.includes(':dm:'); + const dmName = meta?.recipientName ?? (label.toLowerCase().startsWith('dm: ') ? label.slice(4) : undefined); + const bare = label.replace(/^#/, ''); + const candidates = [ + ...(isDm && dmName ? [`@${dmName}`] : []), + ...(bare ? [`#${bare}`, `#${bare.replace(/\s*\([^)]*\)\s*$/, '')}`] : []), + d.id, + ]; + for (const c of candidates) { + if (/\s/.test(c) || c === '#') continue; + const r = this.resolveProseTarget(c); + if ('channelId' in r && r.channelId === d.id) return c; + } + return d.id; + } + /** * Open a channel because something was DELIVERED into it (explicit send * tool or routed speech). Sending into a closed channel is not a thing: diff --git a/test/channel-registry-routing.test.ts b/test/channel-registry-routing.test.ts index 8468c2d37..0f1379b5f 100644 --- a/test/channel-registry-routing.test.ts +++ b/test/channel-registry-routing.test.ts @@ -2,6 +2,7 @@ import { CapabilityGrant, ALL_CAPABILITY_PATHS } from '../src/mcpl/capability-gr import { test } from 'node:test'; import assert from 'node:assert/strict'; import { ChannelRegistry } from '../src/mcpl/channel-registry.js'; +import { parseProsePrefix } from '../src/mcpl/prose-grammar.js'; import type { McplServerRegistry } from '../src/mcpl/server-registry.js'; import type { FeatureSetManager } from '../src/mcpl/feature-set-manager.js'; @@ -526,3 +527,38 @@ test('DM prose targets resolve people-first: @name, prefix-lenient names, and <@ assert.ok('error' in missing && /send_dm/.test(missing.error), 'no-match points at send_dm'); assert.ok('error' in missing && (missing.candidates?.length ?? 0) === 2, 'lists known DMs by name'); }); + +test('proseTargetFor: one whitespace-free token per channel that resolves back to it', async () => { + const { registry } = makeRegistry({ delivered: true }); + await registry.handleChanged('discord', { + added: [ + { id: 'discord:dm:555', type: 'discord', label: 'DM: antra', direction: 'bidirectional', + metadata: { channelType: 'dm', recipientName: 'antra', recipientId: '134390790938951680' } }, + // A DM known only by its label (no metadata): still "@name". + { id: 'discord:dm:557', type: 'discord', label: 'DM: _reim0n', direction: 'bidirectional' }, + { id: 'discord:g1:100', type: 'discord', label: "#fable (antra's server)", direction: 'bidirectional' }, + { id: 'discord:g1:101', type: 'discord', label: '#ops', direction: 'bidirectional' }, + // Same name in two guilds: "#lobby" is ambiguous, the full labels have spaces → the id. + { id: 'discord:g1:102', type: 'discord', label: '#lobby (antra\'s server)', direction: 'bidirectional' }, + { id: 'discord:g2:200', type: 'discord', label: '#lobby (Connectome)', direction: 'bidirectional' }, + ], + } as never); + + const expected: Record = { + 'discord:dm:555': '@antra', + 'discord:dm:557': '@_reim0n', + 'discord:g1:100': '#fable', + 'discord:g1:101': '#ops', + 'discord:g1:102': 'discord:g1:102', + 'discord:g2:200': 'discord:g2:200', + }; + for (const [id, want] of Object.entries(expected)) { + const t = registry.proseTargetFor(id); + assert.equal(t, want, `target for ${id}`); + // The property that matters: the grammar reads it whole, and it routes back here. + assert.equal(parseProsePrefix(`>>${t} hello`).target, t, `${t} parses as one target`); + const r = registry.resolveProseTarget(t!); + assert.ok('channelId' in r && r.channelId === id, `${t} resolves to ${id}`); + } + assert.equal(registry.proseTargetFor('discord:nowhere'), undefined, 'unregistered → undefined'); +}); diff --git a/test/trunk-channel-routing.test.ts b/test/trunk-channel-routing.test.ts index d9108df92..14eb339be 100644 --- a/test/trunk-channel-routing.test.ts +++ b/test/trunk-channel-routing.test.ts @@ -233,6 +233,62 @@ describe('Trunk channel routing (item-3 redux)', () => { await framework.stop(); }); + it('the invitation\'s reply prefix is a single resolvable token, not a label with spaces', async () => { + // Live bug (2026-10-02): a first DM arrived on a closed channel labelled "DM: _reim0n"; the + // invitation said to prefix ">>#DM: _reim0n", which parses as target "#DM:" + body "_reim0n …": + // the reply bounced, and the retained text later went out with a stray "_reim0n" line. + const textOf = (framework: Awaited>) => + framework.getAgent('scout')!.getContextManager().queryMessages({}).messages.at(-1)?.content + .filter((block): block is { type: 'text'; text: string } => block.type === 'text') + .map((block) => block.text) + .join('\n') ?? ''; + const dmEvent = (id: string) => ({ + type: 'mcpl:push-event' as const, + serverId: 'discord', + featureSet: 'discord.messaging', + eventId: `discord_msg_${id}`, + content: [{ type: 'text' as const, text: '_reim0n: hey, I have THE gossip' }], + origin: { source: 'discord', messageId: id, mcplChannelId: 'discord:dm:1555', channelName: 'DM: _reim0n' }, + tags: ['chat:addressed'], + timestamp: new Date().toISOString(), + inferenceId: `i-${id}`, + triggerInference: false, + }); + + // The registry knows the channel: its round-trip target is used. + const framework = await makeFramework(); + const i = internals(framework); + const asked: string[] = []; + i.channelRegistry = { + ensureChannelRegistered: () => {}, + isChannelOpen: () => false, + getDescriptor: () => ({ label: 'DM: _reim0n', capabilities: { history: { maxMessages: 80 } } }), + proseTargetFor: (id: string) => { asked.push(id); return '@_reim0n'; }, + stopAll: () => {}, + }; + i.handleMcplPushEvent(dmEvent('m3')); + const text = textOf(framework); + assert.deepEqual(asked, ['discord:dm:1555']); + assert.match(text, /prefixed with ">>@_reim0n"/); + assert.doesNotMatch(text, />>#DM:/); + assert.match(text, /#DM: _reim0n/, 'the readable label still names the place'); + await framework.stop(); + + // Unknown to the registry: a label with whitespace falls back to the channel id, never verbatim. + const fw2 = await makeFramework(); + const i2 = internals(fw2); + i2.channelRegistry = { + ensureChannelRegistered: () => {}, + isChannelOpen: () => false, + getDescriptor: () => undefined, + proseTargetFor: () => undefined, + stopAll: () => {}, + }; + i2.handleMcplPushEvent(dmEvent('m4')); + assert.match(textOf(fw2), /prefixed with ">>discord:dm:1555"/); + await fw2.stop(); + }); + it('a channel-incoming trunk turn records its triggering channel', async () => { membrane.pushResponse(createMockResponse([{ type: 'text', text: 'the date is ...' }])); const framework = await makeFramework(); From ae8a28ac7c9bf5c5dc58fdd4afe8723292ae3ca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ram=C3=B3n=20Mart=C3=ADnez?= Date: Fri, 2 Oct 2026 13:17:38 +0200 Subject: [PATCH 2/2] fix(mcpl): invitation targets are server-scoped and never whitespace; no prefix when none is safe Review of #215 (greptile): - P1: proseTargetFor looked the channel up by id alone and accepted any candidate that resolved to that id. Ids are unique only within a connection, and a resolved target names a channel by id, so with the same id on two servers the suggested prefix could send the reply through the other server. It now takes the serverId, and returns undefined when the id is registered by more than one server. - P2: the last fallback returned the descriptor id even when it contained whitespace. It now returns undefined when no candidate is whitespace-free and resolves back. The invitation treats a registry that can name targets as authoritative: no safe token means option 1 says it isn't available and points to joining. A registry stand-in without the method still gets a whitespace-free guess. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012ketk7LSSu5jzr5A2X3Qzb --- .../fix-prose-invitation-target.fixed.md | 16 ++++++----- src/framework.ts | 19 +++++++++---- src/mcpl/channel-registry.ts | 18 ++++++++---- test/channel-registry-routing.test.ts | 28 +++++++++++++++++++ test/trunk-channel-routing.test.ts | 26 +++++++++++++---- 5 files changed, 83 insertions(+), 24 deletions(-) diff --git a/changelog.d/fix-prose-invitation-target.fixed.md b/changelog.d/fix-prose-invitation-target.fixed.md index 2d62eb16e..f888c1e8a 100644 --- a/changelog.d/fix-prose-invitation-target.fixed.md +++ b/changelog.d/fix-prose-invitation-target.fixed.md @@ -1,8 +1,10 @@ - The closed-channel invitation's "reply without joining" prefix is now always one - whitespace-free token that resolves back to the channel (`@name` for a DM, else `#label`, - `#name` without the server suffix, or the channel id), from the new - `ChannelRegistry.proseTargetFor()`. It used to quote the label verbatim, and the prefix - grammar reads the target as the first non-whitespace run: a DM labelled `DM: alice` gave - `>>#DM: alice` (target `#DM:`, body `alice …`, so the reply bounced and the retained text - went out with a stray `alice` line), and a suffixed label like `#fable (antra's server)` - delivered `(antra's server)` as text. + whitespace-free token that resolves back to the channel on the server the message came + from (`@name` for a DM, else `#label`, `#name` without the server suffix, or the channel + id), from the new `ChannelRegistry.proseTargetFor(channelId, serverId)`. It used to quote + the label verbatim, and the prefix grammar reads the target as the first non-whitespace + run: a DM labelled `DM: alice` gave `>>#DM: alice` (target `#DM:`, body `alice …`, so the + reply bounced and the retained text went out with a stray `alice` line), and a suffixed + label like `#fable (antra's server)` delivered `(antra's server)` as text. When no token is + safe (the channel id is registered by more than one server, or every option contains + whitespace) the invitation offers no prefix and points to joining the channel instead. diff --git a/src/framework.ts b/src/framework.ts index 08ee5660f..e9fe65228 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -7324,10 +7324,18 @@ export class AgentFramework { const channelLabel = label ? `#${label}` : `"${opts.channelId}"`; const place = opts.guildName ? `${channelLabel} in "${opts.guildName}"` : channelLabel; // The label is for reading; the prefix must be ONE token that resolves back - // here (a DM label "DM: alice" quoted verbatim parses as target "#DM:"). - const target = - this.channelRegistry.proseTargetFor?.(opts.channelId) ?? - (label && !/\s/.test(label) ? `#${label}` : opts.channelId); + // here, on this server (a DM label "DM: alice" quoted verbatim parses as + // target "#DM:"). A registry that can name targets is authoritative: no safe + // token means no prefix option. A stand-in without proseTargetFor gets a + // whitespace-free guess, never a label with spaces. + const namer = this.channelRegistry as { proseTargetFor?: (id: string, serverId?: string) => string | undefined }; + const target = typeof namer.proseTargetFor === 'function' + ? namer.proseTargetFor(opts.channelId, opts.serverId) + : [label ? `#${label}` : undefined, opts.channelId].find((t): t is string => !!t && !/\s/.test(t)); + const replyOption = target + ? `1. Reply without joining — write your reply this turn prefixed with ">>${target}"; it will be delivered there. ` + + `Note: follow-ups to your reply will NOT reach you unless they @-mention you or use the reply feature on your message.\n` + : `1. Reply without joining isn't available here: no single-word target reaches this channel unambiguously. Join it (option 2) to reply.\n`; const missedCount = opts.missedMessages ?? 0; const missedNote = missedCount > 0 @@ -7343,8 +7351,7 @@ export class AgentFramework { `a reply to one of your messages) reach you from it; the rest of its traffic is invisible to you. ` + missedNote + `Your options:\n` + - `1. Reply without joining — write your reply this turn prefixed with ">>${target}"; it will be delivered there. ` + - `Note: follow-ups to your reply will NOT reach you unless they @-mention you or use the reply feature on your message.\n` + + replyOption + `2. Join the channel — call channel_open with channelId "${opts.channelId}" and serverId "${opts.serverId}"` + (maxBackscroll > 0 ? `; to also read recent history, add backscroll (a number up to ${maxBackscroll}) and beforeMessageId "${opts.messageId}".\n` diff --git a/src/mcpl/channel-registry.ts b/src/mcpl/channel-registry.ts index c909979ab..9e31ef7e1 100644 --- a/src/mcpl/channel-registry.ts +++ b/src/mcpl/channel-registry.ts @@ -2452,12 +2452,18 @@ export class ChannelRegistry { * a space can't be quoted verbatim: `>>#DM: alice` parses as target `#DM:` * plus body `alice …`, and `>>#fable (antra's server)` delivers * `(antra's server)` as text. Tried in order: `@name` for a DM, `#label`, - * `#name` (label without its server suffix), the descriptor id. Undefined - * when the channel isn't registered. + * `#name` (label without its server suffix), the descriptor id. + * + * Undefined when there is no safe token: the channel isn't registered (on + * `serverId`, when given), no candidate is whitespace-free and resolves back, + * or the id is registered by more than one server. A resolved target names a + * channel by id alone, and ids are unique only within a connection, so a + * shared id could route the reply through the wrong server. */ - proseTargetFor(channelId: string): string | undefined { - const entry = this.findChannelEntry(channelId); - if (!entry) return undefined; + proseTargetFor(channelId: string, serverId?: string): string | undefined { + const sameId = [...this.channels.values()].filter((e) => e.descriptor.id === channelId); + const entry = serverId ? sameId.find((e) => e.serverId === serverId) : sameId[0]; + if (!entry || sameId.length > 1) return undefined; const d = entry.descriptor; const label = d.label ?? ''; const meta = d.metadata as { channelType?: string; recipientName?: string } | undefined; @@ -2474,7 +2480,7 @@ export class ChannelRegistry { const r = this.resolveProseTarget(c); if ('channelId' in r && r.channelId === d.id) return c; } - return d.id; + return undefined; } /** diff --git a/test/channel-registry-routing.test.ts b/test/channel-registry-routing.test.ts index 0f1379b5f..80a825658 100644 --- a/test/channel-registry-routing.test.ts +++ b/test/channel-registry-routing.test.ts @@ -562,3 +562,31 @@ test('proseTargetFor: one whitespace-free token per channel that resolves back t } assert.equal(registry.proseTargetFor('discord:nowhere'), undefined, 'unregistered → undefined'); }); + +test('proseTargetFor: no token when none is safe (shared id across servers, whitespace-only options)', async () => { + const { registry } = makeRegistry({ delivered: true }); + // The same channel id registered by two connections: a resolved target names a + // channel by id alone, so either server's reply could leave through the other. + await registry.handleChanged('discord-a', { + added: [{ id: 'chan:7', type: 'discord', label: '#shared-a', direction: 'bidirectional' }], + } as never); + await registry.handleChanged('discord-b', { + added: [{ id: 'chan:7', type: 'discord', label: '#shared-b', direction: 'bidirectional' }], + } as never); + assert.equal(registry.proseTargetFor('chan:7', 'discord-a'), undefined); + assert.equal(registry.proseTargetFor('chan:7', 'discord-b'), undefined); + assert.equal(registry.proseTargetFor('chan:7'), undefined); + + // An id with whitespace and a label with whitespace: nothing parses as one target. + await registry.handleChanged('discord-a', { + added: [{ id: 'thread 42', type: 'discord', label: 'thread 42', direction: 'bidirectional' }], + } as never); + assert.equal(registry.proseTargetFor('thread 42', 'discord-a'), undefined); + + // serverId is honoured: a channel registered only on discord-a isn't named for discord-b. + await registry.handleChanged('discord-a', { + added: [{ id: 'chan:8', type: 'discord', label: '#only-a', direction: 'bidirectional' }], + } as never); + assert.equal(registry.proseTargetFor('chan:8', 'discord-a'), '#only-a'); + assert.equal(registry.proseTargetFor('chan:8', 'discord-b'), undefined); +}); diff --git a/test/trunk-channel-routing.test.ts b/test/trunk-channel-routing.test.ts index 14eb339be..dc61fc56c 100644 --- a/test/trunk-channel-routing.test.ts +++ b/test/trunk-channel-routing.test.ts @@ -263,30 +263,46 @@ describe('Trunk channel routing (item-3 redux)', () => { ensureChannelRegistered: () => {}, isChannelOpen: () => false, getDescriptor: () => ({ label: 'DM: _reim0n', capabilities: { history: { maxMessages: 80 } } }), - proseTargetFor: (id: string) => { asked.push(id); return '@_reim0n'; }, + proseTargetFor: (id: string, serverId?: string) => { asked.push(`${serverId}/${id}`); return '@_reim0n'; }, stopAll: () => {}, }; i.handleMcplPushEvent(dmEvent('m3')); const text = textOf(framework); - assert.deepEqual(asked, ['discord:dm:1555']); + assert.deepEqual(asked, ['discord/discord:dm:1555'], 'asked for this channel on the server it came from'); assert.match(text, /prefixed with ">>@_reim0n"/); assert.doesNotMatch(text, />>#DM:/); assert.match(text, /#DM: _reim0n/, 'the readable label still names the place'); await framework.stop(); - // Unknown to the registry: a label with whitespace falls back to the channel id, never verbatim. + // The registry finds no safe token (shared id, whitespace): no prefix is offered at all. const fw2 = await makeFramework(); const i2 = internals(fw2); i2.channelRegistry = { ensureChannelRegistered: () => {}, isChannelOpen: () => false, - getDescriptor: () => undefined, + getDescriptor: () => ({ label: 'DM: _reim0n', capabilities: { history: { maxMessages: 80 } } }), proseTargetFor: () => undefined, stopAll: () => {}, }; i2.handleMcplPushEvent(dmEvent('m4')); - assert.match(textOf(fw2), /prefixed with ">>discord:dm:1555"/); + const t2 = textOf(fw2); + assert.doesNotMatch(t2, /prefixed with ">>/); + assert.match(t2, /Reply without joining isn't available here/); + assert.match(t2, /channel_open with channelId "discord:dm:1555"/, 'joining stays available'); await fw2.stop(); + + // A registry stand-in without proseTargetFor: a whitespace-free guess, never the spaced label. + const fw3 = await makeFramework(); + const i3 = internals(fw3); + i3.channelRegistry = { + ensureChannelRegistered: () => {}, + isChannelOpen: () => false, + getDescriptor: () => undefined, + stopAll: () => {}, + }; + i3.handleMcplPushEvent(dmEvent('m5')); + assert.match(textOf(fw3), /prefixed with ">>discord:dm:1555"/); + await fw3.stop(); }); it('a channel-incoming trunk turn records its triggering channel', async () => {