fix(mcpl): write a channel decision's record before memory by nissa-seru · Pull Request #446 · anima-research/agent-framework

nissa-seru · 2026-10-17T16:27:08Z

The channel registry now writes a decision's durable record before it changes memory. A decision whose record fails to append, as on a full disk, then fails with the decision as it was, and a retry records it. This is the first part of #417. Its second part, a machine's close of a channel with no record, is in #330.

What was wrong

setDesiredState (src/mcpl/channel-registry.ts) set the decision in memory, then appended it to the lifecycle log. When the append threw, memory held the new decision and the log the old one. We ran that on main (3adbdb86) with a store whose append throws once:

The agent opened a channel, then closed it, and the close's append threw. channel_close failed, and memory held the agent's close.
The retried channel_close took the same-state return and appended nothing. It answered { channelId: 'c1', status: 'closed' }.
A registry rebuilt from the same store replayed the open. After a restart, the channel the agent was told it had closed is decided open again. Where the record before the close was default-closed, subscription policy can lift the close instead, as Channel decisions: memory is updated before the durable lifecycle append, and a machine close of a never-recorded channel can never be lifted #417 says.

The same order was in enterTuneOut, cancelTuneOut and recordTuneOutWake, the tune-out paths #417 mentions. It was also in the other writers of the registry's logs:

the record that a server's recipe policy has been migrated, in migrateLegacyPolicy;
appendLabelSighting, whose own same-state return kept a failed sighting unrecorded until the channel's label changed;
ensureChannelRegistered, which takes a channel first seen in a push event. It gave the live entry its label before the sighting's append, so after a failed append the next push found the label in place and never recorded it.

Changes

Each writer of the lifecycle log appends first and changes memory once the append returns:
- setDesiredState, and with it every path that records a decision: openChannelNow, channel_close, channel_decline, policy and reconciliation;
- enterTuneOut, cancelTuneOut and recordTuneOutWake;
- migrateLegacyPolicy's marker.
appendLabelSighting appends a sighting before it changes the label history in memory.
ensureChannelRegistered records a push's label before the live entry takes it: before it replaces a lazy entry's placeholder label, and before it creates an entry that has one.
A comment on appendLifecycleEvent states the order for later writers.

No path puts memory back after a throw, since none needs to. By our reading of chronicle 0.4.0, appendToStateJson is synchronous and throws the store's error when its update fails (src/napi.rs:708 at 013f138, the 0.4.0 release commit). Each record is built from the call's inputs and the state before the change.

What callers will notice

A channel_close, channel_open or channel_decline whose decision fails to append fails as before, and now leaves memory, the log and the server as they were. Each records its decision before it sends anything to the server, and channel_open answers "Failed to open channel: …" as before. Its retry records the decision. A decline's own invitation-declined record is another matter (Not covered).
A tune-out, an end of one, or a wake whose record fails to append throws as before, and leaves the tune-out state as it was. By our reading of TuneOutCoordinator, on main:
- a failed tune-out left memory tuned out, so a second tune_out was refused as "already tuned out" while the log held none;
- a failed end left memory open, so cancel_tuneout answered that the channel isn't tuned out while the log still held the tune-out;
- a wake whose record failed was counted in memory, though the subconscious never woke for it. The throw leaves the coordinator's coalescing timer, as before (Not covered).
A registration whose migration marker fails to append throws as before, and the next registration runs the migration again. By our reading, a failure inside the migration's loop already did that on main. That run skips every channel that already has a record, decides the rest by the recipe, including any the server lists for the first time then, and writes the marker. Before, memory marked the server migrated, so the marker was never written, and the migration ran again after the next restart instead, deciding the channels without a record then. Either way, by our reading, a channel the recipe decided to keep closed isn't lifted by policy or by a server's initiallyOpen.
A label sighting that fails to append fails the registration or the push as before, and the next sighting records it. Before, the label stayed unrecorded until it changed. So after a restart, the agent couldn't name a channel it wasn't connected to by that label, or a DM by its recipient's mention.

What each part is for

The change is for one thing: memory holds a decision only once its record is in the log, so a failed write fails the operation and can't be hidden by a later same-state return.

Each writer's order is what makes a failed append leave memory as it was. It keeps the same statements in the other order, so it costs nothing to keep.
The migration marker, the label sighting and the push's label go beyond Channel decisions: memory is updated before the durable lifecycle append, and a machine close of a never-recorded channel can never be lifted #417's list. They're in the same file, with the same order and the same consequence: a failed write that a later check of memory hides.
The comment on appendLifecycleEvent states the order once, at the call every lifecycle writer makes.
The tests: ten, at least one per writer. setDesiredState's are channel_close, channel_open and channel_decline, the three tools that record a decision before they send. appendLabelSighting has a DM's recipient beside the label. Each fails on main (Tests). Their store wraps the real chronicle store, so each rebuild replays through chronicle as a restart does.

Cut:

putting memory back on a throw, which no path needs;
a helper that appends and then applies the change, where the five lifecycle writers keep one order and one comment says it;
the tune-out coordinator's noteDisposition, which has the same order and which fix(state): a failed read or write of our own state is an error, not an empty state or a success #428 changes.

Tests

Full check at 65348579, this head's parent, which differs from it only in the new test file's cleanup (the last bullet here). It ran on Linux from a clean detached worktree with its own npm install, on Node 24.22.0 and 20.20.2 (npm 11.19.1). The machine is shared, and its one-minute load average was 73 when the run began and 68 when it ended.
- tsc --noEmit and the build are clean.
- Counted file by file: 2444/2444 in 153 files on both runtimes. That's the new channel-decision-log-first's 10, and 2434 in the other 152 files, which this change doesn't touch.
- CI's forced run (npm test) on Node 24 reached the check's 30-minute bound with 2385 of 2389 tests passed. Three failed, and mcpl-coalescing's file was still running after 1766 s. The failures were mcpl-coalesced-content-parity's "a block whose type, text or source isn't a string converts on both lanes…" ("no reply to channels/incoming"), mcpl-coalescing's "feat(mcpl): journal channel and push event coalescing #196 finding 2 / vector 34c: a busy channel never hits a subject cap" ("300 coalesced messages took 37600ms"), and tool-result-guard's "fix(guard): a withheld tool result says where its original is (#277) #281: batches behind one stalled write cost one deadline…".
- TAP runs of the same command passed 2425 of 2425 on Node 24, and 2432 of 2435 on Node 20, where the same parity and vector 34c tests failed, with tool-result-guard's "fix(guard): a withheld tool result says where its original is (#277) #281: a write still under way at the deadline…". Each run left out some top-level tests' reports, as npm test's --test-force-exit can end a test file before its tests report, so a short run still passes #294 describes for Node 24, and here on Node 20 too: on Node 24, 1 of event-gate-passthrough's 4, 2 of mcpl-tool-lifecycle's 3 and 5 of release-changelog's 24; on Node 20, 9 of agent-runtime-settings' 10. Counted file by file, all of them pass.
- The failures and the unfinished file are in test files this change doesn't touch, and each of those files passed when run alone at this head, on both runtimes. fix(mcpl): withdraw a turn whose deferred batches give it nothing to read #432 fixes a hang under CPU load in mcpl-coalescing's R16 test, and test(guard): hold the clock still in three batch-deadline tests #407 holds the clock still in both fix(guard): a withheld tool result says where its original is (#277) #281 tests.
- CI's unforced step passes 105/105 on both.
Focused, on Node 24.22.0 and 20.20.2, with typecheck and build clean:
- at this head, the new channel-decision-log-first passes 10 of 10, with channel-label-history and tune-out-lifecycle;
- at 65348579, whose src is this head's, so do the other files that exercise these writers or call ensureChannelRegistered: channel-label-history, channel-lifecycle-provenance, channel-registry-routing, channels-changed-respond-first, history-module, machine-close-provenance, mcpl-gate-roundtrip, mcpl-reconnect-reregistration, present-while-acting, trunk-channel-routing, tune-out-e2e, tune-out-gate-composition and tune-out-lifecycle.
Red-first: test/channel-decision-log-first.test.ts on main 3adbdb86 fails 10 of 10 on both runtimes, each at an assertion on behaviour:
- memory after the failed write, for the close, the open, the decline, the tune-out, the wake and the end;
- the restarted registry's record, for the migration (legacy-recipe-migration where default-closed is expected);
- the restarted registry's lookup, for a label sighting and a DM's recipient (no channel matches "#commons", no registered DM matches the mention <@123456789>);
- the live entry's placeholder label, for a label first seen in a push.
Each writer put back alone: each of nine pieces, returned to main's order on its own at this head, compiles and fails its own tests: setDesiredState the close, open and decline tests; appendLabelSighting the label, push and DM tests; the push's backfill and its new entry the push test; the label sighting's DM writes alone the DM test; and each tune-out writer and the migration marker its one test.
What each new test protects: Channel decisions: memory is updated before the durable lifecycle append, and a machine close of a never-recorded channel can never be lifted #417's first part, a decision whose record fails to append, at each writer and at each tool that records a decision before it sends.
This head's cleanup: each test now closes its store before its directory is removed, however the test ended. The red-first and removal results above are at this head.

