feat: walk up directory tree to find JS lockfile in monorepos

[ruromero]

Summary

• Add parent-traversal lockfile discovery to Base_javascript, matching the existing Cargo provider pattern
• When analyzing a workspace member's package.json, the provider now walks up the directory tree to find the lock file at the workspace root
• Stops at a package.json with a "workspaces" field (workspace root boundary) — analogous to Cargo's [workspace] boundary
• TRUSTIFY_DA_WORKSPACE_DIR overrides the walk-up (checks only that directory)
• Fixed #executeListCmd and #createLockFile to pass { cwd: manifestDir } to #invokeCommand
• 6 new test cases and test fixtures for workspace member scenarios

Closes #430
Related Jira: TC-3891

Test plan

• Existing npm/pnpm/yarn provider tests pass
• New tests: workspace member walks up and finds lock file
• New tests: workspace member throws when root has no lock file
• New tests: TRUSTIFY_DA_WORKSPACE_DIR override works correctly
• New tests: wrong TRUSTIFY_DA_WORKSPACE_DIR fails even when walk-up would succeed
• Tested locally with VS Code extension batch analysis on a monorepo

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Add monorepo lockfile discovery via directory tree walk-up

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Add parent-traversal lockfile discovery to JavaScript provider for monorepos
  - Walks up directory tree to find lock file at workspace root
  - Stops at package.json with "workspaces" field (workspace root boundary)
  - TRUSTIFY_DA_WORKSPACE_DIR env var overrides walk-up behavior
• Extract lockfile search logic into new _findLockFileDir() method
• Add 4 new test cases with workspace member fixtures
  - Workspace member with lock file at root (walk-up succeeds)
  - Workspace member without lock file at root (walk-up fails)
  - Override with correct TRUSTIFY_DA_WORKSPACE_DIR (succeeds)
  - Override with wrong TRUSTIFY_DA_WORKSPACE_DIR (fails)

Diagram

flowchart LR
  A["Nested package.json<br/>in monorepo"] -->|"_findLockFileDir()"| B["Walk up directory tree"]
  B -->|"Lock file found"| C["Return lock file dir"]
  B -->|"Workspace root found<br/>no lock file"| D["Return null"]
  B -->|"Filesystem root reached"| D
  E["TRUSTIFY_DA_WORKSPACE_DIR<br/>env var set"] -->|"Override"| F["Check only that dir"]
  F -->|"Lock file exists"| C
  F -->|"Lock file missing"| D

Loading

File Changes

1. src/providers/base_javascript.js ✨ Enhancement +57/-7

Implement directory tree walk-up for lockfile discovery

• Add new _findLockFileDir() method that walks up directory tree to find lock file
• Respects workspace root boundary (stops at package.json with "workspaces" field)
• Honors TRUSTIFY_DA_WORKSPACE_DIR override to check only specified directory
• Refactor validateLockFile() to use new _findLockFileDir() method
• Refactor _buildDependencyTree() to use _findLockFileDir() for command execution directory

src/providers/base_javascript.js

---

2. test/providers/javascript.test.js 🧪 Tests +30/-0

Add workspace member lockfile discovery test cases

• Add 2 new test cases for workspace member validation (with and without lock file)
• Add test case verifying workspace member walks up and finds lock file at root
• Add test case verifying workspace member throws when root has no lock file
• Add test case verifying TRUSTIFY_DA_WORKSPACE_DIR override works for workspace members
• Add test case verifying wrong TRUSTIFY_DA_WORKSPACE_DIR fails even when walk-up would succeed

test/providers/javascript.test.js

---

3. test/providers/provider_manifests/npm/workspace_member_with_lock/package.json 🧪 Tests +5/-0

Add workspace root fixture with lock file

