From 671b549603b1195621bdc612a9a8359643e7f173 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 4 Sep 2026 19:25:10 +0000 Subject: [PATCH 1/4] fix(tui): esc from a deep-linked harness wizard returns to the menu `agentcore harness create` with no flags opens the TUI directly on the wizard, so the wizard is the only history entry. Its first-step back was navigate(-1), which had nothing to pop, and esc did nothing. The wizard now takes an explicit onExit: create returns to the harness menu, update returns to the update picker. --- src/components/HarnessWizard.tsx | 9 ++++++--- src/handlers/harness/create/screen.tsx | 7 ++++++- src/handlers/harness/update/screen.tsx | 2 ++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/components/HarnessWizard.tsx b/src/components/HarnessWizard.tsx index 1e5887d76..8108ac50f 100644 --- a/src/components/HarnessWizard.tsx +++ b/src/components/HarnessWizard.tsx @@ -1,6 +1,5 @@ import { useMemo, useState } from "react"; import { Box, Text, useInput, useWindowSize } from "ink"; -import { useNavigate } from "react-router"; import type { Harness, HarnessMemoryConfiguration, @@ -246,6 +245,10 @@ export interface HarnessWizardProps extends ScreenProps { initial?: HarnessFormValues; // onDone is called after a successful submit is acknowledged. onDone: (harnessId: string) => void; + // onExit is called when escape leaves the first step. It navigates to an + // explicit screen rather than popping history: `agentcore harness create` + // opens the wizard as the first history entry, so a pop would go nowhere. + onExit: () => void; } const NAME_PATTERN = /^[a-zA-Z][a-zA-Z0-9_]{0,47}$/; @@ -263,8 +266,8 @@ export function HarnessWizard({ harnessId, initial, onDone, + onExit, }: HarnessWizardProps) { - const navigate = useNavigate(); const opts = coreOptsFromCtx(ctx); const steps: Step[] = useMemo(() => { @@ -290,7 +293,7 @@ export function HarnessWizard({ const next = () => setStepIndex((i) => Math.min(steps.length - 1, i + 1)); const back = () => { - if (stepIndex === 0) navigate(-1); + if (stepIndex === 0) onExit(); else setStepIndex((i) => i - 1); }; diff --git a/src/handlers/harness/create/screen.tsx b/src/handlers/harness/create/screen.tsx index 47e6f3280..7fc70f07b 100644 --- a/src/handlers/harness/create/screen.tsx +++ b/src/handlers/harness/create/screen.tsx @@ -1,13 +1,17 @@ +import { useNavigate } from "react-router"; import type { ScreenProps } from "../../types"; import { HarnessWizard } from "../../../components/HarnessWizard"; import { useFinishFlow } from "../../../components/useFinishFlow"; +const MENU_PATH = "/agentcore/harness"; + // HarnessCreateScreen is the interactive create-harness flow: a step wizard // (name → model → memory → tools → prompt → advanced → review) that ends in a // CreateHarness call. Success lands on the new harness's hub, with esc from // there returning to the harness menu rather than the finished wizard. export function HarnessCreateScreen(props: ScreenProps) { - const finishFlow = useFinishFlow("/agentcore/harness"); + const navigate = useNavigate(); + const finishFlow = useFinishFlow(MENU_PATH); return ( finishFlow(`/agentcore/harness/get/${harnessId}`)} + onExit={() => navigate(MENU_PATH)} /> ); } diff --git a/src/handlers/harness/update/screen.tsx b/src/handlers/harness/update/screen.tsx index 9b716fd59..67c1f47c6 100644 --- a/src/handlers/harness/update/screen.tsx +++ b/src/handlers/harness/update/screen.tsx @@ -31,6 +31,7 @@ export function HarnessUpdateScreen(props: ScreenProps) { } function UpdateWizard({ ctx, core, harnessId }: ScreenProps & { harnessId: string }) { + const navigate = useNavigate(); const opts = coreOptsFromCtx(ctx); const finishFlow = useFinishFlow("/agentcore/harness"); @@ -68,6 +69,7 @@ function UpdateWizard({ ctx, core, harnessId }: ScreenProps & { harnessId: strin breadcrumb={["agentcore", "harness", "update", harnessId]} initial={fromHarness(detail.data.harness!)} onDone={(id) => finishFlow(`/agentcore/harness/get/${id}`)} + onExit={() => navigate("/agentcore/harness/update")} /> ); } From f0f13651c2643e6be39e12c621db1160d1852a4f Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 4 Sep 2026 19:25:10 +0000 Subject: [PATCH 2/4] test(tui): cover esc from every command with a screen Walk the compiled command tree, mount every screen-backed command through the real route table, press esc, and assert the header belongs to a menu above it. This is the counterpart to the command-line-only walk in CliOnlyScreen.test, so a screen added later is covered without a new test. It catches both the policy picker redirect loop (#2225) and the deep-linked wizard. compiledRootCommand() is extracted from the harness so tests can enumerate the tree the same way the app does. --- src/components/Root.test.tsx | 69 ++++++++++++++++++++++++++++++++++++ src/testing/index.tsx | 1 + src/testing/renderScreen.tsx | 26 ++++++++------ 3 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 src/components/Root.test.tsx diff --git a/src/components/Root.test.tsx b/src/components/Root.test.tsx new file mode 100644 index 000000000..8188fff34 --- /dev/null +++ b/src/components/Root.test.tsx @@ -0,0 +1,69 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import type { Command } from "commander"; +import { isTuiCommandSupported } from "../router"; +import { cleanupScreens, compiledRootCommand, renderScreen, waitFor } from "../testing"; + +afterEach(cleanupScreens); + +// screenCommands walks the compiled Commander tree for every command with a +// screen, so a screen added later is covered without a new test. The root is +// skipped: it has nothing to go back to. (CliOnlyScreen.test covers the rest.) +function screenCommands(command: Command, path: string[]): [string[], Command][] { + const here = [...path, command.name()]; + return command.commands + .filter((child) => child.name() !== "help" && isTuiCommandSupported(child)) + .flatMap((child): [string[], Command][] => [ + [[...here, child.name()], child], + ...screenCommands(child, here), + ]); +} + +// menuHeader is the first line RouterScreen renders for a group at `path`. +function menuHeader(path: string[], command: Command): string { + return [...path, command.description()].join(" → "); +} + +// ancestorMenuHeaders lists the menu header of every group above a command, +// nearest first. Escape normally lands on the parent, but a group whose route +// only redirects to its single child (`gateway policy` → `generate`) has no +// menu of its own, so that child's escape skips to the grandparent. +function ancestorMenuHeaders(path: string[], command: Command): string[] { + const headers: string[] = []; + let at = path.slice(0, -1); + for (let cur = command.parent; cur; cur = cur.parent) { + headers.push(menuHeader(at, cur)); + at = at.slice(0, -1); + } + return headers; +} + +function firstLine(frame: string | undefined): string { + return (frame ?? "").split("\n")[0]?.trim() ?? ""; +} + +const SCREENS = screenCommands(compiledRootCommand(), []); + +describe("every command with a screen", () => { + test("there are screens to cover", () => { + expect(SCREENS.length).toBeGreaterThan(50); + }); + + // The route table decides what each path shows and where its escape goes. A + // path that redirects to a screen whose escape targets that same path loops + // in place, so from wherever a command opens, escape must reach a menu above. + test.each(SCREENS.map(([path, command]) => [path.join(" "), path, command] as const))( + "%s opens, and esc returns to a menu above it", + async (_label, path, command) => { + const r = renderScreen("/" + path.join("/")); + // Wide and tall enough that the header never wraps. + await r.resize(220, 200); + const menus = ancestorMenuHeaders(path, command); + expect(menus).not.toContain(firstLine(r.lastFrame())); + + await r.press("escape"); + await waitFor(() => menus.includes(firstLine(r.lastFrame()))).catch(() => {}); + expect(menus).toContain(firstLine(r.lastFrame())); + r.unmount(); + }, + ); +}); diff --git a/src/testing/index.tsx b/src/testing/index.tsx index 580731fb7..d99b7bcb7 100644 --- a/src/testing/index.tsx +++ b/src/testing/index.tsx @@ -21,6 +21,7 @@ export { } from "./TestCoreClient"; export { StreamController } from "./StreamController"; export { + compiledRootCommand, renderScreen, cleanupScreens, keys, diff --git a/src/testing/renderScreen.tsx b/src/testing/renderScreen.tsx index dafede8c6..aefe6f784 100644 --- a/src/testing/renderScreen.tsx +++ b/src/testing/renderScreen.tsx @@ -1,5 +1,6 @@ import { render, cleanup } from "ink-testing-library"; import { QueryClient } from "@tanstack/react-query"; +import type { Command } from "commander"; import { ValueContext, compile, CommandKey, PlatformKey, type Context } from "../router"; import { RegionKey, JsonKey, DebugKey, EndpointKey } from "../handlers/keys"; import { JsonRendererKey } from "../tui"; @@ -22,6 +23,20 @@ import { TestGlobalConfigAccessor } from "./globalConfig"; // synchronous frames, so useInput handlers and TextInput focus behave as in a // real terminal. +// compiledRootCommand compiles the real handler tree into the Commander command +// the app pins as CommandKey. Tests also walk it to enumerate every command, so +// a command added later is covered without a new test. +export function compiledRootCommand(core: TestCoreClient = new TestCoreClient()): Command { + return compile( + createRootHandler(core, { + io: testIO().io, + logger: createSilentLogger(), + globalConfigAccessor: new TestGlobalConfigAccessor(), + }), + ValueContext.EmptyContext(), + ); +} + // baseContext builds the Context a screen needs, mirroring what the app pins // before mounting the TUI: the compiled root Commander command (CommandKey — // RouterScreen walks it to resolve each menu's subcommands), the global flags @@ -32,17 +47,8 @@ function baseContext( endpointUrl?: string, platform: NodeJS.Platform = process.platform, ): Context { - const rootCommand = compile( - createRootHandler(core, { - io: testIO().io, - logger: createSilentLogger(), - globalConfigAccessor: new TestGlobalConfigAccessor(), - }), - ValueContext.EmptyContext(), - ); - return ValueContext.EmptyContext() - .withValue(CommandKey, rootCommand) + .withValue(CommandKey, compiledRootCommand(core)) .withValue(RegionKey, "us-east-1") .withValue(PlatformKey, platform) .withValue(EndpointKey, endpointUrl) From a46418d723190c4a7faca1bb332b9e13e9bfb7cb Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 4 Sep 2026 19:52:27 +0000 Subject: [PATCH 3/4] fix(tui): keep the update wizard's escape as a history pop The update wizard is only reached from the update picker or the detail screen's update action, never as the first history entry, so a pop returns to whichever the user came from. Only the create wizard can be deep-linked and needs an explicit target. Note the same invariant at EndpointWizard's pop, which the route walk cannot reach because every route into it passes a picker first. --- src/components/EndpointWizard.tsx | 1 + src/components/HarnessWizard.tsx | 6 +++--- src/handlers/harness/update/screen.tsx | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/EndpointWizard.tsx b/src/components/EndpointWizard.tsx index 36fd66a38..74372e8f2 100644 --- a/src/components/EndpointWizard.tsx +++ b/src/components/EndpointWizard.tsx @@ -86,6 +86,7 @@ export function EndpointWizard({ const stepKey = steps[stepIndex]!.key; const next = () => setStepIndex((i) => Math.min(steps.length - 1, i + 1)); const back = () => { + // Safe only while every route in passes a picker first; see HarnessWizard.onExit. if (stepIndex === 0) navigate(-1); else setStepIndex((i) => i - 1); }; diff --git a/src/components/HarnessWizard.tsx b/src/components/HarnessWizard.tsx index 8108ac50f..a1a937157 100644 --- a/src/components/HarnessWizard.tsx +++ b/src/components/HarnessWizard.tsx @@ -245,9 +245,9 @@ export interface HarnessWizardProps extends ScreenProps { initial?: HarnessFormValues; // onDone is called after a successful submit is acknowledged. onDone: (harnessId: string) => void; - // onExit is called when escape leaves the first step. It navigates to an - // explicit screen rather than popping history: `agentcore harness create` - // opens the wizard as the first history entry, so a pop would go nowhere. + // onExit runs when escape leaves the first step. A history pop goes nowhere + // when the wizard is the first entry (`agentcore harness create` deep-links + // here), so such callers must navigate to an explicit screen. onExit: () => void; } diff --git a/src/handlers/harness/update/screen.tsx b/src/handlers/harness/update/screen.tsx index 67c1f47c6..4c752c06f 100644 --- a/src/handlers/harness/update/screen.tsx +++ b/src/handlers/harness/update/screen.tsx @@ -69,7 +69,7 @@ function UpdateWizard({ ctx, core, harnessId }: ScreenProps & { harnessId: strin breadcrumb={["agentcore", "harness", "update", harnessId]} initial={fromHarness(detail.data.harness!)} onDone={(id) => finishFlow(`/agentcore/harness/get/${id}`)} - onExit={() => navigate("/agentcore/harness/update")} + onExit={() => navigate(-1)} /> ); } From 46bccc29656b0d6304ac21c66cd684acf0f1e0c8 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Fri, 4 Sep 2026 19:52:28 +0000 Subject: [PATCH 4/4] test(tui): reuse compiledRootCommand and note what the walk tolerates Drop the two inline root compiles now that the harness exports one, and record that the walk accepts any ancestor menu, so it catches a no-op or a loop but not an escape that jumps too far up. --- src/components/CliOnlyScreen.test.tsx | 28 ++++++-------------- src/components/Root.test.tsx | 2 ++ src/handlers/project/project.screen.test.tsx | 20 +++----------- 3 files changed, 13 insertions(+), 37 deletions(-) diff --git a/src/components/CliOnlyScreen.test.tsx b/src/components/CliOnlyScreen.test.tsx index 8a7f2508f..b6b7c91eb 100644 --- a/src/components/CliOnlyScreen.test.tsx +++ b/src/components/CliOnlyScreen.test.tsx @@ -1,35 +1,23 @@ import { test, expect, describe, afterEach } from "bun:test"; import type { Command } from "commander"; import { - renderScreen, - waitForText, cleanupScreens, - createSilentLogger, + compiledRootCommand, menuEntries, - TestCoreClient, - TestGlobalConfigAccessor, - testIO, + renderScreen, + waitForText, } from "../testing"; -import { compile, isTuiCommandSupported, ValueContext } from "../router"; -import { createRootHandler } from "../handlers"; +import { isTuiCommandSupported } from "../router"; afterEach(cleanupScreens); -function compiledRoot(): Command { - return compile( - createRootHandler(new TestCoreClient(), { - io: testIO().io, - logger: createSilentLogger(), - globalConfigAccessor: new TestGlobalConfigAccessor(), - }), - ValueContext.EmptyContext(), - ); -} - // cliOnlyCommands walks the compiled Commander tree for every command without // a screen, so a command added later is covered without a new test. `help` is // Commander's own, not one of ours. -function cliOnlyCommands(command = compiledRoot(), path: string[] = []): [string[], Command][] { +function cliOnlyCommands( + command = compiledRootCommand(), + path: string[] = [], +): [string[], Command][] { const here = [...path, command.name()]; const own: [string[], Command][] = isTuiCommandSupported(command) ? [] : [[here, command]]; return [ diff --git a/src/components/Root.test.tsx b/src/components/Root.test.tsx index 8188fff34..dd5532f9c 100644 --- a/src/components/Root.test.tsx +++ b/src/components/Root.test.tsx @@ -51,6 +51,8 @@ describe("every command with a screen", () => { // The route table decides what each path shows and where its escape goes. A // path that redirects to a screen whose escape targets that same path loops // in place, so from wherever a command opens, escape must reach a menu above. + // Any ancestor menu is accepted, so this catches a no-op or a loop but not + // an escape that jumps further up than it should. test.each(SCREENS.map(([path, command]) => [path.join(" "), path, command] as const))( "%s opens, and esc returns to a menu above it", async (_label, path, command) => { diff --git a/src/handlers/project/project.screen.test.tsx b/src/handlers/project/project.screen.test.tsx index e088de6d4..2a200a24b 100644 --- a/src/handlers/project/project.screen.test.tsx +++ b/src/handlers/project/project.screen.test.tsx @@ -4,6 +4,7 @@ import { waitForFlatText, waitForText, cleanupScreens, + compiledRootCommand, createSilentLogger, menuEntries, TestCoreClient, @@ -11,7 +12,6 @@ import { testIO, } from "../../testing"; import { InvalidEnvironmentError } from "../../errors"; -import { compile, ValueContext } from "../../router"; import { ExitCode } from "../../runnable"; import { createRootHandler } from "../index"; @@ -20,14 +20,7 @@ afterEach(cleanupScreens); // projectSubcommands reads the project group's children off the compiled // Commander tree, so tests driven by it cover any subcommand added later. function projectSubcommands(): string[] { - const root = compile( - createRootHandler(new TestCoreClient(), { - io: testIO().io, - logger: createSilentLogger(), - globalConfigAccessor: new TestGlobalConfigAccessor(), - }), - ValueContext.EmptyContext(), - ); + const root = compiledRootCommand(); const project = root.commands.find((command) => command.name() === "project")!; return project.commands.map((command) => command.name()); } @@ -71,14 +64,7 @@ describe("project menu", () => { // projectCommand resolves a compiled project subcommand by path, for reading // the help the CLI-only screen must match. function projectCommand(...path: string[]) { - const root = compile( - createRootHandler(new TestCoreClient(), { - io: testIO().io, - logger: createSilentLogger(), - globalConfigAccessor: new TestGlobalConfigAccessor(), - }), - ValueContext.EmptyContext(), - ); + const root = compiledRootCommand(); let command = root.commands.find((c) => c.name() === "project")!; for (const name of path) command = command.commands.find((c) => c.name() === name)!; return command;