Skip to content

fix: disconnect keychain before emitting state changes#2505

Merged
broody merged 1 commit intomainfrom
fix/disconnect-clear-localstorage
Mar 23, 2026
Merged

fix: disconnect keychain before emitting state changes#2505
broody merged 1 commit intomainfrom
fix/disconnect-clear-localstorage

Conversation

@broody
Copy link
Contributor

@broody broody commented Mar 23, 2026

Summary

  • Moves keychain.disconnect() and this.close() to run before emitAccountsChanged([]) in ControllerProvider.disconnect()
  • emitAccountsChanged([]) triggers starknet-react state updates that can cause the page to reload/navigate before the keychain disconnect completes, leaving iframe localStorage intact
  • This was the root cause of users remaining logged in after disconnect — the keychain never received the disconnect signal

Test plan

  • Connect via controller, disconnect, reconnect — user is prompted to re-login
  • Verify no regression in starknet-react useAccount/useConnect state after disconnect

🤖 Generated with Claude Code

emitAccountsChanged([]) triggers starknet-react state updates that can
cause the page to reload or navigate before keychain.disconnect() is
called, leaving the iframe localStorage intact. Move the keychain
disconnect and iframe close before emitting so the cleanup completes
before any framework re-render.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
controller-example-next Ready Ready Preview Mar 23, 2026 8:53pm
keychain Ready Ready Preview Mar 23, 2026 8:53pm
keychain-storybook Ready Ready Preview Mar 23, 2026 8:53pm

Request Review

@broody broody merged commit 5526bee into main Mar 23, 2026
12 checks passed
@broody broody deleted the fix/disconnect-clear-localstorage branch March 23, 2026 20:57
@claude
Copy link
Contributor

claude bot commented Mar 23, 2026

Claude finished @broody's task —— View job


Analyzing PR for documentation impact

  • Read the changed controller file
  • Search docs for disconnect-related content
  • Determine if user-facing behavior documentation needs updates
  • Update docs or explain why no changes needed

Analysis Complete

No documentation updates needed.

This PR fixes an internal implementation bug where the order of operations in ControllerProvider.disconnect() was causing the keychain iframe's localStorage to remain intact after disconnect. The fix reorders the operations so that keychain.disconnect() and this.close() run before emitAccountsChanged([]).

Why no docs updates:

  1. Internal bug fix, not API change: The PR corrects broken behavior to match the expected/documented behavior. Users were supposed to be logged out after disconnect — this bug prevented that from happening.

  2. No user-facing API changes: The disconnect() method signature and usage remain identical. Developers call it the same way.

  3. Documentation describes expected behavior: The docs in sessions.md describe what should happen during disconnect (redirect to logout page, then to disconnectRedirectUrl if configured). This PR makes the actual behavior match what's already documented.

  4. Per guidelines - DEFAULT TO NO CHANGES: The rules explicitly state "Internal refactors, test changes, CI changes, and dependency bumps need nothing." This is an internal implementation fix that restores correct behavior.

The existing documentation in docs-repo/src/pages/controller/sessions.md:347 correctly describes disconnect behavior:

When disconnect() is called, users will be redirected to the keychain logout page, and after successful logout, they will be automatically redirected to your specified URL.

This fix ensures that behavior actually works as documented.


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