feat: interactive screen for gateway policy generate - #2173
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice PR. The deep-link fall-through in src/handlers/gateway/policy/generate.tsx mirrors the pattern already established in harness/exec/index.tsx, and the fall-through gate (only when --prompt, --policy-engine-id, and --name are all unspecified and not JSON mode) is a sensible design that keeps the headless path predictable.
A few things I checked and liked:
- Testing seams over mocks.
createGeneratePolicyHandlertakes an injectablerenderGenerateTui, so the deep-link test ingateway.test.tsxverifies the path/core wiring without stubbing I/O. The screen tests drive real React trees throughrenderScreenand the existingTestPolicyClientseam — no filesystem/network mocking added. - Telemetry. Command-path telemetry is emitted automatically by the router (
recordCommandPathinrouter.tsx), so the newgateway policy generatesubroute is already instrumented without extra work. - Cleanup.
aliveRefinGeneratePolicyFormcorrectly guards the async generator loop against state updates after unmount. - Component extraction. Pulling
ErrorPanelout ofHarnessWizard.tsxandEventLogout ofproject/create/screen.tsxintosrc/components/is a clean dedup; no leftover unused imports in the source files.
No blocking issues from me — LGTM to merge.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2173 +/- ##
==========================================
Coverage 97.12% 97.12%
==========================================
Files 535 538 +3
Lines 36844 37035 +191
==========================================
+ Hits 35783 35972 +189
- Misses 1061 1063 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| useInput( | ||
| (input, key) => { | ||
| if (key.escape) { | ||
| navigate(-1); |
There was a problem hiding this comment.
This is a no-op when launched directly through --gateway-id, because the MemoryRouter contains no previous history entry. Please navigate explicitly to /agentcore/gateway/policy/generate.
There was a problem hiding this comment.
Fixed in 9e3946e: esc now navigates to /agentcore/gateway/policy/generate explicitly, verified live from a --gateway-id deep link.
| ); | ||
| let next = await generation.next(); | ||
| while (!next.done) { | ||
| if (!aliveRef.current) return; |
There was a problem hiding this comment.
this prevents state updates but does not cancel or close the generator. The AWS waiter may continue polling after the user leaves. can we abort the operation on Esc/unmount and close the iterator, threading an AbortSignal through generatePolicy?
There was a problem hiding this comment.
Done in 9e3946e: generatePolicy takes an optional AbortSignal, passed to every send and to the waiter's abortSignal. The screen aborts the controller and calls return() on the iterator on esc and on unmount. A screen test asserts the signal is aborted on esc while running, and a Core test asserts an aborted signal rejects with UserCancellationError.
| <Box flexDirection="column"> | ||
| {events.map((message, index) => ( | ||
| <Text key={`${index}-${message}`} color={theme.colors.muted}> | ||
| ✓ {message} |
There was a problem hiding this comment.
A yielded progress step is the currently running step, not a completed one. This can display ✓ Waiting for generation to complete while it is still waiting.
Also, can we reuse the existing TaskList task-state model instead?
There was a problem hiding this comment.
Agreed and done in 9e3946e: the screen now uses TaskList with running/done/failed states, the same model runWithProgress uses. EventLog is no longer promoted, the create screen keeps its private copy untouched.
…ly, render steps with TaskList
|
Claude Security Review: no high-confidence findings. (run) |
What
agentcore gateway policy generate: pick a Gateway, see its attached Policy Engine, type a prompt, watch the generation steps, read the Cedar and findings.eedits the prompt and runs again.policyappears in theagentcore gatewaymenu. A baregateway policy generateopens the picker,--gateway-id Xwithout--promptopens the form on that Gateway.--jsonand any run with--promptstay headless.ErrorPanelandEventLogpromoted from private copies tosrc/componentsand reused.Testing
bun test src: 2794 pass. Four screen tests throughrenderScreen(picker to form, no engine, happy path with exact Core input and rendered Cedar plus theeretry, error phase back to the form), the gateway menu test listspolicy, one deep-link handler test with an injected renderer.DENY_ALLfinding,eretry, esc while running, deep link, headless--jsonstill errors on the missing prompt. Screenshot below.