diff --git a/README.md b/README.md index e4fd7ab..8b48191 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,8 @@ keyboard alone, and the mouse stays first-class. lazily loaded code changes in the Local Changes-style Pierre file tree, switch stacked/split layout in place, jump from timeline comments to their file/thread, read GitHub review threads with replies directly beneath their code, - and add stale-head-guarded comments to selected - line ranges. A compact readiness ledger combines review, + reply to them, resolve or reopen them, and add stale-head-guarded comments to + selected line ranges. A compact readiness ledger combines review, checks, conflicts, merge state, and provider freshness before merging with merge-commit, squash, or rebase through a GitHub-style split control. Authentication stays in the signed-in `gh` / `az` CLI; provider policies diff --git a/ROADMAP.md b/ROADMAP.md index 30ccf47..089a6aa 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1064,8 +1064,9 @@ passes on its measured platform. Doc-only change: PRD §8, `docs/perf-baseline.m rendered Markdown, color-coded checks, discussions with top-level comment creation, current-branch auto-open, and full-width lazy selected-file Pierre diffs shipped 2026-07-13 through authenticated provider CLIs. Inline/review - actions, Azure policies, and merge controls remain; GitLab and Bitbucket are - follow-on provider adapters. + comments plus GitHub thread replies/resolution and merge controls now ship; + batched review actions and richer Azure parity remain. GitLab and Bitbucket + are follow-on provider adapters. - ☐ Telemetry (opt-in, clearly disclosed) - ☐ Localization framework + English baseline - ☑ Performance pass on 100k-commit repos — closed 2026-07-06 with the 0.5 @@ -1641,6 +1642,15 @@ to local `HEAD`; it never includes unstaged drafts or creates the PR itself. Vendor CLI health errors remain distinct from signed-out sessions, and sign-in is preflighted before Strand reports that the browser or CLI flow has started. +**Hosted PR thread writes shipped (2026-07-15):** GitHub review-thread cards +now expose provider-permission-gated Reply and Resolve/Reopen actions. Writes +use stable GraphQL thread node IDs through the signed-in `gh` CLI, with bodies +and variables sent over stdin; the small mutation outcomes patch both the +inline Changes card and flattened Conversation timeline locally, preserving the +mounted Pierre diff, file selection, scroll, focus, and per-thread drafts. +Coordinate-based new comments retain their exact-head guard; replies and thread +state do not need one because they target an existing stable thread. + --- ## 1.1+ — Post-1.0 diff --git a/TASKS.md b/TASKS.md index a2ef290..c89a562 100644 --- a/TASKS.md +++ b/TASKS.md @@ -1326,9 +1326,12 @@ tree: watch the agent work, review fast, accept or reject safely. line-range selection, persistent fetched thread cards with replies and resolved/outdated state, and an annotation-row composer through `repo_pull_request_inline_comment`, with - exact-head validation before publishing. Reply creation, Azure - iteration-tracked inline comments, direct binary attachment uploads, - suggestions, and thread resolution remain. + exact-head validation before publishing. GitHub thread cards now publish + immediate replies and Resolve/Reopen writes through provider-capability- + gated GraphQL mutations, patching Changes + Conversation locally without a + detail/patch reload (`repo_pull_request_thread_reply`, + `repo_pull_request_thread_resolve`). Azure iteration-tracked inline writes, + direct binary attachment uploads, and suggestions remain. - ☐ Submit reviews: approve, request changes, dismiss/update a review where supported. - ◐ PR review ledger + merge-readiness model (see `docs/pull-request-improvements.md`). diff --git a/crates/strand-tauri/src/commands.rs b/crates/strand-tauri/src/commands.rs index 241ca54..491310d 100644 --- a/crates/strand-tauri/src/commands.rs +++ b/crates/strand-tauri/src/commands.rs @@ -302,6 +302,35 @@ pub async fn repo_pull_request_inline_comment( .await } +/// Reply to an existing provider review thread. The provider thread ID is a +/// stable target, so this write does not depend on diff coordinates or head SHA. +#[tauri::command(async)] +pub async fn repo_pull_request_thread_reply( + path: String, + thread_id: String, + body: String, +) -> CmdResult { + run_blocking("pull request thread reply", move || { + pull_requests::reply_to_thread(&path, &thread_id, &body) + .map_err(|message| CmdError { message }) + }) + .await +} + +/// Resolve or reopen an existing provider review thread. +#[tauri::command(async)] +pub async fn repo_pull_request_thread_resolve( + path: String, + thread_id: String, + resolved: bool, +) -> CmdResult { + run_blocking("pull request thread resolution", move || { + pull_requests::set_thread_resolved(&path, &thread_id, resolved) + .map_err(|message| CmdError { message }) + }) + .await +} + /// Merge a hosted pull request through its provider. The expected source /// commit prevents merging unseen updates; provider policies remain enforced. #[tauri::command(async)] diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index 7352746..a78a497 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -85,6 +85,8 @@ fn main() { commands::repo_pull_request_diff, commands::repo_pull_request_comment, commands::repo_pull_request_inline_comment, + commands::repo_pull_request_thread_reply, + commands::repo_pull_request_thread_resolve, commands::repo_pull_request_merge, commands::repo_diff_unstaged, commands::repo_diff_staged, diff --git a/crates/strand-tauri/src/pull_requests.rs b/crates/strand-tauri/src/pull_requests.rs index 088e8c4..7169ec5 100644 --- a/crates/strand-tauri/src/pull_requests.rs +++ b/crates/strand-tauri/src/pull_requests.rs @@ -20,6 +20,7 @@ use crate::ai::bin::{base_command, resolve_cli}; const COMMAND_TIMEOUT: Duration = Duration::from_secs(30); const MAX_COMMENT_BYTES: usize = 65_536; +const MAX_THREAD_ID_BYTES: usize = 512; const MAX_PR_DESCRIPTION_BYTES: usize = 65_536; const MAX_PR_TITLE_BYTES: usize = 512; const MAX_DIFF_BYTES: usize = 16 * 1024 * 1024; @@ -63,6 +64,9 @@ const GITHUB_REVIEW_THREADS_QUERY: &str = r#"query($owner: String!, $repo: Strin id isResolved isOutdated + viewerCanReply + viewerCanResolve + viewerCanUnresolve path line startLine @@ -77,6 +81,24 @@ const GITHUB_REVIEW_THREADS_QUERY: &str = r#"query($owner: String!, $repo: Strin } } }"#; +const GITHUB_THREAD_REPLY_MUTATION: &str = r#"mutation($threadId: ID!, $body: String!) { + addPullRequestReviewThreadReply(input: { + pullRequestReviewThreadId: $threadId, + body: $body + }) { + comment { id body createdAt url path author { login avatarUrl } } + } +}"#; +const GITHUB_THREAD_RESOLVE_MUTATION: &str = r#"mutation($threadId: ID!) { + resolveReviewThread(input: { threadId: $threadId }) { + thread { id isResolved isOutdated viewerCanReply viewerCanResolve viewerCanUnresolve } + } +}"#; +const GITHUB_THREAD_UNRESOLVE_MUTATION: &str = r#"mutation($threadId: ID!) { + unresolveReviewThread(input: { threadId: $threadId }) { + thread { id isResolved isOutdated viewerCanReply viewerCanResolve viewerCanUnresolve } + } +}"#; type Result = std::result::Result; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] @@ -135,9 +157,22 @@ pub struct PullRequestReviewThread { pub side: PullRequestDiffSide, pub is_resolved: bool, pub is_outdated: bool, + pub can_reply: bool, + pub can_resolve: bool, + pub can_unresolve: bool, pub comments: Vec, } +#[derive(Debug, Clone, Serialize)] +pub struct PullRequestReviewThreadUpdate { + pub id: String, + pub is_resolved: bool, + pub is_outdated: bool, + pub can_reply: bool, + pub can_resolve: bool, + pub can_unresolve: bool, +} + #[derive(Debug, Clone, Serialize)] pub struct PullRequest { pub id: u64, @@ -426,6 +461,35 @@ pub fn add_inline_comment( } } +pub fn reply_to_thread(path: &str, thread_id: &str, body: &str) -> Result { + validate_comment(body)?; + validate_thread_id(thread_id)?; + let (_, host) = host_for_path(path)?; + match host { + HostRepo::GitHub { .. } => reply_to_thread_github(path, thread_id, body), + HostRepo::Azure { .. } => Err( + "Azure DevOps review-thread replies are not available yet. Open this pull request on Azure DevOps to reply." + .to_string(), + ), + } +} + +pub fn set_thread_resolved( + path: &str, + thread_id: &str, + resolved: bool, +) -> Result { + validate_thread_id(thread_id)?; + let (_, host) = host_for_path(path)?; + match host { + HostRepo::GitHub { .. } => set_thread_resolved_github(path, thread_id, resolved), + HostRepo::Azure { .. } => Err( + "Azure DevOps review-thread resolution is not available yet. Open this pull request on Azure DevOps to update the thread." + .to_string(), + ), + } +} + pub fn merge( path: &str, id: u64, @@ -771,6 +835,55 @@ fn add_inline_comment_github( Ok(()) } +fn reply_to_thread_github(cwd: &str, thread_id: &str, body: &str) -> Result { + let value = run_github_graphql_mutation( + cwd, + GITHUB_THREAD_REPLY_MUTATION, + serde_json::json!({ "threadId": thread_id, "body": body }), + )?; + parse_github_thread_reply(&value).ok_or_else(|| { + "GitHub accepted the reply request but returned an incomplete comment".to_string() + }) +} + +fn set_thread_resolved_github( + cwd: &str, + thread_id: &str, + resolved: bool, +) -> Result { + let mutation = if resolved { + GITHUB_THREAD_RESOLVE_MUTATION + } else { + GITHUB_THREAD_UNRESOLVE_MUTATION + }; + let value = run_github_graphql_mutation( + cwd, + mutation, + serde_json::json!({ "threadId": thread_id }), + )?; + parse_github_thread_update(&value, resolved).ok_or_else(|| { + "GitHub accepted the thread update but returned incomplete thread state".to_string() + }) +} + +fn github_graphql_payload(query: &str, variables: Value) -> Value { + serde_json::json!({ "query": query, "variables": variables }) +} + +fn run_github_graphql_mutation(cwd: &str, query: &str, variables: Value) -> Result { + let input = serde_json::to_vec(&github_graphql_payload(query, variables)) + .map_err(|error| format!("Could not encode GitHub review-thread request: {error}"))?; + let output = run_command_input( + cwd, + "gh", + &["api", "graphql", "--method", "POST", "--input", "-"], + &[("GH_PROMPT_DISABLED", "1")], + Some(&input), + )?; + serde_json::from_slice(&output) + .map_err(|error| format!("GitHub CLI returned invalid review-thread JSON: {error}")) +} + fn github_inline_comment_payload( body: &str, file_path: &str, @@ -1463,6 +1576,18 @@ fn validate_comment(body: &str) -> Result<()> { Ok(()) } +fn validate_thread_id(thread_id: &str) -> Result<()> { + if thread_id.trim().is_empty() + || thread_id.len() > MAX_THREAD_ID_BYTES + || thread_id.bytes().any(|byte| byte.is_ascii_control()) + { + return Err( + "Review thread is missing or invalid; refresh the pull request and try again".into(), + ); + } + Ok(()) +} + fn validate_create( source_branch: &str, target_branch: &str, @@ -1828,12 +1953,69 @@ fn parse_github_review_threads(value: &Value) -> Vec { .get("isOutdated") .and_then(Value::as_bool) .unwrap_or(false), + can_reply: thread + .get("viewerCanReply") + .and_then(Value::as_bool) + .unwrap_or(false), + can_resolve: thread + .get("viewerCanResolve") + .and_then(Value::as_bool) + .unwrap_or(false), + can_unresolve: thread + .get("viewerCanUnresolve") + .and_then(Value::as_bool) + .unwrap_or(false), comments, }) }) .collect() } +fn parse_github_thread_reply(value: &Value) -> Option { + let comment = value.pointer("/data/addPullRequestReviewThreadReply/comment")?; + let author = text(comment.pointer("/author/login")).unwrap_or_else(|| "unknown".into()); + Some(PullRequestComment { + id: text(comment.get("id"))?, + avatar_url: text(comment.pointer("/author/avatarUrl")) + .or_else(|| github_avatar_url(&author)), + author, + body: text(comment.get("body")).unwrap_or_default(), + created_at: text(comment.get("createdAt")).unwrap_or_default(), + url: text(comment.get("url")).unwrap_or_default(), + is_system: false, + path: text(comment.get("path")), + }) +} + +fn parse_github_thread_update( + value: &Value, + resolved: bool, +) -> Option { + let mutation = if resolved { + "resolveReviewThread" + } else { + "unresolveReviewThread" + }; + let thread = value.pointer(&format!("/data/{mutation}/thread"))?; + Some(PullRequestReviewThreadUpdate { + id: text(thread.get("id"))?, + is_resolved: thread.get("isResolved").and_then(Value::as_bool)?, + is_outdated: thread.get("isOutdated").and_then(Value::as_bool)?, + can_reply: thread + .get("viewerCanReply") + .and_then(Value::as_bool) + .unwrap_or(false), + can_resolve: thread + .get("viewerCanResolve") + .and_then(Value::as_bool) + .unwrap_or(false), + can_unresolve: thread + .get("viewerCanUnresolve") + .and_then(Value::as_bool) + .unwrap_or(false), + }) +} + fn parse_azure_pr( value: &Value, organization: &str, @@ -2291,6 +2473,17 @@ mod tests { } } + #[test] + fn github_review_thread_query_requests_write_capabilities() { + for field in [ + "viewerCanReply", + "viewerCanResolve", + "viewerCanUnresolve", + ] { + assert!(GITHUB_REVIEW_THREADS_QUERY.contains(field)); + } + } + #[test] fn normalizes_github_activity_with_stable_ids() { let value = serde_json::json!({ @@ -2400,6 +2593,7 @@ mod tests { let value = serde_json::json!({ "data": { "repository": { "pullRequest": { "reviewThreads": { "nodes": [{ "id": "PRRT_1", "isResolved": false, "isOutdated": false, + "viewerCanReply": true, "viewerCanResolve": true, "viewerCanUnresolve": false, "path": "src/lib.rs", "line": 29, "startLine": 27, "originalLine": 29, "originalStartLine": 27, "diffSide": "RIGHT", "comments": { "nodes": [ @@ -2419,6 +2613,9 @@ mod tests { assert_eq!((threads[0].start_line, threads[0].end_line), (27, 29)); assert_eq!(threads[0].side, PullRequestDiffSide::Additions); assert!(!threads[0].is_resolved); + assert!(threads[0].can_reply); + assert!(threads[0].can_resolve); + assert!(!threads[0].can_unresolve); assert_eq!(threads[0].comments.len(), 2); assert_eq!(threads[0].comments[1].author, "ada"); assert_eq!(threads[0].comments[0].path.as_deref(), Some("src/lib.rs")); @@ -2431,6 +2628,99 @@ mod tests { assert!(validate_comment(&"x".repeat(MAX_COMMENT_BYTES + 1)).is_err()); } + #[test] + fn review_thread_capabilities_fail_closed_when_missing() { + let value = serde_json::json!({ + "data": { "repository": { "pullRequest": { "reviewThreads": { "nodes": [{ + "id": "PRRT_1", "path": "src/lib.rs", "line": 3, "diffSide": "RIGHT", + "comments": { "nodes": [{ "id": "PRRC_1", "body": "Question", + "createdAt": "2026-07-13T12:00:00Z", "url": "https://example.test/comment", + "author": { "login": "octo" } }] } + }] } } } } + }); + let threads = parse_github_review_threads(&value); + assert_eq!(threads.len(), 1); + assert!(!threads[0].can_reply); + assert!(!threads[0].can_resolve); + assert!(!threads[0].can_unresolve); + } + + #[test] + fn builds_and_parses_github_thread_mutations() { + let payload = github_graphql_payload( + GITHUB_THREAD_REPLY_MUTATION, + serde_json::json!({ "threadId": "PRRT_1", "body": "Fixed." }), + ); + assert_eq!(payload["variables"]["threadId"], "PRRT_1"); + assert_eq!(payload["variables"]["body"], "Fixed."); + assert!(payload["query"] + .as_str() + .unwrap() + .contains("addPullRequestReviewThreadReply")); + + let reply = parse_github_thread_reply(&serde_json::json!({ + "data": { "addPullRequestReviewThreadReply": { "comment": { + "id": "PRRC_2", "body": "Fixed.", "createdAt": "2026-07-15T10:00:00Z", + "url": "https://github.com/acme/repo/pull/42#discussion_r2", "path": "src/lib.rs", + "author": { "login": "ada", "avatarUrl": "https://avatars.example/ada" } + } } } + })) + .unwrap(); + assert_eq!(reply.id, "PRRC_2"); + assert_eq!(reply.path.as_deref(), Some("src/lib.rs")); + + for resolved in [true, false] { + let value = if resolved { + serde_json::json!({ "data": { "resolveReviewThread": { "thread": { + "id": "PRRT_1", "isResolved": true, "isOutdated": false, + "viewerCanReply": true, "viewerCanResolve": false, "viewerCanUnresolve": true + } } } }) + } else { + serde_json::json!({ "data": { "unresolveReviewThread": { "thread": { + "id": "PRRT_1", "isResolved": false, "isOutdated": false, + "viewerCanReply": true, "viewerCanResolve": true, "viewerCanUnresolve": false + } } } }) + }; + let update = parse_github_thread_update(&value, resolved).unwrap(); + assert_eq!(update.is_resolved, resolved); + assert_eq!(update.can_unresolve, resolved); + } + } + + #[test] + fn validates_thread_ids_and_rejects_azure_thread_writes() { + assert!(validate_thread_id("PRRT_kwDOExample").is_ok()); + assert!(validate_thread_id(" ").is_err()); + assert!(validate_thread_id("bad\nid").is_err()); + assert!(validate_thread_id(&"x".repeat(MAX_THREAD_ID_BYTES + 1)).is_err()); + + let dir = std::env::temp_dir().join(format!( + "strand-pr-thread-provider-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + git(&dir, &["init", "-q"]); + git( + &dir, + &[ + "remote", + "add", + "origin", + "https://dev.azure.com/acme/project/_git/repo", + ], + ); + let path = dir.to_str().unwrap(); + assert!(reply_to_thread(path, "thread-1", "Reply") + .unwrap_err() + .contains("not available")); + assert!(set_thread_resolved(path, "thread-1", true) + .unwrap_err() + .contains("not available")); + let _ = std::fs::remove_dir_all(dir); + } + #[test] fn validates_pull_request_creation_fields() { assert!(validate_create("feature", "main", "Ship it", "Details").is_ok()); diff --git a/docs/learnings.md b/docs/learnings.md index 83158c1..b3cbb44 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -1259,3 +1259,25 @@ committed merge-base-to-`HEAD` diff only, keep Codex in its read-only sandbox and Claude Code tools disabled, and return editable text without invoking the host provider. Never describe staged/unstaged work as though it were already in the pull request. + +--- + +## Hosted thread writes use node IDs and provider capabilities, not head guards + +**Rule.** New inline comments remain coordinate writes and must carry the exact +reviewed head SHA. Replies and Resolve/Reopen target an existing provider thread +by its stable node ID instead: gate those controls on the provider's +`viewerCanReply` / `viewerCanResolve` / `viewerCanUnresolve` fields and let the +provider reject stale permissions. Missing capability fields fail closed. + +**Why.** Applying the coordinate-write head guard to thread writes would block +valid replies on outdated threads and valid resolution after a push. GitHub's +thread mutation already names the unambiguous object, while the capability +fields express the signed-in viewer's current authority. + +**How to apply.** Send GraphQL mutations and variables through stdin to the +resolved provider CLI, never interpolate user text into argv. Return the small +comment/thread outcome and patch the active PR locally so a reply does not +reload the rich detail or hosted patch, remount Pierre, or discard scroll, +focus, file selection, and per-thread drafts. Background monitoring can then +seed its activity baseline through the existing post-write path. diff --git a/ui/src/lib/pullRequests.test.ts b/ui/src/lib/pullRequests.test.ts index ae0e830..e7ae7bd 100644 --- a/ui/src/lib/pullRequests.test.ts +++ b/ui/src/lib/pullRequests.test.ts @@ -20,9 +20,11 @@ const { pullRequestReadiness, pullRequestForBranch, relativeTimeLabel, + withPullRequestThreadReply, + withPullRequestThreadUpdate, } = await import('./pullRequests'); -import type { PullRequest } from './types'; +import type { PullRequest, PullRequestComment, PullRequestReviewThread } from './types'; function pullRequest(overrides: Partial = {}): PullRequest { return { @@ -157,3 +159,79 @@ describe('pullRequestForBranch', () => { .toBeNull(); }); }); + +describe('review thread state updates', () => { + const firstComment: PullRequestComment = { + id: 'comment-1', + author: 'octo', + avatar_url: null, + body: 'Please fix this.', + created_at: '2026-07-15T10:00:00Z', + url: 'https://github.com/acme/repo/pull/42#discussion_r1', + is_system: false, + path: 'src/lib.rs', + }; + const thread: PullRequestReviewThread = { + id: 'thread-1', + path: 'src/lib.rs', + start_line: 10, + end_line: 12, + side: 'additions', + is_resolved: false, + is_outdated: false, + can_reply: true, + can_resolve: true, + can_unresolve: false, + comments: [firstComment], + }; + + it('adds a reply to its thread and the chronological Conversation timeline', () => { + const reply: PullRequestComment = { + ...firstComment, + id: 'comment-2', + author: 'ada', + body: 'Fixed.', + created_at: '2026-07-15T09:55:00Z', + path: null, + }; + const updated = withPullRequestThreadReply(pullRequest({ + comments: [firstComment], + review_threads: [thread], + comment_count: 1, + }), thread.id, reply); + expect(updated.review_threads[0].comments.map((comment) => comment.id)) + .toEqual(['comment-2', 'comment-1']); + expect(updated.comments.map((comment) => comment.id)).toEqual(['comment-2', 'comment-1']); + expect(updated.comments[0].path).toBe('src/lib.rs'); + expect(updated.comment_count).toBe(2); + }); + + it('deduplicates a repeated provider reply outcome', () => { + const pr = pullRequest({ comments: [firstComment], review_threads: [thread], comment_count: 1 }); + const once = withPullRequestThreadReply(pr, thread.id, firstComment); + const twice = withPullRequestThreadReply(once, thread.id, firstComment); + expect(twice.review_threads[0].comments).toHaveLength(1); + expect(twice.comments).toHaveLength(1); + expect(twice.comment_count).toBe(1); + }); + + it('updates only the matching thread state and capabilities', () => { + const other = { ...thread, id: 'thread-2' }; + const pr = pullRequest({ review_threads: [thread, other] }); + const updated = withPullRequestThreadUpdate(pr, { + id: thread.id, + is_resolved: true, + is_outdated: false, + can_reply: true, + can_resolve: false, + can_unresolve: true, + }); + expect(updated.review_threads[0]).toMatchObject({ + is_resolved: true, + can_resolve: false, + can_unresolve: true, + }); + expect(updated.review_threads[1]).toBe(other); + expect(updated.comments).toBe(pr.comments); + }); +}); diff --git a/ui/src/lib/pullRequests.ts b/ui/src/lib/pullRequests.ts index 1fecde6..c4ab44f 100644 --- a/ui/src/lib/pullRequests.ts +++ b/ui/src/lib/pullRequests.ts @@ -1,7 +1,12 @@ import { parsePatchFiles, type FileDiffMetadata } from '@pierre/diffs'; import { hashPatch } from './patch'; -import type { PullRequest, PullRequestProvider } from './types'; +import type { + PullRequest, + PullRequestComment, + PullRequestProvider, + PullRequestReviewThreadUpdate, +} from './types'; export type CheckTone = 'success' | 'running' | 'failed' | 'neutral'; @@ -208,6 +213,47 @@ export function parsePullRequestPatch(patch: string): FileDiffMetadata[] { return parsePatchFiles(patch, `pr:${hashPatch(patch)}`, true).flatMap((parsed) => parsed.files); } +export function withPullRequestThreadReply( + pr: PullRequest, + threadId: string, + reply: PullRequestComment, +): PullRequest { + const thread = pr.review_threads.find((candidate) => candidate.id === threadId); + if (!thread) return pr; + const normalizedReply = reply.path ? reply : { ...reply, path: thread.path }; + const reviewThreads = pr.review_threads.map((candidate) => candidate.id === threadId + ? { + ...candidate, + comments: candidate.comments.some((comment) => comment.id === reply.id) + ? candidate.comments + : [...candidate.comments, normalizedReply] + .sort((left, right) => left.created_at.localeCompare(right.created_at)), + } + : candidate); + const comments = [...pr.comments]; + if (!comments.some((comment) => comment.id === reply.id)) comments.push(normalizedReply); + comments.sort((left, right) => left.created_at.localeCompare(right.created_at)); + return { + ...pr, + review_threads: reviewThreads, + comments, + comment_count: comments.length, + }; +} + +export function withPullRequestThreadUpdate( + pr: PullRequest, + update: PullRequestReviewThreadUpdate, +): PullRequest { + if (!pr.review_threads.some((thread) => thread.id === update.id)) return pr; + return { + ...pr, + review_threads: pr.review_threads.map((thread) => thread.id === update.id + ? { ...thread, ...update } + : thread), + }; +} + export function diffStats(file: FileDiffMetadata): { additions: number; deletions: number } { let additions = 0; let deletions = 0; diff --git a/ui/src/lib/tauri.ts b/ui/src/lib/tauri.ts index 3b4cd57..fc898ab 100644 --- a/ui/src/lib/tauri.ts +++ b/ui/src/lib/tauri.ts @@ -26,9 +26,11 @@ import type { PullRequest, PullRequestActivitySnapshot, PullRequestBranchMatch, + PullRequestComment, PullRequestCreateOutcome, PullRequestList, PullRequestMergeStrategy, + PullRequestReviewThreadUpdate, PullRequestSuggestion, RebaseEntry, RebaseStep, @@ -151,6 +153,12 @@ export const tauri = { ) => invoke('repo_pull_request_inline_comment', { path, id, body, filePath, startLine, endLine, side, expectedHead, }), + repoPullRequestThreadReply: (path: string, threadId: string, body: string) => + invoke('repo_pull_request_thread_reply', { path, threadId, body }), + repoPullRequestThreadResolve: (path: string, threadId: string, resolved: boolean) => + invoke('repo_pull_request_thread_resolve', { + path, threadId, resolved, + }), repoPullRequestMerge: ( path: string, id: number, diff --git a/ui/src/lib/types.ts b/ui/src/lib/types.ts index f8bb5e5..f4eabbf 100644 --- a/ui/src/lib/types.ts +++ b/ui/src/lib/types.ts @@ -221,9 +221,21 @@ export interface PullRequestReviewThread { side: 'deletions' | 'additions'; is_resolved: boolean; is_outdated: boolean; + can_reply: boolean; + can_resolve: boolean; + can_unresolve: boolean; comments: PullRequestComment[]; } +export interface PullRequestReviewThreadUpdate { + id: string; + is_resolved: boolean; + is_outdated: boolean; + can_reply: boolean; + can_resolve: boolean; + can_unresolve: boolean; +} + export interface PullRequest { id: number; title: string; diff --git a/ui/src/styles/features.css b/ui/src/styles/features.css index 0cf75f4..e1b2f15 100644 --- a/ui/src/styles/features.css +++ b/ui/src/styles/features.css @@ -5293,8 +5293,16 @@ textarea.clone-input { background: color-mix(in oklch, var(--bg-panel), transparent 25%); } .pr-inline-thread > header strong { color: var(--text); font: 500 10px var(--font-mono); } -.pr-inline-thread > header > div { display: flex; gap: 5px; } -.pr-inline-thread > header span { +.pr-inline-thread-head-actions, +.pr-inline-thread-labels { display: flex; align-items: center; gap: 5px; } +.pr-inline-thread-head-actions { flex-wrap: wrap; justify-content: flex-end; } +.pr-inline-thread-head-actions .h-link { + min-height: 22px; + padding: 2px 5px; + border-radius: var(--r-sm); + font-size: 9px; +} +.pr-inline-thread-labels span { padding: 1px 5px; border: 0.5px solid var(--border-strong); border-radius: 999px; @@ -5338,6 +5346,34 @@ textarea.clone-input { } .pr-inline-avatar img { position: absolute; inset: 0; width: 100%; height: 100%; object-fit: cover; } .pr-inline-thread-comment > .markdown { font-size: 11px; } +.pr-thread-message { + padding: 6px 11px; + border-top: 0.5px solid var(--border); + color: var(--text-muted); + font-size: 10px; +} +.pr-thread-message.ok { color: var(--add); } +.pr-thread-message.error { color: var(--del); } +.pr-thread-reply { + display: grid; + gap: 8px; + padding: 10px 11px; + border-top: 0.5px solid var(--border); + background: color-mix(in oklch, var(--bg-panel), transparent 35%); +} +.pr-thread-reply textarea { + box-sizing: border-box; + width: 100%; + resize: vertical; + padding: 8px 9px; + border: 0.5px solid var(--border-strong); + border-radius: 5px; + outline: none; + background: var(--bg-base); + color: var(--text); + font: 11.5px/1.45 var(--font-ui); +} +.pr-thread-reply textarea:focus { border-color: var(--accent); } .pr-inline-message { padding: 5px 10px; border-bottom: 0.5px solid var(--border); diff --git a/ui/src/views/PullRequests.tsx b/ui/src/views/PullRequests.tsx index 29e180a..ea69ada 100644 --- a/ui/src/views/PullRequests.tsx +++ b/ui/src/views/PullRequests.tsx @@ -17,6 +17,8 @@ import { pullRequestReadiness, pullRequestForBranch, relativeTimeLabel, + withPullRequestThreadReply, + withPullRequestThreadUpdate, } from '../lib/pullRequests'; import { pullRequestActivityChanged, pullRequestFollowKey } from '../lib/pullRequestActivity'; import { errMessage, tauri } from '../lib/tauri'; @@ -408,10 +410,37 @@ function fileGitStatus(file: FileDiffMetadata): GitStatusEntry['status'] { return 'modified'; } -function PullRequestInlineThread({ thread, prUrl }: { thread: PullRequestReviewThread; prUrl: string }) { +function PullRequestInlineThread({ + thread, + prUrl, + replying, + replyDraft, + writeKind, + message, + platform, + onStartReply, + onCancelReply, + onReplyDraft, + onSubmitReply, + onSetResolved, +}: { + thread: PullRequestReviewThread; + prUrl: string; + replying: boolean; + replyDraft: string; + writeKind: 'reply' | 'resolve' | undefined; + message: { tone: 'ok' | 'error'; text: string } | undefined; + platform: string; + onStartReply: () => void; + onCancelReply: () => void; + onReplyDraft: (value: string) => void; + onSubmitReply: () => void; + onSetResolved: (resolved: boolean) => void; +}) { const lineLabel = thread.start_line === thread.end_line ? `${thread.end_line}` : `${thread.start_line}–${thread.end_line}`; + const busy = writeKind != null; return (
{thread.side === 'deletions' ? 'Old' : 'New'} line{thread.start_line === thread.end_line ? '' : 's'} {lineLabel} -
- {thread.is_resolved && Resolved} - {thread.is_outdated && Outdated} +
+
+ {thread.is_resolved && Resolved} + {thread.is_outdated && Outdated} +
+ {thread.can_reply && ( + + )} + {!thread.is_resolved && thread.can_resolve && ( + + )} + {thread.is_resolved && thread.can_unresolve && ( + + )}
{thread.comments.map((comment) => { @@ -447,6 +512,47 @@ function PullRequestInlineThread({ thread, prUrl }: { thread: PullRequestReviewT ); })} + {message && ( +
+ {message.text} +
+ )} + {replying && ( +
{ + event.preventDefault(); + onSubmitReply(); + }} + > +