Skip to content

UX-913: silently refresh embedded auth on stale tabs#2328

Open
malinskibeniamin wants to merge 1 commit intomasterfrom
codex/ux-913-silent-auth-refresh
Open

UX-913: silently refresh embedded auth on stale tabs#2328
malinskibeniamin wants to merge 1 commit intomasterfrom
codex/ux-913-silent-auth-refresh

Conversation

@malinskibeniamin
Copy link
Copy Markdown
Contributor

@malinskibeniamin malinskibeniamin commented Mar 19, 2026

What

  • add a shared Connect token refresh interceptor plus shared embedded auth error notification helpers
  • route embedded Connect transports through the configured fetch path so MF v1 hosts can recover stale bearer tokens through their existing authenticated fetch
  • wire MF v2 TokenManager refresh handling into setup-created clients, router/root transports, AI Gateway transports, and controlplane transports
  • prewarm embedded auth on focus / visibilitychange so stale tabs refresh before refetches surface rendering errors
  • add targeted unit and integration coverage for the shared refresh flow, MF v1 fetch usage, MF v2 prewarm, and hook transports

Why

When an embedded Console tab sits stale long enough for its bearer token to expire, coming back to it can trigger unauthenticated failures and confusing frontend stack traces. This change makes stale-tab recovery silent when possible and falls back to the existing embedded re-auth handoff when it is not.

How

  • extract createTokenRefreshInterceptor() into a shared utility and distribute it through a small context/registry layer
  • in MF v2, build the interceptor from TokenManager.refresh() and register/provide it anywhere embedded Connect transports are created
  • in MF v1, keep the current host API intact and make the embedded Connect transports use config.fetch so host-provided authenticated fetch implementations can refresh/retry requests
  • add a debounced embedded auth prewarm hook that refreshes auth/user state and invalidates active queries/router state after the tab becomes active again
  • centralize embedded auth failure signaling through console:auth-error

Acceptance Criteria

  • stale embedded bearer tokens are silently refreshed before retrying Connect requests
  • stale-tab recovery works for both module federation v1 and module federation v2
  • AI Gateway, controlplane, and setup-created shared clients use the same recovery behavior
  • users no longer hit raw rendering errors / stack traces during a recoverable stale-token flow
  • unrecoverable refresh failures still hand off cleanly to the existing embedded re-login flow
  • no required host-facing API changes were introduced

</TransportProvider>
</ChakraProvider>
</CustomFeatureFlagProvider>
<TokenRefreshInterceptorProvider value={tokenRefreshInterceptor}>
Copy link
Copy Markdown
Contributor Author

@malinskibeniamin malinskibeniamin Mar 23, 2026

Choose a reason for hiding this comment

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

module federation v1 vs v2. token refresh interceptor provider needs to work in both

* Focus and visibility events usually arrive in a burst, so this hook debounces
* them and also deduplicates in-flight work.
*/
export const useEmbeddedAuthPrewarm = ({
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

seems overkill

@malinskibeniamin malinskibeniamin force-pushed the codex/ux-913-silent-auth-refresh branch from 4a6c72a to 9f0bf8e Compare March 24, 2026 09:22
@malinskibeniamin malinskibeniamin force-pushed the codex/ux-913-silent-auth-refresh branch from 9f0bf8e to 94f7e19 Compare March 24, 2026 09:22
@malinskibeniamin
Copy link
Copy Markdown
Contributor Author

@claude review, make sure this works for both module federation v1 and v2

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

Claude finished @malinskibeniamin's task in 3m 0s —— View job


Review: UX-913 Silent Auth Refresh (MF v1 & v2)

  • Read CLAUDE.md for repo guidelines
  • Read all changed files
  • Analyze MF v1 path (embedded-app.tsx)
  • Analyze MF v2 path (federation/console-app.tsx)
  • Review shared utilities
  • Review hook transports
  • Review test coverage
  • Post review findings

Architecture: How v1 vs v2 Differ

The two paths use fundamentally different refresh strategies, which is correct given their different host contracts:

MF v1 (embedded-app.tsx) MF v2 (federation/console-app.tsx)
Refresh mechanism Host's config.fetch handles refresh transparently Explicit TokenManager.refresh() + createTokenRefreshInterceptor
tokenRefreshInterceptor Always undefined (nothing calls setRegisteredTokenRefreshInterceptor) Created from TokenManager, registered globally
TokenRefreshInterceptorProvider value undefined The created interceptor
Prewarm strategy api.refreshUserData() (uses config.fetch) tokenManager.refresh()api.refreshUserData()

This asymmetry is intentional per the PR description, and both paths are coherent within their own contracts.


Issues Found

1. setup() memoization can produce a stale config.dataplaneTransport in MF v2

setup uses memoizeOne (config.ts:402). In MF v2, setRegisteredTokenRefreshInterceptor(tokenRefreshInterceptor) is called at console-app.tsx:187, then setup({...}) at line 191 — which calls setConfig, which reads getRegisteredTokenRefreshInterceptor() to build config.dataplaneTransport.

On unmount, the interceptor is cleared (setRegisteredTokenRefreshInterceptor(undefined) at line 207). On a subsequent remount with the same config args, memoizeOne skips setConfig, so config.dataplaneTransport keeps the old (orphaned) interceptor instance rather than the new one.

In practice configOverrides is likely a new object reference each mount (avoiding the issue), but it's an implicit assumption. api.refreshUserData() used during prewarm goes through config.dataplaneTransport, so this path is affected.

Fix this →

2. MF v1 refresh contract is implicit and untested

For v1 the entire "silent refresh" story depends on the host providing a fetch implementation that silently refreshes expired tokens and retries. This is not validated anywhere — if the host passes window.fetch (the default at embedded-app.tsx:96), stale-tab recovery is completely silent only for the host, not for Connect requests:

// embedded-app.tsx:96-97
const defaultFetch = useMemo(() => window.fetch.bind(window), []);
const configuredFetch = p.fetch ?? defaultFetch;

If no fetch prop is passed, v1 falls back to bare window.fetch with no token refresh, meaning the acceptance criterion "stale embedded bearer tokens are silently refreshed before retrying Connect requests" is not met for MF v1 when no custom fetch is provided.

The embedded-app test only asserts the custom fetch is wired — it doesn't cover the fallback case.

3. getRefreshedToken has a redundant fallback that can mask an empty-string token

In create-token-refresh-interceptor.ts:27:

const token = refreshedToken || getAccessToken();

refreshedToken comes directly from refreshAccessToken() which is tokenManager.refresh() — a function that already stores the token in config.jwt. If refreshedToken is "" (empty string, a falsy value), this silently falls back to getAccessToken() which reads config.jwt — potentially returning the same stale token. A strict null/undefined check would be safer:

const token = refreshedToken ?? getAccessToken();

Fix this →

4. Addressing the self-review comment at embedded-app.tsx:173

The TokenRefreshInterceptorProvider providing undefined in v1 is correct by design. useTokenRefreshInterceptor() correctly falls back through context → registry, both of which are undefined in v1. Hook transports (useAIGatewayTransport, useControlplaneTransport) then produce transports without a refresh interceptor, relying on config.fetch instead. The abstraction works for both.

5. Addressing the self-review comment at use-embedded-auth-prewarm.ts:16 — "seems overkill"

The dual guard (inFlightRef + lastRunAtRef/dedupeMs) serves distinct purposes:

  • inFlightRef blocks while a prewarm is running
  • dedupeMs blocks for 1 second after a prewarm completes

They cover different timing windows. The concern is valid if dedupeMs creates an invisible 1-second window where a legitimate re-activation is silently dropped. Since this is best-effort, this is acceptable — but the default of 1000ms could feel sluggish for a rapid unfocus/focus cycle. Not a bug.


Minor Notes

  • console-app.tsx:281: The prewarm test asserting refresh called 2× (init + prewarm) is correct and a nice behavioral contract.
  • use-embedded-auth-prewarm.ts correctly skips when visibilityState === 'hidden' on focus events (e.g. focus-without-visible from a DevTools panel).
  • notifyEmbeddedAuthError defaults to window.location.pathname. In MF v2 (memory history), this will be the host's URL, not the Console's internal route — which is probably correct for host-side re-auth redirect.
  • The TokenManager.reset() clearing refreshPromise on unmount is important to avoid the orphaned promise resolving after a new interceptor is in place.

Summary

The core architecture works correctly for both MF v1 and v2. The main concerns are: the implicit contract that MF v1 hosts must provide a refresh-capable fetch (worth documenting), the memoizeOne assumption in MF v2 (low risk but fragile), and the || vs ?? token fallback.
·

@malinskibeniamin malinskibeniamin requested review from a team, c-julin, graham-rp, jvorcak and yougotashovel and removed request for a team March 24, 2026 09:42
@malinskibeniamin malinskibeniamin self-assigned this Mar 24, 2026
@malinskibeniamin malinskibeniamin added bug Something isn't working frontend ui/ux labels Mar 24, 2026

function EmbeddedApp({ basePath = '', ...p }: EmbeddedProps) {
const tokenRefreshInterceptor = getRegisteredTokenRefreshInterceptor();
const defaultFetch = useMemo(() => window.fetch.bind(window), []);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we please provide more comments on how these fetch mechanism works across apps? It was never clear to me and it's a bit confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend ui/ux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants