Commit b50b13e99e

b50b13e99ee9b3c55805255f285b2d78729602f4

parent: e61ef1ad41

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-06 23:57 UTC

Fix repo-actions final review findings

Trim --name before sending it to repo fork instead of only testing
blankness on the trimmed value. Render RepoDetailViewModel and
RepoActionsViewModel action errors as separate notices so a stale
error from one can't mask a fresh one from the other. Refresh
bookmark state on pull-to-refresh so a failed initial read isn't
stuck spinning forever. Surface an error when a successful fork
returns no result instead of failing silently.

Adds tests for the whitespace trim, the silent-success fork case,
and the bookmark write/re-read pairing.

Layout: unified · split

gitbay/Repos/RepoActionsViewModel.swift +10 −3
@@ -80,15 +80,22 @@ final class RepoActionsViewModel {
80 80
81 func fork(named name: String?) async -> ForkResult? { 81 func fork(named name: String?) async -> ForkResult? {
82 var argv = ["repo", "fork", repoPath] 82 var argv = ["repo", "fork", repoPath]
83 if let name, !name.trimmingCharacters(in: .whitespaces).isEmpty { 83 if let name {
84 argv += ["--name", name] 84 let trimmed = name.trimmingCharacters(in: .whitespacesAndNewlines)
85 if !trimmed.isEmpty {
86 argv += ["--name", trimmed]
87 }
85 } 88 }
86 working = true 89 working = true
87 actionError = nil 90 actionError = nil
88 notice = nil 91 notice = nil
89 defer { working = false } 92 defer { working = false }
90 do { 93 do {
91 return try await client.run(argv, as: ForkResult.self) 94 guard let result = try await client.run(argv, as: ForkResult.self) else {
95 actionError = "The fork succeeded, but the server reported nothing back."
96 return nil
97 }
98 return result
92 } catch let error as GitbayError { 99 } catch let error as GitbayError {
93 actionError = error.userFacingMessage 100 actionError = error.userFacingMessage
94 return nil 101 return nil
gitbay/Views/Repos/RepoView.swift +10 −2
@@ -24,7 +24,12 @@ struct RepoView: View {
24 var body: some View { 24 var body: some View {
25 List { 25 List {
26 if let detail = model.state.value { 26 if let detail = model.state.value {
27 if let error = model.actionError ?? actionsModel.actionError { 27 if let error = model.actionError {
28 Section {
29 GBNotice(error, .gbWarn)
30 }
31 }
32 if let error = actionsModel.actionError {
28 Section { 33 Section {
29 GBNotice(error, .gbWarn) 34 GBNotice(error, .gbWarn)
30 } 35 }
@@ -97,7 +102,10 @@ struct RepoView: View {
97 .toolbar { toolbar } 102 .toolbar { toolbar }
98 .task { await model.load() } 103 .task { await model.load() }
99 .task { await actionsModel.loadBookmarkState() } 104 .task { await actionsModel.loadBookmarkState() }
100 .refreshable { await model.load() } 105 .refreshable {
106 await model.load()
107 await actionsModel.loadBookmarkState()
108 }
101 .navigationDestination(item: $forkDestination) { route in 109 .navigationDestination(item: $forkDestination) { route in
102 if case .repo(let forkedPath) = route { 110 if case .repo(let forkedPath) = route {
103 RepoView(client: client, path: forkedPath) 111 RepoView(client: client, path: forkedPath)
gitbayTests/RepoActionTests.swift +50
@@ -135,6 +135,21 @@ struct RepoActionsTests {
135 == ["repo", "fork", "krz/gitbay", "--name", "mine"]) 135 == ["repo", "fork", "krz/gitbay", "--name", "mine"])
136 } 136 }
137 137
138 /// Surrounding whitespace must not reach the server — the name is
139 /// trimmed, not just tested for blankness.
140 @Test func forkingWithSurroundingWhitespaceTrimsTheName() async throws {
141 let (client, stub) = try makeClient()
142 stub.enqueue(.init(status: 200, json: """
143 {"protocol_version":1,"data":{"path":"cmc/mine","fork_of":"krz/gitbay"},\
144 "exit_code":0}
145 """))
146 let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay")
147 _ = await model.fork(named: " mine ")
148
149 #expect(try argvOf(try #require(stub.seen.last))
150 == ["repo", "fork", "krz/gitbay", "--name", "mine"])
151 }
152
138 /// A blank name must not send an empty flag value. 153 /// A blank name must not send an empty flag value.
139 @Test func forkingWithABlankNameOmitsTheFlag() async throws { 154 @Test func forkingWithABlankNameOmitsTheFlag() async throws {
140 let (client, stub) = try makeClient() 155 let (client, stub) = try makeClient()
@@ -182,4 +197,39 @@ struct RepoActionsTests {
182 #expect(model.notice == nil) 197 #expect(model.notice == nil)
183 #expect(model.actionError?.isEmpty == false) 198 #expect(model.actionError?.isEmpty == false)
184 } 199 }
200
201 /// A 0-exit fork whose envelope carries no `data` must not fail
202 /// silently — the caller needs to know nothing came back.
203 @Test func aForkThatReportsNothingBackSetsAnError() async throws {
204 let (client, stub) = try makeClient()
205 stub.enqueue(.init(status: 200, json: """
206 {"protocol_version":1,"exit_code":0}
207 """))
208 let model = RepoActionsViewModel(client: client, repoPath: "krz/gitbay")
209 let result = await model.fork(named: nil)
210
211 #expect(result == nil)
212 #expect(model.actionError?.isEmpty == false)
213 }
214
215 /// A bookmark write must be followed by a re-read of `repo
216 /// bookmarks` — that re-read is what keeps `isBookmarked` honest
217 /// instead of an optimistic local guess.
218 @Test func bookmarkingRereadsTheListingAfterTheWrite() async throws {
219 let (client, stub) = try makeClient()
220 stub.enqueue(.init(status: 200, json: bookmarks))
221 let model = RepoActionsViewModel(client: client, repoPath: "krz/other")
222 await model.loadBookmarkState()
223 #expect(model.isBookmarked == false)
224
225 stub.enqueue(.init(status: 200, json: ok))
226 stub.enqueue(.init(status: 200, json: bookmarks))
227 await model.setBookmarked(true)
228
229 let calls = stub.seen.suffix(2)
230 let write = try #require(calls.first)
231 let reread = try #require(calls.last)
232 #expect(try argvOf(write) == ["repo", "bookmark", "krz/other"])
233 #expect(argvFrom(reread.url) == ["repo", "bookmarks"])
234 }
185} 235}