refactor: full-codebase simplify pass — reuse + quality

[CMGS]

Summary

Audited 200+ Go files via 5 parallel agents (reuse / quality / efficiency / modernize / generics). Applied the Tier 1 findings — high-confidence fixes with bounded scope and zero behavior change. Skipped findings that were already-accepted trade-offs (config.json re-reads on clone, accepted in PR #27 Copilot review), or separate-PR scope (generic DoAPIJSON[T] across CH/FC, status event-loop dedup, IndexLayout helper across 4 backends), or churn-beyond-value (info.Hypervisor JSON tag — touches 5+ construction sites for cosmetic gain).

What changed

Reuse (cross-file dedup)

• utils.ReadJSONFile(path, v) — 5+ os.ReadFile + json.Unmarshal duplicates collapsed. Sites:
  • snapshot/envelope.go::ReadSnapshotEnvelope
  • hypervisor/snapshot.go::LoadSnapshotMeta
  • hypervisor/cloudhypervisor/clone.go::parseCHConfig
• errors.Is(err, fs.ErrNotExist) still works through %w so callers (e.g. ReadSnapshotEnvelope → ErrEnvelopeMissing) keep their sentinel mapping.

Quality

• Drop dead VerifyProcess + verifyProcessExe (utils/process.go, process_linux.go, process_other.go). All production callers go through VerifyProcessCmdline with a non-empty expectArg (sockPath); the bare variant was unreachable. Tests of dead code removed.
• types.ImageTypeOCI / types.ImageTypeCloudImg constants replace bare "oci" / "cloudimg" string comparisons in cmd/core/helpers.go::digestPullRef and cmd/vm/run.go::printPostCloneHints. Backend typ consts in images/oci and images/cloudimg now reference the public constant.
• cloneResumeOpts cleanup (hypervisor/cloudhypervisor/clone.go): drops 5 fields (cpu, diskQueueSize, noDirectIO, windows, onDemand) that were just transcribed from vmCfg. Pass *types.VMConfig directly; restoreAndResumeClone reads the originals via opts.vmCfg.*.

Modernize / Generics

Both audits returned zero actionable findings. Codebase already uses slices, maps, cmp.Or, errors.Join, range-over-int idiomatically. Existing generics (Store[T], ResolveRef[T], ForEach[T]) cover the structural-dedup space; remaining candidates are either single-use, semantically distinct per type, or the boxed-any was an intentional encoder sink.

Net diff

13 files changed, 36 insertions(+), 106 deletions(-)

Test plan

• make lint clean on darwin + linux
• go test ./... passes (incl. utils/process_test.go after dead-test removal)
• Testbed regression suite: 25/25 PASS
  • lifecycle (run/inspect/exec/exit-code/stdin/env)
  • vm logs (oneshot, follow, follow with truncate-regrow detection)
  • snapshot save/list/clone/restore (resource flags rejected, defaults inherited)
  • network (--nics 0)
  • help text invariants

Skipped findings (separate PRs / accepted trade-offs)

• Efficiency F1/F2 (multi-read of config.json / cocoon.json on clone/restore) — Copilot review on PR refactor: clone/restore inherit all resources from snapshot #27 accepted the trade-off (~1 KB cached, sub-ms, clone is cold path).
• Generics F1 (utils.DoAPIJSON[T] for HTTP+JSON pattern across CH/FC) — bigger refactor, separate PR.
• Quality F13 (statusEventLoop / statusEventLoopJSON near-duplicates) — needs design for shared diff abstraction, separate PR.
• Quality F12 (info.Hypervisor JSON tag) — would touch 5+ construction sites for cosmetic gain.
• Reuse F2 (IndexLayout helper across 4 backends) — embedding shapes differ, marginal win.

[Copilot]

Pull request overview

This PR performs a codebase-wide simplification/refactor pass focused on deduplication and consistency, while aiming to keep runtime behavior unchanged.

Changes:

• Added utils.ReadJSONFile and refactored multiple JSON read+unmarshal call sites to use it.
• Removed dead VerifyProcess / verifyProcessExe logic and related tests, leaving VerifyProcessCmdline as the single entry point.
• Centralized image backend type strings via types.ImageTypeOCI / types.ImageTypeCloudImg and updated call sites to use the constants.
• Simplified Cloud Hypervisor clone resume options by passing *types.VMConfig instead of copying multiple fields.

Reviewed changes

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

Show a summary per file

File	Description
utils/process.go	Removes VerifyProcess and simplifies VerifyProcessCmdline behavior.
utils/process_test.go	Deletes tests for removed dead APIs; keeps cmdline-verification tests.
utils/process_other.go	Removes now-unused non-Linux verifyProcessExe stub.
utils/process_linux.go	Removes now-unused Linux verifyProcessExe implementation and related import.
utils/atomic.go	Adds ReadJSONFile helper for shared JSON file loading.
types/config.go	Introduces exported image type constants used across the codebase.
snapshot/envelope.go	Refactors snapshot envelope loading to use utils.ReadJSONFile.
hypervisor/snapshot.go	Refactors snapshot metadata loading to use utils.ReadJSONFile.
hypervisor/cloudhypervisor/clone.go	Simplifies clone resume options and refactors CH config parsing to use utils.ReadJSONFile.
images/oci/oci.go	Replaces hardcoded "oci" type string with types.ImageTypeOCI.
images/cloudimg/cloudimg.go	Replaces hardcoded "cloudimg" type string with types.ImageTypeCloudImg.
cmd/vm/run.go	Uses types.ImageTypeCloudImg constant in post-clone hint logic.
cmd/core/helpers.go	Uses types.ImageTypeOCI constant in OCI digest pinning logic.

Comments suppressed due to low confidence (1)

utils/process.go:57

• VerifyProcessCmdline no longer special-cases expectArg == "". On Linux, verifyProcessCmdline uses strings.Contains(cmdline, expectArg), so an empty expectArg always passes and the only remaining check is a substring match on binaryName, which is weaker than the previous /proc/{pid}/exe basename check. Consider restoring a strict executable-name verification path for expectArg == "" (e.g., readlink(/proc/pid/exe) or exact argv[0] basename), or explicitly reject empty expectArg and update callers/docs accordingly to avoid misidentifying/killing the wrong process.

// VerifyProcessCmdline checks binary name and that expectArg appears in
// /proc/{pid}/cmdline. This prevents cross-instance misidentification when
// multiple processes of the same binary are running (e.g. multiple VMs).
// On non-Linux platforms, falls back to IsProcessAlive.
func VerifyProcessCmdline(pid int, binaryName, expectArg string) bool {
	if pid <= 0 {
		return false
	}
	if match, ok := verifyProcessCmdline(pid, binaryName, expectArg); ok {
		return match
	}
	return IsProcessAlive(pid)
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

[Copilot]

@CMGS Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub.

You can ask me to try again later by mentioning me in a new comment.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: cc357837-c54f-4735-b2dc-4a7249d3fa30

Sorry for the inconvenience!