fix(verify): baseline-untracked subtraction + skill's baseline key in the shared gate

[pbean]

Closes #88, Closes #89

Both bugs were surfaced by the PR #86 final review (the intent-gap/patch-restore audit), where each was adversarially verified against main and filed rather than folded into that PR — they are pre-existing, not regressions from #86. They land together because both fixes are in the same ~15-line block: _verify_shared_gates, the workflow-tag / expected-status / baseline-match / proof-of-work gates shared verbatim by verify_dev, verify_dev_bundle and verify_dev_stories.

#88 — the proof-of-work gate now subtracts the baseline snapshot

has_changes_since counted every untracked file and never subtracted baseline_untracked (contrast attempt_dirty directly below it, which does). After any intent-gap halt the saved patch is untracked residue under implementation_artifacts, which _protected_relpaths shields from every reset. #86's T4 fix excludes the latched patch by path, but a from-scratch re-arm (restore_patch=None — the common "the attempted reading was wrong" case) has no latch and never learns the path. A re-driven session that produced zero code changes but flipped the spec status passed the gate on the patch file's mere presence, and finalize_commit's add -A then swept it into the story commit.

has_changes_since gains a keyword-only baseline_untracked, subtracted when non-None; the sole production caller passes task.baseline_untracked. Both snapshot writers (re-arm in runs.py, dispatch-time re-capture in engine.py) run while the patch is on disk, so it lands in the snapshot and stops counting.

None deliberately keeps counting all untracked files — the opposite of attempt_dirty's None = ignore-all. Pinned in the docstring, because it reads like an inconsistency and isn't: the two gates fail open in opposite directions. A proof-of-work gate must fail open toward "work happened" (a pre-snapshot run must not have its gate silently weakened into never seeing new files); a rollback gate must fail open toward "nothing to remove" (never delete a file it cannot prove this attempt created).

#89 — the baseline-match gate reads the key the skill actually writes

The gate read fm.get("baseline_commit", "") and skips the whole check when the key is absent. The generic bmad-dev-auto skill's step-03 stamps baseline_revision; baseline_commit exists only in the result.json devcontract synthesizes, which the gate does not consult (it re-reads frontmatter directly). Net: in production the baseline-match gate never fired for generic-skill sessions, and a spec claiming a stale or foreign baseline sailed through.

The gate now reads both keys via the exact idiom already in devcontract.synthesize_result. It composes with #86's review-F2 re-stamp (re-arm rewrites baseline_revision), so patch-restore re-drives stay consistent by construction.

Fixture honesty. The bug was masked because conftest.write_spec emitted baseline_commit: — fabricating precisely the key the real skill never writes, which made the gate look alive under test. write_spec now stamps baseline_revision like the skill, and _spec_baseline reads either key. This is the load-bearing part: with the fixture honest but the reader unfixed, the two pre-existing tests test_verify_dev_lying_baseline and test_verify_dev_stories_lying_baseline fail — they had been passing vacuously. No other suite breakage surfaced, so no spec stamps both keys.

Tests

• has_changes_since: snapshot subtraction, None counts everything, post-snapshot file counts, tracked edit counts regardless of snapshot.
• Gate level, Proof-of-work gate: untracked halt residue can vacuously satisfy 'changes since baseline' (from-scratch re-drives) #88: baseline-era untracked residue + a bare status flip → retry("no changes…") with no latch in play; baseline_untracked=None → still passes (the fallback pinned); real work still passes with the snapshot in place.
• Gate level, Baseline-match verify gate is dead code for generic bmad-dev-auto sessions (baseline_commit vs baseline_revision) #89: a spec carrying only a mismatched baseline_revision fires the gate (zero coverage before — this was the masked path); a matching one passes.

Mutation-checked: reverting either source fix fails exactly the corresponding new tests (plus the two now-honest pre-existing ones).

Verification

.venv/bin/python -m pytest — 1930 passed, 1 skipped. Full trunk check (no filter) clean.

Follow-up ordering note

Per the bundling plan, #91(d) must land after this: it rewrites the exact has_changes_since call this PR modifies (renaming proof_exclude → extra_exclude and deriving the restore-patch exclusion inside the gate). The baseline_untracked=task.baseline_untracked argument added here has to survive that refactor verbatim.

Summary by CodeRabbit

• Bug Fixes

  • Fixed verification so pre-existing untracked files no longer count as new work in baseline checks.
  • Improved baseline matching to recognize both revision-style and commit-style baseline fields, reducing false failures in dev verification.
  • Tightened proof-of-work checks so leftover untracked artifacts are handled correctly.

• Documentation

  • Updated the changelog with the latest fixes.

[coderabbitai]

Warning

Review limit reached

@pbean, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a7fb5ae5-c4dd-44cd-9139-1e3cb0c0ebc5

📥 Commits

Reviewing files that changed from the base of the PR and between 9f05e07 and 86972e9.

📒 Files selected for processing (1)

• src/bmad_loop/verify.py

Walkthrough

Modified verify.has_changes_since to accept a baseline_untracked snapshot parameter that is subtracted from currently untracked files, closing a vacuous-pass gap for baseline-era residue. Updated shared verify gates to fall back from baseline_commit to baseline_revision. Updated fixtures, tests, and CHANGELOG accordingly.

Changes

Baseline verification gate fixes

Layer / File(s)	Summary
Untracked snapshot subtraction and baseline key fallback src/bmad_loop/verify.py	has_changes_since gains a kw-only baseline_untracked parameter subtracted from current untracked files; _verify_shared_gates falls back to baseline_revision when baseline_commit is absent, and passes task.baseline_untracked into the proof-of-work gate call.
Fixture updates and gate test coverage tests/conftest.py, tests/test_verify.py, CHANGELOG.md	write_spec now stamps baseline_revision; _spec_baseline reads either key; new tests cover untracked residue exclusion, None-snapshot behavior, and baseline-revision gate matching; CHANGELOG documents both fixes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• bmad-code-org/bmad-loop#79: Both PRs change how has_changes_since(...) is invoked in the proof-of-work gate in verify.py.
• bmad-code-org/bmad-loop#86: Both PRs adjust the "no changes since baseline" gate's handling of untracked/working-tree deltas.
• bmad-code-org/bmad-loop#78: Both PRs relate task.baseline_untracked refresh on re-arm to its consumption in verify.has_changes_since.

Poem

A rabbit hopped through untracked weeds,
Subtracting old residue from new deeds. 🐇
"baseline_revision," it whispered with cheer,
No more vacuous passes, the gate is now clear.
Thump-thump for the fix, the tests all agree! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly names the two verify fixes and matches the PR's main changes.
Linked Issues check	✅ Passed	The code and tests implement both linked fixes: baseline_untracked subtraction and baseline_revision fallback for the baseline gate.
Out of Scope Changes check	✅ Passed	The changes stay focused on verify-gate behavior, fixture alignment, and changelog notes with no unrelated functionality added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/verify-gate-baseline-88-89

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/bmad_loop/verify.py (1)

1116-1122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Error message references baseline_commit even when value came from baseline_revision.

Line 1126 hardcodes spec baseline_commit in the retry message, but the value may have been read from baseline_revision. This is a minor cosmetic inconsistency that could confuse debugging when the gate fires for a generic-skill session.

♻️ Proposed fix

-            return VerifyOutcome.retry(
-                f"spec baseline_commit {claimed_baseline[:12]} does not match "
-                f"orchestrator-recorded baseline {task.baseline_commit[:12]}"
-            )
+            return VerifyOutcome.retry(
+                f"spec baseline {claimed_baseline[:12]} does not match "
+                f"orchestrator-recorded baseline {task.baseline_commit[:12]}"
-            )
+            )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/bmad_loop/verify.py` around lines 1116 - 1122, The retry/error message in
the baseline gate is hardcoded to reference baseline_commit even when the value
was sourced from baseline_revision. Update the message in verify.py near
claimed_baseline handling so it reflects the actual frontmatter key used,
matching the same fallback logic as devcontract.synthesize_result. Keep the
wording consistent with whichever key was read, or make the message generic
enough to cover both cases.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/bmad_loop/verify.py`:
- Around line 1116-1122: The retry/error message in the baseline gate is
hardcoded to reference baseline_commit even when the value was sourced from
baseline_revision. Update the message in verify.py near claimed_baseline
handling so it reflects the actual frontmatter key used, matching the same
fallback logic as devcontract.synthesize_result. Keep the wording consistent
with whichever key was read, or make the message generic enough to cover both
cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98a99acd-f58c-4102-8f84-53929e183e56

📥 Commits

Reviewing files that changed from the base of the PR and between 5ec7aa0 and 9f05e07.

📒 Files selected for processing (4)

• CHANGELOG.md
• src/bmad_loop/verify.py
• tests/conftest.py
• tests/test_verify.py

[pbean]

Addressed the CodeRabbit nitpick in 86972e9: the baseline-mismatch retry message now says spec baseline instead of hardcoding baseline_commit, since the value can come from either key (and after this PR typically comes from baseline_revision). No test changes needed — assertions on this path only check the does not match substring.