bundle-source maintenance

[turadg]

incidental

Description

There was some confusion recently about the API of BundleCache. This adds docs for it.

It also adds tests coverage, fixes up types and some runtime cleanup.

The new tests fail under Node 18 because it defaults to CJS for .js files. Since that version is past EOL this also removes it from CI matrix. And while searching it unpins node-version from jobs which aren't meant to test release compatibility (and should always work with latest default Node version). This provides a package.json and defers CI changes to #3084

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

This should go out in a release so it needs a changeset. Patch release I suppose.

Testing Considerations

CI

Compatibility Considerations

no, just docs and types and some small bug fixes

Upgrade Considerations

will address in a changeset file

[changeset-bot]

🦋 Changeset detected

Latest commit: 644ab15

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@endo/bundle-source	Patch
@endo/cli	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Copilot]

Pull request overview

This PR improves @endo/bundle-source maintainability and correctness by tightening types/docs around BundleCache, adding targeted regression tests, and doing some runtime cleanup and CLI doc tweaks (notably mentioning the endoScript format).

Changes:

• Add new tests for BundleCache behavior (immutability, malformed metadata errors, recovery after failed loads) and CLI usage output.
• Strengthen/clarify runtime behavior in bundling/cache code (type cleanup, root validation tightening, memoization cleanup).
• Expand/centralize TypeScript type definitions and enable // @ts-check in more JS modules.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/bundle-source/test/tool-command.test.js	Adds a CLI usage assertion that endoScript appears in --format help output.
packages/bundle-source/test/cache.test.js	Adds regression tests for cache immutability, malformed metadata handling, and load recovery behavior.
packages/bundle-source/src/zip-base64.js	Converts to // @ts-check and refactors bundling inputs/types (including conditions handling).
packages/bundle-source/src/types.ts	Introduces richer exported types for cache/bundling, including shared “powers” and cache interfaces.
packages/bundle-source/src/tool.js	Replaces a TS suppression with an explicit argv type assertion before calling main().
packages/bundle-source/src/script.js	Converts to // @ts-check and refactors bundling inputs/types (including conditions handling).
packages/bundle-source/src/main.js	Updates CLI usage text to include endoScript in the supported format list.
packages/bundle-source/src/fs.js	Adds/clarifies JSDoc return types and aligns neighbor() typing for atomic writers.
packages/bundle-source/src/endo.js	Converts to // @ts-check, strengthens source-map caching requirements, and refactors transform plumbing.
packages/bundle-source/src/bundle-source.js	Refactors option handling to a typed implementation wrapper and removes previous casting suppressions.
packages/bundle-source/cache.js	Adds docstrings, improves option/conditions handling, improves root validation, and hardens memoized-load failure behavior.
.changeset/olive-socks-join.md	Adds a patch changeset describing cache fixes and CLI doc clarification.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This JSDoc assertion types argv as a tuple with at least two positional args ([to, dest, ...rest]), but the CLI accepts flags-only invocations and main() handles arbitrary-length argv. Using an incorrect tuple type can hide real type issues. Consider typing this as string[] (and updating main's JSDoc accordingly) instead of asserting a tuple shape that isn't guaranteed.

Suggested change

	const args = /** @type {[to: string, dest: string, ...rest: string[]]} */ (
	process.argv.slice(2)
	);
	const args = /** @type {string[]} */ (process.argv.slice(2));

[Copilot]

BundleZipBase64Options.importHook references import('@endo/compartment-mapper/node-powers.js').ExitModuleImportHook, but ExitModuleImportHook is defined/exported from the compartment-mapper types module (e.g. @endo/compartment-mapper/src/types.js), not from node-powers.js. As written, this is likely to fail TypeScript type resolution for consumers. Please update the type reference to import ExitModuleImportHook from the module that actually exports it (matching how other compartment-mapper types are imported elsewhere in the repo).

Suggested change

	| import('@endo/compartment-mapper/node-powers.js').ExitModuleImportHook
	| import('@endo/compartment-mapper/src/types.js').ExitModuleImportHook

[Copilot]

evadeCensor() returns a source map object when sourceUrl is provided (and undefined otherwise). This code never passes sourceUrl, so map will always be undefined and transforms will drop source maps. Additionally, even if sourceUrl were provided, the typeof map === 'string' check will discard the returned map object. Consider passing sourceUrl: new URL(specifier, location).href and serializing the returned map (e.g., JSON.stringify(map)) so sourceMap is a string as expected by compartment-mapper.

[Copilot]

The tests create a new temp cache directory via fs.mkdtemp(...) but never remove it. Over many runs (or in watch mode) this can leak files in the OS temp directory. Consider registering a cleanup with t.teardown(() => fs.rm(dest, { recursive: true, force: true })) (or equivalent) in each test or inside makeTempDest by passing t in.

[michaelfig]

Looks really good! I just noticed a few things that could be clarified.

[michaelfig]

There's a more succinct way:

Suggested change

	const error = await t.throwsAsync(() => shellOutInvalidFormat());
	t.regex(error.message, /-f,--format .*endoScript/);
	await t.throwsAsync(() => shellOutInvalidFormat(), { message: /-f,--format .*endoScript/ });

[michaelfig]

More idiomatic:

Suggested change

	});
	await t.throwsAsync(() =>
	cache.validateOrAdd(entry, targetName, t.log),
	{
	instanceOf: SyntaxError,
	message: /Cannot parse JSON from cache-test-malformed-meta/,
	},
	);

[michaelfig]

I'm curious about the rationale, but this at least should have a comment:

Suggested change

	// This promise may be rejected before any concurrent caller awaits it.
	todo.promise.catch(() => {});
	// This promise may be rejected before any concurrent caller awaits it.
	// HardenedJS only logs a warning if the promise is never awaited.
	// We silence the promise completely because we want to ensure the
	// rejection is never logged, with or without HardenedJS.
	todo.promise.catch(() => {});