61a52f8d

Design patches as revision refs instead of branches

a73x 2026-08-09 16:12

Commit message

Design patches as revision refs instead of branches

Each revision becomes an immutable ref in the patch's own namespace, so
patches travel under the refspecs sync already uses and contributing needs
no write access to refs/heads/*. Records the base per revision so interdiff
survives a rebase.

Marks the server-side parts of the governance design superseded: the
pre-receive hook, refs/for conversion and in-band policy all require a
git-collab-specific server, which the design test rules out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs/superpowers/specs/2026-08-09-patch-revision-refs-design.md

@@ @@ -0,0 +1,204 @@ @@
+# 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>/r/<n>      revision n's commit — immutable, write-once
+```
+The `events` rename is forced: git will not let `<id>` be both a ref and a
+directory.
+`sync` already pushes and fetches `refs/collab/patches/*` (`sync.rs:230-233`,
+`sync.rs:344-347`), so revision refs are carried by the existing refspecs with no
+change. A contributor's whole workflow becomes `git-collab sync`, where it is
+currently `git push origin <branch>` followed by `git-collab sync`.
+Revision refs are write-once. A revision is never rewritten; a changed patch adds
+`r/<n+1>`. 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 this
+problem — it three-dots through `resolve_base_tree` — so only the interdiff path
+needs fixing.
+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 `r/<n>` |
+| Staleness, ahead/behind against base tip (`state.rs:448-456`) | Unchanged; anchored to latest `r/<n>` |
+| Merged detection by reachability from base (`state.rs:460`) | Unchanged; anchored to latest `r/<n>` |
+| `patch checkout` creates a local branch (`patch.rs:663-668`) | Unchanged; branches from `r/<n>` |
+`resolve_head` already accepts an OID before falling back to a name lookup
+(`state.rs:436`), so the OID path is partly built.
+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>/r/<n>`. 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 an `r/<n>` 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.

docs/superpowers/specs/2026-08-09-repo-governance-design.md

Old		New
@@ -1,7 +1,21 @@
1	# Repository Governance — Design	1	# Repository Governance — Design
2		2
3	Date: 2026-08-09	3	Date: 2026-08-09
4	Status: Draft — open questions at the end must be settled before implementation	4	Status: Superseded in part — needs revising against
		5	`2026-08-09-patch-revision-refs-design.md`
		6
		7	That document adopts the design test "if a feature only works when the server is
		8	`git-collab-server`, it is not a git-collab feature." Three parts of this design
		9	fail it and are withdrawn: the `pre-receive` hook (Enforcement · At receive), the
		10	`refs/for` contribution class and its conversion (Q3), and in-band policy on
		11	`refs/collab/policy`, which was only enforceable by the hook. Governance returns
		12	to a settings repository and per-repository read/write authorization. Q2 is
		13	answered in the affirmative by that return; Q1 is unaffected.
		14
		15	The ref-class table goes down with the hook as an enforcement mechanism, but
		16	survives as a description of what a host with per-ref control can apply — this
		17	forge cannot. The threat model, the failure modes and the repository lookup key
		18	carry into the revision unchanged.
5		19
6	## Purpose	20	## Purpose
7		21