From 453a39be976da7ac2a45e738f6318ad56648482d Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 1 May 2026 10:54:23 -0800 Subject: [PATCH] feat: show ui for individual failed scripts (#24506) Previously we only showed an error when the startup script failed. Now: - Add a warning icon to each failed script. - Sort the failed script tabs first. - Modify agent health messages to pull the errors notices from the new location instead of from the single agent lifecycle enum. This means errors are shown for each script and the count is accurate. --- .../modules/resources/AgentRow.stories.tsx | 74 +++------- site/src/modules/resources/AgentRow.tsx | 128 ++++++++++-------- site/src/modules/workspaces/health.test.ts | 46 +++++-- site/src/modules/workspaces/health.ts | 55 +++++--- .../WorkspacePage/AgentAlert.stories.tsx | 33 +---- site/src/pages/WorkspacePage/AgentAlert.tsx | 24 ---- site/src/pages/WorkspacePage/Workspace.tsx | 1 - site/src/testHelpers/entities.ts | 71 +++++++++- 8 files changed, 232 insertions(+), 200 deletions(-) diff --git a/site/src/modules/resources/AgentRow.stories.tsx b/site/src/modules/resources/AgentRow.stories.tsx index 139233bd31..a8213ae5d1 100644 --- a/site/src/modules/resources/AgentRow.stories.tsx +++ b/site/src/modules/resources/AgentRow.stories.tsx @@ -98,12 +98,6 @@ const installScriptLogSource: WorkspaceAgentLogSource = { display_name: "Install Script", }; -const startupScriptLogSource: WorkspaceAgentLogSource = { - ...M.MockWorkspaceAgentLogSource, - id: "a1b2c3d4-e5f6-7890-abcd-ef1234567890", - display_name: "Startup Script", -}; - const tabbedLogs = [ { id: 100, @@ -236,58 +230,30 @@ export const StartError: Story = { args: { agent: M.MockWorkspaceAgentStartError, }, -}; - -export const StartErrorWithTimings: Story = { - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - - const scriptTab = await canvas.findByRole("tab", { - name: "Startup Script", - }); - await waitFor(() => - expect(scriptTab).toHaveAttribute("data-state", "active"), - ); - }, - args: { - agent: { - ...M.MockWorkspaceAgentStartError, - logs_length: 2, - log_sources: [startupScriptLogSource], - }, - agentScriptTimings: [ - { - display_name: "Startup Script", - exit_code: 1, - stage: "start", - status: "exit_failure", - started_at: "2021-05-05T00:00:00.000Z", - ended_at: "2021-05-05T00:00:01.000Z", - workspace_agent_id: M.MockWorkspaceAgentStartError.id, - workspace_agent_name: M.MockWorkspaceAgentStartError.name, - }, - ], - }, parameters: { webSocket: [ { event: "message", - data: JSON.stringify([ - { - id: 200, - level: "info", - output: "startup: preparing workspace", - source_id: M.MockWorkspaceAgentLogSource.id, - created_at: fixedLogTimestamp, - }, - { - id: 201, - level: "error", - output: "startup script: command not found", - source_id: startupScriptLogSource.id, - created_at: fixedLogTimestamp, - }, - ]), + data: JSON.stringify( + M.MockWorkspaceAgentStartError.log_sources.flatMap((l, i) => { + return [ + { + id: i, + level: "info", + output: `running '${l.display_name}' script`, + source_id: l.id, + created_at: fixedLogTimestamp, + }, + { + id: i + 100, + level: "error", + output: `stderr from '${l.display_name}' script`, + source_id: l.id, + created_at: fixedLogTimestamp, + }, + ]; + }), + ), }, ], }, diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index da10057904..d331a940ff 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -57,14 +57,8 @@ import { import { useProxy } from "#/contexts/ProxyContext"; import { useClipboard } from "#/hooks/useClipboard"; import { useFeatureVisibility } from "#/modules/dashboard/useFeatureVisibility"; -import { - agentScriptMessages, - getAgentHealthIssues, -} from "#/modules/workspaces/health"; -import { - AgentAlert, - StartScriptFailureDetail, -} from "#/pages/WorkspacePage/AgentAlert"; +import { getAgentHealthIssues } from "#/modules/workspaces/health"; +import { AgentAlert } from "#/pages/WorkspacePage/AgentAlert"; import { AppStatuses } from "#/pages/WorkspacePage/AppStatuses"; import { cn } from "#/utils/cn"; import { AgentApps, organizeAgentApps } from "./AgentApps/AgentApps"; @@ -141,7 +135,6 @@ export const AgentRow: FC = ({ template, onUpdateAgent, initialMetadata, - agentScriptTimings, }) => { const { browser_only, workspace_external_agent } = useFeatureVisibility(); const appSections = organizeAgentApps(agent.apps); @@ -160,12 +153,6 @@ export const AgentRow: FC = ({ const healthIssues = getAgentHealthIssues(agent); const hasAgentIssues = healthIssues.length > 0; const hasWarningIssues = healthIssues.some((i) => i.severity === "warning"); - const failedStartTimings = agentScriptTimings?.filter( - (t) => - t.workspace_agent_id === agent.id && - t.stage === "start" && - t.exit_code !== 0, - ); const { proxy } = useProxy(); const [showLogs, setShowLogs] = useState( (["starting", "start_timeout"].includes(agent.lifecycle_state) || @@ -253,7 +240,7 @@ export const AgentRow: FC = ({ const [selectedLogTab, setSelectedLogTab] = useState( failedStartupScriptSource?.id ?? "all", ); - const sourceLogTabs = agent.log_sources + const sortedSourceLogTabs = agent.log_sources .filter((logSource) => { // Remove the logSources that have no entries. return agentLogs.some( @@ -261,38 +248,57 @@ export const AgentRow: FC = ({ log.source_id === logSource.id && (log.output?.length ?? 0) > 0, ); }) - .map((logSource) => ({ - // Show the icon for the log source if it has one. - // In the startup script case, we show a bespoke play icon. - startIcon: logSource.icon ? ( - - ) : logSource.display_name === STARTUP_SCRIPT_DISPLAY_NAME ? ( - - ) : null, - title: logSource.display_name, - value: logSource.id, - })); - const startupScriptLogTab = sourceLogTabs.find( - (tab) => tab.title === STARTUP_SCRIPT_DISPLAY_NAME, - ); - const sortedSourceLogTabs = sourceLogTabs - .filter((tab) => tab !== startupScriptLogTab) - .sort((a, b) => a.title.localeCompare(b.title)); + .map((logSource) => { + const script = agent.scripts.find( + (s) => s.log_source_id === logSource.id, + ); + return { + // Show the icon for the log source if it has one. + // In the startup script case, we show a bespoke play icon. + startIcon: logSource.icon ? ( + + ) : logSource.display_name === STARTUP_SCRIPT_DISPLAY_NAME ? ( + + ) : null, + title: logSource.display_name, + value: logSource.id, + error: Boolean( + script?.exit_code || (script?.status && script.status !== "ok"), + ), + }; + }) + .sort((a, b) => { + // Errored scripts first, then startup script, then the rest. + if (a.error && !b.error) { + return -1; + } + if (b.error && !a.error) { + return 1; + } + if (a.title === STARTUP_SCRIPT_DISPLAY_NAME) { + return -1; + } + if (b.title === STARTUP_SCRIPT_DISPLAY_NAME) { + return 1; + } + return a.title.localeCompare(b.title); + }); const logTabs: { startIcon?: ReactNode; title: string; value: string; + error: boolean; }[] = [ { title: "All Logs", value: "all", startIcon: , + error: false, }, - ...(startupScriptLogTab ? [startupScriptLogTab] : []), ...sortedSourceLogTabs, ]; const hasAnyLogs = agentLogs.length > 0; @@ -540,27 +546,13 @@ export const AgentRow: FC = ({
{healthIssues.length > 0 && (
- {healthIssues.map((issue) => { - const isStartError = - issue.title === agentScriptMessages.start_error.title; - const detail = - isStartError && failedStartTimings?.length ? ( - - ) : ( - issue.detail - ); - return ( - - ); - })} + {healthIssues.map((issue) => ( + + ))}
)} {hasStartupFeatures && hasAnyLogs && ( @@ -588,6 +580,15 @@ export const AgentRow: FC = ({ {tab.title} + {tab.error && ( + + + + )} ))} {overflowLogTabs.length > 0 && ( @@ -629,6 +630,15 @@ export const AgentRow: FC = ({ {tab.title} + {tab.error && ( + + + + )} ))} diff --git a/site/src/modules/workspaces/health.test.ts b/site/src/modules/workspaces/health.test.ts index 8800cb8791..b94572803b 100644 --- a/site/src/modules/workspaces/health.test.ts +++ b/site/src/modules/workspaces/health.test.ts @@ -4,7 +4,11 @@ import type { WorkspaceAgentLifecycle, WorkspaceAgentStatus, } from "#/api/typesGenerated"; -import { MockWorkspaceAgent } from "#/testHelpers/entities"; +import { + MockWorkspaceAgent, + MockWorkspaceAgentStartError, + MockWorkspaceAgentStartTimeout, +} from "#/testHelpers/entities"; import { getAgentHealthIssues } from "./health"; interface AgentOverrides { @@ -16,9 +20,7 @@ interface AgentOverrides { function buildAgent(overrides: AgentOverrides): WorkspaceAgent { return { ...MockWorkspaceAgent, - status: overrides.status ?? "connected", - lifecycle_state: overrides.lifecycle_state ?? "ready", - parent_id: overrides.parent_id ?? null, + ...overrides, }; } @@ -74,21 +76,36 @@ describe("getAgentHealthIssues", () => { ); }); - it("returns startup script issues", () => { - expect( - getAgentHealthIssues(buildAgent({ lifecycle_state: "start_error" })), - ).toContainEqual( + it("returns script issues", () => { + const issues = getAgentHealthIssues( + buildAgent(MockWorkspaceAgentStartError), + ); + expect(issues).toContainEqual( expect.objectContaining({ - title: "Startup script failed", + 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, }), ); expect( - getAgentHealthIssues(buildAgent({ lifecycle_state: "start_timeout" })), + getAgentHealthIssues(buildAgent(MockWorkspaceAgentStartTimeout)), ).toContainEqual( expect.objectContaining({ - title: "Startup script is taking longer than expected", + title: `"Startup Script" is taking longer than expected`, severity: "warning", prominent: false, }), @@ -119,13 +136,16 @@ describe("getAgentHealthIssues", () => { it("returns multiple issues when multiple conditions match", () => { const issues = getAgentHealthIssues( - buildAgent({ status: "disconnected", lifecycle_state: "start_error" }), + buildAgent({ + ...MockWorkspaceAgentStartError, + status: "disconnected", + }), ); expect(issues).toContainEqual( expect.objectContaining({ title: "Workspace agent has disconnected" }), ); expect(issues).toContainEqual( - expect.objectContaining({ title: "Startup script failed" }), + expect.objectContaining({ title: `"Startup Script" failed` }), ); }); }); diff --git a/site/src/modules/workspaces/health.ts b/site/src/modules/workspaces/health.ts index e1faa72597..ffe52f7af0 100644 --- a/site/src/modules/workspaces/health.ts +++ b/site/src/modules/workspaces/health.ts @@ -100,22 +100,45 @@ export function getAgentHealthIssues( }); } - if (agent.lifecycle_state === "start_error") { - issues.push({ - title: agentScriptMessages.start_error.title, - detail: agentScriptMessages.start_error.detail, - severity: "warning", - prominent: false, - }); - } - - if (agent.lifecycle_state === "start_timeout") { - issues.push({ - title: agentScriptMessages.start_timeout.title, - detail: agentScriptMessages.start_timeout.detail, - severity: "warning", - prominent: false, - }); + // 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. + for (const script of agent.scripts) { + switch (script.status) { + case "timed_out": + issues.push({ + title: `"${script.display_name}" is taking longer than expected`, + detail: `"${script.display_name}" has exceeded the expected time. Check the agent logs for details.`, + severity: "warning", + prominent: false, + }); + break; + case "exit_failure": + if (script.exit_code) { + issues.push({ + title: `"${script.display_name}" failed`, + detail: `"${script.display_name}" exited with ${script.exit_code}. Check the agent logs for details.`, + severity: "warning", + prominent: false, + }); + } else { + issues.push({ + title: `"${script.display_name}" failed`, + detail: `"${script.display_name}" has exited with an error. Check the agent logs for details.`, + severity: "warning", + prominent: false, + }); + } + break; + case "pipes_left_open": + issues.push({ + title: `"${script.display_name}" left pipes open`, + detail: "Check the agent logs for details.", + severity: "warning", + prominent: false, + }); + break; + } } if (agent.status === "connecting") { diff --git a/site/src/pages/WorkspacePage/AgentAlert.stories.tsx b/site/src/pages/WorkspacePage/AgentAlert.stories.tsx index 2f649609c5..464e7cb8ab 100644 --- a/site/src/pages/WorkspacePage/AgentAlert.stories.tsx +++ b/site/src/pages/WorkspacePage/AgentAlert.stories.tsx @@ -1,5 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { AgentAlert, StartScriptFailureDetail } from "./AgentAlert"; +import { AgentAlert } from "./AgentAlert"; const meta: Meta = { title: "pages/WorkspacePage/AgentAlert", @@ -50,34 +50,3 @@ export const WithoutTroubleshootingURL: Story = { troubleshootingURL: undefined, }, }; - -export const WithScriptTimingDetail: Story = { - render: (args) => ( - - } - /> - ), - args: { - title: "Startup script failed", - severity: "warning", - prominent: false, - troubleshootingURL: undefined, - }, -}; diff --git a/site/src/pages/WorkspacePage/AgentAlert.tsx b/site/src/pages/WorkspacePage/AgentAlert.tsx index e47948c5a6..cb35f1a92a 100644 --- a/site/src/pages/WorkspacePage/AgentAlert.tsx +++ b/site/src/pages/WorkspacePage/AgentAlert.tsx @@ -1,5 +1,4 @@ import type { FC, ReactNode } from "react"; -import type { AgentScriptTiming } from "#/api/typesGenerated"; import { Alert, AlertDescription, AlertTitle } from "#/components/Alert/Alert"; import { Button } from "#/components/Button/Button"; @@ -11,29 +10,6 @@ interface AgentAlertProps { troubleshootingURL?: string; } -interface StartScriptFailureDetailProps { - baseDetail: string; - timings: readonly AgentScriptTiming[]; -} - -export const StartScriptFailureDetail: FC = ({ - baseDetail, - timings, -}) => { - return ( - <> - {baseDetail} -
    - {timings.map((t) => ( -
  • - “{t.display_name}” exited with code {t.exit_code} -
    • - ))} -
- - ); -}; - export const AgentAlert: FC = ({ title, detail, diff --git a/site/src/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index bc9268ff72..8aa18a5277 100644 --- a/site/src/pages/WorkspacePage/Workspace.tsx +++ b/site/src/pages/WorkspacePage/Workspace.tsx @@ -219,7 +219,6 @@ export const Workspace: FC = ({ )} workspace={workspace} template={template} - agentScriptTimings={timings?.agent_script_timings} onUpdateAgent={handleUpdate} // On updating the workspace the agent version is also updated /> ))} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 35af473aef..9e95bcb590 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -984,7 +984,7 @@ const MockWorkspaceAgentScript: TypesGen.WorkspaceAgentScript = { script: "echo 'hello world'", start_blocks_login: false, timeout: 0, - display_name: "Say Hello", + display_name: "Startup Script", }; export const MockWorkspaceAgent: TypesGen.WorkspaceAgent = { @@ -1175,6 +1175,14 @@ export const MockWorkspaceAgentStartTimeout: TypesGen.WorkspaceAgent = { id: "test-workspace-agent-start-timeout", name: "a-workspace-agent-timed-out-while-running-startup-script", lifecycle_state: "start_timeout", + scripts: [ + { + ...MockWorkspaceAgentScript, + status: "timed_out", + }, + ], + logs_length: 1, + log_sources: [MockWorkspaceAgentLogSource], }; export const MockWorkspaceAgentStartError: TypesGen.WorkspaceAgent = { @@ -1186,6 +1194,67 @@ export const MockWorkspaceAgentStartError: TypesGen.WorkspaceAgent = { healthy: false, reason: "agent startup script failed", }, + scripts: [ + { + ...MockWorkspaceAgentScript, + exit_code: 1, + status: "exit_failure", + }, + { + ...MockWorkspaceAgentScript, + id: "18eaca83-1221-4fad-b882-d1136981f54d", + log_source_id: "a2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + exit_code: 0, + status: "ok", + display_name: "coder", + }, + { + ...MockWorkspaceAgentScript, + id: "28eaca83-1221-4fad-b882-d1136981f54d", + log_source_id: "b2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + status: "timed_out", + display_name: "time", + }, + { + ...MockWorkspaceAgentScript, + id: "38eaca83-1221-4fad-b882-d1136981f54d", + log_source_id: "c2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + status: "pipes_left_open", + display_name: "pipe", + }, + { + ...MockWorkspaceAgentScript, + id: "48eaca83-1221-4fad-b882-d1136981f54d", + log_source_id: "d2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + display_name: "running", + }, + ], + logs_length: 4, + log_sources: [ + MockWorkspaceAgentLogSource, + { + ...MockWorkspaceAgentLogSource, + id: "a2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + display_name: "coder", + icon: "/icon/coder.svg", + }, + { + ...MockWorkspaceAgentLogSource, + id: "b2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + display_name: "time", + icon: "/icon/folder.svg", + }, + { + ...MockWorkspaceAgentLogSource, + id: "c2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + display_name: "pipe", + }, + { + ...MockWorkspaceAgentLogSource, + id: "d2ee4b8d-b09d-4f4e-a1f1-5e4adf7d53bb", + display_name: "running", + }, + ], }; export const MockWorkspaceAgentShuttingDown: TypesGen.WorkspaceAgent = {