fix(mcpcatalog): tell the model to proceed after enabling an OAuth server

[trungutt]

Problem

When a user asks something that requires an OAuth-protected catalog
server (e.g. "List all pages by David Gageot in Notion"), the model:

1. calls search_remote_mcp_servers,
2. calls enable_remote_mcp_server for notion,
3. receives a tool result ending with "auth: OAuth — an authorization URL will be elicited on the next turn.",
4. then narrates to the user "OAuth will happen on your next request, please ask me again" and stops.

The user must repeat their original prompt to actually get an answer:

The Notion server is now enabled and ready to use. When you make your next request, it will initiate OAuth authorization. […] Would you like me to proceed with searching your Notion workspace for pages and David Gageot's comments?

Why this happens

The runtime actually drives the OAuth flow inside the same loop iteration:

• processToolCalls runs handleEnable, which only registers the toolset.
• reprobe (pkg/runtime/loop.go:608-610) calls getTools → Tools() on the catalog → first-call lazy Start() on the freshly enabled toolset (mcpcatalog.go:371-372).
• Start() runs the MCP handshake, the 401 triggers the OAuth elicitation, the user grants, Start() returns success and the server's tools become available.
• The next loop iteration calls the model with the new tools already in the prompt.

So by the time the next LLM turn begins, OAuth is already complete. The model just doesn't know that — it only sees the previous tool result, which literally said "elicited on the next turn".

Fix

This PR rewrites the two strings the model uses to reason about the post-enable state:

1. handleEnable's OAuth-branch success message now tells the model the host triggers any required authorization transparently the first time the server's tools are used, and that the model should continue with the user's original request now.

2. Instructions() step 3 is hardened from a soft "Use the newly activated tools as you would any other" into an explicit "Immediately proceed with the user's ORIGINAL request […]; do NOT stop and ask the user to repeat themselves; do NOT narrate the OAuth flow".

No runtime behaviour changes; this is a prompt fix only

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This is a prompt-only fix that rewrites two LLM-facing instruction strings in mcpcatalog.go. No runtime logic is changed. Two findings surfaced during review — both are edge-case wording concerns rather than code bugs.

[rumpl]

Hold on, why does the LLM need to know that there is an oauth thing?

[trungutt]

@rumpl Good question — and you're right, it shouldn't. The original wording (auth: OAuth — elicited on the next turn) caused this bug precisely because it leaked OAuth as a concept the model then tried to reason about. My first patch only rewrote that sentence to be less misleading; that left the leaky abstraction in place.

c3c85b3 removes OAuth as an LLM-visible concept from the catalog entirely:

• handleEnable no longer branches on server.Auth.Type. The success message is a single line — "the server's tools will appear on your next turn. Proceed with the user's original request; the host handles any required connection setup." The only conditional emission left is the WARNING: env var(s) NOT set line, which is the one case the model has actual agency (it can ask the user to set the var and retry).
• Instructions() drops "Authentication (OAuth or API key) is deferred" from step 2, drops "do not narrate the OAuth flow" from step 3 (the model no longer learns OAuth exists, so the instruction can just say "do not narrate connection setup"), and reframes step 5 (reset_auth) as "if a previously enabled server starts rejecting requests, clear any persisted credentials" — no mention of OAuth tokens or authorization URLs.
• enable_remote_mcp_server and reset_remote_mcp_server_auth tool descriptions, ResetAuthArgs jsonschema, and handleResetAuth result strings all lose every OAuth/authorization reference.

Internal field names (oauthSuccessHandler, managedOAuth, …), Go package-doc comments, and slog debug messages still say OAuth — those are developer-facing and removing them would obscure intent for anyone reading the package.

A new test assertion (NotContains("OAuth"), NotContains("authorization") in TestEnableAddsServerAndFiresChangeNotification) guards against re-adding the leak.

PTAL.

[docker-agent]

Assessment: 🟢 APPROVE

This is a prompt/message-only fix that removes OAuth-specific wording from LLM-facing tool results and instructions so the model proceeds with the user's original request instead of stopping to ask them to repeat themselves.

What was reviewed:

• pkg/tools/builtin/mcpcatalog/mcpcatalog.go — handleEnable message consolidation, handleResetAuth message updates, Instructions() step 3 hardening, tool description rewrites
• pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go — updated string assertions to match new production strings

Findings summary:

• ✅ No high or medium severity bugs found
• ✅ Auth-type consolidation in handleEnable correctly preserves env-var warnings for all relevant auth types (api_key, custom headers); the removed OAuth-specific branch was informational-only and its removal is intentional
• ✅ All updated test assertions ("enabled", "cleared credentials", "no persisted credentials", assert.NotContains for "OAuth" / "authorization") correctly match the new production string literals
• ℹ️ Minor (not flagged): handleResetAuth's early-exit message "has no persisted credentials" is slightly imprecise for api_key servers (they have credentials, just not ones stored by this tool) — worth a follow-up comment, but not a bug and consistent with the PR's intent of not leaking auth-type details to the model

The fix correctly addresses the root cause: the previous "auth: OAuth — an authorization URL will be elicited on the next turn" string was the signal causing models to pause and narrate instead of proceeding. The new wording and explicit step-3 instruction ("do NOT stop and ask the user to repeat themselves") should resolve the reported behavior.

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

One medium-severity issue found in the changed code. The rest of the prompt-text changes look correct and well-motivated.

[docker-agent]

[MEDIUM] Contradictory model instructions when API-key env vars are missing

When missing is non-empty, the message assembled in handleEnable simultaneously tells the model two conflicting things:

1. "Proceed with the user's original request; the host handles any required connection setup."
2. "WARNING: the following env var(s) are NOT set: … Ask the user to set them, then call disable_remote_mcp_server and enable_remote_mcp_server for this id again, otherwise tool calls will fail."

An LLM receiving both instructions in the same tool-result has no explicit prioritisation signal — the Proceed clause comes first and is unconditional, while the WARNING appears later but doesn't say "ignore the previous instruction". Depending on which instruction the model weighs more heavily it may either:

• dutifully proceed and immediately fail every downstream tool call (env vars not set), or
• correctly pause and ask the user to set the vars but now via a path the new wording was designed to remove.

Suggested fix: move the "Proceed with the user's original request" sentence into the else branch (i.e., only emit it when len(missing) == 0), so it is never paired with a WARNING that contradicts it:

msg := strings.Builder{}
fmt.Fprintf(&msg, "enabled %q (%s) — the server's tools will appear on your next turn.\n", id, server.Title)
fmt.Fprintf(&msg, "endpoint: %s\n", server.URL)
if len(missing) > 0 {
    fmt.Fprintf(&msg, "WARNING: the following env var(s) are NOT set: %s. Ask the user to set them, then call %s and %s for this id again, otherwise tool calls will fail.\n",
        strings.Join(missing, ", "), ToolNameDisable, ToolNameEnable)
} else {
    fmt.Fprintf(&msg, "Proceed with the user's original request; the host handles any required connection setup.\n")
}