Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions src/core/project/manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ describe("FsProjectManager.create", () => {
build: "CodeZip",
entrypoint: "main.py",
codeLocation: "app/hello-world",
runtimeVersion: "PYTHON_3_14",
},
]);
expect(await Bun.file(join(configDir, "aws-targets.json")).json()).toEqual([]);
Expand Down Expand Up @@ -217,6 +218,89 @@ describe("FsProjectManager.create", () => {
});
});

describe("FsProjectManager.build", () => {
// build() requires the CDK app's node_modules; create() with skipInstall
// never produces them, so tests stub the directory in.
async function scaffolded(
subject: FsProjectManager,
directory: string,
withDependencies = true,
): Promise<Project> {
const { project } = await runCreate(subject, {
name: "example",
template: PROJECT_TEMPLATES.HELLO_WORLD_PYTHON,
skipInstall: true,
skipGit: true,
});
if (withDependencies) {
await mkdir(join(directory, "example", "agentcore", "cdk", "node_modules"), {
recursive: true,
});
}
return project;
}

async function drain(generator: AsyncGenerator<ProjectEvent, void>): Promise<ProjectEvent[]> {
const events: ProjectEvent[] = [];
for await (const event of generator) events.push(event);
return events;
}

test("compiles and synthesizes via the generated cdk script", async () => {
const directory = await inTempDirectory();
const { manager: subject, commands } = manager();
const project = await scaffolded(subject, directory);
commands.length = 0; // discard create()'s commands

const events = await drain(subject.build(project));

expect(commands).toEqual([
{
command: ["npm", "run", "cdk", "--", "synth", "--quiet"],
cwd: join(directory, "example", "agentcore", "cdk"),
},
]);
expect(events).toEqual([{ message: "Synthesizing CloudFormation templates" }]);
});

test("fails actionably when the CDK dependencies are missing", async () => {
const directory = await inTempDirectory();
const { manager: subject, commands } = manager();
const project = await scaffolded(subject, directory, false);
commands.length = 0;

await expect(drain(subject.build(project))).rejects.toThrow(/npm install/);
expect(commands).toEqual([]);
});

test("refuses a project managed by a backend it cannot build", async () => {
const directory = await inTempDirectory();
const { manager: subject, commands } = manager();
const project = await scaffolded(subject, directory);
commands.length = 0;

// CDK is the only backend today; the cast stands in for a future one.
const foreign = { ...project, managedBy: "Terraform" as Project["managedBy"] };
await expect(drain(subject.build(foreign))).rejects.toThrow(/unsupported backend: Terraform/);
expect(commands).toEqual([]);
});

test("propagates a synthesis failure", async () => {
const directory = await inTempDirectory();
const { manager: subject } = manager();
const project = await scaffolded(subject, directory);
const failing = new FsProjectManager({
logger: createSilentLogger(),
runner: async () => {
throw new Error("cdk synth exploded");
},
checkTool: async () => {},
});

await expect(drain(failing.build(project))).rejects.toThrow("cdk synth exploded");
});
});

describe("FsProjectManager.resolve", () => {
test("round-trips a project it just created", async () => {
const root = await inTempDirectory();
Expand All @@ -228,6 +312,7 @@ describe("FsProjectManager.resolve", () => {

expect(resolved?.name).toBe("example");
expect(resolved?.rootPath).toBe(join(root, "example"));
expect(resolved?.managedBy).toBe("CDK");
expect(resolved?.runtimes).toHaveLength(1);
});

Expand Down
46 changes: 45 additions & 1 deletion src/core/project/manager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ export class FsProjectManager implements ProjectManager {
const configPath = join(rootPath, "agentcore", "agentcore.json");
try {
const spec = await this.json.read(configPath, ProjectSpecSchema);
return { name: spec.name, rootPath, runtimes: spec.runtimes };
return {
name: spec.name,
rootPath,
managedBy: spec.managedBy,
runtimes: spec.runtimes,
};
} catch (error) {
// A malformed agentcore.json is a user-correctable problem, not a crash.
if (error instanceof DeserializationError) {
Expand Down Expand Up @@ -118,6 +123,45 @@ export class FsProjectManager implements ProjectManager {
return project;
}

public async *build(project: Project): AsyncGenerator<ProjectEvent, void> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this build impl handle alternative project backends like terraform and SDK if we chose to implement those in the future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit I just pushed, we use the managedBy field in agentcore.json

// agentcore.json records which backend owns the project's artifacts. CDK is the
// only one today; a terraform or no-IaC backend adds an arm here rather than
// editing the CDK path.
switch (project.managedBy) {
case "CDK":
yield* this.buildWithCdk(project);
break;
default: {
// Exhaustiveness: a new ManagedBy member fails to compile until it is handled.
const unsupported: never = project.managedBy;
throw new ProjectStateError(
`project '${project.name}' declares an unsupported backend: ${String(unsupported)}`,
);
}
}
}

// Compiles the generated CDK app and synthesizes its CloudFormation templates.
private async *buildWithCdk(project: Project): AsyncGenerator<ProjectEvent, void> {
const cdkDir = join(project.rootPath, "agentcore", "cdk");

// The generated CDK app is built from its own node_modules; without them the
// failure would otherwise surface as an opaque "cdk: not found".
if (!existsSync(join(cdkDir, "node_modules"))) {
throw new ProjectStateError(
`CDK dependencies are missing for project '${project.name}'. ` +
`Run 'cd ${cdkDir} && npm install'.`,
);
}
await this.checkTool("npm", "Install Node.js: https://nodejs.org/");

// The generated package.json defines `cdk` as "npm run build && cdk", so this
// single command compiles the app and then synthesizes it. Synthesis needs no
// AWS credentials: each stack's environment comes from aws-targets.json.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking and fine as a follow-up: synthesis succeeds without credentials, but AWS_PROFILE currently causes the pinned ConfigIO.readAWSDeploymentTargets() to call STS even when every target already has an account. I confirmed this by redirecting STS locally. The build still succeeded, but made six GetCallerIdentity attempts and inherited the retry latency. Could we avoid that fallback when account values are already present so build is fully offline?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

creating a follow up issue for this. thanks for catching this!

yield { message: "Synthesizing CloudFormation templates" };
await this.run(["npm", "run", "cdk", "--", "synth", "--quiet"], cdkDir);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we not using @aws-cdk/toolkit-lib anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I went with a subprocess to keep the PR small. using toolkit-lib means porting the wrapper and the schema pinning first, and deploy is what actually needs those. So I will introduce it when I introduce deploy. It should be easy to switch later, build() already yields events so it's a 2-way door decision

Comment on lines +156 to +162

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we had discussed that build would:

  • Validate Schemas
  • Generate ZIP artifacts for CodeZIP Agents
  • Run CDK Synth

I'm not sure if Generate ZIP artifacts for CodeZIP Agents is happening here or if we decided to move that to a different PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three are happening. schema validation is up front, withProject resolves the project through ProjectSpecSchema, so build can't run on an invalid agentcore.json (#1972 makes that catch the CodeZip runtimeVersion rule and name the field).

the ZIP is generated by synth: the construct library's packager runs uv pip install and stages the asset, so cdk.out gets the zip and the template's CodeConfiguration.Code.S3 points at it.

I validated and checked a real synth to be sure. so there's no separate ZIP step to write, doing our own would duplicate the packager deploy relies on. LMK what you think

}

// Runs a command with its output streamed to the file logger.
private run(command: string[], cwd: string): Promise<void> {
return this.runner(command, { cwd, onOutput: (chunk) => this.logger.debug(chunk) });
Expand Down
4 changes: 4 additions & 0 deletions src/core/project/templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export const TEMPLATES: Record<ProjectTemplate, Template> = {
build: "CodeZip",
entrypoint: "main.py",
codeLocation: "app/hello-world",
// Required for CodeZip builds: the CDK construct library rejects a
// CodeZip runtime with no runtimeVersion, and it is what selects the
// packager. Container builds take their version from the image.
runtimeVersion: "PYTHON_3_14",
},
],
},
Expand Down
25 changes: 20 additions & 5 deletions src/handlers/project/build/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
import { createHandler } from "../../../router";
import { NotImplementedError } from "../../../errors";
import { createHandler, ProjectKey } from "../../../router";
import type { AppIO } from "../../../io";
import type { ProjectManager } from "../types";

export const createBuildProjectHandler = () =>
type BuildProjectHandlerConfig = {
projectManager: ProjectManager;
io: AppIO;
};

export const createBuildProjectHandler = (config: BuildProjectHandlerConfig) =>
createHandler({
name: "build",
description: "build the project's deployable artifacts",
handle: async () => {
throw new NotImplementedError("agentcore project build is not implemented yet");
handle: async (ctx) => {
// withProject has already resolved the enclosing project.
const project = ctx.require(ProjectKey);

// Progress goes to stderr, keeping stdout for machine output. Subprocess
// output goes to the debug log; on failure ProcessFailedError carries it.
for await (const event of config.projectManager.build(project)) {
config.io.stderr.write(`${event.message}\n`);
}

config.io.stderr.write(`Built project '${project.name}'\n`);
Comment on lines +14 to +24

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that this is kept clean. It doesn't need to be aware of the build backend or any of the other steps.

},
});
9 changes: 8 additions & 1 deletion src/handlers/project/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Router } from "../../router";
import { withProject } from "../../middleware";
import type { AppIO } from "../../io";
import { createCreateProjectHandler } from "./create";
import { createAddProjectHandler } from "./add";
Expand All @@ -25,7 +26,13 @@ export function createProjectHandler(config: ProjectHandlerConfig): Router {
project.handler(createDevProjectHandler());
project.handler(createDeployProjectHandler());
project.handler(createStatusProjectHandler());
project.handler(createBuildProjectHandler());
// withProject wraps only the commands that require an existing project, so
// `create` (which refuses to nest inside one) stays unaffected.
project.handler(
withProject({ projectManager: config.projectManager })(
createBuildProjectHandler({ projectManager: config.projectManager, io: config.io }),
),
);

return project;
}
55 changes: 53 additions & 2 deletions src/handlers/project/project.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { afterEach, test, expect, describe } from "bun:test";
import { mkdtemp, rm } from "node:fs/promises";
import { mkdir, mkdtemp, rm } from "node:fs/promises";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { createRootHandler } from "../index";
Expand All @@ -22,7 +22,7 @@ async function run(args: string[]) {
return { io, core };
}

describe.each(["add", "remove", "dev", "deploy", "status", "build"])("project %s", (command) => {
describe.each(["add", "remove", "dev", "deploy", "status"])("project %s", (command) => {
test("throws because it is not implemented yet", async () => {
await expect(run([command])).rejects.toThrow(/not implemented/);
});
Expand Down Expand Up @@ -97,3 +97,54 @@ describe("project create", () => {
await expect(run(["create", "--name", "MyAgent", "--template", "nonsense"])).rejects.toThrow();
});
});

describe("project build", () => {
// Scaffolds a project, then runs from inside it so withProject resolves it.
async function inProject(): Promise<string> {
const directory = await inTempDirectory();
await run(["create", "--name", "MyAgent", "--skip-install", "--skip-git"]);

const projectRoot = join(directory, "MyAgent");
// create --skip-install leaves no node_modules, which build requires.
await mkdir(join(projectRoot, "agentcore", "cdk", "node_modules"), { recursive: true });
process.chdir(projectRoot);
return projectRoot;
}

test("synthesizes the CDK app of the enclosing project", async () => {
const projectRoot = await inProject();
const { io, core } = await run(["build"]);

expect(core.projectCommands).toEqual([
{
command: ["npm", "run", "cdk", "--", "synth", "--quiet"],
cwd: join(projectRoot, "agentcore", "cdk"),
},
]);
expect(io.stderr()).toContain("Synthesizing CloudFormation templates");
expect(io.stderr()).toContain("Built project 'MyAgent'");
});

test("resolves the project from a nested directory", async () => {
const projectRoot = await inProject();
process.chdir(join(projectRoot, "app", "hello-world"));

const { core } = await run(["build"]);

expect(core.projectCommands.map(({ cwd }) => cwd)).toEqual([
join(projectRoot, "agentcore", "cdk"),
]);
});

test("fails with actionable guidance outside a project", async () => {
await inTempDirectory();
await expect(run(["build"])).rejects.toThrow(/No AgentCore project found/);
});

test("fails when the CDK dependencies have not been installed", async () => {
const projectRoot = await inProject();
await rm(join(projectRoot, "agentcore", "cdk", "node_modules"), { recursive: true });

await expect(run(["build"])).rejects.toThrow(/npm install/);
});
});
6 changes: 6 additions & 0 deletions src/handlers/project/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { ManagedBy } from "../../projectSchemas/project";
import type { ProjectRuntime } from "../../projectSchemas/runtime";

/** Available project templates for scaffolding new AgentCore projects. */
Expand Down Expand Up @@ -33,6 +34,8 @@ export type Project = {
name: string;
/** Absolute path to the project root (the parent of agentcore/). */
rootPath: string;
/** The infrastructure backend that owns the project's deployable artifacts. */
managedBy: ManagedBy;
/** The runtimes registered in agentcore.json. */
runtimes: ProjectRuntime[];
};
Expand All @@ -44,6 +47,9 @@ export interface ProjectManager {
/** Scaffold a new AgentCore project from the given template. */
create(input: CreateProjectInput): AsyncGenerator<ProjectEvent, Project>;

/** Compile the project's CDK app and synthesize its CloudFormation templates. */
build(project: Project): AsyncGenerator<ProjectEvent, void>;

/** Locate an existing AgentCore project. Returns undefined if no project can be found. */
resolve(input: ResolveProjectInput): Promise<Project | undefined>;
}
1 change: 1 addition & 0 deletions src/middleware/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export { withTuiOnEmptyFlagsAndArgs } from "./withTuiOnEmptyFlagsAndArgs";
export { withJsonRenderer } from "./withJsonRenderer";
export { withLogging } from "./withLogging";
export { withGlobalConfigAccessor } from "./withGlobalConfigAccessor";
export { withProject } from "./withProject";
13 changes: 12 additions & 1 deletion src/middleware/withProject.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ describe("withProject", () => {
}),
);

await expect(app.route(["node", "app", "check"])).rejects.toThrow(/no AgentCore project found/);
await expect(app.route(["node", "app", "check"])).rejects.toThrow(/No AgentCore project found/);
});

test("defaults to the cwd at invocation time when none is configured", async () => {
const projectManager = new FsProjectManager({ logger: createSilentLogger() });

const app = new Router("app", "test");
app.use(withProject({ projectManager }));
app.handler(createHandler({ name: "check", description: "noop", handle: async () => {} }));

// tmpdir encloses no project, so the message must name the cwd it searched.
await expect(app.route(["node", "app", "check"])).rejects.toThrow(process.cwd());
});
});
22 changes: 15 additions & 7 deletions src/middleware/withProject.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import type { Project, ProjectManager } from "../handlers/project/types";
import { InputValidationError } from "../errors/errors";
import { ProjectStateError } from "../errors/errors";
import { ProjectKey, type Middleware } from "../router";

interface WithProjectConfig {
projectManager: ProjectManager;
cwd: string;
/** Directory to search upwards from. Defaults to the cwd at invocation time. */
cwd?: string;
}

/**
* Middleware that locates the AgentCore project from the configured working
* directory and pins it on the context under {@link ProjectKey}.
* Middleware that locates the AgentCore project enclosing the working directory
* and pins it on the context under {@link ProjectKey}.
* Throws if no project can be found.
*
* @param config - Contains the {@link ProjectManager} and the `cwd` to search from.
* @param config - Contains the {@link ProjectManager} and an optional `cwd` to search from.
*/
export function withProject(config: WithProjectConfig): Middleware {
return (h) => ({
Expand All @@ -23,9 +24,16 @@ export function withProject(config: WithProjectConfig): Middleware {
doesSupportTui: () => h.doesSupportTui(),
children: () => h.children(),
handle: async (ctx, flags, args) => {
const project = await config.projectManager.resolve({ filePath: config.cwd });
// Resolved per invocation rather than at wiring time so the cwd the user
// actually ran in is the one searched.
const from = config.cwd ?? process.cwd();
const project = await config.projectManager.resolve({ filePath: from });
if (!project) {
throw new InputValidationError(`no AgentCore project found at ${config.cwd}`);
throw new ProjectStateError(
`No AgentCore project found at ${from} or any parent directory ` +
`(looked for agentcore/agentcore.json). ` +
`Run 'agentcore project create' to scaffold one.`,
);
}
await h.handle(ctx.withValue<Project>(ProjectKey, project), flags, args);
},
Expand Down
Loading