test: fix three load-only test failures at the cause

AGENTS.md

@@ -16,6 +16,7 @@ This file is split into nested `AGENTS.md` files based on the language/situation
 - Dig into the root cause and fix it at the source. Never work around a fixable bug with a retry, sleep, or timeout.
 - Fail loud and early. Error on unsupported or malformed input rather than warn and continue: supported or refused.
 - Reproduce bugs before fixing them. Land each fix with a regression test that fails without it, when one is easy.
+- Unit tests mock time instead of depending on wall-clock timing or sleeps.
 - Keep the PR focused. No unrelated refactors, formatting churn, or drive-by changes; split when in doubt.
 - Refactor aggressively for long-term maintainability, but re-evaluate the direction as you learn.
 - Propose a course change, even suggest abandoning a PR, rather than finish a half-solution.

js/common/declarations.test.ts

@@ -0,0 +1,50 @@
+import { expect, test } from "bun:test";
+import { problems } from "./declarations";
+
+const root = "/dist";
+
+test("an import of a stripped export is reported", () => {
+	const files = new Map([
+		["/dist/reload.d.ts", "export declare class Reload {}\n"],
+		["/dist/index.d.ts", 'import { ReloadDelay } from "./reload.js";\nexport declare const delay: ReloadDelay;\n'],
+	]);
+	expect(problems(root, files)).toEqual([
+		"index.d.ts: imports ReloadDelay from ./reload.js, which does not export it",
+	]);
+});
+
+test("an import of a file with no declarations is reported", () => {
+	const files = new Map([["/dist/index.d.ts", 'import type { Mock } from "./mock.ts";\n']]);
+	expect(problems(root, files)).toEqual(["index.d.ts: imports ./mock.ts, which emitted no declarations"]);
+});
+
+test("names re-exported through a star, an alias, a default, or a directory index resolve", () => {
+	const files = new Map([
+		["/dist/inner.d.ts", "export interface Delay {}\ndeclare const status = 1;\nexport { status as Status };\n"],
+		["/dist/outer/index.d.ts", 'export * from "../inner.js";\n'],
+		["/dist/element.d.ts", "export default class Element {}\n"],
+		["/dist/page.d.ts", 'import { Delay } from "./outer";\nimport { Status } from ".";\n'],
+		[
+			"/dist/index.d.ts",
+			'export * from "./outer";\nexport type { default as Element } from "./element.tsx";\nimport { type Delay, Status as S } from "./outer/index.ts";\nimport { Other } from "@moq/other";\nimport icon from "./icon.svg?raw";\n',
+		],
+	]);
+	expect(problems(root, files)).toEqual([]);
+});
+
+test("a default import of a module without a default export is reported", () => {
+	const files = new Map([
+		["/dist/element.d.ts", "export declare class Element {}\n"],
+		["/dist/index.d.ts", 'import type Element from "./element.js";\nexport { Element };\n'],
+	]);
+	expect(problems(root, files)).toEqual(["index.d.ts: imports default from ./element.js, which does not export it"]);
+});
+
+test("type-only star re-exports, const enums, and let declarations resolve", () => {
+	const files = new Map([
+		["/dist/types.d.ts", "export declare const enum State { Open }\nexport declare let value: number;\n"],
+		["/dist/outer.d.ts", 'export type * from "./types.js";\n'],
+		["/dist/index.d.ts", 'import type { State, value } from "./outer.js";\n'],
+	]);
+	expect(problems(root, files)).toEqual([]);
+});

js/common/declarations.ts

