diff --git a/src/bmad_loop/verify.py b/src/bmad_loop/verify.py index 1cb26d4..ca0e8af 100644 --- a/src/bmad_loop/verify.py +++ b/src/bmad_loop/verify.py @@ -938,6 +938,43 @@ def artifact_relpaths(paths: ProjectPaths) -> tuple[str, ...]: return tuple(out) +def verify_dev_exclude_relpaths(paths: ProjectPaths, spec_path: Path) -> tuple[str, ...]: + """Repo-relative posix paths the dev/bundle proof-of-work gate excludes from + `has_changes_since` — file-granularity, unlike `artifact_relpaths`' whole-folder + exclusion (still used as-is by `Engine._protected_relpaths` for rollback + protection, a different job). Deliberately does NOT exclude `output_folder`: + in the standard layout it is the parent directory of `implementation_artifacts`/ + `planning_artifacts`, so excluding it as a directory prefix would swallow those + two folders' content right back out of view via the same git-pathspec prefix + match this function exists to avoid. + + Excludes only what a session rewrites regardless of whether it did any real + work: `paths.sprint_status` (every session advances it as routine bookkeeping) + and the session's own claimed `spec_path` (so a bare frontmatter status flip on + it doesn't count). Sibling content under the implementation/planning artifact + dirs — the deferred-work ledger, other stories' specs — is deliberately left + un-excluded, so a story whose entire authorized scope is ledger/spec + reconciliation registers as real work instead of a permanent false "no changes + since baseline". + + `spec_path` comes from a session-reported (untrusted) `spec_file` string, so + it is `.resolve()`d before deriving the relpath, same as `spec_within_roots`: + an un-normalized `..`/`.` segment would still resolve to the real on-disk + file (the OS resolves it), but as a raw string it wouldn't match git's own + normalized path output, silently defeating this exclude and letting a bare + status flip on the session's own spec count as real work.""" + out: list[str] = [] + project = paths.project.resolve() + for path in (paths.sprint_status, spec_path): + try: + rel = path.resolve().relative_to(project).as_posix() + except ValueError: + continue # outside the project tree; nothing to exclude here + if rel and rel != ".": + out.append(rel) + return tuple(out) + + def spec_within_roots(spec_path: Path, paths: ProjectPaths) -> bool: """True if ``spec_path`` is, or sits under, an orchestrator-owned root (the project root or an artifact dir). A mutating repair (the frontmatter-status @@ -1013,7 +1050,9 @@ def verify_dev( if task.baseline_commit: try: if not has_changes_since( - paths.project, task.baseline_commit, exclude=artifact_relpaths(paths) + paths.project, + task.baseline_commit, + exclude=verify_dev_exclude_relpaths(paths, spec_path), ): return VerifyOutcome.retry("no changes in worktree since baseline commit") except GitError as e: @@ -1074,7 +1113,9 @@ def verify_dev_bundle( if task.baseline_commit: try: if not has_changes_since( - paths.project, task.baseline_commit, exclude=artifact_relpaths(paths) + paths.project, + task.baseline_commit, + exclude=verify_dev_exclude_relpaths(paths, spec_path), ): return VerifyOutcome.retry("no changes in worktree since baseline commit") except GitError as e: diff --git a/tests/test_verify.py b/tests/test_verify.py index 815ffcb..ab803a9 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -329,6 +329,19 @@ def test_verify_dev_bundle_absent_dw_ids_passes(project, claim): assert task.spec_file == str(sp) +def test_verify_dev_bundle_ledger_only_counts_as_real_work(project): + """Same false-negative as verify_dev (KNOWN-BUG-ledger-only-story-false-no- + changes.md), on the bundle path: a dw-bundle's entire authorized diff is + the ledger reconciliation itself, with no sprint-status entry to touch.""" + task = make_bundle_task(project) + sp = project.implementation_artifacts / "spec-dw-test-bundle.md" + write_spec(sp, "in-review", task.baseline_commit) + bundle_ledger(project, {"DW-1": "done 2026-06-11", "DW-2": "done 2026-06-11"}) + rj = {"workflow": "auto-dev", "spec_file": str(sp)} + out = verify.verify_dev_bundle(task, project, rj) + assert out.ok + + def test_verify_review_bundle_ledger_gate(project): task = make_bundle_task(project) sp = project.implementation_artifacts / "spec-dw-test-bundle.md" @@ -750,6 +763,100 @@ def test_has_changes_since_excludes_artifact_only_edit(project): ) +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 + artifact-folder content (deferred-work.md, other stories' specs) is left + un-excluded so it can register as real work.""" + sp = spec_path(project, "1-1-a") + rels = verify.verify_dev_exclude_relpaths(project, sp) + assert "_bmad-output/implementation-artifacts/sprint-status.yaml" in rels + assert "_bmad-output/implementation-artifacts/spec-1-1-a.md" in rels + assert "_bmad-output/implementation-artifacts" not in rels + # output_folder itself is NOT excluded here — it is the parent dir of + # implementation_artifacts in the standard layout, so excluding it as a + # prefix would swallow the artifact dirs' content right back out of view. + assert "_bmad-output" not in rels + assert "_bmad-output/implementation-artifacts/deferred-work.md" not in rels + + +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 + path — otherwise the raw string wouldn't match git's own normalized path + output, silently defeating the exclude for that spec.""" + sp = spec_path(project, "1-1-a") + messy = ( + project.output_folder / "planning-artifacts" / ".." / "implementation-artifacts" / sp.name + ) + assert messy != sp # genuinely a different (messier) Path object + assert verify.verify_dev_exclude_relpaths(project, sp) == verify.verify_dev_exclude_relpaths( + project, messy + ) + + +def test_verify_dev_own_spec_status_flip_via_dotdot_path_is_not_real_work(project): + """End-to-end regression: a dev result.json claiming its own spec through a + '..'-laden path (but pointing at the same on-disk file) must not let a bare + status flip slip past the proof-of-work gate.""" + write_sprint(project, {"1-1-a": "review"}) + task = make_task(project) + sp = spec_path(project, "1-1-a") + write_spec(sp, "in-review", task.baseline_commit) + messy = ( + project.output_folder / "planning-artifacts" / ".." / "implementation-artifacts" / sp.name + ) + assert messy.is_file() # same on-disk file as sp, reached via a messier path + + out = verify.verify_dev(task, project, dev_result(messy)) + assert not out.ok and "no changes" in out.reason + + +def test_has_changes_since_ledger_content_counts_with_narrow_exclude(project): + """Reproduces KNOWN-BUG-ledger-only-story-false-no-changes.md: a story whose + entire authorized diff is sibling ledger content must not read as 'no + changes', while a bare own-spec + sprint-status bookkeeping edit still does.""" + baseline = verify.rev_parse_head(project.project) + sp = spec_path(project, "1-1-a") + exclude = verify.verify_dev_exclude_relpaths(project, sp) + + sp.write_text("bookkeeping\n") + project.sprint_status.write_text("bookkeeping\n") + assert verify.has_changes_since(project.project, baseline, exclude=exclude) is False + + project.deferred_work.parent.mkdir(parents=True, exist_ok=True) + project.deferred_work.write_text("### DW-1: reconciled\n\nstatus: done\n") + assert verify.has_changes_since(project.project, baseline, exclude=exclude) is True + + +def test_verify_dev_ledger_only_story_counts_as_real_work(project): + """A 'paper-trail reconciliation only' story (KNOWN-BUG-ledger-only-story- + false-no-changes.md) whose entire real diff sits under implementation_artifacts + (e.g. deferred-work.md) must pass, not false-negative 'no changes'.""" + write_sprint(project, {"1-1-a": "review"}) + task = make_task(project) + sp = spec_path(project, "1-1-a") + write_spec(sp, "in-review", task.baseline_commit) + project.deferred_work.parent.mkdir(parents=True, exist_ok=True) + project.deferred_work.write_text("### DW-1: reconciled\n\nstatus: done\n") + + out = verify.verify_dev(task, project, dev_result(sp)) + assert out.ok + + +def test_verify_dev_own_spec_status_flip_alone_is_not_real_work(project): + """Guards the original loophole the exclusion targets: flipping only the + session's own spec status (plus routine sprint-status bookkeeping) must + still retry as 'no changes', even with the narrower file-level exclude.""" + write_sprint(project, {"1-1-a": "review"}) + task = make_task(project) + sp = spec_path(project, "1-1-a") + write_spec(sp, "in-review", task.baseline_commit) + + out = verify.verify_dev(task, project, dev_result(sp)) + assert not out.ok and "no changes" in out.reason + + def test_spec_within_roots(project, tmp_path): """Specs under the project / artifact roots are writable; an out-of-tree absolute path is refused (guards the reconcile mutation)."""