Skip to content

Commit ed80b03

Browse files
Trigger auto-upgrade when dev is quit via q or Ctrl+C
The legacy dev UI (Dev.tsx) tore down the process tree with a fixed 2s setTimeout immediately after the user pressed q or hit Ctrl+C. Because oclif only invokes the postrun hook after run() resolves, exiting that early meant the auto-upgrade flow never had a chance to run. Users on the standard `shopify app dev` path therefore stopped receiving CLI auto-upgrades after quitting dev. DevSessionUI.tsx already polled postRunHookHasCompleted() before exiting, but its setInterval was never cleared and the 5s ceiling was too tight for an actual upgrade. This change extracts the shutdown logic into a single helper, waitForPostRunHookAndExit, in cli-kit's postrun module. Both Dev.tsx and DevSessionUI.tsx now call it. The helper: - polls postRunHookHasCompleted() and exits as soon as the hook is done, - caps the wait at a configurable maxWaitMs (default 30s) so a stuck upgrade still terminates the process, - properly clears its interval and guards against double-exit, - delegates to the existing treeKill+process.exit path.
1 parent 95e9788 commit ed80b03

5 files changed

Lines changed: 56 additions & 24 deletions

File tree

packages/app/src/cli/services/dev/ui/components/Dev.test.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ vi.mock('@shopify/cli-kit/node/system', async () => {
2727
vi.mock('../../../context.js')
2828
vi.mock('../../fetch.js')
2929
vi.mock('../../processes/dev-session.js')
30+
vi.mock('@shopify/cli-kit/node/hooks/postrun', async () => {
31+
const actual: any = await vi.importActual('@shopify/cli-kit/node/hooks/postrun')
32+
return {
33+
...actual,
34+
waitForPostRunHookAndExit: vi.fn(),
35+
}
36+
})
3037

3138
const developerPlatformClient = testDeveloperPlatformClient()
3239

packages/app/src/cli/services/dev/ui/components/Dev.tsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import {Box, Text, useInput, useStdin} from '@shopify/cli-kit/node/ink'
1010
import {handleCtrlC} from '@shopify/cli-kit/node/ui'
1111
import {openURL} from '@shopify/cli-kit/node/system'
1212
import figures from '@shopify/cli-kit/node/figures'
13-
import {isUnitTest} from '@shopify/cli-kit/node/context/local'
14-
import {treeKill} from '@shopify/cli-kit/node/tree-kill'
13+
import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun'
1514
import {Writable} from 'stream'
1615

1716
export interface DeveloperPreviewController {
@@ -74,12 +73,7 @@ const Dev: FunctionComponent<DevProps> = ({
7473
setIsShuttingDownMessage('Shutting down dev because of an error ...')
7574
} else {
7675
setIsShuttingDownMessage('Shutting down dev ...')
77-
setTimeout(() => {
78-
if (isUnitTest()) return
79-
treeKill(process.pid, 'SIGINT', false, () => {
80-
process.exit(0)
81-
})
82-
}, 2000)
76+
waitForPostRunHookAndExit()
8377
}
8478
clearInterval(pollingInterval.current)
8579
await developerPreview.disable()

packages/app/src/cli/services/dev/ui/components/DevSessionUI.test.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ vi.mock('@shopify/cli-kit/node/system', async () => {
2424
})
2525
vi.mock('@shopify/cli-kit/node/context/local')
2626
vi.mock('@shopify/cli-kit/node/tree-kill')
27+
vi.mock('@shopify/cli-kit/node/hooks/postrun', async () => {
28+
const actual: any = await vi.importActual('@shopify/cli-kit/node/hooks/postrun')
29+
return {
30+
...actual,
31+
waitForPostRunHookAndExit: vi.fn(),
32+
}
33+
})
2734

2835
const mocks = vi.hoisted(() => {
2936
return {

packages/app/src/cli/services/dev/ui/components/DevSessionUI.tsx

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ import {Box, Text, useInput, useStdin} from '@shopify/cli-kit/node/ink'
1717
import {handleCtrlC} from '@shopify/cli-kit/node/ui'
1818
import {openURL, terminalSupportsHyperlinks} from '@shopify/cli-kit/node/system'
1919
import figures from '@shopify/cli-kit/node/figures'
20-
import {isUnitTest} from '@shopify/cli-kit/node/context/local'
21-
import {treeKill} from '@shopify/cli-kit/node/tree-kill'
22-
import {postRunHookHasCompleted} from '@shopify/cli-kit/node/hooks/postrun'
20+
import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun'
2321
import {Writable} from 'stream'
2422

2523
interface DevStatusShortcut extends TabShortcut {
@@ -70,18 +68,7 @@ const DevSessionUI: FunctionComponent<DevSesionUIProps> = ({
7068
setIsShuttingDownMessage('Shutting down dev ...')
7169
await onAbort()
7270
}
73-
if (isUnitTest()) return
74-
75-
// Wait for the post run hook to complete or timeout after 5 seconds.
76-
let totalTime = 0
77-
setInterval(() => {
78-
if (postRunHookHasCompleted() || totalTime > 5000) {
79-
treeKill(process.pid, 'SIGINT', false, () => {
80-
process.exit(0)
81-
})
82-
}
83-
totalTime += 100
84-
}, 100)
71+
waitForPostRunHookAndExit()
8572
})
8673

8774
const errorHandledProcesses = useMemo(() => {

packages/cli-kit/src/public/node/hooks/postrun.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Postrun hook — uses dynamic imports to avoid loading heavy modules (base-command, analytics)
33
* at module evaluation time. These are only needed after the command has already finished.
44
*/
5+
import {treeKill} from '../tree-kill.js'
56
import {Command, Hook} from '@oclif/core'
67

78
let postRunHookCompleted = false
@@ -15,6 +16,42 @@ export function postRunHookHasCompleted(): boolean {
1516
return postRunHookCompleted
1617
}
1718

19+
/**
20+
* Wait for the postrun hook to finish (so auto-upgrade has a chance to run) and then
21+
* tree-kill the current process tree before exiting.
22+
*
23+
* Long-running interactive commands like `app dev` need this when the user terminates
24+
* the command via `q` or Ctrl+C. The dev sub-processes such as servers and watchers keep
25+
* the event loop alive, so even after oclif's postrun hook completes the node process
26+
* won't exit on its own and we have to `treeKill` the process tree. We must not do that
27+
* before the postrun hook has actually finished running auto-upgrade, otherwise we would
28+
* kill the upgrade mid-way while `npm install` is still running.
29+
*
30+
* The flag `postRunHookCompleted` is flipped at the very end of the hook after
31+
* `autoUpgradeIfNeeded` resolves, so polling it here is safe.
32+
*/
33+
export function waitForPostRunHookAndExit(): void {
34+
const pollIntervalMs = 100
35+
// Auto-upgrade can take a while (npm/pnpm/yarn install). Cap the wait generously so
36+
// a stuck upgrade still terminates the process eventually.
37+
const maxWaitMs = 120000
38+
39+
let elapsed = 0
40+
let terminating = false
41+
const handle = setInterval(() => {
42+
if (terminating) return
43+
if (postRunHookHasCompleted() || elapsed >= maxWaitMs) {
44+
terminating = true
45+
clearInterval(handle)
46+
treeKill(process.pid, 'SIGINT', false, () => {
47+
process.exit(0)
48+
})
49+
return
50+
}
51+
elapsed += pollIntervalMs
52+
}, pollIntervalMs)
53+
}
54+
1855
// This hook is called after each successful command run. More info: https://oclif.io/docs/hooks
1956
export const hook: Hook.Postrun = async ({config, Command}) => {
2057
await detectStopCommand(Command as unknown as typeof Command)
@@ -28,9 +65,9 @@ export const hook: Hook.Postrun = async ({config, Command}) => {
2865
const {outputDebug} = await import('../output.js')
2966
const command = Command.id.replace(/:/g, ' ')
3067
outputDebug(`Completed command ${command}`)
31-
postRunHookCompleted = true
3268

3369
if (!command.includes('notifications') && !command.includes('upgrade')) await autoUpgradeIfNeeded()
70+
postRunHookCompleted = true
3471
}
3572

3673
/**

0 commit comments

Comments
 (0)