diff --git a/INSTALL.md b/INSTALL.md index c451585..319ce6b 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -51,6 +51,18 @@ git config --global product-dev-skills.github-author # drop "--admin" if you don't have admin access or want manual approval. git config --global product-dev-skills.pr-merge-flags "--squash --admin" +# Enable automatic merging in /review-pr. Default is false: the pipeline +# reviews, fixes, pushes, and waits for CI, but stops short of running +# `gh pr merge` so you can merge manually. Set to true to opt in to +# auto-merge after CI passes. +git config --global product-dev-skills.auto-merge true + +# Auto-push fixes in /review-pr without prompting. Default is false: +# the pipeline produces fixes locally and runs all local checks, then +# shows you the diff and asks whether to approve (push) or discard +# (revert). Set to true to skip the prompt and push fixes immediately. +git config --global product-dev-skills.auto-fix true + # Paths to scan during /cleanup. Repeat with --add for multiple paths. git config --global --add product-dev-skills.cleanup-paths "$HOME/dev/github/paritytech/host-sdk/target" git config --global --add product-dev-skills.cleanup-paths "$HOME/dev/github/paritytech/host-sdk/.claude/worktrees" diff --git a/skills/review-pr/SKILL.md b/skills/review-pr/SKILL.md index f105bba..a9ac32b 100644 --- a/skills/review-pr/SKILL.md +++ b/skills/review-pr/SKILL.md @@ -12,6 +12,8 @@ disable-model-invocation: false Read these from git config (`git config --get product-dev-skills.`): - `$AUTHOR` from `github-author` (required) — author whose PRs get the full review-fix-merge pipeline - `$MERGE_FLAGS` from `pr-merge-flags` (required) — flags passed to `gh pr merge` (e.g. `--squash --admin` or `--squash`) +- `$AUTO_MERGE` from `auto-merge` (optional, default `false`) — when `true`, the pipeline runs `gh pr merge` after CI is green. When `false` (the default), the pipeline stops after pushing fixes and verifying CI; it never runs `gh pr merge`. Read with `git config --get --type=bool product-dev-skills.auto-merge` and treat any value other than `true` (including unset) as `false`. +- `$AUTO_FIX` from `auto-fix` (optional, default `false`) — when `true`, the pipeline pushes fixes without prompting. When `false` (the default), the pipeline still produces fixes locally and runs all checks, but stops before pushing to show the user the fix diff and ask whether to approve (push) or discard (revert). Read with `git config --get --type=bool product-dev-skills.auto-fix` and treat any value other than `true` (including unset) as `false`. If any required value is unset, stop and tell the user which `git config` command(s) to run. @@ -134,20 +136,26 @@ Run ALL relevant checks based on what files changed: Only after Steps 4, 5, and 6 are clean: -1. `git add` the changed files -2. `git commit` with a descriptive message -3. `git push origin : --force-with-lease` +1. **If `$AUTO_FIX` is `false`**: show the user the fix diff (`git diff`, or a per-file summary if the diff is too large) and ask whether to **approve** (continue and push) or **discard** (revert all local edits and exit). On approve, continue to step 2. On discard, run `git reset --hard HEAD` to drop the local edits, `ExitWorktree` with action `remove`, and end the pipeline. In watcher-driven or non-interactive sessions where you cannot prompt the user, do not push: report the prepared fixes and exit without committing. +2. `git add` the changed files +3. `git commit` with a descriptive message +4. `git push origin : --force-with-lease` ## Step 8: Wait for CI, then Merge 1. Prefer `ek-pr checks $ARGUMENTS` for CI polling/status checks. Fall back to `gh pr checks $ARGUMENTS` only if the helper is unavailable. 2. If a check fails: enter a new worktree, diagnose, fix, push again, and restart from Step 8. -3. **Only after all CI checks pass**: `gh pr merge $ARGUMENTS $MERGE_FLAGS` -4. Confirm merge: `gh pr view $ARGUMENTS --json state -q .state` -5. `ExitWorktree` with action `remove` +3. **If `$AUTO_MERGE` is `false`**: stop here once all CI checks are green. Do NOT run `gh pr merge`. Report to the user that the PR is ready for manual merge, then `ExitWorktree` with action `remove` and skip steps 4–5. +4. **Only after all CI checks pass** (and `$AUTO_MERGE` is not `false`): `gh pr merge $ARGUMENTS $MERGE_FLAGS` +5. Confirm merge: `gh pr view $ARGUMENTS --json state -q .state` +6. `ExitWorktree` with action `remove` **CRITICAL: Never merge before CI checks pass.** If `$MERGE_FLAGS` includes `--admin`, that bypasses branch protection rules (e.g., required reviews) but does NOT replace CI verification. Always wait for all checks to be green first. +**CRITICAL: Respect `$AUTO_MERGE=false`.** When auto-merge is disabled, never invoke `gh pr merge`, never enable GitHub auto-merge, and never bypass this gate. The user merges manually. + +**CRITICAL: Respect `$AUTO_FIX=false`.** When auto-fix is disabled, never push, commit, or merge without explicit user approval of the fix diff. The pipeline must present the diff and let the user choose to approve (push) or discard (revert). Do not assume approval; do not skip the prompt. + --- # Comment-Only Review (external PRs)