poller: remember each message as it is handled, durably (#90 item 1) - #91
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…of-play line (#90 items 2, 6, 7) Why: a mention reached the agent with a 100-char preview of its parent, or "target not in recent window" when the parent was older than the 10-message fetch; settled facts lived in messages the agent never saw. hermes read "new card" as a GPU card for exactly this reason (issue #90). What: - src/common/room-history.mjs: RoomHistory (JSON file next to seen_file, 400 messages per room, persisted every poll) plus threadSuffix (parent in full with author and time, up to two ancestors), previousSuffix (the asker's previous message), stateOfPlayLine ("state:" / "settled:" messages, deduped, newest wins) and ownLastPostLine. - reply-context.resolveReplyTargets(msgs, lookup): resolves targets outside the batch through the history; snippet 120 -> 300 chars. - room-poller: fetch 25 per poll (poller.fetch_limit), remember every batch, one deeper fetch (100) when a reply target is still unknown, and the notification line carries body (400) + thread + previous message for owner messages and mentions. When a room's batch has an owner message or a mention, one STATE OF PLAY line and one YOUR LAST POST HERE line are prepended for that room. - Notification-file contract kept: one physical line per message. - README: poller.history_file, poller.fetch_limit, the state: convention. Tests: 7 new in test/room-history.test.mjs; suite 304/304. Co-Authored-By: Claude Fable 5.1 <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: eefd1ce531
ℹ️ 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".
| newCount++; | ||
| // Handled = queued + notified; only then is it safe to remember it. | ||
| appendNotifications(notifyFile, [line]); | ||
| saveSeenIds(seenFile, seen); |
There was a problem hiding this comment.
When the process exits or a later message throws after this save, an earlier owner message or @mention is already marked seen even though the batch-level triggerNudge has not run. On restart that message is skipped and no longer sets hasOwnerMessage/hasMention, so the configured tmux or command wake can be permanently lost; the DM loop has the same ordering. Persist pending wake state or issue the required nudge before making the message non-retryable.
Useful? React with 👍 / 👎.
| appendNotifications(notifyFile, [line]); | ||
| saveSeenIds(seenFile, seen); |
There was a problem hiding this comment.
On a power loss, appendFileSync does not guarantee that the queue and notification writes have reached stable storage, while saveSeenIds immediately fsyncs the seen file. The marker can therefore survive while the supposedly handled event and notification disappear; after restart the poller skips the message and loses it permanently. The queue and notification files must be flushed before durably committing the seen ID.
Useful? React with 👍 / 👎.
| } finally { | ||
| closeSync(fd); | ||
| } | ||
| renameSync(tmp, path); |
There was a problem hiding this comment.
When the host loses power after this rename, fsyncing only the temporary file does not make the directory-entry replacement durable. Depending on the filesystem, restart can expose the previous seen file or no target at all, causing the exact replay window this change is intended to prevent. Open and fsync the parent directory after renameSync before considering the state durable.
Useful? React with 👍 / 👎.
ThinkOffApp
left a comment
ThinkOffApp
left a comment
There was a problem hiding this comment.
Reviewed the atomic seen-state update and per-message persistence in both room and DM paths. Focused and full test suites pass (299/299); no blocking issues found.
…w); seedRoom() exported with a regression test Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The team-relay poller saved its seen-ids once per batch, and a batch can hold a task that runs for an hour: a restart inside it replayed the whole window. The M5 hermes poller did exactly that on 2026-09-01 (state stale since 31 Aug). Now the notification line and the seen-marker are written right after each handled message, rooms and DMs, and the shared saveSeenIds writes atomically (temp + fsync + rename) so a crash or power loss never leaves a truncated file that reads as 'nothing seen'. The poller's private copy of the seen-id helpers is gone in favour of the shared module. Tests: atomic write round-trip and cap; a live poll where the second message crashes the batch - message 1 is persisted and notified before the crash (fails on the previous code, verified). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t priority line Rebased onto #92 (fa043ae). #92 buffered each room's lines and wrote the state-of-play / own-last-post header with them after the loop; #91 writes each line as it is handled. Reconciled: the header goes to the notification file right before the first line that makes the batch worth a wake, so readers still see header then lines and every line is on disk before its seen-marker. Test: header once, before the owner line, no line written twice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eefd1ce to
48a7225
Compare
|
Rebased onto #92's head (fa043ae) so it merges cleanly right after #92. Reconciliation: #92's state-of-play / own-last-post header is now written lazily — once per room, right before the first line that makes the batch worth a wake — so readers still see header-then-lines while each line is on disk before its seen-marker. New test covers header placement and no duplicate lines. Suite pass 308. |
1 participant
#90 item 1 (split agreed with claudeMB in the room).
Problem: the team-relay poller saved seen-ids once per batch; a batch can hold a task that runs for an hour, so a restart inside it replayed the whole window (the M5 hermes poller did exactly that on 2026-09-01 — state stale since 31 Aug). A plain
writeFileSynccan also leave a truncated file on crash/power loss, which reads as "nothing seen" and replays everything.Change:
src/common/seen-ids.mjs:saveSeenIdswrites a temp file, fsyncs, renames over the target (atomic, durable).src/team-relay/room-poller.mjs: uses the shared module (its private copies are gone); the notification line and the seen-marker are written right after each handled message, in both the room and DM loops. The post-loop nudge/gating logic is unchanged; the post-loop batch append is removed (lines are already on disk).Tests (
test/seen-state.test.mjs): atomic round-trip + cap; a livestartRoomPollerrun against a stubcurlwhere the second message has a numeric body and crashes the batch — message 1 is in the seen-file and its notification line on disk before the crash. Positive control: the same test fails on the previous poller. Suite 299/299.🤖 Generated with Claude Code