diff --git a/backend/src/__tests__/database-stack-update-status.test.ts b/backend/src/__tests__/database-stack-update-status.test.ts new file mode 100644 index 00000000..fd397dc9 --- /dev/null +++ b/backend/src/__tests__/database-stack-update-status.test.ts @@ -0,0 +1,80 @@ +/** + * Coverage for the tri-state stack_update_status accessors on the real + * DatabaseService (against a temp DB, so the migrated schema with check_status / + * last_error is exercised exactly as in production): + * - upsertStackUpdateStatus persists hasUpdate + check_status + last_error + * - getStackUpdateDetail returns the rich per-stack shape + * - getStackUpdateStatus stays the boolean map (fleet contract) + * - recordStackCheckFailure preserves a prior has_update while marking failed + */ +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; +import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb'; + +let tmpDir: string; +let DatabaseService: typeof import('../services/DatabaseService').DatabaseService; + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ DatabaseService } = await import('../services/DatabaseService')); +}); + +afterAll(() => cleanupTestDb(tmpDir)); + +function db() { + return DatabaseService.getInstance(); +} + +beforeEach(() => { + const raw = (db() as unknown as { db: { prepare: (s: string) => { run: () => void } } }).db; + raw.prepare('DELETE FROM stack_update_status').run(); +}); + +const NODE = 1; + +describe('stack_update_status tri-state accessors', () => { + it('persists and reads back check_status + last_error via getStackUpdateDetail', () => { + db().upsertStackUpdateStatus(NODE, 'web', true, 1000, 'ok', null); + db().upsertStackUpdateStatus(NODE, 'api', false, 2000, 'partial', 'Registry unreachable for ghcr.io/acme/api:v1'); + + const detail = db().getStackUpdateDetail(NODE); + expect(detail.web).toEqual({ hasUpdate: true, checkStatus: 'ok', lastError: null, checkedAt: 1000 }); + expect(detail.api).toEqual({ hasUpdate: false, checkStatus: 'partial', lastError: 'Registry unreachable for ghcr.io/acme/api:v1', checkedAt: 2000 }); + }); + + it('defaults check_status to ok when omitted', () => { + db().upsertStackUpdateStatus(NODE, 'web', true, 1000); + expect(db().getStackUpdateDetail(NODE).web.checkStatus).toBe('ok'); + }); + + it('keeps getStackUpdateStatus a boolean map for the fleet contract', () => { + db().upsertStackUpdateStatus(NODE, 'web', true, 1000, 'ok', null); + db().upsertStackUpdateStatus(NODE, 'api', false, 1000, 'failed', 'boom'); + expect(db().getStackUpdateStatus(NODE)).toEqual({ web: true, api: false }); + }); + + it('recordStackCheckFailure preserves a prior has_update while marking failed', () => { + // A stack with a confirmed update, then a scan where every image errored. + db().upsertStackUpdateStatus(NODE, 'web', true, 1000, 'ok', null); + db().recordStackCheckFailure(NODE, 'web', 'Registry unreachable for registry-1.docker.io/library/nginx:latest', 3000); + + const detail = db().getStackUpdateDetail(NODE).web; + expect(detail.hasUpdate).toBe(true); // not erased by the failed check + expect(detail.checkStatus).toBe('failed'); + expect(detail.lastError).toContain('Registry unreachable'); + expect(detail.checkedAt).toBe(3000); + }); + + it('recordStackCheckFailure on a first-ever check inserts has_update 0 + failed', () => { + db().recordStackCheckFailure(NODE, 'fresh', 'auth failed', 4000); + const detail = db().getStackUpdateDetail(NODE).fresh; + expect(detail).toEqual({ hasUpdate: false, checkStatus: 'failed', lastError: 'auth failed', checkedAt: 4000 }); + }); + + it('scopes detail rows to the node', () => { + db().upsertStackUpdateStatus(NODE, 'web', true, 1000, 'ok', null); + db().upsertStackUpdateStatus(2, 'web', false, 1000, 'failed', 'boom'); + expect(Object.keys(db().getStackUpdateDetail(NODE))).toEqual(['web']); + expect(db().getStackUpdateDetail(NODE).web.hasUpdate).toBe(true); + expect(db().getStackUpdateDetail(2).web.checkStatus).toBe('failed'); + }); +}); diff --git a/backend/src/__tests__/image-update-service.test.ts b/backend/src/__tests__/image-update-service.test.ts index e0576932..3fbc8fcf 100644 --- a/backend/src/__tests__/image-update-service.test.ts +++ b/backend/src/__tests__/image-update-service.test.ts @@ -9,6 +9,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; const { mockGetAuthForRegistry, mockGetStackUpdateStatus, mockUpsertStackUpdateStatus, mockClearStackUpdateStatus, + mockRecordStackCheckFailure, mockGetSystemState, mockSetSystemState, mockAddNotificationHistory, mockDispatchAlert, mockGetStacks, mockGetStackContent, mockGetEnvContent, mockEnvExists, @@ -18,6 +19,7 @@ const { mockGetStackUpdateStatus: vi.fn().mockReturnValue({}), mockUpsertStackUpdateStatus: vi.fn(), mockClearStackUpdateStatus: vi.fn(), + mockRecordStackCheckFailure: vi.fn(), mockGetSystemState: vi.fn().mockReturnValue('1'), // default: backfilled mockSetSystemState: vi.fn(), mockAddNotificationHistory: vi.fn(), @@ -48,6 +50,7 @@ vi.mock('../services/DatabaseService', () => ({ upsertStackUpdateStatus: mockUpsertStackUpdateStatus, getStackUpdateStatus: mockGetStackUpdateStatus, clearStackUpdateStatus: mockClearStackUpdateStatus, + recordStackCheckFailure: mockRecordStackCheckFailure, getSystemState: mockGetSystemState, setSystemState: mockSetSystemState, addNotificationHistory: mockAddNotificationHistory, @@ -129,10 +132,10 @@ describe('ImageUpdateService - image ref parsing (via checkImage)', () => { } as any; } - it('returns { hasUpdate: false } for sha256-only refs', async () => { + it('marks sha256-only refs not-checkable (no tag to track)', async () => { const docker = makeMockDocker(); const result = await service.checkImage(docker, 'sha256:abc123'); - expect(result).toEqual({ hasUpdate: false }); + expect(result).toEqual({ hasUpdate: false, notCheckable: true }); }); it('returns error when local image inspect fails', async () => { @@ -165,17 +168,21 @@ describe('ImageUpdateService - image ref parsing (via checkImage)', () => { } }); - it('returns { hasUpdate: false } when no RepoDigests match', async () => { - // Empty RepoDigests means locally built image + it('marks an image with no RepoDigests not-checkable (locally built)', async () => { + // Empty RepoDigests means locally built / not registry-backed. const docker = makeMockDocker([]); const result = await service.checkImage(docker, 'nginx:latest'); - expect(result).toEqual({ hasUpdate: false }); + expect(result).toEqual({ hasUpdate: false, notCheckable: true }); }); - it('returns { hasUpdate: false } when RepoDigests have no sha256', async () => { + it('errors when RepoDigests are present but none resolves a digest', async () => { + // A non-empty set with no usable sha256 digest is ambiguous: surface it as an + // error rather than a silent "up to date". const docker = makeMockDocker(['library/nginx:latest']); const result = await service.checkImage(docker, 'nginx:latest'); - expect(result).toEqual({ hasUpdate: false }); + expect(result.hasUpdate).toBe(false); + expect(result.notCheckable).toBeUndefined(); + expect(result.error).toContain('Could not resolve a local registry digest'); }); }); @@ -376,6 +383,7 @@ services: getStackUpdateStatus: mockGetStackUpdateStatus, upsertStackUpdateStatus: mockUpsertStackUpdateStatus, clearStackUpdateStatus: mockClearStackUpdateStatus, + recordStackCheckFailure: mockRecordStackCheckFailure, getSystemState: mockGetSystemState, setSystemState: mockSetSystemState, addNotificationHistory: mockAddNotificationHistory, @@ -413,7 +421,7 @@ services: expect.stringContaining('stackA'), { stackName: 'stackA', actor: 'system:image-update' }, ); - expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number)); + expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number), 'ok', null); }); it('does not re-fire notification for a stack already known to have updates', async () => { @@ -469,6 +477,115 @@ services: }); }); +// ── Tri-state check status (ok / partial / failed) ──────────────────────── + +describe('ImageUpdateService - check status derivation', () => { + const COMPOSE_ONE = ` +services: + app: + image: nginx:latest +`; + const COMPOSE_TWO = ` +services: + app: + image: nginx:latest + db: + image: postgres:15 +`; + + const fakeDb = () => ({ + getStackUpdateStatus: mockGetStackUpdateStatus, + upsertStackUpdateStatus: mockUpsertStackUpdateStatus, + clearStackUpdateStatus: mockClearStackUpdateStatus, + recordStackCheckFailure: mockRecordStackCheckFailure, + getSystemState: mockGetSystemState, + setSystemState: mockSetSystemState, + addNotificationHistory: mockAddNotificationHistory, + }); + + // Per-image stub so a stack can mix ok / errored / not-checkable results. + function stubCheckImageByRef(service: ImageUpdateService, byRef: Record) { + (service as any).checkImage = vi.fn().mockImplementation((_docker: unknown, imageRef: string) => + Promise.resolve(byRef[imageRef] ?? { hasUpdate: false }), + ); + } + + beforeEach(() => { + vi.clearAllMocks(); + (ImageUpdateService as any).instance = undefined; + mockGetSystemState.mockReturnValue('1'); + mockGetAllContainers.mockResolvedValue([]); + mockEnvExists.mockResolvedValue(false); + }); + + it('records a failure (preserving has_update) and does not notify when every image errors', async () => { + // Even with no prior update (previousState false), a failed check must not + // fire a notification, and must not write has_update via the normal upsert. + mockGetStacks.mockResolvedValue(['stackA']); + mockGetStackContent.mockResolvedValue(COMPOSE_ONE); + mockGetStackUpdateStatus.mockReturnValue({ stackA: true }); + const service = ImageUpdateService.getInstance(); + stubCheckImageByRef(service, { 'nginx:latest': { hasUpdate: false, error: 'Registry unreachable for registry-1.docker.io/library/nginx:latest' } }); + + await (service as any).checkNode(1, 'local', fakeDb()); + + expect(mockRecordStackCheckFailure).toHaveBeenCalledWith(1, 'stackA', expect.stringContaining('Registry unreachable'), expect.any(Number)); + expect(mockUpsertStackUpdateStatus).not.toHaveBeenCalled(); + expect(mockDispatchAlert).not.toHaveBeenCalled(); + }); + + it('marks a stack partial (with a reason) when some images error but others resolve', async () => { + mockGetStacks.mockResolvedValue(['stackA']); + mockGetStackContent.mockResolvedValue(COMPOSE_TWO); + mockGetStackUpdateStatus.mockReturnValue({ stackA: false }); + const service = ImageUpdateService.getInstance(); + stubCheckImageByRef(service, { + 'nginx:latest': { hasUpdate: true }, + 'postgres:15': { hasUpdate: false, error: 'Registry unreachable for registry-1.docker.io/library/postgres:15' }, + }); + + await (service as any).checkNode(1, 'local', fakeDb()); + + expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number), 'partial', expect.stringContaining('Registry unreachable')); + expect(mockRecordStackCheckFailure).not.toHaveBeenCalled(); + // A confirmed update on an ok image still notifies on the false->true transition. + expect(mockDispatchAlert).toHaveBeenCalledTimes(1); + }); + + it('preserves a previously confirmed update through a partial check and does not re-notify', async () => { + // Stack had a confirmed update (previousState true). This scan: the updated + // image errors, the other resolves clean. A partial check must not erase the + // known update (which would re-fire the notification when the image recovers). + mockGetStacks.mockResolvedValue(['stackA']); + mockGetStackContent.mockResolvedValue(COMPOSE_TWO); + mockGetStackUpdateStatus.mockReturnValue({ stackA: true }); + const service = ImageUpdateService.getInstance(); + stubCheckImageByRef(service, { + 'nginx:latest': { hasUpdate: false, error: 'Registry unreachable for registry-1.docker.io/library/nginx:latest' }, + 'postgres:15': { hasUpdate: false }, + }); + + await (service as any).checkNode(1, 'local', fakeDb()); + + expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number), 'partial', expect.stringContaining('Registry unreachable')); + expect(mockRecordStackCheckFailure).not.toHaveBeenCalled(); + expect(mockDispatchAlert).not.toHaveBeenCalled(); + }); + + it('treats a stack whose only image is not-checkable as ok, not failed', async () => { + mockGetStacks.mockResolvedValue(['stackA']); + mockGetStackContent.mockResolvedValue(COMPOSE_ONE); + mockGetStackUpdateStatus.mockReturnValue({ stackA: false }); + const service = ImageUpdateService.getInstance(); + stubCheckImageByRef(service, { 'nginx:latest': { hasUpdate: false, notCheckable: true } }); + + await (service as any).checkNode(1, 'local', fakeDb()); + + expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', false, expect.any(Number), 'ok', null); + expect(mockRecordStackCheckFailure).not.toHaveBeenCalled(); + }); +}); + // ── .env file handling ────────────────────────────────────────────────── describe('ImageUpdateService - .env file handling in checkNode', () => { @@ -482,6 +599,7 @@ services: getStackUpdateStatus: mockGetStackUpdateStatus, upsertStackUpdateStatus: mockUpsertStackUpdateStatus, clearStackUpdateStatus: mockClearStackUpdateStatus, + recordStackCheckFailure: mockRecordStackCheckFailure, getSystemState: mockGetSystemState, setSystemState: mockSetSystemState, addNotificationHistory: mockAddNotificationHistory, @@ -558,6 +676,7 @@ describe('ImageUpdateService - check() concurrency guard', () => { upsertStackUpdateStatus: mockUpsertStackUpdateStatus, getStackUpdateStatus: mockGetStackUpdateStatus, clearStackUpdateStatus: mockClearStackUpdateStatus, + recordStackCheckFailure: mockRecordStackCheckFailure, getSystemState: mockGetSystemState, setSystemState: mockSetSystemState, addNotificationHistory: mockAddNotificationHistory, @@ -885,6 +1004,7 @@ services: getStackUpdateStatus: mockGetStackUpdateStatus, upsertStackUpdateStatus: mockUpsertStackUpdateStatus, clearStackUpdateStatus: mockClearStackUpdateStatus, + recordStackCheckFailure: mockRecordStackCheckFailure, getSystemState: mockGetSystemState, setSystemState: mockSetSystemState, addNotificationHistory: mockAddNotificationHistory, @@ -939,6 +1059,7 @@ services: getStackUpdateStatus: mockGetStackUpdateStatus, upsertStackUpdateStatus: mockUpsertStackUpdateStatus, clearStackUpdateStatus: mockClearStackUpdateStatus, + recordStackCheckFailure: mockRecordStackCheckFailure, getSystemState: mockGetSystemState, setSystemState: mockSetSystemState, addNotificationHistory: mockAddNotificationHistory, diff --git a/backend/src/__tests__/image-updates-routes.test.ts b/backend/src/__tests__/image-updates-routes.test.ts index c10fac30..0743ed4d 100644 --- a/backend/src/__tests__/image-updates-routes.test.ts +++ b/backend/src/__tests__/image-updates-routes.test.ts @@ -46,6 +46,26 @@ describe('GET /api/image-updates', () => { }); }); +describe('GET /api/image-updates/detail', () => { + it('rejects unauthenticated requests with 401', async () => { + const res = await request(app).get('/api/image-updates/detail'); + expect(res.status).toBe(401); + }); + + it('returns the rich per-stack detail shape for authenticated users', async () => { + const nodeId = DatabaseService.getInstance().getDefaultNode()!.id!; + DatabaseService.getInstance().upsertStackUpdateStatus(nodeId, 'detail-web', true, 1000, 'partial', 'Registry unreachable for ghcr.io/acme/api:v1'); + const res = await request(app).get('/api/image-updates/detail').set('Cookie', adminCookie); + expect(res.status).toBe(200); + expect(res.body['detail-web']).toEqual({ + hasUpdate: true, + checkStatus: 'partial', + lastError: 'Registry unreachable for ghcr.io/acme/api:v1', + checkedAt: 1000, + }); + }); +}); + describe('POST /api/image-updates/refresh', () => { it('rejects unauthenticated requests with 401', async () => { const res = await request(app).post('/api/image-updates/refresh'); diff --git a/backend/src/__tests__/registry-api.test.ts b/backend/src/__tests__/registry-api.test.ts new file mode 100644 index 00000000..6f0f07bc --- /dev/null +++ b/backend/src/__tests__/registry-api.test.ts @@ -0,0 +1,134 @@ +/** + * Unit tests for the registry HTTP client: digest-name matching (the local + * RepoDigest vs image-ref comparison) and getRemoteDigest's HEAD-first lookup + * with GET fallback. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { EventEmitter } from 'events'; + +// ── Configurable https mock ─────────────────────────────────────────────── +// getRemoteDigest first fetches an auth token, then probes the manifest. The +// mock routes by URL + method so each test controls the manifest response while +// the token request always succeeds. + +interface FakeResp { statusCode: number; headers: Record; body?: string; } + +const calls: { url: string; method: string }[] = []; +let route: (url: string, method: string) => FakeResp; + +function fakeRequest(url: string, options: { method?: string }, cb: (res: EventEmitter & { statusCode: number; headers: Record }) => void) { + const method = options?.method ?? 'GET'; + calls.push({ url, method }); + const resp = route(url, method); + const res = Object.assign(new EventEmitter(), { statusCode: resp.statusCode, headers: resp.headers }); + const req = Object.assign(new EventEmitter(), { + setTimeout: () => {}, + destroy: () => {}, + end: () => { + cb(res); + queueMicrotask(() => { + if (resp.body) res.emit('data', Buffer.from(resp.body)); + res.emit('end'); + }); + }, + }); + return req; +} + +vi.mock('https', () => ({ default: { request: (...args: unknown[]) => fakeRequest(...(args as Parameters)) } })); +vi.mock('http', () => ({ default: { request: (...args: unknown[]) => fakeRequest(...(args as Parameters)) } })); + +import { repoDigestMatchesRef, getRemoteDigest, parseImageRef } from '../services/registry-api'; + +const TOKEN_BODY = JSON.stringify({ token: 'test-token' }); +const REMOTE = 'sha256:remote000000000000000000000000000000000000000000000000000000'; + +function tokenOk(url: string): FakeResp | null { + if (url.includes('auth.docker.io/token')) return { statusCode: 200, headers: {}, body: TOKEN_BODY }; + return null; +} + +describe('repoDigestMatchesRef', () => { + const parsed = (ref: string) => { + const p = parseImageRef(ref); + if (!p) throw new Error(`unparseable ${ref}`); + return p; + }; + + it('matches an official library image whose RepoDigest omits the library/ prefix', () => { + // The exact false-negative the old substring check missed. + expect(repoDigestMatchesRef('nginx@sha256:abc', parsed('nginx:latest'))).toBe(true); + }); + + it('matches a namespaced Docker Hub image', () => { + expect(repoDigestMatchesRef('linuxserver/sonarr@sha256:abc', parsed('linuxserver/sonarr:latest'))).toBe(true); + }); + + it('treats docker.io / index.docker.io / registry-1.docker.io as the same registry', () => { + expect(repoDigestMatchesRef('docker.io/library/nginx@sha256:abc', parsed('nginx:latest'))).toBe(true); + }); + + it('matches a private-registry image by registry + repo', () => { + expect(repoDigestMatchesRef('ghcr.io/acme/api@sha256:abc', parsed('ghcr.io/acme/api:v1'))).toBe(true); + }); + + it('does not match a different repository', () => { + expect(repoDigestMatchesRef('redis@sha256:abc', parsed('nginx:latest'))).toBe(false); + }); + + it('returns false for an entry without a digest', () => { + expect(repoDigestMatchesRef('nginx:latest', parsed('nginx:latest'))).toBe(false); + }); +}); + +describe('getRemoteDigest HEAD-first lookup', () => { + beforeEach(() => { + calls.length = 0; + }); + + it('returns the digest from a HEAD 200 without issuing a GET', async () => { + route = (url, method) => tokenOk(url) ?? ( + method === 'HEAD' + ? { statusCode: 200, headers: { 'docker-content-digest': REMOTE } } + : { statusCode: 500, headers: {} } + ); + const digest = await getRemoteDigest('registry-1.docker.io', 'library/nginx', 'latest'); + expect(digest).toBe(REMOTE); + const manifestCalls = calls.filter(c => c.url.includes('/manifests/')); + expect(manifestCalls).toHaveLength(1); + expect(manifestCalls[0].method).toBe('HEAD'); + }); + + it('falls back to GET when the registry rejects HEAD with 405', async () => { + route = (url, method) => tokenOk(url) ?? ( + method === 'HEAD' + ? { statusCode: 405, headers: {} } + : { statusCode: 200, headers: { 'docker-content-digest': REMOTE } } + ); + const digest = await getRemoteDigest('registry-1.docker.io', 'library/nginx', 'latest'); + expect(digest).toBe(REMOTE); + expect(calls.filter(c => c.url.includes('/manifests/')).map(c => c.method)).toEqual(['HEAD', 'GET']); + }); + + it('falls back to GET when HEAD 200 omits the digest header', async () => { + route = (url, method) => tokenOk(url) ?? ( + method === 'HEAD' + ? { statusCode: 200, headers: {} } + : { statusCode: 200, headers: { 'docker-content-digest': REMOTE } } + ); + const digest = await getRemoteDigest('registry-1.docker.io', 'library/nginx', 'latest'); + expect(digest).toBe(REMOTE); + expect(calls.filter(c => c.url.includes('/manifests/')).map(c => c.method)).toEqual(['HEAD', 'GET']); + }); + + it('returns null on a hard HEAD failure (429) without a GET retry', async () => { + route = (url, method) => tokenOk(url) ?? ( + method === 'HEAD' + ? { statusCode: 429, headers: {} } + : { statusCode: 200, headers: { 'docker-content-digest': REMOTE } } + ); + const digest = await getRemoteDigest('registry-1.docker.io', 'library/nginx', 'latest'); + expect(digest).toBeNull(); + expect(calls.filter(c => c.url.includes('/manifests/')).map(c => c.method)).toEqual(['HEAD']); + }); +}); diff --git a/backend/src/routes/imageUpdates.ts b/backend/src/routes/imageUpdates.ts index 32d9615b..5b081268 100644 --- a/backend/src/routes/imageUpdates.ts +++ b/backend/src/routes/imageUpdates.ts @@ -37,6 +37,19 @@ imageUpdatesRouter.get('/', authMiddleware, (req: Request, res: Response): void } }); +// Rich per-stack status (hasUpdate + check outcome + reason) for the sidebar and +// readiness view. Auth-only, matching GET /; the boolean GET / is left intact so +// the cross-version fleet aggregation contract is unaffected. +imageUpdatesRouter.get('/detail', authMiddleware, (req: Request, res: Response): void => { + try { + const nodeId = req.nodeId ?? NodeRegistry.getInstance().getDefaultNodeId(); + res.json(DatabaseService.getInstance().getStackUpdateDetail(nodeId)); + } catch (error) { + console.error('Failed to fetch image update detail:', error); + res.status(500).json({ error: 'Failed to fetch image update detail' }); + } +}); + imageUpdatesRouter.post('/refresh', authMiddleware, (req: Request, res: Response): void => { if (!requireAdmin(req, res)) return; try { diff --git a/backend/src/services/DatabaseService.ts b/backend/src/services/DatabaseService.ts index d3458a90..5cf7cf49 100644 --- a/backend/src/services/DatabaseService.ts +++ b/backend/src/services/DatabaseService.ts @@ -24,6 +24,21 @@ export interface GlobalSetting { value: string; } +/** + * Per-stack image-update check outcome. 'ok' = every checkable image was + * reached; 'partial' = some checkable images errored; 'failed' = no checkable + * image could be reached (status undeterminable). Distinguishes a failed check + * from a confirmed "up to date". + */ +export type StackCheckStatus = 'ok' | 'partial' | 'failed'; + +export interface StackUpdateDetail { + hasUpdate: boolean; + checkStatus: StackCheckStatus; + lastError: string | null; + checkedAt: number; +} + export interface StackAlert { id?: number; stack_name: string; @@ -920,6 +935,8 @@ export class DatabaseService { node_id INTEGER NOT NULL DEFAULT 0, stack_name TEXT NOT NULL, has_update INTEGER DEFAULT 0, + check_status TEXT NOT NULL DEFAULT 'ok', + last_error TEXT, checked_at INTEGER NOT NULL, PRIMARY KEY (node_id, stack_name) ); @@ -1548,6 +1565,15 @@ export class DatabaseService { `); } + // Tri-state image-update check outcome. Must run AFTER the composite-PK + // recreate above (that block recreates the table from the original four + // columns, so columns added earlier would be dropped). 'ok' = every + // checkable image was reached; the detector records 'failed'/'partial' + // plus a reason when registry checks could not determine status, so a + // failed check is no longer indistinguishable from "up to date". + maybeAddCol('stack_update_status', 'check_status', "TEXT NOT NULL DEFAULT 'ok'"); + maybeAddCol('stack_update_status', 'last_error', 'TEXT'); + // Drop legacy SSH/TLS columns from pre-0.7 databases (no longer read or written) const legacyCols = ['host', 'port', 'ssh_port', 'ssh_user', 'ssh_password', 'ssh_key', 'tls_ca', 'tls_cert', 'tls_key']; for (const col of legacyCols) { @@ -3161,12 +3187,42 @@ export class DatabaseService { // --- Stack Update Status --- - public upsertStackUpdateStatus(nodeId: number, stackName: string, hasUpdate: boolean, checkedAt: number): void { + public upsertStackUpdateStatus( + nodeId: number, + stackName: string, + hasUpdate: boolean, + checkedAt: number, + checkStatus: StackCheckStatus = 'ok', + lastError: string | null = null, + ): void { this.db.prepare( - `INSERT INTO stack_update_status (node_id, stack_name, has_update, checked_at) - VALUES (?, ?, ?, ?) - ON CONFLICT(node_id, stack_name) DO UPDATE SET has_update = excluded.has_update, checked_at = excluded.checked_at` - ).run(nodeId, stackName, hasUpdate ? 1 : 0, checkedAt); + `INSERT INTO stack_update_status (node_id, stack_name, has_update, check_status, last_error, checked_at) + VALUES (?, ?, ?, ?, ?, ?) + ON CONFLICT(node_id, stack_name) DO UPDATE SET + has_update = excluded.has_update, + check_status = excluded.check_status, + last_error = excluded.last_error, + checked_at = excluded.checked_at` + ).run(nodeId, stackName, hasUpdate ? 1 : 0, checkStatus, lastError, checkedAt); + } + + /** + * Record a fully-failed check (no checkable image could be reached) without + * touching has_update, so a transient registry outage cannot erase a real + * update or flap the notification state. On an existing row it updates only + * check_status / last_error / checked_at, leaving has_update intact; a + * first-ever failed check inserts a row with has_update = 0 so the stack + * still appears with its failure reason. + */ + public recordStackCheckFailure(nodeId: number, stackName: string, lastError: string, checkedAt: number): void { + this.db.prepare( + `INSERT INTO stack_update_status (node_id, stack_name, has_update, check_status, last_error, checked_at) + VALUES (?, ?, 0, 'failed', ?, ?) + ON CONFLICT(node_id, stack_name) DO UPDATE SET + check_status = 'failed', + last_error = excluded.last_error, + checked_at = excluded.checked_at` + ).run(nodeId, stackName, lastError, checkedAt); } public getStackUpdateStatus(nodeId?: number): Record { @@ -3180,6 +3236,27 @@ export class DatabaseService { return result; } + /** + * Rich per-stack update status (hasUpdate + check outcome + reason) for the + * sidebar/readiness UI. GET /api/image-updates stays the boolean map so the + * cross-version fleet aggregation contract is unaffected. + */ + public getStackUpdateDetail(nodeId: number): Record { + const rows = this.db.prepare( + 'SELECT stack_name, has_update, check_status, last_error, checked_at FROM stack_update_status WHERE node_id = ?' + ).all(nodeId) as Array<{ stack_name: string; has_update: number; check_status: string | null; last_error: string | null; checked_at: number }>; + const result: Record = {}; + for (const row of rows) { + result[row.stack_name] = { + hasUpdate: row.has_update === 1, + checkStatus: (row.check_status === 'failed' || row.check_status === 'partial') ? row.check_status : 'ok', + lastError: row.last_error, + checkedAt: row.checked_at, + }; + } + return result; + } + public clearStackUpdateStatus(nodeId: number, stackName: string): void { this.db.prepare('DELETE FROM stack_update_status WHERE node_id = ? AND stack_name = ?').run(nodeId, stackName); } diff --git a/backend/src/services/ImageUpdateService.ts b/backend/src/services/ImageUpdateService.ts index 06924401..708a3396 100644 --- a/backend/src/services/ImageUpdateService.ts +++ b/backend/src/services/ImageUpdateService.ts @@ -8,7 +8,7 @@ import { RegistryService } from './RegistryService'; import { NodeRegistry } from './NodeRegistry'; import { NotificationService } from './NotificationService'; import { sanitizeNotificationMessage } from '../utils/notificationMessage'; -import { parseImageRef, getRemoteDigest } from './registry-api'; +import { parseImageRef, getRemoteDigest, repoDigestMatchesRef } from './registry-api'; import { isDebugEnabled } from '../utils/debug'; import { getErrorMessage } from '../utils/errors'; import { sanitizeForLog } from '../utils/safeLog'; @@ -18,6 +18,13 @@ const BACKFILL_KEY = 'image_update_notifications_backfilled'; export interface ImageCheckResult { hasUpdate: boolean; error?: string; + /** + * The image is not registry-backed (locally built, or a bare digest ref + * with no resolvable tag), so update status is not applicable. Distinct + * from `error`: such an image must be excluded from a stack's pass/fail + * tally rather than counted as a failed or up-to-date check. + */ + notCheckable?: boolean; } /** @@ -549,7 +556,9 @@ export class ImageUpdateService { imageUpdateMap.set(imageRef, await this.checkImage(docker, imageRef)); } catch (e) { console.error(`[ImageUpdateService] Error checking ${sanitizeForLog(imageRef)}:`, sanitizeForLog((e as Error)?.message ?? String(e))); - imageUpdateMap.set(imageRef, { hasUpdate: false, error: String(e) }); + // getErrorMessage (not raw String(e)) because this value can surface + // verbatim in the sidebar tooltip / readiness advisory as lastError. + imageUpdateMap.set(imageRef, { hasUpdate: false, error: getErrorMessage(e, 'Update check failed') }); } await sleep(ImageUpdateService.INTER_IMAGE_DELAY_MS); } @@ -566,7 +575,32 @@ export class ImageUpdateService { let updatesFound = 0; const newlyUpdated: string[] = []; for (const [stackName, images] of stackImages) { - const hasUpdate = Array.from(images).some(img => imageUpdateMap.get(img)?.hasUpdate === true); + // Tally only checkable images: a not-checkable image (locally built, + // or a bare digest ref) is neither a pass nor a failure. + const checkable = Array.from(images) + .map(img => imageUpdateMap.get(img)) + .filter((r): r is ImageCheckResult => !!r && !r.notCheckable); + const errored = checkable.filter(r => r.error !== undefined); + const confirmedHasUpdate = checkable.some(r => r.error === undefined && r.hasUpdate === true); + + // Every checkable image failed: status is undeterminable. Preserve the + // last-known has_update so a transient registry outage neither erases a + // real update nor flaps the notification state. + if (checkable.length > 0 && errored.length === checkable.length) { + db.recordStackCheckFailure(nodeId, stackName, errored[0].error ?? 'Update check failed', now); + continue; + } + + const checkStatus = errored.length > 0 ? 'partial' : 'ok'; + const lastError = errored.length > 0 ? (errored[0].error ?? null) : null; + // Only a fully-ok check is authoritative enough to lower has_update to + // false. On a partial check some image could not be reached, so a + // previously confirmed update is preserved rather than erased (which + // would also re-fire the notification when that image recovers). + const hasUpdate = checkStatus === 'partial' + ? (confirmedHasUpdate || previousState[stackName] === true) + : confirmedHasUpdate; + if (hasUpdate) { updatesFound++; // Notify only on state transition: was false/absent, now true @@ -574,7 +608,7 @@ export class ImageUpdateService { newlyUpdated.push(stackName); } } - db.upsertStackUpdateStatus(nodeId, stackName, hasUpdate, now); + db.upsertStackUpdateStatus(nodeId, stackName, hasUpdate, now, checkStatus, lastError); } // Dispatch notifications for stacks that newly have updates @@ -629,7 +663,8 @@ export class ImageUpdateService { public async checkImage(docker: DockerController, imageRef: string): Promise { const parsed = parseImageRef(imageRef); - if (!parsed) return { hasUpdate: false }; + // A bare digest ref (sha256:...) has no tag to track upstream; not applicable. + if (!parsed) return { hasUpdate: false, notCheckable: true }; if (isDebugEnabled()) { console.log(`[ImageUpdateService] Checking ${imageRef}: registry=${parsed.registry} repo=${parsed.repo} tag=${parsed.tag}`); @@ -647,11 +682,15 @@ export class ImageUpdateService { const inspect = await withTimeout(docker.getDocker().getImage(imageRef).inspect(), ImageUpdateService.SOCKET_TIMEOUT_MS, 'inspect'); const repoDigests: string[] = inspect.RepoDigests ?? []; + // No RepoDigests at all: locally built / not registry-backed, so update + // status does not apply. + if (repoDigests.length === 0) return { hasUpdate: false, notCheckable: true }; + for (const rd of repoDigests) { if (!rd.includes('@sha256:')) continue; const [, digest] = rd.split('@'); - if (rd.includes(parsed.repo) || rd.includes(parsed.registry) || repoDigests.length === 1) { + if (repoDigestMatchesRef(rd, parsed) || repoDigests.length === 1) { localDigest = digest; break; } @@ -660,7 +699,11 @@ export class ImageUpdateService { return { hasUpdate: false, error: `Failed to inspect local image "${imageRef}"` }; } - if (!localDigest) return { hasUpdate: false }; + // RepoDigests were present but none resolved a usable digest: genuinely + // ambiguous, so surface it rather than silently call the image up to date. + if (!localDigest) { + return { hasUpdate: false, error: `Could not resolve a local registry digest for "${imageRef}"` }; + } const remoteDigest = await getRemoteDigest(parsed.registry, parsed.repo, parsed.tag, credentials); if (!remoteDigest) { diff --git a/backend/src/services/registry-api.ts b/backend/src/services/registry-api.ts index 5776436e..d63712d7 100644 --- a/backend/src/services/registry-api.ts +++ b/backend/src/services/registry-api.ts @@ -50,8 +50,9 @@ export function parseImageRef(imageRef: string): ParsedRef | null { return { registry, repo: rest, tag }; } -export function httpGet( +export function httpRequest( url: string, + method: 'GET' | 'HEAD', headers: Record = {}, timeoutMs = 10000, ): Promise { @@ -63,7 +64,7 @@ export function httpGet( settled = true; fn(); }; - const req = lib.get(url, { headers }, (res) => { + const req = lib.request(url, { method, headers }, (res) => { let body = ''; res.on('data', (chunk: Buffer) => { body += chunk.toString(); }); res.on('end', () => finish(() => resolve({ @@ -79,9 +80,18 @@ export function httpGet( req.destroy(err); finish(() => reject(err)); }); + req.end(); }); } +export function httpGet( + url: string, + headers: Record = {}, + timeoutMs = 10000, +): Promise { + return httpRequest(url, 'GET', headers, timeoutMs); +} + export async function getAuthToken( registry: string, repo: string, @@ -129,6 +139,31 @@ const MANIFEST_ACCEPT = [ 'application/vnd.oci.image.manifest.v1+json', ].join(', '); +/** docker.io has three hostnames that all address the same registry. */ +function canonicalRegistry(host: string): string { + if (host === 'docker.io' || host === 'index.docker.io' || host === 'registry-1.docker.io') { + return 'docker.io'; + } + return host; +} + +/** + * True when a local RepoDigest entry ("name@sha256:...") refers to the same + * registry + repository as the parsed image ref. Parses the name side through + * the same normalization as the image ref (Docker Hub's implicit `library/` + * namespace and default registry), replacing a fragile substring check that + * missed `library/*` official images: their RepoDigests read `nginx@sha256:...`, + * never `library/nginx@...`, so `name.includes('library/nginx')` was false. + */ +export function repoDigestMatchesRef(repoDigest: string, parsed: ParsedRef): boolean { + const at = repoDigest.indexOf('@'); + if (at === -1) return false; + const parsedName = parseImageRef(repoDigest.slice(0, at)); + if (!parsedName) return false; + return canonicalRegistry(parsedName.registry) === canonicalRegistry(parsed.registry) + && parsedName.repo === parsed.repo; +} + export async function getRemoteDigest( registry: string, repo: string, @@ -139,10 +174,28 @@ export async function getRemoteDigest( const token = await getAuthToken(registry, repo, credentials); const headers: Record = { Accept: MANIFEST_ACCEPT }; if (token) headers['Authorization'] = `Bearer ${token}`; + const url = `https://${registry}/v2/${repo}/manifests/${tag}`; - const res = await httpGet(`https://${registry}/v2/${repo}/manifests/${tag}`, headers); + // HEAD first: the registry returns docker-content-digest without + // transferring the manifest body, so it does not draw down Docker Hub's + // anonymous pull-rate budget the way a GET does (a GET can self-inflict a + // 429). Fall back to GET only when the registry rejects HEAD (405/501) or + // omits the digest header on a 200. A 401/403/404/429/5xx HEAD returns + // null without a GET retry: the bearer token is fetched up-front, so a + // 401 here is a real auth failure, not a token-scope challenge to retry. + const head = await httpRequest(url, 'HEAD', headers); + if (head.statusCode === 200) { + const digest = head.headers['docker-content-digest']; + if (typeof digest === 'string') return digest; + } else if (head.statusCode !== 405 && head.statusCode !== 501) { + // 401/403/404/429/5xx: a GET would fail the same way. Report as unreachable. + return null; + } + + const res = await httpRequest(url, 'GET', headers); if (res.statusCode !== 200) return null; - return (res.headers['docker-content-digest'] as string) ?? null; + const digest = res.headers['docker-content-digest']; + return typeof digest === 'string' ? digest : null; } catch { return null; } diff --git a/docs/features/auto-update-policies.mdx b/docs/features/auto-update-policies.mdx index 638b261c..4bddd2a9 100644 --- a/docs/features/auto-update-policies.mdx +++ b/docs/features/auto-update-policies.mdx @@ -45,6 +45,25 @@ The interval is node-scoped: each node runs its own scanner on its own cadence, The readiness hero shows this instance's cadence at a glance: when it last checked, when the next check is due, and, right after a manual **Recheck**, how long the 2-minute cooldown has left. +## When a check cannot complete + +A registry check can fail for reasons that have nothing to do with whether an update exists: the registry is unreachable, an authenticated registry has no stored credentials, or Docker Hub's anonymous pull-rate limit has been hit. Sencho treats these as a distinct **check failed** state instead of reporting "up to date", so a failed check is never mistaken for a current image. + +Where it shows: + +- **Sidebar.** A stack whose latest check could not be determined shows a muted indicator on its row. Hover it to read the reason (for example, "Registry unreachable for ghcr.io/acme/api:v1"). +- **Update board.** Stacks whose check failed appear in a "could not be checked" advisory above the card grid, each with its reason, so a stack with no confirmed update is never silently absent. + +A confirmed update from an earlier successful check is kept through a later failed check, so a momentary registry blip does not make a pending update vanish. + +Common causes and how to clear them: + +- **Private or custom registry with no credentials.** Add the registry under **Settings > Registries** so detection can authenticate the same way deploys do. +- **Docker Hub rate limit.** Anonymous pulls are capped per IP address. Signing in to Docker Hub under **Settings > Registries** raises the limit. +- **Blocked network egress.** The node must reach the registry host over HTTPS to read image manifests. + +Once the cause is resolved, the next check (on the interval, or via **Recheck**) clears the failed state. + ## Workflow 1. Open **Update** from the top nav strip. diff --git a/docs/features/sidebar.mdx b/docs/features/sidebar.mdx index 03b2f4b7..a6a568a8 100644 --- a/docs/features/sidebar.mdx +++ b/docs/features/sidebar.mdx @@ -55,7 +55,7 @@ Each row gives you everything you need to read the stack at a glance, in a fixed - **Status pill** on the left. Two uppercase letters in mono type, or a spinner while a lifecycle action is in flight. `UP` (green) means the stack is running with nothing crashed, `DN` (red) means the stack is stopped, and `PT` (amber) means the stack is partially running: at least one container is up and at least one has crashed (exited with an error, died, or is restart-looping). Hover the `PT` pill to see how many containers are running, such as `3/5 running`. A stack whose only stopped container finished cleanly (an init job that exited without error) stays `UP`. - **Stack name** in mono type, truncated with an ellipsis when the row gets tight. - **Label dots** to the right of the name. Up to three colored dots representing the stack's labels render here. If a stack carries more than three labels, a **+N** counter appears for the extras. -- **Update indicator**. When a stack has an image update pending, an extra colored dot appears alongside the label dots. If only a Git source update is pending (no image update), a small Git branch icon shows instead. The image-update dot takes priority when both apply. +- **Update indicator**. When a stack has an image update pending, an extra colored dot appears alongside the label dots. If the last registry check could not be determined (registry unreachable, missing credentials, rate limit), a muted "couldn't check" icon shows instead, with the reason on hover. If only a Git source update is pending (no image update), a small Git branch icon shows. Priority is update dot, then check-failed, then Git pending. - **Hover kebab** on the right edge. Hover the row to reveal a vertical three-dot menu that opens the same actions as right-clicking the row. diff --git a/frontend/src/components/AutoUpdateReadinessView.tsx b/frontend/src/components/AutoUpdateReadinessView.tsx index 54adc7a4..0e642dc4 100644 --- a/frontend/src/components/AutoUpdateReadinessView.tsx +++ b/frontend/src/components/AutoUpdateReadinessView.tsx @@ -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 ( +
+
+
+
    + {failures.map(f => ( +
  • + {f.stack}{f.reason ? `: ${f.reason}` : ''} +
    • + ))} +