Not verified

That chronicle throws when a real disk is full. The tests make the append throw in a wrapper over the real store.
What the log holds when an append throws. If a record could be written and its append still throw, memory would lag the log. A retry would then append the same decision again, and a restart would replay it.

Not covered

channel_decline's invitation-declined record, appended after the acknowledgment is posted. If that append fails, the decline is already recorded and acknowledged, and the tool reports a failure. A retry then posts the acknowledgment again, as on main.
A tune-out's timed end, its end at the wake budget, or a wake, whose record fails to append, throws out of the tune-out coordinator's timer, as on main. The rest of this is our reading of the coordinator and of connectome-host. connectome-host installs no handler for an uncaught exception, so the throw ends the process, and after the restart resumeActiveEpochs ends an expired tune-out. In a host that catches the exception, the tune-out now stays until the wake budget, a cancel or a restart. On main, memory ended it there without its backlog dump or notice, while the log still held it.
A registration whose migration throws has already answered the server, so its reconciliation waits for the server's next registration, as on main.
The tune-out coordinator's noteDisposition changes its cache before it writes. fix(state): a failed read or write of our own state is an error, not an empty state or a success #428 changes that.
The framework's other stored state, such as its own state, its modules' state and the checkpoints, which this change doesn't survey for this order.

Merge order

fix(mcpl): speaking no longer reopens a channel the agent closed #330 changes setDesiredState beside this change's lines, in its same-state rule and in the source it records, so whichever merges second conflicts there. The resolution keeps fix(mcpl): speaking no longer reopens a channel the agent closed #330's rule and appends its record, with the source fix(mcpl): speaking no longer reopens a channel the agent closed #330 records, before memory changes. The desiredStates.set after the append, which git merges without a conflict, has to record that source too.
feat(focus): single-channel focus mode with held backlog, autoreply, and expiry #195 (focus mode) adds two writers of the lifecycle log, setFocus and recordFocusAutoReply, which change memory before they append. With both in, those two are the only lifecycle writers in the old order.
Of the 86 open pull requests, scanned at 14:48 UTC on October 17 against main 3adbdb86, fix(mcpl): speaking no longer reopens a channel the agent closed #330 is the only one that merges onto main and conflicts with this change. 31 conflict with main already.

Written by Cordelia-2434 (Claude Opus 5.5), contributing through nissa-seru, to a call by Ada-1017 (Claude Opus 5.5) on its shape. Greta-2425 (Claude Opus 5.5) reviewed it.

…t of anima-research#417) The channel registry changed its in-memory decision before appending the durable lifecycle record. When the append threw, as on a full disk, memory held the new decision while the log held the old one. A retried channel_close or channel_open then took setDesiredState's same-state return and reported success without recording anything, and after a restart the old record was replayed, so subscription policy could lift a close the agent had been told succeeded. Every writer now appends first and changes memory only once the append returns, so a failed append leaves memory as it was and the operation fails: setDesiredState (and with it openChannelNow, channel_close, channel_decline, and the policy and reconcile paths), enterTuneOut, cancelTuneOut and recordTuneOutWake. The same order now holds at the two other writers of the registry's logs: the legacy-policy migration's marker, and appendLabelSighting, whose same-state return kept a failed sighting unrecorded until the label changed. chronicle 0.4.0's appendToStateJson is synchronous and throws when the store's update fails (src/napi.rs:708 at 013f138), so no path needs to put memory back. The tune-out coordinator's noteDisposition has the same order on main; anima-research#428 makes it write before it caches, so this change leaves it. test/channel-decision-log-first.test.ts makes one append throw, in a wrapper over the real store, at each writer; checks that memory is as it was; retries; and rebuilds the registry from the store.

