fix(admin): three review findings — audit no-op, 404, chapter priority (0.41.1)

- admin_safe_set_is_admin: short-circuit when target.is_admin == value,
  before writing audit. PATCH {is_admin: true} on someone already admin
  previously wrote a misleading "promote_user" row even though the UPDATE
  was a no-op.

- list_chapters (/admin/mangas/:id/chapters): explicit exists() check on
  manga_id, returns 404 instead of 200 [] for a typo'd / deleted manga.

- ChapterSyncState priority: the Failed branch now requires page_count = 0,
  so a chapter with pages on disk AND a historical dead job (from a
  re-download attempt that crashed) stays Synced. The old order
  contradicted Synced's documented "downloaded at some point" contract.
  Doc comments updated alongside the SQL.

Three new regression tests pin the behaviour.

backend/Cargo.lock

@@ -1470,7 +1470,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
 
 [[package]]
 name = "mangalord"
-version = "0.41.0"
+version = "0.41.1"
 dependencies = [
  "anyhow",
  "argon2",

backend/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mangalord"
-version = "0.41.0"
+version = "0.41.1"
 edition = "2021"
 default-run = "mangalord"
 

backend/src/api/admin/mangas.rs

@@ -77,6 +77,11 @@ async fn list_chapters(
     _admin: RequireAdmin,
     Path(manga_id): Path<Uuid>,
 ) -> AppResult<Json<Vec<AdminChapterRow>>> {
+    // Explicit existence check so a typo / deleted manga returns 404
+    // rather than a misleading "no chapters" 200.
+    if !repo::manga::exists(&state.db, manga_id).await? {
+        return Err(AppError::NotFound);
+    }
     let rows = repo::admin_view::list_chapters_with_sync_state(&state.db, manga_id).await?;
     Ok(Json(rows))
 }

backend/src/domain/sync_state.rs

@@ -31,10 +31,12 @@ pub enum ChapterSyncState {
     /// At least one `chapter_sources` row exists AND all of them are
     /// `dropped_at IS NOT NULL`.
     Dropped,
-    /// Most recent `sync_chapter_content` job for this chapter is `dead`
-    /// (terminal failure). Checked BEFORE `NotDownloaded` so the more
+    /// `page_count = 0` AND a `dead` `sync_chapter_content` job exists
+    /// for this chapter. Checked BEFORE `NotDownloaded` so the more
     /// informative "we tried and it died" state wins over "we never
-    /// got around to it".
+    /// got around to it". Does NOT fire when `page_count > 0`, because
+    /// pages on disk mean the chapter IS synced regardless of historical
+    /// job failures — see the priority comment in `repo::admin_view`.
     Failed,
     /// `page_count = 0` and no in-flight or failed job — the chapter
     /// row exists but content has never been downloaded.

backend/src/repo/admin_view.rs

@@ -13,8 +13,14 @@
 //! Priority order for `ChapterSyncState`:
 //!   1. `Downloading` — pending/running `sync_chapter_content` for this id
 //!   2. `Dropped` — chapter has source rows AND all are dropped
-//!   3. `Failed` — most recent terminal `sync_chapter_content` is `dead`
-//!   4. `NotDownloaded` — `page_count = 0`
+//!   3. `Failed` — `page_count = 0` AND a `dead` `sync_chapter_content`
+//!      row exists for this chapter. Constrained to `page_count = 0`
+//!      because once pages are on disk the chapter IS synced — a
+//!      historical dead job (likely from a re-download attempt that
+//!      crashed) is noise that gets reaped after retention. Surfacing
+//!      "Failed" when content is present would contradict
+//!      `ChapterSyncState::Synced`'s "downloaded at some point" contract.
+//!   4. `NotDownloaded` — `page_count = 0`, no in-flight, no dead job
 //!   5. `Synced` — `page_count > 0`
 //!
 //! Reminder: `done` jobs are reaped after `CRAWLER_JOB_RETENTION_DAYS`,
@@ -181,7 +187,8 @@ pub async fn list_chapters_with_sync_state(
                       WHERE cs.chapter_id = c.id AND cs.dropped_at IS NULL
                    )
               THEN 'dropped'
-              WHEN EXISTS (
+              WHEN c.page_count = 0
+               AND EXISTS (
                 SELECT 1 FROM crawler_jobs cj
                  WHERE cj.state = 'dead'
                    AND cj.payload->>'kind' = 'sync_chapter_content'

backend/src/repo/user.rs

@@ -172,6 +172,16 @@ pub async fn admin_safe_set_is_admin(
         return Err(AppError::NotFound);
     };
 
+    // No-op: caller asked to set `is_admin` to its current value. Return
+    // the row as-is without writing an audit entry — otherwise repeated
+    // PATCH calls (browser retry, double-click) pile misleading
+    // "promote_user" rows in `admin_audit` for actions that changed
+    // nothing.
+    if target.is_admin == value {
+        tx.commit().await?;
+        return Ok(target);
+    }
+
     // Recount inside the lock — this is the authoritative read.
     if target.is_admin && !value {
         let admin_count: i64 =

backend/tests/api_admin_mangas.rs

@@ -423,6 +423,56 @@ async fn http_list_chapters_returns_per_chapter_state(pool: PgPool) {
     assert_eq!(items[1]["sync_state"], "not_downloaded");
 }
 
+#[sqlx::test(migrations = "./migrations")]
+async fn http_list_chapters_returns_404_for_unknown_manga(pool: PgPool) {
+    // Regression: used to return 200 [] for a non-existent manga,
+    // which silently rendered "No chapters." for a typo'd / deleted id.
+    let h = common::harness(pool.clone());
+    let (_admin, cookie) = seed_admin(&pool, &h.app).await;
+    let resp = h
+        .app
+        .oneshot(common::get_with_cookie(
+            &format!("/api/v1/admin/mangas/{}/chapters", Uuid::new_v4()),
+            &cookie,
+        ))
+        .await
+        .unwrap();
+    assert_eq!(resp.status(), StatusCode::NOT_FOUND);
+}
+
+#[sqlx::test(migrations = "./migrations")]
+async fn chapter_state_synced_when_pages_present_even_with_dead_job(pool: PgPool) {
+    // Regression: the old CASE prioritised the dead-job branch above
+    // the page_count check, so a chapter with pages on disk AND a
+    // historical dead job (e.g. from a re-download attempt that
+    // crashed) flipped to Failed — contradicting Synced's "downloaded
+    // at some point" contract.
+    seed_source(&pool).await;
+    let m = insert_manga(&pool, "M").await;
+    let c = insert_chapter(&pool, m, 1, 12).await; // pages present
+    insert_chapter_source(&pool, c, "ckey-1", false).await;
+    insert_job(
+        &pool,
+        json!({
+            "kind": "sync_chapter_content",
+            "source_id": SOURCE_ID,
+            "chapter_id": c.to_string(),
+            "source_chapter_key": "ckey-1",
+        }),
+        "dead",
+    )
+    .await;
+
+    let rows = repo::admin_view::list_chapters_with_sync_state(&pool, m)
+        .await
+        .unwrap();
+    assert_eq!(
+        rows[0].sync_state,
+        mangalord::domain::ChapterSyncState::Synced,
+        "pages on disk override historical dead-job noise"
+    );
+}
+
 #[sqlx::test(migrations = "./migrations")]
 async fn http_list_mangas_requires_admin(pool: PgPool) {
     let h = common::harness(pool);

backend/tests/api_admin_users.rs

@@ -330,6 +330,37 @@ async fn promote_writes_audit_row(pool: PgPool) {
     assert_eq!(target, Some(b.id));
 }
 
+#[sqlx::test(migrations = "./migrations")]
+async fn redundant_promote_does_not_write_audit_row(pool: PgPool) {
+    // Regression: PATCH {is_admin: true} on someone already admin used
+    // to UPDATE (no-op) and still INSERT a misleading "promote_user"
+    // audit row. Should short-circuit without touching admin_audit.
+    let h = common::harness(pool.clone());
+    let (_a_name, a_cookie, _a_id) = seed_admin(&pool, &h.app).await;
+    let (b_name, _b_cookie, _b_id) = seed_admin(&pool, &h.app).await; // already admin
+
+    let b = repo::user::find_by_username(&pool, &b_name)
+        .await
+        .unwrap()
+        .unwrap();
+    let resp = h
+        .app
+        .oneshot(common::patch_json_with_cookie(
+            &format!("/api/v1/admin/users/{}", b.id),
+            json!({ "is_admin": true }),
+            &a_cookie,
+        ))
+        .await
+        .unwrap();
+    assert_eq!(resp.status(), StatusCode::OK);
+
+    let (count,): (i64,) = sqlx::query_as("SELECT COUNT(*) FROM admin_audit")
+        .fetch_one(&pool)
+        .await
+        .unwrap();
+    assert_eq!(count, 0, "no-op promote must not write audit row");
+}
+
 #[sqlx::test(migrations = "./migrations")]
 async fn delete_writes_audit_row(pool: PgPool) {
     let h = common::harness(pool.clone());

frontend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "mangalord-frontend",
-  "version": "0.41.0",
+  "version": "0.41.1",
   "private": true,
   "type": "module",
   "scripts": {