Skip to content

Commit 60a6b76

Browse files
[agents] Teach review workflow validation (#242)
* Teach review agents workflow validation * Update wiki submodule pointer for PR #242 * docs: document local actionlint validation --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
1 parent e9f91cd commit 60a6b76

8 files changed

Lines changed: 157 additions & 7 deletions

File tree

.agents/agents/review-guardian.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ and generated-output drift before human review time is spent.
2525
- Treat packaged skills, project agents, workflow wrappers, local actions,
2626
changelog entries, wiki output, and generated reports as first-class review
2727
surfaces when touched.
28+
- Require explicit validation evidence for workflow, local-action, and
29+
packaged-wrapper changes. Prefer deterministic local harnesses, fake
30+
``gh``/``git`` scripts, temporary repositories, ``actionlint`` for workflows,
31+
shell/YAML checks, or a temporary validation PR when GitHub-only behavior
32+
cannot be exercised locally.
33+
- When a workflow pushes commits with ``GITHUB_TOKEN``, verify that required
34+
checks are dispatched or mirrored for the bot-authored commit before treating
35+
the workflow as safe.
2836
- Treat ``.github/wiki`` pointer changes as workflow-managed state when they
2937
line up with wiki preview or wiki maintenance automation, rather than as
3038
automatic evidence of accidental scope creep.
@@ -37,6 +45,8 @@ and generated-output drift before human review time is spent.
3745
- A maintainer wants a fresh rigorous review pass on an existing pull request.
3846
- A change touches workflows, generated outputs, synchronized assets, or other
3947
surfaces where a generic summary would miss important risk.
48+
- A pull request changes workflow permissions, triggers, local action scripts,
49+
packaged workflow wrappers, or bot-authored commit behavior.
4050

4151
## Boundaries
4252

.agents/skills/pull-request-review/SKILL.md

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,16 @@ possible.
1717
2. Inspect behavior before style. Start with source, workflows, generated
1818
outputs, synchronized assets, tests, and documentation that can change
1919
release or automation behavior.
20-
3. Produce the review using the findings-first contract. Read
20+
3. When the pull request changes workflows, local actions, or packaged workflow
21+
wrappers, build an executable validation strategy before declaring the
22+
surface clean. Read
23+
[references/workflow-action-validation.md](references/workflow-action-validation.md).
24+
4. Produce the review using the findings-first contract. Read
2125
[references/review-contract.md](references/review-contract.md).
22-
4. Call out missing or weak tests, missing docs, changelog gaps, generated
26+
5. Call out missing or weak tests, missing docs, changelog gaps, generated
2327
artifact drift, and consumer-sync impacts whenever the changed surfaces make
2428
them relevant.
25-
5. Only after the findings, add a brief summary or note residual risk. If no
29+
6. Only after the findings, add a brief summary or note residual risk. If no
2630
issues are found, say that clearly and mention any remaining verification
2731
gaps.
2832

@@ -38,6 +42,13 @@ possible.
3842
- When ``.github/wiki`` moves, verify whether the wiki preview or wiki
3943
maintenance workflow is expected to refresh the submodule pointer before
4044
treating that change as unrelated drift or scope creep.
45+
- When workflow automation pushes commits with ``GITHUB_TOKEN``, verify that
46+
required checks are dispatched or mirrored for the bot-authored commit before
47+
treating the workflow as safe.
48+
- For workflow, local-action, or packaged-wrapper changes, prefer deterministic
49+
validation such as shell/YAML checks, fake ``gh``/``git`` harnesses,
50+
temporary repositories, or a temporary validation PR when local simulation
51+
cannot cover the behavior.
4152
- Prefer precise repository file references in every finding.
4253
- Review what changed, but reason about downstream consumer impact when the PR
4354
touches packaged assets or synchronized defaults.
@@ -49,6 +60,7 @@ possible.
4960
| Need | Reference |
5061
|------|-----------|
5162
| Decide which changed surfaces deserve the closest scrutiny | [references/surface-priorities.md](references/surface-priorities.md) |
63+
| Validate workflow, local-action, and packaged-wrapper changes | [references/workflow-action-validation.md](references/workflow-action-validation.md) |
5264
| Format the review output in the expected findings-first shape | [references/review-contract.md](references/review-contract.md) |
5365

5466
## Anti-patterns
@@ -58,5 +70,8 @@ possible.
5870
behavioral or workflow risk exists.
5971
- Do not ignore generated files, synced assets, or workflow wrappers when the
6072
pull request touched their sources.
73+
- Do not mark workflow or action changes clean without an explicit validation
74+
strategy and evidence, or a clear residual-risk note when behavior cannot be
75+
exercised before merge.
6176
- Do not claim a pull request is clean without mentioning the verification
6277
scope or any residual gaps.

.agents/skills/pull-request-review/references/review-contract.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,13 @@ Required review themes:
2929
- Generated-output drift.
3030
- Workflow, CI, or release automation risk.
3131
- Consumer-sync or packaged-asset side effects.
32+
- Workflow/action validation evidence for changes under ``.github/workflows``,
33+
``.github/actions``, or ``resources/github-actions``.
3234

3335
If no issues are found:
3436

3537
- State that clearly.
3638
- Mention the scope that was reviewed.
37-
- Call out any remaining test or verification gaps.
39+
- Call out any remaining test or verification gaps, including workflow
40+
behavior that could not be exercised locally or through a temporary
41+
validation run.

.agents/skills/pull-request-review/references/surface-priorities.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ Typical review questions
3333

3434
- Did the implementation change behavior without matching tests?
3535
- Did the workflow wrapper stay aligned with the reusable workflow?
36+
- Did workflow or action changes include executable validation evidence, not
37+
only YAML review?
38+
- If a workflow pushes bot-authored commits, does it dispatch or mirror
39+
required checks for the new commit?
40+
- Are permissions, local action checkouts, and event-specific inputs valid for
41+
reusable workflows and packaged consumer wrappers?
3642
- Did packaged assets change without explaining the consumer impact?
3743
- Did docs, README, changelog, or generated outputs remain consistent with the
3844
implementation?
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
Workflow And Action Validation
2+
==============================
3+
4+
Workflow, local-action, and packaged-wrapper changes MUST receive an explicit
5+
validation strategy during review. These surfaces often fail only after merge,
6+
after a workflow-dispatched run, or after a bot-authored commit exercises a
7+
path that normal pull-request CI does not trigger.
8+
9+
When To Apply
10+
-------------
11+
12+
Use this checklist whenever a pull request touches any of these surfaces:
13+
14+
- ``.github/workflows/**``
15+
- ``.github/actions/**``
16+
- ``resources/github-actions/**``
17+
- workflow-related docs that describe permissions, dispatching, branch
18+
protection, sync behavior, generated previews, or release automation
19+
20+
Required Questions
21+
------------------
22+
23+
Ask these questions before deciding the workflow surface is safe:
24+
25+
- Does the workflow or action push commits with ``GITHUB_TOKEN``?
26+
- If it pushes bot-authored commits, do required checks run, get dispatched, or
27+
get mirrored for the new commit?
28+
- Are the required permissions declared in both the reusable workflow and any
29+
packaged consumer wrapper?
30+
- Are local composite action paths available from the repository and ref used
31+
by the workflow?
32+
- Does the changed automation behave correctly for same-repository pull
33+
requests, forked pull requests, ``push``, ``workflow_dispatch``, and
34+
``workflow_call`` where those events are supported?
35+
- Does the workflow rely on files from ``main`` that might not exist in a
36+
consumer repository or in the installed DevTools package version?
37+
- Does the workflow update generated state such as ``.github/wiki``, Pages
38+
previews, release notes, changelog entries, labels, or project metadata?
39+
- Does the workflow need queueing, cancellation, or required-check mirroring so
40+
branch protection sees the latest commit state?
41+
42+
Validation Strategies
43+
---------------------
44+
45+
Prefer deterministic local validation when it can cover the changed behavior:
46+
47+
- run ``bash -n`` for shell scripts;
48+
- parse changed YAML files or run ``actionlint`` after ensuring it is available;
49+
- use fake ``gh`` and ``git`` wrappers to exercise action scripts without
50+
calling GitHub;
51+
- create temporary Git repositories to validate merge, rebase, conflict,
52+
branch, tag, or submodule behavior;
53+
- run changed PHP helper scripts directly against synthetic fixtures;
54+
- verify packaged wrapper inputs, permissions, and reusable workflow paths stay
55+
aligned with the canonical workflow.
56+
57+
Actionlint Availability
58+
-----------------------
59+
60+
When workflow files are changed, prefer running ``actionlint`` locally. If it is
61+
missing, install it before review when the platform has a safe package manager:
62+
63+
- macOS with Homebrew: ``brew install actionlint``;
64+
- macOS with MacPorts: ``sudo port install actionlint``;
65+
- Debian-based Linux: download the official ``actionlint`` release archive for
66+
the host architecture, unpack the binary into a temporary directory, and run
67+
it from there or install it into a user-local ``PATH`` such as
68+
``~/.local/bin``.
69+
70+
``actionlint`` is distributed as a Go binary. It may use ``shellcheck`` for
71+
``run:`` blocks, and Homebrew installs that dependency automatically. On
72+
Windows, prefer WSL for now unless the repository already documents a native
73+
Windows installation path.
74+
75+
Use the broadest relevant command for the changed surfaces, for example:
76+
77+
.. code-block:: bash
78+
79+
actionlint .github/workflows/*.yml resources/github-actions/*.yml
80+
81+
Record the command and result in the review evidence. If installation is not
82+
possible, record that as a residual validation gap instead of silently skipping
83+
workflow linting.
84+
85+
When local simulation cannot cover the behavior, call out the gap and prefer a
86+
temporary validation branch or pull request. Close that validation PR after
87+
recording the evidence in the real PR or issue. Do not require noisy temporary
88+
PRs for every workflow change when a deterministic local harness covers the
89+
risk.
90+
91+
Bot-Authored Commit Rule
92+
------------------------
93+
94+
Any workflow that pushes commits with ``GITHUB_TOKEN`` MUST be reviewed for
95+
required-check side effects. GitHub usually does not trigger another normal
96+
``pull_request`` workflow run for commits pushed by the built-in workflow
97+
token. If branch protection requires checks on the latest commit, the workflow
98+
SHOULD dispatch the required workflow or publish matching commit statuses for
99+
that bot-authored commit.
100+
101+
Review Output Expectations
102+
--------------------------
103+
104+
The review MUST record one of the following:
105+
106+
- validation evidence, including commands, harnesses, or GitHub runs used;
107+
- a concrete finding when validation reveals a bug or missing permission;
108+
- a residual-risk note when the behavior cannot be exercised before merge.
109+
110+
If the pull request body already contains sufficient validation evidence,
111+
reference it and verify that it covers the changed automation path. If the body
112+
does not, call out the missing validation in the review.

.github/actions/review/render-request/run.cjs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ function classifyTouchedSurfaces(files) {
1515
(file) => file === 'README.md' || file.startsWith('docs/'),
1616
],
1717
[
18-
'Workflow or local GitHub Action logic changed; scrutinize permissions, triggers, release flow, and CI side effects.',
18+
'Workflow or local GitHub Action logic changed; require executable validation of permissions, triggers, bot-authored commits, and CI side effects.',
1919
(file) => file.startsWith('.github/workflows/') || file.startsWith('.github/actions/'),
2020
],
2121
[
22-
'Consumer workflow wrappers changed; verify they still mirror the reusable workflow contract safely.',
22+
'Consumer workflow wrappers changed; verify permissions, inputs, and reusable workflow refs remain aligned with the packaged contract.',
2323
(file) => file.startsWith('resources/github-actions/'),
2424
],
2525
[
@@ -80,6 +80,7 @@ function renderComment({pull, files, focusAreas}) {
8080
`Use $pull-request-review with the review-guardian agent to review PR #${pull.number} (${pull.html_url}).`,
8181
'Lead with findings ordered by severity. Include repository file references whenever possible.',
8282
'Prioritize bugs, regressions, missing tests, missing docs, generated-output drift, and workflow or CI impacts.',
83+
'For workflow/action changes, record validation evidence or residual risk; pay special attention to GITHUB_TOKEN pushes and required-check dispatch/mirroring.',
8384
`Base: ${pull.base.ref}`,
8485
`Head: ${pull.head.ref} @ ${pull.head.sha.slice(0, 7)}`,
8586
'```',
@@ -99,6 +100,7 @@ function renderSummary({pull, focusAreas}) {
99100
'### Findings-first expectations',
100101
'- Lead with bugs, regressions, missing tests, missing docs, generated-output drift, and workflow or CI risk.',
101102
'- Include repository file references whenever possible.',
103+
'- For workflow/action changes, record executable validation evidence or residual risk, especially around bot-authored commits and required checks.',
102104
];
103105

104106
if (focusAreas.length > 0) {

.github/wiki

Submodule wiki updated from 6523430 to 716f76b

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- Auto-resolve pull-request conflicts limited to workflow-managed `.github/wiki` pointers and `CHANGELOG.md` `Unreleased` drift (#192)
13+
- Teach the pull-request review skill, review-guardian agent, and review request brief to require explicit validation strategies for workflow, local-action, and packaged-wrapper changes, including local `actionlint` installation guidance (#241)
1314

1415
### Fixed
1516

0 commit comments

Comments
 (0)