mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): delink agent health from script failures (#26124)
This commit is contained in:
@@ -216,7 +216,7 @@ export const ConnectingWithStartupLogs: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
|
||||
// Agent is connecting (hasAgentIssues=true) but no script has failed.
|
||||
// Agent is connecting (hasConnectivityIssues=true) but no script has failed.
|
||||
// Old code snapped to the Startup Script tab; the fix keeps us on All Logs.
|
||||
const allLogsTab = await canvas.findByRole("tab", { name: "All Logs" });
|
||||
await waitFor(() =>
|
||||
|
||||
@@ -58,7 +58,7 @@ import {
|
||||
import { useProxy } from "#/contexts/ProxyContext";
|
||||
import { useClipboard } from "#/hooks/useClipboard";
|
||||
import { useFeatureVisibility } from "#/modules/dashboard/useFeatureVisibility";
|
||||
import { getAgentHealthIssues } from "#/modules/workspaces/health";
|
||||
import { getAgentConnectivityIssues } from "#/modules/workspaces/health";
|
||||
import { AgentAlert } from "#/pages/WorkspacePage/AgentAlert";
|
||||
import { AppStatuses } from "#/pages/WorkspacePage/AppStatuses";
|
||||
import { cn } from "#/utils/cn";
|
||||
@@ -104,9 +104,11 @@ const statusBorderClassByLifecycle: Partial<
|
||||
starting: "border-border-pending",
|
||||
shutting_down: "border-border-pending",
|
||||
ready: "border-border-success",
|
||||
start_timeout: "border-border-warning",
|
||||
// Script errors and timeouts do not affect agent connectivity; they are
|
||||
// surfaced in the per-script log tabs instead.
|
||||
start_timeout: "border-border-success",
|
||||
shutdown_timeout: "border-border-warning",
|
||||
start_error: "border-border-warning",
|
||||
start_error: "border-border-success",
|
||||
shutdown_error: "border-border-warning",
|
||||
off: "border-border",
|
||||
};
|
||||
@@ -161,13 +163,16 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
const runningScriptsCount = agent.scripts.filter(
|
||||
(s) => s.run_on_start && !s.status,
|
||||
).length;
|
||||
const healthIssues = getAgentHealthIssues(agent);
|
||||
const hasAgentIssues = healthIssues.length > 0;
|
||||
const hasWarningIssues = healthIssues.some((i) => i.severity === "warning");
|
||||
// Use connectivity issues for agent panel styling and visibility decisions.
|
||||
// Script issues are handled separately in the log tabs.
|
||||
const connectivityIssues = getAgentConnectivityIssues(agent);
|
||||
const hasConnectivityIssues = connectivityIssues.length > 0;
|
||||
const hasWarningConnectivityIssues = connectivityIssues.some(
|
||||
(i) => i.severity === "warning",
|
||||
);
|
||||
const { proxy } = useProxy();
|
||||
const [showLogs, setShowLogs] = useState(
|
||||
(["starting", "start_timeout"].includes(agent.lifecycle_state) ||
|
||||
hasAgentIssues) &&
|
||||
(agent.lifecycle_state !== "ready" || hasConnectivityIssues) &&
|
||||
hasStartupFeatures,
|
||||
);
|
||||
const agentLogs = useAgentLogs({ agentId: agent.id, enabled: showLogs });
|
||||
@@ -177,10 +182,10 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
|
||||
useEffect(() => {
|
||||
setShowLogs(
|
||||
(agent.lifecycle_state !== "ready" || hasAgentIssues) &&
|
||||
(agent.lifecycle_state !== "ready" || hasConnectivityIssues) &&
|
||||
hasStartupFeatures,
|
||||
);
|
||||
}, [agent.lifecycle_state, hasAgentIssues, hasStartupFeatures]);
|
||||
}, [agent.lifecycle_state, hasConnectivityIssues, hasStartupFeatures]);
|
||||
|
||||
// This is a layout effect to remove flicker when we're scrolling to the bottom.
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: consider refactoring
|
||||
@@ -331,7 +336,8 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
...sortedSourceLogTabs,
|
||||
];
|
||||
const hasAnyLogs = agentLogs.length > 0;
|
||||
const shouldExpandLogs = showLogs || (!hasStartupFeatures && hasAgentIssues);
|
||||
const shouldExpandLogs =
|
||||
showLogs || (!hasStartupFeatures && hasConnectivityIssues);
|
||||
const shouldShowLogsTabs = hasStartupFeatures && hasAnyLogs;
|
||||
const logTabsMeasureEnabled = shouldShowLogsTabs && showLogs;
|
||||
const {
|
||||
@@ -542,7 +548,7 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
<span>Logs</span>
|
||||
{agent.lifecycle_state === "starting" &&
|
||||
runningScriptsCount > 0 &&
|
||||
healthIssues.length === 0 && (
|
||||
connectivityIssues.length === 0 && (
|
||||
<Badge
|
||||
variant="default"
|
||||
size="xs"
|
||||
@@ -557,18 +563,18 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
<span>{runningScriptsCount}</span>
|
||||
</Badge>
|
||||
)}
|
||||
{hasAgentIssues && (
|
||||
{hasConnectivityIssues && (
|
||||
<Badge
|
||||
variant={hasWarningIssues ? "warning" : "info"}
|
||||
variant={hasWarningConnectivityIssues ? "warning" : "info"}
|
||||
size="xs"
|
||||
className="ml-1.5"
|
||||
>
|
||||
{hasWarningIssues ? (
|
||||
{hasWarningConnectivityIssues ? (
|
||||
<TriangleAlertIcon className="-ml-0.5" />
|
||||
) : (
|
||||
<InfoIcon className="-ml-0.5" />
|
||||
)}
|
||||
<span>{healthIssues.length}</span>
|
||||
<span>{connectivityIssues.length}</span>
|
||||
</Badge>
|
||||
)}
|
||||
</Button>
|
||||
@@ -579,12 +585,13 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
Collapse's `in` condition is needed here,
|
||||
or else the Spinner will also show as Collapse is closing
|
||||
*/}
|
||||
{shouldExpandLogs && !(hasAgentIssues || shouldShowLogsTabs) && (
|
||||
<Spinner size="lg" loading className="block mx-auto" />
|
||||
)}
|
||||
{hasAgentIssues && (
|
||||
{shouldExpandLogs &&
|
||||
!(hasConnectivityIssues || shouldShowLogsTabs) && (
|
||||
<Spinner size="lg" loading className="block mx-auto" />
|
||||
)}
|
||||
{hasConnectivityIssues && (
|
||||
<div className="mb-4 flex flex-col gap-3">
|
||||
{healthIssues.map((issue) => (
|
||||
{connectivityIssues.map((issue) => (
|
||||
<AgentAlert
|
||||
key={`${issue.title}-${issue.detail}`}
|
||||
{...issue}
|
||||
|
||||
@@ -54,6 +54,8 @@ export const Ready: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
// start_error no longer affects the status dot; the agent shows as Ready
|
||||
// because the error is surfaced in the per-script log tabs instead.
|
||||
export const StartupScriptFailed: Story = {
|
||||
args: {
|
||||
agent: {
|
||||
@@ -63,15 +65,12 @@ export const StartupScriptFailed: Story = {
|
||||
},
|
||||
},
|
||||
play: async () => {
|
||||
await expectTooltip(
|
||||
"Startup script failed",
|
||||
agentScriptMessages.start_error.title,
|
||||
agentScriptMessages.start_error.detail,
|
||||
true,
|
||||
);
|
||||
expect(screen.getByRole("status", { name: "Ready" })).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
// start_timeout no longer affects the status dot; the agent shows as Ready
|
||||
// because the timeout is surfaced in the per-script log tabs instead.
|
||||
export const StartupScriptTimeout: Story = {
|
||||
args: {
|
||||
agent: {
|
||||
@@ -81,12 +80,7 @@ export const StartupScriptTimeout: Story = {
|
||||
},
|
||||
},
|
||||
play: async () => {
|
||||
await expectTooltip(
|
||||
"Startup script timeout",
|
||||
agentScriptMessages.start_timeout.title,
|
||||
agentScriptMessages.start_timeout.detail,
|
||||
true,
|
||||
);
|
||||
expect(screen.getByRole("status", { name: "Ready" })).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
@@ -143,6 +137,8 @@ export const ConnectionTimeout: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
// start_error renders as Ready regardless of whether a troubleshoot URL is
|
||||
// provided, because script errors are surfaced in the per-script log tabs.
|
||||
export const StartupScriptFailedNoTroubleshootURL: Story = {
|
||||
args: {
|
||||
agent: {
|
||||
@@ -153,12 +149,7 @@ export const StartupScriptFailedNoTroubleshootURL: Story = {
|
||||
},
|
||||
},
|
||||
play: async () => {
|
||||
await expectTooltip(
|
||||
"Startup script failed",
|
||||
agentScriptMessages.start_error.title,
|
||||
agentScriptMessages.start_error.detail,
|
||||
false,
|
||||
);
|
||||
expect(screen.getByRole("status", { name: "Ready" })).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -126,25 +126,6 @@ interface DevcontainerStatusProps {
|
||||
agent?: WorkspaceAgent;
|
||||
}
|
||||
|
||||
const StartTimeoutLifecycle: FC<AgentStatusProps> = ({ agent }) => (
|
||||
<AgentWarningTooltip
|
||||
ariaLabel="Startup script timeout"
|
||||
title={agentScriptMessages.start_timeout.title}
|
||||
detail={agentScriptMessages.start_timeout.detail}
|
||||
troubleshootingURL={agent.troubleshooting_url}
|
||||
/>
|
||||
);
|
||||
|
||||
const StartErrorLifecycle: FC<AgentStatusProps> = ({ agent }) => (
|
||||
<AgentWarningTooltip
|
||||
ariaLabel="Startup script failed"
|
||||
title={agentScriptMessages.start_error.title}
|
||||
detail={agentScriptMessages.start_error.detail}
|
||||
troubleshootingURL={agent.troubleshooting_url}
|
||||
variant="warning"
|
||||
/>
|
||||
);
|
||||
|
||||
const ShuttingDownLifecycle: FC = () => {
|
||||
return (
|
||||
<Tooltip>
|
||||
@@ -203,11 +184,13 @@ const ConnectedStatus: FC<AgentStatusProps> = ({ agent }) => {
|
||||
if (agent.lifecycle_state === "ready") {
|
||||
return <ReadyLifecycle />;
|
||||
}
|
||||
if (agent.lifecycle_state === "start_timeout") {
|
||||
return <StartTimeoutLifecycle agent={agent} />;
|
||||
}
|
||||
if (agent.lifecycle_state === "start_error") {
|
||||
return <StartErrorLifecycle agent={agent} />;
|
||||
// Script errors and timeouts do not affect agent connectivity.
|
||||
// These states are surfaced in the per-script log tabs instead.
|
||||
if (
|
||||
agent.lifecycle_state === "start_timeout" ||
|
||||
agent.lifecycle_state === "start_error"
|
||||
) {
|
||||
return <ReadyLifecycle />;
|
||||
}
|
||||
if (agent.lifecycle_state === "shutting_down") {
|
||||
return <ShuttingDownLifecycle />;
|
||||
|
||||
@@ -9,7 +9,11 @@ import {
|
||||
MockWorkspaceAgentStartError,
|
||||
MockWorkspaceAgentStartTimeout,
|
||||
} from "#/testHelpers/entities";
|
||||
import { getAgentHealthIssues } from "./health";
|
||||
import {
|
||||
getAgentConnectivityIssues,
|
||||
getAgentHealthIssues,
|
||||
getAgentScriptIssues,
|
||||
} from "./health";
|
||||
|
||||
interface AgentOverrides {
|
||||
status?: WorkspaceAgentStatus;
|
||||
@@ -24,6 +28,128 @@ function buildAgent(overrides: AgentOverrides): WorkspaceAgent {
|
||||
};
|
||||
}
|
||||
|
||||
describe("getAgentConnectivityIssues", () => {
|
||||
it("returns disconnected issue for a disconnected agent", () => {
|
||||
expect(
|
||||
getAgentConnectivityIssues(buildAgent({ status: "disconnected" })),
|
||||
).toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: "Workspace agent has disconnected",
|
||||
severity: "warning",
|
||||
prominent: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns timeout issue for a timed-out agent", () => {
|
||||
expect(
|
||||
getAgentConnectivityIssues(buildAgent({ status: "timeout" })),
|
||||
).toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: "Agent is taking longer than expected to connect",
|
||||
severity: "warning",
|
||||
prominent: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not return script issues", () => {
|
||||
const issues = getAgentConnectivityIssues(
|
||||
buildAgent(MockWorkspaceAgentStartError),
|
||||
);
|
||||
expect(issues).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: `"Startup Script" failed`,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns empty list for healthy ready connected agent", () => {
|
||||
expect(
|
||||
getAgentConnectivityIssues(
|
||||
buildAgent({ status: "connected", lifecycle_state: "ready" }),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns connecting issue for a connecting agent", () => {
|
||||
expect(
|
||||
getAgentConnectivityIssues(
|
||||
buildAgent({ status: "connecting", lifecycle_state: "starting" }),
|
||||
),
|
||||
).toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: "Workspace agent is connecting",
|
||||
severity: "info",
|
||||
prominent: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns shutdown issue for shutdown lifecycle states", () => {
|
||||
for (const lifecycle_state of [
|
||||
"shutting_down",
|
||||
"shutdown_error",
|
||||
"shutdown_timeout",
|
||||
] as const) {
|
||||
expect(
|
||||
getAgentConnectivityIssues(buildAgent({ lifecycle_state })),
|
||||
).toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: "Workspace agent is shutting down",
|
||||
severity: "info",
|
||||
}),
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("getAgentScriptIssues", () => {
|
||||
it("returns script issues", () => {
|
||||
const issues = getAgentScriptIssues(
|
||||
buildAgent(MockWorkspaceAgentStartError),
|
||||
);
|
||||
expect(issues).toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: `"Startup Script" failed`,
|
||||
severity: "warning",
|
||||
prominent: false,
|
||||
}),
|
||||
);
|
||||
expect(issues).toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: `"time" is taking longer than expected`,
|
||||
severity: "warning",
|
||||
prominent: false,
|
||||
}),
|
||||
);
|
||||
expect(issues).toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: `"pipe" left pipes open`,
|
||||
severity: "warning",
|
||||
prominent: false,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not return connectivity issues", () => {
|
||||
const issues = getAgentScriptIssues(buildAgent({ status: "disconnected" }));
|
||||
expect(issues).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
title: "Workspace agent has disconnected",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns empty list when no scripts failed", () => {
|
||||
expect(
|
||||
getAgentScriptIssues(
|
||||
buildAgent({ status: "connected", lifecycle_state: "ready" }),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getAgentHealthIssues", () => {
|
||||
it("returns disconnected issue for a disconnected agent", () => {
|
||||
expect(
|
||||
|
||||
@@ -4,7 +4,7 @@ import type { WorkspaceAgent } from "#/api/typesGenerated";
|
||||
* Canonical messages for startup and shutdown script issues.
|
||||
* Used by the per-agent-row tooltips in AgentStatus; the
|
||||
* start-related entries are also shared with per-agent health
|
||||
* classification in getAgentHealthIssues.
|
||||
* classification in getAgentScriptIssues.
|
||||
*/
|
||||
export const agentScriptMessages = {
|
||||
start_error: {
|
||||
@@ -62,9 +62,12 @@ interface AgentHealthIssue {
|
||||
}
|
||||
|
||||
/**
|
||||
* Classifies all health issues for an individual agent.
|
||||
* Classifies connectivity-related health issues for an individual agent.
|
||||
* These issues affect the agent panel border color and warning visibility.
|
||||
* Script failures are excluded from this function to prevent them from
|
||||
* incorrectly suggesting agent connectivity problems.
|
||||
*/
|
||||
export function getAgentHealthIssues(
|
||||
export function getAgentConnectivityIssues(
|
||||
agent: WorkspaceAgent,
|
||||
): AgentHealthIssue[] {
|
||||
const issues: AgentHealthIssue[] = [];
|
||||
@@ -87,6 +90,10 @@ export function getAgentHealthIssues(
|
||||
});
|
||||
}
|
||||
|
||||
// Shutdown lifecycle states are treated as connectivity concerns rather than
|
||||
// script concerns because the workspace is actively becoming unavailable;
|
||||
// unlike startup script failures, the agent is no longer reachable once
|
||||
// shutdown begins.
|
||||
if (
|
||||
agent.lifecycle_state === "shutting_down" ||
|
||||
agent.lifecycle_state === "shutdown_error" ||
|
||||
@@ -100,9 +107,28 @@ export function getAgentHealthIssues(
|
||||
});
|
||||
}
|
||||
|
||||
// Ignore `start_error` and `start_timeout`, as these will eventually be
|
||||
// removed from agent health. Instead, figure out if a script failed to start
|
||||
// by looking directly at the scripts.
|
||||
if (agent.status === "connecting") {
|
||||
issues.push({
|
||||
title: agentConnectionMessages.connecting.title,
|
||||
detail: agentConnectionMessages.connecting.detail,
|
||||
severity: "info",
|
||||
prominent: false,
|
||||
});
|
||||
}
|
||||
|
||||
return issues;
|
||||
}
|
||||
|
||||
/**
|
||||
* Classifies script-related health issues for an individual agent.
|
||||
* These issues are shown only in the script tabs, not in the agent panel header.
|
||||
*/
|
||||
export function getAgentScriptIssues(
|
||||
agent: WorkspaceAgent,
|
||||
): AgentHealthIssue[] {
|
||||
const issues: AgentHealthIssue[] = [];
|
||||
|
||||
// Check for script failures directly from the scripts array.
|
||||
for (const script of agent.scripts) {
|
||||
switch (script.status) {
|
||||
case "timed_out":
|
||||
@@ -141,14 +167,15 @@ export function getAgentHealthIssues(
|
||||
}
|
||||
}
|
||||
|
||||
if (agent.status === "connecting") {
|
||||
issues.push({
|
||||
title: agentConnectionMessages.connecting.title,
|
||||
detail: agentConnectionMessages.connecting.detail,
|
||||
severity: "info",
|
||||
prominent: false,
|
||||
});
|
||||
}
|
||||
|
||||
return issues;
|
||||
}
|
||||
|
||||
/**
|
||||
* Classifies all health issues for an individual agent, combining both
|
||||
* connectivity and script issues.
|
||||
*/
|
||||
export function getAgentHealthIssues(
|
||||
agent: WorkspaceAgent,
|
||||
): AgentHealthIssue[] {
|
||||
return [...getAgentConnectivityIssues(agent), ...getAgentScriptIssues(agent)];
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user