diff --git a/crates/core/src/pull_request.rs b/crates/core/src/pull_request.rs index 0b706fab5..e2892bc2e 100644 --- a/crates/core/src/pull_request.rs +++ b/crates/core/src/pull_request.rs @@ -188,6 +188,60 @@ pub fn shown(links: &[ThreadPullRequestLink], key: &PullRequestKey) -> bool { }) } +/// GitHub's merge methods, in the order its merge button lists them. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum PullRequestMergeMethod { + Merge, + Squash, + Rebase, +} + +/// How a merge, a branch update or auto-merge may reach a pull request the thread shows. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PullRequestStackRoute { + /// In no native stack: it merges as one pull request. + Single, + /// Layer `index` (from 1, bottom first) of a native stack of `layers`, which GitHub merges + /// with the layers below it. + Layer { index: u32, layers: u32 }, + /// Whether it is in a native stack is not known yet. + Unknown, +} + +/// The route, and the native stack layer the pull request is when it is one. +pub fn stack_route<'a>( + links: &'a [ThreadPullRequestLink], + key: &PullRequestKey, +) -> (PullRequestStackRoute, Option<&'a PullRequestStackLayer>) { + let visible = || links.iter().filter(|link| link.visible()); + let layer = visible().find_map(|link| match &link.stack { + PullRequestStackState::Native(stack) + if link.key.host == key.host && link.key.repository == key.repository => + { + let index = stack + .layers + .iter() + .position(|layer| layer.number == key.number)?; + Some(( + PullRequestStackRoute::Layer { + index: index as u32 + 1, + layers: stack.layers.len() as u32, + }, + &stack.layers[index], + )) + } + _ => None, + }); + match (layer, visible().find(|link| link.key == *key)) { + (Some((route, layer)), _) => (route, Some(layer)), + (None, Some(link)) if link.stack == PullRequestStackState::None => { + (PullRequestStackRoute::Single, None) + } + _ => (PullRequestStackRoute::Unknown, None), + } +} + /// The visible links a watch holds, which keep their thread waiting between wakes. pub fn watched(links: &[ThreadPullRequestLink]) -> impl Iterator { links diff --git a/crates/core/src/pull_request_watch.rs b/crates/core/src/pull_request_watch.rs index 31a40f0df..62d0fc9a9 100644 --- a/crates/core/src/pull_request_watch.rs +++ b/crates/core/src/pull_request_watch.rs @@ -82,7 +82,7 @@ pub enum CheckStatus { impl CheckStatus { /// "action-required" is a finished check that needs someone, so the agent hears about it. - fn failed(self) -> bool { + pub fn failed(self) -> bool { matches!(self, Self::Failure | Self::Cancelled | Self::ActionRequired) } } diff --git a/crates/core/src/settings.rs b/crates/core/src/settings.rs index 2f4aba7e2..025d02509 100644 --- a/crates/core/src/settings.rs +++ b/crates/core/src/settings.rs @@ -777,6 +777,11 @@ pub enum SettingsPatch { project_id: String, value: Option, }, + ProjectMergeMethod { + project_id: String, + method: crate::pull_request::PullRequestMergeMethod, + }, + RemoveAgentCreditsOnMerge(bool), OrchestrateDecisionModels(Vec), OrchestrateChildModels(Vec), OrchestrateChildApproval(ChildApprovalMode), @@ -917,6 +922,13 @@ pub struct Settings { pub auto_settle_on_merge: bool, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub project_settlement_overrides: BTreeMap, + /// The merge method a project's pull requests are merged with unless another is chosen, + /// set from the merge confirmation. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub project_merge_methods: BTreeMap, + /// Whether a merge leaves agents' credit lines out of GitHub's merge message by default. + #[serde(default)] + pub remove_agent_credits_on_merge: bool, /// Built-in orchestration identities and child-model routing table. #[serde(default, skip_serializing_if = "OrchestrateSettings::is_default")] pub orchestrate: OrchestrateSettings, @@ -1042,6 +1054,8 @@ impl Default for Settings { auto_settle_after_days: default_auto_settle_after_days(), auto_settle_on_merge: true, project_settlement_overrides: BTreeMap::new(), + project_merge_methods: BTreeMap::new(), + remove_agent_credits_on_merge: false, orchestrate: OrchestrateSettings::default(), computer_use: ComputerUseSettings::default(), browser: BrowserSettings::default(), @@ -1127,6 +1141,12 @@ impl Settings { self.project_settlement_overrides.remove(&project_id); } } + SettingsPatch::ProjectMergeMethod { project_id, method } => { + self.project_merge_methods.insert(project_id, method); + } + SettingsPatch::RemoveAgentCreditsOnMerge(value) => { + self.remove_agent_credits_on_merge = value; + } SettingsPatch::OrchestrateDecisionModels(value) => { self.orchestrate.decision_models = value; self.orchestrate.normalize_models(false); diff --git a/crates/protocol/src/lib.rs b/crates/protocol/src/lib.rs index 36f8f32e6..1bdc60b89 100644 --- a/crates/protocol/src/lib.rs +++ b/crates/protocol/src/lib.rs @@ -11,15 +11,16 @@ mod pull_request; mod query; pub use preview::{PreviewRequest, PreviewResponse}; pub use pull_request::{ - MAX_PULL_REQUEST_MEDIA_BYTES, PullRequestAction, PullRequestActionResult, PullRequestActor, - PullRequestComment, PullRequestConversation, PullRequestFile, PullRequestFileText, - PullRequestFiles, PullRequestLabel, PullRequestLabelCandidate, PullRequestLabelCandidates, - PullRequestMedia, PullRequestPatch, PullRequestPermissions, PullRequestReaction, - PullRequestReactionContent, PullRequestRead, PullRequestReadResponse, PullRequestRejection, - PullRequestReviewAnchor, PullRequestReviewState, PullRequestReviewThread, - PullRequestReviewVerdict, PullRequestReviewer, PullRequestReviewerCandidate, - PullRequestReviewerCandidates, PullRequestReviewerKind, PullRequestReviewerState, - PullRequestThreadReplies, PullRequestViewedFiles, PullRequestViewedState, + MAX_PULL_REQUEST_MEDIA_BYTES, PullRequestAction, PullRequestActionResult, + PullRequestActionState, PullRequestActor, PullRequestComment, PullRequestConversation, + PullRequestFile, PullRequestFileText, PullRequestFiles, PullRequestLabel, + PullRequestLabelCandidate, PullRequestLabelCandidates, PullRequestMedia, PullRequestMergeState, + PullRequestPatch, PullRequestPermissions, PullRequestReaction, PullRequestReactionContent, + PullRequestRead, PullRequestReadResponse, PullRequestRejection, PullRequestReviewAnchor, + PullRequestReviewState, PullRequestReviewThread, PullRequestReviewVerdict, PullRequestReviewer, + PullRequestReviewerCandidate, PullRequestReviewerCandidates, PullRequestReviewerKind, + PullRequestReviewerState, PullRequestThreadReplies, PullRequestViewedFiles, + PullRequestViewedState, }; pub mod terminal; mod wire; @@ -123,6 +124,12 @@ pub use wire::{ // candidates; a reaction's content is a typed name; the conversation carries // the account's permissions, labels and reviewers, and comments and threads // what the account may do to them. +// Unreleased: RunPullRequestAction also marks ready, converts to draft, +// closes, reopens, reverts, updates the branch, merges and disables +// auto-merge, answering UpToDate, Queued, AutoMergeEnabled and Opened, or a +// rejection InStack or StackUnknown; Query::PullRequest reads ActionState; +// settings carry project_merge_methods and remove_agent_credits_on_merge, +// patched by ProjectMergeMethod and RemoveAgentCreditsOnMerge. pub const PROTOCOL_VERSION: u32 = 10; #[cfg(test)] diff --git a/crates/protocol/src/pull_request.rs b/crates/protocol/src/pull_request.rs index a2f59f2b6..cdf939d54 100644 --- a/crates/protocol/src/pull_request.rs +++ b/crates/protocol/src/pull_request.rs @@ -2,7 +2,7 @@ use agent::FileChangeKind; use serde::{Deserialize, Serialize}; -use tcode_core::session::ReviewSide; +use tcode_core::{pull_request::PullRequestMergeMethod, session::ReviewSide}; /// The largest image a media read returns; past it the host stops reading. pub const MAX_PULL_REQUEST_MEDIA_BYTES: usize = 8 * 1024 * 1024; @@ -39,6 +39,8 @@ pub enum PullRequestRead { /// Whoever the pull request may be sent to for review, with current requests marked. The /// organization's teams are not listed: that needs `read:org`, which a token need not carry. ReviewerCandidates, + /// What merging, updating the branch and the other lifecycle actions would meet now. + ActionState, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -326,6 +328,54 @@ pub struct PullRequestReviewerCandidates { pub complete: bool, } +/// GitHub's merge state status: what merging now would meet. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum PullRequestMergeState { + Clean, + /// Mergeable, with checks that are not passing. + Unstable, + /// Mergeable, with pre-receive hooks to pass. + HasHooks, + /// Requirements such as reviews or checks are not met. + Blocked, + /// The branch protection requires the head to be up to date with the base. + Behind, + /// Conflicts with the base. + Dirty, + Draft, + Unknown, +} + +/// What a lifecycle action would meet: read fresh before a merge or a branch update, and on +/// request. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PullRequestActionState { + /// The head a branch update or a merge must still find. + pub head: String, + pub merge_state: PullRequestMergeState, + /// Commits on the base the head does not have; `None` when GitHub could not compare them. + pub behind_by: Option, + pub merge_queue: bool, + /// The repository's enabled methods, in GitHub's order. + pub merge_methods: Vec, + /// The repository allows auto-merge. + pub auto_merge_allowed: bool, + /// The method auto-merge is armed with. + pub auto_merge: Option, + pub queued: bool, + /// Where in the merge queue, when GitHub says. + pub queue_position: Option, + /// Failing checks of the head by name; a list cut short names only the first page's. + pub failing_checks: Vec, + pub pending_checks: u32, + /// Mark ready, convert to draft, close and reopen. + pub can_update: bool, + pub can_update_branch: bool, + /// Merge, auto-merge and revert. + pub can_merge: bool, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type", content = "content", rename_all = "snake_case")] pub enum PullRequestReadResponse { @@ -337,6 +387,7 @@ pub enum PullRequestReadResponse { Media(PullRequestMedia), LabelCandidates(PullRequestLabelCandidates), ReviewerCandidates(PullRequestReviewerCandidates), + ActionState(PullRequestActionState), } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -396,6 +447,28 @@ pub enum PullRequestAction { add: Vec, remove: Vec, }, + ReadyForReview, + ConvertToDraft, + Close, + Reopen, + /// Opens a pull request that reverses this merged one, linked to the thread. + Revert, + /// Brings the base into the head on GitHub, by a merge commit unless `rebase`. `head` is the + /// commit the user saw; a moved head sends nothing. + UpdateBranch { + head: String, + rebase: bool, + }, + /// Merges at `head` with `method`, or joins the merge queue when the repository has one. + /// With `auto`, a pull request that cannot merge yet has auto-merge armed instead. With + /// `remove_credits`, a merge or squash leaves agents' credit lines out of GitHub's message. + Merge { + head: String, + method: PullRequestMergeMethod, + auto: bool, + remove_credits: bool, + }, + DisableAutoMerge, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -413,6 +486,21 @@ pub enum PullRequestActionResult { }, /// The write was sent and no answer says whether GitHub applied it. Uncertain, + /// The branch already had the base; nothing was sent. + UpToDate, + /// The merge queue took the pull request; it is not merged yet. + Queued { + position: Option, + }, + /// GitHub merges the pull request with `method` once its requirements pass. + AutoMergeEnabled { + method: PullRequestMergeMethod, + }, + /// A new pull request was opened, and linked to the thread. + Opened { + number: u64, + url: String, + }, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -438,4 +526,12 @@ pub enum PullRequestRejection { }, /// A read the write needed failed, so it was not sent. Failed, + /// Layer `index` of a native stack of `layers` merges with the layers below it, which Tcode + /// does not do yet. + InStack { + index: u32, + layers: u32, + }, + /// Whether the pull request is in a native stack is not known yet. + StackUnknown, } diff --git a/crates/runtime/src/app/command_validation.rs b/crates/runtime/src/app/command_validation.rs index d582401c7..2ef7450a3 100644 --- a/crates/runtime/src/app/command_validation.rs +++ b/crates/runtime/src/app/command_validation.rs @@ -47,7 +47,8 @@ impl AppState { }; self.validate_plugin_command(command)?; if let Command::PatchSettings { patch } = command { - if let SettingsPatch::ProjectSettlement { project_id, .. } = patch + if let SettingsPatch::ProjectSettlement { project_id, .. } + | SettingsPatch::ProjectMergeMethod { project_id, .. } = patch && !self .projects .iter() diff --git a/crates/runtime/src/app/pull_requests.rs b/crates/runtime/src/app/pull_requests.rs index 2d9c4e4de..7199ba5ad 100644 --- a/crates/runtime/src/app/pull_requests.rs +++ b/crates/runtime/src/app/pull_requests.rs @@ -367,6 +367,10 @@ impl AppState { reads.reviewer_candidates(&key)?, PullRequestReadResponse::ReviewerCandidates, ), + PullRequestRead::ActionState => reply( + reads.action_state(&key)?, + PullRequestReadResponse::ActionState, + ), PullRequestRead::Media { url, validator } => { let media = reads.media(&key, &url, validator.as_deref())?; let expires_at = match &media { @@ -463,7 +467,8 @@ impl AppState { }) } /// A write to the pull request. Whenever GitHub may have applied it, the sync reads the pull - /// request again. A review takes the account's draft and leaves it only once GitHub took it. + /// request again. A review takes the account's draft and leaves it only once GitHub took it; + /// a revert's new pull request is linked to the thread. pub fn run_pull_request_action( &mut self, session_id: &str, @@ -474,6 +479,34 @@ impl AppState { if let Err(error) = self.linked_pull_request(session_id, &key) { return cx.spawn_background(async move { Err(error) }); } + // A native stack merges a layer with the layers below it, which is not one pull request's + // merge; until that route exists, nothing is sent for one, nor for a pull request whose + // stack is not known yet. + if matches!( + action, + PullRequestAction::Merge { .. } | PullRequestAction::UpdateBranch { .. } + ) { + let links = self + .find_meta(session_id) + .map(|meta| meta.pull_requests) + .unwrap_or_default(); + let rejection = match pull_request::stack_route(&links, &key).0 { + pull_request::PullRequestStackRoute::Single => None, + pull_request::PullRequestStackRoute::Layer { index, layers } => { + Some(tcode_protocol::PullRequestRejection::InStack { index, layers }) + } + pull_request::PullRequestStackRoute::Unknown => { + Some(tcode_protocol::PullRequestRejection::StackUnknown) + } + }; + if let Some(rejection) = rejection { + return cx.spawn_background(async move { + Ok(CommandResponse::PullRequestAction( + PullRequestActionResult::Rejected(rejection), + )) + }); + } + } let drafts = self.review_drafts(session_id); let reads = self.pull_requests.reads.clone(); let writing = key.clone(); @@ -513,8 +546,27 @@ impl AppState { let (review, outcome) = task.await; if !matches!(outcome, PullRequestActionResult::Rejected(_)) { let applied = outcome == PullRequestActionResult::Applied; + let opened = match &outcome { + PullRequestActionResult::Opened { number, url } => Some(( + PullRequestKey::new(&key.host, &key.repository, *number), + url.clone(), + )), + _ => None, + }; let _ = host .enqueue_and_wait(move |state, cx| { + if let Some((opened, url)) = opened + && let Err(error) = state.apply_pull_request_link( + &id, + opened, + url, + PullRequestSource::Created, + true, + cx, + ) + { + log::warn!("the revert pull request was not linked: {error}"); + } if let Some((account, ids, body)) = review { state.pull_requests.submissions += 1; if let Some(mut meta) = state.find_meta(&id) diff --git a/crates/runtime/src/app/pull_requests_tests.rs b/crates/runtime/src/app/pull_requests_tests.rs index 2a1ce64a2..9f4adc6b9 100644 --- a/crates/runtime/src/app/pull_requests_tests.rs +++ b/crates/runtime/src/app/pull_requests_tests.rs @@ -8,6 +8,36 @@ use tcode_services::{github::Credentials, settings::SettingsStore}; use crate::app::test_support::github_fixture as fixture; +/// Sends one command and runs the host until it answers it. +fn acked( + state: &TestEntity, + cx: &mut TestAppContext, + id: u64, + command: Command, +) -> Result { + state.dispatch_command(cx, id, command); + let deadline = std::time::Instant::now() + Duration::from_secs(5); + loop { + cx.run_until_parked(); + if let Some(result) = cx + .drain_outgoing() + .into_iter() + .find_map(|message| match message { + tcode_protocol::HostMessage::Ack { id: acked, result } if acked == id => { + Some(result) + } + _ => None, + }) + { + return result; + } + assert!( + std::time::Instant::now() < deadline, + "the command was answered" + ); + } +} + struct HostReply { state: &'static str, stack: bool, @@ -1154,27 +1184,7 @@ fn a_review_draft_is_the_hosts_across_a_restart_and_a_moved_head_keeps_it_until_ let mut next_id = 0; let mut command = |state: &TestEntity, cx: &mut TestAppContext, command: Command| { next_id += 1; - state.dispatch_command(cx, next_id, command); - let deadline = std::time::Instant::now() + Duration::from_secs(5); - loop { - cx.run_until_parked(); - if let Some(result) = - cx.drain_outgoing() - .into_iter() - .find_map(|message| match message { - tcode_protocol::HostMessage::Ack { id, result } if id == next_id => { - Some(result) - } - _ => None, - }) - { - return result; - } - assert!( - std::time::Instant::now() < deadline, - "the command was answered" - ); - } + acked(state, cx, next_id, command) }; let edit = |edit| Command::EditPullRequestReviewDraft { session_id: "active".into(), @@ -1498,3 +1508,157 @@ fn a_review_draft_is_its_accounts_and_an_unanswered_submission_waits_for_a_later .update(&mut cx, |state, _| state.close_store()) .unwrap(); } + +#[test] +fn a_stack_layer_is_never_merged_alone_and_a_revert_is_linked_as_created() { + const HEAD: &str = "2222222222222222222222222222222222222222"; + let dir = TestStore::new("tcode-pr-lifecycle"); + let fixture = fixture::Fixture::new(); + let api = client(&dir, &fixture); + let operations = Arc::new(Mutex::new(Vec::::new())); + let seen = operations.clone(); + let _server = fixture.serve(move |exchange| { + let sent: Value = serde_json::from_slice(&exchange.body).unwrap_or_default(); + let operation = sent["query"] + .as_str() + .and_then(|query| query.split(['(', ' ']).nth(1)) + .unwrap_or_default() + .to_owned(); + seen.lock().unwrap().push(operation.clone()); + let rest = exchange + .request + .starts_with("GET /repos/sample/project/pulls/1 "); + let reply = match operation.as_str() { + _ if rest => json!({"base": {"sha": HEAD}, "head": {"sha": HEAD}, "node_id": "PR_1"}), + "PullRequestActionState" => json!({"data": {"repository": { + "viewerPermission": "WRITE", "mergeCommitAllowed": true, + "pullRequest": {"id": "PR_1", "headRefOid": HEAD, "mergeStateStatus": "CLEAN"}, + }}}), + "RevertPullRequest" => json!({"data": {"revertPullRequest": {"revertPullRequest": { + "number": 9, "url": "https://github.com/sample/project/pull/9", + }}}}), + "MergePullRequest" => { + json!({"data": {"mergePullRequest": {"pullRequest": {"merged": true}}}}) + } + _ => json!({"data": {"s0": {"pullRequest": pr(1, "OPEN", false)}}}), + }; + exchange.reply(200, "", &serde_json::to_vec(&reply).unwrap()); + }); + let key = PullRequestKey::new("github.com", "sample/project", 1); + let mut cx = TestAppContext::default(); + let state = cx.new_entity(TestClientState::new((*dir).clone())); + state.update(&mut cx, |state, _| { + state.pull_requests = PullRequestRuntime::new(api); + let meta = linked("active", 1, false); + dir.upsert_meta(&meta).unwrap(); + state.sessions.push(meta); + }); + let mut next_id = 0; + let mut act = |state: &TestEntity, cx: &mut TestAppContext, action: PullRequestAction| { + next_id += 1; + let command = Command::RunPullRequestAction { + session_id: "active".into(), + key: key.clone(), + action, + }; + acked(state, cx, next_id, command).unwrap() + }; + let merge = || PullRequestAction::Merge { + head: HEAD.into(), + method: tcode_core::pull_request::PullRequestMergeMethod::Merge, + auto: false, + remove_credits: false, + }; + let update = || PullRequestAction::UpdateBranch { + head: HEAD.into(), + rebase: false, + }; + let rejected = |rejection| { + CommandResponse::PullRequestAction(PullRequestActionResult::Rejected(rejection)) + }; + let set_stack = |state: &TestEntity, cx: &mut TestAppContext, stack| { + state.update(cx, |state, _| { + state + .sessions + .iter_mut() + .find(|meta| meta.id == "active") + .unwrap() + .pull_requests[0] + .stack = stack; + }); + }; + + for action in [merge(), update()] { + assert_eq!( + act(&state, &mut cx, action), + rejected(tcode_protocol::PullRequestRejection::StackUnknown), + "a pull request whose stack is not known yet is not merged as one" + ); + } + let layer = |number: u64| tcode_core::pull_request::PullRequestStackLayer { + url: format!("https://github.com/sample/project/pull/{number}"), + number, + head_branch: format!("layer-{number}"), + state: PullRequestState::Open, + }; + set_stack( + &state, + &mut cx, + PullRequestStackState::Native(PullRequestStack { + id: "stack".into(), + number: 7, + url: "https://github.com/sample/project/stacks/7".into(), + base: "main".into(), + layers: vec![layer(3), layer(1)], + }), + ); + for action in [merge(), update()] { + assert_eq!( + act(&state, &mut cx, action), + rejected(tcode_protocol::PullRequestRejection::InStack { + index: 2, + layers: 2 + }) + ); + } + assert!( + operations.lock().unwrap().is_empty(), + "a stack-routed write reads and sends nothing" + ); + + set_stack(&state, &mut cx, PullRequestStackState::None); + assert_eq!( + act(&state, &mut cx, merge()), + CommandResponse::PullRequestAction(PullRequestActionResult::Applied) + ); + assert_eq!( + act(&state, &mut cx, PullRequestAction::Revert), + CommandResponse::PullRequestAction(PullRequestActionResult::Opened { + number: 9, + url: "https://github.com/sample/project/pull/9".into() + }) + ); + let links = state.read(|state| state.find_meta("active").unwrap().pull_requests); + let revert = links + .iter() + .find(|link| link.key.number == 9) + .expect("the revert is linked to the thread"); + assert_eq!( + (revert.source, revert.url.as_str()), + ( + PullRequestSource::Created, + "https://github.com/sample/project/pull/9" + ) + ); + // The sync reads the pull requests after the writes. + cx.run_until(|_| { + operations + .lock() + .unwrap() + .iter() + .any(|operation| operation == "PullRequestSummaries") + }); + state + .update(&mut cx, |state, _| state.close_store()) + .unwrap(); +} diff --git a/crates/services/src/github/merge_message.rs b/crates/services/src/github/merge_message.rs new file mode 100644 index 000000000..307282004 --- /dev/null +++ b/crates/services/src/github/merge_message.rs @@ -0,0 +1,224 @@ +//! Leaving agents' credits out of the message GitHub would write for a merge or squash. + +const AGENT_EMAILS: [&str; 7] = [ + "noreply@anthropic.com", + "claude@anthropic.com", + "noreply@openai.com", + "codex@openai.com", + "cursoragent@cursor.com", + "copilot@github.com", + "noreply@github.com", +]; + +const AGENT_NAMES: [&str; 9] = [ + "claude code", + "claude", + "codex", + "cursor", + "github copilot", + "copilot", + "opencode", + "grok", + "antigravity", +]; + +const AGENT_SITES: [&str; 9] = [ + "claude.ai", + "www.anthropic.com", + "chatgpt.com", + "openai.com", + "cursor.com", + "github.com", + "opencode.ai", + "x.ai", + "antigravity.google", +]; + +/// The message without standalone agent `Co-authored-by` lines and "Generated with …" footers. +/// People's credits, prose and anything quoted in code stay, and a message with nothing to +/// remove comes back unchanged. +pub fn remove_agent_credits(message: &str) -> String { + let newline = if message.contains("\r\n") { + "\r\n" + } else { + "\n" + }; + let mut fence: Option = None; + let mut removed = false; + let kept: Vec<&str> = message + .split('\n') + .map(|line| line.strip_suffix('\r').unwrap_or(line)) + .filter(|line| { + if line.starts_with(" ") || line.starts_with('\t') { + return true; + } + if let Some((marker, remainder)) = fence_delimiter(line) { + match &fence { + None if marker.starts_with('~') || !remainder.contains('`') => { + fence = Some(marker.to_owned()); + } + Some(open) + if open.as_bytes()[0] == marker.as_bytes()[0] + && marker.len() >= open.len() + && remainder.trim_matches([' ', '\t']).is_empty() => + { + fence = None; + } + _ => {} + } + return true; + } + if fence.is_some() { + return true; + } + let credit = is_agent_author(line) || is_generated_footer(line); + removed |= credit; + !credit + }) + .collect(); + if !removed { + return message.to_owned(); + } + let mut text = kept.join(newline); + // Blank lines the credits leave at the end go with them. + loop { + let trimmed = text.trim_end_matches([' ', '\t']); + let Some(rest) = trimmed.strip_suffix('\n') else { + break; + }; + let rest = rest.strip_suffix('\r').unwrap_or(rest).len(); + text.truncate(rest); + } + text +} + +/// A code fence's marker and what follows it, for a line that opens or closes one. +fn fence_delimiter(line: &str) -> Option<(&str, &str)> { + let indent = line.len() - line.trim_start_matches(' ').len(); + if indent > 3 { + return None; + } + let rest = &line[indent..]; + let symbol = rest.chars().next().filter(|c| *c == '`' || *c == '~')?; + let length = rest.len() - rest.trim_start_matches(symbol).len(); + (length >= 3).then(|| rest.split_at(length)) +} + +/// The line's text without its list marker. +fn credit_text(line: &str) -> &str { + let text = line.trim(); + match text.strip_prefix(['-', '*']) { + Some(rest) if rest.starts_with(char::is_whitespace) => rest.trim_start(), + _ => text, + } +} + +fn is_agent_author(line: &str) -> bool { + let text = credit_text(line); + let Some(prefix) = text.get(..15) else { + return false; + }; + if !prefix.eq_ignore_ascii_case("co-authored-by:") { + return false; + } + let Some(person) = text[15..].strip_suffix('>') else { + return false; + }; + let Some((name, email)) = person.rsplit_once('<') else { + return false; + }; + let email = email.to_ascii_lowercase(); + if email.is_empty() || email.contains('>') || !AGENT_EMAILS.contains(&email.as_str()) { + return false; + } + // GitHub's generic noreply address is also people's; only Copilot's name makes it an agent. + let name = name.trim().to_lowercase(); + email != "noreply@github.com" || name == "github copilot" || name == "copilot" +} + +fn is_generated_footer(line: &str) -> bool { + let footer = unlink_agent_sites(credit_text(line)).to_lowercase(); + let footer = footer + .strip_prefix('🤖') + .map_or(footer.as_str(), str::trim_start); + let Some(rest) = footer + .strip_prefix("generated with") + .or_else(|| footer.strip_prefix("generated by")) + else { + return false; + }; + if !rest.starts_with(char::is_whitespace) { + return false; + } + let rest = rest.trim_start(); + let rest = rest + .strip_suffix(['.', '!']) + .filter(|name| AGENT_NAMES.contains(name)) + .unwrap_or(rest); + AGENT_NAMES.contains(&rest) +} + +/// Markdown links to an agent's site, reduced to their text. +fn unlink_agent_sites(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut rest = text; + while let Some(open) = rest.find('[') { + out.push_str(&rest[..open]); + let candidate = &rest[open..]; + match agent_link(candidate) { + Some((label, length)) => { + out.push_str(label); + rest = &candidate[length..]; + } + None => { + out.push('['); + rest = &candidate[1..]; + } + } + } + out.push_str(rest); + out +} + +/// The label and length of `[label](https://site/…)` at the start of `text`. +fn agent_link(text: &str) -> Option<(&str, usize)> { + let close = text.find(']')?; + let label = &text[1..close]; + if label.is_empty() { + return None; + } + let target = text[close + 1..].strip_prefix("(https://")?; + let site = AGENT_SITES.iter().find(|site| { + target + .get(..site.len()) + .is_some_and(|host| host.eq_ignore_ascii_case(site)) + })?; + let after = &target[site.len()..]; + let path = if after.starts_with('/') { + after + .find(|c: char| c == ')' || c.is_whitespace()) + .unwrap_or(after.len()) + } else { + 0 + }; + after[path..].starts_with(')').then(|| { + let length = text.len() - after.len() + path + 1; + (label, length) + }) +} + +#[cfg(test)] +mod tests { + use super::remove_agent_credits; + + #[test] + fn credits_quoted_in_code_and_people_on_githubs_noreply_address_stay() { + let message = "Explain trailers\r\n\r\n```\r\nCo-authored-by: Claude \r\n```\r\n Generated with Codex\r\n- Co-authored-by: Octo Cat \r\n* Co-authored-by: Copilot \r\nGenerated by codex.\r\n\r\n"; + assert_eq!( + remove_agent_credits(message), + "Explain trailers\r\n\r\n```\r\nCo-authored-by: Claude \r\n```\r\n Generated with Codex\r\n- Co-authored-by: Octo Cat " + ); + let untouched = "Generated with care\n\nCo-authored-by: Ada \n\n"; + assert_eq!(remove_agent_credits(untouched), untouched); + } +} diff --git a/crates/services/src/github/mod.rs b/crates/services/src/github/mod.rs index 28c8d36bc..684b0ff72 100644 --- a/crates/services/src/github/mod.rs +++ b/crates/services/src/github/mod.rs @@ -4,6 +4,7 @@ pub mod api; pub mod credentials; pub mod graphql; pub mod media; +mod merge_message; pub mod pull_request_actions; pub mod pull_request_reads; pub mod pull_request_watch; diff --git a/crates/services/src/github/pull_request_actions.rs b/crates/services/src/github/pull_request_actions.rs index aa89e3a22..f2b40b365 100644 --- a/crates/services/src/github/pull_request_actions.rs +++ b/crates/services/src/github/pull_request_actions.rs @@ -2,24 +2,141 @@ //! whatever a write may have changed is dropped from the reads, so the next read sees it. use super::{ - CredentialError, GitHubError, RequestOptions, RestRequest, + CredentialError, Fresh, GitHubError, RequestOptions, RestRequest, graphql::Document, + merge_message::remove_agent_credits, pull_request_reads::{ - MAX_PAGES, PullRequestReads, Reader, is_revision, percent_encode, reaction_name, + MAX_PAGES, PullRequestReads, READ_TTL, Reader, is_revision, percent_encode, reaction_name, }, + pull_request_watch, }; use serde_json::{Value, json}; use std::{collections::BTreeMap, time::UNIX_EPOCH}; use tcode_core::{ - pull_request::{PullRequestKey, PullRequestReviewDraftComment}, + pull_request::{PullRequestKey, PullRequestMergeMethod, PullRequestReviewDraftComment}, + pull_request_watch::CheckStatus, session::ReviewSide, }; use tcode_protocol::{ - PullRequestAction, PullRequestActionResult as Outcome, PullRequestFile, PullRequestFileText, - PullRequestPatch, PullRequestRejection as Rejection, PullRequestReviewVerdict, - PullRequestReviewer, PullRequestReviewerKind, + PullRequestAction, PullRequestActionResult as Outcome, PullRequestActionState, PullRequestFile, + PullRequestFileText, PullRequestMergeState, PullRequestPatch, + PullRequestRejection as Rejection, PullRequestReviewVerdict, PullRequestReviewer, + PullRequestReviewerKind, }; +/// What `gh pr merge` and `gh pr update-branch` read before they act, with the account's rights, +/// the repository's merge settings and the head's checks. +const ACTION_STATE: &str = "query PullRequestActionState($owner: String!, $name: String!, $number: Int!, $headRef: String!) { repository(owner: $owner, name: $name) { viewerPermission mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed autoMergeAllowed pullRequest(number: $number) { id headRefOid isMergeQueueEnabled mergeStateStatus viewerCanUpdate viewerCanUpdateBranch autoMergeRequest { mergeMethod } mergeQueueEntry { position } baseRef { compare(headRef: $headRef) { behindBy } } commits(last: 1) { nodes { commit { statusCheckRollup { contexts(first: 100) { nodes { __typename ... on StatusContext { context state createdAt } ... on CheckRun { name status conclusion startedAt completedAt checkSuite { workflowRun { workflow { name } } } } } } } } } } } } }"; + +const MERGE_MESSAGE: &str = "query PullRequestMergeMessage($owner: String!, $name: String!, $number: Int!, $method: PullRequestMergeMethod!) { repository(owner: $owner, name: $name) { pullRequest(number: $number) { isMergeQueueEnabled headRefOid viewerMergeBodyText(mergeType: $method) } } }"; + +fn method_name(method: PullRequestMergeMethod) -> &'static str { + match method { + PullRequestMergeMethod::Merge => "MERGE", + PullRequestMergeMethod::Squash => "SQUASH", + PullRequestMergeMethod::Rebase => "REBASE", + } +} + +fn method_of(name: &str) -> Option { + match name { + "MERGE" => Some(PullRequestMergeMethod::Merge), + "SQUASH" => Some(PullRequestMergeMethod::Squash), + "REBASE" => Some(PullRequestMergeMethod::Rebase), + _ => None, + } +} + +/// The pull request's node id and its action state. +fn action_state(response: &Value) -> Result<(String, PullRequestActionState), GitHubError> { + let repository = &response["data"]["repository"]; + let pr = &repository["pullRequest"]; + if pr.is_null() { + return Err(GitHubError::NotFound); + } + let id = pr["id"].as_str().ok_or(GitHubError::InvalidResponse)?; + let head = pr["headRefOid"] + .as_str() + .filter(|head| is_revision(head)) + .ok_or(GitHubError::InvalidResponse)?; + let contexts: Vec = pr["commits"]["nodes"][0]["commit"]["statusCheckRollup"]["contexts"] + ["nodes"] + .as_array() + .cloned() + .unwrap_or_default(); + let checks = pull_request_watch::checks(&contexts); + // Write or above merges and reverts (study: provider permission map). + let writes = matches!( + repository["viewerPermission"].as_str(), + Some("ADMIN" | "MAINTAIN" | "WRITE") + ); + let state = PullRequestActionState { + head: head.to_owned(), + merge_state: match pr["mergeStateStatus"].as_str() { + Some("CLEAN") => PullRequestMergeState::Clean, + Some("UNSTABLE") => PullRequestMergeState::Unstable, + Some("HAS_HOOKS") => PullRequestMergeState::HasHooks, + Some("BLOCKED") => PullRequestMergeState::Blocked, + Some("BEHIND") => PullRequestMergeState::Behind, + Some("DIRTY") => PullRequestMergeState::Dirty, + Some("DRAFT") => PullRequestMergeState::Draft, + _ => PullRequestMergeState::Unknown, + }, + behind_by: pr["baseRef"]["compare"]["behindBy"].as_u64(), + merge_queue: pr["isMergeQueueEnabled"].as_bool() == Some(true), + merge_methods: [ + ("mergeCommitAllowed", PullRequestMergeMethod::Merge), + ("squashMergeAllowed", PullRequestMergeMethod::Squash), + ("rebaseMergeAllowed", PullRequestMergeMethod::Rebase), + ] + .into_iter() + .filter(|(field, _)| repository[*field].as_bool() == Some(true)) + .map(|(_, method)| method) + .collect(), + auto_merge_allowed: repository["autoMergeAllowed"].as_bool() == Some(true), + auto_merge: pr["autoMergeRequest"]["mergeMethod"] + .as_str() + .and_then(method_of), + queued: pr["mergeQueueEntry"].is_object(), + queue_position: pr["mergeQueueEntry"]["position"] + .as_u64() + .map(|position| position as u32), + failing_checks: checks + .iter() + .filter(|check| check.status.failed()) + .map(|check| check.name.clone()) + .collect(), + pending_checks: checks + .iter() + .filter(|check| check.status == CheckStatus::Pending) + .count() as u32, + can_update: pr["viewerCanUpdate"].as_bool() == Some(true), + can_update_branch: pr["viewerCanUpdateBranch"].as_bool() == Some(true), + can_merge: writes, + }; + Ok((id.to_owned(), state)) +} + +/// What GitHub says became of a merge: merged, queued, or armed to merge later. +fn merge_outcome(pr: &Value) -> Outcome { + if pr["merged"].as_bool() == Some(true) { + Outcome::Applied + } else if pr["mergeQueueEntry"].is_object() { + Outcome::Queued { + position: pr["mergeQueueEntry"]["position"] + .as_u64() + .map(|position| position as u32), + } + } else if let Some(method) = pr["autoMergeRequest"]["mergeMethod"] + .as_str() + .and_then(method_of) + { + Outcome::AutoMergeEnabled { method } + } else { + Outcome::Uncertain + } +} + /// A pending comment's id and the revision its lines now read at, if they still do. pub type Moved = (u64, Option); @@ -115,21 +232,137 @@ fn side(side: ReviewSide) -> &'static str { } impl Reader<'_> { - fn mutate(&self, operation: &'static str, query: &str, variables: Value) -> Outcome { + fn mutation( + &self, + operation: &'static str, + query: &str, + variables: Value, + ) -> Result { let Value::Object(variables) = variables else { unreachable!("mutation variables are an object") }; - answered(self.api.graphql( - &self.key.host, - &Document { - query: query.to_owned(), - variables: variables.into_iter().collect::>(), - }, - &RequestOptions { - operation, - ..self.options.clone() + self.api + .graphql( + &self.key.host, + &Document { + query: query.to_owned(), + variables: variables.into_iter().collect::>(), + }, + &RequestOptions { + operation, + ..self.options.clone() + }, + )? + .json() + } + + fn mutate(&self, operation: &'static str, query: &str, variables: Value) -> Outcome { + answered(self.mutation(operation, query, variables)) + } + + /// Read now, not from the cache: a merge or a branch update acts on the head as it stands. + fn action_state(&self) -> Result<(String, PullRequestActionState), GitHubError> { + let response = self.query( + "PullRequestActionState", + ACTION_STATE.to_owned(), + [( + "headRef", + json!(format!("refs/pull/{}/head", self.key.number)), + )], + )?; + action_state(&response) + } + + /// GitHub's merge or squash message without agents' credits, when that differs from it. + fn cleaned_message( + &self, + head: &str, + method: PullRequestMergeMethod, + ) -> Result, Rejection> { + let response = self + .query( + "PullRequestMergeMessage", + MERGE_MESSAGE.to_owned(), + [("method", json!(method_name(method)))], + ) + .map_err(rejection)?; + let pr = &response["data"]["repository"]["pullRequest"]; + let current = pr["headRefOid"].as_str().ok_or(Rejection::Failed)?; + if current != head { + return Err(Rejection::StaleHead { + head: current.to_owned(), + }); + } + // A merge queue writes its own message and ignores one sent with the merge. + if pr["isMergeQueueEnabled"].as_bool() == Some(true) { + return Ok(None); + } + let message = pr["viewerMergeBodyText"] + .as_str() + .ok_or(Rejection::Failed)?; + let cleaned = remove_agent_credits(message); + Ok((cleaned != message).then_some(cleaned)) + } + + fn merge( + &self, + head: &str, + method: PullRequestMergeMethod, + auto: bool, + remove_credits: bool, + ) -> Result { + let (id, state) = self.action_state().map_err(rejection)?; + if state.head != head { + return Err(Rejection::StaleHead { head: state.head }); + } + if !state.merge_methods.contains(&method) { + return Err(Rejection::Invalid); + } + let mut input = json!({ + "pullRequestId": id, + "mergeMethod": method_name(method), + "expectedHeadOid": head, + }); + // Rebasing keeps each commit's own message. + if remove_credits + && method != PullRequestMergeMethod::Rebase + && !state.merge_queue + && let Some(body) = self.cleaned_message(head, method)? + { + input["commitBody"] = json!(body); + } + // A merge queue takes a pull request through auto-merge, and auto-merge asked of one + // that can merge now simply merges it, as `gh pr merge --auto` does. + let arm = state.merge_queue + || (auto + && !matches!( + state.merge_state, + PullRequestMergeState::Clean + | PullRequestMergeState::HasHooks + | PullRequestMergeState::Unstable + )); + let (operation, field, input_type) = if arm { + ( + "EnablePullRequestAutoMerge", + "enablePullRequestAutoMerge", + "EnablePullRequestAutoMergeInput", + ) + } else { + ( + "MergePullRequest", + "mergePullRequest", + "MergePullRequestInput", + ) + }; + let query = format!( + "mutation {operation}($input: {input_type}!) {{ {field}(input: $input) {{ pullRequest {{ merged mergeQueueEntry {{ position }} autoMergeRequest {{ mergeMethod }} }} }} }}" + ); + Ok( + match self.mutation(operation, &query, json!({ "input": input })) { + Ok(answer) => merge_outcome(&answer["data"][field]["pullRequest"]), + Err(error) => answered::<()>(Err(error)), }, - )) + ) } fn send( @@ -206,11 +439,37 @@ fn in_order<'a>(steps: impl IntoIterator>) -> Outcome { } impl PullRequestReads { + pub fn action_state( + &self, + key: &PullRequestKey, + ) -> Result, GitHubError> { + let reader = self.reader(key)?; + self.action_states.read( + reader.read_key("action state"), + || Ok((reader.action_state()?.1, READ_TTL)), + |state| 256 + state.failing_checks.iter().map(String::len).sum::(), + ) + } + /// Every action but a review, whose content is the runtime's draft: see [`Self::submit_review`]. pub fn act(&self, key: &PullRequestKey, action: &PullRequestAction) -> Outcome { self.drop_written(key); let outcome = self.write(key, action).unwrap_or_else(Outcome::Rejected); self.drop_written(key); + // A lifecycle write changes the pull request itself: its head, files and state. + if matches!( + action, + PullRequestAction::ReadyForReview + | PullRequestAction::ConvertToDraft + | PullRequestAction::Close + | PullRequestAction::Reopen + | PullRequestAction::Revert + | PullRequestAction::UpdateBranch { .. } + | PullRequestAction::Merge { .. } + | PullRequestAction::DisableAutoMerge + ) { + self.invalidate(key); + } outcome } @@ -407,6 +666,84 @@ impl PullRequestReads { ]; in_order(steps.into_iter().filter(|(names, _)| !names.is_empty())) } + PullRequestAction::ReadyForReview + | PullRequestAction::ConvertToDraft + | PullRequestAction::Close + | PullRequestAction::Reopen + | PullRequestAction::DisableAutoMerge => { + let (operation, field) = match action { + PullRequestAction::ReadyForReview => { + ("MarkPullRequestReady", "markPullRequestReadyForReview") + } + PullRequestAction::ConvertToDraft => { + ("ConvertPullRequestToDraft", "convertPullRequestToDraft") + } + PullRequestAction::Close => ("ClosePullRequest", "closePullRequest"), + PullRequestAction::Reopen => ("ReopenPullRequest", "reopenPullRequest"), + _ => ("DisablePullRequestAutoMerge", "disablePullRequestAutoMerge"), + }; + let id = self.node_id(&reader).map_err(rejection)?; + reader.mutate( + operation, + &format!( + "mutation {operation}($pullRequestId: ID!) {{ {field}(input: {{ pullRequestId: $pullRequestId }}) {{ clientMutationId }} }}" + ), + json!({"pullRequestId": id}), + ) + } + PullRequestAction::Revert => { + let id = self.node_id(&reader).map_err(rejection)?; + match reader.mutation( + "RevertPullRequest", + "mutation RevertPullRequest($pullRequestId: ID!) { revertPullRequest(input: { pullRequestId: $pullRequestId }) { revertPullRequest { number url } } }", + json!({"pullRequestId": id}), + ) { + Ok(answer) => { + let opened = &answer["data"]["revertPullRequest"]["revertPullRequest"]; + match (opened["number"].as_u64(), opened["url"].as_str()) { + (Some(number), Some(url)) => Outcome::Opened { + number, + url: url.to_owned(), + }, + // Something answered, and may have opened one. + _ => Outcome::Uncertain, + } + } + Err(error) => answered::<()>(Err(error)), + } + } + PullRequestAction::UpdateBranch { head, rebase } => { + if !is_revision(head) { + return Err(Rejection::Invalid); + } + let (id, state) = reader.action_state().map_err(rejection)?; + if state.head != *head { + return Err(Rejection::StaleHead { head: state.head }); + } + if state.behind_by == Some(0) { + return Ok(Outcome::UpToDate); + } + reader.mutate( + "UpdatePullRequestBranch", + "mutation UpdatePullRequestBranch($pullRequestId: ID!, $expectedHeadOid: GitObjectID!, $updateMethod: PullRequestBranchUpdateMethod!) { updatePullRequestBranch(input: { pullRequestId: $pullRequestId, expectedHeadOid: $expectedHeadOid, updateMethod: $updateMethod }) { clientMutationId } }", + json!({ + "pullRequestId": id, + "expectedHeadOid": head, + "updateMethod": if *rebase { "REBASE" } else { "MERGE" }, + }), + ) + } + PullRequestAction::Merge { + head, + method, + auto, + remove_credits, + } => { + if !is_revision(head) { + return Err(Rejection::Invalid); + } + reader.merge(head, *method, *auto, *remove_credits)? + } }) } diff --git a/crates/services/src/github/pull_request_reads.rs b/crates/services/src/github/pull_request_reads.rs index f10fccd9c..41de4d508 100644 --- a/crates/services/src/github/pull_request_reads.rs +++ b/crates/services/src/github/pull_request_reads.rs @@ -28,7 +28,7 @@ use tcode_protocol::{ PullRequestViewedState, }; -const READ_TTL: Duration = Duration::from_secs(60); +pub(super) const READ_TTL: Duration = Duration::from_secs(60); /// A merged pull request changes no more, short of an edited comment. const MERGED_TTL: Duration = Duration::from_secs(600); const VIEWED_TTL: Duration = Duration::from_secs(15); @@ -69,6 +69,7 @@ pub struct PullRequestReads { viewed: ReadCache, pub(super) labels: ReadCache, pub(super) reviewers: ReadCache, + pub(super) action_states: ReadCache, /// Filled only by a successful read, oldest first. node_ids: Mutex>, } @@ -82,7 +83,7 @@ pub(super) struct Reader<'a> { } impl Reader<'_> { - fn read_key(&self, read: impl Into) -> ReadKey { + pub(super) fn read_key(&self, read: impl Into) -> ReadKey { ReadKey { pull_request: self.key.clone(), account: self.account.clone(), @@ -542,6 +543,7 @@ impl PullRequestReads { viewed: ReadCache::new(4 * 1024 * 1024), labels: ReadCache::new(1024 * 1024), reviewers: ReadCache::new(1024 * 1024), + action_states: ReadCache::new(1024 * 1024), node_ids: Mutex::new(Vec::new()), }) } @@ -843,6 +845,7 @@ impl PullRequestReads { self.viewed.invalidate(key); self.labels.invalidate(key); self.reviewers.invalidate(key); + self.action_states.invalidate(key); } } diff --git a/crates/services/src/github/pull_request_watch.rs b/crates/services/src/github/pull_request_watch.rs index b71413f73..ee7b8bc62 100644 --- a/crates/services/src/github/pull_request_watch.rs +++ b/crates/services/src/github/pull_request_watch.rs @@ -313,7 +313,7 @@ fn check_status(raw: &Value) -> CheckStatus { /// One check per name and workflow, keeping its newest run: a rerun replaces the run before it /// where it stood. Survivors sharing a name across workflows are written `workflow / name`. -fn checks(contexts: &[Value]) -> Vec { +pub(super) fn checks(contexts: &[Value]) -> Vec { let at = |raw: &Value, field: &str| { trimmed(&raw[field]).filter(|value| value != "0001-01-01T00:00:00Z") }; diff --git a/crates/services/tests/pull_request_reads.rs b/crates/services/tests/pull_request_reads.rs index a8f0f3d95..6662e6c9f 100644 --- a/crates/services/tests/pull_request_reads.rs +++ b/crates/services/tests/pull_request_reads.rs @@ -4,7 +4,7 @@ use std::{ thread, }; use tcode_core::{ - pull_request::{PullRequestKey, PullRequestReviewDraftComment}, + pull_request::{PullRequestKey, PullRequestMergeMethod, PullRequestReviewDraftComment}, session::ReviewSide, }; use tcode_protocol::{ @@ -1580,7 +1580,8 @@ fn a_write_left_unanswered_is_uncertain_and_never_sent_again() { let fixture = Fixture::new(); let store = Store::new(); let reads = reads(&fixture, &store); - let (_server, log) = serve(fixture, |seen| { + let model = Mutex::new(Lifecycle::default()); + let (_server, log) = serve(fixture, move |seen| { if seen .line .starts_with("POST /repos/octo/repo/issues/7/labels ") @@ -1588,8 +1589,11 @@ fn a_write_left_unanswered_is_uncertain_and_never_sent_again() { return (502, String::new(), Vec::new()); } match seen.operation().as_deref() { - Some("AddPullRequestComment") => (0, String::new(), Vec::new()), - _ => writes_reply(seen), + Some("AddPullRequestComment" | "MergePullRequest") => (0, String::new(), Vec::new()), + Some("RevertPullRequest") => { + json_reply(json!({"data": {"revertPullRequest": {"revertPullRequest": null}}})) + } + _ => lifecycle_reply(&model, seen), } }); let sent = |prefix: &str| { @@ -1622,11 +1626,309 @@ fn a_write_left_unanswered_is_uncertain_and_never_sent_again() { PullRequestActionResult::Uncertain, "a server failure may have applied the write" ); + assert_eq!( + reads.act( + &key(), + &PullRequestAction::Merge { + head: HEAD.into(), + method: PullRequestMergeMethod::Squash, + auto: false, + remove_credits: false, + } + ), + PullRequestActionResult::Uncertain, + "a merge whose answer was lost may have merged" + ); + assert_eq!( + reads.act(&key(), &PullRequestAction::Revert), + PullRequestActionResult::Uncertain, + "an answer that names no revert may still have opened one" + ); assert_eq!( ( sent("AddPullRequestComment"), - sent("POST /repos/octo/repo/issues/7/labels ") + sent("POST /repos/octo/repo/issues/7/labels "), + sent("MergePullRequest"), + sent("RevertPullRequest"), + ), + (1, 1, 1, 1) + ); +} + +/// What GitHub holds of the pull request that the lifecycle reads and writes see. +struct Lifecycle { + head: &'static str, + merge_state: &'static str, + behind_by: u64, + queue: bool, + rebase_allowed: bool, + message: &'static str, +} + +impl Default for Lifecycle { + fn default() -> Self { + Self { + head: HEAD, + merge_state: "CLEAN", + behind_by: 2, + queue: false, + rebase_allowed: true, + message: "Fix the parser\n\nCo-authored-by: Ada ", + } + } +} + +fn lifecycle_reply(model: &Mutex, seen: &Seen) -> (u16, String, Vec) { + let model = model.lock().unwrap(); + match seen.operation().as_deref() { + Some("PullRequestActionState") => json_reply(json!({"data": {"repository": { + "viewerPermission": "WRITE", + "mergeCommitAllowed": true, "squashMergeAllowed": true, "rebaseMergeAllowed": model.rebase_allowed, + "autoMergeAllowed": true, + "pullRequest": { + "id": "PR_node_7", "headRefOid": model.head, + "isMergeQueueEnabled": model.queue, "mergeStateStatus": model.merge_state, + "viewerCanUpdate": true, "viewerCanUpdateBranch": true, + "autoMergeRequest": null, "mergeQueueEntry": null, + "baseRef": {"compare": {"behindBy": model.behind_by}}, + "commits": {"nodes": [{"commit": {"statusCheckRollup": {"contexts": {"nodes": [ + {"__typename": "CheckRun", "name": "lint", "status": "COMPLETED", "conclusion": "FAILURE"}, + {"__typename": "CheckRun", "name": "test", "status": "IN_PROGRESS", "conclusion": null}, + ]}}}}]}, + }, + }}})), + Some("PullRequestMergeMessage") => { + json_reply(json!({"data": {"repository": {"pullRequest": { + "isMergeQueueEnabled": model.queue, "headRefOid": model.head, + "viewerMergeBodyText": model.message, + }}}})) + } + // GitHub merges at once, queues behind a merge queue, or arms auto-merge otherwise. + Some("MergePullRequest") => { + json_reply(json!({"data": {"mergePullRequest": {"pullRequest": { + "merged": true, "mergeQueueEntry": null, "autoMergeRequest": null, + }}}})) + } + Some("EnablePullRequestAutoMerge") => { + let method = seen.variables()["input"]["mergeMethod"].clone(); + json_reply( + json!({"data": {"enablePullRequestAutoMerge": {"pullRequest": if model.queue { + json!({"merged": false, "mergeQueueEntry": {"position": 2}, "autoMergeRequest": null}) + } else { + json!({"merged": false, "mergeQueueEntry": null, "autoMergeRequest": {"mergeMethod": method}}) + }}}}), + ) + } + Some("RevertPullRequest") => json_reply(json!({"data": {"revertPullRequest": { + "revertPullRequest": {"number": 8, "url": "https://github.com/octo/repo/pull/8"}, + }}})), + _ => writes_reply(seen), + } +} + +fn reads_of(log: &Log, operation: &str) -> usize { + log.lock() + .unwrap() + .iter() + .filter(|seen| seen.operation().as_deref() == Some(operation)) + .count() +} + +fn merge(method: PullRequestMergeMethod, auto: bool) -> PullRequestAction { + PullRequestAction::Merge { + head: HEAD.into(), + method, + auto, + remove_credits: false, + } +} + +#[test] +fn a_merge_reads_the_head_fresh_and_tells_merged_queued_and_armed_apart() { + let fixture = Fixture::new(); + let store = Store::new(); + let reads = reads(&fixture, &store); + let model = Arc::new(Mutex::new(Lifecycle::default())); + let answering = model.clone(); + let (_server, log) = serve(fixture, move |seen| lifecycle_reply(&answering, seen)); + use PullRequestMergeMethod::*; + + let state = reads.action_state(&key()).unwrap().value; + assert_eq!( + ( + state.merge_methods.clone(), + state.failing_checks.clone(), + state.pending_checks, + state.behind_by ), - (1, 1) + ( + vec![Merge, Squash, Rebase], + vec!["lint".to_owned()], + 1, + Some(2) + ) + ); + + assert_eq!( + reads.act(&key(), &merge(Squash, false)), + PullRequestActionResult::Applied + ); + assert_eq!( + reads.act(&key(), &merge(Merge, true)), + PullRequestActionResult::Applied, + "auto-merge asked of a pull request that can merge now merges it" + ); + model.lock().unwrap().merge_state = "BLOCKED"; + assert_eq!( + reads.act(&key(), &merge(Squash, true)), + PullRequestActionResult::AutoMergeEnabled { method: Squash } + ); + model.lock().unwrap().queue = true; + assert_eq!( + reads.act(&key(), &merge(Rebase, false)), + PullRequestActionResult::Queued { position: Some(2) }, + "a merge queue takes the pull request; it is not merged" + ); + model.lock().unwrap().rebase_allowed = false; + assert_eq!( + reads.act(&key(), &merge(Rebase, false)), + PullRequestActionResult::Rejected(PullRequestRejection::Invalid), + "a method the repository does not allow sends nothing" + ); + model.lock().unwrap().head = BASE; + assert_eq!( + reads.act(&key(), &merge(Squash, false)), + PullRequestActionResult::Rejected(PullRequestRejection::StaleHead { head: BASE.into() }), + "a head that moved since the user looked sends nothing" + ); + + let input = |method: &str| json!({"input": {"pullRequestId": "PR_node_7", "mergeMethod": method, "expectedHeadOid": HEAD}}); + assert_eq!( + mutations(&log), + vec![ + ("MergePullRequest".into(), input("SQUASH")), + ("MergePullRequest".into(), input("MERGE")), + ("EnablePullRequestAutoMerge".into(), input("SQUASH")), + ("EnablePullRequestAutoMerge".into(), input("REBASE")), + ] + ); +} + +#[test] +fn credits_leave_the_merge_message_only_when_it_has_some() { + let fixture = Fixture::new(); + let store = Store::new(); + let reads = reads(&fixture, &store); + let model = Arc::new(Mutex::new(Lifecycle::default())); + let answering = model.clone(); + let (_server, log) = serve(fixture, move |seen| lifecycle_reply(&answering, seen)); + let cleaning = |method| PullRequestAction::Merge { + head: HEAD.into(), + method, + auto: false, + remove_credits: true, + }; + + reads.act(&key(), &cleaning(PullRequestMergeMethod::Squash)); + model.lock().unwrap().message = "Fix the parser\n\nCo-authored-by: Ada \nCo-authored-by: Claude \n\n🤖 Generated with [Claude Code](https://claude.ai/code)\n"; + reads.act(&key(), &cleaning(PullRequestMergeMethod::Merge)); + reads.act(&key(), &cleaning(PullRequestMergeMethod::Rebase)); + + let bodies: Vec<_> = mutations(&log) + .into_iter() + .map(|(_, variables)| variables["input"]["commitBody"].clone()) + .collect(); + assert_eq!( + bodies, + vec![ + Value::Null, + json!("Fix the parser\n\nCo-authored-by: Ada "), + Value::Null, + ], + "a message with no agent credit is GitHub's own; people stay credited" + ); + assert_eq!( + reads_of(&log, "PullRequestMergeMessage"), + 2, + "rebasing keeps each commit's message, so none is read" + ); +} + +#[test] +fn a_branch_update_carries_the_head_and_is_nothing_when_not_behind() { + let fixture = Fixture::new(); + let store = Store::new(); + let reads = reads(&fixture, &store); + let model = Arc::new(Mutex::new(Lifecycle::default())); + let answering = model.clone(); + let (_server, log) = serve(fixture, move |seen| lifecycle_reply(&answering, seen)); + let update = |head: &str, rebase| PullRequestAction::UpdateBranch { + head: head.into(), + rebase, + }; + + assert_eq!( + reads.act(&key(), &update(HEAD, false)), + PullRequestActionResult::Applied + ); + assert_eq!( + reads.act(&key(), &update(HEAD, true)), + PullRequestActionResult::Applied + ); + assert_eq!( + reads.act(&key(), &update(BASE, false)), + PullRequestActionResult::Rejected(PullRequestRejection::StaleHead { head: HEAD.into() }) + ); + model.lock().unwrap().behind_by = 0; + assert_eq!( + reads.act(&key(), &update(HEAD, false)), + PullRequestActionResult::UpToDate + ); + let update = |method: &str| { + ( + "UpdatePullRequestBranch".to_owned(), + json!({"pullRequestId": "PR_node_7", "expectedHeadOid": HEAD, "updateMethod": method}), + ) + }; + assert_eq!(mutations(&log), vec![update("MERGE"), update("REBASE")]); +} + +#[test] +fn lifecycle_writes_name_the_pull_request_and_a_revert_opens_one() { + let fixture = Fixture::new(); + let store = Store::new(); + let reads = reads(&fixture, &store); + let model = Arc::new(Mutex::new(Lifecycle::default())); + let (_server, log) = serve(fixture, move |seen| lifecycle_reply(&model, seen)); + + for action in [ + PullRequestAction::ReadyForReview, + PullRequestAction::ConvertToDraft, + PullRequestAction::Close, + PullRequestAction::Reopen, + PullRequestAction::DisableAutoMerge, + ] { + assert_eq!(reads.act(&key(), &action), PullRequestActionResult::Applied); + } + assert_eq!( + reads.act(&key(), &PullRequestAction::Revert), + PullRequestActionResult::Opened { + number: 8, + url: "https://github.com/octo/repo/pull/8".into() + } + ); + let named = json!({"pullRequestId": "PR_node_7"}); + assert_eq!( + mutations(&log), + [ + "MarkPullRequestReady", + "ConvertPullRequestToDraft", + "ClosePullRequest", + "ReopenPullRequest", + "DisablePullRequestAutoMerge", + "RevertPullRequest", + ] + .map(|operation| (operation.to_owned(), named.clone())) + .to_vec() ); } diff --git a/crates/ui/src/assets.rs b/crates/ui/src/assets.rs index 1d700c6ad..35c102145 100644 --- a/crates/ui/src/assets.rs +++ b/crates/ui/src/assets.rs @@ -26,6 +26,9 @@ gpui_component_assets::icon_assets!( Users, Tag, CircleDot, + ListOrdered, + Hourglass, + ArrowUpDown, ] ); diff --git a/crates/ui/src/composer/mod.rs b/crates/ui/src/composer/mod.rs index 106887fa2..7761dbeac 100644 --- a/crates/ui/src/composer/mod.rs +++ b/crates/ui/src/composer/mod.rs @@ -456,6 +456,23 @@ impl Composer { self.set_input_text(prefill, window, cx); } + /// Text another surface put here goes after what the user typed, which it never replaces. + fn sync_composer_append(&mut self, window: &mut Window, cx: &mut Context) { + let Some(text) = self + .workspace_store + .update(cx, |store, _cx| store.take_composer_append()) + else { + return; + }; + let typed = self.input.read(cx).value().trim_end().to_owned(); + let text = if typed.is_empty() { + text + } else { + format!("{typed}\n\n{text}") + }; + self.set_input_text(text, window, cx); + } + /// Replace the composer text with `text`, caret at the end. /// Software-keyboard devices wait for an explicit tap to focus. fn set_input_text(&mut self, text: String, window: &mut Window, cx: &mut Context) { @@ -1018,6 +1035,7 @@ impl Render for Composer { self.sync_text_destination(window, cx); self.sync_queued_refill(window, cx); self.sync_native_rewind_prefill(window, cx); + self.sync_composer_append(window, cx); self.sync_fallback_review_draft(window, cx); let composer_state = self.workspace_store.read(cx).composer_state(); let readonly = composer_state.conversation_read_only; diff --git a/crates/ui/src/pull_requests/compose.rs b/crates/ui/src/pull_requests/compose.rs index 5a966acf3..ce87dbefb 100644 --- a/crates/ui/src/pull_requests/compose.rs +++ b/crates/ui/src/pull_requests/compose.rs @@ -104,7 +104,7 @@ pub(super) enum Write { } /// One toast slot per pull request: a newer answer replaces the older one. -struct PullRequestWrite; +pub(super) struct PullRequestWrite; /// The host's reason a write was not done, in words. pub(super) fn rejection_reason(rejection: &PullRequestRejection) -> String { @@ -127,6 +127,14 @@ pub(super) fn rejection_reason(rejection: &PullRequestRejection) -> String { } PullRequestRejection::Refused { .. } => crate::tr!("pull_requests.result.reason_invalid"), PullRequestRejection::Failed => crate::tr!("pull_requests.result.reason_failed"), + PullRequestRejection::InStack { index, layers } => crate::tr!( + "pull_requests.result.reason_stack", + index = index.to_string(), + count = layers.to_string() + ), + PullRequestRejection::StackUnknown => { + crate::tr!("pull_requests.result.reason_stack_unknown") + } } .into_owned() } @@ -208,6 +216,11 @@ fn toast(write: Write, result: &PullRequestActionResult, number: u64) -> Option< .into_owned(), ) } + // Lifecycle answers, which no conversation write gets. + PullRequestActionResult::UpToDate + | PullRequestActionResult::Queued { .. } + | PullRequestActionResult::AutoMergeEnabled { .. } + | PullRequestActionResult::Opened { .. } => return None, PullRequestActionResult::Uncertain => match write { Write::Comment | Write::Edit => Notification::warning( crate::tr!("pull_requests.result.uncertain_comment").into_owned(), @@ -244,7 +257,7 @@ fn toast(write: Write, result: &PullRequestActionResult, number: u64) -> Option< } /// A command that never reached an answer may still have reached GitHub. -fn answer_of( +pub(super) fn answer_of( result: Result, ) -> Result { match result { diff --git a/crates/ui/src/pull_requests/detail.rs b/crates/ui/src/pull_requests/detail.rs index 1155b489d..6a10c3d43 100644 --- a/crates/ui/src/pull_requests/detail.rs +++ b/crates/ui/src/pull_requests/detail.rs @@ -156,6 +156,11 @@ pub(super) struct PullRequestPage { /// A manual refresh reads Files again from page 1, whatever was paged in. pub(super) restart_files: bool, pub(super) writes: super::compose::Writes, + /// What merging and the other lifecycle actions would meet, read while the pull request is + /// open. + pub(super) action_state: Read, + /// The merge method this client chose for the pull request in its menu. + pub(super) merge_method: Option, } pub struct PullRequestView { @@ -172,6 +177,9 @@ pub struct PullRequestView { _subscriptions: [Subscription; 2], } +/// The busy mark of a lifecycle write in flight. +const LIFECYCLE: &str = "lifecycle"; + pub(super) fn now() -> u64 { tcode_core::project::now_secs() } @@ -321,9 +329,19 @@ impl PullRequestView { land: impl FnOnce(&mut PullRequestPage, Result<(PullRequestReadResponse, u64), ProtocolError>) + 'static, ) { - let Some((session, key)) = self.current.clone() else { - return; - }; + if let Some(current) = self.current.clone() { + self.read_of(current, read, cx, land); + } + } + + fn read_of( + &mut self, + (session, key): (String, PullRequestKey), + read: PullRequestRead, + cx: &mut Context, + land: impl FnOnce(&mut PullRequestPage, Result<(PullRequestReadResponse, u64), ProtocolError>) + + 'static, + ) { let task = self.store.update(cx, |store, cx| { store.read_pull_request(session.clone(), key.clone(), read, cx) }); @@ -469,26 +487,171 @@ impl PullRequestView { }); } + pub(super) fn read_action_state(&mut self, cx: &mut Context) { + if let Some(current) = self.current.clone() { + self.read_action_state_of(current, cx); + } + } + + fn read_action_state_of(&mut self, page: (String, PullRequestKey), cx: &mut Context) { + self.pages + .entry(page.clone()) + .or_default() + .action_state + .loading = true; + self.read_of(page, PullRequestRead::ActionState, cx, |page, result| { + page.action_state.loading = false; + match result { + Ok((PullRequestReadResponse::ActionState(state), expires_at)) => { + page.action_state.expires_at = expires_at; + page.action_state.error = None; + page.action_state.loaded_at = now(); + page.action_state.data = Some(state); + // GitHub's state as it now is answers a write that went unanswered. + page.writes.waiting = false; + } + Ok(_) => {} + Err(error) => page.action_state.error = Some(error), + } + }); + } + + /// Whether a lifecycle write to the pull request is in flight, or went unanswered and + /// GitHub has not been read since; the detail and the list rows wait alike. + pub(super) fn lifecycle_busy(&self, session: &str, key: &PullRequestKey) -> bool { + self.pages + .get(&(session.to_owned(), key.clone())) + .is_some_and(|page| page.writes.waiting || page.writes.busy.contains(LIFECYCLE)) + } + + /// Where a lifecycle action goes; its answer has the view read what it may have changed. + /// `shown` carries what the detail has read of it; a list row offers from its link alone. + pub(super) fn lifecycle_target( + &self, + session: &str, + key: &PullRequestKey, + shown: bool, + cx: &mut Context, + ) -> Option { + let current = (session.to_owned(), key.clone()); + let page = self.pages.get(¤t).filter(|_| shown); + let mut target = super::lifecycle::Target::new( + &self.store, + &self.window_state, + session, + key, + page.and_then(|page| page.action_state.data.clone()), + page.and_then(|page| page.merge_method), + cx, + )?; + target.offer.busy = self.lifecycle_busy(session, key); + let view = cx.entity().downgrade(); + let sent = current.clone(); + target.started = std::rc::Rc::new({ + let view = view.clone(); + move |cx| { + let _ = view.update(cx, |view, cx| { + let page = view.pages.entry(sent.clone()).or_default(); + page.writes.busy.insert(LIFECYCLE.into()); + cx.notify(); + }); + } + }); + target.done = std::rc::Rc::new(move |result, _, cx| { + let _ = view.update(cx, |view, cx| { + let page = view.pages.entry(current.clone()).or_default(); + page.writes.busy.remove(LIFECYCLE); + page.action_state.expires_at = 0; + if !matches!(result, tcode_protocol::PullRequestActionResult::Rejected(_)) { + page.conversation.expires_at = 0; + page.files.expires_at = 0; + } + if *result == tcode_protocol::PullRequestActionResult::Uncertain { + page.writes.waiting = true; + // The detail reads it when shown; a row's pull request is read here. + if view.current.as_ref() != Some(¤t) { + view.read_action_state_of(current.clone(), cx); + } + } + cx.notify(); + }); + }); + Some(target) + } + + /// The target of the pull request on view. + fn shown_target(&self, cx: &mut Context) -> Option { + let (session, key) = self.current.as_ref()?; + self.lifecycle_target(session, key, true, cx) + } + + fn run_lifecycle( + &mut self, + action: &super::lifecycle::RunLifecycle, + window: &mut Window, + cx: &mut Context, + ) { + if self.current.as_ref().map(|(_, key)| key) != Some(&action.key) { + cx.propagate(); + return; + } + if let Some(target) = self.shown_target(cx) { + target.run(action.kind, window, cx); + } + } + + fn choose_merge_method( + &mut self, + action: &super::lifecycle::ChooseMergeMethod, + _: &mut Window, + cx: &mut Context, + ) { + if self.current.as_ref().map(|(_, key)| key) != Some(&action.key) { + return; + } + if let Some(page) = self.page_mut() { + page.merge_method = Some(action.method); + } + cx.notify(); + } + /// Starts the reads the visible view needs, and wakes when the soonest of them expires. fn ensure_reads(&mut self, cx: &mut Context) { let tab = self.tab(cx); let Some(key) = self.current.as_ref().map(|(_, key)| key.clone()) else { return; }; + let open = self + .link(cx) + .and_then(|link| link.snapshot) + .is_some_and(|snapshot| snapshot.state == PullRequestState::Open); let Some(page) = self.page_mut() else { return }; let now = now(); + let action_due = open && page.action_state.due(now); + let action_expires = page.action_state.expires_at; let (files_due, conversation_due, viewed_due) = ( page.files.due(now), page.conversation.due(now), page.viewed.due(now), ); + let (conversation_expires, files_expires, viewed_expires) = ( + page.conversation.expires_at, + page.files.expires_at, + page.viewed.expires_at, + ); if conversation_due && page.conversation.data.is_some() { // Media that failed is asked again along with the conversation that names it. crate::store::retry_failed_pull_request_media(&key, cx); } - let mut expiries = vec![page.conversation.expires_at]; + let mut expiries = vec![conversation_expires]; + if open { + expiries.push(action_expires); + } + if action_due { + self.read_action_state(cx); + } if tab == Tab::Files { - expiries.extend([page.files.expires_at, page.viewed.expires_at]); + expiries.extend([files_expires, viewed_expires]); } match tab { Tab::Files => { @@ -539,6 +702,8 @@ impl PullRequestView { page.files.error = None; page.conversation.error = None; page.viewed.error = None; + page.action_state.error = None; + page.action_state.expires_at = 0; page.conversation_view.replies.clear(); let task = self.store.update(cx, |store, cx| { store.command( @@ -588,6 +753,7 @@ impl PullRequestView { page.files.error = None; page.conversation.error = None; page.viewed.error = None; + page.action_state.error = None; } cx.notify(); } @@ -758,6 +924,10 @@ impl PullRequestView { }; chips = chips.child(chip(icon, color, crate::tr!(label).into_owned())); } + if let Some(behind) = self.behind_chip(&key, snapshot, cx) { + has_chip = true; + chips = chips.child(behind); + } if snapshot.mergeability == Mergeability::Conflicting { has_chip = true; chips = chips.child(chip( @@ -843,6 +1013,74 @@ impl PullRequestView { .into_any_element() } + /// How far the head is behind its base, and the updates the account may make from it. Not on + /// a stack layer, whose layers move together, nor while it conflicts. + fn behind_chip( + &self, + key: &PullRequestKey, + snapshot: &tcode_core::pull_request::PullRequestSnapshot, + cx: &mut Context, + ) -> Option { + let offer = self.shown_target(cx)?.offer; + let state = offer.action.as_ref()?; + let behind = state.behind_by.filter(|behind| *behind > 0)?; + if offer.conflicting + || offer.route != tcode_core::pull_request::PullRequestStackRoute::Single + { + return None; + } + let base = snapshot.base_branch.clone(); + let label = if behind == 1 { + crate::tr!("pull_requests.actions.behind_one", base = base.clone()).into_owned() + } else { + crate::tr!( + "pull_requests.actions.behind", + count = behind.to_string(), + base = base.clone() + ) + .into_owned() + }; + let tooltip = crate::tr!( + "pull_requests.actions.update_tooltip", + base = base, + head = snapshot.head_branch.clone() + ) + .into_owned(); + let updates = (state.can_update_branch && !self.read_only(cx)).then(|| key.clone()); + let busy = offer.busy; + let chip = Button::new("pr-behind") + .ghost() + .compact() + .xsmall() + .icon(IconName::ArrowUpDown) + .label(label); + Some(match updates { + Some(key) => chip + .dropdown_menu(move |menu, _, _| { + let item = |kind| { + Box::new(super::lifecycle::RunLifecycle { + key: key.clone(), + kind, + }) + }; + menu.label(tooltip.clone()) + .menu_with_enable( + crate::tr!("pull_requests.actions.update_branch").into_owned(), + item(super::lifecycle::Lifecycle::UpdateBranch), + !busy, + ) + .menu_with_enable( + crate::tr!("pull_requests.actions.update_rebase_menu").into_owned(), + item(super::lifecycle::Lifecycle::UpdateRebase), + !busy, + ) + }) + .into_any_element(), + // News, not an offer, for an account that may not update the branch. + None => chip.tooltip(tooltip).into_any_element(), + }) + } + /// The title, its edit affordance (`pr-title-edit`) and the title editor. fn title_line(&self, title: String, known: bool, cx: &mut Context) -> AnyElement { let compact = self.compact(cx); @@ -1235,7 +1473,10 @@ impl PullRequestView { } /// The menu of the row this pull request was opened from. - fn menu(&self, cx: &App) -> Option { + fn menu(&self, cx: &mut Context) -> Option { + let offer = (!self.read_only(cx)) + .then(|| self.shown_target(cx).map(|target| target.offer)) + .flatten(); let (session, key) = self.current.as_ref()?; let link = self.link(cx); let watchable = self @@ -1250,6 +1491,7 @@ impl PullRequestView { self.url(cx).unwrap_or_default(), link.as_ref(), watchable, + move |_| offer.clone(), )) } @@ -1283,6 +1525,7 @@ impl PullRequestView { }), ) .child(div().flex_1()) + .children(self.primary(false, cx)) .child(self.refresh_button(refreshing, false, cx)) .when_some(url, |bar, url| { bar.child( @@ -1317,6 +1560,19 @@ impl PullRequestView { .into_any_element() } + /// The header's primary action (`pr-primary-action`): one, by the rank, from the host's + /// action state. + fn primary(&self, compact: bool, cx: &mut Context) -> Option { + if self.read_only(cx) { + return None; + } + let target = self.shown_target(cx)?; + let primary = target.offer.primary()?; + Some(super::lifecycle::primary_element( + &target, primary, compact, cx, + )) + } + pub(super) fn refresh_button( &self, refreshing: bool, @@ -1769,8 +2025,19 @@ impl Render for PullRequestView { .on_action(cx.listener(Self::change_watch)) .on_action(cx.listener(Self::on_selection_menu)) .on_action(cx.listener(Self::on_comment_menu)) + .on_action(cx.listener(Self::run_lifecycle)) + .on_action(cx.listener(Self::choose_merge_method)) .when(!compact, |view| view.child(self.sub_bar(cx))) .child(self.header(window, cx)) + .when(compact, |view| { + view.children(self.primary(true, cx).map(|primary| { + div() + .flex_none() + .px(px(material::COMPACT_PAGE_INSET)) + .pb_2() + .child(primary) + })) + }) .children(self.notices(cx)) .child(self.switch(cx)) .child(div().flex_1().min_h_0().flex().flex_col().child(body)) diff --git a/crates/ui/src/pull_requests/lifecycle.rs b/crates/ui/src/pull_requests/lifecycle.rs new file mode 100644 index 000000000..cfabf940e --- /dev/null +++ b/crates/ui/src/pull_requests/lifecycle.rs @@ -0,0 +1,1524 @@ +//! What can become of a pull request: the primary action, the ⋯ menu's lifecycle group, the +//! confirmations, the merge dialog and the words each answer gets. The host reads the pull +//! request fresh before a merge or a branch update and answers every write; nothing here +//! retries one. + +use std::rc::Rc; + +use gpui::{ + Action, AnyElement, App, AppContext as _, Context, Entity, IntoElement, ParentElement as _, + Render, SharedString, Styled as _, Window, div, prelude::FluentBuilder as _, px, +}; +use gpui_base::{h_flex, v_flex}; +use serde::Deserialize; +use tcode_core::pull_request::{ + ChecksState, Mergeability, PullRequestKey, PullRequestMergeMethod, PullRequestStackRoute, + PullRequestState, ThreadPullRequestLink, stack_route, +}; +use tcode_protocol::{ + Command, PullRequestAction, PullRequestActionResult, PullRequestActionState, + PullRequestMergeState, PullRequestRead, PullRequestReadResponse, PullRequestRejection, +}; + +use super::compose::{answer_of, rejection_reason}; +use crate::{ + icon::{Icon, IconName}, + material, + overlay::{DialogButtons, Notification, OverlayExt as _}, + sizing::Sizable as _, + store::WorkspaceStore, + theme::ActiveTheme as _, + widgets::{ + button::{Button, ButtonVariant, ButtonVariants as _}, + checkbox::Checkbox, + menu::{OpenUrl, PopupMenu}, + spinner::Spinner, + }, + window_state::{Destination, WindowState}, +}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +pub(super) enum Lifecycle { + Ready, + Draft, + Close, + Reopen, + Revert, + UpdateBranch, + UpdateRebase, + Merge, + EnableAutoMerge, + DisableAutoMerge, + AskConflicts, + AskChecks, +} + +/// A lifecycle item of a pull request's menu or primary action. +#[derive(Action, Clone, PartialEq, Deserialize)] +#[action(namespace = tcode_pull_requests, no_json)] +pub(super) struct RunLifecycle { + pub(super) key: PullRequestKey, + pub(super) kind: Lifecycle, +} + +/// The method this client merges a pull request with, from the menu's checks. +#[derive(Action, Clone, PartialEq, Deserialize)] +#[action(namespace = tcode_pull_requests, no_json)] +pub(super) struct ChooseMergeMethod { + pub(super) key: PullRequestKey, + pub(super) method: PullRequestMergeMethod, +} + +/// The one action a pull request's header offers first, or a status in its place. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum Primary { + ResolveConflicts, + Ready, + FixChecks, + AutoMergeOn(PullRequestMergeMethod), + Queued(Option), + MergeOnGitHub { + index: u32, + }, + UpdateBranch, + EnableAutoMerge, + Merge { + method: PullRequestMergeMethod, + queue: bool, + }, +} + +/// What a pull request may meet, as the thread's link and the host's action state say. +#[derive(Clone)] +pub(super) struct Offer { + pub(super) key: PullRequestKey, + pub(super) url: String, + pub(super) state: PullRequestState, + pub(super) draft: bool, + pub(super) conflicting: bool, + pub(super) checks_failing: bool, + pub(super) checks_pending: bool, + pub(super) route: PullRequestStackRoute, + /// The host's action state; a list row has none until one of its items is chosen. + pub(super) action: Option, + /// The method a merge would use. + pub(super) method: Option, + /// A write is in flight, or one went unanswered and GitHub has not been read since. + pub(super) busy: bool, +} + +impl Offer { + /// `None` until the link or its stack says what state the pull request is in. The host's + /// action state, read fresh, speaks over the link's snapshot where it has a word. + pub(super) fn new( + key: &PullRequestKey, + links: &[ThreadPullRequestLink], + action: Option, + chosen: Option, + project_default: Option, + busy: bool, + ) -> Option { + let link = links.iter().find(|link| link.key == *key && link.visible()); + let snapshot = link.and_then(|link| link.snapshot.as_ref()); + let (route, layer) = stack_route(links, key); + let state = snapshot + .map(|snapshot| snapshot.state) + .or(layer.map(|layer| layer.state))?; + let url = link + .map(|link| link.url.clone()) + .filter(|url| !url.is_empty()) + .or(layer.map(|layer| layer.url.clone())) + .unwrap_or_default(); + let method = action + .as_ref() + .and_then(|action| merge_method(&action.merge_methods, chosen, project_default)); + let conflicting = match &action { + Some(action) if action.merge_state != PullRequestMergeState::Unknown => { + action.merge_state == PullRequestMergeState::Dirty + } + _ => snapshot.is_some_and(|s| s.mergeability == Mergeability::Conflicting), + }; + let checks = |checks: ChecksState| snapshot.is_some_and(|s| s.checks_state == Some(checks)); + Some(Self { + key: key.clone(), + url, + state, + draft: snapshot.is_some_and(|snapshot| snapshot.is_draft), + conflicting, + checks_failing: match &action { + Some(action) => !action.failing_checks.is_empty(), + None => checks(ChecksState::Failing), + }, + checks_pending: match &action { + Some(action) => action.pending_checks > 0, + None => checks(ChecksState::Pending), + }, + route, + action, + method, + busy, + }) + } + + /// The rank: conflicts, then ready for review, then failing checks, then what is already + /// under way, then the merge itself. A stack whose state is unknown offers no merge. + pub(super) fn primary(&self) -> Option { + let action = self.action.as_ref()?; + if self.state != PullRequestState::Open { + return None; + } + if self.conflicting { + return Some(Primary::ResolveConflicts); + } + if self.draft { + return action.can_update.then_some(Primary::Ready); + } + if self.checks_failing { + return Some(Primary::FixChecks); + } + if let Some(method) = action.auto_merge { + return Some(Primary::AutoMergeOn(method)); + } + if action.queued { + return Some(Primary::Queued(action.queue_position)); + } + match self.route { + PullRequestStackRoute::Layer { index, .. } => { + return Some(Primary::MergeOnGitHub { index }); + } + PullRequestStackRoute::Unknown => return None, + PullRequestStackRoute::Single => {} + } + if action.merge_state == PullRequestMergeState::Behind && action.can_update_branch { + return Some(Primary::UpdateBranch); + } + if !action.can_merge { + return None; + } + if self.checks_pending && action.auto_merge_allowed && !action.merge_queue { + return Some(Primary::EnableAutoMerge); + } + let mergeable = action.merge_queue + || matches!( + action.merge_state, + PullRequestMergeState::Clean + | PullRequestMergeState::Unstable + | PullRequestMergeState::HasHooks + ); + Some(Primary::Merge { + method: self.method.filter(|_| mergeable)?, + queue: action.merge_queue, + }) + } + + fn item(&self, kind: Lifecycle) -> Box { + Box::new(RunLifecycle { + key: self.key.clone(), + kind, + }) + } + + /// The menu's lifecycle group, added once to the row menu every pull request menu builds. + /// Its writes wait while one is in flight or unanswered. + pub(super) fn menu(&self, menu: PopupMenu) -> PopupMenu { + let action = self.action.as_ref(); + let may = |allowed: fn(&PullRequestActionState) -> bool| action.is_none_or(allowed); + let primary = self.primary(); + let open = self.state == PullRequestState::Open; + let number = self.key.number.to_string(); + let write = |menu: PopupMenu, label: &str, kind: Lifecycle| { + menu.menu_with_enable(crate::tr!(label).into_owned(), self.item(kind), !self.busy) + }; + let mut menu = menu.separator(); + if open && may(|action| action.can_update) { + if !self.draft { + menu = write(menu, "pull_requests.actions.draft", Lifecycle::Draft); + } else if primary != Some(Primary::Ready) { + menu = write(menu, "pull_requests.actions.ready", Lifecycle::Ready); + } + } + if open + && self.route == PullRequestStackRoute::Single + && !self.conflicting + && let Some(action) = action + && action.behind_by.is_some_and(|behind| behind > 0) + && action.can_update_branch + { + menu = write( + menu, + "pull_requests.actions.update_branch", + Lifecycle::UpdateBranch, + ); + menu = write( + menu, + "pull_requests.actions.update_rebase_menu", + Lifecycle::UpdateRebase, + ); + } + if open && !self.draft && may(|action| action.can_merge) { + match self.route { + PullRequestStackRoute::Single => { + if !self.conflicting + && !matches!(primary, Some(Primary::Merge { .. })) + && action.is_none_or(|action| !action.merge_methods.is_empty()) + { + // GitHub would refuse it until reviews or checks allow. + let blocked = action.is_some_and(|action| { + action.merge_state == PullRequestMergeState::Blocked + }); + menu = menu.menu_with_enable( + crate::tr!("pull_requests.actions.merge_now").into_owned(), + self.item(Lifecycle::Merge), + !self.busy && !blocked, + ); + if blocked { + menu = menu + .label(crate::tr!("pull_requests.actions.blocked").into_owned()); + } + } + if let Some(action) = action { + if action.auto_merge.is_some() { + menu = write( + menu, + "pull_requests.actions.disable_auto_merge", + Lifecycle::DisableAutoMerge, + ); + } else if action.auto_merge_allowed + && !action.merge_queue + && primary != Some(Primary::EnableAutoMerge) + { + menu = write( + menu, + "pull_requests.actions.enable_auto_merge_menu", + Lifecycle::EnableAutoMerge, + ); + } + if action.merge_methods.len() >= 2 { + menu = menu.label( + crate::tr!("pull_requests.actions.merge_method").into_owned(), + ); + for method in &action.merge_methods { + menu = menu.menu_with_check( + method_label(*method), + self.method == Some(*method), + Box::new(ChooseMergeMethod { + key: self.key.clone(), + method: *method, + }), + ); + } + } + } + } + PullRequestStackRoute::Layer { index, .. } => { + menu = menu.label(stack_reason(&number, index)).menu( + crate::tr!("pull_requests.actions.merge_on_github").into_owned(), + Box::new(OpenUrl(self.url.clone())), + ); + } + PullRequestStackRoute::Unknown => { + menu = menu + .menu_with_enable( + crate::tr!("pull_requests.actions.merge_now").into_owned(), + self.item(Lifecycle::Merge), + false, + ) + .label( + crate::tr!("pull_requests.actions.stack_checking", number = number) + .into_owned(), + ); + } + } + } + menu = menu.separator(); + match self.state { + PullRequestState::Open if may(|action| action.can_update) => { + write(menu, "pull_requests.actions.close_menu", Lifecycle::Close) + } + PullRequestState::Closed if may(|action| action.can_update) => { + write(menu, "pull_requests.actions.reopen", Lifecycle::Reopen) + } + PullRequestState::Merged if may(|action| action.can_merge) => { + write(menu, "pull_requests.actions.revert_menu", Lifecycle::Revert) + } + _ => menu, + } + } +} + +/// The client's choice, then the project's default, then the first the repository enables. +fn merge_method( + enabled: &[PullRequestMergeMethod], + chosen: Option, + project_default: Option, +) -> Option { + [chosen, project_default] + .into_iter() + .flatten() + .find(|method| enabled.contains(method)) + .or_else(|| enabled.first().copied()) +} + +pub(super) fn method_label(method: PullRequestMergeMethod) -> String { + crate::tr!(match method { + PullRequestMergeMethod::Merge => "pull_requests.merge.method_merge", + PullRequestMergeMethod::Squash => "pull_requests.merge.method_squash", + PullRequestMergeMethod::Rebase => "pull_requests.merge.method_rebase", + }) + .into_owned() +} + +fn segment_label(method: PullRequestMergeMethod) -> String { + crate::tr!(match method { + PullRequestMergeMethod::Merge => "pull_requests.merge.segment_merge", + PullRequestMergeMethod::Squash => "pull_requests.merge.segment_squash", + PullRequestMergeMethod::Rebase => "pull_requests.merge.segment_rebase", + }) + .into_owned() +} + +pub(super) fn stack_reason(number: &str, index: u32) -> String { + crate::tr!( + "pull_requests.actions.stack_reason", + number = number.to_owned(), + index = index.to_string() + ) + .into_owned() +} + +fn short(head: &str) -> String { + head.chars().take(7).collect() +} + +type Answered = Rc; + +/// Where a lifecycle action goes and who hears its answer. +#[derive(Clone)] +pub(super) struct Target { + pub(super) store: Entity, + pub(super) window_state: Entity, + pub(super) session: String, + pub(super) offer: Offer, + pub(super) title: String, + pub(super) head_branch: String, + pub(super) base_branch: String, + /// The thread's project, by id and name. + project: Option<(String, String)>, + project_default: Option, + /// Told when the write is sent. + pub(super) started: Rc, + /// Told every answer, after its toast. + pub(super) done: Answered, +} + +impl Target { + /// The pull request as the thread's links show it. + pub(super) fn new( + store: &Entity, + window_state: &Entity, + session: &str, + key: &PullRequestKey, + action: Option, + chosen: Option, + cx: &App, + ) -> Option { + let workspace = store.read(cx); + let links = workspace.pull_requests(session); + let project = workspace + .thread_meta(session) + .and_then(|meta| meta.project_id.clone()) + .and_then(|id| { + workspace + .projects() + .into_iter() + .find(|project| project.id == id) + .map(|project| (project.id, project.name)) + }); + let project_default = project + .as_ref() + .and_then(|(id, _)| workspace.settings().project_merge_methods.get(id).copied()); + let offer = Offer::new(key, links, action, chosen, project_default, false)?; + let snapshot = links + .iter() + .find(|link| link.key == *key) + .and_then(|link| link.snapshot.clone()); + Some(Self { + store: store.clone(), + window_state: window_state.clone(), + session: session.to_owned(), + offer, + title: snapshot + .as_ref() + .map(|s| s.title.clone()) + .unwrap_or_default(), + head_branch: snapshot + .as_ref() + .map(|s| s.head_branch.clone()) + .unwrap_or_default(), + base_branch: snapshot.map(|s| s.base_branch).unwrap_or_default(), + project, + project_default, + started: Rc::new(|_| {}), + done: Rc::new(|_, _, _| {}), + }) + } + + fn number(&self) -> String { + self.offer.key.number.to_string() + } + + /// Whether the thread's composer may be given a message: not read-only on this device, and + /// neither settled nor archived. + fn takes_messages(&self, cx: &App) -> bool { + let workspace = self.store.read(cx); + workspace + .session_status() + .is_some_and(|status| !status.conversation_read_only) + && workspace + .thread_meta(&self.session) + .is_some_and(|meta| meta.archived_at.is_none() && !meta.is_settled()) + } + + /// Writes at once, confirms first, opens the merge dialog, or fills the composer. + pub(super) fn run(self, kind: Lifecycle, window: &mut Window, cx: &mut App) { + let number = self.number(); + match kind { + Lifecycle::Ready => self.send(PullRequestAction::ReadyForReview, kind, window, cx), + Lifecycle::Draft => self.send(PullRequestAction::ConvertToDraft, kind, window, cx), + Lifecycle::Reopen => self.send(PullRequestAction::Reopen, kind, window, cx), + Lifecycle::DisableAutoMerge => { + self.send(PullRequestAction::DisableAutoMerge, kind, window, cx) + } + Lifecycle::UpdateBranch => { + let Some(head) = self.offer.action.as_ref().map(|a| a.head.clone()) else { + return; + }; + self.send( + PullRequestAction::UpdateBranch { + head, + rebase: false, + }, + kind, + window, + cx, + ) + } + Lifecycle::Close => self.confirm( + crate::tr!("pull_requests.actions.close_title", number = number.clone()), + crate::tr!("pull_requests.actions.close_desc", number = number), + crate::tr!("pull_requests.actions.close_confirm"), + ButtonVariant::Danger, + PullRequestAction::Close, + kind, + window, + cx, + ), + Lifecycle::Revert => self.confirm( + crate::tr!( + "pull_requests.actions.revert_title", + number = number.clone() + ), + crate::tr!("pull_requests.actions.revert_desc", number = number), + crate::tr!("pull_requests.actions.revert_confirm"), + ButtonVariant::Primary, + PullRequestAction::Revert, + kind, + window, + cx, + ), + Lifecycle::UpdateRebase => { + let Some(head) = self.offer.action.as_ref().map(|a| a.head.clone()) else { + return; + }; + let title = crate::tr!( + "pull_requests.actions.rebase_title", + head = self.head_branch.clone(), + base = self.base_branch.clone() + ); + let description = crate::tr!( + "pull_requests.actions.rebase_desc", + head = self.head_branch.clone() + ); + self.confirm( + title, + description, + crate::tr!("pull_requests.actions.update_rebase"), + ButtonVariant::Primary, + PullRequestAction::UpdateBranch { head, rebase: true }, + kind, + window, + cx, + ) + } + Lifecycle::Merge => open_merge_dialog(self, false, window, cx), + Lifecycle::EnableAutoMerge => open_merge_dialog(self, true, window, cx), + Lifecycle::AskConflicts | Lifecycle::AskChecks => self.ask(kind, cx), + } + } + + /// Puts a message for the agent into this thread's composer; the user reads, edits and sends + /// it, or not. + fn ask(self, kind: Lifecycle, cx: &mut App) { + let number = self.number(); + let text = if kind == Lifecycle::AskConflicts { + crate::tr!( + "pull_requests.actions.ask_conflicts", + number = number, + head = self.head_branch.clone(), + base = self.base_branch.clone() + ) + .into_owned() + } else { + let failing = self + .offer + .action + .as_ref() + .map(|action| action.failing_checks.clone()) + .unwrap_or_default(); + if failing.is_empty() { + crate::tr!("pull_requests.actions.ask_checks_unnamed", number = number).into_owned() + } else { + let mut names = failing + .iter() + .take(5) + .cloned() + .collect::>() + .join(", "); + if failing.len() > 5 { + names.push(' '); + names.push_str(&crate::tr!( + "pull_requests.actions.and_more", + count = (failing.len() - 5).to_string() + )); + } + crate::tr!( + "pull_requests.actions.ask_checks", + number = number, + names = names + ) + .into_owned() + } + }; + let session = self.session.clone(); + self.store + .update(cx, |store, cx| store.append_to_composer(session, text, cx)); + // On a phone the composer is on the thread page under this one. + if self.window_state.read(cx).destination() == Destination::PullRequest { + self.window_state.update(cx, |state, cx| state.back(cx)); + } + } + + #[allow(clippy::too_many_arguments)] + fn confirm( + self, + title: std::borrow::Cow<'static, str>, + description: std::borrow::Cow<'static, str>, + confirm: std::borrow::Cow<'static, str>, + variant: ButtonVariant, + action: PullRequestAction, + kind: Lifecycle, + window: &mut Window, + cx: &mut App, + ) { + let (title, description, confirm) = ( + title.into_owned(), + description.into_owned(), + confirm.into_owned(), + ); + window.open_alert_dialog(cx, move |alert, _, cx| { + let target = self.clone(); + let action = action.clone(); + alert + .bg(cx.theme().popover) + .title(title.clone()) + .description(description.clone()) + .button_props( + DialogButtons::default() + .ok_variant(variant) + .ok_text(confirm.clone()) + .cancel_text(crate::tr!("pull_requests.actions.cancel")) + .show_cancel(true), + ) + .on_ok(move |_, window, cx| { + target.clone().send(action.clone(), kind, window, cx); + true + }) + }); + } + + /// One write, its toast, then whoever asked. + fn send(self, action: PullRequestAction, kind: Lifecycle, window: &mut Window, cx: &mut App) { + let task = self.command(action, cx); + window + .spawn(cx, async move |cx| { + let result = task.await; + let _ = cx.update(|window, cx| self.answered(kind, &result, window, cx)); + }) + .detach(); + } + + fn command( + &self, + action: PullRequestAction, + cx: &mut App, + ) -> gpui::Task { + (self.started)(cx); + let task = self.store.update(cx, |store, cx| { + store.command( + Command::RunPullRequestAction { + session_id: self.session.clone(), + key: self.offer.key.clone(), + action, + }, + cx, + ) + }); + cx.spawn(async move |_| match answer_of(task.await) { + Ok(result) => result, + Err(error) => PullRequestActionResult::Rejected(PullRequestRejection::Refused { + messages: vec![super::detail::reason(&error)], + }), + }) + } + + fn answered( + &self, + kind: Lifecycle, + result: &PullRequestActionResult, + window: &mut Window, + cx: &mut App, + ) { + if let Some(note) = self.toast(kind, result) { + let key = &self.offer.key; + window.push_notification( + note.id1::(SharedString::from(format!( + "{}/{}#{}", + key.host, key.repository, key.number + ))), + cx, + ); + } + (self.done)(result, window, cx); + } + + /// The words of an answer; a success goes by itself, anything else stays until it is read. + fn toast(&self, kind: Lifecycle, result: &PullRequestActionResult) -> Option { + let number = self.number(); + let (head, base) = (self.head_branch.clone(), self.base_branch.clone()); + let merging = matches!(kind, Lifecycle::Merge | Lifecycle::EnableAutoMerge); + let updating = matches!(kind, Lifecycle::UpdateBranch | Lifecycle::UpdateRebase); + let open_on_github = |note: Notification| { + let url = self.offer.url.clone(); + note.action(move |_, _, _| { + let url = url.clone(); + Button::new("pr-result-open") + .ghost() + .xsmall() + .label(crate::tr!("pull_requests.open_on_github")) + .on_click(move |_, _, cx| cx.open_url(&url)) + }) + }; + let note = match result { + PullRequestActionResult::Applied => match kind { + Lifecycle::Ready => Notification::success( + crate::tr!("pull_requests.result.ready", number = number).into_owned(), + ), + Lifecycle::Draft => Notification::success( + crate::tr!("pull_requests.result.draft", number = number).into_owned(), + ), + Lifecycle::Close => Notification::info( + crate::tr!("pull_requests.result.closed", number = number).into_owned(), + ), + Lifecycle::Reopen => Notification::success( + crate::tr!("pull_requests.result.reopened", number = number).into_owned(), + ), + Lifecycle::DisableAutoMerge => Notification::info( + crate::tr!("pull_requests.result.auto_merge_off", number = number).into_owned(), + ), + Lifecycle::UpdateBranch | Lifecycle::UpdateRebase => Notification::success( + crate::tr!("pull_requests.result.updated_body").into_owned(), + ) + .title(crate::tr!( + "pull_requests.result.updated", + head = head, + base = base + )), + _ => Notification::success( + crate::tr!("pull_requests.result.merged", number = number).into_owned(), + ), + }, + PullRequestActionResult::UpToDate => Notification::info( + crate::tr!("pull_requests.result.up_to_date", head = head, base = base) + .into_owned(), + ), + PullRequestActionResult::Queued { .. } => { + Notification::info(crate::tr!("pull_requests.result.queued_body").into_owned()) + .title(crate::tr!("pull_requests.result.queued", number = number)) + } + PullRequestActionResult::AutoMergeEnabled { method } => Notification::info( + crate::tr!( + "pull_requests.result.auto_merge_on_body", + method = method_label(*method) + ) + .into_owned(), + ) + .title(crate::tr!( + "pull_requests.result.auto_merge_on", + number = number + )), + PullRequestActionResult::Opened { number: new, url } => { + let url = url.clone(); + Notification::success( + crate::tr!( + "pull_requests.result.reverted", + new = new.to_string(), + number = number + ) + .into_owned(), + ) + .action(move |_, _, _| { + let url = url.clone(); + Button::new("pr-result-open") + .ghost() + .xsmall() + .label(crate::tr!("pull_requests.open_on_github")) + .on_click(move |_, _, cx| cx.open_url(&url)) + }) + } + PullRequestActionResult::Rejected(PullRequestRejection::StaleHead { head: actual }) + if merging => + { + Notification::error( + crate::tr!( + "pull_requests.result.stale_body", + expected = self + .offer + .action + .as_ref() + .map(|action| short(&action.head)) + .unwrap_or_default(), + actual = short(actual) + ) + .into_owned(), + ) + .title(crate::tr!( + "pull_requests.result.not_merged_stale", + number = number + )) + } + PullRequestActionResult::Rejected(rejection) => { + let reason = rejection_reason(rejection); + if merging { + return Some(Notification::error(reason).title(crate::tr!( + "pull_requests.result.merge_failed", + number = number + ))); + } + Notification::error(if updating { + crate::tr!( + "pull_requests.result.update_failed", + head = head, + reason = reason + ) + .into_owned() + } else { + crate::tr!( + "pull_requests.result.action_failed", + verb = crate::tr!(match kind { + Lifecycle::Ready => "pull_requests.actions.verb_ready", + Lifecycle::Draft => "pull_requests.actions.verb_draft", + Lifecycle::Close => "pull_requests.actions.verb_close", + Lifecycle::Reopen => "pull_requests.actions.verb_reopen", + Lifecycle::Revert => "pull_requests.actions.verb_revert", + _ => "pull_requests.actions.verb_disable_auto_merge", + }) + .into_owned(), + number = number, + reason = reason + ) + .into_owned() + }) + } + PullRequestActionResult::Uncertain => { + let message = crate::tr!( + "pull_requests.result.uncertain_body", + message = crate::tr!("pull_requests.result.connection_lost").into_owned() + ) + .into_owned(); + let title = if merging { + crate::tr!("pull_requests.result.uncertain_merge", number = number).into_owned() + } else { + crate::tr!( + "pull_requests.result.uncertain", + action = crate::tr!(match kind { + Lifecycle::Ready => "pull_requests.result.action_ready", + Lifecycle::Draft => "pull_requests.result.action_draft", + Lifecycle::Close => "pull_requests.result.action_close", + Lifecycle::Reopen => "pull_requests.result.action_reopen", + Lifecycle::Revert => "pull_requests.result.action_revert", + Lifecycle::UpdateBranch | Lifecycle::UpdateRebase => { + "pull_requests.result.action_update" + } + _ => "pull_requests.result.action_auto_merge", + }) + .into_owned(), + number = number + ) + .into_owned() + }; + open_on_github(Notification::warning(message).title(title)) + } + PullRequestActionResult::Partial { .. } => return None, + }; + let settled = matches!( + result, + PullRequestActionResult::Applied + | PullRequestActionResult::UpToDate + | PullRequestActionResult::Queued { .. } + | PullRequestActionResult::AutoMergeEnabled { .. } + | PullRequestActionResult::Opened { .. } + ); + Some(note.autohide(settled)) + } +} + +/// The merge confirmation: what GitHub holds now, the method, and what the merge would meet. +struct MergeDialog { + target: Target, + auto: bool, + state: Option>, + method: Option, + make_default: bool, + remove_credits: bool, + sending: bool, +} + +impl MergeDialog { + /// The merge reads the pull request fresh; the dialog shows what it read. + fn load(&mut self, cx: &mut Context) { + self.state = None; + let task = self.target.store.update(cx, |store, cx| { + store.read_pull_request( + self.target.session.clone(), + self.target.offer.key.clone(), + PullRequestRead::ActionState, + cx, + ) + }); + cx.spawn(async move |this, cx| { + let result = task.await; + let _ = this.update(cx, |this, cx| { + this.state = Some(match result { + Ok((PullRequestReadResponse::ActionState(state), _)) => { + this.method = merge_method( + &state.merge_methods, + this.target.offer.method, + this.target.project_default, + ); + Ok(state) + } + Ok(_) => Err(String::new()), + Err(error) => Err(super::detail::reason(&error)), + }); + cx.notify(); + }); + }) + .detach(); + } + + fn confirm(&mut self, window: &mut Window, cx: &mut Context) { + let (Some(Ok(state)), Some(method)) = (&self.state, self.method) else { + return; + }; + if self.sending { + return; + } + self.sending = true; + cx.notify(); + if self.make_default + && let Some((project, _)) = self.target.project.clone() + { + self.target.store.update(cx, |store, _| { + store.set_project_merge_method(project, method) + }); + } + let action = PullRequestAction::Merge { + head: state.head.clone(), + method, + auto: self.auto, + remove_credits: self.credits_shown() && self.remove_credits, + }; + let mut target = self.target.clone(); + target.offer.action = Some(state.clone()); + let kind = if self.auto { + Lifecycle::EnableAutoMerge + } else { + Lifecycle::Merge + }; + let task = target.command(action, cx); + cx.spawn_in(window, async move |this, cx| { + let result = task.await; + let _ = this.update_in(cx, |this, window, cx| { + this.sending = false; + window.close_dialog(cx); + target.answered(kind, &result, window, cx); + }); + }) + .detach(); + } + + /// GitHub writes a merge queue's message itself, and a rebase keeps each commit's. + fn credits_shown(&self) -> bool { + matches!(&self.state, Some(Ok(state)) if !state.merge_queue) + && self.method != Some(PullRequestMergeMethod::Rebase) + } + + fn confirm_label(&self) -> String { + let state = self.state.as_ref().and_then(|state| state.as_ref().ok()); + if self.sending { + return crate::tr!(if self.auto { + "pull_requests.merge.enabling" + } else { + "pull_requests.merge.merging" + }) + .into_owned(); + } + if state.is_some_and(|state| state.merge_queue) { + return crate::tr!("pull_requests.merge.add_to_queue").into_owned(); + } + if self.auto { + return crate::tr!("pull_requests.actions.enable_auto_merge").into_owned(); + } + self.method.map(method_label).unwrap_or_default() + } + + fn fact(&self, label: &str, value: impl IntoElement, cx: &App) -> AnyElement { + h_flex() + .gap_2() + .items_start() + .child( + div() + .w(px(104.)) + .flex_none() + .text_size(px(12.)) + .text_color(cx.theme().muted_foreground) + .child(crate::tr!(label).into_owned()), + ) + .child(div().flex_1().min_w_0().text_size(px(13.)).child(value)) + .into_any_element() + } +} + +impl Render for MergeDialog { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let target = &self.target; + let key = &target.offer.key; + let number = key.number.to_string(); + let mono = cx.theme().mono_font_family.clone(); + let muted = cx.theme().muted_foreground; + let state = self.state.as_ref().and_then(|state| state.as_ref().ok()); + let head = state.map(|state| state.head.clone()); + let facts = v_flex() + .gap_1p5() + .child(self.fact("pull_requests.merge.host", key.host.clone(), cx)) + .child( + self.fact( + "pull_requests.merge.repository", + div() + .font_family(mono.clone()) + .child(key.repository.clone()), + cx, + ), + ) + .child( + self.fact( + "pull_requests.merge.pull_request", + div() + .truncate() + .child(format!("#{number} {}", target.title)), + cx, + ), + ) + .child( + self.fact( + "pull_requests.merge.into", + div() + .truncate() + .font_family(mono.clone()) + .child(format!("{} ← {}", target.base_branch, target.head_branch)), + cx, + ), + ) + .child( + self.fact( + "pull_requests.merge.head", + match &head { + Some(head) => div() + .font_family(mono.clone()) + .child(short(head)) + .into_any_element(), + None => Spinner::new().small().into_any_element(), + }, + cx, + ), + ); + let view = cx.entity(); + let method = match (state, self.method) { + (Some(state), Some(chosen)) if state.merge_methods.len() >= 2 => { + let segments = state.merge_methods.iter().map(|method| { + let method = *method; + let view = view.clone(); + material::segment( + SharedString::from(format!("pr-merge-method-{method:?}")), + segment_label(method), + chosen == method, + cx, + ) + .on_change(move |_, _, _, cx| { + view.update(cx, |dialog, cx| { + dialog.method = Some(method); + cx.notify(); + }) + }) + }); + material::segmented_track("pr-merge-method", segments, cx).into_any_element() + } + (Some(_), Some(chosen)) => div() + .text_size(px(13.)) + .child(method_label(chosen)) + .into_any_element(), + (Some(_), None) => div().into_any_element(), + (None, _) => Spinner::new().small().into_any_element(), + }; + let default_box = self + .method + .filter(|method| state.is_some() && Some(*method) != target.project_default) + .zip(target.project.clone()) + .map(|(method, (_, project))| { + let view = view.clone(); + Checkbox::new("pr-merge-default") + .label( + crate::tr!( + "pull_requests.merge.make_default", + method = segment_label(method), + project = project + ) + .into_owned(), + ) + .checked(self.make_default) + .disabled(self.sending) + .on_click(move |checked, _, cx| { + let checked = *checked; + view.update(cx, |dialog, cx| { + dialog.make_default = checked; + cx.notify(); + }) + }) + }); + let credits = + self.credits_shown().then(|| { + let view = view.clone(); + v_flex() + .gap_0p5() + .child( + Checkbox::new("pr-merge-credits") + .label(crate::tr!("pull_requests.merge.remove_credits").into_owned()) + .checked(self.remove_credits) + .disabled(self.sending) + .on_click(move |checked, _, cx| { + let checked = *checked; + view.update(cx, |dialog, cx| { + dialog.remove_credits = checked; + cx.notify(); + }) + }), + ) + .child(div().text_size(px(11.)).text_color(muted).child( + crate::tr!("pull_requests.merge.remove_credits_caption").into_owned(), + )) + }); + let note = |icon: IconName, color, text: String| { + h_flex() + .gap_1p5() + .items_start() + .text_size(px(12.)) + .child( + div() + .pt(px(2.)) + .child(Icon::new(icon).size(px(12.)).text_color(color)), + ) + .child(div().flex_1().min_w_0().child(text)) + }; + let mut notes = Vec::new(); + if let Some(state) = state { + if state.merge_queue { + notes.push(note( + IconName::ListOrdered, + cx.theme().info, + crate::tr!("pull_requests.merge.queue_note", number = number.clone()) + .into_owned(), + )); + } else if self.auto + && let Some(method) = self.method + { + notes.push(note( + IconName::GitMerge, + cx.theme().info, + crate::tr!( + "pull_requests.merge.auto_note", + number = number.clone(), + method = method_label(method) + ) + .into_owned(), + )); + } + let failing = state.failing_checks.len(); + if failing > 0 { + notes.push(note( + IconName::CircleX, + cx.theme().danger, + if failing == 1 { + crate::tr!("pull_requests.merge.checks_failing_one").into_owned() + } else { + crate::tr!( + "pull_requests.merge.checks_failing", + failed = failing.to_string() + ) + .into_owned() + }, + )); + } + if state.pending_checks > 0 { + notes.push(note( + IconName::CircleDashed, + cx.theme().warning, + if state.pending_checks == 1 { + crate::tr!("pull_requests.merge.checks_running_one").into_owned() + } else { + crate::tr!( + "pull_requests.merge.checks_running", + running = state.pending_checks.to_string() + ) + .into_owned() + }, + )); + } + if state.merge_state == PullRequestMergeState::Blocked { + notes.push(note( + IconName::MessageSquareWarning, + cx.theme().danger, + crate::tr!("pull_requests.actions.blocked").into_owned(), + )); + } + if let Some(behind) = state.behind_by.filter(|behind| *behind > 0) { + notes.push(note( + IconName::ArrowUpDown, + muted, + crate::tr!( + "pull_requests.merge.behind", + count = behind.to_string(), + base = target.base_branch.clone() + ) + .into_owned(), + )); + } + } + let failure = match &self.state { + Some(Err(reason)) => Some( + v_flex() + .gap_1() + .text_size(px(12.)) + .text_color(cx.theme().danger) + .child( + crate::tr!("pull_requests.merge.load_failed", number = number).into_owned(), + ) + .when(!reason.is_empty(), |failure| failure.child(reason.clone())) + .child( + Button::new("pr-merge-retry") + .outline() + .xsmall() + .label(crate::tr!("pull_requests.detail.retry")) + .on_click(cx.listener(|this, _, _, cx| this.load(cx))), + ), + ), + _ => None, + }; + let ready = state.is_some() && self.method.is_some(); + v_flex() + .gap_3() + .child(facts) + .child( + v_flex() + .gap_2() + .child( + div() + .text_size(px(12.)) + .text_color(muted) + .child(crate::tr!("pull_requests.merge.method").into_owned()), + ) + .child(method) + .children(default_box), + ) + .children(credits) + .when(!notes.is_empty(), |content| { + content.child(v_flex().gap_1().children(notes)) + }) + .children(failure) + .child( + h_flex() + .gap_2() + .justify_end() + .child( + Button::new("pr-merge-cancel") + .outline() + .small() + .disabled(self.sending) + .label(crate::tr!("pull_requests.actions.cancel")) + .on_click(|_, window, cx| window.close_dialog(cx)), + ) + .child( + Button::new("pr-merge-confirm") + .primary() + .small() + .loading(self.sending) + .disabled(!ready || self.sending) + .label(self.confirm_label()) + .on_click(cx.listener(|this, _, window, cx| this.confirm(window, cx))), + ), + ) + } +} + +fn open_merge_dialog(target: Target, auto: bool, window: &mut Window, cx: &mut App) { + let remove_credits = target + .store + .read(cx) + .settings() + .remove_agent_credits_on_merge; + let number = target.number(); + let compact = target.window_state.read(cx).compact; + let dialog = cx.new(|cx| { + let mut dialog = MergeDialog { + target, + auto, + state: None, + method: None, + make_default: false, + remove_credits, + sending: false, + }; + dialog.load(cx); + dialog + }); + let title = crate::tr!( + if auto { + "pull_requests.merge.title_auto" + } else { + "pull_requests.merge.title" + }, + number = number + ) + .into_owned(); + window.open_dialog(cx, move |base, window, cx| { + let sending = dialog.read(cx).sending; + let width = if compact { + (window.viewport_size().width - px(2. * material::COMPACT_PAGE_INSET)).min(px(480.)) + } else { + px(480.) + }; + base.title(title.clone()) + .w(width) + .keyboard(!sending) + .close_button(!sending) + .overlay_closable(!sending) + .footer(crate::overlay::DialogActions::new()) + .content({ + let dialog = dialog.clone(); + move |content, _, _| content.child(dialog.clone()) + }) + }); +} + +/// The header's primary action or status, `None` where the rank offers none. +pub(super) fn primary_element( + target: &Target, + primary: Primary, + compact: bool, + cx: &App, +) -> AnyElement { + let busy = target.offer.busy; + let number = target.number(); + let key = target.offer.key.clone(); + let url = target.offer.url.clone(); + let chip = |icon: IconName, color, label: String, tooltip: Option| { + let id = SharedString::from(format!("pr-primary-chip-{}", key.number)); + let chip = h_flex() + .id(id) + .flex_none() + .gap_1() + .items_center() + .px_2() + .py(px(2.)) + .rounded(material::radius_chip(cx)) + .bg(gpui::Hsla::opacity(&color, 0.1)) + .text_color(color) + .text_size(px(if compact { 13. } else { 12. })) + .child(Icon::new(icon).size(px(14.))) + .child(label); + match tooltip { + Some(tooltip) => chip + .tooltip(move |window, cx| { + crate::widgets::tooltip::Tooltip::new(tooltip.clone()).build(window, cx) + }) + .into_any_element(), + None => chip.into_any_element(), + } + }; + let run = |kind: Lifecycle| { + let target = target.clone(); + move |_: &gpui::ClickEvent, window: &mut Window, cx: &mut App| { + target.clone().run(kind, window, cx) + } + }; + let button = |icon: IconName, label: String| { + let accessible = crate::tr!( + "pull_requests.actions.primary_label", + label = label.clone(), + number = number.clone() + ) + .into_owned(); + let button = Button::new("pr-primary-action") + .primary() + .icon(icon) + .label(label) + .tooltip(accessible) + .loading(busy) + .disabled(busy); + if compact { + button.w_full() + } else { + button.small() + } + }; + use gpui::{InteractiveElement as _, StatefulInteractiveElement as _}; + match primary { + Primary::ResolveConflicts | Primary::FixChecks => { + let (icon, label, ask, github) = if primary == Primary::ResolveConflicts { + ( + IconName::GitMergeConflict, + crate::tr!("pull_requests.actions.resolve_conflicts").into_owned(), + Lifecycle::AskConflicts, + ( + "pull_requests.actions.resolve_on_github", + format!("{url}/conflicts"), + ), + ) + } else { + ( + IconName::CircleX, + crate::tr!("pull_requests.actions.fix_checks").into_owned(), + Lifecycle::AskChecks, + ( + "pull_requests.actions.checks_on_github", + format!("{url}/checks"), + ), + ) + }; + let can_ask = target.takes_messages(cx); + use crate::widgets::menu::DropdownMenu as _; + button(icon, label) + .dropdown_menu(move |menu, _, _| { + menu.menu_with_enable( + crate::tr!(if can_ask { + "pull_requests.actions.ask_agent" + } else { + "pull_requests.actions.thread_unavailable" + }) + .into_owned(), + Box::new(RunLifecycle { + key: key.clone(), + kind: ask, + }), + can_ask, + ) + .menu( + crate::tr!(github.0).into_owned(), + Box::new(OpenUrl(github.1.clone())), + ) + }) + .into_any_element() + } + Primary::Ready => button( + IconName::GitPullRequest, + crate::tr!("pull_requests.actions.ready").into_owned(), + ) + .on_click(run(Lifecycle::Ready)) + .into_any_element(), + Primary::AutoMergeOn(method) => chip( + IconName::GitMerge, + cx.theme().info, + crate::tr!( + "pull_requests.actions.auto_merge_on", + method = method_label(method) + ) + .into_owned(), + Some(crate::tr!("pull_requests.actions.auto_merge_tooltip").into_owned()), + ), + Primary::Queued(position) => chip( + IconName::ListOrdered, + cx.theme().info, + match position { + Some(position) => crate::tr!( + "pull_requests.actions.queued_position", + position = position.to_string() + ) + .into_owned(), + None => crate::tr!("pull_requests.actions.queued").into_owned(), + }, + None, + ), + Primary::MergeOnGitHub { index } => { + let button = Button::new("pr-primary-action") + .outline() + .icon(IconName::ExternalLink) + .label(crate::tr!("pull_requests.actions.merge_on_github")) + .tooltip(stack_reason(&number, index)) + .on_click(move |_, _, cx| cx.open_url(&url)); + if compact { + button.w_full().into_any_element() + } else { + button.small().into_any_element() + } + } + Primary::UpdateBranch => button( + IconName::ArrowUpDown, + crate::tr!("pull_requests.actions.update_branch").into_owned(), + ) + .tooltip( + crate::tr!( + "pull_requests.actions.update_tooltip", + base = target.base_branch.clone(), + head = target.head_branch.clone() + ) + .into_owned(), + ) + .on_click(run(Lifecycle::UpdateBranch)) + .into_any_element(), + Primary::EnableAutoMerge => button( + IconName::GitMerge, + crate::tr!("pull_requests.actions.enable_auto_merge").into_owned(), + ) + .on_click(run(Lifecycle::EnableAutoMerge)) + .into_any_element(), + Primary::Merge { method, queue } => button( + IconName::GitMerge, + if queue { + crate::tr!("pull_requests.merge.merge_when_ready").into_owned() + } else { + method_label(method) + }, + ) + .on_click(run(Lifecycle::Merge)) + .into_any_element(), + } +} diff --git a/crates/ui/src/pull_requests/mod.rs b/crates/ui/src/pull_requests/mod.rs index 3bc36b144..72e52e4d4 100644 --- a/crates/ui/src/pull_requests/mod.rs +++ b/crates/ui/src/pull_requests/mod.rs @@ -34,6 +34,7 @@ mod compose; mod conversation; mod detail; mod files; +mod lifecycle; mod meta; mod review; @@ -495,7 +496,8 @@ fn change_link( .detach(); } -/// The row's menu: Open on GitHub, Copy link, the watch, and the link condition. +/// The row's menu: Open on GitHub, Copy link, the watch, what can become of the pull request, +/// and the link condition. type RowMenu = std::rc::Rc) -> PopupMenu>; fn row_menu( @@ -503,6 +505,7 @@ fn row_menu( url: String, link: Option<&ThreadPullRequestLink>, watchable: bool, + offer: impl Fn(&App) -> Option + 'static, ) -> RowMenu { let visible = link.is_some_and(|link| link.visible()); let watched = link.is_some_and(|link| link.visible() && link.watch.is_some()); @@ -520,7 +523,7 @@ fn row_menu( }; let source_is_stack = link.is_some_and(|link| link.source == PullRequestSource::Stack); std::rc::Rc::new( - move |menu: PopupMenu, _: &mut Window, _: &mut Context| { + move |menu: PopupMenu, _: &mut Window, cx: &mut Context| { menu.menu( crate::tr!("pull_requests.open_on_github").into_owned(), Box::new(OpenUrl(url.clone())), @@ -543,6 +546,7 @@ fn row_menu( }), ) }) + .when_some(offer(cx), |menu, offer| offer.menu(menu)) .separator() .menu( crate::tr!(if action.linking { @@ -657,6 +661,39 @@ impl PullRequestsPanel { fn change_link(&mut self, action: &ChangeLink, window: &mut Window, cx: &mut Context) { change_link(&self.store, action, window, cx); } + /// A list row offers what its link shows, as its menu opens; the host reads the rest + /// before it writes. + fn offer(&self, key: &PullRequestKey) -> impl Fn(&App) -> Option + 'static { + let (store, detail, key) = (self.store.clone(), self.detail.clone(), key.clone()); + move |cx| { + let store = store.read(cx); + if store + .session_status() + .is_none_or(|status| status.conversation_read_only) + { + return None; + } + let session = store.active_session_id()?; + let busy = detail.read(cx).lifecycle_busy(&session, &key); + lifecycle::Offer::new(&key, store.pull_requests(&session), None, None, None, busy) + } + } + fn run_lifecycle( + &mut self, + action: &lifecycle::RunLifecycle, + window: &mut Window, + cx: &mut Context, + ) { + let Some(session) = self.store.read(cx).active_session_id() else { + return; + }; + let target = self.detail.update(cx, |detail, cx| { + detail.lifecycle_target(&session, &action.key, false, cx) + }); + if let Some(target) = target { + target.run(action.kind, window, cx); + } + } fn row(&self, row: PullRequestRow<'_>, compact: bool, cx: &mut Context) -> AnyElement { let PullRequestRow { key, @@ -775,7 +812,7 @@ impl PullRequestsPanel { .into_any_element() }; let detail = detail.join(" · "); - let menu = row_menu(key.clone(), url.clone(), link, watchable); + let menu = row_menu(key.clone(), url.clone(), link, watchable, self.offer(&key)); let mut signals = h_flex().gap_1().flex_none().items_center(); // The eye leads while the pull request is open or not yet read; the host ends the watch // once it merges or closes. @@ -1330,6 +1367,7 @@ impl Render for PullRequestsPanel { .size_full() .on_action(cx.listener(Self::change_link)) .on_action(cx.listener(Self::change_watch)) + .on_action(cx.listener(Self::run_lifecycle)) .child(rows.p_2()) .child(empty) .children(agent_tools) @@ -1377,6 +1415,7 @@ impl Render for PullRequestsPanel { .gap_3() .on_action(cx.listener(Self::change_link)) .on_action(cx.listener(Self::change_watch)) + .on_action(cx.listener(Self::run_lifecycle)) .child(rows) .child(summary) .children(agent_tools) @@ -1390,6 +1429,7 @@ impl Render for PullRequestsPanel { .min_w_0() .on_action(cx.listener(Self::change_link)) .on_action(cx.listener(Self::change_watch)) + .on_action(cx.listener(Self::run_lifecycle)) .child( div() .id("pr-scroll") diff --git a/crates/ui/src/settings_page.rs b/crates/ui/src/settings_page.rs index 0efcb2432..8638fb5d3 100644 --- a/crates/ui/src/settings_page.rs +++ b/crates/ui/src/settings_page.rs @@ -1319,7 +1319,7 @@ impl SettingsPage { Section::General => self.render_general(cx), Section::ThreadBehavior => self.render_thread_behavior(cx), Section::ProjectThreadRules => v_flex().children(self.project_rules_editor.clone()), - Section::SourceControl => v_flex().child(self.github_panel.clone()), + Section::SourceControl => self.render_source_control(cx), Section::Providers => self.render_providers(window, cx), Section::Usage => self.render_usage(cx), Section::Browser => self.render_browser(cx), @@ -2739,6 +2739,31 @@ impl SettingsPage { .items_center() } + fn render_source_control(&mut self, cx: &mut Context) -> gpui::Div { + let remove_credits = self.store.read(cx).settings().remove_agent_credits_on_merge; + let reset = self.reset_action( + "reset-remove-agent-credits", + remove_credits, + cx, + |this, _, cx| { + this.dispatch_settings(|store| store.set_remove_agent_credits_on_merge(false), cx) + }, + ); + let row = self.toggle_row( + "remove-agent-credits", + crate::tr!("source_control.remove_agent_credits"), + crate::tr!("source_control.remove_agent_credits_description"), + remove_credits, + reset, + cx, + WorkspaceStore::set_remove_agent_credits_on_merge, + ); + v_flex() + .gap(px(24.)) + .child(self.github_panel.clone()) + .child(self.grouped_plain(vec![row], cx)) + } + #[allow(clippy::too_many_arguments)] fn toggle_row( &mut self, diff --git a/crates/ui/src/store/intents.rs b/crates/ui/src/store/intents.rs index a2054b6f1..faf51b2c7 100644 --- a/crates/ui/src/store/intents.rs +++ b/crates/ui/src/store/intents.rs @@ -334,6 +334,16 @@ impl WorkspaceStore { pub fn set_auto_settle_after_days(&mut self, days: Option) { self.patch_settings(SettingsPatch::AutoSettleAfterDays(days)); } + pub fn set_project_merge_method( + &mut self, + project_id: String, + method: tcode_core::pull_request::PullRequestMergeMethod, + ) { + self.patch_settings(SettingsPatch::ProjectMergeMethod { project_id, method }); + } + pub fn set_remove_agent_credits_on_merge(&mut self, enabled: bool) { + self.patch_settings(SettingsPatch::RemoveAgentCreditsOnMerge(enabled)); + } pub fn set_auto_settle_on_merge(&mut self, enabled: bool) { self.patch_settings(SettingsPatch::AutoSettleOnMerge(enabled)); } diff --git a/crates/ui/src/store/mod.rs b/crates/ui/src/store/mod.rs index 080e988ff..b82257685 100644 --- a/crates/ui/src/store/mod.rs +++ b/crates/ui/src/store/mod.rs @@ -225,6 +225,8 @@ pub struct WorkspaceStore { /// One-shot turn navigation requested by a cross-session content search. pending_chat_turn: Option<(String, usize)>, native_rewind_prefills: HashMap, + /// Text a pull request action put into a thread's composer for the user to edit and send. + composer_appends: HashMap, fallback_blocks: HashMap, fallback_reviews: HashMap, conversation_ui: HashMap, @@ -423,6 +425,7 @@ impl WorkspaceStore { active_destination: None, pending_chat_turn: None, native_rewind_prefills: HashMap::new(), + composer_appends: HashMap::new(), fallback_blocks: HashMap::new(), fallback_reviews: HashMap::new(), conversation_ui: HashMap::new(), @@ -3263,6 +3266,25 @@ impl WorkspaceStore { ) } + /// Puts `text` into the thread's composer after whatever is typed there; nothing is sent. + pub fn append_to_composer(&mut self, session_id: String, text: String, cx: &mut Context) { + let pending = self.composer_appends.entry(session_id).or_default(); + if !pending.is_empty() { + pending.push_str("\n\n"); + } + pending.push_str(&text); + cx.emit(StoreChange { + topic: TopicKind::SessionStatus, + }); + cx.notify(); + } + + /// Consume the active session's text from [`Self::append_to_composer`]. + pub fn take_composer_append(&mut self) -> Option { + let active_id = self.session_status_replica.as_ref()?.session_id.clone(); + self.composer_appends.remove(&active_id) + } + /// Consume the active session's prefill delivered by `NativeRewindPrefill`. pub fn take_native_rewind_prefill(&mut self) -> Option { let active_id = self.session_status_replica.as_ref()?.session_id.clone(); diff --git a/locales/en.yml b/locales/en.yml index b962c0000..0f97b8b32 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -1501,6 +1501,8 @@ source_control: source_none: "No credential in use" active_account: "Active gh account" env_override: "An environment token overrides the selected gh account." + remove_agent_credits: "Remove agent credits when merging" + remove_agent_credits_description: "Removes recognised agent Co-authored-by and generated-by lines from merge and squash commit messages. People stay credited. Merge queues and stacks use GitHub's own message." pull_requests: title: "Pull requests" @@ -1841,6 +1843,116 @@ pull_requests: reason_failed: "Tcode could not read GitHub first, so nothing was sent" connection_lost: "The connection to the host was lost before it answered." draft_failed: "Could not update your pending review: %{reason}" + action_auto_merge: "the auto-merge change" + action_close: "closing" + action_draft: "converting to draft" + action_failed: "Could not %{verb} #%{number}: %{reason}" + action_ready: "marking ready" + action_reopen: "reopening" + action_revert: "the revert" + action_update: "the branch update" + auto_merge_off: "Auto-merge off for #%{number}" + auto_merge_on: "Auto-merge on for #%{number}" + auto_merge_on_body: "GitHub merges it with %{method} once its requirements pass." + closed: "Closed #%{number}" + draft: "#%{number} converted to draft" + merge_failed: "Could not merge #%{number}" + merged: "Merged #%{number}" + not_merged_stale: "Not merged: #%{number} changed" + queued: "#%{number} joined the merge queue" + queued_body: "It merges when the queue reaches it." + ready: "#%{number} is ready for review" + reason_stack: "it is layer %{index} of %{count} of a GitHub stack, which merges with the layers below it; Tcode does not do that yet" + reason_stack_unknown: "Tcode is still checking whether it is in a stack" + reopened: "Reopened #%{number}" + reverted: "Opened #%{new} to revert #%{number}" + stale_body: "Its head moved from %{expected} to %{actual}. Nothing was sent. Review the new changes first." + uncertain_merge: "Tcode couldn't confirm whether #%{number} merged" + up_to_date: "%{head} is already up to date with %{base}." + update_failed: "Could not update %{head}: %{reason}" + updated: "Updated %{head} with %{base}" + updated_body: "Pull before pushing from this thread." + actions: + and_more: "and %{count} more" + ask_agent: "Ask the agent in this thread" + ask_checks: "Checks are failing on #%{number}: %{names}. Fix them and push." + ask_checks_unnamed: "Checks are failing on #%{number}. Fix them and push." + ask_conflicts: "#%{number} (%{head}) conflicts with %{base}. Resolve the conflicts on the branch and push." + auto_merge_on: "Auto-merge on · %{method}" + auto_merge_tooltip: "GitHub merges this once its requirements pass." + behind: "%{count} commits behind %{base}" + behind_one: "1 commit behind %{base}" + blocked: "Required reviews or checks are not satisfied." + cancel: "Cancel" + checks_on_github: "Open checks on GitHub" + close_confirm: "Close pull request" + close_desc: "This closes #%{number} without merging it. You can reopen it later." + close_menu: "Close pull request…" + close_title: "Close #%{number}?" + disable_auto_merge: "Disable auto-merge" + draft: "Convert to draft" + enable_auto_merge: "Enable auto-merge" + enable_auto_merge_menu: "Enable auto-merge…" + fix_checks: "Fix failing checks" + merge_method: "Merge method" + merge_now: "Merge now…" + merge_on_github: "Merge on GitHub" + primary_label: "%{label} #%{number}" + queued: "Queued to merge" + queued_position: "Queued to merge · position %{position}" + ready: "Ready for review" + rebase_desc: "GitHub rewrites this branch's commits. Local copies of %{head} must be reset to the new commits before pushing." + rebase_title: "Rebase %{head} onto %{base}?" + reopen: "Reopen pull request" + resolve_conflicts: "Resolve conflicts" + resolve_on_github: "Resolve on GitHub" + revert_confirm: "Create revert pull request" + revert_desc: "This opens a new pull request that reverses the changes merged by #%{number}." + revert_menu: "Revert…" + revert_title: "Revert #%{number}?" + stack_checking: "Checking whether #%{number} is in a stack…" + stack_reason: "#%{number} is layer %{index} of a GitHub stack. Merging it also lands the layers below it, which Tcode does not do yet. Merge on GitHub." + thread_unavailable: "This thread can't take messages" + update_branch: "Update branch" + update_rebase: "Update with rebase" + update_rebase_menu: "Update with rebase…" + update_tooltip: "Merges the latest %{base} into %{head} on GitHub. Pull before pushing again from this thread." + verb_close: "close" + verb_disable_auto_merge: "turn off auto-merge for" + verb_draft: "convert to draft" + verb_ready: "mark ready for review" + verb_reopen: "reopen" + verb_revert: "revert" + merge: + add_to_queue: "Add to merge queue" + auto_note: "GitHub merges #%{number} with %{method} once its requirements pass, which may be immediately." + behind: "%{count} commits behind %{base}." + checks_failing: "%{failed} checks are failing." + checks_failing_one: "1 check is failing." + checks_running: "%{running} checks are still running." + checks_running_one: "1 check is still running." + enabling: "Enabling…" + head: "Head commit" + host: "Host" + into: "Into" + load_failed: "Couldn't read #%{number}'s current state." + make_default: "Use %{method} for %{project} by default" + merge_when_ready: "Merge when ready" + merging: "Merging…" + method: "Method" + method_merge: "Create a merge commit" + method_rebase: "Rebase and merge" + method_squash: "Squash and merge" + pull_request: "Pull request" + queue_note: "This repository uses a merge queue. #%{number} joins the queue and merges when the queue reaches it." + remove_credits: "Remove agent credits from the merge message" + remove_credits_caption: "Removes recognised agent Co-authored-by and generated-by lines. People stay credited." + repository: "Repository" + segment_merge: "Merge commit" + segment_rebase: "Rebase" + segment_squash: "Squash" + title: "Merge #%{number}?" + title_auto: "Enable auto-merge for #%{number}?" agents: title: "Agents" diff --git a/locales/zh-CN.yml b/locales/zh-CN.yml index 7249a4a8a..cec9c0b5c 100644 --- a/locales/zh-CN.yml +++ b/locales/zh-CN.yml @@ -1494,6 +1494,8 @@ source_control: source_none: "未使用凭据" active_account: "当前 gh 账户" env_override: "环境变量令牌会覆盖所选的 gh 账户。" + remove_agent_credits: "合并时移除智能体署名" + remove_agent_credits_description: "从合并提交和压缩合并的提交信息中移除可识别的智能体 Co-authored-by 和 generated-by 行，人员署名保留。合并队列和堆叠使用 GitHub 自己的提交信息。" pull_requests: title: "拉取请求" @@ -1834,6 +1836,116 @@ pull_requests: reason_failed: "Tcode 无法先读取 GitHub，因此未发送任何内容" connection_lost: "主机回应前连接已断开。" draft_failed: "无法更新待提交的审查：%{reason}" + action_auto_merge: "自动合并设置" + action_close: "关闭" + action_draft: "转为草稿" + action_failed: "无法%{verb} #%{number}：%{reason}" + action_ready: "标记为准备好审查" + action_reopen: "重新打开" + action_revert: "还原" + action_update: "分支更新" + auto_merge_off: "已为 #%{number} 停用自动合并" + auto_merge_on: "已为 #%{number} 开启自动合并" + auto_merge_on_body: "满足合并要求后，GitHub 会以“%{method}”合并。" + closed: "已关闭 #%{number}" + draft: "#%{number} 已转为草稿" + merge_failed: "无法合并 #%{number}" + merged: "已合并 #%{number}" + not_merged_stale: "未合并：#%{number} 已发生变化" + queued: "#%{number} 已加入合并队列" + queued_body: "轮到时会自动合并。" + ready: "#%{number} 已准备好审查" + reason_stack: "它是 GitHub 堆叠 %{count} 层中的第 %{index} 层，会与下面的各层一起合并；Tcode 暂不支持" + reason_stack_unknown: "Tcode 仍在检查它是否属于堆叠" + reopened: "已重新打开 #%{number}" + reverted: "已创建 #%{new} 以还原 #%{number}" + stale_body: "其头部提交已从 %{expected} 变为 %{actual}。未发送任何内容。请先查看新的更改。" + uncertain_merge: "Tcode 无法确认 #%{number} 是否已合并" + up_to_date: "%{head} 已与 %{base} 保持同步。" + update_failed: "无法更新 %{head}：%{reason}" + updated: "已用 %{base} 更新 %{head}" + updated_body: "在此对话推送前请先拉取。" + actions: + and_more: "等另外 %{count} 项" + ask_agent: "在此对话中让智能体处理" + ask_checks: "#%{number} 的检查未通过：%{names}。请修复并推送。" + ask_checks_unnamed: "#%{number} 的检查未通过。请修复并推送。" + ask_conflicts: "#%{number}（%{head}）与 %{base} 存在冲突。请在该分支上解决冲突并推送。" + auto_merge_on: "自动合并已开启 · %{method}" + auto_merge_tooltip: "满足合并要求后，GitHub 会自动合并。" + behind: "落后 %{base} %{count} 个提交" + behind_one: "落后 %{base} 1 个提交" + blocked: "必需的审查或检查尚未满足。" + cancel: "取消" + checks_on_github: "在 GitHub 上查看检查" + close_confirm: "关闭拉取请求" + close_desc: "这会关闭 #%{number} 而不合并。你之后可以重新打开它。" + close_menu: "关闭拉取请求…" + close_title: "关闭 #%{number}？" + disable_auto_merge: "停用自动合并" + draft: "转为草稿" + enable_auto_merge: "启用自动合并" + enable_auto_merge_menu: "启用自动合并…" + fix_checks: "修复未通过的检查" + merge_method: "合并方式" + merge_now: "立即合并…" + merge_on_github: "在 GitHub 上合并" + primary_label: "%{label} #%{number}" + queued: "已排队等待合并" + queued_position: "已排队等待合并 · 第 %{position} 位" + ready: "准备好审查" + rebase_desc: "GitHub 会重写此分支的提交。推送前，%{head} 的本地副本必须重置到新的提交。" + rebase_title: "将 %{head} 变基到 %{base}？" + reopen: "重新打开拉取请求" + resolve_conflicts: "解决冲突" + resolve_on_github: "在 GitHub 上解决" + revert_confirm: "创建还原拉取请求" + revert_desc: "这会创建一个新的拉取请求，撤回 #%{number} 合并的更改。" + revert_menu: "还原…" + revert_title: "还原 #%{number}？" + stack_checking: "正在检查 #%{number} 是否属于堆叠…" + stack_reason: "#%{number} 是 GitHub 堆叠的第 %{index} 层。合并它会同时合入它下面的各层，Tcode 暂不支持。请在 GitHub 上合并。" + thread_unavailable: "此对话无法接收消息" + update_branch: "更新分支" + update_rebase: "以变基方式更新" + update_rebase_menu: "以变基方式更新…" + update_tooltip: "在 GitHub 上将最新的 %{base} 合并到 %{head}。在此对话再次推送前请先拉取。" + verb_close: "关闭" + verb_disable_auto_merge: "停用自动合并" + verb_draft: "转为草稿" + verb_ready: "标记为准备好审查" + verb_reopen: "重新打开" + verb_revert: "还原" + merge: + add_to_queue: "加入合并队列" + auto_note: "满足合并要求后，GitHub 会以“%{method}”合并 #%{number}，可能立即发生。" + behind: "落后 %{base} %{count} 个提交。" + checks_failing: "%{failed} 项检查未通过。" + checks_failing_one: "1 项检查未通过。" + checks_running: "%{running} 项检查仍在进行。" + checks_running_one: "1 项检查仍在进行。" + enabling: "正在启用…" + head: "头部提交" + host: "主机" + into: "合并到" + load_failed: "无法读取 #%{number} 的当前状态。" + make_default: "默认对 %{project} 使用%{method}" + merge_when_ready: "就绪后合并" + merging: "正在合并…" + method: "方式" + method_merge: "创建合并提交" + method_rebase: "变基并合并" + method_squash: "压缩并合并" + pull_request: "拉取请求" + queue_note: "此仓库使用合并队列。#%{number} 会加入队列，轮到时再合并。" + remove_credits: "从合并提交信息中移除智能体署名" + remove_credits_caption: "移除可识别的智能体 Co-authored-by 和 generated-by 行，人员署名保留。" + repository: "仓库" + segment_merge: "合并提交" + segment_rebase: "变基" + segment_squash: "压缩" + title: "合并 #%{number}？" + title_auto: "为 #%{number} 启用自动合并？" agents: title: "子代理"