Repository navigation
fix(tests): build the edge provider bindings in their own cached Nx target - #482
Conversation
|
Two findings remain at
The current CI integration path itself is wired correctly: it ran |
|
Thanks for moving the bindings into their own target. CI looks good. I agree with the two findings @sakul-learning left on #485; both are about code in this PR:
Minor: the two edge-binding uploads in |
|
Thanks both, addressed in c9dbd23:
#485 is rebased on top. |
…r Nx cache hits Both packages generate their real build artifact in a postbuild script: provider-generator writes edge-provider-bindings/ and cdktn-cli writes bundle/. Neither directory is in the shared default outputs, so a remote cache hit on the build target restores nothing useful. On Depot runners the integration prepare job then uploads an empty edge-provider-bindings artifact and every matrix job fails on download. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb
… missing upload-artifact only warns when the path is empty, so the matrix fans out and every job fails on download instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb
…arget The bindings were a postbuild side effect of provider-generator's build, undeclared as an output, so a remote cache hit left the directory empty. A dedicated target declares the output and names the generator sources under src/__tests__ as inputs, and the three prepare jobs run it explicitly. The cdktn-cli change moves to its own PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb
…sources into build edge:install copied a directory that pnpm package no longer produces, so the local integration flow failed before Jest started; it now runs the cached build:edge-bindings target first. The provider-generator build target gains the edge schema sources as an input, so the compiled generator can never come from a cache entry older than the sources the bindings target hashes. The two release.yml edge uploads get the same fail-fast setting as integration.yml. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb
c9dbd23 to
dfada09
Compare
### Description Depot runners set `NX_SELF_HOSTED_REMOTE_CACHE_SERVER`, so every Nx task on a `depot-*` runner reads from and writes to a remote cache that PR runs share (#477, #482 and #485 are fallout from that). The two `release.yml` jobs that build the published artifacts, `prepare-release` and `prepare-next`, run on `ubuntu-latest` today. That keeps what we publish out of the shared cache, but only because of which runner the jobs happen to use. This makes it explicit. `NX_SKIP_REMOTE_CACHE: "true"` in `release.yml`'s workflow-level `env` turns off remote cache reads and writes for every job defined in that file, whatever runner it lands on. - The job-local cache stays on, so `pnpm run package` still reuses the `build` tasks from the step before it in the same fresh job. Turning off all caching (`NX_SKIP_NX_CACHE`) would rebuild everything a second time. - Workflow-level `env` doesn't pass through to reusable workflows, so the integration, examples, unit and quality runs that `release.yml` calls keep using the cache. Those are test runs, not published output. - No other workflow builds published output with Nx: `release-publish.yml` publishes the downloaded `dist` artifacts, and `docker.yml` uses a Docker build. ### Verification In Nx 22.7.9, `NX_SKIP_REMOTE_CACHE=true` sets `skipRemoteCache` (`utils/command-line-utils.js`), and the task runner then never creates a remote cache. Running `NX_SKIP_REMOTE_CACHE=true pnpm exec nx run @cdktn/commons:build` locally prints: ``` NX Remote Cache Disabled Nx will continue running, but nothing will be written or read from the remote cache. ``` The next Release run should print the same banner in both prepare jobs. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have performed a self-review of my code - [ ] I have made corresponding changes to the documentation if applicable - [x] My changes generate no new warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
jsteinich
left a comment
There was a problem hiding this comment.
Thanks, all three points are addressed. Building the bindings from edge:install covers every path that copies them, and CI never calls it.
…ts real inputs (#485) ### Description Split out of #482 after review (#482 (comment) and the neighbouring thread on `packages/cdktn-cli/package.json`). `cdktn-cli`'s build target is `tsc --noEmit`; the actual artifact, `bundle/`, is written by `postbuild`. The shared default outputs (`build/`, `.jsii`) do not cover it, so a Depot remote cache hit restores nothing. Declaring `bundle/` as an output is only safe if the hash covers everything the bundle is built from, and two inputs were missing: - **Templates.** The bundle script copies `packages/@cdktn/cli-core/templates` into `bundle/templates`, but `.nxignore` excluded the whole templates directory from the Nx file map, so template edits never changed any hash. The entry exists because Nx reads `templates/typescript/package.json` as a project named `{{ $base }}`. This narrows the ignore to `templates/*/package.json`; every other template file is back in the file map. The same gap applied to `cli-core`'s own cached `package` target, which ships the templates in `dist`. - **`SENTRY_DSN`.** esbuild bakes `process.env.SENTRY_DSN` into the bundle. `pr-unit.yml` builds without the secret and the integration prepare job builds with it, both on Depot runners, so they shared one cache key. It is now an `env` input. ### Verification In a clean worktree with Nx 22.7.9: - `nx show projects` lists 72 projects with the narrowed ignore (73 including `{{ $base }}` without it; ignoring only the template `jest.config.js` does not help). - `nx show projects --affected --files packages/@cdktn/cli-core/templates/typescript/main.ts` now reports `@cdktn/cli-core` and `cdktn-cli` (empty before). - Building `cdktn-cli` twice with `SENTRY_DSN` unset is a cache hit the second time; building with `SENTRY_DSN=probe` re-runs only `cdktn-cli:build` while its eight dependencies stay cached. - Deleting `bundle/` and rebuilding is a cache hit that restores `bundle/bin` and `bundle/templates`. Residual gap: the template `package.json` files themselves must stay out of the file map, so an edit to one of them does not invalidate the bundle. ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have performed a self-review of my code - [ ] I have made corresponding changes to the documentation if applicable - [x] My changes generate no new warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
🤖 Release PR — merge to cut a new release. Kept open and rebased as commits land on `main`. --- <details><summary>0.25.0</summary> ## [0.25.0](v0.24.0...v0.25.0) (2026-10-27) ### ⚠ BREAKING CHANGES * **lib:** `IAssetPackaging` gains `acceptsDirectorySource` as a required member. Custom implementations of the interface must add it. * **lib:** ValidateBinaryVersion and ValidateTerraformVersion are removed (they were only reachable through deep imports, not the cdktn package root). Synth no longer executes the terraform/tofu binary for feature validation; set validateInstalledBinary: true in cdktf.json to verify the installed binary before commands that run it. * **lib:** resource moves (moved blocks) are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.5.0 now fail synth; projects whose installed binary is older than their declared targets should set validateInstalledBinary: true in cdktf.json. * **lib:** import blocks (importFrom / ImportableResource) are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.5.0 now fail synth; projects whose installed binary is older than their declared targets should set validateInstalledBinary: true in cdktf.json. * **lib:** terraform_data resources are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.4.0 now fail synth; projects whose installed binary is older than their declared targets should set * **lib:** cloud backends (the cloud block) are now validated at synth against the project's declared targetVersions (default terraform >=1.5.7, opentofu >=1.6.0) instead of the locally installed binary. Projects that declare terraform targets below 1.1.0 now fail synth; projects whose installed binary is older than their declared targets should set validateInstalledBinary: true in cdktf.json. * **lib:** IAssetPackaging gains a required member omitsDirectoryEntries; external implementations must add it. Custom TerraformAsset assetHash values are now validated against `/^[A-Za-z0-9_.-]+$/` and throw if they contain other characters (e.g. "feature/my-branch"). * **lib:** add asset pipeline foundation types and hash utilities ([#371](#371)) ### Features * **cli:** add a typescript-pnpm init template and deprecate Yarn ([#422](#422)) ([81da164](81da164)) * **cli:** move the TypeScript template to tsx and TypeScript 6 ([#396](#396)) ([b50349d](b50349d)) * **cli:** point AI coding agents at the CDKTN docs MCP server ([#518](#518)) ([f257cd9](f257cd9)) * **cli:** rename `--cdktf-version` to `--cdktn-version` in `init` ([#228](#228)) ([dd3818e](dd3818e)) * **cli:** replace HashiCorp checkpoint telemetry with Sentry usage metrics and consent ([#411](#411)) ([e17046a](e17046a)) * **cli:** report aggregated stack, provider and command usage metrics ([#413](#413)) ([ed9b1e5](ed9b1e5)) * **cli:** report the installed binary, target versions, platform and package manager in usage metrics ([#412](#412)) ([bfe26a8](bfe26a8)) * **cli:** resolve providers from the registry the project targets ([#445](#445)) ([78898ae](78898ae)) * **deps:** adopt jsii 6.0 and TypeScript 6 ([#395](#395)) ([26fa8fa](26fa8fa)) * **gha:** Revert "run the affected unit tests against OpenTofu ([#439](#439))" ([#446](#446)) ([ae015ef](ae015ef)) * **gha:** run the affected unit tests against OpenTofu ([#439](#439)) ([823682d](823682d)) * **gha:** run the affected unit tests against OpenTofu ([#447](#447)) ([c0c4c1c](c0c4c1c)) * **lib:** add asset bundling extension interface ([#442](#442)) ([7839625](7839625)) * **lib:** add asset pipeline foundation types and hash utilities ([#371](#371)) ([a008d12](a008d12)) * **lib:** add asset staging core with no bundling ([#433](#433)) ([44d8bd1](44d8bd1)) * **lib:** add support for single-file bundler outputs ([#464](#464)) ([3592db0](3592db0)) * **lib:** pass ignore strategy through terraform asset ([#472](#472)) ([705e5b9](705e5b9)) * **tests:** add per-test runtime pinning to the integration matrix ([#405](#405)) ([17c2278](17c2278)) ### Bug Fixes * **cli:** catch failures of the background environment collection ([#531](#531)) ([c5feed8](c5feed8)) * **cli:** correct the constructs range comment in the TypeScript template hook ([#408](#408)) ([9fc0e43](9fc0e43)) * **cli:** don't crash after a successful deploy when an output is missing ([#375](#375)) ([0bbd64a](0bbd64a)) * **cli:** fail cleanly when the --outputs-file write fails ([#376](#376)) ([8f9d03a](8f9d03a)) * **cli:** make `cdktn get` remove stale files for resources removed from a provider ([#421](#421)) ([d6e28dc](d6e28dc)) * **cli:** match prebuilt providers regardless of the project's registry ([#493](#493)) ([ea82124](ea82124)) * **cli:** probe with `version` instead of `-v` ([#471](#471)) ([b87ee22](b87ee22)) * **cli:** require patched Node 22.23.4 and bump undici for security fixes ([#516](#516)) ([76436f5](76436f5)) * **cli:** resolve bundled handlers next to the entrypoint ([#414](#414)) ([49600c1](49600c1)) * **cli:** resolve upgraded providers from the CLI's own registry ([#489](#489)) ([10672ee](10672ee)) * **cli:** stop the top-level error handler racing yargs ([#410](#410)) ([5d0c2ef](5d0c2ef)) * **deps:** move jsii-rosetta to 6.0.16 for stream-json 3 ([#463](#463)) ([f1ec16f](f1ec16f)) * **gha:** authenticate to public.ecr.aws in the docker image build ([#519](#519)) ([c65bf2f](c65bf2f)) * **gha:** don't fail the pnpm upgrade jobs on ignored dep builds ([#399](#399)) ([5b66342](5b66342)) * **gha:** move the jsii package cache out of the container HOME ([#514](#514)) ([df5b490](df5b490)) * **gha:** pass -R to gh pr edit in release_github relabel step ([#372](#372)) ([358edc7](358edc7)) * **gha:** push only the prerelease tag from prepare-next ([#476](#476)) ([848f2a6](848f2a6)) * **lib:** avoid terraform init plugin-cache race in test matchers ([#467](#467)) ([26847f9](26847f9)) * **lib:** retry the staging install without --prefer-offline outside CI ([#407](#407)) ([7bb26e7](7bb26e7)) * **provider-generator:** pass the default provider configuration a module requires on get ([#466](#466)) ([a5225ab](a5225ab)) * **provider-generator:** pick the docs registry from targetVersions ([#443](#443)) ([00f2884](00f2884)) * **provider-generator:** pick the module docs registry from targetVersions ([#484](#484)) ([8200330](8200330)) * **provider-generator:** regenerate bindings when the target registry changes ([#490](#490)) ([813e022](813e022)) * **provider-generator:** satisfy module provider configuration aliases on get ([#383](#383)) ([54a8217](54a8217)) * **provider-generator:** stop a provider_functions resource clobbering the functions submodule ([#403](#403)) ([cf19de9](cf19de9)) * **tests:** await the pip install in the python provider command tests ([#478](#478)) ([1c83a36](1c83a36)) * **tests:** bound and retry the dist publish to the local registry ([#425](#425)) ([cc9f9e2](cc9f9e2)) * **tests:** build the edge provider bindings in their own cached Nx target ([#482](#482)) ([f480cc9](f480cc9)) * **tests:** drop unused ts-jest transform from shared jest preset ([#382](#382)) ([03f6e16](03f6e16)) * **tests:** mark the terraform-cloud target Terraform-only in the matrix ([#430](#430)) ([b0deefa](b0deefa)) * **tests:** run example builds with the Nx cache skipped ([#477](#477)) ([3e25e13](3e25e13)) * **tools:** declare the cdktn-cli bundle as a cached Nx output with its real inputs ([#485](#485)) ([10dd118](10dd118)) * **tools:** make the workspace build, package, lint and test on Windows ([#364](#364)) ([8dcddbe](8dcddbe)) ### Code Refactoring * **lib:** Remove the ValidateBinaryVersion synth-time binary probe ([#457](#457)) ([e60142e](e60142e)) * **lib:** Use declared targetVersions for synth-time validation of import blocks ([#454](#454)) ([c0097f4](c0097f4)) * **lib:** Use declared targetVersions for synth-time validation of resource moves ([#453](#453)) ([43edc1e](43edc1e)) * **lib:** Use declared targetVersions for synth-time validation of terraform_data ([#455](#455)) ([a1c125b](a1c125b)) * **lib:** Use declared targetVersions for synth-time validation of the cloud backend ([#456](#456)) ([4a83507](4a83507)) ### Miscellaneous Chores * add OpenTofu and current Terraform to the jsii-terraform image ([#394](#394)) ([2f29d5a](2f29d5a)) * declare complete Nx build outputs so cache hits restore them ([#474](#474)) ([794cf77](794cf77)) * **deps:** move the remaining packages to TypeScript 6 ([#406](#406)) ([c55d360](c55d360)) * **deps:** move the root TypeScript to 6 ([#417](#417)) ([3cae1e2](3cae1e2)) * **deps:** Update xstate to v5.32.4 ([#333](#333)) ([5ebf74f](5ebf74f)) * **deps:** upgrade @sentry/node to 10.x ([#409](#409)) ([2714fff](2714fff)) * **deps:** use caret ranges for unbundled runtime dependencies ([#503](#503)) ([e722357](e722357)) * **docs:** add the test-audit skill for authoring and auditing tests ([#475](#475)) ([909c612](909c612)) * **examples:** move the TypeScript examples to tsx and TypeScript 6 ([#419](#419)) ([857e182](857e182)) * Flexible python version ([#420](#420)) ([955f62a](955f62a)) * **gha:** adopt the new jsii-terraform image and test Terraform 1.5.7 + 1.16.1 ([#398](#398)) ([3ad760b](3ad760b)) * **gha:** gate dependency upgrades on release age ([#418](#418)) ([b611a87](b611a87)) * **gha:** retire Docker Hub publishing of jsii-terraform ([#388](#388)) ([e53cddf](e53cddf)) * **gha:** select the CLI by binary name instead of concatenating a version ([#438](#438)) ([914da56](914da56)) * **gha:** skip the remote Nx cache in the release workflow ([#487](#487)) ([bce91da](bce91da)) * **lib:** stop publishing build state in the cdktn package ([#397](#397)) ([51e4a95](51e4a95)) * npm-check-updates && pnpm update ([#326](#326)) ([7095f6b](7095f6b)) * **readme:** point to the cdktn-io/action GitHub Action ([#504](#504)) ([e387852](e387852)), closes [#58](#58) * **tests:** add timeout handling to spawned commands ([#432](#432)) ([ddb4a8a](ddb4a8a)) * **tests:** run the function-binding scripts with tsx ([#431](#431)) ([fec67bf](fec67bf)) ### Tests * **lib:** ensure TerraformAsset stage within stack own directory ([#434](#434)) ([9a80366](9a80366)) * **lib:** move matcher Docker fixture to a provider version OpenTofu accepts ([#429](#429)) ([7b0da12](7b0da12)) * **tests:** move the python providers fixture to Docker provider 3.9.0 ([#440](#440)) ([a678473](a678473)) * **tests:** move the remaining kreuzwerker/docker pins off unsigned versions ([#459](#459)) ([d855cc1](d855cc1)) * **tests:** move the testing-matchers fixtures to Docker provider 3.x ([#436](#436)) ([8e2c60e](8e2c60e)) * **tests:** re-enable the Python edge integration test ([#404](#404)) ([0a80626](0a80626)) </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>
|
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. |
Description
Same class of bug as #474, in a package it did not cover. Nx restores only a target's declared
outputson a cache hit, and Depot runners serve remote hits for thebuildtargets.The edge provider bindings were a
postbuildside effect of@cdktn/provider-generator's build, written toedge-provider-bindings/, which the shared default outputs do not cover. On a cache hit the directory stayed empty, the integration prepare job uploaded nothing (No files were found with the provided path: packages/@cdktn/provider-generator/edge-provider-bindings), and everylinux_integrationjob failed withArtifact not found for name: edge-provider-bindings. Seen on every integration matrix job of #478 (https://github.com/open-constructs/cdk-terrain/actions/runs/37799770873). The Release workflow is not affected because its prepare jobs run onubuntu-latest; the PR integration prepare job runs on a Depot runner.Following review, the bindings now come from their own cached target instead of a
postbuildhook:build:edge-bindingson@cdktn/provider-generatordepends onbuild, declaresedge-provider-bindings/as its output, and namessrc/__tests__/edge-provider-schema/**as an input alongsideproductionand^production. Moving it out ofpostbuildalso keeps the five-language jsii-pacmak run off every localpnpm build.build:edge-bindingsscript runs it, and the three prepare jobs (integration.yml, bothrelease.ymlprepare jobs) call it afterpnpm run package, right before the upload.if-no-files-found: erroron the three edge-bindings uploads (integration.ymland bothrelease.ymlprepare jobs), so an empty upload fails the prepare job and the matrix is skipped instead of every job failing on download.test/package.json'sedge:installruns the target before copying, so the documented local flow (pnpm package, thenpnpm integration) andcd test && pnpm edge:installkeep working;test/Readme.mddescribes it.@cdktn/provider-generator'sbuildtarget adds the edge schema sources to its inputs, so the compiled generator can never come from a cache entry older than the sources the bindings target hashes (review finding by @sakul-learning and @jsteinich).The
cdktn-clioutput declaration from the first revision moved to #485, since makingbundle/cacheable needs its own inputs fixed first.Verification
In a clean worktree with Nx 22.7.9:
pnpm run build:edge-bindingsgeneratesedge-provider-bindings/{csharp,go,java,python,providers}.src/__tests__/edge-provider-schema/builder.tsre-runsbuild:edge-bindingswhile the upstream builds stay cached.pnpm run edge:installfromtest/builds the chain, generates the five language directories and copies them intotest/edge-provider-bindings.One observation from that testing: in this repo the negative patterns in the
productionnamed input do not exclude anything from task hashes. Editing a test file in@cdktn/commonsre-runs itsbuild, and spelling the exclusion several ways changes nothing, while a positive-only input list behaves correctly. So the stale-bindings scenario could not have occurred as described, because any__tests__edit already invalidatesbuild. The explicit input on the new target keeps that true by declaration. The broader effect, every test edit rebuilding every package, is pre-existing and not addressed here.Integration is left enabled on this PR so the Depot-runner prepare job demonstrates the artifact uploading again.
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01RvtnNBvNuTHU4dG3NFuBeb