Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 carry their GitHub permalink as `url` (GraphQL `PullRequestReviewComment.url`), review summaries and conversation comments as `htmlUrl`, so an agent can link an earlier discussion (a thread's permalink is its first comment's `url`). 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.
Comment thread
EclipseSourceAI marked this conversation as resolved.
Outdated
- **`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
8 changes: 8 additions & 0 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ export interface ReviewComment {
line: number | null;
createdAt: string;
updatedAt: string;
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 @@ -404,6 +405,7 @@ export class GitHubReviewClient {
pageInfo { hasNextPage endCursor }
nodes {
id
url
body
path
line
Expand Down Expand Up @@ -520,6 +522,7 @@ export class GitHubReviewClient {
pageInfo { hasNextPage endCursor }
nodes {
id
url
body
path
line
Expand Down Expand Up @@ -731,6 +734,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 +869,7 @@ export class GitHubReviewClient {
comments(first: 1) {
nodes {
id
url
body
path
line
Expand Down Expand Up @@ -963,6 +968,7 @@ export class GitHubReviewClient {
updatePullRequestReviewComment(input: $input) {
pullRequestReviewComment {
id
url
body
path
line
Expand Down Expand Up @@ -1014,6 +1020,7 @@ export class GitHubReviewClient {
}
pullRequestReviewComment {
id
url
body
path
line
Expand Down Expand Up @@ -1282,6 +1289,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
83 changes: 83 additions & 0 deletions test/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,89 @@ describe("submitReview safety gate", () => {
});
});

describe("getPullRequestReviewContext 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 submitted = (id: string, url: string) => ({
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: "COMMENTED",
author: { login: "bot" },
},
});

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",
]);
});
});

describe("deletePendingReview", () => {
it("reports an explicit deletion flag, since GitHub returns the deleted node still as PENDING", async () => {
const gql = gqlBySubstring({
Expand Down
1 change: 1 addition & 0 deletions test/integration/review-guard.itest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ describe("ReviewGuard against a real repository", () => {
expect(thread!.comments[0].author).toBe(reviewerLogin);
expect(thread!.isResolved).toBe(false);
expect(thread!.comments[0].reactions?.THUMBS_DOWN).toBe(1);
expect(thread!.comments[0].url).toContain(`/pull/${pullNumber}#discussion_r`);
return thread!.id;
});
});
Expand Down