perf(ci): verify restored Apple runner cache with Xcode - #2964
Conversation
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 2 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=".github/actions/setup-apple-runner-build/action.yml">
<violation number="1" location=".github/actions/setup-apple-runner-build/action.yml:90">
P2: The `restore-keys` prefix restores DerivedData built from any earlier source hash, so the 'compatible' cache can come from a tree where native files were deleted, renamed, or the Xcode project/scheme was restructured. The Verify step then only runs an incremental `xcodebuild`, and the isolation scan explicitly only covers the files that recompiled ('Isolation scan covers only the files this build recompiled'); stale products can survive in `Build/Products` and get re-saved under the current key as a 'cached current build'. Bound the fallback instead (for example only allow restoring when the source-hash delta is known-buildable, or break the build on an IBM/clean rebuild when `steps.source-hash` differs from the metadata `write-xcuitest-cache-metadata.mjs` emits), or document the stale-artifact risk the 'verified compatible' message currently hides.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| key: ${{ inputs.cache-key-prefix }}-${{ steps.xcode.outputs.key }}${{ inputs.cache-key-suffix }}-${{ steps.build-variant.outputs.key }}-${{ steps.source-hash.outputs.value }} | ||
| key: ${{ inputs.cache-key-prefix }}-${{ steps.xcode.outputs.key }}${{ inputs.cache-key-suffix }}-${{ steps.build-variant.outputs.key }}-${{ steps.cache-schema.outputs.value }}-${{ steps.source-hash.outputs.value }} | ||
| restore-keys: | | ||
| ${{ inputs.cache-key-prefix }}-${{ steps.xcode.outputs.key }}${{ inputs.cache-key-suffix }}-${{ steps.build-variant.outputs.key }}-${{ steps.cache-schema.outputs.value }}- |
There was a problem hiding this comment.
P2: The restore-keys prefix restores DerivedData built from any earlier source hash, so the 'compatible' cache can come from a tree where native files were deleted, renamed, or the Xcode project/scheme was restructured. The Verify step then only runs an incremental xcodebuild, and the isolation scan explicitly only covers the files that recompiled ('Isolation scan covers only the files this build recompiled'); stale products can survive in Build/Products and get re-saved under the current key as a 'cached current build'. Bound the fallback instead (for example only allow restoring when the source-hash delta is known-buildable, or break the build on an IBM/clean rebuild when steps.source-hash differs from the metadata write-xcuitest-cache-metadata.mjs emits), or document the stale-artifact risk the 'verified compatible' message currently hides.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/actions/setup-apple-runner-build/action.yml, line 90:
<comment>The `restore-keys` prefix restores DerivedData built from any earlier source hash, so the 'compatible' cache can come from a tree where native files were deleted, renamed, or the Xcode project/scheme was restructured. The Verify step then only runs an incremental `xcodebuild`, and the isolation scan explicitly only covers the files that recompiled ('Isolation scan covers only the files this build recompiled'); stale products can survive in `Build/Products` and get re-saved under the current key as a 'cached current build'. Bound the fallback instead (for example only allow restoring when the source-hash delta is known-buildable, or break the build on an IBM/clean rebuild when `steps.source-hash` differs from the metadata `write-xcuitest-cache-metadata.mjs` emits), or document the stale-artifact risk the 'verified compatible' message currently hides.</comment>
<file context>
@@ -79,10 +85,11 @@ runs:
- key: ${{ inputs.cache-key-prefix }}-${{ steps.xcode.outputs.key }}${{ inputs.cache-key-suffix }}-${{ steps.build-variant.outputs.key }}-${{ steps.source-hash.outputs.value }}
+ key: ${{ inputs.cache-key-prefix }}-${{ steps.xcode.outputs.key }}${{ inputs.cache-key-suffix }}-${{ steps.build-variant.outputs.key }}-${{ steps.cache-schema.outputs.value }}-${{ steps.source-hash.outputs.value }}
+ restore-keys: |
+ ${{ inputs.cache-key-prefix }}-${{ steps.xcode.outputs.key }}${{ inputs.cache-key-suffix }}-${{ steps.build-variant.outputs.key }}-${{ steps.cache-schema.outputs.value }}-
- - name: Build Apple runner artifacts on cache miss
</file context>
There was a problem hiding this comment.
Fixed in 840486d. I removed the broad restore-keys fallback, so a native source change gets a cold build and cannot carry stale products forward. Exact-source restores still run Xcode before use; the cache is saved before the current icon patch. The rebased exact-head local gate passed; new live CI is pending.
3181f82 to
840486d
Compare
There was a problem hiding this comment.
1 issue found across 2 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="scripts/__tests__/apple-ci-impact.test.ts">
<violation number="1" location="scripts/__tests__/apple-ci-impact.test.ts:98">
P2: This refactored test pins the cache-schema half of the restore key but never asserts the key contains `steps.source-hash.outputs.value`. The PR's core guarantee (commit 840486d8: a native source change must produce a cold build) depends entirely on that key segment; if it were accidentally dropped from `action.yml`, this test would still pass while stale native products get restored. Add an assertion on the same key.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| 'scripts/build-xcuitest-apple.sh', | ||
| ]); | ||
| expect(steps[restoreIndex]?.with?.key).toContain('steps.cache-schema.outputs.value'); | ||
| expect(steps[restoreIndex]?.with?.['restore-keys']).toBeUndefined(); |
There was a problem hiding this comment.
P2: This refactored test pins the cache-schema half of the restore key but never asserts the key contains steps.source-hash.outputs.value. The PR's core guarantee (commit 840486d: a native source change must produce a cold build) depends entirely on that key segment; if it were accidentally dropped from action.yml, this test would still pass while stale native products get restored. Add an assertion on the same key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/__tests__/apple-ci-impact.test.ts, line 98:
<comment>This refactored test pins the cache-schema half of the restore key but never asserts the key contains `steps.source-hash.outputs.value`. The PR's core guarantee (commit 840486d8: a native source change must produce a cold build) depends entirely on that key segment; if it were accidentally dropped from `action.yml`, this test would still pass while stale native products get restored. Add an assertion on the same key.</comment>
<file context>
@@ -82,14 +83,19 @@ function appleRunnerBuildAction(): { text: string; steps: AppleRunnerBuildStep[]
+ ]);
expect(steps[restoreIndex]?.with?.key).toContain('steps.cache-schema.outputs.value');
- expect(steps[restoreIndex]?.with?.['restore-keys']).toContain('steps.cache-schema.outputs.value');
+ expect(steps[restoreIndex]?.with?.['restore-keys']).toBeUndefined();
expect(cacheInputs(text)).not.toContain('scripts/patch-xcuitest-runner-icon.ts');
});
</file context>
|
The change looks right at 840486d. Restored DerivedData is used only on an exact native source hash, the key includes the action and build script, and the icon patch runs after the save, so the saved products stay unpatched. The test in Smoke Tests is still running. It runs the changed Not blocking: does the |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Restore Apple runner DerivedData only for an exact native source hash, then let Xcode verify it on every CI run. The action and build script form an automatic cache schema; the current icon patch runs after saving unpatched products. This removes the hand-maintained TypeScript invalidation list. The exact-source constraint prevents a prior native tree from leaving stale products in the new cache.
Two CI files changed. The installed package's validated runner-artifact cache remains unchanged: repeat launches skip Xcode entirely. PRs #688 and #900 established that path; #2303 and #2952 introduced the separate CI cache.
Validation
Head
840486d807: planted-red action test, 9/9 focused tests,actionlintfor all six consuming workflows, andpnpm check:affected --runpassed on the rebased head. Prior head3181f828d3passed all PR checks; its first iOS run had a cache miss, built/saved unpatched artifacts, applied the icon patch, and passed smoke (47s setup step, 17m16s runner).Local Xcode 26.2: cold 19.9s, warm 9.7s. Fresh checkout timestamps recompiled 145 Swift units in 9.1s, so CI speed remains unproven. The new head needs an exact-key cache-hit run for measured timing.