fix(apple-runner): never compare an unavailable toolchain probe; name the mismatching cache keys - #2306
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Sentinel review at 05c7ac0. P2: Do not memoize Xcode output before validating its fingerprint. In runner-toolchain-fingerprint.ts, runToolchainProbe caches every nonempty zero-exit answer before parseXcodeVersionOutput can classify it as unparsable. A malformed answer therefore remains cached: after the host becomes healthy, the next request still throws apple_toolchain_probe_unavailable without rerunning xcodebuild. Exact-source fake-host reproduction: both attempts fail; the total probe-call count stays at three. Memoize only a successfully parsed result and add a malformed-output → healthy-output recovery test within the same process (without resetting the memo). The current recovery test only covers a thrown timeout. Separately, the body says #2303 already hashes runner-toolchain-fingerprint.ts, but its reviewed head does not; correct that dependency note. |
05c7ac0 to
fa75e73
Compare
|
Re-reviewed fa75e73 with independent review. The existing P2 remains after the move into runner-cache-metadata.ts: runToolchainProbe (201-211) memoizes nonempty zero-exit Xcode output before parseXcodeVersionOutput (236-251) validates it. A transient malformed answer stays cached even after the host recovers. Cache the parsed successful fingerprint, and prove malformed-output to healthy-output recovery in one process without resetting the memo; the current recovery case only covers a thrown timeout. No additional code finding. Hold readiness; exact-head native and remaining CI evidence is separate. |
A timed-out or failed `xcodebuild -version` / `xcrun --show-sdk-*` probe used to fall back to the literal `unknown`, which was memoized for the process and then persisted into the rebuilt cache's metadata, so every later daemon on a healthy host mismatched again and paid a full build-for-testing. Unavailability is now a distinct outcome with no comparable value: only successful probes are memoized, an unreadable toolchain fails the cache decision with a retriable typed error naming the probe that could not answer, and the CI metadata writer refuses to persist a probe it could not read. The cache_metadata_mismatch diagnostic now lists the differing keys with expected and actual values instead of only saying the metadata differed. The runner-source fingerprint moves to the module that owns the runner's source roots, keeping the cache-metadata module within its size budget without adding a module to the Apple facades' eager closure.
fa75e73 to
cca16c5
Compare
…cision glue Tests mirror source topology: the runner-source fingerprint tests move with the function into runner-source.test.ts and call it directly instead of reaching it through resolveExpectedRunnerCacheMetadata. The rebuild diagnostic's mismatch details move next to the cache state that carries them, so runner-artifact.ts — already past the 500-line extract threshold — gains no behavior.
runToolchainProbe cached every nonempty zero-exit answer before parseXcodeVersionOutput could classify it, so a transient malformed xcodebuild answer stayed cached and every later cache decision in the process kept failing after the host recovered. The memo now holds the complete parsed fingerprint per SDK, written only after all three probes answered and parsed; a failed round keeps nothing, so the next request re-probes. Tests cover malformed-to-healthy recovery in one process without resetting the memo, and that a partial round is not kept.
|
Addressed in d6a89f5. The memo no longer holds raw probe output: |
* origin/main: perf: bundle runtime dependencies and report full install size (#2310)
|
Sentinel review at 4ffd542: Source review clean, independently confirmed. Complete parsed fingerprints are now memoized only after all probes succeed; same-process malformed-to-healthy and partial-round recovery tests close the previous finding. Size CI is blocked by the cross-version report schema (old reporter reads missing base js.rawBytes), not measured size growth. Exact-head iOS was still running at review. Clear those CI blockers before merge; do not suppress the size check. |
|
1 participant
Summary
On a loaded host the Apple toolchain probes (
xcodebuild -version,xcrun --show-sdk-version,--show-sdk-build-version) time out. The probe answeredunknown, memoized it, and persisted it into the rebuilt cache, so the next healthy daemon mismatched and paid a fullbuild-for-testinginside the firstopen.Unavailability is now a distinct outcome with no comparable value: only a complete, parsed fingerprint is memoized (per SDK, after all three probes answered and parsed — a failed or malformed round keeps nothing, so the next request re-probes), and an unreadable toolchain fails the cache decision with a typed
COMMAND_FAILED(reason: apple_toolchain_probe_unavailable,retriable: true) naming it. Skipping the field would rebuild anyway — the fingerprint names the derived-data directory — and risks another Xcode's cache; AGENTS.md keeps optional probes best-effort "only when the feature contract says so."write-xcuitest-cache-metadata.mjsrefuses an unread probe too.cache_metadata_mismatchnow names the differing keys.Validation
Head
4ffd542234(d6a89f5e33parsed-fingerprint memo + oxfmt-only4ffd542234;9f68e00641for everything before it):check:quick,format:check,check:layering, fallow, gate-manifest, depgraph, the eager-closure and test-file-size gates, andvitest --project apple-runner packages/platform-apple/src/runner/__tests__(350 passed; onerunner-clientabort test timed out under host load 20+ and passed twice in isolation).Planted red, same vitest command:
unknownfallback restored → 6 failed (expected [Function] to throw an error).+ [] - [{ key: 'runnerSandboxBuildArgs', ... }]).runner-source.test.ts.9f68e00641'srunToolchainProbe) againstd6a89f5e33's tests → 2 failed: the malformed-to-healthy recovery in one process, and the partial-round-not-kept check.Device evidence: the iOS lane was green on
cca16c55f0, re-running on9f68e00641. Android smoke failed there in the alert-dismiss step; Apple-only diff, main green — flake pending rerun. Affected gate: pending.Tradeoffs
Invalidates the prebuilt runner cache once. The source fingerprint and its tests moved to
runner-source.ts; the mismatch diagnostic sits inrunner-cache.ts, keeping over-budgetrunner-artifact.tsbehavior-free.