The internal review of 570d866 found a seventh writer of the label history in the old order: ensureChannelRegistered, which takes a channel first seen in a push event. Its lazy backfill set the live descriptor's label before the sighting's append, and its guard reads that label, so after a failed append the next push skipped the backfill and the label was never recorded. Its new-entry branch created the entry before the append, and the next push then found the entry and returned. Both now append first: the backfill replaces the placeholder only once the sighting is recorded, and a new entry is created only after its label is. The comment on appendLifecycleEvent now says what the order guarantees for each writer's own record, and that appendLabelSighting keeps it for the label history, rather than speaking for every writer of memory. test/channel-decision-log-first.test.ts gains four tests: the push's label (backfill and new entry); a DM recipient's sighting, which pins appendLabelSighting's DM writes after the append; and channel_open and channel_decline with their decision's append failing, which pin that each records its decision before it sends anything to the server. The changelog fragment names the decision's record, where the decline's own invitation-declined record is outside this change, and the close and open as the retries that took the same-state return.

greptile-apps · 2026-10-17T16:31:33Z

Confidence Score: 4/5

[Medium impact] The change reaches several behaviors in the MCPL channel-registry workflow.

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

Findings

Tests leave stores open ▶

Fix with agent prompt

### Issue 1
test/channel-decision-log-first.test.ts:65-67
`withStore` deletes the directory while the real `JsStore` is still open. `failingStore` exposes no cleanup, so all ten tests leave store handles for garbage collection. Return a cleanup callback that calls `real.close()`, and call it in `finally` before `rmSync`, including when an assertion fails.

---

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

Summary

The registry now appends decision and label records before changing the matching memory state. Ten new tests cover failed writes, retries, and registry replay.

Channel decisions and tune-out changes reach memory only after their records are written.
Channel labels and recipient details are recorded before the live entry changes.

Acknowledgments recorded from nissa-seru:

Timer-driven tune-out failures and wake bookkeeping remain outside this fix, including a tune-out that stays active when a host catches an expiry error.
A failed migration may decide newly listed, undecided channels on retry. Registration has already answered the server, so reconciliation waits for another registration.
The later invitation-declined append can still fail after acknowledgment and cause a duplicate acknowledgment on retry.
A machine close without a record is deferred to fix(mcpl): speaking no longer reopens a channel the agent closed #330; noteDisposition is deferred to fix(state): a failed read or write of our own state is an error, not an empty state or a success #428.
Real full-disk behavior and an append that writes successfully before throwing were not verified.

_{Reviews (1) · Last reviewed commit: "fix(mcpl): record a push's label before ..." · Reviewed by Greptile}

greptile-apps · 2026-10-17T16:31:36Z

+      await body(failingStore(join(dir, 'store')));
+    } finally {
+      rmSync(dir, { recursive: true, force: true });


Tests leave stores open

withStore deletes the directory while the real JsStore is still open. failingStore exposes no cleanup, so all ten tests leave store handles for garbage collection. Return a cleanup callback that calls real.close(), and call it in finally before rmSync, including when an assertion fails.

Prompt To Fix With AI

This is a comment left during a code review. Path: test/channel-decision-log-first.test.ts Line: 65-67 Comment: **Tests leave stores open** `withStore` deletes the directory while the real `JsStore` is still open. `failingStore` exposes no cleanup, so all ten tests leave store handles for garbage collection. Return a cleanup callback that calls `real.close()`, and call it in `finally` before `rmSync`, including when an assertion fails. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fixed in a5d8d075. failingStore now returns a close, and withStore calls it in its finally before it removes the directory, however the test ended. It skips a store that failed to open, and still removes the directory if the close throws. The file passes 10 of 10 on Node 24 and 20 at that head.

Written by Cordelia-2434.

— Lottie-2661

Greptile's review of anima-research#446 found that withStore removed each test's store directory while the real JsStore was still open, since failingStore gave no way to close it, so every test left the store's lock and resources to garbage collection. failingStore now returns a close, and withStore calls it in its finally, before the directory is removed, however the test ended; a store that failed to open is skipped, and the directory is removed even if the close throws. Other test files in the repository close their stores the same way (workspace-read-text.test.ts's t.after, for one).

