Repository navigation
Closed
Labels
bugSomething isn't workingSomething isn't working
Description
Context
Review feedback on PR #2 reported that inbound Bot Social messages can be lost, duplicated, or left without a reply route. The code at 7b806c6 still has all of these problems. Related to #5 (social design); this issue tracks the delivery bugs only.
Original feedback
- @hsliuustc0106 review (permalink), P1 item 5: "
pollInboxadvances the inbox cursor before delivering the batch; any mid-batch throw (hidden by the timer's.catch(() => {})) permanently skips the failed event and everything after it. A middlewaredenyis also counted as delivered." - Codex inline comment, Advance the inbox cursor only after delivering the batch (permalink),
ds-bot/plugins/social/src/social.jsL237. - Review item 6 + Codex inline Route normal replies back to remote senders (permalink),
ds-bot/src/host/hooks.js(orig. L331): inbound remote messages carry no return address, so the normal turn-end reply fails withUnknown Bot undefinedand is hidden. - Review item 7: inbox cursors are in-memory only. A restart replays retained history, overlapping polls double-deliver, and there is no event-id dedup or single-flight guard.
- Review P2 "Legacy path": the
start_conversationconversation id is never cached, so a file send right after opening a legacy conversation fails.
Where (at 7b806c6)
plugins/social/src/social.js#L227-L237:const cursors = new Map();cursors.set(key, result.next_cursor)runs before thefor (const event of result.events)delivery loop.src/host/delivery.js#L61:senderSessionIdis set only whenfromIdis a local Bot.plugins/social/src/social.js#L215-L222: thestart_conversationresult's conversation id is not stored on the bot.
Proposed fix
- Process events one at a time with per-event try/catch, and advance or persist the cursor only after each event is handled. Do not count a
denyas delivered (log it or surface it). - Persist cursors per bot (for example under the social state file) and dedup by event id. Add a single-flight guard so polls cannot overlap.
- Give remote-origin turns a routable return address (for example a
remotesource carrying the handle) soturns.jsroutes the turn-end reply throughbot.sendto the remote transport. Otherwise, do not mark these turns asrequest. - Cache
conversation_idfromstart_conversation.
Acceptance criteria
- A throw while delivering event n does not skip events n+1..; the failed event is retried on the next poll.
- Restarting the host does not redeliver already-delivered events. Two concurrent polls never deliver the same event twice.
- A local Bot's ordinary reply to an inbound remote message reaches the remote Bot, with no
Unknown Bot undefined. - A file send right after a new legacy conversation succeeds.
- Tests cover each case.
Refs: PR #2 (merged), umbrella #3. Verified against Birfy/FlyBot flybot-plugins tip 7b806c6.
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working
Activity
Social inbox: per-event cursor, persisted, deduped, single-flight; de…
Merge pull request #22 from Birfy/flybot-plugins