---
id: branch-pulls
title: A pull publishes the branch it claims
status: open
repos: [atgc]
dependsOn: [pull-requests]
exitCriterion: >
  Every pull whose record carries a `source` has that branch really on the
  knot, on the round the record's newest patch describes.
---

# branch-pulls

`source: {branch}` is a claim the appview believes and never checks. atgc used
to write it over a branch that had never been pushed, which is what made the
tree link on a pull page describe code nobody could fetch.

The default now pushes the branch to the target's knot and asks
`sh.tangled.repo.compare` what it holds, and `--patch-only` keeps the old local
`format-patch` and writes no `source` at all — because a `source` is exactly
the claim that a branch is there. Rounds republish under a
`--force-with-lease` read off the last round's own patch.

What is left is the third shape (a pull from a fork), the branch a
`--patch-only` pull came off, and retargeting one in place. The entry that
asked whether `stack create` should keep writing `source` is not repeated here:
it was answered by pushing the branch, the same way the flat pull was, and
`src/cmd/stack/write.rs` opens with the reasoning.

## What it needs

- [ ] `stack resubmit` should re-push the branch, and today does not — the
      sibling of the `pr resubmit` entry below and left out for the same
      reason: one PR, one thing. Now that `stack create` publishes, a
      reconcile after an amend leaves the knot holding the pre-amend branch,
      so `resubmitCheck` says "should resubmit" forever and the tree link
      describes commits no member's patch matches. The push itself is one
      line; what the entry has to settle is force, since every reconcile this
      command exists for rewrites history and a plain push will be refused
- [ ] `pr resubmit` on a branch-based pull should re-push and re-compare,
      and today does neither. Deliberately left out of the `pr create` change
      so that one PR did one thing, and it is a real gap rather than a tidy
      one: after a resubmit the record's newest round is the local patch
      while the branch on the knot is whatever was last pushed, so the tree
      link and the diff describe different code. The shape is already there —
      `push_and_compare` in `cmd/pr/write.rs` is the whole of it — and the
      question the entry has to answer first is how a resubmit knows which
      shape it is appending to, since the pull record says (`source` present
      and no `source.repo`) but a `--patch-only` pull must not suddenly grow
      one
- [ ] the branch a `--patch-only` pull came off is recorded nowhere, so
      `pr view`, `browse --pr`, `pr diff` and `pr checkout` cannot find it
      from the checkout it was opened in. `pr create --patch-only` says so
      and prints the at-uri, which is the honest stopgap and not an answer.
      The obvious candidate is still a trailer in the patch beside
      `Change-Id:`, and the reason it was not just done is that
      `read::for_branch` matches over *listing items*, which hold a blob CID
      and not the patch — so matching on a trailer means fetching a blob per
      candidate. Worth measuring before building: the candidate set is one
      account's pulls on one repo, which is usually small
- [ ] fork-based pulls, the third shape and the one `pr create` deliberately
      does not implement. Harder than the branch shape in three separate
      ways: `sh.tangled.repo.hiddenRef` on the *fork's* knot under service
      auth, push access on the fork rather than on the target, and the
      compare run there rather than here — and the appview only recognises a
      fork it already knows (`db.GetForkByRepoDid`), so a fork atgc created
      by other means does not qualify. `--patch-only` is the answer until
      this exists

## Done

- [x] `pr create` — records `source: {branch}`. The reasoning that stood
      here was wrong on a checkable fact, so it is corrected rather than
      quietly dropped: this is **not** "the shape every pull request written
      by anything other than atgc carries". Tangled's own web UI passes
      `pullSource = nil` for a patch-based pull
      (`appview/pulls/create.go`, `handlePatchBasedPull`), and only its two
      branch shapes write a `source` at all. What the field buys atgc is
      real — it is the only thing matching a pull back to the branch it came
      off, which `pr view` needs — but the appview reads it as a claim and
      not as a hint. See the entry below
- [x] `pr create` pushes the branch by default, with `--patch-only` for the
      shape it has today. Read off Tangled core at master `1adde466` rather
      than guessed, so the ground was settled before the work started.

      **There is no patchless pull**, and there still isn't:
      `lexicons/pulls/pull.json` requires `rounds` and every round requires
      `patchBlob`, so the patch is gzipped, uploaded and put in the round
      exactly as before. What changed is who computes it and what the record
      is then entitled to claim.

      The default pushes the branch to the target's knot over SSH, `GET`s
      `sh.tangled.repo.compare?repo=<repoDid>&rev1=<target>&rev2=<source>` —
      a public query, no scope and no session — refuses an empty comparison
      as the appview does, and writes `source: {branch}` over a branch that
      is now really there. `--patch-only` keeps the old local
      `git format-patch` and writes **no** `source`, because a `source` is
      exactly the claim that a branch is on the knot and the appview never
      checks it.

      Two things learned in the doing, both cheap to get wrong. The knot
      serializes `types.RepoFormatPatchResponse` with Go's names, not the
      lexicon's: the mailbox arrives as `patch` and the per-commit list as
      `format_patch`, and every field is `omitempty`. And a scenario can take
      a real push without a git transport on the mock knot — the checkout
      carries `url.<bare>.pushInsteadOf`, which rewrites the push target and
      leaves `git remote get-url` alone, so the repo DID is still resolvable
      out of the remote path. Plain `insteadOf` would not have worked;
      `get-url` applies it
