fix: race condition stale tokens/in flight requests - #228
Conversation
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR centralizes source-aware LINE authentication recovery across API, SSE, message, and media flows. It also adds persisted identity and synchronization support for predefined and paid reactions. ChangesSource-aware authentication recovery
Reaction identity and synchronization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant LINE as LINE API/SSE
participant Connector as LineClient
participant Recovery as recoverClientAfterAuthError
participant Session as Session state
LINE->>Connector: request or SSE auth failure
Connector->>Recovery: failed client and auth error
Recovery->>Session: classify and update token/session state
Session-->>Recovery: replacement client or terminal state
Recovery-->>Connector: recovery result
Connector->>LINE: retry with recovered client
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
pkg/connector/auth_recovery_test.go (1)
272-343: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard the
started.Done()call against being reached twice.
started.Done()fires inside the call closure on theold-tokenbranch. It's only reached once per goroutine today because the stale branch always hands back a different token, but any future change that returns a same-token client makes this panic with a negative WaitGroup counter and turns the whole suite red. Usingsync.OnceFuncper goroutine (or signaling beforecallLineUsing) removes that coupling.🤖 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 `@pkg/connector/auth_recovery_test.go` around lines 272 - 343, The stale-call synchronization in TestConcurrentOldTokenFailuresWaitForSingleRecovery should tolerate the callback being invoked more than once. Guard each goroutine’s started.Done() with a per-goroutine sync.OnceFunc, or signal before entering callLineUsing, while preserving the existing wait and recovery assertions.pkg/connector/auth_recovery.go (1)
64-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the locked classification into a helper so
recoverMuunlocks viadefer.Six manual
Unlock()call sites in one function is easy to break on the next edit (an early return added inside the block would deadlock every subsequent recovery).♻️ Sketch
+func (lc *LineClient) classifyAuthErrorLocked(ctx context.Context, failedClient *line.Client, err error) (*line.Client, bool, error) { + lc.recoverMu.Lock() + defer lc.recoverMu.Unlock() + // ...existing checks, returning (client, done, err) +}Then
recoverClientAfterAuthErrorperformsrecoverLineTokenonly after the helper returns without holding the lock.🤖 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 `@pkg/connector/auth_recovery.go` around lines 64 - 103, Refactor recoverClientAfterAuthError so the recoverMu-protected token/session classification is extracted into a helper that acquires recoverMu and releases it with defer. Have the helper return the classification and any replacement client or error needed by the caller, ensuring recoverLineToken runs only after the helper has released the lock; preserve the existing stale-token, logout, stopped, invalidated, and context-cancellation behavior.
🤖 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 `@pkg/connector/client.go`:
- Around line 774-789: Update ensureValidToken to guard the non-auth warning log
with checks that lc.UserLogin and lc.UserLogin.Bridge are non-nil, matching the
existing pattern in auth_recovery.go; only call Bridge.Log.Warn when the logging
dependencies are available, while preserving the current return behavior.
In `@pkg/connector/handlers/audio.go`:
- Around line 51-55: Extract the shared OBS download-and-recovery flow into
Handler.downloadOBSWithRecovery in pkg/connector/handlers/handler.go, preserving
the initial download, tryRecoverClient retry, handleFinalAuthError call, and
returned client/data/error values. Replace the duplicated sequences in
pkg/connector/handlers/audio.go lines 51-55, file.go lines 42-46, and video.go
lines 53-57 with helper calls assigning their respective data variables; all
three sites require direct changes.
In `@pkg/connector/reaction.go`:
- Around line 97-105: Update lineReactionRef.equal to compare paid reactions
using the stable identity represented by networkEmojiID, rather than full
PaidReactionType struct equality. Preserve the existing predefined-type and
nil-handling behavior, while ignoring Version and ResourceType differences when
ProductID and EmojiID match. Add a regression case to
TestStoredLineReactionForMatrixKey with identical MatrixKey/ProductID/EmojiID
but differing Version, verifying both rows are treated as the same reaction.
In `@pkg/connector/sync.go`:
- Around line 1729-1739: Update the recoverClientAfterAuthError call in the
profile-error handling path to classify the fresher profileErr instead of the
stale SSE err. Preserve the existing recoveredClient and errRecover branching,
ensuring recovery uses the latest profile probe signal.
---
Nitpick comments:
In `@pkg/connector/auth_recovery_test.go`:
- Around line 272-343: The stale-call synchronization in
TestConcurrentOldTokenFailuresWaitForSingleRecovery should tolerate the callback
being invoked more than once. Guard each goroutine’s started.Done() with a
per-goroutine sync.OnceFunc, or signal before entering callLineUsing, while
preserving the existing wait and recovery assertions.
In `@pkg/connector/auth_recovery.go`:
- Around line 64-103: Refactor recoverClientAfterAuthError so the
recoverMu-protected token/session classification is extracted into a helper that
acquires recoverMu and releases it with defer. Have the helper return the
classification and any replacement client or error needed by the caller,
ensuring recoverLineToken runs only after the helper has released the lock;
preserve the existing stale-token, logout, stopped, invalidated, and
context-cancellation behavior.
🪄 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
Run ID: ece185d9-c542-458f-b8a7-7f2567f2ae61
📒 Files selected for processing (20)
pkg/connector/auth_recovery.gopkg/connector/auth_recovery_test.gopkg/connector/client.gopkg/connector/connector.gopkg/connector/creategroup.gopkg/connector/e2ee_keys.gopkg/connector/handle_message.gopkg/connector/handlers/audio.gopkg/connector/handlers/file.gopkg/connector/handlers/handler.gopkg/connector/handlers/handler_test.gopkg/connector/handlers/image.gopkg/connector/handlers/post_notification.gopkg/connector/handlers/post_notification_test.gopkg/connector/handlers/video.gopkg/connector/reaction.gopkg/connector/reaction_test.gopkg/connector/sync.gopkg/connector/sync_test.gopkg/connector/userinfo.go
💤 Files with no reviewable changes (1)
- pkg/connector/handlers/post_notification_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Lint with 1.25
- GitHub Check: build-docker
- GitHub Check: build-docker
- GitHub Check: Lint with 1.25
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/handlers/file.gopkg/connector/connector.gopkg/connector/handle_message.gopkg/connector/handlers/audio.gopkg/connector/handlers/image.gopkg/connector/handlers/video.gopkg/connector/creategroup.gopkg/connector/handlers/handler.gopkg/connector/sync_test.gopkg/connector/userinfo.gopkg/connector/auth_recovery.gopkg/connector/handlers/post_notification.gopkg/connector/reaction_test.gopkg/connector/e2ee_keys.gopkg/connector/handlers/handler_test.gopkg/connector/client.gopkg/connector/auth_recovery_test.gopkg/connector/reaction.gopkg/connector/sync.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/handlers/file.gopkg/connector/connector.gopkg/connector/handle_message.gopkg/connector/handlers/audio.gopkg/connector/handlers/image.gopkg/connector/handlers/video.gopkg/connector/creategroup.gopkg/connector/handlers/handler.gopkg/connector/sync_test.gopkg/connector/userinfo.gopkg/connector/auth_recovery.gopkg/connector/handlers/post_notification.gopkg/connector/reaction_test.gopkg/connector/e2ee_keys.gopkg/connector/handlers/handler_test.gopkg/connector/client.gopkg/connector/auth_recovery_test.gopkg/connector/reaction.gopkg/connector/sync.go
pkg/connector/connector.go
📄 CodeRabbit inference engine (AGENTS.md)
Implement
bridgev2.NetworkConnectorandbridgev2.NetworkAPIinterfaces in the connector package for bridge logic
Files:
pkg/connector/connector.go
🔇 Additional comments (37)
pkg/connector/reaction.go (9)
539-560: 🎯 Functional CorrectnessDownstream impact of the
equal()issue.The conflict check here (
found.equal(ref), Line 553) inherits the over-strict identity comparison flagged onequal()at Lines 97-105 in this file; see that comment for the fix and rationale.
39-46: LGTM!
59-96: LGTM!
107-113: LGTM!
365-393: LGTM!
412-421: 🎯 Functional CorrectnessZero-value fallback for missing/invalid
AtMillis.When
reaction.AtMillis.Int64()fails or returns<= 0,timestampstays the Go zero value (year 1) and is passed straight intoBackfillReaction.Timestamp. If the backfill/history pipeline uses this timestamp for chronological placement, a reaction lacking a validAtMilliscould be sorted far out of place in the room history.Please confirm how
bridgev2.BackfillReaction(and the backfill placement logic) treats a zero-valueTimestamp— whether it's special-cased as "unknown" or actually affects ordering.
395-411: LGTM!Also applies to: 422-434
562-600: LGTM!
707-750: LGTM!pkg/connector/connector.go (1)
100-111: LGTM!pkg/connector/reaction_test.go (6)
21-69: LGTM!
437-503: LGTM!
505-577: LGTM!
579-600: LGTM!
735-758: LGTM!
790-828: LGTM!pkg/connector/auth_recovery.go (3)
30-46: LGTM!
12-12: LGTM!Also applies to: 134-134, 150-150
105-114: 🩺 Stability & AvailabilityNo unbounded recursion here. This path exits on the same-token
IsLoggedOutbranch, and stale-token cases return the rotated client instead;recoverTokenis also serialized byrecoverMuand rate-limited byrecentTokenRecoveryWindow.> Likely an incorrect or invalid review comment.pkg/connector/client.go (1)
82-83: LGTM!Also applies to: 318-359
pkg/connector/auth_recovery_test.go (2)
93-98: LGTM!Also applies to: 144-145, 162-163, 180-182, 202-211
435-443: LGTM!Also applies to: 457-517
pkg/connector/handlers/handler.go (1)
22-24: LGTM!Also applies to: 77-99
pkg/connector/handlers/handler_test.go (1)
14-87: LGTM!pkg/connector/handlers/image.go (1)
53-61: LGTM!pkg/connector/handlers/post_notification.go (1)
214-224: LGTM!pkg/connector/sync.go (5)
120-122: LGTM!Also applies to: 155-157, 593-595, 759-761, 1185-1187, 2165-2167, 2287-2289, 2330-2332
828-830: LGTM!Also applies to: 855-858, 1522-1528, 1640-1641
1666-1694: LGTM!Also applies to: 1705-1728
2029-2062: 🩺 Stability & AvailabilityNo issue:
convertReactionnever returnsnil, nil
convertReactionreturns a non-nil*bridgev2.BackfillReactionon success andnil, erron failure, soreaction.Sender.Senderis safe here.> Likely an incorrect or invalid review comment.
2064-2105: 🗄️ Data Integrity & IntegrationNo change needed
HasAllReactions: truewith an emptyReactionslist is the intended authoritative empty state for that sender, whileHasAllUsers: falsekeeps other senders untouched.> Likely an incorrect or invalid review comment.pkg/connector/handle_message.go (1)
30-38: LGTM!pkg/connector/e2ee_keys.go (2)
36-120: LGTM!
235-260: LGTM!pkg/connector/sync_test.go (1)
839-862: LGTM!Also applies to: 864-894, 896-924, 925-961
pkg/connector/userinfo.go (1)
143-157: LGTM!Also applies to: 199-247, 297-323, 355-361
pkg/connector/creategroup.go (1)
37-39: 🗄️ Data Integrity & IntegrationReview stale-token retries on non-idempotent writes.
callLineWithRecoveryretries once after auth errors; if LINE can fail after acceptingCreateChatorRegisterE2EEGroupKey, the helper would replay the write.
| if newClient, ok := h.tryRecoverClient(ctx, client, err); ok { | ||
| client = newClient | ||
| audioData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions) | ||
| } | ||
| h.handleFinalAuthError(ctx, client, err) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract the shared OBS download-with-recovery sequence. All three handlers implement the identical tryRecoverClient → retry → handleFinalAuthError sequence around DownloadOBSWithSIDOptions, differing only by variable name. This is security/reliability-critical auth-recovery logic; keeping three copies risks them drifting apart on future fixes.
pkg/connector/handlers/audio.go#L51-L55: replace with a call to a shared helper, e.g.client, audioData, err = h.downloadOBSWithRecovery(ctx, client, oid, talkMetaMessageID, sid, downloadOptions).pkg/connector/handlers/file.go#L42-L46: replace with the same shared helper call forfileData.pkg/connector/handlers/video.go#L53-L57: replace with the same shared helper call forvideoData.
♻️ Proposed shared helper (add to pkg/connector/handlers/handler.go)
func (h *Handler) downloadOBSWithRecovery(ctx context.Context, client *line.Client, oid, messageID, sid string, opts line.OBSDownloadOptions) (*line.Client, []byte, error) {
data, err := client.DownloadOBSWithSIDOptions(ctx, oid, messageID, sid, opts)
if newClient, ok := h.tryRecoverClient(ctx, client, err); ok {
client = newClient
data, err = client.DownloadOBSWithSIDOptions(ctx, oid, messageID, sid, opts)
}
h.handleFinalAuthError(ctx, client, err)
return client, data, err
}📍 Affects 3 files
pkg/connector/handlers/audio.go#L51-L55(this comment)pkg/connector/handlers/file.go#L42-L46pkg/connector/handlers/video.go#L53-L57
🤖 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 `@pkg/connector/handlers/audio.go` around lines 51 - 55, Extract the shared OBS
download-and-recovery flow into Handler.downloadOBSWithRecovery in
pkg/connector/handlers/handler.go, preserving the initial download,
tryRecoverClient retry, handleFinalAuthError call, and returned
client/data/error values. Replace the duplicated sequences in
pkg/connector/handlers/audio.go lines 51-55, file.go lines 42-46, and video.go
lines 53-57 with helper calls assigning their respective data variables; all
three sites require direct changes.
| func (ref lineReactionRef) equal(other lineReactionRef) bool { | ||
| if ref.typ.PredefinedReactionType != other.typ.PredefinedReactionType { | ||
| return false | ||
| } | ||
| if ref.typ.PaidReactionType == nil || other.typ.PaidReactionType == nil { | ||
| return ref.typ.PaidReactionType == nil && other.typ.PaidReactionType == nil | ||
| } | ||
| return *ref.typ.PaidReactionType == *other.typ.PaidReactionType | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
equal() is stricter than the stable reaction identity, causing false "conflict" failures.
equal() compares the full PaidReactionType struct (including ResourceType/Version) via ==, but networkEmojiID() — the type's own definition of stable identity, used for EmojiID and DB storage — only encodes ProductID+EmojiID. This is not just theoretical: getPaidReactionMXC (Line 318) caches/derives the icon MXC (which becomes ReactionMetadata.MatrixKey) keyed solely by lineSticonURL(prt.ProductID, prt.EmojiID), so two stored reactions for the same paid sticker can legitimately share a MatrixKey while carrying different Version/ResourceType (e.g., after a sticker-pack version bump between two reacts).
storedLineReactionForMatrixKey (Line 553) uses found.equal(ref) as its sole cross-row conflict check, so this realistic scenario is misclassified as a conflicting/ambiguous reaction and resolveMatrixReaction fails with unsupportedMatrixReactionError, even though the two rows refer to the same reaction identity. TestStoredLineReactionForMatrixKey doesn't exercise this case (its "same" fixture is an exact clone).
Align equal() with the stable identity used everywhere else:
🐛 Proposed fix
func (ref lineReactionRef) equal(other lineReactionRef) bool {
- if ref.typ.PredefinedReactionType != other.typ.PredefinedReactionType {
- return false
- }
- if ref.typ.PaidReactionType == nil || other.typ.PaidReactionType == nil {
- return ref.typ.PaidReactionType == nil && other.typ.PaidReactionType == nil
- }
- return *ref.typ.PaidReactionType == *other.typ.PaidReactionType
+ return ref.networkEmojiID() == other.networkEmojiID()
}Also consider adding a regression test in reaction_test.go covering two stored rows with the same MatrixKey/ProductID/EmojiID but differing Version.
📝 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.
| func (ref lineReactionRef) equal(other lineReactionRef) bool { | |
| if ref.typ.PredefinedReactionType != other.typ.PredefinedReactionType { | |
| return false | |
| } | |
| if ref.typ.PaidReactionType == nil || other.typ.PaidReactionType == nil { | |
| return ref.typ.PaidReactionType == nil && other.typ.PaidReactionType == nil | |
| } | |
| return *ref.typ.PaidReactionType == *other.typ.PaidReactionType | |
| } | |
| func (ref lineReactionRef) equal(other lineReactionRef) bool { | |
| return ref.networkEmojiID() == other.networkEmojiID() | |
| } |
🤖 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 `@pkg/connector/reaction.go` around lines 97 - 105, Update
lineReactionRef.equal to compare paid reactions using the stable identity
represented by networkEmojiID, rather than full PaidReactionType struct
equality. Preserve the existing predefined-type and nil-handling behavior, while
ignoring Version and ResourceType differences when ProductID and EmojiID match.
Add a regression case to TestStoredLineReactionForMatrixKey with identical
MatrixKey/ProductID/EmojiID but differing Version, verifying both rows are
treated as the same reaction.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/connector/forced_logout_test.go (1)
96-164: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
t.Fatalfinside a non-test goroutine can hang instead of failing.The
getProfileWithTokenstub (Line 119) callst.Fatalfon an unexpected token. This stub is invoked fromlc.ensureValidToken(...)running inside thego func()at Lines 138-140, not the main test goroutine.t.Fatalf/FailNowmust be called from the goroutine running the test function — calling it elsewhere only terminates that goroutine viaruntime.Goexit(), soensureDonenever receives a value and<-ensureDone(Line 150) hangs until the test binary's global timeout instead of reporting a clear assertion failure. This defeats the test's diagnostic value exactly when it should fire.🐛 Proposed fix: use t.Errorf instead of t.Fatalf in the goroutine-executed stub
getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) { if token == "recovered-token" { return &line.Profile{}, nil } if token != "old-token" { - t.Fatalf("profile token = %q, want old-token or recovered-token", token) + t.Errorf("profile token = %q, want old-token or recovered-token", token) } return nil, errAuthRequired }🤖 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 `@pkg/connector/forced_logout_test.go` around lines 96 - 164, Update the unexpected-token assertion in the getProfileWithToken stub within TestForcedLogoutWinsOverEnsureValidTokenRefresh to use a non-terminating test failure, such as t.Errorf, so the ensureValidToken goroutine can return and send its result through ensureDone. Preserve the existing token validation and diagnostic message.
🤖 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.
Outside diff comments:
In `@pkg/connector/forced_logout_test.go`:
- Around line 96-164: Update the unexpected-token assertion in the
getProfileWithToken stub within TestForcedLogoutWinsOverEnsureValidTokenRefresh
to use a non-terminating test failure, such as t.Errorf, so the ensureValidToken
goroutine can return and send its result through ensureDone. Preserve the
existing token validation and diagnostic message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0c2ac2e-fdce-472f-b387-8d55fa09768a
📒 Files selected for processing (4)
pkg/connector/client.gopkg/connector/forced_logout_test.gopkg/connector/sync.gopkg/connector/sync_test.go
💤 Files with no reviewable changes (1)
- pkg/connector/client.go
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Lint with 1.25
- GitHub Check: build-docker
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/sync_test.gopkg/connector/forced_logout_test.gopkg/connector/sync.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/sync_test.gopkg/connector/forced_logout_test.gopkg/connector/sync.go
🔇 Additional comments (5)
pkg/connector/sync.go (2)
1705-1760: 🎯 Functional CorrectnessFinal fallback branch still classifies against the stale
err/failedClient, not the fresherprofileErr/profileClient.At Line 1734,
lc.recoverClientAfterAuthError(ctx, failedClient, err)is reached only after the profile probe (using the current token) has already failed with a non-logged-out auth error (profileErr, Line 1719). This branch still classifies recovery against the original SSEerr/failedClientrather than the newerprofileErr/profileClient— same issue flagged in a previous review on this function (only theisLoggedOut(profileErr)branch at Lines 1723-1724 was fixed to prefer the profile client/error).🐛 Proposed fix
- recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, failedClient, err) + recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, profileClient, profileErr)
1946-1959: LGTM!Also applies to: 1981-2003, 2065-2103
pkg/connector/forced_logout_test.go (1)
53-94: LGTM!pkg/connector/sync_test.go (2)
896-923: LGTM!
925-947: 🩺 Stability & Availability
UserLoginis nil-safe on this pathmarkLoggedOutByOtherClientLockedreturns early whenlc.UserLogin == nil, so this test won’t hit a nil-pointer panic.> Likely an incorrect or invalid review comment.
No description provided.