| name | gh-review |
| description | A local pull-request review that mirrors the CI review criteria so self-review
and CI cannot drift: build, lint, tests, architecture, and the leak scan, run
through the same scripts CI runs. Works on a PR number or on the local diff
dev...HEAD. Sorts the diff into file buckets and ends with exactly one verdict.
|
gh-review
Purpose
Review a change against the same gates CI enforces, before CI runs or before a
human merges. The point is no drift: this skill calls the same scripts the
ci.yml workflow calls, so a local APPROVE means the same thing a green CI run
means. It ends with one machine-readable verdict line.
When to use
Use to self-review a branch before opening or merging a PR, or to review someone
else's PR locally. Do not use it to replace CI; use it so CI has nothing left to
catch.
Modes
-
PR number: check out the PR head, then review its diff against dev.
gh pr checkout <N>
-
Local diff: review the working branch directly.
git diff dev...HEAD
Pick one mode and state it at the top of the review, because the reader needs to
know what range the verdict covers.
Mirror the CI gates
Run the same checks CI runs, from the repo root. Do not paraphrase them; call
the scripts, because a hand-rolled check drifts from the one CI trusts:
- Canonical gate:
make validate (conventions, leak scan, game-IP guard,
game-free build, every test suite, the architecture DAG, contract drift).
- Leak scan on its own if you need the detail:
bash scripts/check-no-leak.sh.
- Game-IP guard on its own:
bash scripts/guard-no-game-ip.sh.
- Architecture DAG:
dotnet test tests/Sailwind.Architecture.Tests and the
Rust crate-DAG test under cargo test --workspace.
A red result from any of these is at least REQUEST-CHANGES, because CI will fail
the same way.
File-bucket taxonomy
Sort every changed path into one bucket and review each bucket for what it is
prone to break:
- contracts:
contracts/fbs/** and generated output. Check the schema and its
regenerated code landed together, because a schema change without regenerated
code is a wire break.
- api:
packages/Sailwind.Api.*, apps/Sailwind.API. Check the surface hash
and that game members are reached only through the generated seam.
- server:
server/** Rust crates. Check wire-input bounds and the crate DAG.
- client:
apps/Sailwind.Online.Client, packages/Sailwind.Online.*. Check it
shares nothing with the server but contracts/.
- infra:
.github/**, scripts/**, Makefile, .agents/**, .claude/**.
Check no gate was weakened and no hook bypassed.
- docs:
docs/**, *.md. Check RFC 2119 usage, the banned vocabulary, and no
em dash.
Blocker-class findings
Any one of these is a blocker: raise the verdict to at least REQUEST-CHANGES and
name the path:line, because each one breaks a gate the whole team relies on:
- A suppressed or downgraded warning (a
#pragma warning disable, an
#[allow(...)], a <NoWarn>), because warnings are errors here.
- A hand-edited generated file, because the next regeneration silently reverts
it and the drift check fails.
- A game, Unity, or BepInEx reference outside
apps/** and the api-adapters
package, because that reference breaks the game-free CI build.
- A leak term: any private organization, client, product, repository, or person
name, because this repository is public.
Verdict
End with exactly one line whose first token is the verdict:
APPROVE - gates green, no blocker-class finding.
REQUEST-CHANGES - <path:line>: <what and why>.
REJECT - <the change is wrong at its root>.
Exactly one verdict line, because a review with two verdicts or none cannot be
read as a gate result.
Anti-patterns
- Approving without running
make validate.
- Paraphrasing a CI check instead of calling its script, which is how drift
starts.
- Listing a style nit while a blocker-class finding goes unnamed.
- Ending with no verdict line, or with more than one.
References
.github/workflows/ci.yml for the gates this mirrors..agents/rules/quality-gates.md for warnings-as-errors and no-bypass..agents/rules/dependency-hierarchy.md for the layer edges to check..agents/rules/testing.md for the game-free suite split.references/workflow-contract.md for the inputs, outputs, and invariants.
