Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions skills/issue-driven-github-flow/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,27 @@ and visible, not trapped in a chat):
`## 🔍 Review (review agent)`. It ends with an explicit verdict line:
`**Verdict: APPROVED**` or `**Verdict: CHANGES REQUESTED**`.
3. If changes are requested, the planning agent revises in a new comment. Repeat
until the review agent posts `**Verdict: APPROVED**`.
until the review agent posts `**Verdict: APPROVED**` or a hard stop below
forces human escalation.

**Orchestrator loop bound:** the orchestrator owns enforcement, not the
subagents. Maintain a visible counter in the issue comments or dispatch notes
(`Plan-review round 1/3`, then `2/3`, then `3/3`). One round is one planning
comment plus the review verdict for it. The plan ↔ review loop is capped at
**max 3 rounds**; never start a fourth review round.

Stop early and escalate instead of burning remaining rounds when there is no
substantive progress. Compare the latest plan/review against the prior round:
if the reviewer repeats the same objection without new information, or the
planner's revision does not materially change the plan in response, treat the
loop as non-converged. This is a judgment about substantive change, not exact
text equality.

On cap hit or no-progress, STOP and post a concise issue comment prefixed, for
example, `## ⛔ Escalation: plan review did not converge`. Summarize the
unresolved disagreement, include the final round counter (`3/3` when capped),
and hand off to the human for a decision. Never silently loop, silently give up,
or begin implementation without an approved plan or explicit human direction.

Post comments by piping the body on **stdin via a quoted heredoc** — plans and
reviews are full of code fences, backticks, and `$`, and `--body "..."` would let
Expand All @@ -183,8 +203,8 @@ EOF

Don't skip the loop even for "small" changes — the review agent approving a
one-line plan in a single round costs almost nothing, and the discipline is what
keeps the *big* changes honest. Cap it at ~3 rounds; if they can't converge,
stop and bring the disagreement to the user.
keeps the *big* changes honest. Keep the counter visible even when you expect a
single round.

## Step 3 — Implement

Expand Down Expand Up @@ -266,6 +286,24 @@ The code-review agent posts a PR review with `**Verdict: APPROVED**` or
- **Minor** findings are non-blocking unless the human decides otherwise.
- The implementer uses [receiving-code-review.md](references/receiving-code-review.md)
to triage feedback rigorously rather than blindly applying suggestions.
- The orchestrator tracks the code-review ↔ implement loop with a visible
counter (`Code-review fix round 1/3`, `2/3`, `3/3`). One round is one
code-review verdict plus the implementer's response/fix attempt. The loop is
capped at **max 3 rounds**; never request a fourth code-review pass for the
same unresolved Critical/Important findings.
- If the implementation agent reports `BLOCKED` or `NEEDS_CONTEXT`, stop
immediately and escalate to the human instead of using another round.
- If a round produces no substantive progress, stop early and escalate. Compare
the latest diff, implementer status, and review against the prior round: an
implementer making no material edit, or a reviewer repeating the same blocking
objection against an unchanged diff, is non-convergence. This is substantive
comparison, not exact text equality.
- On cap hit, `BLOCKED`, `NEEDS_CONTEXT`, or no-progress, STOP and post a
concise PR comment prefixed, for example, `## ⛔ Escalation: code review did
not converge`. Summarize unresolved Critical/Important findings or the missing
context, include the final round counter when applicable, and hand off to the
human. Do not mark the PR ready until the human resolves or explicitly waives
the blockers.
- The human gives final merge approval. An agent may prepare the PR, but it does
not override human approval.

Expand Down Expand Up @@ -358,12 +396,13 @@ items across status columns and linking the project to the repo.
| Default branch unprotected | Offer branch protection from [branch-protection.md](references/branch-protection.md); require consent before mutation |
| >3 open issues | Create/append to a GitHub Project |
| Multiple independent issues | Use worktrees via [worktrees.md](references/worktrees.md) |
| Plan written | Hand to review agent; iterate until `Verdict: APPROVED` |
| Plan written | Hand to review agent; track visible `Plan-review round N/3`; stop after max 3 rounds or no-progress and escalate to the human |
| Approved plan | Dispatch implementation agent |
| Step 3 branch cut | Move Project item to **In Progress** via [projects.md](references/projects.md); no-op cleanly if no Project exists or Project auth is unavailable |
| Code done | Run verification, commit, open a draft PR |
| Draft PR open | Dispatch 🔬 code-review agent; require a verdict |
| Critical/Important review finding | Loop back to implementation; re-verify and re-review |
| Implementer reports `BLOCKED` / `NEEDS_CONTEXT` | Stop the loop immediately; post the blocker/context gap and hand off to the human |
| Critical/Important review finding | Loop back to implementation with visible `Code-review fix round N/3`; re-verify and re-review, but stop after max 3 rounds or no-progress and escalate |
| PR ready to land | Require human approval and green `gh pr checks` when CI exists |
| PR squash-merged | Move Project item to **Done** via [projects.md](references/projects.md); no-op cleanly if no Project exists or Project auth is unavailable |
| PR merged | `--delete-branch`, sync `main`, confirm issue closed |
Expand Down
14 changes: 14 additions & 0 deletions skills/issue-driven-github-flow/evals/evals.json
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,20 @@
"Asks for explicit consent before applying branch protection",
"Does not mutate branch protection without consent"
]
},
{
"id": 9,
"name": "plan-review-nonconvergence-escalates",
"prompt": "Work on issue #42 where the review agent keeps rejecting the plan with the same unresolved objection.",
"expected_output": "Tracks the plan-review round count visibly, stops after no more than 3 review rounds, posts a concise escalation/hand-off comment to the issue summarizing the unresolved disagreement, and does not keep looping or start implementation without an approved plan.",
"files": [],
"assertions": [
"Orchestrator tracks the plan-review loop with a visible numeric counter",
"No more than 3 review-agent comments are posted for the non-converging plan",
"A final issue comment summarizes the unresolved disagreement and hands off to the human",
"The loop stops instead of requesting another planning/review round after the cap or no-progress guard",
"No implementation branch or PR is created without a review comment ending in 'Verdict: APPROVED'"
]
}
]
}
46 changes: 42 additions & 4 deletions skills/issue-driven-github-flow/references/agent-prompts.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,25 @@ cheaper/faster model is often enough for mechanical implementation of a reviewed
plan, while planning and especially final diff review should use a stronger
model when correctness, security, or architecture judgment matters.

