feat: manage vended CDK project dependencies with minor-version pinning#1777
Open
jesseturner21 wants to merge 1 commit into
Open
feat: manage vended CDK project dependencies with minor-version pinning#1777jesseturner21 wants to merge 1 commit into
jesseturner21 wants to merge 1 commit into
Conversation
Contributor
Package TarballHow to installgh release download pr-1777-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.24.1.tgz |
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
Coverage Report
|
483e37e to
d2eca29
Compare
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
d2eca29 to
360aca0
Compare
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Pin managed dependencies in the vended agentcore/cdk/package.json to tilde ranges (exact for @aws/agentcore-cdk), sync them to the CLI's tested versions during deploy preflight, auto-migrate pre-pinning projects, and error with an upgrade hint when the project was updated by a newer CLI. Opt-out via 'agentcore config disableDependencyManagement true'.
360aca0 to
12821fc
Compare
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
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.
Description
Problem
The CLI vends projects with caret (
^) dependency ranges inagentcore/cdk/package.json, so any upstream minor release (@aws/agentcore-cdk,aws-cdk-lib, …) can land in a customer's project without any CLI change — breaking template code or the CLI's ability to interact with the project. The CLI needs to own these versions on the customer's behalf while staying out of the way of dependencies the customer added themselves.Solution
Pin to minor, let patches float. Managed dependencies are written as tilde ranges (
~x.y.z);@aws/agentcore-cdk(L3 constructs, prerelease-versioned) is pinned exactly. A dependency is "managed" iff its name appears in the CLI's vended assetsrc/assets/cdk/package.json— that asset, baked into each CLI release, is the single source of truth for both the managed list and the known-good versions. Everything else in the customer's file is never touched.On every
agentcore deploy(headless and TUI), a preflight step:npm install -g @aws/agentcore-cli@latestand retry." Higher patch is fine — that's the point of tilde.file:bundled-tarball overrides,git:, …) are skipped with a warning, never rewritten.node_modules+ lockfile inagentcore/cdk/and fresh-install so the installed tree matches the new pins. No-op deploys stay fast.old → newper dep, plus "Dependencies you added yourself were not changed.").Opt-out:
agentcore config disableDependencyManagement trueskips migrate/sync/reinstall entirely; the skew check still runs but downgrades to a warning and deploy proceeds — an escape hatch if we ever ship a bad pin, and support for teams with their own dependency tooling.High-level implementation
src/lib/dependency-management/(new, self-contained) — one deep module behind a single function,syncManagedDependencies({vendedPackageJsonPath, projectDir, disabled}), designed to lift into the planned CLI refactor unchanged (zero imports fromsrc/cli/). Internals:semver.ts(prerelease-aware parse/compare — the existingparseSemVerdrops-alpha.Ntags, which here would make an alpha downgrade look like a no-op),policy.ts(pure plan computation),messages.ts(single source of user-facing wording), and the rewrite/clean-reinstall logic. All notices/warnings are returned as data on the result; callers only display them, so the headless and TUI paths can't drift.CliVersionTooOldError(errorSource: 'user') andDependencySyncError('client') insrc/lib/errors/types.ts, registered in the telemetryErrorNameenum. All failure paths throw typed errors — neverprocess.exit()— so telemetry classification keeps working.src/assets/cdk/package.jsonconverted to the policy form, with tilde bases bumped to what today's caret ranges actually resolve to (e.g.constructs ^10.0.0→~10.7.0, not~10.0.0) so migration never silently downgrades an existing project.@aws/agentcore-cdkexact-pinned to0.1.0-alpha.45.ensureManagedDependencies()adapter (src/cli/operations/deploy/dependency-sync.ts) resolves the vended asset viagetTemplatePathand reads the global-config opt-out; called as a "Sync CDK dependencies" step between validate and build inhandleDeploy(actions.ts) and TUI preflight (useCdkPreflight.ts). The summary rides onDeployResult(so--jsongets structured data; the human notice goes throughonNotice, suppressed under--json), renders in the TUI DeployScreen, and feeds five newdep_sync_*deploy telemetry attributes.disableDependencyManagementadded toGlobalConfigSchemaStrict; the genericagentcore config <key> <value>command handles it with no command changes.Related Issue
Closes #1540
Documentation PR
N/A — behavior is self-describing via the deploy-time notices; can follow up in agent-docs if wanted.
Type of Change
Testing
33 new unit tests in
src/lib/dependency-management/__tests__/:sync.test.ts— end-to-end through the public function against a real temp project dir (only the npm subprocess mocked), one test per design-doc behavior: no-op deploy touches nothing and prints nothing; caret project migrates + reinstalls (node_modules/lockfile actually deleted,npm installinvoked, migration notice includes the opt-out hint) while a user-addedlodashsurvives untouched; higher-minor project throwsCliVersionTooOldErrorwith the upgrade wording; opted-out skew warns and leaves the file byte-identical; deleted managed dep is restored;file:bundled-agentcore-cdk.tgzoverride is skipped with a warning; npm failure / unreadable package.json wrap inDependencySyncError; rewrite preserves user key order and unknown fields.policy.test.ts— plan computation: skew on higher minor and major but not higher patch; prerelease-aware skew on the exact pin (alpha.50 > alpha.45detected, older alpha upgraded); devDependencies managed and the user's section respected; user deps never touched.semver.test.ts— prerelease ordering (0.1.0-alpha.19 < 0.1.0-alpha.20 < 0.1.0), specifier classification (exact/tilde/caret), and unsupported forms (file:,git+https:,workspace:,*,latest, compound ranges).I ran
npm run test:unitandnpm run test:integ(unit: 411 files / 5944 tests pass; integ not run — no integ coverage touched)I ran
npm run typecheckI ran
npm run lintIf I modified
src/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.