Skip to content

Commit be4a3d0

Browse files
committed
chore(build): expand review-pr skill and add approve/reject-pr skills
Strengthen the review-pr agent skill to enforce the client's cross-repo testing reality and zero-GC discipline, and add the companion approve-pr and reject-pr skills. review-pr: - Add Step 2.7 cross-repo tandem-PR testing gate: this repo has no real-server e2e, so server-observable changes need a tandem questdb OSS PR (matching branch, cross-linked), HA changes need a questdb-enterprise PR, and kill -9 scenarios need Enterprise e2e-python tests. A required but missing tandem is Critical and forces the covered behavior UNTESTED. - Add Step 2.6 cross-repo test coverage map (mandatory at every level); a row's covering test may be local or in a linked tandem PR. - Enforce zero-GC on the ingestion hot path: rewrite Reviewer 3, add mandatory Reviewer 11 (adversarial performance & zero-GC), and make a single steady-state per-row allocation a blocking finding (14 reviewers). - Spot whitebox tests and strongly prefer blackbox: flag reflection into internals, private-state assertions, and timing coupling that rot into flakes; require naming the public-API blackbox alternative. - Add Step 4 test/tandem and zero-GC gates plus a rendered coverage map. approve-pr / reject-pr: - Post the conversation's review as an approving or change-request review. - Gate approve on zero open Critical findings plus the zero-GC and test/tandem gates; auto-create the QUEUED FOR MERGE label if absent and guard the READY label removal, since this repo does not define them.
1 parent 24fbbd5 commit be4a3d0

3 files changed

Lines changed: 322 additions & 26 deletions

File tree

.pi/skills/approve-pr/SKILL.md

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
---
2+
name: approve-pr
3+
description: Approve the pull request for the currently checked-out branch. Posts the review from this conversation as an approving review and adds the "QUEUED FOR MERGE" label. Use after review-pr when you decide to approve.
4+
allowed-tools: bash read write
5+
metadata:
6+
argument-hint: "[optional PR number or URL to override auto-detection]"
7+
---
8+
9+
# Approve a QuestDB client pull request
10+
11+
Approve the pull request for the branch that is currently checked out. This skill
12+
is meant to be run **right after `review-pr`** in the same conversation: it takes
13+
the review you just produced and posts it as an approving review. Do not
14+
re-review here — post the existing review.
15+
16+
This repo (`java-questdb-client`) is a submodule of `questdb`. Approving the
17+
client PR does not approve any tandem PR — if the review required a tandem OSS or
18+
Enterprise PR (see the review-pr Step 2.7 gate), that PR must be approved
19+
separately in its own repo.
20+
21+
Actions performed, in order:
22+
1. Post the review as the PR's review body (this is the "PR comment").
23+
2. Approve the PR.
24+
3. Add the `QUEUED FOR MERGE` label (creating it if this repo does not have it yet).
25+
26+
## Step 0: Confirm the review actually clears the bar
27+
28+
Only approve if the most recent `review-pr` report in this conversation has a
29+
verdict of **approve** with, per the review-pr Step 4 gates:
30+
- ZERO open Critical findings, and zero open correctness / concurrency /
31+
resource findings;
32+
- the **Zero-GC gate** satisfied — no open steady-state (per-row / per-producer-call)
33+
allocation or reachable algorithmic inefficiency on the ingestion path;
34+
- the **Test & tandem gate** satisfied — no UNTESTED Critical rows in the Step 2.6
35+
coverage map, and every required tandem PR (OSS e2e / Enterprise / Enterprise
36+
e2e-python) is present, linked, and running its suite.
37+
38+
If any such finding is still open — including a required-but-missing tandem PR —
39+
do NOT approve. Tell the user the PR does not meet the bar and suggest
40+
`reject-pr`.
41+
42+
- If no review report exists in this conversation, STOP and tell the user to run
43+
`/skill:review-pr` first — do not approve unreviewed code.
44+
- Post the review verbatim; do not shorten or rewrite it.
45+
46+
## Step 1: Detect the PR
47+
48+
Auto-detect the PR from the checked-out branch. An explicit PR number/URL in the
49+
arguments overrides detection.
50+
51+
```bash
52+
PR='<explicit PR number/URL from arguments, else empty>'
53+
[ -z "$PR" ] && PR=$(gh pr view --json number -q .number 2>/dev/null)
54+
if [ -z "$PR" ]; then
55+
echo "No PR found for the current branch. Run 'gh pr checkout <n>' or pass a PR number."; exit 1
56+
fi
57+
gh pr view "$PR" --json number,title,author,headRefName,url,state,labels
58+
echo "Approving PR #$PR"
59+
```
60+
61+
State one line to the user: which PR (`#number — title`) you are about to approve,
62+
then proceed.
63+
64+
## Step 2: Write the review body to a file
65+
66+
Write the review body to `/tmp/approve-pr-$PR.md` using the `write` tool (never
67+
inline it into a shell command — reviews contain backticks, quotes, and `$` that
68+
break shell quoting). The body is the full verbatim `review-pr` report.
69+
70+
## Step 3: Post the approval
71+
72+
```bash
73+
gh pr review "$PR" --approve --body-file /tmp/approve-pr-$PR.md
74+
75+
# This repo may not have the label yet — create it idempotently before adding.
76+
if ! gh label list --limit 200 | grep -qx "QUEUED FOR MERGE"; then
77+
gh label create "QUEUED FOR MERGE" \
78+
--color f5e70d \
79+
--description "Approved PR in the merge queue. Do not merge master into this PR." \
80+
--force
81+
fi
82+
gh pr edit "$PR" --add-label "QUEUED FOR MERGE"
83+
```
84+
85+
Notes:
86+
- GitHub does not allow approving your **own** PR. If `gh pr review --approve`
87+
fails for that reason, fall back to
88+
`gh pr comment "$PR" --body-file /tmp/approve-pr-$PR.md`, tell the user the PR
89+
could not be formally approved because it is self-authored, and still add the
90+
`QUEUED FOR MERGE` label.
91+
92+
## Step 4: Confirm
93+
94+
Report back in one or two lines:
95+
- PR number + title, "approved" posted (or the fallback used).
96+
- Whether the `QUEUED FOR MERGE` label was added (and whether it had to be created).
97+
- If the review required a tandem PR, remind the user to approve that tandem in its own repo.
98+
- The PR URL.

.pi/skills/reject-pr/SKILL.md

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
---
2+
name: reject-pr
3+
description: Reject the pull request for the currently checked-out branch. Posts the review from this conversation as a change-request review that tags the PR author, sets the PR to "changes requested", and clears the READY label. Use after review-pr when you decide to reject.
4+
allowed-tools: bash read write
5+
metadata:
6+
argument-hint: "[optional PR number or URL to override auto-detection]"
7+
---
8+
9+
# Reject a QuestDB client pull request
10+
11+
Reject the pull request for the branch that is currently checked out. This skill
12+
is meant to be run **right after `review-pr`** in the same conversation: it takes
13+
the review you just produced and turns it into a blocking change-request on the
14+
PR. Do not re-review here — post the existing review.
15+
16+
Actions performed, in order:
17+
1. Post the review as the PR's review body (this is the "PR comment"), tagging
18+
the PR author.
19+
2. Request changes (sets the PR to the "changes requested" state).
20+
3. Clear the `READY` label (if present).
21+
22+
## Step 0: Locate the review to post
23+
24+
The body you post is **the most recent `review-pr` report in this conversation**
25+
(its Critical / Moderate / Minor / Downgraded / Coverage map / Summary output).
26+
27+
- If no review report exists in this conversation, STOP and tell the user to run
28+
`/skill:review-pr` first — do not fabricate a review.
29+
- Do not shorten or rewrite the review; post it verbatim. You only prepend the
30+
author mention.
31+
- A required-but-missing tandem PR (review-pr Step 2.7) or any UNTESTED Critical
32+
row in the coverage map is on its own sufficient grounds to reject — make sure
33+
the posted review states which tandem is missing and what test it must add.
34+
35+
## Step 1: Detect the PR
36+
37+
Auto-detect the PR from the checked-out branch. An explicit PR number/URL in the
38+
arguments overrides detection.
39+
40+
```bash
41+
PR='<explicit PR number/URL from arguments, else empty>'
42+
[ -z "$PR" ] && PR=$(gh pr view --json number -q .number 2>/dev/null)
43+
if [ -z "$PR" ]; then
44+
echo "No PR found for the current branch. Run 'gh pr checkout <n>' or pass a PR number."; exit 1
45+
fi
46+
gh pr view "$PR" --json number,title,author,headRefName,url,state,labels
47+
AUTHOR=$(gh pr view "$PR" --json author -q .author.login)
48+
echo "Rejecting PR #$PR by @$AUTHOR"
49+
```
50+
51+
State one line to the user: which PR (`#number — title`) and author you are about
52+
to reject, then proceed.
53+
54+
## Step 2: Write the review body to a file
55+
56+
Write the review body to `/tmp/reject-pr-$PR.md` using the `write` tool (never
57+
inline it into a shell command — reviews contain backticks, quotes, and `$` that
58+
break shell quoting). The first line must mention the author:
59+
60+
```
61+
@<AUTHOR>
62+
63+
<the full verbatim review-pr report>
64+
```
65+
66+
## Step 3: Post the rejection
67+
68+
```bash
69+
gh pr review "$PR" --request-changes --body-file /tmp/reject-pr-$PR.md
70+
71+
# Clear the READY label only if present (avoids a spurious error; this repo may
72+
# not define the label at all).
73+
if gh pr view "$PR" --json labels -q '.labels[].name' | grep -qx "READY"; then
74+
gh pr edit "$PR" --remove-label "READY"
75+
fi
76+
```
77+
78+
Notes:
79+
- GitHub does not allow requesting changes on your **own** PR. If
80+
`gh pr review --request-changes` fails for that reason, fall back to
81+
`gh pr comment "$PR" --body-file /tmp/reject-pr-$PR.md`, tell the user the PR
82+
could not be moved to "changes requested" because it is self-authored, and
83+
still clear the `READY` label if present.
84+
85+
## Step 4: Confirm
86+
87+
Report back in one or two lines:
88+
- PR number + title, author tagged, "changes requested" posted (or the fallback used).
89+
- Whether the `READY` label was removed or was already absent.
90+
- The PR URL.

0 commit comments

Comments
 (0)