You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(ios): honor echoMessages=false in client options - #614
On iOS, ClientOptions(echoMessages: false) is decoded as true: READ_VALUE assigns the boxed NSNumber directly to the native BOOL, so the non-null @NO object becomes YES on arm64. This prevents callers from disabling echoes from their own connection.
Use the existing READ_BOOL macro for echoMessages so its value is unboxed with boolValue. Add an AblyFlutterCodecTests XCTest target covering explicit false, explicit true, a missing option, and NSNull. The tests encode the ClientOptions wire format with Flutter's standard writer and pass it through the production AblyFlutterReader; they exercise the native decoding boundary without connecting to Ably.
Validation on an arm64 iPhone 17 simulator running iOS 26.5, with Xcode 26.6 and Flutter 3.44.7:
The new testEchoMessagesFalseIsPreserved regression fails against unchanged upstream production code (xcodebuild exits with 65); the two existing iOS tests pass.
After the fix, ios/run_unit_tests.sh passes all six iOS tests: four codec tests and the two existing stream-channel tests.
flutter test --no-pub: all 65 tests pass.
flutter analyze --no-pub: no issues found, after resolving the root, example, and integration-test app dependencies.
This PR targets main (2.x). Would you consider backporting the same fix to the ably_flutter 1.2.x release line?
Summary by CodeRabbit
Bug Fixes
Client options now correctly preserve an explicitly supplied false value for message echoing. When the option is omitted or set to null, the native SDK default remains in effect.
Tests
Added coverage for true, false, omitted, and null values to verify client-option handling.
No actionable comments were generated in the recent review.
Walkthrough
The iOS client-options reader now decodes echoMessages as a Boolean. A new XCTest target checks explicit true and false values and confirms that omitted or null values retain the native SDK default.
Changes
iOS client-options decoding
Layer / File(s)
Summary
Boolean decoding and tests ios/ably_pubsub_device_flutter/Package.swift, ios/ably_pubsub_device_flutter/Sources/ably_pubsub_device_flutter/codec/AblyFlutterReader.m, ios/ably_pubsub_device_flutter/Tests/AblyFlutterCodecTests/AblyFlutterReaderTests.m
The reader decodes echoMessages as a Boolean. The new test target and tests check explicit true and false values, and verify that omitted or null values retain the native default.
The change decodes explicit false as a Boolean and adds tests for the option values. No merge-blocking risk is identified.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title "fix(ios): honor echoMessages=false in client options" matches the main change. The diff switches the iOS decoding of ClientOptions.echoMessages from READ_VALUE to READ_BOOL, so an explicit …
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "fix(ios): honor echoMessages=false in client options" matches the main change. The diff switches the iOS decoding of ClientOptions.echoMessages from READ_VALUE to READ_BOOL, so an explicit false is no longer treated as true. The title names the platform, the affected option, and the intended behavior. The added AblyFlutterCodecTests target and its tests support the same fix. The title is short, specific, and in a conventional commit form.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (2 skipped: 2 unsupported.)
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
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.
A rabbit checks the values as they flow, True stays true, and false stays so. When fields are missing, defaults remain, Null leaves the native choice the same. The codec hops along its way.
CodeRabbit Verify
Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: dfee3607-d72c-40ae-964f-e45157635731
Commits
Reviewing files that changed from the base of the PR and between 34b2214 and c04e8cb.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Hi maintainers, could you please approve the CI workflows for this PR? Thanks!
This branch has not been deployed
No deployments
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
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.
On iOS,
ClientOptions(echoMessages: false)is decoded astrue:READ_VALUEassigns the boxedNSNumberdirectly to the nativeBOOL, so the non-null@NOobject becomesYESon arm64. This prevents callers from disabling echoes from their own connection.Use the existing
READ_BOOLmacro forechoMessagesso its value is unboxed withboolValue. Add anAblyFlutterCodecTestsXCTest target covering explicitfalse, explicittrue, a missing option, andNSNull. The tests encode the ClientOptions wire format with Flutter's standard writer and pass it through the productionAblyFlutterReader; they exercise the native decoding boundary without connecting to Ably.Validation on an arm64 iPhone 17 simulator running iOS 26.5, with Xcode 26.6 and Flutter 3.44.7:
testEchoMessagesFalseIsPreservedregression fails against unchanged upstream production code (xcodebuildexits with 65); the two existing iOS tests pass.ios/run_unit_tests.shpasses all six iOS tests: four codec tests and the two existing stream-channel tests.flutter test --no-pub: all 65 tests pass.flutter analyze --no-pub: no issues found, after resolving the root, example, and integration-test app dependencies.This PR targets
main(2.x). Would you consider backporting the same fix to theably_flutter1.2.x release line?Summary by CodeRabbit
falsevalue for message echoing. When the option is omitted or set to null, the native SDK default remains in effect.