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..f888c1e8a --- /dev/null +++ b/changelog.d/fix-prose-invitation-target.fixed.md @@ -0,0 +1,10 @@ +- The closed-channel invitation's "reply without joining" prefix is now always one + 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 4bb960be4..e9fe65228 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -7323,6 +7323,19 @@ 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, 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 @@ -7338,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 ">>${channelLabel}"; 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 01c81ec1a..9e31ef7e1 100644 --- a/src/mcpl/channel-registry.ts +++ b/src/mcpl/channel-registry.ts @@ -2445,6 +2445,44 @@ 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 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, 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; + 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 undefined; + } + /** * 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..80a825658 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,66 @@ 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'); +}); + +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 d9108df92..dc61fc56c 100644 --- a/test/trunk-channel-routing.test.ts +++ b/test/trunk-channel-routing.test.ts @@ -233,6 +233,78 @@ 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, serverId?: string) => { asked.push(`${serverId}/${id}`); return '@_reim0n'; }, + stopAll: () => {}, + }; + i.handleMcplPushEvent(dmEvent('m3')); + const text = textOf(framework); + 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(); + + // 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: () => ({ label: 'DM: _reim0n', capabilities: { history: { maxMessages: 80 } } }), + proseTargetFor: () => undefined, + stopAll: () => {}, + }; + i2.handleMcplPushEvent(dmEvent('m4')); + 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 () => { membrane.pushResponse(createMockResponse([{ type: 'text', text: 'the date is ...' }])); const framework = await makeFramework();