From c79b1c2e80b96686809c5135aea1c711e5e4d115 Mon Sep 17 00:00:00 2001 From: "Bradley A. Thornton" Date: Mon, 20 Jul 2026 08:18:03 -0700 Subject: [PATCH] chore(skills): agent-executable follow-up comments in td-pr-contributor-review Teach maintainers to leave a single P0/P1 brief (problem, why, what, acceptance, non-goals, verification, done definition) so another agent can execute review follow-ups without re-deriving intent from chat. --- .../skills/td-pr-contributor-review/SKILL.md | 155 +++++++++++++++++- 1 file changed, 152 insertions(+), 3 deletions(-) diff --git a/.agents/skills/td-pr-contributor-review/SKILL.md b/.agents/skills/td-pr-contributor-review/SKILL.md index a709f89b..f646c1b2 100644 --- a/.agents/skills/td-pr-contributor-review/SKILL.md +++ b/.agents/skills/td-pr-contributor-review/SKILL.md @@ -3,13 +3,13 @@ name: td-pr-contributor-review description: > Use when reviewing and preparing a contributor's pull request (upstream or fork). Use when the user asks to review a PR, get a contributor PR ready, - update a contributor's branch, or ensure a PR meets project standards before - merge. + update a contributor's branch, ensure a PR meets project standards before + merge, or leave an agent-executable follow-up comment for another session. argument-hint: "" user-invocable: true metadata: author: Ansible DevTools Team - version: 1.1.0 + version: 1.2.0 --- > **[Team DevTools]** Running `td-pr-contributor-review` — from [ansible/team-devtools](https://github.com/ansible/team-devtools/tree/main/.agents/skills/td-pr-contributor-review) @@ -144,6 +144,150 @@ EOF Include the issue URL in the PR comment thread so reviewers can verify tracking. +### 5c. Agent-executable follow-up comments + +When the review outcome is **needs work** and a human or another agent will +land the fixes (not this session), post a single PR comment that a follow-up +agent can execute without re-deriving intent from chat history. + +Use this when the user asks to “comment for follow-up”, “leave instructions +for another agent”, or when you deliberately stop after review without +pushing fixes. Prefer fixing in-session when the user asked you to land the +changes; do not dump work into a comment as a substitute for doing the work +you were asked to do. + +#### When to post + +| Situation | Action | +| --- | --- | +| You will push fixes now | Fix, reply on threads, resolve (see `td-pr-review`). No agent-executable dump needed. | +| Contributor / another agent will fix | Post one agent-executable comment (this section). | +| Work is deferred out of this PR | File a GitHub issue (section 5b) **and** link it from the comment. | + +#### Required structure + +Post via `gh pr comment --repo / --body-file …` (or equivalent). +The body **must** include all of the following sections, in order: + +1. **Title / role** — e.g. `## Maintainer assist — agent-executable follow-up` +2. **Context** — one short paragraph: what the PR is for (finding ID, Jira, + user-visible bug), current CI/merge state, and that this comment is the + execution brief. +3. **How to use** — instruct the executing agent to: work items in order; + treat **P0** as merge-blocking; not invent extra scope; reply on the same + thread with commit SHAs when done; respect any “do not file issues” / + “do not push” constraints from the human. +4. **P0 items (merge-blocking)** — one subsection per item (`### P0-1 — …`). +5. **P1 / optional items** — clearly marked non-blocking; “do only if cheap + while touching related code” unless the human said otherwise. +6. **Out of scope / non-goals** — explicit list so the agent does not expand. +7. **Verification** — exact commands for this repo (e.g. `tox -e lint`, + `tox -e py`, or the project’s npm/vitest equivalents). Include rebase + onto `upstream/main` when DR numbers or base drift are involved. +8. **Done definition** — checklist the executing agent must satisfy before + stopping (pushed commits, CI green, PR body updated, reply on thread). + +#### Per-item template (every P0/P1) + +Each actionable item **must** spell out all four parts. Vague “please fix X” +is not enough. + +```markdown +### P0-N — + +**Problem:** What is wrong today (quote code, cite files/lines, name colliding +PRs or DR IDs). Include a minimal snippet when the failing assertion or stub +is the point. + +**Why it matters:** Security, correctness, merge conflict, project policy, or +reviewer/DoD impact. Tie to Jira/finding/DR when relevant. + +**What to do:** Numbered steps with concrete paths and symbols +(`packages/…/file.ts`, `docs/decisions.md`, `gh pr edit …`). When there is a +tradeoff, name **Option A (preferred)** vs **Option B** and justify the +preference so the agent does not flip a coin. + +**Acceptance:** +- [ ] Observable outcome 1 +- [ ] Observable outcome 2 +- [ ] Tests / docs / CI expectation +``` + +#### Writing rules + +- **Prioritize.** Merge-blocking first (`P0`). Nice-to-haves are `P1` or + “optional”. Never mix blocking and optional in one undifferentiated list. +- **Be executable.** Name files, functions, DR numbers, colliding PR numbers, + and commands. Prefer paste-ready PR body markdown when description format + is the ask. +- **Justify tradeoffs.** If two fixes are valid, state which to prefer and why + (e.g. fail-closed vs register-port-earlier for a security guard). +- **Constrain scope.** Explicit non-goals prevent drive-by refactors. +- **Respect issue policy.** Agent-executable comments do **not** replace + section 5b. If an item is “later PR”, file the issue and link it, unless the + human explicitly said not to file issues — then say so in **How to use** and + **Out of scope**. +- **Do not resolve others’ threads** for work that is still open. The + executing agent should only resolve threads they actually fixed. +- **One comment, not many.** Prefer a single consolidated brief over + fragmented nits the next agent must assemble. + +#### Skeleton (copy and fill) + +```markdown +## Maintainer assist — agent-executable follow-up + +**Context:** . + +**How to use this comment:** Execute items in order. **P0** is merge-blocking. +Do not invent extra scope. . When done, reply on this +thread with commit SHAs and which options you chose. + +--- + +### P0-1 — + +**Problem:** … +**Why it matters:** … +**What to do:** … +**Acceptance:** +- [ ] … + +### P0-2 — <title> +… + +### P1 — <optional title> (non-blocking) +… + +--- + +### Out of scope + +- … + +### Verification + +Run the repo’s quality gates (examples): `tox -e lint`, `tox -e py`, +plus any package-specific tests named in the P0 items. Rebase onto +`upstream/main` first when base drift or DR collisions apply. + +### Done definition for the executing agent + +1. All P0 items landed and pushed. +2. CI green. +3. Reply on this thread: SHAs, DR numbers chosen, P1 items included or skipped. +``` + +#### Anti-patterns + +- Prose-only review with no files, acceptance checks, or ordering. +- “Please address the other reviewer’s comments” without restating them as + executable items. +- Mixing “must fix before merge” with “nice follow-up” without labels. +- Asking the agent to open issues when the human said not to (or the reverse: + leaving deferred work with no issue and no explicit waiver). +- Pasting huge diffs instead of pointing at paths and describing the change. + ### 6. What not to include in the review - **Local-only or environment-specific issues** (e.g. commit signing, SSH @@ -163,6 +307,11 @@ When reviewing or preparing a contributor PR: `git push <remote> <local>:<their-branch> --force-with-lease`. - [ ] If you addressed a review comment: follow the `td-pr-review` skill to reply on the thread with explanation + commit SHA and resolve it. +- [ ] If leaving work for a contributor/another agent: post one agent-executable + comment (section 5c) with P0/P1, problem/why/what/acceptance, non-goals, + verification, and a done definition. +- [ ] Deferred out-of-PR work has a GitHub issue (section 5b) unless the human + explicitly waived filing. ## References