From cf6f6e97b4f674c1fa01ad282a90f33dfaf8a25e Mon Sep 17 00:00:00 2001 From: Sage-307 Date: Fri, 2 Oct 2026 01:19:38 -0400 Subject: [PATCH 1/3] fix(routing): isolate fork namespaces by raw channel identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lossy sanitizer maps slack:C1, slack/C1, slack C1, and slack-C1 to the same fork name. Active duplicates fail registration; after restart, a previously unseen channel can reopen another channel’s old Chronicle namespace. Escape unsafe UTF-16 code units with fixed-width tilde escapes, including the escape marker itself. Use a tilde after the configured prefix for every new name, so even already-safe IDs cannot alias names produced by the legacy sanitizer for that prefix. Preserve raw-channel generation counters and historical records. Document the generated-name change and binding-based lookup migration in CHANGELOG.md. Add five regressions covering spelling and Unicode collisions, deterministic names, legacy-name separation, generation hydration, independent live and persisted contexts, and a real legacy-store upgrade. Before the fix, focused compiled Node tests report 22 pass / 5 fail. After the fix and updated exact-name expectations they report 27 pass / 0 fail. Build and diff checks pass. Full-suite validation and independent review follow. Co-Authored-By: GPT-6 Astra --- CHANGELOG.md | 4 + src/mcpl/conversation-router.ts | 16 ++-- test/conversation-router.test.ts | 56 ++++++++++++- test/conversation-routing.test.ts | 130 ++++++++++++++++++++++++++---- 4 files changed, 186 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b9a30b4c..24fe849e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ Releases up to and including 0.7.3 predate this file; for their contents see ## Unreleased +### Breaking + +- **Hosts using conversation forks:** Generated agent names now escape channel IDs losslessly, so distinct channels cannot share a fork name or Chronicle namespace (#44). Names use a new separator, for example `conversation~slack~003aC1-g1`, including for IDs that needed no escaping. Integrations that construct names must instead read `ConversationRouter.getBinding(channelId).agentName` after binding. Existing Chronicle history remains under its original names, and persisted per-channel generation counters continue unchanged; fresh engagements use the new names and never alias the legacy sanitizer's output for the same configured prefix. No store migration or sibling dependency upgrade is required. + ## 0.19.0 — 2026-09-28 ### Added diff --git a/src/mcpl/conversation-router.ts b/src/mcpl/conversation-router.ts index e96cb5703..efac5894b 100644 --- a/src/mcpl/conversation-router.ts +++ b/src/mcpl/conversation-router.ts @@ -67,7 +67,8 @@ export interface ConversationRouterConfig { /** Final system-initiated user message sent to a fork on expiry. */ closurePrompt?: string; - /** Prefix for generated fork agent names. Default 'conversation'. */ + /** Prefix for generated fork agent names. Default 'conversation'. + * Read the generated name from the binding rather than constructing it. */ agentPrefix?: string; /** @@ -274,11 +275,16 @@ export class ConversationRouter { return rule === 'always' || facts.mentioned; } - /** `conversation-slack-C123-g2` — agent names double as Chronicle - * namespace components, so the channel id is sanitized. */ + /** Agent names double as Chronicle namespace components. Escape each + * unsafe UTF-16 code unit (including '~') to preserve every distinct JS + * string, even lone surrogates, without introducing path separators. + * The '~' after the prefix keeps ALL new names outside the old lossy + * sanitizer's output for that prefix, including already-safe channel IDs. + * Example: `conversation~slack~003aC123-g2`. */ private forkAgentName(channelId: string, generation: number): string { const prefix = this.config.agentPrefix ?? 'conversation'; - const safe = channelId.replace(/[^A-Za-z0-9_-]+/g, '-'); - return `${prefix}-${safe}-g${generation}`; + const safe = channelId.replace(/[^A-Za-z0-9_-]/g, (unit) => + `~${unit.charCodeAt(0).toString(16).padStart(4, '0')}`); + return `${prefix}~${safe}-g${generation}`; } } diff --git a/test/conversation-router.test.ts b/test/conversation-router.test.ts index 9a1697784..cd10c6871 100644 --- a/test/conversation-router.test.ts +++ b/test/conversation-router.test.ts @@ -9,6 +9,60 @@ function makeRouter(overrides: Partial { + const channelIds = [ + 'slack:C1', 'slack/C1', 'slack C1', 'slack-C1', 'slack::C1', + 'slack~003aC1', 'slack%3AC1', 'slack_C1', 'slack\\C1', + '', '-', '/', '.', '..', '~', '\0', '\n', + 'café', 'cafe\u0301', '频道', '😀', '\ud800', '\ud801', '\ufffd', + 'slack:C1-g1', 'slack:C1-g2', + ]; + const names = channelIds.map((channelId) => proposedName(channelId)); + assert.equal(new Set(names).size, channelIds.length, 'every raw ID needs a distinct namespace'); + for (let i = 0; i < channelIds.length; i++) { + assert.equal(proposedName(channelIds[i]!), names[i], 'naming is deterministic across routers'); + assert.doesNotMatch(names[i]!, /[/\\\s\0]/, 'channel IDs cannot introduce namespace path separators'); + } +}); + +test('new fork names cannot reuse a legacy sanitized namespace, even for safe IDs', () => { + const router = makeRouter({ agentPrefix: 'support' }); + const names = ['slack:C1', 'slack-C1', '', 'slack~003aC1'].map( + (channelId) => proposedName(channelId, router), + ); + for (const name of names) { + assert.ok(name.startsWith('support'), 'custom prefix is retained'); + assert.ok( + !/^support-[A-Za-z0-9_-]*-g1$/.test(name), + 'every new name must be outside the legacy sanitizer output, not just unsafe IDs', + ); + } +}); + +test('legacy generation counters remain keyed by raw channel ID with the new naming', () => { + const router = makeRouter(); + router.hydrateGenerations({ 'slack:C1': 3, 'slack/C1': 1 }); + const colonName = proposedName('slack:C1', router); + const slashName = proposedName('slack/C1', router); + const safeName = proposedName('slack-C1', router); + assert.match(colonName, /-g4$/); + assert.match(slashName, /-g2$/); + assert.match(safeName, /-g1$/); + assert.notEqual(safeName, 'conversation-slack-C1-g1', 'a new channel must not inherit legacy history'); + assert.deepEqual(router.exportGenerations(), { 'slack:C1': 3, 'slack/C1': 1 }); +}); + // --------------------------------------------------------------------------- // Bind policy // --------------------------------------------------------------------------- @@ -19,7 +73,7 @@ test('DM message binds without a mention (default dm bind: always)', () => { assert.equal(decision.kind, 'spawn'); if (decision.kind === 'spawn') { assert.equal(decision.generation, 1); - assert.match(decision.agentName, /^conversation-slack-D1-g1$/); + assert.equal(decision.agentName, 'conversation~slack~003aD1-g1'); assert.equal(decision.trigger, true); } }); diff --git a/test/conversation-routing.test.ts b/test/conversation-routing.test.ts index cdd3ed954..fc061c99a 100644 --- a/test/conversation-routing.test.ts +++ b/test/conversation-routing.test.ts @@ -9,6 +9,7 @@ import assert from 'node:assert/strict'; import { tmpdir } from 'node:os'; import { mkdtempSync, rmSync } from 'node:fs'; import { join } from 'node:path'; +import { ContextManager } from '@animalabs/context-manager'; import { AgentFramework } from '../src/index.js'; import type { ProcessEvent, ConversationRouterConfig, TraceEvent } from '../src/index.js'; import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js'; @@ -95,7 +96,7 @@ describe('Conversation routing', () => { })); await framework.runUntilIdle(); - const fork = framework.getAgent('conversation-slack-D1-g1'); + const fork = framework.getAgent('conversation~slack~003aD1-g1'); assert.ok(fork, 'fork agent should exist'); // Message landed in the fork's context, not the trunk's. @@ -116,23 +117,124 @@ describe('Conversation routing', () => { // Binding is live. const router = framework.getConversationRouter()!; - assert.equal(router.getBinding('slack:D1')?.agentName, 'conversation-slack-D1-g1'); + assert.equal(router.getBinding('slack:D1')?.agentName, 'conversation~slack~003aD1-g1'); await framework.stop(); }); + it('colliding channel spellings spawn distinct forks with isolated persisted history', async () => { + const channelIds = ['slack:C1', 'slack/C1', 'slack C1', 'slack-C1', 'slack~003aC1']; + const framework = await makeFramework(); + const names: string[] = []; + try { + for (const [i, channelId] of channelIds.entries()) { + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'ack' }])); + framework.pushEvent(incomingEvent({ channelId, text: `PRIVATE-CASE-${i}`, mentioned: true })); + await framework.runUntilIdle(); + } + const router = framework.getConversationRouter()!; + for (const [i, channelId] of channelIds.entries()) { + const binding = router.getBinding(channelId); + assert.ok(binding, `${channelId} needs its own live binding`); + names.push(binding.agentName); + const fork = framework.getAgent(binding.agentName); + assert.ok(fork); + assert.equal(router.channelForAgent(fork.name), channelId); + const { messages } = await fork.getContextManager().compile(); + const text = JSON.stringify(messages); + for (let j = 0; j < channelIds.length; j++) { + assert.equal(text.includes(`PRIVATE-CASE-${j}`), i === j, 'only this channel enters its fork'); + } + } + assert.equal(new Set(names).size, channelIds.length); + } finally { + await framework.stop(); + } + + // Verify the isolation in Chronicle itself, not just live Agent objects. + const reopened = await makeFramework(); + try { + for (const [i, name] of names.entries()) { + const context = await ContextManager.open({ + store: reopened.getStore(), namespace: `conversations/${name}`, isolate: true, + }); + try { + const { messages } = await context.compile(); + const text = JSON.stringify(messages); + for (let j = 0; j < channelIds.length; j++) { + assert.equal(text.includes(`PRIVATE-CASE-${j}`), i === j, 'persisted histories stay isolated'); + } + } finally { + context.close(); + } + } + } finally { + await reopened.stop(); + } + }); + + it('upgrade preserves legacy history and counters without lending that history to a new channel', async () => { + const legacyName = 'conversation-slack-C1-g1'; + const legacy = await makeFramework(); + try { + // Fixture from the old sanitizer: slack:C1 had generation 1 in this namespace. + const context = await ContextManager.open({ + store: legacy.getStore(), namespace: `conversations/${legacyName}`, isolate: true, + }); + context.addMessage('user', [{ type: 'text', text: 'LEGACY-PRIVATE-CASE' }]); + context.close(); + legacy.getStore().setStateJson('framework/conversation-router', { + generations: { 'slack:C1': 1 }, + }); + } finally { + await legacy.stop(); + } + + const framework = await makeFramework(); + try { + for (const channelId of ['slack-C1', 'slack:C1']) { + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'ack' }])); + framework.pushEvent(incomingEvent({ channelId, text: 'new engagement', mentioned: true })); + await framework.runUntilIdle(); + } + const router = framework.getConversationRouter()!; + for (const channelId of ['slack-C1', 'slack:C1']) { + const binding = router.getBinding(channelId); + assert.ok(binding); + assert.equal(binding.generation, channelId === 'slack:C1' ? 2 : 1); + const fork = framework.getAgent(binding.agentName)!; + const { messages } = await fork.getContextManager().compile(); + assert.ok(!JSON.stringify(messages).includes('LEGACY-PRIVATE-CASE'), 'old history must stay with its original namespace'); + assert.notEqual(binding.agentName, legacyName); + } + + const oldContext = await ContextManager.open({ + store: framework.getStore(), namespace: `conversations/${legacyName}`, isolate: true, + }); + try { + const { messages } = await oldContext.compile(); + assert.ok(JSON.stringify(messages).includes('LEGACY-PRIVATE-CASE'), 'legacy history remains readable'); + assert.ok(!JSON.stringify(messages).includes('new engagement'), 'legacy history remains unchanged'); + } finally { + oldContext.close(); + } + } finally { + await framework.stop(); + } + }); + it('channel message without mention is dropped; mention spawns', async () => { membrane.pushResponse(createMockResponse([{ type: 'text', text: 'On it.' }])); const framework = await makeFramework(); framework.pushEvent(incomingEvent({ channelId: 'slack:C1', text: 'just chatting' })); await framework.runUntilIdle(); - assert.equal(framework.getAgent('conversation-slack-C1-g1'), null, 'no fork without mention'); + assert.equal(framework.getAgent('conversation~slack~003aC1-g1'), null, 'no fork without mention'); assert.equal(membrane.calls.length, 0, 'no inference for unrouted messages'); framework.pushEvent(incomingEvent({ channelId: 'slack:C1', text: 'bot, help', mentioned: true })); await framework.runUntilIdle(); - assert.ok(framework.getAgent('conversation-slack-C1-g1'), 'mention spawns fork'); + assert.ok(framework.getAgent('conversation~slack~003aC1-g1'), 'mention spawns fork'); await framework.stop(); }); @@ -148,7 +250,7 @@ describe('Conversation routing', () => { framework.pushEvent(incomingEvent({ channelId: 'slack:C1', text: 'ambient detail' })); await framework.runUntilIdle(); - const fork = framework.getAgent('conversation-slack-C1-g1')!; + const fork = framework.getAgent('conversation~slack~003aC1-g1')!; const { messages } = await fork.getContextManager().compile(); assert.ok( messages.some((m) => JSON.stringify(m.content).includes('ambient detail')), @@ -171,7 +273,7 @@ describe('Conversation routing', () => { framework.pushEvent(incomingEvent({ channelId: 'slack:D2', text: 'hi', channelType: 'im' })); await framework.runUntilIdle(); - const fork = framework.getAgent('conversation-slack-D2-g1')!; + const fork = framework.getAgent('conversation~slack~003aD2-g1')!; const { messages } = await fork.getContextManager().compile(); assert.ok( messages.some((m) => JSON.stringify(m.content).includes('HANDBOOK')), @@ -190,7 +292,7 @@ describe('Conversation routing', () => { await framework.runUntilIdle(); const router = framework.getConversationRouter()!; assert.ok(router.getBinding('slack:D3'), 'spawn should leave a live binding'); - const g1 = framework.getAgent('conversation-slack-D3-g1'); + const g1 = framework.getAgent('conversation~slack~003aD3-g1'); assert.ok(g1, 'g1 fork should exist after spawn'); // Force expiry deterministically and provide the closure-turn response. @@ -208,7 +310,7 @@ describe('Conversation routing', () => { 'closure prompt should be in the fork context', ); assert.equal( - framework.getAgent('conversation-slack-D3-g1'), null, + framework.getAgent('conversation~slack~003aD3-g1'), null, 'closed fork should be disposed once its closure turn finished', ); @@ -216,7 +318,7 @@ describe('Conversation routing', () => { membrane.pushResponse(createMockResponse([{ type: 'text', text: 'hi again' }])); framework.pushEvent(incomingEvent({ channelId: 'slack:D3', text: 'back again', channelType: 'im' })); await framework.runUntilIdle(); - assert.ok(framework.getAgent('conversation-slack-D3-g2'), 'rebind spawns generation 2'); + assert.ok(framework.getAgent('conversation~slack~003aD3-g2'), 'rebind spawns generation 2'); await framework.stop(); }); @@ -230,7 +332,7 @@ describe('Conversation routing', () => { fw1.pushEvent(incomingEvent({ channelId: 'slack:D9', text: 'case one', channelType: 'im' })); await fw1.runUntilIdle(); - assert.ok(fw1.getAgent('conversation-slack-D9-g1'), 'first engagement spawns g1'); + assert.ok(fw1.getAgent('conversation~slack~003aD9-g1'), 'first engagement spawns g1'); await fw1.stop(); // Restart: same store, fresh framework. The generation counter must come @@ -240,8 +342,8 @@ describe('Conversation routing', () => { fw2.pushEvent(incomingEvent({ channelId: 'slack:D9', text: 'case two', channelType: 'im' })); await fw2.runUntilIdle(); - assert.equal(fw2.getAgent('conversation-slack-D9-g1'), null, 'g1 must not be resurrected'); - const g2 = fw2.getAgent('conversation-slack-D9-g2'); + assert.equal(fw2.getAgent('conversation~slack~003aD9-g1'), null, 'g1 must not be resurrected'); + const g2 = fw2.getAgent('conversation~slack~003aD9-g2'); assert.ok(g2, 'restart spawns the next generation, not generation 1 again'); const { messages } = await g2!.getContextManager().compile(); @@ -262,7 +364,7 @@ describe('Conversation routing', () => { const framework = await makeFramework(); framework.pushEvent(incomingEvent({ channelId: 'slack:C7', text: 'bot, help', mentioned: true })); await framework.runUntilIdle(); - const forkName = 'conversation-slack-C7-g1'; + const forkName = 'conversation~slack~003aC7-g1'; assert.ok(framework.getAgent(forkName), 'fork should exist'); const failures: Array<{ tool: string; error: string }> = []; @@ -290,7 +392,7 @@ describe('Conversation routing', () => { const framework = await makeFramework(); framework.pushEvent(incomingEvent({ channelId: 'slack:C7', text: 'bot, help', mentioned: true })); await framework.runUntilIdle(); - const forkName = 'conversation-slack-C7-g1'; + const forkName = 'conversation~slack~003aC7-g1'; assert.ok(framework.getAgent(forkName), 'fork should exist'); // tool:started is emitted AFTER the fence with the (possibly rewritten) From 78f596d94289f8f052b35c2f9bea45581fe51d9a Mon Sep 17 00:00:00 2001 From: Sage-307 Date: Fri, 2 Oct 2026 01:34:06 -0400 Subject: [PATCH 2/3] test(routing): keep namespace regression helper compatible with pure queries The proposedName helper only asks for a new fork name, so its unused route clock does not affect the assertion. Omit now from that helper so this PR composes with the separate issue #46 change that moves clock injection to touch(). Felix identified the combined TypeScript build conflict during independent review. Build and focused compiled Node tests pass: 27 pass / 0 fail. Production code is unchanged from the full-tested cf6f6e9 head. Co-Authored-By: GPT-6 Astra --- test/conversation-router.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/conversation-router.test.ts b/test/conversation-router.test.ts index cd10c6871..69108da08 100644 --- a/test/conversation-router.test.ts +++ b/test/conversation-router.test.ts @@ -14,7 +14,7 @@ function makeRouter(overrides: Partial Date: Fri, 2 Oct 2026 14:11:32 -0400 Subject: [PATCH 3/3] docs: move PR release notes to a unique changelog fragment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upstream reviewer identified conflicts among concurrent PRs that edit Unreleased directly. Felix explicitly authorized this unique fragment in Commons message 19989. Move only this PR’s own bullets verbatim, preserving existing release history and the reviewed runtime/test tree. The fragment satisfies the added-file changelog check; integration and release-fold validation follow before push. Co-Authored-By: GPT-6 Astra --- CHANGELOG.md | 4 ---- changelog.d/206-fork-namespace-identity.breaking.md | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) create mode 100644 changelog.d/206-fork-namespace-identity.breaking.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 24fe849e0..6b9a30b4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,6 @@ Releases up to and including 0.7.3 predate this file; for their contents see ## Unreleased -### Breaking - -- **Hosts using conversation forks:** Generated agent names now escape channel IDs losslessly, so distinct channels cannot share a fork name or Chronicle namespace (#44). Names use a new separator, for example `conversation~slack~003aC1-g1`, including for IDs that needed no escaping. Integrations that construct names must instead read `ConversationRouter.getBinding(channelId).agentName` after binding. Existing Chronicle history remains under its original names, and persisted per-channel generation counters continue unchanged; fresh engagements use the new names and never alias the legacy sanitizer's output for the same configured prefix. No store migration or sibling dependency upgrade is required. - ## 0.19.0 — 2026-09-28 ### Added diff --git a/changelog.d/206-fork-namespace-identity.breaking.md b/changelog.d/206-fork-namespace-identity.breaking.md new file mode 100644 index 000000000..ce54174c0 --- /dev/null +++ b/changelog.d/206-fork-namespace-identity.breaking.md @@ -0,0 +1 @@ +- **Hosts using conversation forks:** Generated agent names now escape channel IDs losslessly, so distinct channels cannot share a fork name or Chronicle namespace (#44). Names use a new separator, for example `conversation~slack~003aC1-g1`, including for IDs that needed no escaping. Integrations that construct names must instead read `ConversationRouter.getBinding(channelId).agentName` after binding. Existing Chronicle history remains under its original names, and persisted per-channel generation counters continue unchanged; fresh engagements use the new names and never alias the legacy sanitizer's output for the same configured prefix. No store migration or sibling dependency upgrade is required.