Skip to content

Comments

Workflow builder: SSE streaming, UI overhaul, PREVIOUS_OUTPUT templates, and local config#5

Merged
matheusgalvao1 merged 24 commits intomainfrom
dev
Feb 24, 2026
Merged

Workflow builder: SSE streaming, UI overhaul, PREVIOUS_OUTPUT templates, and local config#5
matheusgalvao1 merged 24 commits intomainfrom
dev

Conversation

@matheusgalvao1
Copy link
Collaborator

@matheusgalvao1 matheusgalvao1 commented Feb 23, 2026

Summary

  • SSE streaming & progressive chat: Added /api/run-stream SSE endpoint; the run console now renders agent responses as they arrive with per-agent labels, spinners, and correct bubble ordering. Batch /api/run still available but not used by the UI.
  • {{PREVIOUS_OUTPUT}} template: Agent input fields support this token for composing prompts from the previous node's output. Engine state renamed from last_output to previous_output throughout.
  • UI overhaul: Adopted CodeSignal design system split panel, modal, and dropdown components. Removed custom header, added dynamic node widths, refactored zoom controls, cleaned up CSS to use DS tokens. Dark mode fixes for canvas dots and web search button.
  • Run UX: Preflight run-readiness rules, cancel-run UI, clean error handling for failed agent runs.
  • Local config (.config/): config.json (committed) defines providers/models/reasoning efforts loaded at startup. default-workflow.json (gitignored) preloads a workflow on init if present.
  • Removed dotenv: OPENAI_API_KEY must be exported in the shell.

Test plan

  • Run a single-agent workflow and confirm response streams progressively
  • Run a two-agent workflow with {{PREVIOUS_OUTPUT}} passthrough and confirm only the user's initial prompt shows as a user bubble
  • Set a custom agent input like Translate to portuguese: {{PREVIOUS_OUTPUT}} and confirm substitution works
  • Add/remove models in .config/config.json and confirm the model dropdown updates on reload
  • Drop a .config/default-workflow.json and confirm it loads on startup; delete it and confirm fallback to a single Start node
  • Trigger an agent error and confirm a clean error message appears in the run console
  • Verify dark mode looks correct (canvas dots, web search button, chat bubbles)

🤖 Generated with Claude Code

matheusgalvao1 and others added 21 commits February 23, 2026 11:47
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…S_OUTPUT}} template substitution

Agent input fields now support {{PREVIOUS_OUTPUT}} as a template token.
Renames internal state key from last_output to previous_output to match.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds POST /api/run-stream that streams each WorkflowLogEntry to the client
as an SSE event via the engine's onLog callback, then sends a done event
with the final result. Adds matching runWorkflowStream() client function
in api.ts that reads the SSE stream and fires an onLog callback per entry.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…input field refactor

- Switch runWorkflow to use runWorkflowStream for real-time chat updates
- Add onLogEntry handler: shows spinner on step_start, user bubble on
  start_prompt (formatted, before spinner), agent bubble on llm_response
- Only show first agent's start_prompt as user bubble; agent-to-agent
  prompts are internal and hidden
- Rename agent field from "User Prompt Override" to "Input", default to
  {{PREVIOUS_OUTPUT}}, update placeholder text for Input and System Prompt
- Per-node agent name labels on spinner and chat bubbles
- Fix spinner ordering: hide → user bubble → re-show spinner so user
  message always appears above the loading indicator

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds GET /api/default-workflow that reads default-workflow.json from the
project root and returns the graph if valid. On startup the editor fetches
this endpoint and loads the workflow via loadWorkflow(); if the file is
absent or the request fails, it falls back to the usual blank Start node.
default-workflow.json is gitignored so it stays local to each developer.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add GET /api/default-workflow route that reads default-workflow.json
  from the project root and returns the graph if valid (404 if absent)
- Add loadWorkflow() method to replace canvas nodes/connections and re-render
- Add loadDefaultWorkflow() called on init: loads the file if present,
  otherwise falls back to the default Start node
