Skip to content

fix(mcp): report llm_available via the chat-model credential resolver (#200)#204

Open
wernerkasselman-au wants to merge 1 commit into
NVIDIA:mainfrom
wernerkasselman-au:fix/mcp-llm-available-fallback-creds
Open

fix(mcp): report llm_available via the chat-model credential resolver (#200)#204
wernerkasselman-au wants to merge 1 commit into
NVIDIA:mainfrom
wernerkasselman-au:fix/mcp-llm-available-fallback-creds

Conversation

@wernerkasselman-au

Copy link
Copy Markdown
Contributor

What this fixes

Closes the bug I filed as #200. The MCP server decided whether the semantic pass could run by calling resolve_provider_credentials(), which only ever resolves the active provider's credentials. The model the graph actually constructs, though, comes from create_chat_model(), and that one falls back to a standard OpenAI client (OPENAI_API_KEY / OPENAI_BASE_URL) when the active provider is not configured. So on an OpenAI-only setup (the default NVIDIA provider with no NVIDIA_INFERENCE_KEY, but OPENAI_API_KEY set), the gate came out false, the server reported scan_mode: static-only, and it skipped the LLM pass that the CLI would have run on the very same environment.

The change

One line of behaviour: gate llm_available on resolve_chat_model_credentials() (providers/__init__.py:111-117) instead of resolve_provider_credentials(). That resolver already mirrors the OpenAI fallback that create_chat_model() relies on, so the MCP accounting now lines up with the model path the graph actually takes. I left a comment at the call site explaining why the fallback-aware resolver is the right one, so nobody narrows it back to the active provider later.

Tests

  • Updated the three existing accounting tests to patch the new resolver (resolve_chat_model_credentials), since that is the name the server now consults.
  • Added test_run_scan_reports_llm_available_via_openai_fallback, a regression test that patches the active-provider-only resolver to None and the chat-model resolver to a credential, then asserts the server reports the LLM as available. On the old code this fails (it consulted the active-only resolver), so it pins the fix.
uv run ruff check src/ tests/        # clean
uv run ruff format --check src/ tests/  # clean
uv run pytest tests/unit/test_mcp_server.py -q  # 6 passed

One thing I want to flag, in case it was deliberate: if the MCP path was scoped to the active provider on purpose (say, to keep the server from quietly using a developer's ambient OPENAI_API_KEY), then this change is the wrong call and I would rather hear that and close it. Otherwise the accounting should tell the truth about what the scan can do.

Refs #200

run_scan gated the LLM pass on resolve_provider_credentials(), which resolves
only the active provider's credentials. create_chat_model() falls back to a
standard OpenAI client (OPENAI_API_KEY / OPENAI_BASE_URL) when the active
provider is unconfigured, so an OpenAI-only setup reported llm_available=false
and the semantic pass was skipped even though the CLI would have run it.

Gate on resolve_chat_model_credentials(), which includes that fallback, so the
MCP server's accounting matches the model path the graph actually takes. Update
the existing accounting tests to patch the new resolver and add a regression
test for the OpenAI-fallback case.

Refs NVIDIA#200

Signed-off-by: Werner Kasselman <145896621+wernerkasselman-au@users.noreply.github.com>
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