Skip to content

feat: manage vended CDK project dependencies with minor-version pinning#1777

Open
jesseturner21 wants to merge 13 commits into
mainfrom
feat/managed-dependency-pinning
Open

feat: manage vended CDK project dependencies with minor-version pinning#1777
jesseturner21 wants to merge 13 commits into
mainfrom
feat/managed-dependency-pinning

Conversation

@jesseturner21

@jesseturner21 jesseturner21 commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Problem

The CLI vends projects with caret (^) dependency ranges in agentcore/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 asset src/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:

  1. Skew check — if the project declares a managed dep with a higher minor/major than this CLI expects (e.g. a teammate on a newer CLI updated it), throw: "This project requires a newer version of the AgentCore CLI. Run npm install -g @aws/agentcore@latest and retry." Higher patch is fine — that's the point of tilde.
  2. Migrate — projects created before pinning (caret ranges) are rewritten to the pinned form with a one-time explanation.
  3. Sync — managed deps are updated to the current CLI's values; user-added deps are left byte-for-byte untouched; non-semver specifiers (file: bundled-tarball overrides, git:, …) are skipped with a warning, never rewritten.
  4. Install (only if something changed) — run an incremental npm install in agentcore/cdk/ that reconciles the edited package.json against the existing lockfile; node_modules and the lockfile are never deleted, so user-added deps keep their resolved versions and installs stay warm. The install also runs as a self-heal when node_modules is missing even with a matching manifest. If the install fails, the pre-rewrite package.json is restored so a retry recomputes the same plan and re-attempts the install. No-op deploys stay fast.
  5. Report — print a summary of every change (old → new per dep, plus "Dependencies you added yourself were not changed.").

Preview modes: --dry-run and --diff run the sync in check-only mode — the notice reports what a real deploy would change (future tense), but nothing is written and no install runs.

Teardown: teardown deploys are never blocked by the sync — both skew and write/install failures downgrade to warnings, so dependency pinning can never prevent destroying a cost-incurring stack.

Opt-out: agentcore config disableDependencyManagement true skips 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 from src/cli/). All notices/warnings are returned as data on the result; callers only display them, so the headless and TUI paths can't drift.
  • Vended asset pinned — src/assets/cdk/package.json converted 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.
  • Deploy integration — ensureManagedDependencies() adapter (src/cli/operations/deploy/dependency-sync.ts) resolves the vended asset via getTemplatePath and reads the global-config opt-out; called as a "Sync CDK dependencies" step between validate and build in handleDeploy (actions.ts) and TUI preflight (useCdkPreflight.ts).
  • Config — disableDependencyManagement added to GlobalConfigSchemaStrict; the generic agentcore 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

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Testing

40 new unit tests in src/lib/dependency-management/__tests__/ (16 sync + 12 policy + 12 semver), plus new useDevDeploy cases for the dev-flow notice/warnings:

  • I ran npm run test:unit and npm run test:integ (unit: 411 files / 5956 tests pass; integ not run — no integ coverage touched)
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@jesseturner21
jesseturner21 requested a review from a team July 16, 2026 19:46
@github-actions github-actions Bot added size/xl PR size: XL agentcore-harness-reviewing AgentCore Harness review in progress labels Jul 16, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 16, 2026
@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.24.1.tgz

How to install

gh 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

@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 40.3% 15047 / 37333
🔵 Statements 39.56% 16037 / 40535
🔵 Functions 34.42% 2567 / 7457
🔵 Branches 33.71% 10016 / 29710
Generated in workflow #4161 for commit 2543dd7 by the Vitest Coverage Report Action

@jesseturner21
jesseturner21 force-pushed the feat/managed-dependency-pinning branch from 483e37e to d2eca29 Compare July 16, 2026 20:21
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Jul 16, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 16, 2026
@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 16, 2026
@jesseturner21
jesseturner21 force-pushed the feat/managed-dependency-pinning branch from d2eca29 to 360aca0 Compare July 16, 2026 21:50
@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 17, 2026
@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

Hweinstock
Hweinstock reviewed Jul 20, 2026
View reviewed changes

@Hweinstock Hweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work! I like how most of the functionality was isolated to the module. Few questions, but overall makes sense to me. I do think this is worth bug-bashing since it feels pretty high-risk.

