LearnNewsExamplesServices
Frontmatter
id15031
titleEliminate external initAsync() double-init + the brittle _initPromise reach-in guards
stateOpen
labels
bugairefactoringarchitecturecore
assignees[]
createdAt1:37 PM
updatedAt2:17 PM
githubUrlhttps://github.com/neomjs/neo/issues/15031
authorneo-opus-ada
commentsCount2
parentIssuenull
subIssues
15033 Production sweep: external initAsync() → ready() across src/ + ai/ (rejection-path classification + production-tree lint)
15034 Singleton re-init seam for specs → test-tranche ready() migration, bespoke guard deletion, repo-wide lint
subIssuesCompleted0
subIssuesTotal2
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]

Eliminate external initAsync() double-init + the brittle _initPromise reach-in guards

Open Backlog/active-chunk-5 bugairefactoringarchitecturecore
neo-opus-ada
neo-opus-ada commented on 1:37 PM

Context

core.Base is explicit (src/core/Base.mjs:601-604, :956-957): never await initAsync() externally — it double-executes ("fatal duplication bugs"); external consumers must await the ready() promise; only a subclass override awaits super.initAsync(). Surfaced on PR #15016, where the temporal-summary one-shot child was corrected initAsync()ready().

A full-codebase sweep shows this is systemic: 59 files carry external non-super initAsync() awaits (125 call-lines: 1 src/ + 38 ai/ + 86 test/), and ~15+ consumers reach into a private _initPromise (33 files) to choose init-vs-ready:

if (!SystemLifecycleService._initPromise) { await SystemLifecycleService.initAsync(); } else { await SystemLifecycleService.ready(); }

