feat(project): implement project status - #2165
Conversation
agentcore-devx-automation
Bot
added
agentcore-harness-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
Reviewed commit d3997453 per the PR description (rest of the branch belongs to #2158).
What I checked
src/handlers/project/status/index.ts– handler logicsrc/handlers/project/index.ts–withProjectwrapping matches the deploy/build/remove/dev patternsrc/handlers/project/project.test.ts– the stub test replacement asserts thewithProjectwiringsrc/handlers/project/status/index.test.ts– four handler-level tests throughroot.route
Notes
describe()'s spec walk mirrors the declared set inCdkBackend.resolveDeployedResources, and every field it iterates is either.default([])in the schema or guarded with?? [](unassignedTargets,payments). No undefined‑array footguns.- The composite
key(resourceType, name, parent)used to match resolver rows against declared rows is symmetric with what the CDK backend produces, so a resource returned by the resolver will always match its declared row. - Tests keep the real
FsProjectManagerand scaffold a real project in a temp directory, stubbing only at theProjectBackendboundary — matches the guidance on avoiding excessive mocking. - Telemetry isn't wired into sibling handlers (
deploy,build,remove, etc.), so its absence here isn't a regression against an existing pattern. - Scope caveats (
pending-removal, datasets,capacity-providerpending l3-cdk-constructs#336,gateway-targetreported by Id rather than Arn) are all documented in code or the PR description.
No blocking issues. Ready to merge once #2158 lands.
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2165 +/- ##
============================================
- Coverage 97.16% 97.16% -0.01%
============================================
Files 519 519
Lines 35506 35668 +162
============================================
+ Hits 34500 34657 +157
- Misses 1006 1011 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
{ "projectName": "bbStatus30455", "target": "default", "region": "us-west-2",
"resources": [
{ "resourceType": "harness", "name": "bbStatus30455", "deploymentState": "deployed",
"identifier": "arn:aws:bedrock-agentcore:us-west-2:725476964917:harness/bbStatus30455_bbStatus30455-R7ttcnrOG8" },
{ "resourceType": "memory", "name": "user_mem", "deploymentState": "deployed",
"identifier": "arn:aws:bedrock-agentcore:us-west-2:725476964917:memory/bbStatus30455_user_mem-9ndWxKF91x" },
{ "resourceType": "gateway", "name": "tools", "deploymentState": "deployed",
"identifier": "arn:aws:bedrock-agentcore:us-west-2:725476964917:gateway/bbstatus30455-tools-hwyi9qiqkn" },
{ "resourceType": "policy-engine", "name": "guards", "deploymentState": "deployed",
"identifier": "arn:aws:bedrock-agentcore:us-west-2:725476964917:policy-engine/bbStatus30455_guards-qjcpxplfak",
"children": [
{ "resourceType": "policy", "name": "no_pii", "deploymentState": "deployed",
"identifier": "arn:aws:bedrock-agentcore:us-west-2:725476964917:policy-engine/bbStatus30455_guards-qjcpxplfak/policy/no_pii-i9vsi_xtqh" } ] } ] } |
AlexanderRichey
left a comment
AlexanderRichey
left a comment
There was a problem hiding this comment.
This doesn't look like it implements the approach we discussed of calling DescribeStacks. What motivated changing direction here?
There was a problem hiding this comment.
Why is this a binary file?
There was a problem hiding this comment.
My agent add NUL bytes to this file which made github make it into a binary that is now fixed.
d399745 to
40d7f76
Compare
First-run fix:
|
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
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
|
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
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
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.
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.
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.
Returns a JSON report of every declared resource and whether the target's stack holds it. Children nest under their owner rather than carrying a parent name, and identifier is omitted (not null) when undeployed. pending-removal is out of scope: the resolver enumerates the spec, so a resource deleted from the spec but still in the stack is not discoverable.
project create leaves aws-targets.json empty and only project deploy provisions the default target, so status errored on a freshly created project instead of reporting that nothing is deployed yet.
The status resource key joined its parts with NUL bytes, which made git treat the handler as a binary file and hid it from review diffs.
Also drops the unassignedTargets row from project status: nothing in the CLI can create an unassigned gateway target.
Widening the resolver to every declared resource switched its identifiers from bare IDs to ARNs, which broke 'project invoke': GetAgentRuntime and GetHarness both take an ID, and the service builds its IAM policy resource from whatever identifier it is handed, so an ARN failed as AccessDenied rather than a validation error. The backend now takes an identifier kind. resolveProjectResource asks for "id" for the invoke path; resolveProjectResources keeps ARNs for status. Renames resolveDeployedResource(s) to resolveProjectResource(s) and ResolvedDeployedResource to ResolvedProjectResource, and makes the plural return every declared resource as a discriminated union on deploymentState, so 'project status' can report local-only resources instead of silently omitting them.
233201b to
0ddf02c
Compare
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
|
| This doesn't look like it implements the approach we discussed of calling DescribeStacks. What motivated changing direction here?
|
There was a problem hiding this comment.
Built it and drove it against a real account in us-west-2. Status resolves every resource correctly and headless invoke works. Two blockers before merge: the interactive invoke picker is broken (ARNs sent where ids are expected) and child nesting matches parents by name only. Details inline.
| let active = true; | ||
| void core.projectManager | ||
| .resolveDeployedResources(project, { target: "default" }) | ||
| .resolveProjectResources(project, { target: "default" }) |
There was a problem hiding this comment.
Regression from this PR. The manager never forwards an identifier here, so the resolver defaults to ARNs and select() passes them as harnessId/agentRuntimeId. Picking a deployed harness in the TUI fails with Value 'arn:aws:...:harness/...' at 'harnessId' failed to satisfy constraint. Headless invoke was fixed with identifier: "id", this path was not. Returning both id and arn on each row would remove the toggle entirely.
| const top: ResourceStatus[] = []; | ||
| for (const { resource, row } of entries) { | ||
| const owner = resource.parent | ||
| ? entries.find(({ resource: other }) => !other.parent && other.name === resource.parent) |
There was a problem hiding this comment.
Parent is matched by name only. Names are unique per collection, not across them, so a gateway and a policy engine can both be guards. Reproduced live: the deployed policy showed under the gateway and the engine had no children. Match on the expected parent type too.
| region: resolved.target.region, | ||
| resources: nest(resolved.resources), | ||
| }; | ||
| ctx.require(JsonRendererKey).renderJson(status); |
There was a problem hiding this comment.
deploy, build, dev and export in this router check JsonKey and print a human line otherwise. This always dumps JSON, so a bare agentcore project status in a terminal gets a raw blob.
There was a problem hiding this comment.
agentcore project status will just print out the json output for now. If customers want a human readable, Alex is implementing the tui experience after this pr gets merged.
| }); | ||
| }); | ||
|
|
||
| test("rejects a freshly created project that declares no targets", async () => { |
There was a problem hiding this comment.
project create leaves aws-targets.json empty and deploy fills it, so right after create status tells the user to hand-write the file with an example account id. Commit bfef4ea fixed this and drop allowMissing undid it. The "all local-only" test above only passes because the fake backend never throws, the real one always does here.
| const { target, identifier = "arn" } = input; | ||
| const deployedState = await readDeployedState(this.json, project.rootPath); | ||
| const stackArn = deployedState.targets[target.name]?.stackArn; | ||
| if (!stackArn) { |
There was a problem hiding this comment.
For status, a missing stack just means nothing is deployed yet, which is exactly what local-only describes. Deploy and remove can keep the hard failure.
There was a problem hiding this comment.
Yeah that makes sense. I'll let the handlers throw the ProjectStateError if they want too.
| } | ||
| }; | ||
|
|
||
| const declared: Declared[] = [ |
There was a problem hiding this comment.
The spec also has datasets and mcpRuntimeTools, and declaresNothingDeployable in deploy counts both. They are missing here, so a deployed dataset gets no row at all and deploy and status disagree on what a resource is.
There was a problem hiding this comment.
Datasets are being deprecated in favor of the imperative based commands. I think mcpRuntimeTools is also deprecated because I don't see any add command for it. declaresNothingDeployable doesn't exist.
| return byExportName("Policy", parent ?? "", name, suffix); | ||
| case "config-bundle": | ||
| return byExportName("ConfigBundle", name, suffix); | ||
| case "capacity-provider": |
There was a problem hiding this comment.
No capacityProviders field exists anywhere in the spec, so this case and the union member are unreachable, and the never default forces every future switch to keep handling it.
There was a problem hiding this comment.
I was thinking we will add capacityProvider in the new cli but I can remove it for now.
| case "capacity-provider": | ||
| return byExportName("CapacityProvider", name, suffix); | ||
| case "payment": | ||
| // Payments doesn't set a ExportName so we search for OutputKey |
There was a problem hiding this comment.
This links to the L3 AgentCorePayments construct, but the generated project never uses it. The Payment<name>ManagerArn output comes from toCdkId in the CLI's cdk-stack.ts template. Also the workspace rule is no line comments, so these should go.
There was a problem hiding this comment.
But I like these comments. It help developers understand why the credentials and Payments are special cases.
| | "policy-engine" | ||
| | "policy" | ||
| | "config-bundle" | ||
| | "payment" |
There was a problem hiding this comment.
project add and project remove call this payment-manager (see ProjectResource below), status calls it payment. Deriving this from ProjectResource keeps the two from drifting.
|
|
||
| describe("CdkBackend.resolveDeployedResources", () => { | ||
| test("describes the stack once and returns only resources with deployed ID outputs", async () => { | ||
| describe("CdkBackend.resolveProjectResources", () => { |
There was a problem hiding this comment.
The base branch tested -RuntimeId and -Harness-<name>-Id. Now nothing here asks for identifier: "id", so a typo in the suffix logic would make headless invoke report a deployed runtime as not deployed with CI green. One id-mode case would cover it.
| (r.resourceType === "runtime" || r.resourceType === "harness") && | ||
| r.deploymentState === "deployed", | ||
| ) | ||
| .map((resource) => { |
There was a problem hiding this comment.
Why is this code changing in this PR? I thought the scope was the status command.
Resolve resources as ARNs only and convert at the invoke boundary. The identifier toggle defaulted to "arn", so the plural resolver silently handed ARNs to the TUI picker, which passed them as harnessId/agentRuntimeId and failed the service pattern. The toggle is gone; resourceIdFromArn reads the resource id where the data-plane APIs need one. Report resources rather than failing when a target has no stack. For status a missing or deleted stack simply means nothing is deployed, which is what local-only describes; deploy and remove still fail loudly. Report a freshly created project as undeployed. A project only gains its default target on first deploy, so status told the user to hand-write aws-targets.json with an example account id. The region the CLI resolved stands in, in memory, for the default target alone. Nest children under their owner's type. Names are unique per collection but not across them, so a gateway and a policy engine both named 'guards' filed the policy under the gateway. Rename payment to payment-manager and add payment-connector, matching the names project add and project remove already use, and drop capacity-provider: no capacityProviders field exists in the spec, so the case was unreachable. Point the payment output comments at the CLI's own cdk-stack.ts template, which emits those outputs without an exportName, rather than an L3 construct the generated project never instantiates.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
| const project = ctx.require(ProjectKey); | ||
| const name = selectProjectResource(project, "runtime", flags.name); | ||
| const deployed = await core.projectManager.resolveDeployedResource(project, { | ||
| const deployed = await core.projectManager.resolveProjectResource(project, { |
There was a problem hiding this comment.
There doesn't seem to be anything pressing that necessitates this name change. It's causing a lot of cascading changes that make this code hard to review and increase the risk of introducing errors. Let's omit this part of the PR and focus on just the status functionality.
There was a problem hiding this comment.
I renamed resolveDeployResource to resolveProjectResources because it now returns whether a resource was deployed or not. I'll just make resolveProjectResource it's own function in the next revision. Later we can merge the functions.
4 participants
The status handler calls
ProjectManager.resolveProjectResourcesto build its report.resolveProjectResourcesalready resolved runtimes and harnesses by readingCfnOutputvalues via
DescribeStacks, returning each one's bare id. This changes it to return everyresource's ARN, omitting the identifier when the resource is not deployed.
Given an
agentcore.jsonlike this:{ "name": "orders", "version": 1, "runtimes": [{ "name": "assistant" }], "memories": [{ "name": "user_mem" }], "agentCoreGateways": [ { "name": "tools", "targets": [{ "name": "search" }] } ], "policyEngines": [ { "name": "guards", "policies": [{ "name": "no_pii" }] } ] }resolveProjectResourcesreturns:{ "target": { "name": "prod", "account": "111122223333", "region": "us-west-2" }, "resources": [ { "resourceType": "runtime", "name": "assistant", "deploymentState": "deployed", "id": "arn:aws:bedrock-agentcore:us-west-2:111122223333:runtime/assistant-AbCdEf1234" }, { "resourceType": "memory", "name": "user_mem", "deploymentState": "deployed", "id": "arn:aws:bedrock-agentcore:us-west-2:111122223333:memory/user_mem-K3l4M5n6O7" }, { "resourceType": "gateway", "name": "tools", "deploymentState": "local-only" }, { "resourceType": "gateway-target", "name": "search", "parent": "tools", "deploymentState": "local-only" }, { "resourceType": "policy-engine", "name": "guards", "deploymentState": "deployed", "id": "arn:aws:bedrock-agentcore:us-west-2:111122223333:policy-engine/guards-1a2b3c" }, { "resourceType": "policy", "name": "no_pii", "parent": "guards", "deploymentState": "deployed", "id": "arn:aws:bedrock-agentcore:us-west-2:111122223333:policy/no_pii-7d8e9f" } ] }The handler then nests children such as
gateway-targetunder their parent to produce:{ "projectName": "orders", "target": "prod", "region": "us-west-2", "resources": [ { "resourceType": "runtime", "name": "assistant", "deploymentState": "deployed", "identifier": "arn:aws:bedrock-agentcore:us-west-2:111122223333:runtime/assistant-AbCdEf1234" }, { "resourceType": "memory", "name": "user_mem", "deploymentState": "deployed", "identifier": "arn:aws:bedrock-agentcore:us-west-2:111122223333:memory/user_mem-K3l4M5n6O7" }, { "resourceType": "gateway", "name": "tools", "deploymentState": "local-only", "children": [ { "resourceType": "gateway-target", "name": "search", "deploymentState": "local-only" } ] }, { "resourceType": "policy-engine", "name": "guards", "deploymentState": "deployed", "identifier": "arn:aws:bedrock-agentcore:us-west-2:111122223333:policy-engine/guards-1a2b3c", "children": [ { "resourceType": "policy", "name": "no_pii", "deploymentState": "deployed", "identifier": "arn:aws:bedrock-agentcore:us-west-2:111122223333:policy/no_pii-7d8e9f" } ] } ] }If the project isn't deployed, it throws
ProjectStateError:Project 'orders' is not deployed to target 'prod'. Run 'agentcore project deploy --target prod' first.