-
Notifications
You must be signed in to change notification settings - Fork 187
ci(swift): Swift CI/CD only – auto-tag, build-release, release, Packa… #323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
ci(swift): Swift CI/CD only – auto-tag, build-release, release, Packa… #323
Conversation
- Add SDKTestApp: minimal iOS test app (Status, Chat, TTS tabs) - List/download/load LLM and TTS models with progress - Chat: send messages and get LLM responses - TTS: speak text with Piper voices - Add swift-spm-example: SPM consumer example - Point swift-spm-example and validation/swift-spm-consumer at RunanywhereAI/runanywhere-sdks - SDK: clearer Foundation Models error messages; RunAnywhereAI README chat/LLM notes
…ge.swift v0.17.5 - swift-auto-tag.yml: auto-tag swift-v* on push to main - swift-sdk-build-release.yml: Phase 2 – build on swift-v* tag, create release + semver tag - swift-sdk-release.yml: tag trigger removed in favor of Phase 2 - Package.swift: remote binaries for v0.17.5 for consumers
📝 WalkthroughWalkthroughAdds Phase‑1 auto‑tagging and Phase‑2 macOS build/release workflows, flips SPM to remote production binaries with updated checksums, adjusts C++ bridge concurrency/memory and error messages, and introduces a SwiftUI SDK test app plus SPM example and validation manifests. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant AutoTag as Swift Auto‑Tag (Action)
participant Git as Git / Origin
participant CI as macOS Runner (Build & Release)
participant Repo as Repository
participant GHRelease as GitHub Releases
Dev->>AutoTag: push to main
AutoTag->>Git: list existing `swift-v*` tags
AutoTag->>Repo: read state, compute next `swift-vX.Y.Z`
alt tag missing
AutoTag->>Git: create annotated tag `swift-vX.Y.Z`
AutoTag->>Git: push tag
end
Git->>CI: push of `swift-v*` tag triggers build pipeline
CI->>Repo: derive version from tag
CI->>CI: build XCFrameworks, zip assets, compute checksums
alt Package.swift requires update
CI->>Repo: update `Package.swift` (sdkVersion + checksums), commit & push
CI->>Git: move tag to updated commit
end
CI->>GHRelease: create GitHub Release and upload assets
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✏️ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 1328ad3 in 26 seconds. Click for details.
- Reviewed
349lines of code in4files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_OylM7Tsb6jhDBrlk
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 11 comments
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] actions/checkout should fetch tags/complete history for retagging
This workflow deletes and recreates tags (git push origin ":refs/tags/${TAG}" then git tag -f ...), but actions/checkout@v4 defaults to fetch-depth: 1 and may not fetch tags; in that case the local repo may not have the tag ref you’re trying to move/overwrite and can behave unexpectedly (or require extra fetch). Setting with: fetch-depth: 0 (and/or fetch-tags: true) makes tag operations deterministic.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-sdk-build-release.yml
Line: 39:42
Comment:
[P0] `actions/checkout` should fetch tags/complete history for retagging
This workflow deletes and recreates tags (`git push origin ":refs/tags/${TAG}"` then `git tag -f ...`), but `actions/checkout@v4` defaults to `fetch-depth: 1` and may not fetch tags; in that case the local repo may not have the tag ref you’re trying to move/overwrite and can behave unexpectedly (or require extra fetch). Setting `with: fetch-depth: 0` (and/or `fetch-tags: true`) makes tag operations deterministic.
How can I resolve this? If you propose a fix, please make it concise.| - name: Build Native XCFrameworks | ||
| run: | | ||
| ./${COMMONS_DIR}/scripts/build-ios.sh --clean | ||
| echo "XCFrameworks:" | ||
| ls -la ${COMMONS_DIR}/dist/*.xcframework 2>/dev/null || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] ls ${COMMONS_DIR}/dist/*.xcframework won’t list directories
*.xcframework are directories, so ls -la ${COMMONS_DIR}/dist/*.xcframework typically won’t match and will always fall into the || true branch. This can hide whether the build actually produced frameworks. Use a directory glob (or ls -la ${COMMONS_DIR}/dist/*.xcframework/) so it reports correctly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-sdk-build-release.yml
Line: 52:56
Comment:
[P0] `ls ${COMMONS_DIR}/dist/*.xcframework` won’t list directories
`*.xcframework` are directories, so `ls -la ${COMMONS_DIR}/dist/*.xcframework` typically won’t match and will always fall into the `|| true` branch. This can hide whether the build actually produced frameworks. Use a directory glob (or `ls -la ${COMMONS_DIR}/dist/*.xcframework/`) so it reports correctly.
How can I resolve this? If you propose a fix, please make it concise.| - name: Prepare Release Assets | ||
| id: assets | ||
| run: | | ||
| VERSION="${{ steps.version.outputs.version }}" | ||
| COMMONS_DIST="${COMMONS_DIR}/dist" | ||
| mkdir -p release-assets | ||
| for name in RACommons RABackendLLAMACPP RABackendONNX; do | ||
| if [ -d "${COMMONS_DIST}/${name}.xcframework" ]; then | ||
| zip_name="${name}-ios-v${VERSION}.zip" | ||
| (cd "${COMMONS_DIST}" && zip -r "../../../../release-assets/${zip_name}" "${name}.xcframework") | ||
| echo "Created release-assets/${zip_name}" | ||
| fi | ||
| done | ||
| echo "assets_dir=release-assets" >> "$GITHUB_OUTPUT" | ||
| ls -la release-assets/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] Release asset zip path likely incorrect (too many ../)
Inside Prepare Release Assets, you cd "${COMMONS_DIST}" (which is sdk/runanywhere-commons/dist) and then write to ../../../../release-assets/.... From sdk/runanywhere-commons/dist, ../../../ reaches repo root; ../../../../ goes one level above the repo, so the zip may be created outside the workspace (or fail). This will break subsequent checksum and release steps.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-sdk-build-release.yml
Line: 63:77
Comment:
[P0] Release asset zip path likely incorrect (too many `../`)
Inside `Prepare Release Assets`, you `cd "${COMMONS_DIST}"` (which is `sdk/runanywhere-commons/dist`) and then write to `../../../../release-assets/...`. From `sdk/runanywhere-commons/dist`, `../../../` reaches repo root; `../../../../` goes one level above the repo, so the zip may be created outside the workspace (or fail). This will break subsequent checksum and release steps.
How can I resolve this? If you propose a fix, please make it concise.| - name: Compute Checksums | ||
| id: checksums | ||
| run: | | ||
| VERSION="${{ steps.version.outputs.version }}" | ||
| COMMONS_CHECKSUM=$(shasum -a 256 "release-assets/RACommons-ios-v${VERSION}.zip" | cut -d ' ' -f 1) | ||
| LLAMACPP_CHECKSUM=$(shasum -a 256 "release-assets/RABackendLLAMACPP-ios-v${VERSION}.zip" | cut -d ' ' -f 1) | ||
| ONNX_CHECKSUM=$(shasum -a 256 "release-assets/RABackendONNX-ios-v${VERSION}.zip" | cut -d ' ' -f 1) | ||
| echo "commons=$COMMONS_CHECKSUM" >> "$GITHUB_OUTPUT" | ||
| echo "llamacpp=$LLAMACPP_CHECKSUM" >> "$GITHUB_OUTPUT" | ||
| echo "onnx=$ONNX_CHECKSUM" >> "$GITHUB_OUTPUT" | ||
| echo "RACommons: $COMMONS_CHECKSUM" | ||
| echo "RABackendLLAMACPP: $LLAMACPP_CHECKSUM" | ||
| echo "RABackendONNX: $ONNX_CHECKSUM" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] Checksums step assumes all zips exist, but asset creation is conditional
Prepare Release Assets only zips a framework if the directory exists, but Compute Checksums unconditionally runs shasum on all three zip files. If any framework wasn’t produced, the workflow will fail here with “No such file”. Either fail earlier when a framework is missing or make checksum generation conditional on what was actually built.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-sdk-build-release.yml
Line: 79:91
Comment:
[P0] Checksums step assumes all zips exist, but asset creation is conditional
`Prepare Release Assets` only zips a framework if the directory exists, but `Compute Checksums` unconditionally runs `shasum` on all three zip files. If any framework wasn’t produced, the workflow will fail here with “No such file”. Either fail earlier when a framework is missing or make checksum generation conditional on what was actually built.
How can I resolve this? If you propose a fix, please make it concise.| - name: Commit and Push Package.swift Update | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| git add Package.swift | ||
| if git diff --staged --quiet; then | ||
| echo "No Package.swift changes to commit" | ||
| else | ||
| git commit -m "chore(swift): update Package.swift checksums for swift-v${{ steps.version.outputs.version }}" | ||
| git push origin HEAD:main | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] Committing to main on a tag-triggered workflow can fail without checking out main
This workflow triggers on tag pushes, so actions/checkout checks out the tagged commit in a detached HEAD state. git push origin HEAD:main can be rejected if main has advanced since the tag commit. If you intend to update main reliably, you’ll need to explicitly fetch/checkout main and merge/rebase (or otherwise ensure you’re pushing atop the current main).
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-sdk-build-release.yml
Line: 110:120
Comment:
[P0] Committing to `main` on a tag-triggered workflow can fail without checking out `main`
This workflow triggers on tag pushes, so `actions/checkout` checks out the tagged commit in a detached HEAD state. `git push origin HEAD:main` can be rejected if `main` has advanced since the tag commit. If you intend to update `main` reliably, you’ll need to explicitly fetch/checkout `main` and merge/rebase (or otherwise ensure you’re pushing atop the current `main`).
How can I resolve this? If you propose a fix, please make it concise.| - name: Update Tag to Commit with Checksums | ||
| run: | | ||
| TAG="${{ steps.version.outputs.tag }}" | ||
| # Delete remote tag so we can move it to the new commit | ||
| git push origin ":refs/tags/${TAG}" || true | ||
| # Tag current commit (with Package.swift update) | ||
| git tag -f "${TAG}" -m "Swift SDK Release ${TAG}" | ||
| git push origin "${TAG}" | ||
| echo "Tag ${TAG} updated to point to commit with correct checksums" | ||
|
|
||
| - name: Create Semver Tag for SPM | ||
| run: | | ||
| VERSION="${{ steps.version.outputs.version }}" | ||
| # SPM resolves versions from semver tags (e.g. 0.1.2 or v0.1.2) | ||
| # Create tag ${VERSION} on same commit so consumers can use from: "X.Y.Z" | ||
| git push origin ":refs/tags/${VERSION}" 2>/dev/null || true | ||
| git tag -f "${VERSION}" -m "Swift SDK v${VERSION}" | ||
| git push origin "${VERSION}" | ||
| echo "Semver tag ${VERSION} created for SPM resolution" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] Force-moving tags (swift-v* and semver) is unsafe and can break consumers
Deleting and recreating tags (git push origin ":refs/tags/..." + git tag -f) rewrites published refs. If any clients already resolved the old tag (or a release was created), this breaks reproducibility and can confuse SPM caching. If the goal is for the tag to point to the checksum-updated commit, it’s safer to generate the tag after committing Package.swift, rather than moving an existing tag.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-sdk-build-release.yml
Line: 122:140
Comment:
[P0] Force-moving tags (`swift-v*` and semver) is unsafe and can break consumers
Deleting and recreating tags (`git push origin ":refs/tags/..."` + `git tag -f`) rewrites published refs. If any clients already resolved the old tag (or a release was created), this breaks reproducibility and can confuse SPM caching. If the goal is for the tag to point to the checksum-updated commit, it’s safer to generate the tag after committing Package.swift, rather than moving an existing tag.
How can I resolve this? If you propose a fix, please make it concise.| - name: Compute Next Tag | ||
| id: next_tag | ||
| run: | | ||
| set -e | ||
| # List swift-v* tags, strip prefix, keep only X.Y.Z (no prerelease) | ||
| LATEST=$(git tag -l 'swift-v*' | sed 's/^swift-v//' | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -n1 || true) | ||
| if [ -z "$LATEST" ]; then | ||
| NEXT="0.1.1" | ||
| else | ||
| MAJOR=$(echo "$LATEST" | cut -d. -f1) | ||
| MINOR=$(echo "$LATEST" | cut -d. -f2) | ||
| PATCH=$(echo "$LATEST" | cut -d. -f3) | ||
| PATCH=$((PATCH + 1)) | ||
| NEXT="$MAJOR.$MINOR.$PATCH" | ||
| fi | ||
| NEW_TAG="swift-v$NEXT" | ||
| echo "tag=$NEW_TAG" >> "$GITHUB_OUTPUT" | ||
| echo "Computed next tag: $NEW_TAG" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Auto-tagging always bumps patch; version source of truth is unclear
swift-auto-tag.yml computes the next tag by taking the highest existing swift-vX.Y.Z and incrementing PATCH. That means every merge to main is a “patch release”, regardless of what actually changed, and it can diverge from the Swift SDK’s own versioning (e.g., Package.swift’s sdkVersion). If releases should follow an explicit version file or changelog, derive the tag from that instead of auto-incrementing.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-auto-tag.yml
Line: 38:55
Comment:
[P1] Auto-tagging always bumps patch; version source of truth is unclear
`swift-auto-tag.yml` computes the next tag by taking the highest existing `swift-vX.Y.Z` and incrementing PATCH. That means every merge to `main` is a “patch release”, regardless of what actually changed, and it can diverge from the Swift SDK’s own versioning (e.g., `Package.swift`’s `sdkVersion`). If releases should follow an explicit version file or changelog, derive the tag from that instead of auto-incrementing.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| on: | ||
| push: | ||
| branches: [main] | ||
|
|
||
| permissions: | ||
| contents: write | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Broad trigger: tags on every push to main
This workflow triggers on any push to main (including merges, reverts, and automated commits from other workflows). If Phase 2 commits to main, that commit will also trigger Phase 1 and create another tag unless you add a guard (e.g., skip when actor is github-actions[bot] or when commit message matches the checksum update). Otherwise you can end up in a tag churn/feedback loop.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/swift-auto-tag.yml
Line: 15:21
Comment:
[P1] Broad trigger: tags on every push to `main`
This workflow triggers on any push to `main` (including merges, reverts, and automated commits from other workflows). If Phase 2 commits to `main`, that commit will also trigger Phase 1 and create another tag unless you add a guard (e.g., skip when actor is `github-actions[bot]` or when commit message matches the checksum update). Otherwise you can end up in a tag churn/feedback loop.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (3)
With Prompt To Fix With AIThis is a comment left during a code review.
Path: Package.swift
Line: 43:48
Comment:
[P1] Defaulting `useLocalBinaries` to `false` breaks local dev flows that rely on checked-in binaries
With `useLocalBinaries = false`, local builds will try to fetch release assets from GitHub by default. If contributors expect local builds to work without network access (or without a published `v0.17.5` release), this will fail. If the intention is “production default”, consider documenting that local dev must run `--set-local` or adjust the default back to `true` in-repo.
How can I resolve this? If you propose a fix, please make it concise.
In production mode, the binary URLs always point to Prompt To Fix With AIThis is a comment left during a code review.
Path: Package.swift
Line: 219:245
Comment:
[P1] Comment says “v* or swift-v*” but URLs hardcode `releases/download/v\(sdkVersion)`
In production mode, the binary URLs always point to `releases/download/v(sdkVersion)`; they won’t work for releases created under `swift-v*` unless there is also a `vX.Y.Z` release/tag with the same assets. Either adjust the URLs to match the tag you publish assets under, or ensure Phase 2 also creates a `vX.Y.Z` release/tag consistently.
How can I resolve this? If you propose a fix, please make it concise.
The workflow-level permission is Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/swift-sdk-release.yml
Line: 59:61
Comment:
[P2] `permissions: contents: read` conflicts with jobs that push tags/releases
The workflow-level permission is `contents: read`, but later jobs (e.g., `publish`) need `contents: write` to create releases/tags. You do set job-level `permissions` for `publish`, but other steps that might push/commit in other jobs can fail silently depending on paths. Consider setting the minimal required permissions per job consistently (or bump workflow-level if multiple jobs need write).
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/swift-sdk-build-release.yml:
- Around line 183-186: Release notes in the workflow list "iOS 13.0+ / macOS
10.15+" which conflicts with Package.swift's platform declarations (.iOS(.v17)
and .macOS(.v14)); update the workflow release notes to reflect the actual
minimums (iOS 17.0+ and macOS 14.0+) or, if you intend to support older OS
versions, change the Package.swift platforms from .iOS(.v17)/.macOS(.v14) to the
desired minima (e.g., .iOS(.v13)/.macOS(.v10_15)), ensuring the values in the
workflow file and the Package.swift platforms array match exactly.
- Around line 110-120: The git push step currently runs git push origin
HEAD:main without handling failures; update the Commit and Push Package.swift
Update step to detect and handle a failed push of "git push origin HEAD:main"
(the command shown as git push origin HEAD:main) — either add a fallback push
(e.g., try git push origin HEAD:master if main is protected) or ensure the
workflow fails if the push cannot complete (e.g., check exit status and exit
with non-zero or call a fail command) so that a silent failure cannot leave the
tag without Package.swift checksum updates; modify the run block around the git
push invocation accordingly to implement one of these behaviors.
🧹 Nitpick comments (3)
.github/workflows/swift-auto-tag.yml (1)
15-17: Consider adding path filters to avoid unnecessary tag creation.This workflow triggers on every push to main, including changes unrelated to the Swift SDK. This could result in version bumps for non-SDK changes.
Consider adding path filters to scope the trigger:
♻️ Suggested path filter
on: push: branches: [main] + paths: + - 'sdk/runanywhere-swift/**' + - 'sdk/runanywhere-commons/**' + - 'Package.swift'.github/workflows/swift-sdk-build-release.yml (2)
69-75: Fragile relative path in zip command.The path
../../../../release-assets/assumes a specific directory depth. If directory structure changes, this will break silently or create files in unexpected locations.Consider using absolute paths:
♻️ Suggested fix using GITHUB_WORKSPACE
if [ -d "${COMMONS_DIST}/${name}.xcframework" ]; then zip_name="${name}-ios-v${VERSION}.zip" - (cd "${COMMONS_DIST}" && zip -r "../../../../release-assets/${zip_name}" "${name}.xcframework") + (cd "${COMMONS_DIST}" && zip -r "${GITHUB_WORKSPACE}/release-assets/${zip_name}" "${name}.xcframework") echo "Created release-assets/${zip_name}" fi
99-107: Sed patterns are tied to current Package.swift structure.The sed range patterns (e.g.,
/RACommons-ios-v.*zip/,/checksum:/) assume a specific structure where the URL line precedes the checksum line. This works for the current format but could break if:
- The order of properties changes
- Comments are added between URL and checksum
- Additional binary targets with similar names are added
Consider adding validation that checksums were actually updated:
♻️ Suggested validation
sed -i '' '/RABackendONNX-ios-v.*zip/,/checksum:/ s/checksum: "[^"]*"/checksum: "'"$ONNX_CHECKSUM"'"/' Package.swift echo "Updated Package.swift sdkVersion and checksums" grep -A1 "RACommons-ios" Package.swift | head -4 + # Validate checksums were updated + if ! grep -q "$COMMONS_CHECKSUM" Package.swift; then + echo "ERROR: RACommons checksum not found in Package.swift" + exit 1 + fi
|
Hey @josuediazflores |
|
@josuediazflores could you please double check - the first thing should be we need to build the commons library with the 3 xcframeworks and that will then be consumed with the 3 ios swift packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/swift-sdk-build-release.yml:
- Around line 157-165: The tag creation step should be made idempotent and
ensure the release "swift-v" tag points to the release commit: check for
existence of SEMVER_TAG before creating/pushing it (skip or update if it already
exists) and update the "swift-v" tag to point to HEAD (the
checksum-updated/release commit) instead of leaving it on the pre-update commit;
modify the block that constructs SEMVER_TAG and runs git tag -a / git push
origin to first test for the tag (and only create/push if absent) and add a
forced update of the "swift-v" tag to HEAD so reruns are safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@examples/ios/SDKTestApp/SDKTestApp.xcodeproj/project.pbxproj`:
- Line 248: Update the SWIFT_VERSION build setting from 5.0 to 6.0 in the Xcode
project configuration: find the SWIFT_VERSION entries in the project.pbxproj
(both the Debug and Release build configurations) and replace their values with
6.0 so the project uses Swift 6 APIs.
In `@examples/ios/SDKTestApp/SDKTestApp/ContentView.swift`:
- Around line 317-324: The cardStyle View extension uses Color(uiColor:
.secondarySystemBackground) which depends on UIKit and will fail on macOS;
update the Color construction inside cardStyle() to choose the platform-specific
initializer (use UIColor on iOS/tvOS and NSColor on macOS) via conditional
compilation (e.g., `#if` canImport(UIKit) / `#elseif` canImport(AppKit)) so
cardStyle() builds on both platforms and provide a sane fallback (like
Color.secondary) for any other platforms.
🧹 Nitpick comments (2)
examples/ios/SDKTestApp/SDKTestApp.xcodeproj/project.pbxproj (1)
229-229: Hardcoded development team ID may cause signing issues.
DEVELOPMENT_TEAM = 9Z6RJ25W3Zis hardcoded in both Debug and Release configurations. Other contributors will encounter code signing errors unless they override this setting or have access to this team.Consider either:
- Removing the hardcoded value and letting Xcode prompt for team selection
- Using
xcconfigfiles to externalize team configurationexamples/ios/SDKTestApp/SDKTestApp/ContentView.swift (1)
133-137: Cache the DateFormatter for better performance.Creating a new
DateFormatteron everyappendLogcall is inefficient.DateFormatterinitialization is relatively expensive.Proposed fix
+ private static let timestampFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateFormat = "HH:mm:ss" + return formatter + }() + private static func timestamp() -> String { - let formatter = DateFormatter() - formatter.dateFormat = "HH:mm:ss" - return formatter.string(from: Date()) + return timestampFormatter.string(from: Date()) }
| SDKROOT = auto; | ||
| SUPPORTED_PLATFORMS = "iphoneos iphonesimulator macosx"; | ||
| SWIFT_EMIT_LOC_STRINGS = YES; | ||
| SWIFT_VERSION = 5.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update Swift version to 6.0.
SWIFT_VERSION = 5.0 does not align with the coding guidelines requiring Swift 6 APIs. The minimum required version per project learnings is 5.9, but for new code, Swift 6 should be used.
Proposed fix
- SWIFT_VERSION = 5.0;
+ SWIFT_VERSION = 6.0;Apply the same change to the Release configuration at Line 281.
As per coding guidelines: "Use the latest Swift 6 APIs always."
📝 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.
| SWIFT_VERSION = 5.0; | |
| SWIFT_VERSION = 6.0; |
🤖 Prompt for AI Agents
In `@examples/ios/SDKTestApp/SDKTestApp.xcodeproj/project.pbxproj` at line 248,
Update the SWIFT_VERSION build setting from 5.0 to 6.0 in the Xcode project
configuration: find the SWIFT_VERSION entries in the project.pbxproj (both the
Debug and Release build configurations) and replace their values with 6.0 so the
project uses Swift 6 APIs.
| private extension View { | ||
| func cardStyle() -> some View { | ||
| self | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| .padding(16) | ||
| .background(Color(uiColor: .secondarySystemBackground), in: RoundedRectangle(cornerRadius: 16)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UIKit color dependency may break macOS builds.
Color(uiColor: .secondarySystemBackground) uses UIKit's UIColor, but the project targets both iOS and macOS (per SUPPORTED_PLATFORMS in project.pbxproj). This will fail to compile on macOS.
Proposed cross-platform fix
private extension View {
func cardStyle() -> some View {
self
.frame(maxWidth: .infinity, alignment: .leading)
.padding(16)
- .background(Color(uiColor: .secondarySystemBackground), in: RoundedRectangle(cornerRadius: 16))
+ `#if` os(iOS)
+ .background(Color(uiColor: .secondarySystemBackground), in: RoundedRectangle(cornerRadius: 16))
+ `#else`
+ .background(Color(nsColor: .windowBackgroundColor), in: RoundedRectangle(cornerRadius: 16))
+ `#endif`
}
}📝 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.
| private extension View { | |
| func cardStyle() -> some View { | |
| self | |
| .frame(maxWidth: .infinity, alignment: .leading) | |
| .padding(16) | |
| .background(Color(uiColor: .secondarySystemBackground), in: RoundedRectangle(cornerRadius: 16)) | |
| } | |
| } | |
| private extension View { | |
| func cardStyle() -> some View { | |
| self | |
| .frame(maxWidth: .infinity, alignment: .leading) | |
| .padding(16) | |
| `#if` os(iOS) | |
| .background(Color(uiColor: .secondarySystemBackground), in: RoundedRectangle(cornerRadius: 16)) | |
| `#else` | |
| .background(Color(nsColor: .windowBackgroundColor), in: RoundedRectangle(cornerRadius: 16)) | |
| `#endif` | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@examples/ios/SDKTestApp/SDKTestApp/ContentView.swift` around lines 317 - 324,
The cardStyle View extension uses Color(uiColor: .secondarySystemBackground)
which depends on UIKit and will fail on macOS; update the Color construction
inside cardStyle() to choose the platform-specific initializer (use UIColor on
iOS/tvOS and NSColor on macOS) via conditional compilation (e.g., `#if`
canImport(UIKit) / `#elseif` canImport(AppKit)) so cardStyle() builds on both
platforms and provide a sane fallback (like Color.secondary) for any other
platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+ModelAssignment.swift (1)
44-82:⚠️ Potential issue | 🔴 CriticalCritical: Remove timeout from semaphore.wait() to prevent use-after-free.
If
semaphore.wait(timeout:)times out before the Task completes, the callback returns and deallocatesresultPtrwhile the Task may still be writing to it (lines 64, 74). Additionally,outResponse(passed from C++) can be reused or deallocated by the C++ caller after the callback returns, but the Task continues writing to it (lines 58–73). This causes use-after-free corruption on both the Swift and C++ sides.Replace the timeout wait with an indefinite wait to guarantee the Task completes before deallocating
resultPtror returning from the callback:Fix
- _ = semaphore.wait(timeout: .now() + 30) + semaphore.wait() let result = resultPtr.move() resultPtr.deallocate() return resultAdditional note: Using
DispatchSemaphoreto block Task execution is an outdated pattern. Consider refactoring this to a pure Swift 6 concurrency model (e.g.,CheckedContinuationor a structured async wrapper) to comply with current Swift best practices.
🤖 Fix all issues with AI agents
In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Public/Sessions/LiveTranscriptionSession.swift`:
- Around line 71-82: The AsyncStream continuation's onTermination currently
captures `session` strongly and closes a retain cycle via
`session.onPartialCallback -> continuation -> onTermination -> session`; modify
the onTermination closure to capture `session` weakly (e.g., `weak session`) and
then safely set `session?.onPartialCallback = nil` inside the Task, and also
ensure the Task and the earlier `session?.onPartialCallback` assignment don't
create strong captures (use `[weak session]` where appropriate) so
`LiveTranscriptionSession`, `onPartialCallback`, and the `continuation` can be
deallocated.
sdk/runanywhere-swift/Sources/RunAnywhere/Public/Sessions/LiveTranscriptionSession.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Platform.swift (1)
167-193:⚠️ Potential issue | 🔴 CriticalFix undefined
resultassignment in error path.
resultis not declared in this scope, so this won’t compile. Set the pointer instead.🐛 Minimal fix
- result = RAC_ERROR_INTERNAL + resultPtr.pointee = RAC_ERROR_INTERNAL
🤖 Fix all issues with AI agents
In `@examples/ios/SDKTestApp/SDKTestApp/App/TTSView.swift`:
- Around line 113-118: The catch block that currently clears ttsModels and
resets isLoadingModels should also set modelsError with the caught error so
users see why loading failed; inside the await MainActor.run in the catch,
assign modelsError = error.localizedDescription (or a formatted message), keep
ttsModels = [], and isLoadingModels = false, and ensure the UI displays
modelsError the same way downloadError is shown so failures surface to the user.
In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge`+Platform.swift:
- Around line 32-43: The compile error and data race are caused by using an
undefined variable `result` and unsynchronized access to
`lastFoundationModelsErrorMessage`; fix by changing the assignment to use the
provided pointer (set `resultPtr.pointee = RAC_ERROR_INTERNAL` where `result`
was used) and protect all accesses to the static var
`lastFoundationModelsErrorMessage` with an OSAllocatedUnfairLock: add a private
static lock in the same extension, wrap writes from the C++ callback (where you
currently set `lastFoundationModelsErrorMessage`) with lock acquisition, and
provide an atomic `take` helper (e.g., a static method) that locks, reads the
value, clears it, and returns it so `RunAnywhere+TextGeneration.swift` can call
that helper to safely read-and-clear the message; keep using Swift 6 primitives
and avoid NSLock.
In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/LLM/RunAnywhere`+TextGeneration.swift:
- Around line 247-253: The current read-then-clear pattern for
CppBridge.Platform.lastFoundationModelsErrorMessage can drop updates; instead
add and use an atomic "take" accessor on CppBridge.Platform (e.g.,
takeLastFoundationModelsErrorMessage()) that returns the current value and
clears it under the same lock, then replace the read/clear sequence in
RunAnywhere+TextGeneration.swift (the block referencing
CppBridge.Platform.lastFoundationModelsErrorMessage and setting message) to call
that take method and assign message from its return if non-nil.
🧹 Nitpick comments (3)
examples/swift-spm-example/README.md (2)
31-32: Consider using a more generic or verified simulator name."iPhone 17" may not be a valid simulator name. Xcode simulator names typically follow device release names (e.g., "iPhone 16", "iPhone 15 Pro"). Consider using a generic placeholder or a confirmed device name.
📝 Suggested fix
-xcodebuild -scheme SwiftSPMExample -destination 'platform=iOS Simulator,name=iPhone 17' -configuration Debug build +xcodebuild -scheme SwiftSPMExample -destination 'platform=iOS Simulator,name=iPhone 16' -configuration Debug build
44-45: Same simulator name concern.Update the simulator name reference here as well to match a valid device name.
📝 Suggested fix
-3. Set the run destination to an **iOS Simulator** (e.g. iPhone 17). +3. Set the run destination to an **iOS Simulator** (e.g. iPhone 16).examples/ios/SDKTestApp/SDKTestApp/App/TTSView.swift (1)
28-32: Consider caching downloaded model IDs in state instead of computing during render.
RunAnywhere.isModelDownloaded()is called synchronously for every model during each view body evaluation. If this involves file system checks, it could cause UI stuttering, especially with many models.♻️ Suggested approach
Cache downloaded IDs in a
@Stateproperty and refresh after downloads complete:+ `@State` private var downloadedModelIds: Set<String> = [] - private var downloadedModelIds: Set<String> { - Set(ttsModels.filter { model in - RunAnywhere.isModelDownloaded(model.id, framework: model.framework) - }.map(\.id)) - }Then update this set in
loadTTSModels()after fetching models, and indownloadModel()after a successful download:// In downloadModel, after completion: if progress.stage == .completed { downloadedModelIds.insert(model.id) // ... }
| } catch { | ||
| await MainActor.run { | ||
| ttsModels = [] | ||
| isLoadingModels = false | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error feedback when model loading fails.
The catch block clears the models list silently, leaving the user with a generic "No TTS models registered" message even when the actual cause is an SDK or network error. Other functions in this file set corresponding error state (e.g., downloadError, loadError, speakError).
🛠️ Proposed fix
+ `@State` private var modelsError: String?
// ...in loadTTSModels():
} catch {
await MainActor.run {
ttsModels = []
isLoadingModels = false
+ modelsError = error.localizedDescription
}
}Then display modelsError in the UI similar to how downloadError is shown.
🤖 Prompt for AI Agents
In `@examples/ios/SDKTestApp/SDKTestApp/App/TTSView.swift` around lines 113 - 118,
The catch block that currently clears ttsModels and resets isLoadingModels
should also set modelsError with the caught error so users see why loading
failed; inside the await MainActor.run in the catch, assign modelsError =
error.localizedDescription (or a formatted message), keep ttsModels = [], and
isLoadingModels = false, and ensure the UI displays modelsError the same way
downloadError is shown so failures surface to the user.
| /// Last error message from Foundation Models generate (used when C++ reports "Streaming generation failed") | ||
| static var lastFoundationModelsErrorMessage: String? | ||
|
|
||
| /// Maps generic Foundation Models error descriptions to user-friendly messages. | ||
| private static func friendlyFoundationModelsMessage(_ raw: String) -> String { | ||
| if raw.contains("FoundationModels.LanguageModelSession.GenerationError") || | ||
| raw.contains("error -1") || | ||
| raw.contains("The operation couldn't be completed") { | ||
| return "Apple Intelligence is not available on this device or simulator. Try a physical device with Apple Intelligence enabled in Settings, or use a downloaded LLM from the Models tab." | ||
| } | ||
| return raw | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, check the file structure and size
wc -l sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Platform.swiftRepository: RunanywhereAI/runanywhere-sdks
Length of output: 173
🏁 Script executed:
# Read the file to understand context around the flagged lines
cat -n sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Platform.swiftRepository: RunanywhereAI/runanywhere-sdks
Length of output: 15489
🏁 Script executed:
# Search for all usages of lastFoundationModelsErrorMessage
rg "lastFoundationModelsErrorMessage" sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Platform.swift -B 2 -A 2Repository: RunanywhereAI/runanywhere-sdks
Length of output: 734
🏁 Script executed:
# Check for NSLock usage in the file
rg "NSLock" sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Platform.swiftRepository: RunanywhereAI/runanywhere-sdks
Length of output: 56
🏁 Script executed:
# Search for all references to lastFoundationModelsErrorMessage across the repo
rg "lastFoundationModelsErrorMessage" --type swiftRepository: RunanywhereAI/runanywhere-sdks
Length of output: 797
🏁 Script executed:
# Also check if there's code that clears/reads this error message elsewhere
rg "lastFoundationModelsErrorMessage" --type swift -B 2 -A 2Repository: RunanywhereAI/runanywhere-sdks
Length of output: 2821
Fix undefined variable and add thread-safe synchronization for lastFoundationModelsErrorMessage.
Line 184 has a critical compile error: result = RAC_ERROR_INTERNAL references an undefined variable. Should be resultPtr.pointee = RAC_ERROR_INTERNAL.
Additionally, lastFoundationModelsErrorMessage is written from async Tasks in C++ callbacks (line 183) and read+cleared in RunAnywhere+TextGeneration.swift without synchronization—creating both a data race and a race between the read and clear operations. Use OSAllocatedUnfairLock with an atomic take helper to protect access.
🔧 Suggested fix (Swift 6 synchronization primitives)
import CRACommons
import Foundation
+import os
@@
- static var lastFoundationModelsErrorMessage: String?
+ private static let foundationModelsErrorLock = OSAllocatedUnfairLock()
+ private static var _lastFoundationModelsErrorMessage: String?
+
+ static var lastFoundationModelsErrorMessage: String? {
+ get { foundationModelsErrorLock.withLock { _lastFoundationModelsErrorMessage } }
+ set { foundationModelsErrorLock.withLock { _lastFoundationModelsErrorMessage = newValue } }
+ }
+
+ static func takeLastFoundationModelsErrorMessage() -> String? {
+ foundationModelsErrorLock.withLock {
+ defer { _lastFoundationModelsErrorMessage = nil }
+ return _lastFoundationModelsErrorMessage
+ }
+ }Also fix line 184 to use resultPtr.pointee instead of undefined result.
Per coding guidelines: Use Swift 6 APIs; avoid NSLock; use Swift 6 concurrency primitives.
🤖 Prompt for AI Agents
In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge`+Platform.swift
around lines 32 - 43, The compile error and data race are caused by using an
undefined variable `result` and unsynchronized access to
`lastFoundationModelsErrorMessage`; fix by changing the assignment to use the
provided pointer (set `resultPtr.pointee = RAC_ERROR_INTERNAL` where `result`
was used) and protect all accesses to the static var
`lastFoundationModelsErrorMessage` with an OSAllocatedUnfairLock: add a private
static lock in the same extension, wrap writes from the C++ callback (where you
currently set `lastFoundationModelsErrorMessage`) with lock acquisition, and
provide an atomic `take` helper (e.g., a static method) that locks, reads the
value, clears it, and returns it so `RunAnywhere+TextGeneration.swift` can call
that helper to safely read-and-clear the message; keep using Swift 6 primitives
and avoid NSLock.
| var message = errorMsg.map { String(cString: $0) } ?? "Unknown error" | ||
| // Use actual Foundation Models error when C++ reported generic "Streaming generation failed" | ||
| if message == "Streaming generation failed", | ||
| let platformError = CppBridge.Platform.lastFoundationModelsErrorMessage { | ||
| message = platformError | ||
| CppBridge.Platform.lastFoundationModelsErrorMessage = nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make error substitution atomic to avoid lost updates.
Read‑then‑clear can drop a newer message set concurrently. Use a take helper from CppBridge.Platform (get+clear under the same lock).
🔧 Suggested adjustment
- if message == "Streaming generation failed",
- let platformError = CppBridge.Platform.lastFoundationModelsErrorMessage {
- message = platformError
- CppBridge.Platform.lastFoundationModelsErrorMessage = nil
- }
+ if message == "Streaming generation failed",
+ let platformError = CppBridge.Platform.takeLastFoundationModelsErrorMessage() {
+ message = platformError
+ }🤖 Prompt for AI Agents
In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Public/Extensions/LLM/RunAnywhere`+TextGeneration.swift
around lines 247 - 253, The current read-then-clear pattern for
CppBridge.Platform.lastFoundationModelsErrorMessage can drop updates; instead
add and use an atomic "take" accessor on CppBridge.Platform (e.g.,
takeLastFoundationModelsErrorMessage()) that returns the current value and
clears it under the same lock, then replace the read/clear sequence in
RunAnywhere+TextGeneration.swift (the block referencing
CppBridge.Platform.lastFoundationModelsErrorMessage and setting message) to call
that take method and assign message from its return if non-nil.
- CppBridge+Device/ModelAssignment: write to temp pointers in Task, copy to outResponse after wait - CppBridge+Platform: TTS create use handlePtr; generate use responsePtr for strdup - AlamofireDownloadService: capture requiresExtraction in local let for Task - LiveTranscriptionSession: use Sendable Ref for onTermination to avoid capturing self
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Device.swift (1)
114-164: 🛠️ Refactor suggestion | 🟠 MajorReplace
DispatchSemaphoreblocking pattern with Swift 6 concurrency primitives.The HTTP layer correctly enforces a 30-second timeout (
URLSessionConfiguration.timeoutIntervalForRequest = 30.0), so the indefinite blocking risk is mitigated. However, usingDispatchSemaphoreto block a thread waiting for a Task violates the Swift 6 concurrency guidelines. Refactor this C-to-Swift bridge callback to useasync/awaitthroughout instead of blocking on a semaphore, or wrap the async operation using a proper Swift 6 continuation pattern (e.g.,withCheckedThrowingContinuation) if the C callback must remain synchronous.
🤖 Fix all issues with AI agents
In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge`+ModelAssignment.swift:
- Around line 85-96: The semaphore wait uses a 30s timeout which can cause a
use-after-free if the Task writes to
resultPtr/statusCodePtr/responseLengthPtr/responseBodyPtr/errorMessagePtr after
those pointers are deallocated; change the wait call to block indefinitely (use
semaphore.wait()) so the Task always completes before you read from outResponse
and deallocate resultPtr, statusCodePtr, responseLengthPtr, responseBodyPtr, and
errorMessagePtr; ensure the sequence in the function containing
semaphore.wait(timeout: .now() + 30) is updated to wait unconditionally, then
copy values into outResponse.pointee and only afterwards deallocate the pointers
and return outResponse.pointee.result.
🧹 Nitpick comments (1)
sdk/runanywhere-swift/Sources/RunAnywhere/Public/Sessions/LiveTranscriptionSession.swift (1)
79-81: Consider hoistingRefto a nested type for clarity.Defining a class inside a computed property works but is recreated on each access and is unconventional. Moving it to a private nested type improves readability and avoids repeated class definition overhead.
♻️ Suggested refactor
`@MainActor` public final class LiveTranscriptionSession: ObservableObject { private let logger = SDKLogger(category: "LiveTranscription") + + /// Sendable box for weak session reference in async termination handlers + private final class WeakRef: `@unchecked` Sendable { + weak var session: LiveTranscriptionSession? + } // MARK: - Published StateThen in
transcriptions:- // Sendable box so onTermination closure doesn't capture self (Swift 6 concurrency) - final class Ref: `@unchecked` Sendable { - weak var session: LiveTranscriptionSession? - } - let ref = Ref() + let ref = WeakRef() ref.session = self
| _ = semaphore.wait(timeout: .now() + 30) | ||
| return result | ||
| outResponse.pointee.result = resultPtr.pointee | ||
| outResponse.pointee.status_code = statusCodePtr.pointee | ||
| outResponse.pointee.response_length = responseLengthPtr.pointee | ||
| outResponse.pointee.response_body = responseBodyPtr.pointee.map { UnsafePointer($0) } | ||
| outResponse.pointee.error_message = errorMessagePtr.pointee.map { UnsafePointer($0) } | ||
| resultPtr.deallocate() | ||
| statusCodePtr.deallocate() | ||
| responseLengthPtr.deallocate() | ||
| responseBodyPtr.deallocate() | ||
| errorMessagePtr.deallocate() | ||
| return outResponse.pointee.result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -i "CppBridge+ModelAssignment" | head -20Repository: RunanywhereAI/runanywhere-sdks
Length of output: 176
🏁 Script executed:
wc -l "sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+ModelAssignment.swift"Repository: RunanywhereAI/runanywhere-sdks
Length of output: 180
🏁 Script executed:
sed -n '50,120p' "sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+ModelAssignment.swift"Repository: RunanywhereAI/runanywhere-sdks
Length of output: 3512
🏁 Script executed:
sed -n '1,85p' "sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+ModelAssignment.swift"Repository: RunanywhereAI/runanywhere-sdks
Length of output: 3878
🏁 Script executed:
# Let me check if there are any other similar patterns in the codebase that might inform severity
rg "DispatchSemaphore" sdk/runanywhere-swift/Sources/ -A 5 -B 2 | head -50Repository: RunanywhereAI/runanywhere-sdks
Length of output: 2842
🏁 Script executed:
# Check the similar pattern in CppBridge+Device.swift to see if it has timeout handling
sed -n '40,150p' "sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Device.swift"Repository: RunanywhereAI/runanywhere-sdks
Length of output: 6416
🏁 Script executed:
# Check what comes after the Task block in Device.swift
sed -n '150,180p' "sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+Device.swift"Repository: RunanywhereAI/runanywhere-sdks
Length of output: 1559
🏁 Script executed:
# Let me also check the Swift version requirements to understand the context better
cat "sdk/runanywhere-swift/Package.swift" | head -30Repository: RunanywhereAI/runanywhere-sdks
Length of output: 141
🏁 Script executed:
find sdk/runanywhere-swift -name "Package.swift" -o -name "project.pbxproj" | head -5Repository: RunanywhereAI/runanywhere-sdks
Length of output: 56
🏁 Script executed:
# Check lines around 85-96 in ModelAssignment.swift to see if there's any timeout handling
sed -n '80,100p' "sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge+ModelAssignment.swift"Repository: RunanywhereAI/runanywhere-sdks
Length of output: 1095
Timeout wait causes use-after-free if the Task completes after deallocation.
If the semaphore times out at 30 seconds, the code deallocates all pointers while the Task may still execute and write into them. This is a memory safety violation. Remove the timeout and wait unconditionally—the similar pattern in CppBridge+Device.swift uses semaphore.wait() without timeout, which is the safe approach here.
Recommended fix
- _ = semaphore.wait(timeout: .now() + 30)
+ semaphore.wait()📝 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.
| _ = semaphore.wait(timeout: .now() + 30) | |
| return result | |
| outResponse.pointee.result = resultPtr.pointee | |
| outResponse.pointee.status_code = statusCodePtr.pointee | |
| outResponse.pointee.response_length = responseLengthPtr.pointee | |
| outResponse.pointee.response_body = responseBodyPtr.pointee.map { UnsafePointer($0) } | |
| outResponse.pointee.error_message = errorMessagePtr.pointee.map { UnsafePointer($0) } | |
| resultPtr.deallocate() | |
| statusCodePtr.deallocate() | |
| responseLengthPtr.deallocate() | |
| responseBodyPtr.deallocate() | |
| errorMessagePtr.deallocate() | |
| return outResponse.pointee.result | |
| semaphore.wait() | |
| outResponse.pointee.result = resultPtr.pointee | |
| outResponse.pointee.status_code = statusCodePtr.pointee | |
| outResponse.pointee.response_length = responseLengthPtr.pointee | |
| outResponse.pointee.response_body = responseBodyPtr.pointee.map { UnsafePointer($0) } | |
| outResponse.pointee.error_message = errorMessagePtr.pointee.map { UnsafePointer($0) } | |
| resultPtr.deallocate() | |
| statusCodePtr.deallocate() | |
| responseLengthPtr.deallocate() | |
| responseBodyPtr.deallocate() | |
| errorMessagePtr.deallocate() | |
| return outResponse.pointee.result |
🤖 Prompt for AI Agents
In
`@sdk/runanywhere-swift/Sources/RunAnywhere/Foundation/Bridge/Extensions/CppBridge`+ModelAssignment.swift
around lines 85 - 96, The semaphore wait uses a 30s timeout which can cause a
use-after-free if the Task writes to
resultPtr/statusCodePtr/responseLengthPtr/responseBodyPtr/errorMessagePtr after
those pointers are deallocated; change the wait call to block indefinitely (use
semaphore.wait()) so the Task always completes before you read from outResponse
and deallocate resultPtr, statusCodePtr, responseLengthPtr, responseBodyPtr, and
errorMessagePtr; ensure the sequence in the function containing
semaphore.wait(timeout: .now() + 30) is updated to wait unconditionally, then
copy values into outResponse.pointee and only afterwards deallocate the pointers
and return outResponse.pointee.result.
Hey Shubhama, here’s the link to the action I triggered: https://github.com/josuediazflores/runanywhere-sdks/actions/runs/21620064116 |
Double-checked: we do build the commons library first (3 XCFrameworks: RACommons, RABackendLLAMACPP, RABackendONNX), then the Swift SDK step installs those into Binaries/ and builds. I also dropped --build-commons from the Swift step so commons is only built once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/swift-sdk-build-release.yml:
- Around line 106-110: The workflow currently pulls main after building from the
tag which can move the tag/build relationship; instead, after fetching origin
main but before running git pull, verify that the checked-out commit (the tag
build commit, from git rev-parse HEAD) matches origin/main (git rev-parse
origin/main) and fail fast if they differ, then perform the update with git pull
--ff-only origin main so the job errors rather than silently creating a
divergent history; reference the git commands used (git fetch, git rev-parse
HEAD, git rev-parse origin/main, git pull --ff-only) when adding the guard.
…divergent history - Save tag build commit (git rev-parse HEAD) before checkout - Fetch origin main (git fetch) - Compare tag commit vs origin/main (git rev-parse origin/main) - Fail fast if they differ to avoid creating divergent history - Use git pull --ff-only origin main for safe fast-forward only
…ult setup note - Switch c-cpp from autobuild to manual (no build system at repo root) - Add Build C/C++ step: cmake in sdk/runanywhere-commons with minimal opts - Comment: disable Default setup in repo settings when using this workflow
]## Swift CI/CD – Auto-tag, Build & Release, Package.swift
This PR adds Swift SDK CI/CD only: auto-tagging on
main, build-and-release onswift-v*tags, and rootPackage.swiftconfigured for SPM consumers.Summary
main, creates a new git tagswift-vX.Y.Zto mark Swift SDK release points. No builds or macOS runners.swift-v*tag: builds native XCFrameworks and Swift SDK on macOS, computes SHA256 checksums, updates rootPackage.swiftwith checksums and version, commits tomain, moves the tag, and creates a GitHub Release with the ZIPs. SPM consumers resolve the tag and get correct checksums.workflow_call(e.g. from release-all) orworkflow_dispatch. Phase 2 handles tag-driven releases.useLocalBinaries = false,sdkVersion = "0.17.5") so consumers get the right XCFramework URLs and checksums from GitHub releases.Flow
main→ Phase 1 runs → createsswift-vX.Y.Z.swift-v*tag → Phase 2 runs → build, checksums, updatePackage.swift, commit, move tag, create Release with assets..package(url: "https://github.com/RunanywhereAI/runanywhere-sdks", from: "0.17.5")(or the new semver tag) and resolve binary targets from the release.Out of scope for this PR
Important
Introduces CI/CD workflows for Swift SDK auto-tagging and release building, updating
Package.swiftfor remote binary usage.swift-auto-tag.ymlto auto-tagmainbranch merges withswift-vX.Y.Z.swift-sdk-build-release.ymlto build and release onswift-v*tag push, updatingPackage.swiftwith checksums and version.swift-sdk-release.ymlto remove tag trigger, now runs viaworkflow_callorworkflow_dispatch.useLocalBinariestofalsefor remote binary usage.sdkVersionto0.17.5and corresponding checksums for remote binaries.This description was created by
for 1328ad3. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
Greptile Overview
Greptile Summary
This PR adds a two-phase Swift SDK CI/CD setup: a main-branch workflow to auto-create
swift-vX.Y.Ztags, and a tag-triggered macOS workflow to build XCFramework assets, compute checksums, update the rootPackage.swift, and publish a GitHub Release. It also adjusts the existingswift-sdk-release.ymlto run only viaworkflow_call/manual dispatch and switches the rootPackage.swiftto default to remote binaries with updated checksums.Key concerns are around the Phase 2 workflow’s determinism and safety: it performs tag rewriting (delete/recreate tags), commits back to
mainfrom a tag-triggered checkout (detached HEAD / potential non-fast-forward), and has brittle asset pathing and assumptions about produced artifacts. There’s also a naming mismatch betweenswift-v*releases andPackage.swiftURLs that hardcodereleases/download/v(sdkVersion).Confidence Score: 2/5
Important Files Changed
Sequence Diagram
(4/5) You can add custom instructions or style guidelines for the agent here!
Context used:
dashboard- CLAUDE.md (source)