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
2 changes: 1 addition & 1 deletion src/core/project/manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { DeserializationError, ProjectStateError } from "../../errors/errors";
import type { AwsDeploymentTarget } from "../../projectSchemas/aws-targets";
import { ProjectSpecSchema } from "../../projectSchemas/project";
import { FsProjectManager } from "./manager";
import { RUNTIME_TEMPLATE_SHORTCUTS } from "../../handlers/project/shortcuts";
import {
RUNTIME_TEMPLATE_SHORTCUTS,
type CreateProjectInput,
type DeployResult,
type Project,
Expand Down
3 changes: 3 additions & 0 deletions src/core/project/templates/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ const getTemplateResolvers = (assetSource: AssetSource, templateRenderer: Templa
if (input.protocol !== undefined && input.protocol !== "HTTP")
throw new InputValidationError("the strands-python template only supports HTTP");

if (input.scaffoldRuntimeInput.build !== "CodeZip")
throw new InputValidationError("the strands template only supports CodeZip builds");

const filesystemConfigurations = input.filesystemConfigurations ?? [];
const sessionStorageMountPath = filesystemConfigurations.flatMap((configuration) =>
"sessionStorage" in configuration ? [configuration.sessionStorage.mountPath] : [],
Expand Down
97 changes: 74 additions & 23 deletions src/handlers/project/add/runtime/index.test.ts

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.

wait why are adding the build override? shouldn't that also belong to the "locked" param?

for eg: strands-python template can be override to container but the generated files won't contain Dockerfile, while the agentcore.json config would reference one.

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.

The build override should work since its passed as a parameter to the template. I haven't wired up container support for the strands-python one yet, so if its not rejecting that's a bug.

Update: it is a bug, let me just fix that here.

Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ describe("project add runtime", () => {
];

const expectedSpecByLabel: Record<string, Record<string, unknown>> = {
"template overrides to Container": {
build: "Container",
dockerfile: "Dockerfile",
},
"container template build override to CodeZip": {
build: "CodeZip",
},
"all infrastructure flags": {
description: "Configured runtime",
executionRoleArn: "arn:aws:iam::123456789012:role/MyRole",
Expand Down Expand Up @@ -129,6 +136,25 @@ describe("project add runtime", () => {
["--name", "my_agent", "--template", "hello-world-python-container"],
],
["strands-python template preset", ["--name", "my_agent", "--template", "strands-python"]],
[
"template overrides to Container",
[
"--name",
"my_agent",
"--template",
"hello-world-python",
"--build",
"Container",
"--model-provider",
"Bedrock",
"--memory",
"none",
],
],
[
"container template build override to CodeZip",
["--name", "my_agent", "--template", "hello-world-python-container", "--build", "CodeZip"],
],
["custom — all scaffolding flags", ["--name", "my_agent", ...allScaffoldingFlags]],
[
"custom — framework strands",
Expand Down Expand Up @@ -286,9 +312,12 @@ describe("project add runtime", () => {
const spec = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json();
const runtime = spec.runtimes.find((candidate: { name: string }) => candidate.name === name);
expect(runtime).toMatchObject({ entrypoint: "main.py", ...expectedSpecByLabel[label] });
const isContainer = flags.some(
(value) => value === "Container" || value === "hello-world-python-container",
);
expect(await Bun.file(join(projectRoot, "app", name, "main.py")).exists()).toBe(true);
const buildFlagIndex = flags.indexOf("--build");
const isContainer =
buildFlagIndex >= 0
? flags[buildFlagIndex + 1] === "Container"
: flags.includes("hello-world-python-container");
expect(runtime.runtimeVersion).toBe(isContainer ? undefined : "PYTHON_3_14");
});

Expand All @@ -309,30 +338,14 @@ describe("project add runtime", () => {
"none",
],
],
[
"--template and --build are mutually exclusive",
["--name", "my_agent", "--template", "hello-world-python", "--build", "Container"],
],
[
"--template and --language are mutually exclusive",
["--name", "my_agent", "--template", "hello-world-python", "--language", "Python"],
],
[
"--template and --framework are mutually exclusive",
["--name", "my_agent", "--template", "hello-world-python", "--framework", "none"],
],
[
"--template and --model-provider are mutually exclusive",
["--name", "my_agent", "--template", "hello-world-python", "--model-provider", "Bedrock"],
],
[
"--template and --memory are mutually exclusive",
["--name", "my_agent", "--template", "hello-world-python", "--memory", "none"],
],
[
"strands-python only supports HTTP",
["--name", "my_agent", "--template", "strands-python", "--protocol", "MCP"],
],
[
"strands-python only supports CodeZip builds",
["--name", "my_agent", "--template", "strands-python", "--build", "Container"],
],
[
"invalid JSON in --network-config",
["--name", "my_agent", ...template, "--network-config", "{bad}"],
Expand All @@ -345,4 +358,42 @@ describe("project add runtime", () => {
await inProject();
await expect(run(["add", "runtime", ...flags])).rejects.toBeInstanceOf(InputValidationError);
});

test.each([
["language", "Python"],
["framework", "none"],
])("rejects --%s as a template override", async (flagName, value) => {
await inProject();
await expect(
run([
"add",
"runtime",
"--name",
"my_agent",
"--template",
"hello-world-python",
`--${flagName}`,
value,
]),
).rejects.toThrow(`--${flagName} cannot override a template`);
});

test("rejects an incompatible API-key template override", async () => {
const projectRoot = await inProject();
const apiKeyPath = join(projectRoot, "api-key.txt");
await Bun.write(apiKeyPath, "secret-key");

await expect(
run([
"add",
"runtime",
"--name",
"my_agent",
"--template",
"hello-world-python",
"--api-key",
`file://${apiKeyPath}`,
]),
).rejects.toThrow(/API keys are not compatible with Bedrock model providers/);
});
});
31 changes: 22 additions & 9 deletions src/handlers/project/add/runtime/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import { SourceResolver } from "../../../../io";
import {
RUNTIME_TEMPLATE_SHORTCUT_NAMES,
RUNTIME_TEMPLATE_SHORTCUTS,
ScaffoldRuntimeInputSchema,
} from "../../types";
resolveRuntimeTemplateShortcut,
} from "../../shortcuts";
import { ScaffoldRuntimeInputSchema } from "../../types";
import { RuntimeResourceConfigSchema } from "./types";

export const createAddRuntimeHandler = (config: AddProjectResourceConfig) =>
Expand All @@ -23,7 +24,7 @@ export const createAddRuntimeHandler = (config: AddProjectResourceConfig) =>
flag("description", "an optional description of the runtime", z.string().optional()),
flag(
"template",
"a preset of flags to be leveraged in scaffolding the runtime. mutually exclusive with all runtime scaffolding flags",
"a preset of flags for scaffolding the runtime; compatible flags override preset values",
z.enum(RUNTIME_TEMPLATE_SHORTCUT_NAMES).optional(),
),
flag("build", "build type: CodeZip or Container", BuildTypeSchema.optional()),
Expand Down Expand Up @@ -108,19 +109,31 @@ export const createAddRuntimeHandler = (config: AddProjectResourceConfig) =>
] as const;
const presentScaffoldingFlags = scaffoldingFlags.filter((f) => flags[f] !== undefined);
const isTemplate = flags["template"] !== undefined;

if (isTemplate && presentScaffoldingFlags.length > 0)
throw new InputValidationError(
`--template and --${presentScaffoldingFlags[0]} are mutually exclusive`,
);
const lockedFlag = (["language", "framework"] as const).find(
(flagName) => flags[flagName] !== undefined,
);
if (isTemplate && lockedFlag) {
throw new InputValidationError(`--${lockedFlag} cannot override a template`);
}

const isCustom = presentScaffoldingFlags.length > 0;

const source = new SourceResolver({ stdin: config.io.stdin });
const apiKey = await source.resolveSecret("api-key", flags["api-key"]);

const scaffoldRuntimeInput = isTemplate
? RUNTIME_TEMPLATE_SHORTCUTS[flags.template!]
? resolveRuntimeTemplateShortcut(flags.template!, {
runtimeName: flags.name,
...(flags.build !== undefined && {
build: flags.build,
runtimeVersion: flags.build === "CodeZip" ? "PYTHON_3_14" : undefined,
}),
...(flags["model-provider"] !== undefined && {
modelProvider: flags["model-provider"],
}),
...(apiKey !== undefined && { apiKey }),
...(flags.memory !== undefined && { memory: flags.memory }),
})
: isCustom
Comment on lines +112 to 137

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.

OOS for your PR but would we see any value in making a shared component for the shared functionality b/w create and runtime?

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.

YES! I'm hoping to come back to this.

? parseScaffoldRuntimeInput({
runtimeName: flags.name,
Expand Down
34 changes: 24 additions & 10 deletions src/handlers/project/create/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import { SourceResolver, type AppIO } from "../../../io";
import {
RUNTIME_TEMPLATE_SHORTCUT_NAMES,
RUNTIME_TEMPLATE_SHORTCUTS,
ScaffoldRuntimeInputSchema,
type CreateProjectInput,
type ProjectManager,
} from "../types";
resolveRuntimeTemplateShortcut,
} from "../shortcuts";
import { ScaffoldRuntimeInputSchema, type CreateProjectInput, type ProjectManager } from "../types";
import { ProjectNameSchema } from "../../../projectSchemas/project";
import { InputValidationError } from "../../../errors";

Expand All @@ -24,7 +23,7 @@ export const createCreateProjectHandler = (config: CreateProjectHandlerConfig) =
flag("name", "name of the project to create", ProjectNameSchema),
flag(
"template",
"a preset of flags to be leveraged in scaffolding the runtime. mutually exclusive with all runtime scaffolding flags",
"a preset of flags for scaffolding the runtime; compatible flags override preset values",
z.enum(RUNTIME_TEMPLATE_SHORTCUT_NAMES).optional(),
),
flag(
Expand Down Expand Up @@ -75,18 +74,33 @@ export const createCreateProjectHandler = (config: CreateProjectHandlerConfig) =

const presentScaffoldingFlags = scaffoldingFlags.filter((f) => flags[f] !== undefined);
const isTemplate = flags["template"] !== undefined;
if (presentScaffoldingFlags.length > 0 && isTemplate)
throw new InputValidationError(
`--template and --${presentScaffoldingFlags[0]} are mutually exclusive`,
);
const lockedFlag = (["language", "framework"] as const).find(
(flagName) => flags[flagName] !== undefined,
);
if (isTemplate && lockedFlag) {
throw new InputValidationError(`--${lockedFlag} cannot override a template`);
}

const isCustom = presentScaffoldingFlags.length > 0;

const source = new SourceResolver({ stdin: config.io.stdin });
const apiKey = await source.resolveSecret("api-key", flags["api-key"]);

const scaffoldRuntimeInput = isTemplate
? RUNTIME_TEMPLATE_SHORTCUTS[flags["template"]!]
? resolveRuntimeTemplateShortcut(flags["template"]!, {
...(flags["runtime-name"] !== undefined && {
runtimeName: flags["runtime-name"],
}),
...(flags["build"] !== undefined && {
build: flags["build"],
runtimeVersion: flags["build"] === "CodeZip" ? "PYTHON_3_14" : undefined,
}),
...(flags["model-provider"] !== undefined && {
modelProvider: flags["model-provider"],
}),
...(apiKey !== undefined && { apiKey }),
...(flags["memory"] !== undefined && { memory: flags["memory"] }),
})
Comment on lines +90 to +103

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.

Not saying necessarily to change this because it is correct, but this is kind of hard to read at first. To make this more readable we could move the override building logic into resolveRuntimeTemplateShortcut? Passing the optional flag values directly would make this easier to scan and keep the build/runtimeVersion relationship in one place

const scaffoldRuntimeInput = resolveRuntimeTemplateShortcut(flags["template"]!, {
  runtimeName: flags["runtime-name"],
  build: flags["build"],
  modelProvider: flags["model-provider"],
  apiKey,
  memory: flags["memory"],
});

The resolver could ignore undefined values and own the build/runtimeVersion relationship, which would make both callers easier to scan.

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.

I like that! I'll have to rebase #2116 on top of this anyway, so I'm going to merge and address this there.

: isCustom
? parseScaffoldRuntimeInput({
runtimeName: flags["runtime-name"] ?? flags["name"],
Expand Down
56 changes: 41 additions & 15 deletions src/handlers/project/project.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ describe("project create", () => {
expect(core.projectCommands).toEqual([]);
});

test("rejects --template combined with scaffolding flags", async () => {
test.each([
["language", "Python"],
["framework", "none"],
])("rejects --%s as a template override", async (flagName, value) => {
await inTempDirectory();
await expect(
run([
Expand All @@ -123,10 +126,41 @@ describe("project create", () => {
"MyAgent",
"--template",
"hello-world-python",
"--build",
"Container",
`--${flagName}`,
value,
]),
).rejects.toThrow(/--template and --build are mutually exclusive/);
).rejects.toThrow(`--${flagName} cannot override a template`);
});

test("applies compatible overrides to a template", async () => {
const directory = await inTempDirectory();
await run([
"create",
"--name",
"MyProject",
"--template",
"strands-python",
"--runtime-name",
"custom_agent",
"--build",
"CodeZip",
"--model-provider",
"Bedrock",
"--memory",
"none",
"--skip-install",
"--skip-git",
]);

const projectRoot = join(directory, "MyProject");
const spec = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json();
expect(spec.runtimes[0]).toMatchObject({
name: "custom_agent",
build: "CodeZip",
codeLocation: "app/custom_agent",
runtimeVersion: "PYTHON_3_14",
});
expect(await Bun.file(join(projectRoot, "app", "custom_agent", "main.py")).exists()).toBe(true);
});

test("scaffolds from explicit custom flags", async () => {
Expand Down Expand Up @@ -189,26 +223,18 @@ describe("project create", () => {
expect(existsSync(join(directory, "MyProject"))).toBe(false);
});

test("rejects an API key with the Bedrock model provider before scaffolding", async () => {
test("rejects an incompatible API-key template override before scaffolding", async () => {
const directory = await inTempDirectory();
await expect(
run(
[
"create",
"--name",
"MyProject",
"--build",
"CodeZip",
"--language",
"Python",
"--framework",
"none",
"--model-provider",
"Bedrock",
"--template",
"hello-world-python",
"--api-key",
"-",
"--memory",
"none",
"--skip-install",
"--skip-git",
],
Expand Down
Loading
Loading