[antra-tess]

Summary

Implements RFC-006 event coalescing (mcpl PR #5, revision 7, 9a319e2) with content kept in context: no staging area, no journal, no subject cap.

A plain coalesced occurrence is delivered on arrival through the ordinary channel/push path, exactly as today; the coalescer only remembers where it landed. A later occurrence of the same subject removes that message while it is still unread — above the agent's consumed watermark (advanced at every compile of a model request), on the current branch, not a sharded message, and not folded into a summary by compression — and delivers the replacement as a fresh message; once read, it appends. Retraction removes unread content and appends the notice only when some version was read or history is unknown. initial marks the birth of a subject so a never-read create → edit → delete leaves no trace, including after restart or eviction. Deferred batches (push/render) are the only thing held outside context, because a notice has no model-visible content until rendered; they render at the turn's assembly boundary (not on a context-budget restart), bounded by the connection's 5 s timeout, falling back to the admitted content.

On any doubt — restart, branch switch (undo), eviction, unknown history — the host appends, which is RFC-006 principle 4 and what today's host does.

Supersedes #196 (journal design). The five findings from that review are regression tests here.

Changes

• src/mcpl/push-coalescer.ts — the state machine (subjects, receipts with a 1 h retry window, deferred batches, render cancellation rules 1–7), pure bookkeeping behind a CoalescerHost interface.
• src/agent.ts — per-agent consumed watermark ({branch, sequence}), advanced in compileContext/compileWithInjections, set at agent creation.
• src/framework.ts — host implementation: unread check (watermark + summary coverage via getSummariesInRange/getSummary), removal (stored message or deferred-queue entry), delivery through handleMcplChannelIncoming/handleMcplPushEvent (which now report placement and carry coalescingSubject on the wakes they queue), channel-scoped pushes delivered as messages of their channel (forks, tune-out, reply locus), wake withdrawal, assembly at turn start, throttled mcpl/coalescing snapshot (receipts + subject history), -32600 for inference/request during a render.
• src/mcpl/channel-registry.ts / push-handler.ts — coalesced occurrences handed to the coalescer after ordinary admission with the event the ordinary path would have queued; eventId required with coalesce; per-message coalesce_invalid. Channel registration lifetime is unchanged (RFC-006 rev 7 §3.2).
• src/mcpl/server-connection.ts — push/render with its own timeout (from feat(mcpl): journal channel and push event coalescing #196).
• src/mcpl/types.ts — coalesce (with initial), CoalesceResult, eventCoalescing capability with retryWindowMs.
• README section, changelog fragment, two trace event types (mcpl:coalescing, message:removed).

Tests

test/mcpl-coalescing.test.ts — 34 tests over a real loopback WebSocket MCPL server and the mock provider, named by RFC-006 §14 vector: plain (1–7, 9, 35, 38), channel scope and mixed lanes (8, 10, 13–15, 33, 43), wake treatment, deferred (16–27, incl. 27a/27c/27i), recovery (39–41, 47), and the #196 findings:

finding	test
transient reconnect rejects ordinary channel traffic	accepted after reconnect without re-registration
subject cap refuses a busy channel's mention	300 unread + mention, all accepted, < 6 s
delete-before-read after restart/eviction → notice	initial → retracted; without it → noted
assembly on context-budget restart	batch untouched by the restart, rendered at the next fresh turn
undo destroys coalesced input	input delivered on arrival survives the undo; nothing replaceable across the branch switch

Plus: replacement while a turn is alive edits the deferred queue; a message folded by the real AutobiographicalStrategy is no longer replaceable (appends).

Full suite: see the CI run; locally node --test --test-concurrency=4 dist/test/*.test.js (without --test-force-exit) matches upstream 5498805 except for the tests added here.

Not done / known gaps

• A replacement is delivered where a fresh event lands (tail), not in the replaced message's slot; RFC-006 rev 7 §4.1 allows either.
• EventGate debounce batches are not consulted on withdrawal: a replacement matching skip withdraws its pendingRequests wake, but a debounce timer already armed by the gate for the prior occurrence still fires (it carries no content, so nothing withdrawn is shown).
• Module onProcess dispatch is skipped for coalesced deliveries (they bypass the event queue so placement is known synchronously); the built-in MCPL conversion, routing, tune-out and invitation paths all run.
• No producer migration (discord-mcpl / portal-mcpl) in this PR.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 1/5

[High risk] Implements RFC-006 event coalescing across message delivery.

The PR is not ready to merge because coalesced channel delivery and crash recovery can produce incorrect model-visible messages.

Findings

1.  Spurious closed-channel invitation ▶
2.  Channel identity lost after restart ▶
3.  Empty render can reappear ▶
4.  Rejected items change reply routing. ▶

Fix with agent prompt

### Issue 1
src/mcpl/channel-registry.ts:992-993
When the first coalesced message arrives for a registered but closed channel, delivery runs before `markAccepted()` opens it. The incoming handler therefore adds a closed-channel invitation, telling the agent to join a channel that the host accepts as open moments later.

### Issue 2
src/mcpl/push-coalescer.ts:388-391
If the host crashes before the throttled snapshot flush, a plain channel occurrence is recovered from a recent receipt that has neither its platform identity nor its delivery agent. A channel-scoped push edit without origin IDs then uses its event ID instead of the original message and thread IDs. If routing has changed, the correction can also reach a different agent.

### Issue 3
src/framework.ts:7465-7467
If a completed `push/render` returns no content and the host stops after writing the cleared snapshot but before clearing recent receipts, recovery replays the older receipt as a pending batch. No message was published for recovery to detect, so a later turn can deliver the fallback even though the server rendered nothing.

### Issue 4
src/mcpl/channel-registry.ts:971-977
Channel admission changes the default publish channel before this hook validates a coalesced item or recognizes a retry. An item that returns `coalesce_invalid`, or a duplicate that delivers nothing, can therefore redirect a subsequent turn's fallback reply to its channel.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds RFC-006 coalescing across MCPL push and channel events, including unread replacement, deferred rendering, receipts, and recovery. Changes since the prior review also add tool-lifecycle observation and tool-class policy.

• Coalesced channel admission can add a spurious closed-channel invitation.
• Recent-receipt recovery loses channel identity and audience, and an interrupted snapshot flush can resurrect an emptied batch.

_{Reviews (2) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."}

[greptile-apps]

Rejected items change reply routing. Channel admission changes the default publish channel before this hook validates a coalesced item or recognizes a retry. An item that returns coalesce_invalid, or a duplicate that delivers nothing, can therefore redirect a subsequent turn's fallback reply to its channel.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/mcpl/channel-registry.ts
Line: 971-977

Comment:
**Rejected items change reply routing.** Channel admission changes the default publish channel before this hook validates a coalesced item or recognizes a retry. An item that returns `coalesce_invalid`, or a duplicate that delivers nothing, can therefore redirect a subsequent turn's fallback reply to its channel.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[antra-tess]

Additional review of 3666acde5a63ee300dff7fb1e4df39f482afae66: three major findings beyond the current Greptile review. I reproduced each in an isolated checkout using this PR's loopback WebSocket fixture. These are observed failures, not just concerns from reading the code.

1. Channel-scoped render results survive revocation of channels.incoming

Response-time authorization

Reproduction:

1. Register chat and accept a deferred push/event scoped to it.
2. Start assembly and hold the push/render response.
3. Remove channels.incoming from the current effective grant, leaving pushEvents granted and the channel registered.
4. Complete the render with distinctive content.

Observed: that content appears in the model request after revocation.

coalescedAuthorized checks channels.incoming only for the channel delivery lane. For a channel-scoped push, it checks pushEvents, feature-set enablement, and registration, but omits the current channel grant. This violates RFC-006 §3.2 and §5.4 rule 4.

Fix: apply the full current channel authorization check to every channel-scoped occurrence, including push results and fallbacks, at assembly and response time. Add a regression that revokes only channels.incoming while rendering is in flight.

2. Endpoint reassignment reuses the former server's subjects and receipts

Receipt identity · Subject lookup

Reproduction:

1. Endpoint A, configured as editor, sends an unread occurrence with event ID E and key K.
2. Call restartMcplServer('editor', updatedConfig) with a different endpoint B.
3. B sends its own occurrence with ID E, then another occurrence with a fresh ID under K.

Observed: B's first occurrence receives A's acceptance receipt and its content is not delivered. B's second occurrence returns replaced, names A's E as priorEventId, and removes A's unread content.

Both namespaces use the configured server label without a binding identity that changes on endpoint/principal reassignment. This violates RFC-006 §3.2 and vector 42. Transport reconnects should preserve identity; changing the configured peer should not.

Fix: include a host-owned binding identity in both subject and receipt keys, preserve it across reconnects to the same peer, and change it on reassignment. Preserve already accepted work under its original binding rather than silently reinterpreting it as B's work.

3. Replacement can move an unread delivery into a new conversation audience

Replacement path

Reproduction:

1. With conversation routing enabled, deliver an initial channel occurrence into conversation-chat-g1, without running inference.
2. Unbind the channel using the router's unbind operation.
3. Send an edit with the same channel/key. Fresh routing creates conversation-chat-g2.

Observed: the edit returns replaced; the original disappears from g1, and the replacement appears in g2, which never received the original.

acceptPlain removes the previous placement, then calls ordinary delivery for the replacement without constraining it to the previous audience. RFC-006 §3.2 permits narrowing the prior deliveries, but not expanding or moving them to a new context merely because fresh routing changed.

Fix: retain the original delivery audience for replacement and retraction. Apply current policy within that audience; if none remains eligible, remove the old pending delivery without routing its replacement into a new audience. Cover rebinding and analogous routing-policy changes.

All three probes reproduced the behavior above. The branch builds and its existing 34 coalescing tests pass; these scenarios are additional coverage. I would treat all three as merge blockers, with channel authorization and server-binding isolation highest priority.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Review round 1 addressed at d86afc1. All 18 findings were valid; each is fixed and, where observable on the wire, covered by a regression test (12 added; suite is now 46 coalescing tests, full run matches upstream except the pre-existing conversation-routing cleanup error).

Codex

1. Channel render survives revocation of channels.incoming — fixed. coalescedAuthorized now requires the current channels.incoming grant for every channel-scoped occurrence on both lanes, checked at admission, before render and at render response. Test: revoke only channels.incoming while a render is held → rendered content never reaches the request.
2. Endpoint reassignment reuses subjects and receipts — fixed. Subject and receipt keys include a host-owned binding (server id bound to its url/command/args); a reconnect keeps it, restartMcplServer with a different endpoint starts a new one. Test: A's event id reused by B is first, B's edit under A's key does not touch A's unread content.
3. Replacement moves into a new conversation audience — fixed. A replacement or deletion notice carries deliverTo = the agent that holds (or read) the prior; the channel path delivers into that agent directly and never re-routes. Gone agent → delivered nowhere (§3.2 "reach none → remove, append nothing"). Test: unbind the channel, edit → lands in the original fork, no fresh fork spawned.

Greptile

1. Malformed content aborts the batch — fixed; shape validated before conversion on both lanes (per-message coalesce_invalid / -32602). Test added.
2. Rejected items change reply routing — fixed; for coalesced items the default-publish/open update runs only after the coalescer accepts.
3. Channel renders lose their feature set — fixed; the occurrence carries featureSet. Test asserts push/render.featureSet === 'doc' for a channel-scoped batch.
4. Deferred pushes cannot start forks — fixed; the audience for an unbound channel runs the router's bind decision and spawns the fork. Test added.
5. Deferred wakes lose channel routing — fixed; the batch wake carries channelId (scope or derived) and counterparty. Test asserts the reply publishes to the batch's channel.
6. Flushed messages appear read — fixed; a deferred-queue placement is resolved to the stored message by metadata.deferredWriteId, scanning only the unread tail. Test: edit and retract after a flush.
7. Rendered content becomes unwithdrawable — fixed; a materialized render that lands in a busy agent's deferred queue is kept as the unread occupant rather than marked consumed.
8. Watermark counts unseen messages — fixed on both counts: the watermark is taken before the compile and applied only after it succeeds; a tune-out-diverted message is judged by the subconscious's watermark (and is unread if there is no subconscious).
9. Acknowledged receipts can vanish — fixed; each receipt (with the subject's birth history) is written to mcpl/coalescing-recent synchronously at acceptance and merged on restore; the throttled snapshot clears it. Test added.
10. Restart skips untouched renders — fixed; the snapshot marks only RENDERING batches, and only those take the fallback after a restart. Test added.
11. Push edits erase channel identity — fixed; identity merges only defined fields. Test asserts message id, thread and author survive a push edit without origin.
12. Withdrawal loses unrelated wakes — fixed; the quiesce coalescing key includes the coalescing subject.
13. Coalesced events skip module handlers — fixed; the module half of handleProcessEvent is factored out and runs for coalesced deliveries. Test with a spy module.
14. Failed removals still acknowledge replacement — fixed; remove reports failure and the coalescer then treats the prior as possibly read (history unknown, append path).
15. Cancelled renders permit nested inference — fixed; isRendering counts outstanding push/render RPCs, cancelled or not. Test: retract during a held render, then inference/request → -32600.

One design note on 7/8: a coalesced delivery still bypasses the process queue (the coalescer needs the placement before it can answer the wire), so ordering relative to ordinary events already queued is not guaranteed; modules now see it, and the reply ordering on the wire is unaffected.

[antra-tess]

Re-review of d86afc157be3c66f91fe2633dcfde9098ec1e736. The branch builds and all 46 coalescing tests pass locally. The added regressions address the original scenarios, but I reproduced four major remaining failures in deferred delivery/recovery. These are follow-ups to the fixes, not a repeat of the original-head review.

For the recovery probes, I explicitly synced Chronicle, copied its on-disk store at the specified boundary, allowed the fixture to shut down, then reopened the copied image. Thus clean shutdown's snapshot flush cannot mask the state a crash would recover. I did not rerun the full suite for this review.

1. An accepted deferred event can lose its work while retaining the receipt that prevents retry

Recent receipt persistence · Receipt restoration

Reproduction:

1. Send a deferred occurrence and receive accepted: true.
2. Before the two-second subject snapshot, sync the store and capture the recovery image. mcpl/coalescing-recent contains its receipt, while the pending batch exists only in memory.
3. Reopen that image and retry the same occurrence.

Observed: the retry returns the original successful receipt, but there are 0 pending batches, 0 render calls, 0 model calls, and no fallback content. The receipt record preserves only the result and birth history; restoreReceipts cannot reconstruct the accepted payload/batch.

Required: acknowledgement must cover enough recoverable state to deliver the accepted work or its fallback, together with its receipt. Persisting the receipt alone makes loss permanent by suppressing redelivery. Also handle persistence errors rather than acknowledging them as success.

2. Old-binding deferred notices are sent to the newly configured endpoint

Authorization check · Render dispatch

Reproduction:

1. Endpoint A (editor) sends a deferred occurrence containing distinctive private notice data.
2. Before assembly, call restartMcplServer('editor', newConfig) with endpoint B.
3. Run assembly.

Observed: A receives no render; B receives push/render carrying A's event ID and private notice data.

The new binding namespaces correctly separate new admissions, but dispatch still resolves the current connection by serverId, and coalescedAuthorized never compares occ.binding with the current configured binding. Consequently old pending work acquires the replacement peer's authority. This is a cross-binding disclosure, not just duplicate suppression.

Required: verify binding identity before render dispatch and publication, including recovery paths. Preserve, conservatively settle, or reject old-binding work according to its original authority; never send its private notices to the replacement endpoint.

3. Plain → deferred replacement still changes the audience

Deferred admission

Reproduction:

1. Deliver an unread channel occurrence to conversation-chat-g1.
2. Unbind that channel.
3. Send a deferred channel-scoped push with the same subject key, then assemble.

Observed: receipt is replaced; g1 loses the original; a new g2 is spawned and receives the rendered content.

acceptPlain now carries the prior audience, but acceptDeferred removes the prior placement without putting its audience on the batch. wakeForBatch/assembly therefore run fresh routing. The new audienceAgent field does not constrain this transition.

Required: carry the original eligible audience through plain/deferred transitions, subsequent notices, rendering, fallback, and recovery. The deliverTo fix must apply to the entire subject lifecycle, not only plain replacements and deletion notices.

4. Recovery can reissue a render that already started

Render start and persistence timing

Reproduction:

1. Accept a deferred batch, flush its pending snapshot, and sync.
2. Start assembly; hold the server's render response after it receives the request.
3. Sync/capture the recovery image while that RPC is outstanding.
4. Reopen the image and assemble again.

Observed: the server receives push/render for the same batch twice.

The rendering snapshot flag only helps if a snapshot containing it was actually persisted. Starting the RPC does not persist that transition; assemble calls persist after the render finishes. A previously flushed pending snapshot therefore restores as safe to render even though the source-backed server may already have advanced its baseline.

Required: make the render-start boundary recoverable before issuing the RPC, or conservatively use fallback when recovery cannot prove the render never started. Rendering untouched pending work after restart is optional; avoiding replay of a potentially executed render is the guarantee that matters.

All four additional probes reproduced the outcomes above on this exact head. I would keep these as merge blockers, especially the cross-binding notice disclosure and acknowledged-work loss.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 2 addressed at c03b23b. All four reproduced; all four fixed, each with a regression test (50 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

1. Accepted deferred work lost while its receipt survives — the receipt record written at acceptance now carries the accepted batch (latest occurrence, notices, dropped count), so a crash before the snapshot flush restores the work together with the receipt. It is restored as noRender (fallback at the next assembly) because the record cannot prove no render started afterwards. A persistence failure now throws -32000 and fails the acceptance rather than acknowledging it. Test: crash (no snapshot flush) → restart → retry returns the receipt, the fallback is delivered, no render.
2. Old-binding notices sent to the new endpoint — coalescedAuthorized compares occ.binding with the server id's current binding; mismatch is unauthorized everywhere that check runs (admission, wake, before render, at response, at publication). Old-binding pending work is dropped with a revoked audit; its notice data never reaches the replacement peer. Test: deferred with private data from A → restartMcplServer to B → assemble: zero renders, data absent from every RPC, fallback not delivered.
3. Plain → deferred replacement changes the audience — acceptDeferred carries the displaced occupant's agent as deliverTo on the batch, and later notices inherit it; the audience, wake, render delivery, fallback and snapshot all follow it. Test: unread plain in fork g1, unbind, deferred push → no g2, render delivered into g1.
4. Recovery reissues a started render — the snapshot (with the rendering flag) is written synchronously before push/render is issued; if that write fails the batch stays pending as noRender. Test asserts the persisted state shows rendering: true while the RPC is outstanding.

[antra-tess]

Follow-up on c03b23b0a50eabe4cbd01df8c9984566aeaadfdd: build and all 50 coalescing tests pass locally. The round-2 changes make progress, but two major recovery failures remain. I reproduced both by explicitly syncing Chronicle, copying its on-disk store before the throttled snapshot, and reopening that copy after cleanup. These are durable-state recovery probes, not graceful-restart tests.

1. An acknowledged retraction can resurrect the withdrawn batch

Acceptance record · Receipt restoration

Reproduction:

1. Accept deferred E1 with initial: true and distinctive fallback content.
2. Retract it as E2 before assembly; receive outcome: "retracted". In memory there are now zero pending batches.
3. Before the snapshot timer, sync and capture the store, then reopen that image.
4. Retry E2 and run until idle.

Observed: E2 returns its original retracted receipt, yet the next model request contains E1's withdrawn fallback.

The E1 recent record includes batch; E2's record omits it. restoreReceipts restores E1's batch and never clears it when processing E2. Thus a receipt-only record cannot express the completed withdrawal. The same representation also needs to distinguish replacement/discard from “no batch update.”

Required: recovery must apply each accepted subject transition in order, including explicit batch removal/retraction, or commit a complete consistent subject state. Retrying an acknowledged withdrawal must never leave the withdrawn content scheduled for delivery.

2. A completed render is delivered again as fallback after recovery

Publication and in-memory completion

Reproduction:

1. Accept a deferred occurrence and run a full turn. The model/context contains its rendered content, with no fallback.
2. Before the throttled completion snapshot, sync and capture the store. The saved subject snapshot still says rendering: true, while the rendered context message is durable.
3. Reopen that image and run until idle.

Observed: context contains both the original rendered content and a newly delivered fallback for the same occurrence. Only one render RPC ran; the duplicate effect is recovery publication.

Persisting render-start before the RPC prevents repeating the RPC, but does not establish whether its result/fallback was already published. Recovery always treats the old rendering snapshot as pending fallback work, without checking a stable publication identity in context.

Required: make context publication and completion recover consistently, or give publication a durable identity and deduplicate recovery against its existing context message. A fallback is appropriate for an uncertain, unpublished render; it must not become a second delivery after a completed one.

Both probes reproduced these outcomes on this exact head. I would keep them as merge blockers. No full-suite rerun or production-provider exercise was performed for this follow-up.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 3 addressed at c2da972. Both reproduced; both fixed, each with a regression test that crashes the host (no snapshot flush) and reopens the store (52 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

1. Acknowledged retraction resurrects the batch — the receipt record now states the subject's batch after the acceptance, null included, and restoreReceipts applies records in order, so a retraction or plain replacement accepted inside the snapshot window replays as the batch removal it was. Test: deferred E1 → retract E2 → crash → retry E2 returns retracted, no turn, no fallback.
2. Completed render delivered again as fallback — coalesced deliveries now carry coalescingSubject in message metadata (next to eventId), which gives a publication a durable identity. On restore, a batch that was RENDERING whose occurrence is already found in a context is settled as consumed (history some) instead of taking the fallback path. Test: full turn with a rendered batch → crash before the completion snapshot → restart → rendered content exactly once, no fallback, one render RPC; a later retraction is noted.

[antra-tess]

Re-review of c2da9729a48a468f5d5678dfa799d34e63e25c7d: build and all 52 coalescing tests pass locally. I also reran the two previous synced-store recovery probes with the correct-behavior assertions; both now pass. The withdrawn deferred batch stays withdrawn, and a completed render no longer gains a recovery fallback.

One major recovery gap remains for plain occurrences:

A consumed plain event can recover as history: none, suppressing its deletion notice

Birth-history restoration · Publication settlement only handles batches

Reproduction using the PR's WebSocket fixture:

1. Accept plain E1 with initial: true and distinctive content.
2. Run a complete model turn and verify its request includes E1.
3. Before the throttled subject snapshot, explicitly sync Chronicle and copy its on-disk store. Reopen that captured image so cleanup's later snapshot cannot mask the crash window.
4. Send a fresh retraction E2 with a nonempty deletion notice.

Observed: E2 returns accepted: true, outcome: "retracted"; E1 remains in context, but the deletion notice is absent. This is the read-original → deletion case, so the required outcome is noted with the notice appended.

The recent acceptance record says born: "none" and batch: null. restoreReceipts reinstates that birth history without a placement or evidence of subsequent consumption. settlePublished only checks restored deferred batches, so the durable plain message does not correct that claim. A historical birth fact is not proof that the subject is still unread at recovery time.

Required: reconstruct the plain occurrence's publication/consumption knowledge from durable state, or conservatively restore unknown when the recent record cannot prove none. Preserve the receipt for retry deduplication, but do not let it suppress a correction to content the model already read. Add a regression covering plain create → inference → recovery before snapshot → retract.

This probe reproduces on the exact current head. I would keep it as a merge blocker. No full-suite rerun or production-provider test was performed in this follow-up.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 4 addressed at 8628bff. Reproduced and fixed, with the requested regression (53 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

Consumed plain event recovers as history: none — the receipt log's birth fact is now settled against durable state on restore: for every recent record, if the acknowledged occurrence (subject + eventId in message metadata) is found in any agent's context, the subject becomes some with that consumedEventId, exactly as a snapshot occupant is treated. The same wasPublished check that settles RENDERING batches now takes (subject, eventId) and runs for plain records too. The receipt itself is kept, so a retry is still deduplicated. Test: plain create (initial) → full turn → crash before the snapshot → restart → retract returns noted and the notice is appended after the original; the never-read variant also yields noted after a crash, since a stored occurrence counts as read once the watermark restarts at the head.

[antra-tess]

Re-review of 8628bff94db293cae16be595f3dd59c16d9965d8: build and all 53 coalescing tests pass locally. My previous synced-image probes also pass now, including consumed plain create → recovery → deletion notice. That finding is resolved on this head.

One foundational durability blocker remains, demonstrated with actual child-process SIGKILL, without framework.stop(), store.close(), or a post-acceptance store.sync():

Synchronous setStateJson is not a durable acknowledgement or render-start barrier

Acceptance persistence · Render-start persistence

Acceptance probe:

1. Run the PR's loopback fixture in a child Node process and sync its initial store to establish a durable baseline.
2. Send a deferred occurrence. Wait for its actual JSON-RPC response, then notify the parent over IPC.
3. The parent sends SIGKILL and waits for exit, then opens the child's store.

Observed:

{
  "wireResult": {"accepted": true, "coalesce": {"outcome": "first"}},
  "recentReceiptsBeforeKill": 1,
  "recentReceiptsAfterReopen": null,
  "subjectSnapshotAfterReopen": null
}

Both the acknowledged work and its receipt disappear. This violates preservation of accepted work and the restart-safe retry guarantee, even though the acceptance callback returned synchronously.

Render-start probe:

1. Accept a deferred batch, explicitly flush/sync its pending snapshot.
2. Start assembly and hold the render response. Once the server receives push/render, notify the parent.
3. Kill the child and reopen the store.

The in-memory snapshot had rendering: true, but the reopened snapshot contains the old pending batch without that flag. Recovery therefore treats a potentially executed render as safe to issue again. The new saveNow barrier changes in-memory state synchronously but does not durably commit it before the RPC.

The current test crash helper disables the coalescing snapshot timer but still calls framework.stop(). That is useful for testing the snapshot gap, but it flushes storage and does not exercise abrupt termination. My earlier synced-image probes deliberately tested logical consistency after sync; these new probes test the missing durability boundary itself.

Required: wait for a successful durable commit covering recoverable work and its receipt before sending acceptance, and durably commit render-start before issuing the RPC. Group commit/batching is fine; this does not require a sync per journal operation or the previous #196 architecture. Add subprocess-kill coverage at both boundaries, and handle commit failures without subsequently acknowledging an uncommitted in-memory receipt.

I would keep this as a merge blocker. No full-suite rerun or production-provider test was performed in this follow-up.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 5 addressed at e2683b5. Reproduced with a real SIGKILL and fixed at both boundaries, with subprocess-kill coverage as requested (55 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

Synchronous setStateJson is not a durable barrier — agreed. The host now has a group-committed durability barrier (commitCoalescingDurable: one store.sync() per event-loop turn, shared by every acceptance that arrives in that turn). The coalescer awaits it:

• after writing the receipt record (receipt + accepted batch) and before returning the wire result, so the acknowledgement implies the work and its receipt are on disk;
• after the render-start snapshot and before push/render is issued.

A commit failure is a -32000 and the acceptance is not acknowledged; the in-memory receipt is kept so a retry while this process lives is deduplicated, and after a kill nothing of it exists so the retry is a fresh acceptance. No sync per journal operation and no journal: ordinary non-coalesced traffic is untouched.

Cost: 300 serial coalesced channel messages now take ~26 ms each including the fsync (the earlier journal design measured 85 ms); concurrent arrivals share one fsync.

Tests: the wire fixture moved to test/helpers/coalescing-fixture.ts so test/helpers/coalescing-kill-child.ts can run it in a forked process. Two tests kill that child with SIGKILL (no stop(), no close(), no sync()) — one after the wire acceptance of a deferred occurrence, one after the server has received push/render — then reopen the same store on the same port: the retry returns the original receipt and the fallback is delivered; the render is not issued a second time.

[antra-tess]

Re-review of e2683b507587929dabb5a39ee35fca447448d80b: build and all 55 coalescing tests pass locally, including the new subprocess-kill tests. The normal acceptance/render-start durability boundaries now have coverage. I reproduced three major gaps in the failed-commit and interleaving paths introduced by the asynchronous barrier.

1. Retry after a failed commit acknowledges the same uncommitted receipt

Early duplicate return · Receipt retained before failed commit

Reproduction in a child process using the wire fixture:

1. Sync an initial baseline. Make the next store.sync() throw a transient EIO, with subsequent syncs allowed to succeed.
2. Send a deferred occurrence. The response correctly returns -32000 asking the producer to retry.
3. Retry the same occurrence immediately.
4. After receiving the retry's response, the parent sends SIGKILL and reopens the store.

Observed:

{
  "firstResponse": {"error": {"code": -32000}},
  "retryResponse": {"result": {"accepted": true, "coalesce": {"outcome": "first"}}},
  "syncCalls": 1,
  "recentAfterKill": null,
  "snapshotAfterKill": null
}

The duplicate path returns the in-memory receipt before attempting another durability barrier. Keeping the receipt is useful to prevent repeating effects, but it cannot count as committed merely because a retry found it.

Required: distinguish pending/uncommitted receipts from committed ones; retry the commit (or return an error) before acknowledging such a duplicate. Do not replay delivery effects just to obtain a committed receipt.

2. Retraction during the render-start commit wait resurrects the cancelled batch

Render-start await and restoration

Reproduction:

1. Accept an initial deferred batch.
2. Start assembly and hold its commitCoalescingDurable() promise after the render-start snapshot is written, before the RPC.
3. Through the wire, retract that subject; let that acceptance commit and receive retracted.
4. Release the held render-start wait.

Observed: the subject now has one pending batch again. A later assembly calls push/render and publishes the withdrawn content.

Retraction clears state.rendering, but line 512 interprets any cleared rendering state as a reason to assign state.batch = batch. It cannot distinguish cancellation from a persistence failure. The same wait can be overtaken by replacement/newer work.

Required: after each await, verify that this exact rendering instance is still current and not cancelled. Restore work after a failed commit only if that operation still owns it; cancellation/replacement must never restore its old batch or overwrite newer work.

3. Endpoint reassignment during that wait leaks old notice data to the new binding

Authorization before the wait and RPC after it

Reproduction:

1. Endpoint A admits a deferred occurrence containing distinctive private notice data.
2. Hold the render-start commit promise as above.
3. Call restartMcplServer('editor', newConfig) with endpoint B.
4. Release the commit wait.

Observed: A receives zero renders; B receives push/render with A's event ID and private notice data.

The binding check runs before the asynchronous commit. Dispatch afterward resolves the current connection by server label, so the authorization decision is stale. The later response-time check cannot undo the disclosure that already happened.

Required: recheck current binding and grants after the commit await and immediately before dispatch, and bind the dispatch to the validated connection. Apply the same discipline to other async boundaries that can change audience or authority.

All three outcomes were reproduced on this exact head. For the two render interleavings I deliberately held the host commit promise; admission/reassignment used the existing wire/runtime paths. I would keep these as merge blockers. No full-suite rerun or production-provider exercise was performed in this follow-up.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 6 addressed at b08aa77. All three reproduced; all three fixed, each with a regression test (58 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

1. Retry after a failed commit acknowledges an uncommitted receipt — receipts now carry committed, set only when the durability barrier succeeds. The duplicate path goes through the same acknowledge step as a first acceptance: an uncommitted receipt is committed before it is returned, and if that commit fails again the retry gets -32000 again. Effects are never repeated to obtain a commit. Receipts restored from disk are committed by definition. Test: first sync() throws → -32000; the retry triggers another sync() and is acknowledged; a plain occurrence under the same treatment is in context exactly once.
2. Retraction during the render-start commit wait resurrects the batch — after the await, the assembling instance checks that it is still state.rendering and not cancelled; otherwise it does nothing (no restore, no render). A failed commit restores the batch only for the instance that still owns the subject. Test: hold the host's commit promise, retract over the wire (retracted), release → zero renders, zero pending batches, nothing in the next turn.
3. Endpoint reassignment during that wait leaks notice data — authority (grant, registration, binding) is re-checked after the await and immediately before dispatch; render() then builds and sends the RPC synchronously, so the validated connection is the one used. Test: hold the commit, restartMcplServer to another endpoint, release → zero renders, notice data absent, fallback not delivered.

[antra-tess]

Re-review of b08aa773c034e2b5c13b0393e2384795570de567: build and all 58 coalescing tests pass locally. I independently verified the previous three fixes: the failed-sync retry performs a second sync and survives SIGKILL; retraction during the commit wait stays withdrawn; reassignment during that wait sends no render to the replacement endpoint.

Two major neighboring cases still reproduce:

1. Retraction during async audience lookup resurrects the selected batch

Batch selection before await audience

Reproduction:

1. Accept an initial deferred batch.
2. Start assembly and hold coalescedAudience() after assemble has captured state.batch in its local batch variable.
3. Retract the subject over the wire and receive a committed retracted result.
4. Release the audience lookup.

Observed: one push/render is issued and its content is published despite the acknowledged withdrawal.

The commit-wait ownership check is now correct, but this earlier await has no corresponding check. assemble uses its stale local batch after state.batch was cleared by the retraction, assigns a fresh rendering instance, and the later ownership check consequently passes. A plain replacement or another assembly can also change ownership during that lookup.

Required: revalidate the exact selected batch/current state after audience resolution, before clearing/freezing it. Guard the already-rendering path across its audience await too. The same ownership rule needs to cover all awaits in the transition, not only the durability barrier. This probe deliberately held the host audience promise; the retraction used the wire path.

2. The first new acceptance after restart overwrites earlier recovered work's only durable record

Restore loads recent records into the coalescer only · Acceptance rewrites the host's recent array

Reproduction:

1. Accept deferred E1 on subject A. Before the throttled snapshot, sync/capture the on-disk store and reopen it. E1 is correctly recovered from mcpl/coalescing-recent; one batch is pending.
2. Accept deferred E2 on a different subject B. There are now two batches in memory.
3. Before the new snapshot, sync/capture and reopen again.

Observed: only E2 is recovered and delivered; E1's acknowledged fallback never reaches the request. Immediately after step 2, the durable mcpl/coalescing-recent contains only E2, although the main snapshot does not yet contain E1.

initializePushCoalescer replays the saved recent records, but does not hydrate this.coalescingRecentReceipts, which starts empty. The next recordReceipt rewrites the persisted recent state from that new array and erases the prior recovery inputs. The recovered in-memory state has not been checkpointed yet, so nothing else owns E1 durably.

Required: retain the loaded recent records in the host's writable buffer, or durably checkpoint the recovered complete state before replacing/clearing those records. Verify two consecutive restart windows with an intervening acceptance, including preservation of earlier receipts within their retry window.

Both failures reproduce on the exact current head. The recovery probe explicitly synced and copied the store at each boundary, then reopened those copies so graceful cleanup could not mask the windows. I would keep both as merge blockers. No full-suite rerun or production-provider test was performed for this follow-up.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 7 addressed at 3e300e2. Both reproduced; both fixed, each with a regression test (60 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

1. Retraction during the audience lookup resurrects the selected batch — the batch captured before await audience(...) is now frozen only if, after the await, it is still state.batch (object identity: a joining notice mutates the same batch, a retraction or plain replacement clears it) and nothing else is rendering it. The already-rendering path re-checks that its instance is still current before sharing its outcome. Test: hold the host's audience lookup, retract over the wire (retracted), release → no push/render, nothing published.
2. First acceptance after restart overwrites recovered records — the recent records restored at boot are now also kept in the host's writable buffer, so until the next snapshot flush checkpoints the recovered state the next acceptance appends to them rather than replacing their only durable form. Test: accept A → crash → restart → accept B → crash → restart: A's receipt is still honoured and both fallbacks are delivered.

[antra-tess]

Structural change at 0f55b8d, ahead of the next pass. Rounds 6 and 7 were each an await in the assembly path with an admission landing inside it, guarded one at a time; deferred mode is a primary goal of this work and that pattern does not converge on its own.

One critical section. The coalescer now has a single serial lock shared by every state transition:

• admission (accept) — including its delivery, receipt write and durability commit;
• freezing a batch for rendering — the audience lookup (which may spawn a fork), the render-start snapshot and its commit, the authority re-check and the RPC dispatch;
• settling a render — the authority re-check, publication, and the state update.

Only the push/render RPC itself runs outside the lock, so it stays cancellable; a late or cancelled result is discarded under the lock. By construction an admission never observes a half-done assembly transition and an assembly never acts on a batch an admission has moved on from. The per-await identity and ownership checks from rounds 6–7 are kept as cheap belt-and-braces.

Tests: one added for the last window this closes — a retraction arriving between the render result and its publication now waits for the settlement and comes back noted with the notice appended after the rendered content (previously it could cancel a render whose content was about to be published). The two hold-point tests from round 6/7 are adapted to send their retraction without awaiting it before releasing the hold, since admission now queues behind the freeze; they assert on publication rather than on whether the RPC was issued. 61 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error.

[antra-tess]

Review of 0f55b8d82ff607ae982d3dc5462d328c368ae725, including the structural lock change that arrived during this pass. Build and all 61 coalescing tests pass locally. The shared critical section is a substantive improvement: my additional probes confirm that a queued newer admission survives a failed freeze, and a retraction arriving during publication/module dispatch waits for settlement and receives noted with its correction notice. I am not carrying those superseded interleaving findings forward.

One major liveness failure still reproduces:

A transient render-start commit failure removes the batch's only wake without scheduling recovery

Wake cancellation and failed-freeze recovery

Reproduction with the wire fixture:

1. Send an initial deferred occurrence and receive a successful, durably committed acceptance.
2. Make exactly the next store.sync() fail with a transient EIO; later calls succeed.
3. Run until idle. This failure occurs at render-start persistence, after freeze cancels the subject's wake.
4. Call store.sync() successfully to establish that storage has recovered, retry the original occurrence, and run until idle again.

Observed:

{
  "syncCalls": 2,
  "pendingBatches": 1,
  "pendingWakes": 0,
  "modelCalls": 1,
  "renderCalls": 0,
  "fallbackDelivered": false
}

The single model call did not contain the admitted fallback. The retry returns the original successful receipt, with no new wake or commit attempt, and there is no second model call. The batch remains pending indefinitely in an otherwise idle host, despite being authorized, wake-eligible, and already acknowledged. An unrelated wake or restart can rescue it, but the producer was already told it was accepted and should not need to manufacture another occurrence.

The failure path restores state.batch and sets noRender, but returns without restoring/scheduling its delivery opportunity. The new lock prevents overwriting that batch; it does not restore liveness after the failure.

Required: retain or schedule bounded recovery for an eligible batch whose freeze failed, with backoff and current-authority/policy checks. Once storage recovers, deliver its fallback without requiring unrelated traffic, while avoiding a tight retry loop during a persistent outage. Merely returning the cached receipt on producer retry must not leave acknowledged work asleep forever.

This reproduces on the exact head above. I would keep it as a merge blocker. No full-suite rerun or production-provider test was performed for this follow-up.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 8 addressed at 4b7a9f8. Reproduced and fixed, with two regression tests (63 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

Failed render-start commit leaves the batch asleep — freeze now schedules a bounded recovery for a batch whose commit failed: a per-subject timer with backoff (base 1 s, doubling, capped at 60× base) that re-checks the batch is still pending and still authorized before re-queuing its wake, and is cleared whenever the batch freezes, renders, or is withdrawn. A persistent storage outage therefore costs one attempt per backoff step, not a tight loop of model turns. Separately, a producer retry that is answered from the cached receipt re-wakes a sleeping batch immediately, so the retry the producer is entitled to make is itself enough. Tests: (a) commit fails once at render start → the batch is kept, no render, and the recovery wake delivers the fallback with no unrelated traffic; (b) with the backoff timer parked, a producer retry alone re-queues the wake and the fallback is delivered on the next turn.

Thanks for confirming the critical section holds under your interleaving probes.

[antra-tess]

Review of 4b7a9f8cde8955a9649bc254a4b32de2116785a7: build and all 63 coalescing tests pass locally. I independently verified that a single failed render-start sync now recovers through its timer and delivers the fallback without unrelated traffic. The previous liveness finding is fixed for transient failure.

One issue remains in the persistent-outage path:

Recovery never backs off because firing the timer deletes its attempt counter

Recovery timer

scheduleRecovery computes attempt from the previous map entry, but the timer callback deletes that entry before re-waking the batch. If the next freeze also fails, there is no previous entry, so attempt is 1 again. The advertised 1 s → 2 s → 4 s → … backoff never occurs; every failed attempt returns to the base delay.

Reproduction with the wire fixture:

1. Durably accept a deferred occurrence.
2. Make subsequent store.sync() calls fail persistently.
3. Run the initial turn, then allow three recovery timers to fire and run their queued turns. I used recoveryBackoffMs = 30 to shorten the test.

Observed:

{
  "recoveryWakeAttempts": [1, 1, 1],
  "modelCalls": 4,
  "pendingBatches": 1,
  "fallbackDelivered": false
}

Thus an ongoing storage failure repeatedly starts model turns without the content that caused them, always after the base delay. At the default setting this waits one second after each failed attempt rather than increasing toward the intended 60-second cap. This is the persistent-outage model-turn loop the new recovery policy was meant to bound.

Fix: preserve failure-attempt state after a timer fires, separately from whether a timer is armed. Reset it on successful recovery or disposal of the pending work, not merely on wake dispatch. Add a persistent-failure test that verifies successive attempts/delays increase and cap, followed by successful recovery resetting the series.

The two additional probes (transient recovery and persistent failure) both reproduced the behavior described above. I would fix this before merging. No full-suite rerun or production-provider exercise was performed in this pass.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 9 addressed at b402541. Reproduced and fixed, with the requested persistent-failure test (64 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

Recovery never backs off — the attempt count now outlives the armed timer: firing disarms the timer but keeps the entry, so a freeze that fails again schedules at 2× then 4× … up to 60× the base; the count is forgotten only when the batch freezes successfully or is withdrawn/replaced. A producer retry disarms the timer (it wakes immediately) without resetting the count. The recovery-wake audit record now carries attempt and delayMs. Test: with sync() failing persistently, three consecutive recovery wakes fire at attempts [1, 2, 3] with delays [1×, 2×, 4×] base and the batch stays pending with nothing published; when storage recovers the fallback is delivered and the series is cleared.

[antra-tess]

Additional review of b4025415ef06774d037a1710dbdfd243843f874a. The previously reported backoff fix remains verified (increasing delays, cap, and reset after recovery); this is a different major issue in shared resident contexts.

A secondary resident can consume a message that the primary's watermark still allows coalescing to erase

Placement resolves only its owning agent · Unread check uses that reader's watermark

Reproduced through the PR's loopback wire fixture with two ordinary resident agents, agent and other, sharing context:

1. Accept a plain occurrence E1 with initial: true.
2. Run only other's queued inference (filter the fixture's queued requests to that resident). Verify the actual model request contains E1.
3. Send E2 for the same subject through push/event.

Observed: primary watermark remains sequence 0, secondary watermark is sequence 1. E2 returns replaced, and E1 disappears from both residents' shared context, including the reader that already consumed it.

A second probe used retraction for step 3:

{
  "outcome": "retracted",
  "consumedOriginalStillInContext": false,
  "deletionNoticeInContext": false
}

The original was present in other's model request, but retraction deletes it and suppresses its required correction notice. Expected: preserve the consumed original; ordinary edit reports appended, and retraction with a notice reports noted.

The coalescer remembers a single placement agent and asks only that agent whether the stored message is unread. Ordinary residents share the underlying messages, while consumption watermarks are per-agent. Reading through another resident advances its watermark but leaves the placement owner's watermark behind. Optional per-context consumption does not justify this: these agents are reading and mutating the same shared history, not independent copies.

Required: consumption eligibility must account for every reader of the same underlying shared context/branch, or be tracked at that shared-context boundary. Preserve isolation for genuinely separate conversation contexts. Add two-resident regression coverage for both replacement and retraction after only the non-primary resident has consumed the occurrence.

Both probes reproduced on this exact head. The existing build and 64 coalescing tests passed in the preceding verification of the same commit; I did not rerun the unchanged suite or the full suite for these two additional probes. I would treat this as a merge blocker because it violates the central never-rewrite-consumed-history rule.

AI-assisted review by Codex (GPT-6).

[antra-tess]

Round 10 addressed at 301f540. Reproduced and fixed, with the requested two-resident coverage for both replacement and retraction (66 coalescing tests; full suite unchanged apart from the pre-existing conversation-routing cleanup error).

A secondary resident's read did not count — agreed on the root cause: ordinary residents' context managers are not isolated, so they share the un-namespaced messages slot (and the subconscious merges it read-only), while watermarks were per agent and the unread check asked only the placement's owner. The host now keeps the set of agents that read the shared slot (every ordinary resident plus the subconscious; forks and ephemeral agents are isolated and remain their own only readers). A stored message is unread only if no reader of its slot has compiled past it or folded it into a summary; a tune-out-diverted message is judged by the subconscious alone, as before. The watermark itself is now the chronicle head at compile time rather than the slot's last message, so a compile that reads merged-in messages (the subconscious's view) counts them too.

Tests: with residents agent and other, only one of them (each case) runs the wake and its request contains E1; the edit is then appended and E1 stays in both contexts; the retraction is noted and the notice is appended after E1.

[greptile-apps]

Spurious closed-channel invitation When the first coalesced message arrives for a registered but closed channel, delivery runs before markAccepted() opens it. The incoming handler therefore adds a closed-channel invitation, telling the agent to join a channel that the host accepts as open moments later.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/mcpl/channel-registry.ts
Line: 992-993

Comment:
**Spurious closed-channel invitation** When the first coalesced message arrives for a registered but closed channel, delivery runs before `markAccepted()` opens it. The incoming handler therefore adds a closed-channel invitation, telling the agent to join a channel that the host accepts as open moments later.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Channel identity lost after restart If the host crashes before the throttled snapshot flush, a plain channel occurrence is recovered from a recent receipt that has neither its platform identity nor its delivery agent. A channel-scoped push edit without origin IDs then uses its event ID instead of the original message and thread IDs. If routing has changed, the correction can also reach a different agent.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/mcpl/push-coalescer.ts
Line: 388-391

Comment:
**Channel identity lost after restart** If the host crashes before the throttled snapshot flush, a plain channel occurrence is recovered from a recent receipt that has neither its platform identity nor its delivery agent. A channel-scoped push edit without origin IDs then uses its event ID instead of the original message and thread IDs. If routing has changed, the correction can also reach a different agent.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Empty render can reappear If a completed push/render returns no content and the host stops after writing the cleared snapshot but before clearing recent receipts, recovery replays the older receipt as a pending batch. No message was published for recovery to detect, so a later turn can deliver the fallback even though the server rendered nothing.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/framework.ts
Line: 7465-7467

Comment:
**Empty render can reappear** If a completed `push/render` returns no content and the host stops after writing the cleared snapshot but before clearing recent receipts, recovery replays the older receipt as a pending batch. No message was published for recovery to detect, so a later turn can deliver the fallback even though the server rendered nothing.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.