diff --git a/scripts/screenshot/specs/repository-settings-stack-comments.spec.tsx b/scripts/screenshot/specs/repository-settings-stack-comments.spec.tsx new file mode 100644 index 000000000..e60ab2fc2 --- /dev/null +++ b/scripts/screenshot/specs/repository-settings-stack-comments.spec.tsx @@ -0,0 +1,47 @@ +import * as React from "react"; +import { expect, it } from "vitest"; +import userEvent from "@testing-library/user-event"; +import { createTestRepo, openRepo } from "../../../test/utils"; +import { render, screen } from "../../../test/test-utils"; +import { Dashboard } from "../../../src/components/Dashboard"; +import { captureDocument } from "../capture"; + +it("captures the stack comments toggle defaulting on and saving off", async () => { + const { repoPath } = createTestRepo(false); + openRepo(repoPath); + + const user = userEvent.setup(); + render(); + + await user.click(await screen.findByLabelText("Settings")); + await screen.findByRole("tab", { name: /repository/i, selected: true }); + + const toggle = await screen.findByRole("switch", { + name: /post stack comments on pull requests/i, + }); + expect(toggle).toHaveAttribute("aria-checked", "true"); + + await captureDocument(document, { + name: "repository-settings-stack-comments-01-default-on", + viewport: { width: 1440, height: 1200 }, + expectations: [ + "The Repository tab shows a 'Post stack comments on pull requests' row below 'Ignore generated Treq paths'.", + "The row's description says the comment lists the merge order and is visible to everyone on GitHub.", + "The Post stack comments switch is in the on (checked) position.", + ], + }); + + await user.click(toggle); + expect(toggle).toHaveAttribute("aria-checked", "false"); + await user.click(screen.getByRole("button", { name: /save settings/i })); + await screen.findByText("Settings saved"); + + await captureDocument(document, { + name: "repository-settings-stack-comments-02-off-and-saved", + viewport: { width: 1440, height: 1200 }, + expectations: [ + "The Post stack comments switch is in the off (unchecked) position.", + "A 'Settings saved' toast is visible confirming the save.", + ], + }); +}, 60000); diff --git a/src-tauri/src/commands/github.rs b/src-tauri/src/commands/github.rs index 8ea07cb4a..8513a4144 100644 --- a/src-tauri/src/commands/github.rs +++ b/src-tauri/src/commands/github.rs @@ -404,6 +404,31 @@ pub async fn gh_create_pr( .map_err(|e| format!("Failed to join gh_create_pr task: {}", e))? } +/// Post or refresh the stack comment on every open PR in `head_branch`'s +/// stack. The frontend calls this only after `gh_create_pr` has returned, +/// so a failure here cannot fail PR creation. +#[tauri::command] +pub async fn gh_sync_stack_comments( + repo_path: String, + repo_full_name: String, + head_branch: String, +) -> Result { + // A missing `gh` is only an error once the core finds a stack to comment on. + let gh = gh_bin().ok(); + let extended_path = get_extended_path(); + tauri::async_runtime::spawn_blocking(move || { + crate::core::stack_comments::sync_stack_comments( + &repo_path, + &repo_full_name, + &head_branch, + gh.as_deref(), + &extended_path, + ) + }) + .await + .map_err(|e| format!("Failed to join gh_sync_stack_comments task: {e}"))? +} + #[cfg(test)] mod tests { use std::fs; diff --git a/src-tauri/src/core/mod.rs b/src-tauri/src/core/mod.rs index 547b964f7..8feb0e8f9 100644 --- a/src-tauri/src/core/mod.rs +++ b/src-tauri/src/core/mod.rs @@ -27,6 +27,7 @@ pub mod repo; pub mod resolve; pub mod sessions; pub mod skills; +pub mod stack_comments; pub mod stash; pub mod submodules; pub mod workspaces; diff --git a/src-tauri/src/core/stack_comments.rs b/src-tauri/src/core/stack_comments.rs new file mode 100644 index 000000000..80e0ab262 --- /dev/null +++ b/src-tauri/src/core/stack_comments.rs @@ -0,0 +1,550 @@ +//! Stack comments. After Treq creates a pull request for a workspace that +//! sits in a stack, it keeps one comment on every open PR in that stack that +//! lists the layers, so reviewers know the merge order. + +use std::collections::{HashMap, HashSet}; +use std::sync::Mutex; + +use crate::github::{self, CommentWrite, OpenPr}; +use crate::local_db::{self, Workspace}; +use crate::lock_ext::LockExt; + +/// Repository setting that turns stack comments off when set to `"false"`. +pub const STACK_COMMENTS_SETTING: &str = "post_stack_comments"; + +/// Hidden marker that identifies Treq's stack comment on a PR. +pub const STACK_COMMENT_MARKER: &str = ""; + +/// Serializes syncs so a comment found missing is posted before the next +/// sync looks for it. +static SYNC_LOCK: Mutex<()> = Mutex::new(()); + +const STACK_COMMENT_FOOTER: &str = "Stacked with [Treq](https://treq.dev/?utm_source=github&utm_medium=stack_comment&utm_campaign=stack), which rebases each layer when the one below it moves."; + +/// What [`sync_stack_comments`] did. +#[derive(serde::Serialize, Clone, Debug, PartialEq, Eq)] +#[serde(tag = "status", rename_all = "snake_case")] +pub enum StackCommentOutcome { + /// The repository setting is off. + Disabled, + /// Fewer than two open PRs share a stack with the branch. + NotStacked, + Posted { + created: u32, + updated: u32, + unchanged: u32, + }, +} + +/// Workspaces linked by `target_branch`: a child targets its parent's branch. +struct StackTree { + parent: HashMap, + children: HashMap>, +} + +impl StackTree { + fn new(workspaces: &[Workspace]) -> Self { + let branches: HashSet<&str> = workspaces.iter().map(|w| w.branch_name.as_str()).collect(); + let mut parent = HashMap::new(); + let mut children: HashMap> = HashMap::new(); + for workspace in workspaces { + let Some(target) = workspace.target_branch.as_deref() else { + continue; + }; + if target != workspace.branch_name && branches.contains(target) { + parent.insert(workspace.branch_name.clone(), target.to_string()); + children + .entry(target.to_string()) + .or_default() + .push(workspace.branch_name.clone()); + } + } + StackTree { parent, children } + } + + /// Parent, grandparent, and so on, nearest first. + fn ancestors(&self, branch: &str) -> Vec { + let mut seen = HashSet::from([branch.to_string()]); + let mut ancestors = Vec::new(); + let mut current = branch; + while let Some(parent) = self.parent.get(current) { + if !seen.insert(parent.clone()) { + break; + } + ancestors.push(parent.clone()); + current = parent; + } + ancestors + } + + /// Children and their children, depth first. + fn descendants(&self, branch: &str) -> Vec { + let mut seen = HashSet::from([branch.to_string()]); + let mut descendants = Vec::new(); + let mut pending: Vec<&String> = self.children_of(branch).rev().collect(); + while let Some(next) = pending.pop() { + if !seen.insert(next.clone()) { + continue; + } + descendants.push(next.clone()); + pending.extend(self.children_of(next).rev()); + } + descendants + } + + fn children_of(&self, branch: &str) -> impl DoubleEndedIterator { + self.children.get(branch).into_iter().flatten() + } + + /// The stack as `branch` sees it, newest layer first. + fn layers(&self, branch: &str) -> Vec { + let mut layers = self.descendants(branch); + layers.reverse(); + layers.push(branch.to_string()); + for ancestor in self.ancestors(branch) { + // A target_branch cycle makes a branch both an ancestor and a descendant. + if !layers.contains(&ancestor) { + layers.push(ancestor); + } + } + layers + } + + /// Every branch in the workspace tree that holds `branch`. + fn tree_of(&self, branch: &str) -> Vec { + let root = self + .ancestors(branch) + .pop() + .unwrap_or_else(|| branch.to_string()); + let mut tree = self.descendants(&root); + tree.insert(0, root); + tree + } +} + +/// Backslash-escape characters that GitHub Markdown would otherwise format. +fn escape_markdown(text: &str) -> String { + let mut escaped = String::with_capacity(text.len()); + for c in text.chars() { + if matches!(c, '\\' | '`' | '*' | '_' | '~' | '[' | ']' | '<' | '>') { + escaped.push('\\'); + } + escaped.push(c); + } + escaped +} + +/// Render the stack comment for PR `current`. `layers` runs newest first. +pub fn render_stack_comment(layers: &[&OpenPr], current: u64, base_branch: &str) -> String { + let mut body = String::from("**Stack** (merge from the bottom up)\n"); + for layer in layers { + let title = escape_markdown(&layer.title); + if layer.number == current { + body.push_str(&format!("- **#{} {title}** ← this PR\n", layer.number)); + } else { + body.push_str(&format!("- #{} {title}\n", layer.number)); + } + } + body.push_str(&format!( + "- `{base_branch}`\n\n{STACK_COMMENT_FOOTER}\n{STACK_COMMENT_MARKER}" + )); + body +} + +/// Stack comments are on unless the repository setting is `"false"`. An +/// unreadable setting counts as off, so a stored "off" never reads as "on". +fn stack_comments_enabled(repo_path: &str) -> bool { + let db_path = crate::core::resolve_app_db_path(repo_path); + if !db_path.exists() { + return true; + } + match crate::db::Database::new(db_path) + .and_then(|db| db.get_repo_setting(repo_path, STACK_COMMENTS_SETTING)) + { + Ok(value) => value.as_deref() != Some("false"), + Err(e) => { + log::warn!("Skipping stack comments: could not read {STACK_COMMENTS_SETTING}: {e}"); + false + } + } +} + +/// Create or refresh the stack comment on every open PR in `head_branch`'s +/// stack. Never touches GitHub when the setting is off or the branch is not +/// stacked. +pub fn sync_stack_comments( + repo_path: &str, + repo_full_name: &str, + head_branch: &str, + gh_path: Option<&str>, + extended_path: &str, +) -> Result { + if !stack_comments_enabled(repo_path) { + return Ok(StackCommentOutcome::Disabled); + } + let workspaces = local_db::get_workspaces(repo_path)?; + let tree = StackTree::new(&workspaces); + let members = tree.layers(head_branch); + if members.len() < 2 { + return Ok(StackCommentOutcome::NotStacked); + } + + let gh = gh_path.ok_or_else(|| "gh CLI not found".to_string())?; + // Two syncs that both find no comment would both post one. + let _one_sync_at_a_time = SYNC_LOCK.lock_or_recover(); + let mut open_prs = HashMap::new(); + for branch in tree.tree_of(head_branch) { + if let Some(pr) = + github::gh_find_open_pr_for_branch_impl(gh, repo_full_name, &branch, extended_path)? + { + open_prs.insert(branch, pr); + } + } + let targets: Vec<&String> = members + .iter() + .filter(|branch| open_prs.contains_key(*branch)) + .collect(); + if targets.len() < 2 { + return Ok(StackCommentOutcome::NotStacked); + } + + let login = github::gh_authenticated_login_impl(gh, extended_path)?; + let (mut created, mut updated, mut unchanged) = (0, 0, 0); + let mut failures = Vec::new(); + for branch in targets { + let pr = &open_prs[branch]; + let layers: Vec<&OpenPr> = tree + .layers(branch) + .iter() + .filter_map(|layer| open_prs.get(layer)) + .collect(); + let base_branch = layers + .last() + .map_or("", |bottom| bottom.base_ref_name.as_str()); + let body = render_stack_comment(&layers, pr.number, base_branch); + match github::upsert_marked_pr_comment_impl( + gh, + repo_full_name, + pr.number, + &login, + STACK_COMMENT_MARKER, + &body, + extended_path, + ) { + Ok(CommentWrite::Created) => created += 1, + Ok(CommentWrite::Updated) => updated += 1, + Ok(CommentWrite::Unchanged) => unchanged += 1, + Err(e) => { + log::warn!("Failed to update the stack comment on #{}: {e}", pr.number); + failures.push(format!("#{}: {e}", pr.number)); + } + } + } + if !failures.is_empty() { + return Err(format!( + "Could not update the stack comment on {}", + failures.join("; ") + )); + } + Ok(StackCommentOutcome::Posted { + created, + updated, + unchanged, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::core::AppDataDirGuard; + use std::fs; + use std::io::Write; + use tempfile::TempDir; + + fn workspace(branch: &str, target: Option<&str>) -> Workspace { + Workspace { + id: 0, + repo_path: String::new(), + workspace_name: branch.to_string(), + workspace_path: branch.to_string(), + branch_name: branch.to_string(), + created_at: String::new(), + refreshed_at: None, + metadata: None, + target_branch: target.map(str::to_string), + title: branch.to_string(), + description: None, + moved_files: None, + not_on_remote: false, + sparse_patterns: None, + hidden_until: None, + archived: false, + } + } + + fn pr(number: u64, title: &str, base: &str) -> OpenPr { + OpenPr { + number, + title: title.to_string(), + base_ref_name: base.to_string(), + } + } + + #[test] + fn renders_layers_newest_first_and_marks_the_current_pr() { + let top = pr(103, "Add tests", "feat/parser"); + let middle = pr(102, "Refactor parser", "feat/module"); + let bottom = pr(101, "Extract module", "main"); + + let body = render_stack_comment(&[&top, &middle, &bottom], 102, "main"); + + assert_eq!( + body, + "**Stack** (merge from the bottom up)\n\ + - #103 Add tests\n\ + - **#102 Refactor parser** ← this PR\n\ + - #101 Extract module\n\ + - `main`\n\ + \n\ + Stacked with [Treq](https://treq.dev/?utm_source=github&utm_medium=stack_comment&utm_campaign=stack), which rebases each layer when the one below it moves.\n\ + " + ); + } + + #[test] + fn render_escapes_markdown_in_titles() { + let layer = pr(7, "Fix *bar*", "main"); + + let body = render_stack_comment(&[&layer], 7, "main"); + + assert!(body.contains("- **#7 Fix \\ \\*bar\\*** ← this PR\n")); + } + + #[test] + fn layers_put_descendants_above_and_ancestors_below() { + let workspaces = [ + workspace("feat/a", Some("main")), + workspace("feat/b", Some("feat/a")), + workspace("feat/c", Some("feat/b")), + workspace("other", Some("main")), + ]; + let tree = StackTree::new(&workspaces); + + assert_eq!(tree.layers("feat/b"), ["feat/c", "feat/b", "feat/a"]); + assert_eq!(tree.tree_of("feat/c"), ["feat/a", "feat/b", "feat/c"]); + assert_eq!(tree.layers("other"), ["other"]); + } + + #[test] + fn layers_survive_a_target_branch_cycle() { + let workspaces = [ + workspace("feat/a", Some("feat/b")), + workspace("feat/b", Some("feat/a")), + ]; + let tree = StackTree::new(&workspaces); + + assert_eq!(tree.layers("feat/a"), ["feat/b", "feat/a"]); + } + + /// A local DB with the stack `feat/a` <- `feat/b` and the lone `solo`. + fn stacked_repo() -> TempDir { + let repo = TempDir::new().unwrap(); + let path = repo.path().to_str().unwrap(); + for (branch, target) in [("feat/a", "main"), ("feat/b", "feat/a"), ("solo", "main")] { + let id = local_db::add_workspace( + path, + branch.replace('/', "-"), + branch.replace('/', "-"), + branch.to_string(), + None, + None, + None, + ) + .unwrap(); + local_db::update_workspace_target_branch(path, id, target).unwrap(); + } + repo + } + + /// Fake `gh` that logs each call's argv to `calls` and reports `open_prs`. + fn write_stack_gh(dir: &TempDir, open_prs: &[(&str, u64)]) -> String { + let mut cases = String::new(); + for (branch, number) in open_prs { + cases.push_str(&format!( + " \"pr list --repo owner/repo --head={branch} --state open --json number,title,baseRefName --limit 1\")\n echo '[{{\"number\":{number},\"title\":\"PR {number}\",\"baseRefName\":\"main\"}}]' ;;\n" + )); + } + let script = r#"#!/bin/sh +echo "$*" >> 'DIR/calls' +case "$*" in +CASES "pr list "*) echo '[]' ;; + "api user --jq .login") echo alice ;; + "api --paginate "*) + n=$(echo "$3" | cut -d/ -f5) + if [ -e "DIR/body-$n" ]; then + echo "[{\"id\":$n,\"body\":\"\",\"user\":{\"login\":\"alice\"}}]" + else + sleep 0.3 + echo '[]' + fi ;; + "pr comment "*) cat > "DIR/body-$3" ;; + "api --method PATCH "*) cat > /dev/null ;; + *) exit 9 ;; +esac +"# + .replace("DIR", &dir.path().display().to_string()) + .replace("CASES", &cases); + let path = dir.path().join("gh"); + let mut file = fs::File::create(&path).unwrap(); + file.write_all(script.as_bytes()).unwrap(); + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).unwrap(); + path.to_str().unwrap().to_string() + } + + fn gh_calls(dir: &TempDir) -> Vec { + fs::read_to_string(dir.path().join("calls")) + .unwrap_or_default() + .lines() + .map(str::to_string) + .collect() + } + + #[test] + #[cfg(unix)] + fn sync_comments_on_every_open_pr_in_the_stack() { + let app = TempDir::new().unwrap(); + let _guard = AppDataDirGuard::set(app.path()); + let repo = stacked_repo(); + let bin = TempDir::new().unwrap(); + let gh = write_stack_gh(&bin, &[("feat/a", 101), ("feat/b", 102)]); + + let outcome = sync_stack_comments( + repo.path().to_str().unwrap(), + "owner/repo", + "feat/b", + Some(&gh), + "/usr/bin:/bin", + ) + .unwrap(); + + assert_eq!( + outcome, + StackCommentOutcome::Posted { + created: 2, + updated: 0, + unchanged: 0 + } + ); + let calls = gh_calls(&bin); + assert!(calls.contains(&"pr comment 101 --repo owner/repo --body-file -".to_string())); + assert!(calls.contains(&"pr comment 102 --repo owner/repo --body-file -".to_string())); + let bottom = fs::read_to_string(bin.path().join("body-101")).unwrap(); + assert!(bottom.contains("- #102 PR 102\n- **#101 PR 101** ← this PR\n- `main`\n")); + let top = fs::read_to_string(bin.path().join("body-102")).unwrap(); + assert!(top.contains("- **#102 PR 102** ← this PR\n- #101 PR 101\n- `main`\n")); + } + + #[test] + #[cfg(unix)] + fn concurrent_syncs_post_one_comment_per_pr() { + let repo = stacked_repo(); + let repo_path = repo.path().to_str().unwrap().to_string(); + let bin = TempDir::new().unwrap(); + let gh = write_stack_gh(&bin, &[("feat/a", 101), ("feat/b", 102)]); + + let syncs: Vec<_> = ["feat/a", "feat/b"] + .into_iter() + .map(|head| { + let (repo_path, gh) = (repo_path.clone(), gh.clone()); + std::thread::spawn(move || { + let app = TempDir::new().unwrap(); + let _guard = AppDataDirGuard::set(app.path()); + sync_stack_comments(&repo_path, "owner/repo", head, Some(&gh), "/usr/bin:/bin") + }) + }) + .collect(); + for sync in syncs { + sync.join().unwrap().unwrap(); + } + + let posts: Vec = gh_calls(&bin) + .into_iter() + .filter(|call| call.starts_with("pr comment ")) + .collect(); + assert_eq!(posts.len(), 2, "{posts:?}"); + } + + #[test] + #[cfg(unix)] + fn sync_posts_nothing_when_the_setting_is_off() { + let app = TempDir::new().unwrap(); + let _guard = AppDataDirGuard::set(app.path()); + let repo = stacked_repo(); + let repo_path = repo.path().to_str().unwrap(); + let db = crate::db::Database::new(crate::core::resolve_app_db_path(repo_path)).unwrap(); + db.init().unwrap(); + db.set_repo_setting(repo_path, STACK_COMMENTS_SETTING, "false") + .unwrap(); + let bin = TempDir::new().unwrap(); + let gh = write_stack_gh(&bin, &[("feat/a", 101), ("feat/b", 102)]); + + let outcome = sync_stack_comments( + repo_path, + "owner/repo", + "feat/b", + Some(&gh), + "/usr/bin:/bin", + ) + .unwrap(); + + assert_eq!(outcome, StackCommentOutcome::Disabled); + assert!(gh_calls(&bin).is_empty()); + } + + #[test] + #[cfg(unix)] + fn sync_leaves_a_workspace_outside_any_stack_alone() { + let app = TempDir::new().unwrap(); + let _guard = AppDataDirGuard::set(app.path()); + let repo = stacked_repo(); + let bin = TempDir::new().unwrap(); + let gh = write_stack_gh(&bin, &[("solo", 103)]); + + let outcome = sync_stack_comments( + repo.path().to_str().unwrap(), + "owner/repo", + "solo", + Some(&gh), + "/usr/bin:/bin", + ) + .unwrap(); + + assert_eq!(outcome, StackCommentOutcome::NotStacked); + assert!(gh_calls(&bin).is_empty()); + } + + #[test] + #[cfg(unix)] + fn sync_does_not_comment_when_only_one_pr_in_the_stack_is_open() { + let app = TempDir::new().unwrap(); + let _guard = AppDataDirGuard::set(app.path()); + let repo = stacked_repo(); + let bin = TempDir::new().unwrap(); + let gh = write_stack_gh(&bin, &[("feat/b", 102)]); + + let outcome = sync_stack_comments( + repo.path().to_str().unwrap(), + "owner/repo", + "feat/b", + Some(&gh), + "/usr/bin:/bin", + ) + .unwrap(); + + assert_eq!(outcome, StackCommentOutcome::NotStacked); + assert!(gh_calls(&bin) + .iter() + .all(|call| call.starts_with("pr list "))); + } +} diff --git a/src-tauri/src/github.rs b/src-tauri/src/github.rs index 3df1f4535..daa3e5b66 100644 --- a/src-tauri/src/github.rs +++ b/src-tauri/src/github.rs @@ -789,6 +789,171 @@ pub fn gh_create_pr_impl( Err(format!("Could not parse PR number from output: {text}")) } +/// An open pull request, as `gh pr list` reports it. +#[derive(serde::Deserialize, Clone, Debug, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct OpenPr { + pub number: u64, + pub title: String, + pub base_ref_name: String, +} + +/// The open PR in `repo_full_name` whose head is `branch`, if there is one. +pub fn gh_find_open_pr_for_branch_impl( + gh_path: &str, + repo_full_name: &str, + branch: &str, + extended_path: &str, +) -> Result, String> { + // One `--head=` argument, so a branch that starts with `-` stays a value. + let head = format!("--head={branch}"); + let out = run_gh( + gh_path, + &[ + "pr", + "list", + "--repo", + repo_full_name, + &head, + "--state", + "open", + "--json", + "number,title,baseRefName", + "--limit", + "1", + ], + extended_path, + )?; + let bytes = check_gh_output(out)?; + let prs: Vec = + serde_json::from_slice(&bytes).map_err(|e| format!("Failed to parse gh output: {e}"))?; + Ok(prs.into_iter().next()) +} + +/// The login of the GitHub account `gh` is signed in as. +pub fn gh_authenticated_login_impl(gh_path: &str, extended_path: &str) -> Result { + let out = run_gh(gh_path, &["api", "user", "--jq", ".login"], extended_path)?; + let bytes = check_gh_output(out)?; + let login = String::from_utf8_lossy(&bytes).trim().to_string(); + if login.is_empty() { + return Err("gh did not report a signed-in GitHub user".to_string()); + } + Ok(login) +} + +#[derive(serde::Deserialize, Clone, Debug)] +pub struct GhApiUser { + pub login: String, +} + +/// An issue or PR conversation comment from the REST API. +#[derive(serde::Deserialize, Clone, Debug)] +pub struct GhApiComment { + pub id: u64, + #[serde(default)] + pub body: String, + /// `null` when the author's account was deleted. + pub user: Option, +} + +fn checked_repo_full_name(repo_full_name: &str) -> Result<&str, String> { + valid_repo_path(repo_full_name) + .map(|_| repo_full_name) + .ok_or_else(|| format!("Invalid GitHub repository: {repo_full_name}")) +} + +/// Every conversation comment on issue or PR `number`, across all pages. +pub fn gh_list_issue_comments_impl( + gh_path: &str, + repo_full_name: &str, + number: u64, + extended_path: &str, +) -> Result, String> { + let repo = checked_repo_full_name(repo_full_name)?; + let endpoint = format!("repos/{repo}/issues/{number}/comments?per_page=100"); + let out = run_gh(gh_path, &["api", "--paginate", &endpoint], extended_path)?; + let bytes = check_gh_output(out)?; + // `--paginate` prints one JSON array per page, back to back. + let mut comments = Vec::new(); + for page in serde_json::Deserializer::from_slice(&bytes).into_iter::>() { + comments.extend(page.map_err(|e| format!("Failed to parse gh output: {e}"))?); + } + Ok(comments) +} + +/// Replace the body of conversation comment `comment_id`. The body travels +/// over stdin as JSON, never as an argv value. +fn gh_update_issue_comment_impl( + gh_path: &str, + repo_full_name: &str, + comment_id: u64, + body: &str, + extended_path: &str, +) -> Result<(), String> { + let repo = checked_repo_full_name(repo_full_name)?; + let endpoint = format!("repos/{repo}/issues/comments/{comment_id}"); + let payload = serde_json::json!({ "body": body }).to_string(); + let out = run_gh_with_stdin( + gh_path, + &["api", "--method", "PATCH", &endpoint, "--input", "-"], + &payload, + extended_path, + )?; + check_gh_output(out).map(|_| ()) +} + +/// What [`upsert_marked_pr_comment_impl`] did on GitHub. +#[derive(serde::Serialize, Clone, Copy, Debug, PartialEq, Eq)] +pub enum CommentWrite { + Created, + Updated, + Unchanged, +} + +/// The first comment `login` wrote on the PR that contains `marker`. +/// GitHub logins are case-insensitive. +fn find_marked_comment<'a>( + comments: &'a [GhApiComment], + login: &str, + marker: &str, +) -> Option<&'a GhApiComment> { + comments.iter().find(|comment| { + comment + .user + .as_ref() + .is_some_and(|user| user.login.eq_ignore_ascii_case(login)) + && comment.body.contains(marker) + }) +} + +/// Keep exactly one `marker` comment from `login` on PR `pr_number`: edit it +/// when it exists, post it when it does not, and skip the write when the +/// body is already current. Repeat runs never add a second comment. +pub fn upsert_marked_pr_comment_impl( + gh_path: &str, + repo_full_name: &str, + pr_number: u64, + login: &str, + marker: &str, + body: &str, + extended_path: &str, +) -> Result { + let comments = gh_list_issue_comments_impl(gh_path, repo_full_name, pr_number, extended_path)?; + match find_marked_comment(&comments, login, marker) { + Some(existing) if existing.body.replace("\r\n", "\n").trim_end() == body.trim_end() => { + Ok(CommentWrite::Unchanged) + } + Some(existing) => { + gh_update_issue_comment_impl(gh_path, repo_full_name, existing.id, body, extended_path)?; + Ok(CommentWrite::Updated) + } + None => { + gh_create_pr_comment_impl(gh_path, repo_full_name, pr_number, body, extended_path)?; + Ok(CommentWrite::Created) + } + } +} + const CHECK_JSON_FIELDS: &str = "name,bucket,link,startedAt,completedAt"; /// Compute elapsed seconds for a check run from `gh pr checks` timestamps. @@ -2563,4 +2728,193 @@ esac assert!(result.is_err()); } + + const STACK_MARKER: &str = ""; + const COMMENTS_ON_5: &str = "api --paginate repos/owner/repo/issues/5/comments?per_page=100"; + + #[test] + #[cfg(unix)] + fn gh_find_open_pr_for_branch_passes_the_branch_as_one_head_value() { + let bin_dir = TempDir::new().unwrap(); + let gh_path = write_fake_gh( + &bin_dir, + r#"test "$*" = "pr list --repo owner/repo --head=--web --state open --json number,title,baseRefName --limit 1" || exit 9 +echo '[{"number":12,"title":"Refactor parser","baseRefName":"feat/a"}]'"#, + ); + + let pr = + gh_find_open_pr_for_branch_impl(&gh_path, "owner/repo", "--web", "/usr/bin:/bin").unwrap(); + + assert_eq!( + pr, + Some(OpenPr { + number: 12, + title: "Refactor parser".to_string(), + base_ref_name: "feat/a".to_string(), + }) + ); + } + + #[test] + #[cfg(unix)] + fn gh_find_open_pr_for_branch_returns_none_without_an_open_pr() { + let bin_dir = TempDir::new().unwrap(); + let gh_path = write_fake_gh(&bin_dir, "echo '[]'"); + + let pr = + gh_find_open_pr_for_branch_impl(&gh_path, "owner/repo", "feat/a", "/usr/bin:/bin").unwrap(); + + assert_eq!(pr, None); + } + + #[test] + #[cfg(unix)] + fn gh_authenticated_login_reads_the_user_login() { + let bin_dir = TempDir::new().unwrap(); + let gh_path = write_fake_gh( + &bin_dir, + "test \"$*\" = \"api user --jq .login\" || exit 9\necho alice", + ); + + let login = gh_authenticated_login_impl(&gh_path, "/usr/bin:/bin").unwrap(); + + assert_eq!(login, "alice"); + } + + #[test] + #[cfg(unix)] + fn gh_list_issue_comments_reads_every_page() { + let bin_dir = TempDir::new().unwrap(); + let gh_path = write_fake_gh( + &bin_dir, + &format!( + r#"test "$*" = "{COMMENTS_ON_5}" || exit 9 +printf '%s' '[{{"id":1,"body":"a","user":{{"login":"alice"}}}}][{{"id":2,"body":"b","user":null}}]'"# + ), + ); + + let comments = gh_list_issue_comments_impl(&gh_path, "owner/repo", 5, "/usr/bin:/bin").unwrap(); + + let ids: Vec = comments.iter().map(|c| c.id).collect(); + assert_eq!(ids, vec![1, 2]); + } + + #[test] + #[cfg(unix)] + fn upsert_marked_pr_comment_edits_the_users_existing_comment() { + let bin_dir = TempDir::new().unwrap(); + let body_path = bin_dir.path().join("body"); + let gh_path = write_fake_gh( + &bin_dir, + &format!( + r#"case "$*" in + "{COMMENTS_ON_5}") + echo '[{{"id":11,"body":"old {STACK_MARKER}","user":{{"login":"bob"}}}},{{"id":12,"body":"old {STACK_MARKER}","user":{{"login":"Alice"}}}}]' ;; + "api --method PATCH repos/owner/repo/issues/comments/12 --input -") + cat > '{}' ;; + *) exit 9 ;; +esac"#, + body_path.display() + ), + ); + let body = format!("new {STACK_MARKER}"); + + let write = upsert_marked_pr_comment_impl( + &gh_path, + "owner/repo", + 5, + "alice", + STACK_MARKER, + &body, + "/usr/bin:/bin", + ) + .unwrap(); + + assert_eq!(write, CommentWrite::Updated); + let sent: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&body_path).unwrap()).unwrap(); + assert_eq!(sent, serde_json::json!({ "body": body })); + } + + #[test] + #[cfg(unix)] + fn upsert_marked_pr_comment_posts_when_the_user_has_no_marked_comment() { + let bin_dir = TempDir::new().unwrap(); + let body_path = bin_dir.path().join("body"); + let gh_path = write_fake_gh( + &bin_dir, + &format!( + r#"case "$*" in + "{COMMENTS_ON_5}") + echo '[{{"id":11,"body":"{STACK_MARKER}","user":{{"login":"bob"}}}},{{"id":12,"body":"LGTM","user":{{"login":"alice"}}}}]' ;; + "pr comment 5 --repo owner/repo --body-file -") + cat > '{}' ;; + *) exit 9 ;; +esac"#, + body_path.display() + ), + ); + let body = format!("new {STACK_MARKER}"); + + let write = upsert_marked_pr_comment_impl( + &gh_path, + "owner/repo", + 5, + "alice", + STACK_MARKER, + &body, + "/usr/bin:/bin", + ) + .unwrap(); + + assert_eq!(write, CommentWrite::Created); + assert_eq!(fs::read_to_string(&body_path).unwrap(), body); + } + + #[test] + #[cfg(unix)] + fn upsert_marked_pr_comment_leaves_an_unchanged_comment_alone() { + let bin_dir = TempDir::new().unwrap(); + let gh_path = write_fake_gh( + &bin_dir, + &format!( + r#"test "$*" = "{COMMENTS_ON_5}" || exit 9 +echo '[{{"id":12,"body":"same {STACK_MARKER}","user":{{"login":"alice"}}}}]'"# + ), + ); + + let write = upsert_marked_pr_comment_impl( + &gh_path, + "owner/repo", + 5, + "alice", + STACK_MARKER, + &format!("same {STACK_MARKER}"), + "/usr/bin:/bin", + ) + .unwrap(); + + assert_eq!(write, CommentWrite::Unchanged); + } + + #[test] + #[cfg(unix)] + fn upsert_marked_pr_comment_rejects_a_malformed_repo_without_running_gh() { + let bin_dir = TempDir::new().unwrap(); + let ran = bin_dir.path().join("ran"); + let gh_path = write_fake_gh(&bin_dir, &format!("touch '{}'", ran.display())); + + let result = upsert_marked_pr_comment_impl( + &gh_path, + "owner/repo/../../user", + 5, + "alice", + STACK_MARKER, + STACK_MARKER, + "/usr/bin:/bin", + ); + + assert!(result.is_err()); + assert!(!ran.exists()); + } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index ca6860118..a54074431 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -908,6 +908,7 @@ pub fn run() { commands::gh_reopen_pr, commands::gh_set_pr_draft, commands::gh_create_pr, + commands::gh_sync_stack_comments, commands::gh_list_pr_review_threads, commands::linear_list_teams, commands::linear_list_issues, diff --git a/src/components/RepositorySettingsContent.tsx b/src/components/RepositorySettingsContent.tsx index b180cf53c..8d49130bb 100644 --- a/src/components/RepositorySettingsContent.tsx +++ b/src/components/RepositorySettingsContent.tsx @@ -51,6 +51,7 @@ export const RepositorySettingsContent = ({ defaultAgent: string; autoPush: boolean; ignoreGeneratedAgentFiles: boolean; + postStackComments: boolean; reviewPrompt: string; reviewAgent: string; autoReviewTrigger: string; @@ -68,6 +69,7 @@ export const RepositorySettingsContent = ({ agent, autoPushSetting, ignoreGeneratedSetting, + postStackCommentsSetting, reviewPromptSetting, reviewAgentSetting, autoReviewTriggerSetting, @@ -78,6 +80,7 @@ export const RepositorySettingsContent = ({ getRepoSetting(repoPath, "default_agent"), getRepoSetting(repoPath, "auto_push"), getRepoSetting(repoPath, "ignore_generated_treq_paths"), + getRepoSetting(repoPath, "post_stack_comments"), getRepoSetting(repoPath, "review_prompt"), getRepoSetting(repoPath, "review_agent"), getRepoSetting(repoPath, "auto_review_trigger"), @@ -89,6 +92,7 @@ export const RepositorySettingsContent = ({ defaultAgent: agent || "", autoPush: autoPushSetting === "true", ignoreGeneratedAgentFiles: ignoreGeneratedSetting === "true", + postStackComments: postStackCommentsSetting !== "false", reviewPrompt: reviewPromptSetting || "", reviewAgent: reviewAgentSetting || "", autoReviewTrigger: normalizeAutoReviewTrigger(autoReviewTriggerSetting), @@ -105,6 +109,7 @@ export const RepositorySettingsContent = ({ defaultAgent: "", autoPush: false, ignoreGeneratedAgentFiles: false, + postStackComments: true, reviewPrompt: "", reviewAgent: "", autoReviewTrigger: "off", @@ -116,6 +121,7 @@ export const RepositorySettingsContent = ({ defaultAgent, autoPush, ignoreGeneratedAgentFiles, + postStackComments, reviewPrompt, reviewAgent, autoReviewTrigger, @@ -145,6 +151,7 @@ export const RepositorySettingsContent = ({ defaultAgent: string; autoPush: boolean; ignoreGeneratedAgentFiles: boolean; + postStackComments: boolean; reviewPrompt: string; reviewAgent: string; autoReviewTrigger: string; @@ -158,6 +165,7 @@ export const RepositorySettingsContent = ({ defaultAgent, autoPush, ignoreGeneratedAgentFiles, + postStackComments, reviewPrompt, reviewAgent, autoReviewTrigger, @@ -173,6 +181,8 @@ export const RepositorySettingsContent = ({ const setAutoPush = (v: boolean) => updateDraft({ autoPush: v }); const setIgnoreGeneratedAgentFiles = (v: boolean) => updateDraft({ ignoreGeneratedAgentFiles: v }); + const setPostStackComments = (v: boolean) => + updateDraft({ postStackComments: v }); const setReviewPrompt = (v: string) => updateDraft({ reviewPrompt: v }); const setReviewAgent = (v: string) => updateDraft({ reviewAgent: v }); const setAutoReviewTrigger = (v: string) => @@ -193,6 +203,11 @@ export const RepositorySettingsContent = ({ "ignore_generated_treq_paths", ignoreGeneratedAgentFiles ? "true" : "false", ), + setRepoSetting( + repoPath, + "post_stack_comments", + postStackComments ? "true" : "false", + ), setRepoSetting(repoPath, "review_prompt", reviewPrompt), setRepoSetting(repoPath, "review_agent", reviewAgent), setRepoSetting(repoPath, "auto_review_trigger", autoReviewTrigger), @@ -332,6 +347,24 @@ export const RepositorySettingsContent = ({ /> +
+
+ +

