Browse Source

web: recipes cloned after startup become visible, and the panel says why a list is empty

Two halves of the same bug: a recipe repo cloned AFTER the dashboard process
started (the ⟳ button, the background resync, or a first boot whose initial
sync failed) never entered the recipe search path — the clone landed on disk,
every log line said success, and the recipe table stayed empty until a
container restart. The dashboard's refresher now merges the repo cache
(recipe_repos::cached_roots) into its own config on each keepalive tick,
watches newly appeared recipe dirs and re-collects when the set changes.

And because 'nothing appeared' is indistinguishable from 'no repo configured':
POST /api/repos/sync now reports per-repo outcomes (synced/archived/failed with
the git error) and GET /api/repos reports the configured/cached/archived state,
which the panel renders next to the ⟳ button ('no recipe repos configured',
'2/2 repo(s) not synced', 'repos: 1 synced (1 archived)'). Handlers moved to
src/web/repos.rs to stay under the 500-line cap.
Alexander Korolev 9 hours ago
parent
commit
050f0a8c5c
6 changed files with 243 additions and 32 deletions
  1. 80 4
      src/recipe_repos.rs
  2. 29 4
      src/report/page.html
  3. 43 1
      src/report/serve.rs
  4. 0 22
      src/web/api.rs
  5. 3 1
      src/web/mod.rs
  6. 88 0
      src/web/repos.rs

+ 80 - 4
src/recipe_repos.rs

@@ -29,6 +29,26 @@ pub fn cache_base() -> PathBuf {
         .join(".cache/sm121/recipe-repos")
 }
 
+/// Every cloned recipe repo currently on disk (`<cache>/<sha>/`), sorted.
+///
+/// The recipe search path must not depend on *when* a clone happened: the
+/// dashboard is a long-lived process, and a repo cloned after startup (first
+/// boot's sync failed, a URL changed, a manual ⟳ sync) would otherwise stay
+/// invisible until a restart. Callers merge these into the recipe sources.
+pub fn cached_roots() -> Vec<PathBuf> {
+    let base = cache_base();
+    let Ok(entries) = std::fs::read_dir(&base) else {
+        return Vec::new();
+    };
+    let mut roots: Vec<PathBuf> = entries
+        .flatten()
+        .map(|e| e.path())
+        .filter(|p| p.join("recipes").is_dir())
+        .collect();
+    roots.sort();
+    roots
+}
+
 /// `sha256(url)` hex (full; callers truncate for dir names). Stable per
 /// URL — the cache dir is keyed on the repo URL, not the hostname.
 pub(crate) fn url_hash(url: &str) -> String {
@@ -208,13 +228,47 @@ pub fn sync_repos(
     registry: &ReposRegistry,
     force: bool,
 ) -> Vec<PathBuf> {
-    let mut roots = Vec::new();
+    sync_repos_report(repos, base, registry, force)
+        .into_iter()
+        .filter_map(|o| o.root)
+        .collect()
+}
+
+/// Why one configured repo did or did not land in the cache — the dashboard
+/// shows this verbatim, because a silent "nothing appeared" is
+/// indistinguishable from a bug in the recipes list.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct RepoOutcome {
+    pub url: String,
+    /// `synced` | `archived` | `failed`
+    pub state: &'static str,
+    pub root: Option<PathBuf>,
+    pub recipes: Option<usize>,
+    pub error: Option<String>,
+}
+
+/// Like [`sync_repos`], but keeps the per-repo outcome instead of only the
+/// reachable roots.
+pub fn sync_repos_report(
+    repos: &[RecipeRepo],
+    base: &Path,
+    registry: &ReposRegistry,
+    force: bool,
+) -> Vec<RepoOutcome> {
+    let mut out = Vec::new();
     for repo in repos {
         if registry.is_archived(&repo.url) {
             info!(
                 "{}",
                 fluent_i18n::t!("recipe-repo-archived-skip", { "url" => &repo.url })
             );
+            out.push(RepoOutcome {
+                url: repo.url.clone(),
+                state: "archived",
+                root: None,
+                recipes: None,
+                error: None,
+            });
             continue;
         }
         let dir = repo_cache_dir(base, &repo.url);
@@ -224,23 +278,45 @@ pub fn sync_repos(
                     "{}",
                     fluent_i18n::t!("recipe-repo-synced", { "url" => &repo.url, "dir" => dir.display().to_string() })
                 );
-                roots.push(dir);
+                out.push(RepoOutcome {
+                    url: repo.url.clone(),
+                    state: "synced",
+                    recipes: recipe_count(&dir),
+                    root: Some(dir),
+                    error: None,
+                });
             }
             Err(e) => {
                 warn!(
                     "{}",
                     fluent_i18n::t!("recipe-repo-unreachable", { "url" => &repo.url, "err" => e.to_string() })
                 );
+                out.push(RepoOutcome {
+                    url: repo.url.clone(),
+                    state: "failed",
+                    root: None,
+                    recipes: None,
+                    error: Some(e.to_string()),
+                });
             }
         }
     }