+
+ ); +} + 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(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 | 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; + 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. )} + {loading && groups.length === 0 ? (
Loading readiness...
) : groups.length === 0 ? ( @@ -839,6 +886,8 @@ function AutoUpdateReadinessContent({ headerActions }: AutoUpdateReadinessProps) )} + + {loading && groups.length === 0 ? (
Loading readiness... diff --git a/frontend/src/components/EditorLayout/hooks/useStackListState.ts b/frontend/src/components/EditorLayout/hooks/useStackListState.ts index afc4faf2..9dd2075d 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackListState.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackListState.ts @@ -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]); diff --git a/frontend/src/components/__tests__/AutoUpdateReadinessView.test.tsx b/frontend/src/components/__tests__/AutoUpdateReadinessView.test.tsx index 8e396d1a..a993ab6a 100644 --- a/frontend/src/components/__tests__/AutoUpdateReadinessView.test.tsx +++ b/frontend/src/components/__tests__/AutoUpdateReadinessView.test.tsx @@ -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; + const mockedFetchForNode = fetchForNode as unknown as ReturnType; + + 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(); + + 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 diff --git a/frontend/src/components/sidebar/StackList.tsx b/frontend/src/components/sidebar/StackList.tsx index 9b60cc0c..ee8ded36 100644 --- a/frontend/src/components/sidebar/StackList.tsx +++ b/frontend/src/components/sidebar/StackList.tsx @@ -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; stackStatuses: Record; stackCounts: Record; - stackUpdates: Record; + stackUpdates: Record; gitSourcePendingMap: Record; 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={} diff --git a/frontend/src/components/sidebar/StackRow.tsx b/frontend/src/components/sidebar/StackRow.tsx index 6c75f5c1..6b53f4c8 100644 --- a/frontend/src/components/sidebar/StackRow.tsx +++ b/frontend/src/components/sidebar/StackRow.tsx @@ -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) { )} - {/* Fixed trailing icon slot: update dot takes priority over git pending */} + {/* Fixed trailing icon slot: update dot > check-failed > git pending */} {hasUpdate ? ( + ) : checkStatus === 'failed' ? ( + } + label={lastError ? `Update check failed: ${lastError}` : 'Update check failed'} + /> ) : hasGitPending ? ( } diff --git a/frontend/src/components/sidebar/__tests__/StackRow.test.tsx b/frontend/src/components/sidebar/__tests__/StackRow.test.tsx index 1470164e..cff4deff 100644 --- a/frontend/src/components/sidebar/__tests__/StackRow.test.tsx +++ b/frontend/src/components/sidebar/__tests__/StackRow.test.tsx @@ -101,4 +101,26 @@ describe('StackRow', () => { const { container } = render(); 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(); + 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(); + expect(container.querySelector('.bg-update')).not.toBeNull(); + }); + + it('shows no trailing indicator for a clean ok check with no update', () => { + const { container } = render(); + expect(container.querySelector('[data-slot="cursor-container"]')).toBeNull(); + expect(container.querySelector('.bg-update')).toBeNull(); + }); }); diff --git a/frontend/src/hooks/__tests__/useImageUpdates.test.ts b/frontend/src/hooks/__tests__/useImageUpdates.test.ts new file mode 100644 index 00000000..235f7d02 --- /dev/null +++ b/frontend/src/hooks/__tests__/useImageUpdates.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, waitFor } from '@testing-library/react'; + +vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); + +import { apiFetch } from '@/lib/api'; +import { useImageUpdates } from '../useImageUpdates'; + +const mockedFetch = apiFetch as unknown as ReturnType; + +describe('useImageUpdates', () => { + beforeEach(() => { + mockedFetch.mockReset(); + }); + + it('loads the rich detail map from /image-updates/detail', async () => { + mockedFetch.mockImplementation((url: string) => { + if (url === '/image-updates/detail') { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + web: { hasUpdate: true, checkStatus: 'ok', lastError: null, checkedAt: 5 }, + api: { hasUpdate: false, checkStatus: 'failed', lastError: 'Registry unreachable', checkedAt: 6 }, + }), + }); + } + return Promise.resolve({ ok: false, status: 404, json: async () => ({}) }); + }); + + const { result } = renderHook(() => useImageUpdates(1)); + + await waitFor(() => expect(result.current.stackUpdates.web).toBeDefined()); + expect(result.current.stackUpdates.web.hasUpdate).toBe(true); + expect(result.current.stackUpdates.api.checkStatus).toBe('failed'); + expect(result.current.stackUpdates.api.lastError).toBe('Registry unreachable'); + }); + + it('falls back to the boolean map when /detail 404s (older remote node)', async () => { + mockedFetch.mockImplementation((url: string) => { + if (url === '/image-updates/detail') { + return Promise.resolve({ ok: false, status: 404, json: async () => ({}) }); + } + if (url === '/image-updates') { + return Promise.resolve({ ok: true, status: 200, json: async () => ({ web: true, api: false }) }); + } + return Promise.resolve({ ok: false, status: 500, json: async () => ({}) }); + }); + + const { result } = renderHook(() => useImageUpdates(1)); + + await waitFor(() => expect(result.current.stackUpdates.web).toBeDefined()); + // Boolean map is synthesized into the rich shape with checkStatus 'ok'. + expect(result.current.stackUpdates.web).toEqual({ hasUpdate: true, checkStatus: 'ok', lastError: null, checkedAt: 0 }); + expect(result.current.stackUpdates.api.hasUpdate).toBe(false); + }); +}); diff --git a/frontend/src/hooks/useImageUpdates.ts b/frontend/src/hooks/useImageUpdates.ts index a2aa1ee5..a67673a0 100644 --- a/frontend/src/hooks/useImageUpdates.ts +++ b/frontend/src/hooks/useImageUpdates.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback, useRef } from 'react'; import { apiFetch } from '@/lib/api'; +import type { StackUpdateInfo } from '@/types/imageUpdates'; const IMAGE_UPDATE_POLL_MS = 5 * 60 * 1000; @@ -15,15 +16,34 @@ const IMAGE_UPDATE_POLL_MS = 5 * 60 * 1000; * through the active-node header just like before. */ export function useImageUpdates(activeNodeId: number | undefined) { - const [stackUpdates, setStackUpdates] = useState>({}); + const [stackUpdates, setStackUpdates] = useState>({}); const refresh = useCallback(async () => { try { - const res = await apiFetch('/image-updates'); + const res = await apiFetch('/image-updates/detail'); if (res.ok) { - const data = await res.json() as Record; - setStackUpdates(data); + setStackUpdates(await res.json() as Record); + return; } + // A remote node on an older Sencho lacks /detail; fall back to the boolean + // map so update badges keep working until that node is upgraded. + if (res.status === 404) { + const boolRes = await apiFetch('/image-updates'); + if (boolRes.ok) { + const bool = await boolRes.json() as Record; + const synthesized: Record = {}; + for (const [stack, hasUpdate] of Object.entries(bool)) { + synthesized[stack] = { hasUpdate, checkStatus: 'ok', lastError: null, checkedAt: 0 }; + } + setStackUpdates(synthesized); + } else { + console.error('[ImageUpdates] /detail 404 fallback to /image-updates failed:', boolRes.status); + } + return; + } + // Any other non-ok (500, or a proxy 5xx from an unreachable remote): keep + // the last-known state on screen, but do not let the failure go silent. + console.error('[ImageUpdates] /image-updates/detail returned', res.status); } catch (e: unknown) { console.error('[ImageUpdates] fetch failed:', e); } diff --git a/frontend/src/types/imageUpdates.ts b/frontend/src/types/imageUpdates.ts index e6a029bc..9fe4701e 100644 --- a/frontend/src/types/imageUpdates.ts +++ b/frontend/src/types/imageUpdates.ts @@ -23,3 +23,22 @@ export interface ImageUpdateStatus { /** 5-field cron expression when mode is 'cron', null otherwise. */ cronExpression: string | null; } + +/** + * Per-stack image-update check outcome. 'ok' = every checkable image was + * reached; 'partial' = some checkable images errored; 'failed' = no checkable + * image could be reached, so update status is undeterminable (distinct from a + * confirmed "up to date"). + */ +export type CheckStatus = 'ok' | 'partial' | 'failed'; + +/** + * Rich per-stack update status from `GET /api/image-updates/detail`. `lastError` + * carries the failure reason when `checkStatus` is 'failed' or 'partial'. + */ +export interface StackUpdateInfo { + hasUpdate: boolean; + checkStatus: CheckStatus; + lastError: string | null; + checkedAt: number; +}