Skip to content

fix(download): detect ignored transfer directory - #362

Open
lymerin wants to merge 3 commits into
Tencent:mainfrom
lymerin:361
Open

lymerin wants to merge 3 commits into
Tencent:mainfrom
lymerin:361

Conversation

@lymerin

@lymerin lymerin commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #361

Problem

When BrowserSkill and Chrono both suggest a download filename, Chromium prioritizes the extension installed or updated most recently. If Chrono wins, the download can complete outside BrowserSkill’s BrowserSkill/tr_<id>/ directory.

The daemon only imports a browser file whose parent matches the per-transfer directory. A file saved elsewhere therefore cannot become the requested --out file. Previously, the failure did not explain that the suggested directory had been ignored.

Change

  • Request 1 — troubleshooting guidance: Document how to pause or disable the competing extension’s renaming or download takeover. Updating or reinstalling BrowserSkill so that it is more recent than the other extension can also change which suggestion wins; a later update to the other extension can reverse that result.
  • Request 2 — optional hint: Validate the completed download’s final path. Return download_path_mismatch with effect_state: committed when its parent does not match the per-transfer directory. Provide recovery guidance for competing extensions, browser-rejected filenames, and different save locations. Preserve the completed browser file because its location may have been chosen by the user, and direct users to locate it in Downloads before retrying.
  • Request 3 — optional skip-suggestion setting: Not implemented. Skipping the suggestion alone would leave the browser file outside the directory accepted by the daemon, so the transfer still could not complete.

The new parameterized test’s formatting was corrected in edd71a6. Commit bcfddcc adds a comment explaining why the completed path needs validation.

Validation

Tests and CI

  • Targeted extension tests (file-transfer, download-deadline, download-effect-state): 44 passed, including path mismatch and file preservation.
  • Targeted Rust error-rendering test, skill bundle validation, and git diff --check: passed.
  • Latest CI run for bcfddcc: all checks passed, including Biome and Windows installer tests under PowerShell 5.1 and 7.6.6.

Real Windows Edge validation of this PR’s build

The tested BrowserSkill CLI, daemon, and extension were built locally from PR commit edd71a6, which includes the #361 fix. The source tree’s package version remained 0.3.1. Automatic CLI updates were disabled to keep those PR build artifacts fixed throughout validation.

The current head, bcfddcc, adds only an explanatory comment after the browser validation; the functional code is identical to the tested commit.

Environment

  • Windows 11 Home, build 10.0.26200, x64.
  • Microsoft Edge 154.0.4258.37, using a visible window and an isolated test profile.
  • BrowserSkill CLI and daemon compiled from this PR; the extension built from the same commit and loaded unpacked into Edge.
  • Chrono Download Manager 0.13.12, using unmodified files from its official Chrome Web Store package, loaded unpacked. Both extensions therefore used unpacked extension IDs.

Validation exercised the complete PR-built CLI → PR-built daemon → PR-built extension → real Edge download flow. Download commands and browser download events were not mocked.

Reproduction

  1. Build the BrowserSkill CLI and extension from edd71a6, start the built daemon with an isolated BSK_HOME, and load the built extension into Edge.

  2. Before adding Chrono, download a 63-byte text attachment from a local test page to establish the normal baseline.

  3. Add Chrono after BrowserSkill, leaving its default download/naming settings enabled. Use a test-page link to a 1,484-byte HTTPS JSON attachment.

  4. Observe the test tab to obtain the link reference, then run the PR-built CLI:

    bsk download @e1 --out ./requested.json --session <session-id> --tab-id <tab-id> --timeout 30s --json
  5. Disable Chrono in Edge’s extension manager, observe the same tab again, and repeat the identical command with the same session, tab, and output path.

The HTTPS attachment was version.json from the repository’s release assets, used solely as download fixture data.

Observed results

Scenario Result
Normal download without Chrono Exit code 0; the requested output contained the expected 63 bytes.
Chrono overrides the directory suggestion Exit code 3, reason: download_path_mismatch, effect_state: committed, and phase: download. No requested --out file was created. Edge reported the browser download as complete, and the file remained on disk outside the per-transfer directory.
Chrono disabled; identical command repeated Exit code 0; --out contained 1,484 bytes matching the source attachment’s SHA-256. The original browser download from the conflict case also remained intact.

Additional observation: With a plain HTTP attachment, Chrono cancelled the original download and started a replacement with the same URL. This produced ambiguous attribution (download_capture_failed, effect_state: unknown) before final-path validation. Compatibility with that separate takeover behavior remains outside this PR.

This PR improves conflict detection and recovery guidance. Full compatibility with competing download managers and existing cleanup edge cases remain separate follow-up work.

@iuyo5678

Copy link
Copy Markdown
Collaborator

Thanks for the PR. The approach looks sound: detecting the final-path mismatch and providing actionable recovery guidance is a useful improvement.

Before merging, could you please address the following?

  1. Fix the formatting of the new parameterized test so the relevant Biome check passes.

  2. Validate with both BrowserSkill and Chrono installed in a real Windows Edge environment. Please cover:

    • With Chrono overriding BrowserSkill’s directory suggestion, confirm that the command reports download_path_mismatch with effect_state: committed, does not produce the requested --out file, and leaves the completed browser download intact.
    • After disabling or pausing Chrono’s filename/download takeover, confirm that the same command successfully produces --out.
    • Confirm that a normal download without the conflict still works.

Please add the browser and extension versions, reproduction steps, and observed results to the PR description.

The scope should remain clear: this PR improves conflict detection and recovery guidance; full compatibility with competing download managers can be addressed separately. The existing cleanup edge cases can also be tracked independently.

Once the formatting is fixed and the real-browser validation confirms these behaviors, I’d be comfortable moving this PR toward merge.

@lymerin

lymerin commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the PR. The approach looks sound: detecting the final-path mismatch and providing actionable recovery guidance is a useful improvement.

Before merging, could you please address the following?

  1. Fix the formatting of the new parameterized test so the relevant Biome check passes.

  2. Validate with both BrowserSkill and Chrono installed in a real Windows Edge environment. Please cover:

    • With Chrono overriding BrowserSkill’s directory suggestion, confirm that the command reports download_path_mismatch with effect_state: committed, does not produce the requested --out file, and leaves the completed browser download intact.
    • After disabling or pausing Chrono’s filename/download takeover, confirm that the same command successfully produces --out.
    • Confirm that a normal download without the conflict still works.

Please add the browser and extension versions, reproduction steps, and observed results to the PR description.

The scope should remain clear: this PR improves conflict detection and recovery guidance; full compatibility with competing download managers can be addressed separately. The existing cleanup edge cases can also be tracked independently.

Once the formatting is fixed and the real-browser validation confirms these behaviors, I’d be comfortable moving this PR toward merge.

Thanks for the review. Both requested items are now complete:

  1. Fixed the parameterized test formatting in edd71a6. Biome passes in the latest CI run.
  2. Completed real Windows Edge validation with BrowserSkill and Chrono:
    • With Chrono overriding the directory suggestion, the command returned download_path_mismatch with effect_state: committed, created no requested --out file, and preserved the completed browser download.
    • After disabling Chrono, the identical command successfully produced --out with content matching the source.
    • A normal download without Chrono also succeeded.

Environment: Windows 11 build 10.0.26200, Edge 154.0.4258.37, BrowserSkill 0.3.1 built from edd71a6, and Chrono 0.13.12. Both extensions were loaded unpacked; Chrono’s files were unchanged from the official package.

[The latest CI run](https://github.com/Tencent/BrowserSkill/actions/runs/36722168107) passes all checks, including both Windows installer test variants. The earlier pipe error did not recur.

I also observed Chrono cancelling and restarting plain HTTP downloads, causing ambiguous attribution. That separate compatibility behavior remains outside this PR, consistent with the requested scope.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[extension] Filename suggestion silently loses to other download-manager extensions (e.g. Chrono); per-trace folder is dropped

2 participants