fix(verify): stop treating ledger/spec-only stories as "no changes since baseline"#79
Merged
Merged
Conversation
…nce baseline" verify_dev/verify_dev_bundle's proof-of-work gate blanket-excluded the whole implementation_artifacts/planning_artifacts folders from has_changes_since, so a story whose entire authorized scope is ledger/spec reconciliation (e.g. deferred-work.md sync) always produced a false "no changes" verdict and got rolled back/deferred, no matter how much real, reviewed work landed. Add verify_dev_exclude_relpaths(paths, spec_path): excludes only the session's own claimed spec file and the sprint-status ledger (the two things every session rewrites regardless of real work), not the whole artifact folders. Sibling content (deferred-work.md, other stories' specs) now counts. Wired into both has_changes_since call sites in place of artifact_relpaths, which stays as-is for Engine's rollback-protection use (a different job). output_folder is deliberately excluded from the new helper: in the standard layout it's the parent dir of implementation_artifacts, so excluding it as a prefix would swallow that folder's content right back out of view.
WalkthroughAdds ChangesVerify Exclude Gating
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Dev as verify_dev/verify_dev_bundle
participant Exclude as verify_dev_exclude_relpaths
participant Git as has_changes_since
Dev->>Exclude: compute exclude(paths, spec_path)
Exclude-->>Dev: (sprint_status, spec_path) relpaths
Dev->>Git: has_changes_since(baseline_commit, exclude=relpaths)
Git-->>Dev: changed files outside exclude set
alt real work detected
Dev-->>Dev: proof-of-work gate passes
else no changes since baseline
Dev-->>Dev: proof-of-work gate fails
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🤖 Augment PR SummarySummary: Tightens the dev/bundle proof-of-work check so ledger/spec-only stories aren’t falsely treated as “no changes since baseline”.
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| out: list[str] = [] | ||
| for path in (paths.sprint_status, spec_path): | ||
| try: | ||
| rel = path.relative_to(paths.project).as_posix() |
There was a problem hiding this comment.
verify_dev_exclude_relpaths: because spec_path isn’t normalized before relative_to(...).as_posix(), a spec_file containing .. segments could yield an exclude string that doesn’t match git’s normalized path, letting a spec-only status flip be counted as “real work”. Consider normalizing (resolve()) before deriving rel so the exclude set is stable and reliable.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
verify_dev_exclude_relpaths compared spec_path against paths.project via a lexical relative_to(), but spec_path is derived from a session-reported (untrusted) spec_file string. An un-normalized '..'/'.' segment in that string still resolves to the real on-disk file (the OS resolves it when checking spec_path.is_file()), but as a raw string it would not match git's own normalized path output — silently defeating the exclude and letting a bare status flip on the session's own spec count as real work. Resolve both spec_path and paths.project before deriving the relpath, same as spec_within_roots already does for the same session-controlled-path reason. Two new tests: the exclude helper normalizes a '..'-laden path to the same entry as the plain one, and an end-to-end verify_dev regression confirming the bare status flip still retries as "no changes" when claimed through such a path. Addresses review feedback from PR #79 (Augment).
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
pbean
added a commit
that referenced
this pull request
Jul 7, 2026
…ular #79 contract (T3) verify_dev_stories excluded the whole implementation_artifacts/planning_artifacts folders from its proof-of-work gate (the pre-#79 artifact_relpaths blanket). After #79 narrowed verify_dev/verify_dev_bundle to a file-granular exclude (verify_dev_exclude_relpaths: only the session's own spec + the sprint-status ledger), stories mode kept the exact false-negative #79 fixed: a story whose entire authorized scope is ledger/spec reconciliation (e.g. deferred-work.md under implementation_artifacts) always read as "no changes since baseline" and got rolled back/deferred. Swap artifact_relpaths for verify_dev_exclude_relpaths in verify_dev_stories, keeping _stories_relpaths (the spec folder's stories/ subdir + stories.yaml hold only specs and the human-authored manifest, never implementation work). Test: a stories-mode story whose only diff is the deferred-work ledger now passes proof-of-work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
verify_dev/verify_dev_bundle's proof-of-work gate now excludes only the session's own spec file and the sprint-status ledger fromhas_changes_since, via a newverify_dev_exclude_relpaths(paths, spec_path), instead of the wholeimplementation_artifacts/planning_artifactsfolders.Why
The old exclusion existed so a session couldn't fake completion by only flipping its own spec's
status:field, but it hid everything under those folders, including sibling content like the deferred-work ledger. A story whose entire authorized scope is ledger/spec reconciliation (no src/tests touched, by design) therefore always false-negatived "no changes since baseline" and got rolled back or deferred, no matter how much real, reviewed work landed.How
verify_dev_exclude_relpaths: excludespaths.sprint_statusand the claimedspec_pathonly (file, not folder, granularity).has_changes_sincecall sites, replacingartifact_relpaths(paths)(which stays as-is forEngine's rollback-protection use, a different job).output_folderdeliberately stays out of the new exclude set: it's the parent dir ofimplementation_artifactsin the standard layout, so excluding it as a prefix would swallow that folder's content right back out of view.Testing
Seven new tests in
tests/test_verify.py: exclude-helper granularity plus a..-segment normalization guard on the helper, a directhas_changes_sincerepro (ledger content counts, own-spec + sprint bookkeeping alone doesn't), both theverify_devandverify_dev_bundlegates passing end-to-end on a ledger-only diff, and regression guards that a bare own-spec status flip still retries as "no changes" — both directly and when the spec is claimed via a..-laden path. Full suite: 1412 passed, 7 skipped; ruff clean.Related
Independent of the other pending PR (
fix/resolve-baseline-advance); different files, either can land first.Summary by CodeRabbit
Bug Fixes
Tests