From 88cf89194aae4de28f179ff3b656edce9efb0b12 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 29 Aug 2026 11:54:33 +0200 Subject: [PATCH] The lab said nothing was running while two machines were 'incus list' failed because this shell had no permission to reach the daemon, incusOk returned null, and the caller wrote ?? "[]". So 'mesh-lab list' printed 'no scenario instances standing' -- confidently, about a question it had never managed to ask. The comment on incusOk warns about exactly this, in those words: absence and success made indistinguishable. Three of its own callers then did it. Two listings and the live diagram, which would have drawn an empty scenario rather than fail -- a picture that is confidently wrong, which is worse than none. Anything enumerating what exists now goes through enumerate() and throws. incusOk stays right where failure genuinely means no, like instanceExists, and there is a test holding that line so this does not get over-corrected until nothing can be asked at all. Worth noting 'mesh-lab check' already diagnoses this precise cause, down to 'a session that predates it cannot see it'. The diagnosis existed; the listing just never asked for it. --- scenarios/first-node.yml | 13 ++++++++++--- src/diagram/from-live.ts | 7 +++++-- src/incus/client.ts | 35 ++++++++++++++++++++++++++++------ test/enumerate.test.ts | 41 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 test/enumerate.test.ts diff --git a/scenarios/first-node.yml b/scenarios/first-node.yml index 39a2b85..0566555 100644 --- a/scenarios/first-node.yml +++ b/scenarios/first-node.yml @@ -1,8 +1,10 @@ # One machine, raising a substrate from the bundle its host carries. # -# This is the bootstrap class (novox/hq ADR 0009): no forge, no control plane, no delivery. It -# exists to develop the first three steps of raising a mesh — a container runtime, a store, and -# a database inside it — which is as far as the bootstrap can go until a control plane exists. +# This is the bootstrap class (novox/hq ADR 0009): no forge, no control plane to talk to, no +# delivery. It exists to develop the steps of raising a mesh on a machine with no route out — +# a container runtime, a store, the control plane's schema in it, and the broker. +# +# It stops before the control plane *runs*, because there is nothing for it to serve yet. scenario: first-node segments: @@ -15,8 +17,13 @@ machines: at: { segment: hosting, address: [192.0.2.10] } inbound: allow +# Placed into a registry the scenario raises, which is what a real node pulls from anyway. The +# digests below are the ones that registry assigns, and that satisfies pinning: what is required +# is a reference that is exact and cannot move (novox/hq ADR 0006). images: - postgres:17-alpine + - cloudamqp/lavinmq:latest + - mesh-control:development place: all: [host, runtime] diff --git a/src/diagram/from-live.ts b/src/diagram/from-live.ts index 2529ca0..8befac7 100644 --- a/src/diagram/from-live.ts +++ b/src/diagram/from-live.ts @@ -8,7 +8,7 @@ * by the running machine now — nothing is inferred from a file on disk. */ -import { incusOk, taggedNetworks } from "../incus/client.ts"; +import { incus, incusOk, taggedNetworks } from "../incus/client.ts"; import { depthOf, type Diagram, type DiagramMachine, type DiagramSegment } from "./model.ts"; interface RawInstance { @@ -28,7 +28,10 @@ interface RawInstance { export async function diagramFromLive(instanceId: string): Promise { const networks = (await taggedNetworks()).filter((n) => n.instanceId === instanceId); - const json = (await incusOk(["list", "--format", "json"], 30_000)) ?? "[]"; + // Not `incusOk(...) ?? "[]"`. A picture is read from what runs (novox/hq ADR 0018), and a read + // that failed and became an empty list would draw an empty scenario rather than fail — a + // diagram that is confidently wrong, which is worse than no diagram. + const json = (await incus(["list", "--format", "json"], 30_000)).stdout.trim() || "[]"; const parsed = JSON.parse(json) as RawInstance[]; const mine = parsed.filter((i) => i.config?.["user.mesh-lab.instance"] === instanceId); if (mine.length === 0 && networks.length === 0) throw new Error(`no scenario instance '${instanceId}'`); diff --git a/src/incus/client.ts b/src/incus/client.ts index eab22d6..070fc96 100644 --- a/src/incus/client.ts +++ b/src/incus/client.ts @@ -111,6 +111,22 @@ export async function incusOk(args: string[], timeoutMs = 60_000): Promise { + return (await incus(args, timeoutMs)).stdout; +} + /** Did the command work? For commands whose output is not the point. */ export async function succeeds(args: string[], timeoutMs = 60_000): Promise { return (await incusOk(args, timeoutMs)) !== null; @@ -168,14 +184,18 @@ export interface TaggedInstance { * nothing. Metadata is what the instance actually knows about itself. */ export async function taggedInstances(): Promise { - const json = (await incusOk(["list", "--format", "json"], 30_000)) ?? "[]"; + const json = (await enumerate(["list", "--format", "json"])).trim() || "[]"; let parsed: unknown; try { parsed = JSON.parse(json); } catch { - return []; + throw new IncusError(["list", "--format", "json"], + `incus answered something that is not JSON: ${json.slice(0, 200)}`, null); + } + if (!Array.isArray(parsed)) { + throw new IncusError(["list", "--format", "json"], + "incus answered JSON that is not a list of instances", null); } - if (!Array.isArray(parsed)) return []; const tagged: TaggedInstance[] = []; for (const entry of parsed) { @@ -199,14 +219,17 @@ export interface TaggedNetwork { } export async function taggedNetworks(): Promise { - const json = (await incusOk(["network", "list", "--format", "json"], 30_000)) ?? "[]"; + const args = ["network", "list", "--format", "json"]; + const json = (await enumerate(args)).trim() || "[]"; let parsed: unknown; try { parsed = JSON.parse(json); } catch { - return []; + throw new IncusError(args, `incus answered something that is not JSON: ${json.slice(0, 200)}`, null); + } + if (!Array.isArray(parsed)) { + throw new IncusError(args, "incus answered JSON that is not a list of networks", null); } - if (!Array.isArray(parsed)) return []; const tagged: TaggedNetwork[] = []; for (const entry of parsed) { diff --git a/test/enumerate.test.ts b/test/enumerate.test.ts new file mode 100644 index 0000000..93a8b4d --- /dev/null +++ b/test/enumerate.test.ts @@ -0,0 +1,41 @@ +// Set before importing: the client reads MESH_LAB_INCUS once, at module load. +// `false` is a real program that exits non-zero and prints nothing — which is also the worst +// case, because an empty stderr is how a failure arrives with no explanation. +process.env["MESH_LAB_INCUS"] = "false"; + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { instanceExists, taggedInstances, taggedNetworks } from "../src/incus/client.ts"; + +/** + * "I cannot see" must never be answered as "there is nothing there." + * + * This is the fault the comment on `incusOk` warns about, committed by three of its own callers + * writing `?? "[]"`. It cost a session: `mesh-lab list` printed *no scenario instances standing* + * while two were standing, because the shell had no permission to reach the daemon. Nothing was + * wrong with the lab's knowledge of the instances — it had never managed to ask. + * + * The same shape as the fault the node host exists to prevent, in the tool that tests the host: + * a service that does not exist reported as `stopped`. + */ + +test("listing instances fails rather than reporting none", async () => { + await assert.rejects( + () => taggedInstances(), + "a failed `incus list` came back as an empty list; every caller would report nothing running", + ); +}); + +test("listing networks fails rather than reporting none", async () => { + await assert.rejects( + () => taggedNetworks(), + "a failed `incus network list` came back as an empty list", + ); +}); + +test("but a question whose failure genuinely means no still answers no", async () => { + // The distinction worth keeping. `instanceExists` asks about one named thing, and a daemon + // that will not answer is not evidence the instance exists — so false is honest here, and + // making this throw too would be over-correcting until nothing can be asked at all. + assert.equal(await instanceExists("anything"), false); +});