Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion packages/github-bot/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,45 @@ Two prompt templates in `src/prompts.ts`:
- Run `gh pr diff` for the full diff
- Submit a review via `gh api .../reviews`
- Post inline comments via `gh api .../comments`
- Emit applyable `suggestion` fences in inline comment bodies (see below)

**`buildCommentActionPrompt`** — Includes the user's request (with @mention stripped) and
instructions to:

- Check prior conversation via `gh pr view --comments`
- Make code changes and push, or respond with analysis
- Post a summary comment via `gh api .../issues/{n}/comments`
- Reply to a specific review thread (when `commentId` is present)
- Reply to a specific review thread (when `commentId` is present), optionally with an applyable
`suggestion` fence — the summary comment cannot carry one, since an issue comment has no line
anchor

### Applyable Suggestions

`buildSuggestionGuidelines` serves both prompt paths. A fenced `suggestion` block inside a
**line-anchored** comment is what GitHub renders with a "Commit suggestion" button; the fence
content replaces the anchored lines verbatim. Nothing in this package calls a special suggestion
API—the existing inline-comment and thread-reply routes carry the markdown body.

All review, inline-comment, thread-reply, and summary bodies are passed as `-F body=@<file>` rather
than an inline `-f body="…"` argument. The prompts require the agent to use its file-write tool or a
quoted heredoc delimiter; an unquoted heredoc performs command and variable substitution on
untrusted review text before `gh` reads the file.

For a full review, the agent selects the anchor and must prove every selected line falls inside a
diff hunk. For a thread reply, GitHub inherits the parent comment's anchor, so the agent instead
checks that the supplied code-location hunk still matches the current head and emits no fence after
a commit has moved those lines. Both paths require verbatim replacement, exact indentation, a
scratch-copy check, and a prose fallback when the replacement cannot be proved safe.

### Repository Identity in API Routes

Every `gh api repos/...` route embedded in these prompts uses
`encodeRepositoryPathSegments({ repoOwner: owner, repoName: repo })`
(`@open-inspect/shared/types/repositories`) rather than maintaining another route-construction rule.
GitHub login owners are currently single segments, so this is behavior-preserving for webhook
traffic; using the shared helper keeps the prompt aligned with the repository-wide identity
contract, which also permits nested namespaces on other source-control providers. Human-readable
`${owner}/${repo}` prose stays unencoded.

The prompts embed only metadata from the webhook payload. The agent gathers everything else.

Expand Down
108 changes: 96 additions & 12 deletions packages/github-bot/src/prompts.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { encodeRepositoryPathSegments } from "@open-inspect/shared/types/repositories";

function buildCustomInstructionsSection(instructions: string | null | undefined): string {
if (!instructions?.trim()) return "";
return `\n## Custom Instructions\n${instructions}`;
Expand All @@ -14,6 +16,67 @@ function buildCommentGuidelines(isPublicRepo: boolean): string {
- Compose your full response before posting any comments.`;
}

function buildSuggestionGuidelines(anchorMode: "selected" | "inherited"): string {
const anchorRules =
anchorMode === "selected"
? `- Prefer a single-line anchor (\`line\` only) even when the replacement is several lines:
one anchored line may be replaced by any number of lines. Use \`start_line\` only to replace a
contiguous range.
- EVERY anchor line must fall inside a hunk of \`gh pr diff\` — the single \`line\` of a
single-line anchor, and both \`start_line\` and \`line\` of a range. An anchor outside the diff
rejects that comment with HTTP 422 and the feedback is lost.`
: `- A thread reply inherits the parent comment's anchor; do not send \`line\`, \`start_line\`,
or \`side\` fields on the reply request.
- Verify the inherited anchor against the ## Code Location hunk and the current head. If any pushed
commit moved or replaced those lines, post prose and NO fence.`;
const verification =
anchorMode === "selected"
? `1. Run \`gh pr diff\` (it accepts a PR number, URL, or branch — never a file path) and
confirm every anchor line — including a lone \`line\` anchor, not only range endpoints — falls
inside a hunk.
2. Print the exact anchored lines (\`sed -n 'START,ENDp' <path>\`) and confirm your fence is a
correct verbatim replacement for precisely those lines.
3. Apply the replacement to a scratch copy and run the cheapest correctness check the repo offers
for that file (syntax parse, type check, or its linter).`
: `1. Confirm the ## Code Location hunk still describes the current head and identify the exact
inherited lines it anchors.
2. Apply the replacement to a scratch copy and run the cheapest correctness check the repo offers
for that file (syntax parse, type check, or its linter).`;

return `
## Applyable Suggestions
A fenced \`suggestion\` block inside a line-anchored review comment renders in GitHub with a
"Commit suggestion" button, so the author applies your fix in one click. Use one whenever the fix
is a concrete, local edit you can state exactly:

\`\`\`suggestion
<the replacement lines>
\`\`\`

Hard rules — the fence content REPLACES the comment's anchored lines verbatim:
- It is not a diff and not an excerpt. Never put \`+\`/\`-\` markers, \`...\`, placeholders, TODOs,
or prose inside the fence.
- Reproduce the original leading whitespace exactly. A suggestion with wrong indentation still
applies cleanly and breaks the file.
- Include only the anchored lines — no surrounding unchanged lines for context.
${anchorRules}
- One suggestion per comment. The explanation goes above the fence, never inside it.

Verify before you suggest. The repo is checked out on the PR head branch, so check instead of
guessing:
${verification}

If any check fails, or the real fix spans multiple files, needs an import or declaration
elsewhere, or turns on a judgment call, describe the fix in prose and post NO fence. A wrong
suggestion is worse than none: it is one click away from being merged.`;
}

function writeBodyFileInstruction(path: string): string {
return `write \`${path}\` with your file-write tool. If you use the shell, use a quoted heredoc
delimiter such as \`cat > ${path} <<'EOF'\`; an unquoted delimiter performs command and variable
substitution on untrusted review text`;
}

function buildUntrustedUserContentBlock(params: {
source: string;
author: string;
Expand Down Expand Up @@ -60,6 +123,7 @@ export function buildCodeReviewPrompt(params: {
codeReviewInstructions,
isSelfReview = false,
} = params;
const repoPath = encodeRepositoryPathSegments({ repoOwner: owner, repoName: repo });
const reviewEvent = isSelfReview ? "COMMENT" : "COMMENT|APPROVE|REQUEST_CHANGES";
const reviewEventGuidance = isSelfReview
? "Use COMMENT because GitHub does not allow pull request authors to approve their own PRs."
Expand Down Expand Up @@ -107,25 +171,33 @@ ${prDescriptionBlock}
- Performance implications
- Code clarity and maintainability
3. You may read individual files in the repo for additional context beyond the diff
4. When your review is complete, submit it via:
4. When your review is complete, ${writeBodyFileInstruction("/tmp/review.md")}. Then submit it via:

gh api repos/${owner}/${repo}/pulls/${number}/reviews \\
gh api repos/${repoPath}/pulls/${number}/reviews \\
--method POST \\
-f body="<your review summary>" \\
-F body=@/tmp/review.md \\
-f event="${reviewEvent}"

${reviewEventGuidance}

5. For inline comments on specific files:
5. For inline comments on specific files, first ${writeBodyFileInstruction("/tmp/comment.md")}:

gh api repos/${owner}/${repo}/pulls/${number}/comments \\
gh api repos/${repoPath}/pulls/${number}/comments \\
--method POST \\
-f body="<comment>" \\
-F body=@/tmp/comment.md \\
-f path="<file path>" \\
-f commit_id="$(gh api repos/${owner}/${repo}/pulls/${number} --jq '.head.sha')" \\
-f line=<line number> \\
-f commit_id="$(gh api repos/${repoPath}/pulls/${number} --jq '.head.sha')" \\
-F line=<line number> \\
-f side="RIGHT"

\`line\` and \`start_line\` must go through \`-F\` (typed), not \`-f\` (raw string): the API rejects
a string line with \`"3" is not an integer\`. Add \`-F start_line=<first line>\` and
\`-f start_side="RIGHT"\` to anchor a contiguous range instead of a single line.

Prefer a suggestion over prose whenever the fix is a concrete, local edit — see
## Applyable Suggestions below. It is the difference between a review the author has to
re-implement and one they can apply.
${buildSuggestionGuidelines("selected")}
${buildCustomInstructionsSection(codeReviewInstructions)}
${buildCommentGuidelines(isPublic)}`;
}
Expand Down Expand Up @@ -160,6 +232,7 @@ export function buildCommentActionPrompt(params: {
commentId,
commentActionInstructions,
} = params;
const repoPath = encodeRepositoryPathSegments({ repoOwner: owner, repoName: repo });

const intro = head
? `You are working on Pull Request #${number} in ${owner}/${repo}.\nThe repository has been cloned and you are on the ${head} branch.`
Expand All @@ -178,8 +251,19 @@ export function buildCommentActionPrompt(params: {
}

let replyInstruction = "";
let suggestionSection = "";
if (commentId) {
replyInstruction = `\n5. If you need to reply to the specific review thread:\n\n gh api repos/${owner}/${repo}/pulls/${number}/comments/${commentId}/replies \\\n --method POST \\\n -f body="<your reply>"`;
replyInstruction = `
5. If you need to reply to the specific review thread, first ${writeBodyFileInstruction("/tmp/reply.md")}:

gh api repos/${repoPath}/pulls/${number}/comments/${commentId}/replies \\
--method POST \\
-F body=@/tmp/reply.md

A thread reply inherits the parent comment's line anchor, so it can carry an applyable
suggestion. The summary comment cannot: an issue comment has no line anchor, and a suggestion
fence there renders as an inert code block.`;
suggestionSection = `\n${buildSuggestionGuidelines("inherited")}`;
}

return `${intro}${prDetails}${codeLocation}
Expand All @@ -197,11 +281,11 @@ ${buildUntrustedUserContentBlock({
3. Address the request:
- If code changes are needed, make them and push to the current branch
- If it's a question, respond with your analysis
4. When done, post a summary comment on the PR:
4. When done, ${writeBodyFileInstruction("/tmp/summary.md")}. Then post it on the PR:

gh api repos/${owner}/${repo}/issues/${number}/comments \\
gh api repos/${repoPath}/issues/${number}/comments \\
--method POST \\
-f body="<summary of what you did or your response>"${replyInstruction}
-F body=@/tmp/summary.md${replyInstruction}${suggestionSection}
${buildCustomInstructionsSection(commentActionInstructions)}
${buildCommentGuidelines(isPublic)}`;
}
109 changes: 109 additions & 0 deletions packages/github-bot/test/prompts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,74 @@ describe("buildCodeReviewPrompt", () => {
expect(prompt).toContain("repos/acme/widgets/pulls/42/comments");
});

it("encodes a nested-namespace owner as a single route segment in every API call", () => {
const prompt = buildCodeReviewPrompt({ ...baseParams, owner: "group/platform" });
// Use the shared repository-route contract even though GitHub webhook owners are currently
// single-segment logins.
expect(prompt).toContain("repos/group%2Fplatform/widgets/pulls/42/reviews");
expect(prompt).toContain("repos/group%2Fplatform/widgets/pulls/42/comments");
expect(prompt).toContain('-f commit_id="$(gh api repos/group%2Fplatform/widgets/pulls/42 --jq');
expect(prompt).not.toContain("repos/group/platform/widgets");
// The human-readable intro line is prose, not a route, and stays unencoded.
expect(prompt).toContain("Pull Request #42 in group/platform/widgets.");
});

it("teaches the applyable suggestion fence and its range anchors", () => {
const prompt = buildCodeReviewPrompt(baseParams);
// The whole point: a `suggestion` fence in a line-anchored comment body is what GitHub
// renders with a "Commit suggestion" button, and start_line/start_side anchors a range.
expect(prompt).toContain("## Applyable Suggestions");
expect(prompt).toContain("```suggestion");
expect(prompt).toContain("-F start_line=<first line>");
expect(prompt).toContain('-f start_side="RIGHT"');
});

it("passes every markdown body through safely-written files", () => {
const prompt = buildCodeReviewPrompt(baseParams);
expect(prompt).toContain("-F body=@/tmp/review.md");
expect(prompt).toContain("-F body=@/tmp/comment.md");
expect(prompt).not.toContain("-f body=");
expect(prompt).toContain("quoted heredoc");
expect(prompt).toContain("<<'EOF'");
expect(prompt).toContain("untrusted review text");
});

it("sends line numbers as typed integers", () => {
const prompt = buildCodeReviewPrompt(baseParams);
// `-f` is a raw string field, and the API rejects a string line with
// `For 'properties/line', "3" is not an integer` — verified live against the REST API.
expect(prompt).toContain("-F line=<line number>");
expect(prompt).not.toContain("-f line=<line number>");
expect(prompt).toContain('"3" is not an integer');
});

it("fences the suggestion contract against the ways an applied suggestion breaks code", () => {
const prompt = buildCodeReviewPrompt(baseParams);
// A suggestion is one click from merge, so each of these is load-bearing: the fence replaces
// the anchored lines verbatim, so a diff marker, an ellipsis, or lost indentation applies
// cleanly and corrupts the file.
expect(prompt).toContain("REPLACES the comment's anchored lines verbatim");
expect(prompt).toContain("Reproduce the original leading whitespace exactly");
expect(prompt).toContain("HTTP 422");
expect(prompt).toContain("post NO fence");
// And the agent must check rather than guess — it has the head branch checked out.
expect(prompt).toContain("sed -n 'START,ENDp' <path>");
});

it("requires every anchor — a lone line as much as a range endpoint — to sit inside a diff hunk", () => {
const prompt = buildCodeReviewPrompt(baseParams);
// Regression: this used to gate diff-hunk membership on start_line only, so a single-line
// `line` anchor outside the diff could still be suggested and would be rejected with 422.
expect(prompt).toContain("EVERY anchor line must fall inside a hunk of `gh pr diff`");
expect(prompt).toContain("single `line` of a");
expect(prompt).toContain("both `start_line` and `line`");
expect(prompt).toContain("including a lone `line`");
expect(prompt).toContain("not only range endpoints");
// Regression guard: the old wording scoped the requirement to range endpoints only.
expect(prompt).not.toContain("only when BOTH endpoints appear in a hunk");
expect(prompt).not.toContain("gh pr diff <path>");
});

it("limits self-reviews to comments", () => {
const prompt = buildCodeReviewPrompt({ ...baseParams, isSelfReview: true });
expect(prompt).toContain('-f event="COMMENT"');
Expand Down Expand Up @@ -189,9 +257,50 @@ describe("buildCommentActionPrompt", () => {
expect(prompt).not.toContain("reply to the specific review thread");
});

it("offers applyable suggestions only on the inherited thread anchor", () => {
const prompt = buildCommentActionPrompt({ ...baseParams, commentId: 999 });
expect(prompt).toContain("## Applyable Suggestions");
expect(prompt).toContain("```suggestion");
expect(prompt).toContain("-F body=@/tmp/reply.md");
expect(prompt).toContain("-F body=@/tmp/summary.md");
expect(prompt).toContain("inherits the parent comment's anchor");
expect(prompt).toContain("do not send `line`, `start_line`,");
expect(prompt).toContain("If any pushed");
expect(prompt).not.toContain("EVERY anchor line");
expect(prompt).not.toContain("-F start_line=");
// The summary lands on issues/{n}/comments, which has no line anchor, so a fence there is
// inert — the agent has to know which of its two posting paths can carry one.
expect(prompt).toContain("renders as an inert code block");
});

it("omits the suggestion contract when there is no thread to reply to", () => {
const prompt = buildCommentActionPrompt(baseParams);
expect(prompt).not.toContain("## Applyable Suggestions");
expect(prompt).not.toContain("```suggestion");
});

it("includes summary comment instruction with correct repo path", () => {
const prompt = buildCommentActionPrompt(baseParams);
expect(prompt).toContain("repos/acme/widgets/issues/42/comments");
expect(prompt).toContain("-F body=@/tmp/summary.md");
expect(prompt).not.toContain("-f body=");
expect(prompt).toContain("<<'EOF'");
});

it("encodes a nested-namespace owner as a single route segment in every API call", () => {
const prompt = buildCommentActionPrompt({
...baseParams,
owner: "group/platform",
filePath: "src/cache.ts",
diffHunk: "@@ -10,3 +10,5 @@\n+const cache = new Map();",
commentId: 999,
});
// Route construction stays aligned with the shared repository-identity contract.
expect(prompt).toContain("repos/group%2Fplatform/widgets/issues/42/comments");
expect(prompt).toContain("repos/group%2Fplatform/widgets/pulls/42/comments/999/replies");
expect(prompt).not.toContain("repos/group/platform/widgets");
// The human-readable intro line is prose, not a route, and stays unencoded.
expect(prompt).toContain("Pull Request #42 in group/platform/widgets.");
});

it("escapes embedded closing user_content tags in comment body", () => {
Expand Down