feat(integrations): add third-party integration system foundation #219
Conversation
This commit adds the foundation for third-party integrations in PageSpace: Database: - Add userIntegrations table for storing per-user integration configs - Schema supports encrypted API keys, enabled tools, validation status Integration System: - IntegrationDefinition type for defining integrations - Registry pattern for managing available integrations - Tool loader for dynamically loading integration tools - Validation support for testing credentials API Routes: - GET/POST /api/settings/integrations - List/configure integrations - GET/PATCH/DELETE/POST /api/settings/integrations/[id] - Manage integration Settings UI: - New Integrations page at /settings/integrations - Card-based UI for viewing/configuring integrations - Enable/disable toggle, validation, and removal AI Integration: - Integration tools loaded dynamically in chat route - Tools merged with PageSpace tools and filtered by page config First Integration (Apify): - apify_run_actor - Run web scrapers/automation - apify_list_actors - Discover available actors - apify_get_dataset - Retrieve scraped data
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR introduces a comprehensive user-facing integrations system for PageSpace. It includes database schema for storing per-user integration configurations, an integrations framework with types and registry, API routes for CRUD operations and validation, a UI page for managing integrations, and an Apify integration as the first example. Integration tools are now loaded and merged with existing tools in the AI chat system. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsUI as Settings UI<br/>(integrations/page.tsx)
participant API as Integration API<br/>(/api/settings/integrations)
participant DB as Database<br/>(user_integrations)
participant Vault as Encryption<br/>(encrypt/decrypt)
User->>SettingsUI: Open integrations page
SettingsUI->>API: GET /api/settings/integrations
API->>DB: SELECT all integrations + user configs
DB-->>API: Return data
API-->>SettingsUI: Integration list with status
SettingsUI-->>User: Display integration cards
User->>SettingsUI: Enter API key & configure
SettingsUI->>API: POST /api/settings/integrations
API->>Vault: encrypt(apiKey)
Vault-->>API: encryptedApiKey
API->>DB: INSERT user_integrations record
DB-->>API: Success
API-->>SettingsUI: Created integration config
SettingsUI->>SettingsUI: mutate() to refresh
SettingsUI-->>User: Show confirmation
User->>SettingsUI: Click validate credentials
SettingsUI->>API: POST /api/settings/integrations/[id]
API->>DB: SELECT integration config
DB-->>API: encryptedApiKey
API->>Vault: decrypt(encryptedApiKey)
Vault-->>API: apiKey
API->>API: Call integration.validate(apiKey)
API->>DB: UPDATE validationStatus
DB-->>API: Updated
API-->>SettingsUI: Validation result
SettingsUI-->>User: Display status badge
sequenceDiagram
actor User
participant ChatUI as Chat UI
participant ChatAPI as Chat API<br/>(/api/ai/chat)
participant ToolLoader as Tool Loader<br/>(tool-loader.ts)
participant DB as Database<br/>(user_integrations)
participant Vault as Encryption
participant Apify as Apify API
User->>ChatUI: Send message with tool request
ChatUI->>ChatAPI: POST /api/ai/chat
ChatAPI->>ChatAPI: getIntegrationToolsForUser(userId)
ChatAPI->>ToolLoader: getUserIntegrationTools(userId)
ToolLoader->>DB: SELECT enabled integrations for user
DB-->>ToolLoader: Configs with encryptedApiKey
loop For each enabled integration
ToolLoader->>Vault: decrypt(encryptedApiKey)
Vault-->>ToolLoader: apiKey
ToolLoader->>ToolLoader: loadIntegrationTools(config, apiKey)
ToolLoader->>ToolLoader: Call integration.tools(context)
end
ToolLoader-->>ChatAPI: Merged integration tools
ChatAPI->>ChatAPI: Merge with pageSpaceTools
ChatAPI->>ChatAPI: Filter & select enabled tools
ChatAPI->>ChatAPI: Sanitize tool names
ChatAPI->>ChatAPI: Execute AI with tool set
ChatAPI->>Apify: Call integration tool (if selected)
Apify-->>ChatAPI: Tool result
ChatAPI-->>ChatUI: AI response with results
ChatUI-->>User: Display response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/web/src/app/api/settings/integrations/`[integrationId]/route.ts:
- Around line 171-193: When handling apiKey in the route, trim whitespace first
and use the trimmed value for emptiness checks, encryption, and validation:
create a trimmedApiKey = apiKey?.trim() and if trimmedApiKey === '' treat it as
clearing the key (set updates.encryptedApiKey = null,
validationStatus/message/lastValidatedAt accordingly); otherwise encrypt
trimmedApiKey via encrypt(trimmedApiKey) and pass trimmedApiKey into
definition.validate. Also make the config spread explicit when building
mergedConfig by using ...(config ?? {}) merged with existing.config (e.g., const
mergedConfig = { ...(existing.config as Record<string, unknown> || {}),
...(config ?? {}) }) before calling definition.validate so undefined config is
handled clearly.
- Around line 156-169: The code builds an updates object (variable updates) but
does not guard against an empty payload, which causes db.update().set({}) to
generate invalid SQL; before calling the Drizzle update (the code that uses
db.update().set(...)), check if Object.keys(updates).length === 0 and return a
400 response (bad request) with a clear message like "No updatable fields
provided" instead of performing the update; apply the same guard to the other
update block referenced around the second occurrence (the similar updates object
at the 197-203 area) so neither empty payload reaches db.update().set.
In `@apps/web/src/app/settings/integrations/page.tsx`:
- Line 81: The fetcher function (const fetcher = (url: string) => ...) must
check the Response.ok and throw on non-OK responses so SWR receives an error;
modify fetcher to await fetch(url, { credentials: 'include' }), if !response.ok
read the body (text or json) and throw a new Error that includes response.status
and the error body, otherwise return await response.json(); this ensures
useSWR's error variable is populated for 4xx/5xx responses.
🧹 Nitpick comments (6)
apps/web/src/app/settings/integrations/page.tsx (3)
34-79: Consider importing types from the centralized integrations module.These interfaces duplicate the types already exported from
@/lib/integrations. Importing them would ensure consistency and reduce maintenance burden.♻️ Suggested refactor
-// Type for integration status from API -interface IntegrationTool { - name: string; - displayName: string; - description: string; - isWriteTool?: boolean; - tags?: string[]; -} - -interface IntegrationDefinition { - ... -} - -interface UserConfig { - ... -} - -interface IntegrationStatus { - ... -} - -interface IntegrationsResponse { - integrations: IntegrationStatus[]; - configuredCount: number; - enabledCount: number; -} +import type { + IntegrationStatus, + IntegrationDefinition, + IntegrationToolMeta, + UserIntegrationConfig, +} from '@/lib/integrations'; + +interface IntegrationsResponse { + integrations: IntegrationStatus[]; + configuredCount: number; + enabledCount: number; +}
95-98: Consider adding SWR configuration options.Per coding guidelines, consider adding
revalidateOnFocus: falseto prevent unexpected refetches while user is interacting with dialogs.♻️ Suggested configuration
const { data, error, isLoading } = useSWR<IntegrationsResponse>( '/api/settings/integrations', - fetcher + fetcher, + { revalidateOnFocus: false } );
104-104: SharedisValidatingstate affects all integration cards.Currently, clicking "Validate" on one integration disables all other validate buttons. Consider tracking which integration is being validated:
♻️ Suggested fix
-const [isValidating, setIsValidating] = useState(false); +const [validatingId, setValidatingId] = useState<string | null>(null); // In handleValidate: -setIsValidating(true); +setValidatingId(integration.definition.id); // ... -setIsValidating(false); +setValidatingId(null); // In the button: -disabled={isValidating} +disabled={validatingId === integration.definition.id} // and for the spinner: -{isValidating ? ( +{validatingId === integration.definition.id ? (apps/web/src/app/api/settings/integrations/route.ts (1)
91-91: Consider handling malformed JSON in request body.
request.json()throws if the body is not valid JSON. While unlikely in practice, wrapping this in a try-catch provides better error messages to clients.♻️ Suggested improvement
-const body = await request.json(); +let body: unknown; +try { + body = await request.json(); +} catch { + return NextResponse.json( + { error: 'Invalid JSON in request body' }, + { status: 400 } + ); +}apps/web/src/lib/integrations/definitions/apify.ts (1)
20-33: Consider adding request timeout for external API calls.The
apifyRequesthelper doesn't specify a timeout. If the Apify API is slow or unresponsive, requests could hang indefinitely. Consider adding an AbortController with a timeout.♻️ Suggested improvement
async function apifyRequest( path: string, apiKey: string, - options: RequestInit = {} + options: RequestInit = {}, + timeoutMs: number = 30000 ): Promise<Response> { const url = `${APIFY_BASE_URL}${path}`; const headers = { 'Authorization': `Bearer ${apiKey}`, 'Content-Type': 'application/json', ...options.headers, }; - return fetch(url, { ...options, headers }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), timeoutMs); + + try { + return await fetch(url, { ...options, headers, signal: controller.signal }); + } finally { + clearTimeout(timeoutId); + } }apps/web/src/lib/integrations/tool-loader.ts (1)
121-132: Isolate tool creation errors so one integration can’t break all tooling.
IfcreateToolsthrows, it will bubble and can break callers (e.g., AI tool aggregation). Consider catching and skipping that integration.♻️ Suggested change
// Create tools from the integration - const allTools = definition.createTools(context); + let allTools: Record<string, Tool>; + try { + allTools = definition.createTools(context); + } catch (error) { + console.error( + `Failed to create tools for integration ${userConfig.integrationId}:`, + error + ); + return {}; + }
| // Build update object | ||
| const updates: Record<string, unknown> = {}; | ||
|
|
||
| if (enabled !== undefined) { | ||
| updates.enabled = enabled; | ||
| } | ||
|
|
||
| if (config !== undefined) { | ||
| updates.config = { ...(existing.config as Record<string, unknown> || {}), ...config }; | ||
| } | ||
|
|
||
| if (enabledTools !== undefined) { | ||
| updates.enabledTools = enabledTools; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l "apps/web/src/app/api/settings/integrations/[integrationId]/route.ts"Repository: 2witstudios/PageSpace
Length of output: 186
🏁 Script executed:
cat -n "apps/web/src/app/api/settings/integrations/[integrationId]/route.ts" | sed -n '140,220p'Repository: 2witstudios/PageSpace
Length of output: 186
🌐 Web query:
Drizzle ORM empty set() update object behavior SQL
💡 Result:
Short answer: Drizzle ignores undefined fields when building the SET object (to set NULL pass null). It does not have a documented safe auto‑no‑op for an empty update payload — you should guard against calling .set({}) (or passing an object with no defined keys) because there’s no meaningful SET clause to generate. The docs and examples explicitly show ignoring undefined values and the common pattern of checking for an empty inputs array before calling .set(...). [1][2]
References
- Update docs (undefined values are ignored; use null to set NULL). [1]
- Guide example that checks inputs.length === 0 before calling update/.set(...). [2]
🌐 Web query:
Drizzle update set empty object validation
💡 Result:
Short answer: Don’t call db.update(...).set({}) — validate/normalize your payload first and only call .set(...) when it has at least one real column key (and strip undefined). Drizzle’s docs state the object passed to .set must contain column keys (values of undefined are ignored), and there are known issues when keys are constructed dynamically — so guard before calling .set. [1][2]
Suggested patterns
-
Strip undefined and bail out if nothing remains:
- const payload = Object.fromEntries(Object.entries(raw).filter(([,v]) => v !== undefined));
- if (Object.keys(payload).length === 0) return; // nothing to update
- await db.update(table).set(payload).where(...);
-
Build the update only when you have at least one key:
- const updates: Record<string, any> = {};
- if (name != null) updates.name = name;
- if (email != null) updates.email = email;
- if (!Object.keys(updates).length) return;
- await db.update(users).set(updates).where(...);
-
For nested/JSON objects, explicitly set the whole JSON column or use SQL expressions (Drizzle ignores undefined; use null to clear). [1]
References
- Drizzle update docs (notes on .set and undefined values). [1]
- GitHub issue about dynamic/empty keys and update().set behavior. [2]
Guard against empty PATCH payloads before calling the update.
If the request contains no updatable fields, updates stays empty and db.update().set({}) with no defined keys will generate invalid SQL. Drizzle ORM does not safely handle empty update operations, so you must validate the payload and return 400 before calling the update.
🛠️ Suggested fix
// Build update object
const updates: Record<string, unknown> = {};
@@
if (enabledTools !== undefined) {
updates.enabledTools = enabledTools;
}
+ if (Object.keys(updates).length === 0) {
+ return NextResponse.json(
+ { error: 'No fields to update' },
+ { status: 400 }
+ );
+ }
+
// Apply updates
const [updated] = await dbAlso applies to: 197-203
🤖 Prompt for AI Agents
In `@apps/web/src/app/api/settings/integrations/`[integrationId]/route.ts around
lines 156 - 169, The code builds an updates object (variable updates) but does
not guard against an empty payload, which causes db.update().set({}) to generate
invalid SQL; before calling the Drizzle update (the code that uses
db.update().set(...)), check if Object.keys(updates).length === 0 and return a
400 response (bad request) with a clear message like "No updatable fields
provided" instead of performing the update; apply the same guard to the other
update block referenced around the second occurrence (the similar updates object
at the 197-203 area) so neither empty payload reaches db.update().set.
| // Handle API key update | ||
| if (apiKey !== undefined) { | ||
| if (apiKey === '') { | ||
| // Clear API key | ||
| updates.encryptedApiKey = null; | ||
| updates.validationStatus = 'unknown'; | ||
| updates.validationMessage = null; | ||
| updates.lastValidatedAt = null; | ||
| } else { | ||
| // Encrypt and validate new API key | ||
| updates.encryptedApiKey = await encrypt(apiKey.trim()); | ||
|
|
||
| try { | ||
| const mergedConfig = { ...(existing.config as Record<string, unknown> || {}), ...config }; | ||
| const validationResult = await definition.validate(mergedConfig, apiKey); | ||
| updates.validationStatus = validationResult.valid ? 'valid' : 'invalid'; | ||
| updates.validationMessage = validationResult.message; | ||
| updates.lastValidatedAt = new Date(); | ||
| } catch (error) { | ||
| updates.validationStatus = 'invalid'; | ||
| updates.validationMessage = error instanceof Error ? error.message : 'Validation failed'; | ||
| updates.lastValidatedAt = new Date(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/app/api/settings/integrations/[integrationId]/route.ts | sed -n '1,200p'Repository: 2witstudios/PageSpace
Length of output: 186
🌐 Web query:
Next.js 15 Route Handlers optional parameters TypeScript typing patterns
💡 Result:
Short answer with examples:
- Next.js types route params as string | string[] | undefined for optional/optional catch-all segments. Use the built-in generics (PageProps / LayoutProps / RouteContext) to type route handlers. [1][2]
Example for an app route handler with an optional catch-all ([[...slug]]):
-
file: app/shop/[[...slug]]/route.ts
export async function GET(_: Request, context: RouteContext<'/shop/[[...slug]]'>) {
const { params } = context; // params: { slug?: string[] }
const slug = params.slug; // string[] | undefined
// handle undefined (no segments) vs string[] (one+ segments)
}
Example for a single optional param ([id] that might be absent because route is optional):
-
file: app/items/[[id]]/route.ts
export async function GET(_: Request, { params }: RouteContext<'/items/[[id]]'>) {
// params: { id?: string }
if (!params?.id) { /* handle missing id */ }
const id = params.id; // string | undefined
}
Notes:
- For non-catch-all dynamic segments use { slug: string }.
- For catch-all use { slug: string[] } ; for optional catch-all use { slug?: string[] }.
- Narrow runtime values (e.g., assert/validate) before using narrower domain-specific types. [1][2]
Sources:
[1] Next.js docs — File-system conventions: Dynamic Segments (TypeScript examples).
[2] Next.js docs — Routing: Dynamic Routes / TypeScript section.
Trim API key and make config spreading explicit for clarity.
Trim whitespace from the API key before checking if it's empty—this prevents encrypting whitespace-only strings. Additionally, use ...(config ?? {}) to explicitly handle cases where config may be undefined, improving code clarity.
🛠️ Suggested fix
// Handle API key update
if (apiKey !== undefined) {
- if (apiKey === '') {
+ const trimmedApiKey = apiKey.trim();
+ if (trimmedApiKey === '') {
// Clear API key
updates.encryptedApiKey = null;
updates.validationStatus = 'unknown';
updates.validationMessage = null;
updates.lastValidatedAt = null;
} else {
// Encrypt and validate new API key
- updates.encryptedApiKey = await encrypt(apiKey.trim());
+ updates.encryptedApiKey = await encrypt(trimmedApiKey);
try {
- const mergedConfig = { ...(existing.config as Record<string, unknown> || {}), ...config };
- const validationResult = await definition.validate(mergedConfig, apiKey);
+ const mergedConfig = {
+ ...(existing.config as Record<string, unknown> || {}),
+ ...(config ?? {}),
+ };
+ const validationResult = await definition.validate(mergedConfig, trimmedApiKey);
updates.validationStatus = validationResult.valid ? 'valid' : 'invalid';
updates.validationMessage = validationResult.message;
updates.lastValidatedAt = new Date();🤖 Prompt for AI Agents
In `@apps/web/src/app/api/settings/integrations/`[integrationId]/route.ts around
lines 171 - 193, When handling apiKey in the route, trim whitespace first and
use the trimmed value for emptiness checks, encryption, and validation: create a
trimmedApiKey = apiKey?.trim() and if trimmedApiKey === '' treat it as clearing
the key (set updates.encryptedApiKey = null,
validationStatus/message/lastValidatedAt accordingly); otherwise encrypt
trimmedApiKey via encrypt(trimmedApiKey) and pass trimmedApiKey into
definition.validate. Also make the config spread explicit when building
mergedConfig by using ...(config ?? {}) merged with existing.config (e.g., const
mergedConfig = { ...(existing.config as Record<string, unknown> || {}),
...(config ?? {}) }) before calling definition.validate so undefined config is
handled clearly.
| enabledCount: number; | ||
| } | ||
|
|
||
| const fetcher = (url: string) => fetch(url, { credentials: 'include' }).then(r => r.json()); |
There was a problem hiding this comment.
Fetcher should throw on non-OK responses.
The fetcher doesn't check response.ok, so API errors (4xx/5xx) won't trigger SWR's error state. The error variable from useSWR may not be set correctly for failed requests.
🐛 Proposed fix
-const fetcher = (url: string) => fetch(url, { credentials: 'include' }).then(r => r.json());
+const fetcher = async (url: string) => {
+ const res = await fetch(url, { credentials: 'include' });
+ if (!res.ok) {
+ const error = new Error('Failed to fetch integrations');
+ throw error;
+ }
+ return res.json();
+};🤖 Prompt for AI Agents
In `@apps/web/src/app/settings/integrations/page.tsx` at line 81, The fetcher
function (const fetcher = (url: string) => ...) must check the Response.ok and
throw on non-OK responses so SWR receives an error; modify fetcher to await
fetch(url, { credentials: 'include' }), if !response.ok read the body (text or
json) and throw a new Error that includes response.status and the error body,
otherwise return await response.json(); this ensures useSWR's error variable is
populated for 4xx/5xx responses.
This commit adds the foundation for third-party integrations in PageSpace:
Database:
Integration System:
API Routes:
Settings UI:
AI Integration:
First Integration (Apify):
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.