Skip to content

fix(gate): persist sleep and self-wake intent across restarts; re-arm on backward clock step - #194

Open
rufalupa666-netizen wants to merge 16 commits into
anima-research:mainfrom
rufalupa666-netizen:fix/durable-sleep-intent
Open

rufalupa666-netizen wants to merge 16 commits into
anima-research:mainfrom
rufalupa666-netizen:fix/durable-sleep-intent

Conversation

@rufalupa666-netizen

@rufalupa666-netizen rufalupa666-netizen commented Sep 29, 2026 •

Copy link
Copy Markdown

What happened in production

A resident calls sleep(seconds) or skip_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

  • Durable wake journal. Sleep and per-agent self-wake intent live in gate.wake-intents.json, beside gate.json, and are written atomically before a successful durable promise is reported.
  • Two-phase startup recovery. restoreWakeIntents() restores suppression and future timers without writing the journal. admitOverdueWakeIntents() performs at-most-once admission only after AgentFramework.create() has completed successfully. A failed create disposes restored timers and leaves overdue and future intents durable.
  • Quiesce-safe recovery. A quiesced boot restores only. Overdue admission happens at resume(), so another maintenance restart cannot lose an in-memory parked wake.
  • Idempotent restoration. Repeated recovery cannot leak a second self-wake timer; restored timer replacement clears the prior handle.
  • Honest durability receipts. If an arming journal write fails, the live timer remains armed but sleep / skip_reply report durable: false and explain that the wake may not survive restart.
  • Journal hardening. Timestamps must fit the JavaScript Date range and satisfy wakeAt >= armedAt; quarantine is reported only after a successful rename.
  • Clock-step safety. A sleep callback that fires before its wall-clock deadline re-arms for the remainder rather than consuming the wake.
  • Graceful shutdown preserves intent. 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.ts now 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 3b87a3f on upstream base 92125b0:

  • focused wake suite: 26/26
  • full suite: 2,231 tests across 168 suites; 2,225 passed, 0 failed, 6 skipped
  • literal npx tsc --noEmit: clean
  • git diff --check: clean

Changelog: 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.json and 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.

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 0/5 Tier: apex

