feat: add Atlas Stream Processing tools MCP-406#967
Conversation
af3bfff to
e16d079
Compare
|
Question: Integration test scope — is this overkill? @blva The integration tests in this PR cover 40 scenarios including workspace CRUD, connection CRUD (HTTPS, Kafka, Schema Registry), processor lifecycle (create, start, stop, modify, delete), workspace tier updates, networking, logs, and cross-workspace discovery. For comparison, the terraform-provider-mongodbatlas repo — which has dedicated CI infrastructure — covers a similar scope across its streams integration tests:
Our MCP server integration tests overlap significantly with that coverage (workspace CRUD, Kafka/SchemaRegistry/HTTPS connections, processor lifecycle, tier changes). A few things we don't cover that the terraform provider does: VPC/PrivateLink networking, Lambda connections, S3 connections, OAuth auth — but those require external cloud infrastructure. Questions for the team:
|
Pull Request Test Coverage Report for Build 23150993685Details
💛 - Coveralls |
d8ec8c9 to
cd1675c
Compare
| @@ -1,4 +1,4 @@ | |||
| export const previewFeatureValues = ["search", "mcpUI"] as const; | |||
| export const previewFeatureValues = ["search", "mcpUI", "streams"] as const; | |||
There was a problem hiding this comment.
this will be removed after the PR to add the Accuracy test is created.
| "drop-collection", | ||
| "delete-many", | ||
| "drop-index", | ||
| "atlas-streams-manage", |
There was a problem hiding this comment.
this is needed for elicitation to the users that modifies or deletes their streams resources.
|
@blva I tagged you for an initial review so you can get an early look before my next pass tomorrow morning at this PR. I also have some early questions about the Integration testing philosophy in the above comment. |
Hi, this is about testing our integration with the Streams API works well. Terraform uses a different SDK and codebase. We should include int tests in my opinion. I'm OK with a second PR if this one is already too big. |
Okay, I'll add those integration tests in the PR Accuracy test as well if that is okay. Since this has the preview flag on it, it should be enough guardrails before the final PR is merged. |
a9bde70 to
d1778b9
Compare
| mode: "form", | ||
| requestedSchema: Elicitation.CONFIRMATION_SCHEMA, | ||
| }, | ||
| { timeout: 300000 } |
There was a problem hiding this comment.
this is from the elicitation timeout addition
| // - S3 connection creation (requires AWS IAM role ARN registered in Atlas) | ||
| // - AWSKinesisDataStreams connection creation (requires AWS IAM role ARN) | ||
| // - AWSLambda connection creation (requires AWS IAM role ARN) | ||
| // - PrivateLink creation (requires provider-specific infrastructure) |
There was a problem hiding this comment.
These private link connections can take up to 10 minutes to provision, is it okay to have our integration tests running for that long? For reference, the integration tests in the terraform provider for streams can take up to 35 to 40 minutes.
There was a problem hiding this comment.
great point! lets have them running in Code Health Long Running Tests, performance advisor tests also run there
There was a problem hiding this comment.
left a comment mentioning where you can add it (#967 (comment))
| | { accepted: true; fields: Record<string, string> } | ||
| | { accepted: false; fields?: undefined }; | ||
|
|
||
| const ELICITATION_TIMEOUT_MS = 300_000; // 5 minutes for user interaction |
There was a problem hiding this comment.
The timeout was added to the existing confirmAction method in elicitation.ts — the server.elicitInput() call previously had no timeout, which means it would use the SDK's default (which could hang indefinitely if a client never responds). We added { timeout: 300_000 } (5 minutes) to give users reasonable time to respond while preventing the server from hanging forever.
| : {}, | ||
| }, | ||
| headers: { | ||
| Accept: "application/vnd.atlas.2025-03-12+gzip", |
There was a problem hiding this comment.
we should handle the versions in the api client layer, hardcoding the version in the tool is very error prone. if you extend that functionality, that can be in a new PR so that it's easy to revert if changes separately if any bugs show up.
There was a problem hiding this comment.
Agreed, the versions shouldn't be hardcoded in the tool. The top of apiClient.ts (client setup, middleware, etc.) is hand-written, only the wrapper methods below line 210 are auto-generated. So we could add middleware there that sets the right Accept header for gzip endpoints. I'll pull this out into a follow-up PR to keep it easy to revert separately.
ac0fa56 to
fd0a797
Compare
There was a problem hiding this comment.
Pull request overview
Adds Atlas Stream Processing (“streams”) support to the MCP server by introducing four new Atlas tools (discover/build/manage/teardown), shared streams utilities/base class, and extending elicitation + tests/config/docs to support the new workflows.
Changes:
- Add new Atlas Streams tools (
atlas-streams-discover,atlas-streams-build,atlas-streams-manage,atlas-streams-teardown) plus sharedStreamsToolBaseandStreamsArgs. - Extend elicitation to support structured form input (with timeout), and update unit/integration tests accordingly.
- Add streams integration test helpers/workflows and wire streams into telemetry, preview features, config defaults, and README.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adjusts Atlas test skipping behavior via SKIP_ATLAS_TESTS exclusion glob. |
| tests/utils/elicitationMocks.ts | Generalizes mock elicitation content typing to support structured input. |
| tests/unit/tools/atlas/streams/teardown.test.ts | Unit coverage for teardown behaviors (processors/connections/workspaces/networking deletes). |
| tests/unit/tools/atlas/streams/streamsToolBase.test.ts | Unit coverage for streams-specific error handling, telemetry metadata, and connection-name extraction. |
| tests/unit/tools/atlas/streams/streamsArgs.test.ts | Unit coverage for streams name validation schemas. |
| tests/unit/tools/atlas/streams/manage.test.ts | Unit coverage for manage actions (start/stop/modify/update/peering). |
| tests/unit/tools/atlas/streams/discover.test.ts | Unit coverage for discover actions (list/inspect/diagnose/logs/networking). |
| tests/unit/tools/atlas/streams/build.test.ts | Unit coverage for build resources (workspace/connection/processor/privatelink) and validation/elicitation flows. |
| tests/unit/elicitation.test.ts | Adds/updates tests for timeout + new requestInput() structured elicitation method. |
| tests/unit/common/config.test.ts | Updates default confirmation-required tools list to include streams manage/teardown. |
| tests/integration/tools/mongodb/delete/dropIndex.test.ts | Updates integration expectations to include elicitation timeout arg. |
| tests/integration/tools/atlas/streams/workflow.test.ts | End-to-end streams workflow tests covering build/discover/manage/teardown. |
| tests/integration/tools/atlas/streams/teardown.test.ts | Validates teardown tool registration + basic deletion flow. |
| tests/integration/tools/atlas/streams/manage.test.ts | Validates manage tool registration + processor lifecycle actions. |
| tests/integration/tools/atlas/streams/discover.test.ts | Validates discover tool registration and common actions against live Atlas. |
| tests/integration/tools/atlas/streams/build.test.ts | Validates build tool registration and common connection creation flows. |
| tests/integration/tools/atlas/atlasHelpers.ts | Adds describeWithStreams + withWorkspace provisioning helper (workspace + cluster + connections). |
| tests/integration/elicitation.test.ts | Updates integration expectations to include elicitation timeout arg. |
| tests/browser/vitest.config.ts | Adds node:zlib alias for browser tests. |
| tests/browser/polyfills/zlib/index.ts | Adds gunzipSync polyfill stub to satisfy imports. |
| src/tools/atlas/tools.ts | Exports the new streams tools from the Atlas tools index. |
| src/tools/atlas/streams/teardown.ts | Implements atlas-streams-teardown tool and confirmation messaging. |
| src/tools/atlas/streams/streamsToolBase.ts | Adds streams-specific feature gating, error hints, connection-name extraction, telemetry metadata. |
| src/tools/atlas/streams/streamsArgs.ts | Adds shared Zod schemas for streams resource naming. |
| src/tools/atlas/streams/manage.ts | Implements atlas-streams-manage tool (processor/workspace/connection/peering operations). |
| src/tools/atlas/streams/discover.ts | Implements atlas-streams-discover tool (list/inspect/diagnose/logs/networking). |
| src/tools/atlas/streams/build.ts | Implements atlas-streams-build tool with validation and structured elicitation for missing config. |
| src/telemetry/types.ts | Adds StreamsToolMetadata and includes it in tool metadata union. |
| src/elicitation.ts | Adds structured requestInput() + introduces a standard 5-minute timeout for elicitation calls. |
| src/common/schemas.ts | Adds streams to preview features. |
| src/common/config/userConfig.ts | Adds streams manage/teardown to default confirmation-required tools list. |
| README.md | Documents new tools, streams preview feature, and updates config defaults/roles table. |
| @@ -13,7 +13,7 @@ const vitestDefaultExcludes = [ | |||
| const longRunningTests = ["tests/integration/tools/atlas/performanceAdvisor.test.ts"]; | |||
There was a problem hiding this comment.
this is where you should add your test as longRunning
There was a problem hiding this comment.
@jwongmongodb i think you missed this comment about adding your test to the long running tests?
There was a problem hiding this comment.
ah, I misunderstood, I thought the new long running test to be added later will be added here, the current tests are quick. Do you want me to move all the tests here right now (even the current quick ones)?
My comment was for the todo tests to come in the next PR (private link etc)
There was a problem hiding this comment.
oh, I see! I meant only the integration tests that take some time (e.g. creating takes long, etc)
There was a problem hiding this comment.
yep, I think the current tests are quite fast, so I will have the long running ones here in the next PR. I'll fix the cleanup now
| try { | ||
| const buffer = Buffer.from(data as unknown as ArrayBuffer); | ||
| const decompressed = gunzipSync(buffer).toString("utf-8"); | ||
|
|
||
| // Limit output to avoid overwhelming the context | ||
| const lines = decompressed.split("\n").filter((line) => line.trim()); | ||
| const maxLines = 100; | ||
| const truncated = lines.length > maxLines; | ||
| const output = lines.slice(-maxLines).join("\n"); |
There was a problem hiding this comment.
gunzipSync on a bounded API response is fine in practice, the MCP server is single-client, the Atlas API caps log payloads, and the output is truncated to 100 lines regardless. The async swap is a marginal improvement for a scenario that's unlikely to cause real issues.
There was a problem hiding this comment.
wdym by single-client? MCP handles multiple connections via streamable HTTP, there are use-cases where users deploy it in hosts and have multiple clients connecting.
There was a problem hiding this comment.
You're right, that was an incorrect assumption on my part. I was thinking in terms of stdio transport (which is single-client), but with streamable HTTP the server handles multiple concurrent connections. I'll swap to async gunzip to avoid blocking the event loop. (I'll push the fix tmr morning my time.)
…etry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Id describe, add CLOUDP ticket Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ce readiness loop
…kspace deletion in tests
…descriptions and error flags - Validate resourceName/peeringId/workspaceName in getConfirmationMessage() before interpolation to prevent 'undefined' in confirmation prompts (manage.ts, teardown.ts) - Add isError: true to missingFieldsResponse() in build.ts so failed validation is correctly reported as an error - Fix connectionConfig description to reflect Sample needing no config and partial config being valid via elicitation - Fix teardown description to accurately reflect safety check behavior - Add unit tests for missing-field validation in confirmation messages Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
validateSchemaRegistryConfig always defaulted auth type to USER_INFO and required username/password, even when SASL_INHERIT was specified. SASL_INHERIT inherits credentials from a Kafka connection and does not need explicit username/password. Skip credential checks when auth type is SASL_INHERIT. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
df6a241 to
82e7842
Compare
…t running processors in teardown warning
cveticm
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the changes!
Summary
Adds 4 new MCP tools (
atlas-streams-discover,atlas-streams-build,atlas-streams-manage,atlas-streams-teardown) covering the full lifecycle of Atlas Stream Processing resources:Includes shared base class (
StreamsToolBase) with common validation, elicitation flows for destructive confirmations, and reusable argument schemas.Companion PR: This should be used with the agent skill defined in mongodb/agent-skills#10, which provides the LLM with streaming-specific guidance for using these tools effectively.
Test plan
🤖 Generated with Claude Code