From 42138bcb5515312b01e3919950ed78970da8691e Mon Sep 17 00:00:00 2001 From: jariy17 Date: Tue, 1 Sep 2026 19:18:48 +0000 Subject: [PATCH 1/3] refactor(project): merge DeployedProjectResource into ResolvedDeployedResource `DeployedProjectResource` ({resourceType, name, id}) and `ResolvedDeployedResource` ({id, target}) described the same thing at different levels of completeness: the singular was a projection of a plural list item plus the shared target, and `resolveDeployedResource` is literally the plural plus `.find()` plus a throw. Collapse them into one `ResolvedDeployedResource` carrying all four fields, so a resolved resource is self-describing: a caller holding one knows what it is, its physical ID, and which region to address it in. The singular now returns the found item directly instead of rebuilding `{id, target}`. `ResolvedDeployedResources` is retained deliberately. Its target is resolved independently of resource discovery, so an undeployed target yields zero resources while callers still need to report which target and region they read. Pure refactor: same resolved IDs, same target resolution, same ProjectStateError text, same throw-vs-omit semantics. --- src/core/project/backends/cdk.test.ts | 14 ++++++++++++-- src/core/project/backends/cdk.ts | 10 ++++++---- src/core/project/backends/types.ts | 4 ++-- src/core/project/manager.tsx | 2 +- src/handlers/project/invoke/index.test.tsx | 2 ++ .../project/invoke/invoke.screen.test.tsx | 14 ++++++++------ src/handlers/project/types.ts | 19 +++++++++++++------ 7 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/core/project/backends/cdk.test.ts b/src/core/project/backends/cdk.test.ts index 33f509877..f152b528b 100644 --- a/src/core/project/backends/cdk.test.ts +++ b/src/core/project/backends/cdk.test.ts @@ -628,8 +628,18 @@ describe("CdkBackend.resolveDeployedResources", () => { const resources = await subject.backend.resolveDeployedResources(input, { target: TARGET }); expect(resources).toEqual([ - { resourceType: "runtime", name: "checkout_agent", id: "checkout_agent-AbCdEf1234" }, - { resourceType: "harness", name: "support_agent", id: "support_agent-AbCdEf1234" }, + { + resourceType: "runtime", + name: "checkout_agent", + id: "checkout_agent-AbCdEf1234", + target: TARGET, + }, + { + resourceType: "harness", + name: "support_agent", + id: "support_agent-AbCdEf1234", + target: TARGET, + }, ]); expect(subject.stackReads).toHaveLength(1); }); diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index 78b5cac8d..8705c223c 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -3,10 +3,10 @@ import { join } from "node:path"; import type { Stack } from "@aws-sdk/client-cloudformation"; import { MalformedServiceResponseError, ProjectStateError } from "../../../errors/errors"; import type { - DeployedProjectResource, DeployResult, Project, ProjectEvent, + ResolvedDeployedResource, } from "../../../handlers/project/types"; import { FsReadWriteJson, @@ -53,7 +53,7 @@ type StackDescriber = typeof describeStack; function findDeployedResourceId( stack: Stack, - input: Pick, + input: Pick, ): string | undefined { if (!stack.StackName) return undefined; const exportResourceName = input.name.replaceAll("_", "-"); @@ -265,7 +265,7 @@ export class CdkBackend implements ProjectBackend { public async resolveDeployedResources( project: Project, input: ResolveDeployedResourcesBackendInput, - ): Promise { + ): Promise { const { target } = input; const deployedState = await readDeployedState(this.json, project.rootPath); const stackArn = deployedState.targets[target.name]?.stackArn; @@ -291,7 +291,9 @@ export class CdkBackend implements ProjectBackend { ]; return resources.flatMap((resource) => { const id = findDeployedResourceId(stack, resource); - return id ? [{ ...resource, id }] : []; + // The backend already resolved `target`, so each entry carries it and stays + // self-describing once callers pull it out of the list. + return id ? [{ ...resource, id, target }] : []; }); } diff --git a/src/core/project/backends/types.ts b/src/core/project/backends/types.ts index 77c2b0751..dccb11da8 100644 --- a/src/core/project/backends/types.ts +++ b/src/core/project/backends/types.ts @@ -1,8 +1,8 @@ import type { - DeployedProjectResource, DeployResult, Project, ProjectEvent, + ResolvedDeployedResource, TeardownConfirmationHandler, } from "../../../handlers/project/types"; import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; @@ -25,5 +25,5 @@ export interface ProjectBackend { resolveDeployedResources( project: Project, input: ResolveDeployedResourcesBackendInput, - ): Promise; + ): Promise; } diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 45b99d4d6..ac904b12e 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -889,7 +889,7 @@ export class FsProjectManager implements ProjectManager { const resource = resolved.resources.find( ({ resourceType, name }) => resourceType === input.resourceType && name === input.name, ); - if (resource) return { id: resource.id, target: resolved.target }; + if (resource) return resource; const label = input.resourceType === "runtime" ? "Runtime" : "Harness"; throw new ProjectStateError( diff --git a/src/handlers/project/invoke/index.test.tsx b/src/handlers/project/invoke/index.test.tsx index d5fb86b20..91ce9dd4f 100644 --- a/src/handlers/project/invoke/index.test.tsx +++ b/src/handlers/project/invoke/index.test.tsx @@ -85,11 +85,13 @@ function backend() { resourceType: "runtime" as const, name, id: RUNTIME_ID, + target: input.target, })), ...project.spec.harnesses.map(({ name }) => ({ resourceType: "harness" as const, name, id: HARNESS_ID, + target: input.target, })), ]; }, diff --git a/src/handlers/project/invoke/invoke.screen.test.tsx b/src/handlers/project/invoke/invoke.screen.test.tsx index a7c21e53d..30b6a49dc 100644 --- a/src/handlers/project/invoke/invoke.screen.test.tsx +++ b/src/handlers/project/invoke/invoke.screen.test.tsx @@ -7,7 +7,7 @@ import type { import { ProjectSpecSchema } from "../../../projectSchemas/project"; import { ProjectKey } from "../../../router"; import { cleanupScreens, renderScreen, TestCoreClient, waitForText } from "../../../testing"; -import type { DeployedProjectResource, Project } from "../types"; +import type { Project, ResolvedDeployedResource } from "../types"; afterEach(cleanupScreens); @@ -46,14 +46,16 @@ function endpoint(name: string): AgentRuntimeEndpoint { const TARGET = { name: "default", account: "111122223333", region: "eu-west-1" } as const; -const DEPLOYED_RESOURCES: DeployedProjectResource[] = [ - { resourceType: "runtime", name: "checkout", id: "runtime-123" }, - { resourceType: "harness", name: "support", id: "harness-123" }, +const DEPLOYED_RESOURCES: ResolvedDeployedResource[] = [ + { resourceType: "runtime", name: "checkout", id: "runtime-123", target: TARGET }, + { resourceType: "harness", name: "support", id: "harness-123", target: TARGET }, ]; -function core(resources: DeployedProjectResource[] = DEPLOYED_RESOURCES): TestCoreClient { +function core(resources: ResolvedDeployedResource[] = DEPLOYED_RESOURCES): TestCoreClient { const value = new TestCoreClient(); value.projectManager.resolveDeployedResource = async (_project, input) => ({ + resourceType: input.resourceType, + name: input.name, id: input.resourceType === "runtime" ? "runtime-123" : "harness-123", target: TARGET, }); @@ -76,7 +78,7 @@ function core(resources: DeployedProjectResource[] = DEPLOYED_RESOURCES): TestCo describe("project invoke picker", () => { test("lists only resources present in the deployed target", async () => { const screen = renderScreen("/agentcore/project/invoke", { - core: core([{ resourceType: "harness", name: "support", id: "harness-123" }]), + core: core([{ resourceType: "harness", name: "support", id: "harness-123", target: TARGET }]), withContext: (ctx) => ctx.withValue(ProjectKey, project), }); diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index 8fad8a811..09c89fa31 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -145,19 +145,26 @@ export type ResolveDeployedResourcesInput = { target: string; }; -export type DeployedProjectResource = { +/** + * A single project resource resolved to its deployed physical ID, self-describing + * down to the target it lives in: a caller holding one always knows what it is and + * which region to address it in, without carrying a separate envelope alongside. + */ +export type ResolvedDeployedResource = { resourceType: ProjectInvokableResource; name: string; id: string; -}; - -export type ResolvedDeployedResource = { - id: string; target: AwsDeploymentTarget; }; +/** + * Every resource discovered in a target's deployed stack. The target is repeated on + * the envelope because it is resolved independently of what the stack turns out to + * contain: an undeployed target yields zero resources, and callers still need to + * report which target and region they read. + */ export type ResolvedDeployedResources = { - resources: DeployedProjectResource[]; + resources: ResolvedDeployedResource[]; target: AwsDeploymentTarget; }; From 2998f2f8100f1c04c13abedb21f020eee4ad6329 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Tue, 1 Sep 2026 19:34:42 +0000 Subject: [PATCH 2/3] fix(project): keep the declared target authoritative on the singular lookup Merging DeployedProjectResource into ResolvedDeployedResource moved the target on the singular path from the manager (which resolves it from aws-targets.json) to whatever the backend stamped on the item. Both invoke handlers pin RegionKey off that value, so a backend echoing a stale target could redirect an invoke to the wrong region -- not possible before the merge. resolveDeployedResource now overrides the item's copy with the resolved target, and manager.test.ts gains the coverage the two methods never had: a divergence guard, resourceType/name identity, the empty-stack envelope, the undeployed throw path, and the item/envelope target invariant. Also drops three doc comments that misstated ownership: two restated what the types already show, and the cdk.ts one credited the backend with resolving a target the manager had passed in. --- src/core/project/backends/cdk.ts | 2 - src/core/project/manager.test.ts | 128 +++++++++++++++++++++++++++++++ src/core/project/manager.tsx | 5 +- src/handlers/project/types.ts | 11 --- 4 files changed, 132 insertions(+), 14 deletions(-) diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index 8705c223c..516ce0aa9 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -291,8 +291,6 @@ export class CdkBackend implements ProjectBackend { ]; return resources.flatMap((resource) => { const id = findDeployedResourceId(stack, resource); - // The backend already resolved `target`, so each entry carries it and stays - // self-describing once callers pull it out of the list. return id ? [{ ...resource, id, target }] : []; }); } diff --git a/src/core/project/manager.test.ts b/src/core/project/manager.test.ts index 142c5d1d1..1121499bc 100644 --- a/src/core/project/manager.test.ts +++ b/src/core/project/manager.test.ts @@ -21,6 +21,7 @@ import { type DeployResult, type Project, type ProjectEvent, + type ResolvedDeployedResource, } from "../../handlers/project/types"; import { createSilentLogger } from "../../testing"; import type { DeployBackendInput, ProjectBackend } from "./backends/types"; @@ -677,6 +678,133 @@ describe("FsProjectManager.deploy", () => { }); }); +describe("FsProjectManager.resolveDeployedResource(s)", () => { + const DECLARED: AwsDeploymentTarget = { + name: "default", + account: "111122223333", + region: "eu-west-1", + }; + + // A backend whose resolved resources are built from the target it was handed, so a + // test can either echo that target the way CdkBackend does or deliberately defy it. + function resolveManager(build: (target: AwsDeploymentTarget) => ResolvedDeployedResource[]) { + return new FsProjectManager({ + logger: createSilentLogger(), + backends: { + CDK: { + async *build() {}, + async *deploy() { + yield { message: "unused by these tests" }; + return { outputs: {} }; + }, + async resolveDeployedResources(_project, input) { + return build(input.target); + }, + }, + }, + }); + } + + async function projectAt(rootPath: string): Promise { + await mkdir(join(rootPath, "agentcore"), { recursive: true }); + await writeFile(join(rootPath, "agentcore", "aws-targets.json"), JSON.stringify([DECLARED])); + return { + name: "example", + rootPath, + spec: ProjectSpecSchema.parse({ name: "example", version: 1 }), + }; + } + + const item = (overrides: Partial = {}): ResolvedDeployedResource => ({ + resourceType: "runtime", + name: "checkout", + id: "checkout-abc123", + target: DECLARED, + ...overrides, + }); + + test("prefers the declared target over one an item claims", async () => { + const root = await inTempDirectory(); + const rogue: AwsDeploymentTarget = { + name: "default", + account: "111122223333", + region: "us-east-1", + }; + const subject = resolveManager(() => [item({ target: rogue })]); + + const resolved = await subject.resolveDeployedResource(await projectAt(root), { + target: "default", + resourceType: "runtime", + name: "checkout", + }); + + // Both invoke handlers pin RegionKey off this value, so trusting an item's copy + // would route a customer's invoke at whatever region the backend stamped. + expect(resolved.target.region).toBe("eu-west-1"); + }); + + test("distinguishes a runtime and a harness sharing one name", async () => { + const root = await inTempDirectory(); + const project = await projectAt(root); + const subject = resolveManager((target) => [ + item({ resourceType: "runtime", name: "support", id: "runtime-1", target }), + item({ resourceType: "harness", name: "support", id: "harness-2", target }), + ]); + + const runtime = await subject.resolveDeployedResource(project, { + target: "default", + resourceType: "runtime", + name: "support", + }); + const harness = await subject.resolveDeployedResource(project, { + target: "default", + resourceType: "harness", + name: "support", + }); + + expect(runtime).toMatchObject({ resourceType: "runtime", name: "support", id: "runtime-1" }); + expect(harness).toMatchObject({ resourceType: "harness", name: "support", id: "harness-2" }); + }); + + test("keeps the target when the stack holds no resources", async () => { + const root = await inTempDirectory(); + const subject = resolveManager(() => []); + + expect( + await subject.resolveDeployedResources(await projectAt(root), { target: "default" }), + ).toEqual({ resources: [], target: DECLARED }); + }); + + test("reports an undeployed resource with the remediation command", async () => { + const root = await inTempDirectory(); + const subject = resolveManager(() => []); + + await expect( + subject.resolveDeployedResource(await projectAt(root), { + target: "default", + resourceType: "runtime", + name: "checkout", + }), + ).rejects.toThrow(/is not deployed to target 'default'.*project deploy --target default/s); + }); + + test("hands every resource the same target the envelope reports", async () => { + const root = await inTempDirectory(); + // Echoes the handed target, mirroring CdkBackend at cdk.ts:294. + const subject = resolveManager((target) => [ + item({ target }), + item({ name: "other", id: "other-2", target }), + ]); + + const resolved = await subject.resolveDeployedResources(await projectAt(root), { + target: "default", + }); + + expect(resolved.resources).toHaveLength(2); + expect(resolved.resources.every((r) => r.target === resolved.target)).toBe(true); + }); +}); + describe("FsProjectManager.resolve", () => { test("round-trips a project it just created", async () => { const root = await inTempDirectory(); diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index ac904b12e..4f1b016fb 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -889,7 +889,10 @@ export class FsProjectManager implements ProjectManager { const resource = resolved.resources.find( ({ resourceType, name }) => resourceType === input.resourceType && name === input.name, ); - if (resource) return resource; + // The declared target wins over the copy on the item: the manager resolved it + // from aws-targets.json, and both invoke handlers pin the AWS region off this + // value, so trusting a backend's echo would let it redirect the call. + if (resource) return { ...resource, target: resolved.target }; const label = input.resourceType === "runtime" ? "Runtime" : "Harness"; throw new ProjectStateError( diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index 09c89fa31..5b0b2e7b2 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -145,11 +145,6 @@ export type ResolveDeployedResourcesInput = { target: string; }; -/** - * A single project resource resolved to its deployed physical ID, self-describing - * down to the target it lives in: a caller holding one always knows what it is and - * which region to address it in, without carrying a separate envelope alongside. - */ export type ResolvedDeployedResource = { resourceType: ProjectInvokableResource; name: string; @@ -157,12 +152,6 @@ export type ResolvedDeployedResource = { target: AwsDeploymentTarget; }; -/** - * Every resource discovered in a target's deployed stack. The target is repeated on - * the envelope because it is resolved independently of what the stack turns out to - * contain: an undeployed target yields zero resources, and callers still need to - * report which target and region they read. - */ export type ResolvedDeployedResources = { resources: ResolvedDeployedResource[]; target: AwsDeploymentTarget; From e1f44c21fec9aefe7787e81acf3aec1a84bd54ec Mon Sep 17 00:00:00 2001 From: jariy17 Date: Tue, 1 Sep 2026 20:47:44 +0000 Subject: [PATCH 3/3] test: drop the added resolveDeployedResource(s) unit tests The type merge is verified by tsc; these covered pre-existing behavior on methods the refactor did not change, inflating a 22-line refactor's diff. --- src/core/project/manager.test.ts | 128 ------------------------------- 1 file changed, 128 deletions(-) diff --git a/src/core/project/manager.test.ts b/src/core/project/manager.test.ts index 1121499bc..142c5d1d1 100644 --- a/src/core/project/manager.test.ts +++ b/src/core/project/manager.test.ts @@ -21,7 +21,6 @@ import { type DeployResult, type Project, type ProjectEvent, - type ResolvedDeployedResource, } from "../../handlers/project/types"; import { createSilentLogger } from "../../testing"; import type { DeployBackendInput, ProjectBackend } from "./backends/types"; @@ -678,133 +677,6 @@ describe("FsProjectManager.deploy", () => { }); }); -describe("FsProjectManager.resolveDeployedResource(s)", () => { - const DECLARED: AwsDeploymentTarget = { - name: "default", - account: "111122223333", - region: "eu-west-1", - }; - - // A backend whose resolved resources are built from the target it was handed, so a - // test can either echo that target the way CdkBackend does or deliberately defy it. - function resolveManager(build: (target: AwsDeploymentTarget) => ResolvedDeployedResource[]) { - return new FsProjectManager({ - logger: createSilentLogger(), - backends: { - CDK: { - async *build() {}, - async *deploy() { - yield { message: "unused by these tests" }; - return { outputs: {} }; - }, - async resolveDeployedResources(_project, input) { - return build(input.target); - }, - }, - }, - }); - } - - async function projectAt(rootPath: string): Promise { - await mkdir(join(rootPath, "agentcore"), { recursive: true }); - await writeFile(join(rootPath, "agentcore", "aws-targets.json"), JSON.stringify([DECLARED])); - return { - name: "example", - rootPath, - spec: ProjectSpecSchema.parse({ name: "example", version: 1 }), - }; - } - - const item = (overrides: Partial = {}): ResolvedDeployedResource => ({ - resourceType: "runtime", - name: "checkout", - id: "checkout-abc123", - target: DECLARED, - ...overrides, - }); - - test("prefers the declared target over one an item claims", async () => { - const root = await inTempDirectory(); - const rogue: AwsDeploymentTarget = { - name: "default", - account: "111122223333", - region: "us-east-1", - }; - const subject = resolveManager(() => [item({ target: rogue })]); - - const resolved = await subject.resolveDeployedResource(await projectAt(root), { - target: "default", - resourceType: "runtime", - name: "checkout", - }); - - // Both invoke handlers pin RegionKey off this value, so trusting an item's copy - // would route a customer's invoke at whatever region the backend stamped. - expect(resolved.target.region).toBe("eu-west-1"); - }); - - test("distinguishes a runtime and a harness sharing one name", async () => { - const root = await inTempDirectory(); - const project = await projectAt(root); - const subject = resolveManager((target) => [ - item({ resourceType: "runtime", name: "support", id: "runtime-1", target }), - item({ resourceType: "harness", name: "support", id: "harness-2", target }), - ]); - - const runtime = await subject.resolveDeployedResource(project, { - target: "default", - resourceType: "runtime", - name: "support", - }); - const harness = await subject.resolveDeployedResource(project, { - target: "default", - resourceType: "harness", - name: "support", - }); - - expect(runtime).toMatchObject({ resourceType: "runtime", name: "support", id: "runtime-1" }); - expect(harness).toMatchObject({ resourceType: "harness", name: "support", id: "harness-2" }); - }); - - test("keeps the target when the stack holds no resources", async () => { - const root = await inTempDirectory(); - const subject = resolveManager(() => []); - - expect( - await subject.resolveDeployedResources(await projectAt(root), { target: "default" }), - ).toEqual({ resources: [], target: DECLARED }); - }); - - test("reports an undeployed resource with the remediation command", async () => { - const root = await inTempDirectory(); - const subject = resolveManager(() => []); - - await expect( - subject.resolveDeployedResource(await projectAt(root), { - target: "default", - resourceType: "runtime", - name: "checkout", - }), - ).rejects.toThrow(/is not deployed to target 'default'.*project deploy --target default/s); - }); - - test("hands every resource the same target the envelope reports", async () => { - const root = await inTempDirectory(); - // Echoes the handed target, mirroring CdkBackend at cdk.ts:294. - const subject = resolveManager((target) => [ - item({ target }), - item({ name: "other", id: "other-2", target }), - ]); - - const resolved = await subject.resolveDeployedResources(await projectAt(root), { - target: "default", - }); - - expect(resolved.resources).toHaveLength(2); - expect(resolved.resources.every((r) => r.target === resolved.target)).toBe(true); - }); -}); - describe("FsProjectManager.resolve", () => { test("round-trips a project it just created", async () => { const root = await inTempDirectory();