fix(gitignore): anchor .Codex/ to repo root, report outside worktrees (ADR-1078) - #602

ZETETIC-REVIEW: APPROVE

Reviewed final head 5447cbb. Whole PR inspected at 7fcb1cb against origin/main, including delta since 6714ed9, then all six changed paths through final head. No blocking findings. Remote CI at this exact head remains required and is owned by the coordinator.

Independently verified: clean exact head; diff whitespace check; both backend check lists; optional warning exit behavior; main-checkout anchoring; resolved ancestry and both host roots; NUL parser preserving newline and trailing space; real-Git regression coverage; ADR-1079 manifest and exact mirror agreement; no stale ADR-1078 pointers in changed code or documentation; timeout precedent; function size split.

The proposed PR body identifies the behavior, validation versions, targeted mutation limits and Windows filename-test limitation. Its 19-test count described the earlier head and should be refreshed before publication with the final validation result. Reported validation for the earlier head: 19 doctor tests, pinned Ruff 0.16.6 check and format, craftsmanship, wiki gate, and two killed mutants. These executions belong to the implementer/coordinator; reviewer did not rerun them or assert that count for final head.

Final delta inspection verifies that unavailable Git results now produce an honest optional warning with possible causes and a retry command. Empty successful output remains distinct. New tests assert parser markers, Unicode, empty/None results, orphan markers, both registries, CLI warning and exit status, repair text, and MCP readiness/check payload. A separate consumer test covers unavailable Git without blocking CLI or MCP readiness. ADR and changelog document this behavior; the final mirror exactly matches the canonical ADR. The wording observation about the parser docstring has been corrected to inspection unavailable.

Final HEAD and clean status independently checked; final whitespace check passed. Repository remained read-only. No GitHub comment or merge performed.

ZETETIC-REVIEW: REQUEST_CHANGES

Reviewed 5447cbb in a detached worktree at that sha, gates run with the repository's uv.lock environment.

Measured on the head:

pytest tests_py/test_doctor.py tests_py/test_doctor_worktrees.py -q -p no:cacheprovider
29 passed in 3.21s
python scripts/check_craftsmanship.py --base origin/main
Craftsmanship gate: OK
ruff check; ruff format --check            clean
python scripts/check_project_wiki.py       Project wiki mirrors match their canonical sources.
python scripts/check_doc_claims.py         OK (one pre-existing declared exemption)
gh pr checks 602                           every check pass or skipping, none failed
ADR number                                 1079; #600 merged as 9656fc4d with 1078, no collision

Blocking finding, measured by running the PR's mcp_server from a directory that is not a git checkout:

cd /private/tmp/claude-501 && PYTHONPATH=<worktree> python -m mcp_server.doctor
  [WARN] worktree locations (optional)    Unable to inspect worktree locations: not a Git checkout, Git unavailable, or command failed.

check_setup returns the same check with ok: false. The plan this PR implements had "not a git checkout" as ok True, and the earlier head 6714ed9 behaved that way (test_not_a_git_checkout_passes); 7fcb1cb inverted it into test_not_a_git_checkout_warns. Cortex is an MCP server whose cwd is whatever project the host opened, and a project that is not a git repository is an ordinary condition, not a failure to inspect. With this head every preflight in such a project shows a WARN and a fix command (git worktree list --porcelain -z) that cannot succeed there. Git already distinguishes the cases: "not a git repository" exits 128 with that message, while a missing binary raises FileNotFoundError and a hang raises TimeoutExpired. run_with_hard_timeout collapses all of them to None, so the check needs either a stdlib probe before the call (walk Path.cwd() and its parents for a .git entry, which is the discovery git itself performs) or a variant of the helper that exposes the return code. The expected shape: ok True, detail "not a git checkout" on a plain tmp_path; the WARN kept for git missing or timing out (a test with a .git present and run_with_hard_timeout patched to None). ADR-1079's last paragraph should say the same.

Non-blocking: docs/agent-guidance.md and ADR-1079 now name .Codex/worktrees/<name>/ as a second allowed location, following the #601 decision. CLAUDE.md:27 still reads "Every git worktree lives at .claude/worktrees/<name>/", so the non-negotiable and the guidance disagree on the same page set; one line in CLAUDE.md naming the Codex directory closes that.

