diff --git a/packages/cli/drizzle/0007_goofy_doctor_faustus.sql b/packages/cli/drizzle/0007_goofy_doctor_faustus.sql new file mode 100644 index 0000000..9a5d26c --- /dev/null +++ b/packages/cli/drizzle/0007_goofy_doctor_faustus.sql @@ -0,0 +1,17 @@ +DROP INDEX `comment_thread_scope_key_idx`;--> statement-breakpoint +ALTER TABLE `comment_thread` ADD `repoRoot` text NOT NULL DEFAULT '';--> statement-breakpoint +UPDATE `comment_thread` +SET `repoRoot` = COALESCE( + ( + SELECT MIN(`chapter_run`.`repoRoot`) + FROM `chapter_run` + WHERE `comment_thread`.`scopeKey` = CASE + WHEN `chapter_run`.`scopeKind` = 'committed' + THEN 'committed:' || `chapter_run`.`baseSha` || ':' || `chapter_run`.`headSha` || ':' || `chapter_run`.`mergeBaseSha` + ELSE 'workingTree:' || `chapter_run`.`workingTreeRef` || ':' || `chapter_run`.`baseSha` || ':' || `chapter_run`.`headSha` || ':' || `chapter_run`.`mergeBaseSha` + END + HAVING COUNT(DISTINCT `chapter_run`.`repoRoot`) = 1 + ), + `repoRoot` +);--> statement-breakpoint +CREATE INDEX `comment_thread_repo_scope_idx` ON `comment_thread` (`repoRoot`,`scopeKey`); diff --git a/packages/cli/drizzle/0008_worried_reavers.sql b/packages/cli/drizzle/0008_worried_reavers.sql new file mode 100644 index 0000000..01e2f2f --- /dev/null +++ b/packages/cli/drizzle/0008_worried_reavers.sql @@ -0,0 +1,3 @@ +ALTER TABLE `comment_thread` ADD `promotionThreadNodeId` text;--> statement-breakpoint +ALTER TABLE `comment_thread` ADD `promotionRootCommentNodeId` text;--> statement-breakpoint +ALTER TABLE `comment_thread` ADD `promotionReplyCount` integer DEFAULT 0 NOT NULL; \ 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..1abd669 --- /dev/null +++ b/packages/cli/drizzle/meta/0007_snapshot.json @@ -0,0 +1,783 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "64897ea4-131e-4f7c-84be-7d519d50137a", + "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'" + }, + "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 + }, + "repoRoot": { + "name": "repoRoot", + "type": "text", + "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_repo_scope_idx": { + "name": "comment_thread_repo_scope_idx", + "columns": [ + "repoRoot", + "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/0008_snapshot.json b/packages/cli/drizzle/meta/0008_snapshot.json new file mode 100644 index 0000000..f6bbdd3 --- /dev/null +++ b/packages/cli/drizzle/meta/0008_snapshot.json @@ -0,0 +1,805 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "eb9d6133-aeba-4522-82f3-741defc02ae0", + "prevId": "64897ea4-131e-4f7c-84be-7d519d50137a", + "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 + }, + "repoRoot": { + "name": "repoRoot", + "type": "text", + "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 + }, + "promotionThreadNodeId": { + "name": "promotionThreadNodeId", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "promotionRootCommentNodeId": { + "name": "promotionRootCommentNodeId", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "promotionReplyCount": { + "name": "promotionReplyCount", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + } + }, + "indexes": { + "comment_thread_repo_scope_idx": { + "name": "comment_thread_repo_scope_idx", + "columns": [ + "repoRoot", + "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..928813e 100644 --- a/packages/cli/drizzle/meta/_journal.json +++ b/packages/cli/drizzle/meta/_journal.json @@ -50,6 +50,20 @@ "when": 1780873869130, "tag": "0006_puzzling_beast", "breakpoints": true + }, + { + "idx": 7, + "version": "6", + "when": 1785205761887, + "tag": "0007_goofy_doctor_faustus", + "breakpoints": true + }, + { + "idx": 8, + "version": "6", + "when": 1785222129020, + "tag": "0008_worried_reavers", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/cli/package.json b/packages/cli/package.json index 903cce8..23ad385 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -52,12 +52,14 @@ "ignore": "^7.0.5", "open": "^11.0.0", "parse-diff": "^0.11.1", + "proper-lockfile": "^4.1.2", "zod": "^4.3.6" }, "devDependencies": { "@stagereview/types": "workspace:*", "@types/better-sqlite3": "^7.6.13", "@types/node": "^25.6.0", + "@types/proper-lockfile": "^4.1.4", "drizzle-kit": "^0.31.10", "tsdown": "^0.21.10", "typescript": "^5.6.3", diff --git a/packages/cli/src/__tests__/comments.routes.test.ts b/packages/cli/src/__tests__/comments.routes.test.ts index da07e67..227e5fe 100644 --- a/packages/cli/src/__tests__/comments.routes.test.ts +++ b/packages/cli/src/__tests__/comments.routes.test.ts @@ -37,7 +37,7 @@ afterEach(async () => { async function startWithRoutes(): Promise { const db = getDb({ dbPath }); - const handle = await startServer({ webDistPath: webDist, routes: commentRoutes(db) }); + const handle = await startServer({ webDistPath: webDist, routes: commentRoutes(db, tmpDir) }); handles.push(handle); return handle; } @@ -121,6 +121,19 @@ describe("comment threads API", () => { expect(res.status).toBe(403); }); + it("rejects a cross-origin read of local comment bodies", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + await createThread(port, runId); + + const res = await send(port, "GET", `/api/runs/${runId}/comment-threads`, undefined, { + Origin: "http://evil.example", + }); + + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "Cross-origin request rejected" }); + }); + it("POST creates a thread with its root comment and the anchor", async () => { const runId = seedRun(); const { port } = await startWithRoutes(); diff --git a/packages/cli/src/__tests__/github-exec.test.ts b/packages/cli/src/__tests__/github-exec.test.ts new file mode 100644 index 0000000..4a956eb --- /dev/null +++ b/packages/cli/src/__tests__/github-exec.test.ts @@ -0,0 +1,40 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { ghReadOrThrow, ghWriteOrThrow } from "../github/exec.js"; + +let tmpDir: string; +let originalPath: string | undefined; + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-gh-timeout-")); + originalPath = process.env.PATH; + process.env.PATH = `${tmpDir}${path.delimiter}${originalPath ?? ""}`; +}); + +afterEach(async () => { + process.env.PATH = originalPath; + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +describe("gh execution", () => { + it("terminates a hung gh process at the configured deadline", async () => { + const shimPath = path.join(tmpDir, "gh"); + await fs.writeFile(shimPath, "#!/bin/sh\nexec sleep 5\n"); + await fs.chmod(shimPath, 0o755); + const startedAt = Date.now(); + + await expect(ghReadOrThrow(["api", "user"], tmpDir, { timeoutMs: 50 })).rejects.toThrow(); + + expect(Date.now() - startedAt).toBeLessThan(1_000); + }); + + it("does not impose the passive-read deadline on mutation execution", async () => { + const shimPath = path.join(tmpDir, "gh"); + await fs.writeFile(shimPath, "#!/bin/sh\nsleep 0.1\nprintf accepted\n"); + await fs.chmod(shimPath, 0o755); + + await expect(ghWriteOrThrow(["api", "graphql"], tmpDir)).resolves.toBe("accepted"); + }); +}); diff --git a/packages/cli/src/__tests__/review-action-queue.test.ts b/packages/cli/src/__tests__/review-action-queue.test.ts new file mode 100644 index 0000000..ea57731 --- /dev/null +++ b/packages/cli/src/__tests__/review-action-queue.test.ts @@ -0,0 +1,132 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { REVIEW_ACTION_SCOPE, ReviewActionQueue } from "../runs/review-action-queue.js"; + +const tempDirs: string[] = []; + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true }))); +}); + +describe("ReviewActionQueue", () => { + it("creates its lock directory only when the first action runs", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-review-lock-")); + tempDirs.push(tempDir); + const lockDirectory = path.join(tempDir, "missing", "locks"); + const queue = new ReviewActionQueue(lockDirectory); + + await expect(fs.access(lockDirectory)).rejects.toThrow(); + + await queue.run( + { kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot: path.join(tempDir, "repo") }, + async () => undefined, + ); + + await expect(fs.access(lockDirectory)).resolves.toBeUndefined(); + }); + + it("serializes independent queue instances for the same checkout", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-review-lock-")); + tempDirs.push(tempDir); + const repoRoot = path.join(tempDir, "repo"); + await fs.mkdir(repoRoot); + const lockDirectory = path.join(tempDir, "locks"); + const events: string[] = []; + let releaseFirst: () => void = () => { + throw new Error("First action gate was not initialized"); + }; + const firstGate = new Promise((resolve) => { + releaseFirst = resolve; + }); + + const first = new ReviewActionQueue(lockDirectory).run( + { kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot }, + async () => { + events.push("first:start"); + await firstGate; + events.push("first:end"); + }, + ); + await expect.poll(() => events).toEqual(["first:start"]); + + const second = new ReviewActionQueue(lockDirectory).run( + { kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot }, + async () => { + events.push("second"); + }, + ); + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(events).toEqual(["first:start"]); + + releaseFirst(); + await Promise.all([first, second]); + expect(events).toEqual(["first:start", "first:end", "second"]); + }); + + it("serializes the same pull request across checkout-specific queue instances", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-review-lock-")); + tempDirs.push(tempDir); + const lockDirectory = path.join(tempDir, "locks"); + const events: string[] = []; + let releaseFirst: () => void = () => { + throw new Error("First action gate was not initialized"); + }; + const firstGate = new Promise((resolve) => { + releaseFirst = resolve; + }); + + const first = new ReviewActionQueue(lockDirectory).run( + { + kind: REVIEW_ACTION_SCOPE.PULL_REQUEST, + owner: "ReviewStage", + repo: "stage-cli", + prNumber: 70, + }, + async () => { + events.push("first:start"); + await firstGate; + events.push("first:end"); + }, + ); + await expect.poll(() => events).toEqual(["first:start"]); + + const second = new ReviewActionQueue(lockDirectory).run( + { + kind: REVIEW_ACTION_SCOPE.PULL_REQUEST, + owner: "reviewstage", + repo: "STAGE-CLI", + prNumber: 70, + }, + async () => { + events.push("second"); + }, + ); + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(events).toEqual(["first:start"]); + + releaseFirst(); + await Promise.all([first, second]); + expect(events).toEqual(["first:start", "first:end", "second"]); + }); + + it("does not require the checkout or its parent directory to be lockfile-writable", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-review-lock-")); + tempDirs.push(tempDir); + const lockDirectory = path.join(tempDir, "locks"); + let ran = false; + + await new ReviewActionQueue(lockDirectory).run( + { + kind: REVIEW_ACTION_SCOPE.CHECKOUT, + repoRoot: path.join(tempDir, "unmounted", "repo"), + }, + async () => { + ran = true; + }, + ); + + expect(ran).toBe(true); + }); +}); diff --git a/packages/cli/src/__tests__/review-boundaries.routes.test.ts b/packages/cli/src/__tests__/review-boundaries.routes.test.ts new file mode 100644 index 0000000..af7a5f5 --- /dev/null +++ b/packages/cli/src/__tests__/review-boundaries.routes.test.ts @@ -0,0 +1,199 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + makeClosedReview, + makeMissingPendingCommitReview, + makeStalePendingReview, + REVIEW_QUERY_RESULT, + ReviewRouteHarness, +} from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + await harness.teardown(); +}); + +describe("review API — GitHub boundaries", () => { + it("keeps a closed PR's comments visible but disables writes", async () => { + await harness.writeGhShim(makeClosedReview()); + const runId = harness.insertRun(); + const port = await harness.start(); + + const read = await harness.request(port, "GET", `/api/runs/${runId}/review`); + const edit = await harness.request(port, "POST", `/api/runs/${runId}/review/comment/edit`, { + nodeId: "COMMENT_pending", + body: "Nope", + }); + const review = JSON.parse(read.body); + + expect(read.status).toBe(200); + expect(review.github).toBe("available"); + expect(review.threads).toHaveLength(2); + expect(review.canPushToReview).toBe(false); + expect(review.canWriteToGitHub).toBe(false); + expect(edit.status).toBe(409); + expect(JSON.parse(edit.body).error).toMatch(/closed/i); + expect(await harness.logLines()).not.toContainEqual(expect.stringMatching(/^edit-comment/)); + }); + + it("does not reuse a pending review from an earlier head commit", async () => { + await harness.writeGhShim(makeStalePendingReview()); + const runId = harness.insertRun(); + const port = await harness.start(); + + const read = await harness.request(port, "GET", `/api/runs/${runId}/review`); + const reply = await harness.request(port, "POST", `/api/runs/${runId}/review/reply`, { + threadNodeId: "THREAD_pending", + body: "Nope", + pending: true, + }); + const immediateReply = await harness.request(port, "POST", `/api/runs/${runId}/review/reply`, { + threadNodeId: "THREAD_sub", + body: "Published now", + pending: false, + }); + const resolve = await harness.request(port, "POST", `/api/runs/${runId}/review/resolve`, { + threadNodeId: "THREAD_sub", + resolved: true, + }); + + expect(JSON.parse(read.body).canPushToReview).toBe(false); + expect(JSON.parse(read.body).canWriteToGitHub).toBe(true); + expect(reply.status).toBe(409); + expect(JSON.parse(reply.body).error).toMatch(/earlier PR version/i); + expect(immediateReply.status).toBe(200); + expect(resolve.status).toBe(200); + expect(await harness.logLines()).toEqual(expect.arrayContaining(["reply", "resolve-thread"])); + }); + + it("keeps stale-run pending review controls available in read-only mode", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun({ headSha: "d".repeat(40) }); + + const read = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + const review = JSON.parse(read.body); + + expect(read.status).toBe(200); + expect(review.github).toBe("available"); + expect(review.threads).toHaveLength(0); + expect(review.pendingCommentCount).toBe(1); + expect(review.pendingComments).toHaveLength(1); + expect(review.hasPendingReview).toBe(true); + expect(review.canPushToReview).toBe(false); + expect(review.canWriteToGitHub).toBe(false); + }); + + it("treats a pending review with no commit as stale instead of offline", async () => { + await harness.writeGhShim(makeMissingPendingCommitReview()); + const runId = harness.insertRun(); + + const read = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + const review = JSON.parse(read.body); + + expect(read.status).toBe(200); + expect(review.github).toBe("available"); + expect(review.hasPendingReview).toBe(true); + expect(review.canPushToReview).toBe(false); + expect(review.canWriteToGitHub).toBe(true); + }); + + it("reports a missing automatically discovered PR without treating GitHub as offline", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT, { noPullRequest: true }); + const runId = harness.insertRun({ prNumber: null }); + const port = await harness.start(); + + const read = await harness.request(port, "GET", `/api/runs/${runId}/review`); + const write = await harness.request(port, "POST", `/api/runs/${runId}/review/comment`, { + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "No PR", + }); + + expect(JSON.parse(read.body).github).toBe("none"); + expect(write.status).toBe(404); + expect(JSON.parse(write.body).error).toMatch(/no GitHub pull request/i); + }); + + it("does not expose another branch's discovered pending review to a stale run", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT, { discoveredPullRequest: true }); + const runId = harness.insertRun({ prNumber: null, headSha: "d".repeat(40) }); + const port = await harness.start(); + + const read = await harness.request(port, "GET", `/api/runs/${runId}/review`); + const discard = await harness.request(port, "POST", `/api/runs/${runId}/review/discard`); + const review = JSON.parse(read.body); + + expect(read.status).toBe(200); + expect(review.github).toBe("none"); + expect(review.hasPendingReview).toBe(false); + expect(review.pendingComments).toHaveLength(0); + expect(discard.status).toBe(409); + expect(JSON.parse(discard.body).error).toMatch(/isn't tied to the pull request/i); + expect(await harness.logLines()).not.toContain("discard-review"); + }); + + it.each([ + ["edit", "/comment/edit", { nodeId: "COMMENT_other", body: "Nope" }], + ["delete", "/comment/delete", { nodeId: "COMMENT_other" }], + ])("rejects a foreign node id for %s", async (_name, suffix, body) => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review${suffix}`, + body, + ); + + expect(res.status).toBe(400); + expect(JSON.parse(res.body).error).toMatch(/isn't an editable pending comment/i); + expect(await harness.logLines()).not.toContainEqual( + expect.stringMatching(/^(edit|delete)-comment/), + ); + }); + + it.each([ + ["reply", "/reply", { threadNodeId: "THREAD_other", body: "Nope", pending: false }], + ["resolve", "/resolve", { threadNodeId: "THREAD_other", resolved: true }], + ])("rejects a foreign thread id for %s", async (_name, suffix, body) => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review${suffix}`, + body, + ); + + expect(res.status).toBe(400); + expect(JSON.parse(res.body).error).toMatch(/doesn't belong/i); + expect(await harness.logLines()).not.toContainEqual( + expect.stringMatching(/^(reply|resolve-thread)/), + ); + }); + + it("rejects editing a submitted comment even when it belongs to the PR", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment/edit`, + { nodeId: "COMMENT_sub", body: "Nope" }, + ); + + expect(res.status).toBe(400); + expect(JSON.parse(res.body).error).toMatch(/pending comment/i); + expect(await harness.logLines()).not.toContainEqual(expect.stringMatching(/^edit-comment/)); + }); +}); diff --git a/packages/cli/src/__tests__/review-concurrency.routes.test.ts b/packages/cli/src/__tests__/review-concurrency.routes.test.ts new file mode 100644 index 0000000..c91a2ad --- /dev/null +++ b/packages/cli/src/__tests__/review-concurrency.routes.test.ts @@ -0,0 +1,146 @@ +import { eq } from "drizzle-orm"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { chapterRun, comment } from "../db/schema/index.js"; +import { + addLocalThreadToReview, + isLocalThreadPromoting, + isLocalThreadPromotionPending, +} from "../runs/review.js"; +import { REVIEW_ACTION_SCOPE, reviewActions } from "../runs/review-action-queue.js"; +import { EMPTY_REVIEW, ReviewRouteHarness } from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + await harness.teardown(); +}); + +describe("review API — concurrency", () => { + it("serializes concurrent first comments onto one pending review", async () => { + await harness.writeGhShim(EMPTY_REVIEW, { + persistCreatedReview: true, + reviewQueryDelayMs: 75, + }); + const runId = harness.insertRun(); + const port = await harness.start(); + const body = { + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "On the PR", + }; + + const responses = await Promise.all([ + harness.request(port, "POST", `/api/runs/${runId}/review/comment`, body), + harness.request(port, "POST", `/api/runs/${runId}/review/comment`, body), + ]); + + expect(responses.map((res) => res.status)).toEqual([200, 200]); + const log = await harness.logLines(); + expect(log.filter((line) => line.startsWith("create-review"))).toHaveLength(1); + expect(log.filter((line) => line.startsWith("add-thread"))).toHaveLength(2); + }); + + it("rejects a local reply while its thread is being promoted", async () => { + await harness.writeGhShim(EMPTY_REVIEW, { addThreadDelayMs: 150 }); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread(); + const port = await harness.start(); + + const promotion = harness.request(port, "POST", `/api/runs/${runId}/review/add`, { + localThreadId, + }); + await expect + .poll(async () => (await harness.logLines()).some((line) => line.startsWith("add-thread")), { + timeout: 5_000, + }) + .toBe(true); + const reply = await harness.request( + port, + "POST", + `/api/comment-threads/${localThreadId}/replies`, + { body: "Too late" }, + ); + + expect(reply.status).toBe(409); + expect((await promotion).status).toBe(200); + }); + + it("freezes a local thread while its promotion is queued for the checkout", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread(); + const [run] = harness.db + .select() + .from(chapterRun) + .where(eq(chapterRun.id, runId)) + .limit(1) + .all(); + if (!run) throw new Error("seeded run was not found"); + const port = await harness.start(); + let releaseCheckout: (() => void) | undefined; + const blocker = reviewActions.run( + { kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot: run.repoRoot }, + () => + new Promise((resolve) => { + releaseCheckout = resolve; + }), + ); + await expect.poll(() => releaseCheckout !== undefined).toBe(true); + + const promotion = addLocalThreadToReview(harness.db, run, localThreadId); + expect(isLocalThreadPromotionPending(harness.db, localThreadId)).toBe(true); + const reply = await harness.request( + port, + "POST", + `/api/comment-threads/${localThreadId}/replies`, + { body: "Must wait" }, + ); + + expect(reply.status).toBe(409); + releaseCheckout?.(); + await blocker; + await promotion; + }); + + it("promotes an edit that wins the checkout lock before promotion", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread(); + const [run] = harness.db + .select() + .from(chapterRun) + .where(eq(chapterRun.id, runId)) + .limit(1) + .all(); + if (!run) throw new Error("seeded run was not found"); + + const edit = reviewActions.run( + { kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot: run.repoRoot }, + async () => { + if (isLocalThreadPromoting(harness.db, localThreadId)) { + throw new Error("Winning edit was incorrectly rejected as mid-promotion"); + } + harness.db + .update(comment) + .set({ body: "Edited before promotion" }) + .where(eq(comment.threadId, localThreadId)) + .run(); + }, + ); + const promotion = addLocalThreadToReview(harness.db, run, localThreadId); + + const [editResult, promotionResult] = await Promise.allSettled([edit, promotion]); + expect(editResult.status).toBe("fulfilled"); + expect(promotionResult.status).toBe("fulfilled"); + expect((await harness.logLines()).find((line) => line.startsWith("add-thread"))).toContain( + "body=Edited before promotion", + ); + }); +}); diff --git a/packages/cli/src/__tests__/review-mutations.routes.test.ts b/packages/cli/src/__tests__/review-mutations.routes.test.ts new file mode 100644 index 0000000..67f33c3 --- /dev/null +++ b/packages/cli/src/__tests__/review-mutations.routes.test.ts @@ -0,0 +1,104 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { commentThread } from "../db/schema/index.js"; +import { EMPTY_REVIEW, REVIEW_QUERY_RESULT, ReviewRouteHarness } from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); + await harness.writeGhShim(REVIEW_QUERY_RESULT); +}); + +afterEach(async () => { + await harness.teardown(); +}); + +describe("review API — GitHub mutations", () => { + it("discards a pending review", async () => { + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/discard`, + ); + + expect(res.status).toBe(200); + expect(await harness.logLines()).toContain("discard-review"); + }); + + it("adds a pending reply", async () => { + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/reply`, + { threadNodeId: "THREAD_pending", body: "Reply", pending: true }, + ); + + expect(res.status).toBe(200); + expect(await harness.logLines()).toContain("reply"); + }); + + it("edits a pending comment", async () => { + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment/edit`, + { nodeId: "COMMENT_pending", body: "Updated" }, + ); + + expect(res.status).toBe(200); + expect((await harness.logLines()).some((line) => line.startsWith("edit-comment"))).toBe(true); + }); + + it("deletes a pending comment", async () => { + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment/delete`, + { nodeId: "COMMENT_pending" }, + ); + + expect(res.status).toBe(200); + expect(await harness.logLines()).toContain("delete-comment"); + }); + + it("resolves a GitHub review thread", async () => { + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/resolve`, + { threadNodeId: "THREAD_pending", resolved: true }, + ); + + expect(res.status).toBe(200); + expect(await harness.logLines()).toContain("resolve-thread"); + }); + + it("restores a legacy claim when discarding a fresh review completes rollback", async () => { + await harness.writeGhShim(EMPTY_REVIEW, { failAddReply: true, failDeleteComment: true }); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ repoRoot: "", withReply: true }); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/add`, + { localThreadId }, + ); + const [thread] = harness.db.select().from(commentThread).all(); + + expect(res.status).toBe(500); + expect(await harness.logLines()).toContain("discard-review"); + expect(thread?.repoRoot).toBe(""); + }); +}); diff --git a/packages/cli/src/__tests__/review-promotion-resume.routes.test.ts b/packages/cli/src/__tests__/review-promotion-resume.routes.test.ts new file mode 100644 index 0000000..e343838 --- /dev/null +++ b/packages/cli/src/__tests__/review-promotion-resume.routes.test.ts @@ -0,0 +1,117 @@ +import { eq } from "drizzle-orm"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { commentThread } from "../db/schema/index.js"; +import { + makeInterruptedPromotionReview, + makeInterruptedPromotionReviewWithForeignMatchingReply, + makeInterruptedPromotionReviewWithSubmittedReply, + makeResolvedInterruptedPromotionReview, + ReviewRouteHarness, +} from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + await harness.teardown(); +}); + +describe("review API — promotion resume", () => { + it("resumes without duplicating the remote root", async () => { + await harness.writeGhShim(makeInterruptedPromotionReview()); + const { runId, localThreadId } = seedInterruptedPromotion(); + const port = await harness.start(); + + const blockedReply = await harness.request( + port, + "POST", + `/api/comment-threads/${localThreadId}/replies`, + { body: "Must wait" }, + ); + const promotion = await harness.request(port, "POST", `/api/runs/${runId}/review/add`, { + localThreadId, + }); + const log = await harness.logLines(); + + expect(blockedReply.status).toBe(409); + expect(promotion.status, promotion.body).toBe(200); + expect(log.filter((line) => line.startsWith("add-thread"))).toHaveLength(0); + expect(log.filter((line) => line === "reply")).toHaveLength(1); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + }); + + it("reconciles a reply that landed immediately before exit", async () => { + await harness.writeGhShim(makeInterruptedPromotionReview("Reply")); + const { runId, localThreadId } = seedInterruptedPromotion(); + + const promotion = await promote(runId, localThreadId); + + expect(promotion.status, promotion.body).toBe(200); + expect((await harness.logLines()).filter((line) => line === "reply")).toHaveLength(0); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + }); + + it("does not mistake another participant matching reply for the local reply", async () => { + await harness.writeGhShim(makeInterruptedPromotionReviewWithForeignMatchingReply()); + const { runId, localThreadId } = seedInterruptedPromotion(); + + const promotion = await promote(runId, localThreadId); + + expect(promotion.status, promotion.body).toBe(200); + expect((await harness.logLines()).filter((line) => line === "reply")).toHaveLength(1); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + }); + + it("reconciles a viewer reply after its review was submitted", async () => { + await harness.writeGhShim(makeInterruptedPromotionReviewWithSubmittedReply()); + const { runId, localThreadId } = seedInterruptedPromotion(); + + const promotion = await promote(runId, localThreadId); + + expect(promotion.status, promotion.body).toBe(200); + expect((await harness.logLines()).filter((line) => line === "reply")).toHaveLength(0); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + }); + + it("does not resolve a recovered remote thread twice", async () => { + await harness.writeGhShim(makeResolvedInterruptedPromotionReview()); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ resolved: true }); + checkpoint(localThreadId); + + const promotion = await promote(runId, localThreadId); + + expect(promotion.status, promotion.body).toBe(200); + expect((await harness.logLines()).filter((line) => line === "resolve-thread")).toHaveLength(0); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + }); +}); + +function seedInterruptedPromotion(): { runId: string; localThreadId: string } { + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ withReply: true }); + checkpoint(localThreadId); + return { runId, localThreadId }; +} + +function checkpoint(localThreadId: string): void { + harness.db + .update(commentThread) + .set({ + promotionThreadNodeId: "THREAD_new", + promotionRootCommentNodeId: "COMMENT_new", + promotionReplyCount: 0, + }) + .where(eq(commentThread.id, localThreadId)) + .run(); +} + +async function promote(runId: string, localThreadId: string) { + return harness.request(await harness.start(), "POST", `/api/runs/${runId}/review/add`, { + localThreadId, + }); +} diff --git a/packages/cli/src/__tests__/review-promotion-rollback.routes.test.ts b/packages/cli/src/__tests__/review-promotion-rollback.routes.test.ts new file mode 100644 index 0000000..0230e80 --- /dev/null +++ b/packages/cli/src/__tests__/review-promotion-rollback.routes.test.ts @@ -0,0 +1,147 @@ +import { eq } from "drizzle-orm"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { comment, commentThread } from "../db/schema/index.js"; +import { + EMPTY_REVIEW, + makePublishedInterruptedPromotionReview, + ReviewRouteHarness, +} from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await harness.teardown(); +}); + +describe("review API — promotion rollback", () => { + it("preserves resolved state when promoting a local thread", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ resolved: true }); + + const res = await promote(runId, localThreadId); + + expect(res.status).toBe(200); + expect((await harness.logLines()).filter((line) => line === "resolve-thread")).toHaveLength(1); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + }); + + it("rolls back a new remote root when preserving resolution fails", async () => { + await harness.writeGhShim(EMPTY_REVIEW, { failResolve: true }); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ resolved: true }); + + const res = await promote(runId, localThreadId); + + expect(res.status).toBe(500); + expect((await harness.logLines()).filter((line) => line === "delete-comment")).toHaveLength(1); + expect(harness.db.select().from(commentThread).all()).toHaveLength(1); + }); + + it("discards a fresh empty review when its comment fails", async () => { + await harness.writeGhShim(EMPTY_REVIEW, { failAddThread: true }); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment`, + { filePath: "src/foo.ts", side: "additions", startLine: 3, endLine: 3, body: "Bad" }, + ); + + expect(res.status).toBe(500); + expect((await harness.logLines()).filter((line) => line === "discard-review")).toHaveLength(1); + }); + + it("reports a failure to discard a fresh review after its action fails", async () => { + const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + await harness.writeGhShim(EMPTY_REVIEW, { + failAddThread: true, + failDiscardReview: true, + }); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment`, + { filePath: "src/foo.ts", side: "additions", startLine: 3, endLine: 3, body: "Bad" }, + ); + + expect(res.status).toBe(500); + expect(stderr).toHaveBeenCalledWith( + expect.stringContaining( + "Failed to discard newly created GitHub review after action failure: gh: discard failed", + ), + ); + }); + + it("keeps the whole local thread when a promoted reply fails", async () => { + await harness.writeGhShim(EMPTY_REVIEW, { failAddReply: true }); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ withReply: true }); + + const res = await promote(runId, localThreadId); + + expect(res.status).toBe(500); + expect((await harness.logLines()).filter((line) => line === "delete-comment")).toHaveLength(1); + expect( + harness.db + .select() + .from(comment) + .all() + .map((row) => row.body), + ).toEqual(["Root", "Reply"]); + expect(harness.db.select().from(commentThread).all()).toHaveLength(1); + }); + + it("preserves insertion order when comments share a timestamp", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ withReply: true }); + harness.db + .update(comment) + .set({ createdAt: new Date(0) }) + .where(eq(comment.threadId, localThreadId)) + .run(); + + const res = await promote(runId, localThreadId); + const addThread = (await harness.logLines()).find((line) => line.startsWith("add-thread")); + + expect(res.status, res.body).toBe(200); + expect(addThread).toContain("body=Root"); + }); + + it("never deletes a published root when a resumed promotion fails", async () => { + await harness.writeGhShim(makePublishedInterruptedPromotionReview(), { failResolve: true }); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ resolved: true }); + harness.db + .update(commentThread) + .set({ + promotionThreadNodeId: "THREAD_new", + promotionRootCommentNodeId: "COMMENT_new", + }) + .run(); + + const promotion = await promote(runId, localThreadId); + const [savedThread] = harness.db.select().from(commentThread).all(); + + expect(promotion.status).toBe(500); + expect((await harness.logLines()).filter((line) => line === "delete-comment")).toHaveLength(0); + expect(savedThread?.promotionThreadNodeId).toBe("THREAD_new"); + expect(savedThread?.promotionRootCommentNodeId).toBe("COMMENT_new"); + }); +}); + +async function promote(runId: string, localThreadId: string) { + return harness.request(await harness.start(), "POST", `/api/runs/${runId}/review/add`, { + localThreadId, + }); +} diff --git a/packages/cli/src/__tests__/review-read.routes.test.ts b/packages/cli/src/__tests__/review-read.routes.test.ts new file mode 100644 index 0000000..018229b --- /dev/null +++ b/packages/cli/src/__tests__/review-read.routes.test.ts @@ -0,0 +1,170 @@ +import { ReviewResponseSchema } from "@stagereview/types/review"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + HEAD, + makePaginatedThreadReview, + REVIEW_QUERY_RESULT, + ReviewRouteHarness, +} from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + vi.restoreAllMocks(); + await harness.teardown(); +}); + +describe("review API — read", () => { + it("rejects a cross-origin request for private draft review content", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "GET", + `/api/runs/${runId}/review`, + undefined, + { Origin: "https://evil.example" }, + ); + + expect(res.status).toBe(403); + }); + + it("returns local-only when the run has no GitHub remote", async () => { + const runId = harness.insertRun({ originUrl: null }); + harness.seedLocalThread(); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + + expect(res.status).toBe(200); + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + expect(review.github).toBe("none"); + expect(review.threads[0]?.source).toBe("local"); + }); + + it("merges local, pending, and submitted GitHub threads", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun(); + harness.seedLocalThread(); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + expect(review.github).toBe("available"); + expect(review.pendingCommentCount).toBe(1); + expect(review.pendingComments).toEqual([ + { id: "COMMENT_pending", filePath: "src/bar.ts", line: 4, body: "Draft comment" }, + ]); + expect(review.threads.map((t) => t.comments[0]?.state).sort()).toEqual([ + "local", + "pending", + "submitted", + ]); + }); + + it("loads later pages of comments within a GitHub thread", async () => { + await harness.writeGhShim(makePaginatedThreadReview()); + const runId = harness.insertRun(); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + const githubThread = review.threads.find((thread) => thread.source === "github"); + + expect(review.pendingCommentCount).toBe(1); + expect(review.pendingComments.map((comment) => comment.id)).toEqual(["COMMENT_late"]); + expect(githubThread?.comments.map((comment) => comment.id)).toEqual([ + "COMMENT_sub", + "COMMENT_late", + ]); + expect(await harness.logLines()).toContain("get-thread-comments"); + }); + + it("reports offline when a follow-up comment page fails", async () => { + const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + await harness.writeGhShim(makePaginatedThreadReview(), { failThreadComments: true }); + const runId = harness.insertRun(); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + + expect(review.github).toBe("offline"); + expect(review.threads).toEqual([]); + expect(stderr).toHaveBeenCalledWith(expect.stringContaining("gh: follow-up page failed")); + }); + + it("keeps local threads visible when GitHub is offline", async () => { + const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + await harness.writeFailingGhShim(); + const runId = harness.insertRun(); + harness.seedLocalThread(); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + expect(review.github).toBe("offline"); + expect(review.threads).toHaveLength(1); + expect(stderr).toHaveBeenCalledWith(expect.stringContaining("gh: authentication required")); + }); + + it("reports offline when automatic pull request discovery fails", async () => { + const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + await harness.writeFailingGhShim(); + const runId = harness.insertRun({ prNumber: null }); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + expect(review.github).toBe("offline"); + expect(stderr).toHaveBeenCalledWith(expect.stringContaining("gh: authentication required")); + }); + + it("reports offline when the PR cannot be resolved", async () => { + const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + await harness.writeGhShim({ + data: { viewer: { login: "octocat" }, repository: { pullRequest: null } }, + }); + const runId = harness.insertRun(); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + expect(review.github).toBe("offline"); + expect(review.canPushToReview).toBe(false); + expect(stderr).toHaveBeenCalledWith( + expect.stringContaining("Pull request not found on GitHub"), + ); + }); + + it("hides GitHub threads when the run does not match the PR head", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun({ headSha: HEAD.replaceAll("a", "d") }); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + expect(review.github).toBe("available"); + expect(review.threads.every((t) => t.source === "local")).toBe(true); + expect(review.hasPendingReview).toBe(true); + expect(review.canPushToReview).toBe(false); + expect(review.canWriteToGitHub).toBe(false); + }); + + it("hides GitHub threads when the run does not match the PR merge base", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT, { mergeBaseOid: "d".repeat(40) }); + const runId = harness.insertRun(); + + const res = await harness.request(await harness.start(), "GET", `/api/runs/${runId}/review`); + + const review = ReviewResponseSchema.parse(JSON.parse(res.body)); + expect(review.github).toBe("available"); + expect(review.threads.every((t) => t.source === "local")).toBe(true); + expect(review.hasPendingReview).toBe(true); + expect(review.canPushToReview).toBe(false); + expect(review.canWriteToGitHub).toBe(false); + }); +}); diff --git a/packages/cli/src/__tests__/review-recovery.routes.test.ts b/packages/cli/src/__tests__/review-recovery.routes.test.ts new file mode 100644 index 0000000..5aa093c --- /dev/null +++ b/packages/cli/src/__tests__/review-recovery.routes.test.ts @@ -0,0 +1,77 @@ +import { ReviewResponseSchema } from "@stagereview/types/review"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + makeAnchorlessPendingReview, + makeSummaryOnlyPendingReview, + ReviewRouteHarness, +} from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + await harness.teardown(); +}); + +describe("review API — pending review recovery", () => { + it("lists and submits an anchorless pending draft", async () => { + await harness.writeGhShim(makeAnchorlessPendingReview()); + const runId = harness.insertRun(); + const port = await harness.start(); + + const read = await harness.request(port, "GET", `/api/runs/${runId}/review`); + const review = ReviewResponseSchema.parse(JSON.parse(read.body)); + const submit = await harness.request(port, "POST", `/api/runs/${runId}/review/submit`, { + event: "COMMENT", + body: "", + }); + + expect(review.pendingComments).toEqual([ + { id: "COMMENT_outdated", filePath: "src/foo.ts", line: null, body: "Outdated draft" }, + ]); + expect(submit.status).toBe(200); + }); + + it("edits and deletes an anchorless pending draft by its returned id", async () => { + await harness.writeGhShim(makeAnchorlessPendingReview()); + const runId = harness.insertRun(); + const port = await harness.start(); + + const edit = await harness.request(port, "POST", `/api/runs/${runId}/review/comment/edit`, { + nodeId: "COMMENT_outdated", + body: "Updated draft", + }); + const remove = await harness.request(port, "POST", `/api/runs/${runId}/review/comment/delete`, { + nodeId: "COMMENT_outdated", + }); + + expect(edit.status).toBe(200); + expect(remove.status).toBe(200); + expect(await harness.logLines()).toEqual( + expect.arrayContaining([expect.stringMatching(/^edit-comment/), "delete-comment"]), + ); + }); + + it("preserves and submits a summary-only pending review", async () => { + await harness.writeGhShim(makeSummaryOnlyPendingReview()); + const runId = harness.insertRun(); + const port = await harness.start(); + + const read = await harness.request(port, "GET", `/api/runs/${runId}/review`); + const review = ReviewResponseSchema.parse(JSON.parse(read.body)); + const submit = await harness.request(port, "POST", `/api/runs/${runId}/review/submit`, { + event: "COMMENT", + body: "Existing draft summary", + }); + + expect(review.pendingReviewBody).toBe("Existing draft summary"); + expect(submit.status).toBe(200); + expect((await harness.logLines()).find((line) => line.startsWith("submit"))).toContain( + "body=Existing draft summary", + ); + }); +}); diff --git a/packages/cli/src/__tests__/review-submit.routes.test.ts b/packages/cli/src/__tests__/review-submit.routes.test.ts new file mode 100644 index 0000000..5b9bb26 --- /dev/null +++ b/packages/cli/src/__tests__/review-submit.routes.test.ts @@ -0,0 +1,104 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + EMPTY_REVIEW, + makeOwnPullRequestReview, + makeSummaryOnlyPendingReview, + REVIEW_QUERY_RESULT, + ReviewRouteHarness, +} from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + await harness.teardown(); +}); + +describe("review API — submission", () => { + it("submits the pending review with the chosen event", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/submit`, + { event: "APPROVE", body: "LGTM" }, + ); + + expect(res.status).toBe(200); + expect((await harness.logLines()).find((line) => line.startsWith("submit"))).toContain( + "event=APPROVE", + ); + }); + + it("rejects an empty comment review", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/submit`, + { event: "COMMENT", body: " " }, + ); + + expect(res.status).toBe(400); + expect((await harness.logLines()).join("\n")).not.toMatch(/create-review|submit/); + }); + + it("rejects a request for changes without a summary", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/submit`, + { event: "REQUEST_CHANGES", body: " " }, + ); + + expect(res.status).toBe(400); + expect(JSON.parse(res.body)).toEqual({ + error: expect.stringMatching(/summary.*request changes/i), + }); + expect((await harness.logLines()).join("\n")).not.toMatch(/submit/); + }); + + it("rejects a review decision on the viewer's own pull request", async () => { + await harness.writeGhShim(makeOwnPullRequestReview()); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/submit`, + { event: "APPROVE", body: "LGTM" }, + ); + + expect(res.status).toBe(400); + expect(JSON.parse(res.body).error).toMatch(/own pull request/i); + expect((await harness.logLines()).join("\n")).not.toMatch(/create-review|submit/); + }); + + it("allows an existing pending summary to be cleared", async () => { + await harness.writeGhShim(makeSummaryOnlyPendingReview()); + const runId = harness.insertRun(); + + const submit = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/submit`, + { event: "APPROVE", body: "" }, + ); + const log = (await harness.logLines()).find((line) => line.startsWith("submit")) ?? ""; + + expect(submit.status).toBe(200); + expect(log).toContain("body="); + expect(log).not.toContain("Existing draft summary"); + }); +}); diff --git a/packages/cli/src/__tests__/review-test-harness.ts b/packages/cli/src/__tests__/review-test-harness.ts new file mode 100644 index 0000000..d65c3eb --- /dev/null +++ b/packages/cli/src/__tests__/review-test-harness.ts @@ -0,0 +1,607 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import { closeDb, getDb, type StageDb } from "../db/client.js"; +import { chapterRun, comment, commentThread } from "../db/schema/index.js"; +import { commentRoutes } from "../routes/comments.js"; +import { reviewRoutes } from "../routes/review.js"; +import { deriveScopeKey } from "../runs/scope-key.js"; +import { SCOPE_KIND, WORKING_TREE_REF } from "../schema.js"; +import { LOOPBACK_HOST, type ServerHandle, startServer } from "../server.js"; + +export const BASE = "b".repeat(40); +export const HEAD = "a".repeat(40); +export const MERGE_BASE = "c".repeat(40); +export const SCOPE_KEY = deriveScopeKey({ + scopeKind: SCOPE_KIND.COMMITTED, + workingTreeRef: null, + baseSha: BASE, + headSha: HEAD, + mergeBaseSha: MERGE_BASE, +}); +export const GITHUB_ORIGIN = "git@github.com:owner/repo.git"; + +const submittedThread = { + id: "THREAD_sub", + isResolved: false, + path: "src/foo.ts", + line: 10, + startLine: null, + diffSide: "RIGHT", + startDiffSide: null, + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + id: "COMMENT_sub", + url: "https://github.com/owner/repo/pull/5#discussion_r1", + body: "Submitted comment", + bodyHTML: "

Submitted comment

", + createdAt: "2026-01-01T00:00:00Z", + author: { login: "octocat", avatarUrl: "https://x/o.png" }, + pullRequestReview: { state: "COMMENTED" }, + }, + ], + }, +}; + +const pendingThread = { + id: "THREAD_pending", + isResolved: false, + path: "src/bar.ts", + line: 4, + startLine: null, + diffSide: "LEFT", + startDiffSide: null, + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + id: "COMMENT_pending", + url: "https://github.com/owner/repo/pull/5#discussion_r2", + body: "Draft comment", + bodyHTML: "

Draft comment

", + createdAt: "2026-01-02T00:00:00Z", + author: { login: "octocat", avatarUrl: "https://x/o.png" }, + pullRequestReview: { state: "PENDING" }, + }, + ], + }, +}; + +function makeReview( + threads: unknown[], + pendingReviewId: string | null, + pendingReviewBody = "", + options: { + state?: "OPEN" | "CLOSED" | "MERGED"; + pendingReviewCommitOid?: string | null; + viewerDidAuthor?: boolean; + } = {}, +): unknown { + return { + data: { + viewer: { login: "octocat" }, + repository: { + pullRequest: { + id: "PR_node", + state: options.state ?? "OPEN", + viewerDidAuthor: options.viewerDidAuthor ?? false, + headRefOid: HEAD, + baseRefOid: BASE, + reviews: { + nodes: + pendingReviewId === null + ? [] + : [ + { + id: pendingReviewId, + body: pendingReviewBody, + commit: + options.pendingReviewCommitOid === null + ? null + : { oid: options.pendingReviewCommitOid ?? HEAD }, + }, + ], + }, + reviewThreads: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: threads, + }, + }, + }, + }, + }; +} + +export const REVIEW_QUERY_RESULT = makeReview([submittedThread, pendingThread], "REVIEW_pending"); +export const EMPTY_REVIEW = makeReview([], null); + +export function makeSummaryOnlyPendingReview(): unknown { + return makeReview([], "REVIEW_pending", "Existing draft summary"); +} + +export function makeClosedReview(): unknown { + return makeReview([submittedThread, pendingThread], "REVIEW_pending", "", { + state: "CLOSED", + }); +} + +export function makeStalePendingReview(): unknown { + return makeReview([submittedThread, pendingThread], "REVIEW_pending", "", { + pendingReviewCommitOid: "d".repeat(40), + }); +} + +export function makeMissingPendingCommitReview(): unknown { + return makeReview([submittedThread, pendingThread], "REVIEW_pending", "", { + pendingReviewCommitOid: null, + }); +} + +export function makeOwnPullRequestReview(): unknown { + return makeReview([pendingThread], "REVIEW_pending", "", { + viewerDidAuthor: true, + }); +} + +export function makeInterruptedPromotionReview(promotedReplyBody?: string): unknown { + const root = { + ...pendingThread.comments.nodes[0], + id: "COMMENT_new", + body: "Root", + bodyHTML: "

Root

", + }; + return makeReview( + [ + { + ...pendingThread, + id: "THREAD_new", + path: "src/foo.ts", + line: 3, + diffSide: "RIGHT", + comments: { + ...pendingThread.comments, + nodes: + promotedReplyBody === undefined + ? [root] + : [ + root, + { + ...root, + id: "COMMENT_reply", + body: promotedReplyBody, + bodyHTML: `

${promotedReplyBody}

`, + }, + ], + }, + }, + ], + "REVIEW_pending", + ); +} + +export function makePublishedInterruptedPromotionReview(): unknown { + const root = { + ...pendingThread.comments.nodes[0], + id: "COMMENT_new", + body: "Root", + bodyHTML: "

Root

", + pullRequestReview: { state: "COMMENTED" }, + }; + return makeReview( + [ + { + ...pendingThread, + id: "THREAD_new", + path: "src/foo.ts", + line: 3, + diffSide: "RIGHT", + comments: { ...pendingThread.comments, nodes: [root] }, + }, + ], + null, + ); +} + +export function makeInterruptedPromotionReviewWithForeignMatchingReply(): unknown { + const root = { + ...pendingThread.comments.nodes[0], + id: "COMMENT_new", + body: "Root", + bodyHTML: "

Root

", + }; + const foreignReply = { + ...submittedThread.comments.nodes[0], + id: "COMMENT_foreign", + body: "Reply", + bodyHTML: "

Reply

", + author: { login: "collaborator", avatarUrl: "https://x/c.png" }, + }; + return makeReview( + [ + { + ...pendingThread, + id: "THREAD_new", + path: "src/foo.ts", + line: 3, + diffSide: "RIGHT", + comments: { ...pendingThread.comments, nodes: [root, foreignReply] }, + }, + ], + "REVIEW_pending", + ); +} + +export function makeInterruptedPromotionReviewWithSubmittedReply(): unknown { + const root = { + ...submittedThread.comments.nodes[0], + id: "COMMENT_new", + body: "Root", + bodyHTML: "

Root

", + }; + const reply = { + ...submittedThread.comments.nodes[0], + id: "COMMENT_reply", + body: "Reply", + bodyHTML: "

Reply

", + }; + return makeReview( + [ + { + ...submittedThread, + id: "THREAD_new", + path: "src/foo.ts", + line: 3, + diffSide: "RIGHT", + comments: { ...submittedThread.comments, nodes: [root, reply] }, + }, + ], + null, + ); +} + +export function makeResolvedInterruptedPromotionReview(): unknown { + const root = { + ...pendingThread.comments.nodes[0], + id: "COMMENT_new", + body: "Local note", + bodyHTML: "

Local note

", + }; + return makeReview( + [ + { + ...pendingThread, + id: "THREAD_new", + isResolved: true, + path: "src/foo.ts", + line: 3, + diffSide: "RIGHT", + comments: { ...pendingThread.comments, nodes: [root] }, + }, + ], + "REVIEW_pending", + ); +} + +export function makeAnchorlessPendingReview(): unknown { + return makeReview( + [ + { + id: "THREAD_outdated", + isResolved: false, + path: "src/foo.ts", + line: null, + startLine: null, + diffSide: "RIGHT", + startDiffSide: null, + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + id: "COMMENT_outdated", + url: "https://github.com/owner/repo/pull/5#d9", + body: "Outdated draft", + bodyHTML: "

Outdated draft

", + createdAt: "2026-01-03T00:00:00Z", + author: { login: "octocat", avatarUrl: "https://x/o.png" }, + pullRequestReview: { state: "PENDING" }, + }, + ], + }, + }, + ], + "REVIEW_pending", + ); +} + +export function makePaginatedThreadReview(): unknown { + return makeReview( + [ + { + ...submittedThread, + comments: { + pageInfo: { hasNextPage: true, endCursor: "COMMENTS_cursor" }, + nodes: submittedThread.comments.nodes, + }, + }, + ], + "REVIEW_pending", + ); +} + +interface GhShimOptions { + addThreadDelayMs?: number; + discoveredPullRequest?: boolean; + failAddThread?: boolean; + failAddReply?: boolean; + failDeleteComment?: boolean; + failDiscardReview?: boolean; + failResolve?: boolean; + failThreadComments?: boolean; + mergeBaseOid?: string; + noPullRequest?: boolean; + persistCreatedReview?: boolean; + reviewQueryDelayMs?: number; +} + +interface InsertRunOptions { + originUrl?: string | null; + committed?: boolean; + headSha?: string; + prNumber?: number | null; + repoRoot?: string; +} + +interface SeedThreadOptions { + withReply?: boolean; + resolved?: boolean; + repoRoot?: string; +} + +export class ReviewRouteHarness { + private tmpDir = ""; + private dbPath = ""; + private webDist = ""; + private repoRoot = ""; + private binDir = ""; + private prNumber = 0; + private originalPath: string | undefined; + private readonly handles: ServerHandle[] = []; + + get db(): StageDb { + return getDb({ dbPath: this.dbPath }); + } + + async setup(): Promise { + this.tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-review-")); + this.dbPath = path.join(this.tmpDir, "db.sqlite"); + this.webDist = path.join(this.tmpDir, "web-dist"); + this.repoRoot = path.join(this.tmpDir, "repo"); + this.binDir = path.join(this.tmpDir, "bin"); + this.prNumber = Number.parseInt( + createHash("sha256").update(this.tmpDir).digest("hex").slice(0, 7), + 16, + ); + await fs.mkdir(this.webDist); + await fs.writeFile(path.join(this.webDist, "index.html"), ""); + await fs.mkdir(this.repoRoot); + await fs.mkdir(this.binDir); + this.originalPath = process.env.PATH; + process.env.PATH = `${this.binDir}${path.delimiter}${this.originalPath ?? ""}`; + closeDb(); + } + + async teardown(): Promise { + while (this.handles.length > 0) { + const handle = this.handles.pop(); + if (handle) await handle.close(); + } + closeDb(); + process.env.PATH = this.originalPath; + await fs.rm(this.tmpDir, { recursive: true, force: true }); + } + + async writeGhShim(reviewResult: unknown, options: GhShimOptions = {}): Promise { + const reviewPath = path.join(this.tmpDir, "review.json"); + await fs.writeFile(reviewPath, 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=")) || ""); +const fields = args.filter((a) => !a.startsWith("query=") && a !== "-f" && a !== "-F" && a !== "api" && a !== "graphql").join(" "); +const log = ${JSON.stringify(path.join(this.tmpDir, "gh-log.txt"))}; +const reviewPath = ${JSON.stringify(reviewPath)}; +function emit(o) { process.stdout.write(JSON.stringify(o)); } +function sleep(ms) { if (ms > 0) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); } +if (args.some((arg) => arg.includes("/compare/"))) { + emit({ merge_base_commit: { sha: ${JSON.stringify(options.mergeBaseOid ?? MERGE_BASE)} } }); +} else if (args[0] === "pr" && args[1] === "view") { + if (${options.noPullRequest ? "true" : "false"}) { + process.stderr.write("no pull requests found for branch \\"feature\\"\\n"); + process.exit(1); + } + emit(${options.discoveredPullRequest ? `{ number: 5, title: "Other branch", body: "", url: "https://github.com/owner/repo/pull/5", state: "OPEN", isDraft: false, mergedAt: null, createdAt: "2026-01-01T00:00:00Z", author: { login: "octocat" }, headRefName: "other", headRefOid: ${JSON.stringify(HEAD)}, baseRefName: "main" }` : "{}"}); +} else if (query.includes("query GetReviewThreadComments")) { + fs.appendFileSync(log, "get-thread-comments\\n"); + if (${options.failThreadComments ? "true" : "false"}) { process.stderr.write("gh: follow-up page failed\\n"); process.exit(1); } + emit({ data: { node: { comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [{ + id: "COMMENT_late", + url: "https://github.com/owner/repo/pull/5#discussion_r101", + body: "Late draft reply", + bodyHTML: "

Late draft reply

", + createdAt: "2026-01-04T00:00:00Z", + author: { login: "octocat", avatarUrl: "https://x/o.png" }, + pullRequestReview: { state: "PENDING" } + }] + } } } }); +} else if (query.includes("query GetReview")) { + sleep(${options.reviewQueryDelayMs ?? 0}); + const commentFields = query.slice(query.indexOf("comments(first: 100)"), query.indexOf("author {")); + const illegalCommentField = ["databaseId", "diffSide", "startDiffSide"].find((field) => commentFields.includes(field)); + if (illegalCommentField) { + process.stderr.write("gh: PullRequestReviewComment has no field " + illegalCommentField + "\\n"); + process.exit(1); + } + emit(JSON.parse(fs.readFileSync(reviewPath, "utf8"))); +} else if (query.includes("mutation CreatePendingReview")) { + fs.appendFileSync(log, "create-review " + fields + "\\n"); + if (${options.persistCreatedReview ? "true" : "false"}) { + const review = JSON.parse(fs.readFileSync(reviewPath, "utf8")); + review.data.repository.pullRequest.reviews.nodes = [{ + id: "REVIEW_new", + body: "", + commit: { oid: review.data.repository.pullRequest.headRefOid } + }]; + fs.writeFileSync(reviewPath, JSON.stringify(review)); + } + emit({ data: { addPullRequestReview: { pullRequestReview: { id: "REVIEW_new" } } } }); +} else if (query.includes("mutation AddReviewThread")) { + fs.appendFileSync(log, "add-thread " + fields + "\\n"); + sleep(${options.addThreadDelayMs ?? 0}); + if (${options.failAddThread ? "true" : "false"}) { process.stderr.write("gh: line not in diff\\n"); process.exit(1); } + emit({ data: { addPullRequestReviewThread: { thread: { id: "THREAD_new", comments: { nodes: [{ id: "COMMENT_new" }] } } } } }); +} else if (query.includes("mutation ResolveThread")) { + fs.appendFileSync(log, "resolve-thread\\n"); + if (${options.failResolve ? "true" : "false"}) { process.stderr.write("gh: resolve failed\\n"); process.exit(1); } + emit({ data: { resolveReviewThread: { thread: { id: "THREAD_new" } } } }); +} else if (query.includes("mutation DeleteReviewComment")) { + fs.appendFileSync(log, "delete-comment\\n"); + if (${options.failDeleteComment ? "true" : "false"}) { process.stderr.write("gh: delete failed\\n"); process.exit(1); } + emit({ data: { deletePullRequestReviewComment: { pullRequestReviewComment: { id: "COMMENT_new" } } } }); +} else if (query.includes("mutation UpdateReviewComment")) { + fs.appendFileSync(log, "edit-comment " + fields + "\\n"); + emit({ data: { updatePullRequestReviewComment: { pullRequestReviewComment: { id: "COMMENT_new" } } } }); +} else if (query.includes("mutation DiscardReview")) { + fs.appendFileSync(log, "discard-review\\n"); + if (${options.failDiscardReview ? "true" : "false"}) { process.stderr.write("gh: discard failed\\n"); process.exit(1); } + emit({ data: { deletePullRequestReview: { pullRequestReview: { id: "REVIEW_new" } } } }); +} else if (query.includes("mutation AddReviewReply")) { + fs.appendFileSync(log, "reply\\n"); + if (${options.failAddReply ? "true" : "false"}) { process.stderr.write("gh: reply failed\\n"); process.exit(1); } + 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(this.binDir, "gh"), shim); + await fs.chmod(path.join(this.binDir, "gh"), 0o755); + } + + async writeFailingGhShim(): Promise { + await fs.writeFile( + path.join(this.binDir, "gh"), + "#!/bin/sh\nprintf 'gh: authentication required\\n' >&2\nexit 1\n", + ); + await fs.chmod(path.join(this.binDir, "gh"), 0o755); + } + + insertRun(options: InsertRunOptions = {}): string { + const originUrl = options.originUrl === undefined ? GITHUB_ORIGIN : options.originUrl; + const committed = options.committed !== false; + const [row] = this.db + .insert(chapterRun) + .values({ + repoRoot: options.repoRoot ?? this.repoRoot, + originUrl, + prNumber: options.prNumber === undefined ? this.prNumber : options.prNumber, + scopeKind: committed ? SCOPE_KIND.COMMITTED : SCOPE_KIND.WORKING_TREE, + workingTreeRef: committed ? null : WORKING_TREE_REF.WORK, + baseSha: BASE, + headSha: options.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; + } + + seedLocalThread(options: SeedThreadOptions = {}): string { + const [thread] = this.db + .insert(commentThread) + .values({ + repoRoot: options.repoRoot ?? this.repoRoot, + scopeKey: SCOPE_KEY, + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + resolvedAt: options.resolved ? new Date() : null, + }) + .returning({ id: commentThread.id }) + .all(); + if (!thread) throw new Error("seed: thread insert returned no row"); + this.db + .insert(comment) + .values({ threadId: thread.id, body: options.withReply ? "Root" : "Local note" }) + .run(); + if (options.withReply) { + this.db.insert(comment).values({ threadId: thread.id, body: "Reply" }).run(); + } + return thread.id; + } + + async start(): Promise { + const handle = await startServer({ + webDistPath: this.webDist, + routes: [...commentRoutes(this.db, this.repoRoot), ...reviewRoutes(this.db)], + }); + this.handles.push(handle); + return handle.port; + } + + request( + port: number, + method: string, + requestPath: string, + body?: unknown, + headers: Record = {}, + ): 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: requestPath, + agent: false, + headers: + payload === undefined + ? headers + : { + ...headers, + "Content-Type": "application/json", + "Content-Length": Buffer.byteLength(payload), + }, + }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (chunk: Buffer) => chunks.push(chunk)); + res.on("end", () => + resolve({ + status: res.statusCode ?? 0, + body: Buffer.concat(chunks).toString("utf8"), + }), + ); + }, + ); + req.on("error", reject); + req.end(payload); + }); + } + + async logLines(): Promise { + const text = await fs.readFile(path.join(this.tmpDir, "gh-log.txt"), "utf8").catch(() => ""); + return text.split("\n"); + } +} diff --git a/packages/cli/src/__tests__/review-write.routes.test.ts b/packages/cli/src/__tests__/review-write.routes.test.ts new file mode 100644 index 0000000..7cc6e25 --- /dev/null +++ b/packages/cli/src/__tests__/review-write.routes.test.ts @@ -0,0 +1,165 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { commentThread } from "../db/schema/index.js"; +import { + EMPTY_REVIEW, + HEAD, + REVIEW_QUERY_RESULT, + ReviewRouteHarness, +} from "./review-test-harness.js"; + +let harness: ReviewRouteHarness; + +beforeEach(async () => { + harness = new ReviewRouteHarness(); + await harness.setup(); +}); + +afterEach(async () => { + await harness.teardown(); +}); + +describe("review API — writes", () => { + it("promotes a local thread to a pending GitHub comment", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/add`, + { + localThreadId, + }, + ); + + expect(res.status, res.body).toBe(200); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + const createReviewLogs = (await harness.logLines()).filter((line) => + line.startsWith("create-review"), + ); + expect(createReviewLogs).toHaveLength(1); + expect(createReviewLogs[0]).toContain(`commitOID=${HEAD}`); + }); + + it("creates a pending PR comment without storing it locally", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment`, + { + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "On the PR", + }, + ); + + expect(res.status).toBe(200); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + expect((await harness.logLines()).some((line) => line.startsWith("add-thread"))).toBe(true); + }); + + it("rejects PR comments from a working-tree run", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT); + const runId = harness.insertRun({ committed: false }); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment`, + { + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "On the PR", + }, + ); + + expect(res.status).toBe(409); + expect(JSON.parse(res.body)).toEqual({ + error: expect.stringMatching(/committed diff/i), + }); + }); + + it("rejects PR comments when the run has a different merge base", async () => { + await harness.writeGhShim(REVIEW_QUERY_RESULT, { mergeBaseOid: "d".repeat(40) }); + const runId = harness.insertRun(); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/comment`, + { + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "On the PR", + }, + ); + + expect(res.status).toBe(409); + expect(JSON.parse(res.body)).toEqual({ + error: expect.stringMatching(/current PR diff/i), + }); + }); + + it("rejects a local thread from another repository with the same diff", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ repoRoot: "/other/repository" }); + + const res = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/add`, + { localThreadId }, + ); + + expect(res.status).toBe(400); + expect(JSON.parse(res.body)).toEqual({ + error: expect.stringMatching(/repository/i), + }); + }); + + it("keeps an ambiguous legacy thread visible and claimable", async () => { + await harness.writeGhShim(EMPTY_REVIEW); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ repoRoot: "" }); + const port = await harness.start(); + + const read = await harness.request(port, "GET", `/api/runs/${runId}/review`); + const promote = await harness.request(port, "POST", `/api/runs/${runId}/review/add`, { + localThreadId, + }); + + expect(JSON.parse(read.body).threads).toEqual([ + expect.objectContaining({ id: localThreadId, source: "local" }), + ]); + expect(promote.status).toBe(200); + expect(harness.db.select().from(commentThread).all()).toHaveLength(0); + }); + + it("restores an ambiguous legacy claim when the remote write fails", async () => { + await harness.writeGhShim(EMPTY_REVIEW, { failAddThread: true }); + const runId = harness.insertRun(); + const localThreadId = harness.seedLocalThread({ repoRoot: "" }); + + const promote = await harness.request( + await harness.start(), + "POST", + `/api/runs/${runId}/review/add`, + { localThreadId }, + ); + const [thread] = harness.db.select().from(commentThread).all(); + + expect(promote.status).toBe(500); + expect(thread?.repoRoot).toBe(""); + }); +}); diff --git a/packages/cli/src/__tests__/server.test.ts b/packages/cli/src/__tests__/server.test.ts index 0246fff..f5f7925 100644 --- a/packages/cli/src/__tests__/server.test.ts +++ b/packages/cli/src/__tests__/server.test.ts @@ -190,11 +190,21 @@ describe("startServer", () => { }); it("close() stops the server from accepting new connections", async () => { - const handle = await start(); + const marker = "owned-by-server-close-test"; + const handle = await start([ + { + method: "GET", + pattern: "/api/close-probe", + handler: (_req, res) => { + res.end(marker); + }, + }, + ]); const { port } = handle; // Take ownership: don't auto-close, we're closing manually. handles.pop(); await handle.close(); - await expect(rawRequest(port, "/")).rejects.toThrow(); + const response = await rawRequest(port, "/api/close-probe").catch(() => null); + expect(response?.body).not.toBe(marker); }); }); diff --git a/packages/cli/src/db/path.ts b/packages/cli/src/db/path.ts index abbcc0b..f0a865f 100644 --- a/packages/cli/src/db/path.ts +++ b/packages/cli/src/db/path.ts @@ -13,9 +13,14 @@ export function getDbPath(): string { return path.join(dir, DB_FILE); } +/** Writable per-user directory shared by every Stage checkout. */ +export function getStageDataDir(): string { + return path.join(homedir(), STAGE_HOME); +} + function ensureRepoDir(repoRoot: string): string { const hash = createHash("sha256").update(repoRoot.trim()).digest("hex").slice(0, REPO_HASH_LEN); - const dir = path.join(homedir(), STAGE_HOME, hash); + const dir = path.join(getStageDataDir(), hash); mkdirSync(dir, { recursive: true }); return dir; } diff --git a/packages/cli/src/db/schema/comment-thread.ts b/packages/cli/src/db/schema/comment-thread.ts index bce7d27..b0cd25b 100644 --- a/packages/cli/src/db/schema/comment-thread.ts +++ b/packages/cli/src/db/schema/comment-thread.ts @@ -6,6 +6,9 @@ export const commentThread = sqliteTable( "comment_thread", { ...baseColumns(), + // Repository checkout that owns this thread. Scope SHAs alone are not unique + // across upstream repositories and forks that share commit history. + repoRoot: text().notNull(), // Anchors the thread to a diff scope rather than a single run, so comments // survive re-imports of the same diff (mirrors how external_id keys view-state). scopeKey: text().notNull(), @@ -15,8 +18,14 @@ export const commentThread = sqliteTable( endLine: integer().notNull(), /** Null while open; set to the resolution time once resolved. */ resolvedAt: integer({ mode: "timestamp_ms" }), + /** GitHub thread created by an interrupted local-to-remote promotion. */ + promotionThreadNodeId: text(), + /** Root comment used to roll back an interrupted promotion safely. */ + promotionRootCommentNodeId: text(), + /** Number of local replies already copied to the promotion thread. */ + promotionReplyCount: integer().notNull().default(0), }, - (table) => [index("comment_thread_scope_key_idx").on(table.scopeKey)], + (table) => [index("comment_thread_repo_scope_idx").on(table.repoRoot, table.scopeKey)], ); export type CommentThreadRow = typeof commentThread.$inferSelect; diff --git a/packages/cli/src/db/schema/comment.ts b/packages/cli/src/db/schema/comment.ts index 1a9783c..86fa9c1 100644 --- a/packages/cli/src/db/schema/comment.ts +++ b/packages/cli/src/db/schema/comment.ts @@ -1,8 +1,11 @@ +import { sql } from "drizzle-orm"; 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", { @@ -18,3 +21,6 @@ export const comment = sqliteTable( export type CommentRow = typeof comment.$inferSelect; export type CommentInsert = typeof comment.$inferInsert; + +/** Stable insertion order for comments that share the same millisecond timestamp. */ +export const commentInsertionOrder = sql`${comment}.rowid`; diff --git a/packages/cli/src/github/exec.ts b/packages/cli/src/github/exec.ts index 7c20968..4d6ae5c 100644 --- a/packages/cli/src/github/exec.ts +++ b/packages/cli/src/github/exec.ts @@ -2,17 +2,58 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; const execFileAsync = promisify(execFile); +const DEFAULT_GH_TIMEOUT_MS = 30_000; -/** Run a read-only `gh` command in `cwd` and return its stdout. */ -export async function gh(args: string[], cwd: string): Promise { +export interface GhExecOptions { + /** Bound a spawned `gh` process so passive review reads cannot hang the local UI. */ + timeoutMs?: number; +} + +async function execGh(args: string[], cwd: string, options: GhExecOptions): Promise { const { stdout } = await execFileAsync("gh", args, { cwd, encoding: "utf8", maxBuffer: 10 * 1024 * 1024, + timeout: options.timeoutMs, }); return stdout; } +/** Run a passive, read-only `gh` command with a bounded deadline. */ +export async function gh( + args: string[], + cwd: string, + options: GhExecOptions = {}, +): Promise { + return execGh(args, cwd, { timeoutMs: options.timeoutMs ?? DEFAULT_GH_TIMEOUT_MS }); +} + +/** + * Run a `gh` command without a default deadline and surface failures cleanly. + * Non-idempotent mutations must not be timed out: GitHub may have accepted the + * write before the local process is killed, making a retry duplicate the action. + */ +export async function ghWriteOrThrow(args: string[], cwd: string): Promise { + try { + return await execGh(args, cwd, {}); + } catch (err) { + throw new Error(ghErrorMessage(err)); + } +} + +/** Run a read-only `gh` command with a bounded deadline while preserving its failure reason. */ +export async function ghReadOrThrow( + args: string[], + cwd: string, + options: GhExecOptions = {}, +): Promise { + try { + return await gh(args, cwd, options); + } 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/index.ts b/packages/cli/src/github/index.ts index c77aed6..351b4e8 100644 --- a/packages/cli/src/github/index.ts +++ b/packages/cli/src/github/index.ts @@ -9,7 +9,13 @@ export { setAutoMerge, setDraft, } from "./mutations.js"; -export { getChecks, getMergeStatus, getPullRequest, getReviews } from "./pull-request.js"; +export { + getChecks, + getMergeStatus, + getPullRequest, + getPullRequestOrThrow, + getReviews, +} from "./pull-request.js"; export { type PullRequestRefs, parsePullRequestNumber, diff --git a/packages/cli/src/github/pull-request.ts b/packages/cli/src/github/pull-request.ts index 784eb68..f3905e4 100644 --- a/packages/cli/src/github/pull-request.ts +++ b/packages/cli/src/github/pull-request.ts @@ -16,7 +16,7 @@ import { type ReviewerStatus, } from "@stagereview/types/pull-request"; import { z } from "zod"; -import { gh } from "./exec.js"; +import { gh, ghReadOrThrow } from "./exec.js"; import { type GitHubRepo, parseGitHubRepo } from "./repo.js"; // ─── Pull request ───────────────────────────────────────────────────────────── @@ -118,40 +118,67 @@ export async function getPullRequest( originUrl: string | null, prNumber: number | null = null, ): Promise { - const repo = parseGitHubRepo(originUrl); - if (!repo) return null; try { - const viewArgs = - prNumber === null - ? ["pr", "view", "--json", PR_FIELDS.join(",")] - : ["pr", "view", String(prNumber), "--json", PR_FIELDS.join(",")]; - const stdout = await gh(viewArgs, repoRoot); - const parsed = GhPullRequestSchema.safeParse(JSON.parse(stdout)); - if (!parsed.success) return null; - const pr = parsed.data; - // Prefer the REST author (real avatar, bot type, [bot] login); fall back to - // gh's leaner author projection only if the REST lookup fails. - const rest = await fetchRestPullRequest(repoRoot, repo, pr.number); - const user = rest?.user ?? ghAuthorFallback(pr.author); - return { - number: pr.number, - title: pr.title, - body: pr.body ?? "", - html_url: pr.url, - // REST `state` is open|closed; merged implies closed. - state: pr.state === "OPEN" ? "open" : "closed", - draft: pr.isDraft, - merged_at: pr.mergedAt && pr.mergedAt.length > 0 ? pr.mergedAt : null, - created_at: pr.createdAt, - user, - head: { ref: pr.headRefName, sha: pr.headRefOid }, - base: { ref: pr.baseRefName }, - }; + return await getPullRequestOrThrow(repoRoot, originUrl, prNumber); } catch { return null; } } +/** + * Resolve a pull request while preserving GitHub CLI and response-shape failures. + * Review reads use this variant so an unavailable GitHub session is not mistaken + * for a repository that simply has no pull request. + */ +export async function getPullRequestOrThrow( + repoRoot: string, + originUrl: string | null, + prNumber: number | null = null, +): Promise { + const repo = parseGitHubRepo(originUrl); + if (!repo) return null; + const viewArgs = + prNumber === null + ? ["pr", "view", "--json", PR_FIELDS.join(",")] + : ["pr", "view", String(prNumber), "--json", PR_FIELDS.join(",")]; + let stdout: string; + try { + stdout = await ghReadOrThrow(viewArgs, repoRoot); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if ( + /no pull requests found/i.test(message) || + /could not resolve to a PullRequest/i.test(message) + ) { + return null; + } + throw error; + } + const parsed = GhPullRequestSchema.safeParse(JSON.parse(stdout)); + if (!parsed.success) { + throw new Error(`Unexpected response shape from GitHub pull request lookup: ${parsed.error}`); + } + const pr = parsed.data; + // Prefer the REST author (real avatar, bot type, [bot] login); fall back to + // gh's leaner author projection only if the REST lookup fails. + const rest = await fetchRestPullRequest(repoRoot, repo, pr.number); + const user = rest?.user ?? ghAuthorFallback(pr.author); + return { + number: pr.number, + title: pr.title, + body: pr.body ?? "", + html_url: pr.url, + // REST `state` is open|closed; merged implies closed. + state: pr.state === "OPEN" ? "open" : "closed", + draft: pr.isDraft, + merged_at: pr.mergedAt && pr.mergedAt.length > 0 ? pr.mergedAt : null, + created_at: pr.createdAt, + user, + head: { ref: pr.headRefName, sha: pr.headRefOid }, + base: { ref: pr.baseRefName }, + }; +} + // ─── CI checks ────────────────────────────────────────────────────────────── const GhCheckRunSchema = z.object({ diff --git a/packages/cli/src/github/review.ts b/packages/cli/src/github/review.ts new file mode 100644 index 0000000..228fedf --- /dev/null +++ b/packages/cli/src/github/review.ts @@ -0,0 +1,625 @@ +import type { ReviewEvent } from "@stagereview/types/review"; +import { z } from "zod"; +import { ghReadOrThrow, ghWriteOrThrow } 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]; + +// ─── 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) { + viewer { login } + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + id + state + viewerDidAuthor + headRefOid + baseRefOid + reviews(states: PENDING, first: 1) { nodes { id body commit { oid } } } + reviewThreads(first: 50, after: $cursor) { + pageInfo { hasNextPage endCursor } + nodes { + id + isResolved + path + line + startLine + diffSide + startDiffSide + comments(first: 100) { + pageInfo { hasNextPage endCursor } + nodes { + id + url + body + bodyHTML + createdAt + author { login avatarUrl } + pullRequestReview { state } + } + } + } + } + } + } +}`; + +const REVIEW_THREAD_COMMENTS_QUERY = `query GetReviewThreadComments($threadId: ID!, $cursor: String) { + node(id: $threadId) { + ... on PullRequestReviewThread { + comments(first: 100, after: $cursor) { + pageInfo { hasNextPage endCursor } + nodes { + id + url + body + bodyHTML + createdAt + author { login avatarUrl } + pullRequestReview { state } + } + } + } + } +}`; + +const GqlActorSchema = z.object({ login: z.string(), avatarUrl: z.string() }).nullable(); + +const GqlReviewCommentSchema = z.object({ + id: z.string(), + url: z.string(), + body: z.string(), + bodyHTML: z.string(), + createdAt: z.string(), + author: GqlActorSchema, + pullRequestReview: z.object({ state: z.string() }).nullable(), +}); + +const GqlPageInfoSchema = z.object({ + hasNextPage: z.boolean(), + endCursor: z.string().nullable(), +}); + +const GqlReviewCommentsPageSchema = z.object({ + pageInfo: GqlPageInfoSchema, + nodes: z.array(GqlReviewCommentSchema), +}); + +const GqlReviewThreadSchema = z.object({ + id: z.string(), + isResolved: z.boolean(), + path: z.string(), + line: z.number().nullable(), + startLine: z.number().nullable(), + diffSide: z.enum(GITHUB_DIFF_SIDE), + startDiffSide: z.enum(GITHUB_DIFF_SIDE).nullable(), + comments: GqlReviewCommentsPageSchema, +}); + +const ReviewQuerySchema = z.object({ + data: z.object({ + viewer: z.object({ login: z.string() }), + repository: z + .object({ + pullRequest: z + .object({ + id: z.string(), + state: z.enum(["OPEN", "CLOSED", "MERGED"]), + viewerDidAuthor: z.boolean(), + headRefOid: z.string(), + baseRefOid: z.string(), + reviews: z.object({ + nodes: z.array( + z.object({ + id: z.string(), + body: z.string(), + commit: z.object({ oid: z.string() }).nullable(), + }), + ), + }), + reviewThreads: z.object({ + pageInfo: GqlPageInfoSchema, + nodes: z.array(GqlReviewThreadSchema), + }), + }) + .nullable(), + }) + .nullable(), + }), +}); + +const ReviewThreadCommentsQuerySchema = z.object({ + data: z.object({ + node: z + .object({ + comments: GqlReviewCommentsPageSchema, + }) + .nullable(), + }), +}); + +/** A comment within a review thread, tagged with whether it's a draft (pending) or published. */ +export interface ReviewComment { + nodeId: string; + htmlUrl: string; + body: string; + /** GitHub's server-rendered HTML (resolves @mentions, #refs, emoji). */ + bodyHtml: 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; + startLine: number | null; + side: GitHubDiffSide; + startSide: GitHubDiffSide | null; + comments: ReviewComment[]; +} + +export interface GitHubReview { + /** Authenticated viewer, used to distinguish their draft comments during recovery. */ + viewerLogin: string; + /** GraphQL node id of the PR, required by the write mutations. */ + pullRequestNodeId: string; + /** GitHub lifecycle state; only an open PR accepts review writes. */ + state: "OPEN" | "CLOSED" | "MERGED"; + /** True when the viewer opened the PR (GitHub forbids approving your own PR). */ + viewerDidAuthor: boolean; + /** The PR's current head commit — comments anchor to this commit's diff. */ + headRefOid: string; + /** Merge base of the PR's current base and head commits — the other half of its diff identity. */ + mergeBaseOid: string; + /** The viewer's open pending review, or null when they have none. */ + pendingReviewNodeId: string | null; + /** Commit the pending review is pinned to, or null when no review is open. */ + pendingReviewCommitOid: string | null; + /** Existing summary text on the viewer's open pending review. */ + pendingReviewBody: string; + /** Viewer's pending (draft) comments across all threads, including anchorless ones. */ + pendingCommentCount: number; + pendingComments: PendingReviewComment[]; + threads: ReviewThread[]; +} + +export interface PendingReviewComment { + id: string; + filePath: string; + line: number | null; + body: string; +} + +const PENDING_STATE = "PENDING"; +const THREAD_COMMENT_LOAD_CONCURRENCY = 5; + +/** + * 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 viewerLogin = ""; + let state: GitHubReview["state"] = "OPEN"; + let viewerDidAuthor = false; + let headRefOid = ""; + let baseRefOid = ""; + let pendingReviewNodeId: string | null = null; + let pendingReviewCommitOid: string | null = null; + let pendingReviewBody = ""; + let pendingCommentCount = 0; + const pendingComments: PendingReviewComment[] = []; + const threads: ReviewThread[] = []; + let cursor: string | null = null; + + do { + // `-f` keeps string GraphQL variables as strings; `-F` does typed coercion, which + // would mangle a repo/owner literally named `123`, `true`, or `null` into the wrong + // GraphQL type. Only `number` (Int!) uses `-F`. + 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 ghReadOrThrow(args, repoRoot))); + if (!parsed.success) throw new Error("Unexpected response shape from GitHub review query"); + viewerLogin = parsed.data.data.viewer.login; + const pr = parsed.data.data.repository?.pullRequest; + if (!pr) break; + pullRequestNodeId = pr.id; + state = pr.state; + viewerDidAuthor = pr.viewerDidAuthor; + headRefOid = pr.headRefOid; + baseRefOid = pr.baseRefOid; + pendingReviewNodeId = pr.reviews.nodes[0]?.id ?? null; + pendingReviewCommitOid = pr.reviews.nodes[0]?.commit?.oid ?? null; + pendingReviewBody = pr.reviews.nodes[0]?.body ?? ""; + + const nodesWithComments = await loadThreadCommentsInBatches(repoRoot, pr.reviewThreads.nodes); + for (const { node, comments } of nodesWithComments) { + // Count pending (draft) comments across every thread, including outdated/whole-file + // ones dropped below — so the tray count and the empty-review check don't undercount. + for (const c of comments) { + if (c.pullRequestReview?.state !== PENDING_STATE) continue; + pendingCommentCount++; + pendingComments.push({ + id: c.id, + filePath: node.path, + line: node.line, + body: c.body, + }); + } + const root = comments[0]; + if (!root || node.line === null) continue; + threads.push({ + threadNodeId: node.id, + isResolved: node.isResolved, + path: node.path, + line: node.line, + startLine: node.startLine, + side: node.diffSide, + startSide: node.startDiffSide, + comments: comments.map(toReviewComment), + }); + } + cursor = nextCursor(pr.reviewThreads.pageInfo); + } while (cursor !== null); + + // No `pullRequest` in the response (stale/unknown PR number, or repo no longer + // resolves) — treat as unavailable rather than handing back an empty node id that + // later write mutations would post against. + if (pullRequestNodeId === "") throw new Error("Pull request not found on GitHub"); + const mergeBaseOid = await getPullRequestMergeBase(repoRoot, repo, baseRefOid, headRefOid); + + return { + viewerLogin, + pullRequestNodeId, + state, + viewerDidAuthor, + headRefOid, + mergeBaseOid, + pendingReviewNodeId, + pendingReviewCommitOid, + pendingReviewBody, + pendingCommentCount, + pendingComments, + threads, + }; +} + +const CompareSchema = z.object({ + merge_base_commit: z.object({ sha: z.string() }), +}); + +async function getPullRequestMergeBase( + repoRoot: string, + repo: GitHubRepo, + baseRefOid: string, + headRefOid: string, +): Promise { + const stdout = await ghReadOrThrow( + ["api", `repos/${repo.owner}/${repo.repo}/compare/${baseRefOid}...${headRefOid}`], + repoRoot, + ); + return CompareSchema.parse(JSON.parse(stdout)).merge_base_commit.sha; +} + +async function loadThreadCommentsInBatches( + repoRoot: string, + nodes: z.infer[], +): Promise< + { + node: z.infer; + comments: z.infer[]; + }[] +> { + const loaded: { + node: z.infer; + comments: z.infer[]; + }[] = []; + for (let start = 0; start < nodes.length; start += THREAD_COMMENT_LOAD_CONCURRENCY) { + const batch = nodes.slice(start, start + THREAD_COMMENT_LOAD_CONCURRENCY); + loaded.push( + ...(await Promise.all( + batch.map(async (node) => ({ + node, + comments: await loadReviewThreadComments(repoRoot, node.id, node.comments), + })), + )), + ); + } + return loaded; +} + +async function loadReviewThreadComments( + repoRoot: string, + threadNodeId: string, + firstPage: z.infer, +): Promise[]> { + const comments = [...firstPage.nodes]; + let cursor = nextCursor(firstPage.pageInfo); + while (cursor !== null) { + try { + const args = [ + "api", + "graphql", + "-f", + `query=${REVIEW_THREAD_COMMENTS_QUERY}`, + "-f", + `threadId=${threadNodeId}`, + "-f", + `cursor=${cursor}`, + ]; + const parsed = ReviewThreadCommentsQuerySchema.safeParse( + JSON.parse(await ghReadOrThrow(args, repoRoot)), + ); + if (!parsed.success || parsed.data.data.node === null) { + throw new Error("Unexpected response shape from GitHub review comments query"); + } + const page = parsed.data.data.node.comments; + comments.push(...page.nodes); + cursor = nextCursor(page.pageInfo); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error( + `Failed to load all comments for GitHub review thread ${threadNodeId}: ${message}`, + ); + } + } + return comments; +} + +function nextCursor(pageInfo: z.infer): string | null { + if (!pageInfo.hasNextPage) return null; + if (pageInfo.endCursor === null) { + throw new Error("GitHub returned a paginated review connection without a cursor"); + } + return pageInfo.endCursor; +} + +function toReviewComment(c: z.infer): ReviewComment { + return { + nodeId: c.id, + htmlUrl: c.url, + body: c.body, + bodyHtml: c.bodyHTML, + 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!, $commitOID: GitObjectID!) { + addPullRequestReview(input: { pullRequestId: $pullRequestId, commitOID: $commitOID }) { + 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 comments(first: 1) { nodes { 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, + commitOid: string, +): Promise { + const stdout = await ghWriteOrThrow( + gqlArgs(CREATE_PENDING_REVIEW, { pullRequestId: pullRequestNodeId, commitOID: commitOid }), + 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(), + comments: z.object({ nodes: z.array(z.object({ id: z.string() })) }), + }), + }), + }), +}); + +export interface AddedReviewThread { + threadNodeId: string; + rootCommentNodeId: string; +} + +/** Add a line-anchored comment (a new thread) to a pending review. */ +export async function addReviewThread( + repoRoot: string, + input: AddReviewThreadInput, +): Promise { + const stdout = await ghWriteOrThrow( + 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, + ); + const thread = AddedThreadSchema.parse(JSON.parse(stdout)).data.addPullRequestReviewThread.thread; + const root = thread.comments.nodes[0]; + if (!root) throw new Error("GitHub returned a review thread without its root comment"); + return { threadNodeId: thread.id, rootCommentNodeId: root.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 ghWriteOrThrow( + 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 ghWriteOrThrow( + 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 ghWriteOrThrow(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 ghWriteOrThrow( + gqlArgs(SUBMIT_REVIEW, { + pullRequestId: pullRequestNodeId, + reviewId: reviewNodeId, + event, + body, + }), + repoRoot, + ); +} + +/** Throw away the pending review and all its draft comments. */ +export async function discardReview(repoRoot: string, reviewNodeId: string): Promise { + await ghWriteOrThrow(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 ghWriteOrThrow( + gqlArgs(resolved ? RESOLVE_THREAD : UNRESOLVE_THREAD, { threadId: threadNodeId }), + repoRoot, + ); +} diff --git a/packages/cli/src/routes/comments.ts b/packages/cli/src/routes/comments.ts index d171c51..041e413 100644 --- a/packages/cli/src/routes/comments.ts +++ b/packages/cli/src/routes/comments.ts @@ -5,7 +5,7 @@ import { CreateCommentThreadBodySchema, ResolveThreadBodySchema, } from "@stagereview/types/comments"; -import { asc, eq, inArray } from "drizzle-orm"; +import { asc, eq } from "drizzle-orm"; import type { StageDb } from "../db/client.js"; import { LOCAL_USER_ID } from "../db/local-user.js"; import { @@ -13,27 +13,32 @@ import { type CommentThreadRow, chapterRun, comment, + commentInsertionOrder, commentThread, } from "../db/schema/index.js"; +import { type LocalThreadScope, loadLocalThreadRecords } from "../runs/local-comment-threads.js"; +import { isLocalThreadPromoting, isLocalThreadPromotionPending } from "../runs/review.js"; +import { REVIEW_ACTION_SCOPE, reviewActions } from "../runs/review-action-queue.js"; import { deriveScopeKey } from "../runs/scope-key.js"; import type { Route } from "../server.js"; import { parseJsonBody, writeJson } from "./json.js"; import { enforceSameOrigin } from "./pull-request-shared.js"; -export function commentRoutes(db: StageDb): Route[] { +export function commentRoutes(db: StageDb, repoRoot: string): Route[] { return [ - // Threads are anchored to a diff scope, not a run, so they survive re-imports - // of the same diff. We resolve the run's scope key and key every query off it. + // Threads are anchored to a repository + diff scope, not a run, so they + // survive re-imports without crossing into a fork that shares the same SHAs. { method: "GET", pattern: "/api/runs/:runId/comment-threads", - handler: (_req, res, params) => { - const scopeKey = resolveRunScopeKey(db, params.runId); - if (scopeKey === null) { + handler: (req, res, params) => { + if (!enforceSameOrigin(req, res)) return; + const scope = resolveRunScope(db, params.runId); + if (scope === null) { writeJson(res, 404, { error: `Run ${params.runId} not found` }); return; } - writeJson(res, 200, listThreads(db, scopeKey)); + writeJson(res, 200, listThreads(db, scope)); }, }, { @@ -41,8 +46,8 @@ export function commentRoutes(db: StageDb): Route[] { pattern: "/api/runs/:runId/comment-threads", handler: async (req, res, params) => { if (!enforceSameOrigin(req, res)) return; - const scopeKey = resolveRunScopeKey(db, params.runId); - if (scopeKey === null) { + const scope = resolveRunScope(db, params.runId); + if (scope === null) { writeJson(res, 404, { error: `Run ${params.runId} not found` }); return; } @@ -53,7 +58,8 @@ export function commentRoutes(db: StageDb): Route[] { const [threadRow] = tx .insert(commentThread) .values({ - scopeKey, + repoRoot: scope.repoRoot, + scopeKey: scope.scopeKey, filePath: body.filePath, side: body.side, startLine: body.startLine, @@ -79,31 +85,47 @@ export function commentRoutes(db: StageDb): Route[] { handler: async (req, res, params) => { if (!enforceSameOrigin(req, res)) return; const threadId = params.threadId; - if (!threadId || !threadExists(db, threadId)) { - writeJson(res, 404, { error: `Thread ${params.threadId} not found` }); + if (!threadId) { + writeJson(res, 400, { error: "Missing threadId" }); return; } const body = await parseJsonBody(req, res, CommentBodySchema); if (!body) return; + if (isLocalThreadPromotionPending(db, threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); + return; + } - const created = db.transaction((tx) => { - const [commentRow] = tx - .insert(comment) - .values({ threadId, authorId: LOCAL_USER_ID, body: body.body }) - .returning() - .all(); - if (!commentRow) throw new Error("comment insert returned no row"); - // Bump the thread so its updatedAt reflects the latest activity. - tx.update(commentThread) - .set({ updatedAt: new Date() }) - .where(eq(commentThread.id, threadId)) - .run(); - return toCommentDto(commentRow); + await reviewActions.run({ kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot }, async () => { + if (isLocalThreadPromoting(db, threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); + return; + } + if (!threadExists(db, threadId)) { + writeJson(res, 404, { error: `Thread ${threadId} not found` }); + return; + } + const created = db.transaction((tx) => { + const [commentRow] = tx + .insert(comment) + .values({ threadId, authorId: LOCAL_USER_ID, body: body.body }) + .returning() + .all(); + if (!commentRow) throw new Error("comment insert returned no row"); + // Bump the thread so its updatedAt reflects the latest activity. + tx.update(commentThread) + .set({ updatedAt: new Date() }) + .where(eq(commentThread.id, threadId)) + .run(); + return toCommentDto(commentRow); + }); + writeJson(res, 201, created); }); - writeJson(res, 201, created); }, }, { + // Resolve/reopen a local thread. GitHub threads resolve via the separate + // review-resolve route, so this stays unscoped — it only touches local rows. method: "PATCH", pattern: "/api/comment-threads/:threadId", handler: async (req, res, params) => { @@ -115,34 +137,54 @@ 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` }); + if (isLocalThreadPromotionPending(db, threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); return; } - writeJson(res, 200, toThreadDto(updated, threadComments(db, threadId))); + + await reviewActions.run({ kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot }, async () => { + if (isLocalThreadPromoting(db, threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); + 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` }); + return; + } + writeJson(res, 200, toThreadDto(updated, threadComments(db, threadId))); + }); }, }, { method: "DELETE", pattern: "/api/comment-threads/:threadId", - handler: (req, res, params) => { + handler: async (req, res, params) => { if (!enforceSameOrigin(req, res)) return; const threadId = params.threadId; if (!threadId) { writeJson(res, 400, { error: "Missing threadId" }); return; } - // Idempotent: deleting an absent thread is a no-op. The cascade FK - // removes the thread's comments. - db.delete(commentThread).where(eq(commentThread.id, threadId)).run(); - writeJson(res, 200, {}); + if (isLocalThreadPromotionPending(db, threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); + return; + } + await reviewActions.run({ kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot }, async () => { + if (isLocalThreadPromoting(db, threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); + return; + } + // Idempotent: deleting an absent thread is a no-op. The cascade FK + // removes the thread's comments. + db.delete(commentThread).where(eq(commentThread.id, threadId)).run(); + writeJson(res, 200, {}); + }); }, }, { @@ -158,60 +200,89 @@ export function commentRoutes(db: StageDb): Route[] { const body = await parseJsonBody(req, res, CommentBodySchema); if (!body) return; - const [updated] = db - .update(comment) - .set({ body: body.body }) - .where(eq(comment.id, commentId)) - .returning() - .all(); - if (!updated) { - writeJson(res, 404, { error: `Comment ${commentId} not found` }); - return; - } - writeJson(res, 200, toCommentDto(updated)); + await reviewActions.run({ kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot }, async () => { + const [existing] = db + .select({ threadId: comment.threadId }) + .from(comment) + .where(eq(comment.id, commentId)) + .limit(1) + .all(); + if (!existing) { + writeJson(res, 404, { error: `Comment ${commentId} not found` }); + return; + } + if (isLocalThreadPromoting(db, existing.threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); + return; + } + const [updated] = db + .update(comment) + .set({ body: body.body }) + .where(eq(comment.id, commentId)) + .returning() + .all(); + if (!updated) { + writeJson(res, 404, { error: `Comment ${commentId} not found` }); + return; + } + writeJson(res, 200, toCommentDto(updated)); + }); }, }, { method: "DELETE", pattern: "/api/comments/:commentId", - handler: (req, res, params) => { + handler: async (req, res, params) => { if (!enforceSameOrigin(req, res)) return; const commentId = params.commentId; if (!commentId) { writeJson(res, 400, { error: "Missing commentId" }); return; } - // Deleting the last comment removes its now-empty thread so no - // dangling anchors linger. Idempotent for an absent comment. - db.transaction((tx) => { - const [row] = tx + await reviewActions.run({ kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot }, async () => { + const [existing] = db .select({ threadId: comment.threadId }) .from(comment) .where(eq(comment.id, commentId)) .limit(1) .all(); - if (!row) return; - tx.delete(comment).where(eq(comment.id, commentId)).run(); - const remaining = tx - .select({ id: comment.id }) - .from(comment) - .where(eq(comment.threadId, row.threadId)) - .limit(1) - .all(); - if (remaining.length === 0) { - tx.delete(commentThread).where(eq(commentThread.id, row.threadId)).run(); + if (existing && isLocalThreadPromoting(db, existing.threadId)) { + writeJson(res, 409, { error: "This comment thread is being added to the review." }); + return; } + // Deleting the last comment removes its now-empty thread so no + // dangling anchors linger. Idempotent for an absent comment. + db.transaction((tx) => { + const [row] = tx + .select({ threadId: comment.threadId }) + .from(comment) + .where(eq(comment.id, commentId)) + .limit(1) + .all(); + if (!row) return; + tx.delete(comment).where(eq(comment.id, commentId)).run(); + const remaining = tx + .select({ id: comment.id }) + .from(comment) + .where(eq(comment.threadId, row.threadId)) + .limit(1) + .all(); + if (remaining.length === 0) { + tx.delete(commentThread).where(eq(commentThread.id, row.threadId)).run(); + } + }); + writeJson(res, 200, {}); }); - writeJson(res, 200, {}); }, }, ]; } -function resolveRunScopeKey(db: StageDb, runId: string | undefined): string | null { +function resolveRunScope(db: StageDb, runId: string | undefined): LocalThreadScope | null { if (!runId) return null; const [run] = db .select({ + repoRoot: chapterRun.repoRoot, scopeKind: chapterRun.scopeKind, workingTreeRef: chapterRun.workingTreeRef, baseSha: chapterRun.baseSha, @@ -223,37 +294,13 @@ function resolveRunScopeKey(db: StageDb, runId: string | undefined): string | nu .limit(1) .all(); if (!run) return null; - return deriveScopeKey(run); + return { repoRoot: run.repoRoot, scopeKey: deriveScopeKey(run) }; } -function listThreads(db: StageDb, scopeKey: string): CommentThreadDto[] { - const threads = db - .select() - .from(commentThread) - .where(eq(commentThread.scopeKey, scopeKey)) - .orderBy(asc(commentThread.createdAt)) - .all(); - if (threads.length === 0) return []; - - const comments = db - .select() - .from(comment) - .where( - inArray( - comment.threadId, - threads.map((t) => t.id), - ), - ) - .orderBy(asc(comment.createdAt)) - .all(); - - const byThread = new Map(); - for (const c of comments) { - const list = byThread.get(c.threadId); - if (list) list.push(c); - else byThread.set(c.threadId, [c]); - } - return threads.map((t) => toThreadDto(t, byThread.get(t.id) ?? [])); +function listThreads(db: StageDb, scope: LocalThreadScope): CommentThreadDto[] { + return loadLocalThreadRecords(db, scope).map(({ thread, comments }) => + toThreadDto(thread, comments), + ); } function threadComments(db: StageDb, threadId: string): CommentRow[] { @@ -261,7 +308,7 @@ function threadComments(db: StageDb, threadId: string): CommentRow[] { .select() .from(comment) .where(eq(comment.threadId, threadId)) - .orderBy(asc(comment.createdAt)) + .orderBy(asc(comment.createdAt), asc(commentInsertionOrder)) .all(); } diff --git a/packages/cli/src/routes/review.ts b/packages/cli/src/routes/review.ts new file mode 100644 index 0000000..2c6b020 --- /dev/null +++ b/packages/cli/src/routes/review.ts @@ -0,0 +1,165 @@ +import { CreateCommentThreadBodySchema } from "@stagereview/types/comments"; +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, + addPendingComment, + 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), { + sameOrigin: req, + }), + }, + { + 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), + ), + }, + { + // Create a comment directly on the PR as a pending (draft) review comment. + method: "POST", + pattern: "/api/runs/:runId/review/comment", + handler: (req, res, params) => + withRunBody(db, req, res, params.runId, CreateCommentThreadBodySchema, (run, body) => + addPendingComment(run, body), + ), + }, + { + 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", + }); + } +} + +/** Load the run and execute an action, optionally protecting sensitive data or a mutation. */ +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/local-comment-threads.ts b/packages/cli/src/runs/local-comment-threads.ts new file mode 100644 index 0000000..974fd07 --- /dev/null +++ b/packages/cli/src/runs/local-comment-threads.ts @@ -0,0 +1,68 @@ +import { and, asc, eq, inArray, or } from "drizzle-orm"; +import type { StageDb } from "../db/client.js"; +import { + type CommentRow, + type CommentThreadRow, + comment, + commentInsertionOrder, + commentThread, +} from "../db/schema/index.js"; + +/** + * Migration 0007 cannot safely assign a repository to a legacy thread when the + * same diff scope exists in multiple checkouts. Keep those rows visible in every + * matching scope until the first promotion claims one for a concrete repository. + */ +export const UNASSIGNED_REPO_ROOT = ""; + +export interface LocalThreadScope { + repoRoot: string; + scopeKey: string; +} + +export interface LocalThreadRecord { + thread: CommentThreadRow; + comments: CommentRow[]; +} + +/** Load a scope's local threads and all of their comments in two queries. */ +export function loadLocalThreadRecords(db: StageDb, scope: LocalThreadScope): LocalThreadRecord[] { + const threads = db + .select() + .from(commentThread) + .where( + and( + eq(commentThread.scopeKey, scope.scopeKey), + or( + eq(commentThread.repoRoot, scope.repoRoot), + eq(commentThread.repoRoot, UNASSIGNED_REPO_ROOT), + ), + ), + ) + .orderBy(asc(commentThread.createdAt)) + .all(); + if (threads.length === 0) return []; + + const comments = db + .select() + .from(comment) + .where( + inArray( + comment.threadId, + threads.map((thread) => thread.id), + ), + ) + .orderBy(asc(comment.createdAt), asc(commentInsertionOrder)) + .all(); + const commentsByThread = new Map(); + for (const row of comments) { + const threadComments = commentsByThread.get(row.threadId); + if (threadComments) threadComments.push(row); + else commentsByThread.set(row.threadId, [row]); + } + + return threads.map((thread) => ({ + thread, + comments: commentsByThread.get(thread.id) ?? [], + })); +} diff --git a/packages/cli/src/runs/review-action-queue.ts b/packages/cli/src/runs/review-action-queue.ts new file mode 100644 index 0000000..cda1ddf --- /dev/null +++ b/packages/cli/src/runs/review-action-queue.ts @@ -0,0 +1,101 @@ +import { createHash } from "node:crypto"; +import { mkdirSync } from "node:fs"; +import path from "node:path"; +import { lock } from "proper-lockfile"; +import { getStageDataDir } from "../db/path.js"; + +const LOCK_STALE_MS = 30_000; +const LOCK_UPDATE_MS = 10_000; +const LOCK_RETRIES = { + retries: 120, + factor: 1.2, + minTimeout: 50, + maxTimeout: 1_000, + randomize: true, +}; + +export const REVIEW_ACTION_SCOPE = { + CHECKOUT: "checkout", + PULL_REQUEST: "pullRequest", +} as const; + +export type ReviewActionScope = + | { + kind: typeof REVIEW_ACTION_SCOPE.CHECKOUT; + repoRoot: string; + } + | { + kind: typeof REVIEW_ACTION_SCOPE.PULL_REQUEST; + owner: string; + repo: string; + prNumber: number; + }; + +/** + * Serializes review actions within this process and across other `stagereview` + * processes. Lock files live in Stage's writable per-user data directory so the + * same pull request shares a lock across clones and worktrees. + */ +export class ReviewActionQueue { + private readonly tails = new Map>(); + private readonly lockDirectory: string; + + constructor(lockDirectory = path.join(getStageDataDir(), "review-locks")) { + this.lockDirectory = lockDirectory; + } + + async run(scope: ReviewActionScope, action: () => Promise): Promise { + const key = scopeKey(scope); + const previous = this.tails.get(key) ?? Promise.resolve(); + const result = previous.then( + () => this.runLocked(key, action), + () => this.runLocked(key, action), + ); + const settled = result.then( + () => undefined, + () => undefined, + ); + this.tails.set(key, settled); + try { + return await result; + } finally { + if (this.tails.get(key) === settled) this.tails.delete(key); + } + } + + private async runLocked(key: string, action: () => Promise): Promise { + mkdirSync(this.lockDirectory, { recursive: true }); + const lockTargetPath = path.join( + this.lockDirectory, + createHash("sha256").update(key).digest("hex"), + ); + const release = await lock(lockTargetPath, { + realpath: false, + stale: LOCK_STALE_MS, + update: LOCK_UPDATE_MS, + retries: LOCK_RETRIES, + }); + try { + return await action(); + } finally { + await release(); + } + } +} + +/** Shared queue for review mutations and mutations of existing local threads. */ +export const reviewActions = new ReviewActionQueue(); + +function scopeKey(scope: ReviewActionScope): string { + switch (scope.kind) { + case REVIEW_ACTION_SCOPE.CHECKOUT: + return JSON.stringify([scope.kind, path.resolve(scope.repoRoot)]); + case REVIEW_ACTION_SCOPE.PULL_REQUEST: + return JSON.stringify([ + scope.kind, + scope.owner.toLowerCase(), + scope.repo.toLowerCase(), + scope.prNumber, + ]); + } +} diff --git a/packages/cli/src/runs/review.ts b/packages/cli/src/runs/review.ts new file mode 100644 index 0000000..7ce100c --- /dev/null +++ b/packages/cli/src/runs/review.ts @@ -0,0 +1,764 @@ +import { + COMMENT_STATE, + GITHUB_REVIEW_STATUS, + type GitHubReviewComment as GitHubReviewCommentDto, + type GitHubReviewThread as GitHubReviewThreadDto, + type LocalReviewComment as LocalReviewCommentDto, + type LocalReviewThread as LocalReviewThreadDto, + REVIEW_EVENT, + type ReviewEvent, + type ReviewResponse, + type ReviewThread as ReviewThreadDto, + THREAD_SOURCE, +} from "@stagereview/types/review"; +import { and, asc, eq } from "drizzle-orm"; +import type { StageDb } from "../db/client.js"; +import { + type ChapterRunRow, + comment, + commentInsertionOrder, + commentThread, +} from "../db/schema/index.js"; +import { type GitHubRepo, getPullRequestOrThrow, parseGitHubRepo } from "../github/index.js"; +import { + type AddedReviewThread, + addReviewReply, + addReviewThread, + createPendingReview, + deleteReviewComment, + discardReview, + GITHUB_DIFF_SIDE, + type ReviewThread as GitHubApiReviewThread, + type GitHubDiffSide, + type GitHubReview, + getReview, + setThreadResolved, + submitReview, + updateReviewComment, +} from "../github/review.js"; +import { DIFF_SIDE, type DiffSide, SCOPE_KIND } from "../schema.js"; +import { loadLocalThreadRecords, UNASSIGNED_REPO_ROOT } from "./local-comment-threads.js"; +import { REVIEW_ACTION_SCOPE, reviewActions } from "./review-action-queue.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; +} + +/** + * Whether the run's diff IS the PR's current diff. GitHub anchors review comments to + * the PR head-commit's diff, and a run's comment anchors are line numbers from its + * own `base..head`. So the two align only when the run is a committed diff (working- + * tree line numbers aren't the PR's) whose head and merge base match the PR diff. + * This is the load-bearing invariant for both showing live PR threads and adding + * comments to the PR; the live worktree state is irrelevant — a committed run's + * anchors are fixed by its recorded SHAs. + */ +function runMatchesPrDiff(run: ChapterRunRow, review: GitHubReview): boolean { + return ( + run.scopeKind === SCOPE_KIND.COMMITTED && + run.headSha === review.headRefOid && + run.mergeBaseSha === review.mergeBaseOid + ); +} + +function assertGitHubWritable(run: ChapterRunRow, review: GitHubReview): void { + if (review.state !== "OPEN") { + throw new ReviewError("This pull request is closed, so its review is read-only.", 409); + } + if (runMatchesPrDiff(run, review)) return; + const reason = + run.scopeKind !== SCOPE_KIND.COMMITTED + ? "Only comments on a committed diff can be added to the PR — working-tree comments aren't anchored to the PR's commits." + : "This run's diff doesn't match the current PR diff. Re-run against the latest PR base and head to comment on it."; + throw new ReviewError(reason, 409); +} + +function assertPushable(run: ChapterRunRow, review: GitHubReview): void { + assertGitHubWritable(run, review); + if (review.pendingReviewNodeId !== null && review.pendingReviewCommitOid !== review.headRefOid) { + throw new ReviewError( + "Your pending GitHub review belongs to an earlier PR version. Submit or discard it on GitHub before commenting on this run.", + 409, + ); + } +} + +function canWriteToGitHub(run: ChapterRunRow, review: GitHubReview): boolean { + return review.state === "OPEN" && runMatchesPrDiff(run, review); +} + +function canPushToReview(run: ChapterRunRow, review: GitHubReview): boolean { + return ( + canWriteToGitHub(run, review) && + (review.pendingReviewNodeId === null || review.pendingReviewCommitOid === review.headRefOid) + ); +} + +function requireReviewThread(review: GitHubReview, threadNodeId: string): void { + if (review.threads.some((thread) => thread.threadNodeId === threadNodeId)) return; + throw new ReviewError("That GitHub review thread doesn't belong to this pull request.", 400); +} + +function requirePendingComment(review: GitHubReview, nodeId: string): void { + if (review.pendingComments.some((candidate) => candidate.id === nodeId)) return; + const comment = review.threads + .flatMap((thread) => thread.comments) + .find((candidate) => candidate.nodeId === nodeId); + if (comment?.isPending) return; + throw new ReviewError( + "That GitHub comment isn't an editable pending comment on this pull request.", + 400, + ); +} + +// ─── Read: merged local + GitHub review ───────────────────────────────────────── + +function loadLocalThreads(db: StageDb, run: ChapterRunRow): ReviewThreadDto[] { + return loadLocalThreadRecords(db, { + repoRoot: run.repoRoot, + scopeKey: deriveScopeKey(run), + }).map(({ thread, comments }): LocalReviewThreadDto => { + 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): LocalReviewCommentDto => ({ + id: c.id, + state: COMMENT_STATE.LOCAL, + body: c.body, + bodyHtml: null, + author: null, + nodeId: null, + htmlUrl: null, + createdAt: c.createdAt.toISOString(), + }), + ), + }; + }); +} + +function toGitHubThreadDto(t: GitHubApiReviewThread): GitHubReviewThreadDto { + // `line` is non-null (getReview drops anchorless threads); start defaults to line. + const endLine = t.line; + 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): GitHubReviewCommentDto => ({ + id: c.nodeId, + state: c.isPending ? COMMENT_STATE.PENDING : COMMENT_STATE.SUBMITTED, + body: c.body, + bodyHtml: c.bodyHtml, + author: { login: c.authorLogin, avatarUrl: c.authorAvatarUrl || null }, + nodeId: c.nodeId, + htmlUrl: c.htmlUrl, + 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, run); + const base = { + threads: localThreads, + pendingComments: [], + pendingCommentCount: 0, + hasPendingReview: false, + pendingReviewBody: "", + isOwnPullRequest: false, + canPushToReview: false, + canWriteToGitHub: false, + }; + + const repo = parseGitHubRepo(run.originUrl); + if (!repo) return { ...base, github: GITHUB_REVIEW_STATUS.NONE }; + const hasStoredPullRequest = run.prNumber !== null; + + let review: GitHubReview; + try { + let prNumber = run.prNumber; + if (prNumber === null) { + const pr = await getPullRequestOrThrow(run.repoRoot, run.originUrl, null); + prNumber = pr?.number ?? null; + } + if (prNumber === null) return { ...base, github: GITHUB_REVIEW_STATUS.NONE }; + review = await getReview(run.repoRoot, repo, prNumber); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`Failed to load GitHub review: ${message}\n`); + return { ...base, github: GITHUB_REVIEW_STATUS.OFFLINE }; + } + + // The PR's live threads anchor to its head-commit diff. If this run isn't that + // exact diff, overlaying them would mis-anchor comments on unrelated lines, so we + // surface only local line threads. Keep the pending-review lifecycle visible so + // the viewer can inspect and discard drafts even though this run is read-only. + if (!runMatchesPrDiff(run, review)) { + // Automatic discovery follows the checkout's current branch, not the branch + // that created this historical run. A mismatch therefore cannot safely retain + // lifecycle controls: they may belong to an entirely different pull request. + if (!hasStoredPullRequest) return { ...base, github: GITHUB_REVIEW_STATUS.NONE }; + return { + ...base, + github: GITHUB_REVIEW_STATUS.AVAILABLE, + pendingComments: review.pendingComments, + pendingCommentCount: review.pendingCommentCount, + hasPendingReview: review.pendingReviewNodeId !== null, + pendingReviewBody: review.pendingReviewBody, + isOwnPullRequest: review.viewerDidAuthor, + }; + } + + const githubThreads = review.threads.map(toGitHubThreadDto); + return { + github: GITHUB_REVIEW_STATUS.AVAILABLE, + threads: [...localThreads, ...githubThreads], + pendingComments: review.pendingComments, + pendingCommentCount: review.pendingCommentCount, + hasPendingReview: review.pendingReviewNodeId !== null, + pendingReviewBody: review.pendingReviewBody, + isOwnPullRequest: review.viewerDidAuthor, + canPushToReview: canPushToReview(run, review), + canWriteToGitHub: canWriteToGitHub(run, review), + }; +} + +// ─── Write: review actions ────────────────────────────────────────────────────── + +interface ReviewTarget { + repo: GitHubRepo; + prNumber: number; + review: GitHubReview; +} + +interface ReviewIdentity { + repo: GitHubRepo; + prNumber: number; +} + +/** Resolve the run's repository and pull request without reading mutable review state. */ +async function resolveReviewIdentity(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 getPullRequestOrThrow(run.repoRoot, run.originUrl, null); + prNumber = pr?.number ?? null; + } + if (prNumber === null) { + throw new ReviewError("No GitHub pull request found for this run.", 404); + } + return { repo, prNumber }; +} + +/** + * Resolve the PR, acquire its cross-checkout lock, then read fresh review state + * inside the lock before performing a mutation. + */ +async function withLockedReviewTarget( + run: ChapterRunRow, + action: (target: ReviewTarget) => Promise, +): Promise { + const identity = await resolveReviewIdentity(run); + return reviewActions.run( + { + kind: REVIEW_ACTION_SCOPE.PULL_REQUEST, + owner: identity.repo.owner, + repo: identity.repo.repo, + prNumber: identity.prNumber, + }, + async () => { + const review = await getReview(run.repoRoot, identity.repo, identity.prNumber); + return action({ ...identity, review }); + }, + ); +} + +/** The viewer's pending review node id, opening an empty pending review if none is open. */ +async function openPendingReview( + run: ChapterRunRow, + review: GitHubReview, +): Promise<{ reviewNodeId: string; created: boolean }> { + if (review.pendingReviewNodeId !== null) { + if (review.pendingReviewCommitOid !== review.headRefOid) { + throw new ReviewError( + "Your pending GitHub review belongs to an earlier PR version. Submit or discard it on GitHub before commenting on this run.", + 409, + ); + } + return { reviewNodeId: review.pendingReviewNodeId, created: false }; + } + return { + reviewNodeId: await createPendingReview( + run.repoRoot, + review.pullRequestNodeId, + review.headRefOid, + ), + created: true, + }; +} + +/** + * Run an action against the viewer's pending review, opening one if needed. If we + * had to open the review and the action then fails (e.g. an out-of-diff line), the + * just-created empty review is discarded so it doesn't linger on the PR as a stray + * "review to submit". A pre-existing review is never discarded. + */ +async function withPendingReview( + run: ChapterRunRow, + review: GitHubReview, + action: (reviewNodeId: string) => Promise, +): Promise { + const { reviewNodeId, created } = await openPendingReview(run, review); + try { + return await action(reviewNodeId); + } catch (err) { + if (created) { + try { + await discardReview(run.repoRoot, reviewNodeId); + } catch (discardError) { + const message = discardError instanceof Error ? discardError.message : String(discardError); + process.stderr.write( + `Failed to discard newly created GitHub review after action failure: ${message}\n`, + ); + } + } + throw err; + } +} + +// Queued ids reject duplicate promotion requests immediately. The active set starts +// only after promotion owns the checkout lock, so an earlier local mutation wins. +const queuedPromotions = new Set(); +const promotingThreads = new Set(); + +/** True while the local thread is frozen for an in-flight or interrupted promotion. */ +export function isLocalThreadPromoting(db: StageDb, localThreadId: string): boolean { + if (promotingThreads.has(localThreadId)) return true; + const [thread] = db + .select({ + threadNodeId: commentThread.promotionThreadNodeId, + rootCommentNodeId: commentThread.promotionRootCommentNodeId, + }) + .from(commentThread) + .where(eq(commentThread.id, localThreadId)) + .limit(1) + .all(); + return ( + thread !== undefined && (thread.threadNodeId !== null || thread.rootCommentNodeId !== null) + ); +} + +/** True once promotion is queued, active, or interrupted and awaiting recovery. */ +export function isLocalThreadPromotionPending(db: StageDb, localThreadId: string): boolean { + return queuedPromotions.has(localThreadId) || isLocalThreadPromoting(db, localThreadId); +} + +/** + * 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 { + if (queuedPromotions.has(localThreadId)) { + throw new ReviewError("This comment is already being added to the review.", 409); + } + queuedPromotions.add(localThreadId); + try { + await reviewActions.run( + { kind: REVIEW_ACTION_SCOPE.CHECKOUT, repoRoot: run.repoRoot }, + async () => { + promotingThreads.add(localThreadId); + try { + await promoteLocalThread(db, run, localThreadId); + } finally { + promotingThreads.delete(localThreadId); + } + }, + ); + } finally { + queuedPromotions.delete(localThreadId); + } +} + +async function promoteLocalThread( + 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); + if (thread.repoRoot !== run.repoRoot && thread.repoRoot !== UNASSIGNED_REPO_ROOT) { + throw new ReviewError("This comment belongs to another repository.", 400); + } + // The thread must belong to this run's diff scope; its anchor was computed + // against that diff, so promoting one from another scope would mis-anchor. + if (thread.scopeKey !== deriveScopeKey(run)) { + throw new ReviewError("This comment doesn't belong to this run's diff.", 400); + } + const comments = db + .select() + .from(comment) + .where(eq(comment.threadId, localThreadId)) + .orderBy(asc(comment.createdAt), asc(commentInsertionOrder)) + .all(); + const root = comments[0]; + if (!root) throw new ReviewError("Thread has no comments to add to the review.", 400); + const replies = comments.slice(1); + if ((thread.promotionThreadNodeId === null) !== (thread.promotionRootCommentNodeId === null)) { + throw new ReviewError( + "This comment has incomplete promotion state and cannot be resumed.", + 409, + ); + } + if (thread.promotionReplyCount > replies.length) { + throw new ReviewError( + "This comment's promotion progress is invalid and cannot be resumed.", + 409, + ); + } + + await withLockedReviewTarget(run, async ({ review }) => { + assertPushable(run, review); + const side = toGitHubSide(thread.side); + const startLine = thread.endLine !== thread.startLine ? thread.startLine : null; + const wasUnassigned = thread.repoRoot === UNASSIGNED_REPO_ROOT; + if (wasUnassigned) { + const [claimed] = db + .update(commentThread) + .set({ repoRoot: run.repoRoot }) + .where( + and( + eq(commentThread.id, localThreadId), + eq(commentThread.repoRoot, UNASSIGNED_REPO_ROOT), + ), + ) + .returning({ id: commentThread.id }) + .all(); + if (!claimed) { + throw new ReviewError("This comment belongs to another repository.", 400); + } + } + + let addedThread: AddedReviewThread | null = + thread.promotionThreadNodeId !== null && thread.promotionRootCommentNodeId !== null + ? { + threadNodeId: thread.promotionThreadNodeId, + rootCommentNodeId: thread.promotionRootCommentNodeId, + } + : null; + let promotedReplyCount = thread.promotionReplyCount; + let reviewNodeId: string | null = null; + let created = false; + let remoteRootIsPending = false; + let remoteThreadIsResolved = false; + let rootCreatedThisAttempt = false; + let rollbackReplyCount = promotedReplyCount; + try { + if (addedThread !== null) { + const remoteThread = review.threads.find( + (candidate) => candidate.threadNodeId === addedThread?.threadNodeId, + ); + const persistedRoot = remoteThread?.comments.find( + (candidate) => candidate.nodeId === addedThread?.rootCommentNodeId, + ); + // If the persisted remote root was removed manually, restart cleanly. + if (!remoteThread || !persistedRoot) { + clearPromotionProgress(db, localThreadId); + addedThread = null; + promotedReplyCount = 0; + } else { + remoteRootIsPending = persistedRoot.isPending; + remoteThreadIsResolved = remoteThread.isResolved; + // A crash can land a reply immediately before its local checkpoint. + // Reconcile only replies authored by this viewer before sending + // anything again; another participant may have posted the same body. + while ( + promotedReplyCount < replies.length && + remoteThread.comments[promotedReplyCount + 1]?.authorLogin === review.viewerLogin && + remoteThread.comments[promotedReplyCount + 1]?.body === + replies[promotedReplyCount]?.body + ) { + promotedReplyCount++; + } + db.update(commentThread) + .set({ promotionReplyCount: promotedReplyCount }) + .where(eq(commentThread.id, localThreadId)) + .run(); + } + } + rollbackReplyCount = promotedReplyCount; + + if (addedThread === null || promotedReplyCount < replies.length) { + const pendingReview = await openPendingReview(run, review); + reviewNodeId = pendingReview.reviewNodeId; + created = pendingReview.created; + } + if (addedThread === null) { + if (reviewNodeId === null) throw new Error("Pending review was not opened"); + addedThread = await addReviewThread(run.repoRoot, { + pullRequestNodeId: review.pullRequestNodeId, + reviewNodeId, + path: thread.filePath, + body: root.body, + line: thread.endLine, + side, + startLine, + startSide: startLine !== null ? side : null, + }); + rootCreatedThisAttempt = true; + remoteRootIsPending = true; + const persisted = db + .update(commentThread) + .set({ + promotionThreadNodeId: addedThread.threadNodeId, + promotionRootCommentNodeId: addedThread.rootCommentNodeId, + promotionReplyCount: 0, + }) + .where(eq(commentThread.id, localThreadId)) + .run(); + if (persisted.changes !== 1) throw new Error("Local promotion checkpoint was not saved"); + } + for (const [index, reply] of replies.entries()) { + if (index < promotedReplyCount) continue; + if (reviewNodeId === null) throw new Error("Pending review was not opened"); + await addReviewReply(run.repoRoot, addedThread.threadNodeId, reply.body, reviewNodeId); + const persisted = db + .update(commentThread) + .set({ promotionReplyCount: index + 1 }) + .where(eq(commentThread.id, localThreadId)) + .run(); + if (persisted.changes !== 1) throw new Error("Local promotion checkpoint was not saved"); + } + if (thread.resolvedAt !== null && !remoteThreadIsResolved) { + await setThreadResolved(run.repoRoot, addedThread.threadNodeId, true); + } + } catch (err) { + // A pending root can be deleted to roll back its whole partial thread. A + // published root must never be deleted: it may have been submitted on + // GitHub while this process was interrupted. + let remoteRootRolledBack = addedThread === null; + if (addedThread !== null && remoteRootIsPending) { + try { + await deleteReviewComment(run.repoRoot, addedThread.rootCommentNodeId); + remoteRootRolledBack = true; + } catch {} + } + if (created && reviewNodeId !== null) { + try { + await discardReview(run.repoRoot, reviewNodeId); + if (rootCreatedThisAttempt) { + remoteRootRolledBack = true; + } else if (!remoteRootRolledBack) { + db.update(commentThread) + .set({ promotionReplyCount: rollbackReplyCount }) + .where(eq(commentThread.id, localThreadId)) + .run(); + } + } catch {} + } + if (remoteRootRolledBack) clearPromotionProgress(db, localThreadId); + if (wasUnassigned && remoteRootRolledBack) { + db.update(commentThread) + .set({ repoRoot: UNASSIGNED_REPO_ROOT }) + .where(and(eq(commentThread.id, localThreadId), eq(commentThread.repoRoot, run.repoRoot))) + .run(); + } + throw err; + } + // Every comment landed remotely; the cascade removes all local comment rows. + db.delete(commentThread).where(eq(commentThread.id, localThreadId)).run(); + }); +} + +function clearPromotionProgress(db: StageDb, localThreadId: string): void { + db.update(commentThread) + .set({ + promotionThreadNodeId: null, + promotionRootCommentNodeId: null, + promotionReplyCount: 0, + }) + .where(eq(commentThread.id, localThreadId)) + .run(); +} + +export interface PendingCommentAnchor { + filePath: string; + side: DiffSide; + startLine: number; + endLine: number; + body: string; +} + +/** + * Create a comment directly on the PR as a pending (draft) review comment, opening + * the viewer's review if needed. This is the "Comment on the PR" path — unlike + * `addLocalThreadToReview`, nothing is stored locally; the comment lives only on + * GitHub. GitHub anchors it to the PR's current diff, rejecting out-of-diff lines. + */ +export async function addPendingComment( + run: ChapterRunRow, + anchor: PendingCommentAnchor, +): Promise { + await withLockedReviewTarget(run, async ({ review }) => { + assertPushable(run, review); + const side = toGitHubSide(anchor.side); + const startLine = anchor.endLine !== anchor.startLine ? anchor.startLine : null; + await withPendingReview(run, review, (reviewNodeId) => + addReviewThread(run.repoRoot, { + pullRequestNodeId: review.pullRequestNodeId, + reviewNodeId, + path: anchor.filePath, + body: anchor.body, + line: anchor.endLine, + side, + startLine, + startSide: startLine !== null ? side : null, + }), + ); + }); +} + +/** 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 { + await withLockedReviewTarget(run, async ({ review }) => { + if (pending) assertPushable(run, review); + else assertGitHubWritable(run, review); + requireReviewThread(review, threadNodeId); + if (!pending) { + await addReviewReply(run.repoRoot, threadNodeId, body, null); + return; + } + await withPendingReview(run, review, (reviewNodeId) => + 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 { + await withLockedReviewTarget(run, async ({ review }) => { + assertPushable(run, review); + if (review.viewerDidAuthor && event !== REVIEW_EVENT.COMMENT) { + throw new ReviewError("You can't approve or request changes on your own pull request.", 400); + } + if (event === REVIEW_EVENT.REQUEST_CHANGES && body.trim() === "") { + throw new ReviewError("Add a summary to request changes.", 400); + } + if (event === REVIEW_EVENT.COMMENT && body.trim() === "" && review.pendingCommentCount === 0) { + throw new ReviewError( + "Add a summary or at least one pending comment to submit a review.", + 400, + ); + } + await withPendingReview(run, review, (reviewNodeId) => + 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 { + await withLockedReviewTarget(run, async ({ review }) => { + if (run.prNumber === null && !runMatchesPrDiff(run, review)) { + throw new ReviewError( + "This run isn't tied to the pull request currently discovered for the checkout. Re-run with --pr before discarding its review.", + 409, + ); + } + 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 withLockedReviewTarget(run, async ({ review }) => { + assertPushable(run, review); + requirePendingComment(review, nodeId); + 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 withLockedReviewTarget(run, async ({ review }) => { + assertPushable(run, review); + requirePendingComment(review, nodeId); + await deleteReviewComment(run.repoRoot, nodeId); + }); +} + +/** Resolve or reopen a GitHub review thread. */ +export async function resolveGitHubThread( + run: ChapterRunRow, + threadNodeId: string, + resolved: boolean, +): Promise { + await withLockedReviewTarget(run, async ({ review }) => { + assertGitHubWritable(run, review); + requireReviewThread(review, threadNodeId); + await setThreadResolved(run.repoRoot, threadNodeId, resolved); + }); +} diff --git a/packages/cli/src/show.ts b/packages/cli/src/show.ts index 2d71487..20ba2d5 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"; @@ -29,13 +30,15 @@ import { LOOPBACK_HOST, startServer } from "./server.js"; export async function show(jsonPath: string, options: DiffScopeOptions): Promise { const db = getDb(); const { chaptersFile, prNumber } = await buildChaptersFile(jsonPath, options); - const { runId } = insertChaptersFile(db, chaptersFile, readRepoContext(), prNumber); + const repoContext = readRepoContext(); + const { runId } = insertChaptersFile(db, chaptersFile, repoContext, prNumber); const handle = await startServer({ routes: [ ...runRoutes(db), ...viewStateRoutes(db), - ...commentRoutes(db), + ...commentRoutes(db, repoContext.root), + ...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/comments.ts b/packages/types/src/comments.ts index 845a2d9..899b874 100644 --- a/packages/types/src/comments.ts +++ b/packages/types/src/comments.ts @@ -3,8 +3,7 @@ import { DIFF_SIDE } from "./chapters.ts"; // 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. -// 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(), 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..f51102c --- /dev/null +++ b/packages/types/src/review.ts @@ -0,0 +1,174 @@ +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; + +const ReviewCommentBaseSchema = z.object({ + id: z.string(), + body: z.string(), + createdAt: z.string(), +}); + +export const LocalReviewCommentSchema = ReviewCommentBaseSchema.extend({ + state: z.literal(COMMENT_STATE.LOCAL), + bodyHtml: z.null(), + author: z.null(), + nodeId: z.null(), + htmlUrl: z.null(), +}); +export type LocalReviewComment = z.infer; + +export const GitHubReviewCommentSchema = ReviewCommentBaseSchema.extend({ + state: z.union([z.literal(COMMENT_STATE.PENDING), z.literal(COMMENT_STATE.SUBMITTED)]), + // GitHub's server-rendered HTML resolves @mentions, issue references, and emoji. + bodyHtml: z.string(), + author: ReviewCommentAuthorSchema, + nodeId: z.string().min(1), + htmlUrl: z.string(), +}); +export type GitHubReviewComment = z.infer; + +/** A local-only comment or a GitHub-backed pending/submitted comment. */ +export const ReviewCommentSchema = z.union([LocalReviewCommentSchema, GitHubReviewCommentSchema]); +export type ReviewComment = z.infer; + +const ReviewThreadBaseSchema = z.object({ + id: z.string(), + filePath: z.string(), + side: z.enum(DIFF_SIDE), + startLine: z.number().int().positive(), + endLine: z.number().int().positive(), + isResolved: z.boolean(), +}); + +export const LocalReviewThreadSchema = ReviewThreadBaseSchema.extend({ + source: z.literal(THREAD_SOURCE.LOCAL), + threadNodeId: z.null(), + comments: z.array(LocalReviewCommentSchema), +}); +export type LocalReviewThread = z.infer; + +export const GitHubReviewThreadSchema = ReviewThreadBaseSchema.extend({ + source: z.literal(THREAD_SOURCE.GITHUB), + threadNodeId: z.string().min(1), + comments: z.array(GitHubReviewCommentSchema), +}); +export type GitHubReviewThread = z.infer; + +/** A line-anchored local or GitHub thread with source-specific identifier invariants. */ +export const ReviewThreadSchema = z.discriminatedUnion("source", [ + LocalReviewThreadSchema, + GitHubReviewThreadSchema, +]); +export type ReviewThread = z.infer; + +export const PendingReviewCommentSchema = z.object({ + id: z.string(), + filePath: z.string(), + line: z.number().int().positive().nullable(), + body: z.string(), +}); +export type PendingReviewComment = z.infer; + +export const ReviewResponseSchema = z.object({ + github: z.enum(GITHUB_REVIEW_STATUS), + threads: z.array(ReviewThreadSchema), + // Includes anchorless/outdated drafts that cannot be placed in `threads`. + pendingComments: z.array(PendingReviewCommentSchema), + pendingCommentCount: z.number().int().nonnegative(), + hasPendingReview: z.boolean(), + // Existing summary text on the viewer's pending GitHub review. + pendingReviewBody: z.string(), + // The viewer opened this PR — GitHub forbids approving/requesting changes on it. + isOwnPullRequest: z.boolean(), + // Whether pending-review actions are allowed. In addition to an open, matching + // PR diff, any existing pending review must be pinned to the current head. + canPushToReview: z.boolean(), + // Whether direct GitHub thread actions are allowed. This can remain true when an + // older pending review blocks draft writes, but is false for a closed PR or mismatched diff. + canWriteToGitHub: 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; diff --git a/packages/web/src/app/runs.$runId.tsx b/packages/web/src/app/runs.$runId.tsx index 89acff4..284359f 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..ba7a5b1 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, @@ -27,12 +27,13 @@ import { type DraftBodies, type DraftState, findDraftAt, + getDraftGitHubDestination, isSameAnchor, readDraftBody, + setDraftGitHubPreference, upsertDraft, writeDraftBody, } from "@/lib/comment-drafts"; -import { useCommentThreadsContext } from "@/lib/comment-threads-context"; import { type AnnotatedLineRef, COMMENT_SIDE, @@ -40,9 +41,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 +196,9 @@ export function PierreDiffViewer({ }, [allLineRefsByFile, filePath]); // ---- Line-anchored comments ---- - const comments = useCommentThreadsContext(); - const { createThread } = comments; + const comments = useReviewContext(); + const { createLocalThread, createPendingComment } = comments; + const canPushToReview = comments.canPushToReview; const fileThreads = useMemo( () => (filePath ? (comments.threadsByFile.get(filePath) ?? []) : []), [comments.threadsByFile, filePath], @@ -221,9 +224,12 @@ export function PierreDiffViewer({ // Open a composer at an anchor. A row holds at most one composer, so re-opening the // same (side, endLine) adopts the new range's startLine rather than duplicating it. - const openDraft = useCallback((anchor: CommentDraft) => { - setDrafts((prev) => upsertDraft(prev, anchor)); - }, []); + const openDraft = useCallback( + (anchor: CommentDraft) => { + setDrafts((prev) => upsertDraft(prev, anchor, canPushToReview)); + }, + [canPushToReview], + ); const closeDraft = useCallback((draft: CommentDraft) => { clearDraftBody(draftBodiesRef.current, draft.side, draft.endLine); @@ -231,28 +237,31 @@ export function PierreDiffViewer({ }, []); const handleCreateComment = useCallback( - async (draft: CommentDraft, body: string) => { + async (draft: DraftState, body: string, onPr: boolean) => { if (!filePath) return; const setError = (error: string | null) => setDrafts((prev) => prev.map((d) => (isSameAnchor(d, draft.side, draft.endLine) ? { ...d, error } : d)), ); setError(null); + const anchor = { + filePath, + side: draft.side, + startLine: draft.startLine, + endLine: draft.endLine, + body, + }; try { - await createThread({ - filePath, - side: draft.side, - startLine: draft.startLine, - endLine: draft.endLine, - body, - }); + // "Comment on the PR" creates a pending GitHub comment; otherwise it stays local. + if (onPr) await createPendingComment(anchor); + else await createLocalThread(anchor); closeDraft(draft); } catch (err) { setError(err instanceof Error ? err.message : "Failed to add comment"); throw err; // keep the composer open with the body intact } }, - [filePath, createThread, closeDraft], + [filePath, createLocalThread, createPendingComment, closeDraft], ); const handleThreadMouseEnter = useCallback((thread: CommentThread) => { @@ -275,6 +284,8 @@ export function PierreDiffViewer({ const threads = annotation.metadata ?? []; const draft = findDraftAt(drafts, annotation.side, annotation.lineNumber); if (threads.length === 0 && !draft) return null; + const githubDestination = + draft === undefined ? null : getDraftGitHubDestination(draft, canPushToReview); return (
handleThreadMouseEnter(thread)} onMouseLeave={handleThreadMouseLeave} > - +
))} {draft && ( @@ -296,7 +307,7 @@ export function PierreDiffViewer({ // by its anchor to force a clean remount (re-reading its own draft text) // instead of inheriting another composer's in-progress state. writeDraftBody(draftBodiesRef.current, draft.side, draft.endLine, body) } - onSubmit={(body) => handleCreateComment(draft, body)} + onToggleChange={(toGitHub) => + setDrafts((prev) => + setDraftGitHubPreference(prev, draft.side, draft.endLine, toGitHub), + ) + } + destination={ + githubDestination?.available + ? { + toggleLabel: "Add to GitHub review", + defaultOn: githubDestination.defaultOn, + on: { + label: "Pending on GitHub", + description: "Only you can see it until you submit your review.", + isGitHub: true, + }, + off: { + label: "Local only", + description: "Saved on this machine and never sent to GitHub.", + isGitHub: false, + }, + } + : { + label: "Local only", + description: "Saved on this machine and never sent to GitHub.", + isGitHub: false, + } + } + onSubmit={(body, onPr) => + handleCreateComment(draft, body, onPr && githubDestination?.available === true) + } onCancel={() => closeDraft(draft)} /> )} ); }, - [drafts, handleCreateComment, closeDraft, handleThreadMouseEnter, handleThreadMouseLeave], + [ + drafts, + canPushToReview, + handleCreateComment, + closeDraft, + handleThreadMouseEnter, + handleThreadMouseLeave, + ], ); const renderGutterUtility = useCallback( diff --git a/packages/web/src/components/comments/__tests__/comment-form.test.tsx b/packages/web/src/components/comments/__tests__/comment-form.test.tsx new file mode 100644 index 0000000..97f10ae --- /dev/null +++ b/packages/web/src/components/comments/__tests__/comment-form.test.tsx @@ -0,0 +1,59 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { CommentForm } from "../comment-form"; + +afterEach(cleanup); + +describe("comment destination", () => { + it("always explains when a new comment stays local", () => { + render( + , + ); + + expect(screen.getByText("Local only")).toBeTruthy(); + expect(screen.getByText("Saved on this machine and never sent to GitHub.")).toBeTruthy(); + }); + + it("updates the explanation when the GitHub review toggle changes", () => { + const onToggleChange = vi.fn(); + render( + , + ); + + expect(screen.getByText("Pending on GitHub")).toBeTruthy(); + expect(screen.getByRole("button", { name: "Suggestion" })).toBeTruthy(); + fireEvent.click(screen.getByRole("checkbox", { name: "Add to GitHub review" })); + expect(screen.getByText("Local only")).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Suggestion" })).toBeNull(); + expect(onToggleChange).toHaveBeenCalledWith(false); + }); +}); diff --git a/packages/web/src/components/comments/__tests__/review-thread.test.ts b/packages/web/src/components/comments/__tests__/review-thread.test.ts new file mode 100644 index 0000000..ea1bbb6 --- /dev/null +++ b/packages/web/src/components/comments/__tests__/review-thread.test.ts @@ -0,0 +1,139 @@ +import { + COMMENT_STATE, + type GitHubReviewComment, + type GitHubReviewThread, + ReviewThreadSchema, + THREAD_SOURCE, +} from "@stagereview/types/review"; +import { describe, expect, it } from "vitest"; +import { + activeEditingCommentId, + activeReplyingState, + canEditReviewComment, + canPublishReplyImmediately, + canReplyToGitHubThread, + deleteRemovesReplies, + threadChevronClassName, +} from "../review-thread"; + +describe("thread chevron", () => { + it("tracks the controlled collapsible state instead of a shared data-state attribute", () => { + expect(threadChevronClassName(true)).toContain("rotate-90"); + expect(threadChevronClassName(false)).not.toContain("rotate-90"); + }); +}); + +describe("GitHub reply destination", () => { + it("keeps replies pending while every comment in the thread is still a draft", () => { + expect(canPublishReplyImmediately(makeThread([COMMENT_STATE.PENDING]))).toBe(false); + }); + + it("allows an immediate reply once the thread has published content", () => { + expect( + canPublishReplyImmediately(makeThread([COMMENT_STATE.SUBMITTED, COMMENT_STATE.PENDING])), + ).toBe(true); + }); + + it("allows only an immediate reply when an older draft blocks pending writes", () => { + expect(canReplyToGitHubThread(makeThread([COMMENT_STATE.SUBMITTED]), true, false)).toBe(true); + expect(canReplyToGitHubThread(makeThread([COMMENT_STATE.PENDING]), true, false)).toBe(false); + }); + + it("disables replies when the pull request is read-only", () => { + expect(canReplyToGitHubThread(makeThread([COMMENT_STATE.SUBMITTED]), false, false)).toBe(false); + }); + + it("clears an active reply when the thread becomes read-only", () => { + expect(activeReplyingState(true, true)).toBe(true); + expect(activeReplyingState(true, false)).toBe(false); + }); +}); + +describe("GitHub pending comment actions", () => { + it("hides edit and delete when the pending review cannot be written", () => { + const [comment] = makeThread([COMMENT_STATE.PENDING]).comments; + if (!comment) throw new Error("Expected a pending comment"); + + expect(canEditReviewComment(comment, false)).toBe(false); + expect(canEditReviewComment(comment, true)).toBe(true); + }); + + it("closes the editor after a pending comment is submitted", () => { + const pendingThread = makeThread([COMMENT_STATE.PENDING]); + const [pendingComment] = pendingThread.comments; + if (!pendingComment) throw new Error("Expected a pending comment"); + + expect(activeEditingCommentId(pendingThread.comments, pendingComment.id, true)).toBe( + pendingComment.id, + ); + + const submittedThread = makeThread([COMMENT_STATE.SUBMITTED]); + expect(activeEditingCommentId(submittedThread.comments, pendingComment.id, true)).toBeNull(); + }); +}); + +describe("review thread source invariants", () => { + const base = { + id: "thread", + filePath: "src/file.ts", + side: "additions", + startLine: 1, + endLine: 1, + isResolved: false, + comments: [], + }; + + it("rejects a GitHub thread without a node id", () => { + expect( + ReviewThreadSchema.safeParse({ + ...base, + source: THREAD_SOURCE.GITHUB, + threadNodeId: null, + }).success, + ).toBe(false); + }); + + it("rejects a local thread with a GitHub node id", () => { + expect( + ReviewThreadSchema.safeParse({ + ...base, + source: THREAD_SOURCE.LOCAL, + threadNodeId: "THREAD_github", + }).success, + ).toBe(false); + }); +}); + +describe("deleteRemovesReplies", () => { + it("warns for a pending GitHub root but not its reply", () => { + const thread = makeThread([COMMENT_STATE.PENDING, COMMENT_STATE.PENDING]); + const [root, reply] = thread.comments; + if (!root || !reply) throw new Error("Expected a root and reply"); + + expect(deleteRemovesReplies(thread, root)).toBe(true); + expect(deleteRemovesReplies(thread, reply)).toBe(false); + }); +}); + +function makeThread(states: GitHubReviewComment["state"][]): GitHubReviewThread { + return { + id: "thread", + source: THREAD_SOURCE.GITHUB, + threadNodeId: "thread", + filePath: "src/file.ts", + side: "additions", + startLine: 1, + endLine: 1, + isResolved: false, + comments: states.map((state, index) => ({ + id: `comment-${index}`, + state, + body: "Comment", + bodyHtml: "

Comment

", + author: { login: "octocat", avatarUrl: null }, + nodeId: `comment-${index}`, + htmlUrl: `https://github.com/owner/repo/pull/1#discussion_r${index}`, + createdAt: "2026-01-01T00:00:00Z", + })), + }; +} diff --git a/packages/web/src/components/comments/comment-form.tsx b/packages/web/src/components/comments/comment-form.tsx index d431d0d..2b6112c 100644 --- a/packages/web/src/components/comments/comment-form.tsx +++ b/packages/web/src/components/comments/comment-form.tsx @@ -1,11 +1,28 @@ -import { type KeyboardEvent, useEffect, useRef, useState } from "react"; +import { type KeyboardEvent, useEffect, useId, useRef, useState } from "react"; import { Button } from "@/components/ui/button"; +import { Checkbox } from "@/components/ui/checkbox"; import { CommentMarkdownEditor } from "./comment-markdown-editor"; +type DestinationDetails = { + label: string; + description: string; + isGitHub: boolean; +}; + +type CommentDestination = + | DestinationDetails + | { + toggleLabel: string; + on: DestinationDetails; + off: DestinationDetails; + defaultOn?: boolean; + }; + interface CommentFormProps { /** Label for the primary submit button (e.g. "Comment", "Reply", "Update"). */ label: string; - onSubmit: (body: string) => void | Promise; + /** `toggleOn` carries the optional checkbox state; it's `true` when no toggle is shown. */ + onSubmit: (body: string, toggleOn: boolean) => void | Promise; onCancel: () => void; placeholder?: string; error?: string | null; @@ -13,7 +30,11 @@ interface CommentFormProps { initialBody?: string; /** Reports each edit so a parent can persist an in-progress draft across remounts. */ onBodyChange?: (body: string) => void; + /** Reports destination changes so a parent can preserve them across remounts. */ + onToggleChange?: (toggleOn: boolean) => void; autoFocus?: boolean; + /** Explains where a new comment goes, with an optional checkbox to switch destinations. */ + destination?: CommentDestination; } export function CommentForm({ @@ -24,13 +45,28 @@ export function CommentForm({ error, initialBody, onBodyChange, + onToggleChange, autoFocus = true, + destination, }: CommentFormProps) { const [body, setBody] = useState(initialBody ?? ""); const [isSubmitting, setIsSubmitting] = useState(false); + const hasDestinationToggle = destination !== undefined && "toggleLabel" in destination; + const [toggleOn, setToggleOn] = useState( + hasDestinationToggle ? (destination.defaultOn ?? true) : true, + ); + const toggleId = useId(); const textareaRef = useRef(null); const submittingRef = useRef(false); const hasContent = body.trim().length > 0; + const activeDestination = + destination === undefined + ? undefined + : hasDestinationToggle + ? toggleOn + ? destination.on + : destination.off + : destination; useEffect(() => { if (!autoFocus) return; @@ -47,7 +83,7 @@ export function CommentForm({ submittingRef.current = true; setIsSubmitting(true); try { - await onSubmit(trimmed); + await onSubmit(trimmed, hasDestinationToggle ? toggleOn : true); setBody(""); } catch { // The caller surfaces the error; preserve the body so the user can retry. @@ -85,20 +121,52 @@ export function CommentForm({ className="rounded-xl border border-border bg-card transition-shadow has-[textarea:focus-visible]:border-ring has-[textarea:focus-visible]:ring-2 has-[textarea:focus-visible]:ring-ring/20" textareaClassName="max-h-[12rem] overflow-y-auto" previewClassName="max-h-[12rem] overflow-y-auto" + showSuggestion={activeDestination?.isGitHub === true} > {error &&

{error}

} -
- - + {activeDestination && ( +
+

+ Destination + {activeDestination.label} +

+

{activeDestination.description}

+
+ )} +
+ {hasDestinationToggle ? ( + + ) : ( +
+ )} +
+ + +
diff --git a/packages/web/src/components/comments/comment-markdown-editor.tsx b/packages/web/src/components/comments/comment-markdown-editor.tsx index 928f23f..0eba77d 100644 --- a/packages/web/src/components/comments/comment-markdown-editor.tsx +++ b/packages/web/src/components/comments/comment-markdown-editor.tsx @@ -25,6 +25,7 @@ interface CommentMarkdownEditorProps { className?: string; textareaClassName?: string; previewClassName?: string; + showSuggestion?: boolean; onKeyDown?: (event: KeyboardEvent) => void; children?: ReactNode; } @@ -40,6 +41,7 @@ export function CommentMarkdownEditor({ className, textareaClassName, previewClassName, + showSuggestion = false, onKeyDown, children, }: CommentMarkdownEditorProps) { @@ -69,10 +71,9 @@ export function CommentMarkdownEditor({ ); })}
- {/* Suggestion blocks only apply on a PR, so omit that toolbar item for local comments. */} (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); - - function handleResolveToggle() { - const next = !isResolved; - // 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 }); - } - - 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); - try { - await replyToThread({ threadId: thread.id, body }); - setIsReplying(false); - } catch (err) { - setError(errorMessage(err, "Failed to add reply")); - throw err; - } - } - - async function submitEdit(commentId: string, body: string) { - setError(null); - try { - await editComment({ commentId, body }); - setEditingId(null); - } catch (err) { - setError(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); - setDeleteTarget(null); - } - - const idle = !isReplying && editingId === null; - - return ( - -
-
- - - - - - - {isOpen ? "Collapse thread" : "Expand thread"} - - - - {idle && ( -
- - - - - Reply - - { - setIsOpen(true); - setError(null); - setEditingId(root.id); - }} - onDelete={() => setDeleteTarget({ kind: "thread", hasReplies: replies.length > 0 })} - deleteLabel={replies.length > 0 ? "Delete thread" : "Delete"} - /> -
- )} -
- - - {editingId === root.id ? ( - submitEdit(root.id, b)} - onCancel={() => { - setEditingId(null); - setError(null); - }} - /> - ) : ( - - )} - - {replies.length > 0 && ( -
- {replies.map((reply) => ( - { - setError(null); - setEditingId(reply.id); - }} - onCancelEdit={() => { - setEditingId(null); - setError(null); - }} - onSubmitEdit={(b) => submitEdit(reply.id, b)} - onDelete={() => setDeleteTarget({ kind: "comment", commentId: reply.id })} - /> - ))} -
- )} - - {isReplying && ( - { - setIsReplying(false); - setError(null); - }} - /> - )} - -
- - setDeleteTarget(null)} - onConfirm={confirmDelete} - /> - - ); -} - -function ResolveButton({ isResolved, onToggle }: { isResolved: boolean; onToggle: () => void }) { - return ( - - - - - {isResolved ? "Reopen conversation" : "Mark as resolved"} - - ); -} - -function CommentByline({ createdAt }: { createdAt: string }) { - const viewer = useViewer(); - return ( -

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

- ); -} - -function ReplyItem({ - reply, - idle, - isEditing, - error, - onEdit, - onCancelEdit, - onSubmitEdit, - onDelete, -}: { - reply: Comment; - idle: boolean; - isEditing: boolean; - error: string | null; - onEdit: () => void; - onCancelEdit: () => void; - onSubmitEdit: (body: string) => Promise; - onDelete: () => void; -}) { - 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 && } -
- {isEditing ? ( - - ) : ( - - )} -
- ); -} - -function DeleteDialog({ - target, - onCancel, - onConfirm, -}: { - target: DeleteTarget | null; - onCancel: () => void; - onConfirm: () => void; -}) { - const isThreadDelete = target?.kind === "thread" && target.hasReplies; - return ( - { - if (!open) onCancel(); - }} - > - - - {isThreadDelete ? "Delete thread" : "Delete comment"} - - {isThreadDelete - ? "This deletes the whole conversation, including replies. This can't be undone." - : "This deletes the comment. This can't be undone."} - - - - Cancel - - - - - ); -} diff --git a/packages/web/src/components/comments/review-thread.tsx b/packages/web/src/components/comments/review-thread.tsx new file mode 100644 index 0000000..468ea17 --- /dev/null +++ b/packages/web/src/components/comments/review-thread.tsx @@ -0,0 +1,579 @@ +import { + COMMENT_STATE, + type ReviewComment, + type ReviewThread, + THREAD_SOURCE, +} from "@stagereview/types/review"; +import { + ChevronRight, + Circle, + CircleCheck, + GitPullRequestArrow, + MessageSquare, + User, +} from "lucide-react"; +import { useEffect, useState } from "react"; +import { + AlertDialog, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + 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 { formatTimeAgo } from "@/lib/format"; +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"; + +function errorMessage(err: unknown, fallback: string): string { + return err instanceof Error ? err.message : fallback; +} + +const PENDING_BADGE_CN = + "border-yellow-500/50 bg-yellow-50 text-yellow-800 dark:bg-yellow-950/20 dark:text-yellow-200"; + +// Local comments remain editable offline. A pending GitHub comment is editable +// only while this run can write to the current pending review. +export function canEditReviewComment(comment: ReviewComment, canPushToReview: boolean): boolean { + return ( + comment.state === COMMENT_STATE.LOCAL || + (comment.state === COMMENT_STATE.PENDING && canPushToReview) + ); +} + +export function activeEditingCommentId( + comments: ReviewComment[], + editingId: string | null, + canPushToReview: boolean, +): string | null { + if (editingId === null) return null; + const comment = comments.find((candidate) => candidate.id === editingId); + return comment && canEditReviewComment(comment, canPushToReview) ? editingId : null; +} + +function StateBadge({ state }: { state: ReviewComment["state"] }) { + if (state === COMMENT_STATE.PENDING) { + return ( + + Pending + + ); + } + if (state === COMMENT_STATE.LOCAL) { + return ( + + Local + + ); + } + return null; +} + +export function threadChevronClassName(isOpen: boolean): string { + return cn("size-3.5 transition-transform duration-200", isOpen && "rotate-90"); +} + +export function canPublishReplyImmediately(thread: ReviewThread): boolean { + return thread.comments.some((comment) => comment.state === COMMENT_STATE.SUBMITTED); +} + +export function canReplyToGitHubThread( + thread: ReviewThread, + canWriteToGitHub: boolean, + canPushToReview: boolean, +): boolean { + return canWriteToGitHub && (canPushToReview || canPublishReplyImmediately(thread)); +} + +export function activeReplyingState(isReplying: boolean, canReply: boolean): boolean { + return isReplying && canReply; +} + +export function deleteRemovesReplies(thread: ReviewThread, comment: ReviewComment): boolean { + return thread.comments.length > 1 && thread.comments[0]?.id === comment.id; +} + +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 canPushToReview = review.canPushToReview; + const canWriteToGitHub = review.canWriteToGitHub; + + const [isOpen, setIsOpen] = useState(!thread.isResolved); + const [isReplying, setIsReplying] = useState(false); + const [editingId, setEditingId] = useState(null); + const [deleteTarget, setDeleteTarget] = useState(null); + const [error, setError] = useState(null); + + const activeEditingId = activeEditingCommentId(thread.comments, editingId, canPushToReview); + const canReply = !isGitHub || canReplyToGitHubThread(thread, canWriteToGitHub, canPushToReview); + const activeIsReplying = activeReplyingState(isReplying, canReply); + useEffect(() => { + if (editingId !== null && activeEditingId === null) { + setEditingId(null); + setError(null); + } + }, [editingId, activeEditingId]); + useEffect(() => { + if (isReplying && !activeIsReplying) { + setIsReplying(false); + setError(null); + } + }, [isReplying, activeIsReplying]); + + const root = thread.comments[0]; + if (!root) return null; + const replies = thread.comments.slice(1); + const idle = !activeIsReplying && activeEditingId === null; + const publishesImmediately = isGitHub && canPublishReplyImmediately(thread); + const forceImmediateReply = publishesImmediately && !canPushToReview; + + function setOpenError(message: string | null) { + setError(message); + } + + async function handleResolveToggle() { + const next = !thread.isResolved; + const wasOpen = isOpen; + const hasActiveForm = activeIsReplying || activeEditingId !== null || deleteTarget !== null; + if (!next || !hasActiveForm) setIsOpen(!next); + try { + if (thread.source === THREAD_SOURCE.GITHUB) { + await review.resolveGitHub({ threadNodeId: thread.threadNodeId, resolved: next }); + } else { + await review.resolveLocalThread({ threadId: thread.id, resolved: next }); + } + } catch (err) { + setIsOpen(wasOpen); + toastError(err, "Failed to update resolved state"); + } + } + + function handleOpenChange(open: boolean) { + if (!open && (activeIsReplying || activeEditingId !== null || deleteTarget !== null)) return; + setIsOpen(open); + } + + async function submitReply(body: string, startReview: boolean) { + setOpenError(null); + try { + if (thread.source === THREAD_SOURCE.GITHUB) { + // Published threads may choose pending vs immediate; draft-only threads + // always pass true because CommentForm has no destination toggle. + await review.replyGitHub({ threadNodeId: thread.threadNodeId, body, pending: startReview }); + } else { + await review.replyLocal({ threadId: thread.id, body }); + } + setIsReplying(false); + } catch (err) { + setOpenError(errorMessage(err, "Failed to add reply")); + throw err; + } + } + + async function submitEdit(comment: ReviewComment, body: string) { + setOpenError(null); + try { + if (comment.state === COMMENT_STATE.LOCAL) { + await review.editLocalComment({ commentId: comment.id, body }); + } else { + await review.editGitHubComment({ nodeId: comment.nodeId, body }); + } + setEditingId(null); + } catch (err) { + setOpenError(errorMessage(err, "Failed to update comment")); + throw err; + } + } + + 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 { + await review.deleteGitHubComment(comment.nodeId); + } + } catch (err) { + toastError(err, "Failed to delete comment"); + } + } + + async function handleAddToReview() { + try { + await review.addToReview(thread.id); + } catch (err) { + toastError(err, "Failed to add to review"); + } + } + + const rootDeleteRemovesReplies = deleteRemovesReplies(thread, root); + + return ( + +
+
+ + + + + + + {isOpen ? "Collapse thread" : "Expand thread"} + + + + + {idle && ( +
+ {root.state === COMMENT_STATE.LOCAL && canPushToReview && ( + + + + + Add to GitHub review (pending) + + )} + {(!isGitHub || githubAvailable) && canReply && ( + + + + + Reply + + )} + {canEditReviewComment(root, canPushToReview) && ( + { + setIsOpen(true); + setOpenError(null); + setEditingId(root.id); + }} + onDelete={() => setDeleteTarget(root)} + deleteLabel={rootDeleteRemovesReplies ? "Delete thread" : "Delete"} + /> + )} +
+ )} +
+ + + {activeEditingId === root.id ? ( + submitEdit(root, b)} + onCancel={() => { + setEditingId(null); + setOpenError(null); + }} + /> + ) : ( + + )} + + {replies.length > 0 && ( +
+ {replies.map((reply) => ( + { + setOpenError(null); + setEditingId(reply.id); + }} + onCancelEdit={() => { + setEditingId(null); + setOpenError(null); + }} + onSubmitEdit={(b) => submitEdit(reply, b)} + onDelete={() => setDeleteTarget(reply)} + canPushToReview={canPushToReview} + /> + ))} +
+ )} + + {activeIsReplying && ( + + submitReply(body, forceImmediateReply ? false : startReview) + } + onCancel={() => { + setIsReplying(false); + setOpenError(null); + }} + /> + )} + +
+ + setDeleteTarget(null)} + onConfirm={confirmDelete} + /> + + ); +} + +function ResolveButton({ + isResolved, + onToggle, + disabled, +}: { + isResolved: boolean; + onToggle: () => void; + disabled: boolean; +}) { + return ( + + + + + {isResolved ? "Reopen conversation" : "Mark as resolved"} + + ); +} + +// 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; + return ( +

