diff --git a/.changeset/bright-comments-review.md b/.changeset/bright-comments-review.md new file mode 100644 index 0000000..99490b5 --- /dev/null +++ b/.changeset/bright-comments-review.md @@ -0,0 +1,5 @@ +--- +'@kin0992/skills': patch +--- + +Improve review-pr-comments triage by batching decisions and requiring consent before implementing accepted code changes. diff --git a/.github/plugin/marketplace.json b/.github/plugin/marketplace.json index 09c0796..4cdc7cf 100644 --- a/.github/plugin/marketplace.json +++ b/.github/plugin/marketplace.json @@ -17,7 +17,7 @@ { "name": "engineering-skills", "description": "Skills for engineering workflows: triage unresolved pull-request review comments and draft approval-gated replies.", - "version": "0.1.0", + "version": "0.1.1", "source": "./plugins/engineering-skills" }, { diff --git a/evals/review-pr-comments/trigger_tests.yaml b/evals/review-pr-comments/trigger_tests.yaml index 1813f08..2492c2d 100644 --- a/evals/review-pr-comments/trigger_tests.yaml +++ b/evals/review-pr-comments/trigger_tests.yaml @@ -13,6 +13,14 @@ should_trigger_prompts: reason: 'Requests unresolved thread evaluation and reply drafting' confidence: high + - prompt: 'There are lots of unresolved comments on this PR. Triage each one with me, then publish the approved reactions and replies together.' + reason: 'Requests grouped decision collection before GitHub mutations' + confidence: high + + - prompt: 'After we decide which review comments to accept, ask me before making any requested code changes.' + reason: 'Requests explicit implementation consent after comment decisions' + confidence: high + should_not_trigger_prompts: - prompt: 'Reply to this general PR conversation comment.' reason: 'General PR comments have no unresolved review-thread state' diff --git a/package.json b/package.json index ae33881..367e61b 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "@kin0992/oxc-config": "workspace:*", "oxfmt": "catalog:core", "oxlint": "catalog:core", - "turbo": "^2.10.8", + "turbo": "^2.10.9", "typescript": "catalog:core", "vitest": "^4.1.10" }, @@ -38,5 +38,5 @@ "node": ">=24.0.0", "pnpm": ">=11.0.0" }, - "packageManager": "pnpm@11.20.0+sha512.9a6f330a95b66446ea088faf1521405a8a01f07fde7124cc9958dfed52d4bb436737e65b08f85f37b46fcba375092558ac51262b816844b22f63406ed166bfee" + "packageManager": "pnpm@11.21.0+sha512.521705bce689924eac72f5a3587122f362689ef6571e55ba80076fd637c11132ecffada26fad4ea79c485bfddbfd3d5a2a5b05805a77e893de71ec8a6cca3bb1" } diff --git a/packages/skills/src/engineering/review-pr-comments/SKILL.md b/packages/skills/src/engineering/review-pr-comments/SKILL.md index c6570fe..71f711f 100644 --- a/packages/skills/src/engineering/review-pr-comments/SKILL.md +++ b/packages/skills/src/engineering/review-pr-comments/SKILL.md @@ -19,11 +19,23 @@ license: MIT 2. Process one PR and all actionable comments in its unresolved review threads; include bots, but exclude general PR conversation comments. 3. Evaluate each comment against the diff and source context. -4. Ask one question at a time. Record `Accept` with `+1`, `Defer` with `eyes`, - and `Ignore` with `-1`. Do not edit code merely because it was accepted. -5. Suggest possible answers. Post a reply only after exact-text and target - approval; editing a draft is not approval. -6. Never resolve threads or post general PR comments. +4. If there is more than one actionable comment, enter batch triage before + making any GitHub mutation. Ask one question at a time, and for each comment + show its recommendation plus a concise suggested reply. Record `Accept` with + `+1`, `Defer` with `eyes`, and `Reject` with `-1`; do not edit code merely + because a comment was accepted. +5. After all decisions are collected, identify accepted comments that require + code changes and ask explicitly whether to implement them. An `Accept` + decision never authorizes code edits. If approved, implement and validate + the requested changes before finalizing reply drafts; if declined, leave + code unchanged and continue with the decisions. +6. Keep every decision and reply draft grouped locally until triage and any + authorized implementation are complete. Then show the complete grouped plan + and ask for one confirmation to publish it. Do not publish a partial batch. +7. Publish each selected reaction and only the exact reply text approved for + that target. If a draft is edited, ask for approval of the new exact text + before publishing it. +8. Never resolve threads or post general PR comments. Requires an authenticated `gh` CLI. Use `gh api graphql --paginate` to collect threads and comments, and `gh api` REST calls for reactions and approved replies. diff --git a/packages/skills/src/engineering/review-pr-comments/references/gh-commands.md b/packages/skills/src/engineering/review-pr-comments/references/gh-commands.md index 3fba565..f660982 100644 --- a/packages/skills/src/engineering/review-pr-comments/references/gh-commands.md +++ b/packages/skills/src/engineering/review-pr-comments/references/gh-commands.md @@ -75,6 +75,9 @@ query($threadId: ID!, $endCursor: String) { ## Replace a decision reaction +Run these mutations only after the complete grouped plan is approved. Do not +run them while collecting decisions or if the grouped plan is cancelled. + Set `REACTION` to `+1`, `-1`, or `eyes`. Remove only decision reactions made by the authenticated user, then add the selected reaction: diff --git a/packages/skills/src/engineering/review-pr-comments/references/workflow.md b/packages/skills/src/engineering/review-pr-comments/references/workflow.md index cb1c533..7f946c5 100644 --- a/packages/skills/src/engineering/review-pr-comments/references/workflow.md +++ b/packages/skills/src/engineering/review-pr-comments/references/workflow.md @@ -13,45 +13,64 @@ thread. Skip acknowledgements, status messages, duplicates, and replies with no request or technical claim; report the reason. -## Decide and react +## Decide and stage the batch -For each actionable comment, show: +For each actionable comment, show and retain: - author, file/line, and comment URL - requested change - whether it is correct, relevant, and still applicable - recommendation with brief evidence -- concise possible replies when different decisions need different responses +- one concise suggested reply, or an explicit reason no reply is needed Ask one question at a time: - `Accept (👍)` — valid and should be addressed - `Defer (👀)` — valid, but should be handled later -- `Ignore (👎)` — invalid, irrelevant, duplicate, or already addressed +- `Reject (👎)` — invalid, irrelevant, duplicate, or already addressed -After the decision, use the reaction commands in -[`gh-commands.md`](gh-commands.md). Replace only the authenticated user's -previous `+1`, `-1`, or `eyes` decision reactions. +Do not call any reaction or reply mutation while questions remain. Store the +decision, target comment, and suggested reply for each item. For a single +actionable comment, the same staging rules apply; skip the batch summary only +when there is no second item to group. -Do not react to non-actionable comments. A reaction does not mean code changed. -If implementation is explicitly requested, treat it as separate work that may -run asynchronously. +After all decisions are collected, show a grouped summary containing every +comment, selected reaction, and exact reply draft. Identify accepted comments +that require code changes and ask explicitly: + +- `Implement the accepted changes` +- `Leave code unchanged` + +An `Accept` decision does not authorize editing code. If implementation is +approved, make and validate the changes before finalizing any affected reply +drafts. If implementation is declined, leave the code unchanged and continue +with the selected decisions. Do not react to non-actionable comments. A +reaction does not mean code changed. + +After any authorized implementation is complete, show the refreshed grouped +plan and ask for one confirmation to publish it. If the user declines, publish +nothing. If implementation is explicitly requested but the user declines this +consent step, treat it as declined rather than inferring approval. ## Draft and approve a reply Draft a concise reply consistent with the decision. Do not claim a change was implemented or verified unless that work completed. -Show the exact text and target URL, then ask: +During triage, show the exact draft and target URL alongside the decision +question. The decision does not authorize changing the draft or targeting a +different comment. Before the grouped publish confirmation, ask: -- `Post exactly as shown` -- `Edit before posting` -- `Do not reply` +- `Publish the grouped plan` +- `Edit a draft` +- `Cancel publication` -Only the first choice authorizes posting that exact draft. If edited, show the -new exact text and ask again. Reaction approval never authorizes a reply. +If edited, show the new exact text and ask for approval of that draft before +the grouped publish confirmation. Reaction approval never authorizes a reply +unless the exact draft is included in the confirmed plan. A `Defer` or +`Reject` decision may intentionally have no reply. -After exact-text approval, use the review-comment reply command in +After grouped approval, use the reaction and review-comment reply commands in [`gh-commands.md`](gh-commands.md). Never use a general PR comment endpoint and never resolve or unresolve threads. diff --git a/plugins/engineering-skills/.claude-plugin/plugin.json b/plugins/engineering-skills/.claude-plugin/plugin.json index 7760e7d..fc38ec7 100644 --- a/plugins/engineering-skills/.claude-plugin/plugin.json +++ b/plugins/engineering-skills/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "engineering-skills", "description": "AI Skills for engineering workflows: triage unresolved pull-request review comments and draft approval-gated replies.", - "version": "0.1.0", + "version": "0.1.1", "author": { "name": "kin0992" }, diff --git a/plugins/engineering-skills/.github/plugin.json b/plugins/engineering-skills/.github/plugin.json index ac773bc..81263bf 100644 --- a/plugins/engineering-skills/.github/plugin.json +++ b/plugins/engineering-skills/.github/plugin.json @@ -1,7 +1,7 @@ { "name": "engineering-skills", "description": "AI Skills for engineering workflows: triage unresolved pull-request review comments and draft approval-gated replies.", - "version": "0.1.0", + "version": "0.1.1", "author": { "name": "kin0992" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7f4ba09..a4a62ae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -46,8 +46,8 @@ importers: specifier: catalog:core version: 1.77.0 turbo: - specifier: ^2.10.8 - version: 2.10.8 + specifier: ^2.10.9 + version: 2.10.9 typescript: specifier: catalog:core version: 7.0.2 @@ -1047,33 +1047,33 @@ packages: resolution: {integrity: sha512-Y8cK9aggNRsqJVaKUlEYs4s7CvQ1b1ta2DVPyAimb0I2qhzjNk+A+mxvll/klL0RlfuIUei8BF7YWiua4kQqww==} engines: {node: ^20.17.0 || >=22.9.0} - '@turbo/darwin-64@2.10.8': - resolution: {integrity: sha512-po+7rfJfUnFXjWlcoN2RwhErgzCdRtBc1T26vYPcywHlggmCQiQe1uWaE4j+BibI2uY9/2pDoFzMN0rmSaPFOw==} + '@turbo/darwin-64@2.10.9': + resolution: {integrity: sha512-Jh+pTGXLNz8+1tkUU13TI/f+ZOI+OvC4YbHi1H+57iSpLt5DR3xgptd+4sA07RdjdRR/RX/01uQQu2OkbzIefA==} cpu: [x64] os: [darwin] - '@turbo/darwin-arm64@2.10.8': - resolution: {integrity: sha512-+zB2btDJ00lnPRuqOvpVvgl4x34k/djZQGZTTCfjn7JgNCl8QFY5Njo5+dqkY1g/+9gbbsnAvWm9CmJg9ebcXA==} + '@turbo/darwin-arm64@2.10.9': + resolution: {integrity: sha512-aqtpPkiIC4IUas8Vv27oJ3aDfTuP1d5wofd01dZ7gfhHRrIKuFdqQb4imvNnVFahcOViF9Jh8Oi7feDoz8/ciA==} cpu: [arm64] os: [darwin] - '@turbo/linux-64@2.10.8': - resolution: {integrity: sha512-K1dxqiVisyN7cViVsfQLs6xscQbYuI8aO2nbUhFURDACgEDfZRdP/b4CCxeosBJpcMfhYyiibWqJorCnvz9kKg==} + '@turbo/linux-64@2.10.9': + resolution: {integrity: sha512-XyAneUBsS5uNOUOjBSs81zyigMVwwhVUd3u7F2JFMKGQk6F7eNAiOEBASJ+aHLldjYsOEjhfW+5gWIqwziyFFw==} cpu: [x64] os: [android, linux] - '@turbo/linux-arm64@2.10.8': - resolution: {integrity: sha512-Gi77ibVnrE1fEmvr+/wBD/yvRqhwp/RQuCp2+//lv1U1wNFFyVg0V7Wj8FG9FXPFAw5QHReo8rxc9+wBSDZjzA==} + '@turbo/linux-arm64@2.10.9': + resolution: {integrity: sha512-5jAcldLnkuWIjujGhCn2MGIeUwW8IVNOq9Sce4EEzzgLcxmhTasbV0RiW6ZkaRdOjmOCGzeNXPLkDJEOIucOLw==} cpu: [arm64] os: [android, linux] - '@turbo/windows-64@2.10.8': - resolution: {integrity: sha512-znnLO1haJPYTHoKMKwlAvlkjRiYbbhBzME6wIGaMd+fwir23U6jVd1ecaTWWi1fbnRVqxMfgDBKseQ/hLKb83g==} + '@turbo/windows-64@2.10.9': + resolution: {integrity: sha512-u1xGpGlefzuhBedbt/VR2nWdftfFVZwPeDg9e5uZl68sV1fL3HNYTNr5VyHkzgtPK2VTYg1x4OKZuGrh1Gvhtg==} cpu: [x64] os: [win32] - '@turbo/windows-arm64@2.10.8': - resolution: {integrity: sha512-VN30vh3b3Czh2WzYHNTfF1FE0YMZ5aHsLO8dBMGHJewA6792wX6iJR8ZxlzFW6WdOu0gEAKIvlYhfyT81Wkm4Q==} + '@turbo/windows-arm64@2.10.9': + resolution: {integrity: sha512-W2Ub165Qv0iMFljbYKxxbZN+2dVSngnzEjQNEFXtnz5oJVx9XSypmNmUvMtuYMeZ3kdVC1zI7yd/rtuX+SDnbg==} cpu: [arm64] os: [win32] @@ -2130,8 +2130,8 @@ packages: resolution: {integrity: sha512-50QV99kCKH5P/Vs4E2Gzp7BopNV+KzTXqWeaxrfu5IQJBOULRsTIS9seSsOVT8ZnGXzCyx55nYWAi4qJzpZKEQ==} engines: {node: ^20.17.0 || >=22.9.0} - turbo@2.10.8: - resolution: {integrity: sha512-9+8YX5QOkGXzZxcIykTHgaooRHGMWO+jfdyRK0o+rN0U7hBIig2MrJ8r/aNzIPDPhdA73SGb0O+tIztaModTMg==} + turbo@2.10.9: + resolution: {integrity: sha512-Yl9+ukxH+UmPtKidpDkjn82tvPoEvFNb9UACd9vUomN1Ft0cwl3rx0P8yC1D93W9EOsWRMjllvIDG8y25sFOog==} hasBin: true typescript@5.9.3: @@ -3171,22 +3171,22 @@ snapshots: '@tufjs/canonical-json': 2.0.0 minimatch: 10.2.6 - '@turbo/darwin-64@2.10.8': + '@turbo/darwin-64@2.10.9': optional: true - '@turbo/darwin-arm64@2.10.8': + '@turbo/darwin-arm64@2.10.9': optional: true - '@turbo/linux-64@2.10.8': + '@turbo/linux-64@2.10.9': optional: true - '@turbo/linux-arm64@2.10.8': + '@turbo/linux-arm64@2.10.9': optional: true - '@turbo/windows-64@2.10.8': + '@turbo/windows-64@2.10.9': optional: true - '@turbo/windows-arm64@2.10.8': + '@turbo/windows-arm64@2.10.9': optional: true '@types/chai@5.2.3': @@ -4238,14 +4238,14 @@ snapshots: transitivePeerDependencies: - supports-color - turbo@2.10.8: + turbo@2.10.9: optionalDependencies: - '@turbo/darwin-64': 2.10.8 - '@turbo/darwin-arm64': 2.10.8 - '@turbo/linux-64': 2.10.8 - '@turbo/linux-arm64': 2.10.8 - '@turbo/windows-64': 2.10.8 - '@turbo/windows-arm64': 2.10.8 + '@turbo/darwin-64': 2.10.9 + '@turbo/darwin-arm64': 2.10.9 + '@turbo/linux-64': 2.10.9 + '@turbo/linux-arm64': 2.10.9 + '@turbo/windows-64': 2.10.9 + '@turbo/windows-arm64': 2.10.9 typescript@5.9.3: {}