Skip to content

fix(windows): keep output folders out of yt-dlp templates - #105

Open
anhtahaylove wants to merge 2 commits into
vanloctech:mainfrom
anhtahaylove:pr/long-output-folder-history
Open

fix(windows): keep output folders out of yt-dlp templates#105
anhtahaylove wants to merge 2 commits into
vanloctech:mainfrom
anhtahaylove:pr/long-output-folder-history

Conversation

@anhtahaylove

@anhtahaylove anhtahaylove commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep yt-dlp output templates filename-only and pass the download directory through --paths home:
  • apply the same output-directory handling to metadata sidecar downloads
  • preserve filename metadata prefixes, queue/playlist numbering, chapter numbering, and --output-na-placeholder unknown
  • recover output paths only from the current yt-dlp invocation: a unique --print-to-file after_move:filepath capture plus paths emitted by that same child process
  • canonicalize recovered paths and reject missing, unsupported, or outside-output-directory files
  • preserve multiple output paths for split chapters and plugin workflows

Tests

  • bun run biome check --write .
  • bun run tsc -b
  • bun test — 101 passed
  • cargo test playlist_chapter_tests --lib — 18 passed
  • cargo check
  • cargo fmt --all -- --check
  • git diff --check

anhtahaylove added a commit to anhtahaylove/youwee that referenced this pull request Jul 9, 2026
Summary: Refresh tester release notes so the public installer, SHA256, updater path, and smoke-test checklist match 0.19.1-custom.12.

Constraint: Keep this as documentation only; no runtime or release asset changes are included.

Rejected: Did not dọn PR vanloctech#105 worktree because the upstream PR is still open.

Confidence: High; notes match the published youwee-releases assets and updater test already performed.

Scope-risk: Low; this touches only RELEASE_TESTER_NOTES.md.

Directive: Leave PR vanloctech#105 and v0.20.1 monitoring to the existing heartbeat automation.

Tested: bun run biome check --write .

Tested: bun run tsc -b

Tested: cargo check
@vanloctech

Copy link
Copy Markdown
Owner

Thanks for the PR. The --paths home: change makes sense and should help keep yt-dlp output templates filename-only on Windows.
One concern before merging: the new fallback that picks the newest media file in the output directory is not scoped to the current download job. If multiple downloads run in the same folder, or another process/plugin creates a media file around the same time, Youwee could attach the wrong filepath to Library history and download.completed workflows.
Could you make that fallback deterministic? For example, snapshot the output directory before starting yt-dlp, then after success only consider files that are new or modified after this job started. For split chapters, it should also preserve multiple output files rather than reducing everything to a single newest file.
The rest of the approach looks good to me; I mainly want to avoid recording or post-processing the wrong file in concurrent/download-heavy workflows.

@anhtahaylove
anhtahaylove force-pushed the pr/long-output-folder-history branch from 78e327e to 0049b76 Compare July 30, 2026 04:11
@anhtahaylove

Copy link
Copy Markdown
Contributor Author

Updated and force-pushed after rebasing onto the current upstream/main.

What changed:

  • snapshots the output directory immediately before each yt-dlp process;
  • fallback only accepts media files created or modified after that snapshot;
  • keeps every recovered filepath for split chapter outputs while still selecting the newest changed file as the primary history filepath;
  • ignores unchanged pre-existing media and non-media files.

Windows validation:

  • bun run biome check --write .
  • bun run tsc -b
  • cargo test output_fallback_only_returns_files_changed_by_this_job --lib
  • cargo test output_filepaths_merge_printed_and_recovered_chapters --lib
  • cargo check
  • rustfmt --edition 2021 --check src/commands/download.rs
  • git diff --check

All checks passed. The updated head is 0049b76.

anhtahaylove added a commit to anhtahaylove/youwee that referenced this pull request Jul 30, 2026
Summary: Prepare Youwee 0.20.0-custom.52 with authenticated Facebook Story source resolution and deterministic download output recovery.

Constraint: Keep the Firefox AMO extension version independent at 0.19.1.37 and preserve the existing custom updater, Windows Full Installer dependency pack, and open upstream PR vanloctech#105 worktree.

