fix(routing): isolate fork namespaces by raw channel identity

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.

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}`;
   }
 }

test/conversation-router.test.ts

@@ -9,6 +9,60 @@ function makeRouter(overrides: Partial<ConstructorParameters<typeof Conversation
   return new ConversationRouter({ templateAgent: 'trunk', ...overrides });
 }
 
+// ---------------------------------------------------------------------------
+// Fork namespace identity
+// ---------------------------------------------------------------------------
+
+function proposedName(channelId: string, router = makeRouter()): string {
+  const decision = router.route({ channelId, mentioned: true, kind: 'channel' });
+  assert.equal(decision.kind, 'spawn');
+  if (decision.kind !== 'spawn') assert.fail('expected spawn');
+  return decision.agentName;
+}
+
+test('fork names distinguish raw channel IDs, including escape-like and Unicode IDs', () => {
+  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);
   }
 });

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,15 +310,15 @@ 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',
     );
 
     // Next DM spawns a fresh generation.
     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)