Skip to content

feat: add SDK settings - #2582

Merged
steebchen merged 5 commits into
mainfrom
embeddable-api-key
Jun 8, 2026
Merged

steebchen merged 5 commits into
mainfrom
embeddable-api-key

Conversation

@steebchen

@steebchen steebchen commented Jun 8, 2026 •

Copy link
Copy Markdown
Member

Summary

  • add an SDK page under project Settings navigation
  • add owner/admin-only platform secret key routes for SDK session minting
  • prevent SDK platform secret keys from authenticating normal gateway requests
  • expose and update project end-user session settings, markup, and allowed origins
  • document how to create the SDK platform secret key from Settings -> SDK
  • keep the standalone docs page at /features/llm-sdk for the LLM SDK guide
  • cover platform secret create/list/delete, developer access control, and gateway rejection in tests

Tests

  • pnpm format
  • pnpm test:unit (114 files, 1867 passed, 2 skipped)
  • pnpm build

Notes

  • Browser smoke check opened the dashboard route on the UI dev server before the final SDK rename; it hit the expected local API ECONNREFUSED because the API backend was not running.
  • Full build validates /dashboard/[orgId]/[projectId]/settings/sdk.

Summary by CodeRabbit

  • New Features

    • SDK settings page in dashboard to manage platform secret keys and end‑user session settings (enablement, markup %, allowed origins).
    • Platform secret key API endpoints to create, list, and revoke server-side SDK keys.
  • Documentation

    • LLM SDK docs updated with platform secret key setup and branding changed from "Embeddable SDK" to "LLM SDK".
  • Tests

    • Added tests covering platform key creation, listing, deletion, and role-based access.

Copilot AI review requested due to automatic review settings June 8, 2026 12:11
@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 2b6faddd-78f3-4d3a-b2ed-19a4899ff150

📥 Commits

Reviewing files that changed from the base of the PR and between b414b00 and 5afb6ad.

📒 Files selected for processing (5)
  • apps/api/src/routes/keys-api.spec.ts
  • apps/api/src/routes/projects.ts
  • apps/gateway/src/lib/cached-queries.spec.ts
  • apps/gateway/src/lib/cached-queries.ts
  • apps/ui/src/components/settings/sdk-settings.tsx
✅ Files skipped from review due to trivial changes (1)
  • apps/gateway/src/lib/cached-queries.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/api/src/routes/keys-api.spec.ts
  • apps/ui/src/components/settings/sdk-settings.tsx

Walkthrough

This PR adds a complete platform secret key management feature for LLM SDK integration, extends project configuration to support end-user session settings, and renames SDK branding from "Embeddable SDK" to "LLM SDK" throughout the codebase. The backend exposes three new /keys/platform routes (create, list, delete) with role-based access control, the frontend provides a dedicated settings page with forms for session configuration and key management, and documentation is updated with setup instructions.

Changes

Platform Keys and End-User Settings Feature

Layer / File(s) Summary
Platform Key Authorization and Schemas
apps/api/src/routes/keys-api.ts
Zod schemas and assertPlatformKeyAdminAccess helper enforce owner/admin role for platform secret key operations and validate project/organization context.
Platform Key Routes and Related Comments
apps/api/src/routes/keys-api.ts
GET /platform lists masked keys; POST /platform generates sk_... token and inserts apiKey row with audit logging; DELETE /platform/{id} soft-deletes and logs deletion event. Comments updated for aggregate key filtering.
Platform Key Tests and Cached Query Filter
apps/api/src/routes/keys-api.spec.ts, apps/gateway/src/lib/cached-queries.ts, apps/gateway/src/lib/cached-queries.spec.ts
Tests validate creation (token format, DB state), listing (masked-only semantics), deletion (status change), authorization (403 for developer role); cached lookup excludes platform_secret keys and has unit test.
End-User Settings Schemas
apps/api/src/routes/organization.ts, apps/api/src/routes/projects.ts
Project schemas extended with endUserEnabled, endUserMarkupPercent, and allowedOrigins fields for end-user session configuration.
End-User Settings Validation and Authorization
apps/api/src/routes/projects.ts
normalizeAllowedOrigins validates HTTP(S) URLs and deduplicates by origin; PATCH handler enforces owner/admin role when updating end-user settings and normalizes values before persistence.
End-User Settings Audit Metadata
apps/api/src/routes/projects.ts
Audit event generation tracks changes to end-user settings with proper string conversion and JSON normalization for comparison.
SerializedProject Type Update
packages/db/src/types.ts
Updated type definition to include end-user settings fields in serialized project responses; SDK internals comments rebranded to LLM terminology.
SDK Settings Page & Navigation
apps/ui/src/app/dashboard/[orgId]/[projectId]/settings/sdk/page.tsx, apps/ui/src/components/dashboard/dashboard-sidebar.tsx
Server component fetches project data and renders SdkSettings; adds "SDK" submenu item linking to settings/sdk.
SDK Settings Component
apps/ui/src/components/settings/sdk-settings.tsx
Client component manages end-user session toggles, markup percent input, origin validation, platform secret key creation/deletion with toast notifications and clipboard copy functionality.
SDK Documentation
apps/docs/content/features/llm-sdk.mdx
Documentation page rebranded to LLM SDK with new section explaining platform secret key creation and secure server-side storage practices.

SDK Branding Updates (Embeddable → LLM)

Layer / File(s) Summary
API and Lib Module Branding
apps/api/src/routes/*, apps/api/src/lib/*, apps/api/src/index.ts
Documentation and module comments updated to reference LLM SDK across platform routes, authentication libraries, and CORS configuration.
Stripe and Worker Branding
apps/api/src/stripe.ts, apps/worker/src/worker.ts
Comments in payment processing and background job modules updated to describe LLM SDK workflows and wallet operations.
Gateway Module Branding
apps/gateway/src/chat/*, apps/gateway/src/embeddings/*, apps/gateway/src/moderations/*, apps/gateway/src/videos/*, apps/gateway/src/lib/*
Comments describing ephemeral end-user session semantics and wallet binding updated to reference LLM SDK.
Database Schema and Webhook Helpers Branding
packages/db/src/schema.ts, packages/db/src/webhook-helpers.ts
Schema comments for transactions, webhooks, and video jobs updated to reference LLM SDK; type comments reworded.
Documentation and Blog Branding
apps/ui/src/content/blog/2026-06-07-embeddable-ai-credits.md
Blog post and documentation link text updated from Embeddable SDK to LLM SDK.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • theopenco/llmgateway#2555: The new /keys/platform CRUD endpoints and project allowedOrigins validation directly complement #2555's platform-secret key work and runtime origin allowlist checking.

Suggested reviewers

  • smakosh
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add SDK settings' is directly related to the main changes. The PR adds a new SDK settings page under project Settings navigation, implements platform secret key routes, and allows updating end-user session settings—all core components of the SDK settings feature.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch embeddable-api-key

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2ad35be63

ℹ️ 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".

token,
projectId,
description,
keyType: "platform_secret",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject platform secrets in gateway auth

Creating this row as an active platform_secret makes the returned sk_... token usable against normal gateway endpoints, because apps/gateway/src/lib/cached-queries.ts currently authenticates any API key whose token matches and whose keyType is not end_user_customer. In practice, a secret advertised here as only for minting end-user sessions can call /v1/chat/completions directly and bypass the end-user session/wallet controls; either the gateway lookup needs to exclude platform_secret keys or these secrets need to be stored so they cannot authenticate regular LLM requests.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new “Embeddable” project settings area to configure end-user session settings (enablement, markup, allowed origins) and manage platform secret keys used for minting embeddable sessions. This extends both the dashboard UI and API surface to support embeddable SDK configuration, with unit tests covering the new platform key endpoints.

Changes:

  • Add Embeddable settings page in the dashboard (new route + sidebar link) with UI to update project end-user session settings and manage platform secret keys.
  • Extend API project schemas + update handler to support end-user session settings, including origin normalization and owner/admin-only updates.
  • Add platform secret key list/create/revoke routes plus unit tests for platform secret creation/listing/revocation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/db/src/types.ts Expands SerializedProject to include embeddable-related project fields for UI consumption.
apps/ui/src/components/settings/embeddable-settings.tsx New client component for editing end-user session settings and managing platform secret keys.
apps/ui/src/components/dashboard/dashboard-sidebar.tsx Adds “Embeddable” link under Project Settings navigation.
apps/ui/src/app/dashboard/[orgId]/[projectId]/settings/embeddable/page.tsx Adds the new Embeddable settings page route and server-side project fetch.
apps/api/src/routes/projects.ts Adds embeddable fields to project schema and restricts updates to owner/admin; normalizes allowed origins.
apps/api/src/routes/organization.ts Adds embeddable fields to org project schema (project list responses).
apps/api/src/routes/keys-api.ts Introduces platform secret key endpoints and access checks for owner/admin only.
apps/api/src/routes/keys-api.spec.ts Adds unit tests for platform secret create/list/delete and partial role access coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +310 to +314
{platformKeysQuery.error && (
<p className="text-destructive text-sm">
Only organization owners and admins can manage platform keys.
</p>
)}
Comment on lines +192 to +198
const copyCreatedToken = () => {
void navigator.clipboard.writeText(createdToken);
toast({
title: "Secret key copied",
description: "The platform secret key has been copied.",
});
};
Comment thread apps/api/src/routes/keys-api.spec.ts Outdated
Comment on lines +187 to +206
test("POST /keys/platform rejects organization developers", async () => {
await db
.update(tables.userOrganization)
.set({ role: "developer" })
.where(eq(tables.userOrganization.id, "test-user-org-id"));

const res = await app.request("/keys/platform", {
method: "POST",
headers: {
"Content-Type": "application/json",
Cookie: token,
},
body: JSON.stringify({
projectId: "test-project-id",
description: "Developer secret",
}),
});

expect(res.status).toBe(403);
});

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
apps/ui/src/components/settings/embeddable-settings.tsx (1)

35-51: 💤 Low value

Consider wrapping URL parsing to provide a user-friendly error message.

When new URL(origin) throws for malformed input (e.g., "not-a-url"), the user sees a generic "Invalid URL" message. The backend's normalizeAllowedOrigins provides clearer feedback: Invalid allowed origin: ${origin}.

♻️ Suggested improvement
 function normalizeOrigins(value: string) {
 	const origins = value
 		.split(/\r?\n/)
 		.map((origin) => origin.trim())
 		.filter(Boolean);
 	const normalizedOrigins = new Set<string>();

 	for (const origin of origins) {
-		const url = new URL(origin);
+		let url: URL;
+		try {
+			url = new URL(origin);
+		} catch {
+			throw new Error(`Invalid allowed origin: ${origin}`);
+		}
 		if (url.protocol !== "https:" && url.protocol !== "http:") {
 			throw new Error("Origins must use http or https.");
 		}
 		normalizedOrigins.add(url.origin);
 	}

 	return Array.from(normalizedOrigins);
 }
🤖 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/ui/src/components/settings/embeddable-settings.tsx` around lines 35 -
51, The URL constructor in normalizeOrigins can throw for malformed entries;
wrap the new URL(origin) call in a try/catch inside the normalizeOrigins
function and, on failure, throw a new Error with a user-friendly message such as
`Invalid allowed origin: ${origin}` (preserving the invalid origin value)
instead of letting the generic URL error bubble up; keep the existing protocol
check and normalizedOrigins.add(url.origin) logic intact.
apps/api/src/routes/projects.ts (1)

343-354: 💤 Low value

Redundant call to normalizeAllowedOrigins.

The origins are already normalized at line 281. Reuse that value instead of calling the function again.

♻️ Suggested refactor
+	let normalizedAllowedOrigins: string[] | undefined;
 	if (allowedOrigins !== undefined) {
-		updateData.allowedOrigins = normalizeAllowedOrigins(allowedOrigins);
+		normalizedAllowedOrigins = normalizeAllowedOrigins(allowedOrigins);
+		updateData.allowedOrigins = normalizedAllowedOrigins;
 	}
 ...
 	if (allowedOrigins !== undefined) {
-		const normalizedAllowedOrigins = normalizeAllowedOrigins(allowedOrigins);
 		const previousAllowedOrigins = project.allowedOrigins ?? [];
 		if (
-			JSON.stringify(normalizedAllowedOrigins) !==
+			JSON.stringify(normalizedAllowedOrigins!) !==
 			JSON.stringify(previousAllowedOrigins)
 		) {
 			changes.allowedOrigins = {
 				old: previousAllowedOrigins,
-				new: normalizedAllowedOrigins,
+				new: normalizedAllowedOrigins!,
 			};
 		}
 	}
🤖 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/api/src/routes/projects.ts` around lines 343 - 354, You’re calling
normalizeAllowedOrigins again even though the origins were normalized earlier;
remove the redundant normalizeAllowedOrigins call and reuse the
already-normalized value (the variable computed earlier when processing
allowedOrigins) when comparing to project.allowedOrigins and when setting
changes.allowedOrigins; update the comparison to use that existing normalized
value instead of calling normalizeAllowedOrigins(allowedOrigins) and keep the
rest of the logic (previousAllowedOrigins/project.allowedOrigins and
changes.allowedOrigins) unchanged.
🤖 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 `@apps/api/src/routes/projects.ts`:
- Around line 343-354: You’re calling normalizeAllowedOrigins again even though
the origins were normalized earlier; remove the redundant
normalizeAllowedOrigins call and reuse the already-normalized value (the
variable computed earlier when processing allowedOrigins) when comparing to
project.allowedOrigins and when setting changes.allowedOrigins; update the
comparison to use that existing normalized value instead of calling
normalizeAllowedOrigins(allowedOrigins) and keep the rest of the logic
(previousAllowedOrigins/project.allowedOrigins and changes.allowedOrigins)
unchanged.

In `@apps/ui/src/components/settings/embeddable-settings.tsx`:
- Around line 35-51: The URL constructor in normalizeOrigins can throw for
malformed entries; wrap the new URL(origin) call in a try/catch inside the
normalizeOrigins function and, on failure, throw a new Error with a
user-friendly message such as `Invalid allowed origin: ${origin}` (preserving
the invalid origin value) instead of letting the generic URL error bubble up;
keep the existing protocol check and normalizedOrigins.add(url.origin) logic
intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3ec203eb-5d26-401c-b7f0-5d7a5986d2b7

📥 Commits

Reviewing files that changed from the base of the PR and between 9060a48 and b2ad35b.

📒 Files selected for processing (8)
  • apps/api/src/routes/keys-api.spec.ts
  • apps/api/src/routes/keys-api.ts
  • apps/api/src/routes/organization.ts
  • apps/api/src/routes/projects.ts
  • apps/ui/src/app/dashboard/[orgId]/[projectId]/settings/embeddable/page.tsx
  • apps/ui/src/components/dashboard/dashboard-sidebar.tsx
  • apps/ui/src/components/settings/embeddable-settings.tsx
  • packages/db/src/types.ts

@steebchen steebchen changed the title feat: add embeddable settings feat: add LLM SDK settings Jun 8, 2026
@steebchen steebchen changed the title feat: add LLM SDK settings feat: add SDK settings Jun 8, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/ui/src/components/settings/llm-sdk-settings.tsx (1)

35-51: ⚡ Quick win

Wrap URL parsing in try-catch for better error messages.

The new URL(origin) constructor at line 43 can throw if the origin is invalid, but it's not wrapped in a try-catch block. While the outer saveSettings function catches the error, the generic URL constructor error message may be less helpful to users than a specific message. The backend normalizes origins with a try-catch and provides "Invalid allowed origin: ${origin}".

Consider matching the backend pattern for consistent error messages:

Suggested improvement
 function normalizeOrigins(value: string) {
 	const origins = value
 		.split(/\r?\n/)
 		.map((origin) => origin.trim())
 		.filter(Boolean);
 	const normalizedOrigins = new Set<string>();
 
 	for (const origin of origins) {
-		const url = new URL(origin);
+		let url: URL;
+		try {
+			url = new URL(origin);
+		} catch {
+			throw new Error(`Invalid allowed origin: ${origin}`);
+		}
 		if (url.protocol !== "https:" && url.protocol !== "http:") {
 			throw new Error("Origins must use http or https.");
 		}
 		normalizedOrigins.add(url.origin);
 	}
 
 	return Array.from(normalizedOrigins);
 }
🤖 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/ui/src/components/settings/llm-sdk-settings.tsx` around lines 35 - 51,
The URL parsing in normalizeOrigins can throw on invalid input; wrap the new
URL(origin) call inside a try-catch in the normalizeOrigins function and rethrow
a clearer, consistent message (e.g. "Invalid allowed origin: ${origin}") so the
UI matches backend normalization errors; keep the rest of the logic (protocol
check for http/https and normalizedOrigins.add(url.origin)) the same and only
change the error path to use the new, descriptive message.
🤖 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 `@apps/ui/src/components/settings/llm-sdk-settings.tsx`:
- Around line 35-51: The URL parsing in normalizeOrigins can throw on invalid
input; wrap the new URL(origin) call inside a try-catch in the normalizeOrigins
function and rethrow a clearer, consistent message (e.g. "Invalid allowed
origin: ${origin}") so the UI matches backend normalization errors; keep the
rest of the logic (protocol check for http/https and
normalizedOrigins.add(url.origin)) the same and only change the error path to
use the new, descriptive message.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ea0b991f-d0ca-47e7-900b-a62bb458bf91

📥 Commits

Reviewing files that changed from the base of the PR and between 6c0f0ef and 8088719.

📒 Files selected for processing (28)
  • apps/api/src/index.ts
  • apps/api/src/lib/end-user-session-auth.ts
  • apps/api/src/lib/platform-secret-auth.ts
  • apps/api/src/routes/keys-api.spec.ts
  • apps/api/src/routes/keys-api.ts
  • apps/api/src/routes/platform-connect.ts
  • apps/api/src/routes/platform-customers.ts
  • apps/api/src/routes/platform-session-refresh.ts
  • apps/api/src/routes/platform-sessions.ts
  • apps/api/src/routes/platform-wallet.ts
  • apps/api/src/routes/platform-webhooks.ts
  • apps/api/src/routes/projects.ts
  • apps/api/src/stripe.ts
  • apps/docs/content/features/llm-sdk.mdx
  • apps/gateway/src/chat/chat.ts
  • apps/gateway/src/chat/tools/create-log-entry.ts
  • apps/gateway/src/embeddings/embeddings.ts
  • apps/gateway/src/lib/end-user-session.ts
  • apps/gateway/src/moderations/moderations.ts
  • apps/gateway/src/videos/videos.ts
  • apps/ui/src/app/dashboard/[orgId]/[projectId]/settings/llm-sdk/page.tsx
  • apps/ui/src/components/dashboard/dashboard-sidebar.tsx
  • apps/ui/src/components/settings/llm-sdk-settings.tsx
  • apps/ui/src/content/blog/2026-06-07-embeddable-ai-credits.md
  • apps/worker/src/worker.ts
  • packages/db/src/schema.ts
  • packages/db/src/types.ts
  • packages/db/src/webhook-helpers.ts
✅ Files skipped from review due to trivial changes (20)
  • apps/api/src/routes/platform-customers.ts
  • apps/api/src/routes/platform-sessions.ts
  • apps/api/src/routes/platform-connect.ts
  • apps/api/src/index.ts
  • apps/gateway/src/embeddings/embeddings.ts
  • apps/api/src/routes/platform-webhooks.ts
  • apps/gateway/src/lib/end-user-session.ts
  • apps/api/src/routes/platform-session-refresh.ts
  • apps/api/src/lib/end-user-session-auth.ts
  • apps/api/src/lib/platform-secret-auth.ts
  • apps/ui/src/content/blog/2026-06-07-embeddable-ai-credits.md
  • packages/db/src/webhook-helpers.ts
  • apps/gateway/src/moderations/moderations.ts
  • apps/gateway/src/videos/videos.ts
  • apps/api/src/stripe.ts
  • apps/gateway/src/chat/tools/create-log-entry.ts
  • apps/gateway/src/chat/chat.ts
  • apps/api/src/routes/platform-wallet.ts
  • packages/db/src/schema.ts
  • apps/worker/src/worker.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/db/src/types.ts
  • apps/api/src/routes/projects.ts
  • apps/api/src/routes/keys-api.ts
  • apps/api/src/routes/keys-api.spec.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (4)
apps/ui/src/app/dashboard/[orgId]/[projectId]/settings/sdk/page.tsx (1)

36-40: 💤 Low value

Heading text is inconsistent with card title.

The page heading says "SDK" (line 36) while the CardTitle says "LLM SDK" (line 40). Consider using "LLM SDK" for the heading as well to maintain consistency and clarity.

Suggested consistency fix
-						<h2 className="text-3xl font-bold tracking-tight">SDK</h2>
+						<h2 className="text-3xl font-bold tracking-tight">LLM SDK</h2>
🤖 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/ui/src/app/dashboard/`[orgId]/[projectId]/settings/sdk/page.tsx around
lines 36 - 40, The page heading text in the h2 element (className="text-3xl
font-bold tracking-tight") is inconsistent with the CardTitle component
(CardTitle>LLM SDK</CardTitle>); update the h2 string from "SDK" to "LLM SDK" so
the top-level heading and CardTitle match and clearly indicate "LLM SDK".
apps/ui/src/components/settings/sdk-settings.tsx (3)

35-51: ⚡ Quick win

Add explicit error handling in normalizeOrigins.

The new URL(origin) constructor can throw a TypeError if the origin is invalid. While this is caught by the outer try-catch in saveSettings, adding an explicit try-catch here would make the error handling clearer and ensure consistent error messages.

Proposed error handling improvement
 function normalizeOrigins(value: string) {
 	const origins = value
 		.split(/\r?\n/)
 		.map((origin) => origin.trim())
 		.filter(Boolean);
 	const normalizedOrigins = new Set<string>();
 
 	for (const origin of origins) {
-		const url = new URL(origin);
+		let url: URL;
+		try {
+			url = new URL(origin);
+		} catch {
+			throw new Error(`Invalid URL format: ${origin}`);
+		}
 		if (url.protocol !== "https:" && url.protocol !== "http:") {
 			throw new Error("Origins must use http or https.");
 		}
 		normalizedOrigins.add(url.origin);
 	}
 
 	return Array.from(normalizedOrigins);
 }
🤖 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/ui/src/components/settings/sdk-settings.tsx` around lines 35 - 51, The
normalizeOrigins function can throw an uncaught TypeError when new URL(origin)
receives invalid input; wrap the URL construction and protocol check in a
try/catch inside normalizeOrigins to catch invalid origins and rethrow a
clearer, consistent Error (e.g., "Invalid origin: <origin>") so saveSettings
receives a uniform error message. Update normalizeOrigins (referenced by the
function name) to validate each origin string with a try/catch around new
URL(origin) and throw a descriptive Error for invalid values, preserving the
existing deduplication via normalizedOrigins on success.

104-144: 💤 Low value

Consider adding user feedback when origins are normalized.

After successfully saving, the code updates allowedOriginsText with the normalized origins (line 129), which will change the textarea content if the user entered duplicates, extra whitespace, or full URLs. This could surprise users. Consider adding a toast message when normalization occurs to inform them their input was cleaned up.

🤖 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/ui/src/components/settings/sdk-settings.tsx` around lines 104 - 144, The
saveSettings function currently normalizes origins via normalizeOrigins and then
silently updates the textarea state with
setAllowedOriginsText(allowedOrigins.join("\n")), which can surprise users;
modify saveSettings to detect when the normalized origins string differs from
the original allowedOriginsText and, if so, call toast (or reuse the existing
toast pattern) to show a non-destructive informational message (e.g., "Origins
normalized: duplicates/whitespace removed") after successful save but before
closing, referencing normalizeOrigins, setAllowedOriginsText,
allowedOriginsText, and toast so users are informed when their input was cleaned
up.

310-314: 💤 Low value

Error message assumes all query errors are authorization failures.

The error message "Only organization owners and admins can manage platform keys" is displayed for any platformKeysQuery.error, but the error could be a network issue, server error, or other problem unrelated to authorization. Consider checking the error status or using a more generic message with a hint about authorization.

🤖 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/ui/src/components/settings/sdk-settings.tsx` around lines 310 - 314, The
current error UI in sdk-settings.tsx always shows an authorization message when
platformKeysQuery.error exists; instead inspect the error object from
platformKeysQuery (e.g., check error.status, error.response?.status, or a
network flag) and render a specific "permission denied" message only for
401/403, otherwise render a generic "Failed to load platform keys" message
(optionally with a retry hint). Update the conditional around
platformKeysQuery.error to branch on the error status and use
platformKeysQuery.error to surface non-auth errors without assuming
authorization is the cause.
🤖 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 `@apps/ui/src/app/dashboard/`[orgId]/[projectId]/settings/sdk/page.tsx:
- Around line 36-40: The page heading text in the h2 element
(className="text-3xl font-bold tracking-tight") is inconsistent with the
CardTitle component (CardTitle>LLM SDK</CardTitle>); update the h2 string from
"SDK" to "LLM SDK" so the top-level heading and CardTitle match and clearly
indicate "LLM SDK".

In `@apps/ui/src/components/settings/sdk-settings.tsx`:
- Around line 35-51: The normalizeOrigins function can throw an uncaught
TypeError when new URL(origin) receives invalid input; wrap the URL construction
and protocol check in a try/catch inside normalizeOrigins to catch invalid
origins and rethrow a clearer, consistent Error (e.g., "Invalid origin:
<origin>") so saveSettings receives a uniform error message. Update
normalizeOrigins (referenced by the function name) to validate each origin
string with a try/catch around new URL(origin) and throw a descriptive Error for
invalid values, preserving the existing deduplication via normalizedOrigins on
success.
- Around line 104-144: The saveSettings function currently normalizes origins
via normalizeOrigins and then silently updates the textarea state with
setAllowedOriginsText(allowedOrigins.join("\n")), which can surprise users;
modify saveSettings to detect when the normalized origins string differs from
the original allowedOriginsText and, if so, call toast (or reuse the existing
toast pattern) to show a non-destructive informational message (e.g., "Origins
normalized: duplicates/whitespace removed") after successful save but before
closing, referencing normalizeOrigins, setAllowedOriginsText,
allowedOriginsText, and toast so users are informed when their input was cleaned
up.
- Around line 310-314: The current error UI in sdk-settings.tsx always shows an
authorization message when platformKeysQuery.error exists; instead inspect the
error object from platformKeysQuery (e.g., check error.status,
error.response?.status, or a network flag) and render a specific "permission
denied" message only for 401/403, otherwise render a generic "Failed to load
platform keys" message (optionally with a retry hint). Update the conditional
around platformKeysQuery.error to branch on the error status and use
platformKeysQuery.error to surface non-auth errors without assuming
authorization is the cause.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cd0f6224-a5a1-42d4-ae6e-352d75c70f30

📥 Commits

Reviewing files that changed from the base of the PR and between 8088719 and b414b00.

📒 Files selected for processing (7)
  • apps/api/src/routes/keys-api.spec.ts
  • apps/api/src/routes/keys-api.ts
  • apps/api/src/routes/projects.ts
  • apps/docs/content/features/llm-sdk.mdx
  • apps/ui/src/app/dashboard/[orgId]/[projectId]/settings/sdk/page.tsx
  • apps/ui/src/components/dashboard/dashboard-sidebar.tsx
  • apps/ui/src/components/settings/sdk-settings.tsx
✅ Files skipped from review due to trivial changes (1)
  • apps/docs/content/features/llm-sdk.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/api/src/routes/keys-api.spec.ts
  • apps/api/src/routes/keys-api.ts
  • apps/api/src/routes/projects.ts

@steebchen
steebchen enabled auto-merge June 8, 2026 12:45

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5afb6ad07d

ℹ️ 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".

Comment on lines +259 to +260
<Switch
checked={endUserEnabled}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove duplicate Switch checked prop

With this duplicated checked attribute, the new SDK settings component contains invalid JSX (JSX elements cannot have multiple attributes with the same name), so the UI TypeScript/Next build will fail as soon as this page is compiled. Remove one of the duplicate attributes.

Useful? React with 👍 / 👎.

@steebchen
steebchen added this pull request to the merge queue Jun 8, 2026
Merged via the queue into main with commit 4d69383 Jun 8, 2026
18 checks passed
@steebchen
steebchen deleted the embeddable-api-key branch June 8, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants