fix: defer policy engine write and harden policy flow UX#856
fix: defer policy engine write and harden policy flow UX#856jesseturner21 merged 3 commits intomainfrom
Conversation
Previously, pressing Escape on the gateway selection screen during policy engine creation would skip to the success screen because the engine was already written to agentcore.json at the name step. Now the disk write is deferred until the user completes the entire flow, so Escape correctly navigates back to the previous step without persisting a half-configured engine. Constraint: Must not break non-interactive CLI path which still writes immediately via primitive Rejected: Only change Escape to go back without deferring write | engine would still be persisted on back Confidence: high Scope-risk: narrow
Package TarballHow to installnpm install https://github.com/aws/agentcore-cli/releases/download/pr-856-tarball/aws-agentcore-0.8.0.tgz |
Coverage Report
|
There was a problem hiding this comment.
ISSUE BELOW FIXED
Used Option B
Issue: Escape → back to engine-wizard loses the user's previously entered name
Severity: Medium — UX regression
When the user presses Escape on the gateway selection screen, the flow transitions to { name: 'engine-wizard' }, which remounts AddPolicyEngineScreen. That component generates a fresh initialValue via generateUniqueName('MyPolicyEngine', existingEngineNames) — the name the user previously typed and submitted is discarded.
Scenario:
- User types engine name
ProductionGuard→ submits - Gateway selection screen appears (unprotected gateways exist)
- User presses Escape to go back
- Engine name screen reappears with
MyPolicyEngine(or similar generated name) instead ofProductionGuard
Options to address:
Option A — Carry the name in flow state: Add an optional engineName field to the engine-wizard flow state variant. When navigating back from attach-gateways, pass flow.engineName into it. Then thread that through to AddPolicyEngineScreen as an initialValue override. This is the cleanest fix — it's one new field on the state union and a prop on the child component.
// Flow state addition:
| { name: 'engine-wizard'; prefillName?: string }
// Back handler:
onBack={() => setFlow({ name: 'engine-wizard', prefillName: flow.engineName })}
// In AddPolicyEngineScreen rendering:
initialValue={flow.prefillName ?? generateUniqueName('MyPolicyEngine', existingEngineNames)}Option B — Store the pending name in component state: Add a pendingEngineName state variable alongside the existing engineNames state. Set it when handleEngineComplete fires, clear it on success. Use it as the initial value when rendering engine-wizard. Slightly more state to manage but doesn't require changing the flow union type.
Option A is simpler and keeps the state local to the navigation path.
When pressing Escape on the gateway screen to go back to the name step, the previously entered engine name was lost because AddPolicyEngineScreen remounted with a generated default. Now the entered name is stored in pendingEngineName state and passed back as initialName so the user sees their original input. Constraint: Must not change flow state union type to keep diff minimal Rejected: Carry name in FlowState union variant | adds complexity to type for one field Confidence: high Scope-risk: narrow
This test requires a live terminal session and cannot run as a unit test in CI. It was an untracked local file that got staged by mistake.
Problem
Two issues in the policy engine TUI flow:
Escape skipped to success instead of going back — When adding a policy engine through the interactive TUI (
agentcore→ add → policy engine), pressing Escape on the "Attach to gateways" screen would skip forward to the success screen instead of going back. The engine was written toagentcore.jsonimmediately at the name step, so the user couldn't back out without leaving a half-configured engine on disk.Policy generation failures crashed the TUI — Unhandled rejections from the
generate()call hit the globalprocess.exit(1)handler inindex.ts, causing the TUI to exit immediately. The error screen also had no activeuseInputhandler, so Escape did nothing despite the hint text.Fix
Commit 1: Defer engine write until flow completes
policyEnginePrimitive.add()is no longer called at the name step. A newcommitEnginehelper writes the engine (and optionally attaches gateways) only when the user reaches the final confirmation.Commit 2: Preserve engine name on back navigation
pendingEngineNamestate, instead of being replaced with a generated default.Commit 3: Prevent TUI crash on policy generation failure
.catch()safety net ongenerate()to prevent unhandled rejections from reaching the globalprocess.exit(1)handleruseInputhandler for Escape on the error screen so users can go back and retryExecLoggerto write structured logs to.cli/logs/policy-generation/Test plan
agentcore.jsonunchangedagentcore add policy-engine --name X) still works