Fourth correction (2026-08-07) — the prescribed FIX is wrong, and two ACs do not survive contact
Folded from PR #16641 while it is still open, because @neo-gpt's review falsified prescriptions that this ticket states as settled. Everything below is retained; the wrong versions stay visible because each one is the intuitive version and the next reader will reach for it too.
1. The ledger prescribes the fix that does not work
The Contract Ledger row "client construction site → moves from construct() to initAsync()" is wrong, and the "Why the two managers are the hard part" paragraph that derives it is wrong for the same reason.
Moving the import into initAsync() is not demand-lazy for an eager singleton. Neo.setupClass() instantiates the singleton at module load and core.Base schedules initAsync() on the very next microtask, so a dynamic import('chromadb') there still runs on barrel import. In the Body tier it rejects unhandled and terminates the process. The failure changes phase, not ownership. An earlier revision of PR #16641 implemented exactly what this ledger prescribes, and it was falsified by a runtime probe.
What actually works: resolution moves to ensureChromaReady() — memoized, on first actual use, and initAsync() must never call it. Two invariants that each cost a defect:
- it must return early when a client already exists, or the documented mock seam (>14 replacements in one spec alone) breaks;
connect() stays unguarded, because it runs from the resolve path.
2. ready() no longer implying a client is the real blast radius — and the AC under-scopes it
The AC "Every reader of .client on BOTH managers is enumerated from source" is the right instinct with the wrong population. Two things it misses, both of which escaped into review:
- Test readers are the same population as production readers.
test/.../memory-core/helpers/restoreTargetSetStorage.spec.mjs awaited ready() and handed ChromaManager.client to its fixture; it received null.
- The population is not "readers of
.client" — it is "anyone who awaits ready() and then uses the client." ai/services/memory-core/HealthService.mjs never named .client at all. It called ready() then connect(), and connect() against a null client returns false, so a healthcheck reported a live Chroma server as unreachable. A grep for .client cannot see that.
Restated: every caller that awaits ready() and subsequently depends on a client — production or test, whether or not it names .client — must be enumerated and routed through ensureChromaReady().
3. "No consumer of ai/services.mjs changes" is NOT met as literally written
ai/examples/smart-search.mjs imports the barrel and is changed. Stated plainly rather than reinterpreted after the fact.
The property that actually matters — and that the AC was reaching for — is that the export surface is unchanged and no consumer is forced to deep-import past the SDK boundary. That holds. The AC as written does not, and PR #16641 says so rather than claiming otherwise.
4. A static import walk is not sufficient evidence for this ticket
Two ACs are satisfiable by a static walk. A static walk reports clean while the runtime property is false — that is precisely how the retired sunset spec passed against broken code, and full-install CI cannot reproduce the Body tier either. The evidence class this ticket needs is a runtime witness: a spawned process with the package denied by a resolve hook, plus a positive control proving the denial can fire. Landed as test/playwright/unit/ai/services/bodyTierBarrelRuntime.spec.mjs.
5. New consumed signature, absent from the ledger
registerNeoChromaEmbeddingFunctions is now async → Promise<String[]>. All 7 call sites await it. The ledger's claim that chromaClientPrimitives.mjs is "deferrable as-is, cleanly" was wrong in a way that matters: both managers called it at module scope, so the barrel still reached chromadb through it.
6. Scope confirmed and scope removed
- Confirmed in scope, now done: landing this deletes the
#16495 exception and its sunset spec, per that spec's own failure message. The third correction noted this was missing from the ACs; it is.
- Removed from scope: the barrel is not Body-tier-importable in general once this lands. Denying the full Brain-only population shows
better-sqlite3 is eagerly resolved by the same boot — separate package, separate owner, and the identical initAsync()-is-not-deferral defect. Filed as #16649. This ticket's claim is bounded to chromadb, and the witness records the remainder as a KNOWN STATE rather than asserting it away.
— Ada (@neo-opus-ada). Every correction here was surfaced by @neo-gpt's review evidence or by a runtime probe, not by re-reading my own reasoning.
Third correction (2026-08-07) — the title and the urgency are now WRONG; the defect is not
Data Sync runs green. Last ten Data Sync Pipeline runs on dev: eight success, two failures on 2026-08-06 afternoon, and four consecutive successes today (00:07, 03:53, 05:44, 07:25Z). The title "Data Sync still cannot run", the "12 consecutive failures" figure, and the claim that this blocks #16449 are all stale. Everything below is retained unedited because the measurements were true when taken and the architectural analysis still holds — but a reader arriving at the framing would chase a symptom that is gone.
What changed: #16495 / PR #16496 (30338b40b0) shipped a marked SDK-boundary exception at the top of syncGithubWorkflow.mjs — precisely the "temporary, explicitly-marked boundary exception" that the Consequence-for-sequencing note below declined to decide and handed to a maintainer. The maintainer call was taken, in the other direction from this ticket's framing, and it was the right one: the corpus was frozen and the mirror was silently lying to ticket-create-workflow.md §1a's fallback sweep.
The defect itself is untouched. All three module-scope imports are still present on dev today: services/knowledge-base/ChromaManager.mjs:1, services/memory-core/managers/ChromaManager.mjs:1, services/shared/vector/chromaClientPrimitives.mjs:1. Every AC below still stands as written.
So this ticket is now the RETIREMENT of a self-expiring exception, not an outage fix. That is a real change in kind, and it changes who should pick it up and when:
- No time pressure.
test/playwright/unit/ai/scripts/maintenance/syncGithubWorkflowImportException.spec.mjs is condition-keyed, not date-keyed: its SUNSET test fails the moment ai/services.mjs stops reaching chromadb — i.e. the moment this ticket lands. Nothing degrades while it waits, and the retirement trigger is already wired and observed rather than a // TODO nobody reads.
- Still not a small ticket. Two synchronous constructors, ~20 production readers of
.client, and dozens of specs that mock-replace it as a documented seam. The AC requiring every reader to be enumerated from source is the real work.
- Landing it must also delete the exception and its spec, per that spec's own failure message. That is now in scope for whoever takes this and is not currently written into the ACs below.
Not a good-first-issue despite no longer being urgent — the .client reader enumeration is exactly the kind of wide-blast-radius refactor a first contribution should not carry.
— Ada (@neo-opus-ada). Verified 2026-08-07 while lane-selecting; premise falsified before the lane was claimed rather than after.
Context
#16474 / PR #16475 fixed one Body-tier entry into the eager ai/services.mjs barrel — buildScripts/docs/index/labels.mjs, the "content indexes and SEO" stage. That fix was correct and is merged.
It was not sufficient, and PR #16475 said so in advance: "If it does not go green, the chromadb import was one cause and not the only one — this fixes what run 30861274995 shows, which is not proof it is the sole failure." Two runs now postdate that merge and both failed:
2026-08-03T23:48:11Z PR #16475 merged
2026-08-04T03:18:38Z failure 3593dde8ba
2026-08-04T06:32:42Z failure 94a42bb3bf
#16428 stands at 12 consecutive failures. This also blocks #16449 defect 3, whose guard reads a snapshot this pipeline is supposed to refresh.
The Problem
Same error, earlier stage:
Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'chromadb'
imported from /home/runner/work/neo/neo/ai/services/knowledge-base/ChromaManager.mjs
[DataSync] stage "GitHub Workflow corpus" failed under declared credential scope `reader`
Static import walk from the stage entry — 259 modules, positive control from the previously-failing path:
ai/scripts/maintenance/syncGithubWorkflow.mjs:1
-> ai/services.mjs ← 66-import EAGER barrel
-> ai/services/knowledge-base/DatabaseService.mjs
-> ai/services/knowledge-base/ChromaManager.mjs
-> chromadb ← Brain tier onlychromadb lives in package.brain.json; the Body tier this pipeline runs in has zero dependencies. Note this stage is the first collection stage, so it fails before the four DevIndex stages execute — clearing it may expose another. This ticket owns making the pipeline pass this stage, not a claim that it is the last one.
The Architectural Reality
Census: 21 files under ai/scripts/ and buildScripts/ import the barrel. Chasing them individually is whack-a-mole — one fixed, a second fired within hours, nineteen candidates remain.
And the narrow-import remedy does NOT transfer from #16474. Two differences, both measured:
ai/services.mjs:26 carries a side effect: GH_Config.data.syncOnStartup = false. The leaf default is already false (configBase.mjs:96, config.mjs:106), so this is not redundant — it is a forced override that guarantees false regardless of env or overlay, and SyncService.mjs:76 branches on it (if (aiConfig.syncOnStartup)). A direct import silently drops that guarantee, and an overlay setting it true would make this script auto-sync on startup. That is a behaviour change disguised as an import cleanup.
makeSafe is again a no-op for these two calls only — emitGeneratedContentAndDerive and runFullSync appear in no operationId of mcp/server/github-workflow/openapi.yaml, so the wrapper cannot be validating or marshalling them. This must be re-checked per call site and never assumed; #16474's call took no arguments, this one takes an object.
The Fix
Corrected before implementation. This section first prescribed narrowing the import at the call site, mirroring #16474. That fix is architecturally wrong here and must not be applied — the reasoning is below, kept visible because the wrong version is the intuitive one and the next reader will reach for it too.
Narrowing the import fights a documented architectural boundary. syncGithubWorkflow.mjs:35-40 states the policy explicitly — imports route through ai/services.mjs as "the canonical SDK entry point", which "handles Neo namespace bootstrap + auto-disables sync-on-startup side-effects + applies Zod validation at the service boundary", and closes with "No direct ai/mcp/server/... or ai/services/... deep imports." That is not local prose: learn/agentos/v13-path.md names ai/services.mjs as the SDK boundary and the v13 direction, learn/benefits/ArchitectureOverview.md:437 records the post-M6 (#10986) lift into it, and restore.mjs, downloadKnowledgeBase.mjs, uploadKnowledgeBase.mjs and defragChromaDB.mjs each restate "canonical SDK boundary only".
So the barrel import is deliberate, and 21 sites depend on that guarantee. Twenty-one narrow imports would be twenty-one boundary violations.
The defect is one layer deeper: the service graph imports chromadb at module scope, so merely LOADING it requires a Brain-tier package that is only needed when a connection is opened. Fixing that fixes all 21 consumers at once, changes no export surface, and leaves the SDK boundary intact.
Second correction, and the reason this ticket is not a quick one. This section briefly claimed the fix was one line in the KB manager. Measured, the population is three module-scope imports, and the two that matter are behind a synchronous constructor. Recorded because "one line" was wrong in the direction that makes a ticket get picked up as trivial.
module-scope chromadb import |
where the symbol is used |
deferrable as-is? |
services/knowledge-base/ChromaManager.mjs:1 |
:78, inside synchronous construct() |
No — needs client construction moved to initAsync() |
services/memory-core/managers/ChromaManager.mjs:1 |
same shape |
No — same |
services/shared/vector/chromaClientPrimitives.mjs:1 |
:114-115, inside a function |
Yes, cleanly — and both managers import it |
Why the two managers are the hard part. construct() is synchronous, so await import() cannot live there; the client has to be built in initAsync(), which already awaits connect() immediately after. That changes when .client exists, and .client is not private:
daemons/orchestrator/Orchestrator.mjs:534 passes ChromaManager.client onward
services/knowledge-base/VectorService.mjs (3 sites), HealthService.mjs:175, examples/smart-search.mjs:27
- dozens of specs mock-replace
ChromaManager.client = fake — chromaClientPrimitives.mjs:211 documents ">14 occurrences" as a deliberate seam
So this is a real initialization refactor of two core managers with a wide reader set, not an import tidy. It is the right fix and it should be reviewed as what it is.
Consequence for sequencing — stated plainly rather than assumed away. There is no clean narrow unblock: the obvious one (deep-import at the call site) violates the SDK boundary, and the correct one is this refactor. Whoever schedules this should decide knowingly whether the pipeline stays red while it is done properly, or whether a temporary, explicitly-marked boundary exception is preferable. That is a maintainer call and this ticket does not take it.
| Target Surface |
Source of Authority |
Proposed Behavior |
Fallback / Error Semantics |
Docs |
Evidence |
ChromaManager chromadb import |
this ticket |
Deferred to where a client is actually created |
Absent package fails when connecting, not when importing |
module JSDoc |
Body-tier import walk reaches no chromadb |
| client construction site |
ChromaManager:73-79 |
Moves from construct() to initAsync() |
initAsync already awaits connect() immediately after |
module JSDoc |
KB integration coverage |
ai/services.mjs barrel |
v13-path.md |
Unchanged — remains the sole entry point |
n/a |
n/a |
no consumer diff |
The one risk, and it must be witnessed rather than assumed: construct() is synchronous, so the client cannot be built with await import() there. Moving construction into initAsync() is safe only if nothing reads this.client between construct and initAsync completing. Neo's core.Base runs initAsync as part of singleton setup and consumers are supposed to await ready() — but "supposed to" is a claim, and this ticket owns proving it for every reader of this.client.
Acceptance Criteria
Out of Scope
- Restructuring or splitting
ai/services.mjs. The barrel stays the single SDK entry point and its export surface does not change. Deferring one leaf import is what makes a split unnecessary — this was originally scoped here as "the class fix", before the smaller and more correct one was found.
- Auditing the other 20 barrel importers individually. If the deferred import is correct they are fixed by construction, and a per-site sweep would only re-confirm the class assertion above.
#16474's narrow import in buildScripts/docs/index/labels.mjs. It is merged, correct in outcome, and unblocked a live pipeline — but it deep-imports past the SDK boundary, and once this lands it is no longer necessary. Whether it should be reverted to a barrel import is a separate call, flagged rather than assumed here.
- The four DevIndex stages. They have not executed since the failure precedes them; if one also imports the barrel it is the same class and gets the same treatment.
#16449. It consumes the refreshed snapshot; it does not own the pipeline.
Avoided Traps
- Assuming
makeSafe is a no-op because it was last time. It was verified per call site; the previous site passed no arguments and this one passes an object.
- Dropping
syncOnStartup = false as redundant. It duplicates the leaf default by value and not by guarantee.
- Declaring the pipeline fixed on a green local run. Only a scheduled run postdating the merge is evidence, and the previous fix was reported as unverified for exactly this reason — correctly, as it turned out.
Related
#16474 / PR #16475 — the first entry point, merged and correct but insufficient · #16428 — the standing alarm (auto-maintained; not a close target) · #16449 — blocked on the snapshot this refreshes · #16364 / #16389 — the two-tier install split
Origin Session ID: eeacb603-97f1-4241-9b2f-3a542cab6d2c
Retrieval Hint: query_raw_memories("DataSync chromadb eager services barrel second entry point syncOnStartup forced override makeSafe per call site")
Context
#16474/ PR#16475fixed one Body-tier entry into the eagerai/services.mjsbarrel —buildScripts/docs/index/labels.mjs, the "content indexes and SEO" stage. That fix was correct and is merged.It was not sufficient, and PR
#16475said so in advance: "If it does not go green, the chromadb import was one cause and not the only one — this fixes what run 30861274995 shows, which is not proof it is the sole failure." Two runs now postdate that merge and both failed:#16428stands at 12 consecutive failures. This also blocks#16449defect 3, whose guard reads a snapshot this pipeline is supposed to refresh.The Problem
Same error, earlier stage:
Static import walk from the stage entry — 259 modules, positive control from the previously-failing path:
ai/scripts/maintenance/syncGithubWorkflow.mjs:1 -> ai/services.mjs ← 66-import EAGER barrel -> ai/services/knowledge-base/DatabaseService.mjs -> ai/services/knowledge-base/ChromaManager.mjs -> chromadb ← Brain tier onlychromadblives inpackage.brain.json; the Body tier this pipeline runs in has zerodependencies. Note this stage is the first collection stage, so it fails before the four DevIndex stages execute — clearing it may expose another. This ticket owns making the pipeline pass this stage, not a claim that it is the last one.The Architectural Reality
Census: 21 files under
ai/scripts/andbuildScripts/import the barrel. Chasing them individually is whack-a-mole — one fixed, a second fired within hours, nineteen candidates remain.And the narrow-import remedy does NOT transfer from
#16474. Two differences, both measured:ai/services.mjs:26carries a side effect:GH_Config.data.syncOnStartup = false. The leaf default is alreadyfalse(configBase.mjs:96,config.mjs:106), so this is not redundant — it is a forced override that guarantees false regardless of env or overlay, andSyncService.mjs:76branches on it (if (aiConfig.syncOnStartup)). A direct import silently drops that guarantee, and an overlay setting it true would make this script auto-sync on startup. That is a behaviour change disguised as an import cleanup.makeSafeis again a no-op for these two calls only —emitGeneratedContentAndDeriveandrunFullSyncappear in nooperationIdofmcp/server/github-workflow/openapi.yaml, so the wrapper cannot be validating or marshalling them. This must be re-checked per call site and never assumed;#16474's call took no arguments, this one takes an object.The Fix
Narrowing the import fights a documented architectural boundary.
syncGithubWorkflow.mjs:35-40states the policy explicitly — imports route throughai/services.mjsas "the canonical SDK entry point", which "handles Neo namespace bootstrap + auto-disables sync-on-startup side-effects + applies Zod validation at the service boundary", and closes with "No directai/mcp/server/...orai/services/...deep imports." That is not local prose:learn/agentos/v13-path.mdnamesai/services.mjsas the SDK boundary and the v13 direction,learn/benefits/ArchitectureOverview.md:437records the post-M6 (#10986) lift into it, andrestore.mjs,downloadKnowledgeBase.mjs,uploadKnowledgeBase.mjsanddefragChromaDB.mjseach restate "canonical SDK boundary only".So the barrel import is deliberate, and 21 sites depend on that guarantee. Twenty-one narrow imports would be twenty-one boundary violations.
The defect is one layer deeper: the service graph imports
chromadbat module scope, so merely LOADING it requires a Brain-tier package that is only needed when a connection is opened. Fixing that fixes all 21 consumers at once, changes no export surface, and leaves the SDK boundary intact.chromadbimportservices/knowledge-base/ChromaManager.mjs:1:78, inside synchronousconstruct()initAsync()services/memory-core/managers/ChromaManager.mjs:1services/shared/vector/chromaClientPrimitives.mjs:1:114-115, inside a functionWhy the two managers are the hard part.
construct()is synchronous, soawait import()cannot live there; the client has to be built ininitAsync(), which already awaitsconnect()immediately after. That changes when.clientexists, and.clientis not private:daemons/orchestrator/Orchestrator.mjs:534passesChromaManager.clientonwardservices/knowledge-base/VectorService.mjs(3 sites),HealthService.mjs:175,examples/smart-search.mjs:27ChromaManager.client = fake—chromaClientPrimitives.mjs:211documents ">14 occurrences" as a deliberate seamSo this is a real initialization refactor of two core managers with a wide reader set, not an import tidy. It is the right fix and it should be reviewed as what it is.
Consequence for sequencing — stated plainly rather than assumed away. There is no clean narrow unblock: the obvious one (deep-import at the call site) violates the SDK boundary, and the correct one is this refactor. Whoever schedules this should decide knowingly whether the pipeline stays red while it is done properly, or whether a temporary, explicitly-marked boundary exception is preferable. That is a maintainer call and this ticket does not take it.
ChromaManagerchromadbimportchromadbChromaManager:73-79construct()toinitAsync()initAsyncalready awaitsconnect()immediately afterai/services.mjsbarrelv13-path.mdThe one risk, and it must be witnessed rather than assumed:
construct()is synchronous, so the client cannot be built withawait import()there. Moving construction intoinitAsync()is safe only if nothing readsthis.clientbetweenconstructandinitAsynccompleting. Neo'score.BaserunsinitAsyncas part of singleton setup and consumers are supposed to awaitready()— but "supposed to" is a claim, and this ticket owns proving it for every reader ofthis.client.Acceptance Criteria
syncGithubWorkflow.mjsreaches nochromadb, with the pre-fix walk as the positive control.ai/services.mjsitself reaches nochromadb— the class assertion. Per-entry-point walks only prove the entry points someone enumerated.chromadbresolvable) gets past module resolution, with the pre-fix run reproducing the exact CI error as control..clienton BOTH managers is enumerated from source and shown to run afterinitAsync— measured at ~20 production/example sites plus dozens of specs that mock-replace it as a documented seam. This is the move's real risk and the reason it is not a quick fix.memory-core/managers/ChromaManager.mjsreachingchromadbon the same barrel path — the identical mistake as fixing one entry point and declaring the class closed.ai/services.mjschanges — the SDK boundary is preserved, not worked around.#16428alarm is not claimed as resolved by this ticket — it self-closes on a green run, and only a run postdating the fix can say so.Out of Scope
ai/services.mjs. The barrel stays the single SDK entry point and its export surface does not change. Deferring one leaf import is what makes a split unnecessary — this was originally scoped here as "the class fix", before the smaller and more correct one was found.#16474's narrow import inbuildScripts/docs/index/labels.mjs. It is merged, correct in outcome, and unblocked a live pipeline — but it deep-imports past the SDK boundary, and once this lands it is no longer necessary. Whether it should be reverted to a barrel import is a separate call, flagged rather than assumed here.#16449. It consumes the refreshed snapshot; it does not own the pipeline.Avoided Traps
makeSafeis a no-op because it was last time. It was verified per call site; the previous site passed no arguments and this one passes an object.syncOnStartup = falseas redundant. It duplicates the leaf default by value and not by guarantee.Related
#16474/ PR#16475— the first entry point, merged and correct but insufficient ·#16428— the standing alarm (auto-maintained; not a close target) ·#16449— blocked on the snapshot this refreshes ·#16364/#16389— the two-tier install splitOrigin Session ID: eeacb603-97f1-4241-9b2f-3a542cab6d2c
Retrieval Hint:
query_raw_memories("DataSync chromadb eager services barrel second entry point syncOnStartup forced override makeSafe per call site")