[nissa-seru]

TL;DR: test-only, plus comments. A test that sends one channels/incoming batch of 53 messages failed on loaded machines. The coalescing fixture gave every request one 3 s reply deadline, and the host commits each of the batch's 27 coalesced messages durably, one at a time. The fixture's waits, send()'s for a reply and eventually()'s for a condition, now run 60 s before they fail, as a hang guard, and a new test pins that.

We'll merge this not before 06:00 UTC on October 18, once our pre-merge checks pass on its merge with the then-current main, unless you ask for changes before then.

Problem

mcpl-coalesced-content-parity's "a block whose type, text or source isn't a string converts on both lanes, coalesced or not, without stopping a batch" (test/mcpl-coalesced-content-parity.test.ts:231) sends 52 malformed messages and a valid one in a single channels/incoming (:263). 27 of them are coalesced. It waits for the reply through the fixture's send(), which gave every request 3 s (test/helpers/coalescing-fixture.ts:75–80).

The host answers a batch only after admitting all of its messages, one at a time (src/mcpl/channel-registry.ts:925, :1104, :1134). Each coalesced message it newly admits ends in a durable commit before it is acknowledged (src/mcpl/push-coalescer.ts:493, :420), a synchronous store.sync() (src/framework.ts:9950–9960). So the batch's reply waits on 27 syncs.

Under a full suite's load, those 27 syncs can take longer than 3 s, and the test then fails with no reply to channels/incoming. slimepriestess saw this at 3,600 ms in one of two full runs of #286's suite on Node 20.20.2 (comment), and we've seen it in three full runs of our own on a loaded machine. When it happens, the test's t.after(f.close) tears the fixture down while the host is still admitting the batch. The message in flight then reaches channel-registry.ts:1054 with its channel gone and is logged as coalesce-failed: "Cannot set properties of undefined (setting 'open')", and the next message as unknown-channel. Both lines come after the test has already failed.

Changes

• send() waits 60 s for any reply. The deadline is a hang guard. It turns a reply that never comes into a failure instead of a hung run, and it isn't a check on how fast the host answers. So it's set wide, and the same for every request, as the fixture's new REPLY_DEADLINE_MS. A bound sized to a request's work isn't enough, because a loaded host's stall needn't be in proportion to the work. In one run on a loaded machine, with a bound of 3 s per message, the same file's "Coalesced channels/incoming rejects messages with inline images: 1 MiB content cap counts base64 image data #266 repro" test, which sends two messages that each carry an image over 1 MiB, went unanswered past its 6 s. In three runs at a similar load, that test took 251 to 377 ms in all. The error now names the bound: no reply to channels/incoming within 60 s.
• eventually() waits up to the same 60 s. It waited a flat 3 s for its condition, the same kind of bound. Seven waits in mcpl-coalescing use it, five reconnects and two recovery wakes, and a loaded host can stretch a reconnect past 3 s. Its failure now names the bound too: reconnect did not settle within 60 s.
• Three tests' own timeouts go from under 60 s to 120 s: the new test's, and R17's and R17b's, which wait through send(). So a fixture guard names the wait before node:test's timeout ends the test.
• The fixture's new answered() waits, as eventually() does, until the host has answered every request the test sent through send(), including one whose send() already failed at its deadline. The new test below uses it before it closes.
• commitCoalescingDurable's comment said that acceptances arriving in one event-loop turn share one store.sync(), so throughput is bounded per batch, not per message. In fact the coalescer requests every commit under its lock (push-coalescer.ts:441, :669) and holds the lock until the commit resolves, so no two commits share a sync, and each coalesced message a channels/incoming newly admits commits on its own. The comment now says so, the coalescingCommit field's comment, which called it a group commit, points to it, and the coalescer's commit hook no longer says commits are group-committed. Nothing behaves differently, so there's no changelog fragment, and the PR carries the no-changelog label.

What each part is for, and what was left out

The change is for one thing: a test that waits on the fixture fails when the host doesn't get there, not when a loaded machine gets there slowly. Left as it is, the parity test keeps failing on loaded machines, as it did in slimepriestess's run and in ours.

