feat: add manager onboarding control plane with hire-scoped AI context - #10
Merged
Merged
Conversation
Enable managers to create and manage hires, attach per-hire knowledge sources, and run targeted sync so dashboard tasks, chat, and lessons operate with the selected hire context for end-to-end onboarding demos. Made-with: Cursor
|
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 36 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 selected for processing (18)
📝 WalkthroughWalkthroughThe PR transitions the system from a demo trainee-centric platform to a manager-controlled multi-hire onboarding control plane. It introduces hire CRUD operations, per-hire knowledge source attachment, hire-scoped document retrieval, and updates task/chat/lesson endpoints to operate with hire IDs instead of fixed trainee personas. Changes
Sequence DiagramssequenceDiagram
actor Manager
participant UI as Manager UI
participant API as /api/manager/hires/*
participant DataStore as DataStore
participant Sync as /api/sync/knowledge
participant Vectorizer as Vectorizer
participant Supabase as Supabase VectorDB
Manager->>UI: Add hire & attach knowledge sources
UI->>API: POST /manager/hires (create hire)
API->>DataStore: addHire()
DataStore-->>API: Hire created
API-->>UI: { hireId, name, ... }
Manager->>UI: Attach knowledge source to hire
UI->>API: POST /manager/hires/[hireId]/sources
API->>DataStore: addHireSource(hireId, source)
DataStore-->>API: Source attached
API-->>UI: { sourceId, type, title, ... }
Manager->>UI: Sync selected hire
UI->>Sync: POST /api/sync/knowledge { hireId }
Sync->>DataStore: getHireSources(hireId)
DataStore-->>Sync: [ source1, source2, ... ]
Sync->>Vectorizer: syncUserKnowledge(hireId)
Vectorizer->>Vectorizer: Load sources, generate docs, tag [hire:hireId]
Vectorizer->>Supabase: Upsert scoped documents
Supabase-->>Vectorizer: Success
Vectorizer-->>Sync: { scope: { hireId }, sourceCount }
Sync-->>UI: Sync complete
sequenceDiagram
participant User
participant ChatUI as Chat Interface
participant ChatAPI as /api/chat
participant Retrieval as retrieveDocs()
participant Supabase as Supabase VectorDB
participant LLM as Gemini
User->>ChatUI: Send message (context: hireId)
ChatUI->>ChatAPI: POST { message, hireId }
ChatAPI->>Retrieval: retrieveDocs(question, hireId)
Retrieval->>Supabase: Vector search
Supabase-->>Retrieval: All matching documents
Retrieval->>Retrieval: Filter by hire scope:<br/>- [hire:hireId]<br/>- [hire:global]<br/>- no [hire:...] marker
Retrieval-->>ChatAPI: [ scoped docs ]
ChatAPI->>LLM: Generate response with hire-scoped context
LLM-->>ChatAPI: Response
ChatAPI-->>ChatUI: Message + sources
ChatUI-->>User: Display response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/app/manager/page.tsx (1)
22-22:⚠️ Potential issue | 🟡 MinorSwitch selection state and React keys from
nametohireId.
PersonSummarynow carries a stablehireId, but the component still usesperson.nameas the selection identity (activeEmployee/selectedEmployee, theselectedPersonlookup) and as React keys for the filter buttons, the per-employee<article>, and the at-risk<li>. Two hires with the same display name would collide on keys and be conflated in selection — a real possibility now that managers can freely add hires. UsinghireIdremoves this latent bug and decouples selection from rename operations.♻️ Proposed fixes
- const [activeEmployee, setActiveEmployee] = useState<string>("ALL"); + const [activeHireId, setActiveHireId] = useState<string>("ALL"); @@ - const employeeNames = people.map((person) => person.name); - const selectedEmployee = - activeEmployee === "ALL" || employeeNames.includes(activeEmployee) ? activeEmployee : "ALL"; - const selectedPerson = people.find((person) => person.name === selectedEmployee) || null; - const visiblePeople = selectedEmployee === "ALL" ? people : selectedPerson ? [selectedPerson] : []; + const knownHireIds = new Set(people.map((person) => person.hireId)); + const selectedHireId = + activeHireId === "ALL" || knownHireIds.has(activeHireId) ? activeHireId : "ALL"; + const selectedPerson = people.find((person) => person.hireId === selectedHireId) || null; + const visiblePeople = selectedHireId === "ALL" ? people : selectedPerson ? [selectedPerson] : []; @@ - {people.map((person) => ( - <button - key={person.name} - type="button" - onClick={() => setActiveEmployee(person.name)} + {people.map((person) => ( + <button + key={person.hireId} + type="button" + onClick={() => setActiveHireId(person.hireId)} className={`rounded-full border px-3 py-1 text-sm transition ${ - selectedEmployee === person.name + selectedHireId === person.hireId @@ - <article key={person.name} className="rounded border border-slate-700 bg-slate-950 p-4"> + <article key={person.hireId} className="rounded border border-slate-700 bg-slate-950 p-4"> @@ - {atRisk.map((person) => ( - <li key={person.name}> + {atRisk.map((person) => ( + <li key={person.hireId}>Update the "ALL" button's
onClicktosetActiveHireId("ALL")similarly.Also applies to: 64-67, 118-131, 134-135, 222-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/manager/page.tsx` at line 22, The selection and React keys currently use person.name (activeEmployee / selectedEmployee / selectedPerson lookup and button/article/li keys) which can collide; update state and handlers to use hireId instead: rename or replace activeEmployee/setActiveEmployee to activeHireId/setActiveHireId (or keep names but store hireId strings), change any setActiveEmployee("ALL") calls to setActiveHireId("ALL"), update PersonSummary selection checks to compare hireId (e.g., person.hireId === activeHireId), and replace all React key props that use person.name with person.hireId (including filter buttons, per-employee <article>, and at-risk <li>). Ensure selectedEmployee lookup logic and any variables referenced at lines noted (64-67, 118-131, 134-135, 222-227) use hireId consistently.src/lib/dataStore.ts (1)
413-443:⚠️ Potential issue | 🟡 MinorInactive/deleted hires can leak into
employeeswhiletotalEmployeesignores them.
groupedis seeded fromactiveHiresbut tasks are added by theirassigneeId, so a task whoseassigneeIdreferences an inactive hire (or one that was hard-deleted) creates an extra entry. The downstreamemployeesarray then includes that hire whiletotalEmployees: activeHires.lengthdoes not — the manager UI ends up summing N+ employees but reporting a smaller total.Either filter
tasksbyactiveHiresids, or include their owners intotalEmployees.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/dataStore.ts` around lines 413 - 443, In getManagerOverview, tasks for inactive or deleted hires leak into employees because grouping uses task.assigneeId but totalEmployees uses activeHires.length; fix by filtering tasks to only include assigneeIds present in activeHires (or alternatively compute totalEmployees from the grouped keys). Concretely, before reducing into grouped, build a Set of activeHires ids (from activeHires.map(h=>h.id)) and filter the tasks array to tasks.filter(t => activeIds.has(t.assigneeId)), then proceed with the existing reduce and employee mapping so employees and totalEmployees remain consistent; referenced symbols: getManagerOverview, activeHires, tasks, grouped, employees.src/app/dashboard/page.tsx (1)
47-53:⚠️ Potential issue | 🟡 MinorInitial chat welcome still uses the demo persona name.
DEMO_PERSONAS.newHire.nameis baked into the assistant's welcome message even though every other surface (heading, empty state) followsselectedHire?.name. Once a real hire is loaded/selected, the chat opener will mismatch the rest of the dashboard. Consider deriving the welcome message after hires load (e.g., via an effect that updates the seed message whenselectedHirefirst resolves), or postponing the message until then.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/dashboard/page.tsx` around lines 47 - 53, The initial assistant welcome is hard-coded to DEMO_PERSONAS.newHire.name in the messages state; change this so the welcome uses the real hire name (selectedHire?.name) once a hire is loaded: initialize messages without the demo-specific seed (or with a generic placeholder), then add/update the assistant seed message inside a useEffect that watches selectedHire and calls setMessages to replace or insert the message with id "assistant-welcome" and role "assistant" using selectedHire.name (or postpone creating the seed until selectedHire is non-null). Target the messages state, setMessages, and the "assistant-welcome" message in page.tsx.src/lib/retrieval.ts (1)
36-50:⚠️ Potential issue | 🟠 MajorHire-scoped filter on top-3 RPC results can collapse to zero matches.
match_documentsis asked for only the top 3 hits (line 39) and the hire-scope filter is applied client-side afterwards (line 49). When the index contains documents from multiple hires (especially given thatvectorizer.tsnow writes hire-prefixed copies of every global Notion/Drive/Slack doc per sync), the top‑3 by similarity are likely to be other-hire copies of the same content, leaving an emptyscoped[]for the requesting hire even though semantically relevant matches exist further down the ranking.Either over-fetch and then filter-and-trim, or push hire scoping into the RPC so the database returns top‑K within the requested scope.
♻️ Suggested over-fetch + client-side trim
- const { data: documents, error } = await supabaseAdmin.rpc("match_documents", { - query_embedding: `[${embedding.join(",")}]`, - match_threshold: 0.7, - match_count: 3 - }); + const TOP_K = 3; + const { data: documents, error } = await supabaseAdmin.rpc("match_documents", { + query_embedding: `[${embedding.join(",")}]`, + match_threshold: 0.7, + // Over-fetch so the post-filter for hire scope still has candidates. + match_count: hireId ? TOP_K * 10 : TOP_K + }); @@ - const scoped = (documents as MatchedDocument[]).filter((doc) => matchesHireScope(doc.content, hireId)); - return scoped.map((doc) => ({ + const scoped = (documents as MatchedDocument[]) + .filter((doc) => matchesHireScope(doc.content, hireId)) + .slice(0, TOP_K); + return scoped.map((doc) => ({A cleaner long-term fix is to add a hire/scope parameter to the SQL
match_documentsfunction so filtering happens beforeLIMIT.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/retrieval.ts` around lines 36 - 50, The current retrieval calls the RPC match_documents with match_count: 3 and then client-side filters with matchesHireScope(hireId), which can drop all results; either increase match_count (e.g., to 10–20) and then filter-and-trim to return up to 3 hire-scoped MatchedDocument results, or modify the RPC (match_documents) to accept a hire/scope parameter so the DB returns top-K within that hire and update retrieval.ts to pass hireId into the RPC and stop client-side scoping; ensure the returned shape still maps to the same scoped.map(...) output.src/lib/vectorizer.ts (1)
45-98:⚠️ Potential issue | 🟠 MajorPer-hire sync duplicates the entire global corpus into the index.
When
hireIdis provided,notionPages,gDocs, andslackMsgs(which are workspace-global) are each prefixed with[hire:<hireId>]and upserted underexternal_id = ${hireId}:${doc.id}(lines 91–93). Running this for N hires produces N near-identical embeddings of the same Notion/Drive/Slack document, which:
- Multiplies embedding API cost by N for content that hasn't changed
- Bloats
runbook_documentsand the vector index- Causes retrieval to compete top‑K slots between scope-clones of the same content (compounding the post-filter issue in
src/lib/retrieval.ts)- Leaves orphan rows when a hire is deleted (no cleanup path is visible here)
Consider treating only
hireSourcesas hire-scoped and keeping the global Notion/Drive/Slack ingestion at[hire:global]regardless of which trigger ran. Something like:♻️ Sketch
- const rawDocuments = [ - ...notionPages.map((p) => ({ ...p, provider: "notion" as const })), - ...gDocs.map((d) => ({ ...d, provider: "google_drive" as const })), - ...slackMsgs.map((m) => ({ ...m, provider: "slack" as const })), - ...hireSources.map((source) => ({ + const globalDocs = [ + ...notionPages.map((p) => ({ ...p, provider: "notion" as const, scope: "global" as const })), + ...gDocs.map((d) => ({ ...d, provider: "google_drive" as const, scope: "global" as const })), + ...slackMsgs.map((m) => ({ ...m, provider: "slack" as const, scope: "global" as const })), + ]; + const hireDocs = hireSources.map((source) => ({ id: source.id, title: source.title, - content: `${scopeToken}\nKnowledge source URL: ${source.url}\nProvider type: ${source.type}`, + content: `[hire:${source.hireId}]\nKnowledge source URL: ${source.url}\nProvider type: ${source.type}`, provider: providerForType(source.type), - url: source.url - })) - ]; + url: source.url, + scope: source.hireId, + })); + const rawDocuments = [...globalDocs, ...hireDocs]; @@ - external_id: hireId ? `${hireId}:${doc.id}` : doc.id, - title: `${scopeToken} ${doc.title}`, - content: doc.content.includes(scopeToken) ? doc.content : `${scopeToken}\n${doc.content}`, + external_id: doc.scope === "global" ? doc.id : `${doc.scope}:${doc.id}`, + title: `[hire:${doc.scope}] ${doc.title}`, + content: doc.content.includes(`[hire:${doc.scope}]`) ? doc.content : `[hire:${doc.scope}]\n${doc.content}`,You'd also want a delete-on-hire path so removing a hire purges its
external_id LIKE 'hireId:%'rows.Add an index/cleanup migration for
external_idand a deletion routine tied todeleteHireso per-hire sync state doesn't accumulate indefinitely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/vectorizer.ts` around lines 45 - 98, The current per-hire sync prefixes global docs (notionPages, gDocs, slackMsgs) with the hireId causing N duplicates; change rawDocuments construction and upsert logic so only hireSources are stored with hire-scoped external_ids (e.g. `${hireId}:${source.id}`) while workspace-global docs use a global external_id (e.g. `global:${doc.id}` or just `doc.id`) regardless of hireId; update the upsert call in the block that uses generateEmbedding and supabaseAdmin.from("runbook_documents").upsert to compute external_id based on a new doc.scopeType/field (hire vs global) and avoid prefixing global providers, add a cleanup function (e.g. hook into deleteHire) that deletes rows WHERE external_id LIKE `${hireId}:%`, and add a DB migration to index external_id and support efficient deletion/cleanup.
🧹 Nitpick comments (6)
src/lib/types.ts (1)
31-38: Consider exporting a runtime allowlist alongside the union.The
KnowledgeSourceTypevalues are duplicated assourceTypes/allowedarrays insrc/app/api/manager/hires/[hireId]/sources/route.tsandsrc/lib/dataStore.ts(normalizeSourceType). Exporting a singleas consttuple here and deriving the union prevents these lists from drifting.♻️ Proposed refactor
- export type KnowledgeSourceType = - | "notion_page" - | "notion_database" - | "google_doc" - | "google_drive_folder" - | "google_drive_file" - | "slack_channel" - | "url"; + export const KNOWLEDGE_SOURCE_TYPES = [ + "notion_page", + "notion_database", + "google_doc", + "google_drive_folder", + "google_drive_file", + "slack_channel", + "url" + ] as const; + + export type KnowledgeSourceType = (typeof KNOWLEDGE_SOURCE_TYPES)[number];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types.ts` around lines 31 - 38, Export a single runtime allowlist tuple (e.g., SOURCE_TYPES as const) in this module and derive the existing KnowledgeSourceType union from it (using typeof SOURCE_TYPES[number]); update usages of the duplicated arrays (e.g., in route.ts and dataStore.ts/normalizeSourceType) to import and reference SOURCE_TYPES instead of hardcoded lists so there is a single source of truth for allowed source types.src/app/api/manager/hires/route.ts (1)
19-30: Optional: validate
.slice(0, N)onname/role/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/manager/hires/route.ts` around lines 19 - 30, The route currently accepts any non-empty string for name/role/email and persists it; add input sanitization by applying length bounds (e.g., name = String(body.name||"").trim().slice(0, N), role and email similarly) and validate email format with a simple regex before calling addHire; if the email is present but fails validation return NextResponse.json({ error: "invalid email" }, { status: 400 }); keep role/email optional (pass undefined when empty) and ensure you reference the existing variables name, role, email and the addHire call in route.ts when implementing these checks.src/app/api/lesson/route.ts (1)
28-32: Optional: extract the doc-resolution into a small helper.The chained
!doc && body.query ? (await retrieveDocs(...))[0]?.doc : undefinedis correct but dense, andbody.queryisn't currently sent by the dashboard caller — so this is effectively a future-facing branch. Pulling it into aresolveLessonDoc(docId, query, hireId)helper would make the intent (and the unused-from-UI path) explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/lesson/route.ts` around lines 28 - 32, Extract the dense doc-resolution logic into a small helper function named resolveLessonDoc(docId, query, hireId) that returns the resolved document or undefined; inside it, keep the existing behavior (if docId truthy return that, else if query call retrieveDocs(String(query), hireId) and return the first result's .doc, otherwise undefined). Replace the inline expression that computes scopedDoc/selectedDoc with a call to resolveLessonDoc(body.doc, body.query, hireId) and use its result as selectedDoc so the future-facing query branch is explicit and the route handler is clearer.src/app/api/manager/hires/[hireId]/sources/route.ts (1)
36-42: Cast before validation is a minor TS smell.
typeis asserted asKnowledgeSourceTypebefore being checked againstsourceTypes. Validate first, then narrow — that way the type assertion reflects an actually-validated value.♻️ Suggested refactor
- const type = String(body.type || "").trim() as KnowledgeSourceType; + const rawType = String(body.type || "").trim(); const title = String(body.title || "").trim(); const url = String(body.url || "").trim(); const providerRef = String(body.providerRef || "").trim(); - if (!sourceTypes.includes(type)) { + if (!sourceTypes.includes(rawType as KnowledgeSourceType)) { return NextResponse.json({ error: "Invalid source type" }, { status: 400 }); } + const type = rawType as KnowledgeSourceType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/manager/hires/`[hireId]/sources/route.ts around lines 36 - 42, Read the raw input into a temporary string (e.g., const rawType = String(body.type || "").trim()), validate it against sourceTypes using sourceTypes.includes(rawType), return the 400 error if invalid, and only after successful validation narrow it to the proper type (e.g., const type = rawType as KnowledgeSourceType); apply the same pattern for other fields if applicable so assertions happen after validation and the variable type reflects a validated value.src/lib/vectorizer.ts (1)
88-114: Duplicate logic between primary upsert and fallback insert.The two branches (lines 88–98 and 107–114) repeat the row payload verbatim. Easy place for the two paths to drift after future schema changes (e.g., one updated, the other forgotten). Extract once.
♻️ Suggested extraction
+ const row = { + provider: doc.provider, + external_id: hireId ? `${hireId}:${doc.id}` : doc.id, + title: `${scopeToken} ${doc.title}`, + content: doc.content.includes(scopeToken) ? doc.content : `${scopeToken}\n${doc.content}`, + url: "url" in doc ? doc.url : null, + embedding: `[${embedding.join(",")}]`, + }; const { error } = await supabaseAdmin.from("runbook_documents").upsert( - { - provider: doc.provider, - external_id: hireId ? `${hireId}:${doc.id}` : doc.id, - title: `${scopeToken} ${doc.title}`, - content: doc.content.includes(scopeToken) ? doc.content : `${scopeToken}\n${doc.content}`, - url: "url" in doc ? doc.url : null, - embedding: `[${embedding.join(",")}]`, - }, + row, { onConflict: "provider,external_id" } ); @@ - const { error: insertError } = await supabaseAdmin.from("runbook_documents").insert({ - provider: doc.provider, - external_id: hireId ? `${hireId}:${doc.id}` : doc.id, - title: `${scopeToken} ${doc.title}`, - content: doc.content.includes(scopeToken) ? doc.content : `${scopeToken}\n${doc.content}`, - url: "url" in doc ? doc.url : null, - embedding: `[${embedding.join(",")}]`, - }); + const { error: insertError } = await supabaseAdmin.from("runbook_documents").insert(row);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/vectorizer.ts` around lines 88 - 114, The upsert and fallback insert duplicate the same row object (built from doc, hireId, scopeToken, embedding, url), so extract that payload into a single constant (e.g., const row = {...}) before calling supabaseAdmin.from("runbook_documents") and reuse row for both upsert(...) and insert(...); ensure the embedding formatting (`[${embedding.join(",")}]`) and the scoped content logic (`doc.content.includes(scopeToken) ? doc.content : `${scopeToken}\n${doc.content}``) are preserved when building the extracted row so both paths use identical data.src/app/manager/tasks/page.tsx (1)
58-65: Source-loading effect has a race condition and no error handling.If the user toggles between hires quickly, an in-flight response for an earlier
selectedHireIdcan resolve after a later one and overwritesourceswith the wrong hire's data. The!res.okbranch is also silent. Consider an abort signal (or a stale-check guard) and surface a message on failure.♻️ Sketch
useEffect(() => { if (!selectedHireId) return; - void (async () => { - const res = await fetch(`/api/manager/hires/${selectedHireId}/sources`); - const data = await res.json(); - if (res.ok) setSources(data.sources || []); - })(); + const controller = new AbortController(); + void (async () => { + try { + const res = await fetch(`/api/manager/hires/${selectedHireId}/sources`, { signal: controller.signal }); + const data = await res.json(); + if (res.ok) setSources(data.sources || []); + else setMessage(data.error || "Failed to load sources."); + } catch (err) { + if ((err as Error).name !== "AbortError") console.error(err); + } + })(); + return () => controller.abort(); }, [selectedHireId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/manager/tasks/page.tsx` around lines 58 - 65, The effect that loads sources (useEffect tied to selectedHireId) has a race and no error handling; update it to create an AbortController for the fetch and pass its signal, and when the component/selectedHireId changes call controller.abort() to cancel stale requests; additionally, after awaiting the fetch check for signal.aborted (or catch an AbortError) before calling setSources to avoid overwriting with stale data, and handle non-ok responses by setting an error state or logging the response error (e.g., reference useEffect, selectedHireId, setSources and the fetch(`/api/manager/hires/${selectedHireId}/sources`) call so you add abort logic and error handling around that fetch).
🤖 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`:
- Line 42: In the README list item "Select the hire and attach knowledge links
(Notion pages/databases, Google docs/folders/files, Slack channels, URLs)"
change the product name casing by replacing "Google docs" with "Google Docs" so
the line reads "Select the hire and attach knowledge links (Notion
pages/databases, Google Docs/folders/files, Slack channels, URLs)"; locate the
exact sentence in README.md and update only that substring.
In `@src/app/api/manager/hires/`[hireId]/route.ts:
- Around line 10-28: PATCH handler currently does a raw await req.json() which
throws on malformed/empty JSON; change it to parse tolerantly with
req.json().catch(() => ({})) and then validate the result is a plain object
(e.g., typeof body === "object" && body !== null) before proceeding; if
validation fails return NextResponse.json({ error: "Invalid JSON body" }, {
status: 400 }); keep using updateHire(hireId, { ... }) and the same per-field
type guards for name/role/email/active, and preserve the existing not-found and
500 error handling.
In `@src/app/api/manager/hires/`[hireId]/sources/route.ts:
- Around line 1-59: Both GET and POST in this route lack auth/authorization;
before calling getHireSources or addHireSource, verify the caller's session and
that they are allowed to act on the given hireId. Import and call your
Supabase/session helper (or create a helper like getSession/getCurrentUser) at
the top of both handlers (GET and POST), return 401 if no session, then perform
an authorization check (e.g., isUserAuthorizedForHire(user.id, hireId) or
compare to hire.ownerId fetched from the datastore) and return 403 if
unauthorized; only then call getHireSources or addHireSource and proceed. Ensure
providerRef/title/url validation remains the same and use 401/403 responses for
auth failures.
In `@src/app/api/manager/hires/route.ts`:
- Around line 16-24: The POST handler currently calls await req.json() which
will throw on missing/invalid JSON and is bubbled to the outer catch returning
500; update the POST function to separately handle JSON parsing errors: wrap the
await req.json() in its own try/catch (or validate req.body) and when parsing
fails return NextResponse.json({ error: "invalid or missing JSON body" }, {
status: 400 }) so malformed input maps to 400; keep the existing name/role/email
checks and only let unexpected runtime errors fall through to the outer catch.
In `@src/app/api/tasks/`[taskId]/route.ts:
- Around line 43-56: The current flow lets an empty or missing assigneeIds
silently succeed because assigneeIds defaults to [] and the guard only errors
when the client provided IDs but none are valid; change the guard to require at
least one valid hire ID by checking validAssigneeIds.length instead of
assigneeIds.length (i.e., if (validAssigneeIds.length === 0) return
NextResponse.json({ error: "No valid assignees provided" }, { status: 400 })),
locating the logic around assigneeIds, validAssigneeIds, getHires, and
duplicateTask to implement this stricter validation before calling
duplicateTask(taskId, validAssigneeIds).
In `@src/app/api/tasks/route.ts`:
- Around line 30-37: The current logic computes chosenAssigneeId from assigneeId
or the first active hire and then calls addTask, which lets an unknown non-empty
assigneeId silently fall back to defaultHire; update the route handler to
validate a provided assigneeId against the activeHires list (use getHires() and
activeHires), and if assigneeId is non-empty but not found in activeHires return
NextResponse.json(..., { status: 400 }) with an explanatory error; only allow
passing assigneeId through to addTask when it exists in activeHires (otherwise
use the first active hire or fail as you already do).
In `@src/app/dashboard/page.tsx`:
- Around line 81-94: loadHires currently preserves the previous selectedHireId
even if that hire is no longer in the refreshed active list; update the
setSelectedHireId call inside loadHires to re-validate the existing selection
against the new active hires (e.g., compute a set/array of active ids from
active and in the updater callback for setSelectedHireId check if current is in
that set, otherwise set to active[0]?.id or empty string), leaving
setHires(active) as is; reference loadHires, active, setHires, and
setSelectedHireId when making the change.
In `@src/app/manager/tasks/page.tsx`:
- Around line 141-149: When deleting a hire in deleteHire, also clear the
create-task form's assignee field to avoid a stale ID; if you have a setForm (or
similar) updater, update form.assigneeId to an empty string (or null) when the
removed hireId matches form.assigneeId. Update the deleteHire function to call
setForm(prev => ({ ...prev, assigneeId: "" })) (only when prev.assigneeId ===
hireId) along with the existing setHires, setSelectedHireId, setSources, and
setMessage calls.
In `@src/lib/dataStore.ts`:
- Around line 64-76: normalizeHire and normalizeHireSource currently overwrite
updatedAt with now, causing getHires/getHireSources to always detect a change
and rewrite files; change both normalize functions to preserve an existing
persisted updatedAt (i.e., if hire.updatedAt or source.updatedAt exists keep it)
and only set createdAt for new records, but do not set updatedAt there. Instead,
stamp updatedAt = nowIso() explicitly in the actual mutation code paths (the
functions that perform writes/runMutation and create/update operations for hires
and sources) so persisted updatedAt reflects real mutations and the
JSON.stringify comparison no longer always differs.
- Around line 207-237: The removeHire function currently filters out tasks
assigned to the deleted hire before deciding reassignment strategy, which causes
tasks to be lost when cascadeTasks is false but no remaining hires exist; change
the logic in removeHire (and related symbols getHires, getTasks, writeJson,
HIRES_FILE, TASKS_FILE, cascadeTasks, reassignToHireId) so you only remove or
change tasks after determining a reassignment target: if options?.cascadeTasks
=== true then filter out tasks with assigneeId === hireId; else if a
reassignedHire (found via options?.reassignToHireId or remainingHires[0]) exists
then map tasks to replace assigneeId/assignee for tasks assigned to hireId;
otherwise do not drop tasks — either keep tasks intact (write original tasks) or
return a non-success result — and ensure you do not run the initial
tasks.filter(...) before this decision.
In `@src/lib/retrieval.ts`:
- Around line 24-29: The current matchesHireScope function misclassifies
documents that merely contain the substring "[hire:"; change the check to
require the hire marker to be anchored (e.g., a whole line) instead of any
substring. Update matchesHireScope to look for markers that appear as standalone
lines like "[hire:ID]" or "[hire:global]" (use a line-aware regex or split by
lines and trim each line) so only explicit per-document scope markers are
honored; keep the behavior that if hireId is undefined it returns true and that
"[hire:global]" still matches. Also consider a follow-up to store scope in a
structured field instead of embedding it in content.
In `@src/lib/types.ts`:
- Around line 14-15: The Task type currently denormalizes display name via
fields assigneeId and assignee; update the implementation to avoid stale names
by deriving the display name from the Hire record at read-time or by backfilling
tasks when a hire is updated: change consumers that read task.display name to
perform a lookup/join against the Hire entity using assigneeId (or,
alternatively, add logic in PATCH /api/manager/hires/:hireId to update all Task
rows referencing that hire), and remove reliance on the assignee field in
types/Task (or mark it optional/derived) so code uses Hire.name for UI display
(referencing symbols assigneeId, assignee, Hire, and PATCH
/api/manager/hires/:hireId).
In `@src/lib/vectorizer.ts`:
- Around line 36-37: When syncUserKnowledge() is called with no hireId it
currently calls getHireSources(hireId) which returns ALL hire-attached sources
and then tags them with scopeToken="[hire:global]"; change the logic so that
when hireId is undefined you do NOT call getHireSources(undefined) nor include
hire-attached sources in rawDocuments. Instead call or filter for only truly
global/public sources (e.g., getGlobalSources or filter where source.hireId is
null) and build rawDocuments from those, keeping scopeToken="[hire:global]";
when hireId is present keep the existing path using getHireSources(hireId) and
tag docs with `[hire:${hireId}]`. Ensure references to getHireSources,
syncUserKnowledge, rawDocuments, scopeToken and hireId are updated accordingly
so no hire-specific sources are written under the global scope.
---
Outside diff comments:
In `@src/app/dashboard/page.tsx`:
- Around line 47-53: The initial assistant welcome is hard-coded to
DEMO_PERSONAS.newHire.name in the messages state; change this so the welcome
uses the real hire name (selectedHire?.name) once a hire is loaded: initialize
messages without the demo-specific seed (or with a generic placeholder), then
add/update the assistant seed message inside a useEffect that watches
selectedHire and calls setMessages to replace or insert the message with id
"assistant-welcome" and role "assistant" using selectedHire.name (or postpone
creating the seed until selectedHire is non-null). Target the messages state,
setMessages, and the "assistant-welcome" message in page.tsx.
In `@src/app/manager/page.tsx`:
- Line 22: The selection and React keys currently use person.name
(activeEmployee / selectedEmployee / selectedPerson lookup and button/article/li
keys) which can collide; update state and handlers to use hireId instead: rename
or replace activeEmployee/setActiveEmployee to activeHireId/setActiveHireId (or
keep names but store hireId strings), change any setActiveEmployee("ALL") calls
to setActiveHireId("ALL"), update PersonSummary selection checks to compare
hireId (e.g., person.hireId === activeHireId), and replace all React key props
that use person.name with person.hireId (including filter buttons, per-employee
<article>, and at-risk <li>). Ensure selectedEmployee lookup logic and any
variables referenced at lines noted (64-67, 118-131, 134-135, 222-227) use
hireId consistently.
In `@src/lib/dataStore.ts`:
- Around line 413-443: In getManagerOverview, tasks for inactive or deleted
hires leak into employees because grouping uses task.assigneeId but
totalEmployees uses activeHires.length; fix by filtering tasks to only include
assigneeIds present in activeHires (or alternatively compute totalEmployees from
the grouped keys). Concretely, before reducing into grouped, build a Set of
activeHires ids (from activeHires.map(h=>h.id)) and filter the tasks array to
tasks.filter(t => activeIds.has(t.assigneeId)), then proceed with the existing
reduce and employee mapping so employees and totalEmployees remain consistent;
referenced symbols: getManagerOverview, activeHires, tasks, grouped, employees.
In `@src/lib/retrieval.ts`:
- Around line 36-50: The current retrieval calls the RPC match_documents with
match_count: 3 and then client-side filters with matchesHireScope(hireId), which
can drop all results; either increase match_count (e.g., to 10–20) and then
filter-and-trim to return up to 3 hire-scoped MatchedDocument results, or modify
the RPC (match_documents) to accept a hire/scope parameter so the DB returns
top-K within that hire and update retrieval.ts to pass hireId into the RPC and
stop client-side scoping; ensure the returned shape still maps to the same
scoped.map(...) output.
In `@src/lib/vectorizer.ts`:
- Around line 45-98: The current per-hire sync prefixes global docs
(notionPages, gDocs, slackMsgs) with the hireId causing N duplicates; change
rawDocuments construction and upsert logic so only hireSources are stored with
hire-scoped external_ids (e.g. `${hireId}:${source.id}`) while workspace-global
docs use a global external_id (e.g. `global:${doc.id}` or just `doc.id`)
regardless of hireId; update the upsert call in the block that uses
generateEmbedding and supabaseAdmin.from("runbook_documents").upsert to compute
external_id based on a new doc.scopeType/field (hire vs global) and avoid
prefixing global providers, add a cleanup function (e.g. hook into deleteHire)
that deletes rows WHERE external_id LIKE `${hireId}:%`, and add a DB migration
to index external_id and support efficient deletion/cleanup.
---
Nitpick comments:
In `@src/app/api/lesson/route.ts`:
- Around line 28-32: Extract the dense doc-resolution logic into a small helper
function named resolveLessonDoc(docId, query, hireId) that returns the resolved
document or undefined; inside it, keep the existing behavior (if docId truthy
return that, else if query call retrieveDocs(String(query), hireId) and return
the first result's .doc, otherwise undefined). Replace the inline expression
that computes scopedDoc/selectedDoc with a call to resolveLessonDoc(body.doc,
body.query, hireId) and use its result as selectedDoc so the future-facing query
branch is explicit and the route handler is clearer.
In `@src/app/api/manager/hires/`[hireId]/sources/route.ts:
- Around line 36-42: Read the raw input into a temporary string (e.g., const
rawType = String(body.type || "").trim()), validate it against sourceTypes using
sourceTypes.includes(rawType), return the 400 error if invalid, and only after
successful validation narrow it to the proper type (e.g., const type = rawType
as KnowledgeSourceType); apply the same pattern for other fields if applicable
so assertions happen after validation and the variable type reflects a validated
value.
In `@src/app/api/manager/hires/route.ts`:
- Around line 19-30: The route currently accepts any non-empty string for
name/role/email and persists it; add input sanitization by applying length
bounds (e.g., name = String(body.name||"").trim().slice(0, N), role and email
similarly) and validate email format with a simple regex before calling addHire;
if the email is present but fails validation return NextResponse.json({ error:
"invalid email" }, { status: 400 }); keep role/email optional (pass undefined
when empty) and ensure you reference the existing variables name, role, email
and the addHire call in route.ts when implementing these checks.
In `@src/app/manager/tasks/page.tsx`:
- Around line 58-65: The effect that loads sources (useEffect tied to
selectedHireId) has a race and no error handling; update it to create an
AbortController for the fetch and pass its signal, and when the
component/selectedHireId changes call controller.abort() to cancel stale
requests; additionally, after awaiting the fetch check for signal.aborted (or
catch an AbortError) before calling setSources to avoid overwriting with stale
data, and handle non-ok responses by setting an error state or logging the
response error (e.g., reference useEffect, selectedHireId, setSources and the
fetch(`/api/manager/hires/${selectedHireId}/sources`) call so you add abort
logic and error handling around that fetch).
In `@src/lib/types.ts`:
- Around line 31-38: Export a single runtime allowlist tuple (e.g., SOURCE_TYPES
as const) in this module and derive the existing KnowledgeSourceType union from
it (using typeof SOURCE_TYPES[number]); update usages of the duplicated arrays
(e.g., in route.ts and dataStore.ts/normalizeSourceType) to import and reference
SOURCE_TYPES instead of hardcoded lists so there is a single source of truth for
allowed source types.
In `@src/lib/vectorizer.ts`:
- Around line 88-114: The upsert and fallback insert duplicate the same row
object (built from doc, hireId, scopeToken, embedding, url), so extract that
payload into a single constant (e.g., const row = {...}) before calling
supabaseAdmin.from("runbook_documents") and reuse row for both upsert(...) and
insert(...); ensure the embedding formatting (`[${embedding.join(",")}]`) and
the scoped content logic (`doc.content.includes(scopeToken) ? doc.content :
`${scopeToken}\n${doc.content}``) are preserved when building the extracted row
so both paths use identical data.
🪄 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: 3beed5b4-2cbd-4b72-aa0c-28d79aa65df6
📒 Files selected for processing (19)
.env.exampleREADME.mdsrc/app/api/chat/route.tssrc/app/api/lesson/route.tssrc/app/api/manager/hires/[hireId]/route.tssrc/app/api/manager/hires/[hireId]/sources/[sourceId]/route.tssrc/app/api/manager/hires/[hireId]/sources/route.tssrc/app/api/manager/hires/route.tssrc/app/api/sync/knowledge/route.tssrc/app/api/tasks/[taskId]/route.tssrc/app/api/tasks/route.tssrc/app/dashboard/page.tsxsrc/app/manager/page.tsxsrc/app/manager/tasks/page.tsxsrc/lib/dataStore.tssrc/lib/demoTasks.tssrc/lib/retrieval.tssrc/lib/types.tssrc/lib/vectorizer.ts
Comment on lines
+14
to
+15
| assigneeId: string; | ||
| assignee: string; |
There was a problem hiding this comment.
Heads-up: assignee (display name) is denormalized on the task row.
Storing both assigneeId and assignee means renaming a hire via PATCH /api/manager/hires/:hireId won’t propagate to existing tasks until they’re rewritten. That’s acceptable for a demo, but worth a follow-up to derive the display name from the Hire lookup at read-time (or backfill on hire update) so the UI doesn’t show stale names.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/types.ts` around lines 14 - 15, The Task type currently denormalizes
display name via fields assigneeId and assignee; update the implementation to
avoid stale names by deriving the display name from the Hire record at read-time
or by backfilling tasks when a hire is updated: change consumers that read
task.display name to perform a lookup/join against the Hire entity using
assigneeId (or, alternatively, add logic in PATCH /api/manager/hires/:hireId to
update all Task rows referencing that hire), and remove reliance on the assignee
field in types/Task (or mark it optional/derived) so code uses Hire.name for UI
display (referencing symbols assigneeId, assignee, Hire, and PATCH
/api/manager/hires/:hireId).
Harden hire/task APIs and manager UI state handling, correct hire-scoped retrieval/sync behavior, and tighten datastore consistency so hire lifecycle changes no longer create stale assignments, scope leaks, or unstable keys. Made-with: Cursor
Require authenticated and authorized hire access for manager source CRUD and hire-scoped sync endpoints so unauthenticated callers can no longer read or mutate hire-specific onboarding context. Made-with: Cursor
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.
Enable managers to create and manage hires, attach per-hire knowledge sources, and run targeted sync so dashboard tasks, chat, and lessons operate with the selected hire context for end-to-end onboarding demos.
Made-with: Cursor
Summary by CodeRabbit
New Features
Documentation