Skip to content

Commit 9b69bda

Browse files
committed
[agents] Add rigorous pull-request review workflow (#147)
1 parent 65aa915 commit 9b69bda

15 files changed

Lines changed: 507 additions & 7 deletions

File tree

.agents/agents/review-guardian.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
name: review-guardian
3+
description: Review Fast Forward pull requests with a rigorous, findings-first contract before or during human review.
4+
primary-skill: pull-request-review
5+
supporting-skills:
6+
- phpunit-tests
7+
- sphinx-docs
8+
- package-readme
9+
---
10+
11+
# review-guardian
12+
13+
## Purpose
14+
15+
Provide a repeatable, high-signal pull-request review pass that helps
16+
maintainers catch regressions, missing coverage, stale docs, workflow risks,
17+
and generated-output drift before human review time is spent.
18+
19+
## Responsibilities
20+
21+
- Review ready pull requests with findings first and summaries second.
22+
- Prioritize bugs, regressions, missing tests, missing documentation, CI or
23+
workflow risk, generated-output drift, and consumer-sync side effects.
24+
- Reference repository files whenever possible so maintainers can move quickly.
25+
- Treat packaged skills, project agents, workflow wrappers, local actions,
26+
changelog entries, wiki output, and generated reports as first-class review
27+
surfaces when touched.
28+
- Stay reusable across this repository and consumer repositories that
29+
synchronize DevTools assets.
30+
31+
## Use When
32+
33+
- A pull request has just transitioned from draft to ready for review.
34+
- A maintainer wants a fresh rigorous review pass on an existing pull request.
35+
- A change touches workflows, generated outputs, synchronized assets, or other
36+
surfaces where a generic summary would miss important risk.
37+
38+
## Boundaries
39+
40+
- Do not replace human review or branch protection.
41+
- Do not drift into implementation or patch authoring unless explicitly asked.
42+
- Do not weaken the findings-first contract with generic praise or summary
43+
text before the actual issues.
44+
45+
## Primary Skill
46+
47+
- `pull-request-review`
48+
49+
## Supporting Skills
50+
51+
- `phpunit-tests`
52+
- `sphinx-docs`
53+
- `package-readme`
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
name: pull-request-review
3+
description: Review Fast Forward pull requests with a findings-first contract that prioritizes bugs, regressions, missing tests, missing docs, generated-output drift, and workflow risk over general summaries.
4+
---
5+
6+
# Fast Forward Pull Request Review
7+
8+
Use this skill when a pull request is ready for review and maintainers need a
9+
repeatable, high-signal review pass. The review MUST lead with concrete
10+
findings, ordered by severity, with repository file references whenever
11+
possible.
12+
13+
## Workflow
14+
15+
1. Resolve the pull request context, touched files, and highest-risk surfaces.
16+
Read [references/surface-priorities.md](references/surface-priorities.md).
17+
2. Inspect behavior before style. Start with source, workflows, generated
18+
outputs, synchronized assets, tests, and documentation that can change
19+
release or automation behavior.
20+
3. Produce the review using the findings-first contract. Read
21+
[references/review-contract.md](references/review-contract.md).
22+
4. Call out missing or weak tests, missing docs, changelog gaps, generated
23+
artifact drift, and consumer-sync impacts whenever the changed surfaces make
24+
them relevant.
25+
5. Only after the findings, add a brief summary or note residual risk. If no
26+
issues are found, say that clearly and mention any remaining verification
27+
gaps.
28+
29+
## Fast Forward Defaults
30+
31+
- Findings come first. Summaries are secondary.
32+
- Prioritize bugs, regressions, missing coverage, missing documentation,
33+
workflow or CI risks, generated-output drift, and sync side effects over
34+
stylistic nits.
35+
- Treat ``.github/workflows``, ``resources/github-actions``, ``.github/actions``,
36+
``.agents/skills``, ``.agents/agents``, ``README.md``, ``docs/``,
37+
``CHANGELOG.md``, and ``.github/wiki`` as high-signal review surfaces.
38+
- Prefer precise repository file references in every finding.
39+
- Review what changed, but reason about downstream consumer impact when the PR
40+
touches packaged assets or synchronized defaults.
41+
- Do not dilute the review with praise or generic narrative before the
42+
findings.
43+
44+
## Reference Guide
45+
46+
| Need | Reference |
47+
|------|-----------|
48+
| Decide which changed surfaces deserve the closest scrutiny | [references/surface-priorities.md](references/surface-priorities.md) |
49+
| Format the review output in the expected findings-first shape | [references/review-contract.md](references/review-contract.md) |
50+
51+
## Anti-patterns
52+
53+
- Do not lead with a summary before the findings.
54+
- Do not spend the review budget on low-value formatting commentary when
55+
behavioral or workflow risk exists.
56+
- Do not ignore generated files, synced assets, or workflow wrappers when the
57+
pull request touched their sources.
58+
- Do not claim a pull request is clean without mentioning the verification
59+
scope or any residual gaps.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
interface:
2+
display_name: "Fast Forward Rigorous Review"
3+
short_description: "Review Fast Forward pull requests with a findings-first contract"
4+
default_prompt: "Use $pull-request-review to perform a rigorous findings-first review of this Fast Forward pull request."
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
Review Contract
2+
===============
3+
4+
Every rigorous review produced from this skill MUST follow this contract:
5+
6+
1. Findings first.
7+
2. Findings ordered by severity.
8+
3. Each finding anchored to a repository file reference whenever possible.
9+
4. Open questions or assumptions after the findings, not before them.
10+
5. A short summary only after the findings, and only when it adds value.
11+
12+
Expected output shape:
13+
14+
- Findings
15+
- ``High`` / ``Medium`` / ``Low`` severity labels when helpful.
16+
- One issue per bullet or short paragraph.
17+
- Explain the concrete risk or regression, not just the changed code.
18+
- Include the most relevant file path for each finding.
19+
- Open Questions / Assumptions
20+
- Use only when uncertainty affects confidence in the findings.
21+
- Summary
22+
- Briefly describe the net result after the findings.
23+
24+
Required review themes:
25+
26+
- Bugs and regressions.
27+
- Missing or weak tests.
28+
- Missing or stale documentation.
29+
- Generated-output drift.
30+
- Workflow, CI, or release automation risk.
31+
- Consumer-sync or packaged-asset side effects.
32+
33+
If no issues are found:
34+
35+
- State that clearly.
36+
- Mention the scope that was reviewed.
37+
- Call out any remaining test or verification gaps.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
Surface Priorities
2+
==================
3+
4+
When a pull request is ready for review, inspect the highest-risk surfaces
5+
first.
6+
7+
Highest priority
8+
----------------
9+
10+
- ``src/`` when public behavior, orchestration, dependency injection, or
11+
compatibility may have changed.
12+
- ``tests/`` when behavior changed but coverage may be missing or weakened.
13+
- ``.github/workflows/`` and ``.github/actions/`` when permissions, triggers,
14+
release behavior, or CI guarantees may have changed.
15+
- ``resources/github-actions/`` when a repository-facing workflow wrapper can
16+
drift from the reusable workflow implementation.
17+
- ``.agents/skills/`` and ``.agents/agents/`` when packaged prompts or
18+
consumer-synchronized capabilities changed.
19+
20+
Additional Fast Forward review surfaces
21+
--------------------------------------
22+
23+
- ``README.md`` and ``docs/`` for onboarding, command, or workflow drift.
24+
- ``CHANGELOG.md`` for notable user-facing or automation-facing changes.
25+
- ``.github/wiki`` when generated wiki output is touched.
26+
- ``resources/`` when synchronized templates or packaged defaults changed.
27+
28+
Typical review questions
29+
------------------------
30+
31+
- Did the implementation change behavior without matching tests?
32+
- Did the workflow wrapper stay aligned with the reusable workflow?
33+
- Did packaged assets change without explaining the consumer impact?
34+
- Did docs, README, changelog, or generated outputs remain consistent with the
35+
implementation?
36+
- Did the pull request alter release automation, sync flows, or CI permissions
37+
in a risky way?
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
name: Render Rigorous Review Request
2+
description: Render a deterministic review brief for a pull request that is ready for rigorous review.
3+
4+
inputs:
5+
pull-request-number:
6+
description: Pull request number to review.
7+
required: false
8+
default: ''
9+
10+
outputs:
11+
pull-request-number:
12+
description: Resolved pull request number.
13+
value: ${{ steps.render.outputs.pull-request-number }}
14+
comment:
15+
description: Sticky pull-request comment body.
16+
value: ${{ steps.render.outputs.comment }}
17+
summary:
18+
description: GitHub Actions step summary body.
19+
value: ${{ steps.render.outputs.summary }}
20+
21+
runs:
22+
using: composite
23+
steps:
24+
- id: render
25+
uses: actions/github-script@v8
26+
with:
27+
github-token: ${{ github.token }}
28+
script: |
29+
const run = require(`${process.env.GITHUB_ACTION_PATH}/run.cjs`);
30+
await run({github, context, core});
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
const MAX_LISTED_FILES = 12;
2+
3+
function classifyTouchedSurfaces(files) {
4+
const checks = [
5+
[
6+
'Source behavior or orchestration changed; verify regressions, contracts, and dependency boundaries.',
7+
(file) => file.startsWith('src/'),
8+
],
9+
[
10+
'Tests changed; confirm assertions still cover the touched behavior and edge cases.',
11+
(file) => file.startsWith('tests/'),
12+
],
13+
[
14+
'Docs or README changed; verify commands, examples, and generated outputs stayed aligned.',
15+
(file) => file === 'README.md' || file.startsWith('docs/'),
16+
],
17+
[
18+
'Workflow or local GitHub Action logic changed; scrutinize permissions, triggers, release flow, and CI side effects.',
19+
(file) => file.startsWith('.github/workflows/') || file.startsWith('.github/actions/'),
20+
],
21+
[
22+
'Consumer workflow wrappers changed; verify they still mirror the reusable workflow contract safely.',
23+
(file) => file.startsWith('resources/github-actions/'),
24+
],
25+
[
26+
'Packaged agent surfaces changed; verify prompts, sync behavior, and inherited guidance.',
27+
(file) => file.startsWith('.agents/skills/') || file.startsWith('.agents/agents/') || file === 'AGENTS.md',
28+
],
29+
[
30+
'Changelog changed; verify notable behavior and automation changes are documented accurately.',
31+
(file) => file === 'CHANGELOG.md',
32+
],
33+
[
34+
'Wiki output changed; confirm generated content updates are intentional and consistent with the source.',
35+
(file) => file.startsWith('.github/wiki'),
36+
],
37+
[
38+
'Packaged resources changed; verify consumer repositories inherit the right defaults and generated artifacts.',
39+
(file) => file.startsWith('resources/'),
40+
],
41+
];
42+
43+
return checks
44+
.filter(([, matcher]) => files.some(matcher))
45+
.map(([message]) => message);
46+
}
47+
48+
function renderComment({pull, files, focusAreas}) {
49+
const listedFiles = files.slice(0, MAX_LISTED_FILES);
50+
const remainingCount = Math.max(0, files.length - listedFiles.length);
51+
const touchedSurfaces = focusAreas.length > 0
52+
? focusAreas
53+
: ['No special review surface was inferred beyond the normal findings-first review contract.'];
54+
55+
const lines = [
56+
'## Rigorous review requested',
57+
'',
58+
'This pull request is ready for the dedicated `review-guardian` pass powered by `$pull-request-review`.',
59+
'',
60+
`- PR: #${pull.number}${pull.title}`,
61+
`- Author: @${pull.user.login}`,
62+
`- Base: \`${pull.base.ref}\``,
63+
`- Head: \`${pull.head.ref}\` @ \`${pull.head.sha.slice(0, 7)}\``,
64+
'',
65+
'### Review focus',
66+
...touchedSurfaces.map((item) => `- ${item}`),
67+
'',
68+
'### Sample changed files',
69+
...listedFiles.map((file) => `- \`${file}\``),
70+
];
71+
72+
if (remainingCount > 0) {
73+
lines.push(`- ...and ${remainingCount} more files.`);
74+
}
75+
76+
lines.push(
77+
'',
78+
'### Suggested prompt',
79+
'```text',
80+
`Use $pull-request-review with the review-guardian agent to review PR #${pull.number} (${pull.html_url}).`,
81+
'Lead with findings ordered by severity. Include repository file references whenever possible.',
82+
'Prioritize bugs, regressions, missing tests, missing docs, generated-output drift, and workflow or CI impacts.',
83+
`Base: ${pull.base.ref}`,
84+
`Head: ${pull.head.ref} @ ${pull.head.sha.slice(0, 7)}`,
85+
'```',
86+
);
87+
88+
return lines.join('\n');
89+
}
90+
91+
function renderSummary({pull, focusAreas}) {
92+
const lines = [
93+
'## Rigorous review requested',
94+
'',
95+
`- Pull request: [#${pull.number}](${pull.html_url})`,
96+
'- Agent: `review-guardian`',
97+
'- Skill: `$pull-request-review`',
98+
'',
99+
'### Findings-first expectations',
100+
'- Lead with bugs, regressions, missing tests, missing docs, generated-output drift, and workflow or CI risk.',
101+
'- Include repository file references whenever possible.',
102+
];
103+
104+
if (focusAreas.length > 0) {
105+
lines.push('', '### Inferred high-signal surfaces', ...focusAreas.map((item) => `- ${item}`));
106+
}
107+
108+
return lines.join('\n');
109+
}
110+
111+
module.exports = async function run({github, context, core}) {
112+
const owner = context.repo.owner;
113+
const repo = context.repo.repo;
114+
const input = core.getInput('pull-request-number').trim();
115+
const inferredNumber = String(context.payload.pull_request?.number || '').trim();
116+
const pullRequestNumber = input || inferredNumber;
117+
118+
if (pullRequestNumber === '') {
119+
core.setFailed('Unable to resolve a pull request number for the rigorous review workflow.');
120+
return;
121+
}
122+
123+
const pull_number = Number.parseInt(pullRequestNumber, 10);
124+
125+
if (Number.isNaN(pull_number)) {
126+
core.setFailed(`Invalid pull request number: ${pullRequestNumber}`);
127+
return;
128+
}
129+
130+
const {data: pull} = await github.rest.pulls.get({
131+
owner,
132+
repo,
133+
pull_number,
134+
});
135+
136+
const files = await github.paginate(github.rest.pulls.listFiles, {
137+
owner,
138+
repo,
139+
pull_number,
140+
per_page: 100,
141+
});
142+
143+
const changedFiles = files.map((file) => file.filename);
144+
const focusAreas = classifyTouchedSurfaces(changedFiles);
145+
146+
core.setOutput('pull-request-number', String(pull.number));
147+
core.setOutput('comment', renderComment({pull, files: changedFiles, focusAreas}));
148+
core.setOutput('summary', renderSummary({pull, focusAreas}));
149+
};

0 commit comments

Comments
 (0)