Commit 3efee558e0
Verified · cmc
Layout: unified · split
docs/superpowers/plans/2026-09-06-mr09-deps.md added +311
| @@ -0,0 +1,311 @@ | |||
| 1 | # MR 9: Dependency checks 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 two parity rows — `dependency checks on/off` and `dependency status`. | ||
| 6 | |||
| 7 | **Architecture:** A toggle and a report, both inside the existing repository settings screen, which is where the web puts them. One read, two writes, no new screen. | ||
| 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 | |||
| 21 | **The commands, verbatim from the registry:** | ||
| 22 | |||
| 23 | ``` | ||
| 24 | repo deps enable <owner/name> | ||
| 25 | repo deps disable <owner/name> | ||
| 26 | repo deps status <owner/name> | ||
| 27 | ``` | ||
| 28 | |||
| 29 | All three are **three-word paths** — `["repo", "deps", "status", repoPath]`. The app already sends two-word `repo` commands everywhere, so a two-word slip here is the same class of mistake as MR 4's `mr review request`. | ||
| 30 | |||
| 31 | **The status shape**, from `internal/control/deps.go:32-44` and confirmed against the live instance: | ||
| 32 | |||
| 33 | ```json | ||
| 34 | {"enabled":true,"last_check":"2026-09-06T09:47:07.905Z","issue_number":140, | ||
| 35 | "behind":[{"ecosystem":"go","name":"github.com/yuin/goldmark", | ||
| 36 | "current":"v1.8.5","latest":"v1.8.6"}]} | ||
| 37 | ``` | ||
| 38 | |||
| 39 | `enabled` and `behind` are always present. **`last_check`, `last_error` and `issue_number` are `omitempty`** — a repository that has never been checked carries none of them, and that is the ordinary case for a repository whose checks were just switched on. | ||
| 40 | |||
| 41 | **Counting tests — use `xcresulttool`, not grep:** | ||
| 42 | |||
| 43 | ```bash | ||
| 44 | RES=$(ls -td ~/Library/Developer/Xcode/DerivedData/gitbay-*/Logs/Test/*.xcresult | head -1) | ||
| 45 | xcrun xcresulttool get test-results summary --path "$RES" | ||
| 46 | ``` | ||
| 47 | |||
| 48 | Baseline: **321 total, 320 passed, 1 skipped, 0 failed.** | ||
| 49 | |||
| 50 | --- | ||
| 51 | |||
| 52 | ## File Structure | ||
| 53 | |||
| 54 | | File | Responsibility | | ||
| 55 | |------|----------------| | ||
| 56 | | `gitbay/Repos/DepsStatus.swift` (create) | `DepsStatus` and `DepBehind` wire models | | ||
| 57 | | `gitbay/Repos/RepoSettingsViewModel.swift` (modify) | `deps`, `depsError`, `loadDeps()`, `setDepsEnabled(_:)` | | ||
| 58 | | `gitbayTests/DepsTests.swift` (create) | Every test in this plan | | ||
| 59 | | `gitbay/Views/Repos/RepoSettingsView.swift` (modify) | The toggle and the report | | ||
| 60 | |||
| 61 | --- | ||
| 62 | |||
| 63 | ### Task 1: The model and the two actions | ||
| 64 | |||
| 65 | **Files:** Create `gitbay/Repos/DepsStatus.swift`; modify `gitbay/Repos/RepoSettingsViewModel.swift`; test in `gitbayTests/DepsTests.swift` | ||
| 66 | |||
| 67 | **Interfaces produced:** | ||
| 68 | - `nonisolated struct DepsStatus: Decodable, Sendable, Hashable` — `enabled: Bool`, `lastCheck: Date?`, `lastError: String?`, `issueNumber: Int64?`, `behind: [DepBehind]` | ||
| 69 | - `nonisolated struct DepBehind: Decodable, Sendable, Hashable, Identifiable` — `ecosystem: String`, `name: String`, `current: String`, `latest: String`; `id` unique per row | ||
| 70 | - `RepoSettingsViewModel.deps: DepsStatus?`, `.depsError: String?`, `func loadDeps() async`, `func setDepsEnabled(_ on: Bool) async` | ||
| 71 | |||
| 72 | Behaviour, each pinned by a test: | ||
| 73 | 1. All three commands are **three-word paths**: `["repo","deps","status",repo]`, `["repo","deps","enable",repo]`, `["repo","deps","disable",repo]`. | ||
| 74 | 2. `setDepsEnabled(true)` sends `enable`, `setDepsEnabled(false)` sends `disable` — two commands, not a flag. | ||
| 75 | 3. `last_check`, `last_error` and `issue_number` are absent for a never-checked repository. Decoding must not require them; `behind` may be present and empty. | ||
| 76 | 4. **`deps` is `DepsStatus?` and `depsError` is separate.** `nil` deps with no error means "not loaded yet"; `nil` deps with an error means "the read failed". They must be distinguishable — this codebase has hit the collapsed-three-state bug repeatedly, and the settings screen must not render "checks are off" when it means "we could not ask". | ||
| 77 | 5. A refused write surfaces into the existing `actionError` and does not flip the toggle's backing state. | ||
| 78 | 6. `DepBehind.id` is unique across ecosystems — two ecosystems can carry the same package name. | ||
| 79 | |||
| 80 | - [ ] **Step 1: Write the failing tests** | ||
| 81 | |||
| 82 | ```swift | ||
| 83 | import Foundation | ||
| 84 | import Testing | ||
| 85 | @testable import gitbay | ||
| 86 | |||
| 87 | private func makeClient() throws -> (GitbayClient, StubProtocol.Box) { | ||
| 88 | let box = StubProtocol.box() | ||
| 89 | let client = GitbayClient( | ||
| 90 | instance: try GitbayInstance(url: "https://gitbay.org"), | ||
| 91 | token: "test-token", | ||
| 92 | session: box.session() | ||
| 93 | ) | ||
| 94 | return (client, box) | ||
| 95 | } | ||
| 96 | |||
| 97 | private func argvFrom(_ url: URL) -> [String] { | ||
| 98 | URLComponents(url: url, resolvingAgainstBaseURL: false)? | ||
| 99 | .queryItems?.filter { $0.name == "argv" }.compactMap(\.value) ?? [] | ||
| 100 | } | ||
| 101 | |||
| 102 | private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] { | ||
| 103 | let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any]) | ||
| 104 | return try #require(body["argv"] as? [String]) | ||
| 105 | } | ||
| 106 | |||
| 107 | private let fullStatus = """ | ||
| 108 | {"protocol_version":1,"data":{"enabled":true,\ | ||
| 109 | "last_check":"2026-09-06T09:47:07.905Z","issue_number":140,\ | ||
| 110 | "behind":[{"ecosystem":"go","name":"github.com/yuin/goldmark",\ | ||
| 111 | "current":"v1.8.5","latest":"v1.8.6"}]},"exit_code":0} | ||
| 112 | """ | ||
| 113 | |||
| 114 | struct DepsDecodingTests { | ||
| 115 | |||
| 116 | private func decode(_ json: String) throws -> DepsStatus { | ||
| 117 | let decoder = JSONDecoder() | ||
| 118 | decoder.dateDecodingStrategy = .iso8601 | ||
| 119 | return try decoder.decode(DepsStatus.self, from: Data(json.utf8)) | ||
| 120 | } | ||
| 121 | |||
| 122 | /// A repository whose checks were just switched on has never been | ||
| 123 | /// checked, so it carries none of the three omitempty fields. That is | ||
| 124 | /// the ordinary case, not an edge case. | ||
| 125 | @Test func aNeverCheckedRepositoryHasNoTimestampErrorOrIssue() throws { | ||
| 126 | let status = try decode(""" | ||
| 127 | {"enabled":true,"behind":[]} | ||
| 128 | """) | ||
| 129 | #expect(status.enabled) | ||
| 130 | #expect(status.lastCheck == nil) | ||
| 131 | #expect(status.lastError == nil) | ||
| 132 | #expect(status.issueNumber == nil) | ||
| 133 | #expect(status.behind.isEmpty) | ||
| 134 | } | ||
| 135 | |||
| 136 | @Test func aCheckedRepositoryDecodesEverything() throws { | ||
| 137 | let status = try decode(""" | ||
| 138 | {"enabled":true,"last_check":"2026-09-06T09:47:07Z","issue_number":140,\ | ||
| 139 | "behind":[{"ecosystem":"go","name":"modernc.org/sqlite",\ | ||
| 140 | "current":"v1.57.0","latest":"v1.58.0"}]} | ||
| 141 | """) | ||
| 142 | #expect(status.lastCheck != nil) | ||
| 143 | #expect(status.issueNumber == 140) | ||
| 144 | #expect(status.behind.first?.name == "modernc.org/sqlite") | ||
| 145 | #expect(status.behind.first?.latest == "v1.58.0") | ||
| 146 | } | ||
| 147 | |||
| 148 | @Test func aFailedCheckCarriesItsError() throws { | ||
| 149 | let status = try decode(""" | ||
| 150 | {"enabled":true,"last_error":"could not read go.mod","behind":[]} | ||
| 151 | """) | ||
| 152 | #expect(status.lastError == "could not read go.mod") | ||
| 153 | } | ||
| 154 | |||
| 155 | /// Two ecosystems can carry the same package name, so the id must | ||
| 156 | /// include the ecosystem or SwiftUI reuses one row for the other. | ||
| 157 | @Test func behindRowsAreUniqueAcrossEcosystems() throws { | ||
| 158 | let status = try decode(""" | ||
| 159 | {"enabled":true,"behind":[\ | ||
| 160 | {"ecosystem":"go","name":"x","current":"1","latest":"2"},\ | ||
| 161 | {"ecosystem":"npm","name":"x","current":"1","latest":"2"}]} | ||
| 162 | """) | ||
| 163 | let ids = status.behind.map(\.id) | ||
| 164 | #expect(Set(ids).count == 2) | ||
| 165 | } | ||
| 166 | } | ||
| 167 | |||
| 168 | @MainActor | ||
| 169 | struct DepsActionTests { | ||
| 170 | |||
| 171 | /// All three are THREE-word paths. The app sends two-word `repo` | ||
| 172 | /// commands everywhere else, so this is easy to get wrong. | ||
| 173 | @Test func statusUsesTheThreeWordPath() async throws { | ||
| 174 | let (client, stub) = try makeClient() | ||
| 175 | stub.enqueue(.init(status: 200, json: fullStatus)) | ||
| 176 | let model = RepoSettingsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 177 | await model.loadDeps() | ||
| 178 | |||
| 179 | #expect(argvFrom(try #require(stub.seen.last).url) | ||
| 180 | == ["repo", "deps", "status", "krz/gitbay"]) | ||
| 181 | #expect(model.deps?.enabled == true) | ||
| 182 | #expect(model.deps?.behind.count == 1) | ||
| 183 | } | ||
| 184 | |||
| 185 | @Test func enableAndDisableAreDifferentCommands() async throws { | ||
| 186 | let (client, stub) = try makeClient() | ||
| 187 | let model = RepoSettingsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 188 | |||
| 189 | stub.enqueue(.init(status: 200, json: """ | ||
| 190 | {"protocol_version":1,"exit_code":0} | ||
| 191 | """)) | ||
| 192 | stub.enqueue(.init(status: 200, json: fullStatus)) | ||
| 193 | await model.setDepsEnabled(true) | ||
| 194 | var write = try #require(stub.seen.first { $0.method == "POST" }) | ||
| 195 | #expect(try argvOf(write) == ["repo", "deps", "enable", "krz/gitbay"]) | ||
| 196 | |||
| 197 | let (client2, stub2) = try makeClient() | ||
| 198 | let model2 = RepoSettingsViewModel(client: client2, repoPath: "krz/gitbay") | ||
| 199 | stub2.enqueue(.init(status: 200, json: """ | ||
| 200 | {"protocol_version":1,"exit_code":0} | ||
| 201 | """)) | ||
| 202 | stub2.enqueue(.init(status: 200, json: fullStatus)) | ||
| 203 | await model2.setDepsEnabled(false) | ||
| 204 | write = try #require(stub2.seen.first { $0.method == "POST" }) | ||
| 205 | #expect(try argvOf(write) == ["repo", "deps", "disable", "krz/gitbay"]) | ||
| 206 | } | ||
| 207 | |||
| 208 | /// "Could not ask" must not render as "checks are off". | ||
| 209 | @Test func aFailedStatusReadIsDistinguishableFromChecksBeingOff() async throws { | ||
| 210 | let (client, stub) = try makeClient() | ||
| 211 | stub.enqueue(.init(status: 200, json: """ | ||
| 212 | {"protocol_version":1,"error":"denied","exit_code":4} | ||
| 213 | """)) | ||
| 214 | let model = RepoSettingsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 215 | await model.loadDeps() | ||
| 216 | |||
| 217 | #expect(model.deps == nil) | ||
| 218 | #expect(model.depsError?.isEmpty == false) | ||
| 219 | } | ||
| 220 | |||
| 221 | @Test func aRefusedWriteSurfacesAndLeavesTheStatusAlone() async throws { | ||
| 222 | let (client, stub) = try makeClient() | ||
| 223 | stub.enqueue(.init(status: 200, json: fullStatus)) | ||
| 224 | let model = RepoSettingsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 225 | await model.loadDeps() | ||
| 226 | #expect(model.deps?.enabled == true) | ||
| 227 | |||
| 228 | stub.enqueue(.init(status: 200, json: """ | ||
| 229 | {"protocol_version":1,"error":"turning dependency checks on needs admin","exit_code":4} | ||
| 230 | """)) | ||
| 231 | await model.setDepsEnabled(false) | ||
| 232 | |||
| 233 | #expect(model.actionError?.isEmpty == false) | ||
| 234 | // The write failed, so the last known status still says enabled. | ||
| 235 | #expect(model.deps?.enabled == true) | ||
| 236 | } | ||
| 237 | |||
| 238 | /// A successful load after a failed one must clear the error. | ||
| 239 | @Test func depsErrorClearsOnALaterSuccess() async throws { | ||
| 240 | let (client, stub) = try makeClient() | ||
| 241 | stub.enqueue(.init(status: 200, json: """ | ||
| 242 | {"protocol_version":1,"error":"denied","exit_code":4} | ||
| 243 | """)) | ||
| 244 | let model = RepoSettingsViewModel(client: client, repoPath: "krz/gitbay") | ||
| 245 | await model.loadDeps() | ||
| 246 | #expect(model.depsError != nil) | ||
| 247 | |||
| 248 | stub.enqueue(.init(status: 200, json: fullStatus)) | ||
| 249 | await model.loadDeps() | ||
| 250 | |||
| 251 | #expect(model.depsError == nil) | ||
| 252 | #expect(model.deps?.enabled == true) | ||
| 253 | } | ||
| 254 | } | ||
| 255 | ``` | ||
| 256 | |||
| 257 | **Check `RepoSettingsViewModel`'s real initialiser before writing these** and match it; do not change it to suit a test. | ||
| 258 | |||
| 259 | - [ ] **Step 2: Run to verify failure.** | ||
| 260 | - [ ] **Step 3: Implement.** `setDepsEnabled` routes through the file's existing `perform(argv:)`, then reloads the status so the report reflects the change. `loadDeps` sets exactly one of `deps` or `depsError` per attempt and clears the other. | ||
| 261 | - [ ] **Step 4: Run the tests.** | ||
| 262 | - [ ] **Step 5: Commit** — `git commit -m "Dependency check status and toggle"` | ||
| 263 | |||
| 264 | --- | ||
| 265 | |||
| 266 | ### Task 2: The toggle and the report in settings | ||
| 267 | |||
| 268 | **Files:** Modify `gitbay/Views/Repos/RepoSettingsView.swift` | ||
| 269 | |||
| 270 | No unit tests — UI. | ||
| 271 | |||
| 272 | Add a `Dependencies` section after the existing ones (`aboutSection`, `topicsSection`, `visibilitySection`, `branchesSection`, `mergeRulesSection`, `daemonSection`) and match how they are built — each is a `private func …Section(_ loaded:) -> some View` returning a `Section`. | ||
| 273 | |||
| 274 | The section shows, in this order: | ||
| 275 | |||
| 276 | 1. **A toggle** bound to `deps?.enabled`, disabled while the status is unknown. Checks are **off until a repository's admin turns them on**, and the check tells a public registry what the repository depends on — say that under the toggle, because it is the reason someone would decline. | ||
| 277 | 2. **When enabled**, the report: | ||
| 278 | - `last_check` as a relative time, or "never checked" when absent — the ordinary state right after switching on. | ||
| 279 | - `last_error` when present, as a `GBNotice` warning. | ||
| 280 | - A link to the tracking issue when `issue_number` is present, routing to `IssueRoute.issue(repo:number:)`. The worker opens, rewrites and closes that issue, and it is the same copy every other surface reads. | ||
| 281 | - The `behind` rows: ecosystem, name, current → latest. An empty `behind` with a `last_check` means everything is current — say so rather than showing an empty list. | ||
| 282 | 3. **Three distinct states for the read itself**: not loaded (spinner), failed (`depsError` as a notice, so "could not ask" never reads as "checks are off"), and loaded. | ||
| 283 | |||
| 284 | Load with `.task`, and reuse the screen's existing notice mechanism for `actionError`. | ||
| 285 | |||
| 286 | - [ ] **Step 1: The section with its toggle and caption** | ||
| 287 | - [ ] **Step 2: The report, including the never-checked and everything-current cases** | ||
| 288 | - [ ] **Step 3: The three read states** | ||
| 289 | - [ ] **Step 4: Build and run the full suite.** No drop from 320 passed. | ||
| 290 | - [ ] **Step 5: Commit** — `git commit -m "Dependency section in repository settings"` | ||
| 291 | |||
| 292 | --- | ||
| 293 | |||
| 294 | ### Task 3: Flip two parity rows and open the merge request | ||
| 295 | |||
| 296 | - [ ] **Step 1:** Set `dependency checks on/off` and `dependency status` to `yes` for iOS. Touch no other row. | ||
| 297 | |||
| 298 | 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.** | ||
| 299 | |||
| 300 | - [ ] **Step 2:** Run the full suite via `xcresulttool`; record the real numbers. | ||
| 301 | - [ ] **Step 3:** Open the merge request. | ||
| 302 | |||
| 303 | --- | ||
| 304 | |||
| 305 | ## Notes for whoever executes this | ||
| 306 | |||
| 307 | **All three commands are three-word paths.** `["repo","deps","status",…]`. Every other `repo` command in this app is two words, which is exactly why this is easy to get wrong — and a wrong path returns a usage error the screen would render as a failed read. | ||
| 308 | |||
| 309 | **Never-checked is the ordinary case.** `last_check`, `last_error` and `issue_number` are all `omitempty`. A repository whose checks were switched on a moment ago has none of them. | ||
| 310 | |||
| 311 | **"Could not ask" is not "checks are off".** Keep `deps` and `depsError` separate and render three states. This codebase has collapsed that distinction repeatedly. | ||