Skip to content

fix(provider-generator): copy only publishable files into the jsii compile bundle - #292

Merged
jsteinich merged 2 commits into
open-constructs:mainfrom
X-Guardian:chore/constructs-maker-publishable-files
Jul 2, 2026
Merged

jsteinich merged 2 commits into
open-constructs:mainfrom
X-Guardian:chore/constructs-maker-publishable-files

Conversation

@X-Guardian

Copy link
Copy Markdown
Contributor

Description

The @cdktn/provider-generator:build step (its postbuild runs the edge-provider-schema CLI, which generates bindings for all languages) intermittently fails in CI with:

[Error: ENOENT: no such file or directory, stat
 '/__w/cdk-terrain/cdk-terrain/packages/cdktn/.pack-staging/node_modules/semver/ranges/outside.js']

Example failed job: https://github.com/open-constructs/cdk-terrain/actions/runs/28436357153/job/84263776058?pr=285

The cause is in generateJsiiLanguage (constructs-maker.ts). To build the jsii compile bundle, it copies each compile dependency into a staging node_modules. One of those deps is the cdktn package directory itself, and the copy was a blanket fs.copy(dir, targetdir, { dereference: true }) of the whole directory.

packages/cdktn/.pack-staging is a transient tree that the pnpm package step (packages/cdktn/scripts/pack.mjs) creates and rewrites — it rmSyncs it and runs npm install into it. Because .pack-staging is a subdirectory of packages/cdktn, the blanket copy recursed into it. When the two steps overlapped in CI, fs.copy listed a file and then stated it after pack.mjs had already removed it mid-install — hence the ENOENT. Beyond the race, copying .pack-staging (and dist) into the bundle is pure waste: they are build outputs, never jsii compile inputs.