• Create workspace root package.json with "workspaces" field pointing to packages/*
• Serves as test fixture for monorepo with lock file at workspace root

test/providers/provider_manifests/npm/workspace_member_with_lock/package.json

---

View more (3)

4. test/providers/provider_manifests/npm/workspace_member_with_lock/packages/module-a/package.json 🧪 Tests +4/-0

Add workspace member fixture without lock

• Create nested workspace member package.json without lock file
• Serves as test fixture for module that should walk up to find root lock file

test/providers/provider_manifests/npm/workspace_member_with_lock/packages/module-a/package.json

---

5. test/providers/provider_manifests/npm/workspace_member_without_lock/package.json 🧪 Tests +5/-0

Add workspace root fixture without lock file

• Create workspace root package.json with "workspaces" field but no lock file
• Serves as test fixture for monorepo without lock file at workspace root

test/providers/provider_manifests/npm/workspace_member_without_lock/package.json

---

6. test/providers/provider_manifests/npm/workspace_member_without_lock/packages/module-a/package.json 🧪 Tests +4/-0

Add workspace member fixture for failure case

• Create nested workspace member package.json in fixture without lock file
• Serves as test fixture for module that should fail when root has no lock file

test/providers/provider_manifests/npm/workspace_member_without_lock/packages/module-a/package.json

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Workspace SBOM root mismatch 🐞 Bug ✓ Correctness

Description

When a workspace member package.json is analyzed, _buildDependencyTree now runs lockfile
creation and dependency listing from the discovered lockfile directory (often the workspace root),
but SBOM root identity is still taken from the member manifest. This can generate an SBOM whose root
component (member) does not match the dependency tree (workspace root), producing incorrect results.

Code

src/providers/base_javascript.js[1]

_buildDependencyTree(includeTransitive, opts = {}) {

Evidence

_buildDependencyTree resolves cmdDir to the directory containing the lockfile and runs both
lockfile update and list command from there; however #getSBOM always uses
this.#manifest.name/version (the provided manifest path) as the SBOM root. For npm/pnpm,
_listCmdArgs do not target a specific workspace member, so running from cmdDir (workspace root)
will describe the workspace/root dependency view rather than the member, causing a mismatch between
root component and dependencies.

src/providers/base_javascript.js[230-259]
src/providers/javascript_npm.js[1-20]
src/providers/javascript_pnpm.js[1-27]
src/provider.js[58-68]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
For workspace members, commands now run from the lockfile directory (often workspace root), but SBOM root is still derived from the member manifest. This produces SBOMs whose root component does not match the dependency graph.
### Issue Context
- `cmdDir` is set to the directory containing the lockfile.
- `#getSBOM()` uses `this.#manifest` (the provided manifest path) as root component.
- npm/pnpm list args don’t constrain output to a specific workspace member.
### Fix Focus Areas
- src/providers/base_javascript.js[230-259]
- src/providers/javascript_npm.js[13-19]
- src/providers/javascript_pnpm.js[13-19]
- src/providers/javascript_yarn.js[19-25]
### Suggested approach
Pick one (or combine):
1) If `cmdDir !== manifestDir`, adjust list commands to target the member (e.g., pass `--prefix <memberDir>` or workspace flags where supported) so the dependency tree corresponds to the member.
2) Alternatively, if running at workspace root, change SBOM root component to the workspace root package (but this changes semantics for member analysis).
3) Ensure tests assert the produced SBOM corresponds to the intended root (member vs workspace).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Wrong command working dir 🐞 Bug ✓ Correctness

Description

_buildDependencyTree() computes cmdDir (lockfile/workspace-root directory) but the npm/pnpm/yarn
commands still run with opts.cwd defaulting to the manifest’s directory, so lockfile-root
execution is not guaranteed for workspace members on non-Windows.

Code

src/providers/base_javascript.js[R239-245]

_buildDependencyTree(includeTransitive, opts = {}) {
this._version();
const manifestDir = path.dirname(this.#manifest.manifestPath);
-		const workspaceDir = getCustom('TRUSTIFY_DA_WORKSPACE_DIR', null, opts)
-		const cmdDir = workspaceDir ? path.resolve(workspaceDir) : manifestDir;
+		const cmdDir = this._findLockFileDir(manifestDir, opts) || manifestDir;
this.#createLockFile(cmdDir);
let output = this.#executeListCmd(includeTransitive, cmdDir);

Evidence

Although _buildDependencyTree() now passes cmdDir into #createLockFile(cmdDir) and
#executeListCmd(..., cmdDir), neither method passes a cwd to #invokeCommand. #invokeCommand
sets opts.cwd to path.dirname(this.#manifest.manifestPath) when not provided, which is the
workspace member directory (manifestDir), not cmdDir. Additionally, npm/pnpm/yarn _listCmdArgs
implementations ignore the passed directory argument, so cmdDir does not affect arguments either.
Therefore the new lockfile discovery influences provider matching, but not reliably the directory
where commands execute (except partial Windows behavior via process.chdir in #createLockFile).

src/providers/base_javascript.js[239-248]
src/providers/base_javascript.js[361-364]
src/providers/base_javascript.js[408-413]
src/providers/javascript_npm.js[13-19]
src/providers/javascript_pnpm.js[13-19]
src/providers/processors/yarn_classic_processor.js[17-27]
src/providers/processors/yarn_berry_processor.js[22-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Base_javascript._buildDependencyTree()` now finds a lockfile directory (`cmdDir`) for monorepos, but the actual package-manager commands still execute with `cwd` defaulting to the manifest directory (`package.json` location). This undermines the new walk-up behavior for workspace members.
### Issue Context
- `#invokeCommand()` defaults `opts.cwd` to the manifest directory when not provided.
- `#executeListCmd()` and `#createLockFile()` call `#invokeCommand()` without a `cwd`.
### Fix Focus Areas
- Ensure `#executeListCmd()` passes `{ cwd: cmdDir }` (or the passed `manifestDir` parameter) into `#invokeCommand`.
- Ensure `#createLockFile()` passes `{ cwd: cmdDir }` into `#invokeCommand` on all platforms (and consider removing/avoiding `process.chdir` if `cwd` is sufficient).
- src/providers/base_javascript.js[239-246]
- src/providers/base_javascript.js[361-364]
- src/providers/base_javascript.js[380-397]
- src/providers/base_javascript.js[408-413]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. pnpm workspace root missed ☑ 🐞 Bug ✓ Correctness

Description

_findLockFileDir() stops only at a package.json containing workspaces, so pnpm workspaces
defined via pnpm-workspace.yaml have no boundary and lockfile discovery can walk above the actual
workspace root and match an unrelated ancestor lockfile.

Code

src/providers/base_javascript.js[R144-160]

+			// If this directory has a package.json with "workspaces", the lock
+			// file should have been here — stop searching (analogous to Cargo's
+			// [workspace] boundary).
+			const pkgJsonPath = path.join(dir, 'package.json')
+			if (fs.existsSync(pkgJsonPath)) {
+				try {
+					const content = JSON.parse(fs.readFileSync(pkgJsonPath, 'utf-8'))
+					if (content.workspaces) {
+						return null
+					}
+				} catch (_) {
+					// ignore parse errors, keep searching
+				}
+			}
+
+			parent = path.dirname(dir)
+		} while (parent !== dir)

Evidence

The walk-up boundary is implemented by reading package.json and checking content.workspaces.
However, the repository’s JS workspace discovery explicitly supports pnpm workspaces via
pnpm-workspace.yaml, meaning a workspace root may not have package.json#workspaces. In such
cases, the new walk-up search has no workspace-root boundary and can continue past the intended
root.

src/providers/base_javascript.js[144-157]
src/workspace.js[119-140]
src/workspace.js[131-139]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_findLockFileDir()` uses `package.json#workspaces` as the only workspace-root boundary. This misses pnpm workspaces configured by `pnpm-workspace.yaml`, allowing the lockfile search to escape the workspace root.
### Issue Context
`discoverWorkspacePackages()` already treats `pnpm-workspace.yaml` as a valid workspace root signal.
### Fix Focus Areas
- In `_findLockFileDir()`, add a boundary condition similar to the `workspaces` check when `pnpm-workspace.yaml` exists in the current directory:
- If `pnpm-workspace.yaml` is present and the lockfile for the current provider is not present, stop searching and return `null`.
- src/providers/base_javascript.js[127-163]
- src/workspace.js[119-140]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Ambiguous provider selection 🐞 Bug ✓ Correctness

Description

Because match() selects the first JS provider whose validateLockFile() succeeds, the new walk-up
behavior can cause the wrong package-manager provider to be chosen when different lockfiles exist at
different ancestor levels.

Code

src/providers/base_javascript.js[R176-178]

validateLockFile(manifestDir, opts = {}) {
-		const workspaceDir = getCustom('TRUSTIFY_DA_WORKSPACE_DIR', null, opts)
-		const dirToCheck = workspaceDir ? path.resolve(workspaceDir) : manifestDir
-		const lock = path.join(dirToCheck, this._lockFileName())
-		return fs.existsSync(lock)
+		return this._findLockFileDir(manifestDir, opts) !== null
}

Evidence

match() filters providers by manifest type and then chooses the first provider for which
validateLockFile(manifestDir, opts) returns true. With the new walk-up logic, a provider can
return true based on an ancestor lockfile unrelated to the closest/expected lockfile for that
package, and JS providers are tried in a fixed order (pnpm, yarn, npm). This increases the chance of
selecting a different provider than the one implied by a nearer lockfile.

src/provider.js[20-29]
src/provider.js[58-67]
src/providers/base_javascript.js[176-178]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Provider selection can become order-dependent with walk-up lockfile discovery: the first provider that finds *any* ancestor lockfile wins, even if a different lockfile exists closer to the manifest.
### Issue Context
`availableProviders` orders JS providers as pnpm → yarn → npm, and `match()` uses `find()` across the supported providers.
### Fix Focus Areas
- Adjust JS lockfile validation/selection so that when multiple lockfiles exist at different levels, the provider corresponding to the *nearest* lockfile directory (or the manifest directory) is chosen.
- Alternatively, compute a single nearest lockfile directory and then select provider based on lockfile(s) present there.
- src/provider.js[58-68]
- src/providers/base_javascript.js[127-178]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Redundant Windows chdir ☑ 🐞 Bug ⚙ Maintainability

Description

#createLockFile still calls process.chdir(manifestDir) on Windows even though #invokeCommand
is now called with { cwd: manifestDir }. This global directory change is unnecessary and increases
maintenance risk.

Code

src/providers/base_javascript.js[1]

*/

