Skip to content

feat: use external_user_id in embed to save conversation#9304

Merged
AdityaHegde merged 3 commits into
mainfrom
feat/embed-external-user-id
Apr 28, 2026
Merged

feat: use external_user_id in embed to save conversation#9304
AdityaHegde merged 3 commits into
mainfrom
feat/embed-external-user-id

Conversation

@AdityaHegde
Copy link
Copy Markdown
Collaborator

We recently added support for external_user_id to scope conversations in embed context. This PR adds support for it in the app by saving the recently open conversation by external_user_id.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@AdityaHegde AdityaHegde requested review from a team as code owners April 27, 2026 10:54
Copy link
Copy Markdown
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

Backend looks good

Copy link
Copy Markdown
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

Don't put externalUserId on RuntimeClient

RuntimeClient is our connection/transport abstraction: host, instanceId, JWT, transport, request queue. After this PR it also carries an embed-only string that nothing on the runtime side reads — it's stored in the constructor and consumed in exactly one place (BrowserStorageConversationSelector) to namespace a sessionStorage key.

Suggest passing the user id directly into the selector instead. In ConversationManager, construct BrowserStorageConversationSelector({ userId: EmbedStore.getInstance()?.externalUserId }). The selector no longer depends on RuntimeClient, and RuntimeClient, RuntimeProvider, and getRuntimeClient revert to their pre-PR shape.


Developed in collaboration with Claude Code

@AdityaHegde AdityaHegde force-pushed the feat/embed-external-user-id branch from 40656a7 to 8d9b13b Compare April 28, 2026 12:27
@AdityaHegde AdityaHegde requested a review from ericpgreen2 April 28, 2026 12:27
Copy link
Copy Markdown
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

Rename the param to signal intent

userId: string | null reads as too generic at the call site — a reader sees new BrowserStorageConversationSelector(EmbedStore.getInstance()?.externalUserId ?? null) and reasonably asks "why does conversation selection know about users?" The selector isn't really using a user id in any general sense; it's using an optional namespace for the sessionStorage key that happens to be filled with the embed external user id today. A name like scopeId (or userScopeId) honors what the parameter actually does without overcommitting to embed semantics.

Add a constructor JSDoc

One sentence explaining when callers should pass it. For example:

/**
 * @param scopeId Optional namespace for the sessionStorage key. Pass when the
 * same browser tab may serve multiple users (e.g. embed with `external_user_id`)
 * so conversation selection doesn't leak between them.
 */
constructor(scopeId: string | null) {

The class-level docstring's "Selection is scoped per project" line should also pick up "...optionally per user."


Developed in collaboration with Claude Code

@AdityaHegde AdityaHegde merged commit 3baadb2 into main Apr 28, 2026
27 of 32 checks passed
@AdityaHegde AdityaHegde deleted the feat/embed-external-user-id branch April 28, 2026 15:12
AdityaHegde added a commit that referenced this pull request Apr 28, 2026
* feat: use external_user_id in embed to save conversation

* Do not pass through runtime client

* Improve var name and add doc string
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.

3 participants