From 6f8cdc67bea7d19e862ff78eab1efde16218f343 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 22 Jun 2026 11:26:41 +0200 Subject: [PATCH 01/17] feat(PRO-547): add CLI-side GitHub comment sync (pull/push) --- .../cli/drizzle/0007_sloppy_leopardon.sql | 3 + packages/cli/drizzle/meta/0007_snapshot.json | 796 ++++++++++++++++++ packages/cli/drizzle/meta/_journal.json | 7 + .../src/__tests__/comment-sync.routes.test.ts | 295 +++++++ .../cli/src/__tests__/comments.routes.test.ts | 3 +- .../__tests__/review-comment-mapping.test.ts | 81 ++ packages/cli/src/db/schema/comment.ts | 17 +- packages/cli/src/git.ts | 17 + packages/cli/src/github/index.ts | 10 + packages/cli/src/github/review-comments.ts | 226 +++++ packages/cli/src/routes/comments.ts | 56 +- packages/cli/src/runs/comment-sync.ts | 281 +++++++ .../cli/src/runs/review-comment-mapping.ts | 55 ++ packages/types/src/comments.ts | 43 +- 14 files changed, 1885 insertions(+), 5 deletions(-) create mode 100644 packages/cli/drizzle/0007_sloppy_leopardon.sql create mode 100644 packages/cli/drizzle/meta/0007_snapshot.json create mode 100644 packages/cli/src/__tests__/comment-sync.routes.test.ts create mode 100644 packages/cli/src/__tests__/review-comment-mapping.test.ts create mode 100644 packages/cli/src/github/review-comments.ts create mode 100644 packages/cli/src/runs/comment-sync.ts create mode 100644 packages/cli/src/runs/review-comment-mapping.ts diff --git a/packages/cli/drizzle/0007_sloppy_leopardon.sql b/packages/cli/drizzle/0007_sloppy_leopardon.sql new file mode 100644 index 0000000..f412ea2 --- /dev/null +++ b/packages/cli/drizzle/0007_sloppy_leopardon.sql @@ -0,0 +1,3 @@ +ALTER TABLE `comment` ADD `authorAvatarUrl` text;--> statement-breakpoint +ALTER TABLE `comment` ADD `githubCommentId` integer;--> statement-breakpoint +CREATE UNIQUE INDEX `comment_github_comment_id_idx` ON `comment` (`githubCommentId`); \ No newline at end of file diff --git a/packages/cli/drizzle/meta/0007_snapshot.json b/packages/cli/drizzle/meta/0007_snapshot.json new file mode 100644 index 0000000..4a04ad7 --- /dev/null +++ b/packages/cli/drizzle/meta/0007_snapshot.json @@ -0,0 +1,796 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "de6a20c0-0b5b-4353-aa2e-611d0300530b", + "prevId": "a44dabe5-4d9e-445a-994d-fa87195fc4b2", + "tables": { + "chapter": { + "name": "chapter", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "runId": { + "name": "runId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "externalId": { + "name": "externalId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "chapterIndex": { + "name": "chapterIndex", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "summary": { + "name": "summary", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "hunkRefs": { + "name": "hunkRefs", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "keyChanges": { + "name": "keyChanges", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + } + }, + "indexes": { + "chapter_run_idx_unique": { + "name": "chapter_run_idx_unique", + "columns": [ + "runId", + "chapterIndex" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_runId_chapter_run_id_fk": { + "name": "chapter_runId_chapter_run_id_fk", + "tableFrom": "chapter", + "tableTo": "chapter_run", + "columnsFrom": [ + "runId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_file_view": { + "name": "chapter_file_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "chapter_file_view_chapter_id_idx": { + "name": "chapter_file_view_chapter_id_idx", + "columns": [ + "chapterId" + ], + "isUnique": false + }, + "chapter_file_view_user_chapter_path_unique": { + "name": "chapter_file_view_user_chapter_path_unique", + "columns": [ + "userId", + "chapterId", + "filePath" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_file_view_chapterId_chapter_id_fk": { + "name": "chapter_file_view_chapterId_chapter_id_fk", + "tableFrom": "chapter_file_view", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_run": { + "name": "chapter_run", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "repoRoot": { + "name": "repoRoot", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "originUrl": { + "name": "originUrl", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "prNumber": { + "name": "prNumber", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "scopeKind": { + "name": "scopeKind", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "workingTreeRef": { + "name": "workingTreeRef", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "baseSha": { + "name": "baseSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "headSha": { + "name": "headSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "mergeBaseSha": { + "name": "mergeBaseSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "generatedAt": { + "name": "generatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "prologue": { + "name": "prologue", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "chapter_run_created_at_idx": { + "name": "chapter_run_created_at_idx", + "columns": [ + "createdAt" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_view": { + "name": "chapter_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "chapter_view_user_chapter_unique": { + "name": "chapter_view_user_chapter_unique", + "columns": [ + "userId", + "chapterId" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_view_chapterId_chapter_id_fk": { + "name": "chapter_view_chapterId_chapter_id_fk", + "tableFrom": "chapter_view", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "comment": { + "name": "comment", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "threadId": { + "name": "threadId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "authorId": { + "name": "authorId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "authorAvatarUrl": { + "name": "authorAvatarUrl", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "body": { + "name": "body", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "githubCommentId": { + "name": "githubCommentId", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "comment_thread_id_idx": { + "name": "comment_thread_id_idx", + "columns": [ + "threadId" + ], + "isUnique": false + }, + "comment_github_comment_id_idx": { + "name": "comment_github_comment_id_idx", + "columns": [ + "githubCommentId" + ], + "isUnique": true + } + }, + "foreignKeys": { + "comment_threadId_comment_thread_id_fk": { + "name": "comment_threadId_comment_thread_id_fk", + "tableFrom": "comment", + "tableTo": "comment_thread", + "columnsFrom": [ + "threadId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "comment_thread": { + "name": "comment_thread", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "scopeKey": { + "name": "scopeKey", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "side": { + "name": "side", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "startLine": { + "name": "startLine", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "endLine": { + "name": "endLine", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "resolvedAt": { + "name": "resolvedAt", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "comment_thread_scope_key_idx": { + "name": "comment_thread_scope_key_idx", + "columns": [ + "scopeKey" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "file_view": { + "name": "file_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "runId": { + "name": "runId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "file_view_user_run_path_unique": { + "name": "file_view_user_run_path_unique", + "columns": [ + "userId", + "runId", + "filePath" + ], + "isUnique": true + } + }, + "foreignKeys": { + "file_view_runId_chapter_run_id_fk": { + "name": "file_view_runId_chapter_run_id_fk", + "tableFrom": "file_view", + "tableTo": "chapter_run", + "columnsFrom": [ + "runId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "key_change": { + "name": "key_change", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "externalId": { + "name": "externalId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "lineRefs": { + "name": "lineRefs", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + } + }, + "indexes": { + "key_change_chapter_id_idx": { + "name": "key_change_chapter_id_idx", + "columns": [ + "chapterId" + ], + "isUnique": false + } + }, + "foreignKeys": { + "key_change_chapterId_chapter_id_fk": { + "name": "key_change_chapterId_chapter_id_fk", + "tableFrom": "key_change", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "key_change_view": { + "name": "key_change_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "keyChangeId": { + "name": "keyChangeId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "key_change_view_key_change_id_idx": { + "name": "key_change_view_key_change_id_idx", + "columns": [ + "keyChangeId" + ], + "isUnique": false + }, + "key_change_view_user_key_change_unique": { + "name": "key_change_view_user_key_change_unique", + "columns": [ + "userId", + "keyChangeId" + ], + "isUnique": true + } + }, + "foreignKeys": { + "key_change_view_keyChangeId_key_change_id_fk": { + "name": "key_change_view_keyChangeId_key_change_id_fk", + "tableFrom": "key_change_view", + "tableTo": "key_change", + "columnsFrom": [ + "keyChangeId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/packages/cli/drizzle/meta/_journal.json b/packages/cli/drizzle/meta/_journal.json index 1ea682b..ba7fdb7 100644 --- a/packages/cli/drizzle/meta/_journal.json +++ b/packages/cli/drizzle/meta/_journal.json @@ -50,6 +50,13 @@ "when": 1780873869130, "tag": "0006_puzzling_beast", "breakpoints": true + }, + { + "idx": 7, + "version": "6", + "when": 1782119754913, + "tag": "0007_sloppy_leopardon", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/cli/src/__tests__/comment-sync.routes.test.ts b/packages/cli/src/__tests__/comment-sync.routes.test.ts new file mode 100644 index 0000000..2e3fa1f --- /dev/null +++ b/packages/cli/src/__tests__/comment-sync.routes.test.ts @@ -0,0 +1,295 @@ +import fs from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import type { PullCommentsResult, PushCommentsResult } from "@stagereview/types/comments"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { closeDb, getDb } from "../db/client.js"; +import { chapterRun, comment, commentThread } from "../db/schema/index.js"; +import { commentRoutes } from "../routes/comments.js"; +import { SCOPE_KIND, WORKING_TREE_REF } from "../schema.js"; +import { LOOPBACK_HOST, type ServerHandle, startServer } from "../server.js"; + +let tmpDir: string; +let dbPath: string; +let webDist: string; +let repoRoot: string; +let binDir: string; +let originalPath: string | undefined; +const handles: ServerHandle[] = []; + +const HEAD_SHA = "a".repeat(40); +const GITHUB_ORIGIN = "git@github.com:owner/repo.git"; + +// REST review comments the fake `gh` returns for a pull (one root + one reply). +const REVIEW_COMMENTS = [ + [ + { + id: 101, + in_reply_to_id: null, + path: "src/foo.ts", + line: 10, + start_line: 5, + side: "RIGHT", + body: "Root comment", + created_at: "2026-01-01T00:00:00Z", + user: { login: "octocat", avatar_url: "https://example.com/octocat.png", type: "User" }, + }, + { + id: 102, + in_reply_to_id: 101, + path: "src/foo.ts", + line: 10, + start_line: null, + side: "RIGHT", + body: "A reply", + created_at: "2026-01-02T00:00:00Z", + user: { login: "hubot", avatar_url: "https://example.com/hubot.png", type: "Bot" }, + }, + ], +]; + +// `gh`/`git` shims that route on argv and emit canned JSON, so the sync paths run +// end-to-end without network or a real repo. Infrastructure fakes, not mocks. +async function writeShims(opts: { gitHead: string; gitStatus: string }): Promise { + const ghFixture = { + pr: { + number: 5, + title: "Add foo", + body: "", + url: "https://github.com/owner/repo/pull/5", + state: "OPEN", + isDraft: false, + mergedAt: null, + createdAt: "2026-01-01T00:00:00Z", + author: { login: "octocat", is_bot: false }, + headRefName: "feature", + headRefOid: HEAD_SHA, + baseRefName: "main", + }, + restPr: { + user: { login: "octocat", avatar_url: "https://example.com/octocat.png", type: "User" }, + requested_reviewers: [], + }, + graphql: { + data: { + repository: { + pullRequest: { + reviewThreads: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [], + }, + }, + }, + }, + }, + comments: REVIEW_COMMENTS, + }; + await fs.writeFile(path.join(tmpDir, "gh-fixture.json"), JSON.stringify(ghFixture)); + + const ghShim = `#!/usr/bin/env node +const fs = require("node:fs"); +const args = process.argv.slice(2); +const fx = JSON.parse(fs.readFileSync(${JSON.stringify(path.join(tmpDir, "gh-fixture.json"))}, "utf8")); +const log = ${JSON.stringify(path.join(tmpDir, "gh-log.txt"))}; +const counterFile = ${JSON.stringify(path.join(tmpDir, "gh-counter.txt"))}; +function nextId() { + let n = 1000; + try { n = parseInt(fs.readFileSync(counterFile, "utf8"), 10); } catch {} + n += 1; + fs.writeFileSync(counterFile, String(n)); + return n; +} +if (args[0] === "pr" && args[1] === "view") { + process.stdout.write(JSON.stringify(fx.pr)); +} else if (args[0] === "api" && args[1] === "graphql") { + process.stdout.write(JSON.stringify(fx.graphql)); +} else if (args[0] === "api") { + const endpoint = args[1]; + const isPost = args.includes("POST"); + if (/\\/comments$/.test(endpoint) && isPost) { + fs.appendFileSync(log, "create " + args.join(" ") + "\\n"); + process.stdout.write(JSON.stringify({ id: nextId() })); + } else if (/\\/replies$/.test(endpoint) && isPost) { + fs.appendFileSync(log, "reply " + args.join(" ") + "\\n"); + process.stdout.write(JSON.stringify({ id: nextId() })); + } else if (/\\/comments$/.test(endpoint)) { + process.stdout.write(JSON.stringify(fx.comments)); + } else if (/\\/pulls\\/\\d+$/.test(endpoint)) { + process.stdout.write(JSON.stringify(fx.restPr)); + } else { + process.stdout.write("{}"); + } +} +`; + await fs.writeFile(path.join(binDir, "gh"), ghShim); + await fs.chmod(path.join(binDir, "gh"), 0o755); + + const gitShim = `#!/usr/bin/env node +const args = process.argv.slice(2); +if (args.includes("rev-parse")) process.stdout.write(${JSON.stringify(opts.gitHead)}); +else if (args.includes("status")) process.stdout.write(${JSON.stringify(opts.gitStatus)}); +`; + await fs.writeFile(path.join(binDir, "git"), gitShim); + await fs.chmod(path.join(binDir, "git"), 0o755); +} + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-sync-")); + dbPath = path.join(tmpDir, "db.sqlite"); + webDist = path.join(tmpDir, "web-dist"); + repoRoot = path.join(tmpDir, "repo"); + binDir = path.join(tmpDir, "bin"); + await fs.mkdir(webDist); + await fs.writeFile(path.join(webDist, "index.html"), ""); + await fs.mkdir(repoRoot); + await fs.mkdir(binDir); + originalPath = process.env.PATH; + // Shim dir first so `gh`/`git` resolve to the fakes. + process.env.PATH = `${binDir}${path.delimiter}${originalPath ?? ""}`; + closeDb(); +}); + +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.close(); + } + closeDb(); + process.env.PATH = originalPath; + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +function insertRun(scopeKind: string = SCOPE_KIND.COMMITTED): string { + const db = getDb({ dbPath }); + const [row] = db + .insert(chapterRun) + .values({ + repoRoot, + originUrl: GITHUB_ORIGIN, + scopeKind: + scopeKind === SCOPE_KIND.COMMITTED ? SCOPE_KIND.COMMITTED : SCOPE_KIND.WORKING_TREE, + workingTreeRef: scopeKind === SCOPE_KIND.COMMITTED ? null : WORKING_TREE_REF.WORK, + baseSha: "b".repeat(40), + headSha: HEAD_SHA, + mergeBaseSha: "c".repeat(40), + generatedAt: new Date(), + }) + .returning({ id: chapterRun.id }) + .all(); + if (!row) throw new Error("seed: chapter_run insert returned no row"); + return row.id; +} + +function seedLocalThread(): void { + const db = getDb({ dbPath }); + const scopeKey = `committed:${"b".repeat(40)}:${HEAD_SHA}:${"c".repeat(40)}`; + const [thread] = db + .insert(commentThread) + .values({ scopeKey, filePath: "src/foo.ts", side: "additions", startLine: 3, endLine: 3 }) + .returning({ id: commentThread.id }) + .all(); + if (!thread) throw new Error("seed: thread insert returned no row"); + db.insert(comment).values({ threadId: thread.id, body: "Push me" }).run(); +} + +async function start(): Promise { + const db = getDb({ dbPath }); + const handle = await startServer({ webDistPath: webDist, routes: commentRoutes(db) }); + handles.push(handle); + return handle.port; +} + +function post(port: number, p: string): Promise<{ status: number; body: string }> { + return new Promise((resolve, reject) => { + const req = http.request( + { hostname: LOOPBACK_HOST, port, method: "POST", path: p, agent: false }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (c: Buffer) => chunks.push(c)); + res.on("end", () => + resolve({ status: res.statusCode ?? 0, body: Buffer.concat(chunks).toString("utf8") }), + ); + }, + ); + req.on("error", reject); + req.end(); + }); +} + +describe("comment sync API — pull", () => { + it("imports a PR's review comments, then is idempotent on re-pull", async () => { + await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); + const runId = insertRun(); + const port = await start(); + + const first = await post(port, `/api/runs/${runId}/comment-sync/pull`); + expect(first.status).toBe(200); + expect(JSON.parse(first.body) as PullCommentsResult).toEqual({ pulled: 2, skipped: 0 }); + + const db = getDb({ dbPath }); + const rows = db.select().from(comment).all(); + expect(rows).toHaveLength(2); + const root = rows.find((r) => r.githubCommentId === 101); + expect(root?.authorId).toBe("octocat"); + expect(root?.authorAvatarUrl).toBe("https://example.com/octocat.png"); + expect(db.select().from(commentThread).all()).toHaveLength(1); + + const second = await post(port, `/api/runs/${runId}/comment-sync/pull`); + expect(JSON.parse(second.body) as PullCommentsResult).toEqual({ pulled: 0, skipped: 2 }); + expect(db.select().from(comment).all()).toHaveLength(2); + }); +}); + +describe("comment sync API — push guardrails", () => { + it("rejects pushing comments on a working-tree scope", async () => { + await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); + const runId = insertRun(SCOPE_KIND.WORKING_TREE); + const res = await post(await start(), `/api/runs/${runId}/comment-sync/push`); + expect(res.status).toBe(409); + expect(JSON.parse(res.body).error).toMatch(/committed diff/i); + }); + + it("rejects pushing with a dirty working tree", async () => { + await writeShims({ gitHead: HEAD_SHA, gitStatus: " M src/foo.ts" }); + const runId = insertRun(); + const res = await post(await start(), `/api/runs/${runId}/comment-sync/push`); + expect(res.status).toBe(409); + expect(JSON.parse(res.body).error).toMatch(/uncommitted changes/i); + }); + + it("rejects pushing when local HEAD doesn't match the PR head", async () => { + await writeShims({ gitHead: "f".repeat(40), gitStatus: "" }); + const runId = insertRun(); + const res = await post(await start(), `/api/runs/${runId}/comment-sync/push`); + expect(res.status).toBe(409); + expect(JSON.parse(res.body).error).toMatch(/HEAD doesn't match/i); + }); +}); + +describe("comment sync API — push", () => { + it("creates a review comment for a local thread, recording its GitHub id, then skips on re-push", async () => { + await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); + const runId = insertRun(); + seedLocalThread(); + const port = await start(); + + const first = await post(port, `/api/runs/${runId}/comment-sync/push`); + expect(first.status).toBe(200); + const firstResult = JSON.parse(first.body) as PushCommentsResult; + expect(firstResult.pushed).toBe(1); + expect(firstResult.skipped).toBe(0); + expect(firstResult.failed).toEqual([]); + + const db = getDb({ dbPath }); + const [row] = db.select().from(comment).all(); + expect(row?.githubCommentId).toBe(1001); + + const second = await post(port, `/api/runs/${runId}/comment-sync/push`); + const secondResult = JSON.parse(second.body) as PushCommentsResult; + expect(secondResult).toEqual({ pushed: 0, skipped: 1, failed: [] }); + + const log = await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8"); + expect(log.split("\n").filter((l) => l.startsWith("create"))).toHaveLength(1); + }); +}); diff --git a/packages/cli/src/__tests__/comments.routes.test.ts b/packages/cli/src/__tests__/comments.routes.test.ts index da07e67..15bec0a 100644 --- a/packages/cli/src/__tests__/comments.routes.test.ts +++ b/packages/cli/src/__tests__/comments.routes.test.ts @@ -133,7 +133,8 @@ describe("comment threads API", () => { expect(thread.resolvedAt).toBeNull(); expect(thread.comments).toHaveLength(1); expect(thread.comments[0]?.body).toBe("First!"); - expect(thread.comments[0]?.authorId).toBe("local"); + expect(thread.comments[0]?.author).toBeNull(); + expect(thread.comments[0]?.githubCommentId).toBeNull(); const db = getDb({ dbPath }); expect(db.select().from(commentThread).all()).toHaveLength(1); diff --git a/packages/cli/src/__tests__/review-comment-mapping.test.ts b/packages/cli/src/__tests__/review-comment-mapping.test.ts new file mode 100644 index 0000000..83e10e2 --- /dev/null +++ b/packages/cli/src/__tests__/review-comment-mapping.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from "vitest"; +import type { ReviewComment } from "../github/index.js"; +import { + fromGitHubSide, + groupReviewComments, + toGitHubSide, +} from "../runs/review-comment-mapping.js"; + +function comment(over: Partial & { id: number }): ReviewComment { + return { + in_reply_to_id: null, + path: "src/foo.ts", + line: 10, + start_line: null, + side: "RIGHT", + body: "body", + created_at: "2026-01-01T00:00:00Z", + user: { login: "octocat", avatar_url: "https://example.com/a.png", type: "User" }, + ...over, + }; +} + +describe("side mapping", () => { + it("maps local sides to GitHub diff sides", () => { + expect(toGitHubSide("deletions")).toBe("LEFT"); + expect(toGitHubSide("additions")).toBe("RIGHT"); + }); + + it("maps GitHub diff sides back to local sides, defaulting unknown to additions", () => { + expect(fromGitHubSide("LEFT")).toBe("deletions"); + expect(fromGitHubSide("RIGHT")).toBe("additions"); + expect(fromGitHubSide(null)).toBe("additions"); + expect(fromGitHubSide(undefined)).toBe("additions"); + }); +}); + +describe("groupReviewComments", () => { + it("nests replies under their root and orders them oldest-first", () => { + const threads = groupReviewComments([ + comment({ id: 1, body: "root" }), + comment({ + id: 3, + in_reply_to_id: 1, + body: "second reply", + created_at: "2026-01-03T00:00:00Z", + }), + comment({ + id: 2, + in_reply_to_id: 1, + body: "first reply", + created_at: "2026-01-02T00:00:00Z", + }), + ]); + expect(threads).toHaveLength(1); + expect(threads[0]?.root.id).toBe(1); + expect(threads[0]?.replies.map((r) => r.id)).toEqual([2, 3]); + }); + + it("derives the anchor from start_line/line and side", () => { + const [thread] = groupReviewComments([ + comment({ id: 1, side: "LEFT", start_line: 4, line: 8, path: "src/bar.ts" }), + ]); + expect(thread).toMatchObject({ + filePath: "src/bar.ts", + side: "deletions", + startLine: 4, + endLine: 8, + }); + }); + + it("falls back to line for single-line comments", () => { + const [thread] = groupReviewComments([comment({ id: 1, start_line: null, line: 12 })]); + expect(thread?.startLine).toBe(12); + expect(thread?.endLine).toBe(12); + }); + + it("drops comments with no anchorable line (outdated/whole-file)", () => { + const threads = groupReviewComments([comment({ id: 1, line: null })]); + expect(threads).toEqual([]); + }); +}); diff --git a/packages/cli/src/db/schema/comment.ts b/packages/cli/src/db/schema/comment.ts index 1a9783c..c7c9785 100644 --- a/packages/cli/src/db/schema/comment.ts +++ b/packages/cli/src/db/schema/comment.ts @@ -1,4 +1,4 @@ -import { index, sqliteTable, text } from "drizzle-orm/sqlite-core"; +import { index, integer, sqliteTable, text, uniqueIndex } from "drizzle-orm/sqlite-core"; import { LOCAL_USER_ID } from "../local-user.js"; import { baseColumns } from "./columns.js"; import { commentThread } from "./comment-thread.js"; @@ -10,10 +10,23 @@ export const comment = sqliteTable( threadId: text() .notNull() .references(() => commentThread.id, { onDelete: "cascade" }), + /** `local` for comments authored in the CLI; the GitHub login for pulled comments. */ authorId: text().notNull().default(LOCAL_USER_ID), + /** Avatar for non-local authors (pulled GitHub comments); null for the local user. */ + authorAvatarUrl: text(), body: text().notNull(), + /** + * GitHub review-comment database id once the comment is synced — set when a + * comment is pulled from the PR or after a local comment is pushed to it. + * Null marks a comment as local-only and not yet on GitHub. It's the dedup + * key for both directions, so re-syncing never duplicates a comment. + */ + githubCommentId: integer({ mode: "number" }), }, - (table) => [index("comment_thread_id_idx").on(table.threadId)], + (table) => [ + index("comment_thread_id_idx").on(table.threadId), + uniqueIndex("comment_github_comment_id_idx").on(table.githubCommentId), + ], ); export type CommentRow = typeof comment.$inferSelect; diff --git a/packages/cli/src/git.ts b/packages/cli/src/git.ts index cc13868..03d22bd 100644 --- a/packages/cli/src/git.ts +++ b/packages/cli/src/git.ts @@ -211,6 +211,23 @@ export function hasUncommittedChanges(): boolean { return out.length > 0; } +/** Current `HEAD` commit SHA of a specific worktree (used by the PR push guardrails). */ +export function readHeadSha(repoRoot: string): string { + return execFileSync("git", ["-C", repoRoot, "rev-parse", "HEAD"], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); +} + +/** True when a specific worktree has no staged, unstaged, or untracked changes. */ +export function isWorkingTreeClean(repoRoot: string): boolean { + const out = execFileSync("git", ["-C", repoRoot, "status", "--porcelain"], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + return out.length === 0; +} + export interface ResolvedScope { scope: Scope; mergeBaseSha: string; diff --git a/packages/cli/src/github/index.ts b/packages/cli/src/github/index.ts index c77aed6..c2546f8 100644 --- a/packages/cli/src/github/index.ts +++ b/packages/cli/src/github/index.ts @@ -17,4 +17,14 @@ export { resolvePullRequestRefs, } from "./pull-request-ref.js"; export { type GitHubRepo, isGitHubRemote, parseGitHubRepo } from "./repo.js"; +export { + type CreateReviewCommentInput, + createReviewComment, + GITHUB_DIFF_SIDE, + type GitHubDiffSide, + listResolvedRootCommentIds, + listReviewComments, + type ReviewComment, + replyToReviewComment, +} from "./review-comments.js"; export { type GitHubViewer, getGitHubViewer } from "./viewer.js"; diff --git a/packages/cli/src/github/review-comments.ts b/packages/cli/src/github/review-comments.ts new file mode 100644 index 0000000..21d0faf --- /dev/null +++ b/packages/cli/src/github/review-comments.ts @@ -0,0 +1,226 @@ +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; +import { z } from "zod"; +import { gh, ghErrorMessage } from "./exec.js"; +import type { GitHubRepo } from "./repo.js"; + +const execFileAsync = promisify(execFile); + +/** + * GitHub's diff sides. `LEFT` is the base/deletion side, `RIGHT` the head/addition + * side. These map onto the local `DIFF_SIDE` (deletions/additions) in the sync layer. + */ +export const GITHUB_DIFF_SIDE = { + LEFT: "LEFT", + RIGHT: "RIGHT", +} as const; +export type GitHubDiffSide = (typeof GITHUB_DIFF_SIDE)[keyof typeof GITHUB_DIFF_SIDE]; + +// REST review-comment shape we anchor on. `line` is null for an outdated comment +// (its line is no longer in the diff); `start_line` is null for single-line ones. +const ReviewCommentSchema = z.object({ + id: z.number(), + in_reply_to_id: z.number().nullable().optional(), + path: z.string(), + line: z.number().nullable(), + start_line: z.number().nullable().optional(), + side: z.enum(GITHUB_DIFF_SIDE).nullable().optional(), + body: z.string(), + created_at: z.string(), + user: z.object({ login: z.string(), avatar_url: z.string(), type: z.string() }).nullable(), +}); +export type ReviewComment = z.infer; + +/** + * All review comments on a PR, oldest-first across pages. Unlike the read + * adapters that back passive PR context, sync is user-initiated, so a `gh` + * failure throws rather than degrading to an empty list. + */ +export async function listReviewComments( + repoRoot: string, + repo: GitHubRepo, + prNumber: number, +): Promise { + // `--slurp` wraps each page in one array (`[[…], […]]`) so multi-page output stays valid JSON. + const stdout = await ghOrThrow( + ["api", `repos/${repo.owner}/${repo.repo}/pulls/${prNumber}/comments`, "--paginate", "--slurp"], + repoRoot, + ); + const parsed = z.array(z.array(ReviewCommentSchema)).safeParse(JSON.parse(stdout)); + if (!parsed.success) throw new Error("Unexpected response shape from GitHub review comments"); + return parsed.data.flat(); +} + +// ─── Resolved-thread metadata (GraphQL) ───────────────────────────────────────── + +const RESOLVED_THREADS_QUERY = `query GetResolvedThreads($owner: String!, $repo: String!, $number: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $cursor) { + pageInfo { hasNextPage endCursor } + nodes { + isResolved + comments(first: 1) { nodes { databaseId } } + } + } + } + } +}`; + +const ResolvedThreadsSchema = z.object({ + data: z.object({ + repository: z + .object({ + pullRequest: z + .object({ + reviewThreads: z.object({ + pageInfo: z.object({ hasNextPage: z.boolean(), endCursor: z.string().nullable() }), + nodes: z.array( + z.object({ + isResolved: z.boolean(), + comments: z.object({ + nodes: z.array(z.object({ databaseId: z.number().nullable() })), + }), + }), + ), + }), + }) + .nullable(), + }) + .nullable(), + }), +}); + +/** + * Set of root review-comment ids whose thread is resolved on GitHub, keyed by the + * root comment's database id (the same id the REST list reports for the thread's + * first comment). Used to mirror resolution state on pull. + */ +export async function listResolvedRootCommentIds( + repoRoot: string, + repo: GitHubRepo, + prNumber: number, +): Promise> { + const resolved = new Set(); + let cursor: string | null = null; + do { + const args = [ + "api", + "graphql", + "-f", + `query=${RESOLVED_THREADS_QUERY}`, + "-F", + `owner=${repo.owner}`, + "-F", + `repo=${repo.repo}`, + "-F", + `number=${prNumber}`, + ]; + if (cursor !== null) args.push("-F", `cursor=${cursor}`); + const stdout = await ghOrThrow(args, repoRoot); + const parsed = ResolvedThreadsSchema.safeParse(JSON.parse(stdout)); + if (!parsed.success) throw new Error("Unexpected response shape from GitHub review threads"); + const threads = parsed.data.data.repository?.pullRequest?.reviewThreads; + if (!threads) break; + for (const thread of threads.nodes) { + const rootId = thread.comments.nodes[0]?.databaseId; + if (thread.isResolved && rootId != null) resolved.add(rootId); + } + cursor = threads.pageInfo.hasNextPage ? threads.pageInfo.endCursor : null; + } while (cursor !== null); + return resolved; +} + +// ─── Writes ───────────────────────────────────────────────────────────────────── + +export interface CreateReviewCommentInput { + commitId: string; + path: string; + body: string; + side: GitHubDiffSide; + /** End line of the comment (single-line comments set only this). */ + line: number; + /** Set with `startSide` for a multi-line comment. */ + startLine?: number; + startSide?: GitHubDiffSide; +} + +const CreatedCommentSchema = z.object({ id: z.number() }); + +/** Create a new review comment on the PR, returning its GitHub id. Throws on failure. */ +export async function createReviewComment( + repoRoot: string, + repo: GitHubRepo, + prNumber: number, + input: CreateReviewCommentInput, +): Promise { + // `-f` sends a string field, `-F` a typed (numeric) one; together they form the + // JSON request body `gh api` POSTs. Each value is a single argv entry, so commit + // bodies with newlines or shell metacharacters pass through untouched. + const args = [ + "api", + `repos/${repo.owner}/${repo.repo}/pulls/${prNumber}/comments`, + "--method", + "POST", + "-f", + `body=${input.body}`, + "-f", + `commit_id=${input.commitId}`, + "-f", + `path=${input.path}`, + "-f", + `side=${input.side}`, + "-F", + `line=${input.line}`, + ]; + if (input.startLine !== undefined && input.startSide !== undefined) { + args.push("-F", `start_line=${input.startLine}`, "-f", `start_side=${input.startSide}`); + } + const stdout = await ghWrite(args, repoRoot); + return CreatedCommentSchema.parse(JSON.parse(stdout)).id; +} + +/** Reply to an existing review comment thread, returning the new comment's GitHub id. */ +export async function replyToReviewComment( + repoRoot: string, + repo: GitHubRepo, + prNumber: number, + inReplyToId: number, + body: string, +): Promise { + const stdout = await ghWrite( + [ + "api", + `repos/${repo.owner}/${repo.repo}/pulls/${prNumber}/comments/${inReplyToId}/replies`, + "--method", + "POST", + "-f", + `body=${body}`, + ], + repoRoot, + ); + return CreatedCommentSchema.parse(JSON.parse(stdout)).id; +} + +/** Read-only `gh` call that surfaces failures (sync is user-initiated, not passive context). */ +async function ghOrThrow(args: string[], repoRoot: string): Promise { + try { + return await gh(args, repoRoot); + } catch (err) { + throw new Error(ghErrorMessage(err)); + } +} + +/** Run a `gh` write command, returning stdout and surfacing failures with gh's stderr message. */ +async function ghWrite(args: string[], repoRoot: string): Promise { + try { + const { stdout } = await execFileAsync("gh", args, { + cwd: repoRoot, + encoding: "utf8", + maxBuffer: 10 * 1024 * 1024, + }); + return stdout; + } catch (err) { + throw new Error(ghErrorMessage(err)); + } +} diff --git a/packages/cli/src/routes/comments.ts b/packages/cli/src/routes/comments.ts index d171c51..c96d933 100644 --- a/packages/cli/src/routes/comments.ts +++ b/packages/cli/src/routes/comments.ts @@ -9,12 +9,14 @@ import { asc, eq, inArray } from "drizzle-orm"; import type { StageDb } from "../db/client.js"; import { LOCAL_USER_ID } from "../db/local-user.js"; import { + type ChapterRunRow, type CommentRow, type CommentThreadRow, chapterRun, comment, commentThread, } from "../db/schema/index.js"; +import { CommentSyncError, pullComments, pushComments } from "../runs/comment-sync.js"; import { deriveScopeKey } from "../runs/scope-key.js"; import type { Route } from "../server.js"; import { parseJsonBody, writeJson } from "./json.js"; @@ -205,9 +207,56 @@ export function commentRoutes(db: StageDb): Route[] { writeJson(res, 200, {}); }, }, + { + method: "POST", + pattern: "/api/runs/:runId/comment-sync/pull", + handler: (req, res, params) => { + if (!enforceSameOrigin(req, res)) return; + return runSync(db, params.runId, res, pullComments); + }, + }, + { + method: "POST", + pattern: "/api/runs/:runId/comment-sync/push", + handler: (req, res, params) => { + if (!enforceSameOrigin(req, res)) return; + return runSync(db, params.runId, res, pushComments); + }, + }, ]; } +type Res = Parameters[1]; + +/** Run a pull/push sync for a run, mapping CommentSyncError to its status and unexpected errors to 500. */ +async function runSync( + db: StageDb, + runId: string | undefined, + res: Res, + sync: (db: StageDb, run: ChapterRunRow) => Promise, +): Promise { + if (!runId) { + writeJson(res, 400, { error: "Missing runId" }); + return; + } + const [run] = db.select().from(chapterRun).where(eq(chapterRun.id, runId)).limit(1).all(); + if (!run) { + writeJson(res, 404, { error: `Run ${runId} not found` }); + return; + } + try { + writeJson(res, 200, await sync(db, run)); + } catch (err) { + if (err instanceof CommentSyncError) { + writeJson(res, err.status, { error: err.message }); + return; + } + writeJson(res, 500, { + error: err instanceof Error ? err.message : "Failed to sync comments with GitHub", + }); + } +} + function resolveRunScopeKey(db: StageDb, runId: string | undefined): string | null { if (!runId) return null; const [run] = db @@ -291,10 +340,15 @@ function toThreadDto(thread: CommentThreadRow, comments: CommentRow[]): CommentT } function toCommentDto(row: CommentRow): CommentDto { + // `local` comments render as the local reviewer ("You"); others carry the + // GitHub author pulled from the PR. + const author = + row.authorId === LOCAL_USER_ID ? null : { login: row.authorId, avatarUrl: row.authorAvatarUrl }; return { id: row.id, body: row.body, - authorId: row.authorId, + author, + githubCommentId: row.githubCommentId, createdAt: row.createdAt.toISOString(), updatedAt: row.updatedAt.toISOString(), }; diff --git a/packages/cli/src/runs/comment-sync.ts b/packages/cli/src/runs/comment-sync.ts new file mode 100644 index 0000000..33f1ba1 --- /dev/null +++ b/packages/cli/src/runs/comment-sync.ts @@ -0,0 +1,281 @@ +import type { PullCommentsResult, PushCommentsResult } from "@stagereview/types/comments"; +import { asc, eq } from "drizzle-orm"; +import type { StageDb } from "../db/client.js"; +import { LOCAL_USER_ID } from "../db/local-user.js"; +import { type ChapterRunRow, type CommentRow, comment, commentThread } from "../db/schema/index.js"; +import { isWorkingTreeClean, readHeadSha } from "../git.js"; +import { + createReviewComment, + type GitHubRepo, + getPullRequest, + listResolvedRootCommentIds, + listReviewComments, + parseGitHubRepo, + type ReviewComment, + replyToReviewComment, +} from "../github/index.js"; +import { SCOPE_KIND } from "../schema.js"; +import { groupReviewComments, toGitHubSide } from "./review-comment-mapping.js"; +import { deriveScopeKey } from "./scope-key.js"; + +/** + * A sync failure with a user-facing message and the HTTP status the route should + * return. Guardrail violations (409) and missing-PR errors (404) reach the user + * verbatim, so the UI can explain exactly why a sync didn't run. + */ +export class CommentSyncError extends Error { + constructor( + message: string, + readonly status: number, + ) { + super(message); + this.name = "CommentSyncError"; + } +} + +interface SyncTarget { + repo: GitHubRepo; + prNumber: number; + headSha: string; +} + +/** + * Resolve the GitHub PR a run targets, including its current head SHA. Throws a + * CommentSyncError when the run has no GitHub remote or no detectable PR — both + * are conditions the user needs to see, not silent no-ops. + */ +async function resolveSyncTarget(run: ChapterRunRow): Promise { + const repo = parseGitHubRepo(run.originUrl); + if (!repo) { + throw new CommentSyncError("This run isn't associated with a GitHub remote.", 404); + } + const pullRequest = await getPullRequest(run.repoRoot, run.originUrl, run.prNumber); + if (!pullRequest) { + throw new CommentSyncError( + "No GitHub pull request found for this run. Ensure `gh` is authenticated and the branch has an open PR.", + 404, + ); + } + return { repo, prNumber: pullRequest.number, headSha: pullRequest.head.sha }; +} + +// ─── Pull (GitHub → local) ────────────────────────────────────────────────────── + +/** + * Import the PR's review comments into the run's local review. Idempotent: every + * comment is keyed by its GitHub id, so re-pulling skips comments already present + * and never duplicates. Resolved threads on GitHub mark their local thread + * resolved (non-destructive — locally-reopened threads aren't forced back closed). + */ +export async function pullComments(db: StageDb, run: ChapterRunRow): Promise { + const { repo, prNumber } = await resolveSyncTarget(run); + const scopeKey = deriveScopeKey(run); + const [comments, resolvedRootIds] = await Promise.all([ + listReviewComments(run.repoRoot, repo, prNumber), + listResolvedRootCommentIds(run.repoRoot, repo, prNumber), + ]); + const threads = groupReviewComments(comments); + + return db.transaction((tx) => { + let pulled = 0; + let skipped = 0; + + const insertComment = (threadId: string, c: ReviewComment): boolean => { + const existing = tx + .select({ id: comment.id }) + .from(comment) + .where(eq(comment.githubCommentId, c.id)) + .limit(1) + .all(); + if (existing.length > 0) return false; + tx.insert(comment) + .values({ + threadId, + authorId: c.user?.login ?? "ghost", + authorAvatarUrl: c.user?.avatar_url ?? null, + body: c.body, + githubCommentId: c.id, + }) + .run(); + return true; + }; + + for (const thread of threads) { + // Reuse the local thread that already owns the root comment; otherwise create one. + const [existingRoot] = tx + .select({ threadId: comment.threadId }) + .from(comment) + .where(eq(comment.githubCommentId, thread.root.id)) + .limit(1) + .all(); + + let threadId: string; + if (existingRoot) { + threadId = existingRoot.threadId; + skipped++; + } else { + const resolvedAt = resolvedRootIds.has(thread.root.id) ? new Date() : null; + const [threadRow] = tx + .insert(commentThread) + .values({ + scopeKey, + filePath: thread.filePath, + side: thread.side, + startLine: thread.startLine, + endLine: thread.endLine, + resolvedAt, + }) + .returning({ id: commentThread.id }) + .all(); + if (!threadRow) throw new Error("comment_thread insert returned no row"); + threadId = threadRow.id; + insertComment(threadId, thread.root); + pulled++; + } + + for (const reply of thread.replies) { + if (insertComment(threadId, reply)) pulled++; + else skipped++; + } + } + + return { pulled, skipped }; + }); +} + +// ─── Push (local → GitHub) ──────────────────────────────────────────────────────── + +/** + * Block the push unless the local checkout safely matches the PR. PR review + * comments anchor to committed diff positions, so a working-tree scope, a dirty + * tree, or a head that has diverged from the PR would all land comments + * mis-anchored. These are loud failures, not silent skips. + */ +function assertPushable(run: ChapterRunRow, target: SyncTarget): void { + if (run.scopeKind !== SCOPE_KIND.COMMITTED) { + throw new CommentSyncError( + "Only comments on a committed diff can be pushed. Working-tree comments aren't anchored to commits.", + 409, + ); + } + if (!isWorkingTreeClean(run.repoRoot)) { + throw new CommentSyncError( + "Your working tree has uncommitted changes. Commit or stash them so comments anchor to the pushed commit.", + 409, + ); + } + const localHead = readHeadSha(run.repoRoot); + if (localHead !== target.headSha) { + throw new CommentSyncError( + "Your local HEAD doesn't match the PR head. Push or pull your commits so they line up before syncing.", + 409, + ); + } +} + +interface ThreadWithComments { + thread: typeof commentThread.$inferSelect; + comments: CommentRow[]; +} + +function loadThreads(db: StageDb, scopeKey: string): ThreadWithComments[] { + const threads = db + .select() + .from(commentThread) + .where(eq(commentThread.scopeKey, scopeKey)) + .orderBy(asc(commentThread.createdAt)) + .all(); + return threads.map((thread) => ({ + thread, + comments: db + .select() + .from(comment) + .where(eq(comment.threadId, thread.id)) + .orderBy(asc(comment.createdAt)) + .all(), + })); +} + +/** + * Push locally-authored comments to the PR. Comments pulled from GitHub or already + * synced are skipped; each new comment records its GitHub id on success so a later + * push or pull treats it as already-synced. A comment GitHub rejects (e.g. its line + * isn't in the PR diff) is reported as a per-comment failure without aborting the rest. + */ +export async function pushComments(db: StageDb, run: ChapterRunRow): Promise { + const target = await resolveSyncTarget(run); + assertPushable(run, target); + + const result: PushCommentsResult = { pushed: 0, skipped: 0, failed: [] }; + const scopeKey = deriveScopeKey(run); + + const recordSynced = (commentId: string, githubCommentId: number): void => { + db.update(comment).set({ githubCommentId }).where(eq(comment.id, commentId)).run(); + }; + + for (const { thread, comments } of loadThreads(db, scopeKey)) { + const side = toGitHubSide(thread.side); + // GitHub multi-line comments need a start anchor only when the range spans lines. + const startLine = thread.endLine !== thread.startLine ? thread.startLine : undefined; + // Track the root's GitHub id in memory so a reply pushed in the same pass can anchor to it. + const rootGithubId = comments[0]?.githubCommentId ?? null; + let liveRootGithubId = rootGithubId; + + for (let i = 0; i < comments.length; i++) { + const c = comments[i]; + if (!c) continue; + // Only locally-authored comments are ours to push; GitHub-authored ones came from the PR. + if (c.authorId !== LOCAL_USER_ID) { + if (i === 0) liveRootGithubId = c.githubCommentId; + continue; + } + if (c.githubCommentId !== null) { + result.skipped++; + if (i === 0) liveRootGithubId = c.githubCommentId; + continue; + } + + try { + if (i === 0) { + const id = await createReviewComment(run.repoRoot, target.repo, target.prNumber, { + commitId: target.headSha, + path: thread.filePath, + body: c.body, + side, + line: thread.endLine, + startLine, + startSide: startLine !== undefined ? side : undefined, + }); + recordSynced(c.id, id); + liveRootGithubId = id; + result.pushed++; + } else if (liveRootGithubId !== null) { + const id = await replyToReviewComment( + run.repoRoot, + target.repo, + target.prNumber, + liveRootGithubId, + c.body, + ); + recordSynced(c.id, id); + result.pushed++; + } else { + result.failed.push({ + filePath: thread.filePath, + line: thread.endLine, + message: + "The thread's first comment wasn't pushed, so the reply has nothing to anchor to.", + }); + } + } catch (err) { + result.failed.push({ + filePath: thread.filePath, + line: thread.endLine, + message: err instanceof Error ? err.message : "Failed to push comment to GitHub.", + }); + } + } + } + + return result; +} diff --git a/packages/cli/src/runs/review-comment-mapping.ts b/packages/cli/src/runs/review-comment-mapping.ts new file mode 100644 index 0000000..880483b --- /dev/null +++ b/packages/cli/src/runs/review-comment-mapping.ts @@ -0,0 +1,55 @@ +import { GITHUB_DIFF_SIDE, type GitHubDiffSide, type ReviewComment } from "../github/index.js"; +import { DIFF_SIDE, type DiffSide } from "../schema.js"; + +// LEFT is GitHub's base/deletion side, RIGHT the head/addition side. +export function toGitHubSide(side: DiffSide): GitHubDiffSide { + return side === DIFF_SIDE.DELETIONS ? GITHUB_DIFF_SIDE.LEFT : GITHUB_DIFF_SIDE.RIGHT; +} + +export function fromGitHubSide(side: GitHubDiffSide | null | undefined): DiffSide { + return side === GITHUB_DIFF_SIDE.LEFT ? DIFF_SIDE.DELETIONS : DIFF_SIDE.ADDITIONS; +} + +export interface PulledThread { + root: ReviewComment; + replies: ReviewComment[]; + filePath: string; + side: DiffSide; + startLine: number; + endLine: number; +} + +/** + * Group flat review comments into threads. GitHub sets `in_reply_to_id` on every + * reply, pointing at the thread's root. Comments without an anchorable line + * (outdated, or whole-file) are dropped — the local model is line-anchored. + */ +export function groupReviewComments(comments: ReviewComment[]): PulledThread[] { + const repliesByRoot = new Map(); + const roots: ReviewComment[] = []; + for (const c of comments) { + if (c.in_reply_to_id != null) { + const list = repliesByRoot.get(c.in_reply_to_id); + if (list) list.push(c); + else repliesByRoot.set(c.in_reply_to_id, [c]); + } else { + roots.push(c); + } + } + const threads: PulledThread[] = []; + for (const root of roots) { + if (root.line == null) continue; + const replies = (repliesByRoot.get(root.id) ?? []).sort((a, b) => + a.created_at.localeCompare(b.created_at), + ); + threads.push({ + root, + replies, + filePath: root.path, + side: fromGitHubSide(root.side), + startLine: root.start_line ?? root.line, + endLine: root.line, + }); + } + return threads; +} diff --git a/packages/types/src/comments.ts b/packages/types/src/comments.ts index 845a2d9..5fb054d 100644 --- a/packages/types/src/comments.ts +++ b/packages/types/src/comments.ts @@ -1,14 +1,25 @@ import { z } from "zod"; import { DIFF_SIDE } from "./chapters.ts"; +// Author of a pulled GitHub comment. Local comments carry `author: null`, which +// the UI renders as the local reviewer ("You"). +export const CommentAuthorSchema = z.object({ + login: z.string(), + avatarUrl: z.string().nullable(), +}); +export type CommentAuthor = z.infer; + // A single authored comment. Replies are sibling comments sharing a thread, so a // comment carries no positional data of its own — the thread owns the anchor. +// `author` is null for locally-authored comments and set for ones pulled from a +// GitHub PR. `githubCommentId` is non-null once the comment is synced to the PR. // Non-strict (like the other wire response schemas) so the server can add fields // the SPA doesn't yet read without the response failing to parse. export const CommentSchema = z.object({ id: z.string(), body: z.string(), - authorId: z.string(), + author: CommentAuthorSchema.nullable(), + githubCommentId: z.number().int().nullable(), createdAt: z.string(), updatedAt: z.string(), }); @@ -58,3 +69,33 @@ export const ResolveThreadBodySchema = z.object({ resolved: z.boolean(), }); export type ResolveThreadBody = z.infer; + +// ─── GitHub sync ────────────────────────────────────────────────────────────── + +// Outcome of importing PR review comments into the local review. +export const PullCommentsResultSchema = z.object({ + // New local comments created from the PR. + pulled: z.number().int().nonnegative(), + // PR comments already present locally (matched by GitHub id), left untouched. + skipped: z.number().int().nonnegative(), +}); +export type PullCommentsResult = z.infer; + +// A local comment that couldn't be pushed, with the reason surfaced to the user. +export const PushCommentFailureSchema = z.object({ + filePath: z.string(), + line: z.number().int(), + message: z.string(), +}); +export type PushCommentFailure = z.infer; + +// Outcome of pushing locally-authored comments to the PR. +export const PushCommentsResultSchema = z.object({ + // Local comments created on the PR by this push. + pushed: z.number().int().nonnegative(), + // Local comments already on the PR (already had a GitHub id), left untouched. + skipped: z.number().int().nonnegative(), + // Per-comment failures (e.g. line not in the PR diff). + failed: z.array(PushCommentFailureSchema), +}); +export type PushCommentsResult = z.infer; From ffbd8f39996883dc46e5edb5709e0ba88e45cc6e Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 22 Jun 2026 11:31:12 +0200 Subject: [PATCH 02/17] feat(PRO-547): add GitHub comment sync UI and author rendering --- packages/cli/src/runs/comment-sync.ts | 7 ++ .../components/comments/comment-thread.tsx | 46 ++++--- .../pull-request/comment-sync-menu.tsx | 112 ++++++++++++++++++ packages/web/src/lib/use-comment-sync.ts | 71 +++++++++++ .../web/src/routes/pull-request-layout.tsx | 2 + 5 files changed, 220 insertions(+), 18 deletions(-) create mode 100644 packages/web/src/components/pull-request/comment-sync-menu.tsx create mode 100644 packages/web/src/lib/use-comment-sync.ts diff --git a/packages/cli/src/runs/comment-sync.ts b/packages/cli/src/runs/comment-sync.ts index 33f1ba1..dfb1b9f 100644 --- a/packages/cli/src/runs/comment-sync.ts +++ b/packages/cli/src/runs/comment-sync.ts @@ -201,6 +201,13 @@ function loadThreads(db: StageDb, scopeKey: string): ThreadWithComments[] { * synced are skipped; each new comment records its GitHub id on success so a later * push or pull treats it as already-synced. A comment GitHub rejects (e.g. its line * isn't in the PR diff) is reported as a per-comment failure without aborting the rest. + * + * Known limitation: a deletion-side comment authored on a chapter view anchors to + * that view's synthetic intermediate-file line numbers, which can differ from the + * PR's canonical old-line coordinates. Addition-side anchors are always canonical. + * Canonicalizing deletion-side anchors is deferred; GitHub's own "line not in diff" + * rejection is the backstop here, surfacing such a comment as a loud failure rather + * than letting it land mis-anchored silently. */ export async function pushComments(db: StageDb, run: ChapterRunRow): Promise { const target = await resolveSyncTarget(run); diff --git a/packages/web/src/components/comments/comment-thread.tsx b/packages/web/src/components/comments/comment-thread.tsx index f1f2104..888c68d 100644 --- a/packages/web/src/components/comments/comment-thread.tsx +++ b/packages/web/src/components/comments/comment-thread.tsx @@ -115,7 +115,7 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { {isOpen ? "Collapse thread" : "Expand thread"} - + {idle && (
@@ -136,15 +136,20 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { Reply - { - setIsOpen(true); - setError(null); - setEditingId(root.id); - }} - onDelete={() => setDeleteTarget({ kind: "thread", hasReplies: replies.length > 0 })} - deleteLabel={replies.length > 0 ? "Delete thread" : "Delete"} - /> + {/* Pulled GitHub comments are read-only locally; only local comments can be edited/deleted. */} + {root.author === null && ( + { + setIsOpen(true); + setError(null); + setEditingId(root.id); + }} + onDelete={() => + setDeleteTarget({ kind: "thread", hasReplies: replies.length > 0 }) + } + deleteLabel={replies.length > 0 ? "Delete thread" : "Delete"} + /> + )}
)} @@ -236,19 +241,23 @@ function ResolveButton({ isResolved, onToggle }: { isResolved: boolean; onToggle ); } -function CommentByline({ createdAt }: { createdAt: string }) { +// A local comment (`author: null`) renders as the local reviewer; a comment pulled +// from the PR renders its GitHub author. +function CommentByline({ comment }: { comment: Comment }) { const viewer = useViewer(); + const name = comment.author?.login ?? viewer.name; + const avatarUrl = comment.author ? comment.author.avatarUrl : viewer.avatarUrl; return (

- {viewer.avatarUrl && } + {avatarUrl && } - {viewer.name} -

); @@ -276,10 +285,11 @@ function ReplyItem({ return (
- + {/* Only when the whole thread is idle, so opening this reply's editor can't - discard another in-progress edit or reply (matches the root comment). */} - {idle && } + discard another in-progress edit or reply (matches the root comment). + Pulled GitHub replies are read-only locally. */} + {idle && reply.author === null && }
{isEditing ? ( 0 ? "All PR comments are already imported" : "No review comments on the PR", + ); + return; + } + const extra = r.skipped > 0 ? ` (${r.skipped} already imported)` : ""; + toast.success(`Imported ${r.pulled} comment${plural(r.pulled)} from the PR${extra}`); +} + +function toastPushResult(r: PushCommentsResult): void { + if (r.failed.length > 0) { + const pushedMsg = r.pushed > 0 ? `${r.pushed} pushed, ` : ""; + toast.error(`${pushedMsg}${r.failed.length} failed`, { + description: r.failed.map((f) => `${f.filePath}:${f.line} — ${f.message}`).join("\n"), + }); + return; + } + if (r.pushed === 0) { + toast.info(r.skipped > 0 ? "All comments are already on the PR" : "No local comments to push"); + return; + } + const extra = r.skipped > 0 ? ` (${r.skipped} already on the PR)` : ""; + toast.success(`Pushed ${r.pushed} comment${plural(r.pushed)} to the PR${extra}`); +} + +function errorMessage(err: unknown, fallback: string): string { + return err instanceof Error ? err.message : fallback; +} + +/** + * Pull/push controls for syncing the run's comments with its GitHub PR. Only the + * push path enforces guardrails server-side; both surface their outcome — imported, + * skipped, or failed — as a toast so the user always sees what happened. + */ +export function CommentSyncMenu({ runId }: { runId: string }) { + const { pull, push, isPulling, isPushing } = useCommentSync(runId); + const isBusy = isPulling || isPushing; + + async function handlePull() { + try { + toastPullResult(await pull()); + } catch (err) { + toast.error(errorMessage(err, "Failed to import comments from the PR")); + } + } + + async function handlePush() { + try { + toastPushResult(await push()); + } catch (err) { + toast.error(errorMessage(err, "Failed to push comments to the PR")); + } + } + + return ( + + + + + + + + Sync comments with GitHub + + + + + Pull comments from PR + + + + Push comments to PR + + + + ); +} diff --git a/packages/web/src/lib/use-comment-sync.ts b/packages/web/src/lib/use-comment-sync.ts new file mode 100644 index 0000000..9523435 --- /dev/null +++ b/packages/web/src/lib/use-comment-sync.ts @@ -0,0 +1,71 @@ +import { + type PullCommentsResult, + PullCommentsResultSchema, + type PushCommentsResult, + PushCommentsResultSchema, +} from "@stagereview/types/comments"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { commentThreadsQueryKey } from "./use-comment-threads"; + +export type { PullCommentsResult, PushCommentsResult }; + +/** + * POST a sync endpoint, surfacing the server's `{ error }` message verbatim on a + * non-2xx response (guardrail failures and the missing-PR case all carry one). + * The generic `jsonFetch` only reports the status code, which would hide the + * actionable reason a sync was refused. + */ +async function syncFetch(url: string): Promise { + const res = await fetch(url, { method: "POST" }); + const text = await res.text(); + const parsed: unknown = text ? JSON.parse(text) : {}; + if (!res.ok) { + const message = + typeof parsed === "object" && parsed !== null && "error" in parsed + ? String((parsed as { error: unknown }).error) + : `Sync failed (${res.status})`; + throw new Error(message); + } + return parsed as T; +} + +export interface UseCommentSyncResult { + pull: () => Promise; + push: () => Promise; + isPulling: boolean; + isPushing: boolean; +} + +/** + * Pull/push mutations that sync the run's comments with its GitHub PR. Both + * invalidate the comment-threads query on success so freshly imported or + * id-stamped comments render without a manual refresh. + */ +export function useCommentSync(runId: string): UseCommentSyncResult { + const queryClient = useQueryClient(); + const invalidate = () => + queryClient.invalidateQueries({ queryKey: commentThreadsQueryKey(runId) }); + + const pullMutation = useMutation({ + mutationFn: () => + syncFetch(`/api/runs/${encodeURIComponent(runId)}/comment-sync/pull`).then((raw) => + PullCommentsResultSchema.parse(raw), + ), + onSuccess: invalidate, + }); + + const pushMutation = useMutation({ + mutationFn: () => + syncFetch(`/api/runs/${encodeURIComponent(runId)}/comment-sync/push`).then((raw) => + PushCommentsResultSchema.parse(raw), + ), + onSuccess: invalidate, + }); + + return { + pull: pullMutation.mutateAsync, + push: pushMutation.mutateAsync, + isPulling: pullMutation.isPending, + isPushing: pushMutation.isPending, + }; +} diff --git a/packages/web/src/routes/pull-request-layout.tsx b/packages/web/src/routes/pull-request-layout.tsx index 5b3fad2..3fb085b 100644 --- a/packages/web/src/routes/pull-request-layout.tsx +++ b/packages/web/src/routes/pull-request-layout.tsx @@ -2,6 +2,7 @@ import { Link, Outlet, useRouterState } from "@tanstack/react-router"; import { BookOpen, FileText, FoldVertical, Settings2, UnfoldVertical } from "lucide-react"; import { type CSSProperties, useCallback, useMemo, useRef, useState } from "react"; import { DiffSettingsForm } from "@/components/diff/diff-settings-form"; +import { CommentSyncMenu } from "@/components/pull-request/comment-sync-menu"; import { PullRequestHeader } from "@/components/pull-request/pull-request-header"; import { PullRequestHeaderSkeleton } from "@/components/pull-request/pull-request-header-skeleton"; import { SectionLabel } from "@/components/pull-request/section-label"; @@ -279,6 +280,7 @@ export function PullRequestLayout({ runId }: { runId: string }) {
+ {pullRequest !== null && } From 559ee095b23b4e754b4edb290e097a1a828145f6 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 22 Jun 2026 13:01:15 +0200 Subject: [PATCH 03/17] feat(PRO-547): mirror PR resolved state on pull --- .../src/__tests__/comment-sync.routes.test.ts | 27 ++++++++++++++-- packages/cli/src/runs/comment-sync.ts | 32 ++++++++++++++----- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/__tests__/comment-sync.routes.test.ts b/packages/cli/src/__tests__/comment-sync.routes.test.ts index 2e3fa1f..176b875 100644 --- a/packages/cli/src/__tests__/comment-sync.routes.test.ts +++ b/packages/cli/src/__tests__/comment-sync.routes.test.ts @@ -51,7 +51,11 @@ const REVIEW_COMMENTS = [ // `gh`/`git` shims that route on argv and emit canned JSON, so the sync paths run // end-to-end without network or a real repo. Infrastructure fakes, not mocks. -async function writeShims(opts: { gitHead: string; gitStatus: string }): Promise { +async function writeShims(opts: { + gitHead: string; + gitStatus: string; + resolvedRootIds?: number[]; +}): Promise { const ghFixture = { pr: { number: 5, @@ -77,7 +81,10 @@ async function writeShims(opts: { gitHead: string; gitStatus: string }): Promise pullRequest: { reviewThreads: { pageInfo: { hasNextPage: false, endCursor: null }, - nodes: [], + nodes: (opts.resolvedRootIds ?? []).map((id) => ({ + isResolved: true, + comments: { nodes: [{ databaseId: id }] }, + })), }, }, }, @@ -239,6 +246,22 @@ describe("comment sync API — pull", () => { expect(JSON.parse(second.body) as PullCommentsResult).toEqual({ pulled: 0, skipped: 2 }); expect(db.select().from(comment).all()).toHaveLength(2); }); + + it("mirrors the PR's resolved state on pull — resolving, then reopening", async () => { + // Thread 101 starts resolved on GitHub. + await writeShims({ gitHead: HEAD_SHA, gitStatus: "", resolvedRootIds: [101] }); + const runId = insertRun(); + const port = await start(); + + await post(port, `/api/runs/${runId}/comment-sync/pull`); + const db = getDb({ dbPath }); + expect(db.select().from(commentThread).all()[0]?.resolvedAt).not.toBeNull(); + + // Reopened on GitHub since the last pull → the next pull reflects that locally. + await writeShims({ gitHead: HEAD_SHA, gitStatus: "", resolvedRootIds: [] }); + await post(port, `/api/runs/${runId}/comment-sync/pull`); + expect(db.select().from(commentThread).all()[0]?.resolvedAt).toBeNull(); + }); }); describe("comment sync API — push guardrails", () => { diff --git a/packages/cli/src/runs/comment-sync.ts b/packages/cli/src/runs/comment-sync.ts index dfb1b9f..7e3456a 100644 --- a/packages/cli/src/runs/comment-sync.ts +++ b/packages/cli/src/runs/comment-sync.ts @@ -64,8 +64,9 @@ async function resolveSyncTarget(run: ChapterRunRow): Promise { /** * Import the PR's review comments into the run's local review. Idempotent: every * comment is keyed by its GitHub id, so re-pulling skips comments already present - * and never duplicates. Resolved threads on GitHub mark their local thread - * resolved (non-destructive — locally-reopened threads aren't forced back closed). + * and never duplicates. A pull mirrors the PR's resolved state onto the threads it + * owns — resolving or reopening to match GitHub — while purely-local threads (never + * pushed, so absent from GitHub) are left untouched. */ export async function pullComments(db: StageDb, run: ChapterRunRow): Promise { const { repo, prNumber } = await resolveSyncTarget(run); @@ -101,20 +102,35 @@ export async function pullComments(db: StageDb, run: ChapterRunRow): Promise Date: Mon, 22 Jun 2026 13:16:15 +0200 Subject: [PATCH 04/17] feat(PRO-547): sync comment resolution back to GitHub --- .../src/__tests__/comment-sync.routes.test.ts | 110 ++++++++++++++++-- .../cli/src/__tests__/comments.routes.test.ts | 4 +- packages/cli/src/github/index.ts | 4 +- packages/cli/src/github/review-comments.ts | 65 ++++++++--- packages/cli/src/routes/comments.ts | 41 +++++-- packages/cli/src/runs/comment-sync.ts | 74 +++++++++++- .../components/comments/comment-thread.tsx | 14 ++- packages/web/src/lib/use-comment-sync.ts | 33 ++---- packages/web/src/lib/use-comment-threads.ts | 3 +- packages/web/src/lib/use-view-state.ts | 17 ++- 10 files changed, 290 insertions(+), 75 deletions(-) diff --git a/packages/cli/src/__tests__/comment-sync.routes.test.ts b/packages/cli/src/__tests__/comment-sync.routes.test.ts index 176b875..a70e32d 100644 --- a/packages/cli/src/__tests__/comment-sync.routes.test.ts +++ b/packages/cli/src/__tests__/comment-sync.routes.test.ts @@ -81,10 +81,15 @@ async function writeShims(opts: { pullRequest: { reviewThreads: { pageInfo: { hasNextPage: false, endCursor: null }, - nodes: (opts.resolvedRootIds ?? []).map((id) => ({ - isResolved: true, - comments: { nodes: [{ databaseId: id }] }, - })), + // One thread node per root comment, carrying its node id (for resolve/reopen) + // and resolution state (for pull mirroring). + nodes: REVIEW_COMMENTS.flat() + .filter((c) => c.in_reply_to_id == null) + .map((c) => ({ + id: `THREAD_NODE_${c.id}`, + isResolved: (opts.resolvedRootIds ?? []).includes(c.id), + comments: { nodes: [{ databaseId: c.id }] }, + })), }, }, }, @@ -110,7 +115,16 @@ function nextId() { if (args[0] === "pr" && args[1] === "view") { process.stdout.write(JSON.stringify(fx.pr)); } else if (args[0] === "api" && args[1] === "graphql") { - process.stdout.write(JSON.stringify(fx.graphql)); + const joined = args.join(" "); + if (joined.includes("unresolveReviewThread")) { + fs.appendFileSync(log, "unresolve " + joined + "\\n"); + process.stdout.write("{}"); + } else if (joined.includes("resolveReviewThread")) { + fs.appendFileSync(log, "resolve " + joined + "\\n"); + process.stdout.write("{}"); + } else { + process.stdout.write(JSON.stringify(fx.graphql)); + } } else if (args[0] === "api") { const endpoint = args[1]; const isPost = args.includes("POST"); @@ -207,10 +221,26 @@ async function start(): Promise { return handle.port; } -function post(port: number, p: string): Promise<{ status: number; body: string }> { +function request( + port: number, + method: string, + p: string, + body?: unknown, +): Promise<{ status: number; body: string }> { return new Promise((resolve, reject) => { + const payload = body === undefined ? undefined : JSON.stringify(body); const req = http.request( - { hostname: LOOPBACK_HOST, port, method: "POST", path: p, agent: false }, + { + hostname: LOOPBACK_HOST, + port, + method, + path: p, + agent: false, + headers: + payload === undefined + ? {} + : { "Content-Type": "application/json", "Content-Length": Buffer.byteLength(payload) }, + }, (res) => { const chunks: Buffer[] = []; res.on("data", (c: Buffer) => chunks.push(c)); @@ -220,10 +250,14 @@ function post(port: number, p: string): Promise<{ status: number; body: string } }, ); req.on("error", reject); - req.end(); + req.end(payload); }); } +function post(port: number, p: string): Promise<{ status: number; body: string }> { + return request(port, "POST", p); +} + describe("comment sync API — pull", () => { it("imports a PR's review comments, then is idempotent on re-pull", async () => { await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); @@ -316,3 +350,63 @@ describe("comment sync API — push", () => { expect(log.split("\n").filter((l) => l.startsWith("create"))).toHaveLength(1); }); }); + +describe("comment sync API — resolution", () => { + it("mirrors a local resolve/reopen of a PR-originated thread to GitHub", async () => { + await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); + const runId = insertRun(); + const port = await start(); + await post(port, `/api/runs/${runId}/comment-sync/pull`); + + const db = getDb({ dbPath }); + const [thread] = db.select().from(commentThread).all(); + if (!thread) throw new Error("expected a pulled thread"); + + const resolved = await request( + port, + "PATCH", + `/api/runs/${runId}/comment-threads/${thread.id}`, + { + resolved: true, + }, + ); + expect(resolved.status).toBe(200); + expect(db.select().from(commentThread).all()[0]?.resolvedAt).not.toBeNull(); + + const reopened = await request( + port, + "PATCH", + `/api/runs/${runId}/comment-threads/${thread.id}`, + { + resolved: false, + }, + ); + expect(reopened.status).toBe(200); + expect(db.select().from(commentThread).all()[0]?.resolvedAt).toBeNull(); + + const lines = (await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8")).split("\n"); + expect(lines.filter((l) => l.startsWith("resolve"))).toHaveLength(1); + expect(lines.filter((l) => l.startsWith("unresolve"))).toHaveLength(1); + }); + + it("keeps a local-only thread's resolve off GitHub", async () => { + await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); + const runId = insertRun(); + seedLocalThread(); + const port = await start(); + + const db = getDb({ dbPath }); + const [thread] = db.select().from(commentThread).all(); + if (!thread) throw new Error("expected a local thread"); + + const res = await request(port, "PATCH", `/api/runs/${runId}/comment-threads/${thread.id}`, { + resolved: true, + }); + expect(res.status).toBe(200); + expect(db.select().from(commentThread).all()[0]?.resolvedAt).not.toBeNull(); + + // No GitHub mutation for a thread that never lived on the PR. + const log = await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8").catch(() => ""); + expect(log).not.toMatch(/resolve/); + }); +}); diff --git a/packages/cli/src/__tests__/comments.routes.test.ts b/packages/cli/src/__tests__/comments.routes.test.ts index 15bec0a..7acd492 100644 --- a/packages/cli/src/__tests__/comments.routes.test.ts +++ b/packages/cli/src/__tests__/comments.routes.test.ts @@ -165,12 +165,12 @@ describe("comment threads API", () => { const { port } = await startWithRoutes(); const thread = await createThread(port, runId); - const resolved = await send(port, "PATCH", `/api/comment-threads/${thread.id}`, { + const resolved = await send(port, "PATCH", `/api/runs/${runId}/comment-threads/${thread.id}`, { resolved: true, }); expect((resolved.body as CommentThread).resolvedAt).not.toBeNull(); - const reopened = await send(port, "PATCH", `/api/comment-threads/${thread.id}`, { + const reopened = await send(port, "PATCH", `/api/runs/${runId}/comment-threads/${thread.id}`, { resolved: false, }); expect((reopened.body as CommentThread).resolvedAt).toBeNull(); diff --git a/packages/cli/src/github/index.ts b/packages/cli/src/github/index.ts index c2546f8..7ab0437 100644 --- a/packages/cli/src/github/index.ts +++ b/packages/cli/src/github/index.ts @@ -22,9 +22,11 @@ export { createReviewComment, GITHUB_DIFF_SIDE, type GitHubDiffSide, - listResolvedRootCommentIds, listReviewComments, + listReviewThreads, type ReviewComment, + type ReviewThreadInfo, replyToReviewComment, + setReviewThreadResolved, } from "./review-comments.js"; export { type GitHubViewer, getGitHubViewer } from "./viewer.js"; diff --git a/packages/cli/src/github/review-comments.ts b/packages/cli/src/github/review-comments.ts index 21d0faf..5bca1f2 100644 --- a/packages/cli/src/github/review-comments.ts +++ b/packages/cli/src/github/review-comments.ts @@ -51,14 +51,15 @@ export async function listReviewComments( return parsed.data.flat(); } -// ─── Resolved-thread metadata (GraphQL) ───────────────────────────────────────── +// ─── Review-thread metadata (GraphQL) ─────────────────────────────────────────── -const RESOLVED_THREADS_QUERY = `query GetResolvedThreads($owner: String!, $repo: String!, $number: Int!, $cursor: String) { +const REVIEW_THREADS_QUERY = `query GetReviewThreads($owner: String!, $repo: String!, $number: Int!, $cursor: String) { repository(owner: $owner, name: $repo) { pullRequest(number: $number) { reviewThreads(first: 100, after: $cursor) { pageInfo { hasNextPage endCursor } nodes { + id isResolved comments(first: 1) { nodes { databaseId } } } @@ -67,7 +68,7 @@ const RESOLVED_THREADS_QUERY = `query GetResolvedThreads($owner: String!, $repo: } }`; -const ResolvedThreadsSchema = z.object({ +const ReviewThreadsSchema = z.object({ data: z.object({ repository: z .object({ @@ -77,6 +78,7 @@ const ResolvedThreadsSchema = z.object({ pageInfo: z.object({ hasNextPage: z.boolean(), endCursor: z.string().nullable() }), nodes: z.array( z.object({ + id: z.string(), isResolved: z.boolean(), comments: z.object({ nodes: z.array(z.object({ databaseId: z.number().nullable() })), @@ -91,24 +93,32 @@ const ResolvedThreadsSchema = z.object({ }), }); +/** A PR review thread's GraphQL node id and resolution state. */ +export interface ReviewThreadInfo { + nodeId: string; + isResolved: boolean; +} + /** - * Set of root review-comment ids whose thread is resolved on GitHub, keyed by the - * root comment's database id (the same id the REST list reports for the thread's - * first comment). Used to mirror resolution state on pull. + * Review threads keyed by their root comment's database id (the same id the REST + * list reports for the thread's first comment). The node id is needed to resolve + * or reopen a thread; the resolution state mirrors GitHub onto the local review. + * We key off the root comment id we already store, so nothing GitHub-owned needs + * persisting — the node id is looked up live when a thread is resolved. */ -export async function listResolvedRootCommentIds( +export async function listReviewThreads( repoRoot: string, repo: GitHubRepo, prNumber: number, -): Promise> { - const resolved = new Set(); +): Promise> { + const byRootCommentId = new Map(); let cursor: string | null = null; do { const args = [ "api", "graphql", "-f", - `query=${RESOLVED_THREADS_QUERY}`, + `query=${REVIEW_THREADS_QUERY}`, "-F", `owner=${repo.owner}`, "-F", @@ -118,17 +128,46 @@ export async function listResolvedRootCommentIds( ]; if (cursor !== null) args.push("-F", `cursor=${cursor}`); const stdout = await ghOrThrow(args, repoRoot); - const parsed = ResolvedThreadsSchema.safeParse(JSON.parse(stdout)); + const parsed = ReviewThreadsSchema.safeParse(JSON.parse(stdout)); if (!parsed.success) throw new Error("Unexpected response shape from GitHub review threads"); const threads = parsed.data.data.repository?.pullRequest?.reviewThreads; if (!threads) break; for (const thread of threads.nodes) { const rootId = thread.comments.nodes[0]?.databaseId; - if (thread.isResolved && rootId != null) resolved.add(rootId); + if (rootId != null) { + byRootCommentId.set(rootId, { nodeId: thread.id, isResolved: thread.isResolved }); + } } cursor = threads.pageInfo.hasNextPage ? threads.pageInfo.endCursor : null; } while (cursor !== null); - return resolved; + return byRootCommentId; +} + +const RESOLVE_THREAD_MUTATION = `mutation ResolveThread($threadId: ID!) { + resolveReviewThread(input: { threadId: $threadId }) { thread { id } } +}`; + +const UNRESOLVE_THREAD_MUTATION = `mutation UnresolveThread($threadId: ID!) { + unresolveReviewThread(input: { threadId: $threadId }) { thread { id } } +}`; + +/** Resolve or reopen a review thread by its GraphQL node id. Throws on failure. */ +export async function setReviewThreadResolved( + repoRoot: string, + threadNodeId: string, + resolved: boolean, +): Promise { + await ghWrite( + [ + "api", + "graphql", + "-f", + `query=${resolved ? RESOLVE_THREAD_MUTATION : UNRESOLVE_THREAD_MUTATION}`, + "-F", + `threadId=${threadNodeId}`, + ], + repoRoot, + ); } // ─── Writes ───────────────────────────────────────────────────────────────────── diff --git a/packages/cli/src/routes/comments.ts b/packages/cli/src/routes/comments.ts index c96d933..3c02db9 100644 --- a/packages/cli/src/routes/comments.ts +++ b/packages/cli/src/routes/comments.ts @@ -16,7 +16,12 @@ import { comment, commentThread, } from "../db/schema/index.js"; -import { CommentSyncError, pullComments, pushComments } from "../runs/comment-sync.js"; +import { + CommentSyncError, + pullComments, + pushComments, + syncThreadResolution, +} from "../runs/comment-sync.js"; import { deriveScopeKey } from "../runs/scope-key.js"; import type { Route } from "../server.js"; import { parseJsonBody, writeJson } from "./json.js"; @@ -106,8 +111,10 @@ export function commentRoutes(db: StageDb): Route[] { }, }, { + // Run-scoped so resolving a PR-originated thread can mirror the toggle to + // GitHub (it needs the run's repo/PR context). Local-only threads stay local. method: "PATCH", - pattern: "/api/comment-threads/:threadId", + pattern: "/api/runs/:runId/comment-threads/:threadId", handler: async (req, res, params) => { if (!enforceSameOrigin(req, res)) return; const threadId = params.threadId; @@ -117,18 +124,28 @@ export function commentRoutes(db: StageDb): Route[] { } const body = await parseJsonBody(req, res, ResolveThreadBodySchema); if (!body) return; - - const [updated] = db - .update(commentThread) - .set({ resolvedAt: body.resolved ? new Date() : null }) - .where(eq(commentThread.id, threadId)) - .returning() - .all(); - if (!updated) { - writeJson(res, 404, { error: `Thread ${threadId} not found` }); + const runId = params.runId; + if (!runId) { + writeJson(res, 400, { error: "Missing runId" }); + return; + } + const [run] = db.select().from(chapterRun).where(eq(chapterRun.id, runId)).limit(1).all(); + if (!run) { + writeJson(res, 404, { error: `Run ${runId} not found` }); return; } - writeJson(res, 200, toThreadDto(updated, threadComments(db, threadId))); + try { + const updated = await syncThreadResolution(db, run, threadId, body.resolved); + writeJson(res, 200, toThreadDto(updated, threadComments(db, threadId))); + } catch (err) { + if (err instanceof CommentSyncError) { + writeJson(res, err.status, { error: err.message }); + return; + } + writeJson(res, 500, { + error: err instanceof Error ? err.message : "Failed to update thread", + }); + } }, }, { diff --git a/packages/cli/src/runs/comment-sync.ts b/packages/cli/src/runs/comment-sync.ts index 7e3456a..c34f07f 100644 --- a/packages/cli/src/runs/comment-sync.ts +++ b/packages/cli/src/runs/comment-sync.ts @@ -2,17 +2,24 @@ import type { PullCommentsResult, PushCommentsResult } from "@stagereview/types/ import { asc, eq } from "drizzle-orm"; import type { StageDb } from "../db/client.js"; import { LOCAL_USER_ID } from "../db/local-user.js"; -import { type ChapterRunRow, type CommentRow, comment, commentThread } from "../db/schema/index.js"; +import { + type ChapterRunRow, + type CommentRow, + type CommentThreadRow, + comment, + commentThread, +} from "../db/schema/index.js"; import { isWorkingTreeClean, readHeadSha } from "../git.js"; import { createReviewComment, type GitHubRepo, getPullRequest, - listResolvedRootCommentIds, listReviewComments, + listReviewThreads, parseGitHubRepo, type ReviewComment, replyToReviewComment, + setReviewThreadResolved, } from "../github/index.js"; import { SCOPE_KIND } from "../schema.js"; import { groupReviewComments, toGitHubSide } from "./review-comment-mapping.js"; @@ -71,9 +78,9 @@ async function resolveSyncTarget(run: ChapterRunRow): Promise { export async function pullComments(db: StageDb, run: ChapterRunRow): Promise { const { repo, prNumber } = await resolveSyncTarget(run); const scopeKey = deriveScopeKey(run); - const [comments, resolvedRootIds] = await Promise.all([ + const [comments, reviewThreads] = await Promise.all([ listReviewComments(run.repoRoot, repo, prNumber), - listResolvedRootCommentIds(run.repoRoot, repo, prNumber), + listReviewThreads(run.repoRoot, repo, prNumber), ]); const threads = groupReviewComments(comments); @@ -102,7 +109,7 @@ export async function pullComments(db: StageDb, run: ChapterRunRow): Promise { + const [thread] = db + .select() + .from(commentThread) + .where(eq(commentThread.id, threadId)) + .limit(1) + .all(); + if (!thread) throw new CommentSyncError(`Thread ${threadId} not found`, 404); + + const [root] = db + .select({ githubCommentId: comment.githubCommentId }) + .from(comment) + .where(eq(comment.threadId, threadId)) + .orderBy(asc(comment.createdAt)) + .limit(1) + .all(); + + if (root?.githubCommentId != null) { + const target = await resolveSyncTarget(run); + const reviewThreads = await listReviewThreads(run.repoRoot, target.repo, target.prNumber); + const nodeId = reviewThreads.get(root.githubCommentId)?.nodeId; + if (nodeId === undefined) { + throw new CommentSyncError( + "This thread is no longer on the pull request, so its resolved state can't be synced.", + 404, + ); + } + await setReviewThreadResolved(run.repoRoot, nodeId, resolved); + } + + const [updated] = db + .update(commentThread) + .set({ resolvedAt: resolved ? new Date() : null }) + .where(eq(commentThread.id, threadId)) + .returning() + .all(); + if (!updated) throw new Error("comment_thread resolve update returned no row"); + return updated; +} diff --git a/packages/web/src/components/comments/comment-thread.tsx b/packages/web/src/components/comments/comment-thread.tsx index 888c68d..8151a38 100644 --- a/packages/web/src/components/comments/comment-thread.tsx +++ b/packages/web/src/components/comments/comment-thread.tsx @@ -13,6 +13,7 @@ import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { Button } from "@/components/ui/button"; import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible"; import { Markdown } from "@/components/ui/markdown"; +import { toast } from "@/components/ui/sonner"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import { useCommentThreadsContext } from "@/lib/comment-threads-context"; import { formatTimeAgo } from "@/lib/format"; @@ -47,14 +48,23 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { if (!root) return null; const replies = thread.comments.slice(1); - function handleResolveToggle() { + async function handleResolveToggle() { const next = !isResolved; + const wasOpen = isOpen; // Collapse on resolve / expand on reopen — but never collapse out from under an // active reply/edit/delete form (it would unmount CommentForm and drop unsaved // text), mirroring the handleOpenChange guard. const hasActiveForm = isReplying || editingId !== null || deleteTarget !== null; if (!next || !hasActiveForm) setIsOpen(!next); - void setThreadResolved({ threadId: thread.id, resolved: next }); + try { + // For PR-originated threads this also resolves/reopens the thread on GitHub; + // on failure the server leaves local state unchanged, so revert the collapse + // and surface the reason. + await setThreadResolved({ threadId: thread.id, resolved: next }); + } catch (err) { + setIsOpen(wasOpen); + toast.error(errorMessage(err, "Failed to update resolved state")); + } } function handleOpenChange(open: boolean) { diff --git a/packages/web/src/lib/use-comment-sync.ts b/packages/web/src/lib/use-comment-sync.ts index 9523435..68a8482 100644 --- a/packages/web/src/lib/use-comment-sync.ts +++ b/packages/web/src/lib/use-comment-sync.ts @@ -6,29 +6,10 @@ import { } from "@stagereview/types/comments"; import { useMutation, useQueryClient } from "@tanstack/react-query"; import { commentThreadsQueryKey } from "./use-comment-threads"; +import { jsonFetch } from "./use-view-state"; export type { PullCommentsResult, PushCommentsResult }; -/** - * POST a sync endpoint, surfacing the server's `{ error }` message verbatim on a - * non-2xx response (guardrail failures and the missing-PR case all carry one). - * The generic `jsonFetch` only reports the status code, which would hide the - * actionable reason a sync was refused. - */ -async function syncFetch(url: string): Promise { - const res = await fetch(url, { method: "POST" }); - const text = await res.text(); - const parsed: unknown = text ? JSON.parse(text) : {}; - if (!res.ok) { - const message = - typeof parsed === "object" && parsed !== null && "error" in parsed - ? String((parsed as { error: unknown }).error) - : `Sync failed (${res.status})`; - throw new Error(message); - } - return parsed as T; -} - export interface UseCommentSyncResult { pull: () => Promise; push: () => Promise; @@ -48,17 +29,17 @@ export function useCommentSync(runId: string): UseCommentSyncResult { const pullMutation = useMutation({ mutationFn: () => - syncFetch(`/api/runs/${encodeURIComponent(runId)}/comment-sync/pull`).then((raw) => - PullCommentsResultSchema.parse(raw), - ), + jsonFetch(`/api/runs/${encodeURIComponent(runId)}/comment-sync/pull`, { + method: "POST", + }).then((raw) => PullCommentsResultSchema.parse(raw)), onSuccess: invalidate, }); const pushMutation = useMutation({ mutationFn: () => - syncFetch(`/api/runs/${encodeURIComponent(runId)}/comment-sync/push`).then((raw) => - PushCommentsResultSchema.parse(raw), - ), + jsonFetch(`/api/runs/${encodeURIComponent(runId)}/comment-sync/push`, { + method: "POST", + }).then((raw) => PushCommentsResultSchema.parse(raw)), onSuccess: invalidate, }); diff --git a/packages/web/src/lib/use-comment-threads.ts b/packages/web/src/lib/use-comment-threads.ts index 972e3f9..53f0df6 100644 --- a/packages/web/src/lib/use-comment-threads.ts +++ b/packages/web/src/lib/use-comment-threads.ts @@ -85,8 +85,9 @@ export function useCommentThreads(runId: string): UseCommentThreadsResult { const resolveMutation = useMutation({ mutationFn: async ({ threadId, resolved }: { threadId: string; resolved: boolean }) => { + // Run-scoped: the server mirrors the toggle to GitHub for PR-originated threads. await jsonFetch( - `/api/comment-threads/${encodeURIComponent(threadId)}`, + `/api/runs/${encodeURIComponent(runId)}/comment-threads/${encodeURIComponent(threadId)}`, jsonRequest("PATCH", { resolved }), ); }, diff --git a/packages/web/src/lib/use-view-state.ts b/packages/web/src/lib/use-view-state.ts index fa4e402..22e7ccf 100644 --- a/packages/web/src/lib/use-view-state.ts +++ b/packages/web/src/lib/use-view-state.ts @@ -26,13 +26,20 @@ export function viewStateQueryKey(runId: string): readonly unknown[] { export async function jsonFetch(url: string, init?: RequestInit): Promise { const res = await fetch(url, init); + // POST/DELETE handlers (and error responses) can return an empty body — read as + // text first so JSON.parse doesn't throw SyntaxError on `""`. + const text = await res.text(); + const parsed: unknown = text ? JSON.parse(text) : {}; if (!res.ok) { - throw new Error(`${init?.method ?? "GET"} ${url} failed: ${res.status}`); + // Surface the server's `{ error }` message verbatim (sync guardrails and the + // GitHub-write paths carry actionable reasons); fall back to the status code. + const message = + typeof parsed === "object" && parsed !== null && "error" in parsed + ? String((parsed as { error: unknown }).error) + : `${init?.method ?? "GET"} ${url} failed: ${res.status}`; + throw new Error(message); } - // POST/DELETE handlers can return an empty body — read as text first so - // JSON.parse doesn't throw SyntaxError on `""`. - const text = await res.text(); - return (text ? JSON.parse(text) : {}) as T; + return parsed as T; } async function fetchViewState(runId: string): Promise { From 5745864b045966e15b153bff816a65e8030c2a3a Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 22 Jun 2026 15:42:30 +0200 Subject: [PATCH 05/17] feat(PRO-547): add live GitHub review server layer (pending-review lifecycle) --- .../cli/src/__tests__/review.routes.test.ts | 304 ++++++++++++++ packages/cli/src/github/exec.ts | 14 + packages/cli/src/github/review.ts | 386 ++++++++++++++++++ packages/cli/src/routes/review.ts | 152 +++++++ packages/cli/src/runs/review.ts | 292 +++++++++++++ packages/cli/src/show.ts | 2 + packages/types/package.json | 1 + packages/types/src/index.ts | 1 + packages/types/src/review.ts | 126 ++++++ 9 files changed, 1278 insertions(+) create mode 100644 packages/cli/src/__tests__/review.routes.test.ts create mode 100644 packages/cli/src/github/review.ts create mode 100644 packages/cli/src/routes/review.ts create mode 100644 packages/cli/src/runs/review.ts create mode 100644 packages/types/src/review.ts diff --git a/packages/cli/src/__tests__/review.routes.test.ts b/packages/cli/src/__tests__/review.routes.test.ts new file mode 100644 index 0000000..c622d1d --- /dev/null +++ b/packages/cli/src/__tests__/review.routes.test.ts @@ -0,0 +1,304 @@ +import fs from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import type { ReviewResponse } from "@stagereview/types/review"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { closeDb, getDb } from "../db/client.js"; +import { chapterRun, comment, commentThread } from "../db/schema/index.js"; +import { reviewRoutes } from "../routes/review.js"; +import { SCOPE_KIND } from "../schema.js"; +import { LOOPBACK_HOST, type ServerHandle, startServer } from "../server.js"; + +let tmpDir: string; +let dbPath: string; +let webDist: string; +let repoRoot: string; +let binDir: string; +let originalPath: string | undefined; +const handles: ServerHandle[] = []; + +const BASE = "b".repeat(40); +const HEAD = "a".repeat(40); +const MERGE_BASE = "c".repeat(40); +const SCOPE_KEY = `committed:${BASE}:${HEAD}:${MERGE_BASE}`; +const GITHUB_ORIGIN = "git@github.com:owner/repo.git"; + +// One submitted thread (comment 1) and one pending thread (comment 2, viewer's draft). +const REVIEW_QUERY_RESULT = { + data: { + repository: { + pullRequest: { + id: "PR_node", + reviews: { nodes: [{ id: "REVIEW_pending" }] }, + reviewThreads: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + id: "THREAD_sub", + isResolved: false, + comments: { + nodes: [ + { + databaseId: 1, + id: "COMMENT_sub", + path: "src/foo.ts", + body: "Submitted comment", + createdAt: "2026-01-01T00:00:00Z", + line: 10, + startLine: null, + diffSide: "RIGHT", + startDiffSide: null, + author: { login: "octocat", avatarUrl: "https://x/o.png" }, + pullRequestReview: { state: "COMMENTED" }, + }, + ], + }, + }, + { + id: "THREAD_pending", + isResolved: false, + comments: { + nodes: [ + { + databaseId: 2, + id: "COMMENT_pending", + path: "src/bar.ts", + body: "Draft comment", + createdAt: "2026-01-02T00:00:00Z", + line: 4, + startLine: null, + diffSide: "LEFT", + startDiffSide: null, + author: { login: "octocat", avatarUrl: "https://x/o.png" }, + pullRequestReview: { state: "PENDING" }, + }, + ], + }, + }, + ], + }, + }, + }, + }, +}; + +async function writeGhShim(reviewResult: unknown): Promise { + await fs.writeFile(path.join(tmpDir, "review.json"), JSON.stringify(reviewResult)); + const shim = `#!/usr/bin/env node +const fs = require("node:fs"); +const args = process.argv.slice(2); +const query = (args.find((a) => a.startsWith("query=")) || ""); +// Field args only (the GraphQL query itself is multi-line and would break line-based log parsing). +const fields = args.filter((a) => !a.startsWith("query=") && a !== "-f" && a !== "-F" && a !== "api" && a !== "graphql").join(" "); +const log = ${JSON.stringify(path.join(tmpDir, "gh-log.txt"))}; +function emit(o) { process.stdout.write(JSON.stringify(o)); } +if (query.includes("query GetReview")) { + emit(JSON.parse(fs.readFileSync(${JSON.stringify(path.join(tmpDir, "review.json"))}, "utf8"))); +} else if (query.includes("mutation CreatePendingReview")) { + fs.appendFileSync(log, "create-review\\n"); + emit({ data: { addPullRequestReview: { pullRequestReview: { id: "REVIEW_new" } } } }); +} else if (query.includes("mutation AddReviewThread")) { + fs.appendFileSync(log, "add-thread " + fields + "\\n"); + emit({ data: { addPullRequestReviewThread: { thread: { id: "THREAD_new" } } } }); +} else if (query.includes("mutation AddReviewReply")) { + fs.appendFileSync(log, "reply\\n"); + emit({ data: { addPullRequestReviewThreadReply: { comment: { id: "C" } } } }); +} else if (query.includes("mutation SubmitReview")) { + fs.appendFileSync(log, "submit " + fields + "\\n"); + emit({ data: { submitPullRequestReview: { pullRequestReview: { id: "R" } } } }); +} else { + emit({ data: {} }); +} +`; + await fs.writeFile(path.join(binDir, "gh"), shim); + await fs.chmod(path.join(binDir, "gh"), 0o755); +} + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-review-")); + dbPath = path.join(tmpDir, "db.sqlite"); + webDist = path.join(tmpDir, "web-dist"); + repoRoot = path.join(tmpDir, "repo"); + binDir = path.join(tmpDir, "bin"); + await fs.mkdir(webDist); + await fs.writeFile(path.join(webDist, "index.html"), ""); + await fs.mkdir(repoRoot); + await fs.mkdir(binDir); + originalPath = process.env.PATH; + process.env.PATH = `${binDir}${path.delimiter}${originalPath ?? ""}`; + closeDb(); +}); + +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.close(); + } + closeDb(); + process.env.PATH = originalPath; + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +function insertRun(originUrl: string | null): string { + const db = getDb({ dbPath }); + const [row] = db + .insert(chapterRun) + .values({ + repoRoot, + originUrl, + prNumber: 5, + scopeKind: SCOPE_KIND.COMMITTED, + workingTreeRef: null, + baseSha: BASE, + headSha: HEAD, + mergeBaseSha: MERGE_BASE, + generatedAt: new Date(), + }) + .returning({ id: chapterRun.id }) + .all(); + if (!row) throw new Error("seed: chapter_run insert returned no row"); + return row.id; +} + +function seedLocalThread(): string { + const db = getDb({ dbPath }); + const [thread] = db + .insert(commentThread) + .values({ + scopeKey: SCOPE_KEY, + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + }) + .returning({ id: commentThread.id }) + .all(); + if (!thread) throw new Error("seed: thread insert returned no row"); + db.insert(comment).values({ threadId: thread.id, body: "Local note" }).run(); + return thread.id; +} + +async function start(): Promise { + const db = getDb({ dbPath }); + const handle = await startServer({ webDistPath: webDist, routes: reviewRoutes(db) }); + handles.push(handle); + return handle.port; +} + +function request( + port: number, + method: string, + p: string, + body?: unknown, +): Promise<{ status: number; body: string }> { + return new Promise((resolve, reject) => { + const payload = body === undefined ? undefined : JSON.stringify(body); + const req = http.request( + { + hostname: LOOPBACK_HOST, + port, + method, + path: p, + agent: false, + headers: + payload === undefined + ? {} + : { "Content-Type": "application/json", "Content-Length": Buffer.byteLength(payload) }, + }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (c: Buffer) => chunks.push(c)); + res.on("end", () => + resolve({ status: res.statusCode ?? 0, body: Buffer.concat(chunks).toString("utf8") }), + ); + }, + ); + req.on("error", reject); + req.end(payload); + }); +} + +describe("review API — read", () => { + it("returns local-only with github:none when there's no GitHub remote", async () => { + const runId = insertRun(null); + seedLocalThread(); + const res = await request(await start(), "GET", `/api/runs/${runId}/review`); + expect(res.status).toBe(200); + const review = JSON.parse(res.body) as ReviewResponse; + expect(review.github).toBe("none"); + expect(review.threads).toHaveLength(1); + expect(review.threads[0]?.source).toBe("local"); + expect(review.threads[0]?.comments[0]?.state).toBe("local"); + }); + + it("merges local threads with the PR's pending and submitted GitHub threads", async () => { + await writeGhShim(REVIEW_QUERY_RESULT); + const runId = insertRun(GITHUB_ORIGIN); + seedLocalThread(); + const res = await request(await start(), "GET", `/api/runs/${runId}/review`); + const review = JSON.parse(res.body) as ReviewResponse; + expect(review.github).toBe("available"); + expect(review.pendingCommentCount).toBe(1); + expect(review.hasPendingReview).toBe(true); + + const states = review.threads.map((t) => t.comments[0]?.state).sort(); + expect(states).toEqual(["local", "pending", "submitted"]); + // LEFT diff side maps to the deletions side locally. + const pending = review.threads.find((t) => t.comments[0]?.state === "pending"); + expect(pending?.side).toBe("deletions"); + }); + + it("reports github:offline when gh fails", async () => { + // No gh shim on PATH → the review query errors out → offline (local still renders). + await fs.writeFile(path.join(binDir, "gh"), "#!/bin/sh\nexit 1\n"); + await fs.chmod(path.join(binDir, "gh"), 0o755); + const runId = insertRun(GITHUB_ORIGIN); + seedLocalThread(); + const res = await request(await start(), "GET", `/api/runs/${runId}/review`); + const review = JSON.parse(res.body) as ReviewResponse; + expect(review.github).toBe("offline"); + expect(review.threads).toHaveLength(1); + }); +}); + +describe("review API — actions", () => { + it("promotes a local thread to a pending review comment and removes the local copy", async () => { + await writeGhShim({ + data: { + repository: { + pullRequest: { + id: "PR_node", + reviews: { nodes: [] }, + reviewThreads: { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + }, + }, + }, + }); + const runId = insertRun(GITHUB_ORIGIN); + const localThreadId = seedLocalThread(); + const res = await request(await start(), "POST", `/api/runs/${runId}/review/add`, { + localThreadId, + }); + expect(res.status).toBe(200); + + const db = getDb({ dbPath }); + expect(db.select().from(commentThread).all()).toHaveLength(0); + const lines = (await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8")).split("\n"); + expect(lines.filter((l) => l === "create-review")).toHaveLength(1); + expect(lines.filter((l) => l.startsWith("add-thread"))).toHaveLength(1); + }); + + it("submits the pending review with the chosen event", async () => { + await writeGhShim(REVIEW_QUERY_RESULT); + const runId = insertRun(GITHUB_ORIGIN); + const res = await request(await start(), "POST", `/api/runs/${runId}/review/submit`, { + event: "APPROVE", + body: "LGTM", + }); + expect(res.status).toBe(200); + const lines = (await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8")).split("\n"); + const submit = lines.find((l) => l.startsWith("submit")); + expect(submit).toContain("event=APPROVE"); + }); +}); diff --git a/packages/cli/src/github/exec.ts b/packages/cli/src/github/exec.ts index 7c20968..d279fc9 100644 --- a/packages/cli/src/github/exec.ts +++ b/packages/cli/src/github/exec.ts @@ -13,6 +13,20 @@ export async function gh(args: string[], cwd: string): Promise { return stdout; } +/** + * Run a `gh` command and return stdout, surfacing failures as a clean Error with + * gh's stderr message. Use for user-initiated actions (reads and writes alike) + * where the failure reason should reach the user, unlike the passive PR-context + * adapters that degrade to empty. + */ +export async function ghOrThrow(args: string[], cwd: string): Promise { + try { + return await gh(args, cwd); + } catch (err) { + throw new Error(ghErrorMessage(err)); + } +} + /** * Extract the most useful message from a failed `gh`/`git` exec: prefer the * command's stderr (where these tools write human-readable failures), falling diff --git a/packages/cli/src/github/review.ts b/packages/cli/src/github/review.ts new file mode 100644 index 0000000..b40bf29 --- /dev/null +++ b/packages/cli/src/github/review.ts @@ -0,0 +1,386 @@ +import { z } from "zod"; +import { ghOrThrow } from "./exec.js"; +import type { GitHubRepo } from "./repo.js"; + +/** + * GitHub's diff sides. `LEFT` is the base/deletion side, `RIGHT` the head/addition + * side; they map onto the local `DIFF_SIDE` (deletions/additions) in the review layer. + */ +export const GITHUB_DIFF_SIDE = { + LEFT: "LEFT", + RIGHT: "RIGHT", +} as const; +export type GitHubDiffSide = (typeof GITHUB_DIFF_SIDE)[keyof typeof GITHUB_DIFF_SIDE]; + +/** The three events a review can be submitted with, mirroring GitHub's own model. */ +export const REVIEW_EVENT = { + COMMENT: "COMMENT", + APPROVE: "APPROVE", + REQUEST_CHANGES: "REQUEST_CHANGES", +} as const; +export type ReviewEvent = (typeof REVIEW_EVENT)[keyof typeof REVIEW_EVENT]; + +// ─── Read: the PR's review state in one paginated query ───────────────────────── + +// A single GraphQL query gives everything we render: the PR node id (needed by the +// write mutations), the viewer's pending-review node id, and every review thread +// with its comments. Each comment's `pullRequestReview.state` distinguishes a +// PENDING (draft, viewer-only) comment from a submitted one — no REST list or +// local mirror required. +const REVIEW_QUERY = `query GetReview($owner: String!, $repo: String!, $number: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + id + reviews(states: PENDING, first: 1) { nodes { id } } + reviewThreads(first: 50, after: $cursor) { + pageInfo { hasNextPage endCursor } + nodes { + id + isResolved + comments(first: 100) { + nodes { + databaseId + id + path + body + createdAt + line + startLine + diffSide + startDiffSide + author { login avatarUrl } + pullRequestReview { state } + } + } + } + } + } + } +}`; + +const GqlActorSchema = z.object({ login: z.string(), avatarUrl: z.string() }).nullable(); + +const GqlReviewCommentSchema = z.object({ + databaseId: z.number().nullable(), + id: z.string(), + path: z.string(), + body: z.string(), + createdAt: z.string(), + line: z.number().nullable(), + startLine: z.number().nullable(), + diffSide: z.enum(GITHUB_DIFF_SIDE).nullable(), + startDiffSide: z.enum(GITHUB_DIFF_SIDE).nullable(), + author: GqlActorSchema, + pullRequestReview: z.object({ state: z.string() }).nullable(), +}); + +const ReviewQuerySchema = z.object({ + data: z.object({ + repository: z + .object({ + pullRequest: z + .object({ + id: z.string(), + reviews: z.object({ nodes: z.array(z.object({ id: z.string() })) }), + reviewThreads: z.object({ + pageInfo: z.object({ hasNextPage: z.boolean(), endCursor: z.string().nullable() }), + nodes: z.array( + z.object({ + id: z.string(), + isResolved: z.boolean(), + comments: z.object({ nodes: z.array(GqlReviewCommentSchema) }), + }), + ), + }), + }) + .nullable(), + }) + .nullable(), + }), +}); + +/** A comment within a review thread, tagged with whether it's a draft (pending) or published. */ +export interface ReviewComment { + databaseId: number | null; + nodeId: string; + body: string; + createdAt: string; + authorLogin: string; + authorAvatarUrl: string; + /** Pending = part of the viewer's unsubmitted review (only they see it). */ + isPending: boolean; +} + +/** A line-anchored review thread on the PR, with its comments oldest-first. */ +export interface ReviewThread { + threadNodeId: string; + isResolved: boolean; + path: string; + line: number | null; + startLine: number | null; + side: GitHubDiffSide; + startSide: GitHubDiffSide | null; + comments: ReviewComment[]; +} + +export interface GitHubReview { + /** GraphQL node id of the PR, required by the write mutations. */ + pullRequestNodeId: string; + /** The viewer's open pending review, or null when they have none. */ + pendingReviewNodeId: string | null; + threads: ReviewThread[]; +} + +const PENDING_STATE = "PENDING"; + +/** + * The PR's review threads (pending + submitted) as the viewer sees them, plus the + * ids the write mutations need. Threads with no anchorable line (outdated or + * whole-file) are dropped — the review UI is line-anchored. + */ +export async function getReview( + repoRoot: string, + repo: GitHubRepo, + prNumber: number, +): Promise { + let pullRequestNodeId = ""; + let pendingReviewNodeId: string | null = null; + const threads: ReviewThread[] = []; + let cursor: string | null = null; + + do { + const args = [ + "api", + "graphql", + "-f", + `query=${REVIEW_QUERY}`, + "-F", + `owner=${repo.owner}`, + "-F", + `repo=${repo.repo}`, + "-F", + `number=${prNumber}`, + ]; + if (cursor !== null) args.push("-F", `cursor=${cursor}`); + const parsed = ReviewQuerySchema.safeParse(JSON.parse(await ghOrThrow(args, repoRoot))); + if (!parsed.success) throw new Error("Unexpected response shape from GitHub review query"); + const pr = parsed.data.data.repository?.pullRequest; + if (!pr) break; + pullRequestNodeId = pr.id; + pendingReviewNodeId = pr.reviews.nodes[0]?.id ?? null; + + for (const node of pr.reviewThreads.nodes) { + const root = node.comments.nodes[0]; + if (!root || root.line === null) continue; + threads.push({ + threadNodeId: node.id, + isResolved: node.isResolved, + path: root.path, + line: root.line, + startLine: root.startLine, + side: root.diffSide ?? GITHUB_DIFF_SIDE.RIGHT, + startSide: root.startDiffSide, + comments: node.comments.nodes.map(toReviewComment), + }); + } + cursor = pr.reviewThreads.pageInfo.hasNextPage ? pr.reviewThreads.pageInfo.endCursor : null; + } while (cursor !== null); + + return { pullRequestNodeId, pendingReviewNodeId, threads }; +} + +function toReviewComment(c: z.infer): ReviewComment { + return { + databaseId: c.databaseId, + nodeId: c.id, + body: c.body, + createdAt: c.createdAt, + authorLogin: c.author?.login ?? "ghost", + authorAvatarUrl: c.author?.avatarUrl ?? "", + isPending: c.pullRequestReview?.state === PENDING_STATE, + }; +} + +// ─── Write: pending-review lifecycle ──────────────────────────────────────────── + +const CREATE_PENDING_REVIEW = `mutation CreatePendingReview($pullRequestId: ID!) { + addPullRequestReview(input: { pullRequestId: $pullRequestId }) { + pullRequestReview { id } + } +}`; + +const ADD_REVIEW_THREAD = `mutation AddReviewThread($pullRequestId: ID!, $reviewId: ID!, $path: String!, $body: String!, $line: Int!, $startLine: Int, $side: DiffSide!, $startSide: DiffSide) { + addPullRequestReviewThread(input: { pullRequestId: $pullRequestId, pullRequestReviewId: $reviewId, path: $path, body: $body, line: $line, startLine: $startLine, side: $side, startSide: $startSide }) { + thread { id } + } +}`; + +const ADD_REVIEW_REPLY = `mutation AddReviewReply($threadId: ID!, $reviewId: ID, $body: String!) { + addPullRequestReviewThreadReply(input: { pullRequestReviewThreadId: $threadId, pullRequestReviewId: $reviewId, body: $body }) { + comment { id } + } +}`; + +const UPDATE_REVIEW_COMMENT = `mutation UpdateReviewComment($commentId: ID!, $body: String!) { + updatePullRequestReviewComment(input: { pullRequestReviewCommentId: $commentId, body: $body }) { + pullRequestReviewComment { id } + } +}`; + +const DELETE_REVIEW_COMMENT = `mutation DeleteReviewComment($commentId: ID!) { + deletePullRequestReviewComment(input: { id: $commentId }) { + pullRequestReviewComment { id } + } +}`; + +const SUBMIT_REVIEW = `mutation SubmitReview($pullRequestId: ID!, $reviewId: ID!, $event: PullRequestReviewEvent!, $body: String) { + submitPullRequestReview(input: { pullRequestId: $pullRequestId, pullRequestReviewId: $reviewId, event: $event, body: $body }) { + pullRequestReview { id } + } +}`; + +const DISCARD_REVIEW = `mutation DiscardReview($reviewId: ID!) { + deletePullRequestReview(input: { pullRequestReviewId: $reviewId }) { + pullRequestReview { id } + } +}`; + +const RESOLVE_THREAD = `mutation ResolveThread($threadId: ID!) { + resolveReviewThread(input: { threadId: $threadId }) { thread { id } } +}`; + +const UNRESOLVE_THREAD = `mutation UnresolveThread($threadId: ID!) { + unresolveReviewThread(input: { threadId: $threadId }) { thread { id } } +}`; + +function gqlArgs(query: string, fields: Record): string[] { + const args = ["api", "graphql", "-f", `query=${query}`]; + for (const [key, value] of Object.entries(fields)) { + // Omit null fields so GraphQL applies its own null default (e.g. single-line + // comments send no startLine/startSide). + if (value === null) continue; + // `-f` for strings, `-F` for the numeric line fields (typed JSON values). + args.push(typeof value === "number" ? "-F" : "-f", `${key}=${value}`); + } + return args; +} + +const CreatedReviewSchema = z.object({ + data: z.object({ + addPullRequestReview: z.object({ pullRequestReview: z.object({ id: z.string() }) }), + }), +}); + +/** Open an empty pending review on the PR, returning its node id. */ +export async function createPendingReview( + repoRoot: string, + pullRequestNodeId: string, +): Promise { + const stdout = await ghOrThrow( + gqlArgs(CREATE_PENDING_REVIEW, { pullRequestId: pullRequestNodeId }), + repoRoot, + ); + return CreatedReviewSchema.parse(JSON.parse(stdout)).data.addPullRequestReview.pullRequestReview + .id; +} + +export interface AddReviewThreadInput { + pullRequestNodeId: string; + reviewNodeId: string; + path: string; + body: string; + line: number; + side: GitHubDiffSide; + startLine: number | null; + startSide: GitHubDiffSide | null; +} + +const AddedThreadSchema = z.object({ + data: z.object({ + addPullRequestReviewThread: z.object({ thread: z.object({ id: z.string() }) }), + }), +}); + +/** Add a line-anchored comment (a new thread) to a pending review, returning the thread's node id. */ +export async function addReviewThread( + repoRoot: string, + input: AddReviewThreadInput, +): Promise { + const stdout = await ghOrThrow( + gqlArgs(ADD_REVIEW_THREAD, { + pullRequestId: input.pullRequestNodeId, + reviewId: input.reviewNodeId, + path: input.path, + body: input.body, + line: input.line, + startLine: input.startLine, + side: input.side, + startSide: input.startSide, + }), + repoRoot, + ); + return AddedThreadSchema.parse(JSON.parse(stdout)).data.addPullRequestReviewThread.thread.id; +} + +/** Reply to an existing thread, attaching the reply to a pending review when one is open. */ +export async function addReviewReply( + repoRoot: string, + threadNodeId: string, + body: string, + reviewNodeId: string | null, +): Promise { + await ghOrThrow( + gqlArgs(ADD_REVIEW_REPLY, { threadId: threadNodeId, reviewId: reviewNodeId, body }), + repoRoot, + ); +} + +/** Edit a review comment by node id (works for pending and submitted comments). */ +export async function updateReviewComment( + repoRoot: string, + commentNodeId: string, + body: string, +): Promise { + await ghOrThrow(gqlArgs(UPDATE_REVIEW_COMMENT, { commentId: commentNodeId, body }), repoRoot); +} + +/** Delete a review comment by node id (used for pending comments). */ +export async function deleteReviewComment(repoRoot: string, commentNodeId: string): Promise { + await ghOrThrow(gqlArgs(DELETE_REVIEW_COMMENT, { commentId: commentNodeId }), repoRoot); +} + +/** Submit the pending review with the chosen event (Comment / Approve / Request changes). */ +export async function submitReview( + repoRoot: string, + pullRequestNodeId: string, + reviewNodeId: string, + event: ReviewEvent, + body: string, +): Promise { + await ghOrThrow( + gqlArgs(SUBMIT_REVIEW, { + pullRequestId: pullRequestNodeId, + reviewId: reviewNodeId, + event, + body: body.length > 0 ? body : null, + }), + repoRoot, + ); +} + +/** Throw away the pending review and all its draft comments. */ +export async function discardReview(repoRoot: string, reviewNodeId: string): Promise { + await ghOrThrow(gqlArgs(DISCARD_REVIEW, { reviewId: reviewNodeId }), repoRoot); +} + +/** Resolve or reopen a review thread by its node id. */ +export async function setThreadResolved( + repoRoot: string, + threadNodeId: string, + resolved: boolean, +): Promise { + await ghOrThrow( + gqlArgs(resolved ? RESOLVE_THREAD : UNRESOLVE_THREAD, { threadId: threadNodeId }), + repoRoot, + ); +} diff --git a/packages/cli/src/routes/review.ts b/packages/cli/src/routes/review.ts new file mode 100644 index 0000000..fc08ae3 --- /dev/null +++ b/packages/cli/src/routes/review.ts @@ -0,0 +1,152 @@ +import { + AddToReviewBodySchema, + GitHubCommentDeleteBodySchema, + GitHubCommentEditBodySchema, + GitHubReplyBodySchema, + GitHubResolveBodySchema, + SubmitReviewBodySchema, +} from "@stagereview/types/review"; +import { eq } from "drizzle-orm"; +import type { z } from "zod"; +import type { StageDb } from "../db/client.js"; +import { type ChapterRunRow, chapterRun } from "../db/schema/index.js"; +import { + addLocalThreadToReview, + deleteGitHubComment, + discardRunReview, + editGitHubComment, + getReviewForRun, + ReviewError, + replyToGitHubThread, + resolveGitHubThread, + submitRunReview, +} from "../runs/review.js"; +import type { Route, RouteHandler } from "../server.js"; +import { parseJsonBody, writeJson } from "./json.js"; +import { enforceSameOrigin } from "./pull-request-shared.js"; + +type Req = Parameters[0]; +type Res = Parameters[1]; + +export function reviewRoutes(db: StageDb): Route[] { + return [ + { + method: "GET", + pattern: "/api/runs/:runId/review", + handler: (_req, res, params) => + withRun(db, params.runId, res, (run) => getReviewForRun(db, run)), + }, + { + method: "POST", + pattern: "/api/runs/:runId/review/add", + handler: (req, res, params) => + withRunBody(db, req, res, params.runId, AddToReviewBodySchema, (run, body) => + addLocalThreadToReview(db, run, body.localThreadId), + ), + }, + { + method: "POST", + pattern: "/api/runs/:runId/review/submit", + handler: (req, res, params) => + withRunBody(db, req, res, params.runId, SubmitReviewBodySchema, (run, body) => + submitRunReview(run, body.event, body.body), + ), + }, + { + method: "POST", + pattern: "/api/runs/:runId/review/discard", + handler: (req, res, params) => + withRun(db, params.runId, res, (run) => discardRunReview(run), { sameOrigin: req }), + }, + { + method: "POST", + pattern: "/api/runs/:runId/review/reply", + handler: (req, res, params) => + withRunBody(db, req, res, params.runId, GitHubReplyBodySchema, (run, body) => + replyToGitHubThread(run, body.threadNodeId, body.body, body.pending), + ), + }, + { + method: "POST", + pattern: "/api/runs/:runId/review/comment/edit", + handler: (req, res, params) => + withRunBody(db, req, res, params.runId, GitHubCommentEditBodySchema, (run, body) => + editGitHubComment(run, body.nodeId, body.body), + ), + }, + { + method: "POST", + pattern: "/api/runs/:runId/review/comment/delete", + handler: (req, res, params) => + withRunBody(db, req, res, params.runId, GitHubCommentDeleteBodySchema, (run, body) => + deleteGitHubComment(run, body.nodeId), + ), + }, + { + method: "POST", + pattern: "/api/runs/:runId/review/resolve", + handler: (req, res, params) => + withRunBody(db, req, res, params.runId, GitHubResolveBodySchema, (run, body) => + resolveGitHubThread(run, body.threadNodeId, body.resolved), + ), + }, + ]; +} + +function loadRun(db: StageDb, runId: string | undefined, res: Res): ChapterRunRow | null { + if (!runId) { + writeJson(res, 400, { error: "Missing runId" }); + return null; + } + const [run] = db.select().from(chapterRun).where(eq(chapterRun.id, runId)).limit(1).all(); + if (!run) { + writeJson(res, 404, { error: `Run ${runId} not found` }); + return null; + } + return run; +} + +async function respond(res: Res, action: () => Promise): Promise { + try { + writeJson(res, 200, (await action()) ?? {}); + } catch (err) { + if (err instanceof ReviewError) { + writeJson(res, err.status, { error: err.message }); + return; + } + writeJson(res, 500, { + error: err instanceof Error ? err.message : "Review action failed", + }); + } +} + +/** GET-style handler: load the run and run the action. Pass `sameOrigin` to also guard a write. */ +async function withRun( + db: StageDb, + runId: string | undefined, + res: Res, + action: (run: ChapterRunRow) => Promise, + opts: { sameOrigin?: Req } = {}, +): Promise { + if (opts.sameOrigin && !enforceSameOrigin(opts.sameOrigin, res)) return; + const run = loadRun(db, runId, res); + if (!run) return; + await respond(res, () => action(run)); +} + +/** POST-style handler: enforce same-origin, parse + validate the body, then run the action. */ +async function withRunBody( + db: StageDb, + req: Req, + res: Res, + runId: string | undefined, + schema: z.ZodType, + action: (run: ChapterRunRow, body: T) => Promise, +): Promise { + if (!enforceSameOrigin(req, res)) return; + const run = loadRun(db, runId, res); + if (!run) return; + const body = await parseJsonBody(req, res, schema); + if (body === null) return; + await respond(res, () => action(run, body)); +} diff --git a/packages/cli/src/runs/review.ts b/packages/cli/src/runs/review.ts new file mode 100644 index 0000000..c89dfcd --- /dev/null +++ b/packages/cli/src/runs/review.ts @@ -0,0 +1,292 @@ +import { + COMMENT_STATE, + GITHUB_REVIEW_STATUS, + type ReviewComment as ReviewCommentDto, + type ReviewEvent, + type ReviewResponse, + type ReviewThread as ReviewThreadDto, + THREAD_SOURCE, +} from "@stagereview/types/review"; +import { asc, eq } from "drizzle-orm"; +import type { StageDb } from "../db/client.js"; +import { type ChapterRunRow, comment, commentThread } from "../db/schema/index.js"; +import { type GitHubRepo, getPullRequest, parseGitHubRepo } from "../github/index.js"; +import { + addReviewReply, + addReviewThread, + createPendingReview, + deleteReviewComment, + discardReview, + GITHUB_DIFF_SIDE, + type GitHubDiffSide, + type GitHubReview, + type ReviewThread as GitHubReviewThread, + getReview, + setThreadResolved, + submitReview, + updateReviewComment, +} from "../github/review.js"; +import { DIFF_SIDE, type DiffSide } from "../schema.js"; +import { deriveScopeKey } from "./scope-key.js"; + +/** A review action failure with a user-facing message and the route's HTTP status. */ +export class ReviewError extends Error { + constructor( + message: string, + readonly status: number, + ) { + super(message); + this.name = "ReviewError"; + } +} + +// LEFT is GitHub's base/deletion side, RIGHT the head/addition side. +function toGitHubSide(side: DiffSide): GitHubDiffSide { + return side === DIFF_SIDE.DELETIONS ? GITHUB_DIFF_SIDE.LEFT : GITHUB_DIFF_SIDE.RIGHT; +} + +function fromGitHubSide(side: GitHubDiffSide): DiffSide { + return side === GITHUB_DIFF_SIDE.LEFT ? DIFF_SIDE.DELETIONS : DIFF_SIDE.ADDITIONS; +} + +// ─── Read: merged local + GitHub review ───────────────────────────────────────── + +function loadLocalThreads(db: StageDb, scopeKey: string): ReviewThreadDto[] { + const threads = db + .select() + .from(commentThread) + .where(eq(commentThread.scopeKey, scopeKey)) + .orderBy(asc(commentThread.createdAt)) + .all(); + return threads.map((thread): ReviewThreadDto => { + const comments = db + .select() + .from(comment) + .where(eq(comment.threadId, thread.id)) + .orderBy(asc(comment.createdAt)) + .all(); + return { + id: thread.id, + source: THREAD_SOURCE.LOCAL, + threadNodeId: null, + filePath: thread.filePath, + side: thread.side, + startLine: thread.startLine, + endLine: thread.endLine, + isResolved: thread.resolvedAt !== null, + comments: comments.map( + (c): ReviewCommentDto => ({ + id: c.id, + state: COMMENT_STATE.LOCAL, + body: c.body, + author: null, + nodeId: null, + createdAt: c.createdAt.toISOString(), + }), + ), + }; + }); +} + +function toGitHubThreadDto(t: GitHubReviewThread): ReviewThreadDto { + // `line` is non-null (getReview drops anchorless threads); start defaults to line. + const endLine = t.line ?? t.startLine ?? 1; + return { + id: t.threadNodeId, + source: THREAD_SOURCE.GITHUB, + threadNodeId: t.threadNodeId, + filePath: t.path, + side: fromGitHubSide(t.side), + startLine: t.startLine ?? endLine, + endLine, + isResolved: t.isResolved, + comments: t.comments.map( + (c): ReviewCommentDto => ({ + id: c.nodeId, + state: c.isPending ? COMMENT_STATE.PENDING : COMMENT_STATE.SUBMITTED, + body: c.body, + author: { login: c.authorLogin, avatarUrl: c.authorAvatarUrl || null }, + nodeId: c.nodeId, + createdAt: c.createdAt, + }), + ), + }; +} + +/** + * The run's full review: local threads from the DB merged with the PR's live + * GitHub threads (pending + submitted). GitHub failures degrade to `offline` + * (local comments still render) rather than throwing — the read backs passive + * rendering, so it never blanks the review. + */ +export async function getReviewForRun(db: StageDb, run: ChapterRunRow): Promise { + const localThreads = loadLocalThreads(db, deriveScopeKey(run)); + const base = { threads: localThreads, pendingCommentCount: 0, hasPendingReview: false }; + + const repo = parseGitHubRepo(run.originUrl); + if (!repo) return { ...base, github: GITHUB_REVIEW_STATUS.NONE }; + + let prNumber = run.prNumber; + if (prNumber === null) { + const pr = await getPullRequest(run.repoRoot, run.originUrl, null); + prNumber = pr?.number ?? null; + } + if (prNumber === null) return { ...base, github: GITHUB_REVIEW_STATUS.NONE }; + + let review: GitHubReview; + try { + review = await getReview(run.repoRoot, repo, prNumber); + } catch { + return { ...base, github: GITHUB_REVIEW_STATUS.OFFLINE }; + } + + const githubThreads = review.threads.map(toGitHubThreadDto); + const pendingCommentCount = review.threads.reduce( + (n, t) => n + t.comments.filter((c) => c.isPending).length, + 0, + ); + return { + github: GITHUB_REVIEW_STATUS.AVAILABLE, + threads: [...localThreads, ...githubThreads], + pendingCommentCount, + hasPendingReview: review.pendingReviewNodeId !== null, + }; +} + +// ─── Write: review actions ────────────────────────────────────────────────────── + +interface ReviewTarget { + repo: GitHubRepo; + prNumber: number; + review: GitHubReview; +} + +/** Resolve the run's PR and load its live review, throwing a user-facing error when unavailable. */ +async function loadTarget(run: ChapterRunRow): Promise { + const repo = parseGitHubRepo(run.originUrl); + if (!repo) throw new ReviewError("This run isn't associated with a GitHub remote.", 404); + let prNumber = run.prNumber; + if (prNumber === null) { + const pr = await getPullRequest(run.repoRoot, run.originUrl, null); + prNumber = pr?.number ?? null; + } + if (prNumber === null) { + throw new ReviewError("No GitHub pull request found for this run.", 404); + } + const review = await getReview(run.repoRoot, repo, prNumber); + return { repo, prNumber, review }; +} + +/** The viewer's pending review node id, creating an empty pending review if none is open. */ +async function ensurePendingReview(run: ChapterRunRow, review: GitHubReview): Promise { + if (review.pendingReviewNodeId !== null) return review.pendingReviewNodeId; + return createPendingReview(run.repoRoot, review.pullRequestNodeId); +} + +/** + * Promote a local comment thread to the viewer's pending GitHub review: the root + * becomes a new review thread, replies become pending replies, and the local thread + * is removed (it now lives on GitHub as pending). GitHub anchors the comment to the + * PR's current diff, so a line not in that diff is rejected and surfaced as an error. + */ +export async function addLocalThreadToReview( + db: StageDb, + run: ChapterRunRow, + localThreadId: string, +): Promise { + const [thread] = db + .select() + .from(commentThread) + .where(eq(commentThread.id, localThreadId)) + .limit(1) + .all(); + if (!thread) throw new ReviewError(`Thread ${localThreadId} not found`, 404); + const comments = db + .select() + .from(comment) + .where(eq(comment.threadId, localThreadId)) + .orderBy(asc(comment.createdAt)) + .all(); + const root = comments[0]; + if (!root) throw new ReviewError("Thread has no comments to add to the review.", 400); + + const { review } = await loadTarget(run); + const reviewNodeId = await ensurePendingReview(run, review); + const side = toGitHubSide(thread.side); + const startLine = thread.endLine !== thread.startLine ? thread.startLine : null; + + const threadNodeId = await addReviewThread(run.repoRoot, { + pullRequestNodeId: review.pullRequestNodeId, + reviewNodeId, + path: thread.filePath, + body: root.body, + line: thread.endLine, + side, + startLine, + startSide: startLine !== null ? side : null, + }); + for (const reply of comments.slice(1)) { + await addReviewReply(run.repoRoot, threadNodeId, reply.body, reviewNodeId); + } + // Promoted: remove the local copy so it doesn't double up with the live pending one. + db.delete(commentThread).where(eq(commentThread.id, localThreadId)).run(); +} + +/** Reply to a GitHub thread, adding to the viewer's pending review (or as a single comment). */ +export async function replyToGitHubThread( + run: ChapterRunRow, + threadNodeId: string, + body: string, + pending: boolean, +): Promise { + if (!pending) { + await addReviewReply(run.repoRoot, threadNodeId, body, null); + return; + } + const { review } = await loadTarget(run); + const reviewNodeId = await ensurePendingReview(run, review); + await addReviewReply(run.repoRoot, threadNodeId, body, reviewNodeId); +} + +/** Submit the viewer's pending review with the chosen event, opening one if needed (e.g. a bare approval). */ +export async function submitRunReview( + run: ChapterRunRow, + event: ReviewEvent, + body: string, +): Promise { + const { review } = await loadTarget(run); + const reviewNodeId = await ensurePendingReview(run, review); + await submitReview(run.repoRoot, review.pullRequestNodeId, reviewNodeId, event, body); +} + +/** Discard the viewer's pending review and all its draft comments. */ +export async function discardRunReview(run: ChapterRunRow): Promise { + const { review } = await loadTarget(run); + if (review.pendingReviewNodeId === null) { + throw new ReviewError("There's no pending review to discard.", 409); + } + await discardReview(run.repoRoot, review.pendingReviewNodeId); +} + +/** Edit a GitHub review comment by node id (used for pending comments). */ +export async function editGitHubComment( + run: ChapterRunRow, + nodeId: string, + body: string, +): Promise { + await updateReviewComment(run.repoRoot, nodeId, body); +} + +/** Delete a pending GitHub review comment by node id. */ +export async function deleteGitHubComment(run: ChapterRunRow, nodeId: string): Promise { + await deleteReviewComment(run.repoRoot, nodeId); +} + +/** Resolve or reopen a GitHub review thread. */ +export async function resolveGitHubThread( + run: ChapterRunRow, + threadNodeId: string, + resolved: boolean, +): Promise { + await setThreadResolved(run.repoRoot, threadNodeId, resolved); +} diff --git a/packages/cli/src/show.ts b/packages/cli/src/show.ts index 2d71487..8b48f98 100644 --- a/packages/cli/src/show.ts +++ b/packages/cli/src/show.ts @@ -10,6 +10,7 @@ import { commentRoutes } from "./routes/comments.js"; import { diffRoutes } from "./routes/diff.js"; import { pullRequestRoutes } from "./routes/pull-request.js"; import { pullRequestMutationRoutes } from "./routes/pull-request-mutations.js"; +import { reviewRoutes } from "./routes/review.js"; import { runRoutes } from "./routes/runs.js"; import { viewStateRoutes } from "./routes/view-state.js"; import { viewerRoutes } from "./routes/viewer.js"; @@ -36,6 +37,7 @@ export async function show(jsonPath: string, options: DiffScopeOptions): Promise ...runRoutes(db), ...viewStateRoutes(db), ...commentRoutes(db), + ...reviewRoutes(db), ...viewerRoutes(), ...diffRoutes(db), ...pullRequestRoutes(db), diff --git a/packages/types/package.json b/packages/types/package.json index f9635a9..1e14d60 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -12,6 +12,7 @@ "./parsed-diff": "./src/parsed-diff.ts", "./prologue": "./src/prologue.ts", "./pull-request": "./src/pull-request.ts", + "./review": "./src/review.ts", "./view-state": "./src/view-state.ts", "./viewer": "./src/viewer.ts" }, diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 18eb175..ff6b3a3 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -4,5 +4,6 @@ export * from "./diff.ts"; export * from "./parsed-diff.ts"; export * from "./prologue.ts"; export * from "./pull-request.ts"; +export * from "./review.ts"; export * from "./view-state.ts"; export * from "./viewer.ts"; diff --git a/packages/types/src/review.ts b/packages/types/src/review.ts new file mode 100644 index 0000000..fd90f68 --- /dev/null +++ b/packages/types/src/review.ts @@ -0,0 +1,126 @@ +import { z } from "zod"; +import { DIFF_SIDE } from "./chapters.ts"; + +// A comment's lifecycle state. `local` lives only in the CLI; `pending` is a draft +// on the viewer's unsubmitted GitHub review (only they see it); `submitted` is +// published on the PR for everyone. +export const COMMENT_STATE = { + LOCAL: "local", + PENDING: "pending", + SUBMITTED: "submitted", +} as const; +export type CommentState = (typeof COMMENT_STATE)[keyof typeof COMMENT_STATE]; + +// Where a thread originates. `local` threads are CLI-only rows; `github` threads +// are live review threads on the PR (pending and/or submitted comments). +export const THREAD_SOURCE = { + LOCAL: "local", + GITHUB: "github", +} as const; +export type ThreadSource = (typeof THREAD_SOURCE)[keyof typeof THREAD_SOURCE]; + +// The events a review can be submitted with, mirroring GitHub's review model. +export const REVIEW_EVENT = { + COMMENT: "COMMENT", + APPROVE: "APPROVE", + REQUEST_CHANGES: "REQUEST_CHANGES", +} as const; +export type ReviewEvent = (typeof REVIEW_EVENT)[keyof typeof REVIEW_EVENT]; + +// Whether the GitHub review layer is usable for this run. `none` = not a GitHub PR; +// `offline` = `gh` is missing/unauthenticated/unreachable (GitHub actions disabled); +// `available` = the PR's review state loaded. +export const GITHUB_REVIEW_STATUS = { + NONE: "none", + OFFLINE: "offline", + AVAILABLE: "available", +} as const; +export type GitHubReviewStatus = (typeof GITHUB_REVIEW_STATUS)[keyof typeof GITHUB_REVIEW_STATUS]; + +export const ReviewCommentAuthorSchema = z.object({ + login: z.string(), + avatarUrl: z.string().nullable(), +}); +export type ReviewCommentAuthor = z.infer; + +// A single comment in a thread. `author` is null for local comments (the local +// reviewer, "You"). `nodeId` is the GitHub GraphQL id, present for github comments +// (needed to edit/delete pending ones). `commentId` is the local row id for local +// comments. +export const ReviewCommentSchema = z.object({ + id: z.string(), + state: z.enum(COMMENT_STATE), + body: z.string(), + author: ReviewCommentAuthorSchema.nullable(), + nodeId: z.string().nullable(), + createdAt: z.string(), +}); +export type ReviewComment = z.infer; + +// A line-anchored thread. Local and github threads share this shape so the diff +// viewer can place both uniformly; `side`/`startLine`/`endLine` are normalized to +// the local diff convention. `threadNodeId` is set for github threads (resolve/reply). +export const ReviewThreadSchema = z.object({ + id: z.string(), + source: z.enum(THREAD_SOURCE), + threadNodeId: z.string().nullable(), + filePath: z.string(), + side: z.enum(DIFF_SIDE), + startLine: z.number().int().positive(), + endLine: z.number().int().positive(), + isResolved: z.boolean(), + comments: z.array(ReviewCommentSchema), +}); +export type ReviewThread = z.infer; + +export const ReviewResponseSchema = z.object({ + github: z.enum(GITHUB_REVIEW_STATUS), + threads: z.array(ReviewThreadSchema), + // Count of the viewer's pending (draft) comments, for the review tray badge. + pendingCommentCount: z.number().int().nonnegative(), + hasPendingReview: z.boolean(), +}); +export type ReviewResponse = z.infer; + +// ─── Action bodies ────────────────────────────────────────────────────────────── + +// Promote a local comment to a pending GitHub review comment (or reply). The local +// thread/comment is identified by id; the server reads its anchor + body. +export const AddToReviewBodySchema = z.object({ + localThreadId: z.string().min(1), +}); +export type AddToReviewBody = z.infer; + +export const SubmitReviewBodySchema = z.object({ + event: z.enum(REVIEW_EVENT), + body: z.string(), +}); +export type SubmitReviewBody = z.infer; + +// Reply to a github thread. `pending` (default) adds the reply to the viewer's +// pending review; false posts it immediately as a single comment. +export const GitHubReplyBodySchema = z.object({ + threadNodeId: z.string().min(1), + body: z.string().min(1), + pending: z.boolean().default(true), +}); +export type GitHubReplyBody = z.infer; + +// Edit/delete a pending github comment by node id. +export const GitHubCommentEditBodySchema = z.object({ + nodeId: z.string().min(1), + body: z.string().min(1), +}); +export type GitHubCommentEditBody = z.infer; + +export const GitHubCommentDeleteBodySchema = z.object({ + nodeId: z.string().min(1), +}); +export type GitHubCommentDeleteBody = z.infer; + +// Resolve/reopen a github thread by node id. +export const GitHubResolveBodySchema = z.object({ + threadNodeId: z.string().min(1), + resolved: z.boolean(), +}); +export type GitHubResolveBody = z.infer; From d3205016e0ab6caccd259b8b0d74ff0bea7ccc48 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 22 Jun 2026 15:59:56 +0200 Subject: [PATCH 06/17] feat(PRO-547): rebuild comment UI on live-fetch review model; remove mirror --- .../drizzle/0008_supreme_mother_askani.sql | 3 + packages/cli/drizzle/meta/0008_snapshot.json | 775 ++++++++++++++++++ packages/cli/drizzle/meta/_journal.json | 7 + .../src/__tests__/comment-sync.routes.test.ts | 412 ---------- .../cli/src/__tests__/comments.routes.test.ts | 3 +- .../__tests__/review-comment-mapping.test.ts | 81 -- packages/cli/src/db/schema/comment.ts | 19 +- packages/cli/src/github/index.ts | 12 - packages/cli/src/github/review-comments.ts | 265 ------ packages/cli/src/routes/comments.ts | 95 +-- packages/cli/src/runs/comment-sync.ts | 368 --------- .../cli/src/runs/review-comment-mapping.ts | 55 -- packages/types/src/comments.ts | 46 +- packages/web/src/app/runs.$runId.tsx | 6 +- .../components/chapter/pierre-diff-viewer.tsx | 16 +- .../{comment-thread.tsx => review-thread.tsx} | 256 ++++-- .../pull-request/comment-sync-menu.tsx | 112 --- .../components/pull-request/review-panel.tsx | 211 +++++ .../src/lib/__tests__/comment-drafts.test.ts | 8 +- .../comment-threads-context.test.tsx | 92 --- packages/web/src/lib/comment-drafts.ts | 2 +- .../web/src/lib/comment-threads-context.tsx | 48 -- packages/web/src/lib/review-context.tsx | 37 + packages/web/src/lib/use-comment-sync.ts | 52 -- packages/web/src/lib/use-comment-threads.ts | 160 ---- packages/web/src/lib/use-review.ts | 186 +++++ .../web/src/routes/pull-request-layout.tsx | 4 +- 27 files changed, 1428 insertions(+), 1903 deletions(-) create mode 100644 packages/cli/drizzle/0008_supreme_mother_askani.sql create mode 100644 packages/cli/drizzle/meta/0008_snapshot.json delete mode 100644 packages/cli/src/__tests__/comment-sync.routes.test.ts delete mode 100644 packages/cli/src/__tests__/review-comment-mapping.test.ts delete mode 100644 packages/cli/src/github/review-comments.ts delete mode 100644 packages/cli/src/runs/comment-sync.ts delete mode 100644 packages/cli/src/runs/review-comment-mapping.ts rename packages/web/src/components/comments/{comment-thread.tsx => review-thread.tsx} (55%) delete mode 100644 packages/web/src/components/pull-request/comment-sync-menu.tsx create mode 100644 packages/web/src/components/pull-request/review-panel.tsx delete mode 100644 packages/web/src/lib/__tests__/comment-threads-context.test.tsx delete mode 100644 packages/web/src/lib/comment-threads-context.tsx create mode 100644 packages/web/src/lib/review-context.tsx delete mode 100644 packages/web/src/lib/use-comment-sync.ts delete mode 100644 packages/web/src/lib/use-comment-threads.ts create mode 100644 packages/web/src/lib/use-review.ts diff --git a/packages/cli/drizzle/0008_supreme_mother_askani.sql b/packages/cli/drizzle/0008_supreme_mother_askani.sql new file mode 100644 index 0000000..4502898 --- /dev/null +++ b/packages/cli/drizzle/0008_supreme_mother_askani.sql @@ -0,0 +1,3 @@ +DROP INDEX `comment_github_comment_id_idx`;--> statement-breakpoint +ALTER TABLE `comment` DROP COLUMN `authorAvatarUrl`;--> statement-breakpoint +ALTER TABLE `comment` DROP COLUMN `githubCommentId`; \ No newline at end of file diff --git a/packages/cli/drizzle/meta/0008_snapshot.json b/packages/cli/drizzle/meta/0008_snapshot.json new file mode 100644 index 0000000..68dcb73 --- /dev/null +++ b/packages/cli/drizzle/meta/0008_snapshot.json @@ -0,0 +1,775 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "66f19db5-2868-4224-bcde-6f0ed98bedd5", + "prevId": "de6a20c0-0b5b-4353-aa2e-611d0300530b", + "tables": { + "chapter": { + "name": "chapter", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "runId": { + "name": "runId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "externalId": { + "name": "externalId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "chapterIndex": { + "name": "chapterIndex", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "summary": { + "name": "summary", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "hunkRefs": { + "name": "hunkRefs", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "keyChanges": { + "name": "keyChanges", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + } + }, + "indexes": { + "chapter_run_idx_unique": { + "name": "chapter_run_idx_unique", + "columns": [ + "runId", + "chapterIndex" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_runId_chapter_run_id_fk": { + "name": "chapter_runId_chapter_run_id_fk", + "tableFrom": "chapter", + "tableTo": "chapter_run", + "columnsFrom": [ + "runId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_file_view": { + "name": "chapter_file_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "chapter_file_view_chapter_id_idx": { + "name": "chapter_file_view_chapter_id_idx", + "columns": [ + "chapterId" + ], + "isUnique": false + }, + "chapter_file_view_user_chapter_path_unique": { + "name": "chapter_file_view_user_chapter_path_unique", + "columns": [ + "userId", + "chapterId", + "filePath" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_file_view_chapterId_chapter_id_fk": { + "name": "chapter_file_view_chapterId_chapter_id_fk", + "tableFrom": "chapter_file_view", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_run": { + "name": "chapter_run", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "repoRoot": { + "name": "repoRoot", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "originUrl": { + "name": "originUrl", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "prNumber": { + "name": "prNumber", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "scopeKind": { + "name": "scopeKind", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "workingTreeRef": { + "name": "workingTreeRef", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "baseSha": { + "name": "baseSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "headSha": { + "name": "headSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "mergeBaseSha": { + "name": "mergeBaseSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "generatedAt": { + "name": "generatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "prologue": { + "name": "prologue", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "chapter_run_created_at_idx": { + "name": "chapter_run_created_at_idx", + "columns": [ + "createdAt" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_view": { + "name": "chapter_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "chapter_view_user_chapter_unique": { + "name": "chapter_view_user_chapter_unique", + "columns": [ + "userId", + "chapterId" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_view_chapterId_chapter_id_fk": { + "name": "chapter_view_chapterId_chapter_id_fk", + "tableFrom": "chapter_view", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "comment": { + "name": "comment", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "threadId": { + "name": "threadId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "authorId": { + "name": "authorId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "body": { + "name": "body", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "comment_thread_id_idx": { + "name": "comment_thread_id_idx", + "columns": [ + "threadId" + ], + "isUnique": false + } + }, + "foreignKeys": { + "comment_threadId_comment_thread_id_fk": { + "name": "comment_threadId_comment_thread_id_fk", + "tableFrom": "comment", + "tableTo": "comment_thread", + "columnsFrom": [ + "threadId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "comment_thread": { + "name": "comment_thread", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "scopeKey": { + "name": "scopeKey", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "side": { + "name": "side", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "startLine": { + "name": "startLine", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "endLine": { + "name": "endLine", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "resolvedAt": { + "name": "resolvedAt", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "comment_thread_scope_key_idx": { + "name": "comment_thread_scope_key_idx", + "columns": [ + "scopeKey" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "file_view": { + "name": "file_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "runId": { + "name": "runId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "file_view_user_run_path_unique": { + "name": "file_view_user_run_path_unique", + "columns": [ + "userId", + "runId", + "filePath" + ], + "isUnique": true + } + }, + "foreignKeys": { + "file_view_runId_chapter_run_id_fk": { + "name": "file_view_runId_chapter_run_id_fk", + "tableFrom": "file_view", + "tableTo": "chapter_run", + "columnsFrom": [ + "runId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "key_change": { + "name": "key_change", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "externalId": { + "name": "externalId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "lineRefs": { + "name": "lineRefs", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + } + }, + "indexes": { + "key_change_chapter_id_idx": { + "name": "key_change_chapter_id_idx", + "columns": [ + "chapterId" + ], + "isUnique": false + } + }, + "foreignKeys": { + "key_change_chapterId_chapter_id_fk": { + "name": "key_change_chapterId_chapter_id_fk", + "tableFrom": "key_change", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "key_change_view": { + "name": "key_change_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "keyChangeId": { + "name": "keyChangeId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "key_change_view_key_change_id_idx": { + "name": "key_change_view_key_change_id_idx", + "columns": [ + "keyChangeId" + ], + "isUnique": false + }, + "key_change_view_user_key_change_unique": { + "name": "key_change_view_user_key_change_unique", + "columns": [ + "userId", + "keyChangeId" + ], + "isUnique": true + } + }, + "foreignKeys": { + "key_change_view_keyChangeId_key_change_id_fk": { + "name": "key_change_view_keyChangeId_key_change_id_fk", + "tableFrom": "key_change_view", + "tableTo": "key_change", + "columnsFrom": [ + "keyChangeId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/packages/cli/drizzle/meta/_journal.json b/packages/cli/drizzle/meta/_journal.json index ba7fdb7..454ce01 100644 --- a/packages/cli/drizzle/meta/_journal.json +++ b/packages/cli/drizzle/meta/_journal.json @@ -57,6 +57,13 @@ "when": 1782119754913, "tag": "0007_sloppy_leopardon", "breakpoints": true + }, + { + "idx": 8, + "version": "6", + "when": 1782136596225, + "tag": "0008_supreme_mother_askani", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/cli/src/__tests__/comment-sync.routes.test.ts b/packages/cli/src/__tests__/comment-sync.routes.test.ts deleted file mode 100644 index a70e32d..0000000 --- a/packages/cli/src/__tests__/comment-sync.routes.test.ts +++ /dev/null @@ -1,412 +0,0 @@ -import fs from "node:fs/promises"; -import http from "node:http"; -import os from "node:os"; -import path from "node:path"; -import type { PullCommentsResult, PushCommentsResult } from "@stagereview/types/comments"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { closeDb, getDb } from "../db/client.js"; -import { chapterRun, comment, commentThread } from "../db/schema/index.js"; -import { commentRoutes } from "../routes/comments.js"; -import { SCOPE_KIND, WORKING_TREE_REF } from "../schema.js"; -import { LOOPBACK_HOST, type ServerHandle, startServer } from "../server.js"; - -let tmpDir: string; -let dbPath: string; -let webDist: string; -let repoRoot: string; -let binDir: string; -let originalPath: string | undefined; -const handles: ServerHandle[] = []; - -const HEAD_SHA = "a".repeat(40); -const GITHUB_ORIGIN = "git@github.com:owner/repo.git"; - -// REST review comments the fake `gh` returns for a pull (one root + one reply). -const REVIEW_COMMENTS = [ - [ - { - id: 101, - in_reply_to_id: null, - path: "src/foo.ts", - line: 10, - start_line: 5, - side: "RIGHT", - body: "Root comment", - created_at: "2026-01-01T00:00:00Z", - user: { login: "octocat", avatar_url: "https://example.com/octocat.png", type: "User" }, - }, - { - id: 102, - in_reply_to_id: 101, - path: "src/foo.ts", - line: 10, - start_line: null, - side: "RIGHT", - body: "A reply", - created_at: "2026-01-02T00:00:00Z", - user: { login: "hubot", avatar_url: "https://example.com/hubot.png", type: "Bot" }, - }, - ], -]; - -// `gh`/`git` shims that route on argv and emit canned JSON, so the sync paths run -// end-to-end without network or a real repo. Infrastructure fakes, not mocks. -async function writeShims(opts: { - gitHead: string; - gitStatus: string; - resolvedRootIds?: number[]; -}): Promise { - const ghFixture = { - pr: { - number: 5, - title: "Add foo", - body: "", - url: "https://github.com/owner/repo/pull/5", - state: "OPEN", - isDraft: false, - mergedAt: null, - createdAt: "2026-01-01T00:00:00Z", - author: { login: "octocat", is_bot: false }, - headRefName: "feature", - headRefOid: HEAD_SHA, - baseRefName: "main", - }, - restPr: { - user: { login: "octocat", avatar_url: "https://example.com/octocat.png", type: "User" }, - requested_reviewers: [], - }, - graphql: { - data: { - repository: { - pullRequest: { - reviewThreads: { - pageInfo: { hasNextPage: false, endCursor: null }, - // One thread node per root comment, carrying its node id (for resolve/reopen) - // and resolution state (for pull mirroring). - nodes: REVIEW_COMMENTS.flat() - .filter((c) => c.in_reply_to_id == null) - .map((c) => ({ - id: `THREAD_NODE_${c.id}`, - isResolved: (opts.resolvedRootIds ?? []).includes(c.id), - comments: { nodes: [{ databaseId: c.id }] }, - })), - }, - }, - }, - }, - }, - comments: REVIEW_COMMENTS, - }; - await fs.writeFile(path.join(tmpDir, "gh-fixture.json"), JSON.stringify(ghFixture)); - - const ghShim = `#!/usr/bin/env node -const fs = require("node:fs"); -const args = process.argv.slice(2); -const fx = JSON.parse(fs.readFileSync(${JSON.stringify(path.join(tmpDir, "gh-fixture.json"))}, "utf8")); -const log = ${JSON.stringify(path.join(tmpDir, "gh-log.txt"))}; -const counterFile = ${JSON.stringify(path.join(tmpDir, "gh-counter.txt"))}; -function nextId() { - let n = 1000; - try { n = parseInt(fs.readFileSync(counterFile, "utf8"), 10); } catch {} - n += 1; - fs.writeFileSync(counterFile, String(n)); - return n; -} -if (args[0] === "pr" && args[1] === "view") { - process.stdout.write(JSON.stringify(fx.pr)); -} else if (args[0] === "api" && args[1] === "graphql") { - const joined = args.join(" "); - if (joined.includes("unresolveReviewThread")) { - fs.appendFileSync(log, "unresolve " + joined + "\\n"); - process.stdout.write("{}"); - } else if (joined.includes("resolveReviewThread")) { - fs.appendFileSync(log, "resolve " + joined + "\\n"); - process.stdout.write("{}"); - } else { - process.stdout.write(JSON.stringify(fx.graphql)); - } -} else if (args[0] === "api") { - const endpoint = args[1]; - const isPost = args.includes("POST"); - if (/\\/comments$/.test(endpoint) && isPost) { - fs.appendFileSync(log, "create " + args.join(" ") + "\\n"); - process.stdout.write(JSON.stringify({ id: nextId() })); - } else if (/\\/replies$/.test(endpoint) && isPost) { - fs.appendFileSync(log, "reply " + args.join(" ") + "\\n"); - process.stdout.write(JSON.stringify({ id: nextId() })); - } else if (/\\/comments$/.test(endpoint)) { - process.stdout.write(JSON.stringify(fx.comments)); - } else if (/\\/pulls\\/\\d+$/.test(endpoint)) { - process.stdout.write(JSON.stringify(fx.restPr)); - } else { - process.stdout.write("{}"); - } -} -`; - await fs.writeFile(path.join(binDir, "gh"), ghShim); - await fs.chmod(path.join(binDir, "gh"), 0o755); - - const gitShim = `#!/usr/bin/env node -const args = process.argv.slice(2); -if (args.includes("rev-parse")) process.stdout.write(${JSON.stringify(opts.gitHead)}); -else if (args.includes("status")) process.stdout.write(${JSON.stringify(opts.gitStatus)}); -`; - await fs.writeFile(path.join(binDir, "git"), gitShim); - await fs.chmod(path.join(binDir, "git"), 0o755); -} - -beforeEach(async () => { - tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-sync-")); - dbPath = path.join(tmpDir, "db.sqlite"); - webDist = path.join(tmpDir, "web-dist"); - repoRoot = path.join(tmpDir, "repo"); - binDir = path.join(tmpDir, "bin"); - await fs.mkdir(webDist); - await fs.writeFile(path.join(webDist, "index.html"), ""); - await fs.mkdir(repoRoot); - await fs.mkdir(binDir); - originalPath = process.env.PATH; - // Shim dir first so `gh`/`git` resolve to the fakes. - process.env.PATH = `${binDir}${path.delimiter}${originalPath ?? ""}`; - closeDb(); -}); - -afterEach(async () => { - while (handles.length > 0) { - const h = handles.pop(); - if (h) await h.close(); - } - closeDb(); - process.env.PATH = originalPath; - await fs.rm(tmpDir, { recursive: true, force: true }); -}); - -function insertRun(scopeKind: string = SCOPE_KIND.COMMITTED): string { - const db = getDb({ dbPath }); - const [row] = db - .insert(chapterRun) - .values({ - repoRoot, - originUrl: GITHUB_ORIGIN, - scopeKind: - scopeKind === SCOPE_KIND.COMMITTED ? SCOPE_KIND.COMMITTED : SCOPE_KIND.WORKING_TREE, - workingTreeRef: scopeKind === SCOPE_KIND.COMMITTED ? null : WORKING_TREE_REF.WORK, - baseSha: "b".repeat(40), - headSha: HEAD_SHA, - mergeBaseSha: "c".repeat(40), - generatedAt: new Date(), - }) - .returning({ id: chapterRun.id }) - .all(); - if (!row) throw new Error("seed: chapter_run insert returned no row"); - return row.id; -} - -function seedLocalThread(): void { - const db = getDb({ dbPath }); - const scopeKey = `committed:${"b".repeat(40)}:${HEAD_SHA}:${"c".repeat(40)}`; - const [thread] = db - .insert(commentThread) - .values({ scopeKey, filePath: "src/foo.ts", side: "additions", startLine: 3, endLine: 3 }) - .returning({ id: commentThread.id }) - .all(); - if (!thread) throw new Error("seed: thread insert returned no row"); - db.insert(comment).values({ threadId: thread.id, body: "Push me" }).run(); -} - -async function start(): Promise { - const db = getDb({ dbPath }); - const handle = await startServer({ webDistPath: webDist, routes: commentRoutes(db) }); - handles.push(handle); - return handle.port; -} - -function request( - port: number, - method: string, - p: string, - body?: unknown, -): Promise<{ status: number; body: string }> { - return new Promise((resolve, reject) => { - const payload = body === undefined ? undefined : JSON.stringify(body); - const req = http.request( - { - hostname: LOOPBACK_HOST, - port, - method, - path: p, - agent: false, - headers: - payload === undefined - ? {} - : { "Content-Type": "application/json", "Content-Length": Buffer.byteLength(payload) }, - }, - (res) => { - const chunks: Buffer[] = []; - res.on("data", (c: Buffer) => chunks.push(c)); - res.on("end", () => - resolve({ status: res.statusCode ?? 0, body: Buffer.concat(chunks).toString("utf8") }), - ); - }, - ); - req.on("error", reject); - req.end(payload); - }); -} - -function post(port: number, p: string): Promise<{ status: number; body: string }> { - return request(port, "POST", p); -} - -describe("comment sync API — pull", () => { - it("imports a PR's review comments, then is idempotent on re-pull", async () => { - await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); - const runId = insertRun(); - const port = await start(); - - const first = await post(port, `/api/runs/${runId}/comment-sync/pull`); - expect(first.status).toBe(200); - expect(JSON.parse(first.body) as PullCommentsResult).toEqual({ pulled: 2, skipped: 0 }); - - const db = getDb({ dbPath }); - const rows = db.select().from(comment).all(); - expect(rows).toHaveLength(2); - const root = rows.find((r) => r.githubCommentId === 101); - expect(root?.authorId).toBe("octocat"); - expect(root?.authorAvatarUrl).toBe("https://example.com/octocat.png"); - expect(db.select().from(commentThread).all()).toHaveLength(1); - - const second = await post(port, `/api/runs/${runId}/comment-sync/pull`); - expect(JSON.parse(second.body) as PullCommentsResult).toEqual({ pulled: 0, skipped: 2 }); - expect(db.select().from(comment).all()).toHaveLength(2); - }); - - it("mirrors the PR's resolved state on pull — resolving, then reopening", async () => { - // Thread 101 starts resolved on GitHub. - await writeShims({ gitHead: HEAD_SHA, gitStatus: "", resolvedRootIds: [101] }); - const runId = insertRun(); - const port = await start(); - - await post(port, `/api/runs/${runId}/comment-sync/pull`); - const db = getDb({ dbPath }); - expect(db.select().from(commentThread).all()[0]?.resolvedAt).not.toBeNull(); - - // Reopened on GitHub since the last pull → the next pull reflects that locally. - await writeShims({ gitHead: HEAD_SHA, gitStatus: "", resolvedRootIds: [] }); - await post(port, `/api/runs/${runId}/comment-sync/pull`); - expect(db.select().from(commentThread).all()[0]?.resolvedAt).toBeNull(); - }); -}); - -describe("comment sync API — push guardrails", () => { - it("rejects pushing comments on a working-tree scope", async () => { - await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); - const runId = insertRun(SCOPE_KIND.WORKING_TREE); - const res = await post(await start(), `/api/runs/${runId}/comment-sync/push`); - expect(res.status).toBe(409); - expect(JSON.parse(res.body).error).toMatch(/committed diff/i); - }); - - it("rejects pushing with a dirty working tree", async () => { - await writeShims({ gitHead: HEAD_SHA, gitStatus: " M src/foo.ts" }); - const runId = insertRun(); - const res = await post(await start(), `/api/runs/${runId}/comment-sync/push`); - expect(res.status).toBe(409); - expect(JSON.parse(res.body).error).toMatch(/uncommitted changes/i); - }); - - it("rejects pushing when local HEAD doesn't match the PR head", async () => { - await writeShims({ gitHead: "f".repeat(40), gitStatus: "" }); - const runId = insertRun(); - const res = await post(await start(), `/api/runs/${runId}/comment-sync/push`); - expect(res.status).toBe(409); - expect(JSON.parse(res.body).error).toMatch(/HEAD doesn't match/i); - }); -}); - -describe("comment sync API — push", () => { - it("creates a review comment for a local thread, recording its GitHub id, then skips on re-push", async () => { - await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); - const runId = insertRun(); - seedLocalThread(); - const port = await start(); - - const first = await post(port, `/api/runs/${runId}/comment-sync/push`); - expect(first.status).toBe(200); - const firstResult = JSON.parse(first.body) as PushCommentsResult; - expect(firstResult.pushed).toBe(1); - expect(firstResult.skipped).toBe(0); - expect(firstResult.failed).toEqual([]); - - const db = getDb({ dbPath }); - const [row] = db.select().from(comment).all(); - expect(row?.githubCommentId).toBe(1001); - - const second = await post(port, `/api/runs/${runId}/comment-sync/push`); - const secondResult = JSON.parse(second.body) as PushCommentsResult; - expect(secondResult).toEqual({ pushed: 0, skipped: 1, failed: [] }); - - const log = await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8"); - expect(log.split("\n").filter((l) => l.startsWith("create"))).toHaveLength(1); - }); -}); - -describe("comment sync API — resolution", () => { - it("mirrors a local resolve/reopen of a PR-originated thread to GitHub", async () => { - await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); - const runId = insertRun(); - const port = await start(); - await post(port, `/api/runs/${runId}/comment-sync/pull`); - - const db = getDb({ dbPath }); - const [thread] = db.select().from(commentThread).all(); - if (!thread) throw new Error("expected a pulled thread"); - - const resolved = await request( - port, - "PATCH", - `/api/runs/${runId}/comment-threads/${thread.id}`, - { - resolved: true, - }, - ); - expect(resolved.status).toBe(200); - expect(db.select().from(commentThread).all()[0]?.resolvedAt).not.toBeNull(); - - const reopened = await request( - port, - "PATCH", - `/api/runs/${runId}/comment-threads/${thread.id}`, - { - resolved: false, - }, - ); - expect(reopened.status).toBe(200); - expect(db.select().from(commentThread).all()[0]?.resolvedAt).toBeNull(); - - const lines = (await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8")).split("\n"); - expect(lines.filter((l) => l.startsWith("resolve"))).toHaveLength(1); - expect(lines.filter((l) => l.startsWith("unresolve"))).toHaveLength(1); - }); - - it("keeps a local-only thread's resolve off GitHub", async () => { - await writeShims({ gitHead: HEAD_SHA, gitStatus: "" }); - const runId = insertRun(); - seedLocalThread(); - const port = await start(); - - const db = getDb({ dbPath }); - const [thread] = db.select().from(commentThread).all(); - if (!thread) throw new Error("expected a local thread"); - - const res = await request(port, "PATCH", `/api/runs/${runId}/comment-threads/${thread.id}`, { - resolved: true, - }); - expect(res.status).toBe(200); - expect(db.select().from(commentThread).all()[0]?.resolvedAt).not.toBeNull(); - - // No GitHub mutation for a thread that never lived on the PR. - const log = await fs.readFile(path.join(tmpDir, "gh-log.txt"), "utf8").catch(() => ""); - expect(log).not.toMatch(/resolve/); - }); -}); diff --git a/packages/cli/src/__tests__/comments.routes.test.ts b/packages/cli/src/__tests__/comments.routes.test.ts index 7acd492..c040cd5 100644 --- a/packages/cli/src/__tests__/comments.routes.test.ts +++ b/packages/cli/src/__tests__/comments.routes.test.ts @@ -133,8 +133,7 @@ describe("comment threads API", () => { expect(thread.resolvedAt).toBeNull(); expect(thread.comments).toHaveLength(1); expect(thread.comments[0]?.body).toBe("First!"); - expect(thread.comments[0]?.author).toBeNull(); - expect(thread.comments[0]?.githubCommentId).toBeNull(); + expect(thread.comments[0]?.authorId).toBe("local"); const db = getDb({ dbPath }); expect(db.select().from(commentThread).all()).toHaveLength(1); diff --git a/packages/cli/src/__tests__/review-comment-mapping.test.ts b/packages/cli/src/__tests__/review-comment-mapping.test.ts deleted file mode 100644 index 83e10e2..0000000 --- a/packages/cli/src/__tests__/review-comment-mapping.test.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { describe, expect, it } from "vitest"; -import type { ReviewComment } from "../github/index.js"; -import { - fromGitHubSide, - groupReviewComments, - toGitHubSide, -} from "../runs/review-comment-mapping.js"; - -function comment(over: Partial & { id: number }): ReviewComment { - return { - in_reply_to_id: null, - path: "src/foo.ts", - line: 10, - start_line: null, - side: "RIGHT", - body: "body", - created_at: "2026-01-01T00:00:00Z", - user: { login: "octocat", avatar_url: "https://example.com/a.png", type: "User" }, - ...over, - }; -} - -describe("side mapping", () => { - it("maps local sides to GitHub diff sides", () => { - expect(toGitHubSide("deletions")).toBe("LEFT"); - expect(toGitHubSide("additions")).toBe("RIGHT"); - }); - - it("maps GitHub diff sides back to local sides, defaulting unknown to additions", () => { - expect(fromGitHubSide("LEFT")).toBe("deletions"); - expect(fromGitHubSide("RIGHT")).toBe("additions"); - expect(fromGitHubSide(null)).toBe("additions"); - expect(fromGitHubSide(undefined)).toBe("additions"); - }); -}); - -describe("groupReviewComments", () => { - it("nests replies under their root and orders them oldest-first", () => { - const threads = groupReviewComments([ - comment({ id: 1, body: "root" }), - comment({ - id: 3, - in_reply_to_id: 1, - body: "second reply", - created_at: "2026-01-03T00:00:00Z", - }), - comment({ - id: 2, - in_reply_to_id: 1, - body: "first reply", - created_at: "2026-01-02T00:00:00Z", - }), - ]); - expect(threads).toHaveLength(1); - expect(threads[0]?.root.id).toBe(1); - expect(threads[0]?.replies.map((r) => r.id)).toEqual([2, 3]); - }); - - it("derives the anchor from start_line/line and side", () => { - const [thread] = groupReviewComments([ - comment({ id: 1, side: "LEFT", start_line: 4, line: 8, path: "src/bar.ts" }), - ]); - expect(thread).toMatchObject({ - filePath: "src/bar.ts", - side: "deletions", - startLine: 4, - endLine: 8, - }); - }); - - it("falls back to line for single-line comments", () => { - const [thread] = groupReviewComments([comment({ id: 1, start_line: null, line: 12 })]); - expect(thread?.startLine).toBe(12); - expect(thread?.endLine).toBe(12); - }); - - it("drops comments with no anchorable line (outdated/whole-file)", () => { - const threads = groupReviewComments([comment({ id: 1, line: null })]); - expect(threads).toEqual([]); - }); -}); diff --git a/packages/cli/src/db/schema/comment.ts b/packages/cli/src/db/schema/comment.ts index c7c9785..85fa9cf 100644 --- a/packages/cli/src/db/schema/comment.ts +++ b/packages/cli/src/db/schema/comment.ts @@ -1,8 +1,10 @@ -import { index, integer, sqliteTable, text, uniqueIndex } from "drizzle-orm/sqlite-core"; +import { index, sqliteTable, text } from "drizzle-orm/sqlite-core"; import { LOCAL_USER_ID } from "../local-user.js"; import { baseColumns } from "./columns.js"; import { commentThread } from "./comment-thread.js"; +// Local (CLI-only) review comments. GitHub review comments are never mirrored +// here — they're fetched live (see the `review` server layer). export const comment = sqliteTable( "comment", { @@ -10,23 +12,10 @@ export const comment = sqliteTable( threadId: text() .notNull() .references(() => commentThread.id, { onDelete: "cascade" }), - /** `local` for comments authored in the CLI; the GitHub login for pulled comments. */ authorId: text().notNull().default(LOCAL_USER_ID), - /** Avatar for non-local authors (pulled GitHub comments); null for the local user. */ - authorAvatarUrl: text(), body: text().notNull(), - /** - * GitHub review-comment database id once the comment is synced — set when a - * comment is pulled from the PR or after a local comment is pushed to it. - * Null marks a comment as local-only and not yet on GitHub. It's the dedup - * key for both directions, so re-syncing never duplicates a comment. - */ - githubCommentId: integer({ mode: "number" }), }, - (table) => [ - index("comment_thread_id_idx").on(table.threadId), - uniqueIndex("comment_github_comment_id_idx").on(table.githubCommentId), - ], + (table) => [index("comment_thread_id_idx").on(table.threadId)], ); export type CommentRow = typeof comment.$inferSelect; diff --git a/packages/cli/src/github/index.ts b/packages/cli/src/github/index.ts index 7ab0437..c77aed6 100644 --- a/packages/cli/src/github/index.ts +++ b/packages/cli/src/github/index.ts @@ -17,16 +17,4 @@ export { resolvePullRequestRefs, } from "./pull-request-ref.js"; export { type GitHubRepo, isGitHubRemote, parseGitHubRepo } from "./repo.js"; -export { - type CreateReviewCommentInput, - createReviewComment, - GITHUB_DIFF_SIDE, - type GitHubDiffSide, - listReviewComments, - listReviewThreads, - type ReviewComment, - type ReviewThreadInfo, - replyToReviewComment, - setReviewThreadResolved, -} from "./review-comments.js"; export { type GitHubViewer, getGitHubViewer } from "./viewer.js"; diff --git a/packages/cli/src/github/review-comments.ts b/packages/cli/src/github/review-comments.ts deleted file mode 100644 index 5bca1f2..0000000 --- a/packages/cli/src/github/review-comments.ts +++ /dev/null @@ -1,265 +0,0 @@ -import { execFile } from "node:child_process"; -import { promisify } from "node:util"; -import { z } from "zod"; -import { gh, ghErrorMessage } from "./exec.js"; -import type { GitHubRepo } from "./repo.js"; - -const execFileAsync = promisify(execFile); - -/** - * GitHub's diff sides. `LEFT` is the base/deletion side, `RIGHT` the head/addition - * side. These map onto the local `DIFF_SIDE` (deletions/additions) in the sync layer. - */ -export const GITHUB_DIFF_SIDE = { - LEFT: "LEFT", - RIGHT: "RIGHT", -} as const; -export type GitHubDiffSide = (typeof GITHUB_DIFF_SIDE)[keyof typeof GITHUB_DIFF_SIDE]; - -// REST review-comment shape we anchor on. `line` is null for an outdated comment -// (its line is no longer in the diff); `start_line` is null for single-line ones. -const ReviewCommentSchema = z.object({ - id: z.number(), - in_reply_to_id: z.number().nullable().optional(), - path: z.string(), - line: z.number().nullable(), - start_line: z.number().nullable().optional(), - side: z.enum(GITHUB_DIFF_SIDE).nullable().optional(), - body: z.string(), - created_at: z.string(), - user: z.object({ login: z.string(), avatar_url: z.string(), type: z.string() }).nullable(), -}); -export type ReviewComment = z.infer; - -/** - * All review comments on a PR, oldest-first across pages. Unlike the read - * adapters that back passive PR context, sync is user-initiated, so a `gh` - * failure throws rather than degrading to an empty list. - */ -export async function listReviewComments( - repoRoot: string, - repo: GitHubRepo, - prNumber: number, -): Promise { - // `--slurp` wraps each page in one array (`[[…], […]]`) so multi-page output stays valid JSON. - const stdout = await ghOrThrow( - ["api", `repos/${repo.owner}/${repo.repo}/pulls/${prNumber}/comments`, "--paginate", "--slurp"], - repoRoot, - ); - const parsed = z.array(z.array(ReviewCommentSchema)).safeParse(JSON.parse(stdout)); - if (!parsed.success) throw new Error("Unexpected response shape from GitHub review comments"); - return parsed.data.flat(); -} - -// ─── Review-thread metadata (GraphQL) ─────────────────────────────────────────── - -const REVIEW_THREADS_QUERY = `query GetReviewThreads($owner: String!, $repo: String!, $number: Int!, $cursor: String) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $number) { - reviewThreads(first: 100, after: $cursor) { - pageInfo { hasNextPage endCursor } - nodes { - id - isResolved - comments(first: 1) { nodes { databaseId } } - } - } - } - } -}`; - -const ReviewThreadsSchema = z.object({ - data: z.object({ - repository: z - .object({ - pullRequest: z - .object({ - reviewThreads: z.object({ - pageInfo: z.object({ hasNextPage: z.boolean(), endCursor: z.string().nullable() }), - nodes: z.array( - z.object({ - id: z.string(), - isResolved: z.boolean(), - comments: z.object({ - nodes: z.array(z.object({ databaseId: z.number().nullable() })), - }), - }), - ), - }), - }) - .nullable(), - }) - .nullable(), - }), -}); - -/** A PR review thread's GraphQL node id and resolution state. */ -export interface ReviewThreadInfo { - nodeId: string; - isResolved: boolean; -} - -/** - * Review threads keyed by their root comment's database id (the same id the REST - * list reports for the thread's first comment). The node id is needed to resolve - * or reopen a thread; the resolution state mirrors GitHub onto the local review. - * We key off the root comment id we already store, so nothing GitHub-owned needs - * persisting — the node id is looked up live when a thread is resolved. - */ -export async function listReviewThreads( - repoRoot: string, - repo: GitHubRepo, - prNumber: number, -): Promise> { - const byRootCommentId = new Map(); - let cursor: string | null = null; - do { - const args = [ - "api", - "graphql", - "-f", - `query=${REVIEW_THREADS_QUERY}`, - "-F", - `owner=${repo.owner}`, - "-F", - `repo=${repo.repo}`, - "-F", - `number=${prNumber}`, - ]; - if (cursor !== null) args.push("-F", `cursor=${cursor}`); - const stdout = await ghOrThrow(args, repoRoot); - const parsed = ReviewThreadsSchema.safeParse(JSON.parse(stdout)); - if (!parsed.success) throw new Error("Unexpected response shape from GitHub review threads"); - const threads = parsed.data.data.repository?.pullRequest?.reviewThreads; - if (!threads) break; - for (const thread of threads.nodes) { - const rootId = thread.comments.nodes[0]?.databaseId; - if (rootId != null) { - byRootCommentId.set(rootId, { nodeId: thread.id, isResolved: thread.isResolved }); - } - } - cursor = threads.pageInfo.hasNextPage ? threads.pageInfo.endCursor : null; - } while (cursor !== null); - return byRootCommentId; -} - -const RESOLVE_THREAD_MUTATION = `mutation ResolveThread($threadId: ID!) { - resolveReviewThread(input: { threadId: $threadId }) { thread { id } } -}`; - -const UNRESOLVE_THREAD_MUTATION = `mutation UnresolveThread($threadId: ID!) { - unresolveReviewThread(input: { threadId: $threadId }) { thread { id } } -}`; - -/** Resolve or reopen a review thread by its GraphQL node id. Throws on failure. */ -export async function setReviewThreadResolved( - repoRoot: string, - threadNodeId: string, - resolved: boolean, -): Promise { - await ghWrite( - [ - "api", - "graphql", - "-f", - `query=${resolved ? RESOLVE_THREAD_MUTATION : UNRESOLVE_THREAD_MUTATION}`, - "-F", - `threadId=${threadNodeId}`, - ], - repoRoot, - ); -} - -// ─── Writes ───────────────────────────────────────────────────────────────────── - -export interface CreateReviewCommentInput { - commitId: string; - path: string; - body: string; - side: GitHubDiffSide; - /** End line of the comment (single-line comments set only this). */ - line: number; - /** Set with `startSide` for a multi-line comment. */ - startLine?: number; - startSide?: GitHubDiffSide; -} - -const CreatedCommentSchema = z.object({ id: z.number() }); - -/** Create a new review comment on the PR, returning its GitHub id. Throws on failure. */ -export async function createReviewComment( - repoRoot: string, - repo: GitHubRepo, - prNumber: number, - input: CreateReviewCommentInput, -): Promise { - // `-f` sends a string field, `-F` a typed (numeric) one; together they form the - // JSON request body `gh api` POSTs. Each value is a single argv entry, so commit - // bodies with newlines or shell metacharacters pass through untouched. - const args = [ - "api", - `repos/${repo.owner}/${repo.repo}/pulls/${prNumber}/comments`, - "--method", - "POST", - "-f", - `body=${input.body}`, - "-f", - `commit_id=${input.commitId}`, - "-f", - `path=${input.path}`, - "-f", - `side=${input.side}`, - "-F", - `line=${input.line}`, - ]; - if (input.startLine !== undefined && input.startSide !== undefined) { - args.push("-F", `start_line=${input.startLine}`, "-f", `start_side=${input.startSide}`); - } - const stdout = await ghWrite(args, repoRoot); - return CreatedCommentSchema.parse(JSON.parse(stdout)).id; -} - -/** Reply to an existing review comment thread, returning the new comment's GitHub id. */ -export async function replyToReviewComment( - repoRoot: string, - repo: GitHubRepo, - prNumber: number, - inReplyToId: number, - body: string, -): Promise { - const stdout = await ghWrite( - [ - "api", - `repos/${repo.owner}/${repo.repo}/pulls/${prNumber}/comments/${inReplyToId}/replies`, - "--method", - "POST", - "-f", - `body=${body}`, - ], - repoRoot, - ); - return CreatedCommentSchema.parse(JSON.parse(stdout)).id; -} - -/** Read-only `gh` call that surfaces failures (sync is user-initiated, not passive context). */ -async function ghOrThrow(args: string[], repoRoot: string): Promise { - try { - return await gh(args, repoRoot); - } catch (err) { - throw new Error(ghErrorMessage(err)); - } -} - -/** Run a `gh` write command, returning stdout and surfacing failures with gh's stderr message. */ -async function ghWrite(args: string[], repoRoot: string): Promise { - try { - const { stdout } = await execFileAsync("gh", args, { - cwd: repoRoot, - encoding: "utf8", - maxBuffer: 10 * 1024 * 1024, - }); - return stdout; - } catch (err) { - throw new Error(ghErrorMessage(err)); - } -} diff --git a/packages/cli/src/routes/comments.ts b/packages/cli/src/routes/comments.ts index 3c02db9..99179f1 100644 --- a/packages/cli/src/routes/comments.ts +++ b/packages/cli/src/routes/comments.ts @@ -9,19 +9,12 @@ import { asc, eq, inArray } from "drizzle-orm"; import type { StageDb } from "../db/client.js"; import { LOCAL_USER_ID } from "../db/local-user.js"; import { - type ChapterRunRow, type CommentRow, type CommentThreadRow, chapterRun, comment, commentThread, } from "../db/schema/index.js"; -import { - CommentSyncError, - pullComments, - pushComments, - syncThreadResolution, -} from "../runs/comment-sync.js"; import { deriveScopeKey } from "../runs/scope-key.js"; import type { Route } from "../server.js"; import { parseJsonBody, writeJson } from "./json.js"; @@ -111,8 +104,8 @@ export function commentRoutes(db: StageDb): Route[] { }, }, { - // Run-scoped so resolving a PR-originated thread can mirror the toggle to - // GitHub (it needs the run's repo/PR context). Local-only threads stay local. + // Resolve/reopen a local thread. Run-scoped for symmetry with the review + // routes; GitHub threads resolve via the separate review-resolve route. method: "PATCH", pattern: "/api/runs/:runId/comment-threads/:threadId", handler: async (req, res, params) => { @@ -124,28 +117,18 @@ export function commentRoutes(db: StageDb): Route[] { } const body = await parseJsonBody(req, res, ResolveThreadBodySchema); if (!body) return; - const runId = params.runId; - if (!runId) { - writeJson(res, 400, { error: "Missing runId" }); - return; - } - const [run] = db.select().from(chapterRun).where(eq(chapterRun.id, runId)).limit(1).all(); - if (!run) { - writeJson(res, 404, { error: `Run ${runId} not found` }); + + const [updated] = db + .update(commentThread) + .set({ resolvedAt: body.resolved ? new Date() : null }) + .where(eq(commentThread.id, threadId)) + .returning() + .all(); + if (!updated) { + writeJson(res, 404, { error: `Thread ${threadId} not found` }); return; } - try { - const updated = await syncThreadResolution(db, run, threadId, body.resolved); - writeJson(res, 200, toThreadDto(updated, threadComments(db, threadId))); - } catch (err) { - if (err instanceof CommentSyncError) { - writeJson(res, err.status, { error: err.message }); - return; - } - writeJson(res, 500, { - error: err instanceof Error ? err.message : "Failed to update thread", - }); - } + writeJson(res, 200, toThreadDto(updated, threadComments(db, threadId))); }, }, { @@ -224,56 +207,9 @@ export function commentRoutes(db: StageDb): Route[] { writeJson(res, 200, {}); }, }, - { - method: "POST", - pattern: "/api/runs/:runId/comment-sync/pull", - handler: (req, res, params) => { - if (!enforceSameOrigin(req, res)) return; - return runSync(db, params.runId, res, pullComments); - }, - }, - { - method: "POST", - pattern: "/api/runs/:runId/comment-sync/push", - handler: (req, res, params) => { - if (!enforceSameOrigin(req, res)) return; - return runSync(db, params.runId, res, pushComments); - }, - }, ]; } -type Res = Parameters[1]; - -/** Run a pull/push sync for a run, mapping CommentSyncError to its status and unexpected errors to 500. */ -async function runSync( - db: StageDb, - runId: string | undefined, - res: Res, - sync: (db: StageDb, run: ChapterRunRow) => Promise, -): Promise { - if (!runId) { - writeJson(res, 400, { error: "Missing runId" }); - return; - } - const [run] = db.select().from(chapterRun).where(eq(chapterRun.id, runId)).limit(1).all(); - if (!run) { - writeJson(res, 404, { error: `Run ${runId} not found` }); - return; - } - try { - writeJson(res, 200, await sync(db, run)); - } catch (err) { - if (err instanceof CommentSyncError) { - writeJson(res, err.status, { error: err.message }); - return; - } - writeJson(res, 500, { - error: err instanceof Error ? err.message : "Failed to sync comments with GitHub", - }); - } -} - function resolveRunScopeKey(db: StageDb, runId: string | undefined): string | null { if (!runId) return null; const [run] = db @@ -357,15 +293,10 @@ function toThreadDto(thread: CommentThreadRow, comments: CommentRow[]): CommentT } function toCommentDto(row: CommentRow): CommentDto { - // `local` comments render as the local reviewer ("You"); others carry the - // GitHub author pulled from the PR. - const author = - row.authorId === LOCAL_USER_ID ? null : { login: row.authorId, avatarUrl: row.authorAvatarUrl }; return { id: row.id, body: row.body, - author, - githubCommentId: row.githubCommentId, + authorId: row.authorId, createdAt: row.createdAt.toISOString(), updatedAt: row.updatedAt.toISOString(), }; diff --git a/packages/cli/src/runs/comment-sync.ts b/packages/cli/src/runs/comment-sync.ts deleted file mode 100644 index c34f07f..0000000 --- a/packages/cli/src/runs/comment-sync.ts +++ /dev/null @@ -1,368 +0,0 @@ -import type { PullCommentsResult, PushCommentsResult } from "@stagereview/types/comments"; -import { asc, eq } from "drizzle-orm"; -import type { StageDb } from "../db/client.js"; -import { LOCAL_USER_ID } from "../db/local-user.js"; -import { - type ChapterRunRow, - type CommentRow, - type CommentThreadRow, - comment, - commentThread, -} from "../db/schema/index.js"; -import { isWorkingTreeClean, readHeadSha } from "../git.js"; -import { - createReviewComment, - type GitHubRepo, - getPullRequest, - listReviewComments, - listReviewThreads, - parseGitHubRepo, - type ReviewComment, - replyToReviewComment, - setReviewThreadResolved, -} from "../github/index.js"; -import { SCOPE_KIND } from "../schema.js"; -import { groupReviewComments, toGitHubSide } from "./review-comment-mapping.js"; -import { deriveScopeKey } from "./scope-key.js"; - -/** - * A sync failure with a user-facing message and the HTTP status the route should - * return. Guardrail violations (409) and missing-PR errors (404) reach the user - * verbatim, so the UI can explain exactly why a sync didn't run. - */ -export class CommentSyncError extends Error { - constructor( - message: string, - readonly status: number, - ) { - super(message); - this.name = "CommentSyncError"; - } -} - -interface SyncTarget { - repo: GitHubRepo; - prNumber: number; - headSha: string; -} - -/** - * Resolve the GitHub PR a run targets, including its current head SHA. Throws a - * CommentSyncError when the run has no GitHub remote or no detectable PR — both - * are conditions the user needs to see, not silent no-ops. - */ -async function resolveSyncTarget(run: ChapterRunRow): Promise { - const repo = parseGitHubRepo(run.originUrl); - if (!repo) { - throw new CommentSyncError("This run isn't associated with a GitHub remote.", 404); - } - const pullRequest = await getPullRequest(run.repoRoot, run.originUrl, run.prNumber); - if (!pullRequest) { - throw new CommentSyncError( - "No GitHub pull request found for this run. Ensure `gh` is authenticated and the branch has an open PR.", - 404, - ); - } - return { repo, prNumber: pullRequest.number, headSha: pullRequest.head.sha }; -} - -// ─── Pull (GitHub → local) ────────────────────────────────────────────────────── - -/** - * Import the PR's review comments into the run's local review. Idempotent: every - * comment is keyed by its GitHub id, so re-pulling skips comments already present - * and never duplicates. A pull mirrors the PR's resolved state onto the threads it - * owns — resolving or reopening to match GitHub — while purely-local threads (never - * pushed, so absent from GitHub) are left untouched. - */ -export async function pullComments(db: StageDb, run: ChapterRunRow): Promise { - const { repo, prNumber } = await resolveSyncTarget(run); - const scopeKey = deriveScopeKey(run); - const [comments, reviewThreads] = await Promise.all([ - listReviewComments(run.repoRoot, repo, prNumber), - listReviewThreads(run.repoRoot, repo, prNumber), - ]); - const threads = groupReviewComments(comments); - - return db.transaction((tx) => { - let pulled = 0; - let skipped = 0; - - const insertComment = (threadId: string, c: ReviewComment): boolean => { - const existing = tx - .select({ id: comment.id }) - .from(comment) - .where(eq(comment.githubCommentId, c.id)) - .limit(1) - .all(); - if (existing.length > 0) return false; - tx.insert(comment) - .values({ - threadId, - authorId: c.user?.login ?? "ghost", - authorAvatarUrl: c.user?.avatar_url ?? null, - body: c.body, - githubCommentId: c.id, - }) - .run(); - return true; - }; - - for (const thread of threads) { - const resolvedOnGitHub = reviewThreads.get(thread.root.id)?.isResolved ?? false; - // Reuse the local thread that already owns the root comment; otherwise create one. - const [existing] = tx - .select({ id: commentThread.id, resolvedAt: commentThread.resolvedAt }) - .from(comment) - .innerJoin(commentThread, eq(comment.threadId, commentThread.id)) - .where(eq(comment.githubCommentId, thread.root.id)) - .limit(1) - .all(); - - let threadId: string; - if (existing) { - threadId = existing.id; - // "Pull" means mirror the PR: a thread resolved/reopened on GitHub since the - // last pull is reflected locally. Preserve the existing timestamp when already - // resolved so a repeat pull doesn't reset "resolved N ago". - if (resolvedOnGitHub && existing.resolvedAt === null) { - tx.update(commentThread) - .set({ resolvedAt: new Date() }) - .where(eq(commentThread.id, threadId)) - .run(); - } else if (!resolvedOnGitHub && existing.resolvedAt !== null) { - tx.update(commentThread) - .set({ resolvedAt: null }) - .where(eq(commentThread.id, threadId)) - .run(); - } - skipped++; - } else { - const [threadRow] = tx - .insert(commentThread) - .values({ - scopeKey, - filePath: thread.filePath, - side: thread.side, - startLine: thread.startLine, - endLine: thread.endLine, - resolvedAt: resolvedOnGitHub ? new Date() : null, - }) - .returning({ id: commentThread.id }) - .all(); - if (!threadRow) throw new Error("comment_thread insert returned no row"); - threadId = threadRow.id; - insertComment(threadId, thread.root); - pulled++; - } - - for (const reply of thread.replies) { - if (insertComment(threadId, reply)) pulled++; - else skipped++; - } - } - - return { pulled, skipped }; - }); -} - -// ─── Push (local → GitHub) ──────────────────────────────────────────────────────── - -/** - * Block the push unless the local checkout safely matches the PR. PR review - * comments anchor to committed diff positions, so a working-tree scope, a dirty - * tree, or a head that has diverged from the PR would all land comments - * mis-anchored. These are loud failures, not silent skips. - */ -function assertPushable(run: ChapterRunRow, target: SyncTarget): void { - if (run.scopeKind !== SCOPE_KIND.COMMITTED) { - throw new CommentSyncError( - "Only comments on a committed diff can be pushed. Working-tree comments aren't anchored to commits.", - 409, - ); - } - if (!isWorkingTreeClean(run.repoRoot)) { - throw new CommentSyncError( - "Your working tree has uncommitted changes. Commit or stash them so comments anchor to the pushed commit.", - 409, - ); - } - const localHead = readHeadSha(run.repoRoot); - if (localHead !== target.headSha) { - throw new CommentSyncError( - "Your local HEAD doesn't match the PR head. Push or pull your commits so they line up before syncing.", - 409, - ); - } -} - -interface ThreadWithComments { - thread: typeof commentThread.$inferSelect; - comments: CommentRow[]; -} - -function loadThreads(db: StageDb, scopeKey: string): ThreadWithComments[] { - const threads = db - .select() - .from(commentThread) - .where(eq(commentThread.scopeKey, scopeKey)) - .orderBy(asc(commentThread.createdAt)) - .all(); - return threads.map((thread) => ({ - thread, - comments: db - .select() - .from(comment) - .where(eq(comment.threadId, thread.id)) - .orderBy(asc(comment.createdAt)) - .all(), - })); -} - -/** - * Push locally-authored comments to the PR. Comments pulled from GitHub or already - * synced are skipped; each new comment records its GitHub id on success so a later - * push or pull treats it as already-synced. A comment GitHub rejects (e.g. its line - * isn't in the PR diff) is reported as a per-comment failure without aborting the rest. - * - * Known limitation: a deletion-side comment authored on a chapter view anchors to - * that view's synthetic intermediate-file line numbers, which can differ from the - * PR's canonical old-line coordinates. Addition-side anchors are always canonical. - * Canonicalizing deletion-side anchors is deferred; GitHub's own "line not in diff" - * rejection is the backstop here, surfacing such a comment as a loud failure rather - * than letting it land mis-anchored silently. - */ -export async function pushComments(db: StageDb, run: ChapterRunRow): Promise { - const target = await resolveSyncTarget(run); - assertPushable(run, target); - - const result: PushCommentsResult = { pushed: 0, skipped: 0, failed: [] }; - const scopeKey = deriveScopeKey(run); - - const recordSynced = (commentId: string, githubCommentId: number): void => { - db.update(comment).set({ githubCommentId }).where(eq(comment.id, commentId)).run(); - }; - - for (const { thread, comments } of loadThreads(db, scopeKey)) { - const side = toGitHubSide(thread.side); - // GitHub multi-line comments need a start anchor only when the range spans lines. - const startLine = thread.endLine !== thread.startLine ? thread.startLine : undefined; - // Track the root's GitHub id in memory so a reply pushed in the same pass can anchor to it. - const rootGithubId = comments[0]?.githubCommentId ?? null; - let liveRootGithubId = rootGithubId; - - for (let i = 0; i < comments.length; i++) { - const c = comments[i]; - if (!c) continue; - // Only locally-authored comments are ours to push; GitHub-authored ones came from the PR. - if (c.authorId !== LOCAL_USER_ID) { - if (i === 0) liveRootGithubId = c.githubCommentId; - continue; - } - if (c.githubCommentId !== null) { - result.skipped++; - if (i === 0) liveRootGithubId = c.githubCommentId; - continue; - } - - try { - if (i === 0) { - const id = await createReviewComment(run.repoRoot, target.repo, target.prNumber, { - commitId: target.headSha, - path: thread.filePath, - body: c.body, - side, - line: thread.endLine, - startLine, - startSide: startLine !== undefined ? side : undefined, - }); - recordSynced(c.id, id); - liveRootGithubId = id; - result.pushed++; - } else if (liveRootGithubId !== null) { - const id = await replyToReviewComment( - run.repoRoot, - target.repo, - target.prNumber, - liveRootGithubId, - c.body, - ); - recordSynced(c.id, id); - result.pushed++; - } else { - result.failed.push({ - filePath: thread.filePath, - line: thread.endLine, - message: - "The thread's first comment wasn't pushed, so the reply has nothing to anchor to.", - }); - } - } catch (err) { - result.failed.push({ - filePath: thread.filePath, - line: thread.endLine, - message: err instanceof Error ? err.message : "Failed to push comment to GitHub.", - }); - } - } - } - - return result; -} - -// ─── Resolution (bidirectional) ─────────────────────────────────────────────────── - -/** - * Resolve or reopen a thread, keeping GitHub in sync. A thread that originated on - * the PR (its root comment carries a GitHub id) resolves the corresponding GitHub - * review thread first, then mirrors locally only on success — so the local state - * never diverges from the PR (a later pull would otherwise revert it). A local-only - * thread (never pushed, absent from GitHub) just toggles locally. - * - * The GitHub thread's node id isn't stored: it's looked up live from the root - * comment id we already hold, so nothing GitHub-owned is mirrored into our schema. - */ -export async function syncThreadResolution( - db: StageDb, - run: ChapterRunRow, - threadId: string, - resolved: boolean, -): Promise { - const [thread] = db - .select() - .from(commentThread) - .where(eq(commentThread.id, threadId)) - .limit(1) - .all(); - if (!thread) throw new CommentSyncError(`Thread ${threadId} not found`, 404); - - const [root] = db - .select({ githubCommentId: comment.githubCommentId }) - .from(comment) - .where(eq(comment.threadId, threadId)) - .orderBy(asc(comment.createdAt)) - .limit(1) - .all(); - - if (root?.githubCommentId != null) { - const target = await resolveSyncTarget(run); - const reviewThreads = await listReviewThreads(run.repoRoot, target.repo, target.prNumber); - const nodeId = reviewThreads.get(root.githubCommentId)?.nodeId; - if (nodeId === undefined) { - throw new CommentSyncError( - "This thread is no longer on the pull request, so its resolved state can't be synced.", - 404, - ); - } - await setReviewThreadResolved(run.repoRoot, nodeId, resolved); - } - - const [updated] = db - .update(commentThread) - .set({ resolvedAt: resolved ? new Date() : null }) - .where(eq(commentThread.id, threadId)) - .returning() - .all(); - if (!updated) throw new Error("comment_thread resolve update returned no row"); - return updated; -} diff --git a/packages/cli/src/runs/review-comment-mapping.ts b/packages/cli/src/runs/review-comment-mapping.ts deleted file mode 100644 index 880483b..0000000 --- a/packages/cli/src/runs/review-comment-mapping.ts +++ /dev/null @@ -1,55 +0,0 @@ -import { GITHUB_DIFF_SIDE, type GitHubDiffSide, type ReviewComment } from "../github/index.js"; -import { DIFF_SIDE, type DiffSide } from "../schema.js"; - -// LEFT is GitHub's base/deletion side, RIGHT the head/addition side. -export function toGitHubSide(side: DiffSide): GitHubDiffSide { - return side === DIFF_SIDE.DELETIONS ? GITHUB_DIFF_SIDE.LEFT : GITHUB_DIFF_SIDE.RIGHT; -} - -export function fromGitHubSide(side: GitHubDiffSide | null | undefined): DiffSide { - return side === GITHUB_DIFF_SIDE.LEFT ? DIFF_SIDE.DELETIONS : DIFF_SIDE.ADDITIONS; -} - -export interface PulledThread { - root: ReviewComment; - replies: ReviewComment[]; - filePath: string; - side: DiffSide; - startLine: number; - endLine: number; -} - -/** - * Group flat review comments into threads. GitHub sets `in_reply_to_id` on every - * reply, pointing at the thread's root. Comments without an anchorable line - * (outdated, or whole-file) are dropped — the local model is line-anchored. - */ -export function groupReviewComments(comments: ReviewComment[]): PulledThread[] { - const repliesByRoot = new Map(); - const roots: ReviewComment[] = []; - for (const c of comments) { - if (c.in_reply_to_id != null) { - const list = repliesByRoot.get(c.in_reply_to_id); - if (list) list.push(c); - else repliesByRoot.set(c.in_reply_to_id, [c]); - } else { - roots.push(c); - } - } - const threads: PulledThread[] = []; - for (const root of roots) { - if (root.line == null) continue; - const replies = (repliesByRoot.get(root.id) ?? []).sort((a, b) => - a.created_at.localeCompare(b.created_at), - ); - threads.push({ - root, - replies, - filePath: root.path, - side: fromGitHubSide(root.side), - startLine: root.start_line ?? root.line, - endLine: root.line, - }); - } - return threads; -} diff --git a/packages/types/src/comments.ts b/packages/types/src/comments.ts index 5fb054d..899b874 100644 --- a/packages/types/src/comments.ts +++ b/packages/types/src/comments.ts @@ -1,25 +1,13 @@ import { z } from "zod"; import { DIFF_SIDE } from "./chapters.ts"; -// Author of a pulled GitHub comment. Local comments carry `author: null`, which -// the UI renders as the local reviewer ("You"). -export const CommentAuthorSchema = z.object({ - login: z.string(), - avatarUrl: z.string().nullable(), -}); -export type CommentAuthor = z.infer; - // A single authored comment. Replies are sibling comments sharing a thread, so a // comment carries no positional data of its own — the thread owns the anchor. -// `author` is null for locally-authored comments and set for ones pulled from a -// GitHub PR. `githubCommentId` is non-null once the comment is synced to the PR. -// Non-strict (like the other wire response schemas) so the server can add fields -// the SPA doesn't yet read without the response failing to parse. +// These are CLI-local comments; GitHub review comments use the `review` wire model. export const CommentSchema = z.object({ id: z.string(), body: z.string(), - author: CommentAuthorSchema.nullable(), - githubCommentId: z.number().int().nullable(), + authorId: z.string(), createdAt: z.string(), updatedAt: z.string(), }); @@ -69,33 +57,3 @@ export const ResolveThreadBodySchema = z.object({ resolved: z.boolean(), }); export type ResolveThreadBody = z.infer; - -// ─── GitHub sync ────────────────────────────────────────────────────────────── - -// Outcome of importing PR review comments into the local review. -export const PullCommentsResultSchema = z.object({ - // New local comments created from the PR. - pulled: z.number().int().nonnegative(), - // PR comments already present locally (matched by GitHub id), left untouched. - skipped: z.number().int().nonnegative(), -}); -export type PullCommentsResult = z.infer; - -// A local comment that couldn't be pushed, with the reason surfaced to the user. -export const PushCommentFailureSchema = z.object({ - filePath: z.string(), - line: z.number().int(), - message: z.string(), -}); -export type PushCommentFailure = z.infer; - -// Outcome of pushing locally-authored comments to the PR. -export const PushCommentsResultSchema = z.object({ - // Local comments created on the PR by this push. - pushed: z.number().int().nonnegative(), - // Local comments already on the PR (already had a GitHub id), left untouched. - skipped: z.number().int().nonnegative(), - // Per-comment failures (e.g. line not in the PR diff). - failed: z.array(PushCommentFailureSchema), -}); -export type PushCommentsResult = z.infer; diff --git a/packages/web/src/app/runs.$runId.tsx b/packages/web/src/app/runs.$runId.tsx index 0ed666b..4fc243e 100644 --- a/packages/web/src/app/runs.$runId.tsx +++ b/packages/web/src/app/runs.$runId.tsx @@ -1,6 +1,6 @@ import { createFileRoute } from "@tanstack/react-router"; import { Topbar } from "@/components/layout/topbar"; -import { CommentThreadsProvider } from "@/lib/comment-threads-context"; +import { ReviewProvider } from "@/lib/review-context"; import { PullRequestLayout } from "@/routes/pull-request-layout"; export const Route = createFileRoute("/runs/$runId")({ @@ -10,9 +10,9 @@ export const Route = createFileRoute("/runs/$runId")({ function RunLayout() { const { runId } = Route.useParams(); return ( - + - + ); } diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index 091108a..04c4b27 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -19,7 +19,7 @@ import { useState, } from "react"; import { CommentForm } from "@/components/comments/comment-form"; -import { CommentThreadView } from "@/components/comments/comment-thread"; +import { ReviewThreadView } from "@/components/comments/review-thread"; import { buildCommentAnnotations, type CommentDraft, @@ -32,7 +32,6 @@ import { upsertDraft, writeDraftBody, } from "@/lib/comment-drafts"; -import { useCommentThreadsContext } from "@/lib/comment-threads-context"; import { type AnnotatedLineRef, COMMENT_SIDE, @@ -40,9 +39,10 @@ import { type LineRef, SIDE_TO_DIFF, } from "@/lib/diff-types"; +import { useReviewContext } from "@/lib/review-context"; import { resolveSyntaxTheme } from "@/lib/syntax-themes"; -import type { CommentThread } from "@/lib/use-comment-threads"; import { useDiffSettings } from "@/lib/use-diff-settings"; +import type { ReviewThread as CommentThread } from "@/lib/use-review"; import { toSingleSideSelection, useTextSelection } from "@/lib/use-text-selection"; import { LineHighlightOverlay } from "./hunk-highlight-overlay"; import { TextSelectionPopup } from "./text-selection-popup"; @@ -194,8 +194,8 @@ export function PierreDiffViewer({ }, [allLineRefsByFile, filePath]); // ---- Line-anchored comments ---- - const comments = useCommentThreadsContext(); - const { createThread } = comments; + const comments = useReviewContext(); + const { createLocalThread } = comments; const fileThreads = useMemo( () => (filePath ? (comments.threadsByFile.get(filePath) ?? []) : []), [comments.threadsByFile, filePath], @@ -239,7 +239,7 @@ export function PierreDiffViewer({ ); setError(null); try { - await createThread({ + await createLocalThread({ filePath, side: draft.side, startLine: draft.startLine, @@ -252,7 +252,7 @@ export function PierreDiffViewer({ throw err; // keep the composer open with the body intact } }, - [filePath, createThread, closeDraft], + [filePath, createLocalThread, closeDraft], ); const handleThreadMouseEnter = useCallback((thread: CommentThread) => { @@ -287,7 +287,7 @@ export function PierreDiffViewer({ onMouseEnter={() => handleThreadMouseEnter(thread)} onMouseLeave={handleThreadMouseLeave} > - +
))} {draft && ( diff --git a/packages/web/src/components/comments/comment-thread.tsx b/packages/web/src/components/comments/review-thread.tsx similarity index 55% rename from packages/web/src/components/comments/comment-thread.tsx rename to packages/web/src/components/comments/review-thread.tsx index 8151a38..9df7048 100644 --- a/packages/web/src/components/comments/comment-thread.tsx +++ b/packages/web/src/components/comments/review-thread.tsx @@ -1,4 +1,17 @@ -import { ChevronRight, Circle, CircleCheck, MessageSquare, User } from "lucide-react"; +import { + COMMENT_STATE, + type ReviewComment, + type ReviewThread, + THREAD_SOURCE, +} from "@stagereview/types/review"; +import { + ChevronRight, + Circle, + CircleCheck, + GitPullRequestArrow, + MessageSquare, + User, +} from "lucide-react"; import { useState } from "react"; import { AlertDialog, @@ -10,106 +23,159 @@ import { AlertDialogTitle, } from "@/components/ui/alert-dialog"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; +import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible"; import { Markdown } from "@/components/ui/markdown"; import { toast } from "@/components/ui/sonner"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; -import { useCommentThreadsContext } from "@/lib/comment-threads-context"; import { formatTimeAgo } from "@/lib/format"; -import type { Comment, CommentThread } from "@/lib/use-comment-threads"; +import { useReviewContext } from "@/lib/review-context"; +import { GITHUB_REVIEW_STATUS } from "@/lib/use-review"; import { useViewer } from "@/lib/use-viewer"; import { cn } from "@/lib/utils"; import { CommentActions } from "./comment-actions"; import { CommentForm } from "./comment-form"; -type DeleteTarget = - | { kind: "thread"; hasReplies: boolean } - | { kind: "comment"; commentId: string }; - function errorMessage(err: unknown, fallback: string): string { return err instanceof Error ? err.message : fallback; } -export function CommentThreadView({ thread }: { thread: CommentThread }) { - const { replyToThread, setThreadResolved, editComment, deleteThread, deleteComment } = - useCommentThreadsContext(); - const isResolved = thread.resolvedAt !== null; +const PENDING_BADGE_CN = + "border-yellow-500/50 bg-yellow-50 text-yellow-800 dark:bg-yellow-950/20 dark:text-yellow-200"; + +// A pending comment is editable (it's the viewer's own draft); a local comment is +// editable too. Submitted comments live on GitHub and are read-only here. +function canActOn(comment: ReviewComment): boolean { + return comment.state !== COMMENT_STATE.SUBMITTED; +} + +function StateBadge({ state }: { state: ReviewComment["state"] }) { + if (state === COMMENT_STATE.PENDING) { + return ( + + Pending + + ); + } + if (state === COMMENT_STATE.LOCAL) { + return ( + + Local + + ); + } + return null; +} + +export function ReviewThreadView({ thread }: { thread: ReviewThread }) { + const review = useReviewContext(); + const isGitHub = thread.source === THREAD_SOURCE.GITHUB; + const githubAvailable = review.github === GITHUB_REVIEW_STATUS.AVAILABLE; - const [isOpen, setIsOpen] = useState(!isResolved); + const [isOpen, setIsOpen] = useState(!thread.isResolved); const [isReplying, setIsReplying] = useState(false); const [editingId, setEditingId] = useState(null); - const [deleteTarget, setDeleteTarget] = useState(null); + const [deleteTarget, setDeleteTarget] = useState(null); const [error, setError] = useState(null); const root = thread.comments[0]; - // A thread always has a root comment (deleting the last one removes the thread), - // but noUncheckedIndexedAccess types the lookup as possibly-undefined. if (!root) return null; const replies = thread.comments.slice(1); + const idle = !isReplying && editingId === null; + + function setOpenError(message: string | null) { + setError(message); + } async function handleResolveToggle() { - const next = !isResolved; + const next = !thread.isResolved; const wasOpen = isOpen; - // Collapse on resolve / expand on reopen — but never collapse out from under an - // active reply/edit/delete form (it would unmount CommentForm and drop unsaved - // text), mirroring the handleOpenChange guard. const hasActiveForm = isReplying || editingId !== null || deleteTarget !== null; if (!next || !hasActiveForm) setIsOpen(!next); try { - // For PR-originated threads this also resolves/reopens the thread on GitHub; - // on failure the server leaves local state unchanged, so revert the collapse - // and surface the reason. - await setThreadResolved({ threadId: thread.id, resolved: next }); + if (isGitHub && thread.threadNodeId) { + await review.resolveGitHub({ threadNodeId: thread.threadNodeId, resolved: next }); + } else { + await review.resolveLocalThread({ threadId: thread.id, resolved: next }); + } } catch (err) { setIsOpen(wasOpen); - toast.error(errorMessage(err, "Failed to update resolved state")); + toastError(err, "Failed to update resolved state"); } } function handleOpenChange(open: boolean) { - // Keep the thread expanded while the user is mid-action. if (!open && (isReplying || editingId !== null || deleteTarget !== null)) return; setIsOpen(open); } async function submitReply(body: string) { - setError(null); + setOpenError(null); try { - await replyToThread({ threadId: thread.id, body }); + if (isGitHub && thread.threadNodeId) { + await review.replyGitHub({ threadNodeId: thread.threadNodeId, body, pending: true }); + } else { + await review.replyLocal({ threadId: thread.id, body }); + } setIsReplying(false); } catch (err) { - setError(errorMessage(err, "Failed to add reply")); + setOpenError(errorMessage(err, "Failed to add reply")); throw err; } } - async function submitEdit(commentId: string, body: string) { - setError(null); + async function submitEdit(comment: ReviewComment, body: string) { + setOpenError(null); try { - await editComment({ commentId, body }); + if (comment.state === COMMENT_STATE.LOCAL) { + await review.editLocalComment({ commentId: comment.id, body }); + } else if (comment.nodeId) { + await review.editGitHubComment({ nodeId: comment.nodeId, body }); + } setEditingId(null); } catch (err) { - setError(errorMessage(err, "Failed to update comment")); + setOpenError(errorMessage(err, "Failed to update comment")); throw err; } } - function confirmDelete() { - if (!deleteTarget) return; - if (deleteTarget.kind === "thread") void deleteThread(thread.id); - else void deleteComment(deleteTarget.commentId); + async function confirmDelete() { + const comment = deleteTarget; setDeleteTarget(null); + if (!comment) return; + try { + if (comment.state === COMMENT_STATE.LOCAL) { + // Deleting a local root removes the whole thread; a reply removes just itself. + if (comment.id === root?.id) await review.deleteLocalThread(thread.id); + else await review.deleteLocalComment(comment.id); + } else if (comment.nodeId) { + await review.deleteGitHubComment(comment.nodeId); + } + } catch (err) { + toastError(err, "Failed to delete comment"); + } } - const idle = !isReplying && editingId === null; + async function handleAddToReview() { + try { + await review.addToReview(thread.id); + } catch (err) { + toastError(err, "Failed to add to review"); + } + } + + const rootIsDeletableThread = root.state === COMMENT_STATE.LOCAL && replies.length > 0; return (
@@ -124,40 +190,56 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { {isOpen ? "Collapse thread" : "Expand thread"} - - + + + {idle && (
- - - - - Reply - - {/* Pulled GitHub comments are read-only locally; only local comments can be edited/deleted. */} - {root.author === null && ( + {root.state === COMMENT_STATE.LOCAL && githubAvailable && ( + + + + + Add to GitHub review (pending) + + )} + {(!isGitHub || githubAvailable) && ( + + + + + Reply + + )} + {canActOn(root) && ( { setIsOpen(true); - setError(null); + setOpenError(null); setEditingId(root.id); }} - onDelete={() => - setDeleteTarget({ kind: "thread", hasReplies: replies.length > 0 }) - } - deleteLabel={replies.length > 0 ? "Delete thread" : "Delete"} + onDelete={() => setDeleteTarget(root)} + deleteLabel={rootIsDeletableThread ? "Delete thread" : "Delete"} /> )}
@@ -171,10 +253,10 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { initialBody={root.body} placeholder="Edit your comment…" error={error} - onSubmit={(b) => submitEdit(root.id, b)} + onSubmit={(b) => submitEdit(root, b)} onCancel={() => { setEditingId(null); - setError(null); + setOpenError(null); }} /> ) : ( @@ -191,15 +273,15 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { isEditing={editingId === reply.id} error={editingId === reply.id ? error : null} onEdit={() => { - setError(null); + setOpenError(null); setEditingId(reply.id); }} onCancelEdit={() => { setEditingId(null); - setError(null); + setOpenError(null); }} - onSubmitEdit={(b) => submitEdit(reply.id, b)} - onDelete={() => setDeleteTarget({ kind: "comment", commentId: reply.id })} + onSubmitEdit={(b) => submitEdit(reply, b)} + onDelete={() => setDeleteTarget(reply)} /> ))}
@@ -213,7 +295,7 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { onSubmit={submitReply} onCancel={() => { setIsReplying(false); - setError(null); + setOpenError(null); }} /> )} @@ -222,6 +304,7 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { setDeleteTarget(null)} onConfirm={confirmDelete} /> @@ -251,9 +334,9 @@ function ResolveButton({ isResolved, onToggle }: { isResolved: boolean; onToggle ); } -// A local comment (`author: null`) renders as the local reviewer; a comment pulled -// from the PR renders its GitHub author. -function CommentByline({ comment }: { comment: Comment }) { +// Local comments (author null) render as the local reviewer; GitHub comments show +// their author. +function Byline({ comment }: { comment: ReviewComment }) { const viewer = useViewer(); const name = comment.author?.login ?? viewer.name; const avatarUrl = comment.author ? comment.author.avatarUrl : viewer.avatarUrl; @@ -283,7 +366,7 @@ function ReplyItem({ onSubmitEdit, onDelete, }: { - reply: Comment; + reply: ReviewComment; idle: boolean; isEditing: boolean; error: string | null; @@ -295,11 +378,9 @@ function ReplyItem({ return (
- - {/* Only when the whole thread is idle, so opening this reply's editor can't - discard another in-progress edit or reply (matches the root comment). - Pulled GitHub replies are read-only locally. */} - {idle && reply.author === null && } + + + {idle && canActOn(reply) && }
{isEditing ? ( void; onConfirm: () => void; }) { - const isThreadDelete = target?.kind === "thread" && target.hasReplies; return ( - {isThreadDelete ? "Delete thread" : "Delete comment"} + {isThread ? "Delete thread" : "Delete comment"} - {isThreadDelete + {isThread ? "This deletes the whole conversation, including replies. This can't be undone." : "This deletes the comment. This can't be undone."} @@ -353,3 +435,7 @@ function DeleteDialog({ ); } + +function toastError(err: unknown, fallback: string): void { + toast.error(errorMessage(err, fallback)); +} diff --git a/packages/web/src/components/pull-request/comment-sync-menu.tsx b/packages/web/src/components/pull-request/comment-sync-menu.tsx deleted file mode 100644 index adcfe5c..0000000 --- a/packages/web/src/components/pull-request/comment-sync-menu.tsx +++ /dev/null @@ -1,112 +0,0 @@ -import { ArrowDownToLine, ArrowUpFromLine, Github, Loader2 } from "lucide-react"; -import { Button } from "@/components/ui/button"; -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from "@/components/ui/dropdown-menu"; -import { toast } from "@/components/ui/sonner"; -import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; -import { - type PullCommentsResult, - type PushCommentsResult, - useCommentSync, -} from "@/lib/use-comment-sync"; - -function plural(n: number): string { - return n === 1 ? "" : "s"; -} - -function toastPullResult(r: PullCommentsResult): void { - if (r.pulled === 0) { - toast.info( - r.skipped > 0 ? "All PR comments are already imported" : "No review comments on the PR", - ); - return; - } - const extra = r.skipped > 0 ? ` (${r.skipped} already imported)` : ""; - toast.success(`Imported ${r.pulled} comment${plural(r.pulled)} from the PR${extra}`); -} - -function toastPushResult(r: PushCommentsResult): void { - if (r.failed.length > 0) { - const pushedMsg = r.pushed > 0 ? `${r.pushed} pushed, ` : ""; - toast.error(`${pushedMsg}${r.failed.length} failed`, { - description: r.failed.map((f) => `${f.filePath}:${f.line} — ${f.message}`).join("\n"), - }); - return; - } - if (r.pushed === 0) { - toast.info(r.skipped > 0 ? "All comments are already on the PR" : "No local comments to push"); - return; - } - const extra = r.skipped > 0 ? ` (${r.skipped} already on the PR)` : ""; - toast.success(`Pushed ${r.pushed} comment${plural(r.pushed)} to the PR${extra}`); -} - -function errorMessage(err: unknown, fallback: string): string { - return err instanceof Error ? err.message : fallback; -} - -/** - * Pull/push controls for syncing the run's comments with its GitHub PR. Only the - * push path enforces guardrails server-side; both surface their outcome — imported, - * skipped, or failed — as a toast so the user always sees what happened. - */ -export function CommentSyncMenu({ runId }: { runId: string }) { - const { pull, push, isPulling, isPushing } = useCommentSync(runId); - const isBusy = isPulling || isPushing; - - async function handlePull() { - try { - toastPullResult(await pull()); - } catch (err) { - toast.error(errorMessage(err, "Failed to import comments from the PR")); - } - } - - async function handlePush() { - try { - toastPushResult(await push()); - } catch (err) { - toast.error(errorMessage(err, "Failed to push comments to the PR")); - } - } - - return ( - - - - - - - - Sync comments with GitHub - - - - - Pull comments from PR - - - - Push comments to PR - - - - ); -} diff --git a/packages/web/src/components/pull-request/review-panel.tsx b/packages/web/src/components/pull-request/review-panel.tsx new file mode 100644 index 0000000..20f363a --- /dev/null +++ b/packages/web/src/components/pull-request/review-panel.tsx @@ -0,0 +1,211 @@ +import { REVIEW_EVENT, type ReviewEvent } from "@stagereview/types/review"; +import { MessageSquarePlus, Trash2 } from "lucide-react"; +import { useState } from "react"; +import { + AlertDialog, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; +import { toast } from "@/components/ui/sonner"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; +import { useReviewContext } from "@/lib/review-context"; +import { GITHUB_REVIEW_STATUS } from "@/lib/use-review"; +import { cn } from "@/lib/utils"; + +const ACTION_OPTIONS: { event: ReviewEvent; label: string; description: string }[] = [ + { + event: REVIEW_EVENT.COMMENT, + label: "Comment", + description: "Submit general feedback without approval", + }, + { event: REVIEW_EVENT.APPROVE, label: "Approve", description: "Approve this pull request" }, + { + event: REVIEW_EVENT.REQUEST_CHANGES, + label: "Request changes", + description: "Submit feedback that must be addressed", + }, +]; + +function ActionSelector({ + selected, + onSelect, + disabled, +}: { + selected: ReviewEvent; + onSelect: (event: ReviewEvent) => void; + disabled: boolean; +}) { + return ( +
+ {ACTION_OPTIONS.map(({ event, label, description }) => { + const isSelected = selected === event; + return ( + + ); + })} +
+ ); +} + +/** + * The review tray: submit the viewer's pending GitHub review (Comment / Approve / + * Request changes) or discard it. Only shown when the run targets a reachable PR; + * the pending badge counts the viewer's draft comments. + */ +export function ReviewPanel() { + const review = useReviewContext(); + const [open, setOpen] = useState(false); + const [body, setBody] = useState(""); + const [selected, setSelected] = useState(REVIEW_EVENT.COMMENT); + const [isSubmitting, setIsSubmitting] = useState(false); + const [showDiscard, setShowDiscard] = useState(false); + + if (review.github !== GITHUB_REVIEW_STATUS.AVAILABLE) return null; + + const { pendingCommentCount, hasPendingReview } = review; + const hasContent = body.trim().length > 0; + // A bare "Comment" submit with neither body nor pending comments is a no-op. + const canSubmit = + !isSubmitting && (selected !== REVIEW_EVENT.COMMENT || hasContent || pendingCommentCount > 0); + + async function handleSubmit() { + setIsSubmitting(true); + try { + await review.submitReview({ event: selected, body: body.trim() }); + setBody(""); + setOpen(false); + toast.success("Review submitted"); + } catch (err) { + toast.error(err instanceof Error ? err.message : "Failed to submit review"); + } finally { + setIsSubmitting(false); + } + } + + async function handleDiscard() { + setIsSubmitting(true); + try { + await review.discardReview(); + setShowDiscard(false); + setOpen(false); + toast.success("Pending review discarded"); + } catch (err) { + toast.error(err instanceof Error ? err.message : "Failed to discard review"); + } finally { + setIsSubmitting(false); + } + } + + return ( + <> + + + + + + + + Submit your review + + +

Finish your review

+

+ {pendingCommentCount > 0 + ? `${pendingCommentCount} pending comment${pendingCommentCount === 1 ? "" : "s"} will be published.` + : "No pending comments yet — add comments to your review from the diff."} +

+