Skip to content

base red: UpdateInstallCheckTests fakes the frontmost app instead of reading a nil NSApp - #33093

Merged
lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red-update-install-check
Nov 3, 2026
Merged

lawrencecchen merged 1 commit into
feat-cmux-nextfrom
base-red-update-install-check

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Base red on feat-cmux-next: CmuxNextAppTests.UpdateInstallCheckTests crashes in theRelaunchRecordsTheInstallAndTheNextLaunchJudgesIt (UpdateRelaunchFocus.swift:99, "Unexpectedly found nil while implicitly unwrapping an Optional value").

Cause: the test calls updaterWillRelaunchApplication(), which reads UpdaterService.frontmost. Its default is AppKit's answer (UpdateFrontmostApp.current()), which reads NSApp.isActive. The package-test lane runs each suite in its own process, and this suite makes no NSApplication, so NSApp is nil and the read traps. The suite passed only when an earlier suite in the same process had made NSApplication.shared.

Fix: the seam exists for tests ("AppKit in the app, a fake in tests"); UpdateRelaunchFocusTests and UpdateRelaunchHandoffTests set it. The suite's helper now sets it too (this app frontmost, as for a clicked Restart to Update). No product change: the app always has NSApp.

Culprit: 6229004 (#31178) added the suite without the fake.

Owner: no OWNERS.md row covers CmuxNextUpdater.

Testing

Command: cmux-ci run --class light --display --script scripts/ci/package-test-lane.sh --ref SHA --arg=suite --arg=Packages/macOS/CmuxNext --arg=CmuxNextAppTests.UpdateInstallCheckTests (5 runs in one step).

  • Base (feat 73eaec7 on cmuxs-Mac-mini and cmux11s): the suite crashes at UpdateRelaunchFocus.swift:99.
  • Tip: job a554671b75a6b853405d5c5d at feat 11dfe91 (cmux8s) is red on the first two of five rounds for this suite.
  • Head: job 563602e40b08edfe6ed6d384 (cmux-austin-mini-0) at b356d59: the suite passed 5 of 5, no crash.

Each job's lane summary is red for a reason outside this change: on these hosts the target's XCTest pass aborts (signal 6, after Contacts/AddressBook CoreData store errors) or hangs ~680-900 s with no test in flight, after the Swift Testing pass. The Swift Testing results below are the suite's own.

Observed / Not run

Observed: the fleet runs above. python3 scripts/cmux-next/prepush-gate.py passes.
Not run: no app build; test-only change.

Changelog

none

…s sibling suites do

theRelaunchRecordsTheInstallAndTheNextLaunchJudgesIt and
anInstallThatEndsWithTheAppRunningLeavesNoRecord call
updaterWillRelaunchApplication(), which reads UpdaterService.frontmost.
Its default is AppKit's answer (UpdateFrontmostApp.current()), which reads
NSApp.isActive. The package-test lane runs each suite in its own process,
and this suite makes no NSApplication, so NSApp is nil and the read traps
(UpdateRelaunchFocus.swift:99, 'Unexpectedly found nil'): the whole suite
crashes. It passed only when an earlier suite in the same process had made
NSApplication.shared.

The seam is there for tests ('AppKit in the app, a fake in tests');
UpdateRelaunchFocusTests and UpdateRelaunchHandoffTests set it. The helper
now sets it too. No product change: the app always has NSApp.

Culprit: 6229004 (#31178) added the suite without the fake.
Red: fleet job 2bf90b6c18ca136f3fb6c15d at feat 11dfe91.
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@blacksmith-sh

blacksmith-sh Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmphs1rmo7t/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmpoe7u9_6w/summary-missing.json (live):/summary-missing.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

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

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review b356d59: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr33093 -f ref=base-red-update-install-check -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 6b31a88 to compare; do not merge until green; see lane-rules

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent, Opus 5.5 claude-opus-5-5, read-only): SHIP at b356d59

  • The cause is correct. UpdateFrontmostApp.current() reads NSApp.isActive with no nil check (UpdateRelaunchFocus.swift:99). Its only callers run inside Sparkle install callbacks, and AppServices.swift:572 is the only code that makes an UpdaterService. So NSApp is never nil on this path in the app. Only the test process has no NSApp.
  • The test is not weakened. No assertion changes. The fake front-app hook is the same seam its sibling suites use (UpdateRelaunchFocusTests.swift:22,125, UpdateRelaunchHandoffTests.swift:19). Every test runs with automaticInstallInFlight == false, so the fake's answer changes no expected result.
  • No global state is left behind, and the change opens no windows.
  • Fleet: UpdateInstallCheckTests passed 5 of 5 in job 563602e40b08. The lane is red overall only because of the known fleet XCTest-stage AddressBook crash (STATUS 7919/7933).

@lawrencecchen
lawrencecchen marked this pull request as ready for review November 3, 2026 07:37
@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 3, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Correction to my review receipt above: the lanes were red overall because of the test-bundle run-loop guard, which held the main thread in xctest. It was not the macOS AddressBook store; that was only a symptom. #33023 and #33013 fixed it on feat (ecfbb44). This head is older than that fix, so its lane still shows the XCTest stage red. The suite's own result (passes at this head, fails at the tip) is unchanged. No action is needed on this PR.

@lawrencecchen
lawrencecchen merged commit 7edab48 into feat-cmux-next Nov 3, 2026
315 of 348 checks passed
@lawrencecchen
lawrencecchen deleted the base-red-update-install-check branch November 3, 2026 08:06

This branch was previously deployed

1 inactive deployment
ci-routing-agent — b356d59f Deployed Nov 3, 2026 by lawrencecchen via namespace autofix #13098
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant