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
10 changes: 10 additions & 0 deletions .agents/agents/review-guardian.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ and generated-output drift before human review time is spent.
- Treat packaged skills, project agents, workflow wrappers, local actions,
changelog entries, wiki output, and generated reports as first-class review
surfaces when touched.
- Require explicit validation evidence for workflow, local-action, and
packaged-wrapper changes. Prefer deterministic local harnesses, fake
``gh``/``git`` scripts, temporary repositories, ``actionlint`` for workflows,
shell/YAML checks, or a temporary validation PR when GitHub-only behavior
cannot be exercised locally.
- When a workflow pushes commits with ``GITHUB_TOKEN``, verify that required
checks are dispatched or mirrored for the bot-authored commit before treating
the workflow as safe.
- Treat ``.github/wiki`` pointer changes as workflow-managed state when they
line up with wiki preview or wiki maintenance automation, rather than as
automatic evidence of accidental scope creep.
Expand All @@ -37,6 +45,8 @@ and generated-output drift before human review time is spent.
- A maintainer wants a fresh rigorous review pass on an existing pull request.
- A change touches workflows, generated outputs, synchronized assets, or other
surfaces where a generic summary would miss important risk.
- A pull request changes workflow permissions, triggers, local action scripts,
packaged workflow wrappers, or bot-authored commit behavior.

## Boundaries

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

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

## Anti-patterns
Expand All @@ -58,5 +70,8 @@ possible.
behavioral or workflow risk exists.
- Do not ignore generated files, synced assets, or workflow wrappers when the
pull request touched their sources.
- Do not mark workflow or action changes clean without an explicit validation
strategy and evidence, or a clear residual-risk note when behavior cannot be
exercised before merge.
- Do not claim a pull request is clean without mentioning the verification
scope or any residual gaps.
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@ Required review themes:
- Generated-output drift.
- Workflow, CI, or release automation risk.
- Consumer-sync or packaged-asset side effects.
- Workflow/action validation evidence for changes under ``.github/workflows``,
``.github/actions``, or ``resources/github-actions``.

If no issues are found:

- State that clearly.
- Mention the scope that was reviewed.
- Call out any remaining test or verification gaps.
- Call out any remaining test or verification gaps, including workflow
behavior that could not be exercised locally or through a temporary
validation run.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ Typical review questions

- Did the implementation change behavior without matching tests?
- Did the workflow wrapper stay aligned with the reusable workflow?
- Did workflow or action changes include executable validation evidence, not
only YAML review?
- If a workflow pushes bot-authored commits, does it dispatch or mirror
required checks for the new commit?
- Are permissions, local action checkouts, and event-specific inputs valid for
reusable workflows and packaged consumer wrappers?
- Did packaged assets change without explaining the consumer impact?
- Did docs, README, changelog, or generated outputs remain consistent with the
implementation?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
Workflow And Action Validation
==============================

Workflow, local-action, and packaged-wrapper changes MUST receive an explicit
validation strategy during review. These surfaces often fail only after merge,
after a workflow-dispatched run, or after a bot-authored commit exercises a
path that normal pull-request CI does not trigger.

When To Apply
-------------

Use this checklist whenever a pull request touches any of these surfaces:

