fix(guard): mind_commit_guard v1.3 — fail open on shell it cannot attribute (#155)

bin/mind_commit_guard.py

@@ -21,6 +21,30 @@
 (fail-open — a guard that misfires trains bypass-by-default). Escape hatch:
 ``PYAUTO_SKIP_MIND_GUARD=1`` in the environment or as a prefix in the command
 text (for the deliberate exceptional case, e.g. a bulk migration).
+
+v1.3 — fail open on shell this parser cannot attribute
+------------------------------------------------------
+Every live firing of this guard so far has been a false positive **on its own
+author**, and each came from the same root cause: it attributes a commit to a
+repo by parsing arbitrary shell, and the parser's model is ``cd X && git
+commit`` / ``git -C X commit``.
+
+  - v1.0 — regex clause split cut inside a quoted ``gh`` comment body.
+  - v1.1 — a ``cd`` away from Mind before committing was ignored (fixed v1.2).
+  - v1.2 — a ``cd`` inside a ``for``-loop *body* is not a clause-leading token,
+    so ``_cd_target`` never tracked it and the commit resolved to the ambient
+    Mind cwd.
+
+Three false positives, zero confirmed catches of a real bad Mind commit — the
+``-- <files>`` habit has done the actual work. Per
+``docs/agent_failure_modes.md`` §4 a noisy refusal trains bypass-by-default, so
+the guard is narrowed rather than patched again: **it may only DENY when it is
+confident**. When the command contains a construct the clause walk cannot
+follow — a ``for``/``while``/``until``/``case`` compound, a subshell or brace
+group, a function body, or a ``cd`` that is not a clause-leading token — the
+guard allows (see ``_unattributable``). The two high-confidence denials are
+unchanged: a ``git commit`` that unambiguously resolves to a Mind checkout with
+no ``--`` section, and a directory pathspec after ``--``.
 """
 
 from __future__ import annotations
@@ -80,6 +104,41 @@ def _clauses(command: str):
         yield clause
 
 
+# Shell keywords that open a compound the clause walk cannot follow: their
+# bodies are not clauses in this parser's sense, so a `cd` (or the commit
+# itself) inside one is attributed to the wrong repo.
+_COMPOUND_KEYWORDS = frozenset({"for", "while", "until", "select", "case", "esac", "function", "do", "done"})
+
+
+def _unattributable(command: str) -> bool:
+    """True when the command contains shell this parser cannot confidently
+    attribute to a repo — in which case the guard must allow (v1.3).
+
+    Three shapes, all of which produced a live false positive or would:
+
+    - a compound keyword (``for``/``while``/``case``/…): the body is not walked
+      as clauses, so a ``cd`` inside it is invisible (the v1.2 FP);
+    - a subshell ``(...)`` or brace group ``{...}``: same, plus the cwd change
+      is scoped to the group;
+    - a ``cd`` at a non-leading position in a clause: ``_cd_target`` only reads
+      ``tokens[0]``, so such a ``cd`` is silently dropped.
+
+    Quoted text (commit messages, ``gh`` bodies) is a single token by then, so
+    a message that merely *mentions* one of these words cannot trip it — and if
+    a token is ambiguous, the resolution is to allow, which is the safe
+    direction for a guard whose only justification is certainty.
+    """
+    for tokens in _clauses(command):
+        for i, tok in enumerate(tokens):
+            if tok in _COMPOUND_KEYWORDS:
+                return True
+            if tok in {"{", "}"} or "(" in tok or ")" in tok:
+                return True
+            if tok == "cd" and i != 0:
+                return True
+    return False
+
+
 def _under_mind(path: Path) -> Path | None:
     """If ``path`` is inside a PyAutoMind checkout, return that checkout root;
     else None."""
@@ -120,6 +179,11 @@ def check_command(command: str, cwd: str = "") -> str | None:
     # (a `cd`/`git -C` path) or in the ambient cwd. If neither, nothing to do.
     if MIND_MARKER not in command and MIND_MARKER not in (cwd or ""):
         return None
+    # Only deny when confident (v1.3): shell the clause walk cannot attribute
+    # is allowed rather than guessed at — every historical false positive was a
+    # guess of this kind.
+    if _unattributable(command):
+        return None
 
     effective_cwd: Path | None = Path(cwd) if cwd else None
 

docs/agent_failure_modes.md

@@ -95,6 +95,25 @@ Each: catalogue entries caught → why it fires at the decisive moment → cost
    check scoped to one repo; false-positive surface: deliberate multi-file
    registry commits, handled by listing the files (which is the desired
    behaviour anyway).
+
+   **Outcome (2026-07-22, v1.3) — the false-positive surface was the real
+   cost, and it was bigger than predicted.** Deployed as
+   `bin/mind_commit_guard.py`, the guard fired live **three times, every one a
+   false positive on its own author** (v1.0 a quoted `gh` body; v1.1 a `cd`
+   away from Mind; v1.2 a `cd` inside a `for`-loop body), against **zero
+   confirmed catches** of a real bad Mind commit — the `-- <files>` habit did
+   that work. Each FP came from the same place: attributing a commit to a repo
+   by parsing arbitrary shell. Since §4's own argument is that a noisy refusal
+   trains bypass-by-default, v1.3 narrows the guard to **deny only when
+   confident**: `_unattributable()` fails open on compounds (`for`/`while`/
+   `case`), subshells and brace groups, and any `cd` that is not
+   clause-leading. The two high-confidence denials (no `--` section; a
+   directory pathspec) are unchanged. Retiring the guard outright was
+   considered and rejected — the narrowed deny surface is cheap and still
+   covers the exact E1/F1 shape — but note the general lesson: **a refusal
+   whose trigger requires modelling an unbounded input space will spend its
+   budget on false positives.** Prefer refusals whose precondition is
+   *decidable* (mitigation 3's merged-branch check, which has true-positived).
 3. **Stale-claim auto-expiry (refusal-adjacent).** F4 shows completed tasks
    linger in `active.md` and block; the conflict guard catches it late but
    correctly. Cheaper at the source: `worktree_remove` (already the cleanup

tests/test_mind_commit_guard.py

@@ -144,3 +144,53 @@ def test_git_dash_C_into_mind_from_other_cwd_still_denied():
         'git -C /home/x/PyAutoMind commit -m "m"', cwd="/home/x/wt/PyAutoHands"
     )
     assert r is not None
+
+
+# --- v1.3: fail open on shell the clause walk cannot attribute --------------
+# Every live firing of the guard was a false positive on its own author. Each
+# of the three shapes below is allowed now; the narrow high-confidence denials
+# are re-asserted underneath so the narrowing cannot silently widen.
+
+
+def test_for_loop_with_inner_cd_is_allowed():
+    # The 3rd false positive (2026-07-17): the `cd` follows `do`, so it is not
+    # a clause-leading token and v1.2 resolved the commit to the ambient Mind
+    # cwd. These commits were all in workspace repos.
+    r = check_command(
+        'for r in autofit_workspace autolens_workspace; do cd /home/x/$r && '
+        'git commit -m "floors" -- config/general.yaml; done',
+        cwd="/home/x/PyAutoMind",
+    )
+    assert r is None
+
+
+def test_subshell_cd_then_commit_is_allowed():
+    r = check_command(
+        '(cd /home/x/wt/PyAutoHands && git commit -m "m")',
+        cwd="/home/x/PyAutoMind",
+    )
+    assert r is None
+
+
+def test_while_loop_commit_is_allowed():
+    r = check_command(
+        'while read -r f; do git commit -m "m" -- "$f"; done < list.txt',
+        cwd="/home/x/PyAutoMind",
+    )
+    assert r is None
+
+
+def test_compound_word_inside_quoted_message_does_not_fail_open(tmp_path):
+    # `for`/`done` inside a commit message are one shlex token, so they must
+    # NOT be read as compound keywords — the bare-commit denial still fires.
+    mind = tmp_path / "PyAutoMind"
+    mind.mkdir()
+    r = check_command(f'git -C {mind} commit -m "done waiting for the run"')
+    assert r is not None
+
+
+def test_simple_shapes_still_denied_after_v13(tmp_path):
+    mind = tmp_path / "PyAutoMind"
+    (mind / "active").mkdir(parents=True)
+    assert check_command(f'cd {mind} && git commit -m "m"') is not None
+    assert check_command(f'git -C {mind} commit -m "m" -- active/') is not None