07efb856

Correct the merge-detection section of the revision-refs design

a73x 2026-08-09 16:47

Commit message

Correct the merge-detection section of the revision-refs design

check_auto_merge reads base_commit and treats None as "base has not moved",
so dropping that field as the spec proposed would have silently disabled
merge detection for every patch rather than leaving it unchanged.

It also resolves the source branch, so it no-ops today when the branch was
never pushed to the server - confirmed behaviourally while writing server
tests for the web list filtering.

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

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

Old		New
@@ -131,12 +131,32 @@ Four things depend on `PatchState.branch` today and each needs a replacement.
131	\|---\|---\|	131	\|---\|---\|
132	\| `resolve_head` looks up `refs/heads/<branch>` (`state.rs:433-446`) \| Latest `r/<n>` \|	132	\| `resolve_head` looks up `refs/heads/<branch>` (`state.rs:433-446`) \| Latest `r/<n>` \|
133	\| Staleness, ahead/behind against base tip (`state.rs:448-456`) \| Unchanged; anchored to latest `r/<n>` \|	133	\| Staleness, ahead/behind against base tip (`state.rs:448-456`) \| Unchanged; anchored to latest `r/<n>` \|
134	\| Merged detection by reachability from base (`state.rs:460`) \| Unchanged; anchored to latest `r/<n>` \|	134	\| Merged detection (`check_auto_merge`, `state.rs:464-486`) \| See below — not unchanged \|
135	\| `patch checkout` creates a local branch (`patch.rs:663-668`) \| Unchanged; branches from `r/<n>` \|	135	\| `patch checkout` creates a local branch (`patch.rs:663-668`) \| Unchanged; branches from `r/<n>` \|
136		136
137	`resolve_head` already accepts an OID before falling back to a name lookup	137	`resolve_head` already accepts an OID before falling back to a name lookup
138	(`state.rs:436`), so the OID path is partly built.	138	(`state.rs:436`), so the OID path is partly built.
139		139
		140	### Merge detection needs explicit rework
		141
		142	`check_auto_merge` reads `self.base_commit` to decide whether the base branch has
		143	moved (`state.rs:475-479`), and treats `None` as "not moved" via
		144	`unwrap_or(false)`. Dropping `base_commit` as proposed above would therefore
		145	silently disable merge detection for every patch — `base_moved` would always be
		146	false and no patch would ever reach `Merged`. This must be rewritten to read the
		147	base of the latest revision, not deleted and left to default.
		148
		149	It also calls `resolve_head`, so today it silently no-ops when
		150	`refs/heads/<branch>` is absent — `resolve_head` errors, the `let Ok(..) else`
		151	returns, and the patch stays `Open` with no diagnostic. This was confirmed
		152	behaviourally while writing server tests: merge detection requires the source
		153	branch to have been pushed to the server, not merely the base branch. Anchoring
		154	to `r/<n>` removes that requirement, since the revision ref is pushed by `sync`
		155	whether or not any branch was.
		156
		157	Both are reasons the "Merged detection" row is not the no-op it first appears to
		158	be, and it should be treated as its own unit of work with its own tests.
		159
140	Revision recording is the one behaviour that is genuinely lost. Revisions are	160	Revision recording is the one behaviour that is genuinely lost. Revisions are
141	currently inferred while applying events: an observed commit not already in the	161	currently inferred while applying events: an observed commit not already in the
142	revision list becomes a new revision (`state.rs:578-581`). With no branch to	162	revision list becomes a new revision (`state.rs:578-581`). With no branch to