Fix CI: unit-test failures reported green, change-detection dying on PRs#122
Merged
Conversation
run_suite read $? after a branchless 'if', which is always 0, so any XCTest failure exited 0 and CI reported red suites as green. Capture the status directly with '|| exit_code=$?'. Also fix the guard test's xcodebuild stub: its split-stateful case fabricated two log lines per invocation, so the runner's two real passes logged four calls and the guard failed even on correct behavior. Log the actual arguments once per invocation instead.
The --depth=1 fetch of the base and head SHAs grafts away their parent history, so 'git diff BASE...HEAD' dies with 'no merge base' on every pull request and all gated jobs skip. The checkout already uses fetch-depth: 0, so a plain fetch is correct and effectively free.
The split-stateful parallel mode landed in the same commit as the exit-code bug that reported failing suites as green, so it has never had an honestly-reported passing run. With failures propagating again, the parallel pass fails dozens of AppKit-adjacent tests across multiple test runner processes on both macos-15 and macos-26. Return to the serial scope that was green before the split; re-enable parallelism separately once the suite is parallel-safe.
- .zshenv still sourced the pre-rebrand cmux-zsh-integration.zsh, so local zsh shell integration never loaded in shipped builds - inheritedTerminalConfig read a raw surface pointer instead of the quarantining liveSurfaceForGhosttyAccess accessor (use-after-free risk) - developer-tools reveal called show twice and left transition state in flight after synchronous settles, silently queueing later toggles - goBack skipped a page when a live navigation moved past both restored history stacks - the chord-shortcut action cache was refreshed on an async notification queue, so it was stale at the moment a just-configured chord was used
Rebrand leftovers (bash integration path, __cmuxFind* JS globals, 'cmux profiles' copy, half-renamed fixture path), intentional behavior changes (no nightly channel, per-tag debug sockets, alpha rounding, flash ring metrics), and CLI notify mocks that predate the v2 notification.create_for_target RPC.
Both shell-integration scripts still looked for a 'cmux' binary when PROGRAMA_BUNDLED_CLI_PATH was unset, so relay RPCs (TTY reporting, port-scan kicks) silently no-oped for any shell without the bundled CLI env var.
- report-TTY expectation updated to the v2 surface.report_tty JSON the script actually sends (still asserting no panel scope) - the TERM-preservation test's recorder ran on every precmd and truncated its own log; make it one-shot - the bash preexec test polled until the log was merely non-empty and read it before the background ports_kick RPC landed; poll for the second RPC instead
- toggleDeveloperTools now keeps a short settle window in flight even when the transition completed synchronously, so rapid toggle bursts coalesce to the final intent instead of hitting the inspector per call - createFloatingPopup copies the opener's WKProcessPool so popups share the opener's browsing context
BrowserProfileStore persists lastUsedProfileID into UserDefaults.standard, so profile-mutating tests (and prior runs on the same machine) leaked a non-default profile into BrowserPanelRemoteStoreTests, which then resolved a profile-scoped data store instead of .default(). Add a DEBUG-only replaceSharedForTesting seam and give the suite an ephemeral defaults suite per test.
…r insert-at-end - preferredPullRequest accepted 'not a url' because URL(string:) parses relative paths; require absolute http(s) with a host - poll-candidate exclusion for in-flight probes wrongly filtered panels with confirmed metadata; scope it to directory rediscovery only - closeWorkspace tore down workspaces it never owned; guard on tabs membership - insert-at-end browser placement hit Bonsplit's insertion-index no-op short-circuit; use tabs.count as the insertion index
The install hop only writes and chmods the staged bootstrap script; a login shell would source remote rc files as a side effect. Production has sent /bin/sh -c here since before the CLI split.
…on close - focusTabFromNotification now begins its own focus transition so an older pending FocusTransitionCoordinator completion no-ops instead of reapplying its stale captured panel on the next run-loop turn - closePanelWithConfirmation clears the closed surface's notifications unconditionally; the previous gating skipped the clear whenever the last-surface close escalated to workspace/window close
…try fallback - raise the tokenExtraCharacter penalty so a one-character omission from a real word outranks an extra character after a shorter command - pending empty-state reuse assumed query monotonicity, which is false for a typo-tolerant matcher; only exact query matches may reuse it - branch search tokens now include the last path segment with hyphens preserved, matching how directory tokens already index - v2ScheduleTelemetryMutation gains the same self.tabManager fallback every other v2 resolution path has, so report_tty works for directly registered managers - testRenameScoresHigherThanUnrelatedCommand asserted a fuzzy match that no strategy can produce (sibling test asserts nil for the same shape); assert nil explicitly
- hitTest in both host views now fast-paths only keyboard events and runs full divider/pass-through routing for everything else, so programmatic and non-pointer hit-tests stop silently bypassing it - browser divider hits prefer hosted-inspector regions when an app-level divider coincidentally overlaps; terminal-side divider grab shrinks to 2pt next to hosted terminal content so column-0 selection wins - prune detaches anchorless hosted views; reveal marks WebKit rendering state for reattach without firing the heavy lifecycle pair; fully orphaned anchors hide immediately instead of inheriting the reparent grace period - drag-handle capture resolves the window-level top hit (with per-window re-entrancy scoping) so a top-hit titlebar container blocks capture
- overlay focus no longer requires AppDelegate registration when none exists; keeps the cross-tab ownership protection - deferred overlay mount strongly captures its surface for the closure's lifetime; keydown recovery clears desired focus when windowless - shift-backquote ESC fallback checks the original event before Ghostty mods-translation destroys the signal - test fixes with evidence: visibility-policy test contradicted its deterministic sibling since introduction; keydown test asserted a first-responder teardown order AppKit never provided; prune test now drains the anchor inside an autoreleasepool
Replace NSClassFromString lookups and the private setTintColor: selector with compile-time NSGlassEffectView usage under #if compiler(>=6.2) + @available(macOS 26.0, *), matching the pattern the ghostty submodule already uses. This also fixes window-level Liquid Glass never activating: the old code cast NSGlassEffectView to NSVisualEffectView.Type, which fails at runtime because NSGlassEffectView subclasses NSView, so the fallback path always ran even on macOS 26.
…generation - unregisterMainWindow clears the app-wide browserAddressBarFocusedPanelId when no live window owns the panel; the leaked pointer hijacked Cmd+N/Cmd+P in every other window through the omnibar-navigation path - orphaned main-window contexts are pruned before context selection, not only when resolution fails - closeWindowWithConfirmation uses close() (performClose is a no-op without a key window); titlebar accessory detaches in minimal mode - setSearchOverlay only bumps the mutation generation when it actually mutates, so a redundant reactive call can't cancel the pending focus retry - test-side: wire the existing key-window test override, correct an AppKit teardown-order assumption, bound-retry drift installation against in-flight focus reasserts, fix a decoy-overlay setup that never reached production code
- addWorkspace stopped consulting inheritedTerminalConfigForNewWorkspace long ago; the method had zero production callers and existed only for a test override. Delete it and point the sanitization test at the real fontSize-only inheritance path, preserving every assertion - sidebar PR display has iterated all panels since its introduction and the poll-candidate machinery depends on background panels; rewrite the focused-only test to assert the actual contract (all panels in sidebar order, branch-mismatched PRs filtered)
- TerminalSurfaceRegistry used NSHashTable.weakObjects(), which does not reliably weak-store pure Swift classes — every registered surface was permanently retained. Track surfaces in Swift-native weak boxes - BrowserPanelView's register(defaults:) landed in the process-wide registration domain, making BrowserThemeSettings.mode() skip the legacy forced-dark-mode migration for any defaults instance in the process, including real users'; consult the persistent domain instead
- save/restore AppDelegate.shared in suites that construct throwaway delegates; the clobbered singleton emptied the chord-action cache for every later suite - release leaked ghostty test surfaces via the existing helper; replace fixed 50ms sleeps with bounded polling - guard an unguarded removeFirst() that crashed the whole test process under timing contention
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Fixes CI so test failures actually fail the build again, then fixes everything that surfaced: 109 unit tests were failing silently — some for months — because the old workflow treated assertion failures as "expected" and reported green. Along the way this fixes ~25 real shipped bugs the greenwashing hid, including zsh shell integration never loading at all in shipped builds.
Summary
Review attention
Test Plan