Something went wrong. Try again.
atproto git client
Something went wrong. Try again.
Rust
1234567891011121314151617181920212223242526272829303132333435363738394041424344454647484950515253545556575859606162636465666768697071727374757677787980818283848586878889909192939495969798991001011021031041051061071081091101111121131141151161171181191201211221231241251261271281291301311321331341351361371381391401411421431441451461471481491501511521531541551561571581591601611621631641651661671681691701711721731741751761771781791801811821831841851861871881891901911921931941951961971981992002012022032042052062072082092102112122132142152162172182192202212222232242252262272282292302312322332342352362372382392402412422432442452462472482492502512522532542552562572582592602612622632642652662672682692702712722732742752762772782792802812822832842852862872882892902912922932942952962972982993003013023033043053063073083093103113123133143153163173183193203213223233243253263273283293303313323333343353363373383393403413423433443453463473483493503513523533543553563573583593603613623633643653663673683693703713723733743753763773783793803813823833843853863873883893903913923933943953963973983994004014024034044054064074084094104114124134144154164174184194204214224234244254264274284294304314324334344354364374384394404414424434444454464474484494504514524534544554564574584594604614624634644654664674684694704714724734744754764774784794804814824834844854864874884894904914924934944954964974984995005015025035045055065075085095105115125135145155165175185195205215225235245255265275285295305315325335345355365375385395405415425435445455465475485495505515525535545555565575585595605615625635645655665675685695705715725735745755765775785795805815825835845855865875885895905915925935945955965975985996006016026036046056066076086096106116126136146156166176186196206216226236246256266276286296306316326336346356366376386396406416426436446456466476486496506516526536546556566576586596606616626636646656666676686696706716726736746756766776786796806816826836846856866876886896906916926936946956966976986997007017027037047057067077087097107117127137147157167177187197207217227237247257267277287297307317327337347357367377387397407417427437447457467477487497507517527537547557567577587597607617627637647657667677687697707717727737747757767777787797807817827837847857867877887897907917927937947957967977987998008018028038048058068078088098108118128138148158168178188198208218228238248258268278288298308318328338348358368378388398408418428438448458468478488498508518528538548558568578588598608618628638648658668678688698708718728738748758768778788798808818828838848858868878888898908918928938948958968978988999009019029039049059069079089099109119129139149159169179189199209219229239249259269279289299309319329339349359369379389399409419429439449459469479489499509519529539549559569579589599609619629639649659669679689699709719729739749759769779789799809819829839849859869879889899909919929939949959969979989991000100110021003100410051006100710081009101010111012101310141015101610171018101910201021102210231024102510261027102810291030103110321033103410351036103710381039104010411042104310441045104610471048104910501051105210531054105510561057105810591060106110621063106410651066106710681069107010711072107310741075107610771078107910801081108210831084108510861087108810891090109110921093109410951096109710981099110011011102110311041105110611071108110911101111111211131114111511161117111811191120112111221123112411251126112711281129113011311132113311341135113611371138113911401141114211431144114511461147114811491150115111521153115411551156115711581159116011611162116311641165116611671168116911701171117211731174117511761177117811791180118111821183118411851186118711881189119011911192119311941195119611971198119912001201120212031204120512061207120812091210121112121213121412151216121712181219122012211222122312241225122612271228122912301231123212331234123512361237123812391240124112421243124412451246124712481249125012511252125312541255125612571258125912601261126212631264126512661267126812691270127112721273127412751276127712781279128012811282128312841285128612871288128912901291129212931294129512961297129812991300130113021303130413051306130713081309131013111312131313141315131613171318131913201321132213231324132513261327132813291330133113321333133413351336133713381339134013411342134313441345134613471348134913501351135213531354135513561357135813591360136113621363136413651366136713681369137013711372137313741375137613771378137913801381138213831384138513861387138813891390139113921393139413951396139713981399140014011402140314041405140614071408140914101411141214131414141514161417141814191420142114221423142414251426142714281429143014311432143314341435143614371438143914401441144214431444144514461447144814491450145114521453145414551456145714581459146014611462146314641465146614671468146914701471147214731474147514761477147814791480148114821483148414851486148714881489149014911492149314941495149614971498149915001501150215031504150515061507150815091510151115121513151415151516151715181519152015211522152315241525152615271528152915301531153215331534153515361537153815391540154115421543154415451546154715481549155015511552155315541555155615571558155915601561156215631564156515661567156815691570157115721573157415751576157715781579158015811582158315841585158615871588158915901591159215931594159515961597159815991600160116021603160416051606160716081609161016111612161316141615161616171618161916201621162216231624162516261627162816291630163116321633163416351636163716381639164016411642164316441645164616471648164916501651165216531654165516561657165816591660166116621663166416651666166716681669167016711672167316741675167616771678167916801681168216831684168516861687168816891690169116921693169416951696169716981699170017011702170317041705170617071708170917101711171217131714171517161717171817191720172117221723172417251726172717281729173017311732173317341735173617371738173917401741174217431744174517461747174817491750175117521753175417551756175717581759176017611762176317641765176617671768176917701771177217731774177517761777177817791780178117821783178417851786178717881789179017911792179317941795179617971798179918001801180218031804180518061807180818091810181118121813181418151816181718181819182018211822182318241825182618271828182918301831183218331834183518361837183818391840184118421843184418451846184718481849185018511852185318541855185618571858185918601861186218631864186518661867186818691870187118721873187418751876187718781879188018811882188318841885188618871888188918901891189218931894189518961897189818991900190119021903190419051906190719081909191019111912191319141915191619171918191919201921192219231924192519261927192819291930193119321933193419351936193719381939194019411942194319441945194619471948194919501951195219531954195519561957195819591960196119621963196419651966196719681969197019711972197319741975197619771978197919801981198219831984198519861987198819891990199119921993199419951996199719981999200020012002200320042005200620072008200920102011201220132014201520162017201820192020202120222023202420252026202720282029203020312032203320342035203620372038203920402041204220432044204520462047204820492050205120522053205420552056205720582059206020612062206320642065206620672068206920702071207220732074207520762077207820792080208120822083208420852086208720882089209020912092209320942095209620972098209921002101210221032104210521062107210821092110211121122113211421152116211721182119212021212122212321242125212621272128212921302131213221332134213521362137213821392140214121422143214421452146214721482149215021512152215321542155215621572158215921602161216221632164216521662167216821692170217121722173217421752176217721782179218021812182218321842185218621872188218921902191219221932194219521962197219821992200220122022203220422052206220722082209221022112212221322142215221622172218221922202221222222232224222522262227222822292230223122322233223422352236223722382239224022412242224322442245224622472248224922502251225222532254225522562257225822592260226122622263226422652266226722682269227022712272227322742275227622772278227922802281228222832284228522862287228822892290229122922293229422952296229722982299230023012302230323042305230623072308230923102311231223132314231523162317231823192320232123222323232423252326232723282329233023312332233323342335233623372338233923402341234223432344234523462347234823492350235123522353235423552356235723582359236023612362236323642365236623672368236923702371237223732374237523762377237823792380238123822383238423852386238723882389239023912392239323942395239623972398239924002401240224032404240524062407240824092410241124122413241424152416241724182419242024212422242324242425242624272428242924302431243224332434243524362437243824392440244124422443244424452446244724482449245024512452245324542455245624572458245924602461246224632464246524662467246824692470247124722473247424752476247724782479248024812482248324842485248624872488248924902491249224932494249524962497249824992500250125022503250425052506250725082509251025112512251325142515251625172518251925202521252225232524252525262527252825292530253125322533253425352536253725382539254025412542254325442545254625472548254925502551255225532554255525562557255825592560256125622563256425652566256725682569257025712572257325742575257625772578257925802581258225832584258525862587258825892590259125922593259425952596259725982599260026012602260326042605260626072608260926102611261226132614261526162617261826192620262126222623262426252626262726282629263026312632263326342635263626372638263926402641264226432644264526462647264826492650265126522653265426552656265726582659266026612662266326642665266626672668266926702671267226732674267526762677267826792680268126822683268426852686268726882689269026912692269326942695269626972698269927002701270227032704270527062707270827092710271127122713271427152716271727182719272027212722272327242725272627272728272927302731273227332734273527362737273827392740274127422743274427452746274727482749275027512752275327542755275627572758275927602761276227632764276527662767276827692770277127722773277427752776277727782779278027812782278327842785278627872788278927902791279227932794279527962797279827992800280128022803280428052806280728082809281028112812281328142815281628172818281928202821282228232824282528262827282828292830283128322833283428352836283728382839284028412842284328442845284628472848284928502851285228532854285528562857285828592860286128622863286428652866286728682869287028712872287328742875287628772878287928802881288228832884288528862887288828892890289128922893289428952896289728982899290029012902290329042905290629072908290929102911291229132914291529162917291829192920292129222923292429252926292729282929293029312932293329342935293629372938293929402941294229432944294529462947294829492950295129522953295429552956295729582959296029612962296329642965296629672968296929702971297229732974297529762977297829792980298129822983298429852986298729882989299029912992299329942995299629972998299930003001300230033004300530063007300830093010301130123013301430153016301730183019302030213022302330243025302630273028302930303031303230333034303530363037303830393040304130423043304430453046304730483049305030513052305330543055305630573058305930603061306230633064306530663067306830693070307130723073307430753076307730783079308030813082308330843085308630873088308930903091309230933094309530963097309830993100310131023103310431053106310731083109311031113112311331143115311631173118311931203121312231233124312531263127312831293130313131323133313431353136313731383139314031413142314331443145314631473148314931503151315231533154315531563157315831593160316131623163316431653166316731683169317031713172317331743175317631773178317931803181318231833184318531863187318831893190319131923193319431953196319731983199320032013202320332043205320632073208320932103211321232133214321532163217321832193220322132223223322432253226322732283229323032313232323332343235323632373238323932403241324232433244324532463247324832493250325132523253325432553256325732583259326032613262326332643265326632673268326932703271327232733274327532763277327832793280328132823283328432853286328732883289329032913292329332943295329632973298329933003301330233033304330533063307330833093310331133123313331433153316331733183319332033213322332333243325332633273328332933303331333233333334333533363337333833393340334133423343334433453346334733483349335033513352335333543355335633573358335933603361336233633364336533663367336833693370337133723373337433753376337733783379338033813382338333843385338633873388338933903391339233933394339533963397339833993400340134023403340434053406340734083409341034113412341334143415341634173418341934203421342234233424342534263427342834293430343134323433343434353436343734383439344034413442344334443445344634473448344934503451345234533454345534563457345834593460346134623463346434653466346734683469347034713472347334743475347634773478347934803481348234833484348534863487348834893490349134923493349434953496349734983499350035013502350335043505350635073508350935103511351235133514351535163517351835193520352135223523352435253526352735283529353035313532353335343535353635373538353935403541354235433544354535463547354835493550355135523553355435553556355735583559356035613562356335643565356635673568356935703571357235733574357535763577357835793580358135823583358435853586358735883589359035913592359335943595359635973598359936003601360236033604360536063607360836093610361136123613361436153616361736183619362036213622362336243625362636273628362936303631363236333634363536363637363836393640364136423643364436453646364736483649365036513652365336543655365636573658365936603661366236633664366536663667366836693670367136723673367436753676367736783679368036813682368336843685368636873688368936903691369236933694369536963697369836993700370137023703370437053706370737083709371037113712371337143715371637173718371937203721372237233724372537263727372837293730373137323733373437353736373737383739374037413742374337443745374637473748374937503751375237533754375537563757375837593760376137623763376437653766376737683769377037713772377337743775377637773778377937803781378237833784378537863787378837893790379137923793379437953796379737983799380038013802380338043805380638073808380938103811381238133814381538163817381838193820382138223823382438253826382738283829383038313832383338343835383638373838383938403841384238433844384538463847384838493850385138523853385438553856385738583859386038613862386338643865386638673868386938703871//! Changing a pull request: `pr create`, `resubmit`, `edit`, `close`,//! `reopen` and `comment`.//!//! The write half of [`crate::cmd::pr`], and the half where a mistake is not a//! wrong line of output but a record in somebody's repository. Every command//! here settles which account it is acting as before it reads anything and//! announces it, because that decision picks both whose PDS is written to and//! whether the write will be honoured; all of them take `--dry-run`, and the//! dry run is expected to answer "from whom?" as well as "what?".//!//! Three things recur, and they are the reason this is one module rather than//! one file per command://!//! - **Naming a pull is harder than it looks.** Tangled's own URLs identify a//! pull by a number that exists only in an appview's database and is in no//! record and no XRPC response, so [`classify_pull_ref`] tells one apart//! from a record key and [`resolve_pull_ref`] spends a request translating//! it, through the same appview scrape [`crate::cmd::pr::review`] uses. It still//! refuses a handle in an at-uri authority: handles change hands, and this//! string decides whose pull is about to be closed.//! - **Read-modify-write is a race.** `pr resubmit` appends a round and//! `pr edit` rewrites a title; both read the record, change it and put it//! back, and without a precondition two of them running at once both report//! success while one round quietly disappears. [`put_pull`] therefore goes//! through [`crate::clients::atproto::record::put`], which sends the CID it read as//! `swapRecord`.//! - **State is a log, not a field.** A pull's state is whatever the newest//! `sh.tangled.repo.pull.status` record says, so `pr reopen` appends an//! `open` record rather than deleting a `closed` one — see [`state_of`] for//! the ordering rule, and [`crate::model::record::Standing`] for whose records the indexers will//! actually honour.//!//! `pr comment` lives here too, and [`crate::cmd::pr`]'s own documentation says//! why. Reading a pull rather than writing one is [`super::read`]; the patch//! itself is [`crate::cmd::pr::review`].
use super::read::target_repo_did;use crate::clients::git::patch as gitpatch;use crate::clients::git::run as git;use crate::clients::tangled::resolve;use crate::cmd::auth;use crate::lexicon::tangled::{PullState, markdown_plain, markdown_with_blobs};use crate::model::record::{Standing, standing_of};use crate::term::column::ellipsize;use anyhow::{Context, Result, bail};use jacquard::client::AgentSessionExt;use jacquard::types::string::{AtUri, Cid, Datetime};use tangled_lexicon::LexiconSchema;use tangled_lexicon::com_atproto::repo::strong_ref::StrongRef;use tangled_lexicon::sh_tangled::feed::comment::Comment as FeedComment;use tangled_lexicon::sh_tangled::repo::pull::status::{Status as PullStatus, StatusStatus};use tangled_lexicon::sh_tangled::repo::pull::{Pull, Round, Source};
/// A record key as the write path wants it, from the module that owns the/// write path.use crate::clients::atproto::record::{Key, gzip, upload_patch_blob};
/// Tangled's website, where every `view:` link and `url` field points. The/// address and its override are [`crate::clients::endpoints`]'s; this is the/// short spelling the URL builders below use.fn appview() -> String { crate::clients::endpoints::appview()}
#[derive(clap::Args, Debug)]pub(crate) struct CreateArgs { /// PR title (defaults to the subject of the branch's first commit) #[arg(short, long)] pub title: Option<String>, /// PR description; local image paths in it are uploaded and embedded #[arg(short, long)] pub body: Option<String>, /// Read the description from a file (local image paths in it are /// uploaded and embedded); `-` reads stdin #[arg(long, conflicts_with = "body")] pub body_file: Option<String>, /// Target branch on the upstream repo (defaults to its default branch) #[arg(long)] pub target: Option<String>, /// Git remote pointing at the target repo #[arg(long, default_value = "origin")] pub remote: String, /// Open the pull from the patch alone: don't push the branch, and record /// no source on the pull #[arg(long)] pub patch_only: bool, /// Build and describe the PR without sending anything #[arg(long)] pub dry_run: bool, /// Print one JSON object describing what was written (or, with /// --dry-run, what would be) instead of the summary lines #[arg(long)] pub json: bool,}
// ---------------------------------------------------------------------------// `--json` for the commands that write// ---------------------------------------------------------------------------//// A read command's `--json` answers "what is there"; these answer "what did// I just do", which is a different contract and has one field the read// shapes do not: `dry_run`. It is on every one of them, always, and it is// what a caller checks rather than remembering which flags it passed —// a `--dry-run` that printed nothing distinguishable from a real write is// how a script convinces itself it has submitted something.//// Under `--dry-run` the identifiers the write would mint (`uri`, and the// `url` derived from it) are `null` rather than guessed at: the record key// is the PDS's to choose, and inventing one would be the single most// misleading thing this output could do.//// The lines these commands print as a pre-flight summary — target, source,// patch size, title — are not notes; they are the payload, and every one of// them is a field below. So `--json` does not move them to stderr the way it// moves `acting as …`; it prints them once, as data.
/// What `pr create` did, or would have done.#[derive(serde::Serialize, Debug, PartialEq)]pub(super) struct CreatedJson { pub dry_run: bool, /// The pull record's at-uri, `null` on a dry run. pub uri: Option<String>, /// Its page, `null` on a dry run — it is derived from the at-uri, and /// on a real write from the appview's number for it when that resolves. pub url: Option<String>, pub title: String, pub repo_did: String, pub target_branch: String, /// The branch the patch came off. Local information either way — under /// `--patch-only` it is *not* on the record, which is what /// `source_recorded` says. pub source_branch: String, /// Whether the pull carries `source: {branch}`. False under /// `--patch-only`, and then `pr view` on this branch will not find this /// pull, because there is nothing on the record tying the two together. pub source_recorded: bool, /// Whether the branch was pushed to `remote` before the record was /// written. The claim `source_recorded` makes is only true because of /// this, so the two move together. pub pushed: bool, pub commits: usize, pub patch_bytes: usize, pub patch_gzip_bytes: usize, /// Local images the body embeds, which are uploaded to the PDS and the /// paths rewritten. Empty when there are none. pub images: Vec<crate::cmd::images::ImageJson>,}
/// What `pr resubmit` did, or would have done.#[derive(serde::Serialize, Debug, PartialEq)]pub(super) struct ResubmittedJson { pub dry_run: bool, pub uri: Option<String>, pub url: Option<String>, pub rkey: String, pub title: String, pub repo_did: String, pub target_branch: String, pub source_branch: String, /// Whether the pull being appended to carries `source: {branch}`. Read /// off the record, never off a flag: a round does not change a pull's /// shape, so this says which shape was found rather than which one was /// asked for. pub source_recorded: bool, /// Whether the branch was pushed to `remote` before the round was /// written. True exactly when `source_recorded` is — the claim on the /// record is what obliges the push, and a patch-based pull makes none. pub pushed: bool, pub commits: usize, pub patch_bytes: usize, pub patch_gzip_bytes: usize, /// Rounds the record held before this run, and what it would hold after. /// Both, because a round is appended and neither number alone says that. pub rounds_before: usize, pub rounds_after: usize,}
/// What `pr close` and `pr reopen` did, or would have done.#[derive(serde::Serialize, Debug, PartialEq)]pub(super) struct StateChangeJson { pub dry_run: bool, /// The *status* record written, `null` on a dry run and also when the /// pull already had the wanted state — state is a log of records, and /// nothing is appended to it for a no-op. pub uri: Option<String>, pub url: Option<String>, pub pull_uri: String, pub rkey: String, pub title: String, pub author_did: String, pub state_before: String, pub state_after: String, /// `false` when the pull already read the wanted state. The command /// exits 0 either way, so this is the field that tells them apart. pub changed: bool,}
/// What `pr edit` did, or would have done.#[derive(serde::Serialize, Debug, PartialEq)]pub(super) struct EditedJson { pub dry_run: bool, pub uri: Option<String>, pub url: Option<String>, pub rkey: String, /// The title as it stands after the edit — the new one when `--title` /// changed it, the old one otherwise. pub title: String, pub title_changed: bool, pub body_changed: bool, /// `false` when the record already said what was asked for; nothing is /// written then, and the command still exits 0. pub changed: bool, /// Unchanged by an edit, and reported because it is what tells this /// apart from a `pr resubmit` in a log of writes. pub rounds: usize, pub images: Vec<crate::cmd::images::ImageJson>,}
/// What `pr comment` did, or would have done.#[derive(serde::Serialize, Debug, PartialEq)]pub(super) struct CommentedJson { pub dry_run: bool, /// The comment record's at-uri, `null` on a dry run. pub uri: Option<String>, pub url: Option<String>, pub pull_uri: String, pub pull_rkey: String, pub title: String, /// The pull's author, not the commenter — the acting account is on /// stderr, and a comment needs no permission from anyone. pub author_did: String, /// One-based, as every `--round` flag counts. pub round: usize, /// Tangled's own zero-based numbering for the same round, which is what /// its `/round/<i>` URLs use and what the record stores. pub round_index: usize, pub rounds: usize, pub body_bytes: usize, pub images: Vec<crate::cmd::images::ImageJson>,}
pub(crate) async fn create(args: CreateArgs) -> Result<()> { crate::term::jsonout::init(args.json); // Settled before the patch is even built, so that --dry-run answers // "who would this be from?" as well as "what would it contain?". let selection = crate::config::account::select().await?; selection.announce();
let branch = git::current_branch()?; let remote_url = git::remote_url(&args.remote)?;
let target_branch = match args.target { Some(t) => t, None => git::remote_default_branch(&args.remote).unwrap_or_else(|| "main".to_string()), };
let base = format!("{}/{}", args.remote, target_branch); if !git::ref_exists(&base) { crate::term::say::step!(Git, "fetching {} {}...", args.remote, target_branch); git::fetch(&args.remote, &target_branch).map_err(|e| { match target_is_gone(&args.remote, &target_branch) { true => anyhow::anyhow!( "{remote} has no branch {target_branch}\n\ --target names a branch on the repo being sent to; \ `git ls-remote --heads {remote}` lists the ones there are", remote = args.remote, ), false => e.context(format!("no local ref {base} and fetching it failed")), } })?; }
// Marks are the branch saying it means to be several pull requests, and // this command files it as one. Not a refusal — a stack is not always // what somebody wants, and marks left over from a stack that has landed // are ordinary — but the one moment where saying so costs nothing and // silence costs a pull request that has to be closed by hand. let marked = crate::cmd::stack::marks::recorded(&branch); if !marked.is_empty() { crate::term::say::note!( Git, "{branch} has {} mark(s) on it ({}), which `atgc stack create` would file as \ separate pull requests\n\ this files the whole branch as one", marked.len(), marked.join(", "), ); }
let commits = gitpatch::commit_count(&base)?; if commits == 0 { // `Usage`, for the reason exit.rs gives the whole class: the branch // this is standing on is an argument nobody typed, and the wrong one // is a command line that does not mean anything from here. return Err(crate::exit::fail( crate::exit::Exit::Usage, format!( "no commits on {branch} that aren't already on {base}\n\ atgc submits the checked-out branch; check out the one with the work" ), )); } let patch = gitpatch::format_patch(&base)?; if patch.is_empty() { bail!("empty patch for {base}..HEAD"); }
let title = match args.title { Some(t) => t, None => gitpatch::bottom_subject(&base)?, };
// For `pr create` the clear-it case `new_body` distinguishes collapses // into "no body", so the nested Option flattens away. let body = new_body(args.body, args.body_file)?.flatten(); // Scanned before any network traffic: a body that references a file that // is not there, is not an image, or is over the blob cap fails here, by // name, with nothing half-sent. let images = match body.as_deref() { Some(text) => crate::cmd::images::scan(text)?, None => crate::cmd::images::Images::none(), };
let repo = resolve::repo_ref(&remote_url).await?;
// Two warnings off one listing, both best-effort on purpose, all the way // down: neither a listing that cannot be read nor a chain that cannot be // ordered may block a create. The second case was a real brick, found by // injecting a forked chain — records the appview refuses to ingest still // sit in the PDS, and a `?` here made them a wall in front of an // unrelated flat create. match super::read::repo_rows(super::read::Source::PDS, &repo.did).await { Ok(listing) => { let rows = listing.rows; let items: Vec<&serde_json::Value> = rows.iter().map(|r| &r.item).collect(); // Not a refusal — several pulls per branch is legal — but a // branch whose pulls form a stack almost certainly wants the // stack reconciled, not a second flat pull opened beside it. let chain = super::read::for_branch(items.iter().copied(), &branch).and_then(|existing| { crate::cmd::stack::chain_containing( &items, existing["uri"].as_str().unwrap_or_default(), &crate::cmd::stack::closed_uris(&rows), ) .unwrap_or_else(|e| { crate::logging::debug::log(format!("stack warning check skipped: {e}")); None }) }); if let Some(chain) = chain { crate::term::say::warning!( Pds, "branch {branch} already has a stack of {}: this opens a \ separate flat pull beside it.\n\ `atgc stack resubmit` is how the stack itself is updated.", chain.size() ); } // A pull this branch already has open, aimed where this one is // aimed. Not a refusal either — the two above say why several // pulls per branch are legal, and this is the same rule — but // unlike them it is almost never deliberate. It is what a retry // looks like: `pr create` writes a record and then does // best-effort work after it, so a caller that treats any failure // as "nothing happened" and runs the command again opens a // second pull that is a copy of the first. Nothing said so, and // a fleet of agents retrying on a flaky network is exactly the // caller that does it. Saying it costs one line and makes the // duplicate visible in the run that created it. if let Some(existing) = open_pull_from_branch(&rows, &branch) && target_branch_of_item(existing).as_deref() == Some(target_branch.as_str()) { crate::term::say::warning!( Pds, "branch {branch} already has an open pull aimed at {target_branch}:\n\ \x20 \"{}\" ({})\n\ this opens a second one beside it. `atgc pr resubmit` appends a round\n\ to the pull that exists, which is what a retry or a new revision wants.", ellipsize( &crate::term::text::one_line( existing["value"]["title"].as_str().unwrap_or("untitled") ), 60 ), existing["uri"] .as_str() .unwrap_or_default() .rsplit('/') .next() .unwrap_or("?"), ); } // Also not a refusal, and for the same reason: targeting a // branch that is under review is a legal thing to want. It is // just the one shape of flat pull that cannot survive its own // base — see [`open_pull_from_branch`]. if let Some(other) = open_pull_from_branch(&rows, &target_branch) { crate::term::say::warning!( Pds, "target branch {target_branch} is the source of an open pull:\n\ \x20 \"{}\"\n\ Tangled merges by rebasing, so when that one lands this branch goes\n\ with it, and no round can ever be appended here again.\n\ `atgc stack create` chains dependent work instead.", ellipsize( &crate::term::text::one_line( other["value"]["title"].as_str().unwrap_or("untitled") ), 60 ), ); } } Err(e) => crate::logging::debug::log(format!("stack warning check skipped: {e}")), }
// Everything the preview lines below say, as one value the JSON branch // finishes and prints — on a dry run with no identifiers, after the // write with them. let mut report = CreatedJson { dry_run: args.dry_run, uri: None, url: None, title: title.clone(), repo_did: repo.did.clone(), target_branch: target_branch.clone(), source_branch: branch.clone(), source_recorded: !args.patch_only, pushed: false, commits, patch_bytes: patch.len(), patch_gzip_bytes: 0, images: images.listed(), };
if args.dry_run { // The local patch, which is the only one there is without sending // anything: on the default path the knot recomputes it after the // push, and the byte counts can differ. Said so rather than passed // off as the final numbers. let gzipped = gzip(&patch)?; report.patch_gzip_bytes = gzipped.len(); if args.json { return crate::term::jsonout::emit(&report); } preview_target(&report, &repo, &args.remote); preview_patch(&report, &title, &images); println!("dry run; nothing sent"); return Ok(()); }
if !args.json { preview_target(&report, &repo, &args.remote); } // The default path makes the claim true before making it. Everything // before this point was local; from here the branch is on the target's // knot and the patch is the knot's own, which is the same text the web // UI would have put in the record. let patch = match args.patch_only { true => patch, false => { let (pushed, comparison) = push_and_compare( &args.remote, &branch, &target_branch, &repo, None, "`atgc pr create --patch-only` opens the pull from the patch alone, \ with no branch on the knot.", ) .await?; report.pushed = pushed; report.commits = comparison.commits; report.patch_bytes = comparison.patch.len(); comparison.patch } }; let gzipped = gzip(&patch)?; report.patch_gzip_bytes = gzipped.len(); if !args.json { preview_patch(&report, &title, &images); }
let agent = auth::agent_for_did(&selection.did).await?; let gzip_len = gzipped.len(); let blob = upload_patch_blob( &agent, gzipped, format!("uploading patch blob ({gzip_len} bytes gzip)"), None, ) .await?;
let (body, image_blobs) = if images.is_empty() { (body, Vec::new()) } else { let text = body.as_deref().unwrap_or_default(); let (rewritten, blobs) = crate::cmd::images::upload_and_rewrite(&agent, &selection.did, text, &images).await?; (Some(rewritten), blobs) };
// Kept back from the move into the record: the appview lists a pull by // its title, so translating the at-URI below into a number needs it. let listed_title = title.clone(); let pull = Pull { title: title.into(), body: body.map(Into::into), target: crate::lexicon::tangled::pull_target(&repo.did, target_branch)?, // `source` is a claim, not a hint. `appview/models/pull.go` tells the // three shapes of pull apart by this field alone — no `source` is // patch-based, a `source` naming this repo is branch-based — and // nothing ever checks it against reality: ingest parses it and // believes it. A pull carrying `source: {branch}` therefore gets a // `/{repo}/tree/{branch}` link, a `resubmitCheck` asking the knot for // that branch's head, and a web resubmit routed through // `repo.compare`, all of which need the branch to be on the knot. // // So it is written exactly when the push above made it true, and // `--patch-only` writes no `source` at all rather than a softer // version of the same claim. No `repo` either: that is the fork-based // shape, which atgc does not implement — see TODO.md. source: (!args.patch_only).then(|| Source { branch: branch.clone().into(), repo: None, extra_data: None, }), // `pr create` opens a single flat pull; only the stack commands // chain records together. dependent_on: None, rounds: vec![Round { patch_blob: blob.into(), created_at: Datetime::now(), extra_data: None, }], // Through the merge even on a fresh record: two spellings of one // file upload the same content-addressed blob, and the list should // carry each CID once. blobs: crate::cmd::images::merge_blobs(None, image_blobs) .map(|bs| bs.into_iter().map(Into::into).collect()), created_at: Datetime::now(), // A pull atgc opens carries nothing the struct does not name; the // web UI is what sets mentions and references. mentions: None, references: None, extra_data: None, }; pull.validate() .map_err(|e| anyhow::anyhow!("{e}")) .context("pull record would be refused by the lexicon")?;
crate::logging::debug::log(format!( ">> createRecord {}\n{}", crate::lexicon::tangled::PULL_NSID, crate::logging::debug::pretty(&pull) )); let output = agent.create_record(pull, None).await.map_err(|e| { crate::logging::debug::dump_err("createRecord error", &e); anyhow::anyhow!("failed to create pull record: {e}") })?;
let uri = output.uri.to_string(); let link = crate::clients::tangled::web::pulls::pull_url_for_new_record( &repo.web_url, &uri, &listed_title, ) .await; if args.json { report.uri = Some(uri); report.url = link.numbered_url().map(str::to_string); crate::term::jsonout::emit(&report)?; if let Some(note) = link.note() { crate::term::say::note!(Index, "{note}"); } return Ok(()); } println!("created {uri}"); println!("view: {}", crate::term::hyperlink::url(link.url())); if let Some(note) = link.note() { crate::term::say::note!(Index, "{note}"); } patch_only_note(args.patch_only, &branch, &uri); Ok(())}
/// Where this is going and where it came from — the half of the summary that/// is known before anything leaves the machine.////// Printed ahead of the push so that the first output a person sees explains/// what the git progress underneath it is for, rather than arriving after it.fn preview_target(report: &CreatedJson, repo: &resolve::RepoRef, remote: &str) { // `owner/name`, linked, rather than the repo's DID: this is the first // line `pr create` prints, and a DID is the wrong thing to open with. // The `RepoRef` is already in hand here — the remote was resolved to find // the target at all — which is what makes this free, and is exactly what // the `target:` line in `pr merge` lacks. println!("target: {} branch {}", repo.linked(), report.target_branch); // The branch is named either way, because it is the branch this command // is standing on and "that is not the branch I meant" is the mistake // worth catching before a record exists. What differs is whether it is // going anywhere: a recorded source is a claim that the branch is on the // knot, and `--patch-only` declines to make it. match report.source_recorded { true => println!("source: branch {} -> {remote}", report.source_branch), false => println!( "source: branch {} (local; --patch-only records no source)", report.source_branch ), }}
/// What the record will hold. Printed after the compare, because on the/// default path the knot computes the patch and the byte counts are not the/// local ones.fn preview_patch(report: &CreatedJson, title: &str, images: &crate::cmd::images::Images) { println!( "patch: {} commit(s), {} bytes ({} gzipped)", report.commits, report.patch_bytes, report.patch_gzip_bytes ); println!("title: {title}"); for line in images.describe() { println!("image: {line}"); }}
/// The one thing `--patch-only` costs, said once, where it is still cheap to/// act on.////// `pr view` and `browse --pr` find the pull for the branch they are standing/// on through `source.branch`, and a patch-based pull has no source to match./// That is not a bug to work around here — a source is a claim about a knot,/// and this pull is deliberately not making it — but it does mean the at-uri/// printed above is the only handle on this pull from this checkout, and it/// is worth saying so while it is on the screen.fn patch_only_note(patch_only: bool, branch: &str, uri: &str) { if !patch_only { return; } crate::term::say::note!( Pds, "no source recorded, so `atgc pr view` on {branch} will not find this pull.\n\ Name it directly: `atgc pr view {uri}`" );}
/// What the round will hold, and where it lands in the record's history.////// Printed after the compare on a branch-based pull, because the patch in/// the round is then the knot's and the byte counts are not the local ones.fn resubmit_preview_patch(report: &ResubmittedJson, branch: &str) { println!( "patch: {} commit(s) from {branch}, {} bytes ({} gzipped)", report.commits, report.patch_bytes, report.patch_gzip_bytes ); println!( "rounds: {} -> {}", report.rounds_before, report.rounds_after );}
/// Push the branch, then have the target's knot compute the patch.////// The two are one step because neither is any use alone: the compare asks a/// knot to diff a branch it has to have, and pushing a branch nothing records/// is not what was asked for. Doing them in this order is what makes/// `source: {branch}` true at the moment it is written.////// The patch comes back from the knot rather than from `git format-patch`/// here, and [`crate::clients::tangled::compare`] says why: it is the same/// text the web UI would have produced, so a pull resubmitted from a browser/// and one resubmitted from atgc differ by nothing invisible./// `advice` is the caller's way out when the branch cannot be published —/// `--patch-only` for a pull not yet opened, and for a round there is none,/// because the record already made the claim. Passed in rather than written/// here: the failure is the same, and only the caller knows what to do about/// it.async fn push_and_compare( remote: &str, branch: &str, target_branch: &str, repo: &resolve::RepoRef, expected: Option<&str>, advice: &str,) -> Result<(bool, crate::clients::tangled::compare::Comparison)> { // The expected failure is no push access on the target, which is what // `advice` is for: git's own message explains a permission problem and // knows nothing about there being another way to open the pull. let pushed = crate::cmd::publish_branch(remote, branch, &git::head()?, expected, advice)?;
let knot = crate::clients::atproto::did::knot_from_did_doc(&repo.did) .await .ok_or_else(|| { anyhow::anyhow!( "no knot in {}'s DID document, so there is nothing to ask for the patch\n\ {advice}", repo.did ) })?; crate::term::say::step!(Knot, "comparing {target_branch}..{branch} on {knot}..."); let comparison = crate::clients::tangled::compare::compare(&knot, &repo.did, target_branch, branch).await?;
// The appview's own refusal, in its own words: `handleBranchBasedPull` // stops on an empty `format_patch` with "No commits between target and // source". Reachable here even though the local patch was not empty — // the knot is answering about what it has, and a push that raced with // somebody else's is the ordinary way the two disagree. if comparison.commits == 0 || comparison.patch.is_empty() { bail!( "{knot} finds no commits between {target_branch} and {branch}\n\ The branch was pushed; the knot has nothing on it that {target_branch} does not." ); } if comparison.binary_omitted { crate::term::say::warning!( Knot, "{knot} left binary payloads out of the patch, so it will not apply cleanly" ); } crate::logging::debug::log(format!( "compare {target_branch}({})..{branch}({}) merge-base {}: {} commit(s), {} bytes", comparison.rev1, comparison.rev2, comparison.merge_base, comparison.commits, comparison.patch.len() )); Ok((pushed, comparison))}
/// Where the last round left the branch, read off that round's own patch.////// `git format-patch` writes each commit's real sha into its `From <sha>`/// boundary line, and the knot's patches come from the same command/// (`knotserver/git/diff.go`), so this answers for a round written from the/// web as readily as one written here. It is the expectation the round's/// push leases against: the branch on the knot should be exactly where the/// record says the last round left it, and anywhere else means something/// took the branch somewhere this checkout cannot see.////// Never an error. A round that cannot be read — an oversized blob, a PDS/// that will not answer, a merge-tipped branch whose patch omits the tip —/// leaves the push unleased and therefore fast-forward-only, which is what/// it was before any of this. A resubmit does not fail over a lease it/// could not compute; it just declines to overwrite on a guess.async fn recorded_head(did: &str, value: &serde_json::Value, label: &str) -> Option<String> { let pds = crate::clients::atproto::did::pds_or_fail(did).await.ok()?; match crate::cmd::stack::latest_round_patch(&pds, did, value, label).await { Ok(patch) => gitpatch::head_sha(&patch), Err(e) => { crate::logging::debug::log(format!( "no lease for {label}: could not read its latest round's patch: {e:#}" )); None } }}
/// Which shape of pull a round is being appended to.////// Read off the record and nothing else. `pr create` chooses a shape once,/// with `--patch-only`, and a resubmit has no business choosing it again: a/// patch-based pull that grew a `source` on its second round would start/// claiming a branch on the knot that its first round never put there, and/// the appview believes `source` without ever checking it. So there is no/// `--patch-only` on `pr resubmit` — the record already answered.enum Shape { /// No `source`. The pull was opened from a patch alone, and every round /// is a patch alone: formatted here, with nothing pushed anywhere. PatchOnly, /// `source: {branch}` and no `source.repo` — a branch on the target repo /// itself, which is the claim `pr create` makes true by pushing. The /// round has to keep it true, so this branch is pushed again and the /// patch comes back from the knot's own compare. Branch(String),}
/// The shape of `pull`, or a refusal for the one shape atgc cannot append to.////// `source.repo` is the fork-based shape: the branch lives on somebody/// else's knot, and republishing it means service auth and push access over/// there rather than here. `pr create` deliberately does not open those (see/// TODO.md), so nothing atgc wrote can reach this — but the web UI opens/// them, and `pr resubmit` takes any pull the account owns. Refused rather/// than quietly appending a locally formatted patch to a pull whose tree/// link points at a fork this command never touched.fn shape_of(pull: &Pull) -> Result<Shape> { let Some(source) = pull.source.as_ref() else { return Ok(Shape::PatchOnly); }; if let Some(fork) = source.repo.as_ref() { // `Usage`: the account and the pull are both fine, this verb cannot // do it. The website's own resubmit can, because it runs the compare // on the fork's knot. return Err(crate::exit::fail( crate::exit::Exit::Usage, format!( "this pull's source is branch {} on fork {fork}, and atgc does not \ publish to a fork's knot\n\ append the round from the web UI, which runs the compare there", source.branch ), )); } Ok(Shape::Branch(source.branch.to_string()))}
#[derive(clap::Args, Debug)]pub(crate) struct ResubmitArgs { /// The pull request: its number, record key, at:// URI, or Tangled URL #[arg(value_name = "PULL")] pub pr: Option<String>, /// The same, as a flag, for symmetry with the other verbs #[arg(long = "pr", value_name = "PULL", conflicts_with = "pr")] pub pr_flag: Option<String>, /// Git remote pointing at the target repo #[arg(long, default_value = "origin")] pub remote: String, /// Build and describe the round without sending anything #[arg(long)] pub dry_run: bool, /// Print one JSON object describing what was written (or, with /// --dry-run, what would be) instead of the summary lines #[arg(long)] pub json: bool,}
impl ResubmitArgs { /// The pull this names, whichever spelling was used. /// /// The positional and `--pr` are declared `conflicts_with` each other, so /// clap has already refused the case where both are set; this only picks /// whichever one is. /// /// `pub(crate)` rather than the `pub(super)` its siblings in this file /// carry, because the parse test holding both spellings open lives in /// `main.rs` — the same reason [`super::read::ViewArgs::pull`] is. pub(crate) fn pull(&self) -> Option<&str> { self.pr.as_deref().or(self.pr_flag.as_deref()) }}
/// A pull in the listing that is still open and whose *source* is `branch`.////// A pull targeting a branch another open pull came off is the one shape of/// flat pull that cannot outlive its own base. When that other pull lands,/// Tangled merges it by rebasing onto new commits and the branch it was/// written from stops existing on the remote — which leaves this pull with/// nothing to build a round against and no way to point it somewhere else,/// since the target branch is not something any atgc command can change./// Chaining the two is what `stack create` is for, and the reason the stack/// commands correlate by change-id rather than by commit sha.////// "Open" is read loosely: only `merged` and `closed` disqualify a row. A/// fresh pull on somebody else's repo has no status record anywhere yet and/// reads `?`, and warning on `?` is the right way round — a spurious warning/// costs three lines of text, a missed one costs a pull that can never be/// updated again./// The branch a listing item is aimed at.////// The lexicon's `target` is an object and the field has moved once, so this/// reads the current spelling and answers `None` for anything else rather/// than guessing — a record whose target cannot be read is not evidence that/// it is aimed here.fn target_branch_of_item(item: &serde_json::Value) -> Option<String> { crate::model::pull::target_branch_of(&item["value"]).map(str::to_string)}
fn open_pull_from_branch<'a>( rows: &'a [crate::cmd::pr::read::StackRow], branch: &str,) -> Option<&'a serde_json::Value> { rows.iter() .filter(|r| r.state != "merged" && r.state != "closed") .map(|r| &r.item) .find(|item| item["value"]["source"]["branch"].as_str() == Some(branch))}
/// Whether the remote definitively no longer has `branch`.////// False when it has it, and false again when the remote could not be asked/// — offline, no such remote, a host that refused — because a question/// nobody answered is not a "no", and neither of the callers may turn a/// flaky network into a refusal. Only the remote's own empty answer counts.fn target_is_gone(remote: &str, branch: &str) -> bool { let answer = git::remote_branch_exists(remote, branch); crate::logging::debug::log(format!( "target branch check: {remote} has {branch}? {}", match answer { Some(true) => "yes", Some(false) => "no", None => "could not ask", } )); answer == Some(false)}
/// Whether the row for `uri` already reads merged. Pulled out of `resubmit`/// because a round did land on an already-merged pull for real once — the/// merge lands a specific round, and one appended after it would sit in the/// record's history implying it shipped. The guard is exact-match on/// purpose: anything other than the literal state string "merged" — absent,/// unknown, a stale index — is not this refusal's job.fn is_already_merged(rows: &[crate::cmd::pr::read::StackRow], uri: &str) -> bool { rows.iter() .find(|r| r.item["uri"].as_str() == Some(uri)) .is_some_and(|r| r.state == "merged")}
/// Append a round to an existing pull request.////// The record lives in your own PDS, so this reads it back, pushes a fresh/// gzipped patch onto `rounds`, and puts it again. Earlier rounds are left/// untouched — Tangled keeps them addressable at `/pulls/<n>/round/<i>`.pub(crate) async fn resubmit(args: ResubmitArgs) -> Result<()> { crate::term::jsonout::init(args.json); // The pull record lives in the author's own PDS, so "which account" has // to be settled before anything can even be read back, and the same // account must own the write. Selected once, up front, and announced — // appending a round under the wrong identity would create a stray PR // rather than revise the intended one. let selection = crate::config::account::select().await?; selection.announce(); let did = selection.did.clone();
let Some(input) = args.pull() else { return Err(crate::exit::fail( crate::exit::Exit::Usage, "which pull request? pass its number, record key or at:// URI, as in \ `atgc pr resubmit 23`\n\ the checked-out branch supplies the round's patch, not which pull the \ round is appended to", )); };
// This used to be `args.pr.rsplit('/').next()`, which is not a parse: it // took the last path segment of anything and called it a record key. A // number came through as the key "67" — legal enough to pass validation — // and turned into a bare "record not found" from the PDS instead of an // explanation, and somebody else's at-uri came through as their key // against *this* account's PDS. let target = resolve_pull_ref(input, &did, &args.remote).await?; if target.author != did { // `Denied`: there is a session and this is a refusal to write to // somebody else's record — the status a caller branches on to try a // different account rather than to retry. return Err(crate::exit::fail( crate::exit::Exit::Denied, format!( "that pull request belongs to {}; {} cannot append a round to it\n\ a round changes the pull record, which lives in its author's PDS", target.author, selection.display(), ), )); } let rkey = target.rkey; let rkey_key = Key::any_owned(&rkey).map_err(|e| anyhow::anyhow!("bad record key {rkey}: {e}"))?;
let fetched = fetch_own_pull(&did, &rkey).await?; let mut pull = fetched.pull; let target_branch = pull.target.branch.clone(); // Settled before anything is built, and settled by the record: what this // round has to do to be honest is whatever the pull already claims. let shape = shape_of(&pull)?;
// A stack member is refused outright — the one place the stack design // hardens "works on stack members, warns" into a refusal. This command // formats the *whole branch* as one round, so on a stack member it // would swallow every other member's commits into this one pull and // hand the round change-ids belonging to its neighbours: the stack's // correlation is broken for every later reconcile, web or atgc. // (A member holding several commits is fine — `stack resubmit` writes // those — what is not is one member holding all of them.) The check // reads only the author's own PDS: every // member of an own stack is an own record, so no index can be behind on // it — and a PDS that cannot answer could not take the round either. let uri = format!("at://{did}/{}/{rkey}", crate::lexicon::tangled::PULL_NSID); let listing = super::read::repo_rows(super::read::Source::PDS, &pull.target.repo).await?; if !listing.complete() { bail!( "the pull listing hit its page cap, so stack membership cannot be settled\n\ refusing: a whole-branch round on a stack member breaks its stack for good" ); } let rows = listing.rows; let items: Vec<&serde_json::Value> = rows.iter().map(|r| &r.item).collect(); let closed = crate::cmd::stack::closed_uris(&rows); if let Some(chain) = crate::cmd::stack::chain_containing(&items, &uri, &closed)? { // **Only the members that have not landed constrain this.** The // reason a stack member may not take a whole-branch round is in the // message: the round would carry every other member's commits. A // *merged* member's commits are in the target branch already, so // they are ancestors of the base this round is cut against and it // cannot carry them — the objection does not apply to them. // // Counting them anyway is not a cosmetic overcount. A stack whose // lower members have all landed is a pull with one member left to // review, and refusing it here leaves nothing that will take a // round: `stack resubmit` reconciles by change-id and cannot adopt a // pull whose patches carry none, which is every pull `pr create` // ever wrote. That is not hypothetical — it is how a chain in this // repo ended up serviceable by neither verb, and it took an // `atgc api` write to get out of. let live: Vec<&str> = chain .members .iter() .filter_map(|m| m["uri"].as_str()) .filter(|m| *m != uri.as_str()) .filter(|m| crate::cmd::stack::state_of(&rows, m) != "merged") .collect(); if !live.is_empty() { let position = chain.position_of(&uri); // `Usage`: the pull is fine and the account is right, but this // verb is the wrong one for a stack member. The fix is the other // command line the message names. return Err(crate::exit::fail( crate::exit::Exit::Usage, format!( "this pull is {position} of a stack of {}, and a whole-branch round \ would give it every other member's commits\n\ `atgc stack resubmit` reconciles the whole stack with the branch", chain.size() ), )); } crate::logging::debug::log( "every other member of this chain is merged, so a whole-branch round \ carries only this pull's own commits", ); }
// A merged pull is refused, as `stack resubmit` already refuses it: the // merge landed a specific round, and a round appended after it would sit // in the record's history implying it was part of what shipped. The gap // was real — a round went onto a freshly-merged pull because nothing // here looked. The same rows the stack check just read carry the state; // it is read from the author's own PDS, so a merge recorded only by a // *different* repo owner's account can still slip past — best effort, // like the stack warning above. (`pr edit` stays allowed on merged // pulls: fixing a description after the fact is legitimate; growing the // revision history is not.) if is_already_merged(&rows, &uri) { bail!( "this pull request is merged, so a round appended now was never part of \ what landed\n\ open follow-up work on a fresh branch with `atgc pr create`" ); }
let branch = git::current_branch()?; // A branch-based round republishes the branch the record names, so the // checkout has to be standing on it. Standing somewhere else is refused // rather than resolved either way round: pushing this branch under the // recorded name would move a branch nobody asked about, and pushing the // recorded name from a checkout that is not on it would send commits // this command never read. It is also the answer for a source branch // that no longer exists locally — you cannot check out what is gone, and // the way back is the checkout, not the record. if let Shape::Branch(source_branch) = &shape && *source_branch != branch { return Err(crate::exit::fail( crate::exit::Exit::Usage, format!( "this pull's source is branch {source_branch} and the checkout is on \ {branch}, so a round from here would not be the branch the pull names\n\ check out {source_branch}; if it is gone, the pull's history is on the \ knot: `git fetch {} {source_branch}` and branch from it", args.remote ), )); } let base = format!("{}/{}", args.remote, target_branch); // Asked of the remote before anything is built, and asked every time // rather than only when the local ref is missing. `origin/<target>` // outlives the branch it mirrors until something prunes it, so the local // ref alone cannot tell a live base from a deleted one — and a round // built on a deleted one is a patch that silently re-contains commits // that already landed, which is worse than the refusal below. if target_is_gone(&args.remote, &target_branch) { bail!( "the target branch {target_branch} is not on {} any more, so there is nothing \ to build a round against\n\ a pull that targets another pull's branch loses its base when that pull lands: \ Tangled merges by rebasing, and the branch it named goes with it\n\ the record itself is fine, but nothing can retarget it: close this pull and \ open a fresh one with `atgc pr create --target <branch>`", args.remote ); } if !git::ref_exists(&base) { crate::term::say::step!(Git, "fetching {} {}...", args.remote, target_branch); git::fetch(&args.remote, &target_branch) .with_context(|| format!("no local ref {base} and fetching it failed"))?; }
let commits = gitpatch::commit_count(&base)?; if commits == 0 { // `Usage`, for the reason exit.rs gives the whole class: the branch // this is standing on is an argument nobody typed, and the wrong one // is a command line that does not mean anything from here. return Err(crate::exit::fail( crate::exit::Exit::Usage, format!( "no commits on {branch} that aren't already on {base}\n\ atgc submits the checked-out branch; check out the one with the work" ), )); } let patch = gitpatch::format_patch(&base)?; if patch.is_empty() { bail!("empty patch for {base}..HEAD"); }
let remote_url = git::remote_url(&args.remote)?; let repo = resolve::repo_ref(&remote_url).await?;
let branch_based = matches!(shape, Shape::Branch(_)); let mut report = ResubmittedJson { dry_run: args.dry_run, uri: None, url: None, rkey: rkey.to_string(), title: pull.title.to_string(), repo_did: pull.target.repo.to_string(), target_branch: target_branch.to_string(), source_branch: branch.clone(), source_recorded: branch_based, pushed: false, commits, patch_bytes: patch.len(), patch_gzip_bytes: 0, rounds_before: pull.rounds.len(), rounds_after: pull.rounds.len() + 1, };
if !args.json { println!("pr: {} ({})", pull.title, rkey); println!("target: {} branch {}", pull.target.repo, target_branch); // Which shape this round is, in the record's own terms, before the // git progress underneath it starts. A pull that records a source is // one whose branch has to be republished for the round to mean // anything; one that records none is a patch and says so. match branch_based { true => println!("source: branch {branch} -> {}", args.remote), false => println!("source: branch {branch} (local; this pull records none)"), } } if args.dry_run { // The local patch, which is the only one there is without sending // anything. On a branch-based pull the knot recomputes it after the // push and the byte counts can differ, so these are not passed off // as the final numbers. let gzipped = gzip(&patch)?; report.patch_gzip_bytes = gzipped.len(); if args.json { return crate::term::jsonout::emit(&report); } resubmit_preview_patch(&report, &branch); println!("dry run; nothing sent"); return Ok(()); }
// The same two steps `pr create` takes, for the same reason: the round's // patch and the branch on the knot have to describe the same code, or // the pull's tree link and its newest diff disagree. A patch-based pull // pushes nothing, because it claims nothing. let patch = match shape { Shape::PatchOnly => patch, Shape::Branch(ref source_branch) => { // A round is a rewritten branch by definition, so this push has // to be able to move a diverged ref — leased against the head // the pull's own last round recorded, so that a branch somebody // else moved refuses rather than gets overwritten. let expected = recorded_head(&did, &fetched.value, &pull.title).await; let (pushed, comparison) = push_and_compare( &args.remote, source_branch, &target_branch, &repo, expected.as_deref(), "this pull records `source: {branch}`, so the round has to republish it; \ a pull that cannot be pushed to has to be opened `--patch-only` from the \ start.", ) .await?; report.pushed = pushed; report.commits = comparison.commits; report.patch_bytes = comparison.patch.len(); comparison.patch } }; let gzipped = gzip(&patch)?; report.patch_gzip_bytes = gzipped.len(); if !args.json { resubmit_preview_patch(&report, &branch); }
let agent = auth::agent_for_did(&did).await?; let gzip_len = gzipped.len(); let blob = upload_patch_blob( &agent, gzipped, format!("uploading patch blob ({gzip_len} bytes gzip)"), None, ) .await?;
pull.rounds.push(Round { patch_blob: blob.into(), created_at: Datetime::now(), extra_data: None, }); pull.validate() .map_err(|e| anyhow::anyhow!("{e}")) .context("pull record would be refused by the lexicon")?;
crate::logging::debug::log(format!( ">> putRecord {} rkey={rkey} swapRecord={}\n{}", crate::lexicon::tangled::PULL_NSID, fetched.cid.as_deref().unwrap_or("(none)"), crate::logging::debug::pretty(&pull) )); let uri = put_pull(&agent, &did, rkey_key, &pull, fetched.cid.as_deref()).await?;
let link = crate::clients::tangled::web::pulls::pull_url_for_new_record( &repo.web_url, &uri, &pull.title, ) .await; if args.json { report.uri = Some(uri); report.url = link.numbered_url().map(str::to_string); crate::term::jsonout::emit(&report)?; if let Some(note) = link.note() { crate::term::say::note!(Index, "{note}"); } return Ok(()); } println!("updated {uri}"); println!("view: {}", crate::term::hyperlink::url(link.url())); if let Some(note) = link.note() { crate::term::say::note!(Index, "{note}"); } Ok(())}
/// A pull record as it was read back, with the CID of the exact revision the/// value came from.////// The CID is the whole reason this is a struct rather than a bare `Pull`./// Every command that changes a pull is a read-modify-write against a record/// somebody else's process could be writing at the same time — `pr resubmit`/// appending a round while `pr edit` rewrites the body, say — and `putRecord`/// takes a `swapRecord` precondition that makes the write fail rather than/// clobber. Reading the CID and dropping it, which is what this used to do,/// is what makes such a collision silent: both writers exit 0 and one round/// is gone. See [`put_pull`].struct FetchedPull { pull: Pull, /// The same record, untyped, for the one question the parsed shape /// cannot answer cheaply: what the last round's patch blob says. See /// [`recorded_head`]. value: serde_json::Value, /// `None` only if the PDS omitted it, which no implementation does; the /// write then goes ahead unconditioned rather than refusing, because a /// missing precondition is a weaker guarantee and not a reason to make /// the command unusable. cid: Option<String>,}
/// Read one of your own pull records straight from your PDS, parsed into a/// [`Pull`] and carrying the CID [`put_pull`] will swap against.////// [`crate::clients::atproto::pds::get_record`] hands back untyped JSON because its other/// callers want less than a whole [`Pull`]; this is the caller that wants the/// lot, and so is the one that refuses a record written before rounds.async fn fetch_own_pull(did: &str, rkey: &str) -> Result<FetchedPull> { let pds = crate::clients::atproto::did::pds_or_fail(did).await?; let (value, cid) = crate::clients::atproto::pds::get_record( &pds, did, crate::lexicon::tangled::PULL_NSID, rkey, ) .await?; let pull = serde_json::from_value(value.clone()) .with_context(|| format!("pull record {rkey} did not parse"))?; Ok(FetchedPull { pull, value, cid })}
/// Write a pull record back, conditioned on it not having moved since it was/// read.////// The compare-and-swap and the whole `putRecord` request live in/// [`crate::clients::atproto::record::put`], which `repo edit` needs on the same terms. What is/// left here is the noun that failure messages use, so a lost race still says/// "this pull record changed" rather than naming a collection.async fn put_pull( agent: &jacquard::client::Agent<crate::clients::atproto::oauth::Session>, did: &str, rkey: Key, pull: &Pull, swap: Option<&str>,) -> Result<String> { crate::clients::atproto::record::put(agent, "pull", did, rkey, pull, swap).await}
// ---------------------------------------------------------------------------// Pull state: `pr close`, `pr reopen`// ---------------------------------------------------------------------------
/// Which pull a command was told to act on.////// A pull is addressed by the pair (whose PDS it lives in, which record key),/// which is exactly what an at-uri spells out. A bare record key leaves the/// first half to be assumed, and the only defensible assumption is the account/// atgc is acting as — that is what `pr resubmit` has always done, and it is/// right there because `pr resubmit` can only ever touch your own pulls.#[derive(Debug, Clone, PartialEq, Eq)]struct PullRef { /// The pull *author's* DID: the at-uri authority, and so the PDS the pull /// record lives in. Not necessarily the account doing the writing. author: String, rkey: String,}
impl PullRef { fn uri(&self) -> String { format!( "at://{}/{}/{}", self.author, crate::lexicon::tangled::PULL_NSID, self.rkey ) }}
/// What the user typed, told apart from the other spellings but not yet/// looked up.////// The split is between the forms that name a pull all by themselves and the/// one that has to be asked about over the network, so that the telling-apart/// stays a pure function and the one request lives in [`resolve_pull_ref`].#[derive(Debug, Clone, PartialEq, Eq)]enum PullTarget { /// Author and record key, both settled without asking anybody. Local(PullRef), /// The appview's `/pulls/23`, which is not in the record and has to be /// translated — see [`crate::cmd::pr::review::pull_ref_from_number`]. /// /// `repo_url` is the repo a pasted link named, and `None` for a number /// typed bare, which has to borrow the checkout's remote instead. A number /// means nothing without a repo, so a link that carries one must not be /// resolved against whichever repo the caller happens to be standing in — /// these are the verbs that *close* and *edit* pull requests. Number { number: u32, repo_url: Option<String>, },}
/// Turn what a user typed into a pull reference, or explain why it cannot be/// one.////// The interesting case is a bare number, because it is the first thing anyone/// types and because it used to be refused here. The number is `pulls.pull_id`,/// allocated by the appview from a per-repo `repo_pull_seqs` counter when it/// first ingests the pull record, and it lives in the appview's own database:/// no lexicon under `lexicons/` accepts or returns it — not/// `sh.tangled.repo.getPull`, not `listPulls`, not `listStatuses` — and/// Bobbin's item shape (`uri`, `cid`, `value`, `state`, `stateUpdatedAt`,/// `commentCount`) has no field for it either.////// None of which makes it unresolvable, only unqueryable. `/pulls/23` is a/// page, and the page says which record it is about; [`crate::cmd::pr::review`] has/// fetched it and read the `data-aturi` off it since `pr diff` learned to take/// a number. The write commands refused anyway, on the grounds that showing/// the wrong pull wastes a minute and closing the wrong one does not — but the/// thing that argument feared was a guess at "the 23rd pull I can see", and/// that is not what got built. Opening the exact page a number names and/// refusing when it turns out to name two records is the same check a person/// does by clicking the link, and the commands print the pull's title and/// author before they write anything. So the number resolves here too, through/// the one resolver, rather than through a second copy of it.fn classify_pull_ref(input: &str, acting_did: &str) -> Result<PullTarget> { // Every refusal below is `Usage`: the command line does not name a pull // request, and neither retrying nor logging in changes that. See // docs/output.md for what a caller does with each status. let usage = |message: String| crate::exit::fail(crate::exit::Exit::Usage, message); let input = input.trim(); if input.is_empty() { return Err(usage( "no pull request given: pass its number, record key or at:// URI".to_string(), )); }
if let Some(rest) = input.strip_prefix("at://") { let mut parts = rest.split('/'); let (Some(authority), Some(collection), Some(rkey)) = (parts.next(), parts.next(), parts.next()) else { return Err(usage(format!( "{input} is not a complete pull at-uri: it needs an authority, a \ collection and a record key, as in at://did:plc:…/{}/3lxyz…", crate::lexicon::tangled::PULL_NSID ))); }; if collection != crate::lexicon::tangled::PULL_NSID { return Err(usage(format!( "{input} names a {collection} record, not a {}: this command acts on pull \ requests", crate::lexicon::tangled::PULL_NSID ))); } // A handle is legal in an at-uri authority, and is refused here rather // than resolved. Which account a handle names can change — that is why // account selection re-resolves one and cross-checks it — and this // string decides whose PDS a record is read from and whose pull is // about to be closed. An at-uri that is stable is worth insisting on. let author = match crate::lexicon::identity::classify(authority)? { crate::lexicon::identity::Identifier::Did(did) => did, crate::lexicon::identity::Identifier::Handle(handle) => { return Err(usage(format!( "{input} names its author by the handle @{handle}. A handle can change \ hands, so atgc will not read it as a pull's owner: use the at:// URI \ with the author's DID in it, which is the form `atgc status pr` prints" ))); } }; return Ok(PullTarget::Local(PullRef { author, rkey: rkey.to_string(), })); }
// Record keys are TIDs — thirteen characters of base32-sortable — so an // all-digit string is never one, and a number is never taken for a key. if input.bytes().all(|b| b.is_ascii_digit()) { return input .parse() .map(|number| PullTarget::Number { number, repo_url: None, }) .map_err(|_| usage(format!("{input} is too large to be a pull number"))); }
// A pasted link, with or without its scheme: `tangled.org/@who/repo/pulls/23` // is as much an answer as the number in it, and is what the browser's // address bar hands over. if input.contains('/') { return match crate::clients::tangled::web::pulls::pull_in_url(input) { Some(found) => Ok(PullTarget::Number { number: found.number, repo_url: found.repo_url, }), None => Err(usage(format!( "{input} is not a pull request reference. A tangled.org link works when it has \ a /pulls/<number> in it, and this one does not.\n\ Pass the number, the record key or the at:// URI: `atgc pr view` prints all \ three" ))), }; }
// Anything left is taken as a record key in the acting account's own repo. // Validated now rather than at the PDS, so a typo is a local error instead // of a 400 from somebody's server. Key::any_owned(input).map_err(|e| usage(format!("{input} is not a record key: {e}")))?; Ok(PullTarget::Local(PullRef { author: acting_did.to_string(), rkey: input.to_string(), }))}
/// [`classify_pull_ref`], plus the one lookup a number costs.////// Every write command goes through here, so `pr close 23`, `pr close/// 3msg7w7l6hs2x` and `pr close at://did:plc:…/sh.tangled.repo.pull/3msg…` are/// the same command spelled three ways. `remote` is only read for the number/// form, which is what keeps the other two working outside a checkout.async fn resolve_pull_ref(input: &str, acting_did: &str, remote: &str) -> Result<PullRef> { match classify_pull_ref(input, acting_did)? { PullTarget::Local(target) => Ok(target), PullTarget::Number { number: n, repo_url, } => { let (author, rkey) = crate::cmd::pr::review::pull_ref_from_number(n, repo_url.as_deref(), remote) .await?; crate::logging::debug::log(format!( "pull #{n} is at://{author}/{}/{rkey}", crate::lexicon::tangled::PULL_NSID )); Ok(PullRef { author, rkey }) } }}
/// One `sh.tangled.repo.pull.status` record, cut down to what decides state.#[derive(Debug, Clone, PartialEq, Eq)]struct Status { uri: String, created_at: String, status: String,}
/// The state a set of status records adds up to.////// Tangled's rule, followed exactly rather than approximated: the newest/// record by `createdAt` wins, and a tie goes to the greater at-uri. A pull/// with no status records at all is open, which is the common case — the/// lexicon's default is `open` and Tangled writes no record when a pull is/// created, so 49 of the 49 status records in the author's PDS this was built/// against are `closed` or `merged` and not one is `open`.////// Records whose `status` this build does not recognize are skipped rather/// than allowed to win. That matches the appview, which rejects an unknown/// variant at ingest ("unknown pull status variant") and never applies it, so/// letting one decide here would disagree with what Tangled displays.////// `createdAt` is compared as a parsed instant, not as a string. A PDS stamps/// UTC (`…Z`) and another client hands back whatever offset the writer used/// (`…+03:00` appears in this account's own records), and those two do not/// sort lexically against each other: read as strings, a reopen written at/// `04:31:07+03:00` lost to a merge half an hour earlier, so `pr resubmit`/// refused a live pull as merged. An unparseable stamp sorts below anything/// dated, and the raw string breaks a tie ahead of the at-uri so the order/// stays total either way. `latest_states` in [`crate::cmd::pr::read`] orders/// the same way, as it must.////// One divergence is known and not resolvable from here: Bobbin sorts these by/// the record key's TID rather than by `createdAt`, so a record whose/// `createdAt` and whose key disagree about the order will read differently in/// the two services. atgc writes both from the same instant, so nothing it/// writes can create that disagreement.fn state_of(statuses: &[Status]) -> PullState { statuses .iter() .filter(|s| PullState::from_token(&s.status).is_some()) .max_by(|a, b| { crate::model::record::newer_state((&a.created_at, &a.uri), (&b.created_at, &b.uri)) }) .and_then(|s| PullState::from_token(&s.status)) .unwrap_or(PullState::Open)}
/// Every status record in `did`'s PDS that is about `pull_uri`.////// listRecords has no filter, so this pages and picks. What keeps that bounded/// is that record keys are TIDs and `listRecords` returns them newest first: a/// status record about a pull cannot have been written before the pull, so/// once a page ends below the pull's own key there is nothing left to find./// The comparison is strict, because Tangled's own backfill migration wrote/// status records under the *pull's* record key, and those must still be seen.async fn list_statuses(did: &str, pull_uri: &str, pull_rkey: &str) -> Result<StatusWalk> { let pds = crate::clients::atproto::did::pds_or_fail(did).await?; let walk = crate::clients::atproto::pds::records_about( &pds, did, crate::lexicon::tangled::PULL_STATUS_NSID, "pull", &[(pull_uri, pull_rkey)], ) .await?; let found = walk .by_subject .get(pull_uri) .map(|records| { records .iter() .map(|record| Status { uri: record["uri"].as_str().unwrap_or_default().to_string(), created_at: record["value"]["createdAt"] .as_str() .unwrap_or_default() .to_string(), status: record["value"]["status"] .as_str() .unwrap_or_default() .to_string(), }) .collect() }) .unwrap_or_default(); Ok(StatusWalk { statuses: found, complete: walk.complete, })}
/// One account's status records about a pull, and whether they are all of/// them.struct StatusWalk { statuses: Vec<Status>, complete: bool,}
#[derive(clap::Args, Debug)]pub(crate) struct MergeArgs { /// The pull request: its number, record key, at:// URI, or Tangled URL #[arg(value_name = "PULL")] pub pull: Option<String>, /// The same, as a flag, for symmetry with the other verbs #[arg(long = "pr", value_name = "PULL", conflicts_with = "pull")] pub pr: Option<String>, /// Git remote pointing at the repo #[arg(long, default_value = "origin")] pub remote: String, /// Run the knot's merge check and stop; nothing is merged #[arg(long)] pub dry_run: bool, /// Print one JSON object describing what was written (or, with /// --dry-run, what would be) instead of the summary lines #[arg(long)] pub json: bool,}
impl MergeArgs { /// The pull this names, whichever spelling was used. /// /// The positional and `--pr` are declared `conflicts_with` each other, so /// clap has already refused naming one pull twice; this only picks /// whichever of the two carries it. pub(super) fn pull(&self) -> Option<&str> { self.pull.as_deref().or(self.pr.as_deref()) }}
/// What `pr merge` did, or would have done.////// The stack shape's single-pull twin, and deliberately not the same struct:/// this one names the pull, since naming one is how the command is invoked,/// while `stack merge` names a chain and a `--through` cut. As there,/// `merged: false` on a dry run means the knot's check passed — a conflict/// is an error and never reaches this.#[derive(serde::Serialize, Debug, PartialEq)]pub(super) struct MergedJson { pub dry_run: bool, pub merged: bool, pub uri: String, pub rkey: String, pub title: String, pub repo_did: String, pub target_branch: String, /// The host that moved the branch, which no PDS could answer for. pub knot: String, pub url: String,}
/// Land one flat pull request: knot `mergeCheck`, knot `merge` with the/// latest round's patch, then a merged status record.////// Exactly the web's merge button: the knot call is authorized by a/// service-auth token naming this account, and the knot decides whether this/// account may push to the repo. atgc used to decide that itself, by whether/// [`crate::cmd::stack::repo_facts`] found the repo record in this account's/// own PDS, which refused every collaborator who can merge on tangled.org./// A stacked pull is still refused: Tangled merges a stack member together/// with everything beneath it, and that is `atgc stack merge`'s contract,/// not this command's.pub(super) async fn merge(args: MergeArgs) -> Result<()> { crate::term::jsonout::init(args.json); let selection = crate::config::account::select().await?; selection.announce(); let me = selection.did.clone();
let Some(reference) = args.pull() else { return Err(crate::exit::fail( crate::exit::Exit::Usage, "name the pull to merge: a number, record key, at:// URI or URL", )); }; let target = resolve_pull_ref(reference, &me, &args.remote).await?; let uri = target.uri(); let author_pds = crate::clients::atproto::did::pds_or_fail(&target.author).await?; let (value, _cid) = crate::clients::atproto::pds::get_record( &author_pds, &target.author, crate::lexicon::tangled::PULL_NSID, &target.rkey, ) .await?; let title = value["title"].as_str().unwrap_or("(untitled)").to_string(); let repo_did = value["target"]["repo"] .as_str() .ok_or_else(|| anyhow::anyhow!("{title} names no target repo"))? .to_string();
// The stacked refusal needs the listing — being depended on is written // in other records, not this one. let listing = super::read::repo_rows(super::read::Source::EVERY, &repo_did).await?; if !listing.complete() { bail!( "the pull listing hit its page cap, so whether {title} is a stack member \ cannot be settled, and merging one flat skips the pulls beneath it. \ refusing until the listing fits" ); } let rows = listing.rows; let items: Vec<&serde_json::Value> = rows.iter().map(|r| &r.item).collect(); let closed = crate::cmd::stack::closed_uris(&rows); if let Some(chain) = crate::cmd::stack::chain_containing(&items, &uri, &closed)? { // `Usage`, as with `pr resubmit` on a stack member: the wrong verb // for this pull, and the message names the right one. return Err(crate::exit::fail( crate::exit::Exit::Usage, format!( "{title} is part of a stack of {}: Tangled merges a stack member together \ with everything beneath it\n\ `atgc stack merge` is the command for that (--through picks how far up)", chain.size() ), )); }
let my_pds = crate::clients::atproto::did::pds_or_fail(&me).await?; let facts = crate::cmd::stack::repo_facts(&my_pds, &me, &repo_did).await?; let patch = crate::cmd::stack::latest_round_patch(&author_pds, &target.author, &value, &title).await?; // **The same strict reading `stack merge` uses**, and for the reason // written on it there: this is the one place a wrong guess is // unrecoverable, because it names the branch the knot writes to. // // This defaulted to `main`. `stack merge` stopped doing that and this // did not, while both build the same `MergePlan` and hand it to the same // `run_merge` — so a record with a malformed `target.branch` landed a // patch on a branch nobody named, and a pre-rounds record carrying only // `targetBranch` (which `pr view` has always read) merged onto `main` // instead of the branch it asked for. let target_branch = crate::model::pull::target_branch(&value, &target.uri())?;
if !args.json { println!("pr: {title} ({})", target.rkey); // raw-did-ok: the DID comes off the pull record and there is no // `RepoRef` here — unlike `pr create`, this command was given a pull // rather than a remote, so naming the repo would cost a resolve on a // line printed after the person has already named what they meant. // plan/output.md carries this as the open half of naming repos. println!("target: {repo_did} branch {target_branch}"); } let plan = crate::cmd::stack::MergePlan { knot: facts.knot, actor_did: me.clone(), // The pull's author, not whoever is merging: the merge commit // belongs to the person whose patch it is, which is what Tangled's // own merge sends. Only read for a patch `git am` cannot take. author_did: target.author.clone(), owner: facts.owner, repo_did, target_branch, patch, title, body: value["body"].as_str().map(str::to_string), pulls: vec![uri], }; let agent = auth::agent_for_did(&me).await?; crate::cmd::stack::run_merge(&agent, &plan, args.dry_run).await?; if args.json { return crate::term::jsonout::emit(&MergedJson { dry_run: args.dry_run, merged: !args.dry_run, uri: plan.pulls[0].clone(), rkey: target.rkey.clone(), title: plan.title.clone(), repo_did: plan.repo_did.clone(), target_branch: plan.target_branch.clone(), knot: plan.knot.clone(), url: format!("{}/{}/pulls", appview(), plan.repo_did), }); } Ok(())}
#[derive(clap::Args, Debug)]pub(crate) struct StateArgs { /// The pull request: its number, record key, at:// URI, or Tangled URL #[arg(value_name = "PULL")] pub pr: Option<String>, /// The same, as a flag, for symmetry with the other verbs #[arg(long = "pr", value_name = "PULL", conflicts_with = "pr")] pub pr_flag: Option<String>, /// Which repo a pull *number* is numbered against; unused otherwise #[arg(long, default_value = "origin")] pub remote: String, /// Say what would change without writing anything #[arg(long)] pub dry_run: bool, /// Print one JSON object describing what was written (or, with /// --dry-run, what would be) instead of the summary lines #[arg(long)] pub json: bool,}
impl StateArgs { /// The pull this names, whichever spelling was used. /// /// The positional and `--pr` are declared `conflicts_with` each other, so /// clap has already refused the case where both are set; this only picks /// whichever one is. pub(super) fn pull(&self) -> Option<&str> { self.pr.as_deref().or(self.pr_flag.as_deref()) }}
pub(crate) async fn close(args: StateArgs) -> Result<()> { set_state(args, PullState::Closed).await}
pub(crate) async fn reopen(args: StateArgs) -> Result<()> { set_state(args, PullState::Open).await}
/// The open pulls that depend on `uri`, directly or through another member.////// A stack is a chain of `dependentOn` links, so closing a member leaves/// every pull above it depending on a closed pull. Tangled does not stop/// that and neither does this — closing a member is a documented way to take/// work out of a stack — but it is worth saying out loud, because the pulls/// it affects are not the one being named and nothing else on screen mentions/// them.////// Titles rather than counts, because "3 pulls depend on this" tells you/// there is something to check and not what to check.////// Best effort throughout, and silent when it cannot answer. This is a note/// attached to a write that is going to happen anyway; a listing that will/// not load is a reason to say nothing, not a reason to refuse a close the/// user asked for. `EVERY` source for `Source::EVERY`'s documented reason:/// the listing is being read to find a reason to *warn*, and a source that/// sees less cannot warn about more.async fn dependents_above(repo_did: Option<&str>, uri: &str) -> (Vec<String>, bool) { let Some(repo_did) = repo_did else { return (Vec::new(), true); }; let Ok(rows) = super::read::repo_rows(super::read::Source::EVERY, repo_did).await else { return (Vec::new(), true); }; // **A listing that would not load and one that stopped at its cap are // different answers.** The doc above reasons about the first — say // nothing, this is a note on a write that is happening anyway — and the // second used to fall through it: the members past the cap were simply // not counted, so a short list was printed in the voice of a complete // one. "2 pulls depend on it" is worse than silence when there are four. let whole = rows.complete(); let items: Vec<&serde_json::Value> = rows.rows.iter().map(|r| &r.item).collect(); let closed = crate::cmd::stack::closed_uris(&rows.rows); let Ok(Some(chain)) = crate::cmd::stack::chain_containing(&items, uri, &closed) else { return (Vec::new(), whole); }; // Bottom-first, so everything after this one is above it. let position = chain .members .iter() .position(|m| m["uri"].as_str() == Some(uri)); let Some(position) = position else { return (Vec::new(), whole); }; let above: Vec<String> = chain.members[position + 1..] .iter() .filter(|m| { // A member already closed is not made worse by this, and a merged // one is landed. Only the open ones above are left holding a // dependency on something that is about to close. crate::cmd::stack::state_of(&rows.rows, m["uri"].as_str().unwrap_or_default()) == PullState::Open.label() }) .map(|m| { format!( "{} ({})", m["value"]["title"].as_str().unwrap_or("(untitled)"), m["uri"] .as_str() .unwrap_or("?") .rsplit('/') .next() .unwrap_or("?") ) }) .collect(); (above, whole)}
/// Move a pull to `wanted` by appending a status record.////// Appending is the whole mechanism: `sh.tangled.repo.pull.status` records are/// a log keyed by TID, nothing looks one up by key, and the appview recomputes/// a pull's state as the newest record. So reopening is a new `open` record/// rather than a delete of the `closed` one or a put over it — which is also/// what Tangled's own web UI does (`writePullStatusRecords` creates a record/// with a fresh TID for close, reopen and merge alike).async fn set_state(args: StateArgs, wanted: PullState) -> Result<()> { crate::term::jsonout::init(args.json); // Both spellings, because "closeing" is what `{verb}ing` produces and the // messages below are the ones a user reads at the moment something has // just refused to happen. let (verb, gerund) = match wanted { PullState::Open => ("reopen", "reopening"), PullState::Closed => ("close", "closing"), PullState::Merged => ("merge", "merging"), }; // Settled first: the status record is written into *this* account's PDS, // so which account it is decides both whether the write is honored and // where the record lands. let selection = crate::config::account::select().await?; selection.announce(); let acting = selection.did.clone();
let Some(input) = args.pull() else { return Err(crate::exit::fail( crate::exit::Exit::Usage, format!( "which pull request? pass its number, record key or at:// URI, as in \ `atgc pr {verb} 23`\n\ atgc does not guess from the current branch: its own pull records carry no \ source branch, so the guess would fall back to your newest pull on the repo, \ and {gerund} the wrong one is not undoable from here" ), )); }; let target = resolve_pull_ref(input, &acting, &args.remote).await?;
// The pull being closed or reopened need not be yours, so the PDS holding // it need not be either — it is whichever one the author's DID document // names. let author = &target.author; let pds = crate::clients::atproto::did::pds_or_fail(author).await?; let (value, _) = crate::clients::atproto::pds::get_record( &pds, author, crate::lexicon::tangled::PULL_NSID, &target.rkey, ) .await?; let title = value["title"].as_str().unwrap_or("(untitled)").to_string(); // `target_repo_did` reads a Bobbin list item, which wraps the record under // `value`; this is the same record with no envelope around it, so it gets // one to be read by the same rule rather than by a second copy of it. let repo_did = target_repo_did(&serde_json::json!({ "value": value }));
let standing = standing_of(&acting, &target.author, repo_did.as_deref()).await?; if standing == Standing::Neither { // `Denied`, the same answer `issue close` gives for the same shape: // there is a session, and it is the account that is wrong. return Err(crate::exit::fail( crate::exit::Exit::Denied, format!( "{} did not open this pull request and does not own the repo it targets, so a \ status record it writes would be dropped\n\ the pull is {}'s; Tangled honors a status record from the pull's author or \ the target repo's owner\n\ a collaborator with push access can {verb} it through tangled.org: atgc \ cannot check knot collaborator lists, so it does not pretend the write would \ land", selection.display(), target.author, ), )); }
let uri = target.uri(); let walk = list_statuses(&target.author, &uri, &target.rkey).await?; let mut statuses = walk.statuses; let mut complete = walk.complete; if acting != target.author { let mine = list_statuses(&acting, &uri, &target.rkey).await?; statuses.extend(mine.statuses); complete &= mine.complete; } // **And the repo owner's, which is the account this walk used to be // blind to.** // // `standing_of` above names the two accounts Tangled honors — the pull's // author and the target repo's owner — and this walk read only the // first, plus whoever happens to be running the command. So an owner's // records were invisible to the author, and the state below fell through // to `Open` for want of a record that existed. // // That is not a cosmetic wrong answer. The `merged` refusal a few lines // down exists because state is a log and the newest record wins, so a // `closed` written after a `merged` *replaces* it in every reader's // view. A pull merged by the repo owner — the ordinary way a // contributor's pull gets merged — carries its `merged` record in the // owner's PDS, so the author closing it walked straight past the guard // and hid the merge. `an_owners_merge_is_invisible_to_the_authors_close` // in `tests/pr_flows.rs` is that sequence. // Skipped when this account *is* the owner: `standing_of` settled that // above, and author-plus-acting has then already covered both honored // writers. Skipped too when the pull names no repo, because then there // is no owner for Tangled to honor and nothing more to read. if standing != Standing::RepoOwner && let Some(repo_did) = repo_did.as_deref() { match resolve::owner_of(repo_did).await? { Some(owner) if owner != target.author && owner != acting => { let theirs = list_statuses(&owner, &uri, &target.rkey).await?; statuses.extend(theirs.statuses); complete &= theirs.complete; } // The owner is the author, or is running this: already walked. Some(_) => {} // Refused rather than guessed, for the same reason the page cap // below is. An account that was not read is an absence that // reads exactly like "nobody has acted on this pull", and acting // on it means writing over whatever is in there. None => bail!( "cannot tell who owns the repo this pull targets, so its current state \ cannot be settled\n\ refusing to {verb} it: a status record in the owner's repository — a \ merge, most of all — is invisible from here, and the newest record is \ the one every reader believes\n\ the owner is published by the appview and it did not answer; {verb} it \ through tangled.org, which reads its own index", ), } }
// **A state read off a short walk is not a state.** Every decision below // turns on the *absence* of a record — "already closed, nothing written" // and the refusal that keeps a merge from being overwritten are both read // off what did not come back — and a walk that ran out of pages before // reaching this pull produces exactly the same absence as a pull nobody // has acted on. Acting on it would mean writing `closed` over a `merged` // that is sitting one page past the cap, which is the one write this // command refuses outright when it can see it. if !complete { bail!( "the status walk hit its page cap before reaching {}, so this pull's current \ state cannot be settled\n\ refusing to {verb} it: a state record beyond the cap — a merge, most of all — \ is invisible from here, and writing over a merge is the one thing this \ command refuses outright when it can see it\n\ {verb} it through tangled.org, which reads its own index rather than walking \ the records", target.rkey, ); } let current = state_of(&statuses); crate::logging::debug::log(format!( "{} status record(s) for {uri}; current state {}", statuses.len(), current.label() ));
let mut report = StateChangeJson { dry_run: args.dry_run, uri: None, url: repo_did .as_deref() .map(|did| format!("{}/{did}/pulls", appview())), pull_uri: uri.clone(), rkey: target.rkey.clone(), title: title.clone(), author_did: target.author.clone(), state_before: current.label().to_string(), state_after: wanted.label().to_string(), changed: false, };
if !args.json { println!("pr: {title} ({})", target.rkey); println!( "author: {}", crate::term::hyperlink::account( crate::clients::atproto::handles::handle(&target.author) .await .as_deref(), &target.author, ) ); }
if current == wanted { // Nothing is appended to the status log for a no-op, so the // `state_after` a caller reads is the state it already had — and // `changed` is how that reads as "already there" rather than "done". if args.json { report.state_after = current.label().to_string(); return crate::term::jsonout::emit(&report); } println!( "state: {} (already {}; nothing written)", current.label(), current.label() ); return Ok(()); } // Merged is terminal and carries information nothing else does. Replacing // it would be a real change, not a no-op, so it is refused rather than // done quietly — the pull's code is in the target branch either way, and // the marker saying so is the only record of it here. if current == PullState::Merged { bail!( "this pull request is already merged, and {gerund} it would replace that with \ `{}` in Tangled's view of it\n\ state is a log of records and the newest wins, so there is no way to {verb} it \ without hiding the merge; do it through tangled.org if that is really what you want", wanted.label() ); }
report.changed = true; if !args.json { println!("state: {} -> {}", current.label(), wanted.label()); } // Only on the way *out* of open. Reopening a member restores the // dependency the close broke, which is the opposite of a thing to warn // about. if wanted == PullState::Closed { let (above, whole) = dependents_above(repo_did.as_deref(), &uri).await; if !above.is_empty() { crate::term::say::warning!( Index, "{}{} open pull request(s) in this stack depend on it, directly or through \n\ another member, and will be left depending on a closed pull:\n {}\n\ `atgc stack resubmit` relinks the chain around it once the branch no longer \ carries this commit.{}", match whole { true => "", false => "at least ", }, above.len(), above.join("\n "), match whole { true => "", false => "\nthe pull listing hit its page cap, so members past it are not counted here.", } ); } } if args.dry_run { if args.json { return crate::term::jsonout::emit(&report); } println!("dry run; nothing sent"); return Ok(()); }
let record: PullStatus = PullStatus { pull: AtUri::new(uri.clone().into())?, status: StatusStatus::from_value(wanted.token().into()), created_at: Datetime::now(), extra_data: None, }; crate::logging::debug::log(format!( ">> createRecord {}\n{}", crate::lexicon::tangled::PULL_STATUS_NSID, crate::logging::debug::pretty(&record) )); let agent = auth::agent_for_did(&acting).await?; // No rkey: the PDS mints a TID, which is what the lexicon's `key: "tid"` // asks for and what Tangled's own writer does. let output = agent.create_record(record, None).await.map_err(|e| { crate::logging::debug::dump_err("createRecord error", &e); anyhow::anyhow!("failed to write the pull status record: {e}") })?;
if args.json { report.uri = Some(output.uri.to_string()); return crate::term::jsonout::emit(&report); } println!("wrote {}", output.uri); if let Some(repo_did) = repo_did { println!( "view: {}", crate::term::hyperlink::url(&format!("{}/{repo_did}/pulls", appview())) ); } Ok(())}
// ---------------------------------------------------------------------------// `pr edit`// ---------------------------------------------------------------------------
#[derive(clap::Args, Debug)]pub(crate) struct EditArgs { /// The pull request: its number, record key, at:// URI, or Tangled URL #[arg(value_name = "PULL")] pub pr: Option<String>, /// The same, as a flag, for symmetry with the other verbs #[arg(long = "pr", value_name = "PULL", conflicts_with = "pr")] pub pr_flag: Option<String>, /// Which repo a pull *number* is numbered against; unused otherwise #[arg(long, default_value = "origin")] pub remote: String, /// New title #[arg(short, long)] pub title: Option<String>, /// New description; empty clears it, local image paths are uploaded /// and embedded #[arg(short, long)] pub body: Option<String>, /// New description read from a file (local image paths are uploaded /// and embedded), or from stdin with `-` #[arg(long, conflicts_with = "body")] pub body_file: Option<String>, /// Say what would change without writing anything #[arg(long)] pub dry_run: bool, /// Print one JSON object describing what was written (or, with /// --dry-run, what would be) instead of the summary lines #[arg(long)] pub json: bool,}
impl EditArgs { /// The pull this names, whichever spelling was used. /// /// The positional and `--pr` are declared `conflicts_with` each other, so /// clap has already refused the case where both are set; this only picks /// whichever one is. pub(super) fn pull(&self) -> Option<&str> { self.pr.as_deref().or(self.pr_flag.as_deref()) }}
/// Where a new body comes from: [`crate::term::body::read`], with this/// family's noun in the error lines.////// Named here rather than called inline because the nested `Option` is load/// bearing at both call sites: `None` is "leave the body alone" and/// `Some(None)` is "clear it", which is the distinction `pr edit` acts on.fn new_body(body: Option<String>, body_file: Option<String>) -> Result<Option<Option<String>>> { crate::term::body::read(body, body_file, "new body")}
/// Whether a resent body is worth writing back. Equal text alone is not a/// no-op when local images are attached: the stored copy holds the paths/// verbatim, and resending it is exactly what uploads them and rewrites the/// paths to `blob+at://` URIs. Separated because dropping or inverting this/// clause is silent — an edit that only re-embeds local images would print/// "nothing to change" and leave the broken local paths live, with nothing/// here to catch it.fn body_edit_is_a_change( new_body: &Option<String>, stored_body: &Option<String>, images_present: bool,) -> bool { new_body != stored_body || images_present}
/// Change the title and/or body of a pull you authored.////// The record lives in the author's PDS and there is no way to write to/// somebody else's, so unlike `pr close` this really is author-only. Rounds,/// target, source and createdAt ride through untouched: the record is read/// back, two fields are replaced, and the whole thing is put again under the/// CID it was read at.pub(crate) async fn edit(args: EditArgs) -> Result<()> { crate::term::jsonout::init(args.json); let selection = crate::config::account::select().await?; selection.announce(); let acting = selection.did.clone();
let Some(input) = args.pull() else { return Err(crate::exit::fail( crate::exit::Exit::Usage, "which pull request? pass its number, record key or at:// URI, as in \ `atgc pr edit 23 --title '…'`", )); }; let target = resolve_pull_ref(input, &acting, &args.remote).await?; if target.author != acting { return Err(crate::exit::fail( crate::exit::Exit::Denied, format!( "that pull request belongs to {}, and its record lives in that account's PDS: \ {} cannot write to it\n\ only a pull's author can edit its title or body; `atgc pr close` is the one \ that also works on somebody else's pull, because the record it writes is your \ own", target.author, selection.display(), ), )); }
let body = new_body(args.body, args.body_file)?; if args.title.is_none() && body.is_none() { return Err(crate::exit::fail( crate::exit::Exit::Usage, "nothing to change: pass --title, --body or --body-file", )); } // Same order as `pr create`: local refusals before any network traffic. let images = match body.as_ref() { Some(Some(text)) => crate::cmd::images::scan(text)?, _ => crate::cmd::images::Images::none(), };
let fetched = fetch_own_pull(&acting, &target.rkey).await?; let mut pull = fetched.pull; let rkey_key = Key::any_owned(&target.rkey) .map_err(|e| anyhow::anyhow!("bad record key {}: {e}", target.rkey))?;
let mut report = EditedJson { dry_run: args.dry_run, uri: None, url: repo_did_of(&pull).map(|did| format!("{}/{did}/pulls", appview())), rkey: target.rkey.clone(), title: pull.title.to_string(), title_changed: false, body_changed: false, changed: false, rounds: pull.rounds.len(), images: images.listed(), };
if !args.json { println!("pr: {} ({})", pull.title, target.rkey); println!("rounds: {} (unchanged)", pull.rounds.len()); }
let mut changed = false; if let Some(title) = args.title { if title.trim().is_empty() { return Err(crate::exit::fail( crate::exit::Exit::Usage, "a pull request needs a title; --title cannot be emptied", )); } if title.as_str() != pull.title.as_str() { if !args.json { println!("title: {} -> {title}", pull.title); } report.title = title.clone(); report.title_changed = true; pull.title = title.into(); changed = true; } } // A body equal to the stored one still counts as a change when it embeds // local images: the stored copy holds the paths, and the whole point of // resending it is to upload them and put `blob+at://` URIs there instead. if let Some(body) = body && body_edit_is_a_change( &body, &pull.body.as_ref().map(ToString::to_string), !images.is_empty(), ) { if !args.json { println!( "body: {} -> {}", describe_body(pull.body.as_deref()), describe_body(body.as_deref()) ); for line in images.describe() { println!("image: {line}"); } } report.body_changed = true; pull.body = body.map(Into::into); changed = true; } report.changed = changed; if !changed { // Nothing written, and the command still exits 0 — `changed: false` // is what says so, since there is no uri to be absent here that a // dry run would not also lack. if args.json { return crate::term::jsonout::emit(&report); } println!("nothing to change; the record already says this"); return Ok(()); } if args.dry_run { if args.json { return crate::term::jsonout::emit(&report); } println!("dry run; nothing sent"); return Ok(()); }
let agent = auth::agent_for_did(&acting).await?; if !images.is_empty() { let text = pull.body.take().unwrap_or_default(); let (rewritten, new_blobs) = crate::cmd::images::upload_and_rewrite(&agent, &acting, &text, &images).await?; pull.body = Some(rewritten.into()); let existing: Option<Vec<jacquard::types::blob::Blob>> = pull .blobs .take() .map(|bs| bs.into_iter().map(Into::into).collect()); pull.blobs = crate::cmd::images::merge_blobs(existing, new_blobs) .map(|bs| bs.into_iter().map(Into::into).collect()); } pull.validate() .map_err(|e| anyhow::anyhow!("{e}")) .context("pull record would be refused by the lexicon")?; crate::logging::debug::log(format!( ">> putRecord {} rkey={} swapRecord={}\n{}", crate::lexicon::tangled::PULL_NSID, target.rkey, fetched.cid.as_deref().unwrap_or("(none)"), crate::logging::debug::pretty(&pull) )); let uri = put_pull(&agent, &acting, rkey_key, &pull, fetched.cid.as_deref()).await?; if args.json { report.uri = Some(uri); return crate::term::jsonout::emit(&report); } println!("updated {uri}"); if let Some(repo_did) = repo_did_of(&pull) { println!( "view: {}", crate::term::hyperlink::url(&format!("{}/{repo_did}/pulls", appview())) ); } Ok(())}
/// A body, described rather than printed. A PR body is arbitrarily long and/// the point of the line is that it changed, not what it now says.fn describe_body(body: Option<&str>) -> String { match body { None => "(none)".to_string(), Some(text) => format!("{} bytes", text.len()), }}
fn repo_did_of(pull: &Pull) -> Option<String> { let repo = pull.target.repo.as_str(); (!repo.is_empty()).then(|| repo.to_string())}
// ---------------------------------------------------------------------------// `pr comment`// ---------------------------------------------------------------------------
#[derive(clap::Args, Debug)]pub(crate) struct CommentArgs { /// The pull request: its number, record key, at:// URI, or Tangled URL /// /// Defaults to the one this branch is about, as `pr view` finds it. #[arg(value_name = "PULL")] pub pull: Option<String>, /// The same, as a flag, for symmetry with the other verbs #[arg(long, value_name = "PULL", conflicts_with = "pull")] pub pr: Option<String>, /// What to say; local image paths in it are uploaded and embedded #[arg(short, long)] pub body: Option<String>, /// The same, read from a file, or from stdin with `-` #[arg(long, conflicts_with = "body")] pub body_file: Option<String>, /// Which round to comment on, counting from 1 (defaults to the latest) #[arg(long, value_name = "N")] pub round: Option<usize>, /// Whose pull request it is, when a bare record key is ambiguous #[arg(long, value_name = "HANDLE|DID")] pub author: Option<String>, /// Git remote pointing at the target repo #[arg(long, default_value = "origin")] pub remote: String, /// Say what would be written without writing anything #[arg(long)] pub dry_run: bool, /// Print one JSON object describing what was written (or, with /// --dry-run, what would be) instead of the summary lines #[arg(long)] pub json: bool,}
impl CommentArgs { /// The pull this names, whichever spelling was used. /// /// The positional and `--pr` are declared `conflicts_with` each other, so /// clap has already refused naming one pull twice; this only picks /// whichever of the two carries it. pub(super) fn pull(&self) -> Option<&str> { self.pull.as_deref().or(self.pr.as_deref()) }}
/// Convert atgc's one-based `--round` to the record's zero-based/// `pullRoundIdx`, or say why it cannot be.////// The two numberings genuinely differ, and the mismatch is not atgc's to fix/// here. Tangled's round URLs are zero-based — `/pulls/1/round/0` is a/// one-round pull's only round, verified against the live site — while/// `pr diff --round` and `pr checkout --round` have been one-based since they/// landed. Changing those would silently reinterpret every `--round` anyone/// has scripted, so `pr comment` matches its siblings instead of the URL, and/// the printed output names both so the difference is visible rather than/// merely documented.fn round_index(round: Option<usize>, total: usize) -> Result<usize> { match round { None => Ok(total - 1), // A `--round` outside the pull's range is a value that does not mean // anything, which is `Usage` rather than `NotFound`: the pull // resolved fine and the flag is what has to change. Some(0) => Err(crate::exit::fail( crate::exit::Exit::Usage, "rounds are numbered from 1 here, so there is no round 0\n\ (Tangled's own URLs count from 0, so its /round/0 is atgc's --round 1)", )), Some(n) if n <= total => Ok(n - 1), Some(n) => Err(crate::exit::fail( crate::exit::Exit::Usage, format!("this pull request has {total} round(s); there is no round {n}"), )), }}
/// Where a comment body comes from: `--body`, a file, or stdin.////// Deliberately no `$EDITOR`, for the reason [`new_body`] gives at length —/// atgc runs unattended, and a command that spawns an editor either hangs or/// eats its own stdin when nobody is watching. Unlike `pr edit`, an empty body/// has no meaning to fall back on: there is no such thing as clearing a/// comment you have not written yet, so whitespace-only input is an error/// rather than a no-op. The appview agrees and would reject it at ingest/// (`body is empty after HTML sanitization`), but failing here costs nothing/// and failing there is invisible.fn comment_body(body: Option<String>, body_file: Option<String>) -> Result<String> { crate::term::body::required(body, body_file, "comment body", "comment")}
/// Turn a `createRecord` failure into something actionable, with the stale/// grant called out by name.////// A 403 here is very unlikely to be a real permission problem: the record/// goes into the acting account's own PDS, and an account may always write to/// itself. What it means in practice is that the session was granted before/// `repo:sh.tangled.feed.comment` was added to/// [`crate::clients::tangled::scope`]'s scope list,/// so the token authorizes ten other Tangled collections and not this one./// That is exactly the failure this command hit the first time it ran for/// real, and "403 Forbidden" on its own sends you looking at the repo's/// permissions, which are not involved.fn scope_advice(error: &str) -> anyhow::Error { if error.contains("403") || error.contains("Forbidden") { return anyhow::anyhow!( "the PDS refused to create the comment record ({error})\n\ this is almost certainly a session older than `pr comment` itself: comments moved to \ the `sh.tangled.feed.comment` collection, and a grant issued before atgc asked for \ it does not cover it\n\ `atgc auth login` again to re-grant: the scope list is part of the client identity, \ so it cannot be widened in place" ); } anyhow::anyhow!("failed to write the comment record: {error}")}
/// Comment on a pull request.////// # This writes `sh.tangled.feed.comment`, not `sh.tangled.repo.pull.comment`////// The obvious record is the wrong one, and the failure is silent, so it is/// worth being explicit. `lexicons/pulls/comment.json` still describes/// `sh.tangled.repo.pull.comment` and it still looks current. It is not:/// Tangled unified issue, pull and string comments into/// `sh.tangled.feed.comment`, and its ingester now treats the old collection/// as deletes only — creates and updates hit a branch whose entire body is/// `// no-op. sh.tangled.repo.pull.comment is deprecated`. A comment written/// there would be accepted by the PDS, would federate, and would never appear/// anywhere. See [`crate::lexicon::tangled::LEGACY_PULL_COMMENT_NSID`].////// The new record carries two things the old one had no room for: a `subject`/// strongRef instead of a bare at-uri, and `pullRoundIdx`. The round was/// previously appview-local state — a foreign key in its SQLite that was never/// federated at all — which is what makes the old record unusable rather than/// merely superseded.////// # Anyone may comment////// Unlike `pr close`, this needs no standing check, and the difference is real/// rather than an oversight here. The appview's `ingestComment` performs no/// ACL lookup of any kind: it unmarshals, validates the shape, resolves/// mentions and writes. Its jetstream subscription filters by collection and/// not by DID. Bobbin's `list_feed_comments` is likewise unfiltered, where its/// pull-status reads are narrowed to author-or-repo-owner. So both indexers/// honour a comment from anybody, which is evidently the intent — comments are/// public discourse, statuses are a permission.////// That is why there is no equivalent of [`crate::model::record::standing_of`] on this path. Adding/// one would refuse writes that Tangled itself accepts.pub(crate) async fn comment(args: CommentArgs) -> Result<()> { crate::term::jsonout::init(args.json); // Settled before anything is read, so `--dry-run` answers "who would this // be from?" as well as "what would it say?" — the same order `pr create` // and `pr close` use. The record lands in this account's PDS, so the // answer also decides where it goes. let selection = crate::config::account::select().await?; selection.announce(); let acting = selection.did.clone();
// Read the pull reference off the args before the body is moved out of // them; `pull()` borrows, and `comment_body` takes ownership. let reference = args.pull().map(str::to_string); // Read the body before touching the network. A missing --body is the // likeliest mistake, and finding out after three round trips is worse. let body = comment_body(args.body, args.body_file)?; // And scan it on the same principle: a local image path that names no // file, a non-image or an oversized file refuses here, by name. let images = crate::cmd::images::scan(&body)?;
// One resolver for the whole tool. `pr comment 23` works because // `review::resolve_pull` already knows how to turn an appview pull number // into a record, by scraping the `data-aturi` widget off the pull's page — // there is no query that does it, the number being the appview's own // SQLite id. A second scraper would be a second thing to be wrong. let pull = crate::cmd::pr::review::resolve_pull( reference.as_deref(), args.author.as_deref(), &args.remote, ) .await?;
let total = pull.round_count(); let idx = round_index(args.round, total)?;
// Required by the lexicon and enforced by the appview, and the strongRef // is where it comes from. A pull record read back without a CID cannot be // commented on: `subject.cid` has no default, and the appview rejects a // comment whose subject CID does not parse. let cid = pull.cid.clone().with_context(|| { format!( "the PDS returned pull record {} without a CID, and a comment's subject needs one", pull.rkey ) })?;
let author_label = crate::term::hyperlink::account( crate::clients::atproto::handles::handle(&pull.did) .await .as_deref(), &pull.did, );
let mut report = CommentedJson { dry_run: args.dry_run, uri: None, url: target_repo_did(&serde_json::json!({ "value": pull.value })) .map(|did| format!("{}/{did}/pulls", appview())), pull_uri: pull.uri.clone(), pull_rkey: pull.rkey.clone(), title: pull.title().to_string(), author_did: pull.did.clone(), round: idx + 1, round_index: idx, rounds: total, body_bytes: body.len(), images: images.listed(), };
if !args.json { println!("pr: {} ({})", pull.title(), pull.rkey); println!("author: {author_label}"); // Both numberings, because they disagree and the user is about to go // and look for the comment at one of them. println!( "round: {} of {total} (Tangled calls it round {idx})", idx + 1 ); println!("body: {} bytes", body.len()); for line in images.describe() { println!("image: {line}"); } }
// Asked before the write rather than only translated after it. Every // session made before `pr comment` shipped is missing this scope, and // [`scope_advice`] below can only explain the 403 once it has happened; // this refuses first and does not spend a write to find out. It stays as // the backstop, for a store entry that records no scopes at all. let scope = || { crate::clients::tangled::scope::require_scope( crate::cmd::acting_scope(&acting).as_deref(), selection.handle.as_deref(), crate::lexicon::tangled::FEED_COMMENT_NSID, ) }; if args.dry_run { // The scope a real run would need, checked here so a dry run says // what it would cost — as a note, since it describes the session // rather than the comment. if let Err(e) = scope() { crate::term::say::warning!(Auth, "would fail: {e}"); } if args.json { return crate::term::jsonout::emit(&report); } println!("dry run; nothing sent"); return Ok(()); } scope()?;
let agent = auth::agent_for_did(&acting).await?; // `text` and `original` both get the rewritten form, exactly as // `markdown_plain` writes one string to both: `original` exists to // refill an edit box, and the `blob+at://` spelling is the one a later // edit can keep using — the local path stopped meaning anything the // moment it left this machine. let body = if images.is_empty() { markdown_plain(body) } else { let (rewritten, blobs) = crate::cmd::images::upload_and_rewrite(&agent, &acting, &body, &images).await?; markdown_with_blobs(rewritten, crate::cmd::images::merge_blobs(None, blobs)) }; // Not a precondition the way `swapRecord` is: nothing checks that this // CID still matches by the time anyone reads the comment. It goes stale // the moment the subject is rewritten, which for a pull is every // `pr resubmit`, and two comments on the same pull routinely carry // different subject CIDs in the wild. It's recorded because the lexicon // requires it and because it says which revision was being replied to. let subject: StrongRef = StrongRef { uri: AtUri::new(pull.uri.clone().into())?, cid: Cid::new(cid.as_bytes())?, extra_data: None, }; let record = FeedComment::new() .subject(subject) .body(body) .created_at(Datetime::now()) // Zero-based, unlike --round: see FEED_COMMENT_NSID's doc comment. .pull_round_idx(Some(idx as i64)) .build();
crate::logging::debug::log(format!( ">> createRecord {}\n{}", crate::lexicon::tangled::FEED_COMMENT_NSID, crate::logging::debug::pretty(&record) )); // No rkey: the PDS mints a TID, which is what the lexicon's `key: "tid"` // asks for and what Tangled's own writer does. let output = agent.create_record(record, None).await.map_err(|e| { crate::logging::debug::dump_err("createRecord error", &e); scope_advice(&e.to_string()) })?;
// Bobbin's index runs hours behind and tangled.org's is separate, so the // comment not being on the page yet is expected rather than a failure. const LAG: &str = "Tangled indexes comments off the firehose; give it a moment to appear."; if args.json { report.uri = Some(output.uri.to_string()); crate::term::jsonout::emit(&report)?; crate::term::say::note!(Index, "{LAG}"); return Ok(()); } println!("wrote {}", output.uri); if let Some(repo_did) = target_repo_did(&serde_json::json!({ "value": pull.value })) { println!( "view: {}", crate::term::hyperlink::url(&format!("{}/{repo_did}/pulls", appview())) ); } crate::term::say::note!(Index, "{LAG}"); Ok(())}
#[cfg(test)]mod tests { use super::{CreatedJson, StateChangeJson}; use super::{PullRef, PullTarget, Status, comment_body, describe_body, new_body}; use super::{ body_edit_is_a_change, classify_pull_ref, is_already_merged, open_pull_from_branch, round_index, state_of, }; use crate::lexicon::tangled::PullState; use serde_json::Value;
// ----------------------------------------------------------------------- // pull state // -----------------------------------------------------------------------
/// A real page of `com.atproto.repo.listRecords` over the author's /// `sh.tangled.repo.pull.status` collection. Ten records, every one of /// them written by Tangled itself rather than by atgc, so the field names, /// the token spellings and the datetime formats are the live ones. const STATUS_PAGE: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/pull_statuses.json" ));
const ME: &str = "did:plc:nlzmjyfv6loqtxyzvdcznwgf";
/// Read the fixture the way `list_statuses` reads a live page, so the two /// agree about which fields carry the answer. fn fixture_statuses() -> Vec<Status> { serde_json::from_str::<Value>(STATUS_PAGE).unwrap()["records"] .as_array() .unwrap() .iter() .map(|r| Status { uri: r["uri"].as_str().unwrap().to_string(), created_at: r["value"]["createdAt"].as_str().unwrap().to_string(), status: r["value"]["status"].as_str().unwrap().to_string(), }) .collect() }
fn status(created_at: &str, rkey: &str, token: &str) -> Status { Status { uri: format!("at://{ME}/sh.tangled.repo.pull.status/{rkey}"), created_at: created_at.to_string(), status: token.to_string(), } }
/// Every record in the fixture parses to one of the three states, and /// every one of them is terminal. Tangled writes no record when a pull is /// opened — `open` is the lexicon's default and the state of a pull with /// no records at all — so a fixture with no `open` in it is not a gap. #[test] fn live_status_records_carry_the_tokens_this_build_knows() { let statuses = fixture_statuses(); assert_eq!(statuses.len(), 10); for s in &statuses { let state = PullState::from_token(&s.status) .unwrap_or_else(|| panic!("unknown status token {}", s.status)); assert!(matches!(state, PullState::Closed | PullState::Merged)); assert_eq!(state.token(), s.status); } }
/// The rule the appview applies: newest `createdAt` wins, ties go to the /// greater at-uri. This is what makes reopening an append rather than a /// delete, so it is the single most load-bearing assumption in `pr close`. #[test] fn the_newest_status_record_decides_the_state() { // No records at all is open — the common case, since opening a pull // writes nothing. assert_eq!(state_of(&[]), PullState::Open);
let closed = status("2026-08-06T10:00:00Z", "aaa", PullState::Closed.token()); assert_eq!(state_of(std::slice::from_ref(&closed)), PullState::Closed);
// Closed, then reopened: the later record wins, which is exactly the // shape `pr reopen` writes. let reopened = status("2026-08-06T11:00:00Z", "bbb", PullState::Open.token()); assert_eq!( state_of(&[closed.clone(), reopened.clone()]), PullState::Open ); // Order in the slice must not matter: records arrive from two PDSes. assert_eq!(state_of(&[reopened, closed.clone()]), PullState::Open);
// A merge after a close still reads as merged. let merged = status("2026-08-06T12:00:00Z", "ccc", PullState::Merged.token()); assert_eq!(state_of(&[closed, merged]), PullState::Merged); }
/// Two records written in the same millisecond: the appview breaks the tie /// on the at-uri, so this does too. Not hypothetical — Tangled writes one /// status record per pull in a stack from a single handler. #[test] fn a_tie_on_created_at_is_broken_by_the_record_uri() { let same = "2026-08-06T10:00:00Z"; let lower = status(same, "aaa", PullState::Closed.token()); let higher = status(same, "zzz", PullState::Open.token()); assert_eq!(state_of(&[lower.clone(), higher.clone()]), PullState::Open); assert_eq!(state_of(&[higher, lower]), PullState::Open); }
/// The bug that made `pr resubmit` refuse a live pull: a merge stamped /// `04:31:07+03:00` is 01:31Z, half an hour *before* a reopen stamped /// `02:00:00Z`, but its string is lexically larger. Compared as strings /// the merge won and the pull read `merged` forever, which is a state no /// record in the account actually asserts. #[test] fn an_earlier_merge_in_another_offset_loses_to_a_later_reopen() { let merged = status( "2026-08-06T04:31:07+03:00", "zzz", PullState::Merged.token(), ); let reopened = status("2026-08-06T02:00:00Z", "aaa", PullState::Open.token()); assert_eq!( state_of(&[merged.clone(), reopened.clone()]), PullState::Open ); assert_eq!(state_of(&[reopened, merged]), PullState::Open); }
/// The milder spelling of the same hazard, and the likelier one, since /// whether a client writes sub-second precision is not something atgc /// controls: `'.'` sorts below `'Z'`, so `…02:00:00.500Z` lost to /// `…02:00:00Z` half a second earlier. A close and a reopen from the same /// handler land inside one second often enough for this to matter. #[test] fn sub_second_precision_does_not_sink_a_record() { let closed = status("2026-08-06T02:00:00Z", "zzz", PullState::Closed.token()); let reopened = status("2026-08-06T02:00:00.500Z", "aaa", PullState::Open.token()); assert_eq!( state_of(&[closed.clone(), reopened.clone()]), PullState::Open ); assert_eq!(state_of(&[reopened, closed]), PullState::Open); }
/// A stamp nothing can parse must not win over a dated one. Pre-rounds /// records carry an empty `createdAt`, and a record that will not say /// when it was written is no evidence against one that does. #[test] fn an_unparseable_stamp_loses_to_a_dated_one() { let closed = status("2026-08-06T02:00:00Z", "aaa", PullState::Closed.token()); let undated = status("", "zzz", PullState::Open.token()); assert_eq!( state_of(&[closed.clone(), undated.clone()]), PullState::Closed ); assert_eq!(state_of(&[undated, closed]), PullState::Closed); }
/// A status variant this build has never heard of is skipped rather than /// allowed to win. The appview rejects unknown variants at ingest and /// never applies them, so honoring one here would report a state Tangled /// does not show. #[test] fn an_unknown_status_variant_does_not_decide_anything() { let closed = status("2026-08-06T10:00:00Z", "aaa", PullState::Closed.token()); let alien = status( "2026-08-06T23:00:00Z", "zzz", "sh.tangled.repo.pull.status.draft", ); assert_eq!(state_of(&[closed, alien.clone()]), PullState::Closed); // And with nothing else to fall back on, the default stands. assert_eq!(state_of(std::slice::from_ref(&alien)), PullState::Open); }
/// The local form: a bare record key is read as the acting account's own /// pull, which is what `pr resubmit` has always assumed and the only /// assumption available. #[test] fn a_bare_record_key_belongs_to_the_acting_account() { assert_eq!( classify_pull_ref("3msg7w7l6hs2x", ME).unwrap(), PullTarget::Local(PullRef { author: ME.to_string(), rkey: "3msg7w7l6hs2x".to_string(), }) ); // Surrounding whitespace comes from shells and copy-paste. assert_eq!( classify_pull_ref(" 3msg7w7l6hs2x ", ME).unwrap(), PullTarget::Local(PullRef { author: ME.to_string(), rkey: "3msg7w7l6hs2x".to_string(), }) ); }
/// An at-uri names its own author, which is what makes `pr close` usable /// on a pull filed against your repo by somebody else. #[test] fn an_at_uri_names_the_author_it_carries() { let other = "did:plc:wshs7t2adsemcrrd4snkeqli"; let uri = format!("at://{other}/sh.tangled.repo.pull/3msg7w7l6hs2x"); let PullTarget::Local(parsed) = classify_pull_ref(&uri, ME).unwrap() else { panic!("an at-uri needs no lookup"); }; assert_eq!(parsed.author, other); assert_eq!(parsed.rkey, "3msg7w7l6hs2x"); // And round-trips back to the URI it came from. assert_eq!(parsed.uri(), uri); }
/// THE ONE USERS WILL TYPE, and for four releases the one that was /// refused. `pr list` prints a `#` column and `pr view` prints a number, /// so the number is what the tool itself hands you; taking it back is the /// whole point of the classifier telling it apart from a record key. /// /// A leading zero is still the number it looks like: `0042` is 42, not a /// key. Only the lookup can say whether that pull exists. #[test] fn a_pull_number_is_a_number_and_not_a_record_key() { for (input, expected) in [("23", 23), ("1", 1), ("0042", 42), (" 7 ", 7)] { assert_eq!( classify_pull_ref(input, ME).unwrap(), PullTarget::Number { number: expected, repo_url: None, }, "{input:?}" ); } // Past u32 is not a pull number anybody has, and saying so here beats // a confusing 404 from the appview. let err = classify_pull_ref("99999999999999999999", ME) .expect_err("not a u32") .to_string(); assert!(err.contains("too large"), "{err}"); }
/// A pasted link is the number in it *and the repo it is a number in*, /// with or without the scheme — the address bar gives you one and a chat /// message the other. /// /// The repo half is the part that pins a bug: `pr close` and `pr edit` /// write, and resolving a stranger's link against the current checkout is /// how one of these verbs lands on the wrong pull request entirely. #[test] fn a_tangled_url_is_the_number_in_it_and_the_repo_it_is_in() { for (input, scheme) in [ ("https://tangled.org/@permadeath.com/atgc/pulls/23", "https"), ("http://tangled.org/@permadeath.com/atgc/pulls/23", "http"), ("tangled.org/@permadeath.com/atgc/pulls/23", "https"), // The round sub-page is still pull 23 of the same repo. ( "https://tangled.org/@permadeath.com/atgc/pulls/23/round/1", "https", ), ] { assert_eq!( classify_pull_ref(input, ME).unwrap(), PullTarget::Number { number: 23, repo_url: Some(format!("{scheme}://tangled.org/@permadeath.com/atgc")), }, "{input:?}" ); } }
/// The other ways a reference can be wrong, each refused before anything /// leaves the machine. #[test] fn a_reference_that_cannot_name_a_pull_is_refused() { for (input, expected) in [ ("", "no pull request given"), ("at://", "not a complete pull at-uri"), ( &format!("at://{ME}/sh.tangled.repo.pull"), "not a complete pull at-uri", ), // Right shape, wrong collection: this is an issue, not a pull. ( &format!("at://{ME}/sh.tangled.repo.issue/3msg7w7l6hs2x"), "not a sh.tangled.repo.pull", ), // A handle authority is legal in an at-uri and refused anyway: // handles change hands, and this decides whose pull is closed. ( "at://permadeath.com/sh.tangled.repo.pull/3msg7w7l6hs2x", "can change hands", ), // A malformed DID stops at the identifier classifier. ( "at://did:plc:short/sh.tangled.repo.pull/3msg7w7l6hs2x", "24 characters of base32", ), // A URL is only an answer when it names a pull. The repo's own // page does not, and is refused rather than guessed at. ( "https://tangled.org/@permadeath.com/atgc", "not a pull request reference", ), // `<handle>/<rkey>` is a spelling `pr diff` takes and this half // does not, for the same reason it refuses a handle authority: // the string decides whose pull is about to be closed. ( "permadeath.com/3msg7w7l6hs2x", "not a pull request reference", ), ] { let err = classify_pull_ref(input, ME).expect_err(input).to_string(); assert!( err.contains(expected), "{input:?}: expected {expected:?}, got {err}" ); } }
/// `pr edit` has three states for the body and they are all different: /// not mentioned, set to something, and emptied. Collapsing the last two /// would make `--body ''` a no-op and leave a wrong body in place. #[test] fn a_body_can_be_set_cleared_or_left_alone() { assert_eq!(new_body(None, None).unwrap(), None); assert_eq!( new_body(Some("new text".to_string()), None).unwrap(), Some(Some("new text".to_string())) ); // Empty, or only whitespace, clears it. assert_eq!(new_body(Some(String::new()), None).unwrap(), Some(None)); assert_eq!( new_body(Some(" \n ".to_string()), None).unwrap(), Some(None) );
let err = new_body(Some("a".to_string()), Some("f".to_string())) .expect_err("two sources for one field") .to_string(); assert!(err.contains("not both"), "{err}"); }
/// A body is reported by size rather than echoed: it is arbitrarily long /// and the line's job is to say that it moved. #[test] fn a_body_change_is_described_not_printed() { assert_eq!(describe_body(None), "(none)"); assert_eq!(describe_body(Some("hello")), "5 bytes"); }
/// The whole point of the conversion: atgc counts rounds from 1, the /// record counts them from 0, and the default is the newest round. /// /// If this ever inverts, every comment atgc writes lands one round early — /// on a three-round pull that means commenting on the code the author /// already replaced, which reads as a reviewer who did not look. #[test] fn a_round_flag_is_one_based_and_the_record_is_zero_based() { assert_eq!(round_index(None, 1).unwrap(), 0); assert_eq!(round_index(None, 4).unwrap(), 3, "default is the latest"); assert_eq!(round_index(Some(1), 4).unwrap(), 0); assert_eq!(round_index(Some(4), 4).unwrap(), 3); }
/// Round 0 is refused rather than read as the first round, because /// Tangled's URLs *do* call the first round 0 and someone copying one /// across would otherwise silently comment on the round before the one /// they were looking at — or, on a one-round pull, on nothing. #[test] fn round_zero_is_refused_and_says_why() { let err = round_index(Some(0), 3).expect_err("no round 0").to_string(); assert!(err.contains("numbered from 1"), "{err}"); assert!(err.contains("Tangled"), "{err}");
let err = round_index(Some(5), 3) .expect_err("past the end") .to_string(); assert!(err.contains("3 round(s)"), "{err}"); }
/// An empty comment is refused here rather than at the appview, which /// would drop it at ingest without telling anyone. Unlike `pr edit`'s /// body, empty has no second meaning: there is no comment to clear. #[test] fn an_empty_comment_is_refused() { for empty in ["", " ", "\n\t \n"] { let err = comment_body(Some(empty.to_string()), None) .expect_err("empty comment") .to_string(); assert!(err.contains("needs a body"), "{empty:?} gave {err}"); } // Whitespace *around* real text is kept: markdown is // whitespace-sensitive and trimming someone's code fence is not this // function's call to make. assert_eq!( comment_body(Some("\n hi\n".to_string()), None).unwrap(), "\n hi\n" ); }
/// No body at all is a different error from an empty one, and it is the /// one that has to mention `--body-file -`, since the reason a user hits /// it is usually that they went looking for the editor there isn't. #[test] fn a_missing_comment_body_points_at_body_file() { let err = comment_body(None, None) .expect_err("nothing to say") .to_string(); assert!(err.contains("--body-file"), "{err}"); assert!(err.contains("editor"), "{err}");
let err = comment_body(Some("a".to_string()), Some("f".to_string())) .expect_err("two sources for one body") .to_string(); assert!(err.contains("not both"), "{err}"); }
// ----------------------------------------------------------------------- // pr edit's change decision // -----------------------------------------------------------------------
/// Same text, no images: nothing changed, and `pr edit` should say so. #[test] fn identical_body_with_no_images_is_not_a_change() { let body = Some("unchanged".to_string()); assert!(!body_edit_is_a_change(&body, &body, false)); }
/// Same text, but the body embeds a local image: this is the case the /// surrounding comment exists to explain. Equal text still counts as a /// change, because resending it is what uploads the image and rewrites /// the path to a `blob+at://` URI. #[test] fn identical_body_with_images_is_still_a_change() { let body = Some("".to_string()); assert!(body_edit_is_a_change(&body, &body, true)); }
/// Different text is always a change, images or not. #[test] fn different_body_text_is_a_change() { let new = Some("new text".to_string()); let stored = Some("old text".to_string()); assert!(body_edit_is_a_change(&new, &stored, false)); }
// ----------------------------------------------------------------------- // resubmit's merged-pull refusal // -----------------------------------------------------------------------
fn row(uri: &str, state: &str) -> crate::cmd::pr::read::StackRow { crate::cmd::pr::read::StackRow { item: serde_json::json!({"uri": uri}), state: state.to_string(), } }
/// The exact regression the surrounding comment records: the row for /// this pull reads "merged", so appending a round refuses. #[test] fn a_pull_whose_row_reads_merged_is_already_merged() { let rows = [row("at://did:plc:me/sh.tangled.repo.pull/a", "merged")]; assert!(is_already_merged( &rows, "at://did:plc:me/sh.tangled.repo.pull/a" )); }
/// An open pull is not refused. #[test] fn an_open_pull_is_not_already_merged() { let rows = [row("at://did:plc:me/sh.tangled.repo.pull/a", "open")]; assert!(!is_already_merged( &rows, "at://did:plc:me/sh.tangled.repo.pull/a" )); }
/// No row at all for this uri — the common case, since most pulls are /// not stack members and never appear in this listing — is not merged /// either; the guard must not mistake absence for a match. #[test] fn a_pull_with_no_row_is_not_already_merged() { let rows = [row("at://did:plc:me/sh.tangled.repo.pull/other", "merged")]; assert!(!is_already_merged( &rows, "at://did:plc:me/sh.tangled.repo.pull/a" )); }
/// The guard is exact-match on the literal state string: "closed" is /// terminal too, but it is not this refusal's job. #[test] fn a_closed_pull_is_not_already_merged() { let rows = [row("at://did:plc:me/sh.tangled.repo.pull/a", "closed")]; assert!(!is_already_merged( &rows, "at://did:plc:me/sh.tangled.repo.pull/a" )); }
// ----------------------------------------------------------------------- // create's "you are targeting a branch under review" warning // -----------------------------------------------------------------------
fn sourced(branch: &str, title: &str, state: &str) -> crate::cmd::pr::read::StackRow { crate::cmd::pr::read::StackRow { item: serde_json::json!({ "uri": format!("at://did:plc:me/sh.tangled.repo.pull/{branch}"), "value": {"title": title, "source": {"branch": branch}}, }), state: state.to_string(), } }
/// The accident this warns about, as it happened: a pull was opened /// against `claude/matches-entry-cards` while the pull that branch came /// off was still open. That one merged, the branch went with it, and the /// pull left behind could never take another round. #[test] fn a_target_that_another_open_pull_came_off_is_found() { let rows = [ sourced("claude/other-work", "Unrelated", "open"), sourced("claude/matches-entry-cards", "Matches entry cards", "open"), ]; let found = open_pull_from_branch(&rows, "claude/matches-entry-cards") .expect("the pull that branch came off"); assert_eq!(found["value"]["title"], "Matches entry cards"); }
/// A pull that already landed or was abandoned takes its hazard with it: /// the branch is either gone already, in which case the resubmit refusal /// is what speaks, or it was never going to move. #[test] fn a_target_whose_pull_is_finished_is_not_warned_about() { for state in ["merged", "closed"] { let rows = [sourced("claude/landed", "Landed", state)]; assert!(open_pull_from_branch(&rows, "claude/landed").is_none()); } }
/// A fresh pull has no status record anywhere and reads `?`. That is a /// pull whose branch is very much still in play, so it warns. #[test] fn a_target_whose_pull_has_no_known_state_still_warns() { let rows = [sourced("claude/fresh", "Fresh", "?")]; assert!(open_pull_from_branch(&rows, "claude/fresh").is_some()); }
/// The ordinary case — targeting main, which no pull is the source of — /// says nothing. A warning on every create would be worth less than none. #[test] fn the_usual_target_is_not_warned_about() { let rows = [sourced("claude/some-feature", "Some feature", "open")]; assert!(open_pull_from_branch(&rows, "main").is_none()); }
// ----------------------------------------------------------------------- // FeedComment / Markdown — generated types // -----------------------------------------------------------------------
use super::{FeedComment, StrongRef}; use crate::lexicon::tangled::markdown_plain; use crate::lexicon::tangled::{FEED_COMMENT_NSID, MARKDOWN_NSID, PULL_NSID}; use jacquard::types::string::{AtUri, Cid, Datetime}; use tangled_lexicon::sh_tangled::markup::markdown::Markdown;
/// Real `sh.tangled.feed.comment` records, straight off a commenter's PDS /// rather than through Bobbin — Bobbin's list envelope emits `$type` /// twice inside `value`, and what atgc has to match is the record as the /// PDS stores it. const COMMENT_RECORDS: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/feed_comment_records.json" ));
/// Every comment on one real pull request, via Bobbin's /// `sh.tangled.feed.listComments`. Four commenters, two rounds. const COMMENTS_ON_PULL: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/feed_comments_on_pull.json" ));
/// The shape `pr comment` writes has to be the shape Tangled's web UI /// writes, field for field — the appview reads both with one parser and a /// record it does not recognise is dropped at ingest with no feedback to /// the writer. Also proves the generated `extra_data` catch-all actually /// carries `body.$type` back out on serialize, since the generated /// `Markdown` type doesn't model that field itself (see /// `markdown_plain`'s doc comment). #[test] fn a_live_comment_record_round_trips() { let records = serde_json::from_str::<Value>(COMMENT_RECORDS).unwrap()["records"] .as_array() .unwrap() .clone(); assert!(!records.is_empty()); for record in &records { let original = record["value"].clone(); let comment: FeedComment = serde_json::from_value(original.clone()).expect("live record parses"); assert_eq!( comment .body .extra_data .as_ref() .and_then(|e| e.get("$type")) .and_then(|d| d.as_str()), Some(MARKDOWN_NSID) ); assert!(comment.subject.uri.as_str().starts_with("at://")); assert!(!comment.subject.cid.as_str().is_empty()); assert_eq!(serde_json::to_value(&comment).unwrap(), original); } }
/// A comment body anchors its images in `markup.markdown`'s `blobs`, on /// the same terms as a pull's — and a body without any writes no field /// at all, so plain comments keep their exact wire shape. #[test] fn a_markdown_body_carries_blobs_only_when_it_has_them() { let plain = serde_json::to_value(markdown_plain("hello".to_string())).unwrap(); assert!(plain.get("blobs").is_none());
let uri = "blob+at://did:plc:a/bafkreieptvktqly4bb3o2rtqp6tudeyprj4dmtkqqtw5kkqeglvfngdj6m"; let with = serde_json::json!({ "$type": MARKDOWN_NSID, "text": format!(""), "original": format!(""), "blobs": [{ "$type": "blob", "ref": {"$link": "bafkreieptvktqly4bb3o2rtqp6tudeyprj4dmtkqqtw5kkqeglvfngdj6m"}, "mimeType": "image/png", "size": 20730 }] }); let parsed: Markdown = serde_json::from_value(with.clone()).expect("parses"); assert_eq!(parsed.blobs.as_ref().map(Vec::len), Some(1)); assert_eq!(serde_json::to_value(&parsed).unwrap(), with); }
/// Every pull comment carries a round index, and it is zero-based. /// /// This is the field the legacy `sh.tangled.repo.pull.comment` record has /// no room for, and the reason writing one is pointless: the appview /// requires it when the subject is a pull, and drops comments whose index /// is missing. A fixture with a `pullRoundIdx` of 0 in it is also the /// proof that the numbering starts at zero, which is not what atgc's /// `--round` flags use. #[test] fn pull_comments_carry_a_zero_based_round_index() { let items = serde_json::from_str::<Value>(COMMENTS_ON_PULL).unwrap()["items"] .as_array() .unwrap() .clone(); assert!(items.len() >= 2, "fixture should have several comments");
let mut lowest = i64::MAX; for item in &items { let comment: FeedComment = serde_json::from_value(item["value"].clone()) .expect("a Bobbin list item's value is the record"); let collection = comment.subject.uri.as_str().split('/').nth(3).unwrap(); assert_eq!(collection, PULL_NSID, "fixture should be a pull's comments"); let idx = comment .pull_round_idx .expect("a pull comment without a round index is dropped by the appview"); lowest = lowest.min(idx); } assert_eq!(lowest, 0, "round indices count from zero, not from one"); }
/// The subject CID is a snapshot, not a precondition. /// /// Appending a round rewrites the pull record and so changes its CID, and /// nothing reconciles the comments already pointing at the old one. Two /// comments on the same pull naming different CIDs is therefore normal, /// and this fixture contains exactly that — pinned so that nobody later /// "fixes" `pr comment` by validating the CID against the pull's current /// one and starts refusing to comment on any pull that has been resubmitted. #[test] fn comments_on_one_pull_may_name_different_subject_cids() { use std::collections::BTreeSet;
let items = serde_json::from_str::<Value>(COMMENTS_ON_PULL).unwrap()["items"] .as_array() .unwrap() .clone(); let mut uris = BTreeSet::new(); let mut cids = BTreeSet::new(); for item in &items { let comment: FeedComment = serde_json::from_value(item["value"].clone()).unwrap(); uris.insert(comment.subject.uri.as_str().to_string()); cids.insert(comment.subject.cid.as_str().to_string()); } assert_eq!(uris.len(), 1, "fixture is one pull's comments"); assert!( cids.len() > 1, "fixture should span a resubmit, so the subject CID moved" ); }
/// `markdown_plain` writes both spellings of the body. `original` is what /// the appview resolves mentions and references out of, so dropping it /// would silently stop `@handle` in a comment from notifying anyone. #[test] fn a_plain_body_writes_text_and_original_alike() { let body = markdown_plain("ping @permadeath.com".to_string()); let json = serde_json::to_value(&body).unwrap(); assert_eq!(json["$type"], MARKDOWN_NSID); assert_eq!(json["text"], "ping @permadeath.com"); assert_eq!(json["original"], "ping @permadeath.com"); assert_eq!(json.as_object().unwrap().len(), 3, "no other fields"); }
/// An issue comment has no round index, and the field must stay absent /// rather than be written as an explicit null — the appview parses it as a /// `*int` and a null is not the same as a missing key to a validating PDS. #[test] fn a_comment_without_a_round_omits_the_field() { let subject: StrongRef = StrongRef { uri: AtUri::new("at://did:plc:x/sh.tangled.repo.issue/3lxyz".into()).unwrap(), cid: Cid::new(b"bafyreiaaaa").unwrap(), extra_data: None, }; let comment = FeedComment::new() .subject(subject) .body(markdown_plain("hi".to_string())) .created_at(Datetime::now()) .build(); let json = serde_json::to_value(&comment).unwrap(); assert_eq!(json.get("$type").unwrap(), FEED_COMMENT_NSID); assert!(json.get("pullRoundIdx").is_none(), "got {json}"); }
// ----------------------------------------------------------------------- // Pull / Round / Source / Target — generated types // -----------------------------------------------------------------------
use super::{Pull, Source};
/// A real `sh.tangled.repo.pull` as it sits in a PDS today, fetched from /// the public listRecords endpoint. Four rounds, so `pr resubmit`'s /// append-and-put is exercised against a record that has been through it. const NEW_RECORD: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/pull_new_record.json" ));
/// A real record carrying a `source`. Worth a fixture of its own because /// no record has both: `atgc pr create` sends patch-based PRs and never /// writes a `source`, so every record that has one was opened from a /// branch through Tangled's web UI instead. const WITH_SOURCE: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/pull_new_with_source.json" ));
/// A real pull record from before the rounds migration, off tangled.org's /// own PDS. Inline `patch`, `targetRepo` as an at-uri, `targetBranch` /// alongside it, no `target`, no `rounds` — and an empty `createdAt`. const OLD_RECORD: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/pull_old_record.json" ));
fn pull_value_of(record: &str) -> Value { serde_json::from_str::<Value>(record).unwrap()["value"].clone() }
/// The shape `pr resubmit` depends on: read a record back off the PDS, /// push a round onto it, put it again. Everything it touches has to /// survive the round trip unchanged — including `target.repoDid`, which /// the current lexicon no longer names as a field at all (see /// `crate::lexicon::tangled::pull_target`) but this fixture, like every record /// written before the rename, still carries; it has to round-trip /// through `extra_data` since there's no named field to hold it. #[test] fn a_live_record_round_trips() { let original = pull_value_of(NEW_RECORD); let pull: Pull = serde_json::from_value(original.clone()).expect("live record parses");
assert_eq!( pull.target .extra_data .as_ref() .and_then(|e| e.get("repoDid")) .and_then(|d| d.as_str()), Some(pull.target.repo.as_str()), ); assert!(pull.rounds.len() >= 3, "fixture should be multi-round");
let reserialized = serde_json::to_value(&pull).unwrap(); assert_eq!( reserialized, original, "resubmit puts this back; it must not lose or rename a field" ); }
/// The properties the lexicon defines and this struct does not name as /// its own fields. `mentions` and `references` are themselves generated, /// typed fields — unlike the hand-written struct, which kept them in an /// untyped catch-all — so this now also confirms they deserialize to /// the right shape (a `Did` list, an `AtUri` list) rather than merely /// surviving as opaque JSON. /// /// Built by hand rather than captured, because no fixture has one: atgc /// never writes these, and the records it resubmits are its own. /// /// `dependentOn` graduated from a catch-all-only property to a named /// field for the stack commands, so this also pins where each property /// lands: the named field is populated, and the wire shape is identical /// either way. #[test] fn resubmit_preserves_the_properties_the_struct_does_not_name() { let mut original = pull_value_of(NEW_RECORD); let obj = original.as_object_mut().unwrap(); obj.insert( "mentions".into(), serde_json::json!(["did:plc:nlzmjyfv6loqtxyzvdcznwgf"]), ); obj.insert( "references".into(), serde_json::json!(["at://did:plc:abc/sh.tangled.repo.pull/xyz"]), ); obj.insert("dependentOn".into(), serde_json::json!("at://did:plc:abc")); obj.insert("blobs".into(), serde_json::json!([]));
let pull: Pull = serde_json::from_value(original.clone()).expect("parses"); assert_eq!( pull.mentions.as_ref().map(Vec::len), Some(1), "mentions is a named, typed field now" ); assert_eq!( pull.references.as_ref().map(Vec::len), Some(1), "references is a named, typed field now" ); assert_eq!( pull.dependent_on.as_ref().map(|u| u.as_str()), Some("at://did:plc:abc"), "the named field takes it, so a restack can read and rewrite it" ); assert!( pull.blobs.is_some(), "blobs graduated to a named field for body images" );
let reserialized = serde_json::to_value(&pull).unwrap(); assert_eq!( reserialized, original, "a property the struct does not name still has to survive resubmit" ); }
/// A record atgc wrote itself has nothing extra, and `flatten` must not /// add an empty object to what goes on the wire. #[test] fn an_empty_catch_all_serializes_to_nothing() { let original = pull_value_of(NEW_RECORD); let pull: Pull = serde_json::from_value(original.clone()).expect("parses"); assert!( pull.mentions.is_none() && pull.references.is_none(), "fixture was written by atgc" ); assert_eq!(serde_json::to_value(&pull).unwrap(), original); }
/// The same round trip for a record that carries a `source`, which the /// primary fixture predates. #[test] fn a_record_with_a_source_round_trips_too() { let original = pull_value_of(WITH_SOURCE); let pull: Pull = serde_json::from_value(original.clone()).expect("parses"); let source = pull.source.as_ref().expect("fixture carries a source"); assert!(!source.branch.as_str().is_empty()); assert_eq!(serde_json::to_value(&pull).unwrap(), original); }
/// Repos have had their own DIDs since knot v1.13, but the field that /// carries it was renamed and live records still carry both spellings. /// `crate::lexicon::tangled::pull_target` writes both — pinned here because it is /// invisible from the generated type on its own, which as of the current /// lexicon has only `repo` to hold the value; `repoDid` goes in through /// `extra_data`. #[test] fn target_writes_the_repo_did_under_both_names() { let did = "did:plc:gspkabpde4kx47fj3bhiwrms"; let target = crate::lexicon::tangled::pull_target(did, "main").unwrap(); let json = serde_json::to_value(&target).unwrap(); assert_eq!(json["repo"], did); assert_eq!(json["repoDid"], did); assert_eq!(json["branch"], "main"); }
/// Every `sh.tangled.repo.pull` in the wild that carries a source carries /// exactly `{"branch": …}`, so that is what `pr create` writes: the /// optional `repo` stays off the wire rather than going out as the target /// repo's own DID, which would be true and would still be a shape nothing /// else produces. #[test] fn the_source_written_is_a_branch_and_only_a_branch() { let source: Source = Source { branch: "claude/pr-source-branch".into(), repo: None, extra_data: None, }; let json = serde_json::to_value(source).unwrap(); assert_eq!( json, serde_json::json!({ "branch": "claude/pr-source-branch" }) ); }
/// `$type` is defaulted on the way in and always written on the way out, /// so a record fetched without it still puts back as a valid one. #[test] fn record_type_defaults_and_is_always_written() { let mut value = pull_value_of(NEW_RECORD); value.as_object_mut().unwrap().remove("$type"); let pull: Pull = serde_json::from_value(value).expect("parses without $type"); assert_eq!(serde_json::to_value(&pull).unwrap()["$type"], PULL_NSID); }
/// Absent optionals stay absent rather than becoming explicit nulls — a /// null is not the same thing as no key at all to a validating PDS, and /// records written before atgc recorded a source have no key there. #[test] fn absent_optionals_are_omitted_not_nulled() { let mut value = pull_value_of(NEW_RECORD); let obj = value.as_object_mut().unwrap(); obj.remove("body"); obj.remove("source"); let pull: Pull = serde_json::from_value(value).unwrap(); let json = serde_json::to_value(&pull).unwrap(); assert!(json.get("body").is_none(), "got {json}"); assert!(json.get("source").is_none(), "got {json}"); }
/// Where the tolerance actually ends. /// /// `pr list`, `status pr` and `pr view` read pulls as untyped JSON and so /// cope with old records — they check for `rounds` and fall back. `pr /// resubmit` is the one command that parses into `Pull`, and `Pull` /// requires `target` and `rounds`, neither of which an old record has — /// true of the generated type exactly as it was of the hand-written one. /// /// So resubmitting onto a pre-rounds PR fails at the parse. That is /// arguably correct — there is no `rounds` array to append to, and the /// records belong to whoever filed them years ago — but it is a real /// limit, and the error a user sees is "pull record <rkey> did not /// parse", which does not say so. Pinned so the limit is deliberate and /// so anyone widening `Pull` finds out here. #[test] fn old_records_do_not_parse_as_pull() { let value = pull_value_of(OLD_RECORD); assert!( value.get("rounds").is_none(), "fixture should be pre-rounds" ); assert!( value.get("target").is_none(), "fixture should be pre-target" ); assert!( value["patch"].is_string(), "fixture should inline its patch" );
let parsed = serde_json::from_value::<Pull>(value); assert!( parsed.is_err(), "an old record parsing cleanly would mean Pull had grown defaults \ that quietly write an empty rounds array back over someone's PR" ); }
// ----------------------------------------------------------------------- // PullStatus / PullState — generated types // -----------------------------------------------------------------------
use super::{PullStatus, StatusStatus};
#[test] fn a_live_status_record_round_trips() { let records = serde_json::from_str::<Value>(STATUS_PAGE).unwrap()["records"] .as_array() .unwrap() .clone(); assert!(!records.is_empty()); for record in &records { let original = record["value"].clone(); let status: PullStatus = serde_json::from_value(original.clone()).expect("live record parses"); assert!(status.pull.as_str().starts_with("at://")); // Field for field, including `$type` and the camelCase spelling of // createdAt: anything atgc writes has to be indistinguishable from // what Tangled writes, or the appview reads it differently. assert_eq!(serde_json::to_value(&status).unwrap(), original); } }
/// The contract `state_of` (above) and `latest_states` /// (`crate::cmd::pr::read`) both depend on: a status record whose token this /// build doesn't recognize must still deserialize — into /// `StatusStatus::Other`, never a parse error — so the record can be /// seen and skipped rather than making the whole listing unreadable. /// `PullState::from_token`, given that variant's `as_str()`, then /// agrees it names nothing, which is what lets both call sites filter /// it out instead of crashing on it. #[test] fn an_unrecognized_status_token_deserializes_to_other_and_names_no_state() { let record = serde_json::json!({ "$type": "sh.tangled.repo.pull.status", "pull": "at://did:plc:x/sh.tangled.repo.pull/3abc", "status": "sh.tangled.repo.pull.status.abandoned", "createdAt": "2026-08-11T00:00:00.000Z", }); let status: PullStatus = serde_json::from_value(record).expect("an unrecognized token still parses"); assert!(matches!(status.status, StatusStatus::Other(_))); assert_eq!( PullState::from_token(status.status.as_str()), None, "an unrecognized token must not resolve to a state" ); }
// ----------------------------------------------------------------------- // --json // -----------------------------------------------------------------------
/// The one field every writing command's `--json` shares, and the rule /// that goes with it: a dry run says `dry_run: true` and leaves the /// identifiers it did not mint `null`, present rather than omitted. A /// caller checking `.uri` must not have to also check `has("uri")`, and /// a record key is the PDS's to choose — guessing one would be the most /// misleading thing this output could do. #[test] fn a_dry_run_reports_itself_and_mints_no_identifiers() { let created = CreatedJson { dry_run: true, uri: None, url: None, title: "Add --json everywhere".to_string(), repo_did: "did:plc:repo".to_string(), target_branch: "main".to_string(), source_branch: "claude/json-flags".to_string(), source_recorded: true, pushed: true, commits: 3, patch_bytes: 5145, patch_gzip_bytes: 1200, images: Vec::new(), }; let value = serde_json::to_value(&created).unwrap(); assert_eq!(value["dry_run"], true); assert_eq!(value["uri"], Value::Null); assert_eq!(value["url"], Value::Null); assert!(value.get("uri").is_some(), "null, not absent"); assert_eq!(value["images"], serde_json::json!([]));
let mut keys: Vec<&str> = value .as_object() .unwrap() .keys() .map(String::as_str) .collect(); keys.sort_unstable(); assert_eq!( keys, vec![ "commits", "dry_run", "images", "patch_bytes", "patch_gzip_bytes", "pushed", "repo_did", "source_branch", "source_recorded", "target_branch", "title", "uri", "url", ] ); }
/// `pr close` on an already-closed pull writes nothing and exits 0, and /// so does a dry run — `changed` is the field that tells a caller which /// of the two happened, since neither has a status record to point at. #[test] fn a_state_change_that_wrote_nothing_says_so_in_changed() { let noop = StateChangeJson { dry_run: false, uri: None, url: Some("https://tangled.org/did:plc:repo/pulls".to_string()), pull_uri: "at://did:plc:me/sh.tangled.repo.pull/3abc".to_string(), rkey: "3abc".to_string(), title: "Add --json everywhere".to_string(), author_did: "did:plc:me".to_string(), state_before: "closed".to_string(), state_after: "closed".to_string(), changed: false, }; let value = serde_json::to_value(&noop).unwrap(); assert_eq!(value["changed"], false); assert_eq!(value["dry_run"], false); assert_eq!(value["uri"], Value::Null, "nothing was appended to the log"); assert_eq!(value["state_before"], value["state_after"]); }}