Add workflow management support#734
Conversation
- Introduced workflow helpers for JSON formatting and input extraction. - Created shared components for displaying workflow statuses and schemas. - Updated API configuration to include workflow service URLs. - Added new routes and navigation for workflows in the application. - Implemented Workflows page with user and admin portals for managing workflows. - Developed GraphQL API for fetching workflow definitions based on environment and component. - Enhanced runtime repository to store workflow management service URLs. - Created a workflow proxy service to handle requests to the workflow management API. - Updated database schemas and migration scripts to support new workflow features.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rades Merge add_runtime_callback_url.sql and the three workflow permission/domain migration scripts into a single add_workflow_feature_<engine>.sql per database engine. The merged scripts are idempotent (safe to re-run after a partial failure) and remove the ordering trap between the domain-constraint and permission-insert steps. Document the v2 in-place upgrade path in the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Add Runtime dialogs gain an "Enable Workflow Management" switch (off by default, BI only). When enabled, the generated Config.toml includes a [ballerina.workflow.management] section with apiKeyValue pre-filled with the generated runtime secret. The workflow proxy now authenticates to the runtime's management API with X-API-Key: <keyId>.<keyMaterial>, reconstructed from the org secret the runtime registered with (runtimes.key_id). Sent on both proxied requests and the live /workflow/definitions fetch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds end-to-end workflow management support. The backend introduces workflow models, RBAC permissions, runtime callback persistence, GraphQL discovery, and an authenticated proxy. The frontend adds workflow APIs, admin workflow and retry-task portals, human-task views, workflow artifact integration, navigation, schema-driven forms, and runtime configuration toggles. Database initialization and idempotent migration scripts support the new permissions and runtime callback column. Repository development guidance is added in Sequence Diagram(s)sequenceDiagram
participant User
participant WorkflowsPage
participant WorkflowAPI
participant ICPProxy
participant Runtime
User->>WorkflowsPage: open workflow portal
WorkflowsPage->>WorkflowAPI: query or mutate workflow data
WorkflowAPI->>ICPProxy: send scoped workflow request
ICPProxy->>Runtime: forward request with identity and API key
Runtime-->>ICPProxy: return workflow response
ICPProxy-->>WorkflowAPI: return response
WorkflowAPI-->>WorkflowsPage: update portal state
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
rdulmina
changed the title
# Conflicts: # frontend/public/config.json # frontend/src/components/EntryPoints.tsx # frontend/src/config/api.ts # frontend/src/layouts/AppLayout.tsx # icp_server/Dependencies.toml # icp_server/modules/storage/heartbeat_repository.bal
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/OrgRuntimes.tsx (1)
163-219: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWorkflow toggle shown before runtime type is known; silently ignored for MI.
The "Enable Workflow Management" switch (line 219) is rendered unconditionally, before the user selects the BI/MI tab (the tab only appears after the secret is generated). Since
workflowMgtonly affectsbiToml()and is ignored bymiToml(), a user who enables the toggle and later views the MI tab will see no workflow section applied, with no indication why. Consider adding a short note near the toggle clarifying it only applies to BI runtimes, to avoid confusion.Contrast with
frontend/src/pages/Runtime.tsx(Line 168), where the equivalent toggle is only rendered{isBI && ...}because the component type is known upfront there.💡 Suggested clarification
- <FormControlLabel control={<Switch checked={workflowMgt} onChange={(e) => setWorkflowMgt(e.target.checked)} />} label="Enable Workflow Management" sx={{ display: 'flex', mb: 2 }} /> + <FormControlLabel control={<Switch checked={workflowMgt} onChange={(e) => setWorkflowMgt(e.target.checked)} />} label="Enable Workflow Management (BI runtimes only)" sx={{ display: 'flex', mb: 2 }} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/OrgRuntimes.tsx` around lines 163 - 219, The workflow management switch in AddRuntimeModal is shown before the runtime type is chosen, but it only affects biToml() and is ignored by miToml(). Add a short helper note or label near the Switch in OrgRuntimes/AddRuntimeModal explaining that “Enable Workflow Management” applies only to BI runtimes, so users understand why it won’t change MI output; use the existing workflowMgt, biToml, and miToml logic to place the clarification where the toggle is rendered.
🧹 Nitpick comments (4)
frontend/src/config/api.ts (2)
87-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
workflowUrlnot included in config validation.
validateConfigwarns for missinggraphqlUrl,authBaseUrl,observabilityUrlbut omitsworkflowUrl, so a missingVITE_WORKFLOW_URLinconfig.jsonsilently falls back to the default without any operator warning, unlike the other URL fields.♻️ Proposed fix
if (!config.graphqlUrl) missing.push('VITE_GRAPHQL_URL'); if (!config.authBaseUrl) missing.push('VITE_AUTH_BASE_URL'); if (!config.observabilityUrl) missing.push('VITE_OBSERVABILITY_URL'); + if (!config.workflowUrl) missing.push('VITE_WORKFLOW_URL');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/config/api.ts` around lines 87 - 97, The validateConfig function in api.ts is missing workflowUrl from its missing-config checks, so add the VITE_WORKFLOW_URL entry alongside graphqlUrl, authBaseUrl, and observabilityUrl. Update the missing array logic in validateConfig to include config.workflowUrl so the warning emitted by console.warn also covers this field when it is absent.
108-114: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo protection against trailing-slash duplication in
workflowUrl.If an operator sets
VITE_WORKFLOW_URLwith a trailing slash inconfig.json,workflowApiUrlwill produce a doubled slash (.../icp/workflow//<componentId>/...), potentially breaking backend routing.♻️ Proposed fix
export const workflowApiUrl = (componentId: string, environmentId: string, subpath: string): string => - `${window.API_CONFIG.workflowUrl}/${encodeURIComponent(componentId)}/${encodeURIComponent(environmentId)}/${subpath}`; + `${window.API_CONFIG.workflowUrl.replace(/\/$/, '')}/${encodeURIComponent(componentId)}/${encodeURIComponent(environmentId)}/${subpath}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/config/api.ts` around lines 108 - 114, The workflow URL builder in workflowApiUrl does not handle a trailing slash in window.API_CONFIG.workflowUrl, which can create a doubled slash before the component path. Update workflowApiUrl to normalize the base URL before concatenation (for example, by trimming any trailing slash) so the generated workflow proxy URL is consistent regardless of how VITE_WORKFLOW_URL is configured.frontend/src/pages/OrgRuntimes.tsx (1)
152-261: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated
biToml/miToml/AddRuntimeModallogic across files.This file and
frontend/src/pages/Runtime.tsxeach implement near-identicalbiToml,miToml, andAddRuntimeModallogic (config generation, secret dialog, workflow toggle). The newworkflowMgtconditional block duplicates the same TOML fragment in both places, increasing the risk of drift if one is updated without the other.Consider extracting shared TOML-generation helpers (and possibly the modal UI) into a common module under
frontend/src/componentsorfrontend/src/utils.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/OrgRuntimes.tsx` around lines 152 - 261, The `biToml`, `miToml`, and `AddRuntimeModal` logic is duplicated here and in `Runtime.tsx`, including the new `workflowMgt` TOML fragment. Extract the shared config-generation helpers into a common module and have both pages call the same `biToml`/`miToml` functions so the workflow management block stays in one place. If the modal UI is also effectively identical, consider moving `AddRuntimeModal` into a shared component under `frontend/src/components` and reuse it from both pages.icp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sql (1)
20-23: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConstraint name lookup could match multiple rows.
SELECT@cn= name FROM sys.check_constraints WHERE ... definition LIKE '%permission_domain%'assigns to a scalar variable; if more than one CHECK constraint onpermissionshappens to referencepermission_domainin its definition, only one (arbitrarily, the last processed) is captured and dropped, potentially leaving a stale constraint behind on re-runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sql` around lines 20 - 23, The constraint lookup in the permissions migration can return multiple CHECK constraints, but `@cn` only captures one arbitrary match. Update the lookup logic in the migration script to deterministically target the intended constraint in sys.check_constraints for permissions, and handle the case where multiple matches exist by selecting a single exact constraint name or iterating over all matching names before the ALTER TABLE DROP CONSTRAINT step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/workflow/shared.tsx`:
- Around line 58-68: The disclosure header in shared.tsx is mouse-only because
the clickable Stack used in the toggle lacks keyboard semantics and handlers.
Update the toggle container to behave like an accessible button by adding an
appropriate role, tab focus, and keyboard support for Enter and Space, and keep
its click behavior tied to setOpen. Use the Stack-based toggle markup and the
open/setOpen state in this component to locate the fix.
In `@frontend/src/components/workflow/UserPortal.tsx`:
- Around line 201-311: TaskDetailDialog does not handle failed useHumanTask
fetches, so an undefined task is rendered as if it were loaded. Update
TaskDetailDialog to read the query error state from useHumanTask, and when task
is absent because loading failed, show a clear fallback/error message instead of
CodeViewer/jsonPretty(task). Keep the main render path for the existing task
details and use the unique symbols TaskDetailDialog, useHumanTask, and
CodeViewer to locate the fix.
In `@icp_server/modules/storage/runtime_repository.bal`:
- Around line 261-285: The fallback in getRuntimeWorkflowTarget can return a
stale callbackUrl from a non-RUNNING runtime, so remove the rows[0] fallback and
only return a WorkflowTarget when a row with status == "RUNNING" has a valid
callback_url. Keep the existing query/filtering in runtime_repository.bal and
make the function return () when no running runtime is available so
workflow_proxy_service.bal can surface the correct 503 path.
In `@icp_server/workflow_proxy_service.bal`:
- Around line 70-84: The workflow API key is currently attached even when the
callback target uses an unencrypted URL, so update the request path that uses
resolveWorkflowApiKey and target.callbackUrl to avoid sending X-API-Key over
http. Enforce or gate API-key attachment in the workflow proxy flow so the
header is only added for https targets, or fail/document clearly when a runtime
is configured with an http callback URL. Use resolveWorkflowApiKey and the code
that builds the outbound callback request to locate the fix.
- Around line 39-41: The default for workflowProxyAllowInsecureTLS is insecure,
so update the workflowProxyService configurable to default to secure TLS and
require an explicit opt-in for self-signed certs in dev. Adjust the
initialization around workflowProxyAllowInsecureTLS so certificate validation
remains enabled unless a caller deliberately sets the flag, and make sure any
logic in workflow_proxy_service.bal that depends on this configurable still
handles https callbackUrl cases correctly.
---
Outside diff comments:
In `@frontend/src/pages/OrgRuntimes.tsx`:
- Around line 163-219: The workflow management switch in AddRuntimeModal is
shown before the runtime type is chosen, but it only affects biToml() and is
ignored by miToml(). Add a short helper note or label near the Switch in
OrgRuntimes/AddRuntimeModal explaining that “Enable Workflow Management” applies
only to BI runtimes, so users understand why it won’t change MI output; use the
existing workflowMgt, biToml, and miToml logic to place the clarification where
the toggle is rendered.
---
Nitpick comments:
In `@frontend/src/config/api.ts`:
- Around line 87-97: The validateConfig function in api.ts is missing
workflowUrl from its missing-config checks, so add the VITE_WORKFLOW_URL entry
alongside graphqlUrl, authBaseUrl, and observabilityUrl. Update the missing
array logic in validateConfig to include config.workflowUrl so the warning
emitted by console.warn also covers this field when it is absent.
- Around line 108-114: The workflow URL builder in workflowApiUrl does not
handle a trailing slash in window.API_CONFIG.workflowUrl, which can create a
doubled slash before the component path. Update workflowApiUrl to normalize the
base URL before concatenation (for example, by trimming any trailing slash) so
the generated workflow proxy URL is consistent regardless of how
VITE_WORKFLOW_URL is configured.
In `@frontend/src/pages/OrgRuntimes.tsx`:
- Around line 152-261: The `biToml`, `miToml`, and `AddRuntimeModal` logic is
duplicated here and in `Runtime.tsx`, including the new `workflowMgt` TOML
fragment. Extract the shared config-generation helpers into a common module and
have both pages call the same `biToml`/`miToml` functions so the workflow
management block stays in one place. If the modal UI is also effectively
identical, consider moving `AddRuntimeModal` into a shared component under
`frontend/src/components` and reuse it from both pages.
In `@icp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sql`:
- Around line 20-23: The constraint lookup in the permissions migration can
return multiple CHECK constraints, but `@cn` only captures one arbitrary match.
Update the lookup logic in the migration script to deterministically target the
intended constraint in sys.check_constraints for permissions, and handle the
case where multiple matches exist by selecting a single exact constraint name or
iterating over all matching names before the ALTER TABLE DROP CONSTRAINT step.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49ef9c20-f917-459b-9d15-00f2125b4d6e
📒 Files selected for processing (37)
CLAUDE.mdfrontend/public/config.jsonfrontend/src/api/queries.tsfrontend/src/api/workflows.tsfrontend/src/components/EntryPoints.tsxfrontend/src/components/artifact-config.tsxfrontend/src/components/workflow/AdminPortal.tsxfrontend/src/components/workflow/UserPortal.tsxfrontend/src/components/workflow/WorkflowDetailDrawer.tsxfrontend/src/components/workflow/helpers.tsfrontend/src/components/workflow/shared.tsxfrontend/src/config/api.tsfrontend/src/config/routes.tsxfrontend/src/constants/permissions.tsfrontend/src/layouts/AppLayout.tsxfrontend/src/nav.tsfrontend/src/pages/OrgRuntimes.tsxfrontend/src/pages/Runtime.tsxfrontend/src/pages/Workflows.tsxicp_server/Dependencies.tomlicp_server/graphql_api.balicp_server/modules/auth/utils.balicp_server/modules/storage/auth_repository.balicp_server/modules/storage/heartbeat_repository.balicp_server/modules/storage/runtime_repository.balicp_server/modules/types/auth_types.balicp_server/modules/types/types.balicp_server/resources/db/init-scripts/h2_init.sqlicp_server/resources/db/init-scripts/mssql_init.sqlicp_server/resources/db/init-scripts/mysql_init.sqlicp_server/resources/db/init-scripts/postgresql_init.sqlicp_server/resources/db/migration-scripts/README.mdicp_server/resources/db/migration-scripts/add_workflow_feature_h2.sqlicp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sqlicp_server/resources/db/migration-scripts/add_workflow_feature_mysql.sqlicp_server/resources/db/migration-scripts/add_workflow_feature_postgresql.sqlicp_server/workflow_proxy_service.bal
The merge from upstream/main brought in pre-workflow DB snapshots (missing runtimes.callback_url and the workflow_mgt permissions), which broke fresh checkouts and would have shipped a broken DB in the distribution zip. Regenerated from the current h2_init.sql / credentials_h2_init.sql. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The frontend's generic artifact query requests { items, pageInfo } for all
artifact types, but the workflow resolver returned a plain Workflow[] —
GraphQL validation rejected every request, so workflow entry points never
rendered on the integration overview even when the runtime's workflow
management API had definitions.
- Add WorkflowsPage type alongside the other artifact page types
- Accept optional pagination input and page the result via buildPageResult
- Document the workflow query and types in the reference schema
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
icp_server/graphql_api.bal (1)
970-993: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid returning the upstream error body here
errBody.toString()is sent back on non-200 responses, which can surface runtime-specific details in GraphQL errors. Return a generic message and keep the upstream payload in server logs instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icp_server/graphql_api.bal` around lines 970 - 993, The GraphQL error handling in workflowsByEnvironmentAndComponent is leaking upstream response details by returning errBody.toString() on non-200 responses. Update the error path to return a generic error message to the client while preserving the upstream payload only in server-side logs, and keep the existing fetchWorkflowDefinitions/response parsing flow unchanged.
🧹 Nitpick comments (1)
icp_server/schema_graphql.graphql (1)
311-319: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant runtime-reference fields on
Workflow.
Workflowexposes bothruntimeIds: [String!]andruntimes: [ArtifactRuntimeInfo], whereas sibling artifact types with runtime associations (e.g.Task,LocalEntry) expose onlyruntimes: [ArtifactRuntimeInfo]. IfArtifactRuntimeInfoalready carries the runtime ID,runtimeIdsis redundant and adds an extra field for the frontend/back end to keep in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icp_server/schema_graphql.graphql` around lines 311 - 319, The Workflow type currently exposes both runtimeIds and runtimes, which duplicates runtime association data. Update the Workflow schema definition to remove runtimeIds and keep only runtimes, matching the pattern used by Task and LocalEntry in schema_graphql.graphql; then adjust any Workflow resolvers or consumers that reference runtimeIds to use runtimes instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@icp_server/graphql_api.bal`:
- Around line 970-993: The GraphQL error handling in
workflowsByEnvironmentAndComponent is leaking upstream response details by
returning errBody.toString() on non-200 responses. Update the error path to
return a generic error message to the client while preserving the upstream
payload only in server-side logs, and keep the existing
fetchWorkflowDefinitions/response parsing flow unchanged.
---
Nitpick comments:
In `@icp_server/schema_graphql.graphql`:
- Around line 311-319: The Workflow type currently exposes both runtimeIds and
runtimes, which duplicates runtime association data. Update the Workflow schema
definition to remove runtimeIds and keep only runtimes, matching the pattern
used by Task and LocalEntry in schema_graphql.graphql; then adjust any Workflow
resolvers or consumers that reference runtimeIds to use runtimes instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 734c051c-f23e-4d4d-ab03-2aaa44114cd4
⛔ Files ignored due to path filters (2)
icp_server/database/credentials_db.mv.dbis excluded by!**/*.dbicp_server/database/icp_db.mv.dbis excluded by!**/*.db
📒 Files selected for processing (19)
frontend/public/config.jsonfrontend/src/api/queries.tsfrontend/src/components/EntryPoints.tsxfrontend/src/components/artifact-config.tsxfrontend/src/config/api.tsfrontend/src/config/routes.tsxfrontend/src/layouts/AppLayout.tsxfrontend/src/pages/OrgRuntimes.tsxfrontend/src/pages/Runtime.tsxicp_server/graphql_api.balicp_server/modules/storage/auth_repository.balicp_server/modules/storage/heartbeat_repository.balicp_server/modules/storage/runtime_repository.balicp_server/modules/types/types.balicp_server/resources/db/init-scripts/h2_init.sqlicp_server/resources/db/init-scripts/mssql_init.sqlicp_server/resources/db/init-scripts/mysql_init.sqlicp_server/resources/db/init-scripts/postgresql_init.sqlicp_server/schema_graphql.graphql
🚧 Files skipped from review as they are similar to previous changes (16)
- icp_server/modules/storage/auth_repository.bal
- icp_server/modules/storage/runtime_repository.bal
- frontend/src/config/routes.tsx
- frontend/src/api/queries.ts
- frontend/src/layouts/AppLayout.tsx
- icp_server/modules/types/types.bal
- icp_server/modules/storage/heartbeat_repository.bal
- frontend/src/pages/Runtime.tsx
- frontend/src/components/artifact-config.tsx
- frontend/public/config.json
- frontend/src/config/api.ts
- frontend/src/pages/OrgRuntimes.tsx
- icp_server/resources/db/init-scripts/h2_init.sql
- icp_server/resources/db/init-scripts/mysql_init.sql
- icp_server/resources/db/init-scripts/mssql_init.sql
- frontend/src/components/EntryPoints.tsx
Add schema support for start workflow and complete task
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/workflow/helpers.ts`:
- Around line 121-127: Update the object/array handling in the field-validation
helper to validate the parsed JSON value’s shape after JSON.parse succeeds:
require arrays for fields with type `array` and non-null, non-array objects for
fields with type `object`. Record the same field error and avoid assigning
result[f.name] when the parsed value has the wrong type, while preserving valid
JSON handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e559682b-c178-4a06-8a64-aaf0358b6298
📒 Files selected for processing (8)
frontend/src/api/workflows.tsfrontend/src/components/workflow/AdminPortal.tsxfrontend/src/components/workflow/SchemaFormFields.tsxfrontend/src/components/workflow/UserPortal.tsxfrontend/src/components/workflow/WorkflowDetailDrawer.tsxfrontend/src/components/workflow/helpers.tsfrontend/src/components/workflow/shared.tsxfrontend/src/pages/Runtime.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/components/workflow/shared.tsx
- frontend/src/pages/Runtime.tsx
- frontend/src/components/workflow/WorkflowDetailDrawer.tsx
- frontend/src/components/workflow/AdminPortal.tsx
- frontend/src/api/workflows.ts
- Overview: Start Instance action for workflow artifacts (gated by workflow_mgt:manage_workflows), reusing the admin StartWorkflowDialog with the selected workflow preselected - StartWorkflowDialog: success popup showing the started workflow ID (styled as code) with copy and view-instance actions; view deep-links to the Admin Actions tab searched by workflow ID - WorkflowDetailDrawer: narrow lifecycle actions by status (no Resume while running, no actions for closed instances) - Order workflow executions, retry tasks, and human task lists by start time, newest first
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/EntryPoints.tsx`:
- Around line 420-426: Update the Snackbar content in EntryPoints so the Alert
remains mounted during the exit transition instead of conditionally rendering
undefined when workflowToast is null. Render Alert unconditionally, use optional
chaining for workflowToast severity and message, and preserve the existing close
handlers and Snackbar behavior.
In `@frontend/src/components/workflow/AdminPortal.tsx`:
- Around line 447-458: Update the retry-task loading flow around useRetryTasks
and items so selectedType.workflowType filtering is applied across all relevant
pages, not only the first 50-task page. Prefer adding the workflow-name filter
to the server query if supported; otherwise fetch and combine pagination results
before applying the existing splitQualifiedName matching and sortByStartTimeDesc
logic.
In `@icp_server/Dependencies.toml`:
- Line 8: Update the dependency metadata represented by distribution-version in
Dependencies.toml to 2201.13.2, then regenerate the file using the CI toolchain
so its generated dependency entries remain consistent with the updated
distribution version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0fd0db84-1a65-4082-8d51-2618b31bdecc
📒 Files selected for processing (8)
frontend/src/api/workflows.tsfrontend/src/components/EntryPoints.tsxfrontend/src/components/workflow/AdminPortal.tsxfrontend/src/components/workflow/UserPortal.tsxfrontend/src/components/workflow/WorkflowDetailDrawer.tsxfrontend/src/components/workflow/helpers.tsfrontend/src/pages/Workflows.tsxicp_server/Dependencies.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/pages/Workflows.tsx
- frontend/src/components/workflow/UserPortal.tsx
- frontend/src/components/workflow/WorkflowDetailDrawer.tsx
# Conflicts: # frontend/src/components/EntryPoints.tsx # icp_server/database/credentials_db.mv.db # icp_server/database/icp_db.mv.db
95fad5e to
ea1ea31
Compare
- Make SchemaDisclosure toggle keyboard-accessible (button role, focus, Enter/Space handling) - Show an error message in the task detail dialog when the task fetch fails and hide actions for an unloaded task - Validate VITE_WORKFLOW_URL and normalize trailing slashes in the workflow proxy URL builder - Extract the shared workflow-management TOML fragment into utils/runtimeToml.ts and clarify the OrgRuntimes toggle applies to BI runtimes only - Resolve workflow targets only from RUNNING runtimes (no stale callback URL fallback) - Default workflow proxy TLS verification to secure and warn when a plain-http callback URL carries the management API key - Drop all matching permission_domain CHECK constraints in the MSSQL migration before re-adding the named one
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
icp_server/graphql_api.bal (1)
984-990: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse the workflow-specific permission for this query.
This handler checks only integration permissions, so users lacking
workflow_mgt:view_workflowscan still retrieve workflow definitions. Authorize with the corresponding Workflow-Management view permission using the existing scope before callingfetchWorkflowDefinitions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icp_server/graphql_api.bal` around lines 984 - 990, Update the authorization check in the workflow query handler before fetchWorkflowDefinitions to use the workflow-specific view permission workflow_mgt:view_workflows with the existing scope, replacing the integration permission list while preserving the unauthorized response and logging behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/EntryPoints.tsx`:
- Line 409: Update the `sx.ml` condition on the “View Runtimes” `Button` in
`EntryPoints` to include `showInstancesButton` alongside the existing button
visibility flags, so it does not receive `ml: 'auto'` when the instances button
is shown and both buttons remain grouped on the right.
---
Outside diff comments:
In `@icp_server/graphql_api.bal`:
- Around line 984-990: Update the authorization check in the workflow query
handler before fetchWorkflowDefinitions to use the workflow-specific view
permission workflow_mgt:view_workflows with the existing scope, replacing the
integration permission list while preserving the unauthorized response and
logging behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 855ecdad-80ae-4ef8-8198-a2dd0b839f5a
⛔ Files ignored due to path filters (2)
icp_server/database/credentials_db.mv.dbis excluded by!**/*.dbicp_server/database/icp_db.mv.dbis excluded by!**/*.db
📒 Files selected for processing (20)
frontend/src/components/EntryPoints.tsxfrontend/src/components/artifact-config.tsxfrontend/src/components/workflow/UserPortal.tsxfrontend/src/components/workflow/shared.tsxfrontend/src/config/api.tsfrontend/src/pages/OrgRuntimes.tsxfrontend/src/pages/Runtime.tsxfrontend/src/utils/runtimeToml.tsicp_server/graphql_api.balicp_server/modules/storage/auth_repository.balicp_server/modules/storage/heartbeat_repository.balicp_server/modules/storage/runtime_repository.balicp_server/modules/types/types.balicp_server/resources/db/init-scripts/h2_init.sqlicp_server/resources/db/init-scripts/mssql_init.sqlicp_server/resources/db/init-scripts/mysql_init.sqlicp_server/resources/db/init-scripts/postgresql_init.sqlicp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sqlicp_server/schema_graphql.graphqlicp_server/workflow_proxy_service.bal
🚧 Files skipped from review as they are similar to previous changes (15)
- icp_server/modules/storage/auth_repository.bal
- frontend/src/components/artifact-config.tsx
- frontend/src/components/workflow/shared.tsx
- frontend/src/config/api.ts
- icp_server/modules/storage/heartbeat_repository.bal
- frontend/src/pages/OrgRuntimes.tsx
- frontend/src/pages/Runtime.tsx
- icp_server/schema_graphql.graphql
- icp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sql
- icp_server/modules/types/types.bal
- icp_server/resources/db/init-scripts/mysql_init.sql
- frontend/src/components/workflow/UserPortal.tsx
- icp_server/resources/db/init-scripts/mssql_init.sql
- icp_server/resources/db/init-scripts/postgresql_init.sql
- icp_server/workflow_proxy_service.bal
- Gate the workflow definitions GraphQL query by the workflow-specific view/manage permissions instead of integration permissions - Keep upstream response bodies out of client-facing workflow definition errors; log them server-side instead - Remove the redundant Workflow.runtimeIds field (runtimes carries the association) - Fetch and combine all retry-task pages so client-side workflow-name filtering sees the full set - Keep the workflow toast Alert mounted through the Snackbar exit transition and group View Runtimes with the instance buttons
eaa554a to
bbacffe
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
icp_server/graphql_api.bal (1)
976-982: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
auth:buildScopeFromContextto construct the access scope.The resolver manually constructs
types:AccessScopewith a hardcodedorgUuid: 1. As per coding guidelines, useauth:buildScopeFromContextto build the scope before checking permissions.♻️ Proposed refactor
- types:AccessScope scope = { - orgUuid: 1, - projectUuid: projectId, - integrationUuid: componentId, - envUuid: environmentId - }; + types:AccessScope scope = auth:buildScopeFromContext(projectId, componentId, environmentId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icp_server/graphql_api.bal` around lines 976 - 982, Replace the manual types:AccessScope construction in the resolver with auth:buildScopeFromContext, passing the available project, integration, and environment context so the resulting scope is built before permission checks and no hardcoded orgUuid is used.Source: Coding guidelines
🧹 Nitpick comments (1)
frontend/src/components/artifact-config.tsx (1)
84-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse theme tokens for Workflow colors.
The new
colorandbgColorvalues are raw hex literals thatEntryPoints.tsxpasses directly intosx. Resolve these through the theme or semantic palette tokens so the Workflow UI respects theme customization.As per coding guidelines, frontend UI should use theme tokens instead of hardcoded colors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/artifact-config.tsx` at line 84, Update the Workflow entry in the artifact configuration to use existing theme or semantic palette tokens for both color and bgColor instead of raw hex literals. Preserve the current Workflow styling intent while ensuring EntryPoints.tsx receives theme-resolved values that support customization.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@icp_server/graphql_api.bal`:
- Around line 976-982: Replace the manual types:AccessScope construction in the
resolver with auth:buildScopeFromContext, passing the available project,
integration, and environment context so the resulting scope is built before
permission checks and no hardcoded orgUuid is used.
---
Nitpick comments:
In `@frontend/src/components/artifact-config.tsx`:
- Line 84: Update the Workflow entry in the artifact configuration to use
existing theme or semantic palette tokens for both color and bgColor instead of
raw hex literals. Preserve the current Workflow styling intent while ensuring
EntryPoints.tsx receives theme-resolved values that support customization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79c7de2a-9a41-4c32-89e7-54d14cc64771
⛔ Files ignored due to path filters (2)
icp_server/database/credentials_db.mv.dbis excluded by!**/*.dbicp_server/database/icp_db.mv.dbis excluded by!**/*.db
📒 Files selected for processing (21)
frontend/src/api/workflows.tsfrontend/src/components/EntryPoints.tsxfrontend/src/components/artifact-config.tsxfrontend/src/components/workflow/UserPortal.tsxfrontend/src/components/workflow/shared.tsxfrontend/src/config/api.tsfrontend/src/pages/OrgRuntimes.tsxfrontend/src/pages/Runtime.tsxfrontend/src/utils/runtimeToml.tsicp_server/graphql_api.balicp_server/modules/storage/auth_repository.balicp_server/modules/storage/heartbeat_repository.balicp_server/modules/storage/runtime_repository.balicp_server/modules/types/types.balicp_server/resources/db/init-scripts/h2_init.sqlicp_server/resources/db/init-scripts/mssql_init.sqlicp_server/resources/db/init-scripts/mysql_init.sqlicp_server/resources/db/init-scripts/postgresql_init.sqlicp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sqlicp_server/schema_graphql.graphqlicp_server/workflow_proxy_service.bal
🚧 Files skipped from review as they are similar to previous changes (18)
- frontend/src/utils/runtimeToml.ts
- icp_server/modules/storage/auth_repository.bal
- frontend/src/pages/Runtime.tsx
- frontend/src/components/workflow/shared.tsx
- icp_server/modules/storage/runtime_repository.bal
- icp_server/schema_graphql.graphql
- icp_server/modules/types/types.bal
- frontend/src/pages/OrgRuntimes.tsx
- frontend/src/config/api.ts
- icp_server/resources/db/init-scripts/h2_init.sql
- frontend/src/components/EntryPoints.tsx
- icp_server/resources/db/init-scripts/mysql_init.sql
- icp_server/resources/db/init-scripts/mssql_init.sql
- icp_server/resources/db/init-scripts/postgresql_init.sql
- frontend/src/api/workflows.ts
- frontend/src/components/workflow/UserPortal.tsx
- icp_server/workflow_proxy_service.bal
- icp_server/resources/db/migration-scripts/add_workflow_feature_mssql.sql
anuruddhal
left a comment
anuruddhal
left a comment
There was a problem hiding this comment.
Committed binary H2 database files: icp_server/database/credentials_db.mv.db and icp_server/database/icp_db.mv.db changed as binaries, which can't be reviewed as a diff. If these are the dev/test fixtures regenerated for the new schema that's expected — but please confirm they were produced cleanly by initAllH2Databases and don't carry stray local data (e.g. generated org secrets or test users from a dev session).
| configurable decimal workflowProxyTimeout = 30; | ||
|
|
||
| // Cache of http:Clients keyed by callbackUrl so we don't rebuild a client per request. | ||
| isolated map<http:Client> workflowClientCache = {}; |
There was a problem hiding this comment.
This cache is keyed by callbackUrl and never evicted. In Kubernetes, pod churn produces a new callback URL per pod, so the map grows for the life of the server, and stale clients hold on to their sockets/config (a changed workflowProxyTimeout/TLS setting only applies to new entries). A simple size cap, or evicting entries whose URL no longer appears in runtimes, would bound it.
| if roleNames is error { | ||
| return workflowErrorResponse(500, "Failed to resolve user roles: " + roleNames.message()); | ||
| } | ||
| string roles = string:'join(",", ...roleNames); |
There was a problem hiding this comment.
Role names are user-created, so a name containing a comma corrupts the runtime's parsing of x-user-roles — e.g. a role literally named Foo,admin would effectively inject the synthetic admin role. Either reject commas in role names at creation, or strip/escape them here before joining.
| SELECT callback_url, key_id, status | ||
| FROM runtimes | ||
| WHERE component_id = ${componentId} AND environment_id = ${environmentId} | ||
| AND callback_url IS NOT NULL AND callback_url <> '' |
There was a problem hiding this comment.
Minor: this fetches every runtime with a callback URL and then filters status == "RUNNING" in Ballerina. Adding AND status = 'RUNNING' to the WHERE clause is simpler and cheaper — the loop (and the status column in the projection) then goes away, since any returned row is usable.
anuruddhal
left a comment
anuruddhal
left a comment
There was a problem hiding this comment.
Oracle support is now merged to main, so this PR needs to cover Oracle 19c the same way it covers the other four engines. Three gaps noted inline: a query predicate that always returns zero rows on Oracle, the missing oracle_init.sql updates (which would break heartbeats on Oracle entirely, not just workflows), and the missing Oracle upgrade script.
| SELECT callback_url, key_id, status | ||
| FROM runtimes | ||
| WHERE component_id = ${componentId} AND environment_id = ${environmentId} | ||
| AND callback_url IS NOT NULL AND callback_url <> '' |
There was a problem hiding this comment.
This predicate breaks on Oracle 19c: Oracle treats '' as NULL, so callback_url <> '' becomes a comparison with NULL and never matches — the query always returns zero rows, and the workflow proxy permanently answers 503 on Oracle deployments. A portable fix is AND LENGTH(callback_url) > 0 (Oracle: LENGTH(NULL) is NULL → row filtered; other engines: LENGTH('') = 0 → row filtered). Alternatively keep just IS NOT NULL in SQL and skip empty strings in the Ballerina loop below.
| runtime_hostname VARCHAR(255), | ||
| runtime_port VARCHAR(10), | ||
| callback_url VARCHAR(500), | ||
| platform_name VARCHAR(50) NOT NULL DEFAULT 'ballerina', |
There was a problem hiding this comment.
oracle_init.sql (now on main) needs the same changes as the other four init scripts: this callback_url column, 'Workflow-Management' in the permission_domain CHECK constraint, the four workflow_mgt:* permissions, and the role grants. Without the column, the modified heartbeat upsert references a nonexistent column and fails with ORA-00904 on every BI heartbeat — breaking runtime monitoring on Oracle entirely, not just the workflow feature. Note the permission seeds can't be copied verbatim: Oracle 19c doesn't support multi-row INSERT ... VALUES (...), (...), so they need single-row inserts, as oracle_init.sql already does for the existing seeds.
|
|
||
| Pick the script matching your database engine: | ||
|
|
||
| | Engine | Script | |
There was a problem hiding this comment.
Oracle is missing from this table — existing Oracle deployments need an add_workflow_feature_oracle.sql as well. Oracle has no ADD COLUMN IF NOT EXISTS, so idempotency needs a PL/SQL block guarded by user_tab_columns (same idea as the MySQL script's information_schema guard), and the permission-domain CHECK constraint drop/recreate has to use the constraint name from oracle_init.sql.
This is verified and the commited |
2 participants
Overview
This PR adds complete workflow management support to the Integration Control Plane: monitoring and managing BI workflow executions and human tasks from the ICP console, end to end — UI, GraphQL API, runtime proxy, RBAC, and database migrations.
Introduced a separate section to manage workflow-related actions
Feature
Workflow management UI and backend integration
/icp/workflow): forwards console calls to the per-runtime workflow management REST service. The reachable base URL (callbackUrl) is reported via the runtime heartbeat and stored on theruntimesrowGET /workflow/definitionsAPI (not carried in heartbeats)RBAC
workflow_mgt:view_human_tasks,manage_human_tasks,view_workflows,manage_workflowsx-user-id,x-user-roles) so the workflow service can do its own human-task authorizationRuntime API-key authentication
Config.tomlincludes a[ballerina.workflow.management]section withapiKeyValuepre-filled with the generated runtime secret andapiKeyHeader = "X-API-Key"X-API-Key: <keyId>.<keyMaterial>, reconstructed from the org secret the runtime registered with (runtimes.key_id). Runtimes without a recorded key id are called without the header (compatible withenableApiKey = false)Database migrations for existing deployments
Fresh installs get the full schema from the
*_init.sqlscripts. Existing v2 deployments (v2.0.0-beta2 and earlier) must run a single idempotent upgrade script,add_workflow_feature_<engine>.sql(H2 / MySQL / PostgreSQL / MSSQL), which addsruntimes.callback_url, widens the permission-domain constraint, and seeds the workflow permissions + role grants. The in-place upgrade path is documented in the migration-scripts README.Misc
CLAUDE.mdwith build commands and architecture overview for AI-assisted developmentTesting
bal buildcompiles clean; full backend test suite runs in CI (docker-compose.test.yml)v2.0.0-beta2-schema database: column + 4 permissions created, role grant matrix matches fresh installs, re-run is a clean no-op. MySQL/PostgreSQL/MSSQL variants use the same logic with engine idioms but are not execution-tested🤖 Generated with Claude Code