refactor(convex): DRY up backend, separate internals, and harden authorization#397
Merged
Conversation
…direct crypto calls - Extract reusable validator constants for conversations and messages - Add hasRecordsInOrg helper to replace repeated first-record queries - Replace ctx.runAction crypto wrappers with direct encryptString/decryptString imports - Rename getApprovalsByOrganization to listApprovalsByOrganization - Switch public api.* references to internal.* where appropriate - Standardize auth error messages to "Unauthenticated" - Remove duplicate query functions and dead code
Eliminate double-nested paths, flat files, misplaced function types,
and duplicate exports so every public endpoint follows the
/{domain}/{queries|mutations|actions}/{fn} convention.
- Remove duplicate thread query exports (4 endpoints → 2)
- Move 6 SSO provider actions from queries/mutations into actions.ts
- Flatten integrations subdirectories into single queries/mutations/actions files
- Migrate legacy flat files (member, team_members, improve_message, etc.)
- Extract startWorkflow into workflow_engine/mutations.ts
- Rename file/ → files/ for plural consistency
- Regenerate OpenAPI spec (154 → 151 endpoints, 0 uncategorized)
…an up Convex domains Extract build_audit_context helper, replace duplicate public endpoints with transformed variants, remove deprecated functions (regenerate_tone_of_voice, create_product_public), and eliminate redundant type/validator definitions across all Convex domains.
…iminate type casts Drop Public/Internal naming convention from Convex endpoints in favor of clear role-based names (e.g. provisionWebsite for internal creation, resolveWorkflow for internal lookup). Replace unsafe type casts with implicit typing and runtime type narrowing across SSO providers and team members. Remove unused validators, types, and redundant JSDoc comments.
…e OpenAPI YAML Replace all `as` casts with proper type narrowing and shared validators (stepTypeValidator, editModeValidator, workflowStatusValidator). Remove unused imports, redundant comments, obsolete convex-openapi.yaml, and unused paginationOpts from listAutomations.
…h update_product.ts
…e checks to addMember
…n initial scan provisioning
…_*.ts files Extract internalMutation, internalAction, and internalQuery definitions from mixed public-facing files into dedicated internal_mutations.ts, internal_actions.ts, and internal_queries.ts modules across the entire Convex backend. Consolidates scattered single-purpose internal files (e.g., create_human_input_request.ts, execute_integration_internal.ts) into the new convention. Public-facing files now only contain public mutations, queries, and actions.
larryro
force-pushed
the
refactor/convex-dry-and-cleanup
branch
from
55849e3 to
b329ee5
Compare
📝 WalkthroughWalkthroughLarge-scale refactor standardizing Convex API namespaces and visibility: numerous frontend calls switched to new query/mutation/action paths (e.g., public → internal_{queries|mutations|actions}, renamed list/get/update endpoints). Added new internal modules for approvals, conversations, documents, integrations, products, tone_of_voice, websites, streaming, threads; removed/deprecated legacy endpoints/files. Introduced shared helpers (audit context, agent runtime config, org record checks), expanded validators, and adjusted return types/signatures (e.g., explicit returns, renamed exports, argument object patterns). Updated OAuth2/email provider flows to new internal crypto/oauth actions. Standardized auth error strings (“Unauthenticated”) and consolidated function refs to new paths. Estimated code review effort🎯 5 (Critical) | ⏱️ ~180 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 (12)
services/platform/convex/node_only/email_providers/internal_actions.ts (2)
73-87:⚠️ Potential issue | 🟡 MinorAdd explicit timeout and defensive error handling for IMAP verification.
Two reliability concerns:
Missing timeout: Unlike SMTP which has explicit
connectionTimeoutandgreetingTimeout, the IMAP client has no timeout configured. On network issues, this could hang indefinitely.Logout in finally: If
connect()throws, callinglogout()could throw a secondary error that masks the original connection failure.🛡️ Proposed fix
verifyImapConnection: async ({ imapConfig, auth }) => { const client = new ImapFlow({ host: imapConfig.host, port: imapConfig.port, secure: imapConfig.secure, auth, logger: false, + connectionTimeout: 10000, }); try { await client.connect(); } finally { - await client.logout(); + try { + await client.logout(); + } catch { + // Ignore logout errors to preserve original connection error + } } },
92-95: 🧹 Nitpick | 🔵 TrivialRedundant nullish coalescing.
Lines 94-95 use
?? undefinedwhich is a no-op sinceundefined ?? undefinedreturnsundefined. The optional validator fields are alreadyundefinedwhen not provided.✨ Suggested simplification
- passwordAuth: args.passwordAuth ?? undefined, - oauth2Auth: args.oauth2Auth ?? undefined, + passwordAuth: args.passwordAuth, + oauth2Auth: args.oauth2Auth,services/platform/convex/conversations/update_conversation.ts (1)
37-44:⚠️ Potential issue | 🟡 MinorMetadata changes are not captured in the audit trail.
The
metadatafield is updated but not tracked inpreviousStateornewState. If metadata changes should be auditable, capture them similarly to other fields.💡 Optional fix to include metadata in audit trail
if (args.metadata !== undefined) { const existingMetadata = (conversation.metadata as Record<string, unknown>) || {}; + previousState.metadata = existingMetadata; updateData.metadata = { ...existingMetadata, ...(args.metadata as Record<string, unknown>), }; + newState.metadata = updateData.metadata; }services/platform/convex/threads/mutations.ts (1)
52-59:⚠️ Potential issue | 🟠 MajorPass
authUser._idtoupdateChatThreadHelperto verify thread ownership.The mutation authenticates the user but doesn't pass
authUser._idtoupdateChatThreadHelperfor ownership verification, creating a security gap. This is inconsistent withcreateChatThread(line 21-23) which correctly passesauthUser._id, and mirrors the same issue indeleteChatThread(line 41). The helper must receive user identification to validate that the authenticated user owns the thread before allowing modification.services/platform/convex/threads/queries.ts (2)
39-67: 🧹 Nitpick | 🔵 TrivialMissing
returnsvalidator for consistency.The
listThreadsquery (lines 12-20) defines an explicitreturnsvalidator, butgetThreadMessagesStreamingdoes not. Adding areturnsvalidator improves type safety, enables better OpenAPI spec generation (which is a goal of this PR), and maintains consistency across the queries in this file.
68-94:⚠️ Potential issue | 🟠 MajorMissing authorization check: authenticated user can access any thread's messages.
The query authenticates the user but does not verify that the authenticated user owns the requested
threadId. Any authenticated user can access message streams from any thread by providing its ID.Unlike
listThreadswhich filters byuserId, this query passes onlythreadIdto the helper without ownership validation. The helper function (getThreadMessagesStreaming) does not accept a userId parameter and does not perform ownership checks before delegating tolistUIMessagesandsyncStreams.Update the helper signature to accept
userIdand implement thread ownership validation:Suggested fix
if (!authUser) { return { page: [], isDone: true, continueCursor: '', streams: { value: null }, }; } return await getThreadMessagesStreamingHelper(ctx, { threadId: args.threadId, + userId: String(authUser._id), paginationOpts: args.paginationOpts, streamArgs: args.streamArgs, });Then update
getThreadMessagesStreamingto validate thread ownership against the provideduserIdbefore returning messages.services/platform/convex/email_providers/test_existing_provider_logic.ts (1)
173-188:⚠️ Potential issue | 🟠 MajorSMTP is unconditionally tested even for API-based providers; the dummy config will fail to connect before the result override takes effect.
When
sendMethod === 'api',deps.testConnection()is called with a dummy SMTP config (localhost:587). However, the underlyingtestNewProviderConnectionLogicalways invokestestSmtpConnectionLogic, which attempts an actual connection to the provided host. This connection tolocalhost:587will fail or hang before the result is manually overridden, defeating the purpose of skipping SMTP for API-based providers.The
latencyMs: 0anderror: undefinedare also misleading since a failed connection attempt actually occurred.Either add a
skipSmtpflag that prevents the SMTP test from running entirely, or create a separate IMAP-only connection helper for API-based providers.services/platform/convex/sso_providers/internal_mutations.ts (1)
7-24: 🧹 Nitpick | 🔵 TrivialPrefer local validators in internal_mutations (unless intentionally changing the pattern).
This import reverses the established practice of defining these validators inline for boundary visibility. If the consolidation is intentional, consider adding a comment to document the deviation; otherwise, keep the validators local here.
Based on learnings "In services/platform/convex/sso_providers/internal_mutations.ts, validators (platformRoleValidator, roleMappingRuleValidator, providerFeaturesValidator, etc.) are intentionally defined locally rather than imported from the shared validators module (./validators.ts)."
services/platform/convex/sso_providers/actions.ts (1)
107-135:⚠️ Potential issue | 🟡 MinorStandardize auth error message text.
Line 117 uses “Not authenticated” while Line 99 uses “Unauthenticated”. This creates inconsistent error handling.
🔧 Suggested fix
- return { valid: false, error: 'Not authenticated' }; + return { valid: false, error: 'Unauthenticated' };services/platform/app/features/automations/components/automation-navigation.tsx (1)
210-214:⚠️ Potential issue | 🟡 MinorUse the automations navigation key for “archived”.
The archived label in the automations navigation context should use the navigation-specific translation key.
🔧 Proposed fix
- {version.status === 'archived' && - tCommon('status.archived')} + {version.status === 'archived' && + t('navigation.archived')}Based on learnings: prefer using the navigation-specific localization key t('navigation.archived') for archived labels in automations/components.
services/platform/convex/approvals/queries.ts (2)
29-34:⚠️ Potential issue | 🟡 MinorMinor inconsistency:
authUser._idvsString(authUser._id).Line 31 passes
authUser._iddirectly togetOrganizationMember, while other files in this PR (e.g.,documents/queries.tsline 42,integrations/queries.tsline 28) wrap it withString(authUser._id). Verify if this inconsistency causes issues or if both forms are acceptable.
43-134:⚠️ Potential issue | 🟠 MajorThread-based approval queries lack ownership verification, enabling cross-thread access.
The three thread-specific queries (
getPendingIntegrationApprovalsForThread,getWorkflowCreationApprovalsForThread,getHumanInputRequestsForThread) check only that the user is authenticated, but do not verify the user owns or has access to the specifiedthreadId. Since approvals are org-scoped (containorganizationId) and threads are user-scoped (containuserId), an authenticated user can query approvals for any threadId and retrieve approvals they should not access.Add verification that the user owns the thread (via
userId) or is a member of the approval's organization before returning results.
🤖 Fix all issues with AI agents
In `@services/platform/convex/agent_tools/human_input/internal_mutations.ts`:
- Around line 31-44: The current validation only requires options for
'single_select' and 'multi_select', but the UI renders metadata.options for
'yes_no' too; update the validation in internal_mutations.ts (around the block
checking args.format) to either include 'yes_no' in the required-options check
or, preferably, default metadata.options when args.format === 'yes_no' and
args.options is missing/empty (set metadata.options to a Yes/No pair) before
building the HumanInputRequestMetadata object so metadata.options is always
present for the UI rendering.
In `@services/platform/convex/agent_tools/integrations/internal_actions.ts`:
- Line 477: The dynamic import of
'../../workflow_engine/actions/integration/integration_action' inside
executeRestApiBatch (assigned to integrationActionModule) likely exists to avoid
a circular dependency but adds runtime overhead; add a concise in-code comment
above that import explaining the circular-dependency rationale (e.g., "dynamic
import to avoid circular dependency with X, intentional despite runtime cost")
and, if desired, note any plans to refactor; ensure the comment references
executeRestApiBatch and integration_action/integrationActionModule so future
maintainers understand why the pattern is used.
In `@services/platform/convex/agent_tools/integrations/internal_mutations.ts`:
- Around line 37-45: The code calls Date.now() twice when updating the approval
and its metadata, risking inconsistent timestamps; capture a single timestamp
into a variable (e.g., executedAt = Date.now()) before calling ctx.db.patch and
use that variable for both the top-level executedAt and the metadata.executedAt
fields in the ctx.db.patch call (keeping references to args.approvalId,
metadata, executionResult, and executionError unchanged).
In `@services/platform/convex/agent_tools/workflows/internal_mutations.ts`:
- Around line 18-20: The early silent return in the handler for the internal
mutation hides missing-approval problems; in the async handler (the function
using ctx.db.get(args.approvalId) and the approval variable) replace the bare
"if (!approval) return" with explicit handling—either throw a descriptive error
(e.g., referencing args.approvalId and "approval not found") or at minimum log a
warning via ctx.log/processLogger before returning—so callers and logs will
surface missing approvals instead of failing silently.
In `@services/platform/convex/agents/chat/internal_actions.ts`:
- Around line 20-65: The handler in generateResponse currently reads
process.env.OPENAI_MODEL and hardcodes provider 'openai'; instead, import and
call the project's shared runtime config helper to obtain the model and provider
(the same helper used elsewhere in the repo) and pass those values into
generateAgentResponse (keep agentType, createAgent/createChatAgent, debugTag,
enableStreaming, and instructions/CHAT_AGENT_INSTRUCTIONS unchanged); remove the
process.env access and hardcoded 'openai' so provider selection and model config
are centralized.
In `@services/platform/convex/agents/workflow/actions.ts`:
- Around line 45-51: The action should accept workflow IDs as plain strings at
its boundary and only cast to Convex Id<'wfDefinitions'> where a typed ID is
required; change the action arg schema to use v.string() for args.workflowId,
keep using args.workflowId when setting
additionalContext.target_workflow_id/target_workflow_name, and when calling
ctx.runQuery(getResolveWorkflowRef(), { wfDefinitionId: /* cast here */
args.workflowId as Id<'wfDefinitions'> }) perform the cast right at the call
site (referencing args.workflowId, getResolveWorkflowRef, and wfDefinitionId) so
serialization boundaries remain string-typed.
In `@services/platform/convex/approvals/internal_queries.ts`:
- Around line 6-13: getApprovalById is missing a returns validator for its
internalQuery; add a returns entry to the internalQuery call (same pattern used
by getApprovalsForThread) that validates the shape returned by
ApprovalsHelpers.getApproval so runtime validation and docs remain consistent
with other queries. Locate the internalQuery definition for getApprovalById and
add the appropriate returns validator (matching the response type used by
getApprovalsForThread / the approval record shape) to the options passed into
internalQuery.
In `@services/platform/convex/conversations/add_message_to_conversation.ts`:
- Around line 43-55: Add an organization mismatch guard: after fetching
parentConversation (the const parentConversation = await
ctx.db.get(args.conversationId) block) validate that args.organizationId ===
parentConversation.organizationId (or treat missing args.organizationId as
mismatch) and throw an error (e.g., "Organization mismatch" or similar) before
any further processing or writes (before computing direction/deliveryState or
calling resolveDeliveryState and before creating/updating messages). This
ensures add_message_to_conversation.ts cannot perform cross‑org writes when
conversationId leaks.
- Around line 56-72: The metadata spreads in the ctx.db.insert call inside
add_message_to_conversation.ts risk spreading non-object values from
args.metadata and parentConversation.metadata; add runtime guards that verify
typeof metadata === 'object' && metadata !== null (or Array.isArray check if
arrays not allowed) before casting to Record<string, unknown> and spreading into
the metadata object, and apply the same guard where parentConversation.metadata
is spread later (the other spread mentioned around the second metadata usage) so
only validated objects are cast and merged.
In `@services/platform/convex/conversations/bulk_close_conversations.ts`:
- Around line 64-65: The audit log's failedCount calculation (const failedCount
= args.conversationIds.length - successCount) mixes "not found" IDs and actual
patch failures, which can be misleading; update the code around failedCount and
the audit logging in bulkCloseConversations to either (a) rename the audit field
to something like notSuccessfullyClosedCount or (b) compute two explicit
metrics—notFoundCount (IDs that never existed) and patchFailureCount (IDs that
were found but patch failed)—and log both; locate and change the variables and
log usage where failedCount, args.conversationIds, and successCount are
referenced (and update any audit payload keys) so the semantics are clear in the
audit record.
In `@services/platform/convex/conversations/internal_mutations.ts`:
- Around line 163-198: lastId is declared as string | null but is assigned
conv._id (type Id<'conversations'>); change the declaration of lastId to
Id<'conversations'> | null and remove the later cast in the returned nextCursor,
and ensure the Id type is imported from _generated/dataModel (Id) so types line
up with conv._id, lastId, and the returned nextCursor.
In `@services/platform/convex/conversations/internal_queries.ts`:
- Around line 11-28: The internalConversationRecordValidator currently uses
v.optional(v.string()) for the priority field but conversationPriorityValidator
is imported and unused; update the priority entry in
internalConversationRecordValidator to use
v.optional(conversationPriorityValidator) to ensure consistent validation with
conversationStatusValidator, or if the looser string type is intended, remove
the unused conversationPriorityValidator import to avoid dead code (refer to
internalConversationRecordValidator and conversationPriorityValidator to locate
changes).
In `@services/platform/convex/conversations/reopen_conversation.ts`:
- Around line 17-19: The code blindly casts conversation.metadata to
Record<string, unknown> then destructures it into restMetadata; add a runtime
guard to ensure conversation.metadata is a plain object before casting and
destructuring (check typeof conversation.metadata === 'object' &&
conversation.metadata !== null && !Array.isArray(conversation.metadata')), and
if the guard fails treat metadata as an empty object so the destructure of
resolved_at/resolved_by is safe; update the variables referenced
(conversation.metadata, metadata, restMetadata, resolved_at, resolved_by)
accordingly so the restMetadata fallback is correct.
In `@services/platform/convex/conversations/send_message_via_email.ts`:
- Around line 44-58: extractSenderEmail currently accesses
provider.metadata.oauth2_user directly (inconsistently with getProviderEmail);
update extractSenderEmail to mirror the safe pattern: cast provider.metadata to
Record<string, unknown>, read the "oauth2_user" key from that record, verify
it's a string before returning, and keep the same error path when missing; refer
to the extractSenderEmail function and getProviderEmail for the exact access
pattern to replicate.
In `@services/platform/convex/customers/internal_mutations.ts`:
- Around line 56-64: The updates schema currently allows any string for the
source field causing invalid values to be persisted; change the updates field
definition inside the updates v.object (in internal_mutations.ts) to use
v.optional(customerSourceValidator) for the source property instead of
v.optional(v.string()) so it matches create/find validators and ensures only
valid customer sources are accepted.
In `@services/platform/convex/customers/internal_queries.ts`:
- Around line 32-52: The source arg in internalQuery/queryCustomers currently
allows v.optional(v.union(customerSourceValidator, v.array(v.string()))) which
permits arbitrary strings for array inputs; update the schema so array inputs
use the same customerSourceValidator (e.g., v.array(customerSourceValidator))
instead of v.array(v.string()) to ensure consistency with single-value
validation; change the args.source union to
v.optional(v.union(customerSourceValidator, v.array(customerSourceValidator)))
within the queryCustomers/internalQuery definition so
CustomersHelpers.queryCustomers always receives validated source values.
In `@services/platform/convex/documents/internal_actions.ts`:
- Around line 298-330: The deleteDocumentFromRag internal action currently
swallows failures and always returns null, leaving callers unable to detect RAG
deletion failures; update the handler in deleteDocumentFromRag to either
propagate failures or return an explicit status: (a) throw an error when fetch
fails (non-ok response or caught exception) so callers can handle
retry/rollback, or (b) return a structured result like { success: boolean,
error?: string } on failure, and update callers accordingly; if you intend
fire-and-forget behavior instead, add a clear comment above the
deleteDocumentFromRag handler stating that failures are intentionally
logged-only and not propagated.
- Around line 123-136: The handler in internalAction generatePptx uses a
non-null assertion on args.templateStorageId (args.templateStorageId!) even
though the args schema declares templateStorageId as optional; remove the
non-null assertion and pass templateStorageId through as-is to
DocumentsHelpers.generatePptx, or alternatively make templateStorageId required
in the args validator if the helper truly needs it (update the
v.optional(v.id('_storage')) to v.id('_storage') and adjust callers). Ensure
changes reference generatePptx, the args definition (templateStorageId), and
DocumentsHelpers.generatePptx.
In `@services/platform/convex/documents/mutations.ts`:
- Around line 115-119: The declared response schema in the mutation (the returns
v.object block that includes error: v.optional(v.string())) is inconsistent with
the handler which always returns { success: true, documentId } and never sets
error; either remove the optional error field from the returns schema to match
the current behavior (and update any callers/types that expect it) or change the
mutation handler (the function handling this mutation in
services/platform/convex/documents/mutations.ts) to catch errors and return {
success: false, documentId: null, error: err.message } on failure; pick one
approach, update the returns schema and the handler consistently (and any
calling code) so the declared shape matches actual runtime responses.
In `@services/platform/convex/documents/queries.ts`:
- Around line 12-24: The getDocumentById query currently only checks
authentication; update its handler to enforce organization membership by
fetching the document record (via DocumentsHelpers.getDocumentById or
getDocumentByIdTransformed) to obtain the document.organizationId and then call
getOrganizationMember(ctx, { organizationId, userId: authUser.id }) (or
equivalent membership check used by getDocumentByPath/listDocuments); if the
user is not a member return { success: false, error: 'Forbidden' } (or similar).
Also add the same org-check inside
DocumentsHelpers.getDocumentById/getDocumentByIdTransformed (and ensure
transformDocumentsBatch respects org scoping) so helper-level calls cannot
bypass the access control. Ensure you reference the authUser from
authComponent.getAuthUser(ctx) and fail fast before returning document data.
In `@services/platform/convex/files/queries.ts`:
- Line 7: The validator for fileId currently uses v.id('_storage') which will
reject serialized string IDs coming from ctx.runQuery and useQuery; update the
validator for fileId to use v.string() instead (replace v.id('_storage') with
v.string()) so the query accepts the serialized form passed by callers (look for
the fileId field in the query definition in queries.ts).
In `@services/platform/convex/integrations/actions.ts`:
- Around line 46-64: The update action currently delegates authorization to
updateIntegrationLogic and lacks an explicit auth check; add a direct
authorization/permission check at the start of the update action's handler (the
action named update) before calling updateIntegrationLogic—mirror the explicit
auth validation used in testConnection (or the PR's auth hardening pattern) to
verify caller permissions and reject unauthorized requests, referencing the
update action's handler and updateIntegrationLogic to locate where to insert the
check.
- Around line 23-44: The create action currently delegates auth/org validation
to createIntegrationLogic which is inconsistent with testConnection; add an
explicit authorization check in the create action handler before calling
createIntegrationLogic that mirrors testConnection's behavior — e.g., use the
same API/context call used in testConnection (such as calling
api.integrations.queries.list or the organization membership/assertion utility)
to verify ctx's caller is allowed to act on args.organizationId, throw/return a
forbidden error if not authorized, then proceed to await
createIntegrationLogic(ctx, args); keep the existing args/returns intact.
In `@services/platform/convex/integrations/internal_mutations.ts`:
- Around line 84-95: The handler currently builds cleanUpdates via a manual
loop; replace that with the established pattern so ctx.db.patch receives a
properly typed object: derive cleanUpdates from updates using
Object.fromEntries(Object.entries(updates).filter(([_, v]) => v !== undefined))
and then call await ctx.db.patch(integrationId, cleanUpdates); keep the same
identifiers (handler, integrationId, updates, ctx.db.patch) and ensure the
resulting object type aligns with the patch API.
In `@services/platform/convex/lib/agent_completion/function_refs.ts`:
- Around line 51-52: The createRef call in getSaveMessageMetadataRef uses a
non-existent module path; change the module segment 'mutations' to
'internal_mutations' so the path becomes ['message_metadata',
'internal_mutations', 'saveMessageMetadata']; update the return in
getSaveMessageMetadataRef (the createRef<SaveMessageMetadataRef> call)
accordingly to match other refs like startStream and linkApprovalsToMessage.
In `@services/platform/convex/lib/helpers/has_records_in_org.ts`:
- Around line 10-30: The switch in has_records_in_org (cases for
'customers','conversations','products','vendors','websites') repeats the same
db.query(...).withIndex('by_organizationId', q => q.eq('organizationId',
organizationId)).first() pattern; refactor to a single helper that takes the
table name (or a small map from allowed literal names to query functions) and
performs that query once to return r !== null, or if Convex requires literal
table names for type safety, add a tiny wrapper function like
hasRecordsInTable(tableName) used by each case to centralize the logic while
preserving literals; reference the repeated call sites using db.query(...) and
the index name 'by_organizationId' when consolidating.
In `@services/platform/convex/members/queries.ts`:
- Around line 212-273: The getMyTeams query currently loops sequentially over
teamIds and awaits each components.betterAuth.adapter.findMany call (teamResult)
inside the for...of, causing slower behavior; replace the sequential loop with a
parallel approach by mapping teamIds to Promises that call
ctx.runQuery(components.betterAuth.adapter.findMany, {...}) for each teamId,
await Promise.all on that array, then iterate the resolved results to push valid
teams (from each teamResult.page[0]) into the teams array; ensure you preserve
the existing filters (model: 'team' and organizationId check) and the return
shape from getMyTeams.
In `@services/platform/convex/node_only/imap/internal_actions.ts`:
- Around line 22-23: The schema currently marks password and accessToken as
optional but the runtime later assumes one is present (it reads
credentials.password), causing undefined usage; fix by updating the validator
for the credentials object to enforce a discriminated-union or at-least-one rule
(so either password:string or accessToken:string is required), or add a runtime
guard early in the handler that reads credentials (where credentials.password is
used) to throw/return a clear validation error if both are missing; update the
validator that defines password/accessToken and the runtime check that reads
credentials.password accordingly so callers cannot reach the password-branch
with undefined.
In `@services/platform/convex/organizations/save_default_workflows.ts`:
- Around line 22-33: The workflow modules export default values without type
annotations, forcing callers to cast each import (e.g., documentRagSync as
PredefinedWorkflowDefinition) — add explicit PredefinedWorkflowDefinition
annotations to the exported constants in each workflow file (e.g., declare const
documentRagSyncWorkflow: PredefinedWorkflowDefinition = ... and export default
documentRagSyncWorkflow; likewise for onedriveSync,
generalCustomerStatusAssessmentWorkflow, generalProductRecommendationWorkflow,
productRecommendationEmailWorkflow, conversationAutoArchiveWorkflow) so imports
naturally have the correct type and you can remove the casts from the
DEFAULT_WORKFLOWS array.
In `@services/platform/convex/products/create_product.ts`:
- Line 37: The metadata field is being cast precisely as
Doc<'products'>['metadata'] which breaks the repo convention for v.any() fields;
update the insert payload in create_product (the object passed to ctx.db.insert
in create_product.ts) to cast the entire insert object (or at least its metadata
property) as any instead of using Doc<'products'>['metadata'] so it matches the
established pattern used elsewhere for v.any() fields (also apply same approach
for ctx.db.patch usages if present).
In `@services/platform/convex/team_members/mutations.ts`:
- Around line 30-58: Before checking/adding a team member, fetch the team via
ctx.runQuery using components.betterAuth.adapter.findMany with model: 'team' and
where: [{ field: 'id', value: args.teamId, operator: 'eq' }], then verify the
returned team's organizationId matches args.organizationId; if no team is found
or organizationId mismatches, throw an error like 'Team not found in
organization'. Perform this team lookup and org match check before calling the
existingTeamMember query and the subsequent add logic so you prevent
cross-organization membership changes.
- Around line 88-114: The code currently allows any org member to remove any
team member; after calling getOrganizationMember(...) capture its return (e.g.,
const requester = await getOrganizationMember(...)) and enforce authorization:
allow removal only if requester.role is 'owner' or 'admin' (or whatever
admin/owner roles are named) or if String(requester._id) ===
String(memberToRemove.userId) (self-removal). If neither condition holds, throw
an authorization error (e.g., throw new Error('Not authorized to remove team
member')). Use existing symbols: getOrganizationMember, requester (new
variable), memberToRemove (from components.betterAuth.adapter.findOne),
args.teamMemberId and args.organizationId, and team to perform these checks
before proceeding with removal.
In `@services/platform/convex/team_members/queries.ts`:
- Around line 24-31: The catch block that calls authComponent.getAuthUser
currently swallows errors; update it to log the caught error before returning an
empty array so failures are observable. In the try/catch around
authComponent.getAuthUser(ctx) capture the error (e) and call your project
logger or console.warn/console.error with a short message like "getAuthUser
failed" plus the error and any context (e.g., ctx or caller info), then continue
returning [] as before; keep the subsequent authUser null check unchanged.
In `@services/platform/convex/threads/internal_mutations.ts`:
- Around line 6-20: The internalMutation getOrCreateSubThreadAtomic is missing a
returns validator; update the internalMutation call for
getOrCreateSubThreadAtomic to include a returns field that matches the shape
returned by getOrCreateSubThread (use the appropriate validator from your v.*
helpers), e.g. add returns: <validator matching the returned Thread or ID type>,
so the runtime schema and docs match the actual return value and mirror the
pattern used in the public mutations file.
In `@services/platform/convex/tone_of_voice/actions.ts`:
- Around line 12-19: The public action generateToneOfVoice currently skips
auth/authorization; update its handler to call authComponent.getAuthUser(ctx),
throw on missing authUser, then call getOrganizationMember(ctx,
args.organizationId, { userId: String(authUser._id), email: authUser.email,
name: authUser.name }) to verify org membership before invoking
generateToneOfVoiceHelper; ensure you return the helper result only after these
checks so behavior matches retryRagIndexing and mutations that use
authComponent.getAuthUser and getOrganizationMember.
In `@services/platform/convex/tone_of_voice/add_example_message.ts`:
- Around line 36-42: The insert into 'exampleMessages' is missing the
established "as any" cast for the optional metadata field; update the metadata
assignment in the ctx.db.insert call (in add_example_message) to pass
args.metadata as any (e.g., metadata: args.metadata as any) so it matches the
pattern used elsewhere like create_customer.ts and satisfies the schema's
optional jsonRecordValidator handling.
In `@services/platform/convex/tone_of_voice/upsert_tone_of_voice.ts`:
- Around line 27-31: The update currently overwrites existing.metadata with
args.metadata even when undefined; in upsert_tone_of_voice change the
ctx.db.patch call to only include metadata when args.metadata is !== undefined
(so preserve existing.metadata if omitted), and for the ctx.db.insert path set
metadata: args.metadata ?? {} and cast the whole insert/update object with "as
any" (per the existing v.any() pattern); reference ctx.db.patch, ctx.db.insert,
existing, args.metadata, generatedTone and lastUpdated to locate the spots to
change.
In `@services/platform/convex/websites/internal_actions.ts`:
- Around line 5-15: Change the internalAction arg schema so websiteId is
declared as v.string() (since the action crosses a function boundary) and inside
the handler cast/parse that string back to an id before calling
WebsitesHelpers.provisionWebsiteScanWorkflow; update the args block in
provisionWebsiteScanWorkflow to use websiteId: v.string() and in the handler
convert args.websiteId to the repository id type (e.g., via the v.id('websites')
parser or equivalent cast) and pass the converted value to
WebsitesHelpers.provisionWebsiteScanWorkflow.
…ve casts in save_default_workflows
Add maxScanItems to paginateWithFilter to prevent exceeding Convex's 16MB read limit on large documents. Use dynamic pageSize instead of hardcoded 1000 in executions query. Fix click event propagation in rag-status-badge. Simplify conversations route to use Route.useParams() instead of manual pathname parsing.
This was referenced Feb 8, 2026
This was referenced Mar 7, 2026
This was referenced Mar 28, 2026
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
7 tasks
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
lib/helpers/,lib/shared/validators/), removing ~16k lines of duplication and dead codeinternalMutation,internalQuery, andinternalActiondefinitions into dedicatedinternal_*.tsfiles across every domain for cleaner module boundaries and OpenAPI generationstartWorkflow,team_members(add/remove/list), andconversations(validate orgId matches conversation before writes)as any,as unknown, and unsafe casts with proper type guards,Doc<>types, andConvexJsonValuethroughoutconvex-openapi.yaml, redundanttypes.tsfiles, unused pagination options, and deprecated function stubsTest plan
npx convex devand verify all functions register without errorsnpx tsc --noEmit) fromservices/platform/npm run lint --workspace=@tale/platform)Summary by CodeRabbit
New Features
Improvements
Bug Fixes