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) <[email protected]>
This commit is contained in:
@@ -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
|
||||||
@@ -821,6 +821,7 @@ scripts/release-tag v0.4.0 --notes-file /tmp/release-notes.md --push
|
|||||||
|
|
||||||
## Related documents
|
## 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.
|
- [`../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-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).
|
- [`gitea-dual-namespace-deployment.md`](gitea-dual-namespace-deployment.md) — static author/reviewer namespace deployment (#139 decision).
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import inspect
|
|||||||
import re
|
import re
|
||||||
from typing import Any, Callable
|
from typing import Any, Callable
|
||||||
|
|
||||||
|
import issue_acceptance_gate
|
||||||
import issue_lock_provenance
|
import issue_lock_provenance
|
||||||
from review_proofs import (
|
from review_proofs import (
|
||||||
HANDOFF_HEADING,
|
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]]:
|
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)
|
result = issue_lock_provenance.assess_author_reviewer_same_run_report(report_text)
|
||||||
if result.get("proven"):
|
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_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
*_SHARED_ISSUE_LOCK_RULES,
|
||||||
|
_rule_shared_issue_acceptance_gate,
|
||||||
_rule_reviewer_vague_mutations_none,
|
_rule_reviewer_vague_mutations_none,
|
||||||
_rule_conflict_fix_push_proof,
|
_rule_conflict_fix_push_proof,
|
||||||
],
|
],
|
||||||
|
|||||||
@@ -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:
|
||||||
|
<accepted | more-work-required | needs-tests | needs-docs | needs-feature-enhancement | needs-follow-up-issue | blocked>
|
||||||
|
|
||||||
|
WHO_IS_NEXT:
|
||||||
|
<author | reviewer | merger | reconciler | controller | user>
|
||||||
|
|
||||||
|
NEXT_ACTION:
|
||||||
|
<one sentence>
|
||||||
|
|
||||||
|
NEXT_PROMPT:
|
||||||
|
<paste-ready prompt for the next LLM>
|
||||||
|
|
||||||
|
ISSUE:
|
||||||
|
#...
|
||||||
|
|
||||||
|
MERGED_PR:
|
||||||
|
#...
|
||||||
|
|
||||||
|
MERGE_COMMIT:
|
||||||
|
<40-character SHA>
|
||||||
|
|
||||||
|
ACCEPTANCE_CRITERIA_CHECKED:
|
||||||
|
- [x] ...
|
||||||
|
- [ ] ...
|
||||||
|
|
||||||
|
VALIDATION_REVIEWED:
|
||||||
|
<tests/proofs reviewed>
|
||||||
|
|
||||||
|
CONTROLLER_DECISION:
|
||||||
|
<accepted or rejected>
|
||||||
|
|
||||||
|
WHY:
|
||||||
|
<reasoning>
|
||||||
|
|
||||||
|
MISSING_WORK:
|
||||||
|
<none, or exact missing work>
|
||||||
|
|
||||||
|
FOLLOW_UP_ISSUES:
|
||||||
|
<none, or issue list to create>
|
||||||
|
|
||||||
|
BLOCKERS:
|
||||||
|
<none, or exact blockers>
|
||||||
|
|
||||||
|
LAST_UPDATED_BY:
|
||||||
|
<identity/profile/date>
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
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,
|
||||||
|
}
|
||||||
@@ -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 #<N> against its acceptance criteria after merged PR #<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:
|
||||||
|
<accepted | more-work-required | needs-tests | needs-docs | needs-feature-enhancement | needs-follow-up-issue | blocked>
|
||||||
|
|
||||||
|
WHO_IS_NEXT:
|
||||||
|
<author | reviewer | merger | reconciler | controller | user>
|
||||||
|
|
||||||
|
NEXT_ACTION:
|
||||||
|
<one sentence>
|
||||||
|
|
||||||
|
NEXT_PROMPT:
|
||||||
|
<paste-ready prompt for the next LLM>
|
||||||
|
|
||||||
|
ISSUE:
|
||||||
|
#...
|
||||||
|
|
||||||
|
MERGED_PR:
|
||||||
|
#...
|
||||||
|
|
||||||
|
MERGE_COMMIT:
|
||||||
|
<40-character SHA>
|
||||||
|
|
||||||
|
ACCEPTANCE_CRITERIA_CHECKED:
|
||||||
|
- [x] ...
|
||||||
|
- [ ] ...
|
||||||
|
|
||||||
|
VALIDATION_REVIEWED:
|
||||||
|
<tests/proofs reviewed>
|
||||||
|
|
||||||
|
CONTROLLER_DECISION:
|
||||||
|
<accepted or rejected>
|
||||||
|
|
||||||
|
WHY:
|
||||||
|
<reasoning>
|
||||||
|
|
||||||
|
MISSING_WORK:
|
||||||
|
<none, or exact missing work>
|
||||||
|
|
||||||
|
FOLLOW_UP_ISSUES:
|
||||||
|
<none, or issue list to create>
|
||||||
|
|
||||||
|
BLOCKERS:
|
||||||
|
<none, or exact blockers>
|
||||||
|
|
||||||
|
LAST_UPDATED_BY:
|
||||||
|
<identity/profile/date>
|
||||||
|
```
|
||||||
|
|
||||||
|
## 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.
|
||||||
@@ -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()
|
||||||
Reference in New Issue
Block a user