-
Notifications
You must be signed in to change notification settings - Fork 30
refactor(workflow): enhance proxy and step execution handling #2163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🧪 BenchmarkShould we run the MCP Gateway benchmark for this PR? React with 👍 to run the benchmark.
Benchmark will run on the next push after you react. |
Release OptionsShould a new version be published when this PR is merged? React with an emoji to vote on the release type:
Current version: Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/mesh/src/database/index.ts">
<violation number="1" location="apps/mesh/src/database/index.ts:227">
P0: Hardcoded pool `max: 3000` is excessively high and removes configurability. PostgreSQL's default `max_connections` is typically 100, so this would likely cause connection errors. Additionally, this creates inconsistency with `createPostgresDatabase()` which still uses `config.options?.maxConnections || 10`. Restore the configurable default.</violation>
</file>
<file name="apps/mesh/src/api/routes/proxy.ts">
<violation number="1" location="apps/mesh/src/api/routes/proxy.ts:447">
P2: Missing `.catch()` on `client.close()` could cause unhandled promise rejections. The same file uses `client.close().catch(console.error)` in `listTools` and `listPrompts` - this should follow the same pattern for consistency and safety.</violation>
</file>
<file name="apps/mesh/src/web/components/details/workflow/components/step-detail-panel.tsx">
<violation number="1" location="apps/mesh/src/web/components/details/workflow/components/step-detail-panel.tsx:259">
P2: Using truthy check on `output` will incorrectly handle falsy values like `0`, `false`, or `""` as valid step outputs. Use nullish coalescing (`??`) instead to only treat `null`/`undefined` as missing values.</violation>
</file>
<file name="apps/mesh/src/web/components/details/workflow/hooks/derived/use-step-execution-status.ts">
<violation number="1" location="apps/mesh/src/web/components/details/workflow/hooks/derived/use-step-execution-status.ts:28">
P1: Incomplete refactoring: `resultsByStepId` Map is created but never populated after removing the `step_results` processing. All `resultsByStepId.get()` calls will return `undefined`, breaking step status detection. The code should use `executionItem.completed_steps` (which has `success` and `error` arrays) to determine step completion status.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/mesh/src/web/components/details/workflow/hooks/derived/use-step-execution-status.ts
Show resolved
Hide resolved
apps/mesh/src/web/components/details/workflow/components/step-detail-panel.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 10
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
apps/mesh/src/web/components/details/workflow/hooks/derived/use-step-execution-status.ts
Show resolved
Hide resolved
apps/mesh/src/web/components/details/workflow/hooks/derived/use-resolved-refs.ts
Show resolved
Hide resolved
apps/mesh/src/web/components/details/workflow/components/step-detail-panel.tsx
Show resolved
Hide resolved
apps/mesh/src/web/components/details/workflow/hooks/queries/use-workflow-collection-item.ts
Show resolved
Hide resolved
- Updated the proxy routes to improve client connection management and error handling. - Refactored the StepDetailPanel to integrate a new StepCodeSection for editing step code. - Enhanced the WorkflowStepCard to reflect execution status based on completed steps. - Modified polling logic in usePollingWorkflowExecution to streamline data retrieval. - Bumped bindings package version to 1.0.7 and added completed_steps tracking in the workflow schema.
apps/mesh/src/web/components/details/workflow/hooks/derived/use-step-execution-status.ts
Show resolved
Hide resolved
apps/mesh/src/web/components/details/workflow/components/workflow-step-card.tsx
Outdated
Show resolved
Hide resolved
| error: typeof result?.error === "string" ? result.error : undefined, | ||
| // Note: output/error details need to be fetched separately via useExecutionCompletedStep | ||
| output: undefined, | ||
| error: undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Step error messages no longer displayed in ErrorBadge
Low Severity
The StepExecutionStatus.error field is now always set to undefined in useStepExecutionStatuses. The comment mentions using useExecutionCompletedStep instead, but WorkflowStepCard still passes executionStatus?.error to ErrorBadge. This causes all error badges to display "Execution failed" instead of the actual error message, degrading the user experience when debugging workflow failures.
What is this contribution about?
Screenshots/Demonstration
Review Checklist
Note
Improves workflow execution UX and reliability while tightening proxy resource management.
finallyforlistTools/resources/readResource/resourceTemplates/getPrompt; non-blockingclose()incallTool; error logging forlistPrompts.StepCodeSectioninStepDetailPanel;InputSectionopens based on tracking; Output showsoutputorerror; hide Transform whencodeexists.useExecutionCompletedStepto fetch step results;usePollingWorkflowExecutionremovesstep_resultsand increases polling intervals; derived hooks (use-resolved-refs,use-step-execution-status) updated to rely oncompleted_steps.completed_stepswith success/error indicators.completed_stepstoWorkflowExecutionSchema; bump@decocms/meshto1.1.1and@decocms/bindingsto1.0.7.Written by Cursor Bugbot for commit c0a4032. This will update automatically on new commits. Configure here.
Summary by cubic
Improves workflow execution handling with better proxy cleanup and per-step result fetching, plus UI updates: a Step Code editor and accurate step status indicators.
New Features
Refactors
Written for commit c0a4032. Summary will update on new commits.