Repository navigation
feat(ai): expose OpenRouter free tier as a user-selectable provider - #1397
Conversation
The openrouter_free provider was fully wired throughout the codebase (config, key resolution, backend factory, chat route, capabilities detection, sidebar UI) but was masked by USER_VISIBLE_PROVIDERS in the cloud settings API, so users could never select it. Add it to the allow-list — its models all carry the `:free` suffix and never incur PageSpace-side cost, so it's safe to expose alongside pagespace. https://claude.ai/code/session_01AjAWKABbABoZcfvUUJvPau
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 10 minutes and 40 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughExpose Changesopenrouter_free user-visible exposure
Dynamic openrouter_free models (API + UI + tests)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da4f211382
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const USER_VISIBLE_PROVIDERS = new Set(['pagespace']); | ||
| // `openrouter_free` is the OpenRouter free tier — all models carry the `:free` suffix | ||
| // and never count against PageSpace's paid quota, so it is safe to expose alongside pagespace. | ||
| const USER_VISIBLE_PROVIDERS = new Set(['pagespace', 'openrouter_free']); |
There was a problem hiding this comment.
:free models before exposing openrouter_free
Adding openrouter_free to the cloud allow-list makes it selectable, but this route still accepts any non-empty model string for that provider (apps/web/src/app/api/ai/settings/route.ts lines 132-138), and provider creation later forwards openrouter_free directly to the OpenRouter backend without model-tier checks (apps/web/src/lib/ai/core/provider-factory.ts lines 105-109). In cloud mode, an authenticated user can now save provider: "openrouter_free" with a paid model ID (for example a non-:free model), which violates the “never incurs cost” assumption and can trigger unexpected OpenRouter charges.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in both the settings route and provider-factory.ts (both were already patched in the earlier commits). To complete the picture: added test case 'returns 400 when openrouter_free is used with a non-free model' that covers exactly the bypass path described here — sending a non-:free model ID to the PATCH endpoint now returns 400 with an error containing ':free'.
- Add GET /api/ai/openrouter/models route that fetches live
free-tier models from OpenRouter API with 1h revalidation cache
- Extract pure filterFreeModels function (id :free suffix +
zero prompt pricing); 17 tests green
- Wire openrouter_free into ProviderModelSelector alongside
existing ollama/lmstudio dynamic fetch pattern
- Replace static hardcoded model list with empty {} in config
- Guard openrouter_free against paid model IDs in settings
route and provider-factory (defense in depth)
- Add next/server stub + @pagespace/lib/* alias to vitest config
so route handler tests can run (pre-existing broken env)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review🐛 Bug — switching to
|
2witstudios
left a comment
2witstudios
left a comment
There was a problem hiding this comment.
🔬 Code Review
Branch: pu/1397 | PR: feat(ai): dynamic openrouter_free model list
Summary
The core feature work is solid: the new /api/ai/openrouter/models route cleanly mirrors the ollama/lmstudio pattern, the filterFreeModels pure function is well-tested, and the frontend wiring in ProviderModelSelector is correct. Five issues flagged below, one of which is a security concern that needs a response before merge.
Findings
🔴 Security — Removal of revokeAllUserSessions (admin magic-link route)
apps/admin/src/app/api/auth/magic-link/verify/route.ts removes:
await sessionService.revokeAllUserSessions(userId, magic_link_login);This was a defense-in-depth measure: on magic-link login, all existing sessions were revoked, preventing a stolen session token from remaining valid after re-authentication. Removing it means concurrent sessions persist through a magic-link sign-in event. Please confirm this removal is intentional and describe the reason (e.g., needed for multi-device admin access, caused race conditions, etc.). If intentional, consider replacing with a narrower policy (e.g., revoke only non-current-device sessions) rather than removing entirely.
🟡 Convention violation — @pagespace/lib/* regex alias in vitest.config.ts
The new alias:
{ find: /^@pagespace\/lib\/(.*)$/, replacement: path.resolve(__dirname, ../../packages/lib/src/) }violates the project convention: "No vitest aliases — use bun native resolution; proper package.json exports + bun is always sufficient; never add resolve.alias to vitest configs for workspace packages." The alias was added because the test imports @pagespace/lib/logging/logger-config directly. The correct fix is to ensure packages/lib/package.json exports that path, then remove this alias.
🟡 console.error instead of structured logger in ProviderModelSelector
} catch (error) {
console.error(Failed to fetch OpenRouter free models:', error);
}
console.errorloggers.*filterFreeModelstypescript
.reduce<Record<string, string>>((acc, m) => ({ ...acc, [m.id]: m.name }), {});
Object.fromEntries(models.filter(...).map(m => [m.id, m.name]))typescript
headers.set('Location", /);
return new Response(null, { status: 302, headers });The original used an absolute URL (new URL(/, getAdminUrl())). The relative "/" should resolve correctly since the admin app runs on its own origin, but worth confirming this works across reverse-proxy configurations (e.g., Caddy stripping the Host header). Low risk, worth a quick smoke test.
OWASP Top 10 Scan
| # | Vulnerability | Status |
|---|---|---|
| A01 | Broken Access Control | revokeAllUserSessions removal above |
| A02 | Cryptographic Failures | ✅ No changes |
| A03 | Injection | ✅ API key in header only; no interpolation into URLs |
| A04 | Insecure Design | ✅ 503 on unconfigured provider is acceptable |
| A05 | Security Misconfiguration | ✅ No secrets committed; API key sourced from managed config |
| A06 | Vulnerable Components | ✅ No new dependencies |
| A07 | Auth Failures | |
| A08 | Data Integrity | ✅ No pipeline changes |
| A09 | Logging & Monitoring | ✅ Logging present on happy path and errors |
| A10 | SSRF | ✅ URL is hardcoded (openrouter.ai); no user-controlled URL |
What works well
- Clean extraction of
filterFreeModelsas an exported pure function — makes the unit tests crisp - 1h Next.js
revalidatecache on the OpenRouter fetch — right tradeoff between freshness and rate-limit safety - Defense-in-depth
:freesuffix guard inprovider-factory.ts— good belt-and-suspenders with the API settings validation - Test coverage is thorough: pure function, route auth/config/success/failure/unexpected-error paths all covered
next-server-stub.tsis minimal and correct — just enough to makeNextResponse.jsonwork in tests
Verdict: Approve pending resolution of the revokeAllUserSessions removal question and the vitest.config.ts alias convention violation.
| models | ||
| .filter(m => m.id.endsWith(':free') && m.pricing?.prompt === '0') | ||
| .reduce<Record<string, string>>((acc, m) => ({ ...acc, [m.id]: m.name }), {}); | ||
|
|
There was a problem hiding this comment.
Nit: reduce with spread creates a new object on every iteration (O(n²) allocations). At ~100–300 models this is harmless, but Object.fromEntries is cleaner:
export const filterFreeModels = (models: OpenRouterModel[]): Record<string, string> =>
Object.fromEntries(
models
.filter(m => m.id.endsWith(":free") && m.pricing?.prompt === "0")
.map(m => [m.id, m.name])
);There was a problem hiding this comment.
Fixed — switched to Object.fromEntries in filter-utils.ts.
| { find: 'next/server', replacement: path.resolve(__dirname, 'src/test/next-server-stub.ts') }, | ||
| { | ||
| find: /^@pagespace\/lib\/(.*)$/, | ||
| replacement: path.resolve(__dirname, '../../packages/lib/src/$1'), |
There was a problem hiding this comment.
Convention violation: project rule says "No vitest aliases — use bun native resolution; proper package.json exports + bun is always sufficient; never add resolve.alias to vitest configs for workspace packages." This regex alias was added because the test imports @pagespace/lib/logging/logger-config directly. The correct fix is to add that subpath export to packages/lib/package.json and remove this alias. The next/server stub alias on the line above is fine (it is a test-environment shim, not a workspace package).
There was a problem hiding this comment.
Removed the @pagespace/lib regex alias from vitest.config.ts. The ./logging/logger-config subpath export already exists in packages/lib/package.json, so bun resolves it natively in tests without the alias.
| } | ||
| } catch (error) { | ||
| console.error('Failed to fetch OpenRouter free models:', error); | ||
| } finally { |
There was a problem hiding this comment.
Minor: console.error here is consistent with the equivalent fetchOllamaModels / fetchLMStudioModels blocks in this component, so no regression — but worth noting for a future cleanup pass to align with the server-side structured-logger pattern.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/app/api/ai/openrouter/models/route.ts (1)
12-15: ⚡ Quick winAvoid repeated object re-allocation in
filterFreeModels.Using object spread in the reducer creates a new object per model. For larger model lists, this is unnecessary overhead in a route hit path.
♻️ Suggested refactor
export const filterFreeModels = (models: OpenRouterModel[]): Record<string, string> => models .filter(m => m.id.endsWith(':free') && m.pricing?.prompt === '0') - .reduce<Record<string, string>>((acc, m) => ({ ...acc, [m.id]: m.name }), {}); + .reduce<Record<string, string>>((acc, m) => { + acc[m.id] = m.name; + return acc; + }, {});🤖 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 `@apps/web/src/app/api/ai/openrouter/models/route.ts` around lines 12 - 15, The reducer in filterFreeModels creates a new object for every model via object spread, causing unnecessary allocations; change the implementation to mutate the accumulator instead of spreading: in filterFreeModels (and its reduce callback) assign acc[m.id] = m.name and return acc, or replace reduce with a simple forEach that sets result[m.id] = m.name, keeping the same filter predicate (m.id.endsWith(':free') && m.pricing?.prompt === '0') and returning the final Record<string,string>.
🤖 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.
Inline comments:
In `@tasks/openrouter-free-dynamic-models.md`:
- Line 14: Update the documentation sentence that currently uses the internal
key-style names 'lmstudio' and 'ollama' to use product-style capitalized names
"LM Studio" and "Ollama" in prose (e.g., change "mirroring the lmstudio route
shape" to "mirroring the LM Studio route shape"); ensure product names are not
wrapped in inline code backticks in prose and update any other occurrences of
'lmstudio'/'ollama' in the same doc sentence to the proper product casing.
---
Nitpick comments:
In `@apps/web/src/app/api/ai/openrouter/models/route.ts`:
- Around line 12-15: The reducer in filterFreeModels creates a new object for
every model via object spread, causing unnecessary allocations; change the
implementation to mutate the accumulator instead of spreading: in
filterFreeModels (and its reduce callback) assign acc[m.id] = m.name and return
acc, or replace reduce with a simple forEach that sets result[m.id] = m.name,
keeping the same filter predicate (m.id.endsWith(':free') && m.pricing?.prompt
=== '0') and returning the final Record<string,string>.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 607ccdce-c7a7-427d-aa42-21a56ebda719
📒 Files selected for processing (9)
apps/web/src/app/api/ai/openrouter/models/__tests__/route.test.tsapps/web/src/app/api/ai/openrouter/models/route.tsapps/web/src/app/api/ai/settings/route.tsapps/web/src/components/ai/chat/input/ProviderModelSelector.tsxapps/web/src/lib/ai/core/ai-providers-config.tsapps/web/src/lib/ai/core/provider-factory.tsapps/web/src/test/next-server-stub.tsapps/web/vitest.config.tstasks/openrouter-free-dynamic-models.md
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/test/next-server-stub.ts
…test
- Extract filterFreeModels and OpenRouterModel type to filter-utils.ts so
that route.ts only exports HTTP handlers (Next.js 15 prohibits arbitrary
named exports from route files — build was failing with
'"filterFreeModels" is not a valid Route export field')
- Remove next: { revalidate: 3600 } from fetch() options (not valid
RequestInit; Next.js fetch caching not needed for an authed API route)
- Update route test to import filterFreeModels from ../filter-utils and
drop the revalidate assertion from the fetch call check
- Add missing test: openrouter_free rejects non-:free model IDs (400)
to cover the P1 review concern about paid model bypass
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Deployment failed with the following error: |
…fix doc naming - filterFreeModels: replace reduce+spread (O(n²)) with Object.fromEntries per code review feedback from owner - vitest.config.ts: remove @pagespace/lib regex alias — violates project rule 'no vitest aliases for workspace packages'; ./logging/logger-config is already a proper subpath export in packages/lib/package.json so bun resolves it natively - tasks/openrouter-free-dynamic-models.md: use product-style names 'LM Studio' and 'Ollama' in prose (not backtick key-style) per coderabbit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests The PR's next/server alias maps to next-server-stub.ts which only exported NextResponse. Tests that import NextRequest from 'next/server' (mcp-tokens, verify-email) were getting undefined, causing every assertion to fail. Add a minimal NextRequest class that extends the native Request so those test files can construct request objects normally. Also add NextResponse.redirect to complete the stub. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Real Next.js NextResponse.redirect() uses 307 (Temporary Redirect) as default. Our stub was defaulting to 302, causing google-callback-redirect tests to fail with 'expected 302 to be 307'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The new route is a read-only, auth-gated model discovery endpoint that returns only public OpenRouter model metadata — functionally equivalent to ai/ollama/models and ai/lmstudio/models which are already exempt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add DYNAMIC_MODEL_PROVIDERS set to ai-providers-config.ts and use it in
the settings route to skip model validation for providers whose model
list is fetched dynamically at runtime.
Switching to openrouter_free previously returned 400 "Model is required"
because models:{} is empty and the UI sends no model on provider switch.
ollama/lmstudio escaped the same check via ONPREM_ALLOWED_PROVIDERS, but
openrouter_free is a cloud provider so it wasn't covered.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for the detailed bug report — both issues addressed: Bug fix: Added
Security audit gate: Added |
Spreading a Headers instance gives {} because its entries are not
enumerable. Use the Headers constructor to properly merge provided
headers with Content-Type (json) or Location (redirect).
This restores Set-Cookie and Cache-Control headers passed via init.headers
in signup-passkey and magic-link/verify route tests.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Expose
openrouter_freeas a user-selectable provider in cloud mode, with a fully dynamic model list fetched live from the OpenRouter API.What changed
Core feature
openrouter_freetoUSER_VISIBLE_PROVIDERSin the cloud settings API so users can select it in the model picker/api/ai/openrouter/modelsroute that fetches OpenRouter's public model catalog, filters to models with:freesuffix and zero prompt pricing, and returns them in the same{ success, models }shape used by Ollama/LM Studioopenrouter_freemodel list fromai-providers-config.ts— UI now fetches liveProviderModelSelectorto lazily fetch and cache free models, with the same loading/empty-state pattern used for Ollama and LM StudioSecurity enforcement
PATCH /api/ai/settingsnow rejectsopenrouter_free+ any model that doesn't end with:free(HTTP 400)createAIProvider(provider-factory.ts) also enforces the:freesuffix before forwarding to OpenRouter backend, so no paid model can slip through even if the settings check is bypassedCode quality
filterFreeModelsextracted tofilter-utils.ts— Next.js 15 prohibits arbitrary named exports from route files; keeping it in route.ts was causing the build to fail with"filterFreeModels" is not a valid Route export fieldfilterFreeModelsusesObject.fromEntriesinstead of reduce+spread (O(n²) allocations)@pagespace/libregex alias fromvitest.config.ts— violates project rule;./logging/logger-configsubpath export already exists inpackages/lib/package.jsonnext-server-stub.tsextended to exportNextRequest(was missing, causing all tests that importNextRequestto fail silently)Tests added
GET /api/ai/openrouter/models: auth, 503 when unconfigured, model filtering, fetch failures, unexpected errorsfilterFreeModelsunit tests covering:free+ zero-pricing filter logicopenrouter_freeavailability, key-unconfigured 503, key-present 200, non-:freemodel rejection 400 (closes the P1 review concern)How to verify
OPENROUTER_DEFAULT_API_KEYin your.env.localOpenRouter Freeshould appear alongside PageSpace:freehttps://claude.ai/code/session_01AjAWKABbABoZcfvUUJvPau