Skip to content

Mobile host: share the duplicated panel-stream session/coordinator shape - #10417

Closed
lawrencecchen wants to merge 2 commits into
mainfrom
feat-panel-stream-unify
Closed

lawrencecchen wants to merge 2 commits into
mainfrom
feat-panel-stream-unify

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Extracts the panel-stream session bookkeeping that the mobile host duplicated per stream kind into one shared session/coordinator shape, and folds the idle reconcile pass into the existing drive deadline instead of a separate path. Wire formats, event topic names, and JSON keys are unchanged. Net delta from git diff origin/main...HEAD --shortstat: 4 files changed, 112 insertions(+), 170 deletions(-).

Part of the mobile-sync rip-out wave 1.


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

Shares the duplicated mobile panel-stream session bookkeeping and routes idle reconciliation through the existing drive deadline to reduce code and keep behavior stable. Wire topics, payloads, pacing, and JSON keys are unchanged.

  • Adds a shared MobileStreamSessionKey reused by both browser and simulator coordinators; removes per-file SessionKey.
  • In MobileBrowserStreamSession, introduces scheduleAfter(...) and uses it for cadence/settle, idle reconciliation, and state coalescing; .idle now schedules scheduleDeadline(..., reconcile: true) instead of a separate task.
  • In simulator code, extracts releaseOwnership(_:recording:) and recordStream/currentOwnership helpers; in session, adds recordFrame(...). Diagnostics payloads are identical; one lifecycle record now emits before a no-op cache prune.

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

Review in cubic

Summary by CodeRabbit

  • Reliability Improvements

    • Improved consistency when starting, stopping, reconnecting, and ending mobile browser and simulator streams.
    • Enhanced handling of delayed stream updates and idle-state reconciliation.
    • Improved cleanup of completed sessions and cached stream data.
  • Diagnostics

    • Stream and frame activity is now recorded more consistently, providing clearer troubleshooting information without changing existing diagnostic details.

The browser and simulator stream stacks repeat the same shapes inside
each file. Fold the repeats into one helper per file, with wire topics,
payload shapes, and pacing untouched:

- MobileSimulatorStreamCoordinator: one releaseOwnership(_:recording:)
  now owns the ownership-release + closed/stopped lifecycle record that
  stop, connectionClosed, and sessionEnded each duplicated, and local
  recordStream/currentOwnership wrappers replace eight 6-9 line
  MobileSimulatorDiagnostics call blocks (241 -> 202 lines).
- MobileSimulatorStreamSession: a recordFrame wrapper binds the
  session's panelID for the seven frame-lifecycle records.
- MobileBrowserStreamSession: scheduleAfter(_:_:) is the single
  bounded, cancellable sleep-then-act task that scheduleDeadline,
  scheduleIdleReconciliation, and scheduleStateEmission each inlined.

Diagnostics keep identical event payloads; the only reorder is a
lifecycle record now emitted before a cache prune that records nothing.
…ve deadline

The browser and simulator stream coordinators each declared an identical
private SessionKey {connectionID, panelID}. One file-scope
MobileStreamSessionKey (in MobileBrowserStreamCoordinator.swift, no new
pbxproj entry) now keys both session tables; call sites rename only.

MobileBrowserStreamSession.scheduleIdleReconciliation duplicated
scheduleDeadline except for one pacing.requestSettleReconciliation()
call, so scheduleDeadline gains a reconcile flag and the .idle case
passes idleReconcileInterval directly. Topics, payloads, and pacing
are untouched.

The audited cross-file candidates that would not pay were left alone:
the connectionClosed sweep helper (~9 lines for 4 deleted; the
simulator adds per-item ownership release plus a post-loop prune), the
stop(sendClosed:) shells (task sets, teardown, encoders, event types,
and topics all differ), and a shared single-flight loop type (~28
lines plus pbxproj registration against ~20 deleted, with different
drain gating between the two loops).
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 47176453-5fa1-4689-83a6-ba1b87637118

📥 Commits

Reviewing files that changed from the base of the PR and between 882ab10 and e7497b9.

📒 Files selected for processing (4)
  • Sources/Mobile/MobileBrowserStreamCoordinator.swift
  • Sources/Mobile/MobileBrowserStreamSession.swift
  • Sources/Mobile/MobileSimulatorStreamCoordinator.swift
  • Sources/Mobile/MobileSimulatorStreamSession.swift

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


📝 Walkthrough

Walkthrough

The mobile stream coordinators now share session keys. Browser sessions centralize cancellable deadline scheduling. Simulator coordinators and sessions centralize ownership, lifecycle, and frame diagnostics.

Changes

Mobile stream coordination

Layer / File(s) Summary
Shared session identity
Sources/Mobile/MobileBrowserStreamCoordinator.swift, Sources/Mobile/MobileSimulatorStreamCoordinator.swift
Both coordinators use MobileStreamSessionKey for session storage, lookup, lifecycle, acknowledgement, and input-replay operations.
Browser deadline scheduling
Sources/Mobile/MobileBrowserStreamSession.swift
Idle reconciliation, deadline handling, and state-emission coalescing use shared cancellable scheduling helpers.
Simulator diagnostics and lifecycle
Sources/Mobile/MobileSimulatorStreamCoordinator.swift, Sources/Mobile/MobileSimulatorStreamSession.swift
Ownership release and stream recording use coordinator helpers. Frame diagnostics use the session’s recordFrame helper. Session termination prunes caches consistently.

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

Merge Risk: ⚪ Minimal · up to e7497

This PR consolidates duplicated mobile panel-stream bookkeeping without reported contract changes or actionable merge-blocking risk; it is merge-ready after normal checks and review.

Possibly related PRs

Suggested reviewers: azooz2003-bit


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Cmux Swift Actor Isolation ❌ Error The diff introduces module-level pure MobileStreamSessionKey with UUID-only value state but no nonisolated annotation; Swift 6 MainActor-by-default would isolate this shared key to MainActor. Declare MobileStreamSessionKey as nonisolated (and Sendable if it crosses actor boundaries) so the shared value key remains usable without MainActor isolation.
Description check ⚠️ Warning The description explains the changes and scope but omits the required Testing, Demo Video, Review Trigger, and Checklist sections. Add the required sections and record test results, manual verification, demo details, review requests, and checklist status.
✅ Passed checks (23 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: sharing duplicated mobile panel-stream session and coordinator structures.
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 Blocking Runtime ✅ Passed The diff refactors existing cancellable clock sleeps into one helper with the same cadence, idle, and state intervals; simulator keepalive timing is unchanged, with no new blocking wait, lock, sync...
Cmux Browser Automation Off-Main ✅ Passed The diff changes only four Sources/Mobile stream files; TerminalController.swift and ControlCommandExecutionPolicy.swift are unchanged, and no socket-worker browser command routing is introduced.
Cmux Expensive Synchronous Load ✅ Passed The diff adds only session-key, diagnostic, and cancellable scheduling helpers; changed files contain no agent-history loads, file reads, directory scans, or large JSON parsing.
Cmux Cache Substitution Correctness ✅ Passed The diff preserves the existing fresh reader and cached replay paths; it adds no persistence, history, undo, or snapshot cache substitution.
Cmux No Hacky Sleeps ✅ Passed The PR changes only four Swift files. The runtime-no-hacky-sleeps rule explicitly scopes out Swift, so this check is not applicable.
Cmux Algorithmic Complexity ✅ Passed The diff adds no nested collection scans, repeated sorting/filtering, or rescans. Existing linear scans and cache pruning are unchanged in scope and call frequency.
Cmux Swift Concurrency ✅ Passed The diff adds no DispatchQueue, DispatchGroup, Combine, or completion-handler API. Its only new Task is returned and stored in cancellable deadlineTask/stateTask; other Task uses are unchanged.
Cmux Swift @Concurrent ✅ Passed All four changed classes are @MainActor; new scheduleAfter uses an @MainActor Task and closure, and the diff adds no @concurrent or nonisolated async work or new heavy UI call.
Cmux Swift Package Boundaries ✅ Passed The PR refactors existing app-target host stream code; new key and private helpers remain tied to BrowserPanel/SimulatorPanel, globals, and diagnostics, with no package or target change.
Cmux Swiftpm Lockfiles ✅ Passed The diff changes only four Swift source files. No Package.swift, Package.resolved, Xcode project, workflow, or .gitignore file changed, so no lockfile rule applies.
Cmux Swift Logging ✅ Passed The diff adds no print/debugPrint/dump/NSLog, file logging, or Logger declarations; changed diagnostics use MobileSimulatorDiagnostics with Logger and hashed panel handles plus numeric metadata.
Cmux User-Facing Error Privacy ✅ Passed The four-file diff only refactors session keys, scheduling, and internal diagnostics; it adds no string literals or user-facing error/API output.
Cmux Full Internationalization ✅ Passed The four-file diff adds only session keys, scheduling, diagnostics, and comments; string literals are unchanged and no localization catalogs, locale registries, or web messages changed.
Cmux Swiftui State Layout ✅ Passed The diff changes only @MainActor stream coordinators and sessions. It adds no SwiftUI views, state wrappers, layout readers, lazy rows, or render-time state writes.
Cmux Architecture Rethink ✅ Passed The diff centralizes existing cancellable clock scheduling and diagnostic calls; it adds no new sleep path, observer, lock, side channel, duplicate wiring, or UI lifecycle owner.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only mobile stream coordinators and sessions. The diff adds no NSWindow, NSPanel, WindowGroup, identifier, or close-shortcut code.
Cmux Source Artifacts ✅ Passed All four changed paths are hand-written Swift source under Sources/Mobile; the diff adds no artifact directories, binaries, logs, screenshots, caches, or build output.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Placeholder
Cmux No Ambient Global State ✅ Passed The diff adds only the instance-data MobileStreamSessionKey struct at line 6 and private methods; it adds no free function, mutable global, static namespace, or singleton.
✨ 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 feat-panel-stream-unify

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.

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants