Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 16 additions & 2 deletions src/cli/tui/components/DeployStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,13 @@ type ResourceStatus =
| 'UPDATE_FAILED'
| 'DELETE_IN_PROGRESS'
| 'DELETE_COMPLETE'
| 'DELETE_FAILED';
| 'DELETE_FAILED'
| 'ROLLBACK_IN_PROGRESS'
| 'ROLLBACK_COMPLETE'
| 'ROLLBACK_FAILED'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Heads-up: getStatusColor uses suffix matching, so the new ROLLBACK_COMPLETE and UPDATE_ROLLBACK_COMPLETE statuses will render green — same as CREATE_COMPLETE. That's misleading because in CFN those statuses indicate a failed deploy whose rollback finished cleanly, not success.

A few options:

  1. Special-case rollback statuses in getStatusColor so that *ROLLBACK_COMPLETE is yellow/red and *ROLLBACK_IN_PROGRESS is yellow (signalling "recovering from failure").
  2. Check for ROLLBACK in the status name first and color all rollback states yellow/red regardless of suffix.
  3. Leave it as-is if you think the outer ✗ Deploy to AWS Failed banner provides enough context — but note that banner only appears once toolkit-lib throws, which can be well after ROLLBACK_COMPLETE lands on screen.

Option 1 or 2 would give users a clearer signal during the (potentially multi-minute) rollback window that #1610 is specifically about.

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.

Good catch — fixed in 3fdae14. Went with option 2 (the cleanest): getStatusColor now checks for ROLLBACK in the status name first, so all rollback states are colored regardless of suffix:

if (status.includes('ROLLBACK')) return status.endsWith('_FAILED') ? 'red' : 'yellow';
  • ROLLBACK_FAILED / UPDATE_ROLLBACK_FAILED → red (worst case)
  • ROLLBACK_COMPLETE / UPDATE_ROLLBACK_COMPLETE / *ROLLBACK_IN_PROGRESS → yellow ("recovering from failure")

This means ROLLBACK_COMPLETE no longer renders green (the misleading-success case), and the user gets a yellow signal throughout the multi-minute rollback window — exactly the #1610 scenario — instead of waiting for the outer ✗ Deploy to AWS Failed banner.

Added tests under a new status color coding block asserting that CREATE_COMPLETE stays green while ROLLBACK_COMPLETE / UPDATE_ROLLBACK_COMPLETE are NOT green (yellow), in-progress rollbacks are yellow, and ROLLBACK_FAILED is red.

| 'UPDATE_ROLLBACK_IN_PROGRESS'
| 'UPDATE_ROLLBACK_COMPLETE'
| 'UPDATE_ROLLBACK_FAILED';

