|
18 | 18 | 5. Module constants |
19 | 19 | """ |
20 | 20 | import json |
| 21 | +import os |
21 | 22 | import stat |
22 | 23 | import subprocess |
23 | 24 | import sys |
| 25 | +import tempfile |
24 | 26 | from io import StringIO |
25 | 27 | from pathlib import Path |
26 | 28 | from unittest.mock import patch |
|
33 | 35 | HOOK_PATH = str(Path(__file__).parent.parent / "hooks" / "postcompact_archive.py") |
34 | 36 |
|
35 | 37 |
|
36 | | -def run_hook(stdin_data: str | None = None) -> subprocess.CompletedProcess: |
37 | | - """Run the hook as a subprocess and return the result.""" |
| 38 | +def run_hook( |
| 39 | + stdin_data: str | None = None, |
| 40 | + env_root: str | None = None, |
| 41 | +) -> subprocess.CompletedProcess: |
| 42 | + """Run the hook as a subprocess and return the result. |
| 43 | +
|
| 44 | + Config-root isolation (#1191): the postcompact child resolves its write |
| 45 | + target via ``get_claude_config_dir()`` (precedence-1 ``$CLAUDE_CONFIG_DIR``, |
| 46 | + else ``$HOME/.claude``). The autouse ``_isolate_config_root_to_tmp`` fixture |
| 47 | + redirects ``Path.home`` IN-PROCESS ONLY — that ``setattr`` does NOT cross to |
| 48 | + this subprocess child, and the fixture DELIBERATELY does not set the HOME env |
| 49 | + var (see conftest's "WHY NOT ALSO SET HOME ENV" note: a global HOME override |
| 50 | + breaks the telegram ``cwd_is_home`` tests). Without an explicit env pin here, |
| 51 | + a LEAD-frame input (``agent_type`` in ``LEAD_AGENT_TYPES`` + truthy |
| 52 | + ``compact_summary``) opens the ``is_lead`` gate (postcompact_archive.py) and |
| 53 | + the child writes ``compact-summary.txt`` to the operator's REAL |
| 54 | + ``~/.claude/pact-sessions/``. |
| 55 | +
|
| 56 | + Pin BOTH ``HOME`` and ``CLAUDE_CONFIG_DIR`` to a tmp root so the child |
| 57 | + resolves the tmp, never real home — matching the suite's per-test |
| 58 | + subprocess-isolation convention (Form A: ``monkeypatch.setenv`` of |
| 59 | + ``CLAUDE_CONFIG_DIR`` in test_pact_harvest_cli / test_config_dir_*; Form B: |
| 60 | + ``env={**os.environ, "HOME": tmp}`` in test_session_journal). Setting both is |
| 61 | + belt-and-suspenders: ``CLAUDE_CONFIG_DIR`` precedence-1 does the resolution |
| 62 | + work; ``HOME`` is a harmless backup covering any HOME-fallthrough path. |
| 63 | + ``env_root`` defaults to a fresh ``mkdtemp`` so EVERY caller is isolated — |
| 64 | + the latent #1191 gap is closed universally, not just for callers that pass a |
| 65 | + root. |
| 66 | + """ |
| 67 | + if env_root is None: |
| 68 | + env_root = tempfile.mkdtemp(prefix="postcompact-hook-test-") |
38 | 69 | return subprocess.run( |
39 | 70 | [sys.executable, HOOK_PATH], |
40 | 71 | input=stdin_data or "", |
41 | 72 | capture_output=True, |
42 | 73 | text=True, |
43 | 74 | timeout=10, |
| 75 | + env={**os.environ, "HOME": env_root, "CLAUDE_CONFIG_DIR": env_root}, |
44 | 76 | ) |
45 | 77 |
|
46 | 78 |
|
@@ -383,3 +415,89 @@ def test_lead_frame_does_overwrite_real_file(self, monkeypatch, tmp_path): |
383 | 415 | assert summary_path.read_text(encoding="utf-8") == "NEW LEAD SUMMARY", ( |
384 | 416 | "a lead PostCompact must still archive the compact summary" |
385 | 417 | ) |
| 418 | + |
| 419 | + |
| 420 | +# --------------------------------------------------------------------------- |
| 421 | +# #1191: config-root isolation pin for the run_hook subprocess spawn. |
| 422 | +# --------------------------------------------------------------------------- |
| 423 | + |
| 424 | + |
| 425 | +class TestPostcompactRunHookConfigRootIsolation: |
| 426 | + """Delete-the-fix counter-test for run_hook's config-root env pin (#1191). |
| 427 | +
|
| 428 | + ``run_hook`` spawns the postcompact child as a subprocess. The child's |
| 429 | + compact-summary write resolves through ``get_claude_config_dir()`` |
| 430 | + (precedence-1 ``$CLAUDE_CONFIG_DIR``, else ``$HOME/.claude``). The autouse |
| 431 | + ``_isolate_config_root_to_tmp`` fixture redirects ``Path.home`` IN-PROCESS |
| 432 | + ONLY — that ``setattr`` does NOT cross to the subprocess child, and the |
| 433 | + fixture deliberately does NOT set the HOME env var (see conftest's "WHY NOT |
| 434 | + ALSO SET HOME ENV"). Without run_hook's env= pin, a LEAD-frame input opens |
| 435 | + the ``is_lead`` gate and the child writes ``compact-summary.txt`` to the |
| 436 | + operator's REAL ``~/.claude/pact-sessions/``. ``run_hook`` pins BOTH ``HOME`` |
| 437 | + and ``CLAUDE_CONFIG_DIR`` to a tmp root so the child resolves the tmp. |
| 438 | +
|
| 439 | + SOLE-PROVIDER DOCTRINE (#1189 / test_conftest_config_root_isolation.py): |
| 440 | + this test does NOT self-provide HOME/CLAUDE_CONFIG_DIR isolation (no |
| 441 | + ``monkeypatch.setenv``, no in-body env override). It passes |
| 442 | + ``env_root=str(tmp_path)`` only to SELECT the tmp target the pin should |
| 443 | + resolve to; the ISOLATION mechanism is run_hook's env= pin itself. The test |
| 444 | + therefore stays green solely because run_hook applies the pin — the #1189 |
| 445 | + "the test's pass depends on the fix's correctness" shape. |
| 446 | +
|
| 447 | + COUNTER-TEST PROPERTY (the pinning property this class exists for): if the |
| 448 | + ``env=env`` pin is deleted from run_hook (``env_root`` threaded but unused), |
| 449 | + the child inherits the test process's ``os.environ`` — HOME=real-operator- |
| 450 | + home, CLAUDE_CONFIG_DIR absent (scrubbed by the autouse fixture) — resolves |
| 451 | + the REAL ``~/.claude``, and writes ``compact-summary.txt`` there. The |
| 452 | + ``tmp_path`` target this test asserts would be MISSING, so the containment |
| 453 | + assertion FAILS. |
| 454 | +
|
| 455 | + Verified by guarded-mutation (per the #1189 precedent's "Verified by local |
| 456 | + fixture-disable, reverted before staging"): with the fix committed, delete |
| 457 | + ONLY the ``env=env`` line from run_hook, run THIS test under a guarded HOME |
| 458 | + (``HOME=/tmp/<throwaway>``) so the mutation's real-home-resolved write lands |
| 459 | + in the throwaway and NOT the operator's real ``~/.claude``, observe the |
| 460 | + ``tmp_path``-containment assertion FAIL, then restore via |
| 461 | + ``git restore -- pact-plugin/tests/test_postcompact_archive.py`` (recovers |
| 462 | + the committed pinned version byte-identically; ``git diff --quiet -- <file>`` |
| 463 | + exits 0). The guard prevents the leak DURING the probe; the POSITIVE |
| 464 | + ``tmp_path``-containment assertion is the load-bearing signal the delete |
| 465 | + breaks (a not-at-real-home negative is optional belt-and-suspenders and does |
| 466 | + NOT replace the guard — it would only fire AFTER a leak had already fired). |
| 467 | + """ |
| 468 | + |
| 469 | + def test_lead_frame_write_lands_under_pinned_tmp_not_real_home(self, tmp_path): |
| 470 | + """A LEAD-frame PostCompact through run_hook writes compact-summary.txt |
| 471 | + under the run_hook env-pin tmp root, NOT the operator's real home. |
| 472 | +
|
| 473 | + Drives the REAL write path: the LEAD frame (``agent_type`` a lead |
| 474 | + spelling + truthy ``compact_summary``) opens the ``is_lead`` gate, so |
| 475 | + ``write_compact_summary -> get_compact_summary_path -> |
| 476 | + get_claude_config_dir`` fires in the child. With the pin, the child |
| 477 | + resolves ``tmp_path``; without it (delete-the-fix), the child resolves |
| 478 | + the real home and the ``tmp_path`` target is empty. |
| 479 | + """ |
| 480 | + from fixtures.role_frames import postcompact_frame |
| 481 | + |
| 482 | + lead_frame = postcompact_frame( |
| 483 | + "PACT:pact-orchestrator", compact_summary="LEAD COUNTER-TEST SUMMARY" |
| 484 | + ) |
| 485 | + result = run_hook(json.dumps(lead_frame), env_root=str(tmp_path)) |
| 486 | + assert result.returncode == 0, ( |
| 487 | + f"postcompact child exited {result.returncode}; stderr={result.stderr!r}" |
| 488 | + ) |
| 489 | + |
| 490 | + # POSITIVE tmp-target assertion (load-bearing). Robust to the Form A vs |
| 491 | + # Form B leaf shape: CCD=tmp -> tmp/pact-sessions/compact-summary.txt; |
| 492 | + # HOME=tmp -> tmp/.claude/pact-sessions/compact-summary.txt. A recursive |
| 493 | + # glob catches either. |
| 494 | + written = list(tmp_path.glob("**/compact-summary.txt")) |
| 495 | + assert len(written) == 1, ( |
| 496 | + f"expected exactly one compact-summary.txt under the pinned tmp root " |
| 497 | + f"{tmp_path}, found {written}. If run_hook's env= pin is absent, the " |
| 498 | + f"child resolved the REAL ~/.claude (HOME fallthrough — the autouse " |
| 499 | + f"Path.home setattr does not cross to subprocesses) and wrote " |
| 500 | + f"elsewhere: the #1191 latent leak is OPEN. (delete-the-fix: this " |
| 501 | + f"assertion is what the env= pin's absence breaks.)" |
| 502 | + ) |
| 503 | + assert written[0].read_text(encoding="utf-8") == "LEAD COUNTER-TEST SUMMARY" |
0 commit comments