Summary

Fix Pipeline settings path validation for external node module paths.

Review Request #14816 — Created Feb. 12, 2026 and updated Feb. 12, 2026, 9:03 a.m.

Information

Owner

maubin

Repository

Djblets

Branch

release-5.x

Bugs

Depends On

Reviewers

Groups

djblets

People

Description

We recently changed the way that we check whether babel, lessc, rollup
and uglifyjs binaries exist in the node modules path when building
Pipeline settings. We used to hard-code paths like
<node_modules_path>/.bin/rollup and check for their existence, but now
we use the npm exec command.

However, we invoke the command from the current working directory, when we
should actually be invoking it from the parent directory of the node
modules path.

This was made evident when trying to run Power Pack's unit tests. Power Pack
relies on Review Board's node modules, but since the npm exec command was
ran from the Power Pack repository, it was looking in Power Pack's node
modules instead of Review Board's.

This change fixes this so that the binaries are actually checked for in
the node modules paths, instead of the current working directory. If a
binary isn't found in any of the configured node module paths then we
raise the error.

Testing Done


Ran unit tests.
Was able to run Power Pack unit tests without hitting the

ImproperlyConfigured error for rollup that I was hitting before.

Commits

Summary	ID
Fix Pipeline settings path validation for external node module paths. We recently changed the way that we check whether `babel`, `lessc`, `rollup` and `uglifyjs` binaries exist in the node modules path when building Pipeline settings. We used to hard-code paths like `<node_modules_path>/.bin/rollup` and check for their existence, but now we use the `npm exec` command. However, we invoke the command from the current working directory, when we should actually be invoking it from the parent directory of the node modules path. This was made evident when trying to run Power Pack's unit tests. Power Pack relies on Review Board's node modules, but since the `npm exec` command was ran from the Power Pack repository, it was looking in Power Pack's node modules instead of Review Board's. This change fixes this so that the binaries are actually checked for in the node modules paths, instead of the current working directory. If a binary isn't found in any of the configured node module paths then we raise the error.	fb589232b6e1785f61520dcfb59844fb9a73e2ba

Summary

Fix Pipeline settings path validation for external node module paths.

We recently changed the way that we check whether `babel`, `lessc`, `rollup` and `uglifyjs` binaries exist in the node modules path when building Pipeline settings. We used to hard-code paths like `<node_modules_path>/.bin/rollup` and check for their existence, but now we use the `npm exec` command. However, we invoke the command from the current working directory, when we should actually be invoking it from the parent directory of the node modules path. This was made evident when trying to run Power Pack's unit tests. Power Pack relies on Review Board's node modules, but since the `npm exec` command was ran from the Power Pack repository, it was looking in Power Pack's node modules instead of Review Board's. This change fixes this so that the binaries are actually checked for in the node modules paths, instead of the current working directory. If a binary isn't found in any of the configured node module paths then we raise the error.

fb589232b6e1785f61520dcfb59844fb9a73e2ba

Issues

Description	From	Last Updated
Can we call this node_modules_paths?	david	Feb. 12, 2026, 9:03 a.m.
I tested here manually with a missing package and it prompted to install it. I checked the docs to see …	chipx86	Feb. 12, 2026, 1:55 a.m.
If any of the node_modules paths in the list lack this, will we get an error for each one until …	chipx86	Feb. 12, 2026, 1:55 a.m.
I wonder if we should now just log an error if a path in node_modules_path doesn't exist, since we now …	chipx86	Feb. 12, 2026, 1:55 a.m.

flake8 passed.

JSHint passed.

Fix it!

djblets/pipeline/settings.py (Diff revision 1)

An issue was opened. Show all issues

I tested here manually with a missing package and it prompted to install it. I checked the docs to see what the stated behavior was if automating this, and it turns out that it assumes it should install. Looks like we need to pass --no here to disable that.



If any requested packages are not present in the local project dependencies,

then a prompt is printed, which can be suppressed by providing either --yes

or --no. When standard input is not a TTY or a CI environment is detected,

--yes is assumed. The requested packages are installed to a folder in the npm

cache, which is added to the PATH environment variable in the executed

process.

djblets/pipeline/settings.py (Diff revision 1)

An issue was opened. Show all issues

If any of the node_modules paths in the list lack this, will we get an error for each one until found?

djblets/pipeline/settings.py (Diff revision 1)

An issue was opened. Show all issues

I wonder if we should now just log an error if a path in node_modules_path doesn't exist, since we now build a list of paths to try. We'd probably still need to bail if the resulting list of paths is empty.

Fix it!

djblets/pipeline/settings.py (Diff revision 1)
An issue was opened. Show all issues
```
Can we call this node_modules_paths?
```