fix: stop downloads when destination is full (#116)

[shard872]

Description

Fixes #116.

A destination filesystem running out of space is now treated as a run-level fatal condition instead of an ordinary per-file failure. The first ENOSPC event publishes a shared fatal signal, stops new queue intake and retries, and lets active transfers unwind cooperatively at their next stream/write boundary.

The triggering and interrupted transfers keep their .tmp files. Only the triggering URL is added to the failed list; peers interrupted by the shared fatal state are classified as aborted. Both the engine and CLI await their active work, attempt telemetry/report persistence, contain secondary persistence failures, and finish with disk-full-specific messaging.

Cooperative unwinding was chosen over immediate task cancellation because active file writes run in executor threads and cannot be safely cancelled mid-write.

Root cause

The downloader's catch-all per-file error path classified OSError(errno.ENOSPC) like a normal transfer error, allowing the queue and retry machinery to continue consuming bandwidth for data that could not be persisted.

Type of change

• Bug fix
• New feature
• Breaking change
• Documentation update

Testing

• pytest tests/test_disk_full.py -q — 13 passed (run twice before rebase; passed again after integration)
• pytest tests/ -q — 42 passed
• ruff check moon_download.py moon_engine.py moon_cli.py tests/test_disk_full.py tests/test_exit_cleanup.py — passed
• git diff --check — passed

The network-free regressions cover partial and short writes, exact remaining-byte reporting, active-peer exception races, retry/backoff suppression, retry-list and telemetry failures, terminal engine state, both frontends, and integration with #149's in-flight stop handling.

Disclosure

Implementation and review were assisted by OpenAI Codex using GPT-5.6 Terra and GPT-5.6 Sol.

[LeyckerS]

Read the diff against #116's acceptance criteria rather than the description, and all five hold:

• errno-based detection — moon_download.py, except OSError as e: if e.errno == errno.ENOSPC:. Not a string match on the message, which is what the issue asked for.
• The run aborts — RunFatalControl holds the first DiskFullError under a lock and both _do_dl and _browser_worker check fatal_control.is_set() before taking new work. Keeping this separate from the stall-kill state was the right call; overloading kill_evt would have made a full disk look like a stall and re-extract.
• The log names the folder — Disk full in {folder}: need {n:,} bytes to continue, plus the run-level summary line.
• .tmp preserved — confirmed by os.path.getsize(tmp) being read back to compute the shortfall; nothing unlinks it on this path.
• Regression test — thirteen of them.

Three things I want to call out because they are better than what the issue asked for:

_do_dl looks like a 60-line deletion in the diff, but it is the existing body re-indented into try: so that self._inc("_dls",-1) can move to a finally:. It was previously repeated on three exit paths — and this PR adds a fourth, which is exactly the situation where that pattern leaks the counter. Good catch.

The memoryview write loop is not padding either. write() can return a short count on a nearly-full disk instead of raising, and without the loop that silently truncates the file. test_short_write_then_enospc_reports_actual_remaining_bytes is the test that earns it.

Mapping "aborted" to "queue" in _FILE_UI_STATE rather than to "fail" is the detail I would have argued for. #116's actual complaint was that a full disk got reported as an ordinary per-file failure; marking each file fail would have reproduced the bug in the UI while fixing it in the engine.

One sequencing note, not a change request: #149 rewrites the same moon_engine.py region (lines ~175–200) for #65. Merging this first means that PR needs a rebase — that is on me to coordinate, not on you.

Merging.

[LeyckerS]

@shard872 — this is approved and ready to go on my side: CI is green on all eight checks, mergeable is CLEAN, and I have checked the diff against #116's criteria one by one (details in the review above).

The only thing stopping the merge is that the PR is still marked draft, which I cannot flip for you. The body reads as finished work, so I assume it was an oversight — click "Ready for review" and I will squash it straight away.

If it is not an oversight and something is still outstanding, say so and I will hold. No rush either way.

[LeyckerS]

Correcting myself: in the review above I said that merging this first would put the rebase on #149. It went the other way. This PR is still in draft and I cannot flip that flag, #149 was complete and CI-green, so I merged #149 and this one now conflicts with main (CONFLICTING / DIRTY). The rebase landed on you because of my sequencing, not because of anything in your work — sorry about that.

Here is exactly what moved under you, so you are not reverse-engineering it from a conflict marker.

All of it is in _do_dl and stop(). #149 fixed #65, where Stop waited for in-flight transfers to finish:

• Engine.__init__ gained self._active_kill_events = set(), cleared again in start().
• In _do_dl, kill_evt is registered in that set under self._lock, and the await download_file(...) call is wrapped in try: / finally: … discard(kill_evt). This is an inner try: around the single await, not around the whole body — your outer try: / finally: self._inc("_dls",-1) goes around it and the two nest cleanly.
• A new branch was inserted into the chain:

elif msg == "stall_killed" and self._get("_stop_flag"):
    rec.status = "stopped"

It sits above the generic elif msg == "stall_killed":. Please keep that order when you resolve — reversed, a user stop is treated as a stall kill and re-queued, which is the bug #149 exists to fix. Your elif msg == "aborted_disk_full": can go directly after it; the two conditions are disjoint.

• stop() now signals those events with loop.call_soon_threadsafe(kill_evt.set).

One thing that should make your diff smaller: I have already added "stopped": "queue" to _FILE_UI_STATE (moon_engine.py:471), so your "aborted": "queue" line now slots in beside an existing precedent instead of being the only non-obvious entry.

Nothing about the approval changes — the review stands, it is still a yes from me. When you have rebased, mark it ready and I will squash it.

[LeyckerS]

Rebase checked — the branch order in _do_dl survived it intact:

if ok: ...
elif msg == "stall_killed" and self._get("_stop_flag"):   # → "stopped"
elif msg == "aborted_disk_full":                          # → "aborted"
elif msg == "stall_killed":                               # → re-queue

aborted_disk_full sitting between the two stall_killed arms is the right place for it: the conditions are disjoint, and the user-stop arm stays ahead of the generic one, so neither a stop nor a full disk can be re-queued by the retry path.

All eight checks green. Merging — thanks for the rebase, and for the patch.