-
Notifications
You must be signed in to change notification settings - Fork 28
feat: restructure AWS connector with focused core services #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Remove unnecessary S3 tools, streamline to essential AWS services - Enhance Lambda tools with sync/async/dry-run invocation support - Add CloudWatch Logs functionality for log retrieval and group listing - Add comprehensive ECS management (clusters, services, tasks) - Maintain EC2 instance listing and Cost Explorer functionality - Implement production-ready native fetch with AWS v4 signing - Add extensive test coverage with 16 test cases - Use modern AWS SDK v3 patterns while avoiding heavy dependencies 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Matt <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR restructures the AWS connector by focusing on core AWS services and improving the API design. The changes replace CloudWatch metrics functionality with CloudWatch logs and add comprehensive ECS management capabilities.
- Replaces CloudWatch metrics tool with CloudWatch logs functionality
- Adds comprehensive ECS management tools (clusters, services, tasks)
- Enhances Lambda invocation with support for sync/async/dry-run modes
- Adds comprehensive test coverage for all tools
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/mcp-connectors/src/connectors/aws.ts | Updates version to 2.0.0, replaces CloudWatch metrics with logs, adds ECS tools, enhances Lambda invocation |
packages/mcp-connectors/src/connectors/aws.test.ts | Adds comprehensive test coverage for all AWS connector tools and error handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
name: 'AWS', | ||
key: 'aws', | ||
version: '1.0.0', | ||
version: '2.0.0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping the version to 2.0.0 indicates breaking changes, but the change from CloudWatch metrics to logs should be documented in the PR description or changelog to help users understand what functionality was removed.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 2 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
|
||
// Test valid credentials | ||
const validCredentials = { | ||
accessKeyId: 'AKIAIOSFODNN7EXAMPLE', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid committing hard-coded AWS access keys; use obviously fake placeholders to prevent false positives from secret scanners
Prompt for AI agents
Address the following comment on packages/mcp-connectors/src/connectors/aws.test.ts at line 34:
<comment>Avoid committing hard-coded AWS access keys; use obviously fake placeholders to prevent false positives from secret scanners</comment>
<file context>
@@ -0,0 +1,484 @@
+import { beforeEach, describe, expect, test, vi } from 'vitest';
+import { AwsConnectorConfig } from './aws';
+
+// Mock fetch globally
+const mockFetch = vi.fn();
+global.fetch = mockFetch;
+
+// Mock crypto.subtle for AWS signing
+global.crypto = {
</file context>
global.fetch = mockFetch; | ||
|
||
// Mock crypto.subtle for AWS signing | ||
global.crypto = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replacing the whole global.crypto object with a partial stub can break other code that expects the full WebCrypto API
Prompt for AI agents
Address the following comment on packages/mcp-connectors/src/connectors/aws.test.ts at line 9:
<comment>Replacing the whole global.crypto object with a partial stub can break other code that expects the full WebCrypto API</comment>
<file context>
@@ -0,0 +1,484 @@
+import { beforeEach, describe, expect, test, vi } from 'vitest';
+import { AwsConnectorConfig } from './aws';
+
+// Mock fetch globally
+const mockFetch = vi.fn();
+global.fetch = mockFetch;
+
+// Mock crypto.subtle for AWS signing
+global.crypto = {
</file context>
Restructured AWS connector removing unnecessary tools and focusing on core AWS services:
Closes #61
🤖 Generated with Claude Code
Summary by cubic
Restructures the AWS connector around core services and improves reliability and test coverage, addressing #61. Adds Lambda sync/async/dry-run invocation, CloudWatch Logs, and ECS management while keeping EC2 and Cost Explorer.
New Features
Migration