fix(runtime): wait on close for widget dev work already started, with a bound

apps/runtime/src/application/widget-dev-sessions.ts

@@ -128,12 +128,29 @@ export interface WidgetDevSessions {
   forget(root: string): WidgetDevResult<{ root: string; forgotten: boolean; stillCoveredBy?: string }>;
   /** Start watching again every session that was live when the node stopped. */
   resume(): Promise<void>;
-  close(): void;
+  /**
+   * Stop watching every folder, then wait for the work each session already started (a build being followed, a
+   * superseded snapshot being removed) to finish, for at most `closeWaitMs`, so the caller can let go of the database and
+   * the data folder without pulling them from under a removal. Past the bound it resolves anyway: a shutdown never hangs.
+   */
+  close(): Promise<void>;
 }
 
 /** How often a session waiting on the person's answer looks for it, so an approval in the inbox is followed promptly. */
 const ANSWER_POLL_MS = 2_000;
 
+/**
+ * How a superseded snapshot is removed: the promise form of `rm`, retrying a file still held open (on Windows) after 100,
+ * 200, 300, 400 and 500 ms, about 1.5 s in all.
+ */
+export const SNAPSHOT_REMOVAL = { recursive: true, force: true, maxRetries: 5, retryDelay: 100 } as const;
+
+/**
+ * The longest `close` waits for work already started: one snapshot's removal with all its retries, with room to spare,
+ * and well inside the node's shutdown grace.
+ */
+export const WIDGET_DEV_CLOSE_WAIT_MS = 2_000;
+
 interface LiveSession {
   sessionId: string;
   engine: DevEngine;
@@ -202,6 +219,8 @@ export function createWidgetDevSessions(
     answerPollMs?: number;
     /** How long a folder may keep failing to be looked at before its session stops (`DEV_ENGINE_ROOT_UNREADABLE_MS`). */
     rootUnreadableMs?: number;
+    /** The longest `close` waits for work already started (`WIDGET_DEV_CLOSE_WAIT_MS`). */
+    closeWaitMs?: number;
   } = {},
 ): WidgetDevSessions {
   const live = new Map<string, LiveSession>();
@@ -774,7 +793,7 @@ export function createWidgetDevSessions(
         try {
           // The promise form on purpose: on Windows `rmSync` reports a held file as `EBUSY` or `EPERM` at once and never
           // runs its retries. These wait up to about 1.5 s on this session's chain, never on the event loop.
-          await rm(path, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+          await rm(path, SNAPSHOT_REMOVAL);
           removed.push(digest);
         } catch (cause) {
           process.stderr.write(`widget dev: could not remove the superseded snapshot ${digest} yet: ${messageOf(cause)}\n`);
@@ -1111,9 +1130,22 @@ export function createWidgetDevSessions(
       }
     },
 
-    close() {
+    async close() {
       for (const session of live.values()) end(session);
       live.clear();
+      // Each chain already settles rather than rejects; work queued behind a closed session finds it gone and returns.
+      const started = Promise.all([...chains.values()]);
+      let bound: ReturnType<typeof setTimeout> | undefined;
+      const timedOut = new Promise<boolean>((done) => {
+        bound = setTimeout(() => done(true), options.closeWaitMs ?? WIDGET_DEV_CLOSE_WAIT_MS);
+      });
+      try {
+        if (await Promise.race([started.then(() => false), timedOut])) {
+          process.stderr.write("widget dev: work a session started was still running at close; closing anyway\n");
+        }
+      } finally {
+        clearTimeout(bound);
+      }
     },
   };
 }

apps/runtime/src/bootstrap/runtime-bootstrap.ts

@@ -116,6 +116,11 @@ export interface RuntimeBootstrapDeps {
 export interface RuntimeHandles {
   /** Stops the periodic update-check timer (§ update-checks.ts). A no-op on a fixture node, which never starts one. */
   stopUpdateChecks: () => void;
+  /**
+   * Stops watching widget dev folders and waits, bounded, for work their sessions already started, such as removing a
+   * superseded snapshot (`WidgetDevSessions.close`). Awaited before the database closes.
+   */
+  closeWidgetDev: () => Promise<void>;
 }
 
 /**
@@ -632,9 +637,8 @@ export function wireRuntime(deps: RuntimeBootstrapDeps): RuntimeHandles {
   return {
     stopUpdateChecks: () => {
       updateChecks?.stop();
-      // Beside the update check: both are timers and watchers this function started, ended when the node closes.
-      widgetDev.close();
     },
+    closeWidgetDev: () => widgetDev.close(),
   };
 }
 

apps/runtime/src/main.ts

@@ -550,6 +550,8 @@ async function main(): Promise<void> {
     hardStop.unref();
     leaseSweeper.stop();
     runtimeHandles.stopUpdateChecks();
+    // Started now and awaited before the database closes: a snapshot a session is removing finishes, within its bound.
+    const widgetDevClosed = runtimeHandles.closeWidgetDev();
     services.expirySweep?.stop();
     services.artifactSweep?.stop();
     effectNotices.stop();
@@ -591,6 +593,7 @@ async function main(): Promise<void> {
         if (channelsStopped !== undefined && !(await channelsStopped)) {
           process.stderr.write("channel turns were still running at shutdown; closing anyway\n");
         }
+        await widgetDevClosed;
         await modelTurn?.dispose();
         // Every browser token still held is withdrawn where its provider allows, rather than left to lapse.
         await services.browserTokens?.close();

apps/runtime/test/slash-commands.spec.ts

@@ -195,7 +195,7 @@ describe("slash commands in a conversation", () => {
       // The card only offers: nothing starts until the person presses it.
       expect(services.widgetDev.list()).toEqual([]);
     } finally {
-      services.widgetDev.close();
+      await services.widgetDev.close();
     }
   });
 
@@ -210,7 +210,7 @@ describe("slash commands in a conversation", () => {
       // The word is the command's, not a folder: nothing is offered to develop.
       expect(card?.rows.some((row) => row.rowId === "proposed")).toBe(false);
     } finally {
-      services.widgetDev.close();
+      await services.widgetDev.close();
     }
   });
 });

apps/runtime/test/widget-dev-sessions.spec.ts

@@ -31,7 +31,7 @@ import {
   writeRegisteredPreference,
 } from "@clarkcant/core";
 
-import { createWidgetDevSessions } from "../src/application/widget-dev-sessions.ts";
+import { SNAPSHOT_REMOVAL, createWidgetDevSessions } from "../src/application/widget-dev-sessions.ts";
 import { WIDGET_DEV_STORE_MAX, readDevSessions, writeDevSessions } from "../src/application/widget-dev-store.ts";
 import { createDevelopWidgetTool } from "../src/develop-widget-tool.ts";
 import { hostText } from "../src/host-text.ts";
@@ -43,8 +43,11 @@ import { holdDirectory } from "./hold-directory.ts";
 /** A folder whose `stat` fails with `EPERM`, as an antivirus or indexer holding it on Windows makes it fail. */
 const statFailure = vi.hoisted(() => ({ path: undefined as string | undefined }));
 
-/** Seen as the runtime starts to remove a path, in either form, before the removal itself runs. */
-const removal = vi.hoisted(() => ({ starting: undefined as ((path: string) => void) | undefined }));
+/**
+ * Seen as the runtime starts to remove a path, in either form, before the removal itself runs. The promise form waits
+ * for what it returns, so a test can hold a removal (`options` are the ones the runtime asked for).
+ */
+const removal = vi.hoisted(() => ({ starting: undefined as ((path: string, options?: fs.RmOptions) => void | Promise<void>) | undefined }));
 
 vi.mock("node:fs", async (importOriginal) => {
   const actual = await importOriginal<typeof fs>();
@@ -55,7 +58,7 @@ vi.mock("node:fs", async (importOriginal) => {
     return actual.statSync(path, options);
   }) as typeof actual.statSync;
   const rmSync = ((path: fs.PathLike, options?: fs.RmOptions) => {
-    removal.starting?.(resolve(String(path)));
+    void removal.starting?.(resolve(String(path)), options);
     actual.rmSync(path, options);
   }) as typeof actual.rmSync;
   return { ...actual, statSync, rmSync, default: { ...actual, statSync, rmSync } };
@@ -64,12 +67,15 @@ vi.mock("node:fs", async (importOriginal) => {
 vi.mock("node:fs/promises", async (importOriginal) => {
   const actual = await importOriginal<typeof fsPromises>();
   const rm = (async (path: fs.PathLike, options?: fs.RmOptions) => {
-    removal.starting?.(resolve(String(path)));
+    await removal.starting?.(resolve(String(path)), options);
     await actual.rm(path, options);
   }) as typeof actual.rm;
   return { ...actual, rm, default: { ...actual, rm } };
 });
 
+/** The removal itself, past the mock, for a test that tries a path once on its own. */
+const { rm: actualRm } = await vi.importActual<typeof fsPromises>("node:fs/promises");
+
 /** The home folder the node sees, when a test needs it to be one of the test's own folders. */
 const homeOverride = vi.hoisted(() => ({ path: undefined as string | undefined }));
 
@@ -235,12 +241,14 @@ beforeEach(() => {
   deps = { services, now: () => new Date().toISOString() as never };
 });
 
-afterEach(() => {
+afterEach(async () => {
   statFailure.path = undefined;
   homeOverride.path = undefined;
-  services.widgetDev?.close();
+  removal.starting = undefined;
+  // A snapshot a session is still removing is finished (or given up on) before the database and the folder go.
+  await services.widgetDev?.close();
   services.runtime.close();
-  rmSync(dir, { recursive: true, force: true });
+  await removeTestDirectory(dir);
 });
 
 describe("a widget dev session", () => {
@@ -968,13 +976,22 @@ describe("a widget dev session", () => {
     if (first === undefined) throw new Error("the first build left no snapshot");
     const firstPath = resolve(local, first);
     // On Windows a directory that is someone's working directory cannot be removed until they let go; elsewhere the
-    // hold changes nothing. It ends 300 ms after the removal starts, so only a removal that really retries finds it free.
+    // hold changes nothing. Not timed against the retries: one attempt without them meets the hold, the holder is let go
+    // and has exited, and only then does the removal the runtime asked for run, so its outcome never depends on load.
     const hold = holdDirectory(firstPath);
     await hold.ready;
     let released: Promise<void> | undefined;
-    removal.starting = (path) => {
+    let asked: fs.RmOptions | undefined;
+    let held: string | undefined;
+    removal.starting = async (path, options) => {
       if (path !== firstPath || released !== undefined) return;
-      released = new Promise<void>((done) => setTimeout(done, 300)).then(() => hold.release());
+      asked = options;
+      held = await actualRm(path, { ...options, maxRetries: 0 }).then(
+        () => undefined,
+        (cause: unknown) => (cause as NodeJS.ErrnoException).code,
+      );
+      released = hold.release();
+      await released;
     };
     try {
       // The second build supersedes the first, which stays as the generation a rollback returns to; the third prunes it.
@@ -983,6 +1000,10 @@ describe("a widget dev session", () => {
         expect(session(await call("POST", `/widget-dev/sessions/${started.sessionId}/rebuild`)).running?.generation).toBeGreaterThan(1);
       }
       expect(released).toBeDefined();
+      // The removal retries a held file (the promise form; `rmSync` would not), for the budget the runtime names.
+      expect(asked).toMatchObject(SNAPSHOT_REMOVAL);
+      expect(SNAPSHOT_REMOVAL.maxRetries).toBeGreaterThan(0);
+      if (process.platform === "win32") expect(held).toMatch(/^(EBUSY|EPERM)$/);
       expect(existsSync(firstPath)).toBe(false);
       // Removed, so no longer on the session's list of snapshots to try again.
       const listed = readDevSessions(join(dir, "node"))[0]?.snapshots ?? [];
@@ -994,6 +1015,67 @@ describe("a widget dev session", () => {
     }
   });
 
+  /** Start pruning the first build's snapshot and hold its removal until `finish` is called. */
+  async function holdPrune(): Promise<{ firstPath: string; rebuilding: Promise<GatewayResponse>; finish: () => void; removed: () => boolean }> {
+    const started = session(await call("POST", "/widget-dev/sessions", { root }));
+    const local = join(dir, "node", "package-cache", "local");
+    const [first] = readdirSync(local);
+    if (first === undefined) throw new Error("the first build left no snapshot");
+    const firstPath = resolve(local, first);
+    let removing = (): void => undefined;
+    const removingStarted = new Promise<void>((done) => {
+      removing = done;
+    });
+    let finish = (): void => undefined;
+    const gate = new Promise<void>((done) => {
+      finish = done;
+    });
+    let removed = false;
+    removal.starting = async (path) => {
+      if (path !== firstPath) return;
+      removing();
+      await gate;
+      // Marked once the removal itself may run; `existsSync` says when it has.
+      removed = true;
+    };
+    writePackage("<!doctype html><p>second</p>\n");
+    await call("POST", `/widget-dev/sessions/${started.sessionId}/rebuild`);
+    // The third build supersedes the second, so the first is pruned, and its removal waits on the gate.
+    writePackage("<!doctype html><p>third</p>\n");
+    const rebuilding = call("POST", `/widget-dev/sessions/${started.sessionId}/rebuild`);
+    await removingStarted;
+    return { firstPath, rebuilding, finish, removed: () => removed };
+  }
+
+  it("waits on close for a superseded snapshot still being removed", async () => {
+    const { firstPath, rebuilding, finish } = await holdPrune();
+    let closed = false;
+    const closing = Promise.resolve(services.widgetDev?.close()).then(() => {
+      closed = true;
+    });
+    await new Promise((done) => setTimeout(done, 100));
+    expect(closed).toBe(false);
+    finish();
+    await closing;
+    expect(existsSync(firstPath)).toBe(false);
+    expect((await rebuilding).status).toBe(200);
+  });
+
+  it("stops waiting on close after its bound, so a removal that does not end cannot hold a shutdown", async () => {
+    await services.widgetDev?.close();
+    services.widgetDev = createWidgetDevSessions(() => services, { watch: false, closeWaitMs: 200 });
+    const { firstPath, rebuilding, finish, removed } = await holdPrune();
+    const before = Date.now();
+    await services.widgetDev.close();
+    expect(Date.now() - before).toBeGreaterThanOrEqual(150);
+    expect(removed()).toBe(false);
+    expect(existsSync(firstPath)).toBe(true);
+    // Let it end, so the folder is free before the test's own cleanup.
+    finish();
+    expect((await rebuilding).status).toBe(200);
+    expect(existsSync(firstPath)).toBe(false);
+  });
+
   it("forgets the oldest stopped sessions rather than failing when the store is full", async () => {
     const at = new Date(Date.UTC(2026, 0, 1)).toISOString();
     writeDevSessions(