fix(api): a run drives the box under it, and needs a game and a claim

Four things the session endpoints did not do, or did wrongly.

The box had three states and nothing wrote them. A box read `created`
while a run on it was `live`, so every screen showing a person what their
hardware is doing was reading a column no code had ever moved. A run
reaching `live` now makes its box `running`, and a terminal run stops it:
`ended` cleanly, `failed` not, carrying the reason the agent gave. Not
every run state maps — a box has no `starting` on purpose, because that
transition is synchronous from the agent's side and a state nobody sets
is a state that lies. Both writes are one transaction, since "this run is
live" and "the box under it is running" are one fact in two tables, and a
box stuck `running` with nothing on it has nothing to correct it.

`POST /session` accepted any game in the catalog. A run launches as a
Steam account that has to own the game, so one outside the caller's
library is a box that starts, tries to launch and fails minutes later
with nothing to point at; it is now refused up front. Told apart from a
game that does not exist rather than hidden, because the catalog is
public and "you do not own this" is a sentence a person can act on. The
library is a synced copy, so this refuses a game bought since the last
sync — that is a staleness bug in the sync, not a reason to start runs
that cannot work.

Publishing a ticket only refused terminal runs, so a host could publish
an address for a run it had never claimed. A ticket is the address of
something being brought up, so only `starting` and `live` accept one, and
the state is in the write rather than only in the check above it. The two
refusals stay separate answers because they are different mistakes: one
agent skipped a step, the other has nothing left to reach.

The migration that adds the one-active-run index stopped older duplicate
runs without clearing the ticket they had published, which is the
invariant that same migration exists to establish. It clears it now,
verified against a box carrying two unstopped runs.

Nine tests, each checked against the unfixed code first.

packages/core/migrations/0008_session_one_active_run_per_box.sql

@@ -15,8 +15,14 @@
 --
 -- `ended` and not `failed`: nothing about these runs failed. They were work
 -- nobody picked up, and `failed` carries a reason there is none of.
+--
+-- The ticket goes with the state, exactly as the terminal transitions in
+-- `Session` do it. A duplicate that reached `starting` or `live` may have
+-- published an address, and a stopped run that still answers with one is an
+-- address a polling client would dial — with publishing a replacement already
+-- refused, it would also be the last word.
 UPDATE "session" s