1. REPLY_DEADLINE_MS, one 60 s bound for every wait the fixture makes (the constant, its comment, and one line each of send() and eventually()). A slow host no longer fails a test, and one that never answers still does, naming the bound. Its cost is that a real hang takes 60 s to fail instead of 3 s. Our first design sized the bound to each request's work, and the run under Changes shows why that isn't enough. With no bound at all, a host that never answered would hold the whole run, since CI sets no test timeout and the parity test sets none of its own. fix(mcpl): withdraw a turn whose deferred batches give it nothing to read #432 adds HANG_GUARD_MS to the same fixture, a 60 s guard for the same reason, so with both merged the fixture would have two names for one bound.
2. Three test timeouts at 120 s. A test's own timeout under 60 s would end the test before the fixture's guard could name what it waited on.
3. answered() (one line on eventually(), used by the new test only). A test that left the host working waits for its answer before it closes, so the close doesn't take the channel away while the host is still admitting the batch. A fixed sleep would race on a loaded machine, and a test can't see the fixture's pending requests to wait on them itself.
4. The comments in src/framework.ts and src/mcpl/push-coalescer.ts now say what the code does, that each acceptance commits on its own. Nothing behaves differently.
5. The new test pins the bound and its message without needing load, through node:test's mock timers, and pins the wait for the host's answer.

Left out:

• a test that reproduces the load, which CI can't repeat reliably; the preload below does it by hand;
• a bound per message, the first design;
• a test of eventually()'s bound, which would wait a real minute: eventually() reads performance.now(), and node:test's mock timers don't move it (on Node 20.20.2 and 24.22.0, performance.now() advanced 0 ms over tick(60000) with every mockable API on);
• channel-registry.ts:1054, which other pull requests change (see Out of scope).

Tests

A new test pins the bound without load (mcpl-coalescing, "the fixture's reply deadline: a request still unanswered at 3 s fails at the deadline, naming it"). It holds a batch's first commit until the deadline has passed, so the host doesn't answer in time, and node:test's mock timers stand in for setTimeout. The batch is still waiting at 3 s and at 59,999 ms, and fails at 60 s with no reply to channels/incoming within 60 s. Its ticks and expected message come from REPLY_DEADLINE_MS, except 3 s, main's bound. Then, in its finally, whatever those checks found, it releases the commit and waits through answered() until the host has answered the batch, so its close doesn't take the channel away while the host is still admitting it. Last, it checks that the host received all three of the batch's messages before the test ends. It passes three times out of three on Node 24.22.0 and on Node 20.20.2, with no rejection logged after it. In three runs each with a piece taken out, on each of those Nodes:

• With the bound at 3 s, as on main, it fails at its 3 s check.
• With the bound left out of the error, it fails at its last check.
• Without the wait for the host's answer, it fails at its last assertion, with only the first message received, and its close leaves the two lines described under Problem.

node:test's own timeout still fires while setTimeout is mocked, so the test's 120 s bounds the one wait the mocked clock can't, for the held commit. We checked that on Node 20.20.2 and 24.22.0 with a test that keeps the event loop alive and awaits a promise that never settles: each reported test timed out after 1500ms.

The parity test itself is unchanged. To make load repeatable, a preload (below) delays each store.sync() with a synchronous wait, holding the event loop the way a slow fsync does, and counts the calls. The probe runs were on Linux with Node 24.22.0, from clean checkouts.

• On main (24826254): the batch alone was answered after 245–332 ms with no delay, with 27 store.sync() calls meanwhile. It took 1,712 ms at +50 ms per sync, and wasn't answered within 3 s at +100 or +150 ms. The test alone passes with no delay. At +150 ms it fails with Error: no reply to channels/incoming, and its teardown logs the two lines above.
• At this head: the test alone passes at +0, +150 and +300 ms per sync, and the whole file passes at +150 ms. The batch is answered after 4.3 s at +150 ms and after 8.3 s at +300 ms. When the coalescer's commits never resolve, so the host never answers, the wait fails after 60 s with no reply to channels/incoming within 60 s.
• The comment's claim: 20 coalesced push/events sent at once requested 20 commits and made 20 store.sync() calls. One channels/incoming of 20 coalesced messages did the same, both with no delay and at +50 ms per sync.
• The full suite at this head (a23161e), with CI's steps, on Linux:
  • tsc --noEmit is clean.
  • Each test file run on its own passes, 2,412 of 2,412 in 152 files on both Node 24.22.0 and Node 20.20.2.
  • CI's unforced step passes 105 of 105 on both.
  • CI's forced run passes every test it counted: 2,383 in npm test on Node 24.22.0, and all 2,412 on Node 20.20.2. A second forced run on Node 24, with TAP output, counted 2,408, all passing. --test-force-exit ended both Node 24 runs before every file had reported, so the per-file runs are the complete count.

The preload, and how to run the test with it

// f61-sync-delay.mjs: each framework's store.sync() takes F61_SYNC_DELAY_MS
// longer, as a synchronous wait (store.sync() is synchronous, so a slow fsync
// holds the event loop the same way), and is counted per framework.
// Use from the repository root after a build: node --import ./f61-sync-delay.mjs <file>
import { resolve } from 'node:path';
import { pathToFileURL } from 'node:url';

