Uh oh!
There was an error while loading. Please reload this page.
feat: generate Cedar policies from natural language in project add policy - #2127
feat: generate Cedar policies from natural language in project add policy#2127tejaskash wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice, self-contained addition. A few observations, none blocking:
PolicyClientmocks at the AWS SDK boundary viacontrol.sendwithinstanceof-based command dispatch, which matches the existing convention inconfigBundle.test.ts/gateway.test.ts. No excessive mocking.- Telemetry is done centrally in
src/index.tsat the command-run level, so no per-handler instrumentation is needed here. - Pre-flight validation in
handlers/project/add/policy/index.ts(engine exists, no duplicate policy name, gateway resolvable) correctly happens before the ~minute-long generation call, and the write path is only entered aftergeneratePolicysucceeds — so failed generations don't dirty the project spec (verified in the "fails without writing when generation fails" test). - Poll budget is 3s × 40 = 120s with a clear "may still complete" exhaustion message; the loop also handles the Dogwood
definition.policyvariant in addition todefinition.cedar. - The service-name resolution correctly reuses
policyEngineResourceName/gatewayResourceName, keeping it in sync with the L3 constructs.
Minor things I noticed but wouldn't block on:
GetGatewayCommandis called withgatewayIdentifier: deployed.gatewayIdwithout a null check; if the List response ever omittedgatewayIdfor a matched summary, we'd sendundefined. Unlikely in practice.- With
pollDelayMs = 0in tests, the "polling exhausts while still generating" case executes 40 realsendcalls back-to-back; fine, just a note.
LGTM.
4c72bd7 to
ef9f23cComparetejaskash
commented
Aug 27, 2026
Note on the failing |
tejaskash
commented
Aug 27, 2026
Same applies to the failing |
| @@ -7,6 +7,7 @@ import { GatewayClient } from "./gateway"; | |||
| import { HarnessClient } from "./harness"; | |||
There was a problem hiding this comment.
PAUSE FOR STANDUP ASK:
- This is the first time where we have an add project command calling a core client. How should we introduce this? Also, don't you want to confirm the policy before adding to agentcore.json
…PR per review" This reverts commit 529b9fa.
ef9f23c to
20de140Comparecodecov-commenter
commented
Aug 27, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## refactor #2127 +/- ##
=============================================
- Coverage 97.22% 71.23% -26.00%
=============================================
Files 463 115 -348 Lines 28160 10591 -17569 =============================================
- Hits 27378 7544 -19834 - Misses 782 3047 +2265 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Claude Security Review: no high-confidence findings. (run) |
| ); | ||
| } | ||
| const generator = config.policy.generatePolicy( |
There was a problem hiding this comment.
Standup discussion:
- We should just make this imperative command for now. The customers will have to call
agentcore policy generateand then customers will copy and paste the cedar policy toagentcore add policy --cedar policy. It's two way door.
What
Extracts
project add policy --generatefrom #2121 per review, as a stacked PR (base:feat/project-add-policy). This PR re-adds exactly what #2121 dropped:--generate <text>and--gateway <name>onproject add policy: generate the Cedar statement from a natural-language description against the deployed engine and gateway, stream progress, print the generated Cedar and every generation finding, then write the policy into the spec.PolicyClientcore sub-client (src/core/policy.tsx): resolves the deployed engine and gateway by exact service name (via the sharedpolicyEngineResourceName/gatewayResourceNamerules that stay in feat: project add/remove policy-engine and policy with natural-language generation #2121), starts the generation, polls with an injectable delay, surfacesstatusReasonsand findings on failure, accepts bothcedarand Dogwoodpolicydefinition members.CorePolicyClientconsumer-owned interface,TestPolicyClient, and the wiring throughCore/CoreClient/ handler configs.--gateway).Testing
PolicyClienttested with a fake at the SDK.send()boundary (pagination, poll exhaustion,GENERATE_FAILEDreasons, Dogwood members, findings-only assets); handler flows tested through the real root handler withTestPolicyClient.--generate "forbid IAM principals from calling any tool on this gateway" --gateway toolsagainst a deployed stack produced real Cedar with aDENY_ALLfinding printed; the generated policy deployed to ACTIVE and was removed cleanly.Follow-up
deployed-state.json(#2105) can replace the list-and-match resolution with a state-file read; kept out of this PR to keep the extraction a pure move.