From 0a716eaff09dca290e6dce82377cc5b9f425f47f Mon Sep 17 00:00:00 2001 From: notgitika Date: Wed, 19 Aug 2026 20:56:14 -0400 Subject: [PATCH 1/7] feat(project): add deploy command contract --- src/core/project/backends/cdk.ts | 14 +++- src/core/project/backends/types.ts | 9 ++- src/core/project/index.tsx | 2 +- src/core/project/manager.tsx | 10 +++ src/handlers/project/deploy/index.test.ts | 93 +++++++++++++++++++++++ src/handlers/project/deploy/index.ts | 51 +++++++++++-- src/handlers/project/index.ts | 6 +- src/handlers/project/project.test.ts | 14 +++- src/handlers/project/types.ts | 13 ++++ src/projectSchemas/aws-targets.test.ts | 60 +++++++++++++++ src/projectSchemas/aws-targets.ts | 54 +++++++++++++ src/testing/TestCoreClient.tsx | 19 +++-- 12 files changed, 325 insertions(+), 20 deletions(-) create mode 100644 src/handlers/project/deploy/index.test.ts create mode 100644 src/projectSchemas/aws-targets.test.ts create mode 100644 src/projectSchemas/aws-targets.ts diff --git a/src/core/project/backends/cdk.ts b/src/core/project/backends/cdk.ts index f9ea930f8..1915ed70f 100644 --- a/src/core/project/backends/cdk.ts +++ b/src/core/project/backends/cdk.ts @@ -1,10 +1,10 @@ import { existsSync } from "node:fs"; import { join } from "node:path"; -import { ProjectStateError } from "../../../errors/errors"; -import type { Project, ProjectEvent } from "../../../handlers/project/types"; +import { NotImplementedError, ProjectStateError } from "../../../errors/errors"; +import type { DeployResult, Project, ProjectEvent } from "../../../handlers/project/types"; import { requireTool, runProcess, type ProcessRunner } from "../../../io"; import type { Logger } from "../../../logging"; -import type { ProjectBackend } from "./types"; +import type { DeployBackendInput, ProjectBackend } from "./types"; export type CdkBackendConfig = { logger: Logger; @@ -41,4 +41,12 @@ export class CdkBackend implements ProjectBackend { onOutput: (chunk) => this.logger.debug(chunk), }); } + + public async *deploy( + _project: Project, + _input: DeployBackendInput, + ): AsyncGenerator { + yield* []; + throw new NotImplementedError("CDK project deployment is not implemented yet"); + } } diff --git a/src/core/project/backends/types.ts b/src/core/project/backends/types.ts index 1e327cf49..eebeccf29 100644 --- a/src/core/project/backends/types.ts +++ b/src/core/project/backends/types.ts @@ -1,6 +1,13 @@ -import type { Project, ProjectEvent } from "../../../handlers/project/types"; +import type { DeployResult, Project, ProjectEvent } from "../../../handlers/project/types"; +import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; + +export type DeployBackendInput = { + /** Fully resolved account and region selected from aws-targets.json. */ + target: AwsDeploymentTarget; +}; /** Builds the deployable artifacts owned by a project's selected backend. */ export interface ProjectBackend { build(project: Project): AsyncGenerator; + deploy(project: Project, input: DeployBackendInput): AsyncGenerator; } diff --git a/src/core/project/index.tsx b/src/core/project/index.tsx index b4b61cbe0..830a3455d 100644 --- a/src/core/project/index.tsx +++ b/src/core/project/index.tsx @@ -1,3 +1,3 @@ export { FsProjectManager } from "./manager"; export { CdkBackend, type CdkBackendConfig } from "./backends/cdk"; -export type { ProjectBackend } from "./backends/types"; +export type { DeployBackendInput, ProjectBackend } from "./backends/types"; diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 4a426ef1f..223a3a879 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -4,6 +4,8 @@ import { join, relative } from "node:path"; import type { AddResourceInput, CreateProjectInput, + DeployProjectInput, + DeployResult, ResolveProjectInput, Project, ProjectManager, @@ -282,6 +284,14 @@ export class FsProjectManager implements ProjectManager { yield* this.backendFor(project).build(project); } + public async *deploy( + _project: Project, + _input: DeployProjectInput, + ): AsyncGenerator { + yield* []; + throw new NotImplementedError("agentcore project deploy is not implemented yet"); + } + private backendFor(project: Project): ProjectBackend { const backend = this.backends[project.spec.managedBy]; if (!backend) { diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts new file mode 100644 index 000000000..9baf5ceb0 --- /dev/null +++ b/src/handlers/project/deploy/index.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, test } from "bun:test"; +import { ProjectSpecSchema } from "../../../projectSchemas/project"; +import { ProjectKey, Router, ValueContext } from "../../../router"; +import { testIO } from "../../../testing"; +import { JsonRendererKey } from "../../../tui"; +import { JsonKey } from "../../keys"; +import type { + AddResourceInput, + CreateProjectInput, + DeployProjectInput, + DeployResult, + Project, + ProjectEvent, + ProjectManager, +} from "../types"; +import { createDeployProjectHandler } from "."; + +const project: Project = { + name: "orders", + rootPath: "/workspace/orders", + spec: ProjectSpecSchema.parse({ name: "orders", version: 1 }), +}; + +function fakeProjectManager(result: DeployResult, events: ProjectEvent[] = []) { + const calls: { project: Project; input: DeployProjectInput }[] = []; + const manager: ProjectManager = { + async *create(_input: CreateProjectInput) { + yield* []; + return project; + }, + async *build(_project: Project) {}, + async *deploy(deployedProject, input) { + calls.push({ project: deployedProject, input }); + yield* events; + return result; + }, + async resolve() { + return project; + }, + async *addResource(_project: Project, _input: AddResourceInput) { + yield* []; + return project; + }, + }; + return { calls, manager }; +} + +function harness(result: DeployResult, options: { events?: ProjectEvent[]; json?: boolean } = {}) { + const io = testIO(); + const fake = fakeProjectManager(result, options.events); + const handler = createDeployProjectHandler({ projectManager: fake.manager, io: io.io }); + const ctx = ValueContext.EmptyContext() + .withValue(ProjectKey, project) + .withValue(JsonKey, options.json ?? false) + .withValue(JsonRendererKey, { + renderJson: (data) => io.io.stdout.write(`${JSON.stringify(data, null, 2)}\n`), + renderJsonLine: (data) => io.io.stdout.write(`${JSON.stringify(data)}\n`), + }); + const router = new Router("project", "test"); + router.handler(handler); + + return { + ...fake, + io, + run: (args: string[] = []) => router.route(["node", "project", "deploy", ...args], ctx), + }; +} + +describe("project deploy handler", () => { + test("defaults to the default target and keeps progress off stdout", async () => { + const subject = harness( + { outputs: { ZetaUrl: "https://zeta.example", AlphaArn: "arn:alpha" } }, + { events: [{ message: "Preparing deployment" }, { message: "Deploying stack" }] }, + ); + + await subject.run(); + + expect(subject.calls).toEqual([{ project, input: { target: "default" } }]); + expect(subject.io.stderr()).toContain("Preparing deployment\nDeploying stack"); + expect(subject.io.stderr()).toContain("Deployed project 'orders' to target 'default'"); + expect(subject.io.stdout()).toBe("AlphaArn: arn:alpha\nZetaUrl: https://zeta.example"); + }); + + test("passes an explicit target and renders the result as JSON", async () => { + const result = { outputs: { ServiceUrl: "https://service.example" } }; + const subject = harness(result, { json: true }); + + await subject.run(["--target", "staging"]); + + expect(subject.calls).toEqual([{ project, input: { target: "staging" } }]); + expect(JSON.parse(subject.io.stdout())).toEqual(result); + }); +}); diff --git a/src/handlers/project/deploy/index.ts b/src/handlers/project/deploy/index.ts index f73a75565..6e37461fe 100644 --- a/src/handlers/project/deploy/index.ts +++ b/src/handlers/project/deploy/index.ts @@ -1,11 +1,52 @@ -import { createHandler } from "../../../router"; -import { NotImplementedError } from "../../../errors"; +import z from "zod"; +import type { AppIO } from "../../../io"; +import { createHandler, flag, ProjectKey } from "../../../router"; +import { JsonRendererKey } from "../../../tui"; +import { JsonKey } from "../../keys"; +import type { DeployResult, ProjectEvent, ProjectManager } from "../types"; -export const createDeployProjectHandler = () => +type DeployProjectHandlerConfig = { + projectManager: ProjectManager; + io: AppIO; +}; + +async function runDeploy( + config: DeployProjectHandlerConfig, + project: Parameters[0], + target: string, +): Promise { + const deployment = config.projectManager.deploy(project, { target }); + while (true) { + const next = await deployment.next(); + if (next.done) return next.value; + config.io.stderr.write(`${(next.value as ProjectEvent).message}\n`); + } +} + +function renderResult(io: AppIO, result: DeployResult): void { + for (const [key, value] of Object.entries(result.outputs).sort(([a], [b]) => + a.localeCompare(b), + )) { + io.stdout.write(`${key}: ${value}\n`); + } +} + +export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) => createHandler({ name: "deploy", description: "deploy the project to AWS", - handle: async () => { - throw new NotImplementedError("agentcore project deploy is not implemented yet"); + flags: [ + flag("target", "name of the aws-targets.json entry to deploy", z.string().default("default")), + ], + handle: async (ctx, flags) => { + const project = ctx.require(ProjectKey); + const result = await runDeploy(config, project, flags.target); + + config.io.stderr.write(`Deployed project '${project.name}' to target '${flags.target}'\n`); + if (ctx.require(JsonKey)) { + ctx.require(JsonRendererKey).renderJson(result); + } else { + renderResult(config.io, result); + } }, }); diff --git a/src/handlers/project/index.ts b/src/handlers/project/index.ts index 232879425..9581d650c 100644 --- a/src/handlers/project/index.ts +++ b/src/handlers/project/index.ts @@ -43,7 +43,11 @@ export function createProjectHandler(config: ProjectHandlerConfig): Router { }), ), ); - project.handler(createDeployProjectHandler()); + project.handler( + withProject({ projectManager: config.projectManager })( + createDeployProjectHandler({ projectManager: config.projectManager, io: config.io }), + ), + ); project.handler(createStatusProjectHandler()); // withProject wraps only the commands that require an existing project, so // `create` (which refuses to nest inside one) stays unaffected. diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index ced2d9277..390ebd074 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -24,7 +24,7 @@ async function run(args: string[], opts?: { core?: TestCoreClient }) { return { io, core }; } -describe.each(["deploy", "status"])("project %s", (command) => { +describe.each(["status"])("project %s", (command) => { test("throws because it is not implemented yet", async () => { await expect(run([command])).rejects.toThrow(/not implemented/); }); @@ -346,3 +346,15 @@ describe("project build", () => { await expect(run(["build"])).rejects.toThrow(/npm install/); }); }); + +describe("project deploy", () => { + test("requires an AgentCore project", async () => { + await inTempDirectory(); + await expect(run(["deploy"])).rejects.toThrow(/No AgentCore project found/); + }); + + test("remains nonfunctional until deployment support is implemented", async () => { + await inProject(); + await expect(run(["deploy"])).rejects.toThrow(/not implemented/); + }); +}); diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index e804d10f9..6992e185b 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -29,6 +29,16 @@ export type ProjectEvent = { message: string; }; +export type DeployProjectInput = { + /** Name of the aws-targets.json entry to deploy. */ + target: string; +}; + +export type DeployResult = { + /** Outputs returned by the deployed stack, keyed by CloudFormation output name. */ + outputs: Record; +}; + export type ResolveProjectInput = { /** A path to search from when locating the project root. */ filePath: string; @@ -82,6 +92,9 @@ export interface ProjectManager { /** Compile the project's CDK app and synthesize its CloudFormation templates. */ build(project: Project): AsyncGenerator; + /** Deploy the project to one of its configured AWS targets. */ + deploy(project: Project, input: DeployProjectInput): AsyncGenerator; + /** Locate an existing AgentCore project. Returns undefined if no project can be found. */ resolve(input: ResolveProjectInput): Promise; diff --git a/src/projectSchemas/aws-targets.test.ts b/src/projectSchemas/aws-targets.test.ts new file mode 100644 index 000000000..d517a87c9 --- /dev/null +++ b/src/projectSchemas/aws-targets.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test } from "bun:test"; +import { + AgentCoreRegionSchema, + AwsAccountIdSchema, + AwsDeploymentTargetSchema, + AwsDeploymentTargetsSchema, + DeploymentTargetNameSchema, +} from "./aws-targets"; + +const target = { + name: "default", + description: "Default deployment target", + account: "111122223333", + region: "us-east-1", +} as const; + +describe("AWS deployment targets", () => { + test("accepts the scaffolded target shape", () => { + expect(AwsDeploymentTargetSchema.parse(target)).toEqual(target); + expect(AwsDeploymentTargetsSchema.parse([target])).toEqual([target]); + }); + + test.each([ + ["a digit short", "11112222333"], + ["a digit long", "1111222233334"], + ["punctuated", "1111-2222-3333"], + ["an account alias", "my-account"], + ])("rejects an account that is %s", (_label, account) => { + expect(AwsAccountIdSchema.safeParse(account).success).toBe(false); + }); + + test.each(["", "1default", "-default", "_default", "has spaces", "has.dots"])( + "rejects invalid target name %j", + (name) => { + expect(DeploymentTargetNameSchema.safeParse(name).success).toBe(false); + }, + ); + + test("enforces target name and description lengths", () => { + expect(DeploymentTargetNameSchema.safeParse("a".repeat(64)).success).toBe(true); + expect(DeploymentTargetNameSchema.safeParse("a".repeat(65)).success).toBe(false); + expect( + AwsDeploymentTargetSchema.safeParse({ ...target, description: "a".repeat(257) }).success, + ).toBe(false); + }); + + test("accepts only regions supported by AgentCore", () => { + expect(AgentCoreRegionSchema.safeParse("us-gov-west-1").success).toBe(true); + expect(AgentCoreRegionSchema.safeParse("us-west-1").success).toBe(false); + }); + + test("rejects duplicate target names", () => { + const result = AwsDeploymentTargetsSchema.safeParse([target, target]); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues[0]?.message).toBe("Duplicate deployment target name: default"); + } + }); +}); diff --git a/src/projectSchemas/aws-targets.ts b/src/projectSchemas/aws-targets.ts new file mode 100644 index 000000000..7ed276b4c --- /dev/null +++ b/src/projectSchemas/aws-targets.ts @@ -0,0 +1,54 @@ +import { z } from "zod"; +import { uniqueBy } from "./zod-util"; + +export const AgentCoreRegionSchema = z.enum([ + "ap-northeast-1", + "ap-northeast-2", + "ap-south-1", + "ap-southeast-1", + "ap-southeast-2", + "ca-central-1", + "eu-central-1", + "eu-north-1", + "eu-west-1", + "eu-west-2", + "eu-west-3", + "sa-east-1", + "us-east-1", + "us-east-2", + "us-west-2", + "us-gov-west-1", +]); + +export const DeploymentTargetNameSchema = z + .string() + .min(1) + .max(64) + .regex( + /^[a-zA-Z][a-zA-Z0-9_-]*$/, + "Name must start with a letter and contain only alphanumeric characters, hyphens, and underscores", + ) + .describe("Unique identifier for the deployment target"); + +export const AwsAccountIdSchema = z + .string() + .regex(/^[0-9]{12}$/, "AWS account ID must be exactly 12 digits") + .describe("AWS account ID"); + +export const AwsDeploymentTargetSchema = z.object({ + name: DeploymentTargetNameSchema, + description: z.string().max(256).optional(), + account: AwsAccountIdSchema, + region: AgentCoreRegionSchema, +}); + +export type AwsDeploymentTarget = z.infer; + +export const AwsDeploymentTargetsSchema = z.array(AwsDeploymentTargetSchema).superRefine( + uniqueBy( + (target) => target.name, + (name) => `Duplicate deployment target name: ${name}`, + ), +); + +export type AwsDeploymentTargets = z.infer; diff --git a/src/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index 50047db83..580ee111c 100644 --- a/src/testing/TestCoreClient.tsx +++ b/src/testing/TestCoreClient.tsx @@ -1179,6 +1179,7 @@ export class TestGatewayClient implements CoreGatewayClient { type TestCoreClientOptions = { logger?: Logger; json?: ReadWriteJson; + projectManager?: ProjectManager; }; export class TestIdentityClient implements CoreIdentityClient { @@ -1948,13 +1949,15 @@ export class TestCoreClient implements Core { readonly projectCommands: { command: string[]; cwd: string }[] = []; constructor(options?: TestCoreClientOptions) { - this.projectManager = new FsProjectManager({ - logger: options?.logger ?? createSilentLogger(), - json: options?.json, - runner: async (command, { cwd }) => { - this.projectCommands.push({ command, cwd }); - }, - checkTool: async () => {}, // CI hosts don't have uv installed - }); + this.projectManager = + options?.projectManager ?? + new FsProjectManager({ + logger: options?.logger ?? createSilentLogger(), + json: options?.json, + runner: async (command, { cwd }) => { + this.projectCommands.push({ command, cwd }); + }, + checkTool: async () => {}, // CI hosts don't have uv installed + }); } } From dc15b6afb656120b29f32f2899f95a53ac998595 Mon Sep 17 00:00:00 2001 From: notgitika Date: Thu, 20 Aug 2026 00:13:43 -0400 Subject: [PATCH 2/7] fix(project): sync supported deployment regions --- src/projectSchemas/aws-targets.test.ts | 10 ++++++++-- src/projectSchemas/aws-targets.ts | 5 +++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/projectSchemas/aws-targets.test.ts b/src/projectSchemas/aws-targets.test.ts index d517a87c9..742305361 100644 --- a/src/projectSchemas/aws-targets.test.ts +++ b/src/projectSchemas/aws-targets.test.ts @@ -44,8 +44,14 @@ describe("AWS deployment targets", () => { ).toBe(false); }); - test("accepts only regions supported by AgentCore", () => { - expect(AgentCoreRegionSchema.safeParse("us-gov-west-1").success).toBe(true); + test.each(["ap-southeast-5", "ap-southeast-7", "eu-south-1", "eu-south-2", "us-gov-west-1"])( + "accepts supported region %s", + (region) => { + expect(AgentCoreRegionSchema.safeParse(region).success).toBe(true); + }, + ); + + test("rejects an AWS region where AgentCore is unavailable", () => { expect(AgentCoreRegionSchema.safeParse("us-west-1").success).toBe(false); }); diff --git a/src/projectSchemas/aws-targets.ts b/src/projectSchemas/aws-targets.ts index 7ed276b4c..a8e1202de 100644 --- a/src/projectSchemas/aws-targets.ts +++ b/src/projectSchemas/aws-targets.ts @@ -1,15 +1,20 @@ import { z } from "zod"; import { uniqueBy } from "./zod-util"; +// Keep in sync with https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/agentcore-regions.html export const AgentCoreRegionSchema = z.enum([ "ap-northeast-1", "ap-northeast-2", "ap-south-1", "ap-southeast-1", "ap-southeast-2", + "ap-southeast-5", + "ap-southeast-7", "ca-central-1", "eu-central-1", "eu-north-1", + "eu-south-1", + "eu-south-2", "eu-west-1", "eu-west-2", "eu-west-3", From 550dd6191d30dbe5ef4f1df013b130578972297f Mon Sep 17 00:00:00 2001 From: notgitika Date: Thu, 20 Aug 2026 12:28:39 -0400 Subject: [PATCH 3/7] test(project): route the deploy handler test through the real CLI wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test assembled its own ValueContext with ProjectKey, JsonKey and a stub JsonRenderer, which re-implemented what createRootHandler installs — so it could pass while the real wiring was wrong. It now goes through createRootHandler with the fake manager injected via TestCoreClient, the same pattern project.test.ts and add/harness/index.test.ts use, exercising the real --json group flag, the real JSON renderer and the real withProject wrap. Verified by unwrapping deploy from withProject: both tests now fail, where the hand-built context passed. No fixture setup needed since the fake's resolve() returns a project. Also drops the CloudFormation wording from DeployResult.outputs. The shape is already backend-neutral; only the doc comment implied CDK. --- src/handlers/project/deploy/index.test.ts | 48 +++++++++++++---------- src/handlers/project/types.ts | 8 +++- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts index 9baf5ceb0..a3469dff4 100644 --- a/src/handlers/project/deploy/index.test.ts +++ b/src/handlers/project/deploy/index.test.ts @@ -1,9 +1,12 @@ import { describe, expect, test } from "bun:test"; +import { createRootHandler } from "../../index"; import { ProjectSpecSchema } from "../../../projectSchemas/project"; -import { ProjectKey, Router, ValueContext } from "../../../router"; -import { testIO } from "../../../testing"; -import { JsonRendererKey } from "../../../tui"; -import { JsonKey } from "../../keys"; +import { + createSilentLogger, + TestCoreClient, + TestGlobalConfigAccessor, + testIO, +} from "../../../testing"; import type { AddResourceInput, CreateProjectInput, @@ -13,7 +16,6 @@ import type { ProjectEvent, ProjectManager, } from "../types"; -import { createDeployProjectHandler } from "."; const project: Project = { name: "orders", @@ -21,6 +23,11 @@ const project: Project = { spec: ProjectSpecSchema.parse({ name: "orders", version: 1 }), }; +/** + * A ProjectManager whose deploy() succeeds, which the real one cannot do until + * CDK deployment is implemented. resolve() always returns a project, so these + * tests need no scaffolding on disk — withProject is satisfied by the manager. + */ function fakeProjectManager(result: DeployResult, events: ProjectEvent[] = []) { const calls: { project: Project; input: DeployProjectInput }[] = []; const manager: ProjectManager = { @@ -45,24 +52,23 @@ function fakeProjectManager(result: DeployResult, events: ProjectEvent[] = []) { return { calls, manager }; } -function harness(result: DeployResult, options: { events?: ProjectEvent[]; json?: boolean } = {}) { +// Routes through createRootHandler rather than mounting the handler directly, so +// the global --json flag, the real JSON renderer and the withProject wrap are the +// ones the CLI actually installs instead of a context assembled by hand. +function harness(result: DeployResult, events: ProjectEvent[] = []) { const io = testIO(); - const fake = fakeProjectManager(result, options.events); - const handler = createDeployProjectHandler({ projectManager: fake.manager, io: io.io }); - const ctx = ValueContext.EmptyContext() - .withValue(ProjectKey, project) - .withValue(JsonKey, options.json ?? false) - .withValue(JsonRendererKey, { - renderJson: (data) => io.io.stdout.write(`${JSON.stringify(data, null, 2)}\n`), - renderJsonLine: (data) => io.io.stdout.write(`${JSON.stringify(data)}\n`), - }); - const router = new Router("project", "test"); - router.handler(handler); + const fake = fakeProjectManager(result, events); + const core = new TestCoreClient({ projectManager: fake.manager }); + const root = createRootHandler(core, { + io: io.io, + globalConfigAccessor: new TestGlobalConfigAccessor(), + logger: createSilentLogger(), + }); return { ...fake, io, - run: (args: string[] = []) => router.route(["node", "project", "deploy", ...args], ctx), + run: (args: string[] = []) => root.route(["node", "agentcore", "project", "deploy", ...args]), }; } @@ -70,7 +76,7 @@ describe("project deploy handler", () => { test("defaults to the default target and keeps progress off stdout", async () => { const subject = harness( { outputs: { ZetaUrl: "https://zeta.example", AlphaArn: "arn:alpha" } }, - { events: [{ message: "Preparing deployment" }, { message: "Deploying stack" }] }, + [{ message: "Preparing deployment" }, { message: "Deploying stack" }], ); await subject.run(); @@ -83,9 +89,9 @@ describe("project deploy handler", () => { test("passes an explicit target and renders the result as JSON", async () => { const result = { outputs: { ServiceUrl: "https://service.example" } }; - const subject = harness(result, { json: true }); + const subject = harness(result); - await subject.run(["--target", "staging"]); + await subject.run(["--target", "staging", "--json"]); expect(subject.calls).toEqual([{ project, input: { target: "staging" } }]); expect(JSON.parse(subject.io.stdout())).toEqual(result); diff --git a/src/handlers/project/types.ts b/src/handlers/project/types.ts index 6992e185b..669d3f399 100644 --- a/src/handlers/project/types.ts +++ b/src/handlers/project/types.ts @@ -35,7 +35,13 @@ export type DeployProjectInput = { }; export type DeployResult = { - /** Outputs returned by the deployed stack, keyed by CloudFormation output name. */ + /** + * Named outputs the deployment produced, e.g. a runtime ARN or a gateway URL. + * Each backend maps its own notion of outputs into this shape (CDK reads + * CloudFormation stack outputs; a terraform backend would read `terraform + * output`), so no individual key is part of the contract — callers render the + * map rather than indexing into it. + */ outputs: Record; }; From d61a7b5ae8b4c71d4d284e6c3bef5941e6876bc5 Mon Sep 17 00:00:00 2001 From: notgitika Date: Fri, 21 Aug 2026 12:00:53 -0400 Subject: [PATCH 4/7] refactor(project): dispatch deploy through the project backend Resolve the named aws-targets.json entry in FsProjectManager and pass the resolved account and region to the backend, mirroring build(). deploy() previously threw NotImplementedError before dispatching, which left CdkBackend.deploy unreachable and duplicated the same message in two places; the backend is now the single place that reports deployment as unimplemented. Narrow the TestCoreClient seam from a whole-manager override to a single backend override. Handler tests keep the real FsProjectManager, so target resolution and withProject run for real and only the deploy boundary is faked. --- src/core/project/manager.tsx | 38 ++++++- src/handlers/project/deploy/index.test.ts | 130 ++++++++++++++-------- src/handlers/project/deploy/index.ts | 11 +- src/handlers/project/project.test.ts | 16 ++- src/testing/TestCoreClient.tsx | 26 +++-- 5 files changed, 154 insertions(+), 67 deletions(-) diff --git a/src/core/project/manager.tsx b/src/core/project/manager.tsx index 223a3a879..c086e5afc 100644 --- a/src/core/project/manager.tsx +++ b/src/core/project/manager.tsx @@ -35,6 +35,9 @@ import type { HarnessSpecSchema } from "../../projectSchemas/harness"; import z from "zod"; import { CdkBackend } from "./backends/cdk"; import type { ProjectBackend } from "./backends/types"; +import { AwsDeploymentTargetsSchema } from "../../projectSchemas/aws-targets"; + +const TARGETS_EXAMPLE = '[{ "name": "default", "account": "111122223333", "region": "us-east-1" }]'; type ProjectManagerConfig = { logger: Logger; @@ -284,12 +287,39 @@ export class FsProjectManager implements ProjectManager { yield* this.backendFor(project).build(project); } + // Resolves the named target from aws-targets.json before handing off, so the + // backend receives a fully resolved account and region and never has to know + // how targets are stored. The backend owns everything after that point. public async *deploy( - _project: Project, - _input: DeployProjectInput, + project: Project, + input: DeployProjectInput, ): AsyncGenerator { - yield* []; - throw new NotImplementedError("agentcore project deploy is not implemented yet"); + const targetsPath = join(project.rootPath, "agentcore", "aws-targets.json"); + if (!existsSync(targetsPath)) { + throw new ProjectStateError( + `No deployment targets are configured for project '${project.name}'. ` + + `Add ${targetsPath}, for example:\n\n${TARGETS_EXAMPLE}`, + ); + } + + const targets = await this.json.read(targetsPath, AwsDeploymentTargetsSchema); + + if (targets.length === 0) { + throw new ProjectStateError( + `No deployment targets are configured for project '${project.name}'. ` + + `Add at least one to ${targetsPath}, for example:\n\n${TARGETS_EXAMPLE}`, + ); + } + + const target = targets.find((candidate) => candidate.name === input.target); + if (!target) { + throw new ProjectStateError( + `Project '${project.name}' has no deployment target named '${input.target}'. ` + + `${targetsPath} defines: ${targets.map(({ name }) => name).join(", ")}.`, + ); + } + + return yield* this.backendFor(project).deploy(project, { target }); } private backendFor(project: Project): ProjectBackend { diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts index a3469dff4..742685419 100644 --- a/src/handlers/project/deploy/index.test.ts +++ b/src/handlers/project/deploy/index.test.ts @@ -1,64 +1,53 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; import { createRootHandler } from "../../index"; -import { ProjectSpecSchema } from "../../../projectSchemas/project"; import { createSilentLogger, TestCoreClient, TestGlobalConfigAccessor, testIO, } from "../../../testing"; -import type { - AddResourceInput, - CreateProjectInput, - DeployProjectInput, - DeployResult, - Project, - ProjectEvent, - ProjectManager, -} from "../types"; - -const project: Project = { - name: "orders", - rootPath: "/workspace/orders", - spec: ProjectSpecSchema.parse({ name: "orders", version: 1 }), +import type { DeployBackendInput, ProjectBackend } from "../../../core/project"; +import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; +import type { DeployResult, Project, ProjectEvent } from "../types"; + +const DEFAULT_TARGET: AwsDeploymentTarget = { + name: "default", + account: "111122223333", + region: "us-east-1", +}; +const STAGING_TARGET: AwsDeploymentTarget = { + name: "staging", + account: "444455556666", + region: "eu-west-1", }; +const TARGETS = [DEFAULT_TARGET, STAGING_TARGET]; /** - * A ProjectManager whose deploy() succeeds, which the real one cannot do until - * CDK deployment is implemented. resolve() always returns a project, so these - * tests need no scaffolding on disk — withProject is satisfied by the manager. + * A ProjectBackend that deploys successfully, which CdkBackend cannot do until + * CDK deployment is implemented. Stubbing the backend rather than the whole + * manager keeps the real FsProjectManager in the path, so target resolution and + * withProject run for real. */ -function fakeProjectManager(result: DeployResult, events: ProjectEvent[] = []) { - const calls: { project: Project; input: DeployProjectInput }[] = []; - const manager: ProjectManager = { - async *create(_input: CreateProjectInput) { - yield* []; - return project; - }, - async *build(_project: Project) {}, - async *deploy(deployedProject, input) { - calls.push({ project: deployedProject, input }); +function fakeBackend(result: DeployResult, events: ProjectEvent[] = []) { + const calls: { project: Project; input: DeployBackendInput }[] = []; + const backend: ProjectBackend = { + async *build() {}, + async *deploy(project, input) { + calls.push({ project, input }); yield* events; return result; }, - async resolve() { - return project; - }, - async *addResource(_project: Project, _input: AddResourceInput) { - yield* []; - return project; - }, }; - return { calls, manager }; + return { calls, backend }; } -// Routes through createRootHandler rather than mounting the handler directly, so -// the global --json flag, the real JSON renderer and the withProject wrap are the -// ones the CLI actually installs instead of a context assembled by hand. function harness(result: DeployResult, events: ProjectEvent[] = []) { const io = testIO(); - const fake = fakeProjectManager(result, events); - const core = new TestCoreClient({ projectManager: fake.manager }); + const fake = fakeBackend(result, events); + const core = new TestCoreClient({ backends: { CDK: fake.backend } }); const root = createRootHandler(core, { io: io.io, globalConfigAccessor: new TestGlobalConfigAccessor(), @@ -69,19 +58,53 @@ function harness(result: DeployResult, events: ProjectEvent[] = []) { ...fake, io, run: (args: string[] = []) => root.route(["node", "agentcore", "project", "deploy", ...args]), + create: (args: string[]) => root.route(["node", "agentcore", "project", ...args]), }; } +const originalCwd = process.cwd(); +const tempDirectories: string[] = []; + +async function inTempDirectory(): Promise { + const directory = await mkdtemp(join(tmpdir(), "agentcore-deploy-")); + tempDirectories.push(directory); + process.chdir(directory); + // cwd is the realpath (macOS tmpdir lives behind a /var -> /private/var + // symlink), matching the paths the manager derives from process.cwd(). + return process.cwd(); +} + +afterEach(async () => { + process.chdir(originalCwd); + await Promise.all( + tempDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true })), + ); +}); + +/** Scaffolds a project with deployment targets and cds into it. */ +async function inProjectWithTargets( + subject: ReturnType, + targets: unknown = TARGETS, +): Promise { + const directory = await inTempDirectory(); + await subject.create(["create", "--name", "orders", "--skip-install", "--skip-git"]); + const projectRoot = join(directory, "orders"); + await writeFile(join(projectRoot, "agentcore", "aws-targets.json"), JSON.stringify(targets)); + process.chdir(projectRoot); + return projectRoot; +} + describe("project deploy handler", () => { test("defaults to the default target and keeps progress off stdout", async () => { const subject = harness( { outputs: { ZetaUrl: "https://zeta.example", AlphaArn: "arn:alpha" } }, [{ message: "Preparing deployment" }, { message: "Deploying stack" }], ); + await inProjectWithTargets(subject); await subject.run(); - expect(subject.calls).toEqual([{ project, input: { target: "default" } }]); + expect(subject.calls.map(({ input }) => input)).toEqual([{ target: DEFAULT_TARGET }]); expect(subject.io.stderr()).toContain("Preparing deployment\nDeploying stack"); expect(subject.io.stderr()).toContain("Deployed project 'orders' to target 'default'"); expect(subject.io.stdout()).toBe("AlphaArn: arn:alpha\nZetaUrl: https://zeta.example"); @@ -90,10 +113,29 @@ describe("project deploy handler", () => { test("passes an explicit target and renders the result as JSON", async () => { const result = { outputs: { ServiceUrl: "https://service.example" } }; const subject = harness(result); + await inProjectWithTargets(subject); await subject.run(["--target", "staging", "--json"]); - expect(subject.calls).toEqual([{ project, input: { target: "staging" } }]); + expect(subject.calls.map(({ input }) => input)).toEqual([{ target: STAGING_TARGET }]); expect(JSON.parse(subject.io.stdout())).toEqual(result); }); + + test("rejects an unknown target without invoking the backend", async () => { + const subject = harness({ outputs: {} }); + await inProjectWithTargets(subject); + + await expect(subject.run(["--target", "nope"])).rejects.toThrow( + /no deployment target named 'nope'/, + ); + expect(subject.calls).toEqual([]); + }); + + test("requires deployment targets to be configured", async () => { + const subject = harness({ outputs: {} }); + await inProjectWithTargets(subject, []); + + await expect(subject.run()).rejects.toThrow(/No deployment targets are configured/); + expect(subject.calls).toEqual([]); + }); }); diff --git a/src/handlers/project/deploy/index.ts b/src/handlers/project/deploy/index.ts index 6e37461fe..22a875249 100644 --- a/src/handlers/project/deploy/index.ts +++ b/src/handlers/project/deploy/index.ts @@ -3,7 +3,7 @@ import type { AppIO } from "../../../io"; import { createHandler, flag, ProjectKey } from "../../../router"; import { JsonRendererKey } from "../../../tui"; import { JsonKey } from "../../keys"; -import type { DeployResult, ProjectEvent, ProjectManager } from "../types"; +import type { DeployResult, ProjectManager } from "../types"; type DeployProjectHandlerConfig = { projectManager: ProjectManager; @@ -16,11 +16,12 @@ async function runDeploy( target: string, ): Promise { const deployment = config.projectManager.deploy(project, { target }); - while (true) { - const next = await deployment.next(); - if (next.done) return next.value; - config.io.stderr.write(`${(next.value as ProjectEvent).message}\n`); + let next = await deployment.next(); + while (!next.done) { + config.io.stderr.write(`${next.value.message}\n`); + next = await deployment.next(); } + return next.value; } function renderResult(io: AppIO, result: DeployResult): void { diff --git a/src/handlers/project/project.test.ts b/src/handlers/project/project.test.ts index 390ebd074..7d0036e85 100644 --- a/src/handlers/project/project.test.ts +++ b/src/handlers/project/project.test.ts @@ -1,6 +1,6 @@ import { afterEach, test, expect, describe } from "bun:test"; import { existsSync } from "node:fs"; -import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { createRootHandler } from "../index"; @@ -353,8 +353,20 @@ describe("project deploy", () => { await expect(run(["deploy"])).rejects.toThrow(/No AgentCore project found/); }); - test("remains nonfunctional until deployment support is implemented", async () => { + test("reports that a freshly scaffolded project has no deployment targets", async () => { await inProject(); + await expect(run(["deploy"])).rejects.toThrow(/No deployment targets are configured/); + }); + + // Proves the manager reaches CdkBackend.deploy once a target resolves; the + // backend is what remains unimplemented until the CDK deployment PR. + test("remains nonfunctional until deployment support is implemented", async () => { + const projectRoot = await inProject(); + await writeFile( + join(projectRoot, "agentcore", "aws-targets.json"), + JSON.stringify([{ name: "default", account: "111122223333", region: "us-east-1" }]), + ); + await expect(run(["deploy"])).rejects.toThrow(/not implemented/); }); }); diff --git a/src/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index 580ee111c..2238b132c 100644 --- a/src/testing/TestCoreClient.tsx +++ b/src/testing/TestCoreClient.tsx @@ -148,7 +148,8 @@ import type { ProjectManager } from "../handlers/project/types"; import type { Logger } from "../logging"; import type { ReadWriteJson } from "../io"; import { createSilentLogger } from "./logging"; -import { FsProjectManager } from "../core/project"; +import { FsProjectManager, type ProjectBackend } from "../core/project"; +import type { ManagedBy } from "../projectSchemas/project"; // TestCoreClient is a hand-controllable `Core` for tests. It implements the same // interface the real CoreClient satisfies, so it drops straight into @@ -1179,7 +1180,9 @@ export class TestGatewayClient implements CoreGatewayClient { type TestCoreClientOptions = { logger?: Logger; json?: ReadWriteJson; - projectManager?: ProjectManager; + // Stubs one backend rather than the whole manager, so tests keep the real + // FsProjectManager and only the build/deploy boundary is faked. + backends?: Partial>; }; export class TestIdentityClient implements CoreIdentityClient { @@ -1949,15 +1952,14 @@ export class TestCoreClient implements Core { readonly projectCommands: { command: string[]; cwd: string }[] = []; constructor(options?: TestCoreClientOptions) { - this.projectManager = - options?.projectManager ?? - new FsProjectManager({ - logger: options?.logger ?? createSilentLogger(), - json: options?.json, - runner: async (command, { cwd }) => { - this.projectCommands.push({ command, cwd }); - }, - checkTool: async () => {}, // CI hosts don't have uv installed - }); + this.projectManager = new FsProjectManager({ + logger: options?.logger ?? createSilentLogger(), + json: options?.json, + backends: options?.backends, + runner: async (command, { cwd }) => { + this.projectCommands.push({ command, cwd }); + }, + checkTool: async () => {}, // CI hosts don't have uv installed + }); } } From b56cfeebdbd64cc00ed4dd51f22090be2a4d312d Mon Sep 17 00:00:00 2001 From: notgitika Date: Fri, 21 Aug 2026 13:26:58 -0400 Subject: [PATCH 5/7] fix(project): reject underscores in deployment target names Match the name pattern the schema on main already enforces. The CDK normalizes underscores to hyphens, so accepting them here would let a target be written one way and deployed under another. --- src/projectSchemas/aws-targets.test.ts | 2 +- src/projectSchemas/aws-targets.ts | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/projectSchemas/aws-targets.test.ts b/src/projectSchemas/aws-targets.test.ts index 742305361..3e0d695b8 100644 --- a/src/projectSchemas/aws-targets.test.ts +++ b/src/projectSchemas/aws-targets.test.ts @@ -29,7 +29,7 @@ describe("AWS deployment targets", () => { expect(AwsAccountIdSchema.safeParse(account).success).toBe(false); }); - test.each(["", "1default", "-default", "_default", "has spaces", "has.dots"])( + test.each(["", "1default", "-default", "_default", "has_underscore", "has spaces", "has.dots"])( "rejects invalid target name %j", (name) => { expect(DeploymentTargetNameSchema.safeParse(name).success).toBe(false); diff --git a/src/projectSchemas/aws-targets.ts b/src/projectSchemas/aws-targets.ts index a8e1202de..6d847db2d 100644 --- a/src/projectSchemas/aws-targets.ts +++ b/src/projectSchemas/aws-targets.ts @@ -29,9 +29,11 @@ export const DeploymentTargetNameSchema = z .string() .min(1) .max(64) + // Underscores are rejected up front even though the CDK normalizes them to + // hyphens, so a target name means the same thing everywhere it appears. .regex( - /^[a-zA-Z][a-zA-Z0-9_-]*$/, - "Name must start with a letter and contain only alphanumeric characters, hyphens, and underscores", + /^[a-zA-Z][a-zA-Z0-9-]*$/, + "Name must start with a letter and contain only alphanumeric characters and hyphens", ) .describe("Unique identifier for the deployment target"); From 26d38ca3f46ecf828770460fbc3c7a5342777c47 Mon Sep 17 00:00:00 2001 From: notgitika Date: Fri, 21 Aug 2026 14:11:34 -0400 Subject: [PATCH 6/7] fix(errors): surface why deserialization failed The root handler prints only `error.message`, so the zod detail that `FsReadWriteJson.read` put in `cause` never reached the user: a typo'd region or a duplicated target name in a hand-edited file produced only `Failed to deserialize file at ""`, naming the file but not the field. DeserializationError now requires a `details` string and appends it to the message, and json.ts passes the prettified zod error (or the parse error) through. Fixing it in the IO layer rather than catching in `ProjectManager.deploy` covers every hand-edited file at once. Making `details` required rather than optional keeps the opaque message from being reintroduced. Tested at the handler level, since the schema tests pass in isolation without proving the detail reaches stderr. --- src/errors/errors.tsx | 24 ++++---- src/handlers/project/deploy/index.test.ts | 69 +++++++++++++++++++++-- src/io/json.ts | 7 +-- 3 files changed, 82 insertions(+), 18 deletions(-) diff --git a/src/errors/errors.tsx b/src/errors/errors.tsx index 3f7523267..858c497f3 100644 --- a/src/errors/errors.tsx +++ b/src/errors/errors.tsx @@ -107,17 +107,21 @@ export class SourceResolutionError extends InputValidationError { } } +type DeserializationErrorOptions = Omit & { + /** + * Why the file could not be read. Required because the root handler prints + * only `error.message`: a detail left in `cause` never reaches the user, and + * these files are hand-edited, so naming the file without naming the bad + * field leaves them nothing to act on. + */ + details: string; +}; + export class DeserializationError extends AgentCoreCLIError { - constructor( - path: string, - options?: Omit & { - /** Rendered reasons the payload was rejected, appended so the user sees which field to fix. */ - detail?: string; - }, - ) { - const detail = options?.detail ? `\n${options.detail}` : ""; - super(`Failed to deserialize file at "${path}"${detail}`, { - ...options, + constructor(path: string, options: DeserializationErrorOptions) { + const { details, ...errorOptions } = options; + super(`Failed to deserialize file at "${path}":\n\n${details}`, { + ...errorOptions, source: ERROR_SOURCE.USER, }); this.name = "DeserializationError"; diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts index 742685419..e85ee8c65 100644 --- a/src/handlers/project/deploy/index.test.ts +++ b/src/handlers/project/deploy/index.test.ts @@ -81,19 +81,27 @@ afterEach(async () => { ); }); -/** Scaffolds a project with deployment targets and cds into it. */ -async function inProjectWithTargets( +/** Scaffolds a project whose aws-targets.json holds exactly `contents`, and cds into it. */ +async function inProjectWithRawTargets( subject: ReturnType, - targets: unknown = TARGETS, + contents: string, ): Promise { const directory = await inTempDirectory(); await subject.create(["create", "--name", "orders", "--skip-install", "--skip-git"]); const projectRoot = join(directory, "orders"); - await writeFile(join(projectRoot, "agentcore", "aws-targets.json"), JSON.stringify(targets)); + await writeFile(join(projectRoot, "agentcore", "aws-targets.json"), contents); process.chdir(projectRoot); return projectRoot; } +/** Scaffolds a project with deployment targets and cds into it. */ +function inProjectWithTargets( + subject: ReturnType, + targets: unknown = TARGETS, +): Promise { + return inProjectWithRawTargets(subject, JSON.stringify(targets)); +} + describe("project deploy handler", () => { test("defaults to the default target and keeps progress off stdout", async () => { const subject = harness( @@ -139,3 +147,56 @@ describe("project deploy handler", () => { expect(subject.calls).toEqual([]); }); }); + +// aws-targets.json is hand-edited, so these assert on the message the user +// actually sees rather than on the schema in isolation: the reporter prints only +// error.message, so a validation detail left in `cause` may as well not exist. +/** The message the user would see on stderr, since the reporter prints only that. */ +async function messageFrom(command: Promise): Promise { + try { + await command; + } catch (error) { + return (error as Error).message; + } + throw new Error("expected the command to fail"); +} + +describe("project deploy reports which field of aws-targets.json is wrong", () => { + test("names the offending field for an unsupported region", async () => { + const subject = harness({ outputs: {} }); + await inProjectWithTargets(subject, [ + { name: "default", account: "111122223333", region: "us-east-11" }, + ]); + + const message = await messageFrom(subject.run()); + + expect(message).toContain("aws-targets.json"); + expect(message).toContain("at [0].region"); + expect(message).toContain('"us-east-1"'); + expect(subject.calls).toEqual([]); + }); + + test("surfaces the duplicate target name", async () => { + const subject = harness({ outputs: {} }); + await inProjectWithTargets(subject, [DEFAULT_TARGET, DEFAULT_TARGET]); + + await expect(subject.run()).rejects.toThrow(/Duplicate deployment target name: default/); + expect(subject.calls).toEqual([]); + }); + + test("surfaces the account id rule", async () => { + const subject = harness({ outputs: {} }); + await inProjectWithTargets(subject, [{ name: "default", account: "123", region: "us-east-1" }]); + + await expect(subject.run()).rejects.toThrow(/AWS account ID must be exactly 12 digits/); + expect(subject.calls).toEqual([]); + }); + + test("surfaces the parse error for malformed json", async () => { + const subject = harness({ outputs: {} }); + await inProjectWithRawTargets(subject, '[{ "name": "default", }]'); + + await expect(subject.run()).rejects.toThrow(/JSON Parse error/); + expect(subject.calls).toEqual([]); + }); +}); diff --git a/src/io/json.ts b/src/io/json.ts index 064b1481c..d1c610b9f 100644 --- a/src/io/json.ts +++ b/src/io/json.ts @@ -1,8 +1,7 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; import { dirname } from "node:path"; -import type z from "zod"; -import { prettifyError } from "zod"; +import z from "zod"; import { DeserializationError } from "../errors"; import type { Logger } from "../logging"; @@ -34,7 +33,7 @@ export class FsReadWriteJson implements ReadWriteJson { this.logger .child({ filePath, errorName: error.name, errorMessage: error.message }) .error(`failed to parse json file`); - throw new DeserializationError(filePath, { cause: e }); + throw new DeserializationError(filePath, { cause: e, details: error.message }); } } @@ -55,7 +54,7 @@ export class FsReadWriteJson implements ReadWriteJson { .error(`failed to validate parsed json file`); throw new DeserializationError(filePath, { cause: parseResult.error, - detail: prettifyError(parseResult.error), + details: z.prettifyError(parseResult.error), }); } From e8aabe3323f09a3d8a27b36e6334c2bd83f042a9 Mon Sep 17 00:00:00 2001 From: notgitika Date: Fri, 21 Aug 2026 15:27:47 -0400 Subject: [PATCH 7/7] refactor(project): address review feedback on the deploy handler - Inline runDeploy and renderResult. Neither is reused; the manual .next() loop only exists because the outputs are the generator's return value, which `for await` discards, so that reason is now a comment where the loop lives. `build` already drains its generator inline. - Rename the test factory `harness` to `testDeployCommand`. "Harness" is a real AgentCore resource in this codebase, and the name matches the existing `testMemoryCommand` convention. - Collapse inProjectWithRawTargets/inProjectWithTargets into one helper that takes the file contents. Callers pass JSON.stringify(...), so the malformed-JSON case is just another call rather than a second function. - Drop the TestCoreClientOptions comment; the Partial already says it. - Drop the comment above the aws-targets.json tests describing the opaque message, which no longer exists now that DeserializationError requires `details`. The describe name carries the intent. --- src/handlers/project/deploy/index.test.ts | 53 ++++++++++------------- src/handlers/project/deploy/index.ts | 45 ++++++++----------- src/testing/TestCoreClient.tsx | 2 - 3 files changed, 42 insertions(+), 58 deletions(-) diff --git a/src/handlers/project/deploy/index.test.ts b/src/handlers/project/deploy/index.test.ts index e85ee8c65..6384195b7 100644 --- a/src/handlers/project/deploy/index.test.ts +++ b/src/handlers/project/deploy/index.test.ts @@ -44,7 +44,7 @@ function fakeBackend(result: DeployResult, events: ProjectEvent[] = []) { return { calls, backend }; } -function harness(result: DeployResult, events: ProjectEvent[] = []) { +function testDeployCommand(result: DeployResult, events: ProjectEvent[] = []) { const io = testIO(); const fake = fakeBackend(result, events); const core = new TestCoreClient({ backends: { CDK: fake.backend } }); @@ -82,9 +82,9 @@ afterEach(async () => { }); /** Scaffolds a project whose aws-targets.json holds exactly `contents`, and cds into it. */ -async function inProjectWithRawTargets( - subject: ReturnType, - contents: string, +async function inProjectWithTargets( + subject: ReturnType, + contents: string = JSON.stringify(TARGETS), ): Promise { const directory = await inTempDirectory(); await subject.create(["create", "--name", "orders", "--skip-install", "--skip-git"]); @@ -94,17 +94,9 @@ async function inProjectWithRawTargets( return projectRoot; } -/** Scaffolds a project with deployment targets and cds into it. */ -function inProjectWithTargets( - subject: ReturnType, - targets: unknown = TARGETS, -): Promise { - return inProjectWithRawTargets(subject, JSON.stringify(targets)); -} - describe("project deploy handler", () => { test("defaults to the default target and keeps progress off stdout", async () => { - const subject = harness( + const subject = testDeployCommand( { outputs: { ZetaUrl: "https://zeta.example", AlphaArn: "arn:alpha" } }, [{ message: "Preparing deployment" }, { message: "Deploying stack" }], ); @@ -120,7 +112,7 @@ describe("project deploy handler", () => { test("passes an explicit target and renders the result as JSON", async () => { const result = { outputs: { ServiceUrl: "https://service.example" } }; - const subject = harness(result); + const subject = testDeployCommand(result); await inProjectWithTargets(subject); await subject.run(["--target", "staging", "--json"]); @@ -130,7 +122,7 @@ describe("project deploy handler", () => { }); test("rejects an unknown target without invoking the backend", async () => { - const subject = harness({ outputs: {} }); + const subject = testDeployCommand({ outputs: {} }); await inProjectWithTargets(subject); await expect(subject.run(["--target", "nope"])).rejects.toThrow( @@ -140,17 +132,14 @@ describe("project deploy handler", () => { }); test("requires deployment targets to be configured", async () => { - const subject = harness({ outputs: {} }); - await inProjectWithTargets(subject, []); + const subject = testDeployCommand({ outputs: {} }); + await inProjectWithTargets(subject, JSON.stringify([])); await expect(subject.run()).rejects.toThrow(/No deployment targets are configured/); expect(subject.calls).toEqual([]); }); }); -// aws-targets.json is hand-edited, so these assert on the message the user -// actually sees rather than on the schema in isolation: the reporter prints only -// error.message, so a validation detail left in `cause` may as well not exist. /** The message the user would see on stderr, since the reporter prints only that. */ async function messageFrom(command: Promise): Promise { try { @@ -163,10 +152,11 @@ async function messageFrom(command: Promise): Promise { describe("project deploy reports which field of aws-targets.json is wrong", () => { test("names the offending field for an unsupported region", async () => { - const subject = harness({ outputs: {} }); - await inProjectWithTargets(subject, [ - { name: "default", account: "111122223333", region: "us-east-11" }, - ]); + const subject = testDeployCommand({ outputs: {} }); + await inProjectWithTargets( + subject, + JSON.stringify([{ name: "default", account: "111122223333", region: "us-east-11" }]), + ); const message = await messageFrom(subject.run()); @@ -177,24 +167,27 @@ describe("project deploy reports which field of aws-targets.json is wrong", () = }); test("surfaces the duplicate target name", async () => { - const subject = harness({ outputs: {} }); - await inProjectWithTargets(subject, [DEFAULT_TARGET, DEFAULT_TARGET]); + const subject = testDeployCommand({ outputs: {} }); + await inProjectWithTargets(subject, JSON.stringify([DEFAULT_TARGET, DEFAULT_TARGET])); await expect(subject.run()).rejects.toThrow(/Duplicate deployment target name: default/); expect(subject.calls).toEqual([]); }); test("surfaces the account id rule", async () => { - const subject = harness({ outputs: {} }); - await inProjectWithTargets(subject, [{ name: "default", account: "123", region: "us-east-1" }]); + const subject = testDeployCommand({ outputs: {} }); + await inProjectWithTargets( + subject, + JSON.stringify([{ name: "default", account: "123", region: "us-east-1" }]), + ); await expect(subject.run()).rejects.toThrow(/AWS account ID must be exactly 12 digits/); expect(subject.calls).toEqual([]); }); test("surfaces the parse error for malformed json", async () => { - const subject = harness({ outputs: {} }); - await inProjectWithRawTargets(subject, '[{ "name": "default", }]'); + const subject = testDeployCommand({ outputs: {} }); + await inProjectWithTargets(subject, '[{ "name": "default", }]'); await expect(subject.run()).rejects.toThrow(/JSON Parse error/); expect(subject.calls).toEqual([]); diff --git a/src/handlers/project/deploy/index.ts b/src/handlers/project/deploy/index.ts index 22a875249..50fda3346 100644 --- a/src/handlers/project/deploy/index.ts +++ b/src/handlers/project/deploy/index.ts @@ -3,35 +3,13 @@ import type { AppIO } from "../../../io"; import { createHandler, flag, ProjectKey } from "../../../router"; import { JsonRendererKey } from "../../../tui"; import { JsonKey } from "../../keys"; -import type { DeployResult, ProjectManager } from "../types"; +import type { ProjectManager } from "../types"; type DeployProjectHandlerConfig = { projectManager: ProjectManager; io: AppIO; }; -async function runDeploy( - config: DeployProjectHandlerConfig, - project: Parameters[0], - target: string, -): Promise { - const deployment = config.projectManager.deploy(project, { target }); - let next = await deployment.next(); - while (!next.done) { - config.io.stderr.write(`${next.value.message}\n`); - next = await deployment.next(); - } - return next.value; -} - -function renderResult(io: AppIO, result: DeployResult): void { - for (const [key, value] of Object.entries(result.outputs).sort(([a], [b]) => - a.localeCompare(b), - )) { - io.stdout.write(`${key}: ${value}\n`); - } -} - export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) => createHandler({ name: "deploy", @@ -40,14 +18,29 @@ export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) = flag("target", "name of the aws-targets.json entry to deploy", z.string().default("default")), ], handle: async (ctx, flags) => { + // withProject has already resolved the enclosing project. const project = ctx.require(ProjectKey); - const result = await runDeploy(config, project, flags.target); + + // Progress goes to stderr, keeping stdout for machine output. Driven by + // hand rather than `for await` because the outputs we render below are the + // generator's return value, which `for await` discards. + const deployment = config.projectManager.deploy(project, { target: flags.target }); + let next = await deployment.next(); + while (!next.done) { + config.io.stderr.write(`${next.value.message}\n`); + next = await deployment.next(); + } + const result = next.value; config.io.stderr.write(`Deployed project '${project.name}' to target '${flags.target}'\n`); if (ctx.require(JsonKey)) { ctx.require(JsonRendererKey).renderJson(result); - } else { - renderResult(config.io, result); + return; + } + for (const [key, value] of Object.entries(result.outputs).sort(([a], [b]) => + a.localeCompare(b), + )) { + config.io.stdout.write(`${key}: ${value}\n`); } }, }); diff --git a/src/testing/TestCoreClient.tsx b/src/testing/TestCoreClient.tsx index 2238b132c..b97c8594f 100644 --- a/src/testing/TestCoreClient.tsx +++ b/src/testing/TestCoreClient.tsx @@ -1180,8 +1180,6 @@ export class TestGatewayClient implements CoreGatewayClient { type TestCoreClientOptions = { logger?: Logger; json?: ReadWriteJson; - // Stubs one backend rather than the whole manager, so tests keep the real - // FsProjectManager and only the build/deploy boundary is faked. backends?: Partial>; };