diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx index 0282de00..5a3687f1 100644 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx @@ -5,16 +5,18 @@ // no per-row detail is thin, and neither a config-PR panel nor a status // panel has anything to render against without a roster to embed in. // -// Three fetches, all on a refresh-interval cadence like HivesPage: `GET -// /api/agents` (just names — the identity store is the roster, and -// deliberately says nothing about health), `GET /api/config-prs` (agent -// name -> open PR, only agents with one present), and `GET -// /api/agents/status` (one row per roster agent, freshness derived at read -// time — same shape and same roster-not-bucket rule as HivesPage's -// `/api/hives/status`, see `swarm-controller/src/agent_status.rs`). Merged -// client-side into one row per agent rather than N per-agent calls — -// exactly why the bulk endpoints exist instead of looping a single-agent -// one. +// One fetch, on a refresh-interval cadence like HivesPage: `GET +// /api/agents/status` — one row per roster agent (identity store is the +// roster, so a never-reported agent still gets a row), each already +// carrying its freshness/snapshot *and* its open config PR if any. That +// merge used to be three separate fetches (`/api/agents`, `/api/config-prs`, +// `/api/agents/status`) joined client-side by name; per operator review +// feedback ("the view should be filled by a single backend call") the +// join moved server-side instead — see `swarm-controller/src/main.rs`'s +// `get_agents_status` handler for where `config_pr` gets merged in and why +// that's the handler's job rather than `agent_status::AgentStatusReader`'s. +// No more per-page joining left to do here: the wire row *is* the table +// row. // // Owns the "+ agent" trigger too — the roster this populates is the // natural home for the action that populates it; a separate top-level @@ -50,7 +52,10 @@ interface AgentStatusSnapshot { running: boolean; } -interface AgentStatusRow { +// Mirrors `agent_status::AgentStatusRow` field-for-field — this *is* the +// table row now, not a shape assembled from it, so there's no separate +// join-result type to keep in sync with the wire contract by hand. +interface AgentRow { name: string; hive: string | null; freshness: Freshness; @@ -59,12 +64,7 @@ interface AgentStatusRow { // `RelativeTime` recomputes age client-side from `last_seen_unix` // rather than rendering a once-computed-at-fetch value. snapshot: AgentStatusSnapshot | null; -} - -interface AgentRow { - name: string; - configPr: ConfigPrStatus | null; - status: AgentStatusRow | null; + config_pr: ConfigPrStatus | null; } // Same tone/label pairing as HivesPage — one freshness enum shared by both @@ -78,14 +78,13 @@ const FRESHNESS: Record = { const COLUMNS: TableColumn[] = [ { key: 'name', header: 'name', render: (a) => a.name }, - { key: 'hive', header: 'hive', render: (a) => a.status?.hive ?? '—' }, + { key: 'hive', header: 'hive', render: (a) => a.hive ?? '—' }, { key: 'status', header: 'status', render: (a) => { - if (!a.status) return '—'; - const { tone, label } = FRESHNESS[a.status.freshness]; - const text = a.status.snapshot?.status_text; + const { tone, label } = FRESHNESS[a.freshness]; + const text = a.snapshot?.status_text; // A `running: false` snapshot always carries `status_text: null` // (the wire contract's own rule, not something this page derives), // so an agent that reported recently but isn't running still shows @@ -97,10 +96,10 @@ const COLUMNS: TableColumn[] = [ <> {text ? `${text} — ` : ''} {label} - {a.status.last_seen_unix !== null ? ( + {a.last_seen_unix !== null ? ( <> {' '} - () + () ) : null} @@ -113,16 +112,16 @@ const COLUMNS: TableColumn[] = [ key: 'config-pr', header: 'config PR', render: (a) => - a.configPr ? ( + a.config_pr ? ( - #{a.configPr.pr_number} + a.config_pr.html_url ? ( + + #{a.config_pr.pr_number} ) : ( - `#${a.configPr.pr_number}` + `#${a.config_pr.pr_number}` ) } /> @@ -145,34 +144,12 @@ export function AgentsPage() { useRefreshInterval(intervalMs, () => { (async () => { - const [namesRes, prsRes, statusRes] = await Promise.all([ - fetch('/api/agents'), - fetch('/api/config-prs'), - fetch('/api/agents/status'), - ]); - if (!namesRes.ok) { - setError(await readApiError(namesRes)); + const res = await fetch('/api/agents/status'); + if (!res.ok) { + setError(await readApiError(res)); return; } - if (!prsRes.ok) { - setError(await readApiError(prsRes)); - return; - } - if (!statusRes.ok) { - setError(await readApiError(statusRes)); - return; - } - const names = (await namesRes.json()) as string[]; - const prs = (await prsRes.json()) as Record; - const statuses = (await statusRes.json()) as AgentStatusRow[]; - const statusByName = new Map(statuses.map((s) => [s.name, s])); - setRows( - names.map((name) => ({ - name, - configPr: prs[name] ?? null, - status: statusByName.get(name) ?? null, - })), - ); + setRows((await res.json()) as AgentRow[]); // A refresh that succeeds clears a previous failure — otherwise a // transient error would sit on screen forever after the data itself // has recovered. diff --git a/swarm-controller/src/agent_status.rs b/swarm-controller/src/agent_status.rs index 866c3b51..2997debf 100644 --- a/swarm-controller/src/agent_status.rs +++ b/swarm-controller/src/agent_status.rs @@ -45,6 +45,17 @@ pub struct AgentStatusRow { /// failed to decode as [`swarm_queue_client::agent_status::AgentStatus`] /// (logged as a warning naming the agent). pub snapshot: Option, + /// The agent's open config PR, if any. Always `None` coming out of + /// [`AgentStatusReader::view`] itself — this module has no forge + /// client and stays that way, same separation + /// [`crate::status`]/[`crate::agent_status`] already keep from each + /// other. `GET /api/agents/status`'s handler fills this field in + /// after the fact from `AppState::config_prs`, which is the one + /// place that already holds both a roster-shaped answer and a + /// config-PR cache — see the handler for why merging there, not + /// here, is what makes this the single call swarm-ui's agent roster + /// page needs. + pub config_pr: Option, } /// A snapshot as retained by the bucket, with the hive it was published @@ -164,6 +175,7 @@ fn render( last_seen_unix: None, age_seconds: None, snapshot: None, + config_pr: None, }, }) .collect(); @@ -206,6 +218,7 @@ fn row( .and_then(|d| i64::try_from(d.as_secs()).ok()), age_seconds: age.map(|age| age.as_secs()), snapshot: offered.payload.clone(), + config_pr: None, } } diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index cf33cbf2..dc1f67da 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -27,7 +27,7 @@ use forgejo_api::structs::{ use forgejo_api::{ApiErrorKind, Auth, Forgejo, ForgejoError}; use futures_util::{StreamExt as _, TryStreamExt as _}; use reqwest::StatusCode; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, HashMap, HashSet}; use utoipa::ToSchema; @@ -35,7 +35,7 @@ use crate::webhook::DeliveryKind; /// An agent's open config-PR, as [`Client::list_open_config_prs`] reports it /// and `GET /api/agents/{name}/config-pr` serves it. -#[derive(Clone, Debug, PartialEq, Serialize, ToSchema)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, ToSchema)] pub struct ConfigPrStatus { pub pr_number: u64, /// Absent only if Forgejo itself omitted the field — every real PR has diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index b9b7e47b..ed8dd00a 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -889,17 +889,27 @@ async fn get_hives_status( } } -/// What each agent last said about itself, read from the swarm queue at -/// request time. +/// What each agent last said about itself, plus its open config PR if any, +/// read at request time — the single call swarm-ui's agent roster page +/// fills its whole table from (mara, 2026-09-02: "the view should be +/// filled by a single backend call"). /// /// Every agent in the roster gets a row whether or not it has ever /// reported — see the `agent_status` module for why, and for why its hive -/// comes from its own bucket key rather than a second lookup. +/// comes from its own bucket key rather than a second lookup. `config_pr` +/// is merged in here rather than inside `agent_status::AgentStatusReader`: +/// that module has no forge client and is not the place to add one just +/// for this one field, whereas this handler already holds both a +/// roster-shaped status view and `AppState::config_prs` — the one spot +/// that has both without a second, avoidable dependency. Absent on every +/// row when no forge is configured here, same "absence is the answer" +/// shape `get_config_prs` uses — not a reason to fail the whole response +/// over a field that already knows how to be missing. #[utoipa::path( get, path = "/api/agents/status", responses( - (status = 200, description = "a row per agent, freshness derived now", body = Vec), + (status = 200, description = "a row per agent, freshness + config PR derived now", body = Vec), (status = 503, description = "no swarm queue is configured here, no identity bridge is configured, or either store could not be read", body = String), ), tag = "agents" @@ -923,7 +933,15 @@ async fn get_agents_status( StatusUnavailable(detail) })?; match reader.view(&roster, std::time::SystemTime::now()).await { - Ok(rows) => Ok(Json(rows)), + Ok(mut rows) => { + if let Some(cache) = state.config_prs.as_ref() { + let mut config_prs = cache.snapshot(); + for row in &mut rows { + row.config_pr = config_prs.remove(&row.name); + } + } + Ok(Json(rows)) + } Err(e) => { let detail = format!("{e:#}"); tracing::warn!(error = %detail, "reading the agent-status bucket failed");