The fix copies only each dependency's publishable files — the set npm pack would ship — instead of the whole directory:

  • Add resolvePublishableFiles(dir) (constructs-maker.ts): runs npm pack --dry-run --json for the dep and returns its publishable file list (dropping the ..-escape paths npm emits when following pnpm's node_modules symlinks). This is the same technique already used by packages/cdktn/scripts/pack.mjs.

  • Copy that file list into staging instead of the whole directory. Build outputs — .pack-staging, dist, node_modules, *.tsbuildinfo — fall away because they are not in the package's published surface, so the copy no longer walks .pack-staging and the race is gone.

Verification: rebuilt @cdktn/provider-generator end-to-end (tsc + the postbuild edge-CLI that was crashing) with .pack-staging present locally — it passes, and all four language targets (Python, Java, C#, Go) plus the TypeScript providers are generated, confirming nothing needed was dropped from the copy.

Checklist

  • I have updated the PR title to match CDKTN's style guide
  • I have run the linter on my code locally
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works if applicable
  • New and existing unit tests pass locally with my changes

@X-Guardian
X-Guardian marked this pull request as ready for review June 30, 2026 14:29
@X-Guardian
X-Guardian requested a review from a team as a code owner June 30, 2026 14:29
@sakul-learning

Copy link
Copy Markdown
Contributor

Review verdict: Approve ✅

I reviewed the PR diff and the change looks well-scoped: generateJsiiLanguage() now stages only the files that npm pack --dry-run --json says each compile dependency would publish, instead of recursively copying the entire dependency directory. That directly addresses the .pack-staging race described in the PR without adding broader machinery.

Checks I ran locally on PR head 700f3c572133f715fe34c17cc3b0766bc0907753:

  • pnpm exec jest packages/cdktn/test/validations.test.ts --runInBand — passed: 31 tests, 9 snapshots.
  • pnpm exec lerna run --scope cdktn build — passed via Nx cache.
  • pnpm exec nx test cdktn --runInBand — passed: 38 suites / 438 tests / 282 snapshots.
  • pnpm exec nx run @cdktn/provider-generator:build --skip-nx-cache — passed, including dependent builds and the provider-generator tsc + postbuild path (node ./build/__tests__/edge-provider-schema/cli.js ./edge-provider-bindings).
  • As an extra regression check, I created a temporary packages/cdktn/.pack-staging/node_modules/semver/ranges/outside.js sentinel and reran pnpm exec nx run @cdktn/provider-generator:postbuild --skip-nx-cache; it passed, then I removed the temporary sentinel.

No blocking correctness/security/test-value findings from my review.

@jsteinich
jsteinich merged commit 562d4c7 into open-constructs:main Jul 2, 2026
259 checks passed
so0k pushed a commit that referenced this pull request Jul 3, 2026
🤖 Release PR — merge to cut a new release. Kept open and rebased
as commits land on `main`.
---


<details><summary>0.23.4</summary>

##
[0.23.4](v0.23.3...v0.23.4)
(2026-07-02)


### Features

* **cli:** validate Terraform CLI feature versions
([#237](#237))
([32d434c](32d434c))
* declared Terraform/OpenTofu target versions (validation foundation)
([#269](#269))
([0aa2343](0aa2343))
* **lib:** S3-backend - Add `useLockfile` field and update
`dynamodbTable` deprecated JSDoc
([#234](#234))
([32a408f](32a408f))
* **lib:** validate Fn usage against Terraform/OpenTofu versions
([#268](#268))
([cdf8115](cdf8115))


### Bug Fixes

* **cli:** don't mistake terraform error output for a missing-variable
prompt
([#267](#267))
([0ea946f](0ea946f))
* **cli:** release terraform state lock when interrupting diff/deploy
([#284](#284))
([de617db](de617db))
* **cli:** resolve prebuilt Java provider versions via repo1.maven.org
([#285](#285))
([d5af07d](d5af07d))
* **gha:** Adding CHANGELOG.md to prettier ignore list
([#258](#258))
([5160469](5160469))
* **gha:** pull jsii-terraform CI image from GHCR instead of Docker Hub
([#261](#261))
([f19d34b](f19d34b))
* **provider-generator:** copy only publishable files into the jsii
compile bundle
([#292](#292))
([562d4c7](562d4c7))
* **provider-generator:** match shorthand provider sources against
OpenTofu schema keys
([#293](#293))
([fda0103](fda0103))
* **release:** add Changelog title for correct release-please insertion
([#255](#255))
([004818a](004818a))
* **release:** drop devEngines.packageManager that broke npm publish
([ef508a6](ef508a6))
* **tools:** repair generate-function-bindings tool
([#273](#273))
([821e1cf](821e1cf))


### Miscellaneous Chores

* Add Gradle to Mise config
([#271](#271))
([1578270](1578270))
* add pnpm to mise
([#266](#266))
([48ab91f](48ab91f))
* add uv to mise
([#260](#260))
([163cf3d](163cf3d))
* **deps:** bump nrwl/nx-set-shas from
38457b511e60ee4e1da09255b658e480748a1af4 to
afb73a62d26e41464e9254689e1fd6122ee683c1
([#236](#236))
([b5ac018](b5ac018))
* **deps:** bump the github-actions-backward-compatible group across 1
directory with 7 updates
([#235](#235))
([de67d9f](de67d9f))
* **deps:** replace uuid dependency with node:crypto.randomUUID
([#291](#291))
([6d3657a](6d3657a))
* ESLint ignore test edge-provider-bindings
([#270](#270))
([ba407bb](ba407bb))
* Fix Verdaccio logging config
([#272](#272))
([aded64b](aded64b))
* **gha:** parallelize provider binding generation for documentation
examples
([#244](#244))
([0a9c955](0a9c955))
* Migrate package manager from yarn to pnpm
([#170](#170))
([d23da4e](d23da4e))
* migrate planning artifacts to cdktn-planning [skip ci]
([968c358](968c358))
* **release:** add release-please automation for versioning
([#243](#243))
([c759eec](c759eec))
* Remove dependency-pinner tool
([#250](#250))
([42c27d5](42c27d5))
* Remove unused dependencies
([#281](#281))
([c7de65e](c7de65e))
* **tests:** run verdaccio in-process
([#248](#248))
([c17677d](c17677d))
* Update JSII version constraint
([#245](#245))
([d89558f](d89558f))
* Update scheduled workflows to only run on open-constructs/cdk-terrain
([#256](#256))
([ba408cf](ba408cf))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: open-constructs-cdktn[bot] <291052431+open-constructs-cdktn[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants