fix: variable update in non-interactive mode clears all existing variables

[eliajhmauve]

Summary

variable update -i=false -k "NEW=val" silently deletes all existing variables. This is a data-loss bug.

Root cause

Two issues combine to cause this:

1. runUpdateVariable() (update.go:44) unconditionally resets opts.keys with make(map[string]string), wiping the CLI-parsed --key values before they reach the handler
2. runUpdateVariableNonInteractive() passes this empty/partial map directly to UpdateVariables(), which is a full-replace API — unmentioned variables get deleted

Interactive mode is unaffected because runUpdateVariableInteractive fetches all existing variables and rebuilds the map before calling the non-interactive handler.

Fix

Follow the same fetch-and-merge pattern already used by variable create (create.go:115-127) and variable delete (delete.go:138-145):

1. Move make(map[string]string) into the interactive branch only (don't wipe CLI args)
2. Fetch existing variables before calling UpdateVariables
3. Merge user-provided keys on top of existing ones
4. Start the spinner before the API call (was missing s.Start())

Additional fixes in this PR

• variable create interactive: strings.Split(input, "=") → strings.SplitN(input, "=", 2) — values containing = (e.g., DATABASE_URL=postgres://host/db?sslmode=require) were rejected as invalid input
• Typo: varableDeleteCmd → variableDeleteCmd in variable.go

Test plan

• zeabur variable update -i=false --id <svc> -k "NEW_VAR=hello" should add NEW_VAR without affecting existing variables
• zeabur variable update -i=false --id <svc> -k "EXISTING=new_value" should update only that key
• Interactive variable update should still work as before
• zeabur variable create interactive with DATABASE_URL=postgres://host?ssl=require should succeed

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Variable creation now accepts values containing = characters.
  • Variable update now preserves existing variables and only modifies specified ones.
  • Improved error messaging for invalid variable input format.

[coderabbitai]

Walkthrough

Three changes to the variable command handling: the create command now accepts values containing = characters by splitting only on the first delimiter with an improved error message, the update command preserves existing variables during partial updates by fetching and merging with user changes, and a typo in the delete command package alias is fixed.

Changes

Cohort / File(s)	Summary
Input Parsing Enhancement internal/cmd/variable/create/create.go	Changed input parsing to split on the first = only, allowing = characters in the value portion. Error message improved from generic "invalid input" to "invalid input: expected KEY=VALUE format".
Variable Preservation Refactoring internal/cmd/variable/update/update.go	Refactored update flow to preserve existing variables: removed unconditional initialization of opts.keys, moved initialization to interactive path only, and added logic in non-interactive handler to fetch existing variables, merge them with user-provided changes, and persist the merged set.
Package Alias Typo Fix internal/cmd/variable/variable.go	Corrected typo in package alias from varableDeleteCmd to variableDeleteCmd and updated corresponding cmd.AddCommand() call.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main bug fix in the PR: preventing non-interactive variable updates from clearing existing variables. It accurately summarizes the critical data-loss issue being resolved.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/cmd/variable/update/update.go (1)

107-118: Consider adding validation for empty keys in non-interactive mode.

When --key is not provided, opts.keys will be nil, and the update becomes a no-op (existing vars are fetched, nothing is merged, and the same vars are written back). This is safe but potentially confusing for users.

💡 Optional: Add early validation

 func runUpdateVariableNonInteractive(f *cmdutil.Factory, opts *Options) error {
+	if len(opts.keys) == 0 {
+		return fmt.Errorf("--key is required in non-interactive mode")
+	}
+
 	if opts.id == "" && opts.name != "" {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/cmd/variable/update/update.go` around lines 107 - 118, Add an early
validation in runUpdateVariableNonInteractive to error when no keys are provided
in non-interactive mode: check opts.keys (or the single-key flag if applicable)
after resolving opts.id and if it is nil/empty return a clear error like "--key
is required when not running interactively". Update the function
runUpdateVariableNonInteractive to perform this check before fetching/merging
variables so the command fails fast and avoids a no-op update.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/cmd/variable/update/update.go`:
- Around line 107-118: Add an early validation in
runUpdateVariableNonInteractive to error when no keys are provided in
non-interactive mode: check opts.keys (or the single-key flag if applicable)
after resolving opts.id and if it is nil/empty return a clear error like "--key
is required when not running interactively". Update the function
runUpdateVariableNonInteractive to perform this check before fetching/merging
variables so the command fails fast and avoids a no-op update.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98ecaf3c-dc9b-4d6c-a992-ba55a39dead5

📥 Commits

Reviewing files that changed from the base of the PR and between 4f1eb83 and 0ff2f44.

📒 Files selected for processing (3)

• internal/cmd/variable/create/create.go
• internal/cmd/variable/update/update.go
• internal/cmd/variable/variable.go