docs/superpowers/plans/2026-09-06-mr04-mr-lifecycle.md
409 lines · 20222 bytes
1# MR 4: Merge request lifecycle Implementation Plan
2
3> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4
5**Goal:** Close four parity rows at once — `draft, ready`, `retarget`, `request a review`, and `stacked merge requests`.
6
7**Architecture:** All four are already on the wire and none needs a server change. `mr show` and `mr list` carry `draft`, `review_requests`, `stacked_on` and `stacked` as `omitempty` fields the iOS models simply do not decode yet. So this is decoding plus four write actions on a screen that already has a write path.
8
9**Tech Stack:** Swift 6, default `MainActor` isolation, iOS 26.5, SwiftUI, Swift Testing, `@Observable`.
10
11**Spec:** `docs/superpowers/specs/2026-09-06-ios-parity-design.md`
12
13## Global Constraints
14
15- Swift 6 language mode, default `MainActor` isolation. Wire models are `nonisolated struct`s.
16- Swift Testing only — never XCTest.
17- `gitbayTests` is hermetic and offline; network goes through `StubProtocol`.
18- New files under `gitbay/` and `gitbayTests/` need **no** `project.pbxproj` edit.
19- The label model is `IssueLabel`, never `Label`.
20- Never mention Claude, LLMs or AI in commits, comments, or the merge request. No `Co-Authored-By` trailer.
21- Never commit to `main`.
22
23**The commands, verbatim from the registry:**
24
25```
26mr draft <owner/name> <n>
27mr ready <owner/name> <n>
28mr retarget <owner/name> <n> <branch>
29mr review request <owner/name> <n> [--add <user>]... [--remove <user>]...
30mr create <target owner/name> --source [owner/name:]<branch> --target <branch>
31 --title <t> [--body <b> | --file -] [--format md|org] [--draft]
32repo refs <owner/name>
33```
34
35Note `mr review request` is a **three-word path** — `["mr", "review", "request", repo, n]`. The app already sends `["mr", "review", verdict, ...]` for approvals, so getting this wrong silently posts a review instead of requesting one.
36
37**The wire fields**, from `internal/control/mr.go:337-357` — all `omitempty`, all currently undecoded by iOS:
38
39```go
40Draft bool `json:"draft,omitempty"`
41ReviewRequests []string `json:"review_requests,omitempty"`
42StackedOn *stackRef `json:"stacked_on,omitempty"` // {number, title}
43Stacked []stackRef `json:"stacked,omitempty"` // [{number, title}]
44```
45
46`mr list` rows carry `draft` and `stacked_on`; `mr show` carries all four.
47
48**Build and test command:**
49
50```bash
51xcodebuild -project gitbay.xcodeproj -scheme gitbay \
52 -destination 'platform=iOS Simulator,name=iPhone 17' \
53 test -only-testing:gitbayTests
54```
55
56Baseline: **258 passed, 0 failed, 1 skipped** (`LiveInstanceTests` — expected).
57
58---
59
60## File Structure
61
62| File | Responsibility |
63|------|----------------|
64| `gitbay/MRs/MRModels.swift` (modify) | Decode `draft`, `review_requests`, `stacked_on`, `stacked` |
65| `gitbay/MRs/MRDetailViewModel.swift` (modify) | `setDraft`, `retarget`, `requestReview`, `removeReviewRequest`, branch list |
66| `gitbay/MRs/MRCreateViewModel.swift` (modify) | `--draft` on create |
67| `gitbayTests/MRLifecycleTests.swift` (create) | Every test in this plan |
68| `gitbay/Views/MRs/MRView.swift` (modify) | Draft badge, stack section, reviewer rows, the three actions |
69| `gitbay/Views/MRs/MRListView.swift` (modify) | Draft badge, "stacked on" hint |
70
71---
72
73### Task 1: Decode the four fields
74
75**Files:** Modify `gitbay/MRs/MRModels.swift`; test in `gitbayTests/MRLifecycleTests.swift`
76
77**Interfaces produced:**
78- `MergeRequest.draft: Bool` (defaulting false when absent) and `MergeRequest.stackedOn: StackRef?`
79- `MRDetail.draft: Bool`, `.reviewRequests: [String]`, `.stackedOn: StackRef?`, `.stacked: [StackRef]`
80- `nonisolated struct StackRef: Decodable, Sendable, Hashable, Identifiable` — `number: Int64`, `title: String`, `id` is `number`
81
82**The subtlety this task exists to get right.** All four keys are `omitempty`, so **absence is the common case**: a non-draft merge request has no `draft` key at all, not `"draft": false`. `Bool` is not `Optional`, so a plain `let draft: Bool` fails to decode the ordinary merge request. Either declare them optional and expose non-optional accessors, or write `init(from:)` with `decodeIfPresent` and a default. The tests below pin the absent case for every field, because that is what production sends most of the time.
83
84`draft` is a flag, not a fifth state: `state` stays `"open"`. A draft merge request is open but not asking — it does not merge and does not appear in a review queue. `isOpen` must keep meaning what it means.
85
86- [ ] **Step 1: Write the failing tests**
87
88Create `gitbayTests/MRLifecycleTests.swift`:
89
90```swift
91import Foundation
92import Testing
93@testable import gitbay
94
95private func decodeDetail(_ json: String) throws -> MRDetail {
96 let decoder = JSONDecoder()
97 decoder.dateDecodingStrategy = .iso8601
98 return try decoder.decode(MRDetail.self, from: Data(json.utf8))
99}
100
101private func decodeRow(_ json: String) throws -> MergeRequest {
102 let decoder = JSONDecoder()
103 decoder.dateDecodingStrategy = .iso8601
104 return try decoder.decode(MergeRequest.self, from: Data(json.utf8))
105}
106
107private let plainDetail = """
108 {"number":7,"title":"a change","state":"open","author":"cmc",\
109 "source":"feat","target_ref":"main","head_sha":"abc123",\
110 "created_at":"2026-09-01T00:00:00Z"}
111 """
112
113struct MRLifecycleDecodingTests {
114
115 /// Every one of these keys is omitempty. The ordinary merge request
116 /// carries none of them, so absence must decode, not throw.
117 @Test func anOrdinaryMergeRequestHasNoneOfTheNewKeys() throws {
118 let mr = try decodeDetail(plainDetail)
119 #expect(mr.draft == false)
120 #expect(mr.reviewRequests.isEmpty)
121 #expect(mr.stackedOn == nil)
122 #expect(mr.stacked.isEmpty)
123 }
124
125 @Test func aDraftDecodesAndStaysOpen() throws {
126 let mr = try decodeDetail("""
127 {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\
128 "source":"feat","target_ref":"main","head_sha":"abc123",\
129 "created_at":"2026-09-01T00:00:00Z"}
130 """)
131 #expect(mr.draft)
132 // Draft is a flag, not a fifth state.
133 #expect(mr.state == "open")
134 #expect(mr.isOpen)
135 }
136
137 @Test func reviewRequestsDecode() throws {
138 let mr = try decodeDetail("""
139 {"number":7,"title":"a change","state":"open","author":"cmc",\
140 "review_requests":["rae","sam"],\
141 "source":"feat","target_ref":"main","head_sha":"abc123",\
142 "created_at":"2026-09-01T00:00:00Z"}
143 """)
144 #expect(mr.reviewRequests == ["rae", "sam"])
145 }
146
147 @Test func bothHalvesOfAStackDecode() throws {
148 let mr = try decodeDetail("""
149 {"number":7,"title":"a change","state":"open","author":"cmc",\
150 "stacked_on":{"number":6,"title":"the one below"},\
151 "stacked":[{"number":8,"title":"one above"},{"number":9,"title":"another"}],\
152 "source":"feat","target_ref":"main","head_sha":"abc123",\
153 "created_at":"2026-09-01T00:00:00Z"}
154 """)
155 #expect(mr.stackedOn?.number == 6)
156 #expect(mr.stackedOn?.title == "the one below")
157 #expect(mr.stacked.map(\.number) == [8, 9])
158 }
159
160 @Test func aListRowCarriesDraftAndStackedOn() throws {
161 let row = try decodeRow("""
162 {"number":7,"title":"a change","state":"open","draft":true,"author":"cmc",\
163 "stacked_on":{"number":6,"title":"below"},\
164 "source":"feat","target_ref":"main","head_sha":"abc123",\
165 "created_at":"2026-09-01T00:00:00Z"}
166 """)
167 #expect(row.draft)
168 #expect(row.stackedOn?.number == 6)
169 }
170
171 @Test func aPlainListRowHasNeither() throws {
172 let row = try decodeRow("""
173 {"number":7,"title":"a change","state":"open","author":"cmc",\
174 "source":"feat","target_ref":"main","head_sha":"abc123",\
175 "created_at":"2026-09-01T00:00:00Z"}
176 """)
177 #expect(row.draft == false)
178 #expect(row.stackedOn == nil)
179 }
180}
181```
182
183- [ ] **Step 2: Run to verify failure.**
184- [ ] **Step 3: Implement.** Add `StackRef` to `MRModels.swift`, add the four fields to `MRDetail` and the two to `MergeRequest`, and extend both `CodingKeys`. Use whichever of the two approaches above is cleaner, but the non-optional accessors (`draft: Bool`, `reviewRequests: [String]`, `stacked: [StackRef]`) are what Tasks 2 and 3 consume — do not push optionality onto callers.
185- [ ] **Step 4: Run the tests.** Expect PASS and the full suite green.
186- [ ] **Step 5: Commit** — `git commit -m "Decode draft, review requests and the merge request stack"`
187
188---
189
190### Task 2: The four write actions
191
192**Files:** Modify `gitbay/MRs/MRDetailViewModel.swift` and `gitbay/MRs/MRCreateViewModel.swift`; test in `gitbayTests/MRLifecycleTests.swift`
193
194**Interfaces produced:**
195- `MRDetailViewModel.setDraft(_ draft: Bool) async` — `mr draft` when true, `mr ready` when false
196- `MRDetailViewModel.retarget(to branch: String) async`
197- `MRDetailViewModel.requestReview(from user: String) async` / `.removeReviewRequest(_ user: String) async`
198- `MRDetailViewModel.branches: [String]` and `func loadBranches() async` — from `repo refs`, for the retarget picker
199- `MRCreateViewModel` gains a `draft: Bool` input that appends `--draft`
200
201Follow the existing `perform(argv:stdin:)` in `MRDetailViewModel` — it already sets `working`, clears `actionError`, runs, and reloads. Do not restructure it.
202
203**`mr review request` is a three-word path.** The argv is `["mr", "review", "request", repoPath, number, "--add", user]`. The existing approve path sends `["mr", "review", verdict, ...]`, so a two-word mistake here posts a review rather than requesting one — silently, with a 200. The tests below assert the exact argv for this reason.
204
205`repo refs` returns `{branches: [{name, sha}], tags: [...]}`. `RepoRefs` and `RepoRef` already exist and `RefsViewModel.load` already reads this command — reuse both models rather than writing a second pair.
206
207- [ ] **Step 1: Write the failing tests** — append to `gitbayTests/MRLifecycleTests.swift`:
208
209```swift
210@MainActor
211struct MRLifecycleActionTests {
212
213 private func loaded() async throws -> (MRDetailViewModel, StubProtocol.Box) {
214 let box = StubProtocol.box()
215 let client = GitbayClient(
216 instance: try GitbayInstance(url: "https://gitbay.org"),
217 token: "test-token",
218 session: box.session()
219 )
220 box.enqueue(.init(status: 200, json: """
221 {"protocol_version":1,"data":\(plainDetail),"exit_code":0}
222 """))
223 let model = MRDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7)
224 await model.load()
225 return (model, box)
226 }
227
228 private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] {
229 let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any])
230 return try #require(body["argv"] as? [String])
231 }
232
233 private func ok(_ box: StubProtocol.Box) {
234 box.enqueue(.init(status: 200, json: """
235 {"protocol_version":1,"exit_code":0}
236 """))
237 box.enqueue(.init(status: 200, json: """
238 {"protocol_version":1,"data":\(plainDetail),"exit_code":0}
239 """))
240 }
241
242 @Test func markingDraftAndReadyAreDifferentCommands() async throws {
243 let (model, box) = try await loaded()
244 ok(box)
245 await model.setDraft(true)
246 var write = try #require(box.seen.first { $0.method == "POST" })
247 #expect(try argvOf(write) == ["mr", "draft", "krz/gitbay", "7"])
248
249 let (model2, box2) = try await loaded()
250 ok(box2)
251 await model2.setDraft(false)
252 write = try #require(box2.seen.first { $0.method == "POST" })
253 #expect(try argvOf(write) == ["mr", "ready", "krz/gitbay", "7"])
254 }
255
256 @Test func retargetPassesTheBranchPositionally() async throws {
257 let (model, box) = try await loaded()
258 ok(box)
259 await model.retarget(to: "release")
260 let write = try #require(box.seen.first { $0.method == "POST" })
261 #expect(try argvOf(write) == ["mr", "retarget", "krz/gitbay", "7", "release"])
262 }
263
264 /// `mr review request` is a THREE-word path. Two words posts a
265 /// review instead — silently, with a 200.
266 @Test func requestingAReviewUsesTheThreeWordPath() async throws {
267 let (model, box) = try await loaded()
268 ok(box)
269 await model.requestReview(from: "rae")
270 let write = try #require(box.seen.first { $0.method == "POST" })
271 #expect(try argvOf(write)
272 == ["mr", "review", "request", "krz/gitbay", "7", "--add", "rae"])
273 }
274
275 @Test func removingAReviewRequestUsesRemoveNotAdd() async throws {
276 let (model, box) = try await loaded()
277 ok(box)
278 await model.removeReviewRequest("rae")
279 let write = try #require(box.seen.first { $0.method == "POST" })
280 #expect(try argvOf(write)
281 == ["mr", "review", "request", "krz/gitbay", "7", "--remove", "rae"])
282 }
283
284 @Test func branchesComeFromRepoRefs() async throws {
285 let (model, box) = try await loaded()
286 box.enqueue(.init(status: 200, json: """
287 {"protocol_version":1,"data":{"branches":[{"name":"main","sha":"a"},\
288 {"name":"release","sha":"b"}],"tags":[]},"exit_code":0}
289 """))
290 await model.loadBranches()
291 #expect(model.branches == ["main", "release"])
292 }
293
294 @Test func aRefusalSurfacesAndDoesNotReload() async throws {
295 let (model, box) = try await loaded()
296 box.enqueue(.init(status: 200, json: """
297 {"protocol_version":1,"error":"cannot retarget a merged merge request",\
298 "exit_code":1}
299 """))
300 await model.retarget(to: "release")
301 #expect(model.actionError?.isEmpty == false)
302 #expect(model.working == false)
303 }
304}
305
306@MainActor
307struct MRCreateDraftTests {
308
309 @Test func creatingAsADraftAppendsTheFlag() async throws {
310 let box = StubProtocol.box()
311 let client = GitbayClient(
312 instance: try GitbayInstance(url: "https://gitbay.org"),
313 token: "test-token",
314 session: box.session()
315 )
316 box.enqueue(.init(status: 200, json: """
317 {"protocol_version":1,"data":{"number":7},"exit_code":0}
318 """))
319 let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay")
320 model.draft = true
321 _ = await model.create(source: "feat", target: "main", title: "a change", body: "")
322
323 let write = try #require(box.seen.first { $0.method == "POST" })
324 let body = try #require(
325 try JSONSerialization.jsonObject(with: write.body) as? [String: Any]
326 )
327 let argv = try #require(body["argv"] as? [String])
328 #expect(argv.contains("--draft"))
329 }
330
331 @Test func creatingWithoutDraftOmitsTheFlag() async throws {
332 let box = StubProtocol.box()
333 let client = GitbayClient(
334 instance: try GitbayInstance(url: "https://gitbay.org"),
335 token: "test-token",
336 session: box.session()
337 )
338 box.enqueue(.init(status: 200, json: """
339 {"protocol_version":1,"data":{"number":7},"exit_code":0}
340 """))
341 let model = MRCreateViewModel(client: client, repoPath: "krz/gitbay")
342 _ = await model.create(source: "feat", target: "main", title: "a change", body: "")
343
344 let write = try #require(box.seen.first { $0.method == "POST" })
345 let body = try #require(
346 try JSONSerialization.jsonObject(with: write.body) as? [String: Any]
347 )
348 #expect((body["argv"] as? [String])?.contains("--draft") == false)
349 }
350}
351```
352
353`MRCreateViewModel.create(source:target:title:body:)` is the existing signature and the tests above match it. Add `draft` as a settable property rather than a fifth parameter, so the call sites do not all change.
354
355- [ ] **Step 2: Run to verify failure.**
356- [ ] **Step 3: Implement the five methods and the `draft` input.**
357- [ ] **Step 4: Run the tests.**
358- [ ] **Step 5: Commit** — `git commit -m "Draft, retarget and review requests on a merge request"`
359
360---
361
362### Task 3: The UI
363
364**Files:** Modify `gitbay/Views/MRs/MRView.swift` and `gitbay/Views/MRs/MRListView.swift`
365
366No unit tests — UI. Verification is the build plus the suite staying green.
367
368**On the list row:** a `GBChip("draft", .secondary)` when `mr.draft`, and a quiet "stacked on !N" line when `mr.stackedOn != nil`. Match the row's existing density; do not add a line to every row.
369
370**On the detail screen:**
371- Draft badge in the header beside the state.
372- A **Stack** section when `stackedOn != nil || !stacked.isEmpty`, listing both directions — what this is stacked on, and what is stacked on it — each row a `NavigationLink(value: MRRoute.mr(repo:number:))`. Say which direction each is; "stacked on !6" and "!8 is stacked on this" read differently and the user needs to know which.
373- A **Reviewers** section listing `reviewRequests`, each removable, with a field to add one. Reuse the add/remove shape `IssueView` already uses for labels and assignees rather than inventing another.
374- Actions, placed with the existing merge/close controls: **Mark as draft** / **Mark as ready** (whichever the current state implies), and **Retarget**, which presents a branch picker fed by `loadBranches()`.
375
376**Two things the UI must say, because the server enforces them and a silent refusal is worse than a warning:**
3771. **Retargeting stales the reviews** — an approval was of the diff against the old branch. Say so in the confirmation.
3782. **A squash or rebase merge is refused while anything is stacked on this merge request**, since it would rewrite the commits the stack builds on. The merge sheet offers Squash, Fast-forward and Rebase at `MRView.swift:63-65`; when `!stacked.isEmpty`, offer only Fast-forward.
379
380A draft merge request does not merge. When `mr.draft`, the merge control should say so rather than presenting a button the server will refuse.
381
382- [ ] **Step 1: List row — draft chip and stacked hint**
383- [ ] **Step 2: Detail — draft badge, Stack section, Reviewers section**
384- [ ] **Step 3: Detail — draft/ready and retarget actions, with the two warnings above**
385- [ ] **Step 4: Build and run the full suite.** Expect no drop from 258 + Tasks 1-2's additions.
386- [ ] **Step 5: Commit** — `git commit -m "Draft, stack and reviewers on the merge request screen"`
387
388---
389
390### Task 4: Flip four parity rows and open the merge request
391
392- [ ] **Step 1:** In the Merge requests table, set `draft, ready`, `retarget`, `request a review` and `stacked merge requests` to `yes` for iOS. Touch no other row — in particular leave `choose body markup`, which is MR 11.
393
394Land it via a worktree off `origin/main` in `krz/gitbay`, merged `--strategy ff`. That repo requires signed commits, so squash and rebase merges are refused, and its working tree usually holds unrelated work — never switch its branch.
395
396- [ ] **Step 2:** Run the full suite; record the real number.
397- [ ] **Step 3:** Open the merge request.
398
399---
400
401## Notes for whoever executes this
402
403**`mr review request` is three words.** `["mr", "review", "request", ...]`. The approve path is `["mr", "review", verdict, ...]`. A two-word slip posts a review and returns 200, so nothing surfaces — which is exactly why two tests assert the full argv.
404
405**Absence is the common case for all four new fields.** They are `omitempty`. Most merge requests carry none of them, so the decoding must default rather than require. Every test above that pins an absent field is pinning the ordinary case, not an edge case.
406
407**Draft is a flag, not a state.** `state` stays `"open"`. Do not add a `.draft` case to any state enum, and do not let `isOpen` change meaning — every `state = 'open'` rule on the server still means what it did.
408
409**Do not offer a merge strategy the server will refuse.** Squash and rebase are refused while anything is stacked on the merge request.