Skip to content

Commit 4689ffb

Browse files
ruromeroclaude
andauthored
feat: walk up directory tree to find JS lockfile in monorepos (#431)
* feat: walk up directory tree to find JS lockfile in monorepos Add parent-traversal logic to Base_javascript.validateLockFile and _buildDependencyTree, matching the existing Cargo provider pattern. When a nested package.json has no lockfile in its directory, the provider now walks up looking for package-lock.json/yarn.lock/ pnpm-lock.yaml, stopping at a package.json with "workspaces" field (the workspace root boundary) or the filesystem root. This enables single-file stack analysis to work for modules in JS/TS monorepos where the lockfile lives at the workspace root. TRUSTIFY_DA_WORKSPACE_DIR still takes precedence as an explicit override (no walk-up when set). Assisted-by: Claude Code Ref: TC-3891 * fix: pass correct cwd to commands and trim unnecessary comments Address Qodo review: #executeListCmd and #createLockFile now pass { cwd: manifestDir } to #invokeCommand so commands run from the lockfile directory, not the manifest directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Ref: TC-3891 * fix: add pnpm-workspace.yaml as workspace root boundary Address Qodo review: _findLockFileDir now also stops at directories containing pnpm-workspace.yaml, preventing the walk-up from escaping pnpm workspace roots that don't use package.json#workspaces. Ref: TC-3891 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7144952 commit 4689ffb

File tree

10 files changed

+131
-14
lines changed

10 files changed

+131
-14
lines changed

src/providers/base_javascript.js

Lines changed: 63 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -112,18 +112,70 @@ export default class Base_javascript {
112112
}
113113

114114
/**
115-
* Checks if a required lock file exists in the manifest directory or at the workspace root.
116-
* When TRUSTIFY_DA_WORKSPACE_DIR is provided (via env var or opts),
117-
* checks only that directory for the lock file.
115+
* Walks up the directory tree from manifestDir looking for the lock file.
116+
* Stops when the lock file is found, when a package.json with a "workspaces"
117+
* field is encountered without a lock file (workspace root boundary), or
118+
* when the filesystem root is reached.
119+
*
120+
* When TRUSTIFY_DA_WORKSPACE_DIR is set, checks only that directory (no walk-up).
121+
*
122+
* @param {string} manifestDir - The directory to start searching from
123+
* @param {Object} [opts={}] - optional; may contain TRUSTIFY_DA_WORKSPACE_DIR
124+
* @returns {string|null} The directory containing the lock file, or null
125+
* @protected
126+
*/
127+
_isWorkspaceRoot(dir) {
128+
if (fs.existsSync(path.join(dir, 'pnpm-workspace.yaml'))) {
129+
return true
130+
}
131+
const pkgJsonPath = path.join(dir, 'package.json')
132+
if (fs.existsSync(pkgJsonPath)) {
133+
try {
134+
const content = JSON.parse(fs.readFileSync(pkgJsonPath, 'utf-8'))
135+
if (content.workspaces) {
136+
return true
137+
}
138+
} catch (_) {
139+
// ignore parse errors
140+
}
141+
}
142+
return false
143+
}
144+
145+
_findLockFileDir(manifestDir, opts = {}) {
146+
const workspaceDir = getCustom('TRUSTIFY_DA_WORKSPACE_DIR', null, opts)
147+
if (workspaceDir) {
148+
const dir = path.resolve(workspaceDir)
149+
return fs.existsSync(path.join(dir, this._lockFileName())) ? dir : null
150+
}
151+
152+
let dir = path.resolve(manifestDir)
153+
let parent = dir
154+
155+
do {
156+
dir = parent
157+
158+
if (fs.existsSync(path.join(dir, this._lockFileName()))) {
159+
return dir
160+
}
161+
162+
if (this._isWorkspaceRoot(dir)) {
163+
return null
164+
}
165+
166+
parent = path.dirname(dir)
167+
} while (parent !== dir)
168+
169+
return null
170+
}
171+
172+
/**
118173
* @param {string} manifestDir - The base directory where the manifest is located
119-
* @param {{TRUSTIFY_DA_WORKSPACE_DIR?: string}} [opts={}] - optional workspace root
174+
* @param {Object} [opts={}] - optional; may contain TRUSTIFY_DA_WORKSPACE_DIR
120175
* @returns {boolean} True if the lock file exists
121176
*/
122177
validateLockFile(manifestDir, opts = {}) {
123-
const workspaceDir = getCustom('TRUSTIFY_DA_WORKSPACE_DIR', null, opts)
124-
const dirToCheck = workspaceDir ? path.resolve(workspaceDir) : manifestDir
125-
const lock = path.join(dirToCheck, this._lockFileName())
126-
return fs.existsSync(lock)
178+
return this._findLockFileDir(manifestDir, opts) !== null
127179
}
128180

129181
/**
@@ -188,8 +240,7 @@ export default class Base_javascript {
188240
_buildDependencyTree(includeTransitive, opts = {}) {
189241
this._version();
190242
const manifestDir = path.dirname(this.#manifest.manifestPath);
191-
const workspaceDir = getCustom('TRUSTIFY_DA_WORKSPACE_DIR', null, opts)
192-
const cmdDir = workspaceDir ? path.resolve(workspaceDir) : manifestDir;
243+
const cmdDir = this._findLockFileDir(manifestDir, opts) || manifestDir;
193244
this.#createLockFile(cmdDir);
194245

195246
let output = this.#executeListCmd(includeTransitive, cmdDir);
@@ -310,7 +361,7 @@ export default class Base_javascript {
310361
*/
311362
#executeListCmd(includeTransitive, manifestDir) {
312363
const listArgs = this._listCmdArgs(includeTransitive, manifestDir);
313-
return this.#invokeCommand(listArgs);
364+
return this.#invokeCommand(listArgs, { cwd: manifestDir });
314365
}
315366

