thaloco/banger
1
Fork 0
Code Pull requests Releases Packages Activity Actions

cli: delete vm run's dead import path + duplicated git inspection

Browse source 
The CLI carried a full second copy of the workspace import
implementation that `vm run` never actually used:

  - importVMRunRepoToGuest (no callers — the live flow calls the
    daemon's PrepareVMWorkspace RPC instead)
  - prepareVMRunRepoCopy, vmRunCheckoutCommit, vmRunCheckoutScript,
    gitFileURL, runHostCommand (all reachable only from the dead
    importVMRunRepoToGuest)

Plus a duplicated repo-inspection surface that shadowed the
daemon's:

  - inspectVMRunRepo ran every git query the daemon re-ran during
    workspace.prepare (HEAD, branch, identity, origin, overlay list)
  - gitOutput / gitTrimmedOutput / gitResolvedConfigValue /
    parseNullSeparatedOutput / listSubmodules / listOverlayPaths /
    resolveVMRunSourcePath — all identical to the exported
    workspace.* versions
  - vmRunRepoSpec — same fields as workspace.RepoSpec

Replaced with a single minimal preflight:

  func vmRunPreflightRepo(ctx, rawPath) (absPath, err error)

The preflight only checks what the user can fix locally before
banger creates a VM (path exists, sits in a non-bare git repo, no
submodules). The daemon's workspace.prepare RPC does the full
inspection — and returns RepoRoot + RepoName in the response, which
the CLI now threads into the tooling harness instead of computing
them a second time.

Signature changes:

  runVMRun(ctx, ..., *vmRunRepo, ...)   // was: *vmRunRepoSpec
  startVMRunToolingHarness(ctx, client, repoRoot, repoName, progress)
                                        // was: (ctx, client, spec, progress)
  vmRunToolingHarnessScript(plan)       // was: (spec, plan)
  vmRunToolingHarnessLaunchScript(repoName)  // was: (spec)

Tests: the CLI-side git-inspection tests are replaced by a single
TestVMRunPreflightRejectsSubmodules that exercises the preflight.
Everything else (tooling harness script, progress renderer, SSH args,
runVMRun flows) keeps working. The shallow-copy / checkout-script
tests are gone — that code now lives only in
internal/daemon/workspace and is tested there.

Also fixed a latent bug the refactor exposed: vm run's --from flag
defaults to "HEAD", which the daemon reads as "from without branch"
and rejects. CLI now scrubs fromRef when branchName is empty.

Live verified: `banger vm run --name X . -- cmd` boots, workspace
materialises at /root/repo with matching HEAD, exit code propagates.
This commit is contained in:
Thales Maciel 2026-04-19 17:01:26 -03:00
parent ae14b9499d
commit 3a5f4cd40d
No known key found for this signature in database
GPG key ID: 33112E6833C34679
2 changed files with 112 additions and 498 deletions
Download patch file Download diff file Expand all files Collapse all files

200
internal/cli/cli_test.go
View file

@ -16,6 +16,7 @@ import (
"banger/internal/api"
"banger/internal/buildinfo"
"banger/internal/daemon/workspace"
"banger/internal/model"
"banger/internal/system"
"banger/internal/toolingplan"
@ -1133,111 +1134,21 @@ func TestValidateSSHPrereqsFailsForMissingKey(t *testing.T) {
}
}
func TestResolveVMRunSourcePathDefaultsToCWD(t *testing.T) {
origCWD := cwdFunc
t.Cleanup(func() {
cwdFunc = origCWD
})
// CLI-side git inspection moved to internal/daemon/workspace; the
// CLI now runs only a minimal preflight. Those tests live in the
// workspace package. What we still guard here is the preflight
// policy: reject submodules before the VM is created so the user
// gets a fast error instead of an orphaned VM.
want := t.TempDir()
cwdFunc = func() (string, error) {
return want, nil
}
got, err := resolveVMRunSourcePath("")
if err != nil {
t.Fatalf("resolveVMRunSourcePath: %v", err)
}
if got != want {
t.Fatalf("resolveVMRunSourcePath() = %q, want %q", got, want)
}
}
func TestInspectVMRunRepoUsesRepoRootAndOverlayPaths(t *testing.T) {
if _, err := exec.LookPath("git"); err != nil {
t.Skip("git not installed")
}
repoRoot := t.TempDir()
globalConfigPath := filepath.Join(t.TempDir(), "global.gitconfig")
t.Setenv("GIT_CONFIG_GLOBAL", globalConfigPath)
testRunGit(t, repoRoot, "config", "--global", "user.email", "global@example.com")
testRunGit(t, repoRoot, "config", "--global", "user.name", "Global User")
testRunGit(t, repoRoot, "init")
testRunGit(t, repoRoot, "remote", "add", "origin", "https://example.com/repo.git")
testRunGit(t, repoRoot, "config", "user.email", "test@example.com")
testRunGit(t, repoRoot, "config", "user.name", "Banger Test")
if err := os.MkdirAll(filepath.Join(repoRoot, "dir"), 0o755); err != nil {
t.Fatalf("MkdirAll(dir): %v", err)
}
if err := os.WriteFile(filepath.Join(repoRoot, ".gitignore"), []byte("ignored.txt\n"), 0o644); err != nil {
t.Fatalf("WriteFile(.gitignore): %v", err)
}
if err := os.WriteFile(filepath.Join(repoRoot, "tracked.txt"), []byte("tracked\n"), 0o644); err != nil {
t.Fatalf("WriteFile(tracked.txt): %v", err)
}
if err := os.WriteFile(filepath.Join(repoRoot, "dir", "keep.txt"), []byte("keep\n"), 0o644); err != nil {
t.Fatalf("WriteFile(keep.txt): %v", err)
}
testRunGit(t, repoRoot, "add", ".")
testRunGit(t, repoRoot, "commit", "-m", "init")
testRunGit(t, repoRoot, "checkout", "-b", "trunk")
if err := os.WriteFile(filepath.Join(repoRoot, "tracked.txt"), []byte("tracked local\n"), 0o644); err != nil {
t.Fatalf("WriteFile(tracked.txt local): %v", err)
}
if err := os.WriteFile(filepath.Join(repoRoot, "untracked.txt"), []byte("untracked\n"), 0o644); err != nil {
t.Fatalf("WriteFile(untracked.txt): %v", err)
}
if err := os.WriteFile(filepath.Join(repoRoot, "ignored.txt"), []byte("ignored\n"), 0o644); err != nil {
t.Fatalf("WriteFile(ignored.txt): %v", err)
}
spec, err := inspectVMRunRepo(context.Background(), filepath.Join(repoRoot, "dir"), "", "HEAD")
if err != nil {
t.Fatalf("inspectVMRunRepo: %v", err)
}
if spec.RepoRoot != repoRoot {
t.Fatalf("RepoRoot = %q, want %q", spec.RepoRoot, repoRoot)
}
if spec.RepoName != filepath.Base(repoRoot) {
t.Fatalf("RepoName = %q, want %q", spec.RepoName, filepath.Base(repoRoot))
}
if spec.CurrentBranch != "trunk" {
t.Fatalf("CurrentBranch = %q, want trunk", spec.CurrentBranch)
}
if spec.HeadCommit == "" {
t.Fatal("HeadCommit should not be empty")
}
if spec.BaseCommit != spec.HeadCommit {
t.Fatalf("BaseCommit = %q, want head %q", spec.BaseCommit, spec.HeadCommit)
}
if spec.OriginURL != "https://example.com/repo.git" {
t.Fatalf("OriginURL = %q, want https://example.com/repo.git", spec.OriginURL)
}
if spec.GitUserName != "Banger Test" {
t.Fatalf("GitUserName = %q, want Banger Test", spec.GitUserName)
}
if spec.GitUserEmail != "test@example.com" {
t.Fatalf("GitUserEmail = %q, want test@example.com", spec.GitUserEmail)
}
wantOverlay := []string{".gitignore", "dir/keep.txt", "tracked.txt", "untracked.txt"}
if !reflect.DeepEqual(spec.OverlayPaths, wantOverlay) {
t.Fatalf("OverlayPaths = %v, want %v", spec.OverlayPaths, wantOverlay)
}
}
func TestInspectVMRunRepoRejectsSubmodules(t *testing.T) {
func TestVMRunPreflightRejectsSubmodules(t *testing.T) {
repoRoot := t.TempDir()
origHostCommandOutput := hostCommandOutputFunc
origHostCommandOutput := workspace.HostCommandOutputFunc
t.Cleanup(func() {
hostCommandOutputFunc = origHostCommandOutput
workspace.HostCommandOutputFunc = origHostCommandOutput
})
hostCommandOutputFunc = func(ctx context.Context, name string, args ...string) ([]byte, error) {
workspace.HostCommandOutputFunc = func(ctx context.Context, name string, args ...string) ([]byte, error) {
t.Helper()
if name != "git" {
t.Fatalf("command = %q, want git", name)
@ -1255,9 +1166,9 @@ func TestInspectVMRunRepoRejectsSubmodules(t *testing.T) {
}
}
_, err := inspectVMRunRepo(context.Background(), repoRoot, "", "HEAD")
_, err := vmRunPreflightRepo(context.Background(), repoRoot)
if err == nil || !strings.Contains(err.Error(), "submodules") {
t.Fatalf("inspectVMRunRepo() error = %v, want submodule rejection", err)
t.Fatalf("vmRunPreflightRepo() error = %v, want submodule rejection", err)
}
}
@ -1321,7 +1232,7 @@ func TestRunVMRunWorkspacePreparesAndAttaches(t *testing.T) {
var workspaceParams api.VMWorkspacePrepareParams
vmWorkspacePrepareFunc = func(ctx context.Context, socketPath string, params api.VMWorkspacePrepareParams) (api.VMWorkspacePrepareResult, error) {
workspaceParams = params
return api.VMWorkspacePrepareResult{Workspace: model.WorkspacePrepareResult{VMID: vm.ID, GuestPath: "/root/repo"}}, nil
return api.VMWorkspacePrepareResult{Workspace: model.WorkspacePrepareResult{VMID: vm.ID, GuestPath: "/root/repo", RepoName: "repo", RepoRoot: "/tmp/repo"}}, nil
}
buildVMRunToolingPlanFunc = func(context.Context, string) toolingplan.Plan {
return toolingplan.Plan{
@ -1338,10 +1249,7 @@ func TestRunVMRunWorkspacePreparesAndAttaches(t *testing.T) {
return api.VMHealthResult{Name: "devbox", Healthy: false}, nil
}
spec := vmRunRepoSpec{
SourcePath: repoRoot, RepoRoot: repoRoot, RepoName: "repo",
HeadCommit: "deadbeef", CurrentBranch: "main",
}
repo := vmRunRepo{sourcePath: repoRoot}
var stdout, stderr bytes.Buffer
err := runVMRun(
context.Background(),
@ -1350,7 +1258,7 @@ func TestRunVMRunWorkspacePreparesAndAttaches(t *testing.T) {
strings.NewReader(""),
&stdout, &stderr,
api.VMCreateParams{Name: "devbox"},
&spec,
&repo,
nil,
false,
)
@ -1425,7 +1333,7 @@ func TestVMRunPrintsPostCreateProgress(t *testing.T) {
return &testVMRunGuestClient{}, nil
}
vmWorkspacePrepareFunc = func(ctx context.Context, socketPath string, params api.VMWorkspacePrepareParams) (api.VMWorkspacePrepareResult, error) {
return api.VMWorkspacePrepareResult{Workspace: model.WorkspacePrepareResult{VMID: vm.ID, GuestPath: "/root/repo"}}, nil
return api.VMWorkspacePrepareResult{Workspace: model.WorkspacePrepareResult{VMID: vm.ID, GuestPath: "/root/repo", RepoName: "repo", RepoRoot: "/tmp/repo"}}, nil
}
sshExecFunc = func(context.Context, io.Reader, io.Writer, io.Writer, []string) error {
return nil
@ -1434,7 +1342,7 @@ func TestVMRunPrintsPostCreateProgress(t *testing.T) {
return api.VMHealthResult{Name: "devbox", Healthy: false}, nil
}
spec := vmRunRepoSpec{RepoRoot: t.TempDir(), RepoName: "repo", HeadCommit: "deadbeef"}
repo := vmRunRepo{sourcePath: t.TempDir()}
var stdout, stderr bytes.Buffer
err := runVMRun(
context.Background(),
@ -1443,7 +1351,7 @@ func TestVMRunPrintsPostCreateProgress(t *testing.T) {
strings.NewReader(""),
&stdout, &stderr,
api.VMCreateParams{Name: "devbox"},
&spec,
&repo,
nil,
false,
)
@ -1515,7 +1423,7 @@ func TestRunVMRunWarnsWhenToolingHarnessStartFails(t *testing.T) {
return fakeClient, nil
}
vmWorkspacePrepareFunc = func(ctx context.Context, socketPath string, params api.VMWorkspacePrepareParams) (api.VMWorkspacePrepareResult, error) {
return api.VMWorkspacePrepareResult{Workspace: model.WorkspacePrepareResult{VMID: vm.ID, GuestPath: "/root/repo"}}, nil
return api.VMWorkspacePrepareResult{Workspace: model.WorkspacePrepareResult{VMID: vm.ID, GuestPath: "/root/repo", RepoName: "repo", RepoRoot: "/tmp/repo"}}, nil
}
sshExecCalls := 0
sshExecFunc = func(context.Context, io.Reader, io.Writer, io.Writer, []string) error {
@ -1526,7 +1434,7 @@ func TestRunVMRunWarnsWhenToolingHarnessStartFails(t *testing.T) {
return api.VMHealthResult{Healthy: false}, nil
}
spec := vmRunRepoSpec{RepoRoot: t.TempDir(), RepoName: "repo", HeadCommit: "deadbeef"}
repo := vmRunRepo{sourcePath: t.TempDir()}
var stdout, stderr bytes.Buffer
err := runVMRun(
context.Background(),
@ -1535,7 +1443,7 @@ func TestRunVMRunWarnsWhenToolingHarnessStartFails(t *testing.T) {
strings.NewReader(""),
&stdout, &stderr,
api.VMCreateParams{Name: "devbox"},
&spec,
&repo,
nil,
false,
)
@ -1892,7 +1800,7 @@ func TestSplitVMRunArgsPartitionsOnDash(t *testing.T) {
}
func TestVMRunToolingHarnessScriptUsesMiseOnly(t *testing.T) {
script := vmRunToolingHarnessScript(vmRunRepoSpec{RepoName: "repo"}, toolingplan.Plan{
script := vmRunToolingHarnessScript(toolingplan.Plan{
RepoManagedTools: []string{"node"},
Steps: []toolingplan.InstallStep{{Tool: "go", Version: "1.25.0", Source: "go.mod"}},
Skips: []toolingplan.SkipNote{{Target: "python", Reason: "no .python-version"}},
@ -1915,68 +1823,10 @@ func TestVMRunToolingHarnessScriptUsesMiseOnly(t *testing.T) {
}
}
}
func TestPrepareVMRunRepoCopyCreatesShallowMetadataCopy(t *testing.T) {
if _, err := exec.LookPath("git"); err != nil {
t.Skip("git not installed")
}
repoRoot := t.TempDir()
testRunGit(t, repoRoot, "init")
testRunGit(t, repoRoot, "remote", "add", "origin", "https://example.com/repo.git")
for i := 0; i < 12; i++ {
name := fmt.Sprintf("file-%02d.txt", i)
if err := os.WriteFile(filepath.Join(repoRoot, name), []byte(fmt.Sprintf("commit-%02d\n", i)), 0o644); err != nil {
t.Fatalf("WriteFile(%s): %v", name, err)
}
testRunGit(t, repoRoot, "add", name)
testRunGit(t, repoRoot, "commit", "-m", fmt.Sprintf("commit-%02d", i))
}
baseCommit := strings.TrimSpace(testRunGit(t, repoRoot, "rev-parse", "HEAD~5"))
repoCopyDir, cleanup, err := prepareVMRunRepoCopy(context.Background(), vmRunRepoSpec{
RepoRoot: repoRoot,
RepoName: "repo",
BranchName: "feature",
BaseCommit: baseCommit,
HeadCommit: strings.TrimSpace(testRunGit(t, repoRoot, "rev-parse", "HEAD")),
OriginURL: "https://example.com/repo.git",
OverlayPaths: []string{"file-11.txt"},
})
if err != nil {
t.Fatalf("prepareVMRunRepoCopy: %v", err)
}
defer cleanup()
entries, err := os.ReadDir(repoCopyDir)
if err != nil {
t.Fatalf("ReadDir(repoCopyDir): %v", err)
}
if len(entries) != 1 || entries[0].Name() != ".git" {
t.Fatalf("repo copy entries = %v, want only .git", entries)
}
if got := strings.TrimSpace(testRunGit(t, repoCopyDir, "rev-parse", "--is-shallow-repository")); got != "true" {
t.Fatalf("is-shallow-repository = %q, want true", got)
}
if got := strings.TrimSpace(testRunGit(t, repoCopyDir, "config", "--get", "remote.origin.url")); got != "https://example.com/repo.git" {
t.Fatalf("remote.origin.url = %q, want https://example.com/repo.git", got)
}
if _, err := exec.Command("git", "-C", repoCopyDir, "cat-file", "-e", baseCommit+"^{commit}").CombinedOutput(); err != nil {
t.Fatalf("cat-file -e %s^{commit}: %v", baseCommit, err)
}
}
func TestVMRunCheckoutScriptSkipsRepoGitIdentityWhenIncomplete(t *testing.T) {
script := vmRunCheckoutScript(vmRunRepoSpec{
RepoName: "repo",
HeadCommit: "deadbeef",
CurrentBranch: "main",
GitUserName: "Repo User",
})
if strings.Contains(script, `git -C "$DIR" config user.name`) || strings.Contains(script, `git -C "$DIR" config user.email`) {
t.Fatalf("script = %q, want no repo-local git identity commands", script)
}
}
// The shallow-repo-copy + checkout-script paths used to live in the
// CLI. They now live in internal/daemon/workspace and are exercised
// by that package's tests; no need to duplicate here.
func TestVMRunGuestDirIsFixed(t *testing.T) {
if got := vmRunGuestDir(); got != "/root/repo" {