Conversation
WalkthroughThe changes introduce workflow execution functionality to the CLI, including schema-based data validation using Ajv. New types and API methods support workflow runs, with enhanced test coverage for execution scenarios and error handling. The CLI now allows users to execute workflows with optional data, ensuring only enabled workflows are listed and executed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant APIClient
participant Utils
User->>CLI: workflow execute --workflow <name> [--data | --file]
CLI->>APIClient: getAllWorkflows()
APIClient-->>CLI: List of workflows
CLI->>CLI: Find and validate workflow (enabled, exists)
CLI->>Utils: validateData(data, schema)
Utils-->>CLI: Validation result
alt Validation success
CLI->>APIClient: runWorkflow(workflowId, data)
APIClient-->>CLI: Workflow run result
CLI->>User: Output execution result
else Validation failure
CLI->>User: Output validation errors
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code Graph Analysis (1)src/__tests__/cli-commands.test.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (6)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/utils.ts (1)
54-67: Improve error handling and type safety in validateData.The function uses
anytypes which reduces type safety, and the error mapping could be more robust.Consider this improvement:
-export const validateData = (data: any, schema: any) => { +export const validateData = (data: unknown, schema: object) => { const validate = ajv.compile(schema) const valid = validate(data) if (!valid) { return { success: false, - messages: validate.errors?.map((error: any) => error.message) || [] + messages: validate.errors?.map((error) => `${error.instancePath || 'root'}: ${error.message}`) || ['Validation failed'] } } return { success: true, messages: [] } }This provides better error context and improves type safety.
src/cli-commands.ts (1)
143-166: Enhance workflow execution output and error handling.The function provides comprehensive execution details, but the duration display and success determination could be improved.
Consider these enhancements:
export const executeWorkflow = async (workflowId: string, data: any): Promise<CommandResult> => { const messages: string[] = [] - let success = true try { const results = await runWorkflow(workflowId, data) const startTime = new Date(results.started) const endTime = new Date(results.finished) const duration = endTime.getTime() - startTime.getTime() + const success = results.result.success - messages.push(`Workflow executed ${results.result.success ? 'successfully' : 'unsuccessfully'} in ${duration} ms`) + messages.push(`Workflow executed ${success ? 'successfully' : 'unsuccessfully'} in ${duration < 1000 ? duration + ' ms' : (duration / 1000).toFixed(2) + ' seconds'}`) messages.push(`- Status: ${results.status}`) messages.push(`- Started: ${startTime}`) messages.push(`- Finished: ${endTime}`) messages.push(`- Result: ${results.result.message}`) + + return { + messages, + success + } } catch (error) { messages.push(`❌ Failed to execute the workflow: ${error instanceof Error ? error.message : 'Unknown error'}`) - success = false + return { + messages, + success: false + } } - - return { - messages, - success - } }This bases success on the workflow result rather than just API call success and improves duration formatting.
src/index.ts (1)
47-47: TODO acknowledged for future refactoring.The TODO comment indicates this validation logic should be moved to utils when the backend has workflows requiring additional data. This is acceptable for the current implementation.
Would you like me to create an issue to track this refactoring task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.json(1 hunks)src/__tests__/cli-commands.test.ts(3 hunks)src/__tests__/fixtures.ts(2 hunks)src/api-client.ts(2 hunks)src/cli-commands.ts(3 hunks)src/index.ts(2 hunks)src/types.ts(1 hunks)src/utils.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/__tests__/fixtures.ts (1)
src/types.ts (1)
APIWorkflowRunItem(169-175)
src/api-client.ts (1)
src/types.ts (1)
APIWorkflowRunItem(169-175)
src/index.ts (3)
src/api-client.ts (1)
getAllWorkflows(74-81)src/utils.ts (2)
validateData(54-67)handleCommandResult(45-52)src/cli-commands.ts (1)
executeWorkflow(143-166)
src/cli-commands.ts (2)
src/types.ts (1)
CommandResult(205-208)src/api-client.ts (1)
runWorkflow(83-94)
src/__tests__/cli-commands.test.ts (3)
src/cli-commands.ts (2)
printWorkflows(114-141)executeWorkflow(143-166)src/types.ts (2)
APIWorkflowRunItem(169-175)APIErrorResponse(190-196)src/__tests__/fixtures.ts (1)
mockAPIWorkflowRunResponse(134-140)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Test on windows-latest with Node 20.x
- GitHub Check: Test on windows-latest with Node 22.x
- GitHub Check: Test on windows-latest with Node 24.x
- GitHub Check: Test on ubuntu-latest with Node 24.x
- GitHub Check: Test on macOS-latest with Node 24.x
- GitHub Check: Test on ubuntu-latest with Node 20.x
- GitHub Check: Test on ubuntu-latest with Node 22.x
🔇 Additional comments (6)
package.json (1)
42-43: The initial audit failed due to a missing lockfile. Let’s generate one and re-run the security check:#!/bin/bash # Create a minimal lockfile and re-run npm audit for ajv dependencies npm i --package-lock-only npm audit --audit-level=moderate --json | \ jq '.vulnerabilities | to_entries[] | select(.key == "ajv" or .key == "ajv-formats")'src/api-client.ts (1)
83-94: Well-implemented async workflow execution.The function correctly handles asynchronous workflow execution with appropriate status code expectation (202 Accepted) and follows the established error handling pattern.
Minor suggestion for payload construction:
- const payload = data ? { data } : {} + const payload = data !== undefined ? { data } : {}This handles the case where
datais explicitlynullorfalse, which might be valid workflow inputs.src/__tests__/fixtures.ts (1)
127-140: Well-structured test fixtures for workflow execution.The fixture updates properly support the new workflow execution functionality with realistic mock data. The timestamps and result structure align with the expected
APIWorkflowRunIteminterface.src/cli-commands.ts (1)
128-131: Good UX improvement filtering enabled workflows.Filtering out disabled workflows in the CLI output improves user experience by showing only actionable options.
src/types.ts (1)
144-175: LGTM! Well-structured type definitions for workflow execution.The new interfaces and type extensions properly support the workflow execution feature. Good practice adding the TODO comment for future refactoring of
WorkflowOperationItem.src/__tests__/cli-commands.test.ts (1)
435-588: Excellent test coverage for the new workflow execution functionality!The tests comprehensively cover:
- Filtering of disabled workflows
- Successful and unsuccessful workflow execution
- Proper error handling for API and network errors
- Detailed assertions on response messages and timing calculations
The test implementation follows the existing patterns and provides good coverage for the new feature.
Related #1
Related #OpenPathfinder/visionBoard#249 (no merge before this)
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Chores