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 = {