fix: fallback to legacy stack name in deployed state - #2248
Conversation
agentcore-devx-automation
Bot
added
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
![agentcore-devx-automation[bot]](https://avatars.githubusercontent.com/u/2232217?s=60&v=4){kind=small}
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
The fix is small, focused, and correct:
ResourceStateSchema.stackNameis added as optional, so pre-existing legacy state files continue to parse and — thanks to the surroundingpassthrough()— the value is preserved across read-modify-write cycles until a new deploy records astackArnthat will shadow it.- Both
resolveDeployedResourcesandresolveProjectResourcesfall back torecorded.resources.stackNameonly whenstackArnis absent, which matches the "legacy → new" upgrade direction.describeStackaccepts either a name or ARN, so the fallback works without further changes. - I checked the other callers of
readDeployedStateincdk.ts(the credentials read at line 223 andteardown) — neither depends on the stack reference in a way that needs the same fallback, so the change is complete. - Tests exercise the new legacy path via the real
deployed-state.json(temp dir +updateTargetState), only mocking the stack describer, which is the right seam.
Nothing blocking from me.
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2248 +/- ##
=========================================
Coverage 97.04% 97.04%
=========================================
Files 566 566
Lines 39409 39412 +3
=========================================
+ Hits 38244 38247 +3
Misses 1165 1165 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
aidandaly24
left a comment
aidandaly24
left a comment
There was a problem hiding this comment.
One small nit but looks good to me
| const { spec } = project; | ||
| const deployedState = await readDeployedState(this.json, project.rootPath); | ||
| const recorded = deployedState.targets[target.name]; | ||
| const stackReference = recorded?.stackArn ?? recorded?.resources?.stackName; |
There was a problem hiding this comment.
nit: Would it make sense to pull this fallback into deployedState.ts next to the field it reads, something like stackReferenceOf(recorded)? The same expression now appears here and in resolveProjectResources, and the reason it exists is only explained by the schema comment on stackName. Keeping the legacy rule beside the schema means the next reader of the stack binding, like the delete-and-recreate check the stackArn comment describes, gets the fallback without having to know it exists.
| }); | ||
|
|
||
| test("fails without reading AWS when the target has no deployed stack ARN", async () => { | ||
| test("resolves resources from a legacy stack name when the target has no stack ARN", async () => { |
There was a problem hiding this comment.
These tests appear to be asserting on implementations rather than behaviors. Would it be possible to test at the handler level, such that we create a project, insert the legacy stackName field into deployedState, then ensure the status command works as expected?
| const { spec } = project; | ||
| const deployedState = await readDeployedState(this.json, project.rootPath); | ||
| const recorded = deployedState.targets[target.name]; | ||
| const stackReference = recorded?.stackArn ?? recorded?.resources?.stackName; |
4 participants
Description
The refactored CLI updated deployedState.ts to track
stackArn, where our legacy/main deployedState.ts instead trackedstackName. Therefore, inspecting a legacy project with the new CLI resulted in deployed resources showing as 'local-only'.This PR addresses this concern by adding
stackNameas a fallback in our resource resolution logic. This change allows legacy projects to be inspected correctly through the new CLI, without the need for redeploying resources.Example
This example shows a legacy project with two deployed harnesses in the project. The recording shows the following commands:
agentcore-> this is the currently released RC of our refactor branch installed via npm.acr-dev-> alias pointing to my local clone and dev workspace of the CLI. Built from refactor branch including the changes present in this PRScreen.Recording.2026-09-08.at.11.52.03.AM.mov
Type of Change
Testing
How have you tested the change?
Manually verified as shown in screen recording above. Additionally, added a unit test asserting that a legacy stack name is resolved correctly when the target does not have a stack ARN. Focused unit test passed.
bun run test(3180 pass, 0 fail)npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/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.