diff --git a/apps/api/app/middleware/auth.ts b/apps/api/app/middleware/auth.ts index cffcc635..4f8c8ae7 100644 --- a/apps/api/app/middleware/auth.ts +++ b/apps/api/app/middleware/auth.ts @@ -80,7 +80,7 @@ export const auth: MiddlewareHandler = async (c, next) => { properties: { machineID: machine.id, ownerUserID: machine.ownerUserId, - ...(machine.teamId ? { teamID: machine.teamId } : {}) + teamID: machine.teamId } }, next diff --git a/apps/api/app/routes/machine.ts b/apps/api/app/routes/machine.ts index ed5efbf4..2621d133 100644 --- a/apps/api/app/routes/machine.ts +++ b/apps/api/app/routes/machine.ts @@ -208,6 +208,58 @@ export namespace MachineApi { }); } ) + .post( + '/heartbeat', + machineOnly, + describeRoute({ + tags: ['Machine'], + summary: 'Say the host is alive', + description: + 'Records liveness for the calling machine and returns how often it should call back. The interval comes from the server on purpose: a fleet whose cadence can only change by shipping a new agent is a fleet whose cadence never changes. Takes no body — what a host is *running* is reported separately, and reporting a shape we cannot yet act on would be worse than reporting nothing.', + responses: { + 200: { + content: { + 'application/json': { + schema: Result( + z.object({ + lastSeen: z.iso.datetime().meta({ + description: 'When this beat was recorded, by the database’s clock', + example: Examples.Machine.lastSeen + }), + intervalSeconds: z.number().meta({ + description: 'Call back this often', + example: Machine.HEARTBEAT_SECONDS + }) + }) + ) + } + }, + description: 'The beat was recorded' + }, + 403: ErrorResponses[403], + 404: ErrorResponses[404] + } + }), + async (c) => { + const lastSeen = await Machine.touchLastSeen(Actor.machineID); + if (!lastSeen) { + // The credentials authenticated but the row is gone — a host + // deleted mid-beat. It must re-register rather than keep + // beating into nothing, so this is a 404 and not a 200. + throw new VisibleError( + 'not_found', + ErrorCodes.NotFound.RESOURCE_NOT_FOUND, + 'This machine no longer exists' + ); + } + return c.json({ + data: { + lastSeen: lastSeen.toISOString(), + intervalSeconds: Machine.HEARTBEAT_SECONDS + } + }); + } + ) .get( '/me', machineOnly, diff --git a/packages/core/src/actor.ts b/packages/core/src/actor.ts index b8877b16..f1651ea6 100644 --- a/packages/core/src/actor.ts +++ b/packages/core/src/actor.ts @@ -51,7 +51,10 @@ const Machine = z.object({ properties: z.object({ machineID: z.string(), ownerUserID: z.string(), - teamID: z.string().optional() + // Not optional: `machine.teamId` is notNull since 0048, so a host that + // authenticated always has a team and the branch that used to handle + // its absence was handling a state that can no longer exist. + teamID: z.string() }) }); diff --git a/packages/core/src/machine/index.ts b/packages/core/src/machine/index.ts index dcf2af17..710f2fa0 100644 --- a/packages/core/src/machine/index.ts +++ b/packages/core/src/machine/index.ts @@ -171,15 +171,60 @@ export namespace Machine { } ); + /** + * How often a host should say it is alive, in seconds. + * + * Returned to the host on every heartbeat rather than compiled into it: the + * cadence is the control plane's business, and a fleet whose interval can + * only be changed by shipping a new agent is a fleet whose interval never + * changes. Thirty seconds is a placeholder — it is short enough that a dead + * host is noticed within a session's setup, and long enough to be free. + */ + export const HEARTBEAT_SECONDS = 30; + + /** + * A host is considered offline once it has missed this many heartbeats. + * + * Three rather than one, because a single missed beat is a lost packet and + * calling that "offline" would make placement flap. + */ + export const OFFLINE_AFTER_MISSED = 3; + + /** + * Record that a host is alive, and say when that was. + * + * Returns the stored timestamp rather than void so a caller can hand it + * straight back to the host — which is what lets a heartbeat be one round + * trip instead of a write followed by a read. + */ export const touchLastSeen = fn(Info.shape.id, async (id) => { - await Database.use(async (tx) => { - await tx + return Database.use(async (tx) => { + return tx .update(MachineTable) .set({ lastSeen: sql`now()` }) - .where(eq(MachineTable.id, id)); + .where(eq(MachineTable.id, id)) + .returning({ lastSeen: MachineTable.lastSeen }) + .then((rows) => rows.at(0)?.lastSeen ?? null); }); }); + /** + * Whether a host has beaten recently enough to place work on. + * + * Derived from `lastSeen` rather than stored as a column, so there is no + * state to go stale when nothing is running to clear it — a host that stops + * beating becomes offline by the passage of time, which is the one mechanism + * that cannot itself fail. + */ + export function isOnline(lastSeen: Date | string | null): boolean { + if (!lastSeen) { + return false; + } + const at = lastSeen instanceof Date ? lastSeen : new Date(lastSeen); + const age = (Date.now() - at.getTime()) / 1000; + return age <= HEARTBEAT_SECONDS * OFFLINE_AFTER_MISSED; + } + export const fromID = fn(Info.shape.id, async (id) => { return Database.use(async (tx) => { return tx diff --git a/packages/core/src/machine/machine.test.ts b/packages/core/src/machine/machine.test.ts new file mode 100644 index 00000000..8ad5a3d9 --- /dev/null +++ b/packages/core/src/machine/machine.test.ts @@ -0,0 +1,117 @@ +import { afterAll, describe, expect, test } from 'bun:test'; + +import { Fixtures } from '../db/fixtures.js'; +import { testDb } from '../db/test.js'; +import { Identifier } from '../id.js'; +import { Machine } from './index.js'; + +const sql = testDb(); + +const createdUserIds: string[] = []; + +async function newOwner(label: string) { + const o = await Fixtures.owner(label); + createdUserIds.push(o.userId); + return o; +} + +afterAll(async () => { + if (createdUserIds.length > 0) { + await sql`delete from "user" where id in ${sql(createdUserIds)}`; + createdUserIds.length = 0; + } +}); + +describe('Machine registration', () => { + test('registration returns a secret once and stores only its digest', async () => { + const owner = await newOwner('mch-secret'); + const registered = await Machine.register({ + id: Identifier.ascending('machine'), + ownerUserId: owner.userId, + teamId: owner.teamId, + label: 'first-box' + }); + + expect(registered.secret.startsWith('msk_')).toBe(true); + + // The digest is what is stored, so the secret itself must not be findable. + const rows = await sql`select secret_hash from machine where id = ${registered.id}`; + expect(rows[0]!.secret_hash).not.toBe(registered.secret); + expect(rows[0]!.secret_hash).toHaveLength(64); + + // And it never leaves `authenticate`, even in memory. + const authed = await Machine.authenticate({ id: registered.id, secret: registered.secret }); + expect(authed?.id).toBe(registered.id); + expect(JSON.stringify(authed)).not.toContain(rows[0]!.secret_hash); + }); + + test('a wrong secret and a wrong id are refused the same way', async () => { + const owner = await newOwner('mch-wrong'); + const registered = await Machine.register({ + id: Identifier.ascending('machine'), + ownerUserId: owner.userId, + teamId: owner.teamId, + label: 'box' + }); + + expect(await Machine.authenticate({ id: registered.id, secret: 'msk_wrong' })).toBeNull(); + expect( + await Machine.authenticate({ id: 'mch_nosuchmachinenosuchmach_', secret: registered.secret }) + ).toBeNull(); + }); + + test('a host always has a team, so registering without one is impossible', async () => { + const owner = await newOwner('mch-team'); + // `teamId` is notNull since 0048 and required by the schema, so this is a + // validation failure rather than a row with a null team. + // + // `toThrow` and not `rejects.toThrow`: `fn()` parses its input + // synchronously, before any promise exists, so a bad argument never + // becomes a rejected promise. + expect(() => + Machine.register({ + id: Identifier.ascending('machine'), + ownerUserId: owner.userId, + // @ts-expect-error — the point of the test is that this is refused + teamId: null, + label: 'teamless' + }) + ).toThrow(); + }); +}); + +describe('Machine heartbeat', () => { + test('a beat records a time and it moves forward', async () => { + const owner = await newOwner('mch-beat'); + const machineId = await Fixtures.machine(owner); + + expect((await Machine.fromID(machineId))?.lastSeen).toBeNull(); + + const first = await Machine.touchLastSeen(machineId); + expect(first).not.toBeNull(); + + const second = await Machine.touchLastSeen(machineId); + expect(second!.getTime()).toBeGreaterThanOrEqual(first!.getTime()); + }); + + test('beating for a machine that is gone reports nothing rather than pretending', async () => { + // A host deleted mid-beat must be told to re-register, so this returns + // null and the route turns that into a 404. + expect(await Machine.touchLastSeen('mch_deletedmiddeletedmid___')).toBeNull(); + }); + + test('online is derived from the last beat, not stored', async () => { + expect(Machine.isOnline(null)).toBe(false); + expect(Machine.isOnline(new Date())).toBe(true); + + // One missed beat is a lost packet; three is a dead host. Placement must + // not flap on the first. + const oneMissed = new Date(Date.now() - Machine.HEARTBEAT_SECONDS * 1000 - 1000); + expect(Machine.isOnline(oneMissed)).toBe(true); + + const wellPast = new Date( + Date.now() - Machine.HEARTBEAT_SECONDS * (Machine.OFFLINE_AFTER_MISSED + 1) * 1000 + ); + expect(Machine.isOnline(wellPast)).toBe(false); + }); +});