feat(runtime): add Runtime invocation#1820
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #1820 +/- ##
============================================
+ Coverage 94.84% 95.35% +0.51%
============================================
Files 143 151 +8
Lines 7120 8208 +1088
============================================
+ Hits 6753 7827 +1074
- Misses 367 381 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| breadcrumb: string[]; | ||
| description?: string; | ||
| onSelect: (qualifier: string) => void; | ||
| onEscape?: () => void; |
There was a problem hiding this comment.
do we want to allow this to be overridden? My understanding is the esc consistently means go back to the last page right now in the tui, and by allowing an override we open the door to potentially unexpected behavior.
|
|
||
| export class RuntimeClient implements CoreRuntimeClient { | ||
| constructor(private readonly clients: AwsClients) {} | ||
| constructor( |
There was a problem hiding this comment.
should we be injecting the logger here? I see a few places below that might benefit from some detailed debug logs
There was a problem hiding this comment.
I'll add some logging here, agreed.
| throw new TypeError("CUSTOM_JWT requires an HTTPS endpoint"); | ||
| } | ||
| url.pathname = `${url.pathname.replace(/\/?$/, "/")}runtimes/${encodeURIComponent(runtimeId)}/invocations`; | ||
| url.search = new URLSearchParams({ |
There was a problem hiding this comment.
does it makes sense to expand the responsibility of CoreFetch to be a generic HttpClient where we can move some of this logic? I imagine we'll hit other cases where we want to do something similar to here.
There was a problem hiding this comment.
I think that would be a good use case for CoreFetch. I agree that this logic could be moved over I'll scope that out.
| try { | ||
| await renderTuiAt(path, ctx, core, io); | ||
| } catch (error) { | ||
| throw error instanceof TypeError && |
There was a problem hiding this comment.
is it worth defining an explicit error for this case? wondering if depending on the message might be brittle.
| if (jsonOutput && flags["output-file"] !== undefined) { | ||
| throw new UsageError("--json cannot be used with --output-file"); | ||
| } | ||
| if (flags["output-file"] === "") { |
There was a problem hiding this comment.
could this be enforced in the schema?
| @@ -0,0 +1,165 @@ | |||
| import { readFile } from "node:fs/promises"; | |||
There was a problem hiding this comment.
should this request/response live outside of handlers? I kind of thought the original idea is that handlers are lightweight and mirror the command structure, with functionality mostly coming from core or injected through context.
@AlexanderRichey curious to your thoughts here.
| try { | ||
| validateHeaderValue(name, value); | ||
| } catch { | ||
| throw new UsageError(`Invalid header value for ${name}`); |
There was a problem hiding this comment.
is there a way we can give a hint to the user about why its invalid? Also, I think wiring up logging here could be valuable as well.
| } catch (e) { | ||
| const error = e instanceof Error ? e : new Error(String(e)); | ||
| console.error(`${error.name}: ${error.message}`); | ||
| } catch (error) { |
There was a problem hiding this comment.
Would it be simpler to define our own error base class that carries the exit code directly, and then leverage it here?
| return ExitCode.SUCCESS; | ||
| } catch (e) { | ||
| const error = e instanceof Error ? e : new Error(String(e)); | ||
| console.error(`${error.name}: ${error.message}`); |
There was a problem hiding this comment.
I think we want to keep this console.error, it ensures that any initialization errors are logged to the user.
| }); | ||
|
|
||
| test("reports invalid input via command.error (throws under exitOverride)", async () => { | ||
| test("a short alias preserves encounter order for repeated variadic values", async () => { |
There was a problem hiding this comment.
what exactly are we verifying here?
AlexanderRichey
left a comment
There was a problem hiding this comment.
I didn't have time to read the whole thing, but left some initial feedback in the comments.
| const error = e instanceof Error ? e : new Error(String(e)); | ||
| console.error(`${error.name}: ${error.message}`); | ||
| } catch (error) { | ||
| if (error instanceof CommanderError) { |
There was a problem hiding this comment.
I see the need for something that controls the exit code, but I'm not sure this is how we ought to do it. I think we should define something like AgentCoreError that is a custom Error type that has an optional exitCode field. This should probably come in a separate PR.
| // Flag<string, unknown>. | ||
| export interface Flag<N extends string = string, T = unknown> { | ||
| name: N; | ||
| short?: string; |
There was a problem hiding this comment.
I think we should hold off on adding this feature for now. This will end up adding a lot more for us to maintain.
| } from "@aws-sdk/client-bedrock-agentcore-control"; | ||
| import type { CoreOptions } from "../../core/types"; | ||
|
|
||
| export interface RuntimeInvokeRequest { |
There was a problem hiding this comment.
These should be types and not interfaces. Types hold data whereas interfaces are abstract, don't hold data themselves, and have to be implemented by something else.
| } | ||
| responseText += decoder.decode(); | ||
| updateExchange({ response: responseText }); | ||
| let text: string | undefined; |
There was a problem hiding this comment.
There's a whole lot going on here. I tend to think there's a way to simplify all this. Let's find some time next week to discuss.
Description
Adds headless and interactive invocation for the Runtime commands and TUI introduced in #1802.
Headless invocation supports both IAM and CUSTOM_JWT Runtimes. IAM requests use the AgentCore data-plane SDK, while CUSTOM_JWT requests use the generated Runtime data endpoint with an explicit bearer token. Payloads and bearer tokens can come from inline values, files, or stdin, with validation preventing both from consuming stdin at once.
The command maps the public Runtime invocation contract, including endpoint qualifiers, content negotiation, Runtime and MCP session IDs, application headers, MCP metadata, and tracing headers. Application headers are validated against the Runtime allowlist and cannot override reserved protocol headers.
-His available as the short form of--header.Raw output streams the exact response bytes to stdout and writes response metadata to stderr. Binary responses can be written directly to a file, while
--jsonemits one buffered response envelope with UTF-8 or base64 body encoding. Partial responses retain the bytes already received and report whether the stream completed.Without
--payload,runtime invokeopens a persistent console. Users can select a Runtime and endpoint, edit request options, send repeated requests, retain response history, switch targets, toggle completed JSON between raw and formatted views, and save binary responses to a file. Returned Runtime and MCP sessions are adopted only after a response completes.Headless
SIGINT, interactive Escape, route changes, and component unmounts propagate cancellation through Runtime lookup, transport, and response output. Usage failures return exit code 2, interruptions return 130, and transport or stream failures avoid exposing payloads, tokens, headers, or arbitrary upstream error causes. Invoke payloads, application headers, and bearer tokens are marked sensitive for command logging.This change also adds shared abortable-stream handling used by Runtime and Harness, extends the Core Runtime client with invocation transport and injectable fetch support, and documents IAM, CUSTOM_JWT, MCP, binary, JSON, and persistent-console workflows.
Runtime creation and mutation, deployment or project resolution, cross-account invocation, Runtime ARNs, version targeting, custom request paths, and the Runtime
/commandsroute remain outside this change.Related Issue
N/A
Documentation PR
N/A. README command, invocation, output, MCP, and interactive-console documentation is included in this PR.
Type of Change
Testing
Verified from current HEAD
cbd491b0:bun test(406 passed, 0 failed)bun run typecheckbun run lint:checkbun run format:checkbun run buildChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.