feat: verify canonical workflow citation and version in review reports (Closes #296) #380

2026-07-07T08:20:45-05:00

jcwalker3 commented

2026-07-07 08:20:45 -05:00

Summary

Implements the enforcement gap of #296 (expose canonical PR review and merge prompt through project skills and MCP tooling).

Most of #296 already landed: the canonical workflow lives in skills/llm-project-workflow/workflows/review-merge-pr.md (#334/#335), is retrievable via mcp_list_project_skills / mcp_get_skill_guide, is versioned and reviewed like code, and its final-report schema (workflow doc line 34) already demands "workflow version, commit, or hash". What was missing is item 8: validators that reject review reports which did not load or cite the canonical workflow.

Adds to review_proofs.py:

compute_workflow_hash(workflow_text) — short (12-hex) sha256 version hash of the workflow content; deterministic, content-sensitive. Callers hash the current review-merge-pr.md and pass it as the canonical hash.
assess_review_workflow_source(report_text, *, canonical_hash=None) — review reports must cite workflows/review-merge-pr.md (the mcp_get_skill_guide load) and carry a populated Workflow version (or Workflow hash) field. With canonical_hash supplied, a mismatched hash fails as a stale workflow copy and the reason directs the agent to reload via mcp_get_skill_guide. Missing citation, missing/none/unknown version, or stale hash all fail closed.
Mirrors the #337 assess_create_issue_workflow_source pattern for the review/merge task mode.

Tests

New tests/test_review_workflow_source.py: 9 tests — hash determinism, content sensitivity, 12-hex shape, matching-hash pass, missing citation, missing version field, stale hash, none version, and offline validation (no canonical hash supplied) still requiring citation + version (acceptance criteria "tests prevent stale or missing workflow retrieval").
TDD: verified failing on import first; 9/9 pass after.
Full suite in the branch worktree: 1273 passed, 6 skipped, 20 subtests, 1 failed — the single failure is test_final_report_validator.py::TestReviewPrRules::test_clean_reviewer_report_passes, the known #320/#327 fixture interaction present on this branch's clean base (reported in the PR #367 handoff; the other standing failure was fixed by PR #364, now merged). Not introduced by this branch.

Closes #296

🤖 Generated with Claude Code

## Summary Implements the enforcement gap of #296 (expose canonical PR review and merge prompt through project skills and MCP tooling). Most of #296 already landed: the canonical workflow lives in `skills/llm-project-workflow/workflows/review-merge-pr.md` (#334/#335), is retrievable via `mcp_list_project_skills` / `mcp_get_skill_guide`, is versioned and reviewed like code, and its final-report schema (workflow doc line 34) already demands "workflow version, commit, or hash". What was missing is item 8: validators that reject review reports which did not load or cite the canonical workflow. Adds to `review_proofs.py`: - **`compute_workflow_hash(workflow_text)`** — short (12-hex) sha256 version hash of the workflow content; deterministic, content-sensitive. Callers hash the current `review-merge-pr.md` and pass it as the canonical hash. - **`assess_review_workflow_source(report_text, *, canonical_hash=None)`** — review reports must cite `workflows/review-merge-pr.md` (the `mcp_get_skill_guide` load) and carry a populated `Workflow version` (or `Workflow hash`) field. With `canonical_hash` supplied, a mismatched hash fails as a stale workflow copy and the reason directs the agent to reload via `mcp_get_skill_guide`. Missing citation, missing/`none`/`unknown` version, or stale hash all fail closed. - Mirrors the #337 `assess_create_issue_workflow_source` pattern for the review/merge task mode. ## Tests - New `tests/test_review_workflow_source.py`: 9 tests — hash determinism, content sensitivity, 12-hex shape, matching-hash pass, missing citation, missing version field, stale hash, `none` version, and offline validation (no canonical hash supplied) still requiring citation + version (acceptance criteria "tests prevent stale or missing workflow retrieval"). - TDD: verified failing on import first; 9/9 pass after. - Full suite in the branch worktree: `1273 passed, 6 skipped, 20 subtests, 1 failed` — the single failure is `test_final_report_validator.py::TestReviewPrRules::test_clean_reviewer_report_passes`, the known #320/#327 fixture interaction present on this branch's clean base (reported in the PR #367 handoff; the other standing failure was fixed by PR #364, now merged). Not introduced by this branch. Closes #296 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jcwalker3 added 1 commit 2026-07-07 08:20:45 -05:00

feat: verify canonical workflow citation and version in review reports (Closes #296 ) 6946534f3e

The canonical PR review/merge workflow already lives in
skills/llm-project-workflow/workflows/review-merge-pr.md and is exposed
through mcp_list_project_skills / mcp_get_skill_guide (#333-#335), and
its final-report schema already demands a workflow version. What was
missing is enforcement: add compute_workflow_hash (short sha256
version hash of workflow text) and assess_review_workflow_source to
review_proofs. Review reports must cite the canonical workflow file
and carry a populated Workflow version field; when the current
canonical hash is supplied, a mismatched citation fails as a stale
workflow copy, directing the agent to reload via mcp_get_skill_guide.
Fails closed. Tests cover hash determinism and content sensitivity,
matching-hash pass, missing citation, missing version field, stale
hash, version none, and offline validation without a canonical hash.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

sysadmin approved these changes 2026-07-07 09:11:01 -05:00

sysadmin left a comment

All tests pass successfully after merging master (1273 passed, 1 failed [pre-existing]). Reviewed the implementation of assess_review_workflow_source and compute_workflow_hash in review_proofs.py and the test coverage in tests/test_review_workflow_source.py. It correctly computes a short hex hash of the canonical review/merge workflow and validates that review reports cite the workflow source and include a populated workflow version/hash field matching the canonical hash. Verified that the remaining failure is pre-existing on master.

All tests pass successfully after merging master (1273 passed, 1 failed [pre-existing]). Reviewed the implementation of `assess_review_workflow_source` and `compute_workflow_hash` in `review_proofs.py` and the test coverage in `tests/test_review_workflow_source.py`. It correctly computes a short hex hash of the canonical review/merge workflow and validates that review reports cite the workflow source and include a populated workflow version/hash field matching the canonical hash. Verified that the remaining failure is pre-existing on master.

sysadmin merged commit b5ee6567aa into master

2026-07-07 09:11:14 -05:00

jcwalker3 referenced this pull request

2026-07-07 09:20:42 -05:00

Add PR-only queue cleanup mode that dispatches one canonical review per PR #390

jcwalker3 referenced this pull request

2026-07-07 09:20:42 -05:00

Reviewer final reports must pass schema verification before output #391

jcwalker3 referenced this pull request

2026-07-07 10:15:53 -05:00

Add per-PR reviewer leases for safe parallel PR cleanup #407

Sign in to join this conversation.