Skip to content

refactor: architecture hardening + template edit command#34

Merged
hi-lei merged 15 commits into
mainfrom
refactor/wizard-and-template
Apr 9, 2026
Merged

refactor: architecture hardening + template edit command#34
hi-lei merged 15 commits into
mainfrom
refactor/wizard-and-template

Conversation

@hi-lei
Copy link
Copy Markdown
Collaborator

@hi-lei hi-lei commented Apr 9, 2026

Summary

  • Correctness fixes: Surface SSH key resolution warnings, fix os.Stderr bypass, fix context.Background() in pricing cache, fix hostname validation message, reject trailing hyphens in template names, add JSON tags to Entry, fix duplicate {random} expansion
  • Structural refactors: Extract WithSpinner helper (removes 11 boilerplate sites), extract shared fetchInstances, extract TemplatesBaseDir, split wizard.go (1355→776 lines), document createOptions lifecycle, parallelize volume fetching, atomic template writes
  • Test infrastructure: httptest-based mock API harness, runCreate orchestration tests, action tests, template command tests
  • Template UX: New template edit command with field menu, interactive picker for show/delete, show displays all fields

Details

11 commits, 30 files changed, +2015 / -870 lines

Phase A: Correctness (6 fixes)

Fix Impact
SSH key/script resolution warnings Users now see warnings instead of silent data loss
renderDeploymentSummaryio.Writer Consistent with IOStreams abstraction
ensurePricingCache → parent context Ctrl+C properly cancels API calls
Hostname error message "hyphens and underscores" → "hyphens"
Template name trailing hyphens Rejected, consistent with normalizeName
Entry JSON tags + {random} dedup Consistent -o json output, unique random words

Phase B: Structure (7 refactors)

Refactor Before → After
WithSpinner helper 11 boilerplate sites → cmdutil.WithSpinner
fetchInstances 3 duplicate fetch+filter → shared helper
TemplatesBaseDir 5 duplicate path constructions → 1 helper
wizard.go split 1355 lines → 776 + cache/subflows/summary
createOptions docs Implicit → documented 5-stage lifecycle
Volume fetching Sequential N+1 → parallel (max 5)
Template writes Direct write → atomic temp+rename

Phase C: Tests

  • testutil_test.go: httptest mock API server + TestFactory
  • create_test.go: 4 runCreate orchestration tests
  • action_test.go: availableActions + filterByStatus tests
  • command_test.go: normalizeName, parseRef, vmResultToTemplate

Template UX

  • verda template edit [vm/name] — field menu to change specific values
  • verda template show / verda template delete — interactive picker when no arg
  • verda template show — displays all fields including hostname pattern, skip flags

Test plan

  • make test — all packages pass
  • make lint — 0 issues
  • Manual: verda template edit — field menu works, saves correctly
  • Manual: verda template show — all fields displayed
  • Manual: verda template delete — interactive picker + confirmation

🤖 Generated with Claude Code

hi-lei and others added 15 commits April 9, 2026 15:16
Previously, API errors during template name resolution were silently
swallowed. If the wizard was skipped (all fields pre-filled), this could
produce VMs with no SSH keys and no user warning.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… pricing cache

renderDeploymentSummary now accepts an io.Writer parameter instead of
writing directly to os.Stderr, consistent with the IOStreams abstraction.

ensurePricingCache now receives the parent context instead of creating
context.Background(), so Ctrl+C properly cancels in-flight API calls.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Error message said 'hyphens and underscores' but underscores are
actually rejected. Updated to match actual validation behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…random} dedup

- ValidateName now rejects names ending with hyphens, consistent with
  normalizeName which already strips them.
- Entry struct now serializes with lowercase keys in -o json mode.
- Multiple {random} placeholders now generate different words each time.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the generic withSpinner[T] function from wizard.go to cmd/util/spinner.go
as exported WithSpinner and RunWithSpinner helpers. Replace all 11 manual
spinner start/stop boilerplate sites across vm command files with calls to
the shared helpers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidates 3 duplicate fetch+filter patterns into a single helper
with spinner support. Removes filterByValidFrom in favor of shared
filterByStatus.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces 5 duplicate filepath.Join(verdaDir, "templates") constructions
with a single centralized helper.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ons lifecycle

wizard.go: step definitions + flow builder (~776 lines)
wizard_cache.go: apiCache, pricing helpers, instance type utils
wizard_subflows.go: SSH key, startup script, storage interactive flows
wizard_summary.go: deployment summary rendering

Also adds comprehensive lifecycle documentation to createOptions struct.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- fetchInstanceVolumes now uses goroutines with bounded concurrency
  (max 5) instead of sequential N+1 queries.
- Template Save now writes to a .tmp file then renames, preventing
  corruption if the process is interrupted mid-write.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- testutil_test.go: httptest-based test harness with mock API server,
  TestFactory in agent mode, and captured IOStreams.
- create_test.go: runCreate orchestration tests for agent-mode missing
  flags, happy path with all flags, template pre-fill, and no-client error.
- action_test.go: Tests availableActions for offline/running/provisioning
  and filterByStatus with multiple scenarios.
- command_test.go: Tests normalizeName, parseRef, and vmResultToTemplate.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- template edit: field menu to change specific template values. Supports
  interactive picker when no argument given. API-backed fields (instance
  type, location, image, SSH keys, startup script) fetch live choices.
- template show: now displays all fields including hostname_pattern,
  storage_skip, startup_script_skip. Shows "-" for unset, "None (skipped)"
  for explicitly skipped fields.
- template show/delete: optional arg — interactive picker when omitted.
- wizard: add Default functions to billing-type, contract, kind,
  instance-type, image steps for better pre-selection.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reflects architecture hardening refactors: wizard split into 4 files,
shared helpers (WithSpinner, fetchInstances, TemplatesBaseDir), new
template edit command, interactive pickers, createOptions lifecycle,
atomic writes, parallel volume fetching, and warning-based name resolution.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The --all flag now includes all command directories, not just the
original subset.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hi-lei hi-lei merged commit dfa9753 into main Apr 9, 2026
13 checks passed
@hi-lei hi-lei deleted the refactor/wizard-and-template branch April 9, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant