fix(verification): stop QA freshness loop in submodule monorepos

[halindrome]

What

Stop the QA verification lifecycle from looping forever in git-submodule monorepos. verification_is_stale() (scripts/verification-freshness.sh) no longer treats submodule gitlink pointer drift, untracked parent files, or Claude Code's own local config as "the verified product changed."

Closes #652.

Why

In a submodule monorepo the real code lives inside the submodules, so the parent tree is perpetually dirty from two sources that are not the verified work:

1. Submodule gitlink pointer drift — whenever a submodule HEAD moves (normal during development), the parent shows M <submodule>.
2. Untracked files — .env.*, build markers, and notably .claude/settings.local.json (Claude Code's own per-project config, rewritten on permission grants).

The old whole-parent-tree git status --porcelain --untracked-files=normal therefore never returned empty, so every VERIFICATION.md was judged working_tree_changed → stale → QA re-runs → repeat, even on a fully committed repo. Observed live in a 6-submodule monorepo: with zero uncommitted code, all 7 VERIFICATION.md files still reported STALE [working_tree_changed] from 5 pointer entries + 3 untracked files.

How

• When .gitmodules is present, the dirty check tracks real content, not parent bookkeeping: ignore parent gitlink pointers (--ignore-submodules=all) and untracked files on the top-level repo, and treat uncommitted tracked content inside submodules (git submodule foreach --quiet --recursive 'git status --porcelain --untracked-files=no') — or in non-submodule top-level files — as the dirty signal.
• Single-repo projects keep the original whole-tree behavior unchanged; git errors continue to fail closed to stale.
• Exclude Claude Code's own local config (.claude/settings.local.json, .claude/settings.json) everywhere the freshness pathspec is used and in write-verification.sh's verified_at_commit (so write/read stay consistent). Scoped to the settings files only — project deliverables under .claude/ (commands, hooks, agents, skills) still count toward freshness, so Claude-tooling repos aren't silently skipped.

Design note (deliberate): the monorepo branch is gated on auto-detection of .gitmodules, not on the existing monorepo_routing config key. Freshness correctness must not depend on a routing toggle a user could disable while still having submodules — a submodule monorepo with monorepo_routing=false would otherwise still loop.

Testing

• New tests/verification-freshness.bats — 7 behavioral cases built on real git submodule fixtures: single-repo untracked → stale (original behavior preserved); monorepo clean → fresh; untracked parent → fresh; pointer drift → fresh; in-submodule content → stale; .claude/settings.local.json churn → fresh; tracked .claude/commands/*.md change → stale.
• bash -n + shellcheck -S warning clean on both changed scripts.
• All 7 bats cases pass.

🤖 Generated with Claude Code

[halindrome]

QA Review — Round 1 (against the base fix; addressed in 168e65af)

- Model used: Claude Opus 4.8 (read-only Code Reviewer agent, analysis-only)

- What was tested:
  • The submodule-aware dirty check in verification_is_stale() (the .gitmodules branch,
    --ignore-submodules=all on parent, `git submodule foreach --recursive` on submodules).
  • Read/write pathspec symmetry between write-verification.sh and verification-freshness.sh.
  • The .claude/settings*.json exclusion scoping.
  • Live edge cases: untracked-in-submodule, staged/untracked parent files, empty .gitmodules,
    pointer drift; bats suite, bash -n, shellcheck -S warning, trailing newline.

- Expected vs actual:
  1. Untracked (never-committed) files INSIDE a submodule are reported FRESH — genuinely new
     uncommitted product code escapes the dirty check. The parent's --untracked-files=no is
     justified (parent untracked is noise), but inside a submodule the untracked file generally
     IS the work product. Live repro: adding sub/newfile.go to a clean monorepo → fresh. This is
     the inverse of the #652 driver (committed code looping) and is a real false-negative. No test
     covered untracked-in-submodule.
  2. The `.gitmodules` presence — not initialized submodules — gates the whole submodule branch.
     An empty/stale .gitmodules routes a de-facto single repo through the submodule path, silently
     switching parent untracked files from stale → ignored. Live repro: empty committed .gitmodules
     + untracked top-level file → fresh, vs stale without .gitmodules.
  3. The monorepo "untracked parent treated as noise" asymmetry vs the single-repo branch is
     intended but only tested with .env.local (noise), not asserted as a contract.

- Severity: Finding 1 major; Findings 2, 3 minor.

- Confirmed vs hypothetical:
  • Finding 1: CONFIRMED (live repro).  • Finding 2: CONFIRMED (live repro).  • Finding 3: CONFIRMED.

  Non-issues verified: read/write pathspec symmetry correct; --ignore-submodules=all suppresses
  pointer drift correctly; git errors fail closed to stale; exclusion scoping correct
  (.claude/settings*.json excluded, .claude/commands/* deliverable still stale); shellcheck/bash -n
  clean; bats file has a trailing newline.

Addressed in 168e65af (fix(verification): address QA round 1): switched the submodule foreach to --untracked-files=normal (honors each submodule's .gitignore via --exclude-standard; excludes .claude/settings*.json local-config noise); gated the monorepo branch on git submodule status non-empty (actual submodules, not a leftover file); added tests (untracked-new-file-in-submodule → stale; gitignored & local-config noise in submodule → fresh; empty .gitmodules → single-repo).

[halindrome]

QA Review — Round 2 (fresh review against 168e65af; clean, one optional test added in 58a1227a)

- Model used: Claude Opus 4.8 (read-only Code Reviewer agent)

- What was tested:
  • The 2 base commits + round-1 fix; full diff of the 3 changed files.
  • Round-1 verification: (a) the nested double-quoted `:!` pathspec inside the single-quoted
    `git submodule foreach` string; (b) the monorepo gate change to `git submodule status`
    non-empty; (c) read/write pathspec symmetry.
  • Live probes on git 2.50.1: real nested submodule (sub-within-sub) with --recursive;
    uninitialized submodule via clone-without-recurse; pathspec exclusion inside submodules;
    fail-closed-to-stale error handling. bats 11/11, bash -n, shellcheck -S warning, trailing newline.

- Expected vs actual: no discrepancies found.
  1. Monorepo gate (round-1 finding #2) — RESOLVED; empty/stale .gitmodules keeps single-repo path
     (test passes). 2. Nested double-quoted :! pathspec under foreach — parses correctly under sh -c
     per submodule, evaluated relative to each submodule's own root; exclusion verified inside a
     nested submodule. 3. Uninitialized/declared-but-not-cloned submodule — `git submodule status`
     lists it (monorepo path), `git submodule foreach` silently SKIPS it (rc=0), so a clean clone
     reports fresh and does NOT spuriously fail closed. Correct, but had no dedicated test.
     4. Read/write pathspec symmetry — consistent (writer records parent verified_at_commit; read
     side's dirty-check is a separate gate). 5. --ignore-submodules=all does not mask tracked
     top-level work. 6. Fail-closed semantics preserved; library sets no shell options (safe to source).

- Severity: zero critical, zero major. One minor item (finding 3) is a test-coverage suggestion only.

- Confirmed vs hypothetical: all CONFIRMED via live execution (bats 11/11, nested + uninitialized
  probes, lint clean).

ROUND 2 IS CLEAN. No blockers, no new gaps. Optional: add a bats case for an uninitialized/uncloned
submodule to lock in the finding-3 behavior.

Addressed in 58a1227a (fix(verification): address QA round 2): added the suggested bats case — a declared-but-uninitialized submodule (clone without --recurse-submodules) takes the monorepo path but git submodule foreach skips it, so the clean clone correctly reports fresh. 12/12 pass.

[halindrome]

QA Review — Round 3 (fresh review against 58a1227a; clean for merge; boundary documented in d441885d)

- Model used: Claude Opus 4.8 (read-only Code Reviewer agent)

- What was tested:
  • 4 commits + full diff of the 3 changed files; bats 12/12; bash -n + shellcheck -S warning clean;
    bats trailing newline present.
  • Read/write pathspec symmetry (verified_at_commit baseline vs read-side _cur_commit).
  • Live git fixtures probing false-fresh AND false-stale: committed-in-submodule work with and
    without parent pointer bump; timestamp-fallback branch; uninitialized submodule; :! pathspec
    honored inside `git submodule foreach`.
  • Sourcing / set -euo pipefail safety + every live caller (hard-gate.sh, phase-detect.sh,
    vbw-statusline.sh) and their return-code consumption; git-version portability.

- Expected vs actual:
  1. Residual false-fresh: code COMMITTED inside a submodule but with the parent gitlink pointer
     not yet bumped reads as fresh (submodule tree clean → empty; parent shows only `M sub` pointer
     drift, intentionally ignored; parent product commit unchanged → fresh). This is the direct,
     inherent trade-off of the #652 fix: ignoring pointer drift to break the loop necessarily blinds
     the baseline to committed-but-not-bumped submodule work. Once the pointer is bumped the parent
     gets a new product commit and the baseline correctly reports stale (verified). Uncommitted
     submodule work (tracked + untracked-new) is correctly caught. Narrow, sequence-dependent; not a
     regression vs upstream (upstream over-reported stale here — the loop being fixed).
     Severity: minor. Confirmed (live, deterministic). Suggestion: document the boundary.

  No other discrepancies: read/write commit baselines symmetric (no perpetual mismatch loop);
  `:!` excludes honored inside foreach; uninitialized submodule → fresh (test 10); git errors fail
  closed to stale on both legs; library sets no shell options and all 3 callers use set -u + a
  fail-closed fallback stub and consume the function in an `if`; empty/stale .gitmodules cannot flip
  a single repo onto the monorepo path (test 9).

- Severity: one minor (inherent design boundary, no regression).
- Confirmed vs hypothetical: Finding 1 CONFIRMED (live fixture); all no-discrepancy items CONFIRMED.

ROUND VERDICT: CLEAN FOR MERGE. No blockers, no major/critical. The single minor false-fresh gap is
the deliberate boundary of ignoring submodule pointer drift; worth a one-line contract note.

Addressed in d441885d (fix(verification): address QA round 3): documented the re-verify-after-bump boundary in the dirty-check comment (per the reviewer's recommended option — treating drift as stale would reintroduce the #652 loop, so the boundary is accepted, not "fixed"). Behavior unchanged; 12/12 pass.