LearnNewsExamplesServices
Frontmatter
id17188
titleA redundant spin-gated assertion on a pair already joined upstream
stateClosed
labels
bugaitesting
assigneesneo-opus-ada
createdAtAug 15, 2026, 5:47 PM
updatedAtAug 15, 2026, 9:29 PM
githubUrlhttps://github.com/neomjs/neo/issues/17188
authorneo-opus-ada
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 15, 2026, 9:29 PM

A redundant spin-gated assertion on a pair already joined upstream

Closed Backlog/active-chunk-16 bugaitesting
neo-opus-ada
neo-opus-ada commented on Aug 15, 2026, 5:47 PM

⚠️ Scope narrowed by its author, 2026-08-15 — and the original claim was wrong

This ticket originally covered BOTH turn < 3 spins in MailboxService.spec.mjs and claimed they "pass vacuously". Two experiments and one review later, neither half of that survived:

  1. The vacuous-pass claim is false. These tests catch the defect they guard with the turn budget set to zero — detection never lived in the spin. See Superseded original claim at the bottom.
  2. The two spins are not the same case. @neo-gpt's review of PR #17198 established at source that the same-target pair is joined upstream while the global+explicit pair diverges. Only the first is fixable here.

Scope is now the same-target pair only. The divergent pair is #17204, which needs a production seam this ticket places out of scope.

Context

While repairing MailboxService.spec.mjs:1381 for #17186, I dispositioned its two turn < 3 siblings as that ticket's AC-4 requires. The disposition took three passes and each one corrected the last, which is worth recording because the corrections are the content.

The Problem

test/playwright/unit/ai/services/memory-core/MailboxService.spec.mjs:1224"concurrent same-id repairs await one projection mutation (#16767)"

await readEntered;
for (let turn = 0; turn < 3; turn++) {
    await new Promise(resolve => setImmediate(resolve));
}
expect(payloadReads).toBe(1);        // ← pre-release, spin-gated
releasePayloadRead();

const results = await Promise.all([first, second]);
expect(projectCalls).toBe(1);
expect(payloadReads, 'record loading and mutation must share the single-flight pipeline').toBe(1);

Two independent reasons the spin and its assertion do nothing.

1. The assertion is subsumed. "Only one read so far" is strictly weaker than "only one read in total", which the post-completion assertion already checks deterministically after both callers settle.

2. The two callers cannot diverge in the first place. Both pass the same target rather than ids, so both route through getMailboxGraphProjectionRepairCandidates() — whose coalescing promise is process-wide rather than keyed (MailboxService.mjs:1573) — and the id path is the only one that skips it (:2705, idFilter ? null : await …). They are joined upstream of the segment load. Caller two cannot reach its decision independently of caller one, so the turn budget has nothing to synchronise.

Measured, twice:

experiment result
turn budget 0, single-flight intact passes — caller two joins with no wait at all
turn budget 0, single-flight disabled fails at the post-completion assertion, Expected: 1 / Received: 2

The second run is decisive: with the wait removed entirely, the defect is still caught.

The Fix

Delete the pre-release assertion and the turn budget. Both are inert for this pair, and the comment is replaced with the upstream-join reason so the next reader does not re-add a wait to "make it safe".

Acceptance Criteria

  • The pre-release expect(payloadReads).toBe(1) at :1227 and its turn budget are removed
  • Sensitivity: with the single-flight forced to never reuse its pending promise, the test still fails
  • Specificity: correct production passes across 20 consecutive runs, 0 failures — the property a red proof cannot establish
  • Both proved individuallymode: 'serial' at line 25 makes a combined run report 1 did not run for the second test
  • The retained comment names the upstream join as the reason no wait is needed, not "the wait was redundant"

Out of Scope

  • The global+explicit pair (:1298) — #17204. Those two diverge before the join, so a legitimately late caller two can false-red the assertion on correct production, and no test-side anchor can fix it: a joining caller produces no observable, and getMessageWalCandidateSegmentLoad is module-private. That needs a production seam.
  • :1381 — repaired in #17186.
  • Production behaviour. Unchanged here; the seam question belongs to #17204.
  • The guard's blindness to turn-budget spins. check-fixed-sleeps.mjs matches setTimeout at or above 1000 ms, so a spin waiting in event-loop turns is invisible to it — related to #17177's callback-form blind spot.

Avoided Traps

  • My own first analysis. I inferred the failure mode from the assertion's shape — a non-event assertion inverts, therefore it must pass vacuously — and filed on it. One experiment settled it. A defect class that fits is not a defect that is present.
  • Treating the two spins as one case. They look identical and are not; the difference is one argument (target vs ids) routing through a different branch three call levels down.
  • Reading a red proof as sufficient. Sensitivity and specificity are different properties. The first version proved only sensitivity and reasoned as though it had both.
  • Deleting the wait for the divergent pair too. Inert here, load-bearing there — same-looking code, opposite disposition.

Evidence class

L2 — reproduced in-process by mutating getMessageWalCandidateSegmentLoad to never reuse its pending promise, with the turn budget at 0, each test run individually at --workers=1; specificity measured over 20 consecutive runs.

Related

#17186 (the :1381 repair whose AC-4 this discharges) · #17204 (the divergent pair, split from this) · #15861 (the workers: 4 re-land) · PR #17198 · PR #17183 · #17177

Handoff Retrieval Hints

  • query_raw_memories("MailboxService same-target spin upstream join process-wide coalescing promise inert")
  • Falsification recipe: set pending = undefined in getMessageWalCandidateSegmentLoad, set the turn budget to 0, run the test individually. It must still fail Received: 2 — that is what proves the wait was not carrying detection.

Superseded original claim (retained, do not act on)

Both spins assert a non-event, which inverts the failure mode: a short budget makes :1381 fail loudly and makes :1224/:1298 pass vacuously, silently ceasing to test anything. Measured at one turn of headroom (second read at turn 2, budget 3) against a >25× contention multiplier.

Why it was wrong, in two stages.

Stage one (mine). The headroom measurement was real; the conclusion drawn from it was not. I measured when caller two's read lands and treated the pre-release assertion as carrying the test's detection. It does not — the post-completion assertion does, deterministically and independently of the budget. The experiment that settles it (budget 0 + broken single-flight → still fails) takes one run, and I filed before running it.

Stage two (@neo-gpt's review). Even the corrected version treated both spins as one case. They are not: same-target callers are joined upstream and their wait is inert, while global+explicit callers diverge and their wait is the only thing narrowing a real false-RED window. The corrected-but-still-wrong version would have removed a load-bearing wait from the divergent pair on the strength of a measurement taken on the coupled one.

Origin Session ID: 00348bc3-c011-4035-90a3-f0eb62b8c95c

tobiu referenced in commit d3761c5 - "fix(test): delete two redundant spin-gated assertions (#17188) (#17198) on Aug 15, 2026, 9:29 PM
tobiu closed this issue on Aug 15, 2026, 9:29 PM