nflib: Git.is_clean() reports 'could not tell' as 'dirty', with a message that names the wrong cause nf-ca1d ← Beads

open priority 2 task unassigned

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

Filed unclaimed and unlabelled — backlog, for a future triage round. Found by hitting it, not by reading.

What happened

./tools/pr/rebuild_f5.py refused with

``` ✗ Uncommitted changes to tracked files — commit or stash first. ```

on a tree whose git status --short was a single untracked file. The refusal message names a condition that did not hold, and the file it was upset about is one the tool explicitly does not care about (untracked files are safe across the checkout and are left alone by design).

Why

The stray file was called head. On this case-insensitive filesystem that makes HEAD ambiguous:

``` $ git diff --quiet HEAD fatal: ambiguous argument 'HEAD': both revision and filename Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' $ echo $? 128 ```

nflib.git.Git.is_clean() is:

```python return self.raw(["diff", "--quiet", "HEAD"], check=False).ok ```

git diff --quiet has three outcomes — 0 clean, 1 dirty, 128 could not tell — and .ok collapses the last two. So "git could not answer" is reported as "the tree is dirty".

Severity: low, and the direction is right

It fails safe: an unanswerable check refuses to proceed rather than force-pushing over an unknown tree. Nothing was lost. The cost is a diagnostic that sends you looking for uncommitted work that does not exist — I went to git status, saw one untracked file, and had to reproduce the git invocation by hand to find the real cause.

It is also narrow: it needs a file whose name collides with a rev (head, main, a branch name) sitting untracked at the repo root. Agent clones accumulate exactly that kind of scratch file, which is how this one arrived — a stray head from a mistyped redirect.

Shape of a fix

is_clean() distinguishing the three, and callers deciding:

```python result = self.raw(["diff", "--quiet", "HEAD", "--"], check=False) ```

The -- alone fixes *this* instance — it disambiguates rev from path — and is probably worth doing regardless, since every is_clean() caller means "the revision". But the general case (128 from any cause: a corrupt index, no HEAD in a fresh repo) still collapses into "dirty", so the durable fix is a tri-state or an explicit raise on 128.

⚠️ Worth noting the fail-open/fail-closed direction before changing it: 128 -> dirty is the safe collapse for rebuild_f5 and f5, which refuse to stomp the tree. A caller that treated "clean" as the safe answer would want the opposite, and nflib has no such caller today. Do not "fix" it into fail-open.

Same family as several findings in the design-001 chunk (#492 F1, #520 F1, #522 F1): an exit code carries more states than the boolean it is read as, and the extra state is the one nobody tested.

Related: Git.is_clean's docstring already explains why it compares against HEAD rather than the index (#382 F2) — this is the next layer of the same call.

Dependencies

None.

Comments

No comments.

Add a comment