From 67cfeeffc2d473b7650f2f8289f2aee9c88c75a5 Mon Sep 17 00:00:00 2001 From: Nicholas Bucher Date: Fri, 4 Sep 2026 13:13:27 -0400 Subject: [PATCH 1/8] fix(ui): read substrate statuses as words, not wire constants The controller names the actor states it knows and falls back to the protobuf constant for the rest, so `ACTOR_STATE_CRASHED` reached the page unread. The substrate page now turns any constant-shaped status into a word and drops the enum-name prefix that only repeats the column header. Crashed and failed get their own tone rather than sharing amber with the transitions, and the "Actors by status" tags are chips now, so the summary is coloured the same way the table is. Fixes #2702 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Nicholas Bucher --- .../tests/substrate/substrate.spec.ts | 13 +++- ui/src/mocks/fixtures.ts | 6 +- ui/src/pages/SubstratePage.tsx | 67 ++++++++++++------- 3 files changed, 59 insertions(+), 27 deletions(-) diff --git a/ui/playwright/tests/substrate/substrate.spec.ts b/ui/playwright/tests/substrate/substrate.spec.ts index c0b7d9ced..c065e9f8d 100644 --- a/ui/playwright/tests/substrate/substrate.spec.ts +++ b/ui/playwright/tests/substrate/substrate.spec.ts @@ -42,7 +42,7 @@ test("substrate: the inventory renders, and partial runtime data says so", async // depending on how many there are. Both numbers, or the tile is not worth its space. await expect(page.getByTestId("substrate-stat-pools-value")).toHaveText("2"); await expect(page.getByTestId("substrate-stat-templates-value")).toHaveText("1/2"); - // Two running of four: one of the fixture's actors is `Failed` and another + // Two running of four: one of the fixture's actors is crashed and another // `Snapshotting`, which is exactly the case a bare count would hide. await expect(page.getByTestId("substrate-stat-actors-value")).toHaveText("2/4"); await expect(page.getByTestId("substrate-stat-workers-value")).toHaveText("1/2"); @@ -91,6 +91,13 @@ test("substrate: the inventory renders, and partial runtime data says so", async // The pod, with its IP appended — the two facts an operator needs to go and look. await expect(actors).toContainText("kagent/ateom-default-pool-0"); await expect(actors).toContainText("10.42.1.19"); + + // A wire constant is read to the operator as a word, and coloured as the failure it + // is. `ACTOR_STATE_CRASHED` on screen is the controller's vocabulary leaking through. + await expect(actors).not.toContainText("ACTOR_STATE_CRASHED"); + await expect( + actors.locator("[data-tone]").filter({ hasText: "Crashed" }), + ).toHaveAttribute("data-tone", "danger"); }); await test.step("6. the workers, including the one holding nothing", async () => { @@ -233,7 +240,7 @@ test("substrate: the actor list is ordered, windowed, and bounded", async ({ pag const actors = page.getByTestId("substrate-actors-table"); - // Sorted by status, then by id. `Failed` precedes `Running` precedes `Snapshotting`, + // Sorted by status, then by id. `Crashed` precedes `Running` precedes `Snapshotting`, // and the fixture lists them in none of that order. const ids = await actors.locator(".ant-table-row").evaluateAll((rows) => rows.map((row) => row.querySelector(".ant-table-cell")?.textContent?.trim() ?? ""), @@ -372,7 +379,7 @@ test("substrate: the actor and worker tables offer no sort, and the inline ones .getByTestId("substrate-actors-table") .locator(".ant-table-row") .evaluateAll((rows) => - rows.map((row) => row.textContent?.match(/Failed|Running|Snapshotting/)?.[0] ?? ""), + rows.map((row) => row.textContent?.match(/Crashed|Running|Snapshotting/)?.[0] ?? ""), ); expect(statuses).toEqual([...statuses].sort()); }); diff --git a/ui/src/mocks/fixtures.ts b/ui/src/mocks/fixtures.ts index 919d91c72..0ff5aed02 100644 --- a/ui/src/mocks/fixtures.ts +++ b/ui/src/mocks/fixtures.ts @@ -308,7 +308,11 @@ export const mockSubstrateStatus: SubstrateStatusResponse = { // Last in the fixture and first once sorted: ate-api returns actors in no // particular order, so a fixture that is already in the right order cannot tell // a page that sorts from one that does not. - { actorId: "actor-0aa1", status: "Failed", version: 1 }, + // + // The raw wire constant, because that is what a real controller sends for a state + // it has no name for — a fixture of tidy words would let `ACTOR_STATE_CRASHED` + // reach the page unread and no test object. + { actorId: "actor-0aa1", status: "ACTOR_STATE_CRASHED", version: 1 }, // Shares "Running" with actor-7f21, which is what makes a two-key sort observable: // with every status distinct, sorting by status then by id looks the same as // sorting by status alone. diff --git a/ui/src/pages/SubstratePage.tsx b/ui/src/pages/SubstratePage.tsx index 5e928c98f..2dd6f63a3 100644 --- a/ui/src/pages/SubstratePage.tsx +++ b/ui/src/pages/SubstratePage.tsx @@ -95,7 +95,25 @@ const NAMESPACE_PARAM = "namespace"; const ALL_NAMESPACES = ""; /** - * What a status or phase is telling you, as four readings rather than a dozen strings. + * A wire enum as a word: `ACTOR_STATE_CRASHED` reads as `Crashed`. + * + * The controller names the states it knows, but falls back to the protobuf constant for + * any it does not, so an unmapped state reaches this page as a wire symbol. Proto names + * every value after its own enum, and that prefix only repeats the column header, so it + * goes rather than being spelled out as `Actor state crashed`. + * + * Anything not shaped like a constant is returned untouched: a status the controller has + * already written for a reader must not be rewritten by a guess about its casing. + */ +function humanizeEnum(label: string): string { + const value = label.trim(); + if (!/^[A-Z][A-Z0-9]*(_[A-Z0-9]+)+$/.test(value)) return value; + const words = value.replace(/^[A-Z0-9]+_STATE_/, "").toLowerCase().replace(/_/g, " "); + return words.charAt(0).toUpperCase() + words.slice(1); +} + +/** + * What a status or phase is telling you, as five readings rather than a dozen strings. * * The substrate's vocabulary is not a closed enum on the wire: `phase` and `status` are * plain strings that ate-api and the ActorTemplate controller each fill in their own way, @@ -103,17 +121,20 @@ const ALL_NAMESPACES = ""; * `neutral` and is shown as it arrived — inventing a colour for a word this page has * never seen would be a claim about health nobody made. */ -type StatusTone = "healthy" | "warning" | "progress" | "idle" | "neutral"; +type StatusTone = "healthy" | "danger" | "progress" | "idle" | "neutral"; function statusTone(label: string): StatusTone { - const value = label.trim().toLowerCase(); + const value = humanizeEnum(label).trim().toLowerCase(); if (value === "ready" || value === "running") return "healthy"; - if (value === "failed" || value === "suspending") return "warning"; - if (value === "suspended" || value === "unknown" || value === "") return "idle"; - // Substrings, because these arrive spelled several ways: `Resuming`, `WaitingForWorker`, - // `GoldenSnapshotPending`. All of them mean the same thing to a reader — something is - // under way and the next read will say something different. - if (value.includes("resume") || value.includes("wait") || value.includes("golden")) { + // A crashed or failed actor is not a caution, it is the thing that went wrong. + if (value === "failed" || value === "crashed") return "danger"; + if (value === "suspended" || value === "paused" || value === "unknown" || value === "") { + return "idle"; + } + // Shapes rather than words, because these arrive spelled several ways: `Resuming`, + // `Deleting`, `WaitingForWorker`, `GoldenSnapshotPending`. All of them mean the same + // thing to a reader — something is under way and the next read will say otherwise. + if (value.endsWith("ing") || value.includes("wait") || value.includes("golden")) { return "progress"; } return "neutral"; @@ -127,9 +148,10 @@ function statusTone(label: string): StatusTone { * of a light page. `primary` is not among them in any tone — it is a fill chosen to carry * light text, and as ink on this page it measures about 2.2:1. */ -function StatusChip({ label }: { label: string }) { +function StatusChip({ label, count }: { label: string; count?: number }) { const theme = useTheme(); const tone = statusTone(label); + const text = humanizeEnum(label); const pill = { healthy: { @@ -137,10 +159,10 @@ function StatusChip({ label }: { label: string }) { borderColor: theme.color.successBorder, color: theme.color.successText, }, - warning: { - background: theme.color.warningBg, - borderColor: theme.color.warningBorder, - color: theme.color.warningText, + danger: { + background: theme.color.dangerBg, + borderColor: theme.color.dangerBorder, + color: theme.color.dangerText, }, progress: { background: theme.color.infoBg, @@ -166,9 +188,9 @@ function StatusChip({ label }: { label: string }) { /* * The substrate's vocabulary is open-ended: `phase` and `status` are plain * strings, and a value this build has never seen is shown as it arrived. Some - * of them are long — a cluster answered with `ACTOR_STATE_CRASHED`, which at - * one line overflowed its column and printed itself across the next one. So - * the tag wraps inside the width it is given rather than spilling out of it. + * of them are long — `WaitingForWorker` at one line overflowed its column and + * printed itself across the next one. So the tag wraps inside the width it is + * given rather than spilling out of it. */ whiteSpace: "normal", maxWidth: "100%", @@ -176,7 +198,8 @@ function StatusChip({ label }: { label: string }) { }} data-tone={tone} > - {label.trim() === "" ? "not reported" : label} + {text === "" ? "not reported" : text} + {count === undefined ? null : `: ${count.toLocaleString()}`} ); } @@ -945,8 +968,8 @@ export function SubstratePage() { /> ), key: "status", - // Wide enough for the longest status seen on a real cluster - // (`ACTOR_STATE_CRASHED`) without wrapping it to three lines. + // Wide enough for the longest status seen on a real cluster (`Snapshotting`) + // without wrapping it to three lines. width: 190, render: (_, actor) => , }, @@ -1281,9 +1304,7 @@ export function SubstratePage() { Actors by status {inventory.actorStatusCounts.map((entry) => ( - - {entry.status || "not reported"}: {entry.count.toLocaleString()} - + ))} ) : null} From 8915c8d609976d22394bfb985e0998b6abf86398 Mon Sep 17 00:00:00 2001 From: Nicholas Bucher Date: Fri, 4 Sep 2026 13:35:32 -0400 Subject: [PATCH 2/8] fix(ui): cover every substrate actor state, and set deletion apart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fixtures held three of the nine states a controller can report, so the idle tone was drawn by nothing and `ACTOR_STATE_CRASHED` was the only wire constant under test — enough for a humaniser that special-cased that one word. Deletion no longer shares the transition colour. It is the one in-flight state that does not come back, and an actor reading the same shade as one taking a snapshot is an actor nobody looks at twice. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Nicholas Bucher --- .../tests/substrate/substrate.spec.ts | 38 +++++++++++++------ ui/src/mocks/fixtures.ts | 10 +++++ ui/src/pages/SubstratePage.tsx | 13 ++++++- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/ui/playwright/tests/substrate/substrate.spec.ts b/ui/playwright/tests/substrate/substrate.spec.ts index c065e9f8d..b2c41ddba 100644 --- a/ui/playwright/tests/substrate/substrate.spec.ts +++ b/ui/playwright/tests/substrate/substrate.spec.ts @@ -20,7 +20,7 @@ import { expectSettled, loadPage, routes } from "../../helpers/app"; * * The fixture is built for exactly this: `enabled: true` with an `ateApiError` set, two * worker pools across two namespaces, two templates — one Ready in `kagent`, one Pending in - * `platform` — three actors and two workers, one of the workers holding nothing. The third + * `platform` — eight actors and two workers, one of the workers holding nothing. The crashed * actor sits last in the fixture and first once sorted, which is what makes the ordering * testable at all. */ @@ -42,9 +42,9 @@ test("substrate: the inventory renders, and partial runtime data says so", async // depending on how many there are. Both numbers, or the tile is not worth its space. await expect(page.getByTestId("substrate-stat-pools-value")).toHaveText("2"); await expect(page.getByTestId("substrate-stat-templates-value")).toHaveText("1/2"); - // Two running of four: one of the fixture's actors is crashed and another - // `Snapshotting`, which is exactly the case a bare count would hide. - await expect(page.getByTestId("substrate-stat-actors-value")).toHaveText("2/4"); + // Two running of eight: the rest are crashed, deleting, paused, resuming, suspended + // and snapshotting, which is exactly the case a bare count would hide. + await expect(page.getByTestId("substrate-stat-actors-value")).toHaveText("2/8"); await expect(page.getByTestId("substrate-stat-workers-value")).toHaveText("1/2"); await expect(page.getByTestId("substrate-stat-ateapi-value")).toHaveText("connected"); await expect(page.getByTestId("substrate-stat-scope-value")).toHaveText("all"); @@ -92,9 +92,10 @@ test("substrate: the inventory renders, and partial runtime data says so", async await expect(actors).toContainText("kagent/ateom-default-pool-0"); await expect(actors).toContainText("10.42.1.19"); - // A wire constant is read to the operator as a word, and coloured as the failure it - // is. `ACTOR_STATE_CRASHED` on screen is the controller's vocabulary leaking through. - await expect(actors).not.toContainText("ACTOR_STATE_CRASHED"); + // Both wire constants are read to the operator as words — a humaniser that only knew + // `CRASHED` would leave the other one showing the controller's vocabulary. + await expect(actors).not.toContainText("ACTOR_STATE_"); + await expect(actors).toContainText("Deleting"); await expect( actors.locator("[data-tone]").filter({ hasText: "Crashed" }), ).toHaveAttribute("data-tone", "danger"); @@ -240,12 +241,20 @@ test("substrate: the actor list is ordered, windowed, and bounded", async ({ pag const actors = page.getByTestId("substrate-actors-table"); - // Sorted by status, then by id. `Crashed` precedes `Running` precedes `Snapshotting`, - // and the fixture lists them in none of that order. + // Sorted by status, then by id, and the fixture lists them in none of that order. const ids = await actors.locator(".ant-table-row").evaluateAll((rows) => rows.map((row) => row.querySelector(".ant-table-cell")?.textContent?.trim() ?? ""), ); - expect(ids).toEqual(["actor-0aa1", "actor-3b55", "actor-7f21", "actor-9c03"]); + expect(ids).toEqual([ + "actor-0aa1", + "actor-2e40", + "actor-5d17", + "actor-8b91", + "actor-3b55", + "actor-7f21", + "actor-9c03", + "actor-c3f5", + ]); // Windowed: antd renders rows into a virtual holder rather than a plain tbody, which // is what keeps a list of thousands off the page. @@ -303,7 +312,7 @@ test("substrate: each list narrows on its own, and a match is found wherever it // The count beside the heading is now the *matching* total, so the tile is what // keeps the cluster's own size on screen. A reader who searched and found one // actor must not conclude their cluster is running one. - await expect(page.getByTestId("substrate-stat-actors")).toContainText("/4"); + await expect(page.getByTestId("substrate-stat-actors")).toContainText("/8"); }); await test.step("3. and only that card: the other lists are left alone", async () => { @@ -379,7 +388,12 @@ test("substrate: the actor and worker tables offer no sort, and the inline ones .getByTestId("substrate-actors-table") .locator(".ant-table-row") .evaluateAll((rows) => - rows.map((row) => row.textContent?.match(/Crashed|Running|Snapshotting/)?.[0] ?? ""), + rows.map( + (row) => + row.textContent?.match( + /Crashed|Deleting|Paused|Resuming|Running|Snapshotting|Suspended/, + )?.[0] ?? "", + ), ); expect(statuses).toEqual([...statuses].sort()); }); diff --git a/ui/src/mocks/fixtures.ts b/ui/src/mocks/fixtures.ts index 0ff5aed02..67c213acd 100644 --- a/ui/src/mocks/fixtures.ts +++ b/ui/src/mocks/fixtures.ts @@ -317,6 +317,16 @@ export const mockSubstrateStatus: SubstrateStatusResponse = { // with every status distinct, sorting by status then by id looks the same as // sorting by status alone. { actorId: "actor-3b55", status: "Running", version: 1 }, + // Parked rather than broken, and the only status here that reads as neither: + // without it nothing on the page is drawn in the idle tone. + { actorId: "actor-5d17", status: "Paused", version: 1 }, + // The controller's other unnamed state. `ACTOR_STATE_CRASHED` alone would pass a + // humaniser that special-cased that one word; two of them do not. + { actorId: "actor-2e40", status: "ACTOR_STATE_DELETING", version: 1 }, + // A transition, and a word the page recognises by its shape rather than from a + // list — the same rule that has to carry `Suspending` and `Pausing`. + { actorId: "actor-8b91", status: "Resuming", version: 1 }, + { actorId: "actor-c3f5", status: "Suspended", version: 3 }, ], workers: [ { diff --git a/ui/src/pages/SubstratePage.tsx b/ui/src/pages/SubstratePage.tsx index 2dd6f63a3..0e2cbe994 100644 --- a/ui/src/pages/SubstratePage.tsx +++ b/ui/src/pages/SubstratePage.tsx @@ -121,18 +121,22 @@ function humanizeEnum(label: string): string { * `neutral` and is shown as it arrived — inventing a colour for a word this page has * never seen would be a claim about health nobody made. */ -type StatusTone = "healthy" | "danger" | "progress" | "idle" | "neutral"; +type StatusTone = "healthy" | "danger" | "warning" | "progress" | "idle" | "neutral"; function statusTone(label: string): StatusTone { const value = humanizeEnum(label).trim().toLowerCase(); if (value === "ready" || value === "running") return "healthy"; // A crashed or failed actor is not a caution, it is the thing that went wrong. if (value === "failed" || value === "crashed") return "danger"; + // Deletion is in flight like the transitions below and is checked before them, because + // it is the one that does not come back: an actor that reads the same shade as one + // taking a snapshot is an actor nobody looks at twice. + if (value.includes("delet")) return "warning"; if (value === "suspended" || value === "paused" || value === "unknown" || value === "") { return "idle"; } // Shapes rather than words, because these arrive spelled several ways: `Resuming`, - // `Deleting`, `WaitingForWorker`, `GoldenSnapshotPending`. All of them mean the same + // `Suspending`, `WaitingForWorker`, `GoldenSnapshotPending`. All of them mean the same // thing to a reader — something is under way and the next read will say otherwise. if (value.endsWith("ing") || value.includes("wait") || value.includes("golden")) { return "progress"; @@ -164,6 +168,11 @@ function StatusChip({ label, count }: { label: string; count?: number }) { borderColor: theme.color.dangerBorder, color: theme.color.dangerText, }, + warning: { + background: theme.color.warningBg, + borderColor: theme.color.warningBorder, + color: theme.color.warningText, + }, progress: { background: theme.color.infoBg, borderColor: theme.color.infoBorder, From 544fed843bf3dd59d665a10cf569f4c6c37dc7d0 Mon Sep 17 00:00:00 2001 From: Nicholas Bucher Date: Fri, 4 Sep 2026 15:12:09 -0400 Subject: [PATCH 3/8] feat(ui): show the substrate's actor and worker mix as a bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tiles say how many actors are running; nothing said what the rest were doing, and with the table paged there was nowhere to find out — a reader on page one of 410,110 could not learn that most of them had crashed. Each card now carries a bar above its table, a segment per actor or pod, coloured by status and grouped with everything parked at the end. The legend lists every state a controller can report, so `Crashed` is a thing a reader knows can happen before one does. The tab icon is the kagent mark, and `branding.faviconUrl` lets a distribution replace it alongside the name and logo it could already replace. Status borders were 1.28-2.83:1 against their own fill, which is a decorative edge rather than a boundary. Every tone now measures at least 3:1 in both themes, and the idle chip takes the strong border token rather than the app's hairline. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Nicholas Bucher --- ui/docs/app-extensions.md | 5 +- ui/index.html | 7 + .../tests/substrate/substrate-polling.spec.ts | 11 +- .../tests/substrate/substrate.spec.ts | 85 +++ ui/public/favicon.svg | 16 + ui/src/appExtensions/appExtensions.test.ts | 41 +- ui/src/appExtensions/branding.ts | 23 +- ui/src/appExtensions/index.ts | 2 +- ui/src/main.tsx | 4 +- ui/src/pages/SubstratePage.tsx | 516 ++++++++++++++++-- ui/src/theme/theme.ts | 16 +- 11 files changed, 665 insertions(+), 61 deletions(-) create mode 100644 ui/public/favicon.svg diff --git a/ui/docs/app-extensions.md b/ui/docs/app-extensions.md index d9236fd68..d255efb1e 100644 --- a/ui/docs/app-extensions.md +++ b/ui/docs/app-extensions.md @@ -452,8 +452,9 @@ happy with this application's chrome may still want its own mark on it. ```tsx branding: { - AppIcon: MyMark, // receives { collapsed }; supplied whole, like everything else - appName: "My Product", // used for the document title + AppIcon: MyMark, // receives { collapsed }; supplied whole, like everything else + appName: "My Product", // used for the document title + faviconUrl: "/my-mark.svg", // the tab icon; a URL, since the browser loads it itself } ``` diff --git a/ui/index.html b/ui/index.html index 81b27b97e..86318f564 100644 --- a/ui/index.html +++ b/ui/index.html @@ -4,6 +4,13 @@ kagent + + + + + + + diff --git a/ui/src/appExtensions/appExtensions.test.ts b/ui/src/appExtensions/appExtensions.test.ts index c8892e1f0..937abad39 100644 --- a/ui/src/appExtensions/appExtensions.test.ts +++ b/ui/src/appExtensions/appExtensions.test.ts @@ -1,6 +1,7 @@ import { createElement } from "react"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { + applyExtensionBranding, applyExtensionFieldValues, buildSidebarSections, defineExtensionFormField, @@ -426,6 +427,44 @@ describe("composing several installed extensions", () => { }); }); +describe("applyExtensionBranding", () => { + const favicon = () => document.querySelector("link[data-app-favicon]"); + + beforeEach(() => { + // Head first: replacing its contents drops the element with everything else, + // which resets `document.title` to "". + document.head.innerHTML = '<link rel="icon" data-app-favicon href="/favicon.svg" />'; + document.title = "kagent"; + }); + + it("leaves the application's own title and icon alone when an extension sets neither", () => { + applyExtensionBranding({}); + + // Not reset to a default: every extension would otherwise have to restate the + // branding it was perfectly happy with. + expect(document.title).toBe("kagent"); + expect(favicon()?.getAttribute("href")).toBe("/favicon.svg"); + }); + + it("retargets the shipped link rather than adding a second one", () => { + applyExtensionBranding({ appName: "My Product", faviconUrl: "/my-mark.svg" }); + + expect(document.title).toBe("My Product"); + expect(document.querySelectorAll("link[data-app-favicon]")).toHaveLength(1); + expect(favicon()?.href).toContain("/my-mark.svg"); + }); + + it("still applies the icon when the host page shipped no link to retarget", () => { + document.head.innerHTML = ""; + + applyExtensionBranding({ faviconUrl: "/my-mark.svg" }); + + expect( + document.querySelector<HTMLLinkElement>('link[rel="icon"]')?.href, + ).toContain("/my-mark.svg"); + }); +}); + describe("validateAppExtensions", () => { const page = { path: "/insights", element: createElement("div") }; diff --git a/ui/src/appExtensions/branding.ts b/ui/src/appExtensions/branding.ts index ff92436df..be0fd8ba4 100644 --- a/ui/src/appExtensions/branding.ts +++ b/ui/src/appExtensions/branding.ts @@ -27,17 +27,32 @@ export interface ExtensionBranding { AppIcon?: ComponentType<ExtensionAppIconProps>; /** Product name, used for the document title. */ appName?: string; + /** + * Replaces the tab icon. A URL and not a component, unlike `AppIcon`: the browser + * loads this itself from a `<link>`, so there is nothing for us to render. + */ + faviconUrl?: string; } /** - * Applies the document title a distribution asked for. + * Applies the title and tab icon a distribution asked for. * * Takes the merged branding rather than the install, so the "later extension wins" * rule is applied once, where every other singular capability applies it — see * `selectors.ts`. + * + * Both are left alone when unset rather than reset to a default: the document already + * carries this application's own, and writing them unconditionally would mean every + * extension had to restate the branding it was happy with. */ -export function applyExtensionDocumentTitle( - branding: ExtensionBranding | undefined, -): void { +export function applyExtensionBranding(branding: ExtensionBranding | undefined): void { if (branding?.appName) document.title = branding.appName; + if (!branding?.faviconUrl) return; + + // The tag `index.html` ships, or a fresh one if a host page dropped it — a missing + // icon is not a reason to leave the extension's branding unapplied. + const link = + document.querySelector<HTMLLinkElement>("link[data-app-favicon]") ?? + document.head.appendChild(Object.assign(document.createElement("link"), { rel: "icon" })); + link.href = branding.faviconUrl; } diff --git a/ui/src/appExtensions/index.ts b/ui/src/appExtensions/index.ts index 51aa32b7e..d6f7129a5 100644 --- a/ui/src/appExtensions/index.ts +++ b/ui/src/appExtensions/index.ts @@ -144,7 +144,7 @@ export type { ExtensionTableColumn, ExtensionTableId } from "./tableColumns"; // Branding: the product's own name and mark, which is identity rather than // styling and so should not cost a layout replacement. -export { applyExtensionDocumentTitle } from "./branding"; +export { applyExtensionBranding } from "./branding"; export type { ExtensionAppIconProps, ExtensionBranding } from "./branding"; // Navigation overrides: the other half of contributing an entry — changing one diff --git a/ui/src/main.tsx b/ui/src/main.tsx index 48b53f5ef..12faac787 100644 --- a/ui/src/main.tsx +++ b/ui/src/main.tsx @@ -4,7 +4,7 @@ import { isMockMode } from "./api/config"; import { activeAppExtensions } from "./appExtensions/activeExtensions"; import { extensionBranding, extensionThemes } from "./appExtensions/selectors"; import { loadExtensionStylesheets } from "./appExtensions/theme"; -import { applyExtensionDocumentTitle } from "./appExtensions/branding"; +import { applyExtensionBranding } from "./appExtensions/branding"; import { AuthProvider } from "./auth"; import { App } from "./App"; @@ -17,7 +17,7 @@ async function bootstrap() { // Before the first render: a web font that arrives afterwards reflows // everything already painted. loadExtensionStylesheets(extensionThemes(activeAppExtensions)); - applyExtensionDocumentTitle(extensionBranding(activeAppExtensions)); + applyExtensionBranding(extensionBranding(activeAppExtensions)); // Which backend is serving is decided in one place, `api/config.ts`, and read // here rather than re-derived: two independent readings of the same env var diff --git a/ui/src/pages/SubstratePage.tsx b/ui/src/pages/SubstratePage.tsx index 0e2cbe994..e0ea40611 100644 --- a/ui/src/pages/SubstratePage.tsx +++ b/ui/src/pages/SubstratePage.tsx @@ -14,7 +14,8 @@ import { Typography, } from "antd"; import type { ColumnsType } from "antd/es/table"; -import { useTheme } from "@emotion/react"; +import { useTheme, type CSSObject, type Theme } from "@emotion/react"; +import { useThemeMode } from "@/theme/themeMode"; import { Radio, Search } from "lucide-react"; import { PageFrame } from "@/components/Structure/PageFrame"; import { StatTile } from "@/components/dashboard/StatTile"; @@ -29,6 +30,7 @@ import { type SubstrateSortOrder, type SubstrateWorkerSortField, type SubstrateActorTemplateEntry, + type SubstrateStatusCount, type SubstrateWorkerEntry, type SubstrateWorkerPoolEntry, } from "@/api"; @@ -50,10 +52,11 @@ const GROWING_TABLE_HEIGHT = 420; /** * The interval polling starts at, in seconds. * - * A second is quick enough to watch an actor move between workers and slow enough to - * leave running while reading, which is what this control is for. + * Half a second is quick enough to watch an actor move between workers, which is what + * this control is for. It is also the floor below, so the default is the fastest this + * page will ask — a reader who turns polling on wants to see the cluster move. */ -const DEFAULT_POLL_SECONDS = 1; +const DEFAULT_POLL_SECONDS = 0.5; /** * The fastest this page will ask, in seconds. @@ -132,7 +135,15 @@ function statusTone(label: string): StatusTone { // it is the one that does not come back: an actor that reads the same shade as one // taking a snapshot is an actor nobody looks at twice. if (value.includes("delet")) return "warning"; - if (value === "suspended" || value === "paused" || value === "unknown" || value === "") { + // `idle` among them because that is the word the workers table already uses for a pod + // holding no actor, and a parked worker and a parked actor are the same news. + if ( + value === "suspended" || + value === "paused" || + value === "idle" || + value === "unknown" || + value === "" + ) { return "idle"; } // Shapes rather than words, because these arrive spelled several ways: `Resuming`, @@ -145,19 +156,19 @@ function statusTone(label: string): StatusTone { } /** - * A status, coloured by what it means. + * Each tone's three colours, the theme's own rather than antd's presets. * - * The triples are the theme's own rather than antd's presets, for the reason the palette - * states: antd derives a tag's three colours from one foreground token on the assumption - * of a light page. `primary` is not among them in any tone — it is a fill chosen to carry - * light text, and as ink on this page it measures about 2.2:1. + * antd derives a tag's three from one foreground token on the assumption of a light + * page. `primary` is not among them in any tone — it is a fill chosen to carry light + * text, and as ink on this page it measures about 2.2:1. + * + * `color` is the saturated one and the only one that carries meaning on its own: the + * fills are near-identical tints, about ΔE 3 apart, so a stripe painted with them + * would read as one stripe. That is what the bar below fills with, and it is why the + * bar and the chips read the same status the same way. */ -function StatusChip({ label, count }: { label: string; count?: number }) { - const theme = useTheme(); - const tone = statusTone(label); - const text = humanizeEnum(label); - - const pill = { +function statusPalette(theme: Theme): Record<StatusTone, CSSObject> { + return { healthy: { background: theme.color.successBg, borderColor: theme.color.successBorder, @@ -180,7 +191,9 @@ function StatusChip({ label, count }: { label: string; count?: number }) { }, idle: { background: theme.color.bgElevated, - borderColor: theme.color.border, + // `borderStrong` and not `border`: the hairline token is the app's dividers, and at + // 1.4:1 it is a decorative edge rather than a boundary. This one measures 3.5:1. + borderColor: theme.color.borderStrong, color: theme.color.textMuted, }, neutral: { @@ -188,7 +201,15 @@ function StatusChip({ label, count }: { label: string; count?: number }) { borderColor: theme.color.borderStrong, color: theme.color.text, }, - }[tone]; + }; +} + +/** A status, coloured by what it means. */ +function StatusChip({ label }: { label: string }) { + const theme = useTheme(); + const tone = statusTone(label); + const text = humanizeEnum(label); + const pill = statusPalette(theme)[tone]; return ( <Tag @@ -208,11 +229,339 @@ function StatusChip({ label, count }: { label: string; count?: number }) { data-tone={tone} > {text === "" ? "not reported" : text} - {count === undefined ? null : `: ${count.toLocaleString()}`} </Tag> ); } +/** + * A count at a glance: 999 stays 999, 1,100 becomes `1.1k`. + * + * The legend and the bar are read sideways, and a cluster answered with 410,110 actors — + * a row of exact figures there is a row nobody reads. The exact numbers stay where they + * are acted on: the tiles, the section counts and the table. + * + * `K` lowercased because that is the convention for thousands; `M` and above are left as + * `Intl` writes them, where uppercase is the convention instead. + */ +const compactNumber = new Intl.NumberFormat(undefined, { + notation: "compact", + maximumFractionDigits: 1, +}); +const atAGlance = (count: number) => compactNumber.format(count).replace("K", "k"); + +/** + * Every actor state a controller can report, so the legend is the vocabulary rather than + * today's sample: a reader learns that `Crashed` is a thing that happens by seeing it at + * zero, not by waiting for one. + * + * Mirrors `ActorStatusLabel` in `go/core/internal/substrate/list.go`, which names the + * `ate.dev` `ActorState` enum. Drift is not a failure here: this decides only what is + * listed at zero, and any state the controller reports that is missing from it is added + * to the legend from the data — so a new one appears the first time it happens. + */ +const ACTOR_STATES = [ + "Crashed", + "Deleting", + "Pausing", + "Resuming", + "Running", + "Snapshotting", + "Suspending", + "Paused", + "Suspended", + "Unknown", +]; + +/** The same, for a worker: whatever is on it, or nothing. */ +const WORKER_STATES = ["Idle", ...ACTOR_STATES]; + +/** + * How many actors the bar will draw one segment each for. + * + * A segment per actor is what makes the bar countable — eight ticks with two green is + * read, not estimated. It stops being countable long before it stops being drawable, and + * a cluster answered with 410,110 actors, so past this the bar falls back to one + * proportional band per status. The number is where counting gives out, not where the + * browser does. + */ +const ACTORS_DRAWN_INDIVIDUALLY = 120; + +/** + * The whole actor inventory as one bar, coloured by what each actor is doing. + * + * Two running of ten with the rest suspended is two green segments and eight grey. The + * tile above says how many are running; only this says what the other eight are doing, + * and with the table paged it is the one place the whole distribution appears at all — a + * reader on page one of 410,110 actors has otherwise no way to learn that most of them + * have crashed. + * + * The fills are the pills' own text colours, so an actor is the same colour here as in + * the table. Not the pills' fills: those are near-identical tints about ΔE 3 apart, and a + * bar painted with them would read as one long smudge. + */ +function StatusBar({ + counts, + title, + caption, + emptyText, + testId, + vocabulary, + noun, +}: { + counts: SubstrateStatusCount[]; + /** Every status worth listing at zero. Anything counted but missing is added to it. */ + vocabulary: string[]; + /** What is being counted, for the places with room to say it: `Actors`, `Workers`. */ + noun: string; + /** + * The bar's accessible name, announced with its breakdown. Not drawn: the legend + * beneath already names every colour on it, and a heading over a card that is already + * called "Actors" would only say it twice. + */ + title: string; + /** What this bar is counting, when it is not simply the whole scope. */ + caption?: string; + emptyText: string; + testId: string; +}) { + const theme = useTheme(); + const { mode } = useThemeMode(); + const dark = mode === "dark"; + const palette = statusPalette(theme); + // Short in the legend, where the swatch and the column already say what is counted. + const read = (entry: SubstrateStatusCount) => + `${entry.status || "not reported"}: ${atAGlance(entry.count)}`; + // Long wherever the reading stands on its own — `Suspended Actors: 6` rather than a + // number under a status a tooltip has floated away from. + const readFull = (entry: SubstrateStatusCount) => + `${entry.status || "Not reported"} ${noun}: ${atAGlance(entry.count)}`; + + /* + * Counted by the word rather than by the wire value. + * + * A controller that has learned a state sends `Crashed` and one that has not sends + * `ACTOR_STATE_CRASHED`; both read as `Crashed`, and keyed by the raw string they came + * out as two entries — the legend listed `Crashed` twice, once at zero. + */ + const merged = new Map<string, number>(); + for (const entry of counts) { + const key = humanizeEnum(entry.status); + merged.set(key, (merged.get(key) ?? 0) + entry.count); + } + + /* + * Grouped by status and ordered by the word, with everything parked pushed to the end. + * + * Idle is where a bar's dead weight belongs: a cluster that is mostly suspended reads as + * a short band of activity against a long grey tail, rather than having the interesting + * part cut in half by it. Sorted here rather than trusted from the server, because a bar + * whose segments reorder between polls is a bar nobody can point at. + */ + const order = (entries: SubstrateStatusCount[]) => + [...entries].sort((a, b) => { + const parked = (entry: SubstrateStatusCount) => (statusTone(entry.status) === "idle" ? 1 : 0); + return parked(a) - parked(b) || a.status.localeCompare(b.status); + }); + const entries = [...merged].map(([status, count]) => ({ status, count })); + const present = order(entries.filter((entry) => entry.count > 0)); + const total = present.reduce((sum, entry) => sum + entry.count, 0); + const perActor = total > 0 && total <= ACTORS_DRAWN_INDIVIDUALLY; + const summary = [caption, present.map(readFull).join(", ")].filter(Boolean).join(". "); + + // The one place a tone becomes two colours, so a legend key and the segment it explains + // are the same colour by construction rather than by two expressions agreeing. + const paint = (tone: StatusTone): CSSObject => ({ + /* + * The pill's own three colours, not a mix of one of them with the page. + * + * Mixing toward the page is what turned these grey: every tone converges on the + * background as the fill weakens, so at a subtle strength they all read as the same + * washed-out slab. Taking the pill's fill and the pill's own border instead makes a + * segment the same colour as the chip in the row below by construction, rather than + * by two sets of numbers agreeing — and both are lighter than the mix was. + */ + background: `color-mix(in srgb, ${palette[tone].color} var(--seg-fill), ${palette[tone].background})`, + border: `1px solid ${palette[tone].borderColor}`, + }); + + const segment = (tone: StatusTone, key: string, grow: number, first: boolean, last: boolean) => ( + <div + key={key} + data-tone={tone} + css={{ + /* The pill's own colour, as a wash behind its own outline. Both are mixed toward + the page rather than used at full strength — which dims them on a dark page and + lightens them on a light one, from one expression. At full strength eight of + these is a row of paint chips. + The strengths come from the track's own custom properties, so hovering the bar + deepens every segment at once without any of them having to know the tone. */ + ...paint(tone), + flexGrow: grow, + flexBasis: 0, + /* One crashed actor in 410,110 is 0.0002% of the width: without a floor it is not + a pixel, let alone something to point at — and it is the most important thing + on the bar. */ + minWidth: 6, + height: 18, + // Only the two ends are rounded, so the row reads as one bar rather than as a + // line of separate lozenges. + borderRadius: `${first ? 4 : 0}px ${last ? 4 : 0}px ${last ? 4 : 0}px ${first ? 4 : 0}px`, + boxSizing: "border-box", + transition: "background 120ms, border-color 120ms", + }} + /> + ); + + const track = ( + <div + data-testid={testId} + /* The tooltip needs a pointer, which a screen reader has not got and a keyboard + cannot produce. So the same summary is the bar's own name — colour and hover are + never the only things carrying it. */ + role="img" + aria-label={total === 0 ? emptyText : `${title}. ${summary}`} + css={{ + display: "flex", + gap: 3, + minHeight: 18, + /* Hover only: pointing at the bar reveals the breakdown, but nothing happens on + press, and an active state would promise that it does. + Deepening the mix rather than brightening it: `brightness` on a fill that is + mostly page colour washes it out to the page instead of strengthening it, which + on a light theme reads as the segments going transparent. */ + ":hover": { "--seg-fill": dark ? "30%" : "22%" }, + }} + > + {present + .flatMap((entry) => { + const tone = statusTone(entry.status); + return perActor + ? Array.from({ length: entry.count }, (_, i) => ({ tone, key: `${entry.status}-${i}`, grow: 1 })) + : [{ tone, key: entry.status, grow: entry.count }]; + }) + .map((part, index, all) => + segment(part.tone, part.key, part.grow, index === 0, index === all.length - 1), + )} + </div> + ); + + /* + * The legend, in the bar's own order and colours. + * + * The bar says the proportions and the legend says the numbers; between them a reader + * gets both without hovering anything, which is what a tooltip alone cannot give + * someone reading a screenshot or printing the page. + */ + const keys = order( + [...new Set([...vocabulary.map(humanizeEnum), ...merged.keys()])].map((status) => ({ + status, + count: merged.get(status) ?? 0, + })), + ); + + const legend = ( + <div + data-testid={`${testId}-legend`} + css={{ display: "flex", flexWrap: "wrap", gap: "2px 4px", marginTop: 8 }} + > + {keys.map((entry) => ( + <span + key={entry.status} + css={{ + display: "inline-flex", + alignItems: "center", + gap: 6, + fontSize: 12, + /* The padding and the radius are the same whether or not anything holds this + status, and only the fill changes: a highlight that added weight or space + would move every key beside it each time a count crossed zero, on a page + that polls. */ + padding: "2px 8px", + borderRadius: 6, + // A key is something to read past, not text to drag through: selecting it while + // sweeping the pointer along the row is never what anyone meant. + userSelect: "none", + background: entry.count === 0 ? "transparent" : theme.color.bgElevated, + }} + > + {/* A status nothing is in is still worth listing, and still worth being the + quietest thing here — but the fading is mostly the swatch's job. The text at + the swatch's own opacity measured 3.79:1 on a light page, under AA; at 0.95 + it is 4.64:1 there and 7.20:1 on a dark one, and still visibly the quieter. */} + <span + aria-hidden + css={{ + ...paint(statusTone(entry.status)), + width: 10, + height: 10, + borderRadius: 3, + opacity: entry.count === 0 ? 0.45 : 1, + }} + /> + <Text + css={{ + color: entry.count === 0 ? theme.color.textMuted : theme.color.text, + fontSize: 12, + opacity: entry.count === 0 ? 0.95 : 1, + }} + > + {read(entry)} + </Text> + </span> + ))} + </div> + ); + + return ( + /* The empty row keeps its height, with the reason beneath it. A bar that vanished when + a search stopped matching would move the table under a reader at the moment they + were reading why. */ + <div + css={{ + marginBottom: 6, + /* Declared here rather than on the bar, because the legend keys are painted from + the same expressions and are the bar's siblings: on the track they resolved to + nothing outside it, and every key came out invisible. + + At rest this is the pill's fill exactly; hovering pulls it toward the pill's own + saturated colour, further on a dark page where the same step shows less. */ + "--seg-fill": "0%", + }} + > + {total === 0 ? ( + <> + {track} + <Text + data-testid={`${testId}-empty`} + css={{ color: theme.color.textMuted, fontSize: 12, display: "block", marginTop: 8 }} + > + {emptyText} + </Text> + </> + ) : ( + <Tooltip + title={ + <> + {caption ? <div css={{ opacity: 0.75 }}>{caption}</div> : null} + {present.map((entry) => ( + <div key={entry.status}>{readFull(entry)}</div> + ))} + </> + } + > + {/* The bar and its legend under one tooltip: they are the same reading, and a + breakdown reachable from the chart but not from the key that explains it is + a breakdown half the pointers on the page will miss. */} + <div> + {track} + {legend} + </div> + </Tooltip> + )} + </div> + ); +} + /** * A section's name and how many rows are under it, which is worth knowing before * reading them. @@ -496,7 +845,7 @@ function ServerOrder({ data-testid={testId} css={{ color: theme.color.textMuted, fontSize: 12 }} > - Sorted by the server: {labels[field] ?? field} + Sorted: {labels[field] ?? field} {order === "desc" ? ", descending" : ", ascending"} {age ? ` · ${age}` : ""} </Text> @@ -814,8 +1163,68 @@ export function SubstratePage() { ); const actorRows = actors.error ? [] : (actors.data?.actors ?? []); + + /* + * What the bar above the actor table counts. + * + * Unfiltered it is the summary's own counts, which is the only honest source of a whole + * cluster: the table holds one page, and a page counted and drawn as the cluster would + * report eight actors for a deployment running 410,110. + * + * A search has no server-side breakdown, so the matches are counted here from the rows + * that came back — and those are also a page. `actorBarCaption` is what stops the bar + * claiming the rest: it says how many of the matches are actually in it. + */ + const actorBar = useMemo(() => { + if (!actorFilter) { + return { counts: inventory?.actorStatusCounts ?? [], caption: undefined as string | undefined }; + } + const byStatus = new Map<string, number>(); + for (const actor of actorRows) { + byStatus.set(actor.status, (byStatus.get(actor.status) ?? 0) + 1); + } + const matches = actors.data?.totalSize ?? actorRows.length; + return { + counts: [...byStatus].map(([status, count]) => ({ status, count })), + caption: + actorRows.length < matches + ? `Matching “${actorFilter}”: ${atAGlance(actorRows.length)} of ${atAGlance(matches)} shown` + : `Matching “${actorFilter}”: ${atAGlance(matches)}`, + }; + }, [actorFilter, actorRows, actors.data?.totalSize, inventory?.actorStatusCounts]); const workerRows = workers.error ? [] : (workers.data?.workers ?? []); + /* + * The workers bar: one segment per pod, coloured by what the actor on it is doing. + * + * The worker entry carries the actor's id but not its status, so the status is joined + * here from the actors that came back. Both lists are pages, so a worker whose actor is + * not on the loaded page reads as `Not loaded` rather than being coloured by a guess — + * the join is complete exactly when both fit in one page, which is the common case + * locally and not the case on a large cluster. + * + * ponytail: a client-side join, complete only within a page. The durable fix is the + * controller sending the actor's status on the worker entry, which would remove this. + */ + const workerBar = useMemo(() => { + const statusByActor = new Map(actorRows.map((actor) => [actor.actorId, actor.status])); + const byStatus = new Map<string, number>(); + for (const worker of workerRows) { + const status = !worker.actorId + ? "Idle" + : (statusByActor.get(worker.actorId) ?? "Not loaded"); + byStatus.set(status, (byStatus.get(status) ?? 0) + 1); + } + const counts = [...byStatus].map(([status, count]) => ({ status, count })); + const matches = workers.data?.totalSize ?? workerRows.length; + if (!workerFilter && workerRows.length >= matches) return { counts, caption: undefined }; + const shown = `${atAGlance(workerRows.length)} of ${atAGlance(matches)} shown`; + return { + counts, + caption: workerFilter ? `Matching “${workerFilter}”: ${shown}` : shown, + }; + }, [actorRows, workerRows, workerFilter, workers.data?.totalSize]); + /* * The tiles, from the summary's own counts. * @@ -964,7 +1373,7 @@ export function SubstratePage() { /> ), key: "actorId", - width: 320, + width: 300, render: (_, actor) => <span css={mono}>{actor.actorId}</span>, }, { @@ -977,9 +1386,10 @@ export function SubstratePage() { /> ), key: "status", - // Wide enough for the longest status seen on a real cluster (`Snapshotting`) - // without wrapping it to three lines. - width: 190, + // Wide enough for the longest status a controller reports (`Snapshotting`). It was + // 190 while `ACTOR_STATE_CRASHED` could reach the page; the words are shorter than + // the constants were, and the columns no longer overflow their card because of it. + width: 130, render: (_, actor) => <StatusChip label={actor.status} />, }, { @@ -992,7 +1402,7 @@ export function SubstratePage() { /> ), key: "template", - width: 260, + width: 240, render: (_, actor) => actor.actorTemplateName ? qualified(actor.actorTemplateNamespace, actor.actorTemplateName) @@ -1008,10 +1418,13 @@ export function SubstratePage() { /> ), key: "pod", - width: 320, + width: 260, render: (_, actor) => actor.ateomPodName ? ( - <Text css={{ ...mono, ...muted }}> + /* One line, always. A pod name and an IP together outrun the column, and + wrapping them made the row two lines tall — which moves every row under it, + on a page that polls. It runs into the slack on its right instead. */ + <Text css={{ ...mono, ...muted, whiteSpace: "nowrap" }}> {actor.ateomPodNamespace ?? ""}/{actor.ateomPodName} {actor.ateomPodIp ? ` · ${actor.ateomPodIp}` : ""} </Text> @@ -1305,19 +1718,6 @@ export function SubstratePage() { /> </div> - {/* Every actor status, not only the running tally. - Knowing that 1 of 410,110 actors is running says nothing about the other - 410,109 — and on this cluster what it does not say is that most of them - have crashed. The summary counts them all, so the page can. */} - {inventory && inventory.actorStatusCounts.length > 1 ? ( - <Space size={6} wrap data-testid="substrate-actor-status-counts"> - <Text css={{ ...muted, fontSize: 12 }}>Actors by status</Text> - {inventory.actorStatusCounts.map((entry) => ( - <StatusChip key={entry.status} label={entry.status} count={entry.count} /> - ))} - </Space> - ) : null} - <Card title={ <SectionTitle @@ -1428,6 +1828,22 @@ export function SubstratePage() { /> ) : null} + <StatusBar + testId="substrate-actor-status-counts" + title="Actor status" + vocabulary={ACTOR_STATES} + noun="Actors" + counts={actorBar.counts} + caption={actorBar.caption} + emptyText={ + actorFilter + ? "No actors match your search." + : ateApiEnabled + ? "No actors in this scope." + : "ate-api is not configured, so there are no actors to show." + } + /> + <Table<SubstrateActorEntry> data-testid="substrate-actors-table" rowKey={(actor) => actor.actorId} @@ -1436,7 +1852,9 @@ export function SubstratePage() { loading={actors.isLoading} pagination={false} virtual - scroll={{ y: GROWING_TABLE_HEIGHT, x: 1040 }} + /* The sum of the column widths, so the table asks for exactly what it uses: + a wider `x` reserves space no column wants and scrolls the card for it. */ + scroll={{ y: GROWING_TABLE_HEIGHT, x: 930 }} size="small" /* Three different sentences, because they are three different facts and only one is something to act on: a controller with no ate-api endpoint @@ -1514,6 +1932,22 @@ export function SubstratePage() { /> ) : null} + <StatusBar + testId="substrate-worker-status-counts" + title="Worker status" + vocabulary={WORKER_STATES} + noun="Workers" + counts={workerBar.counts} + caption={workerBar.caption} + emptyText={ + workerFilter + ? "No workers match your search." + : ateApiEnabled + ? "No workers in this scope." + : "ate-api is not configured, so there are no workers to show." + } + /> + <Table<SubstrateWorkerEntry> data-testid="substrate-workers-table" rowKey={(worker) => diff --git a/ui/src/theme/theme.ts b/ui/src/theme/theme.ts index 657c2c761..4ba2dcec9 100644 --- a/ui/src/theme/theme.ts +++ b/ui/src/theme/theme.ts @@ -59,13 +59,13 @@ const darkColor = { * filled badge rather than the quiet pill the rest of the page uses. */ successBg: "#0c2c18", - successBorder: "#166534", + successBorder: "#218045", successText: "#4ade80", warningBg: "#33240a", - warningBorder: "#92400e", + warningBorder: "#a95c13", warningText: "#fbbf24", dangerBg: "#3a1417", - dangerBorder: "#991b1b", + dangerBorder: "#be3d3d", dangerText: "#f87171", /** * The brand colour as *foreground* text on the page. @@ -86,7 +86,7 @@ const darkColor = { * 4.2:1 and 3.4:1, both under the 4.5 that small text needs. */ infoBg: "#101c33", - infoBorder: "#1e3a8a", + infoBorder: "#4366af", infoText: "#93c5fd", accentBg: "#1e152e", accentBorder: "#5b21b6", @@ -125,19 +125,19 @@ const lightColor: Record<keyof typeof darkColor, string> = { // inverted. What was there before came out muddy: antd tinted its mid-green // towards a light base and landed on something between the two. successBg: "#dcfce7", - successBorder: "#86efac", + successBorder: "#449e65", successText: "#166534", warningBg: "#fef3c7", - warningBorder: "#fcd34d", + warningBorder: "#bf7e28", warningText: "#92400e", dangerBg: "#fee2e2", - dangerBorder: "#fca5a5", + dangerBorder: "#ce6666", dangerText: "#991b1b", // On a white page the brand purple is already a readable foreground, so this is // `primary`. The token exists so a component need not know which theme it is on. primaryText: "#6d28d9", infoBg: "#eff6ff", - infoBorder: "#bfdbfe", + infoBorder: "#638be8", infoText: "#1d4ed8", accentBg: "#f5f3ff", accentBorder: "#ddd6fe", From 732fa7e49a5c761ff7170cc555f29215ae0c9aea Mon Sep 17 00:00:00 2001 From: Nicholas Bucher <behappy54321@gmail.com> Date: Fri, 4 Sep 2026 15:17:35 -0400 Subject: [PATCH 4/8] fix: say which actor each substrate worker is holding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ateapi.Worker` carries no actor reference — the binding is recorded on the actor, in `ActorStatus.worker_assignment` — so every worker came back with an empty actor and the workers table reported an idle pod for each one, whatever was running on it. Introduced in #2697, which dropped the fields when v0.0.25 removed the singular assignment from `WorkerStatus` and put nothing in their place. Joined in the service, where both whole lists are in hand: the reads that reach a browser are pages, and a join across two pages matches only where they overlap. A worker holds several actors under v0.0.25 and `SubstrateWorker` has room for one, so the lowest actor id on the pod wins. Lowest rather than first seen because ate-api returns actors unordered — by arrival the cell would name a different actor on each poll. Carrying the whole set needs a repeated field in system.proto. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Nicholas Bucher <behappy54321@gmail.com> --- go/core/internal/service/system/service.go | 42 +++++++++++++++++++ .../internal/service/system/service_test.go | 31 +++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/go/core/internal/service/system/service.go b/go/core/internal/service/system/service.go index b7d578e5f..335c808e4 100644 --- a/go/core/internal/service/system/service.go +++ b/go/core/internal/service/system/service.go @@ -397,6 +397,7 @@ func (s *Service) listATEState(ctx context.Context, namespaces []string) ([]Subs } workers = append(workers, workerFromProto(worker)) } + placeActorsOnWorkers(workers, actors) return templates, actors, workers, nil } @@ -436,6 +437,47 @@ func workerFromProto(worker *ateapipb.Worker) SubstrateWorker { } } +// placeActorsOnWorkers fills in what each worker is holding. +// +// The binding is recorded on the actor — `ActorStatus.worker_assignment` names the pod — +// and nowhere on the worker: `ateapi.Worker` carries no actor reference at all, so +// `workerFromProto` above has nothing to read and every worker came back holding nothing. +// A page that reads an absent actor as an idle pod then reports the whole fleet idle +// while it is running, which is what it did. +// +// Joined here rather than in the caller, because this is the one place both whole lists +// are in hand: the reads that reach a browser are pages, and a join across two pages +// matches only where they happen to overlap. +// +// ponytail: a worker can hold several actors since v0.0.25, and `SubstrateWorker` has one +// actor's worth of fields, so this reports the lowest actor id on the pod and says nothing +// about the rest. Lowest rather than first seen because ate-api returns actors in no +// particular order — picked by arrival, the cell would name a different actor on each poll. +// Carrying the whole set needs a repeated field on `SubstrateWorker` in system.proto. +func placeActorsOnWorkers(workers []SubstrateWorker, actors []SubstrateActor) { + type pod struct{ namespace, name string } + holding := make(map[pod]SubstrateActor, len(actors)) + for _, actor := range actors { + if actor.AteomPodName == "" { + continue + } + key := pod{actor.AteomPodNamespace, actor.AteomPodName} + if held, ok := holding[key]; ok && held.ActorID <= actor.ActorID { + continue + } + holding[key] = actor + } + for i := range workers { + actor, ok := holding[pod{workers[i].WorkerNamespace, workers[i].WorkerPod}] + if !ok { + continue + } + workers[i].ActorNamespace = actor.ActorTemplateNamespace + workers[i].ActorTemplate = actor.ActorTemplateName + workers[i].ActorID = actor.ActorID + } +} + func labelSelectorString(ctx context.Context, selector *metav1.LabelSelector) string { if selector == nil { return "" diff --git a/go/core/internal/service/system/service_test.go b/go/core/internal/service/system/service_test.go index b20439363..036258e20 100644 --- a/go/core/internal/service/system/service_test.go +++ b/go/core/internal/service/system/service_test.go @@ -146,10 +146,33 @@ func TestGetSubstrateStatus(t *testing.T) { }}, }}, actors: []*ateapipb.Actor{{ + // Two actors on one pod, because workers hold several since v0.0.25, and + // ate-api returns them in no particular order. This one is later in the + // alphabet and arrives first: a join that kept whichever it saw first + // would report it here and change its mind on the next read. + Metadata: &ateapipb.ResourceMetadata{Name: "actor-2"}, + ActorTemplate: &ateapipb.ObjectRef{Atespace: "team", Name: "template"}, + Status: &ateapipb.ActorStatus{ + State: ateapipb.ActorState_ACTOR_STATE_RUNNING, + WorkerAssignment: &ateapipb.WorkerAssignment{ + WorkerNamespace: "team", + WorkerPool: "pool", + WorkerPod: "worker-0", + }, + }, + }, { Metadata: &ateapipb.ResourceMetadata{Name: "actor-1"}, ActorTemplate: &ateapipb.ObjectRef{Atespace: "team", Name: "template"}, Status: &ateapipb.ActorStatus{ State: ateapipb.ActorState_ACTOR_STATE_RUNNING, + // Where this actor is placed. `ateapi.Worker` carries no actor + // reference, so this is the only record of the binding. + WorkerAssignment: &ateapipb.WorkerAssignment{ + WorkerNamespace: "team", + WorkerPool: "pool", + WorkerPod: "worker-0", + WorkerPodIp: "10.42.0.7", + }, }, }}, workers: []*ateapipb.Worker{{ @@ -176,10 +199,16 @@ func TestGetSubstrateStatus(t *testing.T) { assert.Equal(t, "gvisor", result.ActorTemplates[0].SandboxClass) assert.Equal(t, "kagent", result.ActorTemplates[0].HarnessName) assert.True(t, result.ActorTemplates[0].ManagedByKagent) - require.Len(t, result.Actors, 1) + require.Len(t, result.Actors, 2) assert.Equal(t, "Running", result.Actors[0].Status) require.Len(t, result.Workers, 1) assert.Equal(t, "worker-0", result.Workers[0].WorkerPod) + // The worker says what is on it. Read from the actor's own assignment, because + // the worker message has no field for it — without the join every pod reports + // holding nothing and a running fleet reads as an idle one. + assert.Equal(t, "actor-1", result.Workers[0].ActorID) + assert.Equal(t, "template", result.Workers[0].ActorTemplate) + assert.Equal(t, "team", result.Workers[0].ActorNamespace) assert.Equal(t, int64(3), result.Workers[0].Version) }) From e848172b8e5e84e9ff03ff524afce9fefc7b448e Mon Sep 17 00:00:00 2001 From: Nicholas Bucher <behappy54321@gmail.com> Date: Fri, 4 Sep 2026 16:09:45 -0400 Subject: [PATCH 5/8] chore: trim the worker-join comments Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Nicholas Bucher <behappy54321@gmail.com> --- go/core/internal/service/system/service.go | 19 +++++-------------- ui/src/pages/SubstratePage.tsx | 4 ++-- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/go/core/internal/service/system/service.go b/go/core/internal/service/system/service.go index 8ea7c3f0e..f0a173ef0 100644 --- a/go/core/internal/service/system/service.go +++ b/go/core/internal/service/system/service.go @@ -439,21 +439,12 @@ func workerFromProto(worker *ateapipb.Worker) SubstrateWorker { // placeActorsOnWorkers fills in what each worker is holding. // -// The binding is recorded on the actor — `ActorStatus.worker_assignment` names the pod — -// and nowhere on the worker: `ateapi.Worker` carries no actor reference at all, so -// `workerFromProto` above has nothing to read and every worker came back holding nothing. -// A page that reads an absent actor as an idle pod then reports the whole fleet idle -// while it is running, which is what it did. +// `ateapi.Worker` carries no actor reference — only the actor names its pod — so this is +// the one place both whole lists are in hand to join them. // -// Joined here rather than in the caller, because this is the one place both whole lists -// are in hand: the reads that reach a browser are pages, and a join across two pages -// matches only where they happen to overlap. -// -// ponytail: a worker can hold several actors since v0.0.25, and `SubstrateWorker` has one -// actor's worth of fields, so this reports the lowest actor id on the pod and says nothing -// about the rest. Lowest rather than first seen because ate-api returns actors in no -// particular order — picked by arrival, the cell would name a different actor on each poll. -// Carrying the whole set needs a repeated field on `SubstrateWorker` in system.proto. +// A worker holds several actors since v0.0.25 and `SubstrateWorker` has room for one, so +// the lowest actor id wins. Lowest rather than first seen: ate-api returns actors +// unordered, so by arrival the cell would name a different actor on each poll. func placeActorsOnWorkers(workers []SubstrateWorker, actors []SubstrateActor) { type pod struct{ namespace, name string } holding := make(map[pod]SubstrateActor, len(actors)) diff --git a/ui/src/pages/SubstratePage.tsx b/ui/src/pages/SubstratePage.tsx index e0ea40611..e6d17680f 100644 --- a/ui/src/pages/SubstratePage.tsx +++ b/ui/src/pages/SubstratePage.tsx @@ -1203,8 +1203,8 @@ export function SubstratePage() { * the join is complete exactly when both fit in one page, which is the common case * locally and not the case on a large cluster. * - * ponytail: a client-side join, complete only within a page. The durable fix is the - * controller sending the actor's status on the worker entry, which would remove this. + * A client-side join, complete only within a page. The durable fix is the controller + * sending the actor's status on the worker entry, which would remove this. */ const workerBar = useMemo(() => { const statusByActor = new Map(actorRows.map((actor) => [actor.actorId, actor.status])); From ffe57a7c0448032c2dbf5b75f9ac04e9043be93a Mon Sep 17 00:00:00 2001 From: Nicholas Bucher <behappy54321@gmail.com> Date: Fri, 4 Sep 2026 16:22:14 -0400 Subject: [PATCH 6/8] fix: send each worker's actor status instead of joining it in the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workers bar coloured its segments from the loaded actor page, so searching, sorting or paging the *actors* table recoloured the *workers* one — a pod whose actor fell off the page read as `Not loaded`, a status no cluster reports. The controller has both whole lists when it builds the response, so it carries the status on the worker entry and the page joins nothing. Also from review: - Every actor is offered to the join, not only those the atespace scope kept. A worker running an actor whose template lives elsewhere reported itself idle. - A live actor on a shared pod wins over a parked one; the lowest id breaks ties. - A failed read no longer renders as "No actors in this scope", and the legend stays when a scope empties so the table beneath it does not jump. - The per-actor ceiling is 80, which is the widest row that fits the card. - An extension's PNG favicon is no longer served under an SVG type, and the created link carries the marker so a second call retargets it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Nicholas Bucher <behappy54321@gmail.com> --- go/api/gen/kagent/api/v1alpha1/system.pb.go | 19 ++++-- go/core/internal/grpcserver/system.go | 1 + go/core/internal/service/system/service.go | 36 ++++++++++-- .../internal/service/system/service_test.go | 15 ++++- proto/kagent/api/v1alpha1/system.proto | 3 + ui/src/api/domain/substrate.ts | 2 + ui/src/api/grpc/operations.ts | 1 + ui/src/appExtensions/appExtensions.test.ts | 14 ++++- ui/src/appExtensions/branding.ts | 16 +++-- .../kagent/api/v1alpha1/system_pb.ts | 10 +++- ui/src/mocks/fixtures.ts | 3 + ui/src/mocks/transport.ts | 1 + ui/src/pages/SubstratePage.tsx | 58 ++++++++++++------- 13 files changed, 137 insertions(+), 42 deletions(-) diff --git a/go/api/gen/kagent/api/v1alpha1/system.pb.go b/go/api/gen/kagent/api/v1alpha1/system.pb.go index ba2d169ff..5e88449be 100644 --- a/go/api/gen/kagent/api/v1alpha1/system.pb.go +++ b/go/api/gen/kagent/api/v1alpha1/system.pb.go @@ -776,8 +776,11 @@ type SubstrateWorker struct { ActorId string `protobuf:"bytes,6,opt,name=actor_id,json=actorId,proto3" json:"actor_id,omitempty"` Ip string `protobuf:"bytes,7,opt,name=ip,proto3" json:"ip,omitempty"` Version int64 `protobuf:"varint,8,opt,name=version,proto3" json:"version,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // What the actor on this pod is doing, so a reader of the worker list is not + // sent to the actor list to find out. Empty when the pod holds nothing. + ActorStatus string `protobuf:"bytes,9,opt,name=actor_status,json=actorStatus,proto3" json:"actor_status,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *SubstrateWorker) Reset() { @@ -866,6 +869,13 @@ func (x *SubstrateWorker) GetVersion() int64 { return 0 } +func (x *SubstrateWorker) GetActorStatus() string { + if x != nil { + return x.ActorStatus + } + return "" +} + var File_kagent_api_v1alpha1_system_proto protoreflect.FileDescriptor const file_kagent_api_v1alpha1_system_proto_rawDesc = "" + @@ -928,7 +938,7 @@ const file_kagent_api_v1alpha1_system_proto_rawDesc = "" + "\x10worker_pool_name\x18\n" + " \x01(\tR\x0eworkerPoolName\x120\n" + "\x14in_progress_snapshot\x18\v \x01(\tR\x12inProgressSnapshot\x12\x18\n" + - "\aversion\x18\f \x01(\x03R\aversion\"\x91\x02\n" + + "\aversion\x18\f \x01(\x03R\aversion\"\xb4\x02\n" + "\x0fSubstrateWorker\x12)\n" + "\x10worker_namespace\x18\x01 \x01(\tR\x0fworkerNamespace\x12\x1f\n" + "\vworker_pool\x18\x02 \x01(\tR\n" + @@ -939,7 +949,8 @@ const file_kagent_api_v1alpha1_system_proto_rawDesc = "" + "\x0eactor_template\x18\x05 \x01(\tR\ractorTemplate\x12\x19\n" + "\bactor_id\x18\x06 \x01(\tR\aactorId\x12\x0e\n" + "\x02ip\x18\a \x01(\tR\x02ip\x12\x18\n" + - "\aversion\x18\b \x01(\x03R\aversion2\xbb\x03\n" + + "\aversion\x18\b \x01(\x03R\aversion\x12!\n" + + "\factor_status\x18\t \x01(\tR\vactorStatus2\xbb\x03\n" + "\rSystemService\x12]\n" + "\n" + "GetVersion\x12&.kagent.api.v1alpha1.GetVersionRequest\x1a'.kagent.api.v1alpha1.GetVersionResponse\x12i\n" + diff --git a/go/core/internal/grpcserver/system.go b/go/core/internal/grpcserver/system.go index 32c42222a..b8ae7532e 100644 --- a/go/core/internal/grpcserver/system.go +++ b/go/core/internal/grpcserver/system.go @@ -112,6 +112,7 @@ func (s *systemServer) GetSubstrateStatus(ctx context.Context, request *apiv1alp ActorNamespace: worker.ActorNamespace, ActorTemplate: worker.ActorTemplate, ActorId: worker.ActorID, + ActorStatus: worker.ActorStatus, Ip: worker.IP, Version: worker.Version, }) diff --git a/go/core/internal/service/system/service.go b/go/core/internal/service/system/service.go index f0a173ef0..5fa0a4ba9 100644 --- a/go/core/internal/service/system/service.go +++ b/go/core/internal/service/system/service.go @@ -101,6 +101,7 @@ type SubstrateWorker struct { ActorNamespace string ActorTemplate string ActorID string + ActorStatus string IP string Version int64 } @@ -374,14 +375,20 @@ func (s *Service) listATEState(ctx context.Context, namespaces []string) ([]Subs } actors := make([]SubstrateActor, 0, len(actorsFromAPI)) + // Every actor, scope or no scope. The workers below are filtered by *their* namespace + // and an actor by its template's, so a worker running an actor whose template lives + // elsewhere finds no match in the scoped list and reports itself idle. + placed := make([]SubstrateActor, 0, len(actorsFromAPI)) for _, actor := range actorsFromAPI { if actor == nil { continue } + converted := actorFromProto(actor) + placed = append(placed, converted) if !allowedAtespace(actor.GetActorTemplate().GetAtespace(), allowAll, allowed) { continue } - actors = append(actors, actorFromProto(actor)) + actors = append(actors, converted) } workers := make([]SubstrateWorker, 0, len(workersFromAPI)) @@ -397,7 +404,7 @@ func (s *Service) listATEState(ctx context.Context, namespaces []string) ([]Subs } workers = append(workers, workerFromProto(worker)) } - placeActorsOnWorkers(workers, actors) + placeActorsOnWorkers(workers, placed) return templates, actors, workers, nil } @@ -443,8 +450,26 @@ func workerFromProto(worker *ateapipb.Worker) SubstrateWorker { // the one place both whole lists are in hand to join them. // // A worker holds several actors since v0.0.25 and `SubstrateWorker` has room for one, so -// the lowest actor id wins. Lowest rather than first seen: ate-api returns actors -// unordered, so by arrival the cell would name a different actor on each poll. +// a live actor wins over a parked one and the lowest id breaks the tie. Never first seen: +// ate-api returns actors unordered, so the cell would name a different actor on each poll. +// supersedes reports whether one actor on a pod should be named over another: a running +// one over a parked one, and otherwise the lower id, so the answer does not depend on the +// order ate-api happened to return them in. +func supersedes(candidate, held SubstrateActor) bool { + if parkedActor(candidate) != parkedActor(held) { + return parkedActor(held) + } + return candidate.ActorID < held.ActorID +} + +func parkedActor(actor SubstrateActor) bool { + switch actor.Status { + case "Suspended", "Paused", "Unknown", "": + return true + } + return false +} + func placeActorsOnWorkers(workers []SubstrateWorker, actors []SubstrateActor) { type pod struct{ namespace, name string } holding := make(map[pod]SubstrateActor, len(actors)) @@ -453,7 +478,7 @@ func placeActorsOnWorkers(workers []SubstrateWorker, actors []SubstrateActor) { continue } key := pod{actor.AteomPodNamespace, actor.AteomPodName} - if held, ok := holding[key]; ok && held.ActorID <= actor.ActorID { + if held, ok := holding[key]; ok && !supersedes(actor, held) { continue } holding[key] = actor @@ -466,6 +491,7 @@ func placeActorsOnWorkers(workers []SubstrateWorker, actors []SubstrateActor) { workers[i].ActorNamespace = actor.ActorTemplateNamespace workers[i].ActorTemplate = actor.ActorTemplateName workers[i].ActorID = actor.ActorID + workers[i].ActorStatus = actor.Status } } diff --git a/go/core/internal/service/system/service_test.go b/go/core/internal/service/system/service_test.go index 036258e20..16750a423 100644 --- a/go/core/internal/service/system/service_test.go +++ b/go/core/internal/service/system/service_test.go @@ -164,7 +164,9 @@ func TestGetSubstrateStatus(t *testing.T) { Metadata: &ateapipb.ResourceMetadata{Name: "actor-1"}, ActorTemplate: &ateapipb.ObjectRef{Atespace: "team", Name: "template"}, Status: &ateapipb.ActorStatus{ - State: ateapipb.ActorState_ACTOR_STATE_RUNNING, + // Parked, and the lower id: the pod must still be reported as running + // actor-2, because a suspended actor says nothing about a busy pod. + State: ateapipb.ActorState_ACTOR_STATE_SUSPENDED, // Where this actor is placed. `ateapi.Worker` carries no actor // reference, so this is the only record of the binding. WorkerAssignment: &ateapipb.WorkerAssignment{ @@ -200,13 +202,20 @@ func TestGetSubstrateStatus(t *testing.T) { assert.Equal(t, "kagent", result.ActorTemplates[0].HarnessName) assert.True(t, result.ActorTemplates[0].ManagedByKagent) require.Len(t, result.Actors, 2) - assert.Equal(t, "Running", result.Actors[0].Status) + // By status rather than by position: the fixture now holds one of each, and an + // index here would assert the sort order under the guise of asserting the label. + actorStatuses := map[string]string{} + for _, actor := range result.Actors { + actorStatuses[actor.ActorID] = actor.Status + } + assert.Equal(t, map[string]string{"actor-1": "Suspended", "actor-2": "Running"}, actorStatuses) require.Len(t, result.Workers, 1) assert.Equal(t, "worker-0", result.Workers[0].WorkerPod) // The worker says what is on it. Read from the actor's own assignment, because // the worker message has no field for it — without the join every pod reports // holding nothing and a running fleet reads as an idle one. - assert.Equal(t, "actor-1", result.Workers[0].ActorID) + assert.Equal(t, "actor-2", result.Workers[0].ActorID) + assert.Equal(t, "Running", result.Workers[0].ActorStatus) assert.Equal(t, "template", result.Workers[0].ActorTemplate) assert.Equal(t, "team", result.Workers[0].ActorNamespace) assert.Equal(t, int64(3), result.Workers[0].Version) diff --git a/proto/kagent/api/v1alpha1/system.proto b/proto/kagent/api/v1alpha1/system.proto index 5bc3ef780..63d8f323f 100644 --- a/proto/kagent/api/v1alpha1/system.proto +++ b/proto/kagent/api/v1alpha1/system.proto @@ -94,4 +94,7 @@ message SubstrateWorker { string actor_id = 6; string ip = 7; int64 version = 8; + // What the actor on this pod is doing, so a reader of the worker list is not + // sent to the actor list to find out. Empty when the pod holds nothing. + string actor_status = 9; } diff --git a/ui/src/api/domain/substrate.ts b/ui/src/api/domain/substrate.ts index 7207be4c6..12d483b0f 100644 --- a/ui/src/api/domain/substrate.ts +++ b/ui/src/api/domain/substrate.ts @@ -73,6 +73,8 @@ export interface SubstrateWorkerEntry { actorNamespace?: string; actorTemplate?: string; actorId?: string; + /** What the actor on this pod is doing. Absent when the pod holds nothing. */ + actorStatus?: string; ip?: string; version?: number; } diff --git a/ui/src/api/grpc/operations.ts b/ui/src/api/grpc/operations.ts index 472bc5c80..8bad6eb86 100644 --- a/ui/src/api/grpc/operations.ts +++ b/ui/src/api/grpc/operations.ts @@ -1107,6 +1107,7 @@ function toWorkerEntry(worker: PbSubstrateWorker): SubstrateWorkerEntry { actorNamespace: orUndefined(worker.actorNamespace), actorTemplate: orUndefined(worker.actorTemplate), actorId: orUndefined(worker.actorId), + actorStatus: orUndefined(worker.actorStatus), ip: orUndefined(worker.ip), version: toNumber(worker.version), }; diff --git a/ui/src/appExtensions/appExtensions.test.ts b/ui/src/appExtensions/appExtensions.test.ts index 937abad39..9b75d6ff8 100644 --- a/ui/src/appExtensions/appExtensions.test.ts +++ b/ui/src/appExtensions/appExtensions.test.ts @@ -458,10 +458,18 @@ describe("applyExtensionBranding", () => { document.head.innerHTML = ""; applyExtensionBranding({ faviconUrl: "/my-mark.svg" }); + // Twice: the link it creates carries the marker, so the second call retargets the + // first rather than leaving two icons for the browser to choose between. + applyExtensionBranding({ faviconUrl: "/other-mark.svg" }); - expect( - document.querySelector<HTMLLinkElement>('link[rel="icon"]')?.href, - ).toContain("/my-mark.svg"); + expect(document.querySelectorAll('link[rel="icon"]')).toHaveLength(1); + expect(favicon()?.href).toContain("/other-mark.svg"); + }); + + it("does not advertise SVG for an icon that is not one", () => { + applyExtensionBranding({ faviconUrl: "/my-mark.png" }); + + expect(favicon()?.getAttribute("type")).toBe(""); }); }); diff --git a/ui/src/appExtensions/branding.ts b/ui/src/appExtensions/branding.ts index be0fd8ba4..904260bc6 100644 --- a/ui/src/appExtensions/branding.ts +++ b/ui/src/appExtensions/branding.ts @@ -50,9 +50,17 @@ export function applyExtensionBranding(branding: ExtensionBranding | undefined): if (!branding?.faviconUrl) return; // The tag `index.html` ships, or a fresh one if a host page dropped it — a missing - // icon is not a reason to leave the extension's branding unapplied. - const link = - document.querySelector<HTMLLinkElement>("link[data-app-favicon]") ?? - document.head.appendChild(Object.assign(document.createElement("link"), { rel: "icon" })); + // icon is not a reason to leave the extension's branding unapplied. The new one carries + // the marker too, so a second call retargets this link instead of appending another. + let link = document.querySelector<HTMLLinkElement>("link[data-app-favicon]"); + if (!link) { + link = document.createElement("link"); + link.rel = "icon"; + link.dataset.appFavicon = ""; + document.head.appendChild(link); + } link.href = branding.faviconUrl; + // `index.html` declares SVG. Left alone, a PNG would be served under a type that says + // otherwise, which browsers are entitled to act on when choosing between icons. + link.type = branding.faviconUrl.endsWith(".svg") ? "image/svg+xml" : ""; } diff --git a/ui/src/generated/kagent/api/v1alpha1/system_pb.ts b/ui/src/generated/kagent/api/v1alpha1/system_pb.ts index 84f234834..e244c9ccc 100644 --- a/ui/src/generated/kagent/api/v1alpha1/system_pb.ts +++ b/ui/src/generated/kagent/api/v1alpha1/system_pb.ts @@ -11,7 +11,7 @@ import type { JsonObject, Message } from "@bufbuild/protobuf"; * Describes the file kagent/api/v1alpha1/system.proto. */ export const file_kagent_api_v1alpha1_system: GenFile = /*@__PURE__*/ - fileDesc("CiBrYWdlbnQvYXBpL3YxYWxwaGExL3N5c3RlbS5wcm90bxITa2FnZW50LmFwaS52MWFscGhhMSITChFHZXRWZXJzaW9uUmVxdWVzdCJUChJHZXRWZXJzaW9uUmVzcG9uc2USFgoOa2FnZW50X3ZlcnNpb24YASABKAkSEgoKZ2l0X2NvbW1pdBgCIAEoCRISCgpidWlsZF9kYXRlGAMgASgJIhcKFUdldEN1cnJlbnRVc2VyUmVxdWVzdCJBChZHZXRDdXJyZW50VXNlclJlc3BvbnNlEicKBmNsYWltcxgBIAEoCzIXLmdvb2dsZS5wcm90b2J1Zi5TdHJ1Y3QiFwoVTGlzdE5hbWVzcGFjZXNSZXF1ZXN0IikKCU5hbWVzcGFjZRIMCgRuYW1lGAEgASgJEg4KBnN0YXR1cxgCIAEoCSJMChZMaXN0TmFtZXNwYWNlc1Jlc3BvbnNlEjIKCm5hbWVzcGFjZXMYASADKAsyHi5rYWdlbnQuYXBpLnYxYWxwaGExLk5hbWVzcGFjZSIuChlHZXRTdWJzdHJhdGVTdGF0dXNSZXF1ZXN0EhEKCW5hbWVzcGFjZRgBIAEoCSK2AgoaR2V0U3Vic3RyYXRlU3RhdHVzUmVzcG9uc2USDwoHZW5hYmxlZBgBIAEoCBIVCg1hdGVfYXBpX2Vycm9yGAIgASgJEj4KDHdvcmtlcl9wb29scxgDIAMoCzIoLmthZ2VudC5hcGkudjFhbHBoYTEuU3Vic3RyYXRlV29ya2VyUG9vbBJECg9hY3Rvcl90ZW1wbGF0ZXMYBCADKAsyKy5rYWdlbnQuYXBpLnYxYWxwaGExLlN1YnN0cmF0ZUFjdG9yVGVtcGxhdGUSMwoGYWN0b3JzGAUgAygLMiMua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVBY3RvchI1Cgd3b3JrZXJzGAYgAygLMiQua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVXb3JrZXIiXQoTU3Vic3RyYXRlV29ya2VyUG9vbBIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRIQCghyZXBsaWNhcxgDIAEoBRITCgthdGVvbV9pbWFnZRgEIAEoCSLbAQoWU3Vic3RyYXRlQWN0b3JUZW1wbGF0ZRIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRINCgVwaGFzZRgDIAEoCRIXCg9nb2xkZW5fYWN0b3JfaWQYBCABKAkSFwoPZ29sZGVuX3NuYXBzaG90GAUgASgJEhUKDXNhbmRib3hfY2xhc3MYBiABKAkSFwoPd29ya2VyX3NlbGVjdG9yGAcgASgJEhQKDGhhcm5lc3NfbmFtZRgIIAEoCRIZChFtYW5hZ2VkX2J5X2thZ2VudBgJIAEoCCKwAgoOU3Vic3RyYXRlQWN0b3ISEAoIYWN0b3JfaWQYASABKAkSEAoIYXRlc3BhY2UYAiABKAkSDgoGc3RhdHVzGAMgASgJEiAKGGFjdG9yX3RlbXBsYXRlX25hbWVzcGFjZRgEIAEoCRIbChNhY3Rvcl90ZW1wbGF0ZV9uYW1lGAUgASgJEhsKE2F0ZW9tX3BvZF9uYW1lc3BhY2UYBiABKAkSFgoOYXRlb21fcG9kX25hbWUYByABKAkSFAoMYXRlb21fcG9kX2lwGAggASgJEhcKD2xhdGVzdF9zbmFwc2hvdBgJIAEoCRIYChB3b3JrZXJfcG9vbF9uYW1lGAogASgJEhwKFGluX3Byb2dyZXNzX3NuYXBzaG90GAsgASgJEg8KB3ZlcnNpb24YDCABKAMitAEKD1N1YnN0cmF0ZVdvcmtlchIYChB3b3JrZXJfbmFtZXNwYWNlGAEgASgJEhMKC3dvcmtlcl9wb29sGAIgASgJEhIKCndvcmtlcl9wb2QYAyABKAkSFwoPYWN0b3JfbmFtZXNwYWNlGAQgASgJEhYKDmFjdG9yX3RlbXBsYXRlGAUgASgJEhAKCGFjdG9yX2lkGAYgASgJEgoKAmlwGAcgASgJEg8KB3ZlcnNpb24YCCABKAMyuwMKDVN5c3RlbVNlcnZpY2USXQoKR2V0VmVyc2lvbhImLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0VmVyc2lvblJlcXVlc3QaJy5rYWdlbnQuYXBpLnYxYWxwaGExLkdldFZlcnNpb25SZXNwb25zZRJpCg5HZXRDdXJyZW50VXNlchIqLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0Q3VycmVudFVzZXJSZXF1ZXN0Gisua2FnZW50LmFwaS52MWFscGhhMS5HZXRDdXJyZW50VXNlclJlc3BvbnNlEmkKDkxpc3ROYW1lc3BhY2VzEioua2FnZW50LmFwaS52MWFscGhhMS5MaXN0TmFtZXNwYWNlc1JlcXVlc3QaKy5rYWdlbnQuYXBpLnYxYWxwaGExLkxpc3ROYW1lc3BhY2VzUmVzcG9uc2USdQoSR2V0U3Vic3RyYXRlU3RhdHVzEi4ua2FnZW50LmFwaS52MWFscGhhMS5HZXRTdWJzdHJhdGVTdGF0dXNSZXF1ZXN0Gi8ua2FnZW50LmFwaS52MWFscGhhMS5HZXRTdWJzdHJhdGVTdGF0dXNSZXNwb25zZUJJWkdnaXRodWIuY29tL2thZ2VudC1kZXYva2FnZW50L2dvL2FwaS9nZW4va2FnZW50L2FwaS92MWFscGhhMTthcGl2MWFscGhhMWIGcHJvdG8z", [file_google_protobuf_struct]); + fileDesc("CiBrYWdlbnQvYXBpL3YxYWxwaGExL3N5c3RlbS5wcm90bxITa2FnZW50LmFwaS52MWFscGhhMSITChFHZXRWZXJzaW9uUmVxdWVzdCJUChJHZXRWZXJzaW9uUmVzcG9uc2USFgoOa2FnZW50X3ZlcnNpb24YASABKAkSEgoKZ2l0X2NvbW1pdBgCIAEoCRISCgpidWlsZF9kYXRlGAMgASgJIhcKFUdldEN1cnJlbnRVc2VyUmVxdWVzdCJBChZHZXRDdXJyZW50VXNlclJlc3BvbnNlEicKBmNsYWltcxgBIAEoCzIXLmdvb2dsZS5wcm90b2J1Zi5TdHJ1Y3QiFwoVTGlzdE5hbWVzcGFjZXNSZXF1ZXN0IikKCU5hbWVzcGFjZRIMCgRuYW1lGAEgASgJEg4KBnN0YXR1cxgCIAEoCSJMChZMaXN0TmFtZXNwYWNlc1Jlc3BvbnNlEjIKCm5hbWVzcGFjZXMYASADKAsyHi5rYWdlbnQuYXBpLnYxYWxwaGExLk5hbWVzcGFjZSIuChlHZXRTdWJzdHJhdGVTdGF0dXNSZXF1ZXN0EhEKCW5hbWVzcGFjZRgBIAEoCSK2AgoaR2V0U3Vic3RyYXRlU3RhdHVzUmVzcG9uc2USDwoHZW5hYmxlZBgBIAEoCBIVCg1hdGVfYXBpX2Vycm9yGAIgASgJEj4KDHdvcmtlcl9wb29scxgDIAMoCzIoLmthZ2VudC5hcGkudjFhbHBoYTEuU3Vic3RyYXRlV29ya2VyUG9vbBJECg9hY3Rvcl90ZW1wbGF0ZXMYBCADKAsyKy5rYWdlbnQuYXBpLnYxYWxwaGExLlN1YnN0cmF0ZUFjdG9yVGVtcGxhdGUSMwoGYWN0b3JzGAUgAygLMiMua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVBY3RvchI1Cgd3b3JrZXJzGAYgAygLMiQua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVXb3JrZXIiXQoTU3Vic3RyYXRlV29ya2VyUG9vbBIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRIQCghyZXBsaWNhcxgDIAEoBRITCgthdGVvbV9pbWFnZRgEIAEoCSLbAQoWU3Vic3RyYXRlQWN0b3JUZW1wbGF0ZRIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRINCgVwaGFzZRgDIAEoCRIXCg9nb2xkZW5fYWN0b3JfaWQYBCABKAkSFwoPZ29sZGVuX3NuYXBzaG90GAUgASgJEhUKDXNhbmRib3hfY2xhc3MYBiABKAkSFwoPd29ya2VyX3NlbGVjdG9yGAcgASgJEhQKDGhhcm5lc3NfbmFtZRgIIAEoCRIZChFtYW5hZ2VkX2J5X2thZ2VudBgJIAEoCCKwAgoOU3Vic3RyYXRlQWN0b3ISEAoIYWN0b3JfaWQYASABKAkSEAoIYXRlc3BhY2UYAiABKAkSDgoGc3RhdHVzGAMgASgJEiAKGGFjdG9yX3RlbXBsYXRlX25hbWVzcGFjZRgEIAEoCRIbChNhY3Rvcl90ZW1wbGF0ZV9uYW1lGAUgASgJEhsKE2F0ZW9tX3BvZF9uYW1lc3BhY2UYBiABKAkSFgoOYXRlb21fcG9kX25hbWUYByABKAkSFAoMYXRlb21fcG9kX2lwGAggASgJEhcKD2xhdGVzdF9zbmFwc2hvdBgJIAEoCRIYChB3b3JrZXJfcG9vbF9uYW1lGAogASgJEhwKFGluX3Byb2dyZXNzX3NuYXBzaG90GAsgASgJEg8KB3ZlcnNpb24YDCABKAMiygEKD1N1YnN0cmF0ZVdvcmtlchIYChB3b3JrZXJfbmFtZXNwYWNlGAEgASgJEhMKC3dvcmtlcl9wb29sGAIgASgJEhIKCndvcmtlcl9wb2QYAyABKAkSFwoPYWN0b3JfbmFtZXNwYWNlGAQgASgJEhYKDmFjdG9yX3RlbXBsYXRlGAUgASgJEhAKCGFjdG9yX2lkGAYgASgJEgoKAmlwGAcgASgJEg8KB3ZlcnNpb24YCCABKAMSFAoMYWN0b3Jfc3RhdHVzGAkgASgJMrsDCg1TeXN0ZW1TZXJ2aWNlEl0KCkdldFZlcnNpb24SJi5rYWdlbnQuYXBpLnYxYWxwaGExLkdldFZlcnNpb25SZXF1ZXN0Gicua2FnZW50LmFwaS52MWFscGhhMS5HZXRWZXJzaW9uUmVzcG9uc2USaQoOR2V0Q3VycmVudFVzZXISKi5rYWdlbnQuYXBpLnYxYWxwaGExLkdldEN1cnJlbnRVc2VyUmVxdWVzdBorLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0Q3VycmVudFVzZXJSZXNwb25zZRJpCg5MaXN0TmFtZXNwYWNlcxIqLmthZ2VudC5hcGkudjFhbHBoYTEuTGlzdE5hbWVzcGFjZXNSZXF1ZXN0Gisua2FnZW50LmFwaS52MWFscGhhMS5MaXN0TmFtZXNwYWNlc1Jlc3BvbnNlEnUKEkdldFN1YnN0cmF0ZVN0YXR1cxIuLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0U3Vic3RyYXRlU3RhdHVzUmVxdWVzdBovLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0U3Vic3RyYXRlU3RhdHVzUmVzcG9uc2VCSVpHZ2l0aHViLmNvbS9rYWdlbnQtZGV2L2thZ2VudC9nby9hcGkvZ2VuL2thZ2VudC9hcGkvdjFhbHBoYTE7YXBpdjFhbHBoYTFiBnByb3RvMw", [file_google_protobuf_struct]); /** * @generated from message kagent.api.v1alpha1.GetVersionRequest @@ -398,6 +398,14 @@ export type SubstrateWorker = Message<"kagent.api.v1alpha1.SubstrateWorker"> & { * @generated from field: int64 version = 8; */ version: bigint; + + /** + * What the actor on this pod is doing, so a reader of the worker list is not + * sent to the actor list to find out. Empty when the pod holds nothing. + * + * @generated from field: string actor_status = 9; + */ + actorStatus: string; }; /** diff --git a/ui/src/mocks/fixtures.ts b/ui/src/mocks/fixtures.ts index 67c213acd..f61f3bfe2 100644 --- a/ui/src/mocks/fixtures.ts +++ b/ui/src/mocks/fixtures.ts @@ -336,6 +336,9 @@ export const mockSubstrateStatus: SubstrateStatusResponse = { actorNamespace: "kagent", actorTemplate: "coder-template", actorId: "actor-7f21", + // The controller carries the actor's status onto the worker, so the page never + // has to join two paged reads to colour a pod. + actorStatus: "Running", ip: "10.42.1.19", version: 4, }, diff --git a/ui/src/mocks/transport.ts b/ui/src/mocks/transport.ts index e1b353aa7..c218fe944 100644 --- a/ui/src/mocks/transport.ts +++ b/ui/src/mocks/transport.ts @@ -1395,6 +1395,7 @@ on(SystemService.method.getSubstrateStatus, (input, call) => { actorNamespace: worker.actorNamespace ?? "", actorTemplate: worker.actorTemplate ?? "", actorId: worker.actorId ?? "", + actorStatus: worker.actorStatus ?? "", ip: worker.ip ?? "", version: BigInt(worker.version ?? 0), })), diff --git a/ui/src/pages/SubstratePage.tsx b/ui/src/pages/SubstratePage.tsx index e6d17680f..bceb261cf 100644 --- a/ui/src/pages/SubstratePage.tsx +++ b/ui/src/pages/SubstratePage.tsx @@ -284,7 +284,7 @@ const WORKER_STATES = ["Idle", ...ACTOR_STATES]; * proportional band per status. The number is where counting gives out, not where the * browser does. */ -const ACTORS_DRAWN_INDIVIDUALLY = 120; +const ACTORS_DRAWN_INDIVIDUALLY = 80; /** * The whole actor inventory as one bar, coloured by what each actor is doing. @@ -307,12 +307,15 @@ function StatusBar({ testId, vocabulary, noun, + unread, }: { counts: SubstrateStatusCount[]; /** Every status worth listing at zero. Anything counted but missing is added to it. */ vocabulary: string[]; /** What is being counted, for the places with room to say it: `Actors`, `Workers`. */ noun: string; + /** True when the read failed, so nothing here is a count of anything. */ + unread?: boolean; /** * The bar's accessible name, announced with its breakdown. Not drawn: the legend * beneath already names every colour on it, and a heading over a card that is already @@ -531,12 +534,19 @@ function StatusBar({ {total === 0 ? ( <> {track} - <Text - data-testid={`${testId}-empty`} - css={{ color: theme.color.textMuted, fontSize: 12, display: "block", marginTop: 8 }} - > - {emptyText} - </Text> + {/* Silent when the read failed: the banner above already says so, and "no actors + in this scope" under a broken backend reports a healthy empty cluster. The + legend stays either way — it is ten keys and two rows tall, and dropping it + as the last actor drains moves the table under whoever is reading it. */} + {unread ? null : ( + <Text + data-testid={`${testId}-empty`} + css={{ color: theme.color.textMuted, fontSize: 12, display: "block", marginTop: 8 }} + > + {emptyText} + </Text> + )} + {legend} </> ) : ( <Tooltip @@ -1162,7 +1172,13 @@ export function SubstratePage() { [inventory?.actorTemplates, templateQuery], ); - const actorRows = actors.error ? [] : (actors.data?.actors ?? []); + /* Memoised so the two bars below have a stable dependency: both branches allocate a new + array, so an inline expression changed identity on every render and the memos it fed + recomputed every tick — which is the one thing they exist to avoid. */ + const actorRows = useMemo( + () => (actors.error ? [] : (actors.data?.actors ?? [])), + [actors.error, actors.data?.actors], + ); /* * What the bar above the actor table counts. @@ -1192,27 +1208,23 @@ export function SubstratePage() { : `Matching “${actorFilter}”: ${atAGlance(matches)}`, }; }, [actorFilter, actorRows, actors.data?.totalSize, inventory?.actorStatusCounts]); - const workerRows = workers.error ? [] : (workers.data?.workers ?? []); + const workerRows = useMemo( + () => (workers.error ? [] : (workers.data?.workers ?? [])), + [workers.error, workers.data?.workers], + ); /* * The workers bar: one segment per pod, coloured by what the actor on it is doing. * - * The worker entry carries the actor's id but not its status, so the status is joined - * here from the actors that came back. Both lists are pages, so a worker whose actor is - * not on the loaded page reads as `Not loaded` rather than being coloured by a guess — - * the join is complete exactly when both fit in one page, which is the common case - * locally and not the case on a large cluster. - * - * A client-side join, complete only within a page. The durable fix is the controller - * sending the actor's status on the worker entry, which would remove this. + * The status comes from the worker entry itself, which the controller fills by joining + * the two whole lists. Derived here from the loaded actors instead, it moved whenever + * the *actors* table was searched, sorted or paged — narrowing one list recoloured the + * other, and a pod whose actor was off the page read as a status no cluster reports. */ const workerBar = useMemo(() => { - const statusByActor = new Map(actorRows.map((actor) => [actor.actorId, actor.status])); const byStatus = new Map<string, number>(); for (const worker of workerRows) { - const status = !worker.actorId - ? "Idle" - : (statusByActor.get(worker.actorId) ?? "Not loaded"); + const status = worker.actorId ? (worker.actorStatus || "Unknown") : "Idle"; byStatus.set(status, (byStatus.get(status) ?? 0) + 1); } const counts = [...byStatus].map(([status, count]) => ({ status, count })); @@ -1223,7 +1235,7 @@ export function SubstratePage() { counts, caption: workerFilter ? `Matching “${workerFilter}”: ${shown}` : shown, }; - }, [actorRows, workerRows, workerFilter, workers.data?.totalSize]); + }, [workerRows, workerFilter, workers.data?.totalSize]); /* * The tiles, from the summary's own counts. @@ -1833,6 +1845,7 @@ export function SubstratePage() { title="Actor status" vocabulary={ACTOR_STATES} noun="Actors" + unread={Boolean(summary.error || actors.error)} counts={actorBar.counts} caption={actorBar.caption} emptyText={ @@ -1937,6 +1950,7 @@ export function SubstratePage() { title="Worker status" vocabulary={WORKER_STATES} noun="Workers" + unread={Boolean(summary.error || workers.error)} counts={workerBar.counts} caption={workerBar.caption} emptyText={ From ae1cd288186d91031e6948db5ab5a1683506eddb Mon Sep 17 00:00:00 2001 From: Nicholas Bucher <behappy54321@gmail.com> Date: Fri, 4 Sep 2026 16:26:23 -0400 Subject: [PATCH 7/8] fix: name every actor state, so the status column sorts as it reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ActorStatusLabel` fell back to the wire constant for a state it had not been taught, and callers sort on what it sends while showing what they sorted — so `ACTOR_STATE_DELETING` filed itself under A and read as "Deleting" beneath a heading that says sorted by status. Every value is named now, including ones added after this build. The test walks `ActorState_name` rather than a list, so a new state fails here instead of reaching a reader as a protobuf symbol. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Nicholas Bucher <behappy54321@gmail.com> --- go/core/internal/substrate/list.go | 18 +++++++++++- go/core/internal/substrate/list_test.go | 37 +++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 go/core/internal/substrate/list_test.go diff --git a/go/core/internal/substrate/list.go b/go/core/internal/substrate/list.go index 1d4862d17..85f8172af 100644 --- a/go/core/internal/substrate/list.go +++ b/go/core/internal/substrate/list.go @@ -2,6 +2,7 @@ package substrate import ( "context" + "strings" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -70,6 +71,10 @@ func (c *Client) ListActorTemplates(ctx context.Context, atespace string) ([]*at } // ActorStatusLabel returns a stable human-readable actor status. +// +// Never a wire constant, including for a state this build has not been taught: callers +// sort on what we send and show what they sort, so `ACTOR_STATE_DELETING` would file +// itself under A while reading as "Deleting" under a heading that says sorted by status. func ActorStatusLabel(status ateapipb.ActorState) string { switch status { case ateapipb.ActorState_ACTOR_STATE_RESUMING: @@ -87,6 +92,17 @@ func ActorStatusLabel(status ateapipb.ActorState) string { case ateapipb.ActorState_ACTOR_STATE_UNSPECIFIED: return "Unknown" default: - return status.String() + return humanizeState(status.String()) } } + +// humanizeState turns `ACTOR_STATE_DELETING` into `Deleting`. Protobuf names each value +// after its own enum, and that prefix only repeats the column it is shown in. +func humanizeState(name string) string { + words := strings.ToLower(strings.TrimPrefix(name, "ACTOR_STATE_")) + words = strings.ReplaceAll(words, "_", " ") + if words == "" { + return name + } + return strings.ToUpper(words[:1]) + words[1:] +} diff --git a/go/core/internal/substrate/list_test.go b/go/core/internal/substrate/list_test.go new file mode 100644 index 000000000..481ddf224 --- /dev/null +++ b/go/core/internal/substrate/list_test.go @@ -0,0 +1,37 @@ +package substrate + +import ( + "strings" + "testing" + + "github.com/agent-substrate/substrate/pkg/proto/ateapipb" +) + +func TestActorStatusLabelNeverReturnsAWireConstant(t *testing.T) { + // Every value the enum defines, including any added since this was written: callers + // sort on the label and display it, so a constant sorts by a prefix nobody sees. + names := ateapipb.ActorState_name + if len(names) == 0 { + t.Fatal("ActorState has no values; the enum this reads moved") + } + for number, name := range names { + label := ActorStatusLabel(ateapipb.ActorState(number)) + if strings.Contains(label, "_") { + t.Errorf("%s reads as %q; it should be words", name, label) + } + if label == name { + t.Errorf("%s reaches a reader as its own wire constant", name) + } + } + + for state, want := range map[ateapipb.ActorState]string{ + ateapipb.ActorState_ACTOR_STATE_CRASHED: "Crashed", + ateapipb.ActorState_ACTOR_STATE_DELETING: "Deleting", + ateapipb.ActorState_ACTOR_STATE_RUNNING: "Running", + ateapipb.ActorState_ACTOR_STATE_UNSPECIFIED: "Unknown", + } { + if got := ActorStatusLabel(state); got != want { + t.Errorf("ActorStatusLabel(%v) = %q, want %q", state, got, want) + } + } +} From 62182d9078658b423b916b9b4320fc1db32e98f4 Mon Sep 17 00:00:00 2001 From: Nicholas Bucher <behappy54321@gmail.com> Date: Fri, 4 Sep 2026 17:01:48 -0400 Subject: [PATCH 8/8] revert: take the controller and worker-bar work out of this PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worker→actor mapping is a controller bug with its own cause (#2697) and its own issue (#2709), and fixing it raises a data-model question — a v0.0.25 worker holds several actors while `SubstrateWorker` has room for one — that belongs to whoever owns substrate rather than to a UI change. The workers bar goes with it. `workerFromProto` sets no actor fields at all, so `actorId` is empty on every cluster: the bar would read as an idle fleet while pods are running, which is worse than not drawing it. What stays needs no controller change: the actors bar reads the summary's own counts, and a wire constant is still read as a word by the page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Nicholas Bucher <behappy54321@gmail.com> --- go/api/gen/kagent/api/v1alpha1/system.pb.go | 19 ++---- go/core/internal/grpcserver/system.go | 1 - go/core/internal/service/system/service.go | 61 +------------------ .../internal/service/system/service_test.go | 44 +------------ go/core/internal/substrate/list.go | 18 +----- go/core/internal/substrate/list_test.go | 37 ----------- proto/kagent/api/v1alpha1/system.proto | 3 - .../tests/substrate/substrate.spec.ts | 16 ----- ui/src/api/domain/substrate.ts | 2 - ui/src/api/grpc/operations.ts | 1 - .../kagent/api/v1alpha1/system_pb.ts | 10 +-- ui/src/mocks/fixtures.ts | 3 - ui/src/mocks/transport.ts | 1 - ui/src/pages/SubstratePage.tsx | 47 +------------- 14 files changed, 11 insertions(+), 252 deletions(-) delete mode 100644 go/core/internal/substrate/list_test.go diff --git a/go/api/gen/kagent/api/v1alpha1/system.pb.go b/go/api/gen/kagent/api/v1alpha1/system.pb.go index 5e88449be..ba2d169ff 100644 --- a/go/api/gen/kagent/api/v1alpha1/system.pb.go +++ b/go/api/gen/kagent/api/v1alpha1/system.pb.go @@ -776,11 +776,8 @@ type SubstrateWorker struct { ActorId string `protobuf:"bytes,6,opt,name=actor_id,json=actorId,proto3" json:"actor_id,omitempty"` Ip string `protobuf:"bytes,7,opt,name=ip,proto3" json:"ip,omitempty"` Version int64 `protobuf:"varint,8,opt,name=version,proto3" json:"version,omitempty"` - // What the actor on this pod is doing, so a reader of the worker list is not - // sent to the actor list to find out. Empty when the pod holds nothing. - ActorStatus string `protobuf:"bytes,9,opt,name=actor_status,json=actorStatus,proto3" json:"actor_status,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *SubstrateWorker) Reset() { @@ -869,13 +866,6 @@ func (x *SubstrateWorker) GetVersion() int64 { return 0 } -func (x *SubstrateWorker) GetActorStatus() string { - if x != nil { - return x.ActorStatus - } - return "" -} - var File_kagent_api_v1alpha1_system_proto protoreflect.FileDescriptor const file_kagent_api_v1alpha1_system_proto_rawDesc = "" + @@ -938,7 +928,7 @@ const file_kagent_api_v1alpha1_system_proto_rawDesc = "" + "\x10worker_pool_name\x18\n" + " \x01(\tR\x0eworkerPoolName\x120\n" + "\x14in_progress_snapshot\x18\v \x01(\tR\x12inProgressSnapshot\x12\x18\n" + - "\aversion\x18\f \x01(\x03R\aversion\"\xb4\x02\n" + + "\aversion\x18\f \x01(\x03R\aversion\"\x91\x02\n" + "\x0fSubstrateWorker\x12)\n" + "\x10worker_namespace\x18\x01 \x01(\tR\x0fworkerNamespace\x12\x1f\n" + "\vworker_pool\x18\x02 \x01(\tR\n" + @@ -949,8 +939,7 @@ const file_kagent_api_v1alpha1_system_proto_rawDesc = "" + "\x0eactor_template\x18\x05 \x01(\tR\ractorTemplate\x12\x19\n" + "\bactor_id\x18\x06 \x01(\tR\aactorId\x12\x0e\n" + "\x02ip\x18\a \x01(\tR\x02ip\x12\x18\n" + - "\aversion\x18\b \x01(\x03R\aversion\x12!\n" + - "\factor_status\x18\t \x01(\tR\vactorStatus2\xbb\x03\n" + + "\aversion\x18\b \x01(\x03R\aversion2\xbb\x03\n" + "\rSystemService\x12]\n" + "\n" + "GetVersion\x12&.kagent.api.v1alpha1.GetVersionRequest\x1a'.kagent.api.v1alpha1.GetVersionResponse\x12i\n" + diff --git a/go/core/internal/grpcserver/system.go b/go/core/internal/grpcserver/system.go index b8ae7532e..32c42222a 100644 --- a/go/core/internal/grpcserver/system.go +++ b/go/core/internal/grpcserver/system.go @@ -112,7 +112,6 @@ func (s *systemServer) GetSubstrateStatus(ctx context.Context, request *apiv1alp ActorNamespace: worker.ActorNamespace, ActorTemplate: worker.ActorTemplate, ActorId: worker.ActorID, - ActorStatus: worker.ActorStatus, Ip: worker.IP, Version: worker.Version, }) diff --git a/go/core/internal/service/system/service.go b/go/core/internal/service/system/service.go index 5fa0a4ba9..a8934ba11 100644 --- a/go/core/internal/service/system/service.go +++ b/go/core/internal/service/system/service.go @@ -101,7 +101,6 @@ type SubstrateWorker struct { ActorNamespace string ActorTemplate string ActorID string - ActorStatus string IP string Version int64 } @@ -375,20 +374,14 @@ func (s *Service) listATEState(ctx context.Context, namespaces []string) ([]Subs } actors := make([]SubstrateActor, 0, len(actorsFromAPI)) - // Every actor, scope or no scope. The workers below are filtered by *their* namespace - // and an actor by its template's, so a worker running an actor whose template lives - // elsewhere finds no match in the scoped list and reports itself idle. - placed := make([]SubstrateActor, 0, len(actorsFromAPI)) for _, actor := range actorsFromAPI { if actor == nil { continue } - converted := actorFromProto(actor) - placed = append(placed, converted) if !allowedAtespace(actor.GetActorTemplate().GetAtespace(), allowAll, allowed) { continue } - actors = append(actors, converted) + actors = append(actors, actorFromProto(actor)) } workers := make([]SubstrateWorker, 0, len(workersFromAPI)) @@ -404,7 +397,6 @@ func (s *Service) listATEState(ctx context.Context, namespaces []string) ([]Subs } workers = append(workers, workerFromProto(worker)) } - placeActorsOnWorkers(workers, placed) return templates, actors, workers, nil } @@ -444,57 +436,6 @@ func workerFromProto(worker *ateapipb.Worker) SubstrateWorker { } } -// placeActorsOnWorkers fills in what each worker is holding. -// -// `ateapi.Worker` carries no actor reference — only the actor names its pod — so this is -// the one place both whole lists are in hand to join them. -// -// A worker holds several actors since v0.0.25 and `SubstrateWorker` has room for one, so -// a live actor wins over a parked one and the lowest id breaks the tie. Never first seen: -// ate-api returns actors unordered, so the cell would name a different actor on each poll. -// supersedes reports whether one actor on a pod should be named over another: a running -// one over a parked one, and otherwise the lower id, so the answer does not depend on the -// order ate-api happened to return them in. -func supersedes(candidate, held SubstrateActor) bool { - if parkedActor(candidate) != parkedActor(held) { - return parkedActor(held) - } - return candidate.ActorID < held.ActorID -} - -func parkedActor(actor SubstrateActor) bool { - switch actor.Status { - case "Suspended", "Paused", "Unknown", "": - return true - } - return false -} - -func placeActorsOnWorkers(workers []SubstrateWorker, actors []SubstrateActor) { - type pod struct{ namespace, name string } - holding := make(map[pod]SubstrateActor, len(actors)) - for _, actor := range actors { - if actor.AteomPodName == "" { - continue - } - key := pod{actor.AteomPodNamespace, actor.AteomPodName} - if held, ok := holding[key]; ok && !supersedes(actor, held) { - continue - } - holding[key] = actor - } - for i := range workers { - actor, ok := holding[pod{workers[i].WorkerNamespace, workers[i].WorkerPod}] - if !ok { - continue - } - workers[i].ActorNamespace = actor.ActorTemplateNamespace - workers[i].ActorTemplate = actor.ActorTemplateName - workers[i].ActorID = actor.ActorID - workers[i].ActorStatus = actor.Status - } -} - func labelSelectorString(ctx context.Context, selector *metav1.LabelSelector) string { if selector == nil { return "" diff --git a/go/core/internal/service/system/service_test.go b/go/core/internal/service/system/service_test.go index 16750a423..b20439363 100644 --- a/go/core/internal/service/system/service_test.go +++ b/go/core/internal/service/system/service_test.go @@ -146,35 +146,10 @@ func TestGetSubstrateStatus(t *testing.T) { }}, }}, actors: []*ateapipb.Actor{{ - // Two actors on one pod, because workers hold several since v0.0.25, and - // ate-api returns them in no particular order. This one is later in the - // alphabet and arrives first: a join that kept whichever it saw first - // would report it here and change its mind on the next read. - Metadata: &ateapipb.ResourceMetadata{Name: "actor-2"}, - ActorTemplate: &ateapipb.ObjectRef{Atespace: "team", Name: "template"}, - Status: &ateapipb.ActorStatus{ - State: ateapipb.ActorState_ACTOR_STATE_RUNNING, - WorkerAssignment: &ateapipb.WorkerAssignment{ - WorkerNamespace: "team", - WorkerPool: "pool", - WorkerPod: "worker-0", - }, - }, - }, { Metadata: &ateapipb.ResourceMetadata{Name: "actor-1"}, ActorTemplate: &ateapipb.ObjectRef{Atespace: "team", Name: "template"}, Status: &ateapipb.ActorStatus{ - // Parked, and the lower id: the pod must still be reported as running - // actor-2, because a suspended actor says nothing about a busy pod. - State: ateapipb.ActorState_ACTOR_STATE_SUSPENDED, - // Where this actor is placed. `ateapi.Worker` carries no actor - // reference, so this is the only record of the binding. - WorkerAssignment: &ateapipb.WorkerAssignment{ - WorkerNamespace: "team", - WorkerPool: "pool", - WorkerPod: "worker-0", - WorkerPodIp: "10.42.0.7", - }, + State: ateapipb.ActorState_ACTOR_STATE_RUNNING, }, }}, workers: []*ateapipb.Worker{{ @@ -201,23 +176,10 @@ func TestGetSubstrateStatus(t *testing.T) { assert.Equal(t, "gvisor", result.ActorTemplates[0].SandboxClass) assert.Equal(t, "kagent", result.ActorTemplates[0].HarnessName) assert.True(t, result.ActorTemplates[0].ManagedByKagent) - require.Len(t, result.Actors, 2) - // By status rather than by position: the fixture now holds one of each, and an - // index here would assert the sort order under the guise of asserting the label. - actorStatuses := map[string]string{} - for _, actor := range result.Actors { - actorStatuses[actor.ActorID] = actor.Status - } - assert.Equal(t, map[string]string{"actor-1": "Suspended", "actor-2": "Running"}, actorStatuses) + require.Len(t, result.Actors, 1) + assert.Equal(t, "Running", result.Actors[0].Status) require.Len(t, result.Workers, 1) assert.Equal(t, "worker-0", result.Workers[0].WorkerPod) - // The worker says what is on it. Read from the actor's own assignment, because - // the worker message has no field for it — without the join every pod reports - // holding nothing and a running fleet reads as an idle one. - assert.Equal(t, "actor-2", result.Workers[0].ActorID) - assert.Equal(t, "Running", result.Workers[0].ActorStatus) - assert.Equal(t, "template", result.Workers[0].ActorTemplate) - assert.Equal(t, "team", result.Workers[0].ActorNamespace) assert.Equal(t, int64(3), result.Workers[0].Version) }) diff --git a/go/core/internal/substrate/list.go b/go/core/internal/substrate/list.go index 85f8172af..1d4862d17 100644 --- a/go/core/internal/substrate/list.go +++ b/go/core/internal/substrate/list.go @@ -2,7 +2,6 @@ package substrate import ( "context" - "strings" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -71,10 +70,6 @@ func (c *Client) ListActorTemplates(ctx context.Context, atespace string) ([]*at } // ActorStatusLabel returns a stable human-readable actor status. -// -// Never a wire constant, including for a state this build has not been taught: callers -// sort on what we send and show what they sort, so `ACTOR_STATE_DELETING` would file -// itself under A while reading as "Deleting" under a heading that says sorted by status. func ActorStatusLabel(status ateapipb.ActorState) string { switch status { case ateapipb.ActorState_ACTOR_STATE_RESUMING: @@ -92,17 +87,6 @@ func ActorStatusLabel(status ateapipb.ActorState) string { case ateapipb.ActorState_ACTOR_STATE_UNSPECIFIED: return "Unknown" default: - return humanizeState(status.String()) + return status.String() } } - -// humanizeState turns `ACTOR_STATE_DELETING` into `Deleting`. Protobuf names each value -// after its own enum, and that prefix only repeats the column it is shown in. -func humanizeState(name string) string { - words := strings.ToLower(strings.TrimPrefix(name, "ACTOR_STATE_")) - words = strings.ReplaceAll(words, "_", " ") - if words == "" { - return name - } - return strings.ToUpper(words[:1]) + words[1:] -} diff --git a/go/core/internal/substrate/list_test.go b/go/core/internal/substrate/list_test.go deleted file mode 100644 index 481ddf224..000000000 --- a/go/core/internal/substrate/list_test.go +++ /dev/null @@ -1,37 +0,0 @@ -package substrate - -import ( - "strings" - "testing" - - "github.com/agent-substrate/substrate/pkg/proto/ateapipb" -) - -func TestActorStatusLabelNeverReturnsAWireConstant(t *testing.T) { - // Every value the enum defines, including any added since this was written: callers - // sort on the label and display it, so a constant sorts by a prefix nobody sees. - names := ateapipb.ActorState_name - if len(names) == 0 { - t.Fatal("ActorState has no values; the enum this reads moved") - } - for number, name := range names { - label := ActorStatusLabel(ateapipb.ActorState(number)) - if strings.Contains(label, "_") { - t.Errorf("%s reads as %q; it should be words", name, label) - } - if label == name { - t.Errorf("%s reaches a reader as its own wire constant", name) - } - } - - for state, want := range map[ateapipb.ActorState]string{ - ateapipb.ActorState_ACTOR_STATE_CRASHED: "Crashed", - ateapipb.ActorState_ACTOR_STATE_DELETING: "Deleting", - ateapipb.ActorState_ACTOR_STATE_RUNNING: "Running", - ateapipb.ActorState_ACTOR_STATE_UNSPECIFIED: "Unknown", - } { - if got := ActorStatusLabel(state); got != want { - t.Errorf("ActorStatusLabel(%v) = %q, want %q", state, got, want) - } - } -} diff --git a/proto/kagent/api/v1alpha1/system.proto b/proto/kagent/api/v1alpha1/system.proto index 63d8f323f..5bc3ef780 100644 --- a/proto/kagent/api/v1alpha1/system.proto +++ b/proto/kagent/api/v1alpha1/system.proto @@ -94,7 +94,4 @@ message SubstrateWorker { string actor_id = 6; string ip = 7; int64 version = 8; - // What the actor on this pod is doing, so a reader of the worker list is not - // sent to the actor list to find out. Empty when the pod holds nothing. - string actor_status = 9; } diff --git a/ui/playwright/tests/substrate/substrate.spec.ts b/ui/playwright/tests/substrate/substrate.spec.ts index a711e4423..31da8df6b 100644 --- a/ui/playwright/tests/substrate/substrate.spec.ts +++ b/ui/playwright/tests/substrate/substrate.spec.ts @@ -161,19 +161,6 @@ test("substrate: the inventory renders, and partial runtime data says so", async ).toHaveAttribute("data-tone", "danger"); }); - await test.step("5b. the workers bar is one segment per pod, coloured by what is on it", async () => { - const bar = page.getByTestId("substrate-worker-status-counts"); - // A pod holding a running actor and a pod holding nothing: the busy one takes the - // actor's own colour, and the free one is parked, so it sorts to the end. - await expect( - bar.locator("[data-tone]").evaluateAll((els) => els.map((el) => el.getAttribute("data-tone"))), - ).resolves.toEqual(["healthy", "idle"]); - await expect(bar).toHaveAttribute( - "aria-label", - "Worker status. Running Workers: 1, Idle Workers: 1", - ); - }); - await test.step("6. the workers, including the one holding nothing", async () => { const workers = page.getByTestId("substrate-workers-table"); await expect(workers).toBeVisible(); @@ -284,9 +271,6 @@ test("substrate: an unconfigured ate-api is explained, not reported as broken", await expect(page.getByTestId("substrate-actor-status-counts-empty")).toHaveText( "ate-api is not configured, so there are no actors to show.", ); - await expect(page.getByTestId("substrate-worker-status-counts-empty")).toHaveText( - "ate-api is not configured, so there are no workers to show.", - ); // The two runtime sections name the setting to change. The two Kubernetes ones do not — // they are empty for an unrelated reason, and saying "ate-api" over them would send an diff --git a/ui/src/api/domain/substrate.ts b/ui/src/api/domain/substrate.ts index 12d483b0f..7207be4c6 100644 --- a/ui/src/api/domain/substrate.ts +++ b/ui/src/api/domain/substrate.ts @@ -73,8 +73,6 @@ export interface SubstrateWorkerEntry { actorNamespace?: string; actorTemplate?: string; actorId?: string; - /** What the actor on this pod is doing. Absent when the pod holds nothing. */ - actorStatus?: string; ip?: string; version?: number; } diff --git a/ui/src/api/grpc/operations.ts b/ui/src/api/grpc/operations.ts index 8bad6eb86..472bc5c80 100644 --- a/ui/src/api/grpc/operations.ts +++ b/ui/src/api/grpc/operations.ts @@ -1107,7 +1107,6 @@ function toWorkerEntry(worker: PbSubstrateWorker): SubstrateWorkerEntry { actorNamespace: orUndefined(worker.actorNamespace), actorTemplate: orUndefined(worker.actorTemplate), actorId: orUndefined(worker.actorId), - actorStatus: orUndefined(worker.actorStatus), ip: orUndefined(worker.ip), version: toNumber(worker.version), }; diff --git a/ui/src/generated/kagent/api/v1alpha1/system_pb.ts b/ui/src/generated/kagent/api/v1alpha1/system_pb.ts index e244c9ccc..84f234834 100644 --- a/ui/src/generated/kagent/api/v1alpha1/system_pb.ts +++ b/ui/src/generated/kagent/api/v1alpha1/system_pb.ts @@ -11,7 +11,7 @@ import type { JsonObject, Message } from "@bufbuild/protobuf"; * Describes the file kagent/api/v1alpha1/system.proto. */ export const file_kagent_api_v1alpha1_system: GenFile = /*@__PURE__*/ - fileDesc("CiBrYWdlbnQvYXBpL3YxYWxwaGExL3N5c3RlbS5wcm90bxITa2FnZW50LmFwaS52MWFscGhhMSITChFHZXRWZXJzaW9uUmVxdWVzdCJUChJHZXRWZXJzaW9uUmVzcG9uc2USFgoOa2FnZW50X3ZlcnNpb24YASABKAkSEgoKZ2l0X2NvbW1pdBgCIAEoCRISCgpidWlsZF9kYXRlGAMgASgJIhcKFUdldEN1cnJlbnRVc2VyUmVxdWVzdCJBChZHZXRDdXJyZW50VXNlclJlc3BvbnNlEicKBmNsYWltcxgBIAEoCzIXLmdvb2dsZS5wcm90b2J1Zi5TdHJ1Y3QiFwoVTGlzdE5hbWVzcGFjZXNSZXF1ZXN0IikKCU5hbWVzcGFjZRIMCgRuYW1lGAEgASgJEg4KBnN0YXR1cxgCIAEoCSJMChZMaXN0TmFtZXNwYWNlc1Jlc3BvbnNlEjIKCm5hbWVzcGFjZXMYASADKAsyHi5rYWdlbnQuYXBpLnYxYWxwaGExLk5hbWVzcGFjZSIuChlHZXRTdWJzdHJhdGVTdGF0dXNSZXF1ZXN0EhEKCW5hbWVzcGFjZRgBIAEoCSK2AgoaR2V0U3Vic3RyYXRlU3RhdHVzUmVzcG9uc2USDwoHZW5hYmxlZBgBIAEoCBIVCg1hdGVfYXBpX2Vycm9yGAIgASgJEj4KDHdvcmtlcl9wb29scxgDIAMoCzIoLmthZ2VudC5hcGkudjFhbHBoYTEuU3Vic3RyYXRlV29ya2VyUG9vbBJECg9hY3Rvcl90ZW1wbGF0ZXMYBCADKAsyKy5rYWdlbnQuYXBpLnYxYWxwaGExLlN1YnN0cmF0ZUFjdG9yVGVtcGxhdGUSMwoGYWN0b3JzGAUgAygLMiMua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVBY3RvchI1Cgd3b3JrZXJzGAYgAygLMiQua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVXb3JrZXIiXQoTU3Vic3RyYXRlV29ya2VyUG9vbBIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRIQCghyZXBsaWNhcxgDIAEoBRITCgthdGVvbV9pbWFnZRgEIAEoCSLbAQoWU3Vic3RyYXRlQWN0b3JUZW1wbGF0ZRIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRINCgVwaGFzZRgDIAEoCRIXCg9nb2xkZW5fYWN0b3JfaWQYBCABKAkSFwoPZ29sZGVuX3NuYXBzaG90GAUgASgJEhUKDXNhbmRib3hfY2xhc3MYBiABKAkSFwoPd29ya2VyX3NlbGVjdG9yGAcgASgJEhQKDGhhcm5lc3NfbmFtZRgIIAEoCRIZChFtYW5hZ2VkX2J5X2thZ2VudBgJIAEoCCKwAgoOU3Vic3RyYXRlQWN0b3ISEAoIYWN0b3JfaWQYASABKAkSEAoIYXRlc3BhY2UYAiABKAkSDgoGc3RhdHVzGAMgASgJEiAKGGFjdG9yX3RlbXBsYXRlX25hbWVzcGFjZRgEIAEoCRIbChNhY3Rvcl90ZW1wbGF0ZV9uYW1lGAUgASgJEhsKE2F0ZW9tX3BvZF9uYW1lc3BhY2UYBiABKAkSFgoOYXRlb21fcG9kX25hbWUYByABKAkSFAoMYXRlb21fcG9kX2lwGAggASgJEhcKD2xhdGVzdF9zbmFwc2hvdBgJIAEoCRIYChB3b3JrZXJfcG9vbF9uYW1lGAogASgJEhwKFGluX3Byb2dyZXNzX3NuYXBzaG90GAsgASgJEg8KB3ZlcnNpb24YDCABKAMiygEKD1N1YnN0cmF0ZVdvcmtlchIYChB3b3JrZXJfbmFtZXNwYWNlGAEgASgJEhMKC3dvcmtlcl9wb29sGAIgASgJEhIKCndvcmtlcl9wb2QYAyABKAkSFwoPYWN0b3JfbmFtZXNwYWNlGAQgASgJEhYKDmFjdG9yX3RlbXBsYXRlGAUgASgJEhAKCGFjdG9yX2lkGAYgASgJEgoKAmlwGAcgASgJEg8KB3ZlcnNpb24YCCABKAMSFAoMYWN0b3Jfc3RhdHVzGAkgASgJMrsDCg1TeXN0ZW1TZXJ2aWNlEl0KCkdldFZlcnNpb24SJi5rYWdlbnQuYXBpLnYxYWxwaGExLkdldFZlcnNpb25SZXF1ZXN0Gicua2FnZW50LmFwaS52MWFscGhhMS5HZXRWZXJzaW9uUmVzcG9uc2USaQoOR2V0Q3VycmVudFVzZXISKi5rYWdlbnQuYXBpLnYxYWxwaGExLkdldEN1cnJlbnRVc2VyUmVxdWVzdBorLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0Q3VycmVudFVzZXJSZXNwb25zZRJpCg5MaXN0TmFtZXNwYWNlcxIqLmthZ2VudC5hcGkudjFhbHBoYTEuTGlzdE5hbWVzcGFjZXNSZXF1ZXN0Gisua2FnZW50LmFwaS52MWFscGhhMS5MaXN0TmFtZXNwYWNlc1Jlc3BvbnNlEnUKEkdldFN1YnN0cmF0ZVN0YXR1cxIuLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0U3Vic3RyYXRlU3RhdHVzUmVxdWVzdBovLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0U3Vic3RyYXRlU3RhdHVzUmVzcG9uc2VCSVpHZ2l0aHViLmNvbS9rYWdlbnQtZGV2L2thZ2VudC9nby9hcGkvZ2VuL2thZ2VudC9hcGkvdjFhbHBoYTE7YXBpdjFhbHBoYTFiBnByb3RvMw", [file_google_protobuf_struct]); + fileDesc("CiBrYWdlbnQvYXBpL3YxYWxwaGExL3N5c3RlbS5wcm90bxITa2FnZW50LmFwaS52MWFscGhhMSITChFHZXRWZXJzaW9uUmVxdWVzdCJUChJHZXRWZXJzaW9uUmVzcG9uc2USFgoOa2FnZW50X3ZlcnNpb24YASABKAkSEgoKZ2l0X2NvbW1pdBgCIAEoCRISCgpidWlsZF9kYXRlGAMgASgJIhcKFUdldEN1cnJlbnRVc2VyUmVxdWVzdCJBChZHZXRDdXJyZW50VXNlclJlc3BvbnNlEicKBmNsYWltcxgBIAEoCzIXLmdvb2dsZS5wcm90b2J1Zi5TdHJ1Y3QiFwoVTGlzdE5hbWVzcGFjZXNSZXF1ZXN0IikKCU5hbWVzcGFjZRIMCgRuYW1lGAEgASgJEg4KBnN0YXR1cxgCIAEoCSJMChZMaXN0TmFtZXNwYWNlc1Jlc3BvbnNlEjIKCm5hbWVzcGFjZXMYASADKAsyHi5rYWdlbnQuYXBpLnYxYWxwaGExLk5hbWVzcGFjZSIuChlHZXRTdWJzdHJhdGVTdGF0dXNSZXF1ZXN0EhEKCW5hbWVzcGFjZRgBIAEoCSK2AgoaR2V0U3Vic3RyYXRlU3RhdHVzUmVzcG9uc2USDwoHZW5hYmxlZBgBIAEoCBIVCg1hdGVfYXBpX2Vycm9yGAIgASgJEj4KDHdvcmtlcl9wb29scxgDIAMoCzIoLmthZ2VudC5hcGkudjFhbHBoYTEuU3Vic3RyYXRlV29ya2VyUG9vbBJECg9hY3Rvcl90ZW1wbGF0ZXMYBCADKAsyKy5rYWdlbnQuYXBpLnYxYWxwaGExLlN1YnN0cmF0ZUFjdG9yVGVtcGxhdGUSMwoGYWN0b3JzGAUgAygLMiMua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVBY3RvchI1Cgd3b3JrZXJzGAYgAygLMiQua2FnZW50LmFwaS52MWFscGhhMS5TdWJzdHJhdGVXb3JrZXIiXQoTU3Vic3RyYXRlV29ya2VyUG9vbBIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRIQCghyZXBsaWNhcxgDIAEoBRITCgthdGVvbV9pbWFnZRgEIAEoCSLbAQoWU3Vic3RyYXRlQWN0b3JUZW1wbGF0ZRIRCgluYW1lc3BhY2UYASABKAkSDAoEbmFtZRgCIAEoCRINCgVwaGFzZRgDIAEoCRIXCg9nb2xkZW5fYWN0b3JfaWQYBCABKAkSFwoPZ29sZGVuX3NuYXBzaG90GAUgASgJEhUKDXNhbmRib3hfY2xhc3MYBiABKAkSFwoPd29ya2VyX3NlbGVjdG9yGAcgASgJEhQKDGhhcm5lc3NfbmFtZRgIIAEoCRIZChFtYW5hZ2VkX2J5X2thZ2VudBgJIAEoCCKwAgoOU3Vic3RyYXRlQWN0b3ISEAoIYWN0b3JfaWQYASABKAkSEAoIYXRlc3BhY2UYAiABKAkSDgoGc3RhdHVzGAMgASgJEiAKGGFjdG9yX3RlbXBsYXRlX25hbWVzcGFjZRgEIAEoCRIbChNhY3Rvcl90ZW1wbGF0ZV9uYW1lGAUgASgJEhsKE2F0ZW9tX3BvZF9uYW1lc3BhY2UYBiABKAkSFgoOYXRlb21fcG9kX25hbWUYByABKAkSFAoMYXRlb21fcG9kX2lwGAggASgJEhcKD2xhdGVzdF9zbmFwc2hvdBgJIAEoCRIYChB3b3JrZXJfcG9vbF9uYW1lGAogASgJEhwKFGluX3Byb2dyZXNzX3NuYXBzaG90GAsgASgJEg8KB3ZlcnNpb24YDCABKAMitAEKD1N1YnN0cmF0ZVdvcmtlchIYChB3b3JrZXJfbmFtZXNwYWNlGAEgASgJEhMKC3dvcmtlcl9wb29sGAIgASgJEhIKCndvcmtlcl9wb2QYAyABKAkSFwoPYWN0b3JfbmFtZXNwYWNlGAQgASgJEhYKDmFjdG9yX3RlbXBsYXRlGAUgASgJEhAKCGFjdG9yX2lkGAYgASgJEgoKAmlwGAcgASgJEg8KB3ZlcnNpb24YCCABKAMyuwMKDVN5c3RlbVNlcnZpY2USXQoKR2V0VmVyc2lvbhImLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0VmVyc2lvblJlcXVlc3QaJy5rYWdlbnQuYXBpLnYxYWxwaGExLkdldFZlcnNpb25SZXNwb25zZRJpCg5HZXRDdXJyZW50VXNlchIqLmthZ2VudC5hcGkudjFhbHBoYTEuR2V0Q3VycmVudFVzZXJSZXF1ZXN0Gisua2FnZW50LmFwaS52MWFscGhhMS5HZXRDdXJyZW50VXNlclJlc3BvbnNlEmkKDkxpc3ROYW1lc3BhY2VzEioua2FnZW50LmFwaS52MWFscGhhMS5MaXN0TmFtZXNwYWNlc1JlcXVlc3QaKy5rYWdlbnQuYXBpLnYxYWxwaGExLkxpc3ROYW1lc3BhY2VzUmVzcG9uc2USdQoSR2V0U3Vic3RyYXRlU3RhdHVzEi4ua2FnZW50LmFwaS52MWFscGhhMS5HZXRTdWJzdHJhdGVTdGF0dXNSZXF1ZXN0Gi8ua2FnZW50LmFwaS52MWFscGhhMS5HZXRTdWJzdHJhdGVTdGF0dXNSZXNwb25zZUJJWkdnaXRodWIuY29tL2thZ2VudC1kZXYva2FnZW50L2dvL2FwaS9nZW4va2FnZW50L2FwaS92MWFscGhhMTthcGl2MWFscGhhMWIGcHJvdG8z", [file_google_protobuf_struct]); /** * @generated from message kagent.api.v1alpha1.GetVersionRequest @@ -398,14 +398,6 @@ export type SubstrateWorker = Message<"kagent.api.v1alpha1.SubstrateWorker"> & { * @generated from field: int64 version = 8; */ version: bigint; - - /** - * What the actor on this pod is doing, so a reader of the worker list is not - * sent to the actor list to find out. Empty when the pod holds nothing. - * - * @generated from field: string actor_status = 9; - */ - actorStatus: string; }; /** diff --git a/ui/src/mocks/fixtures.ts b/ui/src/mocks/fixtures.ts index f61f3bfe2..67c213acd 100644 --- a/ui/src/mocks/fixtures.ts +++ b/ui/src/mocks/fixtures.ts @@ -336,9 +336,6 @@ export const mockSubstrateStatus: SubstrateStatusResponse = { actorNamespace: "kagent", actorTemplate: "coder-template", actorId: "actor-7f21", - // The controller carries the actor's status onto the worker, so the page never - // has to join two paged reads to colour a pod. - actorStatus: "Running", ip: "10.42.1.19", version: 4, }, diff --git a/ui/src/mocks/transport.ts b/ui/src/mocks/transport.ts index c218fe944..e1b353aa7 100644 --- a/ui/src/mocks/transport.ts +++ b/ui/src/mocks/transport.ts @@ -1395,7 +1395,6 @@ on(SystemService.method.getSubstrateStatus, (input, call) => { actorNamespace: worker.actorNamespace ?? "", actorTemplate: worker.actorTemplate ?? "", actorId: worker.actorId ?? "", - actorStatus: worker.actorStatus ?? "", ip: worker.ip ?? "", version: BigInt(worker.version ?? 0), })), diff --git a/ui/src/pages/SubstratePage.tsx b/ui/src/pages/SubstratePage.tsx index bceb261cf..209fe6538 100644 --- a/ui/src/pages/SubstratePage.tsx +++ b/ui/src/pages/SubstratePage.tsx @@ -272,9 +272,6 @@ const ACTOR_STATES = [ "Unknown", ]; -/** The same, for a worker: whatever is on it, or nothing. */ -const WORKER_STATES = ["Idle", ...ACTOR_STATES]; - /** * How many actors the bar will draw one segment each for. * @@ -1212,31 +1209,6 @@ export function SubstratePage() { () => (workers.error ? [] : (workers.data?.workers ?? [])), [workers.error, workers.data?.workers], ); - - /* - * The workers bar: one segment per pod, coloured by what the actor on it is doing. - * - * The status comes from the worker entry itself, which the controller fills by joining - * the two whole lists. Derived here from the loaded actors instead, it moved whenever - * the *actors* table was searched, sorted or paged — narrowing one list recoloured the - * other, and a pod whose actor was off the page read as a status no cluster reports. - */ - const workerBar = useMemo(() => { - const byStatus = new Map<string, number>(); - for (const worker of workerRows) { - const status = worker.actorId ? (worker.actorStatus || "Unknown") : "Idle"; - byStatus.set(status, (byStatus.get(status) ?? 0) + 1); - } - const counts = [...byStatus].map(([status, count]) => ({ status, count })); - const matches = workers.data?.totalSize ?? workerRows.length; - if (!workerFilter && workerRows.length >= matches) return { counts, caption: undefined }; - const shown = `${atAGlance(workerRows.length)} of ${atAGlance(matches)} shown`; - return { - counts, - caption: workerFilter ? `Matching “${workerFilter}”: ${shown}` : shown, - }; - }, [workerRows, workerFilter, workers.data?.totalSize]); - /* * The tiles, from the summary's own counts. * @@ -1945,24 +1917,7 @@ export function SubstratePage() { /> ) : null} - <StatusBar - testId="substrate-worker-status-counts" - title="Worker status" - vocabulary={WORKER_STATES} - noun="Workers" - unread={Boolean(summary.error || workers.error)} - counts={workerBar.counts} - caption={workerBar.caption} - emptyText={ - workerFilter - ? "No workers match your search." - : ateApiEnabled - ? "No workers in this scope." - : "ate-api is not configured, so there are no workers to show." - } - /> - - <Table<SubstrateWorkerEntry> +<Table<SubstrateWorkerEntry> data-testid="substrate-workers-table" rowKey={(worker) => `${worker.workerNamespace}/${worker.workerPool}/${worker.workerPod}`