Commit Graph

7 Commits

Author SHA1 Message Date
Dave Kim
91c668e0cc fix: persist SDK session_id on init + split long messages before adapter truncation
Two related bugs that surfaced together when a Discord response exceeded
2000 chars:

1. **Session id lost on mid-turn container exit.** `runPollLoop` was
   calling `setStoredSessionId` only after `processQuery` returned. If
   the container died between the SDK's `init` event (where session_id
   arrives) and the stream completing, the id was never persisted. The
   next wake called `getStoredSessionId()` → undefined and started a
   fresh Claude session, dropping all prior context. Fix: persist
   immediately in the `init` branch inside `processQuery`. The existing
   post-query store becomes a harmless no-op.

2. **Silent truncation past adapter limits.** `chat-sdk-bridge.deliver`
   handed full text straight to `adapter.postMessage`. Discord's adapter
   hard-truncates at 2000 chars; Telegram's at 4096. Responses longer
   than that were cut off without any signal to the user or host. Fix:
   add `maxTextLength` to `ChatSdkBridgeConfig` and a `splitForLimit`
   helper that breaks on paragraph → line → hard-char boundaries, then
   posts chunks sequentially. Files ride on the first chunk; the
   returned id is the first chunk's so edits and reactions still target
   the reply head.

Channel adapter files (Discord, Telegram, …) live on the `channels`
branch — a companion PR wires `maxTextLength: 1900` for Discord and
`4000` for Telegram so the splitter actually engages in those installs.
Without wiring, behavior is unchanged.
2026-04-21 13:04:57 +00:00
gavrielc
a4061a0012 refactor(channels,router): move all policy to router; bridge is transport
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>
2026-04-20 13:55:49 +03:00
gavrielc
b15972284b refactor(channels): shrink bridge shouldEngage to flood gate + subscribe signal
Before this change the bridge and the router both owned engage_mode
policy. Bridge's shouldEngage had a full switch over mention /
mention-sticky / pattern + source-based rules + engage_pattern regex
test + ignored_message_policy accumulate fallback. Router's
evaluateEngage had the same switch against the same fields. Two
parallel logic paths with subtle vocabulary differences (bridge: "which
SDK handler fired"; router: "what isMention says"). Every time we
touched one we had to reason about the other — the Telegram
hasMention bug and the "pattern mode silently drops in group chats"
bug were both drift between the two.

Refactor to one place. Router keeps all per-wiring policy — engage
mode, pattern regex, sender scope, ignored-message policy — unchanged.
Bridge drops to a coarse flood gate + subscribe signal:

  - forward: does this channel have ANY wiring? Forward if yes.
    Unknown channels still forward for subscribed/mention/dm (they may
    be newly auto-created, or will trigger the coming
    channel-registration flow). Unknown channels DROP for new-message
    so we don't flood from every unsubscribed thread the bot happens
    to sit in.

  - stickySubscribe: any mention-sticky wiring on the channel AND the
    source is mention or dm. Coarse union — subscribe is idempotent
    and one call serves every sticky wiring.

The `text` param on shouldEngage is gone (pattern regex lives in the
router now). Four bridge handler sites simplify accordingly. messageToInbound
still carries the SDK-confirmed isMention flag through to the router
unchanged.

Behavioral delta: pure-mention-wired channels (no pattern, no
accumulate) will now see every plain group message reach the router
before being dropped there, where before the bridge dropped at the
transport boundary. Extra DB lookup per dropped message in this
specific case; acceptable for the cleaner seam and can be optimized
back at the bridge if it ever matters in practice.

Bridge tests prune the 10 engage_mode-specific cases that covered
logic now owned by evaluateEngage in the router (host-core.test.ts
covers it end-to-end through routeInbound). Bridge tests keep only
what's bridge-specific: the flood gate and the stickySubscribe
coarse union.

172 tests pass (was 182 — net -10 redundant bridge tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 13:32:08 +03:00
gavrielc
c38e5b11a8 fix(channels): wire accumulate mode through the bridge
The router + session DB were already fully plumbed for
ignored_message_policy='accumulate' — fan-out in routeInbound calls
deliverToAgent(wake=false) for non-engaging agents on accumulate wirings,
writeSessionMessage writes trigger=0, countDueMessages filters trigger=1,
container formatter includes all messages regardless of trigger. But the
Chat SDK bridge dropped non-engaging messages before the router ever saw
them, so accumulate was dead on arrival for every adapter that goes
through the bridge.

Expose ignored_message_policy on ConversationConfig, project it in
buildConversationConfigs, and widen shouldEngage's "forward" decision to
"engage OR accumulate" with the union taken across all wirings on a
conversation. stickySubscribe stays gated on a real engage — subscribing
a thread we'd only silently accumulate on would misrepresent the bot's
presence.

shouldEngage return shape is now { forward, stickySubscribe } — engage
was an internal concept the caller never needed, and conflating it with
forward was the source of this bug.

7 new tests cover: non-engaging messages forwarding under accumulate,
mixed drop/accumulate wirings taking the union, accumulate not
triggering sticky subscribe, unknown-conversation drop precedence over
accumulate, and drop policy preserving existing behavior on engaging
messages.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 11:18:43 +03:00
gavrielc
ce25e1e97c style(channels): prettier line-wrap in chat-sdk-bridge.test.ts
Post-commit reformat picked up by format:fix hook on the previous commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 11:12:40 +03:00
gavrielc
52c6223292 fix(channels): register onNewMessage(/./) to fix pattern mode in group chats
Chat SDK dispatch (per handling-events.mdx) is exclusive and prioritized:
subscribed → onSubscribedMessage; unsubscribed + mention → onNewMention;
unsubscribed + pattern match → onNewMessage. We never registered the third,
so engage_mode='pattern' silently dropped every message in unsubscribed
group threads — the SDK simply never surfaced them anywhere.

Register chat.onNewMessage(/./, …) and route it through shouldEngage with
a new 'new-message' source. Unknown-conversation policy drops for this
source (would otherwise flood from every unwired channel the bot can see).
mention / mention-sticky wirings ignore 'new-message' — they require an
explicit @mention to start a conversation. Pattern wirings evaluate
normally.

Extracted shouldEngage from a closure to an exported function with an
EngageSource type so it's unit-testable. Added 17 tests covering every
source × engage-mode combination, unknown-conversation behavior, invalid
regex fail-open, and multi-wiring union.

Accumulate (ignored_message_policy='accumulate') is still not plumbed —
the bridge drops non-engaging messages entirely instead of forwarding
them as context-only. That requires a trigger: 0 | 1 field on
InboundMessage → router → writeSessionMessage (schema already has the
column). Separate change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 11:11:56 +03:00
gavrielc
27c52205f9 fix(channels): bridge openDM delegates to adapter directly
chat.openDM dispatches via inferAdapterFromUserId, which only recognizes
Discord/Slack/Teams/gChat formats and throws for everything else —
breaking approval delivery on Telegram (numeric IDs) and the other
direct-addressable channels the bridge now wraps. Delegate straight to
adapter.openDM + channelIdFromThreadId, and only expose openDM when the
underlying adapter implements it. Preserves the adapter's native
platform_id encoding (e.g. "telegram:<chatId>") so user_dms caches align
with the messaging_groups rows onInbound wrote.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-17 18:30:38 +03:00