fix(browser): stage Chrome's downloads in the session profile

docs/browser-tab.md

@@ -65,7 +65,10 @@ A file the page downloads lands in the user's Downloads folder
 (`XDG_DOWNLOAD_DIR`, from the environment or `user-dirs.dirs`, on Linux;
 `~/Downloads` elsewhere) under the name the page suggests, made unique the way
 browsers do (`report (1).csv`): nothing already there is overwritten. Chrome
-writes it under its download id and lich renames it once it is complete. A
+writes it under its download id into a staging folder inside the session's own
+profile, and lich moves it to Downloads once it is complete; across
+filesystems the move is a copy. What Chrome downloads on its own, with no page
+announcing it, stays in staging and is deleted with the profile. A
 click or key press that starts one answers with a `downloaded:` line, its path
 and size, after waiting up to ten seconds for it; one still running answers
 `downloading:`, and the next page read says when it ends. `browser_open` and

docs/ceilings.md

@@ -993,11 +993,13 @@ work when nobody knows it and that the call site never shows. The mechanism and
 
 - **A session browser's downloads go straight to the user's Downloads folder** (`internal/browser/downloads.go`,
   `session_downloads.go`, `docs/browser-tab.md`): `XDG_DOWNLOAD_DIR` on Linux, `~/Downloads` on macOS and Windows,
-  even for a Windows user who moved that known folder. While one runs, a file named by Chrome's download id sits
-  there; it takes its real name once complete. The download behaviour is the whole browser's, so what Chrome
-  downloads on its own, with no page, lands there too under its id and is never renamed: the optimization guide's
-  model downloads are switched off (`disabledFeatures`, `chrome.go`), and a background download a later Chrome adds
-  would land. Chrome reloads the page 1.46 to 1.70 seconds after an `<a download>` begins, and the reload aborts a
+  even for a Windows user who moved that known folder. Chrome writes each one under its download id into a staging
+  folder in the session's profile, and only a download a page announced moves to Downloads, once complete. The
+  download behaviour is the whole browser's, so what Chrome downloads on its own, with no page (4 KB to 33 MB files
+  on the macOS and Windows runners), lands in staging, is never moved and goes with the profile when the browser
+  closes; until then it takes the profile's disk. The profile sits in the temp dir, so with Downloads on another
+  filesystem or volume the move is a copy, written under a hidden `.part` name and renamed once whole. A move that
+  fails reports the staged path, which goes with the profile too. Chrome reloads the page 1.46 to 1.70 seconds after an `<a download>` begins, and the reload aborts a
   navigation started before it (`net::ERR_ABORTED`; Chromium 155, with downloads on or off), so an agent's click,
   key press, open or navigate that downloaded holds its answer 2.5 seconds; a navigation the person starts in the
   pane is not held. A download still running after ten seconds is reported in progress and named again when it

internal/browser/chrome.go