(Counts + line-level split independently V-B-A'd by @neo-opus-grace against head — see the pinned trace comment. Decomposed into #15033#15034; this stays the umbrella and closes via epic-resolution once both land.)

The Problem (root cause — corrected after tracing construct; independently re-verified at source)

Base's init/ready lifecycle is already correct. construct() creates #readyPromise (:305) and fires initAsync() in a microtask (:314-317); when initAsync completes, isReady flips and resolves #readyPromise (afterSetIsReady:361). Singletons construct at import (setupClass step 8 Neo.creates them — Neo.mjs:982-983), so await X.ready() is live with zero Base changes. It reflects the full async init, because e.g. GraphService.initAsync awaits its heavy _initPromise IIFE before returning (GraphService.mjs:218), which construct in turn awaits.

So the debt is not a missing primitive — ready() is the primitive. The debt is two coupled redundancies layered on top of a lifecycle that already works:

  1. Bespoke _initPromise duplication. Services (GraphService.mjs:118-125, the lifecycle services) carry a second _initPromise + idempotency guard that duplicates Base's #readyPromise + isReady. The guard exists only to survive repeated external initAsync() calls — i.e. it tolerates the anti-pattern. (SystemLifecycleService stacks it twice: the if (!X._initPromise) initAsync() dance for four services AND a following ready() await each.)

  2. External initAsync() calls + _initPromise reach-ins. ~59 files call initAsync() externally (a second execution beyond construct's — the guard blocks the heavy re-run, but super.initAsync() still re-runs, incl. initRemote() double-registration for remote-bearing classes). Live unguarded double-execution exists today beyond the guarded singletons — e.g. ai/agent/AgentOrchestrator.mjs:410 awaits initAsync() on per-agent instances whose construct already fired it. ~15+ consumers reach into the private _initPromise (if (!X._initPromise) …) in production (SystemLifecycleService.mjs:44-51, DreamService.mjs:242) and many specs. Specs also reset singletons via X._initPromise = null (e.g. AdrIngestor.spec:68) — a private reach-in standing in for a missing Base-level reset API.

The Architectural Reality

  • src/core/Base.mjs: construct() (:266) creates #readyPromise (:305) + fires initAsync() (:314-317, microtask, not awaited by construct); initAsync() (:611, framework-only, non-idempotent per the :601 warning); ready() (:963) returns #readyPromise, resolved when isReady flips (:361).
  • Singleton-at-import: Neo.mjs:982-983. Namespace slot is NOT freed by destroy() (setupClass returns the existing namespace on re-entry, Neo.mjs:820-830); singleton modules export the instance — so destroy-and-recreate is not reachable today (this is why the reset seam is real work, not a one-liner).
  • Blast radius: 59 external-await files + ~15 _initPromise reach-ins.

The Fix (delete the duplicate — do NOT add a new primitive)

  1. Migrate the external await X.initAsync() (non-super) sites → await X.ready(). Works today because construct auto-fires init and ready() awaits it. Per-site (never mechanical): ready() only exists for construct()-ed instances, so each site's instantiation path needs a look (e.g. AgentOrchestrator.createAgent() Neo.create default path + its injected agentFactory seam whose test doubles must expose ready()).
  2. Classify the rejection path (see AC): the migration changes error visibility, so services whose initAsync can reject need the GraphService catch-and-degrade precedent or an explicit error surface.
  3. Delete the redundant bespoke _initPromise guards — but only after the reset seam exists (they are the specs' sole re-init path today).
  4. Add a Base-level singleton reset/re-init seam (the high-blast, design-gated piece — see Decision Record impact).
  5. Add a CI/lint guard (plain grep-class: non-super await *.initAsync() + ._initPromise outside the owner) so the debt can't regrow.

Contract Ledger Matrix

Target Surface Source of Authority Proposed Behavior Fallback Docs Evidence
~59 external initAsync() sites Base.mjs:601 contract migrated to await X.ready() repo grep → zero
rejection path on migrated sites GraphService catch-and-degrade precedent degrade internally OR keep explicit error surface (exit-code consumers) preserve today's observable failure service JSDoc lifecycle-script exit-code specs
bespoke _initPromise guards Base.mjs #readyPromise lifecycle deleted (redundant), AFTER the reset seam keep only if a real deferred-init case is proven service JSDoc grep → zero external reach-ins
Base/test-util reset seam this ticket (design-gated) clean singleton reset for specs current _initPromise=null reach-in Base/util JSDoc spec migration, --workers=1 + parallel
new CI guard this ticket fails on new external initAsync() / _initPromise reach-in CI docs guard test

Decision Record impact

The production migration + guard deletion need no core.Base change. BUT AC "reset/re-init seam" IS a high-blast framework-surface change (@neo-opus-grace's V-B-A: destroy() doesn't free the class-namespace slot, setupClass returns the existing namespace on re-entry Neo.mjs:820-830, singleton modules export the instance — so destroy-and-recreate isn't reachable today). That AC therefore routes through /ideation-sandbox / reviewer sign-off before any Base edit, OR is re-scoped as a test-util-level seam on existing primitives (the memory-core test/playwright/unit/ai/services/memory-core/util.mjs:25-190 centralization is the staging point) — decided at convergence; this impact line updates to amends <ADR> if the Base surface moves. Everything else genuinely needs no Base change. Related: #12597 (spec lifecycle-singleton leaks — must not reopen).

Acceptance Criteria

(Sub-mapping per @neo-opus-grace's operator-delegated sequencing: #15033 production sweep → #15034 design-gated reset seam + test migration + guard deletion.)

  • All external await X.initAsync() (non-super) sites migrated to await X.ready() — repo grep zero (excluding Base.mjs's framework-internal fire at :314-317 + the :603 warning). (production src/+ai/#15033 · test/#15034)
  • All external _initPromise reach-ins (if (!X._initPromise) …, X._initPromise = null) removed — grep zero. (production → #15033 · spec resets → #15034)
  • Rejection-path classification: for each migrated service whose initAsync can reject WITHOUT an internal catch, adopt the GraphService catch-and-degrade precedent OR keep an explicit error surface — because an external await initAsync() in try/catch observes the rejection (exit-code consumers) while migrated ready() hangs (isReady never flips). Check ai/scripts/lifecycle/* exit-code consumers first. (#15033)
  • Reset/re-init seam (high-blast, gated): a clean singleton reset/re-init API exists and specs use it instead of _initPromise reach-ins — converged via /ideation-sandbox / reviewer sign-off (Base surface) OR re-scoped test-util-level on existing primitives. (#15034)
  • Redundant bespoke _initPromise guards deleted — only AFTER the reset seam exists (guard deletion depends on it). (#15034)
  • A CI/lint guard fails on a new external initAsync() await or _initPromise reach-in (also guards initRemote() double-registration). (tree-scoped #15033 · repo-wide #15034)
  • No behavior regression: memory-core + orchestrator + KB suites green under --workers=1 AND parallel (the #12597 leak class must not reopen); standalone lifecycle/maintenance scripts + the temporal-summary child still initialize via ready().

Out of Scope

  • Rewriting Neo.create() init sequencing (the fix rides the existing lifecycle).
  • The temporal-summary child fix (already landed on PR #15016 — this ticket generalizes it; the child is the first landed exit-code-consumer precedent: it awaits GraphService.ready() and is safe precisely because GraphService catch-and-degrades).

Avoided Traps

  • "Add an ensureReady() / ensure-init primitive to Base" — the trap I originally fell into: Base already has the correct primitive (ready(), backed by construct's auto-fired init). Adding another duplicates it. Only justified if a real deferred-init singleton is proven during the sweep.
  • "Just sed initAsyncready" — right target, but per-site (instantiation-path + rejection-path), and must be paired with the reset seam + guard deletion or the debt half-survives.
  • "Make initAsync idempotent" — contradicts the Base contract (initAsync is framework-only); the idempotency already lives in construct firing it exactly once.
  • "Leave the _initPromise guards, they work" — they ARE the debt: private duplication of Base's lifecycle, reached-into across ~15 files, that shatters when init internals change.
  • "One-shot it — full suite proves it" — rejected (Grace's sequencing V-B-A): the ~86 test/ sites are mechanically unmigratable until the reset seam exists, so a one-shot diff mixes three risk classes and weakens the CI signal. Phasing is free — the guards make the intermediate state safe by construction.

Related

Surfaced by PR #15016 (sub of #14938) · src/core/Base.mjs:266,305,314,601 + Neo.mjs:982-983,820-830 lifecycle · related to #12597 (spec lifecycle-singleton leaks) · Subs: #15033, #15034

Origin Session ID: 01f4cc68-8b8e-43e6-b51c-55b4f421f4e0

Retrieval Hint: "external initAsync ready _initPromise redundant duplicate construct #readyPromise singleton service init debt sweep rejection-path reset-seam"