mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-25 01:37:05 +00:00
fix(notifications): align Sencho update alerts with Fleet cache (#1620)
Remove the MonitorService 6-hour version-check cooldown so node_update_available fires when the shared version cache observes a published release, matching the Fleet update button. Dedup and publish-pending gating still prevent spam.
This commit is contained in:
Anso
@@ -505,7 +505,6 @@ describe('MonitorService - host_alerts_enabled toggle', () => {
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await (svc as any).evaluateGlobalSettings({ host_alerts_enabled: '0' });
|
||||
|
||||
expect(mockGetLatestVersionInfo).toHaveBeenCalled();
|
||||
@@ -1096,8 +1095,7 @@ describe('MonitorService - restart_count metric', () => {
|
||||
// ── Sencho version update check ───────────────────────────────────────
|
||||
|
||||
describe('MonitorService - Sencho version check', () => {
|
||||
/** Stateful system_state backing for tests that need getSystemState to
|
||||
* reflect setSystemState writes within the same evaluation. */
|
||||
/** In-memory system_state so get/set round-trip within one evaluation. */
|
||||
function wireStatefulSystemState(seed: Record<string, string> = {}) {
|
||||
const store: Record<string, string> = { ...seed };
|
||||
mockGetSystemState.mockImplementation((key: string) => store[key] ?? null);
|
||||
@@ -1105,6 +1103,16 @@ describe('MonitorService - Sencho version check', () => {
|
||||
return store;
|
||||
}
|
||||
|
||||
async function runEvaluate(): Promise<void> {
|
||||
await (MonitorService.getInstance() as any).evaluate();
|
||||
}
|
||||
|
||||
function updateAvailabilityCalls(): unknown[][] {
|
||||
return mockDispatchAlert.mock.calls.filter(
|
||||
(args: unknown[]) => args[1] === 'node_update_available',
|
||||
);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mockGetGlobalSettings.mockReturnValue({});
|
||||
mockGetNodes.mockReturnValue([]);
|
||||
@@ -1114,15 +1122,11 @@ describe('MonitorService - Sencho version check', () => {
|
||||
it('dispatches notification when newer version available', async () => {
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
mockGetSystemState.mockReturnValue(null); // No previous notification
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
// Reset the version check timer so it runs immediately
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await (svc as any).evaluate();
|
||||
await runEvaluate();
|
||||
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.46.0'));
|
||||
// Message must include the real running version, not "0.0.0".
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('currently running 0.45.0'));
|
||||
expect(mockSetSystemState).toHaveBeenCalledWith('last_sencho_update_notified_version', '0.46.0');
|
||||
});
|
||||
@@ -1130,126 +1134,92 @@ describe('MonitorService - Sencho version check', () => {
|
||||
it('does not re-notify for the same version', async () => {
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
// Running version < last notified, so self-heal does NOT clear the key.
|
||||
// Running < last notified: self-heal leaves the dedup key alone.
|
||||
mockGetSystemState.mockReturnValue('0.46.0');
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await (svc as any).evaluate();
|
||||
await runEvaluate();
|
||||
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.46.0'));
|
||||
});
|
||||
|
||||
it('handles version check failure gracefully', async () => {
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue(null); // CacheService failed + no stale
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
|
||||
// Should not throw
|
||||
await expect((svc as any).evaluate()).resolves.toBeUndefined();
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('available'));
|
||||
});
|
||||
|
||||
it('respects the 6-hour cooldown interval', async () => {
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
// Simulate the check ran 1 hour ago (within 6-hour window)
|
||||
(svc as any).lastVersionCheckAt = Date.now() - 1 * 60 * 60 * 1000;
|
||||
await (svc as any).evaluate();
|
||||
|
||||
// getLatestVersionInfo should not have been called since we're within cooldown
|
||||
expect(mockGetLatestVersionInfo).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);
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await (svc as any).evaluate();
|
||||
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.46.0'));
|
||||
expect(mockSetSystemState).not.toHaveBeenCalledWith('last_sencho_update_notified_version', expect.anything());
|
||||
// Should not have even attempted the lookup.
|
||||
expect(mockGetLatestVersionInfo).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// ── Regression coverage for PR: cooldown leak + dedup self-heal ───────
|
||||
|
||||
it('does NOT advance cooldown when getLatestVersionInfo returns null (retries next cycle)', async () => {
|
||||
it('handles version check failure gracefully and retries next cycle', async () => {
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue(null);
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await expect(runEvaluate()).resolves.toBeUndefined();
|
||||
await expect(runEvaluate()).resolves.toBeUndefined();
|
||||
|
||||
await (svc as any).evaluate();
|
||||
await (svc as any).evaluate();
|
||||
|
||||
// Both evals should attempt the lookup since failures do not lock cooldown.
|
||||
expect(mockGetLatestVersionInfo).toHaveBeenCalledTimes(2);
|
||||
expect((svc as any).lastVersionCheckAt).toBe(0);
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('available'));
|
||||
});
|
||||
|
||||
it('does NOT advance cooldown while a GitHub release is pending registry publish', async () => {
|
||||
it('skips version check when getSenchoVersion returns null', async () => {
|
||||
mockGetSenchoVersion.mockReturnValue(null);
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
|
||||
await runEvaluate();
|
||||
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.46.0'));
|
||||
expect(mockSetSystemState).not.toHaveBeenCalledWith('last_sencho_update_notified_version', expect.anything());
|
||||
expect(mockGetLatestVersionInfo).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('retries while a GitHub release is pending registry publish without notifying', async () => {
|
||||
mockGetSenchoVersion.mockReturnValue('0.93.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.93.0', publishPending: true });
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
|
||||
await (svc as any).evaluate();
|
||||
await (svc as any).evaluate();
|
||||
await runEvaluate();
|
||||
await runEvaluate();
|
||||
|
||||
expect(mockGetLatestVersionInfo).toHaveBeenCalledTimes(2);
|
||||
expect((svc as any).lastVersionCheckAt).toBe(0);
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('available'));
|
||||
});
|
||||
|
||||
it('DOES advance cooldown on a successful lookup (prevents re-fetch inside window)', async () => {
|
||||
it('notifies on the next eval after an up-to-date success when a newer version appears', async () => {
|
||||
const store = wireStatefulSystemState();
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
mockGetLatestVersionInfo
|
||||
.mockResolvedValueOnce({ version: '0.45.0', publishPending: false })
|
||||
.mockResolvedValue({ version: '0.46.0', publishPending: false });
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await runEvaluate();
|
||||
expect(mockGetLatestVersionInfo).toHaveBeenCalledTimes(1);
|
||||
expect(updateAvailabilityCalls()).toHaveLength(0);
|
||||
expect(store.last_sencho_update_notified_version).toBeUndefined();
|
||||
|
||||
await (svc as any).evaluate();
|
||||
const firstCooldown = (svc as any).lastVersionCheckAt;
|
||||
expect(firstCooldown).toBeGreaterThan(0);
|
||||
await runEvaluate();
|
||||
expect(mockGetLatestVersionInfo).toHaveBeenCalledTimes(2);
|
||||
const afterSecond = updateAvailabilityCalls();
|
||||
expect(afterSecond).toHaveLength(1);
|
||||
expect(afterSecond[0][2]).toContain('0.46.0');
|
||||
expect(store.last_sencho_update_notified_version).toBe('0.46.0');
|
||||
|
||||
// Second eval immediately after: cooldown gate should block it.
|
||||
mockGetLatestVersionInfo.mockClear();
|
||||
await (svc as any).evaluate();
|
||||
await runEvaluate();
|
||||
expect(mockGetLatestVersionInfo).toHaveBeenCalledTimes(3);
|
||||
expect(updateAvailabilityCalls()).toHaveLength(1);
|
||||
});
|
||||
|
||||
expect(mockGetLatestVersionInfo).not.toHaveBeenCalled();
|
||||
// Exactly one dispatch across both evals.
|
||||
const availabilityDispatches = mockDispatchAlert.mock.calls.filter(
|
||||
(args: unknown[]) => typeof args[2] === 'string' && args[2].includes('available'),
|
||||
);
|
||||
expect(availabilityDispatches).toHaveLength(1);
|
||||
it('notifies a newer release after a prior notify while still on the old version', async () => {
|
||||
const store = wireStatefulSystemState({ last_sencho_update_notified_version: '0.46.0' });
|
||||
mockGetSenchoVersion.mockReturnValue('0.45.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.47.0', publishPending: false });
|
||||
|
||||
await runEvaluate();
|
||||
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.47.0'));
|
||||
expect(store.last_sencho_update_notified_version).toBe('0.47.0');
|
||||
});
|
||||
|
||||
it('self-heals dedup after user upgrades to the previously-notified version', async () => {
|
||||
// Prior notification stored "0.46.0" back when the user was on 0.45.0.
|
||||
// User has now upgraded to 0.46.0; a new release (0.47.0) just dropped.
|
||||
// Notified for 0.46.0 while on 0.45.0; now running 0.46.0 with 0.47.0 out.
|
||||
const store = wireStatefulSystemState({ last_sencho_update_notified_version: '0.46.0' });
|
||||
mockGetSenchoVersion.mockReturnValue('0.46.0');
|
||||
mockGetLatestVersionInfo.mockResolvedValue({ version: '0.47.0', publishPending: false });
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
await (svc as any).evaluate();
|
||||
await runEvaluate();
|
||||
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.47.0'));
|
||||
expect(store.last_sencho_update_notified_version).toBe('0.47.0');
|
||||
@@ -1259,15 +1229,12 @@ describe('MonitorService - Sencho version check', () => {
|
||||
mockGetSenchoVersion.mockReturnValue('0.93.0');
|
||||
mockGetSystemState.mockReturnValue(null);
|
||||
|
||||
const svc = MonitorService.getInstance();
|
||||
(svc as any).lastVersionCheckAt = 0;
|
||||
|
||||
mockGetLatestVersionInfo.mockResolvedValueOnce({ version: '0.93.0', publishPending: true });
|
||||
await (svc as any).evaluate();
|
||||
await runEvaluate();
|
||||
expect(mockDispatchAlert).not.toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.94.0'));
|
||||
|
||||
mockGetLatestVersionInfo.mockResolvedValueOnce({ version: '0.94.0', publishPending: false });
|
||||
await (svc as any).evaluate();
|
||||
await runEvaluate();
|
||||
expect(mockDispatchAlert).toHaveBeenCalledWith('info', 'node_update_available', expect.stringContaining('0.94.0'));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -173,9 +173,6 @@ export class MonitorService {
|
||||
// causal classification). MonitorService no longer polls for container
|
||||
// exits; see backend/src/services/DockerEventService.ts.
|
||||
|
||||
// Sencho version check cooldown (6 hours between external API calls)
|
||||
private lastVersionCheckAt = 0;
|
||||
private static readonly VERSION_CHECK_INTERVAL_MS = 6 * 60 * 60 * 1000;
|
||||
private static readonly SENCHO_UPDATE_NOTIFIED_KEY = 'last_sencho_update_notified_version';
|
||||
|
||||
private constructor() { }
|
||||
@@ -342,7 +339,7 @@ export class MonitorService {
|
||||
// 3. (Decoupled) Docker janitor disk-usage check runs on its own
|
||||
// slow cadence in evaluateJanitor(); see start() and F-6.
|
||||
|
||||
// 4. Sencho version update check (runs once per VERSION_CHECK_INTERVAL_MS)
|
||||
// 4. Sencho version update check (cache-backed; dedup prevents re-notify)
|
||||
await this.checkSenchoVersion();
|
||||
}
|
||||
|
||||
@@ -536,28 +533,15 @@ export class MonitorService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Check GitHub/Docker Hub for a newer Sencho release and dispatch a
|
||||
* one-shot notification. Uses getLatestVersionInfo() which wraps CacheService
|
||||
* (30 min TTL when published, shorter while a GitHub release awaits registry
|
||||
* publish + inflight dedup) so transient network blips do not cause gaps,
|
||||
* and the check stays consistent with the Fleet update banner.
|
||||
*
|
||||
* The 6-hour cooldown gate prevents bell spam: it is only advanced on a
|
||||
* SUCCESSFUL lookup. A failed lookup retries on the next eval cycle
|
||||
* (30 seconds) instead of locking for 6 hours.
|
||||
* Notify when a newer Sencho release is published. Shares Fleet's
|
||||
* getLatestVersionInfo() cache (30 min published / 3 min publish-pending,
|
||||
* plus inflight dedup). Dedup key last_sencho_update_notified_version
|
||||
* prevents re-notify; lookup failures and publishPending skip so the
|
||||
* next eval can retry.
|
||||
*/
|
||||
private async checkSenchoVersion(): Promise<void> {
|
||||
const sinceLast = Date.now() - this.lastVersionCheckAt;
|
||||
if (sinceLast <= MonitorService.VERSION_CHECK_INTERVAL_MS) {
|
||||
if (isDebugEnabled()) {
|
||||
const nextInMs = MonitorService.VERSION_CHECK_INTERVAL_MS - sinceLast;
|
||||
console.debug(`[Monitor:diag] Sencho version check in cooldown (next in ~${Math.round(nextInMs / 60000)}m)`);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve from the packaged manifest: process.env.npm_package_version is
|
||||
// only set by npm scripts, so it is undefined in Docker (node dist/index.js).
|
||||
// getSenchoVersion() reads the packaged manifest; npm_package_version
|
||||
// is unset under `node dist/index.js` (Docker).
|
||||
const currentVersion = getSenchoVersion();
|
||||
if (!isValidVersion(currentVersion)) {
|
||||
if (isDebugEnabled()) console.debug('[Monitor:diag] Sencho version unresolvable; skipping update notification');
|
||||
@@ -566,55 +550,44 @@ export class MonitorService {
|
||||
|
||||
const versionInfo = await getLatestVersionInfo();
|
||||
if (!versionInfo || !isValidVersion(versionInfo.version)) {
|
||||
// Network failure (GitHub + Docker Hub both down, no stale cache).
|
||||
// Do NOT advance the cooldown so the next eval retries.
|
||||
if (isDebugEnabled()) console.debug('[Monitor:diag] Latest Sencho version unresolvable; will retry next cycle');
|
||||
return;
|
||||
}
|
||||
|
||||
if (versionInfo.publishPending) {
|
||||
// GitHub announced a release before the image is pullable. Retry on
|
||||
// the next eval cycle without advancing the 6-hour cooldown.
|
||||
if (isDebugEnabled()) console.debug('[Monitor:diag] Sencho release pending registry publish; will retry next cycle');
|
||||
return;
|
||||
}
|
||||
|
||||
this.lastVersionCheckAt = Date.now();
|
||||
const latest = versionInfo.version;
|
||||
|
||||
const db = DatabaseService.getInstance();
|
||||
const stateKey = MonitorService.SENCHO_UPDATE_NOTIFIED_KEY;
|
||||
const storedLastNotified = db.getSystemState(stateKey) || '';
|
||||
let lastNotified = db.getSystemState(stateKey) || '';
|
||||
|
||||
// Self-heal: if the user has reached the previously-notified version,
|
||||
// clear the dedup so future releases always trigger a fresh notification.
|
||||
// This also recovers from stale state left over by the pre-586 "0.0.0" bug.
|
||||
let effectiveLastNotified = storedLastNotified;
|
||||
if (storedLastNotified && isValidVersion(storedLastNotified) && semver.gte(currentVersion, storedLastNotified)) {
|
||||
if (isDebugEnabled()) console.debug(`[Monitor:diag] Clearing stale dedup key (running ${currentVersion} >= last notified ${storedLastNotified})`);
|
||||
// Once running >= last notified, clear dedup so a later release can fire.
|
||||
if (lastNotified && isValidVersion(lastNotified) && semver.gte(currentVersion, lastNotified)) {
|
||||
if (isDebugEnabled()) console.debug(`[Monitor:diag] Clearing stale dedup key (running ${currentVersion} >= last notified ${lastNotified})`);
|
||||
db.setSystemState(stateKey, '');
|
||||
effectiveLastNotified = '';
|
||||
lastNotified = '';
|
||||
}
|
||||
|
||||
if (!semver.gt(latest, currentVersion)) {
|
||||
if (isDebugEnabled()) console.debug(`[Monitor:diag] Running ${currentVersion} is up-to-date with latest ${latest}`);
|
||||
return;
|
||||
}
|
||||
|
||||
if (effectiveLastNotified === latest) {
|
||||
if (lastNotified === latest) {
|
||||
if (isDebugEnabled()) console.debug(`[Monitor:diag] Already notified for Sencho ${latest}`);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const notifier = NotificationService.getInstance();
|
||||
await notifier.dispatchAlert('info', 'node_update_available',
|
||||
`Sencho ${latest} is available (currently running ${currentVersion}). Visit the Fleet dashboard to update.`);
|
||||
await NotificationService.getInstance().dispatchAlert(
|
||||
'info',
|
||||
'node_update_available',
|
||||
`Sencho ${latest} is available (currently running ${currentVersion}). Visit the Fleet dashboard to update.`,
|
||||
);
|
||||
db.setSystemState(stateKey, latest);
|
||||
if (isDebugEnabled()) console.debug(`[Monitor:diag] Dispatched version notification: ${currentVersion} -> ${latest}`);
|
||||
} catch (e) {
|
||||
// dispatchAlert normally catches channel errors internally, but the
|
||||
// history insert or WebSocket broadcast can throw on an unhealthy DB.
|
||||
console.error('[MonitorService] Failed to dispatch Sencho version notification:', e);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user