fix: address review feedback for PR #585

packages/browseros/tools/bdev/internal/engine/apply.go

@@ -353,7 +353,7 @@ func operationsFromPatchSet(set patch.PatchSet) []resolve.Operation {
 func operationsFromChanges(repoSet patch.PatchSet, changes []git.FileChange, filters []string) []resolve.Operation {
 	var ops []resolve.Operation
 	for _, change := range changes {
-		rel := strings.TrimPrefix(patch.NormalizeChromiumPath(change.Path), "chromium_patches/")
+		rel := normalizeChangedPatchPath(change.Path)
 		if !patch.PathMatches(rel, filters) {
 			continue
 		}
@@ -365,7 +365,7 @@ func operationsFromChanges(repoSet patch.PatchSet, changes []git.FileChange, fil
 			ChromiumPath: rel,
 			PatchRel:     rel,
 			Op:           patch.OpDelete,
-			OldPath:      patch.NormalizeChromiumPath(change.OldPath),
+			OldPath:      normalizeChangedPatchPath(change.OldPath),
 		})
 	}
 	return ops
@@ -387,6 +387,10 @@ func rejectPath(workspacePath string, op resolve.Operation) string {
 	return ""
 }
 
+func normalizeChangedPatchPath(path string) string {
+	return strings.TrimPrefix(patch.NormalizeChromiumPath(path), "chromium_patches/")
+}
+
 func clearResolveState(workspacePath string) error {
 	if resolve.Exists(workspacePath) {
 		return resolve.Delete(workspacePath)

packages/browseros/tools/bdev/internal/engine/engine_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/browseros-ai/BrowserOS/packages/browseros/tools/bdev/internal/git"
 	"github.com/browseros-ai/BrowserOS/packages/browseros/tools/bdev/internal/patch"
 	"github.com/browseros-ai/BrowserOS/packages/browseros/tools/bdev/internal/repo"
 	"github.com/browseros-ai/BrowserOS/packages/browseros/tools/bdev/internal/resolve"
@@ -91,6 +92,82 @@ func TestPublishReturnsHelpfulErrorWhenNothingChanged(t *testing.T) {
 	}
 }
 
+func TestOperationsFromChangesNormalizesOldPath(t *testing.T) {
+	ops := operationsFromChanges(nil, []git.FileChange{{
+		Status:  "R",
+		Path:    "chromium_patches/chrome/new.cc",
+		OldPath: "chromium_patches/chrome/old.cc",
+	}}, nil)
+
+	if len(ops) != 1 {
+		t.Fatalf("expected 1 operation, got %d", len(ops))
+	}
+	if ops[0].ChromiumPath != "chrome/new.cc" {
+		t.Fatalf("unexpected chromium path: %q", ops[0].ChromiumPath)
+	}
+	if ops[0].OldPath != "chrome/old.cc" {
+		t.Fatalf("unexpected old path: %q", ops[0].OldPath)
+	}
+}
+
+func TestSyncClearsPendingStashAfterSuccessfulNonRebaseRun(t *testing.T) {
+	ctx := context.Background()
+	workspacePath := initGitRepo(t)
+	writeFile(t, filepath.Join(workspacePath, "chrome", "browser.cc"), "base\n")
+	runGit(t, workspacePath, "add", "chrome/browser.cc")
+	runGit(t, workspacePath, "commit", "-m", "workspace base")
+	baseCommit := gitOutput(t, workspacePath, "rev-parse", "HEAD")
+
+	remoteRepo := t.TempDir()
+	runGit(t, remoteRepo, "init", "--bare")
+
+	repoRoot := initGitRepo(t)
+	if err := os.MkdirAll(filepath.Join(repoRoot, "chromium_patches"), 0o755); err != nil {
+		t.Fatalf("MkdirAll: %v", err)
+	}
+	writeFile(t, filepath.Join(repoRoot, "BASE_COMMIT"), baseCommit+"\n")
+	runGit(t, repoRoot, "add", "BASE_COMMIT")
+	runGit(t, repoRoot, "commit", "-m", "patch repo init")
+	runGit(t, repoRoot, "remote", "add", "origin", remoteRepo)
+	runGit(t, repoRoot, "push", "-u", "origin", "HEAD")
+	repoHead := gitOutput(t, repoRoot, "rev-parse", "HEAD")
+
+	repoInfo, err := repo.Load(repoRoot)
+	if err != nil {
+		t.Fatalf("repo.Load: %v", err)
+	}
+	if err := workspace.SaveState(workspacePath, &workspace.State{
+		Version:      1,
+		Workspace:    workspacePath,
+		BaseCommit:   baseCommit,
+		LastSyncRev:  repoHead,
+		PendingStash: "stash@{42}",
+	}); err != nil {
+		t.Fatalf("SaveState: %v", err)
+	}
+
+	result, err := Sync(ctx, SyncOptions{
+		Workspace: workspace.Entry{Name: "ws", Path: workspacePath},
+		Repo:      repoInfo,
+		Remote:    "origin",
+		Rebase:    false,
+	})
+	if err != nil {
+		t.Fatalf("Sync: %v", err)
+	}
+	if result.StashRef != "" {
+		t.Fatalf("expected no new stash ref, got %q", result.StashRef)
+	}
+
+	state, err := workspace.LoadState(workspacePath)
+	if err != nil {
+		t.Fatalf("LoadState: %v", err)
+	}
+	if state.PendingStash != "" {
+		t.Fatalf("expected pending stash to be cleared, got %q", state.PendingStash)
+	}
+}
+
 func initGitRepo(t *testing.T) string {
 	t.Helper()
 	dir := t.TempDir()

packages/browseros/tools/bdev/internal/engine/sync.go

@@ -118,8 +118,8 @@ func Sync(ctx context.Context, opts SyncOptions) (*SyncResult, error) {
 		if err := git.StashPop(ctx, opts.Workspace.Path, result.StashRef); err != nil {
 			return nil, err
 		}
-		state.PendingStash = ""
 	}
+	state.PendingStash = ""
 	state.BaseCommit = opts.Repo.BaseCommit
 	state.LastSyncRev = head
 	state.LastSyncAt = time.Now().UTC()

packages/browseros/tools/bdev/internal/git/git.go

@@ -194,15 +194,6 @@ func ListUntracked(ctx context.Context, dir string, pathspecs []string) ([]strin
 	return lines, nil
 }
 
-func DiffNameStatusAgainst(ctx context.Context, dir string, ref string, pathspecs []string) ([]FileChange, error) {
-	args := []string{"diff", "--name-status", "-M", ref}
-	if len(pathspecs) > 0 {
-		args = append(args, "--")
-		args = append(args, pathspecs...)
-	}
-	return runNameStatus(ctx, dir, args...)
-}
-
 func DiffNameStatusBetween(ctx context.Context, dir string, from string, to string, pathspecs []string) ([]FileChange, error) {
 	args := []string{"diff", "--name-status", "-M", fmt.Sprintf("%s..%s", from, to)}
 	if len(pathspecs) > 0 {

packages/browseros/tools/bdev/internal/git/git_test.go

@@ -2,14 +2,27 @@ package git
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 	"testing"
+	"time"
 )
 
 func TestRunReturnsContextError(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	config := []byte("[alias]\n\thold = !sleep 5\n")
+	if err := os.WriteFile(filepath.Join(home, ".gitconfig"), config, 0o644); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
 
-	if _, err := Run(ctx, t.TempDir(), nil, "status"); err == nil {
-		t.Fatalf("expected context cancellation error")
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
+	defer cancel()
+
+	if _, err := Run(ctx, t.TempDir(), nil, "hold"); err == nil {
+		t.Fatalf("expected timeout error")
+	}
+	if ctx.Err() != context.DeadlineExceeded {
+		t.Fatalf("expected context deadline exceeded, got %v", ctx.Err())
 	}
 }