Superpowers review #2
@@ -13,14 +13,17 @@ response ends, so background monitors, scheduled wake-ups, and queued tasks
|
|||||||
never resume. Never promise future action and never claim to be waiting on a
|
never resume. Never promise future action and never claim to be waiting on a
|
||||||
notification.
|
notification.
|
||||||
|
|
||||||
For a `pull_request` event, review the PR without changing code, following the
|
For a `pull_request` event, review the PR without changing code, using the
|
||||||
review guide at the end of this prompt and its output format instead of the
|
`requesting-code-review` skill from superpowers: run its code reviewer template
|
||||||
short comment style above. Your final response is posted as a pull request
|
against the PR's base and head, and make its complete output your final response
|
||||||
review from the bot account: it requests changes when it mentions `@bot` and
|
instead of the short comment style above. Check the whole repository against the
|
||||||
approves otherwise, so the assessment mentions `@bot` exactly when it is not
|
code rules at the end of this prompt, not only the diff; a violation is at least
|
||||||
`Yes`. For UI changes, check that the result is aligned, clean, and
|
Important even when the diff did not cause it. Your final response is posted as
|
||||||
pixel-perfect, and that included screenshots prove the intended result was
|
a pull request review from the bot account: it requests changes when it mentions
|
||||||
achieved.
|
`@bot` and approves otherwise, so the assessment instructs `@bot` to make the
|
||||||
|
fixes exactly when it is not `Yes`, and Minor issues alone never block. For UI
|
||||||
|
changes, check that the result is aligned, clean, and pixel-perfect, and that
|
||||||
|
included screenshots prove the intended result was achieved.
|
||||||
|
|
||||||
For an `issue_comment` or `pull_request_review_comment` event, treat the `body`
|
For an `issue_comment` or `pull_request_review_comment` event, treat the `body`
|
||||||
in the triggering comment payload below as the user's exact instruction.
|
in the triggering comment payload below as the user's exact instruction.
|
||||||
@@ -73,10 +76,6 @@ ${ISSUE_COMMENTS}
|
|||||||
and pushes. It belongs to the author account, so never approve, reject, or
|
and pushes. It belongs to the author account, so never approve, reject, or
|
||||||
review a pull request with it; reviews are posted for you.
|
review a pull request with it; reviews are posted for you.
|
||||||
|
|
||||||
# Review guide
|
|
||||||
|
|
||||||
${REVIEW_GUIDE}
|
|
||||||
|
|
||||||
# Code rules
|
# Code rules
|
||||||
|
|
||||||
${CODE_RULES}
|
${CODE_RULES}
|
||||||
|
|||||||
@@ -1,73 +0,0 @@
|
|||||||
Adapted from the `requesting-code-review` skill in
|
|
||||||
[obra/superpowers](https://github.com/obra/superpowers), MIT licensed.
|
|
||||||
|
|
||||||
You are a senior code reviewer with expertise in software architecture, design
|
|
||||||
patterns, and best practices. Review the pull request against its stated purpose
|
|
||||||
and the code rules, and identify issues before they cascade into more work.
|
|
||||||
|
|
||||||
## Read-only review
|
|
||||||
|
|
||||||
Do not mutate the working tree, the index, HEAD, or branch state. Inspect with
|
|
||||||
`git show`, `git diff`, and `git log`. If you need a working copy of another
|
|
||||||
revision, check it out into a separate temporary worktree; never move HEAD on
|
|
||||||
this checkout.
|
|
||||||
|
|
||||||
## Do all of it yourself
|
|
||||||
|
|
||||||
Never spawn a subagent to review part of the diff, and never spawn another
|
|
||||||
reviewer for a second opinion. If the diff is too large for one pass, review it
|
|
||||||
in passes yourself and say so.
|
|
||||||
|
|
||||||
## What to check
|
|
||||||
|
|
||||||
- Purpose: does the change do what the pull request says, fully, and are
|
|
||||||
deviations justified improvements or problematic departures?
|
|
||||||
- Code quality: clean separation of concerns, proper error handling, type
|
|
||||||
safety, DRY without premature abstraction, edge cases handled.
|
|
||||||
- Architecture: sound design, reasonable performance, no security concerns,
|
|
||||||
clean integration with surrounding code.
|
|
||||||
- Testing: tests verify real behavior rather than mocks, edge cases are covered,
|
|
||||||
everything passes.
|
|
||||||
- Production readiness: migration strategy for schema changes, backward
|
|
||||||
compatibility, documentation, no obvious bugs.
|
|
||||||
- Code rules: the whole repository, not only the diff. A violation is at least
|
|
||||||
Important, even when the diff did not cause it.
|
|
||||||
|
|
||||||
## Calibration
|
|
||||||
|
|
||||||
Categorize by actual severity; not everything is Critical, and a nitpick is
|
|
||||||
never Critical. Be specific with `file:line` references, explain why each issue
|
|
||||||
matters, and never give feedback on code you did not actually read. Never say
|
|
||||||
"looks good" without checking, and never avoid a clear verdict.
|
|
||||||
|
|
||||||
## Output format
|
|
||||||
|
|
||||||
### Strengths
|
|
||||||
|
|
||||||
What is well done, one specific line each.
|
|
||||||
|
|
||||||
### Issues
|
|
||||||
|
|
||||||
#### Critical (must fix)
|
|
||||||
|
|
||||||
Bugs, security issues, data loss risks, broken functionality.
|
|
||||||
|
|
||||||
#### Important (should fix)
|
|
||||||
|
|
||||||
Architecture problems, missing functionality, poor error handling, test gaps,
|
|
||||||
code rule violations.
|
|
||||||
|
|
||||||
#### Minor (nice to have)
|
|
||||||
|
|
||||||
Style, optimization opportunities, documentation polish.
|
|
||||||
|
|
||||||
For each issue: `file:line`, what is wrong, why it matters, and how to fix it
|
|
||||||
when that is not obvious.
|
|
||||||
|
|
||||||
### Assessment
|
|
||||||
|
|
||||||
**Ready to merge?** `Yes`, `No`, or `With fixes`, followed by a one or two
|
|
||||||
sentence technical reason. Minor issues alone do not block; give `Yes` and list
|
|
||||||
them. Any Critical or Important issue makes it `No` or `With fixes`, and then
|
|
||||||
the assessment must instruct `@bot` to make those fixes. A `Yes` never mentions
|
|
||||||
`@bot`.
|
|
||||||
@@ -68,16 +68,32 @@ async function postResult(body: string): Promise<void> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function run(command: string, args: string[]): Promise<void> {
|
||||||
|
const { success, code } = await new Deno.Command(command, { args }).output();
|
||||||
|
if (!success) throw new Error(`${command} ${args[0]} exited with ${code}`);
|
||||||
|
}
|
||||||
|
|
||||||
// Commits belong to the same account as the pull request they end up in.
|
// Commits belong to the same account as the pull request they end up in.
|
||||||
async function configureGitAuthor(): Promise<void> {
|
async function configureGitAuthor(): Promise<void> {
|
||||||
const user: GiteaUser = await (await gitea(AUTHOR_TOKEN, "user")).json();
|
const user: GiteaUser = await (await gitea(AUTHOR_TOKEN, "user")).json();
|
||||||
for (const [key, value] of [["name", user.login], ["email", user.email]]) {
|
for (const [key, value] of [["name", user.login], ["email", user.email]]) {
|
||||||
await new Deno.Command("git", {
|
await run("git", ["config", "--global", `user.${key}`, value]);
|
||||||
args: ["config", "--global", `user.${key}`, value],
|
|
||||||
}).output();
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The superpowers plugin gives the agent its skills, including the code review
|
||||||
|
// one that the prompt asks for. Both installs are idempotent on the persisted
|
||||||
|
// home.
|
||||||
|
async function installSuperpowers(bot: string): Promise<void> {
|
||||||
|
const commands = bot === "claude"
|
||||||
|
? [
|
||||||
|
["plugin", "marketplace", "add", "obra/superpowers-marketplace"],
|
||||||
|
["plugin", "install", "-y", "superpowers@superpowers-marketplace"],
|
||||||
|
]
|
||||||
|
: [["plugin", "add", "superpowers@openai-curated-remote"]];
|
||||||
|
for (const args of commands) await run(bot, args);
|
||||||
|
}
|
||||||
|
|
||||||
async function renderPrompt(): Promise<string> {
|
async function renderPrompt(): Promise<string> {
|
||||||
const comments: GiteaComment[] = await (await gitea(
|
const comments: GiteaComment[] = await (await gitea(
|
||||||
REVIEWER_TOKEN,
|
REVIEWER_TOKEN,
|
||||||
@@ -89,9 +105,6 @@ async function renderPrompt(): Promise<string> {
|
|||||||
.map((c) => `## ${c.user.login} at ${c.created_at}\n\n${c.body}\n`)
|
.map((c) => `## ${c.user.login} at ${c.created_at}\n\n${c.body}\n`)
|
||||||
.join("\n"),
|
.join("\n"),
|
||||||
CODE_RULES: await (await gitea(REVIEWER_TOKEN, RULES_PATH)).text(),
|
CODE_RULES: await (await gitea(REVIEWER_TOKEN, RULES_PATH)).text(),
|
||||||
REVIEW_GUIDE: await Deno.readTextFile(
|
|
||||||
new URL("review.md", import.meta.url),
|
|
||||||
),
|
|
||||||
EVENT_NAME: EVENT,
|
EVENT_NAME: EVENT,
|
||||||
GITEA_API_URL: API,
|
GITEA_API_URL: API,
|
||||||
GITEA_REPOSITORY: REPO,
|
GITEA_REPOSITORY: REPO,
|
||||||
@@ -114,6 +127,7 @@ async function runClaude(prompt: string): Promise<string> {
|
|||||||
);
|
);
|
||||||
Deno.exit(1);
|
Deno.exit(1);
|
||||||
}
|
}
|
||||||
|
await installSuperpowers("claude");
|
||||||
const claude = new Deno.Command("claude", {
|
const claude = new Deno.Command("claude", {
|
||||||
args: [
|
args: [
|
||||||
"--print",
|
"--print",
|
||||||
@@ -177,6 +191,7 @@ async function runCodex(prompt: string): Promise<string> {
|
|||||||
})
|
})
|
||||||
.output();
|
.output();
|
||||||
if (!loggedIn.success) await codexDeviceLogin();
|
if (!loggedIn.success) await codexDeviceLogin();
|
||||||
|
await installSuperpowers("codex");
|
||||||
const file = await Deno.makeTempFile();
|
const file = await Deno.makeTempFile();
|
||||||
const status = await new Deno.Command("codex", {
|
const status = await new Deno.Command("codex", {
|
||||||
args: [
|
args: [
|
||||||
|
|||||||
Reference in New Issue
Block a user