test(deps): derive vended agentcore-cdk pin from the template - #2117
Merged
Conversation
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
Package TarballHow to installgh 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 |
Contributor
There was a problem hiding this comment.
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:
readFileSyncinfixtures.tsruns at module load. Since vitest discovers only*.test.ts,fixtures.tswon't be picked up as a suite, andpath.resolve(__dirname, '../../../assets/cdk/package.json')correctly points atsrc/assets/cdk/package.jsonfromsrc/lib/dependency-management/__tests__/. The comment aboutsrc/assetsbeing excluded from the TS program matchestsconfig.json.newerPrereleasewill throw if the CDK pin ever moves off a-alpha.Nprerelease. 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.19as 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.
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
force-pushed
the
tj/dedupe-vended-cdk-pin
branch
from
August 26, 2026 21:22
2172854 to
85a851e
Compare
jariy17
marked this pull request as ready for review
August 26, 2026 21:23
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
Coverage Report
|
aidandaly24
approved these changes
Aug 27, 2026
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.
TL;DR
The
@aws/agentcore-cdkpin 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.49was duplicated in:src/assets/cdk/package.json— the real pin (sync.tstreats it as source of truth)plan.test.ts+sync.test.ts— hand-typed copiesEvery 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)
__tests__/fixtures.ts— exposesCDK_PINread from the vended template, plusnewerPrerelease()for skew tests. Reads viafs(notimport) becausesrc/assetsis excluded from the TS program — same pattern the snapshot test andsync.tsalready use.plan.test.ts— usesCDK_PIN/ derivedCDK_PIN_NEWERinstead of the literals.sync.test.ts— usesCDK_PINfor 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-management→ 40 passednpm run typecheck·eslint·prettier --check→ cleanNext
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 → 49with the snapshot refreshed and zero test edits.