mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-19 23:06:49 +00:00
fix: distinguish failed image-update checks from "up to date" (#1470)
* fix: distinguish failed image-update checks from "up to date" The image-update detector collapsed every failure (registry unreachable, missing auth, rate limit, unresolved local digest) into hasUpdate:false and dropped the captured reason, so a failed check was indistinguishable from a current image and never raised a notification, even while a manual stack update still pulled a newer image. Detection now records a tri-state per stack (ok / partial / failed) with the failure reason, exposed via a new GET /api/image-updates/detail (the boolean GET / is unchanged so fleet aggregation is unaffected). A fully-failed check preserves the last known has_update, so a transient outage neither erases a real update nor flaps the notification state. The sidebar shows a muted "couldn't check" indicator with the reason on hover, and the Update board lists stacks whose check failed in a "could not be checked" advisory. Detector hardening: the manifest digest lookup issues HEAD first (falling back to GET) so it no longer draws down Docker Hub's anonymous pull-rate budget, and local RepoDigest matching is normalized so official library/* images resolve their digest instead of falling through to a silent "no update". * fix: preserve confirmed updates through partial checks; tighten failure surfacing Address review findings on the tri-state image-update detection: - A partial check (some images errored) no longer erases a previously confirmed update; only a fully-ok check can lower has_update, so a single image's registry blip cannot drop the stack's update and re-fire the notification on recovery. Adds a regression test. - The image-level catch stores getErrorMessage(e) rather than raw String(e), since that value surfaces verbatim in the sidebar tooltip and readiness advisory. - useImageUpdates and the readiness detail fetch now log unexpected non-ok responses instead of silently leaving stale state. - Remove an unused checkFailedCount derivation (the row indicator is driven by the checkStatus prop). - Reword the recordStackCheckFailure docstring and the HEAD-first comment.
This commit is contained in:
@@ -6,7 +6,7 @@ import { RefreshCw, Shield, AlertTriangle, ShieldAlert, CircleSlash, Clock, Play
|
||||
import { toast } from '@/components/ui/toast-store';
|
||||
import { apiFetch, fetchForNode } from '@/lib/api';
|
||||
import { formatTimeAgo } from '@/lib/relativeTime';
|
||||
import type { ImageUpdateStatus } from '@/types/imageUpdates';
|
||||
import type { ImageUpdateStatus, StackUpdateInfo } from '@/types/imageUpdates';
|
||||
import { useNodes } from '@/context/NodeContext';
|
||||
import { useIsMobile } from '@/hooks/use-is-mobile';
|
||||
import { Masthead, Kicker } from '@/components/mobile/mobile-ui';
|
||||
@@ -496,6 +496,30 @@ function MobileNodeSection({ group, onApply }: { group: NodeGroup; onApply: (sta
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Advisory for local-node stacks whose latest image-update check could not
|
||||
* determine status. These never appear in the card grid (which lists only
|
||||
* confirmed updates), so without this they would be invisible here.
|
||||
*/
|
||||
function CheckFailuresNotice({ failures }: { failures: { stack: string; reason: string | null }[] }) {
|
||||
if (failures.length === 0) return null;
|
||||
return (
|
||||
<div className="rounded-lg border border-warning/30 bg-warning/5 p-3">
|
||||
<div className="flex items-center gap-2 font-mono text-[11px] text-warning">
|
||||
<AlertTriangle className="h-3.5 w-3.5" strokeWidth={1.5} aria-hidden="true" />
|
||||
{failures.length} stack{failures.length !== 1 ? 's' : ''} could not be checked
|
||||
</div>
|
||||
<ul className="mt-1.5 space-y-0.5 pl-5">
|
||||
{failures.map(f => (
|
||||
<li key={f.stack} className="font-mono text-[11px] text-stat-subtitle">
|
||||
<span className="text-stat-value">{f.stack}</span>{f.reason ? `: ${f.reason}` : ''}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
interface AutoUpdateReadinessProps {
|
||||
/** Notifications + more-menu cluster for the mobile masthead, rehomed from the dropped TopBar. */
|
||||
headerActions?: ReactNode;
|
||||
@@ -509,6 +533,10 @@ function AutoUpdateReadinessContent({ headerActions }: AutoUpdateReadinessProps)
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [refreshing, setRefreshing] = useState(false);
|
||||
const [cadence, setCadence] = useState<ImageUpdateStatus | null>(null);
|
||||
// Local-node stacks whose latest check could not determine status. The fleet
|
||||
// list only shows stacks with a confirmed update, so without this a stack
|
||||
// whose checks all fail would silently vanish from this view.
|
||||
const [checkFailures, setCheckFailures] = useState<{ stack: string; reason: string | null }[]>([]);
|
||||
const refreshTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
// Monotonic token guards against stale setGroups from older fetches.
|
||||
const loadTokenRef = useRef(0);
|
||||
@@ -535,9 +563,10 @@ function AutoUpdateReadinessContent({ headerActions }: AutoUpdateReadinessProps)
|
||||
const token = ++loadTokenRef.current;
|
||||
setLoading(true);
|
||||
try {
|
||||
const [statusRes, tasksRes] = await Promise.all([
|
||||
const [statusRes, tasksRes, detailRes] = await Promise.all([
|
||||
apiFetch('/image-updates/fleet', { localOnly: true }),
|
||||
apiFetch('/scheduled-tasks?action=update', { localOnly: true }),
|
||||
apiFetch('/image-updates/detail', { localOnly: true }),
|
||||
]);
|
||||
if (token !== loadTokenRef.current) return;
|
||||
|
||||
@@ -547,6 +576,23 @@ function AutoUpdateReadinessContent({ headerActions }: AutoUpdateReadinessProps)
|
||||
const fleetStatus = await statusRes.json() as FleetUpdateResponse;
|
||||
setReachableNodeCount(Object.keys(fleetStatus).length);
|
||||
|
||||
// Local-node check failures: surfaced separately because the fleet map is
|
||||
// boolean and the card grid only lists stacks with a confirmed update.
|
||||
if (detailRes.ok) {
|
||||
const detail = await detailRes.json() as Record<string, StackUpdateInfo>;
|
||||
setCheckFailures(
|
||||
Object.entries(detail)
|
||||
.filter(([, info]) => info.checkStatus === 'failed')
|
||||
.map(([stack, info]) => ({ stack, reason: info.lastError }))
|
||||
.sort((a, b) => a.stack.localeCompare(b.stack)),
|
||||
);
|
||||
} else {
|
||||
// Clear stale failures rather than persist them across a load, but log:
|
||||
// an empty advisory must not silently stand in for "detail unavailable".
|
||||
console.error('[AutoUpdateReadiness] /image-updates/detail failed:', detailRes.status);
|
||||
setCheckFailures([]);
|
||||
}
|
||||
|
||||
const tasks: ScheduledTask[] = tasksRes.ok ? await tasksRes.json() : [];
|
||||
// A stack is "covered" by an enabled action='update' row when either
|
||||
// a per-stack row targets it or a fleet row targets its node. We pick
|
||||
@@ -805,6 +851,7 @@ function AutoUpdateReadinessContent({ headerActions }: AutoUpdateReadinessProps)
|
||||
{reachableNodeCount} of {onlineNodeCount} nodes reachable. Unreachable nodes are not shown.
|
||||
</div>
|
||||
)}
|
||||
<CheckFailuresNotice failures={checkFailures} />
|
||||
{loading && groups.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-16 font-mono text-xs text-stat-subtitle">Loading readiness...</div>
|
||||
) : groups.length === 0 ? (
|
||||
@@ -839,6 +886,8 @@ function AutoUpdateReadinessContent({ headerActions }: AutoUpdateReadinessProps)
|
||||
</div>
|
||||
)}
|
||||
|
||||
<CheckFailuresNotice failures={checkFailures} />
|
||||
|
||||
{loading && groups.length === 0 ? (
|
||||
<div className="flex items-center justify-center py-16 font-mono text-xs text-stat-subtitle">
|
||||
Loading readiness...
|
||||
|
||||
@@ -289,14 +289,14 @@ export function useStackListState() {
|
||||
all: filteredFiles.length,
|
||||
up: filteredFiles.filter(f => stackStatuses[f] === 'running').length,
|
||||
down: filteredFiles.filter(f => isDownStatus(stackStatuses[f])).length,
|
||||
updates: filteredFiles.filter(f => !!stackUpdates[f]).length,
|
||||
updates: filteredFiles.filter(f => stackUpdates[f]?.hasUpdate).length,
|
||||
}), [filteredFiles, stackStatuses, stackUpdates]);
|
||||
|
||||
const chipFilteredFiles = useMemo(() => {
|
||||
if (filterChip === 'all') return filteredFiles;
|
||||
if (filterChip === 'up') return filteredFiles.filter(f => stackStatuses[f] === 'running');
|
||||
if (filterChip === 'down') return filteredFiles.filter(f => isDownStatus(stackStatuses[f]));
|
||||
if (filterChip === 'updates') return filteredFiles.filter(f => !!stackUpdates[f]);
|
||||
if (filterChip === 'updates') return filteredFiles.filter(f => stackUpdates[f]?.hasUpdate);
|
||||
return filteredFiles;
|
||||
}, [filteredFiles, filterChip, stackStatuses, stackUpdates]);
|
||||
|
||||
|
||||
@@ -124,6 +124,48 @@ describe('AutoUpdateReadinessView desktop Apply now', () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Local-node stacks whose latest check could not determine status never appear
|
||||
* in the card grid (which lists confirmed updates only), so the readiness view
|
||||
* surfaces them in a "could not be checked" advisory fed by a parallel local
|
||||
* /image-updates/detail fetch.
|
||||
*/
|
||||
describe('AutoUpdateReadinessView check-failed advisory', () => {
|
||||
const mockedFetch = apiFetch as unknown as ReturnType<typeof vi.fn>;
|
||||
const mockedFetchForNode = fetchForNode as unknown as ReturnType<typeof vi.fn>;
|
||||
|
||||
afterEach(() => {
|
||||
mockedFetch.mockReset();
|
||||
mockedFetchForNode.mockReset();
|
||||
});
|
||||
|
||||
it('lists local stacks whose check failed, with the reason', async () => {
|
||||
mockedFetch.mockImplementation((url: string) => {
|
||||
if (url === '/image-updates/fleet') return Promise.resolve({ ok: true, json: async () => ({}) });
|
||||
if (url.startsWith('/scheduled-tasks')) return Promise.resolve({ ok: true, json: async () => [] });
|
||||
if (url === '/image-updates/detail') {
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
grafana: { hasUpdate: false, checkStatus: 'failed', lastError: 'Registry unreachable for ghcr.io/acme/grafana:latest', checkedAt: 1 },
|
||||
web: { hasUpdate: false, checkStatus: 'ok', lastError: null, checkedAt: 1 },
|
||||
}),
|
||||
});
|
||||
}
|
||||
return Promise.resolve({ ok: true, json: async () => ({}) });
|
||||
});
|
||||
mockedFetchForNode.mockResolvedValue({ ok: true, json: async () => null });
|
||||
|
||||
render(<AutoUpdateReadinessView />);
|
||||
|
||||
expect(await screen.findByText(/could not be checked/i)).toBeInTheDocument();
|
||||
expect(screen.getByText('grafana')).toBeInTheDocument();
|
||||
expect(screen.getByText(/Registry unreachable for ghcr.io\/acme\/grafana:latest/)).toBeInTheDocument();
|
||||
// An ok stack with no update must not appear in the advisory.
|
||||
expect(screen.queryByText('web')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* CadenceStrip surfaces the control instance's detection cadence by the
|
||||
* readiness card: a past last-check must read as an "ago" value (not the
|
||||
|
||||
@@ -4,6 +4,7 @@ import { useStackKeyboardShortcuts } from '@/hooks/useStackKeyboardShortcuts';
|
||||
import { CommandItem, CommandList } from '@/components/ui/command';
|
||||
import { Skeleton } from '@/components/ui/skeleton';
|
||||
import type { Label } from '@/components/label-types';
|
||||
import type { StackUpdateInfo } from '@/types/imageUpdates';
|
||||
import { StackRow } from './StackRow';
|
||||
import { statusText, statusColor } from './stack-status-utils';
|
||||
import type { StackRowStatus } from './stack-status-utils';
|
||||
@@ -33,7 +34,7 @@ export interface StackListProps {
|
||||
stackLabelMap: Record<string, Label[]>;
|
||||
stackStatuses: Record<string, StackRowStatus | undefined>;
|
||||
stackCounts: Record<string, { running: number; total: number } | undefined>;
|
||||
stackUpdates: Record<string, boolean>;
|
||||
stackUpdates: Record<string, StackUpdateInfo>;
|
||||
gitSourcePendingMap: Record<string, boolean>;
|
||||
pinnedFiles: string[];
|
||||
isCollapsed: (groupKey: string) => boolean;
|
||||
@@ -182,7 +183,9 @@ export function StackList(props: StackListProps & StackListBulkProps) {
|
||||
isBusy={isBusy(file)}
|
||||
isActive={selectedFile === file}
|
||||
labels={stackLabelMap[file] ?? []}
|
||||
hasUpdate={!!stackUpdates[file]}
|
||||
hasUpdate={stackUpdates[file]?.hasUpdate ?? false}
|
||||
checkStatus={stackUpdates[file]?.checkStatus}
|
||||
lastError={stackUpdates[file]?.lastError ?? undefined}
|
||||
hasGitPending={!!gitSourcePendingMap[file]}
|
||||
onSelect={onSelectFile}
|
||||
kebabSlot={<StackKebabMenu file={file} ctx={ctx} />}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { ReactNode } from 'react';
|
||||
import { GitBranch, Loader2 } from 'lucide-react';
|
||||
import { GitBranch, Loader2, AlertCircle } from 'lucide-react';
|
||||
import type { CheckStatus } from '@/types/imageUpdates';
|
||||
import { Cursor, CursorContainer, CursorFollow, CursorProvider } from '@/components/animate-ui/primitives/animate/cursor';
|
||||
import { Checkbox } from '@/components/ui/checkbox';
|
||||
import { LabelDot } from '@/components/LabelPill';
|
||||
@@ -20,6 +21,10 @@ interface StackRowProps {
|
||||
isActive: boolean;
|
||||
labels: Label[];
|
||||
hasUpdate: boolean;
|
||||
// Last image-update check outcome. 'failed' surfaces a muted "couldn't check"
|
||||
// indicator so an undeterminable check is not mistaken for "up to date".
|
||||
checkStatus?: CheckStatus;
|
||||
lastError?: string;
|
||||
hasGitPending: boolean;
|
||||
onSelect: (file: string) => void;
|
||||
kebabSlot: ReactNode;
|
||||
@@ -47,7 +52,7 @@ const MAX_VISIBLE_LABELS = 3;
|
||||
export function StackRow(props: StackRowProps) {
|
||||
const {
|
||||
file, displayName, status, running, total, isBusy, isActive, labels,
|
||||
hasUpdate, hasGitPending, onSelect, kebabSlot,
|
||||
hasUpdate, checkStatus, lastError, hasGitPending, onSelect, kebabSlot,
|
||||
bulkMode = false, isSelected = false, onToggleSelect,
|
||||
} = props;
|
||||
|
||||
@@ -115,7 +120,7 @@ export function StackRow(props: StackRowProps) {
|
||||
</span>
|
||||
)}
|
||||
|
||||
{/* Fixed trailing icon slot: update dot takes priority over git pending */}
|
||||
{/* Fixed trailing icon slot: update dot > check-failed > git pending */}
|
||||
<span className="w-3.5 h-3.5 flex items-center justify-center shrink-0">
|
||||
{hasUpdate ? (
|
||||
<RowTooltip
|
||||
@@ -127,6 +132,11 @@ export function StackRow(props: StackRowProps) {
|
||||
)}
|
||||
label="Update available"
|
||||
/>
|
||||
) : checkStatus === 'failed' ? (
|
||||
<RowTooltip
|
||||
trigger={<AlertCircle className="w-3 h-3 text-muted-foreground/70" strokeWidth={1.5} />}
|
||||
label={lastError ? `Update check failed: ${lastError}` : 'Update check failed'}
|
||||
/>
|
||||
) : hasGitPending ? (
|
||||
<RowTooltip
|
||||
trigger={<GitBranch className="w-3 h-3 text-brand" strokeWidth={1.5} />}
|
||||
|
||||
@@ -101,4 +101,26 @@ describe('StackRow', () => {
|
||||
const { container } = render(<StackRow {...base({ labels })} />);
|
||||
expect(container.querySelectorAll('[style*="--label-"]')).toHaveLength(2);
|
||||
});
|
||||
|
||||
// ── Image-update check status indicator ────────────────────────────────
|
||||
// status='running' renders the pill as plain text (no tooltip), so the only
|
||||
// cursor-container in these rows is the trailing update/check indicator.
|
||||
|
||||
it('shows a muted check-failed indicator when the last check failed and there is no update', () => {
|
||||
const { container } = render(<StackRow {...base({ status: 'running', hasUpdate: false, checkStatus: 'failed', lastError: 'Registry unreachable' })} />);
|
||||
expect(container.querySelector('[data-slot="cursor-container"]')).not.toBeNull();
|
||||
// It is not the update dot.
|
||||
expect(container.querySelector('.bg-update')).toBeNull();
|
||||
});
|
||||
|
||||
it('prefers the update dot over the check-failed indicator', () => {
|
||||
const { container } = render(<StackRow {...base({ status: 'running', hasUpdate: true, checkStatus: 'failed' })} />);
|
||||
expect(container.querySelector('.bg-update')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('shows no trailing indicator for a clean ok check with no update', () => {
|
||||
const { container } = render(<StackRow {...base({ status: 'running', hasUpdate: false, checkStatus: 'ok' })} />);
|
||||
expect(container.querySelector('[data-slot="cursor-container"]')).toBeNull();
|
||||
expect(container.querySelector('.bg-update')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user