Skip to content

perf(ci): reuse iOS runner and narrow native cache key - #2952

Merged
thymikee merged 2 commits into
mainfrom
perf/ios-ci-reuse-native-cache
Sep 25, 2026
Merged

thymikee merged 2 commits into
mainfrom
perf/ios-ci-reuse-native-cache

Conversation

@thymikee

@thymikee thymikee commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Summary

Reuse the prepared XCTest runner across the Settings and gesture source-CLI replays, and in the manual iOS replay suite. Cache the native Xcode build before applying the current icon patch after every restore, so TypeScript-only changes do not force compilation or leave patched artifacts stale. Five files touched.

Validation

Head 47c60083bf13d2aa7d0eb124f6596daf26323678: pnpm format, focused Apple CI impact tests (8/8), pnpm check:gate-manifest, sh -n scripts/build-xcuitest-apple.sh, and pnpm check:affected --run passed. Exact-head iOS CI is queued; the new cache restore/patch path still needs live proof.

Prior head 8e70f7c: iOS smoke was green. Runner PID 36616 served Settings and gesture without a second xcodebuild; gesture passed first attempt in 12.7s, with open at 6.5s versus 33.6s in the earlier cold attempt. Its full gesture step took 22s versus 44s in a recent main run, a one-pair observation.

The manual dispatch passed preflight and all iOS replays with runner PID 15349. Its later full fixture E2E failed after the source/built daemon reset, in the historically parked lane; this does not validate that separate tier. Android passed.

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.83 MB 4.83 MB 0 B
Package (unpacked) 4.83 MB 4.83 MB 0 B
Package (download) 1.45 MB 1.45 MB +7 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.1 ms 26.8 ms +0.7 ms
CLI --help 79.6 ms 78.9 ms -0.7 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread .github/actions/setup-apple-runner-build/action.yml Outdated
@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at 8e70f7c.

.github/actions/setup-apple-runner-build/action.yml:47 drops packages/platform-apple/src/runner/** from the source-hash hashFiles() list entirely. scripts/build-xcuitest-apple.sh:168 (kept in the hash) unconditionally runs node scripts/patch-xcuitest-runner-icon.ts $DERIVED_PATH, which imports and runs applyXctestRunnerAppIconFromDerivedPath from packages/platform-apple/src/runner/runner-icon.ts — copying icon assets, patching CFBundleIcons, and re-codesigning the runner .app before the derived path is cached. That file is a real input to the cached artifact even though it never reaches xcodebuild directly. A behavior-changing edit to runner-icon.ts (codesigning fix, new plist key, bundle-name fix) will no longer change the cache key, so the next run reports a cache hit, skips the build+patch step, and silently restores the old, unpatched runner. Icon-dependent flows like SpringBoard icon-based launch/tap would keep running against stale runner state with no signal, on every Apple platform that shares this action. Keep packages/platform-apple/src/runner/runner-icon.ts (only) in the hashFiles() list instead of removing the whole packages/platform-apple/src/runner/** glob; host.ts and apple-runner-platform.ts are not build/patch inputs and can stay excluded.

Not blocking: scripts/__tests__/apple-ci-impact.test.ts:41 asserts cacheInputs(action).filter(...).toEqual([]), which locks in the bug above rather than guarding against it — once the hash list is fixed this should instead allow exactly packages/platform-apple/src/runner/runner-icon.ts and reject any other packages/ path — but this can be taken or left with the main fix.

The exact-head ios.yml "Smoke Tests" job (.github/workflows/ios.yml:63) directly covers what this PR touches — the cache key and the gesture-pan-duration clean:daemon removal — and was still in progress when this was written, so it isn't a failure yet, just not confirmed green. ci.yml:285 covers unrelated code and doesn't overlap. It would help to see that Smoke Tests run finish green, showing the reused daemon/runner works across Settings and gesture-pan-duration without the manual replay-ios gate masking a failure, and, once the hash-key fix lands, a run showing a runner-icon.ts change actually triggers a cache miss and rebuild. I didn't watch the run to completion or exercise the GitHub Actions cache mechanics directly — this is based on reading the composite action's steps — and I only confirmed the icon-patch call is unconditional in build-xcuitest-apple.sh, not whether other platform gates that share this action and script hit the same step.

@thymikee

Copy link
Copy Markdown
Member Author

Agreed. runner-icon.ts is a cached-artifact input, and following its host-bound calls showed that hashing that one file would still leave a maintenance trap. I changed the boundary in 47c60083bf: the action now caches the unpatched native DerivedData, then runs the current icon patch after every restore or build. The normal build:xcuitest:* path still patches as before. The structural test asserts the cache is saved before the unconditional patch, and the exact-head local affected gate passed. New iOS CI is queued at https://github.com/callstack/agent-device/actions/runs/36117868947; its cache/patch behavior is pending live proof.

The prior head's live iOS smoke passed with runner PID 36616 across Settings and gesture, and gesture passed first attempt. The manual replay suite also passed on PID 15349; the later full fixture E2E failed after its separate daemon reset, so I am not claiming that tier green. The PR body now attributes each result to its tested head.

@thymikee

Copy link
Copy Markdown
Member Author

This is ready for human review at 47c6008. The stale build-key issue from the earlier pass (8e70f7c, #2952 (comment)) is fixed. Not blocking: SKIP_ICON_PATCH's reordering leaves the icon patch step writing the build metadata before the icon patch touches the bundle, so a stale-manifest fallback to scan is possible there, but this can be taken or left.

Smoke Tests in ios.yml and Coverage in ci.yml both cover this change, but both were still running when I checked, so there's nothing to report from them yet. Please check they go green; no further code changes are needed.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 25, 2026
@thymikee
thymikee merged commit 76dbee0 into main Sep 25, 2026
18 of 19 checks passed
@thymikee
thymikee deleted the perf/ios-ci-reuse-native-cache branch September 25, 2026 09:39
@github-actions

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

@thymikee

Copy link
Copy Markdown
Member Author

Post-merge CI update:

  • Main iOS Smoke run 36119703847 passed at merge commit 76dbee0: 68 selected XCTests, Settings replay, gesture replay, and fixture E2E. It missed the new native key, built and saved DerivedData, then ran the icon patch.
  • PR run 36117868947, attempt 2 restored an exact native cache hit, skipped compilation, and ran the icon patch. It then failed one of 68 XCTests: testSynthesizedReplacementPacesAnAppOwnedFieldAtItsAcknowledgeWindow observed 11 app edits in 319 ms, below its 400 ms assertion. The downstream replay steps were skipped. Attempt 1 had passed XCTests but failed later during fixture E2E runner startup.

The successful main run used a newer native-source key, so it does not prove that the PR run's restored binary passes every XCTest. The failed assertion concerns text-entry pacing; these logs do not establish a cache defect. I updated the watcher to check the next main run for a hit on its new key and to track that failure signature separately.

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