fix(definition): update-chart preserves all fields + add --repository-url

[omattsson]

Summary

stackctl definition update-chart was silently wiping repository_url, chart_path, build_pipeline_id, and deploy_order on every call because the request body type (UpdateChartConfigRequest) only carried 4 of the 8 fields the backend stores, and the command's request builder seeded only 4 fields from current (the GET response). Backends that do full-replace PUT then drop the missing fields to zero.

Repro (against backend < the companion fix)

stackctl definition update-chart <def> <chart> --chart-version 0.1.2

• before: repository_url=oci://acr/helm, chart_path=kvk-x, deploy_order=8
• after: repository_url="", chart_path="", deploy_order=0

Subsequent stack deploys failed with helm's non-absolute URLs should be in form of repo_name/path_to_chart because there was no repository to fetch from.

Changes

• pkg/types: UpdateChartConfigRequest now includes every field the backend's ChartConfig stores (repository_url, build_pipeline_id, plus the originals). All omitempty so an absent flag stays out of the JSON body when paired with a merge-safe backend.

• cmd/definition: update-chart seeds req from every field on the current GetDefinitionChart response (including RepoURL, ChartPath, BuildPipelineID, DeployOrder). User flags then override individual fields. DeployOrder is always non-nil because the backend's int is zero-valid (deploy_order=0 is meaningful).

• Adds --repository-url flag. Without this, an empty repository_url that the user inherited from a prior bug (or wants to set explicitly) has no way to be set via stackctl.

• Updates the at-least-one-flag validation to include the new flag.

The companion k8s-stack-manager PR makes the backend merge-safe so older stackctl clients stop wiping fields. Both fixes are needed long-term — backend safety against any client, plus stackctl carrying the full record so PUTs are explicit.

Test plan

• go test ./... — all pass
• Live: update-chart --chart-version 0.1.2 --repository-url oci://... on a previously-wiped chart restores both fields
• Live: helm install after the update-chart pulls the new chart version without registry errors

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added --repository-url to definition update-chart, with URL parsing/validation for supported schemes.
  • Added stronger GET-then-PUT behavior to preserve more chart configuration fields (including repository URL and build pipeline ID).
• Bug Fixes
  • Update-chart requests now more reliably round-trip existing values and prevent clearing fields by omitting empty strings.
  • Improved argument validation to require at least one relevant input flag/file.
• Tests
  • Expanded tests for --repository-url (valid/invalid) and strengthened assertions across PUT request bodies.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bd7ef302-2e9e-4dab-ae93-d9116bfbce11

📥 Commits

Reviewing files that changed from the base of the PR and between b03a50f and e2ffbc8.

📒 Files selected for processing (1)

• cli/cmd/definition.go

🚧 Files skipped from review as they are similar to previous changes (1)

• cli/cmd/definition.go

---

📝 Walkthrough

Walkthrough

Adds repository_url and build_pipeline_id to UpdateChartConfigRequest, wires --repository-url into definition update-chart with URL validation, seeds the PUT request from the fetched chart config, and expands tests for the new field handling.

Changes

Repository URL flag and request struct expansion

Layer / File(s)	Summary
UpdateChartConfigRequest struct expansion cli/pkg/types/types.go	Adds RepositoryURL and BuildPipelineID to UpdateChartConfigRequest and updates the comment to describe full-field GET-then-PUT updates with omitempty behavior.
CLI flag validation and request seeding cli/cmd/definition.go	Adds --repository-url, validates scheme and host for oci, http, and https, includes it in the required-flag check, seeds the request from the fetched chart config, and overrides RepositoryURL only when provided.
Chart update test coverage cli/cmd/definition_test.go	Expands sample chart config data, tightens round-trip assertions, adds valid and invalid --repository-url tests, and resets the new flags in cleanup handlers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: update-chart now preserves all fields and adds a new --repository-url flag.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/update-chart-preserves-fields

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@cli/cmd/definition.go`:
- Around line 431-434: The multi-line comment starting with "Seed every field
from the current record" contains misleading documentation about backend merge
semantics. Remove or revise the portion that explains how the backend handles
absent JSON keys (the part mentioning "Backend merges absent-from-JSON keys as
preserve"), as this describes follow-up behavior and is unsafe to document in
the current context. Keep only the explanation of why the full record is sent,
which is to prevent a full PUT replace from wiping values the user didn't intend
to change.

In `@cli/pkg/types/types.go`:
- Around line 632-645: The godoc comment for the UpdateChartConfigRequest struct
(lines 631-636) contains misleading information about JSON wire protocol
semantics. Remove the false claim that "explicit "" replaces" since omitempty
tags prevent empty strings from being sent as explicit clears. Replace those
lines with a clear explanation of whether the PUT endpoint performs full-field
replacement or partial merge, and explicitly document that clients are
responsible for seeding the request from the current state via GET-then-merge
before sending (as demonstrated by the client logic in definition.go). This
ensures the struct documentation accurately describes the actual PUT semantics
and wire contract.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1a526b8b-a08a-4a7c-9d36-52ec1b0ebdfe

📥 Commits

Reviewing files that changed from the base of the PR and between f1e4ba7 and 20bafdd.

📒 Files selected for processing (2)

• cli/cmd/definition.go
• cli/pkg/types/types.go

[Copilot]

Pull request overview

This pull request fixes stackctl definition update-chart so it no longer wipes backend-stored chart configuration fields during the GET-then-PUT update flow, and adds a new --repository-url flag to allow updating the Helm chart repository URL explicitly.

Changes:

• Expands types.UpdateChartConfigRequest to include the full set of backend chart config fields so updates can round-trip safely.
• Updates definition update-chart to seed all request fields from the current GET response before applying user-provided flag overrides.
• Adds --repository-url with basic URL/scheme validation and includes it in the “at least one flag” guard.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
cli/pkg/types/types.go	Extends UpdateChartConfigRequest to carry all backend chart config fields to prevent silent wiping on PUT.
cli/cmd/definition.go	Seeds PUT requests from the current chart config and adds/validates the new --repository-url flag.
cli/cmd/definition_test.go	Updates fixtures and adds assertions/tests to ensure fields round-trip and --repository-url validation works.