feat: expose CloudFormation stacks to automation - #367
YoungJinJung wants to merge 2 commits into
Conversation
- add a stable failure-first CloudFormation stack JSON query\n- expose the contract through MCP with discovery metadata\n- document usage and cover CLI and MCP wiring
|
Warning Review limit reachedNext included review available in 48 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (7)Check that tests cover API errors, mapping edge cases, and navigation state transitions, not only happy paths.⚙️ CodeRabbit configuration file Files:
Verify that README changes match actual CLI/TUI behavior and that Currently Implemented Features, TUI Key Bindings, Usage, and Configuration content stay aligned with code changes.⚙️ CodeRabbit configuration file Files:
Documentation must match implemented behavior.⚙️ CodeRabbit configuration file Files:
For Go reviews, look beyond compilation and prioritize nil pointer risks, context propagation, AWS SDK pagination, error wrapping, deterministic sorting, and stable table/detail rendering.⚙️ CodeRabbit configuration file Files:
Tests use mock client interfaces (see `rds_test.go` pattern) in Go test files📄 CodeRabbit inference engine (CLAUDE.md) Files:
When adding, modifying, or deleting features, always update `README.md` in parallel with code changes Update `Currently Implemented Features` table in README.md: add new services/features, update status changes (🚧→✅), remove deleted items...📄 CodeRabbit inference engine (CLAUDE.md) Files:
Use lipgloss for styled TUI output — column-aligned tables with dimmed labels in Go implementation files Implement scroll windowing with formula: `visibleLines := max(m.height-N, 5)` in Go TUI implementation📄 CodeRabbit inference engine (CLAUDE.md) Files:
🪛 LanguageToolskills/unic-aws/SKILL.md[style] ~13-~13: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE) 🔇 Additional comments (4)
WalkthroughThe CLI now lists CloudFormation stacks as JSON. The MCP server exposes this listing through ChangesCloudFormation stack discovery
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPServer
participant ResourcesCLI
participant ResourceRepository
MCPClient->>MCPServer: list_cloudformation_stacks
MCPServer->>ResourcesCLI: resources cloudformation-stacks --json
ResourcesCLI->>ResourceRepository: ListCloudFormationStacks
ResourceRepository-->>ResourcesCLI: CloudFormation stacks
ResourcesCLI-->>MCPServer: paginated stack JSON
MCPServer-->>MCPClient: stack status, drift, parameters, and outputs
Merge Risk: ⚪ Minimal · up to The new CloudFormation CLI query and MCP tool have consistent mapping and coverage without an actionable merge-blocking defect. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR successfully exposes CloudFormation stacks to automation with a clean implementation. The changes follow existing patterns, include comprehensive test coverage, and properly integrate with both the CLI and MCP server. No blocking issues identified.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
YoungJinJung
left a comment
YoungJinJung
left a comment
There was a problem hiding this comment.
Reviewed head: cd35eca
Findings
-
Medium — advertise the permission required by the all-stacks request (
internal/mcp/server.go:127,README.md:171)The new MCP metadata and README list only
cloudformation:DescribeStacks, but this command reachesListCloudFormationStacks, which callsDescribeStackswith an emptyStackName(internal/services/aws/cloudformation.go:18). AWS documents that this all-stacks form requires bothcloudformation:ListStacksandcloudformation:DescribeStacks. An identity provisioned fromget_mcp_capabilitiesor the README can therefore receiveAccessDeniedeven though it has every advertised permission.Suggested direction: add
cloudformation:ListStacksto the tool metadata and README permission list, and assert the CloudFormation permission set in the MCP capability tests so discovery cannot drift again. See AWSDescribeStacksauthorization notes.
|
Addressed in 2ed37ff: the CloudFormation MCP capability metadata and README now advertise both cloudformation:DescribeStacks and cloudformation:ListStacks for the all-stacks request, and the capability-alignment test pins that exact permission set. Validation passed: targeted MCP capability test, make test, make build, and git diff --check. |
YoungJinJung
left a comment
YoungJinJung
left a comment
There was a problem hiding this comment.
Re-reviewed head 2ed37ff after the permission follow-up. The tool metadata, README, and capability test now advertise both permissions required for the all-stacks DescribeStacks request. I read the complete diff, tests, prior reviews, discussion, and current CI; no further actionable concerns remain. Local validation passed: make test, make build, and git diff --check. Approval and merge remain for an independent reviewer because this GitHub identity authored the PR.
youngjinjung-linq
left a comment
youngjinjung-linq
left a comment
There was a problem hiding this comment.
Reviewed head 2ed37ff after the CloudFormation permission follow-up. I read the complete nine-file diff, tests, CI, issue context, prior reviews, comments, and thread state. The command reuses the existing paginated failure-first repository path; the stable JSON/MCP contract, UTC serialization, permission metadata, parity guard, and documentation are aligned. No actionable concerns remain. Local validation passed: targeted tests, make test, make build, schema inspection, and git diff --check.
2 participants
Summary
cloudformation-stacksJSON query that preserves failure-first stack orderinglist_cloudformation_stacksand document discovery and permissionsRelated Issues
Related to #360
Validation
go test ./internal/cli ./internal/mcp ./internal/services/awsmake testmake buildunic schema resources cloudformation-stacks --jsongit diff --checkChecklist
docs/branch-naming-harness.mddocs/documentation-harness.md)docs/pages updated if architecture, auth, config, or workflow changedSummary by CodeRabbit
New Features
Documentation