Proxy Bun downloads through CocoaPods cache on iOS#3619
Proxy Bun downloads through CocoaPods cache on iOS#3619
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3619 +/- ##
==========================================
+ Coverage 54.86% 54.91% +0.05%
==========================================
Files 836 836
Lines 35903 35955 +52
Branches 7494 7504 +10
==========================================
+ Hits 19694 19740 +46
- Misses 16114 16120 +6
Partials 95 95 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⏩ The changelog entry check has been skipped since the "no changelog" label is present. |
There was a problem hiding this comment.
Pull request overview
Routes Bun installation network traffic through the CocoaPods cache proxy (when configured) to improve download performance/reliability on iOS workers, with fallbacks and unit coverage to validate the proxy + retry behavior.
Changes:
- Proxy Bun install script download (
bun.sh/install) via CocoaPods cache URL, with direct-download retry on failure. - Proxy Bun GitHub release ZIP downloads by injecting a proxied
GITHUBenv var into the install script run, with direct-install retry on failure. - Add unit tests covering proxied success and both fallback paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/worker/src/runtimeEnvironment.ts | Adds proxy URL rewriting + retry logic for Bun install script download and GitHub release downloads. |
| packages/worker/src/unit/runtimeEnvironment.test.ts | Adds unit tests validating proxy behavior and fallback retries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const proxyUrl = config.cocoapodsCacheUrl; | ||
| try { | ||
| ctx.logger.info(`Installing bun@${versionToInstall}`); | ||
|
|
||
| const bunInstallScriptPath = path.join(os.tmpdir(), `install-bun-${uuidv4()}.sh`); | ||
| const proxiedBunInstallScriptUrl = proxyUrl | ||
| ? rewriteUrlThroughProxy('https://bun.sh/install', proxyUrl) | ||
| : null; |
There was a problem hiding this comment.
installBun enables CocoaPods-cache proxying whenever config.cocoapodsCacheUrl is set, without checking that the current job/runner is actually iOS (e.g. process.platform === 'darwin' or ctx.job.platform === Platform.IOS). This doesn’t match the PR intent (“on iOS”) and can add extra failing requests + warnings on non-iOS workers if the config is ever set globally. Consider gating proxy behavior to iOS/darwin explicitly.
|
|
||
| function rewriteUrlThroughProxy(url: string, proxy: string): string { | ||
| const parsedUrl = new URL(url); | ||
| return url.replace(`${parsedUrl.protocol}//${parsedUrl.host}`, `${proxy}/${parsedUrl.host}`); |
There was a problem hiding this comment.
rewriteUrlThroughProxy can generate URLs with a double-slash path segment when proxy ends with / (e.g. https://cache/ -> https://cache//github.com). Some caching/proxy layers treat these as different paths. Consider normalizing proxy (trim trailing slashes) and building the rewritten URL via URL components rather than string.replace to avoid subtle formatting issues.
| return url.replace(`${parsedUrl.protocol}//${parsedUrl.host}`, `${proxy}/${parsedUrl.host}`); | |
| const proxyUrl = new URL(proxy); | |
| const normalizedProxyPath = proxyUrl.pathname.replace(/\/+$/, ''); | |
| proxyUrl.pathname = `${normalizedProxyPath}/${parsedUrl.host}${parsedUrl.pathname}`.replace( | |
| /\/{2,}/g, | |
| '/' | |
| ); | |
| proxyUrl.search = parsedUrl.search; | |
| proxyUrl.hash = parsedUrl.hash; | |
| return proxyUrl.toString(); |
| it('proxies Bun install script and GitHub release ZIP downloads through CocoaPods cache on iOS', async () => { | ||
| let isFirstTimeCheckingBunVersion = true; | ||
| config.cocoapodsCacheUrl = 'https://cocoapods-cache.example.com'; | ||
|
|
||
| jest.mocked(spawn).mockImplementation((cmd, _args, _opts) => { | ||
| if (cmd === 'bun') { | ||
| const stdout = isFirstTimeCheckingBunVersion ? '1.0.0' : '2.0.0'; | ||
| isFirstTimeCheckingBunVersion = false; | ||
| return Promise.resolve({ | ||
| ...spawnResult, | ||
| stdout, | ||
| }); | ||
| } | ||
| return Promise.resolve(spawnResult); | ||
| }); | ||
|
|
||
| await prepareRuntimeEnvironment(ctx, { bun: '2.0.0' }, false); | ||
|
|
There was a problem hiding this comment.
This test asserts proxying “on iOS”, but it never sets ctx.job.platform (or otherwise constrains the runtime to iOS/darwin). As a result it would still pass even if the proxying applied to Android/Linux too. Setting ctx.job.platform to iOS (and ideally adding a negative test for non-iOS) would make the coverage reflect the intended behavior.
Summary
Testing
yarn test:unit runtimeEnvironment.test.ts --runInBandinpackages/worker