feat(swift-ios): link thread rows to pull requests - #5804
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:trusted
230a113 to
fba2518
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
fba2518 to
d56c1dc
Compare
ApprovabilityVerdict: Needs human review New feature adding pull request links to thread rows, introducing new UI components, async loading logic, and user-facing behavior. Additionally, unresolved Cursor Bugbot findings identify potential issues with unbounded retry loops and stale cache behavior that warrant attention. You can customize Macroscope's approvability policy. Learn more. |
|
@t3dotgg This SwiftUI PR is ready for your review. Its exact head is mergeable; repository/native CI, MacroScope, Cursor, and review-thread audits are clean with zero unresolved threads. The only red status is the unrelated Vercel marketing authorization check. |
4cb3307 to
497f54f
Compare
|
Rebuilt as one focused commit on exact current Theo Verification on this exact Theo-based head:
Conflict classification: stale branch topology + duplicated foundation code, now removed. Unique feature remains wanted. Landing order: merge #5975 first so this UI consumes the cached subscription path; landing this first would remain correct but use the existing expensive refresh path. That is a dependency/performance concern, not a feature conflict. Fresh direct Opus review is unavailable because the confirmed session quota returns HTTP 429 before inference; no independent verdict is claimed. |
d56c1dc to
5f5a76b
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5f5a76b. Configure here.
| reconfigureThreadRows(matching: key) | ||
| } | ||
| pullRequestTasks[key] = (token, task) | ||
| } |
There was a problem hiding this comment.
Stale PR lookup cache
Medium Severity
loadPullRequestIfNeeded treats any stored HomeThreadPullRequestResolution as final, including .resolved(nil). While a checkout key stays visible, the row never refetches, so a missing PR can stay hidden and an existing #number or state can stay outdated after remote changes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5f5a76b. Configure here.
| } catch { | ||
| guard let self, pullRequestTasks[key]?.token == token else { return } | ||
| pullRequestTasks[key] = nil | ||
| return |
There was a problem hiding this comment.
Unbounded PR lookup retries
High Severity
On sourceControlStatus failure, the task entry is cleared and no cooldown is recorded. Visible working rows reconfigure about once per second, so loadPullRequestIfNeeded can immediately retry and call refreshVCSStatus, which also invalidates the server PR-lookup cache.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5f5a76b. Configure here.
1 participant
What changed
Linkwith the short#<number>label.Why
The server already publishes pull-request metadata, but the SwiftUI thread list did not surface it. A manual VCS refresh invalidates the backend PR cache, so list rows use the existing status subscription instead; the source-control screen keeps its explicit refresh behavior.
Scope
This is one native SwiftUI change. Every changed file is under
apps/swift-iosor its native tests. There are noapps/mobile, Expo, React Native, or bridge changes.UI evidence
Owner baseline
0d0c77431c851ff31181fb5f1708f351e0c5e80c:UI capture at
fba2518d76761185499935255b74d1e725936dfd:Integrated simulator safe-destination smoke video — starts on the row and verifies the sanitized destination opens PR #5804 in Safari. The destination launch in this recording was invoked at the simulator OS level because the hosted cell hides nested SwiftUI controls from the semantic automation tree; it is not presented as coordinate-tap evidence.
Final head
d56c1dcf937285609ca31b1c2462d14e9fdb47b9changes only the cache-expiry behavior and its test after this capture; it does not alter the rendered row or link destination.Evidence is Tailnet-only. The downloaded MP4 matched the source byte-for-byte (
f9d14dbf581f6934b4833c39fcaa2ece0e32f11d7c5ccbca63b83afe32f2fa0f, 3,514,304 bytes,video/mp4).Verification
4401D856-DE6C-4769-A649-468778C929F6.apps/swift-ios/Scripts/ci-test.sh: 222 passed across 27 suites;TEST SUCCEEDED.#5804rendered on the matching thread and the sanitized URL opened the actual PR.apps/swift-iosand native Swift tests.Macroscope's valid stale-cache finding was addressed by expiring successful lookup results after five minutes; the focused cache test now covers both success expiry and failure retry.
Independent cross-provider review was attempted with Claude Code 2.1.224 / Claude Opus 5 high, but Claude returned HTTP 429 before inference because the weekly quota was exhausted. No independent-model review is claimed.
Built with GPT-5.6 Sol high in T3 Code's Codex harness.
Note
Add pull request links to thread rows in the iOS workspace view
#1234with an external-link icon) when a matching pull request is found for the thread's branch.projectFaviconClient.sourceControlStatusand cached perHomeThreadPullRequestLookupKey(keyed on environmentID, projectID, branch, and worktreePath); stale lookups are cancelled when rows leave the view.HomeThreadPullRequestPresentationvalidates that the thread and PR branches match, the PR number is positive, and the URL passes a newsafeExternalURLcheck (http/https only, no embedded credentials).Macroscope summarized 5f5a76b.
Note
Low Risk
Scoped to Swift iOS list UI with explicit external URL sanitization; main caveat is extra per-visible-row VCS status fetches, mitigated by keyed caching and task cancellation.
Overview
Swift iOS home thread rows now surface linked pull requests from existing VCS status, without changing server or non-iOS clients.
FeaturePullRequestgainssafeExternalURL, which only allows credential-freehttp/httpsURLs before anything is opened externally.HomeThreadPullRequestPresentationbuilds display data when the thread branch matches status and the PR URL passes that check.The home
UICollectionViewcoordinator loads PR metadata asynchronously viasourceControlStatus, caches results by checkout identity (environment, project, branch, worktree—not thread ID), cancels stale work when rows scroll away, and reconfigures cells when data arrives.FeatureThreadRowshows a compact#<number>Linkin rich and slim layouts; hosted cells add a VoiceOver custom action so opening the PR does not steal the row’s “open task” behavior.Focused tests cover URL rejection, branch matching, lookup-key stability across recycled row IDs, and accessibility copy.
Reviewed by Cursor Bugbot for commit 5f5a76b. Bugbot is set up for automated code reviews on this repo. Configure here.