From 5d95286d349a96d8e4f0605cbfbb3f4e4b7808c5 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Tue, 23 Jun 2026 20:47:03 +0100 Subject: [PATCH 1/2] feat(observatory): surface server rejections from the steer controls Follow-up to the kilo review on #1418/#1420: pause, global cap, and per-lane cap all posted optimistically without inspecting res.ok, so a 4xx/5xx (forbidden, unknown lane, rejected value) left the optimistic value standing with no signal. Route all three through a shared postSteer that checks res.ok, shows a dismissible error banner on failure, clears it on the next success, and always reconciles against the server. Done uniformly so the three controls stay consistent rather than patching one. Two new tests (error shown on a rejected post; cleared after a later success); the existing 11 pass through the refactor. --- desktop/src/apps/ObservatoryApp.test.tsx | 52 ++++++++++++ desktop/src/apps/ObservatoryApp.tsx | 101 ++++++++++++++--------- 2 files changed, 113 insertions(+), 40 deletions(-) diff --git a/desktop/src/apps/ObservatoryApp.test.tsx b/desktop/src/apps/ObservatoryApp.test.tsx index 1a02a64e..2bd32250 100644 --- a/desktop/src/apps/ObservatoryApp.test.tsx +++ b/desktop/src/apps/ObservatoryApp.test.tsx @@ -232,6 +232,58 @@ describe("ObservatoryApp", () => { }); }); + it("surfaces an error when a steer post is rejected by the server", async () => { + const fetchMock = mockFetch({ + "GET /api/observatory/fleet": { ok: true, body: fleetBody }, + "GET /api/observatory/throttle": { ok: true, body: { global: null, lanes: {} } }, + "POST /api/observatory/pause": { ok: false, status: 403, body: { detail: "forbidden" } }, + }); + vi.stubGlobal("fetch", fetchMock); + render(); + await flush(); + + fireEvent.click(screen.getByRole("button", { name: /pause queue/i })); + await flush(); + + await waitFor(() => + expect(screen.getByText(/could not update the pause state/i)).toBeTruthy(), + ); + }); + + it("clears the steer error after a subsequent successful post", async () => { + let pauseOk = false; + const fetchMock = vi.fn().mockImplementation((input: string, init?: RequestInit) => { + const method = (init?.method ?? "GET").toUpperCase(); + if (input === "/api/observatory/fleet") { + return Promise.resolve({ ok: true, status: 200, json: () => Promise.resolve(fleetBody) }); + } + if (input === "/api/observatory/throttle") { + return Promise.resolve({ ok: true, status: 200, json: () => Promise.resolve({ global: null, lanes: {} }) }); + } + if (method === "POST" && input === "/api/observatory/pause") { + const ok = pauseOk; + pauseOk = true; // first attempt fails, the next succeeds + return Promise.resolve({ ok, status: ok ? 200 : 403, json: () => Promise.resolve({}) }); + } + throw new Error(`Unmocked fetch: ${method} ${input}`); + }); + vi.stubGlobal("fetch", fetchMock); + render(); + await flush(); + + fireEvent.click(screen.getByRole("button", { name: /pause queue/i })); + await flush(); + await waitFor(() => + expect(screen.getByText(/could not update the pause state/i)).toBeTruthy(), + ); + + fireEvent.click(screen.getByRole("button", { name: /pause queue/i })); + await flush(); + await waitFor(() => + expect(screen.queryByText(/could not update the pause state/i)).toBeNull(), + ); + }); + it("shows the idle empty state when no agents are working", async () => { vi.stubGlobal( "fetch", diff --git a/desktop/src/apps/ObservatoryApp.tsx b/desktop/src/apps/ObservatoryApp.tsx index ca05eb62..ecfe1c99 100644 --- a/desktop/src/apps/ObservatoryApp.tsx +++ b/desktop/src/apps/ObservatoryApp.tsx @@ -1,5 +1,5 @@ import { useState, useEffect, useCallback } from "react"; -import { Radar, Pause, Play, Loader2, CircleDot, Minus, Plus } from "lucide-react"; +import { Radar, Pause, Play, Loader2, CircleDot, Minus, Plus, AlertCircle } from "lucide-react"; import { Switch } from "@/components/ui"; interface HeldCard { @@ -94,6 +94,7 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) { const [laneCaps, setLaneCaps] = useState>({}); const [loading, setLoading] = useState(true); const [busy, setBusy] = useState(null); + const [steerError, setSteerError] = useState(null); const load = useCallback(async (opts?: { silent?: boolean }) => { if (!opts?.silent) setLoading(true); @@ -128,6 +129,27 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) { return () => clearInterval(id); }, [load]); + // Shared write path for every steer control. Posts the change, surfaces a + // visible error if the server rejects it (so an optimistic value is not left + // standing silently), and always reconciles against the server. + const postSteer = useCallback( + async (url: string, body: object, failMsg: string) => { + try { + const res = await fetch(url, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + setSteerError(res.ok ? null : failMsg); + } catch { + setSteerError(failMsg); + } finally { + await load({ silent: true }); + } + }, + [load], + ); + const setScope = useCallback( async (scope: string, paused: boolean) => { setBusy(scope); @@ -140,40 +162,28 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) { lanes: { ...prev.lanes, [scope]: paused }, }, ); - try { - await fetch("/api/observatory/pause", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ scope, paused }), - }); - await load({ silent: true }); - } catch { - await load({ silent: true }); - } finally { - setBusy(null); - } + await postSteer( + "/api/observatory/pause", + { scope, paused }, + "Could not update the pause state.", + ); + setBusy(null); }, - [load], + [postSteer], ); const setGlobalCap = useCallback( async (next: number | null) => { setBusy("cap"); setCap(next); // optimistic; reconciled on the next poll - try { - await fetch("/api/observatory/throttle", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ scope: "global", max_concurrent: next }), - }); - await load({ silent: true }); - } catch { - await load({ silent: true }); - } finally { - setBusy(null); - } + await postSteer( + "/api/observatory/throttle", + { scope: "global", max_concurrent: next }, + "Could not update the concurrency cap.", + ); + setBusy(null); }, - [load], + [postSteer], ); const setLaneCap = useCallback( @@ -185,20 +195,14 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) { else copy[handle] = next; return copy; }); - try { - await fetch("/api/observatory/throttle", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ scope: handle, max_concurrent: next }), - }); - await load({ silent: true }); - } catch { - await load({ silent: true }); - } finally { - setBusy(null); - } + await postSteer( + "/api/observatory/throttle", + { scope: handle, max_concurrent: next }, + `Could not update the cap for ${handle}.`, + ); + setBusy(null); }, - [load], + [postSteer], ); return ( @@ -234,6 +238,23 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) { )} + {steerError && ( +
+ + {steerError} + +
+ )} + {/* Steer: global concurrency cap (volume knob alongside the pause switch) */}
From 1ac40f98f2117618c327486adafd0e5cd59f2920 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Tue, 23 Jun 2026 20:58:20 +0100 Subject: [PATCH 2/2] fix(observatory): guard the steer error banner against out-of-order writes Fold the CodeRabbit Minor: steer controls are not fully serialized (pause and a cap change use different busy scopes), so two postSteer calls can resolve out of order and a stale result could overwrite a newer one's banner. Stamp each call with a sequence number and only let the latest one set steerError. Deferred the two kilo nits (surfacing the server's specific reason, and distinguishing a network failure from a server rejection) as polish; the generic message is an intentional, accurate choice. --- desktop/src/apps/ObservatoryApp.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/desktop/src/apps/ObservatoryApp.tsx b/desktop/src/apps/ObservatoryApp.tsx index ecfe1c99..fe566da1 100644 --- a/desktop/src/apps/ObservatoryApp.tsx +++ b/desktop/src/apps/ObservatoryApp.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useCallback, useRef } from "react"; import { Radar, Pause, Play, Loader2, CircleDot, Minus, Plus, AlertCircle } from "lucide-react"; import { Switch } from "@/components/ui"; @@ -131,18 +131,23 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) { // Shared write path for every steer control. Posts the change, surfaces a // visible error if the server rejects it (so an optimistic value is not left - // standing silently), and always reconciles against the server. + // standing silently), and always reconciles against the server. A sequence + // guard ensures only the latest write controls the banner: steer controls are + // not fully serialized (pause and a cap change use different busy scopes), so + // an earlier slow response must not overwrite a newer one's result. + const steerSeq = useRef(0); const postSteer = useCallback( async (url: string, body: object, failMsg: string) => { + const seq = ++steerSeq.current; try { const res = await fetch(url, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(body), }); - setSteerError(res.ok ? null : failMsg); + if (seq === steerSeq.current) setSteerError(res.ok ? null : failMsg); } catch { - setSteerError(failMsg); + if (seq === steerSeq.current) setSteerError(failMsg); } finally { await load({ silent: true }); }