-
Notifications
You must be signed in to change notification settings - Fork 14
chore: workflows working e2e #770
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
📝 WalkthroughWalkthroughAdds workflow event handling and job-success orchestration: new event types and handlers for workflow/template creation, a pluggable WorkflowActionOrchestrator invoked on job success, a WorkflowManagerAction that reconciles workflows, workspace integration of workflow components, store additions, and new tests and test helpers. Changes
Sequence DiagramsequenceDiagram
actor Job
participant JobHandler as "Job Handler"
participant Orchestrator as "WorkflowActionOrchestrator"
participant Action as "WorkflowManagerAction"
participant Store as "Store"
participant Manager as "Workflow Manager"
Job->>JobHandler: status change -> Successful
JobHandler->>JobHandler: process status change
JobHandler->>Orchestrator: OnJobSuccess(ctx, job)
Orchestrator->>Action: Execute(ctx, ActionTriggerJobSuccess, job)
Action->>Store: Get WorkflowStep by job.WorkflowStepId
Store-->>Action: WorkflowStep
Action->>Store: Get Workflow by workflowStep.WorkflowId
Store-->>Action: Workflow
Action->>Manager: ReconcileWorkflow(ctx, workflow)
Manager->>Store: update/read workflow state
Store-->>Manager: ack
Manager-->>Action: result
Action-->>Orchestrator: result
Orchestrator-->>JobHandler: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/workspace-engine/test/integration/opts.go`:
- Around line 1420-1427: WithWorkflowBooleanInput builds a boolean input via
c.NewBooleanWorkflowInput(wft.Id) and applies WorkflowInputOption callbacks but
never attaches the created input to the workflow template, so it should be
appended to wft.Inputs; update the function (WithWorkflowBooleanInput) to call
option(ws, input) for each option and then add the resulting input to wft.Inputs
(ensuring the type matches the slice element type on
oapi.WorkflowTemplate.Inputs) so the input is preserved in the template.
🧹 Nitpick comments (7)
apps/workspace-engine/pkg/workspace/store/workflows.go (1)
29-37: Add an exported doc comment forGetByTemplateID.This method is exported; please add a brief doc comment that conveys intent (e.g., template-scoped lookup for workflow orchestration).
💡 Suggested doc comment
+// GetByTemplateID returns workflows keyed by ID for a given template, enabling template-scoped lookups. func (w *Workflows) GetByTemplateID(templateID string) map[string]*oapi.Workflow {As per coding guidelines, “Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods.”
apps/workspace-engine/pkg/events/handler/workflows/workflows.go (1)
11-38: Add doc comments for exported workflow handlers.Both handlers are exported and should have concise doc comments explaining their intent/usage.
💡 Suggested doc comments
+// HandleWorkflowTemplateCreated upserts a workflow template from an incoming event payload. func HandleWorkflowTemplateCreated( @@ +// HandleWorkflowCreated creates a workflow instance from an incoming event payload. func HandleWorkflowCreated(As per coding guidelines, “Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods.”
apps/workspace-engine/pkg/workspace/workflowmanager/action.go (1)
10-42: Add doc comments for exportedWorkflowManagerActionAPIs.Please add brief doc comments for the exported type and its public methods to comply with workspace-engine documentation expectations.
💡 Suggested doc comments
+// WorkflowManagerAction reconciles workflows in response to workflow-related triggers. type WorkflowManagerAction struct { @@ +// NewWorkflowManagerAction constructs a workflow manager action with the given store and manager. func NewWorkflowManagerAction(store *store.Store, manager *Manager) *WorkflowManagerAction { @@ +// Execute runs the action for supported triggers (e.g., job success). func (w *WorkflowManagerAction) Execute(ctx context.Context, trigger ActionTrigger, job *oapi.Job) error {As per coding guidelines, “Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods.”
apps/workspace-engine/pkg/workspace/workflowmanager/workflow_orchestrator.go (1)
9-44: Add doc comments for the exported workflow orchestration API.
These new exported types/functions are undocumented; please add brief Go doc comments for the public surface.As per coding guidelines: Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods.
apps/workspace-engine/pkg/workspace/workspace.go (1)
80-82: Document the new exported workflow accessors.
Please add brief Go doc comments for the newly added Workflow* accessors to align with the workspace-engine comment guideline.As per coding guidelines: Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods.
Also applies to: 100-102, 184-198
apps/workspace-engine/test/integration/creators/workflow.go (1)
10-69: Add doc comments to the new workflow test creators.
These exported helpers should have brief Go doc comments for discoverability.As per coding guidelines: Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods.
apps/workspace-engine/test/integration/opts.go (1)
1329-1354: Document the new workflow option types/helpers.
Please add brief Go doc comments to the newly exported Workflow* option types and helpers.As per coding guidelines: Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods.
Also applies to: 1356-1461, 1463-1485
Summary by CodeRabbit
New Features
Tests