Commit 3c99511a5e

3c99511a5e7f525314c329d0c9e576917a9a26e1

parent: cc8a97bf86

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-07 02:25 UTC

Implementation plan for MR 11: choose body markup

Layout: unified · split

docs/superpowers/plans/2026-09-06-mr11-body-markup.md added +291
@@ -0,0 +1,291 @@
1# MR 11: Choose body markup 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 the last two buildable parity rows — `choose body markup` for merge requests and for issues.
6
7**Architecture:** `--format md|org` on six commands, a picker in two composing surfaces, and one new decoded field. The whole difficulty is that the stored format is what every surface renders from, so an edit must default to the format the text was written in.
8
9**Spec:** `docs/superpowers/specs/2026-09-06-ios-parity-design.md`
10
11## Global Constraints
12
13- Swift 6 language mode, default `MainActor` isolation. Wire models are `nonisolated struct`s.
14- Swift Testing only — never XCTest.
15- `gitbayTests` is hermetic and offline; network goes through `StubProtocol`.
16- New files under `gitbay/` and `gitbayTests/` need **no** `project.pbxproj` edit.
17- The label model is `IssueLabel`; the notification model is `InboxNotification`.
18- Never mention Claude, LLMs or AI in commits, comments, or the merge request. No `Co-Authored-By` trailer.
19- Never commit to `main`.
20- **Waiting on an async request in a test: use the `until(_:_:)` helper** in `gitbayTests/StubProtocol.swift`, not a fixed sleep, and assert on the request identified by its argv rather than `stub.seen.last`. Do not index `stub.seen` directly — go through `#require`. The merge request immediately before this one fixed fifteen sites of exactly that.
21
22**The six commands in scope:**
23
24```
25issue create <repo> --title <t> [--body <b> | --file -] [--format md|org]
26issue edit <repo> <n> [--title <t>] [--body <b> | --file -] [--format md|org]
27issue comment <repo> <n> [--message <m> | --file -] [--format md|org]
28mr create <target repo> --source … --target … --title <t> [--body | --file -] [--format md|org] [--draft]
29mr edit <repo> <n> [--title <t>] [--body <b> | --file -] [--format md|org]
30mr comment <repo> <n> [--message <m> | --file -] [--format md|org]
31```
32
33**Two things are deliberately out of scope:**
34
35- **`release create` and `release edit` also take `--format`**, but the wiki has **no parity row for release markup**. `ReleaseView` shares `ComposeSheet` with the issue and merge request screens, so the format control must be **optional** and the release call site must keep passing nothing. Do not widen this merge request into releases.
36- **`mr diff-comment` has no format column and is always markdown.** It gets no picker.
37
38**Counting tests — `xcresulttool`, not grep:**
39
40```bash
41RES=$(ls -td ~/Library/Developer/Xcode/DerivedData/gitbay-*/Logs/Test/*.xcresult | head -1)
42xcrun xcresulttool get test-results summary --path "$RES"
43```
44
45Baseline: **347 total, 346 passed, 1 skipped, 0 failed.**
46
47---
48
49## File Structure
50
51| File | Responsibility |
52|------|----------------|
53| `gitbay/Issues/IssueModels.swift` (modify) | Decode `body_format` on `IssueDetail` |
54| `gitbay/MRs/MRModels.swift` (modify) | Decode `body_format` on `MRDetail` |
55| `gitbay/Issues/IssueDetailViewModel.swift` (modify) | `--format` on edit and comment |
56| `gitbay/MRs/MRDetailViewModel.swift` (modify) | `--format` on edit and comment |
57| `gitbay/Issues/IssueCreateViewModel.swift` (modify) | `--format` on create |
58| `gitbay/MRs/MRCreateViewModel.swift` (modify) | `--format` on create |
59| `gitbayTests/BodyFormatTests.swift` (create) | Every test in this plan |
60| `gitbay/Views/Shared/ComposeSheet.swift` (modify) | Optional format control |
61| `gitbay/Views/Issues/IssueListView.swift`, `IssueView.swift`, `gitbay/Views/MRs/MRListView.swift`, `MRView.swift` (modify) | Pass the format through; picker on the comment fields |
62
63---
64
65### Task 1: Decode the stored format, and send it
66
67**Files:** the two model files, the four view models; test in `gitbayTests/BodyFormatTests.swift`
68
69**Interfaces produced:**
70- `IssueDetail.bodyFormat: String?` and `MRDetail.bodyFormat: String?`
71- `IssueDetailViewModel.edit(title:body:format:)`, `.comment(_:format:)`
72- `MRDetailViewModel.edit(title:body:format:)`, `.comment(_:format:)`
73- `IssueCreateViewModel.format` and `MRCreateViewModel.format` (settable properties, like the `draft` property MR 4 added — **not** new parameters, which would change every call site)
74
75Behaviour, each pinned by a test:
761. `body_format` is **`omitempty`** — an ordinary body may carry no format key at all. Decoding must not require it. `MRDetail` has a hand-written `init(from:)` covering 24 properties, audited in MR 4; adding one means **re-auditing that nothing was dropped**.
772. `--format` is appended **only when a format is given**. Omitting it lets the server default; sending `--format ""` would be a usage error.
783. All six argv shapes are exact. `--format` goes with the body flags, and the body itself still travels in `stdin` via `--file -` as it does today.
794. **A format of `md` is still sent explicitly when the user chose it.** "The user picked markdown" and "the user expressed no preference" are the same on the wire, but the picker always has a value once shown, so an edit always sends one.
80
81- [ ] **Step 1: Write the failing tests**
82
83```swift
84import Foundation
85import Testing
86@testable import gitbay
87
88private func makeClient() throws -> (GitbayClient, StubProtocol.Box) {
89 let box = StubProtocol.box()
90 let client = GitbayClient(
91 instance: try GitbayInstance(url: "https://gitbay.org"),
92 token: "test-token",
93 session: box.session()
94 )
95 return (client, box)
96}
97
98private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] {
99 let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any])
100 return try #require(body["argv"] as? [String])
101}
102
103private func stdinOf(_ seen: StubProtocol.Seen) throws -> String? {
104 let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any])
105 return body["stdin"] as? String
106}
107
108struct BodyFormatDecodingTests {
109
110 private func decodeIssue(_ json: String) throws -> IssueDetail {
111 let decoder = JSONDecoder()
112 decoder.dateDecodingStrategy = .iso8601
113 return try decoder.decode(IssueDetail.self, from: Data(json.utf8))
114 }
115
116 private func decodeMR(_ json: String) throws -> MRDetail {
117 let decoder = JSONDecoder()
118 decoder.dateDecodingStrategy = .iso8601
119 return try decoder.decode(MRDetail.self, from: Data(json.utf8))
120 }
121
122 /// body_format is omitempty; an ordinary body carries no key.
123 @Test func anIssueWithoutAStoredFormatDecodes() throws {
124 let issue = try decodeIssue("""
125 {"number":7,"title":"t","state":"open","author":"cmc",\
126 "body":"x","created_at":"2026-09-01T00:00:00Z"}
127 """)
128 #expect(issue.bodyFormat == nil)
129 }
130
131 @Test func anIssueStoredAsOrgSaysSo() throws {
132 let issue = try decodeIssue("""
133 {"number":7,"title":"t","state":"open","author":"cmc",\
134 "body":"x","body_format":"org","created_at":"2026-09-01T00:00:00Z"}
135 """)
136 #expect(issue.bodyFormat == "org")
137 }
138
139 @Test func aMergeRequestWithoutAStoredFormatDecodes() throws {
140 let mr = try decodeMR("""
141 {"number":7,"title":"t","state":"open","author":"cmc","source":"f",\
142 "target_ref":"main","head_sha":"a","created_at":"2026-09-01T00:00:00Z"}
143 """)
144 #expect(mr.bodyFormat == nil)
145 }
146
147 /// MRDetail's hand-written initialiser must still decode everything
148 /// it did before the new field was added.
149 @Test func addingTheFormatDidNotDropAnyMergeRequestField() throws {
150 let mr = try decodeMR("""
151 {"number":7,"title":"t","state":"open","draft":true,"author":"cmc","source":"f",\
152 "target_ref":"main","head_sha":"abc","body":"b","body_format":"org",\
153 "milestone":"v1","review_requests":["rae"],\
154 "stacked_on":{"number":6,"title":"below"},\
155 "stacked":[{"number":8,"title":"above"}],\
156 "created_at":"2026-09-01T00:00:00Z","merged_at":"2026-09-02T00:00:00Z",\
157 "merged_by":"cmc","checks_combined":"success","unresolved_threads":2}
158 """)
159 #expect(mr.bodyFormat == "org")
160 #expect(mr.draft)
161 #expect(mr.milestone == "v1")
162 #expect(mr.reviewRequests == ["rae"])
163 #expect(mr.stackedOn?.number == 6)
164 #expect(mr.stacked.map(\.number) == [8])
165 #expect(mr.mergedBy == "cmc")
166 #expect(mr.checksCombined == "success")
167 #expect(mr.unresolvedThreads == 2)
168 }
169}
170
171@MainActor
172struct BodyFormatWriteTests {
173
174 private let issueJSON = """
175 {"protocol_version":1,"data":{"number":7,"title":"t","state":"open","author":"cmc",\
176 "body":"x","body_format":"org","created_at":"2026-09-01T00:00:00Z"},"exit_code":0}
177 """
178
179 @Test func anIssueCommentCarriesItsFormat() async throws {
180 let (client, stub) = try makeClient()
181 stub.enqueue(.init(status: 200, json: issueJSON))
182 let model = IssueDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7)
183 await model.load()
184
185 stub.enqueue(.init(status: 200, json: """
186 {"protocol_version":1,"exit_code":0}
187 """))
188 stub.enqueue(.init(status: 200, json: issueJSON))
189 await model.comment("hello", format: "org")
190
191 let write = try #require(stub.seen.first { $0.method == "POST" })
192 #expect(try argvOf(write)
193 == ["issue", "comment", "krz/gitbay", "7", "--file", "-", "--format", "org"])
194 #expect(try stdinOf(write) == "hello")
195 }
196
197 /// No format given means no flag — the server then defaults. Sending
198 /// `--format ""` would be a usage error.
199 @Test func noFormatMeansNoFlag() async throws {
200 let (client, stub) = try makeClient()
201 stub.enqueue(.init(status: 200, json: issueJSON))
202 let model = IssueDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7)
203 await model.load()
204
205 stub.enqueue(.init(status: 200, json: """
206 {"protocol_version":1,"exit_code":0}
207 """))
208 stub.enqueue(.init(status: 200, json: issueJSON))
209 await model.comment("hello", format: nil)
210
211 let write = try #require(stub.seen.first { $0.method == "POST" })
212 #expect(try argvOf(write).contains("--format") == false)
213 }
214
215 @Test func anIssueEditCarriesItsFormat() async throws {
216 let (client, stub) = try makeClient()
217 stub.enqueue(.init(status: 200, json: issueJSON))
218 let model = IssueDetailViewModel(client: client, repoPath: "krz/gitbay", number: 7)
219 await model.load()
220
221 stub.enqueue(.init(status: 200, json: """
222 {"protocol_version":1,"exit_code":0}
223 """))
224 stub.enqueue(.init(status: 200, json: issueJSON))
225 await model.edit(title: "t2", body: "b2", format: "md")
226
227 let argv = try argvOf(try #require(stub.seen.first { $0.method == "POST" }))
228 #expect(argv.contains("--format"))
229 #expect(argv.contains("md"))
230 }
231}
232```
233
234Add equivalent write tests for `MRDetailViewModel.comment`/`edit` and for both create view models — the create ones assert that setting `format` appends the flag and leaving it nil omits it, in the same shape MR 4 used for `--draft`.
235
236**Provision a stub for every request each flow makes.** `comment` and `edit` write *and* reload. Five tests in this repo have shipped under-provisioned; do not add a sixth.
237
238- [ ] **Step 2: Run to verify failure.**
239- [ ] **Step 3: Implement.** Re-audit `MRDetail.init(from:)` field by field.
240- [ ] **Step 4: Run the tests.**
241- [ ] **Step 5: Commit** — `git commit -m "Send the body markup format on issues and merge requests"`
242
243---
244
245### Task 2: The picker
246
247**Files:** `ComposeSheet.swift` and the four view files
248
249No unit tests — UI.
250
251**`ComposeSheet` gains an optional format binding.** It is used by four screens; the release screen must keep working unchanged, because release markup has no parity row. An `Binding<String>?` defaulting to nil, with the control rendered only when non-nil, keeps that call site untouched.
252
253**The comment fields compose separately**, in `IssueView` and `MRView` — they do not go through `ComposeSheet`. Each needs its own small picker beside its send button.
254
255**The default is the rule that matters:**
256- **Editing an existing body defaults to the format it was stored with**, from `bodyFormat`. Changing it reinterprets prose that already exists, so the picker must start where the text actually is. An org body opened under a markdown picker, saved unchanged, silently becomes markdown — and its tables and links flatten.
257- **A new body defaults to markdown**, as today.
258- **A new comment defaults to markdown.** Do not inherit the parent body's format: the parent's author and the commenter are different people.
259
260Keep the control small — a segmented `md`/`org` picker, or a menu. This sits under a text editor that is the point of the screen; it should not compete with it.
261
262- [ ] **Step 1: Optional format binding on `ComposeSheet`, release call site untouched**
263- [ ] **Step 2: Create and edit sheets pass the format, edits seeded from `bodyFormat`**
264- [ ] **Step 3: Pickers on the two comment fields, defaulting to markdown**
265- [ ] **Step 4: Build and run the full suite.** No drop from 346 passed.
266- [ ] **Step 5: Commit** — `git commit -m "Pick the body markup when writing"`
267
268---
269
270### Task 3: Flip the last two rows and open the merge request
271
272- [ ] **Step 1:** Set `choose body markup` to `yes` for iOS in **both** the Merge requests and the Issues tables.
273
274The prose after the Issues table currently reads that the `no` is *"the absence of a picker on the web form and in the iOS composer, both of which write markdown"*. **Update it** — the web has since gained one and iOS now has one too, so that sentence is wrong in both halves. Say that both surfaces offer a picker, and keep the note that diff-line comments have no format column and are always markdown.
275
276Land 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. **Read the merge request number back from `mr create`'s JSON.**
277
278- [ ] **Step 2:** Run the full suite via `xcresulttool`; record the real numbers.
279- [ ] **Step 3:** Open the merge request.
280
281---
282
283## Notes for whoever executes this
284
285**Releases are out of scope.** `release create`/`edit` take `--format` too, but there is no parity row for it and `ReleaseView` shares `ComposeSheet`. Make the binding optional and leave that call site alone.
286
287**`mr diff-comment` gets no picker.** It has no format column server-side and is always markdown.
288
289**An edit defaults to the stored format.** This is the whole point of the row. Defaulting to markdown would silently convert org prose the first time someone edits a title.
290
291**A new comment defaults to markdown**, not the parent's format — different author, different choice.