Skip to content

Commit ecbc7d4

Browse files
fix: MCP OAuth token refresh, double DCR, and gpt_oss tool result error
- Persist discovered token_url in TokenData so token refresh works for servers using Dynamic Client Registration (fixes empty URL error) - Reuse existing registered_client_id in OAuth start flow to prevent redundant DCR on repeated authentication attempts - Only unpack tool result JSON content back to dict (not list/scalar) for gpt_oss, preventing Jinja2 |items filter failure on non-mappings Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 07e4fdd commit ecbc7d4

File tree

4 files changed

+42
-19
lines changed

4 files changed

+42
-19
lines changed

omlx/admin/routes.py

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4094,18 +4094,25 @@ async def start_mcp_authenticate(name: str, request: Request, is_admin: bool = D
40944094
if not token_url:
40954095
token_url = metadata.get("token_endpoint", "")
40964096
if not client_id:
4097-
reg_endpoint = metadata.get("registration_endpoint")
4098-
if not reg_endpoint:
4099-
raise HTTPException(
4100-
status_code=400,
4101-
detail="OAuth server does not advertise a registration_endpoint. "
4102-
"Set client_id explicitly in the server auth config.",
4103-
)
4104-
try:
4105-
client_id = await _register_dynamic_client(reg_endpoint, redirect_uri)
4097+
# Reuse a previously registered client_id if available (avoid redundant DCR)
4098+
from ..mcp.token_store import TokenStore
4099+
existing_token = TokenStore(cfg.auth.token_store if cfg.auth else None).load(name)
4100+
if existing_token and existing_token.registered_client_id:
4101+
client_id = existing_token.registered_client_id
41064102
registered_client_id = client_id
4107-
except Exception as exc:
4108-
raise HTTPException(status_code=502, detail=f"Dynamic Client Registration failed: {exc}")
4103+
else:
4104+
reg_endpoint = metadata.get("registration_endpoint")
4105+
if not reg_endpoint:
4106+
raise HTTPException(
4107+
status_code=400,
4108+
detail="OAuth server does not advertise a registration_endpoint. "
4109+
"Set client_id explicitly in the server auth config.",
4110+
)
4111+
try:
4112+
client_id = await _register_dynamic_client(reg_endpoint, redirect_uri)
4113+
registered_client_id = client_id
4114+
except Exception as exc:
4115+
raise HTTPException(status_code=502, detail=f"Dynamic Client Registration failed: {exc}")
41094116

41104117
code_verifier = _generate_code_verifier()
41114118
code_challenge = _generate_code_challenge(code_verifier)
@@ -4194,6 +4201,8 @@ async def mcp_oauth_callback(
41944201
)
41954202
if session.get("registered_client_id"):
41964203
token.registered_client_id = session["registered_client_id"]
4204+
if session.get("token_url"):
4205+
token.token_url = session["token_url"]
41974206

41984207
TokenStore().save(session["server_name"], token)
41994208
logger.info("MCP OAuth complete for server '%s'", session["server_name"])

omlx/mcp/oauth.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,14 @@ async def _do_refresh(
431431
payload["scope"] = " ".join(auth_config.scopes)
432432

433433
token_url = auth_config.token_url
434+
if not token_url and stored_token and stored_token.token_url:
435+
token_url = stored_token.token_url
436+
if not token_url:
437+
raise OAuthError(
438+
"Cannot refresh token: no token_url available. "
439+
"Re-authenticate with 'omlx mcp login'."
440+
)
441+
434442
async with httpx.AsyncClient() as client:
435443
resp = await client.post(
436444
token_url,
@@ -447,6 +455,9 @@ async def _do_refresh(
447455
# Propagate the registered client_id so it is persisted on re-save
448456
if stored_token and stored_token.registered_client_id:
449457
token.registered_client_id = stored_token.registered_client_id
458+
# Propagate the discovered token_url so refresh keeps working
459+
if stored_token and stored_token.token_url:
460+
token.token_url = stored_token.token_url
450461
return token
451462

452463

omlx/mcp/token_store.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ class TokenData:
3636
# Persisted client_id obtained via Dynamic Client Registration (DCR).
3737
# Set when the server uses RFC 7591 auto-registration (e.g. Notion remote MCP).
3838
registered_client_id: Optional[str] = None
39+
# Token endpoint URL discovered via RFC 8414 metadata.
40+
# Persisted so token refresh works even when auth_config.token_url is empty.
41+
token_url: Optional[str] = None
3942

4043
@property
4144
def is_expired(self) -> bool:

omlx/server.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,15 +2093,15 @@ async def create_chat_completion(
20932093
# Append tool result turns
20942094
for result_msg in tool_result_msgs:
20952095
msg = dict(result_msg)
2096-
# Harmony chat_template applies |tojson to content — keep as dict/list.
2097-
# Guard against json.loads returning None (e.g. "null") which would
2098-
# cause a TypeError in Jinja2 `in` containment checks on the template.
2096+
# Harmony chat_template applies |tojson to content — keep as dict.
2097+
# Only unpack JSON objects (dicts); lists and scalars stay as JSON
2098+
# strings to avoid the Jinja2 |items filter failing on non-mappings.
20992099
if engine.model_type == "gpt_oss":
21002100
content = msg.get("content", "")
21012101
if isinstance(content, str):
21022102
try:
21032103
parsed = json.loads(content)
2104-
if parsed is not None:
2104+
if isinstance(parsed, dict):
21052105
msg["content"] = parsed
21062106
except (json.JSONDecodeError, ValueError):
21072107
pass
@@ -2788,15 +2788,15 @@ async def stream_chat_completion(
27882788
# Append tool result turns
27892789
for result_msg in tool_result_msgs:
27902790
msg = dict(result_msg)
2791-
# Harmony chat_template applies |tojson to content — keep as dict/list.
2792-
# Guard against json.loads returning None (e.g. "null") which would
2793-
# cause a TypeError in Jinja2 `in` containment checks on the template.
2791+
# Harmony chat_template applies |tojson to content — keep as dict.
2792+
# Only unpack JSON objects (dicts); lists and scalars stay as JSON
2793+
# strings to avoid the Jinja2 |items filter failing on non-mappings.
27942794
if engine.model_type == "gpt_oss":
27952795
content_val = msg.get("content", "")
27962796
if isinstance(content_val, str):
27972797
try:
27982798
parsed = json.loads(content_val)
2799-
if parsed is not None:
2799+
if isinstance(parsed, dict):
28002800
msg["content"] = parsed
28012801
except (json.JSONDecodeError, ValueError):
28022802
pass

0 commit comments

Comments
 (0)