Commit 98579f8580
Verified · cmc
Layout: unified · split
docs/superpowers/plans/2026-09-06-mr08-small-actions.md added +268
| @@ -0,0 +1,268 @@ | ||
| 1 | # MR 8: Build cancel, release delete, org create and rename 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 — `build cancel`, `release delete`, and `org create, rename`. | |
| 6 | ||
| 7 | **Architecture:** Three unrelated one-command actions, batched into one merge request because each is a single method plus a menu item on a screen that already has a write path. They share no code; they share only a shape. | |
| 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`. Neither shadows a framework type. | |
| 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 | ||
| 21 | **The commands, verbatim from the registry:** | |
| 22 | ||
| 23 | ``` | |
| 24 | build cancel <owner/name> <n> | |
| 25 | release delete <owner/name> <tag> --yes | |
| 26 | org create <name> | |
| 27 | org rename <old> <new> | |
| 28 | ``` | |
| 29 | ||
| 30 | **`release delete` requires `--yes`.** It is not optional; omitting it is a usage error. That flag is the server asking for a typed confirmation, so the app confirms before sending it. | |
| 31 | ||
| 32 | **`build cancel` is refused unless the build is `pending` or `running`** (`internal/control/build.go:750`). The refusal reads *"build N is <status>; only a queued or running build can be cancelled"*. Note the wire status is **`pending`**, even though the prose says "queued" — match on `pending`, not `queued`. | |
| 33 | ||
| 34 | **Counting tests — use `xcresulttool`, not grep.** Grepping the build log undercounts by one, because `xcodebuild` splits a result line under parallel output: | |
| 35 | ||
| 36 | ```bash | |
| 37 | RES=$(ls -td ~/Library/Developer/Xcode/DerivedData/gitbay-*/Logs/Test/*.xcresult | head -1) | |
| 38 | xcrun xcresulttool get test-results summary --path "$RES" | |
| 39 | ``` | |
| 40 | ||
| 41 | Baseline: **311 total, 310 passed, 1 skipped, 0 failed.** | |
| 42 | ||
| 43 | --- | |
| 44 | ||
| 45 | ## File Structure | |
| 46 | ||
| 47 | | File | Responsibility | | |
| 48 | |------|----------------| | |
| 49 | | `gitbay/Builds/BuildDetailViewModel.swift` (modify) | `cancel()` | | |
| 50 | | `gitbay/Releases/ReleaseViewModels.swift` (modify) | `delete()` | | |
| 51 | | `gitbay/Orgs/OrgViewModels.swift` (modify) | `createOrg(_:)`, `rename(to:)` | | |
| 52 | | `gitbayTests/SmallActionTests.swift` (create) | Every test in this plan | | |
| 53 | | `gitbay/Views/Builds/BuildDetailView.swift` (modify) | Cancel action | | |
| 54 | | `gitbay/Views/Releases/ReleaseView.swift` (modify) | Delete action | | |
| 55 | | `gitbay/Views/Orgs/OrgView.swift` (modify) | Rename action | | |
| 56 | | `gitbay/Discovery/ProfileViewModel.swift` (modify) | `createOrg(_:)` | | |
| 57 | | `gitbay/Views/Discovery/ProfileView.swift` (modify) | Create action | | |
| 58 | ||
| 59 | --- | |
| 60 | ||
| 61 | ### Task 1: The four methods | |
| 62 | ||
| 63 | **Files:** Modify the three view models above; test in `gitbayTests/SmallActionTests.swift` | |
| 64 | ||
| 65 | **Interfaces produced:** | |
| 66 | - `BuildDetailViewModel.cancel() async` and `var isCancellable: Bool` | |
| 67 | - `ReleaseDetailViewModel.delete() async -> Bool` (true on success, so the view can pop) | |
| 68 | - `OrgViewModel.rename(to newName: String) async -> Bool` | |
| 69 | - `ProfileViewModel.createOrg(_ name: String) async -> Bool` | |
| 70 | ||
| 71 | Each routes through the existing `perform(argv:)` in its file. **Copy the pattern, do not extract a shared helper** — that is a standing decision in this codebase, and these are three unrelated files. | |
| 72 | ||
| 73 | Behaviour, each pinned by a test: | |
| 74 | 1. `cancel()` sends `["build", "cancel", repoPath, String(number)]`. | |
| 75 | 2. `isCancellable` is true only for status `pending` or `running`. **The wire value is `pending`** — matching on `"queued"` would leave the action permanently hidden, silently. | |
| 76 | 3. `delete()` sends `["release", "delete", repoPath, tag, "--yes"]`. The `--yes` is required by the command; a delete without it is a usage error. | |
| 77 | 4. `rename(to:)` sends `["org", "rename", oldName, newName]` — both positional, old first. Swapping them renames the wrong way with no error. | |
| 78 | 5. `createOrg(_:)` sends `["org", "create", name]`. | |
| 79 | 6. Every one trims its input and refuses to send a blank name or tag. | |
| 80 | 7. A refusal surfaces into `actionError`, returns false, and does not pretend success. | |
| 81 | ||
| 82 | - [ ] **Step 1: Write the failing tests** | |
| 83 | ||
| 84 | ```swift | |
| 85 | import Foundation | |
| 86 | import Testing | |
| 87 | @testable import gitbay | |
| 88 | ||
| 89 | private func makeClient() throws -> (GitbayClient, StubProtocol.Box) { | |
| 90 | let box = StubProtocol.box() | |
| 91 | let client = GitbayClient( | |
| 92 | instance: try GitbayInstance(url: "https://gitbay.org"), | |
| 93 | token: "test-token", | |
| 94 | session: box.session() | |
| 95 | ) | |
| 96 | return (client, box) | |
| 97 | } | |
| 98 | ||
| 99 | private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] { | |
| 100 | let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any]) | |
| 101 | return try #require(body["argv"] as? [String]) | |
| 102 | } | |
| 103 | ||
| 104 | private let ok = """ | |
| 105 | {"protocol_version":1,"exit_code":0} | |
| 106 | """ | |
| 107 | ||
| 108 | @MainActor | |
| 109 | struct BuildCancelTests { | |
| 110 | ||
| 111 | /// The wire status is "pending", not "queued" — the refusal message | |
| 112 | /// says "queued" but the value never does. | |
| 113 | @Test func onlyPendingAndRunningBuildsAreCancellable() { | |
| 114 | #expect(BuildDetailViewModel.isCancellable(status: "pending")) | |
| 115 | #expect(BuildDetailViewModel.isCancellable(status: "running")) | |
| 116 | for status in ["success", "failure", "cancelled", "queued", ""] { | |
| 117 | #expect(BuildDetailViewModel.isCancellable(status: status) == false, | |
| 118 | "\(status) should not be cancellable") | |
| 119 | } | |
| 120 | } | |
| 121 | ||
| 122 | @Test func cancelSendsTheBuildNumber() async throws { | |
| 123 | let (client, stub) = try makeClient() | |
| 124 | // The view model's own load response, then the write, then a reload. | |
| 125 | let model = BuildDetailViewModel(client: client, repoPath: "krz/gitbay", number: 966) | |
| 126 | stub.enqueue(.init(status: 200, json: ok)) | |
| 127 | stub.enqueue(.init(status: 200, json: ok)) | |
| 128 | await model.cancel() | |
| 129 | ||
| 130 | let write = try #require(stub.seen.first { $0.method == "POST" }) | |
| 131 | #expect(try argvOf(write) == ["build", "cancel", "krz/gitbay", "966"]) | |
| 132 | } | |
| 133 | } | |
| 134 | ||
| 135 | @MainActor | |
| 136 | struct ReleaseDeleteTests { | |
| 137 | ||
| 138 | /// `--yes` is required by the command, not optional. | |
| 139 | @Test func deleteSendsTheConfirmationFlag() async throws { | |
| 140 | let (client, stub) = try makeClient() | |
| 141 | let model = ReleaseDetailViewModel(client: client, repoPath: "krz/gitbay", tag: "v1.0.0") | |
| 142 | stub.enqueue(.init(status: 200, json: ok)) | |
| 143 | let deleted = await model.delete() | |
| 144 | ||
| 145 | #expect(deleted) | |
| 146 | let write = try #require(stub.seen.first { $0.method == "POST" }) | |
| 147 | #expect(try argvOf(write) == ["release", "delete", "krz/gitbay", "v1.0.0", "--yes"]) | |
| 148 | } | |
| 149 | ||
| 150 | @Test func aRefusedDeleteReturnsFalseAndSurfaces() async throws { | |
| 151 | let (client, stub) = try makeClient() | |
| 152 | let model = ReleaseDetailViewModel(client: client, repoPath: "krz/gitbay", tag: "v1.0.0") | |
| 153 | stub.enqueue(.init(status: 200, json: """ | |
| 154 | {"protocol_version":1,"error":"deleting a release needs write access","exit_code":4} | |
| 155 | """)) | |
| 156 | let deleted = await model.delete() | |
| 157 | ||
| 158 | #expect(deleted == false) | |
| 159 | #expect(model.actionError?.isEmpty == false) | |
| 160 | } | |
| 161 | } | |
| 162 | ||
| 163 | @MainActor | |
| 164 | struct OrgCreateRenameTests { | |
| 165 | ||
| 166 | /// Both names are positional, OLD first. Swapping them renames the | |
| 167 | /// wrong way with no error at all. | |
| 168 | @Test func renameSendsOldThenNew() async throws { | |
| 169 | let (client, stub) = try makeClient() | |
| 170 | let model = OrgViewModel(client: client, orgName: "krz") | |
| 171 | stub.enqueue(.init(status: 200, json: ok)) | |
| 172 | stub.enqueue(.init(status: 200, json: ok)) | |
| 173 | _ = await model.rename(to: "kerouac") | |
| 174 | ||
| 175 | let write = try #require(stub.seen.first { $0.method == "POST" }) | |
| 176 | #expect(try argvOf(write) == ["org", "rename", "krz", "kerouac"]) | |
| 177 | } | |
| 178 | ||
| 179 | @Test func aBlankRenameSendsNothing() async throws { | |
| 180 | let (client, stub) = try makeClient() | |
| 181 | let model = OrgViewModel(client: client, orgName: "krz") | |
| 182 | let renamed = await model.rename(to: " ") | |
| 183 | ||
| 184 | #expect(renamed == false) | |
| 185 | #expect(stub.seen.filter { $0.method == "POST" }.isEmpty) | |
| 186 | } | |
| 187 | ||
| 188 | @Test func createSendsTheName() async throws { | |
| 189 | let (client, stub) = try makeClient() | |
| 190 | stub.enqueue(.init(status: 200, json: ok)) | |
| 191 | stub.enqueue(.init(status: 200, json: ok)) | |
| 192 | let model = ProfileViewModel(client: client, name: "cmc") | |
| 193 | _ = await model.createOrg("newco") | |
| 194 | ||
| 195 | let write = try #require(stub.seen.first { $0.method == "POST" }) | |
| 196 | #expect(try argvOf(write) == ["org", "create", "newco"]) | |
| 197 | } | |
| 198 | ||
| 199 | @Test func aBlankCreateSendsNothing() async throws { | |
| 200 | let (client, stub) = try makeClient() | |
| 201 | let model = ProfileViewModel(client: client, name: "cmc") | |
| 202 | let created = await model.createOrg(" ") | |
| 203 | ||
| 204 | #expect(created == false) | |
| 205 | #expect(stub.seen.filter { $0.method == "POST" }.isEmpty) | |
| 206 | } | |
| 207 | } | |
| 208 | ``` | |
| 209 | ||
| 210 | **The real initialisers, which I checked** — match them exactly and do not change any to suit a test: | |
| 211 | - `BuildDetailViewModel(client:repoPath:number:)` | |
| 212 | - `ReleaseDetailViewModel(client:repoPath:tag:)` | |
| 213 | - `OrgViewModel(client:orgName:)` — note `orgName:`, not `org:` | |
| 214 | - `ProfileViewModel(client:name:)` | |
| 215 | ||
| 216 | **There is no organisation list view model.** Organisations are listed on the profile screen, from `ProfileViewModel.Profile.orgs` (`ProfileView.swift:28,99`), so `createOrg` belongs on `ProfileViewModel` and the button belongs on that screen. Check `ProfileViewModel`'s actual initialiser before writing the test. | |
| 217 | ||
| 218 | `isCancellable` is written above as a static taking a status so it is testable without a loaded model; an instance property derived from the loaded build is equally fine as long as the pending/running/everything-else distinction is directly testable. | |
| 219 | ||
| 220 | - [ ] **Step 2: Run to verify failure.** | |
| 221 | - [ ] **Step 3: Implement the four methods.** | |
| 222 | - [ ] **Step 4: Run the tests.** | |
| 223 | - [ ] **Step 5: Commit** — `git commit -m "Cancel a build, delete a release, create and rename an organisation"` | |
| 224 | ||
| 225 | --- | |
| 226 | ||
| 227 | ### Task 2: The four actions in their screens | |
| 228 | ||
| 229 | **Files:** the four view files above | |
| 230 | ||
| 231 | No unit tests — UI. | |
| 232 | ||
| 233 | - **Build cancel** — on the build detail screen, offered **only when `isCancellable`**. Confirm first: cancelling a running build kills the step within a couple of seconds. Say what it does rather than just "Cancel". | |
| 234 | - **Release delete** — on the release detail screen, destructive, confirmed, and the confirmation must say the assets go too. Pop back to the release list on success. | |
| 235 | - **Org rename** — on the organisation screen. The confirmation **must say clone URLs change**; the command's own usage says so, and a rename that silently breaks everyone's remotes is the worst outcome here. | |
| 236 | - **Org create** — on the profile screen, which is where organisations are listed. A name prompt and the action, then reload so the new organisation appears. | |
| 237 | ||
| 238 | Reuse each screen's existing notice mechanism; do not add a second one. Match how each screen already presents its destructive actions — the repository screen's archive and the label screen's remove are the closest precedents. | |
| 239 | ||
| 240 | - [ ] **Step 1: Build cancel, gated on `isCancellable`** | |
| 241 | - [ ] **Step 2: Release delete with its confirmation and pop** | |
| 242 | - [ ] **Step 3: Org rename with the clone-URL warning** | |
| 243 | - [ ] **Step 4: Org create** | |
| 244 | - [ ] **Step 5: Build and run the full suite.** No drop from 310 passed. | |
| 245 | - [ ] **Step 6: Commit** — `git commit -m "Cancel, delete and organisation actions in their screens"` | |
| 246 | ||
| 247 | --- | |
| 248 | ||
| 249 | ### Task 3: Flip three parity rows and open the merge request | |
| 250 | ||
| 251 | - [ ] **Step 1:** Set `build cancel`, `release delete` and `create, rename` (Organizations) to `yes` for iOS. Touch no other row — in particular leave `delete, transfer` and `API token mint`, which are SSH-only by design, and `dependency checks`/`dependency status`, which are the next merge request. | |
| 252 | ||
| 253 | 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 JSON rather than assuming it.** | |
| 254 | ||
| 255 | - [ ] **Step 2:** Run the full suite via `xcresulttool`; record the real numbers. | |
| 256 | - [ ] **Step 3:** Open the merge request. | |
| 257 | ||
| 258 | --- | |
| 259 | ||
| 260 | ## Notes for whoever executes this | |
| 261 | ||
| 262 | **The build status is `pending`, not `queued`.** The server's own refusal message says "queued or running", but the value it compares is `pending`. Matching on `"queued"` would hide the cancel action forever, silently. | |
| 263 | ||
| 264 | **`--yes` is not optional on `release delete`.** The command requires it. | |
| 265 | ||
| 266 | **`org rename` is old-then-new, both positional.** Reversed, it renames the wrong way and the server cannot tell. | |
| 267 | ||
| 268 | **Three unrelated files.** They share a shape, not code. Copy each file's own `perform`; do not unify them. | |