Skip to content

Conversation

@hiroshihorie
Copy link
Member

@hiroshihorie hiroshihorie commented Feb 6, 2026

Summary by CodeRabbit

  • New Features

    • Added protocol v13 support with improved connection state management and handling of network events for enhanced reliability during disconnections and reconnections.
  • Updates

    • Default protocol version upgraded from v12 to v13, providing access to new capabilities and improved connection behavior.

@coderabbitai
Copy link

coderabbitai bot commented Feb 6, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hiroshi/protocol-v13

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@hiroshihorie hiroshihorie changed the title Hiroshi/protocol v13 Protocol v13 Feb 6, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/src/core/signal_client.dart (1)

461-464: 🛠️ Refactor suggestion | 🟠 Major

Extension method creates empty LeaveRequest without v13 fields.

This extension method creates an empty LeaveRequest() without the reason or action fields set in the class method above. If this method is intentionally kept for some specific use case, it should be documented. Otherwise, consider removing it or aligning it with the updated class method.

Suggested fix to align with v13 semantics
 `@internal`
-void sendLeave() => _sendRequest(lk_rtc.SignalRequest(
-      leave: lk_rtc.LeaveRequest(),
-    ));
+void sendLeave() => _sendRequest(lk_rtc.SignalRequest(
+      leave: lk_rtc.LeaveRequest(
+        reason: lk_models.DisconnectReason.CLIENT_INITIATED,
+        action: lk_rtc.LeaveRequest_Action.DISCONNECT,
+      ),
+    ));
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df41188 and 5f42b49.

📒 Files selected for processing (6)
  • .changes/protocol-v13
  • lib/src/core/engine.dart
  • lib/src/core/signal_client.dart
  • lib/src/extensions.dart
  • lib/src/options.dart
  • lib/src/types/other.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build for Flutter Windows
  • GitHub Check: Build for Flutter Android
  • GitHub Check: Build for Flutter macOS
🔇 Additional comments (6)
.changes/protocol-v13 (1)

1-1: LGTM!

The changelog entry accurately describes the protocol v13 feature addition with LeaveRequest action support.

lib/src/options.dart (1)

64-69: LGTM!

The default protocol version is correctly updated to v13. The engine's LeaveEvent handling maintains backward compatibility with v12 servers by still checking canReconnect when the action is DISCONNECT (the default/unset value).

lib/src/types/other.dart (1)

26-39: LGTM!

The ProtocolVersion.v13 enum member is added following the established pattern. The inline comment accurately documents the key protocol changes (regions in leave request, action obsoleting canReconnect).

lib/src/extensions.dart (1)

56-71: LGTM!

The string mapping for ProtocolVersion.v13 is correctly added following the established pattern.

lib/src/core/engine.dart (1)

1353-1380: Well-structured protocol v13 LeaveEvent handling with proper backward compatibility.

The logic correctly prioritizes the new action field while maintaining backward compatibility with v12 servers:

  1. RESUME (line 1362): Soft reconnect without full restart
  2. RECONNECT or canReconnect=true (line 1366): Full reconnect - handles both v13 RECONNECT action and v12 canReconnect flag
  3. DISCONNECT (line 1370): Default path for clean disconnect, including v12 servers with canReconnect=false

The comment at lines 1359-1361 clearly documents the backward compatibility approach.

lib/src/core/signal_client.dart (1)

200-207: No action needed — the two sendLeave() methods do not cause functional issues.

The instance method at lines 200-207 and the extension method at lines 461-464 have different signatures and contexts:

  • Instance method (public API): Future<void> sendLeave() async with v13 fields (reason: CLIENT_INITIATED, action: DISCONNECT)
  • Extension method (internal API): @internal void sendLeave() creating an empty LeaveRequest(), synchronous return

Dart's method resolution order ensures the instance method takes precedence when called on a SignalClient instance. The extension method's @internal annotation restricts its visibility to internal use only. The code at engine.dart:1387 correctly calls the public instance method via await signalClient.sendLeave().

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant