feat(runner): isolate runtime deps to fix workspace monorepo failures#1381
Merged
Michael Price (michael-pr) merged 32 commits intoJun 26, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR replaces the prior flow-env dependency setup with a managed runtime root model. It adds runtime-env resolution, install, probe, and clear helpers; stages flow runs through a hashed run directory with inner and outer node_modules hops; threads Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
Author
|
CodeRabbit (@coderabbitai) review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/domains/runner/initFlowRuntime.ts (1)
116-123: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude
depsRootin the init cache keyLine 120 keys
initCacheonly bystartDir, but Line 122 passesoptions.depsRootintodoInit. Calls from the same directory with differentdepsRootvalues can incorrectly reuse the first initialization and load the wrong runner.Suggested fix
export function initFlowRuntime( flowPath: string, options: InitFlowRuntimeOptions, fs: Fs = makeDefaultFs(), ): Promise<void> { const startDir = path.dirname(flowPath); + const depsKey = + options.depsRoot === undefined ? "" : path.resolve(options.depsRoot); + const cacheKey = `${startDir}::${depsKey}`; // Cache key is startDir, not fs — tests reusing the same startDir must call // _resetInitCache() between runs. Timeout is omitted deliberately: it is a // single run-global flag, so every flow in a process shares one value. - let p = initCache.get(startDir); + let p = initCache.get(cacheKey); if (!p) { p = doInit(flowPath, options.timeout, fs, options.depsRoot); - initCache.set(startDir, p); + initCache.set(cacheKey, p); } return p; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domains/runner/initFlowRuntime.ts` around lines 116 - 123, The initCache key in the initFlowRuntime function only includes startDir, but doInit receives both startDir and options.depsRoot as parameters. When the same directory is initialized with different depsRoot values, the cache incorrectly reuses the first initialization. Create a composite cache key that combines both startDir and options.depsRoot, then use this composite key in both the initCache.get() call and the initCache.set() call to ensure different depsRoot values for the same directory produce separate cache entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/install/all.ts`:
- Around line 57-59: Move the deps.ensureRuntimeEnv() call and depsRoot
assignment from its current position at line 57 to after the flow target
detection logic (after the hasWeb and hasAndroid checks around lines 73/82).
This ensures runtime environment resolution only occurs when there are actually
installable flows (web or android targets present), preventing unnecessary
managed installs or install failures when only iOS flows or no flows exist.
Update any references to depsRoot to use the value obtained from the deferred
ensureRuntimeEnv call.
In `@src/commands/install/browsers.ts`:
- Around line 21-41: The runtime environment resolution via ensureRuntimeEnv is
being called too early before confirming whether browser flows actually exist.
Move the ensureRuntimeEnv call and the subsequent resolvePlaywrightCli call to
execute lazily after browser targets have been confirmed to exist. This defers
unnecessary work when browser collection is a no-op and prevents offline
failures on otherwise skippable runs. Identify where browser targets are
actually validated in the flow and relocate the depsRoot resolution and runtime
environment setup to occur only after that point.
In `@src/domains/runtimeEnv/ensureRuntimeEnv.ts`:
- Around line 64-65: After the install(managed) call completes successfully, add
a validation check to ensure all pinned dependencies are actually resolved
before returning installed: true. Use the allPinnedResolved function to verify
that the install operation fully materialized the pinned deps for the managed
depsRoot. If the validation check fails, throw an error or fail fast to prevent
reporting success when dependencies were not fully installed.
In `@src/domains/runtimeEnv/installPinned.ts`:
- Around line 35-40: The readiness check in the `installPinned` function at line
35-40 uses `existsSync` to verify that `node_modules/.bin/playwright` exists,
but this is insufficient because the binary could exist while pinned versions
are missing or mismatched. Replace the `existsSync` check on the playwright
binary path with a call to `allPinnedResolved` to properly validate that all
pinned dependencies are correctly installed and resolved. Apply the same fix to
the duplicate check mentioned at lines 64-67.
- Around line 52-54: The error message in the Error constructor that handles
failed managed runtime installation is directly interpolating raw stderr output
from npm, which can expose sensitive information. Replace the raw
`result.stderr.trim()` in the thrown error message with a generic sanitized
error summary. If detailed stderr information is needed for debugging, move the
raw stderr to a separate debug logging call using controlled logging paths,
keeping the thrown error message safe and user-facing while preserving debugging
capabilities.
In `@src/domains/runtimeEnv/resolvePinned.ts`:
- Around line 38-40: In the resolvePinned.ts file, the fs.existsSync check that
verifies the playwright binary existence only looks for the extension-less
playwright script. Modify this condition to check for both the extension-less
variant and the .cmd variant (playwright.cmd) since Windows npm/bun
installations create the .cmd shim alongside or instead of the extension-less
version. Update the condition to return false only if neither variant exists,
allowing the check to properly detect Playwright installations on both Windows
and POSIX systems.
---
Outside diff comments:
In `@src/domains/runner/initFlowRuntime.ts`:
- Around line 116-123: The initCache key in the initFlowRuntime function only
includes startDir, but doInit receives both startDir and options.depsRoot as
parameters. When the same directory is initialized with different depsRoot
values, the cache incorrectly reuses the first initialization. Create a
composite cache key that combines both startDir and options.depsRoot, then use
this composite key in both the initCache.get() call and the initCache.set() call
to ensure different depsRoot values for the same directory produce separate
cache entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 63fe5f4a-7d7c-4936-a062-1d63212bee77
⛔ Files ignored due to path filters (1)
src/commands/__snapshots__/help.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (47)
knip.config.tssrc/commands/doctor/handler.tssrc/commands/flows/hybridRun.test.tssrc/commands/flows/hybridRunDefaults.tssrc/commands/flows/run.register.tssrc/commands/flows/runDefaults.handle.test.tssrc/commands/flows/runDefaults.reporterWiring.test.tssrc/commands/flows/runDefaults.tssrc/commands/install/all.test.tssrc/commands/install/all.tssrc/commands/install/android.tssrc/commands/install/browsers.tssrc/core/messages/flows.tssrc/core/messages/runner.tssrc/core/paths.tssrc/domains/flows/ensureDeps.test.tssrc/domains/flows/ensureDeps.tssrc/domains/runner/createRunner.guards.test.tssrc/domains/runner/createRunner.test.tssrc/domains/runner/initFlowRuntime.test.tssrc/domains/runner/initFlowRuntime.tssrc/domains/runner/loadFlowDefault.test.tssrc/domains/runner/loadFlowDefault.tssrc/domains/runner/runAndroidFlow.test.tssrc/domains/runner/runAndroidFlow.tssrc/domains/runner/runAndroidFlowDeps.tssrc/domains/runner/runInternals.tssrc/domains/runner/runWebFlow.fixtures.tssrc/domains/runner/runWebFlow.tssrc/domains/runner/runWebFlowDeps.tssrc/domains/runner/runnerDeps.test.tssrc/domains/runner/runnerDeps.tssrc/domains/runner/types.tssrc/domains/runtimeEnv/ensureRuntimeEnv.test.tssrc/domains/runtimeEnv/ensureRuntimeEnv.tssrc/domains/runtimeEnv/index.tssrc/domains/runtimeEnv/installPinned.test.tssrc/domains/runtimeEnv/installPinned.tssrc/domains/runtimeEnv/managedEnvDir.test.tssrc/domains/runtimeEnv/managedEnvDir.tssrc/domains/runtimeEnv/pinnedPackages.tssrc/domains/runtimeEnv/resolveDepsRootIfPresent.test.tssrc/domains/runtimeEnv/resolveDepsRootIfPresent.tssrc/domains/runtimeEnv/resolvePinned.test.tssrc/domains/runtimeEnv/resolvePinned.tssrc/domains/runtimeEnv/shimDeps.test.tssrc/domains/runtimeEnv/shimDeps.ts
💤 Files with no reviewable changes (2)
- src/core/messages/flows.ts
- src/domains/flows/ensureDeps.ts
Contributor
Author
|
CodeRabbit (@coderabbitai) review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/doctor/handler.ts`:
- Around line 44-47: The issue is in the resolveDepsRootIfPresent call where
when projectDir is undefined, an empty object is passed as the second argument
instead of providing a fallback. When resolveProjectDirSafe returns undefined,
you should provide the current working directory (cwd) as a fallback value for
projectDir in the ternary operator passed to resolveDepsRootIfPresent, so that
it can still probe for dependencies in the current workspace instead of skipping
the project probe entirely and only checking managed runtime.
In `@src/domains/flows/ensureDeps.ts`:
- Around line 43-46: The catch block around the resolveUniqueEnvDir function
call is too broad and silently masks all errors, including unexpected filesystem
or logic errors. Narrow the catch block to only handle the expected
multi-package conflict error by checking the error message or type before
returning undefined. For unexpected errors, re-throw them so they surface as
actual failures rather than silently falling back to managed runtime. Ideally,
work with the resolveUniqueEnvDir function to use a dedicated error class or
code for the multi-package conflict scenario and match against that specific
error type instead of catching all exceptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dd85e8fb-94b8-4eef-9d45-fa9b8af7fefa
📒 Files selected for processing (29)
src/commands/doctor/handler.tssrc/commands/flows/buildFlowsRunDeps.tssrc/commands/flows/hybridRun.test.tssrc/commands/flows/hybridRunDefaults.tssrc/commands/flows/runDefaults.handle.test.tssrc/commands/flows/runDefaults.reporterWiring.test.tssrc/commands/flows/runDefaults.tssrc/commands/install/all.test.tssrc/commands/install/all.tssrc/commands/install/android.tssrc/commands/install/browsers.fixtures.tssrc/commands/install/browsers.tssrc/commands/resolveDepsRoot.test.tssrc/commands/resolveDepsRoot.tssrc/domains/flows/ensureDeps.tssrc/domains/install/android/index.tssrc/domains/install/android/installAndroid.test.tssrc/domains/install/browsers.tssrc/domains/runner/loadFlowDefault.tssrc/domains/runner/runInternals.tssrc/domains/runtimeEnv/ensureRuntimeEnv.test.tssrc/domains/runtimeEnv/ensureRuntimeEnv.tssrc/domains/runtimeEnv/index.tssrc/domains/runtimeEnv/installPinned.test.tssrc/domains/runtimeEnv/installPinned.tssrc/domains/runtimeEnv/resolveDepsRootIfPresent.tssrc/domains/runtimeEnv/resolvePinned.test.tssrc/domains/runtimeEnv/resolvePinned.tssrc/domains/runtimeEnv/shimDeps.ts
💤 Files with no reviewable changes (1)
- src/domains/runtimeEnv/index.ts
Contributor
Author
✅ Manual verification — monorepo / multi-package compatibility (updated)Updated after the runtime-deps redesign. The original design only reused a project-independent managed dir; the shipped design adds a per-run layered run-dir (inner pinned-executor hop + outer flow-deps hop), channel-keyed runtimes, a binary subprocess worker, and an What it took to get this fully working
Synthetic compatibility matrix (web flow →
|
| Repo shape | Node | Binary | Project mutated? |
|---|---|---|---|
Empty dir (no package.json) |
✓ | — | No |
| npm single-package | ✓ | ✓ | No |
| bun single-package | ✓ | — | No |
npm workspace (flow in leaf, no leaf node_modules) |
✓ | ✓ | No |
| pnpm workspace | ✓ | — | No |
| yarn workspace | ✓ | — | No |
| bun workspace | ✓ | — | No |
| Cross-channel (install with one channel, run with the other — both directions) | ✓ | ✓ | No |
All green, zero project pollution (find . -name node_modules -not -path '*/.qawolf/*' → empty).
Real product flow — the hardest case (project deps + a native module)
email-attachments-can-be-downloaded-from-sent-box from a 449-flow RC env. Its package.json declares diff@^8.0.3, @faker-js/faker, @workos-inc/node, axios, patchright, sharp, ms, etc.
| Channel | Result |
|---|---|
| Node | Runs ~64s of real browser execution (reconciliation → workflow activation → automation), fails only on a live-app locator.waitFor timeout (needs real creds). No dep-resolution / native-load error. |
| Binary | Runs ~64s of real browser execution (sharp + detect-libc + @img/*.node all load via the subprocess worker), fails on the same live-app timeout. No "Could not load the sharp module". |
Reaching real execution on both channels is the win — module resolution and native-addon loading are no longer the failure point.
Tests
Full suite: 1057 pass / 0 fail (144 files); typecheck, lint (oxlint), and knip clean. New/updated coverage for the inner hop (innerHop.test.ts, executes Node to prove the diff-collision is resolved), run-staging relocation, and the worker-command resolution.
Known follow-up
Outer-hop dependency discovery (findNearestNodeModulesDir) can walk up past the project boundary, so a flow project nested inside another repo whose node_modules carries a conflicting version may pick up the wrong dep. Not exercised by the matrix above (flows ran in isolated dirs); tracked as a follow-up.
Contributor
Author
|
CodeRabbit (@coderabbitai) review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/domains/runtimeEnv/managedEnvDir.test.ts`:
- Around line 42-46: The test assertion on the line with
expect(managedEnvDir()).toBe() is comparing against join("/custom/cache", hash)
but managedEnvDir() actually returns join(resolve("/custom/cache"), hash) as
shown in managedEnvDir.ts line 30. On Windows, resolve() adds a drive prefix
which causes the path mismatch. Update the expectation to use
resolve("/custom/cache") when calling join to match the actual behavior of
managedEnvDir() function and ensure the assertion correctly validates the
implementation across all platforms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ee675fa0-4627-4bb1-896b-23a82ad0c3e8
⛔ Files ignored due to path filters (1)
src/commands/__snapshots__/help.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
src/commands/flows/hybridRunDefaults.tssrc/commands/flows/run.register.tssrc/commands/flows/runDefaults.handle.test.tssrc/commands/flows/runDefaults.tssrc/core/messages/runner.tssrc/domains/runtimeEnv/index.tssrc/domains/runtimeEnv/managedEnvDir.test.tssrc/domains/runtimeEnv/managedEnvDir.ts
Contributor
Author
|
CodeRabbit (@coderabbitai) review |
✅ Action performedReview finished.
|
The managed runtime lives at <data-dir>/runtime/<hash> with no in-CLI way to clear it, forcing a hand-typed `rm -rf` that risks deleting the wrong path. Add `qawolf install clear`: a destructive, confirmed command that removes the whole runtime base dir (honoring QAWOLF_RUNTIME_DIR), with a `--yes` flag and structured json/agent output.
The destructive confirm used Clack's selectKey, which renders the message inline on the prompt line. The embedded newline (path on its own line) broke the framed timeline and garbled the y/n keystroke prompt so it could not be answered. Move the path into a Clack note box and keep the confirm message to a single line, matching the destructive-confirm pattern in init.
The destructive confirm used Clack's selectKey, a single-keystroke hotkey prompt with no up/down navigation: arrows did nothing and Enter cancelled instead of submitting, so the prompt felt frozen. Replace it with Clack's standard arrow-navigable confirm, starting the cursor on No so a stray Enter stays safe. Removes the selectKey path entirely; also fixes the same prompt in init and flows pull.
Wrap the removal in withProgress so the user gets immediate feedback while the directory is deleted, matching auth logout. Drop the path from the final message — the confirm note already shows it (human) and the structured output carries it in `dir` (json/agent).
… pre-bundle Phases 1–3: Implement flow-resolution strategy for managed runtime. - loadFlowDefault: Replace @qawolf/flows bare-import rewriting with Bun.build pre-bundling (compiled binary path) or direct import (Node path). Externalize native browser drivers so they resolve via the bundle root's node_modules symlink instead of being inlined. Remove depsRoot arg (no longer used); injection replaces env search. Delete rewriteFlowImports, findFlowsEnvDir, data: URI/sourceURL. - runWebFlow, runAndroidFlow: Drop depsRoot arg to loadFlowDefault. - New linkManagedDeps: Idempotent symlink <bundleRoot>/node_modules -> <depsRoot>/node_modules; exported from runtimeEnv/index. - New stageFlows: Stage raw in-place projects into .qawolf/.local/<hash> (excluding node_modules/.git/.qawolf), remap flow paths. Symlink never pollutes user project; .qawolf bundles used in place. - New copyDir: copyDirExcluding for entry-by-entry copy so destination may live inside source (avoids cp EINVAL). - runDefaults, hybridRunDefaults: Wire stageFlows + linkManagedDeps (symlink only when runtimeEnv.source !== "project"). Pass staged files to flowsRun. hybridRun.test adds fs to ctx.
Contributor
Author
|
CodeRabbit (@coderabbitai) review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/domains/flows/stageFlows.test.ts`:
- Around line 113-127: The test verifies that the staged file exists after
re-running stageFlows but does not verify that the updated content is actually
picked up. Add an assertion that reads the content of the staged file at
join(second.bundleRoot, "a.flow.ts") and verifies it equals "v2" (the updated
content), not "v1" (the original content), to ensure that edits to the original
flow file are properly reflected in the staged directory.
In `@src/domains/flows/stageFlows.ts`:
- Around line 42-45: The stagedDir variable is derived deterministically from
hashProjectDir, which causes concurrent runs with the same projectDir to collide
and delete each other's staging directories mid-execution. To fix this, modify
the stagedDir construction to include a unique identifier per run (such as a
timestamp or UUID) in addition to the hash, ensuring each concurrent execution
gets its own isolated staging directory that won't be removed by other
simultaneous runs. Alternatively, implement a file-based locking mechanism
around the fs.rm and fs.mkdir operations to serialize access to the same staging
directory across concurrent runs.
In `@src/domains/runtimeEnv/clearRuntimeEnv.ts`:
- Around line 12-14: The clearRuntimeEnv function performs a recursive deletion
on a directory path obtained from managedEnvBaseDir() without verifying it is
actually a managed runtime directory, which could lead to accidental deletion of
unrelated data if the environment override is misconfigured. Add validation
logic before the fs.rm call to verify the directory is a legitimate managed
runtime location (using a sentinel file or layout validation), and skip or fail
the deletion operation if validation is missing or fails. This ensures the
function fails closed and protects against dangerous deletions.
In `@src/domains/runtimeEnv/linkManagedDeps.ts`:
- Line 33: The symlink call in linkManagedDeps.ts is hardcoded to use "dir"
which requires administrator privileges on Windows. Replace the hardcoded "dir"
string with platform-aware logic that checks the current operating system: use
"junction" for Windows (which requires no elevated privileges) and "dir" for all
other platforms. The same conditional logic should also be applied to the
corresponding symlink call in the test file linkManagedDeps.test.ts at line 83
to keep the tests platform-aware and consistent with the production code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c65846db-936b-4cae-9b68-b799bae639b2
⛔ Files ignored due to path filters (1)
src/commands/__snapshots__/help.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (26)
skills/qawolf-cli/SKILL.mdsrc/commands/flows/hybridRun.test.tssrc/commands/flows/hybridRunDefaults.tssrc/commands/flows/runDefaults.tssrc/commands/help.test.tssrc/commands/install/clear.tssrc/commands/install/index.tssrc/core/messages/install.tssrc/domains/flows/stageFlows.test.tssrc/domains/flows/stageFlows.tssrc/domains/runner/loadFlowDefault.test.tssrc/domains/runner/loadFlowDefault.tssrc/domains/runner/runAndroidFlow.tssrc/domains/runner/runWebFlow.tssrc/domains/runtimeEnv/clearRuntimeEnv.test.tssrc/domains/runtimeEnv/clearRuntimeEnv.tssrc/domains/runtimeEnv/index.tssrc/domains/runtimeEnv/linkManagedDeps.test.tssrc/domains/runtimeEnv/linkManagedDeps.tssrc/domains/runtimeEnv/managedEnvDir.tssrc/shell/copyDir.tssrc/shell/ui/clack/styledClack.mock.tssrc/shell/ui/clack/styledClack.tssrc/shell/ui/renderers/confirm.test.tssrc/shell/ui/renderers/confirm.tssrc/shell/ui/types.ts
💤 Files with no reviewable changes (2)
- src/shell/ui/clack/styledClack.mock.ts
- src/domains/runner/runAndroidFlow.ts
Michael Price (michael-pr)
force-pushed
the
wiz-10907-potential-incompatibility-with-other-monorepo-and-single
branch
from
5dcd302 to
80fe8d8
Compare
…ir helpers Address review: stage same-basename flows under a per-dir hash subdir when there is no project root (was silently overwriting); match Windows drive-absolute paths in the executor-externalize onResolve filter; capture the npm spawn error message.
Contributor
Author
|
CodeRabbit (@coderabbitai) review |
✅ Action performedReview finished.
|
Contributor
Author
|
CodeRabbit (@coderabbitai) review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/flows/runStagedFlows.ts`:
- Around line 77-89: Register the staged cleanup immediately after
`deps.prepareRunDir` in `runStagedFlows`, before calling `configureTestkit`,
`createAndroidDeps`, or `runWebFlowDeps`, so `staged.cleanup` is always attached
even if those setup calls throw. Use the existing `staged.cleanup` and
`ctx.signals.register` flow to ensure the per-run staging directory is cleaned
up on early failure.
In `@src/domains/runtimeEnv/npmInstall.ts`:
- Around line 13-17: The npm install process in npmInstall() only drains stderr
while stdout is left unread, which can block the child process if stdout fills
up. Update the spawn handling around nodeSpawn("npm", ["install",
"--legacy-peer-deps"], { cwd }) so that child.stdout is either resumed/consumed
or stdout is ignored in the spawn options, while keeping the existing stderr
capture logic intact.
In `@src/domains/runtimeEnv/shimDeps.ts`:
- Around line 158-170: The shim build failure path in shimDeps should not just
log and return; when build() in the shimming routine cannot produce output,
throw an error so installPinned aborts instead of publishing a runtime with
missing shims. Update the failure handling around result.success/output in
shimDeps to propagate the build failure, and add a test covering that a failed
bun.build stops installPinned from completing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 81d9898b-b057-478c-b9a8-dfe86ab54891
📒 Files selected for processing (15)
src/commands/flows/hybridRun.test.tssrc/commands/flows/hybridRunDefaults.tssrc/commands/flows/runDefaults.handle.test.tssrc/commands/flows/runDefaults.tssrc/commands/flows/runStagedFlows.tssrc/domains/runtimeEnv/innerHop.test.tssrc/domains/runtimeEnv/innerHop.tssrc/domains/runtimeEnv/installPinned.tssrc/domains/runtimeEnv/managedEnvDir.test.tssrc/domains/runtimeEnv/managedEnvDir.tssrc/domains/runtimeEnv/npmInstall.tssrc/domains/runtimeEnv/outerHop.tssrc/domains/runtimeEnv/prepareRunDir.test.tssrc/domains/runtimeEnv/prepareRunDir.tssrc/domains/runtimeEnv/shimDeps.ts
Contributor
Author
|
Final results from testing locally: |
Chase J (chajac)
approved these changes
Jun 26, 2026
Chase J (chajac)
left a comment
Chase J (chajac)
left a comment
Contributor
There was a problem hiding this comment.
Only some small curiosities from me, looks great!
Michael Price (michael-pr)
deleted the
wiz-10907-potential-incompatibility-with-other-monorepo-and-single
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to WIZ-10907
Resolves WIZ-10849
Overview of Changes
Running
qawolf flows runcould fail with "Could not find Playwright", and more broadly, users don't want the CLI installing its dependencies into their own project. The original cause: the CLI installed its runtime (Playwright, etc.) into the user's project, and in workspace monorepos (pnpm/yarn/npm) those installs hoist to the repo root — leaving the package's ownnode_modulesempty, so resolution failed.This PR moves the runtime into an isolated location the CLI owns and runs each flow from a per-run, layered staging directory, so the user's project is never written to and a flow's own dependencies still resolve correctly.
How it works
<data-dir>/runtime/<version-hash>): the pinned executor (@qawolf/flows,playwright,@qawolf/emails,@qawolf/testkit, appium×3) installed once on demand. The hash is channel-keyed (Node vs compiled-binary) so the two runtimes can never corrupt each other.…/runtime-runs/): the flow tree is staged intoexec/, with two resolution hops:exec/node_modules) exposes only the 7 pinned packages (one symlink each into the managed runtime). The executor stays authoritative for its own packages, but a flow's ownimport <dep>is not shadowed by the executor's transitive copy.<run-dir>/node_modules) carries the flow's own dependencies — symlinked from the nearest realnode_modules(monorepo/workspace) or installed from the flow'spackage.json(executor packages stripped).BUN_BE_BUN), so flows resolve their ownnode_modules— including native modules likesharp— exactly as the Node channel does. (This replaces an earlier bundle-the-flow approach that could not load native.nodeaddons.)QAWOLF_RUNTIME_DIRrelocates the runtime (CI / read-only home);--deps <dir>supplies your own prepared dir.Caused by:chain) in both console and JUnit output.Out of scope (follow-up): the outer-hop dependency discovery can walk up past the project boundary, so a flow project nested inside another repo with conflicting
node_modulesmay pick up the wrong dependency version — fix tracked separately.Testing
node_modules), runqawolf flows run <pattern>— the flow passes, a note confirms the managed runtime is in use, and nothing is written into the repo.diff,sharp,@faker-js/faker) runs on both the Node CLI and the compiled binary — dependencies (including native modules) resolve from the flow's own install.qawolf flows run --deps <prepared-dir> <pattern>— uses the given dir, installs nothing.QAWOLF_RUNTIME_DIR=<dir> qawolf flows run <pattern>— installs/uses the runtime under that dir.qawolf install clear— succeeds after a flow run (managed base holds only runtime dirs).qawolf doctor— reports the managed runtime location without installing.Checklist
🤖 Generated with Claude Code