diff --git a/src/components/ShellHandoff.tsx b/src/components/ShellHandoff.tsx new file mode 100644 index 000000000..c0c7377c7 --- /dev/null +++ b/src/components/ShellHandoff.tsx @@ -0,0 +1,86 @@ +import { useEffect, useRef, useState } from "react"; +import { Text, useApp, useInput, useStderr, useStdin, useStdout } from "ink"; +import { useLocation, useNavigate } from "react-router"; +import { AgentCoreCLIError, SilentCLIError } from "../errors"; +import type { AppIO } from "../io"; +import { Layout } from "./Layout"; +import { Spinner } from "./ui/spinner"; + +type ShellHandoffProps = { + label: string; + returnPath?: string; + run: (io: AppIO) => Promise; + errorHint?: (error: Error) => string | undefined; +}; + +export function ShellHandoff({ label, returnPath, run, errorHint }: ShellHandoffProps) { + const { exit, suspendTerminal } = useApp(); + const { stdin } = useStdin(); + const { stdout } = useStdout(); + const { stderr } = useStderr(); + const location = useLocation(); + const navigate = useNavigate(); + const requested = useRef(false); + const [error, setError] = useState(null); + const [attempt, setAttempt] = useState(0); + + useInput( + (input, key) => { + if (key.ctrl && input === "c") exit(); + else if (key.escape) { + navigate(returnPath ?? location.pathname.slice(0, location.pathname.lastIndexOf("/")), { + replace: true, + }); + } else if (input === "r") { + requested.current = false; + setError(null); + setAttempt((current) => current + 1); + } + }, + { isActive: error !== null }, + ); + + useEffect(() => { + if (requested.current) return; + requested.current = true; + void (async () => { + try { + await suspendTerminal(() => run({ stdin, stdout, stderr })); + } catch (caught) { + if (!(caught instanceof SilentCLIError)) { + setError(AgentCoreCLIError.fromError(caught)); + return; + } + if (returnPath === undefined) { + exit(caught); + return; + } + } + if (returnPath === undefined) exit(); + else navigate(returnPath, { replace: true }); + })(); + }, [attempt, exit, navigate, returnPath, run, stderr, stdin, stdout, suspendTerminal]); + + if (error) { + const hint = errorHint?.(error); + return ( + + Error: {error.message} + {hint && ( + + {"\n"} + {hint} + + )} + + ); + } + return ; +} diff --git a/src/core/runtimeShell.test.ts b/src/core/runtimeShell.test.ts index 42274f356..36773e28c 100644 --- a/src/core/runtimeShell.test.ts +++ b/src/core/runtimeShell.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { MAX_FRAME_SIZE, ShellChannel } from "bedrock-agentcore/runtime"; +import { InputValidationError } from "../errors"; import type { RuntimeShellRequest } from "../handlers/runtime/types"; import { createRuntimeShellOpener, @@ -204,9 +205,9 @@ describe("createRuntimeShellOpener", () => { expect(delays).toEqual([250, 500]); }); - test("does not retry a non-retryable failure", async () => { + test.each([400, 403])("does not retry an HTTP %s rejection", async (statusCode) => { let attempts = 0; - const failure = new Error("Server rejected WebSocket connection: HTTP 403"); + const failure = new Error(`Server rejected WebSocket connection: HTTP ${statusCode}`); const opener = createRuntimeShellOpener({ createClient: () => ({ openShell: async () => { @@ -217,7 +218,21 @@ describe("createRuntimeShellOpener", () => { sleep: async () => {}, }); - await expect(opener(REQUEST, { region: "us-west-2" })).rejects.toBe(failure); + if (statusCode === 400) { + const error = await opener(REQUEST, { region: "us-west-2" }).catch((error: unknown) => error); + expect(error).toBeInstanceOf(InputValidationError); + expect(error).toMatchObject({ + cause: failure, + source: "user", + exitCode: 2, + }); + expect((error as Error).message).toContain( + "HTTP 400\n\nIf this Runtime is managed by a harness", + ); + expect((error as Error).message).toContain("agentcore harness shell --id "); + } else { + await expect(opener(REQUEST, { region: "us-west-2" })).rejects.toBe(failure); + } expect(attempts).toBe(1); }); }); diff --git a/src/core/runtimeShell.ts b/src/core/runtimeShell.ts index 6271753ea..ac4f2d8d8 100644 --- a/src/core/runtimeShell.ts +++ b/src/core/runtimeShell.ts @@ -7,7 +7,9 @@ import { type ShellSession, } from "bedrock-agentcore/runtime"; import { Buffer } from "node:buffer"; +import { InputValidationError } from "../errors"; import type { RuntimeShellFrame, RuntimeShellSession } from "../handlers/runtime/types"; +import { runtimeShellErrorHint } from "../handlers/runtime/shell/error"; import type { OpenRuntimeShell } from "./runtime"; import type { CoreOptions } from "./types"; @@ -74,6 +76,12 @@ export function createRuntimeShellOpener(config: RuntimeShellOpenerConfig = {}): const session = await client.openShell(input); return new RuntimeShellSessionAdapter(session); } catch (error) { + if (error instanceof Error) { + const hint = runtimeShellErrorHint(error); + if (hint !== undefined) { + throw new InputValidationError(`${error.message}\n\n${hint}`, { cause: error }); + } + } if (attempt >= MAX_ATTEMPTS || !isRetryableUpgrade(error)) throw error; await sleep(delayMs); delayMs *= 2; diff --git a/src/handlers/runtime/shell/error.ts b/src/handlers/runtime/shell/error.ts new file mode 100644 index 000000000..b5d5e44ad --- /dev/null +++ b/src/handlers/runtime/shell/error.ts @@ -0,0 +1,7 @@ +export function runtimeShellErrorHint(error: Error): string | undefined { + if (error.message !== "Server rejected WebSocket connection: HTTP 400") return undefined; + return ( + "If this Runtime is managed by a harness, open its shell with:\n" + + "agentcore harness shell --id " + ); +} diff --git a/src/handlers/runtime/shell/screen.tsx b/src/handlers/runtime/shell/screen.tsx index 6687b3c46..773b5639e 100644 --- a/src/handlers/runtime/shell/screen.tsx +++ b/src/handlers/runtime/shell/screen.tsx @@ -1,12 +1,10 @@ -import { useEffect, useRef } from "react"; -import { useApp, useStderr, useStdin, useStdout } from "ink"; import { useLocation, useNavigate, useParams } from "react-router"; import { RuntimeEndpointPicker } from "../../../components/RuntimeEndpointPicker"; import { RuntimePicker } from "../../../components/RuntimePicker"; -import { Spinner } from "../../../components/ui/spinner"; -import { SilentCLIError } from "../../../errors"; +import { ShellHandoff } from "../../../components/ShellHandoff"; import type { ScreenProps } from "../../types"; import { RuntimeShellLaunchContextKey } from "./launchContext"; +import { runtimeShellErrorHint } from "./error"; import { runRuntimeShell } from "./operation"; type RuntimeShellLocationState = { @@ -64,73 +62,21 @@ export function RuntimeShellScreen(props: ScreenProps) { ); } + const launch = props.ctx.value(RuntimeShellLaunchContextKey); return ( - + runRuntimeShell({ + ...props, + io, + runtimeId, + qualifier, + launchContext: launch?.runtimeId === runtimeId ? launch : undefined, + }) + } /> ); } - -function RuntimeShellHandoff({ - ctx, - core, - runtimeId, - qualifier, - returnPath, -}: ScreenProps & { runtimeId: string; qualifier: string; returnPath?: string }) { - const { exit, suspendTerminal } = useApp(); - const { stdin } = useStdin(); - const { stdout } = useStdout(); - const { stderr } = useStderr(); - const navigate = useNavigate(); - const requested = useRef(false); - const launchContext = ctx.value(RuntimeShellLaunchContextKey); - const initialContext = launchContext?.runtimeId === runtimeId ? launchContext : undefined; - - useEffect(() => { - if (requested.current) return; - requested.current = true; - void (async () => { - try { - await suspendTerminal(() => - runRuntimeShell({ - ctx, - core, - io: { stdin, stdout, stderr }, - runtimeId, - qualifier, - launchContext: initialContext, - }), - ); - } catch (error) { - if (returnPath === undefined || !(error instanceof SilentCLIError)) { - exit(error); - return; - } - } - if (returnPath === undefined) { - exit(); - } else { - navigate(returnPath, { replace: true }); - } - })(); - }, [ - core, - ctx, - exit, - initialContext, - navigate, - qualifier, - returnPath, - runtimeId, - stderr, - stdin, - stdout, - suspendTerminal, - ]); - - return ; -} diff --git a/src/handlers/runtime/shell/shell.screen.test.tsx b/src/handlers/runtime/shell/shell.screen.test.tsx index 48c345ab9..275b06c2e 100644 --- a/src/handlers/runtime/shell/shell.screen.test.tsx +++ b/src/handlers/runtime/shell/shell.screen.test.tsx @@ -1,5 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test"; import { renderTuiAt } from "../../../tui"; +import { createRuntimeShellOpener } from "../../../core/runtimeShell"; import { DebugKey, EndpointKey, JsonKey, RegionKey } from "../../keys"; import { ValueContext } from "../../../router"; import type { RuntimeShellSession } from "../types"; @@ -146,19 +147,69 @@ describe("RuntimeShellScreen", () => { expect(streams.stderr()).toContain("Connected"); }); - test("renderTuiAt propagates unexpected shell failures", async () => { + test("keeps rejected shell connections in the TUI with retry and endpoint back navigation", async () => { const value = core(); - value.runtime.setError(new Error("shell lookup failed")); - const { streams } = ttyTestIO(); + const rejectShell = createRuntimeShellOpener({ + createClient: () => ({ + openShell: async () => { + throw new Error("Server rejected WebSocket connection: HTTP 400"); + }, + }), + }); + const openShell = value.runtime.openRuntimeShell.bind(value.runtime); + let attempts = 0; + let reported: Error | undefined; + value.runtime.openRuntimeShell = async (request, options) => { + if (++attempts > 2) return openShell(request, options); + try { + return await rejectShell(request, options); + } catch (error) { + reported = error as Error; + throw error; + } + }; + const { streams, stdin } = ttyTestIO(); const ctx = ValueContext.EmptyContext() .withValue(RegionKey, "us-east-1") .withValue(EndpointKey, undefined) .withValue(JsonKey, false) .withValue(DebugKey, false); - await expect( - renderTuiAt("/agentcore/runtime/shell/checkout-AbCdEf1234/prod", ctx, value, streams.io), - ).rejects.toThrow("shell lookup failed"); + const rendering = renderTuiAt( + "/agentcore/runtime/shell/checkout-AbCdEf1234/prod", + ctx, + value, + streams.io, + ); + void rendering.catch(() => {}); + try { + const errorText = "HTTP 400"; + await waitFor(() => streams.stdout().includes(errorText)); + expect(streams.stdout()).toContain("checkout-AbCdEf1234"); + expect(streams.stdout()).toContain("retry"); + expect(streams.stdout()).toContain("back"); + expect(streams.stdout().replace(/\s+/g, "")).toContain(reported!.message.replace(/\s+/g, "")); + expect(streams.stdout().replace(/\s+/g, " ")).toContain( + "If this Runtime is managed by a harness", + ); + expect(streams.stdout()).toContain("\nIf this Runtime"); + expect(streams.stdout().replace(/\s+/g, " ")).toContain( + "agentcore harness shell --id ", + ); + const errorsBeforeRetry = streams.stdout().split(errorText).length; + stdin.write("r"); + await waitFor(() => attempts === 2); + await waitFor(() => streams.stdout().split(errorText).length > errorsBeforeRetry); + stdin.write("\x1b"); + await waitFor(() => streams.stdout().includes("choose an endpoint to open a shell")); + await waitFor(() => streams.stdout().includes("updated UTC")); + expect(value.runtime.calls.some((call) => call.method === "listRuntimes")).toBe(false); + stdin.write("\r"); + await waitFor(() => streams.stderr().includes("Session closed")); + expect(attempts).toBe(3); + } finally { + await interruptUntilExit(rendering, stdin); + } }); test("renderTuiAt returns to a requested origin after the shell ends", async () => {