feat: V1 - #14
Open
aripalo wants to merge 12 commits into
Open
Conversation
Extract the versions and commands that were duplicated between this repository's own .projenrc.ts and the AlmaCdkConstructLibrary project type into a single module, so the root project and downstream projects share one source of truth. CDK_DEFAULT_VERSION and CONSTRUCTS_DEFAULT_VERSION move out of src/schemas/almaCdkConstructLibraryOptions.ts and keep their public names, so this is not an API change: jsii ignores module-level constants, and both API.md and .jsii are unaffected. Purely mechanical: every value is identical, and the existing snapshot passes untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pnpmVersion only reached the generated workflows, so a local `pnpm` in the project directory kept resolving to whatever version was installed globally. Write the package.json `packageManager` field as well: pnpm self-downloads and runs that exact version (`managePackageManagerVersions` is on by default since pnpm v10), so local runs and CI now agree. Both values derive from the single PNPM_VERSION constant, so they cannot drift. The field is set in NodeConfig, which both this repository's .projenrc.ts and AlmaCdkConstructLibrary instantiate, so downstream projects get the same pin. Migrating onlyBuiltDependencies to allowBuilds is part of the same change: pnpm v11 deprecates the former and ignores it, which with strictDepBuilds made `pnpm i` fail with ERR_PNPM_IGNORED_BUILDS for unrs-resolver. That would have broken CI as soon as the workflows moved to pnpm 11, independently of the local pin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
managePackageManagerVersions downloads the pinned pnpm, but nothing catches the paths where that did not happen: a contributor who disabled the setting, or a stale corepack pin. Enabling packageManagerStrictVersion turns those into a hard failure instead of silently installing with a different pnpm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pnpm v11 does not implement this setting: switchCliVersion() runs the exact version from the `packageManager` field unconditionally – it switches down as readily as up, ignores manage-package-manager-versions=false, and errors out when the field names another package manager. The string "strictVersion" no longer appears anywhere in the pnpm 11.19.0 distribution. The safety net it was meant to add is therefore already built in, and writing the setting into every generated pnpm-workspace.yaml only implied a guarantee that pnpm silently ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pnpm v11 no longer implements these, so accepting them let downstream projects write pnpm-workspace.yaml entries that are silently ignored – the failure mode we hit with onlyBuiltDependencies, where the build was skipped and strictDepBuilds failed the install. Superseded by allowBuilds (which pnpm documents as replacing them, and which is the only key its createAllowBuildFunction consults besides dangerouslyAllowAllBuilds): onlyBuiltDependencies, onlyBuiltDependenciesFile, ignoredBuiltDependencies, neverBuiltDependencies Now enforced unconditionally by pnpm, so no longer configurable: packageManagerStrict, packageManagerStrictVersion, managePackageManagerVersions Renamed or removed, with their replacements still present: allowNonAppliedPatches (-> allowUnusedPatches), ignorePatchFailures, ignoreDepScripts (-> ignoreScripts), useNodeVersion (-> nodeVersion) Every removal was checked against the pnpm 11.19.0 distribution: none of these names appear in it, in either camelCase or kebab-case. The public pnpmSettings type still comes from the upstream JSON schema, so these keys remain type-visible and are now rejected at synth instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restores the guard dropped in 6baea34. That commit was right that pnpm v11 never reads packageManagerStrictVersion, and wrong that this makes the setting pointless: it is read by whatever pnpm the project is launched with, which is precisely when it earns its place. Verified against a project pinned to pnpm@11.19.0: pnpm 10.33.4, self-switching disabled -> ERROR This project is configured to use v11.19.0 of pnpm. Your current pnpm is v10.33.4 same, guard absent -> silently installs as 10.33.4 Written to .npmrc rather than pnpm-workspace.yaml because pnpm v9 reads settings only from .npmrc – it rejects a settings-only workspace file with "packages field missing or empty" – while v10.16+ honours either and v11 ignores both. One entry therefore covers every version that implements it. Also restores packageManagerStrict, packageManagerStrictVersion and managePackageManagerVersions to the pnpmSettings schema, for the same reason: they are evaluated by the launching pnpm, so "absent from v11" was the wrong test for them. The install-time settings pruned in 239ea49 stay out, since a mismatched client is now stopped before it can install. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 32 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe project centralizes tool versions, updates pnpm workspace and TypeScript configuration, modernizes generated tasks, upgrades CI and release workflows, and refreshes API and upgrade documentation. ChangesToolchain modernization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant BuildJob
participant ArtifactStorage
participant GitHubReleaseJob
participant NpmReleaseJob
ReleaseWorkflow->>BuildJob: start release build
BuildJob->>ArtifactStorage: upload build artifact
ArtifactStorage-->>BuildJob: return artifact_id
BuildJob-->>ReleaseWorkflow: expose artifact_id
ReleaseWorkflow->>GitHubReleaseJob: pass artifact_id and dry_run
GitHubReleaseJob->>ArtifactStorage: download artifact by artifact_id
ReleaseWorkflow->>NpmReleaseJob: pass artifact_id and dry_run
NpmReleaseJob->>ArtifactStorage: download artifact by artifact_id
GitHubReleaseJob-->>ReleaseWorkflow: skip publishing when dry_run is enabled
NpmReleaseJob-->>ReleaseWorkflow: pass dry-run mode to publib
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Projen's `NodePackage` unconditionally declares `pnpm-workspace.yaml` for every pnpm project (via `javascript.PnpmWorkspaceYaml`), so writing that file from our own component now aborts synthesis with "There is already a file under pnpm-workspace.yaml" -- a construct-tree uniqueness check, not a refusal to overwrite anything on disk. Move the hardened workspace defaults into `buildPnpmWorkspaceYamlOptions()` and feed them in as `pnpmOptions.workspaceYamlOptions`. That is the only supported route: projen creates its file with `omitEmpty`, and raw overrides are dropped whenever the base object resolves to nothing, so `addOverride` would silently delete the file instead of extending it. `NodeConfig` keeps the rest of the toolchain pinning (.nvmrc, the `packageManager` field, and the .npmrc strict-version guard). Also part of this upgrade: - bump jsii/jsii-rosetta to ~6.0.7 and the minimum Node version to 22 - adopt projen 0.101's TypeScript config layout, where tsconfig.json, test/tsconfig.json and projenrc/tsconfig.json replace tsconfig.dev.json - refresh assertions that the upgrade made stale (codecov-action v6, Node 22) and the project snapshot Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aripalo
marked this pull request as ready for review
August 5, 2026 10:22
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@API.md`:
- Around line 551-554: Update the dependency-resolution explanation in API.md to
state that npm install can produce multiple concrete constructs versions when
dependency ranges conflict, alongside manual symlinks and monorepo tooling.
Preserve the existing instanceof warning while removing the claim that npm
install cannot create duplicate constructs installations.
In `@README.md`:
- Line 82: Update the README’s generated-files guidance to include all
Projen-managed outputs, specifically .projen/deps.json, .projen/files.json, and
.projen/tasks.json, alongside package.json, .github/workflows/*, and
pnpm-lock.yaml; instruct users to commit changes to these generated files.
In `@src/constants.ts`:
- Around line 43-47: Update CONSTRUCTS_DEFAULT_VERSION in constants.ts to the
intended V1 constructs version, 10.7.2, so omitted constructsVersion values and
generated peer dependencies use ^10.7.2. Refresh the generated .projen/deps.json
and the affected test snapshot, leaving CDK_DEFAULT_VERSION unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca793749-fe68-4134-baa9-c2e32757a0bd
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltest/__snapshots__/AlmaCdkConstructLibrary.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (31)
.eslintrc.json.gitattributes.github/workflows/build.yml.github/workflows/pull-request-lint.yml.github/workflows/release.yml.github/workflows/upgrade-main.yml.gitignore.mergify.yml.npmignore.npmrc.projen/deps.json.projen/files.json.projen/tasks.json.projenrc.tsAPI.mdREADME.mdpackage.jsonpnpm-workspace.yamlprojenrc/tsconfig.jsonsrc/AlmaCdkConstructLibrary.tssrc/NodeConfig.tssrc/constants.tssrc/index.tssrc/pnpmWorkspaceSettings.tssrc/schemas/almaCdkConstructLibraryOptions.tssrc/schemas/pnpmSettings.tstest/AlmaCdkConstructLibrary.test.tstest/pnpmWorkspaceSettings.test.tstest/schemas/almaCdkConstructLibraryOptions.test.tstest/tsconfig.jsontsconfig.json
`NODEJS_WORKFLOW_VERSION` was an alias of `NODEJS_MAX_VERSION`, so widening the supported range would also have moved every generated workflow onto Node 26. Give it its own value so the supported ceiling and the version CI builds on can move independently; workflows and .nvmrc stay on 24. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
src/constant.tsto hold values used by.projenrc.ts(this project itself) andsrc/AlmaCdkConstructLibrary.ts(downstream projects)v11constructs@v10.7Projen@0.101.xandjsii@6toolchainv22(since v20 is EOL'd)v1