From 50650476adf0a05db81700996ca2525a1c1542cb Mon Sep 17 00:00:00 2001 From: iris Date: Wed, 2 Sep 2026 12:42:06 +0200 Subject: [PATCH] swarm-controller+swarm-ui: merge config-PR into GET /api/agents/status Per mara's review call on this PR: "the view should be filled by a single backend call." AgentsPage.tsx was doing three fetches (/api/agents, /api/config-prs, /api/agents/status) and joining them client-side by name. Moves the config-PR join server-side instead: AgentStatusRow gains a config_pr field, populated by get_agents_status's handler from AppState::config_prs after agent_status::AgentStatusReader::view() returns - not inside that module, which has no forge client and stays that way (see the field's doc comment for why the handler is the right layer for this merge, not the reader). AgentsPage.tsx now does exactly one fetch and no client-side joining at all - the wire row is the table row. Dropped the separate AgentStatusRow TS interface (folded into AgentRow, which now mirrors the backend type field-for-field) and the /api/agents + /api/config-prs fetches entirely; neither is needed once /api/agents/status already returns every roster agent with its config PR attached. ConfigPrStatus gained Deserialize (previously Serialize-only) since AgentStatusRow derives both and a struct's derive requires every field to support it. --- .../swarm-ui/src/pages/AgentsPage.tsx | 85 +++++++------------ swarm-controller/src/agent_status.rs | 13 +++ swarm-controller/src/forge.rs | 4 +- swarm-controller/src/main.rs | 28 ++++-- 4 files changed, 69 insertions(+), 61 deletions(-) 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");