Repository navigation
fix(gate): persist sleep and self-wake intent across restarts; re-arm on backward clock step - #194
rufalupa666-netizen wants to merge 16 commits into
Conversation
[Medium impact] The changes reach sleep and self-wake workflows across framework residents. Do not merge yet; several maintenance and restart paths still lose promised wakes. Findings
Fix with agent prompt### Issue 1
src/gate/event-gate.ts:1876
When a sleep expires during quiesce, `wakeFromSleep()` clears the saved intent without checking `this.quiesced`. The framework parks its inference request only in memory, so a restart before `resume()` loses the promised wake. Keep the sleep intent until resume can admit it, as the self-wake callbacks already do.
### Issue 2
src/gate/event-gate.ts:1791-1792
`stopOwned()` stops the inference loop before waiting for streams, maintenance, and operator work. It disposes the gate afterward. If a timer expires during those waits, it removes its saved intent and queues work on the stopped framework. The wake then disappears on restart. Freeze the gate timers when shutdown begins, before those waits.
### Issue 3
src/framework.ts:4651
On a quiesced boot, restored wakes have no timers until this call. If `persistHostMode(null)` throws, such as when its directory becomes unwritable, wake admission is skipped. The `finally` block still reopens the host with `quiesced` already false, so another `resume()` returns immediately. Saved future wakes remain unarmed even after storage is repaired. Ensure admission runs whenever resume leaves the host serving, or keep resume retryable after this failure.
### Issue 4
src/gate/event-gate.ts:2228
Clearing `sleepTimer` here breaks a supported stop/start cycle on the same framework instance. With a caller-owned store, `start()` can run again, but it neither recreates the gate nor admits its retained intents. The sleep deadline remains set while its wake timer is gone, so the promised wake never runs. Re-arm retained wakes when the framework starts a new lifecycle.
### Issue 5
src/gate/event-gate.ts:2016
A saved wake can name a conversation fork that startup never recreates. Forks inherit the template's tools, but only their generation counters survive restart; the next message creates a new generation. Recovery removes the old fork's intent, then both its notice and inference request are dropped because that agent is absent. Restore the saved target or explicitly account for the undeliverable wake before consuming its intent.
### Issue 6
src/gate/event-gate.ts:2008
If the wall clock moves past `wakeAt` while a live self-wake timer still has time left, `resume()` admits the overdue intent but leaves its timer armed. The callback created by `armSelfWake()` does not check whether that intent was already consumed, so it can send another notice and inference request. Cancel the existing timer when admission consumes the intent, and check that the intent is still current before firing.
### Issue 7
src/gate/event-gate.ts:1652
After an overdue self-wake fails journal admission, its intent remains in `selfWakeIntents` without a timer. A later heartbeat can supersede that fallback, but `onInferenceStarted()` only removes the intent inside `if (selfWake)`. Even if storage has recovered, cancellation is never attempted, so a later restart revives the obsolete wake. Cancel pending intents independently of whether a timer handle exists.
### Issue 8
src/gate/event-gate.ts:1833
The exported `EventGate.clearSleep()` changes from a boolean to an object. Existing callers using `if (gate.clearSleep())` now always take that branch, including when `woke` is false. Callers expecting a boolean also stop type-checking. The framework caller is updated, but embedding callers are not protected. Preserve the boolean method and expose the detailed receipt separately.
### Issue 9
src/framework.ts:1749-1750
When a resident has a saved future sleep and a deferred MCPL push batch was pending at shutdown, startup restores the sleep but then queues the batch’s inference request without checking the gate. Starting the framework can run the resident before its promised wake deadline, even without new server input.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryPersists
Reviews (5) · Last reviewed commit: "test(gate): pin resume recovery and vali..." · Reviewed by Greptile |
6d7f017 to
bac160e
Compare
|
Review at 6d7f017 adjudicated; all ten findings addressed at bac160e. Verified each against the code before changing anything.
Receipts at bac160e (on upstream main 5498805): Overlap note: antra-tess's |
|
@greptileai bump |
29341f7 to
e815e8c
Compare
|
Round 2 (29341f7) adjudicated; four findings, all valid, fixed at e815e8c.
Receipts at e815e8c (on 5498805): 986 pass, 0 fail; @greptileai bump |
e815e8c to
de7db7c
Compare
|
Rebased onto main df97a85; round 3 (e815e8c) adjudicated; fixed at de7db7c.
Overlap note: Receipts at de7db7c (on df97a85): 1136 pass, 0 fail; @greptileai bump |
| // Restore sleep suppression before MCPL startup can release buffered input. | ||
| framework.eventGate?.recoverWakeIntents(); |
There was a problem hiding this comment.
When a resident has a saved future sleep and a deferred MCPL push batch was pending at shutdown, startup restores the sleep but then queues the batch’s inference request without checking the gate. Starting the framework can run the resident before its promised wake deadline, even without new server input.
Knowledge Base Used: Event gate pipeline
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/framework.ts
Line: 1749-1750
Comment:
**Restored batches bypass sleep**
When a resident has a saved future sleep and a deferred MCPL push batch was pending at shutdown, startup restores the sleep but then queues the batch’s inference request without checking the gate. Starting the framework can run the resident before its promised wake deadline, even without new server input.
**Knowledge Base Used:** [Event gate pipeline](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/event-gates.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
LariTesserae
left a comment
There was a problem hiding this comment.
Read the full diff against main (0.19/df97a85 base). Mechanism is sound and matches the qa-staging #33/#25 findings: journal as a sibling of gate.json, atomic tmp+rename with 0600, corrupt journal quarantined rather than trusted, recovery hooked in the framework before MCPL startup, [wake-recovery] message with the measured overdue delta instead of silence, re-arm on backward clock step, long sleeps chunked under the setTimeout ceiling. Admission-before-side-effect on overdue recovery (persist first, restore in memory on failure) is the right order.
Two questions, neither blocking:
- Recovery inference timing.
recoverWakeIntents()runs before MCPL subsystems start, and an overdue intent callsrequestInferenceFnright there. If the inference loop can pick that request up before MCPL servers have registered their tools, the recovered agent wakes tool-less for one turn. Is the request guaranteed to sit in the queue until the loop starts after init? A one-line note (or a test asserting the recovered turn sees its tools) would settle it. - Gate-global sleep. Sleep is per gate, not per agent (unchanged semantics), so
agentNamein the journal is informational forsleepand authoritative for self-wakes. Worth one sentence in the journal's doc comment so a future reader doesn't assume per-agent sleep.
Nit: suppressed is persisted but reset to 0 on recovery — the comment says why; fine.
Mechanism verified against the live gate code path; receipts in the description match what I see. From the verdict loop on the QA track.
de7db7c to
ddb97cb
Compare
|
Thanks. Both answered at 83df97f.
Receipts at 83df97f (on df97a85): 1136 pass, 0 fail; |
61ca6cd to
94cd49a
Compare
94cd49a to
7e2376e
Compare
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R1 at 7e2376e · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: CHANGES_REQUESTED (the formal review event is left to the maintainer). Finding ids R1-n are stable; cite them in fix commits.
Reviewed at 7e2376e against main 06303d8 (merge-base 92125b0; the two newer main commits only touch Greptile config).
Verdict: changes requested. The design is right, and most of it holds up:
- the journal is written before
sleepandskip_replyreturn; - future deadlines come back after both a kill and a graceful stop;
- the early-callback re-arm closes #25 for sleep.
This PR exists to make a promised wake survive a restart, and there is still a path where it doesn't. Inside create(), recovery deletes an overdue intent from the journal before steps that can still throw, and nothing puts it back. The test meant to cover this fails before recovery runs, so it passes no matter what recovery does. The same pattern (delete from the journal, hold the wake only in memory) also loses the wake when the host is restarted while quiesced.
We have not reviewed this PR before, so every finding below is new. Where the PR thread says a fix landed, I checked the commit it cites (see R1-1 and R1-4).
Blocker
R1-1. An overdue wake is deleted before create() can still fail, and the failure test never reaches recovery.
What the code does:
src/framework.ts:2467callsrecoverWakeIntents()before:- the MCPL prefix-collision checks (
:2477,:2483); initializeMcpl()(:2490), which rethrows server startup failures;- the tune-out coordinator (
:2495-2525).
- the MCPL prefix-collision checks (
create()has no catch around any of these steps.- For each due intent,
src/gate/event-gate.ts:1973-1979deletes the entry and rewrites the journal. Then:1986-1997adds the[wake-recovery]marker and pushes the inference request intopendingRequests, which is held only in memory. - The comment at the call site (
framework.ts:2463-2466) is accurate whencreate()succeeds: the request waits forstart(), so the recovered turn sees the MCPL tools. It doesn't cover the case wherecreate()fails.
Probe at this head:
- Setup: an overdue
sleepin the journal, and two MCPL servers sharingtoolPrefix: 'dup'.create()throws at:2483, after recovery has run. - The journal is left as
{"version":1,"selfWakes":[]}. - The next successful
create()on the same store queues no recovery request, and there is no[wake-recovery]marker in the context.
So the wake disappears without a trace, which is the original bug, now reached through a failed boot. A deploy with a broken MCPL server config, or a server that fails to start once, is enough to trigger it.
The test does not cover this:
failed Framework.create leaves a due intent in the journal(test/event-gate-selfwake.test.ts:268-287) makescreate()fail withtoolResultInlineMaxChars: 10.- That value is checked at
framework.ts:2394-2399, before recovery. With the test instrumented,recoverWakeIntentsis called 0 times. - Its comment, "validated after gate construction", is true, but recovery no longer runs at gate construction.
The thread says this was fixed and tested (round 1 #2; round 3 #1, "create() fails after recovery → journal intact, no live timer"). Neither de7db7c nor 7e2376e has a catch/dispose path in create(), or a test that fails after recovery.
Fix: split recovery into two steps.
restoreWakeIntents(): read the journal, restore sleep suppression, arm future timers, and make no journal writes. Call it where recovery is called now, so buffered MCPL input still sees the sleep.admitOverdueWakeIntents(): the current admission sequence (delete from the journal, write the marker, queue the request). Call it as the last step ofcreate(), aftercheckToolPatterns()and just beforereturn framework.- Wrap everything from the restore call to the return in a try/catch that calls
framework.eventGate?.dispose()and rethrows. This also fixes R1-3.
This keeps at-most-once: the journal delete still happens before any side effect. The window for losing the wake shrinks to a crash between create() returning and the turn starting.
Test changes:
- Replace the cap-validation case with a config that fails after recovery. Duplicate
toolPrefixvalues work and need no child process. - Put one overdue and one future intent in the journal.
- Assert that both are still in the journal afterwards and that no gate timer is live.
Major
R1-2. A quiesced boot deletes the overdue intent from the journal but only holds the wake in memory. A second restart during the same maintenance window loses it.
- Recovery runs after the persisted quiesce flag is restored (
framework.ts:2438-2442), and admission still deletes the journal entry (event-gate.ts:1979). - The request is then parked in memory by
processInferenceRequests(framework.ts:10320-10340). The quiesced-boot banner (:2447-2450) itself says parked wakes "are gone" after a restart. - Probe with two quiesced boots in a row:
- Boot 1 parks one
wake-recoveryrequest, and the journal'ssleepentry is already gone. - Boot 2 has no request.
- Boot 1 parks one
- Quiesce is the maintenance mode, so restarting while quiesced is the normal case, not an edge case.
- Fix: while quiesced, run only the restore step and leave overdue intents in the journal. Admit them in
resume(), the first point at which a wake can actually start a turn.
R1-3. When create() fails, the restored future timers stay live. They later delete the intent from the journal and send their wake to a framework that was never returned.
- Future sleeps and self-wakes are armed during recovery (
event-gate.ts:1960-1968). Nothing disposes the gate whencreate()throws. - Probe: a future sleep 1.5 s out, with
create()failing at the prefix check.- The orphaned gate's
sleepTimeris still live. - At the deadline,
wakeFromSleepwrites the deletion (:1868), and the journal'ssleepentry goes from present to absent.
- The orphaned gate's
- Who this affects:
- Any embedder or supervisor that catches the rejection and keeps the process running.
- Hosts that log the error and return. The timer is ref'd, so the process stays alive until the deadline (sleeps can last hours) instead of exiting so a supervisor can restart it.
- Fix: the try/catch and
dispose()from R1-1. Add a test that asserts no live timer and an intact journal after a failure that happens after recovery.
Minor
R1-4. recoverWakeIntents() is not idempotent for self-wakes.
armRestoredSelfWake(event-gate.ts:2002-2009) replaces the map entry without clearing the previous timer.- With
autoRecover: trueplus an explicit call (which the method's doc comment invites), a probe shows:- Without
dispose(), one future self-wake fires twice and produces 2 inference requests. - With
dispose(), the leaked first timer still fires afterwards. It keeps the process alive until then, and empties the journal.
- Without
- Sleep is not affected, because
armSleepTimerclears its predecessor. - The thread (round 3 #2) says recovery was made idempotent and tested with
autoRecoverplus an explicit call. Neither the guard nor the test is in de7db7c or 7e2376e. - Fix:
clearTimeoutany existing timer for that agent before arming a new one;- add a
recoveredflag so a second call does nothing; - add the test the thread describes.
R1-5. On the normal (non-recovery) paths, a failed journal write is only logged, so what's on disk can silently differ from what was reported.
tryPersistWakeState (:1943-1948) logs the error and returns false. Only overdue admission checks that result. Everywhere else:
setSleepandarmSelfWake(:1745,:1780) report success with a concreteuntil, but the promise won't survive a restart.- If
wakecallsclearSleepand its write fails (:1833), the agent comes back asleep after a restart. - A sleep or self-wake that fires while its delete fails (
:1868,:1784) runs now and runs again as overdue after a restart. That is a duplicate wake, which breaks the at-most-once promise.
Arming the live timer before writing (Greptile round 1 #3) is the right call. The sibling writeConfig (:959-968) instead throws the write error back to the tool caller.
Fix: keep the live timer, but report the failure in the sleep and skip_reply tool results (for example durable: false plus one sentence of explanation). That way the agent isn't promised something the host can't deliver.
R1-6. Entry validation accepts timestamps outside the Date range. Such an entry is deleted, and then startup throws.
validWakeIntent(:1882-1890) only checksNumber.isFinite.- Probe: a journal
sleepwithwakeAt: -1e20passes validation.- It is admitted and deleted (
:1979). - Then
new Date(...).toISOString()(:1992) throwsRangeError: Invalid time valueout ofrecoverWakeIntents(). - That fails one
create()and leaves the journal as{"selfWakes":[]}.
- It is admitted and deleted (
- Separately, when the quarantine rename fails,
:1915still logs "quarantined as …". - Fix:
- limit
armedAtandwakeAtto ±8.64e15 and requirewakeAt >= armedAt; - log the quarantine path only after a successful rename.
- limit
R1-7. The changelog and PR description promise more than the code does.
changelog.d/durable-wake-intents.fixed.mdsays "reconcile overdue and overdue wakes exactly once":- "overdue" is duplicated;
- the design is at-most-once, as the PR body itself says, and with R1-1 and R1-2 the count can also be zero.
- Suggested wording: "…reconcile overdue wakes at most once, with a visible
[wake-recovery]marker…".
- The PR body is out of date:
- it still describes "startup reconciliation in the constructor"; recovery now runs in
create(), with opt-inautoRecoverfor standalone gates; - it says "six cases", but there are 14 new tests;
- it quotes receipts at 6d7f017.
- it still describes "startup reconciliation in the constructor"; recovery now runs in
Verified fine
Facts from the nomination
- Both facts are confirmed at 7e2376e.
recoverWakeIntents()is called atframework.ts:2467, beforeinitializeMcplat:2490, with the comment at:2463-2466. - Overdue intents are removed from the journal (
event-gate.ts:1973-1979) before the queued request can run; it runs only afterstart(). - The comment's claim about MCPL tools holds for a successful
create().
Atomic journal writes
- Writing a temp file in the same directory and then
renameSync(:1937-1940) is atomic against process death. After a SIGKILL or OOM you get the old journal or the new one, never a half-written one. - Overdue admission writes the journal before any side effect, and restores the intent in memory if the write fails (
:1977-1985, tested). - Neither the file nor the directory is fsynced, so writes are not safe against power loss. That matches the framework's own
writeRecoveryFileand the Discord awareness outbox, so it's an existing convention, not a regression. - One nit: those siblings name the temp file
<path>.<pid>.<uuid>.tmp; this one uses only the pid. A stale temp file left by a crashed process with a reused pid (pid 1 in containers) gets truncated and keeps its old mode, instead of being created with 0600.
Corrupt-file quarantine
- Parse errors, a wrong shape or version, and unreadable files are moved aside to
.corrupt-<ts>rather than overwritten (:1893-1917). - Per-entry type validation skips bad entries and keeps the good ones (tested).
- Nothing is written when there is nothing to change (tested).
- Caveats are in R1-6.
Re-arm after a backward clock step
- When the timer fires early,
wakeFromSleep(:1853-1861) re-arms for the remaining wall-clock time. Delays are split into chunks of at most 2^31−1 ms. setSleepandclearSleepclear the old timer, so a stale callback can't end a newer sleep.- Restored self-wakes also recheck their wall-clock deadline.
- Self-wakes armed in the current process (at most 1 h) still fire on monotonic time. That matches the description, which scopes the fix to sleep, but it's worth a comment in the code.
- After a forward clock step, suppression still ends at the wall-clock deadline while the wake waits for the monotonic timer. That behaviour predates this PR.
Graceful shutdown
dispose()(:2181-2188) clears both sets of timers and keeps the journal.- A planned restart re-arms the wake instead of waking early (tested).
Earlier Greptile item "Restored batches bypass sleep"
- This isn't introduced here. Coalesced MCPL pushes go through the gate when they arrive (
push-handler.ts:296-312, using the gate'sshouldTriggerInference). - A restored batch replays that same arrival-time decision, just as a live batch firing after a later
sleepwould.
Journal path
- The journal name comes from the config file name (
gate.json→gate.wake-intents.json), so gates with different config files don't share a journal. - Sleep is still gate-wide rather than per-agent, as the doc comment now says.
Tests at 7e2376e
npm run build && npm test: 2227 of 2228 pass.- The one failure is
workspace-watcher"re-attaches after the root is removed and recreated". It fails the same way on main 06303d8 in the same environment, so it isn't related to this PR. - The PR's own test file passes 23/23.
- No CI checks are reported on the PR head.
Method. Two independent passes, merged after each claim was re-verified.
Sol pass. Codex gpt-5.6-sol (xhigh, read-only, no network) read the diff and runtime source without running anything. 541 s wall time, 78 shell commands, 3.44 M input tokens (3.24 M cached), 23.8 k output tokens (14.2 k of them reasoning).
Claude pass. Claude (Opus 5.5) built the head and ran the full suite. It also ran targeted probes against the built dist/: a failed create() with overdue and future intents, the PR's failure test instrumented, two quiesced boots, double recovery, and an out-of-range timestamp. It also read the PR thread and earlier pushed heads (de7db7c, ddb97cb, 61ca6cd, 94cd49a).
Who found what. Both: R1-1 (including that its test never reaches recovery), R1-3, R1-4. Claude only: R1-2, and that fixes claimed in the thread are absent from the commits it cites. Sol only, then confirmed by Claude by probe or by reading the code: R1-5, R1-6.
Downgraded or rejected from Sol. Write durability and multiple writers: downgraded from major to a note, because sibling recovery writers follow the same convention and sharing across processes would require sharing a single-writer store. "Startup wakes bypass recovered sleep": rejected as a regression, since gating happens at arrival and the behaviour is inherited. Clock-step asymmetry: folded into Verified fine.
Limits. Sol could not run code or read the PR thread.
|
Thank you for the probes. R1-1 through R1-7 all look right to us, and we'll take them in that order, citing the ids in commits. On the two fixes the thread claimed and the commits don't carry (idempotent recovery, create-fails-after-recovery test): that's on us — they were lost when the branch was squashed, and we didn't diff the result against what we'd described. We'll restore them with tests and check the final diff against the thread before reporting. |
|
R1 addressed at 3b87a3f, one commit per finding:
|
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R2 at 3b87a3f · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: CHANGES_REQUESTED (the formal review event is left to the maintainer). Finding ids R2-n are stable; cite them in fix commits.
Reviewed at 3b87a3f against main 91e70ed (merge-base 92125b0). Main is 87 commits ahead and has rewritten about 700 lines of src/framework.ts, so I also built and tested a local merge of this head into main. The merge is clean, and create() and resume() keep the same shape there.
Nightly dual review, round R2. Reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5. Their results were merged, and the Claude leg re-verified every claim. Finding ids R2-n are stable; please cite them in fix commits.
Verdict: changes requested.
- The round-1 blocker is fixed by construction. An overdue intent now leaves the journal only in the last statement of a successful
create(). The new test reaches recovery and fails if the fix is reverted. - What remains is the same failure R1-2 described, reached a different way. A wake whose deadline falls during a quiesce window is still deleted from the journal and held only in memory, so a second restart in that window loses it (R2-1). The description says this case is closed.
- The other findings are small. One is new this round: the
durablereceipt forskip_replyis written into the model's own tool call (R2-3). The rest are test gaps and narrow races.
Round 1 dispositions
| Id | Disposition at 3b87a3f | Evidence |
|---|---|---|
| R1-1 | Fixed | restoreWakeIntents() (framework.ts:2465) makes no journal writes. Every fallible step after it is inside the new try: prefix checks :2475-2485, initializeMcpl :2489, tune-out coordinator :2494-2524, checkToolPatterns :2555. Admission is the final statement before return (:2557), and the catch disposes the gate (:2560). The new test makes create() throw at the duplicate-prefix check, after restore has run; it captures the restoring gate. It then asserts that both intents are still in the journal and that no timer is live. Reverting the fix makes it fail (mutation table in R2-4). |
| R1-2 | Partial | The case as reported is fixed: a quiesced boot leaves the overdue intent in the journal (:2557). Probe: resume() then admits it once. The journal entry is removed, pendingRequests holds agent:wake-recovery, and the marker is in context. The same loss remains for a wake that falls due during the window (R2-1), and no test exercises the resume() half (R2-4). |
| R1-3 | Fixed | dispose() in the catch. The test asserts that no timer is live. A narrower residual remains: a restored timer that fires during startup, before the failure (R2-2a). |
| R1-4 | Fixed in code, test doesn't pin it | The guards event-gate.ts:1961-1962 and :1982-1983 stop a second call, and armRestoredSelfWake clears the previous handle (:2033-2034). The test passes with both removed (R2-4). |
| R1-5 | Fixed as proposed | durable is returned from setSleep and armSelfWake. The sleep result carries durable and a note (framework.ts:18393-18394); the skip_reply note and durable field are at channel-registry.ts:2978-2981. The delete-failure cases listed in R1-5 are unchanged (R2-5). The receipt's transport has a new problem (R2-3). |
| R1-6 | Fixed in code, test doesn't pin it | validWakeIntent bounds timestamps to ±8.64e15 and requires wakeAt >= armedAt (event-gate.ts:1887-1894). The quarantine message is logged only after the rename succeeds (:1918-1923). The test passes with the bounds removed (R2-4). |
| R1-7 | Fixed | The changelog now says "at most once", and the description has been refreshed. Two of its new sentences overreach (R2-1, R2-2a; listed in R2-7). |
Major
R2-1. A wake that falls due while the host is quiesced is still deleted from the journal and held only in memory. A second restart in the same window loses it.
What the code does:
- By design, gate timers bypass quiesce (
event-gate.ts:1659-1666). A sleep or self-wake that fires while the host is quiesced requests a turn, and the framework parks that request in memory untilresume(). - Every timer callback deletes its journal entry before making that request:
- the
armSelfWaketimer (:1784-1786); wakeFromSleep(:1866-1870);fireRestoredSelfWake(:2045-2047).
- the
- The result is the R1-2 state: the wake is gone from the journal and survives only in memory.
Probes at this head (built dist/, real AgentFramework.create):
-
Quiesced boot. A sleep is due in 0.8 s and a self-wake in 0.9 s. After 1.3 s:
- the journal is
{"version":1,"selfWakes":[]}; pendingRequestsis[agent:sleep, agent:self-wake].
Then
stop(), boot again (still quiesced), andresume(). No request is released and there is no[wake-recovery]marker. The[self-wake]notice survived as a deferred write and lands at resume, but nothing starts a turn. The sleep leaves no trace. - the journal is
-
Quiesce entered at runtime. The host is serving and the agent calls
setSleep(1). The operator quiesces, the deadline passes, thenstop(), a quiesced boot, andresume(). Same result: nothing is pending.
Why it matters:
- The description says "another maintenance restart cannot lose an in-memory parked wake". Quiesce is the mode where restarts are routine.
- A maintenance window only has to span a sleep deadline, or outlast a self-wake (at most 1 h), for this to happen.
- This is not a regression from main, which loses every wake on restart. It is still the failure R1-2 described, and the PR says it is fixed.
Fix:
- In the three timer callbacks, when the gate is quiesced, release the timer handle but leave the intent in memory and in the journal.
- For sleep, suppression still ends on its own once
sleepUntilpasses. admitOverdueWakeIntents()already treats0 < sleepUntil <= nowas due.
- For sleep, suppression still ends on its own once
- Make admission runnable on every
resume(), not once per process. Either dropoverdueWakeIntentsAdmittedor reset it onquiesce().- This also retries an admission whose journal write failed. Today the one-shot flag (
:1982-1983) strands that intent until the next restart.
- This also retries an admission whose journal write failed. Today the one-shot flag (
- Optional: call admission after
persistHostMode(null)(framework.ts:4652) instead of before the deferred-write flush (:4651).- Today, a crash during the flush boots quiesced with the intent already gone.
- Moved, the same crash boots serving with the intent still in the journal.
- Test: a quiesced boot with a future intent that falls due, then a quiesced restart, then
resume(). Expect exactly one recovery request.
R2-2 below proposes a restore/arm split that also covers the boot half of this finding.
Minor
R2-2. Restored future timers are armed before the startup commit point.
restoreWakeIntents() arms timers for future intents immediately (event-gate.ts:1972, :1976), before MCPL startup and before admission. Two consequences follow.
- (a) A wake can be lost even though
create()fails.- Probe: a restored self-wake is due 300 ms out, and
initializeMcplis stubbed to fail after 700 ms. - The timer fires during startup. It deletes its journal entry and queues a turn on a framework that is never returned.
- After the rejection, the journal is
{"version":1,"selfWakes":[]}. - This is R1-3's lost wake, reached before the failure instead of after it. The window is as long as MCPL startup.
- Probe: a restored self-wake is due 300 ms out, and
- (b) Double wake.
- Suppose a restored self-wake's deadline passes between restore and admission, before its callback has run.
- Admission (
:1995-1996,:2002) admits the intent and deletes it, but leaves its timer armed. fireRestoredSelfWake(:2044) never checks that the intent is still current, so the timer later starts a second turn.- Probe with a standalone gate and an injected clock:
[wake-recovery]plus awake-recoveryrequest, then[self-wake]plus aself-wakerequest. - In
create()the window is the synchronous tail after the lastawait, so it is narrow. - Sleep is not affected, because
wakeFromSleepreturns early whensleepUntil === 0.
- Fix:
- Let restore load state only: sleep suppression and the intents map.
- Move timer arming into the admission step. For each intent, it either admits it (due) or arms it (future).
- On a quiesced boot this keeps future timers off until
resume(), which covers the boot half of R2-1. - Independently: clear the agent's timer when admission consumes its self-wake, or have
fireRestoredSelfWakereturn whenselfWakeIntents.get(agent) !== intent.
R2-3. The skip_reply durability receipt is written into the model's own tool call and persisted.
How it gets there:
framework.ts:14997setsinputObj._wake_durable = durableonenrichedCall.input.enrichedCallis a shallow copy (:14878), soinputis the same object the stream yielded.- In membrane 0.5.87, that object is also the
tool_useblock'sinputinroundContent: the yielded call is built withinput: block.inputfrom the sameresponseBlocks. - The framework persists that block as the assistant turn (
:12165,:12206).
Probe: framework, mock membrane, stub channel registry, and a call to skip_reply({reason: "later", wake_in_seconds: 600}). The stored assistant tool_use input is {"_wake_durable":true,"reason":"later","wake_in_seconds":600}.
Consequence: every later request replays the agent's own call with a field the agent never wrote, and the store keeps it permanently. The registry handles the receipt correctly; the problem is how it gets there.
Fix:
- Don't mutate the shared object. For example,
enrichedCall.input = { ...inputObj, _wake_durable: durable }; reassigning the copy's property leavescall.inputuntouched. - Alternatively, pass the receipt alongside the call.
- The existing
delete inputObj.wake_in_secondson the no-gate branch (:15000) aliases the same object. It isn't this PR's, but the same change fixes both.
R2-4. Three of the new boundary tests still pass with their fix reverted.
Mutation runs: revert one fix in the built dist/, then rerun event-gate-selfwake.
| Reverted | Tests failing |
|---|---|
| admit right after restore (R1-1) | 3 |
no dispose() in the catch (R1-3) |
1 |
| admit on a quiesced boot (R1-2) | 1 |
setSleep/armSelfWake always report durable: true (R1-5) |
1 |
resume() never admits (R1-2) |
0 |
no restore guard and no clearTimeout (R1-4) |
0 |
| no timestamp bounds or order check (R1-6) | 0 |
resume(): no test calls it.quiesced create restores overdue intent without admitting itcovers only the boot half.- R1-4: the test asserts
selfWakeTimers.size === 1, which holds with or without the fix. The secondsetoverwrites the first handle, and the leaked handle is not in the map. Count fired requests instead, or assert that the journal is read once. - R1-6:
- The
-1e20value is in the sleep slot. The new admission skipssleepUntil <= 0, so the value never reachestoISOString. Put an out-of-range pair such asarmedAt: -2e20, wakeAt: -1e20on a self-wake instead; without the bounds, that throwsRangeError. - The backwards entry (
armedAt: 10, wakeAt: 9) is simply admitted as overdue when the check is missing, which leaves the same map size. Assert that no request or marker is emitted.
- The
- R1-5: the test covers the gate's return value only. Neither the
sleepresult'sdurable/notenor theskip_replynote is exercised.
R2-5. "At most once" holds only while journal writes succeed.
- R1-5's fix covers arming. Deletions still ignore the result of
tryPersistWakeState:- self-wake fire (
event-gate.ts:1786); - sleep expiry (
:1870); - restored self-wake fire (
:2047); - cancellation at turn start (
:1649); clearSleep(:1835).
- self-wake fire (
wakereturns{ woke }without checking the write (framework.ts:18337-18339).- If the delete fails, the wake runs now, its intent stays in the journal, and the next restart runs it again as overdue. That is a duplicate turn. A failed
clearSleepwrite brings the sleep back after a restart. - Fix for this PR:
- qualify the claim as "at most once while journal writes succeed", in the changelog and in the journal doc comment;
- return
durablefromwakethe waysleepdoes.
- Retrying failed deletes can wait.
R2-6 (nit). armSelfWake reads the clock twice (event-gate.ts:1777-1778).
- If the clock steps back between the two reads by more than the delay,
wakeAt < armedAt. - The validator added in this round then rejects the entry at restart, even though arming reported
durable: true. - Fix:
const armedAt = this.now();and thenwakeAt: armedAt + ms, assetSleepdoes.
R2-7 (nit). Prose that is now partly stale.
- The quiesced-boot banner (
framework.ts:2449-2450) says wakes parked before the restart "are gone". Sleep and self-wake intents that were overdue at boot now survive and are admitted atresume(). - The
resume()comment (:4681-4683) says "one-shot wakes are lost". - The recovery marker (
event-gate.ts:2016) says the wake became overdue "while the host was offline". When it is admitted atresume(), the overdue time includes the quiesce window. - The description says "A failed create disposes restored timers and leaves overdue and future intents durable"; R2-2a is the exception. It also says "another maintenance restart cannot lose an in-memory parked wake"; R2-1 is the exception.
Verified fine
Nomination facts
- R1-1 to R1-3 are described accurately.
- One small correction: round 1 was a GitHub review of type Commented, whose body carried the changes-requested verdict; the formal review event was left to the maintainer. The substance stands.
R1-1, by construction
restoreWakeIntents()performs no writes, andreadWakeStatecatches its own read and parse errors.- Every statement between restore and
returnis inside thetry. The SIGUSR2 hook has its own catch. - The only way an overdue intent leaves the journal while
create()still fails is for admission's own marker write to throw after the delete. That is the at-most-once order round 1 recommended, not a gap. - Before admission, the only journal writes come from restored future timers (R2-2a).
Against current main
- The merge is clean.
create()keeps the same restore, try, admit, and catch structure.- Main's new
resume()checks all run before admission: the surgery refusal, waiting for the maintenance pass, the stop-in-progress refusal, andResumeBlockedError. A refused or blocked resume therefore leaves the intent durable, and a laterresume({ force: true })admits it.
Resume ordering
- Admission at
resume()runs while the framework is still quiesced, so the marker goes through the persisted deferred-write queue and lands in the flush. - The request is counted in
releasedRequestsand released.
Idempotence, bounds, quarantine, receipts
- These read correctly in code. Their test gaps are in R2-4.
Graceful shutdown and the clock-step re-arm
dispose()clears handles and keeps the journal.wakeFromSleepre-arms on an early callback.- Both are unchanged since round 1 and still tested.
Changelog
- The duplicate "overdue" is gone, and the entry says at-most-once.
Test runs
- Focused suite: 26/26 at this head and on the merge with main.
- Full suite: 2241/2241 at this head (169 suites) and 2374/2374 on the merge.
- No CI checks are reported on the PR head.
Method. Two independent passes, merged after each claim was re-verified.
Sol pass. Codex gpt-5.6-sol (xhigh, read-only, no network) read the diff, the round-2 delta and the merge with main; it ran nothing. 563 s wall time, 66 shell commands, 2.50 M input tokens (2.35 M cached), 29.6 k output tokens (21.7 k of them reasoning).
Claude pass. Claude (Opus 5.5) built this head and the merge with main and ran both full suites. It ran probes against the built dist/:
· a quiesced boot, then resume();
· deadlines falling due during a quiesced boot and during a runtime quiesce, each followed by a restart;
· a restored timer firing during a failing create();
· the admission/timer race with an injected clock;
· skip_reply through the framework with a mock membrane, inspecting the stored assistant turn.
It also ran seven fix-reversion mutations, and read the PR thread and the round-1 review.
Who found what. Both: R2-1 (Sol from the quiesce bypass of restored timers; Claude by probe, which also showed the runtime-quiesce path), R2-2a, and the vacuous R1-4 test in R2-4. Sol only, then confirmed by Claude: R2-2b (probe), R2-5, R2-6. Claude only: R2-3, the mutation table in R2-4 (resume(), R1-6), R2-7.
Rejected or downgraded from Sol.
· "Admission deletes the intent before a fallible marker write": rejected as a defect, since this is the at-most-once order and restore makes no writes.
· "resume() admits before the durable host-mode flag is cleared": folded into R2-1 as an optional reorder. It is the same crash-after-admission window the create() path has.
· "No disposed state, so recovery can re-arm after dispose": rejected, because no caller restores after dispose().
· The one-shot flag stranding a failed admission: folded into R2-1's fix.
· Delete-failure duplicates: downgraded from major to minor (R2-5), since they need a storage failure followed by a restart.
· Sol's verified-ok that the shared input object "correctly transports" the receipt: corrected to R2-3.
· Sol's claim that the R1-6 test fails the old validator: true against 7e2376e as a whole, but not at this head.
Limits. Sol could not run code or read the PR thread. The probes stub MCPL startup, the membrane and the channel registry; no real MCPL server was started.
|
R2 at ba3e182, one commit per finding, with what is and isn't on the branch:
|
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R3 at ba3e182 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: CHANGES_REQUESTED (the formal review event is left to the maintainer). Finding ids R3-n are stable; cite them in fix commits.
Reviewed at ba3e182 against main e6a0317 (merge-base 92125b0). Main is 99 commits ahead, so I also built and tested a local merge of this head into main. The merge is clean, and every probe below gives the same result on the PR head and on the merge.
Nightly dual review, round R3. Reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5. Their results were merged, and the Claude leg re-verified every claim. Finding ids R3-n are stable; please cite them in fix commits.
Verdict: changes requested.
- Most of round 2 is fixed. Restore now only loads state, and admission arms or admits. Because of that, a failed
create()leaves the journal untouched and no timer running. Theskip_replyreceipt no longer touches the stored call. Self-wakes that fall due while quiesced now survive a restart. - R2-1 is still open for sleep.
wakeFromSleepdid not get the quiesce guard the two self-wake callbacks got. A sleep that expires during a runtime quiesce is deleted from the journal and lost on a quiesced restart (R3-1). - A new window of the same kind:
stop()closes the queue long before it disposes the gate. Any sleep or self-wake that falls due in between consumes its journal entry into a dead queue, so a graceful restart loses it (R3-2). resume()can double-fire a self-wake whose live timer is due but hasn't run yet (R3-3).- The tests are not mutation-proof yet: 11 of 16 fix reversions leave the suite green, and 10 of those should not (R3-4).
The two questions put to this round:
- Is each R2 finding resolved? R2-2, R2-3, R2-5 and R2-6 are fixed. R2-1, R2-4 and R2-7 are partial.
- Can a sequence still lose or duplicate a wake? Yes.
- Lost: a sleep that expires during quiesce, followed by a restart before
resume()(R3-1). Any wake that falls due insidestop()(R3-2). - Duplicated: a live self-wake that is due when
resume()admits it (R3-3). - Not reproduced: I found no loss or duplicate through a failed
create(); a refused, blocked or forcedresume(); repeated quiesce/resume cycles for self-wakes; or a quiesced boot with future intents. - The remaining loss and duplicate cases need a failed journal write or a crash between the journal delete and the marker landing. The changelog now documents both under "at most once while journal writes succeed".
- Lost: a sleep that expires during quiesce, followed by a restart before
Round 2 dispositions
| Id | Disposition at ba3e182 | Evidence |
|---|---|---|
| R2-1 | Partial | Self-wake is fixed: · both self-wake callbacks return early while quiesced and keep the intent ( event-gate.ts:1790, :2052);· the one-shot flag is gone; · resume() admits after persistHostMode(null) (framework.ts:4650-4651).Probe: a live self-wake falls due during a runtime quiesce, then stop(), a quiesced boot and resume(). The journal keeps the intent throughout, and resume admits it exactly once. Sleep is not fixed: wakeFromSleep (:1861-1888) has no guard (R3-1). None of the self-wake fixes is pinned by a test (R3-4). |
| R2-2 | Fixed | Restore only loads state (:1966-1982). Admission arms future timers (:1987-1990), and create() admits last (framework.ts:2557).· (a): the failed-create window is gone by construction, since nothing is armed before admission. · (b): fireRestoredSelfWake now checks identity (:2053).The same race survives for ordinary (non-restored) timers, which resume() can now reach (R3-3). |
| R2-3 | Fixed | enrichedCall.input = { ...inputObj, _wake_durable: durable } (framework.ts:14996). dispatchChannelToolCall receives enrichedCall, so the receipt arrives and the yielded call is left alone. No test pins it (R3-4). |
| R2-5 | Fixed as scoped | The changelog and the journal doc comment are qualified (changelog.d/…:2, event-gate.ts:549-550). clearSleep returns deletion durability (:1841), and wake returns it (framework.ts:18337-18338). No test pins it. See also R3-6. |
| R2-6 | Fixed | armedAt is read once and wakeAt derived from it (:1779-1783). |
| R2-4 | Partial | Two tests were added, and the invalid-timestamp test now asserts that nothing is emitted (test:362-364). That does pin the wakeAt >= armedAt check, although the author's comment lists it as not yet done. The R1-4 and bounds tests are unchanged and still vacuous. Several round-3 fixes are unpinned (R3-4). |
| R2-7 | Partial | Fixed: the recovery marker (:2022) and the resume() comment. Still stale: the quiesced-boot banner (framework.ts:2449-2450). This round's refactor left new stale comments (R3-5). |
Round 1, for the record.
- R1-1: still fixed.
- R1-2: partial, via R3-1.
- R1-3: fixed, and now by construction: restore arms nothing, so the
dispose()in the catch has nothing to clear. Removing it no longer fails a test, and that is fine. - R1-4: fixed in code; the test is still vacuous.
- R1-5: fixed.
- R1-6: fixed in code; the order check is now pinned, the bounds are not.
- R1-7: fixed, but the prose has drifted again (R3-5).
Major
R3-1. R2-1 remains open for sleep. A sleep that expires while the host is quiesced is still deleted from the journal and held only in memory. (Found by both reviewers.)
What the code does:
- Both self-wake callbacks now return early while quiesced (
event-gate.ts:1790,:2052). The sleep timer's callback has no such check. wakeFromSleep(:1861-1888) clearssleepUntil, persists the deletion (:1876), and requests inference (:1887). The framework parks that request inpendingRequests.- A runtime quiesce leaves a live sleep timer running, because
setQuiesceddoesn't touch it (:1671-1675). A quiesced boot is safe, because restore no longer arms anything.
The author's comment says "a sleep or self-wake callback that fires while quiesced returns". In the code, only the two self-wake sites return.
Probe (real AgentFramework.create, mock membrane):
- A serving host sets
setSleep(1, 'check result', 'agent'), thenquiesce(). - After 1.4 s the journal is
{"version":1,"selfWakes":[]}andpendingRequestsis[agent:sleep]. stop(), a quiesced boot, thenresume(). Nothing is pending, there is no[wake-recovery]marker, and the sleep leaves no trace.
The same run with armSelfWake keeps the intent in the journal, and resume() admits exactly one wake-recovery.
Fix:
- In
wakeFromSleep, after the early-callback re-arm check (:1866-1869), addif (this.quiesced) return;. The handle is already null, and sleep state and the journal stay as they are. - Suppression still ends on time, because it tests
sleepUntil > now. admitOverdueWakeIntentsalready treats0 < sleepUntil <= nowas due (:1992).- Test: run the probe above through the real
quiesce(),stop(),create()andresume(), for both sleep and a live self-wake.
R3-2. A wake that falls due while stop() is running is consumed into the closed queue, so a graceful restart loses it. (Claude only.)
What the code does:
stopOwnedcloses the queue atframework.ts:2710.- It then awaits active streams (
:2754), any in-flight maintenance pass (:2768), the event loop (:2772) and, on main, operator operations up toteardownTimeoutMs. - Only after all of that does it call
eventGate.dispose()(:2792). - The gate's timers keep running for that whole span. When one fires, it deletes its journal entry (
:1792,:1876,:2055) and pushes a request that never runs.
Probe, at the PR head and on the merge:
- Arm a 1 s sleep, or a 1 s self-wake.
- Hold
stop()open for 1.5 s with an in-flight maintenance pass. - After
stop()returns, the journal is{"version":1,"selfWakes":[]}and the request sits on the dead queue ([agent:sleep]or[agent:self-wake]). - A fresh
create()from the same store has nothing pending. For the self-wake, the[self-wake]notice has landed in context, but no turn follows it; the sleep leaves nothing.
How likely this is:
- A quick stop has a short window. Any restart that cancels a turn, waits on a maintenance pass, or waits on an operator operation has a longer one.
- The chance per restart is roughly the window divided by the wake interval. For example, a one-second stop and a 30 s
skip_replycadence lose a wake on about 3 % of restarts. - The description says "Graceful shutdown preserves intent", and the changelog says "including across graceful restarts". This is the path where those claims don't hold.
Fix:
- Release the gate's wake timers at the top of
stopOwned, beforequeue.close(). That means moving the timer half ofdispose()earlier (clearselfWakeTimersandsleepTimer, leave the intents alone), or quiescing the gate there once R3-1's guard is in. - Separately, have
runResumeskip admission once stop has begun (if (!this.stopHasBegun()) …before:4651).- Today that is latent.
flushDeferredWritesnever yields, so a stop can't land between the refusal check (:4611) and admission. - A probe that slows the flush shows the effect. Admission runs into the closed queue after
persistHostMode(null), so the next boot is serving with the intent gone.
- Today that is latent.
- Test: hold
stop()open past a 1 s deadline for both kinds. Assert that the journal still holds them, and that a freshcreate()recovers each exactly once.
Minor
R3-3. resume() can admit a due self-wake while its live timer is still armed, and the timer then fires a second wake. (Both: Sol from reading, Claude by probe.)
What the code does:
admitOverdueWakeIntentsdeletes a due self-wake (:2008) but leaves any handle inselfWakeTimers.- The ordinary
armSelfWakecallback (:1788-1819) deletes and emits without the identity check thatfireRestoredSelfWakenow has (:2053). - Before this round, admission ran once per process, before any agent could arm an ordinary timer. It now runs on every
resume(), so the race is reachable.
Probe:
quiesce(), thenarmSelfWake('agent', 1).- Keep the event loop busy until the deadline has passed, then call
resume()in the same tick. - Result:
pendingRequestsis[agent:wake-recovery, agent:self-wake], and both markers are in context.
The real window is event-loop lag at the moment of resume(), or a forward wall-clock step while quiesced (Date.now() against a monotonic timer). It is narrow, but nothing closes it.
A related gap (Sol): onInferenceStarted cancels an intent only when it has a handle (:1648-1654).
- Today an intent has no handle in three cases: after a quiesced fire, on a quiesced boot, and after a failed admission write (
:2013). - In the first two, no turn can start before
resume()admits the intent. - After a failed write, the intent stays in memory and in the journal with no timer. The next turn doesn't cancel it, so the next restart delivers a wake the agent had already superseded.
Fix:
- When admission consumes a self-wake, clear and delete that agent's handle.
- Give the
armSelfWakecallback the same identity check: captureintent, and return ifthis.selfWakeIntents.get(agentName) !== intent. - Let
onInferenceStarteddrop the intent whether or not it has a handle.
R3-4. R2-4 partial: 11 of 16 fix reversions still pass, and 10 of those should fail.
Method: revert one fix in the built dist/, then rerun event-gate-selfwake, host-quiesce and stop-with-host-operations (116 tests).
| Reverted | Tests failing |
|---|---|
quiesce guard in fireRestoredSelfWake (R2-1) |
1 |
| restore arms future timers again (R2-2a) | 1 |
create() admits even when quiesced (R1-2) |
1 |
setSleep/armSelfWake always report durable: true (R1-5) |
1 |
no wakeAt >= armedAt check (R1-6) |
1 |
quiesce guard in the ordinary armSelfWake callback (R2-1) |
0 |
| one-shot admission flag reinstated (R2-1) | 0 |
resume() never admits (R2-1) |
0 |
| admission never arms a future sleep (R2-2) | 0 |
no identity check in fireRestoredSelfWake (R2-2b) |
0 |
| receipt written back into the shared input (R2-3) | 0 |
clearSleep/wake always report durable: true (R2-5) |
0 |
no has() guard and no clearTimeout(previous) (R1-4) |
0 |
| no restore-once guard (R1-4) | 0 |
| no timestamp bounds (R1-6) | 0 |
no dispose() in the create catch (R1-3) |
0 (expected; see R1-3) |
Notes on the zero rows:
- New quiesce test (
test:400-424):- It drives gate methods directly. Its first gate calls
admitOverdueWakeIntents()while quiesced, which the framework never does. - It only exercises the restored-timer path. The ordinary timer, sleep, the one-shot flag and
AgentFramework.resume()are all unexercised. - One framework-level test (R3-1's) pins the first four rows at once.
- It drives gate methods directly. Its first gate calls
- "Re-arms it after restart" tests (
:182,:221): both check onlygetSleepState().until. Deleting the sleep arming from admission leaves a restored sleep that never wakes, and both still pass. Assert thatsleepTimeris set, or advance past the deadline and count requests. - R1-4 (
:368,:384): both still assert onlyselfWakeTimers.size === 1, which an overwritten handle also satisfies. Count fired requests, or spy onreadWakeState.- Without the restore-once guard, a second restore replaces the intent object. The first timer's identity check then fails, so the wake is silently dropped.
- Bounds: the out-of-range value is still in the sleep slot, where the order check rejects it first. Use an ordered out-of-range self-wake such as
armedAt: -2e20, wakeAt: -1e20. Without the bounds,toISOStringthrows. - Receipts: nothing covers the framework's
sleep/skip_reply/wakeresults or the stored assistanttool_useinput.
R3-5 (nit). Prose that is stale or now false.
- Quiesced-boot banner (
framework.ts:2448-2450): it says parked wakes "are gone". The author has already acknowledged this one. restoreWakeIntentsdoc (event-gate.ts:1965) and thecreate()comment (framework.ts:2463-2464): both say restore restores "future timers". Restore arms nothing now; admission does.clearSleepdoc (:1832): it says "Returns true if the agent was asleep", but the method returns{ woke, durable }.- PR description:
- "restores suppression and future timers": stale.
- "another maintenance restart cannot lose an in-memory parked wake": false for sleep (R3-1).
- "Graceful shutdown preserves intent": see R3-2.
- The at-most-once sentence is unqualified, and the receipts are still the ones from
3b87a3f.
- Changelog: "including across graceful restarts" overclaims until R3-2 is fixed.
R3-6 (nit). EventGate.clearSleep() changed its return type from boolean to an object.
EventGateis exported from the package root (src/index.ts:81).- For an embedder,
if (gate.clearSleep())is now always true. - A GitHub code search found no callers outside this repo.
- Fix: either add a
changedchangelog fragment, or keep the boolean method and expose durability another way.
Verified fine
Nomination facts
- R2-1 to R2-5 are described accurately.
- The list omits R2-6 and R2-7, two nits that were also in round 2. Both are dispositioned above.
Failed create()
restoreWakeIntents()performs no writes and arms no timers.- Every fallible step before admission is inside the
try. - After a rejection, the journal is untouched by construction.
resume() ordering on main
- The surgery refusal, the maintenance wait, the preview loop, the stopping check and
ResumeBlockedErrorall run before the gate is un-quiesced. - So a refused or blocked resume leaves intents durable, and a forced resume admits them.
- From
setQuiesced(false)through admission there is no macrotask boundary, so no turn can start in between.
Repeated quiesce/resume (self-wake)
- Admission arms only intents without a handle (
:1987-1990). - A restored timer that fires while quiesced keeps its intent, and the next
resume()admits it.
R2-3 transport
- membrane 0.5.87 still shares
block.inputbetween the yielded call androundContent. - Reassigning
enrichedCall.inputis therefore enough.
Test runs
- Focused suite: 28/28 at the head and on the merge.
- Full suite: 2233/2233 at the head (168 suites) and 2399/2399 on the merge.
- No CI checks are reported on the PR head.
Method. Two independent passes, merged after each claim was re-verified.
Sol pass. Codex gpt-5.6-sol (xhigh, read-only, no network) read the diff, the round-3 delta, the merge with main, both earlier reviews and the author's replies. It ran nothing. About 510 s wall time, 45 shell commands, 2.54 M input tokens (2.35 M cached), 36.7 k output tokens (28.0 k of them reasoning).
Claude pass. Claude (Opus 5.5) built this head and the merge with main and ran both full suites. It ran probes against the built dist/ with real AgentFramework.create, a mock membrane and no MCPL servers:
· a runtime quiesce with sleep and with self-wake, then a quiesced restart and resume();
· stop() held open past a deadline by an in-flight maintenance pass;
· resume() in the same tick as a due live timer;
· stop() during a slowed resume flush.
It also ran a 16-row fix-reversion mutation table, and used a GitHub code search for callers of clearSleep.
Who found what. Both: R3-1, R3-3 (the duplicate), R3-4 (Sol by reading; Claude by running the mutations, which added the sleep-arming row), R3-5. Sol only, confirmed by Claude: the handleless-intent cancellation gap in R3-3, and the stale restoreWakeIntents and clearSleep doc comments. Claude only: R3-2, the latent resume-during-stop admission, R3-6.
Rejected or adjusted from Sol.
· The handleless-intent half of R3-3 is limited to the failed-write path, because no turn can start between setQuiesced(false) and admission.
· Sol's verified-ok "graceful disposal … preserves journal state" holds for dispose() itself but misses the window before stop() calls it (R3-2).
· Sol's "stop refusal checks precede admission" holds, but doesn't cover a stop that begins after the check (R3-2, latent).
Limits. The probes stub the slow parts of stop() and resume(), and no real MCPL server or provider was started. The R3-3 window was forced by blocking the event loop.
…esearch#389) create() opened the Chronicle store first, and went on to start modules, the liveness watchdog and the event gate and to connect MCPL servers. Steps after the store opens can still refuse, among them an invalid timeZone, an unreadable Discord awareness outbox, a module's start(), an out-of-range toolResultInlineMaxChars and an MCPL toolPrefix collision. Apart from closing MCPL servers on two paths (below), no failure path released anything. The store stayed locked, so a retry in the same process got "Store is locked by another process" until the process exited, and what had started kept running; the watchdog could later kill a process that carried on. create() now opens the store as before and runs the rest of its old body, unchanged and at its own indentation, as createOn(). The framework is named in `building` as soon as it is constructed. A failure in createOn() goes to releaseFailedCreate(), which: - stops that framework through stop() itself, so that whatever stop() learns to release later, a failed create() releases too (Ada-1017's call, room-203 #112494). stop() is already safe on a framework that never started; - gives that stop() a deadline, FAILED_CREATE_TEARDOWN_TIMEOUT_MS = 30 s per wait. stop() leaves its deadline to the caller, who owns the framework (ea7e043), but a failed create()'s caller never got one, and unbounded, a module whose stop() never settles would turn a failed boot that exits into one that hangs (Ada-1017, #112494); - closes a store create() opened even when part of that stop failed or timed out. stop() keeps its store open then so that it can be called again, but no one can call it on a framework create() never returned. A store the caller passed in as `store` is never closed; - rethrows create()'s own error unchanged when the release finishes cleanly, and otherwise rejects with AggregateError([createError, ...releaseErrors]), "AgentFramework.create() failed: <create's message>. Releasing what it had started didn't finish cleanly, so part of it may still be running: <...>". Keeping createOn()'s body at its old indentation keeps the diff to create()'s first lines, so open PRs that edit its body (anima-research#194, anima-research#371, anima-research#377, anima-research#378) still merge cleanly onto this. Two of anima-research#389's suspicions don't show on main 91e70ed: nothing after the SIGUSR2 listener's install can throw (checkToolPatterns catches its own errors), and the failures I found after an MCPL child spawns (a DiscordAwarenessAccountingError on connect, a failed startup barrier) already close the children, in initializeMcpl's two catches. Should a later step fail, stop() closes them. stop() itself never removes that listener; that is F47. test/failed-create-releases.test.ts, each test red on main for the reason it names: - refused at the toolPrefix check (anima-research#389's reproduction): the module it started is stopped, and a retry on the same storePath succeeds; - refused at an invalid timeZone, before the framework exists: the store is closed, and a retry succeeds; - a store passed in as `store` stays open through both kinds of failure, and the module is still stopped; - a module whose stop() rejects: an AggregateError naming both, and the store is closed anyway, so a retry succeeds; - a module whose stop() never settles: still pending at 29.999 s (mocked), rejected at 30 s with the double-fault message naming the timeout; - in a child process, a create() failing after the watchdog started leaves no watchdog: a 5 s busy stretch past its 2 s threshold isn't killed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| this.sleepArmedAt = 0; | ||
| this.sleepNote = undefined; | ||
| this.sleepAgent = undefined; | ||
| this.tryPersistWakeState('expiring sleep'); |
There was a problem hiding this comment.
When a sleep expires during quiesce, wakeFromSleep() clears the saved intent without checking this.quiesced. The framework parks its inference request only in memory, so a restart before resume() loses the promised wake. Keep the sleep intent until resume can admit it, as the self-wake callbacks already do.
Knowledge Base Used: Persistent state and recovery
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gate/event-gate.ts
Line: 1876
Comment:
**Maintenance loses sleep wakes**
When a sleep expires during quiesce, `wakeFromSleep()` clears the saved intent without checking `this.quiesced`. The framework parks its inference request only in memory, so a restart before `resume()` loses the promised wake. Keep the sleep intent until resume can admit it, as the self-wake callbacks already do.
**Knowledge Base Used:** [Persistent state and recovery](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/persistent-state-and-recovery.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| this.selfWakeIntents.delete(agentName); | ||
| this.tryPersistWakeState('firing self-wake'); |
There was a problem hiding this comment.
stopOwned() stops the inference loop before waiting for streams, maintenance, and operator work. It disposes the gate afterward. If a timer expires during those waits, it removes its saved intent and queues work on the stopped framework. The wake then disappears on restart. Freeze the gate timers when shutdown begins, before those waits.
Knowledge Base Used: Persistent state and recovery
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gate/event-gate.ts
Line: 1791-1792
Comment:
**Shutdown can erase wakes**
`stopOwned()` stops the inference loop before waiting for streams, maintenance, and operator work. It disposes the gate afterward. If a timer expires during those waits, it removes its saved intent and queues work on the stopped framework. The wake then disappears on restart. Freeze the gate timers when shutdown begins, before those waits.
**Knowledge Base Used:** [Persistent state and recovery](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/persistent-state-and-recovery.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // flushDeferredWrites); nothing here may reach the plane reopen below. | ||
| await this.flushDeferredWrites('resume'); | ||
| this.persistHostMode(null); | ||
| this.eventGate?.admitOverdueWakeIntents(); |
There was a problem hiding this comment.
Failed resume leaves wakes unarmed
On a quiesced boot, restored wakes have no timers until this call. If persistHostMode(null) throws, such as when its directory becomes unwritable, wake admission is skipped. The finally block still reopens the host with quiesced already false, so another resume() returns immediately. Saved future wakes remain unarmed even after storage is repaired. Ensure admission runs whenever resume leaves the host serving, or keep resume retryable after this failure.
Knowledge Base Used: Persistent state and recovery
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/framework.ts
Line: 4651
Comment:
**Failed resume leaves wakes unarmed**
On a quiesced boot, restored wakes have no timers until this call. If `persistHostMode(null)` throws, such as when its directory becomes unwritable, wake admission is skipped. The `finally` block still reopens the host with `quiesced` already false, so another `resume()` returns immediately. Saved future wakes remain unarmed even after storage is repaired. Ensure admission runs whenever resume leaves the host serving, or keep resume retryable after this failure.
**Knowledge Base Used:** [Persistent state and recovery](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/persistent-state-and-recovery.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // Only process-local timer handles are released; startup follows the same | ||
| // future/re-arm or overdue/reconcile path as an abrupt process loss. | ||
| this.selfWakeTimers.clear(); | ||
| if (this.sleepTimer) { clearTimeout(this.sleepTimer); this.sleepTimer = null; } |
There was a problem hiding this comment.
Clearing sleepTimer here breaks a supported stop/start cycle on the same framework instance. With a caller-owned store, start() can run again, but it neither recreates the gate nor admits its retained intents. The sleep deadline remains set while its wake timer is gone, so the promised wake never runs. Re-arm retained wakes when the framework starts a new lifecycle.
Knowledge Base Used: Agent runtime orchestration
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gate/event-gate.ts
Line: 2228
Comment:
**Restart drops the sleep timer**
Clearing `sleepTimer` here breaks a supported stop/start cycle on the same framework instance. With a caller-owned store, `start()` can run again, but it neither recreates the gate nor admits its retained intents. The sleep deadline remains set while its wake timer is gone, so the promised wake never runs. Re-arm retained wakes when the framework starts a new lifecycle.
**Knowledge Base Used:** [Agent runtime orchestration](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/agent-runtime.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| } else this.selfWakeIntents.set(item.intent.agentName!, item.intent); | ||
| continue; | ||
| } | ||
| const targets = item.intent.agentName ? [item.intent.agentName] : this.getAgentNamesFn(); |
There was a problem hiding this comment.
A saved wake can name a conversation fork that startup never recreates. Forks inherit the template's tools, but only their generation counters survive restart; the next message creates a new generation. Recovery removes the old fork's intent, then both its notice and inference request are dropped because that agent is absent. Restore the saved target or explicitly account for the undeliverable wake before consuming its intent.
Knowledge Base Used: MCPL conversation and inference routing
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gate/event-gate.ts
Line: 2016
Comment:
**Fork wakes lose their target**
A saved wake can name a conversation fork that startup never recreates. Forks inherit the template's tools, but only their generation counters survive restart; the next message creates a new generation. Recovery removes the old fork's intent, then both its notice and inference request are dropped because that agent is absent. Restore the saved target or explicitly account for the undeliverable wake before consuming its intent.
**Knowledge Base Used:** [MCPL conversation and inference routing](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/mcpl-conversation-and-inference.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| for (const item of due) { | ||
| if (item.kind === 'sleep') { | ||
| this.sleepUntil = 0; this.sleepArmedAt = 0; this.sleepAgent = undefined; this.sleepNote = undefined; | ||
| } else this.selfWakeIntents.delete(item.intent.agentName!); |
There was a problem hiding this comment.
If the wall clock moves past wakeAt while a live self-wake timer still has time left, resume() admits the overdue intent but leaves its timer armed. The callback created by armSelfWake() does not check whether that intent was already consumed, so it can send another notice and inference request. Cancel the existing timer when admission consumes the intent, and check that the intent is still current before firing.
Knowledge Base Used: Event gate pipeline
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gate/event-gate.ts
Line: 2008
Comment:
**One self-wake can fire twice**
If the wall clock moves past `wakeAt` while a live self-wake timer still has time left, `resume()` admits the overdue intent but leaves its timer armed. The callback created by `armSelfWake()` does not check whether that intent was already consumed, so it can send another notice and inference request. Cancel the existing timer when admission consumes the intent, and check that the intent is still current before firing.
**Knowledge Base Used:** [Event gate pipeline](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/event-gates.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (selfWake) { | ||
| clearTimeout(selfWake); | ||
| this.selfWakeTimers.delete(agentName); | ||
| this.selfWakeIntents.delete(agentName); |
There was a problem hiding this comment.
Old fallback survives another turn
After an overdue self-wake fails journal admission, its intent remains in selfWakeIntents without a timer. A later heartbeat can supersede that fallback, but onInferenceStarted() only removes the intent inside if (selfWake). Even if storage has recovered, cancellation is never attempted, so a later restart revives the obsolete wake. Cancel pending intents independently of whether a timer handle exists.
Knowledge Base Used: Event gate pipeline
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gate/event-gate.ts
Line: 1652
Comment:
**Old fallback survives another turn**
After an overdue self-wake fails journal admission, its intent remains in `selfWakeIntents` without a timer. A later heartbeat can supersede that fallback, but `onInferenceStarted()` only removes the intent inside `if (selfWake)`. Even if storage has recovered, cancellation is never attempted, so a later restart revives the obsolete wake. Cancel pending intents independently of whether a timer handle exists.
**Knowledge Base Used:** [Event gate pipeline](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/event-gates.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
|
||
| /** End sleep immediately. Returns true if the agent was asleep. */ | ||
| clearSleep(): boolean { | ||
| clearSleep(): { woke: boolean; durable: boolean } { |
There was a problem hiding this comment.
clearSleep() breaks existing callers
The exported EventGate.clearSleep() changes from a boolean to an object. Existing callers using if (gate.clearSleep()) now always take that branch, including when woke is false. Callers expecting a boolean also stop type-checking. The framework caller is updated, but embedding callers are not protected. Preserve the boolean method and expose the detailed receipt separately.
Knowledge Base Used: Agent runtime orchestration
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gate/event-gate.ts
Line: 1833
Comment:
**`clearSleep()` breaks existing callers**
The exported `EventGate.clearSleep()` changes from a boolean to an object. Existing callers using `if (gate.clearSleep())` now always take that branch, including when `woke` is false. Callers expecting a boolean also stop type-checking. The framework caller is updated, but embedding callers are not protected. Preserve the boolean method and expose the detailed receipt separately.
**Knowledge Base Used:** [Agent runtime orchestration](https://app.greptile.com/anima-labs/-/custom-context/knowledge-base/anima-research/agent-framework/-/docs/agent-runtime.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.ba3e182 to
7168daa
Compare
|
R3 is answered at
Full suite at |
Two changes to the same lines: main's anima-research#634 (a sleep's wake ends at dispose() and is armed again at start(); `disposed`, armSleepWake(), resumeSleep(), agentGone()) and this branch's durable wake-intent journal. Both are kept: - armSleepWake() replaces this branch's armSleepTimer(): same chunking, by MAX_TIMER_MS, and nothing is armed while the gate is disposed. armRestoredSelfWake() follows the same rule. - wakeFromSleep() keeps main's re-arm of an early step and this branch's quiesce guard: a deadline reached while quiesced keeps its intent for admitOverdueWakeIntents() at resume. - dispose() sets `disposed` (main) and keeps the durable intents, in memory and in the journal (this branch). resumeSleep() arms the sleep as main does, and arms the kept self-wake intents again the same way; a self-wake asked for while disposed arms nothing and journals nothing. - agentGone() also drops the gone agent's journal entry, so a restart wakes no name that no longer exists. - armSelfWake() returns { inMs, armed, durable }; the framework writes `_wake_durable` into the call only when a wake was armed, and logs main's "not armed: the framework has stopped" line otherwise. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkepYxcPem6pmgRxKd2RBY
|
Pushed Suite on |
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R4 at e91cfb6 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: CHANGES_REQUESTED (the formal review event is left to the maintainer). Finding ids R4-n are stable; cite them in fix commits.
Reviewed at e91cfb6 against main 66c8079 (merge-base 5834c2c)
Nightly dual review, round R4. Reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5. Their results were merged, and the Claude leg re-verified every claim. This post comes from the maintainer's campaign runner; the formal review event is left to the maintainer. Finding ids R4-n are stable; please cite them in fix commits.
Main has moved 48 merges past this branch's merge base. The PR no longer merges: src/gate/event-gate.ts conflicts (R4-2). Everything below was run on the PR head. The head is green on CI (12 check runs) and on a local full suite.
Verdict: changes requested.
- The round-3 wake losses are fixed. Run through the real
quiesce(),stop(),create()andresume():- A sleep that expires during a runtime quiesce survives a quiesced restart, and
resume()admits it exactly once. - A wake that falls due inside
stop()stays in the journal, and the nextcreate()recovers it once. - The
resume()double fire is gone.
- A sleep that expires during a runtime quiesce survives a quiesced restart, and
- The R3-2 fix moved too much. It moved the whole of
EventGate.dispose()to the top ofstop(), where R3 asked for only the wake-timer half. That regresses the gate's debounce path, which this PR doesn't claim to touch. An MCPL event that arrives during a stop now writes a gate line into context mid-teardown. Its debounce timer outlivesstop()and then throwsStore has been closedas an uncaught exception. Main does neither (R4-1). - Two other items remain:
- The conflict with main is small as text, but a textual resolution leaves the restored and recovery wake paths outside main's #723 rule (R4-2).
- Five minors: a resume/stop window that still consumes wakes, two failure-path gaps, a receipt that says
durable: truefor a wake nothing armed, and tests where 11 fix reversions still pass.
The two questions put to this round:
- Are R3-1 to R3-4 fixed and pinned?
- R3-1: fixed and pinned.
- R3-2: partial. The stop window is fixed and pinned, but the fix causes R4-1, and the resume-during-stop half is open (R4-3).
- R3-3: partial. The duplicate is fixed and pinned; the cancellation gap remains.
- R3-4: partial. See R4-7.
- The nomination listed four findings. Round 3 also had two nits, R3-5 and R3-6, both still open and dispositioned below.
- What else is wrong, including what the PR doesn't claim to touch?
- R4-1: debounce, inference buffering and the gate script during
stop(). - R4-2: merging with main.
- R4-4: the merge's new
resumeSleep()re-arm undoes the "no wake when the admission write fails" rule atstart().
- R4-1: debounce, inference buffering and the gate script during
Round 3 dispositions
| Id | At e91cfb6 | Evidence |
|---|---|---|
| R3-1 | Fixed, pinned | wakeFromSleep returns while quiesced and keeps the intent (event-gate.ts:1952-1955).Framework probe, for both sleep and self-wake: a runtime quiesce, stop(), a quiesced boot, start(), then resume(). The journal keeps the intent throughout, and resume gives one [wake-recovery] and one released wake.Reverting the guard fails test:440. That test still drives gate methods directly, including admitOverdueWakeIntents() while quiesced, which the framework never does. |
| R3-2 | Partial | The stop window is fixed: dispose() runs before queue.close() (framework.ts:3974-3978).Probe: a 1 s sleep or self-wake, with stop() held open 1.5 s by a maintenance pass. The journal keeps it, and a fresh create() recovers it once.The order test ( test:591) catches a reversion.But the whole dispose() moved (R4-1), and the resume-during-stop half is still open. R3 called that half latent; it is reachable without slowing anything (R4-3). |
| R3-3 | Partial | The duplicate is fixed: admission clears the live handle (event-gate.ts:2095-2097). test:467 catches a reversion.Not done (1): the identity check in the ordinary armSelfWake callback (:1876-1880). In Node a cleared timer never runs, and every replacement of an intent clears its old handle first, so this is defence in depth, not a live bug.Not done (2): onInferenceStarted still cancels only intents that have a handle (:1667-1673). An intent left without a handle by a failed admission write (:2100-2106) survives the turns that supersede it. See R4-4 and R4-5. |
| R3-4 | Partial | 23-row fix-reversion table: 10 caught, 13 green, 11 of which should fail (R4-7). |
| R3-5 | Partial | Fixed, and now pinned by test:656-657: the quiesced-boot banner.Still stale: the restoreWakeIntents doc (:2051), the create() comment (framework.ts:3545-3546), the clearSleep doc (:1920), and the PR description.New this round: the dispose() comment (R4-8). |
| R3-6 | Not fixed | clearSleep() still returns an object, and there is no changed fragment (R4-9). |
Greptile's round-3 items, for the record.
- Restart drops the sleep timer: fixed by the merge.
resumeSleep()re-arms sleep and kept self-wakes. Probed for both. - Fork wakes lose their target:
- After a crash, the intent is consumed and the notice and request are dropped with a log line. Nothing crashes, and the journal is cleaned up.
- On a graceful stop,
agentGonedeletes it. - Both match the framework's rule that forks end at a restart. The receipt's
durable: truefor a fork does not (R4-6).
- Failed resume leaves wakes unarmed: R4-5.
- Old fallback survives: R3-3 above.
clearSleep()breaks callers: R4-9.
Major
R4-1. The R3-2 fix moved all of EventGate.dispose() to the top of stop(). Gate events that arrive during the stop now reach a half-stopped framework, and their debounce timer outlives stop() and throws. (Found by both reviewers; Claude probed it.)
What changed:
stopOwnedused to calleventGate.dispose()near the end, after the waits on streams, maintenance, the event loop and operator operations. It is still there on main (main:src/framework.ts:4557).- This head calls it at
framework.ts:3977, before any of those waits. dispose()does more than release wake timers (event-gate.ts:2309-2344):- it stops the gate script;
- it clears the debounce timers, the inference buffer and the rate-limit state;
- it empties
inferring.
- Its own comment still says "stop() disposes the gate once its streams and event loop have ended" (
:2334). - MCPL servers stay connected until
framework.ts:4235-4237. Their push and channel-incoming events still pass through the gate's trigger filter in the meantime. handleDebounce(:1530-1561) has no disposed check, and no laterdispose()comes to clear what it arms.
Probe: real AgentFramework.create, a gate.json with one 200 ms debounce policy, and an agent marked as inferring. stop() is held open by an in-flight maintenance pass, and one mcpl:push-event is fed through gate.evaluate mid-stop, as the push handler's filter would.
| main 66c8079 | this head | |
|---|---|---|
inferring mid-stop |
['agent'] |
[] |
| the batch | buffered, dropped at dispose | [Gate: 1 event matched] written into context, agent:gate pushed onto the closed queue |
a second event just before stop() returns |
its timer is cleared by the late dispose | 1 debounce timer alive after stop() |
| 400 ms later | nothing | uncaught Error: Store has been closed, at fireDebounce → deliverEvents → addMessage → ContextManager.appendMessage |
Also:
- Mid-stop, the gate script (
gate.js) no longer decides:evaluatereturnsundefinedfrom a stopped script, so policies and the default decide. - An embedder without an
uncaughtExceptionhandler exits with an error during what should be a clean shutdown. - The window is the whole stop. It is long whenever a stop waits on a cancelled turn, a maintenance pass or an operator operation, and a busy channel with a debounce rule will land in it.
Fix:
- Do what R3 suggested and split the call:
- Before
queue.close(), call a newEventGate.suspendWakes()(name it as you like). It clearssleepTimerandselfWakeTimers, setsdisposed, and leaves the intents and everything else alone. - Keep the full
dispose()where main has it.
- Before
- Optionally, make
handleDebouncea no-op while disposed. That also closes main's much smaller version of this window, between itsdispose()and the MCPLcloseAll. - Tests:
- the probe above as a framework test: no gate line during
stop(), no timer after it, no throw; - keep
test:591, but have it assert on the split call.
- the probe above as a framework test: no gate line during
R4-2. The PR conflicts with main. The textual fix is trivial; the semantic one is not. (Sol found it; Claude confirmed with git merge-tree.)
The conflict:
git merge-tree --write-tree main e91cfb68reports two conflicting hunks inevent-gate.ts: thenode:fs/node:pathimports, and adjacent constructor options (main'schannelShownandisNewestEntrybeside this PR'sautoRecover). Everything else merges textually.
What a textual merge produces:
- Main's #723 (
debce5ab, "a write the agent heard mid-turn starts no turn of its own once it's done") gave the self-wake aWritePlacement. The notice and the wake share it, so the host drops the wake once a request carrying the notice is answered. - After the merge, only the ordinary
armSelfWakecallback carries it. - This PR's two new delivery paths call
addMessageFn/requestInferenceFnwithout it:fireRestoredSelfWake, and the[wake-recovery]marker and request inadmitOverdueWakeIntents. - So a restored or recovered wake whose notice the agent hears mid-turn still starts a redundant turn afterwards. That is the behaviour #723 removed.
Other main changes this resolution needs to respect:
- Main's late full
dispose()(R4-1). - #371 and #452 now treat "missing" as ENOENT from the read rather than
existsSync, and leave unreadable gate files in place. The journal's quarantine-by-rename is defensible for a host-owned file. Keep that difference deliberate, and consider ENOENT-from-read inreadWakeStatetoo.
Fix:
- Merge or rebase onto current main.
- Fold the duplicated fire bodies, the ordinary callback (
:1876-1907) andfireRestoredSelfWake(:2144-2158), into onefireSelfWake(intent)that carries main's placement. Then the next change to self-wake delivery reaches both paths. - Give the recovery marker and request a placement too.
Minor
R4-3. R3-2's resume half: a stop() that lands in resume()'s flush still consumes a due wake, with no artificial slowing. (Sol found it; Claude probed it.)
What the code does:
runResumechecksqueue.isClosedonce (framework.ts:6742), then awaitsflushDeferredWrites(:6788).- That function never awaits internally, but awaiting it still yields one microtask.
Probe: a quiesced boot holding an overdue sleep. Call resume(), then call stop() after N chained microtasks.
- At N=5, the order is
dispose,persistHostMode(null), thenadmitOverdueWakeIntentson the disposed gate. - The journal entry is deleted, and its request goes to the closed queue. The log says
1 parked wake(s) won't run because the framework is stopping. - The next
create()boots serving, with nothing pending: the wake is gone. - At N≤4, resume is refused and everything stays durable.
- The record half (
persistHostMode(null)after stop has begun) also happens on main, so it is filed separately. Consuming the wake is this PR's.
Fix:
- In
admitOverdueWakeIntents, return before touching anything whenthis.disposed. - In
runResume, skip both persist and admission ifstopHasBegun()after the flush. - Test: the microtask probe at a fixed depth.
R4-4. When the admission write fails at create(), start() delivers the wake anyway and leaves it journaled, so the next boot delivers it again. (Claude only.)
What the code does:
admitOverdueWakeIntentsdeclines to emit when its journal write fails (:2100-2106).test:567pins that at gate level.- In the framework, the next call is
start(), thenresumeSleep(), which since the merge does this:- fires an overdue sleep through
wakeFromSleep(:1810-1811); - arms every handleless self-wake intent (
:1812-1814). An overdue one fires at once throughfireRestoredSelfWake.
- fires an overdue sleep through
- Neither path checks that its own deletion was written.
Probe: overdue sleep or self-wake, persistWakeState throwing, then create() and start().
- After
create(): nothing pending. - At
start(): anagent:sleeporagent:self-wakerequest. - The journal still holds the intent.
- The agent gets a plain sleep wake or
[self-wake] … timer elapsednotice, not the[wake-recovery]marker. The next boot replays it.
The changelog's "at most once while journal writes succeed" still holds. But the design (and the test's name) says a failed admission write emits nothing, and in the framework that is no longer true.
Fix:
- Have
resumeSleep()route overdue intents throughadmitOverdueWakeIntents()rather than firing them directly. That path keeps the marker and the write-failure rule. - Or skip intents whose admission write failed until the next admission.
- Test at framework level: a failed write, then
create()andstart(). Nothing is emitted, and the intent is still journaled.
R4-5. If anything in resume()'s try throws, admission is skipped, and a retried resume() never admits. (Sol and Greptile; Claude read-verified it.)
What the code does:
persistHostMode(null)can throw, sincewriteRecoveryFiledoes on ENOSPC or EROFS. So canflushDeferredWrites. Either throw skipsadmitOverdueWakeIntents()(framework.ts:6788-6792).- The
finallystill reopens the MCPL planes withquiescedalready false. - A retry takes the not-quiesced repair branch (
:6689-6701). That branch re-persists the record and returns without admitting.
What is stranded:
- Since the merge, future intents were armed at
start()and fire normally. Overdue ones, whose timers already fired while quiesced and kept their intent, have no handle and stay stranded until the nextstart(). onInferenceStarteddoesn't cancel them either, so a later restart can deliver a superseded wake.
Fix:
- Admit in the repair branch too, or run admission from the
finallywhen!this.quiesced && !this.stopHasBegun(). - Let
onInferenceStarteddrop an intent whether or not it has a handle (R3-3's remainder).
R4-6. skip_reply reports durable: true for a wake nothing armed. (Found by both reviewers.)
What the code does:
- While the gate is disposed (between
dispose()and the nextstart()),armSelfWakereturnsarmed: false, durable: false(event-gate.ts:1864). - The framework sets
_wake_durableonly whenarmed(framework.ts:21546). - The registry computes
durable: input._wake_durable !== false(channel-registry.ts:4817), so the absent field reads astrue. The same happens on theAgentFramework.channelspath, which arms nothing. - R4-1's early dispose widens the window: a turn still finishing during
stop()now meets a disposed gate. - The note's "Self-wake in ~Ns" for an unarmed wake predates this PR. The
durablefield is new.
A related overclaim:
- A conversation fork's
skip_replyalso reportsdurable: true. - But
agentGonedeletes the fork's intent at every graceful stop (event-gate.ts:1844), and a crash drops it at boot, by design.
Fix:
- Pass
armedthrough as well. When nothing was armed, the receipt should say that no wake was scheduled and omitdurable. - For forks, either report
durable: falseor say in the tool description that a fork's wake ends with the fork.
R4-7. R3-4 remainder: 13 of 23 fix reversions still pass, and 11 of those should fail.
Method:
- Revert one fix at a time in the built
dist/. - Rerun
event-gate-selfwake,event-gate-sleep,host-quiesce,stop-with-host-operations,stop-with-work-in-flight,stop-without-streamand the threeskip-reply-wake*files: 276 tests, baseline green.
| Reverted | Failing |
|---|---|
R3-1 sleep quiesce guard in wakeFromSleep |
1 |
quiesce guard in fireRestoredSelfWake |
1 |
| R3-3 admission clears the live handle | 1 |
R3-2 gate disposed before queue.close() |
1 |
resume() admits |
1 |
create() admits only when serving |
1 |
| timestamp bounds | 1 |
wakeAt >= armedAt |
1 |
gate setSleep/armSelfWake report a failed write |
1 |
dispose() keeps durable intents |
10 |
quiesce guard in the ordinary armSelfWake callback |
0 |
identity check in fireRestoredSelfWake |
0 |
| restore-once guard | 0 |
| admission arms a future sleep | 0 |
clearSleep/wake report a failed write |
0 |
| receipt written into the shared input (R2-3) | 0 |
framework sleep result carries durable/note |
0 |
skip_reply receipt carries durable |
0 |
merge: resumeSleep() re-arms kept self-wakes |
0 |
merge: agentGone drops the journal entry |
0 |
onInferenceStarted persists the cancellation |
0 |
admission has() guard |
0 (expected: covered by the next row) |
armRestoredSelfWake clears the previous handle |
0 (expected: covered by the row above) |
Notes on the bold rows:
- Ordinary callback's quiesce guard. This is the R3-1 shape for an ordinarily armed self-wake.
test:413uses a restored intent, which goes throughfireRestoredSelfWake. Add the R3-1 framework probe for anarmSelfWakearmed while serving. - Restore-once guard. Without it, a second
restoreWakeIntents()replaces the intent objects. The live timer's identity check then fails, and the wake is silently dropped.test:397still asserts onlyselfWakeTimers.size === 1. Count fired requests instead. - Future sleep.
test:193andtest:232check onlygetSleepState().until. Assert a livesleepTimer, or advance past the deadline. - Receipts. Nothing drives the framework's
sleep,skip_replyorwakeresults, or the stored assistanttool_useinput. - The merge's two behaviours. Neither has a test:
stop()thenstart()with a pending self-wake, andagentGoneupdating the journal.
Nits
R4-8. Prose that is stale or now false.
dispose()'s comment (event-gate.ts:2334-2342): "stop() disposes the gate once its streams and event loop have ended." That is no longer true (R4-1).- Still stale from R3-5:
- the
restoreWakeIntentsdoc (:2051, "future timers"); - the
create()comment (framework.ts:3545); - the
clearSleepdoc (:1920, "Returns true").
- the
- PR description:
- "restores suppression and future timers": restore arms nothing.
- The receipts are from
3b87a3fon92125b0, with a 26-test focused suite; this head has 32 tests. - Neither the description nor the changelog says that a fork's wake ends with the fork (R4-6).
R4-9. R3-6 not fixed: the exported EventGate.clearSleep() returns { woke, durable } instead of a boolean (event-gate.ts:1921; EventGate is exported at src/index.ts:68).
if (gate.clearSleep())is now always true.- Fix: add a
changedfragment, or keep the boolean and expose durability separately.
Verified fine
Failed create()
restoreWakeIntents()writes nothing and arms nothing.- A rejected
create()leaves the journal untouched and no timer live (test:293; also probed).
Quiesced boot
start()'sresumeSleep()now arms restored timers while quiesced.- Every callback that can fire then keeps its intent: the sleep guard, the
fireRestoredSelfWakeguard, and the ordinary-callback guard. resume()then admits each due intent once.
stop() then start() on one instance
- A sleep or a self-wake kept through
dispose()fires afterresumeSleep(). Probed for both.
A journal naming an absent agent
- At boot, an overdue or future self-wake for a name with no agent is consumed. Its notice and request are dropped with
[addMessage] … not a registered agentand[inference-dropped] … agent_not_found, without a crash.
Clock and timer handling
- Backward clock steps and long sleeps are re-armed in chunks (
:1948-1950,test:519,test:338).
Main's newer gate-file code
- Main's stricter gate-file handling (#371, #452) reads only
gate.jsonand the privileged-users file, and lists no directory. Nothing in it touchesgate.wake-intents.jsonor its.corrupt-*and.tmp-*siblings.
Test runs at this head
- Node 22.22.1, context-manager 0.18.0, membrane 0.6.1, chronicle 0.4.0.
- Focused suite: 32/32.
- Full suite: 6182 tests, 6136 passed, 0 failed, 46 skipped.
- CI: 12 check runs, all green.
Nomination facts
- R3-1 to R3-4 are described accurately.
- The list omits the R3-5 and R3-6 nits. Both are dispositioned above.
Method. Two independent passes, merged after each claim was re-verified.
Sol pass. Codex gpt-5.6-sol (xhigh, read-only, no network) read the diff, the merge commit's decisions, main's changes since the merge base, all three earlier reviews, the author's replies and Greptile's round-3 comments. It ran nothing. About 610 s wall time, 73 shell commands, 6.91 M input tokens (6.66 M cached), 38.5 k output tokens (24.9 k of them reasoning).
Claude pass. Claude (Opus 5.5) built this head and main and ran the full suite at the head. It also ran probes against both builds, with real AgentFramework.create, a mock membrane and no MCPL servers:
· a debounce event during a held stop(), on both builds;
· a runtime quiesce with sleep and with self-wake, then a quiesced restart and resume();
· stop() held past a wake deadline;
· stop() then start() on one gate;
· a journal naming an absent agent;
· a failed admission write followed by start();
· resume() then stop() at a sweep of microtask depths, on both builds.
It also ran the 23-row fix-reversion table, and a git merge-tree against current main.
Who found what. Both: R4-1 (Sol by reading; Claude by probe, with the uncaught throw and the main comparison), R4-6, and the R3 dispositions. Sol only, confirmed by Claude: R4-2, R4-3 (Claude showed it is reachable without slowing) and R4-5. Claude only: R4-4, the measured table in R4-7, R4-8's dispose() comment, and the fork-receipt overclaim.
Rejected or adjusted from Sol.
· Sol's "fork wakes are lost" is reduced to the receipt overclaim in R4-6, because forks end at a restart by design.
· Sol's stale-callback scenario for the missing identity check is not reachable in Node, since a cleared timer never runs. It is kept as defence in depth under R3-3.
· Severities: R4-3, R4-5 and R4-6 are minor rather than major (a one-microtask window, a failed host-mode write, a receipt on a stopping framework). R3-6 stays a nit.
· Sol's static mutation audit is replaced by the measured table.
Limits. No real MCPL server or provider was started. The R4-1 ingress was fed through gate.evaluate, as the push handler's trigger filter calls it. R4-5 was verified by reading, not by a probe.
What happened in production
A resident calls
sleep(seconds)orskip_reply(wake_in_seconds), receives a concrete wake deadline, and the host restarts before it. Previously the wake disappeared: it was neither re-armed nor reconciled overdue, and the restarted resident had no marker explaining the lost promise. This was reproduced for graceful and abrupt restarts (qa-staging #33); the same timer path also covered the backward-clock failure in #25.Change
gate.wake-intents.json, besidegate.json, and are written atomically before a successful durable promise is reported.restoreWakeIntents()restores suppression and future timers without writing the journal.admitOverdueWakeIntents()performs at-most-once admission only afterAgentFramework.create()has completed successfully. A failed create disposes restored timers and leaves overdue and future intents durable.resume(), so another maintenance restart cannot lose an in-memory parked wake.sleep/skip_replyreportdurable: falseand explain that the wake may not survive restart.wakeAt >= armedAt; quarantine is reported only after a successful rename.dispose()clears process-local handles but leaves durable intent for the next process.The recovery guarantee is at most once: overdue intent is durably removed before its marker/inference is emitted. This PR does not add in-flight tool-call “outcome unknown” markers; that remains separate work.
Tests
test/event-gate-selfwake.test.tsnow covers normal self-wake behavior plus durable sleep/self-wake restore, overdue recovery, graceful restart, failed create after restore, quiesced startup, persistence-failure receipts, corrupt journal quarantine, timestamp validation, long timer chunking, anonymous wake fan-out, backward-clock re-arm, and repeated/auto recovery.Final receipts at
3b87a3fon upstream base92125b0:npx tsc --noEmit: cleangit diff --check: cleanChangelog:
changelog.d/durable-wake-intents.fixed.md.Co-developed with qa-engineer, the connectome-host resident that lost the wakes.
Known limitation: branch rewinds (#381)
The wake journal lives beside
gate.jsonand is process-global, not Chronicle-branch-scoped.undoLastTurn()switches the branch but does not withdraw EventGate timers or journal intents, so a persisted or restored wake can fire on the rewound branch — the hazard #381 describes, now also across a restart. The policy that a branch move withdraws pending wakes belongs with #381; this PR does not attempt it.