fix(mcp): room_ack clears only consumed lines, not the whole file#32
Conversation
room_ack blanked the entire notification file, so anything the poller
appended between the agent's last room_list_new read and the ack was
wiped unread (2026-07-08 incident: an owner instruction sat 5 hours
unseen in that window).
Fix: room_list_new remembers the exact bytes it returned (per-session,
keyed by notification file); room_ack removes only those bytes.
* Fast path: pollers only append, so the consumed content is a byte
prefix of the current file — drop the prefix, keep the rest.
* Fallback: if the file was rewritten in between, remove consumed
lines by exact match (multiset semantics).
* Writes go through temp-file + rename (mode preserved) so the
appending poller and file readers never see a half-written file;
appends after the rename land in the new file.
* No prior room_list_new this session → legacy clear-all is kept so
the existing no-args caller contract still works; the response
nudges callers toward read-then-ack.
* room_ack now reports how many lines were consumed and how many
late arrivals were preserved.
Tests: pure-function coverage for prefix/rewrite/multiset/empty-read
cases, file-level regression (append lands between read and ack and
must survive), and an end-to-end stdio regression through bin/iak-mcp.mjs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a677c007d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (line.trim() && (pending.get(line) || 0) > 0) { | ||
| pending.set(line, pending.get(line) - 1); | ||
| removed += 1; |
There was a problem hiding this comment.
In the rotated/edited fallback, this removes the first current line whose text matches a previously read line, even if the consumed copy is no longer in the file and the matching line is a new unread duplicate. The pollers format notifications with second-resolution timestamps and truncated bodies, so repeated messages like the same sender/body in the same second can be byte-identical; after a rotation or manual rewrite this path would silently ack that late duplicate instead of preserving it. Consider keeping an offset/generation invariant for fallback or avoiding destructive exact-match removal when the consumed prefix is gone.
Useful? React with 👍 / 👎.
P1 (codex, #33): the bootstrap told agents to truncate the whole notification file - recreating the exact read-vs-append race #32 just fixed. Instructions now direct agents to the consumed-only room_ack MCP tool, with raw-clear only as a no-MCP fallback plus an immediate re-check. P2: install.sh skipped hook wiring entirely when ~/.claude/settings.json did not exist yet - fresh one-shot installs now get a minimal file created so self-arming works on day one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
The race
The IAK MCP server's
room_acktool blanked the ENTIRE notification file (writeFileSync(notifyFile, '')). Anything the poller appended between the agent's lastroom_list_newread and the ack was wiped unread — silently lost.Real incident (2026-07-08): an owner instruction ("send all 3") landed in exactly that read→ack window and sat 5 hours unseen. This class of loss has bitten twice.
The fix: ack only what was consumed
room_list_newnow remembers the exact raw bytes it returned, per MCP session, keyed by notification file path (so a config change between calls is treated as a cold ack for the new path).room_ackremoves ONLY those bytes:rename(2)d over the notification file, preserving the original file mode. Readers never see a half-written file, and since the pollers re-open the path on everyappendFileSync(no flock anywhere in the repo — checked), appends after the rename land in the new file.room_list_newthenroom_ackwith no args — unchanged. A coldroom_ack(noroom_list_newrecorded this session) keeps the legacy clear-all behavior so existing callers don't break, and the tool response nudges toward read-then-ack.room_acknow reports counts: how many read lines were acked and how many late arrivals were preserved (with a hint to callroom_list_newagain).Watcher note
The rename replaces the inode, so
tail -F-style watchers see it as a rotation and re-open; the IAK Monitor/hook patterns re-read the whole file each time anyway, so surviving lines still trigger delivery.Remaining window (documented, not hidden)
Without any locking protocol on the poller side there is still a microseconds-wide read→rename window inside a single ack call, versus the old minutes-to-hours read→think→truncate window. Adding a lock protocol shared with the pollers would be the follow-up if that ever matters.
Tests
removeConsumedNotifications: prefix drop, drain-to-empty, empty-read-consumes-nothing, rewritten-file fallback, duplicate-line multiset semantics.ackNotificationFile: poller append lands between read and ack and MUST survive; legacy null (cold ack) clear-all; missing-file no-op.bin/iak-mcp.mjs:room_list_new→ simulated poller append →room_ack→ late line survives on disk and the second read/ack cycle drains it.npm test: 110/110 pass (was 100; +10 new, no regressions).Review
@codexmb — requesting adversarial review per the cross-model PR review rule before merge. Particular scrutiny welcome on: the prefix-vs-multiset fallback choice, the cold-ack legacy path, and whether the rename interacts badly with any watcher I missed.
🤖 Generated with Claude Code