From 1df5ddbe7dbf48353adfae003e0e3a16eda4e780 Mon Sep 17 00:00:00 2001 From: lymerin <884917500@qq.com> Date: Tue, 29 Sep 2026 00:14:59 +0800 Subject: [PATCH 1/3] fix(download): detect ignored transfer directory --- .../src/tools/__tests__/file-transfer.test.ts | 62 +++++++++++++++++-- apps/extension/src/tools/download-capture.ts | 20 +++++- apps/extension/src/transport/types.ts | 1 + crates/bsk-cli/skill/references/files.md | 2 + .../skill/references/help-and-recovery.md | 6 ++ crates/bsk-cli/src/cli/render_error.rs | 17 +++++ 6 files changed, 102 insertions(+), 6 deletions(-) diff --git a/apps/extension/src/tools/__tests__/file-transfer.test.ts b/apps/extension/src/tools/__tests__/file-transfer.test.ts index 0cd7948b..62818d1a 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -41,6 +41,7 @@ function fakeEvent unknown>() { function popupDownloadFakes() { const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); + const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); const onDeterminingFilename = fakeEvent< ( @@ -53,7 +54,7 @@ function popupDownloadFakes() { const completed = new Map(); const downloads: DownloadsApi = { onCreated, - onChanged: fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(), + onChanged, onDeterminingFilename, search: vi.fn(async ({ id }: chrome.downloads.DownloadQuery) => { const item = id === undefined ? undefined : completed.get(id); @@ -80,16 +81,31 @@ function popupDownloadFakes() { new Promise((resolve) => { onDeterminingFilename.emit(item, resolve); }), - finish: (item: chrome.downloads.DownloadItem) => { + finish: ( + item: chrome.downloads.DownloadItem, + filename = `/profile/Downloads/BrowserSkill/tr_${item.id}/${item.filename}`, + ) => { const done = { ...item, - filename: `/profile/Downloads/${item.filename}`, + filename, state: "complete", fileSize: 4, } as chrome.downloads.DownloadItem; completed.set(item.id, done); onCreated.emit(done); }, + finishChanged: (item: chrome.downloads.DownloadItem, filename: string) => { + completed.set(item.id, { + ...item, + filename, + state: "complete", + fileSize: 4, + } as chrome.downloads.DownloadItem); + onChanged.emit({ + id: item.id, + state: { current: "complete" }, + } as chrome.downloads.DownloadDelta); + }, popup: (sourceTabId: number, url: string) => onCreatedNavigationTarget.emit({ sourceTabId, @@ -669,7 +685,7 @@ describe("file transfer tools", () => { } as chrome.downloads.DownloadItem; const completed = { ...initial, - filename: "/profile/Downloads/BrowserSkill/tr_1/result.zip", + filename: "C:\\Users\\tester\\Downloads\\BrowserSkill\\tr_1\\result.zip", state: "complete", fileSize: 12, } as chrome.downloads.DownloadItem; @@ -833,6 +849,7 @@ describe("file transfer tools", () => { } as chrome.downloads.DownloadItem; const complete = { ...initial, + filename: "/profile/Downloads/BrowserSkill/tr_21/candidate-first.bin", state: "complete", fileSize: 4, } as chrome.downloads.DownloadItem; @@ -1142,10 +1159,45 @@ describe("file transfer tools", () => { expect(result).toMatchObject({ tab_id: 4, suggested_filename: "report-50.csv", - browser_path: "/profile/Downloads/report-50.csv", + browser_path: "/profile/Downloads/BrowserSkill/tr_50/report-50.csv", }); }); + it.each([ + "C:\\Users\\tester\\Downloads\\report-60.csv", + "C:\\Users\\tester\\Documents\\report-60.csv", + ])( + "rejects a download outside its transfer directory without deleting %s", + async (filename) => { + const fakes = popupDownloadFakes(); + const download = fakes.item(60, "https://example.test/export?id=60"); + + const result = await captureBrowserDownload({ + cdp: silentCdp(), + target: { tabId: 4 }, + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + browserRelativeDir: "BrowserSkill/tr_60", + timeoutMs: 1_000, + trigger: async (markDispatched) => { + markDispatched(); + fakes.popup(4, download.url); + await fakes.offer(download); + fakes.finishChanged(download, filename); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(result).toMatchObject({ + code: "cdp_failed", + data: { reason: "download_path_mismatch", effect_state: "committed" }, + }); + expect(fakes.downloads.removeFile).not.toHaveBeenCalled(); + expect(fakes.downloads.cancel).not.toHaveBeenCalled(); + expect(result).not.toMatchObject({ data: { cleanup_state: "failed" } }); + }, + ); + it.each([ "navigation-first", "candidate-first", diff --git a/apps/extension/src/tools/download-capture.ts b/apps/extension/src/tools/download-capture.ts index 519f306e..9f5691f7 100644 --- a/apps/extension/src/tools/download-capture.ts +++ b/apps/extension/src/tools/download-capture.ts @@ -102,6 +102,12 @@ function safeBasename(filename: string): string { return basename && basename !== "." && basename !== ".." ? basename : "download"; } +function isInBrowserRelativeDir(filename: string, browserRelativeDir: string): boolean { + const parent = filename.split(/[\\/]/).slice(0, -1); + const expected = browserRelativeDir.split("/"); + return parent.slice(-expected.length).join("/") === browserRelativeDir; +} + function sameTarget(source: { tabId?: number; sessionId?: string }, target: CdpTarget): boolean { return source.tabId === target.tabId && source.sessionId === target.sessionId; } @@ -171,6 +177,7 @@ export async function captureBrowserDownload( let capturedId: number | undefined; let settled = false; let succeeded = false; + let preserveOutsideDownload = false; let failureResult: RpcError | undefined; let uniquenessTimer: ReturnType | undefined; let operationTimer: ReturnType | undefined; @@ -398,6 +405,17 @@ export async function captureBrowserDownload( } click = triggered; const item = await completion; + if (!isInBrowserRelativeDir(item.filename, options.browserRelativeDir)) { + // The final location may be user-selected, so leave the file in place. + preserveOutsideDownload = true; + failureResult = transferError( + "cdp_failed", + "download_path_mismatch", + "the browser did not use the requested download directory", + { effectState: "committed", phase: "download" }, + ); + return failureResult; + } succeeded = true; return { click, item }; } catch (err) { @@ -432,7 +450,7 @@ export async function captureBrowserDownload( const cleanupDeadline = Date.now() + CLEANUP_TIMEOUT_MS; const cleanups: Promise[] = []; if (options.cleanupTrigger) cleanups.push(options.cleanupTrigger(cleanupDeadline)); - if (!succeeded && capturedId !== undefined) { + if (!succeeded && !preserveOutsideDownload && capturedId !== undefined) { cleanups.push(cleanupClaimedDownload(options.downloads, capturedId)); } try { diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index 37c01491..cacb4040 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -74,6 +74,7 @@ export type RpcErrorReason = | "file_drop_target_unavailable" | "file_drop_failed" | "download_capture_failed" + | "download_path_mismatch" | "transfer_outcome_unknown" | "transfer_timeout" | "cleanup_failed"; diff --git a/crates/bsk-cli/skill/references/files.md b/crates/bsk-cli/skill/references/files.md index 58843e99..28ca76b7 100644 --- a/crates/bsk-cli/skill/references/files.md +++ b/crates/bsk-cli/skill/references/files.md @@ -17,5 +17,7 @@ Use agent-local paths, not browser-internal staging paths. A successful drop proves dispatch, not site acceptance; observe the attachment. - Download refuses overwrite by default; add `--overwrite` only when replacement is intended. Consult each command's help for other flags. +- For download directory conflicts with other extensions, see + [human steps and recovery](help-and-recovery.md). Remote upload/download are unsupported. diff --git a/crates/bsk-cli/skill/references/help-and-recovery.md b/crates/bsk-cli/skill/references/help-and-recovery.md index ae041cb7..bdf5f152 100644 --- a/crates/bsk-cli/skill/references/help-and-recovery.md +++ b/crates/bsk-cli/skill/references/help-and-recovery.md @@ -34,6 +34,12 @@ substring is enough. | Timeout or unknown effect | Inspect current state before retrying; the action may already have happened. | | `fill_value_mismatch` | Read the field: formatting may still satisfy the request. Correct only a remaining difference; no blind refill or immediate handoff. | | Unsupported operation | Use available capabilities; suggest updating only if the missing feature is needed. | +| `download_path_mismatch` | The browser completed the download outside BrowserSkill's transfer directory, so `bsk download` did not produce `--out`. The browser file is left in place; find it in the Downloads list and decide whether to use or remove it before retrying. A download manager such as Chrono may have overridden the filename suggestion; when one is present, disable or pause its renaming or download takeover. The browser may also have rejected the suggested filename or used a different save location; check both before retrying. Retry only if `--out` is still needed. | + +If both extensions suggest a name, updating or reinstalling BrowserSkill after +the other extension can make BrowserSkill's folder suggestion win. This changes +which extension wins the conflict; a later update to the other extension may +reverse it. Navigation alone (including deprecated help outcome `navigated`) is not completion. For other errors, follow the returned hint and inspect the current state. diff --git a/crates/bsk-cli/src/cli/render_error.rs b/crates/bsk-cli/src/cli/render_error.rs index 1063ff58..a5c9a242 100644 --- a/crates/bsk-cli/src/cli/render_error.rs +++ b/crates/bsk-cli/src/cli/render_error.rs @@ -71,6 +71,7 @@ pub mod reason { pub const FILE_DROP_TARGET_UNAVAILABLE: &str = "file_drop_target_unavailable"; pub const FILE_DROP_FAILED: &str = "file_drop_failed"; pub const DOWNLOAD_CAPTURE_FAILED: &str = "download_capture_failed"; + pub const DOWNLOAD_PATH_MISMATCH: &str = "download_path_mismatch"; pub const TRANSFER_OUTCOME_UNKNOWN: &str = "transfer_outcome_unknown"; pub const TRANSFER_TIMEOUT: &str = "transfer_timeout"; pub const SESSION_BUSY: &str = crate::rpc_reason::SESSION_BUSY; @@ -549,6 +550,13 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend ), exit_code: base.exit_code, }, + (ErrorCode::CdpFailed, reason::DOWNLOAD_PATH_MISMATCH) => RenderInfo { + summary: "the browser did not use the requested download directory", + hint: Some( + "find the saved file in the browser's Downloads list; check for a competing rename, a browser-rejected filename, or a different save location before retrying", + ), + exit_code: base.exit_code, + }, _ => base, } } @@ -817,6 +825,15 @@ mod tests { let info = info_for_error(ErrorCode::CdpFailed, Some(&download)); assert!(info.summary.contains("download could not be attributed")); + let path_mismatch = serde_json::json!({ + "reason": reason::DOWNLOAD_PATH_MISMATCH, + "effect_state": "committed", + "phase": "download" + }); + let info = info_for_error(ErrorCode::CdpFailed, Some(&path_mismatch)); + assert!(info.summary.contains("requested download directory")); + assert!(info.hint.unwrap().contains("browser-rejected filename")); + let unknown = serde_json::json!({ "reason": reason::TRANSFER_OUTCOME_UNKNOWN }); let info = info_for_error(ErrorCode::ProtocolError, Some(&unknown)); assert!(info.summary.contains("outcome could not be confirmed")); From edd71a6f29b11273947a3b230a37071c0a46b94c Mon Sep 17 00:00:00 2001 From: lymerin <884917500@qq.com> Date: Wed, 30 Sep 2026 15:32:36 +0800 Subject: [PATCH 2/3] style(download): format path mismatch test --- .../src/tools/__tests__/file-transfer.test.ts | 59 +++++++++---------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/apps/extension/src/tools/__tests__/file-transfer.test.ts b/apps/extension/src/tools/__tests__/file-transfer.test.ts index 62818d1a..306b670d 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -1166,37 +1166,34 @@ describe("file transfer tools", () => { it.each([ "C:\\Users\\tester\\Downloads\\report-60.csv", "C:\\Users\\tester\\Documents\\report-60.csv", - ])( - "rejects a download outside its transfer directory without deleting %s", - async (filename) => { - const fakes = popupDownloadFakes(); - const download = fakes.item(60, "https://example.test/export?id=60"); - - const result = await captureBrowserDownload({ - cdp: silentCdp(), - target: { tabId: 4 }, - downloads: fakes.downloads, - navigationTargets: fakes.navigationTargets, - browserRelativeDir: "BrowserSkill/tr_60", - timeoutMs: 1_000, - trigger: async (markDispatched) => { - markDispatched(); - fakes.popup(4, download.url); - await fakes.offer(download); - fakes.finishChanged(download, filename); - return { tab_id: 4, x: 10, y: 10 }; - }, - }); - - expect(result).toMatchObject({ - code: "cdp_failed", - data: { reason: "download_path_mismatch", effect_state: "committed" }, - }); - expect(fakes.downloads.removeFile).not.toHaveBeenCalled(); - expect(fakes.downloads.cancel).not.toHaveBeenCalled(); - expect(result).not.toMatchObject({ data: { cleanup_state: "failed" } }); - }, - ); + ])("rejects a download outside its transfer directory without deleting %s", async (filename) => { + const fakes = popupDownloadFakes(); + const download = fakes.item(60, "https://example.test/export?id=60"); + + const result = await captureBrowserDownload({ + cdp: silentCdp(), + target: { tabId: 4 }, + downloads: fakes.downloads, + navigationTargets: fakes.navigationTargets, + browserRelativeDir: "BrowserSkill/tr_60", + timeoutMs: 1_000, + trigger: async (markDispatched) => { + markDispatched(); + fakes.popup(4, download.url); + await fakes.offer(download); + fakes.finishChanged(download, filename); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(result).toMatchObject({ + code: "cdp_failed", + data: { reason: "download_path_mismatch", effect_state: "committed" }, + }); + expect(fakes.downloads.removeFile).not.toHaveBeenCalled(); + expect(fakes.downloads.cancel).not.toHaveBeenCalled(); + expect(result).not.toMatchObject({ data: { cleanup_state: "failed" } }); + }); it.each([ "navigation-first", From bcfddcc49842ee5146f7b28a8ce6e5255e0d617d Mon Sep 17 00:00:00 2001 From: lymerin <884917500@qq.com> Date: Wed, 30 Sep 2026 21:30:37 +0800 Subject: [PATCH 3/3] docs(download): explain final path validation --- apps/extension/src/tools/download-capture.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/extension/src/tools/download-capture.ts b/apps/extension/src/tools/download-capture.ts index 9f5691f7..b25734de 100644 --- a/apps/extension/src/tools/download-capture.ts +++ b/apps/extension/src/tools/download-capture.ts @@ -405,6 +405,7 @@ export async function captureBrowserDownload( } click = triggered; const item = await completion; + // Another extension can override our suggestion, so validate the completed path. if (!isInBrowserRelativeDir(item.filename, options.browserRelativeDir)) { // The final location may be user-selected, so leave the file in place. preserveOutsideDownload = true;