# Issue — tests sharing the default `engine_state/` dir (reproducibility hazard)

Status: fix landed in two layers — per-test autouse fixture (recommended fix below,
landed post-2026-06-15) + session-scoped baseline (2026-07-22, module-scope escape);
nightly full lane is the final suite-wide confirmation. See 2026-07-22 addendum.
Date: 2026-06-15 (addendum 2026-07-22)
Relates: ADR-0146 (engine-state persistence), ADR-0219 (generation-dir checkpoint),
ADR-0220 (identity/provenance reconcile — surfaced the symptom)

## Symptom

During ADR-0220 (PR C) verification, `tests/test_achat.py::test_achat_returns_non_empty_surface`
emitted a spurious:

```
RuntimeWarning: engine identity continuity break: checkpoint was written under
819c4364d599… but this build computes c9e5968ab1fe… — the ratified identity
substrate (packs) changed while the engine was down.
```

It was **not** a regression from the identity split. Re-running with an isolated
state dir made it vanish:

```
CORE_ENGINE_STATE_DIR=$(mktemp -d)/es pytest tests/test_achat.py   # 11 passed, no warning
```

## Root cause

`ChatRuntime(...)` with no `engine_state_path` resolves its store to
`EngineStateStore(None).path == engine_state._DEFAULT_DIR`, which is
`$CORE_ENGINE_STATE_DIR` or the in-repo `engine_state/` dir
(`engine_state/__init__.py:52-56`). That directory is **process-wide shared
state**:

- On construction, a runtime READS it (`store.exists()` → `_load_engine_state`),
  so any test reading a **stale checkpoint** left by an earlier test reconciles
  it (ADR-0220) and can emit a phantom identity-continuity warning.
- Tests that run a turn / checkpoint WRITE a generation-dir checkpoint into the
  same shared dir (the observed pollution was leftover `gen-0583/` `gen-0584/` +
  `current`), so test ordering and prior runs leak into later tests.

This is a classic non-hermetic-test hazard: behaviour depends on what other
tests (or prior local runs) left in `engine_state/`.

## Scope (it is systemic, not a one-off)

In `tests/` at `main@eed20749`:

| Metric | Count |
|---|---|
| `ChatRuntime(...)` constructions | **469** |
| test files constructing it | **123** |
| constructions passing `engine_state_path` (isolated) | 74 |
| constructions passing `no_load_state` (ephemeral, no persist) | 52 |
| **constructions defaulting to the shared dir** | **~340** |

Most defaulting constructions are *victims* (they read the shared dir on
construction); a subset that checkpoint are also *polluters*. CI runs in a clean
checkout so the shared dir starts empty there — which is exactly why this hides:
it bites local runs and ordering-sensitive sessions, not the gate.

## Interim rule (apply now, in review)

Any **new or edited** test that constructs a `ChatRuntime` which loads or
persists runtime state MUST isolate it:

```python
ChatRuntime(config=..., engine_state_path=tmp_path / "engine_state")
# or, for an ephemeral runtime that must not touch persisted state:
ChatRuntime(config=..., no_load_state=True)
```

Subprocess / CLI tests (which re-import in a child process) must set
`CORE_ENGINE_STATE_DIR` in the child env instead (see
`tests/test_l10_always_on_daemon.py::test_real_sigterm_stops_the_daemon_cleanly`
for the pattern).

Do **not** add a bare `ChatRuntime()` that reads/writes the default dir.

## Recommended fix (deferred — needs its own validated PR)

A single root `tests/conftest.py` **autouse** fixture that isolates the default
engine-state dir per test, fixing all ~340 sites at once instead of editing each:

```python
@pytest.fixture(autouse=True)
def _isolate_default_engine_state(tmp_path, monkeypatch):
    import engine_state
    # _DEFAULT_DIR is bound at import, so monkeypatch the module attribute (not
    # just the env var) for in-process runtimes:
    monkeypatch.setattr(engine_state, "_DEFAULT_DIR", tmp_path / "engine_state")
    # ...and set the env var for subprocess/CLI tests that re-import:
    monkeypatch.setenv("CORE_ENGINE_STATE_DIR", str(tmp_path / "engine_state"))
```

Fixture requirements / acceptance criteria for that PR:

1. **Monkeypatch `engine_state._DEFAULT_DIR`** — it is import-time bound, so an
   env var alone does not redirect already-imported in-process runtimes.
2. **Also set `CORE_ENGINE_STATE_DIR`** (or the equivalent) so subprocess/CLI
   tests that re-import in a child process inherit the isolation.
3. **Preserve tests that intentionally verify default-dir behaviour** — opt them
   out via a marker (e.g. `@pytest.mark.uses_default_engine_state`) or an explicit
   override, rather than silently changing their meaning.
4. **Broad/full-suite comparison against the known baseline reds** — the fixture
   changes default behaviour for all 469 constructions, so the PR must run the
   full suite and confirm it surfaces no *new* failures beyond the documented
   ~31 pre-existing reds on `main` (`core test --suite full`). Any genuinely new
   failure is a hidden inter-test dependency to fix, not to mask.

## Why deferred, not bundled

The brief is safe and immediately useful (documents the hazard + the rule). The
fixture, though small in code, changes default behaviour suite-wide and so must
be validated against the full suite — a deliberate cost that belongs in its own
PR rather than riding on identity-doctrine or hygiene-doc work.

## 2026-07-22 addendum — module-scope escape found and closed (session baseline)

The recommended fixture above landed in the root `conftest.py`
(`_isolate_engine_state_default`) and closed the per-test layer. The weekly
audit (2026-07-22) then caught a **residual escape**: pytest sets up
higher-scoped fixtures before function-scoped ones, so a module-/session-scoped
fixture constructing a bare `ChatRuntime()` binds `engine_state._DEFAULT_DIR`
*before* the per-test patch exists.

**Observed evidence**: `tests/test_achat.py`'s module-scoped `runtime` fixture
loaded the live repo `engine_state/` during smoke runs (emitting the ADR-0157
revision-mismatch warning against the real checkpoint) and committed real
generations — the live store's `turn_count` advanced 14989→14990 and
`written_at_revision` was re-stamped to `f94dbd404575` by the suite itself.
Content pollution was nil in that instance (recognizers/candidates byte-identical
across the two test-written generations), but turn-count/lineage provenance was
not test-free.

**Fix**: `_isolate_engine_state_session_baseline` — a session-scoped autouse
fixture (set up before fixtures of any narrower scope) that points
`engine_state._DEFAULT_DIR` + `CORE_ENGINE_STATE_DIR` at a session temp dir.
The per-test fixture continues to provide per-test freshness on top of it; the
`uses_default_engine_state` marker still opts out of the per-test layer only
(default-dir *resolution* semantics are preserved; the live store stays
unreachable at every scope).

**Regression pin**: `tests/test_conftest_engine_state_isolation.py::`
`test_module_scoped_construction_cannot_bind_the_live_store` (req #4) —
constructs a default store from a module-scoped fixture and fails if it
resolves to the repo dir.

**Residual (ledgered, weekly audit T-list)**: the live store's historical
`turn_count` (14990) includes an unknown number of pre-fix test turns, and the
465 persisted discovery candidates predate the fix — a provenance review of
live-store contents is a separate, opt-in remediation ruling.