Skip to content
Merged
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
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ action set and always prefixed with a fixed disclaimer. Two transports: stdio

## Architecture

- **`src/github.ts`**: `GitHubReviewClient` class. The ONLY file that touches GitHub APIs. Uses GraphQL for resolved review-thread context and mutations, and REST for review summaries and general PR comments. Review-thread comments (GraphQL `reactionGroups`) and conversation comments (REST `reactions`) expose a normalized `reactions` map (content -> count, e.g. `THUMBS_DOWN`) so agents can see downvotes. Thread `isResolved`/`resolvedBy` are also returned. Safety boundary: comment/thread mutations exclude the `event` field, a post-creation tripwire verifies `PENDING` state (its error message tells the agent to stop and alert the human, since many MCP clients do not surface tool errors), and write operations are limited to the authenticated user's review. Submission is gated: `submitReview` (the only place `submitPullRequestReview` is called) refuses any action not in the `allowSubmit` set passed to the constructor, and always prefixes the review body with the fixed `submitBody` (an optional caller `additionalBody` is appended below it, never replacing it). Thread resolution is gated too: `resolveReviewThread` runs only when `allowResolve` is set AND the thread's first comment is authored by the authenticated user (it refuses others' threads), so a bot can tidy up its own now-fixed findings but not close anyone else's conversations. PR scoping is enforced here too: when the client is constructed with a `scope` (owner/repo/PR), every public method calls `assertInScope` and refuses input targeting a different PR/repo, and `resolveReviewThread` verifies the thread's PR matches the scope. This is the authoritative boundary. The server-side schema change is convenience on top of it.
- **`src/github.ts`**: `GitHubReviewClient` class. The ONLY file that touches GitHub APIs. Uses GraphQL for resolved review-thread context and mutations, and REST for review summaries and general PR comments. Review-thread comments (GraphQL `reactionGroups`) and conversation comments (REST `reactions`) expose a normalized `reactions` map (content -> count, e.g. `THUMBS_DOWN`) so agents can see downvotes. Thread `isResolved`/`resolvedBy` are also returned. Review comments, review summaries and conversation comments all carry their GitHub permalink as `url`, so an agent can link an earlier discussion (a thread's permalink is its first comment's `url`). The field is named `url` everywhere even though it comes from GraphQL `url` for review comments and REST `html_url` for the REST-sourced ones, since the output shape should be more consistent than the GitHub API is. A pending review comment already carries its final permalink, which starts resolving when the review is submitted, so an agent can cross-link its own findings while drafting. The `ReviewComment.url` doc comment and the pending tool descriptions say so, and the integration suite asserts that a comment's permalink survives submission unchanged. Safety boundary: comment/thread mutations exclude the `event` field, a post-creation tripwire verifies `PENDING` state (its error message tells the agent to stop and alert the human, since many MCP clients do not surface tool errors), and write operations are limited to the authenticated user's review. Submission is gated: `submitReview` (the only place `submitPullRequestReview` is called) refuses any action not in the `allowSubmit` set passed to the constructor, and always prefixes the review body with the fixed `submitBody` (an optional caller `additionalBody` is appended below it, never replacing it). Thread resolution is gated too: `resolveReviewThread` runs only when `allowResolve` is set AND the thread's first comment is authored by the authenticated user (it refuses others' threads), so a bot can tidy up its own now-fixed findings but not close anyone else's conversations. PR scoping is enforced here too: when the client is constructed with a `scope` (owner/repo/PR), every public method calls `assertInScope` and refuses input targeting a different PR/repo, and `resolveReviewThread` verifies the thread's PR matches the scope. This is the authoritative boundary. The server-side schema change is convenience on top of it.
- **`src/server.ts`**: `createMcpServer(client)` factory. Registers `get_pr_review_context`, `list_pending_review`, `add_review_comments`, `modify_review_comment`, and `delete_pending_review` with Zod schemas. Additionally registers `submit` **only when** `client.allowedSubmitActions` is non-empty (its `action` enum is restricted to that set), and `resolve_review_thread` **only when** `client.resolveEnabled` (started with `--allow-resolve`). When `client.scopedPullRequest` is set, the PR tools omit their `owner`/`repo`/`pull_number` arguments and act on the scoped PR implicitly. Shared by both transports.
- **`src/stdio.ts`**: Stdio transport entry point. Connects the MCP server to stdin/stdout for IDE-managed lifetime (Theia, VS Code).
- **`src/http.ts`**: HTTP transport. A plain `node:http` server exposing stateless Streamable HTTP at `/mcp` (POST only, GET/DELETE return 405, other paths 404). No web framework: the MCP transport parses the request body and enforces the Host header itself. Receives `{ port, host }` from the entry point and binds that address (default `127.0.0.1`). Validates the Host header of incoming requests (DNS rebinding protection): loopback aliases only for a loopback bind, plus the bind address and the container-runtime host names (`host.docker.internal`, `host.containers.internal`) for a non-loopback bind.
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ npm link # creates a global symlink to the binary

