feat: gateway policy generate - #2171
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2171 +/- ##
============================================
+ Coverage 97.16% 97.18% +0.01%
============================================
Files 519 522 +3
Lines 35506 35679 +173
============================================
+ Hits 34500 34673 +173
Misses 1006 1006 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
I looked over the diff and pulled the branch locally. The change is well-scoped and reads cleanly:
PolicyClient.generatePolicymocks only at the AWS SDKsend()boundary, so the SDK-provided waiter (waitForPolicyGenerationCompleted) runs its real polling loop against fake responses. Tests exercise the happy path (both engine-inference paths, paginated assets, bothcedarandpolicydefinition members, findings mapping) plus the notable failure modes (no engine attached, no gateway ARN, no generation id,GENERATE_FAILEDreasons, non-translatable prompt, empty assets, waiter timeout) withtest.each. No excessive mocking.- Handler tests use the real
createRootHandler+TestCoreClient/TestPolicyClient, and assert exact stdout/stderr for the human path,--jsonoutput,--jsonerror rendering, and missing-flag rejection before the client is called. - Error taxonomy is sensible (
InputValidationErrorfor missing engine,MalformedServiceResponseErrorfor missing service fields,NetworkingErrorfor the waiter timeout withstatusReasonssurfaced onGENERATE_FAILED). - The sub-router intentionally omits
renderTui/supportedTuiCommandsso a bareagentcore gateway policyfails on the missing flag rather than dropping into the TUI, which matches the PR description and gives scripted callers a predictable failure. - Coupling of
src/core/policy.tsxonsrc/handlers/gateway/policy/typesis consistent with every other Core sub-client in this repo. - No missing telemetry: this repo emits
cli.command_runat the root insrc/index.tsand no handler subtree adds per-command instrumentation, so nothing new is expected here.
Minor observations, not blocking:
formatFindingsinsrc/handlers/gateway/policy/format.tsnumbers findings by the raw asset index (including untranslated fragments that are omitted from stdout), so a user cross-referencingpolicy 2 [INVALID]on stderr with the two Cedar statements on stdout can't always line them up.--jsongives the exact per-policy findings, so this is only a plain-text ergonomics thing.resourceIdFromArninsrc/core/policy.tsxaccepts anything starting witharn:and returns the trailing segment; a malformed ARN likearn:foowould round-trip toarn:foo. The service will still reject it, so no correctness issue.
Nothing here requires changes before merging.
jariy17
left a comment
There was a problem hiding this comment.
Good just fix the tests.
| @@ -0,0 +1,184 @@ | |||
| import { describe, expect, test } from "bun:test"; | |||
There was a problem hiding this comment.
Golden Tests only please. If there is a test case that can't use Golden Tests like Throttling Error Handling, throw it into gateway.test.tsx.
There was a problem hiding this comment.
Done. The Core tests are now golden fixtures recorded against a deployed project in gateway.policy.test.tsx. The two waiter outcomes that cannot be recorded (timeout, GENERATE_FAILED) live in gateway.test.tsx with the control client faked at .send().
|
|
||
| const DEFAULT_WAIT: PolicyGenerationWait = { maxWaitTime: 60, minDelay: 2, maxDelay: 5 }; | ||
|
|
||
| function resourceIdFromArn(value: string): string { |
There was a problem hiding this comment.
nit: OOS but there must be a shared util function for this.
| private readonly wait: PolicyGenerationWait = DEFAULT_WAIT, | ||
| ) {} | ||
|
|
||
| async *generatePolicy( |
There was a problem hiding this comment.
OOS but a lot of our coreClient function's don't yield helpful messages like in here. We should work on this later.
|
|
||
| yield { type: "step", message: `Resolving gateway ${gatewayId}` }; | ||
| const gateway = await control.send(new GetGatewayCommand({ gatewayIdentifier: gatewayId })); | ||
| if (!gateway.gatewayArn) { |
There was a problem hiding this comment.
It's not possible for GetGateway to return no gatewayArn. Do you know why ur agent is being this overprotective. It's probably because aws-sdk-js make every response parameter optional.
There was a problem hiding this comment.
Agreed and removed, along with the same guard on policyGenerationId. Both now use a non-null assertion like the rest of Core. Added a CLAUDE.md guideline so this pattern does not come back.
…move waiter cases to gateway.test.tsx
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
| control plane is faked at .send() while the real PolicyClient and waiter run. | ||
| **/ | ||
| describe("gateway policy generate against a faked control plane", () => { | ||
| function coreWith(status: string, statusReasons?: string[]): Core { |
There was a problem hiding this comment.
Follow Up: Can't you use the settle() function in fixtures.tsx? Settle() will perform a wait while recording the Golden tests.
jariy17
left a comment
There was a problem hiding this comment.
LGTM, just a follow up on the tests.
What
agentcore gateway policy generate --gateway-id <id|arn> --prompt <text>which generates a Cedar policy for a deployed Gateway from a natural-language prompt and prints only the Cedar to stdout, ready to pipe intoproject add policy --statement -.--policy-engine-iddefaults to the Gateway's attached engine,--namenames the generation request,--jsonprints the structured result including the service findings.PolicyClientCore sub-client (core.policy) behind a consumer-ownedCorePolicyClientinterface.project add policy --generateapproach from feat: generate Cedar policies from natural language in project add policy #2127 per review.Testing
src/handlers/gateway/gateway.policy.test.tsxrecorded against a deployed project (human output,--jsonby ARN with an explicit engine, no engine attached, nonexistent engine, untranslatable prompt). Waiter timeout andGENERATE_FAILEDare faked at.send()ingateway.test.tsxalongside the flag validation rows.project add policy, redeployed, confirmed the policyACTIVE, tore down.Screen.Recording.2026-09-02.at.4.12.52.PM.mov