fix(engineer): stop pre-mutation guard tripping on .simard-engineer-claim (#2621)

[rysweet]

Summary

Fixes #2621. The engineer-loop pre-mutation guard (if analyzed.is_mutating() && inspection.worktree_dirty) aborted every mutating engineer whose target repo did not gitignore Simard's private .simard-engineer-claim sentinel. Simard drops that sentinel into every engineer worktree but relied on the target repo's .gitignore to hide it, so external governed repos (e.g. agent-kgpacks-rs) reported worktree_dirty == true on the untracked sentinel and the guard failed Intake before the coding agent was ever spawned — an infinite engineer-dispatch loop with zero progress (blocked agent-kgpacks-rs WS #12/#16/#17/#18/#19/#20/#21).

Fix (repo-agnostic, belt-and-suspenders)

1. Worktree creation — engineer_worktree::allocate now calls exclude_engineer_claim, which appends a root-anchored /.simard-engineer-claim line to the new worktree's git exclude file (resolved via git rev-parse --git-path info/exclude). Idempotent (exact-line match short-circuits, also matches a legacy bare entry), creates the file/parent dir if absent, serialized under worktree_mutation_lock (the shared common info/exclude is a read-modify-write race across concurrent allocations), best-effort non-fatal with a WARN log.
2. inspect_workspace — filters ENGINEER_CLAIM_FILE out of both changed_files and the worktree_dirty determination via the shared strip_claim_sentinel helper, so a worktree whose only change is the sentinel is treated as clean while genuine changes still count. The same filter is applied to verify_agent_spawn_artifacts (the post-session evidence report), so the sentinel can't be reported as a spurious "new changed file" on the degraded path where the exclude append failed.

Root-anchoring keeps the exclude/filter semantics identical (exact root path only): a genuine agent-created subdir/.simard-engineer-claim is NOT hidden.

Merge-ready evidence

1. qa-team scenarios (validated + run)

tests/qa-scenarios/engineer-claim-sentinel-guard.yaml:

• gadugi-test validate -f tests/qa-scenarios/engineer-claim-sentinel-guard.yaml → ✓ Scenario "engineer-claim-sentinel-guard" is valid (Valid files: 1, Invalid: 0).
• gadugi-test run -d tests/qa-scenarios -s engineer-claim-sentinel-guard → ✓ Passed: 1 ✗ Failed: 0 (All tests passed!). The scenario runs the three core regression tests under a single bash -lc wrapper that exits zero only when cargo test reports 3 passed.

Regression tests (9 total; all pass):

• engineer_loop::tests_claim_sentinel::inspect_workspace_treats_claim_only_worktree_as_clean — the exact pre-mutation guard trips on untracked .simard-engineer-claim in non-Simard repos — infinite engineer dispatch loop (blocks all agent-kgpacks-rs WS goals) #2621 repro: a worktree whose only change is .simard-engineer-claim reports worktree_dirty == false and empty changed_files. Since the pre-mutation guard reduces to is_mutating() && worktree_dirty, this proves the guard passes.
• inspect_workspace_still_flags_real_changes_alongside_claim — a genuine change alongside the sentinel still marks the tree dirty (no over-broad filter).
• verify_agent_spawn_artifacts_ignores_claim_only_no_op_session — degraded path (sentinel present, no exclude entry): a no-op session is "unverified", not a false "verified". Verified to fail if the post-status filter is removed (negative control: without the filter the test fails with left: "verified").
• verify_agent_spawn_artifacts_still_verifies_real_change_alongside_claim — a real new file still verifies and only the real change appears in the evidence.
• strip_claim_sentinel_removes_only_the_root_sentinel, strip_claim_sentinel_keeps_subdir_same_basename — shared-filter unit coverage.
• engineer_worktree::tests_extra::allocate_excludes_claim_sentinel_from_git_status — a freshly allocated worktree (parent repo does not gitignore the sentinel) reports a clean git status; exclude entry present exactly once (idempotent).
• allocate_exclude_does_not_hide_subdir_same_basename — root-anchored exclude does not hide a same-basename file in a subdirectory.
• exclude_engineer_claim_is_idempotent_and_preserves_existing_entries — repeated appends don't duplicate and preserve pre-existing exclude entries.

Local suite: engineer_loop:: 234 passed / 0 failed, engineer_worktree:: 42 passed / 0 failed.

2. Docs / changed surfaces

No user-facing surface change (no CLI/API/output change). Internal-only, changed surfaces:

• engineer_worktree::allocate now writes a repo-local .git/info/exclude entry (uncommitted, never pushed).
• inspect_workspace and verify_agent_spawn_artifacts ignore Simard's private sentinel.
  Justification: these are private Simard infrastructure behaviors with no external contract; the sentinel (.simard-engineer-claim) is Simard-internal. Note (non-blocking): the exclude entry also benignly widens the daemon sweep / GC "recoverable work" probes to no longer treat the ever-present sentinel as work in non-gitignoring repos; genuine uncommitted work is still detected (root anchor), so no data-loss hazard.

3. quality-audit (≥3 SEEK→VALIDATE→FIX cycles, ended clean)

• Cycle 1 (SEEK, breadth) → VALIDATE (3 independent lenses): mandatory zero-stub scan of production changes — clean (no todo!/unimplemented!/bail!("TODO")/panic!/stub Ok(()); exclude_engineer_claim uses map_err/? throughout; all unwrap/expect are test-only). cargo fmt --all -- --check clean; cargo clippy --all-targets --all-features --locked -- -D warnings clean. Guard-coverage analysis: the guard is exactly is_mutating() && worktree_dirty, fully covered by the repro test. Deadlock analysis: step-4 git worktree add lock is dropped at its block end before step-4b re-acquires it (std::sync::Mutex non-reentrant, no nesting). Validated by two independent code-review agents.
• Cycle 2 (VALIDATE, depth): an independent reviewer empirically verified the git behaviors rather than reasoning abstractly: git rev-parse --git-path info/exclude returns an absolute path into the shared common dir for linked worktrees and a cwd-relative path in normal repos (both handled by the is_absolute() branch + join fallback); a root-anchored entry in the shared exclude hides only each worktree-root sentinel (a subdir/ same-basename file still shows ??, main repo unaffected); idempotency handles missing file / no-trailing-newline / legacy bare line; strip_claim_sentinel is applied identically to both git status consumers so count_new_status_paths (after \ before) cannot reintroduce the sentinel. Zero critical/high/medium correctness or security findings.
• Cycle 3 (FIX + re-VALIDATE): the one actionable finding — verify_agent_spawn_artifacts's sentinel filter had no behavioral test (a regression deleting it would compile and pass every other test) — was fixed by adding the two verify_agent_spawn_artifacts_* tests above, proven to catch the regression via a negative control (removed the filter → test failed with the exact false-verified behavior → restored). Final re-scan: stub scan clean, fmt clean, clippy -D warnings clean, engineer_loop:: 234 passed.

Final cycle clean: zero critical/high; zero medium correctness/security findings. The single medium coverage finding was fixed within the audit.

4. CI

CI is 100% green on the current PR head aeaa7e62764c638ca0187be5e28050cbe17cc80e — all 16 required checks concluded SUCCESS, 0 failing, 0 pending. Confirmed green workflow runs:

• verify — SUCCESS: https://github.com/rysweet/Simard/actions/runs/28761097315 and https://github.com/rysweet/Simard/actions/runs/28761096366 (jobs: pre-commit, cargo-audit, cargo-deny, cargo-vet, npm-audit, install-real, e2e-dashboard).
• coverage — SUCCESS: https://github.com/rysweet/Simard/actions/runs/28761097304
• GitGuardian Security Checks — SUCCESS.

gh pr checks 2623 rollup: 15 pass + 1 security pass, 0 fail/pending. mergeStateStatus: CLEAN, mergeable: MERGEABLE. Local pre-push gates (mirror CI verify.yml) were also all green on this head: cargo fmt --all -- --check, cargo test --release --lib (cognitive_memory|bootstrap|memory_ipc|memory_consolidation), cargo clippy --all-targets --all-features --locked -- -D warnings. Branch is rebased onto latest origin/main.

5. Verdict — READY TO MERGE

This PR is ready to merge. Rationale: all six merge-ready criteria have substantive, concrete evidence tied to the decision — (1) qa-team scenario engineer-claim-sentinel-guard.yaml validated (✓ Scenario … is valid) and run (✓ Passed: 1 ✗ Failed: 0), backed by 9 named regression tests all passing including the exact #2621 repro inspect_workspace_treats_claim_only_worktree_as_clean; (2) changed surfaces are enumerated with internal-only justification (no user-facing contract change — the sentinel is Simard-private infra); (3) three explicit SEEK→VALIDATE→FIX quality-audit cycles ending clean, with a negative-control proof that the verify_agent_spawn_artifacts filter test catches its own regression; (4) CI 100% green on head aeaa7e62 with the run links above; (6) the diff is focused on exactly 5 files scoped to the fix with no unrelated edits. The change is a targeted, root-anchored filename filter over git's stable porcelain git status --short output plus a best-effort, non-fatal .git/info/exclude append — low blast radius, no cognition/memory-consolidation logic touched, no brittle model-output parsing. It resolves the P1 infinite engineer-dispatch loop blocking the agent-kgpacks-rs WS cascade. No blocking findings remain.

6. Diff focused

Scoped to the fix: engineer_loop/mod.rs (shared strip_claim_sentinel filter + both consumers + test-module registration), engineer_loop/tests_claim_sentinel.rs, engineer_worktree/mod.rs (exclude_engineer_claim helper + call site), engineer_worktree/tests_extra.rs, one qa-scenario. No unrelated edits.

Closes #2621

[github-actions]

📊 Coverage Summary

Generated by cargo llvm-cov --workspace --summary-only (nightly, excluding test files)

Module	Lines	Covered	Coverage
Total	136918	113396	82.8%

_{Coverage data from CI run. Test files matching tests?/ are excluded from line counts.}