- Restore synchronous addDefaultStartNode() + upgradeLegacyNodes() in
  constructor so no empty-canvas flash occurs while the fetch is in flight
- After SplitPanel initialises (canvas has real dimensions), reposition
  the lone Start node to the correct canvas centre — matching Clear Canvas
- Update default fallback position to x:370 y:310
- default-workflow.json is gitignored so it stays local per developer

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move default-workflow.json lookup to .config/ directory
- Add GET /api/config route serving .config/config.json
- Add fetchConfig() + provider/model types to api.ts
- Convert MODEL_OPTIONS/MODEL_EFFORTS to instance properties with
  hardcoded fallbacks; loadConfig() overwrites them from enabled
  providers on startup
- Add .config/config.json with OpenAI provider and model definitions
- Update .gitignore to only ignore .config/default-workflow.json

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 23, 2026

📝 Walkthrough

Walkthrough

This PR refactors the workflow execution model to support server-side streaming via a new POST /run-stream endpoint with Server-Sent Events, introduces configuration management for OpenAI providers and models loaded from .config/config.json, and transitions the server from auto-loading environment variables to explicit configuration. The workflow engine replaces MockLLM with MissingLLM to enforce API key requirements rather than provide mock fallbacks. The web UI integrates dynamic component loading from the design-system submodule, adds zoom controls and split-panel layouts, and implements streaming log consumption with abort/cancellation support. New GET endpoints provide configuration and default workflow retrieval, while documentation is expanded with architecture guidance and run-readiness validation rules.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely describes the main changes: SSE streaming, UI overhaul, PREVIOUS_OUTPUT templates, and local config. It accurately reflects the primary objectives.
Description check ✅ Passed The PR description is comprehensive and directly related to the changeset, covering SSE streaming, template tokens, UI overhaul, run UX improvements, local config, and dotenv removal with a detailed test plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

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 (1)

980-985: ⚠️ Potential issue | 🟡 Minor

Potential crash when effortOptions is empty.

If neither node.data.model nor the first model in this.modelOptions exists in this.modelEfforts, the resulting effortOptions array is empty. Accessing effortOptions[0].value on line 984 will throw a TypeError.

Proposed fix
-            const selectedEffort = effortOptions.find(o => o.value === node.data.reasoningEffort)?.value || effortOptions[0].value;
-            node.data.reasoningEffort = selectedEffort;
+            const selectedEffort = effortOptions.find(o => o.value === node.data.reasoningEffort)?.value || effortOptions[0]?.value;
+            if (selectedEffort) node.data.reasoningEffort = selectedEffort;
🤖 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 980 - 985, The code can
crash when effortOptions is empty; update the logic around effortOptions /
selectedEffort to handle an empty array before accessing effortOptions[0].value:
after building effortOptions (from this.modelEfforts and this.modelOptions)
check if effortOptions.length === 0 and in that case set
node.data.reasoningEffort to a safe default (e.g., null, '' or a sentinel like
'none') or skip assignment, otherwise compute selectedEffort with
effortOptions.find(...) and assign it; reference the variables modelEfforts,
modelOptions, effortOptions, selectedEffort and the property
node.data.reasoningEffort when making the guard and default assignment.
packages/workflow-engine/src/index.js (1)

23-41: ⚠️ Potential issue | 🟠 Major

getGraph() method is missing from the JS build.

The TypeScript source (index.ts, lines 83–85) adds a new public getGraph() method, but this compiled JS file does not include it. The server's persistResult function has a Reflect.get fallback specifically for this case, but the intent (per README and PR objectives) is for getGraph() to be part of the public API.

Regenerate this file from the TypeScript source so the JS build includes getGraph().

#!/bin/bash
# Verify that getGraph is missing from the JS build but present in the TS source
echo "=== JS file: searching for getGraph ==="
rg -n 'getGraph' packages/workflow-engine/src/index.js

echo ""
echo "=== TS file: searching for getGraph ==="
rg -n 'getGraph' packages/workflow-engine/src/index.ts
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflow-engine/src/index.js` around lines 23 - 41, The compiled JS
is missing the new public getGraph() method present in the TypeScript source;
regenerate the compiled output from the TS (or re-run the build/tsc step) so
getGraph() is emitted alongside existing accessors like getResult(), getRunId(),
getStatus(), getLogs(), and ensure the runtime API matches the TS source so
callers (and persistResult's Reflect.get fallback) can call instance.getGraph()
directly rather than relying on the fallback.
🧹 Nitpick comments (7)
apps/web/src/app/workflow-editor.ts (3)

552-558: Dead code: initWebSocket opens a WebSocket connection that is never used.

With the move to SSE streaming (runWorkflowStream), this WebSocket connection is completely unused. It creates a connection on every page load for no benefit. Consider removing it.

🤖 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 552 - 558, The
initWebSocket function opens an unused WebSocket that creates connections on
page load; remove the unused function or its WebSocket instantiation to stop
opening connections. Locate initWebSocket in workflow-editor.ts, delete the new
WebSocket(...) creation and its onmessage handler (or remove the entire
initWebSocket function if not referenced elsewhere), and ensure no callers rely
on initWebSocket so there are no dangling references after removal.

1-1: @ts-nocheck disables all type safety for this file.

The file mixes untyped JS-style class properties with occasional TypeScript annotations (e.g., lines 648, 1400). Consider incrementally adding proper type declarations and removing @ts-nocheck to catch bugs at compile time — especially given the complexity of this class.

🤖 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 1, This file currently disables
all TypeScript checking via "@ts-nocheck"; enable incremental typing by removing
that directive and adding explicit types for the WorkflowEditor class members
and key methods — e.g., annotate the class (WorkflowEditor) properties (state,
props, editor, etc.), the constructor signature, and the methods referenced in
your review (the methods near the previous annotations around lines 648 and
1400) with appropriate interfaces or types; create small interfaces for complex
shapes (e.g., Workflow, Node, Edge, EditorState) and replace implicit any
occurrences, run tsc to surface type errors, and fix each reported error
progressively (start with public properties and method parameters/returns) until
the file compiles without errors.

663-672: Inconsistent API access: raw fetch vs. service layer.

loadConfig uses fetchConfig() from ../services/api, but loadDefaultWorkflow uses raw fetch('/api/default-workflow') directly. For consistency (and to centralize base URL, error handling, and headers), consider adding a fetchDefaultWorkflow function to the API service.

🤖 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 663 - 672, The
loadDefaultWorkflow method is calling fetch('/api/default-workflow') directly
which is inconsistent with loadConfig that uses fetchConfig() from
../services/api; add a new API wrapper fetchDefaultWorkflow in the same service
module (../services/api) that encapsulates base URL, headers and error handling,
export it, then replace the raw fetch call in loadDefaultWorkflow to call
fetchDefaultWorkflow and handle its response (e.g., check result.ok or thrown
errors) before calling this.loadWorkflow; update imports to pull
fetchDefaultWorkflow into workflow-editor.ts and keep the existing try/catch
behavior.
apps/web/index.html (1)

83-94: Cancel button icon may confuse users — it reuses the "End" node icon.

The cancel button (line 92) uses icon-rectangle-2698, which is the same icon used for the "End" node (line 43). A more conventional stop/cancel icon (e.g., icon-close or a dedicated stop icon if available in the DS) would be clearer. This is a minor UX nit.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/index.html` around lines 83 - 94, The cancel button currently reuses
the "End" node icon class (icon-rectangle-2698) which can confuse users; update
the element with id btn-cancel-run to use a conventional cancel/stop icon class
(for example icon-close or the DS's dedicated stop icon) by replacing the
icon-rectangle-2698 class on the <span> inside the button with the appropriate
stop/cancel icon class so the visual meaning matches the button action.
apps/server/src/routes/workflows.ts (1)

155-190: Synchronous file I/O in request handlers.

fs.existsSync and fs.readFileSync block the event loop. For low-traffic config endpoints on a dev tool this is acceptable, but consider using fs.promises for consistency with the rest of the Express app (the Vite middleware in index.ts already uses fs.promises).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/routes/workflows.ts` around lines 155 - 190, The handlers for
router.get('/default-workflow') and router.get('/config') perform blocking I/O
with fs.existsSync and fs.readFileSync; convert both route handlers to async
functions and replace the sync calls with fs.promises (use fs.promises.access or
stat to check existence and fs.promises.readFile to read), await the results,
preserve the same validation flow (validateGraph) and error handling semantics,
and ensure errors are caught in try/catch to log via logger.error and respond
with the same status codes and messages.
packages/workflow-engine/src/index.ts (2)

247-258: Duplicate llm_error log suppression — correct but fragile.

The check compares lastLog.content === message to decide whether to skip logging. This works because executeAgentNode logs the error and then re-throws it, so the same message appears in the last log entry. If future code adds any logging between the throw and the catch, this dedup logic will silently break.

Consider a more explicit approach, such as a tagged error type or a flag on the error object, to make the dedup intent clearer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflow-engine/src/index.ts` around lines 247 - 258, The
duplicate-suppression currently relies on comparing lastLog.content to message
which is fragile; instead mark errors explicitly when they have been logged
(e.g., set a property like (error as any).__llmLogged = true or throw a
LlmLoggedError) in the place that logs and rethrows (for example inside
executeAgentNode), and then update the catch in this file to check that explicit
flag or instanceof LlmLoggedError rather than comparing lastLog.content to
message (keep existing this.log(node.id, 'error', message) only when the flag is
not set).

353-365: key.includes('_approval') may over-match node IDs.

findLastNonApprovalOutput skips any state key containing '_approval'. If a node ever has an ID like "pre_approval_check", its output would be incorrectly skipped. Since node IDs are typically auto-generated this is unlikely, but an exact suffix match (key.endsWith('_approval')) would be safer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/workflow-engine/src/index.ts` around lines 353 - 365, The function
findLastNonApprovalOutput currently skips any state key containing '_approval'
which can over-match (e.g., "pre_approval_check"); update the condition to only
skip keys that exactly end with the suffix by replacing
key.includes('_approval') with key.endsWith('_approval') in
findLastNonApprovalOutput, leaving the existing checks for 'previous_output' and
'pre_approval_output' intact so only true approval-suffixed node IDs are
excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.config/config.json:
- Around line 1-36: The reasoningEfforts arrays for models gpt-5-mini and
gpt-5-nano are missing the "minimal" tier; update the reasoningEfforts for both
model objects (identified by id "gpt-5-mini" and "gpt-5-nano") to
["minimal","low","medium","high"] so they match the pre-5.1 model convention
used by "gpt-5" and the OpenAI API spec.

In @.gitignore:
- Around line 26-28: Add back standard env ignore patterns to .gitignore to
prevent accidental commits of secrets: add entries for .env, .env.local and
.env.*.local (alongside the existing "Local config" block) so that any
developer-created dotenv files are ignored even if the project no longer uses
dotenv; update the .gitignore near the Local config area to include those
patterns.

In `@apps/server/src/routes/workflows.ts`:
- Around line 104-114: The route calls addWorkflow(engine) but never removes it
after completion; ensure the workflow is cleaned up by calling
removeWorkflow(...) after the run finishes. Specifically, after awaiting
engine.run() and persistResult(engine, result) in the /run handler, invoke
removeWorkflow using the same identifier used by addWorkflow (e.g.,
removeWorkflow(engine) or removeWorkflow(engine.runId) depending on the existing
API); also wrap engine.run/persistResult in a try/finally so removeWorkflow is
always executed even on errors. Apply the same change to the /run-stream handler
so completed or errored workflows are removed from the active-workflows store.
- Around line 79-124: The handler must detect client disconnects and stop the
running engine to avoid wasted LLM calls: add a req.on('close', ...) listener
inside the POST '/run-stream' route that (1) marks the response closed so
sendEvent short-circuits (skip res.write after closed), (2) calls a cancellation
method on the engine (e.g., engine.abort() or engine.stop()) — add that method
to WorkflowEngine to propagate cancellation into engine.run and any LLM calls,
and (3) remove the engine from the registry via the existing
addWorkflow/removeWorkflow pattern; ensure the listener is cleaned up after run
completes or errors so no leaks occur.

In `@apps/web/src/app/workflow-editor.ts`:
- Around line 69-72: The race is that loadConfig() and loadDefaultWorkflow() are
started concurrently so loadDefaultWorkflow() can call loadWorkflow() → render()
before modelOptions/modelEfforts are populated, falling back to
DEFAULT_MODEL_OPTIONS/DEFAULT_MODEL_EFFORTS; fix it by awaiting loadConfig()
before calling loadDefaultWorkflow() (or chaining loadDefaultWorkflow() off the
promise returned by loadConfig()), ensuring loadConfig() fully resolves and sets
this.modelOptions and this.modelEfforts prior to loadWorkflow()/render()
executing; update the sequence around addDefaultStartNode(),
upgradeLegacyNodes(), loadConfig(), and loadDefaultWorkflow() so loadConfig() is
awaited and loadDefaultWorkflow() remains a properly awaited async call.

In `@apps/web/src/services/api.ts`:
- Around line 28-37: The current try/catch around JSON.parse swallows the
intentionally thrown Error from inside the try (losing the parsed message);
modify the logic around the text parsing block so that JSON.parse is wrapped in
try/catch but the creation/throwing of the Error using the parsed message
happens outside the catch — i.e., call JSON.parse(text) in a try to obtain
payload (in function handling the fetch response), on success compute message =
payload.error || payload.details || payload.message and then throw new
Error(message || text.trim()), and in the catch only handle JSON.parse failures
by throwing new Error(text.trim()); update the block that currently reads const
text = await res.text(); ... to follow this flow so the parsed message is
preserved.
- Around line 61-69: The error handler in runWorkflowStream currently throws
inside a try that also catches JSON parse failures, causing intentionally thrown
Errors (using payload.error/details) to be caught and replaced with the raw
text; fix by separating parsing from throwing: read res.text(), then attempt
JSON.parse(text) inside its own try-catch that only handles parse failures (set
payload when parse succeeds), and after that decide and throw a single Error
using payload.error || payload.details || text (so you don't re-catch the Error
you just created). Target the error-handling block that references res, text,
and payload in runWorkflowStream and ensure the throw happens outside the
JSON.parse try-catch so the thrown Error isn’t swallowed.

In `@apps/web/src/workflow-editor.css`:
- Around line 622-626: The CSS rule for the selector ".chat-message >
div:last-child" uses the deprecated "word-break: break-word"; remove that
declaration and either rely solely on "overflow-wrap: anywhere" or replace it
with "word-break: normal" to satisfy stylelint and preserve intended wrapping
behavior; update the rule in workflow-editor.css where ".chat-message >
div:last-child" is defined.
- Around line 165-215: Replace the hardcoded circular radius in the
.zoom-stepper rule with the design token: locate the .zoom-stepper selector and
change its border-radius: 50% to use the UI radius token (e.g.,
var(--UI-Radius-radius-m) or the appropriate circular token used across the
project) so theming stays consistent; ensure the element's
width/height/min-width remain equal to preserve the round shape after the token
swap.

In `@design-system`:
- Line 1: Verify that the design-system submodule is pinned to a stable, tagged
release: check the submodule entry referencing commit
450e146b6ce81363abb1a5fdab26868e51b7ab72 in the repository metadata and confirm
that this commit has an official tag or corresponds to an explicitly reviewed
release/milestone in the design-system repo; if it does not, update the
submodule pointer to a proper annotated tag (or add a release tag in the
design-system repo after review) and commit the updated submodule reference so
the project uses a reproducible, tagged release instead of an untagged commit.

---

Outside diff comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 980-985: The code can crash when effortOptions is empty; update
the logic around effortOptions / selectedEffort to handle an empty array before
accessing effortOptions[0].value: after building effortOptions (from
this.modelEfforts and this.modelOptions) check if effortOptions.length === 0 and
in that case set node.data.reasoningEffort to a safe default (e.g., null, '' or
a sentinel like 'none') or skip assignment, otherwise compute selectedEffort
with effortOptions.find(...) and assign it; reference the variables
modelEfforts, modelOptions, effortOptions, selectedEffort and the property
node.data.reasoningEffort when making the guard and default assignment.

In `@packages/workflow-engine/src/index.js`:
- Around line 23-41: The compiled JS is missing the new public getGraph() method
present in the TypeScript source; regenerate the compiled output from the TS (or
re-run the build/tsc step) so getGraph() is emitted alongside existing accessors
like getResult(), getRunId(), getStatus(), getLogs(), and ensure the runtime API
matches the TS source so callers (and persistResult's Reflect.get fallback) can
call instance.getGraph() directly rather than relying on the fallback.

---

Nitpick comments:
In `@apps/server/src/routes/workflows.ts`:
- Around line 155-190: The handlers for router.get('/default-workflow') and
router.get('/config') perform blocking I/O with fs.existsSync and
fs.readFileSync; convert both route handlers to async functions and replace the
sync calls with fs.promises (use fs.promises.access or stat to check existence
and fs.promises.readFile to read), await the results, preserve the same
validation flow (validateGraph) and error handling semantics, and ensure errors
are caught in try/catch to log via logger.error and respond with the same status
codes and messages.

In `@apps/web/index.html`:
- Around line 83-94: The cancel button currently reuses the "End" node icon
class (icon-rectangle-2698) which can confuse users; update the element with id
btn-cancel-run to use a conventional cancel/stop icon class (for example
icon-close or the DS's dedicated stop icon) by replacing the icon-rectangle-2698
class on the <span> inside the button with the appropriate stop/cancel icon
class so the visual meaning matches the button action.

In `@apps/web/src/app/workflow-editor.ts`:
- Around line 552-558: The initWebSocket function opens an unused WebSocket that
creates connections on page load; remove the unused function or its WebSocket
instantiation to stop opening connections. Locate initWebSocket in
workflow-editor.ts, delete the new WebSocket(...) creation and its onmessage
handler (or remove the entire initWebSocket function if not referenced
elsewhere), and ensure no callers rely on initWebSocket so there are no dangling
references after removal.
- Line 1: This file currently disables all TypeScript checking via
"@ts-nocheck"; enable incremental typing by removing that directive and adding
explicit types for the WorkflowEditor class members and key methods — e.g.,
annotate the class (WorkflowEditor) properties (state, props, editor, etc.), the
constructor signature, and the methods referenced in your review (the methods
near the previous annotations around lines 648 and 1400) with appropriate
interfaces or types; create small interfaces for complex shapes (e.g., Workflow,
Node, Edge, EditorState) and replace implicit any occurrences, run tsc to
surface type errors, and fix each reported error progressively (start with
public properties and method parameters/returns) until the file compiles without
errors.
- Around line 663-672: The loadDefaultWorkflow method is calling
fetch('/api/default-workflow') directly which is inconsistent with loadConfig
that uses fetchConfig() from ../services/api; add a new API wrapper
fetchDefaultWorkflow in the same service module (../services/api) that
encapsulates base URL, headers and error handling, export it, then replace the
raw fetch call in loadDefaultWorkflow to call fetchDefaultWorkflow and handle
its response (e.g., check result.ok or thrown errors) before calling
this.loadWorkflow; update imports to pull fetchDefaultWorkflow into
workflow-editor.ts and keep the existing try/catch behavior.

In `@packages/workflow-engine/src/index.ts`:
- Around line 247-258: The duplicate-suppression currently relies on comparing
lastLog.content to message which is fragile; instead mark errors explicitly when
they have been logged (e.g., set a property like (error as any).__llmLogged =
true or throw a LlmLoggedError) in the place that logs and rethrows (for example
inside executeAgentNode), and then update the catch in this file to check that
explicit flag or instanceof LlmLoggedError rather than comparing lastLog.content
to message (keep existing this.log(node.id, 'error', message) only when the flag
is not set).
- Around line 353-365: The function findLastNonApprovalOutput currently skips
any state key containing '_approval' which can over-match (e.g.,
"pre_approval_check"); update the condition to only skip keys that exactly end
with the suffix by replacing key.includes('_approval') with
key.endsWith('_approval') in findLastNonApprovalOutput, leaving the existing
checks for 'previous_output' and 'pre_approval_output' intact so only true
approval-suffixed node IDs are excluded.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8736547 and 08820b0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (21)
  • .config/config.json
  • .gitignore
  • AGENTS.md
  • CLAUDE.md
  • README.md
  • apps/server/package.json
  • apps/server/src/config.ts
  • apps/server/src/index.ts
  • apps/server/src/routes/workflows.ts
  • apps/server/src/services/persistence.ts
  • apps/web/docs/run-readiness.md
  • apps/web/index.html
  • apps/web/src/app/workflow-editor.ts
  • apps/web/src/components/help-modal.ts
  • apps/web/src/data/help-content.ts
  • apps/web/src/main.ts
  • apps/web/src/services/api.ts
  • apps/web/src/workflow-editor.css
  • design-system
  • packages/workflow-engine/src/index.js
  • packages/workflow-engine/src/index.ts
💤 Files with no reviewable changes (4)
  • apps/server/package.json
  • apps/server/src/config.ts
  • apps/web/src/data/help-content.ts
  • apps/web/src/components/help-modal.ts

Comment on lines +104 to +114
try {
const runId = Date.now().toString();
const engine = new WorkflowEngine(graph, {
runId,
llm,
onLog: (entry) => sendEvent({ type: 'log', entry })
});
addWorkflow(engine);

const result = await engine.run();
await persistResult(engine, result);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Completed workflow is never removed from active-workflows store.

addWorkflow(engine) on line 111 stores the engine, but after engine.run() completes (line 113), it is never removed. The /resume handler removes workflows on completion (line 144), but /run and /run-stream do not. Over many runs this accumulates stale entries in memory.

Proposed fix
       const result = await engine.run();
       await persistResult(engine, result);
+      if (result.status !== 'paused') {
+        removeWorkflow(result.runId);
+      }
 
       sendEvent({ type: 'done', result });

Apply the same fix to the /run route (after line 69).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/routes/workflows.ts` around lines 104 - 114, The route calls
addWorkflow(engine) but never removes it after completion; ensure the workflow
is cleaned up by calling removeWorkflow(...) after the run finishes.
Specifically, after awaiting engine.run() and persistResult(engine, result) in
the /run handler, invoke removeWorkflow using the same identifier used by
addWorkflow (e.g., removeWorkflow(engine) or removeWorkflow(engine.runId)
depending on the existing API); also wrap engine.run/persistResult in a
try/finally so removeWorkflow is always executed even on errors. Apply the same
change to the /run-stream handler so completed or errored workflows are removed
from the active-workflows store.

Comment on lines 622 to 626
.chat-message > div:last-child {
white-space: pre-wrap;
overflow-wrap: anywhere;
word-break: break-word;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Replace deprecated word-break: break-word.

Stylelint flags break-word as deprecated; overflow-wrap: anywhere already covers the intent. Consider removing it or using word-break: normal.

🔧 Suggested change
 .chat-message > div:last-child {
     white-space: pre-wrap;
     overflow-wrap: anywhere;
-    word-break: break-word;
 }
🧰 Tools
🪛 Stylelint (17.3.0)

[error] 625-625: Unexpected deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)

(declaration-property-value-keyword-no-deprecated)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/workflow-editor.css` around lines 622 - 626, The CSS rule for
the selector ".chat-message > div:last-child" uses the deprecated "word-break:
break-word"; remove that declaration and either rely solely on "overflow-wrap:
anywhere" or replace it with "word-break: normal" to satisfy stylelint and
preserve intended wrapping behavior; update the rule in workflow-editor.css
where ".chat-message > div:last-child" is defined.

@@ -1 +1 @@
Subproject commit 30ab87bae891f0b621bcb4081e8d53b19557653c
Subproject commit 450e146b6ce81363abb1a5fdab26868e51b7ab72
Copy link

@coderabbitai coderabbitai bot Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Show the log and any associated tags for the new submodule commit
# to confirm it is a stable, reviewed reference.

git -C design-system log --oneline -5
git -C design-system tag --points-at 450e146b6ce81363abb1a5fdab26868e51b7ab72

Repository: CodeSignal/learn_bespoke-agentic-workflow-builder

Length of output: 168


Verify the submodule is pinned to a stable, tagged release.

The submodule commit 450e146b6ce81363abb1a5fdab26868e51b7ab72 has no associated tag. Confirm this commit corresponds to an official release or explicitly reviewed milestone in the design-system repo. Untagged commits risk introducing unreviewed changes and complicate future debugging and auditing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@design-system` at line 1, Verify that the design-system submodule is pinned
to a stable, tagged release: check the submodule entry referencing commit
450e146b6ce81363abb1a5fdab26868e51b7ab72 in the repository metadata and confirm
that this commit has an official tag or corresponds to an explicitly reviewed
release/milestone in the design-system repo; if it does not, update the
submodule pointer to a proper annotated tag (or add a release tag in the
design-system repo after review) and commit the updated submodule reference so
the project uses a reproducible, tagged release instead of an untagged commit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repo doesn't have tags nor releases yet

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matheusgalvao1, understood! Since the design-system repo doesn't have tags or releases yet, pinning to a specific commit is the appropriate approach for now. When the design-system repo establishes a tagging or release process in the future, you can consider adopting those versioned references for easier tracking and rollback.

matheusgalvao1 and others added 3 commits February 24, 2026 10:38
- api.ts: fix catch-swallowing-throw in request() and runWorkflowStream()
  by separating JSON.parse into its own try/catch so parsed error messages
  are not discarded
- workflows.ts: call removeWorkflow() after /run and /run-stream complete
  to prevent memory leak in the active-workflows store; add clientDisconnected
  flag on req 'close' to guard sendEvent() in /run-stream
- workflow-editor.ts: chain loadConfig().then(() => loadDefaultWorkflow())
  so node forms always render with server-config models, not hardcoded defaults
- config.json: add 'minimal' reasoning effort to gpt-5-mini and gpt-5-nano
- .gitignore: restore .env / .env.local / .env.*.local safety-net patterns
- workflow-editor.css: remove deprecated word-break:break-word (redundant
  with overflow-wrap:anywhere); use DS token --UI-Radius-radius-max on
  .zoom-stepper instead of hardcoded 50%

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tream

req 'close' fires prematurely after Express body-parser consumes the POST
body, setting clientDisconnected=true before the engine starts. This caused
sendEvent to be a no-op for the entire run, so the 'done' event was never
delivered and the client threw "Workflow stream ended without a result".

res 'close' fires only when the actual response connection is torn down by
the client, which is the correct signal for SSE disconnect detection.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
--UI-Radius-radius-max does not exist in this design system (scale only
goes up to radius-xl: 32px), so the variable resolved to nothing and the
buttons lost their circular shape. 50% is the correct value for a
fixed-size circular element and has no DS equivalent here.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@matheusgalvao1 matheusgalvao1 merged commit b3d746d into main Feb 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant