mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-07 22:44:16 +00:00
fix: restore deterministic review workflow
This commit is contained in:
Gustavo Madeira Santana
@@ -1,18 +1,22 @@
|
||||
# PR Review Instructions
|
||||
# PR Workflow for Maintainers
|
||||
|
||||
Please read this in full and do not skip sections.
|
||||
This is the single source of truth for the maintainer PR workflow.
|
||||
|
||||
## Triage order
|
||||
|
||||
Process PRs **oldest to newest**. Older PRs are more likely to have merge conflicts and stale dependencies; resolving them first keeps the queue healthy and avoids snowballing rebase pain.
|
||||
|
||||
## Working rule
|
||||
|
||||
Skills execute workflow, maintainers provide judgment.
|
||||
Skills execute workflow. Maintainers provide judgment.
|
||||
Always pause between skills to evaluate technical direction, not just command success.
|
||||
Default mode is local-first, do not write to GitHub until maintainer explicitly says go.
|
||||
|
||||
These three skills must be used in order:
|
||||
|
||||
1. `review-pr`
|
||||
2. `prepare-pr`
|
||||
3. `merge-pr`
|
||||
1. `review-pr` — review only, produce findings
|
||||
2. `prepare-pr` — rebase, fix, gate, push to PR head branch
|
||||
3. `merge-pr` — squash-merge, verify MERGED state, clean up
|
||||
|
||||
They are necessary, but not sufficient. Maintainers must steer between steps and understand the code before moving forward.
|
||||
|
||||
@@ -21,26 +25,64 @@ If submitted code is low quality, ignore it and implement the best solution for
|
||||
|
||||
Do not continue if you cannot verify the problem is real or test the fix.
|
||||
|
||||
## Remote write policy
|
||||
## Script-first contract
|
||||
|
||||
Until the maintainer explicitly approves remote actions, stay local-only.
|
||||
Skill runs should invoke these wrappers automatically. You only need to run them manually when debugging or doing an explicit script-only run:
|
||||
|
||||
Remote actions include:
|
||||
- `scripts/pr-review <PR>`
|
||||
- `scripts/pr review-checkout-main <PR>` or `scripts/pr review-checkout-pr <PR>` while reviewing
|
||||
- `scripts/pr review-guard <PR>` before writing review outputs
|
||||
- `scripts/pr review-validate-artifacts <PR>` after writing outputs
|
||||
- `scripts/pr-prepare init <PR>`
|
||||
- `scripts/pr-prepare validate-commit <PR>`
|
||||
- `scripts/pr-prepare gates <PR>`
|
||||
- `scripts/pr-prepare push <PR>`
|
||||
- Optional one-shot prepare: `scripts/pr-prepare run <PR>`
|
||||
- `scripts/pr-merge <PR>` (verify-only; short form remains backward compatible)
|
||||
- `scripts/pr-merge verify <PR>` (verify-only)
|
||||
- Optional one-shot merge: `scripts/pr-merge run <PR>`
|
||||
|
||||
- Pushing branches.
|
||||
- Posting PR comments.
|
||||
- Editing PR metadata (labels, assignees, state).
|
||||
- Merging PRs.
|
||||
- Editing advisory state or publishing advisories.
|
||||
These wrappers run shared preflight checks and generate deterministic artifacts. They are designed to work from repo root or PR worktree cwd.
|
||||
|
||||
Allowed before approval:
|
||||
## Required artifacts
|
||||
|
||||
- Local code changes.
|
||||
- Local tests and validation.
|
||||
- Drafting copy for PR/advisory comments.
|
||||
- Read-only `gh` commands.
|
||||
- `.local/pr-meta.json` and `.local/pr-meta.env` from review init.
|
||||
- `.local/review.md` and `.local/review.json` from review output.
|
||||
- `.local/prep-context.env` and `.local/prep.md` from prepare.
|
||||
- `.local/prep.env` from prepare completion.
|
||||
|
||||
When approved, perform only the approved remote action, then pause for next instruction.
|
||||
## Structured review handoff
|
||||
|
||||
`review-pr` must write `.local/review.json`.
|
||||
In normal skill runs this is handled automatically. Use `scripts/pr review-artifacts-init <PR>` and `scripts/pr review-tests <PR> ...` manually only for debugging or explicit script-only runs.
|
||||
|
||||
Minimum schema:
|
||||
|
||||
```json
|
||||
{
|
||||
"recommendation": "READY FOR /prepare-pr",
|
||||
"findings": [
|
||||
{
|
||||
"id": "F1",
|
||||
"severity": "IMPORTANT",
|
||||
"title": "Missing changelog entry",
|
||||
"area": "CHANGELOG.md",
|
||||
"fix": "Add a Fixes entry for PR #<PR>"
|
||||
}
|
||||
],
|
||||
"tests": {
|
||||
"ran": ["pnpm test -- ..."],
|
||||
"gaps": ["..."],
|
||||
"result": "pass"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`prepare-pr` resolves all `BLOCKER` and `IMPORTANT` findings from this file.
|
||||
|
||||
## Coding Agent
|
||||
|
||||
Use ChatGPT 5.3 Codex High. Fall back to 5.2 Codex High or 5.3 Codex Medium if necessary.
|
||||
|
||||
## PR quality bar
|
||||
|
||||
@@ -53,6 +95,60 @@ When approved, perform only the approved remote action, then pause for next inst
|
||||
- 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.
|
||||
|
||||
## Rebase and conflict resolution
|
||||
|
||||
Before any substantive review or prep work, **always rebase the PR branch onto current `main` and resolve merge conflicts first**. A PR that cannot cleanly rebase is not ready for review — fix conflicts before evaluating correctness.
|
||||
|
||||
- During `prepare-pr`: rebase onto `main` as the first step, before fixing findings or running gates.
|
||||
- If conflicts are complex or touch areas you do not understand, stop and escalate.
|
||||
- Prefer **rebase** for linear history; **squash** when commit history is messy or unhelpful.
|
||||
|
||||
## Commit and changelog rules
|
||||
|
||||
- In normal `prepare-pr` runs, commits are created via `scripts/committer "<msg>" <file...>`. Use it manually only when operating outside the skill flow; avoid manual `git add`/`git commit` so staging stays scoped.
|
||||
- Follow concise, action-oriented commit messages (e.g., `CLI: add verbose flag to send`).
|
||||
- During `prepare-pr`, use concise, action-oriented subjects **without** PR numbers or thanks; reserve `(#<PR>) thanks @<pr-author>` for the final merge/squash commit.
|
||||
- Group related changes; avoid bundling unrelated refactors.
|
||||
- Changelog workflow: keep the latest released version at the top (no `Unreleased`); after publishing, bump the version and start a new top section.
|
||||
- When working on a PR: add a changelog entry with the PR number and thank the contributor (mandatory in this workflow).
|
||||
- When working on an issue: reference the issue in the changelog entry.
|
||||
- In this workflow, changelog is always required even for internal/test-only changes.
|
||||
|
||||
## Gate policy
|
||||
|
||||
In fresh worktrees, dependency bootstrap is handled by wrappers before local gates. Manual equivalent:
|
||||
|
||||
```sh
|
||||
pnpm install --frozen-lockfile
|
||||
```
|
||||
|
||||
Gate set:
|
||||
|
||||
- Always: `pnpm build`, `pnpm check`
|
||||
- `pnpm test` required unless high-confidence docs-only criteria pass.
|
||||
|
||||
## Co-contributor and clawtributors
|
||||
|
||||
- If we squash, add the PR author as a co-contributor in the commit body using a `Co-authored-by:` trailer.
|
||||
- When maintainer prepares and merges the PR, add the maintainer as an additional `Co-authored-by:` trailer too.
|
||||
- Avoid `--auto` merges for maintainer landings. Merge only after checks are green so the maintainer account is the actor and attribution is deterministic.
|
||||
- For squash merges, set `--author-email` to a reviewer-owned email with fallback candidates; if merge fails due to author-email validation, retry once with the next candidate.
|
||||
- If you review a PR and later do work on it, land via merge/squash (no direct-main commits) and always add the PR author as a co-contributor.
|
||||
- When merging a PR: leave a PR comment that explains exactly what we did, include the SHA hashes, and record the comment URL in the final report.
|
||||
- Manual post-merge step for new contributors: run `bun scripts/update-clawtributors.ts` to add their avatar to the README "Thanks to all clawtributors" list, then commit the regenerated README.
|
||||
|
||||
## Review mode vs landing mode
|
||||
|
||||
- **Review mode (PR link only):** read `gh pr view`/`gh pr diff`; **do not** switch branches; **do not** change code.
|
||||
- **Landing mode (exception path):** use only when normal `review-pr -> prepare-pr -> merge-pr` flow cannot safely preserve attribution or cannot satisfy branch protection. Create an integration branch from `main`, bring in PR commits (**prefer rebase** for linear history; **merge allowed** when complexity/conflicts make it safer), apply fixes, add changelog (+ thanks + PR #), run full gate **locally before committing** (`pnpm build && pnpm check && pnpm test`), commit, merge back to `main`, then `git switch main` (never stay on a topic branch after landing). Important: the contributor needs to be in the git graph after this!
|
||||
|
||||
## Pre-review safety checks
|
||||
|
||||
- Before starting a review when a GH Issue/PR is pasted: `review-pr`/`scripts/pr-review` should create and use an isolated `.worktrees/pr-<PR>` checkout from `origin/main` automatically. Do not require a clean main checkout, and do not run `git pull` in a dirty main checkout.
|
||||
- PR review calls: prefer a single `gh pr view --json ...` to batch metadata/comments; run `gh pr diff` only when needed.
|
||||
- PRs should summarize scope, note testing performed, and mention any user-facing changes or new flags.
|
||||
- Read `docs/help/submitting-a-pr.md` ([Submitting a PR](https://docs.openclaw.ai/help/submitting-a-pr)) for what we expect from contributors.
|
||||
|
||||
## Unified workflow
|
||||
|
||||
Entry criteria:
|
||||
@@ -78,7 +174,6 @@ 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?
|
||||
```
|
||||
@@ -94,27 +189,30 @@ Stop and escalate instead of continuing if:
|
||||
Purpose:
|
||||
|
||||
- Make the PR merge-ready on its head branch.
|
||||
- Rebase onto current `main`, fix blocker/important findings, and run gates.
|
||||
- Rebase onto current `main` first, then fix blocker/important findings, then run gates.
|
||||
- In fresh worktrees, bootstrap dependencies before local gates (`pnpm install --frozen-lockfile`).
|
||||
|
||||
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`.
|
||||
- Final status: `PR is ready for /merge-pr`.
|
||||
|
||||
Maintainer checkpoint before `merge-pr`:
|
||||
|
||||
```
|
||||
Is this the most optimal implementation?
|
||||
Is the code properly scoped?
|
||||
Is the code properly reusing existing logic in the codebase?
|
||||
Is the code properly typed?
|
||||
Is the code hardened?
|
||||
Do we have enough tests?
|
||||
Are tests using fake timers where relevant? (e.g., debounce/throttle, retry backoff, timeout branches, delayed callbacks, polling loops)
|
||||
Do we need regression tests?
|
||||
Are tests using fake timers where appropriate? (e.g., debounce/throttle, retry backoff, timeout branches, delayed callbacks, polling loops)
|
||||
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?
|
||||
Did any changes introduce any potential security vulnerabilities?
|
||||
Take your time, fix it properly, refactor if necessary.
|
||||
```
|
||||
|
||||
Stop and escalate instead of continuing if:
|
||||
@@ -123,59 +221,29 @@ Stop and escalate instead of continuing if:
|
||||
- Fixing findings requires broad architecture changes outside safe PR scope.
|
||||
- Security hardening requirements remain unresolved.
|
||||
|
||||
### Security advisory companion flow
|
||||
|
||||
Use this for GHSA-linked fixes and private reports.
|
||||
|
||||
1. Implement and test the fix locally first, do not edit advisory content yet.
|
||||
2. Land the code fix PR through normal flow, including attribution and changelog where needed.
|
||||
3. Prepare public-safe advisory text:
|
||||
- No internal workflow chatter.
|
||||
- No unnecessary exploit detail.
|
||||
- Clear impact, affected range, fixed range, remediation, credits.
|
||||
4. In GitHub advisory UI, set package ranges in the structured fields:
|
||||
- `Affected versions`: `< fixed_version`
|
||||
- `Patched versions`: `>= fixed_version`
|
||||
Do not rely on description text alone.
|
||||
5. If collaborator can edit text but cannot change advisory state, hand off to a Publisher to move triage -> accepted draft -> publish.
|
||||
6. Advisory comments are posted manually in UI when required by policy. Do not rely on `gh api` automation for advisory comments.
|
||||
|
||||
Maintainer checkpoint for security advisories:
|
||||
|
||||
- Is the rewrite public-safe and free of internal/process notes?
|
||||
- Are affected and patched ranges correctly set in the advisory form fields?
|
||||
- Are credits present and accurate?
|
||||
- Do we have Publisher action if state controls are unavailable?
|
||||
|
||||
### 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.
|
||||
- Use deterministic squash merge flow (`--match-head-commit` + explicit subject/body with co-author trailer), then verify the PR ends in `MERGED` state.
|
||||
- If no required checks are configured on the PR, treat that as acceptable and continue after branch-up-to-date validation.
|
||||
|
||||
Go or no-go checklist before merge:
|
||||
|
||||
- All BLOCKER and IMPORTANT findings are resolved.
|
||||
- Verification is meaningful and regression risk is acceptably low.
|
||||
- Docs and changelog are updated when required.
|
||||
- Changelog is updated (mandatory) and docs are updated when required.
|
||||
- Required CI checks are green and the branch is not behind `main`.
|
||||
|
||||
Expected output:
|
||||
|
||||
- Successful merge commit and recorded merge SHA.
|
||||
- Worktree cleanup after successful merge.
|
||||
- Comment on PR indicating merge was successful.
|
||||
|
||||
Maintainer checkpoint after merge:
|
||||
|
||||
- Were any refactors intentionally deferred and now need follow-up issue(s)?
|
||||
- Did this reveal broader architecture or test gaps we should address?
|
||||
|
||||
## Chasing main mitigation
|
||||
|
||||
To reduce repeated "branch behind main" loops:
|
||||
|
||||
1. Keep prep and merge windows short.
|
||||
2. Rebase/update once, as late as possible, right before final checks.
|
||||
3. Avoid non-essential commits on the PR branch after checks start.
|
||||
4. Prefer merge queue or auto-merge when available.
|
||||
- Run `bun scripts/update-clawtributors.ts` if the contributor is new.
|
||||
|
||||
@@ -1,182 +1,99 @@
|
||||
---
|
||||
name: merge-pr
|
||||
description: Merge a GitHub PR via squash after /preparepr. Use when asked to merge a ready PR. Do not push to main or modify code. Ensure the PR ends in MERGED state and clean up worktrees after success.
|
||||
description: Script-first deterministic squash merge with strict required-check gating, head-SHA pinning, and reliable attribution/commenting.
|
||||
---
|
||||
|
||||
# Merge PR
|
||||
|
||||
## Overview
|
||||
|
||||
Merge a prepared PR via `gh pr merge --squash` and clean up the worktree after success.
|
||||
Merge a prepared PR only after deterministic validation.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Ask for PR number or URL.
|
||||
- If missing, auto-detect from conversation.
|
||||
- If ambiguous, ask.
|
||||
- If missing, use `.local/prep.env` from the PR worktree.
|
||||
|
||||
## Safety
|
||||
|
||||
- Use `gh pr merge --squash` as the only path to `main`.
|
||||
- Do not run `git push` at all during merge.
|
||||
- Do not run gateway stop commands. Do not kill processes. Do not touch port 18792.
|
||||
- Do not execute merge or PR-comment GitHub write actions until maintainer explicitly approves.
|
||||
- Never use `gh pr merge --auto` in this flow.
|
||||
- Never run `git push` directly.
|
||||
- Require `--match-head-commit` during merge.
|
||||
- Wrapper commands are cwd-agnostic; you can run them from repo root or inside the PR worktree.
|
||||
|
||||
## Execution Rule
|
||||
## Execution Contract
|
||||
|
||||
- Execute the workflow. Do not stop after printing the TODO checklist.
|
||||
- If delegating, require the delegate to run commands and capture outputs.
|
||||
|
||||
## Known Footguns
|
||||
|
||||
- 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-<PR>` only after a successful merge.
|
||||
- Expect cleanup to remove `.local/` artifacts.
|
||||
|
||||
## Completion Criteria
|
||||
|
||||
- Ensure `gh pr merge` succeeds.
|
||||
- Ensure PR state is `MERGED`, never `CLOSED`.
|
||||
- Record the merge SHA.
|
||||
- Run cleanup only after merge success.
|
||||
|
||||
## First: Create a TODO Checklist
|
||||
|
||||
Create a checklist of all merge steps, print it, then continue and execute the commands.
|
||||
|
||||
## Setup: Use a Worktree
|
||||
|
||||
Use an isolated worktree for all merge work.
|
||||
1. Validate merge readiness:
|
||||
|
||||
```sh
|
||||
cd ~/dev/openclaw
|
||||
# Sanity: confirm you are in the repo
|
||||
git rev-parse --show-toplevel
|
||||
|
||||
WORKTREE_DIR=".worktrees/pr-<PR>"
|
||||
scripts/pr-merge verify <PR>
|
||||
```
|
||||
|
||||
Run all commands inside the worktree directory.
|
||||
|
||||
## Load Local Artifacts (Mandatory)
|
||||
|
||||
Expect these files from earlier steps:
|
||||
|
||||
- `.local/review.md` from `/reviewpr`
|
||||
- `.local/prep.md` from `/preparepr`
|
||||
Backward-compatible verify form also works:
|
||||
|
||||
```sh
|
||||
ls -la .local || true
|
||||
|
||||
if [ -f .local/review.md ]; then
|
||||
echo "Found .local/review.md"
|
||||
sed -n '1,120p' .local/review.md
|
||||
else
|
||||
echo "Missing .local/review.md. Stop and run /reviewpr, then /preparepr."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ -f .local/prep.md ]; then
|
||||
echo "Found .local/prep.md"
|
||||
sed -n '1,120p' .local/prep.md
|
||||
else
|
||||
echo "Missing .local/prep.md. Stop and run /preparepr first."
|
||||
exit 1
|
||||
fi
|
||||
scripts/pr-merge <PR>
|
||||
```
|
||||
|
||||
2. Run one-shot deterministic merge:
|
||||
|
||||
```sh
|
||||
scripts/pr-merge run <PR>
|
||||
```
|
||||
|
||||
3. Ensure output reports:
|
||||
|
||||
- `merge_sha=<sha>`
|
||||
- `merge_author_email=<email>`
|
||||
- `comment_url=<url>`
|
||||
|
||||
## Steps
|
||||
|
||||
1. Identify PR meta
|
||||
1. Validate artifacts
|
||||
|
||||
```sh
|
||||
gh pr view <PR> --json number,title,state,isDraft,author,headRefName,baseRefName,headRepository,body --jq '{number,title,state,isDraft,author:.author.login,head:.headRefName,base:.baseRefName,headRepo:.headRepository.nameWithOwner,body}'
|
||||
contrib=$(gh pr view <PR> --json author --jq .author.login)
|
||||
head=$(gh pr view <PR> --json headRefName --jq .headRefName)
|
||||
head_repo_url=$(gh pr view <PR> --json headRepository --jq .headRepository.url)
|
||||
require=(.local/review.md .local/review.json .local/prep.md .local/prep.env)
|
||||
for f in "${require[@]}"; do
|
||||
[ -s "$f" ] || { echo "Missing artifact: $f"; exit 1; }
|
||||
done
|
||||
```
|
||||
|
||||
2. Run sanity checks
|
||||
|
||||
Stop if any are true:
|
||||
|
||||
- PR is a draft.
|
||||
- Required checks are failing.
|
||||
- Branch is behind main.
|
||||
2. Validate checks and branch status
|
||||
|
||||
```sh
|
||||
# Checks
|
||||
gh pr checks <PR>
|
||||
|
||||
# Check behind main
|
||||
git fetch origin main
|
||||
git fetch origin pull/<PR>/head:pr-<PR>
|
||||
git merge-base --is-ancestor origin/main pr-<PR> || echo "PR branch is behind main, run /preparepr"
|
||||
scripts/pr-merge verify <PR>
|
||||
source .local/prep.env
|
||||
```
|
||||
|
||||
If anything is failing or behind, stop and say to run `/preparepr`.
|
||||
`scripts/pr-merge` treats “no required checks configured” as acceptable (`[]`), but fails on any required `fail` or `pending`.
|
||||
|
||||
3. Merge PR and delete branch
|
||||
|
||||
If checks are still running, use `--auto` to queue the merge.
|
||||
3. Merge deterministically (wrapper-managed)
|
||||
|
||||
```sh
|
||||
# Check status first
|
||||
check_status=$(gh pr checks <PR> 2>&1)
|
||||
if echo "$check_status" | grep -q "pending\|queued"; then
|
||||
echo "Checks still running, using --auto to queue merge"
|
||||
gh pr merge <PR> --squash --delete-branch --auto
|
||||
echo "Merge queued. Monitor with: gh pr checks <PR> --watch"
|
||||
else
|
||||
gh pr merge <PR> --squash --delete-branch
|
||||
fi
|
||||
scripts/pr-merge run <PR>
|
||||
```
|
||||
|
||||
Before running merge command, pause and ask for explicit maintainer go-ahead.
|
||||
`scripts/pr-merge run` performs:
|
||||
|
||||
If merge fails, report the error and stop. Do not retry in a loop.
|
||||
If the PR needs changes beyond what `/preparepr` already did, stop and say to run `/preparepr` again.
|
||||
- deterministic squash merge pinned to `PREP_HEAD_SHA`
|
||||
- reviewer merge author email selection with fallback candidates
|
||||
- one retry only when merge fails due to author-email validation
|
||||
- co-author trailers for PR author and reviewer
|
||||
- post-merge verification of both co-author trailers on commit message
|
||||
- PR comment retry (3 attempts), then comment URL extraction
|
||||
- cleanup after confirmed `MERGED`
|
||||
|
||||
4. Get merge SHA
|
||||
4. Manual fallback (only if wrapper is unavailable)
|
||||
|
||||
```sh
|
||||
merge_sha=$(gh pr view <PR> --json mergeCommit --jq '.mergeCommit.oid')
|
||||
echo "merge_sha=$merge_sha"
|
||||
scripts/pr merge-run <PR>
|
||||
```
|
||||
|
||||
5. Optional comment
|
||||
5. Cleanup
|
||||
|
||||
Use a literal multiline string or heredoc for newlines.
|
||||
|
||||
```sh
|
||||
gh pr comment <PR> --body "$(printf 'Merged via squash.\n\n- Merge commit: %s\n\nThanks @%s!\n' \"$merge_sha\" \"$contrib\")"
|
||||
```
|
||||
|
||||
6. Verify PR state is MERGED
|
||||
|
||||
```sh
|
||||
gh pr view <PR> --json state --jq .state
|
||||
```
|
||||
|
||||
7. Clean up worktree only on success
|
||||
|
||||
Run cleanup only if step 6 returned `MERGED`.
|
||||
|
||||
```sh
|
||||
cd ~/dev/openclaw
|
||||
|
||||
git worktree remove ".worktrees/pr-<PR>" --force
|
||||
|
||||
git branch -D temp/pr-<PR> 2>/dev/null || true
|
||||
git branch -D pr-<PR> 2>/dev/null || true
|
||||
```
|
||||
Cleanup is handled by `run` after merge success.
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Worktree only.
|
||||
- Do not close PRs.
|
||||
- End in MERGED state.
|
||||
- Clean up only after merge success.
|
||||
- Never push to main. Use `gh pr merge --squash` only.
|
||||
- Do not run `git push` at all in this command.
|
||||
- End in `MERGED`, never `CLOSED`.
|
||||
- Cleanup only after confirmed merge.
|
||||
|
||||
@@ -1,251 +1,122 @@
|
||||
---
|
||||
name: prepare-pr
|
||||
description: Prepare a GitHub PR for merge by rebasing onto main, fixing review findings, running gates, committing fixes, and pushing to the PR head branch. Use after /reviewpr. Never merge or push to main.
|
||||
description: Script-first PR preparation with structured findings resolution, deterministic push safety, and explicit gate execution.
|
||||
---
|
||||
|
||||
# Prepare PR
|
||||
|
||||
## Overview
|
||||
|
||||
Prepare a PR branch for merge with review fixes, green gates, and an updated head branch.
|
||||
Prepare the PR head branch for merge after `/review-pr`.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Ask for PR number or URL.
|
||||
- If missing, auto-detect from conversation.
|
||||
- If ambiguous, ask.
|
||||
- If missing, use `.local/pr-meta.env` if present in the PR worktree.
|
||||
|
||||
## Safety
|
||||
|
||||
- Never push to `main` or `origin/main`. Push only to the PR head branch.
|
||||
- Never run `git push` without specifying remote and branch explicitly. Do not run bare `git push`.
|
||||
- Do not run gateway stop commands. Do not kill processes. Do not touch port 18792.
|
||||
- Never push to `main`.
|
||||
- Only push to PR head with explicit `--force-with-lease` against known head SHA.
|
||||
- Do not run `git clean -fdx`.
|
||||
- Do not run `git add -A` or `git add .`. Stage only specific files changed.
|
||||
- Do not push to GitHub until the maintainer explicitly approves the push step.
|
||||
- Wrappers are cwd-agnostic; run from repo root or PR worktree.
|
||||
|
||||
## Execution Rule
|
||||
## Execution Contract
|
||||
|
||||
- Execute the workflow. Do not stop after printing the TODO checklist.
|
||||
- If delegating, require the delegate to run commands and capture outputs.
|
||||
|
||||
## Known Footguns
|
||||
|
||||
- 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 .`.
|
||||
|
||||
## Completion Criteria
|
||||
|
||||
- Rebase PR commits onto `origin/main`.
|
||||
- Fix all BLOCKER and IMPORTANT items from `.local/review.md`.
|
||||
- Run gates and pass.
|
||||
- Commit prep changes.
|
||||
- Push the updated HEAD back to the PR head branch.
|
||||
- Write `.local/prep.md` with a prep summary.
|
||||
- Output exactly: `PR is ready for /mergepr`.
|
||||
|
||||
## First: Create a TODO Checklist
|
||||
|
||||
Create a checklist of all prep steps, print it, then continue and execute the commands.
|
||||
|
||||
## Setup: Use a Worktree
|
||||
|
||||
Use an isolated worktree for all prep work.
|
||||
1. Run setup:
|
||||
|
||||
```sh
|
||||
cd ~/openclaw
|
||||
# Sanity: confirm you are in the repo
|
||||
git rev-parse --show-toplevel
|
||||
|
||||
WORKTREE_DIR=".worktrees/pr-<PR>"
|
||||
scripts/pr-prepare init <PR>
|
||||
```
|
||||
|
||||
Run all commands inside the worktree directory.
|
||||
2. Resolve findings from structured review:
|
||||
|
||||
## Load Review Findings (Mandatory)
|
||||
- `.local/review.json` is mandatory.
|
||||
- Resolve all `BLOCKER` and `IMPORTANT` items.
|
||||
|
||||
3. Commit scoped changes with concise subjects (no PR number/thanks; those belong on the final merge/squash commit).
|
||||
|
||||
4. Run gates via wrapper.
|
||||
|
||||
5. Push via wrapper (includes pre-push remote verification, one automatic lease-retry path, and post-push API propagation retry).
|
||||
|
||||
Optional one-shot path:
|
||||
|
||||
```sh
|
||||
if [ -f .local/review.md ]; then
|
||||
echo "Found review findings from /reviewpr"
|
||||
else
|
||||
echo "Missing .local/review.md. Run /reviewpr first and save findings."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Read it
|
||||
sed -n '1,200p' .local/review.md
|
||||
scripts/pr-prepare run <PR>
|
||||
```
|
||||
|
||||
## Steps
|
||||
|
||||
1. Identify PR meta (author, head branch, head repo URL)
|
||||
1. Setup and artifacts
|
||||
|
||||
```sh
|
||||
gh pr view <PR> --json number,title,author,headRefName,baseRefName,headRepository,body --jq '{number,title,author:.author.login,head:.headRefName,base:.baseRefName,headRepo:.headRepository.nameWithOwner,body}'
|
||||
contrib=$(gh pr view <PR> --json author --jq .author.login)
|
||||
head=$(gh pr view <PR> --json headRefName --jq .headRefName)
|
||||
head_repo_url=$(gh pr view <PR> --json headRepository --jq .headRepository.url)
|
||||
scripts/pr-prepare init <PR>
|
||||
|
||||
ls -la .local/review.md .local/review.json .local/pr-meta.env .local/prep-context.env
|
||||
jq . .local/review.json >/dev/null
|
||||
```
|
||||
|
||||
2. Fetch the PR branch tip into a local ref
|
||||
2. Resolve required findings
|
||||
|
||||
List required items:
|
||||
|
||||
```sh
|
||||
git fetch origin pull/<PR>/head:pr-<PR>
|
||||
jq -r '.findings[] | select(.severity=="BLOCKER" or .severity=="IMPORTANT") | "- [\(.severity)] \(.id): \(.title) => \(.fix)"' .local/review.json
|
||||
```
|
||||
|
||||
3. Rebase PR commits onto latest main
|
||||
Fix all required findings. Keep scope tight.
|
||||
|
||||
3. Update changelog/docs (changelog is mandatory in this workflow)
|
||||
|
||||
```sh
|
||||
# Move worktree to the PR tip first
|
||||
git reset --hard pr-<PR>
|
||||
|
||||
# Rebase onto current main
|
||||
git fetch origin main
|
||||
git rebase origin/main
|
||||
jq -r '.changelog' .local/review.json
|
||||
jq -r '.docs' .local/review.json
|
||||
```
|
||||
|
||||
If conflicts happen:
|
||||
4. Commit scoped changes
|
||||
|
||||
- Resolve each conflicted file.
|
||||
- Run `git add <resolved_file>` for each file.
|
||||
- Run `git rebase --continue`.
|
||||
Use concise, action-oriented subject lines without PR numbers/thanks. The final merge/squash commit is the only place we include PR numbers and contributor thanks.
|
||||
|
||||
If the rebase gets confusing or you resolve conflicts 3 or more times, stop and report.
|
||||
|
||||
4. Fix issues from `.local/review.md`
|
||||
|
||||
- Fix all BLOCKER and IMPORTANT items.
|
||||
- NITs are optional.
|
||||
- Keep scope tight.
|
||||
|
||||
Keep a running log in `.local/prep.md`:
|
||||
|
||||
- List which review items you fixed.
|
||||
- List which files you touched.
|
||||
- Note behavior changes.
|
||||
|
||||
5. Update `CHANGELOG.md` if flagged in review
|
||||
|
||||
Check `.local/review.md` section H for guidance.
|
||||
If flagged and user-facing:
|
||||
|
||||
- Check if `CHANGELOG.md` exists.
|
||||
Use explicit file list:
|
||||
|
||||
```sh
|
||||
ls CHANGELOG.md 2>/dev/null
|
||||
scripts/committer "fix: <summary>" <file1> <file2> ...
|
||||
```
|
||||
|
||||
- Follow existing format.
|
||||
- Add a concise entry with PR number and contributor.
|
||||
|
||||
6. Update docs if flagged in review
|
||||
|
||||
Check `.local/review.md` section G for guidance.
|
||||
If flagged, update only docs related to the PR changes.
|
||||
|
||||
7. Commit prep fixes
|
||||
|
||||
Stage only specific files:
|
||||
5. Run gates
|
||||
|
||||
```sh
|
||||
git add <file1> <file2> ...
|
||||
scripts/pr-prepare gates <PR>
|
||||
```
|
||||
|
||||
Preferred commit tool:
|
||||
6. Push safely to PR head
|
||||
|
||||
```sh
|
||||
committer "fix: <summary> (#<PR>) (thanks @$contrib)" <changed files>
|
||||
scripts/pr-prepare push <PR>
|
||||
```
|
||||
|
||||
If `committer` is not found:
|
||||
This push step includes:
|
||||
|
||||
- robust fork remote resolution from owner/name,
|
||||
- pre-push remote SHA verification,
|
||||
- one automatic rebase + gate rerun + retry if lease push fails,
|
||||
- post-push PR-head propagation retry,
|
||||
- idempotent behavior when local prep HEAD is already on the PR head,
|
||||
- post-push SHA verification and `.local/prep.env` generation.
|
||||
|
||||
7. Verify handoff artifacts
|
||||
|
||||
```sh
|
||||
git commit -m "fix: <summary> (#<PR>) (thanks @$contrib)"
|
||||
ls -la .local/prep.md .local/prep.env
|
||||
```
|
||||
|
||||
8. Run full gates before pushing
|
||||
8. Output
|
||||
|
||||
```sh
|
||||
pnpm install
|
||||
pnpm build
|
||||
pnpm ui:build
|
||||
pnpm check
|
||||
pnpm test
|
||||
```
|
||||
|
||||
Require all to pass. If something fails, fix, commit, and rerun. Allow at most 3 fix and rerun cycles. If gates still fail after 3 attempts, stop and report the failures. Do not loop indefinitely.
|
||||
|
||||
9. Push updates back to the PR head branch
|
||||
|
||||
```sh
|
||||
# Ensure remote for PR head exists
|
||||
git remote add prhead "$head_repo_url.git" 2>/dev/null || git remote set-url prhead "$head_repo_url.git"
|
||||
|
||||
# Use force with lease after rebase
|
||||
# Double check: $head must NOT be "main" or "master"
|
||||
echo "Pushing to branch: $head"
|
||||
if [ "$head" = "main" ] || [ "$head" = "master" ]; then
|
||||
echo "ERROR: head branch is main/master. This is wrong. Stopping."
|
||||
exit 1
|
||||
fi
|
||||
git push --force-with-lease prhead HEAD:$head
|
||||
```
|
||||
|
||||
Before running the command above, pause and ask for explicit maintainer go-ahead to perform the push.
|
||||
|
||||
10. Verify PR is not behind main (Mandatory)
|
||||
|
||||
```sh
|
||||
git fetch origin main
|
||||
git fetch origin pull/<PR>/head:pr-<PR>-verify --force
|
||||
git merge-base --is-ancestor origin/main pr-<PR>-verify && echo "PR is up to date with main" || echo "ERROR: PR is still behind main, rebase again"
|
||||
git branch -D pr-<PR>-verify 2>/dev/null || true
|
||||
```
|
||||
|
||||
If still behind main, repeat steps 2 through 9.
|
||||
|
||||
11. Write prep summary artifacts (Mandatory)
|
||||
|
||||
Update `.local/prep.md` with:
|
||||
|
||||
- Current HEAD sha from `git rev-parse HEAD`.
|
||||
- Short bullet list of changes.
|
||||
- Gate results.
|
||||
- Push confirmation.
|
||||
- Rebase verification result.
|
||||
|
||||
Create or overwrite `.local/prep.md` and verify it exists and is non-empty:
|
||||
|
||||
```sh
|
||||
git rev-parse HEAD
|
||||
ls -la .local/prep.md
|
||||
wc -l .local/prep.md
|
||||
```
|
||||
|
||||
12. Output
|
||||
|
||||
Include a diff stat summary:
|
||||
|
||||
```sh
|
||||
git diff --stat origin/main..HEAD
|
||||
git diff --shortstat origin/main..HEAD
|
||||
```
|
||||
|
||||
Report totals: X files changed, Y insertions(+), Z deletions(-).
|
||||
|
||||
If gates passed and push succeeded, print exactly:
|
||||
|
||||
```
|
||||
PR is ready for /mergepr
|
||||
```
|
||||
|
||||
Otherwise, list remaining failures and stop.
|
||||
- Summarize resolved findings and gate results.
|
||||
- Print exactly: `PR is ready for /merge-pr`.
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Worktree only.
|
||||
- Do not delete the worktree on success. `/mergepr` may reuse it.
|
||||
- Do not run `gh pr merge`.
|
||||
- Never push to main. Only push to the PR head branch.
|
||||
- Run and pass all gates before pushing.
|
||||
- Do not run `gh pr merge` in this skill.
|
||||
- Do not delete worktree.
|
||||
|
||||
@@ -1,229 +1,142 @@
|
||||
---
|
||||
name: review-pr
|
||||
description: Review-only GitHub pull request analysis with the gh CLI. Use when asked to review a PR, provide structured feedback, or assess readiness to land. Do not merge, push, or make code changes you intend to keep.
|
||||
description: Script-first review-only GitHub pull request analysis. Use for deterministic PR review with structured findings handoff to /prepare-pr.
|
||||
---
|
||||
|
||||
# Review PR
|
||||
|
||||
## Overview
|
||||
|
||||
Perform a thorough review-only PR assessment and return a structured recommendation on readiness for /preparepr.
|
||||
Perform a read-only review and produce both human and machine-readable outputs.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Ask for PR number or URL.
|
||||
- If missing, always ask. Never auto-detect from conversation.
|
||||
- If ambiguous, ask.
|
||||
- If missing, always ask.
|
||||
|
||||
## Safety
|
||||
|
||||
- Never push to `main` or `origin/main`, not during review, not ever.
|
||||
- Do not run `git push` at all during review. Treat review as read only.
|
||||
- Do not stop or kill the gateway. Do not run gateway stop commands. Do not kill processes on port 18792.
|
||||
- Do not perform any GitHub write action (comments, assignees, labels, state changes) unless maintainer explicitly approves it.
|
||||
- Never push, merge, or modify code intended to keep.
|
||||
- Work only in `.worktrees/pr-<PR>`.
|
||||
- Wrapper commands are cwd-agnostic; you can run them from repo root or inside the PR worktree.
|
||||
|
||||
## Execution Rule
|
||||
## Execution Contract
|
||||
|
||||
- Execute the workflow. Do not stop after printing the TODO checklist.
|
||||
- If delegating, require the delegate to run commands and capture outputs, not a plan.
|
||||
|
||||
## Known Failure Modes
|
||||
|
||||
- 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
|
||||
|
||||
- Write casual and direct.
|
||||
- Avoid em dashes and en dashes. Use commas or separate sentences.
|
||||
|
||||
## Completion Criteria
|
||||
|
||||
- Run the commands in the worktree and inspect the PR directly.
|
||||
- Produce the structured review sections A through J.
|
||||
- Save the full review to `.local/review.md` inside the worktree.
|
||||
|
||||
## First: Create a TODO Checklist
|
||||
|
||||
Create a checklist of all review steps, print it, then continue and execute the commands.
|
||||
|
||||
## Setup: Use a Worktree
|
||||
|
||||
Use an isolated worktree for all review work.
|
||||
1. Run wrapper setup:
|
||||
|
||||
```sh
|
||||
cd ~/dev/openclaw
|
||||
# Sanity: confirm you are in the repo
|
||||
git rev-parse --show-toplevel
|
||||
|
||||
WORKTREE_DIR=".worktrees/pr-<PR>"
|
||||
git fetch origin main
|
||||
|
||||
# Reuse existing worktree if it exists, otherwise create new
|
||||
if [ -d "$WORKTREE_DIR" ]; then
|
||||
cd "$WORKTREE_DIR"
|
||||
git checkout temp/pr-<PR> 2>/dev/null || git checkout -b temp/pr-<PR>
|
||||
git fetch origin main
|
||||
git reset --hard origin/main
|
||||
else
|
||||
git worktree add "$WORKTREE_DIR" -b temp/pr-<PR> origin/main
|
||||
cd "$WORKTREE_DIR"
|
||||
fi
|
||||
|
||||
# Create local scratch space that persists across /reviewpr to /preparepr to /mergepr
|
||||
mkdir -p .local
|
||||
scripts/pr-review <PR>
|
||||
```
|
||||
|
||||
Run all commands inside the worktree directory.
|
||||
Start on `origin/main` so you can check for existing implementations before looking at PR code.
|
||||
2. Use explicit branch mode switches:
|
||||
|
||||
- Main baseline mode: `scripts/pr review-checkout-main <PR>`
|
||||
- PR-head mode: `scripts/pr review-checkout-pr <PR>`
|
||||
|
||||
3. Before writing review outputs, run branch guard:
|
||||
|
||||
```sh
|
||||
scripts/pr review-guard <PR>
|
||||
```
|
||||
|
||||
4. Write both outputs:
|
||||
|
||||
- `.local/review.md` with sections A through J.
|
||||
- `.local/review.json` with structured findings.
|
||||
|
||||
5. Validate artifacts semantically:
|
||||
|
||||
```sh
|
||||
scripts/pr review-validate-artifacts <PR>
|
||||
```
|
||||
|
||||
## Steps
|
||||
|
||||
1. Identify PR meta and context
|
||||
1. Setup and metadata
|
||||
|
||||
```sh
|
||||
gh pr view <PR> --json number,title,state,isDraft,author,baseRefName,headRefName,headRepository,url,body,labels,assignees,reviewRequests,files,additions,deletions --jq '{number,title,url,state,isDraft,author:.author.login,base:.baseRefName,head:.headRefName,headRepo:.headRepository.nameWithOwner,additions,deletions,files:.files|length,body}'
|
||||
scripts/pr-review <PR>
|
||||
ls -la .local/pr-meta.json .local/pr-meta.env .local/review-context.env .local/review-mode.env
|
||||
```
|
||||
|
||||
2. Check if this already exists in main before looking at the PR branch
|
||||
|
||||
- Identify the core feature or fix from the PR title and description.
|
||||
- Search for existing implementations using keywords from the PR title, changed file paths, and function or component names from the diff.
|
||||
2. Existing implementation check on main
|
||||
|
||||
```sh
|
||||
# Use keywords from the PR title and changed files
|
||||
rg -n "<keyword_from_pr_title>" -S src packages apps ui || true
|
||||
rg -n "<function_or_component_name>" -S src packages apps ui || true
|
||||
|
||||
git log --oneline --all --grep="<keyword_from_pr_title>" | head -20
|
||||
scripts/pr review-checkout-main <PR>
|
||||
rg -n "<keyword>" -S src extensions apps || true
|
||||
git log --oneline --all --grep "<keyword>" | head -20
|
||||
```
|
||||
|
||||
If it already exists, call it out as a BLOCKER or at least IMPORTANT.
|
||||
|
||||
3. Optional claim step, only with explicit approval
|
||||
|
||||
If the maintainer asks to claim the PR, assign yourself. Otherwise skip this.
|
||||
3. Claim PR
|
||||
|
||||
```sh
|
||||
gh_user=$(gh api user --jq .login)
|
||||
gh pr edit <PR> --add-assignee "$gh_user"
|
||||
gh pr edit <PR> --add-assignee "$gh_user" || echo "Could not assign reviewer, continuing"
|
||||
```
|
||||
|
||||
4. Read the PR description carefully
|
||||
|
||||
Use the body from step 1. Summarize goal, scope, and missing context.
|
||||
|
||||
5. Read the diff thoroughly
|
||||
|
||||
Minimum:
|
||||
4. Read PR description and diff
|
||||
|
||||
```sh
|
||||
scripts/pr review-checkout-pr <PR>
|
||||
gh pr diff <PR>
|
||||
|
||||
source .local/review-context.env
|
||||
git diff --stat "$MERGE_BASE"..pr-<PR>
|
||||
git diff "$MERGE_BASE"..pr-<PR>
|
||||
```
|
||||
|
||||
If you need full code context locally, fetch the PR head to a local ref and diff it. Do not create a merge commit.
|
||||
5. Optional local tests
|
||||
|
||||
Use the wrapper for target validation and executed-test verification:
|
||||
|
||||
```sh
|
||||
git fetch origin pull/<PR>/head:pr-<PR>
|
||||
# Show changes without modifying the working tree
|
||||
|
||||
git diff --stat origin/main..pr-<PR>
|
||||
git diff origin/main..pr-<PR>
|
||||
scripts/pr review-tests <PR> <test-file> [<test-file> ...]
|
||||
```
|
||||
|
||||
If you want to browse the PR version of files directly, temporarily check out `pr-<PR>` in the worktree. Do not commit or push. Return to `temp/pr-<PR>` and reset to `origin/main` afterward.
|
||||
6. Initialize review artifact templates
|
||||
|
||||
```sh
|
||||
# Use only if needed
|
||||
# git checkout pr-<PR>
|
||||
# ...inspect files...
|
||||
|
||||
git checkout temp/pr-<PR>
|
||||
git reset --hard origin/main
|
||||
scripts/pr review-artifacts-init <PR>
|
||||
```
|
||||
|
||||
6. Validate the change is needed and valuable
|
||||
7. Produce review outputs
|
||||
|
||||
Be honest. Call out low value AI slop.
|
||||
- Fill `.local/review.md` sections A through J.
|
||||
- Fill `.local/review.json`.
|
||||
|
||||
7. Evaluate implementation quality
|
||||
Minimum JSON shape:
|
||||
|
||||
Review correctness, design, performance, and ergonomics.
|
||||
```json
|
||||
{
|
||||
"recommendation": "READY FOR /prepare-pr",
|
||||
"findings": [
|
||||
{
|
||||
"id": "F1",
|
||||
"severity": "IMPORTANT",
|
||||
"title": "...",
|
||||
"area": "path/or/component",
|
||||
"fix": "Actionable fix"
|
||||
}
|
||||
],
|
||||
"tests": {
|
||||
"ran": [],
|
||||
"gaps": [],
|
||||
"result": "pass"
|
||||
},
|
||||
"docs": "up_to_date|missing|not_applicable",
|
||||
"changelog": "required"
|
||||
}
|
||||
```
|
||||
|
||||
8. Perform a security review
|
||||
|
||||
Assume OpenClaw subagents run with full disk access, including git, gh, and shell. Check auth, input validation, secrets, dependencies, tool safety, and privacy.
|
||||
|
||||
9. Review tests and verification
|
||||
|
||||
Identify what exists, what is missing, and what would be a minimal regression test.
|
||||
|
||||
10. Check docs
|
||||
|
||||
Check if the PR touches code with related documentation such as README, docs, inline API docs, or config examples.
|
||||
|
||||
- If docs exist for the changed area and the PR does not update them, flag as IMPORTANT.
|
||||
- If the PR adds a new feature or config option with no docs, flag as IMPORTANT.
|
||||
- If the change is purely internal with no user-facing impact, skip this.
|
||||
|
||||
11. Check changelog
|
||||
|
||||
Check if `CHANGELOG.md` exists and whether the PR warrants an entry.
|
||||
|
||||
- If the project has a changelog and the PR is user-facing, flag missing entry as IMPORTANT.
|
||||
- Leave the change for /preparepr, only flag it here.
|
||||
|
||||
12. Answer the key question
|
||||
|
||||
Decide if /preparepr can fix issues or the contributor must update the PR.
|
||||
|
||||
13. Save findings to the worktree
|
||||
|
||||
Write the full structured review sections A through J to `.local/review.md`.
|
||||
Create or overwrite the file and verify it exists and is non-empty.
|
||||
8. Guard + validate before final output
|
||||
|
||||
```sh
|
||||
ls -la .local/review.md
|
||||
wc -l .local/review.md
|
||||
scripts/pr review-guard <PR>
|
||||
scripts/pr review-validate-artifacts <PR>
|
||||
```
|
||||
|
||||
14. Output the structured review
|
||||
|
||||
Produce a review that matches what you saved to `.local/review.md`.
|
||||
|
||||
A) TL;DR recommendation
|
||||
|
||||
- One of: READY FOR /preparepr | NEEDS WORK | NEEDS DISCUSSION | NOT USEFUL (CLOSE)
|
||||
- 1 to 3 sentences.
|
||||
|
||||
B) What changed
|
||||
|
||||
C) What is good
|
||||
|
||||
D) Security findings
|
||||
|
||||
E) Concerns or questions (actionable)
|
||||
|
||||
- Numbered list.
|
||||
- Mark each item as BLOCKER, IMPORTANT, or NIT.
|
||||
- For each, point to file or area and propose a concrete fix.
|
||||
|
||||
F) Tests
|
||||
|
||||
G) Docs status
|
||||
|
||||
- State if related docs are up to date, missing, or not applicable.
|
||||
|
||||
H) Changelog
|
||||
|
||||
- State if `CHANGELOG.md` needs an entry and which category.
|
||||
|
||||
I) Follow ups (optional)
|
||||
|
||||
J) Suggested PR comment (optional)
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Worktree only.
|
||||
- Do not delete the worktree after review.
|
||||
- Review only, do not merge, do not push.
|
||||
- Keep review read-only.
|
||||
- Do not delete worktree.
|
||||
- Use merge-base scoped diff for local context to avoid stale branch drift.
|
||||
|
||||
Reference in New Issue
Block a user