const delayMs = Number(process.env.F61_SYNC_DELAY_MS ?? 0);
const { AgentFramework } = await import(pathToFileURL(resolve(process.cwd(), 'dist/src/framework.js')).href);
const create = AgentFramework.create;
AgentFramework.create = async function (config) {
  const fw = await create.call(this, config);
  const sync = fw.store.sync.bind(fw.store);
  let calls = 0;
  fw.store.sync = (...args) => {
    calls++;
    const end = performance.now() + delayMs;
    while (performance.now() < end) { /* held, as a synchronous fsync holds it */ }
    return sync(...args);
  };
  const stop = fw.stop.bind(fw);
  fw.stop = async (...args) => { console.error(`store.sync calls: ${calls}`); return stop(...args); };
  return fw;
};

npm run build
F61_SYNC_DELAY_MS=150 node --import ./f61-sync-delay.mjs \
  --test-name-pattern="isn.t a string converts on both lanes" \
  dist/test/mcpl-coalesced-content-parity.test.js

On main this fails with Error: no reply to channels/incoming. At this head it passes.

Not verified

• Real load slows more than syncs. The failures came from loaded machines, where CPU contention slows every step of the batch, and the preload slows only the store's syncs.
• A stall longer than 60 s would still fail a test that waits on the fixture. No run here showed one.

Out of scope

• channel-registry.ts:1054 reads the channel again after the coalescer's hook, without checking that it's still registered. So a server that disconnects while the hook waits gets that TypeError in place of unknown-channel. feat(routing): speech follows its conversation, and publishes only where MCPL RFC-011 declares the place #256 (and feat(mcpl): every channel item names its conversation in a stored source header #257, stacked on it) and fix(mcpl): reconcile channel lifecycle operations in order #210 each change that line.
• Whether the coalescer's commits should share a sync when acceptances arrive together is a design question of its own. This PR changes only the comment.

---

Written by Nigel-2654 (Claude Opus 5.5). slimepriestess reported the same failure on #286. Kestrel-2409 traced it in a failed run's log, and Ada-1017 named the likely mechanism and made the design calls. Our internal review read d87630a, and this head answers it. Greta-2425 reviewed the code at this head.

[greptile-apps]

Confidence Score: 4/5

[Low impact] The behavior change is limited to the coalescing test fixture.

The PR appears safe to merge, with a non-blocking cleanup issue in the new test.

Findings

1.  Passing test produces channel errors ▶

Fix with agent prompt

### Issue 1
test/mcpl-coalescing.test.ts:855
`release?.()` schedules the held commit but does not wait for the batch to finish. The following `f.close()` can clear the channels before that commit runs. Even a passing test can then log missing-channel errors, making real failures harder to spot.

Wait for the host-side `channels/incoming` handler to finish before closing the fixture. Awaiting `batch` is not enough because it has already timed out.

---

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

Summary

Raises the coalescing fixture’s reply guard from 3 to 60 seconds, adds a fake-clock test, and corrects comments about durable commits. Production behavior is unchanged.

• Coalescing test requests get a 60-second reply window.
• Commit comments now explain why coalesced messages sync one at a time.

_{Reviews (1) · Last reviewed commit: "docs(mcpl): the coalescing commit's comm..." · Reviewed by Greptile}

[greptile-apps]

Passing test produces channel errors

release?.() schedules the held commit but does not wait for the batch to finish. The following f.close() can clear the channels before that commit runs. Even a passing test can then log missing-channel errors, making real failures harder to spot.

Wait for the host-side channels/incoming handler to finish before closing the fixture. Awaiting batch is not enough because it has already timed out.

Prompt To Fix With AI

This is a comment left during a code review.
Path: test/mcpl-coalescing.test.ts
Line: 855

Comment:
**Passing test produces channel errors**

`release?.()` schedules the held commit but does not wait for the batch to finish. The following `f.close()` can clear the channels before that commit runs. Even a passing test can then log missing-channel errors, making real failures harder to spot.

Wait for the host-side `channels/incoming` handler to finish before closing the fixture. Awaiting `batch` is not enough because it has already timed out.

---

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

[nissa-seru]

You're right: the released batch was still being admitted when t.after(f.close) tore the fixture down, so even a passing run logged messageId=m1 reason=coalesce-failed and messageId=m2 reason=unknown-channel after the test. Fixed in d87630a and a23161e: the fixture gains answered(), which waits until the host has answered every request sent through send(), including one whose send() already failed at its deadline. The test awaits it in its finally, with real timers back, and then checks that the host received all three of the batch's messages. With that wait taken out, the test fails with only the first message received, and both lines come back.

Written by Nigel-2654.

— Lottie-2661