Skip to content

Fix iOS TestFlight crash paths - #9034

Merged
azooz2003-bit merged 3 commits into
mainfrom
fix-ios-crash-r2
Jul 28, 2026
Merged

azooz2003-bit merged 3 commits into
mainfrom
fix-ios-crash-r2

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Move libghostty iOS surface C callbacks out of GhosttySurfaceView.makeSurface(app:) into nonisolated top-level functions so off-main libghostty calls do not inherit UIView main-actor isolation.
  • Add a focused iOS regression test that invokes the surface callbacks from a background queue.
  • Replace raw SwiftUI Divider() context-menu separators in workspace group headers with Section groups to avoid UIKit context-menu layout aborts while measuring visible menu cells.

ASC evidence

  • INTERNAL latest visible crash AGnxHSCbggL4TYIf5AxJkVM, created 2026-07-28T04:53:09.494Z, bundle dev.cmux.app.internal: EXC_CRASH (SIGABRT) during UIKit/SwiftUI context menu layout, with +[NSLayoutConstraint constraintWithItem:...] under -[UIContextMenuInteraction updateVisibleMenuWithBlock:] on thread 0.
  • DEMO latest visible crash is still ADkFBL8Ch61FNTq2u4uRLHA, created 2026-07-26T00:36:17.844Z, bundle dev.cmux.app.demo: EXC_BREAKPOINT (SIGTRAP) on thread 8 at _dispatch_assert_queue_fail -> _swift_task_checkIsolatedSwift, mapped to the GhosttySurfaceView.makeSurface(app:) callback path and Surface.destroy.
  • A newer DEMO report mentioned by dogfood was not visible in ASC when polled after the report.

Verification

  • git diff --check origin/main...HEAD passed.
  • xcodebuild -quiet test -workspace ios/cmux.xcworkspace -scheme cmux-ios -destination 'platform=iOS Simulator,id=38B83108-E1AD-44EB-B521-ACCB0C4C90E4' -derivedDataPath /tmp/cmux-ios-test-cr2-rebased '-only-testing:CmuxMobileTerminalTests/GhosttySurfaceCallbackTests/ghosttySurfaceCallbacksRunOffMainThread()' passed. xcresult summary: totalTestCount: 1, passedTests: 1, failedTests: 0.
  • xcodebuild -quiet build -workspace ios/cmux.xcworkspace -scheme cmux-ios -configuration Release -destination 'generic/platform=iOS' -derivedDataPath /tmp/cmux-ios-build-cr2-rebased CODE_SIGNING_ALLOWED=NO exited 0.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fix two iOS crashes in TestFlight: off‑main libghostty surface callbacks violating main‑actor isolation, and context‑menu layout aborts in workspace group headers. Stabilizes surface IO and context menus.

  • Bug Fixes
    • Centralized libghostty iOS surface C callbacks as nonisolated static C function pointers on GhosttySurfaceBridge (ioWriteCallback, renderPresentedCallback), and registered them from GhosttySurfaceView so off‑main calls don’t inherit UIView main‑actor isolation.
    • Replaced Divider() with Section groups in WorkspaceGroupHeaderRow context menus to avoid UIKit layout crashes.
    • Added GhosttySurfaceCallbackTests.ghosttySurfaceCallbacksRunOffMainThread() to verify callbacks safely run off the main thread.

Written for commit a751cb7. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Improvements
    • Reorganized workspace group context-menu actions into clearer, capability-based sections.
    • Improved organization of terminal surface callback handling to make input/render updates more reliable.
  • Tests
    • Added coverage to confirm terminal surface callbacks execute on a background thread (not the main thread) to help keep the interface responsive.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR reorganizes workspace group context-menu actions into conditional sections and extracts Ghostty surface callbacks into named bridge functions, with a UIKit test verifying background-thread execution.

Changes

Workspace group menu

Layer / File(s) Summary
Conditional context-menu sections
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupHeaderRow.swift
Groups pin/rename, new-workspace, and destructive actions into separate conditional Section blocks while preserving existing actions and state updates.

Ghostty surface callbacks