Evidence

The PR updated lockfile/list command invocation to pass cwd, which already controls the spawned
process working directory via invokeCommand(..., opts). Keeping process.chdir adds global side
effects without benefit.

src/providers/base_javascript.js[371-387]
src/providers/base_javascript.js[397-448]
src/tools.js[146-173]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
On Windows, `#createLockFile` still changes the process working directory even though commands are invoked with `cwd`.
### Issue Context
`invokeCommand` already passes `opts` to `execFileSync` which honors `cwd`.
### Fix Focus Areas
- src/providers/base_javascript.js[371-387]
### Suggested approach
Remove the `process.chdir` / restore block and rely solely on `{ cwd: manifestDir }` for both Windows and non-Windows.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[ruromero]

Verification Report for TC-3891

Check	Result	Details
Scope Containment	✅ PASS	All 7 changed files match the task's Changes section
Diff Size	✅ PASS	+105/−14 across 7 files — proportional to scope
Commit Traceability	❌ FAIL	Neither commit references TC-3891 (task was created after the commits)
Sensitive Patterns	✅ PASS	No sensitive patterns detected
CI Status	✅ PASS	All 4 checks pass (lint+test Node 22/24, PR title, commit messages)
Acceptance Criteria	✅ PASS	5 of 5 criteria met
Verification Commands	N/A	No verification commands in task

