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.
---
a7a9596) — kept for the recordensure_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).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).f5/integration" to "any non-main tree"; the rule itself unchanged.---
a7a9596 passtools/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.
$NF_F5_WORKTREE pointing *inside* the clone is still acceptedrebuild_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.
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.
None.
No comments.