ci(tests): fix AppDelegateShortcutRoutingTests teardown crash + make app-host backtraces cheap

[lawrencecchen]

The tests job has been burning ~10 minutes per run on Swift crash backtraces, and the crashes can silently skip later test suites. Both come from one class.

Why (root cause)

AppDelegateShortcutRoutingTests.setUpWithError() throws XCTSkip on headless CI runners (no window server honors makeKeyAndOrderFront) before it assigns the implicitly-unwrapped originalSettingsFileStore. XCTest still runs tearDown() after a skip, and tearDown force-unwrapped that nil → Fatal error: Unexpectedly found nil → app-host crash. On a headless runner every test in the class skips, so this fired ~23 times per run (confirmed in the tests log).

Each crash then ran a fully symbolicated, interactive Swift backtrace taking 80–87s, because the crash happens in the XCTest host process (cmux DEV.app), which never received the job-level SWIFT_BACKTRACE — only xcodebuild itself did. ~23 × ~83s explains most of the gap between the tests job (~25 min) and the rest of CI (seconds to ~2 min on Blacksmith).

This is not a Blacksmith vs Warp issue (the tests job runs on Warp); it reproduces on any headless macOS runner.

Fixes

1. The crash: originalSettingsFileStore becomes a regular optional and tearDown guards the restore (if let …), so tearDown tolerates setUp bailing early. Runner-agnostic; behavior on real machines is unchanged.
2. Never-again, durable: scripts/ci/xcodebuild_noninteractive.py now exports TEST_RUNNER_SWIFT_BACKTRACE before launching xcodebuild. xcodebuild copies TEST_RUNNER_-prefixed vars (prefix stripped) into the test host environment, so any future app-host crash gets interactive=no,symbolicate=off and returns in well under a second instead of 80s+. One place, covers every app-host test invocation already routed through the wrapper.

Verification

tests log before this PR: ~23 Unexpectedly found nil crashes from this class + multiple Backtrace took 8Xs. After: those crashes are gone (the skipped tests tear down cleanly), and the wrapper keeps any unrelated future crash from stalling CI. CI on this PR is the proof.

Out of scope (noted, not fixed here)

• One unrelated Index out of range while reading buffer crash in preservesNonSecretString — separate bug, separate PR.
• The "tolerate app-host crash by parsing the last summary" design can still silently skip suites; this PR removes the dominant crash source and makes crashes cheap, but the harness-level guarantee is a larger change.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

---

Note

Low Risk
Test teardown and CI wrapper env only; no production app behavior changes.

Overview
Stops headless CI from repeatedly crashing the XCTest app host when AppDelegateShortcutRoutingTests skips in setUpWithError() before saving the keyboard shortcut file store.

originalSettingsFileStore is no longer an implicitly unwrapped optional; tearDown() only restores KeyboardShortcutSettings.settingsFileStore when setup actually assigned it, so skipped tests tear down cleanly instead of force-unwrapping nil.

The CI xcodebuild_noninteractive.py wrapper now sets TEST_RUNNER_SWIFT_BACKTRACE (from job SWIFT_BACKTRACE or a fast default) so the test host gets non-interactive, non-symbolicated backtraces—cutting ~80s hangs per app-host crash across all wrapped test runs.

