Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 74 additions & 9 deletions libs/cli/langgraph_cli/config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import copy
import json
import os
Expand Down Expand Up @@ -1162,7 +1162,39 @@
return env_vars


def _get_node_pm_install_cmd(project_dir: pathlib.Path) -> str:
# These run as part of the install step, before the source would be copied.
_NODE_INSTALL_HOOKS = ("preinstall", "install", "postinstall", "prepare")
Comment thread
open-swe[bot] marked this conversation as resolved.
Outdated


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:
Expand Down Expand Up @@ -1201,13 +1233,19 @@

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":
Expand All @@ -1219,7 +1257,7 @@
else:
install_cmd = "npm i"

return install_cmd
return install_cmd, lockfile


semver_pattern = re.compile(r":(\d+(?:\.\d+)?(?:\.\d+)?)(?:-|$)")
Expand Down Expand Up @@ -1423,7 +1461,7 @@
"# -- 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 --",
]
)
Expand Down Expand Up @@ -1492,7 +1530,9 @@
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
Expand Down Expand Up @@ -1530,16 +1570,41 @@
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),
"",
Expand Down
2 changes: 1 addition & 1 deletion libs/cli/langgraph_cli/uv_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ def copy_from_project_root(
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 --")
Expand Down
131 changes: 131 additions & 0 deletions libs/cli/tests/unit_tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3424,3 +3424,134 @@ def test_double_ampersand_allowed(self) -> None:
)
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", "prepare"]
)
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
Loading