@@ -0,0 +1,128 @@
+// Checks a package's emitted `.d.ts` files for imports the target file no longer exports.
+//
+// `stripInternal` drops an `@internal` export from its own `.d.ts` but leaves the import in any
+// file that names the type, so a published consumer sees a module that does not export it. Only
+// declaration emit shows this, which is why it runs over `dist/` after the build rather than as a
+// test that would have to run the compiler again.
+
+import { dirname, extname, join, relative, resolve } from "node:path";
+import { parse } from "@babel/parser";
+
+type Statement = ReturnType<typeof parse>["program"]["body"][number];
+type Declaration = Extract<Statement, { type: "ExportNamedDeclaration" }>["declaration"];
+
+type FileExports = {
+	names: Set<string>;
+	stars: string[];
+	imports: Array<{ specifier: string; names: string[] }>;
+};
+
+/** Every import in `files` (path to `.d.ts` source) that its relative target does not export. */
+export function problems(root: string, files: Map<string, string>): string[] {
+	const parsed = new Map<string, FileExports>();
+	for (const [file, source] of files) {
+		parsed.set(file, fileExports(source));
+	}
+
+	const found: string[] = [];
+	for (const [file, info] of parsed) {
+		for (const { specifier, names } of info.imports) {
+			const target = dtsPath(parsed, file, specifier);
+			if (!target) continue;
+
+			if (!parsed.has(target)) {
+				found.push(`${relative(root, file)}: imports ${specifier}, which emitted no declarations`);
+				continue;
+			}
+
+			for (const name of names) {
+				if (!exported(parsed, target, name)) {
+					found.push(`${relative(root, file)}: imports ${name} from ${specifier}, which does not export it`);
+				}
+			}
+		}
+	}
+	return found;
+}
+
+// The declaration file a relative specifier resolves to, as a file or a directory index. A package
+// specifier resolves outside the package, and an asset (`./icon.svg?raw`) is typed by the bundler,
+// so neither is checked.
+function dtsPath(files: Map<string, unknown>, from: string, specifier: string): string | undefined {
+	if (!specifier.startsWith(".")) return undefined;
+	const ext = extname(specifier);
+	if (ext && !/^\.(js|jsx|ts|tsx)$/.test(ext)) return undefined;
+	const base = resolve(dirname(from), specifier).replace(/\.(js|jsx|ts|tsx)$/, "");
+	const index = join(base, "index.d.ts");
+	return files.has(index) ? index : `${base}.d.ts`;
+}
+
+function exported(files: Map<string, FileExports>, file: string, name: string, seen = new Set<string>()): boolean {
+	if (seen.has(file)) return false;
+	seen.add(file);
+
+	const info = files.get(file);
+	if (!info) return false;
+	if (info.names.has(name)) return true;
+
+	for (const specifier of info.stars) {
+		const target = dtsPath(files, file, specifier);
+		if (target && exported(files, target, name, seen)) return true;
+	}
+	return false;
+}
+
+function fileExports(source: string): FileExports {
+	const names = new Set<string>();
+	const stars: string[] = [];
+	const imports: Array<{ specifier: string; names: string[] }> = [];
+
+	const ast = parse(source, { sourceType: "module", plugins: [["typescript", { dts: true }]] });
+	for (const statement of ast.program.body) {
+		switch (statement.type) {
+			case "ImportDeclaration":
+				if (statement.specifiers.length === 0) break;
+				imports.push({
+					specifier: statement.source.value,
+					names: statement.specifiers.flatMap((s) => {
+						if (s.type === "ImportDefaultSpecifier") return ["default"];
+						if (s.type === "ImportSpecifier") return [moduleName(s.imported)];
+						return [];
+					}),
+				});
+				break;
+			case "ExportNamedDeclaration": {
+				const imported: string[] = [];
+				for (const s of statement.specifiers) {
+					names.add(moduleName(s.exported));
+					if (s.type === "ExportSpecifier") imported.push(moduleName(s.local));
+				}
+				if (statement.source) imports.push({ specifier: statement.source.value, names: imported });
+				for (const name of declared(statement.declaration)) names.add(name);
+				break;
+			}
+			case "ExportAllDeclaration":
+				stars.push(statement.source.value);
+				break;
+			case "ExportDefaultDeclaration":
+				names.add("default");
+				break;
+		}
+	}
+
+	return { names, stars, imports };
+}
+
+function moduleName(node: { type: "Identifier"; name: string } | { type: "StringLiteral"; value: string }): string {
+	return node.type === "Identifier" ? node.name : node.value;
+}
+
+// The names an `export declare ...` statement introduces.
+function declared(declaration: Declaration | null | undefined): string[] {
+	if (!declaration) return [];
+	if (declaration.type === "VariableDeclaration") {
+		return declaration.declarations.flatMap((d) => (d.id.type === "Identifier" ? [d.id.name] : []));
+	}
+	if (!("id" in declaration) || !declaration.id) return [];
+	return declaration.id.type === "Identifier" ? [declaration.id.name] : [];
+}

