Skip to content

fix(swift-ios): apply audit findings across transport, home, chat, and settings - #10843

Open
t3dotgg wants to merge 1 commit into
t3code/rebuild-mobile-app-swiftfrom
theo/swiftui-audit
Open

fix(swift-ios): apply audit findings across transport, home, chat, and settings#10843
t3dotgg wants to merge 1 commit into
t3code/rebuild-mobile-app-swiftfrom
theo/swiftui-audit

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Sep 8, 2026

Copy link
Copy Markdown
Member

Audit of the SwiftUI iOS client on top of #5178. Five sub-agents reviewed transport, home list, chat, terminal, usage, PRs, and settings. This PR applies the findings that were safe to change without a design discussion. It is one PR so you can pull it down and test it as one unit.

Bugs fixed

  • Dropped WebSocket events. Subscription streams had a bounded buffer that silently dropped values under load. The server already flow-controls with Acks, so the buffer is now unbounded and the drop path is gone. Reconnect backoff caps at 30s and a network path monitor wakes the backoff early when the network comes back.
  • Rejected pairing looped forever. A 401 from the server was treated like a transient failure, so the client polled and reconnected forever. There is now a needsPairing connection state. It stops polling, shows "Pair again" in Home, thread view, Settings, and the connection hub, and opens onboarding prefilled with the endpoint.
  • Links in the transcript did nothing. UIHostingConfiguration does not inherit the SwiftUI environment, so openURL overrides never reached transcript cells. The open handler is now injected per cell.
  • Thread status disagreed with web. Running and error states now come from the session status, matching resolveSidebarThreadStatus in the web sidebar. Working timers use the same source.
  • Activity rows disagreed with web. Runtime warnings with no content, plan boundary tools, and agent internal events are filtered with the same rules as session-logic.ts.
  • Pinned order disagreed with web. Threads carry pinOrderKey and sort with the same rule as sortPinnedThreadsByOrderKey.
  • Capabilities went stale. Thread actions read capabilities from the live server config, not the descriptor cached at pairing time.
  • Worktree creation silently downgraded. The FeatureClient protocol had a chain of default overloads that dropped arguments. Each operation now has one requirement, so a call cannot lose the worktree or identity fields on the way to the server.
  • Outbox queueing matched error strings. Queue decisions are now type based (cancellation, URL errors, RPC timeout or disconnect, HTTP 5xx, environment not connected).
  • Double resolves. Approvals and questions track in-flight request IDs, so a second tap while the first is in flight is ignored.
  • Approval detail could push the buttons off screen. Detail now scrolls inside a bounded height. Cancel and deny buttons meet the minimum tap target.
  • Voice input cancel during preparation did nothing. Cancel now cancels the preparing task.
  • Archive and snooze showed on rows that could not use them. Rows gate both on capabilities and state, in context menus, swipe actions, and accessibility.
  • New thread failures showed a generic alert. The composer shows the real error inline.

Performance

  • Home list no longer re-sorts on every streamed token. Row content changes bump threadRowRevision. Only changes to the order key (pin, state, dates, archive) bump homePresentationRevision and rebuild the shelves.
  • Platform root only reprocesses thread changes when state or membership changed.
  • Terminal buffer diffs. Terminal output is applied as append or replace deltas with a 4KB anchor instead of re-feeding the whole buffer. Surface generation churn is gone.
  • Usage and PR tabs stay mounted. Tabs no longer tear down and refetch on switch. Usage presentation is computed once per load. PR search debounces 400ms with a small cache, and diff parsing runs off the main actor with cancellation.
  • Reset countdowns are the only periodic timer. TimelineView runs at one minute and only for countdowns.
  • Settings are cached after the first load instead of re-decoded on every read.
  • Thread refresh after resolving a request is skipped when the thread is live and already synchronized.

UX

  • Usage, PRs, and Devices use flat sections with dividers instead of rounded cards, matching the rest of the app.
  • Terminal background is true black.
  • PR actions confirm only merge and close. Other actions run directly.
  • Keyboard dismisses interactively while scrolling the transcript, and scroll-to-bottom does not fight a user drag.
  • Draft saving is no longer blocked while a message is sending.
  • Queued and failed user messages show a footer state in the transcript.

Removed

Dead code the audit found: pollShell, unused theme tokens, the needsInput and failed sidebar buckets, the lossy FeatureClient default chain, forwarding overloads in NativeFeatureClient, the outbox removal-retry machinery, and two test files that only asserted callback wiring.