- [x] `stack create` pushes the branch and keeps `source: {branch}` on every
      member, still formatting each member's patch locally. Read off Tangled
      core at master `e4eb96e9`, and the read made the choice rather than
      narrowing it.

      The claim was never the wrong *shape*: `newStack` in
      `appview/pulls/create.go` gives every member of its own stacks the same
      `pullSource` while storing one commit's `fp.Raw` as that member's patch.
      A member's `source` says where the stack came from; its patch says which
      commit of it. So there was nothing to fix in the record — only the push
      that makes it true, exactly as for the flat pull.

      Dropping `source` was never open, which the read settled in one pass.
      `resubmitCheck` in `appview/pulls/single.go` compares the *top* member's
      sha (the stack is stored top-first) against the branch's live head;
      `resubmitBranch` routes the web's resubmit through `repo.compare` and
      re-splits the range by change-id — it dispatches on `IsBranchBased`
      before it ever looks at the stack, so a sourceless member degrades to a
      paste-a-patch box for one commit; `compose.go` will not compose a
      stacked pull that is patch-based at all
      (`isStacked := mode == "stack" && !isPatchBased`). And closer to home,
      `read::for_branch` matches on `source.branch`, which is how `stack
      view`, `stack resubmit` and `stack merge` find a chain — a sourceless
      stack is one atgc could never pick up again. Hence no `--patch-only`
      here: a push that fails refuses and points at `pr create --patch-only`.

      The compare is still asked for, demoted from patch source to proof: an
      empty answer refuses as `handleBranchBasedPull` does, a count that
      disagrees with the plan is a note, and `sourceRev` needs nothing written
      at all — `PullSubmissionFromRecord` derives it from the blob's last
      patch, so the top member's sha becomes the branch head for free.

      One thing learned that nothing above predicted:
      `knotserver/git/diff.go` adds the `Change-Id:` header to a knot's
      format-patch only from a commit object's jj `change-id`, never from a
      `Change-Id:` trailer. So a `--add-change-ids` branch makes a stack the
      *website* cannot resubmit — the appview finds no ids in what the knot
      hands it. `stack create` says so when the compare shows it, and
      `stack resubmit` is unaffected because it injects the headers itself
- [x] `pr resubmit` on a branch-based pull re-pushes and re-compares, so
      the newest round and the branch on the knot describe the same code
      again. Which shape a round follows is read off the record and never
      off a flag — `source` present and no `source.repo` is branch-based —
      which is why there is no `--patch-only` on `pr resubmit` to match the
      one on `pr create`.

      The three awkward cases, decided. A `--patch-only` pull stays
      patch-only forever: a round that grew a `source` would start claiming
      a branch the pull's earlier rounds never published, and the appview
      believes `source` without checking it, so the pull would look
      branch-based back to its first round. Reopening with `pr create` is
      the way to change shape, and it is honest about it. A fork-based pull
      (`source.repo` set, which only the web UI opens) is refused: the
      compare has to run on the fork's knot, which is the same thing that
      keeps `pr create` from opening one. And a round raised from a branch
      other than the one the record names is refused rather than resolved
      either way round — pushing the checked-out branch under the recorded
      name moves a branch nobody asked about, pushing the recorded name from
      here sends commits the round never read — which is also the answer for
      a source branch that no longer exists locally: fetch it from the knot
      and check it out
- [x] a round republishes its branch with a lease, which is what settled the
      force question `stack resubmit` was waiting on. Both resubmits push
      `--force-with-lease=<branch>:<sha>`, and the sha is read off the last
      round's own patch — the `From <sha>` line git and the knot both write
      (`knotserver/git/diff.go` runs plain `format-patch`, no
      `--zero-commit`) — never off the remote-tracking ref, whose argument-
      less lease a `git fetch` quietly disarms by advancing it onto the
      commits worth protecting. So a rewritten branch lands without a manual
      force-push, and a branch somebody else moved refuses with both shas
      named, before anything is sent. `stack resubmit` pushes at all now,
      including on the no-op path, since records that already match an
      unpushed branch are the state it has to repair. With no lease to be
      had — a create, or a patch that names no head — the push is unleased
      and fast-forward-only, which is what it always was