--- id: pull-requests title: A pull request's whole life runs from the terminal status: open repos: [atgc] dependsOn: [sign-in] exitCriterion: > Open, read, list, edit, comment on, close, reopen and merge a pull request without opening a browser. --- # pull-requests A pull request is a record in the **author's** PDS carrying its own gzipped patches, not a branch on a server. Opening one needs no permission on the target repo, pushing a branch never updates one, and its state is a separate newest-wins log of status records rather than a field. `docs/architecture.md` is the long version; every surprise in this epic follows from it. What is here is the verbs and the listings. The three things that are their own line of work sit beside it: publishing the branch a pull claims is [branch-pulls](branch-pulls.md), the appview number that is in no record is [pull-numbers](pull-numbers.md), and reading somebody else's pull is [review](review.md). ## What it needs - [ ] **`pr reopen` on a pull the repo owner closed says "already open; nothing written" and exits 0.** Found by the appview state model on 2026-08-28. `pr close`/`pr reopen` resolve the current state from `list_statuses`, which walks the *pull author's* PDS and, when the acting account differs, the acting account's — and nobody else's. But atgc's own `Standing` says Tangled honors a status record from the author **or the target repo's owner**, so the owner's close lives in a third PDS that this walk never reads. The author then sees no status record at all, concludes `open`, and the reopen is a silent no-op against a pull tangled.org shows as closed. The same blindness makes the `merged` guard weaker than it reads: the command refuses to write over a merge it can see, and an owner's merge status is exactly one it cannot. The walk is careful about page caps for precisely this reason and then reads the wrong number of accounts. The fix wants the owner's DID, which is not free here — `owns_repo` answers "does this actor own it", not "who does", so resolving the owner from the repo DID is a lookup this path does not currently make. Decide where that comes from before adding a third walk - [ ] **The same walk in `issue close`/`issue reopen` has not been checked.** Issues carry the identical shape — a state record in the *acting* account's PDS, honored from the author or the repo owner, newest wins — and the pull version of it read the wrong set of accounts for long enough to hide a merge. Nothing here says the issue one is right; it says nobody has looked yet - [ ] `resolve` refuses `//` rather than resolving it. The knot serves that path, but answers it directly instead of redirecting, so there is no repo DID to read out of a `Location` header and nothing left to follow. It is not a remote Tangled hands out and `repo clone` won't parse one either, so this is a loud error where it used to be a silently wrong DID. Fixing it properly means asking the knot or the appview what repo that path names - [ ] Attach commit bodies to the PR body, which used to ride on the end of the entry above and is unrelated to any of it - [ ] `pr resubmit --target` / `pr edit --target` — retarget a pull in place. The target is a field of a record in your own PDS, so this is a plain `putRecord` and is what the stranded pull above actually wanted; the reason it is not here yet is downstream, not local. Tangled's own UI has no retarget, so how the appview renders a pull whose earlier rounds are patches against a base they never applied to is unproven. Wants a live test first, and probably has to write the retarget and a fresh round as one operation, since a retarget without one leaves the record naming a base its patches do not fit - [ ] `status pr` — the other half: PRs targeting your repos, split from the ones you authored (was `pr status`; see the `status` section) - [ ] `pr close` / `pr reopen` act for the pull's author or the target repo's owner only. The Go appview also honors any collaborator with `repo:push`, but Bobbin has no ACL and honors only author-or-owner, so a collaborator's close would show on tangled.org and be invisible in every `pr list`. Needs a knot ACL query to do properly - [ ] `pr close` / `pr reopen` keep the author-or-owner rule, which is a different trade: nothing refuses that write, so a collaborator's status record would land in their PDS, show on tangled.org and be dropped by Bobbin. Trying is free for a merge, where a knot answers; here it would be a write that silently means nothing - [ ] `status` — cross-repo overview: your open PRs and mentions. The name is free now and the group exists (see the `status` section); this is the bare `atgc status`, which today prints the group's help. `status pr` already covers the authored-PR half, so what is left is the mentions ## Done - [x] **`pr merge` read the target branch with `unwrap_or("main")`.** The guess `stack merge` stopped making, still live on the command people reach for more often — and both build the same `MergePlan` and hand it to the same `run_merge`, so the same knot call had a strict reader on one path and a default on the other. A record with a malformed `target.branch` landed a patch on a branch nobody named; a pre-rounds record carrying only `targetBranch`, which `pr view` has always read, merged onto `main` instead of the branch it asked for. Both unrecoverable. One reader owns it now, with the legacy fallback, refusing when neither field is there. Found by a subagent sweep for repeated rules - [x] **"The newest state record wins" had five orderings and three of them decided state.** `pr close`'s reader broke a tie on the raw `createdAt` *string* before the at-uri — a step the appview has no equivalent of, so `15:00:00+03:00` beat `12:00:00Z`, the same instant written two ways. `pr list`'s broke it on the record key alone, dropping the DID, which is exactly the half that differs when the two accounts Tangled honours both write at the same moment. `issue`'s was correct. So a pull could read `closed` from the command that closes it and `merged` from the command that lists it, off the same records — and two of the three carried doc comments asserting they agreed with each other. `lexicon::tangled::newer_state` is the one comparator, matching `stateWinner`: instant, then at-uri. The other two orderings are diagnostics that need determinism rather than the appview's rule, and are left alone with that written down - [x] A bare record key that is not yours says which account it was looked up in. `author_of_rkey` answers the acting account when nothing else names one, so a key belonging to somebody else is searched for in the wrong repository and comes back a flat 404 — "no sh.tangled.repo.pull record 3mu… in did:plc:bbb…" — naming a DID the reader never mentioned and had no reason to suspect. Both ways out are named now: an at-uri, or `--author `. Only for the bare spelling. An at-uri that resolves to nothing has already said whose account it meant, and the advice would be noise on a refusal that is already exact; both halves are pinned, because the hint is only useful while it stays rare - [x] `pr close`'s "these pulls depend on it" warning says **at least** when the listing it counted from was capped. `dependents_above` reasons carefully about a listing that will not *load* — say nothing, this is a note on a write that is happening anyway — and fell straight through the case where one loads truncated: members past the cap were 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. Found by prediction rather than by walking into it. Two of the three chain reads in `pr/write.rs` refuse a truncated listing before drawing a conclusion and this one did not, which is exactly where the frame in `docs/testing.md` says to look - [x] `issue close`/`issue reopen` were checked for the same defect and do not have it. They read the same two accounts — the issue's author and whoever is acting — and so are equally unable to see a repo owner's state record. What makes that survivable there and not here is that an issue has two states and neither is terminal: `read::state_of` refuses to call an absence `open` unless the author *also* owns the repo (`absence_is_open`), returns `Unknown` otherwise, and a write over an unseen state is at worst a redundant `closed` or a reopen the person asked for. A pull has `merged`, which is terminal and is the only record that the code ever landed anywhere, so the same blindness there destroyed information rather than duplicating it. Recorded because the two look identical at a glance and the next reader will want to know why only one of them needed an owner lookup. The collaborator hole is still open on both sides and needs a knot ACL query, not a different rule - [x] **`pr close`/`pr reopen` read the repo owner's status records, not just the author's and their own.** The `merged` refusal 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 rather than sitting beside it. The guard was right and the state it guarded on was not: `list_statuses` walked the pull's author and the acting account, and a merge performed by the repo owner — the ordinary way a contributor's pull gets merged — lands in the *owner's* PDS. The author saw no status record at all, fell through to `Open`, and atgc printed `open -> closed` over somebody's merge. The quieter form of the same blindness had `pr reopen` answer `already open; nothing written` and exit `0` against a pull the owner had closed. Getting the owner is the awkward part and there is exactly one route: a repo's DID document names its knot and nothing else, and the repo record that names an owner lives in that owner's PDS, which is the thing being looked for. `resolve::owner_of` asks the appview, which publishes the mapping as a `302` from `/` to `//`. That is a new dependency on a service these commands did not previously touch, and it is here because the alternative is writing over merges. An owner that will not resolve is refused rather than assumed absent, on the same reasoning as the page cap beside it — both are absences that cannot be told apart from "nobody has acted". The lookup is skipped when `standing_of` already found the acting account to be the owner, and when the pull names no repo - [x] `pr create` names a pull after the commit it *starts* with, which is what `stack create` has always done — "a member is titled and described by its bottom commit". The flat verb took the tip instead, so a branch was named after its least descriptive commit: the fixup, the test, the register entry. That is not a rare shape. Three pull requests opened against this repo in one day were titled `docs(plan): …` and renamed by hand afterwards, and every one of them was a branch whose author had been careful enough to record what they did. Breaking only in the sense that a default changed; `--title` was always there and is unaffected - [x] `browse --pr ` — the same flag, with the value every other `pr` verb takes. `--pr` was a boolean here and a value flag on `pr view`, `pr diff`, `pr comment` and the rest, so one spelling had two arities and the one command whose whole job is to open a page had no way to name the page. An *optional* value adds the missing half without taking the bare form away: `browse --pr` is still the branch's pull, and `browse --pr 23` opens one from a detached HEAD or from a branch it does not belong to. Free before 1.0; an arity change is breaking after it - [x] A second `pr create` on a branch that already has an open pull aimed at the same target says so. Several pulls per branch are legal — the two warnings beside this one argue it — so this is not a refusal. What it is is the shape a *retry* takes: `pr create` writes its record and then does best-effort work after it, so a caller that reads any failure as "nothing happened" and runs the command again opens a copy of the pull it already has, and nothing said so. An agent fleet retrying on a flaky network is exactly that caller, and this repo now carries the duplicate pulls to prove it. A closed pull on the branch deliberately does not fire it: opening a fresh pull after one was rejected is ordinary, and the way out should not come with a line saying you are doing it wrong - [x] `pr create` — patch-based PRs with --dry-run; DID resolved from the remote's git redirects - [x] `resolve` returns the *repo's* DID, not the owner's. It used to take the first `did:` segment in the path, so a remote spelled `tangled.org//` handed back the owner's account DID and every `pr` command silently asked the appview about the wrong subject. The rule now is that a DID naming a repo by itself is the repo's and a DID with a repo name after it is an account's — verified against the live appview and knot1 for all four path shapes. `repo clone`'s "discard a DID equal to the owner" workaround is gone with it - [x] the "no commits on " refusal from `pr create`/`resubmit` now says the submitted branch is the checked-out one and to check the right branch out — the situation behind nearly every hit, confirmed by four fresh-context agents reaching it from `main` - [x] a target branch that stops existing, at both ends. `pr create` warns when the target is the source branch of a pull that is not merged or closed — the shape that cannot survive its own base, since Tangled merges by rebasing and the branch goes with it — off the listing the stack warning already reads. `pr resubmit` asks `git ls-remote` whether the target is still there and refuses when it is definitively not; `refs/remotes//` outlives the branch it mirrors, so the round used to be built on a stale tip and quietly re-contain commits that had already landed. Only an empty answer from the remote refuses, never an unreachable one. Reported by an agent that hit it for real: a pull opened against an open stack's branch, stranded when the stack merged, recoverable only by closing and reopening - [x] `pr resubmit ` — the last verb that would not take its pull positionally, and so the last place the family's shape broke. Twelve `pr` verbs and seven `issue` verbs against one `--pr `, printed by `atgc agent` two lines above `pr edit 23`, which is where the two spellings were most likely to be read as a distinction that means something. `--pr` stays and is not deprecated: it was this verb's only spelling for its whole life, so it is in scripts and in older notes, and adding a spelling now costs nothing where removing one after 1.0 would be a breaking change. Naming the pull twice is refused, and naming it neither way is now this command's own message rather than clap's report of a missing flag - [x] `pr view ` — accept the same four pull spellings as its siblings; it was the one command in the family that refused a positional, and both fresh-context test agents typed `pr view 2` at it. An unlisted-but-named pull renders from its record - [x] `pr view` — print the current branch's PR: state, title, author, body and one line per round. Matches source.branch and stops there. --web opens the repo's pulls page - [x] It used to fall back to your newest PR on the repo, which since atgc's own PRs carried no source was the path they always took. From a second worktree that reported another branch's merged PR in the format of a right answer, and the `uri:` it printed was what got pasted into `pr close`. `pr diff`, `pr comment` and `pr checkout` guessed the same way and no longer do - [x] `pr status` — your PRs across repos, read from the subject's own PDS rather than Bobbin: every record it wants lives in one repository, so no index can be behind on it. Bobbin is still asked for states written by other people and for comment counts, but is no longer required. Target repo shown as owner/name, falling back to the appview's redirect when Bobbin has not indexed the repo; --state/--limit, --author and --source - [x] `pr comment` — **not** `sh.tangled.repo.pull.comment`, which is deprecated and whose creates the appview ingests as a no-op; the live record is `sh.tangled.feed.comment`, with a `subject` strongRef and a zero-based `pullRoundIdx` that is required when the subject is a pull (atgc's `--round` stays one-based, matching `pr diff`). No standing check: unlike pull statuses, neither the appview nor Bobbin filters who may comment - [x] `pr close` / `pr reopen` — state is a *log* of separate `sh.tangled.repo.pull.status` records, not a field on the pull, so both are an append of a new TID-keyed record and never a put or a delete (verified against the appview's `stateWinner`, which orders by `createdAt` desc then at-uri desc, and against its own web handlers, which create a fresh record for close, reopen and merge alike). The record goes in the *acting* account's PDS, so this works on a pull filed against your repo by somebody else. Reads the current state from the PDSes directly rather than from Bobbin, so a no-op is a no-op even while the index is behind, and refuses to close or reopen a merged pull, which would hide the merge - [x] `pr edit` — title and/or body of your own PR, via put_record on the record itself, leaving rounds untouched. `--body-file` (and `-` for stdin) rather than an `$EDITOR` spawn, which would hang under an agent. Author-only, unlike close/reopen: the pull record is in the author's PDS - [x] Images in PR bodies — `pr create` (which grew `--body-file` for it) and `pr edit` upload each local path a body's `![…](…)` names as an image/* blob (parallel, retries with backoff), rewrite it to the `blob+at://` URI Tangled's renderer resolves, and list it in the record's `blobs` array so the PDS keeps it. Relative paths never render on a pull page (only READMEs get `/raw/` rewriting), so a path that names no file is refused, not published. Stack bodies (from commit messages) get the same treatment in `stack create` and `stack resubmit`, with new blobs merged into a rewritten member's existing anchors by CID. `pr comment` too: `sh.tangled.markup.markdown` turned out to carry its own `blobs` array on the same image/*-at-1MB terms, so a comment body anchors images exactly as a pull body does - [x] `pr resubmit` and `pr edit` send the record's CID as `putRecord`'s `swapRecord` precondition, so two concurrent read-modify-writes no longer both succeed with one silently discarding the other's change. jacquard's `put_record` helper hardcodes `swapRecord: None`, so the request is built by hand — worth reporting upstream alongside the `client_id` bug - [x] `pr resubmit --pr` did not parse its argument at all — it took the last `/`-separated segment and called it a record key, so a number reached the PDS as the key `67` and came back "record not found" with no explanation, and somebody else's at-URI reached it as their key against *your* PDS. Now classified like every other reference, and a pull that is not yours is refused by name rather than by 404 - [x] `pr resubmit` — append a round (gzip patch) to an existing PR - [x] `pr merge` — knot XRPC (`sh.tangled.repo.merge`, `mergeCheck` first), authorized by a service-auth token the PDS mints per call (`crate::knot::xrpc`, the shared spelling of the dance `repo create` grew inline). `repo_facts` finds the knot to call, off the owner's own PDS or off the repo DID's document. After the knot merges, a merged status record is written per landed pull — the knot moved the branch, and without the records every listing keeps calling the pulls open. A stacked pull is refused toward `stack merge` - [x] `pr merge`/`stack merge` are not owner-only after all, and the ACL query this entry wanted is not needed to find that out. A knot authorizes `sh.tangled.repo.merge` with `IsPushAllowed(actor, repoDid)` and routes by `repo`, ignoring the `did`/`name` pair unless it is too old for the `repo-did-input` capability — so the merge can simply be sent, as tangled.org sends it, under the account merging rather than the owner. The owner-record lookup that doubled as the check is now a source of those two legacy fields, and the knot is found in the repo DID's own document when this account holds no record for it. A refusal comes back tagged `AccessControl` and is printed as what it is - [x] `pr list` and `pr list --all` resolved their columns one round trip at a time: one `handle_from_did_doc` per unique author DID and one `repo_name` per unique repo DID, both in plain `for` loops, so a page from fifteen authors was fifteen serial DID-document fetches, and the `--all` half worse — `repo_name` is up to three requests of its own. Both are now `buffer_unordered` at `LABEL_CONCURRENCY`, the spelling `images.rs` already used, joining `numbers_across_repos`'s `JoinSet` two hundred lines away in the same file. The dedupe the loops did by hand is `unique`, kept because one lookup per row would undo the point - [x] `newest` ordered records by comparing `createdAt` as a string, while `Listed::instant` two hundred lines above exists precisely because that does not work: PDS records stamp UTC and an index's come back in whatever offset the writer used, and `…Z` and `…+03:00` do not sort lexically against each other. It now parses, like the sort key it sits beside, and breaks a tie on the raw string so a set with no usable stamps still answers the same way twice. The reachable symptom was `pr view` on a branch with more than one pull behind it, which matches against the *merged* listing and so sees both formats at once. The sibling comparisons in `pr`'s `state_of` and `latest_states` are fine and stay — they read one PDS, so every stamp shares a writer and a format; `cmd/issue/read.rs`'s `newest_state` is not covered by that exemption, merging two accounts' records, and already parsed - [x] the exemption the entry above claimed for `state_of` and `latest_states` was wrong, and it was the one that decided a pull's state. One PDS does not mean one format: this account's own records hold `…+03:00` beside `…Z`, because Tangled's web UI writes the offset the browser was in, and precision wanders too (`.74Z`, `.8Z`, whole seconds). So a status record stamped `04:31:07+03:00` — 01:31Z — outsorted a reopen written at `02:00:00Z` half an hour later, and `…02:00:00.500Z` lost to `…02:00:00Z` because `'.' < 'Z'`. The pull read `merged`, `pr resubmit` refused it as merged and `pr reopen` saw nothing to do. Both now order on the parsed instant with the raw string as a tie-break, the shape `Listed::instant` and `newest` already use, so the doc comment's claim that the two agree stays true. The newest-missing name in the staleness warning was the same string max and moved with them - [x] `status pr` — your pull requests across every repo, moved out from under `pr`. `pr list` is a repo's pull requests and this is an account's, across all of them; that is the largest fact about either listing and neither word carried it while the two were sibling verbs. So the scope went into the first word, where it is read first: `atgc pr …` is this repo, `atgc status …` is not. A surface move and nothing else — same flags, same table, same `--json` rows — with the implementation left in `cmd/pr/read.rs` beside the two listings it shares a gather, a state filter and the `#` sweep with. It also frees the name the cross-repo overview in `misc` has wanted since before this command existed, which was the other half of the problem: the planned `atgc status` could not be added while a verb one level down was called that - [x] Dissolved, and the group with it. `status` was a container built to put scope in the first word, and once `tangled` left for `doctor` it held one verb — a family of one, whose whole justification was being a family. The listing went back to `pr` as `pr list --all`, where the scope is a flag among the other filters rather than a second verb whose name carries nothing. `pr status` and `status pr` are both gone; there is one spelling - [x] `--author` requires `--all`. Widening is per account and not per repo, the records being read from a PDS: a pull lives in the PDS of whoever wrote it, so "every repo" is answerable for one account and there is no listing of everyone's pulls everywhere for an author to narrow. A flag that silently meant something else in the narrow case would be worse than a refusal that says so - [x] `pr list --author` without `--all`: this repo, narrowed to one account. It needed no new gather — a repo listing's complete half was always one account's PDS, so naming somebody else only changes which DID is read — plus a post-filter for the index sources, which have every author in them. The work that was actually in it was the prose: four sentences about completeness said "your PDS" from a time when there was only one answer, and a listing of somebody else's pulls that says "your PDS" is the same class of wrong answer as a stale index. `Whose` in `cmd/pr/read/sources.rs` is those four sentences' one source of truth, and `--author` naming the selected account resolves back to `Mine` so it reads like the bare listing - [x] `browse` — open the repo's tangled.org page, with an optional section path (`atgc browse pulls`); --no-open only prints the URL - [x] `browse --pr` — the current branch's PR instead of a hand-typed section path. `pr view`'s resolution exactly, shared rather than copied: the same gather, the same `source.branch` match, the same refusal to fall back to a newer pull on another branch, so the page it opens is the pull `pr view` would print or the same error saying why there isn't one. Conflicts with a section argument by declaration rather than by precedence - [x] `browse --json` — the URL as data, with the pull's at:// URI beside it when `--pr` resolved one, and whether a browser was actually opened. The one command whose whole output was a URL was the one that could not be read by a script without parsing stdout - [x] A pre-rounds record — patch inline, no `rounds` array, the shape Tangled's own backfill produced — could be viewed and diffed but never landed. `pr view` read it; the reader every `stack` path and `pr merge` share knew only the rounds shape and refused it outright, while `round_count` called the same record one round. Three answers to "where does this pull keep its patch". `model::pull::latest_patch` is the one answer now, and the inline case is merged from the record itself without reaching for a blob