mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(panel): agent detail survives a stopped agent — fatal card only on a real roster miss
GET /orchestrator/agents/{id} 404s for any non-running container, and
the page's error early-return unmounted the DB-backed header/activity
content #529 had placed behind it. The fatal card now gates on the
roster identity lookup; a live-status error degrades in place to a
not-running banner + spawn dialog, with retry disabled on the
deterministic 404.
This commit is contained in:
@@ -6,6 +6,13 @@ import { render, screen } from "@testing-library/react";
|
|||||||
// spawnAgent.mutateAsync({ agentId }) directly from a bare button — no task,
|
// spawnAgent.mutateAsync({ agentId }) directly from a bare button — no task,
|
||||||
// no message, no double-fire guard. Both bare buttons must now render the
|
// no message, no double-fire guard. Both bare buttons must now render the
|
||||||
// shared SpawnAgentDialog instead.
|
// shared SpawnAgentDialog instead.
|
||||||
|
//
|
||||||
|
// Whole-page-replaced-by-error fix: a stopped agent's live-status query 404s
|
||||||
|
// (orchestrator has no running instance) while the agent's roster identity
|
||||||
|
// still resolves fine. That must degrade the live-status area only — not
|
||||||
|
// discard the DB-backed header/activity content already rendered above it.
|
||||||
|
// The full-page fatal card is reserved for a genuinely invalid agent id
|
||||||
|
// (the roster lookup itself failing).
|
||||||
|
|
||||||
vi.mock("next/navigation", () => ({
|
vi.mock("next/navigation", () => ({
|
||||||
useParams: () => ({ agentId: "fe-dev-2" }),
|
useParams: () => ({ agentId: "fe-dev-2" }),
|
||||||
@@ -24,15 +31,19 @@ vi.mock("@/hooks/use-page-refresh", () => ({
|
|||||||
|
|
||||||
vi.mock("@/hooks/use-agents", () => ({
|
vi.mock("@/hooks/use-agents", () => ({
|
||||||
useAgentStatus: vi.fn(),
|
useAgentStatus: vi.fn(),
|
||||||
useAgentDefinition: vi.fn(() => ({ data: undefined })),
|
useAgentDefinition: vi.fn(() => ({
|
||||||
|
data: undefined,
|
||||||
|
isLoading: false,
|
||||||
|
error: undefined,
|
||||||
|
})),
|
||||||
useStopAgent: vi.fn(() => ({ mutateAsync: vi.fn() })),
|
useStopAgent: vi.fn(() => ({ mutateAsync: vi.fn() })),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock("@/components/agents", () => ({
|
vi.mock("@/components/agents", () => ({
|
||||||
AgentStatusCards: () => null,
|
AgentStatusCards: () => <div data-testid="agent-status-cards" />,
|
||||||
ResolveWaitDialog: () => null,
|
ResolveWaitDialog: () => null,
|
||||||
AgentStreamViewer: () => null,
|
AgentStreamViewer: () => null,
|
||||||
AgentActivityPanel: () => null,
|
AgentActivityPanel: () => <div data-testid="agent-activity-panel" />,
|
||||||
SpawnAgentDialog: ({
|
SpawnAgentDialog: ({
|
||||||
agentId,
|
agentId,
|
||||||
agentName,
|
agentName,
|
||||||
@@ -52,27 +63,10 @@ vi.mock("@/components/agents", () => ({
|
|||||||
),
|
),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
import { useAgentStatus } from "@/hooks/use-agents";
|
import { useAgentStatus, useAgentDefinition } from "@/hooks/use-agents";
|
||||||
import AgentDetailPage from "../page";
|
import AgentDetailPage from "../page";
|
||||||
|
|
||||||
describe("AgentDetailPage — spawn dialog parity", () => {
|
describe("AgentDetailPage — spawn dialog parity", () => {
|
||||||
it("renders SpawnAgentDialog (not a bare button) in the error state", () => {
|
|
||||||
vi.mocked(useAgentStatus).mockReturnValue({
|
|
||||||
data: undefined,
|
|
||||||
isLoading: false,
|
|
||||||
error: new Error("not found"),
|
|
||||||
refetch: vi.fn(),
|
|
||||||
} as unknown as ReturnType<typeof useAgentStatus>);
|
|
||||||
|
|
||||||
render(<AgentDetailPage />);
|
|
||||||
|
|
||||||
const dialog = screen.getByTestId("spawn-agent-dialog");
|
|
||||||
expect(dialog).toHaveAttribute("data-agent-id", "fe-dev-2");
|
|
||||||
expect(
|
|
||||||
screen.getByRole("button", { name: /Spawn Agent/i }),
|
|
||||||
).toBeInTheDocument();
|
|
||||||
});
|
|
||||||
|
|
||||||
it("renders SpawnAgentDialog in the header when the agent is not active", () => {
|
it("renders SpawnAgentDialog in the header when the agent is not active", () => {
|
||||||
vi.mocked(useAgentStatus).mockReturnValue({
|
vi.mocked(useAgentStatus).mockReturnValue({
|
||||||
data: {
|
data: {
|
||||||
@@ -99,3 +93,81 @@ describe("AgentDetailPage — spawn dialog parity", () => {
|
|||||||
).not.toBeInTheDocument();
|
).not.toBeInTheDocument();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("AgentDetailPage — live-status error degrades, doesn't discard content", () => {
|
||||||
|
it("keeps header + activity panel and shows a not-running banner when the roster resolves but live status 404s", () => {
|
||||||
|
vi.mocked(useAgentDefinition).mockReturnValue({
|
||||||
|
data: { id: "fe-dev-2", uuid: "uuid-1", name: "Frontend Dev 2" },
|
||||||
|
isLoading: false,
|
||||||
|
error: undefined,
|
||||||
|
} as unknown as ReturnType<typeof useAgentDefinition>);
|
||||||
|
vi.mocked(useAgentStatus).mockReturnValue({
|
||||||
|
data: undefined,
|
||||||
|
isLoading: false,
|
||||||
|
error: new Error("Agent fe-dev-2 not found"),
|
||||||
|
refetch: vi.fn(),
|
||||||
|
} as unknown as ReturnType<typeof useAgentStatus>);
|
||||||
|
|
||||||
|
render(<AgentDetailPage />);
|
||||||
|
|
||||||
|
// DB-backed content that loaded independently of live status must stay.
|
||||||
|
expect(screen.getByText("Frontend Dev 2")).toBeInTheDocument();
|
||||||
|
expect(screen.getByTestId("agent-activity-panel")).toBeInTheDocument();
|
||||||
|
|
||||||
|
// Live-status area degrades to an inline banner, not a whole-page card.
|
||||||
|
expect(screen.getByText("Not running")).toBeInTheDocument();
|
||||||
|
expect(
|
||||||
|
screen.queryByText("Failed to load agent status"),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
expect(screen.queryByTestId("agent-status-cards")).not.toBeInTheDocument();
|
||||||
|
expect(
|
||||||
|
screen.getByRole("button", { name: /Spawn Agent/i }),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows the whole-page fatal card only when the roster lookup itself fails (invalid id)", () => {
|
||||||
|
vi.mocked(useAgentDefinition).mockReturnValue({
|
||||||
|
data: undefined,
|
||||||
|
isLoading: false,
|
||||||
|
error: new Error("Agent not found"),
|
||||||
|
} as unknown as ReturnType<typeof useAgentDefinition>);
|
||||||
|
vi.mocked(useAgentStatus).mockReturnValue({
|
||||||
|
data: undefined,
|
||||||
|
isLoading: false,
|
||||||
|
error: new Error("Agent not found"),
|
||||||
|
refetch: vi.fn(),
|
||||||
|
} as unknown as ReturnType<typeof useAgentStatus>);
|
||||||
|
|
||||||
|
render(<AgentDetailPage />);
|
||||||
|
|
||||||
|
expect(screen.getByText("Failed to load agent status")).toBeInTheDocument();
|
||||||
|
expect(
|
||||||
|
screen.queryByTestId("agent-activity-panel"),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
expect(
|
||||||
|
screen.getByRole("button", { name: /Spawn Agent/i }),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows the status skeleton (not the banner) while the roster is still loading, even if status already errored", () => {
|
||||||
|
vi.mocked(useAgentDefinition).mockReturnValue({
|
||||||
|
data: undefined,
|
||||||
|
isLoading: true,
|
||||||
|
error: undefined,
|
||||||
|
} as unknown as ReturnType<typeof useAgentDefinition>);
|
||||||
|
vi.mocked(useAgentStatus).mockReturnValue({
|
||||||
|
data: undefined,
|
||||||
|
isLoading: false,
|
||||||
|
error: new Error("Agent not found"),
|
||||||
|
refetch: vi.fn(),
|
||||||
|
} as unknown as ReturnType<typeof useAgentStatus>);
|
||||||
|
|
||||||
|
render(<AgentDetailPage />);
|
||||||
|
|
||||||
|
// Definition still resolving — not fatal yet, page renders normally.
|
||||||
|
expect(
|
||||||
|
screen.queryByText("Failed to load agent status"),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
expect(screen.getByText("Not running")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -65,8 +65,17 @@ export default function AgentDetailPage() {
|
|||||||
const router = useRouter();
|
const router = useRouter();
|
||||||
const agentId = params.agentId as string;
|
const agentId = params.agentId as string;
|
||||||
|
|
||||||
const { data: agent, isLoading, error, refetch } = useAgentStatus(agentId);
|
const {
|
||||||
const { data: definition } = useAgentDefinition(agentId);
|
data: agent,
|
||||||
|
isLoading: isStatusLoading,
|
||||||
|
error: statusError,
|
||||||
|
refetch,
|
||||||
|
} = useAgentStatus(agentId);
|
||||||
|
const {
|
||||||
|
data: definition,
|
||||||
|
isLoading: isDefinitionLoading,
|
||||||
|
error: definitionError,
|
||||||
|
} = useAgentDefinition(agentId);
|
||||||
|
|
||||||
const { register, unregister } = usePageRefresh();
|
const { register, unregister } = usePageRefresh();
|
||||||
|
|
||||||
@@ -100,7 +109,13 @@ export default function AgentDetailPage() {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
if (error) {
|
// Fatal only when the agent identity itself doesn't resolve (roster lookup
|
||||||
|
// failed) — a genuinely invalid id. A live-status error (agent not running)
|
||||||
|
// is a normal, expected state for a stopped agent and must not discard the
|
||||||
|
// DB-backed content below (activity panel, header) — see isAgentDown.
|
||||||
|
const isInvalidAgent = !!definitionError && !isDefinitionLoading;
|
||||||
|
|
||||||
|
if (isInvalidAgent) {
|
||||||
return (
|
return (
|
||||||
<div className="space-y-6">
|
<div className="space-y-6">
|
||||||
<Button variant="ghost" onClick={() => router.back()}>
|
<Button variant="ghost" onClick={() => router.back()}>
|
||||||
@@ -132,6 +147,11 @@ export default function AgentDetailPage() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Live status unreachable (stopped/unreachable agent, 404 from the
|
||||||
|
// orchestrator) but the agent identity is real — degrade in place instead
|
||||||
|
// of replacing the page.
|
||||||
|
const isAgentDown = !!statusError && !isStatusLoading;
|
||||||
|
|
||||||
const isActive =
|
const isActive =
|
||||||
agent &&
|
agent &&
|
||||||
["running", "ready", "starting", "waiting_long"].includes(agent.state);
|
["running", "ready", "starting", "waiting_long"].includes(agent.state);
|
||||||
@@ -201,7 +221,7 @@ export default function AgentDetailPage() {
|
|||||||
History exists independent of live status, so render for any slug. */}
|
History exists independent of live status, so render for any slug. */}
|
||||||
<AgentActivityPanel agentSlug={agentId} agentUuid={definition?.uuid} />
|
<AgentActivityPanel agentSlug={agentId} agentUuid={definition?.uuid} />
|
||||||
|
|
||||||
{isLoading ? (
|
{isStatusLoading ? (
|
||||||
<div className="grid gap-4 md:grid-cols-2 lg:grid-cols-4">
|
<div className="grid gap-4 md:grid-cols-2 lg:grid-cols-4">
|
||||||
{Array.from({ length: 4 }).map((_, i) => (
|
{Array.from({ length: 4 }).map((_, i) => (
|
||||||
<Card key={i}>
|
<Card key={i}>
|
||||||
@@ -214,6 +234,29 @@ export default function AgentDetailPage() {
|
|||||||
</Card>
|
</Card>
|
||||||
))}
|
))}
|
||||||
</div>
|
</div>
|
||||||
|
) : isAgentDown ? (
|
||||||
|
<Card className="border-muted-foreground/30">
|
||||||
|
<CardContent className="pt-6">
|
||||||
|
<div className="flex items-center gap-2 text-muted-foreground">
|
||||||
|
<AlertTriangle className="h-5 w-5" />
|
||||||
|
<span>Not running</span>
|
||||||
|
</div>
|
||||||
|
<p className="text-muted-foreground mt-2">
|
||||||
|
Live status is unavailable — this agent isn't currently
|
||||||
|
active. Spawn it to see live state.
|
||||||
|
</p>
|
||||||
|
<SpawnAgentDialog
|
||||||
|
agentId={agentId}
|
||||||
|
agentName={displayName}
|
||||||
|
trigger={
|
||||||
|
<Button className="mt-4">
|
||||||
|
<Play className="h-4 w-4 mr-2" />
|
||||||
|
Spawn Agent
|
||||||
|
</Button>
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
</CardContent>
|
||||||
|
</Card>
|
||||||
) : agent ? (
|
) : agent ? (
|
||||||
<>
|
<>
|
||||||
{/* Status Cards */}
|
{/* Status Cards */}
|
||||||
|
|||||||
@@ -337,6 +337,9 @@ export function useAgentStatus(agentId: string) {
|
|||||||
queryFn: () => orchestratorApi.getAgentStatus(agentId),
|
queryFn: () => orchestratorApi.getAgentStatus(agentId),
|
||||||
enabled: !!agentId,
|
enabled: !!agentId,
|
||||||
refetchInterval: 5000, // Refetch every 5 seconds
|
refetchInterval: 5000, // Refetch every 5 seconds
|
||||||
|
// A 404 (agent not running) is deterministic, not transient — retrying
|
||||||
|
// just delays settling into the degraded "not running" state.
|
||||||
|
retry: false,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user