## Loop-bounding contract for the orchestrator

The parent orchestrator must make both agent loops provably terminating:

- Plan ↔ review is capped at **max 3 rounds**.
- Code-review ↔ implement is capped at **max 3 rounds**.
- Track each loop with a visible counter (`Plan-review round N/3` or
`Code-review fix round N/3`) in comments or dispatch notes; never start round
4.
- `BLOCKED` and `NEEDS_CONTEXT` from the implementation agent short-circuit the
code-review loop immediately; do not spend another round.
- Apply the no-progress guard before the cap: compare the latest plan/diff/review
to the prior round for substantive change, not exact text equality. Repeated
objections or no material edit means non-convergence.
- On cap hit, no-progress, `BLOCKED`, or `NEEDS_CONTEXT`, STOP and post a concise
issue/PR escalation comment summarizing the unresolved disagreement, findings,
or missing context, then hand off to the human. Never silently loop and never
silently give up.

---

## 🗺️ Planning agent
Expand All @@ -37,6 +56,10 @@ model when correctness, security, or architecture judgment matters.
> 4. If you're revising after a review, address each point the reviewer raised
> explicitly, and post a new comment (don't edit the old one — the back-and-
> forth is the record).
> 5. The orchestrator will label each attempt `Plan-review round N/3` and stop
> after max 3 rounds or earlier on no-progress. Make each revision
> substantively address the review; if you cannot resolve an objection, say so
> plainly so the orchestrator can escalate to the human instead of looping.
>
> Post with a quoted heredoc so your markdown (code fences, backticks, `$`)
> isn't mangled by the shell:
Expand Down Expand Up @@ -66,6 +89,10 @@ model when correctness, security, or architecture judgment matters.
> actionable critique — cite files/lines. End with exactly one verdict line:
> - `**Verdict: APPROVED**` — the plan is sound and ready to implement.
> - `**Verdict: CHANGES REQUESTED**` — list what must change.
> 4. The orchestrator caps plan ↔ review at max 3 rounds with a visible
> `Plan-review round N/3` counter. If the same unresolved objection repeats
> or the plan has not substantively changed, call out non-convergence clearly
> so the orchestrator can post the escalation/hand-off comment.
>
> Post with a quoted heredoc so your markdown (code fences, backticks, `$`)
> isn't mangled by the shell:
Expand Down Expand Up @@ -100,13 +127,19 @@ model when correctness, security, or architecture judgment matters.
> improvising a different design — the plan was reviewed for a reason. If
> you're in over your head, stop and escalate with the specific uncertainty;
> don't invent a different architecture to keep moving.
> 4. Write and RUN the automated test the plan called for; confirm it passes.
> 5. Commit: `git add -A` then
> 4. If you're fixing Critical/Important code-review findings, the orchestrator
> will label the attempt `Code-review fix round N/3` and stop after max 3
> rounds or earlier on no-progress. Make a substantive diff for each fix
> attempt. If you cannot safely proceed, return `BLOCKED`; if required context
> is missing, return `NEEDS_CONTEXT`. Those status codes immediately escalate
> to the human and must not burn another round.
> 5. Write and RUN the automated test the plan called for; confirm it passes.
> 6. Commit: `git add -A` then
> `scripts/gitflow.sh commit "<type(scope): summary>" {N}`
> (adds `Closes #{N}.`).
> 6. Open the PR: `scripts/gitflow.sh pr "<conventional title>" {N}`
> 7. Open the PR: `scripts/gitflow.sh pr "<conventional title>" {N}`
> (the title becomes the squash commit). Its test plan should describe the
> green automated check from step 4, not a promise.
> green automated check from step 5, not a promise.
>
> Report the PR URL back. End with exactly one status code:
> - `DONE` — implementation is complete, verified, committed, and the PR is open.
Expand Down Expand Up @@ -146,6 +179,11 @@ model when correctness, security, or architecture judgment matters.
> - `**Verdict: CHANGES REQUESTED**` — Critical or Important issues must be
> resolved, or explicitly waived by the human, before the PR may be marked
> ready or merged.
> 6. The orchestrator caps code-review ↔ implement at max 3 rounds with a visible
> `Code-review fix round N/3` counter. If the same Critical/Important finding
> remains after a prior round, or the diff has not substantively changed,
> identify it as non-convergence so the orchestrator can post the
> escalation/hand-off comment instead of looping.
>
> Post with a quoted heredoc so your markdown (code fences, backticks, `$`)
> isn't mangled by the shell:
Expand Down
Loading