fix: Soft logout with Matrix native OIDC does not send soft_logout flag#2360
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR improves soft-logout handling in the Matrix Dart SDK to support OIDC token expiry scenarios. The sync loop's token-unknown detection now checks both the server's ChangesSoft-logout handling improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2360 +/- ##
==========================================
- Coverage 59.29% 59.29% -0.01%
==========================================
Files 161 161
Lines 20300 20303 +3
==========================================
+ Hits 12037 12038 +1
- Misses 8263 8265 +2
Continue to review full report in Codecov by Harness.
|
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 `@lib/src/client.dart`:
- Around line 2518-2521: The soft-logout check is inverted: the variable
accessTokenExpired (computed from accessTokenExpiresAt) uses
isAfter(DateTime.now()) but should detect expiry, so change the condition in the
accessTokenExpired assignment to use
accessTokenExpiresAt?.isBefore(DateTime.now()) == true (referencing
accessTokenExpired and accessTokenExpiresAt) so expired tokens yield true and
restore the intended soft-logout logic.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 69dca8a3-e378-4c46-bd12-82e6f7f062e8
📒 Files selected for processing (1)
lib/src/client.dart
c600aef to
3aecf1b
Compare
3edcb6b to
332cc95
Compare
77ffff8 to
e5c016b
Compare
e5c016b to
7dee70b
Compare
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit e62de16. Bugbot is set up for automated code reviews on this repo. Configure here. |
BREAKING CHANGE: no longer throws an exception on soft logout network error but retries.
7dee70b to
e62de16
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Expiry flag triggers doomed refresh
- The unknown-token soft-logout path now checks for a stored refresh_token instead of token expiry metadata.
- ✅ Fixed: Refresh failures skip session clear
- Generic soft-logout refresh failures now clear the session instead of leaving the invalid token retrying.
Or push these changes by commenting:
@cursor push 8ae54d042b
Preview (8ae54d042b)
diff --git a/lib/src/client.dart b/lib/src/client.dart
--- a/lib/src/client.dart
+++ b/lib/src/client.dart
@@ -2376,10 +2376,11 @@
rethrow;
} catch (e, s) {
Logs().e(
- 'Unable to refresh session after soft logout. Try again...',
+ 'Unable to refresh session after soft logout. Clearing session...',
e,
s,
);
+ await clear();
return;
}
}();
@@ -2506,7 +2507,9 @@
// The server explicitely moved the session into soft logout state:
final isSoftLogoutState = e.raw.tryGet<bool>('soft_logout') == true;
- final hasRefreshToken = accessTokenExpiresAt != null;
+ final storedClient = await database.getClient(clientName);
+ final hasRefreshToken =
+ storedClient?.tryGet<String>('refresh_token') != null;
if (isSoftLogoutState || hasRefreshToken) {
Logs().w('The user has been logged out! Try to refresh token...', e);You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit e62de16. Configure here.


No description provided.