fix: stop CLI tests rewriting the real .mcp.json, align pre-push ruff with CI (#45)

* fix(tests): stop CLI tests rewriting the real .mcp.json

`setup_reaper._resolve_mcp_target()` picks its write target as the first
existing of $HOME/.mcp.json then cwd/.mcp.json. Under pytest cwd is the repo
root, so `test_success_moves_into_place` -- which invokes `setup-reaper
--install-dir <tmp> --yes` -- rewrote the developer's real project .mcp.json
with pytest tmpdir paths, pointing the reaper MCP server at a directory that
ceased to exist when the test finished. `--yes` skipped the confirmation that
would otherwise have caught it.

The existing autouse `_confine_writes_to_tmp` fixture missed this because it
only redirects PHANTOM_OUTPUT_DIR, and .mcp.json resolution never consults it.

Extend that fixture to redirect all three .mcp.json resolvers into tmp_path,
so no test can reach the real file regardless of which command it invokes.
Fixing it in the fixture rather than at each call site means new tests inherit
the protection. Tests that patch these resolvers themselves are unaffected --
mock.patch applies on top and restores afterward.

Extract `_mcp_candidates()` in uninstall.py so its scan (which feeds the
removal path) is redirectable the same way, instead of an inline literal.

Verified by reproducing: with the guard removed, the single test rewrites
.mcp.json with a fresh tmpdir; with it in place, 1041 passed / 47 skipped and
the file is byte-identical before and after.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* fix(ci): run ruff from dev deps in pre-push, matching CI

The hook ran `uv tool run ruff`, which resolves to the newest ruff available
rather than the `ruff>=0.15` pinned in dev dependencies. CI runs `uv run ruff`.
Once ruff 0.16 landed in the tool cache the two diverged: 0.16 widened its
default rule set, and since this project ships no [tool.ruff] config and relies
on those defaults, the hook reported 251 errors on a tree CI considers clean.

Switch the hook to `uv run` so local and CI enforce the same rules from the
same version. No source changes were needed -- the tree passes cleanly under
the pinned ruff 0.15.12.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* fix(tests): stop the guard triggering first-run auto-setup in CI

The .mcp.json guard broke CI even though it passed locally. The CLI group
callback (phantom/cli/__init__.py) auto-runs `phantom setup` whenever no
.mcp.json holds a phantom entry, and it re-derived $HOME/cwd inline instead of
using the same resolver the writers use. Redirecting only the writers split the
pair: setup wrote into a fresh per-test sandbox while the check kept reading
cwd, so it never saw a configured install and re-ran full setup on every CLI
invocation, printing into output that tests parse as JSON.

This is invisible on a dev machine, where the repo's real .mcp.json satisfies
the check, and only appears where no .mcp.json exists -- i.e. CI. It surfaced
as 11 failures across tests/test_cli_analyze.py.

Route the first-run check through `setup._mcp_candidates()` so the check and the
writers can never disagree again, and seed the sandbox with a phantom entry so
auto-setup stays quiet under test.

The sandbox lives outside tmp_path: some tests point the resolvers at
tmp_path/".mcp.json" and assert on what the command writes there, and
test_no_predictable_tmp_left_behind asserts tmp_path's exact directory listing.
A per-test factory dir satisfies both.

Verified by simulating CI (no .mcp.json in cwd or $HOME, live tests deselected
as CI does since their fixtures aren't committed): 1016 passed, 44 skipped.
Normal local run with the config present: 1041 passed, 47 skipped, .mcp.json
byte-identical, and no ~/.mcp.json created.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

---------

Co-authored-by: Lee Saenz <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

Author: leesaenz

scripts/pre-push

@@ -25,11 +25,15 @@ echo "  No PII found in tracked files."
 
 echo ""
 echo "==> Linting (ruff check)..."
-uv tool run ruff check src/ tests/ packages/
+# `uv run` (not `uv tool run`) so this uses the ruff pinned in the dev
+# dependencies -- the same version CI runs. `uv tool run` resolves to whatever
+# ruff is newest, and a new minor can widen the default rule set, so the hook
+# would fail on rules CI never enforces.
+uv run ruff check src/ tests/ packages/
 
 echo ""
 echo "==> Format check (ruff format)..."
-uv tool run ruff format --check src/ tests/ packages/
+uv run ruff format --check src/ tests/ packages/
 
 echo ""
 echo "==> Running tests (pytest)..."

src/phantom/cli/__init__.py

@@ -29,12 +29,15 @@ def cli(ctx: click.Context) -> None:
     if ctx.invoked_subcommand not in (None, "setup", "uninstall", "version", "update"):
         try:
             import json
-            from pathlib import Path
 
-            mcp_home = Path.home() / ".mcp.json"
-            mcp_cwd = Path.cwd() / ".mcp.json"
+            # Resolve through setup's helper rather than re-deriving
+            # $HOME/cwd here: the two must agree, or setup writes the entry
+            # somewhere this check doesn't look and auto-setup fires on every
+            # invocation. It also gives tests one place to redirect.
+            from phantom.cli.setup import _mcp_candidates
+
             phantom_configured = False
-            for mcp_path in (mcp_home, mcp_cwd):
+            for mcp_path in _mcp_candidates():
                 if mcp_path.exists():
                     data = json.loads(mcp_path.read_text())
                     if "phantom" in data.get("mcpServers", {}):

src/phantom/cli/uninstall.py

@@ -35,14 +35,23 @@ def _get_reaper_scripts_dir() -> Path:
         return Path.home() / ".config" / "REAPER" / "Scripts"
 
 
+def _mcp_candidates() -> list[Path]:
+    """.mcp.json locations to scan, nearest first.
+
+    Kept as a function rather than an inline list so tests can redirect it away
+    from the developer's real config files (see tests/conftest.py).
+    """
+    return [Path.cwd() / ".mcp.json", Path.home() / ".mcp.json"]
+
+
 def _find_artifacts() -> dict:
     """Scan for all Phantom artifacts on disk."""
     artifacts: dict = {}
 
     if _PHANTOM_DIR.exists():
         artifacts["phantom_dir"] = str(_PHANTOM_DIR)
 
-    for mcp_path in [Path.cwd() / ".mcp.json", Path.home() / ".mcp.json"]:
+    for mcp_path in _mcp_candidates():
         if mcp_path.exists():
             try:
                 data = json.loads(mcp_path.read_text())

tests/conftest.py

@@ -4,6 +4,7 @@
 No WAV files are committed to the repository (D-11, D-12).
 """
 
+import json
 import os
 from pathlib import Path
 
@@ -42,7 +43,7 @@
 
 
 @pytest.fixture(autouse=True)
-def _confine_writes_to_tmp(tmp_path, monkeypatch):
+def _confine_writes_to_tmp(tmp_path, tmp_path_factory, monkeypatch):
     """Confine writes to each test's tmp_path by default (Finding 1).
 
     Phantom now confines all file writes to PHANTOM_OUTPUT_DIR (default
@@ -53,6 +54,38 @@ def _confine_writes_to_tmp(tmp_path, monkeypatch):
     """
     monkeypatch.setenv("PHANTOM_OUTPUT_DIR", str(tmp_path))
 
+    # .mcp.json resolution bypasses PHANTOM_OUTPUT_DIR: every writer picks a
+    # target from $HOME/cwd directly. Under pytest, cwd is the repo root, so a
+    # test invoking `setup-reaper --yes` or `setup` would rewrite the developer's
+    # real .mcp.json with tmp_path values, and `uninstall` would strip entries
+    # out of it. Redirect all three resolvers into tmp_path so no test can reach
+    # the real file. Tests that patch these themselves still win (mock.patch
+    # applies on top and restores afterward).
+    # Deliberately OUTSIDE tmp_path, in its own directory. Two constraints
+    # rule out the obvious placements: several tests point the resolvers at
+    # tmp_path/".mcp.json" themselves and assert on what the command writes
+    # there (so seeding that path pre-creates the file under assertion), and
+    # at least one test asserts tmp_path's exact directory listing (so an extra
+    # entry anywhere inside tmp_path breaks it). A fresh factory dir per test
+    # avoids both, and keeps each test's sandbox isolated from its neighbours.
+    mcp_target = tmp_path_factory.mktemp("mcp_sandbox") / ".mcp.json"
+    monkeypatch.setattr(
+        "phantom.cli.setup_reaper._resolve_mcp_target", lambda: mcp_target
+    )
+    monkeypatch.setattr("phantom.cli.setup._mcp_candidates", lambda: [mcp_target])
+    monkeypatch.setattr("phantom.cli.uninstall._mcp_candidates", lambda: [mcp_target])
+
+    # Seed a phantom entry so the CLI group's first-run auto-setup
+    # (phantom/cli/__init__.py) treats this sandbox as already configured.
+    # Without it the sandbox is empty for every test, so every CLI invocation
+    # would run the full `phantom setup` and print it into the output under
+    # assertion -- which breaks any test parsing stdout as JSON. This is only
+    # visible where no real .mcp.json exists, i.e. CI, not a dev machine.
+    mcp_target.write_text(
+        json.dumps({"mcpServers": {"phantom": {"command": "phantom-mcp", "args": []}}})
+        + "\n"
+    )
+
 
 @pytest.fixture
 def mono_sine_440hz():