feat(wizard): shared wizard shell, used by project create and add runtime - #2284
feat(wizard): shared wizard shell, used by project create and add runtime#2284notgitika wants to merge 5 commits into
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice refactor. The shared shell is small and legible, the shape it enforces on callers (one field per step, fields own their useInput) is well-documented, and the extraction of toAddRuntimeInput keeps the flag path and wizard path validating through the same code — which is the interesting correctness win here. Position-by-key handling of conditional branches is a nice touch, and it's covered by wizard.test.tsx.
A few things I checked and am happy with:
- Tests use real dependencies.
runtime.screen.test.tsxwrites into a real project viacreateGatewayProjectTestHarnessand asserts the on-disk spec + scaffolded files;wizard.test.tsxdrives a syntheticTestWizardend-to-end rather than mocking the shell. No I/O mocks beyond what the harness already provides. - Double-submit guard is sound.
submittingref is reset infinally, so a retry after an error path still works. ExitOnErrorroutes failures throughexit(error)so they surface via the normal CLI error path instead of a React stack trace — matches how the existing wizards behaved.project createbehaviour is preserved. The old screen test still passes as-is per the PR description; theModelFieldcompound step is kept intentionally, which is the right call given the shell has no focus model.- Telemetry. No new instrumentation, but the existing project handlers aren't instrumented either, so this isn't a regression.
Non-blocking observations (won't ask for changes):
useMemoaroundstepElements/stepswon't actually memoize becausechildrenis a fresh array literal each render. Harmless; the work is trivial.- Success footer label went from
[enter] exit(old create) to[enter] continue(new shared default). Minor copy change; fine either way.
Nothing that needs to change before merging.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor #2284 +/- ##
============================================
- Coverage 97.06% 97.05% -0.02%
============================================
Files 569 575 +6
Lines 39322 39561 +239
============================================
+ Hits 38167 38394 +227
- Misses 1155 1167 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
| // rather than inside renderTui so that a piped or CI run stays headless and | ||
| // reports a missing required flag as the usage error it is, instead of | ||
| // renderTui's "interactive mode requires a TTY". | ||
| export function withTuiWhenInteractive(core: Core, io: AppIO): Middleware { |
There was a problem hiding this comment.
withTuiOnEmptyFlagsAndArgs above opens the TUI even without a TTY, so a piped/CI add runtime would fail with "requires a TTY" instead of the missing --name error. This is that middleware behind a TTY check. create and status each had it hand-rolled; now they share it.
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
| setHints: (hints: KeyHint[]) => void; | ||
| } | ||
|
|
||
| const WizardContext = React.createContext<WizardControls | null>(null); |
There was a problem hiding this comment.
Please import just createContext, not the whole React library Object.
…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.
0061747 to
ea56301
Compare
|
Claude Security Review: no high-confidence findings. (run) |
| breadcrumb={BREADCRUMB} | ||
| description={DESCRIPTION} | ||
| onCancel={() => navigate(ADD_MENU)} | ||
| onSubmit={() => |
There was a problem hiding this comment.
After the add succeeds the react-query project cache still holds the old spec, so status, invoke, deploy and build in the same TUI session don't see the new runtime. Call setQueryData with the returned project, or drop staleTime Infinity on the seed.
| // The resources with a wizard of their own. Every other resource is listed in | ||
| // the add menu as command line only and opens its help instead (see | ||
| // CliOnlyScreen). | ||
| const projectAdd = new Router("add", "add project resources").supportedTuiCommands("runtime"); |
There was a problem hiding this comment.
Bare agentcore project add prints Commander help and exits 1 because this router has no .default(renderTui(core, io)). Navigating here from agentcore project works, so the two entry points disagree.
| // under itself, not under the key. | ||
| return ( | ||
| <Box flexDirection="column"> | ||
| <Box flexDirection="column" flexGrow={1}> |
There was a problem hiding this comment.
flexGrow here makes the table take all free height in any growing column parent. The gateway policy generate screen now has 14 blank lines between the table and the input. Put flexDirection="column" on the two bordered wrapper Boxes instead and leave the shared table alone.
| <Box flexDirection="column"> | ||
| {phase.kind === "form" && ( | ||
| <> | ||
| <Box paddingX={1}> |
There was a problem hiding this comment.
The old create screen had flexShrink={0} on this Box. Without it the stepper paints over the header Divider on short terminals (80x14 and 80x18).
| // RuntimeNameSchema is the one runtime-name rule: the `--name` flag and the | ||
| // interactive wizard both validate against it, so neither accepts a name the | ||
| // other rejects. | ||
| export const RuntimeNameSchema = AgentNameSchema.max( |
There was a problem hiding this comment.
AgentNameSchema already says max 48 in its message, so the first error the user sees contradicts the max 42 help line. The 42 only matters for the memory suffix, which most templates never create, and RuntimeResourceConfigSchema doesn't enforce it. Truncate in getDefaultMemorySpec and validate with AgentNameSchema instead.
| const { advance, back, isLast } = useWizard(); | ||
|
|
||
| useKeyHints([ | ||
| { key: "↑↓", label: "choose" }, |
There was a problem hiding this comment.
Every other arrow hint in src, including the model step in this same wizard, says "navigate".
| return [...fieldHints, { key: "esc", label: "back" }, { key: "ctrl+c", label: "quit" }]; | ||
| } | ||
|
|
||
| function SuccessPanel({ |
There was a problem hiding this comment.
This is the same block as ConfirmAction's SuccessBody, and isProgressStream is the same check as isProgressGenerator can we export and re-use from one place
| // than in TextAreaField unless the value is genuinely prose: JSON is valid on | ||
| // one line, and this field can be edited in place. | ||
| json?: boolean; | ||
| // example is a dimmed line under `help`, kept on screen while the user types. |
There was a problem hiding this comment.
I don't see any other caller setting json or example on TextField, and both screens pass onError="retry", so the "exit" mode and ExitOnError never run. Remove json, example, onError and ExitOnError. Add them back when a screen needs them.
| const parsed = schema.safeParse(value); | ||
| if (parsed.success) return undefined; | ||
| const issue = parsed.error.issues[0]; | ||
| if (!issue) return "invalid value"; |
There was a problem hiding this comment.
Zod always returns at least one issue on failure. Line 200 in this file already uses the non-null assertion, lets do the same here.
| // reason rather than growing its own rules. | ||
| function validateEntry( | ||
| value: string, | ||
| { label, required, schema, json = false }: ValidateOptions, |
There was a problem hiding this comment.
The new files carry multi-line comment - can we audit and keep the comment only when the code is genuinely hard to follow?
First of a few PRs. This one lands the shared wizard shell and puts two commands (agentcore project create and agetncore project add runtime) on it; the remaining
project addwizards follow separately.What
src/components/wizard/:Wizard: derives the step list from its<Step>children, owns position, key handling and the form → running → success | error phases, and renders the standard Layout + Stepper frame. Position is keyed by step name, so a{condition && <Step/>}branch appearing or disappearing does not move the user. A submit that streamsProgressEvents is rendered with the samedriveProgress+TaskListas create, build and deploy.Step: one question, one field.fields:TextField,ChoiceField,Summary. Each field owns its step'suseInputand publishes its own key hints, so a screen writes questions and nothing else.Callers
project createmoved onto it, unchanged in behavior. Its own controller (phases, step components, success panel, hint plumbing) is deleted; the two-level model step stays as a compound field, since the shell has no notion of focus within a step.project add runtimeis new: name → template → description → review.toAddRuntimeInputis extracted from the handler so the flag path and the wizard validate the same way.The runtime wizard scaffolds from a template. The JSON infrastructure flags,
--model-provider/--api-key(a secret read from stdin orfile://) and the Bedrock Agent import stay on the flag path.Shared components
The fields render existing components rather than new ones:
FormTextInput,FormRadioGroup,KeyValueTable,ErrorPanel,Layout,ui/{stepper,divider,spinner,task-list}. Two fixes at the source:KeyValueTablegrows into the space it is given, so its percentage key cap has a parent width to resolve against when the table is laid out as a row item.FormTextInput/FormRadioGrouprender bare when given no name or help text — the step already asks the question.Testing
bun test3218 pass / 0 fail;tsc --noEmit, oxlint and prettier clean. New:wizard.test.tsxfor the shell,runtime.screen.test.tsxfor the wizard (writes a real project and asserts the spec and scaffolded files),add.screen.test.tsxfor the add menu. Three help-viewport tests inproject.screen.test.tsxmoved toadd payment-manager, sinceadd runtimenow opens a wizard instead of its help.Next
HarnessWizard/EndpointWizardonto the shell (servesharness create/update,endpoint create/update, and givesproject add harnessa screen).