From 15c2dff33a514cb41abfc85ab45312b847688f6d Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 8 May 2026 15:39:00 +0200 Subject: [PATCH 1/3] feat: add opt-in auto-merge config for /review-pr Introduce `product-dev-skills.auto-merge` git config (default `false`) so the review pipeline stops after CI is green instead of running `gh pr merge`, letting users merge manually unless they explicitly opt in. --- INSTALL.md | 6 ++++++ skills/review-pr/SKILL.md | 10 +++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index c451585..b38adeb 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -51,6 +51,12 @@ 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 + # 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..dba07d4 100644 --- a/skills/review-pr/SKILL.md +++ b/skills/review-pr/SKILL.md @@ -12,6 +12,7 @@ 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`. If any required value is unset, stop and tell the user which `git config` command(s) to run. @@ -142,12 +143,15 @@ Only after Steps 4, 5, and 6 are clean: 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. + --- # Comment-Only Review (external PRs) From 874a32842723d02063197db02f785a46bdb9374b Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 8 May 2026 15:58:34 +0200 Subject: [PATCH 2/3] feat: add opt-out auto-fix config for /review-pr Introduce `product-dev-skills.auto-fix` git config (default `true`). When set to `false`, the pipeline presents review findings and stops: no edits, no push, no merge. Lets users get a review without having the assistant modify the PR. --- INSTALL.md | 6 ++++++ skills/review-pr/SKILL.md | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/INSTALL.md b/INSTALL.md index b38adeb..e8299e9 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -57,6 +57,12 @@ git config --global product-dev-skills.pr-merge-flags "--squash --admin" # auto-merge after CI passes. git config --global product-dev-skills.auto-merge true +# Disable automatic fixes in /review-pr. Default is true: the pipeline +# fixes findings, runs local checks, and pushes commits to the PR +# branch. Set to false to stop after presenting findings, leaving the +# PR untouched so you can apply fixes yourself. +git config --global product-dev-skills.auto-fix false + # 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 dba07d4..13a9491 100644 --- a/skills/review-pr/SKILL.md +++ b/skills/review-pr/SKILL.md @@ -13,6 +13,7 @@ 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 `true`) — when `true` (the default), the pipeline fixes findings, runs local checks, and pushes commits to the PR branch. When `false`, the pipeline stops after presenting findings; it never edits files, never pushes, and never merges. Read with `git config --get --type=bool product-dev-skills.auto-fix` and treat any value other than `false` (including unset) as `true`. If any required value is unset, stop and tell the user which `git config` command(s) to run. @@ -68,6 +69,7 @@ Execute the full PR review-fix-merge pipeline for PR #$ARGUMENTS. Follow every s 10. If a live rate-limit event appears during the session with `status=rejected` or with `status=allowed_warning` and utilization `>=95%`, stop launching any additional review work and switch the rest of the session to triage-only behavior immediately. 11. **Apply the PR Review Checklist** (see below) to the findings. 12. Present the **consolidated findings table** to the user. Include ALL severities. +13. **If `$AUTO_FIX` is `false`**: stop here. Report the findings to the user, do not modify any files, do not push, and do not merge. `ExitWorktree` with action `remove` and skip Steps 3–8. ## Step 3: Fix ALL Issues @@ -152,6 +154,8 @@ Only after Steps 4, 5, and 6 are clean: **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 edit files in the PR worktree, never commit, never push, and never invoke `gh pr merge`. The pipeline is review-only in that mode; the user applies the fixes themselves. + --- # Comment-Only Review (external PRs) From f56745396048a3d323444e3b9cf12d5e8c5c4489 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 8 May 2026 16:01:43 +0200 Subject: [PATCH 3/3] fixup: flip auto-fix default to false; prompt to approve/discard Default `auto-fix=false` so the pipeline still produces fixes and runs all local checks but stops before pushing, prompting the user to approve (push) or discard (revert). Setting `auto-fix=true` opts in to the previous push-without-prompting behavior. --- INSTALL.md | 10 +++++----- skills/review-pr/SKILL.md | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index e8299e9..319ce6b 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -57,11 +57,11 @@ git config --global product-dev-skills.pr-merge-flags "--squash --admin" # auto-merge after CI passes. git config --global product-dev-skills.auto-merge true -# Disable automatic fixes in /review-pr. Default is true: the pipeline -# fixes findings, runs local checks, and pushes commits to the PR -# branch. Set to false to stop after presenting findings, leaving the -# PR untouched so you can apply fixes yourself. -git config --global product-dev-skills.auto-fix false +# 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" diff --git a/skills/review-pr/SKILL.md b/skills/review-pr/SKILL.md index 13a9491..a9ac32b 100644 --- a/skills/review-pr/SKILL.md +++ b/skills/review-pr/SKILL.md @@ -13,7 +13,7 @@ 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 `true`) — when `true` (the default), the pipeline fixes findings, runs local checks, and pushes commits to the PR branch. When `false`, the pipeline stops after presenting findings; it never edits files, never pushes, and never merges. Read with `git config --get --type=bool product-dev-skills.auto-fix` and treat any value other than `false` (including unset) as `true`. +- `$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. @@ -69,7 +69,6 @@ Execute the full PR review-fix-merge pipeline for PR #$ARGUMENTS. Follow every s 10. If a live rate-limit event appears during the session with `status=rejected` or with `status=allowed_warning` and utilization `>=95%`, stop launching any additional review work and switch the rest of the session to triage-only behavior immediately. 11. **Apply the PR Review Checklist** (see below) to the findings. 12. Present the **consolidated findings table** to the user. Include ALL severities. -13. **If `$AUTO_FIX` is `false`**: stop here. Report the findings to the user, do not modify any files, do not push, and do not merge. `ExitWorktree` with action `remove` and skip Steps 3–8. ## Step 3: Fix ALL Issues @@ -137,9 +136,10 @@ 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 @@ -154,7 +154,7 @@ Only after Steps 4, 5, and 6 are clean: **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 edit files in the PR worktree, never commit, never push, and never invoke `gh pr merge`. The pipeline is review-only in that mode; the user applies the fixes themselves. +**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. ---