[miniflare] Reconcile dev registry entries instead of deleting and recreating them

.changeset/lucky-pandas-wander.md

@@ -0,0 +1,9 @@
+---
+"miniflare": patch
+---
+
+Stop deleting and recreating every dev registry entry on each config update
+
+Applying options rewrote this instance's dev registry entries by removing them and putting them straight back. Other dev sessions find Workers by watching that directory, so each update briefly looked to them like every Worker in the session had gone away — and a session that had already resolved one of those Workers could be left acting on that, up to and including tearing down a binding to a Worker that never actually stopped running.
+
+Entries are now reconciled instead: Workers that are still present are updated in place, and only the ones that have genuinely gone are removed. Switching to a different registry path still clears the entries from the directory being left behind.

packages/miniflare/src/index.ts

@@ -2684,6 +2684,9 @@ export class Miniflare {
 			this.#log.warn(
 				"Debug port not available — skipping dev registry registration"
 			);
+			// Nothing we advertised before is reachable, so withdraw it rather than
+			// leaving peers pointed at an address we can no longer serve.
+			this.#devRegistry.register({});
 			return;
 		}
 		const debugPortAddress = `127.0.0.1:${debugPort}`;

packages/miniflare/src/shared/dev-registry.ts

@@ -137,11 +137,17 @@ export class DevRegistry {
 		registryPath: string | undefined,
 		onUpdate?: (registry: WorkerRegistry) => void
 	): Promise<void> {
-		// Unregister all registered workers
-		this.unregisterWorkers();
 		this.onUpdate = onUpdate;
 
 		if (registryPath !== this.registryPath) {
+			// Our entries live in the directory we are leaving, so they have to be
+			// removed from there before we switch. When the path is unchanged we
+			// deliberately keep them: `register()` reconciles the set instead, so a
+			// config update no longer deletes and recreates every entry. Other dev
+			// sessions watch this directory, and a deletion is visible to them even
+			// if we put the file straight back.
+			this.unregisterWorkers();
+
 			// Close the existing watcher if it exists.
 			// It will watch the new path if there is any dependent services in a later step
 			await this.watcher?.close();
@@ -159,6 +165,17 @@ export class DevRegistry {
 		// Make sure the registry path exists
 		mkdirSync(this.registryPath, { recursive: true });
 
+		// Drop the entries for Workers this instance no longer has. Workers that
+		// remain are overwritten in place below instead of being deleted and
+		// recreated, so a peer never observes one of its service binding or
+		// `tail_consumers` targets disappearing during a routine config update.
+		for (const name of [...this.registeredWorkers]) {
+			if (!(name in workers)) {
+				this.unregister(name);
+				this.registeredWorkers.delete(name);
+			}
+		}
+
 		for (const [name, definition] of Object.entries(workers)) {
 			const definitionPath = path.join(this.registryPath, name);
 			const existingHeartbeat = this.heartbeats.get(name);

packages/miniflare/test/dev-registry.spec.ts

@@ -1,3 +1,5 @@
+import path from "node:path";
+import { watch } from "chokidar";
 import { getWorkerRegistry, Miniflare } from "miniflare";
 import { describe, onTestFinished, test, vi } from "vitest";
 import { useDispose, useTmp } from "./test-shared";
@@ -1788,3 +1790,108 @@ describe.sequential("DevRegistry", () => {
 		);
 	});
 });
+
+describe("registry churn across config updates", () => {
+	const script = (body: string) =>
+		`export default { async fetch() { return new Response("${body}"); } }`;
+
+	test("does not withdraw a Worker's entry during an unrelated config update", async ({
+		expect,
+	}) => {
+		const unsafeDevRegistryPath = await useTmp();
+		const mf = new Miniflare({
+			name: "stable-worker",
+			unsafeDevRegistryPath,
+			modules: true,
+			script: script("before"),
+		});
+		useDispose(mf);
+		await mf.ready;
+
+		await vi.waitFor(
+			() => {
+				expect(
+					getWorkerRegistry(unsafeDevRegistryPath)["stable-worker"]
+				).toBeDefined();
+			},
+			{ timeout: 10_000, interval: 100 }
+		);
+
+		// Other dev sessions learn about us by watching this directory, so watch it
+		// the same way and record what a peer would actually see.
+		const events: string[] = [];
+		const watcher = watch(unsafeDevRegistryPath, {
+			ignoreInitial: true,
+		});
+		onTestFinished(() => watcher.close());
+		await new Promise((resolve) => watcher.once("ready", resolve));
+		watcher.on("unlink", (file) =>
+			events.push(`unlink:${path.basename(file)}`)
+		);
+		watcher.on("add", (file) => events.push(`add:${path.basename(file)}`));
+		watcher.on("change", (file) =>
+			events.push(`change:${path.basename(file)}`)
+		);
+
+		await mf.setOptions({
+			name: "stable-worker",
+			unsafeDevRegistryPath,
+			modules: true,
+			script: script("after"),
+		});
+
+		// Wait until the update has definitely reached the directory, so that an
+		// empty event list can't be mistaken for a passing assertion.
+		await vi.waitFor(
+			() => {
+				expect(events.some((event) => event.endsWith(":stable-worker"))).toBe(
+					true
+				);
+			},
+			{ timeout: 10_000, interval: 100 }
+		);
+
+		expect(events).not.toContain("unlink:stable-worker");
+		expect(
+			getWorkerRegistry(unsafeDevRegistryPath)["stable-worker"]
+		).toBeDefined();
+	});
+
+	test("withdraws the entry for a Worker removed from the config", async ({
+		expect,
+	}) => {
+		const unsafeDevRegistryPath = await useTmp();
+		const mf = new Miniflare({
+			unsafeDevRegistryPath,
+			workers: [
+				{ name: "kept-worker", modules: true, script: script("kept") },
+				{ name: "dropped-worker", modules: true, script: script("dropped") },
+			],
+		});
+		useDispose(mf);
+		await mf.ready;
+
+		await vi.waitFor(
+			() => {
+				const registry = getWorkerRegistry(unsafeDevRegistryPath);
+				expect(registry["kept-worker"]).toBeDefined();
+				expect(registry["dropped-worker"]).toBeDefined();
+			},
+			{ timeout: 10_000, interval: 100 }
+		);
+
+		await mf.setOptions({
+			unsafeDevRegistryPath,
+			workers: [{ name: "kept-worker", modules: true, script: script("kept") }],
+		});
+
+		await vi.waitFor(
+			() => {
+				const registry = getWorkerRegistry(unsafeDevRegistryPath);
+				expect(registry["kept-worker"]).toBeDefined();
+				expect(registry["dropped-worker"]).toBeUndefined();
+			},
+			{ timeout: 10_000, interval: 100 }
+		);
+	});
+});