diff --git a/.changeset/flows-run-deterministic-exit.md b/.changeset/flows-run-deterministic-exit.md new file mode 100644 index 000000000..6daae7fe6 --- /dev/null +++ b/.changeset/flows-run-deterministic-exit.md @@ -0,0 +1,5 @@ +--- +"@qawolf/cli": patch +--- + +Exit `flows run` deterministically once the run completes. The flow runtime can launch browser processes (e.g. a channel-launched Google Chrome) whose CDP sockets and timers keep Node's event loop alive after teardown, so the CLI previously printed its results and then hung indefinitely. The process now flushes stdout/stderr and exits with the run's exit code (1 on flow failure, 0 on pass) as soon as the command resolves, with a backstop in case a stream stalls. diff --git a/.changeset/ts-js-module-resolution-fallback.md b/.changeset/ts-js-module-resolution-fallback.md new file mode 100644 index 000000000..2f50cc60f --- /dev/null +++ b/.changeset/ts-js-module-resolution-fallback.md @@ -0,0 +1,5 @@ +--- +"@qawolf/cli": patch +--- + +Resolve flow imports whose specifier uses a `.ts` extension but ships as `.js` (and vice versa). Platform-generated bundles often import sibling utilities as `.ts` while the file on disk is `.js`; native Node ESM resolves extensions literally and throws `ERR_MODULE_NOT_FOUND`. A synchronous `module.registerHooks` resolve hook now transparently retries the sibling source extension (`.ts`↔`.js`, `.mts`↔`.mjs`, `.cts`↔`.cjs`) only on resolution failure — literal matches always win and nothing is rewritten on disk. Raises the Node engine floor to `>=22.15.0`, the release that introduced synchronous hooks. diff --git a/package.json b/package.json index c16b11d1a..9f12c918b 100644 --- a/package.json +++ b/package.json @@ -91,7 +91,7 @@ "typescript": "6.0.3" }, "engines": { - "node": ">=22.12.0" + "node": ">=22.15.0" }, "packageManager": "bun@1.3.13" } diff --git a/src/core/errors.test.ts b/src/core/errors.test.ts new file mode 100644 index 000000000..4baf4cfb9 --- /dev/null +++ b/src/core/errors.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "bun:test"; + +import { errorCode, isNoEntError } from "./errors.js"; + +describe("errorCode", () => { + it("returns the string code of an error-like value", () => { + expect(errorCode(Object.assign(Error("boom"), { code: "ENOENT" }))).toBe( + "ENOENT", + ); + expect(errorCode({ code: "ERR_MODULE_NOT_FOUND" })).toBe( + "ERR_MODULE_NOT_FOUND", + ); + }); + + it("returns undefined when no code is present", () => { + expect(errorCode(Error("boom"))).toBeUndefined(); + expect(errorCode({})).toBeUndefined(); + }); + + it("returns undefined for a non-string code", () => { + expect(errorCode({ code: 42 })).toBeUndefined(); + }); + + it("returns undefined for nullish or primitive values", () => { + // oxlint-disable-next-line no-null -- exercises the `err === null` guard branch + expect(errorCode(null)).toBeUndefined(); + expect(errorCode(undefined)).toBeUndefined(); + expect(errorCode("ENOENT")).toBeUndefined(); + }); +}); + +describe("isNoEntError", () => { + it("is true only when the code is ENOENT", () => { + expect(isNoEntError({ code: "ENOENT" })).toBe(true); + expect(isNoEntError({ code: "EACCES" })).toBe(false); + expect(isNoEntError(Error("boom"))).toBe(false); + }); +}); diff --git a/src/core/errors.ts b/src/core/errors.ts index f687a4684..97c235188 100644 --- a/src/core/errors.ts +++ b/src/core/errors.ts @@ -2,11 +2,18 @@ export function errorMessage(err: unknown): string { return err instanceof Error ? err.message : String(err); } +/** + * The `code` of an error-like value (e.g. `ENOENT`, `ERR_MODULE_NOT_FOUND`), or + * undefined when the value carries no string `code`. + */ +export function errorCode(err: unknown): string | undefined { + if (typeof err !== "object" || err === null || !("code" in err)) { + return undefined; + } + const { code } = err; + return typeof code === "string" ? code : undefined; +} + export function isNoEntError(err: unknown): boolean { - return ( - typeof err === "object" && - err !== null && - "code" in err && - (err as { code: unknown }).code === "ENOENT" - ); + return errorCode(err) === "ENOENT"; } diff --git a/src/domains/runner/loadFlowDefault.ts b/src/domains/runner/loadFlowDefault.ts index 57bcba695..aefa5b21b 100644 --- a/src/domains/runner/loadFlowDefault.ts +++ b/src/domains/runner/loadFlowDefault.ts @@ -3,14 +3,14 @@ import { pathToFileURL } from "node:url"; import { runnerMessages } from "~/core/messages/index.js"; import { makeDefaultFs, type Fs } from "~/shell/fs.js"; +import { registerFlowModuleResolver } from "~/shell/resolver/registerFlowModuleResolver.js"; import { type FlowBundler, defaultFlowBundler } from "./bundleFlow.js"; /** * Only the compiled binary needs bundling — it alone cannot resolve exports-map - * bare specifiers from external node_modules. Node and `bun run`/`bun test` - * resolve them directly, so they take the direct-import path. QAWOLF_COMPILED is - * injected via --define at binary build time (see build:binary in package.json). - * Tests inject bundleFlow explicitly to exercise either path deterministically. + * bare specifiers from external node_modules; Node and `bun run` resolve them + * directly. QAWOLF_COMPILED is injected via --define at binary build time (see + * build:binary). Tests inject bundleFlow explicitly to exercise either path. */ const defaultBundleFlow: FlowBundler | undefined = process.env.QAWOLF_COMPILED === "true" ? defaultFlowBundler : undefined; @@ -72,6 +72,7 @@ export async function loadFlowDefault( } = args; if (bundleFlow === undefined) { + registerFlowModuleResolver(); return importDefaultExport(pathToFileURL(flowPath).href, flowPath); } diff --git a/src/main.ts b/src/main.ts index d9d38f9b9..9615f4377 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,5 +1,6 @@ import { createSignalRegistry } from "./shell/signals/createSignalRegistry.js"; import { createProgram } from "./commands/program.js"; +import { flushAndExit } from "./shell/exit.js"; const signals = createSignalRegistry(); @@ -15,4 +16,12 @@ const onSignal = (sig: "SIGINT" | "SIGTERM") => () => { process.on("SIGINT", onSignal("SIGINT")); process.on("SIGTERM", onSignal("SIGTERM")); -createProgram({ signals }).parse(); +// Exit deterministically once the command resolves — see flushAndExit for why. +void createProgram({ signals }) + .parseAsync() + .catch(() => { + if (process.exitCode === undefined) process.exitCode = 1; + }) + .finally(() => + flushAndExit(typeof process.exitCode === "number" ? process.exitCode : 0), + ); diff --git a/src/shell/exit.test.ts b/src/shell/exit.test.ts index 1824f6cac..78acc5168 100644 --- a/src/shell/exit.test.ts +++ b/src/shell/exit.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "bun:test"; -import { exitCodes, exit } from "./exit.js"; +import { exitCodes, exit, flushAndExit } from "./exit.js"; function createFakeProcess() { const stderr: string[] = []; @@ -61,3 +61,76 @@ describe("exit", () => { }); }); }); + +function createFlushFakeProcess() { + const exitCalls: number[] = []; + let stdoutCb: (() => void) | undefined; + let stderrCb: (() => void) | undefined; + const proc = { + stdout: { + write: (_chunk: string, cb: () => void): boolean => { + stdoutCb = cb; + return true; + }, + }, + stderr: { + write: (_chunk: string, cb: () => void): boolean => { + stderrCb = cb; + return true; + }, + }, + exit: (code: number): never => { + exitCalls.push(code); + throw Error("__fake-exit__"); + }, + }; + return { + proc, + exitCalls, + flushStdout: () => stdoutCb?.(), + flushStderr: () => stderrCb?.(), + }; +} + +describe("flushAndExit", () => { + it("exits with the given code once both streams have flushed", () => { + const { proc, exitCalls, flushStdout, flushStderr } = + createFlushFakeProcess(); + flushAndExit(exitCodes.testFailure, proc, () => {}); + flushStdout(); + expect(exitCalls).toEqual([]); + expect(() => flushStderr()).toThrow("__fake-exit__"); + expect(exitCalls).toEqual([1]); + }); + + it("does not exit until both streams have flushed", () => { + const { proc, exitCalls, flushStdout } = createFlushFakeProcess(); + flushAndExit(exitCodes.success, proc, () => {}); + flushStdout(); + expect(exitCalls).toEqual([]); + }); + + it("forces exit via the backstop when a stream stalls", () => { + const { proc, exitCalls } = createFlushFakeProcess(); + let backstop: (() => void) | undefined; + flushAndExit(exitCodes.testFailure, proc, (fn) => { + backstop = fn; + }); + expect(exitCalls).toEqual([]); + expect(() => backstop?.()).toThrow("__fake-exit__"); + expect(exitCalls).toEqual([1]); + }); + + it("exits at most once when flush and backstop both fire", () => { + const { proc, exitCalls, flushStdout, flushStderr } = + createFlushFakeProcess(); + let backstop: (() => void) | undefined; + flushAndExit(exitCodes.testFailure, proc, (fn) => { + backstop = fn; + }); + flushStdout(); + expect(() => flushStderr()).toThrow("__fake-exit__"); + backstop?.(); + expect(exitCalls).toEqual([1]); + }); +}); diff --git a/src/shell/exit.ts b/src/shell/exit.ts index f219037d4..c44d90fd3 100644 --- a/src/shell/exit.ts +++ b/src/shell/exit.ts @@ -24,3 +24,45 @@ export function exit( } return proc.exit(code); } + +type FlushExitProcess = { + readonly stdout: { + readonly write: (chunk: string, cb: () => void) => unknown; + }; + readonly stderr: { + readonly write: (chunk: string, cb: () => void) => unknown; + }; + readonly exit: (code: number) => never; +}; + +/** + * Flush stdout and stderr, then exit with `code`. The flow runtime can leave + * browser processes and CDP sockets the event loop never drains, so the CLI + * exits deterministically once its command resolves. The empty-`write` + * callbacks flush buffered output first; the backstop forces exit if a stream + * stalls (e.g. EPIPE on a closed pipe). + */ +export function flushAndExit( + code: number, + proc: FlushExitProcess = process, + scheduleBackstop: (fn: () => void) => void = (fn) => { + setTimeout(fn, 2000).unref(); + }, +): void { + let exited = false; + const exitOnce = (): void => { + if (exited) return; + exited = true; + proc.exit(code); + }; + + let pending = 2; + const onFlushed = (): void => { + pending -= 1; + if (pending === 0) exitOnce(); + }; + proc.stdout.write("", onFlushed); + proc.stderr.write("", onFlushed); + + scheduleBackstop(exitOnce); +} diff --git a/src/shell/resolver/registerFlowModuleResolver.test.ts b/src/shell/resolver/registerFlowModuleResolver.test.ts new file mode 100644 index 000000000..8d7af48f0 --- /dev/null +++ b/src/shell/resolver/registerFlowModuleResolver.test.ts @@ -0,0 +1,134 @@ +import { afterEach, describe, expect, it, mock } from "bun:test"; + +import { + type FlowResolveHook, + flowResolveHook, + resolveRegisterHooks, +} from "./registerFlowModuleResolver.js"; + +type NextResolve = Parameters[2]; +type Context = Parameters[1]; +type CodedError = Error & { code: string }; + +const fakeContext = { + conditions: [], + importAttributes: {}, + parentURL: undefined, +} as unknown as Context; + +function makeModuleNotFoundError(specifier: string): CodedError { + const err = Error( + `Cannot find module '${specifier}'`, + ) as unknown as CodedError; + err.code = "ERR_MODULE_NOT_FOUND"; + return err; +} + +afterEach(() => { + mock.restore(); +}); + +describe("flowResolveHook", () => { + it("returns the resolution on the first try without attempting a swap", () => { + const result = { url: "file:///path/to/code-snippets.js" }; + const nextResolve = mock(() => result) as unknown as NextResolve; + + const out = flowResolveHook("./code-snippets.js", fakeContext, nextResolve); + + expect(out).toBe(result); + expect(nextResolve).toHaveBeenCalledTimes(1); + expect(nextResolve).toHaveBeenCalledWith("./code-snippets.js", fakeContext); + }); + + it("retries the .js sibling when .ts throws ERR_MODULE_NOT_FOUND", () => { + const tsErr = makeModuleNotFoundError("./code-snippets.ts"); + const jsResult = { url: "file:///path/to/code-snippets.js" }; + const nextResolve = mock((specifier: string) => { + if (specifier.endsWith(".ts")) throw tsErr; + return jsResult; + }) as unknown as NextResolve; + + const out = flowResolveHook("./code-snippets.ts", fakeContext, nextResolve); + + expect(out).toBe(jsResult); + }); + + it("rethrows non-module-not-found errors without attempting a swap", () => { + const syntaxErr = Error("Unexpected token") as unknown as CodedError; + syntaxErr.code = "ERR_SOMETHING_ELSE"; + const nextResolve = mock(() => { + throw syntaxErr; + }) as unknown as NextResolve; + + let caught: unknown; + try { + flowResolveHook("./foo.ts", fakeContext, nextResolve); + } catch (e) { + caught = e; + } + + expect(caught).toBe(syntaxErr); + expect(nextResolve).toHaveBeenCalledTimes(1); + }); + + it("rethrows the original .ts error when the .js swap also fails", () => { + const tsErr = makeModuleNotFoundError("./missing.ts"); + const jsErr = makeModuleNotFoundError("./missing.js"); + const nextResolve = mock((specifier: string) => { + if (specifier.endsWith(".ts")) throw tsErr; + throw jsErr; + }) as unknown as NextResolve; + + let caught: unknown; + try { + flowResolveHook("./missing.ts", fakeContext, nextResolve); + } catch (e) { + caught = e; + } + + expect(caught).toBe(tsErr); + }); + + it("rethrows original error for specifiers with no swappable extension", () => { + const err = makeModuleNotFoundError("axios"); + const nextResolve = mock(() => { + throw err; + }) as unknown as NextResolve; + + let caught: unknown; + try { + flowResolveHook("axios", fakeContext, nextResolve); + } catch (e) { + caught = e; + } + + expect(caught).toBe(err); + expect(nextResolve).toHaveBeenCalledTimes(1); + }); +}); + +describe("resolveRegisterHooks", () => { + it("finds registerHooks on a function carrier (node:module's default export)", () => { + // node:module's default export is the Module function, not a plain object. + const moduleFn = (() => undefined) as unknown as Record; + const registerHooks = (): void => undefined; + moduleFn["registerHooks"] = registerHooks; + + expect(resolveRegisterHooks(moduleFn)).toBe(registerHooks); + }); + + it("finds registerHooks on an object carrier", () => { + const registerHooks = (): void => undefined; + expect(resolveRegisterHooks({ registerHooks })).toBe(registerHooks); + }); + + it("returns undefined when registerHooks is absent (Bun / Node < 22.15)", () => { + expect(resolveRegisterHooks({})).toBeUndefined(); + expect(resolveRegisterHooks(() => undefined)).toBeUndefined(); + }); + + it("returns undefined for nullish or primitive module values", () => { + expect(resolveRegisterHooks(undefined)).toBeUndefined(); + expect(resolveRegisterHooks(42)).toBeUndefined(); + }); +}); diff --git a/src/shell/resolver/registerFlowModuleResolver.ts b/src/shell/resolver/registerFlowModuleResolver.ts new file mode 100644 index 000000000..13e3138db --- /dev/null +++ b/src/shell/resolver/registerFlowModuleResolver.ts @@ -0,0 +1,102 @@ +import nodeModule from "node:module"; + +import { errorCode } from "~/core/errors.js"; +import { swapSourceExtension } from "./swapSourceExtension.js"; + +// Minimal local types for the Node.js synchronous resolve hook API (v22.15+); context is passed through opaquely. +type ResolveContext = { + readonly conditions: readonly string[]; + readonly importAttributes: Record; + readonly parentURL: string | undefined; +}; + +type ResolveFnOutput = { + url: string; + shortCircuit?: boolean; + format?: string | null; +}; + +type NextResolve = ( + specifier: string, + context?: Partial, +) => ResolveFnOutput; + +/** The synchronous Node.js ESM resolve hook signature. */ +export type FlowResolveHook = ( + specifier: string, + context: ResolveContext, + nextResolve: NextResolve, +) => ResolveFnOutput; + +type NodeModuleWithHooks = { + registerHooks: (options: { resolve?: FlowResolveHook }) => void; +}; + +let registered = false; + +function isModuleNotFound(err: unknown): boolean { + return errorCode(err) === "ERR_MODULE_NOT_FOUND"; +} + +/** + * Synchronous ESM resolve hook that retries the sibling source extension when a + * specifier fails with ERR_MODULE_NOT_FOUND. See registerFlowModuleResolver for why. + */ +export function flowResolveHook( + specifier: string, + context: ResolveContext, + nextResolve: NextResolve, +): ResolveFnOutput { + try { + return nextResolve(specifier, context); + } catch (err) { + if (!isModuleNotFound(err)) throw err; + const swapped = swapSourceExtension(specifier); + if (swapped === undefined) throw err; + try { + return nextResolve(swapped, context); + } catch { + throw err; + } + } +} + +/** + * Type guard for a `node:module` value that carries `registerHooks`. The default + * export of `node:module` is the Module *function*, so this admits functions as + * well as objects rather than gating on `typeof === "object"` — which would + * reject the function carrier and never register. + */ +function hasRegisterHooks(mod: unknown): mod is NodeModuleWithHooks { + if (mod === null || (typeof mod !== "object" && typeof mod !== "function")) { + return false; + } + return "registerHooks" in mod && typeof mod.registerHooks === "function"; +} + +/** + * Resolves the `registerHooks` static from a `node:module` value, or undefined + * when absent (Bun, Node < 22.15). + */ +export function resolveRegisterHooks( + mod: unknown, +): NodeModuleWithHooks["registerHooks"] | undefined { + return hasRegisterHooks(mod) ? mod.registerHooks : undefined; +} + +/** + * Registers a synchronous ESM resolve hook that aliases sibling source + * extensions (`.ts`↔`.js`, `.mts`↔`.mjs`, `.cts`↔`.cjs`), retrying the sibling + * on ERR_MODULE_NOT_FOUND — the native-Node equivalent of a bundler's + * extension-alias resolution. No-ops where `module.registerHooks` is + * unavailable (Bun, Node < 22.15). + */ +export function registerFlowModuleResolver(): void { + if (registered) return; + registered = true; + + const registerHooks = resolveRegisterHooks(nodeModule); + if (registerHooks === undefined) return; + + registerHooks({ resolve: flowResolveHook }); +} diff --git a/src/shell/resolver/swapSourceExtension.test.ts b/src/shell/resolver/swapSourceExtension.test.ts new file mode 100644 index 000000000..3674fe3be --- /dev/null +++ b/src/shell/resolver/swapSourceExtension.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "bun:test"; + +import { swapSourceExtension } from "./swapSourceExtension.js"; + +describe("swapSourceExtension", () => { + it("swaps .ts to .js", () => { + expect(swapSourceExtension("../../../utilities/code-snippets.ts")).toBe( + "../../../utilities/code-snippets.js", + ); + }); + + it("swaps .js to .ts", () => { + expect(swapSourceExtension("../../../utilities/code-snippets.js")).toBe( + "../../../utilities/code-snippets.ts", + ); + }); + + it("swaps .mts to .mjs", () => { + expect(swapSourceExtension("./module.mts")).toBe("./module.mjs"); + }); + + it("swaps .mjs to .mts", () => { + expect(swapSourceExtension("./module.mjs")).toBe("./module.mts"); + }); + + it("swaps .cts to .cjs", () => { + expect(swapSourceExtension("./compat.cts")).toBe("./compat.cjs"); + }); + + it("swaps .cjs to .cts", () => { + expect(swapSourceExtension("./compat.cjs")).toBe("./compat.cts"); + }); + + it("returns undefined for bare package specifier", () => { + expect(swapSourceExtension("axios")).toBeUndefined(); + }); + + it("returns undefined for scoped package specifier", () => { + expect(swapSourceExtension("@qawolf/flows/web")).toBeUndefined(); + }); + + it("returns undefined for a package subpath with a known extension", () => { + expect(swapSourceExtension("pkg/file.js")).toBeUndefined(); + }); + + it("returns undefined for a scoped package subpath with a known extension", () => { + expect(swapSourceExtension("@scope/pkg/file.ts")).toBeUndefined(); + }); + + it("swaps an absolute path", () => { + expect(swapSourceExtension("/abs/utilities/code-snippets.ts")).toBe( + "/abs/utilities/code-snippets.js", + ); + }); + + it("swaps a file: URL", () => { + expect(swapSourceExtension("file:///abs/utilities/code-snippets.ts")).toBe( + "file:///abs/utilities/code-snippets.js", + ); + }); + + it("returns undefined for extensionless relative path", () => { + expect(swapSourceExtension("./foo")).toBeUndefined(); + }); + + it("returns undefined for unknown extension", () => { + expect(swapSourceExtension("./data.json")).toBeUndefined(); + }); +}); diff --git a/src/shell/resolver/swapSourceExtension.ts b/src/shell/resolver/swapSourceExtension.ts new file mode 100644 index 000000000..552f516b4 --- /dev/null +++ b/src/shell/resolver/swapSourceExtension.ts @@ -0,0 +1,30 @@ +const extensionSwaps: Record = { + ".ts": ".js", + ".js": ".ts", + ".mts": ".mjs", + ".mjs": ".mts", + ".cts": ".cjs", + ".cjs": ".cts", +}; + +/** + * Maps the trailing source extension of a local file specifier (relative, + * absolute, or `file:` URL) to its sibling and returns the rewritten specifier. + * Returns undefined for bare or package-subpath imports and unknown extensions, + * so only sibling files are ever rewritten — never a dependency module. + */ +export function swapSourceExtension(specifier: string): string | undefined { + const isLocalFileSpecifier = + specifier.startsWith("./") || + specifier.startsWith("../") || + specifier.startsWith("/") || + specifier.startsWith("file:"); + if (!isLocalFileSpecifier) return undefined; + + const dotIndex = specifier.lastIndexOf("."); + if (dotIndex === -1) return undefined; + const ext = specifier.slice(dotIndex); + const swapped = extensionSwaps[ext]; + if (swapped === undefined) return undefined; + return specifier.slice(0, dotIndex) + swapped; +}