feat(toolkit-lib): simplify deploy approval message and surface auto-confirm by sai-ray · Pull Request #1585 · aws/aws-cdk-cli

sai-ray · 2026-06-01T22:13:56Z

Closes #828.

@aws-cdk/toolkit-lib's deploy() previously printed "--require-approval" is enabled and stack includes security-sensitive updates. Do you wish to deploy these changes and then silently auto-confirmed via the default NonInteractiveIoHost. The question-shaped text without a visible answer-shaped action confused programmatic users.

This PR:

keeps the I5060 confirmation request in _deploy (it's the IoHost's job to decide what to do with it).
drops the --require-approval reference from the library's vocabulary.
makes NonInteractiveIoHost's auto-response visible.
moves the CLI-side --require-approval framing into a listener registered by the deploy command.

Changes

@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts: motivation drops the --require-approval reference but keeps the existing security-sensitive vs non-security-sensitive branching:
- Security-sensitive change: Stack includes security-sensitive updates
- Non-security change: Stack includes updates
@aws-cdk/toolkit-lib/lib/toolkit/non-interactive-io-host.ts: requestResponse annotates the message based on the actual default response value:
- defaultResponse: true: (auto-confirmed)
- defaultResponse: false: (auto-denied)
- non-boolean default: (auto-responded with default: <value>)
aws-cdk/lib/cli/cdk-toolkit.ts: deploy() registers a rewrite(IO.CDK_TOOLKIT_I5060, …) listener on CliIoHost that reframes the library's bare-fact question with the --require-approval value, keyed off msg.data.permissionChangeType. Both the CLI's own deploy path and toolkit-lib's deploy action resolve through the same host, so one listener covers both. Registered up front and removed in a finally, matching the cdk destroy reroute. The complete sentences it produces:
- BROADENING + security change: Stack includes security-sensitive updates and "--require-approval" is set to 'broadening'. Do you wish to deploy these changes?
- ANYCHANGE + security change: Stack includes security-sensitive updates and "--require-approval" is set to 'any-change'. Do you wish to deploy these changes?
- ANYCHANGE + non-security change: Stack includes updates and "--require-approval" is set to 'any-change'. Do you wish to deploy these changes?
@aws-cdk/toolkit-lib/test/actions/deploy.test.ts: motivation assertions updated for the new bare-fact text.

Example

Same stack, before vs after this PR:

Before
```
"--require-approval" is enabled and stack includes security-sensitive updates.
Do you wish to deploy these changes
Cdk828Probe: deploying... [1/1]
```
After (cdk deploy, default --require-approval=broadening, broadening change)
```
Stack includes security-sensitive updates and "--require-approval" is set to 'broadening'.
Do you wish to deploy these changes? (y/n)
```
After (cdk deploy --require-approval=any-change, broadening change)
```
Stack includes security-sensitive updates and "--require-approval" is set to 'any-change'.
Do you wish to deploy these changes? (y/n)
```
After (cdk deploy --require-approval=any-change, non-security change)
```
Stack includes updates and "--require-approval" is set to 'any-change'.
Do you wish to deploy these changes? (y/n)
```
After (await toolkit.deploy() programmatic, default NonInteractiveIoHost, security-sensitive change)
```
Stack includes security-sensitive updates. 
Do you wish to deploy these changes? (auto-confirmed)
Cdk828Probe: deploying... [1/1]
```
After (await toolkit.deploy() programmatic, default NonInteractiveIoHost, non-security change)
```
Stack includes updates. 
Do you wish to deploy these changes? (auto-confirmed)
Cdk828Probe: deploying... [1/1]
```
Test plan
- Unit tests in test/actions/deploy.test.ts (motivation assertions updated for the bare-fact text).
- Unit tests in test/cli/io-host/cli-io-host.test.ts: the four wording branches over the rewrite(I5060) listener (broadening/any-change × security/non-security, plus non-I5060 passthrough).
- End-to-end test in test/commands/deploy.test.ts: runs deploy() and asserts the recorded, listener-reframed prompt, plus an IO snapshot.
- Unit tests in test/toolkit/non-interactive-io-host.test.ts: the auto-response annotation for true, false, and non-boolean defaults.
- Verified live against a sandbox account: a real IAM (broadening) diff renders Stack includes security-sensitive updates and "--require-approval" is set to 'broadening'. Do you wish to deploy these changes?
Checklist
- This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed
  - Release notes for the new version:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