Tests

New: TerminalBufferTests, UsagePresentationTests, and a large-chunk delivery test for the WebSocket RPC. Updated tests for the changed protocol signatures, thread status rules, and swipe gating. Full suite run on iPhone 17 Pro simulator.

Not changed

Findings that need a design call or a separate PR: splitting NativeFeatureClient (7800 lines), a settings write queue, DPoP keys in the Secure Enclave, deep-link pairing confirmation while the workspace is visible, local plus remote notification double-fire, relay registration re-POST every 60s, drafts JSON rewrite with inline image bytes, Swift 6 language mode, iPad NavigationStack back gesture, state restoration, and the Ghostty binary path relative to apps/mobile.

Changes made by Claude Fable 5.1 through Claude Code.

…d settings

Five sub-agents audited the SwiftUI client. This commit applies the
findings that were safe to change without a design discussion.

Transport
- Subscription streams no longer drop values under load. The server
  flow-controls with Acks, so the buffer is unbounded.
- Reconnect backoff caps at 30s. A network path monitor wakes the
  backoff early when the network returns.
- A rejected credential (401) is a needsPairing state, not a transient
  failure. Polling stops and every surface offers "Pair again".

Home
- Row content changes bump threadRowRevision. Only order key changes
  rebuild the shelves, so streamed tokens no longer re-sort the list.
- Thread status, activity filters, and pinned order match the web
  client rules.
- Archive and snooze gate on live capabilities and state.
- Capabilities read from the live server config, not the descriptor
  cached at pairing.

Chat
- Transcript links open. The openURL handler is injected per cell
  because UIHostingConfiguration does not inherit the environment.
- Approvals and questions ignore a second tap while one is in flight.
- Approval detail scrolls in a bounded height so buttons stay visible.
- Keyboard dismisses interactively. Scroll to bottom does not fight a
  user drag. Draft saving is not blocked while a message sends.
- Queued and failed user messages show a footer state.
- Voice input cancel during preparation cancels the task.

Terminal, usage, PRs, devices
- Terminal output applies append or replace deltas with a 4KB anchor.
  Background is true black.
- Usage and PR tabs stay mounted. PR search debounces with a small
  cache. Diff parsing runs off the main actor with cancellation.
- Only merge and close confirm. Flat sections replace rounded cards.

FeatureClient
- One requirement per operation. The lossy default chain that dropped
  worktree and identity arguments is gone, along with the forwarding
  overloads in NativeFeatureClient.
- Outbox queueing is type based instead of error string matching.
- Settings are cached after the first load.

Removed dead code: pollShell, unused theme tokens, needsInput and
failed sidebar buckets, outbox removal retry, and two tests that only
asserted callback wiring.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting).

This review would cost an estimated $9.66, which exceeds your per-review limit of $8.00.

The top 3 files driving up this estimate:

File Diff Size Estimate
apps/swift-ios/App/NativeFeatureClient.swift 25.59KB $1.28
apps/swift-ios/Features/Root/FeatureRootModel.swift 22.53KB $1.13
apps/swift-ios/Features/Usage/UsageView.swift 21.02KB $1.05

Tip

To get this pull request reviewed, you can:

  1. Comment @macroscope-app on this PR to request a manual review (monthly spend limits still apply).
  2. Exclude the file(s) above from review by adding a pattern to your .macroscope/ignore.md — note that creating this file replaces Macroscope's built-in default ignores rather than extending them.
  3. Raise your cost limit in your workspace billing settings.

Turn off this reminder going forward

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Sep 8, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR changes core transport defaults and reconnection semantics while also altering credential handling, durable message delivery, thread state, terminal rendering, usage subscriptions, and several user-facing workflows. Its cross-cutting production impact and authentication-related behavior warrant human review.

Not approved because:

  • Per-review cost limit exceeded (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings, or comment @macroscope-app review this PR to bypass the limit and review now. You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 15.1 KiB
Codex Thread snapshot wire 7.0 KiB 7.3 KiB
Codex Live turn WebSocket wire 6.6 KiB 7.8 KiB
Codex Live turn WebSocket decoded 57.1 KiB 66.4 KiB
Codex Live turn messages 10 21
Claude Total thread wire 13.6 KiB 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.3 KiB
Claude Live turn WebSocket wire 6.6 KiB 7.8 KiB
Claude Live turn WebSocket decoded 57.9 KiB 66.4 KiB
Claude Live turn messages 10 21

Baseline: unavailable · PR result: 34782f1 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant