Skip to content

Commit 74e9bd5

Browse files
authored
feat: centralize support TUI command routing logic (#1940)
* feat: centralize support TUI command routing logic * refactor: switch supportedTUICommands from global to handler level
1 parent af7f3bf commit 74e9bd5

24 files changed

Lines changed: 182 additions & 50 deletions

src/components/RouterScreen.tsx

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React, { useMemo, useState } from "react";
22
import { Box, Text, useApp, useInput, useStdin } from "ink";
33
import type { Command } from "commander";
44
import { useNavigate } from "react-router";
5-
import { CommandKey } from "../router";
5+
import { CommandKey, isTuiCommandSupported } from "../router";
66
import { Layout } from "./Layout";
77
import { Divider } from "./ui/divider";
88
import { TextInput } from "./ui/text-input";
@@ -44,18 +44,13 @@ export interface RouterScreenProps extends ScreenProps {
4444
// segment is the app root; the last is the command whose subcommands are the
4545
// menu options.
4646
path: string[];
47-
// omit hides subcommands from the menu by name. The command tree still carries
48-
// them (they remain usable from the CLI), but they are not offered here — used
49-
// when the TUI intentionally does not route a command yet, so it can't fall
50-
// through to the HelpScreen catch-all (which exits the app).
51-
omit?: string[];
5247
}
5348

5449
// RouterScreen renders the interactive command menu for a Router node: a filter
5550
// input at the top and the node's subcommands (read straight off the Commander
5651
// Command) as navigable options below. Selecting an option routes to that
5752
// subcommand's screen.
58-
export function RouterScreen({ ctx, path, omit }: RouterScreenProps) {
53+
export function RouterScreen({ ctx, path }: RouterScreenProps) {
5954
const navigate = useNavigate();
6055
const { isRawModeSupported } = useStdin();
6156
const { exit } = useApp();
@@ -64,9 +59,9 @@ export function RouterScreen({ ctx, path, omit }: RouterScreenProps) {
6459
const options: Option[] = useMemo(
6560
() =>
6661
command.commands
67-
.filter((c) => !omit?.includes(c.name()))
62+
.filter(isTuiCommandSupported)
6863
.map((c) => ({ name: c.name(), description: c.description() })),
69-
[command, omit],
64+
[command],
7065
);
7166

7267
const [query, setQuery] = useState("");

src/handlers/eval/evaluator/evaluator.test.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,19 @@ describe("eval command hierarchy", () => {
167167
);
168168
},
169169
);
170+
171+
test("runs normal validation for a bare CLI-only evaluator command", async () => {
172+
await expect(run(["eval", "evaluator", "delete"])).rejects.toThrow(
173+
"required option '--id <id>' not specified",
174+
);
175+
});
176+
177+
test("prints help for a bare CLI-only evaluator group", async () => {
178+
const stdout = await run(["eval", "evaluator", "llm-as-a-judge"]);
179+
180+
expect(stdout).toContain("Usage: agentcore eval evaluator llm-as-a-judge");
181+
expect(stdout).toContain("Commands:");
182+
});
170183
});
171184

172185
describe("evaluator CRUDL", () => {

src/handlers/eval/evaluator/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export function createEvaluatorHandler(core: Core, io: AppIO): Router {
1313
return new Router("evaluator", "manage AgentCore evaluators")
1414
.use(withTuiOnEmptyFlagsAndArgs(core, io))
1515
.default(renderTui(core, io))
16+
.supportedTuiCommands("get", "list")
1617
.handler(createLlmAsAJudgeHandler(core, io))
1718
.handler(createCodeBasedHandler(core, io))
1819
.handler(createGetEvaluatorHandler(core))
Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
import { RouterScreen } from "../../../components/RouterScreen";
22
import type { ScreenProps } from "../../types";
33

4-
// The read-only TUI offers only get/list. The mutating subcommands
5-
// (llm-as-a-judge and code-based, which host create/update; and delete) stay
6-
// CLI-only for now, so they are omitted from the menu — an unrouted menu entry
7-
// would fall through to the HelpScreen catch-all and exit the app.
8-
const OMIT = ["llm-as-a-judge", "code-based", "delete"];
9-
104
export function EvaluatorScreen(props: ScreenProps) {
11-
return <RouterScreen {...props} path={["agentcore", "eval", "evaluator"]} omit={OMIT} />;
5+
return <RouterScreen {...props} path={["agentcore", "eval", "evaluator"]} />;
126
}

src/handlers/eval/online-eval/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export function createOnlineEvalHandler(core: Core, io: AppIO): Router {
1515
return new Router("online-eval", "manage AgentCore online evaluation configs")
1616
.use(withTuiOnEmptyFlagsAndArgs(core, io))
1717
.default(renderTui(core, io))
18+
.supportedTuiCommands("get", "list")
1819
.handler(createCreateOnlineEvalHandler(core, io))
1920
.handler(createGetOnlineEvalHandler(core))
2021
.handler(createListOnlineEvalHandler(core))

src/handlers/eval/online-eval/online-eval.test.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ describe("eval online-eval command hierarchy", () => {
113113
);
114114
},
115115
);
116+
117+
test("runs normal validation for a bare CLI-only command", async () => {
118+
await expect(run(["eval", "online-eval", "create"])).rejects.toThrow(
119+
"required option '--name <name>' not specified",
120+
);
121+
});
116122
});
117123

118124
describe("online-eval CRUDL", () => {
Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
import { RouterScreen } from "../../../components/RouterScreen";
22
import type { ScreenProps } from "../../types";
33

4-
// The read-only TUI offers only get/list. The mutating subcommands stay CLI-only
5-
// for now, so they are omitted from the menu — an unrouted menu entry would fall
6-
// through to the HelpScreen catch-all and exit the app.
7-
const OMIT = ["create", "update", "pause", "resume", "delete"];
8-
94
export function OnlineEvalScreen(props: ScreenProps) {
10-
return <RouterScreen {...props} path={["agentcore", "eval", "online-eval"]} omit={OMIT} />;
5+
return <RouterScreen {...props} path={["agentcore", "eval", "online-eval"]} />;
116
}

src/handlers/identity/api-key-credential-provider/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { createUpdateApiKeyCredentialProviderHandler } from "./update";
1111
export function createApiKeyCredentialProviderHandler(core: Core, io: AppIO): Router {
1212
return new Router("api-key-credential-provider", "manage API key credential providers")
1313
.default(renderTui(core, io))
14+
.supportedTuiCommands("get", "list")
1415
.handler(createCreateApiKeyCredentialProviderHandler(core, io))
1516
.handler(createGetApiKeyCredentialProviderHandler(core))
1617
.handler(createListApiKeyCredentialProvidersHandler(core))
Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,8 @@
11
import { RouterScreen } from "../../../components/RouterScreen";
22
import type { ScreenProps } from "../../types";
33

4-
const OMIT = ["create", "update", "delete"];
5-
64
export function ApiKeyCredentialProviderScreen(props: ScreenProps) {
75
return (
8-
<RouterScreen
9-
{...props}
10-
path={["agentcore", "identity", "api-key-credential-provider"]}
11-
omit={OMIT}
12-
/>
6+
<RouterScreen {...props} path={["agentcore", "identity", "api-key-credential-provider"]} />
137
);
148
}

src/handlers/identity/identity.test.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,22 @@ describe("api-key-credential-provider TUI dispatch", () => {
8181
test.each([
8282
["identity", ["identity"]],
8383
["api-key-credential-provider", ["identity", "api-key-credential-provider"]],
84-
["create", ["identity", "api-key-credential-provider", "create"]],
8584
["get", ["identity", "api-key-credential-provider", "get"]],
8685
["list", ["identity", "api-key-credential-provider", "list"]],
87-
["update", ["identity", "api-key-credential-provider", "update"]],
88-
["delete", ["identity", "api-key-credential-provider", "delete"]],
8986
] as const)("opens the TUI for a bare `%s`", async (_label, args) => {
9087
await expect(run([...args])).rejects.toThrow(
9188
"interactive mode requires a TTY on stdin and stdout",
9289
);
9390
});
91+
92+
test.each(["create", "update", "delete"] as const)(
93+
"runs normal validation for bare CLI-only `%s`",
94+
async (command) => {
95+
await expect(run(["identity", "api-key-credential-provider", command])).rejects.toThrow(
96+
"required option '--name <name>' not specified",
97+
);
98+
},
99+
);
94100
});
95101

96102
describe("api-key-credential-provider CRUDL", () => {

0 commit comments

Comments
 (0)