Merge pull request #1390 from qawolf/chajac/outer-hop-validate

fix(runner): validate outer hop `node_modules` against declared deps

Author: chajac

.changeset/validate-outer-hop-discovery.md

@@ -0,0 +1,5 @@
+---
+"@qawolf/cli": patch
+---
+
+Fix `ERR_MODULE_NOT_FOUND` for correctly declared flow dependencies: dependency discovery now validates an ancestor `node_modules` before reusing it, and falls back to installing the project's declared deps when none satisfies them (reported as `Installing N project dependencies…`).

src/commands/flows/buildFlowsRunDeps.ts

@@ -22,6 +22,7 @@ type BuildFlowsRunDepsArgs = {
   android: ReturnType<typeof createAndroidDeps>;
   runWebFlowDeps: RunWebFlowDeps;
   flags: FlowsRunFlags;
+  projectDir: string | undefined;
 };
 
 /**
@@ -31,7 +32,7 @@ type BuildFlowsRunDepsArgs = {
  * the injection point for tests) and passed in already awaited.
  */
 export function buildFlowsRunDeps(args: BuildFlowsRunDepsArgs): FlowsRunDeps {
-  const { ctx, resolvedDir, android, runWebFlowDeps, flags } = args;
+  const { ctx, resolvedDir, android, runWebFlowDeps, flags, projectDir } = args;
   return {
     peekFlowMeta: makePeekFlowMeta(ctx.fs),
     installBrowsers: (innerCtx, browsers) =>
@@ -55,6 +56,7 @@ export function buildFlowsRunDeps(args: BuildFlowsRunDepsArgs): FlowsRunDeps {
       fs: ctx.fs,
       stdout: { write: (text: string) => ctx.ui.write(text) },
       stderr: { write: (text: string) => ctx.ui.write(text) },
+      ...(projectDir !== undefined ? { projectDir } : {}),
     }),
     now: () => Date.now(),
   };

src/commands/flows/buildRunReporter.ts