github-actions · 2026-06-01T22:14:26Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

sai-ray · 2026-06-02T18:42:58Z

Thanks @mrgrain and @rix0rrr. Dropped the requireApproval option, so _deploy always emits I5060 on any change and the IoHost decides what to do with it. Simplified the motivation to Changes detected. so the --require-approval reference doesn't leak out of the library. And NonInteractiveIoHost now annotates auto-replies with (auto-confirmed) or (auto-responded with default: …), similar to --yes mode.

@rix0rrr on your second point : if I understand correctly, you're saying the I5060 payload should give the IoHost enough to make its own decision. permissionChangeType and templateDiffs are already on the payload, so the IoHost can correlate the change to whatever flag it exposes (that's what CliIoHost.skipApprovalStep uses today). The message itself still branches on security vs stack diff inside _deploy . should I move that out too, or is it fine where it is? Happy to do it here or as a follow-up.

rix0rrr · 2026-06-03T08:56:56Z

-    // in the non-interactive IoHost, no requests are promptable
-    await this.notify(msg);
+    const annotation = typeof msg.defaultResponse === 'boolean'
+      ? '(auto-confirmed)'


You are assuming that all booleans are always true, apparently.

The defaultResponse boolean could have been false, in which case auto-confirmed is not correct. It could have been auto-denied as well, you don't have enough information to tell.

Fixed. (auto-confirmed) for true, (auto-denied) for false, (auto-responded with default: <value>) for non-boolean.

rix0rrr · 2026-06-03T09:00:12Z

-        const deployMotivation = hasSecurityChanges
-          ? '"--require-approval" is enabled and stack includes security-sensitive updates.'
-          : '"--require-approval" is enabled and stack includes updates.';
+        const deployMotivation = 'Changes detected.';


I think I prefer the specificy of the original message. If the problem is that you cannot refer to --require-approval, then remove that part. But now you've removed all specificity.

Here is the question I want you to ask yourself, at all times: "What is better/clearer for the user?"

So, now answer that question: why is it better for the user to get a message like:

Changes detected. Do you wish to deploy these changes?

Put yourself in the shoes of the user. You've just made changes. You're now running cdk deploy to deploy your changes. Then the tool triumphantly tells you "there are changes!". What would you think at that point?

I think the point was: The original message was good and to the point. But the authority that comes up with that message is in the wrong place, because the toolkit-lib has no business talking about command-line arguments.

So:

You need to produce an accurate message here that does not refer to command line arguments.

You then need to produce an even more accurate message in the CLI, that does refer to command line arguments.

Restored the security-vs-non-security branching, just dropped the --require-approval reference. Library now says Stack includes security-sensitive updates. / Stack includes updates..

…d from auto-denied

rix0rrr · 2026-06-04T07:21:44Z

+          ? 'Stack includes security-sensitive updates.'
+          : 'Stack includes updates.';
        const diffOutput = hasSecurityChanges ? securityDiff.formattedDiff : stackDiff.formattedDiff;


Please put the messaging about the flags back in the CliIoHost.

rix0rrr

Almost there!

rix0rrr · 2026-06-05T08:28:24Z

+      return msg;
+    }
+
+    const data = (msg.data ?? {}) as { motivation?: string; permissionChangeType?: string };


Is there not an existing type definition that models the data field here? I feel like we require this.

In fact, there should be a CDK_TOOLKIT_I5060.is(msg) that will enforce a type guard that will automatically make msg.data have the right type.

Yeah, the type already exists DeployConfirmationRequest in payloads/deploy.ts. It's been wired to I5060 via make.confirm<DeployConfirmationRequest>(...) in messages.ts since before this PR. The untyped cast you commented on is gone and is replaced by the suggested type guard.

Added is() on IoRequestMaker in message-maker.ts (it was missing, only IoMessageMaker had it) and hasSecurityChanges: boolean on DeployConfirmationRequest. The augmenter now uses IO.CDK_TOOLKIT_I5060.is(msg) to narrow msg.data and reads hasSecurityChanges to pick the right phrasing. So the prompt branch is driven by a typed payload field instead of re-deriving from permissionChangeType.

rix0rrr · 2026-06-05T08:33:08Z

+    let prefix: string;
+    switch (this.requireDeployApproval) {
+      case RequireApproval.ANYCHANGE:
+        prefix = `"--require-approval" is set to '${RequireApproval.ANYCHANGE}'. `;


Because you want to reuse the original message, and both need to make sense in their own context, the final sentence here will be very staccato:

require-approval is set to anychange. Stack contains updates. Do you want to continue?

Instead, you can include hasSecurityChanges: true|false into the data object and format a complete sentence here, rather than scrapping about with fragments.

You might have noticed already, but I care quite a lot how CDK "feels". Not just: what the CLI says is technically accurate, but also that it speaks in a normal voice to a normal user. No formal IBM-speak, no vague Microsoft-speak.

Done. Added hasSecurityChanges: boolean to the data object, and the augmenter now writes one of three full sentences instead of stitching fragments onto the library's text:

BROADENING + has-security:

Stack includes security-sensitive updates and "--require-approval" is set to 'broadening'. Do you wish to deploy these changes?

ANYCHANGE + has-security:

Stack includes security-sensitive updates and "--require-approval" is set to 'any-change'. Do you wish to deploy these changes?

ANYCHANGE + non-security:

Stack includes updates and "--require-approval" is set to 'any-change'. Do you wish to deploy these changes?

On the voice, this was what I thought as being both natural and not too informal. Any thoughts/ suggestions?

rix0rrr · 2026-06-05T08:33:56Z

+      // The library emits I5060 with a flag-free motivation. The CLI is the
+      // layer that knows about `--require-approval` and adds it to the
+      // user-facing message before any downstream rendering.
+      const promptMessage = this.augmentDeployApprovalMessage(msg).message;


No need for this function to return a full IoRequest object if all we're going to do is pull out a single field from that object and throw the rest away. You can just have this return a string.

…notice-suppression # Conflicts: # packages/aws-cdk/lib/cli/cdk-toolkit.ts

rix0rrr · 2026-06-10T12:54:28Z

+  /**
+   * Add the `--require-approval` flag to the deploy approval prompt.
+   */
+  private augmentDeployApprovalMessage(msg: IoRequest<any, any>): string {


I'm entirely ready to ship this now. HOWEVER, it would be even better if this used the message formatting feature that @mrgrain just formalized here: #1603

So oooooooooone final thing 😉

mrgrain · 2026-06-10T13:24:44Z

+  /**
+   * True when the change includes security-sensitive updates (IAM or security
+   * group changes). IoHosts use this to phrase the prompt accordingly.
+   */
+  readonly hasSecurityChanges: boolean;


this is redundant, permissionChangeType already includes this.

Suggested change

/**

* True when the change includes security-sensitive updates (IAM or security

* group changes). IoHosts use this to phrase the prompt accordingly.

*/

readonly hasSecurityChanges: boolean;

updated and removed hasSecurityChanges

mrgrain · 2026-06-10T13:25:51Z

          motivation: deployMotivation,
          concurrency,
          permissionChangeType: securityDiff.permissionChangeType,
+          hasSecurityChanges,


redundant permissionChangeType: securityDiff.permissionChangeType has the same info

Suggested change

hasSecurityChanges,

mrgrain · 2026-06-10T13:26:18Z

+      return msg.message;
+    }
+
+    const hasSecurityChanges = msg.data.hasSecurityChanges;


check msg.data.permissionChangeType instead

mrgrain · 2026-06-10T13:26:51Z

              motivation,
              concurrency: this.options.concurrency,
              permissionChangeType: securityDiff.permissionChangeType,
+              hasSecurityChanges,


mrgrain · 2026-06-10T13:31:23Z

nit: For consistency with toolkit-lib change : to .?

mrgrain · 2026-06-10T13:35:45Z

+    }
+    if (this.requireDeployApproval === RequireApproval.ANYCHANGE && !hasSecurityChanges) {
+      return 'Stack includes updates and "--require-approval" is set to \'any-change\'.\nDo you wish to deploy these changes?';
+    }


Hmm. I feel this much harder to read than the old system. I had to read three times to see the differences here. Also this if is going to be annoying if we are adding more states.

