diff --git a/.agents/skills/PR_WORKFLOW.md b/.agents/skills/PR_WORKFLOW.md index 50b3c35616..0af4d32314 100644 --- a/.agents/skills/PR_WORKFLOW.md +++ b/.agents/skills/PR_WORKFLOW.md @@ -1,6 +1,6 @@ # PR Review Instructions -# Please read this in full and do not skip sections. +Please read this in full and do not skip sections. ## Working rule @@ -25,6 +25,7 @@ Do not continue if you cannot verify the problem is real or test the fix. - 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 external-input boundaries typed and validated, including CLI input, environment variables, network payloads, and tool output. - 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. @@ -32,6 +33,12 @@ Do not continue if you cannot verify the problem is real or test the fix. ## Unified workflow +Entry criteria: + +- PR URL/number is known. +- Problem statement is clear enough to attempt reproduction. +- A realistic verification path exists (tests, integration checks, or explicit manual validation). + ### 1) `review-pr` Purpose: @@ -54,6 +61,12 @@ Can we fix up everything? Do we have any questions? ``` +Stop and escalate instead of continuing if: + +- The problem cannot be reproduced or confirmed. +- The proposed PR scope does not match the stated problem. +- The design introduces unresolved security or trust-boundary concerns. + ### 2) `prepare-pr` Purpose: @@ -78,8 +91,15 @@ 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? +Did any changes introduce any potential security vulnerabilities? ``` +Stop and escalate instead of continuing if: + +- You cannot verify behavior changes with meaningful tests or validation. +- Fixing findings requires broad architecture changes outside safe PR scope. +- Security hardening requirements remain unresolved. + ### 3) `merge-pr` Purpose: @@ -87,6 +107,13 @@ 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. +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. +- Required CI checks are green and the branch is not behind `main`. + Expected output: - Successful merge commit and recorded merge SHA. @@ -94,4 +121,5 @@ Expected output: 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?