feat: add invoke functionality to web-ui#1326
Conversation
Package TarballHow to installgh release download pr-1326-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz |
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Nice refactor extracting resolveInvokeTarget into a reusable module — the unit tests in resolve.test.ts are well-structured and only mock at the I/O boundary (token fetch). The new web-UI deployed-invocation path is a useful addition, but I have a few concerns that should be addressed before merging.
Summary of issues:
- Mid-stream errors in the new
handleDeployed*Invocationhelpers leave the client hanging (headers sent, but nores.end()and no error frame). - ~250 lines of new handler code in
invocations.ts(the entire deployed-invocation path) has zero test coverage. handleDeployedInvocationdoesn't accepttargetName, so multi-target projects can only invoke against the first deployed target.
Details inline.
| }) | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Mid-stream errors are silently dropped.
The outer try/catch here only writes an error response when !res.headersSent. But the helpers (handleDeployedHttpInvocation, handleDeployedA2AInvocation, handleDeployedAguiInvocation) all call res.writeHead(200, ...) before iterating result.stream/result.textStream. So if the AWS SDK stream throws partway (transient network error, throttling, expired token mid-call, etc.), control returns here with res.headersSent === true, the catch becomes a no-op, res.end() is never called, and the client hangs until socket timeout.
Options:
- In each helper, wrap the
for awaitin atry/finallythat ensuresres.end()runs, and (incatch) write an SSE error frame likedata: {"error":"..."}\n\nbefore ending so the frontend can surface it. - Or move the
try/catchinto each helper so it has access tores.headersSentand the stream, and emit a structured SSE error event before ending.
The local-agent paths above (handleA2AInvocation, handleAguiInvocation) have a similar gap, but they at least rely on Node's HTTP proxy which forwards transport errors via proxyReq.on('error', ...). The new deployed paths don't have any equivalent safety net.
| agentName, | ||
| sessionId, | ||
| configIO, | ||
| }); |
There was a problem hiding this comment.
targetName is never plumbed through.
The request body is parsed for agentName/prompt/sessionId/userId but not targetName, and resolveInvokeTarget is called without it. For projects with multiple deployment targets, the web UI silently invokes against Object.keys(deployedState.targets)[0] with no way for the user to choose. Either accept a targetName field in the body and forward it here, or (if the frontend doesn't expose target selection yet) at least add a TODO comment so this isn't lost.
| res.write(`data: ${JSON.stringify(chunk)}\n\n`); | ||
| } | ||
| res.end(); | ||
| } |
There was a problem hiding this comment.
No test coverage for the deployed-invocation path.
The entire handleDeployedInvocation flow (~250 LOC, including three streaming helpers and a non-trivial config-loading + resolution + protocol-dispatch chain) has no unit or integration tests. Other handlers in this directory have __tests__/<handler>.test.ts files (memory.test.ts, cloudwatch-traces.test.ts); please add equivalent coverage for at least:
- the early validation paths (no
configRoot, invalid JSON, missingprompt) - successful HTTP dispatch with the AWS SDK calls mocked
- the protocol routing for A2A / AGUI / HTTP
- the resolve-error path (
resolved.success === false) returning 400 with the underlying error message
The AWS SDK invoke functions (invokeAgentRuntimeStreaming, invokeA2ARuntime, invokeAguiRuntime) are real I/O boundaries and the right place to mock.
Description
Adds invoke functionality to the web-ui dev server, allowing users to invoke deployed agents directly from the agent inspector.
Key changes:
handleInvokeinto a new reusableresolveInvokeTarget()function (src/cli/commands/invoke/resolve.ts)?target=deployedquery param support toPOST /invocationsin the web-ui server, routing to deployed runtimes via the AWS SDKType of Change
Testing
npm run test:unitandnpm run test:integnpm run typechecknpm run lintChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.