Acceptance Criteria Detail

#	Criterion	Result
1	Walk-up finds lock file at workspace root from nested module directory	✅ PASS
2	Stops at workspace boundary (package.json with "workspaces" field)	✅ PASS
3	TRUSTIFY_DA_WORKSPACE_DIR overrides the walk-up	✅ PASS
4	Commands run from the correct directory (cwd passed to #invokeCommand)	✅ PASS
5	All existing tests pass with no regressions	✅ PASS

Overall: ⚠️ WARN

Commit traceability: Neither commit references TC-3891 because the Jira task was created after the commits were authored. Consider amending or adding a follow-up commit with the issue reference.

---

This comment was AI-generated by sdlc-workflow/verify-pr v0.5.0.

[ruromero]

/review

[qodo-code-review]

Persistent review updated to latest commit 5dc6216

[ruromero]

Qodo Review Response

Addressed all 5 findings:

1. Workspace SBOM root mismatch — Not a regression. This is the same behavior as the previous TRUSTIFY_DA_WORKSPACE_DIR code path. The walk-up only runs for single-file analysis (e.g. opening a module's package.json in the IDE), where npm/pnpm resolve deps from the root lockfile correctly. In batch mode, TRUSTIFY_DA_WORKSPACE_DIR is set explicitly and the walk-up is bypassed. No change needed.

2. Wrong command working dir — Fixed in the second commit: #executeListCmd and #createLockFile now pass { cwd: manifestDir } to #invokeCommand.

3. pnpm workspace root missed — Fixed in 5c95677. Added _isWorkspaceRoot() that checks both pnpm-workspace.yaml and package.json#workspaces. Added test fixture and test case.

4. Ambiguous provider selection — This requires multiple different lockfiles at different ancestor levels (e.g. yarn.lock at level 1 and package-lock.json at level 2). In practice, workspaces use a single package manager. This is a pre-existing design in match() and not a regression from this PR. No change needed.

5. Redundant Windows chdir — Valid observation, but the process.chdir was added specifically for a known Windows npm bug where --prefix is ignored. Removing it without Windows testing would be risky. Out of scope for this PR.