Skip to content

test(deps): derive vended agentcore-cdk pin from the template - #2117

Merged
aidandaly24 merged 1 commit into
mainfrom
tj/dedupe-vended-cdk-pin
Aug 27, 2026
Merged

test(deps): derive vended agentcore-cdk pin from the template#2117
aidandaly24 merged 1 commit into
mainfrom
tj/dedupe-vended-cdk-pin

Conversation

@jariy17

@jariy17 jariy17 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

TL;DR

The @aws/agentcore-cdk pin was hand-typed in 3 places. This makes the tests read it from the one file that ships it, so a version bump stops being a find-and-replace chore.

Review time: ~5 min. Risk: low — tests only, no runtime src/ change. Motivating commit: ddb32a2 (#2087).

The problem

0.1.0-alpha.49 was duplicated in:

  • src/assets/cdk/package.json — the real pin (sync.ts treats it as source of truth)
  • plan.test.ts + sync.test.ts — hand-typed copies

Every prerelease bump had to edit all of them in lockstep. Miss one → red build, or a stale assertion that passes when it shouldn't.

What changed (3 files)

  1. New __tests__/fixtures.ts — exposes CDK_PIN read from the vended template, plus newerPrerelease() for skew tests. Reads via fs (not import) because src/assets is excluded from the TS program — same pattern the snapshot test and sync.ts already use.
  2. plan.test.ts — uses CDK_PIN / derived CDK_PIN_NEWER instead of the literals.
  3. sync.test.ts — uses CDK_PIN for its 5 pin literals.

Older-version fixtures (alpha.19) stay literal on purpose — they mean "much older", never bumped.

Why no new guardrail literal

The asset snapshot already stores the pin as a literal, so any unintended template change still trips a red snapshot test until someone regenerates it. No point duplicating that.

Verified

  • vitest run --project unit src/lib/dependency-management40 passed
  • npm run typecheck · eslint · prettier --check → clean

Next

Unblocks #2118 (auto-bump in the release flow). The pair was dry-run tested end-to-end in a private mirror: a release run auto-produced a PR bumping the pin alpha.48 → 49 with the snapshot refreshed and zero test edits.

@github-actions github-actions Bot added the size/s PR size: S label Aug 26, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Aug 26, 2026
@agentcore-devx-automation

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 Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.28.0.tgz

How to install

gh release download pr-2117-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.28.0.tgz

@agentcore-devx-automation agentcore-devx-automation Bot 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.

AgentCore Harness Review

Verdict: Looks good

This is a focused test refactor that removes duplicated hardcoded @aws/agentcore-cdk version strings by sourcing CDK_PIN from the vended src/assets/cdk/package.json (the actual source of truth) and deriving a strictly-newer prerelease for skew tests via newerPrerelease. The substitutions in plan.test.ts and sync.test.ts are 1:1 replacements of the previous literals (0.1.0-alpha.49 and 0.1.0-alpha.51), so behavior is preserved while future template bumps no longer require editing multiple test files.

A few observations, none blocking:

  • readFileSync in fixtures.ts runs at module load. Since vitest discovers only *.test.ts, fixtures.ts won't be picked up as a suite, and path.resolve(__dirname, '../../../assets/cdk/package.json') correctly points at src/assets/cdk/package.json from src/lib/dependency-management/__tests__/. The comment about src/assets being excluded from the TS program matches tsconfig.json.
  • newerPrerelease will throw if the CDK pin ever moves off a -alpha.N prerelease. That's an intentional fail-fast, but worth remembering as the vended package approaches a stable release — the fixture will need updating at that point.
  • The "upgrades an older exact-pinned prerelease" test still hardcodes 0.1.0-alpha.19 as the "older" version. That's fine as long as the pin never drops below alpha.20, which is a safe assumption.

No changes required. LGTM.

@agentcore-devx-automation agentcore-devx-automation Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Aug 26, 2026
The @aws/agentcore-cdk pin lived as a hand-typed literal in plan.test.ts and
sync.test.ts as well as in src/assets/cdk/package.json, so every prerelease
bump (e.g. #2087, alpha.48 -> alpha.49) had to edit the same string in all of
them. Read the pin from the vended template instead via a shared fixture, and
derive the skew-test's 'newer' example from it, so bumping the template no
longer touches these tests. The asset snapshot remains the literal guardrail
that trips on any unintended pin change.
@jariy17
jariy17 force-pushed the tj/dedupe-vended-cdk-pin branch from 2172854 to 85a851e Compare August 26, 2026 21:22
@github-actions github-actions Bot added size/s PR size: S and removed size/s PR size: S labels Aug 26, 2026
@jariy17
jariy17 marked this pull request as ready for review August 26, 2026 21:23
@jariy17
jariy17 requested a review from a team August 26, 2026 21:23
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Aug 26, 2026
@agentcore-devx-automation

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 Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 40.99% 15453 / 37694
🔵 Statements 40.24% 16473 / 40935
🔵 Functions 35.15% 2644 / 7522
🔵 Branches 34.18% 10311 / 30161
Generated in workflow #4421 for commit 85a851e by the Vitest Coverage Report Action

@github-actions github-actions Bot added size/s PR size: S and removed size/s PR size: S labels Aug 26, 2026
@aidandaly24
aidandaly24 merged commit d367f79 into main Aug 27, 2026
42 checks passed
@aidandaly24
aidandaly24 deleted the tj/dedupe-vended-cdk-pin branch August 27, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants