diff --git a/internal/cli/commands_vm.go b/internal/cli/commands_vm.go index d4f010a..21618f7 100644 --- a/internal/cli/commands_vm.go +++ b/internal/cli/commands_vm.go @@ -625,9 +625,7 @@ func (d *deps) newVMWorkspacePrepareCommand() *cobra.Command { return err } if !includeUntracked { - if err := d.noteUntrackedSkipped(cmd.Context(), cmd.ErrOrStderr(), resolvedPath); err != nil { - return err - } + d.noteUntrackedSkipped(cmd.Context(), cmd.ErrOrStderr(), resolvedPath) } result, err := d.vmWorkspacePrepare(cmd.Context(), layout.SocketPath, api.VMWorkspacePrepareParams{ IDOrName: args[0], diff --git a/internal/cli/vm_run.go b/internal/cli/vm_run.go index e758b86..e3a049f 100644 --- a/internal/cli/vm_run.go +++ b/internal/cli/vm_run.go @@ -195,9 +195,7 @@ func (d *deps) runVMRun(ctx context.Context, socketPath string, cfg model.Daemon fromRef = repo.fromRef } if !repo.includeUntracked { - if err := d.noteUntrackedSkipped(ctx, stderr, repo.sourcePath); err != nil { - printVMRunWarning(stderr, fmt.Sprintf("count untracked files failed: %v", err)) - } + d.noteUntrackedSkipped(ctx, stderr, repo.sourcePath) } prepared, err := d.vmWorkspacePrepare(ctx, socketPath, api.VMWorkspacePrepareParams{ IDOrName: vmRef, diff --git a/internal/cli/workspace_preview.go b/internal/cli/workspace_preview.go index 15528c9..956d6ea 100644 --- a/internal/cli/workspace_preview.go +++ b/internal/cli/workspace_preview.go @@ -29,25 +29,33 @@ func (d *deps) runWorkspaceDryRun(ctx context.Context, out io.Writer, resolvedPa fmt.Fprintln(out, path) } if !includeUntracked { - if err := d.noteUntrackedSkipped(ctx, out, spec.RepoRoot); err != nil { - return err - } + d.noteUntrackedSkipped(ctx, out, spec.RepoRoot) } return nil } -// noteUntrackedSkipped prints a one-line notice to stderr-analog when -// the repo has untracked non-ignored files that will NOT be copied -// because --include-untracked was not passed. Silent when there are -// no such files, or when the count can't be determined. -func (d *deps) noteUntrackedSkipped(ctx context.Context, out io.Writer, repoRoot string) error { - count, err := d.repoInspector.CountUntrackedPaths(ctx, repoRoot) - if err != nil { - return err +// noteUntrackedSkipped prints a one-line notice when the repo holds +// untracked non-ignored files that will NOT be copied because +// --include-untracked was not passed. +// +// Best-effort: if sourcePath isn't inside a git repo, or git errors, +// or there are no untracked files, the helper stays silent. The +// notice is a courtesy — failing the whole operation over a courtesy +// would be worse than the notice being missing. +// +// Resolves sourcePath to the repo root internally via `git rev-parse +// --show-toplevel` so callers can pass whatever path the user typed. +// Before this helper normalised, subdir inputs ran `ls-files +// --others` scoped to the subdir, which silently underreported the +// skipped files the user needed to know about. +func (d *deps) noteUntrackedSkipped(ctx context.Context, out io.Writer, sourcePath string) { + repoRoot, err := d.repoInspector.GitTrimmedOutput(ctx, sourcePath, "rev-parse", "--show-toplevel") + if err != nil || repoRoot == "" { + return } - if count == 0 { - return nil + count, err := d.repoInspector.CountUntrackedPaths(ctx, repoRoot) + if err != nil || count == 0 { + return } fmt.Fprintf(out, "---\nnote: %d untracked non-ignored file(s) were NOT copied (git-tracked files only by default — pass --include-untracked to include them)\n", count) - return nil } diff --git a/internal/cli/workspace_preview_test.go b/internal/cli/workspace_preview_test.go new file mode 100644 index 0000000..74cac66 --- /dev/null +++ b/internal/cli/workspace_preview_test.go @@ -0,0 +1,120 @@ +package cli + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "banger/internal/daemon/workspace" +) + +// seedRepoWithSubdir creates a git repo with one tracked file, and an +// untracked non-ignored file at the repo root (not under the subdir). +// Returns the repo root and the subdir path. +func seedRepoWithSubdir(t *testing.T) (repoRoot, subDir string) { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skipf("git not on PATH: %v", err) + } + repoRoot = t.TempDir() + run := func(args ...string) { + t.Helper() + cmd := exec.Command(args[0], args[1:]...) + cmd.Dir = repoRoot + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", + "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t", + "GIT_CONFIG_GLOBAL=/dev/null", + ) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("%v: %v\n%s", args, err, out) + } + } + writeFile := func(relPath, content string) { + t.Helper() + full := filepath.Join(repoRoot, relPath) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + run("git", "init", "-q", "-b", "main") + run("git", "config", "commit.gpgsign", "false") + writeFile("tracked.md", "hello\n") + writeFile("sub/kept.txt", "kept\n") + run("git", "add", ".") + run("git", "commit", "-q", "-m", "init") + // Untracked non-ignored file at the ROOT — not under sub/. This is + // what the pre-fix noteUntrackedSkipped would miss when the user + // passed sub/ as the workspace source. + writeFile("ROOT-SECRET.env", "TOKEN=abc\n") + subDir = filepath.Join(repoRoot, "sub") + return repoRoot, subDir +} + +// TestNoteUntrackedSkippedCountsRepoWideEvenFromSubdir pins the bug +// fix: when the user passes a subdirectory of a repo as the workspace +// source, the untracked-files notice must still reflect what will +// actually be skipped at the guest-shipping layer — which is a +// repo-wide concern. Before the fix the helper ran `git -C +// ls-files --others --exclude-standard`, which only sees files under +// the subdir, silently underreporting the real skip count. +func TestNoteUntrackedSkippedCountsRepoWideEvenFromSubdir(t *testing.T) { + repoRoot, subDir := seedRepoWithSubdir(t) + + d := defaultDeps() + d.repoInspector = workspace.NewInspector() + + var out bytes.Buffer + d.noteUntrackedSkipped(context.Background(), &out, subDir) + + got := out.String() + if !strings.Contains(got, "1 untracked") { + t.Fatalf("note = %q, want mention of 1 untracked file (the root-level SECRET.env)", got) + } + _ = repoRoot +} + +// TestNoteUntrackedSkippedSilentOutsideRepo verifies the best-effort +// contract: when sourcePath is not inside any git repo, the helper +// prints nothing and does not error. Callers rely on this so a user +// who points vm run at an ad-hoc directory (or an export tarball +// that's been unpacked) doesn't get the whole operation aborted +// over a courtesy notice. +func TestNoteUntrackedSkippedSilentOutsideRepo(t *testing.T) { + d := defaultDeps() + d.repoInspector = workspace.NewInspector() + + nonRepo := t.TempDir() + var out bytes.Buffer + d.noteUntrackedSkipped(context.Background(), &out, nonRepo) + + if got := out.String(); got != "" { + t.Fatalf("note = %q, want no output outside a git repo", got) + } +} + +// TestNoteUntrackedSkippedSwallowsInspectorErrors verifies that a +// runner that errors on every call produces no output and no panic. +// This is the other half of best-effort: even if git-the-binary is +// somehow broken or missing, the live flow keeps running. +func TestNoteUntrackedSkippedSwallowsInspectorErrors(t *testing.T) { + d := defaultDeps() + d.repoInspector = &workspace.Inspector{ + Runner: func(context.Context, string, ...string) ([]byte, error) { + return nil, &exec.Error{Name: "git", Err: exec.ErrNotFound} + }, + } + + var out bytes.Buffer + d.noteUntrackedSkipped(context.Background(), &out, t.TempDir()) + if got := out.String(); got != "" { + t.Fatalf("note = %q, want silence when inspector runner errors", got) + } +}