Skip to content

Commit cde29fe

Browse files
gumadeirasgumadeiras
committed
added more explicit instructions
1 parent 6d1daf2 commit cde29fe

File tree

1 file changed

+29
-1
lines changed

1 file changed

+29
-1
lines changed

‎.agents/skills/PR_WORKFLOW.md‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# PR Review Instructions
22

3-
# Please read this in full and do not skip sections.
3+
Please read this in full and do not skip sections.
44

55
## Working rule
66

@@ -25,13 +25,20 @@ Do not continue if you cannot verify the problem is real or test the fix.
2525
- Do not trust PR code by default.
2626
- Do not merge changes you cannot validate with a reproducible problem and a tested fix.
2727
- Keep types strict. Do not use `any` in implementation code.
28+
- Keep external-input boundaries typed and validated, including CLI input, environment variables, network payloads, and tool output.
2829
- Keep implementations properly scoped. Fix root causes, not local symptoms.
2930
- Identify and reuse canonical sources of truth so behavior does not drift across the codebase.
3031
- Harden changes. Always evaluate security impact and abuse paths.
3132
- Understand the system before changing it. Never make the codebase messier just to clear a PR queue.
3233

3334
## Unified workflow
3435

36+
Entry criteria:
37+
38+
- PR URL/number is known.
39+
- Problem statement is clear enough to attempt reproduction.
40+
- A realistic verification path exists (tests, integration checks, or explicit manual validation).
41+
3542
### 1) `review-pr`
3643

3744
Purpose:
@@ -54,6 +61,12 @@ Can we fix up everything?
5461
Do we have any questions?
5562
```
5663

64+
Stop and escalate instead of continuing if:
65+
66+
- The problem cannot be reproduced or confirmed.
67+
- The proposed PR scope does not match the stated problem.
68+
- The design introduces unresolved security or trust-boundary concerns.
69+
5770
### 2) `prepare-pr`
5871

5972
Purpose:
@@ -78,20 +91,35 @@ Do we have enough tests?
7891
Do not add performative tests, ensure tests are real and there are no regressions.
7992
Take your time, fix it properly, refactor if necessary.
8093
Do you see any follow-up refactors we should do?
94+
Did any changes introduce any potential security vulnerabilities?
8195
```
8296

97+
Stop and escalate instead of continuing if:
98+
99+
- You cannot verify behavior changes with meaningful tests or validation.
100+
- Fixing findings requires broad architecture changes outside safe PR scope.
101+
- Security hardening requirements remain unresolved.
102+
83103
### 3) `merge-pr`
84104

85105
Purpose:
86106

87107
- Merge only after review and prep artifacts are present and checks are green.
88108
- Use squash merge flow and verify the PR ends in `MERGED` state.
89109

110+
Go or no-go checklist before merge:
111+
112+
- All BLOCKER and IMPORTANT findings are resolved.
113+
- Verification is meaningful and regression risk is acceptably low.
114+
- Docs and changelog are updated when required.
115+
- Required CI checks are green and the branch is not behind `main`.
116+
90117
Expected output:
91118

92119
- Successful merge commit and recorded merge SHA.
93120
- Worktree cleanup after successful merge.
94121

95122
Maintainer checkpoint after merge:
96123

124+
- Were any refactors intentionally deferred and now need follow-up issue(s)?
97125
- Did this reveal broader architecture or test gaps we should address?

0 commit comments

Comments
 (0)