-    roots
+    out
 }
 
 /// Clone/pull the `Config.recipe_repos` entries into the default cache
 /// base (blocking git work off the async runtime). Best-effort: per-repo
 /// failures are warnings, never errors — returns the reachable roots.
 pub async fn sync_recipe_repos(config: &crate::config::Config) -> Vec<PathBuf> {
+    sync_recipe_repos_report(config)
+        .await
+        .into_iter()
+        .filter_map(|o| o.root)
+        .collect()
+}
+
+/// [`sync_recipe_repos`] with per-repo detail (the dashboard's ⟳ result).
+pub async fn sync_recipe_repos_report(config: &crate::config::Config) -> Vec<RepoOutcome> {
     let repos = config.recipe_repos.clone();
     let reg_path = registry_path();
     let base = cache_base();
@@ -248,7 +324,7 @@ pub async fn sync_recipe_repos(config: &crate::config::Config) -> Vec<PathBuf> {
     tokio::task::spawn_blocking(move || {
         crate::locale::apply_global_locale();
         let _ = std::fs::create_dir_all(&base);
-        sync_repos(&repos, &base, &registry, false)
+        sync_repos_report(&repos, &base, &registry, false)
     })
     .await
     .unwrap_or_default()

+ 29 - 4
src/report/page.html

@@ -122,6 +122,7 @@
   </label>
   <button id="prev">‹</button><span id="count"></span><button id="next">›</button>
   <button id="syncbtn" title="sync the recipe repos now (dashboard job; new recipes appear via the live stream)" style="display:none">⟳ sync</button>
+  <span id="repostat" class="dim" style="display:none"></span>
   <span id="live" class="dim" title="live updates">● off</span>
   <button id="bell" title="updates" aria-label="updates">
     <svg width="17" height="17" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M18 8a6 6 0 0 0-12 0c0 7-3 9-3 9h18s-3-2-3-9"/><path d="M13.7 21a2 2 0 0 1-3.4 0"/></svg>
@@ -731,15 +732,39 @@ document.getElementById("mscopy").addEventListener("click", async (e) => {
         setTimeout(() => { e.target.textContent = "copy"; }, 1200); } catch { /* clipboard blocked */ }
 });
 const syncBtn = document.getElementById("syncbtn");
-if (EMBED) syncBtn.style.display = "";
+const repoStat = document.getElementById("repostat");
+// An empty recipe table must say WHY: no repo configured, an archived repo
+// and a repo whose clone keeps failing all render the same otherwise.
+function showRepoStat(text, bad) {
+  if (!repoStat) return;
+  repoStat.style.display = text ? "" : "none";
+  repoStat.textContent = text;
+  repoStat.style.color = bad ? "#e8833a" : "";
+  repoStat.title = text;
+}
+async function loadRepoStatus() {
+  try {
+    const j = await (await fetch("/api/repos")).json();
+    if (!j.configured) { showRepoStat("⚠ no recipe repos configured", true); return; }
+    const arch = (j.repos || []).filter(r => r.archived).length;
+    const bad = (j.repos || []).filter(r => !r.cached && !r.archived).length;
+    if (bad) { showRepoStat(`⚠ ${bad}/${j.configured} repo(s) not synced`, true); return; }
+    if (arch) { showRepoStat(`repos: ${j.configured - arch}/${j.configured} synced (${arch} archived)`, false); return; }
+    showRepoStat(`repos: ${j.configured} synced`, false);
+  } catch { /* hosted mode has no /api/repos (standalone report) */ }
+}
+if (EMBED) { syncBtn.style.display = ""; loadRepoStatus(); }
 syncBtn.addEventListener("click", async () => {
   syncBtn.disabled = true; syncBtn.textContent = "⟳ …";
   try {
     const res = await fetch("/api/repos/sync", { method: "POST" });
     const j = await res.json().catch(() => ({}));
-    syncBtn.textContent = j.ok ? `⟳ ${j.repos} ok` : "⟳ ✗";
-    syncBtn.title = j.detail || (j.ok ? "recipe repos synced" : String(res.status));
-  } catch (err) { syncBtn.textContent = "⟳ ✗"; syncBtn.title = String(err); }
+    const bad = !j.ok;
+    syncBtn.textContent = bad ? "⟳ ✗" : `⟳ ${j.repos} ok`;
+    syncBtn.title = j.detail || (bad ? String(res.status) : "recipe repos synced");
+    showRepoStat(j.detail || "", bad);
+    if (!bad) loadRepoStatus();
+  } catch (err) { syncBtn.textContent = "⟳ ✗"; syncBtn.title = String(err); showRepoStat(String(err), true); }
   setTimeout(() => { syncBtn.disabled = false; syncBtn.textContent = "⟳ sync"; }, 6000);
 });
 // column checkboxes

