Skip to content

OAuth: lazy-register message listener per login session#13895

Merged
rickyrombo merged 2 commits into
mjp-oauth-env-promisefrom
copilot/sub-pr-13892-another-one
Mar 11, 2026
Merged

OAuth: lazy-register message listener per login session#13895
rickyrombo merged 2 commits into
mjp-oauth-env-promisefrom
copilot/sub-pr-13892-another-one

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 11, 2026

Each OAuth instance attached a window.addEventListener('message', ...) in its constructor and never removed it, causing listener accumulation across multiple SDK instantiations and risking duplicate message processing.

Changes

  • Removed constructor listenerwindow.addEventListener no longer called at construction time
  • Lazy registration in loginAsync — listener is attached only when entering the postMessage popup flow; guarded against double-registration if loginAsync is called again while a login is in progress
  • Cleanup in _settleLogin — listener is removed (and popup check interval cleared) as soon as the login resolves or rejects, via a stored _boundMessageHandler reference
// Before: listener lived forever, one per OAuth instance
constructor(...) {
  window.addEventListener('message', (e) => this._receiveMessage(e), false)
}

// After: listener scoped to each login attempt
async loginAsync({ redirectUri = 'postMessage', ... }) {
  if (redirectUri === 'postMessage' && !this._boundMessageHandler) {
    this._boundMessageHandler = (e) => this._receiveMessage(e)
    window.addEventListener('message', this._boundMessageHandler, false)
  }
  // ...
}

private _settleLogin(resultOrError) {
  // resolve/reject promise...
  window.removeEventListener('message', this._boundMessageHandler, false)
  this._boundMessageHandler = null
  this._clearPopupCheckInterval()
}
  • Tests — added removeEventListener + crypto.getRandomValues to the window stub; new OAuth message listener lifecycle suite covering: no listener on construction, listener attached on loginAsync, no duplicate on re-entry, listener removed on settle

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 11, 2026

⚠️ No Changeset found

Latest commit: 8d1825b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

…register on settle

Co-authored-by: rickyrombo <3690498+rickyrombo@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP Address feedback on promisifying OAuth service login OAuth: lazy-register message listener per login session Mar 11, 2026
@rickyrombo rickyrombo marked this pull request as ready for review March 11, 2026 21:39
@rickyrombo rickyrombo merged commit 911baeb into mjp-oauth-env-promise Mar 11, 2026
5 checks passed
@rickyrombo rickyrombo deleted the copilot/sub-pr-13892-another-one branch March 11, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants