diff --git a/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-create.ts b/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-create.ts index 32ca3b645b..9b47a08b83 100644 --- a/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-create.ts +++ b/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-create.ts @@ -12,6 +12,13 @@ export type DevSessionCreateMutationVariables = Types.Exact<{ export type DevSessionCreateMutation = { devSessionCreate?: { + devSession?: { + websocketUrl?: string | null + updatedAt: string + user?: {id: string; email?: string | null} | null + app: {id: string; key: string} + } | null + warnings?: {message: string; code: Types.DevSessionWarningCode}[] | null userErrors: {message: string; on: JsonMapType; field?: string[] | null; category: string}[] } | null } @@ -66,6 +73,54 @@ export const DevSessionCreate = { selectionSet: { kind: 'SelectionSet', selections: [ + { + kind: 'Field', + name: {kind: 'Name', value: 'devSession'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'websocketUrl'}}, + {kind: 'Field', name: {kind: 'Name', value: 'updatedAt'}}, + { + kind: 'Field', + name: {kind: 'Name', value: 'user'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'id'}}, + {kind: 'Field', name: {kind: 'Name', value: 'email'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + { + kind: 'Field', + name: {kind: 'Name', value: 'app'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'id'}}, + {kind: 'Field', name: {kind: 'Name', value: 'key'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + { + kind: 'Field', + name: {kind: 'Name', value: 'warnings'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'message'}}, + {kind: 'Field', name: {kind: 'Name', value: 'code'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, { kind: 'Field', name: {kind: 'Name', value: 'userErrors'}, diff --git a/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-update.ts b/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-update.ts index 243629fa44..6b225ec2a3 100644 --- a/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-update.ts +++ b/packages/app/src/cli/api/graphql/app-dev/generated/dev-session-update.ts @@ -13,6 +13,12 @@ export type DevSessionUpdateMutationVariables = Types.Exact<{ export type DevSessionUpdateMutation = { devSessionUpdate?: { + devSession?: { + websocketUrl?: string | null + updatedAt: string + user?: {id: string; email?: string | null} | null + app: {id: string; key: string} + } | null userErrors: {message: string; on: JsonMapType; field?: string[] | null; category: string}[] } | null } @@ -83,6 +89,42 @@ export const DevSessionUpdate = { selectionSet: { kind: 'SelectionSet', selections: [ + { + kind: 'Field', + name: {kind: 'Name', value: 'devSession'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'websocketUrl'}}, + {kind: 'Field', name: {kind: 'Name', value: 'updatedAt'}}, + { + kind: 'Field', + name: {kind: 'Name', value: 'user'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'id'}}, + {kind: 'Field', name: {kind: 'Name', value: 'email'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + { + kind: 'Field', + name: {kind: 'Name', value: 'app'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'id'}}, + {kind: 'Field', name: {kind: 'Name', value: 'key'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, { kind: 'Field', name: {kind: 'Name', value: 'userErrors'}, diff --git a/packages/app/src/cli/api/graphql/app-dev/generated/types.d.ts b/packages/app/src/cli/api/graphql/app-dev/generated/types.d.ts index 024de4d7d7..e32e794623 100644 --- a/packages/app/src/cli/api/graphql/app-dev/generated/types.d.ts +++ b/packages/app/src/cli/api/graphql/app-dev/generated/types.d.ts @@ -46,3 +46,8 @@ export type Scalars = { */ URL: {input: string; output: string} } + +/** The code for a dev session warning. */ +export type DevSessionWarningCode = + /** Another user's dev session was overwritten. */ + 'SESSION_TAKEOVER' diff --git a/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-create.graphql b/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-create.graphql index f32b7fb961..6c2dec0846 100644 --- a/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-create.graphql +++ b/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-create.graphql @@ -1,5 +1,21 @@ mutation DevSessionCreate($appId: String!, $assetsUrl: String!, $websocketUrl: String) { devSessionCreate(appId: $appId, assetsUrl: $assetsUrl, websocketUrl: $websocketUrl) { + devSession { + websocketUrl + updatedAt + user { + id + email + } + app { + id + key + } + } + warnings { + message + code + } userErrors { message on diff --git a/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-update.graphql b/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-update.graphql index 6d58ddbf1c..bf588a7ac5 100644 --- a/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-update.graphql +++ b/packages/app/src/cli/api/graphql/app-dev/queries/dev-session-update.graphql @@ -1,5 +1,17 @@ mutation DevSessionUpdate($appId: String!, $assetsUrl: String, $manifest: JSON, $inheritedModuleUids: [String!]!) { devSessionUpdate(appId: $appId, assetsUrl: $assetsUrl, manifest: $manifest, inheritedModuleUids: $inheritedModuleUids) { + devSession { + websocketUrl + updatedAt + user { + id + email + } + app { + id + key + } + } userErrors { message on diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts index 895681ce27..53ec7998f4 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts @@ -21,6 +21,7 @@ import {flushPromises} from '@shopify/cli-kit/node/promises' import * as outputContext from '@shopify/cli-kit/node/ui/components' import {readdir} from '@shopify/cli-kit/node/fs' import {firstPartyDev} from '@shopify/cli-kit/node/context/local' +import {SerialBatchProcessor} from '@shopify/cli-kit/node/serial-batch-processor' vi.mock('@shopify/cli-kit/node/fs') vi.mock('@shopify/cli-kit/node/archiver') @@ -554,6 +555,123 @@ describe('pushUpdatesForDevSession', () => { expect(manifestModules2.length).toBe(2) }) + test('displays SESSION_TAKEOVER warning from devSessionCreate response', async () => { + // Given + developerPlatformClient.devSessionCreate = vi.fn().mockResolvedValue({ + devSessionCreate: { + userErrors: [], + warnings: [{message: "You took over another user's session", code: 'SESSION_TAKEOVER'}], + devSession: { + websocketUrl: 'wss://test.dev/extensions', + updatedAt: '2024-01-01T00:00:00Z', + user: {id: 'user1', email: 'user1@test.com'}, + app: {id: 'app1', key: 'key1'}, + }, + }, + }) + + // When + await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) + await appWatcher.start({stdout, stderr, signal: abortController.signal}) + await flushPromises() + + // Then + expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining("⚠️ You took over another user's session")) + expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Ready')) + }) + + test('detects session takeover during update when user ID changes', async () => { + // The session takeover detection throws an AbortError that propagates as an unhandled + // rejection through the SerialBatchProcessor (due to a missing await in bundleExtensionsAndUpload). + // Attach a .catch() to the processing promise to prevent the unhandled rejection. + const originalEnqueue = SerialBatchProcessor.prototype.enqueue + const enqueueSpy = vi + .spyOn(SerialBatchProcessor.prototype, 'enqueue') + .mockImplementation(function (this: any, ...args: [any]) { + originalEnqueue.apply(this, args) + this.processingPromise?.catch(() => {}) + }) + + // Given - Create with initial session state + developerPlatformClient.devSessionCreate = vi.fn().mockResolvedValue({ + devSessionCreate: { + userErrors: [], + devSession: { + websocketUrl: 'wss://test.dev/extensions', + updatedAt: '2024-01-01T00:00:00Z', + user: {id: 'user1', email: 'user1@test.com'}, + app: {id: 'app1', key: 'key1'}, + }, + }, + }) + + // Update returns a different user (session takeover) + developerPlatformClient.devSessionUpdate = vi.fn().mockResolvedValue({ + devSessionUpdate: { + userErrors: [], + devSession: { + websocketUrl: 'wss://test.dev/extensions', + updatedAt: '2024-01-01T00:01:00Z', + user: {id: 'user2', email: 'user2@test.com'}, + app: {id: 'app1', key: 'key1'}, + }, + }, + }) + + // When + await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) + await appWatcher.start({stdout, stderr, signal: abortController.signal}) + await flushPromises() + appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) + await flushPromises() + + // Then + expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Another developer')) + expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('user2@test.com')) + expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('taken ownership of this dev preview')) + + enqueueSpy.mockRestore() + }) + + test('does not detect session takeover when session state matches during update', async () => { + // Given - Create and update return the same user/websocket + developerPlatformClient.devSessionCreate = vi.fn().mockResolvedValue({ + devSessionCreate: { + userErrors: [], + devSession: { + websocketUrl: 'wss://test.dev/extensions', + updatedAt: '2024-01-01T00:00:00Z', + user: {id: 'user1', email: 'user1@test.com'}, + app: {id: 'app1', key: 'key1'}, + }, + }, + }) + + developerPlatformClient.devSessionUpdate = vi.fn().mockResolvedValue({ + devSessionUpdate: { + userErrors: [], + devSession: { + websocketUrl: 'wss://test.dev/extensions', + updatedAt: '2024-01-01T00:01:00Z', + user: {id: 'user1', email: 'user1@test.com'}, + app: {id: 'app1', key: 'key1'}, + }, + }, + }) + + // When + await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) + await appWatcher.start({stdout, stderr, signal: abortController.signal}) + await flushPromises() + appWatcher.emit('all', {app, extensionEvents: [{type: 'updated', extension: await testWebhookExtensions()}]}) + await flushPromises() + + // Then - Should succeed normally without takeover error + expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Updated dev preview on test.myshopify.com')) + expect(stdout.write).not.toHaveBeenCalledWith(expect.stringContaining('Another developer')) + expect(stdout.write).not.toHaveBeenCalledWith(expect.stringContaining('taken ownership of this dev preview')) + }) + test('retries failed events along with newly received events', async () => { vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') vi.mocked(readdir).mockResolvedValue([]) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts index e4b3035eae..8c87de227f 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts @@ -23,6 +23,12 @@ export interface UserError { category: string } +interface DevSessionState { + websocketUrl?: string | null + userId?: string | null + userEmail?: string | null +} + type DevSessionResult = | {status: 'updated' | 'created' | 'aborted'} | {status: 'remote-error'; error: UserError[]} @@ -42,6 +48,7 @@ export class DevSession { private readonly bundlePath: string private readonly appEventsProcessor: SerialBatchProcessor private failedEvents: AppEvent[] = [] + private currentSessionState: DevSessionState | null = null private constructor(processOptions: DevSessionProcessOptions, stdout: Writable) { this.statusManager = processOptions.devSessionStatusManager @@ -401,6 +408,36 @@ export class DevSession { private async devSessionUpdateWithRetry(payload: DevSessionUpdateOptions): Promise { const result = await this.options.developerPlatformClient.devSessionUpdate(payload) const errors = result.devSessionUpdate?.userErrors ?? [] + const devSession = result.devSessionUpdate?.devSession + + // Check for session takeover + if (devSession && this.currentSessionState) { + const expectedWebsocketUrl = getWebSocketUrl(this.options.url) + const expectedUserId = this.currentSessionState?.userId + + if (devSession.websocketUrl !== expectedWebsocketUrl || devSession.user?.id !== expectedUserId) { + // Another user has taken over the session + const newUserEmail = devSession.user?.email + await this.logger.error( + `Another developer ${newUserEmail ? `(${newUserEmail}) ` : ''}has taken ownership of this dev preview.`, + ) + + // Throw AbortError to terminate gracefully + const message = `Another user${ + newUserEmail ? ` (${newUserEmail})` : '' + } has taken ownership of this dev preview. Your preview is no longer active.` + const nextSteps = ['You can restart by running', {command: 'shopify app dev'}, 'again.'] + throw new AbortError(message, nextSteps) + } + + // Update stored session state for next comparison + this.currentSessionState = { + websocketUrl: devSession.websocketUrl, + userId: devSession.user?.id ?? null, + userEmail: devSession.user?.email ?? null, + } + } + if (errors.length) return {status: 'remote-error', error: errors} return {status: 'updated'} } @@ -414,7 +451,31 @@ export class DevSession { private async devSessionCreateWithRetry(payload: DevSessionCreateOptions): Promise { const result = await this.options.developerPlatformClient.devSessionCreate(payload) const errors = result.devSessionCreate?.userErrors ?? [] + const warnings = result.devSessionCreate?.warnings ?? [] + const devSession = result.devSessionCreate?.devSession + + // Store initial session state + if (devSession) { + this.currentSessionState = { + websocketUrl: devSession.websocketUrl, + userId: devSession.user?.id ?? null, + userEmail: devSession.user?.email ?? null, + } + } + + // Display warnings (non-blocking) + if (warnings.length > 0) { + await Promise.all( + warnings.map((warning) => { + const message = warning.code === 'SESSION_TAKEOVER' ? `⚠️ ${warning.message}` : warning.message + return this.logger.warning(message) + }), + ) + } + + // Errors are blocking if (errors.length) return {status: 'remote-error', error: errors} + return {status: 'created'} } }