Skip to content

refactor: extract UI test infrastructure from AppDelegate - #13226

Merged
teamleaderleo merged 7 commits into
mainfrom
refactor/upstream-ui-test-infrastructure
Sep 22, 2026
Merged

teamleaderleo merged 7 commits into
mainfrom
refactor/upstream-ui-test-infrastructure

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Reviewer summary

Moves UI test setup out of the app delegate so test support is easier to change without adding more app-lifecycle branching.

What changed

Extract the AppDelegate's UI-test infrastructure into focused coordinators and keep the AppDelegate as the composition root.

This groups the diagnostics writer, Feed sidebar fixture, and socket sanity fixture into one cohesive slice. It includes the forwarding methods needed to avoid a lazy-property initialization cycle while preserving the existing diagnostics payloads, stage ordering, and test environment behavior.

This branch has been reconstructed directly on current manaflow-ai/cmux:main; it does not depend on the stale fork refactor stack. The project-file diff contains only the five extracted UI-test source registrations.

Validation

  • swiftc -D DEBUG -parse over changed Swift sources
  • scripts/check-pbxproj.sh
  • git diff --check
  • Full cmux-unit build passed on the same extraction after initializing submodules and using the matching GhosttyKit cache.
  • The reviewed result has since been re-rooted onto current upstream main; fresh current-head CI is still required.

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

Moves UI-test setup out of AppDelegate into focused coordinators under Sources/Debug/UITests/, keeping AppDelegate as the composition root while preserving diagnostics payloads, stage ordering, and socket probe/restart behavior.

Behavior changes

  • The feed sidebar fixture now observes pending permission requests with a 15-second deadline instead of polling 75 times at 200 ms.
  • The portal-stats diagnostics callback now runs on the main queue.
  • Five process-identity files add import CmuxFoundation to unblock native compile admission.

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

Review in cubic

Summary by CodeRabbit

  • New Features

    • Improved UI-test diagnostics with more detailed launch, window, display, portal, rendering, and socket-health information.
    • Added automated feed-sidebar testing with reveal, push, pending-state, timeout, and result tracking.
    • Added optional socket sanity checks that can detect unhealthy connections and trigger recovery.
  • Refactor

    • Consolidated UI-test diagnostics and coordination into dedicated components for more consistent behavior.

Replaces #13006 (same commits, head branch moved into the org so it gets the build cache and can be kept current with main).

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b3871bad-4ef5-46ac-8b44-d70a86548b08

📥 Commits

Reviewing files that changed from the base of the PR and between be71faf and eafe78d.

📒 Files selected for processing (7)
  • Sources/AppDelegate.swift
  • Sources/Debug/UITests/FeedSidebarUITestCoordinator.swift
  • Sources/Debug/UITests/FeedSidebarUITestPushClient.swift
  • Sources/Debug/UITests/FeedSidebarUITestRecorder.swift
  • Sources/Debug/UITests/UITestDiagnosticsWriter.swift
  • Sources/Debug/UITests/UITestSocketSanityCoordinator.swift
  • cmux.xcodeproj/project.pbxproj

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

The DEBUG-only UI-test diagnostics logic moves from AppDelegate into feed-sidebar, socket-sanity, and diagnostics-writer components. AppDelegate wires these components and routes stage recording through the new diagnostics writer.

Changes

UI-test diagnostics refactor

Layer / File(s) Summary
Feed sidebar coordinator and build wiring
Sources/AppDelegate.swift, Sources/Debug/UITests/*, cmux.xcodeproj/project.pbxproj
Feed sidebar reveal, push, pending observation, result recording, and socket-line handling move into dedicated DEBUG-only types. AppDelegate starts the coordinator, and the project registers the new sources.
Diagnostics writer migration
Sources/AppDelegate.swift, Sources/Debug/UITests/UITestDiagnosticsWriter.swift
Manifest creation and portal, socket, render, window, launch, and display stage recording move into UITestDiagnosticsWriter.
Socket sanity coordinator migration
Sources/AppDelegate.swift, Sources/Debug/UITests/UITestSocketSanityCoordinator.swift
Socket health probing, delayed scheduling, restart handling, and socket-sanity stage recording move into UITestSocketSanityCoordinator.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to eafe7

No current code defect blocks merging, though outstanding required runtime checks should still complete.

🚥 Pre-merge checks | ✅ 24 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 5 files. (2 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: extracting UI-test infrastructure from AppDelegate.
Description check ✅ Passed The description provides a concrete summary, rationale, implementation details, validation results, and current testing limitations. It is mostly complete, although it does not include the template's …
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 Cloud Persistent Session And Early Input ✅ Passed PASS — The PR changes only AppDelegate UI-test diagnostics/fixtures, five DEBUG UI-test support files, and Xcode project registration. The changed code does not create Cloud terminals, cmux-tui client…
Cmux Swift Actor Isolation ✅ Passed PASS. The authoritative diff adds only DEBUG-gated UI-test infrastructure and wires it inside AppDelegate's existing @MainActor scope. FeedSidebarUITestCoordinator, FeedSidebarUITestRecorder, UITestDi…
Cmux Swift Blocking Runtime ✅ Passed PASS: The timing and blocking constructs are confined to DEBUG UI-test infrastructure. The new ContinuousClock().sleep calls implement the Feed sidebar test deadline and socket-sanity test delay, wh…
Cmux Browser Automation Off-Main ✅ Passed The PR does not change browser socket automation routing. Sources/TerminalController.swift, ControlCommandExecutionPolicy.swift, and their policy tests are unchanged. The only new socket command i…
Cmux Expensive Synchronous Load ✅ Passed The PR does not add or move an expensive synchronous agent-history load. The changed-line search found no additions involving RestorableAgentSessionIndex, SharedLiveAgentIndex, agent stores, trans…
Cmux Cache Substitution Correctness ✅ Passed PASS. The diff does not replace an authoritative persistence, history, undo, or snapshot read with a cache. The Feed sidebar code changes FeedCoordinator.snapshot(pendingOnly: false) to a main-actor…
Cmux No Hacky Sleeps ✅ Passed The check applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. The PR changes only Swift files and Xcode project registration. The timing constructs found are `ContinuousCloc…
Cmux Algorithmic Complexity ✅ Passed PASS: The changed coordinator and diagnostics files are all #if DEBUG UI-test infrastructure. The new collection work is limited to notification-driven fixture checks, fixed diagnostic dictionaries,…
Cmux Swift Concurrency ✅ Passed PASS. The diff does not introduce a prohibited concurrency pattern. It removes the old DispatchQueue.global(qos: .userInitiated).async feed push and replaces it with an awaited Task.detached opera…
Cmux Swift @Concurrent ✅ Passed PASS. The new FeedSidebarUITestPushClient.push(requestId:) is non-actor-isolated and async, but its blocking socket call and JSON parsing run inside an explicit Task.detached boundary. The call fr…
Cmux Swift Package Boundaries ✅ Passed The diff does not introduce production domain logic that requires a SwiftPM boundary. All five extracted files are under Sources/Debug/UITests/ and wrapped in #if DEBUG; they implement UI-test fix…
Cmux Swiftpm Lockfiles ✅ Passed The reviewed diff changes only AppDelegate, adds UI-test Swift sources, and registers those sources in cmux.xcodeproj/project.pbxproj. It does not change Package.swift, .gitignore, or any SwiftPM pack…
Cmux Swift Logging ✅ Passed PASS — the changed UI-test diagnostics are debug-only and do not add a prohibited production logging path. All five new UI-test files are enclosed in #if DEBUG, and the AppDelegate diagnostic calls …
Cmux User-Facing Error Privacy ✅ Passed PASS. The changed UI-test coordinators and diagnostics writer are wrapped in #if DEBUG, and their execution requires UI-test environment variables. They write test artifacts and operator diagnostics…
Cmux Full Internationalization ✅ Passed PASS. The authoritative diff changes only AppDelegate debug UI-test wiring, five new #if DEBUG UI-test coordinators, and Xcode source registration. The added strings are diagnostic/test-result field…
Cmux Swiftui State Layout ✅ Passed The PR does not introduce or materially expand SwiftUI state or layout code. The authoritative diff changes AppDelegate.swift and adds DEBUG UI-test coordinators that use AppKit/Foundation/Observati…
Cmux Architecture Rethink ✅ Passed PASS. The new timing and observation code is confined to #if DEBUG UI-test fixtures. FeedSidebarUITestCoordinator replaces the pre-existing 200 ms polling loop with model observation and a bounded…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The changed Swift files add or refactor only DEBUG UI-test infrastructure under Sources/Debug/UITests and AppDelegate wiring. They do not create or materially change a standalone NSWindow, NSPan…
Cmux Source Artifacts ✅ Passed The authoritative diff changes only hand-written Swift source files and the Xcode project configuration. The five new files are DEBUG UI-test coordinators, a recorder, a diagnostics writer, and a sock…
Cmux No Test Or Debug Seam In Production Source ✅ Passed No new disallowed production seam is introduced. The five added Swift files are isolated under Sources/Debug/UITests/ and each is fully guarded by #if DEBUG, matching the rule's dedicated debug-fo…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 5 files. (2 skipped: 1 unsupported, 1 too large.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no actionable correctness, security, or repository-rule violations identified.

Summary

Refactors DEBUG-only UI-test infrastructure out of AppDelegate into focused coordinators while preserving its role as the composition root.

  • Extracts Feed sidebar reveal, push, pending-state observation, and result recording.
  • Extracts diagnostics manifest generation and socket sanity checks.
  • Keeps blocking Feed socket work off the main actor.
  • Registers the five new source files in the Xcode project.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    AD[AppDelegate composition root]
    FEED[FeedSidebarUITestCoordinator]
    PUSH[FeedSidebarUITestPushClient]
    REC[FeedSidebarUITestRecorder]
    DIAG[UITestDiagnosticsWriter]
    SOCK[UITestSocketSanityCoordinator]
    APP[App and window state]
    SOCKET[Control socket]

    AD --> FEED
    AD --> DIAG
    AD --> SOCK
    FEED --> PUSH
    FEED --> REC
    FEED --> DIAG
    PUSH --> SOCKET
    SOCK --> SOCKET
    DIAG --> APP
Loading

Reviews (4) · Last reviewed commit: "Merge upstream CI baseline for full nati..."

@cursor

cursor Bot commented Sep 20, 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.

Apply upstream 968cef2 (#13236) to unblock native compile admission without merging unrelated main changes.
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Native compile follow-up: applied the exact five-file import repair from upstream #13236 (968cef2005940ee167bed3f8ab01434eb8b7dc2c) in 363075a54ae2801acd3d2187d9402f7efb95a959. These files use CmuxFoundation process identity types but the inherited baseline lacked the import. This is the minimal upstream fix, without merging unrelated main changes. Diff validation passes; fresh native CI at this new head is still required, and no native compile/runtime pass is claimed by this comment.

@teamleaderleo teamleaderleo added the full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks. label Sep 20, 2026
@cursor

cursor Bot commented Sep 20, 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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Repaired the actual warning-budget failure in 2d1eaa33d5eccb382a241eca43c2172d49484635. The NotificationCenter observer is explicitly delivered on .main; its callback now uses MainActor.assumeIsolated before accessing the isolated diagnostics writer. This removes the two introduced actor-isolation warnings without increasing the warning budget or introducing delayed/fire-and-forget work. Current head 11730ba8a8a3bcbe7f9b6fd3905cec61d0e4e83c integrates upstream main 024562cb7f. Swift parse/diff checks pass. Full-ci was set before the final push; run35526988696 confirms full_suite=true. Native validation remains pending.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Full-suite evidence at11730ba8a8a3bcbe7f9b6fd3905cec61d0e4e83c: app+test compilation and Swift warning budget now pass. Runtime shard2 job106124394194 failed initial and retry checkout before testing, with built-in diagnostics showing GitHub DNS failure (nameserver192.168.64.1); escalated runner evidence. Runtime shard6 job106124394167 has actual test failures across shared Dock/persistence/browser/workspace/CLI suites, also observed on13211. Shared baseline repair is coordinated centrally; required runtime checks remain unsatisfied, so this is not merge-ready.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 21, 2026 08:31
@teamleaderleo teamleaderleo removed the full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks. label Sep 22, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo merged commit ba1ded9 into main Sep 22, 2026
49 of 51 checks passed
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