wangjunxiao/nanoclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(channels,router): move all policy to router; bridge is transport

Browse Source 
Follow-up to b159722. That shrank the bridge's shouldEngage to a flood
gate + coarse sticky-subscribe signal. This completes the move —
policy lives exclusively in the router, the bridge is transport-only,
and the conversations map + ChannelSetup.conversations +
ChannelAdapter.updateConversations are all gone.

Key shifts:

1. Subscribe moves from bridge to router.

   Bridge used to call `thread.subscribe()` from its onNewMention /
   onDirectMessage handlers based on a coarse "any mention-sticky wiring
   exists on this channel" check. That forced the decision before the
   router could apply per-wiring engage logic, and it relied on the
   conversations map being current (staleness risk).

   ChannelAdapter gains `subscribe?(platformId, threadId)`. The Chat
   SDK bridge implements it via SqliteStateAdapter.subscribe(threadId)
   (idempotent — a repeat call on an already-subscribed thread is a
   no-op). The router's fan-out loop calls it once per message when
   the first mention-sticky wiring actually engages. Precise, not
   coarse.

2. Short-circuit the drop path with one combined query.

   New `getMessagingGroupWithAgentCount(channelType, platformId)` does
   the messaging_groups lookup AND counts wirings in a single SELECT,
   using the existing UNIQUE(channel_type, platform_id) index on
   messaging_groups and UNIQUE(messaging_group_id, agent_group_id) on
   messaging_group_agents for the JOIN. No new indexes needed.

   routeInbound now short-circuits:
     - No messaging_groups row AND not addressed (no mention/DM)
       → return silently. One DB read, nothing written. This is the
       Discord-bot-in-a-big-guild case; we no longer auto-create rows
       for every plain message in every channel the bot can see.
     - Messaging group exists but no wirings AND not addressed
       → return silently. One DB read.
     - Otherwise fall through to sender resolution + fan-out as before.

   Behavioral change: plain chatter on unwired channels no longer gets
   dropped_messages audit rows, which used to bloat the table. Audit
   still fires on addressed-to-bot drops where the admin cares
   ("someone @-mentioned us but nobody's wired").

3. Bridge is now purely transport.

   Deleted entirely: ConversationConfig, ChannelSetup.conversations,
   ChannelAdapter.updateConversations?, bridge's `conversations` map,
   buildConversationMap, shouldEngage, EngageSource, engageDecision,
   bridge.updateConversations method, src/index.ts
   buildConversationConfigs. Four handlers reduce to "resolve channel
   id, build InboundMessage with isMention, call onInbound". Net
   ~130 LOC deleted from the bridge.

   Collateral: the conversations-map staleness problem is gone. The
   upcoming channel-registration feature doesn't need any map-refresh
   plumbing — when an approval creates a new wiring, the next message
   hits the DB fresh and just works.

Bridge tests prune to the narrow platform-adjacent surface (openDM
delegation, subscribe presence). Host-core test that asserted the
old "auto-create on every unknown message" behavior updates to
reflect the new escalation-gated semantics: plain messages on
unknown channels don't auto-create, mentions do.

159 tests pass (was 172 — net -13, almost entirely from
bridge-engage-mode tests that covered logic now owned by the router
and exercised through host-core.test.ts).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gavrielc
2026-04-20 13:55:49 +03:00
parent b15972284b
commit a4061a0012
8 changed files with 173 additions and 309 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
src/channels/chat-sdk-bridge.test.ts
View File

@@ -2,37 +2,19 @@ import { describe, expect, it } from 'vitest';
import type { Adapter } from 'chat';
import type { ConversationConfig } from './adapter.js';
import { createChatSdkBridge, shouldEngage, type EngageSource } from './chat-sdk-bridge.js';
import { createChatSdkBridge } from './chat-sdk-bridge.js';
function stubAdapter(partial: Partial<Adapter>): Adapter {
return { name: 'stub', ...partial } as unknown as Adapter;
}
function cfg(
partial: Partial<ConversationConfig> & { engageMode: ConversationConfig['engageMode'] },
): ConversationConfig {
return {
platformId: partial.platformId ?? 'C1',
agentGroupId: partial.agentGroupId ?? 'ag-1',
engageMode: partial.engageMode,
engagePattern: partial.engagePattern ?? null,
ignoredMessagePolicy: partial.ignoredMessagePolicy ?? 'drop',
sessionMode: partial.sessionMode ?? 'shared',
};
}
function mapFor(...configs: ConversationConfig[]): Map<string, ConversationConfig[]> {
const map = new Map<string, ConversationConfig[]>();
for (const c of configs) {
const existing = map.get(c.platformId);
if (existing) existing.push(c);
else map.set(c.platformId, [c]);
}
return map;
}
describe('createChatSdkBridge', () => {
// The bridge is now transport-only: forward inbound events, relay outbound
// ops. All per-wiring engage / accumulate / drop / subscribe decisions live
// in the router (src/router.ts routeInbound / evaluateEngage) and are
// exercised by host-core.test.ts end-to-end. These tests only cover the
// bridge's narrow, platform-adjacent surface.
it('omits openDM when the underlying Chat SDK adapter has none', () => {
const bridge = createChatSdkBridge({
adapter: stubAdapter({}),
@@ -59,76 +41,12 @@ describe('createChatSdkBridge', () => {
expect(openDMCalls).toEqual(['user-42']);
expect(platformId).toBe('stub:user-42');
});
});
describe('shouldEngage (bridge-level flood gate + subscribe signal)', () => {
// Per-wiring engage_mode / engage_pattern / ignored_message_policy
// semantics live in the router (evaluateEngage / routeInbound fan-out).
// These tests only cover the bridge's two responsibilities: should we
// forward at all, and should we call thread.subscribe().
describe('flood gate — unknown conversation', () => {
const empty = new Map<string, ConversationConfig[]>();
const carriedSources: EngageSource[] = ['subscribed', 'mention', 'dm'];
for (const source of carriedSources) {
it(`forwards for source='${source}' (may be a newly-auto-created channel or a channel-registration trigger)`, () => {
expect(shouldEngage(empty, 'C-new', source)).toEqual({ forward: true, stickySubscribe: false });
});
}
it("DROPS for source='new-message' (onNewMessage(/./) fires for every unsubscribed thread the bot can see — would flood)", () => {
expect(shouldEngage(empty, 'C-unwired', 'new-message')).toEqual({
forward: false,
stickySubscribe: false,
});
});
});
describe('known conversation — bridge forwards regardless of engage mode', () => {
// Policy lives in the router now. The bridge only knows "has any wiring".
const conv = mapFor(cfg({ engageMode: 'mention' }));
for (const source of ['subscribed', 'mention', 'dm', 'new-message'] as EngageSource[]) {
it(`forwards for source='${source}' — router will decide engage / accumulate / drop per wiring`, () => {
expect(shouldEngage(conv, 'C1', source).forward).toBe(true);
});
}
});
describe('stickySubscribe signal', () => {
it('true when any mention-sticky wiring exists AND source is mention', () => {
const conv = mapFor(cfg({ engageMode: 'mention-sticky' }));
expect(shouldEngage(conv, 'C1', 'mention').stickySubscribe).toBe(true);
});
it('true when any mention-sticky wiring exists AND source is dm', () => {
const conv = mapFor(cfg({ engageMode: 'mention-sticky' }));
expect(shouldEngage(conv, 'C1', 'dm').stickySubscribe).toBe(true);
});
it('false on subscribed — thread is already subscribed, no need to re-subscribe', () => {
const conv = mapFor(cfg({ engageMode: 'mention-sticky' }));
expect(shouldEngage(conv, 'C1', 'subscribed').stickySubscribe).toBe(false);
});
it('false on new-message — mention-sticky requires an explicit mention to start', () => {
const conv = mapFor(cfg({ engageMode: 'mention-sticky' }));
expect(shouldEngage(conv, 'C1', 'new-message').stickySubscribe).toBe(false);
});
it('false for plain mention / pattern wirings (not sticky)', () => {
const mentionConv = mapFor(cfg({ engageMode: 'mention' }));
const patternConv = mapFor(cfg({ engageMode: 'pattern', engagePattern: '.' }));
for (const s of ['subscribed', 'mention', 'dm', 'new-message'] as EngageSource[]) {
expect(shouldEngage(mentionConv, 'C1', s).stickySubscribe).toBe(false);
expect(shouldEngage(patternConv, 'C1', s).stickySubscribe).toBe(false);
}
});
it('fires on coarse union — mixed wirings where any one is mention-sticky', () => {
const conv = mapFor(
cfg({ agentGroupId: 'ag-a', engageMode: 'mention' }),
cfg({ agentGroupId: 'ag-b', engageMode: 'mention-sticky' }),
);
expect(shouldEngage(conv, 'C1', 'mention').stickySubscribe).toBe(true);
it('exposes subscribe (lets the router initiate thread subscription on mention-sticky engage)', () => {
const bridge = createChatSdkBridge({
adapter: stubAdapter({}),
supportsThreads: true,
});
expect(typeof bridge.subscribe).toBe('function');
});
});