Issue #594 - Added logic to inspect nested stacks for the API ID if it is not found in the main stacks (split-stacks support)

[Gleeble]

I took @tobinc's changes and refactored them to instead only look in nested cloudformation stacks to get the graphql api's id. This limits the stacks that will need to be investigated to only those that are part of the main stack.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced AppSync API ID discovery with caching to reduce CloudFormation lookup overhead. The system now comprehensively searches primary and nested stack configurations for more reliable API ID resolution.

[tobinbc]

Nice!

[marcelinhov2]

+1

[sid88in]

@AlexHladin - review please

[sid88in]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

The plugin adds caching infrastructure and enhances API ID discovery to support nested CloudFormation stacks. A cachedValues field stores resolved API IDs across method calls, initialized in the constructor. The getApiId() method now checks the cache, performs a two-step lookup (main stack, then paginated nested stacks), extracts API IDs from stack outputs, and caches results to avoid repeated CloudFormation operations.

Changes

AppSync API ID Caching and Discovery

Layer / File(s)	Summary
Caching infrastructure setup src/index.ts	New private cachedValues field declared on the plugin class stores the resolved apiId, initialized to null in the constructor to enable caching across method calls.
Enhanced API ID discovery with nested stack support src/index.ts	CloudFormation AWS SDK imports are expanded with listStackResources, describeStacks, and Outputs types. The getApiId() method is reworked to check the cache first; if the API ID is not cached, it computes the API logical ID and output key, checks the main stack directly, and if missing, paginates through nested stacks via listStackResources and describeStacks to extract the API ID from outputs. The result is cached and returned; the same "not found / did you forget to deploy?" error is thrown if resolution fails.

Sequence Diagram

sequenceDiagram
  participant Plugin
  participant Cache
  participant CloudFormation
  participant NestedStack

  Plugin->>Cache: Check if apiId cached
  alt Cache hit
    Cache-->>Plugin: Return cached apiId
  else Cache miss
    Plugin->>CloudFormation: Compute API logical ID
    Plugin->>CloudFormation: describeStackResources (main stack)
    CloudFormation-->>Plugin: Check for API resource
    alt API found in main stack
      Plugin->>Cache: Store apiId
      Cache-->>Plugin: apiId cached
      Cache-->>Plugin: Return apiId
    else API not in main stack
      Plugin->>CloudFormation: listStackResources (paginate)
      CloudFormation-->>Plugin: Stack resource summaries
      Plugin->>CloudFormation: describeStacks (nested stack)
      CloudFormation-->>Plugin: Stack with outputs
      NestedStack->>Plugin: Extract API ID from stackOutputKey
      Plugin->>Cache: Store apiId
      Cache-->>Plugin: apiId cached
      Cache-->>Plugin: Return apiId
    end
  end

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A cache born anew, with nested stacks to pursue,
API IDs no more sought again and again,
Through CloudFormation's halls, the plugin now crawls,
With outputs as breadcrumbs, it always will win! 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly reflects the main change: adding logic to search nested stacks for the AppSync API ID when not found in the main stack, with explicit mention of split-stacks support.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

src/index.ts (1)

389-468: ⚡ Quick win

Add focused tests for the new lookup branches.

src/__tests__/commands.test.ts:313-374 still only covers the direct main-stack lookup, so regressions in nested-stack resolution, listStackResources pagination, or the new memoization path will be easy to miss.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/index.ts` around lines 389 - 468, Tests need to be added to cover
nested-stack resolution, listStackResources pagination, and the memoization path
around this.cachedValues.apiId so regressions don't slip through; add focused
unit tests that (1) simulate provider.request responses where the main stack
lacks the Api resource but listStackResources returns multiple pages with nested
AWS::CloudFormation::Stack entries and the ApiId is found in a nested stack's
Outputs (exercise the describeStacks path and stackOutputKey lookup), (2)
simulate pagination by returning NextToken on the first listStackResources call
and the Api found only on a later page, and (3) verify that once
this.cachedValues.apiId is set subsequent calls short-circuit and do not invoke
provider.request again (memoization). Target the test file referenced
(src/__tests__/commands.test.ts) and mock provider.request for the
CloudFormation calls used in this code (describeStackResources,
listStackResources, describeStacks) and assert correct behavior for apiId
resolution and caching.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/index.ts`:
- Around line 389-468: Tests need to be added to cover nested-stack resolution,
listStackResources pagination, and the memoization path around
this.cachedValues.apiId so regressions don't slip through; add focused unit
tests that (1) simulate provider.request responses where the main stack lacks
the Api resource but listStackResources returns multiple pages with nested
AWS::CloudFormation::Stack entries and the ApiId is found in a nested stack's
Outputs (exercise the describeStacks path and stackOutputKey lookup), (2)
simulate pagination by returning NextToken on the first listStackResources call
and the Api found only on a later page, and (3) verify that once
this.cachedValues.apiId is set subsequent calls short-circuit and do not invoke
provider.request again (memoization). Target the test file referenced
(src/__tests__/commands.test.ts) and mock provider.request for the
CloudFormation calls used in this code (describeStackResources,
listStackResources, describeStacks) and assert correct behavior for apiId
resolution and caching.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4738e5a3-e7e4-4172-ad02-9eb6f29f9a6a

📥 Commits

Reviewing files that changed from the base of the PR and between 53cb652 and 79ee536.

📒 Files selected for processing (1)

• src/index.ts