From 587b14d2d26b35705dca1754dee19805f4fa5582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 19 Oct 2026 17:40:12 +0200 Subject: [PATCH 01/21] Expose app dev session files for agents --- .changeset/app-dev-instance-lock.md | 5 + docs/README.md | 1 + docs/cli/app-dev-agent-workflow.md | 46 +++++++++ packages/app/src/cli/commands/app/dev.test.ts | 40 ++++++++ packages/app/src/cli/commands/app/dev.ts | 73 +++++++++----- packages/app/src/cli/services/dev.test.ts | 99 +++++++++++++++++++ packages/app/src/cli/services/dev.ts | 77 +++++++++++---- .../dev-session/dev-session-logger.test.ts | 3 +- .../dev-session/dev-session-logger.ts | 2 +- .../dev-session-status-manager.test.ts | 9 ++ .../dev-session/dev-session-status-manager.ts | 7 +- .../dev/processes/dev-session/dev-session.ts | 19 +++- packages/app/src/cli/services/dev/ui.test.tsx | 17 +++- packages/app/src/cli/services/dev/ui.tsx | 28 +++++- 14 files changed, 367 insertions(+), 59 deletions(-) create mode 100644 .changeset/app-dev-instance-lock.md create mode 100644 docs/cli/app-dev-agent-workflow.md diff --git a/.changeset/app-dev-instance-lock.md b/.changeset/app-dev-instance-lock.md new file mode 100644 index 00000000000..e4f8610153c --- /dev/null +++ b/.changeset/app-dev-instance-lock.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': minor +--- + +Record `app dev` sessions, preview status, and JSONL logs so agents can reuse a running session. diff --git a/docs/README.md b/docs/README.md index 34222272eb1..d3dcd34d424 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,6 +15,7 @@ The list below contains valuable resources for people interested in contributing * [Designing for CLI](./cli/designing-for-cli.md) * [Conventions](./cli/conventions.md) * [JSON output contracts](./cli/json-output.md) +* [App dev session files](./cli/app-dev-agent-workflow.md) * [Performance](./cli/performance.md) * [Debugging](./cli/debugging.md) * [ESLint rules](./cli/eslint-rules.md) diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md new file mode 100644 index 00000000000..d2fc10575a9 --- /dev/null +++ b/docs/cli/app-dev-agent-workflow.md @@ -0,0 +1,46 @@ +# App dev session files + +`shopify app dev` writes one instance JSON file and one JSONL log under +`/.shopify/dev/` for each app configuration. Agents can read these files when +dev was started in another terminal. There is no IPC endpoint or extra command. + +The instance JSON includes the PID, `sessionId`, URLs, `logFile`, and +`details.preview`. The preview contains: + +- `state`: `starting`, `updating`, `ready`, or `error`. +- `message`: the current CLI status message, or `null` before one is available. +- `errors`: the current reported errors. Errors remain during retries and clear + when the CLI reports success. + +This is a snapshot of the CLI's reported state, not a health check or proof that a +specific edit has reached the store. Check new output and test the behaviour before +claiming success. Web-server and browser health require their own checks. + +Each complete JSONL line contains `sessionId`, `timestamp`, `source`, and `message`, +with `stream` or `level` where available. Keep a byte offset to read new lines. +Wait for an incomplete final line to finish. If the session ID changes, reset the +offset: each new run clears the log. Logs have no size limit or rotation, so a long +session can produce a large file. + +## Agent workflow + +1. Find the instance file for the intended app configuration and check its app and + store details. Check the PID and lock heartbeat; a saved file alone does not prove + the process is alive. A live PID blocks takeover even after the fixed 30-second + stale interval. Stop an unresponsive owner before starting another instance. +2. Reuse an existing session. If none is available, run `shopify app dev` through + the agent's background terminal facility. Supply the app, configuration, and + store flags and use `--no-input` when setup and authentication are available. +3. If a second start reports `dev_instance_already_running`, read the reported + instance and log. Do not stop a session started by the developer. +4. Make the change, read new output, inspect `details.preview`, and test the result + in the development store. Use a bounded wait and report errors or timeouts. + +Normal shutdown removes the instance record and retains the log until the next run. +Crashes after startup recover automatically on the next start. If the CLI is killed during +the short lock-acquisition step, a `.lock.acquire` directory can remain. The next start +reports its path. Confirm that no other startup is running before removing that +directory and trying again. + +A crash can leave stale metadata or an incomplete log. Keep these local files out +of commits and external reports; application output and local URLs can contain secrets. diff --git a/packages/app/src/cli/commands/app/dev.test.ts b/packages/app/src/cli/commands/app/dev.test.ts index 098e329f753..e9c6aaede13 100644 --- a/packages/app/src/cli/commands/app/dev.test.ts +++ b/packages/app/src/cli/commands/app/dev.test.ts @@ -15,6 +15,11 @@ import { testProject, } from '../../models/app/app.test-data.js' import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' +import { + acquireDevInstance, + DEV_INSTANCE_ALREADY_RUNNING_CODE, + readDevInstance, +} from '@shopify/cli-kit/node/dev-instance' import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' import {beforeEach, describe, expect, test, vi} from 'vitest' @@ -160,4 +165,39 @@ describe('app dev command', () => { expect(devClean).not.toHaveBeenCalled() }) }) + test('records the running instance while dev runs and releases it afterwards', async () => { + await inTemporaryDirectory(async (tmp) => { + const {store} = mockAppAndStore(tmp) + vi.mocked(getTunnelMode).mockResolvedValue({mode: 'auto'}) + const identity = {command: 'app dev', directory: tmp, key: 'shopify.app.toml'} + let instanceWhileRunning: Awaited> + vi.mocked(dev).mockImplementation(async () => { + instanceWhileRunning = await readDevInstance(identity) + }) + + await Dev.run(['--path', tmp, '--store', store.shopDomain], import.meta.url) + + expect(instanceWhileRunning).toMatchObject({command: 'app dev', key: 'shopify.app.toml', pid: process.pid}) + expect(dev).toHaveBeenCalledWith(expect.objectContaining({devInstance: expect.any(Object)})) + await expect(readDevInstance(identity)).resolves.toBeUndefined() + }) + }) + + test('refuses to start when app dev is already running for the same configuration', async () => { + await inTemporaryDirectory(async (tmp) => { + const {store} = mockAppAndStore(tmp) + vi.mocked(getTunnelMode).mockResolvedValue({mode: 'auto'}) + const running = await acquireDevInstance({command: 'app dev', directory: tmp, key: 'shopify.app.toml'}) + + // The command's error handler renders the AbortError and exits. + await expect(Dev.run(['--path', tmp, '--store', store.shopDomain], import.meta.url)).rejects.toThrow() + await expect( + acquireDevInstance({command: 'app dev', directory: tmp, key: 'shopify.app.toml'}), + ).rejects.toMatchObject({code: DEV_INSTANCE_ALREADY_RUNNING_CODE, details: {instance: running.info}}) + + expect(storeContext).not.toHaveBeenCalled() + expect(dev).not.toHaveBeenCalled() + await running.release() + }) + }) }) diff --git a/packages/app/src/cli/commands/app/dev.ts b/packages/app/src/cli/commands/app/dev.ts index 6fe12af0385..71380f3b005 100644 --- a/packages/app/src/cli/commands/app/dev.ts +++ b/packages/app/src/cli/commands/app/dev.ts @@ -9,6 +9,8 @@ import {Flags} from '@oclif/core' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' import {globalFlags, portFlag} from '@shopify/cli-kit/node/cli' import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' +import {acquireDevInstance} from '@shopify/cli-kit/node/dev-instance' +import {basename} from '@shopify/cli-kit/node/path' export default class Dev extends AppLinkedCommand { public static get requiresSyncAnalytics(): boolean { @@ -142,35 +144,54 @@ export default class Dev extends AppLinkedCommand { forceRelink: flags.reset, userProvidedConfigName: flags.config, }) - const store = await storeContext({ - appContextResult, - storeFqdn: flags.store, - forceReselectStore: flags.reset, - storeCreationMode: 'selection-option', + + // One `app dev` per app configuration: a second one reports the running instance and exits, + // so agents can discover a session started in another terminal and read its log. + const devInstance = await acquireDevInstance({ + command: 'app dev', + directory: appContextResult.app.directory, + key: basename(appContextResult.app.configPath), + details: { + configPath: appContextResult.app.configPath, + appName: appContextResult.remoteApp.title, + clientId: appContextResult.remoteApp.apiKey, + }, }) - const devOptions: DevOptions = { - ...appContextResult, - store, - directory: flags.path, - update: !flags['no-update'], - unsafeValidation: flags['unsafe-validation'], - skipDependenciesInstallation: flags['skip-dependencies-installation'], - commandConfig: this.config, - subscriptionProductUrl: flags['subscription-product-url'], - checkoutCartUrl: flags['checkout-cart-url'], - theme: flags.theme, - themeExtensionPort: flags['theme-app-extension-port'], - storePassword: flags['store-password'], - notify: flags.notify, - graphiqlPort: flags['graphiql-port'], - graphiqlKey: flags['graphiql-key'], - // Must stay undefined when the flag is absent: generateCertificate() only prompts when this is nullish. - installMkcert: flags['install-mkcert'], - tunnel: tunnelMode, - } + try { + const store = await storeContext({ + appContextResult, + storeFqdn: flags.store, + forceReselectStore: flags.reset, + storeCreationMode: 'selection-option', + }) - await dev(devOptions) + const devOptions: DevOptions = { + ...appContextResult, + store, + directory: flags.path, + update: !flags['no-update'], + unsafeValidation: flags['unsafe-validation'], + skipDependenciesInstallation: flags['skip-dependencies-installation'], + commandConfig: this.config, + subscriptionProductUrl: flags['subscription-product-url'], + checkoutCartUrl: flags['checkout-cart-url'], + theme: flags.theme, + themeExtensionPort: flags['theme-app-extension-port'], + storePassword: flags['store-password'], + notify: flags.notify, + graphiqlPort: flags['graphiql-port'], + graphiqlKey: flags['graphiql-key'], + // Must stay undefined when the flag is absent: generateCertificate() only prompts when this is nullish. + installMkcert: flags['install-mkcert'], + tunnel: tunnelMode, + devInstance, + } + + await dev(devOptions) + } finally { + await devInstance.release() + } return {app: appContextResult.app} } } diff --git a/packages/app/src/cli/services/dev.test.ts b/packages/app/src/cli/services/dev.test.ts index 119576f884d..dd8ceaaabb2 100644 --- a/packages/app/src/cli/services/dev.test.ts +++ b/packages/app/src/cli/services/dev.test.ts @@ -2,6 +2,7 @@ import {dev, blockIfMigrationIncomplete} from './dev.js' import {setupDevProcesses} from './dev/processes/setup-dev-processes.js' import {renderDev} from './dev/ui.js' import {fetchAppRemoteConfiguration} from './app/select-app.js' +import {DevSessionStatusManager} from './dev/processes/dev-session/dev-session-status-manager.js' import { testAppLinked, testDeveloperPlatformClient, @@ -15,7 +16,11 @@ import {describe, expect, test, vi} from 'vitest' import {hashString} from '@shopify/cli-kit/node/crypto' import {reportAnalyticsEvent} from '@shopify/cli-kit/node/analytics' import {checkPortAvailability, getAvailableTCPPort} from '@shopify/cli-kit/node/tcp' +import {acquireDevInstance} from '@shopify/cli-kit/node/dev-instance' +import {inTemporaryDirectory, readFile} from '@shopify/cli-kit/node/fs' +import {Writable} from 'stream' +vi.mock('./local-storage.js') vi.mock('./dev/fetch.js') vi.mock('./dev/processes/setup-dev-processes.js') vi.mock('./dev/ui.js') @@ -86,6 +91,100 @@ describe('dev', () => { }) }) +describe('dev with a dev instance', () => { + test.each([false, true])('captures output and session URLs, stopping on lock loss: %s', async (loseLock) => { + await inTemporaryDirectory(async (tmp) => { + const store = testOrganizationStore({shopDomain: 'dev-store.myshopify.com'}) + const app = testAppLinked({directory: tmp}) + const manager = new DevSessionStatusManager() + const devInstance = await acquireDevInstance({command: 'app dev', directory: tmp}) + const ownership = new AbortController() + const previewUrl = 'https://dev-store.myshopify.com/admin/apps/api-key' + let infoWhileRunning: unknown + + vi.mocked(fetchAppRemoteConfiguration).mockResolvedValue({ + name: 'Remote app', + application_url: '', + embedded: true, + }) + vi.mocked(getAvailableTCPPort).mockResolvedValue(3456) + vi.mocked(checkPortAvailability).mockResolvedValue(true) + vi.mocked(setupDevProcesses).mockResolvedValue({ + processes: [ + { + type: 'web', + prefix: 'web', + function: async ({stdout, stderr}: {stdout: Writable; stderr: Writable}) => { + stdout.write('Server listening on 3000\n') + stderr.write('Deprecation warning\n') + }, + options: {}, + } as any, + ], + previewUrl, + graphiqlUrl: undefined, + devSessionStatusManager: manager, + }) + const terminal = new Writable({write: (_chunk, _encoding, callback) => callback()}) + vi.mocked(renderDev).mockImplementation(async ({processes, abortController}) => { + manager.updateStatus({isReady: true}) + manager.setMessage('BUILD_ERROR', ['discount: Invalid input']) + await vi.waitFor(async () => { + infoWhileRunning = JSON.parse(await readFile(devInstance.paths.infoFile)) + expect(infoWhileRunning).toHaveProperty('details.preview.errors', ['discount: Invalid input']) + }) + await Promise.all(processes.map((process) => process.action(terminal, terminal, new AbortController().signal))) + if (loseLock) { + ownership.abort(new Error('Lock lost')) + expect(abortController.signal.aborted).toBe(true) + } + }) + + const running = dev({ + app, + project: testProject({usesWorkspaces: true}), + remoteApp: testOrganizationApp({apiKey: 'api-key'}), + organization: testOrganization(), + specifications: [], + developerPlatformClient: testDeveloperPlatformClient(), + store, + directory: app.directory, + update: false, + commandConfig: {} as any, + skipDependenciesInstallation: true, + tunnel: {mode: 'custom', url: 'https://localhost:3456'}, + devInstance: {...devInstance, abortSignal: ownership.signal}, + }) + if (loseLock) { + await expect(running).rejects.toThrow('Lock lost') + } else { + await running + } + await devInstance.release() + + expect(infoWhileRunning).toMatchObject({ + status: 'running', + details: { + storeFqdn: 'dev-store.myshopify.com', + previewUrl, + preview: {state: 'error', errors: ['discount: Invalid input']}, + }, + }) + const entries = (await readFile(devInstance.paths.logFile)) + .split('\n') + .filter(Boolean) + .map((line) => JSON.parse(line)) + expect(entries).toEqual( + expect.arrayContaining([ + expect.objectContaining({source: 'app dev', message: `Preview URL: ${previewUrl}`}), + expect.objectContaining({source: 'web', stream: 'stdout', message: 'Server listening on 3000'}), + expect.objectContaining({source: 'web', stream: 'stderr', message: 'Deprecation warning'}), + ]), + ) + }) + }) +}) + describe('blockIfMigrationIncomplete', () => { const baseConfig = () => ({ localApp: testAppLinked({}), diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index e5a81c4ad65..ba0e56c40e7 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -15,7 +15,7 @@ import {renderDev} from './dev/ui.js' import {DevProcessFunction} from './dev/processes/types.js' import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js' import {fetchAppRemoteConfiguration} from './app/select-app.js' -import {DevSessionStatusManager} from './dev/processes/dev-session/dev-session-status-manager.js' +import {DevSessionStatus, DevSessionStatusManager} from './dev/processes/dev-session/dev-session-status-manager.js' import {TunnelMode} from './dev/tunnel-mode.js' import {PortDetail, renderPortWarnings} from './dev/port-warnings.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' @@ -36,9 +36,10 @@ import {checkPortAvailability, getAvailableTCPPort} from '@shopify/cli-kit/node/ import {TunnelClient} from '@shopify/cli-kit/node/plugins/tunnel' import {getBackendPort} from '@shopify/cli-kit/node/environment' import {reportAnalyticsEvent} from '@shopify/cli-kit/node/analytics' -import {OutputProcess} from '@shopify/cli-kit/node/output' +import {OutputProcess, outputDebug} from '@shopify/cli-kit/node/output' import {hashString} from '@shopify/cli-kit/node/crypto' import {AbortError} from '@shopify/cli-kit/node/error' +import {DevInstance} from '@shopify/cli-kit/node/dev-instance' export interface DevOptions { app: AppLinkedInterface @@ -63,6 +64,8 @@ export interface DevOptions { graphiqlPort?: number graphiqlKey?: string installMkcert?: boolean + /** The running-instance record for this session: its log receives every process's output. */ + devInstance?: DevInstance } export async function dev(commandOptions: DevOptions) { @@ -328,37 +331,77 @@ async function launchDevProcesses({ devSessionStatusManager: DevSessionStatusManager }) { const abortController = new AbortController() + const devInstance = config.commandOptions.devInstance const processesForTaskRunner: OutputProcess[] = processes.map((process) => { const outputProcess: OutputProcess = { prefix: process.prefix, action: async (stdout, stderr, signal) => { const fn = process.function as DevProcessFunction - return fn({stdout, stderr, abortSignal: signal}, process.options) + const processStdout = devInstance ? devInstance.tee(stdout, {source: process.prefix, stream: 'stdout'}) : stdout + const processStderr = devInstance ? devInstance.tee(stderr, {source: process.prefix, stream: 'stderr'}) : stderr + return fn({stdout: processStdout, stderr: processStderr, abortSignal: signal}, process.options) }, } return outputProcess }) + if (devInstance) { + const urls = { + storeFqdn: config.storeFqdn, + previewUrl, + graphiqlUrl, + appUrl: config.localApp.devApplicationURLs?.applicationUrl, + localUrl: config.network.proxyUrl, + } + await devInstance.update({ + status: 'running', + details: {...urls, preview: previewStatus(devSessionStatusManager.status)}, + }) + await devInstance.log({source: 'app dev', level: 'info', message: `Preview URL: ${previewUrl}`}) + } + const developerPlatformClient = config.developerPlatformClient const app = { id: config.remoteApp.id, developerPlatformClient, } - return renderDev({ - processes: processesForTaskRunner, - previewUrl, - graphiqlUrl, - app, - abortController, - shopFqdn: config.storeFqdn, - devSessionStatusManager, - appURL: config.localApp.devApplicationURLs?.applicationUrl, - appName: config.remoteApp.title, - organizationName: config.commandOptions.organization.businessName, - configPath: config.localApp.configPath, - localURL: config.network.proxyUrl, - }) + const persistStatus = (status: DevSessionStatus) => { + devInstance?.update({details: {preview: previewStatus(status)}}).catch((error: Error) => { + outputDebug(`Failed to save dev status: ${error.message}`) + }) + } + if (devInstance) devSessionStatusManager.on('dev-session-update', persistStatus) + const stopOnLostLock = () => abortController.abort(devInstance?.abortSignal.reason) + try { + devInstance?.abortSignal.throwIfAborted() + devInstance?.abortSignal.addEventListener('abort', stopOnLostLock, {once: true}) + await renderDev({ + processes: processesForTaskRunner, + previewUrl, + graphiqlUrl, + app, + abortController, + shopFqdn: config.storeFqdn, + devSessionStatusManager, + appURL: config.localApp.devApplicationURLs?.applicationUrl, + appName: config.remoteApp.title, + organizationName: config.commandOptions.organization.businessName, + configPath: config.localApp.configPath, + localURL: config.network.proxyUrl, + }) + devInstance?.abortSignal.throwIfAborted() + } finally { + devInstance?.abortSignal.removeEventListener('abort', stopOnLostLock) + if (devInstance) devSessionStatusManager.off('dev-session-update', persistStatus) + } +} + +function previewStatus({isReady, statusMessage, errors}: DevSessionStatus) { + let state = isReady ? 'ready' : 'starting' + if (isReady && statusMessage?.type === 'loading') state = 'updating' + if (statusMessage?.type === 'error') state = 'error' + return {state, message: statusMessage?.message ?? null, errors: errors ?? []} } async function logMetadataForDev(options: { diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts index f00878f1a63..66dce20eb7e 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts @@ -44,7 +44,8 @@ describe('DevSessionLogger', () => { output = [] stdout = { write: (message: string) => { - output.push(message) + expect(message.endsWith('\n')).toBe(true) + output.push(message.slice(0, -1)) return true }, } as unknown as Writable diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts index 93bed1786ff..b997deb39c6 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts @@ -112,7 +112,7 @@ export class DevSessionLogger { // Helper function to print to terminal using output context with stripAnsi disabled. private async log(message: string, prefix?: string) { await useConcurrentOutputContext({outputPrefix: prefix ?? 'app-preview', stripAnsi: false}, () => { - this.stdout.write(message) + this.stdout.write(`${message}\n`) }) } } diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.test.ts index 0b577f0bcaf..532dc433296 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.test.ts @@ -140,3 +140,12 @@ describe('DevSessionStatusManager', () => { }) }) }) + +test('keeps current errors during an update and clears them after success', () => { + const manager = new DevSessionStatusManager() + manager.setMessage('BUILD_ERROR', ['discount: Invalid input']) + manager.setMessage('CHANGE_DETECTED') + expect(manager.status.errors).toEqual(['discount: Invalid input']) + manager.setMessage('UPDATED') + expect(manager.status.errors).toEqual([]) +}) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.ts index 7d41b0d3f7d..23ac28baec6 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-status-manager.ts @@ -15,6 +15,7 @@ const DevSessionStaticMessages = { export interface DevSessionStatus { isReady: boolean + errors?: string[] previewURL?: string graphiqlURL?: string appEmbedded?: boolean @@ -44,8 +45,10 @@ export class DevSessionStatusManager extends EventEmitter { this.emit('dev-session-update', newStatus) } - setMessage(message: keyof typeof DevSessionStaticMessages) { - this.updateStatus({statusMessage: DevSessionStaticMessages[message]}) + setMessage(message: keyof typeof DevSessionStaticMessages, errors?: string[]) { + const statusMessage = DevSessionStaticMessages[message] + const nextErrors = statusMessage.type === 'success' ? [] : errors + this.updateStatus({statusMessage, ...(nextErrors ? {errors: nextErrors} : {})}) } get status(): DevSessionStatus { 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 d799ddf127f..0a9dbc318f7 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 @@ -162,6 +162,10 @@ export class DevSession { private async onStart(event: AppEvent) { const errors = this.parseBuildErrors(event) if (errors.length) { + this.statusManager.setMessage( + 'BUILD_ERROR', + errors.map(({prefix, error}) => `${prefix}: ${error}`), + ) await this.logger.logMultipleErrors(errors) await setImmediate(() => { const affected = errors.map((err) => err.prefix) @@ -193,7 +197,10 @@ export class DevSession { const errors = this.parseBuildErrors(event) if (errors.length) { await this.logger.logMultipleErrors(errors) - this.statusManager.setMessage('BUILD_ERROR') + this.statusManager.setMessage( + 'BUILD_ERROR', + errors.map(({prefix, error}) => `${prefix}: ${error}`), + ) return false } @@ -215,7 +222,10 @@ export class DevSession { private parseBuildErrors(event: AppEvent) { const buildErrors = event.extensionEvents.filter((eve) => eve.buildResult?.status === 'error') return buildErrors.map((error) => ({ - error: 'Build error. Please review your code and try again.', + error: + error.buildResult?.status === 'error' + ? error.buildResult.error || 'Build error. Please review your code and try again.' + : 'Build error. Please review your code and try again.', prefix: error.extension.handle, })) } @@ -237,12 +247,13 @@ export class DevSession { await this.logger.logExtensionUpdateMessages(event, result) this.statusManager.setMessage('READY') } else if (result.status === 'remote-error' || result.status === 'unknown-error') { + const errors = result.error instanceof Error ? [result.error.message] : result.error.map((error) => error.message) await this.logger.logUserErrors(result.error, event?.app.allExtensions ?? []) if (result.error instanceof Error && (result.error as Error & {cause?: string}).cause === 'validation-error') { - this.statusManager.setMessage('VALIDATION_ERROR') + this.statusManager.setMessage('VALIDATION_ERROR', errors) } else { if (event) this.failedEvents.push(event) - this.statusManager.setMessage('REMOTE_ERROR') + this.statusManager.setMessage('REMOTE_ERROR', errors) } } diff --git a/packages/app/src/cli/services/dev/ui.test.tsx b/packages/app/src/cli/services/dev/ui.test.tsx index ad6e27c29e8..b9134ebc410 100644 --- a/packages/app/src/cli/services/dev/ui.test.tsx +++ b/packages/app/src/cli/services/dev/ui.test.tsx @@ -27,7 +27,8 @@ describe('ui', () => { } const abortController = new AbortController() - await renderDev({ + const completed = vi.fn() + const running = renderDev({ processes: [concurrentProcess], previewUrl: 'https://lala.cloudflare.io/', graphiqlUrl: 'https://lala.cloudflare.io/graphiql', @@ -35,7 +36,12 @@ describe('ui', () => { abortController, shopFqdn: 'mystore.shopify.io', devSessionStatusManager, - }) + }).then(completed) + await Promise.resolve() + await Promise.resolve() + expect(completed).not.toHaveBeenCalled() + abortController.abort() + await running expect(concurrentProcess.action).toHaveBeenNthCalledWith( 1, @@ -48,9 +54,12 @@ describe('ui', () => { test("shows preview and GraphiQL URLs when terminal doesn't support TTY", async () => { vi.mocked(terminalSupportsPrompting).mockReturnValue(false) const write = vi.spyOn(process.stdout, 'write').mockImplementation(() => true) + const abortController = new AbortController() const concurrentProcess = { prefix: 'prefix', - action: vi.fn(async (_stdout, _stderr, _signal) => {}), + action: vi.fn(async () => { + abortController.abort() + }), } await renderDev({ @@ -58,7 +67,7 @@ describe('ui', () => { previewUrl: 'https://lala.cloudflare.io/', graphiqlUrl: 'https://lala.cloudflare.io/graphiql', app: {id: '123', developerPlatformClient}, - abortController: new AbortController(), + abortController, shopFqdn: 'mystore.shopify.io', devSessionStatusManager, }) diff --git a/packages/app/src/cli/services/dev/ui.tsx b/packages/app/src/cli/services/dev/ui.tsx index 9eb23042433..3c3b207732e 100644 --- a/packages/app/src/cli/services/dev/ui.tsx +++ b/packages/app/src/cli/services/dev/ui.tsx @@ -81,9 +81,29 @@ async function renderDevNonInteractive({ process.stdout.write(`GraphiQL URL (Admin API): ${graphiqlUrl}\n`) } - return Promise.all( - processes.map(async (concurrentProcess) => { - await concurrentProcess.action(process.stdout, process.stderr, abortController.signal) - }), + const {signal} = abortController + const stop = () => abortController.abort() + process.once('SIGINT', stop) + process.once('SIGTERM', stop) + let onAbort: () => void = () => {} + const stopped = new Promise((resolve) => { + onAbort = resolve + signal.addEventListener('abort', onAbort, {once: true}) + if (signal.aborted) resolve() + }) + const running = processes.map(async (concurrentProcess) => + concurrentProcess.action(process.stdout, process.stderr, signal), ) + try { + // A watcher can finish setup while it continues serving. Keep the session registered until abort. + await Promise.all([...running, stopped]) + } catch (error) { + if (!signal.aborted) throw error + } finally { + abortController.abort() + await Promise.allSettled(running) + signal.removeEventListener('abort', onAbort) + process.removeListener('SIGINT', stop) + process.removeListener('SIGTERM', stop) + } } From 0bde2360de01a9379605828c1478e435860f18e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 19 Oct 2026 17:59:13 +0200 Subject: [PATCH 02/21] Document session identity in the dev status file --- docs/cli/app-dev-agent-workflow.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md index d2fc10575a9..3d75a799194 100644 --- a/docs/cli/app-dev-agent-workflow.md +++ b/docs/cli/app-dev-agent-workflow.md @@ -16,11 +16,11 @@ This is a snapshot of the CLI's reported state, not a health check or proof that specific edit has reached the store. Check new output and test the behaviour before claiming success. Web-server and browser health require their own checks. -Each complete JSONL line contains `sessionId`, `timestamp`, `source`, and `message`, +Each complete JSONL line contains `timestamp`, `source`, and `message`, with `stream` or `level` where available. Keep a byte offset to read new lines. -Wait for an incomplete final line to finish. If the session ID changes, reset the -offset: each new run clears the log. Logs have no size limit or rotation, so a long -session can produce a large file. +Wait for an incomplete final line to finish. If `sessionId` in the instance JSON +changes, reset the offset: each new run clears the log. Logs have no size limit or +rotation, so a long session can produce a large file. ## Agent workflow From 3197d4a88f335b934e71602e3906e59abcfce71e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 10:25:08 +0200 Subject: [PATCH 03/21] Keep app dev log messages and status transitions aligned with terminal output --- docs/cli/app-dev-agent-workflow.md | 4 +++ packages/app/src/cli/services/dev.test.ts | 34 +++++++++++++++++++++-- packages/app/src/cli/services/dev.ts | 31 ++++++++++++++++++--- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md index 3d75a799194..1d32de47b8e 100644 --- a/docs/cli/app-dev-agent-workflow.md +++ b/docs/cli/app-dev-agent-workflow.md @@ -22,6 +22,10 @@ Wait for an incomplete final line to finish. If `sessionId` in the instance JSON changes, reset the offset: each new run clears the log. Logs have no size limit or rotation, so a long session can produce a large file. +Process messages keep their terminal source, including extension handles. Changes +to the status message also produce a log record with source `app dev`. Terminal +colors, spinners, and screen redraws are not included. + ## Agent workflow 1. Find the instance file for the intended app configuration and check its app and diff --git a/packages/app/src/cli/services/dev.test.ts b/packages/app/src/cli/services/dev.test.ts index dd8ceaaabb2..dee418f91c6 100644 --- a/packages/app/src/cli/services/dev.test.ts +++ b/packages/app/src/cli/services/dev.test.ts @@ -3,6 +3,7 @@ import {setupDevProcesses} from './dev/processes/setup-dev-processes.js' import {renderDev} from './dev/ui.js' import {fetchAppRemoteConfiguration} from './app/select-app.js' import {DevSessionStatusManager} from './dev/processes/dev-session/dev-session-status-manager.js' +import {DevSessionLogger} from './dev/processes/dev-session/dev-session-logger.js' import { testAppLinked, testDeveloperPlatformClient, @@ -18,6 +19,7 @@ import {reportAnalyticsEvent} from '@shopify/cli-kit/node/analytics' import {checkPortAvailability, getAvailableTCPPort} from '@shopify/cli-kit/node/tcp' import {acquireDevInstance} from '@shopify/cli-kit/node/dev-instance' import {inTemporaryDirectory, readFile} from '@shopify/cli-kit/node/fs' +import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components' import {Writable} from 'stream' vi.mock('./local-storage.js') @@ -96,10 +98,14 @@ describe('dev with a dev instance', () => { await inTemporaryDirectory(async (tmp) => { const store = testOrganizationStore({shopDomain: 'dev-store.myshopify.com'}) const app = testAppLinked({directory: tmp}) - const manager = new DevSessionStatusManager() + const manager = new DevSessionStatusManager({ + isReady: false, + statusMessage: {type: 'loading', message: 'Preparing dev preview'}, + }) const devInstance = await acquireDevInstance({command: 'app dev', directory: tmp}) const ownership = new AbortController() const previewUrl = 'https://dev-store.myshopify.com/admin/apps/api-key' + const graphiqlUrl = 'http://localhost:3456/graphiql' let infoWhileRunning: unknown vi.mocked(fetchAppRemoteConfiguration).mockResolvedValue({ @@ -117,18 +123,27 @@ describe('dev with a dev instance', () => { function: async ({stdout, stderr}: {stdout: Writable; stderr: Writable}) => { stdout.write('Server listening on 3000\n') stderr.write('Deprecation warning\n') + useConcurrentOutputContext({outputPrefix: 'discount'}, () => { + stdout.write('Building function discount...') + stdout.write('Building GraphQL types...\n') + }) + await new DevSessionLogger(stdout).info('Update accepted') }, options: {}, } as any, ], previewUrl, - graphiqlUrl: undefined, + graphiqlUrl, devSessionStatusManager: manager, }) const terminal = new Writable({write: (_chunk, _encoding, callback) => callback()}) vi.mocked(renderDev).mockImplementation(async ({processes, abortController}) => { + manager.updateStatus({previewURL: previewUrl}) + manager.setMessage('READY') + manager.setMessage('CHANGE_DETECTED') manager.updateStatus({isReady: true}) manager.setMessage('BUILD_ERROR', ['discount: Invalid input']) + manager.updateStatus({graphiqlURL: graphiqlUrl}) await vi.waitFor(async () => { infoWhileRunning = JSON.parse(await readFile(devInstance.paths.infoFile)) expect(infoWhileRunning).toHaveProperty('details.preview.errors', ['discount: Invalid input']) @@ -177,10 +192,25 @@ describe('dev with a dev instance', () => { expect(entries).toEqual( expect.arrayContaining([ expect.objectContaining({source: 'app dev', message: `Preview URL: ${previewUrl}`}), + expect.objectContaining({source: 'app dev', message: `GraphiQL URL (Admin API): ${graphiqlUrl}`}), expect.objectContaining({source: 'web', stream: 'stdout', message: 'Server listening on 3000'}), expect.objectContaining({source: 'web', stream: 'stderr', message: 'Deprecation warning'}), + expect.objectContaining({source: 'discount', message: 'Building function discount...'}), + expect.objectContaining({source: 'discount', message: 'Building GraphQL types...'}), + expect.objectContaining({source: 'app-preview', message: 'Update accepted'}), ]), ) + expect( + entries + .filter(({source, stream}) => source === 'app dev' && !stream) + .filter(({message}) => !message.startsWith('Preview URL:') && !message.startsWith('GraphiQL URL')) + .map(({level, message}) => ({level, message})), + ).toEqual([ + {level: 'info', message: 'Preparing dev preview'}, + {level: 'info', message: 'Ready, watching for changes in your app'}, + {level: 'info', message: 'Change detected, updating dev preview'}, + {level: 'error', message: 'Build error. Please review your code and try again'}, + ]) }) }) }) diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index ba0e56c40e7..d864e0123ac 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -337,8 +337,12 @@ async function launchDevProcesses({ prefix: process.prefix, action: async (stdout, stderr, signal) => { const fn = process.function as DevProcessFunction - const processStdout = devInstance ? devInstance.tee(stdout, {source: process.prefix, stream: 'stdout'}) : stdout - const processStderr = devInstance ? devInstance.tee(stderr, {source: process.prefix, stream: 'stderr'}) : stderr + const processStdout = devInstance + ? devInstance.tee(stdout, {source: process.prefix, stream: 'stdout', stringWritesAreMessages: true}) + : stdout + const processStderr = devInstance + ? devInstance.tee(stderr, {source: process.prefix, stream: 'stderr', stringWritesAreMessages: true}) + : stderr return fn({stdout: processStdout, stderr: processStderr, abortSignal: signal}, process.options) }, } @@ -358,6 +362,9 @@ async function launchDevProcesses({ details: {...urls, preview: previewStatus(devSessionStatusManager.status)}, }) await devInstance.log({source: 'app dev', level: 'info', message: `Preview URL: ${previewUrl}`}) + if (graphiqlUrl) { + await devInstance.log({source: 'app dev', level: 'info', message: `GraphiQL URL (Admin API): ${graphiqlUrl}`}) + } } const developerPlatformClient = config.developerPlatformClient @@ -366,12 +373,28 @@ async function launchDevProcesses({ developerPlatformClient, } + let lastStatusMessage: DevSessionStatus['statusMessage'] const persistStatus = (status: DevSessionStatus) => { - devInstance?.update({details: {preview: previewStatus(status)}}).catch((error: Error) => { + const writes = [devInstance?.update({details: {preview: previewStatus(status)}})] + const message = status.statusMessage + if (message && (message.message !== lastStatusMessage?.message || message.type !== lastStatusMessage?.type)) { + writes.push( + devInstance?.log({ + source: 'app dev', + level: message.type === 'error' ? 'error' : 'info', + message: message.message, + }), + ) + } + lastStatusMessage = message + Promise.all(writes).catch((error: Error) => { outputDebug(`Failed to save dev status: ${error.message}`) }) } - if (devInstance) devSessionStatusManager.on('dev-session-update', persistStatus) + if (devInstance) { + devSessionStatusManager.on('dev-session-update', persistStatus) + persistStatus(devSessionStatusManager.status) + } const stopOnLostLock = () => abortController.abort(devInstance?.abortSignal.reason) try { devInstance?.abortSignal.throwIfAborted() From 003947dcbad4f8608f37b9c4108278dd87ba358e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 13:45:01 +0200 Subject: [PATCH 04/21] Own app dev sessions in the service --- packages/app/src/cli/commands/app/dev.test.ts | 5 ++- packages/app/src/cli/commands/app/dev.ts | 27 +++---------- packages/app/src/cli/services/dev.ts | 23 ++++++++++- packages/app/src/cli/services/dev/ui.test.tsx | 39 ++++++++++++++++++- 4 files changed, 70 insertions(+), 24 deletions(-) diff --git a/packages/app/src/cli/commands/app/dev.test.ts b/packages/app/src/cli/commands/app/dev.test.ts index e9c6aaede13..a20efa5e5bd 100644 --- a/packages/app/src/cli/commands/app/dev.test.ts +++ b/packages/app/src/cli/commands/app/dev.test.ts @@ -23,7 +23,10 @@ import { import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' import {beforeEach, describe, expect, test, vi} from 'vitest' -vi.mock('../../services/dev.js') +vi.mock('../../services/dev.js', async (importOriginal) => ({ + ...(await importOriginal()), + dev: vi.fn(), +})) vi.mock('../../services/dev-clean.js') vi.mock('../../services/app-context.js') vi.mock('../../services/store-context.js') diff --git a/packages/app/src/cli/commands/app/dev.ts b/packages/app/src/cli/commands/app/dev.ts index 71380f3b005..462f75ff63d 100644 --- a/packages/app/src/cli/commands/app/dev.ts +++ b/packages/app/src/cli/commands/app/dev.ts @@ -1,5 +1,5 @@ import {appFlags} from '../../flags.js' -import {dev, DevOptions} from '../../services/dev.js' +import {dev, DevOptions, DevSession} from '../../services/dev.js' import {checkFolderIsValidApp} from '../../models/app/loader.js' import AppLinkedCommand, {AppLinkedCommandOutput} from '../../utilities/app-linked-command.js' import {linkedAppContext} from '../../services/app-context.js' @@ -9,8 +9,6 @@ import {Flags} from '@oclif/core' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' import {globalFlags, portFlag} from '@shopify/cli-kit/node/cli' import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' -import {acquireDevInstance} from '@shopify/cli-kit/node/dev-instance' -import {basename} from '@shopify/cli-kit/node/path' export default class Dev extends AppLinkedCommand { public static get requiresSyncAnalytics(): boolean { @@ -119,6 +117,8 @@ export default class Dev extends AppLinkedCommand { return 'app dev stop' } + private readonly session = new DevSession() + public async run(): Promise { const {flags} = await this.parse(Dev) @@ -145,20 +145,7 @@ export default class Dev extends AppLinkedCommand { userProvidedConfigName: flags.config, }) - // One `app dev` per app configuration: a second one reports the running instance and exits, - // so agents can discover a session started in another terminal and read its log. - const devInstance = await acquireDevInstance({ - command: 'app dev', - directory: appContextResult.app.directory, - key: basename(appContextResult.app.configPath), - details: { - configPath: appContextResult.app.configPath, - appName: appContextResult.remoteApp.title, - clientId: appContextResult.remoteApp.apiKey, - }, - }) - - try { + return this.session.start(appContextResult, async (devInstance) => { const store = await storeContext({ appContextResult, storeFqdn: flags.store, @@ -189,9 +176,7 @@ export default class Dev extends AppLinkedCommand { } await dev(devOptions) - } finally { - await devInstance.release() - } - return {app: appContextResult.app} + return {app: appContextResult.app} + }) } } diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index d864e0123ac..0e6b68ea554 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -39,7 +39,8 @@ import {reportAnalyticsEvent} from '@shopify/cli-kit/node/analytics' import {OutputProcess, outputDebug} from '@shopify/cli-kit/node/output' import {hashString} from '@shopify/cli-kit/node/crypto' import {AbortError} from '@shopify/cli-kit/node/error' -import {DevInstance} from '@shopify/cli-kit/node/dev-instance' +import {acquireDevInstance, type DevInstance} from '@shopify/cli-kit/node/dev-instance' +import {basename} from '@shopify/cli-kit/node/path' export interface DevOptions { app: AppLinkedInterface @@ -68,6 +69,26 @@ export interface DevOptions { devInstance?: DevInstance } +/** Owns the dev instance across setup, execution, and cleanup. */ +export class DevSession { + async start( + {app, remoteApp}: Pick, + execute: (instance: DevInstance) => Promise, + ): Promise { + const instance = await acquireDevInstance({ + command: 'app dev', + directory: app.directory, + key: basename(app.configPath), + details: {configPath: app.configPath, appName: remoteApp.title, clientId: remoteApp.apiKey}, + }) + try { + return await execute(instance) + } finally { + await instance.release() + } + } +} + export async function dev(commandOptions: DevOptions) { const config = await prepareForDev(commandOptions) await actionsBeforeSettingUpDevProcesses(config) diff --git a/packages/app/src/cli/services/dev/ui.test.tsx b/packages/app/src/cli/services/dev/ui.test.tsx index b9134ebc410..d5fa8e18fcb 100644 --- a/packages/app/src/cli/services/dev/ui.test.tsx +++ b/packages/app/src/cli/services/dev/ui.test.tsx @@ -3,7 +3,7 @@ import {DevSessionUI} from './ui/components/DevSessionUI.js' import {DevSessionStatusManager} from './processes/dev-session/dev-session-status-manager.js' import {testDeveloperPlatformClient} from '../../models/app/app.test-data.js' import {afterEach, describe, expect, test, vi} from 'vitest' -import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' +import {mockAndCaptureOutput, withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output' import {AbortController} from '@shopify/cli-kit/node/abort' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' @@ -19,6 +19,43 @@ afterEach(() => { describe('ui', () => { describe('renderDev', () => { + test('a process failure aborts and waits for the other processes to stop', async () => { + vi.mocked(terminalSupportsPrompting).mockReturnValue(false) + const abortController = new AbortController() + const failure = new Error('Process failed') + let stopped = false + const signalListeners = process.listenerCount('SIGTERM') + await withCapturedStandardStreams(async () => { + await expect( + renderDev({ + processes: [ + { + prefix: 'failed', + action: async () => { + throw failure + }, + }, + { + prefix: 'watcher', + action: async (_stdout, _stderr, signal) => { + await new Promise((resolve) => signal.addEventListener('abort', () => resolve(), {once: true})) + await new Promise((resolve) => setTimeout(resolve, 5)) + stopped = true + }, + }, + ], + previewUrl: 'https://example.com', + app: {id: '123', developerPlatformClient}, + abortController, + shopFqdn: 'dev-store.myshopify.com', + devSessionStatusManager: new DevSessionStatusManager(), + }), + ).rejects.toBe(failure) + }) + expect(stopped).toBe(true) + expect(process.listenerCount('SIGTERM')).toBe(signalListeners) + }) + test("doesn't use ink when terminal doesn't support TTY", async () => { vi.mocked(terminalSupportsPrompting).mockReturnValue(false) const concurrentProcess = { From c1eb1bb8b61184b49977a406af6f1b66160cb737 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 16:46:50 +0200 Subject: [PATCH 05/21] Document the 15-second dev stale interval --- docs/cli/app-dev-agent-workflow.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md index 1d32de47b8e..51587c2cf21 100644 --- a/docs/cli/app-dev-agent-workflow.md +++ b/docs/cli/app-dev-agent-workflow.md @@ -30,7 +30,7 @@ colors, spinners, and screen redraws are not included. 1. Find the instance file for the intended app configuration and check its app and store details. Check the PID and lock heartbeat; a saved file alone does not prove - the process is alive. A live PID blocks takeover even after the fixed 30-second + the process is alive. A live PID blocks takeover even after the fixed 15-second stale interval. Stop an unresponsive owner before starting another instance. 2. Reuse an existing session. If none is available, run `shopify app dev` through the agent's background terminal facility. Supply the app, configuration, and From 4c57c84dbdf69c7644c98165bcda75631f9b6688 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 17:01:48 +0200 Subject: [PATCH 06/21] Flush app dev output before signal shutdown --- docs/cli/app-dev-agent-workflow.md | 1 + .../cli/services/dev/ui.integration.test.ts | 86 +++++++++++++++++++ packages/app/src/cli/services/dev/ui.test.tsx | 2 + packages/app/src/cli/services/dev/ui.tsx | 60 +++++++------ packages/cli/src/bootstrap.ts | 7 +- packages/cli/src/index.ts | 7 +- 6 files changed, 134 insertions(+), 29 deletions(-) create mode 100644 packages/app/src/cli/services/dev/ui.integration.test.ts diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md index 51587c2cf21..54317529875 100644 --- a/docs/cli/app-dev-agent-workflow.md +++ b/docs/cli/app-dev-agent-workflow.md @@ -41,6 +41,7 @@ colors, spinners, and screen redraws are not included. in the development store. Use a bounded wait and report errors or timeouts. Normal shutdown removes the instance record and retains the log until the next run. +SIGINT and SIGTERM cancel active dev work and wait for queued log writes before shutdown. Crashes after startup recover automatically on the next start. If the CLI is killed during the short lock-acquisition step, a `.lock.acquire` directory can remain. The next start reports its path. Confirm that no other startup is running before removing that diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts new file mode 100644 index 00000000000..6a15d8d899e --- /dev/null +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -0,0 +1,86 @@ +import {test, expect} from 'vitest' +import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system' +import {fileExists, inTemporaryDirectory, readFile} from '@shopify/cli-kit/node/fs' +import {devInstancePaths} from '@shopify/cli-kit/node/dev-instance' + +const bootstrapUrl = new URL('../../../../../cli/src/bootstrap.ts', import.meta.url).href +const serviceUrl = new URL('../dev.ts', import.meta.url).href +const uiUrl = new URL('./ui.tsx', import.meta.url).href +const statusUrl = new URL('./processes/dev-session/dev-session-status-manager.ts', import.meta.url).href +const sourceLoaderUrl = new URL('../../../../../cli-kit/test/fixtures/cli-kit-source-loader.js', import.meta.url).href + +test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( + '%s keeps the default bootstrap exit when no dev session is running', + {timeout: 20000}, + async (signal) => { + const script = ` + await import(${JSON.stringify(bootstrapUrl)}) + setInterval(() => {}, 1000) + process.kill(process.pid, ${JSON.stringify(signal)}) + ` + const result = await captureOutputWithExitCode( + process.execPath, + ['--loader', 'ts-node/esm', '--loader', sourceLoaderUrl, '--input-type=module', '--eval', script], + {env: {...process.env, NODE_NO_WARNINGS: '1', SHOPIFY_CLI_ENV: 'development'}}, + ) + expect(result.exitCode, result.stderr).toBe(1) + expect(result.stderr).toBe('') + }, +) + +test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( + '%s waits for dev cancellation and queued log writes with the real bootstrap', + {timeout: 20000}, + async (signal) => { + await inTemporaryDirectory(async (directory) => { + const script = ` + await import(${JSON.stringify(bootstrapUrl)}) + const {DevSession} = await import(${JSON.stringify(serviceUrl)}) + const {renderDev} = await import(${JSON.stringify(uiUrl)}) + const {DevSessionStatusManager} = await import(${JSON.stringify(statusUrl)}) + await new DevSession().start({ + app: {directory: ${JSON.stringify(directory)}, configPath: 'shopify.app.toml'}, + remoteApp: {title: 'Local fixture', apiKey: 'fixture'}, + }, async (instance) => { + const abortController = new AbortController() + await renderDev({ + processes: [{prefix: 'web', action: async (stdout, _stderr, signal) => { + const output = instance.tee(stdout, {source: 'web'}) + for (let index = 0; index < 1000; index++) output.write('line ' + index + '\\n') + const stopped = new Promise((resolve) => signal.addEventListener('abort', () => { + output.write('Stopped\\n') + resolve() + }, {once: true})) + process.kill(process.pid, ${JSON.stringify(signal)}) + await stopped + }}], + previewUrl: 'https://example.com', + app: {id: 'fixture', developerPlatformClient: {}}, + abortController, + shopFqdn: 'fixture.myshopify.com', + devSessionStatusManager: new DevSessionStatusManager(), + }) + }) + process.stdout.write('Session cleaned up\\n') + ` + const result = await captureOutputWithExitCode( + process.execPath, + ['--loader', 'ts-node/esm', '--loader', sourceLoaderUrl, '--input-type=module', '--eval', script], + {env: {...process.env, NODE_NO_WARNINGS: '1', SHOPIFY_CLI_ENV: 'development', SHOPIFY_CLI_NO_ANALYTICS: '1'}}, + ) + expect(result.exitCode, result.stderr).toBe(0) + expect(result.stdout).toContain('Session cleaned up') + const paths = devInstancePaths({command: 'app dev', directory, key: 'shopify.app.toml'}) + const records = (await readFile(paths.logFile)) + .trim() + .split('\n') + .map((line) => JSON.parse(line)) + expect(records.map(({message}) => message)).toEqual([ + ...Array.from({length: 1000}, (_, index) => `line ${index}`), + 'Stopped', + ]) + await expect(fileExists(paths.infoFile)).resolves.toBe(false) + await expect(fileExists(paths.lockFile)).resolves.toBe(false) + }) + }, +) diff --git a/packages/app/src/cli/services/dev/ui.test.tsx b/packages/app/src/cli/services/dev/ui.test.tsx index d5fa8e18fcb..fb94b6dccd5 100644 --- a/packages/app/src/cli/services/dev/ui.test.tsx +++ b/packages/app/src/cli/services/dev/ui.test.tsx @@ -25,6 +25,7 @@ describe('ui', () => { const failure = new Error('Process failed') let stopped = false const signalListeners = process.listenerCount('SIGTERM') + const shutdownListeners = process.listenerCount('shopify:signal') await withCapturedStandardStreams(async () => { await expect( renderDev({ @@ -54,6 +55,7 @@ describe('ui', () => { }) expect(stopped).toBe(true) expect(process.listenerCount('SIGTERM')).toBe(signalListeners) + expect(process.listenerCount('shopify:signal')).toBe(shutdownListeners) }) test("doesn't use ink when terminal doesn't support TTY", async () => { diff --git a/packages/app/src/cli/services/dev/ui.tsx b/packages/app/src/cli/services/dev/ui.tsx index 3c3b207732e..4bd462bec13 100644 --- a/packages/app/src/cli/services/dev/ui.tsx +++ b/packages/app/src/cli/services/dev/ui.tsx @@ -40,34 +40,40 @@ export async function renderDev({ configPath?: string localURL?: string }) { - if (terminalSupportsPrompting()) { - return render( - { - await app.developerPlatformClient.devSessionDelete({appId: app.id, shopFqdn}) - }} - />, - { - exitOnCtrlC: false, - }, - ) - } + const stop = () => abortController.abort() + process.on('shopify:signal', stop) + try { + if (terminalSupportsPrompting()) { + return await render( + { + await app.developerPlatformClient.devSessionDelete({appId: app.id, shopFqdn}) + }} + />, + { + exitOnCtrlC: false, + }, + ) + } - await renderDevNonInteractive({ - processes, - previewUrl, - graphiqlUrl, - abortController, - }) + await renderDevNonInteractive({ + processes, + previewUrl, + graphiqlUrl, + abortController, + }) + } finally { + process.removeListener('shopify:signal', stop) + } } async function renderDevNonInteractive({ diff --git a/packages/cli/src/bootstrap.ts b/packages/cli/src/bootstrap.ts index 0eddb65e982..fb066d519d4 100644 --- a/packages/cli/src/bootstrap.ts +++ b/packages/cli/src/bootstrap.ts @@ -28,10 +28,15 @@ process.on('uncaughtException', async (err) => { await renderUncaughtError(err) process.exit(1) }) +const processEvents: NodeJS.EventEmitter = process +let shutdownRequested = false const signals = ['SIGINT', 'SIGTERM', 'SIGQUIT'] signals.forEach((signal) => { process.on(signal, () => { - process.exit(1) + // A repeated signal must not interrupt a command that is flushing its output. + if (shutdownRequested) return + shutdownRequested = processEvents.emit('shopify:signal', signal) + if (!shutdownRequested) process.exit(1) }) }) diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index a7a087bd4ce..dc7320892c5 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -59,10 +59,15 @@ process.on('uncaughtException', async (err) => { await renderUncaughtError(err) process.exit(1) }) +const processEvents: NodeJS.EventEmitter = process +let shutdownRequested = false const signals = ['SIGINT', 'SIGTERM', 'SIGQUIT'] signals.forEach((signal) => { process.on(signal, () => { - process.exit(1) + // A repeated signal must not interrupt a command that is flushing its output. + if (shutdownRequested) return + shutdownRequested = processEvents.emit('shopify:signal', signal) + if (!shutdownRequested) process.exit(1) }) }) From f1869f2d62b70762be7471ab025a5527b36ef7d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 17:01:56 +0200 Subject: [PATCH 07/21] Capture Javy diagnostics in dev session logs --- .../src/cli/services/function/build.test.ts | 47 +++++++++++++++++-- .../app/src/cli/services/function/build.ts | 5 +- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/packages/app/src/cli/services/function/build.test.ts b/packages/app/src/cli/services/function/build.test.ts index 043ab427ec5..c1bebcd99cd 100644 --- a/packages/app/src/cli/services/function/build.test.ts +++ b/packages/app/src/cli/services/function/build.test.ts @@ -27,6 +27,8 @@ import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {inTemporaryDirectory, mkdir, readFile, writeFile, removeFile} from '@shopify/cli-kit/node/fs' import {build as esBuild} from 'esbuild' import {runWithCommandEvents} from '@shopify/cli-kit/node/command-events' +import {acquireDevInstance} from '@shopify/cli-kit/node/dev-instance' +import {Writable} from 'stream' vi.mock('@shopify/cli-kit/node/system') vi.mock('@shopify/cli-kit/node/node-package-manager', async () => { @@ -478,6 +480,43 @@ describe('bundleExtension', () => { }) describe('runJavy', () => { + test('captures compiler diagnostics in the dev session log in text mode', async () => { + const {exec: execute} = + await vi.importActual('@shopify/cli-kit/node/system') + vi.mocked(exec).mockImplementation(async (_command, _args, options) => + execute( + process.execPath, + ['-e', "process.stderr.write('Javy: missing function export\\n'); process.exitCode = 1"], + options, + ), + ) + await inTemporaryDirectory(async (directory) => { + const fun = await testFunctionExtension({dir: directory}) + const instance = await acquireDevInstance({command: 'app dev', directory}) + let terminal = '' + const target = new Writable({ + write: (chunk, _encoding, done) => { + terminal += chunk.toString() + done() + }, + }) + const output = instance.tee(target, {source: 'discount', stringWritesAreMessages: true}) + try { + await expect(runJavy(fun, {stdout: output, stderr: output, app}, derivedDeps)).rejects.toThrow() + await instance.release() + expect(terminal).toContain('Javy: missing function export') + expect( + (await readFile(instance.paths.logFile)) + .trim() + .split('\n') + .map((line) => JSON.parse(line)), + ).toContainEqual(expect.objectContaining({source: 'discount', message: 'Javy: missing function export'})) + } finally { + await instance.release() + } + }) + }) + test('JSON context honors the supplied compiler sinks and signal', async () => { await inTemporaryDirectory(async (directory) => { const fun = await testFunctionExtension({dir: directory}) @@ -516,8 +555,8 @@ describe('runJavy', () => { ], { cwd: ourFunction.directory, - stderr: 'inherit', - stdout: 'inherit', + stderr, + stdout, signal, }, ) @@ -723,8 +762,8 @@ describe('ExportJavyBuilder', () => { ], { cwd: ourFunction.directory, - stderr: 'inherit', - stdout: 'inherit', + stderr, + stdout, signal, }, ) diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index e1d6e792356..57d09db0137 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -29,7 +29,6 @@ import { packageManagerBinaryCommandForDirectory, readAndParsePackageJson, } from '@shopify/cli-kit/node/node-package-manager' -import {commandEventOutputMode} from '@shopify/cli-kit/node/command-events' import {Writable} from 'stream' export const PREFERRED_FUNCTION_NPM_PACKAGE_MAJOR_VERSION = '2' @@ -406,8 +405,8 @@ export async function runJavy( return exec(javy.path, args, { cwd: fun.directory, - stdout: commandEventOutputMode() === 'json' ? options.stdout : 'inherit', - stderr: commandEventOutputMode() === 'json' ? options.stderr : 'inherit', + stdout: options.stdout, + stderr: options.stderr, signal: options.signal, }) } From 2632d6416b5f743e829cd724e5d308e9ffb1b7a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 18:19:24 +0200 Subject: [PATCH 08/21] Document dev log discovery with identity hashes --- docs/cli/app-dev-agent-workflow.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md index 54317529875..222bd420bae 100644 --- a/docs/cli/app-dev-agent-workflow.md +++ b/docs/cli/app-dev-agent-workflow.md @@ -3,6 +3,7 @@ `shopify app dev` writes one instance JSON file and one JSONL log under `/.shopify/dev/` for each app configuration. Agents can read these files when dev was started in another terminal. There is no IPC endpoint or extra command. +Use the instance JSON's `logFile` field to find the log; filenames include an identity hash. The instance JSON includes the PID, `sessionId`, URLs, `logFile`, and `details.preview`. The preview contains: From 77922335602468633cc45d38c21aa8585cdab0ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 18:50:04 +0200 Subject: [PATCH 09/21] Run dev shutdown subprocess tests from workspace sources --- .../src/cli/services/dev/ui.integration.test.ts | 16 +++++++++++++--- packages/app/test/fixtures/app-source-loader.js | 15 +++++++++++++++ packages/app/test/fixtures/tsconfig.json | 13 +++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 packages/app/test/fixtures/app-source-loader.js create mode 100644 packages/app/test/fixtures/tsconfig.json diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index 6a15d8d899e..c75c414725a 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -2,12 +2,14 @@ import {test, expect} from 'vitest' import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system' import {fileExists, inTemporaryDirectory, readFile} from '@shopify/cli-kit/node/fs' import {devInstancePaths} from '@shopify/cli-kit/node/dev-instance' +import {fileURLToPath} from 'url' const bootstrapUrl = new URL('../../../../../cli/src/bootstrap.ts', import.meta.url).href const serviceUrl = new URL('../dev.ts', import.meta.url).href const uiUrl = new URL('./ui.tsx', import.meta.url).href const statusUrl = new URL('./processes/dev-session/dev-session-status-manager.ts', import.meta.url).href -const sourceLoaderUrl = new URL('../../../../../cli-kit/test/fixtures/cli-kit-source-loader.js', import.meta.url).href +const sourceLoaderUrl = new URL('../../../../test/fixtures/app-source-loader.js', import.meta.url).href +const tsconfigPath = fileURLToPath(new URL('../../../../test/fixtures/tsconfig.json', import.meta.url)) test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( '%s keeps the default bootstrap exit when no dev session is running', @@ -21,7 +23,7 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( const result = await captureOutputWithExitCode( process.execPath, ['--loader', 'ts-node/esm', '--loader', sourceLoaderUrl, '--input-type=module', '--eval', script], - {env: {...process.env, NODE_NO_WARNINGS: '1', SHOPIFY_CLI_ENV: 'development'}}, + {env: {...process.env, NODE_NO_WARNINGS: '1', SHOPIFY_CLI_ENV: 'development', TS_NODE_PROJECT: tsconfigPath}}, ) expect(result.exitCode, result.stderr).toBe(1) expect(result.stderr).toBe('') @@ -66,7 +68,15 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( const result = await captureOutputWithExitCode( process.execPath, ['--loader', 'ts-node/esm', '--loader', sourceLoaderUrl, '--input-type=module', '--eval', script], - {env: {...process.env, NODE_NO_WARNINGS: '1', SHOPIFY_CLI_ENV: 'development', SHOPIFY_CLI_NO_ANALYTICS: '1'}}, + { + env: { + ...process.env, + NODE_NO_WARNINGS: '1', + SHOPIFY_CLI_ENV: 'development', + SHOPIFY_CLI_NO_ANALYTICS: '1', + TS_NODE_PROJECT: tsconfigPath, + }, + }, ) expect(result.exitCode, result.stderr).toBe(0) expect(result.stdout).toContain('Session cleaned up') diff --git a/packages/app/test/fixtures/app-source-loader.js b/packages/app/test/fixtures/app-source-loader.js new file mode 100644 index 00000000000..c94cc9320d5 --- /dev/null +++ b/packages/app/test/fixtures/app-source-loader.js @@ -0,0 +1,15 @@ +const cliKitLoader = await import( + new URL('../../../cli-kit/test/fixtures/cli-kit-source-loader.js', import.meta.url).href +) + +// Unit-test CI has no generated workspace modules for these imports. +const workspaceSources = new Map([ + ['@shopify/organizations', '../../../organizations/src/index.js'], + ['@shopify/theme', '../../../theme/src/index.js'], +]) + +export function resolve(specifier, context, nextResolve) { + const source = workspaceSources.get(specifier) + if (source) return nextResolve(new URL(source, import.meta.url).href, context) + return cliKitLoader.resolve(specifier, context, nextResolve) +} diff --git a/packages/app/test/fixtures/tsconfig.json b/packages/app/test/fixtures/tsconfig.json new file mode 100644 index 00000000000..837f0441caf --- /dev/null +++ b/packages/app/test/fixtures/tsconfig.json @@ -0,0 +1,13 @@ +{ + "extends": "../../../../configurations/tsconfig.json", + "compilerOptions": { + "paths": { + "@shopify/cli-kit/*": ["../../../cli-kit/src/public/*"], + "@shopify/organizations": ["../../../organizations/src/index.ts"], + "@shopify/plugin-did-you-mean": ["../../../plugin-did-you-mean/src/index.ts"], + "@shopify/store": ["../../../store/src/index.ts"], + "@shopify/theme": ["../../../theme/src/index.ts"], + "@shopify/theme/*": ["../../../theme/src/*"] + } + } +} From d445199963599f88bb4913c3f69a5378cbcca73f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 21 Oct 2026 12:28:11 +0200 Subject: [PATCH 10/21] Exit non-interactive dev after the postrun hook --- .../app/src/cli/services/dev/ui.integration.test.ts | 11 ++++++++++- packages/app/src/cli/services/dev/ui.test.tsx | 1 + packages/app/src/cli/services/dev/ui.tsx | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index c75c414725a..c7fb3bdae9c 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -8,6 +8,7 @@ const bootstrapUrl = new URL('../../../../../cli/src/bootstrap.ts', import.meta. const serviceUrl = new URL('../dev.ts', import.meta.url).href const uiUrl = new URL('./ui.tsx', import.meta.url).href const statusUrl = new URL('./processes/dev-session/dev-session-status-manager.ts', import.meta.url).href +const postrunUrl = new URL('../../../../../cli-kit/src/public/node/hooks/postrun.ts', import.meta.url).href const sourceLoaderUrl = new URL('../../../../test/fixtures/app-source-loader.js', import.meta.url).href const tsconfigPath = fileURLToPath(new URL('../../../../test/fixtures/tsconfig.json', import.meta.url)) @@ -31,7 +32,7 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( ) test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( - '%s waits for dev cancellation and queued log writes with the real bootstrap', + '%s flushes queued output and exits with background work after the postrun hook', {timeout: 20000}, async (signal) => { await inTemporaryDirectory(async (directory) => { @@ -40,6 +41,7 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( const {DevSession} = await import(${JSON.stringify(serviceUrl)}) const {renderDev} = await import(${JSON.stringify(uiUrl)}) const {DevSessionStatusManager} = await import(${JSON.stringify(statusUrl)}) + const {hook} = await import(${JSON.stringify(postrunUrl)}) await new DevSession().start({ app: {directory: ${JSON.stringify(directory)}, configPath: 'shopify.app.toml'}, remoteApp: {title: 'Local fixture', apiKey: 'fixture'}, @@ -47,6 +49,7 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( const abortController = new AbortController() await renderDev({ processes: [{prefix: 'web', action: async (stdout, _stderr, signal) => { + setInterval(() => {}, 1000) const output = instance.tee(stdout, {source: 'web'}) for (let index = 0; index < 1000; index++) output.write('line ' + index + '\\n') const stopped = new Promise((resolve) => signal.addEventListener('abort', () => { @@ -64,6 +67,9 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( }) }) process.stdout.write('Session cleaned up\\n') + await hook.call({}, {config: {}, Command: {id: 'app:dev'}}) + process.stdout.write('Postrun completed\\n') + setTimeout(() => process.exit(2), 5000) ` const result = await captureOutputWithExitCode( process.execPath, @@ -74,12 +80,15 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( NODE_NO_WARNINGS: '1', SHOPIFY_CLI_ENV: 'development', SHOPIFY_CLI_NO_ANALYTICS: '1', + SHOPIFY_CLI_FORCE_AUTO_UPGRADE: '0', + CI: '1', TS_NODE_PROJECT: tsconfigPath, }, }, ) expect(result.exitCode, result.stderr).toBe(0) expect(result.stdout).toContain('Session cleaned up') + expect(result.stdout).toContain('Postrun completed') const paths = devInstancePaths({command: 'app dev', directory, key: 'shopify.app.toml'}) const records = (await readFile(paths.logFile)) .trim() diff --git a/packages/app/src/cli/services/dev/ui.test.tsx b/packages/app/src/cli/services/dev/ui.test.tsx index fb94b6dccd5..556f11e3d43 100644 --- a/packages/app/src/cli/services/dev/ui.test.tsx +++ b/packages/app/src/cli/services/dev/ui.test.tsx @@ -8,6 +8,7 @@ import {AbortController} from '@shopify/cli-kit/node/abort' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' vi.mock('@shopify/cli-kit/node/system') +vi.mock('@shopify/cli-kit/node/hooks/postrun', () => ({waitForPostRunHookAndExit: vi.fn()})) vi.mock('./ui/components/DevSessionUI.js') const developerPlatformClient = testDeveloperPlatformClient() diff --git a/packages/app/src/cli/services/dev/ui.tsx b/packages/app/src/cli/services/dev/ui.tsx index 4bd462bec13..7d37c1a38ac 100644 --- a/packages/app/src/cli/services/dev/ui.tsx +++ b/packages/app/src/cli/services/dev/ui.tsx @@ -6,6 +6,7 @@ import {AbortController} from '@shopify/cli-kit/node/abort' import React from 'react' import {render} from '@shopify/cli-kit/node/ui' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' +import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun' interface DevProps { processes: OutputProcess[] @@ -71,6 +72,7 @@ export async function renderDev({ graphiqlUrl, abortController, }) + waitForPostRunHookAndExit() } finally { process.removeListener('shopify:signal', stop) } From 1a873491783d997cf00d844d27d2044fceec3a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 21 Oct 2026 12:40:48 +0200 Subject: [PATCH 11/21] Separate complete messages in non-interactive dev output --- packages/app/src/cli/services/dev/ui.test.tsx | 45 ++++++++++++++++++- packages/app/src/cli/services/dev/ui.tsx | 15 ++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/packages/app/src/cli/services/dev/ui.test.tsx b/packages/app/src/cli/services/dev/ui.test.tsx index 556f11e3d43..0d35081b638 100644 --- a/packages/app/src/cli/services/dev/ui.test.tsx +++ b/packages/app/src/cli/services/dev/ui.test.tsx @@ -6,6 +6,8 @@ import {afterEach, describe, expect, test, vi} from 'vitest' import {mockAndCaptureOutput, withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output' import {AbortController} from '@shopify/cli-kit/node/abort' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' +import {outputWarn, unstyled} from '@shopify/cli-kit/node/output' +import {Writable} from 'stream' vi.mock('@shopify/cli-kit/node/system') vi.mock('@shopify/cli-kit/node/hooks/postrun', () => ({waitForPostRunHookAndExit: vi.fn()})) @@ -20,6 +22,45 @@ afterEach(() => { describe('ui', () => { describe('renderDev', () => { + test('separates CLI messages while preserving line endings and subprocess bytes', async () => { + vi.mocked(terminalSupportsPrompting).mockReturnValue(false) + const abortController = new AbortController() + await withCapturedStandardStreams(async ({stdout, stderr}) => { + await renderDev({ + processes: [ + { + prefix: 'app-logs', + action: async (output, errorOutput) => { + await Promise.all( + [output, errorOutput].map(async (stream) => { + stream.write('') + outputWarn('Error while polling app logs.', stream) + outputWarn('Retrying in 5 seconds.', stream) + stream.write('Existing line ending\r\n') + stream.write('636166c3a9', 'hex') + const unicode = Buffer.from('éclair\n') + stream.write(unicode.subarray(0, 1)) + await new Promise((resolve, reject) => + stream.write(unicode.subarray(1), (error) => (error ? reject(error) : resolve())), + ) + }), + ) + abortController.abort() + }, + }, + ], + previewUrl: 'https://example.com', + app: {id: '123', developerPlatformClient}, + abortController, + shopFqdn: 'dev-store.myshopify.com', + devSessionStatusManager: new DevSessionStatusManager(), + }) + const expected = 'Error while polling app logs.\nRetrying in 5 seconds.\nExisting line ending\r\ncafé\néclair\n' + expect(unstyled(stdout())).toBe(`\nPreview URL: https://example.com\n${expected}`) + expect(unstyled(stderr())).toBe(expected) + }) + }) + test('a process failure aborts and waits for the other processes to stop', async () => { vi.mocked(terminalSupportsPrompting).mockReturnValue(false) const abortController = new AbortController() @@ -85,8 +126,8 @@ describe('ui', () => { expect(concurrentProcess.action).toHaveBeenNthCalledWith( 1, - process.stdout, - process.stderr, + expect.any(Writable), + expect.any(Writable), abortController.signal, ) }) diff --git a/packages/app/src/cli/services/dev/ui.tsx b/packages/app/src/cli/services/dev/ui.tsx index 7d37c1a38ac..fb6c709ff24 100644 --- a/packages/app/src/cli/services/dev/ui.tsx +++ b/packages/app/src/cli/services/dev/ui.tsx @@ -7,6 +7,7 @@ import React from 'react' import {render} from '@shopify/cli-kit/node/ui' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun' +import {Writable} from 'stream' interface DevProps { processes: OutputProcess[] @@ -100,7 +101,7 @@ async function renderDevNonInteractive({ if (signal.aborted) resolve() }) const running = processes.map(async (concurrentProcess) => - concurrentProcess.action(process.stdout, process.stderr, signal), + concurrentProcess.action(createTextOutputStream(process.stdout), createTextOutputStream(process.stderr), signal), ) try { // A watcher can finish setup while it continues serving. Keep the session registered until abort. @@ -115,3 +116,15 @@ async function renderDevNonInteractive({ process.removeListener('SIGTERM', stop) } } + +function createTextOutputStream(target: Writable): Writable { + return new Writable({ + decodeStrings: false, + write(chunk: string | Buffer, encoding, callback) { + // CLI string writes are complete messages; subprocess byte chunks can contain partial lines. + const output = typeof chunk === 'string' ? Buffer.from(chunk, encoding) : chunk + const needsNewline = typeof chunk === 'string' && output.length > 0 && output[output.length - 1] !== 10 + target.write(needsNewline ? Buffer.concat([output, Buffer.from('\n')]) : output, callback) + }, + }) +} From a3cebb622fef14991bbe96b117dfb6e12294ff72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 21 Oct 2026 13:28:32 +0200 Subject: [PATCH 12/21] Use scoped dev instance ownership in the app service --- packages/app/src/cli/commands/app/dev.ts | 6 ++--- packages/app/src/cli/services/dev.ts | 26 ++++++++----------- .../cli/services/dev/ui.integration.test.ts | 4 +-- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/packages/app/src/cli/commands/app/dev.ts b/packages/app/src/cli/commands/app/dev.ts index 462f75ff63d..a32290c0cff 100644 --- a/packages/app/src/cli/commands/app/dev.ts +++ b/packages/app/src/cli/commands/app/dev.ts @@ -1,5 +1,5 @@ import {appFlags} from '../../flags.js' -import {dev, DevOptions, DevSession} from '../../services/dev.js' +import {dev, DevOptions, withDevSession} from '../../services/dev.js' import {checkFolderIsValidApp} from '../../models/app/loader.js' import AppLinkedCommand, {AppLinkedCommandOutput} from '../../utilities/app-linked-command.js' import {linkedAppContext} from '../../services/app-context.js' @@ -117,8 +117,6 @@ export default class Dev extends AppLinkedCommand { return 'app dev stop' } - private readonly session = new DevSession() - public async run(): Promise { const {flags} = await this.parse(Dev) @@ -145,7 +143,7 @@ export default class Dev extends AppLinkedCommand { userProvidedConfigName: flags.config, }) - return this.session.start(appContextResult, async (devInstance) => { + return withDevSession(appContextResult, async (devInstance) => { const store = await storeContext({ appContextResult, storeFqdn: flags.store, diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index 0e6b68ea554..44a9af155da 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -39,7 +39,7 @@ import {reportAnalyticsEvent} from '@shopify/cli-kit/node/analytics' import {OutputProcess, outputDebug} from '@shopify/cli-kit/node/output' import {hashString} from '@shopify/cli-kit/node/crypto' import {AbortError} from '@shopify/cli-kit/node/error' -import {acquireDevInstance, type DevInstance} from '@shopify/cli-kit/node/dev-instance' +import {withDevInstance, type DevInstance} from '@shopify/cli-kit/node/dev-instance' import {basename} from '@shopify/cli-kit/node/path' export interface DevOptions { @@ -69,24 +69,20 @@ export interface DevOptions { devInstance?: DevInstance } -/** Owns the dev instance across setup, execution, and cleanup. */ -export class DevSession { - async start( - {app, remoteApp}: Pick, - execute: (instance: DevInstance) => Promise, - ): Promise { - const instance = await acquireDevInstance({ +/** Runs app development while the configuration's dev instance is owned. */ +export function withDevSession( + {app, remoteApp}: Pick, + execute: (instance: DevInstance) => T | Promise, +): Promise { + return withDevInstance( + { command: 'app dev', directory: app.directory, key: basename(app.configPath), details: {configPath: app.configPath, appName: remoteApp.title, clientId: remoteApp.apiKey}, - }) - try { - return await execute(instance) - } finally { - await instance.release() - } - } + }, + execute, + ) } export async function dev(commandOptions: DevOptions) { diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index c7fb3bdae9c..e9851fb2fa8 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -38,11 +38,11 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( await inTemporaryDirectory(async (directory) => { const script = ` await import(${JSON.stringify(bootstrapUrl)}) - const {DevSession} = await import(${JSON.stringify(serviceUrl)}) + const {withDevSession} = await import(${JSON.stringify(serviceUrl)}) const {renderDev} = await import(${JSON.stringify(uiUrl)}) const {DevSessionStatusManager} = await import(${JSON.stringify(statusUrl)}) const {hook} = await import(${JSON.stringify(postrunUrl)}) - await new DevSession().start({ + await withDevSession({ app: {directory: ${JSON.stringify(directory)}, configPath: 'shopify.app.toml'}, remoteApp: {title: 'Local fixture', apiKey: 'fixture'}, }, async (instance) => { From ade2bbcf919cc427d94c554d1b7ad801b8071fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 21 Oct 2026 15:41:55 +0200 Subject: [PATCH 13/21] Separate source startup and shutdown test deadlines --- packages/app/src/cli/services/dev/ui.integration.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index e9851fb2fa8..422585bfe46 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -33,7 +33,8 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( '%s flushes queued output and exits with background work after the postrun hook', - {timeout: 20000}, + // Source loading has its own allowance; the child separately bounds shutdown after the signal. + {timeout: 60000}, async (signal) => { await inTemporaryDirectory(async (directory) => { const script = ` @@ -56,6 +57,10 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( output.write('Stopped\\n') resolve() }, {once: true})) + setTimeout(() => { + process.stderr.write('Dev shutdown timed out\\n') + process.exit(2) + }, 20000) process.kill(process.pid, ${JSON.stringify(signal)}) await stopped }}], From d632b3fb07fd4997f3c079c34a383be95ce68649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 21 Oct 2026 15:48:04 +0200 Subject: [PATCH 14/21] Remove stale stream options after restacking --- docs/cli/app-dev-agent-workflow.md | 2 +- packages/app/src/cli/services/dev.test.ts | 7 ++++--- packages/app/src/cli/services/dev.ts | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md index 222bd420bae..e609d4f932e 100644 --- a/docs/cli/app-dev-agent-workflow.md +++ b/docs/cli/app-dev-agent-workflow.md @@ -18,7 +18,7 @@ specific edit has reached the store. Check new output and test the behaviour bef claiming success. Web-server and browser health require their own checks. Each complete JSONL line contains `timestamp`, `source`, and `message`, -with `stream` or `level` where available. Keep a byte offset to read new lines. +with `level` where available. Keep a byte offset to read new lines. Wait for an incomplete final line to finish. If `sessionId` in the instance JSON changes, reset the offset: each new run clears the log. Logs have no size limit or rotation, so a long session can produce a large file. diff --git a/packages/app/src/cli/services/dev.test.ts b/packages/app/src/cli/services/dev.test.ts index dee418f91c6..857a528c272 100644 --- a/packages/app/src/cli/services/dev.test.ts +++ b/packages/app/src/cli/services/dev.test.ts @@ -189,12 +189,13 @@ describe('dev with a dev instance', () => { .split('\n') .filter(Boolean) .map((line) => JSON.parse(line)) + entries.forEach((entry) => expect(entry).not.toHaveProperty('stream')) expect(entries).toEqual( expect.arrayContaining([ expect.objectContaining({source: 'app dev', message: `Preview URL: ${previewUrl}`}), expect.objectContaining({source: 'app dev', message: `GraphiQL URL (Admin API): ${graphiqlUrl}`}), - expect.objectContaining({source: 'web', stream: 'stdout', message: 'Server listening on 3000'}), - expect.objectContaining({source: 'web', stream: 'stderr', message: 'Deprecation warning'}), + expect.objectContaining({source: 'web', message: 'Server listening on 3000'}), + expect.objectContaining({source: 'web', message: 'Deprecation warning'}), expect.objectContaining({source: 'discount', message: 'Building function discount...'}), expect.objectContaining({source: 'discount', message: 'Building GraphQL types...'}), expect.objectContaining({source: 'app-preview', message: 'Update accepted'}), @@ -202,7 +203,7 @@ describe('dev with a dev instance', () => { ) expect( entries - .filter(({source, stream}) => source === 'app dev' && !stream) + .filter(({source}) => source === 'app dev') .filter(({message}) => !message.startsWith('Preview URL:') && !message.startsWith('GraphiQL URL')) .map(({level, message}) => ({level, message})), ).toEqual([ diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index 44a9af155da..ef292d7dc30 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -355,10 +355,10 @@ async function launchDevProcesses({ action: async (stdout, stderr, signal) => { const fn = process.function as DevProcessFunction const processStdout = devInstance - ? devInstance.tee(stdout, {source: process.prefix, stream: 'stdout', stringWritesAreMessages: true}) + ? devInstance.tee(stdout, {source: process.prefix, stringWritesAreMessages: true}) : stdout const processStderr = devInstance - ? devInstance.tee(stderr, {source: process.prefix, stream: 'stderr', stringWritesAreMessages: true}) + ? devInstance.tee(stderr, {source: process.prefix, stringWritesAreMessages: true}) : stderr return fn({stdout: processStdout, stderr: processStderr, abortSignal: signal}, process.options) }, From 78e2360c55a68f086ccf012149b9d82e9f96f949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 22 Oct 2026 15:09:50 +0200 Subject: [PATCH 15/21] Document shared dev sessions and test shutdown on Windows --- docs/README.md | 2 +- docs/cli/app-dev-agent-workflow.md | 52 --------------- docs/cli/dev-instance-agent-workflow.md | 64 +++++++++++++++++++ .../cli/services/dev/ui.integration.test.ts | 22 ++++--- 4 files changed, 79 insertions(+), 61 deletions(-) delete mode 100644 docs/cli/app-dev-agent-workflow.md create mode 100644 docs/cli/dev-instance-agent-workflow.md diff --git a/docs/README.md b/docs/README.md index d3dcd34d424..48a88de82a9 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,7 +15,7 @@ The list below contains valuable resources for people interested in contributing * [Designing for CLI](./cli/designing-for-cli.md) * [Conventions](./cli/conventions.md) * [JSON output contracts](./cli/json-output.md) -* [App dev session files](./cli/app-dev-agent-workflow.md) +* [Dev Instance files](./cli/dev-instance-agent-workflow.md) * [Performance](./cli/performance.md) * [Debugging](./cli/debugging.md) * [ESLint rules](./cli/eslint-rules.md) diff --git a/docs/cli/app-dev-agent-workflow.md b/docs/cli/app-dev-agent-workflow.md deleted file mode 100644 index e609d4f932e..00000000000 --- a/docs/cli/app-dev-agent-workflow.md +++ /dev/null @@ -1,52 +0,0 @@ -# App dev session files - -`shopify app dev` writes one instance JSON file and one JSONL log under -`/.shopify/dev/` for each app configuration. Agents can read these files when -dev was started in another terminal. There is no IPC endpoint or extra command. -Use the instance JSON's `logFile` field to find the log; filenames include an identity hash. - -The instance JSON includes the PID, `sessionId`, URLs, `logFile`, and -`details.preview`. The preview contains: - -- `state`: `starting`, `updating`, `ready`, or `error`. -- `message`: the current CLI status message, or `null` before one is available. -- `errors`: the current reported errors. Errors remain during retries and clear - when the CLI reports success. - -This is a snapshot of the CLI's reported state, not a health check or proof that a -specific edit has reached the store. Check new output and test the behaviour before -claiming success. Web-server and browser health require their own checks. - -Each complete JSONL line contains `timestamp`, `source`, and `message`, -with `level` where available. Keep a byte offset to read new lines. -Wait for an incomplete final line to finish. If `sessionId` in the instance JSON -changes, reset the offset: each new run clears the log. Logs have no size limit or -rotation, so a long session can produce a large file. - -Process messages keep their terminal source, including extension handles. Changes -to the status message also produce a log record with source `app dev`. Terminal -colors, spinners, and screen redraws are not included. - -## Agent workflow - -1. Find the instance file for the intended app configuration and check its app and - store details. Check the PID and lock heartbeat; a saved file alone does not prove - the process is alive. A live PID blocks takeover even after the fixed 15-second - stale interval. Stop an unresponsive owner before starting another instance. -2. Reuse an existing session. If none is available, run `shopify app dev` through - the agent's background terminal facility. Supply the app, configuration, and - store flags and use `--no-input` when setup and authentication are available. -3. If a second start reports `dev_instance_already_running`, read the reported - instance and log. Do not stop a session started by the developer. -4. Make the change, read new output, inspect `details.preview`, and test the result - in the development store. Use a bounded wait and report errors or timeouts. - -Normal shutdown removes the instance record and retains the log until the next run. -SIGINT and SIGTERM cancel active dev work and wait for queued log writes before shutdown. -Crashes after startup recover automatically on the next start. If the CLI is killed during -the short lock-acquisition step, a `.lock.acquire` directory can remain. The next start -reports its path. Confirm that no other startup is running before removing that -directory and trying again. - -A crash can leave stale metadata or an incomplete log. Keep these local files out -of commits and external reports; application output and local URLs can contain secrets. diff --git a/docs/cli/dev-instance-agent-workflow.md b/docs/cli/dev-instance-agent-workflow.md new file mode 100644 index 00000000000..295035e7738 --- /dev/null +++ b/docs/cli/dev-instance-agent-workflow.md @@ -0,0 +1,64 @@ +# Dev Instance files + +Long-running dev commands write an instance JSON file and a JSONL log under +`/.shopify/dev/`. Agents can read these files when dev was started in +another terminal. There is no IPC endpoint or extra command. + +The instance JSON includes the command, project directory, PID, `sessionId`, +`status`, `logFile`, and command-specific `details`. `status` is `starting` or +`running`. Use `logFile` to find the log; filenames include an identity hash. +A saved instance file alone does not prove that the process is alive. Check its +PID and lock heartbeat before reusing the session. + +## Read new output + +Each complete JSONL line contains `timestamp` and `message`, with `source` and +`level` where available. Terminal colors, spinners, and screen redraws are excluded. +Keep a byte offset to read new lines. Wait for an incomplete final line to finish. +If `sessionId` in the instance JSON changes, reset the offset: each new run clears +the log. Logs have no size limit or rotation. + +## Agent workflow + +1. Find the instance JSON for the intended command and configuration. Check its + identity details, PID, and lock heartbeat. +2. Reuse an existing session. If none is available, start the dev command through + the agent's background terminal facility. Supply the required configuration and + store flags and use `--no-input` when setup and authentication are available. +3. If a second start reports `dev_instance_already_running`, read the reported + instance and log. Do not stop a session started by the developer. +4. Make the change, read new output, inspect the command-specific status, and test + the result. Use a bounded wait and report errors or timeouts. + +Reported status is a snapshot of the CLI's state, not a health check or proof that +an edit has reached the store. Web-server and browser health require their own checks. + +## Shutdown and recovery + +Normal shutdown removes the instance JSON and lock, and retains the log until the +next run. Ctrl+C cancels dev work and waits for queued log writes. On Unix, +SIGINT and SIGTERM use the same shutdown path. Sending these signals through +`process.kill()` on Windows forces termination and does not drain queued writes. + +A live PID blocks takeover, including after the fixed 15-second stale interval. +Stop an unresponsive owner before starting another instance. Crashes after +startup recover automatically on the next start. If the CLI is killed during the +short lock-acquisition step, a `.lock.acquire` directory can remain. The next +start reports its path. Confirm that no other startup is running before removing +that directory and trying again. + +A crash can leave stale metadata or an incomplete log. Keep these local files out +of commits and external reports; output and local URLs can contain secrets. + +## App dev + +`shopify app dev` owns one session per app configuration. The instance details +include the app configuration, app identity, store, URLs, and `preview`: + +- `state`: `starting`, `updating`, `ready`, or `error`. +- `message`: the current CLI status message, or `null` before one is available. +- `errors`: the current reported errors. Errors remain during retries and clear + when the CLI reports success. + +Process messages keep their terminal source, including extension handles. +Changes to the status message also produce a log record with source `app dev`. diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index 422585bfe46..b97f4830686 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -12,14 +12,20 @@ const postrunUrl = new URL('../../../../../cli-kit/src/public/node/hooks/postrun const sourceLoaderUrl = new URL('../../../../test/fixtures/app-source-loader.js', import.meta.url).href const tsconfigPath = fileURLToPath(new URL('../../../../test/fixtures/tsconfig.json', import.meta.url)) -test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( - '%s keeps the default bootstrap exit when no dev session is running', +// Windows process.kill() terminates immediately instead of delivering a catchable signal. +// Exercise handlers on every platform and OS signal delivery where it is supported. +const shutdownCases = ['SIGINT', 'SIGTERM'].flatMap((signal) => + (process.platform === 'win32' ? ['emit'] : ['emit', 'kill']).map((delivery) => ({signal, delivery})), +) + +test.each(shutdownCases)( + '$delivery $signal keeps the default bootstrap exit when no dev session is running', {timeout: 20000}, - async (signal) => { + async ({signal, delivery}) => { const script = ` await import(${JSON.stringify(bootstrapUrl)}) setInterval(() => {}, 1000) - process.kill(process.pid, ${JSON.stringify(signal)}) + process.${delivery}(${delivery === 'kill' ? 'process.pid, ' : ''}${JSON.stringify(signal)}) ` const result = await captureOutputWithExitCode( process.execPath, @@ -31,11 +37,11 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( }, ) -test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( - '%s flushes queued output and exits with background work after the postrun hook', +test.each(shutdownCases)( + '$delivery $signal flushes queued output and exits with background work after the postrun hook', // Source loading has its own allowance; the child separately bounds shutdown after the signal. {timeout: 60000}, - async (signal) => { + async ({signal, delivery}) => { await inTemporaryDirectory(async (directory) => { const script = ` await import(${JSON.stringify(bootstrapUrl)}) @@ -61,7 +67,7 @@ test.skipIf(process.platform === 'win32').each(['SIGINT', 'SIGTERM'])( process.stderr.write('Dev shutdown timed out\\n') process.exit(2) }, 20000) - process.kill(process.pid, ${JSON.stringify(signal)}) + process.${delivery}(${delivery === 'kill' ? 'process.pid, ' : ''}${JSON.stringify(signal)}) await stopped }}], previewUrl: 'https://example.com', From 349478acbf2d4c5a8cb1301bdc4b14cde7629ae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 22 Oct 2026 15:14:25 +0200 Subject: [PATCH 16/21] Preserve the root process during Windows dev shutdown --- .changeset/windows-dev-shutdown.md | 5 +++++ .../cli/services/dev/ui.integration.test.ts | 10 ++++++++-- .../cli-kit/src/public/node/tree-kill.test.ts | 19 +++++++++++++++++++ packages/cli-kit/src/public/node/tree-kill.ts | 14 ++++++++++++-- 4 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 .changeset/windows-dev-shutdown.md diff --git a/.changeset/windows-dev-shutdown.md b/.changeset/windows-dev-shutdown.md new file mode 100644 index 00000000000..d72e3d26b5d --- /dev/null +++ b/.changeset/windows-dev-shutdown.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Preserve the CLI process while stopping child processes during graceful shutdown on Windows. diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index b97f4830686..a92a5810d14 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -1,7 +1,7 @@ -import {test, expect} from 'vitest' +import {test, expect, vi} from 'vitest' import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system' import {fileExists, inTemporaryDirectory, readFile} from '@shopify/cli-kit/node/fs' -import {devInstancePaths} from '@shopify/cli-kit/node/dev-instance' +import {devInstancePaths, isProcessAlive} from '@shopify/cli-kit/node/dev-instance' import {fileURLToPath} from 'url' const bootstrapUrl = new URL('../../../../../cli/src/bootstrap.ts', import.meta.url).href @@ -49,6 +49,7 @@ test.each(shutdownCases)( const {renderDev} = await import(${JSON.stringify(uiUrl)}) const {DevSessionStatusManager} = await import(${JSON.stringify(statusUrl)}) const {hook} = await import(${JSON.stringify(postrunUrl)}) + const {spawn} = await import('node:child_process') await withDevSession({ app: {directory: ${JSON.stringify(directory)}, configPath: 'shopify.app.toml'}, remoteApp: {title: 'Local fixture', apiKey: 'fixture'}, @@ -57,6 +58,8 @@ test.each(shutdownCases)( await renderDev({ processes: [{prefix: 'web', action: async (stdout, _stderr, signal) => { setInterval(() => {}, 1000) + const child = spawn(process.execPath, ['--eval', 'setInterval(() => {}, 1000); setTimeout(() => process.exit(2), 30000)'], {stdio: 'ignore'}) + process.stderr.write('Background PID: ' + child.pid + '\\n') const output = instance.tee(stdout, {source: 'web'}) for (let index = 0; index < 1000; index++) output.write('line ' + index + '\\n') const stopped = new Promise((resolve) => signal.addEventListener('abort', () => { @@ -97,6 +100,9 @@ test.each(shutdownCases)( }, }, ) + const childPid = Number(result.stderr.match(/Background PID: (\d+)/)?.[1]) + expect(childPid).toBeGreaterThan(0) + await vi.waitFor(() => expect(isProcessAlive(childPid)).toBe(false)) expect(result.exitCode, result.stderr).toBe(0) expect(result.stdout).toContain('Session cleaned up') expect(result.stdout).toContain('Postrun completed') diff --git a/packages/cli-kit/src/public/node/tree-kill.test.ts b/packages/cli-kit/src/public/node/tree-kill.test.ts index b9111b26c32..36d1545dbe9 100644 --- a/packages/cli-kit/src/public/node/tree-kill.test.ts +++ b/packages/cli-kit/src/public/node/tree-kill.test.ts @@ -92,6 +92,25 @@ describe('treeKill', () => { }) }) + test('win32: preserves the root process when killRoot is false', async () => { + vi.stubGlobal('process', {...process, platform: 'win32'}) + vi.mocked(spawn).mockReturnValue(createMockProcess({exitCode: 0}) as any) + + await new Promise((resolve) => { + treeKill('1234', 'SIGINT', false, (error) => { + expect(error).toBeUndefined() + resolve() + }) + }) + + expect(spawn).toHaveBeenCalledWith('powershell.exe', [ + '-NoProfile', + '-NonInteractive', + '-Command', + 'Get-CimInstance Win32_Process -Filter "ParentProcessId = 1234 AND ProcessId <> $PID" -ErrorAction Stop | ForEach-Object { taskkill /pid $_.ProcessId /T /F | Out-Null }', + ]) + }) + test('darwin: recursively builds process tree and kills all processes', async () => { vi.stubGlobal('process', {...process, platform: 'darwin'}) const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true) diff --git a/packages/cli-kit/src/public/node/tree-kill.ts b/packages/cli-kit/src/public/node/tree-kill.ts index dd651866e84..e77fcf6b1ee 100644 --- a/packages/cli-kit/src/public/node/tree-kill.ts +++ b/packages/cli-kit/src/public/node/tree-kill.ts @@ -70,12 +70,22 @@ function adaptedTreeKill( switch (process.platform) { case 'win32': { // Security: Use spawn instead of exec to avoid shell injection when killing processes on Windows - const taskkill = spawn('taskkill', ['/pid', rootPid, '/T', '/F']) + // Exclude the querying PowerShell process as well as the root when only children should stop. + const command = killRoot ? 'taskkill' : 'powershell.exe' + const args = killRoot + ? ['/pid', rootPid, '/T', '/F'] + : [ + '-NoProfile', + '-NonInteractive', + '-Command', + `Get-CimInstance Win32_Process -Filter "ParentProcessId = ${rootPid} AND ProcessId <> $PID" -ErrorAction Stop | ForEach-Object { taskkill /pid $_.ProcessId /T /F | Out-Null }`, + ] + const taskkill = spawn(command, args) taskkill.on('close', (code: number) => { if (code === 0) { callback() } else { - callback(new Error(`taskkill exited with code ${code}`)) + callback(new Error(`${command} exited with code ${code}`)) } }) break From 0405cff6d4c6bb09717f875631afa8e47c323949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 22 Oct 2026 15:32:23 +0200 Subject: [PATCH 17/21] Trace Windows subprocess shutdown in CI --- .github/workflows/tests-manual.yml | 3 ++- .../app/src/cli/services/dev/ui.integration.test.ts | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests-manual.yml b/.github/workflows/tests-manual.yml index db7b539e808..c6218b408bb 100644 --- a/.github/workflows/tests-manual.yml +++ b/.github/workflows/tests-manual.yml @@ -17,6 +17,7 @@ on: - 22.12.0 - 24.1.0 - 26.1.0 + - 26.12.1 os: description: 'Operating system' required: true @@ -59,7 +60,7 @@ jobs: with: node-version: ${{ inputs.node-version }} - name: Unit tests - run: pnpm test --output-style=stream + run: pnpm vitest run packages/app/src/cli/services/dev/ui.integration.test.ts --testNamePattern "emit.*SIGINT.*flushes" - name: Setup tmate session if: ${{ failure() && inputs.debug-enabled }} uses: mxschmitt/action-tmate@35b54afac29c97fb54faba5b513f8fbd1882f113 # v3 diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index a92a5810d14..7dad194b9bd 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -44,6 +44,14 @@ test.each(shutdownCases)( async ({signal, delivery}) => { await inTemporaryDirectory(async (directory) => { const script = ` + const childProcess = (await import('node:child_process')).default + const originalSpawn = childProcess.spawn + childProcess.spawn = (...args) => { + process.stderr.write(JSON.stringify({spawn: args[0], args: args[1], pid: process.pid}) + '\\n') + return originalSpawn(...args) + } + (await import('node:module')).syncBuiltinESMExports() + process.once('exit', (code) => process.stderr.write('Exit status: ' + code + '\\n' + new Error().stack + '\\n')) await import(${JSON.stringify(bootstrapUrl)}) const {withDevSession} = await import(${JSON.stringify(serviceUrl)}) const {renderDev} = await import(${JSON.stringify(uiUrl)}) @@ -103,7 +111,7 @@ test.each(shutdownCases)( const childPid = Number(result.stderr.match(/Background PID: (\d+)/)?.[1]) expect(childPid).toBeGreaterThan(0) await vi.waitFor(() => expect(isProcessAlive(childPid)).toBe(false)) - expect(result.exitCode, result.stderr).toBe(0) + expect(result.exitCode, JSON.stringify({...result, stdout: result.stdout.slice(-500)})).toBe(0) expect(result.stdout).toContain('Session cleaned up') expect(result.stdout).toContain('Postrun completed') const paths = devInstancePaths({command: 'app dev', directory, key: 'shopify.app.toml'}) From 479c9f2f3eb42f1f2e8a6fadd8be0a33a4f53b90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 22 Oct 2026 15:36:36 +0200 Subject: [PATCH 18/21] Trace native Windows signal redelivery --- packages/app/src/cli/services/dev/ui.integration.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index 7dad194b9bd..dc9ca68b9cd 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -44,6 +44,11 @@ test.each(shutdownCases)( async ({signal, delivery}) => { await inTemporaryDirectory(async (directory) => { const script = ` + const originalKill = process.kill.bind(process) + process.kill = (...args) => { + process.stderr.write(JSON.stringify({kill: args, pid: process.pid, stack: new Error().stack}) + '\\n') + return originalKill(...args) + } const childProcess = (await import('node:child_process')).default const originalSpawn = childProcess.spawn childProcess.spawn = (...args) => { From 567898ba12f94e65e5db641128bed0490e61f626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 22 Oct 2026 15:51:24 +0200 Subject: [PATCH 19/21] Let dev shutdown handlers drain output before Windows exit --- .changeset/windows-dev-shutdown.md | 3 ++- .github/workflows/tests-manual.yml | 2 +- package.json | 5 ++++- .../src/cli/services/dev/ui.integration.test.ts | 15 +-------------- patches/when-exit@2.1.5.patch | 13 +++++++++++++ pnpm-lock.yaml | 9 +++++++-- 6 files changed, 28 insertions(+), 19 deletions(-) create mode 100644 patches/when-exit@2.1.5.patch diff --git a/.changeset/windows-dev-shutdown.md b/.changeset/windows-dev-shutdown.md index d72e3d26b5d..dc618b51e56 100644 --- a/.changeset/windows-dev-shutdown.md +++ b/.changeset/windows-dev-shutdown.md @@ -1,5 +1,6 @@ --- '@shopify/cli-kit': patch +'@shopify/cli': patch --- -Preserve the CLI process while stopping child processes during graceful shutdown on Windows. +Preserve graceful shutdown and child-process cleanup on Windows. diff --git a/.github/workflows/tests-manual.yml b/.github/workflows/tests-manual.yml index c6218b408bb..4f7302b128b 100644 --- a/.github/workflows/tests-manual.yml +++ b/.github/workflows/tests-manual.yml @@ -60,7 +60,7 @@ jobs: with: node-version: ${{ inputs.node-version }} - name: Unit tests - run: pnpm vitest run packages/app/src/cli/services/dev/ui.integration.test.ts --testNamePattern "emit.*SIGINT.*flushes" + run: pnpm vitest run packages/app/src/cli/services/dev/ui.integration.test.ts packages/cli-kit/src/public/node/tree-kill.test.ts - name: Setup tmate session if: ${{ failure() && inputs.debug-enabled }} uses: mxschmitt/action-tmate@35b54afac29c97fb54faba5b513f8fbd1882f113 # v3 diff --git a/package.json b/package.json index a16442d5847..ed92b5fd19c 100644 --- a/package.json +++ b/package.json @@ -117,7 +117,10 @@ "protobufjs", "unrs-resolver", "yarn" - ] + ], + "patchedDependencies": { + "when-exit@2.1.5": "patches/when-exit@2.1.5.patch" + } }, "resolutions": { "@types/react": "18.3.12", diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index dc9ca68b9cd..a92a5810d14 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -44,19 +44,6 @@ test.each(shutdownCases)( async ({signal, delivery}) => { await inTemporaryDirectory(async (directory) => { const script = ` - const originalKill = process.kill.bind(process) - process.kill = (...args) => { - process.stderr.write(JSON.stringify({kill: args, pid: process.pid, stack: new Error().stack}) + '\\n') - return originalKill(...args) - } - const childProcess = (await import('node:child_process')).default - const originalSpawn = childProcess.spawn - childProcess.spawn = (...args) => { - process.stderr.write(JSON.stringify({spawn: args[0], args: args[1], pid: process.pid}) + '\\n') - return originalSpawn(...args) - } - (await import('node:module')).syncBuiltinESMExports() - process.once('exit', (code) => process.stderr.write('Exit status: ' + code + '\\n' + new Error().stack + '\\n')) await import(${JSON.stringify(bootstrapUrl)}) const {withDevSession} = await import(${JSON.stringify(serviceUrl)}) const {renderDev} = await import(${JSON.stringify(uiUrl)}) @@ -116,7 +103,7 @@ test.each(shutdownCases)( const childPid = Number(result.stderr.match(/Background PID: (\d+)/)?.[1]) expect(childPid).toBeGreaterThan(0) await vi.waitFor(() => expect(isProcessAlive(childPid)).toBe(false)) - expect(result.exitCode, JSON.stringify({...result, stdout: result.stdout.slice(-500)})).toBe(0) + expect(result.exitCode, result.stderr).toBe(0) expect(result.stdout).toContain('Session cleaned up') expect(result.stdout).toContain('Postrun completed') const paths = devInstancePaths({command: 'app dev', directory, key: 'shopify.app.toml'}) diff --git a/patches/when-exit@2.1.5.patch b/patches/when-exit@2.1.5.patch new file mode 100644 index 00000000000..b7a51c5f530 --- /dev/null +++ b/patches/when-exit@2.1.5.patch @@ -0,0 +1,13 @@ +diff --git a/dist/node/interceptor.js b/dist/node/interceptor.js +index a0be92635ade17f6fd5ca75c441179d82e455dd9..72d1b4114cc80d6ae00b58c4b8198cfef9463b61 100644 +--- a/dist/node/interceptor.js ++++ b/dist/node/interceptor.js +@@ -17,7 +17,7 @@ class Interceptor { + for (const callback of this.callbacks) { + callback(); + } +- if (signal) { ++ if (signal && process.listenerCount(signal) === 0) { + if (IS_WINDOWS && (signal !== 'SIGINT' && signal !== 'SIGTERM' && signal !== 'SIGKILL')) { // Windows doesn't support POSIX signals, but Node emulates these 3 signals for us + process.kill(process.pid, 'SIGTERM'); + } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6671b1afa94..cb8fab2c807 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -34,6 +34,11 @@ overrides: nanoid: 3.3.8 graphql: 16.14.2 +patchedDependencies: + when-exit@2.1.5: + hash: dd8a5ad3c180e5b767f6050ad69222568dd4f0111de3769c24841d0caa8f26f7 + path: patches/when-exit@2.1.5.patch + importers: .: @@ -13424,7 +13429,7 @@ snapshots: atomically@2.1.1: dependencies: stubborn-fs: 2.0.0 - when-exit: 2.1.5 + when-exit: 2.1.5(patch_hash=dd8a5ad3c180e5b767f6050ad69222568dd4f0111de3769c24841d0caa8f26f7) auto-bind@4.0.0: {} @@ -18260,7 +18265,7 @@ snapshots: tr46: 5.1.1 webidl-conversions: 7.0.0 - when-exit@2.1.5: {} + when-exit@2.1.5(patch_hash=dd8a5ad3c180e5b767f6050ad69222568dd4f0111de3769c24841d0caa8f26f7): {} which-boxed-primitive@1.1.1: dependencies: From dce1186d5993f68a607fb8028283f3e1b2cf4603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 22 Oct 2026 15:52:42 +0200 Subject: [PATCH 20/21] Restore the standard manual test workflow --- .github/workflows/tests-manual.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/tests-manual.yml b/.github/workflows/tests-manual.yml index 4f7302b128b..db7b539e808 100644 --- a/.github/workflows/tests-manual.yml +++ b/.github/workflows/tests-manual.yml @@ -17,7 +17,6 @@ on: - 22.12.0 - 24.1.0 - 26.1.0 - - 26.12.1 os: description: 'Operating system' required: true @@ -60,7 +59,7 @@ jobs: with: node-version: ${{ inputs.node-version }} - name: Unit tests - run: pnpm vitest run packages/app/src/cli/services/dev/ui.integration.test.ts packages/cli-kit/src/public/node/tree-kill.test.ts + run: pnpm test --output-style=stream - name: Setup tmate session if: ${{ failure() && inputs.debug-enabled }} uses: mxschmitt/action-tmate@35b54afac29c97fb54faba5b513f8fbd1882f113 # v3 From 83d5e5cb8416fec25fd6ef4c3163e600b472b059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 22 Oct 2026 16:12:43 +0200 Subject: [PATCH 21/21] Allow Windows child cleanup its postrun startup budget --- packages/app/src/cli/services/dev/ui.integration.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/dev/ui.integration.test.ts b/packages/app/src/cli/services/dev/ui.integration.test.ts index a92a5810d14..8879ed3ffd7 100644 --- a/packages/app/src/cli/services/dev/ui.integration.test.ts +++ b/packages/app/src/cli/services/dev/ui.integration.test.ts @@ -11,6 +11,8 @@ const statusUrl = new URL('./processes/dev-session/dev-session-status-manager.ts const postrunUrl = new URL('../../../../../cli-kit/src/public/node/hooks/postrun.ts', import.meta.url).href const sourceLoaderUrl = new URL('../../../../test/fixtures/app-source-loader.js', import.meta.url).href const tsconfigPath = fileURLToPath(new URL('../../../../test/fixtures/tsconfig.json', import.meta.url)) +// Windows starts PowerShell to enumerate and stop the remaining child processes. +const postrunExitTimeoutMs = process.platform === 'win32' ? 15000 : 5000 // Windows process.kill() terminates immediately instead of delivering a catchable signal. // Exercise handlers on every platform and OS signal delivery where it is supported. @@ -83,7 +85,10 @@ test.each(shutdownCases)( process.stdout.write('Session cleaned up\\n') await hook.call({}, {config: {}, Command: {id: 'app:dev'}}) process.stdout.write('Postrun completed\\n') - setTimeout(() => process.exit(2), 5000) + setTimeout(() => { + process.stderr.write('Postrun shutdown timed out\\n') + process.exit(2) + }, ${postrunExitTimeoutMs}) ` const result = await captureOutputWithExitCode( process.execPath,