fix(download): report a filename suggestion dropped by another extension

CHANGELOG.md

@@ -9,6 +9,13 @@ Starting from 0.2.0, CLI / Extension / DSH Plugin share the same version number.
 
 ### Fixed
 
+- Extension: `download` no longer silently reports a trace-folder path when a
+  coexisting extension won the `chrome.downloads.onDeterminingFilename` race.
+  Chromium lets only one extension name a download and never tells the losers, so
+  the per-call `BrowserSkill/<trace>/` grouping was dropped while the result still
+  implied the folder layout. The result now carries a `note` when the completed file
+  is outside the trace folder, so the caller trusts the real path
+  ([#361](https://github.com/Tencent/BrowserSkill/issues/361)).
 - Extension: input to a background Agent Window tab no longer keeps failing with
   `input_not_ready` after Chrome drops the session's focus override without a detach
   ([#355](https://github.com/Tencent/BrowserSkill/issues/355)). The session's

apps/extension/src/tools/__tests__/file-transfer.test.ts

@@ -80,10 +80,16 @@ function popupDownloadFakes() {
       new Promise<chrome.downloads.DownloadFilenameSuggestion | undefined>((resolve) => {
         onDeterminingFilename.emit(item, resolve);
       }),
-    finish: (item: chrome.downloads.DownloadItem) => {
+    /**
+     * Completes the download, storing it at `finalPath` (defaults to flat in
+     * the download dir). Tests that care about the trace folder pass the stored
+     * path explicitly: honouring it, or losing it to a coexisting extension that
+     * won the `onDeterminingFilename` race.
+     */
+    finish: (item: chrome.downloads.DownloadItem, finalPath?: string) => {
       const done = {
         ...item,
-        filename: `/profile/Downloads/${item.filename}`,
+        filename: finalPath ?? `/profile/Downloads/${item.filename}`,
         state: "complete",
         fileSize: 4,
       } as chrome.downloads.DownloadItem;
@@ -1293,4 +1299,66 @@ describe("file transfer tools", () => {
       data: { effect_state: "unknown", phase: "trigger" },
     });
   });
+
+  it.each([
+    ["Chrome honours the trace folder", "/profile/Downloads/BrowserSkill/tr_60/report-60.csv"],
+    // The uniquify suffix lands on the basename, so it must not mask an intact folder.
+    [
+      "Chrome uniquifies the repeated name",
+      "/profile/Downloads/BrowserSkill/tr_60/report-60 (1).csv",
+    ],
+  ])("reports no dropped suggestion when %s", async (_label, finalPath) => {
+    const fakes = popupDownloadFakes();
+    const download = fakes.item(60, "https://example.test/export?id=60");
+    let suggested: Promise<chrome.downloads.DownloadFilenameSuggestion | undefined> | undefined;
+
+    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);
+        suggested = fakes.offer(download);
+        fakes.finish(download, finalPath);
+        return { tab_id: 4, x: 10, y: 10 };
+      },
+    });
+
+    await expect(suggested).resolves.toEqual({
+      filename: "BrowserSkill/tr_60/report-60.csv",
+      conflictAction: "overwrite",
+    });
+    expect(result).toMatchObject({ item: { filename: finalPath } });
+    expect(result).not.toHaveProperty("suggestionDropped");
+  });
+
+  it("flags a dropped trace folder when another extension wins the name", async () => {
+    const fakes = popupDownloadFakes();
+    const download = fakes.item(61, "https://example.test/export?id=61");
+
+    const result = await captureBrowserDownload({
+      cdp: silentCdp(),
+      target: { tabId: 4 },
+      downloads: fakes.downloads,
+      navigationTargets: fakes.navigationTargets,
+      browserRelativeDir: "BrowserSkill/tr_61",
+      timeoutMs: 1_000,
+      trigger: async (markDispatched) => {
+        markDispatched();
+        fakes.popup(4, download.url);
+        fakes.offer(download);
+        fakes.finish(download, "/profile/Downloads/report-61.csv");
+        return { tab_id: 4, x: 10, y: 10 };
+      },
+    });
+
+    expect(result).toMatchObject({
+      item: { filename: "/profile/Downloads/report-61.csv" },
+      suggestionDropped: true,
+    });
+  });
 });

apps/extension/src/tools/download-capture.ts

