-
Notifications
You must be signed in to change notification settings - Fork 52
fix(cli): add --endpoint flag to invoke and set SigV4 qualifier #1648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9b327a0
acffba3
8c153bc
e0bb133
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| import { invokeAgentRuntime, invokeAgentRuntimeStreaming, invokeAguiRuntime } from '../agentcore.js'; | ||
| import type { InvokeAgentRuntimeOptions } from '../agentcore.js'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| const commandArgs: Record<string, unknown>[] = []; | ||
| const mockSend = vi.fn(); | ||
|
|
||
| vi.mock('@aws-sdk/client-bedrock-agentcore', () => { | ||
| class MockBedrockAgentCoreClient { | ||
| send = mockSend; | ||
| middlewareStack = { add: vi.fn() }; | ||
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| constructor(_config: unknown) {} | ||
| } | ||
| class MockInvokeAgentRuntimeCommand { | ||
| input: Record<string, unknown>; | ||
| constructor(args: Record<string, unknown>) { | ||
| this.input = args; | ||
| commandArgs.push(args); | ||
| } | ||
| } | ||
| return { | ||
| BedrockAgentCoreClient: MockBedrockAgentCoreClient, | ||
| InvokeAgentRuntimeCommand: MockInvokeAgentRuntimeCommand, | ||
| }; | ||
| }); | ||
|
|
||
| vi.mock('../account.js', () => ({ | ||
| getCredentialProvider: vi | ||
| .fn() | ||
| .mockReturnValue(() => Promise.resolve({ accessKeyId: 'test', secretAccessKey: 'test' })), | ||
| })); | ||
|
|
||
| function makeByteResponse(body: string) { | ||
| return { | ||
| runtimeSessionId: 'sess-1', | ||
| response: { | ||
| transformToByteArray: () => Promise.resolve(new TextEncoder().encode(body)), | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| function makeStreamResponse(body: string) { | ||
| return { | ||
| runtimeSessionId: 'sess-1', | ||
| response: { | ||
| transformToWebStream: () => | ||
| new ReadableStream<Uint8Array>({ | ||
| start(controller) { | ||
| controller.enqueue(new TextEncoder().encode(body)); | ||
| controller.close(); | ||
| }, | ||
| }), | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| const BASE: InvokeAgentRuntimeOptions = { | ||
| region: 'us-east-1', | ||
| runtimeArn: 'arn:aws:bedrock-agentcore:us-east-1:123:runtime/r', | ||
| payload: 'hi', | ||
| }; | ||
|
|
||
| describe('SigV4 invoke qualifier', () => { | ||
| beforeEach(() => { | ||
| commandArgs.length = 0; | ||
| mockSend.mockReset(); | ||
| }); | ||
|
|
||
| it('sets qualifier on the non-streaming command when endpoint is provided', async () => { | ||
| mockSend.mockResolvedValue(makeByteResponse('{"result":"ok"}')); | ||
| await invokeAgentRuntime({ ...BASE, endpoint: 'prod' }); | ||
| expect(commandArgs[0]?.qualifier).toBe('prod'); | ||
| }); | ||
|
|
||
| it('omits qualifier on the non-streaming command when no endpoint is provided', async () => { | ||
| mockSend.mockResolvedValue(makeByteResponse('{"result":"ok"}')); | ||
| await invokeAgentRuntime({ ...BASE }); | ||
| expect(commandArgs[0]).not.toHaveProperty('qualifier'); | ||
| }); | ||
|
|
||
| it('sets qualifier on the streaming command when endpoint is provided', async () => { | ||
| mockSend.mockResolvedValue(makeStreamResponse('data: "hi"\n')); | ||
| const { stream } = await invokeAgentRuntimeStreaming({ ...BASE, endpoint: 'staging' }); | ||
| for await (const _ of stream) { | ||
| // drain | ||
| } | ||
| expect(commandArgs[0]?.qualifier).toBe('staging'); | ||
| }); | ||
|
|
||
| it('omits qualifier on the streaming command when no endpoint is provided', async () => { | ||
| mockSend.mockResolvedValue(makeStreamResponse('data: "hi"\n')); | ||
| const { stream } = await invokeAgentRuntimeStreaming({ ...BASE }); | ||
| for await (const _ of stream) { | ||
| // drain | ||
| } | ||
| expect(commandArgs[0]).not.toHaveProperty('qualifier'); | ||
| }); | ||
|
|
||
| it('sets qualifier on the AGUI command when endpoint is provided', async () => { | ||
| mockSend.mockResolvedValue(makeStreamResponse('data: {}\n\n')); | ||
| await invokeAguiRuntime( | ||
| { region: BASE.region, runtimeArn: BASE.runtimeArn, endpoint: 'prod' }, | ||
| { threadId: 't', runId: 'r', messages: [], tools: [], context: [], state: {}, forwardedProps: {} } | ||
| ); | ||
| expect(commandArgs[0]?.qualifier).toBe('prod'); | ||
| }); | ||
|
|
||
| it('omits qualifier on the AGUI command when no endpoint is provided', async () => { | ||
| mockSend.mockResolvedValue(makeStreamResponse('data: {}\n\n')); | ||
| await invokeAguiRuntime( | ||
| { region: BASE.region, runtimeArn: BASE.runtimeArn }, | ||
| { threadId: 't', runId: 'r', messages: [], tools: [], context: [], state: {}, forwardedProps: {} } | ||
| ); | ||
| expect(commandArgs[0]).not.toHaveProperty('qualifier'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,4 +270,52 @@ describe('invoke command', () => { | |
| expect(result.stderr).not.toContain('requires an interactive terminal'); | ||
| }); | ||
| }); | ||
|
|
||
| // -------------------------------------------------------------------------- | ||
| // Endpoint mode routing. | ||
| // | ||
| // The same no-TTY signature lets us prove the env-var fallback no longer | ||
| // silently drops into the TUI (which would invoke the DEFAULT endpoint). A | ||
| // non-DEFAULT resolved endpoint — whether from --endpoint or the | ||
| // AGENTCORE_RUNTIME_ENDPOINT env var — forces CLI mode. | ||
| // -------------------------------------------------------------------------- | ||
| describe('endpoint mode routing', () => { | ||
| it('AGENTCORE_RUNTIME_ENDPOINT (no prompt/flag/json) forces CLI mode instead of silently routing to the TUI', async () => { | ||
| // Regression for #986/#1554: previously the env var alone never forced CLI | ||
| // mode, so the user dropped into the TUI and silently hit DEFAULT. NO --json | ||
| // and NO other CLI flag here on purpose — the resolved non-DEFAULT endpoint | ||
| // (`endpoint !== undefined` in the gate) is the ONLY thing forcing CLI mode, | ||
| // so this exercises the new clause directly. CLI mode reaches the action | ||
| // layer, which resolves the invoke target and fails with "No deployed | ||
| // targets found" (this project has an agent but no deployed state). If the | ||
| // `endpoint !== undefined` clause were removed, this would instead route to | ||
| // the TUI and hit the requireTTY guard ("requires an interactive terminal"). | ||
| const result = await runCLI(['invoke'], projectDir, { | ||
| env: { ...telemetry.env, AGENTCORE_RUNTIME_ENDPOINT: 'staging' }, | ||
| }); | ||
| expect(result.exitCode).toBe(1); | ||
| expect(result.stderr).toContain('No deployed targets found'); | ||
| expect(result.stderr).not.toContain('requires an interactive terminal'); | ||
| telemetry.assertMetricEmitted({ command: 'invoke', endpoint_source: 'env' }); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test claims to verify the regression "AGENTCORE_RUNTIME_ENDPOINT (no prompt/flag) forces CLI mode instead of silently routing to the TUI", but the invocation passes Fix options:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — fixed. Dropped |
||
|
|
||
| it('records endpoint_source=flag when --endpoint is passed', async () => { | ||
| // --endpoint forces CLI mode; the agent has no named endpoint configured so | ||
| // the action layer rejects it — but the telemetry attr is emitted regardless. | ||
| const result = await runCLI(['invoke', 'hi', '--endpoint', 'prod', '--json'], projectDir, { | ||
| env: telemetry.env, | ||
| }); | ||
| expect(result.exitCode).toBe(1); | ||
| const json = JSON.parse(result.stdout); | ||
| expect(json.success).toBe(false); | ||
| expect(result.stderr).not.toContain('requires an interactive terminal'); | ||
| telemetry.assertMetricEmitted({ command: 'invoke', endpoint_source: 'flag' }); | ||
| }); | ||
|
|
||
| it('records endpoint_source=default when neither --endpoint nor the env var is set', async () => { | ||
| const result = await runCLI(['invoke', 'hi', '--json'], projectDir, { env: telemetry.env }); | ||
| expect(result.exitCode).toBe(1); | ||
| telemetry.assertMetricEmitted({ command: 'invoke', endpoint_source: 'default' }); | ||
| }); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: this comment anchors on the qualifier addition site for the non-streaming SigV4 path, but the issue is in invokeAguiRuntime at agentcore.ts:1088.
invokeAguiRuntimebuildsInvokeAgentRuntimeCommandhere without aqualifier, soagentcore invoke --endpoint prodagainst an AGUI-protocol agent will silently route toDEFAULT— the same bug this PR is fixing for SigV4 HTTP invokes. The PR description's "out-of-scope follow-up" only calls out A2A (invokeA2ARuntime) and MCP (mcpRpcCall), but AGUI has the same gap.Options:
...(options.endpoint && { qualifier: options.endpoint })here (and threadendpointthroughAguiInvokeOptions+ the AGUI call site inaction.ts:636), matching what was done forinvokeAgentRuntime/invokeAgentRuntimeStreaming.--endpointwith a clear error for AGUI/A2A/MCP protocols at the validation layer (validate.tsor inaction.tsbefore the protocol branches), so users aren't silently misled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed (option 1).
AguiInvokeOptionsnow has anendpointfield,invokeAguiRuntimesets...(options.endpoint && { qualifier: options.endpoint })on itsInvokeAgentRuntimeCommand, and the resolved endpoint is threaded through both the CLI AGUI call site (action.ts:636) and the TUI AGUI call site (useInvokeFlow.ts). So HTTP, streaming, and AGUI SigV4 invokes all honor --endpoint now. Added qualifier tests for the AGUI path in agentcore-invoke-qualifier.test.ts. The PR-body follow-up note now scopes only A2A (invokeA2ARuntime) and MCP (mcpRpcCall) as remaining work.