+ When Treq creates a stacked pull request, it comments on each open + pull request in the stack with the merge order. Everyone who can see + the pull request on GitHub sees the comment. +

+
+ +
+
("ready"); const draft = prType === "draft"; + const addToast = useToastStore((s) => s.addToast); const { data: defaultBranch } = useSWR( repoPath ? ["repo-default-branch", repoPath] : null, @@ -129,6 +132,10 @@ export function CreatePrForm({ void invalidateQueries(["gh-prs", repoFullName]); void invalidatePrStatuses(repoPath, head); onSuccess(prNumber); + void syncStackCommentsAfterPrCreate( + { repoPath, repoFullName, headBranch: head }, + addToast, + ); }, }); diff --git a/src/hooks/useCreateWorkspacePr.ts b/src/hooks/useCreateWorkspacePr.ts index 6599983f3..f2098003f 100644 --- a/src/hooks/useCreateWorkspacePr.ts +++ b/src/hooks/useCreateWorkspacePr.ts @@ -1,5 +1,6 @@ import { openUrl } from "@tauri-apps/plugin-opener"; import { ghCreatePr } from "../lib/api"; +import { syncStackCommentsAfterPrCreate } from "../lib/stack-comments"; import { invalidateQueries } from "../lib/swr-cache"; import { useToastStore } from "../stores/toastStore"; import { @@ -73,6 +74,16 @@ export function useCreateWorkspacePr( onClick: () => openUrl(prUrl), }, }); + if (repoPath) { + void syncStackCommentsAfterPrCreate( + { + repoPath, + repoFullName: request.repoFullName, + headBranch: request.branchName, + }, + addToast, + ); + } return number; } catch (error) { addToast({ diff --git a/src/lib/api-github.ts b/src/lib/api-github.ts index 8a7c4cfa1..d8037deea 100644 --- a/src/lib/api-github.ts +++ b/src/lib/api-github.ts @@ -3,6 +3,7 @@ import type { GhListPage, GhPullRequest, GhReviewThread, + StackCommentOutcome, } from "./api-types"; import { invoke } from "@tauri-apps/api/core"; @@ -140,3 +141,11 @@ export const ghCreatePr = ( headBranch, draft, }); + +/** Post or refresh the stack comment on every open PR in `headBranch`'s stack. */ +export const ghSyncStackComments = ( + repoPath: string, + repoFullName: string, + headBranch: string, +): Promise => + invoke("gh_sync_stack_comments", { repoPath, repoFullName, headBranch }); diff --git a/src/lib/api-types-github.ts b/src/lib/api-types-github.ts index 49e936f1e..d648a3ebf 100644 --- a/src/lib/api-types-github.ts +++ b/src/lib/api-types-github.ts @@ -73,3 +73,9 @@ export interface GhReviewThread { diff_side: string; comments: GhReviewComment[]; } + +/** What `gh_sync_stack_comments` did after a PR was created. */ +export type StackCommentOutcome = + | { status: "disabled" } + | { status: "not_stacked" } + | { status: "posted"; created: number; updated: number; unchanged: number }; diff --git a/src/lib/stack-comments.ts b/src/lib/stack-comments.ts new file mode 100644 index 000000000..4371f8ac1 --- /dev/null +++ b/src/lib/stack-comments.ts @@ -0,0 +1,28 @@ +import type { ToastState } from "../stores/toastStore"; +import { ghSyncStackComments } from "./api"; + +export interface CreatedPr { + repoPath: string; + repoFullName: string; + headBranch: string; +} + +/** + * Posts or refreshes the stack comment on a newly created PR and the rest of + * its stack. The PR already exists by then, so a failure only shows a + * warning toast. + */ +export async function syncStackCommentsAfterPrCreate( + { repoPath, repoFullName, headBranch }: CreatedPr, + addToast: ToastState["addToast"], +): Promise { + try { + await ghSyncStackComments(repoPath, repoFullName, headBranch); + } catch (error) { + addToast({ + title: "Stack comment not updated", + description: error instanceof Error ? error.message : String(error), + type: "warning", + }); + } +} diff --git a/test/integration/github-panel-detail.test.tsx b/test/integration/github-panel-detail.test.tsx index ba446b533..4d4e842ce 100644 --- a/test/integration/github-panel-detail.test.tsx +++ b/test/integration/github-panel-detail.test.tsx @@ -19,6 +19,7 @@ const api = vi.hoisted(() => ({ getPrInfoViaGh: vi.fn(), getPrChecksViaGh: vi.fn(), ghCreatePr: vi.fn(), + ghSyncStackComments: vi.fn(), getWorkspaces: vi.fn(), openOrCreateWorkspaceFromPr: vi.fn(), })); @@ -38,6 +39,7 @@ vi.mock("../../src/lib/api", async (importOriginal) => { getPrInfoViaGh: api.getPrInfoViaGh, getPrChecksViaGh: api.getPrChecksViaGh, ghCreatePr: api.ghCreatePr, + ghSyncStackComments: api.ghSyncStackComments, getWorkspaces: api.getWorkspaces, openOrCreateWorkspaceFromPr: api.openOrCreateWorkspaceFromPr, }; @@ -423,6 +425,9 @@ describe("CreatePrForm", () => { beforeEach(() => { user = userEvent.setup(); api.ghCreatePr.mockReset().mockResolvedValue(7); + api.ghSyncStackComments + .mockReset() + .mockResolvedValue({ status: "not_stacked" }); }); async function fillForm() { @@ -499,6 +504,30 @@ describe("CreatePrForm", () => { expect(onSuccess).toHaveBeenCalledWith(7); }); + it("updates the stack comment for the new PR's head branch", async () => { + render( + {}} + onCancel={() => {}} + />, + ); + + await fillForm(); + await user.click( + screen.getByRole("button", { name: /^create pull request$/i }), + ); + + await waitFor(() => { + expect(api.ghSyncStackComments).toHaveBeenCalledWith( + "/tmp/repo", + "acme/treq", + "feat/thing", + ); + }); + }); + it("shows the gh error message without an Error prefix", async () => { api.ghCreatePr.mockRejectedValue(new Error("gh: head branch not found")); render( diff --git a/test/integration/settings.test.tsx b/test/integration/settings.test.tsx index 504046587..bb30f80f2 100644 --- a/test/integration/settings.test.tsx +++ b/test/integration/settings.test.tsx @@ -4,13 +4,15 @@ import { beforeEach, describe, expect, it } from "vitest"; import { createTestRepo, openRepo } from "../utils"; import { render, screen } from "../test-utils"; import { Dashboard } from "../../src/components/Dashboard"; +import { getRepoSetting } from "../../src/lib/api"; import userEvent from "@testing-library/user-event"; describe("Settings integration", () => { let user: ReturnType; + let repoPath: string; beforeEach(() => { - const { repoPath } = createTestRepo(false); + ({ repoPath } = createTestRepo(false)); openRepo(repoPath); user = userEvent.setup(); }); @@ -61,6 +63,29 @@ describe("Settings integration", () => { expect(screen.queryByLabelText(/theme/i)).not.toBeInTheDocument(); }); + it("posts stack comments by default and saves the setting when turned off", async () => { + render(); + + await user.click(await screen.findByLabelText("Settings")); + const toggle = await screen.findByRole("switch", { + name: /post stack comments on pull requests/i, + }); + expect(toggle).toHaveAttribute("aria-checked", "true"); + + await user.click(toggle); + await user.click(screen.getByRole("button", { name: /save settings/i })); + await screen.findByText("Settings saved"); + expect(await getRepoSetting(repoPath, "post_stack_comments")).toBe("false"); + + await user.click(screen.getByRole("button", { name: "Close" })); + await user.click(await screen.findByLabelText("Settings")); + expect( + await screen.findByRole("switch", { + name: /post stack comments on pull requests/i, + }), + ).toHaveAttribute("aria-checked", "false"); + }); + it("returns to the previous page when settings is closed", async () => { render(); diff --git a/test/integration/stack-comments.test.ts b/test/integration/stack-comments.test.ts new file mode 100644 index 000000000..954afe76b --- /dev/null +++ b/test/integration/stack-comments.test.ts @@ -0,0 +1,30 @@ +// @include-parallel +import { describe, expect, it } from "vitest"; +import { + createWorkspace, + ghSyncStackComments, + setRepoSetting, +} from "../../src/lib/api"; +import { createTestRepo } from "../utils"; + +describe("gh_sync_stack_comments", () => { + it("leaves a workspace outside any stack alone", async () => { + const { repoPath } = createTestRepo(false); + await createWorkspace(repoPath, "feat/solo"); + + await expect( + ghSyncStackComments(repoPath, "acme/treq", "feat/solo"), + ).resolves.toEqual({ status: "not_stacked" }); + }); + + it("posts nothing for a stack when the repository setting is off", async () => { + const { repoPath } = createTestRepo(false); + await createWorkspace(repoPath, "feat/parent"); + await createWorkspace(repoPath, "feat/child", "feat/parent"); + await setRepoSetting(repoPath, "post_stack_comments", "false"); + + await expect( + ghSyncStackComments(repoPath, "acme/treq", "feat/child"), + ).resolves.toEqual({ status: "disabled" }); + }); +}); diff --git a/test/integration/workspace/create-pr.test.tsx b/test/integration/workspace/create-pr.test.tsx index c7c23fca1..92c6b6f5f 100644 --- a/test/integration/workspace/create-pr.test.tsx +++ b/test/integration/workspace/create-pr.test.tsx @@ -11,6 +11,7 @@ import { ghCreatePr, getPrInfoViaGh, getWorkspaceStatus, + ghSyncStackComments, pushWorkspaceToRemote, updateWorkspace, } from "../../../src/lib/api"; @@ -40,6 +41,7 @@ vi.mock("../../../src/lib/api", async (importOriginal) => { refreshPrStatuses: vi.fn(async () => undefined), getPrChecksForPr: vi.fn().mockResolvedValue(null), ghCreatePr: vi.fn().mockResolvedValue(42), + ghSyncStackComments: vi.fn(), ghListPrs: vi.fn().mockResolvedValue({ items: [], hasMore: false }), ghListIssues: vi.fn().mockResolvedValue({ items: [], hasMore: false }), pushWorkspaceToRemote: vi.fn( @@ -65,6 +67,9 @@ describe("ShowWorkspace - Create PR", () => { vi.mocked(getCachedPrInfo).mockReset().mockResolvedValue(null); vi.mocked(getPrInfoViaGh).mockReset().mockResolvedValue(null); vi.mocked(ghCreatePr).mockReset().mockResolvedValue(42); + vi.mocked(ghSyncStackComments) + .mockReset() + .mockResolvedValue({ status: "not_stacked" }); vi.mocked(openUrl).mockReset(); }); @@ -341,6 +346,42 @@ describe("ShowWorkspace - Create PR", () => { ).not.toBeInTheDocument(); }); + it("updates the stack comment after the PR is created", async () => { + await setupPushedWorkspaceWithGitHub(); + render(); + + const header = await openWorkspace("feat/create-pr"); + await user.click(await findEnabledCreatePr(header)); + + expect(await screen.findByText("Pull request created")).toBeVisible(); + await waitFor(() => { + expect(ghSyncStackComments).toHaveBeenCalledWith( + repoPath, + "acme/treq", + "feat/create-pr", + ); + }); + expect(vi.mocked(ghCreatePr).mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(ghSyncStackComments).mock.invocationCallOrder[0], + ); + }); + + it("keeps the created PR when the stack comment fails", async () => { + vi.mocked(ghSyncStackComments).mockRejectedValue( + new Error("gh exited with error: HTTP 403"), + ); + await setupPushedWorkspaceWithGitHub(); + render(); + + const header = await openWorkspace("feat/create-pr"); + await user.click(await findEnabledCreatePr(header)); + + expect(await screen.findByText("Pull request created")).toBeVisible(); + expect(await screen.findByText("Stack comment not updated")).toBeVisible(); + expect(screen.getByText("gh exited with error: HTTP 403")).toBeVisible(); + expect(screen.queryByText("Failed to create PR")).toBeNull(); + }); + it("creates a PR with the workspace title and description", async () => { const { title, description } = await setupPushedWorkspaceWithGitHub(); render(); diff --git a/web/docs/concepts/github-integration.mdx b/web/docs/concepts/github-integration.mdx index 0d6c4d139..a59f80580 100644 --- a/web/docs/concepts/github-integration.mdx +++ b/web/docs/concepts/github-integration.mdx @@ -40,6 +40,24 @@ The Review tab Commit control can **Commit and create PR** or **Commit and push* For the full create and view flow, see [Creating and Viewing Pull Requests](/docs/how-to/creating-and-viewing-pull-requests). +## Stack Comments + +A workspace belongs to a stack when its target is another workspace, or when another workspace targets it. See [Workspaces](/docs/concepts/workspaces). When Treq creates a pull request for a stacked workspace, it comments on each open pull request in that stack. The comment lists the stack newest first, marks the pull request it sits on, and ends with the base branch: + +```markdown +**Stack** (merge from the bottom up) +- #103 Add tests +- **#102 Refactor parser** ← this PR +- #101 Extract module +- `main` +``` + +A short footer links to treq.dev. Treq posts the comment through your `gh` login, so it appears under your GitHub account. Every reviewer who can open the pull request can read it, including people who do not use Treq. + +Each pull request keeps one stack comment. When you create another pull request in the stack, Treq edits the existing comments instead of adding new ones. Treq posts nothing when only one pull request in the stack is open. If a comment fails to post, the pull request still exists and Treq shows a warning. + +Stack comments are on by default. To turn them off for a repository, open Settings, switch off **Post stack comments on pull requests** on the Repository tab, and click **Save Settings**. With the setting off, Treq neither posts nor edits stack comments in that repository. + ## GitHub Panel Open **GitHub** in the workspace sidebar to reach Issues and Pull Requests. The panel uses hash routes such as `#/github/prs/open` so the `?repo=` query stays intact. diff --git a/web/docs/how-to/customizing-settings.md b/web/docs/how-to/customizing-settings.md index c97a78b1d..9f68bc9d9 100644 --- a/web/docs/how-to/customizing-settings.md +++ b/web/docs/how-to/customizing-settings.md @@ -26,6 +26,8 @@ Release builds start from the shipped default. Dev builds start with every previ **Auto-push to remote** pushes after every commit in this repository when enabled. It is off by default. Turn it on when you want each commit on the remote without a separate push step. Create PR still pushes on its own when the branch is missing remotely. +**Post stack comments on pull requests** is on by default. When Treq creates a pull request for a stacked workspace, it comments on each open pull request in the stack. Switch it off to stop posting and editing those comments. See [Stack Comments](/docs/concepts/github-integration#stack-comments). + **Branch naming pattern** builds branch names from variables. | Variable | Value | diff --git a/web/docs/security-and-privacy.md b/web/docs/security-and-privacy.md index 4dfe1e9bb..24c696550 100644 --- a/web/docs/security-and-privacy.md +++ b/web/docs/security-and-privacy.md @@ -55,3 +55,7 @@ Audit the code yourself, or follow the public history of changes. These files ar - [Connecting GitHub](/docs/how-to/connecting-github) - [CLI](/docs/reference/cli) - [Contributing](/docs/reference/contributing) + +## Stack Comments on GitHub + +When you create a pull request for a stacked workspace, Treq posts a comment on each open pull request in the stack through your `gh` login. The comment lists the stack's pull requests and links to treq.dev, and anyone who can open those pull requests can read it. To stop it, switch off **Post stack comments on pull requests** in the repository's Settings. See [Stack Comments](/docs/concepts/github-integration#stack-comments).