feat: expose SNS topics to automation - #366
YoungJinJung wants to merge 2 commits into
Conversation
- join topics with per-topic subscriptions and partial-result warnings - register the versioned CLI and MCP contracts - document discovery, permissions, and agent usage
|
Warning Review limit reachedNext included review available in 52 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 (6)
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 (12)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (8)For AWS integration code, focus on SDK client interface mockability, paginator usage, nil/empty response handling, AWS pointer conversion, stable list ordering, and user-facing error messages.⚙️ CodeRabbit configuration file Files:
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 (5)
WalkthroughThe change adds SNS topic and subscription discovery to the resources CLI and MCP server. It joins topic data with subscriptions, preserves partial lookup failures as warnings, exposes versioned JSON output, and updates related documentation and agent-surface registration. ChangesSNS topic discovery
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPServer
participant ResourcesCommand
participant AwsRepository
MCPClient->>MCPServer: call list_sns_topics with profile and region
MCPServer->>ResourcesCommand: execute resources sns-topics --json
ResourcesCommand->>AwsRepository: load SNS topics and subscriptions
AwsRepository-->>ResourcesCommand: topics, subscriptions, and warnings
ResourcesCommand-->>MCPServer: unic.resources.sns-topics.v1 JSON
MCPServer-->>MCPClient: paginated partial-result response
Merge Risk: ⚪ Minimal · up to No actionable issue remains; the SNS discovery feature is ready to merge after normal checks. 🚥 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 14 functions across 9 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 adds SNS topic automation support with a clean, well-tested implementation. The new ListSNSTopicResources function properly joins topics with subscriptions while handling partial results gracefully. All functionality is correctly exposed through both CLI and MCP interfaces with appropriate metadata and comprehensive test coverage. The implementation follows existing patterns and maintains API stability. No blocking issues found - ready to merge.
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: c7e2fed
Findings
- P2 — Preserve the SNS filter-policy scope in the automation contract (
internal/cli/resources.go:64,internal/cli/resources_operations.go:211). The new v1 payload emitsfilter_policy, butGetSubscriptionAttributesalso returnsFilterPolicyScope, which determines whether that JSON is evaluated against message attributes (the default) or the message body. A subscription configured withMessageBodytherefore produces the same automation payload as one usingMessageAttributes, so an agent inspecting delivery behavior can interpret the policy against the wrong input and misdiagnose why messages are filtered. Capture the scope when applying subscription attributes and expose it alongsidefilter_policy(normalizing an absent known scope to the AWS default if desired); add a contract test coveringMessageBodyso the distinction cannot regress.
- expose MessageBody versus MessageAttributes semantics - default an omitted known scope to the AWS default - cover repository mapping and the JSON contract
|
Addressed the filter-policy scope finding in 16f1f75. SNS subscription mapping now captures FilterPolicyScope, defaults an omitted scope to MessageAttributes when a filter policy is present, and emits filter_policy_scope in the v1 automation payload. Repository and JSON contract tests cover MessageBody. Validation passed: make test, make build, focused SNS/CLI/MCP tests, and git diff --check. |
|
/q review |
There was a problem hiding this comment.
Summary
This PR successfully adds SNS topic inspection functionality to the automation surface. The implementation is well-designed with:
✅ Robust Error Handling: Partial results are preserved when individual resources fail, with clear warnings
✅ Comprehensive Testing: Good test coverage including pagination, sorting, denied access scenarios, and contract validation
✅ Safe Concurrency: Proper goroutine batching and synchronization to handle API throttling
✅ Clean Integration: Consistent with existing resource commands and MCP tool patterns
The changes align with the stated goal of exposing SNS topics to automation while maintaining resilience against permission-restricted resources. 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.
There was a problem hiding this comment.
Re-reviewed head 16f1f75 after the filter-policy-scope follow-up. The new commit preserves MessageBody versus MessageAttributes, applies the documented AWS default when the scope is omitted, and covers both repository mapping and the v1 JSON contract. I read the complete 12-file diff, tests, CI, prior reviews, comments, and inline-thread state; no further actionable concerns remain. Local validation passed: make test; make build; git diff --check origin/main...HEAD.
youngjinjung-linq
left a comment
youngjinjung-linq
left a comment
There was a problem hiding this comment.
Reviewed head 16f1f75. I read the complete twelve-file diff, tests, CI, issue context, prior reviews, comments, and the filter-policy-scope follow-up. The partial-result behavior, deterministic ordering, JSON/MCP contracts, permissions, and documentation are aligned; no actionable concerns remain. Local validation passed: make test, make build, and git diff --check.
2 participants
Summary
sns-topicsJSON query that joins topics with subscriptionslist_sns_topicsand document discovery and permissionsRelated Issues
Related to #360
Validation
make testmake buildunic schema resources sns-topics --jsonunic capabilities --jsondiscovery checkgit 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