//! Listing issues: `atgc issue list` and `issue view`. //! //! The read half of [`crate::cmd::issue`], and — like [`crate::cmd::pr::read`], //! whose module documentation argues the same problem at length for pulls — //! nothing in here authenticates. A `sh.tangled.repo.issue` record is public, //! `com.atproto.repo.listRecords` is public, and a session buys this half one //! thing: a DID whose PDS is worth reading. `--author` spends that on somebody //! else's DID instead, and works for accounts nobody here has ever logged in //! to. //! //! # What this half can and cannot answer //! //! An issue record lives in the PDS of the person who *filed* it, not in the //! repo owner's. That is the whole shape of the problem: //! //! - **Your own issues are complete and immediate.** A `listRecords` walk //! over one collection, filtered to the repo you asked about, visible the //! instant the write returns and never behind. Complete is a property of //! the walk and not of the source, though: under `--state` it reads the //! whole collection, and without one it stops at `--limit` and the note //! under the listing says so. //! - **Everyone else's issues on a repo are unanswerable from here.** They //! are scattered across the PDSes of people nothing enumerates — not your //! records, not the knot's — so "every issue on this repo, whoever opened //! it" needs a service that has consumed the firehose. atgc does not ask //! one yet, and the listing says so out loud rather than printing your //! rows under a heading that reads like the repo's. //! //! One service has: Bobbin, and `--source bobbin` is how this half asks it, //! on the same opt-in terms `pr list` uses and for the same reason — the //! index is alpha, its ingest stalls, and a listing that is honestly //! author-scoped beats one that is dishonestly repo-scoped. The merged row //! is [`Listed`], which carries provenance-free record data beside a //! [`State`] that already admits to being unknown. //! //! The third source `pr list` has is not here. `web` is a scrape of //! tangled.org's rendered pull request pages and there is no issue page //! behind it, so [`crate::cmd::pr::read::Source::parse_arg_without_web`] refuses the word rather //! than accepting it and reading one source fewer than the caller asked //! for. //! //! # State is a log, and one PDS is not all of it //! //! An issue's state is whichever `sh.tangled.repo.issue.state` record is //! newest, and Tangled honours one written by the issue's author, the repo's //! owner, or a collaborator. Reading one PDS therefore settles the state only //! when that PDS is the only place an honoured record could be — see //! [`state_of`], which is careful about the difference and reports `?` rather //! than guessing `open`. use crate::clients::atproto::pds::{Evidence, Listing, Reach}; use crate::clients::atproto::record::Key; use crate::clients::git::run as git; use crate::clients::tangled::resolve; use crate::cmd::listing::cuts; use crate::lexicon::tangled::{ISSUE_NSID, ISSUE_STATE_NSID, IssueState}; use crate::term::column::{cell, day, ellipsize, pad_to}; use crate::term::text::one_line; use anyhow::Result; use jacquard::types::string::Datetime; use std::collections::HashMap; use std::str::FromStr; /// Tangled's website, where every `view:` link points. The address and its /// override are [`crate::clients::endpoints`]'s. fn appview() -> String { crate::clients::endpoints::appview() } /// The repo's issue listing on the appview. /// /// Built from the repo's own DID rather than from an `owner/name` lookup: /// tangled.org takes a repo DID as a whole path and redirects to the /// canonical page, which is the same shape `pr close` prints its `view:` line /// with. It is a link to the *listing*, never to the issue — an issue's own /// page is `/issues/`, and the number is the appview's alone. pub(in crate::cmd) fn issues_url(repo_did: &str) -> String { format!("{}/issues", repo_url(repo_did)) } /// The repo's own page. `tangled.org/` 302s to /// `tangled.org//`, so a DID is a usable link target /// even where no handle has been resolved. pub(super) fn repo_url(repo_did: &str) -> String { format!("{}/{repo_did}", appview()) } // --------------------------------------------------------------------------- // Naming an issue // --------------------------------------------------------------------------- /// Which issue a command was told to act on. /// /// An issue 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 — or, when `--author` names one, that account. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct IssueRef { /// The *reporter's* DID: the at-uri authority, and so the PDS the record /// lives in. Not necessarily the account doing the reading or writing. pub author: String, pub rkey: String, } impl IssueRef { pub(super) fn uri(&self) -> String { format!("at://{}/{ISSUE_NSID}/{}", self.author, self.rkey) } } /// Turn what a user typed into an issue reference, or explain why it cannot /// be one. /// /// Pure, and it stays pure, which is the one way this is simpler than /// [`crate::cmd::pr::write`]'s `classify_pull_ref`: that one has a network /// case, because `pr close 23` has to translate an appview pull number by /// fetching the page it names. Every spelling accepted here names the record /// by itself, so nothing needs asking. /// /// The number is refused rather than resolved, and that is a decision with a /// cost — `23` is the first thing anybody types, because it is what the web /// UI shows. See [`no_number`] for what it costs and why it was still the /// right way round for a first cut. pub(super) fn classify_issue_ref(input: &str, author: &str) -> Result { // Every refusal below is `Usage`: the command line does not name an // issue, and no amount of retrying or 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 issue given: pass its 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 issue at-uri: it needs an authority, a \ collection and a record key, as in at://did:plc:…/{ISSUE_NSID}/3lxyz…" ))); }; if collection != ISSUE_NSID { return Err(usage(format!( "{input} names a {collection} record, not a {ISSUE_NSID}: this command \ acts on issues" ))); } // A handle is legal in an at-uri authority and is refused here rather // than resolved, for the reason `pr` refuses one: which account a // handle names can change hands, and this string decides whose PDS is // read and whose issue is about to be closed. 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 an issue's owner: use the at:// URI \ with the author's DID in it, which is the form `atgc issue list` prints" ))); } }; return Ok(IssueRef { 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 Err(no_number(input)); } // A pasted link. Recognised only so that it can be refused with the // number's explanation rather than with "that is not a record key", // which would send somebody looking for a typo they did not make. if input.contains('/') { if input.split('/').any(|segment| segment == "issues") { return Err(no_number(input)); } return Err(usage(format!( "{input} is not an issue reference: pass the record key or the at:// URI, \ both of which `atgc issue view --json` prints" ))); } // Anything left is taken as a record key in `author`'s repository. // 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(IssueRef { author: author.to_string(), rkey: input.to_string(), }) } /// The refusal a Tangled issue number earns, and the reason it is a refusal. /// /// An issue's number is the appview's own database id, exactly as a pull's /// is: it is in no record, in no XRPC response, and the only way back from it /// is to open the page it names and read the record URI off the markup. atgc /// does that for pulls, and TODO.md has carried a line for four releases /// saying that the scrape is a bet on somebody else's HTML which has already /// been observed to fail. /// /// Making a second bet of the same kind, to ship a first cut of a command /// family, is not a trade worth taking blind — so this says what it cannot do /// and names the two spellings that work. Refusing is cheap to reverse; a /// scrape that silently resolves the wrong issue is what `issue close` would /// then act on. fn no_number(input: &str) -> anyhow::Error { crate::exit::fail( crate::exit::Exit::Usage, format!( "{input} looks like a Tangled issue number, and atgc cannot turn one into a \ record yet\n\ the number is the appview's own id: it is in no record and no XRPC response, \ so the only way back is to read it off the page\n\ name the issue by its record key or its at:// URI; `atgc issue list` prints both" ), ) } // --------------------------------------------------------------------------- // Reading one issue // --------------------------------------------------------------------------- /// An issue record as it was read back, with everything a command needs to /// name it again. /// /// The CID is why this is a struct rather than a bare value: `issue edit` is /// a read-modify-write against a record something else could be writing at /// the same time, and `putRecord`'s `swapRecord` precondition is what makes /// that fail rather than clobber. See [`crate::clients::atproto::record::put`]. #[derive(Debug, Clone)] pub(super) struct FetchedIssue { pub uri: String, pub author: String, pub rkey: String, /// The record itself, with no `{"value": …}` envelope around it. pub value: serde_json::Value, /// `None` only if the PDS omitted it, which no implementation does; a /// write then goes ahead unconditioned rather than refusing. pub cid: Option, } impl FetchedIssue { pub(super) fn title(&self) -> &str { self.value["title"].as_str().unwrap_or("(untitled)") } /// The body, or `None` for a record with none and for one whose body is /// only whitespace — the same "nothing worth printing" test `pr view` /// applies. pub(super) fn body(&self) -> Option<&str> { self.value["body"] .as_str() .map(str::trim) .filter(|b| !b.is_empty()) } pub(super) fn created_at(&self) -> Option<&str> { self.value["createdAt"].as_str().filter(|s| !s.is_empty()) } /// The DID of the repo this issue is filed against. /// /// The lexicon types `repo` as a DID and every record atgc writes carries /// one, but the appview still ingests records whose `repo` holds an /// at-uri instead — the pre-DID spelling — and enqueues a rewrite when it /// sees one. That at-uri's authority is the repo *owner's* account, not /// the repo's own DID, and reading it as one is precisely the confusion /// [`crate::clients::tangled::resolve`] exists to prevent. So an old /// record answers `None` here, which is a false negative rather than a /// plausible wrong DID. pub(super) fn repo_did(&self) -> Option<&str> { self.value["repo"] .as_str() .filter(|r| crate::lexicon::identity::is_did(r)) } } /// Read one issue record straight from its author's PDS. /// /// Unauthenticated, like every other read here, so this works on anybody's /// issue and on an account whose session has expired. pub(super) async fn fetch_issue(target: &IssueRef) -> Result { let pds = crate::clients::atproto::did::pds_or_fail(&target.author).await?; let (value, cid) = crate::clients::atproto::pds::get_record(&pds, &target.author, ISSUE_NSID, &target.rkey) .await?; Ok(FetchedIssue { uri: target.uri(), author: target.author.clone(), rkey: target.rkey.clone(), value, cid, }) } // --------------------------------------------------------------------------- // State // --------------------------------------------------------------------------- /// One `sh.tangled.repo.issue.state` record, cut down to what decides state. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct StateEvent { uri: String, created_at: String, state: String, } /// An issue's state, and whether anything available could settle it. /// /// `Unknown` is deliberately not `open`. Open is the lexicon's default for an /// issue *nobody has acted on*, and asserting that takes having seen every /// state record there is. Reading one PDS cannot: a maintainer's close is a /// record in the maintainer's own repository. The distance between "nobody /// acted" and "we cannot see who acted" is the distance between a fact and a /// guess, and this enum is where that line is drawn. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum State { Known(IssueState), Unknown, } impl State { /// What a terminal column prints. `?` is a display convention; `--json` /// uses [`State::known`] and emits `null`. pub(super) fn label(self) -> &'static str { match self { State::Known(state) => state.label(), State::Unknown => "?", } } /// The state as a value, or `None` when nothing settled it — the shape /// `--json` wants, per docs/output.md. pub(super) fn known(self) -> Option { match self { State::Known(state) => Some(state.label().to_string()), State::Unknown => None, } } /// Whether a `--state` filter selects this row. An unknown state matches /// nothing, and the listing says how many rows that hid. fn matches(self, filter: &str) -> bool { matches!(self, State::Known(state) if state.label() == filter) } } /// One `listRecords` row read as the issue it is about and the event it is, /// or `None` for a record that names no issue. /// /// Everything but `issue` is read leniently, because a state record is /// somebody else's write and a listing must not fail over one: an absent /// `createdAt` becomes the empty string, which /// [`crate::model::record::newer_state`] parses to `None` and which /// therefore loses to every dated record rather than winning; an absent `state` becomes a token [`IssueState::from_token`] /// declines, so [`newest_state`] skips it. Both shapes are real — see /// `fixtures::STATES`, where two of twelve captured records carry neither a /// stamp nor a subject. /// /// `issue` is the one field with no lenient reading available. A record that /// does not say which issue it is about cannot be filed under one, and the /// empty string is that case rather than a subject: it matches no at-uri, so /// keeping it would only mean carrying it to be discarded later. fn state_event(record: &serde_json::Value) -> Option<(String, StateEvent)> { let value = &record["value"]; let issue = value["issue"].as_str().filter(|i| !i.is_empty())?; Some(( issue.to_string(), StateEvent { uri: record["uri"].as_str().unwrap_or_default().to_string(), created_at: value["createdAt"].as_str().unwrap_or_default().to_string(), state: value["state"].as_str().unwrap_or_default().to_string(), }, )) } /// Every state record in `did`'s PDS about any of `issues`, keyed by issue /// at-uri. /// /// One walk for the whole page, not one per row. `listRecords` has no filter, /// so this pages and picks, and the picking costs nothing extra once the page /// is in hand — the round trips are the whole expense. A listing of thirty /// issues used to run this thirty times, and sixty when the acting account /// was not the author, for pages that were very often the same bytes. /// /// What keeps the walk bounded is that record keys are TIDs and `listRecords` /// returns them newest first: a state record about an issue cannot have been /// written before the issue, so once a page ends below the *oldest* key being /// asked about there is nothing left to find. Per row that floor was the /// row's own key; for a batch it has to be the minimum, or the walk would /// stop while a later row's records were still ahead of it. The comparison is /// strict, which is how a state record written under an issue's own key — the /// shape Tangled's own backfill produced for pull statuses — is still seen. /// /// A `did` with no PDS is an error, as it was per row: it means the account /// itself could not be resolved. async fn state_events_for( did: &str, issues: &[(&str, &str)], ) -> Result<(HashMap>, bool)> { let pds = crate::clients::atproto::did::pds_or_fail(did).await?; let walk = crate::clients::atproto::pds::records_about(&pds, did, ISSUE_STATE_NSID, "issue", issues) .await?; // A short walk is both said out loud *and* returned. Saying it explains // the `?` to a reader; returning it is what makes the `?` happen at all, // because an absence is only evidence of "open" to somebody who reached // the end of the collection — see // [`crate::model::record::absence_is_open`]. This warning used to be the // whole of it, and promised a `?` the caller then overwrote with `open`. if !walk.complete { crate::term::say::warning!( Pds, "the state walk hit its page cap before reaching the oldest issue asked \ about, so a state below it reads `?` rather than the state it has" ); } let mut found: HashMap> = HashMap::new(); for (issue, records) in walk.by_subject { let events: Vec = records .iter() .filter_map(state_event) .map(|(_, event)| event) .collect(); if !events.is_empty() { found.insert(issue, events); } } Ok((found, walk.complete)) } /// One issue's state records, and whether the walk that found them reached /// the end of the collection. The second half is not decoration: it is what /// says whether finding none of them means anything. async fn state_events( did: &str, issue_uri: &str, issue_rkey: &str, ) -> Result<(Vec, bool)> { let (mut by_issue, complete) = state_events_for(did, &[(issue_uri, issue_rkey)]).await?; Ok((by_issue.remove(issue_uri).unwrap_or_default(), complete)) } /// The state a set of state 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. That is /// the appview's `stateWinner`, which orders by created time descending and /// then by at-uri descending, and it is the same rule /// [`crate::cmd::pr::write`] applies to pull statuses — the two must agree, /// or `issue list` and `issue close` would read one record differently. /// /// "Newest" is on a parsed instant and not on the string, because the records /// being ordered here come from two accounts — see [`crate::model::record::newer_state`]. /// /// A record whose `state` this build does not recognise is skipped rather /// than allowed to win, which is also what the appview does with a variant it /// cannot parse. Letting one decide here would report a state Tangled does /// not show. /// /// `None` when no record settled anything — every record was unreadable, or /// there were none. The caller decides what an absence means, because that /// depends on whose PDS was read; see [`state_of`]. fn newest_state(events: &[StateEvent]) -> Option { events .iter() .filter(|e| IssueState::from_token(&e.state).is_some()) .max_by(|a, b| { crate::model::record::newer_state((&a.created_at, &a.uri), (&b.created_at, &b.uri)) }) .and_then(|e| IssueState::from_token(&e.state)) } /// Whether an absence of state records really means "nobody has acted". /// /// Only when the issue's author also owns the repo it was filed on. Tangled /// honours a state record from the issue's author, the repo's owner, and a /// collaborator with push access; when the first two are the same account, /// every record the first two could have written was in the one PDS this /// already read, and finding none is a fact rather than a gap. /// /// The collaborator is the hole, and it is left open knowingly: atgc cannot /// read a knot's collaborator list, so there is no way to rule one out. The /// pull half has carried exactly this gap since `pr close` shipped, and /// TODO.md records it — a collaborator's close shows on tangled.org and is /// invisible here. Closing it needs a knot ACL query, not a different rule. /// /// Errors are swallowed into `false`, which keeps the `?`: an ownership /// lookup that could not be made is not evidence of anything, and a listing /// must not become an assertion because a request failed. async fn owner_writes_here(author: &str, repo_did: Option<&str>) -> bool { let Some(repo_did) = repo_did else { return false; }; crate::clients::tangled::ownership::owns_repo(author, repo_did) .await .unwrap_or(false) } /// The state of one issue, from every PDS that could hold a record about it. /// /// Two reads at most. The author's PDS always, because an author may close /// their own issue; and the acting account's when it is somebody else, /// because that is where `issue close` on a repo you own puts its record — /// without it, closing an issue and then viewing it would disagree. pub(super) async fn state_of(issue: &FetchedIssue, acting: Option<&str>) -> Result { let (mut events, mut complete) = state_events(&issue.author, &issue.uri, &issue.rkey).await?; if let Some(acting) = acting.filter(|a| *a != issue.author) { let (theirs, acting_complete) = state_events(acting, &issue.uri, &issue.rkey).await?; events.extend(theirs); complete = complete && acting_complete; } crate::logging::debug::log(format!( "{} state record(s) for {}", events.len(), issue.uri )); if let Some(state) = newest_state(&events) { return Ok(State::Known(state)); } let settled = owner_writes_here(&issue.author, issue.repo_did()).await; Ok( match crate::model::record::absence_is_open(settled, complete) { true => State::Known(IssueState::Open), false => State::Unknown, }, ) } // --------------------------------------------------------------------------- // `issue list` // --------------------------------------------------------------------------- /// One issue in a listing, with the state resolution already done. /// /// The shape a second source would merge into: `value` is the record in the /// `{"uri", "value"}` envelope both a PDS listing and an appview item happen /// to agree on, and `state` already admits to being unsettled. Nothing here /// assumes the row came from a PDS. #[derive(Debug, Clone)] struct Listed { uri: String, value: serde_json::Value, state: State, /// The appview's count, `0` for a row no index answered for. Not /// `Option`: Bobbin says `0` for an issue it has indexed and nobody has /// commented on, and a PDS-only row cannot tell that apart from its own /// silence — so the column means "comments the index knows about", /// which is the honest reading of both. comments: u64, /// Whether an index returned this row. The only input to the staleness /// note, and the reason the merge keeps provenance rather than /// discarding it once the two halves are joined. indexed: bool, } impl Listed { fn record(&self) -> &serde_json::Value { &self.value["value"] } /// Whose PDS holds this record, off the at-uri's authority. /// /// Read per row rather than taken from the listing's subject, which was /// safe only while every row came from one account's PDS. A repo-scoped /// index listing spans authors, which is the whole point of it. fn author_did(&self) -> &str { crate::model::record::authority_of(&self.uri).unwrap_or_default() } fn title(&self) -> &str { self.record()["title"].as_str().unwrap_or("(untitled)") } fn repo_did(&self) -> Option<&str> { self.record()["repo"] .as_str() .filter(|r| crate::lexicon::identity::is_did(r)) } fn created_at(&self) -> &str { self.record()["createdAt"].as_str().unwrap_or("") } /// Sort key. Not the raw string: a PDS record stamps UTC while a record /// written by another client can carry an offset, and those two do not /// sort lexically against each other within a day. An unparseable stamp /// yields `None`, which sorts below everything dated rather than winning. fn instant(&self) -> Option { Datetime::from_str(self.created_at()).ok() } fn rkey(&self) -> &str { self.uri.rsplit('/').next().unwrap_or("?") } } /// Does this issue record name `repo_did`? /// /// The filter that makes a repo-scoped listing correct at all. An issue /// record lives in its reporter's PDS *regardless of which repo it names*, so /// this one collection mixes every repo the account has ever filed against; /// listing it unfiltered would answer a question nobody asked. The same trap /// `crate::cmd::pr::read`'s `targets_repo` documents for pulls. fn names_repo(record: &serde_json::Value, repo_did: &str) -> bool { record["value"]["repo"].as_str() == Some(repo_did) } /// The words `--state` accepts on an issue listing. /// /// [`crate::cmd::pr::read::StateFilter`] without `merged`, which is the whole /// of the difference — the same shape [`IssueState`] has next to `PullState`, /// and for the same reason. Kept as its own enum rather than shared, because /// sharing one would make `issue list --state merged` parse and then match /// nothing, which is the bug this closes wearing a different hat. /// /// A `ValueEnum` rather than a `String` because the string took anything, and /// this listing hid it worse than the pull one did: `--state opne` printed /// `no issues for on `, which does not so much as echo the word, /// so nothing on screen hinted that the flag had been misread. /// /// The *input* side alone. [`State::Unknown`] and the tokens /// [`IssueState::from_token`] declines still say what a record says, because /// a state Tangled adds after this build must list rather than crash. /// /// `All` filters nothing, which also makes it the only way to see an issue /// whose state nothing available could settle: an unknown state matches no /// filter, so both other values hide those rows (and say how many). #[derive(clap::ValueEnum, Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum StateFilter { Open, Closed, /// Every state, including an issue whose state is unknown All, } impl StateFilter { /// The label a row's state is compared against, or `None` for `all`, /// which filters nothing. fn label(self) -> Option<&'static str> { match self { StateFilter::Open => Some(IssueState::Open.label()), StateFilter::Closed => Some(IssueState::Closed.label()), StateFilter::All => None, } } } #[derive(clap::Args, Debug)] pub(crate) struct ListArgs { /// Filter by state #[arg(long, default_value = "open")] pub state: StateFilter, /// Maximum number of issues to show /// A floor of 1, the same `--limit` `search` has carried since it /// shipped. `--limit 0` used to parse and then print "no open pull /// requests" — an empty listing that reads as *there are none*, which is /// the stale-index failure this tree opted out of arriving from the /// command line instead. It is now clap's own refusal, naming the range, /// exit 2: the same treatment `--state opne` got and for the same reason. #[arg(long, default_value_t = 30, value_parser = clap::value_parser!(u32).range(1..))] pub limit: u32, /// List issues on every repo, not just this checkout's #[arg(long)] pub all: bool, /// Whose issues to list (handle or DID); defaults to the acting account #[arg(long, value_name = "HANDLE|DID")] pub author: Option, /// Git remote pointing at the repo #[arg(long, default_value = "origin")] pub remote: String, /// Where to read from: pds (default, your own issues and never stale) /// or bobbin, or both comma-separated. The index is opt-in and /// ATGC_USE_BOBBIN=1 adds it to the default. `web` is refused here: the /// page scrape behind it only ever read pull requests #[arg(long)] pub source: Option, /// Print a JSON array instead of a table, one object per issue; /// colour, hyperlinks and ellipsizing are all off #[arg(long)] pub json: bool, } /// Fold an index's rows into the PDS's, keyed by at-uri. /// /// The two halves answer different questions and only one of them is /// authoritative about what it covers. A row the account holds itself came /// from its own PDS and cannot be stale, so its state is kept whenever /// anything settled it and the index only fills in what it could not — a /// close written by a maintainer in *their* repository, which no read of /// this account's PDS can see. A row only the index has is new: it belongs /// to somebody else, and everything about it is the index's word. /// /// `commentCount` comes from the index in every case, there being no other /// source for it: a comment is a record in its commenter's PDS. /// /// `repo_filter` is applied to the index's rows for `names_repo`'s reason, /// which applies just as much to an index as to a collection: `listIssuesBy` /// spans every repo the account has ever filed against, and a repo-scoped /// listing must not quietly widen. fn merge_indexed( rows: &mut Vec, indexed: Vec, repo_filter: Option<&str>, ) { let mut seen: HashMap = HashMap::new(); for (i, row) in rows.iter().enumerate() { seen.insert(row.uri.clone(), i); } for item in indexed { let Some(uri) = item["uri"].as_str().map(str::to_string) else { continue; }; if repo_filter.is_some_and(|repo| !names_repo(&item, repo)) { continue; } let state = item["state"] .as_str() .and_then(IssueState::from_label) .map_or(State::Unknown, State::Known); let comments = item["commentCount"].as_u64().unwrap_or(0); match seen.get(&uri) { // Held here too: keep the record and the state this account's // own PDS settled, and take the count, which only the index has. Some(&i) => { rows[i].indexed = true; rows[i].comments = comments; if rows[i].state == State::Unknown { rows[i].state = state; } } // Somebody else's issue, which is the whole reason to ask. None => rows.push(Listed { uri, value: item, state, comments, indexed: true, }), } } } /// One row of `issue list --json`. /// /// Still deliberately without a `number`: that one is the appview's own /// SQLite id, in no record and in no XRPC response, and TODO.md records what /// resolving it would take. /// /// `comments` is here now because there is finally something to put in it — /// the index's `commentCount`, which arrived with `--source bobbin`. It is /// `0` rather than `null` for a row no index answered for, which is the /// honest reading: Bobbin says `0` for an issue nobody has commented on, and /// a PDS-only row cannot tell that apart from its own silence. `indexed` /// says which case a row is in. #[derive(serde::Serialize, Debug, PartialEq)] pub(crate) struct IssueRowJson { pub uri: String, /// The record key, which is how every `issue` verb names an issue — /// there being no number to name it by. pub rkey: String, pub state: Option, pub title: String, pub author_did: String, /// Without the leading `@` the terminal column prints. pub author_handle: Option, /// RFC 3339, straight from the record's `createdAt`; `null` when the /// record carries none. pub created_at: Option, pub repo_did: Option, /// The repo's issue *listing*, never this issue's own page — see /// [`issues_url`]. pub repo_url: Option, /// Comments the index knows about; `0` for a row it did not answer for. pub comments: u64, /// Whether an index returned this row, so a caller can tell a real `0` /// from an unasked question and a settled state from an inferred one. pub indexed: bool, } /// An account's own issue records, newest first, optionally only those filed /// against one repo. /// /// How far the walk goes is [`Reach`]'s decision rather than a count passed /// in here, for the reason `pr list` had to learn the hard way: an issue's /// state lives in a separate `sh.tangled.repo.issue.state` collection, so a /// walk over issue records cannot tell which of them `--state` is about to /// drop. Stopping at a screenful under a filter — and `--state open` is the /// default — is how a repo whose newest thirty issues are all closed comes to /// list none at all, under a note calling the listing complete. async fn own_issues(did: &str, repo_did: Option<&str>, reach: Reach) -> Result { let pds = crate::clients::atproto::did::pds_or_fail(did).await?; crate::clients::atproto::pds::list_matching(&pds, did, ISSUE_NSID, reach, |record| { match repo_did { Some(repo) => names_repo(record, repo), None => true, } }) .await } /// The footnote that keeps a repo listing honest about whose issues it holds. /// /// Without it the command trades one silent wrong answer for another: four /// rows under the heading of a repo look exactly like "the four issues on /// this repo", which is the same class of mistake as a stale index arrived at /// from the other direction. The rows are true; what needs saying is what is /// not among them. fn note_author_scoped(did: &str, subject_is_you: bool, indexed: bool, complete: bool) { // The old form of this note ended "which atgc does not read for issues // yet: see TODO.md". It does now, so the note's job changed from // announcing an absence to saying which trade is in force. if indexed { crate::term::say::note!( Index, "your own issues came from your PDS and are current; everyone else's came \ from\nBobbin's index, which is alpha and whose ingest stalls, so a recent \ one can be\nmissing. Drop --source to read only your own." ); return; } let whose = match subject_is_you { true => "your issues".to_string(), false => format!("{did}'s issues"), }; // "Complete" describes the walk, not the PDS, and saying it over a walk // that stopped at a screenful is the failure this parameter exists for: // a short listing then reads as a short collection. match complete { true => crate::term::say::note!( Index, "these are {whose}, read from that account's PDS: complete, current, and\n\ silent about anyone else's. Add the index to see everybody's: --source bobbin\n\ (alpha, its ingest stalls), which also carries comment counts." ), false => crate::term::say::note!( Index, "these are the newest of {whose}, read from that account's PDS: current and\n\ silent about anyone else's, but not all of them — under --state all the walk\n\ stops at --limit records. Raise it to see further back, or add the index for\n\ everybody's: --source bobbin (alpha, its ingest stalls)." ), } } /// Where an unsettled state comes from, which is the same sentence whoever /// is asking. const UNSETTLED: &str = "What would settle it is a sh.tangled.repo.issue.state record written \ by somebody\nwhose PDS was not read here, a repo owner or a collaborator, and nothing\n\ available holds one."; /// The footnote that keeps `--state` honest, in whichever of its two /// directions applies. /// /// The two are opposites and saying the wrong one is worse than saying /// nothing. Under a filter an unsettled row matches neither `open` nor /// `closed`, so it is *removed* — and that is exactly when the count matters, /// because it is usually why the listing looks empty. Under `--state all` /// nothing is removed and the same rows are on screen showing `?`, which /// wants the count too and used not to get it: `unknown` was only counted /// when a filter was set, so the one listing that actually prints a `?` was /// the one that never explained it. fn note_unknown_states(unknown: usize, filtered: bool) { if unknown == 0 { return; } let lead = match filtered { true => format!( "{unknown} issue(s) are missing from this listing: their state reads ? and\n\ --state matches neither open nor closed." ), false => format!("{unknown} issue(s) above show state ?."), }; crate::term::say::note!(Index, "{lead}\n{UNSETTLED}"); } /// The same footnote for one issue, which is what `issue view` has. /// /// A separate sentence rather than [`note_unknown_states`] with a count of /// one: that one explains itself in terms of `--state`, and `issue view` has /// no such flag, so it read as an answer to a question nobody had asked. fn note_unknown_state_of_one() { crate::term::say::note!( Index, "This issue's state reads ?, which is not the same as open.\n{UNSETTLED}" ); } /// Resolve the state of every row, one ownership lookup per repo rather than /// per row. /// /// [`state_of`]'s absence rule asks whether the author owns the repo, and a /// page of thirty issues on one repo would otherwise ask that thirty times. /// The answer cannot differ between two rows naming the same repo and the /// same author, which is what makes the cache correct rather than merely /// cheap. async fn settle_states(rows: &mut [Listed], author: &str, acting: Option<&str>) -> Result<()> { let issues: Vec<(&str, &str)> = rows .iter() .map(|r| (r.uri.as_str(), r.rkey())) .collect::>(); // One walk of each PDS holding state records for this page, rather than // one per row of it. Two walks at most: the author's, and the acting // account's when it is somebody else and so may have closed one of these. let (mut events_by_issue, mut complete) = state_events_for(author, &issues).await?; if let Some(acting) = acting.filter(|a| *a != author) { let (theirs, acting_complete) = state_events_for(acting, &issues).await?; for (uri, events) in theirs { events_by_issue.entry(uri).or_default().extend(events); } // Either walk falling short leaves a record unseen, so completeness // is the *conjunction*: the reads answer one question between them. complete = complete && acting_complete; } let mut owned: HashMap = HashMap::new(); for row in rows.iter_mut() { let events = events_by_issue.get(&row.uri).map_or(&[][..], Vec::as_slice); if let Some(state) = newest_state(events) { row.state = State::Known(state); continue; } let settled = match row.repo_did() { Some(repo) => match owned.get(repo) { Some(known) => *known, None => { let answer = owner_writes_here(author, Some(repo)).await; owned.insert(repo.to_string(), answer); answer } }, None => false, }; row.state = match crate::model::record::absence_is_open(settled, complete) { true => State::Known(IssueState::Open), false => State::Unknown, }; } Ok(()) } pub(crate) async fn list(args: ListArgs) -> Result<()> { crate::term::jsonout::init(args.json); // Reading somebody's issues needs no token, only a DID — so `--author` // stays available for an account whose session has expired, and for one // nobody here has ever logged in to. let source = crate::cmd::pr::read::Source::parse_arg_without_web(args.source.as_deref())?; let acting = crate::config::account::select().await.ok().map(|s| s.did); let subject = match &args.author { Some(author) => crate::config::account::actor_did(author).await?, None => match acting.clone() { Some(did) => did, // `NoSession`, because logging in is one of the two fixes and is // the one a caller can automate; docs/output.md spells it 3. None => { return Err(crate::exit::fail( crate::exit::Exit::NoSession, "not logged in, and no --author given: `atgc issue list --author \ ` lists anybody's issues without a session", )); } }, }; let repo = match args.all { true => None, false => { let remote_url = git::remote_url(&args.remote)?; Some(resolve::repo_ref(&remote_url).await?) } }; let want = args.limit as usize; // Read before the records are, because it is what decides how deep to // read them: `--state` is applied below, over records whose state the // walk itself cannot see. See [`Reach::for_listing`]. let filter = args.state.label(); let reach = Reach::for_listing(filter, args.limit); // The PDS half: this account's own issue records, current and silent // about everybody else's — and the whole of them whenever a filter is // in force. let (records, evidence) = match source.uses_pds() { true => { let listing = own_issues(&subject, repo.as_ref().map(|r| r.did.as_str()), reach).await?; (listing.records, listing.evidence) } false => ( Vec::new(), Evidence { reach, truncated: false, }, ), }; let mut rows: Vec = records .into_iter() .filter_map(|value| { let uri = value["uri"].as_str()?.to_string(); Some(Listed { uri, value, state: State::Unknown, comments: 0, indexed: false, }) }) .collect(); // Own records first, so the states settled below are settled on the // half that outranks the index, and only then merged with it. settle_states(&mut rows, &subject, acting.as_deref()).await?; // The index half, when one is opted in to. `listIssues` is every // author's issues on one repo — the question a PDS cannot answer at all, // since the records are scattered across the PDSes of everyone who ever // filed one. let indexed = match source.uses_bobbin() { true => { let (endpoint, subject) = match &repo { Some(repo) => ("listIssues", repo.did.as_str()), None => ("listIssuesBy", subject.as_str()), }; match crate::clients::tangled::bobbin::issues(endpoint, subject, args.limit).await { Ok(items) => items, // Fatal only when the index is the only source left, the // same asymmetry `pr list` draws: an appview outage should // take down the part of the answer that depends on it and // no more. Err(e) if source.uses_pds() && !rows.is_empty() => { crate::term::say::warning!( Index, "{e:#}\ncontinuing with your own issues from the PDS. Other \ accounts' are not listed." ); Vec::new() } Err(e) => return Err(e), } } false => Vec::new(), }; merge_indexed(&mut rows, indexed, repo.as_ref().map(|r| r.did.as_str())); rows.sort_by_key(|r| std::cmp::Reverse(r.instant())); // Whether this page can hold more than one account's issues, which is // what decides the author column and what the closing note has to be // honest about. let spans_authors = source.uses_bobbin(); let unfiltered = rows.len(); // Counted whether or not a filter is set: under one these are the rows // about to be removed, without one they are the rows about to print `?`. // Both want saying; only the first used to be counted. let unknown_all = rows.iter().filter(|r| r.state == State::Unknown).count(); if let Some(filter) = filter { rows.retain(|r| r.state.matches(filter)); } // After the filter, not before it. Cutting to `--limit` first meant the // filter ran on a screenful and could empty it while matching issues sat // unread below the cut — the same defect as the walk's early stop, one // step further down the pipeline, and it fired even when the walk had // read everything. // What `--limit` is about to cut down: `cuts` says how many rows never // made the page, which is the half of "not all of them" that is the // caller's own doing rather than the walk's. let matched = rows.len(); rows.truncate(want); // The filtered note speaks for every row the filter took out, whether or // not it would have fitted on the page; the unfiltered one speaks for // the rows actually printed. let unknown = match filter { Some(_) => unknown_all, None => rows.iter().filter(|r| r.state == State::Unknown).count(), }; if rows.is_empty() { // `[]` on stdout either way — a script should never have to tell "no // issues" apart from "the process died before printing anything". let why = match (filter, unfiltered) { (Some(state), n) if n > 0 => format!("no {state} issues"), _ => match &repo { Some(repo) => format!("no issues for {subject} on {}", repo.linked()), None => format!("no issues for {subject}"), }, }; // The reason goes to stderr under `--json` and to stdout otherwise, // but the two footnotes below it are printed either way. An empty // listing is exactly when the unknown-state count matters most — // it is usually *why* the listing is empty — and returning before // saying so is how a filtered-away row becomes an absence nobody // can account for. match args.json { true => { crate::term::say::note!(Index, "{why}"); println!("[]"); } false => println!("{why}"), } cuts(evidence, matched, rows.len(), "issue records"); note_author_scoped( &subject, args.author.is_none(), spans_authors, evidence.complete(), ); note_unknown_states(unknown, filter.is_some()); return Ok(()); } // One lookup per distinct author on the page rather than one for the // subject: a repo-scoped index listing spans accounts, so the subject's // handle is no longer every row's. // The shared resolver directly: this used to be a one-line wrapper whose // only real effect was to be the third function in this tree called // `author_handles`, each with a different contract. let handles = crate::clients::atproto::handles::handles( rows.iter().map(|row| row.author_did().to_string()), ) .await; if args.json { let listed: Vec = rows .iter() .map(|row| IssueRowJson { uri: row.uri.clone(), rkey: row.rkey().to_string(), state: row.state.known(), title: row.title().to_string(), author_did: row.author_did().to_string(), author_handle: handles.get(row.author_did()).cloned(), created_at: (!row.created_at().is_empty()).then(|| row.created_at().to_string()), repo_did: row.repo_did().map(str::to_string), repo_url: row.repo_did().map(issues_url), comments: row.comments, indexed: row.indexed, }) .collect(); crate::term::jsonout::emit(&listed)?; cuts(evidence, matched, rows.len(), "issue records"); note_author_scoped( &subject, args.author.is_none(), spans_authors, evidence.complete(), ); note_unknown_states(unknown, filter.is_some()); return Ok(()); } for row in &rows { let rkey = row.rkey(); let state = row.state.label(); let title = cell(row.title(), 54, 56); let date = day(row.created_at()); // The author column appears only when the page can span authors, // which is exactly when an index answered: without one every row is // the subject's and a column repeating it is width spent on nothing. let author = match spans_authors { true => format!( " {}", pad_to( &crate::term::hyperlink::cut_account(&ellipsize( &crate::term::hyperlink::account( handles.get(row.author_did()).map(String::as_str), row.author_did(), ), 24 )), 24 ) ), false => String::new(), }; match args.all { true => { let repo = row.repo_did().unwrap_or("?"); println!( "{} {} {}{author} {} {repo}", pad_to(rkey, 15), pad_to(state, 7), title, pad_to(&date, 11) ); } false => println!( "{} {} {}{author} {date}", pad_to(rkey, 15), pad_to(state, 7), title ), } } if let Some(repo) = &repo { println!( "\nview: {}", crate::term::hyperlink::url(&issues_url(&repo.did)) ); } cuts(evidence, matched, rows.len(), "issue records"); note_author_scoped( &subject, args.author.is_none(), spans_authors, evidence.complete(), ); note_unknown_states(unknown, filter.is_some()); Ok(()) } // --------------------------------------------------------------------------- // `issue view` // --------------------------------------------------------------------------- #[derive(clap::Args, Debug)] pub(crate) struct ViewArgs { /// The issue: its record key or at:// URI #[arg(value_name = "ISSUE")] pub issue: Option, /// Also show the discussion (needs the index: --source bobbin) /// /// A comment is a record in its commenter's PDS and nothing enumerates /// the people who have commented on something, so a thread can only come /// from the appview's index. What it shows is therefore what Bobbin has /// seen, and its ingest stalls. #[arg(long)] pub comments: bool, /// Where to read from: pds (default) or bobbin (also: ATGC_USE_BOBBIN=1) #[arg(long)] pub source: Option, /// The same, as a flag, for symmetry with the other verbs #[arg(long = "issue", value_name = "ISSUE", conflicts_with = "issue")] pub issue_flag: Option, /// Whose issue it is, when a bare record key is ambiguous #[arg(long, value_name = "HANDLE|DID")] pub author: Option, /// Print a JSON object instead of the human view #[arg(long)] pub json: bool, } impl ViewArgs { /// The issue this names, whichever spelling was used. pub(crate) fn issue(&self) -> Option<&str> { self.issue.as_deref().or(self.issue_flag.as_deref()) } } /// `issue view --json`'s whole object. #[derive(serde::Serialize, Debug, PartialEq)] pub(crate) struct IssueDetailJson { pub uri: String, pub rkey: String, pub state: Option, pub title: String, pub author_did: String, pub author_handle: Option, pub created_at: Option, /// `null` for an empty or whitespace-only body, matching the text view's /// own "nothing worth printing" test. pub body: Option, pub repo_did: Option, pub repo_url: Option, /// The discussion, oldest first, or absent when it was not asked for. /// /// Named `thread` rather than `comments` because `pr view --json`'s /// `comments` is a *count*, and one word cannot be both in two commands /// a caller reads side by side. /// /// Absent and `[]` mean different things and both are reachable: absent /// is "nobody asked the index", `[]` is "the index was asked and has /// none". Collapsing them would make a thread the index has not caught /// up with indistinguishable from one nobody read. #[serde(skip_serializing_if = "Option::is_none")] pub thread: Option>, } pub(crate) async fn view(args: ViewArgs) -> Result<()> { crate::term::jsonout::init(args.json); let Some(reference) = args.issue() else { return Err(crate::exit::fail( crate::exit::Exit::Usage, "which issue? pass its record key or at:// URI, as in \ `atgc issue view 3msg7w7l6hs2x`\n\ atgc does not guess from the current branch: an issue record names no \ branch, so there is nothing to guess from", )); }; // Before the first request, not beside the use: a `--source` this command // cannot honour is a fact about the command line, and answering it after // two round trips would charge the caller for the mistake and then refuse // anyway. let source = crate::cmd::pr::read::Source::parse_arg_without_web(args.source.as_deref())?; let (target, acting) = resolve_issue(reference, args.author.as_deref()).await?; let issue = fetch_issue(&target).await?; let state = state_of(&issue, acting.as_deref()).await?; let handle = crate::clients::atproto::handles::handle(&issue.author).await; let thread = crate::cmd::read_thread(args.comments, source, &issue.uri).await; if args.json { return crate::term::jsonout::emit(&IssueDetailJson { uri: issue.uri.clone(), rkey: issue.rkey.clone(), state: state.known(), title: issue.title().to_string(), author_did: issue.author.clone(), author_handle: handle, created_at: issue.created_at().map(str::to_string), body: issue.body().map(str::to_string), repo_did: issue.repo_did().map(str::to_string), repo_url: issue.repo_did().map(issues_url), thread: crate::cmd::thread_json(&thread).await, }); } let author = match &handle { Some(h) => format!("{} ({})", crate::term::hyperlink::handle(h), issue.author), None => issue.author.clone(), }; println!("issue: {} ({})", one_line(issue.title()), issue.rkey); println!("author: {author}"); println!("state: {}", state.label()); println!("opened: {}", day(issue.created_at().unwrap_or(""))); if let Some(repo) = issue.repo_did() { // The DID is what the record holds and the only name available // without another lookup; the appview 302s it to `owner/name`, so // linking it costs nothing and lands on the right page. println!( "repo: {}", crate::term::hyperlink::wrap(&repo_url(repo), repo) ); } println!("uri: {}", issue.uri); if let Some(repo) = issue.repo_did() { println!("view: {}", crate::term::hyperlink::url(&issues_url(repo))); } if let Some(body) = issue.body() { // A body is prose and keeps its lines; only what a terminal would act // on comes out. `--json` above carries the record's own value. println!("\n{}", crate::term::text::clean(body)); } crate::cmd::print_thread(&thread).await; if state == State::Unknown { note_unknown_state_of_one(); } Ok(()) } /// Turn a reference into an issue and settle which account is doing the /// reading, in the order every `issue` verb wants them. /// /// `--author` decides whose PDS a bare record key names; without one it is /// the acting account, which is the only defensible assumption and the same /// one `pr resubmit` makes. The acting account is handed back because /// [`state_of`] needs it — a repo owner's close lives in the owner's PDS, and /// that is very often the account running the command. async fn resolve_issue( reference: &str, author: Option<&str>, ) -> Result<(IssueRef, Option)> { let acting = crate::config::account::select().await.ok().map(|s| s.did); let assumed = match author { Some(author) => crate::config::account::actor_did(author).await?, // The empty string is only ever reached for a bare record key: an // at-uri names its own author and needs no session at all, which is // what keeps `issue view at://…` working logged out. The refusal // below turns it back into a sentence. None => acting.clone().unwrap_or_default(), }; let target = classify_issue_ref(reference, &assumed)?; if target.author.is_empty() { return Err(crate::exit::fail( crate::exit::Exit::NoSession, "a bare record key needs an account to look it up in, and there is no \ session and no --author\n\ pass the at:// URI, or name the author with --author ", )); } Ok((target, acting)) } #[cfg(test)] mod tests { use super::super::fixtures; use super::{FetchedIssue, IssueRef, IssueState, Listed, State, StateEvent}; use super::{classify_issue_ref, day, merge_indexed, names_repo, newest_state, state_event}; use serde_json::json; const ME: &str = "did:plc:nlzmjyfv6loqtxyzvdcznwgf"; const OTHER: &str = "did:plc:wshs7t2adsemcrrd4snkeqli"; const REPO: &str = "did:plc:gspkabpde4kx47fj3bhiwrms"; fn event(created_at: &str, rkey: &str, token: &str) -> StateEvent { StateEvent { uri: format!("at://{ME}/sh.tangled.repo.issue.state/{rkey}"), created_at: created_at.to_string(), state: token.to_string(), } } /// A bare record key is read as the assumed account's own issue, which is /// the only assumption available and the one `--author` overrides. #[test] fn a_bare_record_key_belongs_to_the_assumed_account() { assert_eq!( classify_issue_ref("3msg7w7l6hs2x", ME).unwrap(), IssueRef { author: ME.to_string(), rkey: "3msg7w7l6hs2x".to_string(), } ); // Surrounding whitespace comes from shells and copy-paste. assert_eq!( classify_issue_ref(" 3msg7w7l6hs2x ", ME).unwrap().rkey, "3msg7w7l6hs2x" ); } /// An at-uri names its own author, which is what makes `issue close` /// usable on an issue filed against your repo by somebody else — and it /// needs no session, because nothing has to be assumed. #[test] fn an_at_uri_names_the_author_it_carries() { let uri = format!("at://{OTHER}/sh.tangled.repo.issue/3msg7w7l6hs2x"); let parsed = classify_issue_ref(&uri, ME).unwrap(); 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 the one this cut cannot answer. The /// refusal has to say that the number is the appview's rather than /// reading as a malformed record key, or somebody spends the afternoon /// looking for a typo in a number they copied correctly. #[test] fn an_issue_number_is_refused_by_name() { for input in ["23", "1", "0042", " 7 "] { let err = classify_issue_ref(input, ME) .expect_err("a number is not a record key") .to_string(); assert!(err.contains("issue number"), "{input:?}: {err}"); assert!(err.contains("record key"), "{input:?}: {err}"); } // A pasted link gets the same explanation, because it is the same // problem wearing a URL. for input in [ "https://tangled.org/@permadeath.com/atgc/issues/23", "tangled.org/@permadeath.com/atgc/issues/23", ] { let err = classify_issue_ref(input, ME).expect_err(input).to_string(); assert!(err.contains("issue number"), "{input:?}: {err}"); } } /// The other ways a reference can be wrong, each refused before anything /// leaves the machine. #[test] fn a_reference_that_cannot_name_an_issue_is_refused() { for (input, expected) in [ ("", "no issue given"), ("at://", "not a complete issue at-uri"), ( &format!("at://{ME}/sh.tangled.repo.issue"), "not a complete issue at-uri", ), // Right shape, wrong collection: this is a pull, not an issue. ( &format!("at://{ME}/sh.tangled.repo.pull/3msg7w7l6hs2x"), "not a sh.tangled.repo.issue", ), // A handle authority is legal in an at-uri and refused anyway: // handles change hands, and this decides whose issue is closed. ( "at://permadeath.com/sh.tangled.repo.issue/3msg7w7l6hs2x", "can change hands", ), // A malformed DID stops at the identifier classifier. ( "at://did:plc:short/sh.tangled.repo.issue/3msg7w7l6hs2x", "24 characters of base32", ), // A URL that names no issue is not an answer, and is refused // rather than guessed at. ( "https://tangled.org/@permadeath.com/atgc", "not an issue reference", ), ] { let err = classify_issue_ref(input, ME).expect_err(input).to_string(); assert!( err.contains(expected), "{input:?}: expected {expected:?}, got {err}" ); } } /// 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 load-bearing assumption in `issue close`. #[test] fn the_newest_state_record_decides_the_state() { assert_eq!(newest_state(&[]), None); let closed = event("2026-08-14T10:00:00Z", "aaa", IssueState::Closed.token()); assert_eq!( newest_state(std::slice::from_ref(&closed)), Some(IssueState::Closed) ); // Closed, then reopened: the later record wins, which is exactly the // shape `issue reopen` writes. let reopened = event("2026-08-14T11:00:00Z", "bbb", IssueState::Open.token()); assert_eq!( newest_state(&[closed.clone(), reopened.clone()]), Some(IssueState::Open) ); // Order in the slice must not matter: records arrive from two PDSes. assert_eq!( newest_state(&[reopened, closed.clone()]), Some(IssueState::Open) ); // A tie on createdAt breaks on the at-uri, as the appview's // `stateWinner` does — not hypothetical, since two records written // by one handler share a millisecond. let same = "2026-08-14T12:00:00Z"; let lower = event(same, "aaa", IssueState::Closed.token()); let higher = event(same, "zzz", IssueState::Open.token()); assert_eq!( newest_state(&[lower.clone(), higher.clone()]), Some(IssueState::Open) ); assert_eq!(newest_state(&[higher, lower]), Some(IssueState::Open)); } /// **The records being ordered come from two accounts, so the raw /// strings will not do.** /// /// [`state_of`] merges the issue author's state records with the acting /// account's — that is the whole point of it, and it is what makes a repo /// owner's close visible next to the author's own. Two accounts means two /// clients and two stamp formats, and `…Z`, `…±hh:mm` and a fractional /// second do not sort lexically against each other: a reopen at /// `12:00:00-03:00` is fifteen hundred seconds *after* a close at /// `13:00:00Z` and sorts before it as text. The appview settles this on /// `created_micros`, so a string compare here reports a state tangled.org /// does not show. /// /// TODO.md's note on this hazard exempts the pull half's comparisons /// because they read one PDS. This one does not, and that exemption was /// carried over with the code. #[test] fn a_later_record_wins_however_its_writer_spelled_the_offset() { // 12:00-03:00 is 15:00Z, an hour after the close. let closed = event("2026-08-14T14:00:00Z", "aaa", IssueState::Closed.token()); let reopened = event("2026-08-14T12:00:00-03:00", "bbb", IssueState::Open.token()); assert_eq!( newest_state(&[closed.clone(), reopened.clone()]), Some(IssueState::Open), "a string compare puts 12: before 14: and loses the reopen" ); assert_eq!(newest_state(&[reopened, closed]), Some(IssueState::Open)); // The other direction, so the test cannot pass by preferring `open`: // 18:00+02:00 is 16:00Z, an hour after the reopen. let reopened = event("2026-08-14T15:00:00Z", "aaa", IssueState::Open.token()); let closed = event( "2026-08-14T18:00:00+02:00", "bbb", IssueState::Closed.token(), ); assert_eq!(newest_state(&[reopened, closed]), Some(IssueState::Closed)); // A fractional second sorts before `Z` as text and after it in time. let closed = event("2026-08-14T20:00:00Z", "aaa", IssueState::Closed.token()); let reopened = event("2026-08-14T20:00:00.500Z", "bbb", IssueState::Open.token()); assert_eq!(newest_state(&[closed, reopened]), Some(IssueState::Open)); // A stamp that will not parse loses to one that does, rather than // winning on whatever it sorts as. let closed = event("2026-08-14T10:00:00Z", "aaa", IssueState::Closed.token()); let junk = event("yesterday", "zzz", IssueState::Open.token()); assert_eq!(newest_state(&[closed, junk]), Some(IssueState::Closed)); } /// A state variant this build has never heard of is skipped rather than /// allowed to win, and a set containing only unknown variants settles /// nothing — which is a different answer from "open", and the difference /// is what [`State::Unknown`] exists for. #[test] fn an_unknown_state_variant_does_not_decide_anything() { let closed = event("2026-08-14T10:00:00Z", "aaa", IssueState::Closed.token()); let alien = event( "2026-08-14T23:00:00Z", "zzz", "sh.tangled.repo.issue.state.triaged", ); assert_eq!( newest_state(&[closed, alien.clone()]), Some(IssueState::Closed) ); assert_eq!(newest_state(std::slice::from_ref(&alien)), None); // A pull's token must not decide an issue either: both collections // are walked out of the same PDS and only the token tells them apart. let pull = event( "2026-08-14T23:00:00Z", "zzz", crate::lexicon::tangled::PullState::Closed.token(), ); assert_eq!(newest_state(&[pull]), None); } /// The repo filter, which is what makes a repo-scoped listing correct: an /// account's issue collection mixes every repo it has ever filed against. #[test] fn a_listing_keeps_only_the_issues_filed_against_the_repo_asked_about() { let mine = json!({ "uri": "at://x/y/z", "value": { "repo": REPO } }); assert!(names_repo(&mine, REPO)); assert!(!names_repo(&mine, OTHER)); // A record whose `repo` is the pre-DID at-uri spelling matches no // repo DID, and is not made to: the authority in it is the repo // *owner's* account, which is a different identity entirely. let old = json!({ "uri": "at://x/y/z", "value": { "repo": format!("at://{ME}/sh.tangled.repo/atgc") }, }); assert!(!names_repo(&old, REPO)); assert!(!names_repo(&old, ME)); } /// A `--state` filter selects a known state and never an unknown one, and /// the listing counts what that hid rather than dropping it silently. #[test] fn a_state_filter_never_matches_an_unsettled_state() { assert!(State::Known(IssueState::Open).matches("open")); assert!(!State::Known(IssueState::Open).matches("closed")); assert!(!State::Unknown.matches("open")); assert!(!State::Unknown.matches("closed")); // And the two output shapes disagree on purpose: `?` is a column // width's worth of display convention, `null` is what a script reads. assert_eq!(State::Unknown.label(), "?"); assert_eq!(State::Unknown.known(), None); assert_eq!(State::Known(IssueState::Closed).label(), "closed"); assert_eq!( State::Known(IssueState::Closed).known().as_deref(), Some("closed") ); } /// The merge that makes a cross-author listing possible, and the rule /// about which half wins. /// /// Own records outrank the index because they came from this account's /// own PDS and cannot be stale; the index fills in only what the PDS /// could not settle — a close written by a maintainer in *their* /// repository — and supplies the comment count either way, there being /// no other source for it. #[test] fn the_index_fills_gaps_and_adds_rows_without_overruling_your_own_pds() { let mine = |rkey: &str, state: State| Listed { uri: format!("at://{ME}/sh.tangled.repo.issue/{rkey}"), value: json!({ "value": { "title": rkey, "repo": REPO, "createdAt": "2026-08-14T10:00:00Z" } }), state, comments: 0, indexed: false, }; let indexed = |did: &str, rkey: &str, state: &str, comments: u64| { json!({ "uri": format!("at://{did}/sh.tangled.repo.issue/{rkey}"), "state": state, "commentCount": comments, "value": { "title": rkey, "repo": REPO, "createdAt": "2026-08-14T10:00:00Z" }, }) }; let mut rows = vec![ // Settled here, and the index disagrees. The PDS wins. mine("settled", State::Known(IssueState::Closed)), // Nothing settled it here, so the index's answer is the only one. mine("unsettled", State::Unknown), ]; rows[0].comments = 0; merge_indexed( &mut rows, vec![ indexed(ME, "settled", "open", 3), indexed(ME, "unsettled", "closed", 1), indexed("did:plc:someoneelse", "theirs", "open", 7), ], Some(REPO), ); assert_eq!(rows.len(), 3, "the other account's issue was dropped"); assert_eq!(rows[0].state, State::Known(IssueState::Closed)); assert_eq!(rows[0].comments, 3, "the count is the index's either way"); assert!(rows[0].indexed); assert_eq!(rows[1].state, State::Known(IssueState::Closed)); assert_eq!(rows[2].author_did(), "did:plc:someoneelse"); assert_eq!(rows[2].comments, 7); } /// A repo-scoped listing must not quietly widen. `listIssuesBy` spans /// every repo the account has filed against, which is `names_repo`'s /// trap arriving from the index instead of from the collection. #[test] fn the_index_rows_are_filtered_to_the_repo_that_was_asked_about() { let mut rows: Vec = Vec::new(); merge_indexed( &mut rows, vec![ json!({ "uri": "at://did:plc:them/sh.tangled.repo.issue/here", "state": "open", "value": { "title": "here", "repo": REPO }, }), json!({ "uri": "at://did:plc:them/sh.tangled.repo.issue/elsewhere", "state": "open", "value": { "title": "elsewhere", "repo": "did:plc:anotherrepo" }, }), ], Some(REPO), ); assert_eq!(rows.len(), 1); assert_eq!(rows[0].title(), "here"); } /// A state word the index knows and this build does not leaves the row /// unsettled and listable, which is what `from_token` promises for a /// record and now has to hold for the envelope too. #[test] fn an_unknown_state_word_from_the_index_does_not_settle_a_row() { let mut rows: Vec = Vec::new(); merge_indexed( &mut rows, vec![json!({ "uri": "at://did:plc:them/sh.tangled.repo.issue/x", "state": "escalated", "value": { "title": "x", "repo": REPO }, })], Some(REPO), ); assert_eq!(rows.len(), 1, "the row still lists"); assert_eq!(rows[0].state, State::Unknown); } /// A row reports the record key out of its own at-uri, since that is what /// every `issue` verb takes and there is no number to print instead. #[test] fn a_row_names_itself_by_its_record_key() { let row = Listed { uri: format!("at://{ME}/sh.tangled.repo.issue/3msg7w7l6hs2x"), value: json!({ "value": { "title": "a bug", "createdAt": "2026-08-14T10:00:00Z" } }), state: State::Unknown, comments: 0, indexed: false, }; assert_eq!(row.rkey(), "3msg7w7l6hs2x"); assert_eq!(row.author_did(), ME); assert_eq!(row.title(), "a bug"); assert_eq!(day(row.created_at()), "2026-08-14"); // A record with no title is still a row; the listing prints a // placeholder rather than refusing over somebody else's record. let untitled = Listed { uri: "at://x/y/z".to_string(), value: json!({ "value": {} }), state: State::Unknown, comments: 0, indexed: false, }; assert_eq!(untitled.title(), "(untitled)"); assert_eq!(untitled.created_at(), ""); } // ----------------------------------------------------------------------- // Against captured records // ----------------------------------------------------------------------- // // Everything above this line is hand-written JSON, which proves that the // helpers do what they were written to do and nothing about whether the // shapes they were written for are the shapes that exist. These read real // records off `tests/fixtures/`; see `super::super::fixtures` for what // each capture holds and where it came from. fn fetched(fixture: &str) -> FetchedIssue { let captured = fixtures::single(fixture); FetchedIssue { uri: captured["uri"].as_str().unwrap().to_string(), author: captured["uri"] .as_str() .unwrap() .trim_start_matches("at://") .split('/') .next() .unwrap() .to_string(), rkey: captured["uri"] .as_str() .unwrap() .rsplit('/') .next() .unwrap() .to_string(), value: captured["value"].clone(), cid: captured["cid"].as_str().map(str::to_string), } } /// What each reader on [`FetchedIssue`] answers for a record of each /// shape, on the bytes rather than on a description of them. /// /// The pre-DID record is the one that matters. Its `repo` is an at-uri /// whose authority is the repo *owner's* account, so `repo_did` answers /// `None` rather than a plausible wrong DID — and its `createdAt` is /// present and empty, which reads as no stamp rather than as a stamp of /// nothing. Both are what the doc comments on those two methods claim, /// and neither had a record behind it before this. #[test] fn the_shapes_a_live_issue_collection_actually_holds() { let new = fetched(fixtures::NEW_RECORD); assert_eq!(new.repo_did(), Some(fixtures::CORE_REPO)); assert!(new.created_at().is_some()); assert!(new.body().is_some()); let old = fetched(fixtures::OLD_RECORD); assert_eq!( old.repo_did(), None, "an at-uri in `repo` is not a repo DID and is not read as one" ); assert_eq!(old.created_at(), None, "an empty stamp is no stamp"); assert_eq!(old.title(), "xyz"); // The appview's own issue number, in a record — which `issue view 23` // refuses on the grounds that it is in none. It is in this shape and // in nothing written since; see `fixtures::OLD_RECORD`. assert_eq!(old.value["issueId"], 2); } /// The cost of `names_repo` being an exact DID comparison, counted on a /// real collection. /// /// Every one of the ten captured records is an issue on `CORE_REPO`, and /// the filter keeps six: the four that name the repo by at-uri are /// dropped from a repo-scoped listing of the author's own PDS. The index /// does not drop them, because it rewrites `repo` to the DID before /// handing the record back — so `--source bobbin` shows issues that the /// PDS half of the same command cannot, on records the account itself /// wrote. /// /// Left as it is rather than fixed: matching the at-uri too would mean /// resolving it, and the at-uri names the repo *record*, whose key is not /// the repo name and whose authority is the owner. TODO.md carries the /// entry. #[test] fn a_repo_scoped_pds_listing_drops_the_pre_did_records() { let records = fixtures::records(fixtures::PDS_ISSUES); assert_eq!(records.len(), 10, "the captured page changed"); let kept = records .iter() .filter(|r| names_repo(r, fixtures::CORE_REPO)) .count(); assert_eq!(kept, 6); // The same four are on the page all the same, and say which repo // they mean in a property the lexicon does not have. let dropped: Vec<&serde_json::Value> = records .iter() .filter(|r| !names_repo(r, fixtures::CORE_REPO)) .collect(); assert_eq!(dropped.len(), 4); for record in dropped { assert!( record["value"]["repo"] .as_str() .unwrap() .starts_with("at://") ); assert_eq!(record["value"]["repoDid"], fixtures::CORE_REPO); } let indexed = fixtures::items(fixtures::BOBBIN_ISSUES); let seen = indexed .iter() .filter(|i| names_repo(i, fixtures::CORE_REPO)) .count(); assert_eq!(seen, 9, "the index's rewrite passes the same filter"); } /// Every stamp on a captured page parses, in both spellings that page /// carries. `Listed::instant` exists because one account's records come /// back in whatever offset the writing client used, and a listing that /// sorted them as strings would be sorting `…Z` against `…+03:00`. #[test] fn both_stamp_spellings_on_one_page_parse_to_an_instant() { let rows: Vec = fixtures::records(fixtures::PDS_ISSUES) .into_iter() .map(|value| Listed { uri: value["uri"].as_str().unwrap().to_string(), value, state: State::Unknown, comments: 0, indexed: false, }) .collect(); assert!(rows.iter().any(|r| r.created_at().ends_with('Z'))); assert!(rows.iter().any(|r| r.created_at().ends_with("+03:00"))); for row in &rows { assert!( row.instant().is_some(), "{} did not parse", row.created_at() ); } } /// A captured close/reopen/close chain, settled the way the appview /// settles it. /// /// Seven seconds separate the three records, and the listing order is not /// the answer: `newest_state` has to read the stamps. Fed in the order /// the PDS returned them and again reversed, it says `closed` both times. #[test] fn a_captured_reopen_chain_settles_on_the_newest_record() { const ISSUE: &str = "at://did:plc:jge3zxi7lgrfnvhzcgrimeo7/sh.tangled.repo.issue/3msfd4c3gskmf"; let mut events: Vec = fixtures::records(fixtures::STATES_REOPENED) .iter() .filter_map(state_event) .filter(|(issue, _)| issue == ISSUE) .map(|(_, event)| event) .collect(); assert_eq!(events.len(), 3, "the captured chain changed"); assert_eq!(newest_state(&events), Some(IssueState::Closed)); events.reverse(); assert_eq!(newest_state(&events), Some(IssueState::Closed)); } /// The two captured records that are not about anything. /// /// `issue` is the empty string and `createdAt` is absent, both of which /// the lexicon marks required. `state_event` declines them, so they never /// reach the sort — and a listing reading that PDS still answers for the /// ten records beside them rather than failing over these two. #[test] fn a_state_record_that_names_no_issue_is_not_read() { let records = fixtures::records(fixtures::STATES); assert_eq!(records.len(), 12, "the captured page changed"); let read: Vec<(String, StateEvent)> = records.iter().filter_map(state_event).collect(); assert_eq!(read.len(), 10); assert_eq!( records .iter() .filter(|r| r["value"]["issue"] == "" && r["value"]["createdAt"].is_null()) .count(), 2 ); for (_, event) in &read { // Dated, in the sense the ordering cares about: the shared // comparator can parse the stamp, so the record sorts against // real ones rather than falling to the bottom. assert_ne!( crate::model::record::newer_state((&event.created_at, &event.uri), ("", ""),), std::cmp::Ordering::Equal, "every readable record is dated" ); } } }