diff --git a/apps/docs/src/content/docs/admin/observability.md b/apps/docs/src/content/docs/admin/observability.md index a7024066..6ca2ef56 100644 --- a/apps/docs/src/content/docs/admin/observability.md +++ b/apps/docs/src/content/docs/admin/observability.md @@ -42,7 +42,7 @@ token (`Authorization: Bearer `); without the variable, the path doesn't | `kept_backup_last_success_timestamp_seconds` | when the last good backup finished (0 before the first) | | `kept_backup_last_run_status{status}` | 1 for the last finished run's status, 0 for the others | | `kept_disk_used_ratio{volume}` | how full the data disk and a directory backup target are | -| `kept_update_available` | 1 when the update check has seen a newer release | +| `kept_update_available` | 1 when the update check has seen a release newer than the one running | No label ever holds a path or a target. A database that can't answer leaves the backup, disk and update lines out; the scrape itself still succeeds. diff --git a/apps/docs/src/content/docs/admin/security-model.md b/apps/docs/src/content/docs/admin/security-model.md index 931406c4..98abc6e7 100644 --- a/apps/docs/src/content/docs/admin/security-model.md +++ b/apps/docs/src/content/docs/admin/security-model.md @@ -169,7 +169,7 @@ Kept's own re-encoded JPEGs. Exported CSV cells that start like a formula are ne telemetry is switched off. Tracing and error reporting go only to a collector you set ([monitoring](/admin/observability/#tracing-and-error-reporting)). - **The update check is off by default.** On, it asks GitHub for the latest release once a day, - sending nothing but the request ([upgrades](/admin/upgrades/#knowing-theres-a-new-version)). + and once after an upgrade, sending nothing but the request ([upgrades](/admin/upgrades/#knowing-theres-a-new-version)). - **What an AI provider sees**, when a location uses one: photos as a re-encoded copy with EXIF and XMP removed (Kept refuses to send an image that still has them), the fixed prompt and the location's languages for extraction, the question and the tool results the asker's role diff --git a/apps/docs/src/content/docs/admin/upgrades.md b/apps/docs/src/content/docs/admin/upgrades.md index 7e838054..94aded3e 100644 --- a/apps/docs/src/content/docs/admin/upgrades.md +++ b/apps/docs/src/content/docs/admin/upgrades.md @@ -91,13 +91,14 @@ works as before ([Verify a release](/admin/verify-release/)). ### Knowing there's a new version -Kept can check for new versions, and it is **off by default**. On, it asks GitHub once a day for -the latest release of the repository its image was built from, sending nothing but the request (no +Kept can check for new versions, and it is **off by default**. On, it asks GitHub once a day, and +once after each upgrade, for the latest release of the repository its image was built from, sending nothing but the request (no version, no instance identifier). Nothing is downloaded or installed. `KEPT_UPDATE_CHECK=true` turns it on and locks it; `false` locks it off. The switch is in Admin → Sign-up, under "New versions". Admin → Status says "Kept X is available" -when a newer release exists, and its **Check now** asks at once. +when a newer release exists, and its **Check now** asks at once. After an upgrade the notice goes at once, and Kept +asks again within the hour. ## The snapshot before every upgrade diff --git a/apps/server/src/backup/metrics.ts b/apps/server/src/backup/metrics.ts index f447ec5e..3378e281 100644 --- a/apps/server/src/backup/metrics.ts +++ b/apps/server/src/backup/metrics.ts @@ -1,6 +1,7 @@ +import { KEPT_VERSION } from '@kept/shared'; import type pg from 'pg'; import { withSystem } from '../db/scope.js'; -import { UPDATE_CHECK_STATE_KEY } from '../updates/check.js'; +import { storedNewerRelease, UPDATE_CHECK_STATE_KEY } from '../updates/check.js'; import { BACKUP_METRICS_KEY, type BackupMetrics, DISK_STATUS_KEY, diskStatusOf } from './watch.js'; // Step 8's Prometheus gauges (D66, D166), served by /metrics (http/health.ts, behind @@ -13,7 +14,10 @@ export type OpsGauge = { name: string; help: string; samples: [string, number][] const STATUSES = ['ok', 'warning', 'failed'] as const; -export async function opsGauges(system: pg.Pool): Promise { +export async function opsGauges( + system: pg.Pool, + running: string = KEPT_VERSION, +): Promise { const rows = await withSystem(system, async (_tx, client) => { const res = await client.query<{ key: string; value: unknown }>( 'SELECT key, value FROM public.instance_settings WHERE key = ANY($1::text[])', @@ -25,7 +29,8 @@ export async function opsGauges(system: pg.Pool): Promise { const lastSuccess = typeof backup?.lastSuccessAt === 'string' ? Date.parse(backup.lastSuccessAt) : Number.NaN; const disk = diskStatusOf(rows.get(DISK_STATUS_KEY)); - const update = rows.get(UPDATE_CHECK_STATE_KEY) as { latest?: unknown } | undefined; + // Only a release newer than the one running (#552), as the status page shows it. + const update = storedNewerRelease(rows.get(UPDATE_CHECK_STATE_KEY), running); const volumes = (['data', 'backup'] as const).filter((v) => disk[v] !== null); return [ { @@ -46,7 +51,7 @@ export async function opsGauges(system: pg.Pool): Promise { { name: 'kept_update_available', help: 'Whether the opt-in update check found a newer release (1) or not (0).', - samples: [['', update?.latest ? 1 : 0]], + samples: [['', update ? 1 : 0]], }, ]; } diff --git a/apps/server/src/updates/check.test.ts b/apps/server/src/updates/check.test.ts index 8219496e..3eaad6e8 100644 --- a/apps/server/src/updates/check.test.ts +++ b/apps/server/src/updates/check.test.ts @@ -2,6 +2,7 @@ import { createServer, type IncomingHttpHeaders, type Server } from 'node:http'; import type { AddressInfo } from 'node:net'; import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; import { type TestDb, testDb } from '../../test/db.js'; +import { opsGauges } from '../backup/metrics.js'; import { withSystem } from '../db/scope.js'; import { guardedFetch } from '../net/ssrf.js'; import { @@ -227,13 +228,28 @@ describe('updateCheckDue', () => { const at = (iso: string) => new Date(iso); it('asks once a day at the instance hour, or after two days whatever the hour', () => { const empty = { lastCheckedAt: null, latest: null, error: null }; - expect(updateCheckDue(empty, 5, at('2026-10-06T12:07:00Z'))).toBe(true); - const asked = { ...empty, lastCheckedAt: '2026-10-05T05:07:00Z' }; - expect(updateCheckDue(asked, 5, at('2026-10-05T23:07:00Z'))).toBe(false); - expect(updateCheckDue(asked, 5, at('2026-10-06T04:07:00Z'))).toBe(false); - expect(updateCheckDue(asked, 5, at('2026-10-06T05:07:00Z'))).toBe(true); - expect(updateCheckDue(asked, 5, at('2026-10-06T06:07:00Z'))).toBe(false); - expect(updateCheckDue(asked, 5, at('2026-10-07T04:07:00Z'))).toBe(true); + expect(updateCheckDue(empty, 5, at('2026-10-06T12:07:00Z'), '1.2.0')).toBe(true); + const asked = { ...empty, lastCheckedAt: '2026-10-05T05:07:00Z', version: '1.2.0' }; + expect(updateCheckDue(asked, 5, at('2026-10-05T23:07:00Z'), '1.2.0')).toBe(false); + expect(updateCheckDue(asked, 5, at('2026-10-06T04:07:00Z'), '1.2.0')).toBe(false); + expect(updateCheckDue(asked, 5, at('2026-10-06T05:07:00Z'), '1.2.0')).toBe(true); + expect(updateCheckDue(asked, 5, at('2026-10-06T06:07:00Z'), '1.2.0')).toBe(false); + expect(updateCheckDue(asked, 5, at('2026-10-07T04:07:00Z'), '1.2.0')).toBe(true); + }); + + it('an answer from another running version is due at the next hourly run (#552)', () => { + const asked = { + lastCheckedAt: '2026-10-05T05:07:00Z', + latest: null, + error: null, + version: '1.2.0', + }; + // An hour later, not the instance's hour: due only because the server was upgraded. + expect(updateCheckDue(asked, 5, at('2026-10-05T06:07:00Z'), '1.2.0')).toBe(false); + expect(updateCheckDue(asked, 5, at('2026-10-05T06:07:00Z'), '1.3.0')).toBe(true); + // An answer stored before the version was kept with it counts as another version's. + const { version: _, ...legacy } = asked; + expect(updateCheckDue(legacy, 5, at('2026-10-05T06:07:00Z'), '1.2.0')).toBe(true); }); }); @@ -330,6 +346,97 @@ describe('the scheduled check, on the database', () => { expect(seen).toHaveLength(1); }); + // #552: an answer stored while an older version ran outlived the upgrade, so a server running + // 1.14.7 said "Kept 1.14.7 is available". + const storeRaw = (value: unknown) => + inTx((c) => + c.query( + `INSERT INTO public.instance_settings (key, value) VALUES ($1, $2::jsonb) + ON CONFLICT (key) DO UPDATE SET value = excluded.value`, + [UPDATE_CHECK_STATE_KEY, JSON.stringify(value)], + ), + ); + const found = (v: string) => ({ + version: v, + url: `https://github.com/bruce/kept/releases/tag/v${v}`, + publishedAt: '2026-10-01T09:00:00Z', + }); + const readAs = (running: string) => inTx((c) => readUpdateCheck(c, {}, running)); + const updateAvailable = async (running: string) => { + const gauge = (await opsGauges(db.pools.system, running)).find( + (g) => g.name === 'kept_update_available', + ); + return gauge?.samples[0]?.[1] === 1; + }; + + it('never announces the running version, or an older one (#552)', async () => { + await setEnabled(true); + await storeRaw({ + lastCheckedAt: '2026-10-05T05:07:00Z', + latest: found('1.14.7'), + error: null, + hour: 5, + }); + expect((await readAs('1.14.7')).latest).toBeNull(); + expect((await readAs('1.14.8')).latest).toBeNull(); + expect((await readAs('1.14.6')).latest).toEqual(found('1.14.7')); + // The metric reads the same rule. + expect(await updateAvailable('1.14.7')).toBe(false); + expect(await updateAvailable('1.14.6')).toBe(true); + }); + + it('still announces a newer release after an upgrade short of it (#552)', async () => { + await setEnabled(true); + await storeRaw({ + lastCheckedAt: '2026-10-05T05:07:00Z', + latest: found('1.14.8'), + error: null, + version: '1.14.6', + }); + expect((await readAs('1.14.7')).latest).toEqual(found('1.14.8')); + expect(await updateAvailable('1.14.7')).toBe(true); + expect((await readAs('1.14.9')).latest).toBeNull(); + expect(await updateAvailable('1.14.9')).toBe(false); + }); + + it('after an upgrade asks again at the next hourly run, not the next day (#552)', async () => { + await setEnabled(true); + answer = () => ok(release('v1.3.0')); + const now = new Date('2026-10-06T12:07:00Z'); + const run = (version: string, at: Date) => + runScheduledUpdateCheck(inTx, { env: {}, sourceUrl: SOURCE, version, now: at, ...local }); + expect(await run('1.2.0', now)).toBe(true); + const hourLater = new Date('2026-10-06T13:07:00Z'); + expect(await run('1.2.0', hourLater)).toBe(false); + expect(seen).toHaveLength(1); + // Upgraded to 1.3.0: the stored answer was for 1.2.0, so the next run asks. + expect(await run('1.3.0', hourLater)).toBe(true); + expect(seen).toHaveLength(2); + expect((await readAs('1.3.0')).latest).toBeNull(); + // And then waits for the next day again. + expect(await run('1.3.0', new Date('2026-10-06T14:07:00Z'))).toBe(false); + expect(seen).toHaveLength(2); + }); + + it('after an upgrade with the check off: no request at all (#552)', async () => { + await storeRaw({ + lastCheckedAt: '2026-10-05T05:07:00Z', + latest: found('1.3.0'), + error: null, + version: '1.2.0', + hour: 5, + }); + answer = () => ok(release('v1.3.0')); + const asked = await runScheduledUpdateCheck(inTx, { + env: {}, + sourceUrl: SOURCE, + version: '1.3.0', + ...local, + }); + expect(asked).toBe(false); + expect(seen).toHaveLength(0); + }); + it('a rate limit is stored and not retried within the day', async () => { await setEnabled(true); answer = () => ({ status: 403, headers: { 'x-ratelimit-remaining': '0' } }); diff --git a/apps/server/src/updates/check.ts b/apps/server/src/updates/check.ts index 44116f92..b880753e 100644 --- a/apps/server/src/updates/check.ts +++ b/apps/server/src/updates/check.ts @@ -1,6 +1,7 @@ import { randomInt } from 'node:crypto'; import { compareSemver, + KEPT_VERSION, parseSemver, UPDATE_CHECK_ERRORS, type UpdateCheckError, @@ -208,8 +209,9 @@ export async function checkForUpdate(opts: CheckOptions): Promise; +/** + * The stored release, only while it is newer than `running` (#552). The answer was right when it + * was asked, but it outlives an upgrade: a server upgraded to the release it found would + * otherwise say "Kept 1.14.7 is available" while running 1.14.7, until the next daily check. So + * every reader (the status page, the metric) compares again. A running version that isn't semver + * (one set by hand, a commit hash say) can't be compared and keeps the answer; checkForUpdate never + * stores one for it. A local build runs as 0.0.0-dev, which is semver, so every release is newer. + */ +function newerRelease(latest: StoredState['latest'], running: string): UpdateCheckState['latest'] { + if (!latest) return null; + const release = parseSemver(latest.version); + if (!release) return null; + const current = parseSemver(running); + if (!current) return latest; + return compareSemver(release, current) > 0 ? latest : null; +} + +/** The stored row's release newer than `running`, for readers that hold the raw + * `instance_settings.update_check` value (backup/metrics.ts). */ +export function storedNewerRelease( + value: unknown, + running: string = KEPT_VERSION, +): UpdateCheckState['latest'] { + const parsed = StoredState.safeParse(value ?? EMPTY); + return parsed.success ? newerRelease(parsed.data.latest, running) : null; +} + const EMPTY: StoredState = { lastCheckedAt: null, latest: null, error: null }; /** KEPT_UPDATE_CHECK, parsed: true/false locks the switch; undefined leaves it to the admin. */ @@ -246,10 +276,12 @@ export function updateCheckSwitch(env: UpdateCheckEnv, storedEnabled: boolean) { return { value: locked ? env.KEPT_UPDATE_CHECK === true : storedEnabled, locked }; } -/** The status page's `updates` (AdminOpsStatus), from rows already kept. */ +/** The status page's `updates` (AdminOpsStatus), from rows already kept. `latest` only while it + * is newer than the running version (newerRelease). */ export async function readUpdateCheck( client: pg.ClientBase, env: UpdateCheckEnv, + running: string = KEPT_VERSION, ): Promise { const { storedEnabled, stored } = await readKeys(client); const { value, locked } = updateCheckSwitch(env, storedEnabled); @@ -257,7 +289,7 @@ export async function readUpdateCheck( enabled: value, locked, lastCheckedAt: stored.lastCheckedAt, - latest: stored.latest, + latest: newerRelease(stored.latest, running), error: stored.error, }; } @@ -279,17 +311,19 @@ async function writeState(client: pg.ClientBase, state: StoredState) { ); } -/** Stores one answer, keeping the job's hour. */ +/** Stores one answer and the running version it was asked for, keeping the job's hour. */ export async function storeUpdateCheck( client: pg.ClientBase, result: UpdateCheckResult, at: Date, + running: string, ): Promise { const { stored } = await readKeys(client); await writeState(client, { lastCheckedAt: at.toISOString(), latest: result.latest, error: result.error, + version: running, ...(stored.hour !== undefined ? { hour: stored.hour } : {}), }); } @@ -297,12 +331,20 @@ export async function storeUpdateCheck( const HOUR_MS = 3_600_000; /** - * Whether the hourly job should ask now: never asked; or a day has passed and this is the + * Whether the hourly job should ask now: never asked; or the answer was asked for another + * running version (an upgrade or a downgrade since, #552), so the first hourly run after a + * restart on a new version asks, with no request at boot; or a day has passed and this is the * instance's hour; or two days have passed (the server was down at its hour). So it asks about * once a day, and a `rate_limited` answer waits for the next day. */ -export function updateCheckDue(state: StoredState, hour: number, now: Date): boolean { +export function updateCheckDue( + state: StoredState, + hour: number, + now: Date, + running: string, +): boolean { if (!state.lastCheckedAt) return true; + if (state.version !== running) return true; const last = Date.parse(state.lastCheckedAt); if (!Number.isFinite(last)) return true; const elapsed = now.getTime() - last; @@ -332,10 +374,10 @@ export async function runScheduledUpdateCheck( hour = randomInt(24); await writeState(client, { ...stored, hour }); } - return updateCheckDue(stored, hour, now); + return updateCheckDue(stored, hour, now, opts.version); }); if (!due) return false; const result = await checkForUpdate(opts); - await inTx((client) => storeUpdateCheck(client, result, now)); + await inTx((client) => storeUpdateCheck(client, result, now, opts.version)); return true; } diff --git a/apps/server/src/updates/routes.ts b/apps/server/src/updates/routes.ts index 4a26639c..403d603a 100644 --- a/apps/server/src/updates/routes.ts +++ b/apps/server/src/updates/routes.ts @@ -56,7 +56,7 @@ export async function updateRoutes(app: KeptApp, deps: InventoryDeps): Promise { - await storeUpdateCheck(client, result, at); + await storeUpdateCheck(client, result, at, KEPT_VERSION); await audited(tx, { locationId: null, ownerAccountId: null,