Rejected: Do not add another Facebook metadata request, scan unrelated output files, alter browser extension versioning, or clean PR vanloctech#105 before merge.

Confidence: High; all application manifests and changelogs are synchronized, frontend and Rust suites pass, and extension packages build successfully.

Scope-risk: Low; behavior changes are already isolated in the two preceding commits and this commit only publishes their version metadata.

Directive: Publish signed installers and updater assets from v0.20.0-custom.52.

Tested: bun run biome check --write .; bun run tsc -b; bun test (127); cargo check; cargo test --lib (331); cargo test facebook_story --lib; cargo test output_fallback_only_returns_files_changed_by_this_job --lib; cargo test output_filepaths_merge_main_and_split_chapters --lib; bun run ext:package; git diff --check.
@vanloctech

Copy link
Copy Markdown
Owner

Thanks for the update. The --paths home: direction makes sense and should help keep yt-dlp templates filename-only on Windows.

I still can’t merge this yet for two reasons:

  1. The PR is currently conflicting with main. This area has changed since the PR was opened, especially around filename metadata in build_output_template(). Please rebase and make sure the new --paths home: approach preserves filename metadata prefixes, queue/playlist numbering, chapter numbering, and --output-na-placeholder.

  2. The fallback based on “newest changed media file” is better than before, but it is still not fully scoped to the current download job. It excludes unchanged pre-existing files, but another download/process writing to the same folder after the snapshot could still be picked up and recorded as the Library filepath or plugin output. Please make the recovery path more deterministic, or add stronger safeguards/tests around concurrent/unrelated file changes.

The core idea is good, but I’d like those points handled before merge.

@anhtahaylove
anhtahaylove force-pushed the pr/long-output-folder-history branch from 0049b76 to 57d9f97 Compare August 2, 2026 04:20

Copy link
Copy Markdown
Contributor Author

Thanks — I rebased onto current main (e104390) and force-pushed 57d9f97, addressing both points.

  • --paths home: remains separate from the filename-only -o template.
  • Regression coverage verifies filename metadata prefixes, queue/playlist numbering, chapter numbering, and --output-na-placeholder unknown.
  • The directory “newest changed media file” scan has been removed entirely.
  • Recovery now only accepts paths tied to this invocation: its unique --print-to-file after_move:filepath file plus paths emitted by the same yt-dlp child process.
  • Recovered paths are canonicalized, must exist, must be supported media files, and must remain inside the configured output directory.
  • The concurrency regression test creates an unrelated media file in the same directory and an outside-root file; both are ignored while the current job’s main/chapter outputs are retained.

Validation:

  • bun run biome check --write . — passed
  • bun run tsc -b — passed
  • bun test — 101 passed
  • cargo test playlist_chapter_tests --lib — 18 passed
  • cargo check — passed
  • cargo fmt --all -- --check — passed
  • git diff --check — passed

I also ran cargo test --lib on Windows: 222 tests passed; the only 2 failures are existing plugin security tests that assert Unix /tmp paths and are outside this PR’s diff.

anhtahaylove added a commit to anhtahaylove/youwee that referenced this pull request Aug 2, 2026
Summary: Prepare Youwee 0.20.0-custom.53 with the shared createClientId compatibility fallback for WebViews without crypto.randomUUID.

Constraint: Keep the Firefox AMO extension version independent at 0.19.1.37 and preserve the existing custom updater, Windows Full Installer dependency pack, and open upstream PR vanloctech#105 branch.

Rejected: Do not bump the browser extension version, alter updater channels, pull unrelated upstream changes, or add another client ID abstraction.

Confidence: High; the targeted client ID regression tests, full frontend suite, full Rust library suite, static checks, and extension packaging all pass.

Scope-risk: Low; runtime behavior is isolated in commit 326ceb7 and this commit only publishes synchronized version metadata and changelogs.

Directive: Publish signed installers, updater metadata, and extension assets from v0.20.0-custom.53.

Tested: bun run biome check --write .; bun run tsc -b; bun test (129); cargo check; cargo test --lib (331); bun run ext:package; git diff --check.
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.

2 participants