noteUntrackedSkipped: fix subdir underreport + be best-effort everywhere

Two bugs in the untracked-skipped warning, both surfaced by review.

1. Wrong scope for subdir inputs. The helper accepted any path the
   caller had (sourcePath, which may be a user-supplied subdirectory)
   and ran `git -C <path> ls-files --others --exclude-standard`. Git
   scopes that output to the cwd, so pointing `vm run ./repo/sub` at
   a subdir silently underreported untracked files living elsewhere
   in the repo — exactly the files the warning exists to surface.
   Fix: resolve sourcePath to the repo root inside the helper via
   `rev-parse --show-toplevel` before counting.

2. Inconsistent failure handling. The comment said the helper should
   be silent when the count can't be determined; the body returned
   the error. vm_run.go treated the error as non-fatal (logged a
   warning, continued); workspace prepare and --dry-run aborted the
   whole operation on the same helper failure. A courtesy notice
   shouldn't kill the operation.
   Fix: make the helper best-effort in signature and body — no error
   return, swallows rev-parse + count failures, emits nothing when
   there's nothing to say. All three callers lose their error
   branches.

Regression tests:
- subdir input reports the root-level untracked file (the bug case)
- non-repo path produces silence, not a fatal error
- inspector whose runner errors on every call produces silence

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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],

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,

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
 }

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 <subdir>
+// 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)
+	}
+}