Skip to content

fix(nx): fall back to workspace cwd when run-commands target has no cwd#1638

Merged
webpro merged 3 commits into
webpro-nl:mainfrom
liorp:fix/nx-plugin-cwd-fallback
Mar 24, 2026
Merged

fix(nx): fall back to workspace cwd when run-commands target has no cwd#1638
webpro merged 3 commits into
webpro-nl:mainfrom
liorp:fix/nx-plugin-cwd-fallback

Conversation

@liorp

@liorp liorp commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1637

When an nx:run-commands target does not specify options.cwd (which is the common case), the Nx plugin passes cwd: undefined to getInputsFromScripts. Because the spread in WorkspaceWorker ({ ...baseOptions, ...options }) includes undefined-valued properties, this overrides the valid baseOptions.cwd, causing a crash in the bash parser's error handler — path.relative(undefined, ...) throws ERR_INVALID_ARG_TYPE.

Root cause

// packages/knip/src/plugins/nx/index.ts:68
const cwd = target.options?.cwd ? join(options.cwd, target.options.cwd) : undefined;
//                                                                        ^^^^^^^^^ overrides baseOptions.cwd

Fix

Fall back to options.cwd (the workspace root) instead of undefined:

- const cwd = target.options?.cwd ? join(options.cwd, target.options.cwd) : undefined;
+ const cwd = target.options?.cwd ? join(options.cwd, target.options.cwd) : options.cwd;

Tests

Added test/plugins/nx-run-commands-no-cwd.test.ts with two test cases:

  1. No cwd on target — verifies getInputsFromScripts receives the workspace cwd (not undefined)
  2. Explicit cwd on target — verifies cwd is properly joined with the workspace root

Both tests are verified to fail without the fix and pass with it. Existing Nx plugin tests continue to pass.

@webpro

webpro commented Mar 24, 2026

Copy link
Copy Markdown
Member

Why add a different type of test? Please see other (Nx) tests. Running main() is almost entirely end-to-end. Please keep it concise and consistent if at all possible.

@liorp liorp force-pushed the fix/nx-plugin-cwd-fallback branch from 74e6cb1 to b686101 Compare March 24, 2026 15:26
@liorp

liorp commented Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

Removed the redundant serve target from the fixture — the existing fixture already has multiple nx:run-commands targets without options.cwd (webpack, ls-project-root, generate-docs, lint), so adding another one didn't exercise any new code path.

The fix itself is a one-line defensive correctness change: cwd should never be undefined when passed to getInputsFromScripts, since it gets spread over baseOptions in WorkspaceWorker and overrides the valid cwd. The specific crash (in the bash parser's error handler via path.relative(undefined, ...)) is hard to reproduce in an e2e test because unbash rarely throws parse errors, but the fix ensures correct behavior regardless of downstream code paths.

@webpro

webpro commented Mar 24, 2026

Copy link
Copy Markdown
Member

You're right, let's just roll with it 🛼

@pkg-pr-new

pkg-pr-new Bot commented Mar 24, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/knip@1638

npm i https://pkg.pr.new/@knip/language-server@1638

npm i https://pkg.pr.new/@knip/mcp@1638

commit: fe1124a

Lior Pollak and others added 3 commits March 24, 2026 21:33
@liorp
fix(nx): fall back to workspace cwd when run-commands target has no cwd
5a7d5b1 
When an nx:run-commands target does not specify options.cwd (which is the
common case), the Nx plugin was passing `cwd: undefined` to
getInputsFromScripts. Because the spread in WorkspaceWorker
(`{ ...baseOptions, ...options }`) includes undefined-valued properties,
this overrides the valid baseOptions.cwd, causing a crash in the bash
parser's error handler: `path.relative(undefined, ...)` throws
ERR_INVALID_ARG_TYPE.

Fall back to `options.cwd` (the workspace root) instead of `undefined`.

Fixes webpro-nl#1637
@claude @liorp
test(nx): replace unit test with e2e fixture coverage
2369851 
Remove separate unit test file. Instead, add a `bun run` command in
an nx:run-commands target without explicit cwd to the existing fixture.
This exercises the fix end-to-end — crashes without it, passes with it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@liorp
remove redundant serve target from fixture
fc24af6 
The existing fixture already has multiple nx:run-commands targets
without options.cwd (webpack, ls-project-root, generate-docs, lint).
Adding another one doesn't exercise any new code path.
@liorp liorp force-pushed the fix/nx-plugin-cwd-fallback branch from fe1124a to fc24af6 Compare March 24, 2026 19:33
@liorp

liorp commented Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. Waiting on your approval 🙏

@webpro webpro merged commit cf2dc7d into webpro-nl:main Mar 24, 2026
@webpro

webpro commented Mar 24, 2026

Copy link
Copy Markdown
Member

🚀 This pull request is included in v6.0.5. See Release 6.0.5 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

@webpro

webpro commented Mar 24, 2026

Copy link
Copy Markdown
Member

Thanks Lior!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Nx plugin crashes when nx:run-commands target has no options.cwd

2 participants

@liorp @webpro