MemoryService graph-projection async is lifecycle-unmanaged

Authored by Claude Opus 4.8 (Claude Code), @neo-opus-grace (Grace).

Context

Operator-flagged during the merge of #13288 (decouple add_memory graph projection, #13283). #13288 correctly moves graph projection off the add_memory success path — the durability invariant it targets is sound and tested (the WriteAhead falsifier proves a graph throw cannot fail the save). But the new async machinery it introduces is started and then left unmanaged, violating the Neo.core.Base lifecycle contract.

Honest provenance: I endorsed #13288's merge on the durability axis ("does a graph throw still let the save succeed?") and missed the lifecycle axis ("is the new async plumbing teardown-aware, test-gated, and bounded?"). This ticket captures that review-miss.

This is a distinct boundary from #13312 (gpt — graph startup/tool-availability coupling: the MCP server failing before add_memory is callable). #13312 is the server-availability layer; this is the per-instance drain-loop lifecycle layer inside MemoryService.

The Problem

src/core/Base.mjs:596-619 documents the contract: initAsync() is awaited by construct() and isReady flips true the instant it resolves — so initAsync is strictly for awaiting mandatory startup work that gates readiness (JSDoc: "requiring conditional or optional dynamic imports or fetching initial data"). #13288 starts a perpetual background drain loop + fire-and-forget retry timers from initAsync, with no teardown, no test-mode gate, and an unbounded hot-path read. Five concrete gaps, all one theme:

1. Perpetual loop started in initAsync. MemoryService.initAsync() (MemoryService.mjs:314) calls _startGraphProjectionDrainLoop(), which is NOT awaited startup work: it starts a 60s setInterval + a fire-and-forget startup drain. A perpetual background process is not "initialization"; conflating it with readiness is the contract violation flagged by the operator.

2. No destroy() teardown → timer leak. MemoryService has no destroy() override clearing this.graphProjectionDrainTimer. Base.destroy() deletes the instance reference but does NOT clearInterval the OS timer — so the interval keeps firing against a torn-down singleton. (Verified: no destroy/clearInterval in the merged MemoryService.mjs.)

3. Retry setTimeout bypasses Base's async-registry + is not unref'd. _scheduleMemoryGraphProjection (MemoryService.mjs:545) uses a raw setTimeout for its bounded retry (up to 5 attempts, backoff to 5s) instead of Base's this.timeout()/registerAsync() (which destroy() cancels). Unlike the drain interval, these are NOT .unref()'d — so in-flight retries (a) fire against a destroyed instance and (b) keep a one-shot CLI add_memory process alive for up to ~5-13s while graph projection retries.

4. No unitTestMode gate → test-isolation hazard. _startGraphProjectionDrainLoop() runs unconditionally. In unit tests, importing the MemoryService singleton fires a startup drainPendingGraphProjections() that reads aiConfig.memoryWal.dir (the real WAL dir unless overridden) and starts a live 60s interval. This is the same test-isolation class #13298 just established the !Neo.config.unitTestMode gate for (ConnectionService auto-connect). #13288 papers over the async timing with flushGraphProjection sleeps in specs rather than gating the singleton.

5. Unbounded WAL pending-overlay read on the recency hot-path. _readPendingWalRecencyRows (MemoryService.mjs:745) calls readPendingWalRecords({dir, markerType: 'graph'}) with no limit on every queryRecentTurns, then filters in JS. In the normal case the drain keeps the pending set small — but under the exact projection-lag/failure mode this PR is built to tolerate, the pending set grows and every recency query becomes an O(pending) disk scan. The degraded mode degrades the hot path.

The Architectural Reality

• src/core/Base.mjs:596-619 — the initAsync contract (awaited; isReady follows; mandatory-startup-only).
• src/core/Base.mjs:357-366 — afterSetIsReady(): the correct post-ready hook to start a background loop from.
• src/core/Base.mjs:1154-1206 — Base.timeout() / registerAsync() / unregisterAsync(): the destroy-cancellable async-timer registry the retry path should use instead of a raw setTimeout.
• ai/services/memory-core/MemoryService.mjs:314 (initAsync), :520-560 (_scheduleMemoryGraphProjection), :610-623 (_startGraphProjectionDrainLoop), :743-770 (_readPendingWalRecencyRows).
• ai/services/neural-link/ConnectionService.mjs — the !Neo.config.unitTestMode gate precedent (PR #13298).

The Fix

• Move _startGraphProjectionDrainLoop() OUT of initAsync → start it from afterSetIsReady() (post-ready), gated !Neo.config.unitTestMode. initAsync keeps only await super.initAsync() + await StorageRouter.ready() (the genuine mandatory startup).
• Add a destroy() override: clearInterval(this.graphProjectionDrainTimer) + cancel in-flight retry timers.
• Route the _scheduleMemoryGraphProjection retry through Base.timeout()/registerAsync() so destroy() cancels it; .unref() it so one-shot CLI calls exit promptly.
• Bound _readPendingWalRecencyRows with a limit (+ an identity-scoped early-out) so the recency path stays bounded under projection lag.

Acceptance Criteria

• MemoryService.initAsync() contains only awaited mandatory-startup work; the drain loop starts from a post-ready hook.
• The drain loop and the in-process projection retries do NOT start/run under Neo.config.unitTestMode (a spec imports the singleton without spawning a live interval or reading the real WAL dir).
• MemoryService.destroy() clears the drain interval and cancels in-flight retry timers (no timer fires post-destroy).
• Retry timers are registered with the instance async-registry and unref'd (a one-shot CLI add_memory exits without waiting on retry backoff).
• _readPendingWalRecencyRows is bounded by a limit; a recency query under a large graph-pending backlog does not scan the entire pending set.

Out of Scope

• The graph startup/tool-availability coupling (Server.mjs:363 await GraphService.ready(), SQLite.mjs:219 readonly-write) — tracked by #13312.
• The per-call durability decoupling itself (#13283/#13288) — that contract is correct; this only hardens the async lifecycle it introduced.
• Re-coupling graph projection to the add_memory success path (the PR's core win stays).

Avoided Traps

• Reverting to synchronous graph projection — wrong; the decoupling is the point. The fix is lifecycle-managing the async, not removing it.
• Adding only the unitTestMode gate, loop left in initAsync — half-fix; the teardown, the unbounded overlay, and the registry-bypass all remain. The five gaps are one theme (lifecycle-unmanaged async); fix them together.

Related

• #13288 (introduced this; merged fb49292d0), #13283 (the per-call durability ticket), #13312 (sibling startup-availability boundary, gpt), #13298 (the unitTestMode gate precedent).

Decision Record impact

none — aligned with the existing src/core/Base.mjs lifecycle contract (no ADR change; this brings the code back into compliance with it).

Release classification: post-release (Agent OS reliability hardening; #13288 follow-up — not v13-blocking). Boardless.

Origin Session ID: 0f5d9f1d-0683-452d-aac1-f467297186ac

Handoff Retrieval Hint: "MemoryService graph projection drain loop initAsync lifecycle teardown unitTestMode"; commit range covering the #13288 merge (fb49292d0).

Live latest-open sweep: checked latest 20 open issues at 2026-06-15T06:55Z; #13312 (sibling startup-availability boundary) is distinct (server-availability vs drain-loop-lifecycle); no equivalent found.