Workflow mvp integration #821
Conversation
avirupsinha12
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR adds a complete workflow subsystem: new frontend workflow UIs (right-side drawers, template selector, executed-workflow renderer, executions table), typed frontend API clients, server workflow REST routes, Drizzle DB schema + DAOs for templates/executions/tools, file upload/validation handlers, and related types/utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as WorkflowExecutionModal (FE)
participant FEAPI as workflowExecutionsAPI (FE)
participant S as Server /api/v1/workflow
participant DB as DB (Drizzle)
UI->>FEAPI: executeTemplate(templateId, {name, description, file?, formData})
FEAPI->>S: POST /workflow/templates/:id/execute-with-input (FormData)
S->>DB: insert execution + step_executions/tool_executions
S-->>FEAPI: { executionId }
FEAPI-->>UI: executionId
loop Poll every 2s (max attempts/retries)
UI->>S: GET /workflow/executions/:id/status
S->>DB: read execution status
S-->>UI: { status, processingMessage? }
alt completed
UI-->>UI: Stop polling, enable "View Workflow"
else failed
UI-->>UI: Stop polling, show error
end
end
sequenceDiagram
participant Drawer as Config Drawer (AIAgent/Email/Form)
participant FEAPI as workflowToolsAPI / workflowStepsAPI
participant S as Server
participant DB as DB
Drawer->>Drawer: initialize from toolData or stepData
opt Gemini models required
Drawer->>S: GET /workflow/models/gemini (enum)
S-->>Drawer: enum model list
Drawer->>S: SSE /agent/generate-prompt
S-->>Drawer: stream prompt updates (ResponseUpdate / End / Error)
end
Drawer->>FEAPI: updateTool/createStep(...)
FEAPI->>S: PATCH/POST tool or step
S->>DB: persist tool/step
alt API success
S-->>Drawer: OK
else API error
S-->>Drawer: error (Drawer still calls onSave)
end
sequenceDiagram
participant UI as WhatHappensNextUI (FE)
participant FEAPI as workflowStepsAPI
participant S as Server
participant DB as DB
UI->>UI: User edits Python code -> Save
UI->>FEAPI: createStep(templateId, { type: "automated", tool: { type: "python_script", value: {...} } })
FEAPI->>S: POST /workflow/templates/:id/steps
S->>DB: Insert step + tool
S-->>UI: { stepId, toolId }
UI->>UI: onStepCreated callback, close drawer
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests
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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @avirupsinha12, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request delivers the foundational 'Workflow MVP', establishing a comprehensive system for defining, executing, and monitoring automated multi-step processes. It encompasses a new visual builder for frontend users, a robust set of backend APIs for managing workflow entities, and a redesigned database schema for scalability. The integration of custom Python scripting and enhanced email capabilities further extends the platform's automation potential.
Highlights
- Workflow Builder and Execution Visualization: Introduced a new visual workflow builder allowing users to create and configure multi-step processes. This includes dedicated UIs for AI Agent, Email, and Form Submission steps. A key feature is the ability to visualize executed workflows, providing detailed step-by-step inputs and outputs for debugging and monitoring.
- Comprehensive Workflow API: Implemented a robust set of new backend API endpoints to manage all aspects of workflows, including creation, retrieval, and updates for workflow templates, executions, steps, and tools. This also covers secure file handling for uploads and serving within workflows.
- Database Schema Redesign: Transitioned to a new UUID-based database schema for all workflow-related entities, replacing older integer-based IDs. This provides a more flexible, scalable, and robust foundation for future workflow enhancements.
- Python Scripting and HTML Email Support: Enabled the execution of custom Python scripts as integral workflow steps, demonstrated by a new LLM analysis email generation script. The backend email service was also enhanced to support sending rich HTML content, allowing for more dynamic notifications from workflows.
- Frontend Infrastructure Upgrade: Migrated frontend API calls to a new Hono client, streamlining communication with the backend. The workflow dashboard was also revamped to include dedicated tabs for 'Your Workflows' and 'Executions', improving overall user experience and navigation.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a major new feature: a workflow builder and execution engine. The changes are extensive, touching the frontend, backend, and database schemas. The frontend sees a significant refactoring of UI components to support workflow configuration, a new workflow builder canvas, and views for workflow executions. On the backend, new database schemas are introduced for workflows, steps, and tools, along with a comprehensive set of new API endpoints to manage them. While this is a substantial and well-structured feature addition, I've identified a few critical issues, particularly a bug in a database query and the removal of essential health check endpoints, that should be addressed. There are also some areas in the UI that seem incomplete.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 41
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (16)
server/api/chat/jaf-xynemodel-provider.ts (1)
76-92: Don’t drop object-shaped message content; normalize with a safe fallback.If lastMsg.content is an object with a text field (or another JAF-supported shape), the current logic collapses it to "". Prefer a robust normalization that also handles object content (and falls back to getTextContent).
Apply:
- const userQuery = lastMsg?.content || state.context?.userMessage || "" + const rawUserQuery = lastMsg?.content ?? state.context?.userMessage ?? "" ... - const selection = await generateToolSelectionOutput( - typeof userQuery === 'string' ? userQuery : - Array.isArray(userQuery) ? userQuery.map(part => - typeof part === 'string' ? part : part.text || '' - ).join('') : "", + const selection = await generateToolSelectionOutput( + (() => { + const u = rawUserQuery as any + if (typeof u === "string") return u + if (Array.isArray(u)) return u.map(p => (typeof p === "string" ? p : p?.text ?? "")).join("") + if (u && typeof u === "object" && typeof u.text === "string") return u.text + // Fallback to JAF helper to cover any additional shapes + try { return getTextContent(u as any) || "" } catch { return "" } + })(),server/api/chat/agents.ts (1)
1999-2027: Close tracing span before early breaks in assistant_message handler.messageSpan is never ended when content is empty or when tool_calls are present. That leaks spans and skews tracing.
Apply:
case "assistant_message": { const messageSpan = jafStreamingSpan.startSpan("assistant_message") const rawContent = evt.data.message.content || "" const content = typeof rawContent === 'string' ? rawContent : Array.isArray(rawContent) ? rawContent.map(part => typeof part === 'string' ? part : part.text || '' ).join('') : "" const hasToolCalls = Array.isArray(evt.data.message?.tool_calls) && (evt.data.message.tool_calls?.length ?? 0) > 0 if (!content || content.length === 0) { + messageSpan.end() break } if (hasToolCalls) { // Treat assistant content that accompanies tool calls as planning/reasoning, // not as final answer text. Emit as a reasoning step and do not send 'u' updates. await stream.writeSSE({ event: ChatSSEvents.Reasoning, data: JSON.stringify({ text: content, step: { type: AgentReasoningStepType.LogMessage, status: "in_progress", stepSummary: "Model planned tool usage", }, }), }) + messageSpan.end() break }server/integrations/google/utils.ts (2)
100-111: Avoidanyin Google Drive getFile; keep response strongly typedUse retryWithBackoff’s generic to preserve GaxiosResponse typing and keep
.datasafe.- const file: any = await retryWithBackoff( + const file = await retryWithBackoff<GaxiosResponse<drive_v3.Schema$File>>( () => drive.files.get({ fileId, fields, }), `Getting file with fileId ${fileId}`, Apps.GoogleDrive, 0, client, )
138-149: Restore precise typing for Google Docs response (docResponse)Using
anymasks shape changes and breaks editor/type safety. Keep it as GaxiosResponse<docs_v1.Schema$Document>.- const docResponse: any = + const docResponse: GaxiosResponse<docs_v1.Schema$Document> = await retryWithBackoff( () => docs.documents.get({ documentId: file.id as string, }), `Getting document with documentId ${file.id}`, Apps.GoogleDrive, 0, client, )server/integrations/google/index.ts (8)
359-371: Keep Admin SDK listUsers response typed; dropanyUse the concrete GaxiosResponse type to retain
.data.usersguarantees.- const res: any = + const res: GaxiosResponse<admin_directory_v1.Schema$Users> = await retryWithBackoff( () => admin.users.list({ domain: domain, maxResults: 500, orderBy: "email", ...(nextPageToken! ? { pageToken: nextPageToken } : {}), }), `Fetching all users`, Apps.GoogleDrive, )
2155-2164: Bug: missingawaitmakes try/catch ineffective; also type regressed toany
return retryWithBackoff(...)returns a Promise and bypasses the catch block on rejection. Await it and restore strong typing.-export const getSpreadsheet = async ( +export const getSpreadsheet = async ( sheets: sheets_v4.Sheets, id: string, client: GoogleClient, email: string, -): Promise<any | null> => { +): Promise<GaxiosResponse<sheets_v4.Schema$Spreadsheet> | null> => { try { - return retryWithBackoff( - () => sheets.spreadsheets.get({ spreadsheetId: id }), + const res = await retryWithBackoff<GaxiosResponse<sheets_v4.Schema$Spreadsheet>>( + () => sheets.spreadsheets.get({ spreadsheetId: id }), `Fetching spreadsheet with ID ${id}`, Apps.GoogleDrive, 0, client, ) + return res
547-553: Potential runtime crash: unsafe non-null assertion onentryPoints
entryPoints!can be undefined; indexing[0]will throw. Use optional chaining.- const conferenceLink = event?.conferenceData?.entryPoints![0]?.uri + const conferenceLink = event?.conferenceData?.entryPoints?.[0]?.uri
2993-3008: Drive files listing: avoidany; keep FileList typing- const res: any = + const res: GaxiosResponse<drive_v3.Schema$FileList> = await retryWithBackoff( () => drive.files.list({ q: query, pageSize: 100, fields: "nextPageToken, files(id, webViewLink, size, parents, createdTime, modifiedTime, name, owners, fileExtension, mimeType, permissions(id, type, emailAddress))", pageToken: nextPageToken, }),
3044-3055: Google Docs fetch: avoidany; retain document typing- const docResponse: any = + const docResponse: GaxiosResponse<docs_v1.Schema$Document> = await retryWithBackoff( () => docs.documents.get({ documentId: doc.id as string, }),
3386-3402: Drive file count: avoidany; useSchema$FileList- const res: any = + const res: GaxiosResponse<drive_v3.Schema$FileList> = await retryWithBackoff( () => drive.files.list({ q: query, pageSize: 1000, fields: "nextPageToken, files(id)", pageToken: nextPageToken, }),
226-247: Typo drops Gmail failed attachment metric
failedAttottachmentCountis misspelled; the code elsewhere usesfailedAttachmentCount. This results in zero/undefined being reported.- failedAttachments: result.stats.failedAttottachmentCount, + failedAttachments: result.stats.failedAttachmentCount,
889-899: Ensure intervals are always cleared on error paths
setIntervalis created but not cleared if the function throws before the post-successsetTimeout. MovesetIntervaldeclaration outside try and clear in finally.-export const handleGoogleOAuthIngestion = async (data: SaaSOAuthJob) => { +export const handleGoogleOAuthIngestion = async (data: SaaSOAuthJob) => { // ... - try { - // ... - const tracker = new Tracker(Apps.GoogleDrive, AuthType.OAuth) - tracker.setOAuthUser(userEmail) - - const interval = setInterval(() => { + let interval: ReturnType<typeof setInterval> | undefined + try { + const tracker = new Tracker(Apps.GoogleDrive, AuthType.OAuth) + tracker.setOAuthUser(userEmail) + interval = setInterval(() => { sendWebsocketMessage( JSON.stringify({ progress: tracker.getProgress(), userStats: tracker.getOAuthProgress().userStats, startTime: tracker.getStartTime(), }), connector.externalId, ) }, 4000) @@ - } catch (error) { + } catch (error) { // ... - } + } finally { + if (interval) clearInterval(interval) + }Also applies to: 1026-1046
server/integrations/microsoft/sync.ts (1)
724-737: Bug: DeletedItems delta call builds an empty endpoint when no token.makeGraphApiCall(client, "") is invalid. Provide a default delta endpoint for first run.
Fix:
- let deletedItemsEndpoint: string = "" + let deletedItemsEndpoint: string = "" if (deletedItemsDeltaToken && deletedItemsDeltaToken.startsWith("http")) { // Use existing delta token URL const url = new URL(deletedItemsDeltaToken) - deletedItemsEndpoint = url.pathname + url.search + deletedItemsEndpoint = url.pathname + url.search + } else { + // Initial delta for DeletedItems + deletedItemsEndpoint = + "/me/mailFolders/deleteditems/messages/delta?$select=id,internetMessageId&$top=100" } const deletedItemsResponse = await makeGraphApiCall( client, deletedItemsEndpoint, )server/integrations/microsoft/index.ts (2)
904-913: Interval cleanup may leak on errors/long runs; clear in finally.setTimeout-based cleanup might not run before an exception or long ingestion; move clearInterval to a finally block.
Apply:
- const interval = setInterval(() => { + const interval = setInterval(() => { sendWebsocketMessage( JSON.stringify({ progress: tracker.getProgress(), userStats: tracker.getOAuthProgress().userStats, startTime: tracker.getStartTime(), }), connector.externalId, ) }, 4000) ... - setTimeout(() => { - clearInterval(interval) - }, 8000) + // ... +} finally { + clearInterval(interval) + // Optionally ensure WS is closed even on failure + try { closeWs(connector.externalId) } catch {} }Also applies to: 951-954
1169-1176: DOCX buffer type mismatch — wrap with Uint8Array.extractTextAndImagesWithChunksFromDocx in this repo expects a Uint8Array (see usage in sync.ts). Passing ArrayBuffer may break on Node runtimes.
Fix:
- const extractedContent = - await extractTextAndImagesWithChunksFromDocx(docxBuffer) + const extractedContent = + await extractTextAndImagesWithChunksFromDocx(new Uint8Array(docxBuffer))frontend/src/routes/_authenticated/workflow.tsx (1)
681-691: "Create from Blank" should reset template and force editable builderWithout resetting, prior selection can leak into builder; also ensure builder is editable.
- onClick={() => setViewMode("builder")} + onClick={() => { + setSelectedTemplate(null) + setIsExecutionMode(false) + setIsEditableMode(true) + setViewMode("builder") + }}
♻️ Duplicate comments (3)
frontend/src/routes/_authenticated/ChatMessage.test.tsx (3)
149-152: Anchor/escape the “not visible” check (same concern as above)Mirror the regex hardening suggested earlier to avoid accidental matches.
174-176: Anchor/escape the positive “Thinking...” check (same concern as above)Use the anchored, escaped variant with exact three dots.
201-203: Anchor/escape the final “not visible” check (same concern as above)Apply the hardened negative assertion for consistency.
🧹 Nitpick comments (59)
server/api/chat/agents.ts (1)
2249-2262: Avoid unnecessary type-assert; narrow by status.Slightly safer/clearer to assign with a conditional narrow instead of an as-cast.
- const err = (outcome?.status === "error" ? outcome.error : undefined) as JAFError | undefined + const err: JAFError | undefined = + outcome?.status === "error" ? outcome.error : undefinedfrontend/src/routes/_authenticated/ChatMessage.test.tsx (3)
85-96: Minor: tighten scoping and fix misleading inline comment
- Consider using within(container) if ChatMessage ever renders portals/overlays to keep queries strictly scoped.
- Nit: The comment on Line 91 says “Thinking prop has content” but thinking is set to "" here.
Example tweak (optional):
- thinking="" // Thinking prop has content + thinking="" // Thinking text is driven by isStreaming+dots; thinking prop is empty
99-101: Harden matching to avoid false positivesIf THINKING_PLACEHOLDER ever contains regex meta chars or if text like “Rethinking” appears, the current regex could mis-match. Escape and anchor.
- getByText(new RegExp(`${THINKING_PLACEHOLDER}\\.\\.\\.`, "i")), + getByText(new RegExp(`^${escapeRegExp(THINKING_PLACEHOLDER)}\\.\\.\\.$`, "i")),Add once near the top of the file:
function escapeRegExp(s: string) { return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") }
123-126: Stronger negative assertion; optionally assert actions are shown
- Anchor/escape to avoid partial matches.
- Since the test title mentions “action buttons are shown,” optionally assert visibility of mocked icons after stop (e.g., copy/pencil).
- queryByText(new RegExp(THINKING_PLACEHOLDER, "i")), + queryByText(new RegExp(`^${escapeRegExp(THINKING_PLACEHOLDER)}(\\.\\.\\.)?$`, "i")),Optional extra assertions after the stop rerender (icons are already mocked with testids):
// expect(getByTestId("copy-icon")).toBeInTheDocument() // expect(getByTestId("pencil-icon")).toBeInTheDocument()frontend/src/components/workflow/WorkflowUtils.ts (3)
52-60: Comparator: consider deterministic tie-breaks when positions are equal/undefined.Current logic is fine, but sorting can look “random” when many positions are equal/undefined. Add a stable tie-break to improve UX ordering.
-export function customCompare( - a: number | undefined, - b: number | undefined, -): number { - if (a === undefined && b === undefined) return 0 - if (a === undefined) return 1 - if (b === undefined) return -1 - return a - b -} +export function customCompare( + a: number | undefined, + b: number | undefined, + tieBreak: () => number = () => 0, +): number { + if (a === b) return tieBreak() // covers both-undefined and equal numbers + if (a === undefined) return 1 + if (b === undefined) return -1 + const diff = a - b + return diff !== 0 ? diff : tieBreak() +}
114-127: Recursion can blow the call stack on deep graphs; switch to iterative traversal.visited prevents cycles, but very deep hierarchies can still overflow. Iterative DFS/BFS avoids this risk with identical behavior.
-export function fillConnectedChildSteps( - childStepIds: string[], - connectedStepList: string[], - stepDict: Record<string, Step>, - visited: Set<string> = new Set(), -): void { - for (const childStepId of childStepIds) { - if (visited.has(childStepId)) continue - visited.add(childStepId) - connectedStepList.push(childStepId) - const childStep = stepDict[childStepId] - if (childStep?.child_step_ids?.length) { - fillConnectedChildSteps( - childStep.child_step_ids, - connectedStepList, - stepDict, - visited, - ) - } - } -} +export function fillConnectedChildSteps( + childStepIds: string[], + connectedStepList: string[], + stepDict: Record<string, Step>, + visited: Set<string> = new Set(), +): void { + const stack = [...childStepIds] + while (stack.length) { + const id = stack.pop() as string + if (visited.has(id)) continue + visited.add(id) + connectedStepList.push(id) + const child = stepDict[id] + if (child?.child_step_ids?.length) { + stack.push(...child.child_step_ids) + } + } +}
134-139: Unsafe type assertion to LegacyFlow; add a type guard to avoid runtime surprises.Casting may mask shape mismatches if a non-legacy Flow reaches here. Narrow with an in-operator guard and fail fast or handle both shapes.
- const legacyFlow = flow as LegacyFlow - const rootStep = stepDict[legacyFlow.root_step_id] || defaultStep + if (!('root_step_id' in flow) || !('last_step_id' in flow)) { + // TODO: handle non-legacy Flow shape explicitly if applicable + console.warn("flowBFS: received non-LegacyFlow; skipping traversal") + return [[], 0, 0, "00:00:00"] + } + const legacyFlow = flow + const rootStep = stepDict[legacyFlow.root_step_id] || defaultStepIf there is a new Flow shape, reply with its field names and I’ll adapt this to support both.
server/integrations/google/utils.ts (1)
27-32: Break the circular import between utils.ts and index.ts
utils.tsimports from@/integrations/google(index.ts), whileindex.tsimports from./utils. This ESM cycle can yield undefined bindings at runtime depending on load order.
- Extract cross-used helpers (deleteDocument, downloadPDF, downloadDir, safeLoadPDF, getSheetsListFromOneSpreadsheet) into a separate module, e.g.,
server/integrations/google/files-helpers.ts, and have both sides import from it.server/integrations/google/index.ts (1)
2554-2554: Replace stray console.log with structured loggingUse the shared Logger for consistency and log-level control.
- console.log(`PDF SIZE : `, pdfSizeInMB) + Logger.debug(`PDF SIZE: ${pdfSizeInMB}`)server/integrations/microsoft/sync.ts (3)
87-92: Graceful fallback is good; consider lazy eval to avoid stale env reads.Capturing credentials at module load means rotated/late-injected env vars won’t be picked up by long-lived workers.
Apply this change to evaluate creds at runtime:
-const { clientId: MICROSOFT_CLIENT_ID, clientSecret: MICROSOFT_CLIENT_SECRET } = - validateMicrosoftCredentials() +const getMicrosoftCreds = () => validateMicrosoftCredentials()And update usages:
- if (!MICROSOFT_CLIENT_ID || !MICROSOFT_CLIENT_SECRET) { + const { clientId: MICROSOFT_CLIENT_ID, clientSecret: MICROSOFT_CLIENT_SECRET } = getMicrosoftCreds() + if (!MICROSOFT_CLIENT_ID || !MICROSOFT_CLIENT_SECRET) { ... }
1153-1159: Early return leaves jobs opaque; mark them Skipped with a reason.Without status updates, schedulers/observers can’t tell why syncs don’t run.
Apply:
if (!MICROSOFT_CLIENT_ID || !MICROSOFT_CLIENT_SECRET) { Logger.warn( "Skipping Microsoft OAuth changes sync - credentials not configured", ) - return + // Optionally surface the skip in job/history for observability + const apps = [Apps.MicrosoftDrive, Apps.MicrosoftOutlook, Apps.MicrosoftCalendar] + for (const app of apps) { + const jobs = await getAppSyncJobs(db, app, AuthType.OAuth) + for (const job of jobs) { + await updateSyncJob(db, job.id, { + status: SyncJobStatus.Skipped, + lastRanOn: new Date(), + }) + await insertSyncHistory(db, { + workspaceId: job.workspaceId, + workspaceExternalId: job.workspaceExternalId, + dataAdded: 0, + dataDeleted: 0, + dataUpdated: 0, + authType: AuthType.OAuth, + summary: { description: "Skipped: Microsoft credentials not configured" }, + errorMessage: "", + app, + status: SyncJobStatus.Skipped, + config: job.config, + type: SyncCron.ChangeToken, + lastRanOn: new Date(), + }) + } + } + return }
1198-1226: Persist OneDrive delta token even when no item changes to avoid needless re-scans.Currently the job updates config only when items changed; delta token should be advanced regardless.
Minimal adjustment:
- // Update sync job if changes were processed - if (changesExist) { + // Update sync job if changes were processed OR delta token advanced + if (changesExist || (nextDeltaTokenUrl && nextDeltaTokenUrl !== config.driveToken)) { const newConfig: MicrosoftDriveChangeToken = { type: "microsoftDriveDeltaToken", - driveToken: nextDeltaTokenUrl!, + driveToken: nextDeltaTokenUrl || config.driveToken, contactsToken: config.contactsToken || "", lastSyncedAt: new Date(), } ... - Logger.info(`No Microsoft OneDrive changes to sync`) + Logger.info(`No Microsoft OneDrive changes to sync; advanced delta token`)server/integrations/microsoft/index.ts (1)
281-283: Handle all-day events without end.dateTime.Some events only have end.date. Provide a safe fallback.
- endTime: new Date(event.end?.dateTime).getTime(), + endTime: event.end?.dateTime + ? new Date(event.end.dateTime).getTime() + : event.end?.date + ? new Date(event.end.date).getTime() + : undefined,server/ai/modelConfig.ts (1)
757-762: Harden typing and avoid unsafe cast in getActualNameFromEnumAccepting a plain string and casting to Models risks undefined lookups. Add a guard and overloads so valid enums return string, others return null.
-export const getActualNameFromEnum = (enumValue: string): string | null => { - const modelConfig = MODEL_CONFIGURATIONS[enumValue as Models] - return modelConfig?.actualName || null -} +/** + * Returns the provider-specific actual model name for a given Models enum. + * - If a non-enum or unknown key is passed, returns null. + */ +export function getActualNameFromEnum(enumValue: Models): string +export function getActualNameFromEnum(enumValue: string): string | null +export function getActualNameFromEnum(enumValue: Models | string): string | null { + if (typeof enumValue !== "string") return null + if (!Object.prototype.hasOwnProperty.call(MODEL_CONFIGURATIONS, enumValue)) { + return null + } + return MODEL_CONFIGURATIONS[enumValue as Models].actualName +}frontend/src/components/Sidebar.tsx (1)
174-193: LGTM on the Workflow link; optional a11y nit.The whitespace-only change is fine. Optional: add an aria-label to improve screen reader navigation and avoid relying on pathname.includes for active state if nested routes might collide.
- <Link + <Link to="/workflow" - className={cn( + aria-label="Workflow Builder" + className={cn( "flex w-8 h-8 items-center justify-center hover:bg-[#D8DFE680] dark:hover:bg-gray-700 rounded-md mt-[10px]", - location.pathname.includes("/workflow") && + location.pathname.startsWith("/workflow") && "bg-[#D8DFE680] dark:bg-gray-700", )} >frontend/src/components/workflow/WorkflowProvider.tsx (1)
2-2: Avoid duplicate FlowContextProps definitions across files.Types.ts already defines FlowContextProps with broader support (Flow | TemplateFlow | LegacyFlow). Consider importing and using that here to prevent drift.
I can provide a small follow-up patch to re-use the shared type if you’d like.
frontend/src/components/workflow/Types.ts (1)
186-191: FlowProps looks good; consider aligning context typing across files.Optional: unify FlowProps/FlowContextProps usage with WorkflowProvider to avoid parallel, slightly different definitions.
server/services/emailService.ts (1)
57-69: Add UTF-8 charset and (optionally) dual-part bodies for better client compatibility.Some clients render better with explicit Charset; when sending HTML, including a plain-text alternative improves deliverability.
- const emailBody = - contentType === "html" - ? { Html: { Data: body } } - : { Text: { Data: body } } + const emailBody = + contentType === "html" + ? { + Html: { Data: body, Charset: "UTF-8" }, + } + : { + Text: { Data: body, Charset: "UTF-8" }, + } const command = new SendEmailCommand({ Source: this.fromEmail, Destination: { ToAddresses: [to] }, Message: { - Subject: { Data: subject }, + Subject: { Data: subject, Charset: "UTF-8" }, Body: emailBody, }, })If desired, we can emit both parts when contentType === "html" (Text plus Html). Happy to draft that as well.
frontend/src/components/workflow/ActionBar.tsx (2)
55-80: Disable/enable zoom buttons using computed bounds.Tie disabled state to min/max of the step array to avoid magic numbers and remain correct if steps change.
- <button - onClick={handleZoomOut} - disabled={zoomLevel <= 50} + <button + onClick={handleZoomOut} + disabled={zoomLevel <= [50, 75, 100, 125, 150][0]} className="w-6 h-6 flex items-center justify-center rounded-full hover:bg-slate-100 disabled:opacity-50 disabled:cursor-not-allowed transition-colors" > ... - <button - onClick={handleZoomIn} - disabled={zoomLevel >= 150} + <button + onClick={handleZoomIn} + disabled={zoomLevel >= [50, 75, 100, 125, 150][4]} className="w-6 h-6 flex items-center justify-center rounded-full hover:bg-slate-100 disabled:opacity-50 disabled:cursor-not-allowed transition-colors" >Optional: hoist the steps into a module constant (e.g., ZOOM_STEPS) to avoid recreating arrays.
34-53: A11y nit: add aria-labels to icon-only buttons.Helps screen readers and improves accessibility.
- <button + <button + aria-label="Execute workflow" onClick={disabled ? undefined : onExecute} disabled={disabled} ... - <button + <button + aria-label="Zoom out" onClick={handleZoomOut} ... - <button + <button + aria-label="Zoom in" onClick={handleZoomIn}Also applies to: 56-60, 76-81
server/llm-email-script.py (1)
11-12: Avoid hardcoded recipientsRoute recipients via configuration (env/DB) so deployments don’t accidentally email personal addresses.
Example:
- "to": "avirupsinha10@gmail.com", + "to": os.getenv("WORKFLOW_ALERT_EMAIL", "ops@example.com"), @@ - "to": "avirup.sinha@juspay.in", + "to": os.getenv("WORKFLOW_RESULT_EMAIL", "ops@example.com"),Also applies to: 120-121
frontend/src/components/workflow/WorkflowExecutionsTable.tsx (3)
95-121: Show an empty-state row when there’s no dataImproves UX and avoids a blank table.
- <TableBody> - {paginatedExecutions.map((execution) => ( + <TableBody> + {paginatedExecutions.length === 0 && ( + <TableRow> + <TableCell colSpan={5} className="text-center text-gray-500 py-8"> + No executions found + </TableCell> + </TableRow> + )} + {paginatedExecutions.map((execution) => (
97-101: Only show pointer cursor when clickableAvoid misleading cursor when onRowClick isn’t provided.
- <TableRow + <TableRow key={execution.id} onClick={() => onRowClick?.(execution)} - className="cursor-pointer hover:bg-gray-50 dark:hover:bg-gray-800 border-b border-gray-200 dark:border-gray-700" + className={`${onRowClick ? "cursor-pointer" : ""} hover:bg-gray-50 dark:hover:bg-gray-800 border-b border-gray-200 dark:border-gray-700`} >
131-143: Reuse a shared page-size constantKeep pagination behavior consistent with HistoryModal.pageSize (21). Consider exporting a shared constant.
Potential follow-up:
- export const DEFAULT_PAGE_SIZES = [10, 21, 25, 50]
- import and use here and in HistoryModal.
server/api/workflowFileHandler.ts (2)
212-220: Support wildcard MIME (e.g., image/*) in allowedTypesImproves DX and completeness.
- const mimeTypeValid = validation.allowedTypes.some((type) => { + const mimeTypeValid = validation.allowedTypes.some((type) => { if (type.includes("/")) { - // MIME type check - return file.type === type + // MIME type check, allow wildcards like image/* + if (type.endsWith("/*")) { + const prefix = type.split("/")[0] + "/" + return (file.type || "").startsWith(prefix) + } + return file.type === type } else { // Extension check return fileExtension === type.toLowerCase() } })
116-118: Number validation: prefer robust finite checkMinor clarity/readability improvement.
- if (typeof value !== "number" && !!isNaN(Number(value))) { + if (!Number.isFinite(Number(value))) { return { isValid: false, error: `Field '${fieldId}' must be a number` } }frontend/src/components/workflow/WorkflowIcons.tsx (2)
15-23: Mark decorative SVGs as aria-hiddenPrevents noise for screen readers. Apply across all icons.
<svg className={className} width={width} height={height} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" + aria-hidden="true" + focusable="false" >
320-324: Use currentColor for theming instead of hard-coded fillAllows dark-mode/theming to flow.
- fill="#395A0C" + fill="currentColor"frontend/src/components/workflow/executedWorkflowRenderer.tsx (3)
1156-1161: Harden iframe preview against data leakageAdd referrerPolicy to avoid leaking origin on remote assets; keep scripts blocked.
- <iframe + <iframe srcDoc={resultString} className="w-full h-[500px] border-0 rounded" title="Email HTML Preview" - sandbox="allow-same-origin" + sandbox="allow-same-origin" + referrerPolicy="no-referrer" + loading="lazy" />
1934-1935: Timeout type: prefer ReturnType for browserNodeJS.Timeout mismatches in browser builds.
- const [pollingInterval] = useState<NodeJS.Timeout | null>(null) + const [pollingInterval] = useState<ReturnType<typeof setInterval> | null>(null)
1950-2050: Excessive console.log in prod pathsLogs are helpful now but noisy in production. Gate under NODE_ENV or a debug util.
Example:
const debug = (...args: any[]) => { if (process.env.NODE_ENV !== "production") console.log(...args) }Then replace console.log with debug (or strip via build-time plugin).
frontend/src/components/workflow/api/ApiHandlers.ts (5)
96-112: Use numeric pagination totals (or document bigint-as-string) consistentlytotalCount is typed as string. If the API returns numbers, this will force downstream parsing; if it returns bigints-as-strings, please document it and convert at the edge. Recommend number here and coercion at parse-time.
Proposed change:
- totalCount: string + totalCount: numberIf the server does return strings, coerce on receipt:
-const responseData = await response.json() +const responseData = await response.json() +if (responseData?.pagination?.totalCount) + responseData.pagination.totalCount = Number(responseData.pagination.totalCount)
231-236: Likely response shape mismatch for templatesAPI.fetchAllThis endpoint previously returned an envelope ({ success, data, ... }). Returning ApiTemplate[] directly may break consumers. Align with userWorkflowsAPI or use keepEnvelope and expose a consistent surface.
-const response = await api.workflow.templates.$get() -return extractResponseData<ApiTemplate[]>(response) +const response = await api.workflow.templates.$get() +const res = await extractResponseData<{ data: ApiTemplate[] }>(response, { keepEnvelope: true }) +return (res as any).data ?? res
397-408: PATCH semantics and idempotency for updateToolIf the backend partially updates tools, prefer PATCH. Also consider retry with backoff for transient failures via a shared HTTP helper.
Would you like a small wrapper for retries (exponential backoff + abort)?
430-452: CreateStep payload contract: confirm server expects inline tool objectThe schema here sends an embedded tool object. If server expects toolId(s) or separate creation, this will 400. Please confirm route contract and adjust.
I can align this with server/db schema and generate types from OpenAPI/Hono routes to eliminate drift.
480-484: Intentional stub is fine; consider dev-time guardThrowing is fine. Optionally log once with guidance to use updateStep so the error message isn’t repeated across call sites.
-throw new Error("linkSteps endpoint not available in current API. Use updateStep to modify nextStepIds.") +throw new Error("[workflowStepsAPI.linkSteps] Endpoint not available. Use updateStep({ nextStepIds }) instead.")frontend/src/components/workflow/TemplateCard.tsx (1)
43-47: Use provided icon when available and improve alt textYou define template.icon but don’t render it. Fallback to botLogo when absent; alt should reference the template.
- <img src={botLogo} alt="Bot Logo" className="w-5 h-5" /> + <img + src={template.icon || botLogo} + alt={`${template.name} icon`} + className="w-5 h-5" + loading="lazy" + />frontend/src/components/workflow/TemplateSelectionModal.tsx (1)
57-63: Add basic dialog a11y (role, aria) and label the titleImproves screen-reader UX for the modal.
- <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50"> + <div + className="fixed inset-0 bg-black/50 flex items-center justify-center z-50" + role="dialog" + aria-modal="true" + aria-labelledby="template-modal-title" + > ... - <h2 className="text-2xl font-bold text-gray-900 mb-2"> - Select Templates + <h2 id="template-modal-title" className="text-2xl font-bold text-gray-900 mb-2"> + Select a Template </h2>Also applies to: 72-75
frontend/src/components/workflow/WhatHappensNextUI.tsx (2)
245-253: Disable Save when prerequisites are missingAvoid no-op saves if
selectedNodeIdis absent or Python code is empty.- const handleSaveConfiguration = async () => { + const canSave = + !!selectedNodeId && (!showPythonConfig || pythonConfig.pythonCode.trim().length > 0) + + const handleSaveConfiguration = async () => { - if (!selectedNodeId) { - console.error("Missing node ID") - return - } + if (!canSave) return ... - <Button - onClick={handleSaveConfiguration} - disabled={isSaving} + <Button + onClick={handleSaveConfiguration} + disabled={isSaving || !canSave} className="w-full bg-gray-200 hover:bg-gray-300 text-gray-800 rounded-full" >Also applies to: 73-81
53-55: Remove unused local state or use it
setSelectedActionis written but never read; it adds re-renders for no value.- const [, setSelectedAction] = useState<string | null>(null) + // Removed unused selectedAction state ... - } else { - setSelectedAction(action.id) - } + }Also applies to: 260-270
frontend/src/components/workflow/OnFormSubmissionUI.tsx (1)
111-122: Duplicate defaulting of fileTypesYou default
fileTypesboth ingetInitialFormConfigand in this effect. Keep one source of truth (prefer initialization) to reduce surprises.frontend/src/components/workflow/EmailConfigUI.tsx (1)
69-80: Validate and normalize emails before addingPrevents invalid entries and case-variant duplicates.
- const handleAddEmail = () => { - if ( - newEmailAddress && - !emailConfig.emailAddresses.includes(newEmailAddress) - ) { + const handleAddEmail = () => { + const email = newEmailAddress.trim().toLowerCase() + const isValid = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email) + if (isValid && email && !emailConfig.emailAddresses.map(e => e.toLowerCase()).includes(email)) { setEmailConfig((prev) => ({ ...prev, - emailAddresses: [...prev.emailAddresses, newEmailAddress], + emailAddresses: [...prev.emailAddresses, email], })) setNewEmailAddress("") } }frontend/src/routes/_authenticated/workflow.tsx (2)
300-303: Safer totalCount parsingGuard against number/undefined to avoid NaN.
- setExecutionsTotal(parseInt(response.pagination.totalCount)) + setExecutionsTotal( + typeof response.pagination?.totalCount === "number" + ? response.pagination.totalCount + : parseInt(String(response.pagination?.totalCount ?? 0), 10) + )
16-88: Avoid type drift: reuse shared WorkflowTemplate typeYou re-declare a large
WorkflowTemplatehere whilefrontend/src/components/workflow/Types.tsalready exports one. Importing the shared type reduces divergence bugs (e.g., optionalconfig).server/db/workflowTool.ts (2)
1-1: Remove unused import.The symbol and is unused.
-import { and, eq, desc } from "drizzle-orm" +import { eq, desc, inArray } from "drizzle-orm"
119-140: Consider consistent JSON casting/validation.You cast result/value/config to any in different places. Prefer using the zod select schemas (as done elsewhere) for consistent typing and early validation.
Also applies to: 181-196
frontend/src/components/workflow/WorkflowExecutionModal.tsx (2)
101-105: Unused uploading flags.isUploading/isUploaded states are written but not read in UI. Drop them unless you plan to surface them.
- const [, setIsUploading] = useState(false) - const [, setIsUploaded] = useState(false) + // Remove if unusedAlso applies to: 102-104
69-89: Keep SUPPORTED_FILE_TYPES and input accept in sync.Looks aligned. Consider deriving accept string from SUPPORTED_FILE_TYPES keys to avoid divergence.
Also applies to: 536-543
frontend/src/components/workflow/AIAgentConfigUI.tsx (2)
95-124: Model selection resilience.getValidModelId uses current models state; when models load later, the selected model may stay on the default even if invalid. Consider re-validating when models change.
Also applies to: 129-133
126-148: Hidden system prompt suffix: make it explicit or configurable.Appending HIDDEN_APPEND_TEXT on save without visible indication can surprise users. Gate it behind a toggle or badge in the UI.
server/db/workflow.ts (3)
1-1: Remove unused imports.and and users are unused.
-import { and, eq, desc } from "drizzle-orm" +import { eq, desc } from "drizzle-orm" @@ - users,Also applies to: 8-8
63-72: Normalize return-shape consistency.Single-record getters parse with select schemas; list getters cast. For consistency and early validation, parse list rows too.
- return templates as SelectWorkflowTemplate[] + return templates.map(t => selectWorkflowTemplateSchema.parse(t)) @@ - return executions as SelectWorkflowExecution[] + return executions.map(e => selectWorkflowExecutionSchema.parse(e))Also applies to: 177-186
240-251: Add indexes for list/detail hot paths.Given frequent filters on workflowExecution.workflowTemplateId and workflowStepExecution.workflowExecutionId, ensure DB indexes exist in schema to keep these queries fast.
server/server.ts (2)
314-360: Fix misleading API key error messages and logsMessages reference “request body” and “Invalid JSON body” though the key is read from headers/query. This will confuse clients and support.
Apply:
- // Extract API key from request body - apiKey = c.req.header("x-api-key") || (c.req.query("api_key") as string) + // Extract API key from header or query + apiKey = c.req.header("x-api-key") || (c.req.query("api_key") as string) if (!apiKey) { Logger.error( - "API key verification failed: Missing apiKey in request body", + "API key verification failed: Missing API key (x-api-key header or api_key query param).", ) throw new HTTPException(401, { - message: "Missing API key. Please provide apiKey in request body.", + message: + "Missing API key. Provide via x-api-key header or api_key query parameter.", }) } ... - Logger.warn("API key verification failed: Invalid JSON body") + Logger.warn("API key verification failed")
1483-1499: Don’t leak stack traces in production error responses
errorHandlerreturns full stack; both Bun servers setdevelopment: true. Avoid exposing internals in prod.Minimal gating:
-const errorHandler = (error: Error) => { +const errorHandler = (error: Error) => { return new Response(`<pre>${error}\n${error.stack}</pre>`, { headers: { "Content-Type": "text/html" }, }) } -const server = Bun.serve({ +const server = Bun.serve({ fetch: app.fetch, port: config.port, websocket, idleTimeout: 180, - development: true, - error: errorHandler, + development: config.env !== "production", + error: config.env !== "production" ? errorHandler : undefined, }) -const metricServer = Bun.serve({ +const metricServer = Bun.serve({ fetch: internalMetricRouter.fetch, port: config.metricsPort, idleTimeout: 180, - development: true, - error: errorHandler, + development: config.env !== "production", + error: config.env !== "production" ? errorHandler : undefined, })Also applies to: 1492-1507
server/db/schema/tools.ts (1)
1-124: Action: consolidate or remove server/db/schema/tools.tsGiven the multiple conflicts (enums, FKs, table names, PK types, public schemas), keeping this file will fragment the data model.
Preferred action: delete this file and export everything workflow-tool-related from server/db/schema/workflows.ts. If you want, I can draft the re-exports or open a migration/cleanup PR.
server/db/schema/workflows.ts (3)
108-111: Use SQL array defaults for uuid[] to avoid driver/DDL quirks
default([])on a custom uuid[] type can generate invalid DDL or serialize as{}inconsistently. Prefer explicit SQL defaults.- prevStepIds: uuidArray("prev_step_ids").default([]), // Array of UUIDs - nextStepIds: uuidArray("next_step_ids").default([]), // Array of UUIDs - toolIds: uuidArray("tool_ids").default([]), // Array of UUIDs (only one for now) + prevStepIds: uuidArray("prev_step_ids").default(sql`ARRAY[]::uuid[]`), + nextStepIds: uuidArray("next_step_ids").default(sql`ARRAY[]::uuid[]`), + toolIds: uuidArray("tool_ids").default(sql`ARRAY[]::uuid[]`),- prevStepIds: uuidArray("prev_step_ids").default([]), // Array of UUIDs - nextStepIds: uuidArray("next_step_ids").default([]), // Array of UUIDs - toolExecIds: uuidArray("tool_exec_ids").default([]), // Renamed from toolIds, array of UUIDs + prevStepIds: uuidArray("prev_step_ids").default(sql`ARRAY[]::uuid[]`), + nextStepIds: uuidArray("next_step_ids").default(sql`ARRAY[]::uuid[]`), + toolExecIds: uuidArray("tool_exec_ids").default(sql`ARRAY[]::uuid[]`),Also applies to: 174-176
289-294: Schema: consider adding min/max constraints to template nameMinor guardrail to keep data clean.
-export const createWorkflowTemplateSchema = z.object({ - name: z.string().min(1).max(255), +export const createWorkflowTemplateSchema = z.object({ + name: z.string().min(1).max(255).trim(),
355-382: Tighten types in complex template schema
step.typeand tooltypeare free-form strings; use enums to fail fast on invalid builder payloads.- type: z.string(), + type: z.enum(["manual", "automated"]), ... - tools: z.array(z.object({ + tools: z.array(z.object({ id: z.string().optional(), - type: z.string(), + type: z.enum([ + "delay","python_script","slack","gmail","agent", + "merged_node","form","email","ai_agent", + ]),Also applies to: 383-399
avirupsinha12
force-pushed
the
workflow-mvp-integration
branch
from
9b4ab41 to
cd237ac
Compare
avirupsinha12
force-pushed
the
workflow-mvp-integration
branch
from
cd237ac to
3717cd1
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (30)
frontend/src/components/workflow/WorkflowExecutionsTable.tsx (1)
55-56: Fix pagination when totalCount=0 (avoid page 0 and incorrect next state)Clamp totalPages and disable next when no data.
- const totalPages = Math.ceil(totalCount / pageSize) + const totalPages = Math.max(1, Math.ceil((totalCount || 0) / (pageSize || 1))) @@ - {currentPage < totalPages && ( + {totalCount > 0 && currentPage < totalPages && ( @@ - disabled={currentPage === totalPages} + disabled={totalCount === 0 || currentPage >= totalPages}Also applies to: 171-181, 184-193
server/api/workflowFileHandler.ts (4)
91-99: ReDoS via untrusted RegExp pattern — validate and guard compilationCompiling user-supplied regex without bounds can hang the event loop.
- if (validation.pattern) { - const regex = new RegExp(validation.pattern) - if (!regex.test(value)) { + if (validation.pattern) { + if (validation.pattern.length > 256) { + return { isValid: false, error: `Field '${fieldId}' pattern too long` } + } + let regex: RegExp + try { + regex = new RegExp(validation.pattern) + } catch { + return { isValid: false, error: `Field '${fieldId}' pattern is invalid` } + } + if (!regex.test(value)) { return { isValid: false, error: `Field '${fieldId}' format is invalid`, } } }
258-263: Sanitize filename and avoid deprecated substr to prevent path traversalNever trust file.name; strip paths/unsafe chars and use slice().
- const timestamp = Date.now() - const randomSuffix = Math.random().toString(36).substr(2, 9) - const fileExtension = file.name.split(".").pop()?.toLowerCase() || "" - const fileName = `${timestamp}_${randomSuffix}_${file.name}` + const timestamp = Date.now() + const randomSuffix = Math.random().toString(36).slice(2, 11) + const originalBaseName = path.basename(file.name).replace(/[^\w.\-]/g, "_") + const fileExtension = originalBaseName.split(".").pop()?.toLowerCase() || "" + const fileName = `${timestamp}_${randomSuffix}_${originalBaseName}` const filePath = path.join(stepDir, fileName)
265-267: Bun-only write will crash in Node — add fs fallbackGuard Bun and fall back to fs/promises.
- const arrayBuffer = await file.arrayBuffer() - await Bun.write(filePath, new Uint8Array(arrayBuffer)) + const arrayBuffer = await file.arrayBuffer() + const bytes = new Uint8Array(arrayBuffer) + if (typeof (globalThis as any).Bun !== "undefined" && (globalThis as any).Bun.write) { + await (globalThis as any).Bun.write(filePath, bytes) + } else { + const { writeFile } = await import("node:fs/promises") + await writeFile(filePath, Buffer.from(bytes)) + }
251-255: /tmp is ephemeral — make upload root configurable and durableUse an env-configurable base dir; mount a persistent volume or use object storage in prod.
- const baseDir = "/tmp/workflow_uploads" + const baseDir = process.env.WORKFLOW_UPLOAD_DIR || path.join(os.tmpdir(), "workflow_uploads")Add import (top of file):
+import os from "node:os"frontend/src/components/workflow/WhatHappensNextUI.tsx (1)
47-49: Bug: reading wrong shape from toolData; use value.codePersisted shape is { value: { code } }. Current code reads value directly and loses data.
- const [pythonConfig, setPythonConfig] = useState({ - pythonCode: toolData?.value || "", - }) + const [pythonConfig, setPythonConfig] = useState({ + pythonCode: toolData?.value?.code || "", + }) @@ - setPythonConfig({ - pythonCode: toolData?.value || "", - }) + setPythonConfig({ + pythonCode: toolData?.value?.code || "", + })Also applies to: 60-63
frontend/src/components/workflow/OnFormSubmissionUI.tsx (3)
133-142: Fix backend type round‑trip: map "file" back to "upload" on save.You normalize
"upload" -> "file"when reading but persist"file"back. Convert to"upload"before calling the API.- type: field.type, + type: field.type === "file" ? "upload" : field.type,
444-504: File field UI lacks an actual uploader.You render “Uploaded Files” and a remove button, but there’s no input/dropzone to add files, so this state is unreachable from the UI.
{/* File Type Configuration */} + {/* File Upload Input */} + <div className="space-y-2"> + <Label className="text-sm font-medium text-slate-700 dark:text-gray-300"> + Upload test files (not saved; for preview only) + </Label> + <Input + type="file" + multiple + accept={field.fileTypes?.map(t => `.${t}`).join(",")} + onChange={(e) => { + const files = Array.from(e.target.files || []) + setUploadedFiles(prev => ({ + ...prev, + [field.id]: [...(prev[field.id] || []), ...files], + })) + }} + className="w-full dark:bg-gray-800 dark:text-gray-300 dark:border-gray-600" + /> + </div>If uploads aren’t intended in the config drawer, remove the uploadedFiles state and related UI instead.
326-333: No way to add new fields.There’s configuration and remove, but no “Add field” action, which blocks building dynamic forms.
<div className="space-y-3"> {formConfig.fields.map((field) => ( ... ))} </div> + <Button + variant="outline" + size="sm" + onClick={() => + setFormConfig(prev => ({ + ...prev, + fields: [ + ...prev.fields, + { id: crypto.randomUUID(), name: `Field ${prev.fields.length + 1}`, placeholder: "", type: "text" }, + ], + })) + } + className="w-full" + > + + Add Field + </Button>Also applies to: 562-572, 581-589
frontend/src/components/workflow/executedWorkflowRenderer.tsx (5)
139-147: Don’t assume tools[0] is the AI agent. Scan all tools.- const hasAIAgentTool = - tools && tools.length > 0 && tools[0].type === "ai_agent" + const hasAIAgentTool = Array.isArray(tools) && tools.some(t => t?.type === "ai_agent") - const aiConfig = - (step as any).config || (hasAIAgentTool && tools?.[0]?.config) || {} + const aiTool = Array.isArray(tools) ? tools.find(t => t?.type === "ai_agent") : undefined + const aiConfig = (step as any).config || aiTool?.config || {}
326-333: Same: email tool detection should scan all tools.- const hasEmailTool = tools && tools.length > 0 && tools[0].type === "email" + const hasEmailTool = Array.isArray(tools) && tools.some(t => t?.type === "email")
521-523: Same: form tool detection should scan all tools.- const hasFormTool = tools && tools.length > 0 && tools[0].type === "form" + const hasFormTool = Array.isArray(tools) && tools.some(t => t?.type === "form")
734-737: Same: python_script tool detection should scan all tools.- const hasPythonScriptTool = - tools && tools.length > 0 && tools[0].type === "python_script" + const hasPythonScriptTool = + Array.isArray(tools) && tools.some(t => t?.type === "python_script")
153-156: Normalize status strings once; stop comparing raw API values everywhere.Backends return “Success/Failed/Running” and “completed/failed/running”. Normalize first to avoid mis-highlighting.
+// near imports +const normalizeStatus = (s?: string) => + (s || "").toLowerCase().replace("success", "completed") -const isFailed = step.status === "failed" || hasFailedToolExecution +const isFailed = normalizeStatus(step.status) === "failed" || hasFailedToolExecutionRepeat for other comparisons in this file.
Also applies to: 346-350, 527-528, 751-757, 1019-1029, 1293-1305, 1740-1749, 1821-1826
frontend/src/routes/_authenticated/workflow.tsx (2)
363-394: Guard optional config to avoid runtime crashes.Direct access to possibly-undefined config will throw.
- if (workflowTemplate.config.allowed_file_types?.includes('pdf')) { + if (workflowTemplate.config?.allowed_file_types?.includes('pdf')) { return '📄' } - if (workflowTemplate.config.ai_model) { + if (workflowTemplate.config?.ai_model) { return '🤖' } - if (workflowTemplate.config.supports_file_upload) { + if (workflowTemplate.config?.supports_file_upload) { return '📁' }- if (workflowTemplate.config.allowed_file_types?.includes('pdf')) { + if (workflowTemplate.config?.allowed_file_types?.includes('pdf')) { return '#E8F5E8' } - if (workflowTemplate.config.ai_model) { + if (workflowTemplate.config?.ai_model) { return '#EBF4FF' } - if (workflowTemplate.config.supports_file_upload) { + if (workflowTemplate.config?.supports_file_upload) { return '#FEF3C7' }
119-140: Editable-mode flag is accepted but ignored; persist it and pass through.const [viewMode, setViewMode] = useState<"list" | "builder">("list") const [workflows, setWorkflows] = useState<WorkflowTemplate[]>([]) + const [isEditableMode, setIsEditableMode] = useState<boolean>(true)- const handleViewWorkflow = async (templateId: string, editable: boolean = false) => { + const handleViewWorkflow = async (templateId: string, editable: boolean = false) => { try { setIsLoadingTemplate(true) @@ - setSelectedTemplate(template) + setSelectedTemplate(template) setIsExecutionMode(false) + setIsEditableMode(editable) setViewMode("builder")- <WorkflowBuilder + <WorkflowBuilder user={user} onBackToWorkflows={() => { setViewMode("list") setSelectedTemplate(null) setIsExecutionMode(false) }} selectedTemplate={selectedTemplate} isLoadingTemplate={isLoadingTemplate} - isEditableMode={selectedTemplate === null} + isEditableMode={isEditableMode} />Also applies to: 400-421, 983-995
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
173-255: Close SSE connections on end/error/unmount; avoid leaks.Track EventSource in a ref; always close it on End/Error and in a cleanup effect.
- try { + try { // Create the URL with query parameters for EventSource @@ - let eventSource: EventSource | null = null + const eventSourceRef = React.useRef<EventSource | null>(null) let generatedPrompt = "" @@ - eventSource = await createAuthEventSource(url.toString()) + eventSourceRef.current = await createAuthEventSource(url.toString()) @@ - await new Promise((resolve, reject) => { - if (!eventSource) { + await new Promise((resolve, reject) => { + if (!eventSourceRef.current) { reject(new Error("EventSource not created")) return } - eventSource.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { generatedPrompt += event.data }) - eventSource.addEventListener(ChatSSEvents.End, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.End, (event) => { try { const data = JSON.parse(event.data) const finalPrompt = data.fullPrompt || generatedPrompt if (finalPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(finalPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() resolve(finalPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } catch (parseError) { if (generatedPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(generatedPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() resolve(generatedPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } }) - eventSource.addEventListener(ChatSSEvents.Error, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.Error, (event) => { try { const data = JSON.parse(event.data) - eventSource?.close() + eventSourceRef.current?.close() reject(new Error(data.error || "Error in prompt generation")) } catch (parseError) { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("Error in prompt generation")) } }) - eventSource.onerror = () => { - eventSource?.close() + eventSourceRef.current.onerror = () => { + eventSourceRef.current?.close() reject(new Error("Connection error during prompt generation")) } })Add once at component level:
+ React.useEffect(() => { + return () => { + try { (eventSourceRef as any)?.current?.close?.() } catch {} + } + }, [])frontend/src/components/workflow/WorkflowCard.tsx (3)
6-47: Use the shared WorkflowTemplate type; remove local duplicate.-import botLogo from "@/assets/bot-logo.svg" - -interface WorkflowTemplate { - id: string - name: string - description: string - version: string - status: string - config: { - ai_model?: string - max_file_size?: string - auto_execution?: boolean - schema_version?: string - allowed_file_types?: string[] - supports_file_upload?: boolean - } - createdBy: string - rootWorkflowStepTemplateId: string - createdAt: string - updatedAt: string - rootStep?: { ... } -} +import botLogo from "@/assets/bot-logo.svg" +import type { WorkflowTemplate } from "./Types"
57-71: Guard optional fields to avoid “Invalid Date”/undefined.-const formatDate = (dateString: string) => { - const date = new Date(dateString) +const formatDate = (dateString?: string) => { + if (!dateString) return "—" + const date = new Date(dateString) return ( ... ) } ... -Edited at {formatDate(workflow.updatedAt)} +Edited at {formatDate(workflow.updatedAt ?? workflow.createdAt)} ... -workflowDescription={workflow.description} +workflowDescription={workflow.description ?? ""}Also applies to: 94-96, 121-124
74-77: Prefer template-provided icon when available; fallback to bot-logo.- const getTemplateIcon = () => { - // Always use bot-logo.svg for workflow cards - return <img src={botLogo} alt="Bot Logo" className="w-5 h-5" /> - } + const getTemplateIcon = () => { + const emoji = workflow.rootStep?.metadata?.icon + return emoji + ? <span className="text-base leading-none">{emoji}</span> + : <img src={botLogo} alt="Bot Logo" className="w-5 h-5" /> + }frontend/src/components/workflow/WorkflowExecutionModal.tsx (6)
110-112: Fix interval typing for browser environment.NodeJS.Timeout is incorrect for browser environments.
- const [pollingInterval, setPollingInterval] = useState<NodeJS.Timeout | null>( + const [pollingInterval, setPollingInterval] = useState<ReturnType<typeof setInterval> | null>( null, )
347-356: Remove or properly document the fallback polling function.This appears to be development/testing code that simulates completion after 5 seconds.
Either remove this function entirely or add clear documentation explaining when and why it's used as a fallback.
9-58: Import shared WorkflowTemplate type to prevent drift.The WorkflowTemplate interface is already defined in Types.ts. Redefining it here risks type inconsistencies.
-interface WorkflowTemplate { - id: string - name: string - description: string - version: string - status: string - config: { - ai_model?: string - max_file_size?: string - auto_execution?: boolean - schema_version?: string - allowed_file_types?: string[] - supports_file_upload?: boolean - } - createdBy: string - rootWorkflowStepTemplateId: string - createdAt: string - updatedAt: string - rootStep?: { - id: string - workflowTemplateId: string - name: string - description: string - type: string - timeEstimate: number - metadata: { - icon?: string - step_order?: number - schema_version?: string - user_instructions?: string - } - tool?: { - id: string - type: string - value: { - fields?: Array<{ - name: string - type: string - required?: boolean - default?: any - }> - [key: string]: any - } - config: any - createdBy: string - createdAt: string - updatedAt: string - } - } -} +import type { WorkflowTemplate } from "./Types"
170-195: Remove console logs before production.Multiple console.log statements should be removed or replaced with proper logging.
Remove all console.log statements on lines 170, 172, 181, 195, 213, 258-261, 292, 305, 308-309, 314-315, 320-321, 326-327, 341, 348-349, 351-352, 355.
Also applies to: 258-261
211-223: Fix executionId extraction for unwrapped response.The API handler unwraps the response, so
response.datalikely doesn't exist.- // Extract execution ID from response.data.execution.id - const executionId = response.data?.execution?.id + // Extract execution ID robustly from common shapes + const executionId = + response?.execution?.id ?? + response?.executionId ?? + response?.id console.log("📋 Extracted execution ID:", executionId)
291-345: Add cleanup effect to prevent memory leaks.The polling interval needs to be cleared on component unmount.
Add this effect after the state declarations:
React.useEffect(() => { return () => { setPollingInterval((current) => { if (current) clearInterval(current) return null }) } }, [])frontend/src/components/workflow/api/ApiHandlers.ts (4)
114-135: Enhance extractResponseData for robustness.Add support for 204 No Content and better error handling.
-async function extractResponseData<T>(response: Response): Promise<T> { +async function extractResponseData<T>( + response: Response, + opts: { keepEnvelope?: boolean } = {} +): Promise<T> { if (!response.ok) { - const errorData = await response.json().catch(() => ({ message: "Network error" })) - throw new Error(errorData.message || `HTTP ${response.status}: ${response.statusText}`) + const text = await response.text().catch(() => "") + let msg = `HTTP ${response.status}: ${response.statusText}` + try { + const e = text ? JSON.parse(text) : {} + msg = e?.error?.message || e?.message || msg + } catch (_) {} + throw new Error(msg) } - - const responseData = await response.json() + + // Handle 204 No Content + if (response.status === 204) return undefined as unknown as T + + const rawText = await response.text() + const responseData = rawText ? JSON.parse(rawText) : {} + if (opts.keepEnvelope) return responseData as T + // Extract data from success wrapper if present if (responseData.success && responseData.data !== undefined) { return responseData.data as T } // If no success wrapper, return the response with success flag removed if (responseData.success !== undefined) { const { success, ...rest } = responseData return rest as T } return responseData as T }
264-277: Use extractResponseData with keepEnvelope option.Avoid manual JSON parsing to maintain consistency.
- const response = await api.workflow.executions.$get({ query }) - - if (!response.ok) { - const errorData = await response.json().catch(() => ({ message: "Network error" })) - throw new Error(errorData.message || `HTTP ${response.status}: ${response.statusText}`) - } - - const responseData = await response.json() - - // For workflow executions, we need to return the complete response structure - // (success, data, pagination, filters) as the frontend expects these properties - if (responseData.success !== undefined) { - return responseData as WorkflowExecutionsResponse - } - - return responseData as WorkflowExecutionsResponse + const response = await api.workflow.executions.$get({ query }) + return extractResponseData<WorkflowExecutionsResponse>(response, { keepEnvelope: true })
6-50: Import shared WorkflowTemplate type to prevent drift.Redefining WorkflowTemplate locally risks type inconsistencies with the canonical definition in Types.ts.
+import type { WorkflowTemplate } from "../Types" -interface WorkflowTemplate { - id: string - name: string - description: string - version: string - status: string - config: { - ai_model?: string - max_file_size?: string - auto_execution?: boolean - schema_version?: string - allowed_file_types?: string[] - supports_file_upload?: boolean - } - createdBy: string - rootWorkflowStepTemplateId: string - createdAt: string - updatedAt: string - rootStep?: { - id: string - workflowTemplateId: string - name: string - description: string - type: string - timeEstimate: number - metadata: { - icon?: string - step_order?: number - schema_version?: string - user_instructions?: string - } - tool?: { - id: string - type: string - value: any - config: any - createdBy: string - createdAt: string - updatedAt: string - } - } -} + +interface WorkflowTemplateResponse { + data: WorkflowTemplate[] +}
321-357: Fix file upload logic and remove console logs.Multiple issues: potential duplicate document_file entries, console logs in production, and improper value stringification.
const formData = new FormData() // Add required fields matching the curl command formData.append("name", executionData.name) formData.append("description", executionData.description) + let documentFileAppended = false // Add the uploaded file if provided if (executionData.file) { formData.append("document_file", executionData.file) - console.log("✅ File appended to FormData as 'document_file'") - } else { - console.warn("⚠️ No file provided in executionData") + documentFileAppended = true } // Add additional form data fields (excluding name and description to avoid duplicates) - Object.entries(executionData.formData).forEach(([key, value]) => { + Object.entries(executionData.formData ?? {}).forEach(([key, value]) => { if (key !== "name" && key !== "description") { if (value instanceof File) { - // If it's a file in formData, also append it as document_file - formData.append("document_file", value) - console.log(`✅ File from formData[${key}] appended as 'document_file'`) + // Only attach as document_file if none added yet + if (!documentFileAppended) { + formData.append("document_file", value) + documentFileAppended = true + } + } else if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") { + formData.append(key, String(value)) } else { - formData.append(key, String(value)) + // For objects/arrays, use JSON.stringify + formData.append(key, JSON.stringify(value)) } } }) - - // Additional fallback: If no file has been added yet, check if we need to add the main file as document_file - let hasDocumentFile = false - for (const [key] of formData.entries()) { - if (key === "document_file") { - hasDocumentFile = true - break - } - } - - if (!hasDocumentFile && executionData.file) { - formData.append("document_file", executionData.file) - console.log("✅ Added main file as document_file (fallback)") - } - - if (!hasDocumentFile && !executionData.file) { - console.error("❌ No file found to append as document_file!") - }
🧹 Nitpick comments (15)
server/api/chat/jaf-xynemodel-provider.ts (2)
76-91: Normalize userQuery with getTextContent and preserve spacingAvoid custom parsing drift and accidental token glueing; reuse getTextContent and keep word boundaries.
- const userQuery = lastMsg?.content || state.context?.userMessage || "" + const userQueryRaw = lastMsg?.content ?? state.context?.userMessage ?? "" + const userQuery = + typeof userQueryRaw === "string" ? userQueryRaw : getTextContent(userQueryRaw as any) @@ - const selection = await generateToolSelectionOutput( - typeof userQuery === 'string' ? userQuery : - Array.isArray(userQuery) ? userQuery.map(part => - typeof part === 'string' ? part : part.text || '' - ).join('') : "", + const selection = await generateToolSelectionOutput( + userQuery, state.context?.userCtx || "", toolListStr, "", params, state.context?.agentPrompt, )
118-120: Don’t swallow tool-selection errors silentlyLog at debug to aid triage when tool planning fails and we fall back.
- } catch (err) { - // Fall through to direct answer - } + } catch (err) { + // Fall through to direct answer + console.debug("Tool selection failed; falling back to direct answer", err) + }server/types/workflowTypes.ts (1)
1-6: Status strings diverge from UI ("Success" vs "completed")Either:
- Change UI to use enum values, or
- Provide a shared mapper: completed→Success, running→Running, failed→Failed.
This avoids brittle string comparisons.
Also applies to: 26-31
server/api/workflowFileHandler.ts (1)
333-341: 0-valued constraints skipped — use explicit undefined checksTruthiness check drops 0; copy values when present.
- if (field.validation.minLength) + if (field.validation.minLength !== undefined) validation.minLength = field.validation.minLength - if (field.validation.maxLength) + if (field.validation.maxLength !== undefined) validation.maxLength = field.validation.maxLength - if (field.validation.pattern) + if (field.validation.pattern !== undefined) validation.pattern = field.validation.patternserver/db/workflowTool.ts (2)
1-1: Remove unused import-import { and, eq, desc, inArray } from "drizzle-orm" +import { eq, desc, inArray } from "drizzle-orm"
60-73: Preserve input order for multi-ID fetch (optional)If callers expect results in toolIds order, re-order after query.
- const results = await trx + const results = await trx .select() .from(workflowTool) .where(inArray(workflowTool.id, toolIds)) .orderBy(desc(workflowTool.createdAt)) - - return results.map(result => ({ ...result, value: result.value as any, config: result.config as any })) + const byId = new Map(results.map(r => [r.id, r])) + return toolIds + .map(id => byId.get(id)) + .filter(Boolean) + .map(result => ({ ...result!, value: (result as any).value as any, config: (result as any).config as any }))frontend/src/components/workflow/WhatHappensNextUI.tsx (1)
147-156: Micro copy editsSlight grammar fixes for clarity.
- description: "Build autonomous agents, summarise or search documents etc", + description: "Build autonomous agents, summarize or search documents, etc.", @@ - description: "Send emails to added mails", + description: "Send emails to added addresses",Also applies to: 152-156
frontend/src/components/workflow/WorkflowExecutionsTable.tsx (1)
15-17: Status literals diverge from backend enums — mapper found; centralize or standardizeconvertStatus in frontend/src/routes/_authenticated/workflow.tsx maps API statuses ('completed' → 'Success', 'active' → 'Running', 'failed' → 'Failed') (frontend/src/routes/_authenticated/workflow.tsx:316–327). WorkflowExecutionsTable.tsx intentionally uses UI labels (frontend/src/components/workflow/WorkflowExecutionsTable.tsx:15–17), so there's no immediate bug. Extract/centralize the mapper into a shared util and update any codepaths that still compare raw backend strings (e.g., step.status === 'failed' in executedWorkflowRenderer) to use that mapper to avoid fragile string comparisons.
frontend/src/components/workflow/executedWorkflowRenderer.tsx (1)
1950-2050: Excessive console logging; gate behind a debug flag.Current logs include payloads and IDs and will spam consoles in prod.
- console.log("Creating workflow from template:", selectedTemplate) + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.log("Creating workflow from template:", selectedTemplate) + }Apply similarly to other logs in this module.
frontend/src/routes/_authenticated/workflow.tsx (3)
16-88: Deduplicate WorkflowTemplate type; import the shared type from ./Types.Local divergence invites drift and runtime bugs. Reuse the canonical type.
-import { ChevronDown, Plus, Layout, Upload, ChevronRight, Search } from "lucide-react" +import { ChevronDown, Plus, Layout, Upload, ChevronRight, Search } from "lucide-react" +import type { WorkflowTemplate } from "@/components/workflow/Types" - interface WorkflowTemplate { /* ...hundreds of lines... */ }
300-307: Safer totalCount parsing.pagination.totalCount may be number/undefined; parseInt on undefined → NaN.
- setExecutionsTotal(parseInt(response.pagination.totalCount)) + setExecutionsTotal(Number(response.pagination?.totalCount ?? 0))
285-313: Handle missing response.data gracefully.Add guards to avoid empty-table flicker on API shape changes.
- if (response.data && Array.isArray(response.data)) { + if (Array.isArray(response?.data)) {frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
91-99: Model validation ties to a stateful list; revalidate after loading models.If models load after initial config, stale selection may persist. Re-run getValidModelId when modelsLoaded flips true.
React.useEffect(() => { - if (isVisible && !modelsLoaded) { + if (isVisible && !modelsLoaded) { ... } }, [isVisible, modelsLoaded]) + + React.useEffect(() => { + if (isVisible) { + setAgentConfig(prev => ({ ...prev, model: getValidModelId(prev.model) })) + } + }, [modelsLoaded, isVisible])Also applies to: 126-133
frontend/src/components/workflow/api/ApiHandlers.ts (2)
360-367: Add timeout to fetch request.Network requests should have timeouts to prevent hanging.
+ const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 60000) // 60 second timeout + // Use direct fetch for file uploads as Hono client may not handle FormData correctly const response = await fetch( `/api/v1/workflow/templates/${templateId}/execute-with-input`, { method: "POST", body: formData, credentials: "include", // This ensures cookies are sent for authentication + signal: controller.signal, } ) + + clearTimeout(timeoutId)
481-484: Consider implementing linkSteps or documenting the workaround.The function throws an error indicating the endpoint doesn't exist. This should either be implemented or properly documented.
Either implement the endpoint on the backend or update the function documentation to clearly indicate that
updateStepshould be used to modifynextStepIdsinstead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
frontend/bun.lockbis excluded by!**/bun.lockbfrontend/src/assets/Document.svgis excluded by!**/*.svgfrontend/src/assets/android.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
frontend/src/components/Sidebar.tsx(1 hunks)frontend/src/components/workflow/AIAgentConfigUI.tsx(2 hunks)frontend/src/components/workflow/ActionBar.tsx(1 hunks)frontend/src/components/workflow/Default.ts(1 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(3 hunks)frontend/src/components/workflow/OnFormSubmissionUI.tsx(1 hunks)frontend/src/components/workflow/TemplateCard.tsx(1 hunks)frontend/src/components/workflow/TemplateSelectionModal.tsx(1 hunks)frontend/src/components/workflow/Types.ts(3 hunks)frontend/src/components/workflow/WhatHappensNextUI.tsx(1 hunks)frontend/src/components/workflow/WorkflowCard.tsx(1 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(10 hunks)frontend/src/components/workflow/WorkflowExecutionsTable.tsx(1 hunks)frontend/src/components/workflow/WorkflowIcons.tsx(1 hunks)frontend/src/components/workflow/WorkflowProvider.tsx(1 hunks)frontend/src/components/workflow/WorkflowUtils.ts(6 hunks)frontend/src/components/workflow/api/ApiHandlers.ts(1 hunks)frontend/src/components/workflow/executedWorkflowRenderer.tsx(1 hunks)frontend/src/routeTree.gen.ts(3 hunks)frontend/src/routes/_authenticated/ChatMessage.test.tsx(8 hunks)frontend/src/routes/_authenticated/workflow.tsx(12 hunks)server/ai/modelConfig.ts(1 hunks)server/api/chat/agents.ts(1 hunks)server/api/chat/jaf-xynemodel-provider.ts(2 hunks)server/api/testEmail.ts(1 hunks)server/api/workflowFileHandler.ts(1 hunks)server/db/schema/index.ts(1 hunks)server/db/schema/workflows.ts(1 hunks)server/db/workflow.ts(1 hunks)server/db/workflowTool.ts(1 hunks)server/integrations/google/index.ts(4 hunks)server/integrations/google/utils.ts(2 hunks)server/server.ts(4 hunks)server/services/emailService.ts(3 hunks)server/types.ts(1 hunks)server/types/workflowTypes.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- server/api/chat/agents.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- server/db/schema/index.ts
- server/types.ts
- frontend/src/components/Sidebar.tsx
- frontend/src/components/workflow/TemplateCard.tsx
- frontend/src/components/workflow/WorkflowProvider.tsx
- frontend/src/components/workflow/Default.ts
- server/ai/modelConfig.ts
- server/integrations/google/utils.ts
- frontend/src/components/workflow/TemplateSelectionModal.tsx
- server/db/workflow.ts
- server/services/emailService.ts
- frontend/src/components/workflow/Types.ts
- frontend/src/components/workflow/WorkflowIcons.tsx
- server/db/schema/workflows.ts
- server/api/testEmail.ts
- frontend/src/components/workflow/WorkflowUtils.ts
- server/integrations/google/index.ts
- frontend/src/routes/_authenticated/ChatMessage.test.tsx
- frontend/src/components/workflow/ActionBar.tsx
- frontend/src/components/workflow/EmailConfigUI.tsx
- server/server.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-10T05:40:04.427Z
Learnt from: naSim087
PR: xynehq/xyne#525
File: frontend/src/routes/_authenticated/admin/integrations/slack.tsx:141-148
Timestamp: 2025-06-10T05:40:04.427Z
Learning: In frontend/src/routes/_authenticated/admin/integrations/slack.tsx, the ConnectAction enum and related connectAction state (lines 141-148, 469-471) are intentionally kept for future development, even though they appear unused in the current implementation.
Applied to files:
frontend/src/routeTree.gen.ts
🧬 Code graph analysis (11)
frontend/src/components/workflow/WorkflowExecutionsTable.tsx (1)
frontend/src/components/HistoryModal.tsx (1)
pageSize(23-23)
frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
frontend/src/api.ts (1)
api(4-4)frontend/src/hooks/useChatStream.ts (1)
createAuthEventSource(212-243)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(392-423)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(392-423)frontend/src/components/workflow/WorkflowIcons.tsx (2)
BackArrowIcon(393-413)CloseIcon(416-436)
frontend/src/components/workflow/WorkflowExecutionModal.tsx (3)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-248)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowExecutionsAPI(240-389)frontend/src/api.ts (1)
api(4-4)
server/db/workflowTool.ts (2)
server/types.ts (1)
TxnOrClient(301-301)server/db/schema/workflows.ts (6)
SelectWorkflowTool(221-221)workflowTool(104-117)InsertWorkflowTool(236-236)SelectToolExecution(228-228)toolExecution(172-189)InsertToolExecution(243-243)
frontend/src/components/workflow/WorkflowCard.tsx (2)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-248)frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
WorkflowExecutionModal(93-573)
frontend/src/components/workflow/WhatHappensNextUI.tsx (1)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowStepsAPI(426-485)
frontend/src/components/workflow/executedWorkflowRenderer.tsx (2)
frontend/src/components/workflow/Types.ts (5)
Step(64-92)Tool(56-61)Flow(160-165)TemplateFlow(95-99)UserDetail(1-6)frontend/src/components/workflow/WorkflowIcons.tsx (3)
FormDocumentIcon(306-325)EditorIcon(68-85)SettingsIcon(87-104)
frontend/src/components/workflow/api/ApiHandlers.ts (2)
frontend/src/components/workflow/Types.ts (3)
WorkflowTemplate(227-248)TemplateFlow(95-99)Flow(160-165)frontend/src/api.ts (1)
api(4-4)
frontend/src/routes/_authenticated/workflow.tsx (5)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-248)frontend/src/components/workflow/api/ApiHandlers.ts (3)
fetchWorkflows(193-197)userWorkflowsAPI(189-224)workflowExecutionsAPI(240-389)frontend/src/components/workflow/WorkflowCard.tsx (1)
WorkflowCard(54-127)frontend/src/components/workflow/WorkflowExecutionsTable.tsx (1)
WorkflowExecutionsTable(31-203)frontend/src/components/workflow/TemplateSelectionModal.tsx (1)
TemplateSelectionModal(24-165)
server/api/workflowFileHandler.ts (1)
server/logger/index.ts (2)
getLogger(36-93)Subsystem(15-15)
🪛 ast-grep (0.38.6)
server/api/workflowFileHandler.ts
[warning] 91-91: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(validation.pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (2)
frontend/src/routeTree.gen.ts (2)
1-10: LGTM! Auto-generated file correctly excluded from linting.The file header properly indicates this is auto-generated and should not be modified.
357-542: Module augmentation properly extends TanStack Router types.The TypeScript module augmentation correctly adds route type information to the TanStack Router module.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (22)
frontend/src/components/workflow/ActionBar.tsx (2)
43-51: Extract inline SVGs to shared icons (consistency + reuse).Move these SVGs to assets or a shared to match repo conventions.
If you have an Icon component, swap to:
/* example */ import { PlayIcon, MinusIcon, PlusIcon } from '@/components/Icon' ... <PlayIcon className="w-4 h-4" /> <MinusIcon className="w-3 h-3 text-slate-600" /> <PlusIcon className="w-3 h-3 text-slate-600" />Confirm if there’s a project-wide icon pattern to follow.
Also applies to: 61-69, 81-90
16-30: Fix zoom step logic (indexOf breaks for non-standard zoom values).indexOf returns -1 for values like 110, causing "Zoom In" to jump to 50% and "Zoom Out" to do nothing. Compute next/prev via numeric comparison.
Apply this diff:
- const handleZoomIn = () => { - const zoomLevels = [50, 75, 100, 125, 150, 175, 200] - const currentIndex = zoomLevels.indexOf(zoomLevel) - if (currentIndex < zoomLevels.length - 1 && onZoomChange) { - onZoomChange(zoomLevels[currentIndex + 1]) - } - } + const handleZoomIn = () => { + const zoomLevels = [50, 75, 100, 125, 150, 175, 200] + const next = zoomLevels.find((z) => z > zoomLevel) + if (next !== undefined) onZoomChange?.(next) + } - const handleZoomOut = () => { - const zoomLevels = [50, 75, 100, 125, 150, 175, 200] - const currentIndex = zoomLevels.indexOf(zoomLevel) - if (currentIndex > 0 && onZoomChange) { - onZoomChange(zoomLevels[currentIndex - 1]) - } - } + const handleZoomOut = () => { + const zoomLevels = [50, 75, 100, 125, 150, 175, 200] + const prev = [...zoomLevels].reverse().find((z) => z < zoomLevel) + if (prev !== undefined) onZoomChange?.(prev) + }frontend/src/components/workflow/WorkflowExecutionModal.tsx (4)
9-58: Import shared type; remove duplicate interface.Avoid type drift. Use the shared WorkflowTemplate from Types.ts.
import { workflowExecutionsAPI } from "./api/ApiHandlers" import { api } from "../../api" - -interface WorkflowTemplate { - id: string - name: string - description: string - version: string - status: string - config: { - ai_model?: string - max_file_size?: string - auto_execution?: boolean - schema_version?: string - allowed_file_types?: string[] - supports_file_upload?: boolean - } - createdBy: string - rootWorkflowStepTemplateId: string - createdAt: string - updatedAt: string - rootStep?: { - id: string - workflowTemplateId: string - name: string - description: string - type: string - timeEstimate: number - metadata: { - icon?: string - step_order?: number - schema_version?: string - user_instructions?: string - } - tool?: { - id: string - type: string - value: { - fields?: Array<{ - name: string - type: string - required?: boolean - default?: any - }> - [key: string]: any - } - config: any - createdBy: string - createdAt: string - updatedAt: string - } - } -} +import type { WorkflowTemplate } from "./Types"
113-115: Fix browser interval typing.Use ReturnType.
- const [pollingInterval, setPollingInterval] = useState<NodeJS.Timeout | null>( + const [pollingInterval, setPollingInterval] = useState<ReturnType<typeof setInterval> | null>( null, )
286-309: Execution ID extraction still brittle. Handle all common shapes first.ApiHandlers.executeTemplate often returns unwrapped data. Prefer direct fields before nested data.
- // Extract execution ID from response.data.execution.id - const currentExecutionId = response.data?.execution?.id + // Try common shapes first + const currentExecutionId = + response?.execution?.id ?? + response?.executionId ?? + response?.id ?? + response?.data?.execution?.id ?? + response?.data?.id
651-684: Inline SVGs: prefer Lucide icons or assets.Align with repo guidance to avoid inline SVG proliferation.
Also applies to: 511-517, 740-767
frontend/src/components/workflow/AIAgentConfigUI.tsx (2)
503-559: Inline SVGs: replace with existing icon/spinner components.Keeps assets centralized and smaller diffs.
Also applies to: 561-579
184-255: SSE connection can leak; track and close on end/error/unmount.Use a ref and cleanup to avoid dangling EventSource.
+ // Track SSE for cleanup + const eventSourceRef = React.useRef<EventSource | null>(null) @@ - let eventSource: EventSource | null = null let generatedPrompt = "" @@ - eventSource = await createAuthEventSource(url.toString()) + eventSourceRef.current = await createAuthEventSource(url.toString()) @@ - if (!eventSource) { + if (!eventSourceRef.current) { reject(new Error("EventSource not created")) return } @@ - eventSource.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { generatedPrompt += event.data }) @@ - eventSource.addEventListener(ChatSSEvents.End, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.End, (event) => { try { const data = JSON.parse(event.data) const finalPrompt = data.fullPrompt || generatedPrompt if (finalPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(finalPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() resolve(finalPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } catch { if (generatedPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(generatedPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() resolve(generatedPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } }) @@ - eventSource.addEventListener(ChatSSEvents.Error, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.Error, (event) => { try { const data = JSON.parse(event.data) - eventSource?.close() + eventSourceRef.current?.close() reject(new Error(data.error || "Error in prompt generation")) } catch { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("Error in prompt generation")) } }) @@ - eventSource.onerror = () => { - eventSource?.close() + eventSourceRef.current.onerror = () => { + eventSourceRef.current?.close() reject(new Error("Connection error during prompt generation")) }Add once near top-level:
+ React.useEffect(() => () => { try { eventSourceRef.current?.close() } catch {} }, [])frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
173-183: Persist backend type: map "file" back to "upload".Prevents round-trip mismatches.
- type: field.type, + type: field.type === "file" ? "upload" : field.type,Please confirm backend expects "upload" here.
486-568: File field has remove UI but no uploader. Add input.Users can’t add files; uploadedFiles is never populated.
- {field.type === "file" ? ( + {field.type === "file" ? ( <div className="space-y-4"> + {/* Upload control */} + <div className="space-y-2"> + <Label className="text-sm font-medium text-slate-700 dark:text-gray-300"> + Upload Files + </Label> + <input + type="file" + multiple + accept={getFieldById(field.id)?.fileTypes?.map(t => `.${t}`).join(",")} + onChange={(e) => { + const files = Array.from(e.target.files || []) + setUploadedFiles(prev => ({ + ...prev, + [field.id]: [...(prev[field.id] || []), ...files], + })) + }} + className="block w-full text-sm text-slate-700 dark:text-gray-300" + /> + </div>frontend/src/components/workflow/executedWorkflowRenderer.tsx (6)
140-147: Scan all tools, don’t assume index 0.Also read config from the matched tool.
- const hasAIAgentTool = - tools && tools.length > 0 && tools[0].type === "ai_agent" + const aiTool = Array.isArray(tools) ? tools.find(t => t?.type === "ai_agent") : undefined + const hasAIAgentTool = !!aiTool @@ - const aiConfig = - (step as any).config || (hasAIAgentTool && tools?.[0]?.config) || {} + const aiConfig = + (step as any).config || (aiTool as any)?.config || {}
327-350: Same: email tool detection should scan all tools.- const hasEmailTool = tools && tools.length > 0 && tools[0].type === "email" + const emailTool = Array.isArray(tools) ? tools.find(t => t?.type === "email") : undefined + const hasEmailTool = !!emailTool
521-528: Same: form tool detection should scan all tools.- const hasFormTool = tools && tools.length > 0 && tools[0].type === "form" + const formTool = Array.isArray(tools) ? tools.find(t => t?.type === "form") : undefined + const hasFormTool = !!formTool
734-742: Same: python_script detection should scan all tools.- const hasPythonScriptTool = - tools && tools.length > 0 && tools[0].type === "python_script" + const pyTool = Array.isArray(tools) ? tools.find(t => t?.type === "python_script") : undefined + const hasPythonScriptTool = !!pyTool
1019-1029: Normalize status strings once and compare canonical values.API returns vary (“Success/Failed/Running”). Add normalizeStatus helper and use it where you style statuses.
Also applies to: 1294-1304, 1738-1750
1156-1161: Iframe sandbox too permissive.Remove allow-same-origin when rendering untrusted HTML.
- <iframe + <iframe srcDoc={resultString} className="w-full h-[500px] border-0 rounded" title="Email HTML Preview" - sandbox="allow-same-origin" + sandbox="" />frontend/src/routes/_authenticated/workflow.tsx (2)
364-395: Guard optional config to avoid runtime crashes when config is undefined.Direct property access can throw if the API omits config.
- if (workflowTemplate.config.allowed_file_types?.includes('pdf')) { + if (workflowTemplate.config?.allowed_file_types?.includes('pdf')) { return '📄' } - if (workflowTemplate.config.ai_model) { + if (workflowTemplate.config?.ai_model) { return '🤖' } - if (workflowTemplate.config.supports_file_upload) { + if (workflowTemplate.config?.supports_file_upload) { return '📁' }- if (workflowTemplate.config.allowed_file_types?.includes('pdf')) { + if (workflowTemplate.config?.allowed_file_types?.includes('pdf')) { return '#E8F5E8' } - if (workflowTemplate.config.ai_model) { + if (workflowTemplate.config?.ai_model) { return '#EBF4FF' } - if (workflowTemplate.config.supports_file_upload) { + if (workflowTemplate.config?.supports_file_upload) { return '#FEF3C7' }
401-423: Editable-mode flag still not respected by WorkflowBuilder.You track editable via isBuilderMode, but pass isEditableMode={selectedTemplate === null}, ignoring caller intent. Align the prop with state.
- setIsBuilderMode(editable) // Set builder mode based on editable parameter + setIsBuilderMode(editable) // respect caller intent- isEditableMode={selectedTemplate === null} + isEditableMode={isBuilderMode}Optional: if WorkflowBuilder consumes builder exclusively, drop one of the two props to avoid drift.
Also applies to: 998-1011
frontend/src/components/workflow/WorkflowCard.tsx (4)
6-47: Avoid duplicating WorkflowTemplate; import and extend the shared type.Prevents drift and incorrect required-ness.
+import type { WorkflowTemplate as BaseWorkflowTemplate } from "./Types" - -interface WorkflowTemplate { - ... -} +type WorkflowTemplate = BaseWorkflowTemplate & { + rootStep?: { metadata?: { icon?: string } } +}
58-73: Guard optional dates to prevent “Invalid Date” rendering.- const formatDate = (dateString: string) => { - const date = new Date(dateString) + const formatDate = (dateString?: string) => { + if (!dateString) return "—" + const date = new Date(dateString) return ( date.toLocaleDateString("en-US", { month: "2-digit", day: "2-digit", year: "numeric", }) + " " + date.toLocaleTimeString("en-US", { hour: "2-digit", minute: "2-digit", hour12: false, }) ) }- Edited at {formatDate(workflow.updatedAt)} + Edited at {formatDate(workflow.updatedAt ?? workflow.createdAt)}Also applies to: 95-96
75-78: Use template-provided icon when available; fall back to bot logo.- const getTemplateIcon = () => { - // Always use bot-logo.svg for workflow cards - return <img src={botLogo} alt="Bot Logo" className="w-5 h-5" /> - } + const getTemplateIcon = () => { + const icon = workflow.rootStep?.metadata?.icon + return icon ? <span className="text-base">{icon}</span> : <img src={botLogo} alt="Bot Logo" className="w-5 h-5" /> + }
121-126: Provide safe fallback for optional description.- workflowDescription={workflow.description} + workflowDescription={workflow.description ?? ""}
🧹 Nitpick comments (26)
frontend/src/components/workflow/ActionBar.tsx (3)
56-60: Add accessible labels to icon-only zoom buttons.Icon-only controls need aria-label/title for screen readers.
<button onClick={handleZoomOut} disabled={zoomLevel <= 50} + aria-label="Zoom out" + title="Zoom out" className="w-6 h-6 flex items-center justify-center rounded-full hover:bg-slate-100 disabled:opacity-50 disabled:cursor-not-allowed transition-colors" > ... <button onClick={handleZoomIn} disabled={zoomLevel >= 200} + aria-label="Zoom in" + title="Zoom in" className="w-6 h-6 flex items-center justify-center rounded-full hover:bg-slate-100 disabled:opacity-50 disabled:cursor-not-allowed transition-colors" >Also applies to: 76-80
34-41: Set type="button" and simplify onClick.Prevents accidental form submission and removes unnecessary ternary.
- <button - onClick={disabled ? undefined : onExecute} + <button + type="button" + onClick={onExecute} disabled={disabled} className={`px-4 py-2 border-none rounded-full text-sm font-medium flex items-center gap-1.5 transition-all duration-200 ${ disabled ? 'bg-gray-300 text-gray-500 cursor-not-allowed' : 'bg-slate-800 hover:bg-slate-700 text-white cursor-pointer' }`} >
72-74: Stabilize zoom label formatting (optional).Avoids showing long decimals if callers pass fractional zooms.
- {zoomLevel}% + {Math.round(zoomLevel)}%frontend/src/components/ui/dropdown.tsx (9)
1-1: A11y: add proper listbox/option roles, ids, and ARIA states.Wire trigger↔listbox with ids, expose busy/invalid states, and mark options with role/aria-selected.
- import React, { useState, useRef, useEffect } from "react" + import React, { useState, useRef, useEffect, useId } from "react"const searchInputRef = useRef<HTMLInputElement>(null) + const triggerId = useId() + const listboxId = `${triggerId}-listbox`<button ref={triggerRef} type="button" onClick={handleToggle} className={triggerClassName} + id={triggerId} style={{ ...(rounded ? { borderRadius: rounded } : {}), - ...(border ? { border: isOpen ? "1px solid #000000" : border } : {}), - ...(isOpen && !border ? { border: "1px solid #000000" } : {}) }} disabled={disabled} aria-haspopup="listbox" aria-expanded={isOpen} + aria-controls={isOpen ? listboxId : undefined} + aria-busy={loading || undefined} + aria-invalid={error || undefined} >- <div - className="overflow-y-auto" - style={{ maxHeight }} - > + <div + className="overflow-y-auto" + style={{ maxHeight }} + role="listbox" + id={listboxId} + aria-labelledby={triggerId} + >- <button + <button key={option.value} type="button" onClick={() => handleSelect(option.value)} disabled={option.disabled} + role="option" + aria-selected={option.value === value} className={cn(Also applies to: 53-59, 213-216, 222-225, 278-281, 288-309
182-193: Avoid inline border overrides; use ring classes for open state.Inline borders override error styles and dark/light tokens. Prefer conditional ring utilities.
const triggerClassName = cn( "relative w-full flex items-center justify-between transition-colors focus:outline-none", rounded ? "" : "rounded-lg", getSizeStyles(), getVariantStyles(), { "cursor-not-allowed opacity-50": disabled, "border-red-500 dark:border-red-400": error, + "ring-1 ring-gray-400 dark:ring-gray-500": isOpen && !border && !error, }, className )style={{ ...(rounded ? { borderRadius: rounded } : {}), - ...(border ? { border: isOpen ? "1px solid #000000" : border } : {}), - ...(isOpen && !border ? { border: "1px solid #000000" } : {}) + ...(border && !error ? { border } : {}), }}Also applies to: 217-221
71-87: Position threshold should respect maxHeight prop (not hardcoded 200).Use the provided maxHeight to decide flip; add it to deps.
useEffect(() => { - if (isOpen && position === "auto" && triggerRef.current) { + if (isOpen && position === "auto" && triggerRef.current) { + const numericMaxHeight = + typeof maxHeight === "string" && maxHeight.endsWith("px") + ? parseFloat(maxHeight) + : 200 const rect = triggerRef.current.getBoundingClientRect() const viewportHeight = window.innerHeight const spaceBelow = viewportHeight - rect.bottom const spaceAbove = rect.top // If there's more space above than below and not enough space below for dropdown - if (spaceAbove > spaceBelow && spaceBelow < 200) { + if (spaceAbove > spaceBelow && spaceBelow < numericMaxHeight) { setDropdownPosition("top") } else { setDropdownPosition("bottom") } } else if (position !== "auto") { setDropdownPosition(position) } - }, [isOpen, position]) + }, [isOpen, position, maxHeight])
90-109: Outside‑click: prefer pointerdown and clear focus timeout.More reliable across input types and avoids lingering timers.
useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { + const handleClickOutside = (event: PointerEvent) => { if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) { setIsOpen(false) setSearchTerm("") } } - if (isOpen) { - document.addEventListener("mousedown", handleClickOutside) + let focusTimer: number | undefined + if (isOpen) { + document.addEventListener("pointerdown", handleClickOutside) // Focus search input if searchable if (searchable && searchInputRef.current) { - setTimeout(() => searchInputRef.current?.focus(), 100) + focusTimer = window.setTimeout(() => searchInputRef.current?.focus(), 100) } } return () => { - document.removeEventListener("mousedown", handleClickOutside) + if (focusTimer) window.clearTimeout(focusTimer) + document.removeEventListener("pointerdown", handleClickOutside) } }, [isOpen, searchable])
111-125: Keyboard navigation is minimal; add roving index (ArrowUp/Down, Home/End, Enter).Improves usability and screen‑reader parity with native selects. Consider managing an activeIndex, scrolling it into view, and selecting on Enter.
Would you like a patch adding roving focus with aria-activedescendant?
262-274: Label the search input for screen readers.Add aria-label; placeholder isn’t a label.
<input ref={searchInputRef} type="text" value={searchTerm} onChange={(e) => setSearchTerm(e.target.value)} placeholder="Search options..." + aria-label="Search options" className={cn(
28-31: Prop naming: rounded/border are CSS strings; consider clearer names.Names like radius and borderStyle avoid confusion with Tailwind’s rounded/border utilities.
60-66: Minor perf: memoize filteredOptions.Avoid refiltering on unrelated re-renders.
- const filteredOptions = searchable - ? options.filter(option => - option.label.toLowerCase().includes(searchTerm.toLowerCase()) - ) - : options + const filteredOptions = React.useMemo(() => { + if (!searchable) return options + const q = searchTerm.toLowerCase() + return options.filter(o => o.label.toLowerCase().includes(q)) + }, [options, searchable, searchTerm])
195-201: Potential clipping in overflow-hidden parents; consider portal.Rendering in-place can be clipped. Consider a portal via ReactDOM.createPortal for menus.
If you see clipping in drawers/modals, we can wire a portal target prop (e.g., portalContainer).
frontend/src/components/workflow/WorkflowExecutionModal.tsx (3)
123-131: Strengthen unmount cleanup.Clear the latest interval via updater to avoid stale closures.
-useEffect(() => { - return () => { - if (pollingInterval) { - clearInterval(pollingInterval) - } - } -}, [pollingInterval]) +useEffect(() => { + return () => { + setPollingInterval((curr) => { + if (curr) clearInterval(curr) + return null + }) + } +}, [])
412-421: Use API handler for status fetch; drop manual Response handling.Keeps shape consistent and simplifies errors.
- const response = await api.workflow.executions[executionId].status.$get() - - // Check if response is ok - if (!response.ok) { - throw new Error(`HTTP ${response.status}: ${response.statusText}`) - } - - const statusData = await response.json() + const statusData = await workflowExecutionsAPI.fetchStatus(executionId)
244-256: Remove noisy console logs or gate behind a debug flag.These flood prod consoles and may leak data.
Example pattern:
const debug = process.env.NODE_ENV !== "production" debug && console.log("…")Also applies to: 269-274, 286-301, 312-317, 345-349, 381-386, 391-421, 470-479
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
96-125: Set a valid model after enums load.Ensure selected model is in the fetched enum list.
React.useEffect(() => { if (isVisible && !modelsLoaded) { const fetchGeminiModels = async () => { @@ fetchGeminiModels() } }, [isVisible, modelsLoaded]) + + // Ensure model stays valid when models change + React.useEffect(() => { + setAgentConfig((prev) => + models.includes(prev.model) ? prev : { ...prev, model: getValidModelId(prev.model) } + ) + }, [models])frontend/src/components/workflow/OnFormSubmissionUI.tsx (3)
150-161: Don’t force required=true on all fields.Honor existing required and only default for missing file fields.
React.useEffect(() => { setFormConfig(prev => ({ ...prev, fields: prev.fields.map(field => ({ ...field, - fileTypes: field.type === "file" && (!field.fileTypes || field.fileTypes.length === 0) + fileTypes: field.type === "file" && (!field.fileTypes || field.fileTypes.length === 0) ? ["pdf", "doc", "docx", "txt", "jpg", "png"] : field.fileTypes, - required: true + required: field.required !== undefined ? field.required : (field.type === "file") })) })) }, [])
166-201: Remove console logs or guard behind debug.Also applies to: 191-201
552-567: newFileType is global; consider per-field state.Prevents conflicts when multiple fields are open.
frontend/src/components/workflow/EmailConfigUI.tsx (1)
69-81: Trim, validate, and dedupe emails (case-insensitive).Prevents bad/duplicate entries and UX glitches.
- const handleAddEmail = () => { - if ( - newEmailAddress && - !emailConfig.emailAddresses.includes(newEmailAddress) - ) { + const handleAddEmail = () => { + const candidate = newEmailAddress.trim() + if (!candidate) return + const isValid = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(candidate) + const exists = emailConfig.emailAddresses.some(e => e.toLowerCase() === candidate.toLowerCase()) + if (isValid && !exists) { setEmailConfig((prev) => ({ ...prev, - emailAddresses: [...prev.emailAddresses, newEmailAddress], + emailAddresses: [...prev.emailAddresses, candidate], })) setNewEmailAddress("") } }frontend/src/components/workflow/executedWorkflowRenderer.tsx (1)
1950-2107: Excessive console logs.Gate behind a debug flag or remove before merge.
Also applies to: 2172-2193, 2197-2206, 2253-2277, 2369-2415, 2417-2425, 2463-2469
frontend/src/routes/_authenticated/workflow.tsx (5)
14-14: Fix CI: remove unused icon import (TS6133).Upload isn’t used; it breaks the TypeScript build check.
-import { ChevronDown, Plus, Layout, Upload, ChevronRight, Search } from "lucide-react" +import { ChevronDown, Plus, Layout, ChevronRight, Search } from "lucide-react"
151-152: Sort guard: createdAt can be missing; fall back to updatedAt or 0.Prevents NaN comparisons and preserves order when timestamps are absent.
- .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) // Sort by newest first (creation date) + .sort( + (a, b) => + new Date(b.createdAt ?? b.updatedAt ?? 0).getTime() - + new Date(a.createdAt ?? a.updatedAt ?? 0).getTime(), + ) // newest first
302-304: Robust totalCount handling.Avoid parseInt on possibly undefined; coerce safely to number.
- setExecutionsTotal(parseInt(response.pagination.totalCount)) - setExecutionsPage(response.pagination.page) + setExecutionsTotal(Number(response.pagination?.totalCount ?? 0)) + setExecutionsPage(Number(response.pagination?.page ?? 1))
16-88: Type drift: local WorkflowTemplate diverges from shared type; consider extending the shared type instead.Local required fields (config/createdAt/updatedAt) don’t match backend reality and mask nullability issues.
+import type { WorkflowTemplate as BaseWorkflowTemplate } from "@/components/workflow/Types" - -interface WorkflowTemplate { - ... - config: { - ... - }; - createdAt: string; - updatedAt: string; - ... -} +// Narrow extension for fields this module reads that aren’t in the shared type +type WorkflowTemplate = BaseWorkflowTemplate & { + rootStep?: { metadata?: { icon?: string } } +}If you keep the local interface, at minimum make config/createdAt/updatedAt optional and rootWorkflowStepTemplateId nullable to match the API.
148-149: Reduce noisy console logs in production.Wrap logs under a dev guard or use a scoped logger.
- console.log('Workflows API Response:', workflows) + if (process.env.NODE_ENV !== 'production') console.log('Workflows API Response:', workflows)Apply similarly to other console.log blocks in this file.
Also applies to: 173-176, 288-289, 396-399, 428-429, 438-469, 479-566, 558-563, 974-976, 1026-1028
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
frontend/src/components/ui/dropdown.tsx(1 hunks)frontend/src/components/workflow/AIAgentConfigUI.tsx(2 hunks)frontend/src/components/workflow/ActionBar.tsx(1 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(3 hunks)frontend/src/components/workflow/OnFormSubmissionUI.tsx(1 hunks)frontend/src/components/workflow/WorkflowCard.tsx(1 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(10 hunks)frontend/src/components/workflow/executedWorkflowRenderer.tsx(1 hunks)frontend/src/routes/_authenticated/workflow.tsx(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
frontend/src/components/ui/dropdown.tsx (1)
frontend/src/lib/utils.ts (1)
cn(5-7)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(392-423)frontend/src/components/workflow/WorkflowIcons.tsx (2)
BackArrowIcon(393-413)CloseIcon(416-436)
frontend/src/components/workflow/executedWorkflowRenderer.tsx (2)
frontend/src/components/workflow/Types.ts (5)
Step(64-92)Tool(56-61)Flow(160-165)TemplateFlow(95-99)UserDetail(1-6)frontend/src/components/workflow/WorkflowIcons.tsx (3)
FormDocumentIcon(306-325)EditorIcon(68-85)SettingsIcon(87-104)
frontend/src/components/workflow/WorkflowCard.tsx (2)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-248)frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
WorkflowExecutionModal(94-794)
frontend/src/routes/_authenticated/workflow.tsx (6)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-248)frontend/src/components/workflow/api/ApiHandlers.ts (3)
fetchWorkflows(193-197)userWorkflowsAPI(189-224)workflowExecutionsAPI(240-389)frontend/src/api.ts (1)
api(4-4)frontend/src/components/workflow/WorkflowCard.tsx (1)
WorkflowCard(55-129)frontend/src/components/workflow/WorkflowExecutionsTable.tsx (1)
WorkflowExecutionsTable(31-203)frontend/src/components/workflow/TemplateSelectionModal.tsx (1)
TemplateSelectionModal(24-165)
frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
frontend/src/api.ts (1)
api(4-4)frontend/src/hooks/useChatStream.ts (1)
createAuthEventSource(212-243)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(392-423)
frontend/src/components/workflow/EmailConfigUI.tsx (1)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(392-423)
frontend/src/components/workflow/WorkflowExecutionModal.tsx (3)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-248)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowExecutionsAPI(240-389)frontend/src/api.ts (1)
api(4-4)
🪛 GitHub Actions: TypeScript Build Check
frontend/src/components/workflow/executedWorkflowRenderer.tsx
[error] 19-19: TypeScript error TS6133: 'Panel' is declared but its value is never read. (Command: bunx tsc -b)
[error] 1926-1926: TypeScript error TS6133: 'selectedNodes' is declared but its value is never read. (Command: bunx tsc -b)
[error] 1927-1927: TypeScript error TS6133: 'selectedEdges' is declared but its value is never read. (Command: bunx tsc -b)
[error] 2184-2184: TypeScript error TS2353: Object literal may only specify known properties, and 'pathOptions' does not exist in type 'Edge'. (Command: bunx tsc -b)
frontend/src/routes/_authenticated/workflow.tsx
[error] 14-14: TypeScript error TS6133: 'Upload' is declared but its value is never read. (Command: bunx tsc -b)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (10)
frontend/src/components/workflow/executedWorkflowRenderer.tsx (9)
137-145: Scan all tools; don’t assume index 0 for ai_agent.Detect across the whole array and fetch the first matching tool for config.
- const hasAIAgentTool = - tools && tools.length > 0 && tools[0].type === "ai_agent" + const hasAIAgentTool = Array.isArray(tools) && tools.some(t => t?.type === "ai_agent") if (step.type === "ai_agent" || hasAIAgentTool) { // Get config from step or tool - const aiConfig = - (step as any).config || (hasAIAgentTool && tools?.[0]?.config) || {} + const aiTool = Array.isArray(tools) ? tools.find(t => t?.type === "ai_agent") : undefined + const aiConfig = (step as any).config || aiTool?.config || {}
324-333: Same: email tool detection should scan all tools.Use some(...) and keep config derivation null‑safe.
- const hasEmailTool = tools && tools.length > 0 && tools[0].type === "email" + const hasEmailTool = Array.isArray(tools) && tools.some(t => t?.type === "email") @@ - const emailConfig = - (step as any).config || {} + const emailConfig = (step as any).config || {}Also applies to: 335-339, 345-348
519-521: Same: form tool detection should scan all tools.- const hasFormTool = tools && tools.length > 0 && tools[0].type === "form" + const hasFormTool = Array.isArray(tools) && tools.some(t => t?.type === "form")
732-735: Same: python_script tool detection should scan all tools.- const hasPythonScriptTool = - tools && tools.length > 0 && tools[0].type === "python_script" + const hasPythonScriptTool = + Array.isArray(tools) && tools.some(t => t?.type === "python_script")
31-31: Normalize status values once; compare normalized everywhere.API sends “Success/Failed/Running” and “completed/failed/running/active/in_progress”. Add a helper and use it in all comparisons and badges.
+// normalize step/tool status variants to: completed | failed | running | pending | '' +const normalizeStatus = (s?: string) => { + const v = (s || "").toLowerCase() + if (["success", "successful", "completed", "done"].includes(v)) return "completed" + if (["running", "active", "in_progress"].includes(v)) return "running" + if (["failed", "error"].includes(v)) return "failed" + if (!v || v === "pending") return "pending" + return v +}Examples (apply similarly in all listed ranges):
- const hasFailedToolExecution = - tools && tools.some((tool) => (tool as any).status === "failed") - const isFailed = step.status === "failed" || hasFailedToolExecution + const hasFailedToolExecution = + Array.isArray(tools) && tools.some((tool) => normalizeStatus((tool as any).status) === "failed") + const isFailed = normalizeStatus(step.status) === "failed" || hasFailedToolExecution- {step.status === "running" || step.status === "in_progress" + {normalizeStatus(step.status) === "running" ? "In Progress" - : step.status === "completed" || step.status === "done" + : normalizeStatus(step.status) === "completed" ? "Completed" - : step.status === "pending" + : normalizeStatus(step.status) === "pending" ? "Pending" : step.status}- const isSuccess = - step.status === "completed" && - tool.status === "completed" + const isSuccess = + normalizeStatus(step.status) === "completed" && + normalizeStatus(tool.status) === "completed"Also applies to: 151-154, 345-348, 523-526, 737-740, 852-855, 1019-1027, 1291-1303, 1739-1747, 1760-1766, 1798-1805, 1819-1824, 1839-1856
2180-2183: Remove unsupported Edge.pathOptions (TS2353).XYFlow/React Flow Edge doesn’t accept pathOptions; use style/markerEnd only.
style: { stroke: "#D1D5DB", strokeWidth: 2, strokeLinecap: "round", strokeLinejoin: "round", }, - pathOptions: { - borderRadius: 20, - offset: 20, - },style: { stroke: "#6B7280", strokeWidth: 2, strokeLinecap: "round", strokeLinejoin: "round", }, - pathOptions: { - borderRadius: 20, - offset: 20, - },Also applies to: 2219-2223
1930-1930: Remove dead pollingInterval state and cleanup.Never set; causes NodeJS.Timeout type drift in DOM.
- // Cleanup polling on component unmount - const [pollingInterval] = useState<NodeJS.Timeout | null>(null) @@ - useEffect(() => { - return () => { - if (pollingInterval) { - clearInterval(pollingInterval) - } - } - }, [pollingInterval])Also applies to: 2333-2339
893-896: Avoid inline SVG; reuse an icon component or asset.Keeps bundle lean and styling consistent; align with prior guidance.
1154-1159: Iframe sandbox too permissive (allow-same-origin). Remove to enforce unique origin.Rendering untrusted HTML with same‑origin weakens the sandbox.
- <iframe - srcDoc={resultString} - className="w-full h-[500px] border-0 rounded" - title="Email HTML Preview" - sandbox="allow-same-origin" - /> + <iframe + srcDoc={resultString} + className="w-full h-[500px] border-0 rounded" + title="Email HTML Preview" + sandbox="" + referrerPolicy="no-referrer" + />frontend/src/routes/_authenticated/workflow.tsx (1)
16-30: Make config optional and guard all accesses. Prevent runtime crash when API omits config.Local type marks config as required, but upstream may omit it.
- config: { + config?: { @@ - const getTemplateIcon = (workflowTemplate: WorkflowTemplate): string => { + const getTemplateIcon = (workflowTemplate: WorkflowTemplate): string => { @@ - if (workflowTemplate.config.allowed_file_types?.includes('pdf')) { + if (workflowTemplate.config?.allowed_file_types?.includes('pdf')) { return '📄' } - if (workflowTemplate.config.ai_model) { + if (workflowTemplate.config?.ai_model) { return '🤖' } - if (workflowTemplate.config.supports_file_upload) { + if (workflowTemplate.config?.supports_file_upload) { return '📁' } @@ - const getTemplateIconBgColor = (workflowTemplate: WorkflowTemplate): string => { - if (workflowTemplate.config.allowed_file_types?.includes('pdf')) { + const getTemplateIconBgColor = (workflowTemplate: WorkflowTemplate): string => { + if (workflowTemplate.config?.allowed_file_types?.includes('pdf')) { return '#E8F5E8' } - if (workflowTemplate.config.ai_model) { + if (workflowTemplate.config?.ai_model) { return '#EBF4FF' } - if (workflowTemplate.config.supports_file_upload) { + if (workflowTemplate.config?.supports_file_upload) { return '#FEF3C7' }Also applies to: 363-381, 383-394
🧹 Nitpick comments (21)
frontend/src/components/workflow/executedWorkflowRenderer.tsx (2)
1946-1979: Trim verbose console logs or guard behind a debug flag.These spam the console and may leak user data. Keep minimal logs in production.
Also applies to: 2061-2074, 2095-2103, 2193-2195, 2248-2265, 1343-1361, 1439-1443, 1464-1469, 1487-1492, 1505-1521, 1524-1529, 1533-1536, 1565-1578, 1589-1611, 1616-1627, 1645-1672, 1677-1714, 1719-1725, 1759-1779, 1793-1800, 1825-1835
2233-2236: Delete commented‑out onSelectionChange code.Avoid dead/commented code; SCM keeps history.
frontend/src/routes/_authenticated/workflow.tsx (5)
316-328: Harden status mapping to handle variants.Map case‑insensitive inputs and both API shapes: completed/success, running/active, failed.
- const convertStatus = (apiStatus: string): 'Success' | 'Running' | 'Failed' => { - switch (apiStatus) { - case 'completed': - return 'Success' - case 'active': - return 'Running' - case 'failed': - return 'Failed' - default: - return 'Failed' - } - } + const convertStatus = (apiStatus: string): 'Success' | 'Running' | 'Failed' => { + const v = (apiStatus || '').toLowerCase() + if (['completed', 'success', 'successful', 'done'].includes(v)) return 'Success' + if (['running', 'active', 'in_progress'].includes(v)) return 'Running' + if (['failed', 'error'].includes(v)) return 'Failed' + return 'Failed' + }
286-304: Be defensive with pagination types.Pagination counts may be strings or missing. Use Number(...) with fallback.
- setExecutionsTotal(parseInt(response.pagination.totalCount)) - setExecutionsPage(response.pagination.page) + setExecutionsTotal(Number(response.pagination?.totalCount ?? 0)) + setExecutionsPage(Number(response.pagination?.page ?? 1))
401-417: Editable/view mode props look duplicated. Align on a single flag.You pass both isEditableMode and builder. Ensure WorkflowBuilder/ExecutedWorkflowRenderer consume the intended one; otherwise remove the redundant prop.
Also applies to: 986-1011
425-576: Centralize execution fetch/shape normalization.Use workflowExecutionsAPI.fetchById and normalize response once. Reduces branches and console noise.
Do you want a small adapter util that coerces any of the observed shapes into { stepExecutions, toolExecutions, rootWorkflowStepExeId }?
148-156: Reduce console logs or guard behind env flag.Logs are numerous and may include user data. Keep only actionable errors.
Also applies to: 170-197, 199-213, 219-221, 248-270, 286-314, 371-381, 383-394, 404-421, 428-469, 472-576, 692-717, 729-752, 760-783, 787-801, 826-858, 974-976
server/db/workflow.ts (14)
59-68: Return-time validation for lists (use schema parse).
return templates as SelectWorkflowTemplate[]skips runtime validation and can leak unexpected shapes. Prefer parsing the array.Apply:
- return templates as SelectWorkflowTemplate[] + return selectWorkflowTemplateSchema.array().parse(templates)
87-121: Validate inputs on create (step templates) and rely on DB defaults.Currently we accept raw
dataand manually default arrays/metadata. Prefer validating with the insert schema and letting DB defaults fill missing values.Example approach:
- const [step] = await trx - .insert(workflowStepTemplate) - .values({ - workflowTemplateId: data.workflowTemplateId, - name: data.name, - description: data.description, - type: data.type, - parentStepId: data.parentStepId, - prevStepIds: data.prevStepIds || [], - nextStepIds: data.nextStepIds || [], - toolIds: data.toolIds || [], - timeEstimate: data.timeEstimate || 0, - metadata: data.metadata || {}, - }) + // If available, use an insert schema: insertWorkflowStepTemplateSchema.parse(data) + const [step] = await trx + .insert(workflowStepTemplate) + .values({ + workflowTemplateId: data.workflowTemplateId, + name: data.name, + description: data.description, + type: data.type, + parentStepId: data.parentStepId, + prevStepIds: data.prevStepIds, + nextStepIds: data.nextStepIds, + toolIds: data.toolIds, + timeEstimate: data.timeEstimate, + metadata: data.metadata, + }) .returning()Also consider parsing the return with the select schema for consistency.
122-133: Consistent ordering and index hint.This lists steps in ascending
createdAtwhile templates/executions usedesc. Pick one convention repo-wide. If this endpoint will be hot, add/verify an index on(workflow_template_id, created_at)to support the WHERE + ORDER BY.
136-158: Create execution: add input validation and rely on DB defaults.Mirror the step-template recommendation: validate
datawith an insert schema and avoid manual defaulting where the DB already defines defaults.
160-171: Return-time parse for single fetch.Use the select schema to validate shape instead of casting.
Apply:
- return execution ? (execution as SelectWorkflowExecution) : null + return execution ? selectWorkflowTemplateSchema + // Replace with the proper selectWorkflowExecutionSchema + .parse(execution) : nullNote: import the correct
selectWorkflowExecutionSchema.
173-182: Validate list responses (executions).Same comment as templates: avoid raw cast; parse the array with the select schema.
- return executions as SelectWorkflowExecution[] + return selectWorkflowExecutionSchema.array().parse(executions)
201-235: Create step execution: input validation and avoid manual defaults.Same pattern as other create fns: validate with insert schema and lean on DB defaults.
236-247: Normalize return typing; avoid ad-hocas anyfor metadata.Mapping only
metadatatoanyis inconsistent. Prefer schema-parse or a sharedJsonValuetype alias to keep the surface predictable.- return results.map(result => ({ ...result, metadata: result.metadata as any })) + return selectWorkflowStepExecutionSchema.array().parse(results)
249-261: Use schema parse for single fetch (step execution).Follow the same return-time validation approach as above.
- return stepExecution ? ({ ...stepExecution, metadata: stepExecution.metadata as any }) : null + return stepExecution + ? selectWorkflowStepExecutionSchema.parse(stepExecution) + : null
31-44: Standardizemetadatatyping (avoidas any).Today different functions return
metadatatyped differently. Define a sharedJsonValuetype and use it in the select types, or always schema-parse returns sometadatais consistently typed.Also applies to: 103-121, 146-158, 217-235, 240-247, 259-260, 289-290
20-44: Tiny nit: avoid duplicating DB defaults in code.
version || "1.0.0"andconfig || {}are already defaulted in the schema; letting the DB populate keeps one source of truth.- version: data.version || "1.0.0", - config: data.config || {}, + version: data.version, + config: data.config,
174-182: Optional: pagination.
getAllWorkflowExecutionsreturns the full set; add cursor/limit to prevent unbounded scans as data grows.
59-68: Optional: stable secondary sort.Add
idas a tiebreaker to ORDER BY to ensure deterministic pagination.Example:
.orderBy(desc(createdAt), desc(id))Also applies to: 173-182, 122-133, 236-247
70-85: Would you like a quick follow-up PR to add optimistic concurrency?I can add a pattern that requires
updatedAtin the WHERE clause and surfaces a specific error if 0 rows are updated. Helps avoid last-writer-wins surprises.Also applies to: 184-199, 262-290
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/workflow/executedWorkflowRenderer.tsx(1 hunks)frontend/src/routes/_authenticated/workflow.tsx(11 hunks)server/db/workflow.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
server/db/workflow.ts (2)
server/types.ts (1)
TxnOrClient(301-301)server/db/schema/workflows.ts (12)
SelectWorkflowTemplate(215-217)workflowTemplate(60-76)selectWorkflowTemplateSchema(192-192)InsertWorkflowTemplate(230-232)SelectWorkflowStepTemplate(218-220)workflowStepTemplate(79-101)SelectWorkflowExecution(222-224)workflowExecution(120-140)InsertWorkflowExecution(237-239)SelectWorkflowStepExecution(225-227)workflowStepExecution(143-169)InsertWorkflowStepExecution(240-242)
frontend/src/components/workflow/executedWorkflowRenderer.tsx (2)
frontend/src/components/workflow/Types.ts (5)
Step(64-92)Tool(56-61)Flow(160-165)TemplateFlow(95-99)UserDetail(1-6)frontend/src/components/workflow/WorkflowIcons.tsx (3)
FormDocumentIcon(306-325)EditorIcon(68-85)SettingsIcon(87-104)
frontend/src/routes/_authenticated/workflow.tsx (7)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-248)frontend/src/components/workflow/api/ApiHandlers.ts (3)
fetchWorkflows(193-197)userWorkflowsAPI(189-224)workflowExecutionsAPI(240-389)server/db/schema/workflows.ts (1)
workflowTemplate(60-76)frontend/src/api.ts (1)
api(4-4)frontend/src/components/workflow/WorkflowCard.tsx (1)
WorkflowCard(55-129)frontend/src/components/workflow/WorkflowExecutionsTable.tsx (1)
WorkflowExecutionsTable(31-203)frontend/src/components/workflow/TemplateSelectionModal.tsx (1)
TemplateSelectionModal(24-165)
🔇 Additional comments (2)
server/db/workflow.ts (2)
31-44: Nice: runtime-parse on create.Parsing with
selectWorkflowTemplateSchema.parse(template)is a good pattern to keep return shapes consistent.
1-290: Verify caller-side validation & unsafe.set({...data})usageSandbox couldn't scan the repo; run locally to locate DAL callers and unsafe spreads:
- Find DAL callers:
rg -n -C2 -S -g '!/node_modules/' 'createWorkflowTemplate|createWorkflowExecution|createWorkflowStepTemplate|createWorkflowStepExecution|updateWorkflowTemplate|updateWorkflowExecution|updateWorkflowStepExecution|getWorkflowTemplateById|getWorkflowExecutionById|getWorkflowStepExecutionById|getWorkflowStepTemplatesByTemplate|getWorkflowStepExecutionsByExecution|getAllWorkflowTemplates|getAllWorkflowExecutions'- Find unsafe spreads and inspect context:
rg -n -C2 -S -g '!/node_modules/' '.set\s*(\s*{\s*...'
rg -n -C2 -S -g '!/node_modules/' '.set\s*('If any
.set({...data})is present (server/db/workflow.ts update* functions currently use spreads), remove the unsafe spread: explicitly whitelist/map allowed fields in the DAL or enforce strict request-layer validation before passing data into.set().
avirupsinha12
force-pushed
the
workflow-mvp-integration
branch
from
ee3b4b8 to
d5ae87f
Compare
avirupsinha12
force-pushed
the
workflow-mvp-integration
branch
from
28c0529 to
57fa5f8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/server.ts (2)
777-779: Missing leading slash in route path breaks files/upload endpoint"files/upload" becomes "/api/v1files/upload". Add the leading slash.
- .post("files/upload", handleFileUpload) + .post("/files/upload", handleFileUpload)
1136-1141: Redundant/incorrect base path: '/admin' prefixed twiceWithin
.basePath("/admin"), defining"/admin/users/:userId/feedback"yields/api/v1/admin/admin/users/:userId/feedback. Likely unintended duplication vs the earlier"/users/:userId/feedback".- .get( - "/admin/users/:userId/feedback", + .get( + "/users/:userId/feedback", zValidator("query", userAgentLeaderboardQuerySchema), GetAllUserFeedbackMessages, )
♻️ Duplicate comments (22)
frontend/src/components/workflow/ActionBar.tsx (1)
17-31: Fix zoom step logic; avoid indexOf for non-standard zoom values.indexOf(-1) causes “zoom in” from 73% to jump to 50% and “zoom out” to no-op. Compute next/prev via numeric comparison and dedupe levels into a single constant; also bind disabled states to that constant.
@@ -import React from "react" -import { Play, Minus, Plus } from "lucide-react" +import React from "react" +import { Play, Minus, Plus } from "lucide-react" +const ZOOM_LEVELS = [50, 75, 100, 125, 150, 175, 200] as const @@ - const handleZoomIn = () => { - const zoomLevels = [50, 75, 100, 125, 150, 175, 200] - const currentIndex = zoomLevels.indexOf(zoomLevel) - if (currentIndex < zoomLevels.length - 1 && onZoomChange) { - onZoomChange(zoomLevels[currentIndex + 1]) - } - } + const handleZoomIn = () => { + if (!onZoomChange) return + const next = ZOOM_LEVELS.find((z) => z > zoomLevel) + if (next !== undefined) onZoomChange(next) + } @@ - const handleZoomOut = () => { - const zoomLevels = [50, 75, 100, 125, 150, 175, 200] - const currentIndex = zoomLevels.indexOf(zoomLevel) - if (currentIndex > 0 && onZoomChange) { - onZoomChange(zoomLevels[currentIndex - 1]) - } - } + const handleZoomOut = () => { + if (!onZoomChange) return + const prev = [...ZOOM_LEVELS].reverse().find((z) => z < zoomLevel) + if (prev !== undefined) onZoomChange(prev) + } @@ - disabled={zoomLevel <= 50} + disabled={zoomLevel <= ZOOM_LEVELS[0]} @@ - disabled={zoomLevel >= 200} + disabled={zoomLevel >= ZOOM_LEVELS[ZOOM_LEVELS.length - 1]}frontend/src/components/workflow/executedWorkflowRenderer.tsx (4)
133-141: Scan all tools; don’t assume index 0Current checks miss tools beyond tools[0]. Use Array.some/find and (for AI agent) read config from the matched tool.
- const hasAIAgentTool = - tools && tools.length > 0 && tools[0].type === "ai_agent" + const hasAIAgentTool = + Array.isArray(tools) && tools.some((t) => t?.type === "ai_agent") - const aiConfig = - (step as any).config || (hasAIAgentTool && tools?.[0]?.config) || {} + const aiConfig = + (step as any).config || + tools?.find((t) => t?.type === "ai_agent")?.config || + {}- const hasEmailTool = tools && tools.length > 0 && tools[0].type === "email" + const hasEmailTool = + Array.isArray(tools) && tools.some((t) => t?.type === "email")- const hasFormTool = tools && tools.length > 0 && tools[0].type === "form" + const hasFormTool = + Array.isArray(tools) && tools.some((t) => t?.type === "form")- const hasPythonScriptTool = - tools && tools.length > 0 && tools[0].type === "python_script" + const hasPythonScriptTool = + Array.isArray(tools) && tools.some((t) => t?.type === "python_script")Also applies to: 320-321, 515-517, 728-731
1879-1880: Remove unused polling state/cleanupDead code and NodeJS.Timeout in browser context. Clean it up.
- // Cleanup polling on component unmount - const [pollingInterval] = useState<NodeJS.Timeout | null>(null) ... - useEffect(() => { - return () => { - if (pollingInterval) { - clearInterval(pollingInterval) - } - } - }, [pollingInterval]) + // (removed unused polling state/cleanup)Also applies to: 2282-2288
1124-1129: Iframe sandbox too permissive; drop allow-same-origin for untrusted HTMLRendering untrusted HTML with same-origin breaks sandbox isolation and increases XSS impact radius. Use a unique opaque origin.
- <iframe - srcDoc={resultString} - className="w-full h-[500px] border-0 rounded" - title="Email HTML Preview" - sandbox="allow-same-origin" - /> + <iframe + srcDoc={resultString} + className="w-full h-[500px] border-0 rounded" + title="Email HTML Preview" + sandbox="" + referrerPolicy="no-referrer" + />
2129-2132: TS2353: Edge doesn’t have ‘pathOptions’These literals cause type errors and break the build. Remove or migrate to supported props.
style: { stroke: "#D1D5DB", strokeWidth: 2, strokeLinecap: "round", strokeLinejoin: "round", }, - pathOptions: { - borderRadius: 20, - offset: 20, - }, markerEnd: { type: "arrowclosed", color: "#D1D5DB", },style: { stroke: "#6B7280", strokeWidth: 2, strokeLinecap: "round", strokeLinejoin: "round", }, - pathOptions: { - borderRadius: 20, - offset: 20, - }, markerEnd: { type: "arrowclosed", color: "#6B7280", },Also applies to: 2168-2171
server/api/workflowFileHandler.ts (6)
251-254: /tmp is ephemeral; make upload root configurable and persistentOS may purge /tmp. Use a configurable, persistent dir (volume or object storage) and ensure permissions.
- const baseDir = "/tmp/workflow_uploads" + const baseDir = + process.env.WORKFLOW_UPLOAD_DIR ?? "/var/lib/xyne/workflow_uploads"
91-99: User-controlled regex → potential ReDoS; validate before compilingCreating RegExp from arbitrary input can hang the event loop. Bound length, try/catch, and prefer a safe engine (RE2) if possible. This was flagged earlier and still applies.
- if (validation.pattern) { - const regex = new RegExp(validation.pattern) - if (!regex.test(value)) { + if (validation.pattern) { + if (validation.pattern.length > 256) { + return { isValid: false, error: `Field '${fieldId}' pattern too long` } + } + let regex: RegExp + try { + regex = new RegExp(validation.pattern) + } catch { + return { isValid: false, error: `Field '${fieldId}' pattern is invalid` } + } + if (!regex.test(value)) { return { isValid: false, error: `Field '${fieldId}' format is invalid`, } } }Optional: gate via RE2 if available and reject unsafe patterns.
251-256: Sanitize path segments derived from IDsNormalize workflowExecutionId and workflowStepId before path.join to prevent traversal and odd chars. Previously flagged; still needed.
- const executionDir = path.join(baseDir, workflowExecutionId) - const stepDir = path.join(executionDir, workflowStepId) + const sanitizeId = (s: string) => s.replace(/[^A-Za-z0-9_-]/g, "_") + const safeExecutionId = sanitizeId(workflowExecutionId) + const safeWorkflowStepId = sanitizeId(workflowStepId) + const executionDir = path.join(baseDir, safeExecutionId) + const stepDir = path.join(executionDir, safeWorkflowStepId) @@ - "workflow_uploads", - workflowExecutionId, - workflowStepId, + "workflow_uploads", + safeExecutionId, + safeWorkflowStepId, fileName, ) @@ - workflowExecutionId: workflowExecutionId, - workflowStepId: workflowStepId, + workflowExecutionId: workflowExecutionId, + workflowStepId: workflowStepId,Note: keep original IDs in response fields, but only use sanitized versions for filesystem paths.
Also applies to: 270-275, 277-289
258-263: Sanitize filename and avoid deprecated substrPrevent path traversal and weird chars; use slice instead of substr.
- const timestamp = Date.now() - const randomSuffix = Math.random().toString(36).substr(2, 9) - const fileExtension = file.name.split(".").pop()?.toLowerCase() || "" - const fileName = `${timestamp}_${randomSuffix}_${file.name}` + const timestamp = Date.now() + const randomSuffix = Math.random().toString(36).slice(2, 11) + const originalBaseName = path.basename(file.name).replace(/[^\w.\-]/g, "_") + const fileExtension = originalBaseName.split(".").pop()?.toLowerCase() || "" + const fileName = `${timestamp}_${randomSuffix}_${originalBaseName}`
265-267: Bun-only write will crash under Node; add fs fallbackGuard Bun.write and fallback to fs/promises for Node runtimes. Ensure target dir exists.
- const arrayBuffer = await file.arrayBuffer() - await Bun.write(filePath, new Uint8Array(arrayBuffer)) + const arrayBuffer = await file.arrayBuffer() + const bytes = new Uint8Array(arrayBuffer) + if (typeof (globalThis as any).Bun !== "undefined" && (globalThis as any).Bun.write) { + await (globalThis as any).Bun.write(filePath, bytes) + } else { + const { writeFile } = await import("node:fs/promises") + await writeFile(filePath, Buffer.from(bytes)) + }
210-220: Use basename for extension derivationAvoid trusting file.name; strip directory segments first.
- const fileExtension = file.name.split(".").pop()?.toLowerCase() || "" + const baseName = path.basename(file.name) + const fileExtension = baseName.split(".").pop()?.toLowerCase() || ""frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
183-206: Close SSE on unmount and on all error paths (use a ref)
Use a ref for EventSource and ensure cleanup on unmount and when rejecting, to avoid dangling connections. This was raised earlier.@@ - let eventSource: EventSource | null = null + // Track SSE instance for cleanup + const eventSourceRef = React.useRef<EventSource | null>(null) let generatedPrompt = "" @@ - eventSource = await createAuthEventSource(url.toString()) + eventSourceRef.current = await createAuthEventSource(url.toString()) @@ - if (!eventSource) { + if (!eventSourceRef.current) { reject(new Error("EventSource not created")) return } @@ - eventSource.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { generatedPrompt += event.data }) @@ - eventSource.addEventListener(ChatSSEvents.End, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.End, (event) => { try { @@ - eventSource?.close() + eventSourceRef.current?.close() resolve(finalPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } catch (parseError) { @@ - eventSource?.close() + eventSourceRef.current?.close() resolve(generatedPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } }) @@ - eventSource.addEventListener(ChatSSEvents.Error, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.Error, (event) => { try { const data = JSON.parse(event.data) - eventSource?.close() + eventSourceRef.current?.close() reject(new Error(data.error || "Error in prompt generation")) } catch (parseError) { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("Error in prompt generation")) } })Add once in the component:
React.useEffect(() => { return () => { try { (eventSourceRef as any)?.current?.close?.() } catch {} } }, [])Also applies to: 239-255
frontend/src/components/workflow/api/ApiHandlers.ts (4)
262-278: Stop manual parsing; reuse extractor and keep the envelope
Simplifies and unifies response handling.- const response = await api.workflow.executions.$get({ query }) - - if (!response.ok) { - const errorData = await response.json().catch(() => ({ message: "Network error" })) - throw new Error(errorData.message || `HTTP ${response.status}: ${response.statusText}`) - } - - const responseData = await response.json() - - if (responseData.success !== undefined) { - return responseData as WorkflowExecutionsResponse - } - - return responseData as WorkflowExecutionsResponse + const response = await api.workflow.executions.$get({ query }) + return extractResponseData<WorkflowExecutionsResponse>(response, { keepEnvelope: true })
6-50: Don’t redefine WorkflowTemplate locally; import shared type
Prevents drift with server/FE shared Types.-import { Flow, TemplateFlow } from "../Types" +import { Flow, TemplateFlow, WorkflowTemplate } from "../Types" @@ -interface WorkflowTemplateResponse { - data: WorkflowTemplate[] -} -interface WorkflowTemplate { - ... -} +type WorkflowTemplateResponse = { data: WorkflowTemplate[] }
86-86: Execution status union incomplete
Add 'draft' and 'paused' to match server/status API.- status: "completed" | "active" | "failed" + status: "draft" | "active" | "paused" | "completed" | "failed"
114-135: Harden extractResponseData (204/no body, error envelopes, keepEnvelope)
Make the helper robust and reusable.-async function extractResponseData<T>(response: Response): Promise<T> { +async function extractResponseData<T>( + response: Response, + opts: { keepEnvelope?: boolean } = {}, +): Promise<T> { if (!response.ok) { - const errorData = await response.json().catch(() => ({ message: "Network error" })) - throw new Error(errorData.message || `HTTP ${response.status}: ${response.statusText}`) + const text = await response.text().catch(() => "") + let msg = `HTTP ${response.status}: ${response.statusText}` + try { + const e = text ? JSON.parse(text) : {} + msg = e?.error?.message || e?.message || msg + } catch {} + throw new Error(msg) } - - const responseData = await response.json() + if (response.status === 204) return undefined as unknown as T + const raw = await response.text() + const responseData = raw ? JSON.parse(raw) : {} - if (responseData.success && responseData.data !== undefined) { + if (opts.keepEnvelope) return responseData as T + if (responseData.success && responseData.data !== undefined) { return responseData.data as T } if (responseData.success !== undefined) { const { success, ...rest } = responseData return rest as T } return responseData as T }frontend/src/components/workflow/WhatHappensNextUI.tsx (2)
47-49: Read persisted Python code from value.code
Current code reads toolData.value (string/object mismatch). Use value.code.- const [pythonConfig, setPythonConfig] = useState({ - pythonCode: toolData?.value || "", - }) + const [pythonConfig, setPythonConfig] = useState({ + pythonCode: toolData?.value?.code || "", + }) @@ - setPythonConfig({ - pythonCode: toolData?.value || "", - }) + setPythonConfig({ + pythonCode: toolData?.value?.code || "", + })Also applies to: 60-63
69-71: Canonicalize toolType to "python_script"
Avoid dual handling of "python_code" vs "python_script" to prevent drift.- const showPythonConfig = - toolType === "python_code" || toolType === "python_script" + const showPythonConfig = toolType === "python_script"#!/bin/bash # Find remaining "python_code" usages to migrate rg -nP --type=ts --type=tsx '\bpython_code\b' -C2frontend/src/components/workflow/OnFormSubmissionUI.tsx (4)
150-161: Don’t force required=true for every field on mount
Preserve backend/user intent.React.useEffect(() => { - setFormConfig(prev => ({ - ...prev, - fields: prev.fields.map(field => ({ - ...field, - fileTypes: field.type === "file" && (!field.fileTypes || field.fileTypes.length === 0) - ? ["pdf", "doc", "docx", "txt", "jpg", "png"] - : field.fileTypes, - required: true - })) - })) + setFormConfig(prev => ({ + ...prev, + fields: prev.fields.map(field => ({ + ...field, + fileTypes: + field.type === "file" && (!field.fileTypes || field.fileTypes.length === 0) + ? ["pdf", "doc", "docx", "txt", "jpg", "png"] + : field.fileTypes, + })), + })) }, [])
399-406: Missing “Add field” affordance
Users cannot add new fields.Add a button above the list:
<div className="space-y-3"> <Label className="text-sm font-medium text-slate-700 dark:text-gray-300"> Form Elements </Label> + <Button + variant="outline" + size="sm" + onClick={() => + setFormConfig(prev => ({ + ...prev, + fields: [ + ...prev.fields, + { + id: crypto.randomUUID(), + name: `Field ${prev.fields.length + 1}`, + placeholder: "", + type: "text", + required: false, + }, + ], + })) + } + className="mb-2" + > + + Add Field + </Button>
488-569: File config UI lacks upload input (builder can’t attach files)
Expose a file picker or dropzone for preview/testing, or remove Uploaded Files UI to avoid confusion.
173-182: Round‑trip type: map "file" back to backend "upload" on save
Prevents mismatches with server.- type: field.type, + type: field.type === "file" ? "upload" : field.type,
🧹 Nitpick comments (26)
frontend/src/components/workflow/ActionBar.tsx (3)
48-67: Add a11y labels and prevent implicit form submission on icon buttons.Icon-only buttons need accessible names; also add type="button" to avoid unintended submits when nested in a form.
- <button - onClick={handleZoomOut} + <button + type="button" + aria-label="Zoom out" + title="Zoom out" + onClick={handleZoomOut} @@ - <button - onClick={handleZoomIn} + <button + type="button" + aria-label="Zoom in" + title="Zoom in" + onClick={handleZoomIn}
35-45: Execute button: set type and simplify click handling; disable when no handler.Rely on the disabled attribute; avoid conditional onClick and block clicks when onExecute is absent.
- <button - onClick={disabled ? undefined : onExecute} - disabled={disabled} + <button + type="button" + onClick={onExecute} + disabled={disabled || !onExecute} className={`px-4 py-2 border-none rounded-full text-sm font-medium flex items-center gap-1.5 transition-all duration-200 ${ - disabled + disabled || !onExecute ? 'bg-gray-300 text-gray-500 cursor-not-allowed' : 'bg-slate-800 hover:bg-slate-700 text-white cursor-pointer' }`}
1-1: Consider type-only import for React.If using the new JSX transform, prefer
import type React from "react"to avoid a runtime import. Confirm TS config supports it.-import React from "react" +import type React from "react"frontend/src/components/workflow/executedWorkflowRenderer.tsx (4)
146-150: Normalize status values once; compare normalized everywhereAPI returns vary (“Success/Failed/Running”, “completed/failed/running”, “done/in_progress”). Normalize to avoid mis‑styling and logic bugs.
Add helper (top-level, near other utilities):
const normalizeStatus = (s?: string) => { const v = (s || "").toLowerCase() if (v === "success" || v === "done") return "completed" if (v === "in_progress") return "running" if (v === "error") return "failed" return v }Apply samples (repeat pattern file‑wide):
- const hasFailedToolExecution = - tools && tools.some((tool) => (tool as any).status === "failed") - const isFailed = step.status === "failed" || hasFailedToolExecution + const hasFailedToolExecution = + Array.isArray(tools) && tools.some((tool) => normalizeStatus((tool as any).status) === "failed") + const isFailed = normalizeStatus(step.status) === "failed" || hasFailedToolExecution- isActive: isExecution && (step as any).status === "running", - isCompleted: isExecution && (step as any).status === "completed", + isActive: isExecution && normalizeStatus((step as any).status) === "running", + isCompleted: isExecution && normalizeStatus((step as any).status) === "completed",- {step.status === "running" || step.status === "in_progress" + {normalizeStatus(step.status) === "running" ... - : step.status === "completed" || step.status === "done" + : normalizeStatus(step.status) === "completed"- step.status === "completed" + normalizeStatus(step.status) === "completed" ... - step.status === "failed" + normalizeStatus(step.status) === "failed" ... - step.status === "running" + normalizeStatus(step.status) === "running"- const isSuccess = - step.status === "completed" && tool.status === "completed" + const isSuccess = + normalizeStatus(step.status) === "completed" && + normalizeStatus(tool.status) === "completed"- const isFailed = - step.status === "failed" || tool.status === "failed" + const isFailed = + normalizeStatus(step.status) === "failed" || + normalizeStatus(tool.status) === "failed"Also applies to: 2091-2093, 1014-1021, 1254-1264, 1699-1708, 1721-1729, 1759-1761, 1783-1785, 268-277, 340-344, 462-471, 578-589, 791-802, 906-915
286-291: Use both model and ai_model keysBackends differ; prefer model, fall back to ai_model to avoid “using undefined”.
- `AI agent to analyze and summarize documents using ${aiConfig?.model || "gpt-oss-120b"}.`} + `AI agent to analyze and summarize documents using ${aiConfig?.model || aiConfig?.ai_model || "gpt-oss-120b"}.`}
1102-1106: Guard JSON.stringify against cycles; minor robustness for HTML detectionPrevents crashes on cyclic objects and keeps UI responsive.
- const resultString = - typeof result === "object" - ? JSON.stringify(result, null, 2) - : String(result) + let resultString: string + try { + resultString = + typeof result === "object" + ? JSON.stringify(result, null, 2) + : String(result) + } catch { + resultString = String(result) + }Optional: keep current heuristic, or tighten with a cheap startsWith check:
const isHTML = /^\s*<!DOCTYPE html/i.test(resultString) || /^\s*<html[\s>]/i.test(resultString) || /<body[\s>]/i.test(resultString)Also applies to: 1107-1113
2242-2274: Prefer React Flow viewport callbacks over MutationObserver for zoomUse onMove/onMoveEnd to track zoom—simpler and less brittle than DOM observers/listeners.
Example:
- useEffect(() => { - const handleViewportChange = () => { - const viewport = getViewport() - const newZoomLevel = Math.round(viewport.zoom * 100) - setZoomLevel(newZoomLevel) - } - ... - }, [getViewport]) + // In <ReactFlow ... onMove={(_, vp) => setZoomLevel(Math.round(vp.zoom * 100))} />server/api/workflowFileHandler.ts (5)
57-71: Required-field check treats 0/false as “missing”0 is a valid number and unchecked checkboxes may be valid when not required. Tighten emptiness checks to avoid false negatives.
- // Check required fields - if ( - validation.required && - (!value || (typeof value === "string" && value.trim() === "")) - ) { - return { isValid: false, error: `Field '${fieldId}' is required` } - } - - // If field is not required and empty, it's valid - if ( - !validation.required && - (!value || (typeof value === "string" && value.trim() === "")) - ) { - return { isValid: true } - } + const isEmpty = (v: unknown) => + v === undefined || + v === null || + (typeof v === "string" && v.trim() === "") || + (Array.isArray(v) && v.length === 0) + + if (validation.required && isEmpty(value)) { + return { isValid: false, error: `Field '${fieldId}' is required` } + } + if (!validation.required && isEmpty(value)) { + return { isValid: true } + }
115-119: Number validation can be simplifiedThe double negation is redundant; also accept numeric strings explicitly for clarity.
- if (typeof value !== "number" && !!isNaN(Number(value))) { + if (typeof value === "number") { + if (!Number.isFinite(value)) { + return { isValid: false, error: `Field '${fieldId}' must be a number` } + } + } else if (isNaN(Number(value))) { return { isValid: false, error: `Field '${fieldId}' must be a number` } }
137-155: Checkbox “required” semanticsIf a checkbox field is marked required, do we expect it to be checked (true)? Currently false passes earlier “required” check after the refactor unless handled here. Clarify desired behavior and enforce accordingly.
270-276: Use POSIX separators for URLs in relativePathpath.join is OS-dependent. If relativePath is used as a URL, prefer path.posix.join.
- const relativePath = path.join( + const relativePath = path.posix.join( "workflow_uploads", safeExecutionId, safeWorkflowStepId, fileName, )
366-377: Parsing invalid size silently falls back to 10MBFail fast on invalid maxSize config to surface misconfiguration instead of silently defaulting.
- if (!match) { - return 10 * 1024 * 1024 // Default 10MB - } + if (!match) { + throw new Error(`Invalid size string '${sizeStr}'. Expected formats like '10MB', '500KB'.`) + }server/db/schema/workflows.ts (4)
80-92: Add indexes on foreign keys for join performanceFK columns will be queried frequently. Add btree indexes to avoid seq scans in larger datasets.
-import { sql } from "drizzle-orm" +import { sql } from "drizzle-orm" import { uuid, pgTable, text, integer, timestamp, jsonb, pgEnum, customType, + index, } from "drizzle-orm/pg-core" @@ -export const workflowStepTemplate = pgTable("workflow_step_template", { +export const workflowStepTemplate = pgTable("workflow_step_template", { id: uuid("id").notNull().primaryKey().defaultRandom(), workflowTemplateId: uuid("workflow_template_id") .notNull() .references(() => workflowTemplate.id), @@ updatedAt: timestamp("updated_at", { withTimezone: true }) .notNull() .default(sql`NOW()`), // Removed: deletedAt -}) +}, (t) => ({ + idx_step_template_template_id: index("idx_wst_template_id").on(t.workflowTemplateId), +})) @@ -export const workflowStepExecution = pgTable("workflow_step_execution", { +export const workflowStepExecution = pgTable("workflow_step_execution", { id: uuid("id").notNull().primaryKey().defaultRandom(), workflowExecutionId: uuid("workflow_execution_id") .notNull() .references(() => workflowExecution.id), @@ updatedAt: timestamp("updated_at", { withTimezone: true }) .notNull() .default(sql`NOW()`), completedAt: timestamp("completed_at", { withTimezone: true }), // Removed: deletedAt -}) +}, (t) => ({ + idx_wse_exec_id: index("idx_wse_execution_id").on(t.workflowExecutionId), + idx_wse_template_id: index("idx_wse_step_template_id").on(t.workflowStepTemplateId), +})) @@ -export const toolExecution = pgTable("tool_execution", { +export const toolExecution = pgTable("tool_execution", { id: uuid("id").notNull().primaryKey().defaultRandom(), workflowToolId: uuid("workflow_tool_id") // Renamed from toolId .notNull() .references(() => workflowTool.id), - workflowExecutionId: uuid("workflow_execution_id").notNull(), // Renamed from stepId + workflowExecutionId: uuid("workflow_execution_id").notNull().references(() => workflowExecution.id), @@ updatedAt: timestamp("updated_at", { withTimezone: true }) .notNull() .default(sql`NOW()`), -}) +}, (t) => ({ + idx_tool_exec_tool_id: index("idx_tool_exec_tool_id").on(t.workflowToolId), + idx_tool_exec_execution_id: index("idx_tool_exec_execution_id").on(t.workflowExecutionId), +}))Also applies to: 144-158, 172-189
153-154: Step execution uses WorkflowStatus enumIf steps have a narrower lifecycle than templates/executions, consider a dedicated StepStatus to prevent invalid states.
171-179: Missing FK on toolExecution.workflowExecutionIdAdd a foreign key reference to workflowExecution for integrity and better join planning. Patch above includes this.
269-281: Use enums for types in complex template schemaSchemas here use plain strings for step.type and tools[].type. Align with StepType/ToolType to reject invalid inputs early.
- type: z.string(), + type: z.enum(Object.values(StepType) as [string, ...string[]]), @@ - tools: z.array(z.object({ - id: z.string().optional(), - type: z.string(), + tools: z.array(z.object({ + id: z.string().optional(), + type: z.enum(Object.values(ToolType) as [string, ...string[]]), value: z.any().optional(), config: z.record(z.string(), z.any()).optional(), })).optional(),Also applies to: 314-349
frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
65-72: Validate model against fetched list when loading existing config
Ensure existingConfig.model is valid; otherwise fall back. Also revalidate after models load.- model: existingConfig.model, + model: getValidModelId(existingConfig.model),Add:
React.useEffect(() => { setAgentConfig(prev => { const valid = getValidModelId(prev.model) return prev.model === valid ? prev : { ...prev, model: valid } }) }, [models])Also applies to: 78-82
61-62: Likely typo: toolData.val
Use value/config only; “val” isn’t used elsewhere.- existingConfig = toolData.val || toolData.value || toolData.config || {} + existingConfig = toolData.value || toolData.config || {}
100-119: Prefer centralized response extraction
Minor: reuse extractResponseData to avoid ad‑hoc parsing here.frontend/src/components/workflow/api/ApiHandlers.ts (2)
101-101: pagination.totalCount should be a number
Server returns numeric counts.- totalCount: string + totalCount: number
321-371: File upload: guard duplicate document_file, JSON‑stringify objects, add timeout
Prevents duplicate files, preserves object fields, and avoids hanging requests.- Object.entries(executionData.formData).forEach(([key, value]) => { + Object.entries(executionData.formData ?? {}).forEach(([key, value]) => { if (key !== "name" && key !== "description") { if (value instanceof File) { - formData.append("document_file", value) + // only append if not already present + if (!formData.has("document_file")) formData.append("document_file", value) } else { - formData.append(key, String(value)) + formData.append( + key, + typeof value === "string" || typeof value === "number" || typeof value === "boolean" + ? String(value) + : JSON.stringify(value), + ) } } }) @@ - let hasDocumentFile = false - for (const [key] of formData.entries()) { - if (key === "document_file") { - hasDocumentFile = true - break - } - } + const hasDocumentFile = formData.has("document_file") @@ - const response = await fetch( + const ac = new AbortController() + const t = setTimeout(() => ac.abort(), 60_000) + const response = await fetch( `/api/v1/workflow/templates/${templateId}/execute-with-input`, { method: "POST", body: formData, - credentials: "include", // This ensures cookies are sent for authentication + credentials: "include", + signal: ac.signal, } ) + clearTimeout(t) @@ - const responseData = await response.json() - - if (responseData.success && responseData.data !== undefined) { - return responseData.data - } - - if (responseData.success !== undefined) { - const { success, ...rest } = responseData - return rest - } - - return responseData + return extractResponseData<any>(response)frontend/src/components/workflow/WhatHappensNextUI.tsx (1)
111-131: Remove console logs in production paths
Reduce noise; errors are already surfaced.- console.log("Creating visual step with data:", stepData) @@ - console.log("Saving step to API:", stepData) @@ - console.log("Step created successfully:", response) - } catch (error) { - console.error("Failed to save step to API:", error) + } catch (error) { + console.error("Failed to save step to API:", error) }frontend/src/components/workflow/EmailConfigUI.tsx (2)
69-79: Trim and validate emails before adding; prevent duplicates case‑insensitively
Prevents bad entries and dupes.- const handleAddEmail = () => { - if ( - newEmailAddress && - !emailConfig.emailAddresses.includes(newEmailAddress) - ) { + const handleAddEmail = () => { + const raw = newEmailAddress.trim() + const email = raw.toLowerCase() + const isValid = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email) + if (isValid && !emailConfig.emailAddresses.map(e => e.toLowerCase()).includes(email)) { setEmailConfig((prev) => ({ ...prev, - emailAddresses: [...prev.emailAddresses, newEmailAddress], + emailAddresses: [...prev.emailAddresses, email], })) setNewEmailAddress("") } }
111-113: Drop success console log
Keep errors only.- await workflowToolsAPI.updateTool(toolId, updatedToolData) - console.log("Email tool updated successfully") + await workflowToolsAPI.updateTool(toolId, updatedToolData)frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
166-197: Remove console logs from save path
Keep errors; drop noise.- console.log("tool id here",toolId) @@ - const apiResponse = await workflowToolsAPI.updateTool(toolId, updatedToolData) - console.log("tool id here 2",toolId) - console.log("Form tool updated successfully, API response:", apiResponse) + const apiResponse = await workflowToolsAPI.updateTool(toolId, updatedToolData) @@ - console.log("Form configuration saved:", formConfig) onSave?.(formConfig, apiResponse) @@ - console.log("Form configuration saved:", formConfig) onSave?.(formConfig) @@ - console.error("Failed to save form configuration:", error) - + console.error("Failed to save form configuration:", error) onSave?.(formConfig)Also applies to: 200-202, 204-207
147-151: Make newFileType per-field (Record<string,string>) — avoid cross-field interferenceReplace the single shared state const [newFileType, setNewFileType] = useState("") with useState<Record<string,string>>({}) and: use newFileType[field.id] || "" for the input value, onChange -> setNewFileType(prev => ({ ...prev, [field.id]: e.target.value })), have handleFileTypeKeyDown read newFileType[fieldId], and clear only that field's key after addFileType (instead of setNewFileType("")).
File: frontend/src/components/workflow/OnFormSubmissionUI.tsx — declaration: line 147; handleFileTypeKeyDown: line 264; input value/onChange: line 555; addFileType (clear) ~lines 241–268.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
frontend/bun.lockbis excluded by!**/bun.lockbfrontend/src/assets/Document.svgis excluded by!**/*.svgfrontend/src/assets/android.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
frontend/src/components/Sidebar.tsx(1 hunks)frontend/src/components/ui/dropdown.tsx(1 hunks)frontend/src/components/workflow/AIAgentConfigUI.tsx(2 hunks)frontend/src/components/workflow/ActionBar.tsx(1 hunks)frontend/src/components/workflow/Default.ts(1 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(3 hunks)frontend/src/components/workflow/OnFormSubmissionUI.tsx(1 hunks)frontend/src/components/workflow/TemplateCard.tsx(1 hunks)frontend/src/components/workflow/TemplateSelectionModal.tsx(1 hunks)frontend/src/components/workflow/Types.ts(3 hunks)frontend/src/components/workflow/WhatHappensNextUI.tsx(1 hunks)frontend/src/components/workflow/WorkflowCard.tsx(1 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(10 hunks)frontend/src/components/workflow/WorkflowExecutionsTable.tsx(1 hunks)frontend/src/components/workflow/WorkflowIcons.tsx(1 hunks)frontend/src/components/workflow/WorkflowProvider.tsx(1 hunks)frontend/src/components/workflow/WorkflowUtils.ts(6 hunks)frontend/src/components/workflow/api/ApiHandlers.ts(1 hunks)frontend/src/components/workflow/executedWorkflowRenderer.tsx(1 hunks)frontend/src/routes/_authenticated/ChatMessage.test.tsx(8 hunks)frontend/src/routes/_authenticated/workflow.tsx(11 hunks)server/ai/modelConfig.ts(1 hunks)server/api/chat/agents.ts(1 hunks)server/api/chat/jaf-xynemodel-provider.ts(2 hunks)server/api/testEmail.ts(1 hunks)server/api/workflowFileHandler.ts(1 hunks)server/db/schema/index.ts(1 hunks)server/db/schema/workflows.ts(1 hunks)server/db/workflow.ts(1 hunks)server/db/workflowTool.ts(1 hunks)server/integrations/google/index.ts(4 hunks)server/integrations/google/utils.ts(2 hunks)server/server.ts(3 hunks)server/services/emailService.ts(3 hunks)server/types.ts(1 hunks)server/types/workflowTypes.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/src/components/workflow/Default.ts
- frontend/src/components/workflow/WorkflowUtils.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- server/db/schema/index.ts
- frontend/src/components/Sidebar.tsx
- server/api/chat/agents.ts
- frontend/src/components/workflow/WorkflowProvider.tsx
- server/integrations/google/utils.ts
- server/db/workflow.ts
- frontend/src/components/workflow/TemplateSelectionModal.tsx
- server/db/workflowTool.ts
- server/types/workflowTypes.ts
- frontend/src/components/workflow/Types.ts
- frontend/src/routes/_authenticated/ChatMessage.test.tsx
- server/ai/modelConfig.ts
- frontend/src/components/workflow/TemplateCard.tsx
- server/api/testEmail.ts
- frontend/src/routes/_authenticated/workflow.tsx
- frontend/src/components/workflow/WorkflowExecutionsTable.tsx
- server/api/chat/jaf-xynemodel-provider.ts
- server/types.ts
- server/integrations/google/index.ts
- frontend/src/components/workflow/WorkflowExecutionModal.tsx
- frontend/src/components/workflow/WorkflowCard.tsx
- frontend/src/components/workflow/WorkflowIcons.tsx
- frontend/src/components/ui/dropdown.tsx
- server/services/emailService.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-04T09:29:29.879Z
Learnt from: devesh-juspay
PR: xynehq/xyne#758
File: server/db/schema/workflowServiceConfig.ts:51-53
Timestamp: 2025-09-04T09:29:29.879Z
Learning: In the workflowServiceConfig.ts schema, the user prefers to keep currentWorkflowExeId as a simple uuid reference without adding foreign key constraints or onDelete behaviors to the workflowExe table.
Applied to files:
server/db/schema/workflows.ts
📚 Learning: 2025-09-02T16:41:31.729Z
Learnt from: devesh-juspay
PR: xynehq/xyne#758
File: server/db/schema/workflowTools.ts:9-14
Timestamp: 2025-09-02T16:41:31.729Z
Learning: In the workflowTools.ts schema, the user prefers to add notNull() constraint to the type field but does not want onDelete cascade behavior for the workflowTemplateId foreign key reference.
Applied to files:
server/db/schema/workflows.ts
📚 Learning: 2025-09-12T13:28:43.742Z
Learnt from: devesh-juspay
PR: xynehq/xyne#821
File: server/server.ts:771-837
Timestamp: 2025-09-12T13:28:43.742Z
Learning: All workflow API endpoints should be protected with AuthMiddleware to prevent unauthorized access to template creation, execution, tool management, and other sensitive workflow operations.
Applied to files:
server/server.ts
🧬 Code graph analysis (8)
frontend/src/components/workflow/executedWorkflowRenderer.tsx (1)
frontend/src/components/workflow/Types.ts (5)
Step(64-92)Tool(56-61)Flow(160-165)TemplateFlow(95-99)UserDetail(1-6)
frontend/src/components/workflow/EmailConfigUI.tsx (1)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)frontend/src/components/workflow/WorkflowIcons.tsx (2)
BackArrowIcon(393-413)CloseIcon(416-436)
server/server.ts (2)
server/db/schema/workflows.ts (8)
createWorkflowTemplateSchema(270-275)createComplexWorkflowTemplateSchema(314-370)updateWorkflowTemplateSchema(310-311)addStepToWorkflowSchema(395-406)createWorkflowExecutionSchema(372-377)createWorkflowToolSchema(277-281)updateWorkflowToolSchema(283-288)updateWorkflowStepExecutionSchema(382-388)server/api/workflow.ts (31)
createWorkflowTemplateSchema(50-50)CreateWorkflowTemplateApi(2198-2224)createComplexWorkflowTemplateSchema(51-51)CreateComplexWorkflowTemplateApi(2227-2446)ListWorkflowTemplatesApi(84-144)GetWorkflowTemplateApi(147-188)updateWorkflowTemplateSchema(52-52)UpdateWorkflowTemplateApi(2452-2479)ExecuteTemplateApi(2449-2449)ExecuteWorkflowWithInputApi(191-561)AddStepToWorkflowApi(2786-2917)createWorkflowExecutionSchema(53-53)CreateWorkflowExecutionApi(2482-2508)listWorkflowExecutionsQuerySchema(61-61)ListWorkflowExecutionsApi(2511-2611)GetWorkflowExecutionApi(1199-1251)GetWorkflowExecutionStatusApi(1170-1196)createWorkflowToolSchema(55-55)CreateWorkflowToolApi(2614-2638)ListWorkflowToolsApi(2179-2193)GetWorkflowToolApi(2727-2752)UpdateWorkflowToolApi(2641-2724)DeleteWorkflowToolApi(2755-2783)DeleteWorkflowStepTemplateApi(2936-3106)updateWorkflowStepExecutionSchema(56-56)UpdateWorkflowStepExecutionApi(3109-3135)CompleteWorkflowStepExecutionApi(3138-3162)GetFormDefinitionApi(3168-3222)SubmitFormStepApi(3165-3165)ServeWorkflowFileApi(3257-3271)GetGeminiModelEnumsApi(3225-3254)
frontend/src/components/workflow/WhatHappensNextUI.tsx (4)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowStepsAPI(410-469)frontend/src/components/ui/label.tsx (1)
Label(24-24)frontend/src/components/ui/textarea.tsx (1)
Textarea(24-24)frontend/src/components/ui/button.tsx (1)
Button(57-57)
frontend/src/components/workflow/api/ApiHandlers.ts (2)
frontend/src/components/workflow/Types.ts (3)
WorkflowTemplate(227-248)TemplateFlow(95-99)Flow(160-165)frontend/src/api.ts (1)
api(4-4)
frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
frontend/src/api.ts (1)
api(4-4)frontend/src/hooks/useChatStream.ts (1)
createAuthEventSource(212-243)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)
server/api/workflowFileHandler.ts (1)
server/logger/index.ts (2)
getLogger(36-93)Subsystem(15-15)
🪛 ast-grep (0.38.6)
server/api/workflowFileHandler.ts
[warning] 91-91: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(validation.pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (6)
frontend/src/components/workflow/ActionBar.tsx (1)
5-8: Props/defaults look good.Adding disabled?: boolean with a sensible default and wiring it to UI states is clean.
Also applies to: 11-16
server/db/schema/workflows.ts (3)
51-58: Enums centralized via types/workflowTypes — goodSingle source of truth prevents migration drift. Nice.
390-393: What does formSubmissionSchema.stepId reference?Confirm whether this is a workflowStepExecution ID or a template step ID. Name and validations should reflect it to avoid confusion.
59-76: Defaults and removed FKs match prior preferencesNo onDelete cascades and keeping root IDs as simple UUIDs are consistent with past direction.
Also applies to: 119-141
server/server.ts (2)
885-952: Workflow routes are now behind Auth — goodRoutes are registered after
.use("*", AuthMiddleware). Security concern addressed.Please confirm no duplicate registration of these routes elsewhere in the file.
1480-1514: Health check endpoints restoredThanks for adding /health, /health/postgres, /health/vespa back in. Matches ops needs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (3)
175-185: Round‑trip type mismatch: persist "upload" (not "file") and use backend key "filetypes".Current save payload writes type: "file" and key fileTypes; backend commonly expects "upload" and "filetypes" (you already normalize on load). This will break updates.
fields: formConfig.fields.map((field) => ({ id: field.id, label: field.name, - type: field.type, + type: field.type === "file" ? "upload" : field.type, required: field.required, placeholder: field.placeholder, - fileTypes: field.fileTypes, + filetypes: field.fileTypes, // backend key options: field.options, maxSize: field.maxSize, })),#!/bin/bash # Check for backend expectations of "upload" and "filetypes" rg -nC2 -i '\bupload\b|\bfiletypes\b' --type ts --type tsx
151-163: Don’t force required=true for every field on mount.This overwrites user/backend intent and breaks loading optional fields.
React.useEffect(() => { setFormConfig(prev => ({ ...prev, fields: prev.fields.map(field => ({ ...field, fileTypes: field.type === "file" && (!field.fileTypes || field.fileTypes.length === 0) ? ["pdf", "doc", "docx", "txt", "jpg", "png"] : field.fileTypes, - required: true + // preserve existing required; don't override + required: field.required })) })) }, [])
487-571: File field UX incomplete: add an upload input; wire to state.Users can’t add files—only see/remove. Add an input with accept synthesized from fileTypes and handle onChange.
@@ - {field.type === "file" ? ( + {field.type === "file" ? ( <div className="space-y-4"> + {/* Upload Input */} + <div className="space-y-2"> + <Label className="text-sm font-medium text-slate-700 dark:text-gray-300"> + Upload Files + </Label> + <input + type="file" + multiple + accept={((getFieldById(field.id)?.fileTypes || []).map(t => '.' + t)).join(',')} + onChange={(e) => handleFileInputChange(field.id, e.target.files)} + className="block w-full text-sm text-slate-700 dark:text-gray-300" + /> + </div> @@ const removeFile = (fieldId: string, fileIndex: number) => { @@ } + + const handleFileInputChange = (fieldId: string, files: FileList | null) => { + if (!files) return + setUploadedFiles((prev) => ({ + ...prev, + [fieldId]: [...(prev[fieldId] || []), ...Array.from(files)], + })) + }Also applies to: 236-241
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
252-255: Don’t override SSE onerror; track/close EventSource via ref; clean up on unmount.Overriding onerror breaks createAuthEventSource’s refresh logic and risks dangling streams. Use a ref, attach listeners, and close in all paths + on unmount.
+ // Track SSE instance for cleanup + const eventSourceRef = React.useRef<EventSource | null>(null) @@ - let eventSource: EventSource | null = null + let eventSource: EventSource | null = null let generatedPrompt = "" @@ - eventSource = await createAuthEventSource(url.toString()) + eventSource = await createAuthEventSource(url.toString()) + eventSourceRef.current = eventSource @@ - if (!eventSource) { + if (!eventSourceRef.current) { reject(new Error("EventSource not created")) return } @@ - eventSource.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { generatedPrompt += event.data }) @@ - eventSource.addEventListener(ChatSSEvents.End, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.End, (event) => { try { const data = JSON.parse(event.data) const finalPrompt = data.fullPrompt || generatedPrompt if (finalPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(finalPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() resolve(finalPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } catch (parseError) { console.warn("Could not parse end event data:", parseError) if (generatedPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(generatedPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() resolve(generatedPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("No enhanced prompt received from API")) } } }) @@ - eventSource.addEventListener(ChatSSEvents.Error, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.Error, (event) => { try { const data = JSON.parse(event.data) - eventSource?.close() + eventSourceRef.current?.close() reject(new Error(data.error || "Error in prompt generation")) } catch (parseError) { - eventSource?.close() + eventSourceRef.current?.close() reject(new Error("Error in prompt generation")) } }) @@ - eventSource.onerror = () => { - eventSource?.close() - reject(new Error("Connection error during prompt generation")) - } + // Optional observer; don't override onerror to preserve token-refresh behavior + eventSourceRef.current.addEventListener("error", () => { + // Let createAuthEventSource handle refresh; if we see error here, close this stream + eventSourceRef.current?.close() + reject(new Error("Connection error during prompt generation")) + }) })Add once in the component:
React.useEffect(() => { return () => { try { eventSourceRef.current?.close() } catch {} } }, [])Also applies to: 185-201, 203-251
🧹 Nitpick comments (10)
frontend/src/components/workflow/EmailConfigUI.tsx (3)
71-82: Validate and normalize emails; prevent Enter default; use input type="email".
- Trim/lowercase before storing; reject invalid addresses; dedupe case-insensitively.
- Prevent default on Enter to avoid accidental form submit behavior.
- Use a semantic email input for basic browser validation.
const handleAddEmail = () => { - if ( - newEmailAddress && - !emailConfig.emailAddresses.includes(newEmailAddress) - ) { - setEmailConfig((prev) => ({ - ...prev, - emailAddresses: [...prev.emailAddresses, newEmailAddress], - })) - setNewEmailAddress("") - } + const addr = newEmailAddress.trim().toLowerCase() + if (!addr) return + if (!isValidEmail(addr)) return + if (!emailConfig.emailAddresses.map(a => a.toLowerCase()).includes(addr)) { + setEmailConfig((prev) => ({ + ...prev, + emailAddresses: [...prev.emailAddresses, addr], + })) + setNewEmailAddress("") + } } @@ - const handleKeyDown = (e: React.KeyboardEvent) => { - if (e.key === "Enter") { - handleAddEmail() - } - } + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === "Enter") { + e.preventDefault() + handleAddEmail() + } + } @@ - <Input + <Input + type="email" id="add-email" value={newEmailAddress} onChange={(e) => setNewEmailAddress(e.target.value)} onKeyDown={handleKeyDown} placeholder="type email address" className="w-full pr-16 dark:bg-gray-800 dark:text-gray-300 dark:border-gray-600" />Add once (helper), e.g., above the component:
const isValidEmail = (e: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(e)Also applies to: 93-97, 229-236
56-59: Coerce loaded to_email to array to avoid type mismatch.to_email may arrive as a string; ensure emailAddresses is always string[].
- emailAddresses: existingConfig.emailAddresses || existingConfig.to_email || [], + emailAddresses: Array.isArray(existingConfig.emailAddresses) + ? existingConfig.emailAddresses + : Array.isArray(existingConfig.to_email) + ? existingConfig.to_email + : existingConfig.to_email + ? [String(existingConfig.to_email)] + : [],
249-261: Harden avatar color/initial derivation for empty or malformed emails.Avoid NaN index and blank initials.
- const colorIndex = email.charCodeAt(0) % avatarColors.length - const avatarColor = avatarColors[colorIndex] + const firstChar = (email?.trim()?.charAt(0) || "E").toUpperCase() + const code = firstChar.charCodeAt(0) || 0 + const colorIndex = code % avatarColors.length + const avatarColor = avatarColors[colorIndex] @@ - const firstLetter = email.charAt(0).toUpperCase() + const firstLetter = firstCharAlso applies to: 263-274
frontend/src/components/workflow/OnFormSubmissionUI.tsx (3)
401-407: Add “Add field” control to make the builder useful.Currently you can only edit/remove existing fields.
@@ - <div className="space-y-3"> + <div className="space-y-3"> + <div className="flex justify-end mb-2"> + <Button + variant="outline" + size="sm" + onClick={addField} + className="rounded-md" + > + + Add Field + </Button> + </div> {formConfig.fields.map((field) => ( @@ <div className="pt-6 px-0"> <Button onClick={handleSave} className="w-full bg-gray-900 hover:bg-gray-900 text-white rounded-full shadow-none" > Save Configuration </Button> </div> @@ const removeField = (fieldId: string) => { @@ } + + const addField = () => { + const id = (globalThis.crypto?.randomUUID?.() as string) || `${Date.now()}-${Math.random().toString(16).slice(2)}` + setFormConfig((prev) => ({ + ...prev, + fields: [ + ...prev.fields, + { id, name: `Field ${prev.fields.length + 1}`, placeholder: "", type: "text", required: false }, + ], + })) + }Also applies to: 626-636, 221-232
74-75: Harden UUID generation for broader runtimes.crypto.randomUUID may be unavailable in some environments; add a fallback.
- const initialFieldId = crypto.randomUUID() + const initialFieldId = + (globalThis.crypto?.randomUUID?.() as string) || + `${Date.now()}-${Math.random().toString(16).slice(2)}`
168-169: Remove debug logs before merge.Avoid noisy console logs in shipped UI.
- console.log("tool id here",toolId) @@ - const apiResponse = await workflowToolsAPI.updateTool(toolId, updatedToolData) - console.log("tool id here 2",toolId) - console.log("Form tool updated successfully, API response:", apiResponse) - - - console.log("Form configuration saved:", formConfig) + const apiResponse = await workflowToolsAPI.updateTool(toolId, updatedToolData) + // Optionally surface success via toast/snackbarAlso applies to: 193-199
frontend/src/components/workflow/AIAgentConfigUI.tsx (4)
62-64: Probable typo: toolData.val → toolData.value.val isn’t used elsewhere; keep value/config only.
- } else if (toolData) { - - existingConfig = toolData.val || toolData.value || toolData.config || {} + } else if (toolData) { + existingConfig = toolData.value || toolData.config || {}
68-74: Ensure model is valid when loading existing config.Use getValidModelId to avoid undefined or unsupported IDs.
- model: existingConfig.model, + model: getValidModelId(existingConfig.model),
98-126: Revalidate selected model after models load.If models arrive after initial render, normalize the current selection.
React.useEffect(() => { if (isVisible && !modelsLoaded) { @@ fetchGeminiModels() } }, [isVisible, modelsLoaded]) + + // When models list updates, ensure the current model is valid + React.useEffect(() => { + setAgentConfig((prev) => ({ + ...prev, + model: getValidModelId(prev.model), + })) + }, [models])Also applies to: 131-134
153-157: Avoid alert(); prefer inline helper or toast for better UX.Non-blocking, but alerts are jarring.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/workflow/AIAgentConfigUI.tsx(2 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(3 hunks)frontend/src/components/workflow/OnFormSubmissionUI.tsx(1 hunks)frontend/src/components/workflow/executedWorkflowRenderer.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/workflow/executedWorkflowRenderer.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
frontend/src/api.ts (1)
api(4-4)frontend/src/hooks/useChatStream.ts (1)
createAuthEventSource(212-243)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)frontend/src/components/workflow/WorkflowIcons.tsx (2)
BackArrowIcon(393-413)CloseIcon(416-436)
frontend/src/components/workflow/EmailConfigUI.tsx (1)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (3)
152-163: ****This is still forcing
required: trueon every field during mount, which overrides backend/user intent and makes it impossible to load initially optional fields correctly.
175-184: ****Round-trip type mismatch: map "file" back to backend's "upload" on save. You normalize incoming
"upload"to"file"but write back"file". This could break updates when the backend expects "upload".
403-408: ****The form builder UI allows for configuring existing fields, but it's missing a button or mechanism to add new fields. This limits the form to only the fields defined in its initial state.
🧹 Nitpick comments (4)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (4)
168-169: Remove debugging console.log statementsThese console.log statements should be removed before production deployment.
- console.log("tool id here",toolId) - console.log("tool id here 2",toolId)Also applies to: 194-195
196-199: Remove debugging console.log statementsAdditional console.log statements that should be cleaned up for production.
- console.log("Form tool updated successfully, API response:", apiResponse) - console.log("Form configuration saved:", formConfig) - console.log("Form configuration saved:", formConfig)Also applies to: 202-203, 208-208
243-253: Handle edge case: empty string input for file typeThe function correctly validates file types, but should handle the case where a user enters just whitespace or an empty string more explicitly.
const addFileType = (fieldId: string, fileType: string) => { const normalized = normalizeFileType(fileType.trim()) const currentFileTypes = getFieldById(fieldId)?.fileTypes || [] - if (normalized && isValidFileType(normalized) && !currentFileTypes.includes(normalized)) { + if (normalized && normalized.length > 0 && isValidFileType(normalized) && !currentFileTypes.includes(normalized)) { updateField(fieldId, { fileTypes: [...currentFileTypes, normalized] }) setNewFileType("") + } else if (normalized.length === 0) { + setNewFileType("") } }
290-294: Consider using CSS transitions for smoother panel animationThe panel currently uses Tailwind classes for the slide animation. Consider adding CSS transitions to make the animation smoother.
<div className={`fixed top-[80px] right-0 h-[calc(100vh-80px)] bg-white dark:bg-gray-900 border-l border-slate-200 dark:border-gray-700 flex flex-col overflow-hidden z-50 ${ isVisible ? "translate-x-0 w-[380px]" : "translate-x-full w-0" - }`} + } transition-all duration-300 ease-in-out`} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/workflow/AIAgentConfigUI.tsx(2 hunks)frontend/src/components/workflow/EmailConfigUI.tsx(3 hunks)frontend/src/components/workflow/OnFormSubmissionUI.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/workflow/EmailConfigUI.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/workflow/OnFormSubmissionUI.tsx (2)
frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)frontend/src/components/workflow/WorkflowIcons.tsx (1)
BackArrowIcon(393-413)
frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
frontend/src/api.ts (1)
api(4-4)frontend/src/hooks/useChatStream.ts (1)
createAuthEventSource(212-243)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)
🔇 Additional comments (9)
frontend/src/components/workflow/AIAgentConfigUI.tsx (5)
129-149: LGTM! Clean system prompt display/storage separation.The hidden append text mechanism and helper functions provide a clean way to separate what users see from what's actually stored, maintaining backward compatibility while improving UX.
98-126: LGTM! Dynamic model loading with proper error handling.The dynamic Gemini model fetching is well-implemented with appropriate loading states and fallback to defaults on failure.
296-331: LGTM! Robust save workflow with proper error handling.The async save implementation properly handles both API updates and callback invocation, with appropriate error recovery to ensure the save flow continues even if the API call fails.
54-89: Configuration loading logic is thorough and well-structured.The effect properly handles multiple data sources (stepData, toolData) with appropriate fallbacks and model validation.
12-22: Well-designed interface expansion for enhanced flexibility.The new optional props provide good flexibility for different usage contexts while maintaining backward compatibility.
frontend/src/components/workflow/OnFormSubmissionUI.tsx (4)
465-487: LGTM: Well-structured dropdown implementationThe dropdown component integration looks good with proper value handling, comprehensive type options, and appropriate styling props.
537-566: Excellent file type management UXThe pill-based file type editor with inline input provides an intuitive user experience. The implementation correctly handles adding/removing file types and provides good visual feedback.
612-621: Good UX: Prevent removing the last fieldThe conditional rendering ensures users can't accidentally remove all fields, which maintains form integrity.
630-638: Verify Save Configuration button behavior across builder and non-builder modesEnsure the Save button calls handleSave in both builder and non-builder flows, that handleSave disables the button and shows a loading state while the request is in-flight, and that API failures are caught and surfaced to the user with a clear error message and a retry path. Check frontend/src/components/workflow/OnFormSubmissionUI.tsx (≈lines 630–638) and the handleSave implementation; search the repo for other consumers of OnFormSubmissionUI to confirm props like builder/toolId are passed consistently.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
frontend/src/components/workflow/WorkflowCard.tsx (1)
27-30: Use template‑specific icon with fallback instead of a hardcoded bot image.Read from workflow.rootStep?.metadata?.icon and fall back to botLogo.
- const getTemplateIcon = () => { - // Always use bot-logo.svg for workflow cards - return <img src={botLogo} alt="Bot Logo" className="w-5 h-5" /> - } + const getTemplateIcon = () => { + const icon = workflow.rootStep?.metadata?.icon + return ( + <img + src={icon ?? botLogo} + alt={`${workflow.name} icon`} + className="w-5 h-5" + /> + ) + }Also applies to: 39-39
frontend/src/components/workflow/executedWorkflowRenderer.tsx (4)
134-136: Don’t assume the tool is at index 0; scan the array.Fixes missed detections when the target tool isn’t first.
- const hasAIAgentTool = - tools && tools.length > 0 && tools[0].type === "ai_agent" + const hasAIAgentTool = + Array.isArray(tools) && tools.some((t) => t?.type === "ai_agent") @@ - const hasEmailTool = tools && tools.length > 0 && tools[0].type === "email" + const hasEmailTool = + Array.isArray(tools) && tools.some((t) => t?.type === "email") @@ - const hasFormTool = tools && tools.length > 0 && tools[0].type === "form" + const hasFormTool = + Array.isArray(tools) && tools.some((t) => t?.type === "form") @@ - const hasPythonScriptTool = - tools && tools.length > 0 && tools[0].type === "python_script" + const hasPythonScriptTool = + Array.isArray(tools) && tools.some((t) => t?.type === "python_script")Optional: when deriving config, prefer tools.find(t => t.type === "...")?.config over tools?.[0]?.config.
Also applies to: 321-321, 515-517, 728-731
1653-1654: Remove dead pollingInterval state and cleanup effect.It’s never set; also avoids NodeJS.Timeout type in browser code.
- // Cleanup polling on component unmount - const [pollingInterval] = useState<NodeJS.Timeout | null>(null) @@ - // Cleanup polling on component unmount - useEffect(() => { - return () => { - if (pollingInterval) { - clearInterval(pollingInterval) - } - } - }, [pollingInterval]) + // (removed unused polling cleanup)Also applies to: 1985-1991
1012-1021: Normalize status strings once; compare on canonical values.Covers “Success/Failed/Running” vs “completed/failed/running”.
+// helper near top-level (after imports) +const normalizeStatus = (s?: string) => + (s || "").toLowerCase().replace("success", "completed") @@ - {step.status && ( + {step.status && ( <div className="text-xs opacity-70 uppercase tracking-wider font-medium mb-1"> - {step.status === "running" || step.status === "in_progress" + {normalizeStatus(step.status) === "running" || + normalizeStatus(step.status) === "in_progress" ? "In Progress" - : step.status === "completed" || step.status === "done" + : normalizeStatus(step.status) === "completed" || + normalizeStatus(step.status) === "done" ? "Completed" - : step.status === "pending" + : normalizeStatus(step.status) === "pending" ? "Pending" : step.status} </div> )} @@ - const isFailed = step.status === "failed" || hasFailedToolExecution + const isFailed = + normalizeStatus(step.status) === "failed" || hasFailedToolExecution @@ - const isFailed = step.status === "failed" || hasFailedToolExecution + const isFailed = + normalizeStatus(step.status) === "failed" || hasFailedToolExecution @@ - const isFailed = step.status === "failed" || hasFailedToolExecution + const isFailed = + normalizeStatus(step.status) === "failed" || hasFailedToolExecution @@ - const isFailed = step.status === "failed" || hasFailedToolExecution + const isFailed = + normalizeStatus(step.status) === "failed" || hasFailedToolExecution @@ - step.status === "completed" + normalizeStatus(step.status) === "completed" ? "bg-green-100 text-green-700" - : step.status === "failed" + : normalizeStatus(step.status) === "failed" ? "bg-red-100 text-red-700" - : step.status === "running" + : normalizeStatus(step.status) === "running" ? "bg-blue-100 text-blue-700" : "bg-gray-100 text-gray-600" @@ - isActive: isExecution && (step as any).status === "running", - isCompleted: isExecution && (step as any).status === "completed", + isActive: + isExecution && + normalizeStatus((step as any).status) === "running", + isCompleted: + isExecution && + normalizeStatus((step as any).status) === "completed",Apply the same normalizeStatus usage in any remaining status comparisons in this file.
Also applies to: 147-150, 343-344, 521-522, 735-736, 1216-1224, 1805-1807, 1806-1807
1843-1846: Remove unsupported Edge.pathOptions; fixes TS errors and no-op props.pathOptions isn’t a valid Edge prop for SmoothStep; keep styling in style/markerEnd.
style: { stroke: "#D1D5DB", strokeWidth: 2, strokeLinecap: "round", strokeLinejoin: "round", }, - pathOptions: { - borderRadius: 20, - offset: 20, - }, markerEnd: { type: "arrowclosed", color: "#D1D5DB", },const newEdge = { ...params, id: `${params.source}-${params.target}`, type: "smoothstep", animated: false, style: { stroke: "#6B7280", strokeWidth: 2, strokeLinecap: "round", strokeLinejoin: "round", }, - pathOptions: { - borderRadius: 20, - offset: 20, - }, markerEnd: { type: "arrowclosed", color: "#6B7280", }, }Also applies to: 1880-1883
frontend/src/components/workflow/AIAgentConfigUI.tsx (1)
185-255: SSE cleanup regression: track EventSource in a ref, close on unmount, and guard against hangsCurrent code recreates the local
eventSourceand never closes it on component unmount; a mid‑stream unmount will leak the connection. Add a ref + unmount cleanup and replace all local usages with the ref. Also add a hard timeout so the stream can’t hang indefinitely.Apply:
@@ const [isModelDropdownOpen, setIsModelDropdownOpen] = useState(false) const [isEnhancingPrompt, setIsEnhancingPrompt] = useState(false) + // Track SSE instance for cleanup + const eventSourceRef = React.useRef<EventSource | null>(null) @@ - }, [isVisible, modelsLoaded]) + }, [isVisible, modelsLoaded]) + + // Close SSE on unmount + React.useEffect(() => { + return () => { + try { eventSourceRef.current?.close() } catch {} + } + }, []) @@ - let eventSource: EventSource | null = null let generatedPrompt = "" @@ - eventSource = await createAuthEventSource(url.toString()) + eventSourceRef.current = await createAuthEventSource(url.toString()) @@ - await new Promise((resolve, reject) => { - if (!eventSource) { + await new Promise((resolve, reject) => { + if (!eventSourceRef.current) { reject(new Error("EventSource not created")) return } + const timeoutId = window.setTimeout(() => { + try { eventSourceRef.current?.close() } catch {} + reject(new Error("Timed out waiting for prompt generation")) + }, 120000) // 2 min @@ - eventSource.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.ResponseUpdate, (event) => { generatedPrompt += event.data }) @@ - eventSource.addEventListener(ChatSSEvents.End, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.End, (event) => { try { const data = JSON.parse(event.data) const finalPrompt = data.fullPrompt || generatedPrompt if (finalPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(finalPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() + clearTimeout(timeoutId) resolve(finalPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() + clearTimeout(timeoutId) reject(new Error("No enhanced prompt received from API")) } } catch (parseError) { console.warn("Could not parse end event data:", parseError) if (generatedPrompt.trim()) { setAgentConfig((prev) => ({ ...prev, systemPrompt: getFullSystemPrompt(generatedPrompt.trim()), })) setIsEnhancingPrompt(false) - eventSource?.close() + eventSourceRef.current?.close() + clearTimeout(timeoutId) resolve(generatedPrompt) } else { - eventSource?.close() + eventSourceRef.current?.close() + clearTimeout(timeoutId) reject(new Error("No enhanced prompt received from API")) } } }) @@ - eventSource.addEventListener(ChatSSEvents.Error, (event) => { + eventSourceRef.current.addEventListener(ChatSSEvents.Error, (event) => { try { const data = JSON.parse(event.data) - eventSource?.close() + eventSourceRef.current?.close() + clearTimeout(timeoutId) reject(new Error(data.error || "Error in prompt generation")) } catch (parseError) { - eventSource?.close() + eventSourceRef.current?.close() + clearTimeout(timeoutId) reject(new Error("Error in prompt generation")) } }) @@ - eventSource.addEventListener("error", () => { - eventSource?.close() + eventSourceRef.current.addEventListener("error", () => { + eventSourceRef.current?.close() + clearTimeout(timeoutId) reject(new Error("Connection error during prompt generation")) }) })Also applies to: 91-93, 126-126
🧹 Nitpick comments (9)
frontend/src/components/workflow/WorkflowCard.tsx (1)
10-25: Harden date formatting; handle missing/invalid dates.Prevents “Invalid Date” and gracefully falls back.
- const formatDate = (dateString: string) => { - const date = new Date(dateString) - return ( - date.toLocaleDateString("en-US", { - month: "2-digit", - day: "2-digit", - year: "numeric", - }) + - " " + - date.toLocaleTimeString("en-US", { - hour: "2-digit", - minute: "2-digit", - hour12: false, - }) - ) - } + const formatDate = (dateString?: string) => { + if (!dateString) return "—" + const date = new Date(dateString) + if (Number.isNaN(date.getTime())) return "—" + return date.toLocaleString("en-US", { hour12: false }) + } @@ - Edited at {formatDate(workflow.updatedAt)} + Edited at {formatDate(workflow.updatedAt ?? workflow.createdAt)}Also applies to: 47-49
frontend/src/components/workflow/Types.ts (1)
56-61: Align Tool shape with other “value” usages.Rename val → value for consistency with WorkflowTemplate.rootStep.tool.value.
export type Tool = { id: string config: ToolConfig type: string - val: number | ToolValue + value: number | ToolValue }Note: Update any call sites expecting .val.
frontend/src/components/workflow/executedWorkflowRenderer.tsx (1)
31-116: Deduplicate ExecutionWorkflowData; reuse shared types.Move this interface to ./Types (or compose from existing StepExecution/ToolExecution/TemplateFlow) to avoid drift between renderer and modal/API.
frontend/src/components/workflow/AIAgentConfigUI.tsx (6)
66-75: Normalize rehydrated model to a valid optionWhen rehydrating,
existingConfig.modelmay be undefined or not inmodels. UsegetValidModelIdto avoid saving/displaying an invalid value.- model: existingConfig.model, + model: getValidModelId(existingConfig.model),
296-303: Normalize model on save and prevent saving during enhancementAvoid persisting an invalid model; also disable Save while streaming to prevent racey writes.
- const configToSave = { - ...agentConfig, - description: agentConfig.description === "some agent description" ? "" : agentConfig.description - } + const normalizedModel = getValidModelId(agentConfig.model) + const configToSave = { + ...agentConfig, + model: normalizedModel, + description: agentConfig.description === "some agent description" ? "" : agentConfig.description, + }- <Button + <Button onClick={handleSave} + disabled={isEnhancingPrompt} className="w-full bg-gray-900 hover:bg-gray-800 dark:bg-gray-700 dark:hover:bg-gray-600 text-white rounded-full" >Also applies to: 530-534
99-127: Cancel model fetch when unmounting or drawer hidesGuard against setting state on an unmounted component and avoid wasted work if visibility changes mid‑fetch.
- React.useEffect(() => { + React.useEffect(() => { if (isVisible && !modelsLoaded) { - const fetchGeminiModels = async () => { + let cancelled = false + const fetchGeminiModels = async () => { setIsLoadingModels(true) try { - const response = await api.workflow.models.gemini.$get() + const response = await api.workflow.models.gemini.$get() if (response.ok) { const data = await response.json() if (data.success && data.data && Array.isArray(data.data)) { - const enumValues = data.data.map((model: any) => model.enumValue) - setModels(enumValues) - setModelsLoaded(true) + if (!cancelled) { + const enumValues = data.data.map((model: any) => model.enumValue) + setModels(enumValues) + setModelsLoaded(true) + } } } else { console.warn('Failed to fetch Gemini models from API, using defaults') } } catch (error) { console.warn('Error fetching Gemini models:', error) } finally { - setIsLoadingModels(false) + if (!cancelled) setIsLoadingModels(false) } } fetchGeminiModels() } - }, [isVisible, modelsLoaded]) + return () => { cancelled = true } + }, [isVisible, modelsLoaded])
352-366: Add accessible labels to icon‑only controlsProvide
aria-labelfor Back/Close buttons so screen readers can announce them.- <button + <button onClick={onBack} className="flex items-center justify-center" + aria-label="Back" @@ - <button + <button onClick={onClose || onBack} className="flex items-center justify-center" + aria-label="Close"Also applies to: 384-397
153-157: Avoid blockingalertin UX flowsReplace the blocking
alertwith your app’s non‑blocking toast/notification pattern.
304-318: Tool update payload: confirm duplication ofvaluevsconfigis intentionalYou persist full config in both
valueand partially inconfig. If the backend only needs one, drop the duplicate to avoid drift.If both are required, add a brief comment explaining why.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/components/workflow/AIAgentConfigUI.tsx(2 hunks)frontend/src/components/workflow/Types.ts(4 hunks)frontend/src/components/workflow/WorkflowCard.tsx(1 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(10 hunks)frontend/src/components/workflow/executedWorkflowRenderer.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/workflow/WorkflowExecutionModal.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/workflow/executedWorkflowRenderer.tsx (1)
frontend/src/components/workflow/Types.ts (5)
Step(64-92)Tool(56-61)Flow(160-165)TemplateFlow(95-99)UserDetail(1-6)
frontend/src/components/workflow/AIAgentConfigUI.tsx (3)
frontend/src/api.ts (1)
api(4-4)frontend/src/hooks/useChatStream.ts (1)
createAuthEventSource(212-243)frontend/src/components/workflow/api/ApiHandlers.ts (1)
workflowToolsAPI(376-407)
frontend/src/components/workflow/WorkflowCard.tsx (2)
frontend/src/components/workflow/Types.ts (1)
WorkflowCardProps(278-282)frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
WorkflowExecutionModal(34-670)
🔇 Additional comments (1)
frontend/src/components/workflow/Types.ts (1)
227-276: Confirm requiredness of Template fields against API payloads.description/createdBy/updatedAt/status/version/config are all required here. If the API sometimes omits them, this will cause type drift and runtime “undefined” rendering.
Would you confirm the backend schema for WorkflowTemplate and relax optionality where needed? I can generate a follow-up patch once confirmed.
6 participants
Workflow Feature Changelog
This document outlines the database schema changes and new API endpoints introduced for the workflow feature.
Database Schema Changes
The following tables have been added to the database schema in
server/db/schema/workflows.ts:workflow_template: Stores the templates for workflows, including name, description, version, and configuration.workflow_step_template: Defines the individual steps within a workflow template.workflow_tool: Contains the tools that can be used within a workflow step.workflow_execution: Tracks the execution of a workflow template.workflow_step_execution: Tracks the execution of individual steps within a workflow execution.tool_execution: Logs the execution of tools within a workflow step.API Endpoints
The following API endpoints have been added to
server/server.tsunder the/api/v1/workflowprefix:Workflow Templates
POST /templates: Creates a new workflow template.POST /templates/complex: Creates a complex workflow template from a frontend builder.GET /templates: Lists all workflow templates.GET /templates/:templateId: Retrieves a specific workflow template.PUT /templates/:templateId: Updates a workflow template.POST /templates/:templateId/execute: Executes a workflow template.POST /templates/:templateId/execute-with-input: Executes a workflow with a file input.POST /templates/:templateId/steps: Adds a new step to a workflow template.Workflow Executions
POST /executions: Creates a new workflow execution.GET /executions: Lists all workflow executions.GET /executions/:executionId: Retrieves a specific workflow execution.GET /executions/:executionId/status: Gets the status of a workflow execution.Workflow Tools
POST /tools: Creates a new workflow tool.GET /tools: Lists all workflow tools.GET /tools/:toolId: Retrieves a specific workflow tool.PUT /tools/:toolId: Updates a workflow tool.DELETE /tools/:toolId: Deletes a workflow tool.Workflow Steps
DELETE /steps/:stepId: Deletes a workflow step template.PUT /steps/:stepId: Updates a workflow step execution.POST /steps/:stepId/complete: Marks a workflow step as complete.GET /steps/:stepId/form: Retrieves the form definition for a manual step.POST /steps/submit-form: Submits data for a form step.Miscellaneous
GET /files/:fileId: Serves a file associated with a workflow.GET /models/gemini: Retrieves Gemini model enums.