| @@ -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. |