From 33d01ba65e0a0fe250ec9dee2f2af8d8bc141721 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Wed, 8 Jul 2026 03:27:37 -0400 Subject: [PATCH] feat: add controller issue-acceptance gate (Closes #500) Add issue_acceptance_gate validation for Controller Issue Acceptance comments, final-report rules blocking merge-only issue completion claims, documentation/templates, and regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/issue-acceptance-gate.md | 52 +++ docs/llm-workflow-runbooks.md | 1 + final_report_validator.py | 18 ++ issue_acceptance_gate.py | 300 ++++++++++++++++++ .../templates/controller-issue-acceptance.md | 68 ++++ tests/test_issue_acceptance_gate.py | 183 +++++++++++ 6 files changed, 622 insertions(+) create mode 100644 docs/issue-acceptance-gate.md create mode 100644 issue_acceptance_gate.py create mode 100644 skills/llm-project-workflow/templates/controller-issue-acceptance.md create mode 100644 tests/test_issue_acceptance_gate.py diff --git a/docs/issue-acceptance-gate.md b/docs/issue-acceptance-gate.md new file mode 100644 index 0000000..522825b --- /dev/null +++ b/docs/issue-acceptance-gate.md @@ -0,0 +1,52 @@ +# Controller Issue-Acceptance Gate + +A merged PR does not automatically prove an issue is fully satisfied. After +merge, a controller must audit the linked issue against its acceptance criteria +and post a durable handoff before the issue is treated as complete. + +## Workflow position + +1. Author implements the issue and opens a PR. +2. Reviewer reviews the PR. +3. Merger merges the approved PR. +4. **Controller performs issue-acceptance audit.** +5. Controller posts a `## Controller Issue Acceptance` comment with either: + - `STATE: accepted` and checked criteria, or + - a rejection path (`more-work-required`, `needs-tests`, `needs-docs`, etc.) + with `MISSING_WORK` and a paste-ready `NEXT_PROMPT`. + +Gitea may auto-close an issue via `Closes #N` in the PR body. That closure is +merge mechanics only. Controller acceptance is still required before any final +report or queue controller treats the issue as complete. + +## Template + +Use `issue_acceptance_gate.render_controller_acceptance_template()` or the +copy in +[`skills/llm-project-workflow/templates/controller-issue-acceptance.md`](../skills/llm-project-workflow/templates/controller-issue-acceptance.md). + +## Final-report rules + +Final reports must not claim `issue complete` solely because a PR merged. +Either: + +- include a valid `## Controller Issue Acceptance` block with + `STATE: accepted`, or +- explicitly state `controller acceptance pending` and identify the controller + as the next actor. + +`final_report_validator` enforces this through +`issue_acceptance_gate.validate_final_report_issue_acceptance()`. + +## Role boundaries + +- Authors must not mark their own issues accepted. +- Reviewers must not mark issue acceptance unless acting under controller + capability. +- Mergers merge PRs; they do not substitute for controller acceptance. + +## Related + +- #495 — canonical next-action comment templates +- #496 — fail-closed canonical comment validation before posting +- #303 — controller handoff schema for reconciliation workflows \ No newline at end of file diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index e0ff616..a6ceb30 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -821,6 +821,7 @@ scripts/release-tag v0.4.0 --notes-file /tmp/release-notes.md --push ## Related documents +- [`issue-acceptance-gate.md`](issue-acceptance-gate.md) — controller issue-acceptance audit after PR merge (#500). - [`../skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md) — portable cross-project LLM workflow skill. - [`gitea-execution-profiles.md`](gitea-execution-profiles.md) — the profile model. - [`gitea-dual-namespace-deployment.md`](gitea-dual-namespace-deployment.md) — static author/reviewer namespace deployment (#139 decision). diff --git a/final_report_validator.py b/final_report_validator.py index 352a3ff..bc42b93 100644 --- a/final_report_validator.py +++ b/final_report_validator.py @@ -11,6 +11,7 @@ import inspect import re from typing import Any, Callable +import issue_acceptance_gate import issue_lock_provenance from review_proofs import ( HANDOFF_HEADING, @@ -1007,6 +1008,22 @@ def _rule_shared_manual_lock_pr_override(report_text: str) -> list[dict[str, str ) +def _rule_shared_issue_acceptance_gate(report_text: str) -> list[dict[str, str]]: + result = issue_acceptance_gate.validate_final_report_issue_acceptance(report_text) + if not result.get("applicable") or result.get("valid"): + return [] + return _findings_from_reasons( + "shared.issue_acceptance_gate", + result.get("reasons") or [], + field="Controller acceptance", + severity="block", + safe_next_action=( + "add Controller Issue Acceptance proof or state that controller " + "acceptance is pending; do not claim issue complete from PR merge alone" + ), + ) + + def _rule_shared_author_reviewer_same_run(report_text: str) -> list[dict[str, str]]: result = issue_lock_provenance.assess_author_reviewer_same_run_report(report_text) if result.get("proven"): @@ -1095,6 +1112,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = { _rule_shared_controller_handoff, _rule_shared_email_disclosure, *_SHARED_ISSUE_LOCK_RULES, + _rule_shared_issue_acceptance_gate, _rule_reviewer_vague_mutations_none, _rule_conflict_fix_push_proof, ], diff --git a/issue_acceptance_gate.py b/issue_acceptance_gate.py new file mode 100644 index 0000000..cd731c2 --- /dev/null +++ b/issue_acceptance_gate.py @@ -0,0 +1,300 @@ +"""Controller issue-acceptance gate helpers (#500). + +Pure validation for controller acceptance comments and final-report claims +that an issue is complete. Does not post comments or close issues. +""" + +from __future__ import annotations + +import re + +ACCEPTANCE_HEADING = "controller issue acceptance" + +REQUIRED_FIELDS = ( + "STATE", + "WHO_IS_NEXT", + "NEXT_ACTION", + "NEXT_PROMPT", + "ISSUE", + "MERGED_PR", + "MERGE_COMMIT", + "ACCEPTANCE_CRITERIA_CHECKED", + "VALIDATION_REVIEWED", + "CONTROLLER_DECISION", + "WHY", +) + +ACCEPTED_STATES = frozenset({"accepted"}) +REJECTION_STATES = frozenset({ + "more-work-required", + "more_work_required", + "needs-tests", + "needs_tests", + "needs-docs", + "needs_docs", + "needs-feature-enhancement", + "needs_feature_enhancement", + "needs-follow-up-issue", + "needs_follow_up_issue", + "blocked", +}) + +ALLOWED_NEXT_ACTORS = frozenset({ + "controller", + "author", + "reviewer", + "merger", + "reconciler", + "user", +}) + +_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) +_ISSUE_REF_RE = re.compile(r"#\d+") +_PR_REF_RE = re.compile(r"#\d+") +_CHECKED_ITEM_RE = re.compile(r"\[[xX]\]") +_UNCHECKED_ITEM_RE = re.compile(r"\[[\s]\]") + +_CLAIMS_ISSUE_COMPLETE_RE = re.compile( + r"\bissue\s+(?:is\s+)?(?:complete|completed|accepted|closed\s+as\s+complete|fully\s+satisfied)\b|" + r"\bissue\s+acceptance\s*:\s*accepted\b|" + r"\bcontroller\s+acceptance\s*:\s*(?:accepted|complete)\b", + re.IGNORECASE, +) +_MERGE_ONLY_COMPLETE_RE = re.compile( + r"(?:pr\s+merged|merged\s+pr|merge\s+result\s*:\s*merged).{0,120}" + r"(?:issue\s+(?:is\s+)?(?:complete|closed|accepted)|issue\s+complete)", + re.IGNORECASE | re.DOTALL, +) +_CLAIMS_CONTROLLER_ACCEPTANCE_RE = re.compile( + r"controller\s+issue\s+acceptance|controller\s+acceptance\s+(?:posted|complete|pending)", + re.IGNORECASE, +) +_PENDING_ACCEPTANCE_RE = re.compile( + r"controller\s+acceptance\s+(?:pending|required|not\s+(?:yet\s+)?(?:performed|complete))", + re.IGNORECASE, +) + + +def render_controller_acceptance_template() -> str: + """Return the canonical controller issue-acceptance comment template.""" + return """## Controller Issue Acceptance + +STATE: + + +WHO_IS_NEXT: + + +NEXT_ACTION: + + +NEXT_PROMPT: + + +ISSUE: +#... + +MERGED_PR: +#... + +MERGE_COMMIT: +<40-character SHA> + +ACCEPTANCE_CRITERIA_CHECKED: +- [x] ... +- [ ] ... + +VALIDATION_REVIEWED: + + +CONTROLLER_DECISION: + + +WHY: + + +MISSING_WORK: + + +FOLLOW_UP_ISSUES: + + +BLOCKERS: + + +LAST_UPDATED_BY: + +""" + + +def extract_acceptance_fields(text: str | None) -> dict[str, str]: + """Return upper-case labeled fields from a controller acceptance 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: + existing = fields.get(current_key, "") + fields[current_key] = ( + f"{existing}\n{stripped}" if existing else stripped + ) + return fields + + +def contains_acceptance_block(text: str | None) -> bool: + return ACCEPTANCE_HEADING in (text or "").lower() + + +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 _normalize_state(value: str | None) -> str: + return (value or "").strip().lower().replace(" ", "_").replace("-", "_") + + +def validate_controller_acceptance_comment(text: str | None) -> dict: + """Validate a controller issue-acceptance comment.""" + body = text or "" + if not contains_acceptance_block(body): + return { + "valid": False, + "fields": {}, + "reasons": ["missing Controller Issue Acceptance heading"], + } + + fields = extract_acceptance_fields(body) + reasons: list[str] = [] + + for field in REQUIRED_FIELDS: + if _empty_or_placeholder(fields.get(field)): + reasons.append(f"missing required controller acceptance field: {field}") + + state = _normalize_state(fields.get("STATE")) + if state and state not in ACCEPTED_STATES and state not in REJECTION_STATES: + reasons.append( + "STATE must be accepted or a rejection path " + "(more-work-required, needs-tests, needs-docs, " + "needs-feature-enhancement, needs-follow-up-issue, blocked)" + ) + + 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 not _ISSUE_REF_RE.search(fields.get("ISSUE") or ""): + reasons.append("ISSUE must cite an issue number (#N)") + if not _PR_REF_RE.search(fields.get("MERGED_PR") or ""): + reasons.append("MERGED_PR must cite a merged PR number (#N)") + if not _FULL_SHA_RE.search(fields.get("MERGE_COMMIT") or ""): + reasons.append("MERGE_COMMIT must include a full 40-character SHA") + + criteria = fields.get("ACCEPTANCE_CRITERIA_CHECKED") or "" + if not _CHECKED_ITEM_RE.search(criteria) and not _UNCHECKED_ITEM_RE.search(criteria): + reasons.append( + "ACCEPTANCE_CRITERIA_CHECKED must list checked/unchecked criteria items" + ) + + decision = (fields.get("CONTROLLER_DECISION") or "").strip().lower() + if state in ACCEPTED_STATES: + if decision not in {"accepted", "accept"}: + reasons.append("accepted STATE requires CONTROLLER_DECISION: accepted") + if not _CHECKED_ITEM_RE.search(criteria): + reasons.append( + "accepted STATE requires at least one checked acceptance criterion" + ) + if _empty_or_placeholder(fields.get("WHY")): + reasons.append("accepted STATE requires WHY with acceptance rationale") + elif state in REJECTION_STATES: + if decision not in {"rejected", "reject", "more_work_required"}: + reasons.append( + "rejection STATE requires CONTROLLER_DECISION: rejected" + ) + if _empty_or_placeholder(fields.get("NEXT_PROMPT")): + reasons.append( + "rejection STATE requires a paste-ready NEXT_PROMPT for the next actor" + ) + missing = fields.get("MISSING_WORK") or "" + if _empty_or_placeholder(missing): + reasons.append( + "rejection STATE requires MISSING_WORK describing what is still needed" + ) + + return { + "valid": not reasons, + "fields": fields, + "reasons": reasons, + } + + +def claims_issue_complete(text: str | None) -> bool: + """Return True when text claims an issue is complete/accepted.""" + return bool(_CLAIMS_ISSUE_COMPLETE_RE.search(text or "")) + + +def claims_merge_only_issue_complete(text: str | None) -> bool: + """Return True when text treats PR merge as issue completion.""" + return bool(_MERGE_ONLY_COMPLETE_RE.search(text or "")) + + +def claims_controller_acceptance_update(text: str | None) -> bool: + return bool(_CLAIMS_CONTROLLER_ACCEPTANCE_RE.search(text or "")) + + +def notes_controller_acceptance_pending(text: str | None) -> bool: + return bool(_PENDING_ACCEPTANCE_RE.search(text or "")) + + +def validate_final_report_issue_acceptance(report_text: str | None) -> dict: + """Validate issue-completion and controller-acceptance claims in final reports.""" + text = report_text or "" + reasons: list[str] = [] + + complete_claim = claims_issue_complete(text) + merge_only = claims_merge_only_issue_complete(text) + acceptance_claim = claims_controller_acceptance_update(text) + pending_noted = notes_controller_acceptance_pending(text) + has_block = contains_acceptance_block(text) + + if merge_only and not (has_block and validate_controller_acceptance_comment(text)["valid"]): + reasons.append( + "final report treats PR merge as issue completion without controller acceptance proof" + ) + + if complete_claim and not pending_noted: + if not has_block: + reasons.append( + "final report claims issue complete but includes no Controller Issue Acceptance block" + ) + else: + result = validate_controller_acceptance_comment(text) + if not result["valid"]: + reasons.extend(result["reasons"]) + else: + state = _normalize_state(result["fields"].get("STATE")) + if state not in ACCEPTED_STATES: + reasons.append( + "final report claims issue complete but controller STATE is not accepted" + ) + + if acceptance_claim and has_block: + result = validate_controller_acceptance_comment(text) + if not result["valid"]: + reasons.extend(result["reasons"]) + + applicable = complete_claim or merge_only or acceptance_claim or pending_noted + return { + "applicable": applicable, + "valid": not reasons, + "reasons": reasons, + } \ No newline at end of file diff --git a/skills/llm-project-workflow/templates/controller-issue-acceptance.md b/skills/llm-project-workflow/templates/controller-issue-acceptance.md new file mode 100644 index 0000000..148ef30 --- /dev/null +++ b/skills/llm-project-workflow/templates/controller-issue-acceptance.md @@ -0,0 +1,68 @@ +# Controller issue-acceptance prompt + +Use after a PR merges when auditing whether the linked issue is truly complete. + +```text +Audit issue # against its acceptance criteria after merged PR #. +Post a Controller Issue Acceptance comment with checked criteria, validation +reviewed, controller decision, next actor, and paste-ready next prompt. +Do not mark the issue accepted unless every required criterion is satisfied. +``` + +## Comment template + +```text +## Controller Issue Acceptance + +STATE: + + +WHO_IS_NEXT: + + +NEXT_ACTION: + + +NEXT_PROMPT: + + +ISSUE: +#... + +MERGED_PR: +#... + +MERGE_COMMIT: +<40-character SHA> + +ACCEPTANCE_CRITERIA_CHECKED: +- [x] ... +- [ ] ... + +VALIDATION_REVIEWED: + + +CONTROLLER_DECISION: + + +WHY: + + +MISSING_WORK: + + +FOLLOW_UP_ISSUES: + + +BLOCKERS: + + +LAST_UPDATED_BY: + +``` + +## Rejection paths + +When rejecting completion, `STATE` must name the gap (`needs-tests`, +`needs-docs`, `more-work-required`, etc.), `MISSING_WORK` must be explicit, +and `NEXT_PROMPT` must be ready for the next author session. \ No newline at end of file diff --git a/tests/test_issue_acceptance_gate.py b/tests/test_issue_acceptance_gate.py new file mode 100644 index 0000000..ead154d --- /dev/null +++ b/tests/test_issue_acceptance_gate.py @@ -0,0 +1,183 @@ +"""Tests for controller issue-acceptance gate (#500).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import issue_acceptance_gate # noqa: E402 +from final_report_validator import assess_final_report_validator # noqa: E402 + + +def _accepted_comment(**overrides): + lines = [ + "## Controller Issue Acceptance", + "", + "STATE:", + "accepted", + "", + "WHO_IS_NEXT:", + "controller", + "", + "NEXT_ACTION:", + "Close the tracker follow-up after verification.", + "", + "NEXT_PROMPT:", + "Verify deployment checklist for issue #500 and post acceptance.", + "", + "ISSUE:", + "#500", + "", + "MERGED_PR:", + "#503", + "", + "MERGE_COMMIT:", + "0fdc8f582026b72a229d59a172c0a63ac4aaeaf9", + "", + "ACCEPTANCE_CRITERIA_CHECKED:", + "- [x] documentation added", + "- [x] validator tests added", + "", + "VALIDATION_REVIEWED:", + "pytest tests/test_issue_acceptance_gate.py -q", + "", + "CONTROLLER_DECISION:", + "accepted", + "", + "WHY:", + "All acceptance criteria satisfied with proof.", + "", + "MISSING_WORK:", + "none", + "", + "FOLLOW_UP_ISSUES:", + "none", + "", + "BLOCKERS:", + "none", + "", + "LAST_UPDATED_BY:", + "jcwalker3 / prgs-controller / 2026-07-08", + ] + text = "\n".join(lines) + for key, value in overrides.items(): + text = text.replace(f"{key}:\n", f"{key}:\n{value}\n", 1) + return text + + +def _rejection_comment(state="needs-tests"): + text = _accepted_comment() + text = text.replace("STATE:\naccepted", f"STATE:\n{state}") + text = text.replace( + "CONTROLLER_DECISION:\naccepted", + "CONTROLLER_DECISION:\nrejected", + ) + text = text.replace( + "ACCEPTANCE_CRITERIA_CHECKED:\n- [x] documentation added\n- [x] validator tests added", + "ACCEPTANCE_CRITERIA_CHECKED:\n- [ ] regression tests for rejection paths", + ) + text = text.replace( + "MISSING_WORK:\nnone", + "MISSING_WORK:\nAdd regression tests for controller rejection paths.", + ) + text = text.replace( + "NEXT_PROMPT:\nVerify deployment checklist for issue #500 and post acceptance.", + "NEXT_PROMPT:\nImplement the missing tests for issue #500 and reopen the PR.", + ) + return text + + +class TestControllerAcceptanceComment(unittest.TestCase): + def test_accepted_comment_valid(self): + result = issue_acceptance_gate.validate_controller_acceptance_comment( + _accepted_comment() + ) + self.assertTrue(result["valid"], result["reasons"]) + + def test_missing_who_is_next_rejected(self): + text = _accepted_comment().replace("WHO_IS_NEXT:\ncontroller\n", "WHO_IS_NEXT:\n\n") + result = issue_acceptance_gate.validate_controller_acceptance_comment(text) + self.assertFalse(result["valid"]) + self.assertTrue(any("WHO_IS_NEXT" in r for r in result["reasons"])) + + def test_missing_next_prompt_on_rejection(self): + text = _rejection_comment() + text = text.replace( + "NEXT_PROMPT:\nImplement the missing tests for issue #500 and reopen the PR.\n", + "NEXT_PROMPT:\n\n", + ) + result = issue_acceptance_gate.validate_controller_acceptance_comment(text) + self.assertFalse(result["valid"]) + self.assertTrue(any("NEXT_PROMPT" in r for r in result["reasons"])) + + def test_vague_merge_only_completion_detected(self): + text = "PR merged. Issue complete." + self.assertTrue(issue_acceptance_gate.claims_merge_only_issue_complete(text)) + + def test_rejection_paths_require_missing_work(self): + for state in ( + "more-work-required", + "needs-tests", + "needs-docs", + "needs-feature-enhancement", + "needs-follow-up-issue", + ): + text = _rejection_comment(state=state).replace( + "MISSING_WORK:\nAdd regression tests for controller rejection paths.\n", + "MISSING_WORK:\n\n", + ) + result = issue_acceptance_gate.validate_controller_acceptance_comment(text) + self.assertFalse(result["valid"], state) + self.assertTrue(any("MISSING_WORK" in r for r in result["reasons"])) + + def test_accepted_requires_checked_criteria(self): + text = _accepted_comment().replace( + "ACCEPTANCE_CRITERIA_CHECKED:\n- [x] documentation added\n- [x] validator tests added\n", + "ACCEPTANCE_CRITERIA_CHECKED:\n- [ ] documentation added\n", + ) + result = issue_acceptance_gate.validate_controller_acceptance_comment(text) + self.assertFalse(result["valid"]) + self.assertTrue(any("checked acceptance criterion" in r for r in result["reasons"])) + + +class TestFinalReportIntegration(unittest.TestCase): + def test_merge_only_complete_blocks_work_issue_report(self): + report = ( + "## Controller Handoff\n\n" + "- Task: work issue #500\n" + "- Merge result: merged\n" + "- Current status: PR merged; issue complete\n" + ) + result = assess_final_report_validator(report, "work_issue") + self.assertTrue( + any( + f["rule_id"] == "shared.issue_acceptance_gate" + for f in result["findings"] + ), + result["findings"], + ) + + def test_pending_acceptance_allowed(self): + report = ( + "## Controller Handoff\n\n" + "- Task: work issue #500\n" + "- Merge result: merged\n" + "- Current status: controller acceptance pending\n" + ) + result = assess_final_report_validator(report, "work_issue") + self.assertFalse( + any( + f["rule_id"] == "shared.issue_acceptance_gate" + for f in result["findings"] + ), + result["findings"], + ) + + def test_accepted_block_allows_complete_claim(self): + report = _accepted_comment() + "\n\nIssue is complete." + gate = issue_acceptance_gate.validate_final_report_issue_acceptance(report) + self.assertTrue(gate["valid"], gate["reasons"]) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file -- 2.43.7