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.
centralizes the CLI-side --require-approval framing in CliIoHost.

Changes
- @aws-cdk/toolkit-lib/lib/payloads/deploy.ts: added hasSecurityChanges: boolean to DeployConfirmationRequest so IoHosts can phrase prompts based on whether the change touches IAM/SG.
- @aws-cdk/toolkit-lib/lib/api/io/private/message-maker.ts: added is() type guard to IoRequestMaker (mirroring the existing IoMessageMaker.is()) so consumers can narrow msg.data to the request's payload type.
- @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/io-host/cli-io-host.ts: new augmentDeployApprovalMessage helper. Uses IO.CDK_TOOLKIT_I5060.is(msg) to narrow the request, reads hasSecurityChanges and requireDeployApproval, and returns a complete sentence (not a prefix glued onto the existing motivation):
  - 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/lib/cli/cdk-toolkit.ts: emits the bare-fact motivation and the new hasSecurityChanges field; CliIoHost.augmentDeployApprovalMessage owns the user-facing rendering. Single source of truth for the flag vocabulary.
- @aws-cdk/toolkit-lib/test/actions/deploy.test.ts, aws-cdk/test/cli/cdk-toolkit.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]
```
Integrators who want to suppress the message entirely can implement their own IIoHost and drop the CDK_TOOLKIT_I5060 request:
```
import { Toolkit, NonInteractiveIoHost } from '@aws-cdk/toolkit-lib';

class QuietIoHost extends NonInteractiveIoHost {
  async requestResponse(msg) {
    if (msg.code === 'CDK_TOOLKIT_I5060') return msg.defaultResponse;
    return super.requestResponse(msg);
  }
}

await new Toolkit({ ioHost: new QuietIoHost() }).deploy(source);
```
Test plan
- Unit tests in test/actions/deploy.test.ts (27/27 pass; motivation assertions updated).
- Unit tests in test/cli/io-host/cli-io-host.test.ts and test/cli/cdk-toolkit.test.ts (147/147), including 4 new tests covering each augmentDeployApprovalMessage branch.
- Manually verified all reachable prompt branches against real deploys: cdk deploy --yes and await toolkit.deploy() against broadening (IAM role) and non-security (S3 bucket) stacks under --require-approval=broadening and --require-approval=any-change. Each produces a single, correctly-phrased prompt.
- Manually verified all four NonInteractiveIoHost annotation paths (true, false, string, number) via a probe script: each produces the expected annotation and returns the correct value.
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

mrgrain

Need to look at this in more detail. It was by design that toolkit lib doesn't get interactions.

rix0rrr

While wer'e doing this, can we rename it to require-confirmation ?

"Approval" is actually something else than approval.

mrgrain

Right, I don't think I want to encode this into toolkit-lib. We are already ask for confirmation, it is up to the implementor to decide how to proceed here.

For now, we should improve the message/motivation to simply be something like

Changes detected. Do you wish to deploy these changes?

Additionally, the NonInteractiveIoHost should be clearer about what's happening, e.g. print the auto-acceptance to the output like the --yes flag does.

In future, we should have an easy way to drop a specific message:

const ioHost = new NonInteractiveIoHost();
ioHost.on('I5060', (msg) => {
  // do nothing
});

rix0rrr · 2026-06-02T11:13:04Z

I don't disagree @mrgrain, but then shouldn't we pull this all the way through and require the confirmation always, regardless of the change type? And give enough information for the client to correlate the change impact with the command line flag?

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;

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?`;

…notice-suppression

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

sai-ray commented Jun 2, 2026

View reviewed changes

Comment thread packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts Outdated

mrgrain requested changes Jun 2, 2026

View reviewed changes

rix0rrr requested changes Jun 2, 2026

View reviewed changes

mrgrain requested changes Jun 2, 2026

View reviewed changes

refactor(toolkit-lib): pivot to message + IoHost approach for #828

5e9031f

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 2 commits June 10, 2026 13:00

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

a9204b2

…notice-suppression

chore: midwork

a41d958

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!

Uh oh!

mrgrain left a comment

Choose a reason for hiding this comment

Uh oh!

rix0rrr left a comment

Choose a reason for hiding this comment

Uh oh!

mrgrain left a comment

Choose a reason for hiding this comment

Uh oh!

rix0rrr commented Jun 2, 2026

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

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