- ``.github/workflows/**``
- ``.github/actions/**``
- ``resources/github-actions/**``
- workflow-related docs that describe permissions, dispatching, branch
protection, sync behavior, generated previews, or release automation

Required Questions
------------------

Ask these questions before deciding the workflow surface is safe:

- Does the workflow or action push commits with ``GITHUB_TOKEN``?
- If it pushes bot-authored commits, do required checks run, get dispatched, or
get mirrored for the new commit?
- Are the required permissions declared in both the reusable workflow and any
packaged consumer wrapper?
- Are local composite action paths available from the repository and ref used
by the workflow?
- Does the changed automation behave correctly for same-repository pull
requests, forked pull requests, ``push``, ``workflow_dispatch``, and
``workflow_call`` where those events are supported?
- Does the workflow rely on files from ``main`` that might not exist in a
consumer repository or in the installed DevTools package version?
- Does the workflow update generated state such as ``.github/wiki``, Pages
previews, release notes, changelog entries, labels, or project metadata?
- Does the workflow need queueing, cancellation, or required-check mirroring so
branch protection sees the latest commit state?

Validation Strategies
---------------------

Prefer deterministic local validation when it can cover the changed behavior:

- run ``bash -n`` for shell scripts;
- parse changed YAML files or run ``actionlint`` after ensuring it is available;
- use fake ``gh`` and ``git`` wrappers to exercise action scripts without
calling GitHub;
- create temporary Git repositories to validate merge, rebase, conflict,
branch, tag, or submodule behavior;
- run changed PHP helper scripts directly against synthetic fixtures;
- verify packaged wrapper inputs, permissions, and reusable workflow paths stay
aligned with the canonical workflow.

Actionlint Availability
-----------------------

When workflow files are changed, prefer running ``actionlint`` locally. If it is
missing, install it before review when the platform has a safe package manager:

- macOS with Homebrew: ``brew install actionlint``;
- macOS with MacPorts: ``sudo port install actionlint``;
- Debian-based Linux: download the official ``actionlint`` release archive for
the host architecture, unpack the binary into a temporary directory, and run
it from there or install it into a user-local ``PATH`` such as
``~/.local/bin``.

``actionlint`` is distributed as a Go binary. It may use ``shellcheck`` for
``run:`` blocks, and Homebrew installs that dependency automatically. On
Windows, prefer WSL for now unless the repository already documents a native
Windows installation path.

Use the broadest relevant command for the changed surfaces, for example:

.. code-block:: bash

actionlint .github/workflows/*.yml resources/github-actions/*.yml

Record the command and result in the review evidence. If installation is not
possible, record that as a residual validation gap instead of silently skipping
workflow linting.

When local simulation cannot cover the behavior, call out the gap and prefer a
temporary validation branch or pull request. Close that validation PR after
recording the evidence in the real PR or issue. Do not require noisy temporary
PRs for every workflow change when a deterministic local harness covers the
risk.

Bot-Authored Commit Rule
------------------------

Any workflow that pushes commits with ``GITHUB_TOKEN`` MUST be reviewed for
required-check side effects. GitHub usually does not trigger another normal
``pull_request`` workflow run for commits pushed by the built-in workflow
token. If branch protection requires checks on the latest commit, the workflow
SHOULD dispatch the required workflow or publish matching commit statuses for
that bot-authored commit.

Review Output Expectations
--------------------------

The review MUST record one of the following:

- validation evidence, including commands, harnesses, or GitHub runs used;
- a concrete finding when validation reveals a bug or missing permission;
- a residual-risk note when the behavior cannot be exercised before merge.

If the pull request body already contains sufficient validation evidence,
reference it and verify that it covers the changed automation path. If the body
does not, call out the missing validation in the review.
6 changes: 4 additions & 2 deletions .github/actions/review/render-request/run.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ function classifyTouchedSurfaces(files) {
(file) => file === 'README.md' || file.startsWith('docs/'),
],
[
'Workflow or local GitHub Action logic changed; scrutinize permissions, triggers, release flow, and CI side effects.',
'Workflow or local GitHub Action logic changed; require executable validation of permissions, triggers, bot-authored commits, and CI side effects.',
(file) => file.startsWith('.github/workflows/') || file.startsWith('.github/actions/'),
],
[
'Consumer workflow wrappers changed; verify they still mirror the reusable workflow contract safely.',
'Consumer workflow wrappers changed; verify permissions, inputs, and reusable workflow refs remain aligned with the packaged contract.',
(file) => file.startsWith('resources/github-actions/'),
],
[
Expand Down Expand Up @@ -80,6 +80,7 @@ function renderComment({pull, files, focusAreas}) {
`Use $pull-request-review with the review-guardian agent to review PR #${pull.number} (${pull.html_url}).`,
'Lead with findings ordered by severity. Include repository file references whenever possible.',
'Prioritize bugs, regressions, missing tests, missing docs, generated-output drift, and workflow or CI impacts.',
'For workflow/action changes, record validation evidence or residual risk; pay special attention to GITHUB_TOKEN pushes and required-check dispatch/mirroring.',
`Base: ${pull.base.ref}`,
`Head: ${pull.head.ref} @ ${pull.head.sha.slice(0, 7)}`,
'```',
Expand All @@ -99,6 +100,7 @@ function renderSummary({pull, focusAreas}) {
'### Findings-first expectations',
'- Lead with bugs, regressions, missing tests, missing docs, generated-output drift, and workflow or CI risk.',
'- Include repository file references whenever possible.',
'- For workflow/action changes, record executable validation evidence or residual risk, especially around bot-authored commits and required checks.',
];

if (focusAreas.length > 0) {
Expand Down
2 changes: 1 addition & 1 deletion .github/wiki
Submodule wiki updated from 652343 to 716f76
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- Auto-resolve pull-request conflicts limited to workflow-managed `.github/wiki` pointers and `CHANGELOG.md` `Unreleased` drift (#192)
- 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)

### Fixed

Expand Down
Loading