-SET "state" = 'ended', "time_stopped" = now()
+SET "state" = 'ended', "time_stopped" = now(), "ticket" = NULL
 WHERE s."time_stopped" IS NULL
 	AND s."time_deleted" IS NULL
 	AND EXISTS (

packages/core/src/session/index.ts

@@ -1,7 +1,8 @@
-import { and, desc, eq, inArray, isNull, notInArray, sql } from 'drizzle-orm';
+import { and, desc, eq, inArray, isNull, sql } from 'drizzle-orm';
 import z from 'zod';
 
 import { BoxTable, BoxTier } from '../box/box.sql.js';
+import { Box } from '../box/index.js';
 import { Database } from '../db/index.js';
 import { ErrorCodes, VisibleError } from '../error.js';
 import { Examples } from '../examples.js';
@@ -497,6 +498,39 @@ export namespace Session {
 		session: Info | null;
 	}
 
+	/**
+	 * What a run reaching a state means for the box underneath it.
+	 *
+	 * The box has its own three states and nothing was writing them, so a box
+	 * read `created` while a run on it was `live` — the screens that show a
+	 * person what their hardware is doing would all have been wrong. The two
+	 * state machines are not the same shape and should not be: a box has no
+	 * `starting`, deliberately, because that transition is synchronous from
+	 * the agent's side and a state nobody sets is a state that lies. So only
+	 * the states that mean something to the box are mapped, and `requested`
+	 * and `starting` map to nothing at all.
+	 *
+	 * `failed` is a `stopped` box that did not stop cleanly, which is the
+	 * distinction `stopClean` exists for: "it is not running" and "it faulted"
+	 * are different facts and the difference lives in the reason.
+	 */
+	function boxStateFor(
+		run: Info
+	): { state: 'running' | 'stopped'; stopReason: string | null; stopClean: boolean | null } | null {
+		switch (run.state) {
+			case 'live':
+				return { state: 'running', stopReason: null, stopClean: null };
+			case 'ended':
+				return { state: 'stopped', stopReason: null, stopClean: true };
+			case 'failed':
+				// The run's own reason, so a box explains its stop in the words
+				// the agent used rather than in a second wording of one event.
+				return { state: 'stopped', stopReason: run.errorMessage ?? null, stopClean: false };
+			default:
+				return null;
+		}
+	}
+
 	export const transition = fn(
 		z.object({
 			id: Info.shape.id,
@@ -505,40 +539,66 @@ export namespace Session {
 			errorMessage: Info.shape.errorMessage
 		}),
 		async (input): Promise<TransitionResult> => {
-			const current = await forMachine({ id: input.id, machineId: input.machineId });
-			if (!current) return { outcome: 'forbidden', session: null };
-			if (current.state === input.state) return { outcome: 'unchanged', session: current };
-			if (!NEXT_STATES[current.state].includes(input.state)) {
-				return { outcome: 'illegal', session: current };
-			}
+			// One transaction, because "this run is live" and "the box under it
+			// is running" are one fact written to two tables. Committing the
+			// first without the second is how a box gets stuck `running` with
+			// nothing running on it, and nothing here would ever correct it.
+			return Database.transaction(async (): Promise<TransitionResult> => {
+				const current = await forMachine({ id: input.id, machineId: input.machineId });
+				if (!current) return { outcome: 'forbidden', session: null };
+				if (current.state === input.state) return { outcome: 'unchanged', session: current };
+				if (!NEXT_STATES[current.state].includes(input.state)) {
+					return { outcome: 'illegal', session: current };
+				}
 
-			const moved = await compareAndSetState({
-				id: input.id,
-				machineId: input.machineId,
-				from: current.state,
-				to: input.state,
-				errorMessage: input.errorMessage
+				const moved = await compareAndSetState({
+					id: input.id,
+					machineId: input.machineId,
+					from: current.state,
+					to: input.state,
+					errorMessage: input.errorMessage
+				});
+				// The state read above is not the state written below, and the
+				// gap is where two agents race. Nothing moved means somebody
+				// else did — and then the box is that caller's to update, not
+				// this one's.
+				if (!moved) return { outcome: 'lost', session: current };
+
+				const box = boxStateFor(moved);
+				if (box) {
+					await Box.setState({ id: moved.boxId, ...box });
+				}
+
+				return { outcome: 'moved', session: moved };
 			});
-			// The state read above is not the state written below, and the gap
-			// is where two agents race. Nothing moved means somebody else did.
-			if (!moved) return { outcome: 'lost', session: current };
-			return { outcome: 'moved', session: moved };
 		}
 	);
 
 	export interface TicketResult {
-		outcome: 'forbidden' | 'closed' | 'published';
+		outcome: 'forbidden' | 'unclaimed' | 'closed' | 'published';
 		session: Info | null;
 	}
 
+	/**
+	 * The states a run can have an address in.
+	 *
+	 * `starting` is in and `requested` is out, which is the whole distinction:
+	 * a ticket is the address of something being brought up, so publishing one
+	 * means the host has taken the work. It cannot have an address for a run it
+	 * has not claimed, and the terminal states are out because a run that is
+	 * not there has no address at all.
+	 */
+	const ADDRESSABLE = ['starting', 'live'] as const;
+
 	/**
 	 * Publish a ticket for a run, on behalf of the host it is placed on.
 	 *
 	 * A ticket may appear while the state is still `starting` — it is
 	 * republished as addresses are discovered, so the client polls and re-reads
-	 * rather than keeping the first one. A run that has stopped is refused: an
-	 * address for something that is not there can only mislead whoever is
-	 * still polling.
+	 * rather than keeping the first one. Outside {@link ADDRESSABLE} it is
+	 * refused, and the two refusals are separate answers because they are
+	 * different mistakes: a run not yet claimed is an agent that skipped a
+	 * step, and a run that stopped is one that has nothing left to reach.
 	 */
 	export const publishTicket = fn(
 		z.object({
@@ -549,6 +609,7 @@ export namespace Session {
 		async (input): Promise<TicketResult> => {
 			const current = await forMachine({ id: input.id, machineId: input.machineId });
 			if (!current) return { outcome: 'forbidden', session: null };
+			if (current.state === 'requested') return { outcome: 'unclaimed', session: current };
 
 			return Database.use(async (tx) => {
 				return tx
@@ -557,7 +618,10 @@ export namespace Session {
 					.where(
 						and(
 							eq(SessionTable.id, input.id),
-							notInArray(SessionTable.state, ['ended', 'failed']),
+							// The state is in the write and not only in the check
+							// above it, so a run that stops underneath this call
+							// does not acquire an address on the way out.
+							inArray(SessionTable.state, [...ADDRESSABLE]),
 							isNull(SessionTable.timeDeleted),
 							inArray(SessionTable.boxId, boxesOn(tx, input.machineId))
 						)

packages/core/src/session/session.test.ts

@@ -530,3 +530,111 @@ describe('Session tickets and the end of a run', () => {
 		expect(ended?.ticket).toBeNull();
 	});
 });
+
+describe('Session and the box underneath it', () => {
+	test('a live run is what makes its box running', async () => {
+		const { machineId, box, session } = await requestedRun('ses-box-live', 5460);
+		// A box starts out `created` and nothing had ever moved it, so it read
+		// `created` while a run on it was `live`.
+		expect((await Box.fromID(box.id))?.state).toBe('created');
+
+		await Session.transition({ id: session.id, machineId, state: 'starting', errorMessage: null });
+		// `starting` is deliberately not a box state: that transition is
+		// synchronous from the agent's side, so nothing would ever write it.
+		expect((await Box.fromID(box.id))?.state).toBe('created');
+
+		await Session.transition({ id: session.id, machineId, state: 'live', errorMessage: null });
+		const running = await Box.fromID(box.id);
+		expect(running?.state).toBe('running');
+		expect(running?.stopReason).toBeNull();
+		expect(running?.stopClean).toBeNull();
+	});
+
+	test('a run that ends stops its box, cleanly', async () => {
+		const { machineId, box, session } = await requestedRun('ses-box-ended', 5461);
+		await Session.transition({ id: session.id, machineId, state: 'starting', errorMessage: null });
+		await Session.transition({ id: session.id, machineId, state: 'live', errorMessage: null });
+		await Session.transition({ id: session.id, machineId, state: 'ended', errorMessage: null });
+
+		const stopped = await Box.fromID(box.id);
+		expect(stopped?.state).toBe('stopped');
+		expect(stopped?.stopClean).toBe(true);
+		expect(stopped?.stopReason).toBeNull();
+	});
+
+	test('a run that fails stops its box in the words the agent used', async () => {
+		const { machineId, box, session } = await requestedRun('ses-box-failed', 5462);
+		await Session.transition({ id: session.id, machineId, state: 'starting', errorMessage: null });
+		await Session.transition({
+			id: session.id,
+			machineId,
+			state: 'failed',
+			errorMessage: 'the guest never came up'
+		});
+
+		const stopped = await Box.fromID(box.id);
+		expect(stopped?.state).toBe('stopped');
+		// "It is not running" and "it faulted" are different facts, and the
+		// difference lives in the reason rather than in a fourth state.
+		expect(stopped?.stopClean).toBe(false);
+		expect(stopped?.stopReason).toBe('the guest never came up');
+	});
+
+	test('a refused report leaves the box alone', async () => {
+		const { machineId, box, session } = await requestedRun('ses-box-untouched', 5463);
+		const other = await scene('ses-box-otherhost', 5464);
+
+		const refused = await Session.transition({
+			id: session.id,
+			machineId: other.machineId,
+			state: 'starting',
+			errorMessage: null
+		});
+		expect(refused.outcome).toBe('forbidden');
+
+		// An illegal transition does not move the run, so it must not move the
+		// box either — otherwise the box records a run that never happened.
+		const illegal = await Session.transition({
+			id: session.id,
+			machineId,
+			state: 'live',
+			errorMessage: null
+		});
+		expect(illegal.outcome).toBe('illegal');
+		expect((await Box.fromID(box.id))?.state).toBe('created');
+	});
+});
+
+describe('Session tickets need a claim first', () => {
+	test('a run nobody has claimed has no address to publish', async () => {
+		const { machineId, session } = await requestedRun('ses-ticket-unclaimed', 5465);
+
+		// A ticket is the address of something being brought up, so publishing
+		// one for a `requested` run means the agent skipped the claim — the
+		// step that is the only mutual exclusion in the design.
+		const early = await Session.publishTicket({ id: session.id, machineId, ticket: 'too-soon' });
+		expect(early.outcome).toBe('unclaimed');
+		expect((await Session.fromID(session.id))?.ticket).toBeNull();
+
+		await Session.transition({ id: session.id, machineId, state: 'starting', errorMessage: null });
+		const now = await Session.publishTicket({ id: session.id, machineId, ticket: 'in-time' });
+		expect(now.outcome).toBe('published');
+		expect(now.session?.ticket).toBe('in-time');
+	});
+
+	test('the two refusals are different answers, because they are different mistakes', async () => {
+		const { machineId, session } = await requestedRun('ses-ticket-refusals', 5466);
+		const unclaimed = await Session.publishTicket({ id: session.id, machineId, ticket: 'a' });
+
+		await Session.transition({ id: session.id, machineId, state: 'starting', errorMessage: null });
+		await Session.transition({ id: session.id, machineId, state: 'live', errorMessage: null });
+		await Session.transition({ id: session.id, machineId, state: 'ended', errorMessage: null });
+		const closed = await Session.publishTicket({ id: session.id, machineId, ticket: 'b' });
+
+		// One is an agent that has not claimed the work; the other is a run
+		// with nothing left to reach. Collapsing them would tell an agent
+		// retrying the wrong thing.
+		expect(unclaimed.outcome).toBe('unclaimed');
+		expect(closed.outcome).toBe('closed');
+	});
+});