a73x

77912c54

Correct two errors in the merge-recording design

a73x   2026-08-10 07:01

Commit message
Correct two errors in the merge-recording design

The --fixes guards forbade closing alongside a no-op PatchMerge, which made
the retry the next paragraph called for unreachable: on a retry pass the
merge is necessarily a no-op. Restated as an invariant - a merged patch's
fixes issue is closed - so both the inline close and the retry fall out.

Also records that the archived-patch warning is conditional on a walk
happening, which the walk bound can prevent when the archived patch is the
only one on its base.

Both found during implementation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/superpowers/specs/2026-08-09-merge-recording-design.md
Old New
@@ -120,27 +120,43 @@ must never break sync:
120 | Commit unreadable | Warn, continue | 120 | Commit unreadable | Warn, continue |
121 | Revwalk cannot start | Return `Err`; caller skips the scan for this sync | 121 | Revwalk cannot start | Return `Err`; caller skips the scan for this sync |
122 122
123 The archived-patch row is **conditional on a walk happening at all**, and the
124 bounding rule above can prevent one. If an archived patch is the only patch on
125 its base branch, that base is not walked, so no trailer is read and no warning is
126 emitted. The optimisation is right and the warning is a courtesy, so the
127 optimisation wins — but the table overstates the guarantee and the tests should
128 encode the conditional behaviour rather than the absolute.
129
123 ## Auto-closing the referenced issue 130 ## Auto-closing the referenced issue
124 131
125 `--fixes` is documented as "auto-closes on merge" (`cli.rs:309`) and nothing 132 `--fixes` is documented as "auto-closes on merge" (`cli.rs:309`) and nothing
126 implements it. It could not have worked reliably before this design, because 133 implements it. It could not have worked reliably before this design, because
127 merge itself was not reliably detectable. 134 merge itself was not reliably detectable.
128 135
129 When a `PatchMerge` is emitted, an `IssueClose` for the patch's `fixes` issue is 136 The close is driven by an **invariant**, not by an emission: *a merged patch's
130 emitted **at the same moment, in the same operation**. Not during derivation — 137 `fixes` issue is closed*. Anything that establishes or observes the merge works
131 that is the constraint above, and it applies with equal force here. 138 to restore it.
139
140 An earlier draft specified this the other way — "emit the close only alongside a
141 `PatchMerge` that was actually emitted, never alongside one that was a no-op" —
142 and that is self-defeating. Closing writes to a different ref than the merge, so
143 the two are not atomic; if the close fails, the merge still stands, and on the
144 next pass the merge is *necessarily* a no-op. The retry the same section called
145 for was therefore unreachable by its own rule.
146
147 Stated as an invariant, both cases fall out:
148
149 - A freshly recorded merge closes its issue inline, in the same operation.
150 - A patch already merged with its `fixes` issue still open has the close retried.
151
152 Either way it is a write performed by an operation the user invoked — never
153 during derivation. That constraint is unchanged and applies with equal force.
132 154
133 Guards: 155 Guards:
134 156
135 - Skip if the issue is already closed, so re-running the scan appends nothing. 157 - Skip if the issue is already closed, so repeated scans append nothing.
136 - Skip and warn if the issue does not resolve or is ambiguous. 158 - Skip and warn if the issue does not resolve or is ambiguous.
137 - Emit the close only alongside a `PatchMerge` that was *actually* emitted, never 159 - Both paths must be idempotent, since the retry pass runs on every scan.
138 alongside one that was a no-op because the patch was already merged.
139
140 Closing writes to a different ref than the merge, so the two are not atomic. If
141 the close fails the merge still stands; the next scan sees a merged patch with an
142 open `fixes` issue and retries the close. Make that path explicit rather than
143 letting it fall out of the ordering.
144 160
145 ## Stamping the trailer 161 ## Stamping the trailer
146 162