Skip to content

Conversation

@mdelapenya
Copy link
Member

What does this PR do?

This PR does the following:

  • skips one testable example that listed all available tools, as its result is not deteministic. On my host (with Docker Desktop and MCP Toolkit) it returned 12 tools. On GH, it was empty
  • bumps github.com/modelcontextprotocol/go-sdk to v1.0.0, a new major release appeared, so I updated the example.

Why is it important?

Fix CI for those PRs kicking the build for all modules

Related issues

@mdelapenya mdelapenya requested a review from a team as a code owner October 24, 2025 10:52
@mdelapenya mdelapenya added chore Changes that do not impact the existing functionality test flakiness Report a flaky test that happens on the CI labels Oct 24, 2025
@mdelapenya mdelapenya self-assigned this Oct 24, 2025
@mdelapenya mdelapenya added chore Changes that do not impact the existing functionality test flakiness Report a flaky test that happens on the CI labels Oct 24, 2025
@netlify
Copy link

netlify bot commented Oct 24, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit c17d40a
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/68fb5a62c17e4d0007cafe37
😎 Deploy Preview https://deploy-preview-3457--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

Summary by CodeRabbit

  • Dependencies

    • Upgraded the MCP SDK dependency to v1.0.0 (from v0.2.0), providing the latest functionality and improvements.
    • Added support for JSON schema handling through an indirect dependency.
  • Chores

    • Updated internal examples and code paths to align with SDK API changes.

Walkthrough

The mcp SDK dependency was upgraded from v0.2.0 to v1.0.0, accompanied by addition of a jsonschema-go indirect dependency. Example code was refactored to use the updated API, replacing factory-style transport initialization with composite literals and adapting Connect() method calls.

Changes

Cohort / File(s) Summary
Dependency Updates
modules/dockermcpgateway/go.mod
Updated github.com/modelcontextprotocol/go-sdk from v0.2.0 to v1.0.0; added indirect dependency github.com/google/jsonschema-go v0.3.0
Example Code Refactoring
modules/dockermcpgateway/examples_test.go
Replaced mcp.NewSSEClientTransport() factory call with direct &mcp.SSEClientTransport{} composite literal; updated client.Connect() invocation to pass additional third argument (nil); removed prior example output comments

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 SDK springs to one-point-oh so bright,
The factory fades, composite takes flight,
Connect's new signature, three arguments true,
Schemas and gateways, all shiny and new! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "chore(dockermcpgateway): skip testable example as it's not deterministic" directly and clearly summarizes the primary change in the changeset. The title specifically identifies the main action (skipping a testable example) and the key reason for it (non-determinism), which aligns with the core purpose of the PR as described in the objectives—fixing CI failures caused by non-deterministic test output. While the PR also includes an SDK version bump, the title appropriately focuses on the primary motivation for the changes. The title is concise, specific, and would allow a teammate reviewing the history to quickly understand the main change.
Description Check ✅ Passed The pull request description is directly related to the changeset and provides meaningful information about the changes. It clearly explains what the PR does (skipping a non-deterministic testable example and bumping the SDK to v1.0.0), why it's important (fixing CI failures across modules), and provides concrete details such as the different tool counts observed on the author's host versus GitHub. The description is neither vague nor completely off-topic, and it successfully conveys the rationale and scope of the changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55092df and c17d40a.

⛔ Files ignored due to path filters (1)
  • modules/dockermcpgateway/go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • modules/dockermcpgateway/examples_test.go (1 hunks)
  • modules/dockermcpgateway/go.mod (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: test (1.25.x, modules/dockermcpgateway) / test: modules/dockermcpgateway/1.25.x
  • GitHub Check: test (1.24.x, modules/dockermcpgateway) / test: modules/dockermcpgateway/1.24.x
  • GitHub Check: Analyze (go)
🔇 Additional comments (5)
modules/dockermcpgateway/go.mod (2)

34-34: LGTM!

The jsonschema-go indirect dependency is appropriately added as a transitive dependency of the upgraded MCP SDK v1.0.0.


9-9: SDK version verified as current and all usages properly updated.

The latest stable version of github.com/modelcontextprotocol/go-sdk is v1.0.0, confirming the bump is to the latest release. Codebase analysis shows the MCP SDK is used in only one file (modules/dockermcpgateway/examples_test.go), and it has already been updated to the new v1.0.0 API signature (the Connect() call at line 83 includes the required context parameter). No old factory patterns or deprecated APIs remain.

modules/dockermcpgateway/examples_test.go (3)

77-79: LGTM! Transport initialization updated for v1.0.0 API.

The change from NewSSEClientTransport factory function to direct struct instantiation with &mcp.SSEClientTransport{Endpoint: gatewayEndpoint} is correct and follows the updated SDK v1.0.0 API.


98-100: LGTM! Non-deterministic example output correctly handled.

The absence of an // Output: comment block makes this example non-testable, which correctly addresses the PR objective to skip the non-deterministic test. This example still serves as useful documentation while avoiding CI flakiness caused by unpredictable tool counts.


83-83: No issues found with the Connect method update.

The third parameter is *ClientSessionOptions (options for the client session). The parameter is intentionally marked as unused in the signature, and passing nil aligns with v1.0.0 API documentation and README examples. The code change is correct and requires no modifications.


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.

❤️ Share

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

@mdelapenya mdelapenya merged commit 16dfc65 into testcontainers:main Oct 24, 2025
17 checks passed
@mdelapenya mdelapenya deleted the fix-dockermcpgateway-tests branch October 24, 2025 11:12
mdelapenya added a commit to LaurentGoderre/testcontainers-go that referenced this pull request Oct 24, 2025
* main:
  chore(dockermcpgateway): skip testable example as it's not deterministic (testcontainers#3457)
  fix(compose): update to docker compose v2.40.2 and use api.Compose interface (testcontainers#3456)
  chore(deps): bump mkdocs-include-markdown-plugin from 7.1.6 to 7.1.8 (testcontainers#3455)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Changes that do not impact the existing functionality test flakiness Report a flaky test that happens on the CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant