Skip to content

fix(mcp): add api_key support and configurable defaults to MCP query server#691

Open
mvanhorn wants to merge 1 commit intovolcengine:mainfrom
mvanhorn:osc/606-mcp-configurable-defaults
Open

fix(mcp): add api_key support and configurable defaults to MCP query server#691
mvanhorn wants to merge 1 commit intovolcengine:mainfrom
mvanhorn:osc/606-mcp-configurable-defaults

Conversation

@mvanhorn
Copy link
Contributor

Summary

The in-repo MCP server at examples/mcp-query/server.py had three gaps reported in #606:

  1. No API key pass-through - no mechanism to pass authentication credentials when the OpenViking server has root_api_key configured. Added --api-key / OV_API_KEY.

  2. No configurable search scope - the search tool accepted target_uri per-call but had no server-level default, requiring every MCP client to specify the scope. Added --default-uri / OV_DEFAULT_URI as a fallback when the client does not provide target_uri.

  3. Windows localhost failure - the epilog examples used localhost which resolves to ::1 on Windows while the server binds to 127.0.0.1. Fixed to use 127.0.0.1.

Note: The external openviking-mcp PyPI package referenced in #606 is out of scope for this repo. This PR improves the official in-repo MCP example to prevent the same issues.

Changes

  • Added --api-key / OV_API_KEY CLI flag (empty string default = no auth)
  • Added --default-uri / OV_DEFAULT_URI CLI flag (empty string default = search all)
  • Search tool uses _default_uri as fallback when target_uri is not provided by the client
  • Fixed localhost to 127.0.0.1 in argparse epilog examples
  • Added environment variable documentation for new flags

All new flags have backward-compatible defaults - existing deployments are unaffected.

Fixes #606

This contribution was developed with AI assistance (Claude Code).

Test plan

  • uv run server.py starts without errors (backward compatible)
  • uv run server.py --default-uri viking://user/memories - search tool uses default URI
  • uv run server.py --api-key sk-test - API key stored for future use
  • uv run server.py --help shows new flags and updated examples
  • ruff format --check and ruff check pass

…server

The MCP server at examples/mcp-query/server.py had three gaps:

1. No mechanism to pass API key for authenticated OpenViking deployments
2. No server-level default for search target_uri, requiring every client
   to specify the scope on each search call
3. Example URLs used "localhost" which fails on Windows where it resolves
   to ::1 while the server binds to 127.0.0.1

This adds --api-key/OV_API_KEY and --default-uri/OV_DEFAULT_URI CLI flags
with backward-compatible empty-string defaults, and fixes the epilog
examples to use 127.0.0.1.

Fixes volcengine#606

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

@qin-ctx qin-ctx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The --default-uri fallback and the localhost127.0.0.1 fix look correct. However, the --api-key feature is not wired up — the stored value is never consumed by any downstream code.

global _config_path, _data_path, _api_key, _default_uri
_config_path = args.config
_data_path = args.data
_api_key = args.api_key
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Bug] (blocking) _api_key is assigned here but never consumed anywhere in the codebase. It is not passed to Recipe(), OpenVikingConfig, ov.SyncOpenViking, or any HTTP header.

This means the --api-key / OV_API_KEY feature is effectively a no-op — users who set it will still get 401 errors when the server has root_api_key configured.

The value needs to be threaded through to whichever layer performs authentication (likely Recipe or the underlying client). If Recipe doesn't currently accept an api_key parameter and this is deferred work, the --api-key flag should be removed from this PR to avoid shipping a non-functional feature that gives users a false sense of security.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: openviking-mcp is not a general-purpose MCP bridge — default paths and schemas are hardcoded to the Cloudwise knowledge base with no disclosure

2 participants