Skip to content

fix(ios): pay for one toolchain in the snapshot bridge identity read (#2712) - #2765

Merged
thymikee merged 2 commits into
mainfrom
fix/ios-snapshot-identity-single-cold-probe
Sep 23, 2026
Merged

thymikee merged 2 commits into
mainfrom
fix/ios-snapshot-identity-single-cold-probe

Conversation

@thymikee

@thymikee thymikee commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Summary

verify-installed-snapshot-bridge died on xcrun --sdk iphonesimulator --show-sdk-version, twice at the 30 s probe ceiling, and a cache-key field took the iOS smoke job down.

Triage disagreed with the reported cause. That script ran 11:15:44.964 → 11:16:45.862 (60.9 s) against a 120 s deadline, and the identity read runs before the cache lock, so remainingMs cannot reach 0 and the retry cannot have been skipped. The only reading that fits is two 30 s attempts on xcrun: the retry fired and stalled too — #2422's retry only recovers a stall that finished while the killed exec was going away.

The fix is at SnapshotSourceToolchainIdentity. The Simulator SDK ships inside the selected Xcode.app, so the version and build xcodebuild -version reports already pin it: the field left the key and the exec left the identity read. xcrun clang in the build pays that wait once, under the build's own ceiling, where it fits instead of exhausting the budget. A stall now reports toolchain-probe-stalled / native-build-stalled with the command, each attempt's budget and the exec kill as cause, instead of an unattributed xcrun timed out after 30000ms that reads like a device failure. 6 files.

Cache keys change, so existing entries rebuild once; old manifests hash differently and cannot be misread, so no schema bump. Untouched: the sibling runner prober's policy, and the generation circuit a preparation stall still opens.

Validation

4b1cc67ff: pnpm check:affected --run (all runnable gates, 1712 tests), check:fallow, check:layering, check:bundle-owner-files, build, lint, typecheck clean. Six tests fail with the two source files reverted to the merge-base.

Host run the review asked for (macOS 27.0, Xcode 27.0): prepared from source, exit 0 in 1.9 s, second run reused the entry.

$ pnpm --filter @agent-device/platform-apple run \
    verify-installed-snapshot-bridge <checkout> /tmp/sb3
"toolchain": { "xcode": "Xcode 27.0\nBuild version 27A5252f", "macosBuild": "26A428",
  "macosProductVersion": "27.0", "architecture": "arm64",
  "simulatorRuntime": "installed-package-verification" }
$ xcrun --sdk iphonesimulator --show-sdk-version   # the dropped field, on this host
27.0

A cold stall cannot be reproduced on demand; the attribution's real proof is the next cold CI host. ios.yml filters on this directory, so this PR runs the job it fixes.

The identity read probed `xcodebuild -version` and then `xcrun --sdk iphonesimulator
--show-sdk-version` — two Xcode-owned binaries, each a tool the host can stall on before
it answers. On CI the `xcrun` probe lost twice at the shared 30 s ceiling and took the
whole iOS smoke job down over a cache-key field (60.9 s of a 120 s budget, job
106068926059).

The Simulator SDK ships inside the selected `Xcode.app`, so the version and build
`xcodebuild -version` already reports pin it: the field left the cache key and the exec
left the identity read. The `xcrun clang` the build runs pays that wait once, under the
build's own ceiling, where a stall of that length fits instead of exhausting the budget.

A probe or compile that cannot answer now says so at the seam that waited: `timeout` with
`toolchain-probe-stalled` or `native-build-stalled`, the command, the budget each attempt
was armed with, and the exec layer's kill kept as the cause. Before, a job got a bare
`xcrun timed out after 30000ms` stack that reads exactly like a device failure.

Cache entries written before this hash to a different key, so they rebuild once instead of
being misread; no schema bump is needed to prove that.

Closes #2712
@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.80 MB 4.80 MB +639 B
Package (unpacked) 4.80 MB 4.80 MB +639 B
Package (download) 1.43 MB 1.43 MB +227 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.5 ms 26.4 ms -0.1 ms
CLI --help 77.4 ms 76.5 ms -0.9 ms

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at 6f458ca: the code looks good. Dropping the Simulator SDK version from the cache key is sound, because the selected Xcode version and build already pin that SDK, and old manifests cannot match the new key. The retry, deadline and cancellation behavior is unchanged, and a cancel cannot turn into native-build-stalled.

One small note, not blocking: the compile-stall test builds the timeout error by hand instead of through exec.ts (https://github.com/callstack/agent-device/blob/6f458ca/packages/platform-apple/src/snapshot-source/cache.test.ts#L289), so it would not catch a later change to that error's shape.

I did not see the output of the reported host run. A cold xcrun stall cannot be reproduced on demand, so the real proof comes when CI hits a cold host.

Two Smoke Tests jobs are still running. The iOS one runs the snapshot bridge preparation step that this PR changes, so it should finish green before merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 22, 2026
Both stall fixtures built the timeout error by hand, so a probe could keep classifying a
kill `exec.ts` no longer reports as one. They now run a command the exec layer really
kills and carry whatever error it raises; the probe assertions still read the shape
through `isCommandTimeoutError` rather than a message.

Review note on #2765.
@thymikee

Copy link
Copy Markdown
Member Author

Thanks — fixed at 4b1cc67ff, at the fixture rule rather than the one line.

__tests__/exec-timeout-fixture.ts now runs a command the exec layer really kills at its timeoutMs, and both stall fixtures throw whatever exec.ts raises. blockForWholeTimeout keeps advancing the fake clock by the budget the probe was handed, so the deadline arithmetic stays deterministic while the error stops being hand-shaped. cache.test.ts's compile fixture does the same. Both sides still assert through isCommandTimeoutError(error.cause), never a message.

Host run output added to the Validation section. Noted that a cold stall cannot be summoned: the attribution's proof is the next cold host, and it should now read toolchain-probe-stalled rather than a bare xcrun timed out after 30000ms.

Smoke Tests: all green on 6f458ca34; running on this head.

@thymikee

Copy link
Copy Markdown
Member Author

CI on 4b1cc67ff: all checks green (17 pass), and the iOS Smoke Tests job ran the step that was failing — pnpm check:package -- --verify-snapshot-bridge-preparation — and logged Prepared the Simulator snapshot bridge from the clean-installed package. (job 106921424531). Linux/macOS/Android smoke jobs do not enable that flag, so they are unchanged.

@thymikee

Copy link
Copy Markdown
Member Author

4b1cc67 looks good. The delta since 6f458ca is test-only: the fixture now takes its timeout error from the real exec.ts kill path instead of a synthetic one, so the snapshot bridge identity timeout test exercises the same failure the runner produces. No production code changed, and I found nothing left to fix.

CI is green on all 18 checks, including the smoke-test job that runs --verify-snapshot-bridge-preparation, which covers this path.

@thymikee
thymikee merged commit c32a09c into main Sep 23, 2026
18 checks passed
@thymikee
thymikee deleted the fix/ios-snapshot-identity-single-cold-probe branch September 23, 2026 05:39
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-23 05:40 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant