From 1b918f74fbd3cf033b59d11d73f4add9bf24db2f Mon Sep 17 00:00:00 2001 From: omartelo Date: Tue, 27 Oct 2026 17:37:13 -0300 Subject: [PATCH 1/3] feat(browser): save a session browser's downloads to the user's Downloads folder A headless Chrome denies downloads, so a download button in the session's browser did nothing. Chrome now saves them (Browser.setDownloadBehavior, allowAndName, also in the Open in a window relaunch) into the user's Downloads folder (XDG_DOWNLOAD_DIR on Linux, ~/Downloads elsewhere, behind a build-tag seam), and lich renames each to the page's suggested name, made unique with " (n)" so nothing is overwritten. The agent learns of it in the page click and press answer: a "downloaded: ()" line, waiting up to ten seconds for the file, or "downloading:" while it runs, which the next read follows up. The answer also holds until 1.5 s after the download began: Chrome aborts a navigation started sooner and reloads the page (10 of 10 right after an , 0 of 50 with the gap, Chromium 155). The window hears browser-download and shows a toast naming the file with a button that opens its folder. A live test covers , an attachment link, target=_blank, a location.href, a window.open and a blob: download, each landing once in Downloads with the page left where it was. --- CHANGELOG.md | 1 + api/browser.go | 25 ++++ api/testdata/contract.golden | 9 ++ docs/browser-tab.md | 19 ++- docs/ceilings.md | 13 ++- internal/browser/actions.go | 1 + internal/browser/browser.go | 25 ++-- internal/browser/browser_test.go | 1 + internal/browser/chrome.go | 35 +++++- internal/browser/chrome_live_test.go | 4 +- internal/browser/chrome_view.go | 26 ++++- internal/browser/downloads.go | 114 ++++++++++++++++++ internal/browser/downloads_dir_linux.go | 52 +++++++++ internal/browser/downloads_dir_linux_test.go | 42 +++++++ internal/browser/downloads_dir_other.go | 20 ++++ internal/browser/downloads_live_test.go | 111 ++++++++++++++++++ internal/browser/downloads_test.go | 89 ++++++++++++++ internal/browser/fake_test.go | 26 +++-- internal/browser/page.go | 7 +- internal/browser/rpc.go | 3 +- internal/browser/session_downloads.go | 110 +++++++++++++++++ internal/browser/session_downloads_test.go | 117 +++++++++++++++++++ internal/cli/browser_page.go | 27 +++++ internal/cli/browser_page_test.go | 21 ++++ internal/cli/mcp_browser.go | 9 +- native/cmd/lich-native/browser_model_test.go | 22 ++++ native/cmd/lich-native/browser_window.go | 28 ++++- native/cmd/lich-native/main.go | 1 + native/cmd/lich-native/model.go | 6 + native/i18n/locales/en/layouts.json | 2 + native/i18n/locales/es/layouts.json | 2 + native/i18n/locales/ja/layouts.json | 2 + native/i18n/locales/ko/layouts.json | 2 + native/i18n/locales/pt-br/layouts.json | 2 + native/i18n/locales/ru/layouts.json | 2 + native/i18n/locales/zh-cn/layouts.json | 2 + 36 files changed, 946 insertions(+), 32 deletions(-) create mode 100644 internal/browser/downloads.go create mode 100644 internal/browser/downloads_dir_linux.go create mode 100644 internal/browser/downloads_dir_linux_test.go create mode 100644 internal/browser/downloads_dir_other.go create mode 100644 internal/browser/downloads_live_test.go create mode 100644 internal/browser/downloads_test.go create mode 100644 internal/browser/session_downloads.go create mode 100644 internal/browser/session_downloads_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e13280273..30cd4b9a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - The crosshair on a session browser's toolbar lets you leave notes on the page for the agent. The element under the pointer is outlined and named, a click opens a note beside it, and each note pins its element with a number, following it as the page scrolls. Notes wait in a tray floating over the bottom of the page, which never resizes under you. Hovering a note there lights its element, a click edits it, and Send (or Ctrl+Enter) types them into the session as one message: which element, a selector the agent's `browser_click` takes, the React component that drew it in a development build, and your note. Ctrl+Shift+E (Cmd+Shift+E on macOS) turns the crosshair on and off, and Esc leaves it. A send the session refuses keeps the notes and says why in the tray. +- A session's browser downloads files. A download the page starts, from a link, a button, a new tab or a script, lands in your Downloads folder under the name the page gives it, with ` (1)` added rather than overwriting a file already there. The agent's click or key press answers with the file's path and size, and the window shows a toast naming the file, with a button that opens its folder. Before, a download button in the session's browser did nothing. ## [0.80.0] - 2026-10-27 diff --git a/api/browser.go b/api/browser.go index 8daea24d7..f4e8ffc58 100644 --- a/api/browser.go +++ b/api/browser.go @@ -55,8 +55,29 @@ var ( // BrowserAgentEvent is emitted each time the agent acts on its page, so the // pane can show that the page moved because of it, and where it clicked. BrowserAgentEvent = defineEvent[BrowserAgentAct]("browser-agent") + + // BrowserDownloadEvent is emitted when a file a session's page downloaded + // is complete on disk, so the person learns where it went. + BrowserDownloadEvent = defineEvent[BrowserDownload]("browser-download") +) + +// Download states, Chrome's own (Browser.downloadProgress). +const ( + DownloadInProgress = "inProgress" + DownloadCompleted = "completed" + DownloadCanceled = "canceled" ) +// BrowserDownload is one file a session's page downloaded: Path is where it +// lands in the user's Downloads folder, under the name it keeps once +// complete; Bytes is what arrived so far. +type BrowserDownload struct { + Owner string `json:"owner,omitempty"` + Path string `json:"path"` + Bytes int64 `json:"bytes"` + State string `json:"state"` +} + // BrowserState is a session's browser as the window shows it. Open false means // it is gone; ByAgent says the agent started it rather than the window. type BrowserState struct { @@ -227,6 +248,10 @@ type BrowserPageInfo struct { Text string `json:"text"` Interactive []BrowserInteractive `json:"interactive"` Truncated bool `json:"truncated"` + // Downloads are the ones the agent has not heard of in their current + // state: what its last action saved, or one it saw start and that ended + // since. + Downloads []BrowserDownload `json:"downloads,omitempty"` } // BrowserPointOptions is a point of session ID's page, in CSS pixels. diff --git a/api/testdata/contract.golden b/api/testdata/contract.golden index 722a37306..15c9f6bc9 100644 --- a/api/testdata/contract.golden +++ b/api/testdata/contract.golden @@ -238,6 +238,7 @@ tray.SetLabels(TrayLabels) None event appupdate-progress AppUpdateProgress event branch-pull-request BranchPullRequest event browser-agent BrowserAgentAct +event browser-download BrowserDownload event browser-state BrowserState event browser:frame: string event claude-profile-login ProfileLogin @@ -339,6 +340,7 @@ BrowserPageInfo { text string interactive []BrowserInteractive truncated bool + downloads,omitempty []BrowserDownload } BrowserPointOptions { @@ -1546,6 +1548,13 @@ BrowserAgentAct { url,omitempty string } +BrowserDownload { + owner,omitempty string + path string + bytes int64 + state string +} + ProfileLogin { profile string output,omitempty string diff --git a/docs/browser-tab.md b/docs/browser-tab.md index d57c3beb2..ea8b5f3a8 100644 --- a/docs/browser-tab.md +++ b/docs/browser-tab.md @@ -59,8 +59,25 @@ page only answers what is where (`browser.ElementAt`, `browser.Locate`); the outline, the pins and the note are the window's, so nothing is injected into the page. +## Downloads + +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 +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. That answer also +waits until 1.5 seconds after the download began, because Chrome aborts a +navigation started sooner and reloads the page. `browser-download` tells the +window when a file is complete, and the window shows a toast naming it with a +button that opens its folder. The same holds in a window opened with Open in a +window. + ## Providers Claude Code, Codex and opencode receive the tools at spawn. Antigravity, oh-my-pi, Crush, Cursor CLI and Kiro receive them from the lich MCP server the -plugin registers. +plugin registers. Downloads reach all eight the same way: in the answer of the +same tools. diff --git a/docs/ceilings.md b/docs/ceilings.md index 4f3175324..cecd21bb2 100644 --- a/docs/ceilings.md +++ b/docs/ceilings.md @@ -970,7 +970,7 @@ work when nobody knows it and that the call site never shows. The mechanism and `docs/browser-tab.md`): a headless Chrome, Edge, Chromium, Brave, Vivaldi or Helium already installed, whose page reaches the session's browser tab as a JPEG screencast over `/events`, at the pace the window takes frames. The native window cannot hold a webview: on Linux it speaks Wayland without GTK. So the pane - has no DevTools, shows the arrow whatever cursor the page asks for, loads what the page opens in a new tab or window in the page itself, and leaves downloads and the page's own dialogs out; Open in + has no DevTools, shows the arrow whatever cursor the page asks for, loads what the page opens in a new tab or window in the page itself, and leaves the page's own dialogs out; Open in a window moves the page to a real Chromium window for those, reopening it on the same profile with every cookie, since chromedp cannot show a headless process in place. A half-filled form or sessionStorage is lost on that move. Closing the card, or the session's process ending, closes the browser; closing that @@ -991,6 +991,17 @@ work when nobody knows it and that the call site never shows. The mechanism and moves on its own is outlined late. Notes not sent yet live in the window: closing it, or the browser, loses them. +- **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. Chrome aborts a navigation the page starts + within about a second of a download (`net::ERR_ABORTED`, then it reloads the page; Chromium 155, with + downloads on or off), so a click or key press that downloaded holds its answer 1.5 seconds; a navigation the + person starts in the pane, or an agent's `browser_open` right after a download the page started on its own, + is not held. A download still running after ten seconds is reported in progress and named again when it + ends. No tool lists past downloads or cancels one, nothing caps their size, and closing the browser cancels + one still running. + ## Languages - **The prompt language reaches a running session only through lich itself** (`internal/prompt`): text lich diff --git a/internal/browser/actions.go b/internal/browser/actions.go index 7550e1d4a..74db75455 100644 --- a/internal/browser/actions.go +++ b/internal/browser/actions.go @@ -75,6 +75,7 @@ func (s *Service) Info(id string) (PageInfo, error) { } s.remember(sess, info.URL, info.Title) s.agentActed(sess, api.BrowserAgentAct{}) + info.Downloads = s.untold(sess) return info, nil } diff --git a/internal/browser/browser.go b/internal/browser/browser.go index 621612a94..dae949cdb 100644 --- a/internal/browser/browser.go +++ b/internal/browser/browser.go @@ -41,11 +41,13 @@ var ( // opens and the page the agent drives are the same one. Headless until // someone asks for a window; headed after OpenVisible. type Service struct { - mu sync.Mutex - launch Launcher - mkTmp func(dir, pattern string) (string, error) - now func() time.Time - idle time.Duration + mu sync.Mutex + launch Launcher + mkTmp func(dir, pattern string) (string, error) + now func() time.Time + idle time.Duration + // quiet is downloadQuiet, which the suite's fake pages do without. + quiet time.Duration byOwner map[string]*session byID map[string]*session // owners serializes one session's launch, promotion and reuse, so a slow @@ -68,9 +70,13 @@ type session struct { // it, so the reaper leaves it alone like a headed window. byAgent, watching bool // agentAt is where the agent last acted, where its next scroll turns. - agentAt *point - page Page - lastUsed time.Time + agentAt *point + // downloads are the page's, until the agent has heard how each ended; + // downloadBegun is when the last one began. + downloads []*download + downloadBegun time.Time + page Page + lastUsed time.Time } // New starts a service that launches the system Chromium (chromium.FindBrowser) @@ -89,6 +95,7 @@ func newService(launch Launcher, mkTmp func(string, string) (string, error), now mkTmp: mkTmp, now: now, idle: idle, + quiet: downloadQuiet, byOwner: make(map[string]*session), byID: make(map[string]*session), owners: make(map[string]*sync.Mutex), @@ -264,7 +271,7 @@ func (s *Service) start(spec launchSpec) (Handle, error) { id: newID(), owner: spec.owner, dir: spec.dir, url: spec.url, headed: spec.headed, byAgent: spec.byAgent, page: page, lastUsed: s.now(), } - page.Listen(s.frameSink(sess), s.infoSink(sess)) + page.Listen(s.frameSink(sess), s.infoSink(sess), s.downloadSink(sess)) if err := runPage(func(ctx context.Context) error { if len(spec.cookies) > 0 { if err := page.SetCookies(ctx, spec.cookies); err != nil { diff --git a/internal/browser/browser_test.go b/internal/browser/browser_test.go index f23fa6293..a419fe98d 100644 --- a/internal/browser/browser_test.go +++ b/internal/browser/browser_test.go @@ -18,6 +18,7 @@ func testService(t *testing.T, launch *fakeLauncher) *Service { return t.TempDir(), nil }, func() time.Time { return now }, 5*time.Minute) s.now = func() time.Time { return now } + s.quiet = 0 t.Cleanup(func() { _ = s.Cleanup() }) return s } diff --git a/internal/browser/chrome.go b/internal/browser/chrome.go index a75b0a026..e1a52f967 100644 --- a/internal/browser/chrome.go +++ b/internal/browser/chrome.go @@ -4,10 +4,12 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "os" "os/exec" "time" + "github.com/chromedp/cdproto/browser" "github.com/chromedp/cdproto/cdp" "github.com/chromedp/cdproto/input" "github.com/chromedp/cdproto/network" @@ -56,7 +58,11 @@ func chromeLaunch(dir string, headed bool) (Page, error) { allocCancel() return nil, err } - return &chromePage{ctx: ctx, cancel: cancel, allocCancel: allocCancel}, nil + p := &chromePage{ctx: ctx, cancel: cancel, allocCancel: allocCancel} + if err := p.allowDownloads(); err != nil { + slog.Warn("browser: downloads stay off", "err", err) + } + return p, nil } type chromePage struct { @@ -64,6 +70,33 @@ type chromePage struct { cancel context.CancelFunc allocCancel context.CancelFunc listen listeners + // dl names the files the page downloads; nil when downloads are off. + dl *downloads +} + +// 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 { + dir, err := downloadDir() + if err != nil { + return err + } + if err := os.MkdirAll(dir, 0o755); 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). + WithEventsEnabled(true). + Do(cdp.WithExecutor(ctx, c.Browser)) + })) + if err != nil { + return err + } + p.dl = newDownloads(dir) + return nil } func (p *chromePage) Alive() bool { return p.ctx.Err() == nil } diff --git a/internal/browser/chrome_live_test.go b/internal/browser/chrome_live_test.go index e6377eaa0..b0ff232db 100644 --- a/internal/browser/chrome_live_test.go +++ b/internal/browser/chrome_live_test.go @@ -148,7 +148,7 @@ func TestChromeScreencastSendsFramesAndPageInfo(t *testing.T) { case titles <- title: default: } - }) + }, func(string, api.BrowserDownload) {}) ctx, cancel := context.WithTimeout(context.Background(), pageWait) defer cancel() if err := p.Navigate(ctx, srv.URL); err != nil { @@ -225,7 +225,7 @@ func TestChromeLinkToANewTabLoadsInThePageAndKeepsItResponding(t *testing.T) { t.Cleanup(srv.Close) p := pg.(*chromePage) urls := make(chan string, 8) - p.Listen(func(string) {}, func(url, _ string) { urls <- url }) + p.Listen(func(string) {}, func(url, _ string) { urls <- url }, func(string, api.BrowserDownload) {}) ctx, cancel := context.WithTimeout(context.Background(), pageWait) defer cancel() if err := p.Navigate(ctx, srv.URL+"/"); err != nil { diff --git a/internal/browser/chrome_view.go b/internal/browser/chrome_view.go index 26afc1396..dc936d7c4 100644 --- a/internal/browser/chrome_view.go +++ b/internal/browser/chrome_view.go @@ -6,6 +6,7 @@ import ( "math" "sync" + "github.com/chromedp/cdproto/browser" "github.com/chromedp/cdproto/cdp" "github.com/chromedp/cdproto/emulation" "github.com/chromedp/cdproto/input" @@ -30,7 +31,7 @@ type listeners struct { onInfo func(url, title string) } -func (p *chromePage) Listen(onFrame func(data string), onInfo func(url, title string)) { +func (p *chromePage) Listen(onFrame func(data string), onInfo func(url, title string), onDownload func(guid string, d api.BrowserDownload)) { p.listen.mu.Lock() p.listen.onFrame, p.listen.onInfo = onFrame, onInfo p.listen.mu.Unlock() @@ -47,6 +48,29 @@ func (p *chromePage) Listen(onFrame func(data string), onInfo func(url, title st } }) p.keepOneTab() + p.watchDownloads(onDownload) +} + +// watchDownloads hands on each download the page starts and each step it +// takes, in Chrome's order: a progress report never overtakes its begin. +func (p *chromePage) watchDownloads(onDownload func(guid string, d api.BrowserDownload)) { + if p.dl == nil { + return + } + chromedp.ListenBrowser(p.ctx, func(ev any) { + switch e := ev.(type) { + case *browser.EventDownloadWillBegin: + onDownload(e.GUID, p.dl.begin(e.GUID, e.SuggestedFilename)) + case *browser.EventDownloadProgress: + d, ok, err := p.dl.progress(e.GUID, e.ReceivedBytes, string(e.State)) + if err != nil { + slog.Warn("browser: name a finished download", "path", d.Path, "err", err) + } + if ok { + onDownload(e.GUID, d) + } + } + }) } // keepOneTab loads, in this page, what a link or a script opens in a new tab diff --git a/internal/browser/downloads.go b/internal/browser/downloads.go new file mode 100644 index 000000000..d2953c364 --- /dev/null +++ b/internal/browser/downloads.go @@ -0,0 +1,114 @@ +package browser + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "github.com/lichdotdev/lich/api" +) + +// fallbackName is what a download is called when the page suggests no usable +// name. +const fallbackName = "download" + +// 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. +type downloads struct { + dir string + 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)} +} + +// begin reserves a path for the download guid announced. +func (d *downloads) begin(guid, suggested string) api.BrowserDownload { + d.mu.Lock() + defer d.mu.Unlock() + path := d.freeLocked(safeName(suggested)) + d.reserved[guid] = path + return api.BrowserDownload{Path: path, State: api.DownloadInProgress} +} + +// progress applies one progress report, renaming 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() + defer d.mu.Unlock() + path, ok := d.reserved[guid] + if !ok { + return api.BrowserDownload{}, false, nil + } + out := api.BrowserDownload{Path: path, Bytes: int64(received), State: state} + if state == api.DownloadInProgress { + return out, true, nil + } + delete(d.reserved, guid) + if state != api.DownloadCompleted { + return out, true, nil + } + if _, err := os.Lstat(path); err == nil { + // Something else took the name while the file downloaded. + path = d.freeLocked(filepath.Base(path)) + out.Path = path + } + written := filepath.Join(d.dir, guid) + if err := os.Rename(written, path); err != nil { + out.Path = written + return out, true, err + } + return out, true, nil +} + +// freeLocked is name in dir, or its first " (n)" variant that neither exists +// nor is reserved. +func (d *downloads) freeLocked(name string) string { + ext := filepath.Ext(name) + stem := strings.TrimSuffix(name, ext) + for n := 0; ; n++ { + candidate := name + if n > 0 { + candidate = fmt.Sprintf("%s (%d)%s", stem, n, ext) + } + path := filepath.Join(d.dir, candidate) + if d.takenLocked(path) { + continue + } + // A name Lstat cannot read is left to the rename to fail on. + if _, err := os.Lstat(path); err != nil { + return path + } + } +} + +func (d *downloads) takenLocked(path string) bool { + for _, p := range d.reserved { + if p == path { + return true + } + } + return false +} + +// safeName keeps a page-suggested name inside the folder: no separators, no +// dot-only name. +func safeName(name string) string { + name = strings.TrimSpace(strings.Map(func(r rune) rune { + if r == '/' || r == '\\' || r < ' ' { + return '_' + } + return r + }, name)) + if strings.Trim(name, ".") == "" { + return fallbackName + } + return name +} diff --git a/internal/browser/downloads_dir_linux.go b/internal/browser/downloads_dir_linux.go new file mode 100644 index 000000000..f7aaa6e2e --- /dev/null +++ b/internal/browser/downloads_dir_linux.go @@ -0,0 +1,52 @@ +//go:build linux + +package browser + +import ( + "os" + "path/filepath" + "strings" +) + +// downloadDir is the folder a desktop names Downloads: XDG_DOWNLOAD_DIR, set +// in the environment or in user-dirs.dirs (where xdg-user-dirs writes it, +// translated on a desktop in another language), else ~/Downloads. +func downloadDir() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + if dir := os.Getenv("XDG_DOWNLOAD_DIR"); filepath.IsAbs(dir) { + return dir, nil + } + conf := os.Getenv("XDG_CONFIG_HOME") + if !filepath.IsAbs(conf) { + conf = filepath.Join(home, ".config") + } + if b, err := os.ReadFile(filepath.Join(conf, "user-dirs.dirs")); err == nil { + if dir := userDirsDownload(string(b), home); dir != "" { + return dir, nil + } + } + return filepath.Join(home, "Downloads"), nil +} + +// userDirsDownload reads XDG_DOWNLOAD_DIR out of a user-dirs.dirs, whose +// values are "$HOME/…" or an absolute path; "" when it names neither. +func userDirsDownload(text, home string) string { + for _, line := range strings.Split(text, "\n") { + v, ok := strings.CutPrefix(strings.TrimSpace(line), "XDG_DOWNLOAD_DIR=") + if !ok { + continue + } + v = strings.Trim(v, `"`) + if rest, ok := strings.CutPrefix(v, "$HOME"); ok { + v = home + rest + } + if filepath.IsAbs(v) { + return v + } + return "" + } + return "" +} diff --git a/internal/browser/downloads_dir_linux_test.go b/internal/browser/downloads_dir_linux_test.go new file mode 100644 index 000000000..e0d342371 --- /dev/null +++ b/internal/browser/downloads_dir_linux_test.go @@ -0,0 +1,42 @@ +//go:build linux + +package browser + +import ( + "os" + "testing" +) + +func TestUserDirsDownload(t *testing.T) { + for _, tc := range []struct{ text, want string }{ + {"# comment\nXDG_DESKTOP_DIR=\"$HOME/Desktop\"\nXDG_DOWNLOAD_DIR=\"$HOME/Transferências\"\n", "/home/u/Transferências"}, + {"XDG_DOWNLOAD_DIR=\"/data/dl\"", "/data/dl"}, + {"XDG_DOWNLOAD_DIR=\"relative\"", ""}, + {"XDG_MUSIC_DIR=\"$HOME/Music\"", ""}, + } { + if got := userDirsDownload(tc.text, "/home/u"); got != tc.want { + t.Errorf("userDirsDownload(%q) = %q, want %q", tc.text, got, tc.want) + } + } +} + +func TestDownloadDirReadsUserDirs(t *testing.T) { + home, conf := t.TempDir(), t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_DOWNLOAD_DIR", "") + t.Setenv("XDG_CONFIG_HOME", conf) + got, err := downloadDir() + if err != nil || got != home+"/Downloads" { + t.Fatalf("without user-dirs.dirs: %q, %v; want ~/Downloads", got, err) + } + if err := os.WriteFile(conf+"/user-dirs.dirs", []byte("XDG_DOWNLOAD_DIR=\"$HOME/Baixados\"\n"), 0o600); err != nil { + t.Fatal(err) + } + if got, _ := downloadDir(); got != home+"/Baixados" { + t.Fatalf("got %q, want the folder user-dirs.dirs names", got) + } + t.Setenv("XDG_DOWNLOAD_DIR", "/env/dl") + if got, _ := downloadDir(); got != "/env/dl" { + t.Fatalf("got %q, want the environment's", got) + } +} diff --git a/internal/browser/downloads_dir_other.go b/internal/browser/downloads_dir_other.go new file mode 100644 index 000000000..9245d4358 --- /dev/null +++ b/internal/browser/downloads_dir_other.go @@ -0,0 +1,20 @@ +//go:build !linux + +package browser + +import ( + "os" + "path/filepath" +) + +// downloadDir is ~/Downloads, where macOS keeps it and Windows puts it by +// default. +// shortcut: a Windows user who moved the Downloads known folder still gets +// %USERPROFILE%\Downloads; read SHGetKnownFolderPath if someone asks. +func downloadDir() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(home, "Downloads"), nil +} diff --git a/internal/browser/downloads_live_test.go b/internal/browser/downloads_live_test.go new file mode 100644 index 000000000..75f678aca --- /dev/null +++ b/internal/browser/downloads_live_test.go @@ -0,0 +1,111 @@ +package browser + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/lichdotdev/lich/api" + "github.com/lichdotdev/lich/internal/chromium" +) + +// downloadPage holds one way each a page starts a download. /file/ +// answers as an attachment named . +const downloadPage = `Downloads +attr +attachment +blank + + +` + +func TestChromeDownloadsLandInDownloadsAndThePageStays(t *testing.T) { + if _, err := chromium.FindBrowser(exec.LookPath); err != nil { + t.Skip("no Chromium-family browser on this machine") + } + dir := t.TempDir() + // Every OS's home, so the download folder is this test's and never the + // machine user's. + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + t.Setenv("XDG_DOWNLOAD_DIR", filepath.Join(dir, "Downloads")) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if name, ok := strings.CutPrefix(r.URL.Path, "/file/"); ok { + w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", name)) + w.Header().Set("Content-Type", "text/csv") + fmt.Fprint(w, name+",1") + return + } + if strings.HasPrefix(r.URL.Path, "/plain/") { + w.Header().Set("Content-Type", "text/csv") + fmt.Fprint(w, "attr.csv,1") + return + } + w.Header().Set("Content-Type", "text/html") + fmt.Fprint(w, downloadPage) + })) + t.Cleanup(srv.Close) + s := newService(chromeLaunch, func(string, string) (string, error) { return t.TempDir(), nil }, time.Now, idleAfter) + t.Cleanup(func() { + if err := s.Cleanup(); err != nil { + t.Error(err) + } + }) + if _, err := s.Open("s", srv.URL+"/"); err != nil { + t.Fatal(err) + } + for _, id := range []string{"attr", "attachment", "blank", "location", "open", "blob"} { + t.Run(id, func(t *testing.T) { + page := srv.URL + "/" + id + if _, err := s.Go(t.Context(), api.BrowserOpenOptions{ID: "s", URL: page}); err != nil { + t.Fatal(err) + } + info, err := s.ClickAt(t.Context(), api.BrowserClickOptions{ID: "s", BrowserTarget: api.BrowserTarget{Selector: "#" + id}}) + if err != nil { + t.Fatal(err) + } + want := filepath.Join(dir, "Downloads", id+".csv") + got := info.Downloads + for deadline := time.Now().Add(10 * time.Second); len(got) == 0 && time.Now().Before(deadline); { + // A new tab hands its URL to the page after the click returns. + time.Sleep(100 * time.Millisecond) + read, err := s.Read(t.Context(), api.SessionIDOptions{ID: "s"}) + if err != nil { + t.Fatal(err) + } + got = read.Downloads + } + if len(got) != 1 || got[0].Path != want || got[0].State != api.DownloadCompleted { + t.Fatalf("downloads %+v, want %s completed", got, want) + } + if b, err := os.ReadFile(want); err != nil || string(b) != id+".csv,1" { + t.Fatalf("%s holds %q (%v)", want, b, err) + } + read, err := s.Read(t.Context(), api.SessionIDOptions{ID: "s"}) + if err != nil { + t.Fatal(err) + } + if read.URL != page { + t.Fatalf("the page moved to %s", read.URL) + } + }) + } + entries, err := os.ReadDir(filepath.Join(dir, "Downloads")) + if err != nil { + t.Fatal(err) + } + if len(entries) != 6 { + var names []string + for _, e := range entries { + names = append(names, e.Name()) + } + t.Fatalf("Downloads holds %v, want the six files and nothing else", names) + } +} diff --git a/internal/browser/downloads_test.go b/internal/browser/downloads_test.go new file mode 100644 index 000000000..63a0dbb2e --- /dev/null +++ b/internal/browser/downloads_test.go @@ -0,0 +1,89 @@ +package browser + +import ( + "os" + "path/filepath" + "testing" + + "github.com/lichdotdev/lich/api" +) + +func TestDownloadsNameTheFileWithoutOverwriting(t *testing.T) { + dir := t.TempDir() + 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 { + t.Fatal(err) + } + got, ok, err := d.progress("ga", 3, api.DownloadCompleted) + if err != nil || !ok || got.Path != a.Path || got.Bytes != 3 { + t.Fatalf("completed %+v ok=%v err=%v", got, ok, err) + } + if b, _ := os.ReadFile(a.Path); string(b) != "new" { + t.Fatalf("%s holds %q, want the download", a.Path, b) + } + if b, _ := os.ReadFile(filepath.Join(dir, "report.csv")); string(b) != "old" { + t.Fatal("the file already there was overwritten") + } +} + +func TestDownloadsRenameAroundAFileThatTookTheName(t *testing.T) { + dir := t.TempDir() + d := newDownloads(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 { + t.Fatal(err) + } + } + got, _, err := d.progress("g", 1, api.DownloadCompleted) + if err != nil || got.Path != filepath.Join(dir, "a (1).txt") { + t.Fatalf("got %q (err %v) instead of a free name beside %q", got.Path, err, want) + } +} + +func TestDownloadsCanceledFreesTheNameAndUnknownIsIgnored(t *testing.T) { + dir := t.TempDir() + d := newDownloads(dir) + 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) + } + if again := d.begin("h", "x.bin").Path; again != first { + t.Fatalf("a canceled download kept %q reserved; got %q", first, again) + } + if _, ok, _ := d.progress("nobody", 1, api.DownloadCompleted); ok { + t.Fatal("a GUID never begun was reported") + } +} + +func TestDownloadsReportTheWrittenFileWhenTheRenameFails(t *testing.T) { + d := newDownloads(t.TempDir()) + 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") { + t.Fatalf("got %+v ok=%v err=%v, want the GUID file and the error", got, ok, err) + } +} + +func TestSafeNameKeepsTheFileInTheFolder(t *testing.T) { + for in, want := range map[string]string{ + "report.csv": "report.csv", + "../../etc/passwd": ".._.._etc_passwd", + `..\win.ini`: ".._win.ini", + "..": fallbackName, + " ": fallbackName, + "a\nb": "a_b", + } { + if got := safeName(in); got != want { + t.Errorf("safeName(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/browser/fake_test.go b/internal/browser/fake_test.go index fe5c216c8..17a6029a9 100644 --- a/internal/browser/fake_test.go +++ b/internal/browser/fake_test.go @@ -30,11 +30,14 @@ type fakePage struct { clickAt [2]float64 onFrame func(string) onInfo func(url, title string) - casting bool - viewports []Viewport - mice []api.BrowserMouseOptions - mouseErr error - clickErr error + onDownload func(guid string, d api.BrowserDownload) + // onClick runs after a click, as what the page does in answer to it. + onClick func() + casting bool + viewports []Viewport + mice []api.BrowserMouseOptions + mouseErr error + clickErr error // dead is a Chrome whose process ended under the page. dead bool keys []api.BrowserKeyOptions @@ -111,12 +114,17 @@ func (p *fakePage) Info(context.Context) (PageInfo, error) { func (p *fakePage) Click(_ context.Context, t Target) (x, y float64, err error) { p.mu.Lock() - defer p.mu.Unlock() if p.clickErr != nil { + p.mu.Unlock() return 0, 0, p.clickErr } p.clicks = append(p.clicks, t) - return p.clickAt[0], p.clickAt[1], nil + x, y, then := p.clickAt[0], p.clickAt[1], p.onClick + p.mu.Unlock() + if then != nil { + then() + } + return x, y, nil } func (p *fakePage) Type(_ context.Context, text string, clear bool, t Target) error { @@ -194,10 +202,10 @@ func (p *fakePage) SetCookies(_ context.Context, cookies []*network.CookieParam) return nil } -func (p *fakePage) Listen(onFrame func(string), onInfo func(url, title string)) { +func (p *fakePage) Listen(onFrame func(string), onInfo func(url, title string), onDownload func(string, api.BrowserDownload)) { p.mu.Lock() defer p.mu.Unlock() - p.onFrame, p.onInfo = onFrame, onInfo + p.onFrame, p.onInfo, p.onDownload = onFrame, onInfo, onDownload } func (p *fakePage) Screencast(_ context.Context, v Viewport) error { diff --git a/internal/browser/page.go b/internal/browser/page.go index 949db2fd9..622d8e4aa 100644 --- a/internal/browser/page.go +++ b/internal/browser/page.go @@ -72,6 +72,8 @@ type PageInfo struct { Text string `json:"text"` Interactive []Interactive `json:"interactive"` Truncated bool `json:"truncated"` + // Downloads is the service's to fill, never the page script's. + Downloads []api.BrowserDownload `json:"-"` } // Page is one Chromium tab the service drives. chrome.go is the real one; @@ -99,8 +101,9 @@ type Page interface { Cookies(ctx context.Context) ([]*network.CookieParam, error) SetCookies(ctx context.Context, cookies []*network.CookieParam) error // Listen hands the page what to call with each screencast frame (base64 - // JPEG) and with its URL and title whenever they change. - Listen(onFrame func(data string), onInfo func(url, title string)) + // JPEG), with its URL and title whenever they change, and with each step + // of a download it starts. + Listen(onFrame func(data string), onInfo func(url, title string), onDownload func(guid string, d api.BrowserDownload)) Screencast(ctx context.Context, v Viewport) error StopScreencast(ctx context.Context) error Resize(ctx context.Context, v Viewport) error diff --git a/internal/browser/rpc.go b/internal/browser/rpc.go index 57709bf08..0021f1081 100644 --- a/internal/browser/rpc.go +++ b/internal/browser/rpc.go @@ -67,6 +67,7 @@ func (s *Service) pageAfter(id string) (api.BrowserPageInfo, error) { return api.BrowserPageInfo{}, err } s.remember(sess, info.URL, info.Title) + info.Downloads = s.downloadsAfter(sess, downloadWait) return infoAPI(info), nil } @@ -124,7 +125,7 @@ func handleAPI(h Handle) api.BrowserHandle { } func infoAPI(p PageInfo) api.BrowserPageInfo { - out := api.BrowserPageInfo{URL: p.URL, Title: p.Title, Text: p.Text, Truncated: p.Truncated} + out := api.BrowserPageInfo{URL: p.URL, Title: p.Title, Text: p.Text, Truncated: p.Truncated, Downloads: p.Downloads} if p.Interactive == nil { return out } diff --git a/internal/browser/session_downloads.go b/internal/browser/session_downloads.go new file mode 100644 index 000000000..c3b12a0d4 --- /dev/null +++ b/internal/browser/session_downloads.go @@ -0,0 +1,110 @@ +package browser + +import ( + "time" + + "github.com/lichdotdev/lich/api" + "github.com/lichdotdev/lich/internal/events" +) + +// downloadWait bounds how long a click or a key press waits for a download it +// started to finish before answering with it still in progress: a small file +// is on disk within it, and a large one is not worth holding the tool for. +const downloadWait = 10 * time.Second + +// downloadPoll is how often that wait looks. +const downloadPoll = 50 * time.Millisecond + +// downloadQuiet is how long after a download begins a click or a key press +// holds its answer. Chrome aborts a navigation the page starts within about a +// second of a download (net::ERR_ABORTED, then it reloads the page): the +// agent's next step failed 10 times in 10 right after an , and +// none in 50 with this gap (Chromium 155, measured 2026-10-27). +const downloadQuiet = 1500 * time.Millisecond + +// download is one of a session's downloads and the state the agent last +// heard it in, "" before it heard of it. +type download struct { + guid string + now api.BrowserDownload + told string +} + +// downloadSink records each step of the page's downloads and tells the +// window when one is complete on disk. +func (s *Service) downloadSink(sess *session) func(guid string, d api.BrowserDownload) { + return func(guid string, d api.BrowserDownload) { + d.Owner = sess.owner + s.mu.Lock() + completed := sess.noteDownloadLocked(guid, d) + s.mu.Unlock() + if completed { + events.Send(s.emit, api.BrowserDownloadEvent, d) + } + } +} + +// noteDownloadLocked keeps d as guid's latest step, true when it is the one +// that completed it. +func (sess *session) noteDownloadLocked(guid string, d api.BrowserDownload) bool { + for _, dl := range sess.downloads { + if dl.guid == guid { + was := dl.now.State + dl.now = d + return d.State == api.DownloadCompleted && was != api.DownloadCompleted + } + } + sess.downloads = append(sess.downloads, &download{guid: guid, now: d}) + sess.downloadBegun = time.Now() + return d.State == api.DownloadCompleted +} + +// downloadsAfter waits up to wait for the downloads the agent has not heard +// of yet to end, and out the quiet after the last one began, then returns +// what it has not heard. +func (s *Service) downloadsAfter(sess *session, wait time.Duration) []api.BrowserDownload { + deadline := time.Now().Add(wait) + for s.freshInProgress(sess) && time.Now().Before(deadline) { + time.Sleep(downloadPoll) + } + s.mu.Lock() + hold := time.Until(sess.downloadBegun.Add(s.quiet)) + s.mu.Unlock() + if hold > 0 { + time.Sleep(hold) + } + return s.untold(sess) +} + +func (s *Service) freshInProgress(sess *session) bool { + s.mu.Lock() + defer s.mu.Unlock() + for _, dl := range sess.downloads { + if dl.told == "" && dl.now.State == api.DownloadInProgress { + return true + } + } + return false +} + +// untold is each download in a state the agent has not heard, which it now +// has. A download it heard end is forgotten. +func (s *Service) untold(sess *session) []api.BrowserDownload { + s.mu.Lock() + defer s.mu.Unlock() + var out []api.BrowserDownload + kept := sess.downloads[:0] + for _, dl := range sess.downloads { + if dl.told != dl.now.State { + d := dl.now + d.Owner = "" + out = append(out, d) + dl.told = d.State + } + if dl.now.State == api.DownloadInProgress { + kept = append(kept, dl) + } + } + sess.downloads = kept + return out +} diff --git a/internal/browser/session_downloads_test.go b/internal/browser/session_downloads_test.go new file mode 100644 index 000000000..a446950ab --- /dev/null +++ b/internal/browser/session_downloads_test.go @@ -0,0 +1,117 @@ +package browser + +import ( + "reflect" + "testing" + "time" + + "github.com/lichdotdev/lich/api" +) + +func TestClickReportsTheFileItDownloadedAndTellsTheWindowOnce(t *testing.T) { + s, launch, out := viewService(t) + if _, err := s.Open("s", "https://example.com"); err != nil { + t.Fatal(err) + } + p := launch.pages[0] + p.onClick = func() { + p.onDownload("g1", api.BrowserDownload{Path: "/dl/report.csv", State: api.DownloadInProgress}) + p.onDownload("g1", api.BrowserDownload{Path: "/dl/report.csv", Bytes: 12, State: api.DownloadCompleted}) + } + info, err := s.ClickAt(t.Context(), api.BrowserClickOptions{ID: "s", BrowserTarget: api.BrowserTarget{Selector: "#dl"}}) + if err != nil { + t.Fatal(err) + } + want := []api.BrowserDownload{{Path: "/dl/report.csv", Bytes: 12, State: api.DownloadCompleted}} + if !reflect.DeepEqual(info.Downloads, want) { + t.Fatalf("click reported %+v, want %+v", info.Downloads, want) + } + done := out.named(api.BrowserDownloadEvent.Name) + if len(done) != 1 || done[0].(api.BrowserDownload).Owner != "s" || done[0].(api.BrowserDownload).Path != "/dl/report.csv" { + t.Fatalf("window heard %+v, want the one completed download, owned by s", done) + } + again, err := s.Read(t.Context(), api.SessionIDOptions{ID: "s"}) + if err != nil { + t.Fatal(err) + } + if len(again.Downloads) != 0 { + t.Fatalf("a read after the click repeated %+v", again.Downloads) + } +} + +func TestClickWaitsForTheDownloadItStartedToFinish(t *testing.T) { + s, launch, _ := viewService(t) + if _, err := s.Open("s", "https://example.com"); err != nil { + t.Fatal(err) + } + p := launch.pages[0] + p.onClick = func() { + p.onDownload("g1", api.BrowserDownload{Path: "/dl/a.zip", State: api.DownloadInProgress}) + go func() { + time.Sleep(100 * time.Millisecond) + p.onDownload("g1", api.BrowserDownload{Path: "/dl/a.zip", Bytes: 5, State: api.DownloadCompleted}) + }() + } + info, err := s.ClickAt(t.Context(), api.BrowserClickOptions{ID: "s", BrowserTarget: api.BrowserTarget{Selector: "#dl"}}) + if err != nil { + t.Fatal(err) + } + if len(info.Downloads) != 1 || info.Downloads[0].State != api.DownloadCompleted { + t.Fatalf("click reported %+v, want the download completed", info.Downloads) + } +} + +func TestADownloadStillRunningIsReportedAgainWhenItEnds(t *testing.T) { + s, launch, _ := viewService(t) + if _, err := s.Open("s", "https://example.com"); err != nil { + t.Fatal(err) + } + p := launch.pages[0] + p.onDownload("g1", api.BrowserDownload{Path: "/dl/big.iso", Bytes: 3, State: api.DownloadInProgress}) + sess := s.current("s") + if got := s.downloadsAfter(sess, 0); len(got) != 1 || got[0].State != api.DownloadInProgress { + t.Fatalf("got %+v, want it in progress", got) + } + start := time.Now() + if got := s.downloadsAfter(sess, time.Minute); len(got) != 0 || time.Since(start) > time.Second { + t.Fatalf("got %+v after %v: a download already reported in progress must not hold the next action", got, time.Since(start)) + } + p.onDownload("g1", api.BrowserDownload{Path: "/dl/big.iso", Bytes: 9, State: api.DownloadCompleted}) + info, err := s.Read(t.Context(), api.SessionIDOptions{ID: "s"}) + if err != nil { + t.Fatal(err) + } + if len(info.Downloads) != 1 || info.Downloads[0].State != api.DownloadCompleted || info.Downloads[0].Bytes != 9 { + t.Fatalf("read reported %+v, want it completed", info.Downloads) + } + if len(sess.downloads) != 0 { + t.Fatalf("kept %d downloads the agent heard end", len(sess.downloads)) + } +} + +func TestClickHoldsItsAnswerForTheQuietAfterADownload(t *testing.T) { + s, launch, _ := viewService(t) + s.quiet = 200 * time.Millisecond + if _, err := s.Open("s", "https://example.com"); err != nil { + t.Fatal(err) + } + p := launch.pages[0] + target := api.BrowserClickOptions{ID: "s", BrowserTarget: api.BrowserTarget{Selector: "#dl"}} + start := time.Now() + if _, err := s.ClickAt(t.Context(), target); err != nil { + t.Fatal(err) + } + if took := time.Since(start); took >= s.quiet { + t.Fatalf("a click that downloaded nothing took %v", took) + } + p.onClick = func() { + p.onDownload("g1", api.BrowserDownload{Path: "/dl/a.csv", Bytes: 1, State: api.DownloadCompleted}) + } + start = time.Now() + if _, err := s.ClickAt(t.Context(), target); err != nil { + t.Fatal(err) + } + if took := time.Since(start); took < s.quiet { + t.Fatalf("the click answered %v after its download began, before the quiet", took) + } +} diff --git a/internal/cli/browser_page.go b/internal/cli/browser_page.go index 38b78485e..3027899ae 100644 --- a/internal/cli/browser_page.go +++ b/internal/cli/browser_page.go @@ -17,6 +17,10 @@ import ( func pageText(info api.BrowserPageInfo) string { var b strings.Builder fmt.Fprintf(&b, "url: %s\ntitle: %s\n", info.URL, info.Title) + for _, d := range info.Downloads { + b.WriteString(downloadLine(d)) + b.WriteByte('\n') + } if info.Text != "" { fmt.Fprintf(&b, "text: %s\n", info.Text) } @@ -76,3 +80,26 @@ func shortHref(href string, base *url.URL) string { } return href } + +// downloadLine says where a file the page downloaded is, and how far it got. +func downloadLine(d api.BrowserDownload) string { + switch d.State { + case api.DownloadCompleted: + return fmt.Sprintf("downloaded: %s (%s)", d.Path, sizeText(d.Bytes)) + case api.DownloadInProgress: + return fmt.Sprintf("downloading: %s (%s so far)", d.Path, sizeText(d.Bytes)) + default: + return "download canceled: " + d.Path + } +} + +func sizeText(n int64) string { + switch { + case n < 1<<10: + return fmt.Sprintf("%d B", n) + case n < 1<<20: + return fmt.Sprintf("%.0f KB", float64(n)/(1<<10)) + default: + return fmt.Sprintf("%.1f MB", float64(n)/(1<<20)) + } +} diff --git a/internal/cli/browser_page_test.go b/internal/cli/browser_page_test.go index 5e945f4a2..7681d3310 100644 --- a/internal/cli/browser_page_test.go +++ b/internal/cli/browser_page_test.go @@ -49,3 +49,24 @@ func TestPageTextOfAnEmptyPage(t *testing.T) { t.Fatalf("got %q", got) } } + +func TestPageTextNamesTheDownloadsAfterTheTitle(t *testing.T) { + info := api.BrowserPageInfo{URL: "https://x.test/", Title: "X", Text: "Reports", Downloads: []api.BrowserDownload{ + {Path: "/home/u/Downloads/report.csv", Bytes: 12 * 1024, State: api.DownloadCompleted}, + {Path: "/home/u/Downloads/big.iso", Bytes: 3 << 20, State: api.DownloadInProgress}, + {Path: "/home/u/Downloads/gone.zip", State: api.DownloadCanceled}, + {Path: "/home/u/Downloads/tiny.txt", Bytes: 5, State: api.DownloadCompleted}, + }} + want := strings.Join([]string{ + "url: https://x.test/", + "title: X", + "downloaded: /home/u/Downloads/report.csv (12 KB)", + "downloading: /home/u/Downloads/big.iso (3.0 MB so far)", + "download canceled: /home/u/Downloads/gone.zip", + "downloaded: /home/u/Downloads/tiny.txt (5 B)", + "text: Reports", + }, "\n") + if got := pageText(info); got != want { + t.Fatalf("got\n%s\nwant\n%s", got, want) + } +} diff --git a/internal/cli/mcp_browser.go b/internal/cli/mcp_browser.go index f00956e7b..70d51f967 100644 --- a/internal/cli/mcp_browser.go +++ b/internal/cli/mcp_browser.go @@ -39,7 +39,8 @@ var browserTools = []mcpTool{ Description: "Read this session's current page: URL, title, the text in the viewport, and " + "one line per interactive element there: index, tag, label and link. Form values are " + "never included — (filled)/(checked) say that typing landed. click and press already " + - "answer with this, so call it only to look again without acting. Read-only.", + "answer with this, so call it only to look again without acting. Also names a download " + + "that finished since the page was last read. Read-only.", Schema: schema(map[string]any{}), ReadOnly: true, Run: mcpBrowserInfo, @@ -50,7 +51,8 @@ var browserTools = []mcpTool{ "CSS selector: either scrolls the element into view first. x and y are viewport " + "coordinates and must be inside it. Sends a real mouse event, not element.click(). " + "Answers with the page afterwards, as browser_info reads it, once a navigation the " + - "click started has its document.", + "click started has its document. A file the click downloads is saved to the user's " + + "Downloads folder; the answer names its path in a downloaded: line.", Schema: schema(map[string]any{ "index": property("integer", "Index from the last browser_info."), "selector": property("string", "CSS selector."), @@ -95,7 +97,8 @@ var browserTools = []mcpTool{ "Backspace, Delete, Space, ArrowUp/Down/Left/Right, Home, End, PageUp, PageDown). " + "Real key event, not typed text — use browser_type for that. Optional target " + "focuses first, as for browser_type. Answers with the page afterwards, once a " + - "navigation the key started (Enter on a form) has its document.", + "navigation the key started (Enter on a form) has its document, and names a file it " + + "downloaded as browser_click does.", Schema: schema(map[string]any{ "key": property("string", "Named key: Enter, Tab, Escape, …"), "index": property("integer", "Index from the last browser_info."), diff --git a/native/cmd/lich-native/browser_model_test.go b/native/cmd/lich-native/browser_model_test.go index 41821a88c..8beafdf27 100644 --- a/native/cmd/lich-native/browser_model_test.go +++ b/native/cmd/lich-native/browser_model_test.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "image" "image/jpeg" + "path/filepath" "testing" "github.com/lichdotdev/lich/api" @@ -172,3 +173,24 @@ func TestAgentActionsBetweenTwoFramesAreAllKeptInOrder(t *testing.T) { t.Fatalf("marks %+v, want the click then the scroll", two) } } + +func TestADownloadReachesTheWindowAsAToastNamingItsFolder(t *testing.T) { + m, _ := newLayoutsModel(t) + var heard []api.BrowserDownload + m.browserDownloaded = func(d api.BrowserDownload) { heard = append(heard, d) } + path := filepath.Join("home", "u", "Downloads", "report.csv") + send(m, api.BrowserDownloadEvent.Name, api.BrowserDownload{Owner: "orch", Path: path, Bytes: 12, State: api.DownloadCompleted}) + if len(heard) != 1 || heard[0].Path != path { + t.Fatalf("heard %+v, want the one download", heard) + } + var opened string + toast := downloadToast(heard[0], func(dir string) { opened = dir }) + dir := filepath.Join("home", "u", "Downloads") + if toast.Title != "Downloaded report.csv" || len(toast.Lines) != 1 || toast.Lines[0].Text != dir { + t.Fatalf("toast %q lines %+v", toast.Title, toast.Lines) + } + toast.Action.OnClick() + if opened != dir { + t.Fatalf("the toast's button opened %q, want %q", opened, dir) + } +} diff --git a/native/cmd/lich-native/browser_window.go b/native/cmd/lich-native/browser_window.go index f16ed35eb..f09d4c51f 100644 --- a/native/cmd/lich-native/browser_window.go +++ b/native/cmd/lich-native/browser_window.go @@ -1,14 +1,16 @@ package main import ( + "path/filepath" + "github.com/lichdotdev/lich/api" "github.com/lichdotdev/lich/native/backend" "github.com/lichdotdev/lich/native/i18n" "github.com/lichdotdev/lich/native/ui" ) -// The window's side of a session browser: the card's Browser item, and the -// toast that offers a browser the agent opened. +// The window's side of a session browser: the card's Browser item, the toast +// that offers a browser the agent opened, and the one that names a download. // applyBrowser opens the browser of the card the menu was opened on. func (u *window) applyBrowser(act sidebarAction) { @@ -55,6 +57,28 @@ func (u *window) offerBrowser(st api.BrowserState) { }) } +// tellDownload names a file a session's page downloaded and the folder it +// went to, which the toast's button opens. +func (u *window) tellDownload(d api.BrowserDownload) { + u.toaster.Show(downloadToast(d, func(dir string) { + go func() { + if err := u.client.OpenFolder(u.ctx, dir); err != nil { + u.toaster.Show(ui.Toast{Variant: ui.ToastError, Title: i18n.T("sidebar.sessionCard.openFolderFailed", i18n.Args{"error": backend.ErrorText(err)})}) + } + }() + })) +} + +func downloadToast(d api.BrowserDownload, openFolder func(dir string)) ui.Toast { + dir := filepath.Dir(d.Path) + return ui.Toast{ + Title: i18n.T("layouts.browser.downloaded", i18n.Args{"name": filepath.Base(d.Path)}), + Icon: icDownload, + Lines: []ui.ToastLine{{Icon: icFolder, Text: dir}}, + Action: &ui.ToastButton{Label: i18n.T("layouts.browser.showFolder"), OnClick: func() { openFolder(dir) }}, + } +} + // browserTabOf is the id of session's browser tab, "" for none. func (m *model) browserTabOf(session string) string { m.mu.Lock() diff --git a/native/cmd/lich-native/main.go b/native/cmd/lich-native/main.go index 616c9934c..0f668e878 100644 --- a/native/cmd/lich-native/main.go +++ b/native/cmd/lich-native/main.go @@ -239,6 +239,7 @@ func run(w *app.Window, rt backend.Runtime) error { u.model.serviceReveal = u.revealService u.model.duplicate = u.warnDuplicate u.model.browserOpened = u.offerBrowser + u.model.browserDownloaded = u.tellDownload u.model.pluginIncompatible = func() { go u.checkPlugin() } u.model.appUpdateProgress = u.updateProgressed u.palette.closed = u.model.closed diff --git a/native/cmd/lich-native/model.go b/native/cmd/lich-native/model.go index 90ee072c3..d8815e450 100644 --- a/native/cmd/lich-native/model.go +++ b/native/cmd/lich-native/model.go @@ -124,6 +124,8 @@ type model struct { // hears one the agent started. browsers browserStore browserOpened func(api.BrowserState) + // browserDownloaded hears a file a session's page finished downloading. + browserDownloaded func(api.BrowserDownload) } type sessionCwd = api.SessionCwd @@ -411,6 +413,10 @@ func (m *model) follow(ctx context.Context, events <-chan backend.Event) { if a, err := backend.Decode(ev, api.BrowserAgentEvent); err == nil { m.applyBrowserAgent(a, time.Now()) } + case api.BrowserDownloadEvent.Name: + if d, err := backend.Decode(ev, api.BrowserDownloadEvent); err == nil && m.browserDownloaded != nil { + m.browserDownloaded(d) + } case api.SessionFocusEvent.Name: m.sessionFocus(ev) case api.WindowRaiseEvent.Name: diff --git a/native/i18n/locales/en/layouts.json b/native/i18n/locales/en/layouts.json index 00ba8fbd4..5b553c1a3 100644 --- a/native/i18n/locales/en/layouts.json +++ b/native/i18n/locales/en/layouts.json @@ -27,6 +27,8 @@ "opened": "{label} opened a browser", "dismiss": "Dismiss", "open": "Open", + "downloaded": "Downloaded {name}", + "showFolder": "Open folder", "moveBody": "It leaves \"{layout}\". A browser tab holds only {label} and its browser.", "pick": "Leave notes on the page", "pickHint": "Click an element to leave a note", diff --git a/native/i18n/locales/es/layouts.json b/native/i18n/locales/es/layouts.json index 150a30d7e..7fac0615f 100644 --- a/native/i18n/locales/es/layouts.json +++ b/native/i18n/locales/es/layouts.json @@ -27,6 +27,8 @@ "opened": "{label} abrió un navegador", "dismiss": "Descartar", "open": "Abrir", + "downloaded": "Descargado {name}", + "showFolder": "Abrir carpeta", "moveBody": "Sale de \"{layout}\". La pestaña del navegador solo admite {label} y su navegador.", "pick": "Dejar notas en la página", "pickHint": "Haz clic en un elemento para dejar una nota", diff --git a/native/i18n/locales/ja/layouts.json b/native/i18n/locales/ja/layouts.json index feea61bfe..78d3a2fdc 100644 --- a/native/i18n/locales/ja/layouts.json +++ b/native/i18n/locales/ja/layouts.json @@ -25,6 +25,8 @@ "opened": "{label} がブラウザを開きました", "dismiss": "閉じる", "open": "開く", + "downloaded": "{name} をダウンロードしました", + "showFolder": "フォルダを開く", "moveBody": "「{layout}」から外れます。ブラウザのタブには {label} とそのブラウザしか置けません。", "pick": "ページにメモを残す", "pickHint": "要素をクリックしてメモを残す", diff --git a/native/i18n/locales/ko/layouts.json b/native/i18n/locales/ko/layouts.json index 4e542263a..f20afb690 100644 --- a/native/i18n/locales/ko/layouts.json +++ b/native/i18n/locales/ko/layouts.json @@ -25,6 +25,8 @@ "opened": "{label}이(가) 브라우저를 열었습니다", "dismiss": "닫기", "open": "열기", + "downloaded": "{name} 다운로드 완료", + "showFolder": "폴더 열기", "moveBody": "\"{layout}\"에서 빠집니다. 브라우저 탭에는 {label}와(과) 그 브라우저만 둘 수 있습니다.", "pick": "페이지에 메모 남기기", "pickHint": "요소를 클릭해 메모를 남기세요", diff --git a/native/i18n/locales/pt-br/layouts.json b/native/i18n/locales/pt-br/layouts.json index 9f8a7e112..b01f825b3 100644 --- a/native/i18n/locales/pt-br/layouts.json +++ b/native/i18n/locales/pt-br/layouts.json @@ -27,6 +27,8 @@ "opened": "{label} abriu um navegador", "dismiss": "Dispensar", "open": "Abrir", + "downloaded": "{name} baixado", + "showFolder": "Abrir pasta", "moveBody": "Ela sai de \"{layout}\". A aba do navegador só comporta {label} e o navegador dela.", "pick": "Deixar notas na página", "pickHint": "Clique num elemento para deixar uma nota", diff --git a/native/i18n/locales/ru/layouts.json b/native/i18n/locales/ru/layouts.json index b43bd485a..6ef1d14fb 100644 --- a/native/i18n/locales/ru/layouts.json +++ b/native/i18n/locales/ru/layouts.json @@ -31,6 +31,8 @@ "opened": "{label} открыла браузер", "dismiss": "Скрыть", "open": "Открыть", + "downloaded": "Загружен файл {name}", + "showFolder": "Открыть папку", "moveBody": "Она уйдёт из «{layout}». Во вкладке браузера помещаются только {label} и её браузер.", "pick": "Оставить заметки на странице", "pickHint": "Нажмите на элемент, чтобы оставить заметку", diff --git a/native/i18n/locales/zh-cn/layouts.json b/native/i18n/locales/zh-cn/layouts.json index ba2ec587b..1267cf729 100644 --- a/native/i18n/locales/zh-cn/layouts.json +++ b/native/i18n/locales/zh-cn/layouts.json @@ -25,6 +25,8 @@ "opened": "{label} 打开了浏览器", "dismiss": "忽略", "open": "打开", + "downloaded": "已下载 {name}", + "showFolder": "打开文件夹", "moveBody": "它将离开“{layout}”。浏览器标签页只容纳 {label} 及其浏览器。", "pick": "在页面上留下备注", "pickHint": "点击一个元素来留下备注", From 1bea9e2533a022e6f394d0bca9f84c1a2224f254 Mon Sep 17 00:00:00 2001 From: omartelo Date: Tue, 27 Oct 2026 17:45:35 -0300 Subject: [PATCH 2/3] feat(browser): open or navigate to a file's URL downloads it browser_open and browser_navigate on a URL that answers with a file used to fail with "page load error net::ERR_ABORTED", the agent never learning that the file was saved. Chrome aborts that navigation because it became a download and leaves the page where it was; lich now waits up to a second to hear of the download and takes the load as one that worked. The handle the two tools answer with carries the downloads, waited for and held through the quiet as a click's answer is, and lich browser open/navigate print a downloaded: line. A browser launched straight onto a file stays at about:blank instead of closing. The pane's address bar gets the same. --- CHANGELOG.md | 2 +- api/browser.go | 15 +++++--- api/testdata/contract.golden | 1 + docs/browser-tab.md | 5 ++- docs/ceilings.md | 9 ++--- internal/browser/actions.go | 29 +++++++++++---- internal/browser/browser.go | 16 ++++---- internal/browser/chrome.go | 10 +++++ internal/browser/downloads_live_test.go | 43 ++++++++++++++++++++++ internal/browser/fake_test.go | 22 +++++++---- internal/browser/rpc.go | 19 +++++++++- internal/browser/session_downloads.go | 31 ++++++++++++++++ internal/browser/session_downloads_test.go | 34 +++++++++++++++++ internal/browser/view.go | 9 ++++- internal/cli/browser.go | 17 +++++++-- internal/cli/browser_test.go | 13 +++++++ internal/cli/mcp_browser.go | 9 +++-- 17 files changed, 239 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30cd4b9a5..c30b1d92f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - The crosshair on a session browser's toolbar lets you leave notes on the page for the agent. The element under the pointer is outlined and named, a click opens a note beside it, and each note pins its element with a number, following it as the page scrolls. Notes wait in a tray floating over the bottom of the page, which never resizes under you. Hovering a note there lights its element, a click edits it, and Send (or Ctrl+Enter) types them into the session as one message: which element, a selector the agent's `browser_click` takes, the React component that drew it in a development build, and your note. Ctrl+Shift+E (Cmd+Shift+E on macOS) turns the crosshair on and off, and Esc leaves it. A send the session refuses keeps the notes and says why in the tray. -- A session's browser downloads files. A download the page starts, from a link, a button, a new tab or a script, lands in your Downloads folder under the name the page gives it, with ` (1)` added rather than overwriting a file already there. The agent's click or key press answers with the file's path and size, and the window shows a toast naming the file, with a button that opens its folder. Before, a download button in the session's browser did nothing. +- A session's browser downloads files. A download the page starts, from a link, a button, a new tab or a script, lands in your Downloads folder under the name the page gives it, with ` (1)` added rather than overwriting a file already there. The agent can also open a file's URL directly. The agent's click, key press or open answers with the file's path and size, and the window shows a toast naming the file, with a button that opens its folder. Before, a download button in the session's browser did nothing. ## [0.80.0] - 2026-10-27 diff --git a/api/browser.go b/api/browser.go index f4e8ffc58..1ac0e13bb 100644 --- a/api/browser.go +++ b/api/browser.go @@ -157,13 +157,16 @@ type BrowserTextOptions struct { } // BrowserHandle is one session's browser: the id later calls take, and enough -// for a tool result to say which window it is. +// for a tool result to say which window it is. Downloads, on an open or a +// navigate, are what that load downloaded: a URL that answers with a file +// saves it and leaves the page where it was. type BrowserHandle struct { - ID string `json:"id"` - URL string `json:"url"` - Title string `json:"title"` - Owner string `json:"owner"` - Headed bool `json:"headed"` + ID string `json:"id"` + URL string `json:"url"` + Title string `json:"title"` + Owner string `json:"owner"` + Headed bool `json:"headed"` + Downloads []BrowserDownload `json:"downloads,omitempty"` } // BrowserOpenOptions loads a URL in the session's browser, creating a headless diff --git a/api/testdata/contract.golden b/api/testdata/contract.golden index 15c9f6bc9..dfe014c17 100644 --- a/api/testdata/contract.golden +++ b/api/testdata/contract.golden @@ -323,6 +323,7 @@ BrowserHandle { title string owner string headed bool + downloads,omitempty []BrowserDownload } SessionIDOptions { diff --git a/docs/browser-tab.md b/docs/browser-tab.md index ea8b5f3a8..4f20e493c 100644 --- a/docs/browser-tab.md +++ b/docs/browser-tab.md @@ -68,7 +68,10 @@ 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 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. That answer also +`downloading:`, and the next page read says when it ends. `browser_open` and +`browser_navigate` on a URL that answers with a file download it the same way: +Chrome aborts that navigation and the page stays where it was, so the answer +is the handle with `downloads` rather than an error. That answer also waits until 1.5 seconds after the download began, because Chrome aborts a navigation started sooner and reloads the page. `browser-download` tells the window when a file is complete, and the window shows a toast naming it with a diff --git a/docs/ceilings.md b/docs/ceilings.md index cecd21bb2..e59e7a2f7 100644 --- a/docs/ceilings.md +++ b/docs/ceilings.md @@ -996,11 +996,10 @@ work when nobody knows it and that the call site never shows. The mechanism 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. Chrome aborts a navigation the page starts within about a second of a download (`net::ERR_ABORTED`, then it reloads the page; Chromium 155, with - downloads on or off), so a click or key press that downloaded holds its answer 1.5 seconds; a navigation the - person starts in the pane, or an agent's `browser_open` right after a download the page started on its own, - is not held. A download still running after ten seconds is reported in progress and named again when it - ends. No tool lists past downloads or cancels one, nothing caps their size, and closing the browser cancels - one still running. + downloads on or off), so an agent's click, key press, open or navigate that downloaded holds its answer + 1.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 ends. No tool lists past downloads or cancels one, + nothing caps their size, and closing the browser cancels one still running. ## Languages diff --git a/internal/browser/actions.go b/internal/browser/actions.go index 74db75455..490b6180e 100644 --- a/internal/browser/actions.go +++ b/internal/browser/actions.go @@ -182,18 +182,33 @@ func (s *Service) Navigate(id, rawURL string) (Handle, error) { if err != nil { return Handle{}, err } - err = runPage(func(ctx context.Context) error { - return sess.page.Navigate(ctx, u) - }) - if err != nil { - s.drop(sess) + if err := s.goTo(sess, u); err != nil { return Handle{}, err } - s.remember(sess, u, "") - s.agentActed(sess, api.BrowserAgentAct{Action: "navigate", URL: u}) return s.handleOf(sess), nil } +// goTo is the agent loading u in sess's page. A download leaves the page +// where it was, so its URL is not remembered as the page's. +func (s *Service) goTo(sess *session, u string) error { + var download bool + if err := runPage(func(ctx context.Context) error { + var err error + download, err = s.navigate(ctx, sess, u) + return err + }); err != nil { + s.drop(sess) + return err + } + if download { + s.touch(sess) + } else { + s.remember(sess, u, "") + } + s.agentActed(sess, api.BrowserAgentAct{Action: "navigate", URL: u}) + return nil +} + // Reload reloads the current page. func (s *Service) Reload(id string) error { return s.act(id, api.BrowserAgentAct{Action: "reload"}, func(p Page, ctx context.Context) error { return p.Reload(ctx) }) diff --git a/internal/browser/browser.go b/internal/browser/browser.go index dae949cdb..413a0809e 100644 --- a/internal/browser/browser.go +++ b/internal/browser/browser.go @@ -12,7 +12,6 @@ import ( "time" "github.com/chromedp/cdproto/network" - "github.com/lichdotdev/lich/api" ) // idleAfter is how long a headless browser no pane shows sits unused before @@ -121,14 +120,9 @@ func (s *Service) Open(owner, rawURL string) (Handle, error) { if sess == nil { return s.launchFresh(owner, u, false, true) } - if err := runPage(func(ctx context.Context) error { - return sess.page.Navigate(ctx, u) - }); err != nil { - s.drop(sess) + if err := s.goTo(sess, u); err != nil { return Handle{}, err } - s.remember(sess, u, "") - s.agentActed(sess, api.BrowserAgentAct{Action: "navigate", URL: u}) return s.handleOf(sess), nil } @@ -278,7 +272,13 @@ func (s *Service) start(spec launchSpec) (Handle, error) { return err } } - return page.Navigate(ctx, spec.url) + download, err := s.navigate(ctx, sess, spec.url) + if download { + s.mu.Lock() + sess.url = "about:blank" + s.mu.Unlock() + } + return err }); err != nil { return Handle{}, errors.Join(err, page.Close(), os.RemoveAll(spec.dir)) } diff --git a/internal/browser/chrome.go b/internal/browser/chrome.go index e1a52f967..2891c7cd3 100644 --- a/internal/browser/chrome.go +++ b/internal/browser/chrome.go @@ -3,6 +3,7 @@ package browser import ( "context" "encoding/json" + "errors" "fmt" "log/slog" "os" @@ -129,6 +130,9 @@ func (p *chromePage) Navigate(ctx context.Context, url string) error { if err != nil { return err } + if errorText == abortedText { + return errNavAborted + } if errorText != "" { return fmt.Errorf("page load error %s", errorText) } @@ -136,6 +140,12 @@ func (p *chromePage) Navigate(ctx context.Context, url string) error { })) } +// abortedText is Chrome's answer to a navigation that did not load a page, +// among them one that became a download. +const abortedText = "net::ERR_ABORTED" + +var errNavAborted = errors.New("page load error " + abortedText) + // navGrace is how long after an action a navigation it starts may take to // replace the document: until then the old one reads as complete. const navGrace = 150 * time.Millisecond diff --git a/internal/browser/downloads_live_test.go b/internal/browser/downloads_live_test.go index 75f678aca..394200a17 100644 --- a/internal/browser/downloads_live_test.go +++ b/internal/browser/downloads_live_test.go @@ -1,6 +1,7 @@ package browser import ( + "context" "fmt" "net/http" "net/http/httptest" @@ -109,3 +110,45 @@ func TestChromeDownloadsLandInDownloadsAndThePageStays(t *testing.T) { t.Fatalf("Downloads holds %v, want the six files and nothing else", names) } } + +func TestChromeOpenAndNavigateToAFileDownloadIt(t *testing.T) { + if _, err := chromium.FindBrowser(exec.LookPath); err != nil { + t.Skip("no Chromium-family browser on this machine") + } + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + t.Setenv("XDG_DOWNLOAD_DIR", filepath.Join(dir, "Downloads")) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + name := strings.TrimPrefix(r.URL.Path, "/") + w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", name)) + fmt.Fprint(w, name) + })) + t.Cleanup(srv.Close) + s := newService(chromeLaunch, func(string, string) (string, error) { return t.TempDir(), nil }, time.Now, idleAfter) + t.Cleanup(func() { + if err := s.Cleanup(); err != nil { + t.Error(err) + } + }) + // The first open launches the browser on the file, the second loads one + // in it, and the navigate a third. + calls := []func(context.Context, api.BrowserOpenOptions) (api.BrowserHandle, error){s.OpenURL, s.OpenURL, s.Go} + for i, call := range calls { + name := fmt.Sprintf("direct%d.csv", i) + h, err := call(t.Context(), api.BrowserOpenOptions{ID: "s", URL: srv.URL + "/" + name}) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + want := filepath.Join(dir, "Downloads", name) + if len(h.Downloads) != 1 || h.Downloads[0].Path != want || h.Downloads[0].State != api.DownloadCompleted { + t.Fatalf("%s: downloads %+v, want %s completed", name, h.Downloads, want) + } + if b, err := os.ReadFile(want); err != nil || string(b) != name { + t.Fatalf("%s holds %q (%v)", want, b, err) + } + if h.URL != "about:blank" { + t.Fatalf("%s: the page moved to %s", name, h.URL) + } + } +} diff --git a/internal/browser/fake_test.go b/internal/browser/fake_test.go index 17a6029a9..0a4b483ee 100644 --- a/internal/browser/fake_test.go +++ b/internal/browser/fake_test.go @@ -31,13 +31,15 @@ type fakePage struct { onFrame func(string) onInfo func(url, title string) onDownload func(guid string, d api.BrowserDownload) - // onClick runs after a click, as what the page does in answer to it. - onClick func() - casting bool - viewports []Viewport - mice []api.BrowserMouseOptions - mouseErr error - clickErr error + // onClick runs after a click, as what the page does in answer to it; + // onNavigate as a navigation starts. + onClick func() + onNavigate func() + casting bool + viewports []Viewport + mice []api.BrowserMouseOptions + mouseErr error + clickErr error // dead is a Chrome whose process ended under the page. dead bool keys []api.BrowserKeyOptions @@ -79,6 +81,12 @@ func (p *fakePage) Close() error { } func (p *fakePage) Navigate(_ context.Context, url string) error { + p.mu.Lock() + then := p.onNavigate + p.mu.Unlock() + if then != nil { + then() + } p.mu.Lock() defer p.mu.Unlock() if p.navErr != nil { diff --git a/internal/browser/rpc.go b/internal/browser/rpc.go index 0021f1081..56d39e2fe 100644 --- a/internal/browser/rpc.go +++ b/internal/browser/rpc.go @@ -20,7 +20,10 @@ func (s *Service) Show(_ context.Context, opts api.SessionIDOptions) (api.Browse // card has not opened a window. func (s *Service) OpenURL(_ context.Context, opts api.BrowserOpenOptions) (api.BrowserHandle, error) { h, err := s.Open(opts.ID, opts.URL) - return handleAPI(h), err + if err != nil { + return api.BrowserHandle{}, err + } + return s.withDownloads(handleAPI(h)), nil } // Read returns the page the agent is looking at. @@ -79,7 +82,19 @@ func (s *Service) Capture(_ context.Context, opts api.BrowserScreenshotOptions) // Go loads a URL in this session's already-open browser. func (s *Service) Go(_ context.Context, opts api.BrowserOpenOptions) (api.BrowserHandle, error) { h, err := s.Navigate(opts.ID, opts.URL) - return handleAPI(h), err + if err != nil { + return api.BrowserHandle{}, err + } + return s.withDownloads(handleAPI(h)), nil +} + +// withDownloads adds to h the file its navigation downloaded, as a click's +// answer does. +func (s *Service) withDownloads(h api.BrowserHandle) api.BrowserHandle { + if sess := s.current(h.Owner); sess != nil { + h.Downloads = s.downloadsAfter(sess, downloadWait) + } + return h } // ReloadPage reloads this session's page. diff --git a/internal/browser/session_downloads.go b/internal/browser/session_downloads.go index c3b12a0d4..f4b299e9a 100644 --- a/internal/browser/session_downloads.go +++ b/internal/browser/session_downloads.go @@ -1,6 +1,8 @@ package browser import ( + "context" + "errors" "time" "github.com/lichdotdev/lich/api" @@ -15,6 +17,10 @@ const downloadWait = 10 * time.Second // downloadPoll is how often that wait looks. const downloadPoll = 50 * time.Millisecond +// downloadNotice bounds how long a navigation Chrome aborted waits to hear +// whether it became a download: the two arrive in either order. +const downloadNotice = time.Second + // downloadQuiet is how long after a download begins a click or a key press // holds its answer. Chrome aborts a navigation the page starts within about a // second of a download (net::ERR_ABORTED, then it reloads the page): the @@ -108,3 +114,28 @@ func (s *Service) untold(sess *session) []api.BrowserDownload { sess.downloads = kept return out } + +// navigate loads u in sess's page. A URL that answers with a file is a +// download, not a page: Chrome aborts the navigation and the page stays where +// it was, which is a load that worked. +func (s *Service) navigate(ctx context.Context, sess *session, u string) (download bool, err error) { + before := time.Now() + err = sess.page.Navigate(ctx, u) + if errors.Is(err, errNavAborted) && s.downloadSince(sess, before) { + return true, nil + } + return false, err +} + +func (s *Service) downloadSince(sess *session, t time.Time) bool { + deadline := time.Now().Add(downloadNotice) + for { + s.mu.Lock() + began := sess.downloadBegun.After(t) + s.mu.Unlock() + if began || time.Now().After(deadline) { + return began + } + time.Sleep(downloadPoll) + } +} diff --git a/internal/browser/session_downloads_test.go b/internal/browser/session_downloads_test.go index a446950ab..11f91638f 100644 --- a/internal/browser/session_downloads_test.go +++ b/internal/browser/session_downloads_test.go @@ -1,6 +1,8 @@ package browser import ( + "context" + "errors" "reflect" "testing" "time" @@ -115,3 +117,35 @@ func TestClickHoldsItsAnswerForTheQuietAfterADownload(t *testing.T) { t.Fatalf("the click answered %v after its download began, before the quiet", took) } } + +func TestOpenAURLThatDownloadsAnswersWithTheFileAndKeepsThePage(t *testing.T) { + s, launch, _ := viewService(t) + if _, err := s.Open("s", "https://example.com/app"); err != nil { + t.Fatal(err) + } + p := launch.pages[0] + p.navErr = errNavAborted + p.onNavigate = func() { + p.onDownload("g1", api.BrowserDownload{Path: "/dl/r.csv", Bytes: 3, State: api.DownloadCompleted}) + } + for _, call := range []func(context.Context, api.BrowserOpenOptions) (api.BrowserHandle, error){s.OpenURL, s.Go} { + h, err := call(t.Context(), api.BrowserOpenOptions{ID: "s", URL: "https://example.com/r.csv"}) + if err != nil { + t.Fatal(err) + } + if len(h.Downloads) != 1 || h.Downloads[0].Path != "/dl/r.csv" || h.URL != "https://example.com/app" { + t.Fatalf("handle %+v, want the download and the page it left alone", h) + } + } +} + +func TestAnAbortedNavigationThatDownloadedNothingStaysAnError(t *testing.T) { + s, launch, _ := viewService(t) + if _, err := s.Open("s", "https://example.com/app"); err != nil { + t.Fatal(err) + } + launch.pages[0].navErr = errNavAborted + if _, err := s.OpenURL(t.Context(), api.BrowserOpenOptions{ID: "s", URL: "https://example.com/x"}); !errors.Is(err, errNavAborted) { + t.Fatalf("err = %v, want the abort", err) + } +} diff --git a/internal/browser/view.go b/internal/browser/view.go index 2602df6e1..89043b58b 100644 --- a/internal/browser/view.go +++ b/internal/browser/view.go @@ -100,7 +100,14 @@ func (s *Service) Nav(owner, action, rawURL string) error { if err != nil { return err } - fn = func(p Page, ctx context.Context) error { return p.Navigate(ctx, u) } + sess, err := s.live(owner) + if err != nil { + return err + } + fn = func(_ Page, ctx context.Context) error { + _, err := s.navigate(ctx, sess, u) + return err + } default: return fmt.Errorf("unknown browser action %q", action) } diff --git a/internal/cli/browser.go b/internal/cli/browser.go index c080fae36..7fa72cc8b 100644 --- a/internal/cli/browser.go +++ b/internal/cli/browser.go @@ -65,8 +65,7 @@ func (c *client) browserOpen(args []string) error { if err != nil { return err } - fmt.Fprintln(c.stdout, h.URL) - return nil + return c.printHandle(h) } func (c *client) browserInfo(args []string) error { @@ -190,8 +189,7 @@ func (c *client) browserNavigate(args []string) error { if err != nil { return err } - fmt.Fprintln(c.stdout, h.URL) - return nil + return c.printHandle(h) } func (c *client) browserScroll(args []string) error { @@ -293,3 +291,14 @@ func (t *targetFlags) target() api.BrowserTarget { } return api.BrowserTarget{Index: t.index, Selector: sel, X: t.x, Y: t.y} } + +// printHandle prints where the page is, and a line for each file the load +// downloaded. +func (c *client) printHandle(h api.BrowserHandle) error { + lines := []string{h.URL} + for _, d := range h.Downloads { + lines = append(lines, downloadLine(d)) + } + _, err := fmt.Fprintln(c.stdout, strings.Join(lines, "\n")) + return err +} diff --git a/internal/cli/browser_test.go b/internal/cli/browser_test.go index 7e8099d99..42320129a 100644 --- a/internal/cli/browser_test.go +++ b/internal/cli/browser_test.go @@ -24,6 +24,19 @@ func TestBrowserOpenCallsThisSessionsBrowser(t *testing.T) { } } +func TestBrowserOpenNamesAFileItDownloaded(t *testing.T) { + f := newFakeLich(t, `{"id":"b1","url":"about:blank","owner":"s1", + "downloads":[{"path":"/home/u/Downloads/r.csv","bytes":12,"state":"completed"}]}`) + + code, stdout, stderr := run(t, f, "browser", "open", "https://example.com/r.csv") + if code != 0 { + t.Fatalf("exit = %d, stderr = %q", code, stderr) + } + if want := "about:blank\ndownloaded: /home/u/Downloads/r.csv (12 B)\n"; stdout != want { + t.Errorf("stdout = %q, want %q", stdout, want) + } +} + func TestBrowserClickSendsATarget(t *testing.T) { f := newFakeLich(t, `null`) diff --git a/internal/cli/mcp_browser.go b/internal/cli/mcp_browser.go index 70d51f967..6d0109a91 100644 --- a/internal/cli/mcp_browser.go +++ b/internal/cli/mcp_browser.go @@ -18,7 +18,9 @@ var browserTools = []mcpTool{ "this session has none. It runs headless: the user sees the same page in a tab " + "beside this session, which lich offers to open, so no window of its own is needed " + "(headed is false, and that is expected). The URL must be http, https, or omitted " + - "(about:blank). file, javascript and data URLs are refused.", + "(about:blank). file, javascript and data URLs are refused. A URL that answers with a " + + "file downloads it to the user's Downloads folder and leaves the page as it was; " + + "downloads in the answer names its path.", Schema: schema(map[string]any{ "url": property("string", "Page to load. Omit for about:blank."), }), @@ -137,8 +139,9 @@ var browserTools = []mcpTool{ }, }, { - Name: "browser_navigate", - Description: "Load a URL in this session's already-open browser. Same allowlist as browser_open.", + Name: "browser_navigate", + Description: "Load a URL in this session's already-open browser. Same allowlist as browser_open, " + + "and a URL that answers with a file downloads it as browser_open does.", Schema: schema(map[string]any{ "url": property("string", "http(s) URL, or about:blank."), }, "url"), From fa66b2edf67bc46c715797b402de0b625992447b Mon Sep 17 00:00:00 2001 From: omartelo Date: Tue, 27 Oct 2026 17:57:57 -0300 Subject: [PATCH 3/3] fix(browser): download tests on Windows The live download tests moved HOME and USERPROFILE to keep files out of the runner's Downloads, and Chrome on Windows does not start with USERPROFILE moved (websocket url timeout). They now point the download folder itself at a temp dir. An open that became a download was taken for an error on Windows: the navigation and the download can read the same instant from its clock, and the check wanted the download strictly after. --- internal/browser/chrome.go | 6 +++++- internal/browser/downloads_live_test.go | 27 +++++++++++++------------ internal/browser/session_downloads.go | 4 +++- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/internal/browser/chrome.go b/internal/browser/chrome.go index 2891c7cd3..a058ce363 100644 --- a/internal/browser/chrome.go +++ b/internal/browser/chrome.go @@ -75,11 +75,15 @@ type chromePage struct { dl *downloads } +// downloadFolder is downloadDir, which the live tests point at a folder of +// their own: on Windows, Chrome would not start with USERPROFILE moved. +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 { - dir, err := downloadDir() + dir, err := downloadFolder() if err != nil { return err } diff --git a/internal/browser/downloads_live_test.go b/internal/browser/downloads_live_test.go index 394200a17..4ee48726e 100644 --- a/internal/browser/downloads_live_test.go +++ b/internal/browser/downloads_live_test.go @@ -31,12 +31,7 @@ func TestChromeDownloadsLandInDownloadsAndThePageStays(t *testing.T) { if _, err := chromium.FindBrowser(exec.LookPath); err != nil { t.Skip("no Chromium-family browser on this machine") } - dir := t.TempDir() - // Every OS's home, so the download folder is this test's and never the - // machine user's. - t.Setenv("HOME", dir) - t.Setenv("USERPROFILE", dir) - t.Setenv("XDG_DOWNLOAD_DIR", filepath.Join(dir, "Downloads")) + dir := testDownloads(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if name, ok := strings.CutPrefix(r.URL.Path, "/file/"); ok { w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", name)) @@ -72,7 +67,7 @@ func TestChromeDownloadsLandInDownloadsAndThePageStays(t *testing.T) { if err != nil { t.Fatal(err) } - want := filepath.Join(dir, "Downloads", id+".csv") + want := filepath.Join(dir, id+".csv") got := info.Downloads for deadline := time.Now().Add(10 * time.Second); len(got) == 0 && time.Now().Before(deadline); { // A new tab hands its URL to the page after the click returns. @@ -98,7 +93,7 @@ func TestChromeDownloadsLandInDownloadsAndThePageStays(t *testing.T) { } }) } - entries, err := os.ReadDir(filepath.Join(dir, "Downloads")) + entries, err := os.ReadDir(dir) if err != nil { t.Fatal(err) } @@ -115,10 +110,7 @@ func TestChromeOpenAndNavigateToAFileDownloadIt(t *testing.T) { if _, err := chromium.FindBrowser(exec.LookPath); err != nil { t.Skip("no Chromium-family browser on this machine") } - dir := t.TempDir() - t.Setenv("HOME", dir) - t.Setenv("USERPROFILE", dir) - t.Setenv("XDG_DOWNLOAD_DIR", filepath.Join(dir, "Downloads")) + dir := testDownloads(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { name := strings.TrimPrefix(r.URL.Path, "/") w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", name)) @@ -140,7 +132,7 @@ func TestChromeOpenAndNavigateToAFileDownloadIt(t *testing.T) { if err != nil { t.Fatalf("%s: %v", name, err) } - want := filepath.Join(dir, "Downloads", name) + want := filepath.Join(dir, name) if len(h.Downloads) != 1 || h.Downloads[0].Path != want || h.Downloads[0].State != api.DownloadCompleted { t.Fatalf("%s: downloads %+v, want %s completed", name, h.Downloads, want) } @@ -152,3 +144,12 @@ func TestChromeOpenAndNavigateToAFileDownloadIt(t *testing.T) { } } } + +// testDownloads is the folder Chrome saves into for this test, never the +// machine user's Downloads. +func testDownloads(t *testing.T) string { + dir := filepath.Join(t.TempDir(), "Downloads") + downloadFolder = func() (string, error) { return dir, nil } + t.Cleanup(func() { downloadFolder = downloadDir }) + return dir +} diff --git a/internal/browser/session_downloads.go b/internal/browser/session_downloads.go index f4b299e9a..22f17d728 100644 --- a/internal/browser/session_downloads.go +++ b/internal/browser/session_downloads.go @@ -131,7 +131,9 @@ func (s *Service) downloadSince(sess *session, t time.Time) bool { deadline := time.Now().Add(downloadNotice) for { s.mu.Lock() - began := sess.downloadBegun.After(t) + // Not After: Windows' clock can give the navigation and the download + // the same instant. + began := !sess.downloadBegun.Before(t) s.mu.Unlock() if began || time.Now().After(deadline) { return began