feat: add multi-tenancy and teams support#207
Merged
Conversation
📝 WalkthroughWalkthroughThis PR implements multi-tenancy support with team-based data isolation, replaces the Kuzu-based graph database with FalkorDB, and introduces comprehensive team management features. Changes span Docker compose configurations (transitioning services from Kuzu to FalkorDB-based crawler), platform backend (new team-scoped document access controls via row-level security, team membership APIs, and team-aware RAG context), frontend (team management UI components, document upload with team selection), and authentication (trusted headers extension for team information, session-level team storage). The RAG service gains multi-tenant dataset isolation, HNSW index auto-creation, and support for user/team-scoped queries. Document metadata is extended to track creator and associated teams. The schema adds new tables for Better Auth team records and enhanced document fields for multi-tenancy. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
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: 38
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
services/rag/app/routers/documents.py (1)
41-60: Update stub service signature to match cognee_service.add_document.The
_UnavailableCogneeService.add_document()method inservices/rag/app/services/cognee/__init__.pydoes not accept theuser_idanddataset_nameparameters, but the realCogneeService.add_document()method does. When cognee is unavailable, calls fromdocuments.py(lines 58-59) will raiseTypeError: unexpected keyword argument.Update the stub method signature to:
async def add_document( self, content: str, metadata: Optional[dict[str, Any]] = None, document_id: Optional[str] = None, user_id: Optional[str] = None, dataset_name: Optional[str] = None, ) -> dict[str, Any]:services/platform/app/(app)/dashboard/[id]/automations/layout.tsx (2)
99-109: Use semantic<button>element instead of<span>withrole="button".The static analysis correctly identifies that
<span role="button">should be replaced with an actual<button>element for proper accessibility and native keyboard interaction. The same applies to the automation name span below (lines 111-126).♿ Proposed fix for semantic elements
- <span - role="button" - tabIndex={0} - onClick={handleClickAutomation} - className={cn( - 'hidden md:inline text-foreground', - automation?.name && 'text-muted-foreground cursor-pointer', - )} - > - {t('title')} - </span> + <button + type="button" + onClick={handleClickAutomation} + className={cn( + 'hidden md:inline text-foreground bg-transparent border-none p-0 font-inherit text-inherit cursor-pointer', + automation?.name && 'text-muted-foreground', + )} + > + {t('title')} + </button>
111-127: Same accessibility concern: prefer<button>over<span role="button">.This clickable span for entering edit mode should also be a semantic
<button>element. TheonKeyDownhandler becomes unnecessary when using a native button since it handles Enter/Space natively.♿ Proposed fix
- {automation?.name && !editMode && ( - <span - role="button" - tabIndex={0} - className="text-foreground cursor-pointer" - onClick={() => setEditMode(true)} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - setEditMode(true); - } - }} - > - {/* "/" separator - hidden on mobile */} - <span className="hidden md:inline">/ </span> - {automation?.name} - </span> - )} + {automation?.name && !editMode && ( + <button + type="button" + className="text-foreground cursor-pointer bg-transparent border-none p-0 font-inherit text-inherit" + onClick={() => setEditMode(true)} + > + {/* "/" separator - hidden on mobile */} + <span className="hidden md:inline">/ </span> + {automation?.name} + </button> + )}
🤖 Fix all issues with AI agents
In @compose.yml:
- Around line 115-117: The docker-compose service currently exposes the Neo4j
Browser via the ports mapping '7474:7474' which should be disabled in
production; change the compose setup to make that mapping conditional (e.g.,
gate the '7474:7474' entry on an env var like ENABLE_NEO4J_BROWSER or move it
into a compose override file) and update documentation to instruct setting
ENABLE_NEO4J_BROWSER=false (or not including the override) for production so the
Neo4j Browser port is not published.
In @services/crawler/package.json:
- Line 12: The npm script "test:watch" in package.json uses the nonexistent
command "pytest-watch"; change it to the correct entrypoint for pytest-watch,
e.g., replace the command string with "ptw" (or "ptw <args>" as needed) so the
"test:watch" script runs the installed pytest-watch tool; update the
"test:watch" script value in package.json accordingly.
In @services/graph-db/Dockerfile:
- Around line 4-25: Add a Docker HEALTHCHECK to the Dockerfile that probes Neo4j
after startup (use curl against the HTTP endpoint on localhost:7474 with basic
auth using the NEO4J_AUTH env if present) and set sensible timing
(--interval=30s --timeout=5s --start-period=60s --retries=3); update the
Dockerfile near the ENV TALE_VERSION declaration to include a HEALTHCHECK
instruction that returns non-zero on failure so orchestrators get container
health status (this is optional if health is already handled in compose.yml).
In @services/graph-db/pyproject.toml:
- Around line 5-12: The pyproject.toml currently lists an unused dependency
"kuzu==0.11.3"; remove the "kuzu==0.11.3" entry from the dependencies array in
pyproject.toml so only required packages (e.g., fastapi, uvicorn, pydantic,
pydantic-settings, loguru) remain, then run dependency tooling (poetry/pip) or
update lock files accordingly to ensure the unused package is fully removed from
the project environment.
In
@services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/document-row-actions.tsx:
- Around line 128-155: The dialog mounting is inconsistent:
DocumentTeamTagsDialog is always mounted but DocumentDeleteDialog and
DocumentDeleteFolderDialog are conditionally rendered; pick a consistent
strategy—preferably always mount to preserve Radix UI animation state. Remove
the conditional JSX wrappers for DocumentDeleteDialog and
DocumentDeleteFolderDialog and render them unconditionally like
DocumentTeamTagsDialog, keeping their open prop set to dialogs.isOpen.delete and
dialogs.isOpen.deleteFolder, preserving onOpenChange handlers
(dialogs.setOpen.delete / dialogs.setOpen.deleteFolder), onConfirmDelete
callbacks (handleDeleteConfirm / handleDeleteFolderConfirm), loading props
(isDeleting) and name props (fileName / folderName), and keep the existing
comment about mounting for animations.
In
@services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/document-team-tags-dialog.tsx:
- Around line 16-22: The file document-team-tags-dialog.tsx redefines the Team
interface instead of reusing the exported type from the existing hook; remove
the local Team interface declaration and import the Team type from the module
that exports it (use-list-teams.ts) at the top of document-team-tags-dialog.tsx,
then update any references to Team in this file to use the imported type to
ensure consistency.
In
@services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/document-upload-dialog.tsx:
- Around line 125-138: The upload area div lacks keyboard accessibility; update
the element that uses className and onClick (the div wrapping Upload and texts)
to behave like a button by adding role="button", tabIndex={isUploading ? -1 :
0}, and aria-disabled={isUploading}, and implement an onKeyDown handler that
calls handleFileSelect when Enter or Space is pressed (no-op when isUploading).
Keep existing isUploading gating logic and reuse handleFileSelect so mouse and
keyboard activation are consistent.
- Around line 75-87: handleFileChange currently silently drops files larger than
MAX_FILE_SIZE_BYTES; update it to identify rejectedFiles (files.filter(f =>
f.size > MAX_FILE_SIZE_BYTES)) and notify the user when any are rejected (e.g.,
call a toast/error handler or set an error state) with a message listing
filenames or count, then add only validFiles to setSelectedFiles as before and
still reset event.target.value; reference handleFileChange, MAX_FILE_SIZE_BYTES,
setSelectedFiles and the input reset to locate where to add the rejection logic
and user notification.
In
@services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/documents-action-menu.tsx:
- Around line 73-77: DocumentUploadDialog is being rendered unconditionally
which forces its dynamic chunk to load immediately; change its render to match
OneDriveImportDialog by conditionally rendering it only when the dialog is open
(e.g., replace the unconditional <DocumentUploadDialog open={isUploadDialogOpen}
onOpenChange={setIsUploadDialogOpen} organizationId={organizationId} /> with
isUploadDialogOpen && <DocumentUploadDialog ... /> or a similar conditional
render), keeping the same props (isUploadDialogOpen and setIsUploadDialogOpen)
so the dialog only loads when opened.
In
@services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/import-documents-menu.tsx:
- Around line 106-112: documents-action-menu.tsx currently mounts
DocumentUploadDialog unconditionally (relying only on its open prop) which
differs from import-documents-menu.tsx; update documents-action-menu.tsx to
lazy-mount the dialog by wrapping the DocumentUploadDialog JSX with a
conditional render using isUploadDialogOpen (e.g., {isUploadDialogOpen &&
(<DocumentUploadDialog ... />)}), keep the props (open={isUploadDialogOpen},
onOpenChange={setIsUploadDialogOpen}, organizationId) the same, and ensure
onOpenChange still updates setIsUploadDialogOpen so the dialog lifecycle matches
the pattern used in import-documents-menu.tsx.
In
@services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/rag-status-badge.tsx:
- Around line 197-207: The stale branch currently renders the label with
t('rag.status.stale') instead of using the shared helper; update the stale case
to call getStatusLabel(effectiveStatus) (or add a clear comment if using a
different label is intentional) so it matches the other branches that use
getStatusLabel; locate the conditional that checks effectiveStatus === 'stale'
and replace the direct t(...) usage with getStatusLabel(effectiveStatus) while
keeping the surrounding Badge and RetryButton unchanged.
- Around line 105-123: RetryButton is currently declared as a nested function
component inside RagStatusBadge which recreates the component type each render;
replace it with a stable JSX variable or an extracted component: create a const
retryButton = ( ... ) JSX value (or move RetryButton to module scope) that uses
the existing handleRetry, isRetrying and documentId variables/props, then
replace all <RetryButton /> usages with {retryButton}; ensure the disabled,
onClick, title and aria-label wiring remains identical and that any icons
(Loader2/RotateCw) still render based on isRetrying.
In
@services/platform/app/(app)/dashboard/[id]/automations/components/automation-navigation.tsx:
- Around line 206-211: The status label rendering is inconsistent: two branches
use tCommon('status.X') while the archived branch uses t('navigation.archived');
update the archived case to use the common namespace to match the others
(replace the t('navigation.archived') usage with tCommon('status.archived') or
add status.archived to the common translations if missing) so all status labels
use tCommon and remain consistent; target the conditional block that checks
version.status and the t/tCommon calls to make this change.
In
@services/platform/app/(app)/dashboard/[id]/automations/components/automation-steps.tsx:
- Around line 145-174: The minimap width doesn't update when crossing the 768px
breakpoint because ResizeObserver only fires on element size changes; inside the
same useEffect that defines containerRef, MINIMAP_BASE_WIDTH, MINIMAP_MAX_WIDTH
and the updateMinimapSize callback, add a window resize listener (or a
matchMedia listener) that calls updateMinimapSize so the isMobile branch
recalculates on viewport changes, and ensure you remove the listener in the
useEffect cleanup alongside resizeObserver.disconnect(); keep using
setMinimapDimensions as before.
In
@services/platform/app/(app)/dashboard/[id]/settings/teams/components/team-create-dialog.tsx:
- Around line 34-49: The schema's memo depends on the tSettings function which
may change reference each render; extract the translated message into a stable
variable (e.g., const nameRequiredError = tSettings('teams.teamNameRequired'))
and memoize schema with that string as the dependency instead of tSettings, then
pass the unchanged schema to useForm (resolver: zodResolver(schema)); update
references to schema and the translation variable accordingly.
In
@services/platform/app/(app)/dashboard/[id]/settings/teams/components/team-delete-dialog.tsx:
- Around line 24-47: The handleConfirm function in team-delete-dialog.tsx should
track a loading flag to prevent double submissions: add a local boolean state
(e.g., isDeleting) and early-return if already true, set it to true before
calling authClient.organization.removeTeam and clear it in a finally block, and
pass that flag (or its inverse) into the DeleteDialog/button via the component's
loading/disabled prop; ensure you still call onOpenChange(false) and onSuccess()
on success and clear isDeleting in all code paths.
In
@services/platform/app/(app)/dashboard/[id]/settings/teams/components/team-members-dialog.tsx:
- Around line 35-45: The orgMembers query always runs even when the dialog is
closed; change the useQuery call for orgMembers (api.member.listByOrganization)
to pass 'skip' when open is false (similarly to teamMembers) so it only fetches
when open is true and organizationId is available; reference the orgMembers
variable, the useQuery hook, the api.member.listByOrganization key, and the
open/organizationId values when implementing the conditional skip.
In @services/platform/app/(app)/dashboard/[id]/settings/teams/page.tsx:
- Around line 32-57: TeamsSettingsSkeleton is async only to await
getT('settings'); make it synchronous by removing async/await and the getT call
and replace dynamic t(...) usage with static strings for the column headers (or
accept a t parameter from the parent to keep i18n). Update the
TeamsSettingsSkeleton function to be a plain sync component (drop async/await
and getT) and change the DataTableSkeleton columns from { header:
t('teams.columns.name') } / { header: t('teams.columns.created') } to either
literal strings or to use the passed-in t prop if you choose the param approach.
In @services/platform/components/ui/document-icon.tsx:
- Around line 12-14: getExtension currently splits on '.' and treats filenames
without extensions (e.g., "README") or leading-dot files (e.g., ".gitignore") as
having an extension; update getExtension to use lastIndexOf('.') and only return
the substring after that dot when a dot exists and it's not the first character
and not the last character of the name, otherwise return an empty string; keep
the returned value lowercased to preserve existing behavior.
In @services/platform/convex/agent_tools/rag/rag_search_tool.ts:
- Around line 125-128: Wrap the call to
ctx.runQuery(internal.lib.get_user_teams.getSearchableDatasetsInternal, { userId
}) in its own try-catch (the block that sets datasets) so failures produce a
clear, specific error; catch the error, and rethrow or return a new Error with a
message like "failed to resolve searchable datasets" that includes the original
error.message (and optionally the userId) so it’s distinguishable from later
"RAG service HTTP error" handling in the outer catch.
In @services/platform/convex/betterAuth/generated_schema.ts:
- Around line 75-81: The teamMember table currently defines separate indexes on
teamId and userId but lacks a composite index for membership lookups; add a
composite index on (teamId, userId) by updating the table definition (the
defineTable call for teamMember) to include an index like
.index("teamId_userId", ["teamId", "userId"]); because generated_schema.ts is
auto-generated, instead add this override in your schema.ts (where you customize
schema generation) so the generated output includes the composite index for
efficient membership queries.
In @services/platform/convex/documents.ts:
- Around line 932-934: getAuthenticatedUser may return null but you rely on
createdBy, so add an explicit defensive auth check after calling
getAuthenticatedUser (used in the creation blocks where you currently set
createdBy) — if authUser is null throw a clear authentication error (or return a
failure) instead of proceeding, so createdBy cannot be undefined; update both
occurrences that set createdBy (the block using getAuthenticatedUser near the
earlier creation and the similar block around the other occurrence) to perform
this null-check and error throw before using authUser.userId.
In @services/platform/convex/lib/get_user_teams.ts:
- Around line 110-117: The internalQuery declaration for
getSearchableDatasetsInternal is missing an explicit returns validator; add a
returns validator (e.g., returns: v.array(v.string())) to the internalQuery
options so Convex can enforce and document the function's return type, keeping
the existing args and handler that call getSearchableDatasetNames(ctx,
args.userId).
- Around line 47-59: getUserTeamIds currently only fetches up to 1000
memberships and returns memberships.page, which misses additional pages; change
it to loop using ctx.runQuery(components.betterAuth.adapter.findMany) with
paginationOpts.cursor and numItems, accumulating team IDs from each
memberships.page, and continue while memberships.isDone is false (use
memberships.continueCursor to set the next cursor) so the function returns all
team IDs instead of only the first page.
- Around line 14-27: You’ve duplicated the BetterAuthTeamMember and
BetterAuthFindManyResult<T> interfaces in get_user_teams.ts; remove these local
definitions and import the existing BetterAuthTeamMember and
BetterAuthFindManyResult types from the module that defines members types
instead, then update any usages in get_user_teams.ts (e.g., function signatures
and variables) to reference the imported types so the file uses the single
source-of-truth for member-related types.
In @services/platform/convex/member.ts:
- Around line 639-653: The local interfaces BetterAuthTeam and
BetterAuthTeamMember should be centralized: add/export these interfaces from the
existing central members types module (the module that already exports
BetterAuthMember and BetterAuthUser), remove the duplicate local definitions in
member.ts, and update member.ts to import BetterAuthTeam and
BetterAuthTeamMember from that central types module; ensure all usages in this
file and any other files reference the centralized types and run typechecks to
confirm no remaining local duplicates.
In @services/platform/convex/model/documents/get_user_names_batch.ts:
- Around line 1-6: Update the top-of-file docstring in get_user_names_batch.ts
to remove the claim of a "single query" and instead state that the helper
fetches multiple user names concurrently (parallel requests) because the Better
Auth adapter requires one request per user; keep the note about efficiency and
that this approach avoids N+1 query overhead in the calling code while
acknowledging the adapter limitation.
In @services/platform/convex/team_members.ts:
- Around line 10-32: Consolidate duplicate interfaces by removing
BetterAuthTeamMember and BetterAuthFindManyResult<T> from this file and import
them from the existing types module where they are defined; if BetterAuthTeam is
not yet present in that types module, add the BetterAuthTeam interface there and
then import it here as well, and update any references in this file to use the
imported symbols (BetterAuthTeamMember, BetterAuthFindManyResult,
BetterAuthTeam) instead of the local definitions.
- Around line 37-52: The query listByTeam omits a returns validator; add a
returns property to the query config to declare the response type for type
safety (e.g., returns: v.array(...) matching BetterAuthTeamMember). Update the
query({ args: { teamId: v.string() }, handler: ... }) call to include returns
that either references an existing validator for BetterAuthTeamMember or a new
v.object(...) shape matching that type so the Convex function signature is
explicit and type-checked.
- Around line 137-164: listTeamsByUser triggers an N+1 query pattern by calling
components.betterAuth.adapter.findMany once per membership to fetch each team;
fix by replacing the per-membership adapter calls with a single batched
lookup—either migrate to the official @convex-dev/better-auth integration if it
supports batching, or stop using the adapter here and use Convex native APIs
(e.g., use ctx.db.get or ctx.db.multiGet / .withIndex to fetch all team records
for the array of teamIds in one operation) so that listTeamsByUser performs one
query for all teamIds instead of one per membership; if batching is impossible,
document the adapter limitation and add a TODO in listTeamsByUser referencing
components.betterAuth.adapter.findMany and the membership handling to revisit
when adapter supports 'in' semantics.
In @services/platform/package.json:
- Around line 106-108: The package.json currently lists "typescript" in
dependencies; move "typescript": "5.9.3" from the dependencies object into
devDependencies to avoid shipping the TypeScript compiler to production. Update
the package.json so dependencies no longer contain "typescript" and
devDependencies includes "typescript": "5.9.3", then run your package manager
(npm install or npm ci) to update lockfiles; verify builds still work
(tsc/compile-related scripts remain unchanged).
- Line 149: The package.json dependency "vitest-axe": "1.0.0-pre.5" likely
doesn't resolve from npm; update package.json to use a valid, maintained
package/version (for example replace "vitest-axe": "1.0.0-pre.5" with
"@chialab/vitest-axe": "0.19.0" or another verified version) or confirm and
document why the pre-release is required and add a resolvable source; run
npm/yarn install to verify resolution and update any import paths or test
configs referencing vitest-axe accordingly.
In @services/rag/app/services/cognee/cleanup.py:
- Around line 105-109: The index creation uses dynamic identifiers from system
catalogs which are mostly trusted but could contain unusual characters; add a
simple PostgreSQL identifier validator (e.g., _is_valid_identifier using a regex
like r'^[a-zA-Z_][a-zA-Z0-9_]*$') and call it before executing the CREATE INDEX
in the function where table_name and column_name are used (referencing the same
block that calls conn.execute(... CREATE INDEX ...)); if validation fails, log a
warning (use the module's logger) and skip/continue that table instead of
executing the SQL to provide defense-in-depth.
In @services/rag/app/services/cognee/config.py:
- Around line 300-302: Replace the f-string log with loguru's lazy placeholder
style: change the logger.info call in config.py that currently references
settings.graph_db_provider and settings.graph_db_url using an f-string to a
logger.info call using "{}" placeholders and passing settings.graph_db_provider
and settings.graph_db_url as arguments (locate the logger.info line that starts
"Configured Cognee to use" to update).
In @services/rag/app/services/cognee/service.py:
- Around line 349-358: The check for "user" in add_kwargs is dead code because
add_kwargs never gets a "user" key; remove the if block that tries to copy
add_kwargs["user"] into cognify_kwargs and simply build cognify_kwargs with
"datasets" and "incremental_loading" before calling
cognee.cognify(**cognify_kwargs) (i.e., delete the lines referencing
add_kwargs["user"] and the surrounding if condition so cognify_kwargs is only
populated from known values).
In @services/rag/pyproject.toml:
- Around line 1-18: The pyproject.toml is missing a required [build-system]
section; add a top-level [build-system] table after the [project] table with
entries like requires = ["setuptools>=61.0","wheel"] and build-backend =
"setuptools.build_meta" (or another PEP 517 backend your repo uses) so installs
via pip (PEP 517/518) succeed when building the package.
larryro
force-pushed
the
feat/152-multi-tenancy-teams
branch
3 times, most recently
from
e83f8cf to
e97472e
Compare
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
larryro
force-pushed
the
feat/152-multi-tenancy-teams
branch
from
0d62ef2 to
75cdbe5
Compare
larryro
force-pushed
the
feat/152-multi-tenancy-teams
branch
from
f175fe3 to
c9b09d3
Compare
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 42
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
services/platform/convex/model/trusted_headers_authenticate/find_or_create_user_from_headers.ts (2)
22-40: Unusedroleparameter is dead code.The
rolefield is defined inFindOrCreateUserFromHeadersArgs(line 25), normalized on line 40, but never used anywhere in the function. This creates confusion since callers must provide a role that serves no purpose.Either:
- Remove
rolefrom the interface and line 40 if it's truly not needed, or- If kept for API stability, add an underscore prefix and document why it's ignored.
Option 1: Remove unused parameter
export interface FindOrCreateUserFromHeadersArgs { email: string; name: string; - role: string; }// Normalize inputs const email = args.email.toLowerCase().trim(); const name = args.name.trim(); - const role = args.role.toLowerCase().trim();Option 2: Document intentionally ignored parameter
export interface FindOrCreateUserFromHeadersArgs { email: string; name: string; - role: string; + /** `@deprecated` Ignored - role comes from session/JWT in trusted headers mode */ + role: string; }// Normalize inputs const email = args.email.toLowerCase().trim(); const name = args.name.trim(); - const role = args.role.toLowerCase().trim(); + // Note: args.role is intentionally ignored; role comes from session/JWT
129-129: Fragile fallback for ID extraction.The
String(createResult)fallback could produce[object Object]if the result is an object without_idorid. Consider throwing an error instead of silently using a potentially invalid ID.Proposed stricter ID extraction
- userId = createResult._id ?? createResult.id ?? String(createResult); + userId = createResult._id ?? createResult.id; + if (!userId) { + throw new Error('Failed to extract user ID from create result'); + }services/platform/convex/model/chat_agent/generate_agent_response.ts (1)
277-286: Consider a typed context extension for tool consumers.
Extracting a shared interface (e.g.,AgentContextWithOrg) would makeuserTeamIdsandragPrefetchCacheexplicit and easier to reuse.services/rag/app/services/cognee/service.py (1)
501-592: Globaltop_kisn’t enforced across datasets.
Each dataset returnstop_k, then results are concatenated without re-ranking, so the final set can exceed the requested limit and be unordered. Consider sorting and slicing after filtering.🐛 Enforce global top_k
filtered_results = [ r for r in normalized_results if r.get("score", 0) >= threshold ] + if datasets: + filtered_results.sort(key=lambda r: r.get("score", 0), reverse=True) + filtered_results = filtered_results[:effective_top_k]services/rag/app/routers/documents.py (1)
41-60: Client-controlled tenant context creates a cross-tenant data access vulnerability.The endpoints accept
user_idanddataset_namedirectly from client requests with no authentication middleware and no validation against authenticated session context. An attacker can supply arbitrary values to access or write to any user's or team's dataset.Derive these values from trusted authentication context (JWT claims, session tokens, or authenticated request state) before passing them to cognee. Do not accept tenant identifiers from client-supplied form parameters or request bodies.
🤖 Fix all issues with AI agents
In `@scripts/deploy.sh`:
- Line 70: Update the top-level comment string "# - graph-db: Neo4j - single
instance required for data consistency" to reference FalkorDB instead of Neo4j
(e.g., "# - graph-db: FalkorDB - single instance required for data consistency")
so the deploy.sh comment matches the compose.yml/service label and the project's
FalkorDB configuration references.
In `@scripts/dev-tools.sh`:
- Around line 35-36: Remove the unused PROJECT_ROOT variable declaration: delete
the line that assigns PROJECT_ROOT (the assignment that uses SCRIPT_DIR to
compute the parent directory) from the script, or if you actually need it
outside the script, change it to an export; also search the script for any uses
of PROJECT_ROOT to ensure removing it won't break anything and re-run shellcheck
to confirm the warning is gone.
- Around line 108-126: The get_dataset_name and get_user_email functions
currently pipe psql output through tr -d '[:space:]', which removes internal
spaces; change the final transformation so it only trims leading/trailing
whitespace (preserving internal spaces) — replace the tr invocation in both
get_dataset_name and get_user_email with a trim that uses POSIX character class
trimming (for example via sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//') so
dataset names like "My Dataset" keep their internal space while still stripping
surrounding whitespace.
- Around line 140-208: The fallback path in select_kuzu_database currently
returns the unsorted first element (${databases[0]}) while claiming to use the
"most recent" DB; update the invalid-selection handling to actually pick the
newest file by mtime: compute the newest entry from the databases array (e.g.,
loop over "${databases[@]}" using stat -c %Y to compare timestamps or call
docker/stat as used earlier) and echo that path instead of ${databases[0]};
alternatively, if you prefer not to implement timestamp comparison here, change
the error message to not claim "most recent." Ensure you update the
selection-validation block that uses log_error and the subsequent echo to
reference the computed newest_db variable or the revised message.
In
`@services/platform/app/`(app)/dashboard/[id]/(knowledge)/documents/components/document-row-actions.tsx:
- Around line 96-120: The handleReindex handler can fire twice before React
state updates, so add a synchronous in-flight guard using a ref (e.g.,
isReindexingRef) alongside the existing isReindexing/setIsReindexing state: at
the start of handleReindex check the ref and return early if true, otherwise set
the ref to true and then proceed to call retryRagIndexing(documentId); ensure
you clear the ref in the finally block (and still call setIsReindexing(false))
so subsequent clicks are allowed, and reference handleReindex,
isReindexing/setIsReindexing, and retryRagIndexing when making the change.
In
`@services/platform/app/`(app)/dashboard/[id]/(knowledge)/documents/components/document-team-tags-dialog.tsx:
- Around line 138-149: The label currently wraps the Checkbox which triggers a
"label-without-control" a11y lint; update the teams.map rendering in
document-team-tags-dialog.tsx to assign a unique id to each Checkbox (e.g.,
`team-checkbox-${team.id}`) and add a matching htmlFor on the surrounding label
element so the label is explicitly associated with the input; keep the existing
Checkbox props (checked={selectedTeams.has(team.id)} and onCheckedChange={() =>
handleToggleTeam(team.id)}) and the label content (team.name) unchanged.
In
`@services/platform/app/`(app)/dashboard/[id]/(knowledge)/documents/components/document-upload-dialog.tsx:
- Around line 211-221: The label wrapper around the Radix Checkbox lacks an
association with the Checkbox input, causing the accessibility/Biome warning;
fix by either (A) using Checkbox's built-in label prop and supply an id (e.g.,
pass label={team.name} and id={`team-${team.id}`} to Checkbox) and remove the
external <label> wrapper, or (B) generate a stable id (e.g., `team-${team.id}`),
set that id on the Checkbox and add htmlFor on the outer label so the label is
associated with the Checkbox; keep existing props like
checked={selectedTeams.has(team.id)}, onCheckedChange={() =>
handleToggleTeam(team.id)} and disabled={isUploading}.
In
`@services/platform/app/`(app)/dashboard/[id]/(knowledge)/documents/components/import-documents-menu.tsx:
- Around line 106-112: The DocumentUploadDialog instance currently opened when
isUploadDialogOpen is true should receive an onSuccess callback like
OneDriveImportDialog does; update the DocumentUploadDialog props (where it's
rendered with open={isUploadDialogOpen}, onOpenChange={setIsUploadDialogOpen},
organizationId={organizationId}) to pass an onSuccess={() =>
setIsUploadDialogOpen(false)} (or a handler that also refreshes data/updates UI)
so the dialog closes and any post-upload actions run after a successful upload.
In
`@services/platform/app/`(app)/dashboard/[id]/settings/teams/components/team-edit-dialog.tsx:
- Around line 35-65: Update the zod schema so the name field is trimmed before
validation to reject whitespace-only values: change the schema in the schema
const (the z.object that currently uses name: z.string().min(1,...)) to use
z.preprocess((v) => typeof v === 'string' ? v.trim() : v, z.string().min(1,
tSettings('teams.teamNameRequired'))). This keeps TeamFormData typing and
useForm/resolver intact; you can then remove or keep the extra .trim() in
onSubmit (authClient.organization.updateTeam call) as desired, but the key fix
is adding the z.preprocess trimming to the schema.
In
`@services/platform/app/`(app)/dashboard/[id]/settings/teams/components/team-members-dialog.tsx:
- Around line 61-63: The helper getMemberDetails captures orgMembers in a
closure and can become stale when called during render; replace it with an
explicit dependency-aware version (either move the find inline where it's used
or memoize the function/result using React.useMemo or React.useCallback with
orgMembers in the dependency array) so the lookup always reflects the latest
orgMembers; update references to the existing getMemberDetails function
accordingly (e.g., the component that calls getMemberDetails) to use the new
inline/find or the memoized function.
In
`@services/platform/app/`(app)/dashboard/[id]/settings/teams/components/team-table.tsx:
- Around line 57-59: The component TeamTable currently returns null when
isLoading which leaves a blank area; update TeamTable to render the DataTable
skeleton/loading UI instead of null by returning the DataTable (or a dedicated
skeleton component) with its loading prop/state set when isLoading is true so
the table shows a built-in loading skeleton; locate the isLoading check in the
TeamTable component and replace the `return null` branch with rendering the
DataTable in loading mode (or a DataTableSkeleton component) so the component is
self-contained and does not rely on the parent TeamsSettings for loading UI.
In `@services/platform/convex/agent_tools/rag/query_rag_context.ts`:
- Around line 188-203: Replace the untyped requestPayload (currently declared as
Record<string, unknown>) with a concrete TypeScript interface or type describing
the payload shape (e.g., properties query: string, top_k: number,
similarity_threshold: number, include_metadata: boolean, and optional user_id
and datasets) and use that type for the requestPayload variable in the try block
where expandedQuery, topK, similarityThreshold and options are used; update the
conditional assignments (options.userId and options.datasets) to assign into the
typed fields (user_id and datasets) so the compiler enforces correct shapes and
prevents accidental fields on requestPayload.
In `@services/platform/convex/agent_tools/rag/rag_search_tool.ts`:
- Around line 152-162: The datasets array can contain duplicates if userTeamIds
has repeated IDs; update the construction of datasets in rag_search_tool.ts (the
code using userTeamIds, teamIdToDatasetName, DEFAULT_DATASET_NAME, payload, and
debugLog) to deduplicate entries before assigning to payload.datasets and
logging — e.g., map team IDs via teamIdToDatasetName as done now, then remove
duplicates (using a Set or equivalent) while preserving DEFAULT_DATASET_NAME and
original order, and then assign the deduplicated array to payload.datasets and
use that for debugLog.
In `@services/platform/convex/lib/create_chat_agent.ts`:
- Around line 61-77: Replace the non‑English example text inside the "LANGUAGE
MATCHING" comment block with an English‑only example; locate the LANGUAGE
MATCHING comment in create_chat_agent.ts and update its bullet points and
example lines so all user‑facing strings (e.g., the bullets "If the user writes
in Chinese..." and example phrases) are in English, preserving the original
guidance and formatting but removing any non‑English characters.
In `@services/platform/convex/lib/get_user_teams.ts`:
- Around line 58-70: Replace the unsafe cast and silent catch: after calling
ctx.auth.getUserIdentity() check that identity has a trustedTeams property of
type string (avoid (identity as any)?.trustedTeams); then JSON.parse inside a
try/catch that does not swallow errors—on failure log the parse error (via
available logger like ctx.logger or processLogger) or rethrow a descriptive
error mentioning trustedTeams parsing; finally return teams.map(t => t.id) after
validating parsed value is an array of objects with id strings (validate shape
of teams before mapping).
In `@services/platform/convex/lib/rag_prefetch/index.ts`:
- Around line 251-255: The datasets array may contain duplicates when
options.userTeamIds has repeated IDs; before using it later, deduplicate by
building datasets from DEFAULT_DATASET_NAME plus the mapped team names and
removing duplicates (e.g., via a Set) so that the variable datasets contains
unique values—update the code that constructs datasets (referencing
DEFAULT_DATASET_NAME, options.userTeamIds, teamIdToDatasetName, and the datasets
variable) to produce a de-duplicated list.
In `@services/platform/convex/lib/rls/auth/get_trusted_auth_data.ts`:
- Around line 33-39: Replace the blanket cast "(identity as any).trustedRole"
with a targeted narrowing cast or type guard so the type boundary is explicit:
check that "identity" has a "trustedRole" property using a type predicate or
cast to a narrow shape (e.g., identity as { trustedRole?: string }) before
accessing trustedRole in the get_trusted_auth_data logic; update the trustedRole
extraction (and any subsequent usage) to use that narrowed type instead of
"any".
- Around line 41-51: After parsing trustedTeamIdsRaw into trustedTeamIds, add
runtime validation to ensure the result is actually a string array: inside the
try block where JSON.parse(trustedTeamIdsRaw) is assigned, check
Array.isArray(parsed) && parsed.every(item => typeof item === "string"); only
then assign to trustedTeamIds, otherwise leave it as [] (or set to []). Keep the
existing try/catch but perform this validation on the parsed value (referencing
trustedTeamIdsRaw, trustedTeamIds and the parsed intermediate) so non-string or
non-array JSON won't be used.
In `@services/platform/convex/lib/rls/organization/get_user_organizations.ts`:
- Around line 52-58: The mapping in get_user_organizations.ts returns role:
trustedData?.trustedRole || (member.role || 'member').toLowerCase(), but
trustedData.trustedRole is a plain string and may not match the expected
literals; validate and normalize it against the allowed set
['disabled','member','editor','developer','admin'] before returning. Update the
mapping in the function that builds the return objects (the map callback) to:
read trustedData?.trustedRole, coerce to lowercase, check inclusion in the
allowed roles set, and use it only if valid; otherwise fall back to (member.role
|| 'member').toLowerCase() (and also validate member.role similarly) so the
returned role value is guaranteed to be one of the expected literals.
In `@services/platform/convex/model/documents/transform_to_document_item.ts`:
- Around line 135-171: The deduplication loop in batchGetStorageUrls can be
simplified: replace the seenIds/set + uniqueIds loop inside batchGetStorageUrls
with a single Map-based pass that preserves original fileId types (e.g., create
const seen = new Map<string, typeof fileIds[number]>(); for each id compute key
= String(id) and if !seen.has(key) seen.set(key, id); then const uniqueIds =
Array.from(seen.values());), then continue using uniqueIds and
ctx.storage.getUrl as before; this keeps behavior identical but shortens the
code and preserves types.
In `@services/platform/convex/model/documents/update_document.ts`:
- Around line 32-41: The current teamTags validation is skipped when args.userId
is missing, allowing assignment to arbitrary teams; update the check in
update_document to require a userId when teamTags is provided by throwing an
error if args.teamTags !== undefined && !args.userId, or alternatively make
args.userId required for that code path; then proceed to call
getUserTeamIds(ctx, args.userId) and validate each tag against the userTeamSet
as before (referencing args.teamTags, args.userId, getUserTeamIds, and
userTeamSet).
In
`@services/platform/convex/model/trusted_headers_authenticate/create_session_for_trusted_user.ts`:
- Around line 178-180: Replace the use of the logical OR fallback with nullish
coalescing for the trusted fields: change occurrences like trustedRole:
args.trustedRole || null and trustedTeams: args.trustedTeams || null to use ??
(trustedRole: args.trustedRole ?? null, trustedTeams: args.trustedTeams ?? null)
so empty strings are preserved; apply the same change to the other occurrence
around line 187 (the same trustedRole/trustedTeams assignment) in the create
session logic (e.g., within the create_session_for_trusted_user function/object
construction).
In
`@services/platform/convex/model/trusted_headers_authenticate/resolve_team_names.ts`:
- Around line 18-31: resolveTeams currently assumes args.teams is defined and
calls teams.length which will throw if callers pass undefined/null; update
resolveTeams to guard and default to an empty array (e.g. read teams as
args?.teams ?? [] or treat args.teams || []) before checking length and
returning; adjust the ResolveTeamsArgs type to accept teams?: TeamEntry[] if
needed and ensure all references to resolveTeams use the normalized local
variable instead of args.teams.
In `@services/platform/convex/team_members.ts`:
- Around line 97-132: The removeMember mutation lacks an authorization check;
before proceeding (inside handler of removeMember and before querying/deleting
teamMember via components.betterAuth), call the same permission check used by
addMember (e.g., the helper or logic that verifies the caller is a team
admin/owner or has remove-member rights via ctx.auth or a function like
assertTeamPermission/ensureTeamAdmin) and throw an authorization error if it
fails; place this check immediately after extracting args and before the
existing findMany/deleteMany operations so only authorized callers can remove
members.
- Around line 57-92: The addMember mutation lacks server-side authorization
allowing any caller to add members; replace or refactor addMember to enforce
role-based checks like the patterns in member.ts by using mutationWithRLS (or
wrapping the handler with the same RLS logic) and verify the caller’s role on
the target team/org before calling components.betterAuth.adapter.create;
specifically, locate addMember and before creating the teamMember use the same
permission verification used by member.ts (e.g., runQuery to check the caller’s
membership/role on the given teamId or orgId and ensure they are owner/admin)
and throw an authorization error if the check fails.
In `@services/platform/lib/auth/trusted-headers-token.ts`:
- Around line 105-109: The current console.log dumping the full trusted headers
user object (variable headerUser returned by extractTrustedHeaders(request)) can
leak PII; remove or gate this logging behind a dev/debug-only flag and redact
sensitive fields (email, name, teams, etc.) before logging. Modify the place
where headerUser is logged to either (a) only log non-PII metadata (e.g., user
id and a redacted marker) or (b) check a debug/config flag (ENV or logger.level)
and only then log a sanitized version; ensure extractTrustedHeaders and any
subsequent usage remain unchanged except for the logging call.
In `@services/platform/messages/en.json`:
- Around line 1443-1449: The message for the key "deleteFileFailed" is an
incomplete fragment; update its value to a full, clear sentence (for example:
"An unexpected error occurred while deleting the file.") by editing the messages
JSON entry for "deleteFileFailed" so the UI displays a complete error message.
In `@services/platform/proxy.ts`:
- Around line 40-42: The code redundantly calls
response.cookies.delete(cookieName) when authResult.shouldClearOldSession is
true immediately before setting the cookie again; remove the unnecessary
deletion and rely on response.cookies.set(cookieName, ...) to overwrite the
existing cookie (locate the block using authResult.shouldClearOldSession,
response.cookies.delete, and response.cookies.set to update). If the deletion
was meant to change attributes (e.g., path, httpOnly), instead consolidate into
a single response.cookies.set call with the desired attributes to avoid
redundant operations.
- Around line 28-32: The cookie name selection (currently using isHttps ?
'__Secure-better-auth.session_token' : 'better-auth.session_token') is
duplicated; extract it into a single helper (e.g. getSessionCookieName(siteUrl)
or getJwtCookieName(siteUrl)) that takes siteUrl or a boolean isHttps and
returns the correct cookie string, then replace the inline ternary uses (the
local siteUrl/isHttps/cookieName logic and the other occurrence later in the
file) with calls to that helper to centralize the logic and avoid duplication.
In `@services/rag/app/main.py`:
- Around line 76-85: The call to ensure_vector_hnsw_indexes() can return
per-index failures in index_result["errors"] which are currently ignored; after
awaiting ensure_vector_hnsw_indexes() (in the same try block where index_result
is set) check for index_result.get("errors") and log each entry (or the list)
with logger.warning or logger.error to surface partial failures, while
preserving the existing logger.info for created indexes and keeping the outer
exception handler for catastrophic errors.
In `@services/rag/app/models.py`:
- Around line 223-233: The datasets field is currently accepted from callers
without authorization checks; update the query handler logic (e.g., functions
like search_documents or handle_query that consume datasets and user_id) to
enforce authorization: when ENABLE_BACKEND_ACCESS_CONTROL is true, require
user_id to be present and validate each entry in datasets against the set of
datasets the user has access to (reject the request with an unauthorized/400
response if any dataset is not allowed), and when user_id is omitted while
access control is enabled, reject dataset filters outright; use the existing
user identity/access lookup service to fetch allowed dataset names and compare
them to the provided datasets.
- Around line 58-68: The model currently allows dataset_name to be set without
user_id; add a model-level validation in the Pydantic model that owns user_id
and dataset_name (use a model_validator / root validator for Pydantic v2) to: 1)
reject any instance where dataset_name is non-empty and user_id is None by
raising a ValidationError, and 2) call a synchronous/async helper (e.g.,
validate_dataset_belongs_to_user(dataset_name, user_id) or similar service
function) to verify the user actually owns that dataset and raise if the check
fails; update the model that declares user_id and dataset_name to include this
validator so invalid/malicious requests are rejected early.
In `@services/rag/app/services/cognee/cleanup.py`:
- Around line 340-347: The current except block checks cleanup_err by string
matching which is fragile; update the exception handling around the legacy
cleanup (where cleanup_err is caught and logger is used) to explicitly catch
sqlalchemy.exc.ProgrammingError and inspect e.orig.pgcode == '42P01' to detect
an undefined_table error and log the existing warning via logger.warning,
otherwise log the failure with logger.error; fall back to a generic except
Exception as cleanup_err branch to log other errors with logger.error so you
stop relying on string comparisons of type(cleanup_err).__name__.
In `@services/rag/app/services/cognee/config.py`:
- Around line 78-125: The _pre_configure_database_env function should validate
parsed URL components before exporting DB_* envs: after calling
settings.get_database_url() and urlparse(database_url), ensure required parts
(hostname, port, path/database name, username, password as appropriate) are
present and not just defaults; if validation fails, log a warning with the
parsed details and return early (similar to _setup_database_config's checks)
instead of falling back to localhost/empty values, so malformed URLs do not get
written into os.environ and hide misconfiguration during cognee import.
- Around line 128-142: Update the Falkor adapter dependency string to pin it to
a version compatible with cognee==0.5.1 by changing the package entry
"cognee-community-hybrid-adapter-falkor" to include an exact/compatible version
(for example "cognee-community-hybrid-adapter-falkor==0.5.1" or a compatible
specifier) in the project's dependency list; locate the dependency by the
package name "cognee-community-hybrid-adapter-falkor" in the dependency
declarations and replace the unpinned entry with the pinned version.
- Around line 381-465: In _patched_is_empty (the async replacement for
FalkorDBAdapter.is_empty) replace the synchronous call to self.query(query) with
an awaited call so the coroutine is executed: use await self.query(query)
instead of self.query(query); ensure this change is made inside the async def
_patched_is_empty so result is the resolved response object before accessing
result.result_set.
- Around line 206-267: The patch in
_patch_litellm_aembedding/_patched_aembedding incorrectly assumes input is a
list and will iterate strings character-by-character; fix it by detecting str
inputs and leaving them unchanged (or wrapping them as a single-item list only
where appropriate), only run the filtering/list-comprehension when
isinstance(input, list), compute original_input_len from the list case, and keep
the existing early-return EmbeddingResponse behavior for when a list input
becomes empty; update references to litellm.aembedding and _original_aembedding
accordingly.
In `@services/rag/app/services/cognee/utils.py`:
- Around line 104-108: The object-result branch currently returns metadata as an
empty dict, losing fields like chunk_size or chunk_index; update the object-path
return to populate metadata the same way the dict-path does by extracting the
same selective keys from result via getattr (e.g., chunk_size, chunk_index,
source, any other keys used by the dict branch) and include them in the returned
"metadata" dict while preserving existing content, score, and document_id
behavior.
In `@services/rag/Dockerfile`:
- Around line 77-78: Add a short explicit section to
docs/zero-downtime-deployment.md that shows how to override default local
credentials for production by setting environment variables (e.g. DB_PASSWORD,
RAG_DATABASE_URL and any RAG_* overrides) via the deployment tooling used
(systemd/unit, Docker Compose prod override, Kubernetes Secrets/ConfigMap, or
CI/CD pipeline), reference the compose.yml use of env_file and the .env.example
variables, and include a brief example of where to place/secure these values in
a production workflow.
In `@services/rag/pyproject.toml`:
- Line 7: The dependency "cognee-community-hybrid-adapter-falkor" is unpinned
while other packages use exact versions; update the dependency entry to pin it
to the released PyPI version by changing the entry for
cognee-community-hybrid-adapter-falkor to include an exact version specifier
(e.g., ==0.2.0) so builds are reproducible.
In `@services/rag/tests/concurrent_read_test.ts`:
- Around line 34-35: Replace the hardcoded constants RAG_BASE_URL and
DEFAULT_DATASET in concurrent_read_test.ts with values read from environment
variables (e.g. process.env.RAG_BASE_URL and process.env.DEFAULT_DATASET) and
provide the existing strings as fallback defaults; update any test setup that
references RAG_BASE_URL/DEFAULT_DATASET to use the new variables so tests can be
configured via environment without changing source.
- Around line 159-174: The current promise-resolution detection using
Promise.race([...pending[i].then(() => "resolved"), Promise.resolve("pending")])
is incorrect and always returns "pending", causing pending to never shrink;
replace this pattern in the concurrency logic that manages the pending array and
inFlight counter by switching to a self-cleaning Set (or map) of in-flight
promises where each promise removes itself from the Set in its finally() handler
(the same finally() that decrements inFlight), await when the Set size reaches
the concurrency limit to block new starts, and at the end await Promise.all on
the remaining Set entries; update all references to pending, inFlight, and the
Promise.race block so resolved promises are removed immediately and inFlight
remains consistent.
♻️ Duplicate comments (25)
services/rag/app/services/cognee/cleanup.py (1)
105-109: SQL identifier construction is safe but past review suggestion remains applicable.The identifiers come from PostgreSQL system catalogs (
pg_class/pg_attribute), which are trusted. The double-quoted f-string approach is correct for PostgreSQL. The optional identifier validation suggested in the past review comment remains a valid defense-in-depth measure but is not required for correctness.services/rag/pyproject.toml (1)
1-19: Missing[build-system]section — previously flagged.The pyproject.toml still lacks a
[build-system]section required for PEP 517/518 compliant builds. This may cause issues with package installation in the Dockerfile.services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/documents-action-menu.tsx (1)
73-77: Conditional rendering inconsistency defeats lazy-loading benefit.
DocumentUploadDialogis rendered unconditionally whileOneDriveImportDialog(line 79) uses conditional rendering. This causes the dynamically imported chunk to load immediately rather than when the dialog opens.♻️ Align with OneDriveImportDialog pattern
- <DocumentUploadDialog - open={isUploadDialogOpen} - onOpenChange={setIsUploadDialogOpen} - organizationId={organizationId} - /> + {isUploadDialogOpen && ( + <DocumentUploadDialog + open={isUploadDialogOpen} + onOpenChange={setIsUploadDialogOpen} + organizationId={organizationId} + /> + )}services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/document-upload-dialog.tsx (2)
76-88: Silent file rejection provides no user feedback.Files exceeding
MAX_FILE_SIZE_BYTESare silently filtered. Users won't understand why some selected files didn't appear.💡 Add feedback for rejected files
const handleFileChange = useCallback((event: ChangeEvent<HTMLInputElement>) => { const files = Array.from(event.target.files || []); if (files.length === 0) return; // Filter out files that are too large const validFiles = files.filter((file) => file.size <= MAX_FILE_SIZE_BYTES); + const rejectedCount = files.length - validFiles.length; + if (rejectedCount > 0) { + // TODO: Show toast notification for rejected files + console.warn(`${rejectedCount} file(s) exceeded size limit`); + } setSelectedFiles((prev) => [...prev, ...validFiles]);
126-139: Upload area lacks keyboard accessibility.The click-to-upload div cannot be activated via keyboard. Add
role="button",tabIndex, and keyboard event handlers.♿ Add keyboard support
<div + role="button" + tabIndex={isUploading ? -1 : 0} className={cn( 'border-2 border-dashed rounded-lg p-6 text-center cursor-pointer transition-colors', 'hover:border-primary hover:bg-accent/50', isUploading && 'opacity-50 cursor-not-allowed', )} onClick={!isUploading ? handleFileSelect : undefined} + onKeyDown={(e) => { + if (!isUploading && (e.key === 'Enter' || e.key === ' ')) { + e.preventDefault(); + handleFileSelect(); + } + }} >services/platform/convex/team_members.ts (3)
10-32: Duplicate type definitions exist elsewhere.
BetterAuthTeamMemberandBetterAuthFindManyResult<T>are already defined inservices/platform/convex/model/members/types.ts. Import from there to maintain a single source of truth.
37-52: Missingreturnsvalidator.Per Convex conventions, include explicit
returnsvalidators for type safety and documentation.
137-164: N+1 query pattern - one query per team membership.This executes one query per membership to fetch team details. For users with many teams, this could be slow. The Better Auth adapter lacks batch/IN operator support, so consider using Convex's native
db.get()if team IDs map to document IDs.services/rag/app/models.py (1)
282-292: Apply the same dataset-authorization rules to generation requests.
Keep generation scoping consistent with the query path.services/platform/convex/lib/get_user_teams.ts (3)
17-30: Duplicate type definitions - consolidate with existing types.These interfaces duplicate
BetterAuthTeamMemberandBetterAuthFindManyResult<T>fromservices/platform/convex/model/members/types.ts. Import from there to maintain DRY principle.
73-80: Implement complete cursor-based pagination to fetch all team memberships.The function only fetches the first 1000 results without checking
isDoneor looping withcontinueCursor. If a user belongs to more than 1000 teams, incomplete data is silently returned. This is a data integrity issue for access control.
132-139: Addreturnsvalidator for type safety.Per learnings, Convex functions should include explicit
returnsvalidators.♻️ Suggested fix
export const getSearchableDatasetsInternal = internalQuery({ args: { userId: v.string(), }, + returns: v.array(v.string()), handler: async (ctx, args): Promise<string[]> => { return getSearchableDatasetNames(ctx, args.userId); }, });services/rag/app/services/cognee/service.py (2)
157-213: Avoid N+1 dataset lookups.
The current loop issues extra queries perDatarow; consider a single join/batch.♻️ Example join-based approach
- # Get dataset information for each record - entries = [] - for data in matching_data: - dataset_link = ( - await session.execute( - select(DatasetData).where(DatasetData.data_id == data.id) - ) - ).scalars().first() - - if dataset_link: - # Get the dataset name - dataset = ( - await session.execute( - select(Dataset).where(Dataset.id == dataset_link.dataset_id) - ) - ).scalars().first() - - entries.append({ - "data_id": data.id, - "dataset_id": dataset_link.dataset_id, - "dataset_name": dataset.name if dataset else "unknown", - }) - - return entries + data_ids = [d.id for d in matching_data] + result = await session.execute( + select(Data.id, DatasetData.dataset_id, Dataset.name) + .join(DatasetData, DatasetData.data_id == Data.id) + .join(Dataset, Dataset.id == DatasetData.dataset_id) + .where(Data.id.in_(data_ids)) + ) + return [ + {"data_id": row[0], "dataset_id": row[1], "dataset_name": row[2] or "unknown"} + for row in result.fetchall() + ]
385-395: Remove deaduserpropagation in cognify kwargs.
add_kwargsnever sets"user", so this block is unreachable.🧹 Cleanup
- if "user" in add_kwargs: - cognify_kwargs["user"] = add_kwargs["user"]services/rag/app/routers/documents.py (1)
148-155: Same trust-boundary concern for upload inputs.The upload endpoint now accepts
user_id/dataset_namefrom the client; ensure these are derived or validated server-side, not trusted blindly.services/platform/convex/betterAuth/generated_schema.ts (1)
93-99: Composite index onteamMemberfor membership lookups.The
teamMembertable has separate indexes onteamIdanduserId, but a composite index(teamId, userId)would optimize membership checks (e.g., "is user X a member of team Y?"). Since this file is auto-generated, add the override inbetterAuth/schema.ts.services/platform/convex/model/documents/get_user_names_batch.ts (1)
1-6: Docstring still implies a “single query.”The implementation does parallel lookups (one per user). The header should reflect that to avoid misleading readers.
📝 Suggested wording
/** * Batch fetch user names from Better Auth user table * - * This helper efficiently fetches multiple user names in a single query, - * avoiding the N+1 query problem when displaying creator names in document lists. + * This helper efficiently fetches multiple user names in parallel, + * reducing latency compared to sequential N+1 queries when displaying + * creator names in document lists. */services/platform/convex/member.ts (1)
639-653: Centralize Better Auth team types (already flagged).
These interfaces mirror shared member types elsewhere; consider exporting them from the central members types module and importing here to avoid drift.services/platform/app/(app)/dashboard/[id]/settings/teams/components/team-delete-dialog.tsx (1)
24-47: Double-submission prevention still needed.As noted in a previous review,
handleConfirmdoesn't track loading state, which allows multiple delete requests on rapid clicks. Consider adding anisDeletingstate to guard against this.services/platform/app/(app)/dashboard/[id]/settings/teams/components/team-members-dialog.tsx (2)
35-45: SkiporgMembersquery when dialog is closed for consistency.The
teamMembersquery correctly uses'skip'when the dialog is closed, butorgMembersalways fetches regardless of dialog state. This creates unnecessary network requests.♻️ Suggested fix
// Fetch organization members from Convex - const orgMembers = useQuery(api.member.listByOrganization, { - organizationId, - sortOrder: 'asc', - }); + const orgMembers = useQuery( + api.member.listByOrganization, + open ? { organizationId, sortOrder: 'asc' } : 'skip' + );
129-135: Non-null assertion onidentityIdcould cause runtime issues.The filter on line 57 checks for
m.identityId, but TypeScript's type narrowing doesn't propagate through theuseMemoboundary. The!assertion on line 132 could fail if the type system is wrong.🐛 Safer approach using type guard
const availableMembers = useMemo(() => { if (!orgMembers || !teamMembers) return []; const teamMemberIds = new Set(teamMembers.map((m) => m.userId)); - return orgMembers.filter((m) => m.identityId && !teamMemberIds.has(m.identityId)); + return orgMembers.filter( + (m): m is typeof m & { identityId: string } => + !!m.identityId && !teamMemberIds.has(m.identityId) + ); }, [orgMembers, teamMembers]);Then line 132 becomes safe:
- value: m.identityId!, + value: m.identityId,services/platform/app/(app)/dashboard/[id]/settings/teams/page.tsx (1)
32-57: Consider making skeleton synchronous if i18n keys are static.The
TeamsSettingsSkeletonfunction is async solely for thegetTcall. If the translation keys are static strings known at build time, you could inline them or use a sync approach to simplify.services/platform/convex/documents.ts (1)
936-939: Consider explicit auth validation forcreatedBytracking.The code retrieves
authUserbut doesn't validate it before usingauthUser?.userIdforcreatedBy. If authentication state becomes inconsistent, documents could be created without creator tracking.💡 Suggested defensive check
// Get current authenticated user for createdBy tracking const authUser = await getAuthenticatedUser(ctx); - const createdBy = authUser?.userId; + if (!authUser) { + return { success: false, error: 'Not authenticated' }; + } + const createdBy = authUser.userId;Note:
mutationWithRLSenforces authentication, butgetAuthenticatedUserreturnsnullon failure rather than throwing. Adding an explicit check prevents silent failures in creator tracking.services/platform/app/(app)/dashboard/[id]/(knowledge)/documents/components/document-row-actions.tsx (1)
164-191: Inconsistent dialog mounting strategy remains.
DocumentTeamTagsDialogis always mounted while delete dialogs are conditional. If Radix animation state matters, prefer a consistent approach (either always mounted or always conditional).compose.yml (1)
173-178: FalkorDB UI port should be dev-only.Publishing
6380:3000exposes the browser UI; consider gating it behind a dev-only compose override or env flag to reduce production attack surface.
larryro
force-pushed
the
feat/152-multi-tenancy-teams
branch
from
c835696 to
7cf4d4e
Compare
Collaborator
Author
|
Fixed docstring in |
Collaborator
Author
|
Regarding the redundant cookie deletion in |
Collaborator
Author
|
Fixed. Made dialog mounting consistent by always mounting all dialogs ( |
Collaborator
Author
|
Fixed. Conditionally render |
Collaborator
Author
|
Fixed. Extracted the translation string into a stable variable ( |
Collaborator
Author
|
Fixed. Added loading state ( |
Collaborator
Author
|
Fixed both issues:
|
Collaborator
Author
|
I've reviewed this suggestion and the current implementation is intentional. The skeleton needs to use |
Collaborator
Author
|
Fixed. Added composite index |
Collaborator
Author
|
I've verified that |
- Add multi-tenancy and teams support - Replace Kuzu/LanceDB with FalkorDB for graph and vector storage - Add prefetch for first RAG search call - Improve trusted headers debug logging format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use --omit=dev instead of --legacy-peer-deps for cleaner production builds - Add optional dependencies for Linux native binaries (tailwindcss oxide, parcel watcher, swc) - Sort dependencies alphabetically in platform package.json Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Users now receive feedback when selected files are rejected due to exceeding the maximum file size limit. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…leteDialog Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…mber lookup Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The litellm.aembedding API accepts both string and list inputs per the OpenAI Embeddings API spec. The patch was incorrectly typed as list-only which would cause string inputs to be iterated character-by-character. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The cognify_kwargs was checking for "user" in add_kwargs, but add_kwargs only contains dataset_name and node_set, so this condition was always false. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add missing await to self.query() in _patched_is_empty - Validate DATABASE_URL components before exporting DB_* env vars Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Required for PEP 517/518 compliant builds to ensure proper installation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The graph-db service now uses FalkorDB (Redis-based) instead of Kuzu. The kuzu dependency was unused as there are no Python files in the service. Also added build-system section for pyproject.toml compliance. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added defensive auth check in createDocumentFromUpload to ensure documents are never created without createdBy tracking. This is consistent with deleteDocument which already has explicit auth validation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…dTeams Update the create path to use ?? (nullish coalescing) instead of || (logical OR) for trustedRole and trustedTeams fields. This ensures consistent behavior between create and update paths - both now use ?? which preserves empty strings instead of treating them as falsy. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pin the dependency to version 0.2.0 for consistency with other pinned dependencies in pyproject.toml and to ensure reproducible builds. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add team selection UI to OneDrive import dialog settings stage - Pass teamTags to backend API and validate user membership - Propagate teamTags through sync config creation for recurring syncs - Fix Radix Dialog "Maximum update depth exceeded" by unmounting content during close animation - Exclude Storybook files from Docker builds - Define local PDF.js interfaces to avoid type import issues - Add quickXorHash and optional parentReference fields to OneDrive validators - Fix BetterAuth adapter query to use _id field - Optimize Graph API calls with $select to reduce payload size Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove --omit=dev flag from npm ci in Dockerfile to ensure TypeScript type definitions are available during the build stage. Also removes unused @tailwindcss/oxide-linux-x64-gnu root dependency. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Setting NODE_ENV=production before npm install causes devDependencies to be skipped, which breaks TypeScript compilation. Also pin @types/jexl version for consistency. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
larryro
force-pushed
the
feat/152-multi-tenancy-teams
branch
from
47f1373 to
3b703bb
Compare
Move description to dialog body with proper text wrapping and extract just the filename from full document path for cleaner display. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extract and display only the filename from full paths in the document table. This improves readability when documents have deep folder structures, while preserving the full path in the title tooltip. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Replace console.log/error with debug logger in document operations - Add comprehensive logging to RAG upload process with request details - Add file extension validation with supported formats list - Add UnicodeDecodeError handling with helpful error messages - Pre-download tiktoken encodings in Docker to support air-gapped environments - Disable telemetry to prevent SSL errors to external endpoints - Reduce embedding log verbosity in litellm patch Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
3 tasks
This was referenced Feb 4, 2026
This was referenced Mar 11, 2026
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 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
@microsoft/mgt-reactwith lightweightreact-file-iconfor better bundle sizeTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.