fix: drain in-flight preview chain when closing themes:preview

The new add/unlink listeners in this PR mean fs cleanup at the end of
a preview tests (rmSync on the watched dir) fires chokidar events that
schedule a runPreview() the test never awaits. afterEach restored the
fetch stub and called server.close() (fire-and-forget) while runPreview
was still mid-flight; once unstubbed it issued a real fetch that hung
the Linux event loop until GitHub Actions cancelled the job.

Track the in-flight preview promise and await it in close() after the
watcher has stopped emitting. Replace the recursive rerun trampoline
with a do-while loop so a single tracked promise covers the full chain.
Also fixes the same teardown leak when users Ctrl-C the command.

Also makes the serialize-uploads preview test deterministic by gating
the stubbed upload on a test-controlled resolver rather than asserting
upload counts against real chokidar fs events.

Author: luis-almeida

packages/zcli-themes/src/commands/themes/preview.ts

@@ -101,29 +101,32 @@ export default class Preview extends Command {
 
     let previewing = false
     let rerun = false
+    let pendingRun: Promise<void> | undefined
 
     const runPreview = async () => {
-      previewing = true
-      rerun = false
-      try {
-        await preview(themePath, flags)
-        wss.clients.forEach((client) => {
-          if (client.readyState === WebSocket.OPEN) {
-            client.send('reload')
-          }
-        })
-      } catch (e) {
-        this.error(e as Error, { exit: false })
-      } finally {
-        previewing = false
-        if (rerun) runPreview()
-      }
+      do {
+        previewing = true
+        rerun = false
+        try {
+          await preview(themePath, flags)
+          wss.clients.forEach((client) => {
+            if (client.readyState === WebSocket.OPEN) {
+              client.send('reload')
+            }
+          })
+        } catch (e) {
+          this.error(e as Error, { exit: false })
+        } finally {
+          previewing = false
+        }
+      } while (rerun)
+      pendingRun = undefined
     }
 
     const handleThemeChange = (path: string) => {
       this.log(chalk.bold('Change'), path)
       if (previewing) rerun = true
-      else runPreview()
+      else pendingRun = runPreview()
     }
 
     const watcher = chokidar.watch(monitoredPaths, { ignoreInitial: true })
@@ -132,9 +135,17 @@ export default class Preview extends Command {
       .on('unlink', handleThemeChange)
 
     return {
-      close: () => {
-        // Stop watching file changes before terminating the server
-        watcher.close()
+      close: async () => {
+        // Wait for the watcher to flush queued events, then drain any
+        // in-flight preview chain so pending uploads settle before the
+        // server shuts down. Without this, fs cleanup in tests (or a user's
+        // editor) can fire late events whose runPreview() outlives close().
+        await watcher.close()
+        if (pendingRun) {
+          // runPreview() reports its own errors via this.error; we just
+          // need to wait for the chain to settle so close() can proceed.
+          try { await pendingRun } catch { /* already handled */ }
+        }
         server.close()
         wss.close()
       }

packages/zcli-themes/tests/functional/preview.test.ts

@@ -21,7 +21,7 @@ describe('themes:preview', function () {
   })
 
   describe('successful preview', () => {
-    let server: { close: () => void }
+    let server: { close: () => Promise<void> }
 
     const preview = test
       .stdout()
@@ -40,8 +40,8 @@ describe('themes:preview', function () {
         server = await PreviewCommand.run([baseThemePath, '--bind', '0.0.0.0', '--port', '9999'])
       })
 
-    afterEach(() => {
-      server.close()
+    afterEach(async () => {
+      await server.close()
     })
 
     preview
@@ -134,20 +134,35 @@ describe('themes:preview', function () {
     preview
       .it('should serialize rapid successive changes instead of uploading concurrently', async () => {
         await waitForWatcherReady()
+        // Gate each upload on a test-controlled resolver so assertions don't
+        // depend on how the OS's chokidar backend coalesces fs events.
+        const pendingResolvers: Array<() => void> = []
+        fetchStub.withArgs(sinon.match({
+          url: 'https://z3ntest.zendesk.com/hc/api/internal/theming/local_preview',
+          method: 'PUT'
+        })).callsFake(() => new Promise(resolve => {
+          pendingResolvers.push(() => resolve({
+            status: 200,
+            ok: true,
+            text: () => Promise.resolve('')
+          }))
+        }))
         const initialCalls = previewUploadCalls()
         const partialsDir = path.join(baseThemePath, 'templates/partials')
         fs.mkdirSync(partialsDir, { recursive: true })
         const paths = ['a.hbs', 'b.hbs', 'c.hbs'].map(name => path.join(partialsDir, name))
         try {
           for (const p of paths) fs.writeFileSync(p, '<div/>')
           await waitForUploadCount(initialCalls + 1)
-          // Let any queued rerun finish; a working serializer produces at
-          // most one in-flight upload plus one queued rerun.
-          await new Promise(resolve => setTimeout(resolve, 500))
-          const calls = previewUploadCalls() - initialCalls
-          expect(calls).to.be.greaterThan(0)
-          expect(calls).to.be.at.most(2)
+          expect(previewUploadCalls() - initialCalls).to.eq(1)
+          pendingResolvers.shift()?.()
+          await waitForUploadCount(initialCalls + 2)
+          pendingResolvers.shift()?.()
+          await new Promise(resolve => setTimeout(resolve, 100))
+          expect(previewUploadCalls() - initialCalls).to.eq(2)
         } finally {
+          // Drain pending uploads so server.close() in afterEach isn't blocked.
+          pendingResolvers.forEach(fn => fn())
           fs.rmSync(partialsDir, { recursive: true, force: true })
         }
       })