fix(tests): check_docs and the CLI tests write data/hikcentral.db and logs/lifecycle.log into the checkout

gabogg commented

2026-10-02 16:48:59 +00:00

Owner

Agent Brief

Category: bug
Summary: The test suite and the docs check write files into the checkout they run in: an empty data/hikcentral.db and a logs/lifecycle.log full of fake test users. They must write only to temporary locations.

Current behavior:

scripts/check_docs.py creates an empty database.
- It imports app.main to read the OpenAPI routes. The import makes the door service load doors from the default DATABASE_PATH (data/hikcentral.db).
- In a fresh worktree, SQLite creates an empty file there (4 KB, no tables). That is why the check always prints Error loading doors from DB: no such table: door_records.
- In the main checkout the same import opens the real local database (a prod copy used for validation).
- CI and every agent that runs the check do this.
The CLI user-management tests write to the working directory's logs/.
- They create, re-password and delete cli_test_user, cli_prompt_user and cli_passwd_user, and call log_lifecycle_event() without repo_dir or log_file_override.
- Those calls append to ./logs/lifecycle.log relative to wherever pytest runs, so every pytest run, including the pre-commit hook, appends 4 entries.
- One worktree had 80 entries (about 20 runs), recorded as real lifecycle audit events under the developer's OS username.
The effect on worktree cleanup: scripts/wt prune, which wt merge runs, rightly refuses to delete a worktree that holds ignored local files, since they could be real data. So every merged PR's worktree is kept and needs a manual wt rm -f.

Desired behavior:

Running scripts/check_docs.py creates and opens no database file in the checkout. Either point DATABASE_PATH at a temporary location before importing the app, or build the OpenAPI schema without starting services that touch the DB. The "Error loading doors" line goes away.
No test writes to the checkout's logs/ or data/. The CLI tests pass a tmp_path-based location, as the other lifecycle-log tests already do.
Optional hardening: a session-level guard (for example in tests/conftest.py) that sends the default lifecycle log location to a temp dir during tests.
A regression check proves it: after a full pytest run plus scripts/check_docs.py, neither data/ nor logs/ exists in a fresh worktree, or their contents are unchanged.

Acceptance criteria:

In a fresh worktree with no data/ or logs/, python3 scripts/check_docs.py succeeds, prints no DB error, and creates no data/ file.
In the same worktree, a full pytest run creates no logs/lifecycle.log and no data/ file.
An automated test or check enforces both. It must not depend on a developer's local files.
Production behaviour is unchanged: the CLI still writes logs/lifecycle.log under the real install directory.
After this lands, scripts/wt merge / wt prune removes a merged PR's worktree without the "holds ignored local files" warning when nothing else was added locally.

Out of scope:

Changing what the lifecycle log records in production.
Changing wt prune's safety rule about ignored files. It is correct.
Cleaning up existing leftover files in current worktrees; the maintainer handles that.

Found while merging #169 (2026-10-02).

🤖 Generated with Claude Code

## Agent Brief **Category:** bug **Summary:** The test suite and the docs check write files into the checkout they run in: an empty `data/hikcentral.db` and a `logs/lifecycle.log` full of fake test users. They must write only to temporary locations. **Current behavior:** - **`scripts/check_docs.py` creates an empty database.** - It imports `app.main` to read the OpenAPI routes. The import makes the door service load doors from the default `DATABASE_PATH` (`data/hikcentral.db`). - In a fresh worktree, SQLite creates an empty file there (4 KB, no tables). That is why the check always prints `Error loading doors from DB: no such table: door_records`. - In the main checkout the same import opens the real local database (a prod copy used for validation). - CI and every agent that runs the check do this. - **The CLI user-management tests write to the working directory's `logs/`.** - They create, re-password and delete `cli_test_user`, `cli_prompt_user` and `cli_passwd_user`, and call `log_lifecycle_event()` without `repo_dir` or `log_file_override`. - Those calls append to `./logs/lifecycle.log` relative to wherever pytest runs, so every pytest run, including the pre-commit hook, appends 4 entries. - One worktree had 80 entries (about 20 runs), recorded as real lifecycle audit events under the developer's OS username. - **The effect on worktree cleanup:** `scripts/wt prune`, which `wt merge` runs, rightly refuses to delete a worktree that holds ignored local files, since they could be real data. So every merged PR's worktree is kept and needs a manual `wt rm -f`. **Desired behavior:** - Running `scripts/check_docs.py` creates and opens **no database file** in the checkout. Either point `DATABASE_PATH` at a temporary location before importing the app, or build the OpenAPI schema without starting services that touch the DB. The "Error loading doors" line goes away. - No test writes to the checkout's `logs/` or `data/`. The CLI tests pass a `tmp_path`-based location, as the other lifecycle-log tests already do. - Optional hardening: a session-level guard (for example in `tests/conftest.py`) that sends the default lifecycle log location to a temp dir during tests. - A regression check proves it: after a full `pytest` run plus `scripts/check_docs.py`, neither `data/` nor `logs/` exists in a fresh worktree, or their contents are unchanged. **Acceptance criteria:** - [ ] In a fresh worktree with no `data/` or `logs/`, `python3 scripts/check_docs.py` succeeds, prints no DB error, and creates no `data/` file. - [ ] In the same worktree, a full `pytest` run creates no `logs/lifecycle.log` and no `data/` file. - [ ] An automated test or check enforces both. It must not depend on a developer's local files. - [ ] Production behaviour is unchanged: the CLI still writes `logs/lifecycle.log` under the real install directory. - [ ] After this lands, `scripts/wt merge` / `wt prune` removes a merged PR's worktree without the "holds ignored local files" warning when nothing else was added locally. **Out of scope:** - Changing what the lifecycle log records in production. - Changing `wt prune`'s safety rule about ignored files. It is correct. - Cleaning up existing leftover files in current worktrees; the maintainer handles that. Found while merging #169 (2026-10-02). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg added the

ready-for-agent

label

2026-10-02 16:48:59 +00:00

gabogg referenced this issue from a commit

2026-10-02 18:23:10 +00:00

chore: open draft for #216 and #210 (tests stay inside their sandbox)

~~gabogg referenced this issue 2026-10-02 18:23:43 +00:00~~

fix(tests): keep tests inside their sandbox and cut suite time #218

gabogg commented

2026-10-02 18:23:58 +00:00

Author