feat(sim,engine): @linkcode/sim SDK + SimulatorBackend with per-session device ownership (CODE-393)#249
feat(sim,engine): @linkcode/sim SDK + SimulatorBackend with per-session device ownership (CODE-393)#249AprilNEA wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d9670d770
ℹ️ 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".
| this.onChildGone(); | ||
| } | ||
| }); | ||
| // A failed spawn (e.g. missing binary) errors the pipes; a broken pipe means the child is |
There was a problem hiding this comment.
Ignore exit events from replaced sidecars
After a malformed frame, the data handler calls child.kill() and immediately invokes onChildGone(), allowing the next request to spawn a replacement before this old child's asynchronous exit event arrives. This unguarded handler then clears the replacement from this.child, resets its shared decoder, and rejects all of its pending requests, so recovery can fail and leave the replacement process orphaned; associate teardown with the emitting child and ignore events when it is no longer current.
Useful? React with 👍 / 👎.
| await this.backend.boot(udid); | ||
| const claim = this.claims.get(udid); |
There was a problem hiding this comment.
Preserve claims while a service boot is in flight
If the owning session stops while backend.boot() is pending, releaseSession() sees bootedByService === false and deletes the claim. When the boot later succeeds, this lookup finds nothing, leaving a device that the service booted unowned and absent from both idle reclaim and engine-shutdown reclaim; coordinate release with the in-flight boot so a successful boot remains tracked and is eventually shut down.
Useful? React with 👍 / 👎.
| this.drop(udid); | ||
| // Reclaim is best-effort: the device may already be gone (deleted in Xcode, host reboot). | ||
| this.backend.shutdownDevice(udid).catch(noop); |
There was a problem hiding this comment.
Delay freeing a device until reclaim shutdown finishes
At the idle deadline, the claim is removed before shutdownDevice() settles. Because the sidecar executes requests concurrently, another session can claim the same UDID immediately and boot, launch, or capture it while the previous session's shutdown is still running, causing the new operation or device state to be disrupted; retain the claim through the shutdown and only free the matching claim after the promise settles.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds the TypeScript simulator SDK and connects simulator ownership to the engine and packaged daemon. The main changes are:
Confidence Score: 4/5Sidecar replacement and simulator ownership transitions can invalidate active requests or leave devices in the wrong lifecycle state.
What T-Rex did
Important Files Changed
Sequence DiagramsequenceDiagram
participant Session
participant Service as SimulatorService
participant Client as SimSidecarClient
participant Sidecar as linkcode-sim
Session->>Service: operation(sessionId, udid)
Service->>Service: validate or create claim
Service->>Client: correlated request
Client->>Sidecar: framed stdin message
Sidecar-->>Client: result or screenshot frame
Client-->>Service: resolve by requestId
Service-->>Session: result
Session->>Service: releaseSession(sessionId)
Service->>Service: schedule idle reclaim
Service->>Client: shutdownDevice(udid)
Reviews (1): Last reviewed commit: "test(sim): SDK-driven simulator loop int..." | Re-trigger Greptile |
| child.stdin.on('error', () => this.onChildGone()); | ||
| child.stdout.on('error', () => this.onChildGone()); | ||
| child.on('exit', () => this.onChildGone()); | ||
| child.on('error', () => this.onChildGone()); |
There was a problem hiding this comment.
Stale Child Events Kill Replacement
After a crash, a new request can spawn another sidecar before the old child's delayed error, exit, or stdout event arrives. Because every callback mutates shared client state without checking its child, an old event can clear the replacement, reject its pending requests, or feed old bytes into its decoder.
Artifacts
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: focused Vitest configuration used to execute the harness
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
- Keeps the command output available without making the summary code-heavy.
| // Don't let a pending reply keep the host's event loop alive on its own. | ||
| timer.unref(); | ||
| this.pending.set(requestId, { resolve, reject, timer }); | ||
| writeFrame(child.stdin, REQUEST, body); |
There was a problem hiding this comment.
Failed Writes Wait For Deadline
If stdin fails between ensureChild() and this write, the frame may never reach the sidecar while its pending entry remains registered. The caller then waits for the full operation deadline—up to 210 seconds for boot—instead of receiving the write failure immediately.
Artifacts
Repro: focused controlled-stream Vitest harness
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: focused Vitest configuration
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: verbose runtime trace showing the write failure and unresolved boot request
- Keeps the command output available without making the summary code-heavy.
| const claim = this.claims.get(udid); | ||
| if (claim?.sessionId === sessionId) claim.bootedByService = true; |
There was a problem hiding this comment.
Boot Completion Loses Ownership
If the session stops while list() or boot() is pending, releaseSession() removes the claim before bootedByService is set. A successful boot then has no matching claim to update, so the service can treat its own booted simulator as user-owned and leave it running after idle reclaim or engine shutdown.
Artifacts
Repro: deferred-backend lifecycle test harness
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: focused Vitest configuration
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: verbose failing test output and lifecycle trace
- Keeps the command output available without making the summary code-heavy.
| this.drop(udid); | ||
| // Reclaim is best-effort: the device may already be gone (deleted in Xcode, host reboot). | ||
| this.backend.shutdownDevice(udid).catch(noop); |
There was a problem hiding this comment.
Reclaim Frees Device Too Early
The timer drops ownership before shutdownDevice() finishes. Another session can claim and use the same UDID during that gap, after which the previous session's pending shutdown can turn off the newly claimed simulator.
Artifacts
Repro: focused deferred-shutdown SimulatorService test harness
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: focused Vitest configuration used to execute the harness
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: verbose successful test output with chronological ownership-race trace
- Keeps the command output available without making the summary code-heavy.
Summary
Stacked on #247. The TypeScript layer above the
linkcode-simsidecar:@linkcode/sim(new package,packages/host/sim): framed-stdio codec mirroring the Rust proto (zod-validated at the boundary), andSimSidecarClient— lazy spawn, per-request correlation with outer reply deadlines, crash fail-pending + respawn, EOF shutdown. Binary path is a constructor arg; no packaging probing in the package.SimulatorBackendport (structural, mirrors thePtyBackendseam — neither package imports the other) +SimulatorService, the policy layer: device↔session ownership (auto-claim,conflicton cross-session use), ≤4 devices per session (limit_exceeded), 10-min idle reclaim of service-booted devices after their session stops (resuming inside the window keeps the device), user-booted devices never auto-shutdown, engine-shutdown reclaim. Wire (CODE-394) and MCP tools (CODE-395) must route through this service.resolveSimSidecarPath()(LINKCODE_SIM_SIDECAR_PATHoverride → devtarget/releasefallback; non-macOS resolves to none), injected assimulatorBackendonly when configured; the packaged supervisor exports the env fromresourcesPath(macOS only).docs/ENVIRONMENT.mdupdated.Verification
pnpm check:ci+pnpm testgreen (unit: 5 client + 5 codec + 7 registry tests).LINKCODE_SIM_E2E=1, opt-in vitest): cold boot → install (fixture UIKit app) → launch (pid) → screenshot (JPEG >10 KB) → terminate → shutdown, 13s on an iPhone 17 / iOS 26.5 simulator; all devices confirmed shut down afterwards.