Skip to content

fix: detect streamable-http transport for non-Docker catalog servers#2910

Draft
joeyorlando wants to merge 8 commits intomainfrom
fix/catalog-http-transport-detection
Draft

fix: detect streamable-http transport for non-Docker catalog servers#2910
joeyorlando wants to merge 8 commits intomainfrom
fix/catalog-http-transport-detection

Conversation

@joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented Feb 19, 2026

Summary

  • When installing a local MCP server from the Online Catalog that uses --transport streamable-http in its args (e.g. Google Workspace MCP), the platform was not setting transportType or httpPort on the localConfig
  • This happened because parseDockerArgsToLocalConfig() returns null when there's no docker_image, and the fallback code path didn't infer transport type at all
  • The pod deployed with stdin: true and no port exposure, making the server unreachable even though it was listening on HTTP port 8000

Fix

The non-Docker code path in archestra-catalog-tab.tsx now also checks:

  1. Server args for --transport streamable-http
  2. oauth_config.streamable_http_port as a fallback

This ensures catalog servers like taylorwilsdon/google_workspace_mcp get deployed with proper HTTP transport and port exposure.

Related: archestra-ai/website#369 & #2717

When installing a local server from the Online Catalog that uses
`--transport streamable-http` in its args (e.g. Google Workspace MCP),
the platform was not setting `transportType` or `httpPort` on the
localConfig because the docker-args-parser only runs when a docker_image
is present.

This caused the pod to deploy with `stdin: true` and no port exposure,
making the server unreachable even though it was listening on HTTP.

Now the non-Docker code path also checks:
1. Server args for `--transport streamable-http`
2. `oauth_config.streamable_http_port` as a fallback

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Feb 19, 2026

Claude finished @joeyorlando's task in 2m 38s —— View job


Reviewing PR #2910

  • Read repository guidelines (CLAUDE.md)
  • Gather PR diff and review changed files
  • Analyze changes and provide review feedback

Code Review: fix: detect streamable-http transport for non-Docker catalog servers

Overall assessment: This is a well-targeted bug fix with good test coverage. The changes are clean and the approach is sound. A few observations below.


What this PR does (for reviewers)

The PR addresses three related issues:

  1. Frontend transport detection gap (archestra-catalog-tab.tsx): When parseDockerArgsToLocalConfig() returns null (no docker_image), the fallback code path was not detecting --transport streamable-http in args. Now it does.

  2. Graceful 401 handling during tool discovery (mcp-client.ts): OAuth MCP servers (like Google Workspace) return 401 before the user has completed the OAuth flow. Previously this would cause the installation to fail. Now it returns empty tools and lets the user complete OAuth first.

  3. Consistent default port constant (consts.ts + backend files): Introduces DEFAULT_MCP_HTTP_PORT = 8080 to replace scattered hardcoded 8080 values.


Strengths

  • The adjacency check for --transport streamable-http (lines 281-284 in archestra-catalog-tab.tsx) is more robust than the previous includes() checks, which could have false-positived on args like ["--transport", "stdio", "--some-flag", "streamable-http"].
  • Good test coverage for the OAuth 401 handling — tests cover StreamableHTTPError(401), UnauthorizedError, cases without oauthConfig, and the normal success path.
  • Extracting DEFAULT_MCP_HTTP_PORT into a shared constant is a good cleanup.

Issues and suggestions

1. Inconsistency in Docker path transport detection (minor, pre-existing)

The parseDockerArgsToLocalConfig function (docker-args-parser.ts:114-133) only detects streamable-http via the --port flag, not via --transport streamable-http. This means a Docker-based catalog entry that uses --transport streamable-http without --port would not be detected as streamable-http in the Docker code path. The non-Docker path now correctly handles --transport streamable-http, but the Docker path is inconsistent.

This is a pre-existing gap, not introduced by this PR, but worth noting since both code paths should ideally use the same detection logic.

