From 5314accfb0bdbe63d8e40566b4e87d1f3e2599c9 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Fri, 21 Aug 2026 13:32:45 +0100 Subject: [PATCH 01/14] feat: implement timeout and in-progress indicator --- packages/atlas-service/src/provider.tsx | 1 + .../src/store/atlas-signin-reducer.spec.ts | 100 ++++++++++++++---- .../src/store/atlas-signin-reducer.ts | 83 +++++++++++++-- .../store/atlas-signin-store-context.spec.tsx | 38 +++++++ .../src/store/atlas-signin-store-context.tsx | 7 +- .../components/atlas-tool-call-message.tsx | 44 ++++++-- .../tests/collection-indexes-tab.test.ts | 2 +- .../compass-telemetry/src/telemetry-events.ts | 17 +++ 8 files changed, 255 insertions(+), 37 deletions(-) diff --git a/packages/atlas-service/src/provider.tsx b/packages/atlas-service/src/provider.tsx index 7493abc2746..ffc4dd2c59b 100644 --- a/packages/atlas-service/src/provider.tsx +++ b/packages/atlas-service/src/provider.tsx @@ -71,6 +71,7 @@ export { useAtlasSignedInUser, useAtlasLoginActions, useIsAtlasSignInStateResolved, + useIsAtlasSignInInProgress, } from './store/atlas-signin-store-context'; export type { AtlasLoginActions } from './store/atlas-signin-store-context'; export { AtlasConnectionStatus } from './components/atlas-connection-status'; diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 3f9ea85b969..6d91e3dab5c 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -7,6 +7,7 @@ import { AttemptStateMap, performSignInAttempt, signOut, + SIGN_IN_TIMEOUT_MS, } from './atlas-signin-reducer'; import { expect } from 'chai'; import { configureStore } from './atlas-signin-store'; @@ -174,7 +175,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: mockAtlasService as any, }); - void store.dispatch(performSignInAttempt()).catch(() => {}); + void store.dispatch(performSignInAttempt()); // Give it some time for start the sign in attempt. It will be waiting // at isAuthenticated, which never resolves. @@ -186,8 +187,67 @@ describe('atlasSignInReducer', function () { }); }); + describe('sign in timeout', function () { + let clock: Sinon.SinonFakeTimers; + + beforeEach(function () { + clock = sandbox.useFakeTimers({ shouldAdvanceTime: true }); + }); + + afterEach(function () { + clock.restore(); + }); + + it('should time out and reset the state if the flow does not complete in time', async function () { + const isAuthenticatedStub = sandbox + .stub() + .callsFake(({ signal }: { signal: AbortSignal }) => { + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => { + reject(signal.reason); + }); + }); + }); + const store = configureStore({ + atlasAuthService: { isAuthenticated: isAuthenticatedStub } as any, + }); + + const attemptPromise = store.dispatch(performSignInAttempt()); + + await clock.tickAsync(0); + expect(store.getState()).to.have.nested.property('state', 'in-progress'); + + // Advance past the timeout. + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + + expect(store.getState()).to.have.nested.property('state', 'timed-out'); + expect(store.getState()).to.have.nested.property( + 'currentAttemptId', + null + ); + expect(await attemptPromise).to.deep.equal({ status: 'timed-out' }); + }); + + it('should not time out if the flow completes before the timeout', async function () { + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox.stub().resolves(false), + signIn: sandbox.stub().resolves({ sub: '1234' }), + getUserInfo: sandbox.stub().resolves({ sub: '1234' }), + } as any, + }); + + const result = await store.dispatch(performSignInAttempt()); + expect(result).to.have.property('status', 'success'); + expect(store.getState()).to.have.property('state', 'success'); + + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + expect(store.getState()).to.have.property('state', 'success'); + }); + }); + describe('performSignInAttempt', function () { - it('should resolve when sign in flow finishes', async function () { + it('should resolve with a success result when sign in flow finishes', async function () { const mockAtlasService = { isAuthenticated: sandbox.stub().resolves(false), signIn: sandbox.stub().resolves({ sub: '1234' }), @@ -196,11 +256,15 @@ describe('atlasSignInReducer', function () { const store = configureStore({ atlasAuthService: mockAtlasService as any, }); - await store.dispatch(performSignInAttempt()); + const result = await store.dispatch(performSignInAttempt()); + expect(result).to.deep.equal({ + status: 'success', + userInfo: { sub: '1234' }, + }); expect(store.getState()).to.have.property('state', 'success'); }); - it('should reject if sign in fails', async function () { + it('should resolve with an error result if sign in fails', async function () { const mockAtlasService = { isAuthenticated: sandbox.stub().resolves(false), signIn: sandbox.stub().rejects(new Error('Sign in failed')), @@ -209,16 +273,13 @@ describe('atlasSignInReducer', function () { const store = configureStore({ atlasAuthService: mockAtlasService as any, }); - try { - await store.dispatch(performSignInAttempt()); - expect.fail('Expected performSignInAttempt action to throw'); - } catch (err) { - expect(err).to.have.property('message', 'Sign in failed'); - } + const result = await store.dispatch(performSignInAttempt()); + expect(result).to.have.property('status', 'error'); + expect(result).to.have.nested.property('error.message', 'Sign in failed'); expect(store.getState()).to.have.property('state', 'error'); }); - it('should reject if provided signal was aborted', async function () { + it('should resolve with a canceled result if provided signal was aborted', async function () { let resolveSignInCalled = () => {}; const signInCalled: Promise = new Promise( (resolve) => (resolveSignInCalled = resolve) @@ -239,12 +300,7 @@ describe('atlasSignInReducer', function () { performSignInAttempt({ signal: c.signal }) ); c.abort(new Error('Aborted from outside')); - try { - await signInPromise; - throw new Error('Expected signInPromise to throw'); - } catch (err) { - expect(err).to.have.property('message', 'Aborted from outside'); - } + expect(await signInPromise).to.deep.equal({ status: 'canceled' }); expect(store.getState()).to.have.property('state', 'canceled'); // Ensure that we are not leaving a dangling store operation that would conflict with our mocks being reset. @@ -264,14 +320,18 @@ describe('atlasSignInReducer', function () { const firstAttemptPromise = store.dispatch(performSignInAttempt()); const secondAttemptPromise = store.dispatch(performSignInAttempt()); - const [firstUserInfo, secondUserInfo] = await Promise.all([ + const [firstResult, secondResult] = await Promise.all([ firstAttemptPromise, secondAttemptPromise, ]); - // the second call should not have triggered a second signIn call, and both should have the same userInfo + // the second call should not have triggered a second signIn call, and both should resolve to the same result expect(mockAtlasService.signIn).to.have.been.calledOnce; - expect(firstUserInfo).to.deep.equal(secondUserInfo); + expect(firstResult).to.deep.equal({ + status: 'success', + userInfo: { sub: '1234' }, + }); + expect(firstResult).to.deep.equal(secondResult); expect(store.getState()).to.have.property('state', 'success'); }); diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index 3a8f8e2d96f..a109972e16c 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -28,12 +28,19 @@ export type AtlasSignInState = { | 'unauthenticated' | 'in-progress' | 'error' - | 'canceled'; + | 'canceled' + | 'timed-out'; userInfo: null; } | { state: 'success'; userInfo: AtlasUserInfo } ); +export type SignInAttemptResult = + | { status: 'success'; userInfo: AtlasUserInfo } + | { status: 'timed-out' } + | { status: 'canceled' } + | { status: 'error'; error: Error }; + export type AtlasSignInThunkAction< R, A extends AnyAction = AnyAction @@ -55,6 +62,7 @@ export const enum AtlasSignInActions { Success = 'atlas-service/atlas-signin/AtlasSignInSuccess', Error = 'atlas-service/atlas-signin/AtlasSignInError', Cancel = 'atlas-service/atlas-signin/AtlasSignInCancel', + TimedOut = 'atlas-service/atlas-signin/AtlasSignInTimedOut', TokenRefreshFailed = 'atlas-service/atlas-signin/TokenRefreshFailed', SignedOut = 'atlas-service/atlas-signin/SignedOut', } @@ -106,6 +114,8 @@ export type AtlasSignInSignedOutAction = { export type AtlasSignInCancelAction = { type: AtlasSignInActions.Cancel }; +export type AtlasSignInTimedOutAction = { type: AtlasSignInActions.TimedOut }; + const INITIAL_STATE = { state: 'initial' as const, userInfo: null, @@ -120,8 +130,11 @@ type AttemptState = { promise: Promise; resolve: (userInfo: AtlasUserInfo) => void; reject: (reason?: any) => void; + timeoutId?: ReturnType; }; +export const SIGN_IN_TIMEOUT_MS = 10 * 1000; + // Exported for testing purposes only export const AttemptStateMap = new Map(); @@ -249,6 +262,12 @@ const reducer: Reducer = ( return { ...INITIAL_STATE, state: 'canceled' }; } + if ( + isAction(action, AtlasSignInActions.TimedOut) + ) { + return { ...INITIAL_STATE, state: 'timed-out' }; + } + if ( isAction( action, @@ -296,7 +315,10 @@ export const restoreSignInState = (): AtlasSignInThunkAction> => { }; }; -const startAttempt = (fn: () => void): AtlasSignInThunkAction => { +const startAttempt = ( + fn: () => void, + entrypoint: AtlasSignInEntrypoint +): AtlasSignInThunkAction => { return (dispatch, getState) => { if (getState().currentAttemptId) { throw new Error( @@ -305,8 +327,14 @@ const startAttempt = (fn: () => void): AtlasSignInThunkAction => { } const attempt = getAttempt(); dispatch({ type: AtlasSignInActions.AttemptStart, id: attempt.id }); + + attempt.timeoutId = setTimeout(() => { + dispatch(timeoutSignIn(attempt.id, entrypoint)); + }, SIGN_IN_TIMEOUT_MS); + attempt.promise .finally(() => { + clearTimeout(attempt.timeoutId); dispatch({ type: AtlasSignInActions.AttemptEnd, id: attempt.id }); }) .catch(() => { @@ -324,31 +352,49 @@ export const performSignInAttempt = ({ }: { signal?: AbortSignal; entrypoint?: AtlasSignInEntrypoint; -} = {}): AtlasSignInThunkAction> => { +} = {}): AtlasSignInThunkAction> => { return async (dispatch, getState, { track }) => { // Nothing to do if we already signed in const { state, userInfo, currentAttemptId } = getState(); if (state === 'success') { - return userInfo; + return { status: 'success', userInfo }; } if (currentAttemptId) { - return getAttempt(currentAttemptId).promise; + return toSignInAttemptResult( + getAttempt(currentAttemptId).promise, + getState + ); } track('Atlas Sign In Started', { entrypoint }); const attempt = dispatch( startAttempt(() => { void dispatch(signIn()); - }) + }, entrypoint) ); signal?.addEventListener('abort', () => { dispatch(cancelSignIn(signal.reason)); }); - return attempt.promise; + return toSignInAttemptResult(attempt.promise, getState); }; }; +async function toSignInAttemptResult( + promise: Promise, + getState: () => AtlasSignInState +): Promise { + try { + const userInfo = await promise; + return { status: 'success', userInfo }; + } catch (error) { + const state = getState().state; + if (state === 'timed-out') return { status: 'timed-out' }; + if (state === 'canceled') return { status: 'canceled' }; + return { status: 'error', error: error as Error }; + } +} + /** * Sign into Atlas. To be called when the user isn't signed in yet. */ @@ -406,12 +452,35 @@ export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { return; } const attempt = getAttempt(getState().currentAttemptId); + clearTimeout(attempt.timeoutId); attempt.controller.abort(); attempt.reject(reason ?? attempt.controller.signal.reason); dispatch({ type: AtlasSignInActions.Cancel }); }; }; +export const timeoutSignIn = ( + attemptId: number, + entrypoint: AtlasSignInEntrypoint +): AtlasSignInThunkAction => { + return (dispatch, getState, { track }) => { + if (getState().currentAttemptId !== attemptId) { + return; + } + const attempt = getAttempt(attemptId); + clearTimeout(attempt.timeoutId); + attempt.controller.abort(); + attempt.reject(new Error('Sign in timed out')); + openToast('atlas-disconnected', { + title: 'The login to Atlas has timed out, please try again.', + variant: 'note', + timeout: 5000, + }); + track('Atlas Sign In Timed Out', { entrypoint }); + dispatch({ type: AtlasSignInActions.TimedOut }); + }; +}; + export const tokenRefreshFailed = (): AtlasSignInThunkAction => { return (dispatch, _getState) => { dispatch({ type: AtlasSignInActions.TokenRefreshFailed }); diff --git a/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx b/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx index 63e3fc5e212..8d13c2a89c3 100644 --- a/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx +++ b/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx @@ -7,6 +7,7 @@ import { AtlasSignInStoreContext, useAtlasSignedInUser, useIsAtlasSignInStateResolved, + useIsAtlasSignInInProgress, } from './atlas-signin-store-context'; import { AtlasSignInActions } from './atlas-signin-reducer'; import { configureStore } from './atlas-signin-store'; @@ -25,6 +26,8 @@ function renderWithState(actions: AnyAction[]) { store, isResolved: renderHook(() => useIsAtlasSignInStateResolved(), { wrapper }) .result, + isInProgress: renderHook(() => useIsAtlasSignInInProgress(), { wrapper }) + .result, signedInUser: renderHook(() => useAtlasSignedInUser(), { wrapper }).result, }; } @@ -85,3 +88,38 @@ describe('useIsAtlasSignInStateResolved', function () { expect(isResolved.current).to.eq(false); }); }); + +describe('useIsAtlasSignInInProgress', function () { + const cases: [string, AnyAction[], boolean][] = [ + // only in-progress should return true + ['in-progress', [{ type: AtlasSignInActions.Start }], true], + ['initial', [], false], + ['restoring', RESTORING, false], + [ + 'unauthenticated', + [...RESTORING, { type: AtlasSignInActions.RestoringFailed }], + false, + ], + ['success', SUCCESS, false], + [ + 'error', + [ + { type: AtlasSignInActions.Start }, + { type: AtlasSignInActions.Error, error: 'Whoops!' }, + ], + false, + ], + ['canceled', [{ type: AtlasSignInActions.Cancel }], false], + ['timed-out', [{ type: AtlasSignInActions.TimedOut }], false], + ]; + + for (const [state, actions, expected] of cases) { + it(`should return ${String( + expected + )} for the '${state}' state`, function () { + const { store, isInProgress } = renderWithState(actions); + expect(store.getState()).to.have.property('state', state); + expect(isInProgress.current).to.eq(expected); + }); + } +}); diff --git a/packages/atlas-service/src/store/atlas-signin-store-context.tsx b/packages/atlas-service/src/store/atlas-signin-store-context.tsx index ed5d3b9a686..f94b0dcc615 100644 --- a/packages/atlas-service/src/store/atlas-signin-store-context.tsx +++ b/packages/atlas-service/src/store/atlas-signin-store-context.tsx @@ -2,6 +2,7 @@ import React, { useMemo } from 'react'; import { performSignInAttempt, signOut, + type SignInAttemptResult, type AtlasSignInState, } from './atlas-signin-reducer'; import type { ReactReduxContextValue, TypedUseSelectorHook } from 'react-redux'; @@ -54,11 +55,15 @@ export function useIsAtlasSignInStateResolved(): boolean { ); } +export function useIsAtlasSignInInProgress(): boolean { + return useSelector((state) => state.state === 'in-progress'); +} + export type AtlasLoginActions = { signOut: () => Promise; signIn: (opts?: { entrypoint?: AtlasSignInEntrypoint; - }) => Promise; + }) => Promise; }; export function useAtlasLoginActions(): AtlasLoginActions { diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx index 65693c2037c..3860729cf4c 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx @@ -24,6 +24,7 @@ import { useAtlasLoginActions, useAtlasSignedInUser, useIsAtlasSignInStateResolved, + useIsAtlasSignInInProgress, } from '@mongodb-js/atlas-service/provider'; import { CustomToolResult } from './custom-tool-result'; import { getToolCallTitle } from './tool-call-title'; @@ -69,6 +70,20 @@ This is read-only and won't change your cluster.`; ); } +function getApprovalMessage( + isSignInInProgress: boolean, + isUserSignedIn: boolean +) { + if (isSignInInProgress) { + return 'Connecting with Atlas to debug this connection'; + } + if (isUserSignedIn) { + return 'Run Atlas to debug this connection?'; + } + + return 'Connect with Atlas to debug this connection?'; +} + export const AtlasToolCallMessage: React.FunctionComponent< AtlasToolCallMessageProps > = ({ toolCall, connectionInfo, onApprove, onDeny }) => { @@ -80,6 +95,7 @@ export const AtlasToolCallMessage: React.FunctionComponent< const { signIn } = useAtlasLoginActions(); const track = useTelemetry(); const isSignInStateResolved = useIsAtlasSignInStateResolved(); + const isSignInInProgress = useIsAtlasSignInInProgress(); // The card re-renders on every state change, so we only report the prompt the // first time it's actually offered to a signed out user. We also wait for the @@ -119,7 +135,20 @@ export const AtlasToolCallMessage: React.FunctionComponent< const handleAtlasToolApproval = useCallback( (approvalId: string) => { signIn({ entrypoint: getSignInEntrypoint(toolCall.type) }) - .then((userInfo) => onApprove(approvalId, !!userInfo)) + .then((result) => { + switch (result.status) { + case 'success': + onApprove(approvalId, true); + break; + // If sign in timed out, give the user a new change instead of + // rejecting the tool + case 'timed-out': + break; + default: + onApprove(approvalId, false); + break; + } + }) .catch(() => onApprove(approvalId, false)); }, [signIn, onApprove, toolCall.type] @@ -148,19 +177,18 @@ export const AtlasToolCallMessage: React.FunctionComponent< toolDisplayName ); - // TODO(COMPASS-11044): update texts to be generic - const approvalMessage = isUserSignedIn - ? 'Run Atlas to debug this connection?' - : 'Connect with Atlas to debug this connection?'; - + const approvalMessage = getApprovalMessage( + !!isSignInInProgress, + isUserSignedIn + ); // TODO COMPASS-10973: don't render actions if there's no approvalId. return ( <> ; +/** + * This event is fired when the user does not complete the sign in to their Atlas + * account on time. + * + * @category Atlas + */ +type AtlasSignInTimedOutEvent = CommonEvent<{ + name: 'Atlas Sign In Timed Out'; + payload: { + /** + * The surface of the application the sign in was triggered from. + */ + entrypoint: AtlasSignInEntrypoint; + }; +}>; + /** * This event is fired when user signed out from their Atlas account. * @@ -4193,6 +4209,7 @@ export type TelemetryEvent = | AtlasLinkClickedEvent | AtlasSearchIndexesForViewLinkClickedEvent | AtlasSignInErrorEvent + | AtlasSignInTimedOutEvent | AtlasSignInPromptShownEvent | AtlasSignInStartedEvent | AtlasSignInSuccessEvent From 56982d4af92eb128cc766eb3ae90e5583c85c363 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Fri, 21 Aug 2026 13:50:10 +0100 Subject: [PATCH 02/14] add tests --- .../src/store/atlas-signin-reducer.spec.ts | 42 +++++++- .../atlas-tool-call-message.spec.tsx | 95 +++++++++++++++++-- 2 files changed, 128 insertions(+), 9 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 6d91e3dab5c..2e07b88a94a 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -189,6 +189,7 @@ describe('atlasSignInReducer', function () { describe('sign in timeout', function () { let clock: Sinon.SinonFakeTimers; + const ENTRYPOINT = 'assistant-tool-atlas-connection-error-debugger'; beforeEach(function () { clock = sandbox.useFakeTimers({ shouldAdvanceTime: true }); @@ -196,9 +197,10 @@ describe('atlasSignInReducer', function () { afterEach(function () { clock.restore(); + sandbox.restore(); }); - it('should time out and reset the state if the flow does not complete in time', async function () { + async function driveToTimeout() { const isAuthenticatedStub = sandbox .stub() .callsFake(({ signal }: { signal: AbortSignal }) => { @@ -208,33 +210,66 @@ describe('atlasSignInReducer', function () { }); }); }); + const track = sandbox.stub(); + const openToast = sandbox.stub(); + sandbox.replaceGetter(compassComponents, 'openToast', () => openToast); const store = configureStore({ atlasAuthService: { isAuthenticated: isAuthenticatedStub } as any, + track, }); - const attemptPromise = store.dispatch(performSignInAttempt()); + const attemptPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); await clock.tickAsync(0); expect(store.getState()).to.have.nested.property('state', 'in-progress'); // Advance past the timeout. await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + const result = await attemptPromise; + + return { store, track, openToast, result }; + } + + it('resets the state to timed out', async function () { + const { store, result } = await driveToTimeout(); expect(store.getState()).to.have.nested.property('state', 'timed-out'); expect(store.getState()).to.have.nested.property( 'currentAttemptId', null ); - expect(await attemptPromise).to.deep.equal({ status: 'timed-out' }); + expect(result).to.deep.equal({ status: 'timed-out' }); + }); + + it('tracks the timed out event with the entrypoint', async function () { + const { track } = await driveToTimeout(); + + expect( + track.withArgs('Atlas Sign In Timed Out', { entrypoint: ENTRYPOINT }) + ).to.have.been.calledOnce; + }); + + it('shows a toast informing the user that sign in timed out', async function () { + const { openToast } = await driveToTimeout(); + + expect(openToast.withArgs('atlas-disconnected')).to.have.been.calledOnce; + expect(openToast.lastCall.args[1]).to.include({ + title: 'The login to Atlas has timed out, please try again.', + variant: 'note', + }); }); it('should not time out if the flow completes before the timeout', async function () { + const track = sandbox.stub(); const store = configureStore({ atlasAuthService: { isAuthenticated: sandbox.stub().resolves(false), signIn: sandbox.stub().resolves({ sub: '1234' }), getUserInfo: sandbox.stub().resolves({ sub: '1234' }), } as any, + track, }); const result = await store.dispatch(performSignInAttempt()); @@ -243,6 +278,7 @@ describe('atlasSignInReducer', function () { await clock.tickAsync(SIGN_IN_TIMEOUT_MS); expect(store.getState()).to.have.property('state', 'success'); + expect(track).to.not.have.been.calledWith('Atlas Sign In Timed Out'); }); }); diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx index fa7ef945f1e..70843ada906 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx @@ -18,18 +18,41 @@ import { AtlasToolCallMessage } from './atlas-tool-call-message'; class FakeAtlasAuthService { private user: AtlasUserInfo | null; public signIn: sinon.SinonStub; + public resolveSignIn: () => void = () => {}; constructor({ signedIn = false, signInSucceeds = true, - }: { signedIn?: boolean; signInSucceeds?: boolean } = {}) { + deferSignIn = false, + }: { + signedIn?: boolean; + signInSucceeds?: boolean; + deferSignIn?: boolean; + } = {}) { this.user = signedIn ? { sub: 'user-1' } : null; this.signIn = sinon.stub().callsFake(() => { - if (!signInSucceeds) { - return Promise.reject(new Error('sign-in failed')); + const complete = () => { + if (!signInSucceeds) { + throw new Error('sign-in failed'); + } + this.user = { sub: 'user-1' }; + return this.user; + }; + if (!deferSignIn) { + return signInSucceeds + ? Promise.resolve(complete()) + : Promise.reject(new Error('sign-in failed')); } - this.user = { sub: 'user-1' }; - return Promise.resolve(this.user); + // Do not resolve immediately so we can test the in-progress flows + return new Promise((resolve, reject) => { + this.resolveSignIn = () => { + try { + resolve(complete()); + } catch (err) { + reject(err as Error); + } + }; + }); }); } @@ -73,13 +96,19 @@ describe('AtlasToolCallMessage', function () { { signedIn = false, signInSucceeds = true, - }: { signedIn?: boolean; signInSucceeds?: boolean } = {} + deferSignIn = false, + }: { + signedIn?: boolean; + signInSucceeds?: boolean; + deferSignIn?: boolean; + } = {} ) { const onApprove = sinon.stub(); const onDeny = sinon.stub(); const atlasAuthService = new FakeAtlasAuthService({ signedIn, signInSucceeds, + deferSignIn, }); const { renderWithConnections } = createPluginTestHelpers( // eslint-disable-next-line @typescript-eslint/no-unsafe-argument @@ -221,6 +250,60 @@ describe('AtlasToolCallMessage', function () { }); }); + describe('while sign in is in progress', function () { + async function startSignIn(opts: { signedIn?: boolean } = {}) { + const rendered = renderMessage({}, { ...opts, deferSignIn: true }); + const buttonLabel = opts.signedIn ? 'Run' : 'Connect to Atlas'; + await waitFor(() => { + expect(screen.getByText(buttonLabel)).to.exist; + }); + userEvent.click(screen.getByText(buttonLabel)); + // Wait for the store to move into the 'in-progress' state, surfaced as the + // connecting message. + await waitFor(() => { + expect( + screen.getByText('Connecting with Atlas to debug this connection') + ).to.exist; + }); + return rendered; + } + + it('shows the connecting message', async function () { + const { atlasAuthService, onApprove } = await startSignIn(); + + expect(screen.getByText('Connecting with Atlas to debug this connection')) + .to.exist; + + // Let the pending sign in settle so we don't leave a dangling attempt + atlasAuthService.resolveSignIn(); + await waitFor(() => { + expect(onApprove).to.have.been.called; + }); + }); + + it('hides the approval action buttons', async function () { + const { atlasAuthService, onApprove } = await startSignIn(); + + expect(screen.queryByText('Connect to Atlas')).to.not.exist; + expect(screen.queryByText('Skip')).to.not.exist; + + atlasAuthService.resolveSignIn(); + await waitFor(() => { + expect(onApprove).to.have.been.called; + }); + }); + + it('resolves the approval once sign in completes', async function () { + const { onApprove, atlasAuthService } = await startSignIn(); + + atlasAuthService.resolveSignIn(); + + await waitFor(() => { + expect(onApprove).to.have.been.calledOnceWith('approval-1', true); + }); + }); + }); + describe('resolved states', function () { it('shows "Ran" title and hides the action buttons when run', function () { const { container } = renderMessage( From 3561613eb48f607d9008745782b03b0c8f35a4b0 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Fri, 21 Aug 2026 15:24:18 +0100 Subject: [PATCH 03/14] add canceled event and retries tracking --- .../src/store/atlas-signin-reducer.spec.ts | 206 ++++++++++++++++++ .../src/store/atlas-signin-reducer.ts | 42 +++- .../atlas-tool-call-message.spec.tsx | 2 + .../compass-telemetry/src/telemetry-events.ts | 22 ++ 4 files changed, 261 insertions(+), 11 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 2e07b88a94a..88c44fbb5e3 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -171,8 +171,10 @@ describe('atlasSignInReducer', function () { const mockAtlasService = { isAuthenticated: isAuthenticatedStub, }; + const track = sandbox.stub(); const store = configureStore({ atlasAuthService: mockAtlasService as any, + track, }); void store.dispatch(performSignInAttempt()); @@ -182,8 +184,23 @@ describe('atlasSignInReducer', function () { await new Promise((resolve) => setTimeout(resolve, 100)); store.dispatch(cancelSignIn()); expect(store.getState()).to.have.nested.property('state', 'canceled'); + expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('previousOutcome', 'canceled'); expect(isAuthenticatedStub).to.have.been.calledOnce; + expect(track).to.have.been.calledWith('Atlas Sign In Canceled', {}); + }); + + it('should not track a cancel when no sign in is in progress', function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: {} as any, + track, + }); + + store.dispatch(cancelSignIn()); + + expect(track).to.not.have.been.calledWith('Atlas Sign In Canceled'); }); }); @@ -236,6 +253,8 @@ describe('atlasSignInReducer', function () { const { store, result } = await driveToTimeout(); expect(store.getState()).to.have.nested.property('state', 'timed-out'); + expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('previousOutcome', 'timed-out'); expect(store.getState()).to.have.nested.property( 'currentAttemptId', null @@ -313,6 +332,8 @@ describe('atlasSignInReducer', function () { expect(result).to.have.property('status', 'error'); expect(result).to.have.nested.property('error.message', 'Sign in failed'); expect(store.getState()).to.have.property('state', 'error'); + expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('previousOutcome', 'error'); }); it('should resolve with a canceled result if provided signal was aborted', async function () { @@ -390,6 +411,8 @@ describe('atlasSignInReducer', function () { ); expect(track).to.have.been.calledOnceWith('Atlas Sign In Started', { entrypoint: 'assistant-tool-atlas-connection-error-debugger', + attempt: 1, + previousOutcome: null, }); }); @@ -408,6 +431,8 @@ describe('atlasSignInReducer', function () { await store.dispatch(performSignInAttempt()); expect(track).to.have.been.calledOnceWith('Atlas Sign In Started', { entrypoint: 'unknown', + attempt: 1, + previousOutcome: null, }); }); @@ -428,6 +453,187 @@ describe('atlasSignInReducer', function () { }); }); + describe('sign in attempt tracking (retries)', function () { + let clock: Sinon.SinonFakeTimers; + const ENTRYPOINT = 'assistant-tool-atlas-connection-error-debugger'; + + beforeEach(function () { + clock = sandbox.useFakeTimers({ shouldAdvanceTime: true }); + sandbox.replaceGetter(compassComponents, 'openToast', () => + sandbox.stub() + ); + }); + + afterEach(function () { + clock.restore(); + sandbox.restore(); + }); + + function startedCalls(track: Sinon.SinonStub) { + return track + .getCalls() + .filter((call) => call.args[0] === 'Atlas Sign In Started') + .map((call) => call.args[1]); + } + + async function attemptThenTimeout(store: any) { + const attemptPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); + await clock.tickAsync(0); + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + await attemptPromise; + } + + it('first attempt, previousOutcome is null', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox.stub().resolves(false), + signIn: sandbox.stub().resolves({ sub: '1234' }), + getUserInfo: sandbox.stub().resolves({ sub: '1234' }), + } as any, + track, + }); + + await store.dispatch(performSignInAttempt({ entrypoint: ENTRYPOINT })); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + ]); + }); + + it('after a timeout, retrying increases attempt number and fill previousOutcome', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox + .stub() + .callsFake(({ signal }: { signal: AbortSignal }) => { + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason)); + }); + }), + } as any, + track, + }); + + await attemptThenTimeout(store); + + const secondPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); + await clock.tickAsync(0); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + { entrypoint: ENTRYPOINT, attempt: 2, previousOutcome: 'timed-out' }, + ]); + + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + await secondPromise; + }); + + it('after a cancel, reports the previous outcome as canceled when retrying', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox + .stub() + .callsFake(({ signal }: { signal: AbortSignal }) => { + return new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason)); + }); + }), + } as any, + track, + }); + + const c = new AbortController(); + const firstPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT, signal: c.signal }) + ); + await clock.tickAsync(0); + c.abort(new Error('user canceled')); + await firstPromise; + + const secondPromise = store.dispatch( + performSignInAttempt({ entrypoint: ENTRYPOINT }) + ); + await clock.tickAsync(0); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + { entrypoint: ENTRYPOINT, attempt: 2, previousOutcome: 'canceled' }, + ]); + + await clock.tickAsync(SIGN_IN_TIMEOUT_MS); + await secondPromise; + }); + + it('after a failure, reports the previous outcome as error when retrying', async function () { + const track = sandbox.stub(); + const store = configureStore({ + atlasAuthService: { + isAuthenticated: sandbox.stub().resolves(false), + signIn: sandbox.stub().rejects(new Error('Sign in failed')), + getUserInfo: sandbox.stub().resolves({ sub: '1234' }), + } as any, + track, + }); + + await store.dispatch(performSignInAttempt({ entrypoint: ENTRYPOINT })); + await store.dispatch(performSignInAttempt({ entrypoint: ENTRYPOINT })); + + expect(startedCalls(track)).to.deep.equal([ + { entrypoint: ENTRYPOINT, attempt: 1, previousOutcome: null }, + { entrypoint: ENTRYPOINT, attempt: 2, previousOutcome: 'error' }, + ]); + }); + + it('resets the attempt tracking after a successful sign in', function () { + const store = configureStore({ atlasAuthService: {} as any }); + + store.dispatch({ + type: 'atlas-service/atlas-signin/AttemptStart', + id: 1, + }); + store.dispatch({ + type: 'atlas-service/atlas-signin/AtlasSignInError', + error: 'some error', + }); + expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('previousOutcome', 'error'); + + store.dispatch({ + type: 'atlas-service/atlas-signin/AtlasSignInSuccess', + userInfo: { sub: '1234' }, + }); + expect(store.getState()).to.have.property('attemptNumber', 0); + expect(store.getState()).to.have.property('previousOutcome', null); + }); + + it('increments attemptNumber on each AttemptStart', function () { + const store = configureStore({ atlasAuthService: {} as any }); + + expect(store.getState()).to.have.property('attemptNumber', 0); + store.dispatch({ + type: 'atlas-service/atlas-signin/AttemptStart', + id: 1, + }); + expect(store.getState()).to.have.property('attemptNumber', 1); + store.dispatch({ type: 'atlas-service/atlas-signin/AttemptEnd', id: 1 }); + store.dispatch({ + type: 'atlas-service/atlas-signin/AtlasSignInTimedOut', + }); + store.dispatch({ + type: 'atlas-service/atlas-signin/AttemptStart', + id: 2, + }); + expect(store.getState()).to.have.property('attemptNumber', 2); + }); + }); + describe('signOut', function () { let openToastStub: Sinon.SinonStub; diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index a109972e16c..ae74d35999b 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -20,6 +20,8 @@ export type AtlasSignInState = { error: string | null; // For managing attempt state that doesn't belong in the store currentAttemptId: number | null; + attemptNumber: number; + previousOutcome: 'timed-out' | 'canceled' | 'error' | null; } & ( | { state: @@ -122,6 +124,8 @@ const INITIAL_STATE = { error: null, isModalOpen: false, currentAttemptId: null, + attemptNumber: 0, + previousOutcome: null, }; type AttemptState = { @@ -222,6 +226,7 @@ const reducer: Reducer = ( return { ...state, currentAttemptId: action.id, + attemptNumber: state.attemptNumber + 1, }; } @@ -245,6 +250,8 @@ const reducer: Reducer = ( userInfo: action.userInfo, error: null, isModalOpen: false, + attemptNumber: 0, + previousOutcome: null, }; } @@ -255,17 +262,28 @@ const reducer: Reducer = ( userInfo: null, error: action.error, isModalOpen: false, + previousOutcome: 'error', }; } if (isAction(action, AtlasSignInActions.Cancel)) { - return { ...INITIAL_STATE, state: 'canceled' }; + return { + ...INITIAL_STATE, + state: 'canceled', + attemptNumber: state.attemptNumber, + previousOutcome: 'canceled', + }; } if ( isAction(action, AtlasSignInActions.TimedOut) ) { - return { ...INITIAL_STATE, state: 'timed-out' }; + return { + ...INITIAL_STATE, + state: 'timed-out', + attemptNumber: state.attemptNumber, + previousOutcome: 'timed-out', + }; } if ( @@ -319,12 +337,7 @@ const startAttempt = ( fn: () => void, entrypoint: AtlasSignInEntrypoint ): AtlasSignInThunkAction => { - return (dispatch, getState) => { - if (getState().currentAttemptId) { - throw new Error( - "Can't start sign in with prompt while another sign in attempt is in progress" - ); - } + return (dispatch) => { const attempt = getAttempt(); dispatch({ type: AtlasSignInActions.AttemptStart, id: attempt.id }); @@ -367,12 +380,18 @@ export const performSignInAttempt = ({ ); } - track('Atlas Sign In Started', { entrypoint }); const attempt = dispatch( startAttempt(() => { void dispatch(signIn()); }, entrypoint) ); + // The attemptNumber is incremented when AttemptStart is dispatched, so we + // must track the event after it. + track('Atlas Sign In Started', { + entrypoint, + attempt: getState().attemptNumber, + previousOutcome: getState().previousOutcome, + }); signal?.addEventListener('abort', () => { dispatch(cancelSignIn(signal.reason)); }); @@ -445,7 +464,7 @@ export const signIn = (): AtlasSignInThunkAction> => { }; export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { - return (dispatch, getState) => { + return (dispatch, getState, { track }) => { // Can't cancel sign in after the flow was finished indicated by current // attempt id being set to null if (getState().currentAttemptId === null) { @@ -456,6 +475,7 @@ export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { attempt.controller.abort(); attempt.reject(reason ?? attempt.controller.signal.reason); dispatch({ type: AtlasSignInActions.Cancel }); + track('Atlas Sign In Canceled', {}); }; }; @@ -476,8 +496,8 @@ export const timeoutSignIn = ( variant: 'note', timeout: 5000, }); - track('Atlas Sign In Timed Out', { entrypoint }); dispatch({ type: AtlasSignInActions.TimedOut }); + track('Atlas Sign In Timed Out', { entrypoint }); }; }; diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx index 70843ada906..8f6e000b950 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx @@ -184,6 +184,8 @@ describe('AtlasToolCallMessage', function () { await waitFor(() => { expect(track).to.have.been.calledWith('Atlas Sign In Started', { entrypoint: 'assistant-tool-atlas-connection-error-debugger', + attempt: 1, + previousOutcome: null, }); }); }); diff --git a/packages/compass-telemetry/src/telemetry-events.ts b/packages/compass-telemetry/src/telemetry-events.ts index 206754299b1..f31bf052c62 100644 --- a/packages/compass-telemetry/src/telemetry-events.ts +++ b/packages/compass-telemetry/src/telemetry-events.ts @@ -160,9 +160,30 @@ type AtlasSignInStartedEvent = CommonEvent<{ * The surface of the application the sign in was triggered from. */ entrypoint: AtlasSignInEntrypoint; + /** + * The current attempt of the sign in. If the attempt is bigger than 1, + * it means the user is re-trying to sign in after a previous attempt + * did not succeed. + */ + attempt: number; + /** + * How the immediately preceding attempt ended, when this is a retry. + * Absent on the first attempt. + */ + previousOutcome: 'timed-out' | 'canceled' | 'error' | null; }; }>; +/** + * This event is fired when the user aborts the current sign in attempt. + * + * @category Atlas + */ +type AtlasSignInCanceledEvent = CommonEvent<{ + name: 'Atlas Sign In Canceled'; + payload: Record; +}>; + /** * This event is fired when user successfully signed in to their Atlas account * @@ -4208,6 +4229,7 @@ export type TelemetryEvent = | ApplicationLaunchedEvent | AtlasLinkClickedEvent | AtlasSearchIndexesForViewLinkClickedEvent + | AtlasSignInCanceledEvent | AtlasSignInErrorEvent | AtlasSignInTimedOutEvent | AtlasSignInPromptShownEvent From 5ad73d22495b20ef45acf6a75165cceaa0f3f3d4 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Fri, 21 Aug 2026 15:29:40 +0100 Subject: [PATCH 04/14] self-review --- packages/atlas-service/src/store/atlas-signin-reducer.ts | 2 +- .../src/components/atlas-tool-call-message.tsx | 9 +++++---- .../tests/collection-indexes-tab.test.ts | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index ae74d35999b..c1537ad3959 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -137,7 +137,7 @@ type AttemptState = { timeoutId?: ReturnType; }; -export const SIGN_IN_TIMEOUT_MS = 10 * 1000; +export const SIGN_IN_TIMEOUT_MS = 2 * 60 * 1000; // 2 Minutes // Exported for testing purposes only export const AttemptStateMap = new Map(); diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx index 3860729cf4c..2b41fd40400 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx @@ -70,16 +70,17 @@ This is read-only and won't change your cluster.`; ); } +// TODO(COMPASS-11044): update texts to be generic function getApprovalMessage( isSignInInProgress: boolean, isUserSignedIn: boolean ) { - if (isSignInInProgress) { - return 'Connecting with Atlas to debug this connection'; - } if (isUserSignedIn) { return 'Run Atlas to debug this connection?'; } + if (isSignInInProgress) { + return 'Connecting with Atlas to debug this connection'; + } return 'Connect with Atlas to debug this connection?'; } @@ -140,7 +141,7 @@ export const AtlasToolCallMessage: React.FunctionComponent< case 'success': onApprove(approvalId, true); break; - // If sign in timed out, give the user a new change instead of + // If sign in timed out, give the user a new chance instead of // rejecting the tool case 'timed-out': break; diff --git a/packages/compass-e2e-tests/tests/collection-indexes-tab.test.ts b/packages/compass-e2e-tests/tests/collection-indexes-tab.test.ts index efc4b24eb13..a85269476bd 100644 --- a/packages/compass-e2e-tests/tests/collection-indexes-tab.test.ts +++ b/packages/compass-e2e-tests/tests/collection-indexes-tab.test.ts @@ -75,7 +75,7 @@ describe('Collection indexes tab', function () { await browser.dropIndex(createdIndexName, 'drop-index-modal-basic.png'); }); - it.only('supports creating a wildcard index', async function () { + it('supports creating a wildcard index', async function () { const indexName = await browser.createIndex( { fieldName: '$**', From 2ae1fcf1c9faae679af087fa3e92d14354adb52f Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Fri, 21 Aug 2026 16:09:12 +0100 Subject: [PATCH 05/14] copilot review --- packages/atlas-service/src/store/atlas-signin-reducer.ts | 4 ++++ packages/compass-telemetry/src/telemetry-events.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index c1537ad3959..4e7a9243bd3 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -420,6 +420,7 @@ async function toSignInAttemptResult( export const signIn = (): AtlasSignInThunkAction> => { return async (dispatch, getState, { atlasAuthService }) => { const { + id: currentAttemptId, controller: { signal }, resolve, reject, @@ -458,6 +459,7 @@ export const signIn = (): AtlasSignInThunkAction> => { type: AtlasSignInActions.Error, error: (err as Error).message, }); + AttemptStateMap.delete(currentAttemptId); reject(err); } }; @@ -474,6 +476,7 @@ export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { clearTimeout(attempt.timeoutId); attempt.controller.abort(); attempt.reject(reason ?? attempt.controller.signal.reason); + AttemptStateMap.delete(attempt.id); dispatch({ type: AtlasSignInActions.Cancel }); track('Atlas Sign In Canceled', {}); }; @@ -491,6 +494,7 @@ export const timeoutSignIn = ( clearTimeout(attempt.timeoutId); attempt.controller.abort(); attempt.reject(new Error('Sign in timed out')); + AttemptStateMap.delete(attempt.id); openToast('atlas-disconnected', { title: 'The login to Atlas has timed out, please try again.', variant: 'note', diff --git a/packages/compass-telemetry/src/telemetry-events.ts b/packages/compass-telemetry/src/telemetry-events.ts index f31bf052c62..0d446d89d4c 100644 --- a/packages/compass-telemetry/src/telemetry-events.ts +++ b/packages/compass-telemetry/src/telemetry-events.ts @@ -168,7 +168,7 @@ type AtlasSignInStartedEvent = CommonEvent<{ attempt: number; /** * How the immediately preceding attempt ended, when this is a retry. - * Absent on the first attempt. + * Null on the first attempt. */ previousOutcome: 'timed-out' | 'canceled' | 'error' | null; }; From f16cf3a1b579efbcfe16fa9dfd4d5524bf248acf Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Tue, 25 Aug 2026 09:16:13 +0100 Subject: [PATCH 06/14] address pr comments --- .../components/atlas-connection-status.tsx | 6 +- packages/atlas-service/src/provider.tsx | 4 +- .../src/store/atlas-signin-reducer.spec.ts | 19 ++-- .../src/store/atlas-signin-reducer.ts | 28 +++-- .../store/atlas-signin-store-context.spec.tsx | 107 +++++++----------- .../src/store/atlas-signin-store-context.tsx | 45 ++++---- .../atlas-tool-call-message.spec.tsx | 4 + .../components/atlas-tool-call-message.tsx | 40 +------ 8 files changed, 102 insertions(+), 151 deletions(-) diff --git a/packages/atlas-service/src/components/atlas-connection-status.tsx b/packages/atlas-service/src/components/atlas-connection-status.tsx index dcc0d1bf976..f803089e7fc 100644 --- a/packages/atlas-service/src/components/atlas-connection-status.tsx +++ b/packages/atlas-service/src/components/atlas-connection-status.tsx @@ -11,7 +11,7 @@ import { spacing, useDarkMode, } from '@mongodb-js/compass-components'; -import { useAtlasLoginActions, useAtlasSignedInUser } from '../provider'; +import { useAtlasLoginActions, useAtlasSignInStatus } from '../provider'; const containerStyles = css({ display: 'flex', @@ -57,7 +57,7 @@ export const AtlasConnectionStatus: React.FunctionComponent< AtlasConnectionStatusProps > = ({ 'data-testid': dataTestId = 'atlas-connection-status' }) => { const darkMode = useDarkMode(); - const userInfo = useAtlasSignedInUser(); + const signInStatus = useAtlasSignInStatus(); const { signOut } = useAtlasLoginActions(); const handleDisconnect = useCallback(() => { @@ -76,7 +76,7 @@ export const AtlasConnectionStatus: React.FunctionComponent< })(); }, [signOut]); - if (!userInfo) { + if (!signInStatus.user) { return null; } diff --git a/packages/atlas-service/src/provider.tsx b/packages/atlas-service/src/provider.tsx index ffc4dd2c59b..72b83a50b25 100644 --- a/packages/atlas-service/src/provider.tsx +++ b/packages/atlas-service/src/provider.tsx @@ -68,10 +68,8 @@ export { AtlasAuthService } from './atlas-auth-service'; export type { AtlasService } from './atlas-service'; export type { AtlasUserInfo } from './renderer'; export { - useAtlasSignedInUser, + useAtlasSignInStatus, useAtlasLoginActions, - useIsAtlasSignInStateResolved, - useIsAtlasSignInInProgress, } from './store/atlas-signin-store-context'; export type { AtlasLoginActions } from './store/atlas-signin-store-context'; export { AtlasConnectionStatus } from './components/atlas-connection-status'; diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 88c44fbb5e3..b7df5f549f8 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -85,7 +85,7 @@ describe('atlasSignInReducer', function () { const restorePromise = store.dispatch(restoreSignInState()); expect(mockAtlasService.isAuthenticated).to.have.been.calledOnce; expect(store.getState()).to.have.nested.property('state', 'restoring'); - await store.dispatch(signIn()); + await store.dispatch(signIn({ entrypoint: 'unknown' })); expect(mockAtlasService.isAuthenticated).to.have.been.calledTwice; expect(store.getState()).to.have.nested.property('state', 'success'); // Intentionally returning false here so that if action would affect @@ -107,7 +107,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: mockAtlasService as any, }); - await store.dispatch(signIn()); + await store.dispatch(signIn({ entrypoint: 'unknown' })); expect(mockAtlasService.isAuthenticated).to.have.been.calledOnce; expect(mockAtlasService.signIn).not.to.have.been.called; expect(store.getState()).to.have.nested.property('state', 'success'); @@ -123,7 +123,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: mockAtlasService as any, }); - await store.dispatch(signIn()); + await store.dispatch(signIn({ entrypoint: 'unknown' })); expect(mockAtlasService.isAuthenticated).to.have.been.calledOnce; expect(mockAtlasService.signIn).to.have.been.calledOnce; expect(store.getState()).to.have.nested.property('state', 'success'); @@ -138,7 +138,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: mockAtlasService as any, }); - const signInPromise = store.dispatch(signIn()); + const signInPromise = store.dispatch(signIn({ entrypoint: 'unknown' })); // Avoid unhandled rejections AttemptStateMap.get(attemptId)?.promise.catch(() => {}); await signInPromise; @@ -185,7 +185,6 @@ describe('atlasSignInReducer', function () { store.dispatch(cancelSignIn()); expect(store.getState()).to.have.nested.property('state', 'canceled'); expect(store.getState()).to.have.property('attemptNumber', 1); - expect(store.getState()).to.have.property('previousOutcome', 'canceled'); expect(isAuthenticatedStub).to.have.been.calledOnce; expect(track).to.have.been.calledWith('Atlas Sign In Canceled', {}); @@ -254,7 +253,6 @@ describe('atlasSignInReducer', function () { expect(store.getState()).to.have.nested.property('state', 'timed-out'); expect(store.getState()).to.have.property('attemptNumber', 1); - expect(store.getState()).to.have.property('previousOutcome', 'timed-out'); expect(store.getState()).to.have.nested.property( 'currentAttemptId', null @@ -333,7 +331,6 @@ describe('atlasSignInReducer', function () { expect(result).to.have.nested.property('error.message', 'Sign in failed'); expect(store.getState()).to.have.property('state', 'error'); expect(store.getState()).to.have.property('attemptNumber', 1); - expect(store.getState()).to.have.property('previousOutcome', 'error'); }); it('should resolve with a canceled result if provided signal was aborted', async function () { @@ -409,7 +406,7 @@ describe('atlasSignInReducer', function () { entrypoint: 'assistant-tool-atlas-connection-error-debugger', }) ); - expect(track).to.have.been.calledOnceWith('Atlas Sign In Started', { + expect(track).to.have.been.calledWith('Atlas Sign In Started', { entrypoint: 'assistant-tool-atlas-connection-error-debugger', attempt: 1, previousOutcome: null, @@ -429,7 +426,7 @@ describe('atlasSignInReducer', function () { track, }); await store.dispatch(performSignInAttempt()); - expect(track).to.have.been.calledOnceWith('Atlas Sign In Started', { + expect(track).to.have.been.calledWith('Atlas Sign In Started', { entrypoint: 'unknown', attempt: 1, previousOutcome: null, @@ -591,7 +588,7 @@ describe('atlasSignInReducer', function () { ]); }); - it('resets the attempt tracking after a successful sign in', function () { + it('resets the attempt number after a successful sign in', function () { const store = configureStore({ atlasAuthService: {} as any }); store.dispatch({ @@ -603,14 +600,12 @@ describe('atlasSignInReducer', function () { error: 'some error', }); expect(store.getState()).to.have.property('attemptNumber', 1); - expect(store.getState()).to.have.property('previousOutcome', 'error'); store.dispatch({ type: 'atlas-service/atlas-signin/AtlasSignInSuccess', userInfo: { sub: '1234' }, }); expect(store.getState()).to.have.property('attemptNumber', 0); - expect(store.getState()).to.have.property('previousOutcome', null); }); it('increments attemptNumber on each AttemptStart', function () { diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index 4e7a9243bd3..cee4bcb4085 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -21,7 +21,6 @@ export type AtlasSignInState = { // For managing attempt state that doesn't belong in the store currentAttemptId: number | null; attemptNumber: number; - previousOutcome: 'timed-out' | 'canceled' | 'error' | null; } & ( | { state: @@ -125,7 +124,6 @@ const INITIAL_STATE = { isModalOpen: false, currentAttemptId: null, attemptNumber: 0, - previousOutcome: null, }; type AttemptState = { @@ -251,7 +249,6 @@ const reducer: Reducer = ( error: null, isModalOpen: false, attemptNumber: 0, - previousOutcome: null, }; } @@ -262,7 +259,6 @@ const reducer: Reducer = ( userInfo: null, error: action.error, isModalOpen: false, - previousOutcome: 'error', }; } @@ -271,7 +267,6 @@ const reducer: Reducer = ( ...INITIAL_STATE, state: 'canceled', attemptNumber: state.attemptNumber, - previousOutcome: 'canceled', }; } @@ -282,7 +277,6 @@ const reducer: Reducer = ( ...INITIAL_STATE, state: 'timed-out', attemptNumber: state.attemptNumber, - previousOutcome: 'timed-out', }; } @@ -359,6 +353,11 @@ const startAttempt = ( }; }; +const isRelevantPreviousState = (state: AtlasSignInState): boolean => + state.state === 'error' || + state.state === 'canceled' || + state.state === 'timed-out'; + export const performSignInAttempt = ({ signal, entrypoint = 'unknown', @@ -382,7 +381,7 @@ export const performSignInAttempt = ({ const attempt = dispatch( startAttempt(() => { - void dispatch(signIn()); + void dispatch(signIn({ entrypoint })); }, entrypoint) ); // The attemptNumber is incremented when AttemptStart is dispatched, so we @@ -390,7 +389,9 @@ export const performSignInAttempt = ({ track('Atlas Sign In Started', { entrypoint, attempt: getState().attemptNumber, - previousOutcome: getState().previousOutcome, + previousOutcome: isRelevantPreviousState(getState()) + ? (getState().state as 'error' | 'canceled' | 'timed-out') + : null, }); signal?.addEventListener('abort', () => { dispatch(cancelSignIn(signal.reason)); @@ -417,8 +418,12 @@ async function toSignInAttemptResult( /** * Sign into Atlas. To be called when the user isn't signed in yet. */ -export const signIn = (): AtlasSignInThunkAction> => { - return async (dispatch, getState, { atlasAuthService }) => { +export const signIn = ({ + entrypoint, +}: { + entrypoint: AtlasSignInEntrypoint; +}): AtlasSignInThunkAction> => { + return async (dispatch, getState, { atlasAuthService, track }) => { const { id: currentAttemptId, controller: { signal }, @@ -435,6 +440,9 @@ export const signIn = (): AtlasSignInThunkAction> => { userInfo = await atlasAuthService.signIn({ signal, }); + track('Atlas Sign In Prompt Shown', { + entrypoint, + }); } openToast('atlas-sign-in-success', { variant: 'success', diff --git a/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx b/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx index 8d13c2a89c3..ca7bfcbaaea 100644 --- a/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx +++ b/packages/atlas-service/src/store/atlas-signin-store-context.spec.tsx @@ -5,10 +5,9 @@ import { renderHook } from '@mongodb-js/testing-library-compass'; import type { AnyAction } from 'redux'; import { AtlasSignInStoreContext, - useAtlasSignedInUser, - useIsAtlasSignInStateResolved, - useIsAtlasSignInInProgress, + useAtlasSignInStatus, } from './atlas-signin-store-context'; +import type { AtlasSignInStatus } from './atlas-signin-store-context'; import { AtlasSignInActions } from './atlas-signin-reducer'; import { configureStore } from './atlas-signin-store'; @@ -24,102 +23,74 @@ function renderWithState(actions: AnyAction[]) { ); return { store, - isResolved: renderHook(() => useIsAtlasSignInStateResolved(), { wrapper }) - .result, - isInProgress: renderHook(() => useIsAtlasSignInInProgress(), { wrapper }) - .result, - signedInUser: renderHook(() => useAtlasSignedInUser(), { wrapper }).result, + status: renderHook(() => useAtlasSignInStatus(), { wrapper }).result, }; } +const USER = { sub: '1234' }; const RESTORING = [{ type: AtlasSignInActions.RestoringStart }]; const SUCCESS = [ { type: AtlasSignInActions.RestoringStart }, - { type: AtlasSignInActions.RestoringSuccess, userInfo: { sub: '1234' } }, + { type: AtlasSignInActions.RestoringSuccess, userInfo: USER }, ]; -describe('useIsAtlasSignInStateResolved', function () { - const cases: [string, AnyAction[], boolean][] = [ - ['initial', [], false], - ['restoring', RESTORING, false], - // A manual sign in attempt started while restoring makes the reducer - // discard the restoring result, so until this attempt settles we still - // don't know whether the user is signed in. - ['in-progress', [...RESTORING, { type: AtlasSignInActions.Start }], false], +describe('useAtlasSignInStatus', function () { + const cases: [string, AnyAction[], AtlasSignInStatus][] = [ + ['initial', [], { user: null, state: 'initial' }], + ['restoring', RESTORING, { user: null, state: 'restoring' }], + [ + 'in-progress', + [{ type: AtlasSignInActions.Start }], + { user: null, state: 'in-progress' }, + ], [ 'unauthenticated', [...RESTORING, { type: AtlasSignInActions.RestoringFailed }], - true, + { user: null, state: 'unauthenticated' }, ], - ['success', SUCCESS, true], + ['success', SUCCESS, { user: USER, state: 'success' }], [ 'error', [ { type: AtlasSignInActions.Start }, { type: AtlasSignInActions.Error, error: 'Whoops!' }, ], - true, + { user: null, state: 'error' }, + ], + [ + 'canceled', + [{ type: AtlasSignInActions.Cancel }], + { user: null, state: 'canceled' }, + ], + [ + 'timed-out', + [{ type: AtlasSignInActions.TimedOut }], + { user: null, state: 'timed-out' }, ], - ['canceled', [{ type: AtlasSignInActions.Cancel }], true], ]; for (const [state, actions, expected] of cases) { - it(`should return ${String( - expected - )} for the '${state}' state`, function () { - const { store, isResolved } = renderWithState(actions); + it(`reports the status for the '${state}' state`, function () { + const { store, status } = renderWithState(actions); expect(store.getState()).to.have.property('state', state); - expect(isResolved.current).to.eq(expected); + expect(status.current).to.deep.equal(expected); }); } - it('should not report a resolved state while a sign in started during restore is still in flight', function () { - const { store, isResolved, signedInUser } = renderWithState([ + it('does not report a resolved or signed in state while a sign in started during restore is still in flight', function () { + const { store, status } = renderWithState([ ...RESTORING, { type: AtlasSignInActions.Start }, // The restore finishes after the manual attempt started, the reducer - // ignores it - { type: AtlasSignInActions.RestoringSuccess, userInfo: { sub: '1234' } }, + // ignores it. + { type: AtlasSignInActions.RestoringSuccess, userInfo: USER }, ]); expect(store.getState()).to.have.property('state', 'in-progress'); // The user looks signed out here, so anything acting on that (telemetry, - // for example) has to wait for the state to resolve - expect(signedInUser.current).to.eq(null); - expect(isResolved.current).to.eq(false); - }); -}); - -describe('useIsAtlasSignInInProgress', function () { - const cases: [string, AnyAction[], boolean][] = [ - // only in-progress should return true - ['in-progress', [{ type: AtlasSignInActions.Start }], true], - ['initial', [], false], - ['restoring', RESTORING, false], - [ - 'unauthenticated', - [...RESTORING, { type: AtlasSignInActions.RestoringFailed }], - false, - ], - ['success', SUCCESS, false], - [ - 'error', - [ - { type: AtlasSignInActions.Start }, - { type: AtlasSignInActions.Error, error: 'Whoops!' }, - ], - false, - ], - ['canceled', [{ type: AtlasSignInActions.Cancel }], false], - ['timed-out', [{ type: AtlasSignInActions.TimedOut }], false], - ]; - - for (const [state, actions, expected] of cases) { - it(`should return ${String( - expected - )} for the '${state}' state`, function () { - const { store, isInProgress } = renderWithState(actions); - expect(store.getState()).to.have.property('state', state); - expect(isInProgress.current).to.eq(expected); + // for example) has to wait for the state to resolve. + expect(status.current).to.deep.equal({ + user: null, + state: 'in-progress', }); - } + }); }); diff --git a/packages/atlas-service/src/store/atlas-signin-store-context.tsx b/packages/atlas-service/src/store/atlas-signin-store-context.tsx index f94b0dcc615..1ce7e802ec7 100644 --- a/packages/atlas-service/src/store/atlas-signin-store-context.tsx +++ b/packages/atlas-service/src/store/atlas-signin-store-context.tsx @@ -6,7 +6,11 @@ import { type AtlasSignInState, } from './atlas-signin-reducer'; import type { ReactReduxContextValue, TypedUseSelectorHook } from 'react-redux'; -import { createDispatchHook, createSelectorHook } from 'react-redux'; +import { + createDispatchHook, + createSelectorHook, + shallowEqual, +} from 'react-redux'; import type { ThunkDispatch } from 'redux-thunk'; import type { AnyAction } from 'redux'; import type { AtlasAuthService, AtlasUserInfo } from '../provider'; @@ -34,31 +38,30 @@ const useDispatch: () => AtlasSignInDispatch = createDispatchHook( AtlasSignInStoreContext ); -export function useAtlasSignedInUser(): AtlasUserInfo | null { - return useSelector((state) => - state.state === 'success' ? state.userInfo : null - ); -} +export type AtlasSignInStatus = { + user: AtlasUserInfo | null; + state: + | 'initial' + | 'restoring' + | 'unauthenticated' + | 'in-progress' + | 'error' + | 'canceled' + | 'timed-out' + | 'success'; +}; -/** - * Whether we know yet if the user is signed in. The signed in state is restored - * asynchronously on startup, so `useAtlasSignedInUser` returns `null` for an - * already signed in user until that finishes. Anything that shouldn't act on a - * false "signed out" (reporting telemetry, for example) should wait for this. - */ -export function useIsAtlasSignInStateResolved(): boolean { +// one hook, one subscription, derived/curated shape +export function useAtlasSignInStatus() { return useSelector( - (state) => - state.state !== 'initial' && - state.state !== 'restoring' && - state.state !== 'in-progress' + (s): AtlasSignInStatus => ({ + user: s.state === 'success' ? s.userInfo : null, + state: s.state, + }), + shallowEqual ); } -export function useIsAtlasSignInInProgress(): boolean { - return useSelector((state) => state.state === 'in-progress'); -} - export type AtlasLoginActions = { signOut: () => Promise; signIn: (opts?: { diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx index 8f73198c8f0..2f201802f6a 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx @@ -164,6 +164,8 @@ describe('AtlasToolCallMessage', function () { it('tracks the sign in prompt once, with the tool name as entrypoint', async function () { const { track } = renderMessage(); + userEvent.click(screen.getByText('Connect to Atlas')); + await waitFor(() => { expect(track).to.have.been.calledWith('Atlas Sign In Prompt Shown', { entrypoint: 'assistant-tool-atlas-connection-error-debugger', @@ -200,6 +202,8 @@ describe('AtlasToolCallMessage', function () { } as unknown as ToolUIPart, }); + userEvent.click(screen.getByText('Connect to Atlas')); + await waitFor(() => { expect(track).to.have.been.calledWith('Atlas Sign In Prompt Shown', { entrypoint: 'assistant-tool-atlas-some-future-tool', diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx index 2b41fd40400..c704d27b289 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo, useRef } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { css, InlineDefinition, @@ -22,13 +22,10 @@ import { import type { AtlasSignInEntrypoint } from '@mongodb-js/compass-telemetry'; import { useAtlasLoginActions, - useAtlasSignedInUser, - useIsAtlasSignInStateResolved, - useIsAtlasSignInInProgress, + useAtlasSignInStatus, } from '@mongodb-js/atlas-service/provider'; import { CustomToolResult } from './custom-tool-result'; import { getToolCallTitle } from './tool-call-title'; -import { useTelemetry } from '@mongodb-js/compass-telemetry/provider'; /** * Every sign in this card drives is attributed to the assistant tool call that @@ -92,35 +89,10 @@ export const AtlasToolCallMessage: React.FunctionComponent< const toolDisplayName = getToolDisplayName(toolCall.type); const isAwaitingApproval = toolCallState === 'idle' && !!toolCall.approval; const approvalId = toolCall.approval?.id; - const isUserSignedIn = !!useAtlasSignedInUser(); + const atlasSignInStatus = useAtlasSignInStatus(); + const isUserSignedIn = !!atlasSignInStatus.user; + const isSignInInProgress = atlasSignInStatus?.state === 'in-progress'; const { signIn } = useAtlasLoginActions(); - const track = useTelemetry(); - const isSignInStateResolved = useIsAtlasSignInStateResolved(); - const isSignInInProgress = useIsAtlasSignInInProgress(); - - // The card re-renders on every state change, so we only report the prompt the - // first time it's actually offered to a signed out user. We also wait for the - // sign in state to be restored, otherwise an already signed in user looks - // signed out on the first render. - const trackedPromptForApprovalId = useRef(null); - const isSignInPromptShown = - isAwaitingApproval && - isSignInStateResolved && - !isUserSignedIn && - !!approvalId; - - useEffect(() => { - if ( - !isSignInPromptShown || - trackedPromptForApprovalId.current === approvalId - ) { - return; - } - trackedPromptForApprovalId.current = approvalId ?? null; - track('Atlas Sign In Prompt Shown', { - entrypoint: getSignInEntrypoint(toolCall.type), - }); - }, [isSignInPromptShown, approvalId, track, toolCall.type]); const chips = useMemo(() => { if ( @@ -179,7 +151,7 @@ export const AtlasToolCallMessage: React.FunctionComponent< ); const approvalMessage = getApprovalMessage( - !!isSignInInProgress, + isSignInInProgress, isUserSignedIn ); // TODO COMPASS-10973: don't render actions if there's no approvalId. From dd0e763e2fa9b9cf8fe7cf993423c26d5e50b1b7 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Tue, 25 Aug 2026 09:23:32 +0100 Subject: [PATCH 07/14] nit --- packages/atlas-service/src/store/atlas-signin-store-context.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/atlas-service/src/store/atlas-signin-store-context.tsx b/packages/atlas-service/src/store/atlas-signin-store-context.tsx index 1ce7e802ec7..3ec264808b6 100644 --- a/packages/atlas-service/src/store/atlas-signin-store-context.tsx +++ b/packages/atlas-service/src/store/atlas-signin-store-context.tsx @@ -51,7 +51,6 @@ export type AtlasSignInStatus = { | 'success'; }; -// one hook, one subscription, derived/curated shape export function useAtlasSignInStatus() { return useSelector( (s): AtlasSignInStatus => ({ From 49868640624649f5b6ae2fb174f8f720662d6392 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Tue, 25 Aug 2026 16:42:52 +0100 Subject: [PATCH 08/14] address comments and revert prompt shown tracking --- .../src/store/atlas-signin-reducer.spec.ts | 24 +-- .../src/store/atlas-signin-reducer.ts | 164 +++++++++--------- .../atlas-tool-call-message.spec.tsx | 4 - .../components/atlas-tool-call-message.tsx | 30 +++- 4 files changed, 120 insertions(+), 102 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index b7df5f549f8..30e4c6ce664 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -154,7 +154,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: {} as any, }); expect(store.getState()).to.have.nested.property('state', 'initial'); - store.dispatch(cancelSignIn()); + store.dispatch(cancelSignIn('canceled')); expect(store.getState()).to.have.nested.property('state', 'initial'); }); @@ -182,9 +182,9 @@ describe('atlasSignInReducer', function () { // Give it some time for start the sign in attempt. It will be waiting // at isAuthenticated, which never resolves. await new Promise((resolve) => setTimeout(resolve, 100)); - store.dispatch(cancelSignIn()); + store.dispatch(cancelSignIn('canceled')); expect(store.getState()).to.have.nested.property('state', 'canceled'); - expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('attemptNumber', 2); expect(isAuthenticatedStub).to.have.been.calledOnce; expect(track).to.have.been.calledWith('Atlas Sign In Canceled', {}); @@ -197,7 +197,7 @@ describe('atlasSignInReducer', function () { track, }); - store.dispatch(cancelSignIn()); + store.dispatch(cancelSignIn('canceled')); expect(track).to.not.have.been.calledWith('Atlas Sign In Canceled'); }); @@ -252,7 +252,7 @@ describe('atlasSignInReducer', function () { const { store, result } = await driveToTimeout(); expect(store.getState()).to.have.nested.property('state', 'timed-out'); - expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('attemptNumber', 2); expect(store.getState()).to.have.nested.property( 'currentAttemptId', null @@ -271,7 +271,7 @@ describe('atlasSignInReducer', function () { it('shows a toast informing the user that sign in timed out', async function () { const { openToast } = await driveToTimeout(); - expect(openToast.withArgs('atlas-disconnected')).to.have.been.calledOnce; + expect(openToast.withArgs('atlas-timed-out')).to.have.been.calledOnce; expect(openToast.lastCall.args[1]).to.include({ title: 'The login to Atlas has timed out, please try again.', variant: 'note', @@ -330,7 +330,7 @@ describe('atlasSignInReducer', function () { expect(result).to.have.property('status', 'error'); expect(result).to.have.nested.property('error.message', 'Sign in failed'); expect(store.getState()).to.have.property('state', 'error'); - expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('attemptNumber', 2); }); it('should resolve with a canceled result if provided signal was aborted', async function () { @@ -599,24 +599,24 @@ describe('atlasSignInReducer', function () { type: 'atlas-service/atlas-signin/AtlasSignInError', error: 'some error', }); - expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('attemptNumber', 2); store.dispatch({ type: 'atlas-service/atlas-signin/AtlasSignInSuccess', userInfo: { sub: '1234' }, }); - expect(store.getState()).to.have.property('attemptNumber', 0); + expect(store.getState()).to.have.property('attemptNumber', 1); }); it('increments attemptNumber on each AttemptStart', function () { const store = configureStore({ atlasAuthService: {} as any }); - expect(store.getState()).to.have.property('attemptNumber', 0); + expect(store.getState()).to.have.property('attemptNumber', 1); store.dispatch({ type: 'atlas-service/atlas-signin/AttemptStart', id: 1, }); - expect(store.getState()).to.have.property('attemptNumber', 1); + expect(store.getState()).to.have.property('attemptNumber', 2); store.dispatch({ type: 'atlas-service/atlas-signin/AttemptEnd', id: 1 }); store.dispatch({ type: 'atlas-service/atlas-signin/AtlasSignInTimedOut', @@ -625,7 +625,7 @@ describe('atlasSignInReducer', function () { type: 'atlas-service/atlas-signin/AttemptStart', id: 2, }); - expect(store.getState()).to.have.property('attemptNumber', 2); + expect(store.getState()).to.have.property('attemptNumber', 3); }); }); diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index cee4bcb4085..80f02e839d6 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -52,6 +52,12 @@ export type AtlasSignInThunkAction< A >; +class TimeoutError extends Error { + constructor() { + super('Sign in timed out'); + } +} + // @ts-expect-error TODO(COMPASS-10124): replace enums with const kv objects export const enum AtlasSignInActions { RestoringStart = 'atlas-service/atlas-signin/StartRestoring', @@ -123,7 +129,7 @@ const INITIAL_STATE = { error: null, isModalOpen: false, currentAttemptId: null, - attemptNumber: 0, + attemptNumber: 1, }; type AttemptState = { @@ -132,7 +138,6 @@ type AttemptState = { promise: Promise; resolve: (userInfo: AtlasUserInfo) => void; reject: (reason?: any) => void; - timeoutId?: ReturnType; }; export const SIGN_IN_TIMEOUT_MS = 2 * 60 * 1000; // 2 Minutes @@ -248,7 +253,7 @@ const reducer: Reducer = ( userInfo: action.userInfo, error: null, isModalOpen: false, - attemptNumber: 0, + attemptNumber: 1, }; } @@ -327,21 +332,13 @@ export const restoreSignInState = (): AtlasSignInThunkAction> => { }; }; -const startAttempt = ( - fn: () => void, - entrypoint: AtlasSignInEntrypoint -): AtlasSignInThunkAction => { +const startAttempt = (fn: () => void): AtlasSignInThunkAction => { return (dispatch) => { const attempt = getAttempt(); dispatch({ type: AtlasSignInActions.AttemptStart, id: attempt.id }); - attempt.timeoutId = setTimeout(() => { - dispatch(timeoutSignIn(attempt.id, entrypoint)); - }, SIGN_IN_TIMEOUT_MS); - attempt.promise .finally(() => { - clearTimeout(attempt.timeoutId); dispatch({ type: AtlasSignInActions.AttemptEnd, id: attempt.id }); }) .catch(() => { @@ -353,11 +350,6 @@ const startAttempt = ( }; }; -const isRelevantPreviousState = (state: AtlasSignInState): boolean => - state.state === 'error' || - state.state === 'canceled' || - state.state === 'timed-out'; - export const performSignInAttempt = ({ signal, entrypoint = 'unknown', @@ -367,7 +359,7 @@ export const performSignInAttempt = ({ } = {}): AtlasSignInThunkAction> => { return async (dispatch, getState, { track }) => { // Nothing to do if we already signed in - const { state, userInfo, currentAttemptId } = getState(); + const { state, userInfo, currentAttemptId, attemptNumber } = getState(); if (state === 'success') { return { status: 'success', userInfo }; } @@ -382,16 +374,17 @@ export const performSignInAttempt = ({ const attempt = dispatch( startAttempt(() => { void dispatch(signIn({ entrypoint })); - }, entrypoint) + }) ); // The attemptNumber is incremented when AttemptStart is dispatched, so we // must track the event after it. track('Atlas Sign In Started', { entrypoint, - attempt: getState().attemptNumber, - previousOutcome: isRelevantPreviousState(getState()) - ? (getState().state as 'error' | 'canceled' | 'timed-out') - : null, + attempt: attemptNumber, + previousOutcome: + state === 'error' || state === 'canceled' || state === 'timed-out' + ? state + : null, }); signal?.addEventListener('abort', () => { dispatch(cancelSignIn(signal.reason)); @@ -418,62 +411,87 @@ async function toSignInAttemptResult( /** * Sign into Atlas. To be called when the user isn't signed in yet. */ -export const signIn = ({ - entrypoint, -}: { - entrypoint: AtlasSignInEntrypoint; -}): AtlasSignInThunkAction> => { - return async (dispatch, getState, { atlasAuthService, track }) => { +export const signIn = + ({ + entrypoint, + }: { + entrypoint: AtlasSignInEntrypoint; + }): AtlasSignInThunkAction> => + async (dispatch, getState, { atlasAuthService, track }) => { const { id: currentAttemptId, - controller: { signal }, + controller, resolve, reject, } = getAttempt(getState().currentAttemptId); dispatch({ type: AtlasSignInActions.Start }); + const signal = controller.signal; + let timeoutId: ReturnType | undefined; try { throwIfAborted(signal); let userInfo; - if (await atlasAuthService.isAuthenticated({ signal })) { - userInfo = await atlasAuthService.getUserInfo({ signal }); - } else { - userInfo = await atlasAuthService.signIn({ - signal, - }); - track('Atlas Sign In Prompt Shown', { - entrypoint, + + const doSignIn = async () => { + if (await atlasAuthService.isAuthenticated({ signal })) { + userInfo = await atlasAuthService.getUserInfo({ signal }); + } else { + userInfo = await atlasAuthService.signIn({ + signal, + }); + } + openToast('atlas-sign-in-success', { + variant: 'success', + title: `Atlas sign in successful`, + timeout: 10_000, }); - } - openToast('atlas-sign-in-success', { - variant: 'success', - title: `Atlas sign in successful`, - timeout: 10_000, + dispatch({ type: AtlasSignInActions.Success, userInfo }); + AttemptStateMap.clear(); + resolve(userInfo); + }; + const timeoutPromise = new Promise((_resolve, reject) => { + timeoutId = setTimeout(() => { + controller.abort(new TimeoutError()); + reject(new TimeoutError()); + }, SIGN_IN_TIMEOUT_MS); }); - dispatch({ type: AtlasSignInActions.Success, userInfo }); - AttemptStateMap.clear(); - resolve(userInfo); + + await Promise.race([doSignIn(), timeoutPromise]); } catch (err) { - // Only handle error if sign in wasn't aborted by the user, otherwise it - // was already handled in `cancelSignIn` action if (signal.aborted) { - return; + // the canceled flow must be tracked outside of the signIn function + // as it can be triggered by an external caller. + if (!(signal.reason instanceof TimeoutError)) { + return; + } + openToast('atlas-timed-out', { + title: 'The login to Atlas has timed out, please try again.', + variant: 'note', + timeout: 5000, + }); + dispatch({ type: AtlasSignInActions.TimedOut }); + track('Atlas Sign In Timed Out', { entrypoint }); + reject(signal.reason); + } else { + openToast('atlas-sign-in-error', { + variant: 'important', + title: 'Sign in failed', + description: (err as Error).message, + }); + dispatch({ + type: AtlasSignInActions.Error, + error: (err as Error).message, + }); + + reject(err); } - openToast('atlas-sign-in-error', { - variant: 'important', - title: 'Sign in failed', - description: (err as Error).message, - }); - dispatch({ - type: AtlasSignInActions.Error, - error: (err as Error).message, - }); AttemptStateMap.delete(currentAttemptId); - reject(err); + } finally { + // if the timeout is not cleared the promise will be dangling around + clearTimeout(timeoutId); } }; -}; -export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { +export const cancelSignIn = (reason: any): AtlasSignInThunkAction => { return (dispatch, getState, { track }) => { // Can't cancel sign in after the flow was finished indicated by current // attempt id being set to null @@ -481,7 +499,6 @@ export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { return; } const attempt = getAttempt(getState().currentAttemptId); - clearTimeout(attempt.timeoutId); attempt.controller.abort(); attempt.reject(reason ?? attempt.controller.signal.reason); AttemptStateMap.delete(attempt.id); @@ -490,29 +507,6 @@ export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { }; }; -export const timeoutSignIn = ( - attemptId: number, - entrypoint: AtlasSignInEntrypoint -): AtlasSignInThunkAction => { - return (dispatch, getState, { track }) => { - if (getState().currentAttemptId !== attemptId) { - return; - } - const attempt = getAttempt(attemptId); - clearTimeout(attempt.timeoutId); - attempt.controller.abort(); - attempt.reject(new Error('Sign in timed out')); - AttemptStateMap.delete(attempt.id); - openToast('atlas-disconnected', { - title: 'The login to Atlas has timed out, please try again.', - variant: 'note', - timeout: 5000, - }); - dispatch({ type: AtlasSignInActions.TimedOut }); - track('Atlas Sign In Timed Out', { entrypoint }); - }; -}; - export const tokenRefreshFailed = (): AtlasSignInThunkAction => { return (dispatch, _getState) => { dispatch({ type: AtlasSignInActions.TokenRefreshFailed }); diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx index 2f201802f6a..8f73198c8f0 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.spec.tsx @@ -164,8 +164,6 @@ describe('AtlasToolCallMessage', function () { it('tracks the sign in prompt once, with the tool name as entrypoint', async function () { const { track } = renderMessage(); - userEvent.click(screen.getByText('Connect to Atlas')); - await waitFor(() => { expect(track).to.have.been.calledWith('Atlas Sign In Prompt Shown', { entrypoint: 'assistant-tool-atlas-connection-error-debugger', @@ -202,8 +200,6 @@ describe('AtlasToolCallMessage', function () { } as unknown as ToolUIPart, }); - userEvent.click(screen.getByText('Connect to Atlas')); - await waitFor(() => { expect(track).to.have.been.calledWith('Atlas Sign In Prompt Shown', { entrypoint: 'assistant-tool-atlas-some-future-tool', diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx index c704d27b289..35246306be6 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useMemo } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef } from 'react'; import { css, InlineDefinition, @@ -26,6 +26,7 @@ import { } from '@mongodb-js/atlas-service/provider'; import { CustomToolResult } from './custom-tool-result'; import { getToolCallTitle } from './tool-call-title'; +import { useTelemetry } from '@mongodb-js/compass-telemetry/provider'; /** * Every sign in this card drives is attributed to the assistant tool call that @@ -93,6 +94,33 @@ export const AtlasToolCallMessage: React.FunctionComponent< const isUserSignedIn = !!atlasSignInStatus.user; const isSignInInProgress = atlasSignInStatus?.state === 'in-progress'; const { signIn } = useAtlasLoginActions(); + const track = useTelemetry(); + + const isSignInStateResolved = + atlasSignInStatus.state !== 'initial' && + atlasSignInStatus.state !== 'restoring'; + + // The card re-renders on every state change, so we only report the prompt the + // first time it's actually offered to a signed out user. + const trackedPromptForApprovalId = useRef(null); + const isSignInPromptShown = + isAwaitingApproval && + isSignInStateResolved && + !isUserSignedIn && + !!approvalId; + + useEffect(() => { + if ( + !isSignInPromptShown || + trackedPromptForApprovalId.current === approvalId + ) { + return; + } + trackedPromptForApprovalId.current = approvalId ?? null; + track('Atlas Sign In Prompt Shown', { + entrypoint: getSignInEntrypoint(toolCall.type), + }); + }, [isSignInPromptShown, approvalId, track, toolCall.type]); const chips = useMemo(() => { if ( From bed297e9e6ff12bd4219507fc4de8fddb42b8809 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Tue, 25 Aug 2026 16:44:56 +0100 Subject: [PATCH 09/14] bad merge --- .../src/components/atlas-tool-call-message.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx index 35246306be6..c425ef95bd7 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx @@ -98,10 +98,13 @@ export const AtlasToolCallMessage: React.FunctionComponent< const isSignInStateResolved = atlasSignInStatus.state !== 'initial' && - atlasSignInStatus.state !== 'restoring'; + atlasSignInStatus.state !== 'restoring' && + atlasSignInStatus.state !== 'in-progress'; // The card re-renders on every state change, so we only report the prompt the - // first time it's actually offered to a signed out user. + // first time it's actually offered to a signed out user. We also wait for the + // sign in state to be restored, otherwise an already signed in user looks + // signed out on the first render. const trackedPromptForApprovalId = useRef(null); const isSignInPromptShown = isAwaitingApproval && From ad77b41f2ccf33ccf33805c2c9c6c008a02fd556 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Tue, 25 Aug 2026 16:52:44 +0100 Subject: [PATCH 10/14] cancelederror --- .../src/store/atlas-signin-reducer.spec.ts | 20 -------------- .../src/store/atlas-signin-reducer.ts | 27 ++++++++++++------- .../components/atlas-tool-call-message.tsx | 2 +- 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 30e4c6ce664..4cb9d37a68f 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -607,26 +607,6 @@ describe('atlasSignInReducer', function () { }); expect(store.getState()).to.have.property('attemptNumber', 1); }); - - it('increments attemptNumber on each AttemptStart', function () { - const store = configureStore({ atlasAuthService: {} as any }); - - expect(store.getState()).to.have.property('attemptNumber', 1); - store.dispatch({ - type: 'atlas-service/atlas-signin/AttemptStart', - id: 1, - }); - expect(store.getState()).to.have.property('attemptNumber', 2); - store.dispatch({ type: 'atlas-service/atlas-signin/AttemptEnd', id: 1 }); - store.dispatch({ - type: 'atlas-service/atlas-signin/AtlasSignInTimedOut', - }); - store.dispatch({ - type: 'atlas-service/atlas-signin/AttemptStart', - id: 2, - }); - expect(store.getState()).to.have.property('attemptNumber', 3); - }); }); describe('signOut', function () { diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index 80f02e839d6..44ba0a1dd40 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -58,6 +58,12 @@ class TimeoutError extends Error { } } +class CanceledError extends Error { + constructor() { + super('Sign in canceled'); + } +} + // @ts-expect-error TODO(COMPASS-10124): replace enums with const kv objects export const enum AtlasSignInActions { RestoringStart = 'atlas-service/atlas-signin/StartRestoring', @@ -460,17 +466,18 @@ export const signIn = if (signal.aborted) { // the canceled flow must be tracked outside of the signIn function // as it can be triggered by an external caller. - if (!(signal.reason instanceof TimeoutError)) { + if (signal.reason instanceof CanceledError) { return; + } else if (signal.reason instanceof TimeoutError) { + openToast('atlas-timed-out', { + title: 'The login to Atlas has timed out, please try again.', + variant: 'note', + timeout: 5000, + }); + dispatch({ type: AtlasSignInActions.TimedOut }); + track('Atlas Sign In Timed Out', { entrypoint }); + reject(signal.reason); } - openToast('atlas-timed-out', { - title: 'The login to Atlas has timed out, please try again.', - variant: 'note', - timeout: 5000, - }); - dispatch({ type: AtlasSignInActions.TimedOut }); - track('Atlas Sign In Timed Out', { entrypoint }); - reject(signal.reason); } else { openToast('atlas-sign-in-error', { variant: 'important', @@ -499,7 +506,7 @@ export const cancelSignIn = (reason: any): AtlasSignInThunkAction => { return; } const attempt = getAttempt(getState().currentAttemptId); - attempt.controller.abort(); + attempt.controller.abort(new CanceledError()); attempt.reject(reason ?? attempt.controller.signal.reason); AttemptStateMap.delete(attempt.id); dispatch({ type: AtlasSignInActions.Cancel }); diff --git a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx index c425ef95bd7..d3f659640dc 100644 --- a/packages/compass-assistant/src/components/atlas-tool-call-message.tsx +++ b/packages/compass-assistant/src/components/atlas-tool-call-message.tsx @@ -92,7 +92,7 @@ export const AtlasToolCallMessage: React.FunctionComponent< const approvalId = toolCall.approval?.id; const atlasSignInStatus = useAtlasSignInStatus(); const isUserSignedIn = !!atlasSignInStatus.user; - const isSignInInProgress = atlasSignInStatus?.state === 'in-progress'; + const isSignInInProgress = atlasSignInStatus.state === 'in-progress'; const { signIn } = useAtlasLoginActions(); const track = useTelemetry(); From d41a65f8456129baa2af17662dbe2cc087dda9f3 Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Tue, 25 Aug 2026 17:44:45 +0100 Subject: [PATCH 11/14] signin returns SignInAttemptResult instead of rejecting --- .../src/store/atlas-signin-reducer.ts | 59 ++++++------------- 1 file changed, 17 insertions(+), 42 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index 44ba0a1dd40..f197e858077 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -141,9 +141,8 @@ const INITIAL_STATE = { type AttemptState = { id: number; controller: AbortController; - promise: Promise; - resolve: (userInfo: AtlasUserInfo) => void; - reject: (reason?: any) => void; + promise: Promise; + resolve: (attemptResult: SignInAttemptResult) => void; }; export const SIGN_IN_TIMEOUT_MS = 2 * 60 * 1000; // 2 Minutes @@ -158,18 +157,15 @@ function getAttempt(id?: number | null): AttemptState { id = ++attemptId; const controller = new AbortController(); let resolve; - let reject; - const promise = new Promise((res, rej) => { + const promise = new Promise((res, _rej) => { resolve = res; - reject = rej; }); - if (resolve && reject) { + if (resolve) { AttemptStateMap.set(id, { id, controller, promise, resolve: resolve, - reject: reject, }); } } @@ -371,19 +367,9 @@ export const performSignInAttempt = ({ } if (currentAttemptId) { - return toSignInAttemptResult( - getAttempt(currentAttemptId).promise, - getState - ); + return getAttempt(currentAttemptId).promise; } - const attempt = dispatch( - startAttempt(() => { - void dispatch(signIn({ entrypoint })); - }) - ); - // The attemptNumber is incremented when AttemptStart is dispatched, so we - // must track the event after it. track('Atlas Sign In Started', { entrypoint, attempt: attemptNumber, @@ -392,28 +378,18 @@ export const performSignInAttempt = ({ ? state : null, }); + const attempt = dispatch( + startAttempt(() => { + void dispatch(signIn({ entrypoint })); + }) + ); signal?.addEventListener('abort', () => { - dispatch(cancelSignIn(signal.reason)); + dispatch(cancelSignIn()); }); - return toSignInAttemptResult(attempt.promise, getState); + return attempt.promise; }; }; -async function toSignInAttemptResult( - promise: Promise, - getState: () => AtlasSignInState -): Promise { - try { - const userInfo = await promise; - return { status: 'success', userInfo }; - } catch (error) { - const state = getState().state; - if (state === 'timed-out') return { status: 'timed-out' }; - if (state === 'canceled') return { status: 'canceled' }; - return { status: 'error', error: error as Error }; - } -} - /** * Sign into Atlas. To be called when the user isn't signed in yet. */ @@ -428,7 +404,6 @@ export const signIn = id: currentAttemptId, controller, resolve, - reject, } = getAttempt(getState().currentAttemptId); dispatch({ type: AtlasSignInActions.Start }); const signal = controller.signal; @@ -452,7 +427,7 @@ export const signIn = }); dispatch({ type: AtlasSignInActions.Success, userInfo }); AttemptStateMap.clear(); - resolve(userInfo); + resolve({ userInfo, status: 'success' }); }; const timeoutPromise = new Promise((_resolve, reject) => { timeoutId = setTimeout(() => { @@ -476,7 +451,7 @@ export const signIn = }); dispatch({ type: AtlasSignInActions.TimedOut }); track('Atlas Sign In Timed Out', { entrypoint }); - reject(signal.reason); + resolve({ status: 'timed-out' }); } } else { openToast('atlas-sign-in-error', { @@ -489,7 +464,7 @@ export const signIn = error: (err as Error).message, }); - reject(err); + resolve({ status: 'error', error: err as Error }); } AttemptStateMap.delete(currentAttemptId); } finally { @@ -498,7 +473,7 @@ export const signIn = } }; -export const cancelSignIn = (reason: any): AtlasSignInThunkAction => { +export const cancelSignIn = (): AtlasSignInThunkAction => { return (dispatch, getState, { track }) => { // Can't cancel sign in after the flow was finished indicated by current // attempt id being set to null @@ -507,7 +482,7 @@ export const cancelSignIn = (reason: any): AtlasSignInThunkAction => { } const attempt = getAttempt(getState().currentAttemptId); attempt.controller.abort(new CanceledError()); - attempt.reject(reason ?? attempt.controller.signal.reason); + attempt.resolve({ status: 'canceled' }); AttemptStateMap.delete(attempt.id); dispatch({ type: AtlasSignInActions.Cancel }); track('Atlas Sign In Canceled', {}); From 0e11c72a50870cac310c024e9ff834a83c89c68d Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Tue, 25 Aug 2026 18:03:19 +0100 Subject: [PATCH 12/14] lint --- .../src/store/atlas-signin-reducer.spec.ts | 26 +++---------------- 1 file changed, 3 insertions(+), 23 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 4cb9d37a68f..3566bb74984 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -154,7 +154,7 @@ describe('atlasSignInReducer', function () { atlasAuthService: {} as any, }); expect(store.getState()).to.have.nested.property('state', 'initial'); - store.dispatch(cancelSignIn('canceled')); + store.dispatch(cancelSignIn()); expect(store.getState()).to.have.nested.property('state', 'initial'); }); @@ -182,7 +182,7 @@ describe('atlasSignInReducer', function () { // Give it some time for start the sign in attempt. It will be waiting // at isAuthenticated, which never resolves. await new Promise((resolve) => setTimeout(resolve, 100)); - store.dispatch(cancelSignIn('canceled')); + store.dispatch(cancelSignIn()); expect(store.getState()).to.have.nested.property('state', 'canceled'); expect(store.getState()).to.have.property('attemptNumber', 2); @@ -197,7 +197,7 @@ describe('atlasSignInReducer', function () { track, }); - store.dispatch(cancelSignIn('canceled')); + store.dispatch(cancelSignIn()); expect(track).to.not.have.been.calledWith('Atlas Sign In Canceled'); }); @@ -277,26 +277,6 @@ describe('atlasSignInReducer', function () { variant: 'note', }); }); - - it('should not time out if the flow completes before the timeout', async function () { - const track = sandbox.stub(); - const store = configureStore({ - atlasAuthService: { - isAuthenticated: sandbox.stub().resolves(false), - signIn: sandbox.stub().resolves({ sub: '1234' }), - getUserInfo: sandbox.stub().resolves({ sub: '1234' }), - } as any, - track, - }); - - const result = await store.dispatch(performSignInAttempt()); - expect(result).to.have.property('status', 'success'); - expect(store.getState()).to.have.property('state', 'success'); - - await clock.tickAsync(SIGN_IN_TIMEOUT_MS); - expect(store.getState()).to.have.property('state', 'success'); - expect(track).to.not.have.been.calledWith('Atlas Sign In Timed Out'); - }); }); describe('performSignInAttempt', function () { From 95d4b513902645f10435e584af01d820e43124cf Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Wed, 26 Aug 2026 09:50:50 +0100 Subject: [PATCH 13/14] nits and add reason back --- .../src/store/atlas-signin-reducer.spec.ts | 8 +++-- .../src/store/atlas-signin-reducer.ts | 36 +++++++++---------- .../src/components/custom-tool-result.tsx | 6 ++-- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts index 3566bb74984..ed3aeba3652 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.spec.ts @@ -333,8 +333,12 @@ describe('atlasSignInReducer', function () { const signInPromise = store.dispatch( performSignInAttempt({ signal: c.signal }) ); - c.abort(new Error('Aborted from outside')); - expect(await signInPromise).to.deep.equal({ status: 'canceled' }); + const err = new Error('Aborted from outside'); + c.abort(err); + expect(await signInPromise).to.deep.equal({ + status: 'canceled', + reason: err, + }); expect(store.getState()).to.have.property('state', 'canceled'); // Ensure that we are not leaving a dangling store operation that would conflict with our mocks being reset. diff --git a/packages/atlas-service/src/store/atlas-signin-reducer.ts b/packages/atlas-service/src/store/atlas-signin-reducer.ts index f197e858077..b3b575beeed 100644 --- a/packages/atlas-service/src/store/atlas-signin-reducer.ts +++ b/packages/atlas-service/src/store/atlas-signin-reducer.ts @@ -39,7 +39,7 @@ export type AtlasSignInState = { export type SignInAttemptResult = | { status: 'success'; userInfo: AtlasUserInfo } | { status: 'timed-out' } - | { status: 'canceled' } + | { status: 'canceled'; reason?: unknown } | { status: 'error'; error: Error }; export type AtlasSignInThunkAction< @@ -156,18 +156,13 @@ function getAttempt(id?: number | null): AttemptState { if (!id) { id = ++attemptId; const controller = new AbortController(); - let resolve; - const promise = new Promise((res, _rej) => { - resolve = res; + const { promise, resolve } = Promise.withResolvers(); + AttemptStateMap.set(id, { + id, + controller, + promise, + resolve, }); - if (resolve) { - AttemptStateMap.set(id, { - id, - controller, - promise, - resolve: resolve, - }); - } } const attemptState = AttemptStateMap.get(id); if (!attemptState) { @@ -335,7 +330,13 @@ export const restoreSignInState = (): AtlasSignInThunkAction> => { }; const startAttempt = (fn: () => void): AtlasSignInThunkAction => { - return (dispatch) => { + return (dispatch, getState) => { + if (getState().currentAttemptId) { + throw new Error( + "Can't start sign in with prompt while another sign in attempt is in progress" + ); + } + const attempt = getAttempt(); dispatch({ type: AtlasSignInActions.AttemptStart, id: attempt.id }); @@ -384,7 +385,7 @@ export const performSignInAttempt = ({ }) ); signal?.addEventListener('abort', () => { - dispatch(cancelSignIn()); + dispatch(cancelSignIn(signal.reason)); }); return attempt.promise; }; @@ -426,7 +427,6 @@ export const signIn = timeout: 10_000, }); dispatch({ type: AtlasSignInActions.Success, userInfo }); - AttemptStateMap.clear(); resolve({ userInfo, status: 'success' }); }; const timeoutPromise = new Promise((_resolve, reject) => { @@ -466,14 +466,14 @@ export const signIn = resolve({ status: 'error', error: err as Error }); } - AttemptStateMap.delete(currentAttemptId); } finally { + AttemptStateMap.delete(currentAttemptId); // if the timeout is not cleared the promise will be dangling around clearTimeout(timeoutId); } }; -export const cancelSignIn = (): AtlasSignInThunkAction => { +export const cancelSignIn = (reason?: any): AtlasSignInThunkAction => { return (dispatch, getState, { track }) => { // Can't cancel sign in after the flow was finished indicated by current // attempt id being set to null @@ -482,7 +482,7 @@ export const cancelSignIn = (): AtlasSignInThunkAction => { } const attempt = getAttempt(getState().currentAttemptId); attempt.controller.abort(new CanceledError()); - attempt.resolve({ status: 'canceled' }); + attempt.resolve({ status: 'canceled', reason }); AttemptStateMap.delete(attempt.id); dispatch({ type: AtlasSignInActions.Cancel }); track('Atlas Sign In Canceled', {}); diff --git a/packages/compass-assistant/src/components/custom-tool-result.tsx b/packages/compass-assistant/src/components/custom-tool-result.tsx index dc481e61616..2fe8e4e40b5 100644 --- a/packages/compass-assistant/src/components/custom-tool-result.tsx +++ b/packages/compass-assistant/src/components/custom-tool-result.tsx @@ -15,18 +15,18 @@ import type { AtlasConnectionDebugResult } from '@mongodb-js/compass-generative- const cardStyles = css({ borderRadius: spacing[200], - borderColor: palette.gray.light2, + borderColor: palette.gray.light3, }); const cardStylesDarkMode = css({ - borderColor: palette.gray.dark2, + borderColor: palette.gray.dark3, }); const gridStyle = css({ display: 'grid', gridTemplateColumns: 'auto 1fr', columnGap: '16px', - marginTop: '10px', + padding: '12px', }); const titleStyle = css({ From d77c6a2785f30a3b03bba2d2df5b9d1bdcc9404d Mon Sep 17 00:00:00 2001 From: Edjan Michiles Date: Wed, 26 Aug 2026 09:56:21 +0100 Subject: [PATCH 14/14] revert --- .../compass-assistant/src/components/custom-tool-result.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/compass-assistant/src/components/custom-tool-result.tsx b/packages/compass-assistant/src/components/custom-tool-result.tsx index 2fe8e4e40b5..dc481e61616 100644 --- a/packages/compass-assistant/src/components/custom-tool-result.tsx +++ b/packages/compass-assistant/src/components/custom-tool-result.tsx @@ -15,18 +15,18 @@ import type { AtlasConnectionDebugResult } from '@mongodb-js/compass-generative- const cardStyles = css({ borderRadius: spacing[200], - borderColor: palette.gray.light3, + borderColor: palette.gray.light2, }); const cardStylesDarkMode = css({ - borderColor: palette.gray.dark3, + borderColor: palette.gray.dark2, }); const gridStyle = css({ display: 'grid', gridTemplateColumns: 'auto 1fr', columnGap: '16px', - padding: '12px', + marginTop: '10px', }); const titleStyle = css({