refactor(templates): simplify create flow, create flags, and runtime templates. - #2214
Conversation
…ner only for strands, trim agui
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2214 +/- ##
============================================
- Coverage 97.07% 97.04% -0.04%
============================================
Files 544 544
Lines 37865 37535 -330
============================================
- Hits 36759 36427 -332
- Misses 1106 1108 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice cleanup — the surface reduction reads clearly and the tests mostly stayed off mocks (real temp dirs + spy on create rather than a mocked manager, which is the right shape).
Only minor things I noticed; none block merge:
src/handlers/project/add/runtime/index.ts(~L140-L169):source.resolveSecret("api-key", flags["api-key"])runs before the!isImport && !isTemplate && modelFlagsPresentvalidation. If a user invokesproject add runtime --name foo --api-key -(orfile://…) without a template, stdin/file is consumed before the error surfaces. Cheap fix is to move that validation block above theresolveSecretcall so we reject before touching secret input. Same nit applies to--model-provideron the create handler in that no-op sense, though there the side effect is nil.- Purely FYI (not a bug):
agent-python-strands-containerreusesruntimeName: "agent_python_strands". If someone eventually callsadd runtime --template agent-python-strands-containerinside a project that already hasagent-python-strands, they'll collide on the codeLocation. Pre-existing shape, worth keeping in mind if you get user reports. - PR title is still
wip: …; presumably drop before merge.
Everything else — the template asset trims, the shortcut table, the wizard flow, the README, and the schema-driven validation — hangs together well.
|
Claude Security Review: no high-confidence findings. (run) |
| createInput = { ...base }; | ||
| } else { | ||
| const source = new SourceResolver({ stdin: config.io.stdin }); | ||
| const apiKey = await source.resolveSecret("api-key", apiKeyFlag); |
There was a problem hiding this comment.
resolveSecret runs before resolveRuntimeTemplateShortcut checks supportsModelProviderOverride, so --api-key - on an unsupported template blocks on stdin and consumes the secret before failing with the template error.
There was a problem hiding this comment.
Yeah I was trying to centralize this in the resolution, but I see how it creates a worse UX, lemme pop it out into the handlers.
| } | ||
|
|
||
| const source = new SourceResolver({ stdin: config.io.stdin }); | ||
| const apiKey = await source.resolveSecret("api-key", flags["api-key"]); |
There was a problem hiding this comment.
Same ordering as create: the secret is read before the template's supportsModelProviderOverride check, so --api-key -/file:// errors surface before the "not valid with this template" error.
| | --- | --- | --- | | ||
| {{#if identityProviders.[0]}}| `{{identityProviders.[0].envVarName}}` | Yes | {{modelProvider}} API key (local) or Identity provider name (deployed) | | ||
| {{/if}}| `LOCAL_DEV` | No | Set to `1` to use `.env.local` instead of AgentCore Identity | | ||
| | `LOCAL_DEV` | No | Set to `1` to use `.env.local` instead of AgentCore Identity | |
There was a problem hiding this comment.
Nit: LOCAL_DEV is no longer read anywhere in this template (its only consumer was the removed Anthropic/OpenAI/Gemini branch in model/load.ts), so this row can go.
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Added back memory to AGUI and verified e2e by deploying, invoking, and verifying a memory was in the stack. |
| if not api_key: | ||
| raise RuntimeError( | ||
| f"{IDENTITY_ENV_VAR} not found. Add {IDENTITY_ENV_VAR}=your-key to .env.local" | ||
| ) |
There was a problem hiding this comment.
|
Merging since earlier comments were addressed and we have a single approval. Will address remaining comments as a follow-up. |
Problem
We want to simplify the templates!
Solution
longAndShortTermmemory pre-configured.Verification
Help text:
TUI Create Flow
create-flow.mov
Templates E2E verification
Simple happy case
Default to harness on create:
had agents test the following:
Notes