diff --git a/AGENTS.md b/AGENTS.md index 4ec8f680..0fd05b58 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -152,6 +152,9 @@ Layer 5 (binary): code_assistant — CLI, config, feature-gated frontend - Client and snapshot dir injected via `AgentRuntimeOptions::pull_requests`; the lifecycle sweep reads sessions' PRs through it in batches; see `docs/pull-request-view.md` +- Checkout like `gh pr checkout` (`git::pull_request`, here or in a new + worktree session) and stack rebases (`git::stack_rebase`, scratch repo) + are git-crate work the core drives ### Session Storage - One folder per session below `sessions/`, the ID is its path: diff --git a/Cargo.lock b/Cargo.lock index 4cc47604..d7838dfc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3380,6 +3380,7 @@ name = "github" version = "0.1.0" dependencies = [ "async-trait", + "base64 0.22.1", "chrono", "futures", "github", diff --git a/crates/code_assistant_core/src/pull_requests/checkout.rs b/crates/code_assistant_core/src/pull_requests/checkout.rs new file mode 100644 index 00000000..0d741ce9 --- /dev/null +++ b/crates/code_assistant_core/src/pull_requests/checkout.rs @@ -0,0 +1,136 @@ +//! Checking out a pull request: its head as git sees it, from what GitHub +//! reports (see [`git::pull_request`] for the checkout itself). + +use git::pull_request::{ForkHead, PullRequestHead}; +use github::RepoRef; +use github::model::Detail; +use std::path::PathBuf; + +/// Where a pull request is checked out. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CheckoutPlace { + /// In the session's repository: switches the branch it works on. + Here, + /// In a worktree of its own, with a session of its own. + Worktree, +} + +/// A checked-out pull request: the session working on it (a new one for a +/// worktree) and where. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct PullRequestCheckout { + pub session_id: String, + pub path: PathBuf, + pub branch: String, + /// At the pull request's head (local commits keep a branch elsewhere). + pub on_head: bool, +} + +/// The head of `detail` for a checkout in `repo`, whose remotes are +/// `(name, url)`: fetched through the remote of the pull request's +/// repository (else `origin`); a fork pushed to over the protocol that +/// remote uses, when its owner lets maintainers push. +pub fn pull_request_head( + detail: &Detail, + base: &RepoRef, + remotes: &[(String, String)], +) -> PullRequestHead { + let remote = remotes + .iter() + .find(|(_, url)| RepoRef::from_remote_url(url).as_ref() == Some(base)) + .or_else(|| remotes.iter().find(|(name, _)| name == "origin")); + let ssh = remote.is_some_and(|(_, url)| url.starts_with("git@") || url.starts_with("ssh://")); + let fork = detail.is_cross_repository.then(|| ForkHead { + owner: detail.head_repository_owner.clone().unwrap_or_default(), + push_url: detail + .head_repository + .as_ref() + .filter(|_| detail.maintainer_can_modify) + .map(|name| match ssh { + true => format!("git@{}:{name}.git", base.host), + false => format!("https://{}/{name}.git", base.host), + }), + }); + PullRequestHead { + number: detail.number, + branch: detail.head_branch.clone(), + sha: detail.head_sha.clone(), + base_branch: detail.base_branch.clone(), + remote: remote.map_or_else(|| "origin".to_owned(), |(name, _)| name.clone()), + fork, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn detail(fork: bool, maintainers: bool) -> Detail { + let mut detail: Detail = serde_json::from_value(serde_json::json!({ + "number": 8, "title": "t", "body": "", "url": "u", "author": null, + "state": "open", "is_draft": false, "mergeability": "mergeable", + "review_decision": null, "additions": 0, "deletions": 0, "changed_files": 0, + "head_branch": "topic", "head_sha": "abc1234", "head_repository": "contributor/r", + "head_repository_owner": "contributor", "is_cross_repository": false, + "base_branch": "main", "created_at": "2026-10-30T10:00:00Z", + "updated_at": "2026-10-30T10:00:00Z", "merged_at": null, "closed_at": null, + "reviewers": [], "labels": [], "checks": [], "checks_state": null, + "checks_truncated": false, + "merge_capabilities": {"merge": true, "squash": true, "rebase": true}, + "auto_merge_allowed": false, + "viewer_permissions": {"actions": [], "comment": true, "resolve": true, + "verdicts": [], "request_reviewers": true, "update_methods": [], + "labels": true, "maintainer_edits": false}, + "base_comparison": "up-to-date", "behind_by": null, + "auto_merge_enabled": false, "auto_merge_method": null, + "maintainer_can_modify": false + })) + .unwrap(); + detail.is_cross_repository = fork; + detail.maintainer_can_modify = maintainers; + detail + } + + fn remotes(urls: &[(&str, &str)]) -> Vec<(String, String)> { + urls.iter() + .map(|(name, url)| (name.to_string(), url.to_string())) + .collect() + } + + #[test] + fn fetches_through_the_remote_of_the_pull_requests_repository() { + let base = RepoRef::new("github.com", "stippi", "r"); + let head = pull_request_head( + &detail(false, false), + &base, + &remotes(&[ + ("origin", "git@github.com:me/r.git"), + ("upstream", "https://github.com/stippi/r.git"), + ]), + ); + assert_eq!(head.remote, "upstream"); + assert!(head.fork.is_none()); + assert_eq!((head.branch.as_str(), head.number), ("topic", 8)); + } + + #[test] + fn a_fork_is_pushed_to_only_when_maintainers_may() { + let base = RepoRef::new("github.com", "stippi", "r"); + let ssh = remotes(&[("origin", "git@github.com:stippi/r.git")]); + let head = pull_request_head(&detail(true, true), &base, &ssh); + let fork = head.fork.unwrap(); + assert_eq!(fork.owner, "contributor"); + assert_eq!( + fork.push_url.as_deref(), + Some("git@github.com:contributor/r.git") + ); + let head = pull_request_head(&detail(true, false), &base, &ssh); + assert_eq!(head.fork.unwrap().push_url, None); + let https = remotes(&[("origin", "https://github.com/stippi/r.git")]); + let head = pull_request_head(&detail(true, true), &base, &https); + assert_eq!( + head.fork.unwrap().push_url.as_deref(), + Some("https://github.com/contributor/r.git") + ); + } +} diff --git a/crates/code_assistant_core/src/pull_requests/mod.rs b/crates/code_assistant_core/src/pull_requests/mod.rs index 4fc80ed4..996c1f0e 100644 --- a/crates/code_assistant_core/src/pull_requests/mod.rs +++ b/crates/code_assistant_core/src/pull_requests/mod.rs @@ -14,14 +14,18 @@ //! [`UiEvent::PullRequestListChanged`]: crate::ui::UiEvent::PullRequestListChanged mod cache; +mod checkout; mod repo; mod snapshots; mod store; #[cfg(test)] mod tests; +pub use checkout::{CheckoutPlace, PullRequestCheckout}; pub use store::{PullRequestSource, PullRequests}; +pub(crate) use checkout::pull_request_head; + pub(crate) use repo::{github_remote, github_repo, own_pull_request}; use github::RepoRef; @@ -45,6 +49,7 @@ pub enum PullRequestPart { Activity, Diff, FilesViewed, + Stack, } /// Where a session's pull requests live and which one is its own. diff --git a/crates/code_assistant_core/src/pull_requests/store.rs b/crates/code_assistant_core/src/pull_requests/store.rs index d763c6d1..abe5233f 100644 --- a/crates/code_assistant_core/src/pull_requests/store.rs +++ b/crates/code_assistant_core/src/pull_requests/store.rs @@ -11,8 +11,8 @@ use futures::FutureExt; use github::model::{ Action, Activity, Checks, Detail, DiffFileContents, DiffPage, DiffStat, EditableCommentKind, FileChange, FilesViewed, LabelCandidates, ListPage, ListQuery, PullRequestState, - PullRequestUpdate, ReactionContent, ReviewSubmission, Reviewer, ReviewerCandidates, - ThreadCommentsPage, Viewer, + PullRequestUpdate, ReactionContent, ReviewSubmission, Reviewer, ReviewerCandidates, Stack, + StackAction, StackHead, StackRebase, ThreadCommentsPage, Viewer, }; use github::{Priority, PullRequestRef, RepoRef}; use serde::Serialize; @@ -36,6 +36,8 @@ const FILES_VIEWED_TTL: Duration = Duration::from_secs(15); const VIEWER_TTL: Duration = Duration::from_secs(10 * 60); const CANDIDATES_TTL: Duration = Duration::from_secs(10 * 60); const HEAD_TTL: Duration = Duration::from_secs(60); +/// T3 Code reads a stack again after a minute. +const STACK_TTL: Duration = Duration::from_secs(60); const PULL_REQUESTS_HELD: usize = 128; const LISTS_HELD: usize = 64; @@ -89,6 +91,8 @@ pub struct PullRequests { activities: Cache, diffs: Cache, files_viewed: Cache, + /// The stack a pull request is a layer of (`None`: none). + stacks: Cache>, viewers: Cache, reviewer_candidates: Cache, label_candidates: Cache, @@ -96,6 +100,29 @@ pub struct PullRequests { heads: Cache<(RepoRef, String), Option>, } +/// Rebase a stack as `plan` says, in a scratch repository. +async fn rebase(plan: StackRebase) -> Result<()> { + let git = git::GitBinary::new()?; + let layers: Vec = plan + .layers + .into_iter() + .map(|layer| git::stack_rebase::StackBranch { + number: layer.number, + branch: layer.branch, + head_sha: layer.head_sha, + }) + .collect(); + git::stack_rebase::rebase_stack( + &git, + &plan.remote_url, + &plan.auth_header, + &plan.base, + &layers, + ) + .await?; + Ok(()) +} + /// Announces a pull request's part after a background refresh. fn announce( events: &EventStream, @@ -168,6 +195,10 @@ impl PullRequests { policy(|_, _| FILES_VIEWED_TTL, PULL_REQUESTS_HELD), announce(&events, PullRequestPart::FilesViewed, itself), ), + stacks: Cache::new( + policy(|_, _| STACK_TTL, PULL_REQUESTS_HELD), + announce(&events, PullRequestPart::Stack, itself), + ), viewers: Cache::new(policy(|_, _| VIEWER_TTL, VIEWERS_HELD), None), reviewer_candidates: Cache::new(policy(|_, _| CANDIDATES_TTL, CANDIDATES_HELD), None), label_candidates: Cache::new(policy(|_, _| CANDIDATES_TTL, CANDIDATES_HELD), None), @@ -335,6 +366,13 @@ impl PullRequests { .await } + pub async fn stack(&self, pr: PullRequestRef) -> Result>> { + read(&self.stacks, pr, &self.client, |client, pr| async move { + client.stack(&pr).await + }) + .await + } + pub async fn thread_comments( &self, pr: PullRequestRef, @@ -502,6 +540,49 @@ impl PullRequests { self.written(&pr, &parts, written) } + /// Merge the stack through `pr`, or rebase the stack it tops; then + /// every layer `expected` names is read anew. + pub async fn stack_action( + &self, + pr: PullRequestRef, + stack_number: u64, + expected: Vec, + action: StackAction, + ) -> Result<()> { + let done: Result<()> = match action { + StackAction::Merge(method) => self + .interactive() + .merge_stack(&pr, stack_number, &expected, method) + .await + .map_err(Into::into), + StackAction::Rebase => { + match self + .interactive() + .stack_rebase(&pr, stack_number, &expected) + .await + { + Ok(plan) => rebase(plan).await, + Err(e) => Err(e.into()), + } + } + }; + let parts = [ + PullRequestPart::Detail, + PullRequestPart::Checks, + PullRequestPart::Activity, + PullRequestPart::Diff, + PullRequestPart::Stack, + ]; + let mut layers: Vec = expected.iter().map(|head| head.number).collect(); + layers.push(pr.number); + layers.sort_unstable(); + layers.dedup(); + for number in layers { + self.invalidate(&pr.repo.pull_request(number), &parts); + } + done + } + /// Every part of the pull request is read anew on its next read. pub fn refresh(&self, pr: &PullRequestRef) { self.reviewer_candidates.invalidate(|key| key == pr); @@ -512,6 +593,7 @@ impl PullRequests { PullRequestPart::Activity, PullRequestPart::Diff, PullRequestPart::FilesViewed, + PullRequestPart::Stack, ]; self.invalidate(pr, &parts); } @@ -545,6 +627,7 @@ impl PullRequests { PullRequestPart::Activity => self.activities.invalidate(|key| key == pr), PullRequestPart::Diff => self.diffs.invalidate(|key| key.pr == *pr), PullRequestPart::FilesViewed => self.files_viewed.invalidate(|key| key == pr), + PullRequestPart::Stack => self.stacks.invalidate(|key| key == pr), } self.events.publish_app(UiEvent::PullRequestChanged { pr: pr.clone(), diff --git a/crates/code_assistant_core/src/pull_requests/tests.rs b/crates/code_assistant_core/src/pull_requests/tests.rs index 3c061e5a..cd017f23 100644 --- a/crates/code_assistant_core/src/pull_requests/tests.rs +++ b/crates/code_assistant_core/src/pull_requests/tests.rs @@ -418,3 +418,46 @@ async fn refreshing_a_repositorys_lists_reads_them_anew_and_announces_it() { assert!(!read.refreshing, "read anew, not served stale"); assert_eq!(h.requests("PullRequestSearch"), 2); } + +#[tokio::test] +async fn a_stack_action_reads_every_layer_anew_even_when_refused() { + let mut h = harness(); + h.fake + .on_rest( + Method::Get, + "repos/stippi/code-assistant/stacks?pull_request=291", + ) + .json( + json!([{"number": 4, "url": "u", "base": "main", "pull_requests": [ + {"number": 290, "head": {"ref": "a", "sha": "aaa"}, "state": "open"}, + {"number": 291, "head": {"ref": "b", "sha": "bbb"}, "state": "open"} + ]}]), + ); + // The viewer saw stack 3: it changed since. + let expected = vec![ + github::model::StackHead { + number: 290, + head_sha: "aaa".into(), + }, + github::model::StackHead { + number: 291, + head_sha: "bbb".into(), + }, + ]; + let refused = h + .store + .stack_action(pr(), 3, expected, github::model::StackAction::Rebase) + .await + .unwrap_err(); + assert_eq!( + refused.to_string(), + "The stack changed. Refresh it before trying again." + ); + let events = h.drain_events().await; + for number in [290, 291] { + assert!(events.iter().any(|event| matches!( + event, + UiEvent::PullRequestChanged { pr, part: PullRequestPart::Stack } if pr.number == number + ))); + } +} diff --git a/crates/code_assistant_core/src/session/pull_request.rs b/crates/code_assistant_core/src/session/pull_request.rs index 20e654cd..0f77fdfc 100644 --- a/crates/code_assistant_core/src/session/pull_request.rs +++ b/crates/code_assistant_core/src/session/pull_request.rs @@ -54,23 +54,52 @@ pub enum ChecksState { impl PullRequestSnapshot { pub fn from_summary(summary: &Summary, fetched_at: SystemTime) -> Self { - let state = match summary.state { - github::model::PullRequestState::Open if summary.is_draft => PullRequestState::Draft, + Self::from_parts( + (summary.number, &summary.url, &summary.title), + summary.state, + summary.is_draft, + summary.review_decision, + summary.checks_state, + fetched_at, + ) + } + + pub fn from_detail(detail: &github::model::Detail, fetched_at: SystemTime) -> Self { + Self::from_parts( + (detail.number, &detail.url, &detail.title), + detail.state, + detail.is_draft, + detail.review_decision, + detail.checks_state, + fetched_at, + ) + } + + fn from_parts( + (number, url, title): (u64, &str, &str), + state: github::model::PullRequestState, + is_draft: bool, + review: Option, + checks: Option, + fetched_at: SystemTime, + ) -> Self { + let state = match state { + github::model::PullRequestState::Open if is_draft => PullRequestState::Draft, github::model::PullRequestState::Open => PullRequestState::Open, github::model::PullRequestState::Merged => PullRequestState::Merged, github::model::PullRequestState::Closed => PullRequestState::Closed, }; Self { - number: summary.number, - url: summary.url.clone(), - title: summary.title.clone(), + number, + url: url.to_owned(), + title: title.to_owned(), state, - review: summary.review_decision.map(|review| match review { + review: review.map(|review| match review { github::model::ReviewDecision::Approved => ReviewDecision::Approved, github::model::ReviewDecision::ChangesRequested => ReviewDecision::ChangesRequested, github::model::ReviewDecision::ReviewRequired => ReviewDecision::ReviewRequired, }), - checks: summary.checks_state.map(|checks| match checks { + checks: checks.map(|checks| match checks { github::model::ChecksState::Passing => ChecksState::Passing, github::model::ChecksState::Failing => ChecksState::Failing, github::model::ChecksState::Pending => ChecksState::Pending, diff --git a/crates/code_assistant_core/src/session/service/pull_requests.rs b/crates/code_assistant_core/src/session/service/pull_requests.rs index 7841bc0b..34e8b0c1 100644 --- a/crates/code_assistant_core/src/session/service/pull_requests.rs +++ b/crates/code_assistant_core/src/session/service/pull_requests.rs @@ -4,13 +4,18 @@ //! session lanes: a slow host never holds up session commands. use super::*; -use crate::pull_requests::{Fetched, SessionPullRequests}; +use crate::pull_requests::{ + CheckoutPlace, Fetched, PullRequestCheckout, SessionPullRequests, pull_request_head, +}; +use crate::session::pull_request::PullRequestSnapshot; use github::model::{ Action, Activity, Checks, Detail, DiffFileContents, DiffPage, DiffStat, EditableCommentKind, FileChange, FilesViewed, LabelCandidates, ListPage, ListQuery, PullRequestUpdate, - ReactionContent, ReviewSubmission, Reviewer, ReviewerCandidates, ThreadCommentsPage, Viewer, + ReactionContent, ReviewSubmission, Reviewer, ReviewerCandidates, Stack, StackAction, StackHead, + ThreadCommentsPage, Viewer, }; use github::{PullRequestRef, RepoRef}; +use std::time::SystemTime; impl SessionService { /// The GitHub repository of the session's project, its branch and that @@ -44,6 +49,93 @@ impl SessionService { .await } + /// Check `pr` out as `gh pr checkout` does: `Here` in the session's + /// directory (its branch switches), or in a worktree of its own with a + /// new session for the project on it. The session working on it knows + /// it as its pull request. + pub async fn pull_request_checkout( + &self, + session_id: String, + pr: PullRequestRef, + place: CheckoutPlace, + ) -> Result { + self.call_io(move |ctx| async move { + let (project_root, here, project, worktree) = { + let manager = ctx.manager.lock().await; + let session = manager + .get_session(&session_id) + .ok_or_else(|| anyhow!("Session {session_id} not found"))?; + let config = &session.session.config; + ( + session_project_root(&manager, &session_id)?, + session_effective_path(&manager, &session_id)?, + config.initial_project.clone(), + config.worktree_path.clone(), + ) + }; + let detail = ctx.pull_requests.detail(pr.clone()).await?.value; + let path = match place { + CheckoutPlace::Here => here, + // Worktrees hang off the repository itself. + CheckoutPlace::Worktree => project_root.clone(), + }; + let repo = git::GitRepository::open(&path).context("Failed to open git repository")?; + let remotes: Vec<(String, String)> = repo + .remotes() + .into_iter() + .map(|remote| (remote.name, remote.url)) + .collect(); + let head = pull_request_head(&detail, &pr.repo, &remotes); + let done = match place { + CheckoutPlace::Here => git::pull_request::checkout_here(&repo, &head).await, + CheckoutPlace::Worktree => { + git::pull_request::checkout_in_worktree(&repo, &head).await + } + } + .with_context(|| format!("Could not check out pull request #{}", pr.number))?; + + let mut manager = ctx.manager.lock().await; + let session_id = match place { + CheckoutPlace::Here => { + manager.set_session_worktree( + &session_id, + worktree, + Some(done.branch.clone()), + )?; + session_id + } + CheckoutPlace::Worktree => { + let mut config = manager.new_session_config(Some(project_root)); + config.initial_project = project; + let model = SessionModelConfig::new(manager.default_model_name()); + let name = format!("#{} {}", detail.number, detail.title); + let id = manager.create_session_with_config( + Some(name), + Some(config), + Some(model), + )?; + manager.set_session_worktree( + &id, + Some(done.path.clone()), + Some(done.branch.clone()), + )?; + id + } + }; + let snapshot = PullRequestSnapshot::from_detail(&detail, SystemTime::now()); + manager.update_session_lifecycle(&session_id, |lifecycle| { + lifecycle.pull_request = Some(snapshot) + })?; + Ok(PullRequestCheckout { + session_id, + path: done.path, + branch: done.branch, + on_head: done.on_head, + }) + }) + .await + } + // Reads: the held value at once (memory, else disk snapshot) and a // background refresh when it is past its time to live; with nothing // held, they wait for GitHub. @@ -89,6 +181,28 @@ impl SessionService { .await } + pub async fn pull_request_stack(&self, pr: PullRequestRef) -> Result>> { + self.call_io(move |ctx| async move { ctx.pull_requests.stack(pr).await }) + .await + } + + /// Merge the stack through `pr`, or rebase the stack it tops, unless + /// GitHub's stack differs from what `expected` saw. + pub async fn pull_request_stack_action( + &self, + pr: PullRequestRef, + stack_number: u64, + expected: Vec, + action: StackAction, + ) -> Result<()> { + self.call_io(move |ctx| async move { + ctx.pull_requests + .stack_action(pr, stack_number, expected, action) + .await + }) + .await + } + pub async fn pull_request_files_viewed( &self, pr: PullRequestRef, @@ -389,6 +503,118 @@ mod tests { assert_eq!(service.session_pull_requests(bare).await.unwrap(), None); } + /// `git` in `dir`, its output. + fn git_out(dir: &std::path::Path, args: &[&str]) -> String { + let output = std::process::Command::new("git") + .args([ + "-c", + "user.email=t@t", + "-c", + "user.name=t", + "-c", + "commit.gpgsign=false", + ]) + .args(args) + .current_dir(dir) + .output() + .unwrap(); + assert!(output.status.success(), "git {args:?} failed"); + String::from_utf8(output.stdout).unwrap().trim().to_owned() + } + + #[tokio::test] + async fn a_forks_pull_request_is_checked_out_in_a_worktree_with_a_session_of_its_own() { + let tmp = tempfile::tempdir().unwrap(); + // "GitHub": main, and the fork's head at refs/pull/291/head. + let upstream = tmp.path().join("upstream"); + std::fs::create_dir(&upstream).unwrap(); + git_out(&upstream, &["init", "-q", "-b", "main"]); + git_out(&upstream, &["commit", "-q", "--allow-empty", "-m", "init"]); + git_out( + &upstream, + &["commit", "-q", "--allow-empty", "-m", "fork work"], + ); + let head = git_out(&upstream, &["rev-parse", "HEAD"]); + git_out(&upstream, &["update-ref", "refs/pull/291/head", &head]); + git_out(&upstream, &["reset", "-q", "--hard", "HEAD~1"]); + let project = tmp.path().join("code-assistant"); + git_out( + tmp.path(), + &[ + "clone", + "-q", + upstream.to_str().unwrap(), + project.to_str().unwrap(), + ], + ); + + let fake = FakeTransport::new(); + let core = { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../github/tests/fixtures/pull_request_core.json" + ); + let text = std::fs::read_to_string(path).unwrap(); + let mut data = serde_json::from_str::(&text).unwrap()["data"].take(); + // A fork's pull request, maintainers may push. + data["repository"]["pullRequest"]["headRefOid"] = head.clone().into(); + data + }; + fake.on_graphql("PullRequestCore").data(core); + let source = PullRequestSource { + client: fake.client(), + snapshot_dir: tmp.path().join("pull-requests"), + }; + let (service, manager) = + test_service_with_pull_requests(&tmp.path().join("sessions"), source); + let config = crate::session::SessionConfig { + init_path: Some(project.clone()), + initial_project: "code-assistant".into(), + ..crate::session::SessionConfig::default() + }; + let id = service + .create_session_with_config(None, config, None) + .await + .unwrap(); + let pr = RepoRef::new("github.com", "stippi", "code-assistant").pull_request(291); + + let done = service + .pull_request_checkout(id.clone(), pr, CheckoutPlace::Worktree) + .await + .unwrap(); + assert_ne!(done.session_id, id); + assert_eq!(done.branch, "feature/pr-view"); + assert!(done.on_head); + assert_eq!(git_out(&done.path, &["rev-parse", "HEAD"]), head); + // The project itself stays where it was. + assert_eq!(git_out(&project, &["branch", "--show-current"]), "main"); + // Pushed back to the fork, like `gh pr checkout`. + assert_eq!( + git_out(&project, &["config", "branch.feature/pr-view.remote"]), + "https://github.com/contributor/code-assistant.git" + ); + + let manager = manager.lock().await; + let session = manager.get_session(&done.session_id).unwrap(); + assert_eq!( + session.session.config.worktree_path.as_ref(), + Some(&done.path) + ); + assert_eq!( + session.session.config.branch.as_deref(), + Some("feature/pr-view") + ); + assert_eq!(session.session.config.initial_project, "code-assistant"); + let lifecycle = manager.session_lifecycles().unwrap(); + assert_eq!( + lifecycle[&done.session_id] + .pull_request + .as_ref() + .map(|pr| pr.number), + Some(291) + ); + } + #[tokio::test] async fn reads_and_writes_reach_github_through_the_service() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/git/Cargo.toml b/crates/git/Cargo.toml index 6aa609af..a4bc4386 100644 --- a/crates/git/Cargo.toml +++ b/crates/git/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT" anyhow = "1.0.95" notify = { version = "7", features = ["macos_fsevent"] } serde = { version = "1.0.215", features = ["derive"] } -tokio = { version = "1.40.0", features = ["fs", "process"] } +tokio = { version = "1.40.0", features = ["fs", "process", "time"] } tracing = "0.1.40" which = "7.0" diff --git a/crates/git/src/lib.rs b/crates/git/src/lib.rs index dfb17e61..0c40265f 100644 --- a/crates/git/src/lib.rs +++ b/crates/git/src/lib.rs @@ -3,7 +3,9 @@ mod branch; mod diff; mod merged; mod patch; +pub mod pull_request; mod repository; +pub mod stack_rebase; mod types; mod watch; pub mod worktree; diff --git a/crates/git/src/pull_request.rs b/crates/git/src/pull_request.rs new file mode 100644 index 00000000..8c383706 --- /dev/null +++ b/crates/git/src/pull_request.rs @@ -0,0 +1,508 @@ +//! Checking out a pull request's head the way `gh pr checkout` does, here +//! or in a worktree of its own. +//! +//! - **Branch**: named like the head branch; a fork's head named like the +//! base repository's default (or base) branch becomes `owner/branch`. +//! - **Fetch**: a branch of the repository itself from its remote +//! (`+refs/heads/b:refs/remotes/origin/b`); a fork's head as +//! `refs/pull/N/head` from the base repository, no remote added. +//! - **Upstream**: the remote's branch; for a fork, its URL and branch +//! when maintainers may push to it, else the base's `refs/pull/N/head`. +//! - **Updating**: an existing branch only fast-forwards; local commits +//! stay (then the checkout is not on the pull request's head). +//! +//! Here, tracked changes refuse the switch (as in T3 Code). In a worktree, +//! a branch already checked out in another worktree is reused there; one +//! checked out in the repository itself is refused. + +use crate::GitRepository; +use crate::worktree::{list_worktrees, suggest_worktree_path}; +use anyhow::{Context, Result, bail}; +use std::path::{Path, PathBuf}; + +/// The head of a pull request, as GitHub reports it. +#[derive(Clone, Debug)] +pub struct PullRequestHead { + pub number: u64, + /// The branch in the head repository. + pub branch: String, + pub sha: Option, + /// The pull request's base branch. + pub base_branch: String, + /// The base repository's remote here (e.g. `origin`). + pub remote: String, + /// Set when the head lives in a fork. + pub fork: Option, +} + +#[derive(Clone, Debug)] +pub struct ForkHead { + pub owner: String, + /// The fork's clone URL, when its owner lets maintainers push. + pub push_url: Option, +} + +/// Where the pull request was checked out. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct CheckedOut { + pub path: PathBuf, + pub branch: String, + /// The checkout is at the pull request's head (local commits or a + /// diverged branch keep it elsewhere). + pub on_head: bool, +} + +/// The local branch for `head`, as `gh pr checkout` names it. +pub fn local_branch(head: &PullRequestHead, default_branch: Option<&str>) -> String { + match &head.fork { + Some(fork) + if head.branch == head.base_branch || Some(head.branch.as_str()) == default_branch => + { + format!("{}/{}", fork.owner, head.branch) + } + _ => head.branch.clone(), + } +} + +/// What to bring the branch to, and what it tracks. +struct Fetched { + target: String, + upstream: Option<(String, String)>, +} + +async fn git(repo: &GitRepository, dir: &Path, args: &[&str]) -> Result { + repo.git.run(dir, args).await +} + +async fn has_branch(repo: &GitRepository, branch: &str) -> bool { + let full = format!("refs/heads/{branch}"); + git( + repo, + repo.workdir(), + &["rev-parse", "--verify", "--quiet", &full], + ) + .await + .is_ok() +} + +/// The remote's default branch (`origin/HEAD`), when known. +async fn default_branch(repo: &GitRepository, remote: &str) -> Option { + let head = format!("refs/remotes/{remote}/HEAD"); + let name = git( + repo, + repo.workdir(), + &["symbolic-ref", "--quiet", "--short", &head], + ) + .await + .ok()?; + name.strip_prefix(&format!("{remote}/")).map(str::to_owned) +} + +/// Fetch the head: a branch of the repository itself as its remote +/// branch (the pull request's ref when that fails), a fork's head as the +/// pull request's ref. +async fn fetch(repo: &GitRepository, head: &PullRequestHead) -> Result { + let dir = repo.workdir(); + let pull_ref = format!("refs/pull/{}/head", head.number); + if head.fork.is_none() { + let refspec = format!( + "+refs/heads/{0}:refs/remotes/{1}/{0}", + head.branch, head.remote + ); + let fetched = git( + repo, + dir, + &[ + "fetch", + "--quiet", + "--no-tags", + "--", + &head.remote, + &refspec, + ], + ) + .await; + if fetched.is_ok() { + return Ok(Fetched { + target: format!("refs/remotes/{}/{}", head.remote, head.branch), + upstream: Some((head.remote.clone(), format!("refs/heads/{}", head.branch))), + }); + } + } + git( + repo, + dir, + &[ + "fetch", + "--quiet", + "--no-tags", + "--", + &head.remote, + &pull_ref, + ], + ) + .await + .with_context(|| format!("Could not fetch pull request #{}", head.number))?; + let target = git(repo, dir, &["rev-parse", "FETCH_HEAD"]).await?; + let upstream = head.fork.as_ref().map(|fork| match &fork.push_url { + Some(url) => (url.clone(), format!("refs/heads/{}", head.branch)), + None => (head.remote.clone(), pull_ref.clone()), + }); + Ok(Fetched { target, upstream }) +} + +/// Track the upstream like `gh` does: by remote name or URL. +async fn set_upstream(repo: &GitRepository, branch: &str, fetched: &Fetched) -> Result<()> { + let Some((remote, merge)) = &fetched.upstream else { + return Ok(()); + }; + let dir = repo.workdir(); + git( + repo, + dir, + &["config", &format!("branch.{branch}.remote"), remote], + ) + .await?; + git( + repo, + dir, + &["config", &format!("branch.{branch}.merge"), merge], + ) + .await?; + Ok(()) +} + +async fn at(repo: &GitRepository, dir: &Path, sha: Option<&str>) -> bool { + let Some(sha) = sha else { + return false; + }; + git(repo, dir, &["rev-parse", "HEAD"]) + .await + .is_ok_and(|head| head.eq_ignore_ascii_case(sha)) +} + +async fn is_ancestor(repo: &GitRepository, ancestor: &str, of: &str) -> bool { + git( + repo, + repo.workdir(), + &["merge-base", "--is-ancestor", ancestor, of], + ) + .await + .is_ok() +} + +/// Check the pull request out in the repository itself. Tracked changes +/// refuse it. +pub async fn checkout_here(repo: &GitRepository, head: &PullRequestHead) -> Result { + let dir = repo.workdir(); + let changes = git( + repo, + dir, + &["status", "--porcelain", "--untracked-files=no"], + ) + .await?; + if !changes.trim().is_empty() { + bail!( + "This repository has uncommitted changes. Commit or stash them before checking out \ + pull request #{} here, or check it out in its own worktree.", + head.number + ); + } + let default = default_branch(repo, &head.remote).await; + let branch = local_branch(head, default.as_deref()); + let fetched = fetch(repo, head).await?; + if has_branch(repo, &branch).await { + git(repo, dir, &["checkout", "--quiet", &branch, "--"]).await?; + git( + repo, + dir, + &["merge", "--ff-only", "--quiet", &fetched.target], + ) + .await + .with_context(|| { + format!("The branch {branch} has commits the pull request does not have") + })?; + } else { + git( + repo, + dir, + &["checkout", "--quiet", "-b", &branch, &fetched.target, "--"], + ) + .await?; + } + set_upstream(repo, &branch, &fetched).await?; + Ok(CheckedOut { + path: dir.to_path_buf(), + on_head: at(repo, dir, head.sha.as_deref()).await, + branch, + }) +} + +/// Check the pull request out in a worktree of its own: the one that has +/// its branch already, else a new one beside the repository. +pub async fn checkout_in_worktree( + repo: &GitRepository, + head: &PullRequestHead, +) -> Result { + let default = default_branch(repo, &head.remote).await; + let branch = local_branch(head, default.as_deref()); + let full = format!("refs/heads/{branch}"); + let holder = list_worktrees(repo) + .await? + .into_iter() + .find(|worktree| worktree.branch.as_deref() == Some(full.as_str())); + if holder.as_ref().is_some_and(|worktree| worktree.is_main) { + bail!( + "The branch {branch} is checked out in the repository itself. Check the pull \ + request out there, or switch the repository to another branch first." + ); + } + let fetched = fetch(repo, head).await?; + let path = match holder { + Some(worktree) => { + // Moved on only when nothing there would be lost. + let clean = git(repo, &worktree.path, &["status", "--porcelain"]) + .await + .is_ok_and(|status| status.trim().is_empty()); + if clean && is_ancestor(repo, &full, &fetched.target).await { + git( + repo, + &worktree.path, + &["merge", "--ff-only", "--quiet", &fetched.target], + ) + .await?; + } + worktree.path + } + None => { + if !has_branch(repo, &branch).await { + git( + repo, + repo.workdir(), + &["branch", "--", &branch, &fetched.target], + ) + .await?; + } else if is_ancestor(repo, &full, &fetched.target).await { + git( + repo, + repo.workdir(), + &["branch", "--force", "--", &branch, &fetched.target], + ) + .await?; + } + let path = free_path(suggest_worktree_path(repo.workdir(), &branch)); + crate::worktree::create_worktree_for_branch(&repo.git, repo.workdir(), &path, &branch) + .await + .context("Failed to create the worktree")? + } + }; + set_upstream(repo, &branch, &fetched).await?; + Ok(CheckedOut { + on_head: at(repo, &path, head.sha.as_deref()).await, + path, + branch, + }) +} + +/// `path`, or the first of `path-2`, `path-3`, … that does not exist. +fn free_path(path: PathBuf) -> PathBuf { + let mut candidate = path.clone(); + let mut n = 2; + while candidate.exists() { + let mut name = path.file_name().unwrap_or_default().to_os_string(); + name.push(format!("-{n}")); + candidate = path.with_file_name(name); + n += 1; + } + candidate +} + +#[cfg(test)] +mod tests { + use super::*; + use std::process::Command; + + fn run(dir: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .args([ + "-c", + "user.email=t@t", + "-c", + "user.name=T", + "-c", + "commit.gpgsign=false", + ]) + .args(args) + .current_dir(dir) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap().trim().to_owned() + } + + fn commit(dir: &Path, file: &str) -> String { + std::fs::write(dir.join(file), file).unwrap(); + run(dir, &["add", file]); + run(dir, &["commit", "-q", "-m", file]); + run(dir, &["rev-parse", "HEAD"]) + } + + /// A "GitHub" repository with `main`, a branch `feature` (#7) and a + /// fork's head at `refs/pull/8/head`, and a clone of it. + struct Setup { + _dir: tempfile::TempDir, + clone: PathBuf, + feature: String, + fork: String, + } + + fn setup() -> Setup { + let dir = tempfile::TempDir::new().unwrap(); + let upstream = dir.path().join("upstream"); + std::fs::create_dir(&upstream).unwrap(); + run(&upstream, &["init", "-q", "-b", "main"]); + commit(&upstream, "a"); + run(&upstream, &["checkout", "-q", "-b", "feature"]); + let feature = commit(&upstream, "b"); + run(&upstream, &["checkout", "-q", "main"]); + run(&upstream, &["checkout", "-q", "-b", "fork-work"]); + let fork = commit(&upstream, "c"); + run(&upstream, &["update-ref", "refs/pull/8/head", &fork]); + run(&upstream, &["checkout", "-q", "main"]); + run(&upstream, &["branch", "-q", "-D", "fork-work"]); + let clone = dir.path().join("clone"); + run( + dir.path(), + &[ + "clone", + "-q", + upstream.to_str().unwrap(), + clone.to_str().unwrap(), + ], + ); + Setup { + _dir: dir, + clone, + feature, + fork, + } + } + + fn own_head(sha: &str) -> PullRequestHead { + PullRequestHead { + number: 7, + branch: "feature".into(), + sha: Some(sha.into()), + base_branch: "main".into(), + remote: "origin".into(), + fork: None, + } + } + + fn fork_head(sha: &str, branch: &str, push_url: Option<&str>) -> PullRequestHead { + PullRequestHead { + number: 8, + branch: branch.into(), + sha: Some(sha.into()), + base_branch: "main".into(), + remote: "origin".into(), + fork: Some(ForkHead { + owner: "contributor".into(), + push_url: push_url.map(str::to_owned), + }), + } + } + + #[tokio::test] + async fn checks_out_a_branch_of_the_repository_here_tracking_its_remote() { + let setup = setup(); + let repo = GitRepository::open(&setup.clone).unwrap(); + let done = checkout_here(&repo, &own_head(&setup.feature)) + .await + .unwrap(); + assert_eq!(done.branch, "feature"); + assert!(done.on_head); + assert_eq!( + run(&setup.clone, &["rev-parse", "--abbrev-ref", "@{u}"]), + "origin/feature" + ); + } + + #[tokio::test] + async fn checks_out_a_forks_head_from_the_pull_request_ref() { + let setup = setup(); + let repo = GitRepository::open(&setup.clone).unwrap(); + // The fork's branch is named like the base's default branch. + let head = fork_head( + &setup.fork, + "main", + Some("https://github.com/contributor/r.git"), + ); + let done = checkout_here(&repo, &head).await.unwrap(); + assert_eq!(done.branch, "contributor/main"); + assert!(done.on_head); + let config = |key: &str| run(&setup.clone, &["config", key]); + assert_eq!( + config("branch.contributor/main.remote"), + "https://github.com/contributor/r.git" + ); + assert_eq!(config("branch.contributor/main.merge"), "refs/heads/main"); + // Without maintainer edits it tracks the pull request's ref. + let head = fork_head(&setup.fork, "topic", None); + let done = checkout_here(&repo, &head).await.unwrap(); + assert_eq!(done.branch, "topic"); + assert_eq!(config("branch.topic.merge"), "refs/pull/8/head"); + } + + #[tokio::test] + async fn tracked_changes_refuse_a_checkout_here() { + let setup = setup(); + std::fs::write(setup.clone.join("a"), "changed").unwrap(); + let repo = GitRepository::open(&setup.clone).unwrap(); + let refused = checkout_here(&repo, &own_head(&setup.feature)) + .await + .unwrap_err(); + assert!( + refused.to_string().contains("uncommitted changes"), + "{refused}" + ); + } + + #[tokio::test] + async fn a_worktree_is_made_once_and_reused() { + let setup = setup(); + let repo = GitRepository::open(&setup.clone).unwrap(); + let first = checkout_in_worktree(&repo, &own_head(&setup.feature)) + .await + .unwrap(); + assert!(first.on_head); + assert_ne!(first.path, setup.clone); + assert_eq!(run(&first.path, &["branch", "--show-current"]), "feature"); + let again = checkout_in_worktree(&repo, &own_head(&setup.feature)) + .await + .unwrap(); + assert_eq!(again.path, first.path); + // The repository itself keeps its branch. + assert_eq!(run(&setup.clone, &["branch", "--show-current"]), "main"); + } + + #[tokio::test] + async fn a_branch_checked_out_in_the_repository_is_not_taken_into_a_worktree() { + let setup = setup(); + let repo = GitRepository::open(&setup.clone).unwrap(); + checkout_here(&repo, &own_head(&setup.feature)) + .await + .unwrap(); + let refused = checkout_in_worktree(&repo, &own_head(&setup.feature)) + .await + .unwrap_err(); + assert!( + refused.to_string().contains("repository itself"), + "{refused}" + ); + } +} diff --git a/crates/git/src/stack_rebase.rs b/crates/git/src/stack_rebase.rs new file mode 100644 index 00000000..e36a9970 --- /dev/null +++ b/crates/git/src/stack_rebase.rs @@ -0,0 +1,337 @@ +//! Rebasing a stack of branches, ported from T3 Code's +//! `cascadeRebaseStack`: in a scratch repository (the user's checkouts +//! stay untouched) each layer is replayed onto the rebased layer below, +//! bottom to top, and pushed with a lease on the head it had. A layer that +//! conflicts stops the cascade; the layers pushed before it stay. +//! +//! The credential travels in the environment (`GIT_CONFIG_*`), never on a +//! command line. + +use crate::GitBinary; +use std::path::{Path, PathBuf}; +use std::time::Duration; + +/// Each git command may take this long. +const STEP_TIMEOUT: Duration = Duration::from_secs(120); + +/// A branch of the stack and the head it is expected at. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct StackBranch { + pub number: u64, + pub branch: String, + pub head_sha: String, +} + +/// Where a cascade stopped, in T3 Code's words. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum StackRebaseError { + Conflict { + number: u64, + completed: usize, + }, + Git { + step: &'static str, + number: u64, + completed: usize, + detail: String, + }, +} + +impl std::fmt::Display for StackRebaseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + StackRebaseError::Conflict { number, completed } => write!( + f, + "PR #{number} conflicts with the layer below it. {completed} layers were \ + rebased; resolve #{number} with `gh stack rebase` and push." + ), + StackRebaseError::Git { + step, + number, + completed, + .. + } => write!( + f, + "Stack rebase stopped at PR #{number} while {step}. {completed} layers were \ + rebased." + ), + } + } +} + +impl std::error::Error for StackRebaseError {} + +/// A scratch directory, removed when dropped. +struct Scratch(PathBuf); + +impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +struct Cascade<'a> { + git: &'a GitBinary, + dir: &'a Path, + /// `GIT_CONFIG_KEY_n` / `VALUE_n` pairs. + config: Vec<(String, String)>, +} + +impl Cascade<'_> { + /// Run `git args`; its stdout, trimmed. + async fn run(&self, args: &[&str]) -> anyhow::Result { + let mut command = self.git.command(self.dir); + command.env("GIT_CONFIG_COUNT", self.config.len().to_string()); + for (ix, (key, value)) in self.config.iter().enumerate() { + command.env(format!("GIT_CONFIG_KEY_{ix}"), key); + command.env(format!("GIT_CONFIG_VALUE_{ix}"), value); + } + command.args(args); + let output = tokio::time::timeout(STEP_TIMEOUT, command.output()) + .await + .map_err(|_| anyhow::anyhow!("git {} timed out", args.join(" ")))??; + if !output.status.success() { + anyhow::bail!( + "git {} failed: {}", + args.first().copied().unwrap_or_default(), + String::from_utf8_lossy(&output.stderr).trim() + ); + } + Ok(String::from_utf8_lossy(&output.stdout).trim().to_owned()) + } +} + +/// Rebase `layers` (bottom to top) of the repository at `remote_url` onto +/// `base`: each layer onto the rebased layer below, pushed when it moved. +/// `auth_header` is an `http.extraheader` value for the remote. +pub async fn rebase_stack( + git: &GitBinary, + remote_url: &str, + auth_header: &str, + base: &str, + layers: &[StackBranch], +) -> Result<(), StackRebaseError> { + let Some(first) = layers.first() else { + return Ok(()); + }; + let failed = |step: &'static str, number: u64, completed: usize| { + move |error: anyhow::Error| StackRebaseError::Git { + step, + number, + completed, + detail: format!("{error:#}"), + } + }; + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + let scratch = Scratch(std::env::temp_dir().join(format!( + "code-assistant-stack-rebase-{}-{nanos}", + std::process::id() + ))); + std::fs::create_dir_all(&scratch.0) + .map_err(|e| failed("preparing", first.number, 0)(e.into()))?; + let mut cascade = Cascade { + git, + dir: &scratch.0, + config: vec![( + format!("http.{remote_url}.extraheader"), + auth_header.to_owned(), + )], + }; + cascade + .run(&["init", "--quiet"]) + .await + .map_err(failed("preparing", first.number, 0))?; + // Rebased commits need a committer; the user's own when configured. + if cascade.run(&["config", "user.email"]).await.is_err() { + cascade.config.extend([ + ("user.name".to_owned(), "code-assistant".to_owned()), + ( + "user.email".to_owned(), + "noreply@code-assistant.invalid".to_owned(), + ), + ]); + } + cascade + .run(&["remote", "add", "origin", remote_url]) + .await + .map_err(failed("preparing", first.number, 0))?; + let mut refspecs = vec![format!("+refs/heads/{base}:refs/remotes/origin/{base}")]; + refspecs.extend( + layers + .iter() + .map(|l| format!("+refs/heads/{0}:refs/remotes/origin/{0}", l.branch)), + ); + let mut fetch = vec!["fetch", "--quiet", "--no-tags", "origin"]; + fetch.extend(refspecs.iter().map(String::as_str)); + cascade + .run(&fetch) + .await + .map_err(failed("fetching", first.number, 0))?; + + let base_ref = format!("refs/remotes/origin/{base}"); + let (mut parent_old, mut parent_new) = (base_ref.clone(), base_ref.clone()); + for (completed, layer) in layers.iter().enumerate() { + let number = layer.number; + let upstream = match completed { + 0 => cascade + .run(&["merge-base", &base_ref, &layer.head_sha]) + .await + .map_err(failed("reading the fork point", number, completed))?, + _ => parent_old.clone(), + }; + cascade + .run(&["checkout", "--quiet", "--detach", &layer.head_sha]) + .await + .map_err(failed("checking out", number, completed))?; + if cascade + .run(&["rebase", "--quiet", "--onto", &parent_new, &upstream]) + .await + .is_err() + { + let _ = cascade.run(&["rebase", "--abort"]).await; + return Err(StackRebaseError::Conflict { number, completed }); + } + let rebased = cascade + .run(&["rev-parse", "HEAD"]) + .await + .map_err(failed("rebasing", number, completed))?; + if rebased != layer.head_sha { + let lease = format!( + "--force-with-lease=refs/heads/{}:{}", + layer.branch, layer.head_sha + ); + let target = format!("{rebased}:refs/heads/{}", layer.branch); + cascade + .run(&["push", "--quiet", &lease, "origin", &target]) + .await + .map_err(failed("pushing", number, completed))?; + } + parent_old = layer.head_sha.clone(); + parent_new = rebased; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::process::Command; + + fn run(dir: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .args([ + "-c", + "user.email=t@t", + "-c", + "user.name=T", + "-c", + "commit.gpgsign=false", + ]) + .args(args) + .current_dir(dir) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap().trim().to_owned() + } + + fn commit(dir: &Path, file: &str, text: &str) -> String { + std::fs::write(dir.join(file), text).unwrap(); + run(dir, &["add", file]); + run(dir, &["commit", "-q", "-m", file]); + run(dir, &["rev-parse", "HEAD"]) + } + + /// A bare "GitHub" with `main`, `one` on it and `two` on `one`; then + /// `main` moves on. The work happens in `work`, pushed to the remote. + fn stacked() -> (tempfile::TempDir, PathBuf, Vec) { + let dir = tempfile::TempDir::new().unwrap(); + let remote = dir.path().join("remote.git"); + let work = dir.path().join("work"); + run( + dir.path(), + &[ + "init", + "-q", + "--bare", + "-b", + "main", + remote.to_str().unwrap(), + ], + ); + std::fs::create_dir(&work).unwrap(); + run(&work, &["init", "-q", "-b", "main"]); + commit(&work, "base", "base"); + run(&work, &["checkout", "-q", "-b", "one"]); + let one = commit(&work, "one", "one"); + run(&work, &["checkout", "-q", "-b", "two"]); + let two = commit(&work, "two", "two"); + run(&work, &["checkout", "-q", "main"]); + commit(&work, "later", "later"); + run( + &work, + &["push", "-q", remote.to_str().unwrap(), "main", "one", "two"], + ); + let layers = vec![ + StackBranch { + number: 1, + branch: "one".into(), + head_sha: one, + }, + StackBranch { + number: 2, + branch: "two".into(), + head_sha: two, + }, + ]; + (dir, remote, layers) + } + + #[tokio::test] + async fn every_layer_lands_on_the_rebased_layer_below() { + let (_dir, remote, layers) = stacked(); + let git = GitBinary::new().unwrap(); + let url = remote.to_str().unwrap(); + rebase_stack(&git, url, "AUTHORIZATION: basic x", "main", &layers) + .await + .unwrap(); + let parent = |branch: &str| run(&remote, &["rev-parse", &format!("{branch}~1")]); + let tip = |branch: &str| run(&remote, &["rev-parse", branch]); + assert_eq!(parent("one"), tip("main")); + assert_eq!(parent("two"), tip("one")); + assert_ne!(tip("one"), layers[0].head_sha); + } + + #[tokio::test] + async fn a_branch_pushed_to_meanwhile_is_not_overwritten() { + let (dir, remote, layers) = stacked(); + // Someone pushed to `one` since the stack was read. + let work = dir.path().join("work"); + run(&work, &["checkout", "-q", "one"]); + commit(&work, "extra", "extra"); + run(&work, &["push", "-q", remote.to_str().unwrap(), "one"]); + let git = GitBinary::new().unwrap(); + let stopped = rebase_stack(&git, remote.to_str().unwrap(), "x: y", "main", &layers) + .await + .unwrap_err(); + assert!( + matches!( + stopped, + StackRebaseError::Git { + step: "pushing", + number: 1, + completed: 0, + .. + } + ), + "{stopped:?}" + ); + } +} diff --git a/crates/github/Cargo.toml b/crates/github/Cargo.toml index 2de633c6..6b1675ee 100644 --- a/crates/github/Cargo.toml +++ b/crates/github/Cargo.toml @@ -26,6 +26,7 @@ thiserror = "2.0" tokio = { version = "1.48", features = ["process", "sync", "time"] } tracing = "0.1" urlencoding = "2.1" +base64 = "0.22" [dev-dependencies] github = { path = ".", features = ["test-utils"] } diff --git a/crates/github/src/convert/detail.rs b/crates/github/src/convert/detail.rs index babb175d..e8151dae 100644 --- a/crates/github/src/convert/detail.rs +++ b/crates/github/src/convert/detail.rs @@ -56,6 +56,7 @@ pub(crate) fn viewer_permissions(access: &ViewerAccess) -> ViewerPermissions { } if access.can_write { actions.push(PullRequestAction::Merge); + actions.push(PullRequestAction::ApproveWorkflows); if access.auto_merge_allowed && !access.can_merge_now { actions.push(PullRequestAction::EnableAutoMerge); } @@ -391,6 +392,7 @@ pub(crate) fn core(data: CoreData, host: &str) -> Option<(Detail, bool)> { .as_ref() .and_then(|r| merge_method(r.merge_method.as_deref())), maintainer_can_modify: pr.maintainer_can_modify, + workflow_approvals: None, }; Some((detail, more_checks)) } @@ -529,6 +531,7 @@ mod tests { [ PullRequestAction::DeleteBranch, PullRequestAction::Merge, + PullRequestAction::ApproveWorkflows, PullRequestAction::EnableAutoMerge, PullRequestAction::DisableAutoMerge, PullRequestAction::Ready, diff --git a/crates/github/src/convert/list.rs b/crates/github/src/convert/list.rs index 3fc5285e..764d7aca 100644 --- a/crates/github/src/convert/list.rs +++ b/crates/github/src/convert/list.rs @@ -160,6 +160,7 @@ fn entry(row: RawRow) -> ListEntry { .any(|r| trimmed(r.login.as_deref()).is_none() && trimmed(r.slug.as_deref()).is_some()), labels: labels(&row.labels), checks_state: checks_state(&rollup_states(&row.commits)), + stack: None, } } diff --git a/crates/github/src/error.rs b/crates/github/src/error.rs index 139c66e1..82423c2b 100644 --- a/crates/github/src/error.rs +++ b/crates/github/src/error.rs @@ -103,6 +103,10 @@ pub enum Error { #[error("The named subject does not belong to this pull request.")] SubjectScope, + /// Not asked of GitHub: what it would do is not what was meant. + #[error("{0}")] + Refused(String), + #[error("{}", branch_deletion_message(*.0))] BranchDeletionRefused(BranchDeletionRefusal), diff --git a/crates/github/src/graphql/mod.rs b/crates/github/src/graphql/mod.rs index f09ccfc2..3c6eb936 100644 --- a/crates/github/src/graphql/mod.rs +++ b/crates/github/src/graphql/mod.rs @@ -19,6 +19,7 @@ const DOCUMENTS: &[&str] = &[ include_str!("batches.graphql"), include_str!("mutations.graphql"), include_str!("heads.graphql"), + include_str!("stacks.graphql"), ]; /// Every operation by name, ready to send. @@ -114,6 +115,7 @@ mod tests { "MergePullRequest", "PullRequestActionState", "PullRequestActivity", + "PullRequestBranchAccess", "PullRequestBranchToDelete", "PullRequestCheckContexts", "PullRequestCommitStats", @@ -123,6 +125,7 @@ mod tests { "PullRequestNodeId", "PullRequestSearch", "PullRequestSourceBranch", + "PullRequestStackMembership", "PullRequestStats", "PullRequestSubject", "PullRequestSummary", @@ -146,7 +149,13 @@ mod tests { assert!(OPERATIONS.contains_key(name), "{name} is not defined"); } assert_eq!(OPERATIONS.len(), sent.len(), "an operation is never sent"); - for batched in ["PullRequestSummary", "PullRequestStats", "SetFileViewed"] { + for batched in [ + "PullRequestSummary", + "PullRequestStats", + "SetFileViewed", + "PullRequestStackMembership", + "PullRequestBranchAccess", + ] { alias::aliased_operation(operation(batched), 2); } } diff --git a/crates/github/src/graphql/stacks.graphql b/crates/github/src/graphql/stacks.graphql new file mode 100644 index 00000000..3ea99e43 --- /dev/null +++ b/crates/github/src/graphql/stacks.graphql @@ -0,0 +1,29 @@ +# Stacked pull requests (GitHub's stacks): which stack a pull request is a layer of, and whether +# the viewer may push to a layer's branch before rebasing the stack. + +query PullRequestStackMembership($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + stack { + number + size + baseRefName + } + stackEntry { + position + } + } + } +} + +query PullRequestBranchAccess($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + headRepository { + viewerPermission + } + # A fork's branch is writable without repository access when its owner allows it. + maintainerCanModify + } + } +} diff --git a/crates/github/src/model/checks.rs b/crates/github/src/model/checks.rs index f9c51775..b87c8cbb 100644 --- a/crates/github/src/model/checks.rs +++ b/crates/github/src/model/checks.rs @@ -43,4 +43,7 @@ pub struct Checks { pub head_sha: Option, pub checks: Vec, pub checks_state: Option, + /// See `Detail::workflow_approvals`. + #[serde(default)] + pub workflow_approvals: Option, } diff --git a/crates/github/src/model/mod.rs b/crates/github/src/model/mod.rs index 450e46b0..bee5d940 100644 --- a/crates/github/src/model/mod.rs +++ b/crates/github/src/model/mod.rs @@ -7,6 +7,7 @@ mod common; mod files; mod pull_request; mod review; +mod stack; pub use activity::*; pub use checks::*; @@ -14,3 +15,4 @@ pub use common::*; pub use files::*; pub use pull_request::*; pub use review::*; +pub use stack::*; diff --git a/crates/github/src/model/pull_request.rs b/crates/github/src/model/pull_request.rs index 44123608..340ab66d 100644 --- a/crates/github/src/model/pull_request.rs +++ b/crates/github/src/model/pull_request.rs @@ -77,6 +77,9 @@ pub struct ListEntry { pub has_team_review_request: bool, pub labels: Vec