fix(cli): detect expired token on deploy and offer interactive re-auth#21335
Open
Manibharadwaj wants to merge 7 commits into
Open
fix(cli): detect expired token on deploy and offer interactive re-auth#21335Manibharadwaj wants to merge 7 commits into
Manibharadwaj wants to merge 7 commits into
Conversation
When the active remote's API key is no longer valid (DB reset, key
revocation, workspace deletion), 'twenty deploy' / 'twenty dev' failed
with a generic 'Upload failed: Token has expired.' message and no
remediation path. The user had to know to manually re-run
'remote add' with a freshly minted key.
This change:
- Tags 401 responses from FileApi.uploadAppTarball with isAuthError
so callers can detect auth failures without string-matching.
- Adds an optional isAuthError field to FailingApiResponse (additive,
no breaking change for existing call sites).
- Adds a 'reauth-helper' utility with a token-expired matcher and a
TTY-gated interactive prompt that surfaces the exact remediation
command and the Settings -> Developers link.
- Wires the helper into appDeploy: on auth failure, prints the
better message, prompts the user, and retries the upload once
if the user confirms. In non-TTY contexts (CI), the prompt is
suppressed and the error is surfaced as before, preserving
scriptable behavior.
- Adds a unit test covering the token-expired matcher.
Closes twentyhq#20197
|
👋 Thanks for contributing to Twenty! Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run. Looking forward to your contribution! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-sdk/src/cli/utilities/auth/__tests__/reauth-helper.test.ts">
<violation number="1" location="packages/twenty-sdk/src/cli/utilities/auth/__tests__/reauth-helper.test.ts:8">
P2: This test expects `Unauthenticated` to match, but the current matcher does not include that string, so the test is invalid and will fail.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| 'Token has expired.', | ||
| 'Upload failed: Token has expired.', | ||
| 'Unauthorized', | ||
| 'Unauthenticated', |
Contributor
There was a problem hiding this comment.
P2: This test expects Unauthenticated to match, but the current matcher does not include that string, so the test is invalid and will fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-sdk/src/cli/utilities/auth/__tests__/reauth-helper.test.ts, line 8:
<comment>This test expects `Unauthenticated` to match, but the current matcher does not include that string, so the test is invalid and will fail.</comment>
<file context>
@@ -0,0 +1,34 @@
+ 'Token has expired.',
+ 'Upload failed: Token has expired.',
+ 'Unauthorized',
+ 'Unauthenticated',
+ 'Invalid API key',
+ 'invalid api key provided',
</file context>
Contributor
Author
|
Taking few moments for review and fix it |
…olation The unit test was importing from reauth-helper which pulls in inquirer, chalk, ConfigService, and ApiService — side-effect dependencies that break in the vitest node environment. This extracts isTokenExpiredMessage into a standalone pure module (token-expired-detector.ts) with zero side-effect imports, so the test runs cleanly without mocking.
The cubic-dev-ai reviewer flagged that Unauthenticated was in the test but not in the regex. Added unauthenticated as a match variant since some servers return 401 with Unauthenticated instead of Unauthorized.
🔍 Automated Pre-Review✅ No issues detected - This PR is ready for human review. 🧭 External PR Quality Review🟠 Needs triage for the following reason(s):
cc @prastoin Checks
Detailed findings (duplicate candidates, standards notes, summary) are in the workflow run logs. Automated pre-review — human approval still required. |
…ctor Break long import line to comply with printWidth=80 and oxfmt singleQuote/trailingComma rules.
Manibharadwaj
force-pushed
the
fix/cli-expired-token-interactive-reauth
branch
from
485da99 to
98f531d
Compare
martmull
requested changes
Jun 10, 2026
martmull
left a comment
martmull
left a comment
Contributor
There was a problem hiding this comment.
hey, nice stuff, i added some comments.
I suggest you to also use this reauthentication prompt for all api calls
|
|
||
| export type ReauthOutcome = 'reauthenticated' | 'declined' | 'non-interactive'; | ||
|
|
||
| const isInteractive = (): boolean => process.stdin.isTTY === true; |
Contributor
There was a problem hiding this comment.
lets avoid this we don't use it anywhere
Comment on lines
+28
to
+35
| `Your API key for remote "${remoteName}" is no longer valid`, | ||
| '(the workspace may have been reset, or the key was revoked).', | ||
| '', | ||
| 'Re-authenticate with:', | ||
| ` twenty remote:add --as ${remoteName} --api-key <NEW_KEY>`, | ||
| '', | ||
| `Generate a new key at: ${apiUrl}/settings/developers`, | ||
| ].join('\n'); |
Contributor
There was a problem hiding this comment.
Suggested change
| `Your API key for remote "${remoteName}" is no longer valid`, | |
| '(the workspace may have been reset, or the key was revoked).', | |
| '', | |
| 'Re-authenticate with:', | |
| ` twenty remote:add --as ${remoteName} --api-key <NEW_KEY>`, | |
| '', | |
| `Generate a new key at: ${apiUrl}/settings/developers`, | |
| ].join('\n'); | |
| `Authentication failed on remote "${remoteName}"` |
I think authentication failed log is enough as you prompt to open the browser right after
| const { authValid } = await apiService.validateAuth(); | ||
|
|
||
| if (authValid) { | ||
| console.log(chalk.green(`✓ Remote "${remoteName}" is still valid.`)); |
Contributor
There was a problem hiding this comment.
Suggested change
| console.log(chalk.green(`✓ Remote "${remoteName}" is still valid.`)); | |
| console.log(chalk.green(`✓ Authenticated to "${remoteName}".`)); |
|
|
||
| console.log( | ||
| chalk.yellow( | ||
| `Re-run: twenty remote:add --as ${remoteName} --api-key <NEW_KEY>`, |
Contributor
There was a problem hiding this comment.
Consistency with other logs
Suggested change
| `Re-run: twenty remote:add --as ${remoteName} --api-key <NEW_KEY>`, | |
| `yarn twenty remote:add`, |
Comment on lines
+49
to
+51
| console.log( | ||
| chalk.gray(`Generate a new key at: ${config.apiUrl}/settings/developers`), | ||
| ); |
Contributor
There was a problem hiding this comment.
Suggested change
| console.log( | |
| chalk.gray(`Generate a new key at: ${config.apiUrl}/settings/developers`), | |
| ); |
| const errorMessage = | ||
| typeof uploadResult.error === 'string' ? uploadResult.error : ''; | ||
|
|
||
| if (uploadResult.isAuthError || isTokenExpiredMessage(errorMessage)) { |
Contributor
There was a problem hiding this comment.
Suggested change
| if (uploadResult.isAuthError || isTokenExpiredMessage(errorMessage)) { | |
| const { authValid } = await apiService.validateAuth(); | |
| if (uploadResult.isAuthError || !authValid) { |
| const TOKEN_EXPIRED_PATTERN = | ||
| /token has expired|unauthori[sz]ed|unauthenticated|invalid api key/i; | ||
|
|
||
| export const isTokenExpiredMessage = ( |
Contributor
There was a problem hiding this comment.
lets remove this and use authValid
| @@ -0,0 +1,34 @@ | |||
| import { isTokenExpiredMessage } from '../token-expired-detector'; | |||
|
|
|||
| describe('isTokenExpiredMessage', () => { | |||
Contributor
There was a problem hiding this comment.
lets remove as we suggest to remove isTokenExpiredMessage method
Comment on lines
+62
to
+77
| if (uploadResult.isAuthError || isTokenExpiredMessage(errorMessage)) { | ||
| const remoteName = ConfigService.getActiveRemote(); | ||
| const configService = new ConfigService(); | ||
| const config = await configService.getConfigForRemote(remoteName); | ||
|
|
||
| console.error(formatAuthError(config.apiUrl, remoteName)); | ||
| const outcome = await promptForReauthentication(remoteName); | ||
|
|
||
| if (outcome === 'reauthenticated') { | ||
| const retryResult = await apiService.uploadAppTarball({ tarballBuffer }); | ||
|
|
||
| if (retryResult.success) { | ||
| return { success: true, data: retryResult.data }; | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
we should add this logic directly in the packages/twenty-sdk/src/cli/utilities/api/api-client.ts service so we call promptForReauthentication for each api call and not only deploy.
…reauth to api-client - Remove token-expired-detector.ts and its test (per review request) Use apiService.validateAuth() / authValid instead of regex-based detection - Simplify deploy.ts: use isAuthError only (removed isTokenExpiredMessage), simplified auth error message to 'Authentication failed on remote' - Add promptForReauthentication to api-client.ts 401 interceptor so re-auth works for ALL API calls, not just deploy - Clean up reauth-helper.ts: removed token-expired-detector import/export, simplified success message, consistent log style with chalk.green
Contributor
Author
|
Hey @martmull 👋 Addressed all your review feedback:
Net change: -85 lines — cleaner and re-auth now works globally across all CLI commands. Ready for re-review whenever you get a chance. Thanks! 🙏 |
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-sdk/src/cli/utilities/auth/__tests__/reauth-helper.test.ts">
<violation number="1" location="packages/twenty-sdk/src/cli/utilities/auth/__tests__/reauth-helper.test.ts:8">
P2: This test expects `Unauthenticated` to match, but the current matcher does not include that string, so the test is invalid and will fail.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Fix oxfmt formatting in deploy.ts and reauth-helper.ts (CI lint failure) - Add _reauthAttempted flag on axios config to prevent interceptor recursion when 401 retry also returns 401 - Use disableInterceptors:true in reauth-helper.ts validateAuth() call to prevent interceptor firing during reauth validation - Add authValid check in deploy.ts as fallback for GraphQL auth errors that don't set isAuthError (replaces removed isTokenExpiredMessage) - Change 'twenty remote:add' to 'yarn twenty remote:add' for consistency - Remove unused ConfigService import and redundant help text in reauth-helper.ts Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-sdk/src/cli/utilities/auth/__tests__/reauth-helper.test.ts">
<violation number="1" location="packages/twenty-sdk/src/cli/utilities/auth/__tests__/reauth-helper.test.ts:8">
P2: This test expects `Unauthenticated` to match, but the current matcher does not include that string, so the test is invalid and will fail.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Prevents crash when error.config is undefined, which could mask the original auth error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
I have read the CONTRIBUTING.md file.
YES
What kind of change does this PR introduce?
Fix (CLI) —
twenty deploynow detects an expired/invalid API key on the active remote and offers an interactive re-auth flow (TTY only). In non-TTY contexts the behavior is unchanged: a clear error and a non-zero exit.Fixes #20197
What is the current behavior?
After a workspace DB reset, key revocation, or workspace deletion, both
twenty deployand (effectively)twenty devfail with:The message is technically correct but gives the user no way forward. They have to know to mint a new key from Settings → Developers and re-run
twenty remote add --local --api-key <NEW_KEY>. This came up while testing PR #20181 and is the same friction on any DB reset, key revocation, or workspace deletion.What is the new behavior?
Two changes, layered:
(1) Better error message + remediation hint
When the upload returns a 401 or its message matches a token-expired pattern (
/token has expired|unauthori[sz]ed|invalid api key/i),appDeploynow prints:(2) Interactive re-auth prompt (TTY only)
If the process is attached to a TTY, after the hint is printed the user is prompted:
remote:add. The originalappDeployis then retried once.DEPLOY_FAILEDerror is surfaced (same code, better message).Acceptance criteria
Reproduction
twenty deploy— confirm the happy path.core.appTokencleared) and re-runtwenty deploy— confirm the new hint + prompt fire and the retry succeeds.twenty deploy < /dev/nullor viascript -qc '') — confirm the prompt is suppressed and the scriptable exit-1 behavior is preserved.Implementation notes
FileApi.uploadAppTarballnow tags 401 responses with anisAuthError: trueflag on the failingApiResponse. The existingerrorstring is still populated so callers that don't check the flag continue to work — additive, no breaking change.FailingApiResponse<TError>gained an optionalisAuthError?: booleanfield. The otherApiResponsecall sites in the SDK don't need to set it.@/cli/utilities/auth/reauth-helper.tsis new. It owns:isTokenExpiredMessage(...)— pure matcher, easy to unit-test, used as a backstop if a non-401 message still says "expired" (GraphQL returns 200 with errors in some cases).promptForReauthentication(remoteName)— TTY-gatedinquirer.confirmprompt that re-validates the token and either returns'reauthenticated','declined', or'non-interactive'.@/cli/operations/deploy.tsis the single call site that wires the helper. The helper is structured so it can be reused from the dev orchestrator's upload step (a follow-up) without changes.__tests__/reauth-helper.test.tscovers the matcher: positive cases, negative cases, case-insensitivity, and nullish input.Out of scope (per the issue)
--localremotes.authenticate(...)flow inremote.tsis fine; the prompt here just tells the user to re-run it).Files changed
Happy to address feedback and split this into two PRs (hint-only first, prompt-on-top) if the maintainers prefer a smaller first cut.