Skip to content

Conversation

@julien-c
Copy link
Member

@julien-c julien-c commented May 13, 2025

in case we pass a custom baseURL, then HF_TOKEN is actually not required.

@julien-c julien-c requested review from Copilot, evalstate and gary149 May 13, 2025 17:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes the HF_TOKEN environment variable optional by updating type definitions, client instantiation, CLI error handling, and documentation.

  • Updated McpClient and Agent type annotations to allow an optional API key.
  • Modified the CLI to require HF_TOKEN only when no ENDPOINT_URL is provided.
  • Revised README to reflect the optional nature of HF_TOKEN.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/mcp-client/test/McpClient.spec.ts Removed fallback to an empty string for apiKey in tests
packages/mcp-client/src/McpClient.ts Updated the apiKey parameter type to be optional and passed to InferenceClient
packages/mcp-client/src/Agent.ts Updated the apiKey parameter type to be optional
packages/mcp-client/cli.ts Changed CLI error handling to check for ENDPOINT_URL or HF_TOKEN
packages/mcp-client/README.md Updated documentation to denote HF_TOKEN as optional
Comments suppressed due to low confidence (4)

packages/mcp-client/test/McpClient.spec.ts:13

  • Previously, a fallback empty string was provided if HF_TOKEN was unset. Verify that passing undefined instead of an empty string does not lead to unexpected behavior in tests.
apiKey: process.env.HF_TOKEN,

packages/mcp-client/src/McpClient.ts:53

  • Since apiKey is now optional, ensure that the InferenceClient properly handles an undefined value or consider implementing a default value if necessary.
apiKey?: string;

packages/mcp-client/src/Agent.ts:69

  • As with McpClient, ensure that any usage of an undefined apiKey is handled gracefully, particularly when interfacing with services expecting a non-null token.
apiKey?: string;

packages/mcp-client/cli.ts:65

  • Confirm that the new condition accurately covers scenarios where an endpoint is provided but HF_TOKEN is not; ensure that local inference providers using ENDPOINT_URL are fully supported.
if (!ENDPOINT_URL && !process.env.HF_TOKEN) {

@julien-c julien-c requested a review from Wauplin May 13, 2025 17:02
@julien-c julien-c merged commit 70b41a6 into main May 15, 2025
5 checks passed
@julien-c julien-c deleted the mcp-client-token-not-required-for-local-models branch May 15, 2025 10:25
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.

4 participants