Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions tests/test_hooks_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,39 @@
)


@pytest.fixture(autouse=True)
def _isolated_existing_palace_root(monkeypatch, tmp_path):
"""Give every test an isolated, *existing* PALACE_ROOT/STATE_DIR.

Regression for #1510: nine save / log / precompact tests assumed
``~/.mempalace`` existed and only passed in the full suite because an
earlier test file (``test_cli.py``) created it as a side effect, so
the ``_palace_root_exists()`` kill-switch was satisfied. Run in
isolation they short-circuited and failed.

Defaulting every test to a per-test palace root that exists makes
them robust on their own and protects future tests from the same
trap. ``_MINE_PID_DIR`` is patched too: it is derived from
``STATE_DIR`` *at module import* (hooks_cli.py:277), so patching
``STATE_DIR`` alone would leave mine-spawning tests writing PID files
under the import-time location instead of the per-test root. The
state dir is created so the docstring's "existing" promise holds.

Tests that exercise the absent-root kill-switch path call
``_redirect_palace_root`` (or set their own PALACE_ROOT) *after* this
fixture; ``monkeypatch``'s last-write-wins means they keep their
absent/file root and teardown still restores the real module value.
"""
root = tmp_path / ".mempalace"
state_dir = root / "hook_state"
state_dir.mkdir(parents=True, exist_ok=True)
monkeypatch.setattr(hooks_cli_mod, "PALACE_ROOT", root)
monkeypatch.setattr(hooks_cli_mod, "STATE_DIR", state_dir)
monkeypatch.setattr(hooks_cli_mod, "_MINE_PID_DIR", state_dir / "mine_pids")
monkeypatch.setattr(hooks_cli_mod, "_state_dir_initialized", False)
Comment on lines +57 to +63

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The fixture patches PALACE_ROOT and STATE_DIR, but it misses _MINE_PID_DIR. Since _MINE_PID_DIR is initialized at module import time (line 277 of hooks_cli.py), it retains its original value pointing to the real ~/.mempalace even after STATE_DIR is patched. This causes tests that spawn mine processes (like _spawn_mine or _ingest_transcript) to create PID files in the real user directory, leading to filesystem leaks and potential conflicts with a running instance.

Additionally, the docstring promises an existing STATE_DIR, but the code only creates the PALACE_ROOT. I've updated the suggestion to create both and ensure _MINE_PID_DIR is also isolated.

Suggested change
root = tmp_path / ".mempalace"
root.mkdir(exist_ok=True)
monkeypatch.setattr(hooks_cli_mod, "PALACE_ROOT", root)
monkeypatch.setattr(hooks_cli_mod, "STATE_DIR", root / "hook_state")
monkeypatch.setattr(hooks_cli_mod, "_state_dir_initialized", False)
root = tmp_path / ".mempalace"
state_dir = root / "hook_state"
state_dir.mkdir(parents=True, exist_ok=True)
monkeypatch.setattr(hooks_cli_mod, "PALACE_ROOT", root)
monkeypatch.setattr(hooks_cli_mod, "STATE_DIR", state_dir)
monkeypatch.setattr(hooks_cli_mod, "_MINE_PID_DIR", state_dir / "mine_pids")
monkeypatch.setattr(hooks_cli_mod, "_state_dir_initialized", False)

return root


# --- _mempalace_python ---


Expand Down
Loading