Skip to content

Make authorizationUrl and launch exclusive#31

Merged
koistya merged 5 commits intokriasoft:mainfrom
mina-asham:fix-parameters
Jan 24, 2026
Merged

Make authorizationUrl and launch exclusive#31
koistya merged 5 commits intokriasoft:mainfrom
mina-asham:fix-parameters

Conversation

@mina-asham
Copy link
Contributor

mina-asham and others added 2 commits January 24, 2026 19:23
- With the release of version 2, we added the `launch` option to customize launching the `authorizationUrl`, but if `launch` is not provided then `authorizationUrl` shouldn't be provided as it's unused, this makes both options exclusive
- Types: explicit Headless vs Managed modes with `never` constraints
- Runtime: fix bug where `launch: undefined` caused crash (use typeof check)
- browser-auth: conditionally use managed/headless based on launch presence
- Add getRedirectUrl() helper for headless mode usability
- Tests: update to use valid patterns for each mode
@koistya
Copy link
Member

koistya commented Jan 24, 2026

Hey! Just pushed a follow-up commit that tightens things up a bit.

The core idea of your PR is solid — making these options exclusive makes the API clearer. I noticed a few rough edges while testing:

  1. Runtime bug: When browser-auth passes launch: this._launch and _launch is undefined, the "launch" in options check still returns true (key exists, value is undefined), which would try to call undefined(). Switched to typeof launch === "function" to be safe.

  2. Types: Made the two modes more explicit — Headless (no URL, no launch) vs Managed (both required). Added never constraints so the intent is clearer in the type definitions.

  3. browser-auth: Updated to conditionally build the options object based on whether _launch exists, so it properly uses headless mode when no launcher is configured.

  4. Usability: Added a small getRedirectUrl() helper so headless users can easily build the redirect URI for their auth URL without having to know the internal URL structure.

  5. Tests: Updated a few tests that were passing authorizationUrl without launch to use proper headless mode instead.

Let me know what you think.

Without `launch`, browserAuth ignores the authorization URL from the
MCP SDK, causing the callback server to wait indefinitely.

Updated examples and docs to consistently include `launch: open`.
@koistya koistya merged commit 30dbb3a into kriasoft:main Jan 24, 2026
5 checks passed
@mina-asham
Copy link
Contributor Author

Hey! Just pushed a follow-up commit that tightens things up a bit.

The core idea of your PR is solid — making these options exclusive makes the API clearer. I noticed a few rough edges while testing:

  1. Runtime bug: When browser-auth passes launch: this._launch and _launch is undefined, the "launch" in options check still returns true (key exists, value is undefined), which would try to call undefined(). Switched to typeof launch === "function" to be safe.
  2. Types: Made the two modes more explicit — Headless (no URL, no launch) vs Managed (both required). Added never constraints so the intent is clearer in the type definitions.
  3. browser-auth: Updated to conditionally build the options object based on whether _launch exists, so it properly uses headless mode when no launcher is configured.
  4. Usability: Added a small getRedirectUrl() helper so headless users can easily build the redirect URI for their auth URL without having to know the internal URL structure.
  5. Tests: Updated a few tests that were passing authorizationUrl without launch to use proper headless mode instead.

Let me know what you think.

Looks good to me, I have a couple of nit picks (sorry wasn't fast enough to comment before merge):

  • Probably worth pulling the defaults into constants now that they are used in two places (getRedirectUrl and getAuthCode)
  • We can strengthen this condition from: "authorizationUrl" in options && typeof (options as any).launch === "function" to "authorizationUrl" in options && options.authorizationUrl && "launch" in options && typeof options.launch === "function"

Thanks for fixing the rest of the bits I missed, love the use of never I felt I was missing something with my typing!

@koistya
Copy link
Member

koistya commented Jan 24, 2026

Both suggestions are reasonable improvements, indeed! Pushed in update in #32.

@koistya koistya changed the title Fix: Make authorizationUrl and launch exclusive Make authorizationUrl and launch exclusive Jan 24, 2026
@koistya koistya added the bug Something isn't working label Jan 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: V2 API shouldn't require authorizationUrl if launch is not supplied

2 participants