diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index a9e9fded..7c620754 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -112,6 +112,68 @@ destructive script in this directory can be **execution-tested** repo); without it the only repository a test could reach was this one, so the branch-delete path was covered by review alone. (BACKLOG #1037.) +## "Will be permanently discarded" is an INDEX test, not a LOSS test — `recoverable.ps1` + +> **A `##`, not a `###` under "Remove one".** The question governs every way a tree goes away — +> `remove.ps1`, `prune-merged.ps1`, a rescue, the archive dialog, a manual directory delete — so +> nesting it under the one script it does *not* concern hides it from a reader scanning the headings +> for exactly this. + +**The archive dialog's warning about untracked files is wrong in the common case, and the common +case is a worktree branched behind `main`.** It reasons from *not in this worktree's index* straight +to *will be lost*, and skips the question that decides it: **is the content somewhere else.** + +A tree cut before a file landed does not have that file in its index. A copy of it sitting in the +tree is therefore untracked **there** while tracked on `main` — recoverable, and named in the +warning anyway. Every session branched behind `main` meets this, on every file that landed since, +and the prompt arrives exactly when a seat is trying to finish. + +Measured on this repository 2026-08-24, on a tree detached at `720f9436`, one commit before +`tests/test_ci_retry_native_crash.py` landed at `6e758a87`, holding `main`'s copy of that file: + +``` +git status --porcelain -> ?? tests/test_ci_retry_native_crash.py +git hash-object -> 5498a64cad057ed729d7220591246f75f2d21f15 +git rev-parse origin/main: -> 5498a64cad057ed729d7220591246f75f2d21f15 +``` + +Identical. The dialog called that permanent loss. + +**Answer it with [`recoverable.ps1`](../scripts/coord/recoverable.ps1) rather than by eye:** + +``` +pwsh -NoProfile -File scripts\coord\recoverable.ps1 # this worktree +pwsh -NoProfile -File scripts\coord\recoverable.ps1 -Worktree

-Json +``` + +It exits non-zero if any file is at risk. Each row carries a **binary `Verdict`** — the only thing a +caller has to act on — and a **`Reason`** from a closed set, because three of the four causes are +`AT-RISK` and "absent" is not the same problem as "modified": + +| `Verdict` | `Reason` | Means | +|---|---|---| +| `RECOVERABLE` | `identical` | byte-identical to the ref — the warning is wrong about this file | +| `AT-RISK` | `absent` | genuinely nowhere else | +| `AT-RISK` | `modified` | the path is on `main`, **the local edit is not** | +| `AT-RISK` | `unreadable` | the working copy could not be hashed — see the rules below | + +**The `modified` row is why an existence check is not enough.** "Is it on main" answers *yes* for a +file whose local edit is the only thing that would be lost. `Reason` is machine-readable on purpose: +without it a consumer would have to parse the `Detail` sentence, which interpolates the ref name. + +**Two rules it is built on, and both are about which way to be wrong.** + +- **Anything it cannot read is reported AT-RISK, never clean** — the same direction + [`occupancy.ps1`](../scripts/coord/occupancy.ps1) states for its own fence. A false `AT-RISK` + costs a look; a false `RECOVERABLE` costs the file. +- **`-NoFetch` is safe for the same reason.** A stale ref can only fail to contain something that + has since landed, so it can only move a file from `RECOVERABLE` to `AT-RISK`. It cannot invent a + match. The ref and its sha are printed with every run, because a verdict quoted without the ref it + was computed against cannot be re-checked. + +**The dialog is the Claude Code harness, not this repository's code.** Nothing here changes its +wording; this answers the question it raises but cannot itself answer. (BACKLOG #1298.) + ## Prune the finished ones — `prune-merged.ps1` Worktrees pile up. [`prune-merged.ps1`](../scripts/worktree/prune-merged.ps1) sweeps the finished diff --git a/scripts/coord/recoverable.ps1 b/scripts/coord/recoverable.ps1 new file mode 100644 index 00000000..450e1870 --- /dev/null +++ b/scripts/coord/recoverable.ps1 @@ -0,0 +1,204 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +<# +.SYNOPSIS + Answer whether this worktree's UNTRACKED files would actually be lost, which "untracked" does + not tell you (BACKLOG #1298). + +.DESCRIPTION + The archive dialog warns that untracked files "will be permanently discarded". That reasons from + "not in THIS WORKTREE'S INDEX" straight to "will be lost", and skips the only question that + decides it: is the content somewhere else. It is an INDEX test presented as a LOSS test. + + THE TWO ARE DIFFERENT FOR AN ORDINARY AND CONSTANT REASON. A worktree whose base is behind `main` + does not have the files that landed since it branched. Any copy of one of those files sitting in + the tree is untracked THERE while being tracked on `main` -- so the file is fully recoverable and + the warning is wrong. Every session branched behind `main` meets this, on every file that landed + since, and the prompt arrives at exactly the moment a seat is trying to finish. + + The mechanism was REPRODUCED on this repository before this script was written, on a worktree + detached one commit before a file landed, holding `main`'s copy of it: git called it untracked + while the two blob hashes were identical. **The measurement, with its shas, is stated ONCE in + docs/WORKTREES.md under "Will be permanently discarded" -- read it there.** Six sha citations + maintained in two files is how the two copies drift. + +.NOTES + THE DIALOG IS THE CLAUDE CODE HARNESS AND IS NOT THIS REPOSITORY'S CODE. Nothing here can change + its wording, and a change that tries has misread the item. This script exists so a human or a + session can answer the question the dialog raises but cannot itself answer. + + ANYTHING THIS SCRIPT CANNOT READ IS REPORTED AT RISK, NEVER CLEAN. That is the same direction + occupancy.ps1 states for its own fence -- an unplaceable record makes the whole fence unavailable, + because the failure that cannot be attributed is exactly the one that might be destroyed. Here the + cost of the two errors is not symmetric: a false AT RISK costs a look, a false RECOVERABLE costs + the file. + + NOT FETCHING ERRS THE SAME WAY, WHICH IS WHY -NoFetch IS SAFE. A stale `origin/main` can only + fail to contain something that has since landed, so it can only move a file from RECOVERABLE to + AT RISK. It cannot invent a match. The ref actually compared against is printed with every run, + because a verdict without the ref it was computed against is not a result. + +.PARAMETER Worktree + The worktree to examine. Defaults to the current one. + +.PARAMETER Ref + What to consider "somewhere else". Defaults to origin/main. + +.PARAMETER NoFetch + Skip the fetch. See the note above: this can only over-warn. + +.PARAMETER Json + Emit the rows as JSON instead of a table. + +.EXAMPLE + pwsh -NoProfile -File scripts\coord\recoverable.ps1 + pwsh -NoProfile -File scripts\coord\recoverable.ps1 -Worktree C:\path\to\wt -NoFetch +#> +[CmdletBinding()] +param( + [string]$Worktree, + [string]$Ref = "origin/main", + [switch]$NoFetch, + [switch]$Json +) + +$ErrorActionPreference = 'Stop' + +# NOTE ON $LASTEXITCODE, since every git call below leans on it: a non-zero exit is a RESULT here, +# not an error. `cat-file -e` exits 1 for a path that is simply absent from the ref, which is one of +# the three verdicts. $ErrorActionPreference='Stop' does not turn a native exit code into a throw, +# so the calls are made directly and their exit code read immediately after. + +if (-not $Worktree) { $Worktree = (Get-Location).Path } +if (-not (Test-Path -LiteralPath $Worktree)) { throw "no such worktree: $Worktree" } +$script:Root = (Resolve-Path -LiteralPath $Worktree).Path + +$top = & git -C $script:Root rev-parse --show-toplevel 2>$null +if ($LASTEXITCODE -ne 0 -or -not $top) { throw "not inside a git worktree: $script:Root" } +$script:Root = $top + +if (-not $NoFetch) { + & git -C $script:Root fetch origin --quiet 2>$null | Out-Null +} + +$refSha = & git -C $script:Root rev-parse --verify --quiet "$Ref" 2>$null +if (-not $refSha) { + throw "cannot resolve $Ref in $script:Root -- refusing to judge anything against a ref that does not exist" +} + +# --untracked-files=all, because the default collapses an untracked DIRECTORY to a single entry and +# every file beneath it would then go unexamined while the run still reported a clean verdict. +# -z, because a path may contain a space, and porcelain v1 QUOTES such paths rather than emitting +# them raw -- parsing the quoted form is a second, silently different unescaper. +$raw = & git -C $script:Root status --porcelain --untracked-files=all -z 2>$null +$entries = @() +if ($raw) { $entries = ($raw -split "`0") | Where-Object { $_ } } + +$rows = @() +foreach ($e in $entries) { + if ($e.Length -lt 4 -or $e.Substring(0, 2) -ne '??') { continue } + $path = $e.Substring(3) + + # RESET PER ITERATION. Load-bearing: without these, a hash from the PREVIOUS file survives into + # this row on any branch that does not assign it, and the row then reports a sha for a file + # nobody hashed. $verdict and $reason need no reset -- every branch below assigns both. + $wtHash = $null + $refHash = $null + + # ONE call, not two. `rev-parse :` already exits non-zero when the path is absent + # from the ref, so the `cat-file -e` existence probe that used to sit here asked a question this + # line answers on its way to the sha -- a third of the per-file work for nothing. Verified: + # `git rev-parse origin/main:no/such/path` exits 128, `origin/main:README.md` exits 0. + $refHash = & git -C $script:Root rev-parse "${Ref}:${path}" 2>$null + $onRef = ($LASTEXITCODE -eq 0 -and $refHash) + + if (-not $onRef) { + $refHash = $null + $verdict = 'AT-RISK' + $reason = 'absent' + } + else { + $wtHash = & git -C $script:Root hash-object -- "$path" 2>$null + if ($LASTEXITCODE -ne 0 -or -not $wtHash) { + # UNREADABLE COUNTS AS AT RISK. It is on the ref, so it is tempting to call it clean -- + # but we could not read the working copy, so we do not know it is the same content, and + # the one thing we must never do is tell someone a file is safe when we could not look. + $verdict = 'AT-RISK' + $reason = 'unreadable' + } + elseif ($wtHash -eq $refHash) { + $verdict = 'RECOVERABLE' + $reason = 'identical' + } + else { + $verdict = 'AT-RISK' + $reason = 'modified' + } + } + + # REASON IS THE MACHINE-READABLE ARM AND DETAIL IS THE SENTENCE. Verdict answers the only + # question a caller has to act on -- would this be lost -- and is deliberately BINARY. But three + # of its four causes are AT-RISK, and a consumer that wants to tell "absent" from "modified" + # would otherwise have to parse English with the ref name interpolated into it. Reason is a + # closed set: identical, absent, modified, unreadable. + $detail = switch ($reason) { + 'absent' { "absent from $Ref" } + 'modified' { "on $Ref but MODIFIED here" } + 'identical' { "identical to $Ref" } + 'unreadable' { 'UNREADABLE -- could not hash the working copy; treated as at risk, not as clean' } + } + + $rows += [pscustomobject]@{ + Path = $path + Verdict = $verdict + Reason = $reason + Detail = $detail + WorktreeSha = $wtHash + RefSha = $refHash + } +} + +$atRisk = @($rows | Where-Object { $_.Verdict -eq 'AT-RISK' }) +$recoverable = @($rows | Where-Object { $_.Verdict -eq 'RECOVERABLE' }) + +if ($Json) { + [pscustomobject]@{ + Worktree = $script:Root + Ref = $Ref + RefSha = $refSha + Fetched = (-not $NoFetch) + Untracked = $rows.Count + AtRisk = $atRisk.Count + Recoverable = $recoverable.Count + Rows = $rows + } | ConvertTo-Json -Depth 5 +} +else { + Write-Output "worktree : $script:Root" + # THE REF IS PART OF THE MEASUREMENT, not decoration. A verdict quoted without the ref it was + # computed against cannot be re-checked by whoever reads it. + Write-Output "compared : $Ref at $refSha$(if ($NoFetch) { ' (NOT fetched -- can only over-warn)' })" + Write-Output "" + if ($rows.Count -eq 0) { + Write-Output "no untracked files. Nothing for the archive warning to be about." + } + else { + foreach ($r in ($rows | Sort-Object Verdict, Path)) { + Write-Output ("{0,-12} {1}" -f $r.Verdict, $r.Path) + Write-Output (" {0}" -f $r.Detail) + } + Write-Output "" + Write-Output ("untracked {0}: {1} AT-RISK, {2} recoverable." -f $rows.Count, $atRisk.Count, $recoverable.Count) + if ($atRisk.Count -eq 0) { + Write-Output "Every untracked file here is already on $Ref, byte for byte. The archive warning is wrong about all of them." + } + else { + Write-Output "The AT-RISK rows are the only ones the archive warning actually describes. Commit or copy them first." + } + } +} + +# Exit 1 when anything is at risk, so this can be used as a check and not only read by a human. +# Nothing at risk -- including the no-untracked-files case -- exits 0. +if ($atRisk.Count -gt 0) { exit 1 } +exit 0 diff --git a/tests/test_coord_recoverable.py b/tests/test_coord_recoverable.py new file mode 100644 index 00000000..c7bc5bc9 --- /dev/null +++ b/tests/test_coord_recoverable.py @@ -0,0 +1,292 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Untracked is an INDEX fact; lost is a CONTENT fact (BACKLOG #1298). + +The archive dialog warns that untracked files "will be permanently discarded". It reasons from "not +in THIS WORKTREE'S index" straight to "will be lost", and skips the only question that decides it: +is the content somewhere else. + +The two come apart for an ordinary and constant reason. A worktree branched behind ``main`` does not +have the files that landed since, so a copy of one of them sitting in that tree is untracked THERE +while tracked on ``main`` -- fully recoverable, and the warning is wrong about it. + +**Every test below pairs the reassuring answer with the alarming one over the SAME command.** A +helper that always said RECOVERABLE would pass a suite that only ever fed it recoverable files, and +that is the failure mode that matters here: a false RECOVERABLE costs the file, while a false +AT-RISK costs a look. The three arms -- absent, modified, identical -- are asserted to produce three +different verdicts, so the cases cannot funnel to one assertion. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +SCRIPT = ROOT / "scripts" / "coord" / "recoverable.ps1" +TIMEOUT = 120 + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None or os.name != "nt", + reason="recoverable.ps1 needs pwsh on Windows", +) + + +def git(repo: Path, *args: str) -> str: + proc = subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=True, + ) + return proc.stdout + + +def run_helper(worktree: Path, ref: str = "origin/main") -> tuple[int, dict[str, object]]: + """Drive the REAL script as a subprocess, as -Json, and return (exit code, payload).""" + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(SCRIPT), + "-Worktree", + str(worktree), + "-Ref", + ref, + "-NoFetch", + "-Json", + ], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + assert proc.stdout.strip(), f"helper produced no stdout:\n{proc.stderr}" + return proc.returncode, dict(json.loads(proc.stdout)) + + +def by_path(payload: dict[str, object], field: str) -> dict[str, str]: + """Map path -> one field of its row. One unpacker, so the isinstance dance is written once.""" + rows = payload["Rows"] + assert isinstance(rows, list) + out: dict[str, str] = {} + for r in rows: + out[str(r["Path"])] = str(r[field]) + return out + + +def verdicts(payload: dict[str, object]) -> dict[str, str]: + return by_path(payload, "Verdict") + + +def reasons(payload: dict[str, object]) -> dict[str, str]: + return by_path(payload, "Reason") + + +@pytest.fixture +def upstream_and_behind(tmp_path: Path) -> tuple[Path, Path]: + """An 'origin' whose main carries a file, and a clone parked one commit BEHIND it. + + This is the shape the item is about, built rather than described: the behind-tree genuinely does + not have the later file in its index, which is what makes git call a copy of it untracked. + """ + up = tmp_path / "up" + up.mkdir() + git(up, "init", "-q", "-b", "main") + git(up, "config", "user.email", "t@example.invalid") + git(up, "config", "user.name", "t") + (up / "base.txt").write_text("base\n", encoding="utf-8") + git(up, "add", "base.txt") + git(up, "commit", "-qm", "base") + + (up / "landed.txt").write_text("landed content\n", encoding="utf-8") + git(up, "add", "landed.txt") + git(up, "commit", "-qm", "land a file") + + behind = tmp_path / "behind" + git(tmp_path, "clone", "-q", str(up), str(behind)) + git(behind, "config", "user.email", "t@example.invalid") + git(behind, "config", "user.name", "t") + # Park one commit behind: this tree's index has no landed.txt. + first = git(behind, "rev-list", "--max-parents=0", "HEAD").strip() + git(behind, "checkout", "-q", "--detach", first) + assert not (behind / "landed.txt").exists() + return up, behind + + +def test_a_landed_file_in_a_behind_worktree_is_recoverable_not_lost( + upstream_and_behind: tuple[Path, Path], +) -> None: + """The reassuring arm, and the one the dialog gets wrong.""" + _, behind = upstream_and_behind + (behind / "landed.txt").write_text("landed content\n", encoding="utf-8") + + assert "?? landed.txt" in git(behind, "status", "--porcelain"), ( + "precondition: git must call this untracked, or the test is not about the reported case" + ) + + code, payload = run_helper(behind) + print(json.dumps(payload, indent=2)) + assert verdicts(payload)["landed.txt"] == "RECOVERABLE" + assert payload["AtRisk"] == 0 + assert code == 0, "nothing at risk must exit 0, so this is usable as a check" + + +def test_a_file_absent_from_the_ref_is_at_risk( + upstream_and_behind: tuple[Path, Path], +) -> None: + """The alarming arm. Same command, same tree, opposite answer.""" + _, behind = upstream_and_behind + (behind / "genuinely_new.txt").write_text("nobody else has this\n", encoding="utf-8") + + code, payload = run_helper(behind) + print(json.dumps(payload, indent=2)) + assert verdicts(payload)["genuinely_new.txt"] == "AT-RISK" + assert payload["AtRisk"] == 1 + assert code == 1, "anything at risk must exit non-zero" + + +def test_a_file_on_the_ref_but_locally_modified_is_at_risk( + upstream_and_behind: tuple[Path, Path], +) -> None: + """The arm a name-only or existence-only check would miss. + + The path IS on the ref, so 'is it on main' answers yes and a helper that stopped there would + call it recoverable. The local edit is the thing that would actually be lost. + """ + _, behind = upstream_and_behind + (behind / "landed.txt").write_text("landed content\nplus a local edit\n", encoding="utf-8") + + code, payload = run_helper(behind) + print(json.dumps(payload, indent=2)) + assert verdicts(payload)["landed.txt"] == "AT-RISK" + # The reason must be `modified`, NOT `absent`. Both are AT-RISK, so Verdict alone cannot tell + # this test from the one above it -- and an existence-only check would have said RECOVERABLE. + got_reasons = reasons(payload) + assert got_reasons["landed.txt"] == "modified", ( + f"expected the on-ref-but-edited reason, got {got_reasons}; if this says 'absent' the ref " + "lookup is failing and every file would be reported at risk for the wrong reason" + ) + assert code == 1 + + +def test_the_three_arms_do_not_collapse_to_one_answer( + upstream_and_behind: tuple[Path, Path], +) -> None: + """All three at once: a suite whose cases all produce the same string proves nothing. + + This is the test that would catch a helper hard-coded to one verdict, which every single-arm + test above would pass individually. + """ + _, behind = upstream_and_behind + (behind / "landed.txt").write_text("landed content\n", encoding="utf-8") + (behind / "genuinely_new.txt").write_text("nobody else has this\n", encoding="utf-8") + (behind / "base.txt").write_text("base\nlocally edited\n", encoding="utf-8") + + code, payload = run_helper(behind) + got = verdicts(payload) + print(json.dumps(payload, indent=2)) + + assert got["landed.txt"] == "RECOVERABLE" + assert got["genuinely_new.txt"] == "AT-RISK" + + # AND THE REASONS MUST DIFFER, not just the verdicts. Verdict is deliberately BINARY -- three of + # its four causes are AT-RISK -- so a suite that only asserts Verdict cannot tell "absent from + # the ref" from "on the ref but modified", which is the distinction the docs call the whole + # point. Without this block a consumer would have to parse English with the ref interpolated + # into it, and nothing here would notice if Reason collapsed to one value. + got_reasons = reasons(payload) + assert got_reasons["landed.txt"] == "identical" + assert got_reasons["genuinely_new.txt"] == "absent" + assert len(set(got_reasons.values())) == 2, f"the reasons collapsed to one value: {got_reasons}" + + # base.txt is TRACKED and modified, so it is not untracked and not this script's subject. + assert "base.txt" not in got, ( + "a tracked-but-modified file is not what the archive warning is about; including it would " + "widen the report past the population the item names" + ) + assert payload["AtRisk"] == 1 + assert payload["Recoverable"] == 1 + assert code == 1 + + +def test_an_untracked_directory_is_expanded_rather_than_reported_as_one_entry( + upstream_and_behind: tuple[Path, Path], +) -> None: + """git's default porcelain collapses an untracked DIRECTORY to a single entry. + + Every file beneath it would then go unexamined while the run still printed a verdict, which is + the silent-undercount shape. --untracked-files=all is what prevents it, and this pins that. + """ + _, behind = upstream_and_behind + nested = behind / "newdir" / "deeper" + nested.mkdir(parents=True) + (nested / "a.txt").write_text("a\n", encoding="utf-8") + (nested / "b.txt").write_text("b\n", encoding="utf-8") + + code, payload = run_helper(behind) + got = verdicts(payload) + print(json.dumps(payload, indent=2)) + + assert "newdir/deeper/a.txt" in got, f"the directory was not expanded: {sorted(got)}" + assert "newdir/deeper/b.txt" in got, f"the directory was not expanded: {sorted(got)}" + assert payload["Untracked"] == 2 + assert code == 1 + + +def test_the_ref_it_compared_against_is_reported( + upstream_and_behind: tuple[Path, Path], +) -> None: + """A verdict without the ref it was computed against cannot be re-checked by its reader.""" + _, behind = upstream_and_behind + (behind / "landed.txt").write_text("landed content\n", encoding="utf-8") + + _, payload = run_helper(behind) + assert payload["Ref"] == "origin/main" + sha = str(payload["RefSha"]) + assert len(sha) == 40, f"expected a full sha, got {sha!r}" + assert sha == git(behind, "rev-parse", "origin/main").strip() + + +def test_an_unresolvable_ref_refuses_rather_than_reporting_everything_clean( + upstream_and_behind: tuple[Path, Path], +) -> None: + """The direction that matters: cannot-compare must never render as nothing-to-worry-about. + + If the ref does not resolve, every file is 'absent from the ref' by construction -- which would + be the correct AT-RISK answer for the wrong reason, and would look identical to a real one. The + script refuses the run instead. + """ + _, behind = upstream_and_behind + (behind / "landed.txt").write_text("landed content\n", encoding="utf-8") + + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(SCRIPT), + "-Worktree", + str(behind), + "-Ref", + "origin/no-such-branch-zzq", + "-NoFetch", + "-Json", + ], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + assert proc.returncode != 0, "an unresolvable ref must not exit 0" + combined = proc.stdout + proc.stderr + assert "refusing" in combined.lower(), f"expected an explicit refusal, got:\n{combined}" diff --git a/tests/tooling_manifest.txt b/tests/tooling_manifest.txt index 38371023..3dc18ad0 100644 --- a/tests/tooling_manifest.txt +++ b/tests/tooling_manifest.txt @@ -62,6 +62,7 @@ tests/test_coord_overlap_attribution.py tests/test_coord_overlap_cache.py tests/test_coord_overlap_signals.py tests/test_coord_presence.py +tests/test_coord_recoverable.py tests/test_coord_dispatch_gate.py tests/test_coord_seat_clock_alarm.py tests/test_coord_throughput.py