feat(project): resolve all deployed resource types (prereq for project status) - #2158
Closed
jariy17 wants to merge 6 commits into
Closed
feat(project): resolve all deployed resource types (prereq for project status)#2158jariy17 wants to merge 6 commits into
project status)#2158jariy17 wants to merge 6 commits into
Conversation
agentcore-devx-automation
Bot
added
agentcore-harness-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2158 +/- ##
============================================
- Coverage 97.25% 97.25% -0.01%
============================================
Files 508 508
Lines 33902 33974 +72
============================================
+ Hits 32972 33041 +69
- Misses 930 933 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
![agentcore-devx-automation[bot]](https://avatars.githubusercontent.com/u/2232217?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice cleanup. The changes are cohesive and the design choices around graceful fallback are sensible:
- The
satisfies Record<CfnOutputResource, …>onEXPORT_PARTSgives us a compile-time guarantee that newDeployableResourcevariants can't silently resolve to "not found" — good future-proofing. - The
.flatMapthat drops resources without an id, combined with the explicit TODOs oncapacity-providerandpayment, makes the speculative export-name shapes for not-yet-emitted outputs (KnowledgeBase-*-Id,GatewayTarget-*-Id,ConfigBundle-*-Id,Dataset-*-Id,Payment…ManagerId,CapacityProvider-*-Id) safe: wrong guesses will manifest as an omitted row, not a crash. - Tests use temp dirs and only mock at real I/O boundaries (CDK runner, credential resolver,
describeStack). Not excessive. - No telemetry needed — this PR is an internal API refactor with no new user-facing surface (
allowMissingisn't wired to a consumer yet).
Two minor observations, not blockers:
EXPORT_PARTS["gateway-target"]ignores theparentgateway name (unlikepolicy, which threads it in). If the eventual CDK output for gateway targets includes the parent gateway (which it likely will, to avoid collisions when two gateways declare targets with the same name), this row will need updating along with l3-cdk-constructs#336. Fine to defer; just worth having on the radar when that output ships.- The
Payment${toCdkId(name)}ManagerIdOutputKeyformat is currently a prediction — noCfnOutputemitting it exists in eitherassets/cdk/lib/cdk-stack.tsor the L3 constructs. The comment acknowledges this, and it fails soft, so OK.
No changes required.
jariy17
marked this pull request as draft
September 1, 2026 18:15
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
added 3 commits
September 1, 2026 21:08
…+harnesses Broaden ProjectManager.resolveDeployedResources so it returns every deployed resource (memory, knowledge-base, credential, evaluator, online-eval, gateway, gateway-target, policy-engine, policy, config-bundle, dataset, payment) with its physical id, alongside the existing runtimes and harnesses. - Widen DeployedProjectResource.resourceType to a new DeployableResource union; add optional parent (policy->engine, gateway-target->gateway). - Generalize findDeployedResourceId with an EXPORT_PARTS table typed 'satisfies Record<CfnOutputResource, ...>' so a missing type is a COMPILE error, never a silent miss. ExportName format replicated from @aws/agentcore-cdk logical-ids (not importable: CDK lib, not a CLI dep). - credential ids come from the deployed-state credentials map (never a stack output); payment matches by OutputKey (its CfnOutputs set no ExportName) with a TODO to add exportName in the cdk and fold it in. - Add allowMissing so an undeployed target yields [] instead of throwing; deploy and remove keep the hard failure. Stack resolution unchanged (stackArn-from-state). - Guard the invoke picker to only list runtime/harness now that the resolver returns more types. This function will back the new 'agentcore project status' handler, which returns a JSON status of the project's deployed resources to the customer.
Datasets are out of scope for 'agentcore project status', so remove them from the resolver: the DeployableResource union, the EXPORT_PARTS table, the spec iteration, and the resolver test. The 'satisfies Record<CfnOutputResource, ...>' guard proves the union and the table stayed in sync after the removal.
findDeployedResourceId only knew CloudFormation exports, so its two exceptions leaked outward: payment matched by OutputKey in an early return, and credential -- which is never a stack output at all -- was branched on by the caller. "Where does this id come from" lived in three places. Fold all fourteen types into one resolveResourceId switch that takes both sources (the stack and deployed-state's credential ARNs). The `never` default makes a new DeployableResource a compile error instead of a resource that silently vanishes from `project status`. Renamed off find* because it no longer only searches the stack. Export name literals are unchanged -- each was verified against a real stack, so they are deliberately not derived from the resourceType.
jariy17
force-pushed
the
feat/resolve-deployed-resources-all-types
branch
from
September 1, 2026 21:14
eb01111 to
4d64751
Compare
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
resolveResourceId had a single call site and took a two-source parameter object (stack + credentialArns) purely to reach values that were already locals there. Closing over them instead removes the parameter object.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
project status surfaces these to customers, where an ARN is the useful identifier. gateway-target stays on its id: AgentCoreMcp exports no -Arn for it yet.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
Contributor
Author
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
2 participants
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.
No description provided.