fix(deps): pin @aws-cdk/toolkit-lib yaml to v1 to fix 'yaml/types' resolution - #2122
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
|
looking into failing CI |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Changes requested
The override itself is correct and well-motivated — @aws-cdk/toolkit-lib@1.38.2 declares yaml: ^1, its yaml-cfn.js does require("yaml/types") which only resolves in yaml@1.x (v2 has no ./types in its exports map), and pinning the nested yaml to ^1 matches upstream's declared range. Bun supports npm's nested overrides syntax, so this works for both npm and bun install paths.
Issue: bun.lock is not updated
package.json is the only file changed in this PR, but every CI workflow in this repo runs bun install --frozen-lockfile (.github/workflows/build.yml, unit-test.yml, check.yml). Bun's --frozen-lockfile errors out when package.json and bun.lock disagree, which is what an added override does — the resolved yaml version under @aws-cdk/toolkit-lib in the lockfile will not reflect the new constraint until the lockfile is regenerated.
This matches what's happening on the PR: all build, unit-test, and check jobs across Linux/macOS/Windows are currently failing.
Fix: run bun install locally (without --frozen-lockfile) and commit the updated bun.lock alongside the package.json change. That will both make CI pass and ensure downstream bun compile builds embed the v1 copy of yaml/types deterministically (which is the whole point of the fix).
Once the lockfile is regenerated, it's worth spot-checking the bun.lock diff to confirm the toolkit-lib yaml entry is pinned to a 1.x version — that verifies the override actually took effect rather than silently being ignored.
be50d65 to
d93c63b
Compare
|
forgot to commit bun.lock earlier just added it |
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2122 +/- ##
=========================================
Coverage 97.41% 97.41%
=========================================
Files 453 453
Lines 27637 27637
=========================================
Hits 26922 26922
Misses 715 715 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
CI windows failure seems like it's from the branch but let me confirm |
…solution
@aws-cdk/toolkit-lib's yaml-cfn.js does require("yaml/types") at import
time, a subpath that only exists in yaml v1. When an install topology
resolves toolkit-lib's yaml to the hoisted yaml v2 (whose exports map
blocks ./types), the module fails to load with
ERR_PACKAGE_PATH_NOT_EXPORTED ("Cannot find module 'yaml/types'").
This bites bun compile, which inlines toolkit-lib and embeds whatever
the build machine resolves.
Add a nested override forcing toolkit-lib's yaml to ^1 so it always
resolves the v1 nested copy that ships yaml/types.
d93c63b to
367f995
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Hweinstock
left a comment
There was a problem hiding this comment.
makes sense. I'm not still not fully clear on why its resolving in the wrong version if its pinned here https://github.com/aws/aws-cdk-cli/blob/02523f01b0a45a04582628ef3cd53160a3287032/packages/%40aws-cdk/toolkit-lib/package.json#L132, but I guess its because its not bundled in, so it tries to pull it in at runtime via require which doesn't check the version.
|
Yeah, |
What
Add a nested
overridesentry forcing@aws-cdk/toolkit-lib'syamldependency to^1.Why
@aws-cdk/toolkit-lib'syaml-cfn.jsdoesrequire("yaml/types")at import time — a subpath that only exists inyamlv1.yaml@2(pulled transitively, e.g. via thelint-stageddevDep) has anexportsmap that does not expose./types, so any install topology that resolves toolkit-lib'syamlto the hoisted v2 fails to load the module with:This is especially dangerous for
bun compile, which inlines toolkit-lib (onlybundle()marks it external) and embeds whateveryaml/typesthe build machine resolves.Fix
Guarantees toolkit-lib always resolves the nested
yaml@1copy (which shipstypes.js).Testing
require.resolve('yaml/types')from toolkit-lib resolves to the v1 nested copy under bothnodeandbun.ERR_PACKAGE_PATH_NOT_EXPORTED) and confirmed the override prevents it.project deploy(CDK synth + CloudFormation) exercised end-to-end after the change.