[rghamilton3]

Summary

• enable Supabase OAuth 2.1 with dynamic client registration and add the SPA consent route
• add the FastMCP 4 service with caller-token PostgREST access, RLS-scoped recipe and shopping-list tools, item merging, and sanitized transport errors
• add compensating cleanup for failed child writes, regression coverage, Docker/Compose deployment assets, and connector setup documentation

Verification

• cd mcp && uv lock --check && uv sync --locked && uv run pytest — 4 passed
• cd mcp && uv run python -m compileall -q src tests
• bun run check — 0 errors and 0 warnings
• targeted ESLint and Prettier checks passed
• Docker image built and Compose health check reached healthy
• local Supabase OAuth DCR/consent approval completed with authorization code callback
• end-to-end MCP smoke test created and merged recipe/shopping data under RLS

[github-actions]

Claude finished @rghamilton3's task in 8m 39s —— View job

---

Claude's Review

• Gather context (PR diff, changed files, CLAUDE.md conventions)
• Understand the request (custom instruction: run /code-review on this PR)
• Run code review across changed files
• Post review findings

Reviewed the MCP connector (mcp/) and the new OAuth consent route (src/routes/oauth/consent/+page.svelte). Nice scope on this PR — RLS-scoped tools via caller token, item merging, and compensating cleanup on failed writes are all solid patterns. Found a few real issues, ordered by severity:

1. add_shopping_items silently drops recipe attribution on merge — mcp/src/nexus_mcp/server.py:183-209
When a new item matches an existing PENDING row, existing_rows is only selected with id,name,unit,quantity (no needed_for/category), and the PATCH at line 205-208 only updates quantity. If the same ingredient is requested for two different recipes across two tool calls, the second recipe's attribution is lost — the item's "For: …" list stays stuck on the first recipe, which contradicts the per-recipe attribution the schema (needed_for) is designed to carry.
Fix this →

2. add_shopping_items never checks the target list's status — mcp/src/nexus_mcp/server.py:176-179
status is fetched (select: "id,name,status") but never inspected. Nothing (RLS, DB constraint, or this code) stops the tool from adding PENDING items to a COMPLETED/ARCHIVED list — a stale list_id from an earlier turn would silently resurrect a "done" list. Worth an explicit status check that raises a ToolError for non-open lists.

3. Shopping-item categorization bypasses the app's deterministic categorizer — mcp/src/nexus_mcp/items.py:45
The web app derives category deterministically from item name via src/lib/shopping/categorize.ts. The MCP tool schema instead asks the calling LLM to pick a Category itself (defaulting to OTHER), so items added through the connector can end up categorized differently — or just dumped in OTHER — compared to the same ingredients added through the app.

4. Sequential awaited PATCH calls in the merge loop — mcp/src/nexus_mcp/server.py:200-219
Each matched item issues its own awaited db.patch inside the for item in merged loop, serializing N round-trips to PostgREST instead of batching or asyncio.gather-ing them. Minor for typical list sizes, but worth noting for larger shopping lists.

5. Consent page: .split on possibly-undefined scope — src/routes/oauth/consent/+page.svelte:12-17

const scopes = $derived(
	authorization?.scope
		.split(' ')
		...
);

The ?. only guards authorization being null — if a DCR-registered client's authorization request has no scope (optional in OAuth 2.1), authorization.scope is undefined and .split throws, breaking the consent screen. Should be authorization?.scope?.split(' ').

Note: I also flagged the Dockerfile's base image (astral/uv:0.12.13-python3.13-trixie-slim) as a possible non-existent registry path, but the PR description states the Docker image was built and Compose reached healthy, so that's evidently fine — disregard.

Nothing else stood out — RLS scoping via the caller's own PostgREST token, the cleanup-on-failure pattern in test_cleanup.py, and the sanitized transport error handling all look correct.