Enterprise integrations - #8
Merged
Merged
Conversation
…ion, env-var Slack channel, upsert conflict key
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 26 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR integrates vector search capabilities into a Next.js application by introducing Supabase with pgvector support, OAuth authentication for Notion/Google Drive/Slack, document ingestion from multiple enterprise sources, and a retrieval pipeline that generates embeddings and performs semantic search over stored documents. Changes
Sequence DiagramssequenceDiagram
participant User
participant App as Next.js App
participant Ingestion as Ingestion Sources
participant Embed as Embedding Service
participant Supabase as Supabase DB
User->>App: Trigger syncUserKnowledge()
App->>Ingestion: Promise.all([fetchNotionPages(), fetchDriveDocuments(), fetchSlackChannelHistory()])
par Parallel Fetches
Ingestion-->>App: Notion pages
Ingestion-->>App: Drive documents
Ingestion-->>App: Slack messages
end
Note over App: Normalize to rawDocuments with provider tags
loop For each document
App->>Embed: Generate embedding from content
Embed-->>App: embedding vector (768-dim)
App->>Supabase: Upsert to runbook_documents<br/>(provider, external_id, embedding)
Supabase-->>App: Success or error
end
App-->>User: Log synced document count
sequenceDiagram
participant User
participant App as Next.js App
participant Embed as Embedding Service
participant Supabase as Supabase DB
User->>App: retrieveDocs(question)
App->>Embed: Generate embedding from question
Embed-->>App: query_embedding vector
App->>Supabase: RPC match_documents()<br/>(query_embedding, threshold=0.7, count=3)
Supabase->>Supabase: Cosine similarity search
Supabase-->>App: Top 3 matches with similarity scores
Note over App: Transform results to {id, title, content, score}
App-->>User: Retrieved documents
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 |
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
11-17:⚠️ Potential issue | 🔴 CriticalMissing runtime dependencies — next build will fail.
The PR adds modules with unmet imports:
src/lib/ingestion/gdrive.ts→import { google } from "googleapis";src/lib/ingestion/slack.ts→import { WebClient } from "@slack/web-api";src/lib/ingestion/notion.ts→import { Client } from "@notionhq/client";None of these packages are declared in
dependencies. The build will fail at module resolution, andnpm installwon't pull them transitively from@supabase/*.Add to
dependencies:"dependencies": { + "@notionhq/client": "^2.2.15", + "@slack/web-api": "^7.7.0", "@supabase/ssr": "^0.10.2", "@supabase/supabase-js": "^2.104.1", + "googleapis": "^144.0.0", "next": "16.2.4", "react": "19.2.4", "react-dom": "19.2.4" },Verify the latest stable releases on npm and pin to the versions used in development.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 11 - 17, The project is missing runtime dependencies required by src/lib/ingestion/gdrive.ts (import { google } from "googleapis"), src/lib/ingestion/slack.ts (import { WebClient } from "@slack/web-api"), and src/lib/ingestion/notion.ts (import { Client } from "@notionhq/client"); add "googleapis", "@slack/web-api", and "@notionhq/client" to package.json under dependencies (not devDependencies), pin each to the latest stable versions you used in development (verify on npm), update package.json accordingly and run npm install to ensure module resolution succeeds during next build.
🧹 Nitpick comments (7)
src/lib/ingestion/slack.ts (1)
11-11: Add explicitPromise<IngestionDoc[]>return type.Match
fetchNotionPages(): Promise<IngestionDoc[]>fromsrc/lib/ingestion/notion.ts:16to keep the ingestion sources interchangeable at the type level.-export async function fetchSlackChannelHistory(channelId: string) { +export async function fetchSlackChannelHistory( + channelId: string +): Promise<Array<{ id: string; title: string; content: string }>> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ingestion/slack.ts` at line 11, The function fetchSlackChannelHistory should declare an explicit return type Promise<IngestionDoc[]> to match fetchNotionPages and keep ingestion sources interchangeable; update the fetchSlackChannelHistory signature to return Promise<IngestionDoc[]>, ensure the IngestionDoc type is imported/used in src/lib/ingestion/slack.ts, and adjust any internal return values to conform to IngestionDoc[] if necessary.src/lib/ingestion/gdrive.ts (1)
17-17: Add explicitPromise<IngestionDoc[]>return type for parity withfetchNotionPages.
src/lib/ingestion/notion.tsdeclaresexport async function fetchNotionPages(): Promise<IngestionDoc[]>. Mirroring that here both documents the contract and surfaces shape drift at compile time whenvectorizer.tsconsumes both sources.-export async function fetchDriveDocuments() { +export async function fetchDriveDocuments(): Promise<Array<{ id: string; title: string; content: string }>> {(Or import/share the
IngestionDocinterface across the ingestion modules.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ingestion/gdrive.ts` at line 17, The fetchDriveDocuments function lacks an explicit return type; add Promise<IngestionDoc[]> to its signature (or import/shared IngestionDoc) so its contract matches fetchNotionPages and mismatches are caught when vectorizer.ts consumes both sources; update the export declaration for fetchDriveDocuments to export async function fetchDriveDocuments(): Promise<IngestionDoc[]> and ensure the IngestionDoc type is imported/used from the shared definition.src/utils/supabase/client.ts (1)
3-10: Validate env vars at module load instead of relying on!.If
NEXT_PUBLIC_SUPABASE_URLorNEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEYis missing, the non-null assertion will produce a confusing runtime failure deep inside@supabase/ssrrather than a clear configuration error at startup.♻️ Suggested validation
-const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL; -const supabaseKey = process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY; - -export const createClient = () => - createBrowserClient( - supabaseUrl!, - supabaseKey!, - ); +const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL; +const supabaseKey = process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY; + +if (!supabaseUrl || !supabaseKey) { + throw new Error( + "Missing NEXT_PUBLIC_SUPABASE_URL or NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY" + ); +} + +export const createClient = () => createBrowserClient(supabaseUrl, supabaseKey);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/supabase/client.ts` around lines 3 - 10, The module currently uses non-null assertions for supabaseUrl and supabaseKey which causes obfuscated runtime failures; update the top-level initialization to validate process.env.NEXT_PUBLIC_SUPABASE_URL and process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY when the module loads and throw a clear Error if either is missing, then have createClient call createBrowserClient with the validated values (referencing supabaseUrl, supabaseKey, createClient, and createBrowserClient) so failures surface as a descriptive startup/configuration error instead of deep library errors.supabase/migrations/00000000000000_init_vector_db.sql (1)
17-18: Use HNSW instead of IVFFlat for this index.IVFFlat performs poorly when the index is built on an empty table (as in this migration). It yields poor recall until after data is loaded and the index is explicitly
REINDEXed. HNSW has no such limitation and is the official Supabase recommendation for most workloads under 50M vectors.Suggested change
-CREATE INDEX runbook_documents_embedding_idx ON runbook_documents USING ivfflat (embedding vector_cosine_ops) -WITH (lists = 100); +CREATE INDEX runbook_documents_embedding_idx + ON runbook_documents + USING hnsw (embedding vector_cosine_ops);If IVFFlat must be retained, document a post-ingest
REINDEXstep and tunelists ≈ rows/1000.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/00000000000000_init_vector_db.sql` around lines 17 - 18, Summary: Replace IVFFlat with HNSW for the vector index because IVFFlat performs poorly on empty tables. Update the CREATE INDEX statement that defines runbook_documents_embedding_idx on runbook_documents to use USING hnsw (embedding vector_cosine_ops) instead of USING ivfflat, remove the WITH (lists = 100) clause (or replace it with HNSW params such as m and ef_construction if desired), and keep the index name and operator the same; if you decide to retain IVFFlat, instead add a note to the migration or README to run a post-ingest REINDEX and tune lists ≈ rows/1000.src/lib/retrieval.ts (1)
4-45: Consider preservingurlandproviderinRetrievedDoc.The RPC already returns
urlandprovider(permatch_documentsinsupabase/migrations/00000000000000_init_vector_db.sql), and the README to-do list mentions a chat panel that will surface sources. Dropping these fields here forces the chat UI / citations layer to re-fetch by id later. Keeping them now costs nothing.♻️ Suggestion
interface RetrievedDoc { doc: { id: string; title: string; content: string; + url: string | null; + provider: string; }; score: number; } ... return ((documents ?? []) as MatchedDocument[]).map((doc) => ({ doc: { id: doc.id, title: doc.title, - content: doc.content + content: doc.content, + url: doc.url ?? null, + provider: doc.provider, }, score: doc.similarity }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/retrieval.ts` around lines 4 - 45, The RetrievedDoc shape currently drops url and provider returned by the RPC; update the RetrievedDoc interface to include url and provider, and modify the mapping in retrieveDocs (and the MatchedDocument usage) to preserve doc.url and doc.provider into the returned structure so callers (chat UI / citation layer) can use source metadata without re-fetching by id; ensure any type assertions/casts still align with the updated RetrievedDoc and MatchedDocument definitions and the match_documents RPC.src/lib/vectorizer.ts (1)
13-17: Skip Slack ingestion when the channel id is missing instead of falling back to"".Defaulting
SLACK_ONBOARDING_CHANNEL_IDto""propagates an emptychannelintoslack.conversations.history({ channel: "" }), which (when a bot token is configured) will issue an authenticated API call that is guaranteed to fail withchannel_not_foundand is then swallowed by the catch iningestion/slack.ts. Cheap to short-circuit here.♻️ Suggestion
- const [notionPages, gDocs, slackMsgs] = await Promise.all([ - fetchNotionPages(), - fetchDriveDocuments(), - fetchSlackChannelHistory(process.env.SLACK_ONBOARDING_CHANNEL_ID ?? "") - ]); + const slackChannelId = process.env.SLACK_ONBOARDING_CHANNEL_ID; + const [notionPages, gDocs, slackMsgs] = await Promise.all([ + fetchNotionPages(), + fetchDriveDocuments(), + slackChannelId ? fetchSlackChannelHistory(slackChannelId) : Promise.resolve([]) + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/vectorizer.ts` around lines 13 - 17, The code currently always calls fetchSlackChannelHistory with SLACK_ONBOARDING_CHANNEL_ID defaulting to "" which causes a failing API call; update the Promise.all invocation so you only call fetchSlackChannelHistory when process.env.SLACK_ONBOARDING_CHANNEL_ID is truthy—otherwise skip the call and set slackMsgs to an empty array (or similar no-op result). Specifically, adjust the array passed to Promise.all around fetchSlackChannelHistory (keeping fetchNotionPages and fetchDriveDocuments intact) and handle the returned values from Promise.all to assign slackMsgs when the fetch was executed or an empty array when skipped.src/lib/ingestion/notion.ts (1)
23-53: RAG quality will be silently capped — no pagination and paragraph-only extraction.Two retrieval-quality gaps that are easy to miss in demos but bite real workspaces:
notion.search()andnotion.blocks.children.list()both paginate withnext_cursor; this code only reads the first page, so workspaces with >100 pages or pages with >100 blocks are silently truncated.- Only
block.type === "paragraph"is captured.heading_1/2/3,bulleted_list_item,numbered_list_item,to_do,quote,callout,code, and toggles are dropped — for many runbook pages this is the bulk of the content.♻️ Suggested direction
// loop until next_cursor is null for both search() and blocks.children.list() // extract rich_text from any block whose value has a `rich_text` array: const RICH_TEXT_BLOCKS = new Set([ "paragraph", "heading_1", "heading_2", "heading_3", "bulleted_list_item", "numbered_list_item", "to_do", "quote", "callout", "code", ]); // for (const block of blocks.results) { // if (!("type" in block) || !RICH_TEXT_BLOCKS.has(block.type)) continue; // const rt = (block as any)[block.type]?.rich_text ?? []; // content += rt.map((t: any) => t.plain_text).join("") + "\n"; // }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ingestion/notion.ts` around lines 23 - 53, The code only fetches the first page of results from notion.search and notion.blocks.children.list and only extracts paragraph blocks, causing silent truncation and lost content; update the logic in the function that calls notion.search and the block-fetching loop (referenced as notion.search and notion.blocks.children.list) to paginate until next_cursor is null (accumulate all response.results across pages) and replace the paragraph-only extraction with a rich-text extraction set (e.g., RICH_TEXT_BLOCKS including "paragraph","heading_1","heading_2","heading_3","bulleted_list_item","numbered_list_item","to_do","quote","callout","code") where for each block you check "type" in block, ensure the type is in that set, then read the rich_text array via block[block.type]?.rich_text and append plain_text; preserve the existing fallback to `Notion page: ${title}` and trim content before pushing into pages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 21-26: Replace the hardcoded Supabase credentials in README.md
(the NEXT_PUBLIC_SUPABASE_URL and NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY entries)
with generic placeholders (e.g. <your-supabase-project-url>,
<your-supabase-publishable-key>) and add a short note pointing readers to the
canonical secret store (1Password/Vault/team chat) for the real values so no
real project URL or publishable key is committed.
In `@src/app/api/auth/`[provider]/route.ts:
- Around line 10-40: The OAuth callback handler in route.ts is missing
validation of the OAuth "state" parameter; update the code that reads const {
searchParams } = new URL(req.url) to also read searchParams.get("state") and
verify it against the server-side expected value (e.g., a cookie or signed JWT
you previously issued for params.provider), returning NextResponse.json({ error:
"Invalid OAuth state" }, { status: 400 }) when it is absent or mismatched;
ensure this check occurs before treating the callback as a successful connection
and before returning the success payload so that params.provider only yields
success after state verification.
- Around line 6-46: The route handler GET treats params as a plain object but in
Next.js 16 params is a Promise; await the incoming params before using it (e.g.,
const { params } = await ... or const resolvedParams = await params) so checks
and logs use the real provider string; update all uses of params.provider in GET
(including the EXPECTED_PROVIDERS.includes check, error logs, and success
message) to read from the awaited/resolved params variable and return the same
responses as before.
In `@src/lib/ingestion/gdrive.ts`:
- Around line 9-15: The OAuth2 client is created via google.auth.OAuth2 but
never authenticated, so Drive calls (e.g., files.list) will 401; fix by loading
the user's refresh_token from your storage (e.g., Supabase) and calling
auth.setCredentials({ refresh_token }) before returning the client (the OAuth2
instance created with google.auth.OAuth2), and if no refresh_token is available
throw a clear error instead of returning an unauthenticated client; also update
the surrounding try/catch that currently swallows errors and returns [] to allow
auth failures to bubble (or convert them to a specific thrown error) so callers
don't treat a 0-result sync as success.
- Around line 33-38: The placeholder content in fetchDriveDocuments is producing
near-identical embeddings; replace the hard-coded template with the actual
Google Docs body by calling drive.files.export({ fileId: f.id!, mimeType:
"text/plain" }) for each file, map to async handlers and return Promise.all(...)
from fetchDriveDocuments so exports are awaited (compatible with
syncUserKnowledge's Promise.all), set each returned object's content to the
exported plain-text body (or a safe fallback like empty string on error), and
ensure downstream embedding via generateEmbedding uses that real content instead
of the template.
In `@src/lib/ingestion/slack.ts`:
- Around line 5-9: getSlackClient currently falls back to
process.env.SLACK_CLIENT_SECRET which is the OAuth app secret, not a bearer bot
token; remove that fallback and only use process.env.SLACK_BOT_TOKEN so
WebClient is instantiated with a proper bot token. Update getSlackClient to
check only SLACK_BOT_TOKEN (and return null if absent) and ensure no code paths
pass SLACK_CLIENT_SECRET into WebClient to avoid invalid_auth and leaking the
client secret.
- Around line 25-31: The current mapper returns synthetic IDs for messages
missing msg.ts (id: msg.ts || Date.now().toString()) and includes
empty/whitespace text, which breaks idempotent upserts (onConflict:
"provider,external_id") and pollutes vectors; change the ingestion to first
filter result.messages to only include items with a valid msg.ts and non-empty
non-whitespace msg.text, then map to { id: msg.ts, title: `Slack Message from
${msg.user || "Unknown"}`, content: msg.text } so no synthetic IDs are generated
and blank messages are skipped (this touches the mapping logic that produces
id/title/content and affects the upsert behavior defined by provider,external_id
in vectorizer.ts).
In `@src/lib/retrieval.ts`:
- Around line 33-45: The mapping over `documents` can throw if `documents` is
null (Supabase RPC returns T | null); in the function containing this block (the
vector search return logic) add a guard that checks for null/undefined
`documents` before mapping (e.g., if (!documents) return []) and only map when
`documents` is an array of MatchedDocument; keep the existing error log path for
`error` and return an empty array on null to safely handle the RPC nullable
return type.
In `@src/lib/supabase-admin.ts`:
- Around line 3-13: The file defines supabaseUrl and supabaseServiceKey with
placeholder fallbacks which can hide misconfiguration; change this to fail fast
by requiring process.env.NEXT_PUBLIC_SUPABASE_URL and
process.env.SUPABASE_SERVICE_ROLE_KEY to be present and throw a clear error if
missing before calling createClient (affecting supabaseUrl, supabaseServiceKey,
and supabaseAdmin); also verify no client-bundled code imports this module by
searching for imports of supabase-admin.ts and scanning src/app and files with
"use client" to ensure the service-role key never reaches the browser.
In `@src/lib/vectorizer.ts`:
- Around line 40-47: The upsert is invalid because onConflict:
"provider,external_id" requires a UNIQUE constraint that the migration
(supabase/migrations/00000000000000_init_vector_db.sql) doesn't create and the
constructed url `https://${doc.provider}.com/${doc.id}` is wrong for providers;
fix by either adding a UNIQUE constraint on (provider, external_id) in the
migration or change the upsert conflict target to the existing primary key
(e.g., onConflict: "id") in the supabaseAdmin.from("runbook_documents").upsert
call, and stop writing fabricated links by using the real URL if provided (e.g.,
url: doc.url ?? null) instead of the template
`https://${doc.provider}.com/${doc.id}` so null/empty is persisted until
provider modules supply correct page/webViewLink/getPermalink values.
In `@src/utils/supabase/middleware.ts`:
- Around line 7-37: The createClient function currently instantiates
createServerClient (supabase) but never calls supabase.auth.getUser(), so the
cookies setAll callback never runs and sessions aren't refreshed; move this file
to the repository root as middleware.ts, rename the exported function to
updateSession (or export default middleware) to avoid confusion with any server
createClient, and inside that function after creating the createServerClient
call invoke await supabase.auth.getUser() to trigger the cookies.setAll handler
and refresh tokens; ensure the function returns the updated NextResponse (using
NextResponse.next with the response.cookies populated) and is exported as the
Next.js middleware so it runs on each request.
In `@supabase/migrations/00000000000000_init_vector_db.sql`:
- Around line 5-14: The schema for runbook_documents is missing a UNIQUE
constraint/index for the (provider, external_id) pair which causes the upsert in
vectorizer.ts (onConflict: "provider,external_id") to fail; modify the CREATE
TABLE to either add a UNIQUE(provider, external_id) constraint or create a
UNIQUE INDEX on (provider, external_id), and decide how to handle nullable
external_id (make external_id NOT NULL if every source provides an id, or
instead create a partial unique index like UNIQUE(provider, external_id) WHERE
external_id IS NOT NULL so NULLs are allowed but duplicates for real IDs are
prevented).
---
Outside diff comments:
In `@package.json`:
- Around line 11-17: The project is missing runtime dependencies required by
src/lib/ingestion/gdrive.ts (import { google } from "googleapis"),
src/lib/ingestion/slack.ts (import { WebClient } from "@slack/web-api"), and
src/lib/ingestion/notion.ts (import { Client } from "@notionhq/client"); add
"googleapis", "@slack/web-api", and "@notionhq/client" to package.json under
dependencies (not devDependencies), pin each to the latest stable versions you
used in development (verify on npm), update package.json accordingly and run npm
install to ensure module resolution succeeds during next build.
---
Nitpick comments:
In `@src/lib/ingestion/gdrive.ts`:
- Line 17: The fetchDriveDocuments function lacks an explicit return type; add
Promise<IngestionDoc[]> to its signature (or import/shared IngestionDoc) so its
contract matches fetchNotionPages and mismatches are caught when vectorizer.ts
consumes both sources; update the export declaration for fetchDriveDocuments to
export async function fetchDriveDocuments(): Promise<IngestionDoc[]> and ensure
the IngestionDoc type is imported/used from the shared definition.
In `@src/lib/ingestion/notion.ts`:
- Around line 23-53: The code only fetches the first page of results from
notion.search and notion.blocks.children.list and only extracts paragraph
blocks, causing silent truncation and lost content; update the logic in the
function that calls notion.search and the block-fetching loop (referenced as
notion.search and notion.blocks.children.list) to paginate until next_cursor is
null (accumulate all response.results across pages) and replace the
paragraph-only extraction with a rich-text extraction set (e.g.,
RICH_TEXT_BLOCKS including
"paragraph","heading_1","heading_2","heading_3","bulleted_list_item","numbered_list_item","to_do","quote","callout","code")
where for each block you check "type" in block, ensure the type is in that set,
then read the rich_text array via block[block.type]?.rich_text and append
plain_text; preserve the existing fallback to `Notion page: ${title}` and trim
content before pushing into pages.
In `@src/lib/ingestion/slack.ts`:
- Line 11: The function fetchSlackChannelHistory should declare an explicit
return type Promise<IngestionDoc[]> to match fetchNotionPages and keep ingestion
sources interchangeable; update the fetchSlackChannelHistory signature to return
Promise<IngestionDoc[]>, ensure the IngestionDoc type is imported/used in
src/lib/ingestion/slack.ts, and adjust any internal return values to conform to
IngestionDoc[] if necessary.
In `@src/lib/retrieval.ts`:
- Around line 4-45: The RetrievedDoc shape currently drops url and provider
returned by the RPC; update the RetrievedDoc interface to include url and
provider, and modify the mapping in retrieveDocs (and the MatchedDocument usage)
to preserve doc.url and doc.provider into the returned structure so callers
(chat UI / citation layer) can use source metadata without re-fetching by id;
ensure any type assertions/casts still align with the updated RetrievedDoc and
MatchedDocument definitions and the match_documents RPC.
In `@src/lib/vectorizer.ts`:
- Around line 13-17: The code currently always calls fetchSlackChannelHistory
with SLACK_ONBOARDING_CHANNEL_ID defaulting to "" which causes a failing API
call; update the Promise.all invocation so you only call
fetchSlackChannelHistory when process.env.SLACK_ONBOARDING_CHANNEL_ID is
truthy—otherwise skip the call and set slackMsgs to an empty array (or similar
no-op result). Specifically, adjust the array passed to Promise.all around
fetchSlackChannelHistory (keeping fetchNotionPages and fetchDriveDocuments
intact) and handle the returned values from Promise.all to assign slackMsgs when
the fetch was executed or an empty array when skipped.
In `@src/utils/supabase/client.ts`:
- Around line 3-10: The module currently uses non-null assertions for
supabaseUrl and supabaseKey which causes obfuscated runtime failures; update the
top-level initialization to validate process.env.NEXT_PUBLIC_SUPABASE_URL and
process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY when the module loads and throw
a clear Error if either is missing, then have createClient call
createBrowserClient with the validated values (referencing supabaseUrl,
supabaseKey, createClient, and createBrowserClient) so failures surface as a
descriptive startup/configuration error instead of deep library errors.
In `@supabase/migrations/00000000000000_init_vector_db.sql`:
- Around line 17-18: Summary: Replace IVFFlat with HNSW for the vector index
because IVFFlat performs poorly on empty tables. Update the CREATE INDEX
statement that defines runbook_documents_embedding_idx on runbook_documents to
use USING hnsw (embedding vector_cosine_ops) instead of USING ivfflat, remove
the WITH (lists = 100) clause (or replace it with HNSW params such as m and
ef_construction if desired), and keep the index name and operator the same; if
you decide to retain IVFFlat, instead add a note to the migration or README to
run a post-ingest REINDEX and tune lists ≈ rows/1000.
🪄 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: badc7df1-f576-476b-9d03-0fb596988b2b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
README.mdpackage.jsonsrc/app/api/auth/[provider]/route.tssrc/lib/ingestion/gdrive.tssrc/lib/ingestion/notion.tssrc/lib/ingestion/slack.tssrc/lib/retrieval.tssrc/lib/supabase-admin.tssrc/lib/vectorizer.tssrc/utils/supabase/client.tssrc/utils/supabase/middleware.tssrc/utils/supabase/server.tssupabase/migrations/00000000000000_init_vector_db.sql
Comment on lines
+10
to
+40
| try { | ||
| const { searchParams } = new URL(req.url); | ||
| const code = searchParams.get("code"); | ||
| const error = searchParams.get("error"); | ||
|
|
||
| // Fallback parsing for the user | ||
| // The user will set these blank strings later when they register apps. | ||
| const EXPECTED_PROVIDERS = ["notion", "google", "slack"]; | ||
|
|
||
| if (!EXPECTED_PROVIDERS.includes(params.provider)) { | ||
| return NextResponse.json({ error: "Unsupported platform provider" }, { status: 400 }); | ||
| } | ||
|
|
||
| if (error) { | ||
| console.error(`OAuth error from ${params.provider}:`, error); | ||
| return NextResponse.json({ error: "User rejected OAuth handshake" }, { status: 403 }); | ||
| } | ||
|
|
||
| if (!code) { | ||
| return NextResponse.json({ error: "Missing authorization code" }, { status: 400 }); | ||
| } | ||
|
|
||
| // In a live system, we would exchange this 'code' for a RefreshToken and secure them inside our unified Supabase Auth table. | ||
| // e.g. await fetch("https://slack.com/api/oauth.v2.access", { ... }) | ||
| // await supabaseAdmin.from('provider_tokens').upsert({ provider: params.provider, token: ... }) | ||
|
|
||
| return NextResponse.json({ | ||
| success: true, | ||
| message: `Successfully connected ${params.provider}! Background synchronization started.`, | ||
| warning: "Demo Mode: App Token exchange gracefully bypassed due to missing API keys." | ||
| }); |
There was a problem hiding this comment.
Missing OAuth state validation — CSRF risk on the callback.
The handler accepts the callback without verifying a state parameter against a value the server previously issued (typically a signed cookie/JWT bound to the user's session). In OAuth 2.0 this is the primary defence against CSRF on the redirect URI; without it an attacker can trick a logged-in user into linking the attacker's third-party account.
Even though tokens aren't yet exchanged, the response declares success: true and "Successfully connected …", which downstream UI is likely to treat as a binding event. Add state issuance/verification before this is wired to a real token exchange or persistence step.
🛡️ Sketch of the missing check
const state = searchParams.get("state");
const expected = req.cookies.get(`oauth_state_${provider}`)?.value;
if (!state || !expected || state !== expected) {
return NextResponse.json({ error: "Invalid OAuth state" }, { status: 400 });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/auth/`[provider]/route.ts around lines 10 - 40, The OAuth
callback handler in route.ts is missing validation of the OAuth "state"
parameter; update the code that reads const { searchParams } = new URL(req.url)
to also read searchParams.get("state") and verify it against the server-side
expected value (e.g., a cookie or signed JWT you previously issued for
params.provider), returning NextResponse.json({ error: "Invalid OAuth state" },
{ status: 400 }) when it is absent or mismatched; ensure this check occurs
before treating the callback as a successful connection and before returning the
success payload so that params.provider only yields success after state
verification.
- auth route: await params Promise (Next.js 16 breaking change) - supabase-admin: lazy Proxy init, fail-fast at runtime not build time - notion.ts: paginated search + blocks, extract all rich-text block types - slack.ts: use SLACK_BOT_TOKEN only, filter blank messages, no synthetic IDs - gdrive.ts: real file export via files.export, require refresh_token - retrieval.ts: null guard on documents, preserve url+provider for citations - vectorizer.ts: skip Slack when channel missing, url=null not fabricated - SQL migration: UNIQUE(provider,external_id), HNSW index, external_id NOT NULL - middleware.ts: renamed updateSession, added getUser() session refresh - client.ts: moved env validation inside function body - README: replaced hardcoded credentials with placeholders - chat/route.ts: added missing await on async retrieveDocs
This was referenced Apr 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores