From 9d18cc31984759ecd784e32099a929df2ee0b5eb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 10:22:31 +0000 Subject: [PATCH 1/2] fix(hygiene): never condemn the checked-out branch; survive a missing root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two guard bugs found by a full hygiene run. **tidy condemned the branch being worked on.** Both the counting leg (prescan_tidy) and the plan leg (enumerate_condemn_candidates) filtered only the literal names main/master/HEAD and the default branch, never the branch HEAD actually points at. A freshly cut branch is an ancestor of origin/ until its first commit, so it always read merged=yes and was reported as "recommend straight delete" — on a real run this recommended deleting the session's own working branch in all 9 scanned repos. Since the tidy path files condemned entries async with no per-item gate, that is a bad thing to get wrong. Both legs now skip HEAD's branch; the guard is narrow, so a merged branch that is not checked out is still condemned as before. **refs crashed on a missing scan root.** Resolver.__init__ called root.iterdir() unguarded, raising FileNotFoundError when PYAUTO_ROOT does not point at a real directory — and in the default all-mode scan the traceback printed twice, burying the rest of the worklist. It now degrades to a "not scannable here" row, matching what the extras and config modes already do. Regression tests cover both, including the narrowness of the tidy guard and the all-mode scan that produced the double traceback. Full suite: 209 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HySLvBY16YnStJKHdDCCbU --- agents/conductors/hygiene/_hygiene_refs.py | 20 ++++- agents/conductors/hygiene/hygiene.sh | 16 +++- tests/test_hygiene_conductor.py | 88 ++++++++++++++++++++++ 3 files changed, 120 insertions(+), 4 deletions(-) diff --git a/agents/conductors/hygiene/_hygiene_refs.py b/agents/conductors/hygiene/_hygiene_refs.py index acba3b21..9f5a2017 100644 --- a/agents/conductors/hygiene/_hygiene_refs.py +++ b/agents/conductors/hygiene/_hygiene_refs.py @@ -345,7 +345,11 @@ class Resolver: def __init__(self, root: Path): self.root = root self.indexes: dict[str, RepositoryIndex] = {} - self.siblings = {path.name for path in root.iterdir() if path.is_dir()} + self.siblings = ( + {path.name for path in root.iterdir() if path.is_dir()} + if root.is_dir() + else set() + ) self.suppressed = 0 def index(self, name: str) -> RepositoryIndex | None: @@ -550,6 +554,20 @@ def summary_for(findings: list[Finding], repository_count: int, skipped: int) -> def row_for(root: Path) -> dict: + # An absent scan root is "nothing to say", not a crash. The extras and + # config modes already degrade this way; refs used to let the missing + # directory surface as a FileNotFoundError traceback, which in the default + # all-mode scan printed twice and buried the rest of the worklist. + if not root.is_dir(): + return { + "mode": "refs", + "kind": "finding", + "status": "clean", + "count": 0, + "summary": f"not scannable here: {root} is not present", + "delegate": "/refactor", + "findings": [], + } findings, repository_count, skipped = scan(root) return { "mode": "refs", diff --git a/agents/conductors/hygiene/hygiene.sh b/agents/conductors/hygiene/hygiene.sh index 8d077d7d..e27e5057 100755 --- a/agents/conductors/hygiene/hygiene.sh +++ b/agents/conductors/hygiene/hygiene.sh @@ -122,9 +122,13 @@ prescan_tidy() { dir="$ROOT/$repo" [[ -d "$dir/.git" || -f "$dir/.git" ]] || continue scanned=$((scanned + 1)) - local b s g + local b s g cur + # The branch HEAD points at is the one being WORKED ON — never debris. + cur=$(git -C "$dir" symbolic-ref --quiet --short HEAD 2>/dev/null) + local -a skip=(-e main -e master -e HEAD) + [[ -n "$cur" ]] && skip+=(-e "$cur") b=$(git -C "$dir" for-each-ref --format='%(refname:short)' refs/heads 2>/dev/null \ - | grep -vxE 'main|master|HEAD' | wc -l | tr -d ' ') + | grep -vxF "${skip[@]}" | wc -l | tr -d ' ') s=$(git -C "$dir" stash list 2>/dev/null | wc -l | tr -d ' ') g=$(git -C "$dir" branch -vv 2>/dev/null | grep -c '\[gone\]' || true) branches=$((branches + b)); stashes=$((stashes + s)); gone=$((gone + g)) @@ -417,15 +421,21 @@ fi # status vs the default branch) and stashes across the managed checkouts. Echoes # TSV rows: "\t\t\t". enumerate_condemn_candidates() { - local repo dir def br + local repo dir def br cur for repo in "${LIB_REPOS[@]}" "${ORG_REPOS[@]}"; do dir="$ROOT/$repo" [[ -d "$dir/.git" || -f "$dir/.git" ]] || continue def=$(git -C "$dir" symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null | sed 's@^origin/@@') [[ -n "$def" ]] || def=main + # The branch HEAD points at is the one being WORKED ON — never condemn it. + # A freshly cut branch is an ancestor of origin/ until its first + # commit, so without this it always reads merged=yes and is recommended for + # straight delete: every agent session would be told to bin its own branch. + cur=$(git -C "$dir" symbolic-ref --quiet --short HEAD 2>/dev/null) while IFS= read -r br; do [[ -z "$br" ]] && continue case "$br" in main|master|HEAD|"$def") continue ;; esac + [[ -n "$cur" && "$br" == "$cur" ]] && continue local merged=no if git -C "$dir" merge-base --is-ancestor "$br" "origin/$def" 2>/dev/null \ || git -C "$dir" merge-base --is-ancestor "$br" "$def" 2>/dev/null; then diff --git a/tests/test_hygiene_conductor.py b/tests/test_hygiene_conductor.py index 059320c3..3af09966 100644 --- a/tests/test_hygiene_conductor.py +++ b/tests/test_hygiene_conductor.py @@ -832,3 +832,91 @@ def test_refs_findings_reach_the_default_worklist(tmp_path): assert result.returncode == 0, result.stderr rows = {row["mode"]: row for row in json.loads(result.stdout)["rows"]} assert rows["refs"]["count"] == len(_refs_row(tmp_path)["findings"]) > 0 + + +def _init_git_repo_with_branch(path, branch, default="main"): + """A checkout whose HEAD is `branch`, cut from an unpushed `default`. + + Mirrors the state every agent session starts in: the working branch exists + locally, shares its tip with the default branch, and has no commits of its + own yet. + """ + _init_git_repo(path) + run = lambda *a: subprocess.run( # noqa: E731 + ["git", "-C", str(path), *a], check=True, capture_output=True + ) + run("config", "user.email", "t@example.com") + run("config", "user.name", "t") + run("checkout", "-q", "-b", default) + (path / "f.txt").write_text("x\n") + run("add", "f.txt") + run("commit", "-q", "-m", "base") + run("checkout", "-q", "-b", branch) + + +def test_tidy_never_condemns_the_checked_out_branch(tmp_path): + # Regression: a freshly cut agent branch is an ancestor of the default + # branch until its first commit, so it reads merged=yes. Without a HEAD + # guard the conductor recommends "straight delete" of the branch the + # session is actively working on. + _init_git_repo_with_branch( + tmp_path / "PyAutoBrain", "claude/some-agent-run-abc123" + ) + + result = _run(["tidy", "--json"], tmp_path) + + assert result.returncode == 0, result.stderr + doc = json.loads(result.stdout) + assert [c["locator"] for c in doc["candidates"]] == [] + + +def test_tidy_prescan_does_not_count_the_checked_out_branch(tmp_path): + # The same guard on the counting leg, so the ranked worklist and the + # condemn plan cannot disagree about what is debris. + _init_git_repo_with_branch( + tmp_path / "PyAutoBrain", "claude/some-agent-run-abc123" + ) + + result = _run(["--json"], tmp_path) + rows = {row["mode"]: row for row in json.loads(result.stdout)["rows"]} + assert rows["tidy"]["count"] == 0 + assert "0 stale branches" in rows["tidy"]["summary"] + + +def test_tidy_still_condemns_a_branch_that_is_not_checked_out(tmp_path): + # The guard is narrow: only HEAD is spared, not every merged branch. + repo = tmp_path / "PyAutoBrain" + _init_git_repo_with_branch(repo, "claude/some-agent-run-abc123") + subprocess.run( + ["git", "-C", str(repo), "branch", "feature/spent"], + check=True, + capture_output=True, + ) + + doc = json.loads(_run(["tidy", "--json"], tmp_path).stdout) + + assert [c["locator"] for c in doc["candidates"]] == ["feature/spent"] + + +def test_refs_reports_not_scannable_for_a_missing_root(tmp_path): + # An absent scan root is "nothing to say", matching extras/config — not a + # FileNotFoundError traceback printed into the middle of the worklist. + missing = tmp_path / "not-a-checkout-root" + + result = _run(["refs", "--json"], missing) + + assert result.returncode == 0, result.stderr + row = json.loads(result.stdout)["row"] + assert row["count"] == 0 and row["status"] == "clean" + assert "not scannable" in row["summary"] + assert "Traceback" not in result.stderr + + +def test_default_scan_survives_a_missing_root(tmp_path): + # The all-mode scan is where the traceback used to print twice. + result = _run(["--json"], tmp_path / "not-a-checkout-root") + + assert result.returncode == 0, result.stderr + assert "Traceback" not in result.stderr + rows = {row["mode"]: row for row in json.loads(result.stdout)["rows"]} + assert rows["refs"]["count"] == 0 From 566fa731ed91d5595fd53b6b58f1e75b676b833d Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Wed, 26 Aug 2026 16:56:32 -0400 Subject: [PATCH 2/2] test(hygiene): drop the two tests for the superseded missing-root guard test_refs_reports_not_scannable_for_a_missing_root and test_default_scan_survives_a_missing_root are branch-only (neither exists on main) and assert the status=clean behaviour this merge dropped. main's test_json_stays_parseable_when_a_helper_crashes asserts the opposite contract -- a missing root is an error row, not a clean one -- and that is the one the Brain board's cloud render depends on. Full PyAutoBrain suite: 557 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QsDakzvrNxSAH1cfvW6nQm --- tests/test_hygiene_conductor.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/tests/test_hygiene_conductor.py b/tests/test_hygiene_conductor.py index 5ddb9a52..4e92bd6b 100644 --- a/tests/test_hygiene_conductor.py +++ b/tests/test_hygiene_conductor.py @@ -1046,30 +1046,6 @@ def test_tidy_still_condemns_a_branch_that_is_not_checked_out(tmp_path): assert [c["locator"] for c in doc["candidates"]] == ["feature/spent"] -def test_refs_reports_not_scannable_for_a_missing_root(tmp_path): - # An absent scan root is "nothing to say", matching extras/config — not a - # FileNotFoundError traceback printed into the middle of the worklist. - missing = tmp_path / "not-a-checkout-root" - - result = _run(["refs", "--json"], missing) - - assert result.returncode == 0, result.stderr - row = json.loads(result.stdout)["row"] - assert row["count"] == 0 and row["status"] == "clean" - assert "not scannable" in row["summary"] - assert "Traceback" not in result.stderr - - -def test_default_scan_survives_a_missing_root(tmp_path): - # The all-mode scan is where the traceback used to print twice. - result = _run(["--json"], tmp_path / "not-a-checkout-root") - - assert result.returncode == 0, result.stderr - assert "Traceback" not in result.stderr - rows = {row["mode"]: row for row in json.loads(result.stdout)["rows"]} - assert rows["refs"]["count"] == 0 - - # --- Body-map-derived coverage ------------------------------------------------- # # The conductor scans repositories, so WHICH repositories must come from the body