Stop in-flight downloads when stopping engine

[Allen58562]

Fixes #65

Summary

• track active download kill events in Engine
• set those events when Engine.stop() is requested so in-flight downloads abort promptly
• treat user-stopped transfers as stopped, not failed or stall-killed
• add a regression test that verifies stop completes quickly, preserves partial .tmp files, and does not write failed_links.txt

I chose a registry of per-download kill events instead of a single shared event so the existing stall-kill mechanism stays separate from a user-initiated stop.

Testing

• pytest tests/test_exit_cleanup.py -q
• pytest tests/ -q

On Windows I set TEMP and TMP to a writable workspace directory before running pytest.

[LeyckerS]

Welcome, and this is a good first pull request — the design is right and you made the two judgement calls I would have made. One display gap, then it merges.

What is right

The registry rather than a shared event, and for the reason you gave. A single event would have conflated a user-initiated stop with a stall kill, and kills_so_far feeds the re-extract logic — a Stop would have started counting against the stall budget. Keeping them separate is the correct read of what those two mechanisms mean.

loop.call_soon_threadsafe(kill_evt.set). This is the part that would have been a subtle race in most first attempts. stop() runs on the HTTP handler thread; asyncio.Event.set() is not thread-safe and calling it directly from there is undefined. Scheduling it on the engine's loop is right, and you did it without being told.

Registering in _do_dl and discarding in a finally, so the set cannot leak entries when a download raises. And clearing it in start() so a second run does not inherit stale events.

The test is the strongest part. It asserts what the issue actually cares about: stop returns quickly, fail == 0, kills == 0, and the partial .tmp survives. That last one is the whole point — an abort that deleted the partial would technically stop the download and destroy the resume it exists to protect.

Blocking: "stopped" is not a state the interface knows

rec.status = "stopped" has no entry in _FILE_UI_STATE (moon_engine.py:460-466), and _files_payload falls back on the default:

state = self._FILE_UI_STATE.get(rec.status, "queue")   # line 607

So every file you stopped renders in the GUI as queued — waiting to start. Press Stop, watch the transfers you just interrupted relabel themselves as pending, and you cannot tell whether the stop worked. They also sort with the pending rows, because RANK.queue is 3 in web/app.js, so they jump back up above the finished ones.

It needs a state of its own. Five small additions:

• moon_engine.py:460 — "stopped": "stopped", in _FILE_UI_STATE
• web/app.js:517 — stopped: "st_stopped", in STATE_KEY
• web/app.js — st_stopped in both dictionaries; the English one sits beside st_ok/st_fail/st_kill around line 53 and the Italian around line 106. Something like "stopped" / "interrotto"
• web/app.js:442 — a RANK entry so stopped rows sort with the finished tail rather than the queue
• web/styles.css — optional, but .frow[data-state="stopped"] with a neutral or amber accent would distinguish it; .chip.warn at line 304 shows the amber already in use

One smaller thing

The test deletes failed_links.txt from the repository directory:

failed_links = pathlib.Path(moon_engine.__file__).with_name("failed_links.txt")
failed_links.unlink(missing_ok=True)

If a developer has just done a real run, that file holds their failed links and the test removes it. The assertion is worth keeping — checking a stop does not write failures is exactly right — but point it somewhere it cannot destroy real output, or assert on its modification time rather than deleting it first.

Why this is one round and not a rejection

The hard part of #65 was aborting mid-chunk without corrupting the partial, and that was already written — the issue was that nothing connected it to the button. You connected it, correctly, thread-safely, with the semantics kept separate. What is left is telling the user what happened, which is five lines across two files.

This is the issue I have called the most valuable one open for five days. Good to see it moving.

[LeyckerS]

Checked the three claims in the description against the diff; all three hold, and the hard part is right.

Engine.stop() runs on the caller's thread while the events live on the engine's loop, and asyncio.Event.set() is not thread-safe — loop.call_soon_threadsafe(kill_evt.set) is the correct way to bridge that, and guarding it with loop.is_running() covers the race where stop arrives after the loop has gone. The registry itself is read under self._lock into a local list before the loop is touched, so it cannot mutate mid-iteration.

The try:/finally: discard around download_file is what makes the registry safe over a long run — without it the set would grow for the lifetime of the process. Easy thing to leave out; you didn't.

Ordering the elif msg == "stall_killed" and self._get("_stop_flag") branch above the generic stall_killed branch is also load-bearing. Reversed, a user-initiated stop would be counted as a stall kill and re-queued, which is the bug the PR exists to fix.

Keeping the registry separate from the stall-kill path rather than reusing one shared event was the right call, for the same reason #150 kept its disk-full state separate: a stop must not look like a stall, or the retry machinery undoes it.

One observation, not blocking: "stopped" is not in _FILE_UI_STATE (moon_engine.py:460), so it falls through .get(rec.status, "queue") and renders as queued. That is the same end state #150 chose deliberately for "aborted", so the behaviour is right — but it is implicit here and explicit there. I will add the one-line entry myself so the table lists every status a record can hold; that is bookkeeping on my side, not a change to your work.

Merging this first: it was opened first, it is complete, and it is the smaller of the two touching this region.