js/common/package.ts

@@ -6,6 +6,7 @@ import { copyFileSync, existsSync, readFileSync, writeFileSync } from "node:fs";
 import { basename, join, resolve } from "node:path";
 import { publint } from "publint";
 import { formatMessage } from "publint/utils";
+import { problems } from "./declarations.ts";
 
 console.log("✍️  Rewriting package.json...");
 const pkg = JSON.parse(readFileSync("package.json", "utf8"));
@@ -120,6 +121,18 @@ if (messages.length > 0) {
 	process.exit(1);
 }
 
+console.log("🔍 Checking declaration imports...");
+const declarations = new Map<string, string>();
+for (const rel of new Bun.Glob("**/*.d.ts").scanSync("dist")) {
+	const file = resolve("dist", rel);
+	declarations.set(file, readFileSync(file, "utf8"));
+}
+const unresolved = problems(resolve("dist"), declarations);
+if (unresolved.length > 0) {
+	for (const problem of unresolved) console.error(problem);
+	process.exit(1);
+}
+
 console.log("📦 Package built successfully in dist/");
 
 // Optionally emit a jsr.json so the package can also publish to JSR (jsr.io).

js/json/src/snapshot/encoder.ts

@@ -42,6 +42,14 @@ export interface Config<T> {
 	// `"none"`/unset (the default) writes plaintext JSON frames. A {@link Decoder} reading them
 	// must set the same {@link compression}.
 	compression?: Compression;
+
+	/**
+	 * Bytes a group may hold before it rolls, defaulting to moq-net's per-group cache limit. Lets a
+	 * test reach the limit without megabytes of JSON.
+	 *
+	 * @internal
+	 */
+	maxGroupBytes?: number;
 }
 
 /** One encoded frame, and the group boundary it implies. */
