Reviewer mutations require canonical workflow load proof #389

Closed
opened 2026-07-07 08:51:09 -05:00 by jcwalker3 · 2 comments
Owner

Problem

Reviewer mutating tools (gitea_review_pr, gitea_submit_pr_review, gitea_merge_pr, and related reviewer write paths) can execute without proving the canonical skills/llm-project-workflow/workflows/review-merge-pr.md workflow was loaded in the current session.

Issue #296 added durable workflow storage, mcp_get_skill_guide retrieval, and report-level citation checks (assess_review_workflow_source). Issue #339 added queue-status verifier rules when a workflow is cited. Neither fails closed at mutation time if an agent skips workflow load and goes straight to review/merge tools.

Required behavior

  1. Before any reviewer mutation, MCP must require workflow load proof:
    • citation of workflows/review-merge-pr.md or successful mcp_get_skill_guide retrieval for that path
    • populated Workflow version / Workflow hash field
    • hash matches current canonical workflow content (stale copies fail closed)
  2. Reviewer write tools must block when proof is missing, stale, or from a prior session.
  3. Denial must return structured recovery handoff: reload workflow via mcp_get_skill_guide, cite path + hash, then rerun from inventory.
  4. Read-only tools (gitea_view_pr, gitea_list_prs, gitea_whoami, capability resolve) remain allowed before load proof.

Acceptance criteria

  • Reviewer mutation without workflow load proof fails closed (no review/approve/merge/request-changes).
  • Stale workflow hash fails closed even if path is cited.
  • Valid load proof in current session allows reviewer mutations through existing capability gates.
  • Tests cover: missing citation, missing hash, stale hash, valid proof, and read-only pre-load path.
  • Runbook documents mutation-time gate; prompts remain guidance, MCP gate is authority.

Related issues

  • #296 — workflow storage and report citation (partial; does not gate mutations)
  • #339 — queue-status verifier when workflow cited
  • #334 — extracted canonical review-merge workflow
  • #290 — broader state-machine umbrella (complementary)

Non-goals

  • Do not weaken existing author/reviewer profile separation.
  • Do not replace final-report validators; add mutation-time enforcement alongside them.
## Problem Reviewer mutating tools (`gitea_review_pr`, `gitea_submit_pr_review`, `gitea_merge_pr`, and related reviewer write paths) can execute without proving the canonical `skills/llm-project-workflow/workflows/review-merge-pr.md` workflow was loaded in the **current session**. Issue **#296** added durable workflow storage, `mcp_get_skill_guide` retrieval, and **report-level** citation checks (`assess_review_workflow_source`). Issue **#339** added queue-status verifier rules when a workflow is cited. Neither fails closed at **mutation time** if an agent skips workflow load and goes straight to review/merge tools. ## Required behavior 1. Before any reviewer mutation, MCP must require workflow load proof: - citation of `workflows/review-merge-pr.md` or successful `mcp_get_skill_guide` retrieval for that path - populated `Workflow version` / `Workflow hash` field - hash matches current canonical workflow content (stale copies fail closed) 2. Reviewer write tools must block when proof is missing, stale, or from a prior session. 3. Denial must return structured recovery handoff: reload workflow via `mcp_get_skill_guide`, cite path + hash, then rerun from inventory. 4. Read-only tools (`gitea_view_pr`, `gitea_list_prs`, `gitea_whoami`, capability resolve) remain allowed before load proof. ## Acceptance criteria - [ ] Reviewer mutation without workflow load proof fails closed (no review/approve/merge/request-changes). - [ ] Stale workflow hash fails closed even if path is cited. - [ ] Valid load proof in current session allows reviewer mutations through existing capability gates. - [ ] Tests cover: missing citation, missing hash, stale hash, valid proof, and read-only pre-load path. - [ ] Runbook documents mutation-time gate; prompts remain guidance, MCP gate is authority. ## Related issues - **#296** — workflow storage and report citation (partial; does not gate mutations) - **#339** — queue-status verifier when workflow cited - **#334** — extracted canonical review-merge workflow - **#290** — broader state-machine umbrella (complementary) ## Non-goals - Do not weaken existing author/reviewer profile separation. - Do not replace final-report validators; add mutation-time enforcement alongside them.
Author
Owner

