Feat/tools/multi obj #1713
Feat/tools/multi obj #1713
Conversation
- Introduced `schema-mapping.ts` for schema traversal, path conversion, and resource field collection. - Added `publishToCommunity` and `publishReviewStatus` fields to `WorkflowApp` type. - Enhanced `SchemaProperty` type with `const`, `oneOf`, and `anyOf` properties for better schema validation. - Updated `JsonSchema` to include `omitFields` for response filtering. - Refactored `ResponseSchema` to directly reference `JsonSchema`. - Updated OpenAPI schema definitions to reflect new properties and types. - Modified generated types to include new schema features and resource tags.
…organization and functionality
- Added tool-context.ts for managing request context, including user, request ID, and LangChain config. - Updated tool.module.ts to include BillingModule. - Refactored tool.service.ts to streamline tool instantiation and remove unused queue injection. - Enhanced utility exports in index.ts to include tool-context. - Extended OpenAPI schema with new components for post-processing context and results. - Generated TypeScript types for new OpenAPI schema components.
…nce related components
WalkthroughAdds a Billing module and BillingService, replaces single creditCost with discountedPrice/originalPrice across credit flows, introduces PostHandlerService and ResourceHandler for post-execution processing and uploads, extends Composio to support OAuth and API-key tools, adds frontend OAuth popup flow and JSON-schema utilities/types across OpenAPI/generated schemas. Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
refly/apps/api/src/modules/tool/dynamic-tooling/core/handler-post.ts
Lines 180 to 185 in 1708db9
Preserve array responses when extracting file metadata
The post-handler now allows HandlerResponse.data to be an array, but extractFileIdToTopLevel still assumes an object and spreads the array into an object literal (data: { ...(response.data as Record<string, unknown>), ...extractedResources }). Any tool that returns an array of results will have its response shape corrupted into an object with numeric keys, breaking consumers expecting an array and potentially losing ordering. The resource extraction should keep arrays intact rather than coercing them to objects.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx (1)
546-633:selectItemshould be wrapped withuseCallbackand added torenderListItemdependencies.The
selectItemfunction usesisPolling,isOpening, andopenOAuthPopupbut is not memoized withuseCallback. Additionally,renderListItemcallsselectItembut doesn't include it in its dependency array, which could lead to stale closure issues.Based on learnings, always use useCallback for function props to maintain referential equality.
- const selectItem = (item: MentionItem) => { + const selectItem = useCallback((item: MentionItem) => { // Handle create variable case if (item.variableId === 'create-variable') { handleAddVariable(); return; } // Handle unauth OAuth tools - open OAuth popup instead of inserting if ( item.isInstalled === false && (item.source === 'toolsets' || item.source === 'tools') && item.toolsetId ) { // Prevent multiple OAuth popups if (isPolling || isOpening) { return; } // Open OAuth popup for authorization if (openOAuthPopup) { openOAuthPopup(item.toolsetId); } return; } command(item); - }; + }, [handleAddVariable, isPolling, isOpening, openOAuthPopup, command]);Then add
selectItemtorenderListItem's dependency array:}, [categoryConfigs, query, formatVariableValue, currentLanguage, t]); + }, [categoryConfigs, query, formatVariableValue, currentLanguage, t, selectItem]);packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/index.tsx (1)
353-375: Extension recreation onisPolling/isOpeningchanges may disrupt the UI.Including
isPollingandisOpeningin theuseMemodependency array causes the extension to be recreated whenever these values change. This will destroy and recreate the MentionList popup during the OAuth flow, potentially causing a disruptive user experience.Consider using refs for these values to avoid extension recreation:
+ // Use refs for reactive OAuth state to avoid extension recreation + const isPollingRef = useRef(isPolling); + const isOpeningRef = useRef(isOpening); + + useEffect(() => { + isPollingRef.current = isPolling; + }, [isPolling]); + + useEffect(() => { + isOpeningRef.current = isOpening; + }, [isOpening]); // Create mention extension with custom suggestion const mentionExtension = useMemo(() => { return createMentionExtension({ handleCommand, hasUserInteractedRef, allItemsRef, mentionPosition, setIsMentionListVisible, - openOAuthPopup, - isPolling, - isOpening, + openOAuthPopup, // This is stable from useCallback in the hook + isPollingRef, + isOpeningRef, mentionComponentRef, }); }, [ handleCommand, hasUserInteractedRef, allItemsRef, mentionPosition, setIsMentionListVisible, openOAuthPopup, - isPolling, - isOpening, mentionComponentRef, ]);Then update the extension and MentionList to read from the refs.
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mention-extension.ts (1)
285-306: OAuth props not updated inonUpdatehandler allows stale values during suggestion popup.The
onUpdatehandler passes onlyqueryandplacementtocomponent.updateProps(), but doesn't includeisPolling,isOpening, andopenOAuthPopup. SinceselectItemin MentionList uses these props to guard against selections during OAuth flows (if (isPolling || isOpening) return;), stale values can cause the guard to fail if these flags change while the suggestion popup is visible.While the extension is recreated when these values change (via the useMemo dependency array in the parent), updates during an active popup don't propagate to the MentionList component. This creates a window where a user could click a mention item while OAuth is in progress, bypassing the intended protection.
onUpdate(props) { latestProps = props; // Update latest props - component.updateProps({ ...props, query: props.query || '' }); + component.updateProps({ + ...props, + query: props.query || '', + openOAuthPopup, + isPolling, + isOpening, + });Alternatively, consider using refs for these reactive values instead of capturing them in the closure, since they can change independently of extension lifecycle.
apps/api/src/modules/credit/credit.service.ts (1)
752-795: Guard against undefineddiscountedPrice/originalPriceinsyncToolCreditUsageto prevent silent NaN corruption of credit recordsThe DTO allows these fields to be optional (
discountedPrice?: number; originalPrice?: number), but the method destructures them without validation and passes them directly intodeductCreditsAndCreateUsage. WithstrictNullChecksdisabled intsconfig.json, undefined values flow unchecked into arithmetic operations.Inside
deductCreditsAndCreateUsage:
remainingCost = creditCost(if undefined, becomes NaN afterremainingCost -= deductAmount)Math.min(recharge.balance, remainingCost)produces NaNamount: creditCostwrites NaN to the databaseWhile current call sites (billing.service.ts, tool.service.ts) always provide numeric values, the code is vulnerable to future modifications or job queue replays that omit these fields.
Additionally,
toolCallIdandtoolCallMetaare passed in the free-price path but omitted entirely from the usageData object in the paid path, creating an asymmetry. Both paths referencetoolCallMetain the description string, so the fields should be included consistently in the persisted record.Recommended fix:
async syncToolCreditUsage(data: SyncToolCreditUsageJobData) { - const { uid, discountedPrice, originalPrice, timestamp, resultId, toolCallMeta } = data; + const { uid, discountedPrice, originalPrice, timestamp, resultId, toolCallMeta, toolCallId } = data; + + if (discountedPrice == null) { + throw new Error( + `syncToolCreditUsage called without discountedPrice for uid=${uid}, resultId=${resultId}`, + ); + } + + const effectiveOriginalPrice = originalPrice ?? discountedPrice;Then use both guaranteed-numeric values below, and include
toolCallId/toolCallMetain the usageData passed todeductCreditsAndCreateUsage:- // Use the extracted method to handle credit deduction await this.deductCreditsAndCreateUsage( uid, - discountedPrice, + discountedPrice, { usageId: genCreditUsageId(), actionResultId: resultId, version: data.version, usageType: 'tool_call', + toolCallId, + toolCallMeta, createdAt: timestamp, description: `Tool call: ${toolCallMeta?.toolsetKey}.${toolCallMeta?.toolName}`, }, - originalPrice, + effectiveOriginalPrice, );apps/api/src/modules/tool/composio/composio.service.ts (2)
36-44: Unguarded access tothis.composiowhen API key is missing.If
COMPOSIO_API_KEYis not configured,this.composioremainsundefined, but methods likefetchTools,executeTool,ensureAuthConfig, etc. access it without guards, causing runtime errors.Consider either throwing during construction or adding a guard method:
+ private ensureComposioInitialized(): void { + if (!this.composio) { + throw new Error('Composio is not initialized. COMPOSIO_API_KEY must be configured.'); + } + } + async fetchTools(userId: string, integrationId: string): Promise<any[]> { + this.ensureComposioInitialized(); const tools = await this.composio.tools.get(userId, {
146-159: Add null check forconnectedAccountIdbefore Composio API call.
connection.connectedAccountIdcould be null based on the database schema. Callingthis.composio.connectedAccounts.delete()with a null value may cause unexpected behavior.- if (this.composio) { + if (this.composio && connection.connectedAccountId) { try { const result = await this.composio.connectedAccounts.delete(connection.connectedAccountId);
♻️ Duplicate comments (1)
apps/api/src/modules/tool/tool.service.ts (1)
1154-1179: Similar error handling concern with parallel instantiation.The parallel instantiation here also uses
Promise.allwhich will fail-fast if either config-based or external API key instantiation fails. This duplicates the concern raised in theinstantiateToolsetsmethod.Consider using
Promise.allSettledfor more resilient error handling, or ensure that failures in one category should prevent all toolsets from being instantiated.
🧹 Nitpick comments (30)
packages/skill-template/src/skills/agent.ts (1)
116-119: Line 118 exceeds the 100-character maximum.The template literal on line 118 is approximately 109 characters, violating the coding guideline of 100-character max line length.
Refactor to keep the line under 100 characters by breaking the message:
const toolNames = validTools.map((tool) => tool.name); this.engine.logger.info( - `Binding ${validTools.length} valid tools to LLM with tool_choice="auto": [${toolNames.join(', ')}]`, + `Binding ${validTools.length} valid tools to LLM with tool_choice="auto"`, + { toolNames: toolNames.join(', ') }, );Alternatively, extract the message to a variable:
const toolNames = validTools.map((tool) => tool.name); + const toolNamesStr = toolNames.join(', '); this.engine.logger.info( - `Binding ${validTools.length} valid tools to LLM with tool_choice="auto": [${toolNames.join(', ')}]`, + `Binding ${validTools.length} valid tools to LLM with tool_choice="auto": [${toolNamesStr}]`, );As per coding guidelines, the max line length must not exceed 100 characters.
packages/request/src/requests/types.gen.ts (2)
6747-6766: Redundant union type in index signature.The index signature
[key: string]: unknown | string | ComposioSchemaPropertycontains a redundant union sinceunknownalready subsumes all other types. This appears to be auto-generated from the OpenAPI schema, so the source schema may need adjustment if stricter typing is desired.
6768-6785: Same redundant union pattern in index signature.Similar to
ComposioToolSchema, the index signature[key: string]: unknown | string | booleanis redundant due tounknown.apps/api/src/modules/tool/dynamic-tooling/adapters/http-adapter.ts (1)
716-721: JSON handling for Buffer/ArrayBuffer is correct; consider guardingJSON.parsefor robustnessThe new separation between
BufferandArrayBufferand the use ofBuffer.from(new Uint8Array(response.data))is a solid fix for cross‑environment handling of binary JSON responses. Behavior on valid JSON looks good.To make this path more resilient (and consistent with
parseResponseDataabove), I recommend wrapping bothJSON.parsecalls here in a smalltry/catchand falling back to the rawresponse.dataon parse failure. That avoids unexpected runtime errors if a polling/result endpoint ever returns malformed JSON (or non‑JSON) while still advertisingapplication/json.For example:
- if (contentType.includes('application/json')) { - if (Buffer.isBuffer(response.data)) { - return JSON.parse(response.data.toString('utf-8')); - } - if (response.data instanceof ArrayBuffer) { - return JSON.parse(Buffer.from(new Uint8Array(response.data)).toString('utf-8')); - } - return response.data; - } + if (contentType.includes('application/json')) { + if (Buffer.isBuffer(response.data)) { + try { + return JSON.parse(response.data.toString('utf-8')); + } catch { + return response.data; + } + } + if (response.data instanceof ArrayBuffer) { + try { + const text = Buffer.from(new Uint8Array(response.data)).toString('utf-8'); + return JSON.parse(text); + } catch { + return response.data; + } + } + return response.data; + }This keeps the happy path identical while hardening the adapter against unexpected upstream responses.
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx (1)
642-667: Consider providing user feedback when OAuth is already in progress.When
isPollingorisOpeningis true, the function silently returns without any feedback. Consider adding a message to inform users that authorization is already in progress.// Handle unauth OAuth tools - open OAuth popup instead of inserting if ( item.isInstalled === false && (item.source === 'toolsets' || item.source === 'tools') && item.toolsetId ) { // Prevent multiple OAuth popups if (isPolling || isOpening) { + message.info(t('canvas.richChatInput.oauthInProgress')); return; } // Open OAuth popup for authorization - if (openOAuthPopup) { - openOAuthPopup(item.toolsetId); - } + openOAuthPopup?.(item.toolsetId); return; }packages/ai-workspace-common/src/requests/types.gen.ts (4)
655-716: CanvasTemplate.creditUsage shape looks reasonable; consider consistency with other credit fieldsAdding
creditUsage?: number | nullonCanvasTemplatealigns with the rest of the billing work and preserves backward compatibility via optional + nullable. The only thing to watch is consistency: elsewhere (e.g.,CreateShareRequest.creditUsage?: number) this isnumber | undefinedonly. If API semantics are the same (“unknown / unset” vs “0 credits”), it may be worth aligning these to the same union to avoid subtle differences in frontend handling, or at least documenting whennullvsundefinedoccurs in responses.
4711-4725: SandboxExecuteParams.language narrowing is fine; reuse the alias to avoid driftConfining
languageto'python' | 'javascript' | 'shell'is a good tightening for sandbox execution. You also introduced the exported alias:export type language = 'python' | 'javascript' | 'shell';To avoid future divergence, consider reusing this alias in
SandboxExecuteParams:- language: 'python' | 'javascript' | 'shell'; + language: language;This keeps the single source of truth if more languages are added later.
6677-6825: New post-handler, Composio, and tool-creation context types are coherentThe new types (
PostHandlerContext,PostHandlerResult,ComposioConnectedAccount,ComposioToolSchema,ComposioSchemaProperty,ToolCreationContext,authType) look internally consistent and match their intended roles:
PostHandlerContexthas all the fields required for billing and attribution; makinguser,toolName,toolsetName,toolsetKey, andcreditCostrequired is appropriate for post-processing.PostHandlerResultmirrors other *Result types and allows attaching files and metadata.- Composio types sensibly mirror a JSON-schema-like shape while keeping unknown extensions typed as
unknown.ToolCreationContextbundles everything needed to instantiate a DynamicStructuredTool.Minor nits you could consider later:
ToolCreationContext.authTypecould reuse the newauthTypealias instead of repeating the union.- If
creditCostmay sometimes be conceptually “not applicable”, you might wantcreditCost?: numberornumber | nullfor symmetry with other credit fields.Overall this is a clean extension of the public type surface.
7966-8063: SchemaProperty / JsonSchema extensions are a good fit for richer JSON Schema supportThe additions:
SchemaProperty.const?: unknown;SchemaProperty.oneOf?: Array<SchemaProperty>;SchemaProperty.anyOf?: Array<SchemaProperty>;JsonSchema.omitFields?: Array<string>;export type ResponseSchema = JsonSchema;ParsedMethodConfig.responseSchema: JsonSchema;nicely expand the expressiveness of your tool schemas (discriminators and unions) and add a lightweight way to strip fields from responses (
omitFields). Usingunknownhere is appropriate and keeps you away fromany.Given
ResponseSchemais now just an alias, if you plan to standardize onJsonSchemait might be worth adding a JSDoc@deprecatednote toResponseSchemain a follow-up so new code doesn’t target the alias.Also applies to: 8221-8221
apps/api/src/modules/credit/credit.dto.ts (1)
15-27: Tighten SyncToolCreditUsageJobData price fields to match service expectations
SyncToolCreditUsageJobDatamarksdiscountedPrice/originalPriceas optional, butCreditService.syncToolCreditUsagecurrently treatsdiscountedPriceas a required numeric value (compares it to 0 and passes it into credit deduction). To avoid mismatches between type and runtime assumptions, consider either:
- Making
discountedPrice(and likelyoriginalPrice) required here, or- Explicitly handling
undefined/missing values insyncToolCreditUsage(see comment incredit.service.ts).This will prevent callers from constructing job payloads that compile but lead to invalid billing behavior at runtime.
Also applies to: 36-56
packages/openapi-schema/src/types.gen.ts (1)
6677-6825: New post‑handler and Composio/tool creation types look consistent with downstream consumersThe additions for
PostHandlerContext/PostHandlerResult, Composio types, andToolCreationContext/authTypemirror the shapes used in the request/common packages and provide a clear contract for post‑processing and OAuth‑backed tools. Just make sure the OpenAPI generators for all packages stay in sync so these shared types do not drift over time.apps/api/src/modules/tool/billing/billing.service.ts (1)
18-33: Consider externalizing hardcoded constants.The
FREE_TOOL_LOOKUP_KEYSandFREE_TOOLSET_KEYSconstants are hardcoded. Consider moving these to configuration (e.g., environment variables or a config file) to allow easier updates without code changes.Example approach:
// In a config file or module export const BILLING_CONFIG = { freeLookupKeys: process.env.FREE_TOOL_LOOKUP_KEYS?.split(',') || ['refly_plus_yearly_stable_v3'], freeToolsetKeys: process.env.FREE_TOOLSET_KEYS?.split(',') || [ 'nano_banana_pro', // ... etc ], };apps/api/src/modules/tool/resource.service.ts (3)
564-568: Avoidanytype for object parameter.The
objparameter should have a more specific type to improve type safety and code clarity.+interface BufferObject { + buffer: Buffer; + mimetype?: string; +} + private async uploadObjectResource( user: User, canvasId: string, - obj: any, + obj: BufferObject | Record<string, unknown>, fileNameTitle?: string, ): Promise<DriveFile | null> { + const bufferObj = obj as BufferObject; - if (!obj.buffer || !Buffer.isBuffer(obj.buffer)) { + if (!bufferObj.buffer || !Buffer.isBuffer(bufferObj.buffer)) { return null; } - const mimeType = obj.mimetype; + const mimeType = bufferObj.mimetype;
586-599: Remove or standardize debug log statements.These
[DEBUG]prefixed log messages appear to be development artifacts. Either remove them or use a consistent logging approach (e.g.,this.logger.debug()).- this.logger.log(`[DEBUG] Uploaded to storage, storageKey: ${uploadResult.storageKey}`); + this.logger.debug(`Uploaded to storage, storageKey: ${uploadResult.storageKey}`); const driveFile = await this.driveService.createDriveFile(user, { // ... }); - this.logger.log( - `[DEBUG] Created DriveFile, fileId: ${driveFile.fileId}, size: ${driveFile.size}, type: ${driveFile.type}`, - ); + this.logger.debug( + `Created DriveFile, fileId: ${driveFile.fileId}, size: ${driveFile.size}, type: ${driveFile.type}`, + );
691-703: Avoidanytype in type assertion.Use
unknownwith type guards instead ofanyfor safer type handling.private async resolveFileIdToFormat(value: unknown, format: string): Promise<string | Buffer> { // Extract fileId from value - let fileId = typeof value === 'string' ? value : (value as any)?.fileId; + let fileId: string | undefined; + if (typeof value === 'string') { + fileId = value; + } else if (value && typeof value === 'object' && 'fileId' in value) { + fileId = (value as { fileId: string }).fileId; + } + if (!fileId) { throw new Error('Invalid resource value: missing fileId'); }apps/api/src/modules/tool/dynamic-tooling/core/handler-post.ts (1)
72-76: Use nullish coalescing (??) instead of logical OR (||) for fallback values.Per coding guidelines, prefer nullish coalescing for potentially undefined values. This ensures that empty strings (if valid) are not incorrectly replaced.
toolsetKey: - (request.metadata?.toolsetKey as string) || - (request.provider as string) || + (request.metadata?.toolsetKey as string) ?? + (request.provider as string) ?? 'unknown_toolset',packages/openapi-schema/schema.yml (4)
10735-10763: Add non-negative constraint and confirm credit units.Consider adding minimum: 0 for creditCost. Also confirm whether credits are fractional; elsewhere CreditUsage.amount is integer, so alignment avoids confusion.
creditCost: type: number description: Credit cost for this tool execution + minimum: 0
10784-10801: Stabilize Composio status surface.Add an enum (e.g., pending, active, revoked) to status for stronger contracts, or at least an example.
status: - type: string - description: Connection status + type: string + description: Connection status + enum: [pending, active, revoked] + example: active
12145-12150: omitFields is useful; consider example usage.Optional: add an examples block showing omission in a response to guide integrators.
12636-12643: HandlerResponse.data now supports array shape; verify clients.This is a breaking surface change for typed clients (union). Confirm UI/API consumers handle both object and array cases.
Add a short note in API docs/changelog about the union shape to aid adopters.
apps/api/src/modules/tool/composio/post-handler.service.ts (2)
44-77: Consider adding error information to the result.The error handling silently returns partial results when post-processing fails. While this prevents the entire tool execution from failing, users may not be aware that billing or file uploads did not complete successfully.
Consider either:
- Adding an
errororwarningsfield toPostHandlerResultto communicate partial failures- Re-throwing the error if post-processing is critical to the tool execution flow
Example:
return result; } catch (error) { this.logger.error( `Post-processing failed for ${context.toolName}: ${(error as Error).message}`, (error as Error).stack, ); + result.error = (error as Error).message; return result; }
151-210: Consider adding recursion depth limit.The
processObjectDatamethod recursively processes nested objects and arrays without a depth limit. Pathologically deep structures could cause stack overflow or performance issues.Consider adding a depth parameter to track recursion depth:
- private async processObjectData( + private async processObjectData( data: Record<string, unknown>, context: PostHandlerContext, + depth: number = 0, ): Promise<{ data: Record<string, unknown>; files: DriveFile[] }> { + const MAX_DEPTH = 10; + if (depth > MAX_DEPTH) { + this.logger.warn(`Max recursion depth reached for ${context.toolName}`); + return { data, files: [] }; + } // ... rest of the method - const nestedResult = await this.processObjectData( + const nestedResult = await this.processObjectData( value as Record<string, unknown>, context, + depth + 1, );apps/api/src/modules/tool/utils/schema-utils.ts (2)
106-161: Consider adding recursion depth limit and circular reference protection.The
fillDefaultValuesfunction recursively processes nested objects and arrays without protection against:
- Excessive recursion depth
- Circular references in the data structure
Consider adding a depth parameter and visited set for safety:
export function fillDefaultValues( params: Record<string, unknown>, schema: JsonSchema | SchemaProperty, depth: number = 0, visited: WeakSet<object> = new WeakSet(), ): Record<string, unknown> { const MAX_DEPTH = 10; if (depth > MAX_DEPTH) { return params; } if (visited.has(params)) { return params; // Circular reference detected } visited.add(params); // ... rest of the implementation }
235-267: Consider adding recursion protection.The function mutates the input object (by design, indicated by the
voidreturn type) but lacks protection against deep recursion or circular references.Consider adding a depth limit or visited tracking similar to the suggestion for
fillDefaultValues. Since this function is for internal utility use, the risk may be lower, but it's worth considering for robustness.packages/openapi-schema/src/schemas.gen.ts (1)
9411-9580: New post-handler and Composio context schemas look consistent; minor context/requirements nitsStructurally these schemas plug in cleanly with existing
User,DriveFile, andGenericToolsetTypeschemas, and the required fields sets are coherent. Two small points to consider:
ToolCreationContextSchema.descriptionmentionsUser/userIdcoming fromgetCurrentUser()but there is nouser-ish field in this schema. If user data is intentionally out-of-band, the description can be clarified to avoid confusion.connectedAccountIdis required even whenauthTypecan be'apikey'. If non-Composio / API-key-based tools do not always have a connected-account concept, you may want this field optional.If you want to clarify point (1) without changing behavior, you could update the description like this:
export const ToolCreationContextSchema = { type: 'object', - description: - 'Context for creating a DynamicStructuredTool. User/userId comes from getCurrentUser() at runtime.', + description: + 'Context for creating a DynamicStructuredTool. User information is resolved via getCurrentUser() at runtime and is not part of this schema definition.',apps/api/src/modules/tool/composio/composio.service.ts (5)
431-441: Empty catch block silently swallows JSON parse errors.Silent error handling makes debugging difficult. Consider logging at debug level to aid troubleshooting.
} catch { - // Invalid metadata, continue to create new auth config + this.logger.debug(`Invalid metadata JSON for ${integrationId}, will create new auth config`); }
470-476: Hardcoded integration IDs for API key field selection.Magic strings like
'alpha_vantage'and'hunter'violate coding guidelines and create maintenance burden. Consider storing the API key field name in the toolset inventory configuration.- const integrationIdLower = integrationId.toLowerCase(); - const useGenericApiKey = - integrationIdLower === 'alpha_vantage' || integrationIdLower === 'hunter'; - - const apiKeyConfig = useGenericApiKey ? { generic_api_key: apiKey } : { api_key: apiKey }; + // Retrieve apiKeyFieldName from inventory (defaults to 'api_key') + const inventory = await this.prisma.toolsetInventory.findUnique({ + where: { key: integrationId }, + select: { metadata: true }, + }); + const apiKeyFieldName = inventory?.metadata?.apiKeyFieldName ?? 'api_key'; + const apiKeyConfig = { [apiKeyFieldName]: apiKey };
492-492: Prefer nullish coalescing over logical OR for status fallback.Per coding guidelines, use
??instead of||when dealing with potentially undefined values to avoid unintended falsy coercion.- status: connectionRequest.status || 'active', + status: connectionRequest.status ?? 'active',
553-574: Consider usingunknownwith type guards instead ofany.Per coding guidelines, avoid
anytype when possible. While external schema data is dynamic, usingunknownwith runtime checks improves type safety.- private isToolValid(tool: { - function?: { name?: string; description?: string; parameters?: Record<string, any> }; + private isToolValid(tool: { + function?: { name?: string; description?: string; parameters?: Record<string, unknown> }; description?: string; }): boolean { const fn = tool?.function; if (!fn?.name) return false; // Skip deprecated tools const description = fn?.description ?? tool?.description ?? ''; if (/deprecated/i.test(description)) { return false; } // Skip tools with deprecated properties - const params = (fn.parameters ?? {}) as Record<string, any>; - const properties = params?.properties ?? {}; + const params = fn.parameters ?? {}; + const properties = (params as { properties?: Record<string, unknown> })?.properties ?? {}; const hasDeprecatedProps = Object.values(properties).some( - (prop: any) => prop?.deprecated === true, + (prop) => typeof prop === 'object' && prop !== null && (prop as { deprecated?: boolean }).deprecated === true, );
284-289: Type assertion onlabelDict.enmay fail silently.The
as stringassertion assumeslabelDict.enis always a string, but if the structure differs, the assignment could produce unexpected values.const inventoryItem = await this.inventoryService.getInventoryItem(appSlug); - const toolsetName = (inventoryItem?.definition?.labelDict?.en as string) ?? appSlug; + const labelEn = inventoryItem?.definition?.labelDict?.en; + const toolsetName = typeof labelEn === 'string' ? labelEn : appSlug;
| "handlebars": "^4.7.8", | ||
| "helmet": "^7.1.0", | ||
| "ioredis": "^5.6.1", | ||
| "json-schema-traverse": "^1.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest stable version json-schema-traverse npm package
💡 Result:
Latest stable version: 0.3.1. [1][2]
Sources:
[1] npm package page for json-schema-traverse v0.3.1.
[2] npm registry listing / package history for json-schema-traverse.
🏁 Script executed:
# Search for json-schema-traverse usage in the codebase
rg "json-schema-traverse" --type ts --type tsx --type js --type jsonRepository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Find schema-utils files mentioned in the review comment
fd "schema-utils" --type fRepository: refly-ai/refly
Length of output: 105
🏁 Script executed:
# Search for json-schema-traverse imports with corrected syntax
rg "json-schema-traverse" --type-list | head -20Repository: refly-ai/refly
Length of output: 447
🏁 Script executed:
# Search more broadly for json-schema-traverse
rg "json-schema-traverse" -A 3 -B 3Repository: refly-ai/refly
Length of output: 3302
🏁 Script executed:
# Check the schema-utils.ts file content
cat -n apps/api/src/modules/tool/utils/schema-utils.tsRepository: refly-ai/refly
Length of output: 14157
The json-schema-traverse library is actively used; however, verify the version specification.
The library is confirmed to be used in apps/api/src/modules/tool/utils/schema-utils.ts for schema traversal (line 13 import, line 324 in collectResourceFields function). However, the version specification ^1.0.0 requires clarification. Web search indicates that 0.3.1 is the latest stable version, whereas the package.json and pnpm-lock.yaml show version 1.0.0 is installed. Confirm whether 1.0.0 is intentional (e.g., a newer major version or pre-release) or if the version constraint should be adjusted to align with the latest stable release.
🤖 Prompt for AI Agents
In apps/api/package.json around line 104, the dependency "json-schema-traverse":
"^1.0.0" conflicts with upstream info showing 0.3.1 as the latest stable and the
library is imported in apps/api/src/modules/tool/utils/schema-utils.ts (import
at line 13, usage in collectResourceFields around line 324); confirm whether
1.0.0 was intentionally chosen (a newer major or pre-release) and if not, change
the package.json to the correct stable version (e.g., "0.3.1"), run pnpm install
to update pnpm-lock.yaml, run the test suite and lint to ensure the API/exports
are compatible, and if there are breaking API changes between versions adjust
the import/usage in schema-utils.ts accordingly or pin to the intended version
with a comment explaining the choice.
| async processBilling(options: ProcessBillingOptions): Promise<ProcessBillingResult> { | ||
| const { uid, toolName, toolsetKey, discountedPrice, originalPrice, billingConfig, params } = | ||
| options; | ||
|
|
||
| try { | ||
| // Determine credit cost | ||
| let finalDiscountedPrice = 1; | ||
| let finalOriginalPrice = 1; | ||
|
|
||
| if (discountedPrice !== undefined && discountedPrice > 0) { | ||
| // Direct credit cost provided (Composio scenario) | ||
| finalDiscountedPrice = discountedPrice; | ||
| finalOriginalPrice = originalPrice; | ||
| } else if (billingConfig?.enabled) { | ||
| // Calculate from billing config (dynamic-tooling scenario) | ||
| finalDiscountedPrice = calculateCredits(billingConfig, params || {}); | ||
| finalOriginalPrice = finalDiscountedPrice; // No discount in dynamic-tooling scenario | ||
| } |
There was a problem hiding this comment.
Clarify default credit cost and validate originalPrice.
- Lines 57-58: The default credit cost is set to 1 without explanation. Add a comment explaining why this default is chosen.
- Line 63:
originalPriceis assigned directly without checking if it's defined, which could result inundefinedbeing assigned tofinalOriginalPrice. Consider using nullish coalescing:finalOriginalPrice = originalPrice ?? discountedPrice;
Apply this diff:
// Determine credit cost
- let finalDiscountedPrice = 1;
- let finalOriginalPrice = 1;
+ // Default to 1 credit if no pricing is provided
+ let finalDiscountedPrice = 1;
+ let finalOriginalPrice = 1;
if (discountedPrice !== undefined && discountedPrice > 0) {
// Direct credit cost provided (Composio scenario)
finalDiscountedPrice = discountedPrice;
- finalOriginalPrice = originalPrice;
+ finalOriginalPrice = originalPrice ?? discountedPrice;
} else if (billingConfig?.enabled) {🤖 Prompt for AI Agents
apps/api/src/modules/tool/billing/billing.service.ts around lines 51 to 68: the
code sets default credit costs to 1 with no explanation and assigns
originalPrice directly which can yield undefined; add a short comment explaining
why default credit cost is 1 (e.g., safe minimal default or backwards
compatibility) and change the assignment to use a safe fallback so
finalOriginalPrice = originalPrice ?? discountedPrice (or another sensible
fallback) to avoid assigning undefined when originalPrice is missing.
| if (authType === 'oauth') { | ||
| const connectionStatus = await this.checkAppStatus(user, integrationId); | ||
| if (connectionStatus.status !== 'active') { | ||
| continue; | ||
| } | ||
| const params = (fn.parameters ?? {}) as Record<string, any>; | ||
| const properties = params?.properties ?? {}; | ||
| const deprecatedProps = Object.keys(properties) | ||
| .map((key) => (properties[key]?.deprecated ? key : null)) | ||
| .filter(Boolean); | ||
| return deprecatedProps.length === 0; | ||
| }) | ||
| .map((tool) => { | ||
| const fn = tool.function; | ||
| const toolName = fn?.name ?? 'unknown_tool'; | ||
| const toolSchema = JSONSchemaToZod.convert(fn.parameters ?? {}) as any; | ||
| return new DynamicStructuredTool({ | ||
| name: toolName, | ||
| description: fn?.description ?? `OAuth tool: ${toolName}`, | ||
| schema: toolSchema, | ||
| func: async (input, runManager, config: SkillRunnableConfig) => { | ||
| try { | ||
| const result = await this.executeTool( | ||
| user, | ||
| connectionStatus.connectedAccountId ?? '', | ||
| toolName, | ||
| input, | ||
| ); | ||
| if (result?.successful) { | ||
| // Add credit billing logic after successful execution | ||
| if (creditCost > 0) { | ||
| const resultId = config.configurable.resultId; | ||
| const version = config.configurable.version; | ||
| const jobData: SyncToolCreditUsageJobData = { | ||
| uid: user.uid, | ||
| creditCost: creditCost, | ||
| timestamp: new Date(), | ||
| toolCallId: runManager?.runId, | ||
| toolCallMeta: { | ||
| toolName: toolName, | ||
| toolsetId: toolset.id, | ||
| toolsetKey: toolset.toolset?.key, | ||
| }, | ||
| resultId, | ||
| version, | ||
| }; | ||
| await this.creditService.syncToolCreditUsage(jobData); | ||
| } | ||
| return JSON.stringify(result.data ?? null); | ||
| } | ||
| return JSON.stringify({ | ||
| error: result?.error ?? 'Unknown Composio execution error', | ||
| }); | ||
| } catch (error) { | ||
| this.logger.error( | ||
| `Failed to execute Composio tool ${toolName}: ${error instanceof Error ? error.message : error}`, | ||
| ); | ||
| return JSON.stringify({ | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
| }, | ||
| metadata: { | ||
| name: toolName, | ||
| type: toolset.type, | ||
| toolsetKey: toolset.toolset?.key, | ||
| toolsetName: toolset.name, | ||
| }, | ||
| }); | ||
| userId = user.uid; | ||
| connectedAccountId = connectionStatus.connectedAccountId ?? ''; | ||
| } else { | ||
| // API Key: use global refly_global connection (lazy loading) | ||
| connectedAccountId = await this.checkApiKeyStatus(integrationId); | ||
| userId = 'refly_global'; | ||
| } |
There was a problem hiding this comment.
Empty string fallback for connectedAccountId may cause API errors.
If connectionStatus.connectedAccountId is null/undefined, the code falls back to an empty string, which will be passed to executeTool. This could cause Composio API errors or undefined behavior.
Consider skipping the toolset when connectedAccountId is missing:
if (connectionStatus.status !== 'active') {
continue;
}
userId = user.uid;
- connectedAccountId = connectionStatus.connectedAccountId ?? '';
+ if (!connectionStatus.connectedAccountId) {
+ this.logger.warn(`No connectedAccountId for OAuth toolset ${integrationId}, skipping`);
+ continue;
+ }
+ connectedAccountId = connectionStatus.connectedAccountId;
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (authType === 'oauth') { | |
| const connectionStatus = await this.checkAppStatus(user, integrationId); | |
| if (connectionStatus.status !== 'active') { | |
| continue; | |
| } | |
| const params = (fn.parameters ?? {}) as Record<string, any>; | |
| const properties = params?.properties ?? {}; | |
| const deprecatedProps = Object.keys(properties) | |
| .map((key) => (properties[key]?.deprecated ? key : null)) | |
| .filter(Boolean); | |
| return deprecatedProps.length === 0; | |
| }) | |
| .map((tool) => { | |
| const fn = tool.function; | |
| const toolName = fn?.name ?? 'unknown_tool'; | |
| const toolSchema = JSONSchemaToZod.convert(fn.parameters ?? {}) as any; | |
| return new DynamicStructuredTool({ | |
| name: toolName, | |
| description: fn?.description ?? `OAuth tool: ${toolName}`, | |
| schema: toolSchema, | |
| func: async (input, runManager, config: SkillRunnableConfig) => { | |
| try { | |
| const result = await this.executeTool( | |
| user, | |
| connectionStatus.connectedAccountId ?? '', | |
| toolName, | |
| input, | |
| ); | |
| if (result?.successful) { | |
| // Add credit billing logic after successful execution | |
| if (creditCost > 0) { | |
| const resultId = config.configurable.resultId; | |
| const version = config.configurable.version; | |
| const jobData: SyncToolCreditUsageJobData = { | |
| uid: user.uid, | |
| creditCost: creditCost, | |
| timestamp: new Date(), | |
| toolCallId: runManager?.runId, | |
| toolCallMeta: { | |
| toolName: toolName, | |
| toolsetId: toolset.id, | |
| toolsetKey: toolset.toolset?.key, | |
| }, | |
| resultId, | |
| version, | |
| }; | |
| await this.creditService.syncToolCreditUsage(jobData); | |
| } | |
| return JSON.stringify(result.data ?? null); | |
| } | |
| return JSON.stringify({ | |
| error: result?.error ?? 'Unknown Composio execution error', | |
| }); | |
| } catch (error) { | |
| this.logger.error( | |
| `Failed to execute Composio tool ${toolName}: ${error instanceof Error ? error.message : error}`, | |
| ); | |
| return JSON.stringify({ | |
| error: error instanceof Error ? error.message : String(error), | |
| }); | |
| } | |
| }, | |
| metadata: { | |
| name: toolName, | |
| type: toolset.type, | |
| toolsetKey: toolset.toolset?.key, | |
| toolsetName: toolset.name, | |
| }, | |
| }); | |
| userId = user.uid; | |
| connectedAccountId = connectionStatus.connectedAccountId ?? ''; | |
| } else { | |
| // API Key: use global refly_global connection (lazy loading) | |
| connectedAccountId = await this.checkApiKeyStatus(integrationId); | |
| userId = 'refly_global'; | |
| } | |
| if (authType === 'oauth') { | |
| const connectionStatus = await this.checkAppStatus(user, integrationId); | |
| if (connectionStatus.status !== 'active') { | |
| continue; | |
| } | |
| userId = user.uid; | |
| if (!connectionStatus.connectedAccountId) { | |
| this.logger.warn(`No connectedAccountId for OAuth toolset ${integrationId}, skipping`); | |
| continue; | |
| } | |
| connectedAccountId = connectionStatus.connectedAccountId; | |
| } else { | |
| // API Key: use global refly_global connection (lazy loading) | |
| connectedAccountId = await this.checkApiKeyStatus(integrationId); | |
| userId = 'refly_global'; | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/composio/composio.service.ts around lines 353 to
364, the code currently falls back to an empty string when
connectionStatus.connectedAccountId is null/undefined which can be passed to
executeTool and cause API errors; change the logic to detect a missing
connectedAccountId and skip this toolset (e.g., continue the loop) or log and
return an error instead of assigning '' — ensure connectedAccountId is only set
when non-empty and add a clear guard that prevents calling executeTool with an
empty or missing connectedAccountId.
| // Capture user inside context before it's gone | ||
| const currentUser = getCurrentUser(); | ||
| // For OAuth tools, use current user's uid; for API Key tools, use 'refly_global' | ||
| const userId = context.authType === 'oauth' ? currentUser?.uid : 'refly_global'; | ||
| const executionResult = await this.executeTool( | ||
| userId, | ||
| context.connectedAccountId, | ||
| toolName, | ||
| toolInput, | ||
| ); | ||
| return { result: executionResult, user: currentUser }; |
There was a problem hiding this comment.
Potential undefined userId when currentUser is null for OAuth tools.
If context.authType === 'oauth' and getCurrentUser() returns null/undefined, userId will be undefined, causing the executeTool call to fail or behave unexpectedly.
Add validation for the current user:
const currentUser = getCurrentUser();
+ if (context.authType === 'oauth' && !currentUser?.uid) {
+ throw new Error('OAuth tool execution requires authenticated user context');
+ }
// For OAuth tools, use current user's uid; for API Key tools, use 'refly_global'
- const userId = context.authType === 'oauth' ? currentUser?.uid : 'refly_global';
+ const userId = context.authType === 'oauth' ? currentUser.uid : 'refly_global';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Capture user inside context before it's gone | |
| const currentUser = getCurrentUser(); | |
| // For OAuth tools, use current user's uid; for API Key tools, use 'refly_global' | |
| const userId = context.authType === 'oauth' ? currentUser?.uid : 'refly_global'; | |
| const executionResult = await this.executeTool( | |
| userId, | |
| context.connectedAccountId, | |
| toolName, | |
| toolInput, | |
| ); | |
| return { result: executionResult, user: currentUser }; | |
| // Capture user inside context before it's gone | |
| const currentUser = getCurrentUser(); | |
| if (context.authType === 'oauth' && !currentUser?.uid) { | |
| throw new Error('OAuth tool execution requires authenticated user context'); | |
| } | |
| // For OAuth tools, use current user's uid; for API Key tools, use 'refly_global' | |
| const userId = context.authType === 'oauth' ? currentUser.uid : 'refly_global'; | |
| const executionResult = await this.executeTool( | |
| userId, | |
| context.connectedAccountId, | |
| toolName, | |
| toolInput, | |
| ); | |
| return { result: executionResult, user: currentUser }; |
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/composio/composio.service.ts around lines 610 to
620, the code assigns userId based on currentUser but doesn't handle the case
where getCurrentUser() returns null for oauth authType; add a null-check after
capturing currentUser and before deriving userId: if context.authType ===
'oauth' and currentUser is null/undefined, throw a clear error (or return a
failed execution result) explaining that an authenticated user is required,
otherwise proceed to set userId = currentUser.uid; keep the existing
'refly_global' for non-oauth paths and ensure any thrown error is logged or
handled consistently with surrounding error handling.
| const fileNameTitle = (request?.params as Record<string, unknown>)?.file_name_title as | ||
| | string | ||
| | 'untitled'; |
There was a problem hiding this comment.
Incorrect type union syntax for fallback value.
The type annotation | 'untitled' is a union type, not a fallback assignment. If you intend to provide a fallback value, use ?? 'untitled'.
- const fileNameTitle = (request?.params as Record<string, unknown>)?.file_name_title as
- | string
- | 'untitled';
+ const fileNameTitle = ((request?.params as Record<string, unknown>)?.file_name_title as string)
+ ?? 'untitled';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fileNameTitle = (request?.params as Record<string, unknown>)?.file_name_title as | |
| | string | |
| | 'untitled'; | |
| const fileNameTitle = ((request?.params as Record<string, unknown>)?.file_name_title as string) | |
| ?? 'untitled'; |
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/resource.service.ts around lines 116 to 118, the
current code uses a union type `| 'untitled'` mistakenly as a fallback; change
it so the variable is typed as string (e.g., cast the param value to string or
type the variable as string | undefined) and apply a nullish coalescing fallback
when assigning the value — for example, extract the param into a string variable
and use `?? 'untitled'` to provide the default; ensure the params cast remains
correct and remove the incorrect union from the type annotation.
| export const useOAuthPopup = (options: UseOAuthPopupOptions = {}) => { | ||
| const { t } = useTranslation(); | ||
| const queryClient = useQueryClient(); | ||
| const { onSuccess, onError, pollingInterval = 2000, maxPollingAttempts = 60 } = options; | ||
|
|
||
| const [status, setStatus] = useState<OAuthPopupStatus>('idle'); | ||
| const [currentToolsetKey, setCurrentToolsetKey] = useState<string | null>(null); | ||
|
|
||
| const popupRef = useRef<Window | null>(null); | ||
| const pollingTimerRef = useRef<ReturnType<typeof setInterval> | null>(null); | ||
| const pollingAttemptsRef = useRef<number>(0); | ||
|
|
||
| const { mutateAsync: authorizeOAuth } = useComposioOAuthAuthorize(); | ||
|
|
||
| /** | ||
| * Clean up polling and popup | ||
| */ | ||
| const cleanup = useCallback(() => { | ||
| if (pollingTimerRef.current) { | ||
| clearInterval(pollingTimerRef.current); | ||
| pollingTimerRef.current = null; | ||
| } | ||
| pollingAttemptsRef.current = 0; | ||
| }, []); |
There was a problem hiding this comment.
Missing cleanup effect on unmount.
The hook sets up polling timers but doesn't clean them up when the component unmounts. If the component using this hook unmounts while polling is active, the interval will continue running and may cause memory leaks or state updates on unmounted components.
Add a cleanup effect:
export const useOAuthPopup = (options: UseOAuthPopupOptions = {}) => {
const { t } = useTranslation();
const queryClient = useQueryClient();
const { onSuccess, onError, pollingInterval = 2000, maxPollingAttempts = 60 } = options;
const [status, setStatus] = useState<OAuthPopupStatus>('idle');
const [currentToolsetKey, setCurrentToolsetKey] = useState<string | null>(null);
const popupRef = useRef<Window | null>(null);
const pollingTimerRef = useRef<ReturnType<typeof setInterval> | null>(null);
const pollingAttemptsRef = useRef<number>(0);
const { mutateAsync: authorizeOAuth } = useComposioOAuthAuthorize();
/**
* Clean up polling and popup
*/
const cleanup = useCallback(() => {
if (pollingTimerRef.current) {
clearInterval(pollingTimerRef.current);
pollingTimerRef.current = null;
}
pollingAttemptsRef.current = 0;
}, []);
+ // Cleanup on unmount
+ useEffect(() => {
+ return () => {
+ cleanup();
+ };
+ }, [cleanup]);🤖 Prompt for AI Agents
In packages/ai-workspace-common/src/hooks/use-oauth-popup.ts around lines 107 to
130, the hook starts polling timers but lacks a component unmount cleanup; add a
useEffect with an empty dependency array that calls the existing cleanup
function and also closes and nulls popupRef.current if a popup is open
(popupRef.current.close() and set to null) to ensure intervals are cleared and
no state updates occur after unmount; keep effect dependencies minimal (none) so
it only runs on unmount.
| pollingTimerRef.current = setInterval(async () => { | ||
| pollingAttemptsRef.current += 1; | ||
|
|
||
| // Check if popup is closed by user | ||
| if (isPopupClosed()) { | ||
| // Check one more time if auth succeeded before popup closed | ||
| const isAuthorized = await pollOAuthStatus(toolsetKey); | ||
| if (isAuthorized) { | ||
| cleanup(); | ||
| setStatus('success'); | ||
| clearOAuthCache(); // Clear cache on success | ||
| // Refetch queries to refresh tool list immediately (use predicate to match all queries starting with the key) | ||
| await queryClient.refetchQueries({ queryKey: composioOAuthStatusKeyFn(toolsetKey) }); | ||
| await queryClient.refetchQueries({ | ||
| predicate: (query) => | ||
| Array.isArray(query.queryKey) && query.queryKey[0] === useListUserToolsKey, | ||
| }); | ||
| onSuccess?.(toolsetKey); | ||
| } else { | ||
| cleanup(); | ||
| setStatus('cancelled'); | ||
| clearOAuthCache(); // Clear cache on cancel | ||
| // User closed popup without completing auth - this is OK, not an error | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // Check if max attempts reached | ||
| if (pollingAttemptsRef.current >= maxPollingAttempts) { | ||
| cleanup(); | ||
| clearOAuthCache(); // Clear cache on timeout | ||
| if (popupRef.current && !popupRef.current.closed) { | ||
| popupRef.current.close(); | ||
| } | ||
| setStatus('failed'); | ||
| const errorMsg = t('canvas.richChatInput.oauthTimeout'); | ||
| onError?.(errorMsg); | ||
| message.error(errorMsg); | ||
| return; | ||
| } | ||
|
|
||
| // Poll for status | ||
| const isAuthorized = await pollOAuthStatus(toolsetKey); | ||
| if (isAuthorized) { | ||
| cleanup(); | ||
| clearOAuthCache(); // Clear cache on success | ||
| if (popupRef.current && !popupRef.current.closed) { | ||
| popupRef.current.close(); | ||
| } | ||
| setStatus('success'); | ||
| // Refetch queries to refresh tool list immediately | ||
| await queryClient.refetchQueries({ queryKey: composioOAuthStatusKeyFn(toolsetKey) }); | ||
| await queryClient.refetchQueries({ | ||
| predicate: (query) => | ||
| Array.isArray(query.queryKey) && query.queryKey[0] === useListUserToolsKey, | ||
| }); | ||
| onSuccess?.(toolsetKey); | ||
| } | ||
| }, pollingInterval); |
There was a problem hiding this comment.
Potential overlapping async operations in polling interval.
The setInterval callback is async, but setInterval doesn't wait for the async operation to complete before scheduling the next iteration. If pollOAuthStatus or queryClient.refetchQueries takes longer than pollingInterval (2s), multiple polling operations could overlap.
Consider using a recursive setTimeout pattern instead:
const startPolling = useCallback(
(toolsetKey: string) => {
setStatus('polling');
pollingAttemptsRef.current = 0;
- pollingTimerRef.current = setInterval(async () => {
+ const poll = async () => {
pollingAttemptsRef.current += 1;
// Check if popup is closed by user
if (isPopupClosed()) {
// ... existing logic
+ return;
}
// Check if max attempts reached
if (pollingAttemptsRef.current >= maxPollingAttempts) {
// ... existing logic
+ return;
}
// Poll for status
const isAuthorized = await pollOAuthStatus(toolsetKey);
if (isAuthorized) {
// ... existing logic
+ return;
}
- }, pollingInterval);
+
+ // Schedule next poll
+ pollingTimerRef.current = setTimeout(poll, pollingInterval);
+ };
+
+ // Start first poll after interval
+ pollingTimerRef.current = setTimeout(poll, pollingInterval);
},Committable suggestion skipped: line range outside the PR's diff.
| type: object | ||
| description: Result from post-processing tool execution | ||
| required: | ||
| - data | ||
| properties: | ||
| data: | ||
| description: Processed data (may be uploaded to OSS) | ||
| files: | ||
| type: array | ||
| description: Files uploaded during post-processing | ||
| items: | ||
| $ref: '#/components/schemas/DriveFile' | ||
| creditCost: | ||
| type: number | ||
| description: Credit cost recorded | ||
| metadata: | ||
| type: object | ||
| additionalProperties: true | ||
| description: Metadata about processing | ||
| ComposioConnectedAccount: |
There was a problem hiding this comment.
Schema for PostHandlerResult.data is too loose.
data is untyped; define shape (object) or mirror HandlerResponse flexibility to aid clients and validation. Also add non-negative constraint to creditCost.
PostHandlerResult:
type: object
description: Result from post-processing tool execution
required:
- data
properties:
- data:
- description: Processed data (may be uploaded to OSS)
+ data:
+ description: Processed data (may be uploaded to OSS)
+ oneOf:
+ - type: object
+ additionalProperties: true
+ - type: array
+ items:
+ type: object
+ additionalProperties: true
files:
type: array
description: Files uploaded during post-processing
items:
$ref: '#/components/schemas/DriveFile'
creditCost:
type: number
description: Credit cost recorded
+ minimum: 0
metadata:
type: object
additionalProperties: true
description: Metadata about processing🤖 Prompt for AI Agents
In packages/openapi-schema/schema.yml around lines 10764 to 10783, the
PostHandlerResult schema leaves the data property untyped and creditCost
unconstrained; change data from an untyped property to a defined schema (at
minimum set type: object with additionalProperties: true, or better, reference
the same structure used by HandlerResponse to preserve its flexibility/shape),
and add a minimum: 0 constraint to creditCost so it cannot be negative; ensure
files and metadata remain unchanged and update required/description accordingly.
| type: object | ||
| description: Context for creating a DynamicStructuredTool. User/userId comes from getCurrentUser() at runtime. | ||
| required: | ||
| - connectedAccountId | ||
| - authType | ||
| - creditCost | ||
| - toolsetType | ||
| - toolsetKey | ||
| - toolsetName | ||
| properties: | ||
| connectedAccountId: | ||
| type: string | ||
| description: Connected account ID from Composio | ||
| authType: | ||
| type: string | ||
| enum: | ||
| - oauth | ||
| - apikey | ||
| description: Authentication type | ||
| creditCost: | ||
| type: number | ||
| description: Credit cost for tool execution | ||
| toolsetType: | ||
| $ref: '#/components/schemas/GenericToolsetType' | ||
| description: Toolset type identifier | ||
| toolsetKey: | ||
| type: string | ||
| description: Toolset key | ||
| toolsetName: | ||
| type: string | ||
| description: Toolset display name | ||
| GenericToolsetType: |
There was a problem hiding this comment.
Unify authType with existing ToolsetAuthType to avoid enum drift.
Use the shared ToolsetAuthType (credentials, oauth, config_based) or at least rename apikey → credentials for consistency with AuthPattern/ToolsetAuthType.
- authType:
- type: string
- enum:
- - oauth
- - apikey
- description: Authentication type
+ authType:
+ $ref: '#/components/schemas/ToolsetAuthType'
+ description: Authentication type (reuses global enum)Alternatively:
- enum:
- - oauth
- - apikey
+ enum:
+ - oauth
+ - credentials📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type: object | |
| description: Context for creating a DynamicStructuredTool. User/userId comes from getCurrentUser() at runtime. | |
| required: | |
| - connectedAccountId | |
| - authType | |
| - creditCost | |
| - toolsetType | |
| - toolsetKey | |
| - toolsetName | |
| properties: | |
| connectedAccountId: | |
| type: string | |
| description: Connected account ID from Composio | |
| authType: | |
| type: string | |
| enum: | |
| - oauth | |
| - apikey | |
| description: Authentication type | |
| creditCost: | |
| type: number | |
| description: Credit cost for tool execution | |
| toolsetType: | |
| $ref: '#/components/schemas/GenericToolsetType' | |
| description: Toolset type identifier | |
| toolsetKey: | |
| type: string | |
| description: Toolset key | |
| toolsetName: | |
| type: string | |
| description: Toolset display name | |
| GenericToolsetType: | |
| type: object | |
| description: Context for creating a DynamicStructuredTool. User/userId comes from getCurrentUser() at runtime. | |
| required: | |
| - connectedAccountId | |
| - authType | |
| - creditCost | |
| - toolsetType | |
| - toolsetKey | |
| - toolsetName | |
| properties: | |
| connectedAccountId: | |
| type: string | |
| description: Connected account ID from Composio | |
| authType: | |
| $ref: '#/components/schemas/ToolsetAuthType' | |
| description: Authentication type (reuses global enum) | |
| creditCost: | |
| type: number | |
| description: Credit cost for tool execution | |
| toolsetType: | |
| $ref: '#/components/schemas/GenericToolsetType' | |
| description: Toolset type identifier | |
| toolsetKey: | |
| type: string | |
| description: Toolset key | |
| toolsetName: | |
| type: string | |
| description: Toolset display name | |
| GenericToolsetType: |
🤖 Prompt for AI Agents
In packages/openapi-schema/schema.yml around lines 10834 to 10865, the authType
enum in this schema is inconsistent with the shared ToolsetAuthType and may
drift; replace the local authType definition with a reference to the existing
ToolsetAuthType component (or, if keeping local, rename/align values to match
ToolsetAuthType by changing apikey → credentials and including the full set:
credentials, oauth, config_based) so the spec uses the canonical auth enum
across schemas.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx (1)
672-697: WrapselectItemwithuseCallbackto maintain referential equality.The
selectItemfunction is used as an event handler (line 576) and should be wrapped withuseCallbackto prevent unnecessary re-renders and maintain referential equality.Based on learnings, function props should always use useCallback in React.
Apply this refactor:
- const selectItem = (item: MentionItem) => { + const selectItem = useCallback((item: MentionItem) => { // Handle create variable case if (item.variableId === 'create-variable') { handleAddVariable(); return; } // Handle unauth OAuth tools - open OAuth popup instead of inserting if ( item.isInstalled === false && (item.source === 'toolsets' || item.source === 'tools') && item.toolsetId ) { // Prevent multiple OAuth popups if (isPolling || isOpening) { return; } // Open OAuth popup for authorization if (openOAuthPopup) { openOAuthPopup(item.toolsetId); } return; } command(item); - }; + }, [handleAddVariable, isPolling, isOpening, openOAuthPopup, command]);packages/ai-workspace-common/src/requests/types.gen.ts (1)
8577-8591: Fix handler-post.ts to handle array responses: extractFileIdToTopLevel does not support the new array case.The
HandlerResponse.datatype now accepts either an object or an array of objects. However,extractFileIdToTopLevel()in handler-post.ts (line 124) uses an insufficient type guard (typeof response.data !== 'object') that passes for arrays. Line 184 then attempts to spreadresponse.dataas an object, which corrupts array data by converting it to numeric keys.Add an array check and handle both cases:
- If
response.datais an array, either skip extraction or map over array items individually- If it's an object, proceed with current logic
The
mapResourceFields()in resource.service.ts already demonstrates proper array handling withArray.isArray()checks—apply the same pattern here.packages/request/src/requests/types.gen.ts (1)
8577-8665:extractFileIdToTopLevelrequires update to handle array dataThe change allowing
HandlerResponse.datato be either an object or array introduces a potential issue inextractFileIdToTopLevel(packages/api/src/modules/tool/dynamic-tooling/core/handler-post.ts). The function checkstypeof response.data !== 'object'but doesn't handle the case wheredatais an array of objects at the top level—it assumes a single object structure when extractingfileIdandfilesproperties. ThepersistOutputResourcesfunction already handles both forms correctly. Verify that handlers returning array data work as intended with the file extraction logic, and updateextractFileIdToTopLevelif needed.
♻️ Duplicate comments (3)
packages/openapi-schema/schema.yml (2)
10879-10889: Type PostHandlerResult.data and bound creditCost ≥ 0.data is untyped; mirror HandlerResponse flexibility. Also add minimum: 0 for creditCost. This matches prior feedback.
properties: - data: - description: Processed data (may be uploaded to OSS) + data: + description: Processed data (may be uploaded to OSS) + oneOf: + - type: object + additionalProperties: true + - type: array + items: + type: object + additionalProperties: true files: @@ creditCost: type: number description: Credit cost recorded + minimum: 0
10958-10963: Unify authType with ToolsetAuthType to avoid enum drift.Reuse the canonical enum instead of a local oauth/apikey list (or rename apikey → credentials and include config_based).
Option A (preferred):
- authType: - type: string - enum: - - oauth - - apikey - description: Authentication type + authType: + $ref: '#/components/schemas/ToolsetAuthType' + description: Authentication type (reuses global enum)Option B:
- enum: - - oauth - - apikey + enum: + - oauth + - credentials + - config_basedpackages/request/src/requests/types.gen.ts (1)
6714-6857: PostHandlerContext still missingtoolsetKey, misaligned with other packages
PostHandlerContextexposestoolNameandtoolsetNamebut no distincttoolsetKey. Other packages (e.g., ai‑workspace‑common/openapi‑schema) use bothtoolsetName(display) andtoolsetKey(stable identifier); keeping onlytoolsetNamehere makes it hard for post‑handlers to reliably identify the toolset when names change or collide. Please add atoolsetKey: stringfield in the source schema and regenerate this file so all packages agree, rather than editing this generated file directly. The surrounding Composio‑/ToolCreation‑related types look consistent otherwise.
🧹 Nitpick comments (7)
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx (1)
662-662: Remove unusedcurrentLanguagefrom dependency array.The
currentLanguagevariable is included in the dependency array but is not used within therenderListItemfunction body. This causes unnecessary re-renders when the language changes.Apply this diff:
- [categoryConfigs, query, formatVariableValue, currentLanguage, t], + [categoryConfigs, query, formatVariableValue, t],packages/ai-workspace-common/src/requests/types.gen.ts (2)
6714-6766: Consider validation for creditCost values.The
creditCostfield in bothPostHandlerContext(line 6737) andPostHandlerResult(line 6759) is typed asnumber, which allows negative values. If credit costs should always be non-negative, consider adding runtime validation or using a branded type to ensure positive values.export type PostHandlerContext = { // ... other fields /** * Credit cost for this tool execution */ - creditCost: number; + creditCost: number; // Consider: positive number validation at runtime
6788-6826: Index signatures in Composio schema types could be more precise.Lines 6806 and 6825 use index signatures with union types that include the property types themselves:
[key: string]: unknown | string | ComposioSchemaProperty;(line 6806)[key: string]: unknown | string | boolean;(line 6825)While this reflects the flexible nature of Composio's JSON schema, it reduces type safety. TypeScript will allow any of these types for additional properties, making it harder to catch errors.
Note: Since this is auto-generated from OpenAPI schema, this may accurately reflect the upstream API design.
If the Composio API actually has stricter constraints on additional properties, consider refining the schema definition in the source OpenAPI specification.
packages/openapi-schema/src/types.gen.ts (1)
6714-6862: Post-handler, Composio, and tool-creation context types look consistent; a couple of small cleanups are possible.
PostHandlerContext/PostHandlerResultand the Composio-related types integrate cleanly with existingUser,DriveFile, and toolset types; nothing blocking from a type-correctness standpoint. Two minor nits:
ToolCreationContext.authTypein Line 6839 duplicates the union also exported asauthTypein Line 6861. Consider reusing the alias in the field or dropping the alias if it's not used externally to avoid divergence later.- The index signatures in
ComposioToolSchemaandComposioSchemaPropertyeffectively collapse tounknownbecause of theunknown | ...union; you can simplify them to[key: string]: unknownfor clarity without losing flexibility.packages/request/src/requests/types.gen.ts (1)
8007-8104: JSON schema extensions (const/oneOf/anyOf+omitFields) look good but require traversal supportExtending
SchemaPropertywithconst,oneOf, andanyOfand simplifyingJsonSchema/ResponseSchemato a singleJsonSchematype with optionalomitFieldsis coherent, but it assumes all schema‑processing code (parsers, validators, and response traversals) understands these new fields and no longer relies on the removedadditionalProperties. Please double‑check that the new schema traversal utilities in this PR handleoneOf/anyOfcomposition semantics and respectomitFieldswhen shaping responses.apps/api/src/modules/tool/tool.service.ts (1)
972-988: ConfirmcomposioService.instantiateToolsetscorrectly filters OAuth toolsets
instantiateToolsetsnow delegates OAuth tool instantiation tocomposioService.instantiateToolsets(user, toolsets, 'oauth')using the fulltoolsetsarray. This is fine as long as the Composio service internally filters for the relevant OAuth/external‑OAuth toolsets and ignores others; otherwise you could end up double‑instantiating regular/MCP toolsets or doing unnecessary work.Please verify that
ComposioService.instantiateToolsets:
- Only considers OAuth‑style toolsets when mode is
'oauth', and- Safely ignores non‑matching entries.
packages/openapi-schema/src/schemas.gen.ts (1)
9479-9536: PostHandlerContext/Result schemas are coherent with the billing modelThe context/result shapes look consistent (user + tool + creditCost in context; data/files/creditCost/metadata in result). One small improvement: if
creditCostinPostHandlerResultSchemais always computed and persisted, consider marking it as required to keep the schema in sync with your invariants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/api/src/modules/tool/tool.service.ts(7 hunks)packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx(3 hunks)packages/ai-workspace-common/src/requests/types.gen.ts(8 hunks)packages/i18n/src/en-US/ui.ts(1 hunks)packages/i18n/src/zh-Hans/ui.ts(1 hunks)packages/openapi-schema/schema.yml(5 hunks)packages/openapi-schema/src/schemas.gen.ts(8 hunks)packages/openapi-schema/src/types.gen.ts(8 hunks)packages/request/src/requests/types.gen.ts(8 hunks)packages/skill-template/src/skills/agent.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/src/zh-Hans/ui.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/skill-template/src/skills/agent.ts
- packages/i18n/src/en-US/ui.ts
🧰 Additional context used
📓 Path-based instructions (20)
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{jsx,tsx}: Always use tailwind css to style the component
Always wrap pure components with React.memo to prevent unnecessary re-renders
Always use useMemo for expensive computations or complex object creation
Always use useCallback for function props to maintain referential equality
Always specify proper dependency arrays in useEffect to prevent infinite loops
Always avoid inline object/array creation in render to prevent unnecessary re-renders
Always use proper key props when rendering lists
Always split nested components with closures into separate components to avoid performance issues and improve code maintainability
**/*.{jsx,tsx}: Always wrap pure components with React.memo to prevent unnecessary re-renders
Always use useMemo for expensive computations or complex object creation in React
Always use useCallback for function props to maintain referential equality in React
Always specify proper dependency arrays in useEffect to prevent infinite loops in React
Always avoid inline object/array creation in render to prevent unnecessary re-renders in React
Always use proper key props when rendering lists in React (avoid using index when possible)
Always split nested components with closures into separate components in React
Use lazy loading for components that are not immediately needed in React
Debounce handlers for events that might fire rapidly (resize, scroll, input) in React
Implement fallback UI for components that might fail in React
Use error boundaries to catch and handle runtime errors in React
**/*.{jsx,tsx}: Place each attribute on a new line when a component has multiple attributes in JSX
Use self-closing tags for elements without children in JSX
Keep JSX expressions simple, extract complex logic to variables
Put closing brackets for multi-line JSX on a new line
**/*.{jsx,tsx}: Component file names should match the component name
Organize function components in order: imports, type definitions, constants, component function, hook calls, e...
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{jsx,tsx,css}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{jsx,tsx,css}: Use Tailwind CSS for styling components
Follow the utility-first approach with Tailwind CSS
Group related utility classes together in Tailwind CSS
Prefer Tailwind utilities over custom CSS when possible
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.tspackages/openapi-schema/schema.yml
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.tspackages/openapi-schema/schema.yml
**/*.{jsx,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Each component file should contain only one main component
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Explicitly type props with interfaces or types in React components
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsxpackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool/tool.service.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/request/src/requests/types.gen.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
Use React best practices for frontend code
Files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
apps/api/src/**/*.{controller,service}.ts
📄 CodeRabbit inference engine (.cursor/rules/06-api-structure.mdc)
Implement proper error handling in API modules
Files:
apps/api/src/modules/tool/tool.service.ts
🧠 Learnings (8)
📚 Learning: 2025-11-25T03:03:31.945Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/01-code-style.mdc:0-0
Timestamp: 2025-11-25T03:03:31.945Z
Learning: Applies to **/*.{jsx,tsx} : Always use useCallback for function props to maintain referential equality in React
Applied to files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
📚 Learning: 2025-11-25T03:03:31.945Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/01-code-style.mdc:0-0
Timestamp: 2025-11-25T03:03:31.945Z
Learning: Applies to **/*.{jsx,tsx} : Always use useMemo for expensive computations or complex object creation in React
Applied to files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
📚 Learning: 2025-11-25T03:02:45.779Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T03:02:45.779Z
Learning: Applies to **/*.{jsx,tsx} : Always use useCallback for function props to maintain referential equality
Applied to files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
📚 Learning: 2025-11-25T03:02:45.779Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T03:02:45.779Z
Learning: Applies to **/*.{jsx,tsx} : Always use useMemo for expensive computations or complex object creation
Applied to files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
📚 Learning: 2025-11-25T03:04:41.334Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-11-25T03:04:41.334Z
Learning: Applies to **/*.{tsx,ts} : Use the translation wrapper component and useTranslation hook in components
Applied to files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
📚 Learning: 2025-11-25T03:03:31.945Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/01-code-style.mdc:0-0
Timestamp: 2025-11-25T03:03:31.945Z
Learning: Applies to **/*.{jsx,tsx} : Always specify proper dependency arrays in useEffect to prevent infinite loops in React
Applied to files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
📚 Learning: 2025-11-25T03:04:05.715Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/05-code-organization.mdc:0-0
Timestamp: 2025-11-25T03:04:05.715Z
Learning: Applies to **/*.{jsx,tsx} : Use React state hooks for component internal state
Applied to files:
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
📚 Learning: 2025-11-25T03:04:12.836Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/06-api-structure.mdc:0-0
Timestamp: 2025-11-25T03:04:12.836Z
Learning: Applies to apps/api/src/{auth,user,project,canvas,rag,knowledge,search,skill,share,code-artifact}/**/*.{controller,service}.ts : Use dependency injection for module dependencies in NestJS
Applied to files:
apps/api/src/modules/tool/tool.service.ts
🧬 Code graph analysis (1)
packages/ai-workspace-common/src/requests/types.gen.ts (1)
packages/request/src/requests/types.gen.ts (2)
User(474-483)DriveFile(7514-7579)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build
🔇 Additional comments (16)
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx (1)
679-694: OAuth handling logic looks good.The implementation correctly intercepts uninstalled tool selections and triggers OAuth authentication instead of direct insertion. The
isPollingandisOpeningguards prevent duplicate popup attempts.packages/ai-workspace-common/src/requests/types.gen.ts (3)
8025-8036: LGTM: JSON Schema discriminator support added.The additions of
const,oneOf, andanyOffields toSchemaPropertyproperly support JSON Schema discriminator patterns and polymorphic type definitions. These are standard JSON Schema features and the implementation is correct.
8266-8266: Consistent with ResponseSchema redefinition.The
responseSchemafield types inParsedMethodConfig(line 8266),HandlerContext(line 8646), andHandlerConfig(line 8668) have been updated to useJsonSchemainstead ofResponseSchema. This is consistent withResponseSchemabecoming an alias ofJsonSchema(line 8107).These changes maintain type consistency across the codebase.
Also applies to: 8646-8646, 8668-8668
8081-8107: No breaking changes detected with ResponseSchema redefinition.ResponseSchema is not imported or used in any application code—it exists only as a type definition in auto-generated files. Redefining it as an explicit alias to JsonSchema is a documentation clarification with no impact on consumers. The new
omitFieldsoptional field is backward compatible and is already properly implemented in the codebase (e.g.,apps/api/src/modules/tool/resource.service.tshandles field removal correctly).packages/openapi-schema/src/types.gen.ts (5)
8011-8076: ExtendedSchemaProperty(const/oneOf/anyOf) is reasonable and matches common JSON Schema patterns.Adding
const,oneOf, andanyOfwith recursiveSchemaPropertyreferences is structurally sound and should support discriminator-style and compositional schemas without affecting existing callers.
8078-8107:JsonSchema/ResponseSchematighten the contract; confirm that all usages expect an object root.Constraining
JsonSchema.typeto'object', requiring apropertiesmap, and addingomitFields?: string[](withResponseSchemanow just an alias) makes the schema model more precise and simpler. This looks good as long as all tool schemas you parse are indeed object-rooted; if any existing configs use non-objecttop-level types, they'll now be rejected at the type level.
8244-8296:ParsedMethodConfig.responseSchemausingJsonSchemakeeps request/response typing symmetric.Switching
responseSchematoJsonSchemaaligns it with theschemafield and the newResponseSchemaalias. This is a straightforward, internally consistent change and should be source-compatible for code that previously referencedResponseSchema.
8577-8630:HandlerResponse.datanow allows arrays; ensure all consumers handle both shapes.Typing
dataas either a single object orArray<object>is a sensible generalization for multi-item tool results, but it is a breaking type change for any code that assumeddatawas always an object. Please double-check downstream handlers/consumers to branch onArray.isArray(response.data)or normalize to one shape.
8632-8681:HandlerContext/HandlerConfigswitching toJsonSchemaforresponseSchemais consistent with the new schema model.Using
JsonSchemahere keeps everything in the handler pipeline on the same, stricter schema type and matchesParsedMethodConfig. No issues from a typing perspective; just confirm the runtime code that populates these fields is updated to supplyomitFieldswhen needed.packages/openapi-schema/schema.yml (3)
12240-12266: Nice addition: JsonSchema description + omitFields.
12268-12270: Good: ResponseSchema now aliases JsonSchema.
12752-12759: HandlerResponse.data shape aligned (object|array).This improves client typing and validation.
apps/api/src/modules/tool/tool.service.ts (2)
1-3: Import cleanup from@nestjs/commonlooks correctSwitching the Nest imports to just
InjectableandLoggeris consistent with the current constructor (no optional/queue injection left), so this change is safe.
1045-1182: Verify ComposioService contract assumptions and clarify external API key toolset typeThe refactor successfully separates static, config-based, and external API key toolsets. Two items need clarity:
Minimal mapped shape is safe
The mapped objects forexternalApiKeyToolsetscontain only{ id, name, type: 'regular', toolset: { toolsetId, name, key } }. This works becauseComposioService.instantiateToolsetsonly accessestoolset.toolset?.key,toolset.type, andtoolset.name—all present in the mapped shape. Optional fields likebuiltin,uninstalled, andselectedToolsare not accessed.Type assignment needs clarification
The code assignstype: 'regular'to external API key toolsets, butGenericToolsetTypeonly defines'regular' | 'mcp' | 'external_oauth'. No type variant exists for'external_apikey'(theauthTypein the database). This works at runtime due to type narrowing but creates semantic confusion. Consider whetherGenericToolsetTypeshould be extended with an'external_apikey'variant or if the current assignment is intentional for Composio compatibility.Credit payload is correct
TheSyncToolCreditUsageJobDatastructure matches the DTO definition and downstream billing logic. BothoriginalPriceanddiscountedPriceare set tocreditCost, which is appropriate for static/global tools where no discount logic applies.packages/openapi-schema/src/schemas.gen.ts (2)
12019-12048: HandlerContext/HandlerConfig responseSchema alignment with JsonSchema is consistentPointing
HandlerContextSchema.responseSchemaandHandlerConfigSchema.responseSchematoJsonSchemakeeps everything aligned with the new central schema model and matches the changes toResponseSchemaSchemaandParsedMethodConfigSchema. No issues spotted here.
11261-11557: ResponseSchema simplification is safe; no consumers expect old layout
- The type alias
ResponseSchema = JsonSchemacorrectly reflects actual runtime usage across all packages.ToolMethodConfigSchema.responseSchemaremains a string (JSON-serialized), andParsedMethodConfigSchema.responseSchemacorrectly referencesJsonSchema(parsed object). The parsing flow viaparseJsonSchema()infactory.service.tsis well-established and validates against the new shape.validateJsonSchema()enforces the expected structure (object type with properties), which aligns with the newJsonSchemaSchemadefinition.- No code references the old
ResponseSchemalayout orResponseSchemaSchemaexport, and all consumers (handler.ts,handler-post.ts,inventory.service.ts) properly treat responseSchema as aJsonSchemaobject.
| export const MentionList: React.FC<{ | ||
| items: MentionItem[]; | ||
| command: any; | ||
| placement?: 'top' | 'bottom'; | ||
| isMentionListVisible?: boolean; | ||
| query?: string; | ||
| // OAuth props passed from parent | ||
| openOAuthPopup?: (toolsetKey: string) => Promise<any>; | ||
| isPolling?: boolean; | ||
| isOpening?: boolean; | ||
| }> = ({ | ||
| items, | ||
| command, | ||
| placement = 'bottom', | ||
| isMentionListVisible, | ||
| query = '', | ||
| openOAuthPopup, | ||
| isPolling = false, | ||
| isOpening = false, | ||
| }) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap component with React.memo to prevent unnecessary re-renders.
As per coding guidelines, pure components should always be wrapped with React.memo. This component would benefit from memoization given its complexity and the number of props it receives.
Based on learnings, always wrap pure components with React.memo to prevent unnecessary re-renders.
-export const MentionList: React.FC<{
+export const MentionList = React.memo<{
items: MentionItem[];
command: any;
placement?: 'top' | 'bottom';
isMentionListVisible?: boolean;
query?: string;
// OAuth props passed from parent
openOAuthPopup?: (toolsetKey: string) => Promise<any>;
isPolling?: boolean;
isOpening?: boolean;
-}> = ({
+}>(({
items,
command,
placement = 'bottom',
isMentionListVisible,
query = '',
openOAuthPopup,
isPolling = false,
isOpening = false,
}) => {
// ... component body ...
-};
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const MentionList: React.FC<{ | |
| items: MentionItem[]; | |
| command: any; | |
| placement?: 'top' | 'bottom'; | |
| isMentionListVisible?: boolean; | |
| query?: string; | |
| // OAuth props passed from parent | |
| openOAuthPopup?: (toolsetKey: string) => Promise<any>; | |
| isPolling?: boolean; | |
| isOpening?: boolean; | |
| }> = ({ | |
| items, | |
| command, | |
| placement = 'bottom', | |
| isMentionListVisible, | |
| query = '', | |
| openOAuthPopup, | |
| isPolling = false, | |
| isOpening = false, | |
| }) => { | |
| export const MentionList = React.memo<{ | |
| items: MentionItem[]; | |
| command: any; | |
| placement?: 'top' | 'bottom'; | |
| isMentionListVisible?: boolean; | |
| query?: string; | |
| // OAuth props passed from parent | |
| openOAuthPopup?: (toolsetKey: string) => Promise<any>; | |
| isPolling?: boolean; | |
| isOpening?: boolean; | |
| }>(({ | |
| items, | |
| command, | |
| placement = 'bottom', | |
| isMentionListVisible, | |
| query = '', | |
| openOAuthPopup, | |
| isPolling = false, | |
| isOpening = false, | |
| }) => { |
🤖 Prompt for AI Agents
In
packages/ai-workspace-common/src/components/canvas/launchpad/rich-chat-input/mentionList.tsx
around lines 49 to 68, the functional component is not memoized; wrap the
component export with React.memo to avoid unnecessary re-renders (e.g., replace
the direct export with a memoized export using React.memo(MentionList) or assign
a const = React.memo(...) and export that). Ensure React is imported so
React.memo is available and set a displayName on the memoized component if
helpful for debugging; keep prop types the same so memo can shallow-compare
props.
| isMentionListVisible?: boolean; | ||
| query?: string; | ||
| // OAuth props passed from parent | ||
| openOAuthPopup?: (toolsetKey: string) => Promise<any>; |
There was a problem hiding this comment.
Avoid using any type for the Promise return value.
The openOAuthPopup prop uses Promise<any> as its return type. As per coding guidelines, avoid using any type whenever possible.
Apply this diff to use a more specific type:
- openOAuthPopup?: (toolsetKey: string) => Promise<any>;
+ openOAuthPopup?: (toolsetKey: string) => Promise<void>;Or if the OAuth popup returns specific data, define a proper type for it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| openOAuthPopup?: (toolsetKey: string) => Promise<any>; | |
| openOAuthPopup?: (toolsetKey: string) => Promise<void>; |
| creditCost: | ||
| type: number | ||
| description: Credit cost for this tool execution | ||
| fileNameTitle: |
There was a problem hiding this comment.
Constrain creditCost to non‑negative.
Add minimum: 0 to prevent negative credits in PostHandlerContext.
creditCost:
type: number
description: Credit cost for this tool execution
+ minimum: 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| creditCost: | |
| type: number | |
| description: Credit cost for this tool execution | |
| fileNameTitle: | |
| creditCost: | |
| type: number | |
| description: Credit cost for this tool execution | |
| minimum: 0 | |
| fileNameTitle: |
🤖 Prompt for AI Agents
In packages/openapi-schema/schema.yml around lines 10867 to 10870, the
creditCost property allows negative values; update the creditCost schema to
enforce non‑negative values by adding "minimum: 0" under the creditCost
definition (keep type: number and description intact) so the schema rejects
negative credit costs used by PostHandlerContext.
| const: | ||
| description: Constant value for discriminator matching in oneOf/anyOf | ||
| oneOf: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/SchemaProperty' | ||
| description: One of the listed schemas must match | ||
| anyOf: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/SchemaProperty' | ||
| description: Any of the listed schemas can match |
There was a problem hiding this comment.
SchemaProperty.const lacks a type.
Define allowed types for const to aid validation (string/number/integer/boolean).
const:
- description: Constant value for discriminator matching in oneOf/anyOf
+ description: Constant value for discriminator matching in oneOf/anyOf
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean🤖 Prompt for AI Agents
In packages/openapi-schema/schema.yml around lines 12192 to 12203, the
SchemaProperty.const entry is missing a type, so add a type declaration limiting
allowed primitive types to string, number, integer, or boolean (e.g., add type:
[string, number, integer, boolean] under const) so validators know which
primitive const values are permitted; keep the change local to the const block
and preserve the existing description.
| export const ComposioConnectedAccountSchema = { | ||
| type: 'object', | ||
| description: 'Composio connected account structure from API response', | ||
| required: ['id'], | ||
| properties: { | ||
| id: { | ||
| type: 'string', | ||
| description: 'Connected account ID', | ||
| }, | ||
| status: { | ||
| type: 'string', | ||
| description: 'Connection status', | ||
| }, | ||
| toolkit: { | ||
| type: 'object', | ||
| properties: { | ||
| slug: { | ||
| type: 'string', | ||
| description: 'Toolkit slug identifier', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } as const; | ||
|
|
||
| export const ComposioToolSchemaSchema = { | ||
| type: 'object', | ||
| description: 'Composio tool JSON schema structure', | ||
| properties: { | ||
| type: { | ||
| type: 'string', | ||
| description: 'Schema type', | ||
| }, | ||
| properties: { | ||
| type: 'object', | ||
| additionalProperties: { | ||
| $ref: '#/components/schemas/ComposioSchemaProperty', | ||
| }, | ||
| description: 'Schema properties', | ||
| }, | ||
| required: { | ||
| type: 'array', | ||
| items: { | ||
| type: 'string', | ||
| }, | ||
| description: 'Required property names', | ||
| }, | ||
| }, | ||
| additionalProperties: true, | ||
| } as const; | ||
|
|
||
| export const ComposioSchemaPropertySchema = { | ||
| type: 'object', | ||
| description: 'Composio schema property definition', | ||
| properties: { | ||
| type: { | ||
| type: 'string', | ||
| description: 'Property type', | ||
| }, | ||
| description: { | ||
| type: 'string', | ||
| description: 'Property description', | ||
| }, | ||
| deprecated: { | ||
| type: 'boolean', | ||
| description: 'Whether property is deprecated', | ||
| }, | ||
| }, | ||
| additionalProperties: true, | ||
| } as const; | ||
|
|
||
| export const ToolCreationContextSchema = { | ||
| type: 'object', | ||
| description: | ||
| 'Context for creating a DynamicStructuredTool. User/userId comes from getCurrentUser() at runtime.', | ||
| required: [ | ||
| 'connectedAccountId', | ||
| 'authType', | ||
| 'creditCost', | ||
| 'toolsetType', | ||
| 'toolsetKey', | ||
| 'toolsetName', | ||
| ], | ||
| properties: { | ||
| connectedAccountId: { | ||
| type: 'string', | ||
| description: 'Connected account ID from Composio', | ||
| }, | ||
| authType: { | ||
| type: 'string', | ||
| enum: ['oauth', 'apikey'], | ||
| description: 'Authentication type', | ||
| }, | ||
| creditCost: { | ||
| type: 'number', | ||
| description: 'Credit cost for tool execution', | ||
| }, | ||
| toolsetType: { | ||
| $ref: '#/components/schemas/GenericToolsetType', | ||
| description: 'Toolset type identifier', | ||
| }, | ||
| toolsetKey: { | ||
| type: 'string', | ||
| description: 'Toolset key', | ||
| }, | ||
| toolsetName: { | ||
| type: 'string', | ||
| description: 'Toolset display name', | ||
| }, | ||
| }, | ||
| } as const; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for authType enum values to check for consistency
echo "=== Searching for authType usages ==="
rg -n "authType" . --type ts --type tsx --type js --type jsx -A 2 -B 2 | head -100
echo -e "\n=== Checking for apiKey variations ==="
rg -n "(apiKey|apikey|api_key)" . --type ts --type tsx --type js --type jsx | head -50
echo -e "\n=== Searching for ToolCreationContext references ==="
rg -n "ToolCreationContext" . --type ts --type tsx --type js --type jsx -A 2 -B 2 | head -100Repository: refly-ai/refly
Length of output: 290
🏁 Script executed:
#!/bin/bash
# Broader search without file type restrictions
echo "=== Searching for authType ==="
rg "authType" . -i -A 2 -B 2 | head -150
echo -e "\n=== Searching for 'oauth' and 'apikey' enum patterns ==="
rg "(oauth|apikey)" . -i --max-count 20
echo -e "\n=== Looking at schemas.gen.ts file ==="
wc -l packages/openapi-schema/src/schemas.gen.ts
echo -e "\n=== Check if this is auto-generated ==="
head -20 packages/openapi-schema/src/schemas.gen.tsRepository: refly-ai/refly
Length of output: 50372
🏁 Script executed:
#!/bin/bash
# Check the specific lines mentioned in the review
sed -n '9538,9648p' packages/openapi-schema/src/schemas.gen.ts | head -120Repository: refly-ai/refly
Length of output: 2600
Fix ToolCreationContext description
The description on ToolCreationContextSchema mentions User/userId comes from getCurrentUser(), but this schema has no user/userId fields, which is misleading. Update to:
- description:
- 'Context for creating a DynamicStructuredTool. User/userId comes from getCurrentUser() at runtime.',
+ description:
+ 'Context for creating a DynamicStructuredTool. User identity is resolved at runtime and not included here.',The authType enum values ('oauth' | 'apikey') are consistent throughout the codebase and require no changes.
🤖 Prompt for AI Agents
In packages/openapi-schema/src/schemas.gen.ts around lines 9538 to 9648, the
ToolCreationContextSchema description incorrectly mentions "User/userId comes
from getCurrentUser()" though no user or userId fields exist; remove that
misleading clause and replace the description with a concise accurate statement
such as "Context for creating a DynamicStructuredTool" (or similar phrasing that
does not reference user/userId), leaving the required properties and enum for
authType unchanged.
| oneOf: [ | ||
| { | ||
| type: 'object', | ||
| additionalProperties: true, | ||
| }, | ||
| { | ||
| type: 'array', | ||
| items: { | ||
| type: 'object', | ||
| additionalProperties: true, | ||
| }, | ||
| }, | ||
| ], | ||
| description: 'Response data (object or array of objects)', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for HandlerResponse usage and related patterns
rg -n "HandlerResponse" --type ts --type tsx --type js --type jsxRepository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Look for how data property is accessed/used
rg -n "\.data\s*[?\.=\[]|data\s*\??\." --type ts --type tsx --type js --type jsx -A 2 -B 2Repository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Check if there are type definitions for HandlerResponse
fd -e ts -e tsx | xargs grep -l "interface HandlerResponse\|type HandlerResponse" 2>/dev/nullRepository: refly-ai/refly
Length of output: 233
🏁 Script executed:
# Look at the generated file to understand context better
wc -l packages/openapi-schema/src/schemas.gen.tsRepository: refly-ai/refly
Length of output: 106
🏁 Script executed:
# Search for HandlerResponse definition
rg -n "HandlerResponse" packages/openapi-schema/src/types.gen.tsRepository: refly-ai/refly
Length of output: 94
🏁 Script executed:
# Search for HandlerResponse usage across the repo
rg -n "HandlerResponse" --max-count=50Repository: refly-ai/refly
Length of output: 2613
🏁 Script executed:
# Check the specific schema section in the generated file
sed -n '11940,11965p' packages/openapi-schema/src/schemas.gen.tsRepository: refly-ai/refly
Length of output: 623
🏁 Script executed:
# Look for how data is accessed in consumers
rg -n "\.data" packages/ | grep -i response | head -20Repository: refly-ai/refly
Length of output: 2436
🏁 Script executed:
# Check the HandlerResponse type definition
cat -n packages/common-types/src/request.tsRepository: refly-ai/refly
Length of output: 344
🏁 Script executed:
# Check the handler.ts createSuccessResponse implementation
sed -n '200,230p' apps/api/src/modules/tool/dynamic-tooling/core/handler.tsRepository: refly-ai/refly
Length of output: 850
🏁 Script executed:
# Look at mcp-utils.ts context around line 118
sed -n '110,130p' packages/skill-template/src/utils/mcp-utils.tsRepository: refly-ai/refly
Length of output: 799
🏁 Script executed:
# Search for tests related to HandlerResponse
rg -n "HandlerResponse" --glob="*.test.ts" --glob="*.spec.ts"Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for createSuccessResponse calls
rg -n "createSuccessResponse" -A 1 -B 1Repository: refly-ai/refly
Length of output: 681
🏁 Script executed:
# Look at handler implementations to see what data they return
sed -n '125,180p' apps/api/src/modules/tool/dynamic-tooling/core/handler.tsRepository: refly-ai/refly
Length of output: 1832
🏁 Script executed:
# Check if there are direct HandlerResponse returns with array data
rg -n "return\s*{.*success.*data.*\[" -A 2Repository: refly-ai/refly
Length of output: 179
🏁 Script executed:
# Look at the schema.yml to understand when this change was made
sed -n '12740,12800p' packages/openapi-schema/schema.ymlRepository: refly-ai/refly
Length of output: 1754
🏁 Script executed:
# Look for executeRequest implementations
rg -n "protected.*executeRequest|async.*executeRequest" -A 15 apps/Repository: refly-ai/refly
Length of output: 3044
🏁 Script executed:
# Check the HTTP adapter to see what adapterResponse.data might be
rg -n "adapterResponse" -B 5 -A 5 apps/api/src/modules/tool/dynamic-tooling/core/handler.tsRepository: refly-ai/refly
Length of output: 879
🏁 Script executed:
# Search for any handler that might return arrays
rg -n "data.*=.*\[\]|data:\s*\[" apps/api/src/modules/tool/dynamic-tooling/ | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Check git log for the schema file
git log --oneline -20 packages/openapi-schema/schema.ymlRepository: refly-ai/refly
Length of output: 136
🏁 Script executed:
# Check if there's a CHANGELOG or recent commits
git log --oneline -10 packages/openapi-schema/src/schemas.gen.tsRepository: refly-ai/refly
Length of output: 136
🏁 Script executed:
# Look at the actual commit that changed the schema
git log -p -1 --follow -- packages/openapi-schema/schema.yml | head -100Repository: refly-ai/refly
Length of output: 40
Schema allows data to be object or array; review consumers for backward compatibility
The recent change to allow data as either an object or array of objects (via the feat/tools/multi-obj branch) reflects actual backend capability. However, existing consumers that assume data is always an object may break when handlers return arrays. Most code uses defensive optional chaining (?.), but direct property access will fail for array responses.
- Review code that accesses specific properties on
data(e.g.,.fileId,.total) to ensure it handles both shapes. - Document which handlers return arrays vs. objects to prevent ambiguity for API consumers.
- Note: The generic
HandlerResponse<T>type supports both cases when used properly; consumers with explicit types should be unaffected.
🤖 Prompt for AI Agents
In packages/openapi-schema/src/schemas.gen.ts around lines 11947 to 11960 the
schema for response.data was changed to allow either an object or an array of
objects; update callers and docs so consumers handle both shapes: scan code
paths that access response.data.<property> and replace unsafe direct access with
a runtime guard (e.g., Array.isArray(response.data) to branch, or typeof checks)
or narrow the generic HandlerResponse<T> to the correct T before accessing
properties, add unit tests for handlers that return arrays to prevent
regressions, and update API docs/handler annotations to clearly list which
endpoints return arrays versus single objects so clients can rely on the correct
shape.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/request/src/requests/types.gen.ts (1)
8577-8591: FixextractFileIdToTopLevelto handle array data inHandlerResponse.The
datafield inHandlerResponsenow supports both single objects and arrays, but the code at line 184 inhandler-post.tsassumes data is always an object:data: { ...(response.data as Record<string, unknown>), ...extractedResources, },If
response.datais an array, spreading it will create numeric keys ({0: item, 1: item}) instead of preserving the array structure, breaking resource extraction. Add anArray.isArray()check before this operation to handle both cases correctly:data: Array.isArray(response.data) ? response.data : { ...(response.data as Record<string, unknown>), ...extractedResources, },Also verify that all other code paths handling
HandlerResponse.dataproperly validate the data type before accessing or spreading it.
♻️ Duplicate comments (1)
apps/api/package.json (1)
104-104: Version concern already flagged in previous review.The json-schema-traverse version specification has already been reviewed. Please refer to the previous review comment regarding the version 1.0.0 vs 0.3.1 discrepancy and ensure the intended version is confirmed.
Also applies to: 152-152
🧹 Nitpick comments (1)
apps/api/src/modules/tool/utils/schema-utils.ts (1)
118-134: Consider using nullish coalescing for cleaner null/undefined checks.The explicit check
result[key] === undefined || result[key] === nullon line 120 could be simplified using the nullish coalescing operator, which aligns with the coding guidelines.Consider this refactor:
for (const [key, propertySchema] of Object.entries(properties)) { - // If value is undefined or null, apply default if exists - if (result[key] === undefined || result[key] === null) { + // Apply default if value is nullish + if (result[key] == null) { if ('default' in propertySchema) {Note: Using
== nullchecks for bothnullandundefined, which is the standard JavaScript idiom.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/api/package.json(2 hunks)apps/api/src/modules/tool/utils/schema-utils.ts(1 hunks)packages/request/src/requests/types.gen.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
packages/request/src/requests/types.gen.tsapps/api/package.jsonapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
packages/request/src/requests/types.gen.tsapps/api/package.jsonapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
packages/request/src/requests/types.gen.tsapps/api/package.jsonapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
packages/request/src/requests/types.gen.tsapps/api/package.jsonapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
packages/request/src/requests/types.gen.tsapps/api/src/modules/tool/utils/schema-utils.ts
🧠 Learnings (3)
📚 Learning: 2025-11-25T03:04:12.836Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/06-api-structure.mdc:0-0
Timestamp: 2025-11-25T03:04:12.836Z
Learning: Applies to apps/api/src/{auth,user,project,canvas,rag,knowledge,search,skill,share,code-artifact}/**/*.{ts,tsx} : Follow NestJS module structure with controllers, services, and DTOs in dedicated module directories
Applied to files:
apps/api/src/modules/tool/utils/schema-utils.ts
📚 Learning: 2025-11-25T03:04:12.836Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/06-api-structure.mdc:0-0
Timestamp: 2025-11-25T03:04:12.836Z
Learning: Applies to apps/api/src/{utils,common}/** : Organize shared utilities in dedicated utility and common directories
Applied to files:
apps/api/src/modules/tool/utils/schema-utils.ts
📚 Learning: 2025-11-25T03:04:21.098Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/07-extension-structure.mdc:0-0
Timestamp: 2025-11-25T03:04:21.098Z
Learning: Applies to apps/extension/src/utils/**/*.{ts,tsx,js,jsx} : Utility functions should be located in apps/extension/src/utils directory
Applied to files:
apps/api/src/modules/tool/utils/schema-utils.ts
🧬 Code graph analysis (2)
packages/request/src/requests/types.gen.ts (2)
packages/ai-workspace-common/src/requests/types.gen.ts (4)
User(474-483)DriveFile(7518-7583)SchemaProperty(8011-8076)JsonSchema(8081-8100)packages/openapi-schema/src/types.gen.ts (4)
User(474-483)DriveFile(7518-7583)SchemaProperty(8011-8076)JsonSchema(8081-8100)
apps/api/src/modules/tool/utils/schema-utils.ts (1)
packages/request/src/requests/types.gen.ts (3)
ResourceField(8109-8126)SchemaProperty(8011-8076)JsonSchema(8081-8100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build
🔇 Additional comments (7)
apps/api/src/modules/tool/utils/schema-utils.ts (6)
59-86: LGTM! Schema parsing and validation logic is well-structured.Both functions properly define explicit return types and implement robust error handling with descriptive error messages.
346-383: LGTM! Discriminator-based option matching is well-implemented.The function correctly defines explicit return types and implements safe logic for finding matching schema options. The non-null assertion on line 363 is safe because the filter on line 351 ensures
propertiesexists for allobjectOptions.
44-54: Add explicit return type to public function.As per coding guidelines, public API functions should have explicit return types for better type safety and API clarity.
Apply this diff:
-export function jsonSchemaToZod(schema: JsonSchema): z.ZodTypeAny { +export function jsonSchemaToZod(schema: JsonSchema): z.ZodTypeAny {Wait, the return type is already present. Let me re-examine... Actually, the return type
z.ZodTypeAnyis declared. This is correct. My mistake.
321-341: Add explicit return type to public function.As per coding guidelines, public API functions should always define explicit return types, especially for improved API clarity and type safety.
Apply this diff:
-export function collectResourceFields(schema: JsonSchema): ResourceField[] { +export function collectResourceFields(schema: JsonSchema): ResourceField[] {Actually, looking at line 321, the return type
ResourceField[]is already declared. This is correct.
221-227: No action needed — Node.js 20.19.0+ fully supports ES2018 regex features.The project requires Node.js >=20.19.0, which includes comprehensive ES2018 support. Negative lookbehind assertions in regular expressions are compatible and have been standard since Node.js 10.0. The regex pattern on line 224 is safe to use.
14-14: Useimport typefor type-only imports.The
znamespace from 'zod' is only used for type annotations (z.ZodTypeAny) in this file, not for runtime values. As per coding guidelines, type-only imports should use theimport typesyntax.Apply this diff:
-import type { z } from 'zod'; +import type { type z } from 'zod';Or alternatively, import the specific type directly if exported:
import type { ZodTypeAny } from 'zod';⛔ Skipped due to learnings
Learnt from: CR Repo: refly-ai/refly PR: 0 File: .cursor/rules/03-typescript-guidelines.mdc:0-0 Timestamp: 2025-11-25T03:03:43.116Z Learning: Applies to **/*.{ts,tsx} : Always import types explicitly using the `import type` syntaxLearnt from: CR Repo: refly-ai/refly PR: 0 File: .cursor/rules/03-typescript-guidelines.mdc:0-0 Timestamp: 2025-11-25T03:03:43.116Z Learning: Applies to **/*.{ts,tsx} : Group type imports separately from value importsLearnt from: CR Repo: refly-ai/refly PR: 0 File: .cursor/rules/03-typescript-guidelines.mdc:0-0 Timestamp: 2025-11-25T03:03:43.116Z Learning: Applies to **/*.{ts,tsx} : Minimize creating local type aliases for imported typespackages/request/src/requests/types.gen.ts (1)
6714-6742: LGTM! Past review issue resolved.The
toolsetKeyfield has been added toPostHandlerContextas requested in the previous review. The type now correctly distinguishes betweentoolsetName(display name) andtoolsetKey(unique identifier), matching the structure in other packages.
| /** | ||
| * Composio tool JSON schema structure | ||
| */ | ||
| export type ComposioToolSchema = { | ||
| /** | ||
| * Schema type | ||
| */ | ||
| type?: string; | ||
| /** | ||
| * Schema properties | ||
| */ | ||
| properties?: { | ||
| [key: string]: ComposioSchemaProperty; | ||
| }; | ||
| /** | ||
| * Required property names | ||
| */ | ||
| required?: Array<string>; | ||
| [key: string]: unknown | string | ComposioSchemaProperty; | ||
| }; | ||
|
|
||
| /** | ||
| * Composio schema property definition | ||
| */ | ||
| export type ComposioSchemaProperty = { | ||
| /** | ||
| * Property type | ||
| */ | ||
| type?: string; | ||
| /** | ||
| * Property description | ||
| */ | ||
| description?: string; | ||
| /** | ||
| * Whether property is deprecated | ||
| */ | ||
| deprecated?: boolean; | ||
| [key: string]: unknown | string | boolean; | ||
| }; |
There was a problem hiding this comment.
Index signatures overly permissive with unknown in union types.
Lines 6806 and 6825 define index signatures that include unknown in their union types:
- Line 6806:
[key: string]: unknown | string | ComposioSchemaProperty; - Line 6825:
[key: string]: unknown | string | boolean;
Since unknown can represent any type, including it in a union effectively allows any value, defeating TypeScript's type safety. This is similar to using [key: string]: any, which violates the coding guideline to avoid the any type.
Suggested fix:
export type ComposioToolSchema = {
type?: string;
properties?: {
[key: string]: ComposioSchemaProperty;
};
required?: Array<string>;
- [key: string]: unknown | string | ComposioSchemaProperty;
+ [key: string]: string | ComposioSchemaProperty | Array<string> | { [key: string]: ComposioSchemaProperty };
};
export type ComposioSchemaProperty = {
type?: string;
description?: string;
deprecated?: boolean;
- [key: string]: unknown | string | boolean;
+ [key: string]: string | boolean;
};Note: If the Composio API truly allows arbitrary values, use unknown alone without the union, and add runtime validation.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/api/src/modules/tool/billing/billing.service.ts (1)
60-73: Clarify default credit cost and guard originalPrice/params with safe fallbacks.Right now
finalDiscountedPrice/finalOriginalPricedefault to1without explanation, andfinalOriginalPrice = originalPricecan assignundefinedwhenoriginalPriceis omitted. Also,params || {}is slightly off the project guideline that prefers nullish coalescing.To make the behavior explicit and avoid accidental
undefined, consider:- // Determine credit cost - let finalDiscountedPrice = 1; - let finalOriginalPrice = 1; + // Default to charging 1 credit when no explicit pricing is provided (backwards compatible) + let finalDiscountedPrice = 1; + let finalOriginalPrice = 1; @@ - // Direct credit cost provided (Composio scenario) - finalDiscountedPrice = discountedPrice; - finalOriginalPrice = originalPrice; + // Direct credit cost provided (Composio scenario) + finalDiscountedPrice = discountedPrice; + // Fall back to discounted price when originalPrice is not provided + finalOriginalPrice = originalPrice ?? discountedPrice; @@ - // Calculate from billing config (dynamic-tooling scenario) - finalDiscountedPrice = calculateCredits(billingConfig, params || {}); + // Calculate from billing config (dynamic-tooling scenario) + finalDiscountedPrice = calculateCredits(billingConfig, params ?? {});This keeps defaults intentional and avoids propagating
undefinedwhile following the nullish-coalescing guideline.
🧹 Nitpick comments (1)
apps/api/src/modules/tool/billing/billing.service.ts (1)
129-153: Subscription lookup helper is solid; consider a tiny readability tweak.The Prisma query correctly filters active, non‑canceled subscriptions and restricts the projection to
lookupKey. As a minor readability/consistency tweak, you could hoistnew Date()into a local variable so it’s evaluated once and makes the intent clearer:private async checkFreeToolAccess(uid: string): Promise<boolean> { try { + const now = new Date(); const subscription = await this.prisma.subscription.findFirst({ where: { uid, status: 'active', - OR: [{ cancelAt: null }, { cancelAt: { gt: new Date() } }], + OR: [{ cancelAt: null }, { cancelAt: { gt: now } }], },Not critical, but it slightly improves readability and avoids recreating
Dateinline.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/modules/tool/billing/billing.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
apps/api/src/modules/tool/billing/billing.service.ts
apps/api/src/**/*.{controller,service}.ts
📄 CodeRabbit inference engine (.cursor/rules/06-api-structure.mdc)
Implement proper error handling in API modules
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
apps/api/src/modules/tool/billing/billing.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
apps/api/src/modules/tool/billing/billing.service.ts
🧬 Code graph analysis (1)
apps/api/src/modules/tool/billing/billing.service.ts (4)
apps/api/src/modules/tool/billing/billing.dto.ts (2)
ProcessBillingOptions(11-42)ProcessBillingResult(47-56)apps/api/src/modules/tool/utils/billing.ts (1)
calculateCredits(15-59)apps/api/src/modules/credit/credit.dto.ts (1)
SyncToolCreditUsageJobData(15-27)apps/api/src/modules/tool/tool-context.ts (3)
getResultId(145-148)getResultVersion(154-157)getToolCallId(136-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build / Build
🔇 Additional comments (1)
apps/api/src/modules/tool/billing/billing.service.ts (1)
110-120: Error handling strategy for billing failures looks appropriate.Catching errors, logging them with tool context, and returning
{ success: false, discountedPrice: 0 }without throwing is a reasonable trade‑off so billing issues don’t break tool execution.
| /** | ||
| * Toolset keys that are always free (no credit charge) | ||
| */ | ||
| const FREE_TOOLSET_KEYS = [ | ||
| 'nano_banana_pro', | ||
| 'instagram', | ||
| 'tiktok', | ||
| 'facebook', | ||
| 'twitter', | ||
| 'fish_audio', | ||
| 'fal_image', | ||
| 'fal_video', | ||
| 'fal_audio', | ||
| ]; |
There was a problem hiding this comment.
Align FREE_TOOLSET_KEYS comments with actual subscription‑gated behavior.
FREE_TOOLSET_KEYS is documented as “always free (no credit charge)”, and the block at Line 75 says “specific toolsets are always free”, but the implementation only makes them free if checkFreeToolAccess(uid) returns true (subscription‑based).
Consider updating comments (or logic) so the behavior is unambiguous, for example:
-/**
- * Toolset keys that are always free (no credit charge)
- */
+/**
+ * Toolset keys that are eligible for subscription-based free access
+ * (only free when user has a qualifying subscription)
+ */
@@
- // Apply toolset-based free access (specific toolsets are always free)
+ // Apply subscription-based free access for eligible toolsets
if (FREE_TOOLSET_KEYS.includes(toolsetKey)) {
- // Apply subscription-based discount
+ // Apply subscription-based discountThis avoids future confusion about whether these toolsets are unconditionally free or only free for certain plans.
Also applies to: 75-85
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/billing/billing.service.ts around lines 25-38 and
75-85, the comments state these toolset keys are "always free", but the code
only grants free access when checkFreeToolAccess(uid) is true; update the
comments to accurately reflect that these keys are "free for users/plans that
pass checkFreeToolAccess(uid)" (or, if the intent is unconditional free access,
change the logic to bypass checkFreeToolAccess for these keys), and make the
wording consistent in both locations so the behavior is unambiguous.
| // Record credit usage | ||
| const jobData: SyncToolCreditUsageJobData = { | ||
| uid, | ||
| discountedPrice: finalDiscountedPrice, | ||
| originalPrice: finalOriginalPrice, | ||
| timestamp: new Date(), | ||
| resultId: getResultId(), | ||
| version: getResultVersion(), | ||
| toolCallId: getToolCallId(), | ||
| toolCallMeta: { | ||
| toolName, | ||
| toolsetKey, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "SyncToolCreditUsageJobData" apps/api -C 3Repository: refly-ai/refly
Length of output: 5562
🏁 Script executed:
# Find the type/interface definition
rg -n "interface SyncToolCreditUsageJobData|type SyncToolCreditUsageJobData" apps/apiRepository: refly-ai/refly
Length of output: 148
🏁 Script executed:
# Find the implementations of these getter functions
rg -n "getResultId|getResultVersion|getToolCallId" apps/api/src/modules/tool/billing/billing.service.ts -B 2 -A 5Repository: refly-ai/refly
Length of output: 749
🏁 Script executed:
cat apps/api/src/modules/credit/credit.dto.ts | head -50Repository: refly-ai/refly
Length of output: 1222
🏁 Script executed:
cat apps/api/src/modules/tool/tool-context.tsRepository: refly-ai/refly
Length of output: 4088
🏁 Script executed:
sed -n '752,780p' apps/api/src/modules/credit/credit.service.tsRepository: refly-ai/refly
Length of output: 1009
Guard optional tool-context values before constructing SyncToolCreditUsageJobData.
The functions getResultId(), getResultVersion(), and getToolCallId() all return string | undefined and number | undefined respectively, but SyncToolCreditUsageJobData defines resultId, version, and toolCallId as required (non-optional) fields. Passing undefined to these required fields violates the type contract and will cause issues when the data is processed downstream in credit.service.ts.
Depending on the intended behavior:
- Make these fields optional in
SyncToolCreditUsageJobDataand conditionally include them:
const resultId = getResultId();
const version = getResultVersion();
const toolCallId = getToolCallId();
const jobData: SyncToolCreditUsageJobData = {
uid,
discountedPrice: finalDiscountedPrice,
originalPrice: finalOriginalPrice,
timestamp: new Date(),
...(resultId && { resultId }),
...(typeof version === 'number' && { version }),
...(toolCallId && { toolCallId }),
toolCallMeta: { toolName, toolsetKey },
};- Or enforce explicit fallback values if these fields are always required:
resultId: getResultId() ?? 'unknown',
version: getResultVersion() ?? 0,
toolCallId: getToolCallId() ?? 'unknown',Ensure the DTO definition and runtime behavior are aligned.
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/billing/billing.service.ts around lines 87 to 100,
the values returned by getResultId(), getResultVersion(), and getToolCallId()
can be undefined but are being assigned to required fields on
SyncToolCreditUsageJobData; either make those fields optional on the DTO and
conditionally spread them into jobData when present, or provide explicit
fallback values (e.g., 'unknown' for strings and 0 for numeric version) so the
constructed jobData never contains undefined for required properties; update the
SyncToolCreditUsageJobData type if you choose the conditional inclusion approach
to reflect optional fields and adjust downstream consumers accordingly.
Summary
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Tip
Close issue syntax:
Fixes #<issue number>orResolves #<issue number>, see documentation for more details.Impact Areas
Please check the areas this PR affects:
Screenshots/Videos
Checklist
Important
Please review the checklist below before submitting your pull request.
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint godsSummary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.