fix: load fetchart and fetch artwork after retag

[Iranman]

Second correction pass — new head: de21ab1 (previously f8ccfa4, base unchanged 14c7f02). This pass fixes the six remaining blockers from the prior review: the GitHub-only CI failure (unit-tests/security both red on f8ccfa4), test isolation, timeout handling, empty-result accounting, artwork-retry truthfulness, and reconciliation identity. Still draft, still unmerged, still not merged into main.

The previous PR description's claim of "all six workflows green" at f8ccfa4 was wrong — GitHub's Linux runner failed unit-tests and security (10 errors each) even though the same suite passed locally on Windows 5+ times. That claim is retracted below.

GitHub-only CI failure: found and fixed

gh run view --log-failed on both failing workflows showed the identical failure, both failing during Python unittest discovery:

beets.dbcore.db.DBAccessError: attempt to write a readonly database. Check file permissions: the database file or its directory may not be writable.
  File ".../tests/test_post_retag_artwork_integration.py", line 100, in setUp
    self.aid = _seed_album(APP, mb_albumid=MB_ALBUMID)
  File ".../tests/test_post_retag_artwork_integration.py", line 82, in _seed_album
    album = app_module.lib.add_album([item])

10 errors, Ran 1257 tests — all in AiImportFolderSequenceTests.setUp.

Root cause, found by reproducing the exact CI command in a matching Linux/Python 3.13 container (not guessed from the traceback alone): unittest.addModuleCleanup stores its callbacks in a single process-wide list (unittest.case._module_cleanups), not one scoped per module. Under unittest discover, every test module is imported (registering its cleanups) before any test runs; whichever module's tests finish first then drains and fires every other module's still-pending cleanups too — including shutil.rmtree(<some other module's own temp root>) for modules whose tests hadn't run yet. This is confirmed with a minimal repro: importing test_artwork_retry_reconciliation then test_post_retag_artwork_integration, running only the first module's suite, and observing the second module's temp root already deleted (os.path.isdir(...) → False) before any of its own tests executed. Never reproduced on Windows locally because the specific module ordering happened not to trigger it there.

Separately, a related but distinct gap: even with directory-deletion fixed, app.py's LIB_PATH/lib/_AI_BATCH_STATE_DIR are computed once at import time from env vars. app.py is a process-wide singleton (sys.modules['app']); whichever of these two test modules imports it last wins those globals for both modules' tests, regardless of which module's tests actually run first. A one-time rebind at import time isn't enough — confirmed by deliberately loading the modules via python -m unittest test_post_retag_artwork_integration test_artwork_retry_reconciliation ... (this module first, the other module importing second and last), which reproduced a new, different failure via this PR's own path-containment safety assertion.

Fix, in both tests/test_post_retag_artwork_integration.py and tests/test_artwork_retry_reconciliation.py:

• Replaced unittest.addModuleCleanup(shutil.rmtree, ...) / addModuleCleanup(_env_patcher.stop) with atexit.register(...) — fires once at real process exit, immune to the shared-list-draining behavior above.
• Added _bind_app_globals_to_this_test_module(APP, _TMP_ROOT), which explicitly rebinds app.LIB_PATH, app.lib (a fresh beets.library.Library, closing the previous one), and app._AI_BATCH_STATE_DIR to this module's own temp root. Called once at import and again in every test's setUp() — the setUp call is what makes this correct regardless of import/run order between sibling modules, not just regardless of order relative to unrelated modules.
• Kept a _assert_path_owned_by_test() safety assertion before every raw SQL mutation in _clear_library(), verifying APP.LIB_PATH actually resolves under this module's own temp root — this is what caught the import-order gap above during validation, rather than letting it silently corrupt another module's temp directory.

Verified, not assumed: ran pip install -r requirements.txt && python -m unittest discover -s tests -p "test_*.py" (the literal unit-tests.yml/security.yml commands) inside python:3.13-slim in Docker — 13 consecutive full-discovery runs, 1271 tests each, all green except one confirmed-unrelated pre-existing flake (see below). Also re-ran the focused reproduction with modules loaded in reversed/explicit order 5x, all green. This is the same failure mode the task explicitly warned about; it's now fixed and reproduced-fixed under the real CI toolchain, not just asserted from local runs.

Pre-existing, unrelated flake found during this validation (not touched by this PR): tests/test_ai_batch_retry_race.py::SimultaneousRecoverStartsExactlyOneWorkerTests::test_concurrent_recover_requests_start_exactly_one_worker failed once in ~20 full-discovery runs (thread-timing race asserting exactly one winner among concurrent recover requests, occasionally saw two). That file was last modified in an unrelated, already-merged PR (#15) and has zero diff on this branch — flagging for separate follow-up, not fixed here (out of this PR's scope).

The five other corrections

1. Subprocess timeouts (rc=124) no longer treated as success for mbsync/write/move. _ai_import_folder's per-stage loop previously only checked for -9 (cancelled); any other non-zero code including 124 (timeout) was logged and silently ignored, continuing on to recording-ID repair and artwork fetching with unverified tag state. Now r2.returncode == 124 raises immediately, same as cancellation. (Step 1, the initial import call, already raised correctly for rc >= 2, which includes 124 — no change needed there.) Four new tests cover mbsync/write/move/import timeout at each stage, each asserting _repair_album_art is never called.

2. Empty _ai_import_folder() results no longer counted as a successful import. The AI-batch worker previously did if not import_result: import_result = {} and then still ran imported += 1 / status="imported". Every real return path from _ai_import_folder returns a populated dict, so a falsy result is itself an internal failure; it now raise RuntimeError(...), routing into the existing except branch (queued for review, import_failed) instead of a false "imported" status.

3. Artwork retry reconciliation now preserves cancelled/timed_out, not just fetched/failed. _repair_album_art now tracks whether the FetchArt subprocess itself timed out and threads that through to a distinguishable "fetchart timed out" error string when no fallback art is found either. POST /api/albums/<aid>/fetch-art's job now records a structured terminal_outcome (success/cancelled/timed_out/failed) via the job's update_state(), and is tagged with metadata={"type": "album_art_repair", "album_id": aid}.

4. Reconciliation requires an exact batch/folder/job identity — no "latest batch" fallback. POST /api/ai-batch/reconcile-artwork now requires batch_job_id, folder_id, and artwork_job_id (400 batch_job_id_required / folder_id_required / artwork_job_id_required if any is missing/empty). The _ai_batch_latest_state() fallback is removed entirely. The artwork job is looked up from the real job store and validated server-side (404 artwork_job_not_found, 400 artwork_job_mismatch if type/album_id don't match this folder, 409 artwork_job_not_terminal if still running) rather than trusted from the client. Truth table: real on-disk art present → fetched always wins, even if the job claims failure/cancellation/timeout; otherwise the job's terminal_outcome maps to cancelled / timed_out / failed. IntakePanel.tsx and client.ts updated to send the retry job's own ID as artwork_job_id; the "Needs attention" chip now reads "Artwork retry cancelled" / "Artwork retry timed out" / "Artwork failed" instead of a single generic label.

5. FetchArt namespace diagnostic no longer matches the literal string "site-packages". That string is layout-specific (absent for editable installs, some distro dist-packages layouts, etc.) — the GitHub Actions runner itself resolves beets to /home/runner/.local/lib/python3.12/site-packages/..., which happened to still contain the string, but relying on it was fragile. _fetchart_namespace_probe() now checks that the resolved module path does not live under this app's own bundled code directory, cross-checked with importlib.metadata.distribution("beets") actually resolving. Verified against the real, unmodified --no-cache Docker image: /api/setup/status still reports fetchart.operational: true with the new logic (confirmed via a live curl against the running container, not a UI claim).

Files changed (this pass, on top of f8ccfa4)

• app.py — mbsync/write/move timeout handling, empty-import-result raise, _repair_album_art timeout tracking, /api/albums/<aid>/fetch-art job metadata + terminal outcome, /api/ai-batch/reconcile-artwork identity enforcement + truth table
• routes_setup.py — _fetchart_namespace_probe() portability rewrite
• frontend/src/api/client.ts — reconcileArtwork() now takes artworkJobId
• frontend/src/features/intake/IntakePanel.tsx — sends artwork_job_id, distinct cancelled/timed-out/failed chip labels
• tests/test_post_retag_artwork_integration.py — isolation fix (atexit + per-test rebind + containment assertion), 4 new/replaced timeout tests
• tests/test_artwork_retry_reconciliation.py — isolation fix, rewritten for mandatory artwork_job_id, 8 new tests (cancelled/timed_out/mismatch/not-terminal/required-field 400s/no-fallback)
• tests/test_ai_batch_import_reliability.py — 1 new static-assertion test confirming the empty-result raise

Tests run

• Focused modules (test_post_retag_artwork_integration, test_artwork_retry_reconciliation, test_routes_setup, test_ai_batch_import_reliability, test_post_retag_artwork_fetch) run individually and together, in both natural and reversed module-load order, 5x consecutively — clean (117 tests/run in the combined form).
• Full python -m unittest discover -s tests -p "test_*.py": 13 consecutive clean runs under the actual CI toolchain (Docker, python:3.13-slim, pip install -r requirements.txt, matching unit-tests.yml/security.yml exactly) — 1271 tests/run, plus additional clean runs on Windows locally. One unrelated pre-existing flake noted above, not introduced by this PR and not in scope.
• python -m py_compile app.py helpers_mb.py job_engine.py routes_jobs.py routes_lidarr.py routes_setup.py scripts/security_secret_scan.py scripts/validate_compose_security.py scripts/verify_security_config.py — clean.
• scripts/security_secret_scan.py — clean. scripts/validate_compose_security.py — ok: true (pre-existing unrelated warnings only). scripts/verify_security_config.py — clean (pre-existing unset-env-var list only, expected in a dev checkout).
• Frontend: npm ci (0 vulnerabilities), npm run typecheck, npm run lint, npm run build — all clean.
• Docker: real docker build --pull=false (no stale layers reused for the changed files), container started, /health/live → alive, /api/setup/status → fetchart.operational: true verified via live curl, clean shutdown, no errors in container logs.

Live GitHub CI at de21ab1

All six required workflows green: lint (frontend-lint), typecheck (frontend-typecheck), node-build (build), docker-build (compose-verification), unit-tests (python-tests ×2 matrix), security (security ×2 matrix). Verified via gh pr checks 32 --watch against the live run, not inferred from local results.

Known limitations / incomplete

• The live TrueNAS incident's actual root cause on that deployment remains unidentified — unchanged from the prior pass, still not claimed fixed here.
• The pre-existing test_ai_batch_retry_race.py concurrency flake found during this validation (see above) is unfixed; it's unrelated to this PR's changes and is flagged for separate follow-up.
• _ai_import_folder's "no album found" early-return path still reports artwork_status: "skipped_no_album" without attempting FetchArt — unchanged from the prior pass.
• PR feat: enforce matching decisions for playlist suggestions #31 (playlist matching-contract suggestions) will still need rebasing once this merges, since both touch overlapping areas of app.py; not addressed here per scope.