feat(cli): structured exit codes for moon_cli.py

[AdvaitVarhade]

Fixes #32 by implementing structured, predictable exit codes in moon_cli.py to allow automated callers to reliably distinguish success, partial failure, complete failure, and pre-flight configuration errors.

Changes

• Exit Code 0: Every file in the batch completed successfully (fail_count == 0 and ok_count > 0).
• Exit Code 1: Run completed but some URLs failed (fail_count > 0), run was interrupted (KeyboardInterrupt), or an unhandled crash occurred.
• Exit Code 2: Pre-flight error where execution could not start (missing/empty --urls file).
• Exit Code 3: Total failure where every attempted URL failed (fail_count > 0 and ok_count == 0).
• Documented these codes in docs/CLI.md.
• Added automated coverage for all exit codes in tests/test_cli_exit_codes.py.

[LeyckerS]

@AdvaitVarhade — the exit-code work itself is right, including the detail that is easy to get wrong: total failure is tested before partial failure, so code 3 is reachable.

if ok == 0 and fail > 0:
    sys.exit(3)
elif fail > 0:
    sys.exit(1)
else:
    sys.exit(0)

Reversed, exit(3) would be dead code. Documenting the scheme in docs/CLI.md and covering each branch in tests/test_cli_exit_codes.py is exactly what #32 asked for.

Two things before this can go in.

1. moon_extract.py is outside the scope of #32. The narrowing in _extract_datanodes_on_context is #46 / #130 work — good work, and it belongs in a pull request that says so. You know this rule better than most; you have been on the right side of it several times. The changelog quotes these descriptions, and "structured exit codes for moon_cli.py" cannot honestly cover an exception-handling change in the extractor. Please take it out and open it separately — I will review it on its own and it will not be a hard sell.

2. While it is out, this is worth knowing. The new handler is:

except (PlaywrightError, PlaywrightTimeoutError):

Those are the same set. Verified rather than assumed:

>>> from playwright.async_api import Error as E, TimeoutError as T
>>> issubclass(T, E)
True
>>> [c.__name__ for c in T.__mro__]
['TimeoutError', 'Error', 'Exception', 'BaseException', 'object']

PlaywrightTimeoutError subclasses PlaywrightError, so the tuple catches precisely what except PlaywrightError: catches. The behaviour is correct — timeouts are caught, which is what you want on a wait_for_load_state(..., timeout=25000) — but it reads as though two distinct failures are handled, and a future reader may "simplify" it by removing the wrong half. This is the same shape as the note on #130; naming only PlaywrightError and letting the comment say that timeouts are included is clearer. Not blocking, and not blocking in the follow-up PR either — just do not lose the comment.

Also: this is currently conflicting with main. #150 landed since you opened it and rewrote a large part of moon_cli.py, including the try/except block at the bottom that your exit codes attach to. The run function's return is part of that region, so the rebase is not purely mechanical — check that ok/fail still mean what you expect after the disk-full changes, in particular that a run aborted by ENOSPC produces the exit code you want. My reading is that it should be 1 rather than 3 even when nothing succeeded, because the run did not fail per-URL — but that is a judgement call and I would rather you make it deliberately and write it into docs/CLI.md than inherit it by accident.

#154 also touches moon_cli.py. I will sequence the two of you; no coordination needed on your side.

[LeyckerS]

moon_extract.py is out — thank you. What is left is exactly #32 and nothing else.

Checked the rebase rather than assuming it was mechanical, since #150 rewrote the region this attaches to:

The aborted branch is the right call and it is first.

if aborted:      sys.exit(1)
elif ok == 0 and fail > 0: sys.exit(3)
elif fail > 0:   sys.exit(1)
else:            sys.exit(0)

A run stopped by a full disk reports 1, not 3, even when nothing succeeded — because the URLs did not individually fail, the run was cut short. That is the judgement I would have made and you have written it into docs/CLI.md, which is what I care about more than the number itself.

run() still has one exit point. I checked this specifically, because a three-value unpack in main() would raise TypeError if any path returned None. The two returns inside the function body are both at twelve-space indentation inside the nested do_dl coroutine; run() itself falls through to your new return ok_count, fail_count, disk_full is not None. Safe.

Verified end to end on your branch rather than from the tests alone:

missing urls file  -> 2
empty urls file    -> 2
argparse error     -> 2

That last one is worth noting in your favour: argparse already exits 2 on a usage error, so choosing 2 for pre-flight problems makes the CLI consistent with its own parser instead of inventing a third convention. pytest tests/ -q → 50 passed.

One behaviour change that is correct but worth flagging in the changelog rather than burying: the setup errors moved from 1 to 2. Any script that currently tests if [ $? -ne 0 ] is unaffected, but one testing -eq 1 specifically would change meaning. It is the right move — that is the whole point of the issue — and I will call it out in the release notes.

Merging.