feat(tool-call): implement tool call management and enhance action de… #1441
Conversation
…tail structure - Introduced ToolCallResult model to manage tool call data, including status, input, output, and timestamps. - Updated ActionDetail and ActionStep to include toolCalls, allowing for better tracking of tool interactions within actions. - Enhanced the ToolCallService to handle tool call persistence, grouping, and status updates. - Improved the rendering of tool call results in the UI, ensuring compliance with coding standards and performance optimizations. - Added necessary updates to OpenAPI schema and TypeScript types for better integration and type safety.
WalkthroughAdds end-to-end tool-call tracking: Prisma schema fields/index for ToolCallResult; new ToolCallModule/Service (XML/SSE/persist/fetch/group/derive-status); integrates tool calls into SkillInvoker and Action flows; frontend parsing, rendering, SSE, types, OpenAPI, canvas node versioning, and i18n updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Frontend
participant SkillInvoker
participant ToolCallService
participant DB[(Database)]
participant SSE
Frontend->>SkillInvoker: invoke action (resultId, version)
SkillInvoker->>ToolCallService: generateToolUseXML(startTs)
ToolCallService-->>SkillInvoker: xmlChunk
SkillInvoker->>ToolCallService: emitToolUseStream(res,{xmlContent,toolCallId,step})
ToolCallService->>SSE: writeSSEResponse(event: tool_call_stream, xml)
SSE-->>Client: stream tool_use chunk
alt tool completes
SkillInvoker->>ToolCallService: persistToolCallResult(..., status=COMPLETED)
else tool errors
SkillInvoker->>ToolCallService: persistToolCallResult(..., status=FAILED, error)
end
ToolCallService->>DB: upsert ToolCallResult
ToolCallService->>DB: deriveAndUpdateActionStatus(resultId, version)
sequenceDiagram
participant Client
participant ActionService
participant ToolCallService
participant DB[(Database)]
Client->>ActionService: getActionResult(resultId, version)
ActionService->>DB: fetch ActionResult + steps
ActionService->>ToolCallService: fetchToolCalls(resultId, version)
ToolCallService->>DB: query ToolCallResult[]
DB-->>ToolCallService: toolCalls
ToolCallService->>ToolCallService: groupToolCallsByStep(steps, toolCalls)
ToolCallService-->>ActionService: attachToolCallsToSteps(...)
ActionService-->>Client: ActionDetail with steps[].toolCalls and derived status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsx (2)
233-244: Retry payload ignores the latest selected toolsets.handleRetry sends selectedToolsets from node metadata, not user-updated state. Use the state variable to avoid silently dropping user choices.
Apply:
- selectedToolsets: nodeSelectedToolsets, + selectedToolsets,Also add selectedToolsets to the dependency array.
- }, [resultId, title, canvasId, invokeAction, resetFailedState, setNodeData, node.id, node.data]); + }, [resultId, title, canvasId, invokeAction, resetFailedState, setNodeData, node.id, node.data, selectedToolsets]);
236-244: Retry should use the current query, not the title.Users may edit the query; sending title loses intent.
Apply:
- query: title, + query: currentQuery ?? title,And include currentQuery in dependencies:
- }, [resultId, title, canvasId, invokeAction, resetFailedState, setNodeData, node.id, node.data]); + }, [resultId, title, currentQuery, canvasId, invokeAction, resetFailedState, setNodeData, node.id, node.data, selectedToolsets]);apps/api/src/modules/skill/skill-invoker.service.ts (3)
169-171: Potential null dereference on userPo.uiLocaleuserPo can be null; direct access risks a crash.
Apply:
- uiLocale: userPo.uiLocale, + uiLocale: userPo?.uiLocale,As per coding guidelines: use optional chaining when accessing object properties.
274-275: Avoid logging raw user input (PII/compliance risk)Logging full input may leak PII in logs. Log high‑level metadata only.
Suggested:
- this.logger.log( - `invoke skill with input: ${JSON.stringify(input)}, resultId: ${resultId}, version: ${version}`, - ); + this.logger.log( + `invoke skill: resultId=${resultId}, version=${version}, queryLength=${String(input?.query?.length ?? 0)}` + );Consider adding a redact helper for structured logs. Based on coding guidelines.
836-836: Guard SSE end event when res is undefinedAvoid calling writeSSEResponse with undefined res.
Apply:
- writeSSEResponse(res, { event: 'end', resultId, version }); + if (res) { + writeSSEResponse(res, { event: 'end', resultId, version }); + }packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx (1)
61-82: Missing prop typing:data-tool-toolset-keyis used but not declared.This breaks TS type-checking.
interface ToolCallProps { 'data-tool-name'?: string; + 'data-tool-toolset-key'?: string; 'data-tool-call-id'?: string; 'data-tool-call-status'?: string; 'data-tool-created-at'?: string; 'data-tool-updated-at'?: string; 'data-tool-arguments'?: string; 'data-tool-result'?: string; 'data-tool-type'?: 'use' | 'result'; ... 'data-tool-error'?: string; id?: string; mode?: MarkdownMode; }packages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx (1)
806-815: Invalid Tailwind classz-1confirmed; however, suggested replacementz-10may be semantically incorrect.The review comment is correct that
z-1is not a valid Tailwind class—it is not defined in the project's Tailwind configuration. However, the suggested replacement toz-10appears semantically wrong for a base node container. Standard Tailwind only providesz-0,z-10,z-20,z-30,z-40, andz-50. For a primary container element beneath overlays (which usez-10),z-0would be the appropriate Tailwind value.This issue also exists across multiple canvas node files (start.tsx, website.tsx, video.tsx, skill.tsx, resource.tsx, code-artifact.tsx, audio.tsx, etc.), not just skill-response.tsx, suggesting a systemic pattern rather than an isolated mistake.
🧹 Nitpick comments (21)
packages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsx (1)
223-231: Guard against rapid retries causing duplicate versions/requests.Double-clicks can compute the same nextVersion and fire multiple invokes.
Options:
- Disable the retry button while isStreaming or during a local “retrying” flag.
- Derive nextVersion from the latest store result/version inside the callback before invoking.
Example minimal guard:
+ const [retrying, setRetrying] = useState(false); const handleRetry = useCallback(() => { + if (retrying || isStreaming) { return; } + setRetrying(true); // Reset failed state before retrying resetFailedState(resultId); // Update node status immediately to show "waiting" state const nextVersion = (node.data?.metadata?.version || 0) + 1; setNodeData(node.id, { ...node.data, metadata: { ...node.data?.metadata, status: 'waiting', version: nextVersion, }, }); invokeAction( { resultId, - query: title, + query: currentQuery ?? title, selectedSkill: { name: actionMeta?.name || 'commonQnA', }, contextItems, - selectedToolsets: nodeSelectedToolsets, + selectedToolsets, version: nextVersion, }, { entityId: canvasId, entityType: 'canvas', }, ); + setRetrying(false); }, [/* include: retrying, isStreaming, currentQuery, selectedToolsets, ... */]);Also applies to: 243-249
apps/api/src/modules/tool-call/tool-call.module.ts (1)
5-10: Module scaffold is minimal and correct.Provides and exports ToolCallService; good separation. Consider adding @global() if widely used to reduce import churn (optional).
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.ts (1)
35-46: Type safety and Visitor usage.Everything is typed as any; tighten types and visitor signatures to match unist/hast.
Apply:
-import { visit } from 'unist-util-visit'; +import { visit } from 'unist-util-visit'; +import type { Root } from 'hast'; +import type { Parent } from 'unist'; -function rehypePlugin() { - return (tree: any) => { +function rehypePlugin() { + return (tree: Root) => { visit(tree, (node, index, parent) => { // ... }); // Post-pass: merge duplicated tool nodes by callId, keeping the first occurrence - const toolNodes: Array<{ node: any; index: number; parent: any }> = []; - visit(tree, (node: any, index: number | null, parent: any) => { + const toolNodes: Array<{ node: any; index: number; parent: Parent }> = []; + visit(tree, (node: any, index: number | null, parent: Parent | null) => { // ... }); - const firstByCallId = new Map<string, { node: any; index: number; parent: any }>(); - const toRemove: Array<{ index: number; parent: any }> = []; + const firstByCallId = new Map<string, { node: any; index: number; parent: Parent }>(); + const toRemove: Array<{ index: number; parent: Parent }> = []; // Remove duplicates... - for (const { index, parent } of sortedRemovals) { - if (Array.isArray(parent?.children)) { - parent.children.splice(index, 1); - } - } + for (const { index, parent } of sortedRemovals) { + if (Array.isArray((parent as Parent).children)) { + (parent as Parent).children.splice(index, 1); + } + } }; }Optionally declare a Plugin<[], Root> type for the export.
Based on learningsAlso applies to: 48-50, 74-81
packages/utils/src/step.ts (1)
1-1: Consider using explicit type import syntax.The coding guidelines specify to use explicit
import typesyntax for type-only imports and group them separately. IfToolCallResultis only used for type annotations, consider:As per coding guidelines.
-import { ActionStep, Artifact, ActionResult, ToolCallResult } from '@refly/openapi-schema'; +import type { ActionStep, Artifact, ActionResult, ToolCallResult } from '@refly/openapi-schema';Note: Only apply this if
ToolCallResultis used purely for type annotations and not at runtime.packages/openapi-schema/src/types.gen.ts (1)
3246-3249: Consider improving auto-generated type alias naming.The type alias
status2(tool call status) andstatus3(test result status at line 5424) use generic numeric suffixes that don't convey their semantic meaning. This can cause confusion, especially sincestatus2appears in other generated files with different values (e.g.,'success' | 'failed' | 'unknown'inpackages/ai-workspace-common/src/requests/types.gen.ts).Since this is an auto-generated file, consider updating the OpenAPI schema or code generator to produce more descriptive type alias names, such as:
ToolCallStatusinstead ofstatus2(line 3249)ProviderTestStatusinstead ofstatus3(line 5424)This would improve type clarity and reduce the risk of using the wrong status type.
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (1)
154-164: Type the toolCalls merge to OpenAPI type; avoid anyReplace any-casts with ToolCallResult and a narrow mapper to keep types sound.
Example:
+import type { ToolCallResult } from '@refly/openapi-schema'; ... - const merged = mergeToolCallById(updatedStep.toolCalls as any, parsed as any); - updatedStep.toolCalls = merged as any; + const merged = mergeToolCallById( + (updatedStep.toolCalls as ToolCallResult[] | undefined) as any, + parsed as any + ) as unknown as ToolCallResult[]; + updatedStep.toolCalls = merged;Optionally add a mapper from ParsedToolCall → ToolCallResult to avoid casts entirely.
apps/api/prisma/schema.prisma (1)
1558-1558: Index likely needs stepName for step-level grouping/filtering.If queries group tool calls by step in a result/version, add a composite index including stepName for efficient lookups.
- @@index([resultId, version]) + @@index([resultId, version]) + @@index([resultId, version, stepName])packages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx (3)
146-149: Avoidanyfor props; typemodelInfo.Use the generated OpenAPI type and import it as a type to keep TS soundness.
- modelInfo: any; + modelInfo: import('@refly/openapi-schema').ModelInfo | undefined;As per coding guidelines.
303-305: Remove console logging in production path.Console noise in effects hampers observability; use a debug logger or guard by env.
- console.log( - `[Pilot Step Sync] Updating node ${id} status from ${data?.metadata?.status} to ${currentPilotStep.status}`, - ); + // optionally: debug('pilotStepSync', { nodeId: id, from: data?.metadata?.status, to: currentPilotStep.status });
838-852: Use brand palette for state colors.Replace raw
text-green-500with the project Success color (#12B76A) utility/token to match branding.- <IconLoading className="h-3 w-3 animate-spin text-green-500" /> + <IconLoading className="h-3 w-3 animate-spin text-[#12B76A]" />As per coding guidelines.
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (3)
8-9: Regex from variable is static here; simplify and avoid false ReDoS warning.Since TOOL_USE_TAG is constant, inline the literal to keep it simple and slightly faster.
-const TOOL_USE_REGEX = new RegExp(`<${TOOL_USE_TAG}[^>]*>([\\s\\S]*?)</${TOOL_USE_TAG}>`, 'i'); +const TOOL_USE_REGEX = /<tool_use[^>]*>([\s\S]*?)<\/tool_use>/i;If you plan to change TOOL_USE_TAG dynamically in future, keep current form and document safe inputs.
44-60: Unbounded caches can grow over long sessions.extractionCache never evicts. Scope caches per-process call or cap with a tiny LRU.
-const extractionCache = new Map<string, string>(); +const extractionCache = new Map<string, string>(); +const MAX_CACHE = 200; ... // Cache the result for future use extractionCache.set(cacheKey, result); + if (extractionCache.size > MAX_CACHE) { + const firstKey = extractionCache.keys().next().value; + extractionCache.delete(firstKey); + }Apply similar bounds to jsonParseCache.
330-348: Only first tool_use block is processed; consider multi-block chunks.If a chunk contains multiple tool_use blocks, downstream will miss subsequent ones.
Parse iteratively with a global regex or loop advancing lastIndex to return all matches.
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx (2)
154-163: Avoid inline style objects; use Tailwind utilities with conditional classes.Prevents re-renders and follows style guide.
- <ToolOutlined - className="text-gray-500 dark:text-gray-400" - style={{ - fontSize: '16px', - marginRight: '12px', - transition: 'transform 0.2s ease-in-out', - transform: isCollapsed ? 'rotate(-90deg)' : 'rotate(0deg)', - }} - /> + <ToolOutlined + className={[ + 'text-gray-500 dark:text-gray-400', + 'mr-3 transition-transform duration-200 ease-in-out', + isCollapsed ? '-rotate-90' : 'rotate-0', + 'text-[16px]', + ].join(' ')} + />As per coding guidelines.
169-188: Status UI looks good; ensure colors align with brand tokens.Consider using brand Success color (#12B76A) for completed and Error color (#F04438) for failed in class names.
apps/api/src/modules/tool-call/tool-call.service.ts (6)
1-6: Add explicit return type for fetchToolCalls and import the DB model typePublic APIs should declare explicit return types. Use the generated ToolCallResult type.
Apply:
import { Injectable, Logger } from '@nestjs/common'; import type { ActionStepMeta } from '@refly/openapi-schema'; import type { Response } from 'express'; import { ActionResult, ActionStatus } from '../../generated/client'; +import type { ToolCallResult as DbToolCallResult } from '../../generated/client';- async fetchToolCalls(resultId: string, version: number) { + async fetchToolCalls(resultId: string, version: number): Promise<DbToolCallResult[]> { return this.prisma.toolCallResult.findMany({Also applies to: 147-156
158-179: Avoid any[] and make grouping generic and type‑safeUse a generic for tool calls to retain types and remove any[].
Apply:
- groupToolCallsByStep( - steps: Array<{ name: string }>, - toolCalls: Array<{ stepName?: string | null }>, - ): Map<string, any[]> { + groupToolCallsByStep<TCall extends { stepName?: string | null }>( + steps: Array<{ name: string }>, + toolCalls: TCall[], + ): Map<string, TCall[]> { const existingStepNames = new Set(steps.map((step) => step.name)); const fallbackStepName = steps.length > 0 ? steps[steps.length - 1].name : undefined; - return toolCalls.reduce<Map<string, any[]>>((acc, call) => { + return toolCalls.reduce<Map<string, TCall[]>>((acc, call) => {
181-189: Propagate concrete tool call type to steps instead of any[]Keep strong typing when attaching tool calls to steps.
Apply:
- attachToolCallsToSteps<T extends { name: string }>( - steps: T[], - toolCallsByStep: Map<string, any[]>, - ): Array<T & { toolCalls?: any[] }> { + attachToolCallsToSteps<TStep extends { name: string }, TCall>( + steps: TStep[], + toolCallsByStep: Map<string, TCall[]>, + ): Array<TStep & { toolCalls?: TCall[] }> { return steps.map((step) => ({ ...step, toolCalls: toolCallsByStep.get(step.name), })); }
191-217: Use ToolCallStatus enum in derivation and declare explicit return typeAvoid magic strings; return type should be explicit.
Apply:
- async deriveAndUpdateActionStatus(result: ActionResult, toolCalls: Array<{ status: string }>) { + async deriveAndUpdateActionStatus( + result: ActionResult, + toolCalls: Array<{ status: ToolCallStatus }>, + ): Promise<ActionStatus> { let actionStatus = result.status; if (toolCalls.length > 0) { - const hasFailed = toolCalls.some((tc) => tc.status === 'failed'); - const hasExecuting = toolCalls.some((tc) => tc.status === 'executing'); - const allCompleted = toolCalls.every((tc) => tc.status === 'completed'); + const hasFailed = toolCalls.some((tc) => tc.status === ToolCallStatus.FAILED); + const hasExecuting = toolCalls.some((tc) => tc.status === ToolCallStatus.EXECUTING); + const allCompleted = toolCalls.every((tc) => tc.status === ToolCallStatus.COMPLETED);
25-26: Remove unused startTimestamps mapDead code; also risks accidental retention growth if later used without cleanup.
Apply:
- private readonly startTimestamps = new Map<string, number>();
117-131: Consider accepting unknown for input/output/error and normalize internallyCallers shouldn’t need to pre‑stringify. Accept unknown and stringify once in persistToolCall for consistency.
Apply:
- input: string | undefined, - output: string | undefined, + input: unknown | undefined, + output: unknown | undefined,- input?: string; - output?: string; - error?: string; + input?: unknown; + output?: unknown; + error?: unknown;Also applies to: 221-234
📜 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 (21)
apps/api/prisma/schema.prisma(2 hunks)apps/api/src/modules/action/action.dto.ts(1 hunks)apps/api/src/modules/action/action.module.ts(1 hunks)apps/api/src/modules/action/action.service.ts(3 hunks)apps/api/src/modules/skill/skill-invoker.service.ts(4 hunks)apps/api/src/modules/skill/skill.module.ts(2 hunks)apps/api/src/modules/tool-call/tool-call.module.ts(1 hunks)apps/api/src/modules/tool-call/tool-call.service.ts(1 hunks)packages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsx(3 hunks)packages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx(3 hunks)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.ts(2 hunks)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx(5 hunks)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts(4 hunks)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.ts(1 hunks)packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts(3 hunks)packages/i18n/src/en-US/ui.ts(1 hunks)packages/i18n/src/zh-Hans/ui.ts(1 hunks)packages/openapi-schema/schema.yml(3 hunks)packages/openapi-schema/src/schemas.gen.ts(3 hunks)packages/openapi-schema/src/types.gen.ts(4 hunks)packages/utils/src/step.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,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.Use JSDoc style comments for functions and classes in JavaScript/TypeScript
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals
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
Validate object properties before destructuring
Use ES6+ features like arrow functions, destructuring, and spread operators
Use async/await instead of raw promises for asynchronous code
**/*.{js,jsx,ts,tsx}: Maximum line length of 100 characters
Use 2 spaces for indentation, no tabs
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
No trailing whitespace at the end of lines
**/*.{js,jsx,ts,tsx}: Each file should contain only one main component
File names should match the component name
Keep component files focused with minimal dependencies
Group import statements in the following order: 1. React/framework libraries 2. Third-party libraries 3. Internal modules 4. Relative path imports (parent directories first, then child directories) 5. Type imports 6. Style imports
Sort imports alphabetically within each group
Leave a blank line between import groups
Components should be organized in the following order: 1. Import statements 2. Type definitions 3. Constant declarations 4. Component function 5. Hook calls 6. Event handlers 7. Helper render functions 8. JSX return statement
Extract complex logic in...
Files:
packages/i18n/src/en-US/ui.tsapps/api/src/modules/tool-call/tool-call.module.tsapps/api/src/modules/skill/skill.module.tspackages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsxpackages/i18n/src/zh-Hans/ui.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tsapps/api/src/modules/action/action.module.tspackages/openapi-schema/src/types.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tsapps/api/src/modules/skill/skill-invoker.service.tsapps/api/src/modules/action/action.dto.tsapps/api/src/modules/action/action.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.tspackages/utils/src/step.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - it defeats the purpose of TypeScript
When tempted to useany, useunknowntype instead and add proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new ones
Use TypeScript's utility types to derive new types:Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>
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 typesExplicitly type props interfaces/types
Files:
packages/i18n/src/en-US/ui.tsapps/api/src/modules/tool-call/tool-call.module.tsapps/api/src/modules/skill/skill.module.tspackages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsxpackages/i18n/src/zh-Hans/ui.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tsapps/api/src/modules/action/action.module.tspackages/openapi-schema/src/types.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tsapps/api/src/modules/skill/skill-invoker.service.tsapps/api/src/modules/action/action.dto.tsapps/api/src/modules/action/action.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.tspackages/utils/src/step.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
Follow NestJS patterns for backend code
Files:
packages/i18n/src/en-US/ui.tsapps/api/src/modules/tool-call/tool-call.module.tsapps/api/src/modules/skill/skill.module.tspackages/i18n/src/zh-Hans/ui.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tsapps/api/src/modules/action/action.module.tspackages/openapi-schema/src/types.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tsapps/api/src/modules/skill/skill-invoker.service.tsapps/api/src/modules/action/action.dto.tsapps/api/src/modules/action/action.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.tspackages/utils/src/step.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/openapi-schema/src/schemas.gen.ts
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}: Use Primary color (#155EEF) as the main brand color for buttons, links, and accents
Use Error color (#F04438) for error states and destructive actions
Use Success color (#12B76A) for success states and confirmations
Use Warning color (#F79009) for warnings and important notifications
Use Info color (#0BA5EC) for informational elements
Files:
packages/i18n/src/en-US/ui.tsapps/api/src/modules/tool-call/tool-call.module.tsapps/api/src/modules/skill/skill.module.tspackages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsxpackages/i18n/src/zh-Hans/ui.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tsapps/api/src/modules/action/action.module.tspackages/openapi-schema/src/types.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tsapps/api/src/modules/skill/skill-invoker.service.tsapps/api/src/modules/action/action.dto.tsapps/api/src/modules/action/action.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.tspackages/utils/src/step.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx
packages/i18n/src/{en-US,zh-Hans}/**
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
packages/i18n/src/{en-US,zh-Hans}/**: Add new translation keys to both language files: packages/i18n/src/en-US and packages/i18n/src/zh-Hans
Maintain consistency in naming conventions for translation keys
Use descriptive keys that reflect the content in translation files
Group related translation keys together
Use namespaces for different sections of the application in translation keys
Follow a hierarchical structure for nested components in translation keys
Support dynamic content with placeholders in translation strings
Files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
**/*.{css,scss,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{css,scss,less,styl,js,jsx,ts,tsx}: 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
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/i18n/src/en-US/ui.tsapps/api/src/modules/tool-call/tool-call.module.tsapps/api/src/modules/skill/skill.module.tspackages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsxpackages/i18n/src/zh-Hans/ui.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tsapps/api/src/modules/action/action.module.tspackages/openapi-schema/src/types.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tsapps/api/src/modules/skill/skill-invoker.service.tsapps/api/src/modules/action/action.dto.tsapps/api/src/modules/action/action.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.tspackages/utils/src/step.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx
**/*.{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
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 (avoid using index when possible)
Always split nested components with closures into separate components
Use lazy loading for components that are not immediately needed
Debounce handlers for events that might fire rapidly (resize, scroll, input)
Implement fallback UI for components that might fail
Use error boundaries to catch and handle runtime errors
Use Tailwind CSS for styling components
Group related utility classes together
Prefer Tailwind utilities over custom CSS when possible
**/*.{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 lineUse React best practices for frontend code
Files:
packages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsxpackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx
**/index.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Use index files to export multiple components from a directory
Files:
packages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.tsx: Use the translation wrapper component when displaying user-facing text in components
Ensure all user-facing text in components is translatable
Files:
packages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsxpackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-21T02:58:52.235Z
Learnt from: CR
PR: refly-ai/refly#0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-07-21T02:58:52.235Z
Learning: Applies to packages/i18n/src/{en-US,zh-Hans}/** : Add new translation keys to both language files: packages/i18n/src/en-US and packages/i18n/src/zh-Hans
Applied to files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
🧬 Code graph analysis (11)
apps/api/src/modules/tool-call/tool-call.module.ts (2)
apps/api/src/modules/action/action.module.ts (1)
Module(9-15)apps/api/src/modules/skill/skill.module.ts (1)
Module(35-81)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (1)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.ts (1)
TOOL_USE_TAG_RENDER(5-5)
apps/api/src/modules/action/action.module.ts (2)
apps/api/src/modules/skill/skill.module.ts (1)
Module(35-81)apps/api/src/modules/tool-call/tool-call.module.ts (1)
Module(5-10)
packages/openapi-schema/src/types.gen.ts (3)
packages/agent-tools/src/base.ts (1)
ToolCallResult(4-25)packages/ai-workspace-common/src/requests/types.gen.ts (1)
status2(5348-5348)packages/request/src/requests/types.gen.ts (1)
status2(5348-5348)
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (2)
packages/openapi-schema/src/types.gen.ts (1)
ActionStep(1634-1669)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (2)
parseToolCallFromChunk(439-497)mergeToolCallById(502-514)
apps/api/src/modules/skill/skill-invoker.service.ts (1)
apps/api/src/modules/tool-call/tool-call.service.ts (1)
persistToolCall(219-284)
apps/api/src/modules/action/action.dto.ts (3)
packages/openapi-schema/src/types.gen.ts (2)
ActionStep(1634-1669)ToolCallResult(3187-3244)packages/utils/src/typesafe.ts (1)
pick(3-9)packages/ai-workspace-common/src/utils/parse.ts (1)
safeParseJSON(1-10)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.ts (1)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (1)
TOOL_USE_TAG_RENDER(5-5)
packages/utils/src/step.ts (1)
packages/openapi-schema/src/types.gen.ts (1)
ToolCallResult(3187-3244)
apps/api/src/modules/tool-call/tool-call.service.ts (4)
apps/api/src/modules/action/action.service.ts (1)
Injectable(13-303)apps/api/src/modules/skill/skill-invoker.service.ts (1)
Injectable(58-1034)packages/openapi-schema/src/types.gen.ts (5)
type(748-748)ActionStepMeta(1602-1607)status(297-297)ActionResult(1726-1837)ActionStatus(1547-1547)apps/api/src/utils/response.ts (1)
writeSSEResponse(11-15)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx (1)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.ts (1)
parseToolCallStatus(8-17)
🪛 ast-grep (0.39.6)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts
[warning] 7-7: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(<${TOOL_USE_TAG}[^>]*>([\\s\\S]*?)</${TOOL_USE_TAG}>, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (22)
packages/ai-workspace-common/src/components/canvas/node-preview/skill-response/index.tsx (1)
186-207: Status text effect misses actionMeta in deps.If the skill changes without a status/steps change, text may desync. Add actionMeta?.name.
- }, [result?.status, steps, t]); + }, [result?.status, steps, t, actionMeta?.name]);apps/api/src/modules/skill/skill.module.ts (1)
33-50: Wiring ToolCallModule into SkillModule looks correct.Exports/imports pattern aligns with Nest DI; no issues spotted.
apps/api/src/modules/action/action.module.ts (1)
9-13: ActionModule now depends on ToolCallModule — OK.Dependency graph remains sane alongside forwardRef(SkillModule).
If ActionService injects ToolCallService, ensure ToolCallModule is not also provided elsewhere to avoid duplicate providers in tests.
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.ts (2)
1-6: LGTM! Clean enum definition.The ToolCallStatus enum is well-structured with clear status values that align with the backend schema. String literals correctly use single quotes as per coding guidelines.
14-15: Consider semantic correctness of default behavior.The function defaults to
EXECUTINGfor any undefined or unrecognized input. While this may be intentional for progressive loading scenarios, consider whetherundefinedornullinput should explicitly default toEXECUTINGor potentially throw an error for invalid values.Please verify that defaulting to
EXECUTINGfor all unknown/undefined values aligns with the intended UI behavior, especially when tool call data hasn't been loaded yet.packages/utils/src/step.ts (2)
86-102: Good defensive array handling and deduplication logic.The tool call merging logic correctly:
- Uses
Array.isArray()checks before operations- Applies nullish coalescing (
??) for defaults- Deduplicates by
callIdusing a Map- Handles missing
callIdgracefully- Uses optional chaining (
?.) appropriatelyOne minor observation: Line 98 uses
?? {}as a fallback for a non-existent entry. While this works, the empty object might not fully satisfyToolCallResulttype constraints, though TypeScript's structural typing likely accepts it with the spread.
104-116: LGTM! Clean integration of toolCalls field.The
toolCallsfield is properly integrated into the mergedActionStepobject alongside other metadata fields. The field will beundefinedwhen no tool calls exist, which is consistent with the optional nature oftoolCallsin theActionSteptype.packages/openapi-schema/src/types.gen.ts (3)
3181-3185: LGTM! SkillEvent extension for tool call tracking.The
toolCallResultfield is properly added toSkillEventas an optional field with appropriate JSDoc documentation. This enables streaming of tool call status during skill execution.
3187-3244: Well-structured ToolCallResult type definition.The type definition is comprehensive with appropriate field types and documentation. The flexible
input/outputtypes (object | string) accommodate various tool response formats.Note: There's a naming collision with
ToolCallResultinpackages/agent-tools/src/base.ts, which has a different structure (status: 'success' | 'error'vsstatus: 'executing' | 'completed' | 'failed'). While both serve different purposes (internal tool execution vs public API representation), this could cause confusion. Consider namespacing or renaming one of them in future refactors if they're used in overlapping contexts.
1665-1668: LGTM! ActionStep extended with toolCalls tracking.The
toolCallsfield is appropriately added as an optional array ofToolCallResult, enabling steps to track multiple tool invocations. This aligns with the PR objective to track tool interactions within actions.apps/api/src/modules/skill/skill-invoker.service.ts (1)
282-289: Double enqueueing syncRequestUsageYou add 'syncRequestUsage' at start and again in finally; likely double counts.
Decide on one place (start or end) and remove the duplicate, or make the two intents distinct (e.g., "request_started" vs "request_finished") to avoid overcounting.
Also applies to: 854-864
packages/openapi-schema/src/schemas.gen.ts (4)
2179-2186: ActionStep.toolCalls addition looks goodThe step now exposes tool calls; aligns with backend usage.
2350-2356: ActionResult.toolCalls surface approvedAction-level aggregation of tool calls is useful for summaries.
4414-4420: SkillEvent.toolCallResult field addition LGTMEnables streaming single-call updates.
4421-4490: ToolCallResult schema shape is sensible; confirm enum mappingStatus enum uses lowercase strings. Ensure backend enum-to-JSON mapping matches ('executing' | 'completed' | 'failed').
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (1)
209-211: Tool call sync on stream: good integrationParsing tool_use XML per chunk and merging by callId is appropriate.
apps/api/src/modules/action/action.service.ts (2)
63-69: Read handler mutates state (deriveAndUpdateActionStatus)getActionResult triggers a DB write via deriveAndUpdateActionStatus; unexpected side effect in a GET.
Consider moving status reconciliation to the write/stream path or running it asynchronously (queue) to keep GET idempotent and low-latency.
26-27: ToolCallService injection: LGTMWiring looks correct; improves step enrichment.
packages/ai-workspace-common/src/components/canvas/nodes/skill-response.tsx (1)
412-432: Version incrementation looks correct; ensure server expectsversionin payload.Client updates metadata and passes
version: nextVersionto invokeAction. Verify backend uses this to write ActionResult/ToolCallResult versions.apps/api/src/modules/action/action.dto.ts (1)
27-36: DTO for steps: good; verifytoolCallsordering and dedup policy.If multiple tool calls share callId, ensure upstream uses merge semantics and stable sort (by createdAt).
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx (1)
110-115: Result content prioritization is fine; ensure i18n for error text elsewhere.No change required; note that hasResult computes from error/result consistently.
apps/api/src/modules/tool-call/tool-call.service.ts (1)
249-283: Reconsider the key constraint strategy; callId is already globally uniqueThe original concern about collisions is not valid:
callIdis already defined as globally unique in the schema (@unique), so the current upsert key is safe and will reliably find the correct record without overwriting unrelated records.However, two valid concerns remain:
- The update block mutates
resultIdandversion(identity fields), which is semantically problematic even though callId uniqueness prevents collisions.- Error is unconditionally set (
error: data.error ?? undefined), clearing prior errors when undefined is provided.Since
callIdis globally unique, you may keep the current where clause as-is. Focus instead on preventing identity field mutations and conditional error updates:update: { - resultId: ids.resultId, - version: ids.version, - uid: data.uid ?? '', - toolsetId: data.toolsetId ?? '', - toolName: data.toolName ?? '', - stepName: data.stepName ?? undefined, + // do not update identity fields (resultId, version) + // only update optional fields when explicitly provided + ...(data.uid !== undefined ? { uid: data.uid } : {}), + ...(data.toolsetId !== undefined ? { toolsetId: data.toolsetId } : {}), + ...(data.toolName !== undefined ? { toolName: data.toolName } : {}), + ...(data.stepName !== undefined ? { stepName: data.stepName } : {}), // only overwrite when a new value is provided ...(data.input !== undefined ? { input: toJSONStr(data.input) } : {}), ...(data.output !== undefined ? { output: toJSONStr(data.output) } : {}), status: data.status ?? ToolCallStatus.EXECUTING, - error: data.error ?? undefined, + ...(data.error !== undefined ? { error: data.error } : {}), updatedAt: new Date(data.updatedAt), deletedAt: toDateOrNull(data.deletedAt), },Likely an incorrect or invalid review comment.
| callId String @unique @map("call_id") | ||
| /// UID |
There was a problem hiding this comment.
Global unique callId can collide across results/versions; prefer composite unique.
run_id may repeat across separate results or retries. Make uniqueness scoped per result/version to avoid constraint violations.
Apply:
- callId String @unique @map("call_id")
+ callId String @map("call_id")
+ @@unique([resultId, version, callId])📝 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.
| callId String @unique @map("call_id") | |
| /// UID | |
| callId String @map("call_id") | |
| @@unique([resultId, version, callId]) | |
| /// UID |
🤖 Prompt for AI Agents
In apps/api/prisma/schema.prisma around lines 1533-1534, remove the global
@unique on callId and instead enforce uniqueness scoped to the result/version by
adding a composite unique constraint at the model level (e.g., @@unique([callId,
result_id]) or @@unique([callId, version]) depending on which column represents
the result/version) so run_id/retries won't collide; also delete the @unique
attribute from the callId field and ensure the chosen secondary column name
matches the model's field.
| /// Status: enum | ||
| status String @default("executing") @map("status") | ||
| /// Error message |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Status is a free-form String; enforce enum for integrity.
Define a Prisma enum ('executing' | 'completed' | 'failed') to prevent invalid states and align with OpenAPI.
+enum ToolCallStatus {
+ executing
+ completed
+ failed
+}
...
- status String @default("executing") @map("status")
+ status ToolCallStatus @default(executing) @map("status")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/api/prisma/schema.prisma around lines 1547 to 1549, the status field is
currently a free-form String; change it to use a Prisma enum to enforce allowed
values. Add a Prisma enum declaration (executing, completed, failed) in the
schema, update the model's status field type from String to that enum, keep the
@default("executing") but switch to the enum default (executing) and retain
@map("status") so the DB column name stays the same; after schema change run
prisma migrate to apply the enum to the database.
| export function toolCallResultPO2DTO(toolCall: ToolCallResultModel): ToolCallResult { | ||
| return { | ||
| callId: toolCall.callId, | ||
| uid: toolCall.uid, | ||
| toolsetId: toolCall.toolsetId, | ||
| toolName: toolCall.toolName, | ||
| input: safeParseJSON(toolCall.input || '{}'), | ||
| output: safeParseJSON(toolCall.output || '{}'), | ||
| error: toolCall.error || '', | ||
| status: toolCall.status as 'executing' | 'completed' | 'failed', | ||
| createdAt: toolCall.createdAt.getTime(), | ||
| updatedAt: toolCall.updatedAt.getTime(), | ||
| deletedAt: toolCall.deletedAt?.getTime(), | ||
| }; |
There was a problem hiding this comment.
Map stepName and preserve raw input/output strings when JSON parse fails.
Currently stepName is dropped; safeParseJSON returns undefined on invalid JSON, losing original content.
export function toolCallResultPO2DTO(toolCall: ToolCallResultModel): ToolCallResult {
return {
callId: toolCall.callId,
uid: toolCall.uid,
toolsetId: toolCall.toolsetId,
toolName: toolCall.toolName,
- input: safeParseJSON(toolCall.input || '{}'),
- output: safeParseJSON(toolCall.output || '{}'),
+ input: safeParseJSON(toolCall.input, () => toolCall.input) ?? toolCall.input ?? undefined,
+ output: safeParseJSON(toolCall.output, () => toolCall.output) ?? toolCall.output ?? undefined,
error: toolCall.error || '',
status: toolCall.status as 'executing' | 'completed' | 'failed',
+ stepName: (toolCall as any).stepName, // or add to generated model type if available
createdAt: toolCall.createdAt.getTime(),
updatedAt: toolCall.updatedAt.getTime(),
deletedAt: toolCall.deletedAt?.getTime(),
};
}Based on openapi schema.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/api/src/modules/action/action.dto.ts around lines 38 to 51, the DTO
mapping drops stepName and loses raw input/output when safeParseJSON returns
undefined; update the mapper to include toolCall.stepName -> stepName and change
input/output handling to try JSON.parse and on failure keep the original string
(e.g., parsedInput = tryParse(toolCall.input) ?? toolCall.input ?? '{}'), so the
DTO preserves the raw string if parsing fails while still returning parsed
objects when valid.
| import { DirectConnection } from '@hocuspocus/server'; | ||
| import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/dist/messages'; | ||
| import { AIMessage, HumanMessage } from '@langchain/core/messages'; | ||
| import { InjectQueue } from '@nestjs/bullmq'; |
There was a problem hiding this comment.
Use public LangChain entrypoints, not internal /dist paths
Importing from '@langchain/core/dist/messages' is brittle and may break on upgrades.
Apply:
-import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/dist/messages';
+import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/messages';📝 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.
| import { DirectConnection } from '@hocuspocus/server'; | |
| import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/dist/messages'; | |
| import { AIMessage, HumanMessage } from '@langchain/core/messages'; | |
| import { InjectQueue } from '@nestjs/bullmq'; | |
| import { DirectConnection } from '@hocuspocus/server'; | |
| import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/messages'; | |
| import { AIMessage, HumanMessage } from '@langchain/core/messages'; | |
| import { InjectQueue } from '@nestjs/bullmq'; |
🤖 Prompt for AI Agents
In apps/api/src/modules/skill/skill-invoker.service.ts around lines 3-6, the
file imports message types from the internal path
'@langchain/core/dist/messages' which is brittle; replace that import with the
package's public entrypoint (import the needed types — AIMessageChunk,
BaseMessage, MessageContentComplex and any message classes — from the documented
public export of LangChain, e.g. the top-level package export such as
'@langchain/core' or the library's documented schema entrypoint), and
consolidate any message imports so no code imports from '/dist' paths.
| const toJSONStr = (value: unknown): string => { | ||
| if (value === undefined || value === null) return undefined; | ||
| try { | ||
| return typeof value === 'string' ? value : JSON.stringify(value); | ||
| } catch { | ||
| return ''; | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Fix compile-time bug: toJSONStr returns undefined despite string return type
toJSONStr declares a string return but returns undefined for nullish input. This will fail type-checking and can propagate empty writes unexpectedly.
Apply:
- const toJSONStr = (value: unknown): string => {
- if (value === undefined || value === null) return undefined;
+ const toJSONStr = (value: unknown): string => {
+ if (value === undefined || value === null) return '';
try {
return typeof value === 'string' ? value : JSON.stringify(value);
} catch {
return '';
}
};🤖 Prompt for AI Agents
In apps/api/src/modules/tool-call/tool-call.service.ts around lines 236-244,
toJSONStr is declared to return string but returns undefined for null/undefined
inputs; change the function signature to return string | undefined (or
alternatively return an empty string) and keep the nullish branch returning
undefined, then update any callers to handle the possible undefined (e.g.,
coalesce to '' or skip writing) so types align and no compile-time error occurs.
| const existing = firstByCallId.get(callId); | ||
| if (!existing) { | ||
| firstByCallId.set(callId, entry); | ||
| } else { | ||
| // Merge properties from later node into the first one, preferring non-empty values | ||
| const targetProps = existing.node.properties ?? {}; | ||
| const sourceProps = entry.node.properties ?? {}; | ||
| for (const key of Object.keys(sourceProps)) { | ||
| const val = sourceProps[key]; | ||
| if (val !== undefined && val !== null && String(val).trim() !== '') { | ||
| targetProps[key] = val; | ||
| } | ||
| } | ||
| existing.node.properties = targetProps; | ||
| // Mark the later duplicate for removal | ||
| toRemove.push({ index: entry.index, parent: entry.parent }); | ||
| } |
There was a problem hiding this comment.
“Non-empty” check will treat {} and [] as non-empty (stringified), overwriting useful props.
String(val).trim() is truthy for objects/arrays, causing unintended overwrites.
Use a semantic isMeaningful guard:
- const val = sourceProps[key];
- if (val !== undefined && val !== null && String(val).trim() !== '') {
- targetProps[key] = val;
- }
+ const val = sourceProps[key];
+ const isMeaningful =
+ (typeof val === 'string' && val.trim().length > 0) ||
+ (Array.isArray(val) && val.length > 0) ||
+ (val && typeof val === 'object' && Object.keys(val).length > 0) ||
+ typeof val === 'number' ||
+ typeof val === 'boolean';
+ if (isMeaningful) {
+ targetProps[key] = val;
+ }📝 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 existing = firstByCallId.get(callId); | |
| if (!existing) { | |
| firstByCallId.set(callId, entry); | |
| } else { | |
| // Merge properties from later node into the first one, preferring non-empty values | |
| const targetProps = existing.node.properties ?? {}; | |
| const sourceProps = entry.node.properties ?? {}; | |
| for (const key of Object.keys(sourceProps)) { | |
| const val = sourceProps[key]; | |
| if (val !== undefined && val !== null && String(val).trim() !== '') { | |
| targetProps[key] = val; | |
| } | |
| } | |
| existing.node.properties = targetProps; | |
| // Mark the later duplicate for removal | |
| toRemove.push({ index: entry.index, parent: entry.parent }); | |
| } | |
| const existing = firstByCallId.get(callId); | |
| if (!existing) { | |
| firstByCallId.set(callId, entry); | |
| } else { | |
| // Merge properties from later node into the first one, preferring non-empty values | |
| const targetProps = existing.node.properties ?? {}; | |
| const sourceProps = entry.node.properties ?? {}; | |
| for (const key of Object.keys(sourceProps)) { | |
| const val = sourceProps[key]; | |
| const isMeaningful = | |
| (typeof val === 'string' && val.trim().length > 0) || | |
| (Array.isArray(val) && val.length > 0) || | |
| (val && typeof val === 'object' && Object.keys(val).length > 0) || | |
| typeof val === 'number' || | |
| typeof val === 'boolean'; | |
| if (isMeaningful) { | |
| targetProps[key] = val; | |
| } | |
| } | |
| existing.node.properties = targetProps; | |
| // Mark the later duplicate for removal | |
| toRemove.push({ index: entry.index, parent: entry.parent }); | |
| } |
| const status = (attrs['data-tool-call-status'] ?? ToolCallStatus.FAILED.toString()) as | ||
| | 'executing' | ||
| | 'completed' | ||
| | 'failed'; | ||
|
|
There was a problem hiding this comment.
Status normalization is inconsistent; defaulting to ToolCallStatus.FAILED.toString() risks casing mismatch.
OpenAPI/UI expect lowercase ('executing'|'completed'|'failed'). Default should be computed from presence of result/error, not enum.toString().
- // the status is always failed if the result is not present
- const status = (attrs['data-tool-call-status'] ?? ToolCallStatus.FAILED.toString()) as
- | 'executing'
- | 'completed'
- | 'failed';
+ const rawStatus = attrs['data-tool-call-status']?.toLowerCase?.();
+ const hasResult = !!attrs['data-tool-result'];
+ const hasError = !!attrs['data-tool-error'];
+ const status: 'executing' | 'completed' | 'failed' =
+ rawStatus === 'completed' ? 'completed' :
+ rawStatus === 'failed' ? 'failed' :
+ rawStatus === 'executing' ? 'executing' :
+ hasResult ? 'completed' :
+ hasError ? 'failed' : 'executing';🤖 Prompt for AI Agents
In
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts
around lines 457-461, the status default uses ToolCallStatus.FAILED.toString()
which can produce a casing mismatch and isn’t derived from the node data;
instead determine the default status from the presence of
attrs['data-tool-call-result'] (=> 'completed') or attrs['data-tool-call-error']
(=> 'failed'), otherwise 'executing', then normalize the final value to a
lowercase string ('executing'|'completed'|'failed') before casting/using it so
it matches the OpenAPI/UI expectations.
| const argsStr = attrs['data-tool-arguments']; | ||
| const resultStr = attrs['data-tool-result']; | ||
|
|
||
| const parsed: ParsedToolCall = { | ||
| callId, | ||
| status, | ||
| createdAt, | ||
| updatedAt, | ||
| toolsetId: attrs['data-tool-toolset-key'] ?? undefined, | ||
| toolName: attrs['data-tool-name'] ?? undefined, | ||
| stepName, | ||
| input: safeParse(argsStr) as Record<string, unknown> | undefined, | ||
| output: safeParse(resultStr), | ||
| error: status === 'failed' ? String(resultStr ?? '') : undefined, | ||
| uid: undefined, | ||
| }; |
There was a problem hiding this comment.
Bug: error is taken from resultStr; should use parsed <error> content.
This mislabels successful results as errors and hides real error messages.
- const resultStr = attrs['data-tool-result'];
+ const resultStr = attrs['data-tool-result'];
+ const errorStr = attrs['data-tool-error'];
...
const parsed: ParsedToolCall = {
callId,
status,
createdAt,
updatedAt,
toolsetId: attrs['data-tool-toolset-key'] ?? undefined,
toolName: attrs['data-tool-name'] ?? undefined,
stepName,
input: safeParse(argsStr) as Record<string, unknown> | undefined,
output: safeParse(resultStr),
- error: status === 'failed' ? String(resultStr ?? '') : undefined,
+ error: status === 'failed' ? String(errorStr ?? '') : undefined,
uid: undefined,
};🤖 Prompt for AI Agents
In
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts
around lines 479 to 494, the code sets error from resultStr which mislabels
normal results as errors; change the error assignment to read the actual error
payload (e.g. attrs['data-tool-error'] or the parsed <error> content) instead of
resultStr, parse it with safeParse and coerce to string only when status ===
'failed' so parsed.error contains the real error message or undefined otherwise.
| export const parseToolCallStatus = (val?: string): ToolCallStatus => { | ||
| switch (val) { | ||
| case ToolCallStatus.COMPLETED: | ||
| return ToolCallStatus.COMPLETED; | ||
| case ToolCallStatus.FAILED: | ||
| return ToolCallStatus.FAILED; | ||
| default: | ||
| return ToolCallStatus.EXECUTING; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add JSDoc comments for the function.
The function is missing JSDoc documentation which is required per coding guidelines for JavaScript/TypeScript functions.
As per coding guidelines.
Apply this diff to add JSDoc:
+/**
+ * Parses a string value to a ToolCallStatus enum value.
+ * @param val - Optional status string to parse
+ * @returns Parsed ToolCallStatus, defaults to EXECUTING for unknown values
+ */
export const parseToolCallStatus = (val?: string): ToolCallStatus => {
switch (val) {
case ToolCallStatus.COMPLETED:
return ToolCallStatus.COMPLETED;
case ToolCallStatus.FAILED:
return ToolCallStatus.FAILED;
default:
return ToolCallStatus.EXECUTING;
}
};📝 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 parseToolCallStatus = (val?: string): ToolCallStatus => { | |
| switch (val) { | |
| case ToolCallStatus.COMPLETED: | |
| return ToolCallStatus.COMPLETED; | |
| case ToolCallStatus.FAILED: | |
| return ToolCallStatus.FAILED; | |
| default: | |
| return ToolCallStatus.EXECUTING; | |
| } | |
| }; | |
| /** | |
| * Parses a string value to a ToolCallStatus enum value. | |
| * @param val - Optional status string to parse | |
| * @returns Parsed ToolCallStatus, defaults to EXECUTING for unknown values | |
| */ | |
| export const parseToolCallStatus = (val?: string): ToolCallStatus => { | |
| switch (val) { | |
| case ToolCallStatus.COMPLETED: | |
| return ToolCallStatus.COMPLETED; | |
| case ToolCallStatus.FAILED: | |
| return ToolCallStatus.FAILED; | |
| default: | |
| return ToolCallStatus.EXECUTING; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.ts
around lines 8 to 17, the parseToolCallStatus function lacks the required JSDoc
block; add a concise JSDoc comment immediately above the function that describes
its purpose, documents the parameter (val?: string) and its possible values,
specifies the return type (ToolCallStatus), and notes the default behavior when
val is unrecognized (returns ToolCallStatus.EXECUTING), following project JSDoc
style.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/api/src/modules/skill/skill-invoker.service.ts (3)
4-6: Use public LangChain entrypoints and type-only importsImporting from '@langchain/core/dist/messages' is brittle. Switch to the public entrypoint and mark type-only imports. Also keep value imports (AIMessage, HumanMessage) as runtime imports.
Apply:
-import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/dist/messages'; -import { AIMessage, HumanMessage } from '@langchain/core/messages'; +import type { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/messages'; +import { AIMessage, HumanMessage } from '@langchain/core/messages';Based on learnings.
690-692: Re-enable stream suppression during tool executionPrevents tool output from duplicating in the chat stream.
- // if (event?.metadata?.langgraph_node === 'tools') { - // break; - // } + if (event?.metadata?.langgraph_node === 'tools') { + break; + }
583-614: Guard against empty toolCallId before persistingAvoid upserting with empty callId by validating and fixing it once.
const persistToolCall = async ( status: ToolCallStatus, data: { input: string | undefined; output: string | undefined; errorMessage?: string; }, ) => { + if (!toolCallId) { + this.logger.warn('Missing toolCallId; generating fallback'); + toolCallId = randomUUID(); + } const toolsetId = String(event.metadata?.toolsetKey ?? ''); const toolName = String(event.metadata?.name ?? '');
🧹 Nitpick comments (3)
apps/api/src/modules/skill/skill-invoker.service.ts (3)
312-321: Type config reads and add default for streamIdleTimeoutEnsure numeric type and sane default.
- const streamIdleTimeout = this.config.get('skill.streamIdleTimeout'); + const streamIdleTimeout = this.config.get<number>('skill.streamIdleTimeout', 45000); @@ - if (!streamIdleTimeout || streamIdleTimeout <= 0) { + if (streamIdleTimeout <= 0) {As per coding guidelines.
13-18: Use type-only imports for schema typesThese are types at compile time; prefer import type.
-import { +import type { ActionResult, ActionStep, Artifact, CreditBilling, SkillEvent, TokenUsageItem, User, ProviderItem, } from '@refly/openapi-schema';As per coding guidelines.
109-115: Removeanywith a narrow type guard for vision capabilityAvoid
(providerItem?.config as any). Use a guard.type ProviderConfig = { capabilities?: { vision?: boolean } }; const hasVision = (cfg: unknown): cfg is ProviderConfig => !!cfg && typeof cfg === 'object' && 'capabilities' in (cfg as any); const canSee = hasVision(providerItem?.config) && !!providerItem.config.capabilities?.vision; if (result.input?.images?.length > 0 && canSee) { // ... }As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/modules/skill/skill-invoker.service.ts(4 hunks)packages/i18n/src/en-US/ui.ts(1 hunks)packages/i18n/src/zh-Hans/ui.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/i18n/src/en-US/ui.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,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.Use JSDoc style comments for functions and classes in JavaScript/TypeScript
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals
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
Validate object properties before destructuring
Use ES6+ features like arrow functions, destructuring, and spread operators
Use async/await instead of raw promises for asynchronous code
**/*.{js,jsx,ts,tsx}: Maximum line length of 100 characters
Use 2 spaces for indentation, no tabs
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
No trailing whitespace at the end of lines
**/*.{js,jsx,ts,tsx}: Each file should contain only one main component
File names should match the component name
Keep component files focused with minimal dependencies
Group import statements in the following order: 1. React/framework libraries 2. Third-party libraries 3. Internal modules 4. Relative path imports (parent directories first, then child directories) 5. Type imports 6. Style imports
Sort imports alphabetically within each group
Leave a blank line between import groups
Components should be organized in the following order: 1. Import statements 2. Type definitions 3. Constant declarations 4. Component function 5. Hook calls 6. Event handlers 7. Helper render functions 8. JSX return statement
Extract complex logic in...
Files:
apps/api/src/modules/skill/skill-invoker.service.tspackages/i18n/src/zh-Hans/ui.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - it defeats the purpose of TypeScript
When tempted to useany, useunknowntype instead and add proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new ones
Use TypeScript's utility types to derive new types:Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>
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 typesExplicitly type props interfaces/types
Files:
apps/api/src/modules/skill/skill-invoker.service.tspackages/i18n/src/zh-Hans/ui.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
Follow NestJS patterns for backend code
Files:
apps/api/src/modules/skill/skill-invoker.service.tspackages/i18n/src/zh-Hans/ui.ts
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}: Use Primary color (#155EEF) as the main brand color for buttons, links, and accents
Use Error color (#F04438) for error states and destructive actions
Use Success color (#12B76A) for success states and confirmations
Use Warning color (#F79009) for warnings and important notifications
Use Info color (#0BA5EC) for informational elements
Files:
apps/api/src/modules/skill/skill-invoker.service.tspackages/i18n/src/zh-Hans/ui.ts
**/*.{css,scss,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{css,scss,less,styl,js,jsx,ts,tsx}: 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
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
apps/api/src/modules/skill/skill-invoker.service.tspackages/i18n/src/zh-Hans/ui.ts
packages/i18n/src/{en-US,zh-Hans}/**
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
packages/i18n/src/{en-US,zh-Hans}/**: Add new translation keys to both language files: packages/i18n/src/en-US and packages/i18n/src/zh-Hans
Maintain consistency in naming conventions for translation keys
Use descriptive keys that reflect the content in translation files
Group related translation keys together
Use namespaces for different sections of the application in translation keys
Follow a hierarchical structure for nested components in translation keys
Support dynamic content with placeholders in translation strings
Files:
packages/i18n/src/zh-Hans/ui.ts
🧠 Learnings (1)
📚 Learning: 2025-07-21T02:58:52.235Z
Learnt from: CR
PR: refly-ai/refly#0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-07-21T02:58:52.235Z
Learning: Applies to packages/i18n/src/{en-US,zh-Hans}/** : Add new translation keys to both language files: packages/i18n/src/en-US and packages/i18n/src/zh-Hans
Applied to files:
packages/i18n/src/zh-Hans/ui.ts
🧬 Code graph analysis (1)
apps/api/src/modules/skill/skill-invoker.service.ts (1)
apps/api/src/modules/tool-call/tool-call.service.ts (1)
persistToolCall(219-284)
⏰ 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 (4)
packages/i18n/src/zh-Hans/ui.ts (1)
2947-2948: LGTM! Both language files properly updated.The Chinese translations for
parametersandresultare accurate and properly formatted. The corresponding English translations are confirmed to exist at lines 3000-3001 inpackages/i18n/src/en-US/ui.tswith matching structure and key names. Both files follow the required coding guidelines for maintaining parallel translation keys.apps/api/src/modules/skill/skill-invoker.service.ts (3)
31-31: Add UUID fallback for tool call IDsYou’ll need a safe fallback ID when run_id is missing. Import randomUUID once.
[raise_recommended_refactor]
import { EventEmitter } from 'node:events'; +import { randomUUID } from 'node:crypto';
694-708: Confirm extractChunkContent handles undefined chunkchunk can be undefined; ensure helper is defensive to avoid runtime errors.
If not, guard:
- const { content, reasoningContent } = extractChunkContent(chunk); + const { content, reasoningContent } = chunk ? extractChunkContent(chunk) : { content: '', reasoningContent: '' };
286-295: Incorrect review comment — syncRequestUsage is idempotent and does not double-countThe consumer implementation shows
syncRequestUsagerecalculates the total request count from theactionResulttable and updates (not increments) the meter with that total. This design is fully idempotent—multiple job invocations produce identical results and do not compound usage counts. The double enqueue is redundant but does not cause the double-counting concern raised.Likely an incorrect or invalid review comment.
| import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/dist/messages'; | ||
| import { AIMessage, HumanMessage } from '@langchain/core/messages'; | ||
| import { InjectQueue } from '@nestjs/bullmq'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify MessageContentComplex availability from public export
If MessageContentComplex isn’t publicly exported, switch to the correct public type (e.g., MessageContent).
🌐 Web query:
Is `MessageContentComplex` exported from '@langchain/core/messages' in LangChain JS v0.3.x? If not, what is the correct public type for complex (text + image_url) message content?
💡 Result:
Short answer: No — MessageContentComplex is an internal/type-only alias and is not exported from the public entrypoint @langchain/core/messages in v0.3.x. Use the public MessageContent type (which accepts a string or an array of complex content blocks, i.e. text + image_url blocks) for multimodal (text + image_url) message content. [1][2]
Sources:
- MessageContentComplex / MessageContent type declarations (langchain-core v0.3 API docs). [1]
- Module index for @langchain/core/messages showing the public exports (types are not re-exported from the entrypoint). [2]
Use public MessageContent type instead of internal MessageContentComplex
MessageContentComplex is an internal/type-only alias and is not exported from the public entrypoint @langchain/core/messages in v0.3.x. Replace the import with the public MessageContent type (which accepts a string or an array of complex content blocks, i.e. text + image_url blocks):
import { AIMessageChunk, BaseMessage, MessageContent } from '@langchain/core/messages';
import { AIMessage, HumanMessage } from '@langchain/core/messages';
import { InjectQueue } from '@nestjs/bullmq';Also consolidate the LangChain imports into a single statement from the public entrypoint (not /dist/messages).
🤖 Prompt for AI Agents
In apps/api/src/modules/skill/skill-invoker.service.ts around lines 4 to 6, the
code imports MessageContentComplex from the internal path
'@langchain/core/dist/messages' — replace that with the public MessageContent
type and consolidate LangChain imports to the public entrypoint; specifically,
import AIMessageChunk, BaseMessage, and MessageContent from
'@langchain/core/messages' (and keep AIMessage, HumanMessage from the same
single import) instead of using the /dist path and the internal alias.
| // tool callId, now we use first time returned run_id as tool call id | ||
| let toolCallId = ''; | ||
| const startTs = Date.now(); | ||
| for await (const event of skill.streamEvents(input, { |
There was a problem hiding this comment.
Per-call timestamps and safe toolCallId fallback
startTs is captured once and reused; on concurrent/serial tools this skews createdAt and XML. Also run_id can be empty. Track per-call toolStartTs and generate a fallback ID.
- // tool callId, now we use first time returned run_id as tool call id
- let toolCallId = '';
- const startTs = Date.now();
+ // Per-call tool identifiers and timestamps
+ let toolCallId = '';
+ let toolStartTs = 0;
@@
- const buildToolUseXML = (includeResult: boolean, errorMsg: string, updatedTs: number) =>
+ const buildToolUseXML = (includeResult: boolean, errorMsg: string, updatedTs: number) =>
this.toolCallService.generateToolUseXML({
toolCallId,
includeResult,
errorMsg,
metadata: {
name: event.metadata?.name,
type: event.metadata?.type as string | undefined,
toolsetKey: event.metadata?.toolsetKey,
toolsetName: event.metadata?.toolsetName,
},
input: event.data?.input,
output: event.data?.output,
- startTs: startTs,
+ startTs: toolStartTs,
updatedTs: updatedTs,
});
@@
- const createdAt = startTs;
+ const createdAt = toolStartTs || Date.now();
const updatedAt = Date.now();
@@
- if (event.event === 'on_tool_start') {
- toolCallId = String(event.run_id ?? '');
+ if (event.event === 'on_tool_start') {
+ toolCallId = String(event.run_id ?? '') || randomUUID();
+ toolStartTs = Date.now();
// Persist tool call result
await persistToolCall(ToolCallStatus.EXECUTING, {
input: event.data?.input,
output: '',
});Also applies to: 566-581, 596-611, 615-635
🤖 Prompt for AI Agents
In apps/api/src/modules/skill/skill-invoker.service.ts around lines 535-538 (and
similarly for 566-581, 596-611, 615-635): startTs is captured once outside the
per-tool-call loop which skews createdAt/XML when multiple tool calls occur, and
run_id may be empty so toolCallId can be blank; move the timestamp capture
inside the per-event/per-call handling (e.g., toolStartTs = Date.now() at the
start of each tool call), use that per-call timestamp for createdAt/XML, and
derive toolCallId from the first non-empty run_id for that call or generate a
safe fallback ID (timestamp+counter or UUID) when run_id is empty; apply the
same per-call timestamp and fallback-id logic to the other listed ranges.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/modules/skill/skill-invoker.service.ts (1)
535-624: Track tool-call timestamps per invocation
startTsis captured once outside the event loop, so every tool call (even ones that begin later) is persisted with the samecreatedAtand XML<createdAt>value. This makes timelines inaccurate and can invert durations when multiple tools fire. Please capture the timestamp when each tool actually starts—e.g., storeDate.now()keyed bytoolCallIdonon_tool_start, reuse it foron_tool_end/on_tool_error, and clear it after releasing the ID.- // tool callId, now we use first time returned run_id as tool call id - const startTs = Date.now(); + const toolStartTs = new Map<string, number>(); @@ - const buildToolUseXML = (includeResult: boolean, errorMsg: string, updatedTs: number) => + const buildToolUseXML = (includeResult: boolean, errorMsg: string, updatedTs: number) => { + const startTs = toolStartTs.get(toolCallId) ?? updatedTs; this.toolCallService.generateToolUseXML({ toolCallId, includeResult, errorMsg, metadata: { name: toolName, type: event.metadata?.type as string | undefined, toolsetKey: toolsetId, toolsetName: event.metadata?.toolsetName, }, input: event.data?.input, output: event.data?.output, - startTs: startTs, + startTs, updatedTs: updatedTs, }); + }; @@ if (event.event === 'on_tool_start') { + toolStartTs.set(toolCallId, Date.now()); await persistToolCall(ToolCallStatus.EXECUTING, { input: event.data?.input, output: '', }); @@ if (event.event === 'on_tool_error') { @@ this.toolCallService.releaseToolCallId({ resultId, version, toolName, toolsetId, runId, }); + toolStartTs.delete(toolCallId); break; } if (event.event === 'on_tool_end') { @@ this.toolCallService.releaseToolCallId({ resultId, version, toolName, toolsetId, runId, }); + toolStartTs.delete(toolCallId); break; }
♻️ Duplicate comments (2)
apps/api/src/modules/skill/skill-invoker.service.ts (1)
4-6: Use public LangChain exportsImporting from
@langchain/core/dist/messagespulls internal files that LangChain does not guarantee to keep stable. Please switch to the documented public entry point (@langchain/core/messages) and use the exportedMessageContenttype instead of the internalMessageContentComplex.-import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/dist/messages'; -import { AIMessage, HumanMessage } from '@langchain/core/messages'; +import { + AIMessage, + AIMessageChunk, + BaseMessage, + HumanMessage, + MessageContent, +} from '@langchain/core/messages';apps/api/src/modules/tool-call/tool-call.service.ts (1)
93-125: Escape tool-call payload before emitting XML
generateToolUseXMLwrites raw input/output/error directly into XML. Any<,&, quotes, or literal HTML coming from a tool will break the markup and exposes the renderer to XSS. Please add an XML escaping helper (and stringify non-strings safely) before interpolating values.- const lines: string[] = []; + const toStringSafe = (value: unknown): string => { + try { + return typeof value === 'string' ? value : JSON.stringify(value); + } catch { + return ''; + } + }; + const escapeXml = (raw: string): string => + raw + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + + const lines: string[] = []; @@ - lines.push('<arguments>'); - lines.push(input ? (typeof input === 'string' ? input : JSON.stringify(input)) : ''); - lines.push('</arguments>'); + lines.push('<arguments>'); + lines.push(escapeXml(toStringSafe(input))); + lines.push('</arguments>'); @@ - lines.push( - `<result>${output ? (typeof output === 'string' ? output : JSON.stringify(output)) : ''}</result>`, - ); - lines.push( - `<error>${errorMsg ? (typeof errorMsg === 'string' ? errorMsg : JSON.stringify(errorMsg)) : ''}</error>`, - ); + lines.push(`<result>${escapeXml(toStringSafe(output))}</result>`); + lines.push(`<error>${escapeXml(toStringSafe(errorMsg))}</error>`); @@ - lines.push( - `<result>${output ? (typeof output === 'string' ? output : JSON.stringify(output)) : ''}</result>`, - ); + lines.push(`<result>${escapeXml(toStringSafe(output))}</result>`);
🧹 Nitpick comments (1)
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (1)
223-225: Consider simplifying the tool_call event routing.The
onToolCallhandler is a thin wrapper that simply forwards toonSkillStream. SinceonSkillStreamalready callsupdateToolCallsFromXmlto parse tool_use XML from content, having a separateonToolCallcallback adds indirection without clear benefit.Consider either:
- Remove the separate
onToolCallhandler and let tool_call events go through the standardonSkillStreampath- If tool_call events require distinct processing, implement that logic here instead of just forwarding
Based on the current implementation, option 1 seems more appropriate unless there's a future plan to handle tool_call events differently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/api/src/modules/skill/skill-invoker.service.ts(4 hunks)apps/api/src/modules/tool-call/tool-call.service.ts(1 hunks)packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts(5 hunks)packages/ai-workspace-common/src/utils/sse-post.ts(3 hunks)packages/openapi-schema/schema.yml(4 hunks)packages/openapi-schema/src/schemas.gen.ts(4 hunks)packages/openapi-schema/src/types.gen.ts(5 hunks)packages/skill-template/src/base.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,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.Use JSDoc style comments for functions and classes in JavaScript/TypeScript
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals
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
Validate object properties before destructuring
Use ES6+ features like arrow functions, destructuring, and spread operators
Use async/await instead of raw promises for asynchronous code
**/*.{js,jsx,ts,tsx}: Maximum line length of 100 characters
Use 2 spaces for indentation, no tabs
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
No trailing whitespace at the end of lines
**/*.{js,jsx,ts,tsx}: Each file should contain only one main component
File names should match the component name
Keep component files focused with minimal dependencies
Group import statements in the following order: 1. React/framework libraries 2. Third-party libraries 3. Internal modules 4. Relative path imports (parent directories first, then child directories) 5. Type imports 6. Style imports
Sort imports alphabetically within each group
Leave a blank line between import groups
Components should be organized in the following order: 1. Import statements 2. Type definitions 3. Constant declarations 4. Component function 5. Hook calls 6. Event handlers 7. Helper render functions 8. JSX return statement
Extract complex logic in...
Files:
packages/skill-template/src/base.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/ai-workspace-common/src/utils/sse-post.tspackages/openapi-schema/src/schemas.gen.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool-call/tool-call.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - it defeats the purpose of TypeScript
When tempted to useany, useunknowntype instead and add proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new ones
Use TypeScript's utility types to derive new types:Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>
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 typesExplicitly type props interfaces/types
Files:
packages/skill-template/src/base.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/ai-workspace-common/src/utils/sse-post.tspackages/openapi-schema/src/schemas.gen.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool-call/tool-call.service.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
Follow NestJS patterns for backend code
Files:
packages/skill-template/src/base.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/ai-workspace-common/src/utils/sse-post.tspackages/openapi-schema/src/schemas.gen.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool-call/tool-call.service.ts
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}: Use Primary color (#155EEF) as the main brand color for buttons, links, and accents
Use Error color (#F04438) for error states and destructive actions
Use Success color (#12B76A) for success states and confirmations
Use Warning color (#F79009) for warnings and important notifications
Use Info color (#0BA5EC) for informational elements
Files:
packages/skill-template/src/base.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/ai-workspace-common/src/utils/sse-post.tspackages/openapi-schema/src/schemas.gen.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool-call/tool-call.service.ts
**/*.{css,scss,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{css,scss,less,styl,js,jsx,ts,tsx}: 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
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/skill-template/src/base.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/ai-workspace-common/src/utils/sse-post.tspackages/openapi-schema/src/schemas.gen.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool-call/tool-call.service.ts
🧬 Code graph analysis (6)
packages/skill-template/src/base.ts (3)
packages/openapi-schema/src/types.gen.ts (1)
SkillEvent(3123-3186)packages/ai-workspace-common/src/requests/types.gen.ts (1)
SkillEvent(3114-3173)packages/request/src/requests/types.gen.ts (1)
SkillEvent(3114-3173)
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (2)
packages/openapi-schema/src/types.gen.ts (3)
ActionStep(1634-1669)ActionStepMeta(1602-1607)SkillEvent(3123-3186)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (2)
parseToolCallFromChunk(439-497)mergeToolCallById(502-514)
packages/ai-workspace-common/src/utils/sse-post.ts (1)
packages/openapi-schema/src/types.gen.ts (1)
SkillEvent(3123-3186)
apps/api/src/modules/skill/skill-invoker.service.ts (1)
apps/api/src/modules/tool-call/tool-call.service.ts (1)
persistToolCall(269-334)
packages/openapi-schema/src/types.gen.ts (3)
packages/agent-tools/src/base.ts (1)
ToolCallResult(4-25)packages/ai-workspace-common/src/requests/types.gen.ts (1)
status2(5348-5348)packages/request/src/requests/types.gen.ts (1)
status2(5348-5348)
apps/api/src/modules/tool-call/tool-call.service.ts (3)
apps/api/src/modules/skill/skill-invoker.service.ts (1)
Injectable(59-1066)apps/api/src/modules/action/action.service.ts (1)
Injectable(13-303)packages/openapi-schema/src/types.gen.ts (5)
type(748-748)ActionStepMeta(1602-1607)status(297-297)ActionResult(1726-1837)ActionStatus(1547-1547)
⏰ 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 (4)
packages/skill-template/src/base.ts (1)
267-267: LGTM!The
tool_callevent addition toSkillEventMapis consistent with other event definitions and properly typed.packages/openapi-schema/src/types.gen.ts (3)
1665-1668: LGTM!The
toolCallsfield additions toActionStepandActionResultare properly typed and documented. The optional nature is appropriate since not all action steps will involve tool calls.Also applies to: 1801-1804
3120-3120: LGTM!The
tool_callevent type addition toSkillEventTypeand the correspondingtoolCallResultfield inSkillEventfollow the established pattern for event-specific data fields.Also applies to: 3182-3185
3188-3250: Verify consistency with other ToolCallResult definitions.The
ToolCallResulttype is well-defined with comprehensive fields for tracking tool call lifecycle. However, note thatpackages/agent-tools/src/base.tsalso defines aToolCallResultinterface with a different structure (status: 'success' | 'error' instead of 'executing' | 'completed' | 'failed').Ensure this naming doesn't cause confusion and that the two types serve distinct purposes (API/persistence vs. internal tool execution).
| if (skillEvent.event === 'tool_call') { | ||
| onToolCall?.(skillEvent); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Clarify tool_call event handling and add safeguard.
The current implementation immediately invokes onToolCall and bypasses batching with continue. This means:
- If
onToolCallis undefined: tool_call events are silently dropped (the optional chaining on line 265 returns undefined, thencontinueskips adding to batch) - No batching/throttling: Unlike other events, tool_call events are processed immediately
Consider whether this is intentional. If tool_call events need immediate processing, document why. If not, remove the continue to let them flow through normal batching:
if (skillEvent.event === 'tool_call') {
onToolCall?.(skillEvent);
- continue;
}Or, if immediate processing is required, provide a fallback:
if (skillEvent.event === 'tool_call') {
- onToolCall?.(skillEvent);
- continue;
+ if (onToolCall) {
+ onToolCall(skillEvent);
+ continue;
+ }
+ // Fallback: add to batch if no handler provided
}📝 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 (skillEvent.event === 'tool_call') { | |
| onToolCall?.(skillEvent); | |
| continue; | |
| } | |
| if (skillEvent.event === 'tool_call') { | |
| onToolCall?.(skillEvent); | |
| } |
| if (skillEvent.event === 'tool_call') { | |
| onToolCall?.(skillEvent); | |
| continue; | |
| } | |
| if (skillEvent.event === 'tool_call') { | |
| if (onToolCall) { | |
| onToolCall(skillEvent); | |
| continue; | |
| } | |
| // Fallback: add to batch if no handler provided | |
| } |
🤖 Prompt for AI Agents
In packages/ai-workspace-common/src/utils/sse-post.ts around lines 264 to 267,
tool_call events are currently handled by calling onToolCall?.(skillEvent) and
then continue, which both drops the event when onToolCall is undefined and
bypasses normal batching. Decide the intended behavior and implement one of two
fixes: (A) If tool_call must be processed immediately, explicitly call
onToolCall when present but always also push the event into the regular batch
queue (remove the continue) so it isn’t lost when the handler is undefined, and
add a short comment explaining immediate processing; or (B) If tool_call should
follow normal batching/throttling, remove the onToolCall invocation and the
continue so the event flows into the batch; in either case add a brief inline
comment documenting the choice and add a small test or runtime assertion to
ensure events are not silently dropped.
| # input can be structured object or raw string; keep it flexible | ||
| oneOf: | ||
| - type: object | ||
| - type: string | ||
| description: Tool input arguments | ||
| output: | ||
| # output can be structured object or raw string | ||
| oneOf: | ||
| - type: object | ||
| - type: string | ||
| description: Tool output result | ||
| error: | ||
| type: string | ||
| description: Error message if tool execution failed | ||
| status: | ||
| type: string | ||
| description: Tool call status | ||
| enum: | ||
| - executing | ||
| - completed | ||
| - failed | ||
| createdAt: | ||
| type: number | ||
| description: Tool call start timestamp (milliseconds) | ||
| updatedAt: |
There was a problem hiding this comment.
Allow arbitrary JSON payloads for tool call input/output.
The schema constrains input/output to object|string, but our tool runner already emits arrays, booleans, numbers, and nulls (e.g., web-search returns a result list, boolean toggles, numeric counters). Generated clients based on this spec will treat those perfectly valid responses as type violations. Please model these fields as “any JSON value” so client SDKs stay in sync with what the backend actually serializes.
Apply this diff to loosen the payload typing:
input:
- # input can be structured object or raw string; keep it flexible
- oneOf:
- - type: object
- - type: string
- description: Tool input arguments
+ $ref: '#/components/schemas/ToolCallPayload'
description: Tool input arguments
output:
- # output can be structured object or raw string
- oneOf:
- - type: object
- - type: string
- description: Tool output result
+ $ref: '#/components/schemas/ToolCallPayload'
+ description: Tool output resultAnd add this helper schema once under components.schemas:
+ ToolCallPayload:
+ description: Arbitrary JSON payload captured from the tool invocation.
+ nullable: true
+ oneOf:
+ - type: object
+ additionalProperties: true
+ - type: array
+ items: {}
+ - type: string
+ - type: number
+ - type: integer
+ - type: booleanCommittable suggestion skipped: line range outside the PR's diff.
| type: 'number', | ||
| description: 'Tool call start timestamp (milliseconds)', | ||
| }, | ||
| updatedAt: { | ||
| type: 'number', | ||
| description: 'Tool call last update timestamp (milliseconds)', | ||
| }, | ||
| deletedAt: { | ||
| type: 'number', | ||
| description: 'Tool call deletion timestamp (milliseconds)', | ||
| }, |
There was a problem hiding this comment.
Fix ToolCallResult timestamp schema mismatch
createdAt, updatedAt, and deletedAt are stored as DateTime in Prisma and are serialized to ISO strings when the API returns tool call records. Declaring them as number (milliseconds) makes the OpenAPI contract lie about the wire format and will break generated clients. Please change these fields to type: 'string' with format: 'date-time' (and make deletedAt nullable) so the schema matches the actual payload.
- createdAt: {
- type: 'number',
- description: 'Tool call start timestamp (milliseconds)',
- },
- updatedAt: {
- type: 'number',
- description: 'Tool call last update timestamp (milliseconds)',
- },
- deletedAt: {
- type: 'number',
- description: 'Tool call deletion timestamp (milliseconds)',
- },
+ createdAt: {
+ type: 'string',
+ format: 'date-time',
+ description: 'Tool call start time',
+ },
+ updatedAt: {
+ type: 'string',
+ format: 'date-time',
+ description: 'Tool call last update time',
+ },
+ deletedAt: {
+ oneOf: [
+ { type: 'string', format: 'date-time' },
+ { type: 'null' },
+ ],
+ description: 'Tool call deletion time (null when active)',
+ },📝 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: 'number', | |
| description: 'Tool call start timestamp (milliseconds)', | |
| }, | |
| updatedAt: { | |
| type: 'number', | |
| description: 'Tool call last update timestamp (milliseconds)', | |
| }, | |
| deletedAt: { | |
| type: 'number', | |
| description: 'Tool call deletion timestamp (milliseconds)', | |
| }, | |
| createdAt: { | |
| type: 'string', | |
| format: 'date-time', | |
| description: 'Tool call start time', | |
| }, | |
| updatedAt: { | |
| type: 'string', | |
| format: 'date-time', | |
| description: 'Tool call last update time', | |
| }, | |
| deletedAt: { | |
| oneOf: [ | |
| { type: 'string', format: 'date-time' }, | |
| { type: 'null' }, | |
| ], | |
| description: 'Tool call deletion time (null when active)', | |
| }, |
🤖 Prompt for AI Agents
In packages/openapi-schema/src/schemas.gen.ts around lines 4478 to 4488, the
ToolCallResult timestamp fields are declared as numbers but the API serializes
them as ISO datetime strings; change createdAt and updatedAt to type: 'string'
with format: 'date-time', and change deletedAt to type: 'string', format:
'date-time' and make it nullable (or allow null via nullable: true or type:
['string','null'] depending on schema style) so the OpenAPI schema matches the
actual JSON payload.
- Updated tool call management to include consolidated tool call history by step, improving tracking and reducing duplication. - Introduced new event types for tool call start and stream, enhancing event handling capabilities. - Refactored SkillInvokerService and ToolCallService to utilize the new ToolCallResult model for better data management. - Improved UI rendering for tool call results, ensuring compliance with coding standards and performance optimizations. - Updated OpenAPI schema and TypeScript types to reflect new event structures and tool call handling improvements.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/ai-workspace-common/src/utils/sse-post.ts (1)
142-147: Start event fires repeatedly; set the guard flag.
onSkillStartis gated byisSkillFirstMessagebut the flag is never set to false, causing repeated starts. Set it to false after the first start.if (isSkillFirstMessage) { - onSkillStart(lastStartEvent); + onSkillStart(lastStartEvent); + isSkillFirstMessage = false; }packages/ai-workspace-common/src/hooks/canvas/use-update-action-result.ts (2)
15-26: Token-usage cache key ignores values; can return stale totals.Keying on step names only will miss updates when usage changes but names don’t. Include usage in the key (e.g., hash of
steps.map(s => s.tokenUsage)).- return (steps: ActionStep[]) => { - const cacheKey = JSON.stringify(steps.map((s) => s?.name)); + return (steps: ActionStep[]) => { + const cacheKey = JSON.stringify( + steps.map((s) => ({ name: s?.name, usage: s?.tokenUsage ?? [] })) + );
84-95: Array comparison by length only can skip real updates.
isNodeDataEqualtreats arrays equal if lengths match, missing content changes (e.g., same artifact count but different items). Compare a stable fingerprint instead (ids or JSON of minimal fields).- else if (Array.isArray(newMetadata[key])) { - const oldArray = oldMetadata[key] || []; - const newArray = newMetadata[key] || []; - if (oldArray.length !== newArray.length) { - return false; - } - } + else if (Array.isArray(newMetadata[key])) { + const oldArray = oldMetadata[key] || []; + const newArray = newMetadata[key] || []; + if (oldArray.length !== newArray.length) return false; + const oldSig = JSON.stringify(oldArray); + const newSig = JSON.stringify(newArray); + if (oldSig !== newSig) return false; + }apps/api/src/modules/skill/skill-invoker.service.ts (1)
544-635: Track per-call timestamps instead of a singlestartTs
startTsis captured once before the event loop, so every tool call is persisted and emitted with the samecreatedAt. When multiple tools run in one action, the auditing data becomes meaningless and completion XML shows identical start times. Please store a per-tool timestamp (e.g.,Map<toolCallId, number>) onon_tool_start, reuse it in the XML builder/persistence, and fall back toDate.now()only if the ID was never seen.- const startTs = Date.now(); + const toolStartTimes = new Map<string, number>(); ... - const buildToolUseXML = (includeResult: boolean, errorMsg: string, updatedTs: number) => + const buildToolUseXML = ( + includeResult: boolean, + errorMsg: string, + updatedTs: number, + startTs: number, + ) => this.toolCallService.generateToolUseXML({ toolCallId, includeResult, errorMsg, metadata: { name: toolName, type: event.metadata?.type as string | undefined, toolsetKey: toolsetId, toolsetName: event.metadata?.toolsetName, }, input: event.data?.input, output: event.data?.output, - startTs: startTs, + startTs, updatedTs: updatedTs, }); ... if (event.event === 'on_tool_start') { if (!toolCallIds.has(toolCallId)) { toolCallIds.add(toolCallId); + const startedAt = Date.now(); + toolStartTimes.set(toolCallId, startedAt); await persistToolCall(ToolCallStatus.EXECUTING, { input: event.data?.input, output: '', }); // Send XML for executing state - const xmlContent = buildToolUseXML(false, '', Date.now()); + const xmlContent = buildToolUseXML(false, '', Date.now(), startedAt); ... if (event.event === 'on_tool_error') { const errorMsg = String((event.data as any)?.error ?? 'Tool execution failed'); await persistToolCall(ToolCallStatus.FAILED, { input: undefined, output: event.data?.output, errorMessage: errorMsg, }); // Send XML for failed state - const xmlContent = buildToolUseXML(false, errorMsg, Date.now()); + const startTs = toolStartTimes.get(toolCallId) ?? Date.now(); + const xmlContent = buildToolUseXML(false, errorMsg, Date.now(), startTs); ... if (event.event === 'on_tool_end') { await persistToolCall(ToolCallStatus.COMPLETED, { input: undefined, output: event.data?.output, }); // Extract tool_call_chunks from AIMessageChunk if (event.metadata.langgraph_node === 'tools' && event.data?.output) { ... - const xmlContent = buildToolUseXML(true, '', Date.now()); + const startTs = toolStartTimes.get(toolCallId) ?? Date.now(); + const xmlContent = buildToolUseXML(true, '', Date.now(), startTs); } this.toolCallService.releaseToolCallId({ resultId, version, toolName, toolsetId, runId, }); toolCallIds.delete(toolCallId); + toolStartTimes.delete(toolCallId);
♻️ Duplicate comments (8)
packages/ai-workspace-common/src/utils/sse-post.ts (1)
272-275: Don’t droptool_call_startwhen no handler is provided.Current logic
onToolCallStart?.(...); continue;silently drops the event if the handler is undefined. Fall back to batching so UIs still receive it.- if (skillEvent.event === 'tool_call_start') { - onToolCallStart?.(skillEvent); - continue; - } + if (skillEvent.event === 'tool_call_start') { + if (onToolCallStart) { + onToolCallStart(skillEvent); + continue; + } + // No handler: allow it to flow through the batch. + }packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (2)
439-444: Normalize status; derive sane default.Defaulting via
ToolCallStatus.FAILED.toString()risks casing mismatch and wrong default. Infer from presence of result/error and normalize to lowercase.- // the status is always failed if the result is not present - const status = (attrs['data-tool-call-status'] ?? ToolCallStatus.FAILED.toString()) as - | 'executing' - | 'completed' - | 'failed'; + const rawStatus = attrs['data-tool-call-status']?.toLowerCase?.(); + const hasResult = !!attrs['data-tool-result']; + const hasErrorAttr = !!attrs['data-tool-error']; + const status: 'executing' | 'completed' | 'failed' = + rawStatus === 'completed' ? 'completed' : + rawStatus === 'failed' ? 'failed' : + rawStatus === 'executing' ? 'executing' : + hasResult ? 'completed' : + hasErrorAttr ? 'failed' : 'executing';
465-477: Bug:erroris taken from result; use<error>content.Set
errorfromdata-tool-error, notdata-tool-result, and parse it consistently.- output: safeParse(resultStr) as Record<string, unknown> | undefined, - error: status === 'failed' ? String(resultStr ?? '') : undefined, + output: safeParse(resultStr) as Record<string, unknown> | undefined, + error: status === 'failed' ? String(attrs['data-tool-error'] ?? '') : undefined,packages/openapi-schema/schema.yml (1)
6825-6836: Allow any JSON shape for tool-call input/output.The ToolCallService persists whatever JSON the tool returns (arrays, booleans, numbers, null, nested mixes). Constraining
input/outputtoobject|stringmeans generated clients will reject perfectly valid responses we already emit. Please loosen these fields to “any JSON value,” e.g. by factoring them through a helper schema that covers object/array/primitive/null.input: - # input can be structured object or raw string; keep it flexible - oneOf: - - type: object - - type: string + $ref: '#/components/schemas/ToolCallPayload' description: Tool input arguments output: - # output can be structured object or raw string - oneOf: - - type: object - - type: string - description: Tool output result + $ref: '#/components/schemas/ToolCallPayload' + description: Tool output result error: @@ + ToolCallPayload: + description: Arbitrary JSON payload captured from the tool invocation. + nullable: true + oneOf: + - type: object + additionalProperties: true + - type: array + items: {} + - type: string + - type: number + - type: integer + - type: booleanpackages/openapi-schema/src/schemas.gen.ts (1)
4479-4489: Fix ToolCallResult timestamp types to match API (ISO date-time, not numbers).createdAt/updatedAt/deletedAt are serialized as ISO strings; declaring number breaks clients. Make them string date-time (deletedAt nullable).
Apply this diff:
- createdAt: { - type: 'number', - description: 'Tool call start timestamp (milliseconds)', - }, - updatedAt: { - type: 'number', - description: 'Tool call last update timestamp (milliseconds)', - }, - deletedAt: { - type: 'number', - description: 'Tool call deletion timestamp (milliseconds)', - }, + createdAt: { + type: 'string', + format: 'date-time', + description: 'Tool call start time', + }, + updatedAt: { + type: 'string', + format: 'date-time', + description: 'Tool call last update time', + }, + deletedAt: { + oneOf: [ + { type: 'string', format: 'date-time' }, + { type: 'null' }, + ], + description: 'Tool call deletion time (null when active)', + },Run to confirm the type in generated types matches:
#!/bin/bash rg -nC2 'export type ToolCallResult' packages/openapi-schema/src/types.gen.ts rg -nC2 'createdAt' packages/openapi-schema/src/types.gen.tsapps/api/src/modules/tool-call/tool-call.service.ts (2)
318-325: Fix return type mismatch intoJSONStr
toJSONStris declared to return astringbut returnsundefinedfor nullish values. That fails TypeScript strict checking and can propagate unexpectedundefinedinto Prisma writes. Return an empty string (or widen the signature) so callers always receive a string.- const toJSONStr = (value: unknown): string => { - if (value === undefined || value === null) return undefined; + const toJSONStr = (value: unknown): string => { + if (value === undefined || value === null) return '';
94-124: Escape XML payload before streaming
generateToolUseXMLinjects rawinput,output, anderrorMsgdirectly into XML. If those strings contain<,>,&, or quotes, the markup breaks and we risk XSS in downstream consumers rendering the XML. Please JSON-stringify as needed and escape XML entities before interpolation.- const lines: string[] = []; + const toStr = (value: unknown): string => { + try { + return typeof value === 'string' ? value : JSON.stringify(value); + } catch { + return ''; + } + }; + const escapeXml = (raw: string): string => + raw + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + + const lines: string[] = []; lines.push('<tool_use>'); lines.push(`<callId>${toolCallId}</callId>`); lines.push(`<name>${name ?? ''}</name>`); lines.push(`<type>${type ?? ''}</type>`); lines.push(`<toolsetKey>${toolsetKey}</toolsetKey>`); lines.push(`<toolsetName>${toolsetName ?? ''}</toolsetName>`); lines.push('<arguments>'); - lines.push(input ? (typeof input === 'string' ? input : JSON.stringify(input)) : ''); + lines.push(escapeXml(toStr(input))); lines.push('</arguments>'); lines.push(`<createdAt>${startTs}</createdAt>`); if (errorMsg) { - lines.push( - `<result>${output ? (typeof output === 'string' ? output : JSON.stringify(output)) : ''}</result>`, - ); - lines.push( - `<error>${errorMsg ? (typeof errorMsg === 'string' ? errorMsg : JSON.stringify(errorMsg)) : ''}</error>`, - ); + lines.push(`<result>${escapeXml(toStr(output))}</result>`); + lines.push(`<error>${escapeXml(toStr(errorMsg))}</error>`); lines.push(`<status>${ToolCallStatus.FAILED}</status>`); lines.push(`<updatedAt>${updatedTs}</updatedAt>`); } else if (includeResult) { - lines.push( - `<result>${output ? (typeof output === 'string' ? output : JSON.stringify(output)) : ''}</result>`, - ); + lines.push(`<result>${escapeXml(toStr(output))}</result>`); lines.push(`<status>${ToolCallStatus.COMPLETED}</status>`); lines.push(`<updatedAt>${updatedTs}</updatedAt>`);apps/api/src/modules/skill/skill-invoker.service.ts (1)
4-6: Avoid internal LangChain/distimport and use public MessageContent typePulling
MessageContentComplexfrom@langchain/core/dist/messagesis an internal path that breaks whenever LangChain reshuffles its build output, and the public docs point toMessageContentfrom@langchain/core/messagesfor multimodal payloads. Please switch the import to the public entrypoint and type the content withMessageContentinstead. (readmex.com)Here’s an example fix:
-import { AIMessageChunk, BaseMessage, MessageContentComplex } from '@langchain/core/dist/messages'; -import { AIMessage, HumanMessage } from '@langchain/core/messages'; +import { AIMessage, AIMessageChunk, BaseMessage, HumanMessage, MessageContent } from '@langchain/core/messages'; ... - let messageContent: string | MessageContentComplex[] = query; + let messageContent: MessageContent = query;
🧹 Nitpick comments (16)
.vscode/launch.json (2)
15-17: Restrict inspector to localhost.
NODE_OPTIONS: "--inspect=0.0.0.0:9229"exposes the debugger on all interfaces. Prefer--inspect=127.0.0.1:9229to avoid unintended remote access.- "NODE_OPTIONS": "--inspect=0.0.0.0:9229" + "NODE_OPTIONS": "--inspect=127.0.0.1:9229"
10-20: Optional: exclude node_modules from stepping.Add
"**/node_modules/**"toskipFilesfor a cleaner debug experience.- "skipFiles": ["<node_internals>/**"] + "skipFiles": ["<node_internals>/**", "**/node_modules/**"]packages/ai-workspace-common/src/utils/sse-post.ts (2)
174-180: Handle bothtool_call_streamandtool_call_end(naming divergence).Types add
tool_call_end, code handlestool_call_stream. Support both to stay forward/backward compatible until unified.- if (eventsByType.tool_call_stream?.length) { - for (const event of eventsByType.tool_call_stream) { - onToolCallStream?.(event); - } - } + const toolCallStreams = [ + ...(eventsByType.tool_call_stream ?? []), + ...(eventsByType.tool_call_end ?? []), + ]; + if (toolCallStreams.length) { + for (const event of toolCallStreams) { + onToolCallStream?.(event); + } + }
66-81:onStartprop is unused.Remove it or invoke it (e.g., right before reading or when first
startarrives) to avoid dead API.- onStart: () => void;Or call it once before starting the read loop.
packages/ai-workspace-common/src/hooks/canvas/use-update-action-result.ts (1)
215-217: Avoid double JSON serialize/parse for cloning.
JSON.parse(JSON.stringify(...))is expensive and loses types. Theupdatesobject is created locally; pass it directly or shallow-clone if needed.- const nodeUpdates = JSON.parse( - JSON.stringify(buildNodeUpdates(resultId, merged as Partial<ActionResult>)), - ); + const nodeUpdates = buildNodeUpdates(resultId, merged as Partial<ActionResult>);and similarly below.
Also applies to: 324-326
packages/openapi-schema/src/schemas.gen.ts (1)
4448-4466: Loosen object payloads for tool input/output (optional).For object variants under input/output, specify additionalProperties: true to avoid “empty object” inference in strict generators.
input: { oneOf: [ { - type: 'object', + type: 'object', + additionalProperties: true, }, { type: 'string', }, ], description: 'Tool input arguments', }, output: { oneOf: [ { - type: 'object', + type: 'object', + additionalProperties: true, }, { type: 'string', }, ], description: 'Tool output result', },Also applies to: 4450-4462
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (4)
215-241: Consider parsing tool XML in onSkillStream as well.Tool-use tags can appear in regular model stream; parsing here improves resilience.
const updatedStep: ActionStep = findOrCreateStep(result.steps ?? [], step); updatedStep.content = (updatedStep.content ?? '') + (content ?? ''); if (!updatedStep.reasoningContent) { updatedStep.reasoningContent = reasoningContent ?? ''; } else { updatedStep.reasoningContent = (updatedStep.reasoningContent ?? '') + (reasoningContent ?? ''); } + // Also merge tool call state from any tool_use XML in the stream + updateToolCallsFromXml(updatedStep, step, content ?? '');Based on coding guidelines
440-472: Good integration of tool-call stream; minor follow-ups.
- Artifact handling and step cache updates are correct.
- Optionally also merge skillEvent.toolCallResult when present to support backends that send structured payloads.
- // merge tool calls status - updateToolCallsFromXml(updatedStep, step, content); + // merge tool calls status + updateToolCallsFromXml(updatedStep, step, content); + if (skillEvent.toolCallResult) { + updatedStep.toolCalls = mergeToolCallById( + updatedStep.toolCalls ?? [], + skillEvent.toolCallResult + ); + }
591-596: Avoidanyin event wrapper.Use
unknown[](or a generic rest) to preserve type-safety.- const wrapEventHandler = - (handler: (...args: any[]) => void) => - (...args: any[]) => { + const wrapEventHandler = + (handler: (...args: unknown[]) => void) => + (...args: unknown[]) => { resetTimeout(); handler(...args); };Based on coding guidelines
618-621: Optional: clear step cache on invoke cleanup.Prevents stale cache if stream ends without end/error.
- return () => { - timeoutCleanup(); - }; + return () => { + timeoutCleanup(); + clearLatestSteps(resultId); + };packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx (6)
2-2: Import useCallback for stable handlers.-import React, { useMemo, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react';Based on coding guidelines
19-21: Remove inline style; use Tailwind for animation timing.- className="w-[18px] h-[18px] text-gray-500 dark:text-gray-400 animate-spin" - style={{ animationDuration: '1.1s' }} + className="w-[18px] h-[18px] text-gray-500 dark:text-gray-400 animate-[spin_1100ms_linear_infinite]"Based on coding guidelines
38-39: Use brand colors for status icons.- className="w-[18px] h-[18px] text-green-500 dark:text-green-400" + className="w-[18px] h-[18px] text-[#12B76A]"- className="w-[18px] h-[18px] text-red-500 dark:text-red-400" + className="w-[18px] h-[18px] text-[#F04438]"As per coding guidelines
Also applies to: 55-56
91-101: Add stable toggle handler and localization for fallback labels.- const [isCollapsed, setIsCollapsed] = useState(true); + const [isCollapsed, setIsCollapsed] = useState(true); + const handleToggle = useCallback(() => setIsCollapsed((v) => !v), []); @@ - const toolName = props['data-tool-name'] || 'unknown'; - const toolsetKey = props['data-tool-toolset-key'] || 'unknown'; + const unknownText = t('components.markdown.unknown', 'Unknown'); + const toolName = props['data-tool-name'] ?? unknownText; + const toolsetKey = props['data-tool-toolset-key'] ?? unknownText;As per coding guidelines
148-154: Accessibility + remove inline styles on header toggle.- <div - className="flex items-center px-4 py-2 cursor-pointer select-none bg-gray-50 dark:bg-gray-700 min-h-[44px]" - onClick={() => setIsCollapsed(!isCollapsed)} + <div + className="flex items-center px-4 py-2 cursor-pointer select-none bg-gray-50 dark:bg-gray-700 min-h-[44px]" + role="button" + tabIndex={0} + aria-expanded={!isCollapsed} + onClick={handleToggle} + onKeyDown={(e) => (e.key === 'Enter' || e.key === ' ') && (e.preventDefault(), handleToggle())} style={{ fontFamily: 'Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace', }} > @@ - <ToolOutlined - className="text-gray-500 dark:text-gray-400" - style={{ - fontSize: '16px', - marginRight: '12px', // Adjusted margin for spacing - transition: 'transform 0.2s ease-in-out', - transform: isCollapsed ? 'rotate(-90deg)' : 'rotate(0deg)', - }} - /> + <ToolOutlined + className={[ + 'text-gray-500 dark:text-gray-400', + 'mr-3 text-[16px]', + 'transition-transform duration-200 ease-in-out', + isCollapsed ? '-rotate-90' : 'rotate-0', + ].join(' ')} + />As per coding guidelines
Also applies to: 156-164
114-116: Minor: nullish coalescing is redundant here.parseToolCallStatus already defaults; keep as-is or simplify.
- const toolCallStatus = - parseToolCallStatus(props['data-tool-call-status']) ?? ToolCallStatus.EXECUTING; + const toolCallStatus = parseToolCallStatus(props['data-tool-call-status']);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.vscode/launch.json(1 hunks)apps/api/src/modules/skill/skill-invoker.service.ts(5 hunks)apps/api/src/modules/tool-call/tool-call.service.ts(1 hunks)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx(5 hunks)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts(4 hunks)packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts(7 hunks)packages/ai-workspace-common/src/hooks/canvas/use-update-action-result.ts(5 hunks)packages/ai-workspace-common/src/requests/types.gen.ts(1 hunks)packages/ai-workspace-common/src/utils/sse-post.ts(5 hunks)packages/openapi-schema/schema.yml(4 hunks)packages/openapi-schema/src/schemas.gen.ts(4 hunks)packages/openapi-schema/src/types.gen.ts(5 hunks)packages/skill-template/src/base.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/skill-template/src/base.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,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.Use JSDoc style comments for functions and classes in JavaScript/TypeScript
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals
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
Validate object properties before destructuring
Use ES6+ features like arrow functions, destructuring, and spread operators
Use async/await instead of raw promises for asynchronous code
**/*.{js,jsx,ts,tsx}: Maximum line length of 100 characters
Use 2 spaces for indentation, no tabs
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
No trailing whitespace at the end of lines
**/*.{js,jsx,ts,tsx}: Each file should contain only one main component
File names should match the component name
Keep component files focused with minimal dependencies
Group import statements in the following order: 1. React/framework libraries 2. Third-party libraries 3. Internal modules 4. Relative path imports (parent directories first, then child directories) 5. Type imports 6. Style imports
Sort imports alphabetically within each group
Leave a blank line between import groups
Components should be organized in the following order: 1. Import statements 2. Type definitions 3. Constant declarations 4. Component function 5. Hook calls 6. Event handlers 7. Helper render functions 8. JSX return statement
Extract complex logic in...
Files:
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/ai-workspace-common/src/hooks/canvas/use-update-action-result.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/ai-workspace-common/src/utils/sse-post.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/openapi-schema/src/types.gen.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - it defeats the purpose of TypeScript
When tempted to useany, useunknowntype instead and add proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new ones
Use TypeScript's utility types to derive new types:Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>
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 typesExplicitly type props interfaces/types
Files:
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/ai-workspace-common/src/hooks/canvas/use-update-action-result.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/ai-workspace-common/src/utils/sse-post.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/openapi-schema/src/types.gen.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
Follow NestJS patterns for backend code
Files:
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/ai-workspace-common/src/hooks/canvas/use-update-action-result.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/utils/sse-post.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/openapi-schema/src/types.gen.ts
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,styl,js,jsx,ts,tsx}: Use Primary color (#155EEF) as the main brand color for buttons, links, and accents
Use Error color (#F04438) for error states and destructive actions
Use Success color (#12B76A) for success states and confirmations
Use Warning color (#F79009) for warnings and important notifications
Use Info color (#0BA5EC) for informational elements
Files:
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/ai-workspace-common/src/hooks/canvas/use-update-action-result.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/ai-workspace-common/src/utils/sse-post.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/openapi-schema/src/types.gen.ts
**/*.{css,scss,less,styl,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{css,scss,less,styl,js,jsx,ts,tsx}: 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
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.tspackages/ai-workspace-common/src/requests/types.gen.tspackages/ai-workspace-common/src/hooks/canvas/use-update-action-result.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/openapi-schema/src/schemas.gen.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/ai-workspace-common/src/utils/sse-post.tsapps/api/src/modules/tool-call/tool-call.service.tspackages/openapi-schema/src/types.gen.ts
**/*.{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
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 (avoid using index when possible)
Always split nested components with closures into separate components
Use lazy loading for components that are not immediately needed
Debounce handlers for events that might fire rapidly (resize, scroll, input)
Implement fallback UI for components that might fail
Use error boundaries to catch and handle runtime errors
Use Tailwind CSS for styling components
Group related utility classes together
Prefer Tailwind utilities over custom CSS when possible
**/*.{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 lineUse React best practices for frontend code
Files:
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.tsx: Use the translation wrapper component when displaying user-facing text in components
Ensure all user-facing text in components is translatable
Files:
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-21T01:56:17.029Z
Learnt from: CR
PR: refly-ai/refly#0
File: .cursor/rules/03-typescript-guidelines.mdc:0-0
Timestamp: 2025-07-21T01:56:17.029Z
Learning: Applies to **/*.{ts,tsx} : When tempted to use `any`, use `unknown` type instead and add proper type guards
Applied to files:
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts
📚 Learning: 2025-07-21T01:56:17.029Z
Learnt from: CR
PR: refly-ai/refly#0
File: .cursor/rules/03-typescript-guidelines.mdc:0-0
Timestamp: 2025-07-21T01:56:17.029Z
Learning: Applies to **/*.{ts,tsx} : Avoid using `any` type whenever possible - it defeats the purpose of TypeScript
Applied to files:
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts
🧬 Code graph analysis (8)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (2)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/rehypePlugin.ts (1)
TOOL_USE_TAG_RENDER(5-5)packages/openapi-schema/src/types.gen.ts (2)
ToolCallResult(3189-3246)status(297-297)
packages/ai-workspace-common/src/hooks/canvas/use-update-action-result.ts (1)
packages/openapi-schema/src/types.gen.ts (2)
ActionResult(1726-1837)status(297-297)
apps/api/src/modules/skill/skill-invoker.service.ts (3)
packages/openapi-schema/src/types.gen.ts (2)
ToolCallResult(3189-3246)status(297-297)packages/ai-workspace-common/src/utils/parse.ts (1)
safeParseJSON(1-10)apps/api/src/modules/tool-call/tool-call.service.ts (1)
persistToolCall(301-366)
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (3)
packages/openapi-schema/src/types.gen.ts (3)
ActionStep(1634-1669)ActionStepMeta(1602-1607)SkillEvent(3124-3187)packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (2)
parseToolCallFromChunk(422-480)mergeToolCallById(485-497)packages/stores/src/stores/action-result.ts (1)
useActionResultStore(77-427)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx (1)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/types.ts (1)
parseToolCallStatus(8-17)
packages/ai-workspace-common/src/utils/sse-post.ts (3)
packages/ai-workspace-common/src/requests/types.gen.ts (1)
SkillEvent(3116-3175)packages/openapi-schema/src/types.gen.ts (1)
SkillEvent(3124-3187)packages/request/src/requests/types.gen.ts (1)
SkillEvent(3114-3173)
apps/api/src/modules/tool-call/tool-call.service.ts (2)
packages/openapi-schema/src/types.gen.ts (5)
type(748-748)status(297-297)ToolCallResult(3189-3246)ActionResult(1726-1837)ActionStatus(1547-1547)apps/api/src/modules/action/action.dto.ts (1)
toolCallResultPO2DTO(38-52)
packages/openapi-schema/src/types.gen.ts (3)
packages/agent-tools/src/base.ts (1)
ToolCallResult(4-25)packages/ai-workspace-common/src/requests/types.gen.ts (1)
status2(5350-5350)packages/request/src/requests/types.gen.ts (1)
status2(5348-5348)
🪛 ast-grep (0.39.6)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts
[warning] 8-8: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(<${TOOL_USE_TAG}[^>]*>([\\s\\S]*?)</${TOOL_USE_TAG}>, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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 (9)
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/toolProcessor.ts (1)
8-10: Regex from constant is fine; ignore ReDoS warning.
TOOL_USE_TAGis a constant, not user input. The dynamic RegExp here is safe.packages/ai-workspace-common/src/requests/types.gen.ts (1)
3112-3114: Verified: SkillEvent and ActionResult types are missing toolCallResult and toolCalls fields respectively.The openapi-schema file includes
toolCallResult?: ToolCallResult;in SkillEvent, but packages/ai-workspace-common/src/requests/types.gen.ts lacks this field. Similarly, ActionResult in openapi-schema includestoolCalls?: Array<ToolCallResult>;, which is also absent from the ai-workspace-common version. These generated files are out of sync. Regenerate from the updated OpenAPI schema to restore these properties and enable typed access to tool-related data in client flows.packages/openapi-schema/src/schemas.gen.ts (4)
2179-2185: Schema extension looks good (ActionStep.toolCalls).References the shared ToolCallResult; consistent with surrounding patterns.
2350-2356: Schema extension looks good (ActionResult.toolCalls).Aligns with step-level field and enables action-level aggregation.
4350-4352: New skill event types acknowledged.Enum additions for tool_call_start/tool_call_stream are appropriate.
4416-4421: SkillEvent.toolCallResult addition is correct.Keeps event payload self-contained for tool-call streams.
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (3)
193-214: Latest-steps cache utilities look good.Clear, minimal, and scoped by resultId; aligns with the streaming use case.
435-437: Event aliasing is fine.Forwarding onToolCallStart to onToolCallStream keeps handling unified.
323-324: Cache invalidation on end/error looks good.Prevents mixing stale steps across runs.
Also applies to: 389-390
| console.log('aiMessages', aiMessages); | ||
| return [new HumanMessage({ content: messageContent }), ...aiMessages]; |
There was a problem hiding this comment.
Remove raw console.log of AI messages
Dumping the entire aiMessages array to stdout leaks user input and intermediate tool data into logs. That’s a privacy/compliance problem and bloats log volume. Please drop this statement or replace it with a sanitized Logger.debug that redacts content.
- console.log('aiMessages', aiMessages);🤖 Prompt for AI Agents
In apps/api/src/modules/skill/skill-invoker.service.ts around lines 144-145,
remove the raw console.log('aiMessages', aiMessages) which leaks user/input/tool
data; either delete the statement or replace it with a call to the module's
Logger at debug level that only logs non-sensitive metadata (e.g.,
aiMessages.length, message types, or a redacted summary) using a redaction
helper before logging so no raw content is emitted to stdout or persisted.
| // Update toolCalls on a step by parsing tool_use XML in the current chunk content | ||
| const updateToolCallsFromXml = ( | ||
| updatedStep: ActionStep, | ||
| step: ActionStepMeta | undefined, | ||
| content: string, | ||
| ): void => { | ||
| const parsed = parseToolCallFromChunk(content, step?.name); | ||
| if (!parsed) return; | ||
| updatedStep.toolCalls = mergeToolCallById(updatedStep.toolCalls, parsed); | ||
| }; |
There was a problem hiding this comment.
Fix type mismatch when merging tool calls.
updatedStep.toolCalls is optional; mergeToolCallById expects an array.
Apply this diff:
- updatedStep.toolCalls = mergeToolCallById(updatedStep.toolCalls, parsed);
+ updatedStep.toolCalls = mergeToolCallById(updatedStep.toolCalls ?? [], parsed);🤖 Prompt for AI Agents
In packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts around
lines 156 to 165, updatedStep.toolCalls is optional but mergeToolCallById
requires an array; ensure you pass a non-undefined array and assign the merged
result back. Change the call to use a fallback array (e.g.,
updatedStep.toolCalls ?? []) when invoking mergeToolCallById and set
updatedStep.toolCalls to the returned array so the types align and the optional
field is populated after merging.
| | 'tool_call_start' | ||
| | 'tool_call_end' | ||
| | 'error'; |
There was a problem hiding this comment.
🧩 Analysis chain
Align event names with runtime usage and server schema.
Code elsewhere handles tool_call_stream, but this union adds tool_call_start and tool_call_end only. Pick one naming set and use it consistently across schema, generated types, and clients. If streaming emits tool_call_stream, include it here; if the server sends tool_call_end, update clients accordingly.
🏁 Script executed:
#!/bin/bash
# Locate all tool_call* event literals used across the repo to confirm canon names.
rg -nP --type=ts -C2 "tool_call_(start|end|stream)" | sed -n '1,200p'Length of output: 4785
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if tool_call_end is referenced anywhere in the codebase
rg -nP "tool_call_end" --type=ts --type=js
Length of output: 79
🏁 Script executed:
#!/bin/bash
# Also check for tool_call_end in all file types (not just ts/js)
rg "tool_call_end" -nLength of output: 79
Replace tool_call_end with tool_call_stream at line 3113.
The union type contains tool_call_end, which is never emitted or handled anywhere in the codebase. All runtime code emits tool_call_stream instead (see skill-invoker.service.ts, sse-post.ts handlers, and tool-call.service.ts type signature). The openapi-schema source also defines tool_call_start + tool_call_stream, not tool_call_end. This mismatch will cause type errors in consuming code.
Change line 3113 from | 'tool_call_end' to | 'tool_call_stream'.
🤖 Prompt for AI Agents
In packages/ai-workspace-common/src/requests/types.gen.ts around lines 3112 to
3114, the union type incorrectly includes 'tool_call_end' which is never
emitted; replace the 'tool_call_end' member with 'tool_call_stream' so the type
matches runtime code and OpenAPI schema, updating the union to include
'tool_call_stream' instead of 'tool_call_end'.
…tail structure
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
UI
Streaming
API / Schema
Localization