Comment thread src/lib/dependency-management/semver.ts
Comment thread src/lib/dependency-management/types.ts
Comment thread src/cli/commands/deploy/actions.ts Outdated
Comment thread e2e-tests/dependency-sync.test.ts
Comment thread src/lib/dependency-management/sync.ts Outdated
Comment thread src/cli/commands/deploy/actions.ts
jariy17
jariy17 previously approved these changes Jul 21, 2026
View reviewed changes

@jariy17 jariy17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me

@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@jesseturner21
feat: manage vended CDK project dependencies with minor-version pinning
e20763a 
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'.
@jesseturner21
fix: address review findings — correct install command, non-destructi…
c900b7e 
…ve sync, preview/teardown safety, surfaced warnings, dev notices, failure telemetry

- messages.ts: drop the wrong @aws/agentcore-cli package name; install command is now
  threaded in from the CLI adapter (getDistroConfig().installCommand) with a standalone
  default, keeping src/lib free of src/cli imports
- index.ts: never delete node_modules or the lockfile — npm reconciles the edited
  package.json incrementally; install also runs when node_modules is missing so a
  previously failed install self-heals on retry
- skew error now names the skewed deps, uses the distro-aware install command, and
  points at the disableDependencyManagement opt-out for intentional manual bumps
- mode: 'check' computes the plan/warnings/notice (future tense) without writing or
  installing; deploy --dry-run/--diff (CLI + TUI diffMode) run check-only, and teardown
  deploys downgrade skew to a warning so it can't block destroying a stack
- dep-sync warnings now reach the terminal via postDeployWarnings (CLI) and a rendered
  warnings block (DeployScreen); dev flows print/render the notice + warnings too
  (runCliDeploy console output, useDevDeploy reads them from the deploy result instead
  of multiplexing onNotice, DevScreen renders them in their own block)
- depSync hoisted out of the try in handleDeploy and carried on the failure result;
  command.tsx records dep_sync_* attrs regardless of success via a shared
  toDepSyncAttrs() also used by useDeployFlow
- shared SYNC_CDK_DEPENDENCIES_STEP constant; sync skipped under AGENTCORE_SKIP_INSTALL
@jesseturner21
fix: keep dependency-sync warnings out of postDeployWarnings so infor…
834ba2e 
…mational warnings don't exit 2

The bundled-tarball override (file:bundled-agentcore-cdk.tgz) always yields a
'left unmanaged' dep-sync warning in e2e/dev builds. Merging depSync.warnings
into postDeployWarnings made every such deploy exit 2, failing e2e deploy
assertions. Dep-sync warnings are informational: print them from
printDeployResult (headless CLI) instead — the TUI and dev paths already
render result.dependencySync.warnings separately.
@jesseturner21
fix: carry depSync on failure returns, restore manifest on install fa…
6f28ed7 
…ilure, persist dev notice, print warnings on failed deploys, never block teardown

