Skip to content

Avoid signing out of a device when renaming it - #9374

Open
MiSikora wants to merge 1 commit into
feature/mehow/simple-sync/deep-linkingfrom
feature/mehow/simple-sync/rename-race-condition
Open

Avoid signing out of a device when renaming it#9374
MiSikora wants to merge 1 commit into
feature/mehow/simple-sync/deep-linkingfrom
feature/mehow/simple-sync/rename-race-condition

Conversation

@MiSikora

@MiSikora MiSikora commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Task/Issue URL: https://app.asana.com/1/137249556945/project/1216103556496795/task/1217093845234581?focus=true
Tech Design URL (if applicable): https://app.asana.com/1/137249556945/project/1216103556496795/task/1216509582049467?focus=true
API Proposals URL(s) (if applicable): N/A

Description

I ran into this right after setting up sync on a fresh device. I renamed the device from the Sync settings, and when I returned to the screen the device was no longer synced. On a second device the renamed entry was still listed, and because this device had dropped out of the account I could sync it again, which left me with a duplicate and three devices in the list.

The rename does not use a dedicated endpoint. It re-authenticates the device, which issues a new sync token and replaces the one already stored. While that happens the app is still doing other authenticated work in the background, such as periodically refreshing the device list. If one of those requests is in flight with the old token at the moment it gets rotated, the server rejects it with an invalid credentials response. That response was treated as a signal that the account was gone, so the local sync data was cleared even though the account was still valid. Because nothing was actually removed on the server, the device kept appearing on other devices and syncing again created a duplicate.

This was less likely to happen before the Sync settings redesign. The rename used to be a dialog opened directly on the main Sync settings screen, so the rename and any refresh stayed on one screen. In the redesign the rename is confirmed on a separate screen, and the device list is refreshed when coming back to the main screen after the edit is confirmed. That return refresh lands right after the token is rotated, which makes the timing that triggers the bug much more likely.

The fix records which token each request used and compares it against the token currently stored when an invalid credentials response comes back. The local account is only cleared when the failing request used the token that is still current, or when the request carried no token at all. A rejection for a token that has since been replaced is treated as stale and ignored, so a rename no longer signs the device out. A genuine invalid credentials response for the current token still clears the account as before.

The behavior is behind the preventStaleTokenLogout flag, which is on by default.

Steps to test this PR

This was a race condition and is hard to reproduce reliably, so the steps below are a smoke test to confirm that renaming keeps the device synced.

Rename a device

  • Set up Sync & Backup on a device.
  • Tap the device in the list of synced devices.
  • Tap "Edit Name".
  • Enter a new name.
  • Tap "Done".
  • Verify the device is still synced and shows its new name.

UI changes

N/A


Note

Medium Risk
Changes when local sync credentials are cleared on 401, which affects session persistence; behavior is gated by a remote toggle and preserves clearing for the current token.

Overview
Fixes a race where renaming a device (re-login with a new sync token) could leave the device unsynced: a concurrent API call still using the old token could get 401 invalid credentials, and SyncServiceRemote would clearAll() local sync state as if the account were dead.

removeKeysIfInvalid now receives the token that failed (from the request Authorization header or explicit call-site token). It only wipes the store when that token still matches syncStore.token, or when the token is unknown (keeps prior behavior). A 401 on a superseded token is ignored when preventStaleTokenLogout is enabled (new toggle on SyncFeature, default true).

Tests cover current-token 401 (still clears), rotated-token 401 (no clear), and toggle-off (still clears).

Reviewed by Cursor Bugbot for commit e789f61. Bugbot is set up for automated code reviews on this repo. Configure here.

@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/rename-race-condition branch from 07f68ff to db4e5d8 Compare August 3, 2026 12:44
@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/deep-linking branch from 59a8b49 to 8f178bf Compare August 3, 2026 12:44
@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/rename-race-condition branch from db4e5d8 to 6f60557 Compare August 3, 2026 13:13
@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/deep-linking branch from 8f178bf to 46408dc Compare August 3, 2026 13:13

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6f60557. Configure here.

// token swap can 401 on the old token while the account is still valid. Only wipe local state
// when the token that failed is still the current one (or unknown, preserving prior behavior).
val tokenStillCurrent = requestToken == null || requestToken == syncStore.token
if (!syncFeature.preventStaleTokenLogout().isEnabled() || tokenStillCurrent) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stale token 401 still triggers misleading signed-out notification

Medium Severity

The fix correctly prevents syncStore.clearAll() on a stale-token 401, but SyncInvalidTokenInterceptor — an OkHttp interceptor that fires on every sync-endpoint 401 — still shows a "signed out" notification for those same stale-token responses. After a rename, the device correctly stays synced, but the user receives a misleading "signed out" notification. The interceptor needs the same stale-token guard (compare request token against current stored token) to avoid this false notification.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6f60557. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure if this should be fixed here. I'll create a follow up task in the backlog and update this comment once I do it.

@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/rename-race-condition branch from 6f60557 to 8790942 Compare August 3, 2026 16:30
@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/deep-linking branch 2 times, most recently from 0f72b4f to 28ff820 Compare August 4, 2026 07:11
@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/rename-race-condition branch from 8790942 to 288957a Compare August 4, 2026 07:11
@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/deep-linking branch 2 times, most recently from 0ce8707 to 6f426ea Compare August 4, 2026 14:41
@MiSikora
MiSikora force-pushed the feature/mehow/simple-sync/rename-race-condition branch from 288957a to e789f61 Compare August 4, 2026 15:15
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.

2 participants