From 6e5f51cf420f196cd0f629439513be7e5b1bbeb6 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 16 Jul 2026 11:38:31 -0400 Subject: [PATCH 1/2] fix(boxel-cli): register realm-watch signal handlers before the initial poll The realm-watch loop wired up its SIGINT/SIGTERM (and abort-signal) stop triggers only after the initial poll. An interrupt during a slow first poll therefore left the lock file and process-registry entry dangling. Wire up the stop triggers before the initial poll and skip the poll when a stop has already arrived, so Ctrl+C cleans up at any point. The integration tests observed this asynchronously-populated state (the lock file, the registered handlers) with a fixed 150ms sleep, which loses under CI load once lock acquisition, watcher init, and the initial network poll exceed that window. Wait on the state with a bounded poll-until-condition helper that reports the observed state on timeout. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/commands/realm/watch/start.ts | 92 +++++++++++-------- .../tests/integration/realm-watch.test.ts | 79 +++++++++++++--- 2 files changed, 116 insertions(+), 55 deletions(-) diff --git a/packages/boxel-cli/src/commands/realm/watch/start.ts b/packages/boxel-cli/src/commands/realm/watch/start.ts index 01fe5b79af..8643b8725b 100644 --- a/packages/boxel-cli/src/commands/realm/watch/start.ts +++ b/packages/boxel-cli/src/commands/realm/watch/start.ts @@ -567,56 +567,68 @@ export async function watchRealms( // Best effort — registry failures must never block the watch. } - await tickAll(); - scheduleNextTick(); + // Wire up the stop triggers (an abort signal, or SIGINT/SIGTERM when none is + // supplied) before the first poll so an interrupt during a slow initial tick + // still tears down cleanly - clearing the timer, shutting down watchers, + // releasing locks, and unregistering the process. + let resolveDone: () => void = () => {}; + const done = new Promise((resolve) => { + resolveDone = resolve; + }); - await new Promise((resolve) => { - let sigintHandler: (() => void) | null = null; - let sigtermHandler: (() => void) | null = null; + let sigintHandler: (() => void) | null = null; + let sigtermHandler: (() => void) | null = null; - const cleanup = async () => { - if (stopped) return; - stopped = true; - if (timeoutId !== null) { - clearTimeout(timeoutId); - timeoutId = null; - } - for (const w of watchers) w.shutdown(); - if (sigintHandler) process.off('SIGINT', sigintHandler); - if (sigtermHandler) process.off('SIGTERM', sigtermHandler); - for (const dir of lockedDirs) { - try { - await releaseWatchLock(dir); - } catch { - // Best effort \u2014 a leftover lock will be detected as stale next run. - } - } + const cleanup = async () => { + if (stopped) return; + stopped = true; + if (timeoutId !== null) { + clearTimeout(timeoutId); + timeoutId = null; + } + for (const w of watchers) w.shutdown(); + if (sigintHandler) process.off('SIGINT', sigintHandler); + if (sigtermHandler) process.off('SIGTERM', sigtermHandler); + for (const dir of lockedDirs) { try { - await unregisterCurrentProcess(); + await releaseWatchLock(dir); } catch { - // Best effort \u2014 leftover entries are pruned on next read. + // Best effort - a leftover lock will be detected as stale next run. } - resolve(); - }; + } + try { + await unregisterCurrentProcess(); + } catch { + // Best effort - leftover entries are pruned on next read. + } + resolveDone(); + }; - if (options.signal) { - if (options.signal.aborted) { - void cleanup(); - return; - } + if (options.signal) { + if (options.signal.aborted) { + void cleanup(); + } else { options.signal.addEventListener('abort', () => void cleanup(), { once: true, }); - } else { - sigintHandler = () => { - if (!quiet) console.log(`\n${FG_CYAN}\u21c5 Watch stopped${RESET}`); - void cleanup(); - }; - sigtermHandler = sigintHandler; - process.on('SIGINT', sigintHandler); - process.on('SIGTERM', sigtermHandler); } - }); + } else { + sigintHandler = () => { + if (!quiet) console.log(`\n${FG_CYAN}\u21c5 Watch stopped${RESET}`); + void cleanup(); + }; + sigtermHandler = sigintHandler; + process.on('SIGINT', sigintHandler); + process.on('SIGTERM', sigtermHandler); + } + + // Skip the initial poll if a stop already arrived before we got here. + if (!stopped) { + await tickAll(); + scheduleNextTick(); + } + + await done; return { watchers }; } diff --git a/packages/boxel-cli/tests/integration/realm-watch.test.ts b/packages/boxel-cli/tests/integration/realm-watch.test.ts index d1677e7351..4329de224b 100644 --- a/packages/boxel-cli/tests/integration/realm-watch.test.ts +++ b/packages/boxel-cli/tests/integration/realm-watch.test.ts @@ -247,6 +247,28 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +/** + * Polls `predicate` until it returns true, then resolves; throws with + * `describeFailure()` once `timeoutMs` elapses. Used to wait on state that + * watchRealms populates asynchronously (the lock file, the SIGINT/SIGTERM + * handlers) instead of racing it with a fixed sleep that can lose under CI + * load. + */ +async function waitFor( + predicate: () => boolean | Promise, + describeFailure: () => string, + timeoutMs = REMOTE_VISIBILITY_TIMEOUT_MS, +): Promise { + let deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (await predicate()) return; + await sleep(25); + } + throw new Error( + `waitFor timed out after ${timeoutMs}ms: ${describeFailure()}`, + ); +} + beforeAll(async () => { // Realm starts empty; tests seed remote files via authedRealmFetch so they // produce realistic mtimes that change between writes. @@ -607,10 +629,12 @@ describe('realm watch (integration)', () => { signal: firstController.signal, }); - // Wait for the first run to acquire the lock. - await sleep(150); - let lockPath = path.join(localDir, '.boxel-watch.lock'); + // Wait for the first run to acquire the lock instead of racing a fixed delay. + await waitFor( + () => fs.existsSync(lockPath), + () => `first watch did not create a lock at ${lockPath}`, + ); expect(fs.existsSync(lockPath)).toBe(true); let secondResult = await watchRealms([{ realmUrl, localDir }], { @@ -649,7 +673,25 @@ describe('realm watch (integration)', () => { signal: controller.signal, }); - await sleep(150); + // Wait for the stale lock to be overwritten with our pid, tolerating a + // transient malformed read while acquireWatchLock rewrites the file. + await waitFor( + () => { + try { + return ( + JSON.parse(fs.readFileSync(lockPath, 'utf8')).pid === process.pid + ); + } catch { + return false; + } + }, + () => { + let raw = fs.existsSync(lockPath) + ? fs.readFileSync(lockPath, 'utf8') + : ''; + return `stale lock not overwritten with pid ${process.pid}; lock contents: ${raw}`; + }, + ); let parsed = JSON.parse(fs.readFileSync(lockPath, 'utf8')); expect(parsed.pid).toBe(process.pid); @@ -777,20 +819,27 @@ describe('realm watch (integration)', () => { quiet: true, }); - await sleep(150); - - let addedSigint = process - .listeners('SIGINT') - .filter((l) => !originalSigint.includes(l)); - let addedSigterm = process - .listeners('SIGTERM') - .filter((l) => !originalSigterm.includes(l)); - expect(addedSigint).toHaveLength(1); - expect(addedSigterm).toHaveLength(1); + // watchRealms registers its SIGINT/SIGTERM handlers asynchronously (after + // acquiring the lock and initializing the watcher). Wait for them to + // appear rather than racing a fixed delay that can lose under CI load. + let addedSigint = () => + process.listeners('SIGINT').filter((l) => !originalSigint.includes(l)); + let addedSigterm = () => + process.listeners('SIGTERM').filter((l) => !originalSigterm.includes(l)); + await waitFor( + () => addedSigint().length === 1 && addedSigterm().length === 1, + () => + `expected watchRealms to add exactly one SIGINT and one SIGTERM handler; ` + + `saw ${addedSigint().length} SIGINT / ${addedSigterm().length} SIGTERM ` + + `(total SIGINT listeners: ${process.listeners('SIGINT').length}, ` + + `SIGTERM: ${process.listeners('SIGTERM').length})`, + ); + expect(addedSigint()).toHaveLength(1); + expect(addedSigterm()).toHaveLength(1); // Invoke the registered handler directly instead of process.emit('SIGINT'), // which would also trigger any unrelated SIGINT listeners on the runner. - (addedSigint[0] as () => void)(); + (addedSigint()[0] as () => void)(); let result = await runPromise; expect(result.error).toBeUndefined(); From d76f7445f7d7aa8a7409b9d69cea657e069e0c0c Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 16 Jul 2026 12:17:46 -0400 Subject: [PATCH 2/2] fix(boxel-cli): don't block realm-watch shutdown on the initial poll Registering a signal handler suppresses Node's default Ctrl+C termination, so once the stop triggers are wired up before the initial poll, a stop that fires while that poll is still in flight would leave watchRealms parked on the poll - cleanup runs but the function can't return until the (possibly wedged) poll resolves, making the watch look hung. Race the initial poll against the stop so an interrupt returns promptly. When the stop wins, the still-pending poll is inert: the watcher is already shut down and the next-tick scheduler is guarded by the stopped flag. Add an integration test that stops the watch while the initial poll is blocked and asserts watchRealms returns and releases the lock without the poll ever completing. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/commands/realm/watch/start.ts | 17 +++++- .../tests/integration/realm-watch.test.ts | 56 +++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/boxel-cli/src/commands/realm/watch/start.ts b/packages/boxel-cli/src/commands/realm/watch/start.ts index 8643b8725b..31dddd4858 100644 --- a/packages/boxel-cli/src/commands/realm/watch/start.ts +++ b/packages/boxel-cli/src/commands/realm/watch/start.ts @@ -622,10 +622,21 @@ export async function watchRealms( process.on('SIGTERM', sigtermHandler); } - // Skip the initial poll if a stop already arrived before we got here. + // Run the initial poll, but let a stop that arrives mid-poll win the race so + // an interrupt during a slow first poll returns promptly instead of blocking + // on the poll (a registered signal handler suppresses Node's default Ctrl+C + // termination, so a wedged poll would otherwise look like a hung process). + // When the stop wins, the still-pending poll is inert: the watcher is already + // shut down and scheduleNextTick is guarded by `stopped`. A stop that already + // arrived before we got here skips the poll entirely. if (!stopped) { - await tickAll(); - scheduleNextTick(); + await Promise.race([ + (async () => { + await tickAll(); + scheduleNextTick(); + })().catch(() => {}), + done, + ]); } await done; diff --git a/packages/boxel-cli/tests/integration/realm-watch.test.ts b/packages/boxel-cli/tests/integration/realm-watch.test.ts index 4329de224b..9f07b49ace 100644 --- a/packages/boxel-cli/tests/integration/realm-watch.test.ts +++ b/packages/boxel-cli/tests/integration/realm-watch.test.ts @@ -848,6 +848,62 @@ describe('realm watch (integration)', () => { expect(fs.existsSync(path.join(localDir, '.boxel-watch.lock'))).toBe(false); }); + it('returns promptly when stopped while the initial poll is still blocked', async () => { + let localDir = makeLocalDir(); + await writeRemoteFile( + realmUrl, + watchFixture('hungpoll'), + 'export const x = 1;\n', + ); + let originalSigint = [...process.listeners('SIGINT')]; + + // Gate the initial poll (2nd _mtimes: 1 = initialize, 2 = initial tickAll + // poll) so it never resolves on its own, simulating a wedged first poll. + let mtimesCallCount = 0; + let releaseGate: () => void = () => {}; + let gate = new Promise((resolve) => { + releaseGate = resolve; + }); + let gatedAuth: RealmAuthenticator = { + authedRealmFetch: async (input, init) => { + let url = typeof input === 'string' ? input : input.toString(); + if (url.endsWith('_mtimes')) { + mtimesCallCount++; + if (mtimesCallCount === 2) await gate; + } + return profileManager.authedRealmFetch(input, init); + }, + }; + + // No `signal` → the SIGINT handler is registered before the initial poll. + let runPromise = watchRealms([{ realmUrl, localDir }], { + authenticator: gatedAuth, + intervalMs: 1000, + debounceMs: 25, + quiet: true, + }); + + await waitFor( + () => + process.listeners('SIGINT').filter((l) => !originalSigint.includes(l)) + .length === 1, + () => 'watchRealms did not register a SIGINT handler before the poll', + ); + let handler = process + .listeners('SIGINT') + .filter((l) => !originalSigint.includes(l))[0]; + + // Stop while the initial poll is still blocked. watchRealms must return + // rather than staying parked on the wedged poll (the registered handler + // suppresses Node's default Ctrl+C exit, so a park would look like a hang). + (handler as () => void)(); + let result = await runPromise; + releaseGate(); + expect(result.error).toBeUndefined(); + expect(fs.existsSync(path.join(localDir, '.boxel-watch.lock'))).toBe(false); + expect(process.listeners('SIGINT')).toEqual(originalSigint); + }); + it('downgrades a pending modify to a delete when the remote file disappears', async () => { let localDir = makeLocalDir(); await writeRemoteFile(