| Tool | Availability | Description |
| ----------------------- | ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `get_pr_review_context` | always | Get PR author/message, submitted review summaries, inline review threads with resolved state and reactions, and general PR comments |
| `list_pending_review` | always | List the authenticated user's pending draft review, including all current pending review comments |
| `get_pr_review_context` | always | Get PR author/message, submitted review summaries, inline review threads with resolved state, reactions and permalinks, and general PR comments |
| `list_pending_review` | always | List the authenticated user's pending draft review, including all current pending review comments and their permalinks |
| `add_review_comments` | always | Add one or more comments to the authenticated user's pending review, creating the pending review if needed |
| `modify_review_comment` | always | Update or delete one comment from the authenticated user's pending review |
| `delete_pending_review` | always | Delete the authenticated user's pending review and all its comments |
Expand Down
26 changes: 22 additions & 4 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ export interface ReviewSummary {
body: string;
submittedAt: string | null;
commitId: string | null;
htmlUrl: string | null;
/** Permalink to the submitted review. */
url: string | null;
authorAssociation: string;
}

Expand All @@ -165,6 +166,15 @@ export interface ReviewComment {
line: number | null;
createdAt: string;
updatedAt: string;
/**
* Permalink to the comment. GraphQL types this as non-null, but it stays
* nullable here so a response that omits it degrades to null instead of
* failing the whole call. A comment in a pending review already carries its
* final permalink, which starts resolving once the review is submitted. Since
* all comments of a review go live at the same moment, one pending comment can
* link another.
*/
url: string | null;
Comment thread
EclipseSourceAI marked this conversation as resolved.
/** Reaction content -> count, only for counts > 0 (e.g. THUMBS_DOWN). */
reactions?: Record<string, number>;
pullRequestReview?: {
Expand Down Expand Up @@ -200,7 +210,8 @@ export interface PullRequestConversationComment {
body: string;
createdAt: string;
updatedAt: string;
htmlUrl: string;
/** Permalink to the comment. */
url: string;
authorAssociation: string;
/** Reaction content -> count, only for counts > 0 (e.g. THUMBS_DOWN). */
reactions?: Record<string, number>;
Expand Down Expand Up @@ -404,6 +415,7 @@ export class GitHubReviewClient {
pageInfo { hasNextPage endCursor }
nodes {
id
url
body
path
line
Expand Down Expand Up @@ -520,6 +532,7 @@ export class GitHubReviewClient {
pageInfo { hasNextPage endCursor }
nodes {
id
url
body
path
line
Expand Down Expand Up @@ -585,7 +598,7 @@ export class GitHubReviewClient {
body: review.body ?? "",
submittedAt: review.submitted_at ?? null,
commitId: review.commit_id ?? null,
htmlUrl: review.html_url ?? null,
url: review.html_url ?? null,
authorAssociation: review.author_association,
});
}
Expand Down Expand Up @@ -622,7 +635,7 @@ export class GitHubReviewClient {
body: comment.body ?? "",
createdAt: comment.created_at,
updatedAt: comment.updated_at,
htmlUrl: comment.html_url,
url: comment.html_url,
authorAssociation: comment.author_association,
reactions: this.mapRestReactions((comment as any).reactions),
});
Expand Down Expand Up @@ -731,6 +744,7 @@ export class GitHubReviewClient {
pageInfo { hasNextPage endCursor }
nodes {
id
url

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path (plus the add/update mutations) now hands out a #discussion_r link for comments in a PENDING review, which nobody can open until the review is submitted. Document that on the field so an agent doesn't quote a draft link into a comment body, and a maintainer should confirm the real behaviour via the integration suite.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not hold up. The user of the review-guard can see them and once the review is submitted, everyone can. So it's safe to use.

body
path
line
Expand Down Expand Up @@ -865,6 +879,7 @@ export class GitHubReviewClient {
comments(first: 1) {
nodes {
id
url
body
path
line
Expand Down Expand Up @@ -963,6 +978,7 @@ export class GitHubReviewClient {
updatePullRequestReviewComment(input: $input) {
pullRequestReviewComment {
id
url
body
path
line
Expand Down Expand Up @@ -1014,6 +1030,7 @@ export class GitHubReviewClient {
}
pullRequestReviewComment {
id
url
body
path
line
Expand Down Expand Up @@ -1282,6 +1299,7 @@ export class GitHubReviewClient {
body: comment.body ?? "",
path: comment.path ?? "",
line: comment.line ?? null,
url: comment.url ?? null,
createdAt: comment.createdAt ?? "",
updatedAt: comment.updatedAt ?? "",
reactions: this.mapReactionGroups(comment),
Expand Down
12 changes: 11 additions & 1 deletion src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ export function createMcpServer(client: GitHubReviewClient): McpServer {
const scopeNote = scope
? ` This server is pinned to ${scope.owner}/${scope.repo}#${scope.pullNumber}. The PR is implicit (no owner/repo/pull_number arguments).`
: "";
// A pending comment's permalink is already its final one, so cross-linking
// findings inside a draft review works. Every tool that hands back pending
// comments says so, since the link does not resolve while drafting and an
// agent would otherwise assume it is broken.
const pendingPermalinkNote =
" Each pending comment already carries its final `url` permalink. It starts resolving once the review is submitted, and because all comments of a review go live together, one pending comment may link another.";
const target = (args: Record<string, unknown>): PullRequestInput =>
scope ?? {
owner: args.owner as string,
Expand All @@ -103,7 +109,8 @@ export function createMcpServer(client: GitHubReviewClient): McpServer {
// -- get_pr_review_context -------------------------------------------------
server.tool(
"get_pr_review_context",
"Get the pull request author/message plus all submitted PR discussion context: review summaries, inline review threads with resolved state, and general PR comments." +
"Get the pull request author/message plus all submitted PR discussion context: review summaries, inline review threads with resolved state and reactions, and general PR comments. " +
"Every review, review comment and PR comment carries its GitHub permalink as `url`, so you can link an earlier discussion when you refer to one (a thread's permalink is its first comment's `url`)." +
scopeNote,
prFields,
async (args) => {
Expand All @@ -121,6 +128,7 @@ export function createMcpServer(client: GitHubReviewClient): McpServer {
server.tool(
"list_pending_review",
"List the authenticated user's current pending (draft) review on a pull request, including all current pending review comments. Returns null if there is no pending review." +
pendingPermalinkNote +
scopeNote,
prFields,
async (args) => {
Expand All @@ -138,6 +146,7 @@ export function createMcpServer(client: GitHubReviewClient): McpServer {
server.tool(
"add_review_comments",
"Add one or more comments to the authenticated user's pending review. Creates the pending review if it does not already exist. The review is NOT submitted." +
pendingPermalinkNote +
scopeNote,
{ ...prFields, ...AddReviewCommentsFields },
async (args) => {
Expand All @@ -159,6 +168,7 @@ export function createMcpServer(client: GitHubReviewClient): McpServer {
server.tool(
"modify_review_comment",
"Update or delete one comment from the authenticated user's pending review. This cannot modify submitted review comments." +
pendingPermalinkNote +
scopeNote,
{ ...prFields, ...ModifyReviewCommentFields },
async (args) => {
Expand Down
154 changes: 154 additions & 0 deletions test/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,160 @@ describe("submitReview safety gate", () => {
});
});

describe("permalinks", () => {
function contextGql(commentNodes: unknown[]) {
return gqlBySubstring({
"reviewThreads(first: 100": {
repository: {
pullRequest: {
id: "PR_NODE",
number: 1,
title: "t",
body: "b",
author: { login: "alice" },
url: "https://github.com/octo/hello/pull/1",
state: "OPEN",
isDraft: false,
createdAt: "2026-01-01T00:00:00Z",
updatedAt: "2026-01-02T00:00:00Z",
baseRefName: "main",
headRefName: "feature",
reviewThreads: {
pageInfo: { hasNextPage: false, endCursor: null },
nodes: [
{
id: "T_1",
path: "a.ts",
line: 3,
diffSide: "RIGHT",
subjectType: "LINE",
isResolved: false,
isOutdated: false,
isCollapsed: false,
comments: {
totalCount: commentNodes.length,
pageInfo: { hasNextPage: false, endCursor: null },
nodes: commentNodes,
},
},
],
},
},
},
},
});
}

const commentNode = (id: string, url: string, reviewState = "COMMENTED") => ({
id,
url,
body: `body of ${id}`,
path: "a.ts",
line: 3,
createdAt: "2026-01-01T00:00:00Z",
updatedAt: "2026-01-01T00:00:00Z",
author: { login: "bot" },
pullRequestReview: {
id: "REV_1",
databaseId: 10,
state: reviewState,
author: { login: "bot" },
},
});
const submitted = (id: string, url: string) => commentNode(id, url);
const draft = (id: string, url: string) => commentNode(id, url, "PENDING");

const DISCUSSION_URL = "https://github.com/octo/hello/pull/1#discussion_r1";

it("exposes each thread comment's permalink", async () => {
Comment thread
EclipseSourceAI marked this conversation as resolved.
const { client } = makeClient(
{},
{
gql: contextGql([
submitted("C_1", DISCUSSION_URL),
submitted("C_2", "https://github.com/octo/hello/pull/1#discussion_r2"),
]),
},
);

const context = await client.getPullRequestReviewContext(PR);
expect(context.threads).toHaveLength(1);
expect(context.threads[0].comments.map((comment) => comment.url)).toEqual([
DISCUSSION_URL,
"https://github.com/octo/hello/pull/1#discussion_r2",
]);
});

// The pending path is the one an agent uses to quote its own drafts, and the
// `?? null` fallback in the mapper would let a dropped `url` selection
// degrade silently, so it gets its own assertion.
it("exposes each pending review comment's permalink", async () => {
const gql = gqlBySubstring({
"node(id: $reviewId)": {
node: {
id: "REV_1",
databaseId: 10,
state: "PENDING",
body: "",
createdAt: "2026-01-01T00:00:00Z",
author: { login: "bot" },
comments: {
totalCount: 1,
pageInfo: { hasNextPage: false, endCursor: null },
nodes: [draft("C_1", DISCUSSION_URL)],
},
},
},
});
const { client } = makeClient(
{},
{ gql, octokit: makeOctokitStub("bot", [PENDING_REVIEW_REST]) },
);

const review = await client.listPendingReview(PR);
expect(review?.comments.map((comment) => comment.url)).toEqual([DISCUSSION_URL]);
});

// The REST-sourced fields spell the permalink `html_url`, but the output shape
// normalizes every one of them to `url`.
it("exposes review summary and conversation comment permalinks as url", async () => {
const REVIEW_URL = "https://github.com/octo/hello/pull/1#pullrequestreview-10";
const PR_COMMENT_URL = "https://github.com/octo/hello/pull/1#issuecomment-20";
const octokit = makeOctokitStub("bot", [
{
state: "COMMENTED",
node_id: "REV_1",
id: 10,
user: { login: "alice" },
body: "looks good",
submitted_at: "2026-01-01T00:00:00Z",
commit_id: "abc123",
html_url: REVIEW_URL,
author_association: "MEMBER",
},
]);
octokit.issues.listComments.mockResolvedValue({
data: [
{
node_id: "IC_1",
id: 20,
user: { login: "alice" },
body: "ping",
created_at: "2026-01-01T00:00:00Z",
updated_at: "2026-01-01T00:00:00Z",
html_url: PR_COMMENT_URL,
author_association: "MEMBER",
},
],
});
const { client } = makeClient({}, { gql: contextGql([]), octokit });

const context = await client.getPullRequestReviewContext(PR);
expect(context.reviews.map((review) => review.url)).toEqual([REVIEW_URL]);
expect(context.prComments.map((comment) => comment.url)).toEqual([PR_COMMENT_URL]);
});
});

describe("deletePendingReview", () => {
it("reports an explicit deletion flag, since GitHub returns the deleted node still as PENDING", async () => {
const gql = gqlBySubstring({
Expand Down
Loading