From 92043cd846f5d7d6dab32e5006d571651d07f717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Mon, 21 Sep 2026 18:51:55 +0200 Subject: [PATCH 1/7] feat(github): support reactions and user installation discovery --- README.md | 13 + apps/web/content/docs/github.mdx | 13 + .../github/src/__tests__/reactions.test.ts | 258 ++++++++++++++++++ .../src/__tests__/user-installations.test.ts | 147 ++++++++++ packages/@emulators/github/src/entities.ts | 11 + packages/@emulators/github/src/helpers.ts | 61 ++++- packages/@emulators/github/src/index.ts | 2 + packages/@emulators/github/src/routes/apps.ts | 65 ++++- .../@emulators/github/src/routes/comments.ts | 6 + .../@emulators/github/src/routes/reactions.ts | 124 +++++++++ .../@emulators/github/src/routes/repos.ts | 1 + packages/@emulators/github/src/store.ts | 3 + packages/emulate/src/index.ts | 3 + skills/github/SKILL.md | 20 ++ 14 files changed, 721 insertions(+), 6 deletions(-) create mode 100644 packages/@emulators/github/src/__tests__/reactions.test.ts create mode 100644 packages/@emulators/github/src/__tests__/user-installations.test.ts create mode 100644 packages/@emulators/github/src/routes/reactions.ts diff --git a/README.md b/README.md index 079095795..fcae4c686 100644 --- a/README.md +++ b/README.md @@ -700,6 +700,8 @@ JWT authentication: sign a JWT with `{ iss: "" }` using the app's privat Installation access tokens act as the configured GitHub App bot for repository writes. Repository ownership, selected repository access, and requested App permissions remain enforced. Pull request merges require `contents: write` on the base repository. Pull request branch updates require `pull_requests: write` on the pull request repository and `contents: write` on the head repository. +User tokens can discover accessible installations with `GET /user/installations` and their repositories with `GET /user/installations/:installation_id/repositories`. Both endpoints paginate; discovery respects installation repository selection and explicit user access through ownership, collaboration, or organization membership. Public visibility alone does not grant discovery access. + Inspect secret-free metadata for minted installation tokens at `GET /_emulate/installation-tokens`. **App webhook delivery**: When events occur on repos where a GitHub App is installed, the emulator mirrors real GitHub behavior: @@ -904,6 +906,17 @@ Every endpoint below is fully stateful. Creates, updates, and deletes persist in - Commit comments: full CRUD on `/repos/:owner/:repo/commits/:sha/comments` - Repo-wide listings for each type +Issue responses and issue-comment webhooks include `pull_request` metadata when the issue represents a pull request. + +### Reactions + +- `GET/POST /repos/:owner/:repo/issues/:number/reactions` - list/create issue or pull-request body reactions +- `GET/POST /repos/:owner/:repo/issues/comments/:id/reactions` - list/create issue-comment reactions +- `GET/POST /repos/:owner/:repo/pulls/comments/:id/reactions` - list/create review-comment reactions +- `DELETE` on any of these paths with `/:reaction_id` - remove a reaction + +Lists support `content`, `page`, and `per_page`. Repeating a reaction by the same user returns the existing reaction; issue and comment summaries reflect stored reactions. Reaction changes do not emit webhooks. + ### Reviews - `GET /repos/:owner/:repo/pulls/:number/reviews` - list - `POST /repos/:owner/:repo/pulls/:number/reviews` - create (with inline comments) diff --git a/apps/web/content/docs/github.mdx b/apps/web/content/docs/github.mdx index ead306261..3300bda8c 100644 --- a/apps/web/content/docs/github.mdx +++ b/apps/web/content/docs/github.mdx @@ -42,6 +42,8 @@ Expiry is informational and does not change authorization behavior. `createEmula Installation access tokens act as the configured GitHub App bot for repository writes. Repository ownership, selected repository access, and requested App permissions remain enforced. Pull request merges require `contents: write` on the base repository. Pull request branch updates require `pull_requests: write` on the pull request repository and `contents: write` on the head repository. +User tokens can discover accessible installations with `GET /user/installations` and their repositories with `GET /user/installations/:installation_id/repositories`. Both endpoints paginate; discovery respects installation repository selection and explicit user access through ownership, collaboration, or organization membership. Public visibility alone does not grant discovery access. + The CLI can generate omitted keys when you request a private delivery file: ```bash @@ -142,6 +144,17 @@ Organization `members` are optional. Each entry references a seeded user by `log - Commit comments: full CRUD on `/repos/:owner/:repo/commits/:sha/comments` - Repo-wide listings for each type +Issue responses and issue-comment webhooks include `pull_request` metadata when the issue represents a pull request. + +## Reactions + +- `GET/POST /repos/:owner/:repo/issues/:number/reactions` - list/create issue or pull-request body reactions +- `GET/POST /repos/:owner/:repo/issues/comments/:id/reactions` - list/create issue-comment reactions +- `GET/POST /repos/:owner/:repo/pulls/comments/:id/reactions` - list/create review-comment reactions +- `DELETE` on any of these paths with `/:reaction_id` - remove a reaction + +Lists support `content`, `page`, and `per_page`. Repeating a reaction by the same user returns the existing reaction; issue and comment summaries reflect stored reactions. Reaction changes do not emit webhooks. + ## Reviews - `GET /repos/:owner/:repo/pulls/:number/reviews` - list diff --git a/packages/@emulators/github/src/__tests__/reactions.test.ts b/packages/@emulators/github/src/__tests__/reactions.test.ts new file mode 100644 index 000000000..cced429c2 --- /dev/null +++ b/packages/@emulators/github/src/__tests__/reactions.test.ts @@ -0,0 +1,258 @@ +import { createServer } from "node:http"; +import { once } from "node:events"; +import { describe, it, expect, beforeEach } from "vitest"; +import { + Hono, + Store, + WebhookDispatcher, + authMiddleware, + createApiErrorHandler, + createErrorHandler, + type AppEnv, + type TokenMap, +} from "@emulators/core"; +import { getGitHubStore, githubPlugin, seedFromConfig } from "../index.js"; + +const base = "http://localhost:4000"; +const repoPath = "/repos/octocat/hello-world"; + +function createTestApp() { + const store = new Store(); + const webhooks = new WebhookDispatcher(); + const tokenMap: TokenMap = new Map(); + const app = new Hono(); + app.onError(createApiErrorHandler()); + app.use("*", createErrorHandler()); + app.use("*", authMiddleware(tokenMap)); + githubPlugin.register(app, store, webhooks, base, tokenMap); + seedFromConfig(store, base, { + users: [{ login: "octocat" }, { login: "hubot" }], + repos: [ + { owner: "octocat", name: "hello-world", auto_init: true }, + { owner: "octocat", name: "private-repo", private: true }, + ], + }); + const gh = getGitHubStore(store); + for (const user of gh.users.all()) { + tokenMap.set(user.login, { login: user.login, id: user.id, scopes: ["repo", "delete_repo"] }); + } + return { app, gh, webhooks, tokenMap }; +} + +describe("GitHub reactions routes", () => { + let context: ReturnType; + let targets: Record; + + function request(path: string, method = "GET", body?: unknown, token: string | null = "octocat") { + return context.app.request(`${base}${path}`, { + method, + headers: { + ...(token ? { Authorization: `Bearer ${token}` } : {}), + ...(body === undefined ? {} : { "Content-Type": "application/json" }), + }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + } + + beforeEach(async () => { + context = createTestApp(); + const issue = await request(`${repoPath}/issues`, "POST", { title: "Issue" }); + expect(issue.status).toBe(201); + const issueBody = (await issue.json()) as { number: number }; + const branch = await request(`${repoPath}/branches/main`); + const { commit } = (await branch.json()) as { commit: { sha: string } }; + const ref = await request(`${repoPath}/git/refs`, "POST", { ref: "refs/heads/feature", sha: commit.sha }); + expect(ref.status).toBe(201); + const pull = await request(`${repoPath}/pulls`, "POST", { title: "Pull request", head: "feature", base: "main" }); + expect(pull.status).toBe(201); + const pullBody = (await pull.json()) as { number: number }; + const comment = await request(`${repoPath}/issues/${pullBody.number}/comments`, "POST", { body: "Discussion" }); + expect(comment.status).toBe(201); + const commentBody = (await comment.json()) as { id: number }; + const review = await request(`${repoPath}/pulls/${pullBody.number}/comments`, "POST", { + body: "Review", + path: "README.md", + line: 1, + side: "RIGHT", + commit_id: commit.sha, + }); + expect(review.status).toBe(201); + const reviewBody = (await review.json()) as { id: number }; + targets = { + issue: `${repoPath}/issues/${issueBody.number}`, + pull: `${repoPath}/issues/${pullBody.number}`, + issueComment: `${repoPath}/issues/comments/${commentBody.id}`, + reviewComment: `${repoPath}/pulls/comments/${reviewBody.id}`, + }; + }); + + it.each(["issue", "pull", "issueComment", "reviewComment"])( + "creates, lists, and deletes %s reactions", + async (target) => { + const path = `${targets[target]}/reactions`; + const created = await request(path, "POST", { content: "heart" }); + expect(created.status).toBe(201); + const reaction = (await created.json()) as { id: number }; + expect(reaction).toMatchObject({ + id: expect.any(Number), + node_id: expect.any(String), + user: { login: "octocat" }, + content: "heart", + created_at: expect.any(String), + }); + const duplicate = await request(path, "POST", { content: "heart" }); + expect(duplicate.status).toBe(200); + expect(await duplicate.json()).toEqual(reaction); + const otherUser = await request(path, "POST", { content: "heart" }, "hubot"); + expect(otherUser.status).toBe(201); + const otherReaction = await otherUser.json(); + const list = await request(path, "GET", undefined, null); + expect(list.status).toBe(200); + expect(await list.json()).toEqual([reaction, otherReaction]); + + if (target !== "pull") { + const subject = await request(targets[target]); + expect(await subject.json()).toMatchObject({ + reactions: { url: `${base}${path}`, total_count: 2, heart: 2, eyes: 0 }, + }); + } + const removed = await request(`${path}/${reaction.id}`, "DELETE"); + expect(removed.status).toBe(204); + expect(await removed.text()).toBe(""); + expect(await (await request(path)).json()).toEqual([otherReaction]); + if (target !== "pull") { + expect(await (await request(targets[target])).json()).toMatchObject({ + reactions: { total_count: 1, heart: 1 }, + }); + } + }, + ); + + it("filters before paginating and preserves the query in Link headers", async () => { + const path = `${targets.issueComment}/reactions`; + await request(path, "POST", { content: "heart" }); + await request(path, "POST", { content: "eyes" }); + await request(path, "POST", { content: "heart" }, "hubot"); + const first = await request(`${path}?content=heart&per_page=1`); + expect(await first.json()).toMatchObject([{ content: "heart", user: { login: "octocat" } }]); + expect(first.headers.get("link")).toContain("content=heart"); + expect(first.headers.get("link")).toContain('rel="next"'); + const second = await request(`${path}?content=heart&per_page=1&page=2`); + expect(await second.json()).toMatchObject([{ content: "heart", user: { login: "hubot" } }]); + expect(await (await request(`${path}?page=3&per_page=1&content=heart`)).json()).toEqual([]); + }); + + it("validates content and confines reactions to their subject, repository, and author", async () => { + const path = `${targets.issue}/reactions`; + expect((await request(path, "POST", { content: "invalid" })).status).toBe(422); + expect((await request(path, "POST", {})).status).toBe(422); + expect((await request(`${path}?content=invalid`)).status).toBe(422); + expect((await request(path, "POST", { content: "heart" }, null)).status).toBe(401); + expect((await request(`${repoPath}/issues/999/reactions`)).status).toBe(404); + expect( + (await request(targets.issueComment.replace("/issues/comments/", "/pulls/comments/") + "/reactions")).status, + ).toBe(404); + expect((await request(targets.issueComment.replace("hello-world", "private-repo") + "/reactions")).status).toBe( + 404, + ); + const created = await request(path, "POST", { content: "heart" }); + const reaction = (await created.json()) as { id: number }; + expect((await request(`${targets.pull}/reactions/${reaction.id}`, "DELETE")).status).toBe(404); + expect((await request(`${path}/${reaction.id}`, "DELETE", undefined, "hubot")).status).toBe(403); + expect(await (await request(`${targets.pull}/reactions`)).json()).toEqual([]); + expect(await (await request(path)).json()).toHaveLength(1); + }); + + it("enforces repository and installation permissions and attributes App reactions to the bot", async () => { + const privatePath = "/repos/octocat/private-repo/issues"; + await request(privatePath, "POST", { title: "Private issue" }); + const repo = context.gh.repos.findOneBy("full_name", "octocat/private-repo")!; + const user = context.gh.users.findOneBy("login", "octocat")!; + for (const [token, permissions, repositoryIds] of [ + ["reader", { issues: "read" }, [repo.id]], + ["writer", { issues: "write" }, [repo.id]], + ["wrong-permission", { pull_requests: "write" }, [repo.id]], + ["wrong-repo", { issues: "write" }, []], + ] as const) { + context.tokenMap.set(token, { + login: "app", + id: 42, + scopes: [], + installation: { + appId: 42, + installationId: 99, + accountId: user.id, + accountType: "User", + permissions: { ...permissions }, + repositoryIds: [...repositoryIds], + repositorySelection: "selected", + }, + }); + } + const path = `${privatePath}/1/reactions`; + expect((await request(path, "GET", undefined, "hubot")).status).toBe(403); + expect((await request(path, "GET", undefined, "reader")).status).toBe(200); + expect((await request(path, "POST", { content: "eyes" }, "reader")).status).toBe(403); + expect((await request(path, "POST", { content: "eyes" }, "wrong-permission")).status).toBe(403); + expect((await request(path, "POST", { content: "eyes" }, "wrong-repo")).status).toBe(403); + const created = await request(path, "POST", { content: "eyes" }, "writer"); + expect(created.status).toBe(201); + const reaction = (await created.json()) as { id: number }; + expect(reaction).toMatchObject({ user: { login: "app-42[bot]", type: "Bot" } }); + expect((await request(`${path}/${reaction.id}`, "DELETE", undefined, "reader")).status).toBe(403); + expect((await request(`${path}/${reaction.id}`, "DELETE", undefined, "writer")).status).toBe(204); + }); + + it("removes stored reactions when their comment or repository is deleted", async () => { + for (const target of Object.values(targets)) await request(`${target}/reactions`, "POST", { content: "+1" }); + expect(context.gh.reactions.all()).toHaveLength(4); + expect((await request(targets.issueComment, "DELETE")).status).toBe(204); + expect((await request(targets.reviewComment, "DELETE")).status).toBe(204); + expect(context.gh.reactions.all()).toHaveLength(2); + expect((await request(repoPath, "DELETE")).status).toBe(204); + expect(context.gh.reactions.all()).toHaveLength(0); + }); + + it("identifies pull-request issue comments in webhook payloads, including reaction summaries", async () => { + const payloads: Array<{ issue: unknown; comment: unknown }> = []; + const receiver = createServer(async (req, res) => { + const chunks: Buffer[] = []; + for await (const chunk of req) chunks.push(Buffer.from(chunk)); + payloads.push(JSON.parse(Buffer.concat(chunks).toString())); + res.writeHead(200).end(); + }); + receiver.listen(0, "127.0.0.1"); + await once(receiver, "listening"); + const address = receiver.address(); + if (!address || typeof address === "string") throw new Error("Missing webhook address"); + context.webhooks.register({ + url: `http://127.0.0.1:${address.port}`, + active: true, + events: ["*"], + owner: "octocat", + repo: "hello-world", + }); + try { + await request(`${targets.pull}/reactions`, "POST", { content: "heart" }); + const comment = await request(`${targets.pull}/comments`, "POST", { body: "Pull request discussion" }); + expect(comment.status).toBe(201); + const commentBody = (await comment.json()) as { id: number }; + await expect.poll(() => context.webhooks.getDeliveries().length).toBe(1); + expect(payloads).toHaveLength(1); + expect(payloads[0]).toMatchObject({ + issue: { + html_url: `${base}/octocat/hello-world/pull/2`, + pull_request: { url: `${base}${repoPath}/pulls/2`, html_url: `${base}/octocat/hello-world/pull/2` }, + reactions: { heart: 1, total_count: 1 }, + }, + comment: { html_url: `${base}/octocat/hello-world/pull/2#issuecomment-${commentBody.id}` }, + }); + await request(`${targets.issue}/comments`, "POST", { body: "Issue discussion" }); + await expect.poll(() => context.webhooks.getDeliveries().length).toBe(2); + expect(payloads[1].issue).not.toHaveProperty("pull_request"); + expect(payloads[1].issue).toMatchObject({ html_url: `${base}/octocat/hello-world/issues/1` }); + } finally { + await new Promise((resolve, reject) => receiver.close((error) => (error ? reject(error) : resolve()))); + } + }); +}); diff --git a/packages/@emulators/github/src/__tests__/user-installations.test.ts b/packages/@emulators/github/src/__tests__/user-installations.test.ts new file mode 100644 index 000000000..c54ed4677 --- /dev/null +++ b/packages/@emulators/github/src/__tests__/user-installations.test.ts @@ -0,0 +1,147 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { + Hono, + Store, + WebhookDispatcher, + authMiddleware, + createApiErrorHandler, + createErrorHandler, + type AppEnv, + type TokenMap, +} from "@emulators/core"; +import { githubPlugin, getGitHubStore, seedFromConfig } from "../index.js"; + +const base = "http://localhost:4000"; + +function createTestApp() { + const store = new Store(); + const webhooks = new WebhookDispatcher(); + const tokenMap: TokenMap = new Map(); + const app = new Hono(); + app.onError(createApiErrorHandler()); + app.use("*", createErrorHandler()); + app.use("*", authMiddleware(tokenMap)); + githubPlugin.register(app, store, webhooks, base, tokenMap); + seedFromConfig(store, base, { + users: [{ login: "octocat" }, { login: "bob" }, { login: "outsider" }], + orgs: [{ login: "acme", members: [{ login: "octocat" }] }], + repos: [ + { owner: "octocat", name: "owned", private: true }, + { owner: "acme", name: "selected", private: true }, + { owner: "acme", name: "unselected", private: true }, + { owner: "bob", name: "shared", private: true }, + { owner: "bob", name: "private", private: true }, + { owner: "bob", name: "public" }, + { owner: "outsider", name: "public" }, + ], + apps: [ + { + app_id: 100, + slug: "test-app", + name: "Test App", + private_key: "fake-key", + permissions: { contents: "read" }, + installations: [ + { installation_id: 1, account: "octocat" }, + { installation_id: 2, account: "acme", repository_selection: "selected", repositories: ["selected"] }, + { installation_id: 3, account: "bob" }, + { installation_id: 4, account: "outsider" }, + ], + }, + ], + }); + const gh = getGitHubStore(store); + for (const user of gh.users.all()) { + tokenMap.set(`${user.login}-token`, { login: user.login, id: user.id, scopes: ["repo", "user"] }); + } + tokenMap.set("installation-token", { + ...tokenMap.get("octocat-token")!, + installation: { + installationId: 1, + appId: 100, + accountId: gh.users.findOneBy("login", "octocat")!.id, + accountType: "User", + permissions: { contents: "read" }, + repositoryIds: [], + repositorySelection: "all", + }, + }); + return app; +} + +describe("user installation discovery", () => { + let app: Hono; + const get = (path: string, token = "octocat-token") => + app.request(`${base}${path}`, { headers: { Authorization: `Bearer ${token}` } }); + + beforeEach(async () => { + app = createTestApp(); + const response = await app.request(`${base}/repos/bob/shared/collaborators/octocat`, { + method: "PUT", + headers: { Authorization: "Bearer bob-token", "Content-Type": "application/json" }, + body: JSON.stringify({ permission: "pull" }), + }); + expect(response.status).toBe(201); + }); + + it("lists owned, organization, and collaborator installations with pagination", async () => { + const response = await get("/user/installations?per_page=2"); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + total_count: 3, + installations: [ + { id: 1, account: { login: "octocat" }, permissions: { contents: "read" } }, + { id: 2, account: { login: "acme" }, repository_selection: "selected" }, + ], + }); + expect(response.headers.get("Link")).toContain('page=2>; rel="next"'); + const next = await get("/user/installations?per_page=2&page=2"); + expect(await next.json()).toMatchObject({ total_count: 3, installations: [{ id: 3 }] }); + }); + + it("intersects selected repositories with the user's explicit access", async () => { + const response = await get("/user/installations/2/repositories"); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + total_count: 1, + repositories: [{ full_name: "acme/selected", private: true, permissions: { pull: true } }], + }); + + const collaborator = await get("/user/installations/3/repositories"); + expect(await collaborator.json()).toMatchObject({ + total_count: 1, + repositories: [{ full_name: "bob/shared", permissions: { admin: false, push: false, pull: true } }], + }); + }); + + it("paginates all-repository installations and includes newly created repositories", async () => { + const created = await app.request(`${base}/user/repos`, { + method: "POST", + headers: { Authorization: "Bearer octocat-token", "Content-Type": "application/json" }, + body: JSON.stringify({ name: "new-repo", private: true }), + }); + expect(created.status).toBe(201); + const response = await get("/user/installations/1/repositories?per_page=1"); + expect(await response.json()).toMatchObject({ + total_count: 2, + repositories: [{ full_name: "octocat/owned", permissions: { admin: true } }], + }); + expect(response.headers.get("Link")).toContain('page=2>; rel="next"'); + const next = await get("/user/installations/1/repositories?per_page=1&page=2"); + expect(await next.json()).toMatchObject({ total_count: 2, repositories: [{ full_name: "octocat/new-repo" }] }); + }); + + it("does not expose unknown installations or grant discovery through public visibility", async () => { + expect((await get("/user/installations/999/repositories")).status).toBe(404); + expect((await get("/user/installations/4/repositories")).status).toBe(404); + }); + + it.each(["/user/installations", "/user/installations/1/repositories"])( + "requires a user token at %s", + async (path) => { + expect((await app.request(`${base}${path}`)).status).toBe(401); + expect((await get(path, "invalid-token")).status).toBe(401); + expect((await get(path, "installation-token")).status).toBe(403); + }, + ); +}); diff --git a/packages/@emulators/github/src/entities.ts b/packages/@emulators/github/src/entities.ts index 545e5eb2f..45c220468 100644 --- a/packages/@emulators/github/src/entities.ts +++ b/packages/@emulators/github/src/entities.ts @@ -223,6 +223,17 @@ export interface GitHubReview extends Entity { submitted_at: string | null; } +export type GitHubReactionContent = "+1" | "-1" | "laugh" | "hooray" | "confused" | "heart" | "rocket" | "eyes"; + +export interface GitHubReaction extends Entity { + node_id: string; + repo_id: number; + subject_type: "issue" | "issue_comment" | "review_comment"; + subject_id: number; + user_id: number; + content: GitHubReactionContent; +} + export interface GitHubIssueEvent extends Entity { node_id: string; repo_id: number; diff --git a/packages/@emulators/github/src/helpers.ts b/packages/@emulators/github/src/helpers.ts index b88f20a67..bf6343589 100644 --- a/packages/@emulators/github/src/helpers.ts +++ b/packages/@emulators/github/src/helpers.ts @@ -15,6 +15,7 @@ import type { GitHubCheckRun, GitHubCheckSuite, GitHubReview, + GitHubReaction, GitHubWorkflow, GitHubWorkflowRun, GitHubJob, @@ -359,7 +360,17 @@ export function formatIssue(issue: GitHubIssue, store: GitHubStore, baseUrl: str labels_url: `${repoUrl}/issues/${issue.number}/labels{/name}`, comments_url: `${repoUrl}/issues/${issue.number}/comments`, events_url: `${repoUrl}/issues/${issue.number}/events`, - html_url: `${baseUrl}/${repo.full_name}/issues/${issue.number}`, + html_url: `${baseUrl}/${repo.full_name}/${issue.is_pull_request ? "pull" : "issues"}/${issue.number}`, + ...(issue.is_pull_request + ? { + pull_request: { + url: `${repoUrl}/pulls/${issue.number}`, + html_url: `${baseUrl}/${repo.full_name}/pull/${issue.number}`, + diff_url: `${baseUrl}/${repo.full_name}/pull/${issue.number}.diff`, + patch_url: `${baseUrl}/${repo.full_name}/pull/${issue.number}.patch`, + }, + } + : {}), id: issue.id, node_id: issue.node_id, number: issue.number, @@ -379,7 +390,7 @@ export function formatIssue(issue: GitHubIssue, store: GitHubStore, baseUrl: str closed_at: issue.closed_at, closed_by: closedBy ? formatUser(closedBy, baseUrl) : null, body: issue.body, - reactions: defaultReactions(`${repoUrl}/issues/${issue.number}`), + reactions: formatReactions(store, issue.repo_id, "issue", issue.id, `${repoUrl}/issues/${issue.number}`), timeline_url: `${repoUrl}/issues/${issue.number}/timeline`, performed_via_github_app: null, author_association: computeAuthorAssociation(store, issue.user_id, issue.repo_id), @@ -530,9 +541,10 @@ export function formatComment(comment: GitHubComment, store: GitHubStore, baseUr const repoUrl = `${baseUrl}/repos/${repo.full_name}`; if (comment.comment_type === "issue") { + const issue = store.issues.findBy("repo_id", repo.id).find((i) => i.number === comment.issue_number); return { url: `${repoUrl}/issues/comments/${comment.id}`, - html_url: `${baseUrl}/${repo.full_name}/issues/${comment.issue_number}#issuecomment-${comment.id}`, + html_url: `${baseUrl}/${repo.full_name}/${issue?.is_pull_request ? "pull" : "issues"}/${comment.issue_number}#issuecomment-${comment.id}`, issue_url: `${repoUrl}/issues/${comment.issue_number}`, id: comment.id, node_id: comment.node_id, @@ -541,7 +553,13 @@ export function formatComment(comment: GitHubComment, store: GitHubStore, baseUr updated_at: comment.updated_at, author_association: computeAuthorAssociation(store, comment.user_id, comment.repo_id), body: comment.body, - reactions: defaultReactions(`${repoUrl}/issues/comments/${comment.id}`), + reactions: formatReactions( + store, + repo.id, + "issue_comment", + comment.id, + `${repoUrl}/issues/comments/${comment.id}`, + ), performed_via_github_app: null, }; } @@ -565,7 +583,13 @@ export function formatComment(comment: GitHubComment, store: GitHubStore, baseUr created_at: comment.created_at, updated_at: comment.updated_at, author_association: computeAuthorAssociation(store, comment.user_id, comment.repo_id), - reactions: defaultReactions(`${repoUrl}/pulls/comments/${comment.id}`), + reactions: formatReactions( + store, + repo.id, + "review_comment", + comment.id, + `${repoUrl}/pulls/comments/${comment.id}`, + ), line: comment.line, side: comment.side ?? "RIGHT", subject_type: comment.subject_type ?? "line", @@ -734,6 +758,33 @@ function defaultReactions(url: string) { }; } +function formatReactions( + store: GitHubStore, + repoId: number, + subjectType: GitHubReaction["subject_type"], + subjectId: number, + url: string, +) { + const summary = defaultReactions(url); + for (const reaction of store.reactions.findBy("subject_id", subjectId)) { + if (reaction.repo_id !== repoId || reaction.subject_type !== subjectType) continue; + summary.total_count++; + summary[reaction.content]++; + } + return summary; +} + +export function formatReaction(reaction: GitHubReaction, store: GitHubStore, baseUrl: string) { + const user = store.users.get(reaction.user_id); + return { + id: reaction.id, + node_id: reaction.node_id, + user: user ? formatUser(user, baseUrl) : null, + content: reaction.content, + created_at: reaction.created_at, + }; +} + export function lookupRepo(store: GitHubStore, owner: string, repoName: string) { const fullName = `${owner}/${repoName}`; return store.repos.findOneBy("full_name", fullName); diff --git a/packages/@emulators/github/src/index.ts b/packages/@emulators/github/src/index.ts index 1ba16372f..f63dfa18d 100644 --- a/packages/@emulators/github/src/index.ts +++ b/packages/@emulators/github/src/index.ts @@ -19,6 +19,7 @@ import { issuesRoutes } from "./routes/issues.js"; import { pullsRoutes } from "./routes/pulls.js"; import { commentsRoutes } from "./routes/comments.js"; import { reviewsRoutes } from "./routes/reviews.js"; +import { reactionsRoutes } from "./routes/reactions.js"; import { labelsAndMilestonesRoutes } from "./routes/labels.js"; import { branchesAndGitRoutes } from "./routes/branches.js"; import { contentsRoutes } from "./routes/contents.js"; @@ -663,6 +664,7 @@ export const githubPlugin: ServicePlugin = { pullsRoutes(ctx); commentsRoutes(ctx); reviewsRoutes(ctx); + reactionsRoutes(ctx); labelsAndMilestonesRoutes(ctx); branchesAndGitRoutes(ctx); orgsAndTeamsRoutes(ctx); diff --git a/packages/@emulators/github/src/routes/apps.ts b/packages/@emulators/github/src/routes/apps.ts index a3c3f6783..d2f4073b5 100644 --- a/packages/@emulators/github/src/routes/apps.ts +++ b/packages/@emulators/github/src/routes/apps.ts @@ -1,11 +1,74 @@ import { randomBytes } from "crypto"; import type { RouteContext, AuthApp } from "@emulators/core"; +import { forbidden, notFound, parsePagination, setLinkHeader } from "@emulators/core"; import { getGitHubStore } from "../store.js"; -import { formatApp } from "../helpers.js"; +import { formatApp, formatRepo, generateNodeId } from "../helpers.js"; +import { assertAuthenticatedUser, isOrgMember } from "../route-helpers.js"; +import type { GitHubAppInstallation, GitHubRepo, GitHubUser } from "../entities.js"; export function appsRoutes({ app, store, baseUrl, tokenMap }: RouteContext): void { const gh = getGitHubStore(store); + function installationRepositories(installation: GitHubAppInstallation): GitHubRepo[] { + return gh.repos + .all() + .filter( + (repo) => + repo.owner_id === installation.account_id && + repo.owner_type === installation.account_type && + (installation.repository_selection === "all" || installation.repository_ids.includes(repo.id)), + ); + } + + function hasExplicitRepoAccess(user: GitHubUser, repo: GitHubRepo): boolean { + if (repo.owner_type === "User" && repo.owner_id === user.id) return true; + if (repo.owner_type === "Organization" && isOrgMember(gh, user.id, repo.owner_id)) return true; + return gh.collaborators.findBy("repo_id", repo.id).some((collaborator) => collaborator.user_id === user.id); + } + + function canAccessInstallation(user: GitHubUser, installation: GitHubAppInstallation): boolean { + if (installation.account_type === "User" && installation.account_id === user.id) return true; + if (installation.account_type === "Organization" && isOrgMember(gh, user.id, installation.account_id)) return true; + return installationRepositories(installation).some((repo) => hasExplicitRepoAccess(user, repo)); + } + + app.get("/user/installations", (c) => { + const authUser = c.get("authUser"); + if (authUser?.installation) throw forbidden(); + const user = assertAuthenticatedUser(gh, authUser); + const installations = gh.appInstallations.all().filter((installation) => canAccessInstallation(user, installation)); + const { page, per_page } = parsePagination(c); + setLinkHeader(c, installations.length, page, per_page); + const start = (page - 1) * per_page; + + return c.json({ + total_count: installations.length, + installations: installations.slice(start, start + per_page).map((installation) => { + const ghApp = gh.apps.findOneBy("app_id", installation.app_id); + return formatInstallation(installation, ghApp, baseUrl); + }), + }); + }); + + app.get("/user/installations/:installation_id/repositories", (c) => { + const authUser = c.get("authUser"); + if (authUser?.installation) throw forbidden(); + const user = assertAuthenticatedUser(gh, authUser); + const installationId = parseInt(c.req.param("installation_id"), 10); + const installation = gh.appInstallations.findOneBy("installation_id", installationId); + if (!installation || !canAccessInstallation(user, installation)) throw notFound(); + // Public visibility alone is not an explicit grant for user installation discovery. + const repositories = installationRepositories(installation).filter((repo) => hasExplicitRepoAccess(user, repo)); + const { page, per_page } = parsePagination(c); + setLinkHeader(c, repositories.length, page, per_page); + const start = (page - 1) * per_page; + + return c.json({ + total_count: repositories.length, + repositories: repositories.slice(start, start + per_page).map((repo) => formatRepo(repo, gh, baseUrl, user.id)), + }); + }); + function requireApp(c: any): AuthApp | null { const authApp = c.get("authApp") as AuthApp | undefined; if (!authApp) { diff --git a/packages/@emulators/github/src/routes/comments.ts b/packages/@emulators/github/src/routes/comments.ts index 607440519..39e1685b0 100644 --- a/packages/@emulators/github/src/routes/comments.ts +++ b/packages/@emulators/github/src/routes/comments.ts @@ -169,6 +169,9 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const ownerLogin = ownerLoginOf(gh, repo); gh.comments.delete(comment.id); + for (const reaction of gh.reactions.findBy("subject_id", comment.id)) { + if (reaction.subject_type === "issue_comment") gh.reactions.delete(reaction.id); + } if (issue) adjustIssueCommentCount(gh, issue, -1); webhooks.dispatch( @@ -296,6 +299,9 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const ownerLogin = ownerLoginOf(gh, repo); gh.comments.delete(comment.id); + for (const reaction of gh.reactions.findBy("subject_id", comment.id)) { + if (reaction.subject_type === "review_comment") gh.reactions.delete(reaction.id); + } if (pr) adjustPrReviewCommentCount(gh, pr, -1); webhooks.dispatch( diff --git a/packages/@emulators/github/src/routes/reactions.ts b/packages/@emulators/github/src/routes/reactions.ts new file mode 100644 index 000000000..2e03f2396 --- /dev/null +++ b/packages/@emulators/github/src/routes/reactions.ts @@ -0,0 +1,124 @@ +import type { Context, RouteContext } from "@emulators/core"; +import { ApiError, forbidden, parseJsonBody, parsePagination, setLinkHeader } from "@emulators/core"; +import type { GitHubReaction, GitHubReactionContent } from "../entities.js"; +import { formatReaction, generateNodeId, lookupRepo } from "../helpers.js"; +import { assertAuthenticatedActor, assertRepoPermission, notFoundResponse } from "../route-helpers.js"; +import { getGitHubStore } from "../store.js"; + +const reactionContents: GitHubReactionContent[] = [ + "+1", + "-1", + "laugh", + "hooray", + "confused", + "heart", + "rocket", + "eyes", +]; + +function parseContent(value: unknown): GitHubReactionContent { + if (typeof value !== "string" || !reactionContents.includes(value as GitHubReactionContent)) { + throw new ApiError(422, "Validation failed"); + } + return value as GitHubReactionContent; +} + +export function reactionsRoutes({ app, store, baseUrl }: RouteContext): void { + const gh = getGitHubStore(store); + const subjects: Array<{ + path: string; + type: GitHubReaction["subject_type"]; + permission: string; + }> = [ + { path: "issues/:issue_number", type: "issue", permission: "issues" }, + { path: "issues/comments/:comment_id", type: "issue_comment", permission: "issues" }, + { path: "pulls/comments/:comment_id", type: "review_comment", permission: "pull_requests" }, + ]; + + for (const subject of subjects) { + const path = `/repos/:owner/:repo/${subject.path}/reactions`; + + function resolveSubject(c: Context, required: "read" | "write") { + const repo = lookupRepo(gh, c.req.param("owner")!, c.req.param("repo")!); + if (!repo) throw notFoundResponse(); + assertRepoPermission(gh, c.get("authUser"), repo, subject.permission, required); + + let subjectId: number; + if (subject.type === "issue") { + const number = Number(c.req.param("issue_number")); + const issue = gh.issues.findBy("repo_id", repo.id).find((i) => i.number === number); + if (!issue) throw notFoundResponse(); + subjectId = issue.id; + } else { + const comment = gh.comments.get(Number(c.req.param("comment_id"))); + const commentType = subject.type === "issue_comment" ? "issue" : "review"; + if (!comment || comment.repo_id !== repo.id || comment.comment_type !== commentType) { + throw notFoundResponse(); + } + subjectId = comment.id; + } + + return { repoId: repo.id, subjectId }; + } + + app.get(path, (c) => { + const { repoId, subjectId } = resolveSubject(c, "read"); + const query = c.req.query("content"); + const content = query === undefined ? undefined : parseContent(query); + let reactions = gh.reactions + .findBy("subject_id", subjectId) + .filter((reaction) => reaction.repo_id === repoId && reaction.subject_type === subject.type); + if (content !== undefined) reactions = reactions.filter((reaction) => reaction.content === content); + reactions.sort((a, b) => a.id - b.id); + const { page, per_page } = parsePagination(c); + setLinkHeader(c, reactions.length, page, per_page); + const start = (page - 1) * per_page; + return c.json(reactions.slice(start, start + per_page).map((reaction) => formatReaction(reaction, gh, baseUrl))); + }); + + app.post(path, async (c) => { + const { repoId, subjectId } = resolveSubject(c, "write"); + const actor = assertAuthenticatedActor(gh, c.get("authUser")); + const raw = await parseJsonBody(c); + const content = parseContent(raw.content); + const existing = gh.reactions + .findBy("subject_id", subjectId) + .find( + (reaction) => + reaction.repo_id === repoId && + reaction.subject_type === subject.type && + reaction.user_id === actor.id && + reaction.content === content, + ); + if (existing) return c.json(formatReaction(existing, gh, baseUrl)); + + const reaction = gh.reactions.insert({ + node_id: "", + repo_id: repoId, + subject_type: subject.type, + subject_id: subjectId, + user_id: actor.id, + content, + }); + gh.reactions.update(reaction.id, { node_id: generateNodeId("Reaction", reaction.id) }); + return c.json(formatReaction(gh.reactions.get(reaction.id)!, gh, baseUrl), 201); + }); + + app.delete(`${path}/:reaction_id`, (c) => { + const { repoId, subjectId } = resolveSubject(c, "write"); + const actor = assertAuthenticatedActor(gh, c.get("authUser")); + const reaction = gh.reactions.get(Number(c.req.param("reaction_id"))); + if ( + !reaction || + reaction.repo_id !== repoId || + reaction.subject_type !== subject.type || + reaction.subject_id !== subjectId + ) { + throw notFoundResponse(); + } + if (reaction.user_id !== actor.id) throw forbidden(); + gh.reactions.delete(reaction.id); + return c.body(null, 204); + }); + } +} diff --git a/packages/@emulators/github/src/routes/repos.ts b/packages/@emulators/github/src/routes/repos.ts index 8692befe6..bc422de30 100644 --- a/packages/@emulators/github/src/routes/repos.ts +++ b/packages/@emulators/github/src/routes/repos.ts @@ -207,6 +207,7 @@ function deleteRepoCascade(gh: GitHubStore, repo: GitHubRepo) { delByRepo(gh.labels); delByRepo(gh.milestones); delByRepo(gh.comments); + delByRepo(gh.reactions); delByRepo(gh.reviews); delByRepo(gh.issueEvents); delByRepo(gh.branches); diff --git a/packages/@emulators/github/src/store.ts b/packages/@emulators/github/src/store.ts index a069391cf..535d49a4c 100644 --- a/packages/@emulators/github/src/store.ts +++ b/packages/@emulators/github/src/store.ts @@ -13,6 +13,7 @@ import type { GitHubMilestone, GitHubComment, GitHubReview, + GitHubReaction, GitHubIssueEvent, GitHubBranch, GitHubBranchProtection, @@ -52,6 +53,7 @@ export interface GitHubStore { milestones: Collection; comments: Collection; reviews: Collection; + reactions: Collection; issueEvents: Collection; branches: Collection; branchProtections: Collection; @@ -92,6 +94,7 @@ export function getGitHubStore(store: Store): GitHubStore { milestones: store.collection("github.milestones", ["repo_id", "number"]), comments: store.collection("github.comments", ["repo_id"]), reviews: store.collection("github.reviews", ["repo_id", "pull_number"]), + reactions: store.collection("github.reactions", ["repo_id", "subject_id"]), issueEvents: store.collection("github.issue_events", ["repo_id", "issue_number"]), branches: store.collection("github.branches", ["repo_id"]), branchProtections: store.collection("github.branch_protections", ["repo_id"]), diff --git a/packages/emulate/src/index.ts b/packages/emulate/src/index.ts index 909a38606..27c07a10c 100644 --- a/packages/emulate/src/index.ts +++ b/packages/emulate/src/index.ts @@ -63,6 +63,9 @@ GitHub API coverage: Checks retain the authenticated installation's App in API responses and webhooks. Suites are separated by App and commit; latest runs are grouped by App and check name. User-token checks return app: null; request-body app_id cannot change attribution. + User installation discovery respects explicit repository access and supports pagination. + Issue, pull-request body, and comment reactions support list/create/delete with derived counts. + Pull-request issue-comment webhooks include issue.pull_request metadata. Reactions emit no webhooks. Inspect minted installation-token metadata at GET /_emulate/installation-tokens. Linear API coverage: diff --git a/skills/github/SKILL.md b/skills/github/SKILL.md index 598f3eb58..b8b056886 100644 --- a/skills/github/SKILL.md +++ b/skills/github/SKILL.md @@ -368,6 +368,18 @@ curl http://localhost:4001/repos/octocat/hello-world/pulls/comments curl http://localhost:4001/repos/octocat/hello-world/comments ``` +Issue-comment webhooks include `issue.pull_request` metadata for pull requests, with `/pull/` HTML URLs. + +### Reactions + +List or create reactions with `GET/POST` on these paths: + +- `/repos/:owner/:repo/issues/:number/reactions` (issues and pull-request bodies) +- `/repos/:owner/:repo/issues/comments/:id/reactions` +- `/repos/:owner/:repo/pulls/comments/:id/reactions` + +Append `/:reaction_id` and use `DELETE` to remove a reaction. Create with `{ "content": "heart" }` or another GitHub reaction value. Lists support `content`, `page`, and `per_page`; duplicate reactions by the same user return the existing reaction. Issue and comment summaries track stored reactions. Reaction changes do not emit webhooks. + ### Reviews ```bash @@ -462,6 +474,12 @@ curl http://localhost:4001/app/installations \ curl http://localhost:4001/app/installations/100 \ -H "Authorization: Bearer " +# Discover installations and repositories accessible to a user +curl http://localhost:4001/user/installations \ + -H "Authorization: Bearer $TOKEN" +curl http://localhost:4001/user/installations/100/repositories \ + -H "Authorization: Bearer $TOKEN" + # Create installation access token (mints ghs_... token) curl -X POST http://localhost:4001/app/installations/100/access_tokens \ -H "Authorization: Bearer " \ @@ -474,6 +492,8 @@ curl http://localhost:4001/orgs/my-org/installation curl http://localhost:4001/users/octocat/installation ``` +User discovery endpoints paginate and respect installation repository selection plus explicit user access through ownership, collaboration, or organization membership. Public visibility alone does not grant discovery access. + App webhook delivery: when events occur, the emulator POSTs `event_callback` payloads to configured `webhook_url` with `X-GitHub-Event` and `X-Hub-Signature-256` headers. ### Releases From 78e9465702c92e110c8f5fa49d7f70fb2957223e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Tue, 22 Sep 2026 07:23:40 +0200 Subject: [PATCH 2/7] fix(github): synchronize pushed review comment locations --- README.md | 3 + apps/web/content/docs/github.mdx | 3 + .../github/src/__tests__/reactions.test.ts | 11 +- .../src/__tests__/review-comments.test.ts | 210 ++++++++++++++++++ .../github/src/pull-request-helpers.ts | 41 ++++ .../@emulators/github/src/routes/branches.ts | 2 + .../@emulators/github/src/routes/comments.ts | 72 ++++-- .../@emulators/github/src/routes/contents.ts | 3 + packages/emulate/src/index.ts | 2 + skills/github/SKILL.md | 2 + 10 files changed, 334 insertions(+), 15 deletions(-) create mode 100644 packages/@emulators/github/src/__tests__/review-comments.test.ts create mode 100644 packages/@emulators/github/src/pull-request-helpers.ts diff --git a/README.md b/README.md index fcae4c686..f0e3587c3 100644 --- a/README.md +++ b/README.md @@ -903,11 +903,14 @@ Every endpoint below is fully stateful. Creates, updates, and deletes persist in ### Comments - Issue comments: full CRUD on `/repos/:owner/:repo/issues/:number/comments` - Review comments: full CRUD on `/repos/:owner/:repo/pulls/:number/comments` +- Review replies: `POST /repos/:owner/:repo/pulls/:number/comments/:id/replies` - Commit comments: full CRUD on `/repos/:owner/:repo/commits/:sha/comments` - Repo-wide listings for each type Issue responses and issue-comment webhooks include `pull_request` metadata when the issue represents a pull request. +Review comments validate paths and lines against stored commit diffs. Comments on unpushed files or lines return `422` until those changes are pushed. Git refs and Contents API writes advance open pull-request heads and emit `pull_request.synchronize` webhooks. Review replies retain their parent's location. Legacy diff positions, binary diffs, and full multiline-range validation are not covered. + ### Reactions - `GET/POST /repos/:owner/:repo/issues/:number/reactions` - list/create issue or pull-request body reactions diff --git a/apps/web/content/docs/github.mdx b/apps/web/content/docs/github.mdx index 3300bda8c..7d86f90fc 100644 --- a/apps/web/content/docs/github.mdx +++ b/apps/web/content/docs/github.mdx @@ -141,11 +141,14 @@ Organization `members` are optional. Each entry references a seeded user by `log - Issue comments: full CRUD on `/repos/:owner/:repo/issues/:number/comments` - Review comments: full CRUD on `/repos/:owner/:repo/pulls/:number/comments` +- Review replies: `POST /repos/:owner/:repo/pulls/:number/comments/:id/replies` - Commit comments: full CRUD on `/repos/:owner/:repo/commits/:sha/comments` - Repo-wide listings for each type Issue responses and issue-comment webhooks include `pull_request` metadata when the issue represents a pull request. +Review comments validate paths and lines against stored commit diffs. Comments on unpushed files or lines return `422` until those changes are pushed. Git refs and Contents API writes advance open pull-request heads and emit `pull_request.synchronize` webhooks. Review replies retain their parent's location. Legacy diff positions, binary diffs, and full multiline-range validation are not covered. + ## Reactions - `GET/POST /repos/:owner/:repo/issues/:number/reactions` - list/create issue or pull-request body reactions diff --git a/packages/@emulators/github/src/__tests__/reactions.test.ts b/packages/@emulators/github/src/__tests__/reactions.test.ts index cced429c2..be1bc864a 100644 --- a/packages/@emulators/github/src/__tests__/reactions.test.ts +++ b/packages/@emulators/github/src/__tests__/reactions.test.ts @@ -63,6 +63,13 @@ describe("GitHub reactions routes", () => { const { commit } = (await branch.json()) as { commit: { sha: string } }; const ref = await request(`${repoPath}/git/refs`, "POST", { ref: "refs/heads/feature", sha: commit.sha }); expect(ref.status).toBe(201); + const changed = await request(`${repoPath}/contents/review.md`, "PUT", { + branch: "feature", + message: "Add reviewed file", + content: Buffer.from("Review this\n").toString("base64"), + }); + expect(changed.status).toBe(201); + const head = (await changed.json()) as { commit: { sha: string } }; const pull = await request(`${repoPath}/pulls`, "POST", { title: "Pull request", head: "feature", base: "main" }); expect(pull.status).toBe(201); const pullBody = (await pull.json()) as { number: number }; @@ -71,10 +78,10 @@ describe("GitHub reactions routes", () => { const commentBody = (await comment.json()) as { id: number }; const review = await request(`${repoPath}/pulls/${pullBody.number}/comments`, "POST", { body: "Review", - path: "README.md", + path: "review.md", line: 1, side: "RIGHT", - commit_id: commit.sha, + commit_id: head.commit.sha, }); expect(review.status).toBe(201); const reviewBody = (await review.json()) as { id: number }; diff --git a/packages/@emulators/github/src/__tests__/review-comments.test.ts b/packages/@emulators/github/src/__tests__/review-comments.test.ts new file mode 100644 index 000000000..221d163d7 --- /dev/null +++ b/packages/@emulators/github/src/__tests__/review-comments.test.ts @@ -0,0 +1,210 @@ +import { createServer } from "node:http"; +import { once } from "node:events"; +import { beforeEach, describe, expect, it } from "vitest"; +import { + Hono, + Store, + WebhookDispatcher, + authMiddleware, + createApiErrorHandler, + type AppEnv, + type TokenMap, +} from "@emulators/core"; +import { getGitHubStore, githubPlugin, seedFromConfig } from "../index.js"; + +const base = "http://localhost:4000"; +const repoPath = "/repos/octocat/hello-world"; + +describe("GitHub review comment locations", () => { + let app: Hono; + let webhooks: WebhookDispatcher; + let pull: { number: number; head: { sha: string } }; + + function request(path: string, method = "GET", body?: unknown) { + return app.request(`${base}${repoPath}${path}`, { + method, + headers: { Authorization: "Bearer test-token", "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + } + + beforeEach(async () => { + const store = new Store(); + webhooks = new WebhookDispatcher(); + const tokenMap: TokenMap = new Map(); + app = new Hono(); + app.onError(createApiErrorHandler()); + app.use("*", authMiddleware(tokenMap)); + githubPlugin.register(app, store, webhooks, base, tokenMap); + seedFromConfig(store, base, { + users: [{ login: "octocat" }], + repos: [{ owner: "octocat", name: "hello-world", auto_init: true }], + }); + const user = getGitHubStore(store).users.findOneBy("login", "octocat")!; + tokenMap.set("test-token", { id: user.id, login: user.login, scopes: ["repo"] }); + const branch = (await (await request("/branches/main")).json()) as { commit: { sha: string } }; + expect((await request("/git/refs", "POST", { ref: "refs/heads/feature", sha: branch.commit.sha })).status).toBe( + 201, + ); + expect( + ( + await request("/contents/scenario.md", "PUT", { + branch: "feature", + message: "Add scenario", + content: Buffer.from("First line\n").toString("base64"), + }) + ).status, + ).toBe(201); + const response = await request("/pulls", "POST", { title: "Review", head: "feature", base: "main" }); + expect(response.status).toBe(201); + pull = (await response.json()) as typeof pull; + }); + + async function pushChanges() { + const head = (await (await request(`/git/commits/${pull.head.sha}`)).json()) as { tree: { sha: string } }; + const tree = (await ( + await request("/git/trees", "POST", { + base_tree: head.tree.sha, + tree: [ + { path: "scenario.md", mode: "100644", type: "blob", content: "First line\nAdded line\n" }, + { path: "new.md", mode: "100644", type: "blob", content: "New file\n" }, + ], + }) + ).json()) as { sha: string }; + const commit = (await ( + await request("/git/commits", "POST", { + message: "Push the commented changes", + tree: tree.sha, + parents: [pull.head.sha], + }) + ).json()) as { sha: string }; + expect((await request("/git/refs/heads/feature", "PATCH", { sha: commit.sha })).status).toBe(200); + return commit.sha; + } + + it("rejects missing files and out-of-diff lines until their changes are pushed", async () => { + const create = (path: string, line: number, sha = pull.head.sha, extra = {}) => + request(`/pulls/${pull.number}/comments`, "POST", { + body: "Review this", + path, + line, + side: "RIGHT", + commit_id: sha, + ...extra, + }); + expect((await create("new.md", 1)).status).toBe(422); + expect((await create("scenario.md", 2)).status).toBe(422); + expect((await create("README.md", 1)).status).toBe(422); + expect((await create("scenario.md", 1, "missing-commit")).status).toBe(422); + expect((await create("scenario.md", 1)).status).toBe(201); + + const sha = await pushChanges(); + expect(await (await request(`/pulls/${pull.number}`)).json()).toMatchObject({ head: { sha } }); + expect((await create("new.md", 1, sha)).status).toBe(201); + expect((await create("scenario.md", 2, sha)).status).toBe(201); + expect((await create("scenario.md", 3, sha)).status).toBe(422); + expect((await create("scenario.md", 2, sha, { start_line: 3, start_side: "RIGHT" })).status).toBe(422); + expect((await create("scenario.md", 2)).status).toBe(422); + }); + + it("keeps review replies on their parent location and rejects missing or nested parents", async () => { + const response = await request(`/pulls/${pull.number}/comments`, "POST", { + body: "Parent", + path: "scenario.md", + line: 1, + side: "RIGHT", + commit_id: pull.head.sha, + }); + expect(response.status).toBe(201); + const parent = (await response.json()) as { id: number }; + const reply = await request(`/pulls/${pull.number}/comments/${parent.id}/replies`, "POST", { body: "Reply" }); + expect(reply.status).toBe(201); + const body = (await reply.json()) as { id: number }; + expect(body).toMatchObject({ + body: "Reply", + in_reply_to_id: parent.id, + path: "scenario.md", + line: 1, + side: "RIGHT", + commit_id: pull.head.sha, + }); + expect( + (await request(`/pulls/${pull.number}/comments`, "POST", { body: "Another reply", in_reply_to: parent.id })) + .status, + ).toBe(201); + expect((await request(`/pulls/${pull.number}/comments/999999/replies`, "POST", { body: "Missing" })).status).toBe( + 422, + ); + expect( + (await request(`/pulls/${pull.number}/comments/${body.id}/replies`, "POST", { body: "Nested" })).status, + ).toBe(422); + }); + + it("emits synchronize webhooks for ref and contents writes, but not unchanged or closed heads", async () => { + const receiver = createServer((_req, res) => res.writeHead(200).end()); + receiver.listen(0, "127.0.0.1"); + await once(receiver, "listening"); + const address = receiver.address(); + if (!address || typeof address === "string") throw new Error("Missing webhook address"); + webhooks.register({ + url: `http://127.0.0.1:${address.port}`, + active: true, + events: ["pull_request"], + owner: "octocat", + repo: "hello-world", + }); + const synchronizations = () => + webhooks.getDeliveries().filter((delivery) => delivery.action === "synchronize" && delivery.success); + try { + const sha = await pushChanges(); + await expect.poll(() => synchronizations().length).toBe(1); + expect(synchronizations()[0].payload).toMatchObject({ + action: "synchronize", + before: pull.head.sha, + after: sha, + number: pull.number, + pull_request: { head: { sha } }, + repository: { full_name: "octocat/hello-world" }, + sender: { login: "octocat" }, + }); + expect((await request("/git/refs/heads/feature", "PATCH", { sha })).status).toBe(200); + const added = await request("/contents/next.md", "PUT", { + branch: "feature", + message: "Add next", + content: Buffer.from("Next\n").toString("base64"), + }); + expect(added.status).toBe(201); + const addedBody = (await added.json()) as { content: { sha: string }; commit: { sha: string } }; + await expect.poll(() => synchronizations().length).toBe(2); + expect(synchronizations()[1].payload).toMatchObject({ before: sha, after: addedBody.commit.sha }); + const removed = await request("/contents/next.md", "DELETE", { + branch: "feature", + message: "Remove next", + sha: addedBody.content.sha, + }); + expect(removed.status).toBe(200); + const removedBody = (await removed.json()) as { commit: { sha: string } }; + await expect.poll(() => synchronizations().length).toBe(3); + expect(synchronizations()[2].payload).toMatchObject({ + before: addedBody.commit.sha, + after: removedBody.commit.sha, + }); + expect((await request(`/pulls/${pull.number}`, "PATCH", { state: "closed" })).status).toBe(200); + expect( + ( + await request("/contents/closed.md", "PUT", { + branch: "feature", + message: "Closed change", + content: Buffer.from("Closed\n").toString("base64"), + }) + ).status, + ).toBe(201); + expect(await (await request(`/pulls/${pull.number}`)).json()).toMatchObject({ + head: { sha: removedBody.commit.sha }, + }); + expect(synchronizations()).toHaveLength(3); + } finally { + await new Promise((resolve, reject) => receiver.close((error) => (error ? reject(error) : resolve()))); + } + }); +}); diff --git a/packages/@emulators/github/src/pull-request-helpers.ts b/packages/@emulators/github/src/pull-request-helpers.ts new file mode 100644 index 000000000..5ca262558 --- /dev/null +++ b/packages/@emulators/github/src/pull-request-helpers.ts @@ -0,0 +1,41 @@ +import type { WebhookDispatcher } from "@emulators/core"; +import type { GitHubRepo, GitHubUser } from "./entities.js"; +import type { GitHubStore } from "./store.js"; +import { formatPullRequest, formatRepo, formatUser } from "./helpers.js"; +import { ownerLoginOf } from "./route-helpers.js"; + +/** Advance open pull requests and notify their base repositories when a head branch changes. */ +export function synchronizePullRequestHeads( + gh: GitHubStore, + webhooks: WebhookDispatcher, + repo: GitHubRepo, + ref: string, + sha: string, + user: GitHubUser, + baseUrl: string, +): void { + if (!ref.startsWith("refs/heads/")) return; + const branch = ref.slice("refs/heads/".length); + for (const pr of gh.pullRequests.all()) { + if (pr.state !== "open" || pr.head_repo_id !== repo.id || pr.head_ref !== branch || pr.head_sha === sha) continue; + const baseRepo = gh.repos.get(pr.repo_id); + if (!baseRepo) continue; + const before = pr.head_sha; + const updated = gh.pullRequests.update(pr.id, { head_sha: sha })!; + webhooks.dispatch( + "pull_request", + "synchronize", + { + action: "synchronize", + before, + after: sha, + number: pr.number, + pull_request: formatPullRequest(updated, gh, baseUrl), + repository: formatRepo(baseRepo, gh, baseUrl), + sender: formatUser(user, baseUrl), + }, + ownerLoginOf(gh, baseRepo), + baseRepo.name, + ); + } +} diff --git a/packages/@emulators/github/src/routes/branches.ts b/packages/@emulators/github/src/routes/branches.ts index df3b3a519..64c404e25 100644 --- a/packages/@emulators/github/src/routes/branches.ts +++ b/packages/@emulators/github/src/routes/branches.ts @@ -1,6 +1,7 @@ import type { RouteContext } from "@emulators/core"; import { ApiError, parseJsonBody, parsePagination, setLinkHeader } from "@emulators/core"; import { getGitHubStore } from "../store.js"; +import { synchronizePullRequestHeads } from "../pull-request-helpers.js"; import type { GitHubStore } from "../store.js"; import type { GitHubBranch, @@ -804,6 +805,7 @@ export function branchesAndGitRoutes({ app, store, webhooks, baseUrl }: RouteCon } gh.refs.update(r.id, { sha: newSha }); syncBranchFromRef(gh, repo, fullRef, newSha); + synchronizePullRequestHeads(gh, webhooks, repo, fullRef, newSha, user, baseUrl); webhooks.dispatch( "push", undefined, diff --git a/packages/@emulators/github/src/routes/comments.ts b/packages/@emulators/github/src/routes/comments.ts index 39e1685b0..d92a22b0c 100644 --- a/packages/@emulators/github/src/routes/comments.ts +++ b/packages/@emulators/github/src/routes/comments.ts @@ -4,6 +4,7 @@ import { ApiError, parseJsonBody, parsePagination, setLinkHeader } from "@emulat import { getGitHubStore } from "../store.js"; import type { GitHubStore } from "../store.js"; import type { GitHubComment, GitHubCommit, GitHubIssue, GitHubPullRequest, GitHubRepo } from "../entities.js"; +import { blobText, diffText, findCommitBySha, flattenTree } from "../git-helpers.js"; import { formatComment, formatIssue, @@ -80,6 +81,45 @@ function adjustPrReviewCommentCount(gh: GitHubStore, pr: GitHubPullRequest, delt gh.pullRequests.update(pr.id, { review_comments: Math.max(0, pr.review_comments + delta) }); } +function reviewDiffContainsLine(patch: string | undefined, line: unknown, side: unknown): boolean { + if (typeof line !== "number" || !Number.isInteger(line) || line < 1) return false; + for (const hunk of (patch ?? "").matchAll(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/gm)) { + const offset = side === "LEFT" ? 1 : 3; + const start = Number(hunk[offset]); + const count = Number(hunk[offset + 1] ?? 1); + if (line >= start && line < start + count) return true; + } + return false; +} + +function validateReviewCommentLocation( + gh: GitHubStore, + pr: GitHubPullRequest, + commitSha: string, + path: string | null, + raw: Record, +): void { + const base = findCommitBySha(gh, pr.repo_id, pr.base_sha); + const head = findCommitBySha(gh, pr.head_repo_id, commitSha); + if (!base || !head || path === null) throw new ApiError(422, "Validation failed"); + const before = flattenTree(gh, pr.repo_id, base.tree_sha).blobs.get(path); + const after = flattenTree(gh, pr.head_repo_id, head.tree_sha).blobs.get(path); + if ((!before && !after) || before?.sha === after?.sha) { + throw new ApiError(422, "Path is not part of the pull request diff"); + } + if (raw.subject_type === "file") return; + const oldText = before ? blobText(gh, pr.repo_id, before.sha) : ""; + const newText = after ? blobText(gh, pr.head_repo_id, after.sha) : ""; + const patch = oldText !== null && newText !== null ? diffText(oldText, newText).patch : undefined; + if (raw.position !== undefined) return; + if ( + !reviewDiffContainsLine(patch, raw.line, raw.side) || + (raw.start_line !== undefined && !reviewDiffContainsLine(patch, raw.start_line, raw.start_side)) + ) { + throw new ApiError(422, "Line is not part of the pull request diff"); + } +} + export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): void { const gh = getGitHubStore(store); @@ -553,7 +593,7 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): return c.json(body); }); - app.post("/repos/:owner/:repo/pulls/:pull_number/comments", async (c) => { + const createReviewComment = async (c: Context) => { const owner = c.req.param("owner")!; const repoName = c.req.param("repo")!; const repo = lookupRepo(gh, owner, repoName); @@ -575,16 +615,18 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const commitSha = typeof raw.commit_id === "string" && raw.commit_id.trim() ? raw.commit_id.trim() : pr.head_sha; let inReplyTo: number | null = null; - if (raw.in_reply_to_id !== undefined && raw.in_reply_to_id !== null) { - const rid = - typeof raw.in_reply_to_id === "number" ? raw.in_reply_to_id : parseInt(String(raw.in_reply_to_id), 10); + let parent: GitHubComment | undefined; + const replyId = (c.req.param("comment_id") || undefined) ?? raw.in_reply_to ?? raw.in_reply_to_id; + if (replyId !== undefined && replyId !== null) { + const rid = typeof replyId === "number" ? replyId : parseInt(String(replyId), 10); if (!Number.isFinite(rid)) throw new ApiError(422, "Validation failed"); - const parent = gh.comments.get(rid); + parent = gh.comments.get(rid); if ( !parent || parent.repo_id !== repo.id || parent.comment_type !== "review" || - parent.pull_number !== pullNumber + parent.pull_number !== pullNumber || + parent.in_reply_to_id !== null ) { throw new ApiError(422, "Validation failed"); } @@ -617,20 +659,22 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): if (position !== null && !Number.isFinite(position)) throw new ApiError(422, "Validation failed"); if (line !== null && !Number.isFinite(line)) throw new ApiError(422, "Validation failed"); + if (inReplyTo === null) validateReviewCommentLocation(gh, pr, commitSha, pathVal, raw); + const row = gh.comments.insert({ node_id: "", repo_id: repo.id, issue_number: null, pull_number: pullNumber, - commit_sha: commitSha, + commit_sha: parent?.commit_sha ?? commitSha, body: raw.body, user_id: actor.id, in_reply_to_id: inReplyTo, - path: pathVal, - position: position !== null && Number.isFinite(position) ? position : null, - line: line !== null && Number.isFinite(line) ? line : null, - side, - subject_type: subjectType, + path: parent?.path ?? pathVal, + position: parent?.position ?? (position !== null && Number.isFinite(position) ? position : null), + line: parent?.line ?? (line !== null && Number.isFinite(line) ? line : null), + side: parent?.side ?? side, + subject_type: parent?.subject_type ?? subjectType, comment_type: "review", review_id: null, } as Omit); @@ -657,7 +701,9 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): ); return c.json(commentFmt, 201); - }); + }; + app.post("/repos/:owner/:repo/pulls/:pull_number/comments", createReviewComment); + app.post("/repos/:owner/:repo/pulls/:pull_number/comments/:comment_id/replies", createReviewComment); app.get("/repos/:owner/:repo/commits/:commit_sha/comments", (c) => { const owner = c.req.param("owner")!; diff --git a/packages/@emulators/github/src/routes/contents.ts b/packages/@emulators/github/src/routes/contents.ts index caa7a686f..5cab8e673 100644 --- a/packages/@emulators/github/src/routes/contents.ts +++ b/packages/@emulators/github/src/routes/contents.ts @@ -1,6 +1,7 @@ import type { AppEnv, Context, RouteContext } from "@emulators/core"; import { ApiError, parseJsonBody } from "@emulators/core"; import { getGitHubStore } from "../store.js"; +import { synchronizePullRequestHeads } from "../pull-request-helpers.js"; import type { GitHubStore } from "../store.js"; import type { GitHubBranch, GitHubCommit, GitHubRef, GitHubRepo, GitHubTree, GitHubUser } from "../entities.js"; import { formatRepo, formatUser, generateNodeId, lookupRepo, timestamp } from "../helpers.js"; @@ -711,6 +712,7 @@ export function contentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): repo.name, ); + synchronizePullRequestHeads(gh, webhooks, repo, `refs/heads/${branchName}`, commit.sha, user, baseUrl); const entry: FileTreeEntry = { mode: existing?.type === "blob" ? existing.mode : "100644", type: "blob", @@ -793,6 +795,7 @@ export function contentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): repo.name, ); + synchronizePullRequestHeads(gh, webhooks, repo, `refs/heads/${branchName}`, commit.sha, user, baseUrl); return c.json({ content: null, commit: formatGitCommit(repo, commit, baseUrl) }); }); } diff --git a/packages/emulate/src/index.ts b/packages/emulate/src/index.ts index 27c07a10c..49ae06569 100644 --- a/packages/emulate/src/index.ts +++ b/packages/emulate/src/index.ts @@ -66,6 +66,8 @@ GitHub API coverage: User installation discovery respects explicit repository access and supports pagination. Issue, pull-request body, and comment reactions support list/create/delete with derived counts. Pull-request issue-comment webhooks include issue.pull_request metadata. Reactions emit no webhooks. + Review comments validate diff paths/lines; replies keep their parent's location. + Git refs and Contents writes advance open PR heads and emit pull_request.synchronize. Inspect minted installation-token metadata at GET /_emulate/installation-tokens. Linear API coverage: diff --git a/skills/github/SKILL.md b/skills/github/SKILL.md index b8b056886..f337cdaa6 100644 --- a/skills/github/SKILL.md +++ b/skills/github/SKILL.md @@ -370,6 +370,8 @@ curl http://localhost:4001/repos/octocat/hello-world/comments Issue-comment webhooks include `issue.pull_request` metadata for pull requests, with `/pull/` HTML URLs. +Review comments validate paths and lines against stored commit diffs. Unpushed files or lines return `422` until pushed. Git refs and Contents API writes advance open pull-request heads and emit `pull_request.synchronize`. Use `POST /repos/:owner/:repo/pulls/:number/comments/:id/replies` with `{ "body": "Reply" }` to preserve the parent's thread and location. Legacy diff positions, binary diffs, and full multiline-range validation are not covered. + ### Reactions List or create reactions with `GET/POST` on these paths: From 30963ee495b6843b2477f39845ee5940a339428c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Tue, 22 Sep 2026 13:27:04 +0200 Subject: [PATCH 3/7] feat(github): support pending reviews and GraphQL collaboration --- README.md | 14 + apps/web/content/docs/github.mdx | 14 + packages/@emulators/github/README.md | 14 + packages/@emulators/github/package.json | 3 +- .../src/__tests__/review-comments.test.ts | 106 ++++++- packages/@emulators/github/src/entities.ts | 3 + packages/@emulators/github/src/index.ts | 2 + .../@emulators/github/src/routes/comments.ts | 63 +++-- .../@emulators/github/src/routes/graphql.ts | 261 ++++++++++++++++++ .../@emulators/github/src/routes/reviews.ts | 81 +++++- packages/emulate/src/index.ts | 3 + pnpm-lock.yaml | 3 + skills/github/SKILL.md | 15 + 13 files changed, 540 insertions(+), 42 deletions(-) create mode 100644 packages/@emulators/github/src/routes/graphql.ts diff --git a/README.md b/README.md index f0e3587c3..381ea0b1b 100644 --- a/README.md +++ b/README.md @@ -924,9 +924,23 @@ Lists support `content`, `page`, and `per_page`. Repeating a reaction by the sam - `GET /repos/:owner/:repo/pulls/:number/reviews` - list - `POST /repos/:owner/:repo/pulls/:number/reviews` - create (with inline comments) - `GET/PUT /repos/:owner/:repo/pulls/:number/reviews/:id` - get/update +- `DELETE /repos/:owner/:repo/pulls/:number/reviews/:id` - discard a pending review and its comments - `POST /repos/:owner/:repo/pulls/:number/reviews/:id/events` - submit - `PUT /repos/:owner/:repo/pulls/:number/reviews/:id/dismissals` - dismiss +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. + +### GraphQL collaboration + +The `POST /graphql` endpoint supports a collaboration subset backed by the same pull requests and comments as REST: + +- `repository.pullRequest` with paginated `reviewThreads`, comment identities, and resolution state +- `addPullRequestReviewThread` to add an inline comment to an existing pending review using its `pullRequestReviewId` +- `convertPullRequestToDraft` and `markPullRequestReadyForReview` +- `resolveReviewThread` and `unresolveReviewThread` + +Draft/ready transitions and public thread resolution changes emit the corresponding webhooks. This is not a complete GitHub GraphQL schema; diff-validation limitations also apply to GraphQL comments. + ### Labels & Milestones - Labels: full CRUD, add/remove from issues, replace all - Milestones: full CRUD, state transitions, issue counts diff --git a/apps/web/content/docs/github.mdx b/apps/web/content/docs/github.mdx index 7d86f90fc..baf0dbe11 100644 --- a/apps/web/content/docs/github.mdx +++ b/apps/web/content/docs/github.mdx @@ -163,9 +163,23 @@ Lists support `content`, `page`, and `per_page`. Repeating a reaction by the sam - `GET /repos/:owner/:repo/pulls/:number/reviews` - list - `POST /repos/:owner/:repo/pulls/:number/reviews` - create (with inline comments) - `GET/PUT /repos/:owner/:repo/pulls/:number/reviews/:id` - get/update +- `DELETE /repos/:owner/:repo/pulls/:number/reviews/:id` - discard a pending review and its comments - `POST /repos/:owner/:repo/pulls/:number/reviews/:id/events` - submit - `PUT /repos/:owner/:repo/pulls/:number/reviews/:id/dismissals` - dismiss +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. + +## GraphQL collaboration + +The `POST /graphql` endpoint supports a collaboration subset backed by the same pull requests and comments as REST: + +- `repository.pullRequest` with paginated `reviewThreads`, comment identities, and resolution state +- `addPullRequestReviewThread` to add an inline comment to an existing pending review using its `pullRequestReviewId` +- `convertPullRequestToDraft` and `markPullRequestReadyForReview` +- `resolveReviewThread` and `unresolveReviewThread` + +Draft/ready transitions and public thread resolution changes emit the corresponding webhooks. This is not a complete GitHub GraphQL schema; diff-validation limitations also apply to GraphQL comments. + ## Labels & Milestones - Labels: full CRUD, add/remove from issues, replace all diff --git a/packages/@emulators/github/README.md b/packages/@emulators/github/README.md index 9a8054e7c..8f67fe2dc 100644 --- a/packages/@emulators/github/README.md +++ b/packages/@emulators/github/README.md @@ -80,9 +80,23 @@ npm install @emulators/github - `GET /repos/:owner/:repo/pulls/:number/reviews` — list - `POST /repos/:owner/:repo/pulls/:number/reviews` — create (with inline comments) - `GET/PUT /repos/:owner/:repo/pulls/:number/reviews/:id` — get/update +- `DELETE /repos/:owner/:repo/pulls/:number/reviews/:id` — discard a pending review and its comments - `POST /repos/:owner/:repo/pulls/:number/reviews/:id/events` — submit - `PUT /repos/:owner/:repo/pulls/:number/reviews/:id/dismissals` — dismiss +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. + +### GraphQL collaboration + +The `POST /graphql` endpoint supports a collaboration subset backed by the same pull requests and comments as REST: + +- `repository.pullRequest` with paginated `reviewThreads`, comment identities, and resolution state +- `addPullRequestReviewThread` to add an inline comment to an existing pending review using its `pullRequestReviewId` +- `convertPullRequestToDraft` and `markPullRequestReadyForReview` +- `resolveReviewThread` and `unresolveReviewThread` + +Draft/ready transitions and public thread resolution changes emit the corresponding webhooks. This is not a complete GitHub GraphQL schema; diff-validation limitations also apply to GraphQL comments. + ### Labels & Milestones - Labels: full CRUD, add/remove from issues, replace all - Milestones: full CRUD, state transitions, issue counts diff --git a/packages/@emulators/github/package.json b/packages/@emulators/github/package.json index c2f95df65..2806435a1 100644 --- a/packages/@emulators/github/package.json +++ b/packages/@emulators/github/package.json @@ -38,7 +38,8 @@ "prepack": "node ../../../scripts/prepare-package.mjs" }, "dependencies": { - "@emulators/core": "workspace:*" + "@emulators/core": "workspace:*", + "graphql": "^17.0.0" }, "devDependencies": { "tsup": "^8", diff --git a/packages/@emulators/github/src/__tests__/review-comments.test.ts b/packages/@emulators/github/src/__tests__/review-comments.test.ts index 221d163d7..51aaebc5b 100644 --- a/packages/@emulators/github/src/__tests__/review-comments.test.ts +++ b/packages/@emulators/github/src/__tests__/review-comments.test.ts @@ -18,7 +18,7 @@ const repoPath = "/repos/octocat/hello-world"; describe("GitHub review comment locations", () => { let app: Hono; let webhooks: WebhookDispatcher; - let pull: { number: number; head: { sha: string } }; + let pull: { number: number; node_id: string; head: { sha: string } }; function request(path: string, method = "GET", body?: unknown) { return app.request(`${base}${repoPath}${path}`, { @@ -82,6 +82,110 @@ describe("GitHub review comment locations", () => { return commit.sha; } + it("validates a review's comments before creating it and permits submitted summary edits", async () => { + const input = { + body: "Summary", + event: "COMMENT", + comments: [{ path: "new.md", line: 1, side: "RIGHT", body: "Inline" }], + }; + expect((await request(`/pulls/${pull.number}/reviews`, "POST", input)).status).toBe(422); + expect(await (await request(`/pulls/${pull.number}/reviews`)).json()).toEqual([]); + await pushChanges(); + const response = await request(`/pulls/${pull.number}/reviews`, "POST", input); + expect(response.status).toBe(201); + const review = (await response.json()) as { id: number }; + expect(await (await request(`/pulls/${pull.number}/reviews/${review.id}/comments`)).json()).toMatchObject([ + { body: "Inline", line: 1 }, + ]); + const edited = await request(`/pulls/${pull.number}/reviews/${review.id}`, "PUT", { body: "Revised summary" }); + expect(edited.status).toBe(200); + expect(await edited.json()).toMatchObject({ body: "Revised summary", state: "COMMENTED" }); + }); + + it("adds draft comments incrementally before submission and deletes discarded pending reviews", async () => { + const reviewsPath = `/pulls/${pull.number}/reviews`; + const created = await request(reviewsPath, "POST", { body: "Draft" }); + expect(created.status).toBe(201); + const review = (await created.json()) as { id: number; node_id: string }; + expect((await request(reviewsPath, "POST", { body: "Another pending review" })).status).toBe(422); + async function addComment(path: string) { + const response = await app.request(`${base}/graphql`, { + method: "POST", + headers: { Authorization: "Bearer test-token", "Content-Type": "application/json" }, + body: JSON.stringify({ + query: + "mutation($input: AddPullRequestReviewThreadInput!) { addPullRequestReviewThread(input: $input) { thread { comments(first: 1) { nodes { databaseId body url } } } } }", + variables: { + input: { pullRequestReviewId: review.node_id, body: "Inline draft", path, line: 1, side: "RIGHT" }, + }, + }), + }); + return response.json() as Promise<{ data?: unknown; errors?: unknown[] }>; + } + expect((await addComment("new.md")).errors).toBeDefined(); + expect(await (await request(`${reviewsPath}/${review.id}/comments`)).json()).toEqual([]); + expect((await addComment("scenario.md")).errors).toBeUndefined(); + expect(await (await request(`${reviewsPath}/${review.id}`)).json()).toMatchObject({ state: "PENDING" }); + expect(await (await request(`${reviewsPath}/${review.id}/comments`)).json()).toMatchObject([ + { body: "Inline draft", pull_request_review_id: review.id }, + ]); + expect((await request(`${reviewsPath}/${review.id}/events`, "POST", { event: "COMMENT" })).status).toBe(200); + expect((await request(`${reviewsPath}/${review.id}`, "DELETE")).status).toBe(422); + expect((await addComment("scenario.md")).errors).toBeDefined(); + const discarded = await request(reviewsPath, "POST", { + comments: [{ path: "scenario.md", line: 1, side: "RIGHT", body: "Discard this" }], + }); + const draft = (await discarded.json()) as { id: number }; + expect((await request(`${reviewsPath}/${draft.id}`, "DELETE")).status).toBe(200); + expect((await request(`${reviewsPath}/${draft.id}/comments`)).status).toBe(404); + }); + + it("shares draft and thread resolution state between GraphQL and REST", async () => { + async function query(query: string, variables: Record) { + const response = await app.request(`${base}/graphql`, { + method: "POST", + headers: { Authorization: "Bearer test-token", "Content-Type": "application/json" }, + body: JSON.stringify({ query, variables }), + }); + expect(response.status).toBe(200); + const result = (await response.json()) as { data: Record; errors?: unknown[] }; + expect(result.errors).toBeUndefined(); + return result.data; + } + await query( + "mutation($id: ID!) { convertPullRequestToDraft(input: {pullRequestId: $id}) { pullRequest { isDraft } } }", + { id: pull.node_id }, + ); + expect(await (await request(`/pulls/${pull.number}`)).json()).toMatchObject({ draft: true }); + await query( + "mutation($id: ID!) { markPullRequestReadyForReview(input: {pullRequestId: $id}) { pullRequest { isDraft } } }", + { id: pull.node_id }, + ); + expect(await (await request(`/pulls/${pull.number}`)).json()).toMatchObject({ draft: false }); + const created = await request(`/pulls/${pull.number}/comments`, "POST", { + body: "Review this", + path: "scenario.md", + line: 1, + side: "RIGHT", + commit_id: pull.head.sha, + }); + const comment = (await created.json()) as { id: number }; + const data = await query( + 'query($number: Int!) { repository(owner: "octocat", name: "hello-world") { pullRequest(number: $number) { reviewThreads(first: 1) { nodes { id isResolved comments(first: 1) { nodes { databaseId } } } pageInfo { hasNextPage } } } } }', + { number: pull.number }, + ); + const thread = data.repository.pullRequest.reviewThreads.nodes[0]; + expect(thread).toMatchObject({ isResolved: false, comments: { nodes: [{ databaseId: comment.id }] } }); + for (const resolved of [true, false]) { + const mutation = resolved ? "resolveReviewThread" : "unresolveReviewThread"; + const result = await query( + `mutation($id: ID!) { ${mutation}(input: {threadId: $id}) { thread { id isResolved } } }`, + { id: thread.id }, + ); + expect(result[mutation].thread).toEqual({ id: thread.id, isResolved: resolved }); + } + }); + it("rejects missing files and out-of-diff lines until their changes are pushed", async () => { const create = (path: string, line: number, sha = pull.head.sha, extra = {}) => request(`/pulls/${pull.number}/comments`, "POST", { diff --git a/packages/@emulators/github/src/entities.ts b/packages/@emulators/github/src/entities.ts index 45c220468..db166f1d5 100644 --- a/packages/@emulators/github/src/entities.ts +++ b/packages/@emulators/github/src/entities.ts @@ -210,6 +210,9 @@ export interface GitHubComment extends Entity { comment_type: "issue" | "review" | "commit"; /** Set for line comments created as part of a pull request review. */ review_id: number | null; + /** Resolution of the inline thread rooted at this comment. Replies inherit their root's state. */ + resolved?: boolean; + resolved_by?: number | null; } export interface GitHubReview extends Entity { diff --git a/packages/@emulators/github/src/index.ts b/packages/@emulators/github/src/index.ts index f63dfa18d..0a1f5af5f 100644 --- a/packages/@emulators/github/src/index.ts +++ b/packages/@emulators/github/src/index.ts @@ -19,6 +19,7 @@ import { issuesRoutes } from "./routes/issues.js"; import { pullsRoutes } from "./routes/pulls.js"; import { commentsRoutes } from "./routes/comments.js"; import { reviewsRoutes } from "./routes/reviews.js"; +import { graphqlRoutes } from "./routes/graphql.js"; import { reactionsRoutes } from "./routes/reactions.js"; import { labelsAndMilestonesRoutes } from "./routes/labels.js"; import { branchesAndGitRoutes } from "./routes/branches.js"; @@ -664,6 +665,7 @@ export const githubPlugin: ServicePlugin = { pullsRoutes(ctx); commentsRoutes(ctx); reviewsRoutes(ctx); + graphqlRoutes(ctx); reactionsRoutes(ctx); labelsAndMilestonesRoutes(ctx); branchesAndGitRoutes(ctx); diff --git a/packages/@emulators/github/src/routes/comments.ts b/packages/@emulators/github/src/routes/comments.ts index d92a22b0c..83005675a 100644 --- a/packages/@emulators/github/src/routes/comments.ts +++ b/packages/@emulators/github/src/routes/comments.ts @@ -92,7 +92,7 @@ function reviewDiffContainsLine(patch: string | undefined, line: unknown, side: return false; } -function validateReviewCommentLocation( +export function validateReviewCommentLocation( gh: GitHubStore, pr: GitHubPullRequest, commitSha: string, @@ -303,19 +303,20 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const commentFmt = formatComment(comment, gh, baseUrl); if (!commentFmt) throw notFoundResponse(); - webhooks.dispatch( - "pull_request_review_comment", - "edited", - { - action: "edited", - comment: commentFmt, - pull_request: pr ? formatPullRequest(pr, gh, baseUrl) : null, - repository: formatRepo(repo, gh, baseUrl), - sender: formatUser(actor, baseUrl), - }, - ownerLogin, - repo.name, - ); + if (!comment.review_id || gh.reviews.get(comment.review_id)?.state !== "PENDING") + webhooks.dispatch( + "pull_request_review_comment", + "edited", + { + action: "edited", + comment: commentFmt, + pull_request: pr ? formatPullRequest(pr, gh, baseUrl) : null, + repository: formatRepo(repo, gh, baseUrl), + sender: formatUser(actor, baseUrl), + }, + ownerLogin, + repo.name, + ); return c.json(commentFmt); }); @@ -344,19 +345,20 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): } if (pr) adjustPrReviewCommentCount(gh, pr, -1); - webhooks.dispatch( - "pull_request_review_comment", - "deleted", - { - action: "deleted", - comment: commentFmt, - pull_request: pr ? formatPullRequest(pr, gh, baseUrl) : null, - repository: formatRepo(repo, gh, baseUrl), - sender: formatUser(actor, baseUrl), - }, - ownerLogin, - repo.name, - ); + if (!comment.review_id || gh.reviews.get(comment.review_id)?.state !== "PENDING") + webhooks.dispatch( + "pull_request_review_comment", + "deleted", + { + action: "deleted", + comment: commentFmt, + pull_request: pr ? formatPullRequest(pr, gh, baseUrl) : null, + repository: formatRepo(repo, gh, baseUrl), + sender: formatUser(actor, baseUrl), + }, + ownerLogin, + repo.name, + ); return c.body(null, 204); }); @@ -581,7 +583,12 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): let list = gh.comments .findBy("repo_id", repo.id) - .filter((x) => x.comment_type === "review" && x.pull_number === pullNumber); + .filter( + (x) => + x.comment_type === "review" && + x.pull_number === pullNumber && + (!x.review_id || gh.reviews.get(x.review_id)?.state !== "PENDING"), + ); list = sortComments(list, sort, direction); const total = list.length; setLinkHeader(c, total, page, per_page); diff --git a/packages/@emulators/github/src/routes/graphql.ts b/packages/@emulators/github/src/routes/graphql.ts new file mode 100644 index 000000000..26e8168d6 --- /dev/null +++ b/packages/@emulators/github/src/routes/graphql.ts @@ -0,0 +1,261 @@ +import { buildSchema, graphql } from "graphql"; +import type { RouteContext } from "@emulators/core"; +import { parseJsonBody } from "@emulators/core"; +import { getGitHubStore } from "../store.js"; +import type { GitHubComment, GitHubPullRequest } from "../entities.js"; +import { formatComment, formatPullRequest, formatRepo, formatUser, generateNodeId, lookupRepo } from "../helpers.js"; +import { + assertRepoPermission, + assertRepoWrite, + getActorUser, + notFoundResponse, + ownerLoginOf, +} from "../route-helpers.js"; +import { validateReviewCommentLocation } from "./comments.js"; + +const schema = buildSchema(` + type Query { repository(owner: String!, name: String!): Repository } + type Repository { pullRequest(number: Int!): PullRequest } + type PullRequest { + id: ID! + number: Int! + title: String! + body: String! + state: String! + isDraft: Boolean! + reviewThreads(first: Int!, after: String): ReviewThreadConnection! + } + type ReviewThreadConnection { nodes: [PullRequestReviewThread!]!, pageInfo: PageInfo! } + type PageInfo { hasNextPage: Boolean!, endCursor: String } + type User { databaseId: Int!, login: String!, avatarUrl: String! } + type PullRequestReviewThread { id: ID!, isResolved: Boolean!, resolvedBy: User, comments(first: Int!): CommentConnection! } + type CommentConnection { nodes: [PullRequestReviewComment!]! } + type PullRequestReviewComment { id: ID!, databaseId: Int!, body: String!, url: String! } + enum DiffSide { LEFT RIGHT } + enum PullRequestReviewThreadSubjectType { LINE FILE } + input AddPullRequestReviewThreadInput { + pullRequestReviewId: ID!, body: String!, path: String!, line: Int, side: DiffSide, + startLine: Int, startSide: DiffSide, subjectType: PullRequestReviewThreadSubjectType, clientMutationId: String + } + input ConvertPullRequestToDraftInput { pullRequestId: ID!, clientMutationId: String } + input MarkPullRequestReadyForReviewInput { pullRequestId: ID!, clientMutationId: String } + input ResolveReviewThreadInput { threadId: ID!, clientMutationId: String } + input UnresolveReviewThreadInput { threadId: ID!, clientMutationId: String } + type PullRequestPayload { pullRequest: PullRequest!, clientMutationId: String } + type ReviewThreadPayload { thread: PullRequestReviewThread!, clientMutationId: String } + type Mutation { + addPullRequestReviewThread(input: AddPullRequestReviewThreadInput!): ReviewThreadPayload! + convertPullRequestToDraft(input: ConvertPullRequestToDraftInput!): PullRequestPayload! + markPullRequestReadyForReview(input: MarkPullRequestReadyForReviewInput!): PullRequestPayload! + resolveReviewThread(input: ResolveReviewThreadInput!): ReviewThreadPayload! + unresolveReviewThread(input: UnresolveReviewThreadInput!): ReviewThreadPayload! + } +`); + +export function graphqlRoutes({ app, store, webhooks, baseUrl }: RouteContext): void { + const gh = getGitHubStore(store); + + function threadNode(root: GitHubComment) { + return { + id: generateNodeId("PullRequestReviewThread", root.id), + isResolved: root.resolved ?? false, + resolvedBy: root.resolved_by + ? (() => { + const user = gh.users.get(root.resolved_by); + return user + ? { databaseId: user.id, login: user.login, avatarUrl: formatUser(user, baseUrl).avatar_url } + : null; + })() + : null, + comments: ({ first }: { first: number }) => ({ + nodes: [ + root, + ...gh.comments.findBy("repo_id", root.repo_id).filter((comment) => comment.in_reply_to_id === root.id), + ] + .slice(0, first) + .map((comment) => ({ + id: comment.node_id, + databaseId: comment.id, + body: comment.body, + url: formatComment(comment, gh, baseUrl)?.html_url, + })), + }), + }; + } + + function pullNode(pr: GitHubPullRequest, viewerId?: number) { + return { + id: pr.node_id, + number: pr.number, + title: pr.title, + body: pr.body ?? "", + state: pr.merged ? "MERGED" : pr.state.toUpperCase(), + isDraft: pr.draft, + reviewThreads: ({ first, after }: { first: number; after?: string }) => { + if (first < 1 || first > 100) throw new Error("first must be between 1 and 100"); + const roots = gh.comments + .findBy("repo_id", pr.repo_id) + .filter( + (comment) => + comment.comment_type === "review" && + comment.pull_number === pr.number && + !comment.in_reply_to_id && + (!comment.review_id || + gh.reviews.get(comment.review_id)?.state !== "PENDING" || + comment.user_id === viewerId), + ) + .sort((a, b) => a.id - b.id); + const start = after ? roots.findIndex((comment) => String(comment.id) === after) + 1 : 0; + const page = roots.slice(start, start + first); + return { + nodes: page.map(threadNode), + pageInfo: { + hasNextPage: start + first < roots.length, + endCursor: page.length ? String(page[page.length - 1]!.id) : null, + }, + }; + }, + }; + } + + app.post("/graphql", async (c) => { + const raw = await parseJsonBody(c); + const auth = c.get("authUser"); + + function addThread(input: { + pullRequestReviewId: string; + body: string; + path: string; + line?: number; + side?: "LEFT" | "RIGHT"; + startLine?: number; + startSide?: "LEFT" | "RIGHT"; + subjectType?: "LINE" | "FILE"; + clientMutationId?: string; + }) { + const review = gh.reviews.all().find((candidate) => candidate.node_id === input.pullRequestReviewId); + if (!review) throw notFoundResponse(); + const repo = gh.repos.get(review.repo_id)!; + const actor = assertRepoWrite(gh, auth, repo, "pull_requests"); + if (review.user_id !== actor.id || review.state !== "PENDING") + throw new Error("Only the reviewer can add comments to a pending review"); + const pr = gh.pullRequests + .findBy("repo_id", repo.id) + .find((candidate) => candidate.number === review.pull_number); + if (!pr || pr.state !== "open") throw new Error("Pull request is not open"); + const subjectType = input.subjectType === "FILE" ? "file" : "line"; + validateReviewCommentLocation(gh, pr, pr.head_sha, input.path, { + line: input.line, + side: input.side ?? "RIGHT", + start_line: input.startLine, + start_side: input.startSide, + subject_type: subjectType, + }); + const row = gh.comments.insert({ + node_id: "", + repo_id: repo.id, + issue_number: null, + pull_number: pr.number, + commit_sha: pr.head_sha, + body: input.body, + user_id: actor.id, + in_reply_to_id: null, + path: input.path, + position: null, + line: input.line ?? null, + side: input.side ?? "RIGHT", + subject_type: subjectType, + comment_type: "review", + review_id: review.id, + } as Omit); + gh.comments.update(row.id, { node_id: generateNodeId("PullRequestReviewComment", row.id) }); + return { thread: threadNode(gh.comments.get(row.id)!), clientMutationId: input.clientMutationId }; + } + + function changeDraft(input: { pullRequestId: string; clientMutationId?: string }, draft: boolean) { + const pr = gh.pullRequests.all().find((candidate) => candidate.node_id === input.pullRequestId); + if (!pr) throw notFoundResponse(); + const repo = gh.repos.get(pr.repo_id)!; + const actor = assertRepoWrite(gh, auth, repo, "pull_requests"); + if (pr.state !== "open") throw new Error("Pull request is not open"); + if (pr.draft !== draft) { + gh.pullRequests.update(pr.id, { draft }); + const action = draft ? "converted_to_draft" : "ready_for_review"; + webhooks.dispatch( + "pull_request", + action, + { + action, + pull_request: formatPullRequest(gh.pullRequests.get(pr.id)!, gh, baseUrl), + repository: formatRepo(repo, gh, baseUrl), + sender: formatUser(actor, baseUrl), + }, + ownerLoginOf(gh, repo), + repo.name, + ); + } + return { pullRequest: pullNode(gh.pullRequests.get(pr.id)!), clientMutationId: input.clientMutationId }; + } + + function changeResolution(input: { threadId: string; clientMutationId?: string }, resolved: boolean) { + const root = gh.comments + .all() + .find( + (comment) => + comment.comment_type === "review" && + !comment.in_reply_to_id && + generateNodeId("PullRequestReviewThread", comment.id) === input.threadId, + ); + if (!root) throw notFoundResponse(); + const repo = gh.repos.get(root.repo_id)!; + const actor = assertRepoWrite(gh, auth, repo, "pull_requests"); + const pr = gh.pullRequests.findBy("repo_id", repo.id).find((candidate) => candidate.number === root.pull_number); + if (!pr) throw notFoundResponse(); + if ((root.resolved ?? false) !== resolved) { + gh.comments.update(root.id, { resolved, resolved_by: resolved ? actor.id : null }); + const action = resolved ? "resolved" : "unresolved"; + webhooks.dispatch( + "pull_request_review_thread", + action, + { + action, + thread: { id: root.id, node_id: input.threadId, comments: [formatComment(root, gh, baseUrl)] }, + pull_request: formatPullRequest(pr, gh, baseUrl), + repository: formatRepo(repo, gh, baseUrl), + sender: formatUser(actor, baseUrl), + }, + ownerLoginOf(gh, repo), + repo.name, + ); + } + return { thread: threadNode(gh.comments.get(root.id)!), clientMutationId: input.clientMutationId }; + } + + return c.json( + await graphql({ + schema, + source: typeof raw.query === "string" ? raw.query : "", + variableValues: raw.variables as Record | undefined, + operationName: typeof raw.operationName === "string" ? raw.operationName : undefined, + rootValue: { + repository: ({ owner, name }: { owner: string; name: string }) => { + const repo = lookupRepo(gh, owner, name); + if (!repo) return null; + assertRepoPermission(gh, auth, repo, "pull_requests"); + return { + pullRequest: ({ number }: { number: number }) => { + const pr = gh.pullRequests.findBy("repo_id", repo.id).find((candidate) => candidate.number === number); + return pr ? pullNode(pr, auth ? getActorUser(gh, auth)?.id : undefined) : null; + }, + }; + }, + convertPullRequestToDraft: ({ input }: { input: { pullRequestId: string } }) => changeDraft(input, true), + addPullRequestReviewThread: ({ input }: { input: Parameters[0] }) => addThread(input), + markPullRequestReadyForReview: ({ input }: { input: { pullRequestId: string } }) => changeDraft(input, false), + resolveReviewThread: ({ input }: { input: { threadId: string } }) => changeResolution(input, true), + unresolveReviewThread: ({ input }: { input: { threadId: string } }) => changeResolution(input, false), + }, + }), + ); + }); +} diff --git a/packages/@emulators/github/src/routes/reviews.ts b/packages/@emulators/github/src/routes/reviews.ts index 4a9a80573..b90a78a3e 100644 --- a/packages/@emulators/github/src/routes/reviews.ts +++ b/packages/@emulators/github/src/routes/reviews.ts @@ -15,7 +15,14 @@ import { lookupRepo, timestamp, } from "../helpers.js"; -import { assertRepoPermission, assertRepoWrite, notFoundResponse, ownerLoginOf } from "../route-helpers.js"; +import { + assertRepoPermission, + assertRepoWrite, + getActorUser, + notFoundResponse, + ownerLoginOf, +} from "../route-helpers.js"; +import { validateReviewCommentLocation } from "./comments.js"; function findPull(gh: GitHubStore, repoId: number, pullNumber: number): GitHubPullRequest | undefined { return gh.pullRequests.findBy("repo_id", repoId).find((p) => p.number === pullNumber); @@ -83,7 +90,7 @@ function dispatchReviewWebhook( pr: GitHubPullRequest, actor: GitHubUser, baseUrl: string, - action: "submitted" | "dismissed", + action: "submitted" | "dismissed" | "edited", ) { const ownerLogin = ownerLoginOf(gh, repo); const reviewFmt = formatReview(review, gh, baseUrl); @@ -120,7 +127,11 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): if (!pr) throw notFoundResponse(); const { page, per_page } = parsePagination(c); - const list = gh.reviews.findBy("repo_id", repo.id).filter((r) => r.pull_number === pullNumber); + const auth = c.get("authUser"); + const viewer = auth ? getActorUser(gh, auth) : undefined; + const list = gh.reviews + .findBy("repo_id", repo.id) + .filter((r) => r.pull_number === pullNumber && (r.state !== "PENDING" || r.user_id === viewer?.id)); list.sort((a, b) => a.id - b.id); const total = list.length; setLinkHeader(c, total, page, per_page); @@ -164,8 +175,27 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): typeof raw.commit_id === "string" && raw.commit_id.trim() ? raw.commit_id.trim() : pr.head_sha || generateSha(); const state: GitHubReview["state"] = event ? eventToState(event) : "PENDING"; + if ( + state === "PENDING" && + gh.reviews + .findBy("repo_id", repo.id) + .some( + (review) => review.pull_number === pullNumber && review.user_id === actor.id && review.state === "PENDING", + ) + ) + throw new ApiError(422, "A pending review already exists for this user"); const submittedAt = event ? timestamp() : null; + // Reject the entire review before storing anything if one inline comment is invalid. + const commentsArr = Array.isArray(raw.comments) ? raw.comments : []; + for (const entry of commentsArr) { + if (!entry || typeof entry !== "object") throw new ApiError(422, "Validation failed"); + const comment = entry as Record; + if (typeof comment.body !== "string" || typeof comment.path !== "string") + throw new ApiError(422, "Validation failed"); + validateReviewCommentLocation(gh, pr, commitId, comment.path, comment); + } + const row = gh.reviews.insert({ node_id: "", repo_id: repo.id, @@ -179,14 +209,13 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): gh.reviews.update(row.id, { node_id: generateNodeId("PullRequestReview", row.id) }); const review = gh.reviews.get(row.id)!; - const commentsArr = Array.isArray(raw.comments) ? raw.comments : []; for (const entry of commentsArr) { if (!entry || typeof entry !== "object") throw new ApiError(422, "Validation failed"); const o = entry as Record; if (typeof o.path !== "string" || !o.path.trim()) throw new ApiError(422, "Validation failed"); const pos = typeof o.position === "number" && Number.isFinite(o.position) ? o.position : parseInt(String(o.position), 10); - if (!Number.isFinite(pos)) throw new ApiError(422, "Validation failed"); + if (!Number.isFinite(pos) && typeof o.line !== "number") throw new ApiError(422, "Validation failed"); if (typeof o.body !== "string") throw new ApiError(422, "Validation failed"); const cRow = gh.comments.insert({ @@ -199,9 +228,9 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): user_id: actor.id, in_reply_to_id: null, path: o.path, - position: pos, - line: null, - side: "RIGHT", + position: Number.isFinite(pos) ? pos : null, + line: typeof o.line === "number" ? o.line : null, + side: o.side === "LEFT" ? "LEFT" : "RIGHT", subject_type: "line", comment_type: "review", review_id: review.id, @@ -232,19 +261,41 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const review = findReview(gh, repo, pullNumber, reviewId); if (!review) throw notFoundResponse(); + const auth = c.get("authUser"); + if (review.state === "PENDING" && (!auth || getActorUser(gh, auth)?.id !== review.user_id)) + throw notFoundResponse(); const json = formatReview(review, gh, baseUrl); if (!json) throw notFoundResponse(); return c.json(json); }); + app.delete("/repos/:owner/:repo/pulls/:pull_number/reviews/:review_id", (c) => { + const repo = lookupRepo(gh, c.req.param("owner")!, c.req.param("repo")!); + if (!repo) throw notFoundResponse(); + const actor = assertRepoWrite(gh, c.get("authUser"), repo, "pull_requests"); + const review = findReview(gh, repo, Number(c.req.param("pull_number")), Number(c.req.param("review_id"))); + if (!review) throw notFoundResponse(); + if (review.user_id !== actor.id) throw new ApiError(403, "Only the reviewer can discard this review"); + if (review.state !== "PENDING") throw new ApiError(422, "Only pending reviews can be deleted"); + const json = formatReview(review, gh, baseUrl); + for (const comment of gh.comments.findBy("review_id", review.id)) { + for (const reaction of gh.reactions.findBy("subject_id", comment.id)) { + if (reaction.subject_type === "review_comment") gh.reactions.delete(reaction.id); + } + gh.comments.delete(comment.id); + } + gh.reviews.delete(review.id); + return c.json(json); + }); + app.put("/repos/:owner/:repo/pulls/:pull_number/reviews/:review_id", async (c) => { const owner = c.req.param("owner")!; const repoName = c.req.param("repo")!; const repo = lookupRepo(gh, owner, repoName); if (!repo) throw notFoundResponse(); - assertRepoWrite(gh, c.get("authUser"), repo, "pull_requests"); + const actor = assertRepoWrite(gh, c.get("authUser"), repo, "pull_requests"); const pullNumber = parseInt(c.req.param("pull_number")!, 10); const reviewId = parseInt(c.req.param("review_id")!, 10); @@ -252,9 +303,7 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const existing = findReview(gh, repo, pullNumber, reviewId); if (!existing) throw notFoundResponse(); - if (existing.state !== "PENDING") { - throw new ApiError(422, "Validation failed"); - } + if (existing.user_id !== actor.id) throw new ApiError(403, "Only the reviewer can edit this review"); const raw = await parseJsonBody(c); if (typeof raw.body !== "string" && raw.body !== null) { @@ -265,6 +314,10 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const updated = gh.reviews.update(reviewId, { body: bodyVal }); if (!updated) throw notFoundResponse(); + const pr = findPull(gh, repo.id, pullNumber); + if (pr && updated.state !== "PENDING") + dispatchReviewWebhook(webhooks, gh, repo, updated, pr, actor, baseUrl, "edited"); + const json = formatReview(updated, gh, baseUrl); if (!json) throw notFoundResponse(); return c.json(json); @@ -290,6 +343,7 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): if (review.state !== "PENDING") { throw new ApiError(422, "Validation failed"); } + if (review.user_id !== actor.id) throw new ApiError(403, "Only the reviewer can submit this review"); const raw = await parseJsonBody(c); const event = parseSubmitEvent(raw.event); @@ -366,6 +420,9 @@ export function reviewsRoutes({ app, store, webhooks, baseUrl }: RouteContext): if (!review) throw notFoundResponse(); const { page, per_page } = parsePagination(c); + const auth = c.get("authUser"); + if (review.state === "PENDING" && (!auth || getActorUser(gh, auth)?.id !== review.user_id)) + throw notFoundResponse(); const { sort, direction } = parseCommentSort(c, "asc"); let list = gh.comments diff --git a/packages/emulate/src/index.ts b/packages/emulate/src/index.ts index 49ae06569..9c6378b41 100644 --- a/packages/emulate/src/index.ts +++ b/packages/emulate/src/index.ts @@ -68,6 +68,9 @@ GitHub API coverage: Pull-request issue-comment webhooks include issue.pull_request metadata. Reactions emit no webhooks. Review comments validate diff paths/lines; replies keep their parent's location. Git refs and Contents writes advance open PR heads and emit pull_request.synchronize. + Pending reviews support incremental GraphQL comments, submission, and discard; summaries are editable. + POST /graphql also supports paginated review threads, resolution, and draft/ready transitions. + GraphQL is a collaboration subset, not the complete GitHub schema. Inspect minted installation-token metadata at GET /_emulate/installation-tokens. Linear API coverage: diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f5c6456ad..eaacffbae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -643,6 +643,9 @@ importers: '@emulators/core': specifier: workspace:* version: link:../core + graphql: + specifier: ^17.0.0 + version: 17.0.0 devDependencies: tsup: specifier: ^8 diff --git a/skills/github/SKILL.md b/skills/github/SKILL.md index f337cdaa6..bed615d53 100644 --- a/skills/github/SKILL.md +++ b/skills/github/SKILL.md @@ -393,6 +393,21 @@ curl -X POST http://localhost:4001/repos/octocat/hello-world/pulls/1/reviews \ -d '{"event": "APPROVE", "body": "LGTM"}' ``` +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. + +Use `DELETE /repos/:owner/:repo/pulls/:number/reviews/:id` to discard a pending review and its comments. Submitted reviews cannot be deleted. + +### GraphQL collaboration + +The `POST /graphql` endpoint supports a collaboration subset backed by the same pull requests and comments as REST: + +- `repository.pullRequest` with paginated `reviewThreads`, comment identities, and resolution state +- `addPullRequestReviewThread` to add an inline comment to an existing pending review using its `pullRequestReviewId` +- `convertPullRequestToDraft` and `markPullRequestReadyForReview` +- `resolveReviewThread` and `unresolveReviewThread` + +Draft/ready transitions and public thread resolution changes emit the corresponding webhooks. This is not a complete GitHub GraphQL schema; diff-validation limitations also apply to GraphQL comments. + ### Labels & Milestones Full CRUD for labels and milestones. Add/remove labels from issues, replace all labels. List labels for a milestone. From d8b0cfba65e85bc1df1c3af563155d906b9ab1af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Thu, 24 Sep 2026 18:45:08 +0200 Subject: [PATCH 4/7] fix(github): protect draft comments and emit retarget events --- README.md | 4 +- apps/web/content/docs/github.mdx | 4 +- packages/@emulators/github/README.md | 4 +- .../src/__tests__/review-comments.test.ts | 96 ++++++++++++++++++- .../@emulators/github/src/routes/comments.ts | 14 ++- .../@emulators/github/src/routes/pulls.ts | 9 +- packages/emulate/src/index.ts | 2 + skills/github/SKILL.md | 4 +- 8 files changed, 127 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 381ea0b1b..e53e82275 100644 --- a/README.md +++ b/README.md @@ -900,6 +900,8 @@ Every endpoint below is fully stateful. Creates, updates, and deletes persist in - `POST/DELETE /repos/:owner/:repo/pulls/:number/requested_reviewers` - manage reviewers - `PUT /repos/:owner/:repo/pulls/:number/update-branch` - update branch +Updating only a pull request's `base` branch emits `pull_request.edited` with the new base ref and SHA. + ### Comments - Issue comments: full CRUD on `/repos/:owner/:repo/issues/:number/comments` - Review comments: full CRUD on `/repos/:owner/:repo/pulls/:number/comments` @@ -928,7 +930,7 @@ Lists support `content`, `page`, and `per_page`. Repeating a reaction by the sam - `POST /repos/:owner/:repo/pulls/:number/reviews/:id/events` - submit - `PUT /repos/:owner/:repo/pulls/:number/reviews/:id/dismissals` - dismiss -Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Individual comment reads and repository-wide comment listings also hide pending review comments from other users and anonymous readers. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. ### GraphQL collaboration diff --git a/apps/web/content/docs/github.mdx b/apps/web/content/docs/github.mdx index baf0dbe11..0c02e5289 100644 --- a/apps/web/content/docs/github.mdx +++ b/apps/web/content/docs/github.mdx @@ -137,6 +137,8 @@ Organization `members` are optional. Each entry references a seeded user by `log - `POST/DELETE /repos/:owner/:repo/pulls/:number/requested_reviewers` - manage reviewers - `PUT /repos/:owner/:repo/pulls/:number/update-branch` - update branch +Updating only a pull request's `base` branch emits `pull_request.edited` with the new base ref and SHA. + ## Comments - Issue comments: full CRUD on `/repos/:owner/:repo/issues/:number/comments` @@ -167,7 +169,7 @@ Lists support `content`, `page`, and `per_page`. Repeating a reaction by the sam - `POST /repos/:owner/:repo/pulls/:number/reviews/:id/events` - submit - `PUT /repos/:owner/:repo/pulls/:number/reviews/:id/dismissals` - dismiss -Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Individual comment reads and repository-wide comment listings also hide pending review comments from other users and anonymous readers. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. ## GraphQL collaboration diff --git a/packages/@emulators/github/README.md b/packages/@emulators/github/README.md index 8f67fe2dc..ffe200d8a 100644 --- a/packages/@emulators/github/README.md +++ b/packages/@emulators/github/README.md @@ -70,6 +70,8 @@ npm install @emulators/github - `POST/DELETE /repos/:owner/:repo/pulls/:number/requested_reviewers` — manage reviewers - `PUT /repos/:owner/:repo/pulls/:number/update-branch` — update branch +Updating only a pull request's `base` branch emits `pull_request.edited` with the new base ref and SHA. + ### Comments - Issue comments: full CRUD on `/repos/:owner/:repo/issues/:number/comments` - Review comments: full CRUD on `/repos/:owner/:repo/pulls/:number/comments` @@ -84,7 +86,7 @@ npm install @emulators/github - `POST /repos/:owner/:repo/pulls/:number/reviews/:id/events` — submit - `PUT /repos/:owner/:repo/pulls/:number/reviews/:id/dismissals` — dismiss -Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Individual comment reads and repository-wide comment listings also hide pending review comments from other users and anonymous readers. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. ### GraphQL collaboration diff --git a/packages/@emulators/github/src/__tests__/review-comments.test.ts b/packages/@emulators/github/src/__tests__/review-comments.test.ts index 51aaebc5b..53bfad2ec 100644 --- a/packages/@emulators/github/src/__tests__/review-comments.test.ts +++ b/packages/@emulators/github/src/__tests__/review-comments.test.ts @@ -20,10 +20,10 @@ describe("GitHub review comment locations", () => { let webhooks: WebhookDispatcher; let pull: { number: number; node_id: string; head: { sha: string } }; - function request(path: string, method = "GET", body?: unknown) { + function request(path: string, method = "GET", body?: unknown, token: string | null = "test-token") { return app.request(`${base}${repoPath}${path}`, { method, - headers: { Authorization: "Bearer test-token", "Content-Type": "application/json" }, + headers: { ...(token ? { Authorization: `Bearer ${token}` } : {}), "Content-Type": "application/json" }, body: body === undefined ? undefined : JSON.stringify(body), }); } @@ -37,11 +37,13 @@ describe("GitHub review comment locations", () => { app.use("*", authMiddleware(tokenMap)); githubPlugin.register(app, store, webhooks, base, tokenMap); seedFromConfig(store, base, { - users: [{ login: "octocat" }], + users: [{ login: "octocat" }, { login: "reader" }], repos: [{ owner: "octocat", name: "hello-world", auto_init: true }], }); const user = getGitHubStore(store).users.findOneBy("login", "octocat")!; tokenMap.set("test-token", { id: user.id, login: user.login, scopes: ["repo"] }); + const reader = getGitHubStore(store).users.findOneBy("login", "reader")!; + tokenMap.set("reader-token", { id: reader.id, login: reader.login, scopes: ["repo"] }); const branch = (await (await request("/branches/main")).json()) as { commit: { sha: string } }; expect((await request("/git/refs", "POST", { ref: "refs/heads/feature", sha: branch.commit.sha })).status).toBe( 201, @@ -140,6 +142,50 @@ describe("GitHub review comment locations", () => { expect((await request(`${reviewsPath}/${draft.id}/comments`)).status).toBe(404); }); + it("keeps pending review comments private on individual and repository-wide reads until submission", async () => { + const reviewsPath = `/pulls/${pull.number}/reviews`; + const created = await request(reviewsPath, "POST", { + body: "Draft", + comments: [{ path: "scenario.md", line: 1, side: "RIGHT", body: "Private draft" }], + }); + expect(created.status).toBe(201); + const review = (await created.json()) as { id: number }; + const [draft] = (await (await request(`${reviewsPath}/${review.id}/comments`)).json()) as { id: number }[]; + const published = await request(`/pulls/${pull.number}/comments`, "POST", { + body: "Published comment", + path: "scenario.md", + line: 1, + side: "RIGHT", + commit_id: pull.head.sha, + }); + expect(published.status).toBe(201); + const comment = (await published.json()) as { id: number }; + + expect(await (await request(`/pulls/comments/${draft.id}`)).json()).toMatchObject({ body: "Private draft" }); + expect(await (await request("/pulls/comments")).json()).toMatchObject([{ id: draft.id }, { id: comment.id }]); + for (const token of ["reader-token", null]) { + expect((await request(`/pulls/comments/${draft.id}`, "GET", undefined, token)).status).toBe(404); + const listed = await request("/pulls/comments?per_page=1", "GET", undefined, token); + expect(await listed.json()).toMatchObject([{ id: comment.id }]); + expect(listed.headers.get("Link")).toBeNull(); + expect(await (await request(`/pulls/${pull.number}/comments`, "GET", undefined, token)).json()).toMatchObject([ + { id: comment.id }, + ]); + expect((await request(`/pulls/comments/${comment.id}`, "GET", undefined, token)).status).toBe(200); + } + + expect((await request(`${reviewsPath}/${review.id}/events`, "POST", { event: "COMMENT" })).status).toBe(200); + for (const token of ["reader-token", null]) { + expect(await (await request(`/pulls/comments/${draft.id}`, "GET", undefined, token)).json()).toMatchObject({ + body: "Private draft", + }); + expect(await (await request("/pulls/comments", "GET", undefined, token)).json()).toMatchObject([ + { id: draft.id }, + { id: comment.id }, + ]); + } + }); + it("shares draft and thread resolution state between GraphQL and REST", async () => { async function query(query: string, variables: Record) { const response = await app.request(`${base}/graphql`, { @@ -244,6 +290,50 @@ describe("GitHub review comment locations", () => { ).toBe(422); }); + it("emits an edited webhook with the new base snapshot when only the target branch changes", async () => { + const main = (await (await request("/branches/main")).json()) as { commit: { sha: string } }; + expect((await request("/git/refs", "POST", { ref: "refs/heads/release", sha: main.commit.sha })).status).toBe(201); + const added = await request("/contents/release.md", "PUT", { + branch: "release", + message: "Prepare release", + content: Buffer.from("Release\n").toString("base64"), + }); + expect(added.status).toBe(201); + const release = (await added.json()) as { commit: { sha: string } }; + const receiver = createServer((_req, res) => res.writeHead(200).end()); + receiver.listen(0, "127.0.0.1"); + await once(receiver, "listening"); + const address = receiver.address(); + if (!address || typeof address === "string") throw new Error("Missing webhook address"); + webhooks.register({ + url: `http://127.0.0.1:${address.port}`, + active: true, + events: ["pull_request"], + owner: "octocat", + repo: "hello-world", + }); + try { + const updated = await request(`/pulls/${pull.number}`, "PATCH", { base: "release" }); + expect(updated.status).toBe(200); + const snapshot = { base: { ref: "release", sha: release.commit.sha }, head: { sha: pull.head.sha } }; + expect(await updated.json()).toMatchObject(snapshot); + expect(await (await request(`/pulls/${pull.number}`)).json()).toMatchObject(snapshot); + await expect.poll(() => webhooks.getDeliveries().filter((delivery) => delivery.success)).toHaveLength(1); + expect(webhooks.getDeliveries()[0]).toMatchObject({ + event: "pull_request", + action: "edited", + payload: { + action: "edited", + pull_request: { number: pull.number, ...snapshot }, + repository: { full_name: "octocat/hello-world" }, + sender: { login: "octocat" }, + }, + }); + } finally { + await new Promise((resolve, reject) => receiver.close((error) => (error ? reject(error) : resolve()))); + } + }); + it("emits synchronize webhooks for ref and contents writes, but not unchanged or closed heads", async () => { const receiver = createServer((_req, res) => res.writeHead(200).end()); receiver.listen(0, "127.0.0.1"); diff --git a/packages/@emulators/github/src/routes/comments.ts b/packages/@emulators/github/src/routes/comments.ts index 83005675a..b0d5eb711 100644 --- a/packages/@emulators/github/src/routes/comments.ts +++ b/packages/@emulators/github/src/routes/comments.ts @@ -18,6 +18,7 @@ import { assertRepoContentsRead, assertRepoPermission, assertRepoWrite, + getActorUser, notFoundResponse, ownerLoginOf, } from "../route-helpers.js"; @@ -273,6 +274,11 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const comment = getCommentForRepo(gh, repo, commentId, "review"); if (!comment) throw notFoundResponse(); + const review = comment.review_id ? gh.reviews.get(comment.review_id) : undefined; + const auth = c.get("authUser"); + if (review?.state === "PENDING" && (!auth || getActorUser(gh, auth)?.id !== review.user_id)) + throw notFoundResponse(); + const json = formatComment(comment, gh, baseUrl); if (!json) throw notFoundResponse(); return c.json(json); @@ -373,7 +379,13 @@ export function commentsRoutes({ app, store, webhooks, baseUrl }: RouteContext): const { page, per_page } = parsePagination(c); const { sort, direction } = parseCommentSort(c, "asc"); - let list = gh.comments.findBy("repo_id", repo.id).filter((x) => x.comment_type === "review"); + const auth = c.get("authUser"); + const viewer = auth ? getActorUser(gh, auth) : undefined; + let list = gh.comments.findBy("repo_id", repo.id).filter((comment) => { + if (comment.comment_type !== "review") return false; + const review = comment.review_id ? gh.reviews.get(comment.review_id) : undefined; + return review?.state !== "PENDING" || review.user_id === viewer?.id; + }); list = sortComments(list, sort, direction); const total = list.length; setLinkHeader(c, total, page, per_page); diff --git a/packages/@emulators/github/src/routes/pulls.ts b/packages/@emulators/github/src/routes/pulls.ts index faf94944d..c2f27a082 100644 --- a/packages/@emulators/github/src/routes/pulls.ts +++ b/packages/@emulators/github/src/routes/pulls.ts @@ -481,7 +481,7 @@ export function pullsRoutes({ app, store, webhooks, baseUrl }: RouteContext): vo return c.json(prFmt, 201); }); - app.get("/repos/:owner/:repo/pulls/:pull_number", (c) => { + app.get("/repos/:owner/:repo/pulls/:pull_number{[0-9]+}", (c) => { const owner = c.req.param("owner")!; const repoName = c.req.param("repo")!; const repo = lookupRepo(gh, owner, repoName); @@ -593,7 +593,12 @@ export function pullsRoutes({ app, store, webhooks, baseUrl }: RouteContext): vo ownerLogin, repo.name, ); - } else if (typeof body.title === "string" || typeof body.body === "string" || body.body === null) { + } else if ( + typeof body.title === "string" || + typeof body.body === "string" || + body.body === null || + (typeof body.base === "string" && body.base.trim()) + ) { webhooks.dispatch( "pull_request", "edited", diff --git a/packages/emulate/src/index.ts b/packages/emulate/src/index.ts index 9c6378b41..d4c23a1ab 100644 --- a/packages/emulate/src/index.ts +++ b/packages/emulate/src/index.ts @@ -68,7 +68,9 @@ GitHub API coverage: Pull-request issue-comment webhooks include issue.pull_request metadata. Reactions emit no webhooks. Review comments validate diff paths/lines; replies keep their parent's location. Git refs and Contents writes advance open PR heads and emit pull_request.synchronize. + Base-only pull-request retargets emit pull_request.edited with the new base ref and SHA. Pending reviews support incremental GraphQL comments, submission, and discard; summaries are editable. + Individual and repository-wide comment reads expose pending review comments only to their author. POST /graphql also supports paginated review threads, resolution, and draft/ready transitions. GraphQL is a collaboration subset, not the complete GitHub schema. Inspect minted installation-token metadata at GET /_emulate/installation-tokens. diff --git a/skills/github/SKILL.md b/skills/github/SKILL.md index bed615d53..04e94ca99 100644 --- a/skills/github/SKILL.md +++ b/skills/github/SKILL.md @@ -343,6 +343,8 @@ curl -X PUT http://localhost:4001/repos/octocat/hello-world/pulls/1/merge \ # Commits, files, requested reviewers, update branch ``` +Updating only a pull request's `base` branch emits `pull_request.edited` with the new base ref and SHA. + ### Comments ```bash @@ -393,7 +395,7 @@ curl -X POST http://localhost:4001/repos/octocat/hello-world/pulls/1/reviews \ -d '{"event": "APPROVE", "body": "LGTM"}' ``` -Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. +Omit `event` when creating a review to keep it pending. Each reviewer can have one pending review per pull request; review listings and review-specific reads expose it only to its author. Individual comment reads and repository-wide comment listings also hide pending review comments from other users and anonymous readers. Inline comments are validated before a review is stored. Pending edits emit no public webhooks, while submission and submitted-summary edits emit review events. Use `DELETE /repos/:owner/:repo/pulls/:number/reviews/:id` to discard a pending review and its comments. Submitted reviews cannot be deleted. From 3d7f18afe430c66249482af04e041e0d0c83c3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Tue, 6 Oct 2026 14:18:52 +0200 Subject: [PATCH 5/7] List repositories granted to installation tokens --- README.md | 2 + apps/web/content/docs/github.mdx | 2 + packages/@emulators/github/README.md | 2 + .../src/__tests__/user-installations.test.ts | 81 ++++++++++++++++++- .../@emulators/github/src/route-helpers.ts | 2 +- packages/@emulators/github/src/routes/apps.ts | 25 +++++- packages/emulate/src/index.ts | 1 + skills/github/SKILL.md | 4 + 8 files changed, 114 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e53e82275..2a6da2221 100644 --- a/README.md +++ b/README.md @@ -702,6 +702,8 @@ Installation access tokens act as the configured GitHub App bot for repository w User tokens can discover accessible installations with `GET /user/installations` and their repositories with `GET /user/installations/:installation_id/repositories`. Both endpoints paginate; discovery respects installation repository selection and explicit user access through ownership, collaboration, or organization membership. Public visibility alone does not grant discovery access. +Installation tokens can list their repositories with `GET /installation/repositories`. The endpoint paginates and intersects the token's selection with current installation grants. Removed or suspended installations cannot discover repositories. + Inspect secret-free metadata for minted installation tokens at `GET /_emulate/installation-tokens`. **App webhook delivery**: When events occur on repos where a GitHub App is installed, the emulator mirrors real GitHub behavior: diff --git a/apps/web/content/docs/github.mdx b/apps/web/content/docs/github.mdx index 0c02e5289..b41f9fb82 100644 --- a/apps/web/content/docs/github.mdx +++ b/apps/web/content/docs/github.mdx @@ -44,6 +44,8 @@ Installation access tokens act as the configured GitHub App bot for repository w User tokens can discover accessible installations with `GET /user/installations` and their repositories with `GET /user/installations/:installation_id/repositories`. Both endpoints paginate; discovery respects installation repository selection and explicit user access through ownership, collaboration, or organization membership. Public visibility alone does not grant discovery access. +Installation tokens can list their repositories with `GET /installation/repositories`. The endpoint paginates and intersects the token's selection with current installation grants. Removed or suspended installations cannot discover repositories. + The CLI can generate omitted keys when you request a private delivery file: ```bash diff --git a/packages/@emulators/github/README.md b/packages/@emulators/github/README.md index ffe200d8a..4d4443dc0 100644 --- a/packages/@emulators/github/README.md +++ b/packages/@emulators/github/README.md @@ -231,6 +231,8 @@ npx emulate start --service github --seed emulate.config.yaml \ The destination must not exist. emulate removes inherited ACLs, verifies effective owner-only access, and publishes complete JSON before opening listeners or configuring portless. Handled startup failures remove the invocation-owned artifact. A hard termination can leave a complete artifact that must be removed manually after confirming no invocation is using it. Explicit keys are excluded from the artifact. Linux requires `setfacl` and `getfacl` from the `acl` package. The flag fails closed when access controls cannot be verified and is not supported on Windows. Without `--generated-secrets-file`, CLI seed files continue requiring `private_key`. +Installation tokens can list repositories with `GET /installation/repositories`. Results paginate and intersect the token's selection with current installation grants. Removed or suspended installations cannot discover repositories. User-token discovery remains limited to explicit repository access. + ## Links - [Full documentation](https://emulate.dev/github) diff --git a/packages/@emulators/github/src/__tests__/user-installations.test.ts b/packages/@emulators/github/src/__tests__/user-installations.test.ts index c54ed4677..84588919d 100644 --- a/packages/@emulators/github/src/__tests__/user-installations.test.ts +++ b/packages/@emulators/github/src/__tests__/user-installations.test.ts @@ -66,16 +66,18 @@ function createTestApp() { repositorySelection: "all", }, }); - return app; + return { app, gh, tokenMap }; } -describe("user installation discovery", () => { +describe("installation discovery", () => { let app: Hono; + let gh: ReturnType; + let tokenMap: TokenMap; const get = (path: string, token = "octocat-token") => app.request(`${base}${path}`, { headers: { Authorization: `Bearer ${token}` } }); beforeEach(async () => { - app = createTestApp(); + ({ app, gh, tokenMap } = createTestApp()); const response = await app.request(`${base}/repos/bob/shared/collaborators/octocat`, { method: "PUT", headers: { Authorization: "Bearer bob-token", "Content-Type": "application/json" }, @@ -136,6 +138,79 @@ describe("user installation discovery", () => { expect((await get("/user/installations/4/repositories")).status).toBe(404); }); + it("lists installation-token repositories with pagination and includes new grants", async () => { + const created = await app.request(`${base}/user/repos`, { + method: "POST", + headers: { Authorization: "Bearer octocat-token", "Content-Type": "application/json" }, + body: JSON.stringify({ name: "new-repo", private: true }), + }); + expect(created.status).toBe(201); + + const response = await get("/installation/repositories?per_page=1", "installation-token"); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + total_count: 2, + repository_selection: "all", + repositories: [{ full_name: "octocat/owned", private: true }], + }); + expect(response.headers.get("Link")).toContain('page=2>; rel="next"'); + const next = await get("/installation/repositories?per_page=1&page=2", "installation-token"); + expect(await next.json()).toMatchObject({ + total_count: 2, + repositories: [{ full_name: "octocat/new-repo" }], + }); + }); + + it("intersects installation-token selections with current repository grants", async () => { + const installation = gh.appInstallations.findOneBy("installation_id", 3)!; + const shared = gh.repos.findOneBy("full_name", "bob/shared")!; + const privateRepo = gh.repos.findOneBy("full_name", "bob/private")!; + tokenMap.set("selected-token", { + login: "bob", + id: installation.account_id, + scopes: ["contents:read"], + installation: { + installationId: installation.installation_id, + appId: installation.app_id, + accountId: installation.account_id, + accountType: installation.account_type, + permissions: { contents: "read" }, + repositoryIds: [shared.id, privateRepo.id], + repositorySelection: "selected", + }, + }); + gh.appInstallations.update(installation.id, { + repository_selection: "selected", + repository_ids: [shared.id], + }); + + const response = await get("/installation/repositories", "selected-token"); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + total_count: 1, + repository_selection: "selected", + repositories: [{ full_name: "bob/shared" }], + }); + + gh.appInstallations.update(installation.id, { repository_ids: [] }); + expect(await (await get("/installation/repositories", "selected-token")).json()).toMatchObject({ + total_count: 0, + repositories: [], + }); + }); + + it("requires an active installation token for installation repository discovery", async () => { + expect((await app.request(`${base}/installation/repositories`)).status).toBe(401); + expect((await get("/installation/repositories")).status).toBe(401); + expect((await get("/installation/repositories", "invalid-token")).status).toBe(401); + + const installation = gh.appInstallations.findOneBy("installation_id", 1)!; + gh.appInstallations.update(installation.id, { suspended_at: new Date().toISOString() }); + expect((await get("/installation/repositories", "installation-token")).status).toBe(404); + gh.appInstallations.delete(installation.id); + expect((await get("/installation/repositories", "installation-token")).status).toBe(404); + }); + it.each(["/user/installations", "/user/installations/1/repositories"])( "requires a user token at %s", async (path) => { diff --git a/packages/@emulators/github/src/route-helpers.ts b/packages/@emulators/github/src/route-helpers.ts index 5a6f06e33..f2abbbeaa 100644 --- a/packages/@emulators/github/src/route-helpers.ts +++ b/packages/@emulators/github/src/route-helpers.ts @@ -26,7 +26,7 @@ export function getActorUser(gh: GitHubStore, authUser: AuthUser): GitHubUser | return gh.users.findOneBy("login", authUser.login); } -function installationCanAccessRepo(authUser: AuthUser, repo: GitHubRepo): boolean { +export function installationCanAccessRepo(authUser: AuthUser, repo: GitHubRepo): boolean { const installation = authUser.installation; if (!installation) return false; if (repo.owner_id !== installation.accountId || repo.owner_type !== installation.accountType) return false; diff --git a/packages/@emulators/github/src/routes/apps.ts b/packages/@emulators/github/src/routes/apps.ts index d2f4073b5..e4ecaa200 100644 --- a/packages/@emulators/github/src/routes/apps.ts +++ b/packages/@emulators/github/src/routes/apps.ts @@ -3,7 +3,7 @@ import type { RouteContext, AuthApp } from "@emulators/core"; import { forbidden, notFound, parsePagination, setLinkHeader } from "@emulators/core"; import { getGitHubStore } from "../store.js"; import { formatApp, formatRepo, generateNodeId } from "../helpers.js"; -import { assertAuthenticatedUser, isOrgMember } from "../route-helpers.js"; +import { assertAuthenticatedUser, installationCanAccessRepo, isOrgMember } from "../route-helpers.js"; import type { GitHubAppInstallation, GitHubRepo, GitHubUser } from "../entities.js"; export function appsRoutes({ app, store, baseUrl, tokenMap }: RouteContext): void { @@ -69,6 +69,29 @@ export function appsRoutes({ app, store, baseUrl, tokenMap }: RouteContext): voi }); }); + app.get("/installation/repositories", (c) => { + const authUser = c.get("authUser"); + const grant = authUser?.installation; + if (!grant) return c.json({ message: "Requires installation authentication" }, 401); + const installation = gh.appInstallations + .all() + .find((candidate) => candidate.installation_id === grant.installationId && candidate.app_id === grant.appId); + if (!installation || installation.suspended_at) throw notFound(); + // Intersect the token's selection with current grants, including repositories added after issuance. + const repositories = installationRepositories(installation).filter((repo) => + installationCanAccessRepo(authUser, repo), + ); + const { page, per_page } = parsePagination(c); + setLinkHeader(c, repositories.length, page, per_page); + const start = (page - 1) * per_page; + + return c.json({ + total_count: repositories.length, + repository_selection: grant.repositorySelection, + repositories: repositories.slice(start, start + per_page).map((repo) => formatRepo(repo, gh, baseUrl)), + }); + }); + function requireApp(c: any): AuthApp | null { const authApp = c.get("authApp") as AuthApp | undefined; if (!authApp) { diff --git a/packages/emulate/src/index.ts b/packages/emulate/src/index.ts index d4c23a1ab..dc215eb4b 100644 --- a/packages/emulate/src/index.ts +++ b/packages/emulate/src/index.ts @@ -64,6 +64,7 @@ GitHub API coverage: Suites are separated by App and commit; latest runs are grouped by App and check name. User-token checks return app: null; request-body app_id cannot change attribution. User installation discovery respects explicit repository access and supports pagination. + Installation tokens list paginated repositories within their current installation grants. Issue, pull-request body, and comment reactions support list/create/delete with derived counts. Pull-request issue-comment webhooks include issue.pull_request metadata. Reactions emit no webhooks. Review comments validate diff paths/lines; replies keep their parent's location. diff --git a/skills/github/SKILL.md b/skills/github/SKILL.md index 04e94ca99..a9bca234d 100644 --- a/skills/github/SKILL.md +++ b/skills/github/SKILL.md @@ -505,6 +505,10 @@ curl -X POST http://localhost:4001/app/installations/100/access_tokens \ -H "Content-Type: application/json" \ -d '{"permissions": {"contents": "read"}}' +# List repositories granted to an installation token (supports pagination) +curl http://localhost:4001/installation/repositories \ + -H "Authorization: Bearer " + # Find installation for repo / org / user curl http://localhost:4001/repos/my-org/org-repo/installation curl http://localhost:4001/orgs/my-org/installation From 34cce1dda7e2e488fe95f35164ce8014e4b24660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Wed, 14 Oct 2026 22:49:32 +0200 Subject: [PATCH 6/7] Support commit status history and webhooks --- README.md | 1 + apps/web/content/docs/github.mdx | 1 + .../github/src/__tests__/checks.test.ts | 50 ++++++++++++++ .../github/src/__tests__/reactions.test.ts | 4 +- packages/@emulators/github/src/entities.ts | 10 +++ packages/@emulators/github/src/routes/apps.ts | 2 +- .../@emulators/github/src/routes/checks.ts | 66 ++++++++++++++++++- packages/@emulators/github/src/store.ts | 3 + packages/emulate/src/index.ts | 1 + skills/github/SKILL.md | 4 ++ 10 files changed, 138 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2a6da2221..81d5cf51e 100644 --- a/README.md +++ b/README.md @@ -989,6 +989,7 @@ Draft/ready transitions and public thread resolution changes emit the correspond - Secrets: repo + org CRUD ### Checks +- Commit statuses: create and list by commit or ref, newest first, with pagination and signed `status` webhooks. Status history is retained; combined-status summaries are not implemented. - Check runs: create, update, get, annotations, rerequest, list by ref/suite. Ref based lookups accept branch and tag refs containing slashes. - Check suites: create, get, preferences, rerequest, list by ref. Ref based lookups accept branch and tag refs containing slashes. - Automatic suite status rollup from check run results diff --git a/apps/web/content/docs/github.mdx b/apps/web/content/docs/github.mdx index b41f9fb82..17ea28e72 100644 --- a/apps/web/content/docs/github.mdx +++ b/apps/web/content/docs/github.mdx @@ -236,6 +236,7 @@ Draft/ready transitions and public thread resolution changes emit the correspond ## Checks +- Commit statuses: create and list by commit or ref, newest first, with pagination and signed `status` webhooks. Status history is retained; combined-status summaries are not implemented. - Check runs: create, update, get, annotations, rerequest, list by ref/suite. Ref based lookups accept branch and tag refs containing slashes. - Check suites: create, get, preferences, rerequest, list by ref. Ref based lookups accept branch and tag refs containing slashes. - Automatic suite status rollup from check run results diff --git a/packages/@emulators/github/src/__tests__/checks.test.ts b/packages/@emulators/github/src/__tests__/checks.test.ts index 1e8642d3b..f9cc681a6 100644 --- a/packages/@emulators/github/src/__tests__/checks.test.ts +++ b/packages/@emulators/github/src/__tests__/checks.test.ts @@ -53,6 +53,56 @@ describe("GitHub checks routes", () => { app = createTestApp().app; }); + it("retains paginated commit-status history and enforces repository access", async () => { + const branch = await app.request(`${base}/repos/octocat/hello-world/branches/main`, { + headers: authHeaders(), + }); + const { commit } = (await branch.json()) as { commit: { sha: string } }; + const path = `${base}/repos/octocat/hello-world`; + const ref = await app.request(`${path}/git/refs`, { + method: "POST", + headers: jsonHeaders(), + body: JSON.stringify({ ref: "refs/heads/status/nested", sha: commit.sha }), + }); + expect(ref.status).toBe(201); + for (const state of ["pending", "failure", "success"]) { + const response = await app.request(`${path}/statuses/${commit.sha}`, { + method: "POST", + headers: jsonHeaders(), + body: JSON.stringify({ state, context: "ci", target_url: "https://ci.example.com" }), + }); + expect(response.status).toBe(201); + expect(await response.json()).toMatchObject({ + state, + context: "ci", + target_url: "https://ci.example.com", + creator: { login: "octocat" }, + }); + } + const first = await app.request(`${path}/commits/status/nested/statuses?per_page=2`, { + headers: authHeaders(), + }); + expect(first.status).toBe(200); + expect(first.headers.get("link")).toContain('rel="next"'); + expect(await first.json()).toMatchObject([{ state: "success" }, { state: "failure" }]); + const last = await app.request(`${path}/commits/${commit.sha}/statuses?per_page=2&page=2`, { + headers: authHeaders(), + }); + expect(await last.json()).toMatchObject([{ state: "pending" }]); + const invalid = await app.request(`${path}/statuses/${commit.sha}`, { + method: "POST", + headers: jsonHeaders(), + body: JSON.stringify({ state: "completed" }), + }); + expect(invalid.status).toBe(422); + const anonymous = await app.request(`${path}/statuses/${commit.sha}`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ state: "success" }), + }); + expect(anonymous.status).toBe(401); + }); + it("lists check runs and suites for refs containing slashes", async () => { const commits = await app.request(`${base}/repos/octocat/hello-world/commits`, { headers: authHeaders() }); const [head] = (await commits.json()) as Array<{ sha: string }>; diff --git a/packages/@emulators/github/src/__tests__/reactions.test.ts b/packages/@emulators/github/src/__tests__/reactions.test.ts index be1bc864a..d4e40790a 100644 --- a/packages/@emulators/github/src/__tests__/reactions.test.ts +++ b/packages/@emulators/github/src/__tests__/reactions.test.ts @@ -244,7 +244,7 @@ describe("GitHub reactions routes", () => { const comment = await request(`${targets.pull}/comments`, "POST", { body: "Pull request discussion" }); expect(comment.status).toBe(201); const commentBody = (await comment.json()) as { id: number }; - await expect.poll(() => context.webhooks.getDeliveries().length).toBe(1); + await expect.poll(() => payloads.length).toBe(1); expect(payloads).toHaveLength(1); expect(payloads[0]).toMatchObject({ issue: { @@ -255,7 +255,7 @@ describe("GitHub reactions routes", () => { comment: { html_url: `${base}/octocat/hello-world/pull/2#issuecomment-${commentBody.id}` }, }); await request(`${targets.issue}/comments`, "POST", { body: "Issue discussion" }); - await expect.poll(() => context.webhooks.getDeliveries().length).toBe(2); + await expect.poll(() => payloads.length).toBe(2); expect(payloads[1].issue).not.toHaveProperty("pull_request"); expect(payloads[1].issue).toMatchObject({ html_url: `${base}/octocat/hello-world/issues/1` }); } finally { diff --git a/packages/@emulators/github/src/entities.ts b/packages/@emulators/github/src/entities.ts index db166f1d5..2206025d0 100644 --- a/packages/@emulators/github/src/entities.ts +++ b/packages/@emulators/github/src/entities.ts @@ -491,6 +491,16 @@ export interface GitHubCheckSuite extends Entity { app_id: number | null; } +export interface GitHubCommitStatus extends Entity { + repo_id: number; + sha: string; + user_id: number; + state: "pending" | "success" | "failure" | "error"; + context: string; + description: string | null; + target_url: string | null; +} + export interface GitHubOAuthApp extends Entity { client_id: string; client_secret: string; diff --git a/packages/@emulators/github/src/routes/apps.ts b/packages/@emulators/github/src/routes/apps.ts index e4ecaa200..7e057f8a7 100644 --- a/packages/@emulators/github/src/routes/apps.ts +++ b/packages/@emulators/github/src/routes/apps.ts @@ -2,7 +2,7 @@ import { randomBytes } from "crypto"; import type { RouteContext, AuthApp } from "@emulators/core"; import { forbidden, notFound, parsePagination, setLinkHeader } from "@emulators/core"; import { getGitHubStore } from "../store.js"; -import { formatApp, formatRepo, generateNodeId } from "../helpers.js"; +import { formatApp, formatRepo } from "../helpers.js"; import { assertAuthenticatedUser, installationCanAccessRepo, isOrgMember } from "../route-helpers.js"; import type { GitHubAppInstallation, GitHubRepo, GitHubUser } from "../entities.js"; diff --git a/packages/@emulators/github/src/routes/checks.ts b/packages/@emulators/github/src/routes/checks.ts index 5ff105359..b6e37e46d 100644 --- a/packages/@emulators/github/src/routes/checks.ts +++ b/packages/@emulators/github/src/routes/checks.ts @@ -4,6 +4,7 @@ import { getGitHubStore } from "../store.js"; import type { GitHubStore } from "../store.js"; import type { GitHubCheckRun, + GitHubCommitStatus, GitHubCheckSuite, GitHubCommit, GitHubRepo, @@ -296,7 +297,70 @@ function dispatchCheckSuite( export function checksRoutes({ app, store, webhooks, baseUrl }: RouteContext): void { const gh = getGitHubStore(store); - // --- Preferences (static path before :check_suite_id) --- + const formatStatus = (row: GitHubCommitStatus, repo: GitHubRepo) => ({ + id: row.id, + node_id: generateNodeId("Status", row.id), + state: row.state, + context: row.context, + description: row.description, + target_url: row.target_url, + created_at: row.created_at, + updated_at: row.updated_at, + url: `${baseUrl}/repos/${repo.full_name}/statuses/${row.sha}`, + creator: formatUser(gh.users.get(row.user_id)!, baseUrl), + }); + app.post("/repos/:owner/:repo/statuses/:sha", async (c) => { + const repo = lookupRepo(gh, c.req.param("owner")!, c.req.param("repo")!); + if (!repo) throw notFoundResponse(); + const actor = assertRepoWrite(gh, c.get("authUser"), repo, "statuses"); + const sha = resolveRefToHeadSha(gh, repo, c.req.param("sha")!); + if (!sha) throw notFoundResponse(); + const body = await parseJsonBody(c); + if (typeof body.state !== "string" || !["pending", "success", "failure", "error"].includes(body.state)) { + throw new ApiError(422, "Invalid commit status state"); + } + const row = gh.commitStatuses.insert({ + repo_id: repo.id, + sha, + user_id: actor.id, + state: body.state as GitHubCommitStatus["state"], + context: typeof body.context === "string" ? body.context : "default", + description: typeof body.description === "string" ? body.description : null, + target_url: typeof body.target_url === "string" ? body.target_url : null, + }); + void webhooks.dispatch( + "status", + void 0, + { + ...formatStatus(row, repo), + sha, + name: repo.full_name, + branches: gh.branches + .findBy("repo_id", repo.id) + .filter((branch) => branch.sha === sha) + .map((branch) => ({ name: branch.name })), + repository: formatRepo(repo, gh, baseUrl), + sender: formatUser(actor, baseUrl), + }, + ownerLoginOf(gh, repo), + repo.name, + ); + return c.json(formatStatus(row, repo), 201); + }); + app.get("/repos/:owner/:repo/commits/:ref{.+}/statuses", (c) => { + const repo = lookupRepo(gh, c.req.param("owner")!, c.req.param("repo")!); + if (!repo) throw notFoundResponse(); + assertRepoPermission(gh, c.get("authUser"), repo, "statuses"); + const sha = resolveRefToHeadSha(gh, repo, c.req.param("ref")!); + if (!sha) throw notFoundResponse(); + const { page, per_page } = parsePagination(c); + const rows = gh.commitStatuses + .findBy("repo_id", repo.id) + .filter((row) => row.sha === sha) + .sort((a, b) => b.id - a.id); + setLinkHeader(c, rows.length, page, per_page); + return c.json(rows.slice((page - 1) * per_page, page * per_page).map((row) => formatStatus(row, repo))); + }); app.patch("/repos/:owner/:repo/check-suites/preferences", async (c) => { const owner = c.req.param("owner")!; const repoName = c.req.param("repo")!; diff --git a/packages/@emulators/github/src/store.ts b/packages/@emulators/github/src/store.ts index 535d49a4c..2ceb70883 100644 --- a/packages/@emulators/github/src/store.ts +++ b/packages/@emulators/github/src/store.ts @@ -32,6 +32,7 @@ import type { GitHubSecret, GitHubCheckRun, GitHubCheckSuite, + GitHubCommitStatus, GitHubOAuthApp, GitHubApp, GitHubAppInstallation, @@ -72,6 +73,7 @@ export interface GitHubStore { secrets: Collection; checkRuns: Collection; checkSuites: Collection; + commitStatuses: Collection; oauthApps: Collection; apps: Collection; appInstallations: Collection; @@ -113,6 +115,7 @@ export function getGitHubStore(store: Store): GitHubStore { secrets: store.collection("github.secrets", ["repo_id", "org_id"]), checkRuns: store.collection("github.check_runs", ["repo_id", "head_sha"]), checkSuites: store.collection("github.check_suites", ["repo_id", "head_sha"]), + commitStatuses: store.collection("github.commit_statuses", ["repo_id", "sha"]), oauthApps: store.collection("github.oauth_apps", ["client_id"]), apps: store.collection("github.apps", ["slug"]), appInstallations: store.collection("github.app_installations", [ diff --git a/packages/emulate/src/index.ts b/packages/emulate/src/index.ts index dc215eb4b..5b6576416 100644 --- a/packages/emulate/src/index.ts +++ b/packages/emulate/src/index.ts @@ -55,6 +55,7 @@ Custom emulators: Docs: https://emulate.dev/docs/custom-emulators GitHub API coverage: + Commit statuses support create, paginated history by ref, and signed status webhooks; combined summaries are not implemented. Includes repository contents, raw downloads, raw media negotiation for file Contents and README responses, commit history, commit details, ref comparisons, organization membership seeding with member/admin roles, and Checks list-by-ref endpoints for branch and tag refs containing slashes. diff --git a/skills/github/SKILL.md b/skills/github/SKILL.md index a9bca234d..ab3100685 100644 --- a/skills/github/SKILL.md +++ b/skills/github/SKILL.md @@ -584,6 +584,10 @@ curl -X POST http://localhost:4001/repos/octocat/hello-world/check-runs \ -d '{"name": "CI", "head_sha": "abc123", "status": "completed", "conclusion": "success"}' # Check suites: create, get, rerequest, preferences, list by ref. Ref based lookups accept branch and tag refs containing slashes. +# Commit statuses: POST /repos/:owner/:repo/statuses/:sha; GET /repos/:owner/:repo/commits/:ref/statuses. +# Lists retain history in newest-first order with page/per_page and Link headers; writes emit signed status webhooks. +# Combined-status summaries are not implemented. + # Check runs: list for suite, annotations. Ref based lookups accept branch and tag refs containing slashes. # Automatic suite status rollup from check run results ``` From a43293ec3035cfe3e28f73addbd66816fcc9da0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Wed, 14 Oct 2026 23:15:10 +0200 Subject: [PATCH 7/7] Complete requested reviewer lifecycle behavior --- README.md | 5 +- apps/web/content/docs/github.mdx | 5 +- .../src/__tests__/requested-reviewers.test.ts | 129 ++++++++++++++++++ .../@emulators/github/src/routes/pulls.ts | 45 +++++- .../@emulators/github/src/routes/reviews.ts | 6 + .../@emulators/github/src/routes/users.ts | 6 + packages/emulate/src/index.ts | 2 + skills/github/SKILL.md | 2 + 8 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 packages/@emulators/github/src/__tests__/requested-reviewers.test.ts diff --git a/README.md b/README.md index 81d5cf51e..ec3137b3b 100644 --- a/README.md +++ b/README.md @@ -848,6 +848,7 @@ Every endpoint below is fully stateful. Creates, updates, and deletes persist in - `GET /user` - authenticated user - `PATCH /user` - update profile - `GET /users/:username` - get user +- `GET /user/:account_id` - resolve the current user profile by durable ID - `GET /users` - list users - `GET /users/:username/repos` - list user repos - `GET /users/:username/orgs` - list user orgs @@ -899,11 +900,13 @@ Every endpoint below is fully stateful. Creates, updates, and deletes persist in - `PUT /repos/:owner/:repo/pulls/:number/merge` - merge (with branch protection enforcement) - `GET /repos/:owner/:repo/pulls/:number/commits` - list commits - `GET /repos/:owner/:repo/pulls/:number/files` - list files -- `POST/DELETE /repos/:owner/:repo/pulls/:number/requested_reviewers` - manage reviewers +- `GET/POST/DELETE /repos/:owner/:repo/pulls/:number/requested_reviewers` - list or manage user and team reviewers; removals emit `pull_request.review_request_removed` - `PUT /repos/:owner/:repo/pulls/:number/update-branch` - update branch Updating only a pull request's `base` branch emits `pull_request.edited` with the new base ref and SHA. +Pull request authors cannot be requested to review their own pull request. Submitting a review removes the author from the outstanding requested users; pending reviews retain the request. Team requests remain until explicitly removed. + ### Comments - Issue comments: full CRUD on `/repos/:owner/:repo/issues/:number/comments` - Review comments: full CRUD on `/repos/:owner/:repo/pulls/:number/comments` diff --git a/apps/web/content/docs/github.mdx b/apps/web/content/docs/github.mdx index 17ea28e72..99935be57 100644 --- a/apps/web/content/docs/github.mdx +++ b/apps/web/content/docs/github.mdx @@ -81,6 +81,7 @@ Organization `members` are optional. Each entry references a seeded user by `log - `GET /user` - authenticated user - `PATCH /user` - update profile - `GET /users/:username` - get user +- `GET /user/:account_id` - resolve the current user profile by durable ID - `GET /users` - list users - `GET /users/:username/repos` - list user repos - `GET /users/:username/orgs` - list user orgs @@ -136,11 +137,13 @@ Organization `members` are optional. Each entry references a seeded user by `log - `PUT /repos/:owner/:repo/pulls/:number/merge` - merge (with branch protection enforcement) - `GET /repos/:owner/:repo/pulls/:number/commits` - list commits - `GET /repos/:owner/:repo/pulls/:number/files` - list files -- `POST/DELETE /repos/:owner/:repo/pulls/:number/requested_reviewers` - manage reviewers +- `GET/POST/DELETE /repos/:owner/:repo/pulls/:number/requested_reviewers` - list or manage user and team reviewers; removals emit `pull_request.review_request_removed` - `PUT /repos/:owner/:repo/pulls/:number/update-branch` - update branch Updating only a pull request's `base` branch emits `pull_request.edited` with the new base ref and SHA. +Pull request authors cannot be requested to review their own pull request. Submitting a review removes the author from the outstanding requested users; pending reviews retain the request. Team requests remain until explicitly removed. + ## Comments - Issue comments: full CRUD on `/repos/:owner/:repo/issues/:number/comments` diff --git a/packages/@emulators/github/src/__tests__/requested-reviewers.test.ts b/packages/@emulators/github/src/__tests__/requested-reviewers.test.ts new file mode 100644 index 000000000..45599e740 --- /dev/null +++ b/packages/@emulators/github/src/__tests__/requested-reviewers.test.ts @@ -0,0 +1,129 @@ +import { createServer } from "node:http"; +import { once } from "node:events"; +import { beforeEach, describe, expect, it } from "vitest"; +import { + Hono, + Store, + WebhookDispatcher, + authMiddleware, + createApiErrorHandler, + type AppEnv, + type TokenMap, +} from "@emulators/core"; +import { getGitHubStore, githubPlugin, seedFromConfig } from "../index.js"; + +const base = "http://localhost:4000"; +const repoPath = "/repos/acme/knowledge"; + +describe("requested reviewers", () => { + let app: Hono; + let gh: ReturnType; + let webhooks: WebhookDispatcher; + let pull: { number: number }; + + const request = (path: string, method = "GET", body?: unknown, token = "octocat-token") => + app.request(`${base}${path}`, { + method, + headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + const reviewersPath = () => `${repoPath}/pulls/${pull.number}/requested_reviewers`; + + beforeEach(async () => { + const store = new Store(); + webhooks = new WebhookDispatcher(); + const tokens: TokenMap = new Map(); + app = new Hono(); + app.onError(createApiErrorHandler()); + app.use("*", authMiddleware(tokens)); + githubPlugin.register(app, store, webhooks, base, tokens); + seedFromConfig(store, base, { + users: [{ login: "octocat" }, { login: "reviewer" }], + orgs: [{ login: "acme", members: [{ login: "octocat", role: "admin" }, { login: "reviewer" }] }], + repos: [{ owner: "acme", name: "knowledge", auto_init: true }], + }); + gh = getGitHubStore(store); + for (const user of gh.users.all()) + tokens.set(`${user.login}-token`, { id: user.id, login: user.login, scopes: ["repo"] }); + const created = await request(`${repoPath}/pulls`, "POST", { title: "Review", head: "feature", base: "main" }); + expect(created.status).toBe(201); + pull = (await created.json()) as typeof pull; + }); + + it("reads current user identities and rejects requesting the pull request author", async () => { + const reviewer = gh.users.findOneBy("login", "reviewer")!; + gh.users.update(reviewer.id, { login: "renamed-reviewer" }); + expect(await (await request(`/user/${reviewer.id}`)).json()).toMatchObject({ + id: reviewer.id, + login: "renamed-reviewer", + }); + expect((await request("/user/999999")).status).toBe(404); + expect((await request(reviewersPath(), "POST", { reviewers: ["octocat", "renamed-reviewer"] })).status).toBe(422); + expect(await (await request(reviewersPath())).json()).toEqual({ users: [], teams: [] }); + expect((await request(reviewersPath(), "POST", { reviewers: ["renamed-reviewer"] })).status).toBe(200); + expect(await (await request(reviewersPath())).json()).toMatchObject({ + users: [{ id: reviewer.id, login: "renamed-reviewer" }], + teams: [], + }); + }); + + it("emits removals only for stored user and team requests", async () => { + const created = await request("/orgs/acme/teams", "POST", { name: "Reviewers" }); + const team = (await created.json()) as { id: number; slug: string }; + expect( + (await request(reviewersPath(), "POST", { reviewers: ["reviewer"], team_reviewers: [team.slug] })).status, + ).toBe(200); + const receiver = createServer((_req, response) => response.writeHead(200).end()); + receiver.listen(0, "127.0.0.1"); + await once(receiver, "listening"); + const address = receiver.address(); + if (!address || typeof address === "string") throw new Error("Missing receiver address"); + webhooks.register({ + url: `http://127.0.0.1:${address.port}`, + active: true, + events: ["pull_request"], + owner: "acme", + repo: "knowledge", + }); + try { + expect(await (await request(reviewersPath())).json()).toMatchObject({ + users: [{ login: "reviewer" }], + teams: [{ id: team.id, slug: team.slug }], + }); + for (let attempt = 0; attempt < 2; attempt++) + expect( + (await request(reviewersPath(), "DELETE", { reviewers: ["reviewer"], team_reviewers: [team.slug] })).status, + ).toBe(200); + expect(await (await request(reviewersPath())).json()).toEqual({ users: [], teams: [] }); + await expect.poll(() => webhooks.getDeliveries().filter((delivery) => delivery.success)).toHaveLength(2); + const removed = webhooks.getDeliveries(); + expect(removed.map(({ action }) => action)).toEqual(["review_request_removed", "review_request_removed"]); + expect(removed[0]?.payload).toMatchObject({ + requested_reviewer: { login: "reviewer" }, + pull_request: { requested_reviewers: [], requested_teams: [] }, + }); + expect(removed[1]?.payload).toMatchObject({ + requested_team: { id: team.id }, + pull_request: { requested_reviewers: [], requested_teams: [] }, + }); + } finally { + await new Promise((resolve, reject) => receiver.close((error) => (error ? reject(error) : resolve()))); + } + }); + + it("retains a pending review request until submission and clears immediate submissions", async () => { + const path = `${repoPath}/pulls/${pull.number}/reviews`; + expect((await request(reviewersPath(), "POST", { reviewers: ["reviewer"] })).status).toBe(200); + const pending = await request(path, "POST", { body: "Draft" }, "reviewer-token"); + expect(pending.status).toBe(201); + const review = (await pending.json()) as { id: number }; + expect(await (await request(reviewersPath())).json()).toMatchObject({ users: [{ login: "reviewer" }] }); + expect((await request(`${path}/${review.id}/events`, "POST", { event: "COMMENT" }, "reviewer-token")).status).toBe( + 200, + ); + expect(await (await request(reviewersPath())).json()).toEqual({ users: [], teams: [] }); + expect((await request(reviewersPath(), "POST", { reviewers: ["reviewer"] })).status).toBe(200); + expect((await request(path, "POST", { body: "Approved", event: "APPROVE" }, "reviewer-token")).status).toBe(201); + expect(await (await request(reviewersPath())).json()).toEqual({ users: [], teams: [] }); + }); +}); diff --git a/packages/@emulators/github/src/routes/pulls.ts b/packages/@emulators/github/src/routes/pulls.ts index c2f27a082..c4171ad1f 100644 --- a/packages/@emulators/github/src/routes/pulls.ts +++ b/packages/@emulators/github/src/routes/pulls.ts @@ -21,6 +21,7 @@ import type { import { formatPullRequest, formatRepo, + formatTeamBrief, formatUser, generateNodeId, generateSha, @@ -806,6 +807,16 @@ export function pullsRoutes({ app, store, webhooks, baseUrl }: RouteContext): vo ); }); + app.get("/repos/:owner/:repo/pulls/:pull_number/requested_reviewers", (c) => { + const repo = lookupRepo(gh, c.req.param("owner")!, c.req.param("repo")!); + if (!repo) throw notFoundResponse(); + assertRepoPermission(gh, c.get("authUser"), repo, "pull_requests"); + const pr = findPull(gh, repo.id, Number(c.req.param("pull_number"))); + if (!pr) throw notFoundResponse(); + const formatted = formatPullRequest(pr, gh, baseUrl)!; + return c.json({ users: formatted.requested_reviewers, teams: formatted.requested_teams }); + }); + app.post("/repos/:owner/:repo/pulls/:pull_number/requested_reviewers", async (c) => { const owner = c.req.param("owner")!; const repoName = c.req.param("repo")!; @@ -832,6 +843,8 @@ export function pullsRoutes({ app, store, webhooks, baseUrl }: RouteContext): vo : []; const newUserIds = reviewerLogins.map((login) => lookupUserByLogin(gh, login).id); + if (newUserIds.includes(pr.user_id)) + throw new ApiError(422, "Pull request authors cannot be requested to review their own pull request."); let newTeamIds: number[] = []; if (teamSlugs.length > 0) { if (repo.owner_type !== "Organization") { @@ -870,7 +883,7 @@ export function pullsRoutes({ app, store, webhooks, baseUrl }: RouteContext): vo const repo = lookupRepo(gh, owner, repoName); if (!repo) throw notFoundResponse(); - assertRepoWrite(gh, c.get("authUser"), repo, "pull_requests"); + const actor = assertRepoWrite(gh, c.get("authUser"), repo, "pull_requests"); const pullNumber = parseInt(c.req.param("pull_number")!, 10); if (!Number.isFinite(pullNumber)) throw notFoundResponse(); @@ -900,7 +913,35 @@ export function pullsRoutes({ app, store, webhooks, baseUrl }: RouteContext): vo gh.pullRequests.update(pr.id, { requested_reviewer_ids, requested_team_ids }); const fresh = gh.pullRequests.get(pr.id)!; - return c.json(formatPullRequest(fresh, gh, baseUrl)!); + const prFmt = formatPullRequest(fresh, gh, baseUrl)!; + const removed = [ + ...pr.requested_reviewer_ids + .filter((id) => removeUserIds.has(id)) + .map((id) => ({ + requested_reviewer: formatUser(gh.users.get(id)!, baseUrl), + })), + ...pr.requested_team_ids + .filter((id) => removeTeamIds.has(id)) + .map((id) => ({ + requested_team: formatTeamBrief(gh.teams.get(id)!, gh, baseUrl), + })), + ]; + for (const requested of removed) { + webhooks.dispatch( + "pull_request", + "review_request_removed", + { + action: "review_request_removed", + ...requested, + pull_request: prFmt, + repository: formatRepo(repo, gh, baseUrl), + sender: formatUser(actor, baseUrl), + }, + ownerLoginOf(gh, repo), + repo.name, + ); + } + return c.json(prFmt); }); app.put("/repos/:owner/:repo/pulls/:pull_number/update-branch", async (c) => { diff --git a/packages/@emulators/github/src/routes/reviews.ts b/packages/@emulators/github/src/routes/reviews.ts index b90a78a3e..bf9ebe0d5 100644 --- a/packages/@emulators/github/src/routes/reviews.ts +++ b/packages/@emulators/github/src/routes/reviews.ts @@ -92,6 +92,12 @@ function dispatchReviewWebhook( baseUrl: string, action: "submitted" | "dismissed" | "edited", ) { + if (action === "submitted") { + pr = + gh.pullRequests.update(pr.id, { + requested_reviewer_ids: pr.requested_reviewer_ids.filter((id) => id !== actor.id), + }) ?? pr; + } const ownerLogin = ownerLoginOf(gh, repo); const reviewFmt = formatReview(review, gh, baseUrl); if (!reviewFmt) return; diff --git a/packages/@emulators/github/src/routes/users.ts b/packages/@emulators/github/src/routes/users.ts index 1da820b7e..83ae7daa1 100644 --- a/packages/@emulators/github/src/routes/users.ts +++ b/packages/@emulators/github/src/routes/users.ts @@ -71,6 +71,12 @@ export function usersRoutes({ app, store, baseUrl }: RouteContext): void { return c.json(formatUserFull(user, baseUrl)); }); + app.get("/user/:account_id{[0-9]+}", (c) => { + const user = gh.users.get(Number(c.req.param("account_id"))); + if (!user) throw notFoundResponse(); + return c.json(formatUserFull(user, baseUrl)); + }); + app.patch("/user", async (c) => { const authUser = c.get("authUser"); if (!authUser) { diff --git a/packages/emulate/src/index.ts b/packages/emulate/src/index.ts index 5b6576416..afe15b0bb 100644 --- a/packages/emulate/src/index.ts +++ b/packages/emulate/src/index.ts @@ -55,6 +55,8 @@ Custom emulators: Docs: https://emulate.dev/docs/custom-emulators GitHub API coverage: + User ID lookup and requested-reviewer listing use stored identities; removals emit review_request_removed. + Requests reject the PR author; submitted reviews clear that user request while drafts retain it. Commit statuses support create, paginated history by ref, and signed status webhooks; combined summaries are not implemented. Includes repository contents, raw downloads, raw media negotiation for file Contents and README responses, commit history, commit details, ref comparisons, organization membership seeding with member/admin roles, diff --git a/skills/github/SKILL.md b/skills/github/SKILL.md index ab3100685..ef27fba25 100644 --- a/skills/github/SKILL.md +++ b/skills/github/SKILL.md @@ -345,6 +345,8 @@ curl -X PUT http://localhost:4001/repos/octocat/hello-world/pulls/1/merge \ Updating only a pull request's `base` branch emits `pull_request.edited` with the new base ref and SHA. +`GET /user/:account_id` resolves a current username by durable user ID. `GET /repos/:owner/:repo/pulls/:number/requested_reviewers` returns `{ users, teams }`. Requesting the pull request author returns HTTP 422; deleting requests emits `pull_request.review_request_removed` for each removed user or team. Submitting a review clears its author’s user request; pending reviews retain it and team requests require explicit removal. + ### Comments ```bash