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, and centralizes the CLI-side --require-approval framing inCliIoHost.

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/io-host/cli-io-host.ts: new augmentDeployApprovalMessage helper. When CliIoHost receives an I5060, it reinjects the --require-approval framing based on requireDeployApproval:
- BROADENING + broadening change → prepends "--require-approval" is enabled.
- ANYCHANGE → prepends "--require-approval" is set to 'any-change'.
aws-cdk/lib/cli/cdk-toolkit.ts: emits the same bare-fact motivation as toolkit-lib; CliIoHost.augmentDeployApprovalMessage adds the framing. 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)

"--require-approval" is enabled. Stack includes security-sensitive updates: Do you wish to deploy these changes? (y/n)

After (cdk deploy --require-approval=any-change, broadening change)

"--require-approval" is set to 'any-change'. Stack includes security-sensitive updates: 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 pass with updated assertion.
Manually verified all four 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-prefixed 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

codecov-commenter · 2026-06-02T02:43:49Z

Codecov Report

❌ Patch coverage is 54.90196% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.06%. Comparing base (1bdaaaf) to head (01637af).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/aws-cdk/lib/cli/io-host/cli-io-host.ts	53.19%	21 Missing and 1 partial ⚠️
packages/aws-cdk/lib/cli/cdk-toolkit.ts	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1585      +/-   ##
==========================================
- Coverage   88.16%   88.06%   -0.11%     
==========================================
  Files          76       76              
  Lines       10841    10909      +68     
  Branches     1494     1511      +17     
==========================================
+ Hits         9558     9607      +49     
- Misses       1255     1274      +19     
  Partials       28       28

Flag	Coverage Δ
suite.unit	`88.06% <54.90%> (-0.11%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

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.

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.

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 temporarily deployed to run-tests June 2, 2026 02:39 — with GitHub Actions Inactive

github-actions Bot added the bug label Jun 2, 2026

sai-ray added 2 commits June 1, 2026 22:44

feat(toolkit-lib): add requireApproval option to deploy

90ff4a1

test(toolkit-lib): cover requireApproval gate

41c4504

sai-ray force-pushed the sai/require-approval-notice-suppression branch from d1786c1 to 41c4504 Compare June 2, 2026 02:44

sai-ray marked this pull request as ready for review June 2, 2026 02:44

sai-ray temporarily deployed to no-approval June 2, 2026 02:44 — with GitHub Actions Inactive

sai-ray temporarily deployed to run-tests June 2, 2026 02:48 — 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

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!

codecov-commenter commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

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

codecov-commenter commented Jun 2, 2026 •

edited

Loading

rix0rrr Jun 3, 2026 •

edited

Loading