Merge pull request 'feat: require canonical next-action state comments (Closes #495)' (#499) from feat/issue-495-canonical-next-action-comments into master

canonical_state_comments.py

@@ -0,0 +1,171 @@
+"""Canonical state comment validation helpers (#495).
+
+These helpers validate durable issue/PR/discussion state handoff comments.
+They are intentionally pure and do not post comments; MCP mutation hooks are
+owned by #496.
+"""
+
+from __future__ import annotations
+
+import re
+
+CANONICAL_HEADINGS = (
+    "canonical issue state",
+    "canonical pr state",
+    "canonical discussion summary",
+)
+
+REQUIRED_FIELDS = ("STATE", "WHO_IS_NEXT", "NEXT_ACTION", "NEXT_PROMPT")
+ALLOWED_NEXT_ACTORS = {
+    "controller",
+    "author",
+    "reviewer",
+    "merger",
+    "reconciler",
+    "user",
+}
+
+VAGUE_NEXT_ACTIONS = {
+    "continue",
+    "handle this",
+    "do it",
+    "fix it",
+    "proceed",
+    "follow up",
+    "next",
+    "tbd",
+    "todo",
+    "n/a",
+    "none",
+}
+
+_FIELD_RE = re.compile(r"^\s*(?:[-*]\s*)?([A-Z][A-Z0-9_ ]+)\s*:\s*(.*)$")
+_FULL_SHA_RE = re.compile(r"\b[0-9a-f]{40}\b", re.IGNORECASE)
+_CLAIMS_STATE_UPDATE_RE = re.compile(
+    r"canonical\s+(?:issue|pr|discussion)?\s*state|"
+    r"state\s+comment\s+(?:posted|created|updated)|"
+    r"next[- ]action\s+comment\s+(?:posted|created|updated)",
+    re.IGNORECASE,
+)
+
+
+def extract_state_fields(text: str | None) -> dict[str, str]:
+    """Return upper-case labeled fields from a canonical state block."""
+    fields: dict[str, str] = {}
+    current_key: str | None = None
+    for line in (text or "").splitlines():
+        match = _FIELD_RE.match(line)
+        if match:
+            current_key = match.group(1).strip().upper().replace(" ", "_")
+            fields[current_key] = match.group(2).strip()
+            continue
+        stripped = line.strip()
+        if current_key and stripped and not stripped.startswith("#"):
+            existing = fields.get(current_key, "")
+            fields[current_key] = (
+                f"{existing}\n{stripped}" if existing else stripped
+            )
+    return fields
+
+
+def contains_canonical_state_block(text: str | None) -> bool:
+    lower = (text or "").lower()
+    return any(heading in lower for heading in CANONICAL_HEADINGS)
+
+
+def claims_canonical_state_update(text: str | None) -> bool:
+    """Return True when text claims a canonical state/next-action update."""
+    return bool(_CLAIMS_STATE_UPDATE_RE.search(text or ""))
+
+
+def _empty_or_placeholder(value: str | None) -> bool:
+    value = (value or "").strip().lower()
+    return not value or value in {"none", "n/a", "unknown", "tbd", "<...>"}
+
+
+def _vague_next_action(value: str | None) -> bool:
+    normalized = re.sub(r"\s+", " ", (value or "").strip().lower())
+    return normalized in VAGUE_NEXT_ACTIONS
+
+
+def validate_canonical_state_comment(text: str | None) -> dict:
+    """Validate a canonical state comment or embedded final-report block.
+
+    Returns a dict with ``valid`` and ``reasons``. The validator focuses on
+    fields that make continuation possible: current state, next actor, next
+    action, and paste-ready next prompt, plus a few contradiction checks.
+    """
+    fields = extract_state_fields(text)
+    reasons: list[str] = []
+
+    for field in REQUIRED_FIELDS:
+        if _empty_or_placeholder(fields.get(field)):
+            reasons.append(f"missing required canonical state field: {field}")
+
+    actor = (fields.get("WHO_IS_NEXT") or "").strip().lower()
+    if actor and actor not in ALLOWED_NEXT_ACTORS:
+        reasons.append(
+            "WHO_IS_NEXT must be one of: "
+            + ", ".join(sorted(ALLOWED_NEXT_ACTORS))
+        )
+
+    if _vague_next_action(fields.get("NEXT_ACTION")):
+        reasons.append("NEXT_ACTION is too vague for durable continuation")
+
+    state = (fields.get("STATE") or "").strip().lower().replace("-", "_")
+    if "ready_to_merge" in state or (
+        state == "approved" and "pr" in (text or "").lower()
+    ):
+        review_status = (fields.get("REVIEW_STATUS") or "").lower()
+        head_sha = fields.get("HEAD_SHA") or ""
+        merge_ready = (fields.get("MERGE_READY") or "").lower()
+        if "approved" not in review_status:
+            reasons.append("ready-to-merge state requires approved REVIEW_STATUS")
+        if not _FULL_SHA_RE.search(head_sha):
+            reasons.append("ready-to-merge state requires full HEAD_SHA proof")
+        if merge_ready and not merge_ready.startswith(("yes", "true")):
+            reasons.append("ready-to-merge state contradicts MERGE_READY")
+
+    if "superseded" in state:
+        canonical = fields.get("CANONICAL_ITEM") or fields.get("SUPERSEDED_BY")
+        if _empty_or_placeholder(canonical):
+            reasons.append("superseded state requires canonical item proof")
+
+    if "blocked" in state:
+        blockers = fields.get("BLOCKERS") or ""
+        if _empty_or_placeholder(blockers):
+            reasons.append("blocked state requires BLOCKERS/unblock condition")
+
+    return {
+        "valid": not reasons,
+        "fields": fields,
+        "reasons": reasons,
+    }
+
+
+def validate_final_report_state_update(report_text: str | None) -> dict:
+    """Validate canonical state-update claims inside a final report."""
+    text = report_text or ""
+    if not claims_canonical_state_update(text) and not contains_canonical_state_block(text):
+        return {
+            "applicable": False,
+            "valid": True,
+            "reasons": [],
+        }
+
+    if not contains_canonical_state_block(text):
+        return {
+            "applicable": True,
+            "valid": False,
+            "reasons": [
+                "final report claims a canonical state update but includes no canonical state block"
+            ],
+        }
+
+    result = validate_canonical_state_comment(text)
+    return {
+        "applicable": True,
+        "valid": result["valid"],
+        "fields": result["fields"],
+        "reasons": result["reasons"],
+    }

docs/canonical-state-comments.md

@@ -0,0 +1,183 @@
+# Canonical State Comments
+
+Gitea is the durable system of record for workflow continuation. When a
+comment changes issue, PR, or discussion state, it should leave enough
+information for the next role to continue without private chat history.
+
+Canonical comments answer:
+
+- what state the object is in
+- who acts next
+- what the next actor should do
+- the exact prompt the next actor should run
+- which proof, blocker, or dependency matters
+
+Non-workflow discussion comments do not need this template.
+
+## Issue State
+
+Use this when an issue becomes ready, blocked, in progress, PR-open,
+superseded, merged, or otherwise changes workflow direction.
+
+```text
+## Canonical Issue State
+
+STATE:
+<ready-for-author | in-progress | blocked | PR-open | needs-review | ready-to-merge | merged | closed | superseded>
+
+WHO_IS_NEXT:
+<controller | author | reviewer | merger | reconciler | user>
+
+NEXT_ACTION:
+<specific one-sentence action>
+
+NEXT_PROMPT:
+<paste-ready prompt for the next role>
+
+WHAT_HAPPENED:
+<latest meaningful event>
+
+WHY:
+<decision rationale>
+
+RELATED_DISCUSSION:
+<link/reference or none>
+
+RELATED_PRS:
+- #...
+
+BRANCH:
+<branch or none>
+
+HEAD_SHA:
+<40-character SHA or none>
+
+VALIDATION:
+<tests/proofs or none>
+
+BLOCKERS:
+<blocker and unblock condition, or none>
+
+LAST_UPDATED_BY:
+<identity/profile/date>
+```
+
+## PR State
+
+Use this when a PR needs review, receives changes requested, is approved,
+is stale, is superseded, or becomes ready for merge.
+
+```text
+## Canonical PR State
+
+STATE:
+<needs-review | changes-requested | approved | stale-approval | ready-to-merge | merged | blocked | superseded>
+
+WHO_IS_NEXT:
+<controller | author | reviewer | merger | reconciler | user>
+
+NEXT_ACTION:
+<specific one-sentence action>
+
+NEXT_PROMPT:
+<paste-ready prompt for the next role>
+
+WHAT_HAPPENED:
+<latest meaningful event>
+
+WHY:
+<decision rationale>
+
+ISSUE:
+#...
+
+BASE:
+<branch>
+
+HEAD:
+<branch>
+
+HEAD_SHA:
+<40-character SHA>
+
+REVIEW_STATUS:
+<none | approved | changes-requested | stale | contaminated>
+
+VALIDATION:
+<tests/proofs>
+
+BLOCKERS:
+<blockers or none>
+
+SUPERSEDES:
+<PRs or none>
+
+SUPERSEDED_BY:
+<PR or none>
+
+MERGE_READY:
+<yes/no and why>
+
+LAST_UPDATED_BY:
+<identity/profile/date>
+```
+
+## Discussion Summary
+
+Discussions should normally have at least five substantive comments before
+conversion into issues. A controller may waive that only for tiny mechanical,
+urgent, or explicitly trivial work.
+
+```text
+## Canonical Discussion Summary
+
+STATE:
+<needs-more-discussion | ready-for-issues | issues-created | closed>
+
+WHO_IS_NEXT:
+<controller | author | reviewer | user>
+
+DECISION:
+<what was decided>
+
+WHY:
+<reasoning and tradeoffs>
+
+SUBSTANTIVE_COMMENTS:
+<count and summary>
+
+ISSUES_TO_CREATE_OR_CREATED:
+- #...
+
+DEPENDENCY_ORDER:
+<order or none>
+
+NON_GOALS:
+<non-goals>
+
+OPEN_QUESTIONS:
+<questions or none>
+
+NEXT_ACTION:
+<specific one-sentence action>
+
+NEXT_PROMPT:
+<paste-ready prompt for the next role>
+
+LAST_UPDATED_BY:
+<identity/profile/date>
+```
+
+## Validation Rules
+
+The final-report validator rejects canonical state update claims when the
+report omits the canonical block or when the block lacks:
+
+- `STATE`
+- `WHO_IS_NEXT`
+- `NEXT_ACTION`
+- `NEXT_PROMPT`
+
+It also rejects vague next actions such as `continue`, ready-to-merge states
+without approval/head-SHA proof, superseded states without canonical item
+proof, and blocked states without an unblock condition.

docs/llm-workflow-runbooks.md

@@ -7,6 +7,11 @@ package of the MCP Control Plane: creating issues, implementing them, opening
 and reviewing pull requests, merging, and closing out — safely and
 reproducibly.
 
+Canonical state comments for durable issue/PR/discussion continuation are
+documented in [`canonical-state-comments.md`](canonical-state-comments.md).
+Use them when a workflow-changing comment needs to leave the next actor, next
+action, and paste-ready prompt in Gitea.
+
 > For the **project-agnostic** version of these operating rules (issue-first,
 > isolated worktrees, no self-review/merge, profile safety, cleanup, fail-closed)
 > that can be copied into any repository, see the reusable skill

final_report_validator.py

@@ -1372,6 +1372,22 @@ def _rule_reviewer_review_mutation(
     )
 
 
+def _rule_shared_canonical_state_update(report_text: str) -> list[dict[str, str]]:
+    from canonical_state_comments import validate_final_report_state_update
+
+    result = validate_final_report_state_update(report_text)
+    if not result.get("applicable") or result.get("valid"):
+        return []
+    return [
+        validator_finding(
+            "shared.canonical_state_update",
+            "block",
+            "Canonical state update",
+            reason,
+            "include a canonical state block with STATE, WHO_IS_NEXT, NEXT_ACTION, and NEXT_PROMPT",
+        )
+        for reason in (result.get("reasons") or ["invalid canonical state update"])
+    ]
 def _rule_reviewer_mutation_capability_proof(report_text: str) -> list[dict[str, str]]:
     from reviewer_mutation_capability_proof import assess_mutation_capability_proof
 
@@ -1420,6 +1436,7 @@ _SHARED_ISSUE_LOCK_RULES = (
     _rule_shared_issue_lock_external_state,
     _rule_shared_manual_lock_pr_override,
     _rule_shared_author_reviewer_same_run,
+    _rule_shared_canonical_state_update,
 )
 
 _SHARED_CLEANUP_PROOF_RULES = (

skills/llm-project-workflow/SKILL.md

@@ -167,6 +167,7 @@ Ready-to-copy task prompts live in [`templates/`](templates/):
 - [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md)
 - [`worktree-cleanup.md`](templates/worktree-cleanup.md)
 - [`release-tag.md`](templates/release-tag.md)
+- [`canonical-state-comments.md`](templates/canonical-state-comments.md)
 
 ## Adapting to a project
 
@@ -195,4 +196,3 @@ If and only if a controller posts a durable `BOOTSTRAP REVIEW AUTHORIZATION
 `docs/bootstrap-review-path.md`
 
 Otherwise stop with BLOCKED + DIAGNOSE. Bootstrap never weakens normal PR gates.
-

skills/llm-project-workflow/templates/canonical-state-comments.md

@@ -0,0 +1,148 @@
+# Template: canonical state comments
+
+Use these only for workflow-changing comments. Casual discussion does not need
+the full template.
+
+## Issue
+
+```text
+## Canonical Issue State
+
+STATE:
+<ready-for-author | in-progress | blocked | PR-open | needs-review | ready-to-merge | merged | closed | superseded>
+
+WHO_IS_NEXT:
+<controller | author | reviewer | merger | reconciler | user>
+
+NEXT_ACTION:
+<specific one-sentence action>
+
+NEXT_PROMPT:
+<paste-ready prompt for the next role>
+
+WHAT_HAPPENED:
+<latest meaningful event>
+
+WHY:
+<decision rationale>
+
+RELATED_DISCUSSION:
+<link/reference or none>
+
+RELATED_PRS:
+- #...
+
+BRANCH:
+<branch or none>
+
+HEAD_SHA:
+<40-character SHA or none>
+
+VALIDATION:
+<tests/proofs or none>
+
+BLOCKERS:
+<blocker and unblock condition, or none>
+
+LAST_UPDATED_BY:
+<identity/profile/date>
+```
+
+## PR
+
+```text
+## Canonical PR State
+
+STATE:
+<needs-review | changes-requested | approved | stale-approval | ready-to-merge | merged | blocked | superseded>
+
+WHO_IS_NEXT:
+<controller | author | reviewer | merger | reconciler | user>
+
+NEXT_ACTION:
+<specific one-sentence action>
+
+NEXT_PROMPT:
+<paste-ready prompt for the next role>
+
+WHAT_HAPPENED:
+<latest meaningful event>
+
+WHY:
+<decision rationale>
+
+ISSUE:
+#...
+
+BASE:
+<branch>
+
+HEAD:
+<branch>
+
+HEAD_SHA:
+<40-character SHA>
+
+REVIEW_STATUS:
+<none | approved | changes-requested | stale | contaminated>
+
+VALIDATION:
+<tests/proofs>
+
+BLOCKERS:
+<blockers or none>
+
+SUPERSEDES:
+<PRs or none>
+
+SUPERSEDED_BY:
+<PR or none>
+
+MERGE_READY:
+<yes/no and why>
+
+LAST_UPDATED_BY:
+<identity/profile/date>
+```
+
+## Discussion
+
+```text
+## Canonical Discussion Summary
+
+STATE:
+<needs-more-discussion | ready-for-issues | issues-created | closed>
+
+WHO_IS_NEXT:
+<controller | author | reviewer | user>
+
+DECISION:
+<what was decided>
+
+WHY:
+<reasoning and tradeoffs>
+
+SUBSTANTIVE_COMMENTS:
+<count and summary>
+
+ISSUES_TO_CREATE_OR_CREATED:
+- #...
+
+DEPENDENCY_ORDER:
+<order or none>
+
+NON_GOALS:
+<non-goals>
+
+OPEN_QUESTIONS:
+<questions or none>
+
+NEXT_ACTION:
+<specific one-sentence action>
+
+NEXT_PROMPT:
+<paste-ready prompt for the next role>
+
+LAST_UPDATED_BY:
+<identity/profile/date>
+```

tests/test_canonical_state_comments.py

@@ -0,0 +1,143 @@
+"""Tests for canonical state comment validation (#495)."""
+import sys
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from canonical_state_comments import (  # noqa: E402
+    validate_canonical_state_comment,
+    validate_final_report_state_update,
+)
+
+
+GOOD_ISSUE_STATE = """## Canonical Issue State
+
+STATE:
+ready-for-author
+
+WHO_IS_NEXT:
+author
+
+NEXT_ACTION:
+Implement the focused validator and open a PR.
+
+NEXT_PROMPT:
+Find issue #495, load the work-issue workflow, implement the validator, and open a PR.
+
+WHAT_HAPPENED:
+Controller selected the canonical tracking issue.
+
+WHY:
+The duplicate issue points here as canonical.
+
+RELATED_PRS:
+none
+
+BRANCH:
+none
+
+HEAD_SHA:
+none
+
+VALIDATION:
+none
+
+BLOCKERS:
+none
+"""
+
+
+class TestCanonicalStateComments(unittest.TestCase):
+    def test_good_issue_state_validates(self):
+        result = validate_canonical_state_comment(GOOD_ISSUE_STATE)
+        self.assertTrue(result["valid"])
+        self.assertEqual(result["reasons"], [])
+
+    def test_missing_next_actor_is_rejected(self):
+        result = validate_canonical_state_comment(
+            GOOD_ISSUE_STATE.replace("WHO_IS_NEXT:\nauthor", "WHO_IS_NEXT:\n")
+        )
+        self.assertFalse(result["valid"])
+        self.assertIn("WHO_IS_NEXT", " ".join(result["reasons"]))
+
+    def test_missing_next_prompt_is_rejected(self):
+        result = validate_canonical_state_comment(
+            GOOD_ISSUE_STATE.replace(
+                "NEXT_PROMPT:\nFind issue #495, load the work-issue workflow, implement the validator, and open a PR.",
+                "NEXT_PROMPT:\n",
+            )
+        )
+        self.assertFalse(result["valid"])
+        self.assertIn("NEXT_PROMPT", " ".join(result["reasons"]))
+
+    def test_vague_next_action_is_rejected(self):
+        result = validate_canonical_state_comment(
+            GOOD_ISSUE_STATE.replace(
+                "NEXT_ACTION:\nImplement the focused validator and open a PR.",
+                "NEXT_ACTION:\ncontinue",
+            )
+        )
+        self.assertFalse(result["valid"])
+        self.assertIn("too vague", " ".join(result["reasons"]))
+
+    def test_ready_to_merge_requires_approval_and_head_sha(self):
+        text = """## Canonical PR State
+
+STATE:
+ready-to-merge
+
+WHO_IS_NEXT:
+merger
+
+NEXT_ACTION:
+Merge PR #12 after checking the current head.
+
+NEXT_PROMPT:
+Load the merge workflow and merge PR #12 if gates pass.
+
+ISSUE:
+#11
+
+HEAD_SHA:
+abc123
+
+REVIEW_STATUS:
+none
+
+MERGE_READY:
+yes
+"""
+        result = validate_canonical_state_comment(text)
+        self.assertFalse(result["valid"])
+        reasons = " ".join(result["reasons"])
+        self.assertIn("approved REVIEW_STATUS", reasons)
+        self.assertIn("full HEAD_SHA", reasons)
+
+    def test_superseded_requires_canonical_item(self):
+        text = GOOD_ISSUE_STATE.replace("STATE:\nready-for-author", "STATE:\nsuperseded")
+        result = validate_canonical_state_comment(text)
+        self.assertFalse(result["valid"])
+        self.assertIn("canonical item", " ".join(result["reasons"]))
+
+    def test_blocked_requires_unblock_condition(self):
+        text = GOOD_ISSUE_STATE.replace("STATE:\nready-for-author", "STATE:\nblocked")
+        text = text.replace("BLOCKERS:\nnone", "BLOCKERS:\n")
+        result = validate_canonical_state_comment(text)
+        self.assertFalse(result["valid"])
+        self.assertIn("BLOCKERS", " ".join(result["reasons"]))
+
+    def test_final_report_claim_without_block_is_rejected(self):
+        report = "Posted canonical state comment on issue #495."
+        result = validate_final_report_state_update(report)
+        self.assertTrue(result["applicable"])
+        self.assertFalse(result["valid"])
+
+    def test_final_report_without_state_claim_not_applicable(self):
+        result = validate_final_report_state_update("ordinary final report")
+        self.assertFalse(result["applicable"])
+        self.assertTrue(result["valid"])
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_final_report_validator.py

@@ -351,6 +351,92 @@ class TestReconciliationRules(unittest.TestCase):
         )
 
 
+class TestCanonicalStateUpdateRules(unittest.TestCase):
+    def test_claimed_state_comment_without_block_blocks(self):
+        report = _reconcile_handoff() + "\nPosted canonical state comment on PR #99."
+        result = assess_final_report_validator(report, "reconcile_already_landed")
+        self.assertTrue(result["blocked"])
+        self.assertTrue(
+            any(f["rule_id"] == "shared.canonical_state_update" for f in result["findings"])
+        )
+
+    def test_state_comment_missing_next_prompt_blocks(self):
+        report = (
+            _reconcile_handoff()
+            + """
+
+## Canonical PR State
+
+STATE:
+needs-review
+
+WHO_IS_NEXT:
+reviewer
+
+NEXT_ACTION:
+Review PR #99.
+
+NEXT_PROMPT:
+
+ISSUE:
+#98
+
+HEAD_SHA:
+0fdc8f582026b72a229d59a172c0a63ac4aaeaf9
+
+REVIEW_STATUS:
+none
+
+MERGE_READY:
+no
+"""
+        )
+        result = assess_final_report_validator(report, "reconcile_already_landed")
+        self.assertTrue(result["blocked"])
+        reasons = " ".join(f["reason"] for f in result["findings"])
+        self.assertIn("NEXT_PROMPT", reasons)
+
+    def test_valid_state_comment_claim_passes_shared_rule(self):
+        report = (
+            _reconcile_handoff()
+            + """
+
+## Canonical PR State
+
+STATE:
+needs-review
+
+WHO_IS_NEXT:
+reviewer
+
+NEXT_ACTION:
+Review PR #99 against the pinned head.
+
+NEXT_PROMPT:
+Load the review workflow and review PR #99.
+
+ISSUE:
+#98
+
+HEAD_SHA:
+0fdc8f582026b72a229d59a172c0a63ac4aaeaf9
+
+REVIEW_STATUS:
+none
+
+MERGE_READY:
+no
+
+BLOCKERS:
+none
+"""
+        )
+        result = assess_final_report_validator(report, "reconcile_already_landed")
+        self.assertFalse(
+            any(f["rule_id"] == "shared.canonical_state_update" for f in result["findings"])
+        )
+
+
 class TestCanonicalReconcileSchema(unittest.TestCase):
     """Issue #307: reconciliation workflows emit only the canonical schema."""