feat(wizard): interactive project add config-bundle - #2288
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Changes requested
Really nice refactor — factoring the wizard shell out of ProjectCreateScreen into <Wizard>/<Step> and shared fields, threading duplicate‑step‑name detection through render, and driving the shared driveProgress/TaskList path uniformly all read very cleanly. Tests use real dependencies (project harness, renderScreen, ttyTestIO) rather than mocks, which is exactly the right shape. One issue worth addressing before merging:
add config-bundle no longer reports missing --components clearly
src/handlers/project/add/config-bundle/index.ts used to have:
const components = parseJsonFlagWithSchema("components", componentsText, ComponentsSchema);
if (components === undefined) {
throw new InputValidationError("required option '--components <components>' not specified");
}That explicit branch is gone. The new flow is:
const components = parseJsonFlag<unknown>("components", await source.resolveText("components", flags.components));
const input = toAddConfigBundleInput({ ..., components, ... });When a user runs agentcore project add config-bundle --name foo (no --components), flags.components is undefined, source.resolveText will either surface an empty/undefined text, and the user ends up with either a parseJsonFlag JSON error or ComponentsSchema.safeParse(undefined) → Invalid value for option '--components': …expected record, received undefined. That's noticeably worse than the previous "required option '--components ' not specified" message, and there's no coverage for this case now that the check is removed (the headless dispatch test only covers missing --name).
Options:
- Restore the explicit
if (flags.components === undefined) throw new InputValidationError("required option '--components <components>' not specified")guard next to the existing!flags.namecheck. - Confirm
parseJsonFlag/toAddConfigBundleInputproduce a good message for the missing case and add a test asserting the exact user‑facing string.
Either way, please add a dispatch test for --name foo (no --components) so we don't regress this again.
Worth confirming (not blocking)
src/handlers/project/add/runtime/index.ts: the--nameflag schema tightens fromz.string().max(42)toRuntimeNameSchema(letters/digits/underscores, must start with a letter). IfRuntimeResourceConfigSchemawas already enforcing the pattern downstream this is just moving validation earlier, but if any existing users have hyphenated runtime names (my-agent), this becomes a breaking flag‑path change. Please double‑check thatAgentNameSchemais whatRuntimeResourceConfigSchemaalready validated names against.AddRuntimeScreen/AddConfigBundleScreendefaultonErrorto"exit", so anaddResourcefailure tears the TUI down rather than dropping back to the menu the user navigated from. That's inconsistent with the "one keystroke away from adding another resource" comment ononDone. Not a code correctness issue, but worth deciding intentionally — foradd, either"retry"(if partial state is recoverable) or a menu‑return error path may be a better UX thanexit.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor #2288 +/- ##
==========================================
Coverage 97.06% 97.07%
==========================================
Files 569 576 +7
Lines 39322 39744 +422
==========================================
+ Hits 38167 38580 +413
- Misses 1155 1164 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1981966 to
b0a19c5
Compare
|
Thanks — going through these. Missing Runtime name schema. Not a breaking change.
One unrelated change in the latest push: the components step is now a textarea (enter inserts a newline, ctrl+d continues), since a components map is usually pasted pretty-printed, and the description step is gone — |
|
Claude Security Review: no high-confidence findings. (run) |
b0a19c5 to
a3d5681
Compare
|
Claude Security Review: no high-confidence findings. (run) |
…time Adds src/components/wizard: a Wizard shell that owns the step list, position, key handling and the form → running → success | error phases, with Step and the TextField/ChoiceField/Summary fields. Fields publish their own key hints and own their step's input handling, so a screen only declares questions. Migrates project create onto it (−380 lines of its own controller) and adds the first add wizard, project add runtime: name → template → description → review. Both paths build their input through one builder — create through resolveScaffoldHarnessInput/resolveRuntimeTemplateShortcut as before, add runtime through the new toAddRuntimeInput — so a value the flag path accepts is not one a wizard rejects. The runtime wizard scaffolds from a template; the JSON infrastructure flags, the model provider/api-key and the Bedrock Agent import stay on the flag path. Also: KeyValueTable grows into the space it is given, so its percentage key cap resolves when it is laid out as a row item, and FormTextInput/ FormRadioGroup render bare when given no name or help text, since the step already asks the question.
…lidation - a bare `project add runtime` on a TTY now opens the wizard: the add router uses withTuiWhenInteractive, which is withTuiOnEmptyFlagsAndArgs behind a TTY gate. Flags, --json and non-TTY runs stay headless. The same middleware replaces the hand-rolled dispatch shims on create and status. - text fields validate the value as typed rather than a trimmed copy, so a step cannot accept " my_agent " and then submit it. - one shared RuntimeNameSchema (42 characters, the memory-name cap less "Memory") behind both --name and the wizard.
The wizard asks only what scaffolding a runtime needs — name and template. --description still sets one on the flag path.
Wizard takes a doneLabel so the success footer can name what enter does; add runtime navigates back instead of exiting, the way build and deploy do.
An add screen the user navigated to should not tear the TUI down over a rejected name: onError=retry reports the failure in the wizard and returns to the form with the answers intact. project create keeps exit.
A bare `agentcore project add config-bundle` in a TTY now opens a wizard: name → components → branch → commit message → review. The components map is collected in a textarea (enter inserts a newline, ctrl+d continues), because it is normally copied out of a file pretty-printed; that is the same editor and the same keys the harness wizard's prompt step uses. The flag path and the wizard now share one input builder, so both apply the same components schema and the same default branch, and both report a bad components map the same way. Nothing about the flag path's behaviour changes. The wizard does not ask for a description — --description still sets one. A failed add reports itself and hands the form back rather than exiting, so a rejected name does not cost the user the map they just pasted.
a3d5681 to
0eb21c0
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Stacked on #2284 — review the last commit only (
feat(wizard): interactive project add config-bundle). The earlier commits in this diff are #2284 and go away once it merges.Adds the wizard behind a bare
agentcore project add config-bundle: name → components → description → branch → commit → review. Enter on the success panel goes back to the add menu.Bundle assembly moves out of the flag handler into
toAddConfigBundleInput, which both paths call, so the wizard cannot accept a bundle the flags would reject.--componentsand the wizard's components step share oneComponentsSchema, andmainlineis now a named default rather than a literal in the flag declaration.No behaviour change on the flag path. Every other resource under
project addstays command line only.