Commit a4c70fe5c1
Verified · cmc
Layout: unified · split
docs/superpowers/plans/2026-09-06-mr07-repo-actions.md added +300
| @@ -0,0 +1,300 @@ | |||
| 1 | # MR 7: Repository actions — fork, watch/mute, bookmark 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 three parity rows — `fork`, `watch, mute`, and `bookmark, bookmark list`. | ||
| 6 | |||
| 7 | **Architecture:** Three repository-level actions on one screen, all thin writes over existing commands. The complication is that the server reports back almost none of the resulting state, so each of the three gets a different treatment depending on what is actually readable. | ||
| 8 | |||
| 9 | **Spec:** `docs/superpowers/specs/2026-09-06-ios-parity-design.md` | ||
| 10 | |||
| 11 | ## What the server does and does not report | ||
| 12 | |||
| 13 | This shaped every decision below, and it is worth stating plainly because it is not what the spec assumed. | ||
| 14 | |||
| 15 | | State | Readable? | Consequence | | ||
| 16 | |-------|-----------|-------------| | ||
| 17 | | Watch / mute | **No.** `store.RepoWatchState` exists but is internal; no command exposes it | Two stateless actions, not a toggle | | ||
| 18 | | Bookmarked | **Indirectly** — `repo bookmarks` lists them, membership is derivable | A real toggle, at the cost of one list read | | ||
| 19 | | `fork_of` | **No** on `repo show`; only on `repo fork`'s own response (`internal/control/mr.go:94`) | Cannot show "forked from X" for an existing repository | | ||
| 20 | |||
| 21 | Filed upstream as **krz/gitbay#178**. Do not work around any of it by inventing local persistence — the app renders server state; it does not remember its own. | ||
| 22 | |||
| 23 | ## Global Constraints | ||
| 24 | |||
| 25 | - Swift 6 language mode, default `MainActor` isolation. Wire models are `nonisolated struct`s. | ||
| 26 | - Swift Testing only — never XCTest. | ||
| 27 | - `gitbayTests` is hermetic and offline; network goes through `StubProtocol`. | ||
| 28 | - New files under `gitbay/` and `gitbayTests/` need **no** `project.pbxproj` edit. | ||
| 29 | - The label model is `IssueLabel`; the notification model is `InboxNotification`. Neither shadows a framework type, and neither should be renamed back. | ||
| 30 | - Never mention Claude, LLMs or AI in commits, comments, or the merge request. No `Co-Authored-By` trailer. | ||
| 31 | - Never commit to `main`. | ||
| 32 | |||
| 33 | **The commands, verbatim from the registry:** | ||
| 34 | |||
| 35 | ``` | ||
| 36 | repo fork <owner/name> [--name <n>] | ||
| 37 | repo watch <owner/name> | ||
| 38 | repo unwatch <owner/name> | ||
| 39 | repo bookmark <owner/name> | ||
| 40 | repo unbookmark <owner/name> | ||
| 41 | repo bookmarks | ||
| 42 | ``` | ||
| 43 | |||
| 44 | Note `repo bookmarks` — **plural, and it takes no argument**. It is not `repo bookmark list`. | ||
| 45 | |||
| 46 | `repo fork` returns `{"path": "<new owner/name>", "fork_of": "<source>"}`. | ||
| 47 | |||
| 48 | **Counting tests:** | ||
| 49 | |||
| 50 | ```bash | ||
| 51 | grep -cE "^Test case '[^']*' passed" /tmp/out.txt | ||
| 52 | grep -cE "^Test case '[^']*' failed" /tmp/out.txt | ||
| 53 | grep -cE "^Test case '[^']*' skipped" /tmp/out.txt | ||
| 54 | ``` | ||
| 55 | |||
| 56 | Baseline: **297 passed, 1 skipped, 0 failed.** | ||
| 57 | |||
| 58 | --- | ||
| 59 | |||
| 60 | ## File Structure | ||
| 61 | |||
| 62 | | File | Responsibility | | ||
| 63 | |------|----------------| | ||
| 64 | | `gitbay/Repos/RepoActionsViewModel.swift` (create) | `ForkResult` model, bookmark state, and the five actions | | ||
| 65 | | `gitbayTests/RepoActionTests.swift` (create) | Every test in this plan | | ||
| 66 | | `gitbay/Views/Repos/RepoView.swift` (modify) | The actions, in the repository's menu | | ||
| 67 | |||
| 68 | --- | ||
| 69 | |||
| 70 | ### Task 1: The actions and the bookmark state | ||
| 71 | |||
| 72 | **Files:** Create `gitbay/Repos/RepoActionsViewModel.swift`; test in `gitbayTests/RepoActionTests.swift` | ||
| 73 | |||
| 74 | **Interfaces produced:** | ||
| 75 | - `nonisolated struct ForkResult: Decodable, Sendable, Hashable` — `path: String`, `forkOf: String` | ||
| 76 | - `@Observable @MainActor final class RepoActionsViewModel` — `isBookmarked: Bool?`, `actionError: String?`, `notice: String?`, `working: Bool`, `func loadBookmarkState() async`, `func setBookmarked(_:) async`, `func watch() async`, `func unwatch() async`, `func fork(named:) async -> ForkResult?` | ||
| 77 | |||
| 78 | Behaviour, each pinned by a test: | ||
| 79 | 1. `repo bookmarks` takes **no repository argument** — argv is exactly `["repo", "bookmarks"]`. Passing the path would be a usage error. | ||
| 80 | 2. `isBookmarked` is `Bool?`: `nil` until known. It is derived by testing membership of the `repo bookmarks` result, and a failed load leaves it `nil` rather than `false` — "we don't know" and "not bookmarked" are different, and the UI must not claim the latter when it means the former. This is the same three-state lesson MRs 4 and 5 each had to learn. | ||
| 81 | 3. `setBookmarked(true)` sends `repo bookmark`; `setBookmarked(false)` sends `repo unbookmark`. Two commands, not a flag. | ||
| 82 | 4. `watch()` and `unwatch()` send their commands and report the result. **They do not attempt to track state** — nothing reports it back, and inventing a local guess would show the user something the server never said. | ||
| 83 | 5. `fork(named:)` sends `["repo", "fork", repoPath]`, appending `--name <n>` only when a name is given, and returns the decoded `ForkResult` so the caller can navigate to the new repository. | ||
| 84 | 6. A refusal surfaces into `actionError` and does not clear a prior success `notice` misleadingly. | ||
| 85 | |||
| 86 | - [ ] **Step 1: Write the failing tests** | ||
| 87 | |||
| 88 | ```swift | ||
| 89 | import Foundation | ||
| 90 | import Testing | ||
| 91 | @testable import gitbay | ||
| 92 | |||
| 93 | private func makeClient() throws -> (GitbayClient, StubProtocol.Box) { | ||
| 94 | let box = StubProtocol.box() | ||
| 95 | let client = GitbayClient( | ||
| 96 | instance: try GitbayInstance(url: "https://gitbay.org"), | ||
| 97 | token: "test-token", | ||
| 98 | session: box.session() | ||
| 99 | ) | ||
| 100 | return (client, box) | ||
| 101 | } | ||
| 102 | |||
| 103 | private func argvFrom(_ url: URL) -> [String] { | ||
| 104 | URLComponents(url: url, resolvingAgainstBaseURL: false)? | ||
| 105 | .queryItems?.filter { $0.name == "argv" }.compactMap(\.value) ?? [] | ||
| 106 | } | ||
| 107 | |||
| 108 | private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] { | ||
| 109 | let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any]) | ||
| 110 | return try #require(body["argv"] as? [String]) | ||
| 111 | } | ||
| 112 | |||
| 113 | private let bookmarks = """ | ||
| 114 | {"protocol_version":1,"data":[{"path":"krz/gitbay","visibility":"public"},\ | ||
| 115 | {"path":"krz/solar","visibility":"public"}],"exit_code":0} | ||
| 116 | """ | ||
| 117 | private let ok = """ | ||
| 118 | {"protocol_version":1,"exit_code":0} | ||
| 119 | """ | ||
| 120 | |||
| 121 | @MainActor | ||
| 122 | struct RepoActionsTests { | ||
| 123 | |||
| 124 | /// `repo bookmarks` is plural and takes NO repository argument. | ||
| 125 | @Test func theBookmarkListingTakesNoArgument() async throws { | ||
| 126 | let (client, stub) = try makeClient() | ||
| 127 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 128 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 129 | await model.loadBookmarkState() | ||
| 130 | |||
| 131 | #expect(argvFrom(try #require(stub.seen.last).url) == ["repo", "bookmarks"]) | ||
| 132 | #expect(model.isBookmarked == true) | ||
| 133 | } | ||
| 134 | |||
| 135 | @Test func aRepoAbsentFromTheListingIsNotBookmarked() async throws { | ||
| 136 | let (client, stub) = try makeClient() | ||
| 137 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 138 | let model = RepoActionsViewModel(client: client, repoPath: "krz/other") | ||
| 139 | await model.loadBookmarkState() | ||
| 140 | |||
| 141 | #expect(model.isBookmarked == false) | ||
| 142 | } | ||
| 143 | |||
| 144 | /// "We don't know" is not "not bookmarked". A failed load must leave | ||
| 145 | /// the state unknown rather than claiming the repository is not | ||
| 146 | /// bookmarked, which would draw the wrong control. | ||
| 147 | @Test func aFailedListingLeavesTheStateUnknown() async throws { | ||
| 148 | let (client, stub) = try makeClient() | ||
| 149 | stub.enqueue(.init(status: 200, json: """ | ||
| 150 | {"protocol_version":1,"error":"denied","exit_code":4} | ||
| 151 | """)) | ||
| 152 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 153 | await model.loadBookmarkState() | ||
| 154 | |||
| 155 | #expect(model.isBookmarked == nil) | ||
| 156 | } | ||
| 157 | |||
| 158 | @Test func bookmarkingAndUnbookmarkingAreDifferentCommands() async throws { | ||
| 159 | let (client, stub) = try makeClient() | ||
| 160 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 161 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 162 | await model.loadBookmarkState() | ||
| 163 | |||
| 164 | stub.enqueue(.init(status: 200, json: ok)) | ||
| 165 | stub.enqueue(.init(status: 200, json: bookmarks)) | ||
| 166 | await model.setBookmarked(false) | ||
| 167 | var write = try #require(stub.seen.first { $0.method == "POST" }) | ||
| 168 | #expect(try argvOf(write) == ["repo", "unbookmark", "krz/gitbay"]) | ||
| 169 | |||
| 170 | let (client2, stub2) = try makeClient() | ||
| 171 | stub2.enqueue(.init(status: 200, json: bookmarks)) | ||
| 172 | let model2 = RepoActionsViewModel(client: client2, repoPath: "krz/other") | ||
| 173 | await model2.loadBookmarkState() | ||
| 174 | stub2.enqueue(.init(status: 200, json: ok)) | ||
| 175 | stub2.enqueue(.init(status: 200, json: bookmarks)) | ||
| 176 | await model2.setBookmarked(true) | ||
| 177 | write = try #require(stub2.seen.first { $0.method == "POST" }) | ||
| 178 | #expect(try argvOf(write) == ["repo", "bookmark", "krz/other"]) | ||
| 179 | } | ||
| 180 | |||
| 181 | @Test func watchAndUnwatchSendTheirOwnCommands() async throws { | ||
| 182 | let (client, stub) = try makeClient() | ||
| 183 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 184 | |||
| 185 | stub.enqueue(.init(status: 200, json: ok)) | ||
| 186 | await model.watch() | ||
| 187 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "watch", "krz/gitbay"]) | ||
| 188 | |||
| 189 | stub.enqueue(.init(status: 200, json: ok)) | ||
| 190 | await model.unwatch() | ||
| 191 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "unwatch", "krz/gitbay"]) | ||
| 192 | } | ||
| 193 | |||
| 194 | @Test func forkingReturnsTheNewPath() async throws { | ||
| 195 | let (client, stub) = try makeClient() | ||
| 196 | stub.enqueue(.init(status: 200, json: """ | ||
| 197 | {"protocol_version":1,"data":{"path":"cmc/gitbay","fork_of":"krz/gitbay"},\ | ||
| 198 | "exit_code":0} | ||
| 199 | """)) | ||
| 200 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 201 | let result = await model.fork(named: nil) | ||
| 202 | |||
| 203 | #expect(result?.path == "cmc/gitbay") | ||
| 204 | #expect(result?.forkOf == "krz/gitbay") | ||
| 205 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "fork", "krz/gitbay"]) | ||
| 206 | } | ||
| 207 | |||
| 208 | @Test func forkingWithANameAppendsTheFlag() async throws { | ||
| 209 | let (client, stub) = try makeClient() | ||
| 210 | stub.enqueue(.init(status: 200, json: """ | ||
| 211 | {"protocol_version":1,"data":{"path":"cmc/mine","fork_of":"krz/gitbay"},\ | ||
| 212 | "exit_code":0} | ||
| 213 | """)) | ||
| 214 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 215 | _ = await model.fork(named: "mine") | ||
| 216 | |||
| 217 | #expect(try argvOf(try #require(stub.seen.last)) | ||
| 218 | == ["repo", "fork", "krz/gitbay", "--name", "mine"]) | ||
| 219 | } | ||
| 220 | |||
| 221 | /// A blank name must not send an empty flag value. | ||
| 222 | @Test func forkingWithABlankNameOmitsTheFlag() async throws { | ||
| 223 | let (client, stub) = try makeClient() | ||
| 224 | stub.enqueue(.init(status: 200, json: """ | ||
| 225 | {"protocol_version":1,"data":{"path":"cmc/gitbay","fork_of":"krz/gitbay"},\ | ||
| 226 | "exit_code":0} | ||
| 227 | """)) | ||
| 228 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 229 | _ = await model.fork(named: " ") | ||
| 230 | |||
| 231 | #expect(try argvOf(try #require(stub.seen.last)) == ["repo", "fork", "krz/gitbay"]) | ||
| 232 | } | ||
| 233 | |||
| 234 | @Test func aRefusedForkReturnsNilAndSurfaces() async throws { | ||
| 235 | let (client, stub) = try makeClient() | ||
| 236 | stub.enqueue(.init(status: 200, json: """ | ||
| 237 | {"protocol_version":1,"error":"a repository named gitbay already exists",\ | ||
| 238 | "exit_code":1} | ||
| 239 | """)) | ||
| 240 | let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 241 | let result = await model.fork(named: nil) | ||
| 242 | |||
| 243 | #expect(result == nil) | ||
| 244 | #expect(model.actionError?.isEmpty == false) | ||
| 245 | #expect(model.working == false) | ||
| 246 | } | ||
| 247 | } | ||
| 248 | ``` | ||
| 249 | |||
| 250 | - [ ] **Step 2: Run to verify failure.** | ||
| 251 | - [ ] **Step 3: Implement.** Copy `perform(argv:)` from `RepoSettingsViewModel` for the void writes; `fork` needs `client.run(_:as:)` since it decodes a result. Reuse `RepoSummary` for the bookmarks listing — I checked the live response, `{path, description, visibility, bookmarks}`, and `RepoSummary` decodes it directly (the extra `bookmarks` count is ignored, as Swift's `Decodable` ignores unknown keys). Do not write a new model. | ||
| 252 | - [ ] **Step 4: Run the tests.** | ||
| 253 | - [ ] **Step 5: Commit** — `git commit -m "Fork, watch, mute and bookmark a repository"` | ||
| 254 | |||
| 255 | --- | ||
| 256 | |||
| 257 | ### Task 2: The actions on the repository screen | ||
| 258 | |||
| 259 | **Files:** Modify `gitbay/Views/Repos/RepoView.swift` | ||
| 260 | |||
| 261 | No unit tests — UI. | ||
| 262 | |||
| 263 | Put all five in the repository screen's toolbar menu, beside the existing pin and archive actions. Study how those are presented and match them. | ||
| 264 | |||
| 265 | **Each of the three rows gets the treatment its readable state allows:** | ||
| 266 | |||
| 267 | - **Bookmark** — a real toggle, because the state is derivable. While `isBookmarked` is `nil` the control must not claim either state: show it disabled or as a spinner, never as "not bookmarked". Load the state with `.task`. | ||
| 268 | - **Watch / Mute** — **two separate actions, not a toggle.** Nothing reports which one is current, so a toggle would be lying. Say what each does rather than implying a state: "Watch this repository" and "Mute this repository". A brief caption noting that the current setting is not shown is honest and cheap; krz/gitbay#178 tracks fixing it properly. | ||
| 269 | - **Fork** — an action that prompts for an optional name, then navigates to the new repository on success using `RepoRoute.repo(result.path)`. On refusal, surface the server's message: "already exists" is the common one and the user needs to read it. | ||
| 270 | |||
| 271 | `RepoView` already surfaces `actionError`-style messages for pin and archive — reuse that, do not add a second notice mechanism. | ||
| 272 | |||
| 273 | - [ ] **Step 1: Bookmark toggle with its unknown state** | ||
| 274 | - [ ] **Step 2: Watch and Mute as two actions** | ||
| 275 | - [ ] **Step 3: Fork with an optional name and navigation on success** | ||
| 276 | - [ ] **Step 4: Build and run the full suite.** No drop from 297. | ||
| 277 | - [ ] **Step 5: Commit** — `git commit -m "Repository actions in the repo menu"` | ||
| 278 | |||
| 279 | --- | ||
| 280 | |||
| 281 | ### Task 3: Flip three parity rows and open the merge request | ||
| 282 | |||
| 283 | - [ ] **Step 1:** In the Repositories table set `fork`, `watch, mute` and `bookmark, bookmark list` to `yes` for iOS. Touch no other row. | ||
| 284 | |||
| 285 | Land 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 output rather than assuming it.** | ||
| 286 | |||
| 287 | - [ ] **Step 2:** Run the full suite; record the real number. | ||
| 288 | - [ ] **Step 3:** Open the merge request, noting that watch and mute ship stateless and why. | ||
| 289 | |||
| 290 | --- | ||
| 291 | |||
| 292 | ## Notes for whoever executes this | ||
| 293 | |||
| 294 | **`repo bookmarks` is plural and takes no argument.** Not `repo bookmark list`, and not `repo bookmarks <owner/name>`. | ||
| 295 | |||
| 296 | **Do not invent state the server does not report.** Watch and mute have no readable state; a toggle would show the user something the server never said. Two actions is the honest rendering, and krz/gitbay#178 tracks the fix. | ||
| 297 | |||
| 298 | **`nil` is not `false` for the bookmark state.** A failed listing means unknown, and drawing "not bookmarked" would be a claim the app cannot support. | ||
| 299 | |||
| 300 | **`fork_of` is not on `repo show`.** Do not try to display "forked from X" for an existing repository; it is only on `repo fork`'s own response, which is why the fork action returns it. | ||