PR #530 follow-ups: worktree guard residuals and a fourth vacuous assertion nf-9cqx ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/536 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #530. All advisory — none blocked that PR, which is CONVERGED. Filed unclaimed.

---

✅ Items 1–3 were taken in the PR itself (a7a9596) — kept for the record
  1. ensure_worktree treated the clone's own main worktree as "ours." git worktree list --porcelain always lists the main working tree first, so a path-only compare answered *yes* for the clone and took the reuse branch — checkout --detach -f, reset --hard, clean -fd, in the session's tree. Fixed with two independent guards: worktree_registered now requires the stanza to be detached, and ensure_worktree refuses path == root by name before any git call. Both mutation-pinned; re-verified independently (each half removed alone goes red).
  2. Half of test_the_FETCH_precedes_the_worktree_reset was vacuous. Now asserts against the worktree list read, which is unconditionally present, and drive()'s harness emits git's real listing shape (main working tree included).
  3. CLAUDE.md's probe_gate rationale was stale. Reason generalised from "this clone routinely sits on f5/integration" to "any non-main tree"; the rule itself unchanged.

---

Still open — residuals from the a7a9596 pass
4. A fourth vacuous assertion, in the test added for item 1

tools/tests/test_rebuild_f5.py, test_the_CLONE_ITSELF_is_refused_and_never_reset:

```python with self.assertRaises(rf5.WorktreeUnavailable): _, _, errs = capture(rf5.ensure_worktree, git, CLONE, CLONE) self.assertIn("points at the clone itself", errs) # <- never runs ```

capture() does not swallow the exception, so on the green path the assertIn is unreachable and errs is never bound. Verified by mutation: replacing the whole refusal message with err(f"✗ nope ({root}).") leaves all 65 tests green.

The functional pin is fine — assertRaises plus the zero-destructive-commands assertion are both red under either R1 mutation — so nothing is behaviourally unchecked. What is unchecked is the operator-facing message, which is the only thing that tells someone what they misconfigured. Fix is to move the assertion below the with block and bind errs there, as test_a_FILE_at_the_worktree_path_is_refused_not_crashed_on already does for the file it checks.

Notable only because it is the fourth instance of the same shape in this PR (assertNotIn("-x", …), the .uid file list, the fetch ordering, and now this), and it arrived in the commit whose message documents having found the third.

5. $NF_F5_WORKTREE pointing *inside* the clone is still accepted

rebuild_f5.py's own comment states the rule:

> The worktree lives BESIDE the clone, never inside it. Inside, Godot would scan it as project content — a nested checkout that .gdignore does not stop (#372) — and it would land in every git status the session runs.

The new guard compares path against root for exact equality, so $NF_F5_WORKTREE=<clone>/scratch passes it, is not registered, is an empty/absent directory, and git worktree add --detach creates a nested worktree happily. The clone's git status then shows it, and git add -A hits git's "adding embedded git repository" path — #503's contamination class, through the same env var item 1 was about, one step removed.

Lower severity than item 1 (pollution, not destruction of uncommitted work) and it needs a more deliberate misconfiguration, which is why it is here rather than on the PR. One Path(path).is_relative_to(root) — or os.path.commonpath — alongside the equality check closes it, and the rule is already written down in the module.

6. The foreign-directory refusal now misdescribes a state it can reach

Requiring detached means a worktree of this repo at the target path that happens to be on a branch falls through to:

``` ✗ {path} exists and is not a git worktree — refusing to touch it. Move it aside, or point $NF_F5_WORKTREE somewhere else. ```

It *is* a git worktree, and the advice is wrong for it. The reachable way in is ordinary: an operator sent to the worktree to resolve an unrecorded conflict does git checkout -b fixit out of habit, and every subsequent rebuild refuses with a message that tells them to move the directory aside. The right advice is the one the reuse-failure branch already prints — git worktree remove --force <path> — or simply git checkout --detach. Refusing is correct; only the wording is.

Dependencies

None.

Comments

No comments.

Add a comment