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/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 1e5887d76..a1a937157 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 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; } 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/components/Root.test.tsx b/src/components/Root.test.tsx new file mode 100644 index 000000000..dd5532f9c --- /dev/null +++ b/src/components/Root.test.tsx @@ -0,0 +1,71 @@ +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. + // 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) => { + 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/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..4c752c06f 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(-1)} /> ); } 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; 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)