Skip to content

feat(toolkit-lib): simplify deploy approval message and surface auto-confirm #1585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sai-ray wants to merge 20 commits into
base: main
Choose a base branch
from
Open

feat(toolkit-lib): simplify deploy approval message and surface auto-confirm #1585

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
90ff4a1
feat(toolkit-lib): add requireApproval option to deploy
sai-ray Jun 1, 2026
41c4504
test(toolkit-lib): cover requireApproval gate
sai-ray Jun 2, 2026
5e9031f
refactor(toolkit-lib): pivot to message + IoHost approach for #828
sai-ray Jun 2, 2026
a1f1697
fix(toolkit-lib): keep specific motivation, distinguish auto-confirme…
sai-ray Jun 4, 2026
0eea727
chore: mid work
sai-ray Jun 4, 2026
67cd2b0
chore: midwork
sai-ray Jun 4, 2026
176ee61
chore: midwork
sai-ray Jun 5, 2026
01637af
chore: midwork
sai-ray Jun 5, 2026
a98a20c
chore: midwork
sai-ray Jun 8, 2026
5839bb4
chore: midwork
sai-ray Jun 8, 2026
6ac4963
chore: midwork
sai-ray Jun 8, 2026
e2f9454
Merge remote-tracking branch 'origin/main' into sai/require-approval-…
sai-ray Jun 8, 2026
8f46813
chore: midwork
sai-ray Jun 8, 2026
052988a
chore: midwork
sai-ray Jun 8, 2026
a9204b2
Merge remote-tracking branch 'origin/main' into sai/require-approval-…
sai-ray Jun 10, 2026
a41d958
chore: midwork
sai-ray Jun 11, 2026
cad6741
Merge remote-tracking branch 'origin/main' into sai/require-approval-…
sai-ray Jul 1, 2026
6d8e25e
chore: midwork
sai-ray Jul 2, 2026
3ceeb11
chore: midwork
sai-ray Jul 2, 2026
c34ca43
Merge remote-tracking branch 'origin/main' into sai/require-approval-…
sai-ray Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions packages/@aws-cdk/toolkit-lib/lib/toolkit/non-interactive-io-host.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as util from 'node:util';
import * as chalk from 'chalk';
import type { IIoHost, IoMessage, IoMessageLevel, IoRequest } from '../api/io';
import { isMessageRelevantForLevel } from '../api/io/private';
Expand Down Expand Up @@ -120,12 +121,20 @@ export class NonInteractiveIoHost implements IIoHost {
/**
* Notifies the host of a message that requires a response.
*
* If the host does not return a response the suggested
* default response from the input message will be used.
* The non-interactive IoHost cannot prompt the user, so it always returns
* the request's `defaultResponse`. To make this visible, the message is
* annotated to indicate the auto-response — same convention as the CLI's
* `--yes` mode.
*/
public async requestResponse<DataType, ResponseType>(msg: IoRequest<DataType, ResponseType>): Promise<ResponseType> {
// in the non-interactive IoHost, no requests are promptable
await this.notify(msg);
const annotation = typeof msg.defaultResponse === 'boolean'
? (msg.defaultResponse ? '(auto-confirmed)' : '(auto-denied)')
: `(auto-responded with default: ${util.format(msg.defaultResponse)})`;

await this.notify({
...msg,
message: `${msg.message} ${annotation}`,
});
return msg.defaultResponse;
}

Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,14 +858,14 @@ export class Toolkit extends CloudAssemblySourceBuilder {
const stackDiff = formatter.formatStackDiff();

// Send a request response with the diff as part of the message,
// and the template diff as data
// (IoHost decides whether to print depending on permissionChangeType)
// and the template diff as data. The IoHost decides how to handle the
// request — interactively prompt the user, auto-confirm, or suppress.
const hasSecurityChanges = securityDiff.permissionChangeType !== PermissionChangeType.NONE;
const deployMotivation = hasSecurityChanges
? '"--require-approval" is enabled and stack includes security-sensitive updates.'
: '"--require-approval" is enabled and stack includes updates.';
? 'Stack includes security-sensitive updates'
: 'Stack includes updates';
const diffOutput = hasSecurityChanges ? securityDiff.formattedDiff : stackDiff.formattedDiff;
const deployQuestion = `${diffOutput}\n\n${deployMotivation}\nDo you wish to deploy these changes`;
const deployQuestion = `${diffOutput}\n\n${deployMotivation}. Do you wish to deploy these changes?`;
const deployConfirmed = await ioHelper.requestResponse(IO.CDK_TOOLKIT_I5060.req(deployQuestion, {
motivation: deployMotivation,
concurrency,
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ IAM Statement Changes
action: 'deploy',
level: 'info',
code: 'CDK_TOOLKIT_I5060',
message: expect.stringContaining('Do you wish to deploy these changes'),
message: expect.stringContaining('Do you wish to deploy these changes?'),
data: expect.objectContaining({
motivation: expect.stringContaining('stack includes security-sensitive updates.'),
motivation: 'Stack includes security-sensitive updates',
permissionChangeType: 'broadening',
templateDiffs: expect.objectContaining({
Stack1: expect.objectContaining({
Expand Down Expand Up @@ -114,12 +114,12 @@ IAM Statement Changes

// Message includes formatted stack diff (not just security diff)
expect(request).toContain('AWS::S3::Bucket');
expect(request).toContain('Do you wish to deploy these changes');
expect(request).toContain('Do you wish to deploy these changes?');

expect(ioHost.requestSpy).toHaveBeenCalledWith(expect.objectContaining({
code: 'CDK_TOOLKIT_I5060',
data: expect.objectContaining({
motivation: expect.stringContaining('stack includes updates.'),
motivation: 'Stack includes updates',
permissionChangeType: 'none',
}),
}));
Expand Down
6 changes: 4 additions & 2 deletions packages/aws-cdk/lib/cli/cdk-toolkit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,9 +608,11 @@ export class CdkToolkit {
const securityDiff = formatter.formatSecurityDiff();
if (requiresApproval(requireApproval, securityDiff.permissionChangeType)) {
const hasSecurityChanges = securityDiff.permissionChangeType !== PermissionChangeType.NONE;
// Bare-fact motivation. `CliIoHost.augmentDeployApprovalMessage`
// adds the `--require-approval` framing for terminal users.
const motivation = hasSecurityChanges
? '"--require-approval" is enabled and stack includes security-sensitive updates'
: `"--require-approval" is set to '${RequireApproval.ANYCHANGE}'`;
? 'Stack includes security-sensitive updates'
: 'Stack includes updates';
const diffOutput = hasSecurityChanges ? securityDiff.formattedDiff : formatter.formatStackDiff().formattedDiff;
await this.ioHost.asIoHelper().defaults.info(diffOutput);

Expand Down
51 changes: 47 additions & 4 deletions packages/aws-cdk/lib/cli/io-host/cli-io-host.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,44 @@ export class CliIoHost implements IIoHost {
}
}

/**
* Augment toolkit-lib's flag-free I5060 motivation with the
* `--require-approval` framing so CLI users see flag context.
*/
private augmentDeployApprovalMessage<DataType, ResponseType>(msg: IoRequest<DataType, ResponseType>): IoRequest<DataType, ResponseType> {
const approvalToolkitCodes = ['CDK_TOOLKIT_I5060'];
if (!(msg.code && approvalToolkitCodes.includes(msg.code))) {
return msg;
}

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

@rix0rrr rix0rrr Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sai-ray sai-ray Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const baseMotivation = data.motivation;
if (!baseMotivation) {
return msg;
}

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

@rix0rrr rix0rrr Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sai-ray sai-ray Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

break;
case RequireApproval.BROADENING:
if (data.permissionChangeType !== 'broadening') {
return msg;
}
prefix = '"--require-approval" is enabled. ';
break;
default:
return msg;
}

@mrgrain mrgrain Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@sai-ray sai-ray Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


const augmentedMotivation = prefix + baseMotivation;
return {
...msg,
message: msg.message.replace(baseMotivation, augmentedMotivation),
};
}

/**
* Determines the output stream, based on message and configuration.
*/
Expand Down Expand Up @@ -464,13 +502,18 @@ export class CliIoHost implements IIoHost {
return true;
}

// 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;

@rix0rrr rix0rrr Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// In --yes mode, respond for the user if we can
if (this.autoRespond) {
// respond with yes to all confirmations
if (isConfirmationPrompt(msg)) {
await this.notify({
...msg,
message: `${chalk.cyan(msg.message)} (auto-confirmed)`,
message: `${chalk.cyan(promptMessage)} (auto-confirmed)`,
});
return true;
}
Expand All @@ -479,7 +522,7 @@ export class CliIoHost implements IIoHost {
if (msg.defaultResponse) {
await this.notify({
...msg,
message: `${chalk.cyan(msg.message)} (auto-responded with default: ${util.format(msg.defaultResponse)})`,
message: `${chalk.cyan(promptMessage)} (auto-responded with default: ${util.format(msg.defaultResponse)})`,
});
return msg.defaultResponse;
}
Expand All @@ -498,7 +541,7 @@ export class CliIoHost implements IIoHost {
// Basic confirmation prompt
// We treat all requests with a boolean response as confirmation prompts
if (isConfirmationPrompt(msg)) {
const confirmed = await promptly.confirm(`${chalk.cyan(msg.message)} (y/n)`);
const confirmed = await promptly.confirm(`${chalk.cyan(promptMessage)} (y/n)`);
if (!confirmed) {
throw new ToolkitError('AbortedByUser', 'Aborted by user');
}
Expand All @@ -508,7 +551,7 @@ export class CliIoHost implements IIoHost {
// Asking for a specific value
const prompt = extractPromptInfo(msg);
const desc = responseDescription ?? prompt.default;
const answer = await promptly.prompt(`${chalk.cyan(msg.message)}${desc ? ` (${desc})` : ''}`, {
const answer = await promptly.prompt(`${chalk.cyan(promptMessage)}${desc ? ` (${desc})` : ''}`, {
default: prompt.default,
trim: true,
});
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/test/cli/cdk-toolkit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ describe('deploy', () => {
// The confirmation prompt should use the non-security motivation and report no permission changes
expect(requestSpy).toHaveBeenCalledWith(expect.objectContaining({
code: 'CDK_TOOLKIT_I5060',
message: expect.stringContaining("'any-change'"),
message: 'Stack includes updates: Do you wish to deploy these changes?',
data: expect.objectContaining({
permissionChangeType: 'none',
}),
Expand Down
Loading