Round-2 review fixes for managed dependency pinning (#1540):

- handleDeploy: every explicit failure return after the sync step now
  carries dependencySync, so dep_sync_* telemetry and the rewrite notice
  survive deploys that fail at a later step (not just the catch path).
- syncManagedDependencies: on npm install failure, restore the pre-rewrite
  package.json before throwing DependencySyncError — otherwise a retry sees
  a matching manifest plus a stale node_modules, skips the install, and
  deploys against the old installed tree (the exact skew the feature
  prevents). Tests pin restore + successful retry.
- Dev TUI: the dep-sync notice/warnings arrived in the same render batch
  that transitioned out of 'deploying', so they were visible for at most
  one frame. They now persist into the post-deploy modes (harness chooser /
  InvokeScreen) and print to the normal buffer on the browser-launch path.
- Headless CLI: the non-JSON failure branch now prints
  dependencySync.notice/warnings to stderr, so a failed deploy still
  explains the package.json rewrite and any downgraded skew.
- Teardown: DependencySyncError (write/install failure) at both sync call
  sites is downgraded to a warning via teardownSyncFailureResult and the
  teardown proceeds — extending the treatSkewAsWarning invariant that
  nothing about pinning may block destroying a cost-incurring stack.
@jesseturner21
fix: don't re-print dep-sync notice on failed deploys — onNotice alre…
cc64e14 
…ady printed it live
@jesseturner21
refactor: rename dep-sync module files and fields per review; add e2e…
b476bc6 
… suite

- policy.ts -> plan.ts, index.ts -> sync.ts with thin re-export barrel
- DependencySyncResult.migrated -> migratedFromCaret (consistent with SyncPlan)
- SKIPPED_SYNC_RESULT -> NO_OP_SYNC_RESULT via noOpSyncResult factory
- DeployResult.dependencySync -> dependencySyncResult; depSync local -> shorthand
- move toDepSyncAttrs into operations/deploy/dependency-sync.ts
- readManifest 'what' param -> fileDescription; install -> installResult
- drop #1540 issue references from comments
- add e2e-tests/dependency-sync.test.ts (migration+real deploy, upgrade,
  skew failure, opt-out warning)
@jesseturner21
refactor: address review round 1
2191a9a 
- add outcome discriminant to DependencySyncResult (synced/check-only/
  opted-out/skipped/failure-suppressed/failed) + dep_sync_outcome telemetry
- attach failedSyncResult on sync-step failures so telemetry records them
- reset dep-sync state in startPreflight retries
- accept v-prefixed semver versions (v1.2.3, ~v1.2.3)
- replace NO_OP_SYNC_RESULT singleton with per-call factory
- warn (not success) on suppressed teardown sync failure in headless progress
- PreflightResult.dependencySync -> dependencySyncResult
- export PackageManifest from barrel; drop redundant type re-exports
- delete dead formatVersion; fix orphaned doc comment
- retitle e2e opt-out test to what it actually proves; drop stale #1540 ref
@jesseturner21
polish: address final review nits
52c102a 
- TUI logs warn (not success) for suppressed teardown sync failure
- dedupe suppressed-failure warning line in deploy log
- e2e: add 'failed' outcome to DepSyncJson union + assert it in skew test
- add unit tests for CLI-layer sync factories, skip branch, toDepSyncAttrs,
  and disabled-wins-over-check outcome precedence
- fix outcome doc overclaim; rename endStep error param to message
@jesseturner21
refactor: replace non-null assertion with ??= when applying planned c…
af5569e 
…hanges

Addresses review feedback on whether the section is guaranteed to exist.
It is (a change is only planned for a section findDeclared located), but
??= keeps the write safe if that invariant ever changes and matches the
restored-deps loop below.
@jesseturner21
refactor: delegate semver parse/compare to node-semver
47bf613 
Per review: node-semver covers parsing and full-precedence comparison
(incl. prerelease ordering), so use it for that and delete the hand-rolled
comparator. Keep the thin prefix classifier — node-semver's Range API
expands ~/^ to comparators and erases the specifier kind the sync policy
needs. semver was already in the transitive tree; the CLI bundle absorbs it.
@jesseturner21
refactor: centralize deploy failure returns in fail() helper
30b57de 
Per review: threading dependencySyncResult through every failure return
was fragile. fail(error) closes over logger and the current
dependencySyncResult and performs the shared ritual (finalize log, attach
logPath + sync result), collapsing 11 failure sites — forgetting the
dep_sync_* telemetry field is no longer possible. endStep('error') and
per-site logging stay at call sites; success returns stay explicit.
@jesseturner21
refactor: drop compareVersions pass-through wrapper
95aa9a3 
plan.ts imports node-semver's compare directly; parseVersion stays as it
carries real policy (strict mode keeps wildcards/ranges unsupported, and
normalizes the uppercase-V form npm accepts but node-semver rejects).
@jesseturner21
fix: bump transitive deps to clear npm audit high findings
2543dd7 
npm audit fix — resolves the js-yaml quadratic-CPU advisories
(GHSA-h67p-54hq-rp68 / GHSA-52cp-r559-cp3m, 4.1.1 -> 4.3.0) and the
@babel/core sourceMappingURL file-read advisory; remaining findings are
moderate/low, below the security:audit --audit-level=high gate.
@agentcore-devx-automation

agentcore-devx-automation Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Hweinstock Hweinstock Hweinstock left review comments
@jariy17 jariy17 jariy17 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

size/xl PR size: XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: CLI should manage dependencies of created projects.

3 participants

@jesseturner21 @Hweinstock @jariy17