Conversation
📝 WalkthroughWalkthroughThe changes refactor the workflow API from template-focused naming to workflow-focused naming, introduce new types for workflow inputs and dispatching, and update the deployment data model to use arrays of system identifiers instead of single values. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cmd/ctrlc/root/ui/fetcher.go`:
- Around line 303-313: The deployment rows are showing raw dep.SystemIds which
is inconsistent with fetchDeployments that shows system names; update
fetchDeploymentsForResource to display system names instead of IDs by resolving
dep.SystemIds to names (either use dep.SystemNames if the API already returns
it, or look up names via the existing systems map/context used elsewhere in this
file) and then join those names into the systemIds string used in the tableRow
cols; if name resolution isn't possible, at minimum change the displayed label
(or add a note) to indicate these are IDs so users aren't confused.
In `@internal/api/client.gen.go`:
- Around line 351-352: The OpenAPI spec must be updated to match the generated
client: replace the old dependsOnDeploymentSelector: Selector object with a
dependsOn field of type string (CEL expression) in the DeploymentDependencyRule
schema in openapi.json; update any schema definitions, property names, examples,
and references that still use dependsOnDeploymentSelector or Selector to
reference dependsOn (string) so DeploymentDependencyRule.DependsOn in the client
and the spec are consistent.
- Around line 204-205: OpenAPI schemas define systemId as a singular string but
the generated Go models use SystemIds []string (seen in CreateDeploymentRequest,
CreateEnvironmentRequest, Deployment, Environment, UpsertDeploymentRequest,
UpsertEnvironmentRequest); update openapi.json so each of those schema objects
replaces the property "systemId" (type: string) with "systemIds" (type: array,
items: { type: "string" }), and update any required lists, examples, and
references that mention "systemId" to "systemIds" to keep the spec consistent
with the generated types (verify usages in CreateDeploymentRequest,
CreateEnvironmentRequest, Deployment, Environment, UpsertDeploymentRequest,
UpsertEnvironmentRequest).
🧹 Nitpick comments (2)
cmd/ctrlc/root/ui/fetcher.go (2)
101-108: Consider usingstrings.Jointo simplify the system name aggregation.This manual concatenation pattern is duplicated at lines 303–310 for system IDs. Both can be replaced with
strings.Joinfor clarity and to eliminate duplication.♻️ Suggested refactor
Add
"strings"to the imports, then:- // Get system names (now plural) - systemNames := "" - if len(item.Systems) > 0 { - systemNames = item.Systems[0].Name - for i := 1; i < len(item.Systems); i++ { - systemNames += ", " + item.Systems[i].Name - } - } + // Get system names (now plural) + names := make([]string, len(item.Systems)) + for i, s := range item.Systems { + names[i] = s.Name + } + systemNames := strings.Join(names, ", ")And similarly for lines 303–310:
- // Join system IDs (now plural) - systemIds := "" - if len(dep.SystemIds) > 0 { - systemIds = dep.SystemIds[0] - for i := 1; i < len(dep.SystemIds); i++ { - systemIds += ", " + dep.SystemIds[i] - } - } + systemIds := strings.Join(dep.SystemIds, ", ")
349-350: Column header "SYSTEM" could be updated to "SYSTEMS" to reflect the new plural data model.Since deployments now have multiple systems, the header text should match.
| // Join system IDs (now plural) | ||
| systemIds := "" | ||
| if len(dep.SystemIds) > 0 { | ||
| systemIds = dep.SystemIds[0] | ||
| for i := 1; i < len(dep.SystemIds); i++ { | ||
| systemIds += ", " + dep.SystemIds[i] | ||
| } | ||
| } | ||
| rows = append(rows, tableRow{ | ||
| id: dep.Id, | ||
| cols: []string{dep.Name, dep.Slug, dep.SystemId, desc}, | ||
| cols: []string{dep.Name, dep.Slug, systemIds, desc}, |
There was a problem hiding this comment.
Displaying raw system IDs may not be user-friendly.
In fetchDeployments (line 111), the column shows system names, but here in fetchDeploymentsForResource it shows raw system IDs. This inconsistency may confuse users. Consider resolving the IDs to names if the API response includes that information, or at minimum note this as a known limitation.
🤖 Prompt for AI Agents
In `@cmd/ctrlc/root/ui/fetcher.go` around lines 303 - 313, The deployment rows are
showing raw dep.SystemIds which is inconsistent with fetchDeployments that shows
system names; update fetchDeploymentsForResource to display system names instead
of IDs by resolving dep.SystemIds to names (either use dep.SystemNames if the
API already returns it, or look up names via the existing systems map/context
used elsewhere in this file) and then join those names into the systemIds string
used in the tableRow cols; if name resolution isn't possible, at minimum change
the displayed label (or add a note) to indicate these are IDs so users aren't
confused.
| SystemIds []string `json:"systemIds"` | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for legacy single-system usage or old type name.
rg -n -C2 '\bSystemId\b|\bDeploymentAndSystem\b'Repository: ctrlplanedev/cli
Length of output: 628
🏁 Script executed:
#!/bin/bash
# Look for JSON payloads still sending "systemId".
rg -n -C2 '"systemId"'Repository: ctrlplanedev/cli
Length of output: 4512
🏁 Script executed:
#!/bin/bash
# Check the actual content at the specified lines in the file to understand the change
head -250 internal/api/client.gen.go | tail -50Repository: ctrlplanedev/cli
Length of output: 2600
🏁 Script executed:
#!/bin/bash
# Get the model names at the specified lines
sed -n '200,230p; 335,350p; 455,465p; 958,965p; 981,990p; 8045,8055p; 10855,10865p' internal/api/client.gen.go | head -100Repository: ctrlplanedev/cli
Length of output: 4271
🏁 Script executed:
#!/bin/bash
# Check if openapi.json has matching schemas with systemIds (plural)
rg -n '"systemIds"' openapi.json | head -20Repository: ctrlplanedev/cli
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Extract model definitions around the affected lines to identify them
ast-grep --pattern 'type $_ struct {
$$$
SystemIds $_
$$$
}'Repository: ctrlplanedev/cli
Length of output: 4655
🏁 Script executed:
#!/bin/bash
# Check if openapi.json schemas use systemIds (plural) instead of systemId
rg -A5 '"CreateDeploymentRequest"|"CreateEnvironmentRequest"|"Deployment"|"Environment"|"UpsertDeploymentRequest"|"UpsertEnvironmentRequest"' openapi.json | grep -E '(systemId|systemIds|properties)' | head -40Repository: ctrlplanedev/cli
Length of output: 227
🏁 Script executed:
#!/bin/bash
# Get the remaining two affected lines
sed -n '8045,8055p; 10855,10865p' internal/api/client.gen.goRepository: ctrlplanedev/cli
Length of output: 664
🏁 Script executed:
#!/bin/bash
# Search for schema definitions in openapi.json for the affected models
rg -B2 -A20 '"CreateDeploymentRequest":|"CreateEnvironmentRequest":|"Deployment":|"Environment":|"UpsertDeploymentRequest":|"UpsertEnvironmentRequest":' openapi.json | grep -E '(CreateDeployment|CreateEnvironment|Deployment|Environment|UpsertDeployment|UpsertEnvironment|systemId|systemIds|properties|required)' | head -60Repository: ctrlplanedev/cli
Length of output: 490
🏁 Script executed:
#!/bin/bash
# Get exact schema for CreateDeploymentRequest from openapi.json
python3 << 'EOF'
import json
import re
with open('openapi.json', 'r') as f:
spec = json.load(f)
# Check schemas for affected models
affected_models = [
'CreateDeploymentRequest',
'CreateEnvironmentRequest',
'Deployment',
'Environment',
'UpsertDeploymentRequest',
'UpsertEnvironmentRequest'
]
for model in affected_models:
if model in spec['components']['schemas']:
schema = spec['components']['schemas'][model]
props = schema.get('properties', {})
if 'systemId' in props or 'systemIds' in props:
print(f"\n{model}:")
if 'systemId' in props:
print(f" - Has systemId (singular): {props['systemId']}")
if 'systemIds' in props:
print(f" - Has systemIds (plural): {props['systemIds']}")
EOFRepository: ctrlplanedev/cli
Length of output: 480
🏁 Script executed:
#!/bin/bash
# Check if there are any code examples or tests that might show expected usage
rg -n 'SystemIds|systemIds' --type go | grep -v 'client.gen.go' | head -20Repository: ctrlplanedev/cli
Length of output: 488
🏁 Script executed:
#!/bin/bash
# Check DeploymentAndSystems definition and usage
rg -B3 -A10 'type DeploymentAndSystems' internal/api/client.gen.goRepository: ctrlplanedev/cli
Length of output: 531
🏁 Script executed:
#!/bin/bash
# Check all occurrences of systemId vs systemIds in the spec
echo "=== systemId (singular) in openapi.json ==="
rg -c '"systemId"' openapi.json
echo -e "\n=== systemIds (plural) in openapi.json ==="
rg -c '"systemIds"' openapi.json
echo -e "\n=== systemId (singular) in client.gen.go ==="
rg -c '\bsystemId\b' internal/api/client.gen.go
echo -e "\n=== systemIds (plural) in client.gen.go ==="
rg -c '\bsystemIds\b' internal/api/client.gen.goRepository: ctrlplanedev/cli
Length of output: 246
🏁 Script executed:
#!/bin/bash
# Check if there are any tests or marshaling code that reveal issues
rg -n 'Unmarshal|Marshal' cmd/ctrlc/root/ui/fetcher.goRepository: ctrlplanedev/cli
Length of output: 42
Update openapi.json schemas to use systemIds (plural, array) instead of systemId (singular, string).
The generated Go code correctly uses SystemIds []string for:
- CreateDeploymentRequest
- CreateEnvironmentRequest
- Deployment
- Environment
- UpsertDeploymentRequest
- UpsertEnvironmentRequest
However, the OpenAPI specification still defines these as singular systemId (string type). This spec-to-code mismatch means:
- The specification no longer matches the implementation
- Other clients generated from the spec will expect the wrong field
- API documentation is incorrect
The calling code in cmd/ctrlc/root/ui/fetcher.go already handles SystemIds as a slice correctly, confirming the implementation change is intentional. Update openapi.json to reflect the schema change from systemId to systemIds with type array of strings.
🤖 Prompt for AI Agents
In `@internal/api/client.gen.go` around lines 204 - 205, OpenAPI schemas define
systemId as a singular string but the generated Go models use SystemIds []string
(seen in CreateDeploymentRequest, CreateEnvironmentRequest, Deployment,
Environment, UpsertDeploymentRequest, UpsertEnvironmentRequest); update
openapi.json so each of those schema objects replaces the property "systemId"
(type: string) with "systemIds" (type: array, items: { type: "string" }), and
update any required lists, examples, and references that mention "systemId" to
"systemIds" to keep the spec consistent with the generated types (verify usages
in CreateDeploymentRequest, CreateEnvironmentRequest, Deployment, Environment,
UpsertDeploymentRequest, UpsertEnvironmentRequest).
| // DependsOn CEL expression to match upstream deployment(s) that must have a successful release before this deployment can proceed. | ||
| DependsOn string `json:"dependsOn"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find usages that may still assume the old selector object
rg -n -C2 '\bDeploymentDependencyRule\b'
rg -n -C2 '\.DependsOn\b'Repository: ctrlplanedev/cli
Length of output: 1640
🏁 Script executed:
#!/bin/bash
# Check the DeploymentDependencyRule definition in openapi.json
rg -A 20 '"DeploymentDependencyRule"' openapi.jsonRepository: ctrlplanedev/cli
Length of output: 656
🏁 Script executed:
#!/bin/bash
# Search for builders or constructor usage
rg -n 'DeploymentDependencyRule\s*{' --type go
rg -n 'NewDeploymentDependencyRule\|DeploymentDependencyRule{' --type go
# Also check for any other schema/spec files
fd -e json -e yaml -e yml | xargs rg -l 'DeploymentDependencyRule' 2>/dev/nullRepository: ctrlplanedev/cli
Length of output: 381
🏁 Script executed:
#!/bin/bash
# Search for DeploymentDependencyRule usage in Go files
rg -n 'DeploymentDependencyRule' --type go -A 3 -B 1
# Also check if there are any builders that reference the old field
rg -n 'dependsOnDeploymentSelector|DependsOn' --type goRepository: ctrlplanedev/cli
Length of output: 1458
Update the OpenAPI specification to match the code change.
DeploymentDependencyRule.DependsOn has been changed to a string in the generated client code, but openapi.json still defines the old schema with dependsOnDeploymentSelector as a Selector object. Update the spec to define dependsOn as a string (CEL expression) instead. This is critical for API contract consistency.
🤖 Prompt for AI Agents
In `@internal/api/client.gen.go` around lines 351 - 352, The OpenAPI spec must be
updated to match the generated client: replace the old
dependsOnDeploymentSelector: Selector object with a dependsOn field of type
string (CEL expression) in the DeploymentDependencyRule schema in openapi.json;
update any schema definitions, property names, examples, and references that
still use dependsOnDeploymentSelector or Selector to reference dependsOn
(string) so DeploymentDependencyRule.DependsOn in the client and the spec are
consistent.
2 participants
Summary by CodeRabbit
New Features
API Changes