@@ -106,6 +114,7 @@ export interface Pending extends Encoded {
 export class Encoder<T> {
 	#config: Config<T>;
 	#compress: boolean;
+	#maxGroupBytes: number;
 
 	// The last encoded value, normalized through JSON so it matches what landed on the wire. The
 	// baseline every delta is diffed against, and `undefined` until the first snapshot.
@@ -140,6 +149,7 @@ export class Encoder<T> {
 	constructor(config: Config<T> = {}) {
 		this.#config = config;
 		this.#compress = isDeflate(config.compression);
+		this.#maxGroupBytes = config.maxGroupBytes ?? Group.MAX_GROUP_CACHE_BYTES;
 	}
 
 	/**
@@ -218,7 +228,7 @@ export class Encoder<T> {
 			// can come out slightly larger than its input, so the plaintext is not an upper bound.
 			// Compressing first advances the window, but `#snapshot` opens a fresh one, so an
 			// over-budget delta costs only the wasted compression.
-			if (this.#snapshotLen + this.#deltaBytes + payload.length <= Group.MAX_GROUP_CACHE_BYTES) {
+			if (this.#snapshotLen + this.#deltaBytes + payload.length <= this.#maxGroupBytes) {
 				this.#last = json;
 				this.#deltaBytes += payload.length;
 				this.#groupFrames += 1;

js/json/src/snapshot/snapshot.test.ts

@@ -1,6 +1,7 @@
 import { expect, test } from "bun:test";
 import { Group, Error as NetError, StreamCode, Time, Track } from "@moq/net";
 import { Consumer } from "./consumer.ts";
+import { Encoder } from "./encoder.ts";
 import { Producer } from "./producer.ts";
 
 type Value = Record<string, unknown>;
@@ -358,38 +359,38 @@ test("a delta that would overflow the snapshot rolls a new one instead", async (
 
 test("a compressed delta is gated on its encoded size, not its plaintext", async () => {
 	// A sync-flushed DEFLATE frame can come out larger than its input, so the plaintext is not an
-	// upper bound on what lands in the group. A snapshot that fills the cache to within a few bytes
-	// plus a tiny patch that compresses to more than it measures would otherwise slip through the
-	// gate and evict frame 0.
+	// upper bound on what lands in the group. A patch that fits the budget by its plaintext but not
+	// by its encoded size would otherwise slip through the gate and overflow the group.
+	const value = { v: "x".repeat(1000) };
+	const patched = { ...value, q: "a" };
+	const plaintext = JSON.stringify({ q: "a" }).length;
+
+	// Measure the frames with the default budget, which admits the delta.
+	const probe = new Encoder<Value>({ compression: "deflate" });
+	const snapshot = probe.update(value);
+	snapshot?.commit();
+	const delta = probe.update(patched);
+	expect(delta?.keyframe).toBe(false);
+	expect(delta?.payload.length).toBeGreaterThan(plaintext);
+
+	// A budget with room for the plaintext patch but not the encoded one.
+	const maxGroupBytes = (snapshot?.payload.length ?? 0) + plaintext;
 	const track = new Track.Producer("test");
-	const producer = new Producer<Value>({ track, compression: "deflate" });
-
-	// Highly repetitive, so the compressed snapshot lands just under the cap.
-	producer.update({ v: "x".repeat(Group.MAX_GROUP_CACHE_BYTES) });
-	producer.update({ v: "x".repeat(Group.MAX_GROUP_CACHE_BYTES), q: "a" });
+	const producer = new Producer<Value>({ track, compression: "deflate", maxGroupBytes });
+	producer.update(value);
+	producer.update(patched);
 	producer.finish();
 
-	// Whatever the split, no group may exceed the cache, and the newest value must be readable.
-	const subscriber = track.subscribe({ maxAge: REPLAY_LATENCY }).ordered();
-	for (;;) {
-		const group = await subscriber.nextGroup();
-		if (!group) break;
-		let bytes = 0;
-		for (;;) {
-			const frame = await group.readFrame();
-			if (!frame) break;
-			bytes += frame.payload.byteLength;
-		}
-		expect(bytes).toBeLessThanOrEqual(Group.MAX_GROUP_CACHE_BYTES);
-	}
+	// The patch rolled into a fresh snapshot rather than joining the first group.
+	expect(await structure(track.subscribe({ maxAge: REPLAY_LATENCY }).ordered())).toEqual([1, 1]);
 
 	const consumer = new Consumer<Value>({
 		track: track.subscribe({ maxAge: REPLAY_LATENCY }),
 		compression: "deflate",
 	});
 	const values: Value[] = [];
-	for await (const value of consumer) values.push(value);
-	expect(values[values.length - 1]).toEqual({ v: "x".repeat(Group.MAX_GROUP_CACHE_BYTES), q: "a" });
+	for await (const out of consumer) values.push(out);
+	expect(values[values.length - 1]).toEqual(patched);
 });
 
 // A malformed or failed group must reach the caller; only an explicit retention gap is resumable.

js/justfile

@@ -84,7 +84,7 @@ test $FILES="":
     	exit 0
     fi
     bun install --frozen-lockfile
-    bun test common/deps.test.ts common/workers.test.ts
+    bun test common/declarations.test.ts common/deps.test.ts common/workers.test.ts
     if tty -s; then
         bun run --filter='*' --elide-lines=0 test
     else