diff --git a/src/channels/adapter.ts b/src/channels/adapter.ts index 33f382548..34b367555 100644 --- a/src/channels/adapter.ts +++ b/src/channels/adapter.ts @@ -22,6 +22,20 @@ export interface ConversationConfig { engageMode: 'pattern' | 'mention' | 'mention-sticky'; /** Regex source when engageMode='pattern'. '.' is the "always" sentinel. */ engagePattern?: string | null; + /** + * What to do with messages this wiring doesn't engage on. + * + * 'drop' — discard silently + * 'accumulate' — still forward to the host so the router can store the + * message in this agent's session with trigger=0. It + * rides along as context when the agent next wakes, but + * doesn't wake it on its own. + * + * The bridge reads this to decide whether to forward a non-engaging + * message at all — if any wiring on a conversation has 'accumulate', the + * bridge forwards and lets the router apply the per-wiring decision. + */ + ignoredMessagePolicy?: 'drop' | 'accumulate'; sessionMode: 'shared' | 'per-thread' | 'agent-shared'; } diff --git a/src/channels/chat-sdk-bridge.test.ts b/src/channels/chat-sdk-bridge.test.ts index 667fc7f74..aad8d0a03 100644 --- a/src/channels/chat-sdk-bridge.test.ts +++ b/src/channels/chat-sdk-bridge.test.ts @@ -17,6 +17,7 @@ function cfg( agentGroupId: partial.agentGroupId ?? 'ag-1', engageMode: partial.engageMode, engagePattern: partial.engagePattern ?? null, + ignoredMessagePolicy: partial.ignoredMessagePolicy ?? 'drop', sessionMode: partial.sessionMode ?? 'shared', }; } @@ -66,26 +67,26 @@ describe('shouldEngage', () => { const sources: EngageSource[] = ['subscribed', 'mention', 'dm']; for (const source of sources) { it(`forwards for source='${source}' (may be a not-yet-wired group)`, () => { - expect(shouldEngage(empty, 'C1', source, '')).toEqual({ engage: true, stickySubscribe: false }); + expect(shouldEngage(empty, 'C1', source, '')).toEqual({ forward: true, stickySubscribe: false }); }); } it("DROPS for source='new-message' (would flood from unwired channels)", () => { expect(shouldEngage(empty, 'C1', 'new-message', 'hello')).toEqual({ - engage: false, + forward: false, stickySubscribe: false, }); }); }); - describe("engageMode='mention'", () => { + describe("engageMode='mention' + ignoredMessagePolicy='drop' (default)", () => { const conv = mapFor(cfg({ engageMode: 'mention' })); - it('engages on mention + dm', () => { - expect(shouldEngage(conv, 'C1', 'mention', '').engage).toBe(true); - expect(shouldEngage(conv, 'C1', 'dm', '').engage).toBe(true); + it('forwards on mention + dm', () => { + expect(shouldEngage(conv, 'C1', 'mention', '').forward).toBe(true); + expect(shouldEngage(conv, 'C1', 'dm', '').forward).toBe(true); }); - it('does NOT engage on subscribed or new-message (prevents keep-firing + flooding)', () => { - expect(shouldEngage(conv, 'C1', 'subscribed', '').engage).toBe(false); - expect(shouldEngage(conv, 'C1', 'new-message', '').engage).toBe(false); + it('does NOT forward on subscribed or new-message (prevents keep-firing + flooding)', () => { + expect(shouldEngage(conv, 'C1', 'subscribed', '').forward).toBe(false); + expect(shouldEngage(conv, 'C1', 'new-message', '').forward).toBe(false); }); it('never asks to subscribe', () => { for (const s of ['subscribed', 'mention', 'dm', 'new-message'] as EngageSource[]) { @@ -96,37 +97,37 @@ describe('shouldEngage', () => { describe("engageMode='mention-sticky'", () => { const conv = mapFor(cfg({ engageMode: 'mention-sticky' })); - it('engages on mention + dm with stickySubscribe=true', () => { - expect(shouldEngage(conv, 'C1', 'mention', '')).toEqual({ engage: true, stickySubscribe: true }); - expect(shouldEngage(conv, 'C1', 'dm', '')).toEqual({ engage: true, stickySubscribe: true }); + it('forwards on mention + dm with stickySubscribe=true', () => { + expect(shouldEngage(conv, 'C1', 'mention', '')).toEqual({ forward: true, stickySubscribe: true }); + expect(shouldEngage(conv, 'C1', 'dm', '')).toEqual({ forward: true, stickySubscribe: true }); }); - it('engages on subscribed follow-ups without re-subscribing', () => { - expect(shouldEngage(conv, 'C1', 'subscribed', '')).toEqual({ engage: true, stickySubscribe: false }); + it('forwards subscribed follow-ups without re-subscribing', () => { + expect(shouldEngage(conv, 'C1', 'subscribed', '')).toEqual({ forward: true, stickySubscribe: false }); }); - it('does NOT engage on new-message (explicit mention required to start)', () => { - expect(shouldEngage(conv, 'C1', 'new-message', '').engage).toBe(false); + it('does NOT forward on new-message (explicit mention required to start)', () => { + expect(shouldEngage(conv, 'C1', 'new-message', '').forward).toBe(false); }); }); describe("engageMode='pattern'", () => { - it('pattern="." engages on every source except new-message-with-unknown', () => { + it('pattern="." forwards on every source (when conversation is known)', () => { const conv = mapFor(cfg({ engageMode: 'pattern', engagePattern: '.' })); for (const s of ['subscribed', 'mention', 'dm', 'new-message'] as EngageSource[]) { - expect(shouldEngage(conv, 'C1', s, 'anything').engage).toBe(true); + expect(shouldEngage(conv, 'C1', s, 'anything').forward).toBe(true); } }); it('tests regex against text on new-message (the main bug fix)', () => { const conv = mapFor(cfg({ engageMode: 'pattern', engagePattern: '^!report' })); - expect(shouldEngage(conv, 'C1', 'new-message', '!report now').engage).toBe(true); - expect(shouldEngage(conv, 'C1', 'new-message', 'nothing to see').engage).toBe(false); + expect(shouldEngage(conv, 'C1', 'new-message', '!report now').forward).toBe(true); + expect(shouldEngage(conv, 'C1', 'new-message', 'nothing to see').forward).toBe(false); }); it('pattern regex applies on every source (symmetry)', () => { const conv = mapFor(cfg({ engageMode: 'pattern', engagePattern: 'deploy' })); for (const s of ['subscribed', 'mention', 'dm', 'new-message'] as EngageSource[]) { - expect(shouldEngage(conv, 'C1', s, 'time to deploy').engage).toBe(true); - expect(shouldEngage(conv, 'C1', s, 'weather today').engage).toBe(false); + expect(shouldEngage(conv, 'C1', s, 'time to deploy').forward).toBe(true); + expect(shouldEngage(conv, 'C1', s, 'weather today').forward).toBe(false); } }); @@ -139,7 +140,50 @@ describe('shouldEngage', () => { it('invalid regex fails open (admin sees something rather than silent drop)', () => { const conv = mapFor(cfg({ engageMode: 'pattern', engagePattern: '[unclosed' })); - expect(shouldEngage(conv, 'C1', 'new-message', 'x').engage).toBe(true); + expect(shouldEngage(conv, 'C1', 'new-message', 'x').forward).toBe(true); + }); + }); + + describe("ignoredMessagePolicy='accumulate'", () => { + it('forwards non-engaging new-message so the router can store it as context (trigger=0)', () => { + const conv = mapFor(cfg({ engageMode: 'mention', ignoredMessagePolicy: 'accumulate' })); + // Plain message in unsubscribed group — mention rule says no engage, + // but accumulate says forward anyway. + expect(shouldEngage(conv, 'C1', 'new-message', 'chit chat')).toEqual({ + forward: true, + stickySubscribe: false, + }); + }); + + it('forwards non-engaging subscribed messages for accumulation', () => { + // mention wiring in a subscribed thread: the mention handler already + // fired once, thread is now subscribed, follow-ups route here. The + // base 'mention' rule wouldn't engage without an @-mention, but + // accumulate says capture the context. + const conv = mapFor(cfg({ engageMode: 'mention', ignoredMessagePolicy: 'accumulate' })); + expect(shouldEngage(conv, 'C1', 'subscribed', 'follow up talk').forward).toBe(true); + }); + + it('does NOT set stickySubscribe purely from accumulate (avoid misleading bot presence)', () => { + const conv = mapFor(cfg({ engageMode: 'mention-sticky', ignoredMessagePolicy: 'accumulate' })); + expect(shouldEngage(conv, 'C1', 'new-message', 'plain').stickySubscribe).toBe(false); + }); + + it("accumulate doesn't override the 'unknown conversation → drop new-message' rule", () => { + // Unknown conversation (not in map): accumulate can't be read because + // there's no config to read from. We still drop. + const empty = new Map(); + expect(shouldEngage(empty, 'C-unknown', 'new-message', 'x').forward).toBe(false); + }); + + it("drop policy + non-engaging message → doesn't forward", () => { + const conv = mapFor(cfg({ engageMode: 'mention', ignoredMessagePolicy: 'drop' })); + expect(shouldEngage(conv, 'C1', 'new-message', 'plain').forward).toBe(false); + }); + + it('engaging message with drop policy still forwards (engage wins regardless)', () => { + const conv = mapFor(cfg({ engageMode: 'mention', ignoredMessagePolicy: 'drop' })); + expect(shouldEngage(conv, 'C1', 'mention', '@bot hi').forward).toBe(true); }); }); @@ -152,8 +196,17 @@ describe('shouldEngage', () => { cfg({ agentGroupId: 'ag-a', engageMode: 'mention' }), cfg({ agentGroupId: 'ag-b', engageMode: 'pattern', engagePattern: '^hi' }), ); - expect(shouldEngage(conv, 'C1', 'new-message', 'hi there').engage).toBe(true); - expect(shouldEngage(conv, 'C1', 'new-message', 'something else').engage).toBe(false); + expect(shouldEngage(conv, 'C1', 'new-message', 'hi there').forward).toBe(true); + expect(shouldEngage(conv, 'C1', 'new-message', 'something else').forward).toBe(false); + }); + + it('any accumulate wiring causes forward even if all others would drop', () => { + const conv = mapFor( + cfg({ agentGroupId: 'ag-a', engageMode: 'mention', ignoredMessagePolicy: 'drop' }), + cfg({ agentGroupId: 'ag-b', engageMode: 'mention', ignoredMessagePolicy: 'accumulate' }), + ); + // Plain message: ag-a would drop, ag-b would accumulate → forward. + expect(shouldEngage(conv, 'C1', 'new-message', 'plain').forward).toBe(true); }); it('stickySubscribe from any mention-sticky wiring wins', () => { @@ -161,7 +214,7 @@ describe('shouldEngage', () => { cfg({ agentGroupId: 'ag-a', engageMode: 'mention' }), cfg({ agentGroupId: 'ag-b', engageMode: 'mention-sticky' }), ); - expect(shouldEngage(conv, 'C1', 'mention', '')).toEqual({ engage: true, stickySubscribe: true }); + expect(shouldEngage(conv, 'C1', 'mention', '')).toEqual({ forward: true, stickySubscribe: true }); }); }); }); diff --git a/src/channels/chat-sdk-bridge.ts b/src/channels/chat-sdk-bridge.ts index f2daf1166..aa980abf0 100644 --- a/src/channels/chat-sdk-bridge.ts +++ b/src/channels/chat-sdk-bridge.ts @@ -85,21 +85,28 @@ export interface ChatSdkBridgeConfig { export type EngageSource = 'subscribed' | 'mention' | 'dm' | 'new-message'; /** - * Should a message from (channelId, source, text) engage any of the wired - * agents on this conversation? + * Should a message from (channelId, source, text) be forwarded to the host, + * and if so, should the bridge subscribe the thread? * * Exported for testability — see `chat-sdk-bridge.test.ts`. * - * We take the union across wired agents: if any wiring would engage, the - * message is forwarded. Per-agent filtering after that happens in the host - * router (see `src/router.ts` pickAgents). + * We take the union across wired agents: if any wiring would engage OR any + * wiring has `ignoredMessagePolicy='accumulate'`, the message is forwarded. + * The host router then does the per-wiring decision in `deliverToAgent` — + * engaging agents get `trigger=1` (wake), accumulating agents get + * `trigger=0` (store as context, don't wake), drop-policy agents are + * skipped (see `src/router.ts` routeInbound fan-out). + * + * `stickySubscribe` is only set when an actual engage happens (not just + * accumulate) — subscribing a thread we'd only silently accumulate on would + * misrepresent the bot's presence to other users. */ export function shouldEngage( conversations: Map, channelId: string, source: EngageSource, text: string, -): { engage: boolean; stickySubscribe: boolean } { +): { forward: boolean; stickySubscribe: boolean } { const configs = conversations.get(channelId); // Unknown conversation — behavior diverges by source: @@ -112,28 +119,30 @@ export function shouldEngage( // the bot is merely *present* in but not wired to. Forwarding // everything would flood the host. if (!configs || configs.length === 0) { - return { engage: source !== 'new-message', stickySubscribe: false }; + return { forward: source !== 'new-message', stickySubscribe: false }; } let engage = false; + let accumulate = false; let stickySubscribe = false; for (const cfg of configs) { + let cfgEngages = false; switch (cfg.engageMode) { case 'mention': - if (source === 'mention' || source === 'dm') engage = true; + if (source === 'mention' || source === 'dm') cfgEngages = true; break; case 'mention-sticky': if (source === 'mention' || source === 'dm') { - engage = true; + cfgEngages = true; stickySubscribe = true; } else if (source === 'subscribed') { // Thread was already subscribed on a prior mention — treat as // engage-all so follow-ups in the thread reach the agent. - engage = true; + cfgEngages = true; } - // source='new-message' → do not engage. mention-sticky requires an - // explicit mention to start the conversation. + // source='new-message' → does not engage (requires explicit mention + // to start). Accumulate policy is evaluated below if set. break; case 'pattern': { // Pattern evaluates on any source that delivers a plain message — @@ -143,19 +152,27 @@ export function shouldEngage( // only fire on mentions whose text contains 'foo'. const pattern = cfg.engagePattern ?? '.'; try { - if (pattern === '.' || new RegExp(pattern).test(text)) engage = true; + if (pattern === '.' || new RegExp(pattern).test(text)) cfgEngages = true; } catch { // Invalid regex → fail open so the admin can see something is // happening and fix the pattern. - engage = true; + cfgEngages = true; } break; } } - if (engage && stickySubscribe) break; + + if (cfgEngages) { + engage = true; + } else if (cfg.ignoredMessagePolicy === 'accumulate') { + // Wiring doesn't engage on this message but wants it captured as + // context for its session — forward so the router can write it with + // trigger=0. + accumulate = true; + } } - return { engage, stickySubscribe }; + return { forward: engage || accumulate, stickySubscribe }; } export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter { @@ -189,7 +206,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter channelId: string, source: EngageSource, text: string, - ): { engage: boolean; stickySubscribe: boolean } { + ): { forward: boolean; stickySubscribe: boolean } { return shouldEngage(conversations, channelId, source, text); } @@ -278,7 +295,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter const channelId = adapter.channelIdFromThreadId(thread.id); const text = typeof message.text === 'string' ? message.text : ''; const decision = engageDecision(channelId, 'subscribed', text); - if (!decision.engage) return; + if (!decision.forward) return; await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message)); }); @@ -288,7 +305,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter const channelId = adapter.channelIdFromThreadId(thread.id); const text = typeof message.text === 'string' ? message.text : ''; const decision = engageDecision(channelId, 'mention', text); - if (!decision.engage) return; + if (!decision.forward) return; await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message)); if (decision.stickySubscribe) { await thread.subscribe(); @@ -312,9 +329,9 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter channelId, sender: (message.author as any)?.fullName ?? (message.author as any)?.userId ?? 'unknown', threadId: thread.id, - engage: decision.engage, + forward: decision.forward, }); - if (!decision.engage) return; + if (!decision.forward) return; await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message)); if (decision.stickySubscribe) { await thread.subscribe(); @@ -339,7 +356,7 @@ export function createChatSdkBridge(config: ChatSdkBridgeConfig): ChannelAdapter const channelId = adapter.channelIdFromThreadId(thread.id); const text = typeof message.text === 'string' ? message.text : ''; const decision = engageDecision(channelId, 'new-message', text); - if (!decision.engage) return; + if (!decision.forward) return; await setupConfig.onInbound(channelId, thread.id, await messageToInbound(message)); }); diff --git a/src/index.ts b/src/index.ts index 9bb51bebb..4958eef74 100644 --- a/src/index.ts +++ b/src/index.ts @@ -163,6 +163,7 @@ function buildConversationConfigs(channelType: string): ConversationConfig[] { agentGroupId: agent.agent_group_id, engageMode: agent.engage_mode, engagePattern: agent.engage_pattern, + ignoredMessagePolicy: agent.ignored_message_policy, sessionMode: agent.session_mode, }); }