Operator spec supplement (author filing — no duplicate created)

This issue already matches the requested title. Recording the fuller implementation spec here so acceptance criteria are explicit before implementation.

Problem (supplement)

The canonical review/merge workflow says it must be loaded before PR work and reported with workflow source/version/hash. Today this is still mostly prompt/document behavior. LLMs can skip or forget to load the canonical workflow, then produce free-form reviews, blocked handoffs with stale merge instructions, or incomplete final reports.

Required behavior (supplement)

Reviewer mutation tools must fail closed unless the current session has loaded the canonical workflow and recorded its source/hash.

Acceptance criteria (canonical implementation checklist)

  1. Add an MCP helper such as gitea_load_review_workflow.
  2. The helper returns:
    • workflow path/source
    • task mode
    • workflow version, commit, or hash
    • final report schema path/hash
    • whether the active prompt conflicts with the loaded workflow
  3. gitea_submit_pr_review, gitea_mark_final_review_decision, and gitea_merge_pr fail closed unless workflow-load proof exists in the current session.
  4. Capability reports include whether workflow-load proof is present.
  5. Final reports must include the workflow source/hash returned by the helper.
  6. Tests prove review/merge mutations are blocked without workflow-load proof.
  7. Tests prove stale workflow-load proof from a prior session/run cannot authorize a new review.
  8. Tests prove blocked handoffs do not include approve/merge replay commands after workflow-load failure.

Non-goals

  • Do not make workflow loading optional for reviewer mutation.
  • Do not rely on prompt text alone.
  • Do not allow local fallback to bypass workflow-load proof.
## Operator spec supplement (author filing — no duplicate created) This issue already matches the requested title. Recording the fuller implementation spec here so acceptance criteria are explicit before implementation. ### Problem (supplement) The canonical review/merge workflow says it must be loaded before PR work and reported with workflow source/version/hash. Today this is still mostly prompt/document behavior. LLMs can skip or forget to load the canonical workflow, then produce free-form reviews, blocked handoffs with stale merge instructions, or incomplete final reports. ### Required behavior (supplement) Reviewer mutation tools must fail closed unless the current session has loaded the canonical workflow and recorded its source/hash. ### Acceptance criteria (canonical implementation checklist) 1. Add an MCP helper such as `gitea_load_review_workflow`. 2. The helper returns: - workflow path/source - task mode - workflow version, commit, or hash - final report schema path/hash - whether the active prompt conflicts with the loaded workflow 3. `gitea_submit_pr_review`, `gitea_mark_final_review_decision`, and `gitea_merge_pr` fail closed unless workflow-load proof exists in the current session. 4. Capability reports include whether workflow-load proof is present. 5. Final reports must include the workflow source/hash returned by the helper. 6. Tests prove review/merge mutations are blocked without workflow-load proof. 7. Tests prove stale workflow-load proof from a prior session/run cannot authorize a new review. 8. Tests prove blocked handoffs do not include approve/merge replay commands after workflow-load failure. ### Non-goals - Do not make workflow loading optional for reviewer mutation. - Do not rely on prompt text alone. - Do not allow local fallback to bypass workflow-load proof.
jcwalker3 added the status:in-progress label 2026-07-07 08:55:23 -05:00
Author
Owner

Issue claim heartbeat

<!-- gitea-issue-claim-heartbeat:v1 --> **Issue claim heartbeat** - kind: claim - issue: #389 - branch: feat/issue-389-review-workflow-load-proof - phase: claimed - profile: prgs-author - pr: none - blocker: none - next_action: create worktree and begin implementation
sysadmin removed the status:in-progress label 2026-07-08 04:16:25 -05:00
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#389