fix: ship a workspaces monorepo function's own dependencies when building in source

[bnusunny]

Issue #, if available: Fixes #933

#931 has merged, and this PR has been rebased onto it. The diff is now exactly its own two commits (4aab4bc and eebcfc3) — the earlier instruction to skip a leading #931 commit no longer applies. The rebase also dropped two docstrings this branch had been carrying from before #931's final review round, so it no longer reverts anything that just landed. It builds on SubprocessNpm, the install_dir refactor and ExperimentalFlag.NodejsMonorepo from #931, and the fixture's lockfile pins versions below what its manifests allow, so the assertions only mean what they say because the lockfile is honoured.

Description of changes

nodejs_npm with --build-in-source produces an artifact with no dependencies at all for an npm workspaces monorepo, and reports success. npm hoists a workspace member's dependencies to the monorepo root, so nothing is installed beside the function; the artifacts link looks for node_modules next to the function, finds none, and silently skips. The function then fails at runtime with Cannot find module.

Opt-in while it rolls out. Both changes are behind experimentalNodejsMonorepo - sam-cli's
ExperimentalFlag.NodejsMonorepo, the same flag #931 uses. Without it npm is not asked where its project
root is and the artifacts link stays exactly where it is today, which for a monorepo means this bug is
still there: that is deliberate, so a release can carry the fix while users opt into it.
test_without_the_flag_a_monorepo_keeps_the_plain_link_and_npm_is_not_asked pins that side.

Two changes, one per commit.

cabe7ee — link the artifacts to wherever npm actually installed. SubprocessNpm.resolve_project_root asks npm itself (npm prefix, cached per directory), and only a project root outside the install directory redirects the link. npm answers with the install directory itself for anything that is not a workspace member, so every existing path — including the external-manifest one, which links through the source tree — is left exactly as it was.

070a021 — ship only the function's own dependencies. Linking the whole hoisted root would put every sibling function's dependencies into every artifact. NodejsNpmLinkDependencyClosureAction links just the packages the function resolves, each under its own name:

case	behaviour
a package the function depends on	linked into the artifacts under its own name, scope included
a workspace sibling (@mono/shared)	linked under its package name, resolved to its source directory
a nested copy npm kept because a dependency pins a conflicting version	left nested — hoisting it would shadow the top-level version for every other caller
a sibling function's dependency	not linked
npm cannot answer	the whole installed tree is linked rather than guessing: an over-complete node_modules still runs
a package npm named that has no readable manifest	the build fails loudly, rather than placing it where node will not find it

Both path comparisons go through os.path.normcase. They compare npm's stdout against a build-computed path, and on Windows two spellings that differ only in case — a drive letter included — name the same directory; reading them as different roots would redirect the link for a project that is not a workspace member, or hoist a nested copy. It is a no-op off Windows.

The esbuild workflow needs none of this: it bundles what each entry point imports into the output file, resolving through the hoisted root, so its artifacts were already correct.

New fixture manifests and lockfiles under the nodejs_npm integration testdata/ directory are test data, not a dependency change to this package.

Design notes

Detail that reviewers asked for, kept here rather than in the code comments.

Names come from the install path, not the manifest. npm supports aliases: "lodash4": "npm:lodash@^4.0.0" installs at node_modules/lodash4 while the manifest inside still says "name": "lodash". Reading the manifest renames the package, and require("lodash4") then fails with Cannot find module — the same class of failure this PR fixes. npm ls --parseable prints only the path, so _link_name takes the segments after the last node_modules component, scope included.

That also keeps an untrusted value out of a filesystem path: a dependency's manifest is third-party input, and "name": "../../../evil" or "/tmp/x" joined onto the destination writes outside the artifacts. The manifest is read only for a path that is not under node_modules — a workspace dependency, which npm reports as its own source directory — and that value is validated against NODE_MODULES_PACKAGE_NAME.

Two guards, covering different things. With the name rule relaxed to ^.*$, the containment assert alone stops ../../../evil and .. but allows /tmp/evil and a/b/c; both of those stay inside the artifacts, because os.path.join(destination, *"/tmp/evil".split("/")) passes an empty first segment rather than an absolute path. So the name rule is the primary control and the assert covers traversal. The assert resolves the link's parent and appends the final component unresolved — resolving the link itself follows an existing symlink to its target, which is outside the artifacts by design, and would refuse every legitimate re-link — then normalises, because commonpath compares components without interpreting a trailing ...

