mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-16 21:48:45 +00:00
fix(notifications): resolve version notification showing 0.0.0 and backfill missing image update notifications (#586)
- Read running Sencho version from the packaged manifest via getSenchoVersion() instead of process.env.npm_package_version, which is undefined when launched via node dist/index.js (production Docker). Skip the update check entirely when the version cannot be resolved. - Add a one-time backfill pass in ImageUpdateService so users who upgraded to a Sencho version with the notification pipeline receive a catch-up entry for stacks already flagged as having updates before the upgrade. - Surface dispatch failures as error-level entries in the in-app notification bell via a direct notification_history write, so misconfigured webhooks are visible without tailing logs. - Extend test coverage for both paths: mock getSenchoVersion (including the null-version case) and add dispatch-path tests for transition, no-re-fire, backfill, and error surfacing. - Expand alerts-notifications docs with per-instance 6-hour check cadence, an example version message, a backfill note, and three new troubleshooting entries.
This commit is contained in:
@@ -6,8 +6,26 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
// ── Hoisted mocks ──────────────────────────────────────────────────────
|
||||
|
||||
const { mockGetAuthForRegistry } = vi.hoisted(() => ({
|
||||
const {
|
||||
mockGetAuthForRegistry,
|
||||
mockGetStackUpdateStatus, mockUpsertStackUpdateStatus, mockClearStackUpdateStatus,
|
||||
mockGetSystemState, mockSetSystemState, mockAddNotificationHistory,
|
||||
mockDispatchAlert,
|
||||
mockGetStacks, mockGetStackContent, mockGetEnvContent,
|
||||
mockGetAllContainers,
|
||||
} = vi.hoisted(() => ({
|
||||
mockGetAuthForRegistry: vi.fn().mockResolvedValue(null),
|
||||
mockGetStackUpdateStatus: vi.fn().mockReturnValue({}),
|
||||
mockUpsertStackUpdateStatus: vi.fn(),
|
||||
mockClearStackUpdateStatus: vi.fn(),
|
||||
mockGetSystemState: vi.fn().mockReturnValue('1'), // default: backfilled
|
||||
mockSetSystemState: vi.fn(),
|
||||
mockAddNotificationHistory: vi.fn(),
|
||||
mockDispatchAlert: vi.fn().mockResolvedValue(undefined),
|
||||
mockGetStacks: vi.fn().mockResolvedValue([]),
|
||||
mockGetStackContent: vi.fn().mockResolvedValue(''),
|
||||
mockGetEnvContent: vi.fn().mockRejectedValue(new Error('no env')),
|
||||
mockGetAllContainers: vi.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
vi.mock('../services/RegistryService', () => ({
|
||||
@@ -23,19 +41,40 @@ vi.mock('../services/DatabaseService', () => ({
|
||||
getInstance: () => ({
|
||||
getGlobalSettings: () => ({ developer_mode: '0' }),
|
||||
getNodes: () => [],
|
||||
upsertStackUpdateStatus: vi.fn(),
|
||||
getStackUpdateStatus: () => ({}),
|
||||
clearStackUpdateStatus: vi.fn(),
|
||||
upsertStackUpdateStatus: mockUpsertStackUpdateStatus,
|
||||
getStackUpdateStatus: mockGetStackUpdateStatus,
|
||||
clearStackUpdateStatus: mockClearStackUpdateStatus,
|
||||
getSystemState: mockGetSystemState,
|
||||
setSystemState: mockSetSystemState,
|
||||
addNotificationHistory: mockAddNotificationHistory,
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../services/NotificationService', () => ({
|
||||
NotificationService: {
|
||||
getInstance: () => ({
|
||||
dispatchAlert: mockDispatchAlert,
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../services/FileSystemService', () => ({
|
||||
FileSystemService: { getInstance: () => ({}) },
|
||||
FileSystemService: {
|
||||
getInstance: () => ({
|
||||
getStacks: mockGetStacks,
|
||||
getStackContent: mockGetStackContent,
|
||||
getEnvContent: mockGetEnvContent,
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../services/DockerController', () => ({
|
||||
default: { getInstance: () => ({}) },
|
||||
default: {
|
||||
getInstance: () => ({
|
||||
getAllContainers: mockGetAllContainers,
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../services/NodeRegistry', () => ({
|
||||
@@ -298,3 +337,108 @@ services:
|
||||
expect(extractImagesFromCompose(yaml, {})).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Notification dispatch on state transitions ────────────────────────
|
||||
|
||||
describe('ImageUpdateService - notification dispatch', () => {
|
||||
const COMPOSE = `
|
||||
services:
|
||||
app:
|
||||
image: nginx:latest
|
||||
`;
|
||||
|
||||
const fakeDb = () => ({
|
||||
getStackUpdateStatus: mockGetStackUpdateStatus,
|
||||
upsertStackUpdateStatus: mockUpsertStackUpdateStatus,
|
||||
clearStackUpdateStatus: mockClearStackUpdateStatus,
|
||||
getSystemState: mockGetSystemState,
|
||||
setSystemState: mockSetSystemState,
|
||||
addNotificationHistory: mockAddNotificationHistory,
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
(ImageUpdateService as any).instance = undefined;
|
||||
// Default: backfill complete so transition logic applies normally.
|
||||
mockGetSystemState.mockReturnValue('1');
|
||||
mockGetStacks.mockResolvedValue(['stackA']);
|
||||
mockGetStackContent.mockResolvedValue(COMPOSE);
|
||||
mockGetAllContainers.mockResolvedValue([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* Stubs the private checkImage method so tests don't need to mock
|
||||
* the entire registry-fetch stack.
|
||||
*/
|
||||
function stubCheckImage(service: ImageUpdateService, hasUpdate: boolean) {
|
||||
(service as any).checkImage = vi.fn().mockResolvedValue({ hasUpdate });
|
||||
}
|
||||
|
||||
it('dispatches notification when a stack transitions from no-update to has-update', async () => {
|
||||
mockGetStackUpdateStatus.mockReturnValue({ stackA: false });
|
||||
const service = ImageUpdateService.getInstance();
|
||||
stubCheckImage(service, true);
|
||||
|
||||
await (service as any).checkNode(1, 'local', fakeDb());
|
||||
|
||||
expect(mockDispatchAlert).toHaveBeenCalledTimes(1);
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith(
|
||||
'info',
|
||||
expect.stringContaining('stackA'),
|
||||
'stackA',
|
||||
);
|
||||
expect(mockUpsertStackUpdateStatus).toHaveBeenCalledWith(1, 'stackA', true, expect.any(Number));
|
||||
});
|
||||
|
||||
it('does not re-fire notification for a stack already known to have updates', async () => {
|
||||
mockGetStackUpdateStatus.mockReturnValue({ stackA: true });
|
||||
const service = ImageUpdateService.getInstance();
|
||||
stubCheckImage(service, true);
|
||||
|
||||
await (service as any).checkNode(1, 'local', fakeDb());
|
||||
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('backfills catch-up notifications once for pre-existing has_update rows', async () => {
|
||||
// Simulate a stale DB: two stacks already have has_update = true,
|
||||
// but the backfill flag is not set.
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
mockGetStacks.mockResolvedValue(['stackA', 'stackB']);
|
||||
mockGetStackUpdateStatus.mockReturnValue({ stackA: true, stackB: true });
|
||||
const service = ImageUpdateService.getInstance();
|
||||
stubCheckImage(service, true);
|
||||
|
||||
await (service as any).checkNode(1, 'local', fakeDb());
|
||||
|
||||
expect(mockDispatchAlert).toHaveBeenCalledTimes(2);
|
||||
const dispatched = mockDispatchAlert.mock.calls.map(call => call[2]);
|
||||
expect(dispatched).toEqual(expect.arrayContaining(['stackA', 'stackB']));
|
||||
expect(mockSetSystemState).toHaveBeenCalledWith('image_update_notifications_backfilled', '1');
|
||||
|
||||
// Second run with backfill flag set and the same state: no further notifications.
|
||||
vi.clearAllMocks();
|
||||
mockGetSystemState.mockReturnValue('1');
|
||||
mockGetStacks.mockResolvedValue(['stackA', 'stackB']);
|
||||
mockGetStackUpdateStatus.mockReturnValue({ stackA: true, stackB: true });
|
||||
stubCheckImage(service, true);
|
||||
|
||||
await (service as any).checkNode(1, 'local', fakeDb());
|
||||
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('surfaces dispatch failures as an error entry in notification history', async () => {
|
||||
mockGetStackUpdateStatus.mockReturnValue({ stackA: false });
|
||||
mockDispatchAlert.mockRejectedValueOnce(new Error('webhook timeout'));
|
||||
const service = ImageUpdateService.getInstance();
|
||||
stubCheckImage(service, true);
|
||||
|
||||
await (service as any).checkNode(1, 'local', fakeDb());
|
||||
|
||||
expect(mockAddNotificationHistory).toHaveBeenCalledWith(expect.objectContaining({
|
||||
level: 'error',
|
||||
message: expect.stringContaining('webhook timeout'),
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,6 +15,7 @@ const { mockGetGlobalSettings, mockGetNodes, mockGetStackAlerts, mockAddContaine
|
||||
mockCurrentLoad, mockMem, mockFsSize,
|
||||
mockExecAsync,
|
||||
mockFetchLatestSenchoVersion,
|
||||
mockGetSenchoVersion,
|
||||
} = vi.hoisted(() => ({
|
||||
mockGetGlobalSettings: vi.fn().mockReturnValue({}),
|
||||
mockGetNodes: vi.fn().mockReturnValue([]),
|
||||
@@ -36,6 +37,7 @@ const { mockGetGlobalSettings, mockGetNodes, mockGetStackAlerts, mockAddContaine
|
||||
mockFsSize: vi.fn().mockResolvedValue([{ mount: '/', use: 30 }]),
|
||||
mockExecAsync: vi.fn().mockResolvedValue({ stdout: '' }),
|
||||
mockFetchLatestSenchoVersion: vi.fn().mockRejectedValue(new Error('not configured')),
|
||||
mockGetSenchoVersion: vi.fn().mockReturnValue(null),
|
||||
}));
|
||||
|
||||
vi.mock('../services/DatabaseService', () => ({
|
||||
@@ -70,6 +72,15 @@ vi.mock('../utils/version-check', () => ({
|
||||
fetchLatestSenchoVersion: (...args: unknown[]) => mockFetchLatestSenchoVersion(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../services/CapabilityRegistry', async () => {
|
||||
const semver = await import('semver');
|
||||
return {
|
||||
isValidVersion: (v: string | null | undefined): v is string =>
|
||||
!!v && v !== 'unknown' && v !== '0.0.0-dev' && !!semver.default.valid(v),
|
||||
getSenchoVersion: () => mockGetSenchoVersion(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../services/NotificationService', () => ({
|
||||
NotificationService: {
|
||||
getInstance: () => ({
|
||||
@@ -599,8 +610,7 @@ describe('MonitorService - restart_count metric', () => {
|
||||
|
||||
describe('MonitorService - Sencho version check', () => {
|
||||
it('dispatches notification when newer version available', async () => {
|
||||
// Set current version
|
||||
process.env.npm_package_version = '0.45.0';
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockFetchLatestSenchoVersion.mockResolvedValue('0.46.0');
|
||||
mockGetSystemState.mockReturnValue(null); // No previous notification
|
||||
mockGetGlobalSettings.mockReturnValue({});
|
||||
@@ -613,11 +623,13 @@ describe('MonitorService - Sencho version check', () => {
|
||||
await (svc as any).evaluate();
|
||||
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith('info', expect.stringContaining('0.46.0'));
|
||||
// Message must include the real running version, not "0.0.0".
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith('info', expect.stringContaining('currently running 0.45.0'));
|
||||
expect(mockSetSystemState).toHaveBeenCalledWith('last_sencho_update_notified_version', '0.46.0');
|
||||
});
|
||||
|
||||
it('does not re-notify for the same version', async () => {
|
||||
process.env.npm_package_version = '0.45.0';
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockFetchLatestSenchoVersion.mockResolvedValue('0.46.0');
|
||||
mockGetSystemState.mockReturnValue('0.46.0'); // Already notified for this version
|
||||
mockGetGlobalSettings.mockReturnValue({});
|
||||
@@ -632,7 +644,7 @@ describe('MonitorService - Sencho version check', () => {
|
||||
});
|
||||
|
||||
it('handles version check failure gracefully', async () => {
|
||||
process.env.npm_package_version = '0.45.0';
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockFetchLatestSenchoVersion.mockRejectedValue(new Error('Network down'));
|
||||
mockGetGlobalSettings.mockReturnValue({});
|
||||
mockGetNodes.mockReturnValue([]);
|
||||
@@ -647,7 +659,7 @@ describe('MonitorService - Sencho version check', () => {
|
||||
});
|
||||
|
||||
it('respects the 6-hour cooldown interval', async () => {
|
||||
process.env.npm_package_version = '0.45.0';
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockFetchLatestSenchoVersion.mockResolvedValue('0.46.0');
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
mockGetGlobalSettings.mockReturnValue({});
|
||||
@@ -662,4 +674,21 @@ describe('MonitorService - Sencho version check', () => {
|
||||
// fetchLatestSenchoVersion should not have been called since we're within cooldown
|
||||
expect(mockFetchLatestSenchoVersion).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips version check when getSenchoVersion returns null', async () => {
|
||||
// Simulates the production-Docker scenario that previously leaked "0.0.0"
|
||||
mockGetSenchoVersion.mockReturnValue(null);
|
||||
mockFetchLatestSenchoVersion.mockResolvedValue('0.46.0');
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
mockGetGlobalSettings.mockReturnValue({});
|
||||
mockGetNodes.mockReturnValue([]);
|
||||
mockGetStackAlerts.mockReturnValue([]);
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await (svc as any).evaluate();
|
||||
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', expect.stringContaining('0.46.0'));
|
||||
expect(mockSetSystemState).not.toHaveBeenCalledWith('last_sencho_update_notified_version', expect.anything());
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user