Skip to content

feat(observatory): surface server rejections from the steer controls #1421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jaylfc merged 2 commits into from
Jun 23, 2026
Merged

feat(observatory): surface server rejections from the steer controls #1421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d95286
feat(observatory): surface server rejections from the steer controls
jaylfc Jun 23, 2026
1ac40f9
fix(observatory): guard the steer error banner against out-of-order w…
jaylfc Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions desktop/src/apps/ObservatoryApp.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(<ObservatoryApp windowId="w1" />);
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(<ObservatoryApp windowId="w1" />);
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",
Expand Down
101 changes: 61 additions & 40 deletions desktop/src/apps/ObservatoryApp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -94,6 +94,7 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) {
const [laneCaps, setLaneCaps] = useState<Record<string, number>>({});
const [loading, setLoading] = useState(true);
const [busy, setBusy] = useState<string | null>(null);
const [steerError, setSteerError] = useState<string | null>(null);

const load = useCallback(async (opts?: { silent?: boolean }) => {
if (!opts?.silent) setLoading(true);
Expand Down Expand Up @@ -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);

@kilo-code-bot kilo-code-bot Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: The server's rejection body is discarded, so the user sees only the generic failMsg even when the API returned a specific reason.

The PR description explicitly motivates this change on the cases "forbidden", "unknown lane", and "rejected value" — the whole point is that the server can tell the user why the write was refused, and currently that detail is thrown away by postSteer (no res.json() call, no body.detail extraction). The new test on line 239 even mocks body: { detail: "forbidden" }, which would be a useful string to surface — but the production code never reads it.

Consider parsing the response body on !res.ok and preferring body.detail (or body.message) over the hard-coded failMsg, with the generic message kept as a fallback when the body is missing or unparseable. This is the highest-value gap in this change.

} catch {
setSteerError(failMsg);

@kilo-code-bot kilo-code-bot Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: The catch branch reuses the same caller-supplied failMsg ("Could not update the pause state.") that the !res.ok branch uses, which is misleading for network/CORS/abort failures.

For a fetch that throws before any response is received, the user is told the server rejected the state when in reality the request never arrived. Consider a distinct message for the thrown branch (e.g. "Could not reach the server.") or, better, inline whatever detail err carries. Low priority but cheap to fix while you're already touching this path.

} finally {
await load({ silent: true });
}
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
[load],
);

const setScope = useCallback(
async (scope: string, paused: boolean) => {
setBusy(scope);
Expand All @@ -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(
Expand All @@ -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 (
Expand Down Expand Up @@ -234,6 +238,23 @@ export function ObservatoryApp({ windowId: _windowId }: { windowId: string }) {
</div>
)}

{steerError && (
<div
className="flex items-center gap-2 border-b border-red-500/20 bg-red-500/10 px-5 py-2 text-sm text-red-400"
role="alert"
>
<AlertCircle size={14} className="shrink-0" />
{steerError}
<button
type="button"
onClick={() => setSteerError(null)}
className="ml-auto text-xs text-red-400/80 transition-colors hover:text-red-400"
>
Dismiss
</button>
</div>
)}

{/* Steer: global concurrency cap (volume knob alongside the pause switch) */}
<div className="flex items-center gap-3 border-b border-shell-border px-5 py-2.5">
<span className="text-xs font-medium uppercase tracking-wide text-shell-text-tertiary">
Expand Down
Loading