+ 43 - 1
src/report/serve.rs

@@ -82,6 +82,11 @@ pub(crate) async fn refresh_loop(
     fleet_rx: Option<tokio::sync::broadcast::Receiver<crate::web::events::Event>>,
 ) {
     let mut fleet_rx = fleet_rx;
+    // The dashboard outlives its own startup: a recipe repo cloned later (a
+    // manual ⟳ sync, the background loop, a first boot whose sync failed) has
+    // to become visible without a restart. This is a private copy of the
+    // config — the CLI paths already append the roots at process start.
+    let mut cfg: Config = (*cfg).clone();
     let (ev_tx, mut ev_rx) = tokio::sync::mpsc::unbounded_channel::<()>();
     let watcher = notify::recommended_watcher(move |res: Result<notify::Event, notify::Error>| {
         if let Ok(ev) = res
@@ -91,9 +96,12 @@ pub(crate) async fn refresh_loop(
         }
     });
     let mut watcher = watcher.ok();
+    let mut watched: std::collections::HashSet<std::path::PathBuf> = Default::default();
     if let Some(w) = &mut watcher {
         for dir in crate::config::watchable_recipe_dirs(&cfg) {
-            let _ = w.watch(&dir, notify::RecursiveMode::Recursive);
+            if w.watch(&dir, notify::RecursiveMode::Recursive).is_ok() {
+                watched.insert(dir);
+            }
         }
     }
     let mut last = String::new();
@@ -153,6 +161,40 @@ pub(crate) async fn refresh_loop(
             // snapshot arrived (the add-on has no ledger: `fleet_up` is its
             // only running signal, so it must be recomputed from the probe).
             _ = tokio::time::sleep(keepalive) => {
+                // A repo cloned since the last tick (manual sync, the add-on's
+                // background loop, a first boot whose sync failed) must become
+                // visible without a restart: watch it and re-collect.
+                let mut dirs_changed = false;
+                for root in crate::recipe_repos::cached_roots() {
+                    let src = root.join("recipes").display().to_string();
+                    if !cfg.recipe_sources.contains(&src) {
+                        cfg.recipe_sources.push(src);
+                        cfg.docker_image_sources.push(root.display().to_string());
+                        dirs_changed = true;
+                    }
+                }
+                for dir in crate::config::watchable_recipe_dirs(&cfg) {
+                    if watched.contains(&dir) {
+                        continue;
+                    }
+                    if let Some(w) = &mut watcher
+                        && w.watch(&dir, notify::RecursiveMode::Recursive).is_ok()
+                    {
+                        watched.insert(dir);
+                        dirs_changed = true;
+                    }
+                }
+                if dirs_changed {
+                    rows = collect(&cfg, archived);
+                    crate::report::stale::cross_check(&cfg, &mut rows, &cluster_nodes, &mut fleet).await;
+                    hull_ts = fleet.as_ref().map(|f| f.ts);
+                    let json = serde_json::to_string(&rows).unwrap_or_default();
+                    if json != last {
+                        last = json;
+                        let _ = tx.send(last.clone());
+                    }
+                    continue;
+                }
                 let ledger = crate::report::stale::ledger_open(&cfg);
                 let fresh_fleet = fleet.as_ref().is_some_and(|f| Some(f.ts) != hull_ts);
                 if rows.is_empty() || crate::report::stale::ledger_differs(&rows, &ledger) {

+ 0 - 22
src/web/api.rs

@@ -292,28 +292,6 @@ pub(crate) async fn recipes(app: State<AppState>) -> Json<serde_json::Value> {
     Json(serde_json::json!({ "recipes": out }))
 }
 
-/// `POST /api/repos/sync` — clone/pull every configured recipe repo right
-/// now (the same best-effort batch the command start runs; unreachable
-/// repos are skipped, never fatal). A single in-flight guard: a second
-/// sync while one is running gets a 409, not a second git storm. The
-/// recipes panel's ⟳ button calls it; the report refresher's file watcher
-/// then picks up the new/changed recipes on its own.
-pub(crate) async fn repos_sync(app: State<AppState>) -> Result<Json<serde_json::Value>, ApiError> {
-    static SYNCING: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);
-    if SYNCING.swap(true, std::sync::atomic::Ordering::SeqCst) {
-        return Err((
-            StatusCode::CONFLICT,
-            "a repo sync is already running".into(),
-        ));
-    }
-    let config = app.config.clone();
-    let roots = crate::recipe_repos::sync_recipe_repos(&config).await;
-    SYNCING.store(false, std::sync::atomic::Ordering::SeqCst);
-    Ok(Json(
-        serde_json::json!({ "ok": true, "repos": roots.len() }),
-    ))
-}
-
 // ---------------------------------------------------------------------------
 // P3: log windows — attach a streaming `docker logs -f` per node+recipe+rank
 // (idempotent), detach, and fetch ring history for a fresh window.

+ 3 - 1
src/web/mod.rs

@@ -14,6 +14,7 @@ pub mod jobs;
 pub mod logs;
 pub mod model_settings;
 pub mod probe;
+mod repos;
 pub mod ssh_stream;
 pub mod topo;
 
@@ -265,7 +266,8 @@ pub fn router(app: AppState) -> Router {
         .route("/api/model-settings", get(model_settings::handle))
         .route("/api/power", post(api::power))
         .route("/api/recipes", get(api::recipes))
-        .route("/api/repos/sync", post(api::repos_sync))
+        .route("/api/repos/sync", post(repos::repos_sync))
+        .route("/api/repos", get(repos::repos_status))
         .route("/api/login", post(login))
         .route(
             "/api/logs",

+ 88 - 0
src/web/repos.rs

@@ -0,0 +1,88 @@
+//! Recipe-repo API: sync now (`POST /api/repos/sync`) and report the
+//! configured/cached state (`GET /api/repos`).
+//!
+//! The dashboard must be able to explain an empty recipe list: a repo that
+//! was never configured, one that is archived, and one whose clone keeps
+//! failing all look identical in the table. Both handlers therefore return
+//! per-repo detail, which the panel renders next to the ⟳ button.
+
+use axum::Json;
+use axum::extract::State;
+use axum::http::StatusCode;
+
+/// `POST /api/repos/sync` — clone/pull every configured recipe repo right
+/// now (the same best-effort batch the command start runs; unreachable
+/// repos are skipped, never fatal). A single in-flight guard: a second
+/// sync while one is running gets a 409, not a second git storm. The
+/// recipes panel's ⟳ button calls it; the report refresher's file watcher
+/// then picks up the new/changed recipes on its own.
+pub(crate) async fn repos_sync(
+    State(app): State<super::AppState>,
+) -> Result<Json<serde_json::Value>, StatusCode> {
+    static SYNCING: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);
+    if SYNCING.swap(true, std::sync::atomic::Ordering::SeqCst) {
+        return Err(StatusCode::CONFLICT);
+    }
+    let config = app.config.clone();
+    let outcomes = crate::recipe_repos::sync_recipe_repos_report(&config).await;
+    SYNCING.store(false, std::sync::atomic::Ordering::SeqCst);
+    let synced = outcomes.iter().filter(|o| o.root.is_some()).count();
+    let failed: Vec<String> = outcomes
+        .iter()
+        .filter(|o| o.state == "failed")
+        .map(|o| format!("{}: {}", o.url, o.error.clone().unwrap_or_default()))
+        .collect();
+    let detail = if outcomes.is_empty() {
+        "no recipe repos configured (set sm121_recipe_repos / [[recipe_repos]])".to_string()
+    } else if !failed.is_empty() {
+        format!(
+            "{synced} synced, {} failed — {}",
+            failed.len(),
+            failed.join("; ")
+        )
+    } else {
+        format!("{synced} synced")
+    };
+    Ok(Json(serde_json::json!({
+        "ok": failed.is_empty(),
+        "repos": synced,
+        "configured": outcomes.len(),
+        "detail": detail,
+        "outcomes": outcomes.iter().map(|o| serde_json::json!({
+            "url": o.url,
+            "state": o.state,
+            "recipes": o.recipes,
+            "error": o.error,
+        })).collect::<Vec<_>>(),
+    })))
+}
+
+/// `GET /api/repos` — what the recipe-repo config looks like right now, so
+/// the recipes panel can say *why* it is empty instead of showing a blank
+/// table (an unreachable repo, an archived one, or no `[[recipe_repos]]` at
+/// all are otherwise indistinguishable from a broken recipe list).
+pub(crate) async fn repos_status(State(app): State<super::AppState>) -> Json<serde_json::Value> {
+    let cached = crate::recipe_repos::cached_roots();
+    let registry = crate::recipe_repos::registry_path()
+        .map(|p| crate::recipe_repos::load_registry(&p))
+        .unwrap_or_default();
+    let repos: Vec<serde_json::Value> = app
+        .config
+        .recipe_repos
+        .iter()
+        .map(|r| {
+            let root = crate::recipe_repos::cache_base()
+                .join(&crate::recipe_repos::url_hash(&r.url)[..16]);
+            serde_json::json!({
+                "url": r.url,
+                "cached": root.is_dir(),
+                "archived": registry.is_archived(&r.url),
+            })
+        })
+        .collect();
+    Json(serde_json::json!({
+        "configured": repos.len(),
+        "cached_roots": cached.len(),
+        "repos": repos,
+    }))
+}