fix(mcp): ignore a token cache that is not a JSON object 🤖🤖🤖

[sushant-mishra-dtu]

What this fixes

_load_cached_token() promises in its own docstring to load a cached token "if present
and well-formed", but it only validates the entry, never the top-level document:

def _load_cached_token(server_url: str) -> OAuthToken | None:
    """Load a cached OAuth token for ``server_url`` if present and well-formed."""
    path = _token_cache_path()
    try:
        data = json.loads(path.read_text())
    except (OSError, ValueError):
        return None
    entry = data.get(server_url)          # src/nooa/mcp/oauth.py:799

json.loads returns a list, str, int or None for any valid-but-non-object file.
data.get then raises AttributeError, which the (OSError, ValueError) guard does not
cover — so instead of falling back to a fresh OAuth flow, the exception escapes into the
MCP connection path and the server simply fails to connect. Recovering requires the user
to find and delete .nooa/mcp_tokens.json by hand.

The inconsistency

Its sibling _save_cached_token() guards the identical read, twenty lines below at
oauth.py:818-820:

try:
    data = json.loads(path.read_text())
    if not isinstance(data, dict):
        data = {}
except (OSError, ValueError):
    data = {}

So the write path already treats a non-object cache as recoverable. Only the read path
does not. This change brings the two into line.

Reproduction

  JSON array   -> RAISED AttributeError: 'list' object has no attribute 'get'
  JSON string  -> RAISED AttributeError: 'str' object has no attribute 'get'
  JSON null    -> RAISED AttributeError: 'NoneType' object has no attribute 'get'

the sibling save path handles the same input:
  _save_cached_token -> OK (isinstance guard at oauth.py:819-820)

The fix

Two lines, mirroring the sibling:

if not isinstance(data, dict):
    return None

A cache file that is not a JSON object is now treated exactly like a missing or corrupt
one — return None and let the caller start a fresh OAuth flow. No signature change, and
no behaviour change for a well-formed cache.

Test

test_load_cached_token_ignores_non_object_cache, parametrized over the three shapes
([], "nope", null), placed beside the existing test_token_cache_roundtrip.
Verified to fail on the unfixed tree before being kept:

FAILED tests/test_mcp/test_oauth_discovery.py::test_load_cached_token_ignores_non_object_cache[array-[]]
FAILED tests/test_mcp/test_oauth_discovery.py::test_load_cached_token_ignores_non_object_cache[string-"nope"]
FAILED tests/test_mcp/test_oauth_discovery.py::test_load_cached_token_ignores_non_object_cache[null-null]
E   AttributeError: 'NoneType' object has no attribute 'get'
src/nooa/mcp/oauth.py:799: AttributeError

Scope

Only the guard. Two adjacent observations in this file are deliberately left alone as
separate concerns: _save_cached_token writes the cache with a plain write_text()
rather than the tmp+replace() pattern used elsewhere, and the read_text() calls here
carry no explicit encoding. Happy to raise either separately if useful.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8a575b55-605b-40f0-af7a-a493880c0716

📥 Commits

Reviewing files that changed from the base of the PR and between e137e1b and 9ee9e20.

📒 Files selected for processing (2)

• src/nooa/mcp/oauth.py
• tests/test_mcp/test_oauth_discovery.py

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.

---

📝 Walkthrough

Walkthrough

The OAuth token cache loader now treats non-dictionary JSON contents as cache misses. Parameterized tests cover arrays, strings, and null.

Changes

OAuth cache validation

Layer / File(s)	Summary
Validate cached token shape src/nooa/mcp/oauth.py, tests/test_mcp/test_oauth_discovery.py	_load_cached_token returns None when the parsed cache content is not a dictionary. Regression tests cover array, string, and null payloads.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 9ee9e

Non-object OAuth token-cache JSON now safely behaves as a cache miss instead of interrupting authentication, with regression coverage for the affected JSON shapes. The change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: ignoring token-cache files whose JSON root is not an object. The emojis add minor noise but do not reduce clarity.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}