Skip to content

Make the MCP smoke checks work in both authentication modes - #1333

Closed
jiaenren wants to merge 1 commit into
jiaenr/mcp-shared-redis-and-replicasfrom
jiaenr/mcp-smoke-checks-proxy-mode
Closed

Make the MCP smoke checks work in both authentication modes#1333
jiaenren wants to merge 1 commit into
jiaenr/mcp-shared-redis-and-replicasfrom
jiaenr/mcp-smoke-checks-proxy-mode

Conversation

@jiaenren

@jiaenren jiaenren commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #1331. Found by the first OIDC-proxy deployment.

Issue #None

What the deploy found

The suite failed before reaching any assertion. It authenticates with the OSMO-issued OETF token, which FastMCP rejects — in proxy mode FastMCP verifies caller tokens against the identity provider. Evidence from the run: POST /mcp returned 200 for proxy-issued tokens and 401 for the suite's token within the same minute.

So this was never "a stale expectation". The suite was written for direct mode, where the Gateway authenticates and MCP trusts injected headers. Staging still runs direct mode, which is why the incompatibility only appeared when proxy mode was deployed for the first time.

This matters for the PR that removes direct mode. Once that lands, staging moves to proxy mode and this suite would break there too — so this fix is a prerequisite, not cleanup.

Change

Split along the line that actually matters — whether a caller token is needed.

Unauthenticated, runs in both modes (new test_public_discovery_surface): the 401 challenge and its resource_metadata pointer, the RFC 9728 document, and in proxy mode that FastMCP advertises authorization_endpoint under /mcp and that the health endpoints the /mcp prefix would otherwise publish return 404.

This is the surface a client uses to bootstrap OAuth, so needing no token is the point.

Authenticated (catalog + workflow round-trip): skip in proxy mode with a message naming OSMO_MCP_ACCESS_TOKEN, which runs them there when an identity-provider token is supplied.

Two related fixes

  • The expected catalog was missing osmo_list_tasks (added by Add MCP task list tool #1311), so the exact-equality assertion would have failed 25-vs-26 had it ever run. Two counts in src/service/mcp/README.md were stale for the same reason — corrected to 26 tools / 15 read-only, verified against the registry.
  • The suite never ran anywhere. It carried an auth tag, and the kind environment's exclude_tags: [auth] dropped it — which is exactly why the staleness went unnoticed. It is now tagged by what it needs deployed (mcp), with kind excluding mcp explicitly, since kind keeps MCP disabled.

Verification

  • bazel test //src/service/mcp/... //test/smoke/... — 76/76 pass
  • render-tests.sh, validate-mcp-chart.sh — pass
  • The behavioural half needs a redeploy of a development deployment to confirm against a live proxy-mode deployment

🤖 Generated with Claude Code

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

A dev-instance deploy of OIDC-proxy mode showed this suite failing before
it reached any assertion: it authenticates with the OSMO-issued OETF
token, which FastMCP rejects because it verifies caller tokens against
the identity provider. The suite was written for direct mode, where the
Gateway authenticates and MCP trusts injected headers, and staging still
runs that mode -- so the incompatibility only surfaced once proxy mode
was deployed for the first time.

Split the suite along the line that actually matters. The unauthenticated
surface a client uses to bootstrap OAuth needs no token and is now its own
test that runs in both modes: the 401 challenge and its resource_metadata
pointer, the RFC 9728 document, and -- in proxy mode -- that FastMCP
advertises its authorization endpoint under /mcp and that the health
endpoints the /mcp prefix would otherwise publish return 404.

The authenticated catalog and workflow checks skip in proxy mode with a
message naming OSMO_MCP_ACCESS_TOKEN, which runs them there when an
identity-provider token is supplied.

Two related fixes. The expected catalog was missing osmo_list_tasks,
added by #1311, so the exact-equality assertion would have failed at 25
versus 26 had it ever run; the two counts in the service README were
stale for the same reason. And the suite carried an `auth` tag, which the
kind environment's exclude_tags dropped -- so it never ran anywhere, which
is why the staleness went unnoticed. It is now tagged by what it needs
deployed, with kind excluding `mcp` explicitly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jiaenren
jiaenren requested a review from a team as a code owner August 27, 2026 00:07
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 81b79f12-f6e3-49f2-86de-331a3f0f8808

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

@jiaenren

Copy link
Copy Markdown
Collaborator Author

Consolidated into #1334 to reduce the number of PRs in this stack for review.

The smoke-check changes exist because direct mode is going away, so they belong with the commit that removes it.

No content is lost: the cumulative diff of the stack against main is byte-identical to before the consolidation (verified by comparing the resulting tree). Closing this one.

@jiaenren jiaenren closed this Aug 28, 2026
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.

1 participant