[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

  1. P1 Maintenance loses sleep wakes ▶
  2. P1 Shutdown can erase wakes ▶
  3. P1 Failed resume leaves wakes unarmed ▶
  4. P1 Restart drops the sleep timer ▶
  5. P1 Fork wakes lose their target ▶
  6. P1 One self-wake can fire twice ▶
  7. P1 Old fallback survives another turn ▶
  8. P1 `clearSleep()` breaks existing callers ▶
  9. P1 Restored batches bypass sleep ▶
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.

Summary

Persists sleep and skip_reply wake deadlines in a sibling journal and separates startup restoration from wake admission.

  • Sleep and self-wake requests survive host restarts.
  • Sleep timers keep waiting until their wall-clock deadline.

Reviews (5) · Last reviewed commit: "test(gate): pin resume recovery and vali..." · Reviewed by Greptile

Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts
Comment thread src/gate/event-gate.ts Outdated
@rufalupa666-netizen

Copy link
Copy Markdown
Author

Review at 6d7f017 adjudicated; all ten findings addressed at bac160e. Verified each against the code before changing anything.

  1. Shared journal path — valid. Journal is now derived from the gate's own config path (gate.json → gate.wake-intents.json); two gates in one directory no longer share a file.
  2. Recovery erased before initialization — valid, and the most important one. Recovery no longer runs in the constructor. EventGate.recoverWakeIntents() is called once at the end of a successful AgentFramework.create(); a due entry is removed from the journal only after its inference has been enqueued. Test: create() failing after gate construction leaves the intent in the journal for the next start.
  3. Write failure strands the resident — valid. The timer is armed before the journal write; the write is wrapped and logged, never leaving sleep set without a timer. Same on the expiry paths.
  4. Anonymous sleep not persisted — valid. Persisted with no agent name; on recovery it fans out to every registered agent, exactly as a normal expiry does.
  5. Unreadable journal treated as empty — valid. ENOENT is the only "empty" case; any read/parse failure quarantines the file as <name>.corrupt-<timestamp> and logs it, so the intents stay available for diagnosis instead of being overwritten.
  6. Startup write on empty state — valid. Nothing is written when there is nothing to change; a read-only gate directory starts as before.
  7. Long sleeps spin timers — valid. Each timer interval is clamped to 2^31−1 ms and re-armed in chunks until the wall-clock deadline.
  8. Malformed entries prevent startup — valid. Entries are validated (agent name string-or-absent, numeric armedAt/wakeAt, string source); invalid ones are skipped and logged.
  9. Suppressed count — not persisted per event, deliberately: a write per suppressed event is the wrong cost. getSleepState().suppressed is now documented as counting the current process only and resets after recovery.
  10. Recovery drops the note — valid. The saved note is included in the [wake-recovery] marker and in the inference reason.

Receipts at bac160e (on upstream main 5498805): npm run build && npm test → 983 pass, 0 fail; tsc --noEmit clean; git diff --check clean; targeted wake suites 20/20.

Overlap note: antra-tess's feat/mcpl-event-coalescing (today) also touches event-gate.ts fields and dispose(); we'll rebase on whichever lands first.

@rufalupa666-netizen

Copy link
Copy Markdown
Author

@greptileai bump

Comment thread src/framework.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
@rufalupa666-netizen

Copy link
Copy Markdown
Author

Round 2 (29341f7) adjudicated; four findings, all valid, fixed at e815e8c.

  1. Startup can wake sleeping residents — valid. recoverWakeIntents() now runs in create() before MCPL servers are initialised and their buffered input released, so a restored sleep is in place when input is evaluated. Test added.
  2. Overdue wakes can replay — valid; we over-corrected in round 1. Order is now: remove the due entry (durable) → marker → inference. If the journal write fails, that entry is skipped and logged and stays for the next start — at-most-once preserved, at the cost of a delayed wake rather than a duplicate. Test added.
  3. Standalone gates skip restoration — valid. EventGate gains autoRecover (default false) for embedders that construct a gate outside AgentFramework.create(); recoverWakeIntents() documented.
  4. Wake timers cannot keep processes alive — valid, and it contradicts the unref() suggested on EventGate.dispose() does not clear sleepTimer #192. Reverted: dispose() now clears every timer (which was EventGate.dispose() does not clear sleepTimer #192's actual complaint), so unref() is unnecessary and would let a standalone process exit before a promised wake. Test keeps asserting dispose() leaves no live timer.

Receipts at e815e8c (on 5498805): 986 pass, 0 fail; tsc --noEmit clean; git diff --check clean.

@greptileai bump

Comment thread src/framework.ts Outdated
Comment thread src/gate/event-gate.ts Outdated
@rufalupa666-netizen

Copy link
Copy Markdown
Author

Rebased onto main df97a85; round 3 (e815e8c) adjudicated; fixed at de7db7c.

  1. Failed startup can erase wakes — valid. Recovery now runs after all configuration/MCPL validation and before buffered input is released; if create() still fails after recovery, the failure path disposes the gate (clears timers, keeps the journal). Test: create() fails after recovery → journal intact, no live timer.
  2. Repeated recovery duplicates self-wakes — valid. recoverWakeIntents() is idempotent per intent: an already-armed intent is not armed again. Test with autoRecover + explicit call.
  3. Wake timers cannot keep processes alive — stale: it cites event-gate.ts:undefined-1795; no unref() remains in event-gate.ts at e815e8c or de7db7c (reverted in round 2, noted on EventGate.dispose() does not clear sleepTimer #192).

Overlap note: fix/batched-wake-addressed-locus (antra-tess) adds an EventGate option resolveRouteChannel; this PR adds autoRecover to the same options interface — trivial textual overlap, no semantic one.

Receipts at de7db7c (on df97a85): 1136 pass, 0 fail; tsc --noEmit clean; git diff --check clean.

@greptileai bump

Comment thread src/framework.ts Outdated
Comment on lines +1749 to +1750
// Restore sleep suppression before MCPL startup can release buffered input.
framework.eventGate?.recoverWakeIntents();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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

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 LariTesserae left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Recovery inference timing. recoverWakeIntents() runs before MCPL subsystems start, and an overdue intent calls requestInferenceFn right 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.
  2. Gate-global sleep. Sleep is per gate, not per agent (unchanged semantics), so agentName in the journal is informational for sleep and 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.

@rufalupa666-netizen

Copy link
Copy Markdown
Author

Thanks. Both answered at 83df97f.

  1. Recovery inference timing: the recovery request is enqueued before MCPL startup but the inference loop only begins processing after create() completes, so the recovered turn sees its registered tools — code evidence and a comment at the call site (ddb97cb), with a test asserting the recovered turn has its MCPL tools.
  2. Gate-global sleep: one sentence in the journal's doc comment — sleep is gate-global (unchanged), agentName is informational for sleep wake targeting and authoritative for self-wakes.

Receipts at 83df97f (on df97a85): 1136 pass, 0 fail; tsc --noEmit clean; git diff --check clean.

@rufalupa666-netizen

Copy link
Copy Markdown
Author

Rebased onto main 02adf27 and squashed to one commit (94cd49a) — the earlier "unref timers" subject no longer described the code and is gone. 1392 pass, 0 fail; tsc --noEmit clean; git diff --check clean.

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 sleep and skip_reply return;
  • 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:2467 calls recoverWakeIntents() before:
    • the MCPL prefix-collision checks (:2477, :2483);
    • initializeMcpl() (:2490), which rethrows server startup failures;
    • the tune-out coordinator (:2495-2525).
  • create() has no catch around any of these steps.
  • For each due intent, src/gate/event-gate.ts:1973-1979 deletes the entry and rewrites the journal. Then :1986-1997 adds the [wake-recovery] marker and pushes the inference request into pendingRequests, which is held only in memory.
  • The comment at the call site (framework.ts:2463-2466) is accurate when create() succeeds: the request waits for start(), so the recovered turn sees the MCPL tools. It doesn't cover the case where create() fails.

Probe at this head:

  • Setup: an overdue sleep in the journal, and two MCPL servers sharing toolPrefix: '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) makes create() fail with toolResultInlineMaxChars: 10.
  • That value is checked at framework.ts:2394-2399, before recovery. With the test instrumented, recoverWakeIntents is 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 of create(), after checkToolPatterns() and just before return 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 toolPrefix values 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-recovery request, and the journal's sleep entry is already gone.
    • Boot 2 has no request.
  • 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 when create() throws.
  • Probe: a future sleep 1.5 s out, with create() failing at the prefix check.
    • The orphaned gate's sleepTimer is still live.
    • At the deadline, wakeFromSleep writes the deletion (:1868), and the journal's sleep entry goes from present to absent.
  • 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: true plus 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.
  • Sleep is not affected, because armSleepTimer clears its predecessor.
  • The thread (round 3 #2) says recovery was made idempotent and tested with autoRecover plus an explicit call. Neither the guard nor the test is in de7db7c or 7e2376e.
  • Fix:
    • clearTimeout any existing timer for that agent before arming a new one;
    • add a recovered flag 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:

  • setSleep and armSelfWake (:1745, :1780) report success with a concrete until, but the promise won't survive a restart.
  • If wake calls clearSleep and 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 checks Number.isFinite.
  • Probe: a journal sleep with wakeAt: -1e20 passes validation.
    • It is admitted and deleted (:1979).
    • Then new Date(...).toISOString() (:1992) throws RangeError: Invalid time value out of recoverWakeIntents().
    • That fails one create() and leaves the journal as {"selfWakes":[]}.
  • Separately, when the quarantine rename fails, :1915 still logs "quarantined as …".
  • Fix:
    • limit armedAt and wakeAt to ±8.64e15 and require wakeAt >= armedAt;
    • log the quarantine path only after a successful rename.

R1-7. The changelog and PR description promise more than the code does.

  • changelog.d/durable-wake-intents.fixed.md says "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-in autoRecover for standalone gates;
    • it says "six cases", but there are 14 new tests;
    • it quotes receipts at 6d7f017.

Verified fine

Facts from the nomination

  • Both facts are confirmed at 7e2376e. recoverWakeIntents() is called at framework.ts:2467, before initializeMcpl at :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 after start().
  • 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 writeRecoveryFile and 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.
  • setSleep and clearSleep clear 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's shouldTriggerInference).
  • A restored batch replays that same arrival-time decision, just as a live batch firing after a later sleep would.

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.

@rufalupa666-netizen

Copy link
Copy Markdown
Author

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.

@rufalupa666-netizen

Copy link
Copy Markdown
Author

R1 addressed at 3b87a3f, one commit per finding:

  • R1-1 / R1-3 bcf621f: recovery is split into restoreWakeIntents() (no journal writes, before MCPL init) and admitOverdueWakeIntents() (last step of a successful create()); the remainder of create() is wrapped so a failure after restore disposes the gate and rethrows. The cap-validation test is replaced by a duplicate-toolPrefix failure after restore, with one overdue and one future intent: both stay in the journal, no timer is live.
  • R1-2 aad80f4: a quiesced boot restores only; resume() admits overdue intents.
  • R1-4 dd246e6: restore/admit are idempotent; a replaced self-wake timer is cleared first; the autoRecover + explicit-call regression is now actually in the branch.
  • R1-5 54b8e36: a failed journal write keeps the live timer and reports durable: false with one sentence in the sleep/skip_reply result.
  • R1-6 d8fa116: timestamps bounded to ±8.64e15, wakeAt >= armedAt; quarantine is logged only after a successful rename.
  • R1-7 5febd3b: changelog says at-most-once; the PR description is refreshed.
  • Boundary regressions for all of the above: 3b87a3f.
    Receipts: focused 26/26; full serial suite 2,247 tests / 170 suites, 2,241 passed, 0 failed, 6 skipped; tsc --noEmit and git diff --check clean.

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 durable receipt for skip_reply is 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 until resume().
  • Every timer callback deletes its journal entry before making that request:
    • the armSelfWake timer (:1784-1786);
    • wakeFromSleep (:1866-1870);
    • fireRestoredSelfWake (:2045-2047).
  • 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":[]};
    • pendingRequests is [agent:sleep, agent:self-wake].

    Then stop(), boot again (still quiesced), and resume(). 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.

  • Quiesce entered at runtime. The host is serving and the agent calls setSleep(1). The operator quiesces, the deadline passes, then stop(), a quiesced boot, and resume(). 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 sleepUntil passes.
    • admitOverdueWakeIntents() already treats 0 < sleepUntil <= now as due.
  • Make admission runnable on every resume(), not once per process. Either drop overdueWakeIntentsAdmitted or reset it on quiesce().
    • This also retries an admission whose journal write failed. Today the one-shot flag (:1982-1983) strands that intent until the next restart.
  • 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 initializeMcpl is 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.
  • (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 a wake-recovery request, then [self-wake] plus a self-wake request.
    • In create() the window is the synchronous tail after the last await, so it is narrow.
    • Sleep is not affected, because wakeFromSleep returns early when sleepUntil === 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 fireRestoredSelfWake return when selfWakeIntents.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:14997 sets inputObj._wake_durable = durable on enrichedCall.input.
  • enrichedCall is a shallow copy (:14878), so input is the same object the stream yielded.
  • In membrane 0.5.87, that object is also the tool_use block's input in roundContent: the yielded call is built with input: block.input from the same responseBlocks.
  • 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 leaves call.input untouched.
  • Alternatively, pass the receipt alongside the call.
  • The existing delete inputObj.wake_in_seconds on 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 it covers only the boot half.
  • R1-4: the test asserts selfWakeTimers.size === 1, which holds with or without the fix. The second set overwrites 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 -1e20 value is in the sleep slot. The new admission skips sleepUntil <= 0, so the value never reaches toISOString. Put an out-of-range pair such as armedAt: -2e20, wakeAt: -1e20 on a self-wake instead; without the bounds, that throws RangeError.
    • 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.
  • R1-5: the test covers the gate's return value only. Neither the sleep result's durable/note nor the skip_reply note 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).
  • wake returns { 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 clearSleep write 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 durable from wake the way sleep does.
  • 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 then wakeAt: armedAt + ms, as setSleep does.

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 at resume().
  • 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 at resume(), 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, and readWakeState catches its own read and parse errors.
  • Every statement between restore and return is inside the try. 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, and ResumeBlockedError. A refused or blocked resume therefore leaves the intent durable, and a later resume({ 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 releasedRequests and 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.
  • wakeFromSleep re-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.

@rufalupa666-netizen

Copy link
Copy Markdown
Author

R2 at ba3e182, one commit per finding, with what is and isn't on the branch:

  • R2-1 / R2-2 / R2-6 5838095: a sleep or self-wake callback that fires while quiesced returns and leaves the intent in memory and in the journal (src/gate/event-gate.ts, two if (this.quiesced) return sites); the one-shot admission flag is gone and resume() calls admitOverdueWakeIntents() every time (src/framework.ts:4651); restore loads state only and admission arms or admits, so no timer is live before the commit point; armedAt is read once (event-gate.ts:1741, :1779).
  • R2-3 a870c01: enrichedCall.input = { ...inputObj, _wake_durable: durable } — the model's stored call is never mutated (src/framework.ts:14996).
  • R2-5 0e8c100: the changelog and the journal doc say "at most once while journal writes succeed" (changelog.d/durable-wake-intents.fixed.md:2, event-gate.ts:549); wake returns durable (framework.ts:18337-18338).
  • R2-7 — partial. The recovery marker now says "while the host was offline or quiesced" (event-gate.ts:2022) and the resume() comment no longer says one-shot wakes are lost; the quiesced-boot banner at framework.ts:2449-2450 still says parked wakes "are gone", which is no longer true for sleep and self-wake intents; next commit.
  • R2-4 — partial. ba3e182 adds two tests: restore without arming plus repeated restore reads once, and a future wake that falls due while quiesced survives a restart and admits once on resume(). The three vacuous tests from your mutation table — R1-4 counting fired requests, R1-6 an out-of-range self-wake pair and a backwards entry asserting no request, R1-5 the sleep result's durable/note — are not yet strengthened; next commit.
  • Added to the description: durable intents are not branch-scoped, so a restored wake can fire on a rewound branch (An operator's undo leaves a pending wake to run on the rewound branch, and that run clears the redo stack #381); that policy belongs with An operator's undo leaves a pending wake to run on the rewound branch, and that run clears the redo stack #381.
    Receipts: full serial suite on this head (run admitted 5 s after the commit) 2,249 tests / 170 suites, 2,243 passed, 0 failed, 6 skipped; tsc --noEmit and git diff --check clean.

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. The skip_reply receipt no longer touches the stored call. Self-wakes that fall due while quiesced now survive a restart.
  • R2-1 is still open for sleep. wakeFromSleep did 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 inside stop() (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 forced resume(); 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".

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) clears sleepUntil, persists the deletion (:1876), and requests inference (:1887). The framework parks that request in pendingRequests.
  • A runtime quiesce leaves a live sleep timer running, because setQuiesced doesn'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):

  1. A serving host sets setSleep(1, 'check result', 'agent'), then quiesce().
  2. After 1.4 s the journal is {"version":1,"selfWakes":[]} and pendingRequests is [agent:sleep].
  3. stop(), a quiesced boot, then resume(). 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), add if (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.
  • admitOverdueWakeIntents already treats 0 < sleepUntil <= now as due (:1992).
  • Test: run the probe above through the real quiesce(), stop(), create() and resume(), 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:

  • stopOwned closes the queue at framework.ts:2710.
  • It then awaits active streams (:2754), any in-flight maintenance pass (:2768), the event loop (:2772) and, on main, operator operations up to teardownTimeoutMs.
  • 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:

  1. Arm a 1 s sleep, or a 1 s self-wake.
  2. Hold stop() open for 1.5 s with an in-flight maintenance pass.
  3. After stop() returns, the journal is {"version":1,"selfWakes":[]} and the request sits on the dead queue ([agent:sleep] or [agent:self-wake]).
  4. 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_reply cadence 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, before queue.close(). That means moving the timer half of dispose() earlier (clear selfWakeTimers and sleepTimer, leave the intents alone), or quiescing the gate there once R3-1's guard is in.
  • Separately, have runResume skip admission once stop has begun (if (!this.stopHasBegun()) … before :4651).
    • Today that is latent. flushDeferredWrites never 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.
  • Test: hold stop() open past a 1 s deadline for both kinds. Assert that the journal still holds them, and that a fresh create() 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:

  • admitOverdueWakeIntents deletes a due self-wake (:2008) but leaves any handle in selfWakeTimers.
  • The ordinary armSelfWake callback (:1788-1819) deletes and emits without the identity check that fireRestoredSelfWake now 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:

  1. quiesce(), then armSelfWake('agent', 1).
  2. Keep the event loop busy until the deadline has passed, then call resume() in the same tick.
  3. Result: pendingRequests is [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 armSelfWake callback the same identity check: capture intent, and return if this.selfWakeIntents.get(agentName) !== intent.
  • Let onInferenceStarted drop 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.
  • "Re-arms it after restart" tests (:182, :221): both check only getSleepState().until. Deleting the sleep arming from admission leaves a restored sleep that never wakes, and both still pass. Assert that sleepTimer is set, or advance past the deadline and count requests.
  • R1-4 (:368, :384): both still assert only selfWakeTimers.size === 1, which an overwritten handle also satisfies. Count fired requests, or spy on readWakeState.
    • 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, toISOString throws.
  • Receipts: nothing covers the framework's sleep/skip_reply/wake results or the stored assistant tool_use input.

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.
  • restoreWakeIntents doc (event-gate.ts:1965) and the create() comment (framework.ts:2463-2464): both say restore restores "future timers". Restore arms nothing now; admission does.
  • clearSleep doc (: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.

  • EventGate is 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 changed changelog 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 ResumeBlockedError all 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.input between the yielded call and roundContent.
  • Reassigning enrichedCall.input is 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.

nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 17, 2026
…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>
@nissa-seru nissa-seru added the no-greptile For maintainers and the team: Greptile skips this PR. Not for general use. label Oct 17, 2026
Comment thread src/gate/event-gate.ts
this.sleepArmedAt = 0;
this.sleepNote = undefined;
this.sleepAgent = undefined;
this.tryPersistWakeState('expiring sleep');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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

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.

Comment thread src/gate/event-gate.ts
Comment on lines +1791 to +1792
this.selfWakeIntents.delete(agentName);
this.tryPersistWakeState('firing self-wake');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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

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.

Comment thread src/framework.ts
// flushDeferredWrites); nothing here may reach the plane reopen below.
await this.flushDeferredWrites('resume');
this.persistHostMode(null);
this.eventGate?.admitOverdueWakeIntents();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Comment thread src/gate/event-gate.ts
// 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; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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

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.

Comment thread src/gate/event-gate.ts
} else this.selfWakeIntents.set(item.intent.agentName!, item.intent);
continue;
}
const targets = item.intent.agentName ? [item.intent.agentName] : this.getAgentNamesFn();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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

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.

Comment thread src/gate/event-gate.ts Outdated
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!);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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

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.

Comment thread src/gate/event-gate.ts
if (selfWake) {
clearTimeout(selfWake);
this.selfWakeTimers.delete(agentName);
this.selfWakeIntents.delete(agentName);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Comment thread src/gate/event-gate.ts

/** End sleep immediately. Returns true if the agent was asleep. */
clearSleep(): boolean {
clearSleep(): { woke: boolean; durable: boolean } {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

@rufalupa666-netizen

Copy link
Copy Markdown
Author

R3 is answered at 7168daa9, rebased onto main 2f05c28 (merges clean onto today's 97ca7eba); every round's commits are kept, nothing squashed.

  • R3-1/R3-3 quiesce and shutdown wake races: 8327768d (src/gate/event-gate.ts:1893-1896 keeps a quiesced sleep; :2036 clears a live self-wake timer at admission).
  • R3-2/R3-4 EventGate disposal before queue close: 7168daa9 (src/framework.ts:3649), with a shutdown-order test at test/event-gate-selfwake.test.ts:590.
  • R2-7 the startup banner no longer says the wakes "are gone" (src/framework.ts:3238).
  • R2-4 the three tests that asserted nothing now do: 13 assertions added across test/event-gate-selfwake.test.ts:440,467,629.

Full suite at 7168daa9, run under TSM 2.0 on the resident host (run r1-9a6345, 18m20s): 5,325 tests, 5,277 passed, 47 skipped, 1 failed — test/pack-installs-alone.test.ts:294, a local npm linked-install mismatch outside the gate. git diff --check clean.

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
@rufalupa666-netizen

Copy link
Copy Markdown
Author

Pushed e91cfb68: main (5834c2c4) merged into this branch, round commits untouched. The conflicts were with #634 in setSleep/wakeFromSleep/dispose; both are kept, and the merge commit's message lists each decision. The one worth a look: resumeSleep() now also re-arms the self-wake intents kept through dispose(), since a stop()+start() in one process otherwise left them journaled with no timer.

Suite on e91cfb68 (CM 0.18.0, membrane 0.6.0, Node 22.22.1): 6202 tests, 6131 pass, 2 fail, 69 skipped; the two are code-execution-env, unchanged from main.

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() and resume():
    • 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 next create() recovers it once.
    • The resume() double fire is gone.
  • The R3-2 fix moved too much. It moved the whole of EventGate.dispose() to the top of stop(), 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 outlives stop() and then throws Store has been closed as 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: true for 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 at start().

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, agentGone deletes it.
    • Both match the framework's rule that forks end at a restart. The receipt's durable: true for 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:

  • stopOwned used to call eventGate.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 later dispose() 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: evaluate returns undefined from a stopped script, so policies and the default decide.
  • An embedder without an uncaughtException handler 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 new EventGate.suspendWakes() (name it as you like). It clears sleepTimer and selfWakeTimers, sets disposed, and leaves the intents and everything else alone.
    • Keep the full dispose() where main has it.
  • Optionally, make handleDebounce a no-op while disposed. That also closes main's much smaller version of this window, between its dispose() and the MCPL closeAll.
  • 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.

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 e91cfb68 reports two conflicting hunks in event-gate.ts: the node:fs/node:path imports, and adjacent constructor options (main's channelShown and isNewestEntry beside this PR's autoRecover). 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 a WritePlacement. 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 armSelfWake callback carries it.
  • This PR's two new delivery paths call addMessageFn/requestInferenceFn without it: fireRestoredSelfWake, and the [wake-recovery] marker and request in admitOverdueWakeIntents.
  • 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 in readWakeState too.

Fix:

  • Merge or rebase onto current main.
  • Fold the duplicated fire bodies, the ordinary callback (:1876-1907) and fireRestoredSelfWake (:2144-2158), into one fireSelfWake(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:

  • runResume checks queue.isClosed once (framework.ts:6742), then awaits flushDeferredWrites (: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), then admitOverdueWakeIntents on 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 when this.disposed.
  • In runResume, skip both persist and admission if stopHasBegun() 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:

  • admitOverdueWakeIntents declines to emit when its journal write fails (:2100-2106). test:567 pins that at gate level.
  • In the framework, the next call is start(), then resumeSleep(), 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 through fireRestoredSelfWake.
  • 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(): an agent:sleep or agent:self-wake request.
  • The journal still holds the intent.
  • The agent gets a plain sleep wake or [self-wake] … timer elapsed notice, 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 through admitOverdueWakeIntents() 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() and start(). 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, since writeRecoveryFile does on ENOSPC or EROFS. So can flushDeferredWrites. Either throw skips admitOverdueWakeIntents() (framework.ts:6788-6792).
  • The finally still reopens the MCPL planes with quiesced already 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 next start().
  • onInferenceStarted doesn'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 finally when !this.quiesced && !this.stopHasBegun().
  • Let onInferenceStarted drop 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 next start()), armSelfWake returns armed: false, durable: false (event-gate.ts:1864).
  • The framework sets _wake_durable only when armed (framework.ts:21546).
  • The registry computes durable: input._wake_durable !== false (channel-registry.ts:4817), so the absent field reads as true. The same happens on the AgentFramework.channels path, 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 durable field is new.

A related overclaim:

  • A conversation fork's skip_reply also reports durable: true.
  • But agentGone deletes the fork's intent at every graceful stop (event-gate.ts:1844), and a crash drops it at boot, by design.

Fix:

  • Pass armed through as well. When nothing was armed, the receipt should say that no wake was scheduled and omit durable.
  • For forks, either report durable: false or 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-stream and the three skip-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:413 uses a restored intent, which goes through fireRestoredSelfWake. Add the R3-1 framework probe for an armSelfWake armed 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:397 still asserts only selfWakeTimers.size === 1. Count fired requests instead.
  • Future sleep. test:193 and test:232 check only getSleepState().until. Assert a live sleepTimer, or advance past the deadline.
  • Receipts. Nothing drives the framework's sleep, skip_reply or wake results, or the stored assistant tool_use input.
  • The merge's two behaviours. Neither has a test: stop() then start() with a pending self-wake, and agentGone updating 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 restoreWakeIntents doc (:2051, "future timers");
    • the create() comment (framework.ts:3545);
    • the clearSleep doc (:1920, "Returns true").
  • PR description:
    • "restores suppression and future timers": restore arms nothing.
    • The receipts are from 3b87a3f on 92125b0, 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 changed fragment, 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()'s resumeSleep() now arms restored timers while quiesced.
  • Every callback that can fire then keeps its intent: the sleep guard, the fireRestoredSelfWake guard, 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 after resumeSleep(). 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 agent and [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.json and the privileged-users file, and lists no directory. Nothing in it touches gate.wake-intents.json or 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

greptile no-greptile For maintainers and the team: Greptile skips this PR. Not for general use.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants