feat: workflow list and trigger commands#68
Open
adityachoudhari26 wants to merge 1 commit intomainfrom
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis change adds a new Changes
Sequence Diagram(s)sequenceDiagram
actor CLI as List Command
participant Viper as Viper Config
participant APIClient as API Client
participant Backend as Backend API
participant Output as Output Handler
CLI->>Viper: Read url, api-key, workspace
CLI->>APIClient: NewAPIKeyClientWithResponses
Note over APIClient: Initialize authenticated client
CLI->>Backend: ListWorkflows(workspaceID, params)
Backend-->>CLI: Workflows response
CLI->>Output: HandleResponseOutput
Output-->>CLI: Formatted result
sequenceDiagram
actor CLI as Trigger Command
participant Flags as Flag Parser
participant Viper as Viper Config
participant APIClient as API Client
participant Backend as Backend API
participant Output as Output Handler
CLI->>Flags: Parse --input flags as key=value pairs
Flags-->>CLI: Validated input map
CLI->>Viper: Read url, api-key, workspace
CLI->>APIClient: NewAPIKeyClientWithResponses
Note over APIClient: Initialize authenticated client
CLI->>Backend: CreateWorkflowRun(workspaceID, workflowID, inputs)
Backend-->>CLI: Workflow run response
CLI->>Output: HandleResponseOutput
Output-->>CLI: Formatted result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
internal/api/client.gen.go (1)
295-312: Confirm the minimum server version for the workflow contract changes.Lines 295-312 and 1317-1368 now only read/write
jobAgents, and Line 14994 only treats201as the typed create success. If this CLI can still target pre-migration API deployments, workflow reads will silently dropjobsand create calls will fall off the typed happy path. Please either version-gate these commands or keep dual-format handling in the API/spec until the rollout is complete.Also applies to: 1068-1073, 1317-1368, 10783-10787, 14993-14999
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/client.gen.go` around lines 295 - 312, The generated client now only serializes/deserializes CreateWorkflow.JobAgents and treats HTTP 201 as the sole typed create success, which will break compatibility with pre-migration APIs that use jobs or different success codes; update the client to either check the server version before using the new schema or implement dual-format handling: in functions that marshal/unmarshal CreateWorkflow and CreateWorkflowJobAgent (and any create workflow response handling that currently assumes 201), add logic to accept both "jobs" and "jobAgents" fields when reading workflows and to emit both formats (or choose format based on a negotiated/minimum server version); likewise, adjust the create response parsing to accept alternative success codes or fall back to untyped handling when the typed 201 path fails, using the CreateWorkflow and CreateWorkflowJobAgent types as the primary identifiers to locate relevant code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/ctrlc/root/workflow/list.go`:
- Line 30: The call to client.GetWorkspaceID(cmd.Context(), workspace) can
return a zero UUID on failure; before calling client.ListWorkflows, check the
returned workspaceID for a zero/empty UUID and return an explicit error (or wrap
with fmt.Errorf) so we fail fast and don't send an invalid ID to ListWorkflows;
update the block around GetWorkspaceID and the subsequent call to ListWorkflows
to validate workspaceID and return early if it is the zero value.
- Around line 33-38: Replace the silent-ignore behavior for negative pagination
flags by validating the limit and offset variables before assigning to params:
check if limit < 0 or offset < 0 and return a CLI validation error (e.g.,
formatted error via fmt.Errorf) instead of falling through; only set
params.Limit and params.Offset when values are non-negative and valid. Update
the code paths around the existing assignments to params.Limit/params.Offset so
invalid inputs cause an early error return from the command handler (or pre-run)
and valid inputs continue to set the pointers as currently done.
In `@cmd/ctrlc/root/workflow/trigger.go`:
- Line 33: The code assigns workspaceID via client.GetWorkspaceID but doesn't
guard against a failed lookup returning a zero UUID; before making the trigger
call, check that workspaceID is not the zero value (compare against uuid.Nil or
the project's zero-UUID constant) and return a clear error (e.g., "workspace not
found" with the original workspace string) if it is zero to avoid calling the
trigger with an invalid identifier; update the function that calls
client.GetWorkspaceID and the subsequent trigger invocation to perform this
validation and short-circuit with an error.
- Around line 37-42: The parsing loop that uses strings.Cut(input, "=")
currently allows an empty key (e.g., "--input =foo"); after extracting key,
value, found in trigger.go validate that found is true and also that key != ""
(non-empty); if key is empty return a descriptive error (e.g., "invalid input
format %q, empty key, expected key=value") instead of assigning into the inputs
map so empty keys are rejected (refer to the variables key, value, found and the
inputs map where the assignment inputs[key] = value occurs).
---
Nitpick comments:
In `@internal/api/client.gen.go`:
- Around line 295-312: The generated client now only serializes/deserializes
CreateWorkflow.JobAgents and treats HTTP 201 as the sole typed create success,
which will break compatibility with pre-migration APIs that use jobs or
different success codes; update the client to either check the server version
before using the new schema or implement dual-format handling: in functions that
marshal/unmarshal CreateWorkflow and CreateWorkflowJobAgent (and any create
workflow response handling that currently assumes 201), add logic to accept both
"jobs" and "jobAgents" fields when reading workflows and to emit both formats
(or choose format based on a negotiated/minimum server version); likewise,
adjust the create response parsing to accept alternative success codes or fall
back to untyped handling when the typed 201 path fails, using the CreateWorkflow
and CreateWorkflowJobAgent types as the primary identifiers to locate relevant
code paths.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c68df1ae-9981-47c0-8b11-71cebac9d0be
📒 Files selected for processing (5)
cmd/ctrlc/root/root.gocmd/ctrlc/root/workflow/list.gocmd/ctrlc/root/workflow/trigger.gocmd/ctrlc/root/workflow/workflow.gointernal/api/client.gen.go
| return fmt.Errorf("failed to create API client: %w", err) | ||
| } | ||
|
|
||
| workspaceID := client.GetWorkspaceID(cmd.Context(), workspace) |
Contributor
There was a problem hiding this comment.
Fail fast when workspace resolution returns a zero UUID.
Line 30 can produce a zero UUID on lookup failure, and Line 40 then sends that value to the API. Return an explicit error before calling ListWorkflows to avoid misleading failures.
Proposed fix
RunE: func(cmd *cobra.Command, args []string) error {
apiURL := viper.GetString("url")
apiKey := viper.GetString("api-key")
workspace := viper.GetString("workspace")
+ if workspace == "" {
+ return fmt.Errorf("workspace is required")
+ }
client, err := api.NewAPIKeyClientWithResponses(apiURL, apiKey)
if err != nil {
return fmt.Errorf("failed to create API client: %w", err)
}
workspaceID := client.GetWorkspaceID(cmd.Context(), workspace)
+ if workspaceID.String() == "00000000-0000-0000-0000-000000000000" {
+ return fmt.Errorf("failed to resolve workspace %q", workspace)
+ }Also applies to: 40-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/ctrlc/root/workflow/list.go` at line 30, The call to
client.GetWorkspaceID(cmd.Context(), workspace) can return a zero UUID on
failure; before calling client.ListWorkflows, check the returned workspaceID for
a zero/empty UUID and return an explicit error (or wrap with fmt.Errorf) so we
fail fast and don't send an invalid ID to ListWorkflows; update the block around
GetWorkspaceID and the subsequent call to ListWorkflows to validate workspaceID
and return early if it is the zero value.
Comment on lines
+33
to
+38
| if limit > 0 { | ||
| params.Limit = &limit | ||
| } | ||
| if offset > 0 { | ||
| params.Offset = &offset | ||
| } |
Contributor
There was a problem hiding this comment.
Validate pagination flags instead of silently ignoring invalid values.
Negative --limit / --offset currently fall through and are treated as unset. It's better to return a direct CLI validation error.
Proposed fix
RunE: func(cmd *cobra.Command, args []string) error {
+ if limit < 0 {
+ return fmt.Errorf("--limit must be >= 0")
+ }
+ if offset < 0 {
+ return fmt.Errorf("--offset must be >= 0")
+ }
+
apiURL := viper.GetString("url")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/ctrlc/root/workflow/list.go` around lines 33 - 38, Replace the
silent-ignore behavior for negative pagination flags by validating the limit and
offset variables before assigning to params: check if limit < 0 or offset < 0
and return a CLI validation error (e.g., formatted error via fmt.Errorf) instead
of falling through; only set params.Limit and params.Offset when values are
non-negative and valid. Update the code paths around the existing assignments to
params.Limit/params.Offset so invalid inputs cause an early error return from
the command handler (or pre-run) and valid inputs continue to set the pointers
as currently done.
| return fmt.Errorf("failed to create API client: %w", err) | ||
| } | ||
|
|
||
| workspaceID := client.GetWorkspaceID(cmd.Context(), workspace) |
Contributor
There was a problem hiding this comment.
Guard against unresolved workspace IDs before triggering runs.
Line 33 can yield a zero UUID when workspace lookup fails; Line 48 then issues the trigger call with that invalid workspace identifier. Return a clear error first.
Proposed fix
workspaceID := client.GetWorkspaceID(cmd.Context(), workspace)
+ if workspaceID.String() == "00000000-0000-0000-0000-000000000000" {
+ return fmt.Errorf("failed to resolve workspace %q", workspace)
+ }
inputs := make(map[string]interface{})Also applies to: 48-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/ctrlc/root/workflow/trigger.go` at line 33, The code assigns workspaceID
via client.GetWorkspaceID but doesn't guard against a failed lookup returning a
zero UUID; before making the trigger call, check that workspaceID is not the
zero value (compare against uuid.Nil or the project's zero-UUID constant) and
return a clear error (e.g., "workspace not found" with the original workspace
string) if it is zero to avoid calling the trigger with an invalid identifier;
update the function that calls client.GetWorkspaceID and the subsequent trigger
invocation to perform this validation and short-circuit with an error.
Comment on lines
+37
to
+42
| key, value, found := strings.Cut(input, "=") | ||
| if !found { | ||
| return fmt.Errorf("invalid input format %q, expected key=value", input) | ||
| } | ||
| inputs[key] = value | ||
| } |
Contributor
There was a problem hiding this comment.
Reject empty input keys in --input key=value.
--input =foo currently passes parsing and sends an empty key in the payload map. Validate key presence before assignment.
Proposed fix
for _, input := range inputFlags {
key, value, found := strings.Cut(input, "=")
- if !found {
- return fmt.Errorf("invalid input format %q, expected key=value", input)
+ if !found || strings.TrimSpace(key) == "" {
+ return fmt.Errorf("invalid input format %q, expected key=value with non-empty key", input)
}
inputs[key] = value
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| key, value, found := strings.Cut(input, "=") | |
| if !found { | |
| return fmt.Errorf("invalid input format %q, expected key=value", input) | |
| } | |
| inputs[key] = value | |
| } | |
| key, value, found := strings.Cut(input, "=") | |
| if !found || strings.TrimSpace(key) == "" { | |
| return fmt.Errorf("invalid input format %q, expected key=value with non-empty key", input) | |
| } | |
| inputs[key] = value | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/ctrlc/root/workflow/trigger.go` around lines 37 - 42, The parsing loop
that uses strings.Cut(input, "=") currently allows an empty key (e.g., "--input
=foo"); after extracting key, value, found in trigger.go validate that found is
true and also that key != "" (non-empty); if key is empty return a descriptive
error (e.g., "invalid input format %q, empty key, expected key=value") instead
of assigning into the inputs map so empty keys are rejected (refer to the
variables key, value, found and the inputs map where the assignment inputs[key]
= value occurs).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes