-
Notifications
You must be signed in to change notification settings - Fork 4
fix(auth): explain managed-workspace claim rejections during login #471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Zertsov
wants to merge
5
commits into
main
Choose a base branch
from
voz/aie-1668-cli-autoclaim-show-the-managed-workspace-403-instead-of-no
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
717058a
fix(auth): explain managed-workspace claim rejections during login
Zertsov 30f2445
Apply suggestion from @thiskevinwang
Zertsov 02c67ee
fix(auth): close the string in the managed-workspace next steps
Zertsov 6ed181b
refactor(auth): print the managed-workspace API message verbatim
Zertsov 5111a80
test(auth): match the generic managed-workspace API message
Zertsov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "clerk": patch | ||
| --- | ||
|
|
||
| `clerk auth login` now explains why a keyless application could not be claimed when the active workspace is managed by an integration such as Vercel Marketplace or Stripe. When the Platform API rejects the claim with the error code `accountless_application_managed_workspace`, the CLI prints the API message, for example "Unable to claim - The target application cannot be claimed into the current workspace. Select a different workspace and try again.", instead of the generic "no active organization" warning, and keeps the local claim token so the next `clerk auth login` from a workspace you own claims the application. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { test, expect, describe, afterEach, beforeEach, mock, spyOn } from "bun:test"; | ||
| import { AuthError } from "../../lib/errors.ts"; | ||
| import { useCaptureLog, credentialStoreStubs, configStubs } from "../../test/lib/stubs.ts"; | ||
| import type { AutoclaimResult } from "../../lib/autoclaim.ts"; | ||
|
|
||
| const actualConstants = await import("../../lib/constants.ts"); | ||
| const actualEnvironment = await import("../../lib/environment.ts"); | ||
|
|
@@ -20,6 +21,9 @@ const mockIsHuman = mock(); | |
| const mockConfirm = mock(); | ||
| const mockOpenBrowser = mock(); | ||
| const mockEnsureFirstApplication = mock<() => Promise<void>>(() => Promise.resolve()); | ||
| const mockAttemptAutoclaim = mock<() => Promise<AutoclaimResult>>(() => | ||
| Promise.resolve({ status: "not_keyless" }), | ||
| ); | ||
|
|
||
| mock.module("../../lib/credential-store.ts", () => ({ | ||
| ...credentialStoreStubs, | ||
|
|
@@ -93,7 +97,7 @@ mock.module("../../lib/first-application.ts", () => ({ | |
| })); | ||
|
|
||
| mock.module("../../lib/autoclaim.ts", () => ({ | ||
| attemptAutoclaim: async () => ({ status: "not_keyless" }), | ||
| attemptAutoclaim: () => mockAttemptAutoclaim(), | ||
| })); | ||
|
|
||
| const { setLogLevel } = await import("../../lib/log.ts"); | ||
|
|
@@ -127,6 +131,8 @@ describe("login", () => { | |
| mockOpenBrowser.mockReset(); | ||
| mockEnsureFirstApplication.mockReset(); | ||
| mockEnsureFirstApplication.mockResolvedValue(undefined); | ||
| mockAttemptAutoclaim.mockReset(); | ||
| mockAttemptAutoclaim.mockResolvedValue({ status: "not_keyless" }); | ||
| mockIsHuman.mockReturnValue(false); | ||
| mockOpenBrowser.mockResolvedValue({ ok: true, launcher: "test" }); | ||
| mockRevokeToken.mockResolvedValue("revoked"); | ||
|
|
@@ -588,6 +594,38 @@ describe("login", () => { | |
| expect(mockEnsureFirstApplication).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| test("explains a managed-workspace claim rejection with the provider named by the API", async () => { | ||
| mockGetValidToken.mockResolvedValue("existing-token"); | ||
| mockGetAuth.mockResolvedValue({ userId: "user_123" }); | ||
| mockFetchUserInfo.mockResolvedValue({ userId: "user_123", email: "[email protected]" }); | ||
| mockAttemptAutoclaim.mockResolvedValue({ | ||
| status: "managed_workspace", | ||
| longMessage: | ||
| "The target application cannot be claimed into the current workspace. Select a different workspace and try again.", | ||
| }); | ||
|
|
||
| await runLogin(); | ||
|
|
||
| expect(captured.err).toContain( | ||
| "Unable to claim - The target application cannot be claimed into the current workspace. Select a different workspace and try again.", | ||
| ); | ||
| expect(captured.err).not.toContain("claim the application there"); | ||
| expect(captured.err).not.toContain("does not have an active organization"); | ||
| }); | ||
|
|
||
| test("falls back to a generic provider when the managed-workspace rejection has no long message", async () => { | ||
| mockGetValidToken.mockResolvedValue("existing-token"); | ||
| mockGetAuth.mockResolvedValue({ userId: "user_123" }); | ||
| mockFetchUserInfo.mockResolvedValue({ userId: "user_123", email: "[email protected]" }); | ||
| mockAttemptAutoclaim.mockResolvedValue({ status: "managed_workspace", longMessage: null }); | ||
|
|
||
| await runLogin(); | ||
|
|
||
| expect(captured.err).toContain( | ||
| "Unable to claim - this workspace is managed by an integration provider.", | ||
| ); | ||
| }); | ||
|
|
||
| test("does not call ensureFirstApplication when existing session is reused", async () => { | ||
| mockGetValidToken.mockResolvedValue("existing-token"); | ||
| mockGetAuth.mockResolvedValue({ userId: "user_123" }); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: clerk/cli
Length of output: 23971
Handle blank
longMessagevalues before interpolation.AutoclaimResultallowslongMessage: string | null, and API parsing does not reject blank strings. A blank value bypasses??and producesUnable to claim -without guidance. Use the generic fallback for blank values while preserving non-blank messages.🤖 Prompt for AI Agents