fix(site): defer external app API key generation to on-click - #27907
fix(site): defer external app API key generation to on-click#27907jakehwll wants to merge 3 commits into
Conversation
…in-useapplink-to-on-click-instead
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f150788c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| generateKeyMutation.mutate(undefined, { | ||
| onSuccess: ({ key }) => { | ||
| notifyOnOpenExternalAppFailed(); | ||
| location.href = buildHref(key); | ||
| }, |
There was a problem hiding this comment.
Keep token callbacks outside closing menu items
In the TaskApps and WorkspacePill dropdowns, selecting an item closes and unmounts ExternalAppMenuItem or AppMenuItem; when the POST outlives that close, TanStack Query drops the per-call callbacks passed to mutate after the observer loses its listeners. The key is then minted, but neither the success navigation nor the failure toast runs. Put these callbacks in the useMutation options or hoist the mutation above the dropdown so token-bearing apps open reliably and errors remain visible.
AGENTS.md reference: site/AGENTS.md:L16-L17
Useful? React with 👍 / 👎.
| // For apps that require a session token this href intentionally omits the | ||
| // token; the `onClick` handler mints one and navigates to the final URL. | ||
| // Callers still render it as an anchor for apps that don't need a token. | ||
| const href = buildHref(""); |
There was a problem hiding this comment.
Do not expose a tokenless href for token-backed apps
For an app that requires a session token, this removes $SESSION_TOKEN and still exposes the result as a real anchor URL. Middle-clicking, choosing the browser's native Open Link action, or otherwise following the anchor without React's onClick launches the custom protocol with an empty token, so the app cannot authenticate and no key is generated. Token-backed actions should not expose the incomplete URL as a navigable href.
Useful? React with 👍 / 👎.
| const getApiKey = spyOn(API, "getApiKey").mockImplementation( | ||
| () => new Promise(() => {}), | ||
| ); |
There was a problem hiding this comment.
Install the API spy before rendering the story
Storybook invokes play after the component has rendered and its effects have run, so installing the spy here cannot observe API calls made during mounting. A regression back to eager key fetching can therefore issue getApiKey() before this spy exists, while the subsequent not.toHaveBeenCalled() assertion still passes. Set up the mock before rendering so this story actually covers the deferred-fetch invariant required by FE1.
AGENTS.md reference: site/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
Resolves DEVEX-460.
Problem
useAppLinkminted a session key on mount via auseQuery:Whenever any page mounted
useAppLinkfor an external app that embeds$SESSION_TOKENin its URL (JetBrains Gateway, Coder Desktop, etc.), it firedPOST /api/v2/users/me/keyson render, even if the user never clicked the link. Each call minted a fresh session key and produced acreated tokenaudit-log entry. Simply navigating the dashboard generated a stream ofcreated tokenentries with no real connection activity.This is the follow-up to #22318 (AIGOV-24), which only fixed the built-in VS Code / VS Code Insiders buttons in
WorkspacesTable.Change
Mirror the on-click minting pattern from #22318, applied to the shared
useAppLinkhook:useQuery(apiKey())with auseMutation(() => API.getApiKey())that runs only when the user clicks a token-bearing external app.onClickmints the key, builds the final URL, and navigates vialocation.href(these are always custom-protocol, non-HTTP external apps).hrefno longer embeds a token; the hook owns opening for token apps.hasTokenfield withisLoading(mint in-flight) and update call sites so token apps are always clickable, with a loading affordance while a key is being minted.TaskAppsexternal-app menu items now render an anchor with the hook'sonClick(instead of aRouterLinkthat bypassed it), so the deferred mint runs there too.No API key is minted until the user actually opens an external app.
Affected call sites
site/src/modules/apps/useAppLink.ts(core change)site/src/modules/resources/AppLink/AppLink.tsxsite/src/pages/AgentsPage/components/WorkspacePill.tsxsite/src/pages/TaskPage/TaskApps.tsxTesting
Added Storybook interaction tests (
playfunctions) inAppLink.stories.tsxthat assert the fix directly:ExternalAppDefersSessionToken: a token-bearing external app mints no key on render, and mints exactly one on click.ExternalAppWithoutSessionTokenNeverMints: an external app without$SESSION_TOKENnever mints a key, even on click.Also verified:
tsc -p .(frontend typecheck) passesbiome checkon changed files passesvitestforAppLink(incl. new stories),apps,WorkspacePill,TaskApps, andAppStatusesstories/tests passManual verification checklist (recommended before merge)
$SESSION_TOKEN(e.g. JetBrains Gateway), open DevTools → Network filtered onusers/me/keys, reload/workspacesand the workspace detail page, and confirm noPOSTfires on render.POSTfires only when the app link is clicked, and the app opens correctly.