diff --git a/api/org_tangled/gitmergeCheck.go b/api/org_tangled/gitmergeCheck.go deleted file mode 100644 index 45a1e6e40..000000000 --- a/api/org_tangled/gitmergeCheck.go +++ /dev/null @@ -1,52 +0,0 @@ -// Code generated by cmd/lexgen (see Makefile's lexgen); DO NOT EDIT. - -package org_tangled - -// schema: org.tangled.git.mergeCheck - -import ( - "context" - - "github.com/bluesky-social/indigo/lex/util" -) - -const ( - GitMergeCheckNSID = "org.tangled.git.mergeCheck" -) - -// GitMergeCheck_Conflict is a "conflict" in the org.tangled.git.mergeCheck schema. -type GitMergeCheck_Conflict struct { - // filename: Path of the conflicted file - Filename string `json:"filename" cborgen:"filename"` - // reason: Why the file conflicts - Reason string `json:"reason" cborgen:"reason"` -} - -// GitMergeCheck_Input is the input argument to a org.tangled.git.mergeCheck call. -type GitMergeCheck_Input struct { - // branch: Target branch. Defaults to the ticket's targetBranch, falling back to the repo's default branch. - Branch *string `json:"branch,omitempty" cborgen:"branch,omitempty"` - // patch: Record key of the org.tangled.review.patch record to check - Patch string `json:"patch" cborgen:"patch"` - // repo: DID of the repository - Repo string `json:"repo" cborgen:"repo"` -} - -// GitMergeCheck_Output is the output of a org.tangled.git.mergeCheck call. -type GitMergeCheck_Output struct { - Conflicts []*GitMergeCheck_Conflict `json:"conflicts" cborgen:"conflicts"` - // isConflicted: Whether the merge has unresolved conflicts - IsConflicted bool `json:"isConflicted" cborgen:"isConflicted"` - // message: Additional detail, e.g. unrelated histories - Message *string `json:"message,omitempty" cborgen:"message,omitempty"` -} - -// GitMergeCheck calls the XRPC method "org.tangled.git.mergeCheck". -func GitMergeCheck(ctx context.Context, c util.LexClient, input *GitMergeCheck_Input) (*GitMergeCheck_Output, error) { - var out GitMergeCheck_Output - if err := c.LexDo(ctx, util.Procedure, "application/json", "org.tangled.git.mergeCheck", nil, input, &out); err != nil { - return nil, err - } - - return &out, nil -} diff --git a/api/org_tangled/reviewmergePatch.go b/api/org_tangled/reviewmergePatch.go deleted file mode 100644 index 21e8abd5d..000000000 --- a/api/org_tangled/reviewmergePatch.go +++ /dev/null @@ -1,41 +0,0 @@ -// Code generated by cmd/lexgen (see Makefile's lexgen); DO NOT EDIT. - -package org_tangled - -// schema: org.tangled.review.mergePatch - -import ( - "context" - - "github.com/bluesky-social/indigo/lex/util" -) - -const ( - ReviewMergePatchNSID = "org.tangled.review.mergePatch" -) - -// ReviewMergePatch_Input is the input argument to a org.tangled.review.mergePatch call. -type ReviewMergePatch_Input struct { - // patch: Record key of the org.tangled.review.patch record to merge - Patch string `json:"patch" cborgen:"patch"` - // repo: DID of the repository - Repo string `json:"repo" cborgen:"repo"` -} - -// ReviewMergePatch_Output is the output of a org.tangled.review.mergePatch call. -type ReviewMergePatch_Output struct { - // merged: Patch records merged by this call - Merged []string `json:"merged" cborgen:"merged"` - // tip: Commit Object ID the target branch ended on; the current tip when no move was needed - Tip string `json:"tip" cborgen:"tip"` -} - -// ReviewMergePatch calls the XRPC method "org.tangled.review.mergePatch". -func ReviewMergePatch(ctx context.Context, c util.LexClient, input *ReviewMergePatch_Input) (*ReviewMergePatch_Output, error) { - var out ReviewMergePatch_Output - if err := c.LexDo(ctx, util.Procedure, "application/json", "org.tangled.review.mergePatch", nil, input, &out); err != nil { - return nil, err - } - - return &out, nil -} diff --git a/knot2/crates/knot-config/src/lib.rs b/knot2/crates/knot-config/src/lib.rs index d5da00b97..beaaba66c 100644 --- a/knot2/crates/knot-config/src/lib.rs +++ b/knot2/crates/knot-config/src/lib.rs @@ -203,13 +203,6 @@ impl HomepageConfig { #[derive(Debug, Config)] pub struct GitConfig { - /// Committer identity stamped on merge commits the knot creates. - #[config(env = "KNOT_GIT_USER_NAME", default = "Tangled")] - pub user_name: String, - - #[config(env = "KNOT_GIT_USER_EMAIL", default = "noreply@tangled.sh")] - pub user_email: String, - #[config(env = "KNOT_GIT_OBJECT_FORMAT", default = "sha256")] pub object_format: String, } @@ -275,8 +268,8 @@ pub struct XrpcConfig { #[config(env = "KNOT_XRPC_LANGUAGES_BUDGET_MS", default = 1_000)] pub languages_budget_ms: u64, - /// Body limit for the merge and mergeCheck procedures, whose patch payloads - /// routinely exceed the general XRPC body limit. + /// Body limit for the seedRepo procedure, whose record batches routinely + /// exceed the general XRPC body limit. #[config(env = "KNOT_XRPC_MAX_PATCH_BYTES", default = 16_777_216)] pub max_patch_bytes: u64, @@ -732,14 +725,6 @@ impl KnotConfig { self.xrpc.fork_fetch_timeout_ms > 0, "xrpc.fork_fetch_timeout_ms must be greater than zero", ), - check( - !self.git.user_name.trim().is_empty(), - "git.user_name mustn't be empty", - ), - check( - !self.git.user_email.trim().is_empty(), - "git.user_email mustn't be empty", - ), check( self.object_format().is_some(), "git.object_format must be \"sha1\" or \"sha256\"", @@ -1306,8 +1291,6 @@ mod tests { write_refill_ms: 500, }, git: GitConfig { - user_name: "Tangled".to_string(), - user_email: "noreply@tangled.sh".to_string(), object_format: "sha1".to_string(), }, secrets: SecretsConfig { diff --git a/knot2/crates/knot-git/src/lib.rs b/knot2/crates/knot-git/src/lib.rs index 7319811f7..d65c8c3d2 100644 --- a/knot2/crates/knot-git/src/lib.rs +++ b/knot2/crates/knot-git/src/lib.rs @@ -10,7 +10,6 @@ mod patch; mod patch_apply; mod patch_parse; mod reads; -mod replay; mod repo; mod staging; @@ -42,7 +41,6 @@ pub use reads::{ AnnotatedTag, BlameHunk, BranchInfo, BranchTip, LastCommit, LogLimit, LogSkip, PathEntry, SizedEntry, Submodule, TagInfo, }; -pub use replay::{ReplayError, onto as replay_onto}; pub use repo::{ AdvertScope, HeadRef, Layout, PackHash, PackfileUri, PackfileUrl, RefClass, RefRecord, RefTxn, RefUpdate, ReflogUpdate, Repo, is_branch, knot_shard, repo_shard, diff --git a/knot2/crates/knot-git/src/replay.rs b/knot2/crates/knot-git/src/replay.rs deleted file mode 100644 index 3a8f35e56..000000000 --- a/knot2/crates/knot-git/src/replay.rs +++ /dev/null @@ -1,160 +0,0 @@ -use std::collections::{HashMap, VecDeque}; - -use knot_types::{Oid, RepoPath}; - -use crate::error::GitError; -use crate::objects::{CommitRange, FileChange, Identity}; -use crate::patch_apply::{NewCommit, StagedAction, StagedChange}; -use crate::repo::Repo; - -#[derive(Debug, thiserror::Error)] -pub enum ReplayError { - #[error("the commit history has a cycle the replay can't order")] - Cycle, - #[error("{path} changed on the target branch too")] - Conflict { path: RepoPath }, - #[error("{path} went missing from its own commit between two reads")] - Vanished { path: RepoPath }, - #[error(transparent)] - Git(#[from] GitError), -} - -pub fn onto( - repo: &Repo, - tip: Oid, - range: CommitRange, - committer: &Identity, -) -> Result { - let walked = repo.compare(range)?.commits; - let ordered = order(repo, &walked)?; - ordered - .into_iter() - .try_fold((tip, tip), |(parent, running), oid| { - let commit = repo.find_commit(oid)?; - let against = commit.parents.first().copied().unwrap_or(range.base); - let staged = restage(repo, running, against, oid)?; - if staged.is_empty() { - return Ok((parent, running)); - } - let tree = repo.find_commit(running)?.tree; - let next_tree = repo.write_staged_tree(tree, &staged)?; - let written = repo.write_commit(&NewCommit { - tree: next_tree, - parents: vec![parent], - author: commit.author.clone(), - committer: committer.clone(), - message: commit.message.clone(), - extra_headers: Vec::new(), - })?; - Ok((written, written)) - }) - .map(|(tip, _)| tip) -} - -fn order(repo: &Repo, walked: &[Oid]) -> Result, ReplayError> { - let inside: HashMap = walked - .iter() - .enumerate() - .map(|(at, oid)| (*oid, at)) - .collect(); - let parents: Vec> = walked - .iter() - .map(|oid| { - repo.find_commit(*oid) - .map(|commit| { - commit - .parents - .iter() - .filter_map(|parent| inside.get(parent).copied()) - .collect() - }) - .map_err(ReplayError::from) - }) - .collect::, ReplayError>>()?; - let mut waits: Vec = parents.iter().map(Vec::len).collect(); - let mut unlocks: Vec> = vec![Vec::new(); walked.len()]; - parents - .iter() - .enumerate() - .for_each(|(at, of)| of.iter().for_each(|parent| unlocks[*parent].push(at))); - let mut ready: VecDeque = waits - .iter() - .enumerate() - .filter_map(|(at, waits)| (*waits == 0).then_some(at)) - .collect(); - let mut ordered = Vec::with_capacity(walked.len()); - while let Some(at) = ready.pop_front() { - ordered.push(walked[at]); - unlocks[at].iter().for_each(|child| { - waits[*child] -= 1; - if waits[*child] == 0 { - ready.push_back(*child); - } - }); - } - if ordered.len() == walked.len() { - Ok(ordered) - } else { - Err(ReplayError::Cycle) - } -} - -fn restage( - repo: &Repo, - running: Oid, - against: Oid, - commit: Oid, -) -> Result, ReplayError> { - let changes = repo.compare(CommitRange { - base: against, - head: commit, - })?; - let intents = changes.changes.into_iter().flat_map(|change| match change { - FileChange::Added { path, oid } => vec![(path, None, Some(oid))], - FileChange::Deleted { path, oid } => vec![(path, Some(oid), None)], - FileChange::Modified { path, old, new } => vec![(path, Some(old), Some(new))], - FileChange::Renamed { from, to, old, new } => { - vec![(from, Some(old), None), (to, None, Some(new))] - } - }); - intents - .map(|(path, old, new)| { - let current = repo - .entry_at(running, &path) - .map_err(ReplayError::from)? - .map(|entry| entry.oid); - let staged = match current { - current if current == new => None, - current if current == old => Some(match new { - Some(oid) => { - let kind = repo - .entry_at(commit, &path)? - .ok_or_else(|| ReplayError::Vanished { path: path.clone() })? - .kind; - StagedChange { - action: StagedAction::PutBlob { oid, kind }, - path, - } - } - None => StagedChange { - path, - action: StagedAction::Remove, - }, - }), - _ => { - return Err(ReplayError::Conflict { path }); - } - }; - Ok(staged) - }) - .collect::>, ReplayError>>() - .map(|staged| { - // removals first, so a folder that became a file is emptied before the file lands on it - let (mut removals, puts): (Vec<_>, Vec<_>) = staged - .into_iter() - .flatten() - .partition(|change| matches!(change.action, StagedAction::Remove)); - removals.extend(puts); - removals - }) -} diff --git a/knot2/crates/knot-server/src/main.rs b/knot2/crates/knot-server/src/main.rs index 9628e2c38..f54f04c10 100644 --- a/knot2/crates/knot-server/src/main.rs +++ b/knot2/crates/knot-server/src/main.rs @@ -28,7 +28,7 @@ use knot_config::HomepageSource; use knot_index::Index; use knot_runtime::{Clock, HttpTransport, OsEntropy, ReqwestHttp, SystemClock}; use knot_secrets::{MasterKey, SealedStore}; -use knot_types::{ActorId, AuthorName, BranchName, CiLogsAddr, Email, ObjectCount}; +use knot_types::{ActorId, BranchName, CiLogsAddr, ObjectCount}; use knot_xrpc::XrpcState; use tower_http::services::ServeFile; @@ -490,10 +490,6 @@ async fn run(config: knot_config::Validated) -> anyhow::Result<()> { Duration::from_millis(config.xrpc.languages_budget_ms), )), }; - let committer = knot_xrpc::Committer { - name: AuthorName::new(config.git.user_name.clone()), - email: Email::new(config.git.user_email.clone()), - }; let reservations = Arc::new(knot_xrpc::Reservations::new( knot_xrpc::ReservationTtl::new(config.xrpc.reservation_ttl_secs as i64), knot_xrpc::PerActorQuota::new(config.xrpc.per_actor_reservations as usize), @@ -642,7 +638,6 @@ async fn run(config: knot_config::Validated) -> anyhow::Result<()> { cob_locks: Arc::new(knot_xrpc::CobLocks::default()), reservations, proxy_trust, - committer, byte_limits, budgets, git_http, @@ -699,18 +694,6 @@ async fn run(config: knot_config::Validated) -> anyhow::Result<()> { let state = Arc::clone(&xrpc_state); tokio::task::spawn_blocking(move || knot_xrpc::materialize::enable_surfaces(&state)); } - { - let state = Arc::clone(&xrpc_state); - tokio::spawn(async move { - let finished = knot_xrpc::seal_pending_merges(&state).await; - if finished > 0 { - tracing::info!( - finished, - "Boot finished the pull merges that the last run left half-done" - ); - } - }); - } let seq_state = Arc::clone(&xrpc_state); let seq_meta_path = seq_state.meta_path.clone(); let resolver: Arc = { diff --git a/knot2/crates/knot-sim/src/harness.rs b/knot2/crates/knot-sim/src/harness.rs index 6be6be388..8982755fc 100644 --- a/knot2/crates/knot-sim/src/harness.rs +++ b/knot2/crates/knot-sim/src/harness.rs @@ -28,7 +28,7 @@ use knot_types::{ KnotServiceUrl, Oid, OwnerDid, RecordRkey, RefName, RepoDid, RepoName, RepoRkey, UnixSeconds, }; use knot_xrpc::{ - BlobReadBudget, BodyLimit, Budgets, ByteLimits, CobLocks, Committer, GlobalInflight, + BlobReadBudget, BodyLimit, Budgets, ByteLimits, CobLocks, GlobalInflight, GlobalQuota, LanguagesReadBudget, LimitConfig, MaxWireBytes, PerActorQuota, PerPeerInflight, PreAuthLimiter, ReadBudget, ReservationTtl, Reservations, ResponseLimit, TreeReadBudget, XrpcState, @@ -104,7 +104,6 @@ impl Faults { pub(crate) enum SocialStage { Ticket, Patch, - Merge, } #[derive(Clone)] @@ -113,7 +112,6 @@ pub(crate) struct SocialProgress { pub(crate) base: Oid, pub(crate) head: Oid, pub(crate) ticket: Option, - pub(crate) pull: Option, } impl SocialProgress { @@ -123,7 +121,6 @@ impl SocialProgress { base, head, ticket: None, - pull: None, } } @@ -135,13 +132,7 @@ impl SocialProgress { false } SocialStage::Patch => { - self.pull = rkey; - self.stage = SocialStage::Merge; - false - } - SocialStage::Merge => { self.ticket = None; - self.pull = None; self.stage = SocialStage::Ticket; true } @@ -835,10 +826,6 @@ fn assemble_router(parts: StateParts) -> (Router, FakeState) { GlobalQuota::new(256), )), proxy_trust: knot_types::ProxyTrust::default(), - committer: Committer { - name: AuthorName::new("knot"), - email: Email::new("knot@nel.pet"), - }, byte_limits: ByteLimits { body: BodyLimit::new(256 * 1024), response: ResponseLimit::new(8 * 1024 * 1024), diff --git a/knot2/crates/knot-sim/src/realdata.rs b/knot2/crates/knot-sim/src/realdata.rs index 1b2aad727..19a481915 100644 --- a/knot2/crates/knot-sim/src/realdata.rs +++ b/knot2/crates/knot-sim/src/realdata.rs @@ -28,7 +28,6 @@ const DRIVE_TAG: u64 = 0x11c7_0005; const TICKET_COLLECTION: &str = "org.tangled.track.ticket"; const CREATE_RECORD_NSID: &str = "com.atproto.repo.createRecord"; const DELETE_RECORD_NSID: &str = "com.atproto.repo.deleteRecord"; -const PULL_MERGE_NSID: &str = "org.tangled.review.mergePatch"; const SUBMIT_PATCH_NSID: &str = "org.tangled.review.submitPatch"; fn nsid(name: &str) -> Nsid { @@ -433,7 +432,6 @@ fn mutate( base, head, ticket, - pull, } = progress; let (signer, did) = actors .owner_repos @@ -476,14 +474,6 @@ fn mutate( } }), ), - SocialStage::Merge => ( - "mergePull", - nsid(PULL_MERGE_NSID), - json!({ - "repo": repo, - "patch": pull.expect("pull rkey") - }), - ), }; let request = signed_post( harness, diff --git a/knot2/crates/knot-sim/tests/lfs_roundtrip.rs b/knot2/crates/knot-sim/tests/lfs_roundtrip.rs index f7de17a0d..3e022d7c9 100644 --- a/knot2/crates/knot-sim/tests/lfs_roundtrip.rs +++ b/knot2/crates/knot-sim/tests/lfs_roundtrip.rs @@ -21,7 +21,7 @@ use knot_runtime::{ }; use knot_secrets::{MasterKey, SealedStore}; use knot_types::{ - AccountDid, ActorId, AdmissionPolicy, AuthorName, Did, Email, KnotHostname, KnotId, + AccountDid, ActorId, AdmissionPolicy, Did, KnotHostname, KnotId, KnotServiceUrl, Nsid, OwnerDid, RepoDid, RepoName, RepoRkey, ServiceDid, UnixSeconds, }; use tempfile::TempDir; @@ -395,10 +395,6 @@ async fn spawn(published_line: String, with_h3: bool) -> World { knot_xrpc::GlobalQuota::new(16), )), proxy_trust: knot_types::ProxyTrust::default(), - committer: knot_xrpc::Committer { - name: AuthorName::new("Tangled"), - email: Email::new("noreply@tangled.sh"), - }, byte_limits: knot_xrpc::ByteLimits { pack: knot_xrpc::MaxWireBytes::new(1 << 30), ..knot_xrpc::ByteLimits::default() diff --git a/knot2/crates/knot-ssh/tests/ssh_push.rs b/knot2/crates/knot-ssh/tests/ssh_push.rs index b9f28df6c..d3d20957a 100644 --- a/knot2/crates/knot-ssh/tests/ssh_push.rs +++ b/knot2/crates/knot-ssh/tests/ssh_push.rs @@ -453,10 +453,6 @@ async fn spawn_server_core( knot_xrpc::GlobalQuota::new(16), )), proxy_trust: knot_types::ProxyTrust::default(), - committer: knot_xrpc::Committer { - name: knot_types::AuthorName::new("Tangled"), - email: knot_types::Email::new("noreply@tangled.sh"), - }, byte_limits: knot_xrpc::ByteLimits::default(), budgets: knot_xrpc::Budgets::default(), git_http: Arc::new(knot_runtime::FakeHttp::new( diff --git a/knot2/crates/knot-xrpc/src/forks.rs b/knot2/crates/knot-xrpc/src/forks.rs index 512332e6f..99ad14396 100644 --- a/knot2/crates/knot-xrpc/src/forks.rs +++ b/knot2/crates/knot-xrpc/src/forks.rs @@ -6,11 +6,12 @@ use url::Url; use knot_git::{Filter, Haves, RefUpdate, Repo, Wants}; use knot_pack::{FetchError, HaveOids, UpstreamRefs, WantOids}; use knot_runtime::{Clock, HttpTransport}; -use knot_types::{ObjectFormat, Oid, OriginUrl, OwnerDid, RefName, RepoDid, RepoName}; +use knot_types::{ObjectFormat, Oid, OriginUrl, OwnerDid, RefName, RepoDid, RepoName, RepoRkey}; use crate::body::SourceUrl; use crate::error::XrpcError; -use crate::reads::{HostedRepo, require_hosted}; +use crate::query::RepoArg; +use crate::reads::{HostedRepo, repo_not_found, require_hosted, resolve_repo}; use crate::{XrpcState, run_blocking}; #[derive(Clone)] @@ -59,13 +60,23 @@ impl LocalPath { } } +pub(crate) fn resolve_by_name( + state: &XrpcState, + owner: &OwnerDid, + name: &RepoName, +) -> Result { + let rkey = RepoRkey::new(name.as_str()).map_err(|_| repo_not_found())?; + let owner = owner.clone(); + resolve_repo(state, &RepoArg::OwnerRkey { owner, rkey }) +} + fn resolve_local( state: &XrpcState, path: &LocalPath, ) -> Result { match path { LocalPath::Did(did) => require_hosted(state, did.clone()), - LocalPath::Named { owner, name } => crate::merge::resolve_by_name(state, owner, name), + LocalPath::Named { owner, name } => resolve_by_name(state, owner, name), } } diff --git a/knot2/crates/knot-xrpc/src/lib.rs b/knot2/crates/knot-xrpc/src/lib.rs index 08f5b34b9..a3e5b4a2d 100644 --- a/knot2/crates/knot-xrpc/src/lib.rs +++ b/knot2/crates/knot-xrpc/src/lib.rs @@ -18,7 +18,6 @@ mod lfs; mod locks; pub mod materialize; mod members; -mod merge; pub mod migrate; mod mint; mod out; @@ -46,12 +45,11 @@ pub use knot_resource::{ }; pub use lfs::LfsWeb; pub use locks::CobLocks; -pub use merge::Committer; pub use outbox::{Outbox, OutboxLimits}; pub use receive::advertiser as receive_advertiser; pub use refrecords::RefProjection; pub use reservations::{GlobalQuota, PerActorQuota, ReservationTtl, Reservations}; -pub use social::{RecordKeys, RepoMethod, seal_pending_merges}; +pub use social::{RecordKeys, RepoMethod}; use std::collections::BTreeSet; use std::net::IpAddr; @@ -204,7 +202,6 @@ pub struct XrpcState { pub cob_locks: Arc, pub reservations: Arc, pub proxy_trust: knot_types::ProxyTrust, - pub committer: Committer, pub byte_limits: ByteLimits, pub budgets: Budgets, pub git_http: Arc, @@ -370,11 +367,7 @@ impl FromRequestParts for Method { } pub fn router(state: Arc>) -> Router { - let merge_routes = Router::new() - .route(merge::MERGE_CHECK_ROUTE, post(merge::merge_check::)) - .layer(DefaultBodyLimit::max(state.byte_limits.patch.get())); Router::new() - .merge(merge_routes) .route( invite_codes::REDEEM_ROUTE, post(invite_codes::redeem_invite_code::), @@ -392,7 +385,6 @@ pub fn router(state: Arc>) -> Router social::LIST_RECORD_VERSIONS_ROUTE, get(social::list_record_versions::), ) - .route(social::MERGE_PATCH_ROUTE, post(social::merge_pull::)) .route( social::SUBMIT_PATCH_ROUTE, post(social::submit_patch::), diff --git a/knot2/crates/knot-xrpc/src/merge.rs b/knot2/crates/knot-xrpc/src/merge.rs deleted file mode 100644 index 103614045..000000000 --- a/knot2/crates/knot-xrpc/src/merge.rs +++ /dev/null @@ -1,109 +0,0 @@ -use std::sync::Arc; - -use axum::Json; -use axum::body::Bytes; -use axum::extract::State; -use axum::response::{IntoResponse, Response}; -use http::StatusCode; -use serde::{Deserialize, Serialize}; - -use knot_git::Repo; -use knot_runtime::{Clock, HttpTransport}; -use knot_types::{ - AuthorName, BranchName, Email, Oid, OwnerDid, RecordRkey, RefName, RepoDid, RepoName, RepoPath, - RepoRkey, -}; - -use crate::XrpcState; -use crate::decode; -use crate::error::XrpcError; -use crate::query::RepoArg; -use crate::reads::{HostedRepo, repo_not_found, require_hosted, resolve_repo}; -pub(crate) const MERGE_CHECK_ROUTE: &str = "/xrpc/org.tangled.git.mergeCheck"; -pub(crate) const CONFLICT_MESSAGE: &str = "patch can't be applied cleanly"; - -const REPLAY_CONFLICT: &str = "changed on the target branch too"; - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Committer { - pub name: AuthorName, - pub email: Email, -} - -#[derive(Deserialize)] -#[serde(deny_unknown_fields)] -struct MergeCheckInput { - repo: RepoDid, - patch: RecordRkey, - #[serde(default)] - branch: Option, -} - -#[derive(Serialize)] -struct ConflictOut { - filename: RepoPath, - reason: String, -} - -#[derive(Serialize)] -struct MergeCheckOutput { - #[serde(rename = "isConflicted")] - is_conflicted: bool, - conflicts: Vec, - #[serde(skip_serializing_if = "Option::is_none")] - message: Option, -} - -impl MergeCheckOutput { - fn clean() -> Self { - Self { - is_conflicted: false, - conflicts: Vec::new(), - message: None, - } - } - - fn conflicted(path: RepoPath) -> Self { - Self { - is_conflicted: true, - conflicts: vec![ConflictOut { - filename: path, - reason: REPLAY_CONFLICT.to_string(), - }], - message: Some(CONFLICT_MESSAGE.to_string()), - } - } -} - -pub(crate) fn resolve_by_name( - state: &XrpcState, - owner: &OwnerDid, - name: &RepoName, -) -> Result { - let rkey = RepoRkey::new(name.as_str()).map_err(|_| repo_not_found())?; - let owner = owner.clone(); - resolve_repo(state, &RepoArg::OwnerRkey { owner, rkey }) -} - -pub(crate) fn branch_tip(repo: &Repo, refname: &RefName) -> Result { - repo.find_ref(refname)? - .ok_or_else(|| XrpcError::invalid_request("No such branch to merge into")) -} - -pub(crate) async fn merge_check( - State(state): State>>, - body: Bytes, -) -> Result { - let input: MergeCheckInput = decode(&body)?; - require_hosted(&state, input.repo.clone())?; - let checked = - crate::social::check_patch_merge(&state, &input.repo, &input.patch, input.branch).await?; - Ok(check_response(match checked { - crate::social::PatchMerge::Clean => MergeCheckOutput::clean(), - crate::social::PatchMerge::Conflict { path } => MergeCheckOutput::conflicted(path), - })) -} - -fn check_response(output: MergeCheckOutput) -> Response { - (StatusCode::OK, Json(output)).into_response() -} diff --git a/knot2/crates/knot-xrpc/src/refrecords.rs b/knot2/crates/knot-xrpc/src/refrecords.rs index dceab0790..bdf20eb1f 100644 --- a/knot2/crates/knot-xrpc/src/refrecords.rs +++ b/knot2/crates/knot-xrpc/src/refrecords.rs @@ -7,7 +7,7 @@ use knot_record::chain; use knot_record::git_refs; use knot_record::manifest::{MANIFEST_COLLECTION, MANIFEST_RKEY}; use knot_runtime::{Clock, HttpTransport}; -use knot_types::{AccountDid, ObjectFormat, Oid, PushOptions, RefName, RepoDid, ServiceDid}; +use knot_types::{AccountDid, ObjectFormat, PushOptions, RepoDid, ServiceDid}; use crate::error::XrpcError; use crate::run_blocking; @@ -93,42 +93,6 @@ fn out_of_step(did: &RepoDid, error: &dyn std::fmt::Display) { ); } -pub(crate) async fn seal_ref_move( - state: &Arc>, - did: RepoDid, - refname: RefName, - old: Option, - new: Oid, - actor: AccountDid, -) -> Result<(), XrpcError> { - let layout = state.layout.clone(); - let state = Arc::clone(state); - run_blocking(move || -> Result<(), XrpcError> { - let repo = layout.open(&did)?; - let update = match old { - Some(old) => RefUpdate::Update { - name: refname, - old, - new, - }, - None => RefUpdate::Create { name: refname, new }, - }; - let applied = std::slice::from_ref(&update); - sync_refs(&state, &repo, &did, applied); - // a merge skips the push, so tell the spindle here - notify_spindle( - &state, - &repo, - &did, - applied, - &actor, - &PushOptions::default(), - ); - Ok(()) - }) - .await -} - pub(crate) async fn sync_after( state: &Arc>, did: &RepoDid, @@ -295,7 +259,7 @@ fn get_repo_spindle(repo: &Repo) -> Result, String> { #[cfg(test)] mod tests { - use knot_types::PushOption; + use knot_types::{Oid, PushOption, RefName}; use super::*; diff --git a/knot2/crates/knot-xrpc/src/social.rs b/knot2/crates/knot-xrpc/src/social.rs index d1adb9cbf..55124a9e7 100644 --- a/knot2/crates/knot-xrpc/src/social.rs +++ b/knot2/crates/knot-xrpc/src/social.rs @@ -8,7 +8,6 @@ use axum::response::{IntoResponse, Response}; use cid::Cid; use http::HeaderMap; use jacquard_common::types::tid::Ticker; -use jacquard_repo::storage::BlockStore; use knot_acl::{KnotAcl, RepoAccess}; use knot_cob::{ Approved, ChangePayload, CobError, CobHome, CobStore, Materialized, Record, WriteContext, @@ -42,7 +41,7 @@ use knot_runtime::{Clock, HttpTransport, Signer}; use knot_types::{ AccountDid, AtUri, BranchName, ChangeId, CobId, ContributionPermission, ContributionPolicy, Decision, MstKey, Oid, OwnerDid, RecordAddress, RecordBody, RecordCid, RecordCollection, - RecordRkey, RefName, RepoDid, RepoName, RepoPath, RepoRkey, ServiceDid, SubjectNumber, Tid, + RecordRkey, RepoDid, RepoName, RepoRkey, ServiceDid, SubjectNumber, Tid, TypeName, UnixSeconds, }; use serde::de::DeserializeOwned; @@ -2500,7 +2499,6 @@ fn pinned( ledger: &Ledger<'_, H, C>, address: &RecordAddress, range: CommitRange, - stage: &'static str, ) { [range.base, range.head] .into_iter() @@ -2522,45 +2520,12 @@ fn pinned( tracing::warn!( repo = %ledger.repo, %error, - "A patch's keep refs wouldn't write {stage}; the boot pass retries them" + "A patch's keep refs wouldn't write after the open" ) }) .ok(); } -#[derive(Deserialize)] -struct StoredPatch { - base: OidDraft, - head: OidDraft, -} - -fn patch_ends(git: &Repo, address: &RecordAddress) -> Result { - let tip = chain::tip(git)? - .ok_or_else(|| XrpcError::internal("a repository with a live patch has its chain tip"))?; - let tree = chain::load_tree(git, &tip); - let store = knot_record::blocks::GitBlocks::at(git, tip.blocks); - let bytes = futures::executor::block_on(async { - let cid = tree - .get(address.mst_key().as_str()) - .await - .map_err(|error| XrpcError::internal(error.to_string()))? - .ok_or_else(|| { - XrpcError::internal("a patch that passed its liveness check has a serving leaf") - })?; - store - .get(&cid) - .await - .map_err(|error| XrpcError::internal(error.to_string()))? - .ok_or_else(|| XrpcError::internal("a serving patch's block is missing")) - })?; - let stored: StoredPatch = serde_ipld_dagcbor::from_slice(bytes.as_ref()) - .map_err(|error| XrpcError::internal(format!("a patch record doesn't parse: {error}")))?; - Ok(CommitRange { - base: stored.base.oid, - head: stored.head.oid, - }) -} - fn patch_ticket( ledger: &Ledger<'_, H, C>, ticket_address: &RecordAddress, @@ -2638,7 +2603,7 @@ async fn open_patch( require_commit(ledger, parsed.range.base, "base")?; require_commit(ledger, parsed.range.head, "head")?; require_ancestor(ledger, parsed.range.base, parsed.range.head)?; - pinned(ledger, &address, parsed.range, "after the open"); + pinned(ledger, &address, parsed.range); Ok(None) } Err(other) => Err(other), @@ -2832,657 +2797,3 @@ pub(crate) async fn list_record_versions( response_limit, ) } - -pub(crate) const MERGE_PATCH_ROUTE: &str = "/xrpc/org.tangled.review.mergePatch"; - -const MERGE_DENIED: &str = "merging belongs to the repository's owner and collaborators"; - -const MERGE_LANDING_ATTEMPTS: usize = 16; - -#[derive(Deserialize)] -#[serde(deny_unknown_fields)] -struct MergePatchInput { - repo: RepoDid, - patch: RecordRkey, -} - -struct MergeMove { - branch: knot_types::RefName, - old: Oid, - new: Oid, -} - -struct CompletedMove { - branch: knot_types::RefName, - old: Oid, - new: Oid, - editor: AccountDid, -} - -impl CompletedMove { - fn by(editor: AccountDid, moved: &MergeMove) -> Self { - Self { - branch: moved.branch.clone(), - old: moved.old, - new: moved.new, - editor, - } - } -} - -fn sunk(sink: &Mutex>, moved: CompletedMove) { - sink.lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()) - .push(moved); -} - -#[derive(Serialize)] -struct MergePatchOutput { - tip: Oid, - merged: Vec>, -} - -fn replay_refused(error: knot_git::ReplayError) -> XrpcError { - match error { - knot_git::ReplayError::Conflict { path } => XrpcError::named( - http::StatusCode::CONFLICT, - "MergeConflict", - format!( - "Merge failed due to conflicts: {CONFLICT_MESSAGE}: {path} changed on the \ - target branch too", - CONFLICT_MESSAGE = crate::merge::CONFLICT_MESSAGE - ), - ), - knot_git::ReplayError::Vanished { .. } | knot_git::ReplayError::Cycle => { - XrpcError::internal(error.to_string()) - } - knot_git::ReplayError::Git(error) => XrpcError::from(error), - } -} - -fn move_branch(git: &Repo, moved: &MergeMove) -> Result<(), knot_git::GitError> { - git.update_ref(&knot_git::RefUpdate::Update { - name: moved.branch.clone(), - old: moved.old, - new: moved.new, - }) -} - -struct LiveTicket { - address: RecordAddress, - object: CobId, - live: SocialObject, -} - -#[derive(Deserialize)] -struct StoredTicket { - title: StoredTitle, - #[serde(default)] - body: Option, - #[serde(rename = "tangledLabels", default)] - tangled_labels: Option>, - #[serde(rename = "targetBranch", default)] - target_branch: Option, - #[serde(default)] - mentions: Option>, - #[serde(default)] - references: Option>>, -} - -#[derive(Deserialize)] -struct StoredTitle { - text: RecordTicketTitle, -} - -#[derive(Deserialize)] -struct StoredBody { - text: MarkdownBody, - #[serde(default)] - blobs: Option>, -} - -fn stored_ticket_of(git: &Repo, ticket: &LiveTicket) -> Result { - let version = ticket - .live - .version() - .ok_or_else(|| XrpcError::internal("A live ticket is missing its version"))?; - let body = tree_body( - &CobStore::new(git), - &collection_type(ticket.address.collection()), - ticket.object, - version, - )? - .ok_or_else(|| XrpcError::internal("A live ticket is missing its record bytes"))?; - serde_ipld_dagcbor::from_slice(body.as_bytes()) - .map_err(|error| XrpcError::internal(format!("The ticket record doesn't parse: {error}"))) -} - -fn merge_target(git: &Repo, stored: &StoredTicket) -> Option { - stored - .target_branch - .as_ref() - .map(TargetBranch::head_ref) - .or_else(|| git.default_branch()) -} - -fn complete_ticket( - ledger: &Ledger<'_, H, C>, - cx: &WriteContext<'_>, - ticket: &LiveTicket, -) -> Result<(), XrpcError> { - let from = ticket - .live - .version() - .ok_or_else(|| XrpcError::internal("A live ticket is missing its version"))?; - let stored = stored_ticket_of(ledger.git, ticket)?; - let unusable = |what: &'static str| { - move |error| XrpcError::internal(format!("The ticket's {what} is unusable: {error}")) - }; - let StoredTicket { - title, - body, - tangled_labels, - target_branch, - mentions, - references, - } = stored; - let (text, attachments) = match body { - Some(body) => (Some(body.text), body.blobs), - None => (None, None), - }; - let prose = Prose::new(title.text, text, mentions, references).map_err(unusable("prose"))?; - let (record, body) = ticket_body_of( - prose, - TicketState::Completed, - tangled_labels, - target_branch, - attachments, - ticket.live.created_at(), - cx.actor().clone(), - )?; - let produces = record.cid(); - let change: TicketChange = SocialChange::Edit { - editing: Editing { - author: cx.actor().clone(), - from, - produces, - edited_at: ledger.now, - }, - body, - }; - ledger.write( - cx, - &ticket.address, - change, - Doing::Edit { - object: ticket.object, - from, - body: &record, - }, - )?; - Ok(()) -} - -fn committer_at( - state: &XrpcState, - now: UnixSeconds, -) -> knot_git::Identity { - knot_git::Identity { - name: state.committer.name.clone(), - email: state.committer.email.clone(), - time: now, - offset_seconds: 0, - } -} - -fn committer_of(ledger: &Ledger<'_, H, C>) -> knot_git::Identity { - committer_at(ledger.state, ledger.now) -} - -/// The merge a patch landing takes, without any of its writes: fast-forward -/// when the tip already sits under the head, the tip itself when the head is -/// already in, and a replay of the range's commits onto the tip otherwise. -/// The replay writes the commits it lands into the object store, where they -/// stay unreachable until a real merge lands them. -pub(crate) fn merge_dry_run( - git: &Repo, - committer: &knot_git::Identity, - tip: Oid, - range: CommitRange, -) -> Result { - match git.merge_base(tip, range.head)? { - Some(merged) if merged == tip => Ok(range.head), - Some(merged) if merged == range.head => Ok(tip), - _ => knot_git::replay_onto(git, tip, range, committer), - } -} - -fn merge_outcome( - ledger: &Ledger<'_, H, C>, - committer: &knot_git::Identity, - tip: Oid, - range: CommitRange, -) -> Result { - merge_dry_run(ledger.git, committer, tip, range).map_err(replay_refused) -} - -fn merge_pair( - ledger: &Ledger<'_, H, C>, - cx: &WriteContext<'_>, - ticket: &LiveTicket, - range: CommitRange, - branch: knot_types::RefName, -) -> Result { - if ledger.git.find_ref(&branch)?.is_none() { - return Err(XrpcError::named( - http::StatusCode::CONFLICT, - "NoSuchBranch", - format!( - "the pull's target branch {} was deleted before the merge began", - branch, - ), - )); - } - let committer = committer_of(ledger); - let mut completed = false; - for attempt in 0..MERGE_LANDING_ATTEMPTS { - let tip = crate::merge::branch_tip(ledger.git, &branch)?; - let merged_at = merge_outcome(ledger, &committer, tip, range)?; - if !completed { - complete_ticket(ledger, cx, ticket)?; - completed = true; - } - let moved = MergeMove { - branch: branch.clone(), - old: tip, - new: merged_at, - }; - if moved.old == moved.new { - return Ok(moved); - } - match move_branch(ledger.git, &moved) { - Ok(()) => return Ok(moved), - Err(error) - if attempt + 1 < MERGE_LANDING_ATTEMPTS - && matches!( - error, - knot_git::GitError::AtomicRefs(_) | knot_git::GitError::Reference { .. } - ) => {} - Err(error) => { - return Err(XrpcError::named( - http::StatusCode::CONFLICT, - "BranchMoved", - format!( - "the target branch moved off {} while the merge ran ({error}); the \ - completed ticket stands, and the boot pass finishes the move", - moved.old, - ), - )); - } - } - } - Err(XrpcError::internal(format!( - "the merge stopped after {MERGE_LANDING_ATTEMPTS} attempts at a branch that kept moving" - ))) -} - -fn finish_move( - ledger: &Ledger<'_, H, C>, - moved: &MergeMove, -) -> Result { - let current = crate::merge::branch_tip(ledger.git, &moved.branch)?; - if current == moved.new { - return Ok(false); - } - if current == moved.old { - move_branch(ledger.git, moved).map_err(XrpcError::from)?; - return Ok(true); - } - Ok(false) -} - -fn ticket_of_patch( - ledger: &Ledger<'_, H, C>, - patch_live: &SocialObject, -) -> Result { - let address = patch_live.parent().cloned().ok_or_else(|| { - XrpcError::internal("A patch's parent is its ticket, and this patch has none") - })?; - let (object, live) = standing(ledger, &address)?; - Ok(LiveTicket { - address, - object, - live, - }) -} - -fn mend_patch( - ledger: &Ledger<'_, H, C>, - address: &RecordAddress, -) -> Result, XrpcError> { - let range = patch_ends(ledger.git, address)?; - let (_, patch_live) = standing(ledger, address)?; - let live = ticket_of_patch(ledger, &patch_live)?; - let ticket = opened_ticket(ledger, live.object)?; - if ticket.state != TicketState::Completed { - return Ok(None); - } - pinned(ledger, address, range, "at boot"); - let stored = stored_ticket_of(ledger.git, &live)?; - let branch = merge_target(ledger.git, &stored).ok_or_else(|| { - XrpcError::internal("A completed patch's repository has no default branch") - })?; - let tip = crate::merge::branch_tip(ledger.git, &branch)?; - let merged_at = merge_outcome(ledger, &committer_of(ledger), tip, range)?; - let moved = MergeMove { - branch, - old: tip, - new: merged_at, - }; - match finish_move(ledger, &moved) { - Ok(true) => Ok(Some(CompletedMove::by(live.live.editor().clone(), &moved))), - Ok(false) => Ok(None), - Err(error) => { - tracing::warn!( - repo = %ledger.repo, - %error, - "A completed patch's branch move didn't finish; the next boot retries it" - ); - Ok(None) - } - } -} - -fn warned(repo: &RepoDid, step: &'static str, found: Result) -> Option { - found - .inspect_err(|error| tracing::warn!(repo = %repo, %error, "{step}")) - .ok() -} - -fn reconcile_repo_pulls( - state: &Arc>, - repo: &RepoDid, -) -> Result, XrpcError> { - let _guard = state.cob_locks.repo(repo); - let _social = state.index.guard_social(repo)?; - let git = state.layout.open(repo)?; - let patches = ready( - state.index.social_of_kind(repo, &patch_collection()), - "social", - )?; - let signer = state.secrets.signer(&state.knot_did)?; - let ledger = Ledger { - state, - git: &git, - repo, - signer: &signer, - now: state.now(), - }; - Ok(patches - .iter() - .filter_map(|object| { - warned( - repo, - "A patch's address wouldn't resolve off the projection at boot", - ready(state.index.social_address_of(repo, *object), "social"), - ) - .flatten() - .map(|address| (object, address)) - }) - .filter_map(|(object, address)| { - warned( - repo, - "A patch wouldn't materialize at boot, so its keep refs and branch moves wait", - state_at(&git, &address, *object).map(|(state, _)| state.object().cloned()), - ) - .map(|_| address) - }) - .filter_map(|address| { - warned( - repo, - "A patch wouldn't reconcile at boot; the next boot retries it", - mend_patch(&ledger, &address), - ) - .flatten() - }) - .collect()) -} - -pub async fn seal_pending_merges( - state: &Arc>, -) -> usize { - let reconciling = Arc::clone(state); - let finished = run_blocking(move || { - Ok(reconciling - .index - .hosted_repos() - .into_iter() - .map(|repo| { - let moves = warned( - &repo, - "A repository's pending-pull pass didn't run; the next boot retries it", - reconcile_repo_pulls(&reconciling, &repo), - ) - .unwrap_or_default(); - (repo, moves) - }) - .collect::>()) - }) - .await - .unwrap_or_else(|error| { - tracing::warn!(%error, "The pending-pull pass didn't run; the next boot retries it"); - Vec::new() - }); - let mut sealed = 0; - for (repo, moves) in finished { - sealed += seal_moves( - state, - &repo, - &moves, - "A branch move finished at boot wouldn't seal; the next boot retries it", - ) - .await; - } - sealed -} - -async fn seal_moves( - state: &Arc>, - repo: &RepoDid, - moves: &[CompletedMove], - warning: &'static str, -) -> usize { - for moved in moves { - if let Err(error) = crate::refrecords::seal_ref_move( - state, - repo.clone(), - moved.branch.clone(), - Some(moved.old), - moved.new, - moved.editor.clone(), - ) - .await - { - tracing::warn!(repo = %repo, %error, "{warning}"); - } - } - moves.len() -} - -fn patch_not_found(rkey: &RecordRkey) -> XrpcError { - XrpcError::named( - http::StatusCode::NOT_FOUND, - "PatchNotFound", - format!("no org.tangled.review.patch record stands under the key {rkey}"), - ) -} - -pub(crate) struct LivePatch { - pub(crate) range: CommitRange, - /// The ticket's target branch, or the repository's default branch; a - /// caller who sends a branch of its own overrides this. - pub(crate) target: Option, -} - -fn live_patch( - state: &XrpcState, - git: &Repo, - repo: &RepoDid, - rkey: &RecordRkey, -) -> Result { - let address = RecordAddress::new(patch_collection(), rkey.clone()); - let object = match object_at(state, repo, &address) { - Ok(object) => object, - Err(missing) if missing.status() == http::StatusCode::NOT_FOUND => { - return Err(patch_not_found(rkey)); - } - Err(other) => return Err(other), - }; - let live = serving_object(git, &address, object)?.ok_or_else(|| patch_not_found(rkey))?; - let range = patch_ends(git, &address)?; - let ticket_branch = match live.parent() { - Some(ticket) => ticket_target(state, git, repo, ticket)?, - None => None, - }; - let target = ticket_branch.or_else(|| git.default_branch()); - Ok(LivePatch { range, target }) -} - -fn ticket_target( - state: &XrpcState, - git: &Repo, - repo: &RepoDid, - address: &RecordAddress, -) -> Result, XrpcError> { - let object = match object_at(state, repo, address) { - Ok(object) => object, - Err(missing) if missing.status() == http::StatusCode::NOT_FOUND => return Ok(None), - Err(other) => return Err(other), - }; - let live = match serving_object(git, address, object)? { - Some(live) => live, - None => return Ok(None), - }; - let stored = stored_ticket_of( - git, - &LiveTicket { - address: address.clone(), - object, - live, - }, - )?; - Ok(merge_target(git, &stored)) -} - -pub(crate) enum PatchMerge { - Clean, - /// The replay stops at the first file that changed on the target branch - /// too, so one conflict is all it can report. - Conflict { - path: RepoPath, - }, -} - -pub(crate) async fn check_patch_merge( - state: &Arc>, - repo: &RepoDid, - rkey: &RecordRkey, - branch: Option, -) -> Result { - warm_social(state, repo).await?; - let index_state = Arc::clone(state); - let layout = state.layout.clone(); - let repo = repo.clone(); - let rkey = rkey.clone(); - let override_branch = branch.map(|branch| branch.head_ref()); - let committer = committer_at(state, state.now()); - run_blocking(move || { - let git = layout.open(&repo)?; - let patch = live_patch(&index_state, &git, &repo, &rkey)?; - let branch = override_branch.or(patch.target).ok_or_else(|| { - XrpcError::invalid_request( - "the repository has no default branch to check against, and the patch's \ - ticket cites no target branch", - ) - })?; - let tip = crate::merge::branch_tip(&git, &branch)?; - match merge_dry_run(&git, &committer, tip, patch.range) { - Ok(_) => Ok(PatchMerge::Clean), - Err(knot_git::ReplayError::Conflict { path }) => Ok(PatchMerge::Conflict { path }), - Err(knot_git::ReplayError::Vanished { .. } | knot_git::ReplayError::Cycle) => Err( - XrpcError::internal("the patch's history can't be ordered for a replay"), - ), - Err(knot_git::ReplayError::Git(error)) => Err(XrpcError::from(error)), - } - }) - .await -} -pub(crate) async fn merge_pull( - State(state): State>>, - headers: HeaderMap, - method: crate::Method, - body: Bytes, -) -> Result { - let input = decode::(&body)?; - let address = RecordAddress::new(patch_collection(), input.patch); - let session = session(&state, &headers, &method, input.repo.clone()).await?; - crate::authorize_push(&state, &session.actor, &input.repo, MERGE_DENIED).await?; - let moved_refs = Arc::new(Mutex::new(Vec::::new())); - let sink = Arc::clone(&moved_refs); - let outcome = on_repo(&state, session, move |ledger, cx| { - let (_, patch_live) = standing(ledger, &address)?; - let range = patch_ends(ledger.git, &address)?; - let live = ticket_of_patch(ledger, &patch_live)?; - let ticket = opened_ticket(ledger, live.object)?; - match ticket.state { - TicketState::Completed => { - return Err(XrpcError::named( - http::StatusCode::CONFLICT, - "AlreadyMerged", - format!("patch at {} is already merged", address.at_uri(ledger.repo)), - )); - } - TicketState::Closed => { - return Err(XrpcError::named( - http::StatusCode::CONFLICT, - "ClosedTicket", - format!( - "the patch at {} is on a closed ticket", - address.at_uri(ledger.repo), - ), - )); - } - TicketState::Open => {} - } - let stored = stored_ticket_of(ledger.git, &live)?; - let branch = merge_target(ledger.git, &stored).ok_or_else(|| { - XrpcError::named( - http::StatusCode::CONFLICT, - "NoSuchBranch", - "the repository has no default branch to merge into", - ) - })?; - let moved = merge_pair(ledger, cx, &live, range, branch)?; - if moved.old != moved.new { - sunk(&sink, CompletedMove::by(cx.actor().clone(), &moved)); - } - Ok(MergePatchOutput { - tip: moved.new, - merged: vec![address.at_uri(ledger.repo)], - }) - }) - .await; - let moves = std::mem::take( - &mut *moved_refs - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()), - ); - seal_moves( - &state, - &input.repo, - &moves, - "Post-receive after patch merge failed", - ) - .await; - outcome -} diff --git a/knot2/crates/knot-xrpc/src/tests.rs b/knot2/crates/knot-xrpc/src/tests.rs index 9848622ed..8ccb89e04 100644 --- a/knot2/crates/knot-xrpc/src/tests.rs +++ b/knot2/crates/knot-xrpc/src/tests.rs @@ -369,10 +369,6 @@ fn state_from( cob_locks: Arc::new(crate::CobLocks::default()), reservations, proxy_trust: knot_types::ProxyTrust::default(), - committer: crate::Committer { - name: AuthorName::new("Tangled"), - email: Email::new("noreply@tangled.sh"), - }, byte_limits: crate::ByteLimits::default(), budgets: crate::Budgets::default(), git_http, @@ -3478,11 +3474,11 @@ async fn resolve_by_name_matches_the_rkey_case_sensitively() { ); assert!( - crate::merge::resolve_by_name(&*state, &owner, &RepoName::new("anemone").unwrap()).is_ok(), + crate::forks::resolve_by_name(&*state, &owner, &RepoName::new("anemone").unwrap()).is_ok(), "the exact rkey resolves" ); assert!( - crate::merge::resolve_by_name(&*state, &owner, &RepoName::new("Anemone").unwrap()).is_err(), + crate::forks::resolve_by_name(&*state, &owner, &RepoName::new("Anemone").unwrap()).is_err(), "rkeys are case-sensitive, so a differently-cased name mustn't resolve at all" ); } @@ -4293,9 +4289,8 @@ async fn health_is_unauthenticated_and_exempt_from_shedding() { ); } -mod merge_endpoints { +mod patch_endpoints { use super::*; - use std::path::Path; use knot_git::{EntryKind, Identity, NewCommit, RefUpdate, StagedAction, StagedChange}; use knot_types::{Oid, RefName, UnixSeconds}; @@ -4341,30 +4336,6 @@ mod merge_endpoints { commit } - fn main_tip(world: &World, repo_did: &RepoDid) -> Oid { - world - .layout - .open(repo_did) - .unwrap() - .find_ref(&RefName::new("refs/heads/main").unwrap()) - .unwrap() - .unwrap() - } - - fn file_count(dir: &Path) -> usize { - std::fs::read_dir(dir) - .map(|entries| { - entries - .flatten() - .map(|entry| match entry.file_type() { - Ok(kind) if kind.is_dir() => file_count(&entry.path()), - _ => 1, - }) - .sum() - }) - .unwrap_or(0) - } - fn ident() -> Identity { Identity { name: AuthorName::new("nel"), @@ -4396,23 +4367,6 @@ mod merge_endpoints { .unwrap() } - async fn patch_record(world: &World, repo: &RepoDid, base: Oid, head: Oid) -> RecordRkey { - let (ticket, _) = Ticket::on(world, repo).opened(&world.member, "kelp").await; - let written = ok(submit_patch_as( - world, - &world.member, - repo, - json!({ - "ticket": rkey_of(&ticket), - "base": { "oid": base.to_hex() }, - "head": { "oid": head.to_hex() }, - "sourceRepo": repo - }), - ) - .await); - rkey_of(&uri_of(&written)) - } - #[tokio::test] async fn patch_keeps_its_source_and_refuses_a_bad_branch() { let world = World::new(); @@ -4487,91 +4441,6 @@ mod merge_endpoints { } } - #[tokio::test] - async fn merge_check_reports_clean_conflicted_and_missing_patches() { - let world = World::new(); - world.grandfather(MEMBER_HOST); - let repo_did = create_repo_helper(&world, "scallop").await; - let base = seed_main(&world, &repo_did, &[("reef.txt", "old line\n")]); - let head = advance(&world, &repo_did, base, "reef.txt", "new line\n"); - let patch = patch_record(&world, &repo_did, base, head).await; - - let drifted = advance( - &world, - &repo_did, - base, - "reef.txt", - "someone else moved it\n", - ); - let git = world.layout.open(&repo_did).unwrap(); - git.update_ref(&RefUpdate::Create { - name: RefName::new("refs/heads/side").unwrap(), - new: drifted, - }) - .unwrap(); - let objects_before = file_count(&git.objects_dir()); - - let input = |branch: &BranchName, patch: &RecordRkey| { - body(json!({ - "repo": repo_did, - "branch": branch, - "patch": patch, - })) - }; - - let clean = json_of( - crate::merge::merge_check( - world.state(), - input(&BranchName::new("main").unwrap(), &patch), - ) - .await - .unwrap(), - ) - .await; - assert_eq!(clean["isConflicted"], false); - assert_eq!(clean["conflicts"], serde_json::json!([])); - assert!(clean.get("message").is_none()); - - let conflicted = json_of( - crate::merge::merge_check( - world.state(), - input(&BranchName::new("side").unwrap(), &patch), - ) - .await - .unwrap(), - ) - .await; - assert_eq!(conflicted["isConflicted"], true); - assert_eq!(conflicted["conflicts"][0]["filename"], "reef.txt"); - assert_eq!( - conflicted["conflicts"][0]["reason"], - "changed on the target branch too" - ); - assert_eq!(conflicted["message"], "patch can't be applied cleanly"); - - let missing = crate::merge::merge_check( - world.state(), - input( - &BranchName::new("main").unwrap(), - &RecordRkey::new("3jzfcijpj2z2a").unwrap(), - ), - ) - .await; - assert_eq!( - missing.unwrap_err().into_response().status(), - StatusCode::NOT_FOUND, - "a missing patch record is a 404, not a conflict" - ); - - let git = world.layout.open(&repo_did).unwrap(); - assert_eq!( - file_count(&git.objects_dir()), - objects_before, - "merge check must write nothing into the object database" - ); - assert_eq!(main_tip(&world, &repo_did), base); - } - #[tokio::test] async fn a_repo_did_addresses_a_repo_whose_rkey_is_its_lowercased_name() { let world = World::new(); @@ -4591,39 +4460,12 @@ mod merge_endpoints { resolve(&world, &RepoName::new("Periwinkle").unwrap()), Resolved::Ready(None) )); - let repo_did = match resolve(&world, &RepoName::new("periwinkle").unwrap()) { - Resolved::Ready(Some(did)) => did, - other => panic!("Repo wasn't registered under its lowercased name: {other:?}"), - }; - let base = seed_main(&world, &repo_did, &[("reef.txt", "old line\n")]); - - let rejections = [ - ( - json!({ "did": format!("did:web:{MEMBER_HOST}"), "name": "periwinkle", "branch": "main", "patch": "3jzfcijpj2z2a" }), - StatusCode::BAD_REQUEST, - "we'll 400 an owner and a name instead of a repo DID", - ), - ( - json!({ "repo": "did:plc:limpet", "branch": "main", "patch": "3jzfcijpj2z2a" }), - StatusCode::NOT_FOUND, - "we'll 404 a repo DID that this knot doesn't host", - ), - ( - json!({ "repo": "periwinkle", "branch": "main", "patch": "3jzfcijpj2z2a" }), - StatusCode::BAD_REQUEST, - "we'll 400 a repo that isn't a DID", + assert!( + matches!( + resolve(&world, &RepoName::new("periwinkle").unwrap()), + Resolved::Ready(Some(_)) ), - ]; - for (input, want, why) in rejections { - let response = - into_response(crate::merge::merge_check(world.state(), body(input)).await); - assert_eq!(response.status(), want, "{why}"); - } - - assert_eq!( - main_tip(&world, &repo_did), - base, - "the merge check doesn't move the branch" + "Repo wasn't registered under its lowercased name" ); } } @@ -7310,10 +7152,6 @@ mod pulls { self.world.layout.open(self.repo).unwrap() } - fn tip(&self) -> Oid { - self.git().find_ref(&main_ref()).unwrap().unwrap() - } - fn set_main(&self, tip: Oid) { self.git() .update_ref(&RefUpdate::Create { @@ -7323,32 +7161,6 @@ mod pulls { .unwrap(); } - fn move_main(&self, old: Oid, new: Oid) { - self.git() - .update_ref(&RefUpdate::Update { - name: main_ref(), - old, - new, - }) - .unwrap(); - } - - fn drop_main(&self, old: Oid) { - self.git() - .update_ref(&RefUpdate::Delete { - name: main_ref(), - old, - }) - .unwrap(); - } - - fn has(&self, tip: Oid, path: &str) -> bool { - self.git() - .entry_at(tip, &knot_types::RepoPath::new(path).unwrap()) - .unwrap() - .is_some() - } - fn commit(&self, parents: &[Oid], staged: &[StagedChange], message: &str) -> Oid { let repo = self.git(); let base_tree = parents @@ -7414,10 +7226,6 @@ mod pulls { submit_patch_as(self.world, actor, self.repo, record).await } - async fn opened(&self, actor: &Actor, record: serde_json::Value) -> AtUri { - uri_of(&ok(self.open(actor, record).await)) - } - async fn refused_open(&self, record: serde_json::Value) -> serde_json::Value { failed( self.open(&self.world.stranger, record).await, @@ -7425,29 +7233,6 @@ mod pulls { ) } - async fn merge(&self, actor: &Actor, rkey: &RecordRkey) -> (StatusCode, serde_json::Value) { - on_repo( - self.world, - crate::social::merge_pull, - actor, - &nsid("org.tangled.review.mergePatch"), - self.repo, - json!({ "repo": self.repo, "patch": rkey }), - ) - .await - } - - async fn merged(&self, rkey: &RecordRkey) -> serde_json::Value { - ok(self.merge(&self.world.member, rkey).await) - } - - async fn refused_merge(&self, rkey: &RecordRkey) -> serde_json::Value { - failed( - self.merge(&self.world.member, rkey).await, - StatusCode::CONFLICT, - ) - } - async fn at(&self, uri: &AtUri) -> serde_json::Value { ok(record_at(self.world, self.repo, uri, None).await) } @@ -7617,427 +7402,15 @@ mod pulls { } #[tokio::test] - async fn merged_patch_completes_the_ticket_and_moves_the_branch() { - let (world, repo, _base, head, ticket, uri, _) = kelp_patch().await; - let pulls = Pulls::on(&world, &repo); - let tickets = Ticket::on(&world, &repo); - let merged = pulls.merged(&rkey_of(&uri)).await; - assert_eq!(merged["tip"], head.to_hex()); - assert_eq!(pulls.tip(), head); - let ticket_record = pulls.at(&ticket).await; - assert_eq!(ticket_record["value"]["state"], "completed"); - - let again = pulls.refused_merge(&rkey_of(&uri)).await; - message_contains(&again, &["already merged"]); - let from = cid_of(&pulls.at(&ticket).await); - let reopened = ok(tickets - .set_state(&world.stranger, &ticket, &from, TicketState::Closed) - .await); - assert_ne!( - cid_of(&reopened), - from, - "completed isn't terminal; the merged ticket still takes a state write" - ); - assert_eq!(pulls.at(&ticket).await["value"]["state"], CLOSED); - } - - #[tokio::test] - async fn merge_rejects_a_closed_ticket() { - let (world, repo, base, _head, ticket, uri, _) = kelp_patch().await; - let pulls = Pulls::on(&world, &repo); - let tickets = Ticket::on(&world, &repo); - let from = cid_of(&pulls.at(&ticket).await); - ok(tickets - .set_state(&world.stranger, &ticket, &from, TicketState::Closed) - .await); - let shut = pulls.refused_merge(&rkey_of(&uri)).await; - message_contains(&shut, &["is on a closed ticket"]); - assert_eq!(pulls.tip(), base); - } - - #[tokio::test] - async fn diverged_patch_replays_and_conflicts_show_the_file() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("kelp.txt", "forest"), ("rock.txt", "grey")], "base"); - let side = pulls.commit_with(&[base], &[("kelp.txt", "tide")], "side"); - pulls.set_main(side); - let touching = pulls.commit_with(&[base], &[("kelp.txt", "reef")], "touch"); - let clean = pulls.commit_with(&[base], &[("sand.txt", "warm")], "clean"); - - let (ticket, _) = pulls.ticket(&world.stranger, "clash").await; - let clash = pulls - .opened(&world.stranger, pulls.record(&ticket, base, touching)) - .await; - let refused = pulls.refused_merge(&rkey_of(&clash)).await; - message_contains(&refused, &["Merge failed due to conflicts", "kelp.txt"]); - assert_eq!( - pulls.at(&ticket).await["value"]["state"], - serde_json::json!("open"), - "a conflicted replay leaves the ticket open, so the merge can be retried" - ); - - pulls.move_main(side, base); - let merged = pulls.merged(&rkey_of(&clash)).await; - assert_ne!(merged["tip"], serde_json::json!("")); - assert_eq!( - pulls.at(&ticket).await["value"]["state"], - serde_json::json!("completed"), - "the retried merge completes the ticket once the conflict is gone" - ); - - let (ticket2, _) = pulls.ticket(&world.stranger, "neat").await; - let neat = pulls - .opened(&world.stranger, pulls.record(&ticket2, base, clean)) - .await; - let merged = pulls.merged(&rkey_of(&neat)).await; - let tip = pulls.tip(); - assert_ne!(tip, clean, "a replayed merge writes new commits"); - assert_ne!(merged["tip"], serde_json::json!("")); - assert!(pulls.has(tip, "sand.txt")); - assert!(pulls.has(tip, "rock.txt")); - } - - #[tokio::test] - async fn the_replay_merges_different_files_in_one_folder() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("reef/base.txt", "forest")], "base"); - let side = pulls.commit_with(&[base], &[("reef/target.txt", "tide")], "side"); - pulls.set_main(side); - let feature = pulls.commit_with(&[base], &[("reef/feature.txt", "warm")], "feature"); - let (ticket, _) = pulls.ticket(&world.stranger, "folder").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, feature)) - .await; - pulls.merged(&rkey_of(&uri)).await; - let tip = pulls.tip(); - ["reef/base.txt", "reef/target.txt", "reef/feature.txt"] - .iter() - .for_each(|path| assert!(pulls.has(tip, path), "{path} survives the merge")); - } - - #[tokio::test] - async fn the_replay_conflicts_on_one_file_changed_on_both_sides_of_a_folder() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("reef/kelp.txt", "forest")], "base"); - let side = pulls.commit_with(&[base], &[("reef/kelp.txt", "tide")], "side"); - pulls.set_main(side); - let touching = pulls.commit_with(&[base], &[("reef/kelp.txt", "warm")], "touch"); - let (ticket, _) = pulls.ticket(&world.stranger, "clash").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, touching)) - .await; - let refused = pulls.refused_merge(&rkey_of(&uri)).await; - message_contains( - &refused, - &["Merge failed due to conflicts", "reef/kelp.txt"], - ); - } - - #[tokio::test] - async fn the_replay_merges_different_files_in_nested_folders() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("a/b/c/base.txt", "forest")], "base"); - let side = pulls.commit_with( - &[base], - &[("a/b/c/target.txt", "tide"), ("a/b/top.txt", "tide")], - "side", - ); - pulls.set_main(side); - let feature = pulls.commit_with( - &[base], - &[("a/b/c/feature.txt", "warm"), ("a/b/d/new.txt", "warm")], - "feature", - ); - let (ticket, _) = pulls.ticket(&world.stranger, "nested").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, feature)) - .await; - pulls.merged(&rkey_of(&uri)).await; - let tip = pulls.tip(); - [ - "a/b/c/base.txt", - "a/b/c/target.txt", - "a/b/top.txt", - "a/b/c/feature.txt", - "a/b/d/new.txt", - ] - .iter() - .for_each(|path| assert!(pulls.has(tip, path), "{path} survives the merge")); - } - - #[tokio::test] - async fn the_replay_adds_a_file_to_a_folder_the_target_emptied() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with( - &[], - &[("reef/old.txt", "forest"), ("rock.txt", "grey")], - "base", - ); - let side = pulls.commit( - &[base], - &[StagedChange { - path: knot_types::RepoPath::new("reef/old.txt").unwrap(), - action: StagedAction::Remove, - }], - "empty the folder", - ); - pulls.set_main(side); - let feature = pulls.commit_with(&[base], &[("reef/new.txt", "warm")], "feature"); - let (ticket, _) = pulls.ticket(&world.stranger, "emptied").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, feature)) - .await; - pulls.merged(&rkey_of(&uri)).await; - let tip = pulls.tip(); - assert!(pulls.has(tip, "reef/new.txt")); - assert!(!pulls.has(tip, "reef/old.txt")); - assert!(pulls.has(tip, "rock.txt")); - } - - #[tokio::test] - async fn the_replay_merges_a_file_the_pull_turned_into_a_folder() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("reef", "forest"), ("rock.txt", "grey")], "base"); - let side = pulls.commit_with(&[base], &[("sand.txt", "tide")], "side"); - pulls.set_main(side); - let feature = pulls.commit( - &[base], - &[ - StagedChange { - path: knot_types::RepoPath::new("reef").unwrap(), - action: StagedAction::Remove, - }, - StagedChange { - path: knot_types::RepoPath::new("reef/kelp.txt").unwrap(), - action: StagedAction::Put { - content: b"warm".to_vec(), - kind: EntryKind::Blob, - }, - }, - ], - "file to folder", - ); - let (ticket, _) = pulls.ticket(&world.stranger, "file to folder").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, feature)) - .await; - pulls.merged(&rkey_of(&uri)).await; - let tip = pulls.tip(); - assert!(pulls.has(tip, "reef/kelp.txt")); - assert!(pulls.has(tip, "sand.txt")); - } - - #[tokio::test] - async fn the_replay_merges_a_folder_the_pull_turned_into_a_file() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with( - &[], - &[("reef/kelp.txt", "forest"), ("rock.txt", "grey")], - "base", - ); - let side = pulls.commit_with(&[base], &[("sand.txt", "tide")], "side"); - pulls.set_main(side); - let feature = pulls.commit( - &[base], - &[ - StagedChange { - path: knot_types::RepoPath::new("reef/kelp.txt").unwrap(), - action: StagedAction::Remove, - }, - StagedChange { - path: knot_types::RepoPath::new("reef").unwrap(), - action: StagedAction::Put { - content: b"warm".to_vec(), - kind: EntryKind::Blob, - }, - }, - ], - "folder to file", - ); - let (ticket, _) = pulls.ticket(&world.stranger, "folder to file").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, feature)) - .await; - pulls.merged(&rkey_of(&uri)).await; - let tip = pulls.tip(); - assert!(!pulls.has(tip, "reef/kelp.txt")); - assert!(pulls.has(tip, "reef")); - assert!(pulls.has(tip, "sand.txt")); - } - - #[tokio::test] - async fn the_replay_conflicts_on_a_file_the_target_deleted_and_the_pull_changed() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("reef/old.txt", "forest")], "base"); - let side = pulls.commit( - &[base], - &[StagedChange { - path: knot_types::RepoPath::new("reef/old.txt").unwrap(), - action: StagedAction::Remove, - }], - "delete it", - ); - pulls.set_main(side); - let feature = pulls.commit_with(&[base], &[("reef/old.txt", "warm")], "feature"); - let (ticket, _) = pulls.ticket(&world.stranger, "delete vs modify").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, feature)) - .await; - let refused = pulls.refused_merge(&rkey_of(&uri)).await; - message_contains(&refused, &["Merge failed due to conflicts", "reef/old.txt"]); - } - - #[tokio::test] - async fn boot_mends_a_rolled_back_move_and_leaves_an_advanced_branch_alone() { - let (world, repo, base, head, _ticket, uri, _) = kelp_patch().await; - let pulls = Pulls::on(&world, &repo); - pulls.merged(&rkey_of(&uri)).await; - assert_eq!(pulls.tip(), head); - - pulls.move_main(head, base); - let finished = crate::social::seal_pending_merges(&world.state).await; - assert_eq!( - finished, 1, - "the boot pass finishes the branch move the completed ticket recorded" - ); - assert_eq!(pulls.tip(), head); - - let onward = pulls.commit_with(&[head], &[("kelp.txt", "atoll")], "onward"); - pulls.move_main(head, onward); - let mended = crate::social::seal_pending_merges(&world.state).await; - assert_eq!( - mended, 0, - "a branch the merge already moved past isn't mended" - ); - assert_eq!(pulls.tip(), onward); - } - - #[tokio::test] - async fn the_pulls_author_cant_merge_it() { - let (world, repo, _, _, _ticket, uri, _) = kelp_patch().await; - let pulls = Pulls::on(&world, &repo); - let rkey = rkey_of(&uri); - for outsider in [&world.stranger, &world.passerby] { - let refused = failed(pulls.merge(outsider, &rkey).await, StatusCode::FORBIDDEN); - message_contains(&refused, &["merging belongs to"]); - } - } - - #[tokio::test] - async fn the_replay_stages_a_renamed_file_at_its_new_path() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("kelp.txt", "forest"), ("rock.txt", "grey")], "base"); - let side = pulls.commit_with(&[base], &[("rock.txt", "warm")], "side"); - pulls.set_main(side); - let moved = pulls.commit( - &[base], - &[ - StagedChange { - path: knot_types::RepoPath::new("kelp.txt").unwrap(), - action: StagedAction::Remove, - }, - StagedChange { - path: knot_types::RepoPath::new("reef.txt").unwrap(), - action: StagedAction::Put { - content: b"atoll".to_vec(), - kind: EntryKind::Blob, - }, - }, - ], - "rename kelp", - ); - let (ticket, _) = pulls.ticket(&world.stranger, "kelp").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, moved)) - .await; - pulls.merged(&rkey_of(&uri)).await; - - let git = pulls.git(); - let tip = pulls.tip(); - let expected = git - .entry_at(moved, &knot_types::RepoPath::new("reef.txt").unwrap()) - .unwrap() - .expect("the renamed file exists at its new path in the patch's own commit"); - assert_eq!( - git.entry_at(tip, &knot_types::RepoPath::new("reef.txt").unwrap()) - .unwrap() - .expect("the renamed file is at its new path after the merge") - .oid, - expected.oid, - "the renamed file keeps its own content" - ); - assert!( - !pulls.has(tip, "kelp.txt"), - "the renamed-away path doesn't survive the merge" - ); - assert!( - pulls.has(tip, "rock.txt"), - "the target branch's own change stays" - ); - } - - #[tokio::test] - async fn a_merge_commit_replays_after_its_parents() { - let (world, repo) = ticket_world().await; - let pulls = Pulls::on(&world, &repo); - let base = pulls.commit_with(&[], &[("kelp.txt", "forest"), ("rock.txt", "grey")], "base"); - let left = pulls.commit_with(&[base], &[("a.txt", "left")], "left"); - let right = pulls.commit_with(&[base], &[("b.txt", "right")], "right"); - let joined = pulls.commit_with( - &[left, right], - &[("a.txt", "left"), ("b.txt", "right")], - "join the sides", - ); - let side = pulls.commit_with(&[base], &[("rock.txt", "warm")], "side"); - pulls.set_main(side); - let (ticket, _) = pulls.ticket(&world.stranger, "join").await; - let uri = pulls - .opened(&world.stranger, pulls.record(&ticket, base, joined)) - .await; - pulls.merged(&rkey_of(&uri)).await; - let tip = pulls.tip(); - assert_ne!(tip, joined, "a replayed merge writes new commits"); - assert!(pulls.has(tip, "a.txt")); - assert!(pulls.has(tip, "b.txt")); - assert!( - pulls.has(tip, "rock.txt"), - "the target branch's own change stays" - ); - } - - #[tokio::test] - async fn a_merge_into_a_deleted_branch_identifies_the_branch() { - let (world, repo, base, _head, _ticket, uri, _) = kelp_patch().await; - let pulls = Pulls::on(&world, &repo); - pulls.drop_main(base); - let refused = pulls.refused_merge(&rkey_of(&uri)).await; - message_contains( - &refused, - &["target branch refs/heads/main was deleted before the merge began"], - ); - } - - #[tokio::test] - async fn a_ticket_targets_a_branch_and_the_merge_moves_it() { + async fn a_ticket_targets_a_branch_that_exists() { let (world, repo) = ticket_world().await; let pulls = Pulls::on(&world, &repo); let base = pulls.commit_with(&[], &[("kelp.txt", "forest")], "base"); - let head = pulls.commit_with(&[base], &[("kelp.txt", "reef")], "head"); pulls.set_main(base); - let reef = RefName::new("refs/heads/reef").unwrap(); pulls .git() .update_ref(&RefUpdate::Create { - name: reef.clone(), + name: RefName::new("refs/heads/reef").unwrap(), new: base, }) .unwrap(); @@ -8046,31 +7419,8 @@ mod pulls { let ticket = uri_of(&ok(tickets .create(&world.stranger, TicketDraft::fresh(&repo).on("reef").record) .await)); - assert_eq!( - ok(record_at(&world, &repo, &ticket, None).await)["value"]["targetBranch"], - "reef" - ); - - let patch_uri = uri_of(&ok(pulls - .open(&world.stranger, pulls.record(&ticket, base, head)) - .await)); - pulls.merged(&rkey_of(&patch_uri)).await; - assert_eq!( - pulls.git().find_ref(&reef).unwrap(), - Some(head), - "the merge moves the ticket's targetBranch" - ); - assert_eq!( - pulls.tip(), - base, - "and the default branch stays where it was" - ); - let completed = ok(record_at(&world, &repo, &ticket, None).await); - assert_eq!(completed["value"]["state"], "completed"); - assert_eq!( - completed["value"]["targetBranch"], "reef", - "completing the ticket keeps its targetBranch" - ); + let created = ok(record_at(&world, &repo, &ticket, None).await); + assert_eq!(created["value"]["targetBranch"], "reef"); for (draft, fragments) in [ ( @@ -8089,7 +7439,7 @@ mod pulls { .put( &world.stranger, &ticket, - &cid_of(&completed), + &cid_of(&created), TicketDraft::fresh(&repo).on("no-such-branch").record, ) .await, diff --git a/knot2/crates/knot-xrpc/tests/common/mod.rs b/knot2/crates/knot-xrpc/tests/common/mod.rs index e356aaf6e..c9ebc5352 100644 --- a/knot2/crates/knot-xrpc/tests/common/mod.rs +++ b/knot2/crates/knot-xrpc/tests/common/mod.rs @@ -219,7 +219,7 @@ use knot_runtime::{ }; use knot_secrets::{MasterKey, SealedStore}; use knot_types::{ - AccountDid, ActorId, AtUri, AuthorName, Did, DidRkey, Email, KnotHostname, KnotId, + AccountDid, ActorId, AtUri, Did, DidRkey, KnotHostname, KnotId, KnotServiceUrl, Nsid, ObjectFormat, Oid, OwnerDid, RecordAddress, RecordCid, RecordCollection, RecordRkey, RepoDid, RepoName, RepoRkey, ServiceDid, UnixSeconds, }; @@ -708,10 +708,6 @@ impl World { GlobalQuota::new(16), )), proxy_trust: knot_types::ProxyTrust::default(), - committer: knot_xrpc::Committer { - name: AuthorName::new("Tangled"), - email: Email::new("noreply@tangled.sh"), - }, byte_limits, budgets: Budgets::default(), git_http: Arc::new(FakeHttp::new(|_request: &HttpRequest| { diff --git a/knot2/crates/knot-xrpc/tests/projection.rs b/knot2/crates/knot-xrpc/tests/projection.rs index 34c60fb65..650055109 100644 --- a/knot2/crates/knot-xrpc/tests/projection.rs +++ b/knot2/crates/knot-xrpc/tests/projection.rs @@ -4,7 +4,7 @@ use std::collections::BTreeMap; use common::{ World, commit_file, create_patch, create_ticket, get_json, http_push, nsid, owner, post_as, - post_authed, serve_pushes, sh_git, + post_authed, serve_pushes, sh_git, ticket_collection, }; use knot_index::Resolved; use knot_record::chain; @@ -104,7 +104,7 @@ async fn the_full_lifecycle_keeps_the_mst_projection_exact() { "the read surface lists the pushed branch" ); - let (ticket, _) = create_ticket(&world, &did, "kelp", "a forest").await; + let (ticket, ticket_cid) = create_ticket(&world, &did, "kelp", "a forest").await; assert_projection(&world, &did).await; sh_git(work.path(), &["checkout", "-q", "-b", "feature"]); @@ -121,28 +121,34 @@ async fn the_full_lifecycle_keeps_the_mst_projection_exact() { assert!(ok, "feature push failed: {detail}"); assert_projection(&world, &did).await; - let (patch, _) = create_patch(&world, &did, &ticket, &main_sha, &feature_sha).await; + create_patch(&world, &did, &ticket, &main_sha, &feature_sha).await; assert_projection(&world, &did).await; - let (status, merged) = post_as( + let collection = ticket_collection(); + let (status, completed) = post_as( &world, - &nsid("org.tangled.review.mergePatch"), + &nsid("com.atproto.repo.putRecord"), &owner(), &ServiceDid::new(did.as_str()).unwrap(), - serde_json::json!({ "repo": did, "patch": patch }), + serde_json::json!({ + "repo": did, + "collection": &collection, + "rkey": ticket, + "swapRecord": ticket_cid.to_string(), + "record": { + "$type": &collection, + "repo": did, + "title": "kelp", + "body": "a forest", + "state": "completed", + "createdAt": "2026-09-11T00:00:00Z" + } + }), ) .await; - assert_eq!(status, http::StatusCode::OK, "mergePatch failed: {merged}"); - assert_eq!(merged["tip"], feature_sha.to_hex()); + assert_eq!(status, http::StatusCode::OK, "completing failed: {completed}"); assert_projection(&world, &did).await; - let bare = world.layout.repo_path(&did).unwrap(); - assert_eq!( - sh_git(&bare, &["rev-parse", "refs/heads/main"]), - feature_sha.to_hex(), - "the merge moved main to the feature tip" - ); - let (status, deleted) = post_authed( &world, &nsid("org.tangled.git.deleteBranch"), diff --git a/knot2/crates/knot-xrpc/tests/reads.rs b/knot2/crates/knot-xrpc/tests/reads.rs index 97854deb7..7549f445c 100644 --- a/knot2/crates/knot-xrpc/tests/reads.rs +++ b/knot2/crates/knot-xrpc/tests/reads.rs @@ -16,7 +16,7 @@ use knot_xrpc::{ArchiveLimit, ResponseLimit}; use common::{ World, archive_full, assert_immutable_round_trip, assert_post_rejected, assert_warming, commit_file, empty_repo, get, get_error, get_json, get_with_headers, nsid, owner, post_as, - post_authed, post_json, ref_list, repo_dids, seeded, seeded_feature_branch, sh_git, sh_git_at, + post_authed, ref_list, repo_dids, seeded, sh_git, sh_git_at, }; #[tokio::test] @@ -1395,139 +1395,6 @@ async fn languages_omit_files_with_zero_size() { assert!(value.get("totalFiles").is_none()); } -#[tokio::test] -async fn merge_check_reports_clean_and_conflicted_against_a_branch() { - let world = World::new(); - let (did, main_sha, feature_sha) = - seeded_feature_branch(&world, &RepoName::new("periwinkle").unwrap()); - common::enable_chain(&world, &did); - let (ticket, _) = common::create_ticket(&world, &did, "kelp", "a forest").await; - let (patch, _) = common::create_patch(&world, &did, &ticket, &main_sha, &feature_sha).await; - - let clean = post_json( - &world, - "/xrpc/org.tangled.git.mergeCheck", - serde_json::json!({ "repo": did, "patch": patch, "branch": "main" }), - ) - .await; - assert_eq!(clean.0, StatusCode::OK, "mergeCheck failed: {}", clean.1); - assert_eq!(clean.1["isConflicted"], false); - assert_eq!(clean.1["conflicts"], serde_json::json!([])); - - // a side branch that also edits reef.txt conflicts with the patch's own edit - let applied = tempfile::tempdir().unwrap(); - let bare = world.layout.repo_path(&did).unwrap(); - sh_git( - applied.path(), - &["clone", "-q", bare.to_str().unwrap(), "."], - ); - sh_git( - applied.path(), - &["checkout", "-q", "-b", "side", &main_sha.to_hex()], - ); - commit_file( - applied.path(), - "reef.txt", - b"someone else got here first\n", - "side edit", - "2026-06-01T12:40:00+02:00", - ); - sh_git( - applied.path(), - &["push", "-q", bare.to_str().unwrap(), "refs/heads/side"], - ); - - let conflicted = post_json( - &world, - "/xrpc/org.tangled.git.mergeCheck", - serde_json::json!({ "repo": did, "patch": patch, "branch": "side" }), - ) - .await; - assert_eq!( - conflicted.0, - StatusCode::OK, - "mergeCheck reports a conflict, it doesn't error: {}", - conflicted.1 - ); - assert_eq!(conflicted.1["isConflicted"], true); - assert_eq!(conflicted.1["conflicts"][0]["filename"], "reef.txt"); - assert_eq!(conflicted.1["message"], "patch can't be applied cleanly"); -} - -#[tokio::test] -async fn a_binary_patch_round_trips_through_merge_check_and_merge_patch() { - let world = World::new(); - let (did, main_sha, feature_sha) = - seeded_feature_branch(&world, &RepoName::new("periwinkle").unwrap()); - common::enable_chain(&world, &did); - let (ticket, _) = common::create_ticket(&world, &did, "kelp", "a forest").await; - let (patch, _) = common::create_patch(&world, &did, &ticket, &main_sha, &feature_sha).await; - - let diff = get_json( - &world, - &format!("/xrpc/org.tangled.git.getDiff?repo={did}&base={main_sha}&head={feature_sha}"), - ) - .await; - let binaries = diff["diffs"] - .as_array() - .unwrap() - .iter() - .filter(|entry| !entry["hasByteChanges"].is_null()) - .map(|entry| { - [ - entry["lhsSrc"]["path"].as_str().unwrap(), - entry["rhsSrc"]["path"].as_str().unwrap(), - ] - .into_iter() - .find(|path| !path.is_empty()) - .unwrap() - .to_string() - }) - .collect::>(); - assert_eq!( - binaries, - ["anchor.bin", "deep water.bin", "pearl.bin", "shell.bin"], - "the diff lists every entry whose payload is binary" - ); - - let (status, check) = post_json( - &world, - "/xrpc/org.tangled.git.mergeCheck", - serde_json::json!({ "repo": did, "patch": patch, "branch": "main" }), - ) - .await; - assert_eq!(status, StatusCode::OK, "mergeCheck failed: {check}"); - assert_eq!(check["isConflicted"], false); - - let (status, merged) = post_as( - &world, - &nsid("org.tangled.review.mergePatch"), - &owner(), - &knot_types::ServiceDid::new(did.as_str()).unwrap(), - serde_json::json!({ "repo": did, "patch": patch }), - ) - .await; - assert_eq!(status, StatusCode::OK, "mergePatch failed: {merged}"); - assert_eq!(merged["tip"], feature_sha.to_hex()); - - let bare = world.layout.repo_path(&did).unwrap(); - assert_eq!( - sh_git(&bare, &["rev-parse", "refs/heads/main^{tree}"]), - sh_git(&bare, &["rev-parse", &format!("{feature_sha}^{{tree}}")]), - "landing the patch rebuilds the feature tree byte for byte" - ); - let tree = sh_git(&bare, &["ls-tree", "refs/heads/main"]); - assert!( - tree.lines() - .any(|line| line.starts_with("100755") && line.ends_with("hull.bin")), - "the exec bit survived the round trip: {tree}" - ); - assert!( - !tree.contains("anchor.bin") && tree.contains("deep water.bin"), - "the patch's delete and add both reached the tree: {tree}" - ); -} - #[tokio::test] async fn list_records_serves_the_grant_collections() { let world = World::administered(); diff --git a/knot2/crates/knot-xrpc/tests/spindle_outbox.rs b/knot2/crates/knot-xrpc/tests/spindle_outbox.rs index 9f489cd8c..1dffa18e7 100644 --- a/knot2/crates/knot-xrpc/tests/spindle_outbox.rs +++ b/knot2/crates/knot-xrpc/tests/spindle_outbox.rs @@ -1,10 +1,7 @@ mod common; -use common::{ - World, commit_file, create_patch, create_ticket, http_push, nsid, owner, post_as, post_authed, - serve_pushes, sh_git, -}; -use knot_types::{Oid, RepoDid, ServiceDid, UnixSeconds}; +use common::{World, commit_file, http_push, nsid, owner, post_authed, serve_pushes, sh_git}; +use knot_types::{RepoDid, UnixSeconds}; const SPINDLE: &str = "did:web:spindle.test"; @@ -91,38 +88,3 @@ async fn a_push_is_queued_for_the_spindle_even_when_it_asks_to_skip_ci() { ] ); } - -#[tokio::test] -async fn a_merge_seal_is_queued_for_the_spindle() { - let (world, did, work) = seeded().await; - let main_sha = Oid::from_hex(&sh_git(work.path(), &["rev-parse", "HEAD"])).unwrap(); - sh_git(work.path(), &["checkout", "-q", "-b", "feature"]); - commit_file( - work.path(), - "reef.rs", - b"fn reef() {}\n", - "reef", - "2026-06-01T12:31:00+02:00", - ); - let feature_sha = Oid::from_hex(&sh_git(work.path(), &["rev-parse", "HEAD"])).unwrap(); - let addr = serve_pushes(&world).await; - let (ok, detail) = http_push(work.path(), addr, &did, &owner(), &["main", "feature"]).await; - assert!(ok, "push failed: {detail}"); - let (ticket, _) = create_ticket(&world, &did, "kelp", "a forest").await; - let (patch, _) = create_patch(&world, &did, &ticket, &main_sha, &feature_sha).await; - - let (status, merged) = post_as( - &world, - &nsid("org.tangled.review.mergePatch"), - &owner(), - &ServiceDid::new(did.as_str()).unwrap(), - serde_json::json!({ "repo": did, "patch": patch }), - ) - .await; - assert_eq!(status, http::StatusCode::OK, "mergePatch failed: {merged}"); - - let queued = world.state.outbox.queued(); - let body = &queued.last().unwrap().1; - assert_eq!(body["event"]["ref"], "refs/heads/main"); - assert_eq!(body["event"]["newSha"], feature_sha.to_hex()); -} diff --git a/knot2/example.toml b/knot2/example.toml index c0a3ae3ec..0df7bfb25 100644 --- a/knot2/example.toml +++ b/knot2/example.toml @@ -132,17 +132,6 @@ #write_refill_ms = 500 [git] -# Committer identity stamped on merge commits the knot creates. -# -# Can also be specified via environment variable `KNOT_GIT_USER_NAME`. -# -# Default value: "Tangled" -#user_name = "Tangled" - -# Can also be specified via environment variable `KNOT_GIT_USER_EMAIL`. -# Default value: "noreply@tangled.sh" -#user_email = "noreply@tangled.sh" - # Can also be specified via environment variable `KNOT_GIT_OBJECT_FORMAT`. # Default value: "sha256" #object_format = "sha256" @@ -217,8 +206,8 @@ # Default value: 1000 #languages_budget_ms = 1000 -# Body limit for the merge and mergeCheck procedures, whose patch payloads -# routinely exceed the general XRPC body limit. +# Body limit for the seedRepo procedure, whose record batches routinely +# exceed the general XRPC body limit. # # Can also be specified via environment variable `KNOT_XRPC_MAX_PATCH_BYTES`. # diff --git a/lexicons/org/tangled/git/mergeCheck.json b/lexicons/org/tangled/git/mergeCheck.json deleted file mode 100644 index 9e0de5799..000000000 --- a/lexicons/org/tangled/git/mergeCheck.json +++ /dev/null @@ -1,85 +0,0 @@ -{ - "lexicon": 1, - "id": "org.tangled.git.mergeCheck", - "defs": { - "main": { - "type": "procedure", - "description": "Check whether a patch record merges cleanly into its target branch, without writing anything.", - "input": { - "encoding": "application/json", - "schema": { - "type": "object", - "required": ["repo", "patch"], - "properties": { - "repo": { - "type": "string", - "format": "did", - "description": "DID of the repository" - }, - "patch": { - "type": "string", - "format": "cid", - "description": "Record key of the org.tangled.review.patch record to check" - }, - "branch": { - "type": "string", - "description": "Target branch. Defaults to the ticket's targetBranch, falling back to the repo's default branch." - } - } - } - }, - "output": { - "encoding": "application/json", - "schema": { - "type": "object", - "required": ["isConflicted", "conflicts"], - "properties": { - "isConflicted": { - "type": "boolean", - "description": "Whether the merge has unresolved conflicts" - }, - "conflicts": { - "type": "array", - "items": { - "type": "ref", - "ref": "#conflict" - } - }, - "message": { - "type": "string", - "description": "Additional detail, e.g. unrelated histories" - } - } - } - }, - "errors": [ - { - "name": "RepoNotFound", - "description": "Repo is not registered on this knot" - }, - { - "name": "PatchNotFound", - "description": "No org.tangled.review.patch record exists under the given key" - }, - { - "name": "InvalidRequest", - "description": "Invalid request parameters" - } - ] - }, - "conflict": { - "type": "object", - "required": ["filename", "reason"], - "properties": { - "filename": { - "type": "string", - "description": "Path of the conflicted file" - }, - "reason": { - "type": "string", - "description": "Why the file conflicts" - } - } - } - } -} diff --git a/lexicons/org/tangled/review/mergePatch.json b/lexicons/org/tangled/review/mergePatch.json deleted file mode 100644 index 48e3ac0c2..000000000 --- a/lexicons/org/tangled/review/mergePatch.json +++ /dev/null @@ -1,72 +0,0 @@ -{ - "lexicon": 1, - "id": "org.tangled.review.mergePatch", - "defs": { - "main": { - "type": "procedure", - "description": "Merge a patch into its target branch. The merged status is written in the same critical section as the branch update.", - "input": { - "encoding": "application/json", - "schema": { - "type": "object", - "required": ["repo", "patch"], - "properties": { - "repo": { - "type": "string", - "format": "did", - "description": "DID of the repository" - }, - "patch": { - "type": "string", - "format": "cid", - "description": "Record key of the org.tangled.review.patch record to merge" - } - } - } - }, - "output": { - "encoding": "application/json", - "schema": { - "type": "object", - "required": ["tip", "merged"], - "properties": { - "tip": { - "type": "string", - "description": "Commit Object ID the target branch ended on; the current tip when no move was needed" - }, - "merged": { - "type": "array", - "items": { - "type": "string", - "format": "at-uri" - }, - "description": "Patch records merged by this call" - } - } - } - }, - "errors": [ - { - "name": "MergeConflict", - "description": "The merge has unresolved conflicts" - }, - { - "name": "ClosedTicket", - "description": "The patch's ticket is closed" - }, - { - "name": "AlreadyMerged", - "description": "The patch is already merged" - }, - { - "name": "BranchMoved", - "description": "The target branch kept moving during the merge; a boot pass finishes the move once the branch is back at the old tip" - }, - { - "name": "NoSuchBranch", - "description": "The target branch doesn't exist" - } - ] - } - } -}