MCP OAuth Stage 8: user-facing docs (#98) - #144
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive documentation for the experimental MCP OAuth feature, enabling the plugin to act as an OAuth 2.1 authorization server for Model Context Protocol clients. The updates span the README, configuration reference, lifecycle hooks, and a new dedicated guide detailing the flow, endpoints, and the withMCPAuth wrapper. Feedback suggests adding the package property to the README's configuration snippet to ensure consistency and correct plugin resolution.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4011523 to
aaeb8df
Compare
0e6303e to
55095af
Compare
gemini review on #144: add `package: '@harperfast/oauth'` to the MCP quickstart's config.yaml so it matches the main Quick Start / configuration.md examples and loads correctly when copy-pasted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Developer Experience (DevExp) ReviewThis documentation PR is outstanding and extremely thorough. It anticipates real-world integration hurdles (such as multi-component Here are a few highly focused, non-blocking suggestions to refine the developer experience and achieve absolute documentation perfection: 1. Harmonize timing/execution language for
|
…uage DevExp review on #144 (Antigravity/Gemini), non-blocking: 1. lifecycle-hooks.md said the hook fires "before the response returns" — contradicting mcp-oauth.md's not-awaited/detached wording. Harmonized to the fire-and-forget language (it runs detached; side effects may complete after the client has the token). 2. Dropped the unused `next` param from the mcp-oauth.md quickstart handler (a leaf urlPath route doesn't use it). 3. Added a comment noting MCP messages are JSON-RPC 2.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
All three addressed in |
gemini review on #144: the quickstart handler returned a bare { jsonrpc, result } object, but Harper HTTP listeners return { status, body, headers? } (as the README and the integration fixture do). Wrap the JSON-RPC response as the body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
claude review on #144: the quickstart block (labeled resources.ts) calls server.http() but imported only withMCPAuth — copy-pasting it verbatim would throw ReferenceError: server is not defined. Add `import { server } from 'harper'`, matching the README quickstart. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…handler gemini review on #144 (non-blocking): make explicit that request.mcp is defined inside a withMCPAuth-guarded handler (the guard rejects missing/invalid tokens first), so strict-TS users don't need optional chaining. Clarified the quickstart comments in mcp-oauth.md + README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
heskew
left a comment
There was a problem hiding this comment.
docs/mcp-oauth.md:161 has the path-appended Protected Resource Metadata URL backwards. For a resource like https://host/mcp, the implementation builds and serves /.well-known/oauth-protected-resource/mcp (PRM_PATH + resource path), and withMCPAuth advertises that form in WWW-Authenticate. The current text says /mcp/.well-known/oauth-protected-resource, which sends users/proxies to the wrong route. Please flip the example to /.well-known/oauth-protected-resource/mcp.
🤖 Posted by Codex (gpt-5.5) on Nathan's behalf
Codex review on #144: the RFC 9728 path-appended Protected Resource Metadata URL was written `/mcp/.well-known/oauth-protected-resource`, but the implementation (wellKnown.ts `protectedResourceMetadataUrl` = origin + PRM_PATH + resourcePath) serves `/.well-known/oauth-protected-resource/mcp` — the well-known segment sits between the origin and the resource path. Flipped both occurrences (the discovery note + the WWW-Authenticate example) to the correct form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed in |
…ples gemini review on #144 (non-blocking, devexp): the onMCPTokenIssued examples use `tables.McpClient` — note that `tables` is a Harper global (so it isn't imported) and `McpClient` is an illustrative app-owned table the plugin doesn't provide. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the app-author documentation for MCP OAuth, with docs/mcp-oauth.md as the single deep guide (flow diagram, endpoint reference, the withMCPAuth wrapper + both registration models + options + cross-component use, the onMCPTokenIssued hook, audit events, production-deployment checklist, troubleshooting, and a hand-rolled-server migration guide). Links the MCP spec (2025-06-18) and RFCs 6749/6750/7591/7636/8252/8414/8707/9728. Rebased onto main after #134 merged. #134 had already added withMCPAuth sections to README and configuration.md; those are trimmed here to short pointers into docs/mcp-oauth.md so the wrapper isn't documented in three places. configuration.md also: drops the "(work in progress)" marker, documents signingKeyPem/signingAlgorithm/accessTokenTtl/refreshTokenTtl, and fixes the JWKS note. docs/lifecycle-hooks.md documents onMCPTokenIssued. Documents onMCPTokenIssued (#141), so merge this after #141. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the hook's behavior after #141: it runs detached and is not awaited, so it never delays/blocks the token response — and its side effects may complete after the client already has the token. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gemini review on #144: add `package: '@harperfast/oauth'` to the MCP quickstart's config.yaml so it matches the main Quick Start / configuration.md examples and loads correctly when copy-pasted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…uage DevExp review on #144 (Antigravity/Gemini), non-blocking: 1. lifecycle-hooks.md said the hook fires "before the response returns" — contradicting mcp-oauth.md's not-awaited/detached wording. Harmonized to the fire-and-forget language (it runs detached; side effects may complete after the client has the token). 2. Dropped the unused `next` param from the mcp-oauth.md quickstart handler (a leaf urlPath route doesn't use it). 3. Added a comment noting MCP messages are JSON-RPC 2.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gemini review on #144: the quickstart handler returned a bare { jsonrpc, result } object, but Harper HTTP listeners return { status, body, headers? } (as the README and the integration fixture do). Wrap the JSON-RPC response as the body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
claude review on #144: the quickstart block (labeled resources.ts) calls server.http() but imported only withMCPAuth — copy-pasting it verbatim would throw ReferenceError: server is not defined. Add `import { server } from 'harper'`, matching the README quickstart. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…handler gemini review on #144 (non-blocking): make explicit that request.mcp is defined inside a withMCPAuth-guarded handler (the guard rejects missing/invalid tokens first), so strict-TS users don't need optional chaining. Clarified the quickstart comments in mcp-oauth.md + README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex review on #144: the RFC 9728 path-appended Protected Resource Metadata URL was written `/mcp/.well-known/oauth-protected-resource`, but the implementation (wellKnown.ts `protectedResourceMetadataUrl` = origin + PRM_PATH + resourcePath) serves `/.well-known/oauth-protected-resource/mcp` — the well-known segment sits between the origin and the resource path. Flipped both occurrences (the discovery note + the WWW-Authenticate example) to the correct form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ples gemini review on #144 (non-blocking, devexp): the onMCPTokenIssued examples use `tables.McpClient` — note that `tables` is a Harper global (so it isn't imported) and `McpClient` is an illustrative app-owned table the plugin doesn't provide. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
b42e504 to
83c8442
Compare
Closes #98 — Stage 8 of the MCP OAuth epic (#86). App-author documentation for MCP OAuth.
Its dependencies — #134 (
withMCPAuth) and #141 (audit +onMCPTokenIssued) — are both merged, so this is unblocked. Docs-only; merges cleanly into currentmain(#141 touched no docs).What's added / changed
docs/mcp-oauth.md(new) — the single deep guide: end-to-end flow diagram, endpoint reference (discovery / authorize / token / JWKS), thewithMCPAuthwrapper (both registration models, options, cross-component use viagetConfig), theonMCPTokenIssuedhook, audit events (all three types, incl.oauth.mcp.token.rejected), a production-deployment checklist, troubleshooting, and a migration guide from a hand-rolled MCP authorization server. Links the MCP authorization spec (2025-06-18) and RFCs 6749/6750/7591/7636/8252/8414/8707/9728.README.md— MCP OAuth section trimmed to a quickstart (config +withMCPAuth) plus a pointer todocs/mcp-oauth.md; added the doc to the index.docs/configuration.md— dropped the "(work in progress)" marker; documentedsigningKeyPem,signingAlgorithm,accessTokenTtl,refreshTokenTtl; fixed the JWKS note; trimmed the inlinewithMCPAuthhow-to to a pointer + a security note.docs/lifecycle-hooks.md— documentedonMCPTokenIssued(the MCP-client analog ofonLogin), reflecting MCP OAuth Stage 6: audit logging + onMCPTokenIssued hook (#96) #141's fire-and-forget / not-awaited behavior.Design: single source of truth
#134 had already landed
withMCPAuthwalkthroughs in bothREADME.mdanddocs/configuration.md. To avoid three copies that drift, the deep wrapper/flow/hook material now lives once indocs/mcp-oauth.md;README.mdandconfiguration.mdcarry short pointers into it (so those two files are net-trimmed here).Notes
prettier --checkclean; all internal anchors resolve.🤖 Generated with Claude Code