Layer / File(s) Summary
Callback extraction and threading validation
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceBridge.swift, Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift, Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/GhosttySurfaceCallbackTests.swift
Adds named callback entry points, wires them into surface creation, and tests invocation from a background queue.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: lawrencecchen

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PASS: The diff only moves C callbacks into nonisolated bridge functions and adds a background-thread test; no new implicit MainActor models, unsafe Sendable refs, or bg UI-store access.
Cmux Swift Blocking Runtime ✅ Passed PASS: No new blocking waits/sleeps/syncs/locks were introduced in production code; the only NSLock is pre-existing, and the new DispatchQueue usage is test-only scaffolding.
Cmux Browser Automation Off-Main ✅ Passed No browser automation code changed; the diff only touches iOS Ghostty callbacks and context-menu sections, so the socket-worker routing rule is not implicated.
Cmux Expensive Synchronous Load ✅ Passed Diff only groups menu sections and rewires Ghostty callbacks; no agent-history/session/transcript load was added or moved onto a main/interactive path.
Cmux Cache Substitution Correctness ✅ Passed Diff only moves Ghostty C callbacks into static entry points and groups menu actions; no persistence/history/undo/snapshot read was swapped for a cache/opportunistic value.
Cmux No Hacky Sleeps ✅ Passed Diff is Swift-only; the no-hacky-sleeps rule is out of scope here, and no new sleep/timer/polling delays were introduced.
Cmux Algorithmic Complexity ✅ Passed No scalable-collection scans or hot-path sorting/filtering were added; the diff only refactors fixed-size context-menu sections and callback wiring.
Cmux Swift Concurrency ✅ Passed Diff removes a production DispatchQueue.main.async and only adds test-only queue use at a callback boundary, which the rule explicitly allows.
Cmux Swift @Concurrent ✅ Passed No changed async helper needs @concurrent; the new callbacks are synchronous C entry points, and the regression test explicitly hops to a background queue.
Cmux Swift Package Boundaries ✅ Passed PASS: the diff is UI/menu wiring and Ghostty integration glue in package targets; no independently testable reusable domain logic was kept in the app target.
Cmux Swiftpm Lockfiles ✅ Passed PR touches only Swift source/tests; no Package.swift, Package.resolved, .gitignore, or Xcode project/workflow files changed.
Cmux Swift Logging ✅ Passed No added or changed print/debugPrint/dump/NSLog/Logger usage in the diff; only callback refactors and a test, so logging rules are satisfied.
Cmux User-Facing Error Privacy ✅ Passed Diff only changes context-menu grouping and callback plumbing; no user-facing error, alert, command-output, or recovery copy was added or modified.
Cmux Full Internationalization ✅ Passed No new user-facing copy was added; the workspace menu keeps existing L10n.string keys, and no xcstrings/locale files changed. Other edits are non-user-facing.
Cmux Swiftui State Layout ✅ Passed No new SwiftUI state/layout anti-patterns were introduced; WorkspaceGroupHeaderRow only refactors contextMenu sections, and the other edits are UIKit bridge/test code.
Cmux Architecture Rethink ✅ Passed Required platform bridge and context-menu layout fix; no sleeps, polling, duplicate wiring, or split ownership introduced, and async is test-only.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Diff only refactors a context menu, Ghostty callbacks, and a test; no NSWindow/NSPanel/WindowGroup or cmuxAuxiliaryWindowIdentifiers changes.
Cmux Source Artifacts ✅ Passed All changed paths are source/test files; no logs, caches, DerivedData, screenshots, or scratch/artifact dirs appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed New callbacks are used by GhosttySurfaceView production code and tests; no #if DEBUG/test-only accessor or visibility-widening seam was added under Sources.
Cmux No Ambient Global State ✅ Passed No new ambient globals appear: the Ghostty callbacks are static let constants inside GhosttySurfaceBridge, and no top-level mutable state or singleton was added.
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing iOS TestFlight crash paths.
Description check ✅ Passed Summary, ASC evidence, and verification are thorough, but the Demo Video, Review Trigger, and Checklist sections are missing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-ios-crash-r2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift`:
- Around line 14-33: Move cmuxIOSurfaceIOWriteCallback and
cmuxIOSurfaceRenderPresentedCallback onto GhosttySurfaceBridge as nonisolated
static callback methods, preserving their existing callback behavior and
userdata handling. Update any callback registrations or references to use the
bridge-owned methods, and remove the top-level functions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c476565e-fe6b-43b6-ab43-03237959579a

📥 Commits

Reviewing files that changed from the base of the PR and between 0aec6ce and a4de568.

📒 Files selected for processing (3)
  • Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceGroupHeaderRow.swift
  • Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
  • Packages/iOS/CmuxMobileTerminal/Tests/CmuxMobileTerminalTests/GhosttySurfaceCallbackTests.swift

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@azooz2003-bit
azooz2003-bit merged commit 9946d58 into main Jul 28, 2026
6 checks passed
@azooz2003-bit
azooz2003-bit deleted the fix-ios-crash-r2 branch July 28, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant