Ios Sim Editor - #215
Merged
Merged
Conversation
Safety checkpoint capturing in-progress iOS simulator editor work prior to merging main (windows sync #213) into this branch.
Prepare lane for review: includes accumulated UI, services, and docs work for the iOS simulator editor surface plus supporting renderer, main-process, CLI, and iOS app changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lifecycle helper probes both stable and unstable_* method names, but the type only declared the unstable_* variants. Add optional closeSession / resumeSession properties so the stable-prefixed branch typechecks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub. |
Important Review skippedToo many files! This PR contains 152 files, which is 2 over the limit of 150. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (20)
📒 Files selected for processing (152)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
arul28
commented
Apr 28, 2026
OwnerAuthor
@copilot review but do not make fixes |
Copilot stopped work on behalf of arul28 due to an error
April 28, 2026 20:28
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
arul28
commented
Apr 28, 2026
OwnerAuthor
@CodeRabbit review |
✅ Actions performedReview triggered.
|
arul28
commented
Apr 28, 2026
OwnerAuthor
@codex review |
| position: number; | ||
| }; | ||
| let xcodeMcpBridge: XcodeMcpBridge | null = null; |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
xcodeMcpBridge is declared as a module-level let variable, making it a process-wide singleton shared by every IosSimulatorService instance. When a project context is closed, dispose() at line 3377 calls closeXcodeMcpBridge(xcodeMcpBridge, ...), which kills the child process and rejects all pending requests — even those belonging to a still-active service instance from another project.
// line 188 — module-level globalletxcodeMcpBridge: XcodeMcpBridge|null=null;// line 3377 — dispose kills the shared bridgeif(xcodeMcpBridge)closeXcodeMcpBridge(xcodeMcpBridge,newError("iOS simulator service disposed."));Impact: If a user switches projects or has multiple windows, disposing one project's service tears down preview rendering for all others, causing unexpected errors. Fix: move the bridge into the service instance (a field on the closure) so each instance manages its own lifecycle, or add reference counting to the global.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:8ba89ebdf3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+3237
to
+3241
| if (streamProcess || streamPollTimer) { | ||
| if (streamStatus.deviceUdid === device.udid && streamStatus.running) { | ||
| return streamStatus; | ||
| } | ||
| await stopStream(); |
There was a problem hiding this comment.
The early-return guard in startStream only checks streamProcess/streamPollTimer, so an already-running simulator-window-capture session (which sets neither) falls through and is restarted on repeat calls for the same device. In that path startWindowCaptureStream calls stopStream() first, which emits an unnecessary stream-stopped/stream-started sequence and can cause visible flicker or transient state resets for existing listeners even though no device/backend change was requested.
Useful? React with 👍 / 👎.
Comment on lines
+957
to
+961
| server.listen(0, "127.0.0.1", () => { | ||
| const address = server.address(); | ||
| const port = typeof address === "object" && address ? address.port : 0; | ||
| server.close(() => resolve(port)); | ||
| }); |
There was a problem hiding this comment.
getFreePort closes a probe server and returns the port, then startMjpegStreamServer later binds the real HTTP server to that same port. Another process can claim the port in that gap, leading to intermittent EADDRINUSE failures when starting live stream backends. Binding the final server directly to port 0 (and then reading the assigned port) avoids this TOCTOU race.
Useful? React with 👍 / 👎.
…view CI: - Restore favicon.svg referenced by docs.json (file was deleted in main's perf cleanup but the docs.json reference remained, breaking validate-docs and ci-pass). Review (apps/desktop/src/main/services/ios/iosSimulatorService.ts): - Move xcodeMcpBridge from module scope into the IosSimulatorService factory closure so each instance owns its own bridge lifecycle and init lock; prevents a dispose() in one project from killing pending requests in another. - Make startMjpegStreamServer bind atomically with listen(0, ...) and read the assigned port from server.address(); closes the getFreePort/listen TOCTOU race. - normalizeLaunchArguments now throws on non-string array elements instead of coercing null/objects via String(), matching the throwing style of normalizeLaunchEnvironment. - Document that anonymous (chatSessionId == null) sessions are unowned by design so future readers don't read it as a bypass. - Extend startStream early-return to also fire on streamStatus.running (matching device + backend); previously simulator-window-capture reruns fell through and emitted a spurious stopped/started pair. Addresses: greptile 3156998830, 3156998896, 3156998953, 3156999010, 3156999080; capy-ai 3157011255; codex 3157022831, 3157022837. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arul28
commented
Apr 28, 2026
OwnerAuthor
@copilot review but do not make fixes |
Copilot stopped work on behalf of arul28 due to an error
April 28, 2026 20:50
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Greptile Summary
This PR introduces a full iOS Simulator editor panel into the desktop app, adding a 3413-line
IosSimulatorService(launch/build/stream/inspect/preview), a 2626-lineChatIosSimulatorPanelReact component with interact/inspect/preview modes, shared IPC and type definitions, and an Xcode MCP bridge for Preview Lab rendering. Several issues flagged in the prior review have been addressed: the MJPEG server now uses OS-assigned port 0,normalizeLaunchArgumentsvalidates string types, andstartStreamnow guards onstreamStatus.running.Confidence Score: 4/5
Safe to merge — no blocking logic errors; two P2 style/hardening items remain.
All findings from the prior review are addressed. The two remaining issues are P2: a TOCTOU race on the idb_companion port (inherent architectural limitation, idb is optional) and synchronous file I/O that can momentarily stall the event loop on large projects. Neither blocks correctness.
apps/desktop/src/main/services/ios/iosSimulatorService.ts — ensureCompanion (getFreePort race) and collectSwiftFiles/inferSwiftUITabItems/listPreviewTargets (synchronous I/O loops)
Important Files Changed
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "ship: iteration 1 — fix validate-docs, a..." | Re-trigger Greptile