From 6d1daf2ba5c8778f7cf0f6230c615e20b89eec2c Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Sat, 7 Feb 2026 15:22:08 -0500 Subject: [PATCH] adding PR review workflow --- .agents/skills/PR_WORKFLOW.md | 97 ++++++++++++++++++++++++++++++ .agents/skills/merge-pr/SKILL.md | 2 +- .agents/skills/prepare-pr/SKILL.md | 2 +- .agents/skills/review-pr/SKILL.md | 2 +- 4 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 .agents/skills/PR_WORKFLOW.md diff --git a/.agents/skills/PR_WORKFLOW.md b/.agents/skills/PR_WORKFLOW.md new file mode 100644 index 0000000000..50b3c35616 --- /dev/null +++ b/.agents/skills/PR_WORKFLOW.md @@ -0,0 +1,97 @@ +# PR Review Instructions + +# Please read this in full and do not skip sections. + +## Working rule + +Skills execute workflow, maintainers provide judgment. +Always pause between skills to evaluate technical direction, not just command success. + +These three skills must be used in order: + +1. `review-pr` +2. `prepare-pr` +3. `merge-pr` + +They are necessary, but not sufficient. Maintainers must steer between steps and understand the code before moving forward. + +Treat PRs as reports first, code second. +If submitted code is low quality, ignore it and implement the best solution for the problem. + +Do not continue if you cannot verify the problem is real or test the fix. + +## PR quality bar + +- Do not trust PR code by default. +- Do not merge changes you cannot validate with a reproducible problem and a tested fix. +- Keep types strict. Do not use `any` in implementation code. +- Keep implementations properly scoped. Fix root causes, not local symptoms. +- Identify and reuse canonical sources of truth so behavior does not drift across the codebase. +- Harden changes. Always evaluate security impact and abuse paths. +- Understand the system before changing it. Never make the codebase messier just to clear a PR queue. + +## Unified workflow + +### 1) `review-pr` + +Purpose: + +- Review only: correctness, value, security risk, tests, docs, and changelog impact. +- Produce structured findings and a recommendation. + +Expected output: + +- Recommendation: ready, needs work, needs discussion, or close. +- `.local/review.md` with actionable findings. + +Maintainer checkpoint before `prepare-pr`: + +``` +What problem are they trying to solve? +What is the most optimal implementation? +Is the code properly scoped? +Can we fix up everything? +Do we have any questions? +``` + +### 2) `prepare-pr` + +Purpose: + +- Make the PR merge-ready on its head branch. +- Rebase onto current `main`, fix blocker/important findings, and run gates. + +Expected output: + +- Updated code and tests on the PR head branch. +- `.local/prep.md` with changes, verification, and current HEAD SHA. +- Final status: `PR is ready for /mergepr`. + +Maintainer checkpoint before `merge-pr`: + +``` +Is this the most optimal implementation? +Is the code properly scoped? +Is the code properly typed? +Is the code hardened? +Do we have enough tests? +Do not add performative tests, ensure tests are real and there are no regressions. +Take your time, fix it properly, refactor if necessary. +Do you see any follow-up refactors we should do? +``` + +### 3) `merge-pr` + +Purpose: + +- Merge only after review and prep artifacts are present and checks are green. +- Use squash merge flow and verify the PR ends in `MERGED` state. + +Expected output: + +- Successful merge commit and recorded merge SHA. +- Worktree cleanup after successful merge. + +Maintainer checkpoint after merge: + +- Did this reveal broader architecture or test gaps we should address? diff --git a/.agents/skills/merge-pr/SKILL.md b/.agents/skills/merge-pr/SKILL.md index 83e3eb473b..73b851edc3 100644 --- a/.agents/skills/merge-pr/SKILL.md +++ b/.agents/skills/merge-pr/SKILL.md @@ -28,7 +28,7 @@ Merge a prepared PR via `gh pr merge --squash` and clean up the worktree after s ## Known Footguns -- If you see "fatal: not a git repository", you are in the wrong directory. Use `~/Development/openclaw`, not `~/openclaw`. +- If you see "fatal: not a git repository", you are in the wrong directory. Use `~/dev/openclaw` if available; otherwise ask user. - Read `.local/review.md` and `.local/prep.md` in the worktree. Do not skip. - Clean up the real worktree directory `.worktrees/pr-` only after a successful merge. - Expect cleanup to remove `.local/` artifacts. diff --git a/.agents/skills/prepare-pr/SKILL.md b/.agents/skills/prepare-pr/SKILL.md index 82c07759b9..8a7450cc3d 100644 --- a/.agents/skills/prepare-pr/SKILL.md +++ b/.agents/skills/prepare-pr/SKILL.md @@ -30,7 +30,7 @@ Prepare a PR branch for merge with review fixes, green gates, and an updated hea ## Known Footguns -- If you see "fatal: not a git repository", you are in the wrong directory. Use `~/openclaw`. +- If you see "fatal: not a git repository", you are in the wrong directory. Use `~/dev/openclaw` if available; otherwise ask user. - Do not run `git clean -fdx`. - Do not run `git add -A` or `git add .`. diff --git a/.agents/skills/review-pr/SKILL.md b/.agents/skills/review-pr/SKILL.md index 00cef64ac9..2095f7f891 100644 --- a/.agents/skills/review-pr/SKILL.md +++ b/.agents/skills/review-pr/SKILL.md @@ -28,7 +28,7 @@ Perform a thorough review-only PR assessment and return a structured recommendat ## Known Failure Modes -- If you see "fatal: not a git repository", you are in the wrong directory. Use `~/openclaw`. +- If you see "fatal: not a git repository", you are in the wrong directory. Use `~/dev/openclaw` if available; otherwise ask user. - Do not stop after printing the checklist. That is not completion. ## Writing Style for Output