fix(web): remove @ts-nocheck from workflow-editor and enforce strict checks#9
Merged
matheusgalvao1 merged 3 commits intomainfrom Feb 25, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThe PR introduces extensive TypeScript typings and explicit method signatures across the workflow editor, adding interfaces for workflow state, nodes, ports, conditions, UI instances, and run results. It replaces many loose inline typings with stronger types (with some provisional Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/workflow-editor.ts (1)
317-327:⚠️ Potential issue | 🟠 MajorReplace broad
anyannotations on core editor APIs.This change compiles, but these
anysignatures erase most of the strict-mode guarantees you're trying to restore. The file defines type aliases (WorkflowState,Point,Viewport,IfCondition,WorkflowNodeData) but many core methods still use untypedanyparameters (state, node, connection, event, and coordinate flows are unchecked).🔧 Tightening example
+type DropdownItem = { value: string; label: string }; -async setupDropdown(container: any, items: any, selectedValue: any, placeholder: any, onSelect: any) { +async setupDropdown( + container: HTMLElement, + items: DropdownItem[], + selectedValue: string, + placeholder: string, + onSelect: (value: string) => void +) { -setWorkflowState(state: any) { +setWorkflowState(state: WorkflowState) { -screenToWorld(clientX: any, clientY: any) { +screenToWorld(clientX: number, clientY: number): Point { -addNode(type: any, x: any, y: any) { +addNode(type: string, x: number, y: number) {Per coding guidelines, "Use TypeScript strict mode everywhere" and "Use TypeScript with
strictmode enabled across all apps and packages".Also applies to: 341-348, 452-455, 875-883, 1024-1034, 1290-1296, 1558-1567, 1972-1978
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 317 - 327, The method setupDropdown and several other core editor APIs use broad any types which undermines strict-mode—replace those anys with concrete types: change setupDropdown(container: any, items: any, selectedValue: any, placeholder: any, onSelect: any) to a typed signature (e.g., container: HTMLElement | string, items: DropdownItem[] or the editor's DropdownItem type, selectedValue: string | number | null, placeholder: string, onSelect: (value: string | number) => void) and use the actual Dropdown constructor type returned by getDropdownCtor() for DropdownCtor; similarly audit and replace anys in the other affected methods (the ones interacting with WorkflowState, Point, Viewport, IfCondition, WorkflowNodeData, node, connection, and event flows) to use those declared aliases (WorkflowState, Point, Viewport, IfCondition, WorkflowNodeData) or explicit DOM/event types so the compiler enforces strict types across setupDropdown, getDropdownCtor, and the listed API methods.
🤖 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/web/src/app/workflow-editor.ts`:
- Around line 1359-1364: The code assumes effortOptions has at least one element
and dereferences effortOptions[0].value which can throw; update the logic around
effortOptions/selectedEffort in the block that builds effortOptions (using
this.modelEfforts and this.modelOptions) to first check if effortOptions.length
> 0 and only then use effortOptions[0].value, otherwise pick a safe fallback
(e.g., keep node.data.reasoningEffort unchanged, set to null/empty string, or a
defined default) so node.data.reasoningEffort is not assigned from an undefined
index; ensure references to effortOptions, selectedEffort and
node.data.reasoningEffort are guarded accordingly.
---
Outside diff comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 317-327: The method setupDropdown and several other core editor
APIs use broad any types which undermines strict-mode—replace those anys with
concrete types: change setupDropdown(container: any, items: any, selectedValue:
any, placeholder: any, onSelect: any) to a typed signature (e.g., container:
HTMLElement | string, items: DropdownItem[] or the editor's DropdownItem type,
selectedValue: string | number | null, placeholder: string, onSelect: (value:
string | number) => void) and use the actual Dropdown constructor type returned
by getDropdownCtor() for DropdownCtor; similarly audit and replace anys in the
other affected methods (the ones interacting with WorkflowState, Point,
Viewport, IfCondition, WorkflowNodeData, node, connection, and event flows) to
use those declared aliases (WorkflowState, Point, Viewport, IfCondition,
WorkflowNodeData) or explicit DOM/event types so the compiler enforces strict
types across setupDropdown, getDropdownCtor, and the listed API methods.
Collaborator
Author
|
Addressed CodeRabbit feedback in commit ee04610. What was fixed:
Validation rerun:
|
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 (2)
apps/web/src/app/workflow-editor.ts (2)
872-877:⚠️ Potential issue | 🟠 Major
this.approveBtnandthis.rejectBtnare alwaysnullatinitButtonstime — these listeners are dead code.
this.approveBtnandthis.rejectBtnare initialized tonull(lines 265–266) and are only populated insideshowApprovalMessageat lines 1854–1859, where click handlers are already wired directly. Theif (this.approveBtn)guards ininitButtonswill never pass on the first (and only) call.🗑️ Suggested cleanup
- if (this.approveBtn) { - this.approveBtn.addEventListener('click', () => this.submitApprovalDecision('approve')); - } - if (this.rejectBtn) { - this.rejectBtn.addEventListener('click', () => this.submitApprovalDecision('reject')); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 872 - 877, initButtons currently adds click listeners guarded by checks on this.approveBtn and this.rejectBtn but those properties are null at initButtons time and are later assigned (and already wired) in showApprovalMessage, so the listeners are dead code; remove the redundant conditional blocks in initButtons that reference this.approveBtn and this.rejectBtn (and their addEventListener calls) to avoid dead branches, keeping the click wiring that exists in showApprovalMessage and ensuring submitApprovalDecision remains called only from the handlers defined where the buttons are actually created.
1731-1736:⚠️ Potential issue | 🟠 Major
connection.sourceHandle(typestring | undefined) assigned toConnectionStart.handle(typestring).
WorkflowConnection.sourceHandleis optional (sourceHandle?: stringin@agentic/types), making itstring | undefined. TheConnectionStart.handlefield requiresstringwith no undefined allowed. This assignment violates TypeScript strict mode's type safety and should be guarded with a fallback value.Fix:
- handle: connection.sourceHandle, + handle: connection.sourceHandle ?? '',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1731 - 1736, The assignment to this.connectionStart sets ConnectionStart.handle (type string) from connection.sourceHandle which is WorkflowConnection.sourceHandle (type string | undefined); ensure you guard against undefined by providing a fallback before assigning. Update the logic around this.connectionStart (the block that assigns nodeId/handle/x/y) to coerce connection.sourceHandle to a definite string (e.g., use a default like "" or a sensible handle constant) or perform a conditional check and set handle accordingly so ConnectionStart.handle always receives a string value.
♻️ Duplicate comments (1)
apps/web/src/app/workflow-editor.ts (1)
1430-1452: The previouseffortOptions[0]crash is now correctly guarded.
effortOptions[0]?.value ?? ''and the wrappingif (selectedEffort)guard prevent the node rendering crash when no effort options are configured.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1430 - 1452, The code previously crashed when no effort options existed; ensure you compute modelEfforts and effortSource (using modelEfforts || this.modelEfforts[this.modelOptions[0]] || []) then build effortOptions, pick selectedEffort via effortOptions.find(...)? .value ?? effortOptions[0]?.value ?? '' and only call this.setupDropdown (with effortDropdown, effortOptions, selectedEffort, 'Select effort', ...) and assign data.reasoningEffort when selectedEffort is truthy to avoid rendering with empty values; update references to modelEfforts, effortSource, effortOptions, selectedEffort, effortDropdown, data.reasoningEffort and this.setupDropdown accordingly.
🧹 Nitpick comments (11)
apps/web/src/app/workflow-editor.ts (11)
2082-2087:result.status !== 'paused'in the finalelsebranch is alwaystrue.By the time this
elsebranch is reached,'paused'and'completed'and'failed'have all been handled above. The inner check is unreachable as written.🧹 Suggested cleanup
} else { this.clearApprovalMessage(); - if (result.status !== 'paused') { - this.hideAgentSpinner(); - this.setWorkflowState('idle'); - } + this.hideAgentSpinner(); + this.setWorkflowState('idle'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 2082 - 2087, The inner conditional check "result.status !== 'paused'" inside the final else branch is unreachable because 'paused', 'completed', and 'failed' are already handled earlier; remove the redundant check and always run the cleanup logic (call clearApprovalMessage(), hideAgentSpinner(), and setWorkflowState('idle')) in that branch, or if a specific status should be excluded, replace the condition with the correct status check on result.status to reflect the intended behavior.
256-256: Redundant inline comment —WorkflowStatetype makes it obsolete.🧹 Suggested cleanup
- this.workflowState = 'idle'; // 'idle' | 'running' | 'paused' + this.workflowState = 'idle';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` at line 256, The inline comment " 'idle' | 'running' | 'paused' " after the assignment to this.workflowState is redundant because the WorkflowState type already documents the allowed values; remove that inline comment from the assignment to this.workflowState in workflow-editor.ts (the line initializing this.workflowState = 'idle') so the code relies on the WorkflowState type for documentation and avoids duplication.
2127-2127:pollForRunparameters should be(runId: string, knownLogCount: number)instead ofany.Both types are known at all call sites —
runIdfromWorkflowRunResult.runIdandknownLogCountfromresult.logs.length.🛡️ Suggested fix
- pollForRun(runId: any, knownLogCount: any) { + pollForRun(runId: string, knownLogCount: number) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` at line 2127, Change the pollForRun parameter types from any to concrete types: update the method signature pollForRun(runId: any, knownLogCount: any) to pollForRun(runId: string, knownLogCount: number), and adjust any local variable annotations or JSDoc in the same function if present; use WorkflowRunResult.runId as the source for runId and result.logs.length as the source for knownLogCount at call sites to ensure callers match the new types.
839-843:getElementById('btn-run')duplicatesthis.runButton— use the existing field.
this.runButtonis already populated in the constructor (line 253). Querying the DOM again ininitButtonsis redundant and creates a local shadowrunBtnthat could diverge if the DOM is manipulated between construction and init.♻️ Suggested fix
- const runBtn = document.getElementById('btn-run'); - if (runBtn) { - runBtn.addEventListener('click', () => this.runWorkflow()); - } + if (this.runButton) { + this.runButton.addEventListener('click', () => this.runWorkflow()); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 839 - 843, initButtons currently queries getElementById('btn-run') again and creates a local runBtn instead of using the existing this.runButton set in the constructor; remove the redundant DOM query in initButtons and attach the click listener to this.runButton (or guard that this.runButton exists) so initButtons uses the already-populated field; update initButtons to call this.runButton.addEventListener('click', () => this.runWorkflow()) only when this.runButton is non-null.
1159-1163:deleteNodeparameter should be typed asstring, notany.Node IDs are always
string(constructed as`node_${this.nextNodeId++}`). Usinganysilently acceptsundefinedor non-string IDs, which would corrupt the filter.🛡️ Suggested fix
- deleteNode(id: any) { - this.nodes = this.nodes.filter((n: any) => n.id !== id); - this.connections = this.connections.filter((c: any) => c.source !== id && c.target !== id); + deleteNode(id: string) { + this.nodes = this.nodes.filter((n) => n.id !== id); + this.connections = this.connections.filter((c) => c.source !== id && c.target !== id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1159 - 1163, The deleteNode method currently accepts id: any which allows non-string/undefined values to pass into the nodes and connections filters; change the parameter signature of deleteNode to id: string and update any callers that pass non-string values to supply a string (IDs are constructed as `node_${this.nextNodeId++}`) so the filters on this.nodes and this.connections remain correct; after changing the signature, ensure the existing calls still compile and keep the existing logic that calls this.render() and this.updateRunButton().
1172-1172:(n: any)annotation onEditorNode[]forEach is unnecessary.
this.nodesisEditorNode[]; the callback parameter is already inferred. This pattern repeats across many forEach/filter/find/reduce calls in the file (lines 558, 566, 576, 591, 607–616, 962, 989, 1106, 1160, 1172, and others). A single pass to remove the inline: anyannotations on typed-array callbacks would reduce noise and keep strict mode meaningful.♻️ Representative fix (apply consistently throughout the file)
- this.nodes.forEach((n: any) => this.renderNode(n)); + this.nodes.forEach((n) => this.renderNode(n));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` at line 1172, Remove unnecessary inline ": any" annotations from callbacks where the container is already strongly typed (e.g., change this.nodes.forEach((n: any) => this.renderNode(n)) to this.nodes.forEach(n => this.renderNode(n))). Search for occurrences across the file (examples: this.nodes.forEach, array.filter/find/reduce callbacks around the noted spots) and delete the ": any" on the parameter so TypeScript will use the existing EditorNode[] (or other array) element type; keep the callback logic and function names like renderNode intact.
1047-1049:saveRunIdparameter should bestring, notany.
runIdis always a string (sourced fromWorkflowRunResult.runId);anylets accidental non-string values pass through tolocalStorage.setItem.🛡️ Suggested fix
- saveRunId(runId: any) { + saveRunId(runId: string) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1047 - 1049, The saveRunId method currently accepts runId: any which can allow non-string values to be stored; change the signature of saveRunId to accept runId: string and update any callsites to pass a string (or call String(runId) before passing) so localStorage.setItem(WorkflowEditor.RUN_KEY, runId) always receives a string; keep the existing try/catch around localStorage access. Reference: saveRunId and WorkflowEditor.RUN_KEY.
1063-1091:loadWorkflowtrusts rawlocalStorage/server JSON without runtime shape validation.Both callers pass unvalidated
JSON.parseorres.json()output asWorkflowGraphInput. TypeScript types are erased at runtime, so malformed stored state can still produce unexpectedundefinedaccesses deeper in the editor.Consider adding a lightweight guard (or Zod schema) that checks
Array.isArray(graph.nodes)andArray.isArray(graph.connections)beforeloadWorkflowprocesses the data. The existing null-coalescing (?? []) already helps, but typed node fields (e.g.,node.type,node.data) have no runtime guarantees.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1063 - 1091, Add runtime shape validation before passing parsed JSON to loadWorkflow: in loadInitialWorkflow and loadDefaultWorkflow (after JSON.parse / await res.json()), verify that graph is an object and Array.isArray(graph.nodes) and Array.isArray(graph.connections), and that each node has required fields like typeof node.id === 'string' and typeof node.type === 'string' (or use a small Zod/schema validator). If validation fails, do not call loadWorkflow; instead fall back to loadDefaultWorkflow (from loadInitialWorkflow) or keep the default start node (from loadDefaultWorkflow). Reference WorkflowEditor.STORAGE_KEY, loadInitialWorkflow, loadDefaultWorkflow, loadWorkflow, graph.nodes and graph.connections when making the changes.
906-906:Promiseresolve callback typed asany— loses thebooleanconstraint.
confirmedis alwaysboolean; the generic should be explicit so TypeScript tracks the resolved value.🛡️ Suggested fix
- return await new Promise((resolve: any) => { + return await new Promise<boolean>((resolve) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` at line 906, The Promise created with "new Promise((resolve: any) =>" loses the boolean type; change it to an explicitly typed Promise<boolean> and type the resolve callback accordingly (e.g., "new Promise<boolean>((resolve: (value: boolean) => void) =>" or "new Promise<boolean>((resolve) =>") so TypeScript tracks that the resolved value is boolean; update the occurrence inside workflow-editor.ts where "new Promise((resolve: any) =>" is used.
294-294:catchparameter typed asany— useunknownto stay consistent with strict mode.🛡️ Suggested fix
- }).catch((err: any) => { + }).catch((err: unknown) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` at line 294, The catch handler currently types the error parameter as (err: any); change it to (err: unknown) and then narrow it before use (e.g., const error = err instanceof Error ? err : new Error(String(err))) so you don't rely on any typings; update the catch callback that currently reads (err: any) => { ... } to (err: unknown) => { const error = err instanceof Error ? err : new Error(String(err)); /* use error.message / error.stack */ ... } to keep strict-mode safety when logging or rethrowing.
1785-1788:getPathDnumeric parameters all typed asany— usenumber.All four parameters are always numbers;
anydefeats arithmetic type checking.🛡️ Suggested fix
- getPathD(startX: any, startY: any, endX: any, endY: any) { + getPathD(startX: number, startY: number, endX: number, endY: number): string {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1785 - 1788, Change the getPathD signature to use explicit numeric types (startX: number, startY: number, endX: number, endY: number) and a string return type, replacing the current any types so arithmetic is type-checked; update any internal local vars if needed (e.g., controlPointOffset inferred as number) and adjust callers or exported type declarations referencing getPathD to match the new signature.
🤖 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/web/src/app/workflow-editor.ts`:
- Line 959: The upgradeLegacyNodes method's parameter shouldRender is declared
as any; change its type to boolean (upgradeLegacyNodes(shouldRender: boolean =
false)) to tighten typing and satisfy strict mode, update any callers that pass
non-boolean values to pass a proper boolean or coerce appropriately (e.g.,
!!value), and run type checks to ensure no other places rely on non-boolean
behavior of shouldRender.
- Around line 703-721: Remove the dead method renderEffortSelect from the file
(it's not used; renderNodeForm now builds the effort dropdown using
setupDropdown), and delete its entire function body to avoid dead code. If you
choose to ever reintroduce similar logic, ensure you guard against an empty
options array before dereferencing options[0] (e.g., check options.length > 0)
and preserve the same behavior of setting node.data.reasoningEffort and wiring
the change handler.
---
Outside diff comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 872-877: initButtons currently adds click listeners guarded by
checks on this.approveBtn and this.rejectBtn but those properties are null at
initButtons time and are later assigned (and already wired) in
showApprovalMessage, so the listeners are dead code; remove the redundant
conditional blocks in initButtons that reference this.approveBtn and
this.rejectBtn (and their addEventListener calls) to avoid dead branches,
keeping the click wiring that exists in showApprovalMessage and ensuring
submitApprovalDecision remains called only from the handlers defined where the
buttons are actually created.
- Around line 1731-1736: The assignment to this.connectionStart sets
ConnectionStart.handle (type string) from connection.sourceHandle which is
WorkflowConnection.sourceHandle (type string | undefined); ensure you guard
against undefined by providing a fallback before assigning. Update the logic
around this.connectionStart (the block that assigns nodeId/handle/x/y) to coerce
connection.sourceHandle to a definite string (e.g., use a default like "" or a
sensible handle constant) or perform a conditional check and set handle
accordingly so ConnectionStart.handle always receives a string value.
---
Duplicate comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 1430-1452: The code previously crashed when no effort options
existed; ensure you compute modelEfforts and effortSource (using modelEfforts ||
this.modelEfforts[this.modelOptions[0]] || []) then build effortOptions, pick
selectedEffort via effortOptions.find(...)? .value ?? effortOptions[0]?.value ??
'' and only call this.setupDropdown (with effortDropdown, effortOptions,
selectedEffort, 'Select effort', ...) and assign data.reasoningEffort when
selectedEffort is truthy to avoid rendering with empty values; update references
to modelEfforts, effortSource, effortOptions, selectedEffort, effortDropdown,
data.reasoningEffort and this.setupDropdown accordingly.
---
Nitpick comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 2082-2087: The inner conditional check "result.status !==
'paused'" inside the final else branch is unreachable because 'paused',
'completed', and 'failed' are already handled earlier; remove the redundant
check and always run the cleanup logic (call clearApprovalMessage(),
hideAgentSpinner(), and setWorkflowState('idle')) in that branch, or if a
specific status should be excluded, replace the condition with the correct
status check on result.status to reflect the intended behavior.
- Line 256: The inline comment " 'idle' | 'running' | 'paused' " after the
assignment to this.workflowState is redundant because the WorkflowState type
already documents the allowed values; remove that inline comment from the
assignment to this.workflowState in workflow-editor.ts (the line initializing
this.workflowState = 'idle') so the code relies on the WorkflowState type for
documentation and avoids duplication.
- Line 2127: Change the pollForRun parameter types from any to concrete types:
update the method signature pollForRun(runId: any, knownLogCount: any) to
pollForRun(runId: string, knownLogCount: number), and adjust any local variable
annotations or JSDoc in the same function if present; use
WorkflowRunResult.runId as the source for runId and result.logs.length as the
source for knownLogCount at call sites to ensure callers match the new types.
- Around line 839-843: initButtons currently queries getElementById('btn-run')
again and creates a local runBtn instead of using the existing this.runButton
set in the constructor; remove the redundant DOM query in initButtons and attach
the click listener to this.runButton (or guard that this.runButton exists) so
initButtons uses the already-populated field; update initButtons to call
this.runButton.addEventListener('click', () => this.runWorkflow()) only when
this.runButton is non-null.
- Around line 1159-1163: The deleteNode method currently accepts id: any which
allows non-string/undefined values to pass into the nodes and connections
filters; change the parameter signature of deleteNode to id: string and update
any callers that pass non-string values to supply a string (IDs are constructed
as `node_${this.nextNodeId++}`) so the filters on this.nodes and
this.connections remain correct; after changing the signature, ensure the
existing calls still compile and keep the existing logic that calls
this.render() and this.updateRunButton().
- Line 1172: Remove unnecessary inline ": any" annotations from callbacks where
the container is already strongly typed (e.g., change this.nodes.forEach((n:
any) => this.renderNode(n)) to this.nodes.forEach(n => this.renderNode(n))).
Search for occurrences across the file (examples: this.nodes.forEach,
array.filter/find/reduce callbacks around the noted spots) and delete the ":
any" on the parameter so TypeScript will use the existing EditorNode[] (or other
array) element type; keep the callback logic and function names like renderNode
intact.
- Around line 1047-1049: The saveRunId method currently accepts runId: any which
can allow non-string values to be stored; change the signature of saveRunId to
accept runId: string and update any callsites to pass a string (or call
String(runId) before passing) so localStorage.setItem(WorkflowEditor.RUN_KEY,
runId) always receives a string; keep the existing try/catch around localStorage
access. Reference: saveRunId and WorkflowEditor.RUN_KEY.
- Around line 1063-1091: Add runtime shape validation before passing parsed JSON
to loadWorkflow: in loadInitialWorkflow and loadDefaultWorkflow (after
JSON.parse / await res.json()), verify that graph is an object and
Array.isArray(graph.nodes) and Array.isArray(graph.connections), and that each
node has required fields like typeof node.id === 'string' and typeof node.type
=== 'string' (or use a small Zod/schema validator). If validation fails, do not
call loadWorkflow; instead fall back to loadDefaultWorkflow (from
loadInitialWorkflow) or keep the default start node (from loadDefaultWorkflow).
Reference WorkflowEditor.STORAGE_KEY, loadInitialWorkflow, loadDefaultWorkflow,
loadWorkflow, graph.nodes and graph.connections when making the changes.
- Line 906: The Promise created with "new Promise((resolve: any) =>" loses the
boolean type; change it to an explicitly typed Promise<boolean> and type the
resolve callback accordingly (e.g., "new Promise<boolean>((resolve: (value:
boolean) => void) =>" or "new Promise<boolean>((resolve) =>") so TypeScript
tracks that the resolved value is boolean; update the occurrence inside
workflow-editor.ts where "new Promise((resolve: any) =>" is used.
- Line 294: The catch handler currently types the error parameter as (err: any);
change it to (err: unknown) and then narrow it before use (e.g., const error =
err instanceof Error ? err : new Error(String(err))) so you don't rely on any
typings; update the catch callback that currently reads (err: any) => { ... } to
(err: unknown) => { const error = err instanceof Error ? err : new
Error(String(err)); /* use error.message / error.stack */ ... } to keep
strict-mode safety when logging or rethrowing.
- Around line 1785-1788: Change the getPathD signature to use explicit numeric
types (startX: number, startY: number, endX: number, endY: number) and a string
return type, replacing the current any types so arithmetic is type-checked;
update any internal local vars if needed (e.g., controlPointOffset inferred as
number) and adjust callers or exported type declarations referencing getPathD to
match the new signature.
Collaborator
Author
|
Addressed the newest CodeRabbit review in commit 3141b92. Updates made:
Validation rerun:
|
Collaborator
Author
|
Correction: previous summary comment had shell-formatting noise. Clean summary for commit 3141b92:
Validation rerun:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/workflow-editor.ts (1)
1294-1304:⚠️ Potential issue | 🟠 MajorEscape
node.typebefore injecting into HTML.The fallback label path interpolates raw
node.typeintoinnerHTML(Line 1303). Since graph data can come from storage/network, this creates an avoidable XSS surface.🔒 Suggested fix
- return `<span class="icon icon-primary"></span>${node.type}`; + return `<span class="icon icon-primary"></span>${escapeHtml(String(node.type || ''))}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1294 - 1304, The fallback branch of getNodeLabel currently injects raw node.type into HTML, creating an XSS risk; update that return to escape/sanitize node.type before concatenation (e.g. use the existing escapeHtml helper) so the fallback becomes something like building the label with escapeHtml(node.type) instead of node.type; change the fallback return in getNodeLabel to use escapeHtml(node.type) (and keep the existing icon markup) to prevent unsafe HTML injection.
🧹 Nitpick comments (1)
apps/web/src/app/workflow-editor.ts (1)
567-627: Remove explicitanytype annotations in graph-critical validation methods to maintain strict mode compliance.These methods (
getRunDisableReason,onPortMouseDown,onPortMouseUp) already work with properly typed class properties (nodes: EditorNode[],connections: WorkflowConnection[]), but explicitanyannotations on callback parameters weaken compile-time guarantees in validation logic. TypeScript can infer correct types automatically.Type cleanup suggestions
- const startNodes = this.nodes.filter((node: any) => node.type === 'start'); + const startNodes = this.nodes.filter((node) => node.type === 'start'); - const nodeIdSet = new Set(this.nodes.map((node: any) => node.id)); + const nodeIdSet = new Set(this.nodes.map((node) => node.id)); - this.connections.forEach((conn: any) => { + this.connections.forEach((conn) => { - onPortMouseDown(e: any, nodeId: any, handle: any) { + onPortMouseDown(e: MouseEvent, nodeId: string, handle: string) { - onPortMouseUp(e: any, nodeId: any, handle: any) { + onPortMouseUp(e: MouseEvent, nodeId: string, handle: string) {Also applies to: 1639-1660
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 567 - 627, Remove the explicit any annotations in the graph-validation callbacks: change parameter types in getRunDisableReason callbacks and the similar callbacks onPortMouseDown and onPortMouseUp to use the actual domain types (e.g., node => EditorNode, conn => WorkflowConnection) or rely on type inference instead of (node: any) and (conn: any); update uses of conn.sourceHandle/conn.source/conn.target and node.id/node.type to match those types so TypeScript type-checks correctly (references: getRunDisableReason, onPortMouseDown, onPortMouseUp, IF_FALLBACK_HANDLE, getIfConditionIndexFromHandle).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 1294-1304: The fallback branch of getNodeLabel currently injects
raw node.type into HTML, creating an XSS risk; update that return to
escape/sanitize node.type before concatenation (e.g. use the existing escapeHtml
helper) so the fallback becomes something like building the label with
escapeHtml(node.type) instead of node.type; change the fallback return in
getNodeLabel to use escapeHtml(node.type) (and keep the existing icon markup) to
prevent unsafe HTML injection.
---
Nitpick comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 567-627: Remove the explicit any annotations in the
graph-validation callbacks: change parameter types in getRunDisableReason
callbacks and the similar callbacks onPortMouseDown and onPortMouseUp to use the
actual domain types (e.g., node => EditorNode, conn => WorkflowConnection) or
rely on type inference instead of (node: any) and (conn: any); update uses of
conn.sourceHandle/conn.source/conn.target and node.id/node.type to match those
types so TypeScript type-checks correctly (references: getRunDisableReason,
onPortMouseDown, onPortMouseUp, IF_FALLBACK_HANDLE,
getIfConditionIndexFromHandle).
This was referenced Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
tsconfig.base.json has strict: true, but @ts-nocheck disabled TypeScript checking for the largest UI module in the web app. This change restores compile-time safety for that module without changing user-facing behavior.
Validation
Scope