The rest of the delta since 6714ed9 holds up: the -z parser keeps newlines and trailing spaces in paths (test_reports_exact_path_with_newline creates a real worktree with both), orphan markers are ignored, the symlink escape and the prefix sibling .Codex/worktrees-other are rejected, and _worktree_locations is under the 40-line limit after the split.

ZETETIC-REVIEW: APPROVE

This supersedes the REQUEST_CHANGES of round two (#602 (comment)). Both findings from that round are fixed and verified at head 28aea2af, rebased onto origin/main 4bd7e7f0. Everything below was measured in a read-only worktree at that head, with the repository interpreter (.venv/bin/python, Python 3.13.7) reached through a symlink to the main checkout's venv, so the executor's replaced-venv artifact does not enter the picture.

Base and scope

$ git merge-base HEAD origin/main
4bd7e7f05a40ddd66aa59ec68ce19f7f17ea9a20

$ git diff origin/main --stat
 .gitignore                                         |   2 +-
 CHANGELOG.md                                       |  13 ++
 CLAUDE.md                                          |   3 +-
 ...ion-git-worktrees-as-an-optional-doctor-warn.md |  35 +++++
 docs/agent-guidance.md                             |   7 +-
 mcp_server/doctor.py                               | 154 +++++++++++++++++++++
 tests_py/test_doctor.py                            | 129 +++++++++++++++++
 tests_py/test_doctor_worktrees.py                  | 140 +++++++++++++++++++
 ...ion-git-worktrees-as-an-optional-doctor-warn.md |  33 +++++
 wiki/manifest.json                                 |   4 +
 10 files changed, 515 insertions(+), 5 deletions(-)

The round-two blocker: a project without git

Fixed. From a directory with no .git anywhere up the tree:

$ cd /private/tmp/claude-501
$ PYTHONPATH=<worktree> <repo>/.venv/bin/python -m mcp_server.doctor
  [OK  ] worktree locations (optional)    not a git checkout

From the checkout root, the check names both allowed roots:

  [OK  ] worktree locations (optional)    all worktrees under /Users/cdeust/Developments/anthropic-partnership/Cortex/.claude/worktrees or /Users/cdeust/Developments/anthropic-partnership/Cortex/.Codex/worktrees

And in a real checkout where git cannot answer, with a stub git on PATH that exits 127:

$ env PATH=<stub-dir> <repo>/.venv/bin/python -m mcp_server.doctor
  [WARN] worktree locations (optional)    Unable to inspect worktree locations: Git unavailable or command failed.
  2. worktree locations (optional):
     → Install Git and ensure it's on PATH; retry git worktree list --porcelain -z.

No [FAIL], exit code unchanged. The distinction the review asked for is the one now implemented, and ADR-1079's closing paragraph states it in the same terms as the code.

_is_git_checkout semantics, probed

linked worktree root (.git is a file?): True  -> True
subdir of repo:                               -> True
root /:                                       -> False
tmp non-repo:                                 -> False
from a subdir, the check still resolves the main worktree:
  True  all worktrees under .../.claude/worktrees or .../.Codex/worktrees

The upward walk matches git's own discovery on every case I could construct, with one gap and one residue, both recorded below as non-blocking.

The gitignore anchor

$ git check-ignore -q sub/.Codex/x ; echo $?
1
$ git check-ignore -q .Codex/x ; echo $?
0

Gates

$ python scripts/check_craftsmanship.py --base origin/main
Craftsmanship gate: OK
$ uvx [email protected] check .      -> All checks passed!
$ uvx [email protected] format --check . -> 3377 files already formatted
$ python -m pyright mcp_server/doctor.py -> 0 errors, 0 warnings, 0 informations
$ python scripts/check_doc_claims.py -> doc claims OK (1 declared not-a-claim exemption)
$ python scripts/check_project_wiki.py -> Project wiki mirrors match their canonical sources.
$ python scripts/craftsmanship_decisions.py -> exit 0

Longest consecutive comment block in the touched sources: 6 lines in tests_py/test_doctor_worktrees.py, 3 in mcp_server/doctor.py, under the 8-line decision threshold. ADR-1079 occupies one canonical file and one mirror, and sits between 1078 and 1080 in wiki/manifest.json without collision. The CHANGELOG entry is in [Unreleased].

Tests

$ pytest tests_py/test_doctor.py tests_py/test_doctor_worktrees.py tests_py/handlers/test_check_setup.py -q
40 passed in 5.86s

$ pytest <those three> tests_py/shared/test_subprocess_safe.py -q
51 passed in 13.86s

$ pytest -q -p no:randomly
8831 passed, 8 skipped, 2 warnings, 459 subtests passed in 674.85s (0:11:14)

Zero failures on the full suite at this head. The two tests_py/hooks/test_entry.py::test_entry_matches_launcher_on_benign_event failures the author reported do not reproduce here. I ran the controlled comparison rather than taking the "pre-existing, unrelated" reading on trust:

worktree at 28aea2af (PR head):        2 passed in 216.95s
worktree at 4bd7e7f0 (origin/main):   11 passed in 293.66s
primary checkout at 4bd7e7f0:          2 failed in 171.70s
    AssertionError: assert '## Cortex — ... as you go.\n' == ''
    + ## Cortex — First Run

Same interpreter in all three. The variable is the checkout, not the diff: the failure reproduces in the primary working copy at main and vanishes in a clean worktree at main. Blast radius checked directly, tests_py/hooks/test_entry.py and mcp_server/hooks/entry.py contain no reference to doctor. The deferral is therefore verified rather than asserted, but it wants an issue of its own: the parity test is sensitive to something in the primary checkout's local state, which is a latent false alarm for the next person who runs pytest there.

CI at this head

$ gh pr checks 602 --repo cdeust/Cortex --watch --fail-fast
Craftsmanship Gate  pass  16s
Lint                pass  18s
Type Check          pass  1m38s
Test (Python 3.10)  pass  5m43s
Test (Python 3.11)  pass  8m14s
Test (Python 3.12)  pass  8m32s
Test (Python 3.13)  pass  7m2s
Test (SQLite backend)          pass  6m44s
Test (Windows, SQLite backend) pass  5m51s
CI Green            pass  6s
[exited with code 0]

Every required job passes, the two Docker Build jobs and the scheduled fuzz batch skip as designed.

Non-blocking

A malformed .git entry still lands in the WARN branch with a repair instruction that cannot help. Measured in a directory holding an empty .git directory, git present on PATH:

$ git worktree list --porcelain -z
fatal: not a git repository (or any of the parent directories): .git
  [WARN] worktree locations (optional)    Unable to inspect worktree locations: Git unavailable or command failed.
     → Install Git and ensure it's on PATH; retry git worktree list --porcelain -z.

An empty .git file gives fatal: invalid gitfile format. The detail line's "or command failed" covers the case honestly and the check stays optional, so nothing is broken; it is the fix text that misleads, telling an owner who has git installed to install git. Conditioning that first clause on shutil.which("git") would keep the useful half of the message and drop the wrong half.

The probe reads the filesystem only, so it does not see GIT_DIR. A session started with GIT_DIR exported and no .git in the cwd's ancestry reports "not a git checkout" where git itself would have listed worktrees. That is a silent under-report on an advisory check, cheap to leave as is, worth knowing.

mcp_server/doctor.py goes from 394 to 548 lines against the project's 300-line cap. The gate is right to pass: .craftsmanship-baseline.json already carries a file-size entry for this file, and that entry records presence, not magnitude, so a file already over the cap can absorb any amount of new code invisibly. The four added functions form one cohesive unit and the tests are already split along exactly that seam as tests_py/test_doctor_worktrees.py; moving _worktree_list, _worktree_classification, _is_git_checkout, _under and _worktree_locations into mcp_server/doctor_worktrees.py would return doctor.py to its pre-PR size and cost nothing but an import. Worth a follow-up, not a reason to hold this merge.

Verdict

APPROVE at 28aea2afa76085d16cc6d7b01765dc69a4fe0814. The completion ledger's paths reconcile against the diff, every branch I enumerated maps to a named test, and the one deferral in the report is verified by measurement rather than by phrasing. File the parity-test issue against tests_py/hooks/test_entry.py, not against this branch.