Repository navigation
Prefer request origin for MCP OAuth resource metadata - #657
Conversation
APP_BASE_URL remains the fallback for background work so email/workflows keep a stable public host after account moves without breaking clients that connect via heykody.dev. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe PR updates base URL resolution to prefer inbound request origins, adjusts callers and tests to use that behavior, initializes SSR auth session secrets before cookie reads, and revises documentation for ChangesRequest-origin-based base URL and SSR auth session initialization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/app/app-base-url.node.test.ts (1)
4-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for malformed-URL fallback branches and
URLinput.The new try/catch fallback paths (invalid
requestUrlstring, invalidAPP_BASE_URLstring) and theURL-instance branch of therequestUrltype aren't exercised by any test here.✅ Suggested additional cases
+test('getAppBaseUrl falls through on malformed requestUrl/APP_BASE_URL', () => { + expect( + getAppBaseUrl({ + env: { APP_BASE_URL: 'https://configured.example' }, + requestUrl: 'not-a-valid-url', + }), + ).toBe('https://configured.example') + + expect( + getAppBaseUrl({ + env: { APP_BASE_URL: 'not-a-valid-url' }, + }), + ).toBe('https://heykody.dev') + + expect( + getAppBaseUrl({ + env: {}, + requestUrl: new URL('https://kody-production.kody-a99.workers.dev/mcp'), + }), + ).toBe('https://kody-production.kody-a99.workers.dev') +})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/app-base-url.node.test.ts` around lines 4 - 39, Add tests for the uncovered fallback branches in getAppBaseUrl: exercise an invalid requestUrl string so the request-origin parsing path falls back cleanly, and an invalid APP_BASE_URL string so the env fallback path returns the default heykody.dev value. Also add a case where requestUrl is passed as a URL instance to verify the URL-specific branch behaves the same as a string input. Use the existing getAppBaseUrl test file and keep the assertions focused on the returned base URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/src/app/app-base-url.node.test.ts`:
- Around line 4-39: Add tests for the uncovered fallback branches in
getAppBaseUrl: exercise an invalid requestUrl string so the request-origin
parsing path falls back cleanly, and an invalid APP_BASE_URL string so the env
fallback path returns the default heykody.dev value. Also add a case where
requestUrl is passed as a URL instance to verify the URL-specific branch behaves
the same as a string input. Use the existing getAppBaseUrl test file and keep
the assertions focused on the returned base URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ac4ee88d-7dc6-4f0c-80f3-f5ad8e243304
📒 Files selected for processing (9)
docs/contributing/architecture/request-lifecycle.mddocs/contributing/setup-manifest.mdpackages/worker/src/app/app-base-url.node.test.tspackages/worker/src/app/app-base-url.tspackages/worker/src/email/package-subscriptions.tspackages/worker/src/execute-maintenance.tspackages/worker/src/mcp-auth.workers.test.tspackages/worker/src/mcp/generated-ui-app-base-url.node.test.tspackages/worker/src/package-runtime/package-workflows.ts
💤 Files with no reviewable changes (2)
- packages/worker/src/email/package-subscriptions.ts
- packages/worker/src/package-runtime/package-workflows.ts
|
🔎 Preview deployed: https://kody-pr-657.kody-a99.workers.dev Worker: Mocks:
|
renderAppPage now configures COOKIE_SECRET before reading session cookies so /oauth/authorize no longer 500s for Cursor (and other MCP clients) that retain cookies from prior visits. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
getAppBaseUrlso MCP protected-resource metadata matches the host clients connect to;APP_BASE_URL/https://heykody.devremain the fallback for background work.renderAppPagenow configuresCOOKIE_SECRETbefore reading session cookies so/oauth/authorizeno longer 500s when the browser sends any Cookie header (this was blocking Cursor reconnect after the account move).APP_BASE_URL=https://heykody.devand redeployed; leftover KCD cleanup tracked in Clean up leftover Kody resources on the old KCD Cloudflare account #656.Test plan
getAppBaseUrland MCP auth metadatarenderAppPagesucceeds with a Cookie header when session secret was not preconfigured/oauth/authorizereturns 200 with and without Cookie headersPreview
https://kody-pr-657.kody-a99.workers.dev
System recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@158aac1d· Head:2d0b6cf1Classification: extends — changes public origin resolution for MCP OAuth and fixes authorize SSR cookie init; no new primitives.
Primitives touched
mcp-oauthapp-uirenderAppPageinitializes session secret for non-appHandler SSR entriesmcp-servergetAppBaseUrlworkflows/emailSystem map
Review focus
/oauth/authorizewith Cookie headers returns the SPA, not the standalone 500 HTML.