interface ParsedResource {
resourceType: string;
Expand All @@ -75,8 +81,16 @@ interface ParsedResource {

/**
* Get color for a resource status.
*
* Rollback states are checked first: in CloudFormation a ROLLBACK indicates a
* failed deploy whose changes are being reverted, so even a "clean" finish
* (ROLLBACK_COMPLETE / UPDATE_ROLLBACK_COMPLETE) is a failure, not a success.
* Coloring those green would mislead the user (see #1610). A failed rollback is
* the worst case (red); an in-progress or completed rollback signals
* "recovering from failure" (yellow).
*/
function getStatusColor(status: ResourceStatus): string | undefined {
if (status.includes('ROLLBACK')) return status.endsWith('_FAILED') ? 'red' : 'yellow';
if (status.endsWith('_COMPLETE')) return 'green';

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.

does this mean that UPDATE_ROLLBACK_COMPLETE would also render green? doesn't that mean that the deploy failed?

if (status.endsWith('_FAILED')) return 'red';
if (status.endsWith('_IN_PROGRESS')) return 'cyan';
Expand All @@ -103,7 +117,7 @@ function parseResourceMessage(msg: DeployMessage): ParsedResource | null {
// Format: "StackName | STATUS | AWS::Service::Resource | LogicalId"
const resourceMatch = /(AWS::\S+)/.exec(text);
const statusMatch =
/(CREATE_IN_PROGRESS|CREATE_COMPLETE|CREATE_FAILED|UPDATE_IN_PROGRESS|UPDATE_COMPLETE|UPDATE_FAILED|DELETE_IN_PROGRESS|DELETE_COMPLETE|DELETE_FAILED)/.exec(
/(UPDATE_ROLLBACK_IN_PROGRESS|UPDATE_ROLLBACK_COMPLETE|UPDATE_ROLLBACK_FAILED|ROLLBACK_IN_PROGRESS|ROLLBACK_COMPLETE|ROLLBACK_FAILED|CREATE_IN_PROGRESS|CREATE_COMPLETE|CREATE_FAILED|UPDATE_IN_PROGRESS|UPDATE_COMPLETE|UPDATE_FAILED|DELETE_IN_PROGRESS|DELETE_COMPLETE|DELETE_FAILED)/.exec(
text
);

Expand Down
81 changes: 80 additions & 1 deletion src/cli/tui/components/__tests__/DeployStatus.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,17 @@ import { DeployStatus } from '../DeployStatus.js';
import { render } from 'ink-testing-library';
import React from 'react';
import stripAnsi from 'strip-ansi';
import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';

// Force ink/chalk to emit ANSI color codes so the status color-coding tests are
// deterministic regardless of TTY/CI. vi.hoisted is lifted above the ink import
// by vitest, so FORCE_COLOR is set before ink evaluates its color support.
vi.hoisted(() => {
process.env.FORCE_COLOR = '1';
});

// ink/chalk ANSI foreground color codes.
const ANSI = { green: '\x1b[32m', red: '\x1b[31m', yellow: '\x1b[33m', cyan: '\x1b[36m' } as const;

function makeMsg(
message: string,
Expand Down Expand Up @@ -95,6 +105,21 @@ describe('DeployStatus', () => {
expect(lastFrame()).not.toContain('Some general info');
});

it('renders ROLLBACK statuses instead of dropping the events', () => {
const messages = [
makeResourceMsg('CloudFormation::Stack', 'ROLLBACK_IN_PROGRESS'),
makeResourceMsg('BedrockAgentCore::Gateway', 'UPDATE_ROLLBACK_IN_PROGRESS'),
makeResourceMsg('CloudFormation::Stack', 'ROLLBACK_FAILED'),
];

const { lastFrame } = render(<DeployStatus messages={messages} isComplete={false} hasError={false} />);
const frame = lastFrame()!;

expect(frame).toContain('ROLLBACK_IN_PROGRESS');
expect(frame).toContain('UPDATE_ROLLBACK_IN_PROGRESS');
expect(frame).toContain('ROLLBACK_FAILED');
});

it('shows only last 8 resource events', () => {
const messages = Array.from({ length: 12 }, (_, i) =>
makeResourceMsg(`Service::Resource${i}`, 'CREATE_COMPLETE')
Expand All @@ -112,6 +137,60 @@ describe('DeployStatus', () => {
});
});

describe('status color coding', () => {
// Returns the ANSI color code wrapping the line that contains the status, e.g.
// for "...\x1b[33mService::Resource ROLLBACK_COMPLETE\x1b[39m" -> "\x1b[33m".
// A single resource line is rendered as one colored Text node, so the opening
// code is the last ANSI escape before the status word on that line.
function colorOf(frame: string, status: string): string | undefined {
const line = frame.split('\n').find(l => l.includes(status));
if (!line) return undefined;
const before = line.slice(0, line.indexOf(status));
// eslint-disable-next-line no-control-regex
const codes = before.match(/\x1b\[\d+m/g);
return codes?.[codes.length - 1];
}

it('renders CREATE_COMPLETE green (sanity check)', () => {
const messages = [makeResourceMsg('Lambda::Function', 'CREATE_COMPLETE')];
const { lastFrame } = render(<DeployStatus messages={messages} isComplete={false} hasError={false} />);

expect(colorOf(lastFrame()!, 'CREATE_COMPLETE')).toBe(ANSI.green);
});

it('does NOT render ROLLBACK_COMPLETE green — it is a failed deploy, not a success (#1610)', () => {
const messages = [makeResourceMsg('CloudFormation::Stack', 'ROLLBACK_COMPLETE')];
const { lastFrame } = render(<DeployStatus messages={messages} isComplete={false} hasError={false} />);

const color = colorOf(lastFrame()!, 'ROLLBACK_COMPLETE');
expect(color).not.toBe(ANSI.green);
expect(color).toBe(ANSI.yellow);
});

it('does NOT render UPDATE_ROLLBACK_COMPLETE green', () => {
const messages = [makeResourceMsg('BedrockAgentCore::Gateway', 'UPDATE_ROLLBACK_COMPLETE')];
const { lastFrame } = render(<DeployStatus messages={messages} isComplete={false} hasError={false} />);

const color = colorOf(lastFrame()!, 'UPDATE_ROLLBACK_COMPLETE');
expect(color).not.toBe(ANSI.green);
expect(color).toBe(ANSI.yellow);

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.

shouldn't update rollback complete be red (since it indicates a failed deploy but a successful cleanup)?

});

it('renders rollback in-progress states yellow (recovering from failure)', () => {
const messages = [makeResourceMsg('CloudFormation::Stack', 'ROLLBACK_IN_PROGRESS')];
const { lastFrame } = render(<DeployStatus messages={messages} isComplete={false} hasError={false} />);

expect(colorOf(lastFrame()!, 'ROLLBACK_IN_PROGRESS')).toBe(ANSI.yellow);
});

it('renders ROLLBACK_FAILED red (worst case)', () => {

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.

I would think that any "rollback" which indicates that certain resources were not deployed should be red. because yellow seems more like a warning or just a statement. what do you think?

const messages = [makeResourceMsg('CloudFormation::Stack', 'ROLLBACK_FAILED')];
const { lastFrame } = render(<DeployStatus messages={messages} isComplete={false} hasError={false} />);

expect(colorOf(lastFrame()!, 'ROLLBACK_FAILED')).toBe(ANSI.red);
});
});

describe('progress bar', () => {
it('renders progress bar with completed/total count', () => {
const messages = [makeMsg('deploying', 'CDK_TOOLKIT_I5502', { completed: 3, total: 10 })];
Expand Down
Loading