@@ -15,6 +15,7 @@ export type BuildRunReporterDeps = {
   fs: Fs;
   stdout?: WriteSink;
   stderr?: WriteSink;
+  projectDir?: string;
 };
 
 /**
@@ -28,7 +29,11 @@ export function buildRunReporter(
 ): Reporter {
   const stdout = deps.stdout ?? process.stdout;
   const stderr = deps.stderr ?? process.stderr;
-  const console = createConsoleReporter({ stdout, stderr });
+  const console = createConsoleReporter({
+    stdout,
+    stderr,
+    ...(deps.projectDir !== undefined ? { projectDir: deps.projectDir } : {}),
+  });
   // `undefined` means the flag was absent; an empty string (`--junit=`) still
   // enables it and resolves to the default path.
   if (flags.junit === undefined) return console;

src/commands/flows/hybridRun.test.ts

@@ -46,6 +46,7 @@ beforeEach(() => {
   prepareRunDirMock.mockResolvedValue({
     files: [],
     runDir: "/mock/run",
+    outerHop: { mode: "none" },
     cleanup: async () => {},
   });
   configureTestkitMock.mockResolvedValue(undefined);
@@ -121,6 +122,7 @@ describe("handleHybridFlowsRun", () => {
     prepareRunDirMock.mockResolvedValue({
       files: ["/mock/.qawolf/my-env/login.flow.ts"],
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup: async () => {},
     });
 
@@ -165,12 +167,14 @@ describe("handleHybridFlowsRun", () => {
       deps,
     );
 
-    expect(prepareRunDirMock).toHaveBeenCalledWith({
-      files: [`${envDir}/login.flow.ts`],
-      projectDir: undefined,
-      depsRoot: "/managed",
-      runRoot: runStagingRoot(),
-    });
+    expect(prepareRunDirMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        files: [`${envDir}/login.flow.ts`],
+        projectDir: undefined,
+        depsRoot: "/managed",
+        runRoot: runStagingRoot(),
+      }),
+    );
   });
 
   it("passes staged files from prepareRunDir to flowsRun", async () => {
@@ -182,6 +186,7 @@ describe("handleHybridFlowsRun", () => {
     prepareRunDirMock.mockResolvedValue({
       files: ["/mock/run/exec/login.flow.ts"],
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup: async () => {},
     });
 
@@ -211,6 +216,7 @@ describe("handleHybridFlowsRun", () => {
     prepareRunDirMock.mockResolvedValue({
       files: ["/mock/.qawolf/my-env/login.flow.ts"],
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup,
     });
 
@@ -234,6 +240,7 @@ describe("handleHybridFlowsRun", () => {
     prepareRunDirMock.mockResolvedValue({
       files: ["/mock/.qawolf/my-env/login.flow.ts"],
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup: async () => {},
     });
 
@@ -305,6 +312,7 @@ describe("handleHybridFlowsRun", () => {
         "/mock/.qawolf/my-env/b.flow.ts",
       ],
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup: async () => {},
     });
 

src/commands/flows/runDefaults.handle.test.ts

@@ -92,6 +92,7 @@ beforeEach(() => {
   prepareRunDirMock.mockResolvedValue({
     files: [],
     runDir: "/mock/run",
+    outerHop: { mode: "none" },
     cleanup: async () => {},
   });
   configureTestkitMock.mockResolvedValue(undefined);
@@ -243,19 +244,22 @@ describe("handleFlowsRun", () => {
 
     await handleFlowsRun(makeCtx(), undefined, defaultFlags(), makeDeps());
 
-    expect(prepareRunDirMock).toHaveBeenCalledWith({
-      files: ["/some/flow.ts"],
-      projectDir: undefined,
-      depsRoot: "/managed",
-      runRoot: runStagingRoot(),
-    });
+    expect(prepareRunDirMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        files: ["/some/flow.ts"],
+        projectDir: undefined,
+        depsRoot: "/managed",
+        runRoot: runStagingRoot(),
+      }),
+    );
   });
 
   it("passes staged files from prepareRunDir to flowsRun", async () => {
     expandPatternsMock.mockResolvedValue(["/some/flow.ts"]);
     prepareRunDirMock.mockResolvedValue({
       files: ["/mock/run/exec/flow.ts"],
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup: async () => {},
     });
 
@@ -276,6 +280,7 @@ describe("handleFlowsRun", () => {
     prepareRunDirMock.mockResolvedValue({
       files: ["/some/flow.ts"],
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup,
     });
 

src/commands/flows/runDefaults.reporterWiring.test.ts

@@ -70,6 +70,7 @@ function makeDeps(
     prepareRunDir: async (args) => ({
       files: args.files,
       runDir: "/mock/run",
+      outerHop: { mode: "none" },
       cleanup: async () => {},
     }),
     configureTestkit: async () => {},

src/commands/flows/runStagedFlows.test.ts

@@ -19,6 +19,7 @@ const flowsRunMock = mock<StagedRunDeps["flowsRun"]>();
 const runWebFlowDepsMock = mock<(...args: unknown[]) => Promise<unknown>>();
 const cleanupMock = mock<() => Promise<void>>();
 const uiInfoMock = mock<(message: string) => void>();
+const debugMock = mock<(message: string) => void>();
 
 const trackedMocks = [
   resolveDepsRootMock,
@@ -28,6 +29,7 @@ const trackedMocks = [
   runWebFlowDepsMock,
   cleanupMock,
   uiInfoMock,
+  debugMock,
 ];
 
 function makeDeps(): StagedRunDeps {
@@ -64,7 +66,7 @@ function makeCtx(): CommandContext {
     isInteractive: false,
     signals: noopSignals,
     fs: makeMemoryFs(),
-    log: () => makeNoopLogger(),
+    log: () => ({ ...makeNoopLogger(), debug: debugMock }),
     ui: { ...makeFakeUI("human"), info: uiInfoMock },
   } as unknown as CommandContext;
 }
@@ -80,6 +82,7 @@ beforeEach(() => {
   prepareRunDirMock.mockResolvedValue({
     files: ["/mock/run/exec/flow.ts"],
     runDir: "/mock/run",
+    outerHop: { mode: "none" },
     cleanup: cleanupMock,
   });
   configureTestkitMock.mockResolvedValue(undefined);
@@ -188,4 +191,46 @@ describe("runStagedFlows", () => {
       overrideDir: "/custom/deps",
     });
   });
+
+  it("renders a status line when the outer-hop fallback install starts", async () => {
+    await runStagedFlows({
+      ctx: makeCtx(),
+      files: ["/some/flow.ts"],
+      flags: defaultFlags(),
+      deps: makeDeps(),
+    });
+
+    const call = prepareRunDirMock.mock.calls[0]?.[0];
+    expect(call?.onInstallStart).toBeDefined();
+    call?.onInstallStart?.(3);
+    expect(uiInfoMock).toHaveBeenCalledWith(
+      runnerMessages.installingProjectDeps(3),
+    );
+  });
+
+  it("debug-logs rejected outer-hop candidates when the fallback install ran", async () => {
+    prepareRunDirMock.mockResolvedValue({
+      files: ["/mock/run/exec/flow.ts"],
+      runDir: "/mock/run",
+      outerHop: {
+        mode: "install",
+        depCount: 1,
+        rejected: [{ dir: "/host/node_modules", missing: ["date-fns"] }],
+      },
+      cleanup: cleanupMock,
+    });
+
+    await runStagedFlows({
+      ctx: makeCtx(),
+      files: ["/some/flow.ts"],
+      flags: defaultFlags(),
+      deps: makeDeps(),
+    });
+
+    expect(debugMock).toHaveBeenCalledWith(
+      runnerMessages.outerHopCandidateRejected("/host/node_modules", [
+        "date-fns",
+      ]),
+    );
+  });
 });

src/commands/flows/runStagedFlows.ts

@@ -79,8 +79,22 @@ export async function runStagedFlows(
     projectDir,
     depsRoot: runtimeEnv.depsRoot,
     runRoot: runStagingRoot(),
+    onInstallStart: (depCount) =>
+      ctx.ui.info(runnerMessages.installingProjectDeps(depCount)),
   });
 
+  if (staged.outerHop.mode === "install") {
+    const log = ctx.log("runner");
+    for (const candidate of staged.outerHop.rejected) {
+      log.debug(
+        runnerMessages.outerHopCandidateRejected(
+          candidate.dir,
+          candidate.missing,
+        ),
+      );
+    }
+  }
+
   // Register cleanup before the setup calls below so a throw in configureTestkit
   // / runWebFlowDeps never leaks the per-run staging directory.
   const unregisterCleanup = ctx.signals.register(staged.cleanup);
@@ -94,7 +108,14 @@ export async function runStagedFlows(
       ctx,
       staged.files,
       flags,
-      buildFlowsRunDeps({ ctx, resolvedDir, android, runWebFlowDeps, flags }),
+      buildFlowsRunDeps({
+        ctx,
+        resolvedDir,
+        android,
+        runWebFlowDeps,
+        flags,
+        projectDir,
+      }),
     );
   } finally {
     unregisterCleanup();

src/core/errors.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from "bun:test";
 
-import { errorCode, isNoEntError } from "./errors.js";
+import { errorCode, extractMissingPackage, isNoEntError } from "./errors.js";
 
 describe("errorCode", () => {
   it("returns the string code of an error-like value", () => {
@@ -36,3 +36,43 @@ describe("isNoEntError", () => {
     expect(isNoEntError(Error("boom"))).toBe(false);
   });
 });
+
+describe("extractMissingPackage", () => {
+  it("extracts the package name from an ESM resolution error", () => {
+    const text =
+      "Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'date-fns' imported from /x/y.js";
+    expect(extractMissingPackage(text)).toBe("date-fns");
+  });
+
+  it("extracts a scoped package name from a CJS resolution error", () => {
+    expect(extractMissingPackage("Cannot find module '@faker-js/faker'")).toBe(
+      "@faker-js/faker",
+    );
+  });
+
+  it("returns undefined for non-resolution errors", () => {
+    expect(extractMissingPackage("locator timeout")).toBeUndefined();
+  });
+
+  it("returns undefined for a relative file path specifier", () => {
+    expect(
+      extractMissingPackage(
+        "Cannot find module './helper.js' imported from /x/y.js",
+      ),
+    ).toBeUndefined();
+  });
+
+  it("returns undefined for an absolute file path specifier", () => {
+    expect(
+      extractMissingPackage(
+        "Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/run/exec/helper.js' imported from /x/y.js",
+      ),
+    ).toBeUndefined();
+  });
+
+  it("returns undefined for a Windows drive-letter path specifier", () => {
+    expect(
+      extractMissingPackage("Cannot find module 'C:\\flows\\helper.js'"),
+    ).toBeUndefined();
+  });
+});

src/core/errors.ts

@@ -17,3 +17,20 @@ export function errorCode(err: unknown): string | undefined {
 export function isNoEntError(err: unknown): boolean {
   return errorCode(err) === "ENOENT";
 }
+
+const missingPackagePattern = /Cannot find (?:package|module) '([^']+)'/;
+const pathLikeSpecifierPattern = /^(?:\.|\/|[A-Za-z]:[\\/])/;
+
+/**
+ * The package name from a Node "Cannot find package 'x'" / "Cannot find
+ * module 'x'" resolution error text, or undefined when the text is not a
+ * module-resolution failure or when the specifier is a file path (relative,
+ * absolute, or Windows drive-letter prefix) rather than a bare package name.
+ */
+export function extractMissingPackage(text: string): string | undefined {
+  const specifier = missingPackagePattern.exec(text)?.[1];
+  if (specifier === undefined || pathLikeSpecifierPattern.test(specifier)) {
+    return undefined;
+  }
+  return specifier;
+}