^{Reviewed by Cursor Bugbot for commit 16eec1b. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Fixes a teardown crash in AppDelegateShortcutRoutingTests on headless CI and makes app-host Swift crash backtraces non-interactive and fast, removing long stalls and preventing skipped suites.

• Bug Fixes
  • Made originalSettingsFileStore optional and guarded its restore in tearDown(), so tests that XCTSkip in setUpWithError() don’t crash the app host.
  • In scripts/ci/xcodebuild_noninteractive.py, export TEST_RUNNER_SWIFT_BACKTRACE (from SWIFT_BACKTRACE or a fast default) so the test host gets interactive=no,symbolicate=off; app-host crashes now return quickly instead of hanging.
  • Updated .github/swift-file-length-budget.tsv to allow the extra lines added by the teardown nil-guard.

^{Written for commit 16eec1b. Summary will update on new commits.}

Summary by CodeRabbit

• Tests

  • Improved stability of app delegate shortcut routing tests to prevent crashes during setup phase.

• Chores

  • Enhanced CI/build processes with improved Swift crash backtrace configuration for more effective test diagnostics.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Canceled		Jun 19, 2026 1:21am
cmux-staging	Building	Preview, Comment	Jun 19, 2026 1:21am

[coderabbitai]

📝 Walkthrough

Walkthrough

AppDelegateShortcutRoutingTests changes originalSettingsFileStore from an implicitly-unwrapped to a plain optional and guards its restoration in tearDown(). The CI script xcodebuild_noninteractive.py gains logic to default TEST_RUNNER_SWIFT_BACKTRACE for non-interactive crash backtracing.

Changes

Test teardown safety and CI backtrace default

Layer / File(s)	Summary
Optional store guard in test setup/teardown cmuxTests/AppDelegateShortcutRoutingTests.swift	originalSettingsFileStore is changed from an implicitly-unwrapped optional to a plain optional; tearDown() now restores KeyboardShortcutSettings.settingsFileStore only when the property is non-nil, preventing a crash when setUpWithError() throws XCTSkip before assignment.
Default TEST_RUNNER_SWIFT_BACKTRACE in CI script scripts/ci/xcodebuild_noninteractive.py	Inserts a block in main() that sets TEST_RUNNER_SWIFT_BACKTRACE to the job-level SWIFT_BACKTRACE value when available, otherwise falls back to interactive=no,timeout=0s,symbolicate=off,color=no, and only applies the default when the variable is not already set.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

Possibly related PRs

• manaflow-ai/cmux#4440: Also wires SWIFT_BACKTRACE-related environment variables to control crash backtrace behavior in XCTest CI runs.
• manaflow-ai/cmux#4874: Touches AppDelegateShortcutRoutingTests CI handling by skipping a specific hanging test method, directly related to the same test class being fixed here.

Poem

🐇 A nil store once caused such a fright,
When XCTSkip leapt up in the night.
Now tearDown checks first,
Avoids the worst burst,
And backtraces hum without byte-sized plight! ✨

🚥 Pre-merge checks | ✅ 21 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (21 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the two main changes: fixing AppDelegateShortcutRoutingTests teardown crash and improving app-host backtrace performance in CI.
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	The PR modifies only test code (cmuxTests/AppDelegateShortcutRoutingTests.swift) and a Python CI script, not production Swift. The test change is a nil-safety fix (optional vs implicitly-unwrapped...
Cmux Swift Blocking Runtime	✅ Passed	This PR introduces no blocking or timing-based synchronization in production Swift code. Changes are limited to: (1) a test file (AppDelegateShortcutRoutingTests.swift) converting an implicitly-unw...
Cmux Expensive Synchronous Load	✅ Passed	This PR does not contain production Swift changes—only test infrastructure and CI improvements. The custom check applies explicitly to "production Swift changes"; these changes are test-only and CI...
Cmux Cache Substitution Correctness	✅ Passed	The PR modifies only test code (cmuxTests/AppDelegateShortcutRoutingTests.swift) and CI infrastructure (scripts/ci/xcodebuild_noninteractive.py), neither of which are production Swift/TypeScript/Ja...
Cmux No Hacky Sleeps	✅ Passed	PR introduces no new sleep/timer/delay/polling constructs. Swift changes are out of scope; Python runtime script addition only sets environment variable to prevent (not introduce) long backtraces.
Cmux Algorithmic Complexity	✅ Passed	Both modified files avoid algorithmic complexity violations: AppDelegateShortcutRoutingTests.swift is test scaffolding (explicitly pass-through per rules), and xcodebuild_noninteractive.py adds onl...
Cmux Swift Concurrency	✅ Passed	No legacy async patterns introduced. Changes fix optional unwrapping in test tearDown and set env var in Python script—neither introduces DispatchQueue, Task, Combine, or completion handlers.
Cmux Swift @Concurrent	✅ Passed	PR modifies only synchronous Swift test methods and a property. No async functions, @concurrent annotations, or nonisolated async work was introduced or modified. Changes comply with concurrent ann...
Cmux Swift File And Package Boundaries	✅ Passed	All changes are either test code (cmuxTests/AppDelegateShortcutRoutingTests.swift) or CI scripts (xcodebuild_noninteractive.py), both explicitly allowed under the rule's exception for test fixtures...
Cmux Swiftpm Lockfiles	✅ Passed	All cmux-owned SwiftPM packages include Package.resolved in the diff; package .gitignore files don't ignore Package.resolved (only .build/); root Xcode Package.resolved is included; complies with s...
Cmux Swift Logging	✅ Passed	No logging violations found. Changes are in test code (allowed) and CI script (CLI output allowed). No new print/NSLog/debugPrint/dump statements added.
Cmux User-Facing Error Privacy	✅ Passed	Changes only modify test code and internal CI infrastructure; no user-facing error messages, alerts, or user-visible text are added or modified that would expose sensitive details like environment...
Cmux Full Internationalization	✅ Passed	PR modifies only test code (cmuxTests/AppDelegateShortcutRoutingTests.swift) and CI operational scripts (scripts/ci/xcodebuild_noninteractive.py) with no user-facing strings, localization APIs, str...
Cmux Swiftui State Layout	✅ Passed	PR contains no SwiftUI state/layout changes. Changes are confined to test setup/teardown logic and CI environment variable configuration, which fall outside the scope of the SwiftUI state layout re...
Cmux Architecture Rethink	✅ Passed	Both changes are allowed cases per the rule: test-only correctness fix with clear invariant (optional binding guards tearDown after setUp fails early) and required platform bridge infrastructure (e...
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR changes only test teardown logic and CI environment variables; no new windows, NSWindow/NSPanel/NSWindowController/SwiftUI Window/WindowGroup code introduced or materially changed.
Cmux Source Artifacts	✅ Passed	Both changed files are hand-written source code (Swift test and Python CI script) in appropriate source directories, with no artifact patterns, hidden scratch directories, or generated/build output.
Description check	✅ Passed	The PR description is comprehensive and follows the expected template structure with Summary, Testing, and clear explanations of root causes and fixes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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-tests-teardown-crash

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

Fixes a CI-reproducible teardown crash in AppDelegateShortcutRoutingTests on headless runners where XCTSkip in setUpWithError() left originalSettingsFileStore nil before tearDown() force-unwrapped it, and adds TEST_RUNNER_SWIFT_BACKTRACE forwarding in the CI wrapper to cap any future app-host crash backtrace at well under a second.

• originalSettingsFileStore is now a proper optional (?) and tearDown guards the restore with if let, so skipped tests tear down cleanly without crashing the app host.
• scripts/ci/xcodebuild_noninteractive.py exports TEST_RUNNER_SWIFT_BACKTRACE=interactive=no,timeout=0s,symbolicate=off,color=no (or inherits from SWIFT_BACKTRACE) via os.environ.setdefault, propagating the setting through xcodebuild's TEST_RUNNER_ prefix-stripping into every app-host test invocation.
• .github/swift-file-length-budget.tsv is updated to reflect the 4-line growth in the test file.

Confidence Score: 5/5

All changes are scoped to tests and CI tooling; no production app code, user-facing paths, or data models are touched.

The Swift change is a minimal, correct narrowing of an IUO to an optional with a matching guard — it eliminates a real crash without altering any behavior on non-headless machines. The Python change sets an env var before pty.fork() using setdefault, so any externally-supplied TEST_RUNNER_SWIFT_BACKTRACE is preserved, and the fallback value only affects CI backtrace verbosity. Both changes are self-contained, well-commented, and easy to revert if needed.

No files require special attention.

Important Files Changed

Filename	Overview
cmuxTests/AppDelegateShortcutRoutingTests.swift	IUO → optional for originalSettingsFileStore with if let guard in tearDown; fixes nil crash on headless CI when setUpWithError XCTSkips before assignment.
scripts/ci/xcodebuild_noninteractive.py	Adds os.environ.setdefault("TEST_RUNNER_SWIFT_BACKTRACE", ...) before pty.fork() so the XCTest app host inherits non-interactive, unsymbolicated backtrace config via xcodebuild's TEST_RUNNER_ prefix-stripping.
.github/swift-file-length-budget.tsv	Increments the line budget for AppDelegateShortcutRoutingTests.swift from 12215 to 12219 to match the 4-line diff.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant XCTest
    participant setUpWithError
    participant tearDown
    participant SettingsStore as KeyboardShortcutSettings

    Note over XCTest,SettingsStore: Headless CI (before fix)
    XCTest->>setUpWithError: run
    setUpWithError-->>XCTest: throws XCTSkip (no window server)
    Note over setUpWithError: originalSettingsFileStore never assigned (nil IUO)
    XCTest->>tearDown: run (XCTest always calls tearDown)
    tearDown->>SettingsStore: "settingsFileStore = originalSettingsFileStore!"
    Note over tearDown: 💥 Fatal: Unexpectedly found nil → app-host crash

    Note over XCTest,SettingsStore: Headless CI (after fix)
    XCTest->>setUpWithError: run
    setUpWithError-->>XCTest: throws XCTSkip
    Note over setUpWithError: originalSettingsFileStore stays nil (Optional)
    XCTest->>tearDown: run
    tearDown->>tearDown: if let originalSettingsFileStore (nil → skip)
    Note over tearDown: ✅ No crash, teardown completes cleanly

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant XCTest
    participant setUpWithError
    participant tearDown
    participant SettingsStore as KeyboardShortcutSettings

    Note over XCTest,SettingsStore: Headless CI (before fix)
    XCTest->>setUpWithError: run
    setUpWithError-->>XCTest: throws XCTSkip (no window server)
    Note over setUpWithError: originalSettingsFileStore never assigned (nil IUO)
    XCTest->>tearDown: run (XCTest always calls tearDown)
    tearDown->>SettingsStore: "settingsFileStore = originalSettingsFileStore!"
    Note over tearDown: 💥 Fatal: Unexpectedly found nil → app-host crash

    Note over XCTest,SettingsStore: Headless CI (after fix)
    XCTest->>setUpWithError: run
    setUpWithError-->>XCTest: throws XCTSkip
    Note over setUpWithError: originalSettingsFileStore stays nil (Optional)
    XCTest->>tearDown: run
    tearDown->>tearDown: if let originalSettingsFileStore (nil → skip)
    Note over tearDown: ✅ No crash, teardown completes cleanly

Loading

_{Reviews (2): Last reviewed commit: "Trim comment + refresh Swift length budg..." | Re-trigger Greptile}