Conversation
- Update MockVM.run() signature to include networkMode and clipboard parameters added to the VM base class - Update VMDetailsPrinterTests to expect the network column in table output
…working Restructure the lume release artifact from a standalone CLI binary into a macOS .app bundle so that a provisioning profile can be loaded by the OS, enabling the com.apple.vm.networking restricted entitlement for bridged networking support in release builds. Closes #1076
…ments flag, show errors
The lume_lume.bundle is a flat SPM resource directory (no Info.plist), not a proper macOS bundle. codesign was failing with "bundle format unrecognized" which caused silent fallback to adhoc signing. Fix: use --deep on the .app bundle so codesign handles nested code automatically and seals flat resource directories properly.
The lume_lume.bundle is a flat SPM resource directory without Info.plist. When placed in Contents/MacOS/, codesign fails with "bundle format unrecognized" and silently falls back to adhoc signing. Move it to Contents/Resources/ only, which codesign seals as data.
…urce bundle fix Move lume_lume.bundle to Contents/Resources/ instead of Contents/MacOS/ to avoid codesign "bundle format unrecognized" errors. Also fix --entitlement -> --entitlements typo in build-release.sh.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📦 Publishable packages changed
Add |
📝 WalkthroughWalkthroughImplements macOS .app bundle distribution with provisioning profile signing and enhanced notarization workflow to support the Changes
Sequence Diagram(s)sequenceDiagram
participant Build as Build System
participant Sign as Codesigner
participant Profile as Provisioning Profile
participant Notary as Apple Notarization
participant Staple as Stapling Service
participant Archive as Archive Creator
Build->>Build: Assemble .app bundle<br/>(binary + resources)
Build->>Sign: Sign app bundle<br/>(Developer ID)
Sign->>Profile: Embed provisioning profile
Profile->>Sign: Profile embedded
Sign->>Build: Bundle signed
Build->>Notary: Submit .pkg for notarization
Notary->>Notary: Scan & validate
Notary->>Build: Return ticket ID
Build->>Staple: Staple ticket to .pkg
Staple->>Build: Stapled
Build->>Archive: Create version-stamped<br/>tar.gz + checksums
Archive->>Archive: Generate SHA256
Archive->>Build: Release artifacts ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
…esolution SPM's auto-generated Bundle.module looks up resources via Bundle.main.bundleURL (the .app root), NOT Bundle.main.resourceURL (Contents/Resources/). Placing lume_lume.bundle in Contents/Resources/ would cause a fatal crash at runtime when Bundle.module tries to load it. Move the resource bundle to the .app root level across all three build scripts (build-release-notarized.sh, build-release.sh, install-local.sh). This keeps it out of Contents/MacOS/ (which breaks codesign) while ensuring SPM can find it at runtime. Also adds *.provisionprofile to .gitignore.
📦 Publishable packages changed
Add |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/cd-swift-lume.yml (1)
78-83:⚠️ Potential issue | 🟡 MinorMinor: Duplicate condition check.
Lines 78-80 and 81-83 both check
inputs.versionwith identical logic. The second branch is unreachable.Proposed fix
elif [[ -n "${{ inputs.version }}" ]]; then VERSION="${{ inputs.version }}" echo "Using version from input: $VERSION" - elif [[ -n "${{ inputs.version }}" ]]; then - VERSION="${{ inputs.version }}" - echo "Using version from workflow_call input: $VERSION" else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cd-swift-lume.yml around lines 78 - 83, There is a duplicated elif block that checks [[ -n "${{ inputs.version }}" ]] and sets VERSION/echos the same message; remove the second redundant elif (the duplicate conditional and its echo) so the VERSION assignment only occurs once, keeping the unique conditional that sets VERSION and echoes "Using version from input: $VERSION".
🧹 Nitpick comments (1)
libs/lume/scripts/build/build-release.sh (1)
54-57: Usedittoinstead ofcp -Rfor copying the signed.appbundle.
dittopreserves extended attributes, resource forks, and HFS metadata required for macOS code signatures. Usingcp -Rrisks silently dropping signature-critical metadata (particularly xattrs), which can invalidate the bundle's signature.Proposed fix
# Install .app bundle rm -rf "$APP_INSTALL_DIR/lume.app" -cp -R ".release/lume.app" "$APP_INSTALL_DIR/" +ditto ".release/lume.app" "$APP_INSTALL_DIR/lume.app"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/lume/scripts/build/build-release.sh` around lines 54 - 57, Replace the cp -R call when installing the signed .app with macOS-aware ditto to preserve extended attributes and resource forks: after removing the old bundle (rm -rf "$APP_INSTALL_DIR/lume.app"), use ditto ".release/lume.app" "$APP_INSTALL_DIR/lume.app" (and keep the same variables APP_INSTALL_DIR and .release/lume.app references); also ensure the script checks ditto exit status and fails if ditto returns non-zero so signature-preserving copy errors are not ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/lume/scripts/build/build-release-notarized.sh`:
- Around line 109-111: The shell invocation that rebuilds the user keychain list
uses an unquoted command substitution: security list-keychains -d user -s
"$KEYCHAIN_PATH" $(security list-keychains -d user | tr -d '"'); to avoid
word-splitting when keychain paths contain spaces, wrap the command substitution
in quotes (i.e., use "$(security list-keychains -d user | tr -d '"')" ) so the
entire output is treated as a single argument; update the call around the
existing security/list-keychains usage and keep the existing KEYCHAIN_PATH
variable usage intact.
- Line 236: The script currently runs cd "$RELEASE_DIR" without checking its
result; update the code around the cd "$RELEASE_DIR" invocation to detect
failure and abort safely—after attempting to change directory, test the exit
status and if cd failed print a clear error to stderr including the $RELEASE_DIR
value and exit with a nonzero status (or use a safe shell construct that
achieves the same), so subsequent commands won't run in the wrong directory.
In `@libs/lume/scripts/build/build-release.sh`:
- Around line 14-16: The script reuses APP_BUNDLE (variable
APP_BUNDLE=".release/lume.app") and can carry stale files between runs; update
build-release.sh to remove the existing bundle before recreating it: when
APP_BUNDLE is set, delete or rm -rf the existing "$APP_BUNDLE" (and any related
Contents dir) prior to the mkdir -p "$APP_BUNDLE/Contents/MacOS" step and apply
the same cleanup before the later assembly steps around the block referenced at
lines 30-33 so the release is built from a clean slate.
- Around line 35-37: The final codesign invocation is missing the --deep flag so
the nested lume_lume.bundle placed at the app root may remain unsigned; update
the second codesign call that signs "$APP_BUNDLE" to include --deep (and keep
the entitlements on the main executable call), then add a verification step
after signing that uses codesign --verify (with --deep/--strict/verbose flags)
against "$APP_BUNDLE" to confirm the nested bundle (lume_lume.bundle) and all
nested code are properly signed.
---
Outside diff comments:
In @.github/workflows/cd-swift-lume.yml:
- Around line 78-83: There is a duplicated elif block that checks [[ -n "${{
inputs.version }}" ]] and sets VERSION/echos the same message; remove the second
redundant elif (the duplicate conditional and its echo) so the VERSION
assignment only occurs once, keeping the unique conditional that sets VERSION
and echoes "Using version from input: $VERSION".
---
Nitpick comments:
In `@libs/lume/scripts/build/build-release.sh`:
- Around line 54-57: Replace the cp -R call when installing the signed .app with
macOS-aware ditto to preserve extended attributes and resource forks: after
removing the old bundle (rm -rf "$APP_INSTALL_DIR/lume.app"), use ditto
".release/lume.app" "$APP_INSTALL_DIR/lume.app" (and keep the same variables
APP_INSTALL_DIR and .release/lume.app references); also ensure the script checks
ditto exit status and fails if ditto returns non-zero so signature-preserving
copy errors are not ignored.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/cd-swift-lume.ymllibs/lume/resources/Info.plistlibs/lume/resources/lume.entitlementslibs/lume/scripts/build/build-release-notarized.shlibs/lume/scripts/build/build-release.shlibs/lume/scripts/install-local.shlibs/lume/scripts/install.shlibs/lume/scripts/uninstall.shlibs/lume/tests/Mocks/MockVM.swiftlibs/lume/tests/VM/VMDetailsPrinterTests.swift
| security list-keychains -d user -s "$KEYCHAIN_PATH" $(security list-keychains -d user | tr -d '"') | ||
| security list-keychains | ||
| fi |
There was a problem hiding this comment.
Quote command substitution to prevent word splitting.
The $(security list-keychains ...) output is unquoted, which can cause word splitting issues if keychain paths contain spaces.
Proposed fix
- security list-keychains -d user -s "$KEYCHAIN_PATH" $(security list-keychains -d user | tr -d '"')
+ security list-keychains -d user -s "$KEYCHAIN_PATH" "$(security list-keychains -d user | tr -d '"')"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| security list-keychains -d user -s "$KEYCHAIN_PATH" $(security list-keychains -d user | tr -d '"') | |
| security list-keychains | |
| fi | |
| security list-keychains -d user -s "$KEYCHAIN_PATH" "$(security list-keychains -d user | tr -d '"')" | |
| security list-keychains | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 109-109: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/lume/scripts/build/build-release-notarized.sh` around lines 109 - 111,
The shell invocation that rebuilds the user keychain list uses an unquoted
command substitution: security list-keychains -d user -s "$KEYCHAIN_PATH"
$(security list-keychains -d user | tr -d '"'); to avoid word-splitting when
keychain paths contain spaces, wrap the command substitution in quotes (i.e.,
use "$(security list-keychains -d user | tr -d '"')" ) so the entire output is
treated as a single argument; update the call around the existing
security/list-keychains usage and keep the existing KEYCHAIN_PATH variable usage
intact.
|
|
||
| # Create versioned archives of the package with OS identifier in the name | ||
| log "essential" "Creating archives in $RELEASE_DIR..." | ||
| cd "$RELEASE_DIR" |
There was a problem hiding this comment.
Add error handling for cd command.
If cd fails, subsequent commands would execute in the wrong directory, potentially causing incorrect behavior or data loss.
Proposed fix
-cd "$RELEASE_DIR"
+cd "$RELEASE_DIR" || { log "error" "Failed to change to release directory"; exit 1; }🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 236-236: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/lume/scripts/build/build-release-notarized.sh` at line 236, The script
currently runs cd "$RELEASE_DIR" without checking its result; update the code
around the cd "$RELEASE_DIR" invocation to detect failure and abort safely—after
attempting to change directory, test the exit status and if cd failed print a
clear error to stderr including the $RELEASE_DIR value and exit with a nonzero
status (or use a safe shell construct that achieves the same), so subsequent
commands won't run in the wrong directory.
| APP_BUNDLE=".release/lume.app" | ||
| mkdir -p "$APP_BUNDLE/Contents/MacOS" | ||
|
|
There was a problem hiding this comment.
Reset the app bundle before assembly to prevent stale release artifacts.
Right now the script reuses .release/lume.app. If a previous run had files (for example embedded.provisionprofile) and the current run doesn’t, stale content can be shipped.
Proposed fix
APP_BUNDLE=".release/lume.app"
+rm -rf "$APP_BUNDLE"
mkdir -p "$APP_BUNDLE/Contents/MacOS"Also applies to: 30-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/lume/scripts/build/build-release.sh` around lines 14 - 16, The script
reuses APP_BUNDLE (variable APP_BUNDLE=".release/lume.app") and can carry stale
files between runs; update build-release.sh to remove the existing bundle before
recreating it: when APP_BUNDLE is set, delete or rm -rf the existing
"$APP_BUNDLE" (and any related Contents dir) prior to the mkdir -p
"$APP_BUNDLE/Contents/MacOS" step and apply the same cleanup before the later
assembly steps around the block referenced at lines 30-33 so the release is
built from a clean slate.
| # Ad-hoc sign the bundle | ||
| codesign --force --entitlements ./resources/lume.entitlements --sign - "$APP_BUNDLE/Contents/MacOS/lume" | ||
| codesign --force --sign - "$APP_BUNDLE" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine the relevant lines
cat -n libs/lume/scripts/build/build-release.sh | sed -n '30,45p'Repository: trycua/cua
Length of output: 770
🏁 Script executed:
# Search for nested bundle references in the codebase
rg -i "lume_lume\.bundle" --type sh --type mdRepository: trycua/cua
Length of output: 1982
🏁 Script executed:
# Check the resources directory for bundle files
find . -name "*.bundle" -o -name "*lume*.bundle" 2>/dev/null | head -20Repository: trycua/cua
Length of output: 36
🏁 Script executed:
# Look for bundle creation patterns in build scripts
rg -i "\.bundle" libs/lume/scripts/ -A 2 -B 2 --type shRepository: trycua/cua
Length of output: 3768
🏁 Script executed:
# Get full context around the bundle copy and codesign commands
cat -n libs/lume/scripts/build/build-release.sh | sed -n '20,50p'Repository: trycua/cua
Length of output: 1475
🏁 Script executed:
# Check if there's any signing of nested bundles elsewhere in the script
rg -i "codesign" libs/lume/scripts/build/build-release.sh -B 5 -A 5Repository: trycua/cua
Length of output: 964
🏁 Script executed:
# Look at the complete build-release.sh file to understand full signing flow
wc -l libs/lume/scripts/build/build-release.shRepository: trycua/cua
Length of output: 97
Use --deep flag for code signing and add verification to ensure nested bundle is signed.
The nested lume_lume.bundle is placed at the app root (line 25) by design for SPM compatibility. The current signing (lines 38-39) doesn't use the --deep flag, which may leave the nested bundle improperly signed. Add --deep to the final codesign and verify the signature:
# Ad-hoc sign the bundle
codesign --force --entitlements ./resources/lume.entitlements --sign - "$APP_BUNDLE/Contents/MacOS/lume"
-codesign --force --sign - "$APP_BUNDLE"
+codesign --force --deep --sign - "$APP_BUNDLE"
+codesign --verify --deep --strict "$APP_BUNDLE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/lume/scripts/build/build-release.sh` around lines 35 - 37, The final
codesign invocation is missing the --deep flag so the nested lume_lume.bundle
placed at the app root may remain unsigned; update the second codesign call that
signs "$APP_BUNDLE" to include --deep (and keep the entitlements on the main
executable call), then add a verification step after signing that uses codesign
--verify (with --deep/--strict/verbose flags) against "$APP_BUNDLE" to confirm
the nested bundle (lume_lume.bundle) and all nested code are properly signed.
SPM's auto-generated Bundle.module looks up resources via
Bundle.main.bundleURL (the .app root), but codesign rejects content
at the .app root ("unsealed contents") and in Contents/MacOS/
("bundle format unrecognized"). The only valid location for codesign
is Contents/Resources/, but Bundle.module doesn't check there.
Solution:
- Add Bundle.lumeResources custom accessor that checks resourceURL
first (for .app bundles) then bundleURL (for standalone binaries)
- Replace all Bundle.module usages in UnattendedConfig.swift
- Revert build scripts to place lume_lume.bundle in Contents/Resources/
The unused SPM-generated Bundle.module is never accessed, so its
fatalError path is never triggered.
📦 Publishable packages changed
Add |
Summary
Restructures the lume release binary as a
.appbundle to support macOS bridged networking via thecom.apple.vm.networkingentitlement. This restricted entitlement requires a provisioning profile, which in turn requires a proper.appbundle structure.What changed
.appbundle assembly in all three build scripts (build-release-notarized.sh,build-release.sh,install-local.sh):Contents/MacOS/lumeInfo.pliststamped with version atContents/Info.plistContents/embedded.provisionprofilelume_lume.bundle) placed inContents/Resources/#!/bin/sh exec .../lume.app/Contents/MacOS/lume "$@")Codesign fixes in
build-release-notarized.sh:--options runtimefor hardened runtime (required for notarization)--timestampfor secure timestamps (required for notarization)--entitlementsflag (was singular--entitlement)--keychainflag + keychain search list setup for CIcodesign -dvvverification step after signingdittoinstead ofcp -Rfor pkg staging (preserves signatures)Notarization error reporting: Added
notarytool logfetching on failure to capture Apple's detailed error JSONBundle.module resolution fix: SPM's auto-generated
Bundle.modulelooks atBundle.main.bundleURL(the.approot), but:.approot fails codesign ("unsealed contents")Contents/MacOS/fails codesign ("bundle format unrecognized")Contents/Resources/works for codesign, butBundle.moduledoesn't check thereBundle.lumeResourcescustom accessor that checksresourceURLfirst (.appbundles) thenbundleURL(standalone), replacing allBundle.moduleusage inUnattendedConfig.swift.gitignore: Added*.provisionprofileCI verification
CodeDirectory flags=0x10000(runtime)— hardened runtimeAuthority=Developer ID Application— proper certificate chainTimestamp— secure timestamp presentRequired CI secrets
PROVISIONING_PROFILE_BASE64— Base64-encoded Developer ID provisioning profile withcom.apple.vm.networkingentitlement for bundle IDcom.trycua.lumeTest plan
swift buildcompiles with newResourceBundle.swiftlume run --unattended sequoialoads preset correctly from.appbundle at runtime.pkg