fix(channels,db): exact instance dispatch, FK-check scoping, migration-safe skill snippets

Review-round fixes on the instance dimension:
- delivery/typing resolve adapters by exact registry key, never the
  channelType fallback — a named instance with an offline adapter gets
  offline handling, not a cross-identity send through a sibling bot;
  the fallback scan (channelType-only callers) now warns when it
  resolves through a differently-keyed instance
- migration runner only fails on FK violations a migration introduced:
  pre-existing latent orphans (FK-OFF CLI surgery) are logged and
  carried, not turned into a boot crash-loop
- typing re-trigger updates the full address (channelType, platformId,
  threadId, instance) together — no torn entries on agent-shared
  sessions spanning instances
- bridge rejects empty/whitespace instance names (URL-route and
  state-namespace safety)
- add-github / add-linear SKILL.md wiring inserts include the NOT NULL
  instance column
- drop the 10s same-platform boot stagger: operational policy, not
  substrate — reintroducible skill-side for gateway-mode installs

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: gavrielc

.claude/skills/add-github/SKILL.md

@@ -111,8 +111,8 @@ Run `/manage-channels` to wire the GitHub channel to an agent group, or insert m
 
 ```sql
 -- Create messaging group (one per repo)
-INSERT INTO messaging_groups (id, channel_type, platform_id, name, is_group, unknown_sender_policy, created_at)
-VALUES ('mg-github-myrepo', 'github', 'github:owner/repo', 'owner/repo', 1, '<policy>', datetime('now'));
+INSERT INTO messaging_groups (id, channel_type, platform_id, instance, name, is_group, unknown_sender_policy, created_at)
+VALUES ('mg-github-myrepo', 'github', 'github:owner/repo', 'github', 'owner/repo', 1, '<policy>', datetime('now'));
 
 -- Wire to agent group
 INSERT INTO messaging_group_agents (id, messaging_group_id, agent_group_id, trigger_rules, response_scope, session_mode, priority, created_at)

.claude/skills/add-linear/SKILL.md

@@ -119,8 +119,8 @@ Run `/manage-channels` to wire the Linear channel to an agent group, or insert m
 
 ```sql
 -- Create messaging group (one per team)
-INSERT INTO messaging_groups (id, channel_type, platform_id, name, is_group, unknown_sender_policy, created_at)
-VALUES ('mg-linear-eng', 'linear', 'linear:ENG', 'Engineering', 1, 'public', datetime('now'));
+INSERT INTO messaging_groups (id, channel_type, platform_id, instance, name, is_group, unknown_sender_policy, created_at)
+VALUES ('mg-linear-eng', 'linear', 'linear:ENG', 'linear', 'Engineering', 1, 'public', datetime('now'));
 
 -- Wire to agent group
 INSERT INTO messaging_group_agents (id, messaging_group_id, agent_group_id, trigger_rules, response_scope, session_mode, priority, created_at)

src/channels/channel-registry.test.ts