@@ -8,6 +8,7 @@ import (
 	"log/slog"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"time"
 
 	"github.com/chromedp/cdproto/browser"
@@ -61,7 +62,7 @@ func chromeLaunch(dir string, headed bool) (Page, error) {
 		return nil, err
 	}
 	p := &chromePage{ctx: ctx, cancel: cancel, allocCancel: allocCancel}
-	if err := p.allowDownloads(); err != nil {
+	if err := p.allowDownloads(dir); err != nil {
 		slog.Warn("browser: downloads stay off", "err", err)
 	}
 	return p, nil
@@ -70,9 +71,10 @@ func chromeLaunch(dir string, headed bool) (Page, error) {
 // disabledFeatures replaces chromedp's own disable-features list, which it
 // repeats, adding the optimization guide's model downloads. Those go through
 // the profile's download manager with no page and no download event, so with
-// downloads allowed Chrome saved them into the user's Downloads as files
-// named by a GUID, 4 KB to 33 MB each (Chrome on the Windows and macOS CI
-// runners, 2026-10-27).
+// downloads allowed Chrome saved them into the download folder as files named
+// by a GUID, 4 KB to 33 MB each (Chrome on the Windows and macOS CI runners,
+// 2026-10-27). They stay in the profile's staging folder now; these spare
+// the bytes.
 const disabledFeatures = "site-per-process,Translate,BlinkGenPropertyTrees," +
 	"OptimizationGuideModelDownloading,OptimizationTargetPrediction"
 
@@ -90,27 +92,32 @@ type chromePage struct {
 var downloadFolder = downloadDir
 
 // allowDownloads saves what the page downloads into the user's Downloads
-// folder, with events. A headless Chrome denies downloads by default, and a
-// click on a download button did nothing.
-func (p *chromePage) allowDownloads() error {
+// folder, with events, by way of a staging folder in the profile dir. A
+// headless Chrome denies downloads by default, and a click on a download
+// button did nothing.
+func (p *chromePage) allowDownloads(profile string) error {
 	dir, err := downloadFolder()
 	if err != nil {
 		return err
 	}
 	if err := os.MkdirAll(dir, 0o755); err != nil {
 		return err
 	}
+	staging := filepath.Join(profile, stagingDir)
+	if err := os.MkdirAll(staging, 0o700); err != nil {
+		return err
+	}
 	c := chromedp.FromContext(p.ctx)
 	err = p.run(nil, chromedp.ActionFunc(func(ctx context.Context) error {
 		return browser.SetDownloadBehavior(browser.SetDownloadBehaviorBehaviorAllowAndName).
-			WithDownloadPath(dir).
+			WithDownloadPath(staging).
 			WithEventsEnabled(true).
 			Do(cdp.WithExecutor(ctx, c.Browser))
 	}))
 	if err != nil {
 		return err
 	}
-	p.dl = newDownloads(dir)
+	p.dl = newDownloads(staging, dir)
 	return nil
 }
 

internal/browser/downloads.go

@@ -1,32 +1,45 @@
 package browser
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 	"sync"
 
 	"github.com/lichdotdev/lich/api"
+	"github.com/lichdotdev/lich/internal/profilestore"
 )
 
 // fallbackName is what a download is called when the page suggests no usable
 // name.
 const fallbackName = "download"
 
+// stagingDir is the folder inside the session's profile Chrome writes into.
+const stagingDir = "lich-downloads"
+
+// partSuffix marks a download still being copied into Downloads.
+const partSuffix = ".part"
+
 // downloads names what a page saves. Chrome writes each file under its GUID
-// (allowAndName) into dir; begin reserves the name the person will see, the
-// suggested one made unique the way browsers do ("report (1).csv"), and the
-// completed file is renamed to it, so nothing already there is overwritten.
+// (allowAndName) into staging; begin reserves the name the person will see in
+// dir, the suggested one made unique the way browsers do ("report (1).csv"),
+// and the completed file is moved to it, so nothing already there is
+// overwritten. The download behaviour is the whole browser's, so what Chrome
+// downloads on its own, with no page, lands in staging too; no begin names it,
+// nothing moves it, and it goes with the profile.
 type downloads struct {
-	dir string
-	mu  sync.Mutex
+	staging, dir string
+	// rename is os.Rename, which the tests make cross filesystems.
+	rename func(from, to string) error
+	mu     sync.Mutex
 	// reserved are the paths in-flight downloads will take, by GUID.
 	reserved map[string]string
 }
 
-func newDownloads(dir string) *downloads {
-	return &downloads{dir: dir, reserved: make(map[string]string)}
+func newDownloads(staging, dir string) *downloads {
+	return &downloads{staging: staging, dir: dir, rename: os.Rename, reserved: make(map[string]string)}
 }
 
 // begin reserves a path for the download guid announced.
@@ -38,7 +51,7 @@ func (d *downloads) begin(guid, suggested string) api.BrowserDownload {
 	return api.BrowserDownload{Path: path, State: api.DownloadInProgress}
 }
 
-// progress applies one progress report, renaming the file into place once it
+// progress applies one progress report, moving the file into place once it
 // is complete. ok is false for a GUID begin never saw.
 func (d *downloads) progress(guid string, received float64, state string) (api.BrowserDownload, bool, error) {
 	d.mu.Lock()
@@ -60,14 +73,43 @@ func (d *downloads) progress(guid string, received float64, state string) (api.B
 		path = d.freeLocked(filepath.Base(path))
 		out.Path = path
 	}
-	written := filepath.Join(d.dir, guid)
-	if err := os.Rename(written, path); err != nil {
+	written := filepath.Join(d.staging, guid)
+	if err := d.move(written, path); err != nil {
 		out.Path = written
 		return out, true, err
 	}
 	return out, true, nil
 }
 
+// move renames from to to, and copies where a rename cannot go: the profile
+// and Downloads may sit on different filesystems, or volumes on Windows. The
+// copy is written under a temporary name beside to and renamed once whole, so
+// a half-copied file never carries the download's name.
+func (d *downloads) move(from, to string) error {
+	err := d.rename(from, to)
+	var linkErr *os.LinkError
+	if err == nil || !errors.As(err, &linkErr) || !profilestore.CrossDevice(linkErr.Err) {
+		return err
+	}
+	info, err := os.Stat(from)
+	if err != nil {
+		return err
+	}
+	part := filepath.Join(filepath.Dir(to), "."+filepath.Base(from)+partSuffix)
+	err = profilestore.CopyFile(from, part)
+	if err == nil {
+		err = os.Chmod(part, info.Mode().Perm())
+	}
+	if err == nil {
+		err = os.Rename(part, to)
+	}
+	if err != nil {
+		_ = os.Remove(part)
+		return err
+	}
+	return os.Remove(from)
+}
+
 // freeLocked is name in dir, or its first " (n)" variant that neither exists
 // nor is reserved.
 func (d *downloads) freeLocked(name string) string {

internal/browser/downloads_other_test.go

@@ -0,0 +1,8 @@
+//go:build !windows
+
+package browser
+
+import "syscall"
+
+// errCrossDevice is what rename answers for a file moving across filesystems.
+var errCrossDevice error = syscall.EXDEV

internal/browser/downloads_test.go

@@ -1,6 +1,8 @@
 package browser
 
 import (
+	"errors"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
@@ -9,17 +11,17 @@ import (
 )
 
 func TestDownloadsNameTheFileWithoutOverwriting(t *testing.T) {
-	dir := t.TempDir()
+	d := testDownloadsAt(t)
+	dir := d.dir
 	if err := os.WriteFile(filepath.Join(dir, "report.csv"), []byte("old"), 0o600); err != nil {
 		t.Fatal(err)
 	}
-	d := newDownloads(dir)
 	a := d.begin("ga", "report.csv")
 	b := d.begin("gb", "report.csv")
 	if a.Path != filepath.Join(dir, "report (1).csv") || b.Path != filepath.Join(dir, "report (2).csv") {
 		t.Fatalf("reserved %q and %q", a.Path, b.Path)
 	}
-	if err := os.WriteFile(filepath.Join(dir, "ga"), []byte("new"), 0o600); err != nil {
+	if err := os.WriteFile(filepath.Join(d.staging, "ga"), []byte("new"), 0o600); err != nil {
 		t.Fatal(err)
 	}
 	got, ok, err := d.progress("ga", 3, api.DownloadCompleted)
@@ -32,14 +34,17 @@ func TestDownloadsNameTheFileWithoutOverwriting(t *testing.T) {
 	if b, _ := os.ReadFile(filepath.Join(dir, "report.csv")); string(b) != "old" {
 		t.Fatal("the file already there was overwritten")
 	}
+	if _, err := os.Lstat(filepath.Join(d.staging, "ga")); !errors.Is(err, fs.ErrNotExist) {
+		t.Fatalf("the staged file is still there (%v)", err)
+	}
 }
 
 func TestDownloadsRenameAroundAFileThatTookTheName(t *testing.T) {
-	dir := t.TempDir()
-	d := newDownloads(dir)
+	d := testDownloadsAt(t)
+	dir := d.dir
 	want := d.begin("g", "a.txt").Path
-	for _, name := range []string{"a.txt", "g"} {
-		if err := os.WriteFile(filepath.Join(dir, name), []byte(name), 0o600); err != nil {
+	for _, path := range []string{filepath.Join(dir, "a.txt"), filepath.Join(d.staging, "g")} {
+		if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -50,8 +55,7 @@ func TestDownloadsRenameAroundAFileThatTookTheName(t *testing.T) {
 }
 
 func TestDownloadsCanceledFreesTheNameAndUnknownIsIgnored(t *testing.T) {
-	dir := t.TempDir()
-	d := newDownloads(dir)
+	d := testDownloadsAt(t)
 	first := d.begin("g", "x.bin").Path
 	if got, ok, _ := d.progress("g", 0, api.DownloadCanceled); !ok || got.State != api.DownloadCanceled {
 		t.Fatalf("canceled reported %+v", got)
@@ -65,14 +69,74 @@ func TestDownloadsCanceledFreesTheNameAndUnknownIsIgnored(t *testing.T) {
 }
 
 func TestDownloadsReportTheWrittenFileWhenTheRenameFails(t *testing.T) {
-	d := newDownloads(t.TempDir())
+	d := testDownloadsAt(t)
 	d.begin("g", "a.txt")
 	got, ok, err := d.progress("g", 1, api.DownloadCompleted)
-	if err == nil || !ok || got.Path != filepath.Join(d.dir, "g") {
+	if err == nil || !ok || got.Path != filepath.Join(d.staging, "g") {
 		t.Fatalf("got %+v ok=%v err=%v, want the GUID file and the error", got, ok, err)
 	}
 }
 
+func TestDownloadsCopyAcrossFilesystems(t *testing.T) {
+	d := testDownloadsAt(t)
+	d.rename = func(from, to string) error {
+		return &os.LinkError{Op: "rename", Old: from, New: to, Err: errCrossDevice}
+	}
+	want := d.begin("g", "a.txt").Path
+	staged := filepath.Join(d.staging, "g")
+	if err := os.WriteFile(staged, []byte("data"), 0o640); err != nil {
+		t.Fatal(err)
+	}
+	got, _, err := d.progress("g", 4, api.DownloadCompleted)
+	if err != nil || got.Path != want {
+		t.Fatalf("got %+v (err %v), want %s", got, err, want)
+	}
+	if b, err := os.ReadFile(want); err != nil || string(b) != "data" {
+		t.Fatalf("%s holds %q (%v)", want, b, err)
+	}
+	if _, err := os.Lstat(staged); !errors.Is(err, fs.ErrNotExist) {
+		t.Fatalf("the staged file is still there (%v)", err)
+	}
+	if names := dirNames(t, d.dir); len(names) != 1 {
+		t.Fatalf("Downloads holds %v, want only the download", names)
+	}
+}
+
+func TestDownloadsLeaveWhatNoPageAnnouncedInStaging(t *testing.T) {
+	d := testDownloadsAt(t)
+	stray := filepath.Join(d.staging, "stray")
+	if err := os.WriteFile(stray, []byte("model"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if _, ok, err := d.progress("stray", 5, api.DownloadCompleted); ok || err != nil {
+		t.Fatalf("a file no page announced was handled (ok=%v err=%v)", ok, err)
+	}
+	if names := dirNames(t, d.dir); len(names) != 0 {
+		t.Fatalf("Downloads holds %v, want nothing", names)
+	}
+	if _, err := os.Lstat(stray); err != nil {
+		t.Fatalf("the stray file left staging: %v", err)
+	}
+}
+
+// testDownloadsAt is a downloads staging in one temporary folder and saving
+// into another.
+func testDownloadsAt(t *testing.T) *downloads {
+	return newDownloads(t.TempDir(), t.TempDir())
+}
+
+func dirNames(t *testing.T, dir string) []string {
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var names []string
+	for _, e := range entries {
+		names = append(names, e.Name())
+	}
+	return names
+}
+
 func TestSafeNameKeepsTheFileInTheFolder(t *testing.T) {
 	for in, want := range map[string]string{
 		"report.csv":       "report.csv",

internal/browser/downloads_windows_test.go

@@ -0,0 +1,6 @@
+package browser
+
+import "golang.org/x/sys/windows"
+
+// errCrossDevice is what rename answers for a file moving across volumes.
+var errCrossDevice error = windows.ERROR_NOT_SAME_DEVICE

internal/profilestore/link_windows.go

@@ -42,7 +42,7 @@ func linkFile(target, path string) (copied bool, err error) {
 	if !filepath.IsAbs(target) {
 		target = filepath.Join(filepath.Dir(path), target)
 	}
-	return true, copyFile(target, path)
+	return true, CopyFile(target, path)
 }
 
 // IsLink reports whether path is a link this package made: a symlink, or a

internal/profilestore/move.go

@@ -97,7 +97,7 @@ func move(from, to string) error {
 	}
 	err := os.Rename(from, to)
 	var linkErr *os.LinkError
-	if err == nil || !errors.As(err, &linkErr) || !crossDevice(linkErr.Err) {
+	if err == nil || !errors.As(err, &linkErr) || !CrossDevice(linkErr.Err) {
 		return err
 	}
 	if err := copyTree(from, to); err != nil {
@@ -135,7 +135,7 @@ func copyTree(from, to string) error {
 			dirs = append(dirs, dirMode{dst, info.Mode().Perm()})
 			return os.MkdirAll(dst, 0o700)
 		}
-		if err := copyFile(path, dst); err != nil {
+		if err := CopyFile(path, dst); err != nil {
 			return err
 		}
 		return os.Chmod(dst, info.Mode().Perm())
@@ -178,7 +178,8 @@ func copyLink(src, dst string) error {
 	return err
 }
 
-func copyFile(src, dst string) error {
+// CopyFile copies src to dst, which it creates and never overwrites.
+func CopyFile(src, dst string) error {
 	in, err := os.Open(src)
 	if err != nil {
 		return err