Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 663d670 and 4f8b20f785031e2054b4814771345d25255f7be3. 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe macOS build script removes the Priority: ⬇️ Low 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description clearly explains the failure, the implementation, the reason for the approach, testing, and scope. However, the required Linked Issue section does not provide an issue link and states that no issue exists. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4f8b20f to
deb72c5
Compare
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The new single-command multi-arch build only resolves on Swift 6.4+. On the SwiftPM ≤ 6.3 toolchain this repo actually builds with (macos-15 = Xcode 16.x), more than one --arch silently switches SwiftPM to the Xcode build system, whose products land in .build/apple/Products/Release with no .build/release alias — so the unchanged read at binaryPath fails with ENOENT. This is precisely the untested case the PR body flags.
Reviewed changes
- One-shot multi-arch build: replaces the two per-architecture
swift build --tripleinvocations and thelipofuse with a singleswift build -c release --arch arm64 --arch x86_64, reading the result from.build/release/orca-computer-use-macos.
🚨 The fix's path contract holds only on Swift 6.4+, but the repo builds on Xcode 16.x
The change is correct on the toolchain it was tested against (Swift 6.4, where .swiftbuild is the default and maintains a .build/release symlink), but it is not engine-stable. Every macOS build path in this repo runs on a macos-15 image — computer-e2e.yml:148 (macos-15) and the release/daily/hourly/adhoc builds on blacksmith-6vcpu-macos-15 (release-mac-build.yml:30, daily-mac-build.yml:80, hourly-mac-build.yml:127, adhoc-mac-build.yml:99) — and mobile-ios-release.yml:30 records that macos-15 is "Xcode 16.x". On any of those, the new invocation fails and takes build:mac/build:mac:release down with it, which is the same failure class the PR set out to fix.
The previous --triple form left architectures empty, so it always used the native engine and its per-triple paths existed on both old and new toolchains; the regression is introduced by the switch to repeated --arch.
Technical details
# Multi-arch `swift build` is not engine-stable before Swift 6.4
## Affected sites
- `config/scripts/build-computer-macos.mjs:32-42` — the new `swift build ... --arch arm64 --arch x86_64` call.
- `config/scripts/build-computer-macos.mjs:7` — `binaryPath` hardcodes `.build/release/orca-computer-use-macos`.
- `config/scripts/build-computer-macos.mjs:26` — `chmodSync(binaryPath, 0o755)` is where the missing path surfaces (ENOENT).
## Evidence
- SwiftPM ≤ 6.3 (`swift-6.1-RELEASE`/`swift-6.2-RELEASE`/`swift-6.3-RELEASE`, `Sources/CoreCommands/Options.swift`):
`public var buildSystem { return self.architectures.count > 1 ? .xcode : self._buildSystem }`.
With >1 `--arch`, the Xcode engine is selected regardless of `--build-system`.
- `Sources/SPMBuildCore/Triple+Extensions.swift` (6.1): `buildSystem == .xcode ? "apple" : ...`, so
`dataPath = .build/apple`; `BuildParameters.buildPath` for xcode = `dataPath/Products/<Config>` =
`.build/apple/Products/Release`.
- `Sources/XCBuildSupport/XcodeBuildSystem.swift` (6.1) shells out to `xcodebuild --derivedDataPath
<dataPath>` and contains no `createSymbolicLink` call, so `.build/release` is never created.
- SwiftPM `main` (`Sources/CoreCommands/Options.swift`): default `_buildSystem = .swiftbuild`, and
`.native` + >1 arch is forced to `.swiftbuild`. `SwiftBuildSystem.build` creates `.build/release`
via `createBuildSymbolicLinks`, which is why the author's 6.4 test passes.
- `swift build --show-bin-path` reports the engine's real products dir, never `.build/release`
(`.build/out/Products/Release` on swiftbuild, `.build/apple/Products/Release` on xcode), so the
comment at lines 29-30 describes the symlink as if it were the bin path.
## Required outcome
- The helper path must be resolved for the engine SwiftPM actually selects, so the script works on
both the pre-6.4 Xcode engine and the 6.4+ swiftbuild engine.
## Suggested approach (optional)
- Capture `swift build -c release --package-path <pkg> --arch arm64 --arch x86_64 --show-bin-path`
and read `<binPath>/orca-computer-use-macos`; `--show-bin-path` works without building and is
engine-correct. A small helper that captures stdout is needed since `run()` uses `stdio: 'inherit'`.
- Or revert to two native `--triple` builds and keep the `lipo` fuse, which was engine-stable.
- Also update/remove the comment at lines 29-30 once the path is no longer hardcoded.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| run('swift', [ | ||
| 'build', | ||
| '-c', | ||
| 'release', | ||
| '--package-path', | ||
| packagePath, | ||
| '--arch', | ||
| 'arm64', | ||
| '--arch', | ||
| 'x86_64' | ||
| ]) |
There was a problem hiding this comment.
Repeated --arch is not equivalent to the old two-step build on SwiftPM ≤ 6.3: architectures.count > 1 forces the Xcode build system, which writes to .build/apple/Products/Release and never creates the .build/release alias, so chmodSync(binaryPath) at line 26 throws ENOENT on the macos-15 (Xcode 16.x) toolchain the repo builds with.
Technical details
# `.build/release` is not the Xcode engine's products dir
## Affected sites
- `config/scripts/build-computer-macos.mjs:32-42` — new multi-arch invocation.
- `config/scripts/build-computer-macos.mjs:7` — `binaryPath` assumes `.build/release`.
- `config/scripts/build-computer-macos.mjs:26` — `chmodSync(binaryPath, 0o755)` fails first.
## Evidence
- SwiftPM ≤ 6.3 `Sources/CoreCommands/Options.swift`:
`return self.architectures.count > 1 ? .xcode : self._buildSystem`.
- `Sources/SPMBuildCore/Triple+Extensions.swift`: xcode build system maps the scratch subdir to `"apple"`.
- `Sources/XCBuildSupport/XcodeBuildSystem.swift`: runs `xcodebuild --derivedDataPath <dataPath>`, no symlink creation.
- SwiftPM `main`: default `.swiftbuild`; `.native` + >1 arch forced to `.swiftbuild`, whose `build()`
creates `.build/release` via `createBuildSymbolicLinks` (why Swift 6.4 passes).
## Required outcome
- Resolve the product path for whichever engine SwiftPM selects, on both ≤6.3 and 6.4+.
## Suggested approach (optional)
- Read the directory from `swift build ... --show-bin-path` and append `orca-computer-use-macos`
(needs a stdout-capturing variant of `run()`), or restore per-`--triple` builds + `lipo`.
There was a problem hiding this comment.
ℹ️ No new changes to review —
deb72c56is a rebase of the previously reviewed tree onto a newermain(identical diff bytes to4f8b20f7), so the earlier review still stands.
Reviewed changes
- Rebase only: no file content changed since the last pullfrog review;
config/scripts/build-computer-macos.mjsis byte-identical.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
ae033d0 to
5253f4f
Compare
SwiftPM no longer writes per-triple output to .build/<triple>/release, so the two --triple builds left lipo with no inputs and the build failed on Swift 6.4. Repeated --arch produces a universal binary at the bin path directly.
5253f4f to
284f0a9
Compare
This branch has not been deployed
1 participant
ELI5
On a current Swift toolchain, building the macOS app fails. The step that fuses the Intel and Apple Silicon copies of the computer-use helper into one binary can't find them, because SwiftPM no longer puts them where that step looks.
What Changed
Before:
pnpm build:computer-macosfailed on Swift 6.4 with:The Swift builds themselves reported success, so the failure only surfaced at the fuse step, and it takes the whole
build:macdown with it.After: the helper builds and packages normally.
Mechanism:
config/scripts/build-computer-macos.mjsranswift build --triple <triple>once per architecture and then read each result from.build/<triple>/release/. SwiftPM's current default build system does not write per-triple output to those paths — it writes to.build/out/Products/Release— soliporeceived no inputs.The loop and the
lipocall are both gone. Oneswift build --arch arm64 --arch x86_64produces a universal binary directly at the package's bin path, which is where the rest of the script already expected it.Why
Repeated
--archis SwiftPM's own supported way to ask for a universal binary, and it behaves identically on the older build system, so this is not a version-gated fix.The alternative was
--build-system native, which restores the old layout and is what the existing paths assume. I rejected it: SwiftPM prints a deprecation warning for that flag and says it will be removed, so it would buy one release and need doing again. This version is also strictly less code — the per-architecture loop and thelipoinvocation both disappear.Linked Issue
No existing issue found for this. Happy to open one if you'd prefer the tracking.
Visual Proof
N/A — build tooling only, no user-visible or behavioral change.
Testing
macOS 26.5, Apple Silicon, Swift 6.4 (swiftlang-6.4.0.34.1).
rm -rf native/computer-use-macos/.buildto rule out cached artifactspnpm run build:computer-macos— succeeds where it previously failedlipo -archs "native/computer-use-macos/.build/release/Orca Computer Use.app/Contents/MacOS/orca-computer-use-macos"→x86_64 arm64codesign --verify --stricton the helper app → passeselectron-builderpackaging of a signed macOS build → completes, helper present and valid inside the bundleNot tested on an older Swift toolchain; the flag is long-supported, but a second pair of eyes on that would be welcome.
No test added: the failure is a property of the installed Swift toolchain's output layout, which a unit test cannot observe without invoking a real
swift build.AI Disclosure
Written with Claude (Opus 5) via Claude Code. Diagnosis, the fix and all verification steps above were run and reviewed by me.
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
macOS-only build script; no runtime, security, SSH, mobile or compatibility surface. The produced binary is byte-equivalent in architecture coverage to what the previous two-step build produced.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)