timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

test_the_suite_does_not_write_to_the_PRODUCTION_log uses global mutable state as its oracle — any concurrent session reddens it #822

New Issue
Closed
opened 2026-08-22 23:51:22 +02:00 by timothy · 2 comments
timothy commented 2026-08-22 23:51:22 +02:00
Owner
Copy Link
Copy Source

Found while running the suite during #807. Not a defect in #807's change; filed separately.

The race

scripts/tests/test_hook_fire_log.py::test_the_suite_does_not_write_to_the_PRODUCTION_log snapshots st_mtime_ns of ~/.cache/ersatztv/hook-fire/*.jsonl before and after running a hook, and asserts the two are equal.

That directory is the shared production hook-fire log that every live Claude Code session on this machine writes to. So any other session firing a hook inside that window flips an mtime and the test fails, reporting a test run modified the production hook-fire log — an accusation about the suite, when the writer was an unrelated process.

Observed: one red in a full-suite run, green in isolation immediately afterwards; independently corroborated during review, which found a file in that directory written 13 seconds earlier by another session.

This repo now routinely runs several sessions and worktree-isolated review agents at once, so this is a live and guaranteed race rather than a theoretical one.

Why it matters beyond the flake

The failure message is confidently wrong about the cause, which is the expensive part — it points the next reader at the test suite rather than at concurrency, and the honest diagnosis costs a re-run plus a directory listing to reach. A flake that misattributes is worse than one that just fails.

It also weakens a real guard: the invariant (the suite must not write to the production log) is worth keeping, and a test that cries wolf gets waved through, which is how ci.decisions-lifecycle-flake already has to be documented as "do nothing".

Options

  • Scope the assertion to files the test could plausibly have touched (e.g. only paths whose names the test's own fixture would generate), rather than the whole directory.
  • Or snapshot with an exclusive lock / a per-run subdirectory, so the oracle is not global mutable state.
  • Or have hooks under test write to a redirected XDG_CACHE_HOME, making the production path unreachable from the suite by construction — dedup by construction rather than by assertion, per docs/defect-shapes-773.md §4 detector C. This is probably the right one: it makes the property structural instead of observed.

Done-when

  • The test's oracle no longer depends on state other processes can write
  • Witnessed: the test stays green while another process writes to ~/.cache/ersatztv/hook-fire/
  • The invariant it guards (the suite does not write the production log) still fails when genuinely violated
  • Adversarial review passed
Found while running the suite during #807. Not a defect in #807's change; filed separately. ## The race `scripts/tests/test_hook_fire_log.py::test_the_suite_does_not_write_to_the_PRODUCTION_log` snapshots `st_mtime_ns` of `~/.cache/ersatztv/hook-fire/*.jsonl` before and after running a hook, and asserts the two are equal. That directory is the **shared production hook-fire log that every live Claude Code session on this machine writes to**. So any other session firing a hook inside that window flips an mtime and the test fails, reporting `a test run modified the production hook-fire log` — an accusation about the suite, when the writer was an unrelated process. Observed: one red in a full-suite run, green in isolation immediately afterwards; independently corroborated during review, which found a file in that directory written 13 seconds earlier by another session. This repo now routinely runs several sessions and worktree-isolated review agents at once, so this is a live and guaranteed race rather than a theoretical one. ## Why it matters beyond the flake The failure message is confidently wrong about the cause, which is the expensive part — it points the next reader at the test suite rather than at concurrency, and the honest diagnosis costs a re-run plus a directory listing to reach. A flake that misattributes is worse than one that just fails. It also weakens a real guard: the invariant (the suite must not write to the production log) is worth keeping, and a test that cries wolf gets waved through, which is how `ci.decisions-lifecycle-flake` already has to be documented as "do nothing". ## Options - Scope the assertion to files the test could plausibly have touched (e.g. only paths whose names the test's own fixture would generate), rather than the whole directory. - Or snapshot with an exclusive lock / a per-run subdirectory, so the oracle is not global mutable state. - Or have hooks under test write to a redirected `XDG_CACHE_HOME`, making the production path unreachable from the suite by construction — dedup by construction rather than by assertion, per `docs/defect-shapes-773.md` §4 detector C. This is probably the right one: it makes the property structural instead of observed. ## Done-when - [x] The test's oracle no longer depends on state other processes can write - [x] Witnessed: the test stays green while another process writes to `~/.cache/ersatztv/hook-fire/` - [x] The invariant it guards (the suite does not write the production log) still fails when genuinely violated - [x] Adversarial review passed
timothy added the priority: medium label 2026-08-22 23:51:22 +02:00
timothy added the in-progress label 2026-08-28 23:10:34 +02:00
timothy commented 2026-08-28 23:10:49 +02:00
Author
Owner
Copy Link
Copy Source

Claiming #809 + #822 together as a single-mechanism bundle: both are the same guard
(scripts/tests/test_hook_fire_log.py::test_the_suite_does_not_write_to_the_PRODUCTION_log) and the
same conftest isolation seam. #822 is the oracle reading global mutable state; #809 is the isolation
being per-file rather than per-suite. Both bodies independently land on the same structural answer —
make the production path unreachable from the suite by construction, with one place that builds the
hook subprocess env — so fixing them separately would mean building the mechanism twice.

Claude Code session, worktree off origin/main.

Claiming #809 + #822 together as a single-mechanism bundle: both are the same guard (`scripts/tests/test_hook_fire_log.py::test_the_suite_does_not_write_to_the_PRODUCTION_log`) and the same conftest isolation seam. #822 is the oracle reading global mutable state; #809 is the isolation being per-file rather than per-suite. Both bodies independently land on the same structural answer — make the production path unreachable from the suite by construction, with one place that builds the hook subprocess env — so fixing them separately would mean building the mechanism twice. Claude Code session, worktree off `origin/main`.
timothy commented 2026-08-29 04:32:49 +02:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: Fixed in http://192.168.1.95:3000/timothy/ersatztv/pulls/874, together with #809 — the two are one mechanism. test_the_suite_does_not_write_to_the_PRODUCTION_log no longer reads ~/.cache/ersatztv/hook-fire/ at all. It now asserts that the environment a module sees at IMPORT time already resolves away from the shared log, and that a hook driven with that snapshot lands its records in the isolated directory.

Root cause: The oracle was global mutable state. The assertion compared st_mtime_ns across a directory every live Claude Code session on the machine writes to, so any concurrent session's hook flipped an mtime inside the window. The failure message then blamed the suite, which is the expensive part — the honest diagnosis cost a re-run plus a directory listing.

Decisions/conventions changed: Added testing.suite-isolated-from-production-hook-fire-log, which states the rule as establish the property structurally, never by observing the shared directory.

Reusable knowledge: The three options this issue listed were scope-the-assertion, lock/subdir, and make-the-path-unreachable. The third is the only one that holds — but not via XDG_CACHE_HOME as suggested: scripts/hook-fire-log.sh derives its default from $HOME, not XDG_CACHE_HOME, so redirecting the latter changes nothing. Two further gotchas: a Python reimplementation of a shell ${VAR:-default} fails open if it drifts, so it is differential-tested against the shell function every run; and it must compare realpath identity rather than bytes, because shell concatenation and pathlib render the same directory differently (/x/ vs /x, $HOME//.cache vs $HOME/.cache).

Verification: The race was reproduced on demand rather than argued: with a thread touching a file in the shared directory, the withdrawn mtime oracle goes RED (a test run modified the production hook-fire log) while the replacement stays GREEN and the records provably land in the isolated dir. The invariant still fails when genuinely violated — the launch guard rejects a launch carrying no isolated dir, one pointed at either shared log, one pointed inside a shared log, and one whose relative value resolves onto a shared log via the child's cwd; and the nested-pytest mutation proof shows records leaking into a fake HOME when the pre-collection isolation is removed. Full suite 1228 passed / 2 skipped.

Deferred: none. (Guard blind spots are enumerated under #809.)

Docs updated: docs/guard-inventory.md, the new decision record + regenerated catalog, docs/README.md, and docstrings in test_worktree_ownership_guard.py.

## Closing record **Outcome:** Fixed in http://192.168.1.95:3000/timothy/ersatztv/pulls/874, together with #809 — the two are one mechanism. `test_the_suite_does_not_write_to_the_PRODUCTION_log` no longer reads `~/.cache/ersatztv/hook-fire/` at all. It now asserts that the environment a module sees at IMPORT time already resolves away from the shared log, and that a hook driven with that snapshot lands its records in the isolated directory. **Root cause:** The oracle was global mutable state. The assertion compared `st_mtime_ns` across a directory every live Claude Code session on the machine writes to, so any concurrent session's hook flipped an mtime inside the window. The failure message then blamed the suite, which is the expensive part — the honest diagnosis cost a re-run plus a directory listing. **Decisions/conventions changed:** Added `testing.suite-isolated-from-production-hook-fire-log`, which states the rule as *establish the property structurally, never by observing the shared directory*. **Reusable knowledge:** The three options this issue listed were scope-the-assertion, lock/subdir, and make-the-path-unreachable. The third is the only one that holds — but **not** via `XDG_CACHE_HOME` as suggested: `scripts/hook-fire-log.sh` derives its default from `$HOME`, not `XDG_CACHE_HOME`, so redirecting the latter changes nothing. Two further gotchas: a Python reimplementation of a shell `${VAR:-default}` **fails open** if it drifts, so it is differential-tested against the shell function every run; and it must compare `realpath` identity rather than bytes, because shell concatenation and `pathlib` render the same directory differently (`/x/` vs `/x`, `$HOME//.cache` vs `$HOME/.cache`). **Verification:** The race was reproduced on demand rather than argued: with a thread touching a file in the shared directory, the withdrawn mtime oracle goes **RED** (`a test run modified the production hook-fire log`) while the replacement stays **GREEN** and the records provably land in the isolated dir. The invariant still fails when genuinely violated — the launch guard rejects a launch carrying no isolated dir, one pointed at either shared log, one pointed inside a shared log, and one whose relative value resolves onto a shared log via the child's `cwd`; and the nested-pytest mutation proof shows records leaking into a fake `HOME` when the pre-collection isolation is removed. Full suite 1228 passed / 2 skipped. **Deferred:** none. (Guard blind spots are enumerated under #809.) **Docs updated:** `docs/guard-inventory.md`, the new decision record + regenerated catalog, `docs/README.md`, and docstrings in `test_worktree_ownership_guard.py`.
timothy removed the in-progress label 2026-08-29 04:32:54 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels priority: medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#822