Conversation
…st-launch hook is armed Xcode's simctl and devicectl shims run `xcodebuild -runFirstLaunch` before every call when their EXPECTED_VERSION differs from the installed CoreSimulator or CoreDevice CFBundleVersion. That cleanup deletes every device in ~/Library/Developer/XCTestDevices, which the runner redirect points at a scoped --ios-simulator-device-set set, so the user's devices were wiped. acquireXcodebuildSimulatorSetRedirect now probes every tool declared in XCRUN_TOOL_NAMES (one shared 2 s budget, fail closed) before installing the redirect, and refuses with reason xctest_device_set_cleanup_armed, the rule row's static hint, and details.xcrunShims. Both callers (the build and the session start) go through it, so an armed host refuses before build-for-testing and before test-without-building. Refs #2935
…coped simulator sets Refs #2935
|
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/platform-apple/src/core/xcrun-shim-first-launch.ts">
<violation number="1" location="packages/platform-apple/src/core/xcrun-shim-first-launch.ts:52">
P3: When the shared 2 s budget expires, every unanswered probe reads as armed with `shimPath: null`, so `describeArmedShim` reports "could not be located" and the refusal's static hint sends the user to change the Xcode selection — even though nothing was actually missing. On any host where the four `xcrun --find` spawns plus shim reads plus plutil reads do not all fit in 2 s (cold CoreSimulator, loaded machine), all redirected iOS builds are hard-refused with a diagnosis that points nowhere. Track whether the armed record came from a real locate failure versus a budget expiry (e.g. set a `timedOut` flag on `evidence` in the abort handler), and let the message/hint distinguish the two.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| export async function probeXcrunShimFirstLaunchHooks( | ||
| options: XcrunShimProbeOptions = {}, | ||
| ): Promise<XctestDeviceSetCleanupArming> { | ||
| const signal = AbortSignal.timeout(XCRUN_SHIM_PROBE_BUDGET_MS); |
There was a problem hiding this comment.
P3: When the shared 2 s budget expires, every unanswered probe reads as armed with shimPath: null, so describeArmedShim reports "could not be located" and the refusal's static hint sends the user to change the Xcode selection — even though nothing was actually missing. On any host where the four xcrun --find spawns plus shim reads plus plutil reads do not all fit in 2 s (cold CoreSimulator, loaded machine), all redirected iOS builds are hard-refused with a diagnosis that points nowhere. Track whether the armed record came from a real locate failure versus a budget expiry (e.g. set a timedOut flag on evidence in the abort handler), and let the message/hint distinguish the two.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/core/xcrun-shim-first-launch.ts, line 52:
<comment>When the shared 2 s budget expires, every unanswered probe reads as armed with `shimPath: null`, so `describeArmedShim` reports "could not be located" and the refusal's static hint sends the user to change the Xcode selection — even though nothing was actually missing. On any host where the four `xcrun --find` spawns plus shim reads plus plutil reads do not all fit in 2 s (cold CoreSimulator, loaded machine), all redirected iOS builds are hard-refused with a diagnosis that points nowhere. Track whether the armed record came from a real locate failure versus a budget expiry (e.g. set a `timedOut` flag on `evidence` in the abort handler), and let the message/hint distinguish the two.</comment>
<file context>
@@ -0,0 +1,174 @@
+export async function probeXcrunShimFirstLaunchHooks(
+ options: XcrunShimProbeOptions = {},
+): Promise<XctestDeviceSetCleanupArming> {
+ const signal = AbortSignal.timeout(XCRUN_SHIM_PROBE_BUDGET_MS);
+ return await Promise.all(
+ XCRUN_TOOL_NAMES.map(async (tool) => await probeWithinBudget(tool, options, signal)),
</file context>
There was a problem hiding this comment.
Addressed or already resolved as of 34ab803. The per-point disposition is in #2947 (comment).
…s and type why a shim is armed - The probe spends COLD_TOOLCHAIN_PROBE_TIMEOUT_MS, the budget sized for the first-exec xcrun stall, instead of its own 2 s, and it follows the request's abort signal, so a canceled build or session gives the device-set lock back as a cancellation. - XCRUN_TOOLS carries a firstLaunchShim trait per tool; only simctl and devicectl are probed. - Each armed entry in details.xcrunShims carries armedBy (version_mismatch, version_unreadable, shim_unreadable, shim_not_located, probe_out_of_budget, probe_canceled), and the message follows it. - One shared fake-shim fixture, built on the captured Xcode 26.2 simctl shim, serves the core and runner tests and the startup-failure fixture table.
|
Addressed the review at 957bb81. New head 8f14569. Worked from the five quality notes (no maintainer review is on record for this PR yet — only 1. 2 s budget → shared cold-toolchain budget + abort signal 2. Probe only tools that can carry the hook 3. One shared fake-shim fixture 4. Tests 5. Typed Gross diff: before 17 files +1039/-6 → after 21 files +1145/-16 (net dedupe ~120 lines against the new coverage). Gate on 8f14569: Leg A re-run (armed host, 8f14569): Leg B (non-armed host) is still not runnable here and remains the maintainer's merge condition. |
There was a problem hiding this comment.
All reported issues were addressed across 18 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Fix all with cubic | Re-trigger cubic
… back and cover every armedBy reason in docs The XCTest device-set redirect's shim-armed refusal handed the lock back without checking whether that give-back could restore the host's own device set, unlike the no-redirect path beside it; a failed restore was silently dropped in favor of the shim refusal. It now throws the restore failure first, with a test that forces the give-back's own reconcile to fail after the redirect-in reconcile already succeeded. The armed-shim hint, and the CLI help/commands.md/installation.md prose, described the refusal as only a version mismatch. All three now cover every armedBy case the probe can fail closed on (unreadable or missing shim, unreadable version data, a probe that ran out of its budget) and stop promising xcrunShims always carries readable versions. Also: the startup-failure fixture's xcodeVersion/output field comments now record the xctest-device-set-redirect entry's version.plist/shim-text exception instead of only stating the xcodebuild -version shape, and the positive-case test for the redirect's typed classification now passes a realistic armed shim instead of an empty xcrunShims array the redirect never actually publishes.
|
At 8f14569, the refusal logic looks sound, but the PR is not ready yet because the one test that proves it works on a real host hasn't been run. CI is green (21 checks), but that only exercises the Apple runner acquire path through fakes; no simulator lane opens a scoped set on an armed or disarmed host, so it can't confirm the refusal behaves correctly on live devices. What's missing is Leg B: a live run on a non-armed host, where Not blocking: Is there a smaller shape here? The probe touches a new host-port entry, the device-set gate, and a classification row, and the issue calls for a per-tool list, so a single-module probe seems like the real size. The one place that looks reducible is the test-only A few things I couldn't check: whether each test actually kills a mutation, since the mutation script wasn't run; which Xcode versions are supported, which affects how much devicectl's absence on Xcode 14 or earlier matters; whether the shim text format holds for Xcode 26.x/27 beyond the captured 26.2 fixture (only Leg B or a second capture would confirm this doesn't fail closed on a different phrasing); and whether Once Leg B runs clean and the device survives, this looks ready for human review. |
…plexity gate acquireXcodebuildSimulatorSetRedirect repeated the same hand-back-then-check-restoreFailure shape at three exits; the third occurrence, added to fail-close the shim-refusal path, tipped the function over fallow's cyclomatic/cognitive complexity threshold. Extract handBackOrThrowRestoreFailure so all three exits share one implementation instead of one more inlined branch.
…a production option xcrunShimPaths on XcrunShimProbeOptions let tests answer `xcrun --find` without going through the tool-provider, the seam testing.md already requires (no production exports or test-only dependency injection). Production call sites never passed it. withFakeXcrunHost already fakes `xcrun --find` through the real tool-provider seam, so every caller that passed xcrunShimPaths was already running inside that fake and can rely on it instead.
|
Addressed the review at 8f14569. New head Leg B is still open. This Mac is armed: simctl 1051.17.7 against CoreSimulator 1155.4, and devicectl 506.6 against CoreDevice 629.3. It can't produce a disarmed run, so Leg B needs a host whose shims match the installed frameworks, or the macOS CI image. On this host, the armed Leg A has run twice (957bb81, 8f14569). A runner-backed command is refused with the typed reason in about 50 ms, before any build-for-testing. XCTestDevices never becomes a symlink, and the scoped device survives the Non-blocking notes, all three taken:
Your questions:
Gate on |
…ellation apart from armedBy
|
This follow-up to the review on 8f14569 fixes the refusal path itself; what's still missing is proof it works on a real toolchain, not fakes. The route that now runs on every scoped-set simulator build (https://github.com/callstack/agent-device/blob/34ab803/packages/platform-apple/src/runner/runner-device-set.ts#L122) has no live proof behind it. A cold Smoke Tests was still queued when this was reviewed, so there's no failure to attribute either way. That job builds the simulator runner through Not blocking: the new error hint (https://github.com/callstack/agent-device/blob/34ab803/packages/platform-apple/src/runner/runner-error-classification.ts#L447) covers a version mismatch and a timed-out probe but gives no remediation for I didn't run the new restore-failure test; I judged it would have failed on the old code by reading 8f14569's refusal branch against the test's EACCES predicate, so that's a read, not a run. You mentioned four comments on 34ab803 are still being worked — the phase-deadline cap in Before this can merge, we need that live cold scoped-set open on a host that isn't armed, showing |
… the phase budget, not an armed shim Only the cold-toolchain budget now reads a stopped shim as probe_out_of_budget. A stop by the owning phase's clock, or a phase already spent when the probe starts, fails the redirect with runner_phase_budget_exhausted at the one gate both callers share. The session-side redirect spends the startup time read before the build, like the launch after it.
|
Addressed the review at 34ab803. New head 9573ece. 1. Probe ignores the phase deadline ( 2. Fake 3. No real mid-probe cancel coverage ( 4. Follow-on found while fixing #1: once the probe was phase-capped, a phase that ran out during the probe was still misreported as an armed shim ( Gate on 9573ece: Live evidence:
Not invalidated: the armed-host Leg A run from the prior head is unchanged by this delta (same Leg B (cold scoped-set open on a non-armed host) is still open and remains the maintainer's merge condition. |
There was a problem hiding this comment.
4 issues found across 13 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/platform-apple/src/core/__tests__/xcrun-shim-fixtures.ts">
<violation number="1" location="packages/platform-apple/src/core/__tests__/xcrun-shim-fixtures.ts:122">
P3: After `abortOf` resolves, the fake `runCommand` returns a successful `--find` result, so the losing branch of `probeWithinBudget`'s `Promise.race` keeps running and reads shim text and plists after the budget abort, mutating `host.plistReads` past the stall. The real executor rejects a timed-out/aborted `xcrun --find`, and `locateShim` swallows that into a `null` shim path. Return a failed result once the stall aborts so the ghost continuation stops at the stalled `--find`, matching the real executor and keeping `plistReads` empty for the stalled-probe test.</violation>
<violation number="2" location="packages/platform-apple/src/core/__tests__/xcrun-shim-fixtures.ts:122">
P3: `findStalls` does not guarantee a stall: with no `options.signal`, `abortOf` returns immediately and the fixture answers `xcrun --find` as if nothing was stalled; with a signal that never aborts, it waits forever and the suite hangs until the framework timeout. Bound the wait by the `timeoutMs` the probe already passes, using the abort signal only as an early exit.</violation>
</file>
<file name="packages/platform-apple/src/runner/runner-artifact.ts">
<violation number="1" location="packages/platform-apple/src/runner/runner-artifact.ts:475">
P2: This activates shim probing for every scoped-set runner build, but the artifact tests only exercise fake `xcrun` and plist providers. Add a macOS/Xcode-gated integration test covering a real non-armed shim and probe cancellation before relying on this safety path.</violation>
</file>
<file name="packages/platform-apple/src/runner/__tests__/runner-session-lifecycle.test.ts">
<violation number="1" location="packages/platform-apple/src/runner/__tests__/runner-session-lifecycle.test.ts:443">
P3: `vi.mocked(Date.now).mockRestore()` in this `finally` throws `TypeError: Date.now.mockRestore is not a function` whenever `mockEnsureXctestrunArtifact` was never invoked, because `vi.spyOn(Date, 'now')` lives inside that mock's implementation and no spy then exists. Today the build always runs before the acquire/refusal so it happens to be safe, but the PR's own goal is to refuse before any xcodebuild phase; the first reorder that achieves it turns a clean assertion failure into an unreadable crash. Guard the restore (or hoist the spy creation).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| const performanceBuildSettings = resolveRunnerPerformanceBuildSettings(); | ||
| const sandboxBuildArgs = resolveRunnerSandboxBuildArgs(); | ||
| await withXcodebuildSimulatorSetRedirect(device, async () => { | ||
| await withXcodebuildSimulatorSetRedirect(device, options.budget ?? {}, async () => { |
There was a problem hiding this comment.
P2: This activates shim probing for every scoped-set runner build, but the artifact tests only exercise fake xcrun and plist providers. Add a macOS/Xcode-gated integration test covering a real non-armed shim and probe cancellation before relying on this safety path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/runner/runner-artifact.ts, line 475:
<comment>This activates shim probing for every scoped-set runner build, but the artifact tests only exercise fake `xcrun` and plist providers. Add a macOS/Xcode-gated integration test covering a real non-armed shim and probe cancellation before relying on this safety path.</comment>
<file context>
@@ -472,8 +472,7 @@ async function buildRunnerXctestrun(
const sandboxBuildArgs = resolveRunnerSandboxBuildArgs();
- const redirectOptions = { signal: options.budget?.signal };
- await withXcodebuildSimulatorSetRedirect(device, redirectOptions, async () => {
+ await withXcodebuildSimulatorSetRedirect(device, options.budget ?? {}, async () => {
try {
await runCmdStreaming(
</file context>
| runCommand: async (cmd, args, options): Promise<ExecResult> => { | ||
| const tool = cmd === 'xcrun' && args[0] === '--find' ? args[1] : undefined; | ||
| if (tool !== undefined) host.finds.push(tool); | ||
| if (tool !== undefined && host.findStalls) await abortOf(options?.signal); |
There was a problem hiding this comment.
P3: After abortOf resolves, the fake runCommand returns a successful --find result, so the losing branch of probeWithinBudget's Promise.race keeps running and reads shim text and plists after the budget abort, mutating host.plistReads past the stall. The real executor rejects a timed-out/aborted xcrun --find, and locateShim swallows that into a null shim path. Return a failed result once the stall aborts so the ghost continuation stops at the stalled --find, matching the real executor and keeping plistReads empty for the stalled-probe test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/core/__tests__/xcrun-shim-fixtures.ts, line 122:
<comment>After `abortOf` resolves, the fake `runCommand` returns a successful `--find` result, so the losing branch of `probeWithinBudget`'s `Promise.race` keeps running and reads shim text and plists after the budget abort, mutating `host.plistReads` past the stall. The real executor rejects a timed-out/aborted `xcrun --find`, and `locateShim` swallows that into a `null` shim path. Return a failed result once the stall aborts so the ghost continuation stops at the stalled `--find`, matching the real executor and keeping `plistReads` empty for the stalled-probe test.</comment>
<file context>
@@ -112,9 +116,10 @@ export async function withFakeXcrunHost<T>(
+ runCommand: async (cmd, args, options): Promise<ExecResult> => {
const tool = cmd === 'xcrun' && args[0] === '--find' ? args[1] : undefined;
if (tool !== undefined) host.finds.push(tool);
+ if (tool !== undefined && host.findStalls) await abortOf(options?.signal);
const found = tool === undefined ? undefined : host.xcrunShimPaths[tool as XcrunShimToolName];
return found
</file context>
| if (tool !== undefined && host.findStalls) await abortOf(options?.signal); | |
| if (tool !== undefined && host.findStalls) { | |
| await abortOf(options?.signal); | |
| return { exitCode: 1, stdout: '', stderr: 'xcrun: error: aborted' }; | |
| } |
| error instanceof AppError && error.details?.reason === 'xctest_device_set_cleanup_armed', | ||
| ); | ||
| } finally { | ||
| vi.mocked(Date.now).mockRestore(); |
There was a problem hiding this comment.
P3: vi.mocked(Date.now).mockRestore() in this finally throws TypeError: Date.now.mockRestore is not a function whenever mockEnsureXctestrunArtifact was never invoked, because vi.spyOn(Date, 'now') lives inside that mock's implementation and no spy then exists. Today the build always runs before the acquire/refusal so it happens to be safe, but the PR's own goal is to refuse before any xcodebuild phase; the first reorder that achieves it turns a clean assertion failure into an unreadable crash. Guard the restore (or hoist the spy creation).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/runner/__tests__/runner-session-lifecycle.test.ts, line 443:
<comment>`vi.mocked(Date.now).mockRestore()` in this `finally` throws `TypeError: Date.now.mockRestore is not a function` whenever `mockEnsureXctestrunArtifact` was never invoked, because `vi.spyOn(Date, 'now')` lives inside that mock's implementation and no spy then exists. Today the build always runs before the acquire/refusal so it happens to be safe, but the PR's own goal is to refuse before any xcodebuild phase; the first reorder that achieves it turns a clean assertion failure into an unreadable crash. Guard the restore (or hoist the spy creation).</comment>
<file context>
@@ -403,33 +403,52 @@ test('an armed xcrun shim refuses a scoped-set session before the runner launche
+ error instanceof AppError && error.details?.reason === 'xctest_device_set_cleanup_armed',
+ );
+ } finally {
+ vi.mocked(Date.now).mockRestore();
+ }
</file context>
| vi.mocked(Date.now).mockRestore(); | |
| if (vi.isMockFunction(Date.now)) vi.mocked(Date.now).mockRestore(); |
| runCommand: async (cmd, args, options): Promise<ExecResult> => { | ||
| const tool = cmd === 'xcrun' && args[0] === '--find' ? args[1] : undefined; | ||
| if (tool !== undefined) host.finds.push(tool); | ||
| if (tool !== undefined && host.findStalls) await abortOf(options?.signal); |
There was a problem hiding this comment.
P3: findStalls does not guarantee a stall: with no options.signal, abortOf returns immediately and the fixture answers xcrun --find as if nothing was stalled; with a signal that never aborts, it waits forever and the suite hangs until the framework timeout. Bound the wait by the timeoutMs the probe already passes, using the abort signal only as an early exit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform-apple/src/core/__tests__/xcrun-shim-fixtures.ts, line 122:
<comment>`findStalls` does not guarantee a stall: with no `options.signal`, `abortOf` returns immediately and the fixture answers `xcrun --find` as if nothing was stalled; with a signal that never aborts, it waits forever and the suite hangs until the framework timeout. Bound the wait by the `timeoutMs` the probe already passes, using the abort signal only as an early exit.</comment>
<file context>
@@ -112,9 +116,10 @@ export async function withFakeXcrunHost<T>(
+ runCommand: async (cmd, args, options): Promise<ExecResult> => {
const tool = cmd === 'xcrun' && args[0] === '--find' ? args[1] : undefined;
if (tool !== undefined) host.finds.push(tool);
+ if (tool !== undefined && host.findStalls) await abortOf(options?.signal);
const found = tool === undefined ? undefined : host.xcrunShimPaths[tool as XcrunShimToolName];
return found
</file context>
|
This follows up on the review at 34ab803. The refusal logic looks right, but the live-validation gap it flagged is still open at 9573ece. Every cold scoped-set simulator build now runs probeXcrunShimFirstLaunchHooks before installDeviceSetRedirect, and the probe fails closed (https://github.com/callstack/agent-device/blob/9573ece/packages/platform-apple/src/runner/runner-device-set.ts#L126). If the probe misreads a healthy toolchain as armed, it refuses every scoped-set runner build, through shim_not_located, version_unreadable on a shim phrasing other than the captured 26.2 text, or probe_out_of_budget. No CI lane opens a scoped set, and the author confirms Leg B has not run on 9573ece or any earlier head, so a false positive here would take out every --ios-simulator-device-set user with nothing in CI to catch it. What this needs to satisfy: a cold Not blocking: runnerPhaseBudgetExhaustedError's message no longer fits both call sites now that the phase clock can also stop the probe mid-run, so it could use a message true for both, or let the probe path pass its own — take it or leave it. CI is green across all 21 checks, but no lane opens a --ios-simulator-device-set scoped set, so the probe and refusal route this PR changes has no CI coverage. The default-set smoke job exits through the !needsRedirect branch before the probe ever runs. I did not run the tests, the mutation script, or check:affected, so the author's gate output and the 5-of-5 mutant kills are taken as reported. I did not trace the new mid-probe cancel case in runner-device-set-cleanup-arming.test.ts (the onPlistRead hook) line by line, so its correctness rests on the author's description and the probe code I read. The shim text format beyond the captured Xcode 26.2 fixture is still unverified; only a live run on another toolchain would show it fails closed there too. No conflicts. What's left before this can merge is that live Leg B run at 9573ece, on a host that isn't armed: a cold scoped-set open that reaches simulator_set_redirect, then build-for-testing, then a successful test-without-building, with the device kept in the set afterward. |
|
Closing in favor of #2963 (CI green), which removes the XCTestDevices redirect instead of guarding it. |
1 participant
Summary
acquireXcodebuildSimulatorSetRedirectnow refuses to redirect~/Library/Developer/XCTestDevicesto a scoped simulator set when an xcrun shim (
simctl/devicectl) has an armed first-launch hook —its
EXPECTED_VERSIONdiffers from the installed CoreSimulator/CoreDevice version. An armed shimruns
xcodebuild -runFirstLaunchon every call, deleting every device inXCTestDevices; with theredirect installed that wipes the user's scoped set. The probe reads shim text and plist paths
(never hard-coded), failing closed on any unreadable value or a 2 s timeout. The refusal is
COMMAND_FAILEDwithdetails.reason: 'xctest_device_set_cleanup_armed', a static hint, anddetails.xcrunShims; both the build and session acquire paths refuse before anyxcodebuildphase.Default-set sims and a set already equal to
XCTestDevicesnever probe.Closes #2935
17 files touched (7 production, 8 test, 2 docs); net production +247 lines. Gross diff is 1,045
lines, 45 over budget — almost entirely the new fail-closed test matrix (318 lines) the issue
requires.
Validation
Tested at
9573ece558(base34ab80353c).pnpm check:affected --run: pass, first run — "all runnable checks passed" (510 vitest files /3899 tests, command-docs 12/12).
pnpm check:fallow --base origin/main: no issues in 22 changedfiles.
the owning phase's remaining deadline (
min(30s cold cap, phase remaining)), not just the 30 scold budget; a fake that missed an already-fired abort signal now waits on a real abort listener
and asserts the probe started; a new test covers a request canceled while a shim is mid-read
(previously only pre-aborted was covered);
probe_canceledis now unrepresentable at the probeboundary (
{ canceled: true } | { canceled: false; xcrunShims }) instead of a deadarmedBybranch.
misreported as an armed shim (
probe_out_of_budget); fixed so a phase-owned stop reportsrunner_phase_budget_exhaustedand only the 30 s cold cap can produceprobe_out_of_budget.xctest_device_set_cleanup_armed, probe ~50 ms, device survives every-runFirstLaunchcleanup.condition.
pipeline.