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

CHANGELOG.md

@@ -31,6 +31,22 @@ breaking changes may land in a minor release.
 
 ### Fixed
 
+- **Baseline-era untracked residue no longer vacuously satisfies the proof-of-work gate.**
+  `has_changes_since` counted every untracked file. After an intent-gap halt the saved patch is
+  untracked residue under the artifact dirs every reset deliberately protects, so a from-scratch
+  re-arm — which never learns the patch's path — let a re-driven session that produced nothing but a
+  spec status flip pass the gate on that file's mere presence, and `finalize_commit`'s `add -A` swept
+  it into the story commit. The gate now subtracts the task's `baseline_untracked` snapshot. A `None`
+  snapshot (a pre-upgrade run) still counts every untracked file — deliberately the opposite of
+  `attempt_dirty`'s ignore-all, because a proof-of-work gate has to fail open toward "work happened".
+  (closes #88)
+- **The baseline-match verify gate was dead code for generic dev sessions.** The gate read the spec's
+  `baseline_commit` and skipped itself when that key was absent — but `bmad-dev-auto` stamps
+  `baseline_revision`; `baseline_commit` exists only in the orchestrator's synthesized `result.json`.
+  In production the check never fired, so a spec claiming a stale or foreign baseline sailed through.
+  The gate now reads either key, the idiom `devcontract` already used. The test fixture stamps
+  `baseline_revision` like the real skill does, so it can no longer fabricate the key that hid this.
+  (closes #89)
 - **Unit keys with git-ref-illegal characters no longer break worktree runs.** `unit_branch_name`
   built `bmad-loop/<run_id>/<unit_key>` from the raw ids, so a key or `--run-id` carrying `:`, `..`,
   `@{`, a space or a trailing `.lock` cleared the (already-sanitized) worktree dir only to die at

src/bmad_loop/verify.py

@@ -138,19 +138,41 @@ def same_commit(a: str, b: str) -> bool:
     return a.startswith(b) or b.startswith(a)
 
 
-def has_changes_since(repo: Path, baseline: str, exclude: tuple[str, ...] = ()) -> bool:
+def has_changes_since(
+    repo: Path,
+    baseline: str,
+    exclude: tuple[str, ...] = (),
+    *,
+    baseline_untracked: list[str] | None = None,
+) -> bool:
     """True if tracked changes since baseline OR untracked files exist.
 
     `exclude` is repo-relative posix dir prefixes whose changes don't count —
     used by the dev/bundle proof-of-work gate to ignore the orchestrator-owned
     BMAD artifact folders (see `artifact_relpaths`), so a session that only
     rewrites its own spec (e.g. the frontmatter-status reconcile) under those
     folders doesn't register as real implementation work. Mirrors
-    `attempt_dirty`'s exclusion. Default `()` keeps the unscoped behavior."""
+    `attempt_dirty`'s exclusion. Default `()` keeps the unscoped behavior.
+
+    `baseline_untracked` is the untracked-file snapshot taken when the baseline
+    was recorded; when given, those files already existed before the session ran
+    and are subtracted, so pre-session residue (e.g. an earlier halt's saved
+    intent-gap patch, which `_protected_relpaths` shields from every reset) can
+    never masquerade as this session's work.
+
+    `None` means count EVERY untracked file — deliberately the *opposite* of
+    `attempt_dirty`'s `None` = ignore-all, and not an oversight. 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), while a rollback gate must fail open
+    toward "nothing to remove" (never delete a file it cannot prove this attempt
+    created). Keep it that way."""
     rc, _ = _git(repo, "diff", "--quiet", baseline, "--", ".", *_exclude_specs(exclude))
     if rc != 0:
         return True
     created = untracked_files(repo)
+    if baseline_untracked is not None:
+        created -= set(baseline_untracked)
     created = {p for p in created if not _path_under_any(p, exclude)}
     return bool(created)
 
@@ -1091,17 +1113,28 @@ def _verify_shared_gates(
             f"spec status is {status!r}, expected {expected_status!r}: {spec_path}"
         )
 
-    claimed_baseline = str(fm.get("baseline_commit", "")).strip()
+    # The generic bmad-dev-auto skill stamps `baseline_revision`, never
+    # `baseline_commit` — that name exists only in the result.json devcontract
+    # synthesizes, which this gate does not consult (it re-reads frontmatter).
+    # An absent key skips the check below, so reading `baseline_commit` alone
+    # made this gate dead code for every generic-skill session. Read both, the
+    # same idiom as `devcontract.synthesize_result`.
+    claimed_baseline = str(fm.get("baseline_commit", fm.get("baseline_revision", ""))).strip()
     if task.baseline_commit and claimed_baseline not in ("", "NO_VCS"):
         if not same_commit(claimed_baseline, task.baseline_commit):
             return VerifyOutcome.retry(
-                f"spec baseline_commit {claimed_baseline[:12]} does not match "
+                f"spec baseline {claimed_baseline[:12]} does not match "
                 f"orchestrator-recorded baseline {task.baseline_commit[:12]}"
             )
 
     if proof_exclude is not None and task.baseline_commit:
         try:
-            if not has_changes_since(paths.project, task.baseline_commit, exclude=proof_exclude):
+            if not has_changes_since(
+                paths.project,
+                task.baseline_commit,
+                exclude=proof_exclude,
+                baseline_untracked=task.baseline_untracked,
+            ):
                 return VerifyOutcome.retry("no changes in worktree since baseline commit")
         except GitError as e:
             return VerifyOutcome.escalate(str(e))

tests/conftest.py

@@ -183,9 +183,14 @@ def set_sprint(paths: ProjectPaths, key: str, status: str) -> None:
 
 
 def write_spec(path: Path, status: str, baseline: str, *, prose_status: str | None = None) -> None:
+    """Write a spec the way the real bmad-dev-auto skill does. The skill's step-03
+    stamps `baseline_revision` and NEVER `baseline_commit` (that name exists only
+    in the orchestrator's synthesized result.json), so this fixture stamps the
+    same key — a reader that only knows `baseline_commit` must fail a test here,
+    not sail through production (issue #89)."""
     body = (
         f"---\ntitle: 'test'\ntype: 'feature'\nstatus: '{status}'\n"
-        f"baseline_commit: '{baseline}'\n---\n\n## Intent\n\ntest spec\n"
+        f"baseline_revision: '{baseline}'\n---\n\n## Intent\n\ntest spec\n"
     )
     if prose_status is not None:
         # mirror bmad-dev-auto's terminal finalize: it appends a `## Auto Run
@@ -291,8 +296,11 @@ def effect(spec: SessionSpec) -> SessionResult:
 
 
 def _spec_baseline(path: Path) -> str:
+    """Read back whichever baseline key a spec carries: `write_spec` stamps
+    `baseline_revision` like the real skill, but hand-rolled fixture specs (and
+    re-arm's re-stamp) may carry either."""
     for line in path.read_text().splitlines():
-        if line.startswith("baseline_commit:"):
+        if line.startswith(("baseline_commit:", "baseline_revision:")):
             return line.split(":", 1)[1].strip().strip("'\"")
     return ""
 

tests/test_verify.py

@@ -1092,6 +1092,36 @@ def test_has_changes_since_excludes_artifact_only_edit(project):
     )
 
 
+def test_has_changes_since_subtracts_baseline_untracked(project):
+    """Untracked files already on disk when the baseline snapshot was taken are
+    not this session's work. `None` deliberately keeps counting all of them —
+    the opposite of `attempt_dirty`'s ignore-all — so a pre-snapshot run's
+    proof-of-work gate is never silently weakened."""
+    baseline = verify.rev_parse_head(project.project)
+    (project.project / "residue.txt").write_text("left by an earlier halt\n")
+
+    assert verify.has_changes_since(project.project, baseline) is True
+    assert verify.has_changes_since(project.project, baseline, baseline_untracked=None) is True
+    assert (
+        verify.has_changes_since(project.project, baseline, baseline_untracked=["residue.txt"])
+        is False
+    )
+
+    # a file created after the snapshot is this session's
+    (project.project / "fresh.txt").write_text("this session\n")
+    assert (
+        verify.has_changes_since(project.project, baseline, baseline_untracked=["residue.txt"])
+        is True
+    )
+    # and a tracked edit counts regardless of how complete the snapshot is
+    (project.project / "fresh.txt").unlink()
+    (project.project / "src.txt").write_text("real\n")
+    assert (
+        verify.has_changes_since(project.project, baseline, baseline_untracked=["residue.txt"])
+        is True
+    )
+
+
 def test_verify_dev_exclude_relpaths_is_file_granular(project):
     """Unlike artifact_relpaths (whole-folder), this excludes only the
     sprint-status ledger and the session's own claimed spec file — sibling
@@ -1145,6 +1175,63 @@ def test_verify_dev_latched_restore_patch_is_not_proof_of_work(project):
     assert not out.ok and "no changes" in out.reason
 
 
+def test_verify_dev_baseline_era_untracked_is_not_proof_of_work(project):
+    """#88: the from-scratch case the T4 latch exclusion cannot reach. After an
+    intent-gap halt the saved patch is untracked residue under the protected
+    artifact dirs, and a from-scratch re-arm (`restore_patch=None`) never learns
+    its path — but the re-arm's snapshot captured it. Subtracting
+    `baseline_untracked` is the only mechanical close: a re-driven session that
+    produced nothing but a status flip must not pass the gate on that residue."""
+    write_sprint(project, {"1-1-a": "review"})
+    sp = spec_path(project, "1-1-a")
+    write_spec(sp, "in-review", "NO_VCS")
+    git(project.project, "add", "-A")
+    git(project.project, "commit", "-q", "-m", "baseline")
+    task = make_task(project)
+    residue = project.implementation_artifacts / "attempt.patch"
+    residue.write_text("stale intent-gap diff\n", encoding="utf-8")
+    rel = residue.relative_to(project.project).as_posix()
+
+    # no latch (from-scratch re-arm) — only the snapshot can rule the residue out
+    assert task.restore_patch is None
+    task.baseline_untracked = [rel]
+    out = verify.verify_dev(task, project, dev_result(sp))
+    assert not out.ok and "no changes" in out.reason
+
+    # None (a pre-snapshot run) still counts every untracked file: the gate fails
+    # open toward "work happened", never toward a silently disabled gate
+    task.baseline_untracked = None
+    assert verify.verify_dev(task, project, dev_result(sp)).ok
+
+    # real work passes with the snapshot in place
+    task.baseline_untracked = [rel]
+    (project.project / "src.txt").write_text("real work\n")
+    assert verify.verify_dev(task, project, dev_result(sp)).ok
+
+
+def test_verify_dev_baseline_gate_reads_the_skills_baseline_revision_key(project):
+    """#89: the generic bmad-dev-auto skill's step-03 stamps `baseline_revision`;
+    `baseline_commit` exists only in the orchestrator's synthesized result.json.
+    Reading just the latter made the baseline-match gate dead code in production
+    (an absent key skips the check), so a spec claiming a foreign baseline sailed
+    through. Masked for years by fixtures that stamped the key the skill never
+    writes."""
+    write_sprint(project, {"1-1-a": "review"})
+    task = make_task(project)
+    sp = spec_path(project, "1-1-a")
+
+    write_spec(sp, "in-review", "0" * 40)  # a foreign baseline, skill-style key
+    body = sp.read_text()
+    assert "baseline_revision:" in body and "baseline_commit:" not in body
+    out = verify.verify_dev(task, project, dev_result(sp))
+    assert not out.ok and "does not match" in out.reason
+
+    # matching baseline + real work → the gate passes
+    write_spec(sp, "in-review", task.baseline_commit)
+    (project.project / "src.txt").write_text("real work\n")
+    assert verify.verify_dev(task, project, dev_result(sp)).ok
+
+
 def test_verify_dev_exclude_relpaths_normalizes_dotdot_segments(project):
     """A spec_path with a lexical '..' hop (as an un-normalized session-reported
     spec_file could produce) must resolve to the same exclude entry as the plain