2. Frontend fallback port changed from 8000 to 8080 (archestra-catalog-tab.tsx:294)

In the previous version of the non-Docker fallback (before this PR's predecessor commit), the hardcoded fallback was 8000:

httpPort: isStreamableHttp ? (streamableHttpPort ?? 8000) : undefined,

Now it's:

httpPort: isStreamableHttp ? (streamableHttpPort ?? DEFAULT_MCP_HTTP_PORT) : undefined,

Since DEFAULT_MCP_HTTP_PORT = 8080, this changes the fallback port from 8000 to 8080 for non-Docker catalog servers that don't specify streamable_http_port. The PR description mentions Google Workspace MCP listening on port 8000. If that's the case, this change would use port 8080 (incorrect) instead of 8000 unless streamable_http_port is explicitly set in the catalog entry. Worth double-checking that Google Workspace MCP (and similar servers) set oauth_config.streamable_http_port explicitly, or that 8080 is the correct default for those servers.

3. OAuth redirect rewrite refactor (archestra-catalog-tab.tsx:222-235)

This PR includes a change to how rewrittenOauth is structured — for requires_proxy servers, the oauthConfig is now passed through as-is. The old code skipped requires_proxy servers entirely (they got undefined). This is a behavior change: requires_proxy servers now get their oauthConfig included in the catalog item creation payload. The added comment explains this is intentional ("redirect rewriting is handled server-side"), which makes sense, but it's a separate fix from the transport detection issue. Consider calling this out in the PR description.

4. client.close() not called on auth error path (mcp-client.ts:1552-1558)

When a 401 is encountered, the code returns [] immediately without calling await client.close(). The connect() call on line 1523 may have partially established a connection (transport open, etc.) before the 401 error. Other return paths (line 1534) do call client.close(). Consider adding cleanup:

if (isAuthError) {
  logger.info(/* ... */);
  try { await client.close(); } catch { /* ignore */ }
  return [];
}

This would prevent potential resource leaks in the MCP SDK transport layer.

5. Commit message "wip"

Several commits in this branch use "wip" as the message (c62abb6, 725d3be, 95169d7). Consider squashing these before merge to keep the git history clean.


Verdict

The core fix is correct and well-tested. The main actionable items are:

@London-Cat
Copy link
Collaborator

London-Cat commented Feb 19, 2026

📊 Reputation Summary

User Rep Pull Requests Activity Assigned Core Reactions
joeyorlando ⚡ 2312 92✅ 4🔄 4❌ 100 issues, 50 comments 4

How is the score calculated? Read about it in the Reputation Bot repository 🤖

joeyorlando and others added 6 commits February 19, 2026 16:33
When an OAuth MCP server (e.g. Google Workspace MCP with
EXTERNAL_OAUTH21_PROVIDER=true) is installed, tool discovery via
connectAndGetTools() would fail with 401 because no valid OAuth
token exists yet — the user hasn't completed the OAuth flow.

Previously this caused 3 retries (15+ seconds wasted) and marked
the installation as "error". Now, when a 401/UnauthorizedError
occurs during tool discovery and the catalog item has oauthConfig,
we return empty tools immediately. The installation succeeds with
0 tools, and tools will be discovered after the user completes
the OAuth flow via "Connect".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

Playwright test results

passed  415 passed
flaky  4 flaky

Details

stats  419 tests across 48 suites
duration  7 minutes, 5 seconds
commit  734979b

Flaky tests

api › api/chat-settings.spec.ts › Chat API Keys Access Control › member should be able to read chat API keys
api › api/llm-proxy/token-cost-limits.spec.ts › LLMProxy-TokenCostLimits-vLLM › blocks request when profile token cost limit is exceeded
api › api/metrics.spec.ts › Metrics API › returns metrics when authentication is provided
chromium › ui/chat.spec.ts › Chat-UI-anthropic › can send a message and receive a response from Anthropic

@joeyorlando joeyorlando marked this pull request as draft February 19, 2026 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants