LearnNewsExamplesServices
Frontmatter
id16617
titleThree unit specs read live plane state, so their verdict tracks corpus fill, not the diff
stateOpen
labels
bugaitestingagent-os
assigneesneo-opus-vega
createdAtAug 7, 2026, 10:22 AM
updatedAtAug 24, 2026, 6:54 PM
githubUrlhttps://github.com/neomjs/neo/issues/16617
authorneo-opus-vega
commentsCount2
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]

Three unit specs read live plane state, so their verdict tracks corpus fill, not the diff

Open Backlog/active-chunk-13 bugaitestingagent-os
neo-opus-vega
neo-opus-vega commented on Aug 7, 2026, 10:22 AM

Rewritten 2026-08-08T10:05Z — current facts only. Prior body was 20,039 bytes, mostly a withdrawn worktree-default argument and its correction trail. That exchange is in the comments and in @neo-opus-ada's A2A; it is not scope.

unit/ specs whose verdict does not track the diff. The count started at three and is now two — the third was investigated and the claim did not survive contact with the source.

The instances

spec defect
SearchService.noModel.spec.mjs:36 live-plane coupled — asserted against the canonical KB collection's live row count. Fixed.
scripts/maintenance/backup.spec.mjs:104 NOT coupled — vacuous instead. The graph subsystem cannot be reached from a host run at all, so the 5-subfolder assertion passes on existence while nothing is exported. Different defect, opposite fix (see below).
same file a failure in a beforeAll (:55, :784, :928) aborts sibling tests in its describe block — unaffected by the above, still open
services/memory-core/HealthService.spec.mjs:1463 live-plane coupled, and it fires on CI rather than locally. Added 2026-08-19 — a third instance, found from the other end: a red pipeline, not a sweep. FIXED in 6a2ec2fca0 (rides PR #17372).

Instance 3 — HealthService.spec.mjs:1463 (#13458: collection count timeout…)

The coupling. The test stubs StorageRouter.getMemoryCollection to hang, then calls healthcheck({chromaProbeTimeoutMs: 5}). But #checkDatabaseConnections probes three collections — memory, summary, temporalSummary — and only the first is stubbed. The other two issue real Chroma counts against a run-scoped store: playwright.config.unit.mjs declares chroma-setup as a project with teardown: chroma-teardown, so one Chroma serves the whole run and its contents depend on what ran before.

So the verdict tracks corpus fill, exactly as this ticket's title says — a 5 ms budget against a collection whose size is decided by test ordering.

The assertion compounds it into a hard failure rather than a soft one. result.details is an array, so expect(...).toContain(string) is exact-element equality, not the substring check the line reads as. The message joins per-collection errors with ; , so it passes only when exactly one collection times out:

Expected value: "Failed to access collections: memory collection count health probe timed out after 5ms"
Received array: ["Failed to access collections: memory collection count health probe timed out after 5ms;
                  temporal-summary collection count health probe timed out after 5ms"]

Why a worker retry does not heal it — and why that matters for triage. Chroma is run-scoped, not worker-scoped, so a retry gets a fresh worker and the same slow collection. Observed: red, then red again on retry, then green on a fresh run. A passing same-sha re-run therefore proves nothing here, because a leak and a flake are indistinguishable through repetition; the discriminator is scope, not repetition (@neo-opus-grace, PR #17372 review).

Reproduction attempts — all negative, recorded so nobody re-derives them:

scope result
the spec file alone, CI=1 122 passed (@neo-opus-grace)
traced blast radius, 7 files, CI worker count 1798 passed (@neo-opus-grace)
whole memory-core/ tree, one pool 1795 passed
entire unit suite, CI=1, 4 workers 14216 passed, 1 unrelated flaky

Local hardware answers the real counts inside 5 ms; a loaded CI runner does not. That the same local full-suite run flaked a different timing-sensitive spec (TextEmbeddingService.retry.spec.mjs:1038) is corroboration that this is a population, not one bad test.

Instance 3 — root cause and fix, landed

The root cause is narrower than "reads live plane state": one getter of three was missing from the harness. originals captured and beforeEach stubbed getMemoryCollection and getSummaryCollection. getTemporalSummaryCollection was neither captured nor stubbed — so it fell through to real Chroma on every test that reached the probe. It is also exactly the collection named in the CI error, which is what closed the diagnosis.

@neo-opus-grace's sharpening, which is better than my original framing: the probe is designed to report several collection failures in one joined string, and the assertion demanded exactly one. So the test asserts a shape the probe's own contract permits it to violate — ordering only decided whether it got away with it. That reframes the defect from order-sensitivity to a contract violation, and it explains the toContain choice: on a string that means substring, on an array it silently means exact equality, so a reasonable author could write that line and be wrong with nothing pointing at it until a second collection is slow.

Landed in 6a2ec2fca0: the third getter is captured, stubbed and restored beside its siblings, and the assertion is arrayContaining + stringContaining.

Verified in the direction that matters, not just the easy one. Making temporal-summary hang as well — reproducing the CI condition of two simultaneous timeouts — the test now passes where it previously failed. The isolation half means that condition can no longer arise from corpus fill at all, so the two fixes are belt and braces rather than one dressed as two.

Attribution, settled with @neo-opus-grace and recorded because it is the interesting part: the defect is the coupling — run-scoped shared Chroma plus a per-test millisecond budget measured against it — and not any one spec. It was exposed by two added tests in an unrelated directory perturbing file→worker assignment. Presence exposes; behaviour cannot cause. A latent defect that needs a perturbation to surface was always going to find one.

The fix, and it is not "raise the timeout". Stub all three collections so the test measures the timeout path rather than the corpus, and assert with expect.arrayContaining([expect.stringContaining(...)]) — or against details[0] — so a second timeout does not turn a correct behaviour into a red. Raising the budget would only move the fill level at which it fires.

⚠️ find test -name 'backup*.spec.mjs' returns five. This ticket means test/playwright/unit/ai/scripts/maintenance/backup.spec.mjs. I read the orchestrator/scheduling one first, found no live-plane reads, and nearly closed two live ACs as already-fixed.

⛔ BOTH of my mechanisms for this spec were wrong. The graph lives INSIDE DOCKER (@tobiu, 2026-08-08)

There is no live-plane coupling in backup.spec.mjs to remove. I claimed one twice and each claim was a layer of the same error — reasoning about "live plane state" without asking where the test runs versus where the data lives.

  • Claim 1 (original ticket): readdirSync over a live bundle root. FalsebeforeAll builds a per-pid temp workRoot, fakes the KB/MC collections, and removes it in afterAll.
  • Claim 2 (my first correction): the graph source reads the live plane. Also false. The Memory Core graph SQLite lives in the container at planeDataRoot = /app/.neo-ai-data. A host-run unit/ spec resolves a different planeDataRoot entirely and cannot reach the container's store — the same shape as the Chroma correction on this ticket, where reachability depended on an overlay port publication rather than on the code.
  • And isolation already exists above where I was working, for the third time on this ticket: configBase.mjs:229 declares graphProd and a sibling graphTest, and storagePaths.graph is a formula selecting between them on UNIT_TEST_MODE — described in-file as "safe-by-construction".

⛔ A THIRD wrong mechanism for this spec — the graph-vacuity claim did not survive execution (2026-08-24)

The claim removed here was: #exportGraph always takes its uninitialised-graph branch in a host run, so the graph/ folder is created while nothing is exported, and "the assertion has always been vacuous in unit runs."

Measured at origin/dev 4e6d8da73e, by running the spec rather than reading the branch:

subsystems.graph = {message: "Export complete. Exported ... 17 graph elements.", count: 17,
                    graph: {collection: "native-graph", backupFile: ".../graph-backup-....jsonl",
                            expected: 17, exported: 17, skipped: 0}}
meta.integrity   = [{kb, pass, 1/1}, {mc, pass, 2/2}, {graph, pass, 17/17}]

Graph exports, and reaches pass. It does not take the skip branch. The reason is in this body already: configBase.mjs:229 selects graphTest under UNIT_TEST_MODE via a formula this ticket itself called "safe-by-construction" — recorded two corrections ago, then reasoned past. The ⚠️ Bound the removed text carried ("the branch above is read, not run") was the right instinct and the run went the other way.

That is the third wrong mechanism for backup.spec.mjs on this ticket, after the two already recorded above. All three share one shape: reasoning about where data lives without running the thing that would say.

What actually survives, and it is smaller and still worth doing

The vacuity was real; it was on different subsystems.

runBackup creates seven bundle folders — kb, mc, graph, concepts, trajectories, mailbox, ledgers. The assertion looped over five, so mailbox and ledgers sat outside every assertion in the file. Both export nothing in this fixture:

mailbox = {copied: 0, note: "source not present: sent-to-cull.jsonl"}
ledgers = {copied: 0, healAttempts: {copied: 0, note: ...}, healEvents: {copied: 0, note: ...},
           recoveryRuns: {copied: 0, note: ...}}

So the AC's subject holds exactly as written — a subsystem that exports nothing passed unnoticed — and the fix is the same property assertion. Only the named subsystem changed.

Also found while sizing it: copyJsonlSource returns {copied: jsonlFiles.length} with no note for a source directory that exists but holds zero .jsonl files (backup.mjs:1452, warn-only). That is a genuine silent-zero path in production, and the new assertion fails on it.

The root cause, and why isolation rather than provisioning

KB's ChromaManager created its client with no database, so every connection landed on Chroma's default. Memory Core has isolated its Chroma writes all along; KB simply never did.

Precision on reachability (@tobiu, 2026-08-08 — my original framing was too broad). "Any unit/ spec could reach the live canonical collection" is not universally true, and stating it that way hid the real mechanism. Chroma runs inside Docker, and the base docker-compose.yml publishes no port for it — networks: only — so under that composition a host-run spec cannot reach it at all. The reachability came entirely from the local overlay: docker-compose.local-agent-os.yml:27-28 maps 127.0.0.1:8000:8000, and configBase.mjs:812 sets portProd: 8000. So on the canonical local Agent OS plane — the one we all run — a host spec resolving localhost:8000 + default_database did reach the containerised production store, which is how the SearchService verdict came to track corpus fill.

And isolation already existed one layer above where I was working. configBase.mjs:810-813 splits hostProd/hostTest and portProd: 8000 / portTest: 18180, so engines.chroma.useTestDatabase selects a different Chroma instance, not just a different database name. The database-level isolation this ticket adds is a second layer beneath that, not the only one.

The fix stands on the narrower and more honest justification: passing database explicitly removes the spec's dependence on which plane it happens to run on, which is what makes a verdict portable. That is a smaller claim than "it could hit production", and it is the true one.

The fix is isolation, not linking every worktree's data. Linking bends the environment to match the tests' bad assumption; isolation removes the assumption.

Acceptance criteria

  • SearchService.noModel.spec.mjs passes with the canonical KB collection both empty and populated — the fixture supplies the count, and a third arm covers an UNREADABLE collection (.catch(() => null) must not read as empty).

  • The KB ChromaManager consumes chromaTestIsolation as its Memory Core sibling does, so a unit/ spec resolves a per-worker database rather than the plane's default — one authority for test isolation, not a second beside it. Delivered in #16667 / PR #16666, merged. The ensure-guard shipped comparing against the resolved chromaDatabase rather than one toggle, after @neo-kimi-iris found that a single-toggle guard misses the template-resolver arm entirely.

  • backup.spec.mjs asserts that each subsystem EXPORTED, or explicitly names the skip. Split to #17711, delivered by PR #17709. Each recovery substrate must reach integrity pass with source/bundle parity above zero; each copy subsystem must have copied rows or carry the note naming its absent source. No count is pinned for graph — its size is corpus-dependent, so a number would re-introduce this ticket's headline defect; pass is unreachable at zero, which carries "exported, and completely" without naming a size. Red-proved by two mutations (silent copied: 0 -> mailbox: {"copied":0}; forced graph skip branch -> graph: exported nothing, or its skip is unnamed), and the replaced folder loop stayed green under the first.

  • A failure in backup.spec.mjs no longer aborts 42 sibling tests. Split to #17711, delivered by PR #17709. Cause was test.describe.configure({mode: 'serial'}) at FILE scope; only the orchestrator block earns it (it mutates the KB/MC singleton accessors across beforeAll/afterAll), while the other three import pure functions and scope fixtures to a pid+timestamp temp dir. Measured with an injected beforeAll throw — before: 1 failed / 42 did not run / 2 passed · after: 1 failed / 22 did not run / 22 passed — so twenty previously-aborted tests now run and pass under the identical failure.

    • ⚠️ Second clause of this AC was corrected, not satisfied, and it is worth saying which. As written it also demanded "a failure count without a did not run count for that file" — a different set from "sibling tests": the first is the 20 tests in unrelated blocks, the second is all 42. The second is unreachable while the orchestrator block stays serial, because Playwright's serial mode skips the remainder of its scope after ANY failure — the residual 22 are that block's own tests, which genuinely depend on the fixture that failed. Converting its hooks to beforeEach/afterEach was tried and reverted: the count stayed at 22 (the hook type is not what causes the skip) and it would have cost nineteen fixture rebuilds for no measured gain. Reaching zero would mean dropping the serial constraint, which exists for real singleton-race safety. I am recording the conflation rather than quietly meeting the weaker half.
  • Negative control: the full unit/ai/ suite is green on a machine with a populated plane AND on one with an empty plane. Both, because each known instance fails in only one of those states.

  • The residual guard is a RUNTIME guard, not a static check — measured 2026-08-24, and this AC's original instrument choice cannot see its own subject. Scan of all 1087 unit specs, classifying every new Database(...) / new BetterSqlite(...) call site by argument shape:

    argument shape count
    ':memory:' 38
    temp/pid-scoped literal 0
    variable — decided at resolution time 20
    other expression 7
    absolute literal path 0

    Two consequences, and together they retire the "static check" framing:

    1. A static text check has nothing to convict. The only shape it can decide — an absolute literal path — occurs zero times. Such a check would be green on day one, making it a ratchet (future-regression prevention) and never a repair. Shipping it as though it closed a gap would be a guard doing no work.
    2. The real residue is invisible to it. 20 call sites pass a variable, so the path is decided at resolution time. Isolation works by config selection, so what isolation cannot reach is precisely a path that never consults the config — and that is exactly the population static text cannot classify either. The two "cannots" name the same 20 sites. Therefore the guard must observe resolution: a harness-level assertion that every sqlite/Chroma handle opened during a unit/ run resolves to ':memory:' or inside that run's temp root. A ratchet-only static check on absolute literals may ride along as a cheap belt, but only if labelled as a ratchet with zero current findings. Why a token scan is not the fallback: grepping neo-ai-data / better-sqlite3 / absolute paths across unit/ returns 54 / 47 / 25 file-hits, and sampling shows they are legitimate — planeConfig.spec.mjs asserts on resolvePlaneDataRoot({rootDir: '/tmp/seat-x'}) where the string is the subject, queries.spec.mjs opens ':memory:', Database.spec.mjs legitimately exercises the graph DB. A token-scan guard would emit ~126 findings, nearly all false, which is worse than no guard.
  • The guard's enforcement reach is stated in its own output — which directories it scans and which argument shapes it can decide. The measurement above is why this clause earns its place rather than being hygiene: any static component is blind to the 20 resolution-time call sites, so a green run WILL be read as covering them unless the output says otherwise.

Method note that cost real time

Bare npx playwright test does not import configTemplateResolver, so NEO_TEST_CONFIG_TEMPLATES is unset and test-mode toggles read false. Run -c test/playwright/playwright.config.mjs, as CI does. Measured: chromaDatabase resolved to default_database under the bare invocation and to the per-worker test database under the real one.

Out of scope

  • The 10-row count-vs-export gap in the live host graph DB (294,347 counted / 294,337 exported, skipped 0 unreadable) — a real data-integrity question with its own owner. This ticket makes the test independent of it.
  • The worktree data-linking default. bootstrapWorktree.mjs --link-data already implements the right thing (symlink gitignored children, never the parent, never per-process pid dirs; blocklist not allowlist). The residual defect is that an unlinked worktree is silent about being unlinked — not that the default is wrong. My original framing here claimed otherwise and is withdrawn.

Related

Split 2026-08-24: the two backup.spec.mjs criteria above were split into leaf #17711 and delivered by PR #17709. Reason: they are jointly one-PR-deliverable, and a PR against this umbrella could only carry Refs — which keeps it in draft, and a draft is not reviewable. The leaf gives the work an honest close target; this ticket stays open for the populated/empty-plane negative control, the residual static check, and its enforcement-reach statement.

#16667 (the delivered KB-isolation half) · #16463 · #15798

Origin Session ID: 4141258c-36d3-4788-b0c2-ab3ebe0867be

Retrieval Hint: query_raw_memories("unit specs read live plane state, KB ChromaManager no database, chromaTestIsolation")

tobiu referenced in commit 8314d15 - "The KB Chroma client is isolated like its Memory Core sibling (#16617) (#16666) on Aug 8, 2026, 4:45 PM
tobiu referenced in commit 2d36ab1 - "feat(ai): the startup head's line ceiling is a named leaf, and its envelope agrees with its payload (#17371) (#17372) on Aug 19, 2026, 9:44 AM
tobiu referenced in commit c19333b - "test(backup): a bundle folder no longer stands in for the export that should fill it (#17711) (#17709) on Aug 24, 2026, 6:40 PM