fix(toolkit-lib): stale DescribeStacks read fails fresh stack creates, masked as NoStack

[svozza]

Fixes #1802

Two independent bugs that combine to fail fresh stack creates under CloudFormation read pressure with NoStack: CloudFormationStack object does not hold a stack. One commit each.

1. A stale DescribeStacks read failed a running deployment

DescribeStacks is eventually consistent, so a poll issued after ExecuteChangeSet can still report the pre-execution REVIEW_IN_PROGRESS status. StackStatus.isInProgress excludes review, so that read fell into the isReviewInProgress carve-out in stabilizeStack, which returned the stack as stable; waitForStackDeploy then rejected the deployment with StackDeployFailed while CloudFormation went on to reach CREATE_COMPLETE.

A stack cannot transition from an in-progress state back to REVIEW_IN_PROGRESS, so that status is a stale read whenever it is reported for a stack whose execution we know has been issued. stabilizeStack now keeps polling in that case.

Two details worth calling out:

• Identified by stack id, not by a flag. Polling by name can observe a different stack that a concurrent operation created under the same name (stack A: UPDATE_IN_PROGRESS → deleted → stack B same name: REVIEW_IN_PROGRESS). That review status is genuine, and treating it as stale would wait forever, since waitFor has no timeout. Comparing stack ids keeps the two cases apart.
• monitorDeployment passes the executing stack id in. Nothing guarantees the first DescribeStacks after execution observes the new status, so recognising a stale read cannot depend on having seen the operation in progress first.

Stale reads are tolerated in bounded number (STALE_REVIEW_READ_TOLERANCE), so a stack genuinely left in REVIEW_IN_PROGRESS still terminates the wait and reaches the pre-execution behaviour the carve-out was written for — nothing moves an unexecuted ChangeSet on its own.

2. NoStack masked the real error on any failed create

monitorDeployment passed finalState.wrapped to the diagnoser from inside its catch block. finalState is still the pre-deploy lookup there, which holds no stack when creating one from scratch, so the getter threw NoStack — and because that happened while evaluating an argument, it replaced the deployment error being reported.

This affected every failed deployment of a new stack, not only those caused by bug 1: a resource failure that rolled the stack back reported NoStack instead of naming the resource.

It now describes the stack that was actually deployed. The pre-deploy lookup is the wrong input even when it does hold a stack, since it describes a state the deployment has since left and the diagnoser reads the status off it. Diagnosing is best-effort, so a failed lookup leaves the original deployment error to propagate rather than replacing it with an ErrorDiagnosisFailed that says less.

Testing

Unit tests only; no new AWS resource types or cross-service interactions, so no integ test.

• cfn-api-stabilization.test.ts (new) — stale read mid-wait, stale read on the first poll, a different stack id treated as genuine, persistent review terminating rather than hanging, and the abandoned-ChangeSet escape hatch still failing fast.
• deploy-stack-error-surfacing.test.ts (new) — failing create via change-set and direct, with and without rollback, plus a failing update of an existing stack to guard against a fix that only works when the stack is missing.

Every new test was confirmed to fail against the unfixed code and pass after. Full toolkit-lib suite passes (1896 tests).

deploy-stack-polling-interval.test.ts needed one update: it asserts waitForStackDeploy's exact argument list, which the new parameter changes.

How this was found

The e2e CI for Powertools for AWS Lambda (TypeScript) — ~40 parallel jobs deploying small stacks into one account/region — failed ~15% of matrix cells per run. CloudTrail across three failing stacks showed every DescribeStacks returning 200 with no errorCode (a genuinely absent stack returns ValidationError) under heavy ThrottlingException on CFN reads, consistent with a stale replica read.

Both fixes were validated there as a load-time monkey-patch before being written properly here — patchWaitForStackDeploy retried stabilization on the spurious error (bug 1), and patchWrappedGetter made wrapped non-throwing (bug 2).

Results: the matrix went 40/40 green against 6/40 failing on baseline, and in a later run 13 genuine fresh-create failures all surfaced their real DeploymentErrors through the path that previously produced NoStack.

The patch is a workaround rather than a model for this PR — it retries at the waitForStackDeploy boundary rather than fixing the stale-read classification inside stabilizeStack, and making wrapped return {} hides a real invariant instead of not violating it. What it does establish is that the two behaviours being changed here are the ones responsible for the failures.

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

[svozza]

Flagging a design choice for maintainer input.

This adds a fifth positional parameter, so waitForStackDeploy now reads (cfn, ioHelper, stackName, stabilizationPollingInterval, executingStackId), and stabilizeStack below takes the same shape. Two optional undefined-able tail parameters of different types are easy to transpose at a call site.

An options object would read better and scale if more parameters get added. I kept positional parameters to keep the diff minimal, and because each function has a single caller today — but I'm happy to switch in this PR if you'd prefer. Neither function is exported from lib/index.ts, so there's no API Extractor impact either way.

Happy to go whichever way you'd rather.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.29%. Comparing base (536ad69) to head (42effda).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1803      +/-   ##
==========================================
- Coverage   90.32%   90.29%   -0.04%     
==========================================
  Files          80       80              
  Lines       12124    12124              
  Branches     1716     1714       -2     
==========================================
- Hits        10951    10947       -4     
- Misses       1139     1143       +4     
  Partials       34       34              

Flag	Coverage Δ
suite.unit	90.29% <ø> (-0.04%)	⬇️

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.