Compare commits

...
Author SHA1 Message Date
Elior Nataf Lackritz 750f31482e fix(cli): cover every npm install lifecycle hook
npm runs prepublish, preprepare and postprepare on install too, so a project
using one of those would have had its install layer split and the hook would
run against a source tree that is not there yet.
2026-08-12 16:03:24 -04:00
Elior Nataf Lackritz 77fbf25d41 perf(cli): cache the Node install layer across source-only changes
The generated Node Dockerfile copies source before running the install, so
the first source change invalidates the install layer and every dependency
is reinstalled. A production build showed 0/3 steps cached with `npm ci`
rebuilding at 16.9s.

Copy `package.json` and the lockfile first instead. Falls back to the
current output when there is no lockfile, when an install lifecycle hook is
present, when a custom install command is set, or when the config is nested
in a workspace.
2026-08-12 15:39:54 -04:00
3 changed files with 223 additions and 10 deletions
+82 -9
View File
@@ -1162,7 +1162,47 @@ def _build_runtime_env_vars(config: Config) -> list[str]:
return env_vars
def _get_node_pm_install_cmd(project_dir: pathlib.Path) -> str:
# npm runs all of these as part of an install, before the source would be copied.
_NODE_INSTALL_HOOKS = (
"preinstall",
"install",
"postinstall",
"prepublish",
"preprepare",
"prepare",
"postprepare",
)
def _splittable_node_manifests(
project_dir: pathlib.Path, lockfile: str | None
) -> list[str] | None:
"""Return manifests to copy before installing, or None if unsafe to split.
A lockfile is required: without one the install resolves versions at build
time, so a cached layer could pin an older resolution than a clean build.
"""
if lockfile is None:
return None
manifest = project_dir / "package.json"
try:
if not manifest.is_file():
return None
with open(manifest) as f:
package_json = json.load(f)
except (OSError, ValueError):
return None
if not isinstance(package_json, dict):
return None
scripts = package_json.get("scripts") or {}
if any(hook in scripts for hook in _NODE_INSTALL_HOOKS):
return None
return ["package.json", lockfile]
def _get_node_pm_install_cmd(project_dir: pathlib.Path) -> tuple[str, str | None]:
"""Return the install command and the lockfile it was chosen from."""
def test_file(file_name):
full_path = project_dir / file_name
try:
@@ -1201,13 +1241,19 @@ def _get_node_pm_install_cmd(project_dir: pathlib.Path) -> str:
if yarn:
install_cmd = "yarn install --frozen-lockfile"
lockfile = "yarn.lock"
elif pnpm:
install_cmd = "pnpm i --frozen-lockfile"
lockfile = "pnpm-lock.yaml"
elif npm:
install_cmd = "npm ci"
lockfile = "package-lock.json"
elif bun:
install_cmd = "bun i"
lockfile = "bun.lockb"
else:
# No lockfile, so the install resolves versions at build time.
lockfile = None
pkg_manager_name = get_pkg_manager_name()
if pkg_manager_name == "yarn":
@@ -1219,7 +1265,7 @@ def _get_node_pm_install_cmd(project_dir: pathlib.Path) -> str:
else:
install_cmd = "npm i"
return install_cmd
return install_cmd, lockfile
semver_pattern = re.compile(r":(\d+(?:\.\d+)?(?:\.\d+)?)(?:-|$)")
@@ -1423,7 +1469,7 @@ ADD {relpath} /deps/{name}
"# -- Installing JS dependencies --",
f"ENV NODE_VERSION={config.get('node_version') or DEFAULT_NODE_VERSION}",
f"WORKDIR {local_deps.working_dir}",
f"RUN {_get_node_pm_install_cmd(config_path.parent)} && tsx /api/langgraph_api/js/build.mts",
f"RUN {_get_node_pm_install_cmd(config_path.parent)[0]} && tsx /api/langgraph_api/js/build.mts",
"# -- End of JS dependencies install --",
]
)
@@ -1492,7 +1538,9 @@ def node_config_to_docker(
install_root = (
pathlib.Path(build_context).resolve() if build_context else config_path.parent
)
install_cmd = install_command or _get_node_pm_install_cmd(install_root)
detected_cmd, detected_lockfile = _get_node_pm_install_cmd(install_root)
install_cmd = install_command or detected_cmd
relative_workdir = ""
if build_context:
relative_workdir = _calculate_relative_workdir(config_path, build_context)
container_name = pathlib.Path(build_context).name
@@ -1530,16 +1578,41 @@ def node_config_to_docker(
else:
build_workdir = faux_path
source_root = faux_path if not build_context else container_root
# Excluded: a custom install command may read files we have not copied yet,
# and a nested config means workspace manifests the root copy would miss.
manifests = (
_splittable_node_manifests(install_root, detected_lockfile)
if install_command is None and not relative_workdir
else None
)
if manifests:
add_steps = [
*(f"ADD {name} {source_root}/{name}" for name in manifests),
"",
f"WORKDIR {install_workdir}",
"",
install_step,
"",
f"ADD . {source_root}",
]
else:
add_steps = [
f"ADD . {source_root}",
"",
f"WORKDIR {install_workdir}",
"",
install_step,
]
docker_file_contents = [
f"FROM {image_str}",
"",
os.linesep.join(config["dockerfile_lines"]),
"",
f"ADD . {faux_path if not build_context else container_root}",
"",
f"WORKDIR {install_workdir}",
"",
install_step,
*add_steps,
"",
os.linesep.join(env_vars),
"",
+1 -1
View File
@@ -1019,7 +1019,7 @@ def python_config_to_docker_uv_lock(
docker_plan.add_instruction("WORKDIR", plan.working_dir)
docker_plan.add_instruction(
"RUN",
f"{_get_node_pm_install_cmd(plan.target_root)} && "
f"{_get_node_pm_install_cmd(plan.target_root)[0]} && "
"tsx /api/langgraph_api/js/build.mts",
)
docker_plan.add_raw("# -- End of JS dependencies install --")
+140
View File
@@ -3424,3 +3424,143 @@ class TestHasDisallowedBuildCommandContent:
)
def test_valid_commands_allowed(self, cmd: str) -> None:
assert not has_disallowed_build_command_content(cmd)
class TestNodeDependencyLayerOrdering:
"""Dependency manifests are copied before source so the install layer caches.
Without this the first source change invalidates the install, and a JS
deployment reinstalls every dependency on every push.
"""
def _project(
self,
tmp_path: pathlib.Path,
*,
lockfile: str | None,
scripts: dict[str, str] | None = None,
) -> pathlib.Path:
package_json: dict = {"name": "agent"}
if scripts:
package_json["scripts"] = scripts
(tmp_path / "package.json").write_text(json.dumps(package_json))
if lockfile:
(tmp_path / lockfile).write_text("")
(tmp_path / "graphs").mkdir()
(tmp_path / "graphs" / "agent.js").write_text("")
config_path = tmp_path / "langgraph.json"
config_path.write_text("{}")
return config_path
def _dockerfile(self, config_path: pathlib.Path, **kwargs) -> str:
actual, _ = config_to_docker(
config_path,
validate_config(
{"node_version": "20", "graphs": {"agent": "./graphs/agent.js:graph"}}
),
base_image="langchain/langgraphjs-api",
**kwargs,
)
return clean_empty_lines(actual)
def test_manifests_copied_before_install(self, tmp_path: pathlib.Path) -> None:
config_path = self._project(tmp_path, lockfile="package-lock.json")
lines = self._dockerfile(config_path).splitlines()
add_manifest = lines.index(
f"ADD package.json /deps/{tmp_path.name}/package.json"
)
add_lock = lines.index(
f"ADD package-lock.json /deps/{tmp_path.name}/package-lock.json"
)
install = lines.index("RUN npm ci")
add_source = lines.index(f"ADD . /deps/{tmp_path.name}")
assert add_manifest < install
assert add_lock < install
assert install < add_source
def test_lockfile_choice_follows_package_manager(
self, tmp_path: pathlib.Path
) -> None:
config_path = self._project(tmp_path, lockfile="pnpm-lock.yaml")
dockerfile = self._dockerfile(config_path)
assert f"ADD pnpm-lock.yaml /deps/{tmp_path.name}/pnpm-lock.yaml" in dockerfile
assert "package-lock.json" not in dockerfile
def test_no_lockfile_keeps_source_first(self, tmp_path: pathlib.Path) -> None:
# No lockfile means the install resolves at build time, so caching it is wrong.
config_path = self._project(tmp_path, lockfile=None)
lines = self._dockerfile(config_path).splitlines()
assert lines.index(f"ADD . /deps/{tmp_path.name}") < lines.index("RUN npm i")
assert not any(line.startswith("ADD package.json") for line in lines)
def test_nested_config_keeps_source_first(self, tmp_path: pathlib.Path) -> None:
# A workspace keeps manifests in subdirectories the root copy would miss.
root = tmp_path / "repo"
root.mkdir()
(root / "package.json").write_text(json.dumps({"name": "root"}))
(root / "package-lock.json").write_text("")
pkg = root / "packages" / "agent"
pkg.mkdir(parents=True)
(pkg / "graphs").mkdir()
(pkg / "graphs" / "agent.js").write_text("")
config_path = pkg / "langgraph.json"
config_path.write_text("{}")
lines = self._dockerfile(config_path, build_context=str(root)).splitlines()
assert lines.index("ADD . /deps/repo") < lines.index("RUN npm ci")
assert not any(line.startswith("ADD package.json") for line in lines)
def test_custom_install_command_keeps_source_first(
self, tmp_path: pathlib.Path
) -> None:
# A custom command may read files the manifest copy would not include.
config_path = self._project(tmp_path, lockfile="package-lock.json")
lines = self._dockerfile(
config_path,
install_command="npm run bootstrap",
build_context=str(tmp_path),
).splitlines()
assert lines.index(f"ADD . /deps/{tmp_path.name}") < lines.index(
"RUN npm run bootstrap"
)
@pytest.mark.parametrize(
"hook",
[
"preinstall",
"install",
"postinstall",
"prepublish",
"preprepare",
"prepare",
"postprepare",
],
)
def test_install_hook_keeps_source_first(
self, tmp_path: pathlib.Path, hook: str
) -> None:
# A hook referencing a project file would hit ENOENT: source is not copied yet.
config_path = self._project(
tmp_path,
lockfile="package-lock.json",
scripts={hook: "node scripts/setup.js"},
)
lines = self._dockerfile(config_path).splitlines()
assert lines.index(f"ADD . /deps/{tmp_path.name}") < lines.index("RUN npm ci")
assert not any(line.startswith("ADD package.json") for line in lines)
def test_only_the_chosen_lockfile_is_copied(self, tmp_path: pathlib.Path) -> None:
# Install picks yarn, so copying the npm lockfile would bust the cache for nothing.
config_path = self._project(tmp_path, lockfile="yarn.lock")
(tmp_path / "package-lock.json").write_text("")
dockerfile = self._dockerfile(config_path)
assert f"ADD yarn.lock /deps/{tmp_path.name}/yarn.lock" in dockerfile
assert "ADD package-lock.json" not in dockerfile