From e9c2c0cf8ecf1615004fe88a3629096357d91f53 Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 28 Jun 2026 22:01:38 -0400 Subject: [PATCH] fix: classify degraded remote stacks as partial on the compatibility path (#1511) When a remote node's bulk /stacks/statuses is unavailable or returns the legacy plain-string format, the sidebar derived a stack's status from its containers by treating "any container running" as healthy. A stack with one running and one crashed container was shown green UP and excluded from the Down filter. Re-derive status from the per-stack container list in both compatibility cases, mirroring the backend classifier: a stack with a running container and a genuinely crashed one (dead, or exited/restarting with a non-zero code) is now partial, while a cleanly finished one-shot container (exit 0) stays running. Legacy plain-string bulk responses are routed through this path too, since they have already collapsed the degraded case. --- .../EditorLayout/hooks/useStackListState.ts | 76 ++++++++------- .../__tests__/stack-status-utils.test.ts | 96 ++++++++++++++++++- .../components/sidebar/stack-status-utils.ts | 61 ++++++++++++ 3 files changed, 197 insertions(+), 36 deletions(-) diff --git a/frontend/src/components/EditorLayout/hooks/useStackListState.ts b/frontend/src/components/EditorLayout/hooks/useStackListState.ts index 9dd2075d..668d3931 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackListState.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackListState.ts @@ -9,12 +9,36 @@ import { useBulkStackActions, type BulkAction } from '@/hooks/useBulkStackAction import { useCrossNodeStackSearch } from '@/hooks/useCrossNodeStackSearch'; import { SENCHO_LABELS_CHANGED } from '@/lib/events'; import { isInputFocused, isPaletteOpen } from '@/lib/keyboard-guards'; -import type { StackAction, StackActionResult, ContainerInfo } from '../EditorView'; +import type { StackAction, StackActionResult } from '../EditorView'; import type { Label as StackLabel } from '../../label-types'; import type { FilterChip } from '../../sidebar/sidebar-types'; -import { isDownStatus } from '../../sidebar/stack-status-utils'; +import { isDownStatus, classifyContainersStatus, isBulkStatusObjectFormat } from '../../sidebar/stack-status-utils'; import type { StackRowStatus } from '../../sidebar/stack-status-utils'; +/** Compatibility path for remote nodes whose `/stacks/statuses` is absent or + * returns the legacy plain-string format: query each stack's containers and + * classify them so a degraded (partial) stack is not reported as healthy. */ +async function deriveStatusesFromContainers( + fileList: string[], +): Promise> { + const results = await Promise.allSettled( + fileList.map(async (file) => { + const containersRes = await apiFetch(`/stacks/${file}/containers`); + if (!containersRes.ok) return { file, status: 'unknown' as StackRowStatus }; + const containers = await containersRes.json(); + return { + file, + status: Array.isArray(containers) ? classifyContainersStatus(containers) : 'unknown', + }; + }), + ); + const out: Record = {}; + for (const result of results) { + if (result.status === 'fulfilled') out[result.value.file] = result.value.status; + } + return out; +} + interface StackStatus { [key: string]: StackRowStatus; } @@ -170,50 +194,32 @@ export function useStackListState() { setFiles(fileList); setFilesNodeId(fetchNodeId); - // Fetch all stack statuses in a single bulk call (falls back to per-stack queries for older remote nodes) + // Fetch all stack statuses in a single bulk call. Only the current object + // format can express `partial`; a node lacking the endpoint or returning + // the legacy plain-string format is re-derived from per-stack containers + // so a crashed container is not hidden behind a healthy sibling. const statusRes = await apiFetch('/stacks/statuses'); if (stale()) return fileList; - let bulkStatuses: Record | null = null; + let bulkStatuses: Record = {}; const bulkPorts: Record = {}; const bulkCounts: StackCounts = {}; - if (statusRes.ok) { - const raw = await statusRes.json(); - bulkStatuses = {}; - // Handle both old format (plain string) and new format ({ status, mainPort, running, total }) - for (const [key, val] of Object.entries(raw)) { - if (typeof val === 'string') { - bulkStatuses[key] = val as StackRowStatus; - } else if (val && typeof val === 'object' && 'status' in val) { - const info = val as StackStatusInfo; - bulkStatuses[key] = info.status; - if (info.mainPort) bulkPorts[key] = info.mainPort; - if (info.running !== undefined && info.total !== undefined) { - bulkCounts[key] = { running: info.running, total: info.total }; - } + + const raw: unknown = statusRes.ok ? await statusRes.json() : null; + if (isBulkStatusObjectFormat(raw)) { + for (const [key, val] of Object.entries(raw as Record)) { + bulkStatuses[key] = val.status; + if (val.mainPort) bulkPorts[key] = val.mainPort; + if (val.running !== undefined && val.total !== undefined) { + bulkCounts[key] = { running: val.running, total: val.total }; } } } else { - // Fallback: query each stack individually (remote node may not have bulk endpoint) - const statusResults = await Promise.allSettled( - fileList.map(async (file) => { - const containersRes = await apiFetch(`/stacks/${file}/containers`); - if (!containersRes.ok) return { file, status: 'unknown' as const }; - const containers = await containersRes.json(); - const hasRunning = Array.isArray(containers) && containers.some((c: ContainerInfo) => c.State === 'running'); - return { file, status: hasRunning ? 'running' as const : (Array.isArray(containers) && containers.length > 0 ? 'exited' as const : 'unknown' as const) }; - }) - ); - bulkStatuses = {}; - for (const result of statusResults) { - if (result.status === 'fulfilled') { - bulkStatuses[result.value.file] = result.value.status; - } - } + bulkStatuses = await deriveStatusesFromContainers(fileList); } setStackStatuses(prev => { const next: StackStatus = {}; for (const file of fileList) { - const status = bulkStatuses?.[file] ?? 'unknown'; + const status = bulkStatuses[file] ?? 'unknown'; next[file] = (file in stackActionsRef.current) ? (prev[file] ?? status) : status; } return next; diff --git a/frontend/src/components/sidebar/__tests__/stack-status-utils.test.ts b/frontend/src/components/sidebar/__tests__/stack-status-utils.test.ts index 405157ec..7725b194 100644 --- a/frontend/src/components/sidebar/__tests__/stack-status-utils.test.ts +++ b/frontend/src/components/sidebar/__tests__/stack-status-utils.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { statusText, statusColor, isDownStatus } from '../stack-status-utils'; +import { statusText, statusColor, isDownStatus, classifyContainersStatus, isBulkStatusObjectFormat } from '../stack-status-utils'; describe('stack-status-utils', () => { describe('statusText', () => { @@ -31,4 +31,98 @@ describe('stack-status-utils', () => { expect(isDownStatus(undefined)).toBe(false); }); }); + + describe('classifyContainersStatus', () => { + it('returns unknown for an empty container list', () => { + expect(classifyContainersStatus([])).toBe('unknown'); + }); + + it('returns running when every container is up', () => { + expect(classifyContainersStatus([ + { State: 'running', Status: 'Up 2 hours' }, + { State: 'running', Status: 'Up 2 hours' }, + ])).toBe('running'); + }); + + it('returns partial when a container has crashed alongside a running one', () => { + expect(classifyContainersStatus([ + { State: 'running', Status: 'Up 2 hours' }, + { State: 'exited', Status: 'Exited (1) 5 minutes ago' }, + ])).toBe('partial'); + }); + + it('treats a dead container as a crash even with a running sibling', () => { + expect(classifyContainersStatus([ + { State: 'running', Status: 'Up 2 hours' }, + { State: 'dead', Status: 'Dead' }, + ])).toBe('partial'); + }); + + it('treats a crash-looping container as partial', () => { + expect(classifyContainersStatus([ + { State: 'running', Status: 'Up 1 minute' }, + { State: 'restarting', Status: 'Restarting (1) 3 seconds ago' }, + ])).toBe('partial'); + }); + + it('does not degrade a stack for a cleanly finished one-shot container', () => { + expect(classifyContainersStatus([ + { State: 'running', Status: 'Up 2 hours' }, + { State: 'exited', Status: 'Exited (0) 1 hour ago' }, + ])).toBe('running'); + }); + + it('treats an exited container with an unreadable code as a crash', () => { + expect(classifyContainersStatus([ + { State: 'running', Status: 'Up 2 hours' }, + { State: 'exited', Status: 'Exited' }, + ])).toBe('partial'); + }); + + it('ignores a non-running, non-failed container (created) when a sibling runs', () => { + expect(classifyContainersStatus([ + { State: 'running', Status: 'Up 2 hours' }, + { State: 'created', Status: 'Created' }, + ])).toBe('running'); + }); + + it('returns exited when no container is running', () => { + expect(classifyContainersStatus([ + { State: 'exited', Status: 'Exited (1) 5 minutes ago' }, + { State: 'created', Status: 'Created' }, + ])).toBe('exited'); + }); + }); + + describe('isBulkStatusObjectFormat', () => { + it('accepts the current object format', () => { + expect(isBulkStatusObjectFormat({ + web: { status: 'running', running: 2, total: 2 }, + db: { status: 'partial', running: 1, total: 2 }, + })).toBe(true); + }); + + it('treats an empty object (node with no stacks) as the current format', () => { + expect(isBulkStatusObjectFormat({})).toBe(true); + }); + + it('rejects the legacy plain-string format', () => { + expect(isBulkStatusObjectFormat({ web: 'running', db: 'exited' })).toBe(false); + }); + + it('rejects a mixed response with any plain-string entry', () => { + expect(isBulkStatusObjectFormat({ + web: { status: 'running' }, + db: 'running', + })).toBe(false); + }); + + it('rejects an object entry missing a status field', () => { + expect(isBulkStatusObjectFormat({ web: { running: 1, total: 1 } })).toBe(false); + }); + + it('rejects null', () => { + expect(isBulkStatusObjectFormat(null)).toBe(false); + }); + }); }); diff --git a/frontend/src/components/sidebar/stack-status-utils.ts b/frontend/src/components/sidebar/stack-status-utils.ts index 6855ca55..025c049a 100644 --- a/frontend/src/components/sidebar/stack-status-utils.ts +++ b/frontend/src/components/sidebar/stack-status-utils.ts @@ -19,3 +19,64 @@ export function statusColor(status: StackRowStatus, isBusy: boolean): string { export function isDownStatus(status: StackRowStatus | undefined): boolean { return status === 'exited' || status === 'partial'; } + +/** Minimal container shape needed to classify a stack's status. */ +interface ContainerStateInfo { + State: string; + Status?: string; +} + +/** Exit code parsed from a Docker status string like "Exited (1) 2 hours ago". + * Returns null when no parenthesized code is present (e.g. "Up 3 hours"). */ +function parseExitCode(status: string | undefined): number | null { + if (!status) return null; + const match = /\((\d+)\)/.exec(status); + return match ? Number(match[1]) : null; +} + +/** Whether a container is a genuine crash rather than a clean finish. Mirrors + * the backend bulk-status classifier so the compatibility fallback agrees with + * a current node's `/stacks/statuses`: a dead container always counts, and an + * exited or restarting one counts only with a non-zero (or unreadable) code, so + * a finished init job (exit 0) does not mark its stack degraded. */ +function isContainerFailed(state: string, status: string | undefined): boolean { + if (state === 'dead') return true; + if (state === 'exited' || state === 'restarting') { + const code = parseExitCode(status); + return code === null ? true : code !== 0; + } + return false; +} + +/** Whether a parsed `/stacks/statuses` response is the current object format + * (`{ status, ... }` per stack) rather than the legacy plain-string format. + * Only the object format can express `partial`; a legacy plain-string response + * has already collapsed a degraded stack into "running", so it (like a missing + * endpoint) must be re-derived from per-stack containers. An empty object is + * the current format for a node with no stacks. */ +export function isBulkStatusObjectFormat(raw: unknown): boolean { + return ( + raw !== null && + typeof raw === 'object' && + Object.values(raw as Record).every( + (val) => val !== null && typeof val === 'object' && 'status' in val, + ) + ); +} + +/** Derive a stack's row status from its container list, distinguishing a fully-up + * stack from one that is partially degraded (some running, some crashed). Used by + * the compatibility path for remote nodes whose bulk status endpoint is absent or + * predates partial-status support, where trusting "any container running" would + * show a degraded stack as healthy. */ +export function classifyContainersStatus(containers: ContainerStateInfo[]): StackRowStatus { + if (containers.length === 0) return 'unknown'; + let running = 0; + let failed = 0; + for (const c of containers) { + if (c.State === 'running') running += 1; + else if (isContainerFailed(c.State, c.Status)) failed += 1; + } + if (running === 0) return 'exited'; + return failed > 0 ? 'partial' : 'running'; +}