@@ -133,10 +133,9 @@ describe('channel registry — instance keying', () => {
   afterEach(async () => {
     const { teardownChannelAdapters } = await import('./channel-registry.js');
     await teardownChannelAdapters();
-    vi.useRealTimers();
     // Drop this test's registrations so later describe blocks (which import
     // the registry without resetting) start from an empty registry instead
-    // of inheriting same-channelType pairs (and their 10s stagger).
+    // of inheriting same-channelType pairs.
     vi.resetModules();
   });
 
@@ -154,11 +153,7 @@ describe('channel registry — instance keying', () => {
     reg.registerChannelAdapter('slack-worker', { factory: () => worker });
     reg.registerChannelAdapter('slack-tester', { factory: () => tester });
 
-    vi.useFakeTimers();
-    const init = reg.initChannelAdapters(mockSetup);
-    await vi.runAllTimersAsync();
-    await init;
-    vi.useRealTimers();
+    await reg.initChannelAdapters(mockSetup);
 
     expect(reg.getChannelAdapter('slack-worker')).toBe(worker);
     expect(reg.getChannelAdapter('slack-tester')).toBe(tester);
@@ -172,11 +167,7 @@ describe('channel registry — instance keying', () => {
     reg.registerChannelAdapter('slack-tester', { factory: () => named });
     reg.registerChannelAdapter('slack', { factory: () => unnamed });
 
-    vi.useFakeTimers();
-    const init = reg.initChannelAdapters(mockSetup);
-    await vi.runAllTimersAsync();
-    await init;
-    vi.useRealTimers();
+    await reg.initChannelAdapters(mockSetup);
 
     // Exact key (default instance keyed by channelType) beats the fallback
     // scan, even though the named sibling registered first.
@@ -191,44 +182,54 @@ describe('channel registry — instance keying', () => {
     const second = createMockAdapter('slack', 'slack-worker');
     reg2.registerChannelAdapter('slack-tester', { factory: () => first });
     reg2.registerChannelAdapter('slack-worker', { factory: () => second });
-    vi.useFakeTimers();
-    const init2 = reg2.initChannelAdapters(mockSetup);
-    await vi.runAllTimersAsync();
-    await init2;
-    vi.useRealTimers();
+    await reg2.initChannelAdapters(mockSetup);
     expect(reg2.getChannelAdapter('slack')).toBe(first);
   });
 
-  it('staggers the second same-channelType adapter setup by 10s; different platforms unstaggered', async () => {
-    vi.useFakeTimers();
+  it('does NOT reroute default-instance outbound through a named sibling when the default adapter is missing', async () => {
+    // The default Slack app is offline (token rotated, factory returned
+    // null, …) while a named sibling boots fine. Outbound for the default
+    // instance must get the offline-adapter handling (drop into the retry
+    // path) — NEVER a cross-identity send through the sibling bot.
     const reg = await import('./channel-registry.js');
-    const a = createMockAdapter('slack', 'slack-a');
-    const b = createMockAdapter('slack', 'slack-b');
-    const c = createMockAdapter('discord');
-    reg.registerChannelAdapter('slack-a', { factory: () => a });
-    reg.registerChannelAdapter('slack-b', { factory: () => b });
-    reg.registerChannelAdapter('discord', { factory: () => c });
-
-    const init = reg.initChannelAdapters(mockSetup);
-
-    // Flush microtasks without advancing the clock: a sets up, b is held.
-    await vi.advanceTimersByTimeAsync(0);
-    expect(a.setupTimes).toHaveLength(1);
-    expect(b.setupTimes).toHaveLength(0);
-    expect(c.setupTimes).toHaveLength(0);
-
-    // Not yet at the stagger window.
-    await vi.advanceTimersByTimeAsync(9_000);
-    expect(b.setupTimes).toHaveLength(0);
-
-    // Crossing 10s releases b; discord (different platform) follows with
-    // no additional stagger of its own.
-    await vi.advanceTimersByTimeAsync(1_000);
-    expect(b.setupTimes).toHaveLength(1);
-    await vi.advanceTimersByTimeAsync(0);
-    expect(c.setupTimes).toHaveLength(1);
-
-    await init;
+    const tester = createMockAdapter('slack', 'slack-tester');
+    reg.registerChannelAdapter('slack-tester', { factory: () => tester });
+    reg.registerChannelAdapter('slack', { factory: () => null });
+
+    await reg.initChannelAdapters(mockSetup);
+
+    // Exact lookup (delivery/typing path): the default key resolves nothing.
+    expect(reg.getChannelAdapterExact('slack')).toBeUndefined();
+    // Fallback-capable lookup (channelType-only callers) still resolves.
+    expect(reg.getChannelAdapter('slack')).toBe(tester);
+
+    // The delivery bridge dispatches by exact key: a default-instance
+    // message (instance === channelType after backfill) is dropped, not
+    // delivered through the sibling's identity.
+    const bridge = reg.createChannelDeliveryAdapter();
+    const result = await bridge.deliver(
+      'slack',
+      'slack:C1',
+      null,
+      'chat',
+      JSON.stringify({ text: 'to the default bot' }),
+      undefined,
+      'slack',
+    );
+    expect(result).toBeUndefined();
+    expect(tester.delivered).toHaveLength(0);
+
+    // Sanity: the same bridge DOES deliver when the exact instance is live.
+    await bridge.deliver(
+      'slack',
+      'slack:C1',
+      null,
+      'chat',
+      JSON.stringify({ text: 'to the tester bot' }),
+      undefined,
+      'slack-tester',
+    );
+    expect(tester.delivered).toHaveLength(1);
   });
 });
 

src/channels/channel-registry.ts

@@ -4,7 +4,8 @@
  * Channels self-register on import. The host calls initChannelAdapters() at startup
  * to instantiate and set up all registered adapters.
  */
-import type { ChannelAdapter, ChannelRegistration, ChannelSetup } from './adapter.js';
+import type { ChannelAdapter, ChannelRegistration, ChannelSetup, OutboundFile } from './adapter.js';
+import type { ChannelDeliveryAdapter } from '../delivery.js';
 import { log } from '../log.js';
 
 const SETUP_RETRY_DELAYS_MS = [2000, 5000, 10000];
@@ -18,14 +19,6 @@ function isNetworkError(err: unknown): err is Error {
 
 const sleep = (ms: number) => new Promise<void>((resolve) => setTimeout(resolve, ms));
 
-/**
- * Two gateway instances of one platform (e.g. two Discord bots) identifying
- * simultaneously from one IP trip platform rate limits at boot. Stagger the
- * second (and later) same-platform adapter's setup. Inert for installs with
- * one adapter per platform — no two registrations share a channelType.
- */
-const SAME_CHANNEL_SETUP_STAGGER_MS = 10_000;
-
 const registry = new Map<string, ChannelRegistration>();
 const activeAdapters = new Map<string, ChannelAdapter>();
 
@@ -34,22 +27,81 @@ export function registerChannelAdapter(name: string, registration: ChannelRegist
   registry.set(name, registration);
 }
 
+/** Get a live adapter by its EXACT registry key (instance name; default
+ *  instances are keyed by channelType itself). No channelType fallback —
+ *  callers that address a specific instance (outbound delivery, typing)
+ *  must never be rerouted through a sibling instance: that would send
+ *  through the wrong bot identity with the wrong token. A missing key
+ *  means the owning adapter is offline; callers apply their normal
+ *  offline-adapter handling. */
+export function getChannelAdapterExact(key: string): ChannelAdapter | undefined {
+  return activeAdapters.get(key);
+}
+
 /** Get a live adapter by instance name, falling back to any adapter of the
- *  given channel type. channelType-only callers (user-id prefix resolution
- *  and cold DMs in user-dm.ts, approval delivery in channel-approval.ts)
- *  must still resolve when every instance of a platform is named — first
- *  registered wins (Map insertion order, deterministic). Default instances
- *  are keyed by channelType itself, so single-instance installs always hit
- *  the exact-key path. */
+ *  given channel type. The fallback exists ONLY for channelType-only callers
+ *  (user-id prefix resolution and cold DMs in user-dm.ts, approval delivery
+ *  in channel-approval.ts, the router's thread-policy probe when an event
+ *  carries no instance) — they must still resolve when every instance of a
+ *  platform is named. First registered wins (Map insertion order,
+ *  deterministic). Default instances are keyed by channelType itself, so
+ *  single-instance installs always hit the exact-key path. Instance-addressed
+ *  dispatch (delivery, typing) must use getChannelAdapterExact instead. */
 export function getChannelAdapter(key: string): ChannelAdapter | undefined {
   const exact = activeAdapters.get(key);
   if (exact) return exact;
-  for (const adapter of activeAdapters.values()) {
-    if (adapter.channelType === key) return adapter;
+  for (const [registryKey, adapter] of activeAdapters) {
+    if (adapter.channelType === key) {
+      log.warn('Channel adapter fallback: requested key resolved through a differently-keyed instance', {
+        requested: key,
+        resolvedKey: registryKey,
+      });
+      return adapter;
+    }
   }
   return undefined;
 }
 
+/**
+ * Build the host's outbound delivery bridge: dispatches delivery-poll and
+ * typing traffic into the adapter registry. Resolution is EXACT-key only —
+ * `instance ?? channelType`. For default-instance messaging_groups rows the
+ * stored instance IS the channelType, which matches default-registered
+ * adapters, so single-instance behavior is unchanged. A named instance whose
+ * adapter is offline gets the normal offline-adapter handling (warn + drop
+ * into the delivery retry path) — never a cross-identity send through a
+ * sibling bot of the same platform.
+ */
+export function createChannelDeliveryAdapter(): ChannelDeliveryAdapter {
+  return {
+    async deliver(
+      channelType: string,
+      platformId: string,
+      threadId: string | null,
+      kind: string,
+      content: string,
+      files?: OutboundFile[],
+      instance?: string,
+    ): Promise<string | undefined> {
+      const adapter = getChannelAdapterExact(instance ?? channelType);
+      if (!adapter) {
+        log.warn('No adapter for channel type', { channelType, instance });
+        return;
+      }
+      return adapter.deliver(platformId, threadId, { kind, content: JSON.parse(content), files });
+    },
+    async setTyping(
+      channelType: string,
+      platformId: string,
+      threadId: string | null,
+      instance?: string,
+    ): Promise<void> {
+      const adapter = getChannelAdapterExact(instance ?? channelType);
+      await adapter?.setTyping?.(platformId, threadId);
+    },
+  };
+}
+
 /** Get all active adapters. */
 export function getActiveAdapters(): ChannelAdapter[] {
   return [...activeAdapters.values()];
@@ -70,7 +122,6 @@ export function getChannelContainerConfig(name: string): ChannelRegistration['co
  * Skips adapters that return null (missing credentials).
  */
 export async function initChannelAdapters(setupFn: (adapter: ChannelAdapter) => ChannelSetup): Promise<void> {
-  const activeChannelTypes = new Set<string>();
   for (const [name, registration] of registry) {
     try {
       const adapter = await registration.factory();
@@ -79,17 +130,6 @@ export async function initChannelAdapters(setupFn: (adapter: ChannelAdapter) =>
         continue;
       }
 
-      // Same-platform stagger: a second instance of an already-active
-      // platform waits before identifying (gateway logins from one IP).
-      if (activeChannelTypes.has(adapter.channelType)) {
-        log.info('Staggering same-platform adapter setup', {
-          channel: name,
-          type: adapter.channelType,
-          delayMs: SAME_CHANNEL_SETUP_STAGGER_MS,
-        });
-        await sleep(SAME_CHANNEL_SETUP_STAGGER_MS);
-      }
-
       const setup = setupFn(adapter);
       // Transient network failures during adapter init (e.g. Telegram deleteWebhook
       // hitting a DNS hiccup at boot) would otherwise leave the channel permanently
@@ -125,7 +165,6 @@ export async function initChannelAdapters(setupFn: (adapter: ChannelAdapter) =>
         log.warn('Duplicate adapter instance key — overwriting previous adapter', { key, channel: name });
       }
       activeAdapters.set(key, adapter);
-      activeChannelTypes.add(adapter.channelType);
       log.info('Channel adapter started', { channel: name, type: adapter.channelType, instance: key });
     } catch (err) {
       log.error('Failed to start channel adapter', { channel: name, err });

src/channels/chat-sdk-bridge.test.ts

@@ -126,6 +126,18 @@ describe('createChatSdkBridge — instance identity', () => {
       ).toThrow(/URL-safe/);
     }
   });
+
+  it('rejects empty and whitespace-only instance names (config bug — fail loud)', () => {
+    // '' is falsy: a truthiness guard would skip it, dead-ending the
+    // webhook route ('/webhook/' + '') and collapsing the state namespace
+    // into the default instance's unprefixed keyspace — the exact
+    // cross-bot dedupe/lock collisions the namespace exists to prevent.
+    for (const bad of ['', ' ', '   ', '\t']) {
+      expect(() =>
+        createChatSdkBridge({ adapter: stubAdapter({ name: 'slack' }), instance: bad, supportsThreads: true }),
+      ).toThrow(/URL-safe/);
+    }
+  });
 });
 
 describe('createChatSdkBridge.setup — webhook route and state namespace', () => {
@@ -137,8 +149,7 @@ describe('createChatSdkBridge.setup — webhook route and state namespace', () =
     return stubAdapter({
       name: 'slack',
       initialize: async () => {},
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    } as unknown as Partial<Adapter>) as any;
+    } as unknown as Partial<Adapter>);
   }
 
   beforeEach(async () => {

src/channels/chat-sdk-bridge.ts

@@ -53,7 +53,7 @@ export interface ChatSdkBridgeConfig {
    * name. Drives the registry key, the webhook route (/webhook/<instance>),
    * and the Chat SDK state namespace. channelType is NOT affected — user
    * identity, formatting, and container config stay keyed on the platform.
-   * Must be URL-safe: no '/', '?', ':' or whitespace.
+   * Must be URL-safe: non-empty, only letters, digits, '.', '_' or '-'.
    */
   instance?: string;
   concurrency?: ConcurrencyStrategy;
@@ -133,9 +133,14 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter
   // The instance name becomes a webhook route segment (the route regex is
   // [^/?]+) and ':' is the state-namespace delimiter — reject anything that
   // would break either, at construction time rather than at first webhook.
-  if (config.instance && /[/?:\s]/.test(config.instance)) {
+  // Positive allow-list (not a deny-list): also rejects '' and
+  // whitespace-only names, which are config bugs — '' is falsy, so it
+  // would skip a truthiness guard, dead-end the webhook route, and
+  // collapse the state namespace into the default instance's keyspace.
+  if (config.instance !== undefined && !/^[A-Za-z0-9._-]+$/.test(config.instance)) {
     throw new Error(
-      `chat-sdk bridge instance ${JSON.stringify(config.instance)} must be URL-safe: no '/', '?', ':' or whitespace`,
+      `chat-sdk bridge instance ${JSON.stringify(config.instance)} must be URL-safe: ` +
+        `non-empty, only letters, digits, '.', '_' or '-'`,
     );
   }
   const transformText = (t: string): string => (config.transformOutboundText ? config.transformOutboundText(t) : t);