QA Review - Dedicated Download Service for Critical Run Artifacts
Metadata
- Package:
docs/work-packages/20260619_dedicated_download_service/ - Reviewer: Codex QA review
- Date: 2026-06-19
- Scope reviewed:
wepppy/microservices/download/app.pywepppy/microservices/download/README.mdtests/microservices/test_dedicated_download_service.pytests/docker/unit/test_download_service_routing.pydocker/caddy/Caddyfiledocker/caddy/Caddyfile.wepp1docker/docker-compose.dev.ymldocker/docker-compose.prod.ymldocker/docker-compose.prod.wepp1.yml
- Related artifacts:
- Security review:
docs/work-packages/20260619_dedicated_download_service/artifacts/20260619_security_review.md - ExecPlan:
docs/work-packages/20260619_dedicated_download_service/prompts/active/dedicated_download_service_execplan.md
- Security review:
Verdict
- Gate status: pass for local implementation
- Release recommendation: ship with conditions. Production completion still requires wepp1 service/Caddy rollout and live production archive smoke/log evidence.
- Unresolved findings:
- High: 0
- Medium: 0
- Low: 0
Findings and Disposition
| ID | Severity | Area | Finding | Evidence | Disposition | Status |
|---|---|---|---|---|---|---|
| QA-01 | Medium | Streaming implementation | Archive response streaming used synchronous file reads inside the ASGI response loop, which could block other requests in the same worker during slow NFS reads. | Initial wepppy/microservices/download/app.py iterated a synchronous file generator from an async response body. |
Resolved by moving file open, seek, read, and close operations through asyncio.to_thread(...) and changing _stream_file_range to an async iterator. |
Resolved |
| QA-02 | Low | Caddy route parity | Caddy route matching only captured lowercase .zip, while the service accepted uppercase ZIP extensions. Uppercase archives would fall through to the legacy browse download path and lose dedicated-service observability. |
Initial archive_download_proxy regex ended in \.zip$; service used archive_subpath.casefold().endswith(".zip"). |
Resolved by changing both Caddyfiles to [Zz][Ii][Pp] and adding routing coverage for /archives/run.ZIP. |
Resolved |
Coverage Review
- Full archive
GETreturns expected bytes and attachment headers. HEADreturns archive metadata without body.- Single closed, open-ended, and suffix byte ranges return
206withContent-Range. - Unsatisfiable range returns
416withContent-Range: bytes */<size>. - Public-run anonymous archive access remains allowed.
- Private-run navigation requests redirect for cookie minting while API-style requests return
401. - Bearer token access remains accepted for private archive access.
- Traversal, hidden, repeated-separator, and non-ZIP paths are rejected or not found.
- Structured logs include route family, request id, run id, config, path category, basename, size, method, status, range start/end, bytes, duration, outcome, client IP, and user agent while excluding Authorization and absolute run-root paths.
- Caddy route tests prove exact archive ZIP traffic matches before the broader browse matcher and non-archive downloads remain on browse.
- Compose config validation proves dev, prod, and prod+wepp1 service definitions parse.
Validation Evidence
wctl run-pytest tests/microservices/test_dedicated_download_service.py tests/docker/unit/test_download_service_routing.py -q- 19 passed after QA fixes.PYTHONPATH=/home/workdir/wepppy .venv/bin/pytest tests/microservices/test_dedicated_download_service.py tests/microservices/test_download.py tests/microservices/test_browse_auth_routes.py tests/microservices/test_browse_security.py tests/microservices/test_browse_routes.py tests/docker/unit/test_download_service_routing.py -q- 140 passed after QA/security dispositions.wctl run-pytest tests/microservices/test_dedicated_download_service.py tests/microservices/test_download.py tests/microservices/test_browse_auth_routes.py tests/microservices/test_browse_security.py tests/microservices/test_browse_routes.py tests/docker/unit/test_download_service_routing.py -q- 140 passed after the local stack was restarted.docker run --rm -v /workdir/wepppy/docker/caddy/Caddyfile:/etc/caddy/Caddyfile:ro caddy:2-alpine caddy validate --config /etc/caddy/Caddyfile- valid configuration with preexisting formatting/header warnings.docker run --rm -v /workdir/wepppy/docker/caddy/Caddyfile.wepp1:/etc/caddy/Caddyfile:ro caddy:2-alpine caddy validate --config /etc/caddy/Caddyfile- valid configuration with preexisting formatting/header warnings.- Local Caddy smoke against
https://wc.bearhive.duckdns.org/weppcloud/runs/honeyed-marathoner/disturbed9002/download/archives/mdobre-conceivable-hickory.20260518T012715Z.zip:HEADreturned200,Accept-Ranges: bytes,Content-Length: 2516876934,Server: uvicorn,Via: 1.1 Caddy, and request id43dc51062fa54ab3a3a61bc0bc3836ec.- Full
GETreturned200and downloaded2516876934bytes in12.207687seconds at reported curl speed206171483bytes/s. Range: bytes=0-1048575returned206,Content-Range: bytes 0-1048575/2516876934, and downloaded1048576bytes.- Sparse curl resume from byte
2515828358returned206,Content-Range: bytes 2515828358-2516876933/2516876934, and completed by downloading only the final1048576bytes. docker compose --env-file docker/.env -f docker/docker-compose.dev.yml logs --tail=80 downloadshowed matchingdownload.completeevents for theHEAD, fullGET, and both206probes with bytes, duration, request id, route family, range start/end, and sanitized basename.
Residual QA Risk
- Live browser resume behavior and production throughput cannot be closed from local tests alone. This remains a production rollout validation item, not a local implementation blocker.
- NFS remains a shared storage dependency. The dedicated service improves isolation and observability but does not remove NFS as a possible bottleneck.
Sign-off
- QA reviewer: Codex QA review, 2026-06-19