feat(project): add TUI wizards for six more project add resources - #2167
Closed
notgitika wants to merge 4 commits into
Closed
feat(project): add TUI wizards for six more project add resources#2167notgitika wants to merge 4 commits into
project add resources#2167notgitika wants to merge 4 commits into
Conversation
Selecting `add` from the project menu reported "no interactive screen yet".
It now opens the resource menu, and four of the fifteen resources have a
wizard: memory, gateway, policy-engine and config-bundle. The other eleven
keep the not-implemented screen and are unchanged on the command line.
The three wizards that already existed (project create, HarnessWizard,
EndpointWizard) had each hand-rolled the same shell: a step list, a phase
machine, esc-goes-back, and a per-step useInput that spends 30 lines asking
for one string. That shell is now one component. A screen declares its
questions as <Step> children and branches with a plain conditional, so a
step that does not apply is absent from the flow and from the stepper:
{isCustomJwt && (
<Step name="authorizer-configuration" question="...">
<TextAreaField ... />
</Step>
)}
Position is keyed by step name rather than index, because branches have
different lengths and a step appearing must not move the user.
Screens submit through projectManager.addResource, as project create submits
through projectManager.create - not through the handler, whose result goes to
a stderr captured at wiring time that Ink's alternate screen would swallow.
To keep the two entry points from drifting, the screens reuse the handlers'
own helpers rather than copies of them: toDefaultStrategy,
EventExpiryDurationSchema, ComponentsSchema, gatewayResourceName and
policyEngineResourceName, three of which are newly exported for it.
Validation messages come from the flags' own schemas, so the wizard rejects
what the flag rejects and says the same thing about it.
Required-ness is a field prop, not a schema change: flag schemas stay
`.optional()` so Commander cannot reject a bare command before the TUI
middleware runs (69aa0c1), and the handler's own throw stays authoritative.
Screens resolve the project themselves via ProjectGate. withProject wraps
`handle` only, so middleware never runs for a screen the user navigated to
and ProjectKey is absent unless the launching command happened to set it.
The gate reports the same not-found guidance the CLI prints.
The Form* components now omit their label and help rows when passed empty
strings, so a field whose <Step> already asks the question renders just the
control instead of restating itself three times over. Existing callers pass
non-empty strings and are unaffected.
Two defects the shell's own tests found: the first frame rendered a footer
with no action key, because fields publish hints from an effect; and two
enter presses in one Ink drain submitted twice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… flags and wizard Each handler now exports `XxxInput` and `toAddXxxInput(...)`, the single place defaults, cross-field rules and resource assembly live. `handle` reduces to flags → input; the screen's `onSubmit` reduces to form values → input. Neither path states what a resource is, so they cannot drift. This fixes one drift already present: the flag path validated --authorizer-configuration with the strict schema, the wizard with the non-strict one, so the SDK's `customJWTAuthorizer` casing was rejected by flags but accepted by the wizard and then failed at submit. Both now share GatewayAuthorizerConfigurationInputSchema; a screen test pins it. Also in the shell: duplicate <Step name> throws at render, and the one-field-per-step invariant is stated on Step with the compound-field route for anything that needs more.
CI runs oxlint 1.80, whose react(refs) rule rejects `latest.current = hints` in useKeyHints. The effect now runs on every render and consults the ref only inside the effect, publishing when the hints' content differs from what the footer already shows — the same behaviour, with the ref where React wants it.
policy, gateway-connector, online-eval, online-insight, evaluator llm-as-a-judge and payment-manager get a wizard on the shared <Wizard> shell, following the recipe the first four established: each handler exports `XxxInput` + `toAddXxxInput(...)`, its `handle` reduces to flag conversion, and the screen's `onSubmit` reduces to form conversion. Every existing flag test now runs through the shared builder. Two small additions to the shell, both driven by these six: - `Prerequisite` stands in for a wizard whose first question has no possible answer — a Policy needs a Policy Engine, a connector needs a Gateway — naming what is missing and the command that adds it. - `values.ts` (blankToUndefined, splitList, numberSchema) turns text answers into the typed values the builders take, so an optional blank or a comma-separated list means the same thing on every wizard. online-eval and online-insight share their source and sampling steps through `monitoring-steps.tsx`, so the two say the same things and produce the same shapes. `evaluator` is a command group, so it gets a RouterScreen menu like `add` itself. Not included: harness, runtime, credentials, gateway-target and payment-connector, which still route to the not-implemented screen.
agentcore-devx-automation
Bot
added
agentcore-harness-reviewing
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
![agentcore-devx-automation[bot]](https://avatars.githubusercontent.com/u/2232217?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice work — the six new wizards follow the same recipe as #2166, and by pushing conversion into toAddXxxInput(...) builders both entry paths converge on a single assembler and single set of validation messages. A few things I looked at carefully:
- The
Prerequisitescreen andvalues.tshelpers are the right level of abstraction; extractingnumberSchemaand threading it throughmemory's retention step keeps the wizard's parsing rules identical to the flag path. monitoring-steps.tsxcorrectly drops answers from the branch the user didn't see (agent vs log‑groups), so it can't trip the schema's mutually-exclusive rule.React.Children.toArrayflattening is the standard pattern and works here.- The online-eval wizard's
required={values.projectEvaluators.length === 0}on the built‑in evaluators step is the right guard: it recomputes when the multi‑select changes, and when there are no project evaluators the field is always required, so the schema's "at least one" rule can't fail at submit. InsightIdSchemaandJudgeModelSchemabeing exported so the wizard step surfaces the same message the flag path surfaces is exactly the right move.- payment-manager:
PaymentSpendLimitSchemadoesn't accept empty, but withrequired=falsethe wizard lets an empty text pass validation andblankToUndefinedfunnels it into the builder'sDEFAULT_SPEND_LIMITfallback — this matches the flag path's default behavior. - Tests exercise both the flag path and the wizard through the shared builder, use real project scaffolding via
createGatewayProjectTestHarness, and assert onagentcore.jsonwithtoEqual/toMatchObject. No mocking concerns.
No changes requested — safe to merge once #2166 lands.
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
Contributor
Author
|
temporarily closing this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Six more
project addresources get a wizard, on the shell #2166 introduced: policy, gateway-connector, online-eval, online-insight, evaluator llm-as-a-judge, payment-manager. That makes ten of fifteen. The remaining five (harness,runtime,credentials,gateway-target,payment-connector) keep the not-implemented screen.policygateway-connectoronline-evalonline-insightevaluatorllm-as-a-judge: name → level → model → instructions → scale → custom scale → description → reviewpayment-managerWhere a project resource can answer a question, it is offered:
policypicks from the project's Policy Engines,gateway-connectorfrom its Gateways and Knowledge Bases,online-evalfrom its runtimes and evaluators. Where the project has none and the wizard cannot proceed without one, aPrerequisitescreen names what is missing and the command that adds it.Design
Each resource follows the recipe #2166 established, unchanged:
The handler exports
XxxInput+toAddXxxInput;handlereduces to flag conversion; the screen'sonSubmitreduces to form conversion. Every existing flag test now exercises the shared builder, and the screen tests exercise the same builder from the form side — so the two paths write byte-identicalagentcore.jsonentries, which the tests assert withtoEqualagainst the full object.Two things moved into builders that had been handler-body-only: online-insight's
Builtin.Insight.*/ARN check (nowInsightIdSchema, also used by the wizard step), and llm-as-a-judge's model-ID check (nowJudgeModelSchema, likewise). gateway-connector's builder takes a discriminatedtarget— shortcut or complete configuration — so the JSON path goes through it too rather than around it.Shell additions
Both small, both driven by these six:
Prerequisite— for a wizard whose first question has no possible answer. esc returns to the add menu.values.ts—blankToUndefined,splitList,numberSchema. Text answers → the typed values builders take. memory's retention step now usesnumberSchemainstead of its own copy.online-evalandonline-insightshare their source/sampling/enable steps throughmonitoring-steps.tsx— a hook returning arrays of<Step>s, whichReact.Children.toArrayflattens into the wizard's step list. The two wizards ask the same questions in the same words and produce the same shapes.evaluatoris a command group, soagentcore/project/add/evaluatoris aRouterScreenmenu likeadditself, reading its kinds off the Commander tree.Decisions worth a reviewer's attention
--tags,--kms-key-arn,--encryption-key-arn(all six); payment-manager's--tool-allowlistand--network-preferences; gateway-connector's--connector-configurationJSON path. All remain available on the command line and pass through the same builder.Builtin.*evaluators or insights in the codebase, so those steps are comma-separated text. Project evaluators are offered as a multi-select first, and the text step isrequiredonly when nothing was picked there — the schema's "at least one evaluator" rule is never reached at submit.LogGroupNamesSchemais extracted fromOnlineEvalConfigSchema(same definition, now named) so the log-groups step validates against the schema's own 1–5 bound.Testing
bun test: 2656 pass, 0 fail.tsc --noEmit,oxlint(1.74 and 1.80),prettier --checkclean.17 new screen tests. Each resource has a "same as the flag path" test asserting the written spec with
toEqual, plus its branch behaviour (prerequisite screens, conditional steps, picker vs. text) and one field-validation case using the schema's own message.