feat(openrouter): use org-specific models endpoint when user has an organization#2326
feat(openrouter): use org-specific models endpoint when user has an organization#2326kilo-code-bot[bot] wants to merge 10 commits intomainfrom
Conversation
| ): Promise<NextResponse<{ error: string; message: string } | OpenRouterModelsResponse>> { | ||
| request: NextRequest | ||
| ): Promise<NextResponse<{ error: string; message?: string } | OpenRouterModelsResponse>> { | ||
| const { organizationId } = await getUserFromAuth({ adminOnly: false }); |
There was a problem hiding this comment.
WARNING: Auth lookup can fail the public models endpoint
getDirectByokModels() currently swallows getUserFromAuth/database failures and falls back to the public OpenRouter list. Moving getUserFromAuth to the top level means any exception from session or user lookup aborts the handler before getEnhancedOpenRouterModels() runs, so a transient auth/database failure now turns this otherwise public endpoint into a 500 instead of a degraded response.
There was a problem hiding this comment.
Fixed: is now wrapped in with a try/catch, matching the same defensive pattern as the original . Errors fall back to the non-org path, so the endpoint remains available even if auth/database is unavailable.
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)CRITICAL
Other Observations (not in diff)None. Files Reviewed (4 files)
Reviewed by gpt-5.4-20260305 · 611,220 tokens |
| user: { id: 'test-user-id' }, | ||
| authFailedResponse: null, | ||
| })), | ||
| getUserFromAuth: jest.fn().mockImplementation(async () => ({ |
There was a problem hiding this comment.
WARNING: resetAllMocks() clears this implementation before each test
This suite still calls jest.resetAllMocks() in beforeEach, which resets getUserFromAuth back to a no-op mock. GET() now immediately destructures organizationId from that result, so the tests will start throwing before they reach the route logic unless the implementation is re-applied in beforeEach (or the suite switches to clearAllMocks()).
There was a problem hiding this comment.
Fixed: the route was restructured so getUserFromAuth is called defensively in getAuthContext() (try/catch), and tests don't need to mock it. The resetAllMocks() issue no longer applies.
…emove tRPC call from route
| import { KILO_ORGANIZATION_ID } from '@/lib/organizations/constants'; | ||
| import { listAvailableCustomLlms } from '@/lib/custom-llm/listAvailableCustomLlms'; | ||
| import { getDirectByokModelsForOrganization } from '@/lib/providers/direct-byok'; | ||
| import { createAllowPredicateFromDenyList } from '@/lib/model-allow.server'; |
There was a problem hiding this comment.
CRITICAL: Missing import breaks this router
This refactor removed createAllowPredicateFromDenyList from the imports, but updateAllowLists and updateDefaultModel still call it later in the file. That leaves organization-settings-router.ts with an unresolved identifier and should fail typecheck/build as soon as this file is compiled.
Summary
When
getUserFromAuthreturns anorganizationId(i.e., the request includes anx-kilocode-organizationidheader from the extension), the/api/openrouter/modelsendpoint now delegates tocaller.organizations.settings.listAvailableModels. This means org members get:Previously, this endpoint always returned the global model list + user-level BYOK models regardless of organization context.
Verification
listAvailableModelstRPC procedure to confirm it handles org-level BYOK, custom LLMs, and enterprise deny-lists.organizationIdfield inGetAuthResponseis only set when a validx-kilocode-organizationidheader is present.git diffto verify the change is minimal and correct.Visual Changes
N/A
Reviewer Notes
The non-org path is unchanged. The
getDirectByokModelshelper (user-level BYOK) is only called in the non-org branch. Org-level BYOK is handled insidelistAvailableModels. Note thatgetUserFromAuthis called once at the top ofGETto check fororganizationId; the helpergetDirectByokModelsalso calls it internally, but Next.js caches theheaders()call per request so there is no meaningful overhead.