Skip to content

Commit 8d89a4d

Browse files
fix(process): enforce plan approval before implementation (#370)
Add automated enforcement for plan approval workflow: - DangerJS Rule 9: warns when linked issue lacks plan comment or approval - Updated skill-spec.md issue template with Plan Approval section - Created general-issue.md template with Plan Approval section - Updated issue-driven-delivery skill with Automated Enforcement section - Added Plan Approval Workflow section to CONTRIBUTING.md - Referenced Issue #177 as workflow exemplar Refs: #292 Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 75c6702 commit 8d89a4d

File tree

5 files changed

+207
-8
lines changed

5 files changed

+207
-8
lines changed
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
name: General Issue
3+
about: Report a bug, request a feature, or propose a change
4+
title: ""
5+
labels: []
6+
assignees: ""
7+
---
8+
9+
## Summary
10+
11+
{Brief description of the issue, feature, or change}
12+
13+
## Plan Approval
14+
15+
<!--
16+
Before implementation begins, post a plan comment on this issue:
17+
1. Create plan file at docs/plans/YYYY-MM-DD-issue-N-description.md (if complex)
18+
2. Post comment with "## Plan" header summarizing implementation approach
19+
3. Wait for approval comment before starting implementation
20+
4. See Issue #177 for exemplar
21+
22+
Plan comment template:
23+
## Plan
24+
**Approach:** {Brief description}
25+
**Files to change:** {List}
26+
**Testing:** {How you'll verify}
27+
**Awaiting approval to proceed.**
28+
-->
29+
30+
- [ ] Plan comment posted
31+
- [ ] Approval received
32+
33+
## Context
34+
35+
{Background information, motivation, or problem statement}
36+
37+
## Acceptance Criteria
38+
39+
- [ ] {Criterion 1}
40+
- [ ] {Criterion 2}
41+
- [ ] {Criterion 3}
42+
43+
## Additional Information
44+
45+
{Any other relevant details, links, or screenshots}

.github/ISSUE_TEMPLATE/skill-spec.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,26 @@ assignees: ""
1212

1313
{One sentence: what agents should do when this skill triggers}
1414

15+
## Plan Approval
16+
17+
<!--
18+
Before implementation begins, post a plan comment on this issue:
19+
1. Create plan file at docs/plans/YYYY-MM-DD-issue-N-description.md (if complex)
20+
2. Post comment with "## Plan" header summarizing implementation approach
21+
3. Wait for approval comment before starting implementation
22+
4. See Issue #177 for exemplar
23+
24+
Plan comment template:
25+
## Plan
26+
**Approach:** {Brief description}
27+
**Files to change:** {List}
28+
**Testing:** {How you'll verify}
29+
**Awaiting approval to proceed.**
30+
-->
31+
32+
- [ ] Plan comment posted
33+
- [ ] Approval received
34+
1535
## Skill Priority
1636

1737
- Priority: P{0-4} ({Safety & Integrity | Quality & Correctness | Consistency & Governance |

CONTRIBUTING.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,30 @@ agent-facing documentation that helps AI agents apply proven techniques, pattern
2727
- See [README.md](README.md) for repository standards
2828
- See [AGENTS.md](AGENTS.md) for agent-specific rules
2929

30+
## Plan Approval Workflow
31+
32+
Before implementation begins on any issue, a plan must be posted and approved:
33+
34+
1. **Post plan comment** on the issue with "## Plan" header summarizing approach
35+
2. **Wait for approval** - approval comment must contain "Approval acknowledged" or "Plan approved"
36+
3. **Begin implementation** - only after explicit approval exists
37+
38+
**Enforcement:** DangerJS Rule 9 warns on PRs where the linked issue lacks plan approval.
39+
40+
**Exemplar:** [Issue #177](https://github.com/mcj-coder/development-skills/issues/177) demonstrates
41+
the complete workflow with proper plan formatting and approval.
42+
43+
**Plan comment format:**
44+
45+
```markdown
46+
## Plan
47+
48+
**Approach:** {Brief description}
49+
**Files to change:** {List}
50+
**Testing:** {How you'll verify}
51+
**Awaiting approval to proceed.**
52+
```
53+
3054
## Creating Skills
3155

3256
### Process Overview

dangerfile.js

Lines changed: 100 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// dangerfile.js - Evidence validation rules for PR enforcement
22
// Implements automated checks from issue-driven-delivery skill
3-
// Issue: #300
3+
// Issues: #300, #290, #291, #292
44

55
const { danger, warn, fail, message } = require("danger");
66

@@ -48,22 +48,25 @@ function findCheckedItemsWithoutEvidence(text) {
4848
return itemsWithoutEvidence;
4949
}
5050

51-
// Helper to safely get issue body (linked issue from PR)
52-
async function getLinkedIssueBody() {
51+
// Helper to extract linked issue number from PR body
52+
function getLinkedIssueNumber() {
5353
const prBody = danger.github.pr.body || "";
54-
55-
// Extract issue number from "Closes #N" or "Fixes #N" patterns
5654
const issueMatch = prBody.match(/(?:closes|fixes|resolves)\s+#(\d+)/i);
57-
if (!issueMatch) {
55+
return issueMatch ? parseInt(issueMatch[1]) : null;
56+
}
57+
58+
// Helper to safely get issue body (linked issue from PR)
59+
async function getLinkedIssueBody() {
60+
const issueNumber = getLinkedIssueNumber();
61+
if (!issueNumber) {
5862
return null;
5963
}
6064

61-
const issueNumber = issueMatch[1];
6265
try {
6366
const issue = await danger.github.api.issues.get({
6467
owner: danger.github.thisPR.owner,
6568
repo: danger.github.thisPR.repo,
66-
issue_number: parseInt(issueNumber),
69+
issue_number: issueNumber,
6770
});
6871
return issue.data.body || "";
6972
} catch (error) {
@@ -80,6 +83,72 @@ async function getLinkedIssueBody() {
8083
}
8184
}
8285

86+
// Helper to get comments on the linked issue
87+
async function getLinkedIssueComments() {
88+
const issueNumber = getLinkedIssueNumber();
89+
if (!issueNumber) {
90+
return null;
91+
}
92+
93+
try {
94+
const comments = await danger.github.api.issues.listComments({
95+
owner: danger.github.thisPR.owner,
96+
repo: danger.github.thisPR.repo,
97+
issue_number: issueNumber,
98+
});
99+
return comments.data || [];
100+
} catch (error) {
101+
console.error(
102+
`Failed to fetch comments for issue #${issueNumber}: ${error.message || error}`,
103+
);
104+
return null;
105+
}
106+
}
107+
108+
/**
109+
* Check if a comment contains a plan (implementation plan for approval)
110+
* Plan indicators:
111+
* - Contains "## Plan" or "## Implementation Plan" or "## Refinement"
112+
* - Links to docs/plans/ directory
113+
* - Contains "awaiting approval" or "ready for approval"
114+
*/
115+
function isPlanComment(commentBody) {
116+
if (!commentBody) return false;
117+
const lowerBody = commentBody.toLowerCase();
118+
119+
// Check for plan headers
120+
if (/##\s*(implementation\s+)?plan/i.test(commentBody)) return true;
121+
if (/##\s*refinement/i.test(commentBody)) return true;
122+
123+
// Check for plan file links
124+
if (/docs\/plans\//.test(commentBody)) return true;
125+
126+
// Check for approval request language
127+
if (
128+
lowerBody.includes("awaiting approval") ||
129+
lowerBody.includes("ready for approval") ||
130+
lowerBody.includes("plan ready for approval")
131+
)
132+
return true;
133+
134+
return false;
135+
}
136+
137+
/**
138+
* Check if a comment indicates plan approval
139+
*/
140+
function isApprovalComment(commentBody) {
141+
if (!commentBody) return false;
142+
const lowerBody = commentBody.toLowerCase();
143+
144+
return (
145+
lowerBody.includes("approval acknowledged") ||
146+
lowerBody.includes("approved to proceed") ||
147+
lowerBody.includes("plan approved") ||
148+
/proceed(ing)?\s+with/.test(lowerBody)
149+
);
150+
}
151+
83152
// Main validation logic
84153
async function validate() {
85154
const prBody = danger.github.pr.body || "";
@@ -214,6 +283,29 @@ async function validate() {
214283
}
215284
}
216285

286+
// Rule 9: Plan approval enforcement
287+
// Issues must have a plan comment with approval before implementation
288+
const issueComments = await getLinkedIssueComments();
289+
if (issueComments) {
290+
const hasPlanComment = issueComments.some((c) => isPlanComment(c.body));
291+
const hasApprovalComment = issueComments.some((c) =>
292+
isApprovalComment(c.body),
293+
);
294+
295+
if (!hasPlanComment) {
296+
warn(
297+
`[Issue] No plan comment found on linked issue. ` +
298+
`Post a plan comment with "## Plan" header or link to docs/plans/ before implementation. ` +
299+
`See Issue #177 for exemplar.`,
300+
);
301+
} else if (!hasApprovalComment) {
302+
warn(
303+
`[Issue] Plan found but no approval comment detected. ` +
304+
`Add approval comment (e.g., "Approval acknowledged" or "Plan approved") before implementation.`,
305+
);
306+
}
307+
}
308+
217309
// Success message if all critical checks pass
218310
const failures = danger.fails || [];
219311
if (failures.length === 0) {

skills/issue-driven-delivery/SKILL.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,24 @@ gh issue view N --json comments --jq '.comments[].body' | grep -qiE "approved|lg
935935
gh issue view N --json body,comments --jq '[.body, .comments[].body] | join(" ")' | grep -qiE "blocked by|depends on|no dependencies|dependencies: none" && echo "PASS: Dependencies documented" || echo "WARN: Dependencies not explicitly documented"
936936
```
937937

938+
### Automated Enforcement
939+
940+
Plan approval is enforced automatically via DangerJS (Rule 9). When a PR references an issue:
941+
942+
1. **Plan comment check** - DangerJS verifies the linked issue has a plan comment containing:
943+
- "## Plan" or "## Implementation Plan" or "## Refinement" header
944+
- Link to `docs/plans/` directory
945+
- "Awaiting approval" or "Ready for approval" language
946+
947+
2. **Approval check** - DangerJS verifies an approval comment exists containing:
948+
- "Approval acknowledged" or "Plan approved"
949+
- "Approved to proceed" or "Proceeding with"
950+
951+
PRs will receive warnings if plan approval is missing. See `dangerfile.js` Rule 9 for implementation.
952+
953+
**Exemplar:** [Issue #177](https://github.com/mcj-coder/development-skills/issues/177) demonstrates
954+
the complete plan approval workflow with proper comment formatting.
955+
938956
### DoR Failure Handling
939957

940958
If DoR validation fails, do NOT transition to implementation. Instead:

0 commit comments

Comments
 (0)