[Bugfix] tests/unit/api_manifest_test.py rewrites the tracked api/manifest.json — every suite run dirties the working tree #401

New issue

Open

opened 2026-07-27 08:00:40 +00:00 by john · 0 comments

john commented

2026-07-27 08:00:40 +00:00

Owner

Summary

tests/unit/api_manifest_test.py exercises tools/generate_api_manifest.py by running the real generator against its real output path. The generator writes api/manifest.json, which is a tracked file. So merely running the test suite modifies a committed file.

Surfaced during review of mcrogueface-002 (branch sim/integration); pre-existing, unrelated to that work.

Reproduction

From a clean tree:

$ git status --short api/manifest.json          # (nothing)
$ cd build && ./mcrogueface --headless --exec ../tests/unit/api_manifest_test.py
PASS: manifest deterministic, schema valid, 59 objects / 13 functions
$ cd .. && git status --short api/manifest.json
 M api/manifest.json

The diff is the two provenance fields the generator stamps from the live repo state:

commit : b77aeec -> 1523533
dirty  : True    -> False

Why it matters

"The tree is clean" stops being a usable signal. Anyone who runs run_tests.py before committing now has a modified tracked file that they did not author, and the natural reaction — git add -A — silently commits a manifest stamped with the wrong commit and an arbitrary dirty value.
It fights tools/hooks/pre-commit, which is the component that is supposed to own this file: the hook regenerates it from a freshly built binary and git adds it as its only index write. Two writers, one file, different trigger conditions.
The stamped values are inherently unstable (commit is whatever HEAD is; dirty is whatever the tree looked like at that instant), so the churn is guaranteed, not occasional.

Suggested fix

Point the generator at a temp directory for the duration of the test and assert against that output, rather than against the repository's own artifact. The test's actual assertions — determinism across two runs, schema version, presence and types of the top-level keys — do not need the canonical path; they only need a generated manifest.

If the generator cannot currently be redirected, that redirect is the fix: a test should not be able to mutate a tracked file as a side effect of asserting on it.

The pre-commit hook (tools/hooks/pre-commit), the intended sole writer of api/manifest.json
#380 — shipped artifacts gated by nothing

## Summary `tests/unit/api_manifest_test.py` exercises `tools/generate_api_manifest.py` by **running the real generator against its real output path**. The generator writes `api/manifest.json`, which is a tracked file. So merely running the test suite modifies a committed file. Surfaced during review of `mcrogueface-002` (branch `sim/integration`); pre-existing, unrelated to that work. ## Reproduction From a clean tree: ``` $ git status --short api/manifest.json # (nothing) $ cd build && ./mcrogueface --headless --exec ../tests/unit/api_manifest_test.py PASS: manifest deterministic, schema valid, 59 objects / 13 functions $ cd .. && git status --short api/manifest.json M api/manifest.json ``` The diff is the two provenance fields the generator stamps from the live repo state: ``` commit : b77aeec -> 1523533 dirty : True -> False ``` ## Why it matters 1. **"The tree is clean" stops being a usable signal.** Anyone who runs `run_tests.py` before committing now has a modified tracked file that they did not author, and the natural reaction — `git add -A` — silently commits a manifest stamped with the *wrong* commit and an arbitrary `dirty` value. 2. It fights `tools/hooks/pre-commit`, which is the component that is *supposed* to own this file: the hook regenerates it from a freshly built binary and `git add`s it as its only index write. Two writers, one file, different trigger conditions. 3. The stamped values are inherently unstable (`commit` is whatever HEAD is; `dirty` is whatever the tree looked like at that instant), so the churn is guaranteed, not occasional. ## Suggested fix Point the generator at a temp directory for the duration of the test and assert against *that* output, rather than against the repository's own artifact. The test's actual assertions — determinism across two runs, schema version, presence and types of the top-level keys — do not need the canonical path; they only need *a* generated manifest. If the generator cannot currently be redirected, that redirect is the fix: a test should not be able to mutate a tracked file as a side effect of asserting on it. ## Related - The pre-commit hook (`tools/hooks/pre-commit`), the intended sole writer of `api/manifest.json` - #380 — shipped artifacts gated by nothing