Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions crates/core/src/pull_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = &ThreadPullRequestLink> {
links
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/pull_request_watch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
20 changes: 20 additions & 0 deletions crates/core/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,11 @@ pub enum SettingsPatch {
project_id: String,
value: Option<ProjectSettlementSettings>,
},
ProjectMergeMethod {
project_id: String,
method: crate::pull_request::PullRequestMergeMethod,
},
RemoveAgentCreditsOnMerge(bool),
OrchestrateDecisionModels(Vec<OrchestrateChildModel>),
OrchestrateChildModels(Vec<OrchestrateChildModel>),
OrchestrateChildApproval(ChildApprovalMode),
Expand Down Expand Up @@ -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<String, ProjectSettlementSettings>,
/// 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<String, crate::pull_request::PullRequestMergeMethod>,
/// 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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
Expand Down
25 changes: 16 additions & 9 deletions crates/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down
98 changes: 97 additions & 1 deletion crates/protocol/src/pull_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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<u64>,
pub merge_queue: bool,
/// The repository's enabled methods, in GitHub's order.
pub merge_methods: Vec<PullRequestMergeMethod>,
/// The repository allows auto-merge.
pub auto_merge_allowed: bool,
/// The method auto-merge is armed with.
pub auto_merge: Option<PullRequestMergeMethod>,
pub queued: bool,
/// Where in the merge queue, when GitHub says.
pub queue_position: Option<u32>,
/// Failing checks of the head by name; a list cut short names only the first page's.
pub failing_checks: Vec<String>,
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 {
Expand All @@ -337,6 +387,7 @@ pub enum PullRequestReadResponse {
Media(PullRequestMedia),
LabelCandidates(PullRequestLabelCandidates),
ReviewerCandidates(PullRequestReviewerCandidates),
ActionState(PullRequestActionState),
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
Expand Down Expand Up @@ -396,6 +447,28 @@ pub enum PullRequestAction {
add: Vec<PullRequestReviewer>,
remove: Vec<PullRequestReviewer>,
},
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)]
Expand All @@ -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<u32>,
},
/// 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)]
Expand All @@ -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,
}
3 changes: 2 additions & 1 deletion crates/runtime/src/app/command_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
54 changes: 53 additions & 1 deletion crates/runtime/src/app/pull_requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading