feat(project): build and deploy TUI screens render the CLI's own progress steps - #2172
Conversation
agentcore-devx-automation
Bot
added
agentcore-harness-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2172 +/- ##
============================================
+ Coverage 97.07% 97.11% +0.03%
============================================
Files 535 538 +3
Lines 36872 37167 +295
============================================
+ Hits 35794 36094 +300
+ Misses 1078 1073 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![agentcore-devx-automation[bot]](https://avatars.githubusercontent.com/u/2232217?s=60&v=4){kind=small}
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice refactor. Extracting applyProgressEvent/settleProgress as pure functions and driving both the CLI's inline TaskList and the new TUI screens off the same reducer is exactly the right shape — the TUI genuinely becomes a frame around the CLI's own progress rather than a parallel implementation. The ConfirmAction extension via a discriminated Promise | AsyncGenerator union is backward-compatible and the Symbol.asyncIterator check is a safe discriminator.
A few observations, none blocking:
src/handlers/project/deploy/screen.tsx: the target is hard-coded toDEFAULT_TARGET_NAMEwith no way to choose another from the TUI. Likely intentional for a first pass, but worth a follow-up once the picker exists.src/components/ConfirmAction.tsx:116-119: the running phase advertisesesc backandctl+c quitin the key hints, but nouseInputhandler is installed while running, soescis a no-op there. That's arguably the correct behavior (you don't want to abort a deploy mid-flight), but the hint is misleading — consider droppingescfrom the hints duringrunning.- Tests use real temp directories, scaffold via the real
project createhandler, and stub only at theProjectBackendboundary. Good example of the pattern to prefer. - No new telemetry, but the sibling
build/deploycommand handlers don't emit any either, so this doesn't introduce a gap that wasn't already there.
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
github-actions
Bot
added
size/xl
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
…gress steps `project build` and `project deploy` open from the TUI menu instead of reporting "not implemented". Each screen runs the very generator the command runs — projectManager.build / projectManager.deploy — and renders its step events through the same TaskList runWithProgress renders inline on the command line, so the two paths show identical steps, glyphs and output tails. Nothing about progress is re-implemented: the event→task fold is extracted from runWithProgress into applyProgressEvent / settleProgress and both renderers call it. The screens are ConfirmAction instances — the existing confirm→run→ success|error body behind `harness delete` — which learns to accept a progress generator as its action and shows the TaskList while it streams. Its summary/success rows now align on the longest label rather than a fixed 8 columns. The command's other behaviours carry over from the same source: builtMessage / deployedMessage / teardownQuestion are exported from the handlers and shown on the screens; declaresNothingDeployable decides whether the confirmation is the teardown question, and confirming it is the pre-answered decision the backend consults, as with --yes. useProject / ProjectGate resolve the enclosing project for a screen the user navigated to, using withProject's now-exported not-found message; ProjectInvokePickerScreen drops its inline copy of both.
…rogress driving
Review follow-ups on the build/deploy screens:
- The deploy success title came from the preflight declaresNothingDeployable
heuristic, which the backend's post-synth count can disagree with, so the
screen could say "Project removed" over "Deployed project…". The action
now returns its own title from result.tornDown via deployedMessage, the
same line the command prints; ConfirmAction's action may return
{ title, rows } for outcomes only known after running.
- ProjectGate's resolution error offered no way off but ctl+c. It takes an
onBack and handles esc, advertised in the footer.
- ConfirmAction's rows are rendered by KeyValueTable instead of a second
longest-label renderer.
- The generator drain lives once, in driveProgress; runWithProgress and
ConfirmAction each pass only how to draw the tasks.
- The running phase no longer advertises esc, since nothing listens for it
mid-action.
…tion unless tearing down Shaped by trying the screens on a real project: - build starts as soon as the screen opens — it changes nothing outside the project — and shows no header. Done, it suggests `agentcore project deploy` and enter returns to the project menu instead of exiting. - deploy confirms only when the spec declares nothing deployable, the one case the command asks its readline question; otherwise it deploys at once. With several targets in aws-targets.json it first asks which (a DataTable of target/account/region, the TUI's stand-in for --target); with one or none it uses that or `default`. ProjectManager gains listTargets for this; resolveTarget delegates to it. - deploy no longer errors on a fresh project: resolveTarget returning undefined (no aws-targets.json yet) was fed straight to useQuery, which treats undefined data as an error. The manager's "Created default deployment target" step now streams through as it does on the CLI. - the header is just project/target; stack outputs are not listed (the command prints them only with --json); the success hint reads "go back". ConfirmAction: message, title and rows are optional (no question → run when ready; no header → no box), doneLabel names where enter leads, nextSteps lists follow-up commands, and an error without a confirmation to return to leaves instead of re-running.
… code doesn't say
…eProject, recoverable target loading Review follow-ups on aws#2172: - ConfirmAction takes `trigger: {kind:"confirm"; message} | {kind:"immediate"}` instead of inferring immediacy from a missing message. The initial phase follows the trigger, so an immediate action never paints a y/n footer. - useProject is a useQuery (seed as initialData, never refetched); ProjectGate and the invoke screen keep their shape. LoadingFrame is the shared spinner-or-error with esc back and r retry, used by ProjectGate and by the deploy screen's target loading, which previously offered only ctl+c. - The create wizard drives its progress through driveProgress and renders TaskList, dropping its own event list.
4ccf0a4 to
8dd11f0
Compare
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
|
Demo of the new
Demo: |
tejaskash
left a comment
tejaskash
left a comment
There was a problem hiding this comment.
Verified locally: tsc --noEmit clean, touched tests pass (202/202). No blockers, a few low-severity notes inline.
| const targets = useQuery({ | ||
| queryKey: ["project-targets", project.rootPath], | ||
| queryFn: () => core.projectManager.listTargets(project), | ||
| }); |
There was a problem hiding this comment.
With the default gcTime this list is served from cache on a second visit, so DeployConfirm mounts and immediate fires run() before the refetch lands. If aws-targets.json gained a second target in between, targetName flips to undefined, the picker renders, and DeployConfirm unmounts while the deploy keeps running detached.
Since this is a local file the CLI reads fresh every time, gcTime: 0 here (or gating on !targets.isFetching) would avoid it.
There was a problem hiding this comment.
Agreed. I fixed it and added a regression test
|
|
||
| const declared = targets.data; | ||
| const targetName = | ||
| chosen ?? (declared.length <= 1 ? (declared[0]?.name ?? DEFAULT_TARGET_NAME) : undefined); |
There was a problem hiding this comment.
One declared target that is not named default gets deployed to here, whereas the CLI with no --target would provision and deploy default. The PR description says this is intended, flagging only so it stays a conscious choice.
There was a problem hiding this comment.
yes, this was intended
| }, | ||
| // A seeded project is authoritative — it is what the launching command ran | ||
| // against — so it is never refetched from the cwd. | ||
| ...(seed && { initialData: seed, staleTime: Infinity }), |
There was a problem hiding this comment.
Same cache shape as the targets query in the deploy screen: without a seed, a revisit reads project.spec from cache and declaresNothingDeployable and the action closure act on it before the refetch. Milder here since the backend re-reads from disk, but gcTime: 0 would make both behave the same way.
| const [phase, setPhase] = useState<Phase>({ kind: "confirm" }); | ||
| const cancel = onCancel ?? (() => navigate(-1)); | ||
| const [phase, setPhase] = useState<Phase>({ | ||
| kind: trigger.kind === "confirm" ? "confirm" : "idle", |
There was a problem hiding this comment.
Phase is fixed from trigger.kind once at mount. A caller that mounts with isPending: true and later switches the trigger from immediate to confirm would stay idle and run the action with no question asked. No current caller does this, but it is easy to trip on in a component meant to guard destructive actions. Either document that trigger.kind must be stable or resolve the phase when isPending flips false.
There was a problem hiding this comment.
Fixed by keeping the action in a waiting phase until loading completes, then snapshotting the latest trigger and confirmation message.
| } | ||
|
|
||
| function toItems(rows: SummaryRow[]): Record<string, string> { | ||
| return Object.fromEntries(rows.map((row) => [row.label, row.value])); |
There was a problem hiding this comment.
Converting to a Record drops what SummaryRow[] promises: duplicate labels collapse to the last value and integer-like labels get reordered by object key ordering. Nothing hits this today. If KeyValueTable is the renderer, taking Record<string, string> at the prop boundary would make the type match what is shown.
| // A read-only lookup, so callers (e.g. the deploy handler's up-front teardown | ||
| // confirmation) can name the target's account and region without triggering | ||
| // the default-target provisioning deploy performs. | ||
| public async listTargets(project: Project): Promise<AwsDeploymentTarget[]> { |
There was a problem hiding this comment.
deploy further up still reads aws-targets.json inline. It also needs the exists check for its error branches, so not a one-line swap, but the read itself could go through listTargets now that it owns that path.
| @@ -116,7 +118,7 @@ import type { Context } from "../router"; | |||
| // PROJECT_COMMANDS are the `agentcore project` subcommands that are listed in | |||
| // the menu but have no screen of their own yet (`create` has the wizard). Each | |||
There was a problem hiding this comment.
Stale now: invoke, build, and deploy have screens too.
There was a problem hiding this comment.
this one is gone in the next PR
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
4 participants
What
project buildandproject deployopen from the TUI menu instead of reporting "not implemented", and show exactly the progress the command shows — same steps, same ✓/✕/spinner glyphs, same output tail under the running step.build starts as soon as the screen opens (nothing to confirm — it only writes inside the project). Done, it suggests
agentcore project deploy; enter returns to the project menu.deploy behaves as the command does: it deploys at once, and asks only in the one case the command asks — when
agentcore.jsondeclares nothing deployable, so deploying would remove the stack. Then the confirmation is the CLI's teardown question, and "y" is the pre-answered--yes. With several targets inaws-targets.jsonit first asks which (the TUI's stand-in for--target); with one or none it uses that ordefault, provisioning it on first deploy exactly as the command does.How
Everything on screen is a CLI piece wrapped in the TUI frame:
projectManager.build(project)/.deploy(project, …)yield* core.projectManager.build(project)TaskList, rendered inline byrunWithProgressTaskList, rendered byConfirmActionwhile the generator streamsrunWithProgressdriveProgress(over pureapplyProgressEvent/settleProgress) insrc/tui/progress.tsx; both renderers call it and supply only how to draw the tasksConfirmAction(the confirm → run → success | error body behindharness delete)AsyncGenerator<ProgressEvent, …>as itsactionand shows theTaskListin place of the bare spinner.message/title/rowsbecome optional (no question → run when ready; no header → no box). Existing callers unchangedConfirm(y/N)teardownQuestion(), exported from the handler and used by both;declaresNothingDeployable()(now exported) decides whether it is askedBuilt project '…'/Deployed project '…' to target '…'builtMessage()/deployedMessage(), exported and used by both. The deploy outcome is read fromresult.tornDown, as the command does — not from the preflight heuristic, which synthesis can disagree withDataTable(the invoke picker)KeyValueTableConfirmAction's header and result rows (replacing its fixed-width renderer)withProject's not-found messageprojectNotFoundMessage(), exported;useProject/ProjectGateshow it for a screen the user navigated to, esc back to the menuStack outputs are not listed on success, matching the command, which prints them only with
--json. Bareagentcore project deployon the command line is unchanged.Also
ProjectManager.listTargets(project)— the declared targets,[]with no file;resolveTargetdelegates to it.ProjectInvokePickerScreendrops its inline project-resolution effect and message copy foruseProject.ConfirmActiondoesn't advertiseescwhile the action runs — nothing listens for it mid-flight, by design — and gainsdoneLabel/nextSteps/onCancel.flatFrame/waitForFlatTexttest helpers.Testing
bun test: 2754 pass, 0 fail.tsc --noEmit,oxlint,prettier --checkclean.9 screen tests stub the backend exactly as the handler tests do, so both paths are asserted against identical events: build happy (enter → menu) / failure (✕ step with tail kept, esc → menu) / outside a project (esc → menu); deploy one target / several targets (picker) / fresh project with no
aws-targets.json(default provisioned, step streamed) / empty-project teardown confirm / result-vs-heuristic disagreement / failure. 6 unit tests for the extracted fold and driver.Driven by hand against a real project as well.