+ + {avatarUrl && } + + + + + {name} + {comment.htmlUrl ? ( + + + + ) : ( + + )} +

+ ); +} + +// GitHub comments render GitHub's own server-rendered HTML (resolves @mentions, +// #refs, emoji); local comments render their raw markdown. +function CommentBody({ comment }: { comment: ReviewComment }) { + return comment.bodyHtml !== null ? ( + + ) : ( + + ); +} + +function ReplyItem({ + reply, + idle, + isEditing, + error, + onEdit, + onCancelEdit, + onSubmitEdit, + onDelete, + canPushToReview, +}: { + reply: ReviewComment; + idle: boolean; + isEditing: boolean; + error: string | null; + onEdit: () => void; + onCancelEdit: () => void; + onSubmitEdit: (body: string) => Promise; + onDelete: () => void; + canPushToReview: boolean; +}) { + return ( +
+
+ + + {idle && canEditReviewComment(reply, canPushToReview) && ( + + )} +
+ {isEditing && canEditReviewComment(reply, canPushToReview) ? ( + + ) : ( + + )} +
+ ); +} + +function DeleteDialog({ + target, + isThread, + onCancel, + onConfirm, +}: { + target: ReviewComment | null; + isThread: boolean; + onCancel: () => void; + onConfirm: () => void; +}) { + return ( + { + if (!open) onCancel(); + }} + > + + + {isThread ? "Delete thread" : "Delete comment"} + + {isThread + ? "This deletes the whole conversation, including replies. This can't be undone." + : "This deletes the comment. This can't be undone."} + + + + Cancel + + + + + ); +} + +function toastError(err: unknown, fallback: string): void { + toast.error(errorMessage(err, fallback)); +} 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..5c948f1 --- /dev/null +++ b/packages/web/src/components/pull-request/review-panel.tsx @@ -0,0 +1,341 @@ +import { + type PendingReviewComment, + REVIEW_EVENT, + type ReviewEvent, +} from "@stagereview/types/review"; +import { ChevronRight, CornerDownLeft, MessageSquarePlus, Trash2 } from "lucide-react"; +import { type KeyboardEvent, useEffect, useMemo, useRef, useState } from "react"; +import { CommentMarkdownEditor } from "@/components/comments/comment-markdown-editor"; +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 { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible"; +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 { canSubmitReview } from "@/lib/review-submission"; +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", + }, +]; + +// Flatten the viewer's pending comments, grouped by file, for the "what you're +// about to submit" list. +function collectPendingByFile( + comments: PendingReviewComment[], +): Map { + const byFile = new Map(); + for (const comment of comments) { + const list = byFile.get(comment.filePath) ?? []; + if (!byFile.has(comment.filePath)) byFile.set(comment.filePath, list); + list.push(comment); + } + return byFile; +} + +function ActionSelector({ + selected, + onSelect, + disabled, + isOwnPullRequest, +}: { + selected: ReviewEvent; + onSelect: (event: ReviewEvent) => void; + disabled: boolean; + isOwnPullRequest: boolean; +}) { + return ( +
+ {ACTION_OPTIONS.map(({ event, label, description }) => { + const isSelected = selected === event; + // GitHub forbids approving / requesting changes on your own PR. + const blockedByOwnership = isOwnPullRequest && event !== REVIEW_EVENT.COMMENT; + const isDisabled = disabled || blockedByOwnership; + return ( + + ); + })} +
+ ); +} + +function PendingCommentsList({ + byFile, + count, +}: { + byFile: Map; + count: number; +}) { + const [open, setOpen] = useState(false); + if (count === 0) return null; + return ( + + + + Pending comments + + {count} + + + +
+
+ {[...byFile.entries()].map(([path, comments]) => ( +
+

{path}

+
+ {comments.map((c) => ( +
+ + {c.line === null ? "Outdated" : `L${c.line}`} + + {c.body} +
+ ))} +
+
+ ))} +
+
+ + + ); +} + +const isMac = typeof navigator !== "undefined" && /Mac|iPhone|iPad/.test(navigator.platform); + +/** + * 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 badge counts the viewer's draft comments and the list shows what will publish. + */ +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); + const textareaRef = useRef(null); + + const { pendingCommentCount, hasPendingReview, isOwnPullRequest, canPushToReview } = review; + const pendingByFile = useMemo( + () => collectPendingByFile(review.pendingComments), + [review.pendingComments], + ); + useEffect(() => setBody(review.pendingReviewBody), [review.pendingReviewBody]); + + if (review.github !== GITHUB_REVIEW_STATUS.AVAILABLE) return null; + + // On your own PR only "Comment" is allowed; coerce the effective event so a stale + // Approve/Request-changes selection can never be submitted (the radios are disabled, + // but the prior `selected` state would otherwise persist). + const effectiveEvent = + isOwnPullRequest && selected !== REVIEW_EVENT.COMMENT ? REVIEW_EVENT.COMMENT : selected; + const canSubmit = + canPushToReview && + canSubmitReview({ + event: effectiveEvent, + body, + pendingCommentCount, + isSubmitting, + }); + + function selectAction(event: ReviewEvent) { + if (isOwnPullRequest && event !== REVIEW_EVENT.COMMENT) return; + setSelected(event); + } + + async function handleSubmit() { + if (!canSubmit) return; + setIsSubmitting(true); + try { + await review.submitReview({ event: effectiveEvent, 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); + } + } + + function handleKeyDown(e: KeyboardEvent) { + if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) { + e.preventDefault(); + void handleSubmit(); + } + } + + return ( + <> + + + + + + + + + + + {canPushToReview ? "Submit your review" : "This GitHub review is read-only"} + + + +

Finish your review

+

+ {!canPushToReview + ? "This GitHub review is read-only. You can still discard an existing pending review." + : pendingCommentCount > 0 + ? `${pendingCommentCount} pending comment${pendingCommentCount === 1 ? "" : "s"} will be published.` + : "No pending comments yet — add comments to your review from the diff."} +

+ + + +
+ + +
+ + + + { + if (!v && !isSubmitting) setShowDiscard(false); + }} + > + + + Discard review + + This deletes all your pending comments on the PR. This can't be undone. + + + + Cancel + + + + + + ); +} diff --git a/packages/web/src/lib/__tests__/comment-drafts.test.ts b/packages/web/src/lib/__tests__/comment-drafts.test.ts index f755893..bd783e1 100644 --- a/packages/web/src/lib/__tests__/comment-drafts.test.ts +++ b/packages/web/src/lib/__tests__/comment-drafts.test.ts @@ -1,3 +1,4 @@ +import type { LocalReviewThread as CommentThread } from "@stagereview/types/review"; import { describe, expect, it } from "vitest"; import { buildCommentAnnotations, @@ -6,30 +7,36 @@ import { type DraftBodies, type DraftState, findDraftAt, + getDraftGitHubDestination, isSameAnchor, readDraftBody, + setDraftGitHubPreference, upsertDraft, writeDraftBody, } from "../comment-drafts"; -import type { CommentThread } from "../use-comment-threads"; function makeThread( over: Partial & Pick, ): CommentThread { return { id: `t-${over.side}-${over.endLine}`, + source: "local", + threadNodeId: null, filePath: "a.ts", startLine: over.endLine, - resolvedAt: null, - createdAt: "2026-06-08T00:00:00.000Z", - updatedAt: "2026-06-08T00:00:00.000Z", + isResolved: false, comments: [], ...over, }; } -function draftState(side: CommentDraft["side"], startLine: number, endLine: number): DraftState { - return { side, startLine, endLine, error: null }; +function draftState( + side: CommentDraft["side"], + startLine: number, + endLine: number, + toGitHub = false, +): DraftState { + return { side, startLine, endLine, error: null, toGitHub }; } function rowFor( @@ -96,18 +103,25 @@ describe("draft anchor helpers", () => { describe("upsertDraft", () => { it("appends a new draft when no composer occupies the row", () => { - const result = upsertDraft([draftState("additions", 5, 5)], draftState("deletions", 8, 10)); + const result = upsertDraft( + [draftState("additions", 5, 5)], + draftState("deletions", 8, 10), + true, + ); expect(result).toHaveLength(2); expect(findDraftAt(result, "deletions", 10)?.startLine).toBe(8); + expect(findDraftAt(result, "deletions", 10)?.toGitHub).toBe(true); }); it("adopts the new startLine when re-opening the same (side, endLine) row", () => { const existing = { ...draftState("additions", 3, 10), error: "boom" as string | null }; - const result = upsertDraft([existing], draftState("additions", 7, 10)); + const result = upsertDraft([existing], draftState("additions", 7, 10), true); expect(result).toHaveLength(1); expect(result[0]?.startLine).toBe(7); // A re-drag clears any stale submit error. expect(result[0]?.error).toBeNull(); + // GitHub becoming available must not change an already-open local-only composer. + expect(result[0]?.toGitHub).toBe(false); }); it("leaves other open drafts untouched when updating one", () => { @@ -115,17 +129,53 @@ describe("upsertDraft", () => { const result = upsertDraft( [other, draftState("additions", 3, 10)], draftState("additions", 7, 10), + true, ); expect(result).toContain(other); expect(findDraftAt(result, "additions", 10)?.startLine).toBe(7); }); it("opens a separate composer for a different endLine", () => { - const result = upsertDraft([draftState("additions", 3, 10)], draftState("additions", 3, 15)); + const result = upsertDraft( + [draftState("additions", 3, 10)], + draftState("additions", 3, 15), + false, + ); expect(result).toHaveLength(2); }); }); +describe("getDraftGitHubDestination", () => { + it("offers GitHub as an explicit opt-in when it becomes available after opening", () => { + expect(getDraftGitHubDestination(draftState("additions", 5, 5), true)).toEqual({ + available: true, + defaultOn: false, + }); + }); + + it("withdraws GitHub when it becomes unavailable after opening", () => { + expect(getDraftGitHubDestination(draftState("additions", 5, 5, true), false)).toEqual({ + available: false, + defaultOn: false, + }); + }); + + it("restores the user's local choice when GitHub returns", () => { + const opened = [draftState("additions", 5, 5, true)]; + const switchedLocal = setDraftGitHubPreference(opened, "additions", 5, false); + const draft = switchedLocal[0]; + + expect(draft && getDraftGitHubDestination(draft, false)).toEqual({ + available: false, + defaultOn: false, + }); + expect(draft && getDraftGitHubDestination(draft, true)).toEqual({ + available: true, + defaultOn: false, + }); + }); +}); + describe("draft body store", () => { it("reads, writes, and clears text keyed by (side, endLine)", () => { const bodies: DraftBodies = new Map(); diff --git a/packages/web/src/lib/__tests__/comment-threads-context.test.tsx b/packages/web/src/lib/__tests__/comment-threads-context.test.tsx deleted file mode 100644 index 8deb9b0..0000000 --- a/packages/web/src/lib/__tests__/comment-threads-context.test.tsx +++ /dev/null @@ -1,92 +0,0 @@ -// @vitest-environment happy-dom - -import { act, render, waitFor } from "@testing-library/react"; -import { afterEach, describe, expect, it, vi } from "vitest"; -import { toast } from "@/components/ui/sonner"; -import { CommentThreadsProvider } from "../comment-threads-context"; -import { makeWrapper } from "./fixtures"; - -vi.mock("@/components/ui/sonner", () => ({ toast: { error: vi.fn(), dismiss: vi.fn() } })); - -afterEach(() => { - vi.unstubAllGlobals(); - vi.clearAllMocks(); -}); - -function stubFetch(status: number, body: string): void { - vi.stubGlobal( - "fetch", - vi.fn( - async () => new Response(body, { status, headers: { "Content-Type": "application/json" } }), - ), - ); -} - -describe("CommentThreadsProvider", () => { - it("surfaces a failed threads fetch as a toast so it isn't mistaken for no comments", async () => { - stubFetch(500, "boom"); - const { Wrapper } = makeWrapper(); - - render( - - diff - , - { wrapper: Wrapper }, - ); - - await waitFor(() => - expect(vi.mocked(toast.error)).toHaveBeenCalledWith( - "Couldn't load comments", - expect.objectContaining({ id: "comment-threads-error" }), - ), - ); - }); - - it("does not toast when the fetch succeeds with no comments", async () => { - stubFetch(200, "[]"); - const { Wrapper } = makeWrapper(); - - render( - - diff - , - { wrapper: Wrapper }, - ); - - await waitFor(() => expect(vi.mocked(fetch)).toHaveBeenCalledTimes(1)); - expect(vi.mocked(toast.error)).not.toHaveBeenCalled(); - }); - - it("dismisses the error toast once a later fetch recovers", async () => { - let calls = 0; - vi.stubGlobal( - "fetch", - vi.fn(async () => { - calls += 1; - return calls === 1 - ? new Response("boom", { status: 500 }) - : new Response("[]", { status: 200, headers: { "Content-Type": "application/json" } }); - }), - ); - const { client, Wrapper } = makeWrapper(); - - render( - - diff - , - { wrapper: Wrapper }, - ); - - await waitFor(() => expect(vi.mocked(toast.error)).toHaveBeenCalled()); - // Ignore the no-op dismiss that runs before any error appears. - vi.mocked(toast.dismiss).mockClear(); - - await act(async () => { - await client.refetchQueries(); - }); - - await waitFor(() => - expect(vi.mocked(toast.dismiss)).toHaveBeenCalledWith("comment-threads-error"), - ); - }); -}); diff --git a/packages/web/src/lib/__tests__/review-submission.test.ts b/packages/web/src/lib/__tests__/review-submission.test.ts new file mode 100644 index 0000000..740298f --- /dev/null +++ b/packages/web/src/lib/__tests__/review-submission.test.ts @@ -0,0 +1,60 @@ +import { REVIEW_EVENT } from "@stagereview/types/review"; +import { describe, expect, it } from "vitest"; +import { canSubmitReview } from "@/lib/review-submission"; + +describe("canSubmitReview", () => { + it("requires a summary when requesting changes even with pending comments", () => { + expect( + canSubmitReview({ + event: REVIEW_EVENT.REQUEST_CHANGES, + body: " ", + pendingCommentCount: 2, + isSubmitting: false, + }), + ).toBe(false); + }); + + it("allows requesting changes with a summary", () => { + expect( + canSubmitReview({ + event: REVIEW_EVENT.REQUEST_CHANGES, + body: "Please address the race.", + pendingCommentCount: 0, + isSubmitting: false, + }), + ).toBe(true); + }); + + it("allows a bodyless approval", () => { + expect( + canSubmitReview({ + event: REVIEW_EVENT.APPROVE, + body: "", + pendingCommentCount: 0, + isSubmitting: false, + }), + ).toBe(true); + }); + + it("requires content or a pending comment for a comment review", () => { + expect( + canSubmitReview({ + event: REVIEW_EVENT.COMMENT, + body: "", + pendingCommentCount: 0, + isSubmitting: false, + }), + ).toBe(false); + }); + + it("disables submission while a review is being submitted", () => { + expect( + canSubmitReview({ + event: REVIEW_EVENT.APPROVE, + body: "Looks good", + pendingCommentCount: 2, + isSubmitting: true, + }), + ).toBe(false); + }); +}); diff --git a/packages/web/src/lib/__tests__/use-review-local-overlay.test.tsx b/packages/web/src/lib/__tests__/use-review-local-overlay.test.tsx new file mode 100644 index 0000000..7def396 --- /dev/null +++ b/packages/web/src/lib/__tests__/use-review-local-overlay.test.tsx @@ -0,0 +1,105 @@ +// @vitest-environment happy-dom + +import type { CommentThread } from "@stagereview/types/comments"; +import type { ReviewResponse } from "@stagereview/types/review"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useReview } from "../use-review"; +import { makeWrapper } from "./fixtures"; + +const localThread = (id: string): CommentThread => ({ + id: `THREAD_${id}`, + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + resolvedAt: null, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + comments: [ + { + id: `COMMENT_${id}`, + body: `${id} comment`, + authorId: "local", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }, + ], +}); + +const FIRST_THREAD = localThread("first"); +const SECOND_THREAD = localThread("second"); +const AVAILABLE_REVIEW: ReviewResponse = { + github: "available", + threads: [], + pendingComments: [], + pendingCommentCount: 0, + hasPendingReview: false, + pendingReviewBody: "", + isOwnPullRequest: false, + canPushToReview: true, + canWriteToGitHub: true, +}; + +afterEach(() => vi.unstubAllGlobals()); + +describe("useReview local overlay", () => { + it("ignores an older reconciliation that finishes after a newer local write", async () => { + let postCount = 0; + const reconcileResolvers: Array<(response: Response) => void> = []; + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review")) return jsonResponse(AVAILABLE_REVIEW); + if (url.endsWith("/comment-threads") && method === "POST") { + postCount += 1; + return jsonResponse(postCount === 1 ? FIRST_THREAD : SECOND_THREAD); + } + if (url.endsWith("/comment-threads")) { + return new Promise((resolve) => reconcileResolvers.push(resolve)); + } + return new Response("not found", { status: 404 }); + }), + ); + const { Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + await waitFor(() => expect(result.current.github).toBe("available")); + + for (const body of ["first comment", "second comment"]) { + await act(async () => { + await result.current.createLocalThread({ + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body, + }); + }); + } + await waitFor(() => expect(reconcileResolvers).toHaveLength(2)); + expect(result.current.threads.map((thread) => thread.id)).toEqual([ + "THREAD_first", + "THREAD_second", + ]); + + const resolveFirst = reconcileResolvers.at(0); + const resolveSecond = reconcileResolvers.at(1); + if (!resolveFirst || !resolveSecond) throw new Error("Expected two reconciliation requests"); + await act(async () => resolveSecond(jsonResponse([FIRST_THREAD, SECOND_THREAD]))); + await act(async () => resolveFirst(jsonResponse([FIRST_THREAD]))); + + expect(result.current.threads.map((thread) => thread.id)).toEqual([ + "THREAD_first", + "THREAD_second", + ]); + }); +}); + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} diff --git a/packages/web/src/lib/__tests__/use-review-promotion.test.tsx b/packages/web/src/lib/__tests__/use-review-promotion.test.tsx new file mode 100644 index 0000000..798a09c --- /dev/null +++ b/packages/web/src/lib/__tests__/use-review-promotion.test.tsx @@ -0,0 +1,167 @@ +// @vitest-environment happy-dom + +import type { CommentThread } from "@stagereview/types/comments"; +import type { ReviewResponse, ReviewThread } from "@stagereview/types/review"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useReview } from "../use-review"; +import { makeWrapper } from "./fixtures"; + +const LOCAL_THREAD: CommentThread = { + id: "THREAD_local", + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + resolvedAt: null, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + comments: [ + { + id: "COMMENT_local", + body: "Promote me", + authorId: "local", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }, + ], +}; + +const REMOTE_THREAD: ReviewThread = { + id: "THREAD_remote", + source: "github", + threadNodeId: "THREAD_remote", + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + isResolved: false, + comments: [ + { + id: "COMMENT_remote", + state: "pending", + body: "Promote me", + bodyHtml: "

Promote me

", + author: { login: "octocat", avatarUrl: null }, + nodeId: "COMMENT_remote", + htmlUrl: "https://github.com/owner/repo/pull/1#discussion_r1", + createdAt: "2026-01-01T00:00:00Z", + }, + ], +}; + +const review = (promoted: boolean): ReviewResponse => ({ + github: "available", + threads: promoted ? [REMOTE_THREAD] : [], + pendingComments: [], + pendingCommentCount: promoted ? 1 : 0, + hasPendingReview: promoted, + pendingReviewBody: "", + isOwnPullRequest: false, + canPushToReview: true, + canWriteToGitHub: true, +}); + +afterEach(() => vi.unstubAllGlobals()); + +describe("useReview promotion", () => { + it("does not resolve a GitHub mutation until the review refetch completes", async () => { + let reviewReads = 0; + let releaseRefetch: (response: Response) => void = () => { + throw new Error("Review refetch gate was not initialized"); + }; + const refetchGate = new Promise((resolve) => { + releaseRefetch = resolve; + }); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review") && method === "GET") { + reviewReads += 1; + return reviewReads === 1 ? jsonResponse(review(false)) : refetchGate; + } + if (url.endsWith("/review/comment") && method === "POST") return jsonResponse({}); + return new Response("not found", { status: 404 }); + }), + ); + const { Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + await waitFor(() => expect(result.current.github).toBe("available")); + + let mutationSettled = false; + const mutation = result.current + .createPendingComment({ + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "Pending comment", + }) + .finally(() => { + mutationSettled = true; + }); + await waitFor(() => expect(reviewReads).toBe(2)); + expect(mutationSettled).toBe(false); + + releaseRefetch(jsonResponse(review(true))); + await act(async () => mutation); + expect(mutationSettled).toBe(true); + await waitFor(() => expect(result.current.pendingCommentCount).toBe(1)); + }); + + it("removes the local overlay even when its post-promotion refresh fails", async () => { + let promoted = false; + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review") && method === "GET") return jsonResponse(review(promoted)); + if (url.endsWith("/comment-threads") && method === "POST") { + return jsonResponse(LOCAL_THREAD); + } + if (url.endsWith("/comment-threads") && method === "GET") { + return promoted ? new Response("offline", { status: 500 }) : jsonResponse([LOCAL_THREAD]); + } + if (url.endsWith("/review/add") && method === "POST") { + promoted = true; + return jsonResponse({}); + } + return new Response("not found", { status: 404 }); + }), + ); + const { Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + await waitFor(() => expect(result.current.github).toBe("available")); + + await act(async () => { + await result.current.createLocalThread({ + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "Promote me", + }); + }); + await waitFor(() => + expect(result.current.threads.map((thread) => thread.id)).toEqual(["THREAD_local"]), + ); + + await act(async () => { + await result.current.addToReview("THREAD_local"); + }); + + await waitFor(() => + expect(result.current.threads.map((thread) => thread.id)).toEqual(["THREAD_remote"]), + ); + }); +}); + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} diff --git a/packages/web/src/lib/__tests__/use-review-reconciliation.test.tsx b/packages/web/src/lib/__tests__/use-review-reconciliation.test.tsx new file mode 100644 index 0000000..6136bce --- /dev/null +++ b/packages/web/src/lib/__tests__/use-review-reconciliation.test.tsx @@ -0,0 +1,178 @@ +// @vitest-environment happy-dom + +import type { Comment, CommentThread } from "@stagereview/types/comments"; +import type { LocalReviewThread, ReviewResponse } from "@stagereview/types/review"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { reviewQueryKey, useReview } from "../use-review"; +import { makeWrapper } from "./fixtures"; + +const localThread = (id: string): CommentThread => ({ + id: `THREAD_${id}`, + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + resolvedAt: null, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + comments: [ + { + id: `COMMENT_${id}`, + body: `${id} comment`, + authorId: "local", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }, + ], +}); + +const FIRST_THREAD = localThread("first"); +const SECOND_THREAD = localThread("second"); +const REPLY: Comment = { + id: "COMMENT_reply", + body: "Reply", + authorId: "local", + createdAt: "2026-01-01T00:01:00Z", + updatedAt: "2026-01-01T00:01:00Z", +}; +const THREAD_WITH_REPLY: CommentThread = { + ...FIRST_THREAD, + comments: [...FIRST_THREAD.comments, REPLY], +}; + +const toReviewThread = (thread: CommentThread): LocalReviewThread => ({ + id: thread.id, + source: "local", + threadNodeId: null, + filePath: thread.filePath, + side: thread.side, + startLine: thread.startLine, + endLine: thread.endLine, + isResolved: thread.resolvedAt !== null, + comments: thread.comments.map((comment) => ({ + id: comment.id, + state: "local", + body: comment.body, + bodyHtml: null, + author: null, + nodeId: null, + htmlUrl: null, + createdAt: comment.createdAt, + })), +}); + +const reviewWith = (threads: CommentThread[]): ReviewResponse => ({ + github: "available", + threads: threads.map(toReviewThread), + pendingComments: [], + pendingCommentCount: 0, + hasPendingReview: false, + pendingReviewBody: "", + isOwnPullRequest: false, + canPushToReview: true, + canWriteToGitHub: true, +}); + +afterEach(() => vi.unstubAllGlobals()); + +describe("useReview reconciliation", () => { + it("does not duplicate a thread or reply already present in a concurrent review result", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review")) return jsonResponse(reviewWith([THREAD_WITH_REPLY])); + if (url.endsWith("/comment-threads") && method === "POST") { + return jsonResponse(THREAD_WITH_REPLY); + } + if (url.endsWith("/replies") && method === "POST") return jsonResponse(REPLY); + return new Response("offline", { status: 500 }); + }), + ); + const { Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + await waitFor(() => expect(result.current.github).toBe("available")); + + await act(async () => { + await result.current.createLocalThread({ + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "first comment", + }); + await result.current.replyLocal({ threadId: FIRST_THREAD.id, body: REPLY.body }); + }); + + expect(result.current.threads).toHaveLength(1); + expect(result.current.threads[0]?.comments).toHaveLength(2); + }); + + it("uses a successful review refetch started after the local mutation", async () => { + let review = reviewWith([]); + let localReadFails = false; + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review")) return jsonResponse(review); + if (url.endsWith("/comment-threads") && method === "POST") { + return jsonResponse(FIRST_THREAD); + } + if (url.endsWith(`/comment-threads/${FIRST_THREAD.id}`) && method === "DELETE") { + return jsonResponse({}); + } + if (url.endsWith("/comment-threads")) { + return localReadFails + ? new Response("offline", { status: 500 }) + : jsonResponse([FIRST_THREAD]); + } + return new Response("not found", { status: 404 }); + }), + ); + const { client, Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + await waitFor(() => expect(result.current.github).toBe("available")); + + await act(async () => { + await result.current.createLocalThread({ + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "first comment", + }); + }); + await waitFor(() => + expect(result.current.threads.map((thread) => thread.id)).toEqual(["THREAD_first"]), + ); + + review = reviewWith([FIRST_THREAD, SECOND_THREAD]); + await act(async () => { + await client.invalidateQueries({ queryKey: reviewQueryKey("run1") }); + }); + + await waitFor(() => + expect(result.current.threads.map((thread) => thread.id)).toEqual([ + "THREAD_first", + "THREAD_second", + ]), + ); + + localReadFails = true; + await act(async () => { + await result.current.deleteLocalThread(FIRST_THREAD.id); + }); + expect(result.current.threads.map((thread) => thread.id)).toEqual(["THREAD_second"]); + }); +}); + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} diff --git a/packages/web/src/lib/__tests__/use-review-run-navigation.test.tsx b/packages/web/src/lib/__tests__/use-review-run-navigation.test.tsx new file mode 100644 index 0000000..a6d0a8f --- /dev/null +++ b/packages/web/src/lib/__tests__/use-review-run-navigation.test.tsx @@ -0,0 +1,104 @@ +// @vitest-environment happy-dom + +import type { CommentThread } from "@stagereview/types/comments"; +import type { ReviewResponse } from "@stagereview/types/review"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { reviewQueryKey, useReview } from "../use-review"; +import { makeWrapper } from "./fixtures"; + +const CREATED_THREAD: CommentThread = { + id: "THREAD_first", + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + resolvedAt: null, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + comments: [ + { + id: "COMMENT_first", + body: "First comment", + authorId: "local", + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }, + ], +}; + +const EMPTY_REVIEW: ReviewResponse = { + github: "available", + threads: [], + pendingComments: [], + pendingCommentCount: 0, + hasPendingReview: false, + pendingReviewBody: "", + isOwnPullRequest: false, + canPushToReview: true, + canWriteToGitHub: true, +}; + +afterEach(() => vi.unstubAllGlobals()); + +describe("useReview run navigation", () => { + it("reconciles a late mutation only into the run that originated it", async () => { + let resolveCreate: ((response: Response) => void) | undefined; + let markCreateStarted: (() => void) | undefined; + const createStarted = new Promise((resolve) => { + markCreateStarted = resolve; + }); + const createResponse = new Promise((resolve) => { + resolveCreate = resolve; + }); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review")) return jsonResponse(EMPTY_REVIEW); + if (url === "/api/runs/run-a/comment-threads" && method === "POST") { + markCreateStarted?.(); + return createResponse; + } + return new Response("not found", { status: 404 }); + }), + ); + const { client, Wrapper } = makeWrapper(); + const { result, rerender } = renderHook(({ runId }) => useReview(runId), { + wrapper: Wrapper, + initialProps: { runId: "run-a" }, + }); + await waitFor(() => expect(result.current.github).toBe("available")); + + let pendingCreate: Promise | undefined; + await act(async () => { + pendingCreate = result.current.createLocalThread({ + filePath: "src/foo.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "First comment", + }); + await createStarted; + }); + + rerender({ runId: "run-b" }); + await waitFor(() => expect(result.current.github).toBe("available")); + await act(async () => { + resolveCreate?.(jsonResponse(CREATED_THREAD)); + await pendingCreate; + }); + + expect(result.current.threads).toHaveLength(0); + const runACache = client.getQueryData<{ review: ReviewResponse }>(reviewQueryKey("run-a")); + expect(runACache?.review.threads.map((thread) => thread.id)).toEqual(["THREAD_first"]); + }); +}); + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} diff --git a/packages/web/src/lib/__tests__/use-review.test.tsx b/packages/web/src/lib/__tests__/use-review.test.tsx new file mode 100644 index 0000000..0902b9d --- /dev/null +++ b/packages/web/src/lib/__tests__/use-review.test.tsx @@ -0,0 +1,199 @@ +// @vitest-environment happy-dom + +import type { CommentThread } from "@stagereview/types/comments"; +import type { ReviewResponse, ReviewThread } from "@stagereview/types/review"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useReview } from "../use-review"; +import { makeWrapper } from "./fixtures"; + +const GITHUB_THREAD: ReviewThread = { + id: "THREAD_github", + source: "github", + threadNodeId: "THREAD_github", + filePath: "src/github.ts", + side: "additions", + startLine: 2, + endLine: 2, + isResolved: false, + comments: [ + { + id: "COMMENT_github", + state: "submitted", + body: "GitHub comment", + bodyHtml: "

GitHub comment

", + author: { login: "octocat", avatarUrl: null }, + nodeId: "COMMENT_github", + htmlUrl: "https://github.com/owner/repo/pull/1#discussion_r1", + createdAt: "2026-01-01T00:00:00Z", + }, + ], +}; + +const AVAILABLE_REVIEW: ReviewResponse = { + github: "available", + threads: [GITHUB_THREAD], + pendingComments: [], + pendingCommentCount: 0, + hasPendingReview: false, + pendingReviewBody: "", + isOwnPullRequest: false, + canPushToReview: true, + canWriteToGitHub: true, +}; + +const OFFLINE_REVIEW: ReviewResponse = { + github: "offline", + threads: [], + pendingComments: [], + pendingCommentCount: 0, + hasPendingReview: false, + pendingReviewBody: "", + isOwnPullRequest: false, + canPushToReview: false, + canWriteToGitHub: false, +}; + +const LOCAL_THREAD: CommentThread = { + id: "THREAD_local", + filePath: "src/local.ts", + side: "additions", + startLine: 3, + endLine: 3, + resolvedAt: null, + createdAt: "2026-01-02T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + comments: [ + { + id: "COMMENT_local", + body: "Local comment", + authorId: "local", + createdAt: "2026-01-02T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + }, + ], +}; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("useReview", () => { + it("refreshes local threads without replacing cached GitHub review data", async () => { + let reviewReads = 0; + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review") && method === "GET") { + reviewReads += 1; + return jsonResponse(reviewReads === 1 ? AVAILABLE_REVIEW : OFFLINE_REVIEW); + } + if (url.endsWith("/comment-threads") && method === "POST") { + return jsonResponse(LOCAL_THREAD); + } + if (url.endsWith("/comment-threads") && method === "GET") { + return jsonResponse([LOCAL_THREAD]); + } + return new Response("not found", { status: 404 }); + }), + ); + const { Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + await waitFor(() => expect(result.current.github).toBe("available")); + + await act(async () => { + await result.current.createLocalThread({ + filePath: "src/local.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "Local comment", + }); + }); + + await waitFor(() => expect(result.current.threads).toHaveLength(2)); + expect(result.current.github).toBe("available"); + expect(result.current.threads.map((thread) => thread.id)).toEqual([ + "THREAD_local", + "THREAD_github", + ]); + expect(reviewReads).toBe(1); + }); + + it("does not reject a successful write when the local refresh fails", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + if (url.endsWith("/review")) return jsonResponse(AVAILABLE_REVIEW); + if ((init?.method ?? "GET") === "POST") return jsonResponse(LOCAL_THREAD); + return new Response("offline", { status: 500 }); + }), + ); + const { Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + await waitFor(() => expect(result.current.github).toBe("available")); + + await expect( + result.current.createLocalThread({ + filePath: "src/local.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "Local comment", + }), + ).resolves.toBeDefined(); + }); + + it("keeps a local write visible when the initial review fetch finishes later", async () => { + let resolveReview: ((response: Response) => void) | undefined; + const pendingReview = new Promise((resolve) => { + resolveReview = resolve; + }); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const method = init?.method ?? "GET"; + if (url.endsWith("/review")) return pendingReview; + if (url.endsWith("/comment-threads") && method === "POST") { + return jsonResponse(LOCAL_THREAD); + } + if (url.endsWith("/comment-threads")) return jsonResponse([LOCAL_THREAD]); + return new Response("not found", { status: 404 }); + }), + ); + const { Wrapper } = makeWrapper(); + const { result } = renderHook(() => useReview("run1"), { wrapper: Wrapper }); + + await act(async () => { + await result.current.createLocalThread({ + filePath: "src/local.ts", + side: "additions", + startLine: 3, + endLine: 3, + body: "Local comment", + }); + }); + await waitFor(() => + expect(result.current.threads.map((thread) => thread.id)).toEqual(["THREAD_local"]), + ); + + if (!resolveReview) throw new Error("Review response gate was not initialized"); + resolveReview(jsonResponse(AVAILABLE_REVIEW)); + await waitFor(() => expect(result.current.github).toBe("available")); + expect(result.current.threads.map((thread) => thread.id)).toEqual([ + "THREAD_local", + "THREAD_github", + ]); + }); +}); + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} diff --git a/packages/web/src/lib/comment-drafts.ts b/packages/web/src/lib/comment-drafts.ts index 210672a..05a0e3d 100644 --- a/packages/web/src/lib/comment-drafts.ts +++ b/packages/web/src/lib/comment-drafts.ts @@ -1,6 +1,6 @@ import type { DiffLineAnnotation } from "@pierre/diffs"; import type { DiffSide } from "@/lib/diff-types"; -import type { CommentThread } from "@/lib/use-comment-threads"; +import type { ReviewThread as CommentThread } from "@/lib/use-review"; /** An in-progress comment the reviewer is composing, anchored to a line range. */ export interface CommentDraft { @@ -12,6 +12,8 @@ export interface CommentDraft { /** A draft plus the last submit error for its composer (null while clean). */ export interface DraftState extends CommentDraft { error: string | null; + /** The user's current destination preference, initialized from availability when opened. */ + toGitHub: boolean; } /** @@ -60,15 +62,41 @@ export function findDraftAt( return drafts.find((draft) => isSameAnchor(draft, side, endLine)); } +/** GitHub can appear later, but only drafts opened with it available default to sending there. */ +export function getDraftGitHubDestination( + draft: DraftState, + canPushToReview: boolean, +): { available: boolean; defaultOn: boolean } { + return { + available: canPushToReview, + defaultOn: canPushToReview && draft.toGitHub, + }; +} + +export function setDraftGitHubPreference( + drafts: readonly DraftState[], + side: DiffSide, + endLine: number, + toGitHub: boolean, +): DraftState[] { + return drafts.map((draft) => + isSameAnchor(draft, side, endLine) ? { ...draft, toGitHub } : draft, + ); +} + /** * Opens a composer for the anchor. Since a row holds at most one composer, re-opening * the same `(side, endLine)` doesn't duplicate it — instead it adopts the new range's * `startLine` (a re-drag that shares the end line but widens/narrows the span) and * clears any stale submit error. */ -export function upsertDraft(drafts: readonly DraftState[], anchor: CommentDraft): DraftState[] { +export function upsertDraft( + drafts: readonly DraftState[], + anchor: CommentDraft, + toGitHub: boolean, +): DraftState[] { if (!findDraftAt(drafts, anchor.side, anchor.endLine)) { - return [...drafts, { ...anchor, error: null }]; + return [...drafts, { ...anchor, error: null, toGitHub }]; } return drafts.map((draft) => isSameAnchor(draft, anchor.side, anchor.endLine) diff --git a/packages/web/src/lib/comment-threads-context.tsx b/packages/web/src/lib/comment-threads-context.tsx deleted file mode 100644 index 42d2862..0000000 --- a/packages/web/src/lib/comment-threads-context.tsx +++ /dev/null @@ -1,48 +0,0 @@ -import { createContext, type ReactNode, useContext, useEffect } from "react"; -import { toast } from "@/components/ui/sonner"; -import { type UseCommentThreadsResult, useCommentThreads } from "./use-comment-threads"; - -const CommentThreadsContext = createContext(null); - -const LOAD_ERROR_TOAST_ID = "comment-threads-error"; - -/** - * Provides the run's comment threads + mutations to the diff tree without - * prop-drilling through FileDiffList. Mounted once at the run layout. - */ -export function CommentThreadsProvider({ - runId, - children, -}: { - runId: string; - children: ReactNode; -}) { - const value = useCommentThreads(runId); - - // A failed threads fetch is otherwise indistinguishable from "no comments" — - // the diff still renders, but the overlay is silently empty. Surface it as a - // toast (React Query only sets `error` once its retries are exhausted), and - // dismiss it once a later fetch recovers so a stale message doesn't linger. - useEffect(() => { - if (!value.error) { - toast.dismiss(LOAD_ERROR_TOAST_ID); - return; - } - // Stable id so a re-fire (StrictMode double-mount, remount with a cached error, - // refetch failing with a new error reference) updates one toast instead of stacking. - toast.error("Couldn't load comments", { - id: LOAD_ERROR_TOAST_ID, - description: value.error instanceof Error ? value.error.message : undefined, - }); - }, [value.error]); - - return {children}; -} - -export function useCommentThreadsContext(): UseCommentThreadsResult { - const ctx = useContext(CommentThreadsContext); - if (!ctx) { - throw new Error("useCommentThreadsContext must be used within a CommentThreadsProvider"); - } - return ctx; -} diff --git a/packages/web/src/lib/review-context.tsx b/packages/web/src/lib/review-context.tsx new file mode 100644 index 0000000..8dbdcf8 --- /dev/null +++ b/packages/web/src/lib/review-context.tsx @@ -0,0 +1,37 @@ +import { createContext, type ReactNode, useContext, useEffect } from "react"; +import { toast } from "@/components/ui/sonner"; +import { type UseReviewResult, useReview } from "./use-review"; + +const ReviewContext = createContext(null); + +const LOAD_ERROR_TOAST_ID = "review-error"; + +/** + * Provides the run's merged review (local + GitHub threads) and its mutations to + * the diff tree without prop-drilling. Mounted once at the run layout. + */ +export function ReviewProvider({ runId, children }: { runId: string; children: ReactNode }) { + const value = useReview(runId); + + // A failed review fetch is otherwise indistinguishable from "no comments" — the + // diff still renders but the overlay is silently empty. Surface it as a toast, + // and dismiss it once a later fetch recovers so a stale message doesn't linger. + useEffect(() => { + if (!value.error) { + toast.dismiss(LOAD_ERROR_TOAST_ID); + return; + } + toast.error("Couldn't load review comments", { + id: LOAD_ERROR_TOAST_ID, + description: value.error instanceof Error ? value.error.message : undefined, + }); + }, [value.error]); + + return {children}; +} + +export function useReviewContext(): UseReviewResult { + const ctx = useContext(ReviewContext); + if (!ctx) throw new Error("useReviewContext must be used within a ReviewProvider"); + return ctx; +} diff --git a/packages/web/src/lib/review-submission.ts b/packages/web/src/lib/review-submission.ts new file mode 100644 index 0000000..2fefa42 --- /dev/null +++ b/packages/web/src/lib/review-submission.ts @@ -0,0 +1,17 @@ +import { REVIEW_EVENT, type ReviewEvent } from "@stagereview/types/review"; + +interface ReviewSubmissionInput { + event: ReviewEvent; + body: string; + pendingCommentCount: number; + isSubmitting: boolean; +} + +export function canSubmitReview(input: ReviewSubmissionInput): boolean { + if (input.isSubmitting) return false; + if (input.event === REVIEW_EVENT.REQUEST_CHANGES) return input.body.trim().length > 0; + if (input.event === REVIEW_EVENT.COMMENT) { + return input.body.trim().length > 0 || input.pendingCommentCount > 0; + } + return true; +} diff --git a/packages/web/src/lib/use-comment-threads.ts b/packages/web/src/lib/use-comment-threads.ts deleted file mode 100644 index 972e3f9..0000000 --- a/packages/web/src/lib/use-comment-threads.ts +++ /dev/null @@ -1,159 +0,0 @@ -import { - type Comment, - type CommentThread, - CommentThreadsResponseSchema, - type CreateCommentThreadBody, -} from "@stagereview/types/comments"; -import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { useMemo } from "react"; -import { jsonFetch } from "./use-view-state"; - -export type { Comment, CommentThread, CreateCommentThreadBody }; - -const COMMENT_THREADS_ROOT = "comment-threads"; - -export function commentThreadsQueryKey(runId: string): readonly unknown[] { - return [COMMENT_THREADS_ROOT, runId]; -} - -async function fetchCommentThreads(runId: string): Promise { - // Parse at the boundary so server-side schema drift surfaces as a query error - // here, not as a render crash deeper in the diff. - const raw = await jsonFetch(`/api/runs/${encodeURIComponent(runId)}/comment-threads`); - return CommentThreadsResponseSchema.parse(raw); -} - -const jsonRequest = (method: string, body?: unknown): RequestInit => ({ - method, - headers: { "Content-Type": "application/json" }, - body: body === undefined ? undefined : JSON.stringify(body), -}); - -export interface UseCommentThreadsResult { - threads: CommentThread[]; - /** Stable reference; rebuilt only when the underlying query data changes. */ - threadsByFile: ReadonlyMap; - isLoading: boolean; - error: unknown; - createThread: (input: CreateCommentThreadBody) => Promise; - replyToThread: (input: { threadId: string; body: string }) => Promise; - setThreadResolved: (input: { threadId: string; resolved: boolean }) => Promise; - editComment: (input: { commentId: string; body: string }) => Promise; - deleteThread: (threadId: string) => Promise; - deleteComment: (commentId: string) => Promise; -} - -/** - * Loads the comment threads anchored to a run's diff scope and exposes the - * thread/comment mutations. The local server commits synchronously, so each - * mutation simply invalidates the query rather than maintaining optimistic - * caches — the refetch round-trip is effectively instant. - */ -export function useCommentThreads(runId: string): UseCommentThreadsResult { - const queryClient = useQueryClient(); - const queryKey = useMemo(() => commentThreadsQueryKey(runId), [runId]); - - const { data, isLoading, error } = useQuery({ - queryKey, - queryFn: () => fetchCommentThreads(runId), - enabled: runId !== "", - }); - - const threads = useMemo(() => data ?? [], [data]); - const threadsByFile = useMemo(() => groupByFile(threads), [threads]); - - const invalidate = () => queryClient.invalidateQueries({ queryKey }); - - const createMutation = useMutation({ - mutationFn: (input: CreateCommentThreadBody) => - jsonFetch( - `/api/runs/${encodeURIComponent(runId)}/comment-threads`, - jsonRequest("POST", input), - ), - onSuccess: invalidate, - }); - - const replyMutation = useMutation({ - mutationFn: async ({ threadId, body }: { threadId: string; body: string }) => { - await jsonFetch( - `/api/comment-threads/${encodeURIComponent(threadId)}/replies`, - jsonRequest("POST", { body }), - ); - }, - onSuccess: invalidate, - }); - - const resolveMutation = useMutation({ - mutationFn: async ({ threadId, resolved }: { threadId: string; resolved: boolean }) => { - await jsonFetch( - `/api/comment-threads/${encodeURIComponent(threadId)}`, - jsonRequest("PATCH", { resolved }), - ); - }, - onSuccess: invalidate, - }); - - const editMutation = useMutation({ - mutationFn: async ({ commentId, body }: { commentId: string; body: string }) => { - await jsonFetch( - `/api/comments/${encodeURIComponent(commentId)}`, - jsonRequest("PATCH", { body }), - ); - }, - onSuccess: invalidate, - }); - - const deleteThreadMutation = useMutation({ - mutationFn: async (threadId: string) => { - await jsonFetch( - `/api/comment-threads/${encodeURIComponent(threadId)}`, - jsonRequest("DELETE"), - ); - }, - onSuccess: invalidate, - }); - - const deleteCommentMutation = useMutation({ - mutationFn: async (commentId: string) => { - await jsonFetch(`/api/comments/${encodeURIComponent(commentId)}`, jsonRequest("DELETE")); - }, - onSuccess: invalidate, - }); - - return useMemo( - () => ({ - threads, - threadsByFile, - isLoading, - error, - createThread: createMutation.mutateAsync, - replyToThread: replyMutation.mutateAsync, - setThreadResolved: resolveMutation.mutateAsync, - editComment: editMutation.mutateAsync, - deleteThread: deleteThreadMutation.mutateAsync, - deleteComment: deleteCommentMutation.mutateAsync, - }), - [ - threads, - threadsByFile, - isLoading, - error, - createMutation.mutateAsync, - replyMutation.mutateAsync, - resolveMutation.mutateAsync, - editMutation.mutateAsync, - deleteThreadMutation.mutateAsync, - deleteCommentMutation.mutateAsync, - ], - ); -} - -function groupByFile(threads: CommentThread[]): ReadonlyMap { - const map = new Map(); - for (const thread of threads) { - const list = map.get(thread.filePath); - if (list) list.push(thread); - else map.set(thread.filePath, [thread]); - } - return map; -} diff --git a/packages/web/src/lib/use-review.ts b/packages/web/src/lib/use-review.ts new file mode 100644 index 0000000..bcca017 --- /dev/null +++ b/packages/web/src/lib/use-review.ts @@ -0,0 +1,418 @@ +import { + CommentSchema, + CommentThreadSchema, + CommentThreadsResponseSchema, + type CreateCommentThreadBody, + type Comment as LocalComment, + type CommentThread as LocalCommentThread, +} from "@stagereview/types/comments"; +import { + COMMENT_STATE, + GITHUB_REVIEW_STATUS, + type GitHubReviewStatus, + type LocalReviewComment, + type LocalReviewThread, + type PendingReviewComment, + type ReviewEvent, + type ReviewResponse, + ReviewResponseSchema, + type ReviewThread, + THREAD_SOURCE, +} from "@stagereview/types/review"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useMemo, useRef, useState } from "react"; +import { jsonFetch } from "./use-view-state"; + +export type { CreateCommentThreadBody, GitHubReviewStatus, ReviewEvent, ReviewThread }; +export { GITHUB_REVIEW_STATUS }; + +const REVIEW_ROOT = "review"; + +interface ReviewQueryData { + generation: number; + review: ReviewResponse; +} + +interface ReviewMutationOrigin { + runId: string; + queryKey: readonly unknown[]; +} + +export function reviewQueryKey(runId: string): readonly unknown[] { + return [REVIEW_ROOT, runId]; +} + +async function fetchReview(runId: string): Promise { + const raw = await jsonFetch(`/api/runs/${encodeURIComponent(runId)}/review`); + return ReviewResponseSchema.parse(raw); +} + +async function fetchLocalThreads(runId: string): Promise { + const raw = await jsonFetch(`/api/runs/${encodeURIComponent(runId)}/comment-threads`); + return CommentThreadsResponseSchema.parse(raw).map(toReviewThread); +} + +function toReviewComment(comment: LocalComment): LocalReviewComment { + return { + id: comment.id, + state: COMMENT_STATE.LOCAL, + body: comment.body, + bodyHtml: null, + author: null, + nodeId: null, + htmlUrl: null, + createdAt: comment.createdAt, + }; +} + +function toReviewThread(thread: LocalCommentThread): LocalReviewThread { + 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: thread.comments.map(toReviewComment), + }; +} + +const jsonRequest = (method: string, body?: unknown): RequestInit => ({ + method, + headers: { "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), +}); + +export interface UseReviewResult { + threads: ReviewThread[]; + threadsByFile: ReadonlyMap; + github: GitHubReviewStatus; + pendingComments: PendingReviewComment[]; + pendingCommentCount: number; + hasPendingReview: boolean; + pendingReviewBody: string; + isOwnPullRequest: boolean; + canPushToReview: boolean; + canWriteToGitHub: boolean; + isLoading: boolean; + error: unknown; + // Local comments (CLI-only, work offline). + createLocalThread: (input: CreateCommentThreadBody) => Promise; + // Create a comment directly on the PR as a pending review comment. + createPendingComment: (input: CreateCommentThreadBody) => Promise; + replyLocal: (input: { threadId: string; body: string }) => Promise; + editLocalComment: (input: { commentId: string; body: string }) => Promise; + deleteLocalThread: (threadId: string) => Promise; + deleteLocalComment: (commentId: string) => Promise; + resolveLocalThread: (input: { threadId: string; resolved: boolean }) => Promise; + // GitHub review actions. + addToReview: (localThreadId: string) => Promise; + submitReview: (input: { event: ReviewEvent; body: string }) => Promise; + discardReview: () => Promise; + replyGitHub: (input: { threadNodeId: string; body: string; pending: boolean }) => Promise; + editGitHubComment: (input: { nodeId: string; body: string }) => Promise; + deleteGitHubComment: (nodeId: string) => Promise; + resolveGitHub: (input: { threadNodeId: string; resolved: boolean }) => Promise; +} + +/** + * The run's merged review — local threads plus the PR's live pending/submitted + * GitHub threads — and the mutations that act on each. Local mutations refresh only + * the local endpoint and merge those rows into the cache, so a transient GitHub + * outage cannot evict cached PR threads. GitHub mutations refetch the merged review. + */ +export function useReview(runId: string): UseReviewResult { + const queryClient = useQueryClient(); + const queryKey = useMemo(() => reviewQueryKey(runId), [runId]); + const [localOverlay, setLocalOverlay] = useState<{ + runId: string; + threads: LocalReviewThread[]; + reviewGeneration: number; + } | null>(null); + const localRefreshGeneration = useRef(0); + const reviewRequestGeneration = useRef(0); + + const { + data: queryData, + isLoading, + error, + } = useQuery({ + queryKey, + queryFn: async () => { + const generation = ++reviewRequestGeneration.current; + return { generation, review: await fetchReview(runId) }; + }, + enabled: runId !== "", + }); + + const data = queryData?.review; + const completedReviewGeneration = queryData?.generation ?? 0; + const refreshedLocalThreads = + localOverlay?.runId === runId && localOverlay.reviewGeneration >= completedReviewGeneration + ? localOverlay.threads + : null; + const threads = useMemo(() => { + const reviewThreads = data?.threads ?? []; + if (refreshedLocalThreads === null) return reviewThreads; + return [ + ...refreshedLocalThreads, + ...reviewThreads.filter((thread) => thread.source !== THREAD_SOURCE.LOCAL), + ]; + }, [data, refreshedLocalThreads]); + const pendingComments = useMemo(() => data?.pendingComments ?? [], [data]); + const threadsByFile = useMemo(() => groupByFile(threads), [threads]); + const updateLocalThreads = (update: (threads: LocalReviewThread[]) => LocalReviewThread[]) => { + const generation = ++localRefreshGeneration.current; + const reviewGeneration = reviewRequestGeneration.current; + setLocalOverlay((current) => { + const cachedReview = queryClient.getQueryData(queryKey); + const localThreads = + current?.runId === runId && current.reviewGeneration >= (cachedReview?.generation ?? 0) + ? current.threads + : (cachedReview?.review.threads.filter( + (thread): thread is LocalReviewThread => thread.source === THREAD_SOURCE.LOCAL, + ) ?? []); + return { runId, threads: update(localThreads), reviewGeneration }; + }); + // The mutation response above is authoritative. This best-effort read fills + // any rows not yet cached, but only the newest mutation may reconcile state. + void fetchLocalThreads(runId) + .then((localThreads) => { + if (localRefreshGeneration.current !== generation) return; + setLocalOverlay({ runId, threads: localThreads, reviewGeneration }); + }) + .catch(() => {}); + }; + const captureMutationOrigin = (): ReviewMutationOrigin => ({ runId, queryKey }); + const updateLocalThreadsForOrigin = ( + origin: ReviewMutationOrigin, + update: (threads: LocalReviewThread[]) => LocalReviewThread[], + ) => { + if (origin.runId === runId) { + updateLocalThreads(update); + return; + } + // A mutation may settle after this hook navigates to another run. Reconcile + // only the cache entry that originated the request; never put A's rows into + // B's component-local overlay. + queryClient.setQueryData(origin.queryKey, (current) => { + if (!current) return current; + const localThreads = current.review.threads.filter( + (thread): thread is LocalReviewThread => thread.source === THREAD_SOURCE.LOCAL, + ); + return { + ...current, + review: { + ...current.review, + threads: [ + ...update(localThreads), + ...current.review.threads.filter((thread) => thread.source !== THREAD_SOURCE.LOCAL), + ], + }, + }; + }); + void queryClient.invalidateQueries({ queryKey: origin.queryKey, refetchType: "none" }); + }; + const removePromotedLocalThread = (origin: ReviewMutationOrigin, localThreadId: string) => { + updateLocalThreadsForOrigin(origin, (threads) => + threads.filter((thread) => thread.id !== localThreadId), + ); + }; + // GitHub-affecting actions (submit/resolve/reply/promote) change PR-level state — + // reviewer decisions, the merge button — that lives behind separate, infinitely- + // stale query keys. Refresh those too so the PR header doesn't go stale until reload. + const invalidateGitHub = async (origin: ReviewMutationOrigin) => { + await Promise.all([ + queryClient.invalidateQueries({ queryKey: origin.queryKey }), + queryClient.invalidateQueries({ queryKey: ["pull-request", origin.runId] }), + queryClient.invalidateQueries({ queryKey: ["pull-request-reviews", origin.runId] }), + queryClient.invalidateQueries({ queryKey: ["pull-request-merge-status", origin.runId] }), + queryClient.invalidateQueries({ queryKey: ["pull-request-checks", origin.runId] }), + ]); + }; + + const runPath = (suffix: string) => `/api/runs/${encodeURIComponent(runId)}${suffix}`; + + const m = { + createLocalThread: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: async (input: CreateCommentThreadBody) => + CommentThreadSchema.parse( + await jsonFetch(runPath("/comment-threads"), jsonRequest("POST", input)), + ), + onSuccess: (thread, _input, origin) => + updateLocalThreadsForOrigin(origin, (threads) => + threads.some((current) => current.id === thread.id) + ? threads + : [...threads, toReviewThread(thread)], + ), + }), + createPendingComment: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (input: CreateCommentThreadBody) => + jsonFetch(runPath("/review/comment"), jsonRequest("POST", input)), + onSuccess: (_data, _input, origin) => invalidateGitHub(origin), + }), + replyLocal: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: async ({ threadId, body }: { threadId: string; body: string }) => + CommentSchema.parse( + await jsonFetch( + `/api/comment-threads/${encodeURIComponent(threadId)}/replies`, + jsonRequest("POST", { body }), + ), + ), + onSuccess: (comment, { threadId }, origin) => + updateLocalThreadsForOrigin(origin, (threads) => + threads.map((thread) => { + if ( + thread.id !== threadId || + thread.comments.some((current) => current.id === comment.id) + ) { + return thread; + } + return { ...thread, comments: [...thread.comments, toReviewComment(comment)] }; + }), + ), + }), + editLocalComment: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: async ({ commentId, body }: { commentId: string; body: string }) => + CommentSchema.parse( + await jsonFetch( + `/api/comments/${encodeURIComponent(commentId)}`, + jsonRequest("PATCH", { body }), + ), + ), + onSuccess: (comment, _input, origin) => + updateLocalThreadsForOrigin(origin, (threads) => + threads.map((thread) => ({ + ...thread, + comments: thread.comments.map((current) => + current.id === comment.id ? toReviewComment(comment) : current, + ), + })), + ), + }), + deleteLocalThread: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (threadId: string) => + jsonFetch(`/api/comment-threads/${encodeURIComponent(threadId)}`, jsonRequest("DELETE")), + onSuccess: (_data, threadId, origin) => + updateLocalThreadsForOrigin(origin, (threads) => + threads.filter((thread) => thread.id !== threadId), + ), + }), + deleteLocalComment: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (commentId: string) => + jsonFetch(`/api/comments/${encodeURIComponent(commentId)}`, jsonRequest("DELETE")), + onSuccess: (_data, commentId, origin) => + updateLocalThreadsForOrigin(origin, (threads) => + threads.flatMap((thread) => { + const comments = thread.comments.filter((comment) => comment.id !== commentId); + return comments.length === 0 ? [] : [{ ...thread, comments }]; + }), + ), + }), + resolveLocalThread: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: async ({ threadId, resolved }: { threadId: string; resolved: boolean }) => + CommentThreadSchema.parse( + await jsonFetch( + `/api/comment-threads/${encodeURIComponent(threadId)}`, + jsonRequest("PATCH", { resolved }), + ), + ), + onSuccess: (updated, _input, origin) => + updateLocalThreadsForOrigin(origin, (threads) => + threads.map((thread) => (thread.id === updated.id ? toReviewThread(updated) : thread)), + ), + }), + addToReview: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (localThreadId: string) => + jsonFetch(runPath("/review/add"), jsonRequest("POST", { localThreadId })), + onSuccess: (_data, localThreadId, origin) => { + removePromotedLocalThread(origin, localThreadId); + return invalidateGitHub(origin); + }, + }), + submitReview: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (input: { event: ReviewEvent; body: string }) => + jsonFetch(runPath("/review/submit"), jsonRequest("POST", input)), + onSuccess: (_data, _input, origin) => invalidateGitHub(origin), + }), + discardReview: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: () => jsonFetch(runPath("/review/discard"), jsonRequest("POST")), + onSuccess: (_data, _input, origin) => invalidateGitHub(origin), + }), + replyGitHub: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (input: { threadNodeId: string; body: string; pending: boolean }) => + jsonFetch(runPath("/review/reply"), jsonRequest("POST", input)), + onSuccess: (_data, _input, origin) => invalidateGitHub(origin), + }), + editGitHubComment: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (input: { nodeId: string; body: string }) => + jsonFetch(runPath("/review/comment/edit"), jsonRequest("POST", input)), + onSuccess: (_data, _input, origin) => invalidateGitHub(origin), + }), + deleteGitHubComment: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (nodeId: string) => + jsonFetch(runPath("/review/comment/delete"), jsonRequest("POST", { nodeId })), + onSuccess: (_data, _input, origin) => invalidateGitHub(origin), + }), + resolveGitHub: useMutation({ + onMutate: captureMutationOrigin, + mutationFn: (input: { threadNodeId: string; resolved: boolean }) => + jsonFetch(runPath("/review/resolve"), jsonRequest("POST", input)), + onSuccess: (_data, _input, origin) => invalidateGitHub(origin), + }), + }; + + return { + threads, + threadsByFile, + github: data?.github ?? GITHUB_REVIEW_STATUS.NONE, + pendingComments, + pendingCommentCount: data?.pendingCommentCount ?? 0, + hasPendingReview: data?.hasPendingReview ?? false, + pendingReviewBody: data?.pendingReviewBody ?? "", + isOwnPullRequest: data?.isOwnPullRequest ?? false, + canPushToReview: data?.canPushToReview ?? false, + canWriteToGitHub: data?.canWriteToGitHub ?? false, + isLoading, + error, + createLocalThread: m.createLocalThread.mutateAsync, + createPendingComment: async (i) => void (await m.createPendingComment.mutateAsync(i)), + replyLocal: async (i) => void (await m.replyLocal.mutateAsync(i)), + editLocalComment: async (i) => void (await m.editLocalComment.mutateAsync(i)), + deleteLocalThread: async (id) => void (await m.deleteLocalThread.mutateAsync(id)), + deleteLocalComment: async (id) => void (await m.deleteLocalComment.mutateAsync(id)), + resolveLocalThread: async (i) => void (await m.resolveLocalThread.mutateAsync(i)), + addToReview: async (id) => void (await m.addToReview.mutateAsync(id)), + submitReview: async (i) => void (await m.submitReview.mutateAsync(i)), + discardReview: async () => void (await m.discardReview.mutateAsync()), + replyGitHub: async (i) => void (await m.replyGitHub.mutateAsync(i)), + editGitHubComment: async (i) => void (await m.editGitHubComment.mutateAsync(i)), + deleteGitHubComment: async (id) => void (await m.deleteGitHubComment.mutateAsync(id)), + resolveGitHub: async (i) => void (await m.resolveGitHub.mutateAsync(i)), + }; +} + +function groupByFile(threads: ReviewThread[]): ReadonlyMap { + const map = new Map(); + for (const thread of threads) { + const list = map.get(thread.filePath); + if (list) list.push(thread); + else map.set(thread.filePath, [thread]); + } + return map; +} diff --git a/packages/web/src/lib/use-view-state.ts b/packages/web/src/lib/use-view-state.ts index fa4e402..87633a1 100644 --- a/packages/web/src/lib/use-view-state.ts +++ b/packages/web/src/lib/use-view-state.ts @@ -26,15 +26,33 @@ 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(); if (!res.ok) { - throw new Error(`${init?.method ?? "GET"} ${url} failed: ${res.status}`); + // Surface the server's `{ error }` message verbatim (the review/write paths + // carry actionable reasons), but tolerate a non-JSON error body (e.g. an HTML + // proxy error) — fall back to the status code rather than throwing SyntaxError. + throw new Error( + errorBodyMessage(text) ?? `${init?.method ?? "GET"} ${url} failed: ${res.status}`, + ); } - // 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; } +function errorBodyMessage(text: string): string | null { + if (!text) return null; + try { + const parsed: unknown = JSON.parse(text); + if (typeof parsed === "object" && parsed !== null && "error" in parsed) { + return String(parsed.error); + } + } catch { + // Non-JSON error body — let the caller fall back to the status code. + } + return null; +} + async function fetchViewState(runId: string): Promise { // Parse at the boundary so a server-side schema drift surfaces as a query // error here, not as a render crash deeper in the component tree. diff --git a/packages/web/src/routes/chapter-detail-page.tsx b/packages/web/src/routes/chapter-detail-page.tsx index 1840f05..d335701 100644 --- a/packages/web/src/routes/chapter-detail-page.tsx +++ b/packages/web/src/routes/chapter-detail-page.tsx @@ -53,6 +53,7 @@ export function ChapterDetailPage({ runId, chapterNumber }: ChapterDetailPagePro return (
+ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b5e6ea7..ecfdd4f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -53,6 +53,9 @@ importers: parse-diff: specifier: ^0.11.1 version: 0.11.1 + proper-lockfile: + specifier: ^4.1.2 + version: 4.1.2 zod: specifier: ^4.3.6 version: 4.4.2 @@ -66,6 +69,9 @@ importers: '@types/node': specifier: ^25.6.0 version: 25.6.0 + '@types/proper-lockfile': + specifier: ^4.1.4 + version: 4.1.4 drizzle-kit: specifier: ^0.31.10 version: 0.31.10 @@ -2253,6 +2259,9 @@ packages: '@types/node@25.6.0': resolution: {integrity: sha512-+qIYRKdNYJwY3vRCZMdJbPLJAtGjQBudzZzdzwQYkEPQd+PJGixUL5QfvCLDaULoLv+RhT3LDkwEfKaAkgSmNQ==} + '@types/proper-lockfile@4.1.4': + resolution: {integrity: sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ==} + '@types/react-dom@19.2.3': resolution: {integrity: sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==} peerDependencies: @@ -2261,6 +2270,9 @@ packages: '@types/react@19.2.14': resolution: {integrity: sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==} + '@types/retry@0.12.5': + resolution: {integrity: sha512-3xSjTp3v03X/lSQLkczaN9UIEwJMoMCA1+Nb5HfbJEQWogdeQIyVtTvxPXDQjZ5zws8rFQfVfRdz03ARihPJgw==} + '@types/trusted-types@2.0.7': resolution: {integrity: sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==} @@ -2278,6 +2290,7 @@ packages: '@ungap/structured-clone@1.3.0': resolution: {integrity: sha512-WmoN8qaIAo7WTYWbAZuG8PYEhn5fkz7dZrqTBZ7dtt//lL2Gwms1IcnQ5yHqjDfX8Ft5j4YzDM23f87zBfDe9g==} + deprecated: Potential CWE-502 - Update to 1.3.1 or higher '@upsetjs/venn.js@2.0.0': resolution: {integrity: sha512-WbBhLrooyePuQ1VZxrJjtLvTc4NVfpOyKx0sKqioq9bX1C1m7Jgykkn8gLrtwumBioXIqam8DLxp88Adbue6Hw==} @@ -3478,6 +3491,9 @@ packages: resolution: {integrity: sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==} engines: {node: ^10.13.0 || ^12.13.0 || ^14.15.0 || >=15.0.0} + proper-lockfile@4.1.2: + resolution: {integrity: sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA==} + property-information@7.1.0: resolution: {integrity: sha512-TwEZ+X+yCJmYfL7TPUOcvBZ4QfoT5YenQiJuX//0th53DE6w0xxLEtfK3iyryQFddXuvkIk51EEgrJQ0WJkOmQ==} @@ -3617,6 +3633,10 @@ packages: resolution: {integrity: sha512-oMA2dcrw6u0YfxJQXm342bFKX/E4sG9rbTzO9ptUcR/e8A33cHuvStiYOwH7fszkZlZ1z/ta9AAoPk2F4qIOHA==} engines: {node: '>=18'} + retry@0.12.0: + resolution: {integrity: sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow==} + engines: {node: '>= 4'} + rfdc@1.4.1: resolution: {integrity: sha512-q1b3N5QkRUWUl7iyylaaj3kOpIT0N2i9MqIEQXP73GVsN9cw3fdx8X63cEmWhJGi2PPCF23Ijp7ktmd39rawIA==} @@ -3696,6 +3716,9 @@ packages: siginfo@2.0.0: resolution: {integrity: sha512-ybx0WO1/8bSBLEWXZvEd7gMW3Sn3JFlW3TvX1nREbDLRNQNaeNN8WK0meBwPdAaOI7TtRRRJn/Es1zhrrCHu7g==} + signal-exit@3.0.7: + resolution: {integrity: sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==} + signal-exit@4.1.0: resolution: {integrity: sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==} engines: {node: '>=14'} @@ -5949,6 +5972,10 @@ snapshots: dependencies: undici-types: 7.19.2 + '@types/proper-lockfile@4.1.4': + dependencies: + '@types/retry': 0.12.5 + '@types/react-dom@19.2.3(@types/react@19.2.14)': dependencies: '@types/react': 19.2.14 @@ -5957,6 +5984,8 @@ snapshots: dependencies: csstype: 3.2.3 + '@types/retry@0.12.5': {} + '@types/trusted-types@2.0.7': optional: true @@ -7379,6 +7408,12 @@ snapshots: ansi-styles: 5.2.0 react-is: 17.0.2 + proper-lockfile@4.1.2: + dependencies: + graceful-fs: 4.2.11 + retry: 0.12.0 + signal-exit: 3.0.7 + property-information@7.1.0: {} pump@3.0.4: @@ -7605,6 +7640,8 @@ snapshots: onetime: 7.0.0 signal-exit: 4.1.0 + retry@0.12.0: {} + rfdc@1.4.1: {} robust-predicates@3.0.3: {} @@ -7719,6 +7756,8 @@ snapshots: siginfo@2.0.0: {} + signal-exit@3.0.7: {} + signal-exit@4.1.0: {} simple-concat@1.0.1: {}