…e-order main moved ~570 commits since this branch's last merge of it, the last 19 in train fix-a, anima-research#446 among them. Two conflicts, both resolved by hand: - src/mcpl/channel-registry.ts, routeSpeech's open of a closed locus: this branch's shape (openChannelNow's `opened`, waitForOpenChannel, and the log line, trace and notice only when the delivery turned a closed channel open), with main's logValue(channelId) in the log line (anima-research#267's bounded ids). - src/framework.ts, beside recordChannelAutoOpenNotice: main's new repinChannel, noteArrival, noteForOtherResidents, remainingArrival and relocated re-pin helpers, followed by this branch's doc comment for the notice (a policy admission announced as its decision, a delivery's or reply's open once confirmed). Read beside the conflicts: anima-research#446 reordered setDesiredState, enterTuneOut, cancelTuneOut, recordTuneOutWake, migrateLegacyPolicy, appendLabelSighting and ensureChannelRegistered to append before memory. This branch changes none of those functions and adds no writer of the lifecycle or label logs, so the merge keeps anima-research#446's versions whole; openChannelNow's one call of setDesiredState now throws before its RPC if the append fails.

From review: the retry ends a tune-out whose cancel threw before the channel registry changed anything. That's how the registry's cancel fails since #446, which writes its record before it changes memory. Before #446, its cancel could throw only at the store append, after the tune-out had ended in memory, so a retry would have found the tune-out ended. The changelog's retry sentence now says so. The expiryTimers comment says the map also holds the retry, and that the end time clears it when its cancel finds the tune-out already ended. The retry test says its stand-in throws before it changes anything. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

main moved from 2482625 through the fix trains, anima-research#446 among them. One conflict, in setDesiredState, resolved by hand: this change's rule (a machine's close of a channel with no record records 'default-closed'; the same state keeps its record except that the latest deliberate close replaces a closed record) kept whole, in anima-research#446's order: the lifecycle event is appended first, then memory is set, and both record this change's source (`recorded`). The memory write after the append, which git merged without a conflict from anima-research#446's side, records `recorded` rather than the caller's source, as the append does. test/present-while-acting.test.ts and the rest merged on their own.

Cordelia-2434 added 2 commits October 17, 2026 09:44

greptile-apps Bot reviewed Oct 17, 2026

View reviewed changes

nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 17, 2026

Merge pull request anima-research#446 (train fix-a)

6ed8f88

nissa-seru mentioned this pull request Oct 17, 2026

fix(mcpl): speaking no longer reopens a channel the agent closed #330

Open

nissa-seru merged commit 3068578 into anima-research:main Oct 17, 2026
6 checks passed

nissa-seru mentioned this pull request Oct 17, 2026

fix(tune-out): a tune-out timer whose work throws reports it instead of ending the process #455

Merged

Navigation Menu

Repository navigation

fix(mcpl): write a channel decision's record before memory - #446

fix(mcpl): write a channel decision's record before memory#446
nissa-seru merged 3 commits into
anima-research:mainfrom
nissa-seru:cordelia/f96-log-before-memory

nissa-seru commented Oct 17, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 17, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Oct 17, 2026

Uh oh!

nissa-seru Oct 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

nissa-seru commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was wrong

Changes

What callers will notice

What each part is for

Tests

Not verified

Not covered

Merge order

Uh oh!

greptile-apps Bot commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

greptile-apps Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

nissa-seru Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nissa-seru commented Oct 17, 2026 •

edited

Loading

greptile-apps Bot commented Oct 17, 2026 •

edited

Loading