I suggest you keep this as a templating string:

const updateTypeText = hasSecurityChanges ? "security-sensitive updates" : "updates"; const requireApproval = this.requireDeployApproval; // maybe? not sure. return `Stack includes ${updateTypeText} and "--require-approval" is set to '${requireApproval}'.\nDo you wish to deploy these changes?`;

switched to the suggested template string form. it's now in the rewrite listener in cdk-toolkit.ts

…notice-suppression

…notice-suppression # Conflicts: # packages/@aws-cdk/toolkit-lib/lib/api/io/private/message-maker.ts # packages/@aws-cdk/toolkit-lib/lib/toolkit/non-interactive-io-host.ts # packages/aws-cdk/lib/cli/io-host/cli-io-host.ts

…notice-suppression

sai-ray · 2026-07-02T17:47:30Z

Reworked to use the CliIoHost listener mechanism.

--require-approval framing now lives in a rewrite(IO.CDK_TOOLKIT_I5060, …) listener registered in deploy(). toolkit-lib emits the bare motivation, the CLI adds the flag framing. One listener covers both deploy paths.

Updated this PR now that the request-listener support from #1666 and #1679 is available. This also relies on the cork-replay fix in #1701, which has now merged, to avoid re-applying the rewrite during replay.

sai-ray had a problem deploying to automation June 1, 2026 22:14 — with GitHub Actions Failure

sai-ray temporarily deployed to automation June 1, 2026 22:14 — with GitHub Actions Inactive

github-actions Bot added the p2 label Jun 1, 2026

sai-ray temporarily deployed to no-approval June 1, 2026 22:14 — with GitHub Actions Inactive

aws-cdk-automation requested a review from a team June 1, 2026 22:16

sai-ray temporarily deployed to run-tests June 1, 2026 22:18 — with GitHub Actions Inactive

rix0rrr requested changes Jun 3, 2026

View reviewed changes

fix(toolkit-lib): keep specific motivation, distinguish auto-confirme…

a1f1697

…d from auto-denied

rix0rrr reviewed Jun 4, 2026

View reviewed changes

sai-ray added 4 commits June 4, 2026 14:58

chore: mid work

0eea727

chore: midwork

67cd2b0

chore: midwork

176ee61

chore: midwork

01637af

rix0rrr reviewed Jun 5, 2026

View reviewed changes

sai-ray added 6 commits June 8, 2026 00:37

chore: midwork

a98a20c

chore: midwork

5839bb4

chore: midwork

6ac4963

Merge remote-tracking branch 'origin/main' into sai/require-approval-…

e2f9454

…notice-suppression # Conflicts: # packages/aws-cdk/lib/cli/cdk-toolkit.ts

chore: midwork

8f46813

chore: midwork

052988a

rix0rrr reviewed Jun 10, 2026

View reviewed changes

mrgrain requested changes Jun 10, 2026

View reviewed changes

mrgrain reviewed Jun 10, 2026

View reviewed changes

mrgrain requested changes Jun 10, 2026

View reviewed changes

sai-ray added 5 commits June 10, 2026 13:00

Merge remote-tracking branch 'origin/main' into sai/require-approval-…

a9204b2

…notice-suppression

chore: midwork

a41d958

chore: midwork

6d8e25e

chore: midwork

3ceeb11

sai-ray mentioned this pull request Jul 2, 2026

fix(cli): don't re-run listeners when replaying corked messages #1701

Merged

2 tasks

Merge remote-tracking branch 'origin/main' into sai/require-approval-…

c34ca43

…notice-suppression

Uh oh!

Conversation

sai-ray commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Example

Test plan

Checklist

Uh oh!

github-actions Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

sai-ray commented Jun 2, 2026

Uh oh!

rix0rrr Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rix0rrr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rix0rrr Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgrain Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrgrain Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sai-ray commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sai-ray commented Jun 1, 2026 •

edited

Loading

github-actions Bot commented Jun 1, 2026 •

edited

Loading

rix0rrr Jun 3, 2026 •

edited

Loading

rix0rrr Jun 10, 2026 •

edited

Loading

mrgrain Jun 10, 2026 •

edited

Loading

mrgrain Jun 10, 2026 •

edited

Loading