feat(toolkit-lib): diagnose hook failures in the central stack diagnoser - #1786
Merged
Conversation
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends CloudFormation hook-failure diagnostics across the toolkit by centralizing GetHookResult fetching/formatting in a shared helper and wiring that into both deploy monitoring and the central stack diagnoser, so hook failure details are available beyond the live activity stream.
Changes:
- Introduces a shared
fetchHookResultDetails()helper to retrieve and formatGetHookResultdetails (with warning behavior on failures). - Correlates failed hook-invocation events with subsequent resource failures via
ResourceErrors, and surfaces those details as additional diagnostic context inCloudFormationStackDiagnoser. - Updates the stack activity monitor to reuse the shared hook-result helper for live event enrichment.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/@aws-cdk/toolkit-lib/lib/api/stack-events/hook-result-details.ts | Adds shared hook result fetching helper and related warning logic. |
| packages/@aws-cdk/toolkit-lib/lib/api/stack-events/stack-activity-monitor.ts | Switches live hook detail enrichment to the shared fetch helper. |
| packages/@aws-cdk/toolkit-lib/lib/api/stack-events/resource-errors.ts | Tracks failed hook events and attaches correlated hook failures to resource errors. |
| packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts | Enriches resource-error diagnosis with hook failure details (via GetHookResult or fallback). |
| packages/@aws-cdk/toolkit-lib/test/api/stack-events/resource-errors.test.ts | Adds unit tests for hook failure correlation behavior. |
| packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts | Adds unit tests verifying diagnoser hook-detail enrichment and fallback behavior. |
Comments suppressed due to low confidence (1)
packages/@aws-cdk/toolkit-lib/lib/api/stack-events/resource-errors.ts:169
- hookFailuresFor() also computes the lookup key even when StackId/LogicalResourceId are missing, which again falls back to an empty-stack key. Returning early avoids accidental matches and makes the correlation rules explicit.
private hookFailuresFor(event: StackEvent): ResourceHookFailure[] {
const recorded = this.seenHookFailures.get(hookFailureKey(event)) ?? [];
const reason = event.ResourceStatusReason ?? '';
return recorded.filter((h) => reason.includes(h.hookType));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Formation Hooks When a CloudFormation Hook (e.g. a Guard Hook or Lambda Hook) fails a resource during a deployment, the stack events only carry a terse 'The following hook(s) failed: [X]'. The actual failure details (Guard rule annotations, remediation messages, Lambda hook reasons) live in the GetHookResult API. cdk deploy already fetches them for its live activity stream, but cdk diagnose showed nothing beyond the terse message. Move hook-error diagnosis into the central CloudFormationStackDiagnoser: - ResourceErrors now correlates failed hook events to the resource errors they cause (resolving the 'FIXME: Check hooks') and records them on a new hookFailures field. - The GetHookResult fetch (including the bootstrap-version permission hint) is extracted into a shared fetchHookResultDetails helper, used by the deploy monitor's live enrichment, the diagnoser's change-set hook path, and the new stack-event hook path. - The diagnoser fetches hook failure details for stack-event errors and attaches them as additional diagnostic context. This is gated behind a new fetchHookFailureDetails option, enabled only by the diagnose action: during a deployment the activity stream already shows the details live, so deploy output is unchanged. Also fixes a malformed permissions warning (stray newline and quote) that was carried over from the monitor code.
iliapolo
requested changes
Jul 30, 2026
| const [sourceTrace, additionalContext, hookContext] = await Promise.all([ | ||
| sourceTracePromise, | ||
| this.investigateResourceBestEffort(err), | ||
| this.hookFailureContext(err), |
Contributor
There was a problem hiding this comment.
This adds a new API call, deserves an integ test.
Contributor
Author
There was a problem hiding this comment.
You're right, we test the deploy flow but now introduce this API call for diagnose. Will add an integ test.
Covers the new GetHookResult fetch in the central stack diagnoser: deploys the guard-hook-app fixture's non-compliant stack with --no-rollback, then asserts that cdk diagnose output contains the hook failure details that are only available via the GetHookResult API.
The Diagnosis wrapper doesn't expose 'problems' directly; use the value returned by assertProblem(), which narrows to the 'problem' variant.
iliapolo
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1788
Description
Details for CloudFormation hook failures are currently provided when hooks fail during
cdk deploy, but not incdk diagnose.For cdk diagnose, only the short reason would be available (e.g. "The following hook(s) failed: [X]"), since it doesn't have a live activity monitor.
Current state
Hook details available:
cdk deploylive activity monitorcdk deployfinal output (since we already got it from the love activity monitor)Hook details not available:
cdk diagnosefinal outputCurrent
cdk diagnoseexampleState after this change
No changes to
cdk deployoutput.cdk diagnoseafter this change:In this change, we move hook error detail fetching to a shared helper (
fetchHookResultDetails), so that it's available in diagnose without code duplication.We move hook-error diagnosis into the central diagnoser, which both
cdk deployandcdk diagnoseconverge into:GetHookResultfetch is extracted into a sharedfetchHookResultDetailshelper.fetchHookResultDetails).Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license