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
32 changes: 29 additions & 3 deletions .agents/skills/pr-batch/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,28 @@ For high-risk cases above, run Claude's `/simplify` after all required review pa

Before merge, wait for requested or configured review agents such as Claude, CodeRabbit, Greptile, Cursor Bugbot, and Codex review to finish for the current head SHA. Poll CI with bounded commands and timeouts; use narrow required-check commands such as `gh pr checks <PR> --required` for required CI readiness, then also fetch all checks or explicit review-agent checks so non-required reviewers are not hidden. Avoid long-lived `gh ... --watch`. Ignore superseded cancelled workflow rows unless they are current required checks or current configured review-agent checks. If live state cannot be verified, report it as `UNKNOWN` instead of guessing. AI review systems are advisory unless they identify a confirmed blocker: correctness regression, failing test, security issue, API contract break, data-loss risk, or missing required maintainer approval. Their approvals, positive issue comments, and "no actionable comments" summaries are useful evidence, but they do not count as required GitHub approval objects. For high-risk or concurrent-batch PRs, run or request the adversarial PR review workflow in `.agents/workflows/adversarial-pr-review.md`. A completed check is not enough when review comments exist: classify and resolve or explicitly waive actionable findings before merging. Treat untriaged `BLOCKING`, `Must Fix`, `MUST-FIX`, `Changes Requested`, correctness, security, regression, compatibility, and missing-changelog findings as merge blockers unless a maintainer explicitly waives them.

Comment on lines 112 to 114

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Imprecise section pointer for canonical rules

The pointer "apply the canonical full-CI uncertainty rule and Coordinator Closeout Lane from .agents/workflows/pr-processing.md under Plan To Goal Handoff" misidentifies the target location. Both Question And Decision Handling (which contains the full-CI uncertainty rule) and Coordinator Closeout Lane are sibling ### sections under High-Concurrency Batch Launch, not children of Plan To Goal Handoff. An agent that navigates to Plan To Goal Handoff and reads downward within only that heading will find the inline goal-prompt echo of these rules but may miss the canonical numbered steps. Pointing to Question And Decision Handling and Coordinator Closeout Lane by name would eliminate the ambiguity.

For blocking questions, stop work on that target, surface the question to the coordinator or maintainer, and mark the issue/PR with the agreed pending-question state. For non-blocking questions where you make a decision and continue, record the decision in the PR description before review or merge.
At the final review/readiness gate, apply the canonical full-CI uncertainty rule and Coordinator Closeout Lane from `.agents/workflows/pr-processing.md` under **Plan To Goal Handoff**.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cross-reference "under Plan To Goal Handoff" is inaccurate. In pr-processing.md the heading hierarchy under ## High-Concurrency Batch Launch is:

### Plan To Goal Handoff          ← line ~241 (sibling)
### Question And Decision Handling ← where the full-CI rule lives
### Coordinator Closeout Lane      ← line ~367 (sibling)

Both sections are siblings of Plan To Goal Handoff, not nested under it. An agent following this pointer lands on the wrong section. Suggest:

Suggested change
At the final review/readiness gate, apply the canonical full-CI uncertainty rule and Coordinator Closeout Lane from `.agents/workflows/pr-processing.md` under **Plan To Goal Handoff**.
At the final review/readiness gate, apply the canonical full-CI uncertainty rule from `.agents/workflows/pr-processing.md` **Question And Decision Handling** and the **Coordinator Closeout Lane** in the same file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phrase "under Plan To Goal Handoff" is slightly imprecise. The canonical standalone definitions are in sibling sections of that section:

  • Full-CI uncertainty rule is defined in ### Question And Decision Handling (the `` block)
  • Coordinator Closeout Lane is defined in ### Coordinator Closeout Lane

Both are also embedded inline in the goal-prompt template text within Plan To Goal Handoff, but those are normative prose copies, not the authoritative definitions. Consider naming the target sections directly:

Suggested change
At the final review/readiness gate, apply the canonical full-CI uncertainty rule and Coordinator Closeout Lane from `.agents/workflows/pr-processing.md` under **Plan To Goal Handoff**.
At the final review/readiness gate, apply the canonical full-CI uncertainty rule from `### Question And Decision Handling` and the `### Coordinator Closeout Lane` in `.agents/workflows/pr-processing.md`.


Before final handoff, kill or confirm no stray GitHub polling processes are still running. Final state for every target must be one of: merged PR; open PR waiting on checks/review; blocked needing user input; or no-PR with an evidence-backed issue/PR comment. Final handoff must list branches, PR URLs, issue outcomes, validations, last-known CI state, blockers, no-PR comments, and next actions.
For blocking questions, stop work on that target, surface a structured question to the coordinator or maintainer, and mark the issue/PR with the agreed pending-question state. Report the question/comment URL as `blocked needing user input`; do not open a speculative PR. For non-blocking questions where you make a decision and continue, record the decision in the PR description before review or merge.

Before final handoff, follow the canonical final-state and `Immediate maintainer attention` / `FYI / decisions made` split in `.agents/workflows/pr-processing.md`.
```

## Question And Decision Handling

Classify every unresolved question before continuing:

- **Blocking question**: the implementation, validation, or merge decision would be unsafe without maintainer input. Stop work on that target until answered. Subagents should return the blocking question to the coordinator instead of guessing. For multi-machine batches, post a structured issue or PR comment and, if the repo uses labels for this workflow, apply `codex-pending-question`.
- **Blocking question**: the implementation, validation, or merge decision would be unsafe without maintainer input. Stop work on that target until answered. Subagents should return the blocking question to the coordinator instead of guessing. For multi-machine batches, post a structured issue or PR comment and, if the repo uses labels for this workflow, apply `codex-pending-question`. A worker handoff should include the question/comment URL as that target's blocked final state.
- **Non-blocking decision**: a reasonable local decision can be made without increasing merge risk. Continue work, but add a clearly formatted decision note to the PR description so later review across merged PRs can surface these items quickly.

<!-- Keep this full-CI uncertainty rule in sync with `.agents/workflows/pr-processing.md`. -->

Full-CI uncertainty at the final readiness gate after local validation and the
final push is a non-blocking decision. Request full CI with `+ci-run-full`,
record the reason, re-fetch and wait for the newly requested current-head checks,
and continue the readiness flow instead of escalating it as an immediate
maintainer question.
Comment thread
justin808 marked this conversation as resolved.

Suggested PR description section:

```markdown
Expand All @@ -137,6 +147,14 @@ Suggested PR description section:

Before merge or final readiness, scan the PR description for the decision log and make sure each non-blocking decision is still accurate after review changes.

## Batch Handoff Format
Comment thread
justin808 marked this conversation as resolved.

Use the canonical Batch Handoff Format in
`.agents/workflows/pr-processing.md`. In short, split final batch handoffs into
**Immediate maintainer attention** for true blockers and questions only, and
**FYI / decisions made** for decisions, validations, review state, full-CI
requests already handled, and no-PR rationales.

## Coordination State

Use exact lane assignments as the primary coordination mechanism. Labels are helpful but not sufficient.
Expand Down Expand Up @@ -173,3 +191,11 @@ When worker subagents are explicitly authorized:
- Tell workers they are not alone in the codebase and must not revert others' edits.
- Keep write scopes disjoint unless the main agent serializes integration.
- The main agent owns final PR creation, status reporting, full-CI decisions, and merge sequencing.

## Coordinator Closeout Lane
Comment thread
justin808 marked this conversation as resolved.

For the complete numbered sequence, follow the canonical closeout lane in
`.agents/workflows/pr-processing.md` instead of stopping at PR creation. The
coordinator owns the live re-fetch, current-head checks and review-thread triage,
release-mode or accelerated-RC confidence refresh, full-CI request and waitback
when uncertainty remains, and any authorized ready/merge action.
Loading
Loading