Fix user-agent, identify-merge, and thread-safety bugs; declare tvOS#7
Open
srna wants to merge 6 commits into
Open
Fix user-agent, identify-merge, and thread-safety bugs; declare tvOS#7srna wants to merge 6 commits into
srna wants to merge 6 commits into
Conversation
ProcessInfo.operatingSystemVersionString is localized and human-readable
("Version 14.5 (Build 23F79)"), so splitting on spaces and taking index 1
breaks on non-English locales. Build the OS version token from
operatingSystemVersion's major/minor/patch components instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sdkVersion still reported 0.0.1 while the released tag is 1.0.0. This string is sent in the openpanel-sdk-version header and embedded in every user agent, so bump it to the next release version, 1.0.1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The OpenPanel/<version> suffix was appended before the isEmpty check, so when the WKWebView JS evaluation failed or timed out the user agent became " OpenPanel/x.y.z" instead of falling back to getBasicUserAgent(). Check for the empty result first, then append the suffix (getBasicUserAgent already appends it itself). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(new as AnyObject).value resolved through AnyObject dynamic lookup, producing an Any?? that was implicitly coerced to Any (compiler warning) and left the merged value double-wrapped. Cast to AnyCodable and unwrap its underlying value instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
send() appended to queue and flush() read/cleared it from arbitrary threads with no synchronization. Route both through the existing globalQueue barrier pattern: barrier-append in send(), atomic snapshot-and-clear in flush(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The source already guards automatic tracking with #if os(iOS) || os(tvOS), but the platform was never declared and UIKit was only imported for iOS. Add .tvOS(.v13) and import UIKit on tvOS so the declared platforms match the code (WebKit is unavailable on tvOS, so the generic user agent path is used there). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe SDK declares tvOS support, updates platform-specific user-agent generation and the SDK version, synchronizes deferred payload queue access, and changes ChangesSDK Runtime Updates
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/OpenPanel/OpenPanel.swift`:
- Around line 48-52: Update the user-agent lookup used by initialize() so it
does not synchronously fetch or construct WKWebView on the caller thread,
especially the main thread. Make the lookup asynchronous or cache its result,
ensuring initialize() remains non-blocking while preserving the fallback user
agent with the OpenPanel SDK suffix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c95aef42-6985-4f00-91d2-3ec1668671f6
📒 Files selected for processing (2)
Package.swiftSources/OpenPanel/OpenPanel.swift
Comment on lines
48
to
+52
| if userAgent.isEmpty { | ||
| userAgent = getBasicUserAgent() | ||
| return getBasicUserAgent() | ||
| } | ||
| return userAgent | ||
|
|
||
| return userAgent + " OpenPanel/\(OpenPanel.sdkVersion)" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if getBasicUserAgent already appends the SDK version
rg -A 5 'func getBasicUserAgent' Sources/OpenPanel/OpenPanel.swiftRepository: Openpanel-dev/swift-sdk
Length of output: 381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='Sources/OpenPanel/OpenPanel.swift'
echo '--- outline ---'
ast-grep outline "$file" --view expanded || true
echo '--- numbered excerpt ---'
sed -n '1,220p' "$file" | cat -nRepository: Openpanel-dev/swift-sdk
Length of output: 12146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Sources/OpenPanel/OpenPanel.swift | nl -baRepository: Openpanel-dev/swift-sdk
Length of output: 201
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('Sources/OpenPanel/OpenPanel.swift').read_text()
for needle in ['getBasicUserAgent', 'sdkVersion', 'semaphore.wait', 'DispatchQueue.main.async', 'WKWebView']:
print(f'--- {needle} ---')
idx = text.find(needle)
if idx == -1:
print('not found')
continue
start = max(0, text.rfind('\n', 0, idx-400))
end = min(len(text), idx+800)
snippet = text[start:end]
print(snippet)
PYRepository: Openpanel-dev/swift-sdk
Length of output: 6343
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='Sources/OpenPanel/OpenPanel.swift'
echo '--- call sites for DeviceInfo.getUserAgent ---'
rg -n 'DeviceInfo\.getUserAgent|getUserAgent\(' Sources/OpenPanel/OpenPanel.swift
echo '--- OpenPanel.initialize excerpt ---'
sed -n '298,380p' "$file" | cat -nRepository: Openpanel-dev/swift-sdk
Length of output: 3870
Sources/OpenPanel/OpenPanel.swift:30-52 — Avoid blocking initialize() while fetching the user agent. The fallback already appends OpenPanel/\(OpenPanel.sdkVersion), but this path can still block the caller for up to 1s if initialize() runs on the main thread, and WKWebView is created before the DispatchQueue.main.async hop. Make the lookup async or cache the result instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/OpenPanel/OpenPanel.swift` around lines 48 - 52, Update the
user-agent lookup used by initialize() so it does not synchronously fetch or
construct WKWebView on the caller thread, especially the main thread. Make the
lookup asynchronous or cache its result, ensuring initialize() remains
non-blocking while preserving the fallback user agent with the OpenPanel SDK
suffix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six small, independent fixes — one commit each — found while reviewing the SDK. No public API changes.
Fixes
operatingSystemVersioninstead of splitting the localizedoperatingSystemVersionString, which Apple documents as display-only and not machine-parseable.sdkVersionaligned with the release version (0.0.1 → 1.0.1). The string goes into theopenpanel-sdk-versionheader and every user agent; it had never been bumped past 0.0.1 even though the repo is tagged 1.0.0.isEmptycheck ran after appendingOpenPanel/<version>, so the string was never empty andgetBasicUserAgent()was dead code — a WKWebView failure/timeout produced a UA of justOpenPanel/0.0.1. The check now runs first.identifyproperty merge no longer uses AnyObject dynamic lookup.(new as AnyObject).valueresolved via the ObjC runtime against a boxed Swift struct, returning nil at runtime and corrupting merged properties (the compiler flagged it with anAny??coercion warning). Replaced with(new as? AnyCodable)?.value ?? new; the build is now warning-free.waitForProfilebuffer was appended and flushed from arbitrary threads with no synchronization, despite the README's thread-safety claim. Appends now go through the existing barrier queue andflush()snapshots-and-clears atomically.os(tvOS)branches, but without the platform declaration they had never compiled.Verification
swift build(macOS) with zero warnings;xcodebuildBUILD SUCCEEDED for iOS Simulator and tvOS Simulator.Note on releasing
Please cut a new tag after merging: SPM users on
.upToNextMajor(from: "1.0.0")are still resolving tag 1.0.0 and have never received the macOS compile fix (9b0c524) that is already onmain. A new release finally ships it to them, along with these fixes.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes