[REF-1261][REF-1262] Enhance targeted node editing functionality in Copilot#2132
Merged
[REF-1261][REF-1262] Enhance targeted node editing functionality in Copilot#2132
Conversation
- Introduced NodeEditContext to manage state for targeted node editing, capturing relevant information for user-selected nodes. - Implemented useNodeEditContext hook to synchronize selected node context with the Copilot store. - Added NodeEditBanner component to display editing context and options for modifying or extending nodes. - Updated various components to utilize the new node editing context, ensuring consistent use of optional chaining and nullish coalescing. - Enhanced workflow patch operations to support targeted editing, allowing for incremental updates to workflow plans. This update improves user experience by providing clear context and options for editing nodes directly within the Copilot interface.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces targeted node editing context and incremental workflow plan patching capabilities. It adds infrastructure to track selected nodes, pass node context through the skill invocation pipeline, and apply patch operations incrementally to canvas data rather than regenerating plans entirely. A new GetCanvasSnapshot tool provides canvas state summaries. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Canvas as Flow Component
participant Hook as useNodeEditContext
participant Store as Copilot Store
participant Backend as Copilot Agent
User->>Canvas: Select skillResponse node
Canvas->>Hook: Node selection change detected
Hook->>Hook: Compute graph context<br/>(upstream/downstream nodes)
Hook->>Store: setNodeEditContext(canvasId, context)
Store->>Store: Update nodeEditContext state
Canvas->>Canvas: Render with context
User->>Canvas: Trigger Copilot action
Canvas->>Canvas: Retrieve nodeEditContext from store
Canvas->>Backend: invokeAction with nodeEditContext
Backend->>Backend: buildInvokeConfig includes nodeEditContext
Backend->>Backend: buildWorkflowCopilotPrompt with context
Backend->>Backend: Agent processes patch operations
Backend->>Canvas: Return patch operations
Canvas->>Canvas: applyIncrementalChangesToCanvas
Canvas->>Canvas: Update nodes/edges/variables
sequenceDiagram
participant Agent as Copilot Agent
participant Tool as GetCanvasSnapshot Tool
participant Service as SkillEngineService
participant Canvas as Canvas Service
participant Frontend as Canvas UI
Agent->>Tool: Invoke GetCanvasSnapshot
Tool->>Service: getCanvasData(canvasId)
Service->>Canvas: getCanvasRawData(canvasId)
Canvas-->>Service: {nodes, edges, variables, title}
Service-->>Tool: Canvas state
Tool->>Tool: Generate snapshot<br/>(node/edge counts, previews)
Tool->>Tool: Truncate long content
Tool-->>Agent: Snapshot summary
Agent->>Agent: Include in response
Agent-->>Frontend: Patch operations + snapshot
Frontend->>Frontend: Render CopilotSnapshotRenderer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/agent-tools/src/copilot/index.ts (1)
168-182: Use validated parsed data instead of unchecked input with type assertions.After validating with
parseWorkflowPlanPatchat line 125, use the parsed result (parsed.data?.operations) instead of the originalinput.operations. This ensures you're always working with validated data and removes the unnecessary type assertion. The nullish coalescing provides defensive safety.🛠️ Suggested change
const result = await reflyService.patchWorkflowPlan(user, { planId: planId!, - operations: input.operations as WorkflowPatchOperation[], + operations: parsed.data?.operations ?? [], resultId, resultVersion, }); return { status: 'success', data: { planId: result.planId, version: result.version, }, - summary: `Successfully patched workflow plan with ID: ${result.planId} and created new version: ${result.version}. Applied ${input.operations.length} operation(s).`, + summary: `Successfully patched workflow plan with ID: ${result.planId} and created new version: ${result.version}. Applied ${(parsed.data?.operations ?? []).length} operation(s).`, };Per coding guidelines, prefer validated parsed data and nullish coalescing over direct access with type assertions.
packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx (1)
43-54: Use optional chaining and nullish coalescing for safe property access on map lookups.Line 53 should use optional chaining when accessing the
nodeEditContextmap and normalize tonullfor consistency with how the value is used throughout the component.🛠️ Suggested change
- nodeEditContext: state.nodeEditContext[canvasId], + nodeEditContext: state.nodeEditContext?.[canvasId] ?? null,This follows the coding guidelines for optional chaining and ensures stable values when working with map-like store properties.
🤖 Fix all issues with AI agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx`:
- Around line 174-195: The code extracts variableOperations from
applyIncrementalChangesToCanvas but then ignores them and blindly calls
setVariables(finalPlan.variables ?? []), which can clear variables; instead,
when variableOperations?.length exists you must apply those operations to the
current variables and pass the resulting list to setVariables. Locate where
variable state is stored (e.g., currentVariables from state or a variablesRef),
implement or call a utility to apply create/update/delete operations (e.g.,
applyVariableOperations(currentVariables, variableOperations)), compute
newVariables, then call setVariables(newVariables, { archiveOldFiles: false })
rather than using finalPlan.variables; keep existing behavior when no
variableOperations are present.
In
`@packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsx`:
- Around line 18-46: Replace the fallback using logical OR with nullish
coalescing for the node title: update the displayTitle assignment to use
currentState.title ?? t('copilot.nodeEdit.untitled') (referencing displayTitle
and currentState.title) so empty strings are preserved; do not add any extra
guards around canvasId from useCanvasContext() because that hook already
guarantees a string.
In `@packages/canvas-common/src/workflow-plan.ts`:
- Around line 433-458: planVariableToWorkflowVariable currently casts incoming
strings to union types allowing invalid values through; update it to
clamp/normalize incoming enums by explicitly mapping/checking
planVariable.variableType to one of 'string'|'option'|'resource' (default
'string' if not valid), map each value.type to 'text'|'resource' (default
'text'), and map resource.fileType to 'document'|'image'|'video'|'audio'
(default or omit if invalid). Apply the same normalization logic in the
corresponding updateVariable conversion (lines ~911-931) so both creation and
updates only produce allowed enum values in the WorkflowVariable payloads.
In `@packages/openapi-schema/schema.yml`:
- Around line 12873-12917: WorkflowPatchData currently lacks support for
option-type variables so patch requests cannot change option variables; update
the WorkflowPatchData schema (the object under WorkflowPatchData in the diff) to
include "option" in the variableType enum (matching
WorkflowVariable.variableType), and add properties to carry option configuration
such as an options array (items referencing the existing or new
VariableOption/VariableValue schema) and a selectionMode field (enum like
"single"/"multiple" or referencing VariableSelectionMode) so patch operations
can update option lists and selection behavior; ensure property names match the
runtime model (e.g., variableType, options, selectionMode) and reference
existing component schemas where appropriate.
In `@packages/openapi-schema/src/types.gen.ts`:
- Around line 8078-8134: The WorkflowPatchData type's variableType union is
missing the 'option' member, causing patches to reject valid updates; update the
OpenAPI schema definition that describes WorkflowPatchData (and/or the
referenced WorkflowVariable.variableType enum) to include 'option' in the
allowed values, then regenerate the TypeScript types so
WorkflowPatchData.variableType becomes 'string' | 'resource' | 'option' and
consumers can PATCH option-typed variables without casting.
In `@packages/request/src/requests/types.gen.ts`:
- Around line 8089-8110: The WorkflowPatchData type's variableType union is
missing 'option' causing inconsistency with WorkflowVariable.variableType;
update the schema so WorkflowPatchData.variableType includes 'option' (i.e.,
change the union for variableType in WorkflowPatchData to 'string' | 'option' |
'resource') and then regenerate the OpenAPI-generated types so the updated union
appears in packages/request/src/requests/types.gen.ts; target the
WorkflowPatchData and ensure it matches WorkflowVariable.variableType.
In `@packages/stores/src/stores/copilot.ts`:
- Around line 139-160: The setNodeEditMode updater should use optional chaining
when reading state.nodeEditContext[canvasId] to match the pattern used
elsewhere; in the setNodeEditMode function replace the direct lookup into
state.nodeEditContext with an optional-chained access (e.g., const
currentContext = state.nodeEditContext?.[canvasId]) and keep the early return if
no context, while leaving setNodeEditContext's spread logic as-is since
state.nodeEditContext is always an object.
🧹 Nitpick comments (14)
packages/stores/src/stores/copilot.ts (2)
1-1: Use type-only import for OpenAPI types.Line 1 only brings in types, so switch to
import type(and keep type imports in their own group) to avoid runtime dependencies.♻️ Suggested change
-import { CopilotSession, CanvasNodeType } from '@refly/openapi-schema'; +import type { CopilotSession, CanvasNodeType } from '@refly/openapi-schema';As per coding guidelines, use type-only imports and separate type import groups.
6-33: Avoid duplicating NodeEditContext; reuse the shared OpenAPI type.The OpenAPI schema already defines
NodeEditContext(with optionalcurrentState/graphContext). Duplicating it here risks drift and forces stricter requirements than the API contract. Consider aliasing or extending the shared type instead.♻️ Suggested change
-import type { CopilotSession, CanvasNodeType } from '@refly/openapi-schema'; +import type { + CopilotSession, + NodeEditContext as OpenApiNodeEditContext, +} from '@refly/openapi-schema'; -export interface NodeEditContext { - /** The internal node ID from ReactFlow */ - nodeId: string; - ... - /** The edit mode: modify the current node or extend from it */ - editMode: 'modify' | 'extend'; -} +export type NodeEditContext = OpenApiNodeEditContext;As per coding guidelines, prefer extending existing types over creating new ones.
packages/agent-tools/src/copilot/index.ts (1)
11-11: Use type-only import for OpenAPI types.Line 11 imports types only; switch to
import typeand keep type imports grouped separately.♻️ Suggested change
-import { User, WorkflowPlanRecord, WorkflowPatchOperation } from '@refly/openapi-schema'; +import type { User, WorkflowPlanRecord, WorkflowPatchOperation } from '@refly/openapi-schema';As per coding guidelines, use type-only imports and separate type import groups.
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (1)
5-20: Preferimport typefor schema-only types.This avoids runtime imports and aligns with TS type-only import guidance. If any of these are runtime values, keep them as value imports.
♻️ Suggested change
-import { +import type { ActionMessage, ActionResult, ActionStatus, ActionStep, ActionStepMeta, AgentMode, Artifact, CodeArtifactType, Entity, GenericToolset, InvokeSkillRequest, ModelInfo, NodeEditContext, SkillEvent, } from '@refly/openapi-schema';packages/skill-template/src/prompts/copilot-agent.ts (1)
1-1: Guard nodeEditContext field access and avoid magic numbers in the prompt.Direct dereferences can throw if the payload is partial at runtime. Also, hoist the preview length to a named constant for clarity and reuse.
♻️ Suggested change
// Build node edit context section if provided let nodeEditContextSection = ''; if (params.nodeEditContext) { + const QUERY_PREVIEW_LENGTH = 100; + const nodeEditContext = params.nodeEditContext; + const taskId = nodeEditContext?.taskId ?? ''; + const currentTitle = nodeEditContext?.currentState?.title ?? ''; + const currentQuery = nodeEditContext?.currentState?.query ?? ''; + const currentToolsets = nodeEditContext?.currentState?.toolsets ?? []; + const upstreamTasks = nodeEditContext?.graphContext?.upstreamTaskIds ?? []; + nodeEditContextSection = ` ## Targeted Node Editing Mode @@ -| **modify** | \`patch_workflow\` | \`updateTask\` | Update the selected task (taskId: "${params.nodeEditContext.taskId}"); only change what user requests; preserve other fields | -| **extend** | \`patch_workflow\` | \`createTask\` | Create new task with \`dependentTasks: ["${params.nodeEditContext.taskId}"]\` to extend from selected node | +| **modify** | \`patch_workflow\` | \`updateTask\` | Update the selected task (taskId: "${taskId}"); only change what user requests; preserve other fields | +| **extend** | \`patch_workflow\` | \`createTask\` | Create new task with \`dependentTasks: ["${taskId}"]\` to extend from selected node | @@ -- Current state: ${params.nodeEditContext.currentState?.title ? `"${params.nodeEditContext.currentState.title}"` : 'Untitled'} -- Current query/prompt: ${params.nodeEditContext.currentState?.query ? `"${params.nodeEditContext.currentState.query.substring(0, 100)}..."` : 'None'} -- Current toolsets: ${params.nodeEditContext.currentState?.toolsets?.length ? params.nodeEditContext.currentState.toolsets.join(', ') : 'None'} +- Current state: ${currentTitle ? `"${currentTitle}"` : 'Untitled'} +- Current query/prompt: ${currentQuery ? `"${currentQuery.substring(0, QUERY_PREVIEW_LENGTH)}..."` : 'None'} +- Current toolsets: ${currentToolsets.length ? currentToolsets.join(', ') : 'None'} @@ -- Upstream context available: ${params.nodeEditContext.graphContext?.upstreamTaskIds?.join(', ') || 'None (this is a root task)'} +- Upstream context available: ${upstreamTasks.length ? upstreamTasks.join(', ') : 'None (this is a root task)'}Also applies to: 15-18, 25-66
packages/ai-workspace-common/src/hooks/canvas/use-node-edit-context.ts (2)
1-16: Useimport typefor type-only imports and add an explicit hook return type.This aligns with TS guidelines and avoids runtime imports for types.
♻️ Suggested change
-import { CanvasNode, SkillNodeMeta } from '@refly/canvas-common'; -import { useCopilotStoreShallow, NodeEditContext } from '@refly/stores'; +import type { CanvasNode, SkillNodeMeta } from '@refly/canvas-common'; +import { useCopilotStoreShallow } from '@refly/stores'; +import type { NodeEditContext } from '@refly/stores'; @@ -export const useNodeEditContext = () => { +export const useNodeEditContext = (): void => {
22-25: Guard the nodes array before calling.filter.This prevents a potential crash if ReactFlow’s store ever returns
undefinedand satisfies the array-existence guideline.♻️ Suggested change
- const selectedNodes = useStore( - useShallow((state) => state.nodes.filter((node) => node.selected)), - ); + const selectedNodes = useStore( + useShallow((state) => state.nodes?.filter?.((node) => node.selected) ?? []), + );apps/api/src/modules/workflow/workflow-plan.service.ts (1)
4-10: Preferimport typefor schema-only workflow plan types.This keeps runtime imports lean and adheres to TS type-only import guidance. If any of these are runtime values, keep them as value imports.
♻️ Suggested change
-import { +import type { GetWorkflowPlanDetailData, WorkflowPlan, WorkflowPlanRecord, WorkflowPatchOperation, User, } from '@refly/openapi-schema';packages/openapi-schema/src/schemas.gen.ts (1)
11319-11352: Consider op-specific schema variants to enforce required fields.Right now only
opis required, so invalid combinations (e.g.,updateTaskwithouttaskId/data) still validate. Consider aoneOfwithopconst + required fields per operation to tighten the API contract (update inschema.ymland regenerate).packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsx (2)
77-104: Avoid inline onClick lambdas for mode buttonsInline lambdas create new handlers each render. Memoize the two button handlers to keep referential stability and align with the performance guidelines. As per coding guidelines, avoid inline function creation in render and use
useCallbackfor function props.♻️ Memoized handlers
+ const handleModifyClick = useCallback(() => { + handleModeChange('modify'); + }, [handleModeChange]); + + const handleExtendClick = useCallback(() => { + handleModeChange('extend'); + }, [handleModeChange]); ... - onClick={() => handleModeChange('modify')} + onClick={handleModifyClick} ... - onClick={() => handleModeChange('extend')} + onClick={handleExtendClick}
17-17: Add JSDoc + explicit return type for the exported componentThe component is exported and should carry a JSDoc comment and an explicit return type (
JSX.Element | null) to align with the TypeScript/JSX guidelines. As per coding guidelines, please add JSDoc for functions and explicit return types.packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx (1)
21-21: Use type-only import forCanvasNode/CanvasEdgeThese are used as types only; switch to
import typeto keep runtime bundles leaner and satisfy TS import guidelines. As per coding guidelines, always import types explicitly usingimport type.♻️ Suggested change
-import { CanvasNode, CanvasEdge } from '@refly/openapi-schema'; +import type { CanvasNode, CanvasEdge } from '@refly/openapi-schema';packages/canvas-common/src/workflow-plan.ts (2)
1-12: Useimport typefor OpenAPI-only symbolsThese are type-only imports, so
import typekeeps the emitted JS clean and follows the project’s TS conventions.♻️ Suggested update
-import { +import type { GenericToolset, RawCanvasData, CanvasNode, WorkflowVariable, WorkflowTask, WorkflowPlan, ModelInfo, VariableValue, WorkflowPatchOperation as OpenApiWorkflowPatchOperation, } from '@refly/openapi-schema';As per coding guidelines.
688-695: Remove unnecessaryanycasts; properties are already typedLines 688–695 and 773–777 use
anyto access properties that are already explicitly typed inCanvasNodeData:
node.datais typed asCanvasNodeData<T>, which definesentityId: string(required) andmetadata?: T(optional)- Direct access without
anycasts preserves type safetySimilarly, the
nodesandedgesparameters already satisfyAddNodeParam's expected types (Node[]andEdge[]) sinceCanvasNodeextendsNode, making theas any[]casts unnecessary.Suggested refactor
- const metadata = (node.data as any)?.metadata; + const metadata = node.data.metadata; const taskId = metadata?.taskId;- taskIdToEntityId.set(taskId, (node.data as any)?.entityId); + taskIdToEntityId.set(taskId, node.data.entityId);- nodes: nodes as any[], - edges: edges as any[], + nodes, + edges,
Comment on lines
+174
to
+195
| // For patch operations with patchOperations available, use incremental changes | ||
| if (isPatchOperation && finalPlan.patchOperations?.length) { | ||
| const currentEdges = getEdges() as CanvasEdge[]; | ||
|
|
||
| const { nodes, edges, affectedNodeIds, variableOperations } = applyIncrementalChangesToCanvas( | ||
| finalPlan.patchOperations, | ||
| currentNodes, | ||
| currentEdges, | ||
| tools?.data ?? [], | ||
| { defaultModel: defaultAgentModel }, | ||
| ); | ||
|
|
||
| setNodes(nodes); | ||
| setEdges(edges); | ||
|
|
||
| // Handle variable operations | ||
| if (variableOperations?.length) { | ||
| // For now, we'll handle this by setting all variables including updates | ||
| // The setVariables function will merge with existing variables | ||
| const existingVariables = finalPlan.variables ?? []; | ||
| setVariables(existingVariables, { archiveOldFiles: false }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "applyIncrementalChangesToCanvas" -g '*.ts' -g '*.tsx' -A 3 -B 1Repository: refly-ai/refly
Length of output: 172
🏁 Script executed:
rg -n -C5 "variableOperations" -g '*.ts' -g '*.tsx'Repository: refly-ai/refly
Length of output: 7454
🏁 Script executed:
rg -n "setVariables" -g '*.ts' -g '*.tsx' -A 2 -B 1Repository: refly-ai/refly
Length of output: 8753
Apply variableOperations instead of ignoring them
The extracted variableOperations are never applied. When variableOperations exist, passing only finalPlan.variables ?? [] bypasses the operations and can lose data. If patch_workflow returns operations with no full variable list, calling setVariables([], {archiveOldFiles: false}) silently clears variables. Apply the operations (create, update, delete) to the current variables or restructure to fetch the complete plan before calling setVariables.
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx`
around lines 174 - 195, The code extracts variableOperations from
applyIncrementalChangesToCanvas but then ignores them and blindly calls
setVariables(finalPlan.variables ?? []), which can clear variables; instead,
when variableOperations?.length exists you must apply those operations to the
current variables and pass the resulting list to setVariables. Locate where
variable state is stored (e.g., currentVariables from state or a variablesRef),
implement or call a utility to apply create/update/delete operations (e.g.,
applyVariableOperations(currentVariables, variableOperations)), compute
newVariables, then call setVariables(newVariables, { archiveOldFiles: false })
rather than using finalPlan.variables; keep existing behavior when no
variableOperations are present.
Comment on lines
+18
to
+46
| const { t } = useTranslation(); | ||
| const { canvasId } = useCanvasContext(); | ||
|
|
||
| const { nodeEditContext, setNodeEditContext, setNodeEditMode } = useCopilotStoreShallow( | ||
| (state) => ({ | ||
| nodeEditContext: state.nodeEditContext[canvasId], | ||
| setNodeEditContext: state.setNodeEditContext, | ||
| setNodeEditMode: state.setNodeEditMode, | ||
| }), | ||
| ); | ||
|
|
||
| const handleClear = useCallback(() => { | ||
| setNodeEditContext(canvasId, null); | ||
| }, [canvasId, setNodeEditContext]); | ||
|
|
||
| const handleModeChange = useCallback( | ||
| (mode: 'modify' | 'extend') => { | ||
| setNodeEditMode(canvasId, mode); | ||
| }, | ||
| [canvasId, setNodeEditMode], | ||
| ); | ||
|
|
||
| if (!nodeEditContext) { | ||
| return null; | ||
| } | ||
|
|
||
| const { currentState, editMode } = nodeEditContext; | ||
| const displayTitle = currentState.title || t('copilot.nodeEdit.untitled'); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the complete file to understand the context
cat -n packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsxRepository: refly-ai/refly
Length of output: 4430
🏁 Script executed:
# Search for the useCanvasContext hook definition
fd -t f "useCanvasContext" packages/ai-workspace-common/src --exec grep -l "export.*useCanvasContext"Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for the useCopilotStoreShallow hook definition
fd -t f "copilot" packages/ai-workspace-common/src --type f -name "*store*" | head -10Repository: refly-ai/refly
Length of output: 288
🏁 Script executed:
# Search for useCanvasContext definition
rg -t ts -t tsx "export.*useCanvasContext" --max-count 5Repository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Search for copilot store files
fd copilot packages/ai-workspace-common/src -type f | head -20Repository: refly-ai/refly
Length of output: 228
🏁 Script executed:
# Also search in the stores package
fd -t f "store" packages | grep -i copilot | head -10Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Look for the stores package
fd -t f "packages/stores" --max-depth 1Repository: refly-ai/refly
Length of output: 423
🏁 Script executed:
# Search for useCanvasContext definition with proper syntax
rg "export.*useCanvasContext" --max-count 5Repository: refly-ai/refly
Length of output: 165
🏁 Script executed:
# Find copilot store files
find packages -name "*copilot*store*" -o -name "*store*copilot*" | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Look for stores directory and files
find packages -type d -name "stores" | head -5Repository: refly-ai/refly
Length of output: 100
🏁 Script executed:
# Search for nodeEditContext in the codebase
rg "nodeEditContext" --max-count 10 -A 2Repository: refly-ai/refly
Length of output: 8727
🏁 Script executed:
# Check useCanvasContext implementation
cat packages/ai-workspace-common/src/context/canvas.tsxRepository: refly-ai/refly
Length of output: 29557
🏁 Script executed:
# Check copilot store implementation
cat packages/stores/src/stores/copilot.tsRepository: refly-ai/refly
Length of output: 6267
🏁 Script executed:
# Search for NodeEditContext type definition
rg "type NodeEditContext|interface NodeEditContext" -A 10Repository: refly-ai/refly
Length of output: 2202
🏁 Script executed:
# Verify if currentState is guaranteed to exist after the guard
grep -A 5 "if (!nodeEditContext)" packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsxRepository: refly-ai/refly
Length of output: 236
🏁 Script executed:
# Check other files using the same pattern to confirm best practices
rg "nodeEditContext\[canvasId\]" --context 5 | head -40Repository: refly-ai/refly
Length of output: 3360
Use nullish coalescing for optional field access
currentState.title is an optional field, so use ?? instead of || to align with coding guidelines. However, canvasId from useCanvasContext() is guaranteed to be a string—the hook throws an error if no context is available, so guards for canvasId are unnecessary.
- const displayTitle = currentState.title || t('copilot.nodeEdit.untitled');
+ const displayTitle = currentState.title ?? t('copilot.nodeEdit.untitled');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { t } = useTranslation(); | |
| const { canvasId } = useCanvasContext(); | |
| const { nodeEditContext, setNodeEditContext, setNodeEditMode } = useCopilotStoreShallow( | |
| (state) => ({ | |
| nodeEditContext: state.nodeEditContext[canvasId], | |
| setNodeEditContext: state.setNodeEditContext, | |
| setNodeEditMode: state.setNodeEditMode, | |
| }), | |
| ); | |
| const handleClear = useCallback(() => { | |
| setNodeEditContext(canvasId, null); | |
| }, [canvasId, setNodeEditContext]); | |
| const handleModeChange = useCallback( | |
| (mode: 'modify' | 'extend') => { | |
| setNodeEditMode(canvasId, mode); | |
| }, | |
| [canvasId, setNodeEditMode], | |
| ); | |
| if (!nodeEditContext) { | |
| return null; | |
| } | |
| const { currentState, editMode } = nodeEditContext; | |
| const displayTitle = currentState.title || t('copilot.nodeEdit.untitled'); | |
| const { t } = useTranslation(); | |
| const { canvasId } = useCanvasContext(); | |
| const { nodeEditContext, setNodeEditContext, setNodeEditMode } = useCopilotStoreShallow( | |
| (state) => ({ | |
| nodeEditContext: state.nodeEditContext[canvasId], | |
| setNodeEditContext: state.setNodeEditContext, | |
| setNodeEditMode: state.setNodeEditMode, | |
| }), | |
| ); | |
| const handleClear = useCallback(() => { | |
| setNodeEditContext(canvasId, null); | |
| }, [canvasId, setNodeEditContext]); | |
| const handleModeChange = useCallback( | |
| (mode: 'modify' | 'extend') => { | |
| setNodeEditMode(canvasId, mode); | |
| }, | |
| [canvasId, setNodeEditMode], | |
| ); | |
| if (!nodeEditContext) { | |
| return null; | |
| } | |
| const { currentState, editMode } = nodeEditContext; | |
| const displayTitle = currentState.title ?? t('copilot.nodeEdit.untitled'); |
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsx`
around lines 18 - 46, Replace the fallback using logical OR with nullish
coalescing for the node title: update the displayTitle assignment to use
currentState.title ?? t('copilot.nodeEdit.untitled') (referencing displayTitle
and currentState.title) so empty strings are preserved; do not add any extra
guards around canvasId from useCanvasContext() because that hook already
guarantees a string.
Comment on lines
433
to
+458
| export const planVariableToWorkflowVariable = ( | ||
| planVariable: WorkflowVariable, | ||
| ): WorkflowVariable => { | ||
| planVariable: PlanVariableInput, | ||
| ): WorkflowVariable | null => { | ||
| // Ensure required fields are present | ||
| if (!planVariable.variableId || !planVariable.name) { | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| variableId: planVariable.variableId, | ||
| variableType: planVariable.variableType as 'string' | 'option' | 'resource', | ||
| variableType: (planVariable.variableType as 'string' | 'option' | 'resource') ?? 'string', | ||
| name: planVariable.name, | ||
| value: planVariable.value?.map((value) => ({ | ||
| type: value?.type as 'text' | 'resource', | ||
| text: value?.text, | ||
| // Only include resource if it has the required fields | ||
| ...(value?.resource?.name && value?.resource?.fileType | ||
| ? { | ||
| resource: { | ||
| name: value.resource.name, | ||
| fileType: value.resource.fileType, | ||
| }, | ||
| } | ||
| : {}), | ||
| })), | ||
| description: planVariable.description, | ||
| value: | ||
| planVariable.value?.map((value) => ({ | ||
| type: (value?.type as 'text' | 'resource') ?? 'text', | ||
| text: value?.text, | ||
| // Only include resource if it has the required fields | ||
| ...(value?.resource?.name && value?.resource?.fileType | ||
| ? { | ||
| resource: { | ||
| name: value.resource.name, | ||
| fileType: value.resource.fileType as 'document' | 'image' | 'video' | 'audio', | ||
| }, | ||
| } | ||
| : {}), | ||
| })) ?? [], |
There was a problem hiding this comment.
Clamp loose variable/value types to allowed enums
The current casts let arbitrary strings through (e.g., variableType = 'foo'), which can leak invalid values into WorkflowVariable and patch updates. Normalize to allowed enums before defaulting.
✅ Suggested normalization (apply similarly in updateVariable conversion)
- return {
- variableId: planVariable.variableId,
- variableType: (planVariable.variableType as 'string' | 'option' | 'resource') ?? 'string',
+ const normalizedVariableType =
+ planVariable.variableType === 'string' ||
+ planVariable.variableType === 'option' ||
+ planVariable.variableType === 'resource'
+ ? planVariable.variableType
+ : 'string';
+ return {
+ variableId: planVariable.variableId,
+ variableType: normalizedVariableType,
name: planVariable.name,
value:
planVariable.value?.map((value) => ({
- type: (value?.type as 'text' | 'resource') ?? 'text',
+ type: value?.type === 'resource' ? 'resource' : 'text',
text: value?.text,Also applies to: 911-931
🤖 Prompt for AI Agents
In `@packages/canvas-common/src/workflow-plan.ts` around lines 433 - 458,
planVariableToWorkflowVariable currently casts incoming strings to union types
allowing invalid values through; update it to clamp/normalize incoming enums by
explicitly mapping/checking planVariable.variableType to one of
'string'|'option'|'resource' (default 'string' if not valid), map each
value.type to 'text'|'resource' (default 'text'), and map resource.fileType to
'document'|'image'|'video'|'audio' (default or omit if invalid). Apply the same
normalization logic in the corresponding updateVariable conversion (lines
~911-931) so both creation and updates only produce allowed enum values in the
WorkflowVariable payloads.
Comment on lines
+12873
to
+12917
| type: object | ||
| description: Data for updating a task or variable | ||
| properties: | ||
| title: | ||
| type: string | ||
| description: New display title for the task | ||
| prompt: | ||
| type: string | ||
| description: New prompt or instruction for this task | ||
| dependentTasks: | ||
| type: array | ||
| items: | ||
| type: string | ||
| description: New list of task IDs that must execute before this task | ||
| toolsets: | ||
| type: array | ||
| items: | ||
| type: string | ||
| description: New list of toolset IDs for this task | ||
| variableType: | ||
| type: string | ||
| enum: | ||
| - string | ||
| - resource | ||
| description: New variable type | ||
| name: | ||
| type: string | ||
| description: New variable name | ||
| description: | ||
| type: string | ||
| description: New variable description | ||
| required: | ||
| type: boolean | ||
| description: Whether this variable is required | ||
| resourceTypes: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/VariableResourceType' | ||
| description: New accepted resource types | ||
| value: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/VariableValue' | ||
| description: New variable values | ||
| WorkflowPatchOperation: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the schema file to see WorkflowVariable definition and confirm current WorkflowPatchData state
fd -t f "schema.yml" packages/openapi-schema/ --exec cat -n {} \; | head -100Repository: refly-ai/refly
Length of output: 4058
🏁 Script executed:
# Get file size and structure
wc -l packages/openapi-schema/schema.ymlRepository: refly-ai/refly
Length of output: 98
🏁 Script executed:
# Search for WorkflowVariable and WorkflowPatchData definitions
rg -n "WorkflowVariable:|WorkflowPatchData:" packages/openapi-schema/schema.yml -A 20Repository: refly-ai/refly
Length of output: 1515
🏁 Script executed:
# Search for variableType enum definitions in the schema
rg -n "variableType:" packages/openapi-schema/schema.yml -B 2 -A 8Repository: refly-ai/refly
Length of output: 1348
🏁 Script executed:
# Search TypeScript implementation for patch operations and option handling
rg -n "WorkflowPatchData|WorkflowPatchOperation" --type=ts -B 3 -A 5Repository: refly-ai/refly
Length of output: 24932
🏁 Script executed:
# Search for option variable handling in codebase
rg -n "variableType.*option|option.*variable" --type=ts -B 2 -A 2Repository: refly-ai/refly
Length of output: 50370
🏁 Script executed:
# Search for isSingle property references
rg -n "isSingle" --type=ts packagesRepository: refly-ai/refly
Length of output: 4746
Add support for option-type variables in WorkflowPatchData.
WorkflowVariable.variableType supports option, but WorkflowPatchData.variableType does not. This prevents patch operations from updating option variables and their configuration (option lists and selection mode).
🛠️ Suggested schema extension
variableType:
type: string
enum:
- string
+ - option
- resource
description: New variable type
+ options:
+ type: array
+ items:
+ type: string
+ description: Option values for option-type variables
+ isSingle:
+ type: boolean
+ description: Whether the option variable is single-select or multi-select📝 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.
Suggested change
| type: object | |
| description: Data for updating a task or variable | |
| properties: | |
| title: | |
| type: string | |
| description: New display title for the task | |
| prompt: | |
| type: string | |
| description: New prompt or instruction for this task | |
| dependentTasks: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of task IDs that must execute before this task | |
| toolsets: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of toolset IDs for this task | |
| variableType: | |
| type: string | |
| enum: | |
| - string | |
| - resource | |
| description: New variable type | |
| name: | |
| type: string | |
| description: New variable name | |
| description: | |
| type: string | |
| description: New variable description | |
| required: | |
| type: boolean | |
| description: Whether this variable is required | |
| resourceTypes: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableResourceType' | |
| description: New accepted resource types | |
| value: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableValue' | |
| description: New variable values | |
| WorkflowPatchOperation: | |
| type: object | |
| description: Data for updating a task or variable | |
| properties: | |
| title: | |
| type: string | |
| description: New display title for the task | |
| prompt: | |
| type: string | |
| description: New prompt or instruction for this task | |
| dependentTasks: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of task IDs that must execute before this task | |
| toolsets: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of toolset IDs for this task | |
| variableType: | |
| type: string | |
| enum: | |
| - string | |
| - option | |
| - resource | |
| description: New variable type | |
| options: | |
| type: array | |
| items: | |
| type: string | |
| description: Option values for option-type variables | |
| isSingle: | |
| type: boolean | |
| description: Whether the option variable is single-select or multi-select | |
| name: | |
| type: string | |
| description: New variable name | |
| description: | |
| type: string | |
| description: New variable description | |
| required: | |
| type: boolean | |
| description: Whether this variable is required | |
| resourceTypes: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableResourceType' | |
| description: New accepted resource types | |
| value: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableValue' | |
| description: New variable values | |
| WorkflowPatchOperation: |
🤖 Prompt for AI Agents
In `@packages/openapi-schema/schema.yml` around lines 12873 - 12917,
WorkflowPatchData currently lacks support for option-type variables so patch
requests cannot change option variables; update the WorkflowPatchData schema
(the object under WorkflowPatchData in the diff) to include "option" in the
variableType enum (matching WorkflowVariable.variableType), and add properties
to carry option configuration such as an options array (items referencing the
existing or new VariableOption/VariableValue schema) and a selectionMode field
(enum like "single"/"multiple" or referencing VariableSelectionMode) so patch
operations can update option lists and selection behavior; ensure property names
match the runtime model (e.g., variableType, options, selectionMode) and
reference existing component schemas where appropriate.
Comment on lines
+8078
to
+8134
| /** | ||
| * Type of patch operation to apply to a workflow plan | ||
| */ | ||
| export type WorkflowPatchOp = | ||
| | 'updateTitle' | ||
| | 'createTask' | ||
| | 'updateTask' | ||
| | 'deleteTask' | ||
| | 'createVariable' | ||
| | 'updateVariable' | ||
| | 'deleteVariable'; | ||
|
|
||
| /** | ||
| * Data for updating a task or variable | ||
| */ | ||
| export type WorkflowPatchData = { | ||
| /** | ||
| * New display title for the task | ||
| */ | ||
| title?: string; | ||
| /** | ||
| * New prompt or instruction for this task | ||
| */ | ||
| prompt?: string; | ||
| /** | ||
| * New list of task IDs that must execute before this task | ||
| */ | ||
| dependentTasks?: Array<string>; | ||
| /** | ||
| * New list of toolset IDs for this task | ||
| */ | ||
| toolsets?: Array<string>; | ||
| /** | ||
| * New variable type | ||
| */ | ||
| variableType?: 'string' | 'resource'; | ||
| /** | ||
| * New variable name | ||
| */ | ||
| name?: string; | ||
| /** | ||
| * New variable description | ||
| */ | ||
| description?: string; | ||
| /** | ||
| * Whether this variable is required | ||
| */ | ||
| required?: boolean; | ||
| /** | ||
| * New accepted resource types | ||
| */ | ||
| resourceTypes?: Array<VariableResourceType>; | ||
| /** | ||
| * New variable values | ||
| */ | ||
| value?: Array<VariableValue>; | ||
| }; |
There was a problem hiding this comment.
WorkflowPatchData.variableType blocks 'option' updates
WorkflowVariable.variableType now supports 'option', but patch updates still only accept 'string' | 'resource'. That prevents valid updates and forces downstream casts. Please update the OpenAPI schema and regenerate types so patch updates can set 'option'.
🐛 Suggested schema outcome (regenerate after updating schema)
- variableType?: 'string' | 'resource';
+ variableType?: 'string' | 'resource' | 'option';📝 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.
Suggested change
| /** | |
| * Type of patch operation to apply to a workflow plan | |
| */ | |
| export type WorkflowPatchOp = | |
| | 'updateTitle' | |
| | 'createTask' | |
| | 'updateTask' | |
| | 'deleteTask' | |
| | 'createVariable' | |
| | 'updateVariable' | |
| | 'deleteVariable'; | |
| /** | |
| * Data for updating a task or variable | |
| */ | |
| export type WorkflowPatchData = { | |
| /** | |
| * New display title for the task | |
| */ | |
| title?: string; | |
| /** | |
| * New prompt or instruction for this task | |
| */ | |
| prompt?: string; | |
| /** | |
| * New list of task IDs that must execute before this task | |
| */ | |
| dependentTasks?: Array<string>; | |
| /** | |
| * New list of toolset IDs for this task | |
| */ | |
| toolsets?: Array<string>; | |
| /** | |
| * New variable type | |
| */ | |
| variableType?: 'string' | 'resource'; | |
| /** | |
| * New variable name | |
| */ | |
| name?: string; | |
| /** | |
| * New variable description | |
| */ | |
| description?: string; | |
| /** | |
| * Whether this variable is required | |
| */ | |
| required?: boolean; | |
| /** | |
| * New accepted resource types | |
| */ | |
| resourceTypes?: Array<VariableResourceType>; | |
| /** | |
| * New variable values | |
| */ | |
| value?: Array<VariableValue>; | |
| }; | |
| /** | |
| * Type of patch operation to apply to a workflow plan | |
| */ | |
| export type WorkflowPatchOp = | |
| | 'updateTitle' | |
| | 'createTask' | |
| | 'updateTask' | |
| | 'deleteTask' | |
| | 'createVariable' | |
| | 'updateVariable' | |
| | 'deleteVariable'; | |
| /** | |
| * Data for updating a task or variable | |
| */ | |
| export type WorkflowPatchData = { | |
| /** | |
| * New display title for the task | |
| */ | |
| title?: string; | |
| /** | |
| * New prompt or instruction for this task | |
| */ | |
| prompt?: string; | |
| /** | |
| * New list of task IDs that must execute before this task | |
| */ | |
| dependentTasks?: Array<string>; | |
| /** | |
| * New list of toolset IDs for this task | |
| */ | |
| toolsets?: Array<string>; | |
| /** | |
| * New variable type | |
| */ | |
| variableType?: 'string' | 'resource' | 'option'; | |
| /** | |
| * New variable name | |
| */ | |
| name?: string; | |
| /** | |
| * New variable description | |
| */ | |
| description?: string; | |
| /** | |
| * Whether this variable is required | |
| */ | |
| required?: boolean; | |
| /** | |
| * New accepted resource types | |
| */ | |
| resourceTypes?: Array<VariableResourceType>; | |
| /** | |
| * New variable values | |
| */ | |
| value?: Array<VariableValue>; | |
| }; |
🤖 Prompt for AI Agents
In `@packages/openapi-schema/src/types.gen.ts` around lines 8078 - 8134, The
WorkflowPatchData type's variableType union is missing the 'option' member,
causing patches to reject valid updates; update the OpenAPI schema definition
that describes WorkflowPatchData (and/or the referenced
WorkflowVariable.variableType enum) to include 'option' in the allowed values,
then regenerate the TypeScript types so WorkflowPatchData.variableType becomes
'string' | 'resource' | 'option' and consumers can PATCH option-typed variables
without casting.
Comment on lines
+8089
to
+8110
| export type WorkflowPatchData = { | ||
| /** | ||
| * New display title for the task | ||
| */ | ||
| title?: string; | ||
| /** | ||
| * New prompt or instruction for this task | ||
| */ | ||
| prompt?: string; | ||
| /** | ||
| * New list of task IDs that must execute before this task | ||
| */ | ||
| dependentTasks?: Array<string>; | ||
| /** | ||
| * New list of toolset IDs for this task | ||
| */ | ||
| toolsets?: Array<string>; | ||
| /** | ||
| * New variable type | ||
| */ | ||
| variableType?: 'string' | 'resource'; | ||
| /** |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the structure of WorkflowPatchData around the given lines
sed -n '8089,8130p' packages/request/src/requests/types.gen.tsRepository: refly-ai/refly
Length of output: 853
🏁 Script executed:
# Search for WorkflowVariable type definition in the same file
grep -n "type WorkflowVariable" packages/request/src/requests/types.gen.ts | head -20Repository: refly-ai/refly
Length of output: 95
🏁 Script executed:
# Search for all variableType definitions/references in the file
grep -n "variableType" packages/request/src/requests/types.gen.ts | head -30Repository: refly-ai/refly
Length of output: 280
🏁 Script executed:
# Check if there's an OpenAPI schema file
find packages -name "*.openapi.*" -o -name "*openapi*" -o -name "*.yaml" -o -name "*.yml" | grep -i workflowRepository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Read WorkflowVariable definition starting at line 8421
sed -n '8421,8475p' packages/request/src/requests/types.gen.tsRepository: refly-ai/refly
Length of output: 1167
🏁 Script executed:
# Get context around line 8449 to see what type it belongs to
sed -n '8435,8460p' packages/request/src/requests/types.gen.tsRepository: refly-ai/refly
Length of output: 574
Update WorkflowPatchData.variableType to include 'option'
WorkflowPatchData.variableType supports only 'string' | 'resource' (line 8109), but WorkflowVariable.variableType includes 'string' | 'option' | 'resource' (line 8449). WorkflowVariable explicitly documents options field as "only valid when variable type is option". If patch operations cannot set or update option-type variables, this breaks consistency. Update the OpenAPI schema to include 'option' in the patch operation's variableType field and regenerate this file.
🤖 Prompt for AI Agents
In `@packages/request/src/requests/types.gen.ts` around lines 8089 - 8110, The
WorkflowPatchData type's variableType union is missing 'option' causing
inconsistency with WorkflowVariable.variableType; update the schema so
WorkflowPatchData.variableType includes 'option' (i.e., change the union for
variableType in WorkflowPatchData to 'string' | 'option' | 'resource') and then
regenerate the OpenAPI-generated types so the updated union appears in
packages/request/src/requests/types.gen.ts; target the WorkflowPatchData and
ensure it matches WorkflowVariable.variableType.
Comment on lines
+139
to
+160
| setNodeEditContext: (canvasId: string, context: NodeEditContext | null) => | ||
| set((state) => ({ | ||
| nodeEditContext: { | ||
| ...state.nodeEditContext, | ||
| [canvasId]: context, | ||
| }, | ||
| })), | ||
|
|
||
| setNodeEditMode: (canvasId: string, editMode: 'modify' | 'extend') => | ||
| set((state) => { | ||
| const currentContext = state.nodeEditContext[canvasId]; | ||
| if (!currentContext) return state; | ||
| return { | ||
| nodeEditContext: { | ||
| ...state.nodeEditContext, | ||
| [canvasId]: { | ||
| ...currentContext, | ||
| editMode, | ||
| }, | ||
| }, | ||
| }; | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/stores/src/stores/copilot.ts | sed -n '130,170p'Repository: refly-ai/refly
Length of output: 172
🏁 Script executed:
head -n 200 packages/stores/src/stores/copilot.tsRepository: refly-ai/refly
Length of output: 6267
Use optional chaining for consistency when accessing nodeEditContext values.
Line 149 should use optional chaining to align with the pattern used elsewhere in the file (e.g., removeHistoryTemplateSession). However, the spread in setNodeEditContext does not need nullish coalescing since state.nodeEditContext is always initialized as an object.
🛠️ Suggested change
setNodeEditMode: (canvasId: string, editMode: 'modify' | 'extend') =>
set((state) => {
- const currentContext = state.nodeEditContext[canvasId];
+ const currentContext = state.nodeEditContext?.[canvasId] ?? null;
if (!currentContext) return state;🤖 Prompt for AI Agents
In `@packages/stores/src/stores/copilot.ts` around lines 139 - 160, The
setNodeEditMode updater should use optional chaining when reading
state.nodeEditContext[canvasId] to match the pattern used elsewhere; in the
setNodeEditMode function replace the direct lookup into state.nodeEditContext
with an optional-chained access (e.g., const currentContext =
state.nodeEditContext?.[canvasId]) and keep the early return if no context,
while leaving setNodeEditContext's spread logic as-is since
state.nodeEditContext is always an object.
- Introduced comprehensive documentation for the Node Edit Context feature, detailing its architecture, functionality, and core capabilities. - Included flowcharts and sequence diagrams to illustrate the user interaction and backend processing for targeted node editing. - Documented the core implementation details, including file changes, type definitions, and key design decisions. - Enhanced clarity on the incremental update mechanism and the distinction between modify and extend modes for user actions. This documentation serves to improve understanding and facilitate future development related to the Node Edit Context feature.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/openapi-schema/schema.yml (1)
11770-11794:⚠️ Potential issue | 🟠 Major
WorkflowPatchDatacannot patch option variables yet.
WorkflowVariable.variableTypesupportsoption, butWorkflowPatchData.variableType(Line 11770) does not. Also, patch payload lacks option config fields (options,isSingle), soupdateVariablecannot fully update option-type variables.🛠️ Suggested schema alignment
WorkflowPatchData: type: object description: Data for updating a task or variable properties: variableType: type: string enum: - string + - option - resource description: New variable type + options: + type: array + items: + type: string + description: Option values for option-type variables + isSingle: + type: boolean + description: Whether option variable is single-select🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/openapi-schema/schema.yml` around lines 11770 - 11794, The WorkflowPatchData schema is missing support for option-type variables: update the variableType enum in WorkflowPatchData to include "option" (matching WorkflowVariable.variableType) and add the option-specific patch fields (e.g., options: array of the appropriate option item schema, and isSingle: boolean) so updateVariable can fully patch option-type variables; refer to the existing WorkflowVariable definition and the VariableOption/VariableValue schemas to reuse the exact field names and item $ref types when adding these fields.packages/openapi-schema/src/types.gen.ts (1)
7870-7897:⚠️ Potential issue | 🟠 Major
WorkflowPatchData.variableTypestill blocks'option'and creates type-surface drift.Line 7870 and Line 7896 only allow
'string' | 'resource', whileWorkflowVariable.variableType(Line 8294) allows'option'. This still prevents valid option-variable patch updates and contributes to thevariableType2divergence at Line 8316.🐛 Suggested generated outcome
export type WorkflowPatchData = { ... - variableType?: 'string' | 'resource'; + variableType?: 'string' | 'option' | 'resource'; ... }; -export type variableType = 'string' | 'resource'; +export type variableType = 'string' | 'option' | 'resource';Also applies to: 8316-8316
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/openapi-schema/src/types.gen.ts` around lines 7870 - 7897, The union for variable types in WorkflowPatchData.variableType and the exported type alias variableType currently only permits 'string' | 'resource' and must be expanded to include 'option' to match WorkflowVariable.variableType and avoid the variableType2 divergence; update the optional property WorkflowPatchData.variableType and the exported type alias variableType (and any related alias variableType2) to accept 'string' | 'resource' | 'option' so the patch type surface aligns with WorkflowVariable.
🧹 Nitpick comments (4)
apps/api/src/modules/skill/skill-invoker.service.ts (1)
403-405: Remove redundantcopilotSessionIdreassignment.
copilotSessionIdis already set inconfig.configurableat Line 345, so this block is now duplicate and can drift.♻️ Proposed cleanup
- if (data.copilotSessionId) { - config.configurable.copilotSessionId = data.copilotSessionId; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/skill/skill-invoker.service.ts` around lines 403 - 405, Remove the duplicate assignment of copilotSessionId: locate the block that checks data.copilotSessionId and assigns it to config.configurable.copilotSessionId (the code around the copilotSessionId check in skill-invoker.service.ts) and delete it, since copilotSessionId is already set earlier on config.configurable (see the prior assignment near line 345). Ensure no other logic depends on this second assignment before removing it.packages/openapi-schema/src/types.gen.ts (1)
8005-8011: Reuse the exportededitModealias inNodeEditContext.
NodeEditContext.editModecurrently duplicates the string union already exported aseditMode. Referencing the alias avoids drift.As per coding guidelines, "Avoid magic numbers and strings - use named constants in TypeScript/JavaScript".♻️ Suggested tweak
- editMode: 'modify' | 'extend'; + editMode: editMode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/openapi-schema/src/types.gen.ts` around lines 8005 - 8011, The NodeEditContext type currently repeats the string union "'modify' | 'extend'"; change its editMode property to reference the already exported alias editMode instead (i.e., use editMode as the type for NodeEditContext.editMode) so the declaration reuses the existing type alias and avoids duplication; ensure the symbol name editMode is in scope or exported/imported appropriately if needed.packages/canvas-common/src/workflow-plan.ts (2)
719-727: Consider avoidinganycasts for node data access.Lines 720, 725 use
anyto accessmetadataandentityIdfromnode.data. Per coding guidelines, preferunknownwith type guards or define a proper interface for the node data structure.♻️ Suggested approach
+interface SkillResponseNodeData { + entityId?: string; + metadata?: { + taskId?: string; + [key: string]: unknown; + }; +} + for (const node of currentNodes) { - const metadata = (node.data as any)?.metadata; - const taskId = metadata?.taskId; + const nodeData = node.data as SkillResponseNodeData | undefined; + const taskId = nodeData?.metadata?.taskId; if (taskId) { taskIdToNode.set(taskId, node); taskIdToNodeId.set(taskId, node.id); - taskIdToEntityId.set(taskId, (node.data as any)?.entityId); + taskIdToEntityId.set(taskId, nodeData?.entityId ?? ''); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas-common/src/workflow-plan.ts` around lines 719 - 727, The loop over currentNodes is using unsafe (node.data as any) casts to read metadata and entityId; define a proper interface (e.g. NodeData with optional metadata{taskId?: string} and entityId?: string) or use nodeData: unknown plus a type-guard function isNodeData(obj): obj is NodeData, then replace (node.data as any)?.metadata and (node.data as any)?.entityId with typed-safe accesses and checks before setting taskIdToNode, taskIdToNodeId, and taskIdToEntityId so the compiler enforces the shape instead of relying on any.
737-985: Add default case to switch for defensive handling.The switch statement covers all
WorkflowPatchOpvalues but lacks adefaultcase. While TypeScript's exhaustiveness checking helps, a defensive default prevents silent failures if new operations are added or if runtime data contains unexpected values.🛡️ Suggested fix
case 'deleteVariable': { if (variableId) { variableOperations.push({ type: 'delete', variableId, }); } break; } + + default: { + // Defensive: log or handle unknown operation types + const _exhaustiveCheck: never = op; + console.warn(`Unknown patch operation type: ${_exhaustiveCheck as string}`); + break; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas-common/src/workflow-plan.ts` around lines 737 - 985, The switch on op in the workflow patch handling (the switch starting with cases 'updateTitle', 'createTask', 'updateTask', 'deleteTask', 'createVariable', 'updateVariable', 'deleteVariable') lacks a default branch; add a default case that defensively handles unexpected ops (e.g., log a warning via the same logger/context used in this module or throw a clear Error) so unknown WorkflowPatchOp values don't silently pass — implement the default at the end of the switch to either throw or processLogger.warn with the unexpected op value and return/break.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx`:
- Line 92: The code directly indexes state.nodeEditContext[canvasId], which can
be undefined for missing keys; change the access to use optional chaining
(state.nodeEditContext?.[canvasId]) wherever nodeEditContext is indexed (e.g.,
in the object passed on line with nodeEditContext) to safely return undefined
instead of throwing—update usages in chat-box.tsx that reference
state.nodeEditContext[canvasId] to use ?.[] and keep existing variable names
(state, nodeEditContext, canvasId) unchanged.
In `@packages/openapi-schema/schema.yml`:
- Around line 11818-11821: The YAML for WorkflowPatchOperation.data currently
has a duplicate $ref entry causing a parse error; edit the
WorkflowPatchOperation.data property to remove the second/extra "$ref:
\"#/components/schemas/WorkflowVariable\"" so that it matches the pattern used
by the adjacent variable property — i.e., a single $ref pointing to
"#/components/schemas/WorkflowPatchData" and a description field referencing
WorkflowVariable as needed.
---
Duplicate comments:
In `@packages/openapi-schema/schema.yml`:
- Around line 11770-11794: The WorkflowPatchData schema is missing support for
option-type variables: update the variableType enum in WorkflowPatchData to
include "option" (matching WorkflowVariable.variableType) and add the
option-specific patch fields (e.g., options: array of the appropriate option
item schema, and isSingle: boolean) so updateVariable can fully patch
option-type variables; refer to the existing WorkflowVariable definition and the
VariableOption/VariableValue schemas to reuse the exact field names and item
$ref types when adding these fields.
In `@packages/openapi-schema/src/types.gen.ts`:
- Around line 7870-7897: The union for variable types in
WorkflowPatchData.variableType and the exported type alias variableType
currently only permits 'string' | 'resource' and must be expanded to include
'option' to match WorkflowVariable.variableType and avoid the variableType2
divergence; update the optional property WorkflowPatchData.variableType and the
exported type alias variableType (and any related alias variableType2) to accept
'string' | 'resource' | 'option' so the patch type surface aligns with
WorkflowVariable.
---
Nitpick comments:
In `@apps/api/src/modules/skill/skill-invoker.service.ts`:
- Around line 403-405: Remove the duplicate assignment of copilotSessionId:
locate the block that checks data.copilotSessionId and assigns it to
config.configurable.copilotSessionId (the code around the copilotSessionId check
in skill-invoker.service.ts) and delete it, since copilotSessionId is already
set earlier on config.configurable (see the prior assignment near line 345).
Ensure no other logic depends on this second assignment before removing it.
In `@packages/canvas-common/src/workflow-plan.ts`:
- Around line 719-727: The loop over currentNodes is using unsafe (node.data as
any) casts to read metadata and entityId; define a proper interface (e.g.
NodeData with optional metadata{taskId?: string} and entityId?: string) or use
nodeData: unknown plus a type-guard function isNodeData(obj): obj is NodeData,
then replace (node.data as any)?.metadata and (node.data as any)?.entityId with
typed-safe accesses and checks before setting taskIdToNode, taskIdToNodeId, and
taskIdToEntityId so the compiler enforces the shape instead of relying on any.
- Around line 737-985: The switch on op in the workflow patch handling (the
switch starting with cases 'updateTitle', 'createTask', 'updateTask',
'deleteTask', 'createVariable', 'updateVariable', 'deleteVariable') lacks a
default branch; add a default case that defensively handles unexpected ops
(e.g., log a warning via the same logger/context used in this module or throw a
clear Error) so unknown WorkflowPatchOp values don't silently pass — implement
the default at the end of the switch to either throw or processLogger.warn with
the unexpected op value and return/break.
In `@packages/openapi-schema/src/types.gen.ts`:
- Around line 8005-8011: The NodeEditContext type currently repeats the string
union "'modify' | 'extend'"; change its editMode property to reference the
already exported alias editMode instead (i.e., use editMode as the type for
NodeEditContext.editMode) so the declaration reuses the existing type alias and
avoids duplication; ensure the symbol name editMode is in scope or
exported/imported appropriately if needed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/api/src/modules/skill/skill-invoker.service.tspackages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsxpackages/ai-workspace-common/src/hooks/canvas/use-invoke-action.tspackages/canvas-common/src/workflow-plan.tspackages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.tspackages/openapi-schema/schema.ymlpackages/openapi-schema/src/schemas.gen.tspackages/openapi-schema/src/types.gen.tspackages/skill-template/src/prompts/templates/copilot-agent-system.mdpackages/stores/src/stores/copilot.ts
✅ Files skipped from review due to trivial changes (1)
- packages/skill-template/src/prompts/templates/copilot-agent-system.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts
- packages/stores/src/stores/copilot.ts
- packages/openapi-schema/src/schemas.gen.ts
| pendingFiles: state.pendingFiles[canvasId], | ||
| setPendingPrompt: state.setPendingPrompt, | ||
| setPendingFiles: state.setPendingFiles, | ||
| nodeEditContext: state.nodeEditContext[canvasId], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the store file for copilot
store_file=$(find . -type f -name '*copilot*.ts' -o -name '*store*.ts' | grep -E 'copilot.*store|store.*copilot' | head -n1)
if [ -z "$store_file" ]; then
# Try broader search
store_file=$(fd 'copilot' --type f | grep -E '\.ts$' | grep -i store | head -n1)
fi
echo "=== Store file search ==="
if [ -n "$store_file" ]; then
echo "Found: $store_file"
echo ""
echo "=== nodeEditContext initialization ==="
rg -n -C5 'nodeEditContext' "$store_file" | head -50
else
echo "Store file not found via exact match, searching for nodeEditContext references..."
rg -l 'nodeEditContext' --type ts --type tsx | head -5
fi
echo ""
echo "=== Checking chat-box.tsx context around line 92 ==="
chat_file="packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx"
if [ -f "$chat_file" ]; then
rg -n 'nodeEditContext' "$chat_file" | head -20
echo ""
sed -n '85,100p' "$chat_file" | cat -n
fiRepository: refly-ai/refly
Length of output: 3138
Guard store map access with optional chaining.
Line 92 directly indexes state.nodeEditContext[canvasId] without safe access. While the object is always initialized, the property access can return undefined if the key doesn't exist.
🛡️ Proposed fix
- nodeEditContext: state.nodeEditContext[canvasId],
+ nodeEditContext: state.nodeEditContext?.[canvasId],Use optional chaining to comply with the coding guideline: "Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript."
📝 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.
Suggested change
| nodeEditContext: state.nodeEditContext[canvasId], | |
| nodeEditContext: state.nodeEditContext?.[canvasId], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx` at
line 92, The code directly indexes state.nodeEditContext[canvasId], which can be
undefined for missing keys; change the access to use optional chaining
(state.nodeEditContext?.[canvasId]) wherever nodeEditContext is indexed (e.g.,
in the object passed on line with nodeEditContext) to safely return undefined
instead of throwing—update usages in chat-box.tsx that reference
state.nodeEditContext[canvasId] to use ?.[] and keep existing variable names
(state, nodeEditContext, canvasId) unchanged.
Comment on lines
+11818
to
11821
| data: | ||
| $ref: '#/components/schemas/WorkflowPatchData' | ||
| description: Update data (for updateTask, updateVariable operations) | ||
| $ref: "#/components/schemas/WorkflowVariable" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, read the specific lines mentioned in the review
echo "=== Reading lines 11818-11821 from schema.yml ==="
sed -n '11818,11821p' packages/openapi-schema/schema.yml | cat -n
echo ""
echo "=== Reading broader context (lines 11810-11830) ==="
sed -n '11810,11830p' packages/openapi-schema/schema.yml | cat -nRepository: refly-ai/refly
Length of output: 1405
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import sys, subprocess, pkgutil
from pathlib import Path
try:
from ruamel.yaml import YAML
except Exception:
subprocess.check_call([sys.executable, "-m", "pip", "install", "ruamel.yaml", "-q"])
from ruamel.yaml import YAML
yaml = YAML(typ='rt')
yaml.allow_duplicate_keys = False
path = Path("packages/openapi-schema/schema.yml")
with path.open("r", encoding="utf-8") as f:
try:
result = yaml.load(f)
print("OK: YAML parsed successfully")
# Let's check what value was assigned to the 'data' key
print("\nChecking WorkflowPatchOperation.data structure:")
if 'components' in result and 'schemas' in result['components']:
if 'WorkflowPatchOperation' in result['components']['schemas']:
data_value = result['components']['schemas']['WorkflowPatchOperation']['properties'].get('data')
print(f"data value: {data_value}")
except Exception as e:
print("FAIL: YAML parsing error:", str(e)[:200])
PYRepository: refly-ai/refly
Length of output: 184
Remove the duplicate $ref under WorkflowPatchOperation.data.
Line 11821 contains a second $ref that causes YAML parsing to fail with "mapping values are not allowed here". The property should follow the same pattern as the adjacent variable property: a single $ref with a description. The second $ref must be removed.
🛠️ Minimal fix
data:
$ref: '#/components/schemas/WorkflowPatchData'
description: Update data (for updateTask, updateVariable operations)
- $ref: "#/components/schemas/WorkflowVariable"📝 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.
Suggested change
| data: | |
| $ref: '#/components/schemas/WorkflowPatchData' | |
| description: Update data (for updateTask, updateVariable operations) | |
| $ref: "#/components/schemas/WorkflowVariable" | |
| data: | |
| $ref: '#/components/schemas/WorkflowPatchData' | |
| description: Update data (for updateTask, updateVariable operations) |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 11821-11821: syntax error: mapping values are not allowed here
(syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/openapi-schema/schema.yml` around lines 11818 - 11821, The YAML for
WorkflowPatchOperation.data currently has a duplicate $ref entry causing a parse
error; edit the WorkflowPatchOperation.data property to remove the second/extra
"$ref: \"#/components/schemas/WorkflowVariable\"" so that it matches the pattern
used by the adjacent variable property — i.e., a single $ref pointing to
"#/components/schemas/WorkflowPatchData" and a description field referencing
WorkflowVariable as needed.
- Implemented getCanvasData method in SkillEngineService to retrieve canvas details with optional chaining and nullish coalescing for safety. - Added GetCanvasSnapshot tool to retrieve current canvas state, including nodes and edges, with proper error handling. - Updated tool service to include the new GetCanvasSnapshot tool and integrated it into the existing workflow. - Removed NodeEditBanner component from Copilot as it is no longer needed. - Added CopilotSnapshotRenderer for rendering canvas snapshot tool calls in markdown. - Updated internationalization files to include translations for canvas snapshot messages.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agent-tools/src/copilot/index.ts (1)
134-191:⚠️ Potential issue | 🟠 MajorUse the validated patch payload consistently after parsing.
Line 135 validates with
parseWorkflowPlanPatch, but Lines 144, 180, and 191 still read from rawinput. That can bypass parser normalization/sanitization.Suggested fix
- let planId = input.planId; + const validatedInput = parsed.data; + let planId = validatedInput.planId; @@ const result = await reflyService.patchWorkflowPlan(user, { planId: planId!, - operations: input.operations as WorkflowPatchOperation[], + operations: validatedInput.operations as WorkflowPatchOperation[], resultId, resultVersion, }); @@ - summary: `Successfully patched workflow plan with ID: ${result.planId} and created new version: ${result.version}. Applied ${input.operations.length} operation(s).`, + summary: `Successfully patched workflow plan with ID: ${result.planId} and created new version: ${result.version}. Applied ${validatedInput.operations.length} operation(s).`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` around lines 134 - 191, The patch validator parseWorkflowPlanPatch returns normalized/sanitized data in parsed.value but the code continues to use the raw input; update all places that currently read from input to use parsed.value instead — specifically: initialize planId from parsed.value.planId (not input.planId), pass parsed.value.operations to reflyService.patchWorkflowPlan (instead of input.operations) and use parsed.value.operations.length in the success summary; keep existing checks (copilotSessionId/resultId/resultVersion) and types but consistently consume parsed.value for the validated payload when calling patchWorkflowPlan and when composing the response summary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/modules/skill/skill-engine.service.ts`:
- Around line 145-150: The returned object from the call to getCanvasRawData
uses nullish coalescing for title/nodes/edges but leaves variables possibly
undefined; update the return to default variables as well (e.g., variables:
rawData.variables ?? {}) so callers always receive a defined value; locate the
return in SkillEngineService where rawData is assigned from
this.canvasService.getCanvasRawData and add the nullish-coalescing default for
rawData.variables in that returned object.
In `@packages/agent-tools/src/copilot/index.ts`:
- Around line 383-391: The snapshot currently returns raw canvasData.variables
which can leak secrets/PII; instead, replace that field with a minimized
projection (e.g., map canvasData.variables to only non-sensitive metadata such
as {name, type, readOnly, description} or a masked placeholder for value) and
ensure no raw value/secret fields are included. Locate the return object that
contains nodes: nodesToReturn and edges: edgesToReturn (the block referencing
canvasData.variables) and change variables: canvasData.variables to variables:
canvasData.variables.map(v => ({ name: v.name, type: v.type, description:
v.description, value: '[REDACTED]' })) or similar projection that omits any raw
value/secret properties.
---
Outside diff comments:
In `@packages/agent-tools/src/copilot/index.ts`:
- Around line 134-191: The patch validator parseWorkflowPlanPatch returns
normalized/sanitized data in parsed.value but the code continues to use the raw
input; update all places that currently read from input to use parsed.value
instead — specifically: initialize planId from parsed.value.planId (not
input.planId), pass parsed.value.operations to reflyService.patchWorkflowPlan
(instead of input.operations) and use parsed.value.operations.length in the
success summary; keep existing checks (copilotSessionId/resultId/resultVersion)
and types but consistently consume parsed.value for the validated payload when
calling patchWorkflowPlan and when composing the response summary.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/api/src/modules/skill/skill-engine.service.tsapps/api/src/modules/tool/tool.service.tspackages/agent-tools/src/builtin/interface.tspackages/agent-tools/src/copilot/index.tspackages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsxpackages/ai-workspace-common/src/components/markdown/plugins/tool-call/internal-tool-renderers/copilot-snapshot-renderer.tsxpackages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsxpackages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsx
- packages/agent-tools/src/builtin/interface.ts
- packages/i18n/src/en-US/ui.ts
Comment on lines
+145
to
+150
| const rawData = await this.canvasService.getCanvasRawData(user, params.canvasId); | ||
| return { | ||
| title: rawData.title ?? '', | ||
| nodes: rawData.nodes ?? [], | ||
| edges: rawData.edges ?? [], | ||
| variables: rawData.variables, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd 'canvas.service.ts' -t fRepository: refly-ai/refly
Length of output: 103
🏁 Script executed:
rg -n 'getCanvasRawData' --type ts -C 5Repository: refly-ai/refly
Length of output: 23668
🏁 Script executed:
rg -n 'CanvasRawData|type.*Canvas.*Data' --type tsRepository: refly-ai/refly
Length of output: 10687
🏁 Script executed:
cat -n apps/api/src/modules/skill/skill-engine.service.ts | sed -n '140,155p'Repository: refly-ai/refly
Length of output: 849
🏁 Script executed:
rg -n 'type RawCanvasData' --type ts -A 15Repository: refly-ai/refly
Length of output: 2194
🏁 Script executed:
cat -n packages/openapi-schema/src/types.gen.ts | sed -n '3246,3270p'Repository: refly-ai/refly
Length of output: 630
🏁 Script executed:
cat -n packages/openapi-schema/src/types.gen.ts | sed -n '3202,3245p'Repository: refly-ai/refly
Length of output: 1100
🏁 Script executed:
rg -n 'export type CanvasData' --type ts -A 20 packages/openapi-schema/src/types.gen.ts | head -40Repository: refly-ai/refly
Length of output: 497
🏁 Script executed:
cat -n apps/api/src/modules/canvas/canvas.service.ts | sed -n '259,305p'Repository: refly-ai/refly
Length of output: 1587
🏁 Script executed:
rg -n 'variables:' apps/api/src/modules/skill/skill-engine.service.ts -B 3 -A 3Repository: refly-ai/refly
Length of output: 309
🏁 Script executed:
# Check how getCanvasData result is used in skill-engine
ast-grep --pattern 'getCanvasData.*{
$$$
}'Repository: refly-ai/refly
Length of output: 40
Normalize variables default for consistency with other properties.
The title, nodes, and edges properties all apply nullish coalescing defaults, but variables does not. Since variables is an optional property in RawCanvasData, it should also be defaulted to maintain consistent handling of potentially undefined values.
Proposed fix
getCanvasData: async (user, params) => {
const rawData = await this.canvasService.getCanvasRawData(user, params.canvasId);
return {
title: rawData.title ?? '',
nodes: rawData.nodes ?? [],
edges: rawData.edges ?? [],
- variables: rawData.variables,
+ variables: rawData.variables ?? [],
};
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/modules/skill/skill-engine.service.ts` around lines 145 - 150,
The returned object from the call to getCanvasRawData uses nullish coalescing
for title/nodes/edges but leaves variables possibly undefined; update the return
to default variables as well (e.g., variables: rawData.variables ?? {}) so
callers always receive a defined value; locate the return in SkillEngineService
where rawData is assigned from this.canvasService.getCanvasRawData and add the
nullish-coalescing default for rawData.variables in that returned object.
Comment on lines
+383
to
+391
| data: { | ||
| title: canvasData.title, | ||
| nodeCount: allNodes.length, | ||
| edgeCount: allEdges.length, | ||
| nodeTypeCounts, | ||
| nodes: nodesToReturn, | ||
| edges: edgesToReturn, | ||
| variables: canvasData.variables, | ||
| truncated, |
There was a problem hiding this comment.
Redact variable payloads in canvas snapshots.
Line 390 returns raw canvasData.variables, which can leak secrets/PII through tool output. Return a minimized projection instead.
Suggested fix
- variables: canvasData.variables,
+ variables: (canvasData.variables ?? []).map((variable) => ({
+ variableId: variable.variableId,
+ name: variable.name,
+ variableType: variable.variableType,
+ required: variable.required,
+ })),📝 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.
Suggested change
| data: { | |
| title: canvasData.title, | |
| nodeCount: allNodes.length, | |
| edgeCount: allEdges.length, | |
| nodeTypeCounts, | |
| nodes: nodesToReturn, | |
| edges: edgesToReturn, | |
| variables: canvasData.variables, | |
| truncated, | |
| data: { | |
| title: canvasData.title, | |
| nodeCount: allNodes.length, | |
| edgeCount: allEdges.length, | |
| nodeTypeCounts, | |
| nodes: nodesToReturn, | |
| edges: edgesToReturn, | |
| variables: (canvasData.variables ?? []).map((variable) => ({ | |
| variableId: variable.variableId, | |
| name: variable.name, | |
| variableType: variable.variableType, | |
| required: variable.required, | |
| })), | |
| truncated, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-tools/src/copilot/index.ts` around lines 383 - 391, The
snapshot currently returns raw canvasData.variables which can leak secrets/PII;
instead, replace that field with a minimized projection (e.g., map
canvasData.variables to only non-sensitive metadata such as {name, type,
readOnly, description} or a masked placeholder for value) and ensure no raw
value/secret fields are included. Locate the return object that contains nodes:
nodesToReturn and edges: edgesToReturn (the block referencing
canvasData.variables) and change variables: canvasData.variables to variables:
canvasData.variables.map(v => ({ name: v.name, type: v.type, description:
v.description, value: '[REDACTED]' })) or similar projection that omits any raw
value/secret properties.
- Updated the getCanvasData method to use specific types for nodes, edges, and variables, enhancing type safety. - Removed the unused NodeEditBanner component to streamline the codebase. - Simplified variable handling in CopilotMessage by directly using finalPlan.variables. - Improved metadata access in applyIncrementalChangesToCanvas with optional chaining. - Cleaned up internationalization files by removing obsolete node edit translations.
- Implemented getLatestWorkflowPlanByCanvas in WorkflowPlanService to retrieve the latest workflow plan for a specific canvas based on the most recent copilot session. - Updated SkillEngineService to include the new method in the service interface. - Modified ReflyService interface to declare the new method for consistency. - Enhanced error handling for missing canvas ID in the new method. - Updated Copilot logic to utilize the new method for retrieving workflow plans when the plan ID is not provided.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/canvas-common/src/workflow-plan.ts (1)
962-964:⚠️ Potential issue | 🟡 MinorApply same type normalization for variableType.
This cast has the same issue—normalize before assignment.
if (data.variableType !== undefined) { - updateData.variableType = data.variableType as 'string' | 'option' | 'resource'; + updateData.variableType = + data.variableType === 'string' || + data.variableType === 'option' || + data.variableType === 'resource' + ? data.variableType + : 'string'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas-common/src/workflow-plan.ts` around lines 962 - 964, The assignment to updateData.variableType currently just casts data.variableType; instead normalize the incoming value before assignment (same approach used elsewhere in this file for other fields). Replace the direct cast of data.variableType with a normalization step that maps/validates data.variableType into the allowed union ('string' | 'option' | 'resource') and only then set updateData.variableType; use the same normalization helper or logic pattern used for other fields in workflow-plan.ts to ensure invalid values are handled consistently for data.variableType and updateData.variableType.packages/agent-tools/src/copilot/index.ts (1)
420-420:⚠️ Potential issue | 🟠 MajorRedact variables in canvas snapshot output (still leaking raw payloads).
Line 420 returns
canvasData.variablesdirectly, which can expose secret/PII fields through tool output. Return a minimized projection instead.🔐 Proposed fix
- variables: canvasData.variables, + variables: (canvasData.variables ?? []).map((variable) => ({ + variableId: variable.variableId, + name: variable.name, + variableType: variable.variableType, + required: variable.required, + })),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` at line 420, Don't return canvasData.variables directly in the canvas snapshot output; instead replace the raw payload with a sanitized projection that strips or masks sensitive values. Locate where canvasData.variables is included in the snapshot, iterate its entries and produce a minimized shape (e.g., { key: variableName, type: typeof value, value: "[REDACTED]" } or include metadata like length/type but never the raw value), and return that projection in place of canvasData.variables so no secret/PII is leaked.
🧹 Nitpick comments (3)
packages/canvas-common/src/workflow-plan.ts (1)
303-319: Consider avoiding double cast with proper type alignment.The
as unknown aspattern is typically a code smell. Sincedatacomes from a validated schema, consider aligning the OpenAPI type with the Zod schema to avoid this intermediate cast, or extract a shared type definition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas-common/src/workflow-plan.ts` around lines 303 - 319, The double-cast "data as unknown as Partial<z.infer<typeof workflowPatchDataSchema>>" is a smell; instead align the actual parameter/type with the Zod schema by creating a shared type alias (e.g., type WorkflowPatchData = z.infer<typeof workflowPatchDataSchema>) and use Partial<WorkflowPatchData> for the incoming data or the function parameter so you can remove the intermediate cast and directly use that typed variable where typedData is referenced (affecting workflowPatchDataSchema, the typedData usage, and construction of updatedVar); alternatively ensure the OpenAPI-generated type matches WorkflowPatchData so no runtime cast is necessary.packages/agent-tools/src/copilot/index.ts (2)
11-16: Useimport typefor type-only OpenAPI imports.These symbols (
User,WorkflowPlanRecord,WorkflowPatchOperation,CanvasNode) are used only in type annotations throughout this file. Convert toimport typeto make this explicit and prevent unnecessary runtime import emission.Proposed fix
-import { +import type { User, WorkflowPlanRecord, WorkflowPatchOperation, CanvasNode, } from '@refly/openapi-schema';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` around lines 11 - 16, The import currently brings runtime values for OpenAPI types; change the line importing User, WorkflowPlanRecord, WorkflowPatchOperation, and CanvasNode so it uses a type-only import (e.g., `import type { User, WorkflowPlanRecord, WorkflowPatchOperation, CanvasNode } from '@refly/openapi-schema'`) so these symbols are treated as types only and do not emit runtime imports; update the single import statement in packages/agent-tools/src/copilot/index.ts where those symbols are referenced.
374-393: Replaceanytype annotations with proper typed guards.Lines 374–392 use
anytype which weakens type safety and makes metadata handling brittle. The code would benefit from explicit typing with proper type guards and optional chaining for safer property access.Suggested refactoring
+ type SnapshotToolset = { id?: string; toolset?: { key?: string } }; + type SnapshotMetadata = { + query?: string; + selectedToolsets?: SnapshotToolset[]; + taskId?: string; + }; - const metadata = (node.data as any)?.metadata; - const base: Record<string, any> = { + const metadata = (node.data as { metadata?: SnapshotMetadata } | undefined)?.metadata; + const base: Record<string, unknown> = { id: node.id, type: node.type, title: node.data?.title, entityId: node.data?.entityId, contentPreview: node.data?.contentPreview ? truncateContent(node.data.contentPreview, MAX_PREVIEW_LENGTH) : undefined, }; // For skillResponse nodes, include query and toolsets so Agent can understand the workflow if (node.type === 'skillResponse' && metadata) { - if (metadata.query) { + if (metadata?.query) { base.query = truncateContent(metadata.query, 200); } - if (metadata.selectedToolsets?.length) { + if (metadata?.selectedToolsets?.length) { base.toolsets = metadata.selectedToolsets - .map((t: any) => t.id ?? t.toolset?.key) + .map((t) => t.id ?? t.toolset?.key) - .filter(Boolean); + .filter((key): key is string => Boolean(key)); } - if (metadata.taskId) { + if (metadata?.taskId) { base.taskId = metadata.taskId; } }Aligns with guideline:
**/*.{ts,tsx}: Avoid using any type – use unknown type instead with proper type guards.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` around lines 374 - 393, The code is using loose any types for node.data and metadata; replace any with unknown and add explicit type guards (e.g., isNodeData, isSkillResponseMetadata) that validate shapes before access, then narrow types for node.data and metadata prior to building base; specifically update references around node, (node.data as any)?.metadata, and the skillResponse branch to use the guards and optional chaining so truncateContent, MAX_PREVIEW_LENGTH, metadata.query, and metadata.selectedToolsets are only accessed when their types are confirmed (use typeof checks for strings and Array.isArray for selectedToolsets, and map using validated toolset objects with id or toolset.key).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/modules/workflow/workflow-plan.service.ts`:
- Around line 235-268: Update the method return types to match the DTO
conversion: change getLatestWorkflowPlanByCanvas and getLatestWorkflowPlan
signatures to return Promise<WorkflowPlanRecord | null> (instead of
Promise<WorkflowPlan | null>) because workflowPlanPO2DTO returns a
WorkflowPlanRecord; ensure any related type imports/exports align with
WorkflowPlanRecord so the compiler sees the correct return type.
In `@packages/agent-tools/src/copilot/index.ts`:
- Around line 186-189: The call to reflyService.patchWorkflowPlan is passing
unsafely-cast input.operations; instead use the validated parser output from
parseWorkflowPlanPatch (which runs against workflowPlanPatchSchema). Replace the
type assertion input.operations as WorkflowPatchOperation[] with
parsed.data.operations (the result of parseWorkflowPlanPatch) when constructing
the patch payload so the service receives the validated/transformed operations
array.
In `@packages/canvas-common/src/workflow-plan.ts`:
- Around line 941-975: In the updateVariable case, normalize and validate
enum-like fields instead of blind casting: when building convertedValue from
data.value, explicitly map v.type to either 'text' or 'resource' (defaulting to
'text' for unknown values) and validate v.resource.fileType against the allowed
set ('document'|'image'|'video'|'audio') before assigning to
result.resource.fileType; likewise normalize data.variableType into the allowed
WorkflowVariable types ('string'|'option'|'resource') before setting
updateData.variableType. Update the logic around convertedValue, updateData, and
the object pushed to variableOperations so only validated, normalized enum
values (not unchecked casts) are stored.
---
Duplicate comments:
In `@packages/agent-tools/src/copilot/index.ts`:
- Line 420: Don't return canvasData.variables directly in the canvas snapshot
output; instead replace the raw payload with a sanitized projection that strips
or masks sensitive values. Locate where canvasData.variables is included in the
snapshot, iterate its entries and produce a minimized shape (e.g., { key:
variableName, type: typeof value, value: "[REDACTED]" } or include metadata like
length/type but never the raw value), and return that projection in place of
canvasData.variables so no secret/PII is leaked.
In `@packages/canvas-common/src/workflow-plan.ts`:
- Around line 962-964: The assignment to updateData.variableType currently just
casts data.variableType; instead normalize the incoming value before assignment
(same approach used elsewhere in this file for other fields). Replace the direct
cast of data.variableType with a normalization step that maps/validates
data.variableType into the allowed union ('string' | 'option' | 'resource') and
only then set updateData.variableType; use the same normalization helper or
logic pattern used for other fields in workflow-plan.ts to ensure invalid values
are handled consistently for data.variableType and updateData.variableType.
---
Nitpick comments:
In `@packages/agent-tools/src/copilot/index.ts`:
- Around line 11-16: The import currently brings runtime values for OpenAPI
types; change the line importing User, WorkflowPlanRecord,
WorkflowPatchOperation, and CanvasNode so it uses a type-only import (e.g.,
`import type { User, WorkflowPlanRecord, WorkflowPatchOperation, CanvasNode }
from '@refly/openapi-schema'`) so these symbols are treated as types only and do
not emit runtime imports; update the single import statement in
packages/agent-tools/src/copilot/index.ts where those symbols are referenced.
- Around line 374-393: The code is using loose any types for node.data and
metadata; replace any with unknown and add explicit type guards (e.g.,
isNodeData, isSkillResponseMetadata) that validate shapes before access, then
narrow types for node.data and metadata prior to building base; specifically
update references around node, (node.data as any)?.metadata, and the
skillResponse branch to use the guards and optional chaining so truncateContent,
MAX_PREVIEW_LENGTH, metadata.query, and metadata.selectedToolsets are only
accessed when their types are confirmed (use typeof checks for strings and
Array.isArray for selectedToolsets, and map using validated toolset objects with
id or toolset.key).
In `@packages/canvas-common/src/workflow-plan.ts`:
- Around line 303-319: The double-cast "data as unknown as
Partial<z.infer<typeof workflowPatchDataSchema>>" is a smell; instead align the
actual parameter/type with the Zod schema by creating a shared type alias (e.g.,
type WorkflowPatchData = z.infer<typeof workflowPatchDataSchema>) and use
Partial<WorkflowPatchData> for the incoming data or the function parameter so
you can remove the intermediate cast and directly use that typed variable where
typedData is referenced (affecting workflowPatchDataSchema, the typedData usage,
and construction of updatedVar); alternatively ensure the OpenAPI-generated type
matches WorkflowPatchData so no runtime cast is necessary.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/api/src/modules/skill/skill-engine.service.tsapps/api/src/modules/workflow/workflow-plan.service.tspackages/agent-tools/src/builtin/interface.tspackages/agent-tools/src/copilot/index.tspackages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsxpackages/canvas-common/src/workflow-plan.tspackages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.tspackages/stores/src/stores/copilot.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/i18n/src/zh-Hans/ui.ts
- packages/stores/src/stores/copilot.ts
- apps/api/src/modules/skill/skill-engine.service.ts
- packages/agent-tools/src/builtin/interface.ts
Comment on lines
+235
to
+268
| /** | ||
| * Get the latest workflow plan for a canvas by looking up the most recent copilot session | ||
| */ | ||
| async getLatestWorkflowPlanByCanvas( | ||
| user: User, | ||
| params: { canvasId: string }, | ||
| ): Promise<WorkflowPlan | null> { | ||
| const { canvasId } = params; | ||
| if (!canvasId) { | ||
| throw new ParamsError('Canvas ID is required'); | ||
| } | ||
|
|
||
| // Step 1: Find the most recent copilot session for this canvas | ||
| const session = await this.prisma.copilotSession.findFirst({ | ||
| where: { canvasId, uid: user.uid }, | ||
| orderBy: { createdAt: 'desc' }, | ||
| }); | ||
|
|
||
| if (!session) { | ||
| return null; | ||
| } | ||
|
|
||
| // Step 2: Find the latest plan version in that session | ||
| const latestPlan = await this.prisma.workflowPlan.findFirst({ | ||
| where: { copilotSessionId: session.sessionId, uid: user.uid }, | ||
| orderBy: { version: 'desc' }, | ||
| }); | ||
|
|
||
| if (!latestPlan) { | ||
| return null; | ||
| } | ||
|
|
||
| return this.workflowPlanPO2DTO(latestPlan as WorkflowPlanPO); | ||
| } |
There was a problem hiding this comment.
Return type mismatch in method signature.
The JSDoc states the method returns WorkflowPlan | null but workflowPlanPO2DTO returns WorkflowPlanRecord. The same inconsistency exists in getLatestWorkflowPlan on line 217. Both methods should declare Promise<WorkflowPlanRecord | null> for consistency with the DTO conversion.
🔧 Proposed fix
/**
* Get the latest workflow plan for a canvas by looking up the most recent copilot session
*/
async getLatestWorkflowPlanByCanvas(
user: User,
params: { canvasId: string },
- ): Promise<WorkflowPlan | null> {
+ ): Promise<WorkflowPlanRecord | null> {
const { canvasId } = params;Similarly for getLatestWorkflowPlan:
async getLatestWorkflowPlan(
user: User,
params: { copilotSessionId: string },
- ): Promise<WorkflowPlan | null> {
+ ): Promise<WorkflowPlanRecord | null> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/modules/workflow/workflow-plan.service.ts` around lines 235 -
268, Update the method return types to match the DTO conversion: change
getLatestWorkflowPlanByCanvas and getLatestWorkflowPlan signatures to return
Promise<WorkflowPlanRecord | null> (instead of Promise<WorkflowPlan | null>)
because workflowPlanPO2DTO returns a WorkflowPlanRecord; ensure any related type
imports/exports align with WorkflowPlanRecord so the compiler sees the correct
return type.
Comment on lines
186
to
189
| const result = await reflyService.patchWorkflowPlan(user, { | ||
| planId: planId!, | ||
| operations: input.operations, | ||
| operations: input.operations as WorkflowPatchOperation[], | ||
| resultId, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and the specific lines in question
fd "index.ts" packages/agent-tools/src/copilot --type fRepository: refly-ai/refly
Length of output: 99
🏁 Script executed:
# Get the actual code context around lines 186-189
cat -n packages/agent-tools/src/copilot/index.ts | sed -n '175,200p'Repository: refly-ai/refly
Length of output: 1106
🏁 Script executed:
# Find where parseWorkflowPlanPatch is defined and what it returns
rg -n "parseWorkflowPlanPatch" packages/agent-tools/src/copilot/ --type=ts -A 5Repository: refly-ai/refly
Length of output: 975
🏁 Script executed:
# Look for the schema definition
rg -n "workflowPlanPatchSchema" packages/agent-tools/src/copilot/ --type=ts -B 2 -A 5Repository: refly-ai/refly
Length of output: 1177
🏁 Script executed:
# Find where parseWorkflowPlanPatch is defined - likely in canvas-common
fd "canvas-common" --type dRepository: refly-ai/refly
Length of output: 81
🏁 Script executed:
# Look for schema or parser definitions related to workflowPlanPatch
rg -n "parseWorkflowPlanPatch\s*=" --type=ts -A 10Repository: refly-ai/refly
Length of output: 933
🏁 Script executed:
# Also check imports - maybe it's defined in a separate file
rg -n "export.*parseWorkflowPlanPatch" --type=ts -A 5Repository: refly-ai/refly
Length of output: 556
🏁 Script executed:
# Find WorkflowPlanPatch type definition
rg -n "type WorkflowPlanPatch|interface WorkflowPlanPatch" --type=ts -A 15Repository: refly-ai/refly
Length of output: 1248
🏁 Script executed:
# Also look for workflowPlanPatchSchema definition
rg -n "workflowPlanPatchSchema\s*=" --type=ts -A 20Repository: refly-ai/refly
Length of output: 1956
🏁 Script executed:
# Show the complete flow from line 135 to 191
cat -n packages/agent-tools/src/copilot/index.ts | sed -n '135,191p'Repository: refly-ai/refly
Length of output: 2283
Use validated schema output instead of type-casting raw input.
parseWorkflowPlanPatch validates and transforms the input according to workflowPlanPatchSchema, but line 188 still uses input.operations as WorkflowPatchOperation[] with an unsafe type assertion. Replace with parsed.data.operations to use the validated, properly-typed output from the parser.
Proposed fix
const result = await reflyService.patchWorkflowPlan(user, {
planId: planId!,
- operations: input.operations as WorkflowPatchOperation[],
+ operations: parsed.data.operations,
resultId,
resultVersion,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/agent-tools/src/copilot/index.ts` around lines 186 - 189, The call
to reflyService.patchWorkflowPlan is passing unsafely-cast input.operations;
instead use the validated parser output from parseWorkflowPlanPatch (which runs
against workflowPlanPatchSchema). Replace the type assertion input.operations as
WorkflowPatchOperation[] with parsed.data.operations (the result of
parseWorkflowPlanPatch) when constructing the patch payload so the service
receives the validated/transformed operations array.
Comment on lines
+941
to
+975
| case 'updateVariable': { | ||
| if (variableId && data) { | ||
| // Convert value array to ensure proper typing | ||
| const convertedValue: VariableValue[] | undefined = data.value?.map((v) => { | ||
| const result: VariableValue = { | ||
| type: (v?.type as 'text' | 'resource') ?? 'text', | ||
| text: v?.text, | ||
| }; | ||
| // Only include resource if it has the required fields | ||
| if (v?.resource?.name && v?.resource?.fileType) { | ||
| result.resource = { | ||
| name: v.resource.name, | ||
| fileType: v.resource.fileType as 'document' | 'image' | 'video' | 'audio', | ||
| }; | ||
| } | ||
| return result; | ||
| }); | ||
|
|
||
| const updateData: Partial<WorkflowVariable> = {}; | ||
| if (data.name !== undefined) updateData.name = data.name; | ||
| if (data.description !== undefined) updateData.description = data.description; | ||
| if (data.variableType !== undefined) { | ||
| updateData.variableType = data.variableType as 'string' | 'option' | 'resource'; | ||
| } | ||
| if (data.required !== undefined) updateData.required = data.required; | ||
| if (data.resourceTypes !== undefined) updateData.resourceTypes = data.resourceTypes; | ||
| if (convertedValue !== undefined) updateData.value = convertedValue; | ||
|
|
||
| variableOperations.push({ | ||
| type: 'update', | ||
| variableId, | ||
| data: updateData, | ||
| }); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Apply consistent type normalization for variable updates.
The updateVariable case has the same loose type casting issue as planVariableToWorkflowVariable. Line 946 casts v?.type and line 953 casts fileType without validation, which can propagate invalid enum values.
🔧 Proposed fix for consistent normalization
const convertedValue: VariableValue[] | undefined = data.value?.map((v) => {
const result: VariableValue = {
- type: (v?.type as 'text' | 'resource') ?? 'text',
+ type: v?.type === 'resource' ? 'resource' : 'text',
text: v?.text,
};
// Only include resource if it has the required fields
if (v?.resource?.name && v?.resource?.fileType) {
+ const normalizedFileType =
+ v.resource.fileType === 'document' ||
+ v.resource.fileType === 'image' ||
+ v.resource.fileType === 'video' ||
+ v.resource.fileType === 'audio'
+ ? v.resource.fileType
+ : 'document';
result.resource = {
name: v.resource.name,
- fileType: v.resource.fileType as 'document' | 'image' | 'video' | 'audio',
+ fileType: normalizedFileType,
};
}
return result;
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas-common/src/workflow-plan.ts` around lines 941 - 975, In the
updateVariable case, normalize and validate enum-like fields instead of blind
casting: when building convertedValue from data.value, explicitly map v.type to
either 'text' or 'resource' (defaulting to 'text' for unknown values) and
validate v.resource.fileType against the allowed set
('document'|'image'|'video'|'audio') before assigning to
result.resource.fileType; likewise normalize data.variableType into the allowed
WorkflowVariable types ('string'|'option'|'resource') before setting
updateData.variableType. Update the logic around convertedValue, updateData, and
the object pushed to variableOperations so only validated, normalized enum
values (not unchecked casts) are stored.
…apshot usage - Added new entries to the Tool Selection Guide for `get_canvas_snapshot` to clarify its use cases. - Updated the Default Preference section to include guidance on when to use `get_canvas_snapshot`. - Introduced a new section on `get_canvas_snapshot` usage, detailing scenarios and expected data returned. - Included important fallback instructions for handling cases when no workflow plan exists.
anthhub
changed the title
Deploying refly-branch-test with
|
| Latest commit: |
8f38d81
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9b1276b4.refly-branch-test.pages.dev |
| Branch Preview URL: | https://feat-copilot-patch.refly-branch-test.pages.dev |
…MCP server configuration
mrcfps
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This update improves user experience by providing clear context and options for editing nodes directly within the Copilot interface.
Summary by CodeRabbit
Release Notes