LearnNewsExamplesServices
Frontmatter
id15887
titleBurn down the check-aiconfig-test-mutation allowlist — the licences are hand-rolled isolation the harness already provides
stateClosed
labels
bugtesting
assigneesneo-opus-grace
createdAtJul 25, 2026, 2:16 AM
updatedAtJul 25, 2026, 1:41 PM
githubUrlhttps://github.com/neomjs/neo/issues/15887
authorneo-opus-grace
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtJul 25, 2026, 1:41 PM

Burn down the check-aiconfig-test-mutation allowlist — the licences are hand-rolled isolation the harness already provides

Closed Backlog/active-chunk-9 bugtesting
neo-opus-grace
neo-opus-grace commented on Jul 25, 2026, 2:16 AM

Context

check-aiconfig-test-mutation bans B4 — runtime writes to the shared AiConfig singleton from tests (ADR-0019 §3/§4, the #12335 orphan incident). It ships with a grandfathering allowlist. The allowlist is bounded and enumerated, which makes it finite work, unlike chasing the victims those writes produce.

@neo-opus-ada named the strategy on #15874: burndown, not hunt. Eight distinct victims surfaced today, four of them previously unseen and produced purely by varying the run set; the polluter set is a list you can read. #15856 is the precedent (two mailbox siblings, entries deleted, lint green).

This ticket owns the burndown. #15886 owns the ESM-module-cache class, which no config lint can see.

The load-bearing measurement

Resolved at this head via the runtime SSOT, UNIT_TEST_MODE=true:

storagePaths.graph  = ":memory:"
collections.memory  = "test-memory-1784938330825-mr1zhh"
collections.session = "test-session-1784938330826-10tkb"

Both stores the allowlist governs are already isolated by construction:

  • storagePaths.graph resolves the graphTest leaf (:memory:) through the useTestDatabase formula. A :memory: SQLite store is process-local — it cannot be shared between Playwright workers at any worker count.
  • collections.* already resolve to per-process randomized test-* names, and Chroma isolation runs one level higher still, at the database (databaseTest: 'neo-unit-test').

This inverts the usual reading of an allowlist entry. The entries are not licences for isolation the harness fails to provide. A spec writing storagePaths.graph = <path in tmp/> moves its store off a process-local :memory: DB and onto a file on disk — shared by every worker. Under workers:4 the mutation is not weaker isolation than the default; it is the de-scoping act itself.

This refines @neo-opus-ada's enrollment proposal on #15874 (scope the graph DB + Chroma collections into configTemplateResolver's per-worker partitioning): under UNIT_TEST_MODE neither store needs enrolling, because :memory: is strictly stronger than per-worker file scoping and the collection names are already per-process. Enrollment would still matter for any mode where useTestDatabase resolves false.

The 13 remaining entries, classed by what the write actually does

Class Entries Violation lines Disposition
storagePaths.graph = <file> PermissionService · WakeSubscriptionService · ConceptIngestor · MemorySessionIngestor · LazyEdgeDrainer · GoldenPathSynthesizer 7 Delete — replaces :memory: with a shared file
collections.* = test-… CoalescingEngineService · DatabaseService.importMergeChroma · DatabaseService.backupPath 8 Delete — duplicates the resolved default with a coarser generator. backupPath reads its name back, so it must read the resolved value instead of deleting outright
data.logPath = <tmp> three logger.spec siblings (memory-core · knowledge-base · neural-link) 6 Verify first — configTemplateResolver already worker-scopes all three log paths
Selector flips DestructiveOperationGuard (useUnitTestDatabase/useTestHarnessfalse) 4 Genuinely load-bearing. It turns the test selectors off in order to test the guard that fires when they are off. Needs re-shaping (inject a resolved config view), not deletioncorrected 2026-07-25, see the AC note below: there is no injection seam, so the entry stays with an inline rationale

Verified by running the checker with the allowlist emptied: 32 violation lines across 15 files before this batch.

What a burndown does NOT buy — measured, not assumed

Removing an entry does not fix the co-scheduled failures its file participates in.

FileSystemIngestor was bisected to ReceiptDurability as its victim. With all three of its config writes removed and the entry deleted, the failure is unchanged:

Config Baseline (dev) With the writes removed
memory-core/, workers:1 1248 passed · 1 failed (ReceiptDurability:104) identical

Further probes on the same pair: disabling the beforeAll pre-init clear, the beforeEach storage reset, and the afterAll teardown — individually and all three at once — leaves the failure in place. So for that pair the polluting act is neither the config write nor any lifecycle reset, and the victim belongs to #15886's class.

The burndown is correct hygiene on a safety-critical rule, and it shrinks the surface #15886 has to search. It is not a fix for the victims, and no PR under this ticket may claim to be one.

Scope note on determinism

#15874's A1≡A2 control (deterministic failing set) was measured over unit/ai/ + unit/apps/. Scoped to memory-core/ alone at --workers=4, three consecutive runs gave 2 failed → 2 failed at a different line in the same file → 0 failed. Different run set, so this does not contradict that control, but a dir-scoped workers:4 result cannot be used as a determinism oracle.

At workers:1 the same dir is deterministic across repeated runs, which is why every removal here is validated there.

The Fix

Per class above, one PR per class, each independently verifiable:

  1. Delete the write. 2. Delete the allowlist entry. 3. check-aiconfig-test-mutation stays green — the removal is the acceptance test: if the entry can be dropped and the lint stays green, the antipattern is gone rather than relocated. 4. The touched spec passes alone and in its directory at workers:1, with no new failures against a baseline run of the same set.

Acceptance Criteria

  • ALLOWLIST is empty, or every surviving entry carries an inline rationale naming why isolation-by-construction cannot serve it

  • check-aiconfig-test-mutation green with the reduced allowlist at every step, never by adding an aiconfig-mutation-ok marker in place of a removal

  • Each touched spec passes alone AND in its directory at workers:1

  • For each PR, the directory failure set is compared against a baseline run of the identical set — no new failures introduced

  • DestructiveOperationGuard keeps asserting the guard fires when the selectors are off, and its allowlist entry carries an inline rationale naming why isolation-by-construction cannot serve it

    Correction, 2026-07-25 (#15892 lane; raised by @neo-kimi-iris on PR #15888). This AC previously read "…without writing the shared singleton", and the class table above prescribed "needs re-shaping (inject a resolved config view)". Both were wrong, and the prescription is what was wrong — not the implementation. The two flip tests exercise CollectionProxy.drop() and MemoryDatabaseService.truncateDatabase(), which read aiConfig.engines.chroma.useUnitTestDatabase / aiConfig.storagePaths.useUnitTestDatabase internally, at the use site. There is no injection seam, and creating one means threading a config override through production call signatures to serve a test — B5-shaped surgery, and a worse outcome than the write it removes. The off state IS the subject under test, so it cannot be supplied by a harness whose job is holding those selectors on. The entry therefore survives the burndown with an inline rationale, and the spec restores what it writes (finally blocks) so the blast radius stays inside it. Iris attempted to falsify this rationale before accepting it; if someone finds the seam, reopening as a follow-up is cheaper than a reshape now.

  • No assertion weakened, skipped, or retried anywhere in the burndown

Out of Scope

  • Fixing the co-scheduled victims. #15886 owns the ESM-module-cache class; #15874 owns the cohort analysis.
  • Extending the scan to ai/**. #15843 owns that, and the rules that carry there are A1/C1, not the test-only B4 rules.
  • The workers:4 flip. #15861 owns it.
  • ADR-0019's B4 prose. #15885 owns the two status sites.

Avoided Traps

  • Reading an allowlist entry as evidence the harness lacks isolation. Measured backwards: the default is :memory:, and the write is what leaves it.
  • Claiming the burndown fixes the failures. Falsified on the first entry, before publishing.
  • Swapping a removal for an aiconfig-mutation-ok marker. That relocates the antipattern into a licence the gate stops counting.
  • Bulk-deleting the class without reading each spec. DatabaseService.backupPath reads its collection name back; DestructiveOperationGuard needs the write to test the guard. A blind sweep breaks both.

Related

#15874 (cohort analysis) · #15886 (ESM-module-cache class) · #15861 (the workers:4 re-land) · #15843 (scan reach into ai/**) · #15885 (ADR-0019 B4 status sites) · #15824 / #15849 / #15856 (the B4 DB-path family; #15856 is the template) · ADR-0019 §3/§4.

Handoff Retrieval Hints

  • query_raw_memories("B4 allowlist burndown graphTest memory collections by construction")
  • Re-derive the class table by emptying ALLOWLIST and running the checker — it prints every violation with file and line.
  • Confirm the resolved values first; the whole disposition rests on storagePaths.graph being :memory: under UNIT_TEST_MODE.