Skip to content

feat(check-inbox): watermark to prevent stale message re-delivery#199

Open
fujibee wants to merge 2 commits into
mainfrom
check-inbox-watermark-pr171
Open

feat(check-inbox): watermark to prevent stale message re-delivery#199
fujibee wants to merge 2 commits into
mainfrom
check-inbox-watermark-pr171

Conversation

@fujibee

@fujibee fujibee commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Rebased proxy for #171 by @tayhiga-prog onto current main (commits de63c78 + 796f79d, authorship preserved). Adds a watermark so the Stop-hook inbox check doesn't re-surface already-seen messages. Supersedes #171 (slipped from 1.1.0 on a CI miss; rebases cleanly now). Note: no dedicated bats yet — reviewer to decide if one is required. Targets 1.1.1.

tayhiga-prog and others added 2 commits June 22, 2026 17:54
Address review feedback from fujibee on PR #171:

1. Seed the watermark from the latest *read* message for this agent
   (WHERE to_agent='$AGENT' AND read_at IS NOT NULL) instead of the
   global MAX(id). This ensures the first turn-mode check drains
   current unread messages, fixing the three CI test failures where
   a just-sent message was skipped. The safety-valve clamp is also
   scoped to to_agent.

2. Add `| tr -d '\r'` to agmsg_sqlite() (on-disk wrapper), mirroring
   agmsg_sqlite_mem(). On Windows, sqlite3.exe emits \r\n; without
   stripping, the watermark integer validation rejects the value and
   the watermark silently resets to 0 on every check.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@fujibee

fujibee commented Jun 23, 2026

Copy link
Copy Markdown
Owner Author

Thanks — the watermark design (seed from each agent's latest read message, drain once, then gate) is sound. One change before merge: the | tr -d '\r' added to agmsg_sqlite() in storage.sh is too broad. agmsg_sqlite streams message bodies (inbox/history/watch/check-inbox), so stripping CR there removes legitimate carriage returns inside user messages — #180 deliberately confined CR-stripping to agmsg_sqlite_mem for this reason. Please revert agmsg_sqlite and scope the strip to just the scalar watermark captures (a small agmsg_sqlite_scalar helper, or a local in check-inbox).

Please also add bats coverage: (1) unread newer than the latest-read id is delivered and advances the watermark; (2) the same unread isn't re-delivered even if a read_at update fails; (3) clamp on DB recreate / id shrink; (4) a CR-containing body round-trips intact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants