docs/superpowers/specs/2026-08-09-patch-revision-refs-design.md
Ref: Size: 13.3 KiB History
# Patches Without Branches — Design Date: 2026-08-09 Status: Draft ## Purpose A patch currently *is* a branch: `PatchState.branch` names a `refs/heads/*` ref (`state.rs:208`, `patch.rs:101`), and the patch's commits reach the remote only because the contributor pushed that branch by hand. This design removes the dependency. Each revision becomes an immutable ref inside the patch's own namespace, so a patch and all of its revisions travel under the refspecs `sync` already uses, and contributing requires no write access to `refs/heads/*`. ## Design test > If a feature only works when the server is `git-collab-server`, it is not a > git-collab feature. git-collab's claim is that collaboration lives in the repository and works between any two clones. Anything that needs a server-side hook, a receive-pack wrapper, or an intercepted push fails the test and is out of scope by construction. This rules out Gerrit-style `refs/for/<branch>`: it requires a server that rewrites a virtual ref into a patch, so it would work on this forge and nowhere else. `patch create` and `patch revise` remain the entry points, and they are client-side. ## Ref layout ``` refs/collab/patches/<id>/events the event DAG (today's refs/collab/patches/<id>) refs/collab/patches/<id>/rev/<oid> a revision's commit, pinned ``` The `events` rename is forced: git will not let `<id>` be both a ref and a directory. **Revision refs are named by commit OID, not by revision number.** An earlier draft used `r/<n>`, and it does not survive contact with concurrency: two clones revising offline each claim `r/2`, and the loser's push is rejected as a non-fast-forward *permanently* — a number the remote has already published cannot be renegotiated, so sync wedges on every retry. Reconciling the numbering across clones is possible but makes `r/<n>` stop meaning "revision n": three clones were observed converging on `r/2`, `r/3`, `r/4` pointing at the DAG's revisions 4, 3 and 2, consistently and permanently. Naming by OID dissolves the problem rather than managing it. The name is derived from the content, so a ref never moves, two clones can never disagree about it, and every push is additive by construction. Revision *numbering* stays purely a property of the DAG, which is where it already lives and where it is already authoritative — "what is revision 2" is answered by walking events, exactly as today. The refs' only job is to keep revision commits reachable. That job needs a set, not a sequence. `sync` already pushes and fetches `refs/collab/patches/*` (`sync.rs:230-233`, `sync.rs:344-347`). `*` crosses `/` in both git refspecs and git2's `references_glob`, so push, fetch and enumeration genuinely need no change — this was verified against the implementation, not assumed. A contributor's whole workflow becomes `git-collab sync`, where it is currently `git push origin <branch>` followed by `git-collab sync`. **`reconcile_refs` is the exception, and it is not optional.** It treats every fetched ref under the prefix as an event DAG. Revision refs point at *source* commits, which carry no event signature, so it would run `signing::verify_ref` over them and reject every patch on sync. Reconciliation must classify by ref shape: `<id>/events` reconciles as a DAG, revision refs are handled separately. Stating that the refspecs are unchanged is true and insufficient; the code behind them is not. Revision refs are never adopted from a remote. They are derived from the reconciled, signed DAG — a ref is written for each commit the DAG lists, and nothing outside the DAG can be referenced. A remote cannot plant a reachable object by pushing a ref the local DAG does not vouch for, so the boundary is closed by construction rather than by validation. Revision refs are write-once, and with OID naming this is trivially true: a ref whose name is its content has no reason to move. This makes the objects behind every revision permanently reachable — today they survive only while the branch happens to still reach them, so an author who rebases and force-pushes silently strips earlier revisions of their blobs and leaves interdiff and revision-anchored inline comments (`state.rs:133`, `state.rs:197`) pointing at objects `git gc` is entitled to delete. ## Identity Patch identity is declared, not derived. The id lives in the event DAG and is computed from nothing in the commits, so squashing, rebasing, reordering, or replacing the work wholesale leaves it intact. This is the role Gerrit fills with a `Change-Id` trailer and a client-side `commit-msg` hook. git-collab needs neither: the event is already a side-band that can say which patch a revision belongs to, so nothing has to be smuggled through the commit message. Duplicate detection changes accordingly. It currently scans open patches for one whose `branch` field matches (`patch.rs:106-112`), which is why patches break when worked on from ephemeral worktree branches — two worktrees on the same branch name collide, and one worktree with a generated branch name never matches its own patch. After this change a revision names its patch id explicitly and branch names carry no meaning. ## Base per revision `base_ref` stays on the patch: the target branch is a property of the patch. `base_commit` moves onto `Revision` as `base`, and `PatchState.base_commit` (`state.rs:209-211`) is removed rather than kept as a second source of truth. ``` patch a1b2c3d4 base_ref = "main" r1 commit A1 base B1 branched off main@B1 r2 commit A2 base B1 no rebase — base unchanged r3 commit A3 base B2 main moved; rebased onto B2 ``` `base` is `merge_base(<base_ref tip>, <revision commit>)`, computed when the revision is recorded and stored. Merge-base rather than branch tip: if the base branch advances and the author does not rebase, the merge-base is unchanged, so `base` moves only when the author actually moves. "Did the author rebase between these two revisions" is then exactly "did `base` change". The computation already exists — `resolve_base_tree` (`patch.rs:454-466`) does it at display time. The change is *when*. Recomputing at display time is what loses the information: by the time an interdiff is requested, the base branch has moved and there is no way to reconstruct where an earlier revision stood. ## Interdiff `interdiff` (`patch.rs:501-525`) is currently a flat tree-to-tree diff between two revision trees with no base awareness. When a rebase separates the two revisions, the result includes every upstream commit rebased over, burying the author's actual response to review. Single-revision `diff` does not have *that* problem — it three-dots through `resolve_base_tree` — but an earlier draft of this spec claimed it was therefore fine, and that was wrong. `resolve_base_tree` diffs against `merge_base(base_tip, head)`, so once the patch is merged by fast-forward the merge-base *is* the head and the diff renders empty. Every merged patch's historical diff is unreadable, which is when a reviewer is most likely to want it. Tracked as `57575b50`; it is a separate defect from the interdiff, not a consequence of it. With `base` on each revision, there are two cases. **Bases equal.** Tree diff, as today. This is also the pure-squash case: squashing does not change the resulting tree, so the interdiff is empty. That is the correct answer, and a useful one — it tells the reviewer the author restructured commits and changed no code. **Bases differ.** A rebase happened. Cherry-pick the *older* revision onto the *newer* revision's base, then tree-diff the result against the newer revision. Upstream churn cancels and only the author's changes remain. `git2::merge_commits` does this in memory; no working tree or checkout is involved. The direction is load-bearing. The newer revision is the one under review and must be shown exactly as its author recorded it; the older revision is the reference point being brought forward. Replaying the newer one backwards would show the reviewer a synthesised version of the revision they are evaluating. A conflicting cherry-pick is reported as such. It means upstream and the patch touched the same lines, and there is no honest "only the author's changes" view to render. `git range-diff` is the fallback worth considering here, but not as the primary path: it emits a format nothing downstream — web UI, TUI, or inline comment anchoring — knows how to render. ## What the branch was doing Four things depend on `PatchState.branch` today and each needs a replacement. | Today | After | |---|---| | `resolve_head` looks up `refs/heads/<branch>` (`state.rs:433-446`) | Latest revision's commit, from the DAG | | Staleness, ahead/behind against base tip (`state.rs:448-456`) | Unchanged; anchored to the latest revision | | Merged detection (`check_auto_merge`, `state.rs:464-486`) | See below — not unchanged | | `patch checkout` creates a local branch (`patch.rs:663-668`) | Unchanged; branches from the latest revision | `resolve_head` already accepts an OID before falling back to a name lookup (`state.rs:436`), so the OID path is partly built. ### Merge detection needs explicit rework `check_auto_merge` reads `self.base_commit` to decide whether the base branch has moved (`state.rs:475-479`), and treats `None` as "not moved" via `unwrap_or(false)`. Dropping `base_commit` as proposed above would therefore silently disable merge detection for every patch — `base_moved` would always be false and no patch would ever reach `Merged`. This must be rewritten to read the base of the *latest* revision, not deleted and left to default. It also calls `resolve_head`, so today it silently no-ops when `refs/heads/<branch>` is absent — `resolve_head` errors, the `let Ok(..) else` returns, and the patch stays `Open` with no diagnostic. This was confirmed behaviourally while writing server tests: merge detection requires the source branch to have been pushed to the server, not merely the base branch. Anchoring to the revision refs removes that requirement, since they are pushed by `sync` whether or not any branch was. Both are reasons the "Merged detection" row is not the no-op it first appears to be, and it should be treated as its own unit of work with its own tests. Revision recording is the one behaviour that is genuinely lost. Revisions are currently inferred while applying events: an observed commit not already in the revision list becomes a new revision (`state.rs:578-581`). With no branch to observe, `patch revise` becomes the explicit act that records one. This is the same operation Gerrit performs on a push to `refs/for`, moved to the client. ## Ref lifecycle `delete` removes a single reference (`patch.rs:677-681`) and `close` archives one via `state::archive_patch_ref` (`patch.rs:694-698`). Both now operate on a subtree: `<id>/events` plus every `<id>/rev/<oid>`. Archiving must move the whole subtree under `refs/collab/archive/patches/<id>/` so that closed patches keep their revisions and remain reviewable. ## Migration Pre-release; no compatibility guarantee. Patches in the old layout are migrated on first use rather than supported indefinitely: - `refs/collab/patches/<id>` becomes `<id>/events`. - Each recorded revision with a non-empty `commit` gets a `rev/<oid>` ref, provided the object is still present. Revisions whose objects were already lost to a force-push cannot be recovered and keep today's `""`-means-unknown convention (`state.rs:164-167`). - Revisions with no stored `base` fall back to recomputing the merge-base against the current base branch, which is exactly today's behaviour — old patches degrade to what they already do rather than failing. ## Testing - A patch created and synced with no `git push` of any branch is fully visible to a second clone, including its revision commits. - Squashing a revision's commits without changing the tree produces an empty interdiff. - Rebasing onto a base branch that has advanced produces an interdiff containing only the author's changes. - A rebase that conflicts with upstream reports a conflict rather than emitting a misleading diff. - An inline comment anchored to revision 1 still resolves after the author rebases and records revision 2. - Two worktrees with generated branch names each revise their own patch without collision. - A closed patch retains all revision refs under the archive namespace. - A patch migrated from the old layout diffs and interdiffs correctly. ## Consequences for governance `2026-08-09-repo-governance-design.md` assumes a server-side `pre-receive` hook and `refs/for` conversion. Both fail the design test and are withdrawn; that document needs revising against this one. The relevant consequence here is that a contributor no longer needs write access to `refs/heads/*` at all — only to `refs/collab/*`. This server's authorization is per-repository and cannot express that distinction, so it does not by itself constrain a contributor on this forge. On any host that does have per-ref control — GitHub branch protection, Gitea, gitolite — the constraint applies for free, because the access is never requested. The remaining question of how this forge scopes agent credentials belongs to the governance document. ## Out of scope - `refs/for/<branch>` and any server-side push interception. - Server-side enforcement of ref classes. - Signature verification changes; revision refs are ordinary commits and the event DAG remains the signed surface.