Same-name collisions are resolved explicitly. A function pinning its own version of a package the root also hoists yields install_dir/node_modules/lodash and project_root/node_modules/lodash; neither is nested inside the other, so both reach the link step, and create_symlink_or_copy returns early on an existing destination — whichever npm printed first used to win. _packages_by_name gives the function's own copy precedence, since that is what its code resolves. The hoisted version is not lost for anyone else: every package is linked as a symlink into the real tree, so a dependency resolving lodash from inside its real directory still walks up to the root's copy.

This is not fixable by treating install_dir as a nesting container, which is the obvious reading of "the nesting test never matches install_dir". project_root contains every hoisted package, so adding it as a container would exclude all of them, and excluding the function's own pinned copy would reinstate the bug this action exists to fix.

The no-closure fallback is an overlay, not one link. Linking project_root/node_modules as the artifacts' node_modules is simpler and is not a superset of what the function resolves: a member's dependency that conflicts with the hoisted version is installed under the member's own node_modules, and a single link to the root cannot carry it, so the function would resolve the root's different version or nothing at all. _link_every_installed_package links entries one at a time from the root then the function's own directory, resolved into one mapping before anything is linked (linking as it goes would keep the first of each name, the opposite of the needed precedence). That also removes a dangling link: os.symlink succeeds against a missing target on POSIX, so a function with no installed dependencies previously got a node_modules pointing nowhere.

Failing the build instead was the alternative. Rejected because resolve_dependency_closure returns None whenever npm ls exits non-zero, and a missing peer dependency does that on a perfectly usable tree — common enough that failing would make the flag unusable for many real monorepos.

Description of how you validated changes

The new integration test fails without the fix. With this branch's tests kept and only the three production files reverted to #931, test_build_in_source_in_workspaces_monorepo_links_the_hoisted_dependencies fails on all five supported runtimes with AssertionError: False is not true : the artifacts have no node_modules at all — the #933 symptom exactly.

The fixture is a workspaces monorepo with two functions on disjoint dependencies (minimal-request-promise, ms), a shared workspace package both depend on, and a version conflict the shared package pins so npm keeps a nested copy. Against the real npm, across every supported runtime, the test asserts each function's artifacts carry its own dependency and the shared package but not its sibling's, that the nested conflicting copy stays nested, and that node -e "require('./included.js')" succeeds from each artifacts directory — so the artifact is covered, not just the installed tree.

Unit tests cover the closure selection (test_links_only_this_function_s_dependencies, test_links_a_workspace_dependency_under_its_own_name, test_leaves_a_nested_copy_inside_the_dependency_that_pins_it), the workflow wiring for both the monorepo and the unchanged non-monorepo path, and npm prefix resolution including its per-directory caching and that a failure is cached too.

One test per review finding, each mutation-checked — reverting the fix fails the test:

finding	test
alias name lost to the manifest	test_an_aliased_dependency_keeps_the_name_npm_installed_it_under
traversal from a manifest name	test_a_hostile_manifest_name_cannot_write_outside_the_artifacts
same-name collision decided by output order	test_the_function_s_own_pinned_copy_wins_a_name_it_shares_with_the_hoisted_one
fallback dropped non-hoisted dependencies	test_the_fallback_carries_the_function_s_own_non_hoisted_dependencies
fallback left a dangling symlink	test_the_fallback_links_nothing_rather_than_a_dangling_node_modules

The manifest is now needed only outside node_modules, so test_fails_loudly_when_a_reported_package_has_no_manifest became test_fails_loudly_when_a_workspace_dependency_has_no_manifest, with test_a_package_under_node_modules_needs_no_manifest_at_all pinning the other half.

• 864 unit tests pass; ruff check and black --check clean.
• nodejs + esbuild integration workflows: 300 passed on npm 11.19.0, Linux. Also run on npm 10.9.9 — because npm prefix and hoisting are npm-version dependent.
• Windows is untested outside CI. The link steps go through the existing create_symlink_or_copy, which falls back to a copy when os.symlink is not permitted; the windows-latest lanes on this PR are green.

Checklist

• Review the generative AI contribution guidelines

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 587257c..070a021
Files: 34 (6 source/test-logic files reviewed in depth; JSON/JS fixtures skimmed)
Comments: 3

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 587257c..bb7368b
Files: 36 (8 logic files reviewed in depth; JSON/JS fixtures skimmed)
Comments: 4

[aws-sam-tooling-bot]

Code Review Results

Reviewed: 6509bec..eebcfc3
Files: 16
Comments: 1