Skip to content

round-16: No cell distinguishes the #164 fenced evict from an unconditional evict #173

Description

@roninjin10

Severity: p2
Area: engine-store
Slug: corruption-evict-fence-direction-untested

Claim

The new corruption-branch evict passes ifRecordedBy from the poisoned row so a concurrent fresh row survives as a CAS no-op — but every cell reaching this branch has no competing row, so replacing the fenced call with an unconditional cache.evict(keyDigest) keeps all three #164 cells green while the branch would start deleting valid sibling-run evidence.

Evidence

  • packages/engine-store/src/internal/ActivityPersistence.ts:499-504 — the fenced evict (ifRecordedBy from cached.value.recordedRunId/recordedEventSeq), justified by the round-15: Cache-hit corruption never evicts the poisoned row #164/round-9: #110 eviction guard is read-then-unfenced-DELETE, and the acknowledged residual race has no ticket #119 comment at 484-498.
  • grep for ifRecordedBy over packages/engine-store/test/ReplayCorruptionClassification.test.ts returns nothing.
  • The only ifRecordedBy no-op/CAS assertions anywhere: packages/engine-store/test/CacheProvenanceGeneration.test.ts:119-142 (a laggard evict issued by the test itself against the stale_read_set generation, asserting laggardDeleted === false / survivorPresent === true) and packages/journal/test/CacheStore.test.ts:121-145 (unit-level CacheStore CAS). Neither drives the corruption branch.
  • The corruption tests never land a competing fresh row: ReplayCorruptionClassification.test.ts:229-256 evicts unconditionally from the test before re-dispatch; the strict/tolerant cells at 109-182 have a single recorded row.

Failure scenario

A refactor drops the fence (or a future edit makes it unconditional). The corruption branch then deletes a fresh sibling-run cache row while quarantining stale poisoned evidence, destroying valid evidence — and the suite stays fully green.

Verifier confirmation

Mutation-tested directly. Replaced the fenced evict at 499-504 with an unconditional cache.evict(keyDigest) and ran npx vitest run test/ReplayCorruptionClassification.test.ts → 12/12 passed, and the full engine-store suite → 59 files / 330 tests all passed. No cell anywhere in engine-store constrains the fence direction on this branch; the mutant fully survives. Source file restored to original (git diff clean). Caveat: verified within packages/engine-store; the cross-package ifRecordedBy grep found no covering cell elsewhere either.

Missing test

After the corrupt row is observed, land a fresh row with different provenance before the evict, then assert Option.isSome(cache.get(keyDigest)) and that the survivor is the fresh row.

Found by the iterate-until-dry loop, round 15 review.

Metadata

Metadata

Assignees

No one assigned

    Labels

    second-reviewsecond-pass review finding

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions