From 275b622243b809bd267eaf260b7cc2b3490ce02e Mon Sep 17 00:00:00 2001 From: Domagoj Zecevic Date: Mon, 7 Sep 2026 15:06:52 +0200 Subject: [PATCH] feat(downloads): show live download progress --- .ai/HANDOFF.md | 39 +++++++ .ai/REVIEW.md | 43 +++++++ .ai/TASKS.md | 2 +- README.md | 4 +- app.py | 308 +++++++++++++++++++++++++++++++++++++++++++------ 5 files changed, 356 insertions(+), 40 deletions(-) diff --git a/.ai/HANDOFF.md b/.ai/HANDOFF.md index 5e22246..38912cf 100644 --- a/.ai/HANDOFF.md +++ b/.ai/HANDOFF.md @@ -140,3 +140,42 @@ Append-only role handoff log. Each role adds one entry when its step is complete | Next Role | none | --- + +### T-004 — implement — 2026-09-07T12:56:52Z + +| Field | Value | +|-------|-------| +| Agent | codex | +| Summary | Added in-memory background download jobs, live polling progress, automatic file delivery, and cleanup. | +| Files Changed | app.py, README.md, .ai/TASKS.md, .ai/HANDOFF.md | +| Validation | `python3 -m py_compile app.py` passed (`python` is unavailable); mocked container job lifecycle check for progress, delivery cleanup, and error state passed; `docker build -t yt-dl .`, container progress UI smoke test, and `git diff --check` passed | +| Commit | Pending reviewer approval | +| Next Role | review | + +--- + +### T-004 — review — 2026-09-07T13:10:00Z + +| Field | Value | +|-------|-------| +| Agent | claude | +| Summary | Reviewed the job-based download/progress/delivery split against `.ai/PLAN.md`; re-ran `py_compile` and drove the full job lifecycle live through the built Docker image against real YouTube URLs (progress polling, auto-delivered file, 404/409 semantics, server-side error path with no crash) — all passed with 3 non-required minor/nit notes. | +| Files Changed | .ai/REVIEW.md, .ai/TASKS.md, .ai/HANDOFF.md | +| Verdict | PASS_WITH_NOTES | +| Blocking Findings | none | +| Next Role | implement | + +--- + +### T-004 — implement — 2026-09-07T13:06:07Z + +| Field | Value | +|-------|-------| +| Agent | codex | +| Summary | Squashed the reviewed background-job download progress experience into the task commit. | +| Files Changed | app.py, README.md, .ai/REVIEW.md, .ai/TASKS.md, .ai/HANDOFF.md | +| Validation | Reviewer verification passed; `git diff --check` passed | +| Commit | Created by this handoff's task commit | +| Next Role | none | + +--- diff --git a/.ai/REVIEW.md b/.ai/REVIEW.md index ad4043f..26defdb 100644 --- a/.ai/REVIEW.md +++ b/.ai/REVIEW.md @@ -108,3 +108,46 @@ No blocker, major, minor, or nit findings. **Risks:** - None outstanding. `sanitize_filename` strips characters outside `\w`/whitespace/`- _ . ( ) , '`; this only affects the `Content-Disposition` header value used for the browser's save-as name, not the on-disk path used to read the file, so there is no path-traversal or injection concern from unsanitized titles. + +--- + +## T-004 — Job-based `POST /download` + `GET /progress/{job_id}` + `GET /download/{job_id}/file`, in-memory job store, yt-dlp `progress_hooks`, frontend polling + progress bar UI + +**Verdict:** PASS_WITH_NOTES + +### Findings + +1. **[minor]** `app.py:296-315` (`progress_hook`) — `job.percent` is only updated inside `if job.total_bytes:`, so when yt-dlp can't report a `total_bytes`/`total_bytes_estimate` (e.g. some DASH/live formats), the progress bar stays at 0% for the whole download even though `downloaded_bytes` is advancing and shown as text. Not required: the plan explicitly allows "downloaded size only if total is unknown" as the fallback UX, and this doesn't affect the common case (verified live: percent tracked correctly when total is known). + - Required fix: No. +2. **[minor]** `app.py:296-367` — `progress_hooks` only reports raw download percent; ffmpeg post-processing (mp3 extraction, video mux) after the download hits 100% is not reflected, so the bar can sit at 100% for a few extra seconds while the file is finalized before `status` flips to `finished`. Acceptable per plan (`postprocessor_hooks` was offered as one of two options, not mandatory), and verified it does not stall indefinitely or misreport. + - Required fix: No. +3. **[nit]** `app.py:356-362` — the friendlier `"The download could not be prepared."` message set by `progress_hook` on a hook-level `status == "error"` is generally overwritten by the broader `except Exception as exc: ... job.error_message = str(exc)` handler in `run_job`, since yt-dlp raises after invoking the hook. In practice this means the browser sees yt-dlp's raw exception text (confirmed in testing: `"ERROR: [generic] not-a-video: Unable to download webpage: HTTP Error 404..."`) rather than the friendlier hook message. Same class of note as the T-001 review (raw exception text surfaced to the browser); not a regression introduced by this task and not required to fix here. + - Required fix: No. + +No blocker or major findings. + +### Verification + +**Steps performed:** +- Re-read `.ai/PLAN.md` (Phase 2 spec for T-004) and diffed it against `app.py` / `README.md`; the three-endpoint split, `JobState`/`_jobs`/`_jobs_lock`, `progress_hooks`-driven updates, `run_in_executor` fire-and-forget scheduling (`_pending_jobs` set to avoid GC warnings), stale-job TTL sweep piggybacked on `POST /download`, and the frontend polling/progress-bar/button-disable/error-handling all match the plan. +- `python3 -m py_compile app.py` — succeeded. +- Built the actual Docker image (`docker build -t yt-dl-review2 .`) and ran it, then drove the real HTTP flow end-to-end against real YouTube URLs: + - `POST /download` (playlist-context MP3 URL) → `{"job_id": ...}`; polling `GET /progress/{job_id}` every ~1s showed `status: "downloading"` with `percent`/`speed`/`downloaded_bytes`/`total_bytes` advancing correctly (0 → 61% → 100%), then `status: "finished"`. + - `GET /download/{job_id}/file` after `finished` → 200, correct MP3 bytes (`file` confirms valid MPEG audio), title-based `Content-Disposition` filename (reusing T-003's sanitization) — auto-delivered with no extra click needed on the client side. + - Job cleanup confirmed: `GET /progress/{job_id}` after the file was served → 404 `"Download job not found."`, confirming `BackgroundTask(cleanup_job, ...)` removed the job and temp dir after delivery. + - `GET /progress/` and `GET /download//file` → both 404 as specified. + - Fetched `GET /download/{job_id}/file` on a still-`downloading` job → 409 `"The download is not ready yet."`, matching the plan's pre-completion semantics. + - **Error path**: posted a non-YouTube, non-existent URL (`http://example.com/not-a-video`) → job transitioned to `status: "error"` with a descriptive `error` message, no server crash (`GET /` still returned 200 immediately after), and `docker logs` showed no traceback — matches "server-side failure shows an error in place of the bar with no crash." + - Confirmed no temp-dir leak on the error path (`docker exec ... ls /tmp` showed the errored job's temp dir already removed); a separate job's temp dir that was intentionally never fetched was still present, consistent with the documented "removed after delivery, or after a timeout for abandoned jobs" behavior (not fetching a finished job's file is expected to leave it until TTL/next sweep). + - Cleaned up: stopped the test container, removed the `yt-dl-review2`/`yt-dl-review` test images, removed local scratch files. +- Reviewed `README.md` changes: accurately documents the job/progress/file endpoint flow, automatic delivery, and non-persistent in-memory job state. + +**Findings from verification:** All acceptance criteria for T-004 hold: +- Progress bar appears on download start and updates with percent/speed/downloaded-total size (verified live with real percentages/speeds). +- File auto-delivers to the browser on completion with the correct sanitized filename (server-side `fetch` + `Content-Disposition` flow verified; same header-parsing logic as T-003, already confirmed to round-trip correctly in the browser). +- Server-side failure shows an error in place of the bar with no crash (verified: job marked `error`, server remained responsive). +- `python -m py_compile app.py` passes. + +**Risks:** +- Progress percent can remain at 0% for formats where yt-dlp cannot report a total size (noted above, minor, not required). +- The in-memory job store means an app restart mid-download loses all job state/progress for any in-flight browser sessions; this matches the explicitly stated "no persistent storage" design constraint and is now documented in `README.md`. diff --git a/.ai/TASKS.md b/.ai/TASKS.md index e5d7343..2e90808 100644 --- a/.ai/TASKS.md +++ b/.ai/TASKS.md @@ -23,4 +23,4 @@ Command expectations: | T-001 | FastAPI backend (`app.py`) + inline HTML/JS frontend (`requirements.txt`) | done | `GET /` returns 200 with HTML containing "Download MP3"; `POST /download` with valid URL + mode streams a file; invalid URL returns 400 JSON with `.message` | Reviewer re-ran mocked FastAPI route checks (GET /, valid download, invalid/empty/missing-field errors, yt-dlp failure, unicode filename) — all passed; see `.ai/REVIEW.md` | none | | T-002 | Dockerfile + README.md (build/run docs) | done | `docker build -t yt-dl .` exits 0; `docker run --rm -p 8080:8080 yt-dl` starts server; `curl http://localhost:8080/` returns HTML with download buttons | Reviewer re-ran `docker build` + container smoke test, plus a live end-to-end MP3 and video download against a real YouTube URL through the container — all passed; see `.ai/REVIEW.md` | none | | T-003 | `app.py` (`noplaylist` option, `sanitize_filename` helper, title-based `FileResponse` filename, frontend `Content-Disposition`-based `link.download` fix) + `README.md` update | done | Pasting a playlist/`list=` URL downloads only the referenced video; saved filename matches the video title with emoji/icons stripped (normal chars kept) instead of a UUID, with correct extension; `python -m py_compile app.py` passes | Reviewer re-ran `py_compile`, unit-checked `sanitize_filename`, and ran a live end-to-end playlist-link MP3 download + a plain video download through the built Docker image against real YouTube URLs — all passed; see `.ai/REVIEW.md` | none | -| T-004 | `app.py` (job-based `POST /download` + `GET /progress/{job_id}` + `GET /download/{job_id}/file`, in-memory job store, yt-dlp `progress_hooks`, frontend polling + progress bar UI) + `README.md` update | ready_for_implement | Progress bar appears below the buttons on download start and updates with percent/speed/downloaded-total size; file auto-delivers to the browser on completion with the correct sanitized filename; server-side failure shows an error in place of the bar with no crash; `python -m py_compile app.py` passes | | implement | +| T-004 | `app.py` (job-based `POST /download` + `GET /progress/{job_id}` + `GET /download/{job_id}/file`, in-memory job store, yt-dlp `progress_hooks`, frontend polling + progress bar UI) + `README.md` update | done | Progress bar appears below the buttons on download start and updates with percent/speed/downloaded-total size; file auto-delivers to the browser on completion with the correct sanitized filename; server-side failure shows an error in place of the bar with no crash; `python -m py_compile app.py` passes | Reviewer re-ran `py_compile` and drove the full job lifecycle live through the built Docker image against real YouTube URLs (progress polling, auto-delivered file, 404/409 semantics, server-side error path with no crash) — all passed; see `.ai/REVIEW.md` | none | diff --git a/README.md b/README.md index 6164ba3..3c48a75 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,9 @@ docker run --rm -p 8080:8080 yt-dl Open [http://localhost:8080](http://localhost:8080) in a browser, paste a YouTube URL, then select **Download MP3** or **Download Video**. -Downloads are prepared on the server before they are sent to your browser. Large videos and higher-quality formats can take several minutes, depending on the source video and your network connection. +Downloads run as temporary in-memory jobs, so a progress bar shows percentage, speed, and downloaded size while the server prepares the file. When it finishes, the file is delivered to your browser automatically. Large videos and higher-quality formats can take several minutes, depending on the source video and your network connection. + +The browser starts a download with `POST /download`, polls `GET /progress/{job_id}`, and retrieves the completed file from `GET /download/{job_id}/file`. Job state and temporary files are removed after delivery (or after a timeout for abandoned jobs), and are not retained across server restarts. If a pasted video link includes playlist context (such as a `list=` parameter), the app downloads only that referenced video. Downloaded files use the video title, with emoji and unsupported symbols removed while normal letters (including non-Latin characters), digits, and common punctuation are retained. An emoji-only title falls back to the video ID. diff --git a/app.py b/app.py index 2d86549..1d46e8f 100644 --- a/app.py +++ b/app.py @@ -4,8 +4,12 @@ import asyncio import re import shutil import tempfile +import threading +import time +from dataclasses import dataclass from pathlib import Path from typing import Literal +from uuid import uuid4 import yt_dlp from fastapi import FastAPI, Form, HTTPException, Request @@ -30,6 +34,9 @@ HTML = """ main { max-width: 36rem; padding: 2rem; width: 100%; } input { box-sizing: border-box; font-size: 1rem; padding: .8rem; width: 100%; } button { cursor: pointer; font-size: 1rem; margin: 1rem .5rem 0 0; padding: .75rem 1rem; } + #progress-wrap { padding-top: 1rem; } + #progress-bar { width: 100%; } + #progress-text { min-height: 1.5rem; padding-top: .35rem; } #status { color: #fca5a5; min-height: 1.5rem; padding-top: 1rem; } @@ -41,10 +48,118 @@ HTML = """ +
@@ -89,6 +200,29 @@ HTML = """ app = FastAPI() +@dataclass +class JobState: + """Transient state for a browser-requested download.""" + + status: Literal["downloading", "finished", "error"] + percent: float = 0 + speed: float | None = None + downloaded_bytes: int = 0 + total_bytes: int | None = None + temp_dir: str = "" + file_path: str | None = None + filename: str | None = None + error_message: str | None = None + mode: Literal["mp3", "video"] = "mp3" + created_at: float = 0 + + +_JOB_TTL_SECONDS = 30 * 60 +_jobs: dict[str, JobState] = {} +_jobs_lock = threading.Lock() +_pending_jobs: set[asyncio.Future[None]] = set() + + def sanitize_filename(title: str, fallback_id: str) -> str: """Return a safe, human-readable download filename stem.""" cleaned = _FILENAME_ALLOWED.sub("", title or "") @@ -101,6 +235,27 @@ def cleanup_tmpdir(path: str) -> None: shutil.rmtree(path, ignore_errors=True) +def cleanup_job(job_id: str) -> None: + """Remove a delivered job and its temporary download directory.""" + with _jobs_lock: + job = _jobs.pop(job_id, None) + if job: + cleanup_tmpdir(job.temp_dir) + + +def cleanup_stale_jobs() -> None: + """Best-effort removal of abandoned download jobs.""" + cutoff = time.monotonic() - _JOB_TTL_SECONDS + with _jobs_lock: + stale_jobs = [ + _jobs.pop(job_id) + for job_id, job in list(_jobs.items()) + if job.created_at < cutoff + ] + for job in stale_jobs: + cleanup_tmpdir(job.temp_dir) + + @app.exception_handler(HTTPException) async def http_exception_handler(_: Request, exc: HTTPException) -> JSONResponse: """Expose errors in the format expected by the browser client.""" @@ -123,14 +278,47 @@ async def home() -> str: @app.post("/download") async def download( url: str = Form(...), mode: Literal["mp3", "video"] = Form(...) -) -> FileResponse: - """Download and stream a requested audio or video file.""" +) -> JSONResponse: + """Start a download job and return its identifier immediately.""" if not url.strip() or not url.lower().startswith("http"): raise HTTPException(status_code=400, detail="Please provide a valid URL starting with http.") + cleanup_stale_jobs() + job_id = str(uuid4()) temp_dir = tempfile.mkdtemp(prefix="youtube-download-") output_template = str(Path(temp_dir) / "%(title)s.%(ext)s") - ydl_opts: dict[str, object] = {"outtmpl": output_template, "noplaylist": True} + + with _jobs_lock: + _jobs[job_id] = JobState( + status="downloading", temp_dir=temp_dir, mode=mode, created_at=time.monotonic() + ) + + def progress_hook(progress: dict[str, object]) -> None: + """Copy yt-dlp progress updates into the job visible to the browser.""" + with _jobs_lock: + job = _jobs.get(job_id) + if not job: + return + if progress.get("status") == "error": + job.status = "error" + job.error_message = "The download could not be prepared." + return + if progress.get("status") != "downloading": + return + downloaded = progress.get("downloaded_bytes") + total = progress.get("total_bytes") or progress.get("total_bytes_estimate") + speed = progress.get("speed") + job.downloaded_bytes = int(downloaded) if isinstance(downloaded, (int, float)) else 0 + job.total_bytes = int(total) if isinstance(total, (int, float)) else None + job.speed = float(speed) if isinstance(speed, (int, float)) else None + if job.total_bytes: + job.percent = min(100, job.downloaded_bytes / job.total_bytes * 100) + + ydl_opts: dict[str, object] = { + "outtmpl": output_template, + "noplaylist": True, + "progress_hooks": [progress_hook], + } if mode == "mp3": ydl_opts.update( { @@ -147,25 +335,69 @@ async def download( else: ydl_opts.update({"format": "bestvideo+bestaudio/best", "merge_output_format": "mp4"}) - def fetch_media() -> dict[str, object]: - with yt_dlp.YoutubeDL(ydl_opts) as downloader: - return downloader.extract_info(url, download=True) - - try: - info = await asyncio.get_running_loop().run_in_executor(None, fetch_media) - downloads = [path for path in Path(temp_dir).iterdir() if path.is_file()] - if len(downloads) != 1: - raise RuntimeError("The downloaded file could not be identified.") - media_path = downloads[0] - title = str(info.get("title") or "") - video_id = str(info.get("id") or "download") - display_name = sanitize_filename(title, video_id) + media_path.suffix - except Exception as exc: - cleanup_tmpdir(temp_dir) - raise HTTPException(status_code=500, detail=str(exc)) from exc + def run_job() -> None: + try: + with yt_dlp.YoutubeDL(ydl_opts) as downloader: + info = downloader.extract_info(url, download=True) + downloads = [path for path in Path(temp_dir).iterdir() if path.is_file()] + if len(downloads) != 1: + raise RuntimeError("The downloaded file could not be identified.") + media_path = downloads[0] + title = str(info.get("title") or "") + video_id = str(info.get("id") or "download") + display_name = sanitize_filename(title, video_id) + media_path.suffix + with _jobs_lock: + job = _jobs.get(job_id) + if job: + job.status = "finished" + job.percent = 100 + job.file_path = str(media_path) + job.filename = display_name + except Exception as exc: + cleanup_tmpdir(temp_dir) + with _jobs_lock: + job = _jobs.get(job_id) + if job: + job.status = "error" + job.error_message = str(exc) + + future = asyncio.get_running_loop().run_in_executor(None, run_job) + _pending_jobs.add(future) + future.add_done_callback(_pending_jobs.discard) + return JSONResponse({"job_id": job_id}) + + +@app.get("/progress/{job_id}") +async def progress(job_id: str) -> JSONResponse: + """Return the browser-safe state of an active download job.""" + with _jobs_lock: + job = _jobs.get(job_id) + if not job: + raise HTTPException(status_code=404, detail="Download job not found.") + payload = { + "status": job.status, + "percent": job.percent, + "speed": job.speed, + "downloaded_bytes": job.downloaded_bytes, + "total_bytes": job.total_bytes, + "error": job.error_message, + } + return JSONResponse(payload) + +@app.get("/download/{job_id}/file") +async def download_file(job_id: str) -> FileResponse: + """Stream a completed job's file, cleaning its state after delivery.""" + with _jobs_lock: + job = _jobs.get(job_id) + if not job: + raise HTTPException(status_code=404, detail="Download job not found.") + if job.status != "finished" or not job.file_path or not job.filename: + raise HTTPException(status_code=409, detail="The download is not ready yet.") + file_path = job.file_path + filename = job.filename return FileResponse( - media_path, - filename=display_name, - background=BackgroundTask(cleanup_tmpdir, temp_dir), + file_path, + filename=filename, + background=BackgroundTask(cleanup_job, job_id), )