@@ -81,6 +81,12 @@ export interface DownloadCaptureOptions {
 export interface DownloadCaptureResult {
   click: ClickResult;
   item: chrome.downloads.DownloadItem;
+  /**
+   * Set when the completed path is not under `browserRelativeDir`, so another
+   * extension won the `chrome.downloads.onDeterminingFilename` race and the
+   * trace-scoped folder grouping was dropped. The download itself succeeded.
+   */
+  suggestionDropped?: boolean;
 }
 
 interface DownloadIntent {
@@ -102,6 +108,27 @@ function safeBasename(filename: string): string {
   return basename && basename !== "." && basename !== ".." ? basename : "download";
 }
 
+/** Path segments of a Chrome download path, ignoring separator flavour. */
+function pathSegments(path: string): string[] {
+  return path.split(/[\\/]+/).filter((segment) => segment.length > 0);
+}
+
+/**
+ * Chromium lets only one extension answer `onDeterminingFilename`, and never
+ * tells the losers they lost. Chrome may also uniquify a repeated name with a
+ * `" (1)"` suffix, so compare the directory portion only: if the completed file
+ * is not under the trace-scoped `browserRelativeDir`, the suggestion was
+ * overridden elsewhere even though the download itself succeeded.
+ */
+function suggestionWasDropped(filename: string, browserRelativeDir: string): boolean {
+  const wanted = pathSegments(browserRelativeDir);
+  if (wanted.length === 0) return false;
+  const actual = pathSegments(filename).slice(0, -1);
+  return !wanted.every(
+    (segment, index) => actual[index + actual.length - wanted.length] === segment,
+  );
+}
+
 function sameTarget(source: { tabId?: number; sessionId?: string }, target: CdpTarget): boolean {
   return source.tabId === target.tabId && source.sessionId === target.sessionId;
 }
@@ -399,7 +426,8 @@ export async function captureBrowserDownload(
     click = triggered;
     const item = await completion;
     succeeded = true;
-    return { click, item };
+    const suggestionDropped = suggestionWasDropped(item.filename, options.browserRelativeDir);
+    return { click, item, ...(suggestionDropped ? { suggestionDropped } : {}) };
   } catch (err) {
     const effect: TransferEffectState =
       capturedId !== undefined

apps/extension/src/tools/download.ts

@@ -16,6 +16,15 @@ import { enforceAgentWindow, isRpcError, lookupSession, resolveTargetTab } from
 
 let downloadActive = false;
 
+/**
+ * Result note explaining that the completed file landed outside the trace-scoped
+ * folder. Chromium lets only one extension answer
+ * `chrome.downloads.onDeterminingFilename` and never tells the losers they lost,
+ * so a coexisting download-manager extension silently overrides the suggestion.
+ */
+export const DOWNLOAD_SUGGESTION_DROPPED_NOTE =
+  "the completed download is not under the trace folder: another extension took over chrome.downloads.onDeterminingFilename, so the per-call folder grouping was dropped (the file itself downloaded fine) — disable that extension's download renaming to restore it";
+
 export type { DownloadsApi, NavigationTargetsApi } from "./download-capture";
 
 export interface DownloadDeps extends InteractionDeps {
@@ -71,6 +80,7 @@ export async function handleDownload(
       mime: item.mime || undefined,
       danger: item.danger,
       browser_path: item.filename,
+      ...(capture.suggestionDropped ? { note: DOWNLOAD_SUGGESTION_DROPPED_NOTE } : {}),
     };
   } finally {
     downloadActive = false;

apps/extension/src/transport/types.ts

@@ -723,6 +723,13 @@ export interface DownloadResult {
   danger?: string;
   browser_path?: string;
   transfer_id?: string;
+  /**
+   * Set when the completed file is not under the trace-scoped
+   * `browserRelativeDir`, meaning another extension won the
+   * `chrome.downloads.onDeterminingFilename` race. The download succeeded; only
+   * the per-call folder grouping was lost.
+   */
+  note?: string;
 }
 
 // --------------------------------------------------------------------------

crates/bsk-cli/skill/references/help-and-recovery.md

@@ -33,6 +33,7 @@ substring is enough.
 | Unknown tab/session | List current tabs/sessions; never guess IDs or use another task's session. |
 | 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. |
+| Download `note` about the trace folder | The file downloaded fine but landed outside the per-call folder: another extension took over `chrome.downloads.onDeterminingFilename`. Trust the reported path; do not assume the folder layout. Ask the user to disable that extension's download renaming to restore it. |
 | Unsupported operation | Use available capabilities; suggest updating only if the missing feature is needed. |
 
 Navigation alone (including deprecated help outcome `navigated`) is not completion.

crates/bsk-protocol/src/tools/file_transfer.rs

@@ -104,6 +104,12 @@ pub struct DownloadResult {
     /// Opaque id returned by the daemon to the CLI.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub transfer_id: Option<String>,
+    /// Set when the completed file is not under the trace-scoped
+    /// `browserRelativeDir`, meaning another extension won the
+    /// `chrome.downloads.onDeterminingFilename` race. The download succeeded;
+    /// only the per-call folder grouping was lost.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub note: Option<String>,
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)]
@@ -210,4 +216,32 @@ mod tests {
         assert!(value.get("browser_relative_dir").is_none());
         assert!(value.get("max_byte_size").is_none());
     }
+
+    fn download_result(note: Option<String>) -> DownloadResult {
+        DownloadResult {
+            tab_id: 4,
+            used_ref: None,
+            used_selector: None,
+            suggested_filename: "report.csv".into(),
+            byte_size: 12,
+            mime: None,
+            danger: None,
+            browser_path: Some("/profile/Downloads/BrowserSkill/tr_1/report.csv".into()),
+            transfer_id: None,
+            note,
+        }
+    }
+
+    #[test]
+    fn download_result_omits_the_note_when_the_suggestion_was_honoured() {
+        let value = serde_json::to_value(download_result(None)).unwrap();
+        assert!(value.get("note").is_none());
+    }
+
+    #[test]
+    fn download_result_carries_the_note_when_the_suggestion_was_dropped() {
+        let value =
+            serde_json::to_value(download_result(Some("another extension won".into()))).unwrap();
+        assert_eq!(value["note"], "another extension won");
+    }
 }

packages/dsh-plugin-browserskill/skill/references/help-and-recovery.md

@@ -44,4 +44,8 @@ backends to bypass limits. Borrow confirmation still applies.
 - Timeout/unknown effect: inspect before retrying; the action may have happened.
 - Unconfirmed fill: read the field. Formatting may satisfy the goal; correct only a
   remaining difference instead of blindly refilling or requesting help.
+- Download `note` about the trace folder: the file downloaded fine but landed outside
+  the per-call folder, because another extension took over
+  `chrome.downloads.onDeterminingFilename`. Trust the reported path; do not assume
+  the folder layout. Ask the user to disable that extension's download renaming.
 - Other errors: follow the hint; on unrecoverable failure, report and stop the owned session.