316367
/**
@@ -332,14 +383,12 @@ export default class Base_javascript {
332383
const isWindows = os.platform() === 'win32';
333384

334385
if (isWindows) {
335-
// On Windows, --prefix flag doesn't work as expected
336-
// Instead of installing from the prefix folder, it installs from current working directory
337386
process.chdir(manifestDir);
338387
}
339388

340389
try {
341390
const args = this._updateLockFileCmdArgs(manifestDir);
342-
this.#invokeCommand(args);
391+
this.#invokeCommand(args, { cwd: manifestDir });
343392
} finally {
344393
if (isWindows) {
345394
process.chdir(originalDir);

test/providers/javascript.test.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ suite('testing the javascript-npm data provider', async () => {
5353
[
5454
{ name: 'npm/with_lock_file', validation: true },
5555
{ name: 'npm/without_lock_file', validation: false },
56+
{ name: 'npm/workspace_member_with_lock/packages/module-a', validation: true },
57+
{ name: 'npm/workspace_member_without_lock/packages/module-a', validation: false },
5658
{ name: 'pnpm/with_lock_file', validation: true },
5759
{ name: 'pnpm/without_lock_file', validation: false },
5860
{ name: 'yarn-classic/with_lock_file', validation: true },
@@ -168,4 +170,38 @@ suite('testing the javascript-npm data provider', async () => {
168170
expect(provider.isSupported('package.json')).to.be.true
169171
})
170172

173+
test('verify workspace member walks up and finds lock file at workspace root', () => {
174+
const manifest = 'test/providers/provider_manifests/npm/workspace_member_with_lock/packages/module-a/package.json'
175+
const provider = match(manifest, availableProviders)
176+
expect(provider).to.not.be.null
177+
expect(provider.isSupported('package.json')).to.be.true
178+
})
179+
180+
test('verify workspace member throws when workspace root has no lock file', () => {
181+
const manifest = 'test/providers/provider_manifests/npm/workspace_member_without_lock/packages/module-a/package.json'
182+
expect(() => match(manifest, availableProviders))
183+
.to.throw('package.json requires a lock file')
184+
})
185+
186+
test('verify match with opts.TRUSTIFY_DA_WORKSPACE_DIR overrides walk-up for workspace member', () => {
187+
const manifest = 'test/providers/provider_manifests/npm/workspace_member_with_lock/packages/module-a/package.json'
188+
const opts = { TRUSTIFY_DA_WORKSPACE_DIR: 'test/providers/provider_manifests/npm/workspace_member_with_lock' }
189+
const provider = match(manifest, availableProviders, opts)
190+
expect(provider).to.not.be.null
191+
expect(provider.isSupported('package.json')).to.be.true
192+
})
193+
194+
test('verify pnpm workspace member stops at pnpm-workspace.yaml boundary', () => {
195+
const manifest = 'test/providers/provider_manifests/pnpm/workspace_member_without_lock/packages/module-a/package.json'
196+
expect(() => match(manifest, availableProviders))
197+
.to.throw('package.json requires a lock file')
198+
})
199+
200+
test('verify match with wrong TRUSTIFY_DA_WORKSPACE_DIR fails even when walk-up would succeed', () => {
201+
const manifest = 'test/providers/provider_manifests/npm/workspace_member_with_lock/packages/module-a/package.json'
202+
const opts = { TRUSTIFY_DA_WORKSPACE_DIR: 'test/providers/provider_manifests/npm/workspace_member_without_lock' }
203+
expect(() => match(manifest, availableProviders, opts))
204+
.to.throw('package.json requires a lock file')
205+
})
206+
171207
}).beforeAll(() => clock = useFakeTimers(new Date('2023-08-07T00:00:00.000Z'))).afterAll(() => clock.restore());

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

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "test-workspace-root",
3+
"private": true,
4+
"workspaces": ["packages/*"]
5+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "module-a",
3+
"version": "1.0.0"
4+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "test-workspace-root",
3+
"private": true,
4+
"workspaces": ["packages/*"]
5+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "module-a",
3+
"version": "1.0.0"
4+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "test-pnpm-workspace-root",
3+
"private": true
4+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "module-a",
3+
"version": "1.0.0"
4+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
packages:
2+
- 'packages/*'

0 commit comments

Comments
 (0)