From daece5ce87f482bbe8a9068a05e253cf5b35b48f Mon Sep 17 00:00:00 2001 From: flint Date: Sat, 10 Oct 2026 21:25:32 +0000 Subject: [PATCH 1/3] fix(agent): the create remedy names both branches when the key read-back fails (cli#593) A failed key read-back leaves it unknown whether an Agent row exists, but the remedy printed only the no-row branch. If the row did exist, `flair agent add` would then refuse with no next step. The remedy now names both branches; the definite no-row remedy is unchanged. A test runs every create refusal path and the success path with a marker operator credential and asserts the marker never reaches stdout, stderr or a thrown error. Refs #593 --- .../fixed-593-create-remedy-unknown-row.md | 5 + packages/cli/src/commands/agent.ts | 24 +++- .../test/agent-create-registration.test.ts | 106 +++++++++++++++++- 3 files changed, 125 insertions(+), 10 deletions(-) create mode 100644 .changelog/unreleased/fixed-593-create-remedy-unknown-row.md diff --git a/.changelog/unreleased/fixed-593-create-remedy-unknown-row.md b/.changelog/unreleased/fixed-593-create-remedy-unknown-row.md new file mode 100644 index 00000000..d7e5c1a9 --- /dev/null +++ b/.changelog/unreleased/fixed-593-create-remedy-unknown-row.md @@ -0,0 +1,5 @@ +- **`tps agent create` names both remedy branches when a failed key read-back leaves the Agent row's existence unknown (Refs #593).** + + With the row's existence unknown, the remedy now names `flair agent remove ` + (which also deletes that agent's Memory and Soul rows) as well as + `flair agent add --keys-dir `; the definite no-row remedy is unchanged. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index ecf94a0a..526c473d 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -251,9 +251,16 @@ function storedKeyAsHex(stored: string): string | null { return bytes.length === 32 ? bytes.toString("hex") : null; } +/** Whether an Agent row was confirmed to exist when the registration is refused. */ +type RowState = "exists" | "absent" | "unknown"; + /** * cli#512 — when Flair is reachable, `create` exits non-zero here unless the * stored key reads back equal to the generated key. + * + * cli#593 — `rowState` is three-valued. A failed read-back leaves the row's + * existence unknown, and the remedy then names both branches rather than the + * no-row one. */ function refuseRegistration( id: string, @@ -261,7 +268,7 @@ function refuseRegistration( identityDir: string, detail: string, writeError: string | null, - rowExists: boolean, + rowState: RowState, ): never { const cause = writeError ? `${detail}; the registration write failed: ${writeError}` : detail; console.error(`❌ Agent '${id}' is not registered in Flair at ${flairUrl} — ${cause}.`); @@ -269,13 +276,18 @@ function refuseRegistration( ` Flair cannot store a key generated here yet (it drops publicKey on Agent PUT/PATCH; a supported operation is flair#2266).`, ); const copy = `copy ${join(identityDir, `${id}.key`)} and ${join(identityDir, `${id}.pub`)} into the Flair host's keys dir, run \`flair agent add ${id} --keys-dir \` there, then re-run \`tps agent create\``; - if (rowExists) { + if (rowState === "exists") { console.error( ` Until then: run \`flair agent remove ${id}\` on the Flair host, then ${copy}.`, ); console.error( ` ⚠️ \`flair agent remove\` deletes the agent's Agent row, its Memory and Soul rows, and its key files in the keys dir (unless --keep-keys); registering a key on an existing pending row without deleting is flair#2266, not yet available.`, ); + } else if (rowState === "unknown") { + console.error( + ` Until then: if \`flair agent show ${id}\` finds the agent, it already exists: run \`flair agent remove ${id}\` on the Flair host first (this also deletes that agent's Memory and Soul rows), then ${copy}.`, + ); + console.error(` Otherwise: ${copy}.`); } else { console.error(` Until then: ${copy}.`); } @@ -374,14 +386,16 @@ async function createAgent(args: AgentArgs): Promise { try { stored = await flair.readStoredPublicKey(id); } catch (e) { - refuseRegistration(id, flairUrl, identityDir, `the read-back failed (${e instanceof Error ? e.message : String(e)})`, writeError, false); + // The read failed with a non-404 error: whether a row exists is unknown, so + // the remedy must name the remove branch as well as the add branch (cli#593). + refuseRegistration(id, flairUrl, identityDir, `the read-back failed (${e instanceof Error ? e.message : String(e)})`, writeError, "unknown"); } if (!stored.found) { - refuseRegistration(id, flairUrl, identityDir, "no Agent row exists", writeError, false); + refuseRegistration(id, flairUrl, identityDir, "no Agent row exists", writeError, "absent"); } else if (stored.publicKey === null || storedKeyAsHex(stored.publicKey) !== pubKeyHex) { const found = stored.publicKey === null ? "the row has no public key" : `the stored public key is '${stored.publicKey}'`; - refuseRegistration(id, flairUrl, identityDir, `${found}, not the generated key`, writeError, true); + refuseRegistration(id, flairUrl, identityDir, `${found}, not the generated key`, writeError, "exists"); } console.log(successLine ?? ` Agent '${id}' registered in Flair.`); } diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index 76cf6788..df5a131f 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -63,11 +63,13 @@ function refusePut(url: URL, id: string): Response | undefined { async function runCreate( url: string, id: string, - flags: { noSeed?: boolean } = {}, -): Promise<{ exitCode: number | undefined; stdout: string; stderr: string }> { + flags: { noSeed?: boolean; credential?: string } = {}, +): Promise<{ exitCode: number | undefined; stdout: string; stderr: string; error: string | null }> { const home = mkdtempSync(join(tmpdir(), "create-registration-")); const savedHome = process.env.HOME; + const savedCredential = process.env.FLAIR_ADMIN_AUTH; process.env.HOME = home; + if (flags.credential !== undefined) process.env.FLAIR_ADMIN_AUTH = flags.credential; const stdout: string[] = []; const stderr: string[] = []; const logSpy = spyOn(console, "log").mockImplementation((...args: unknown[]) => { stdout.push(args.join(" ")); }); @@ -77,18 +79,21 @@ async function runCreate( exitCode = code; throw new Error(`exit:${code}`); }) as never); + let error: string | null = null; try { await runAgent({ action: "create", id, name: id, flairUrl: url, noSeed: flags.noSeed }); } catch (err) { - if (!String(err).includes("exit:")) throw err; + const message = err instanceof Error ? err.message : String(err); + if (!message.includes("exit:")) error = message; } finally { exitSpy.mockRestore(); logSpy.mockRestore(); errSpy.mockRestore(); if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome; + if (savedCredential === undefined) delete process.env.FLAIR_ADMIN_AUTH; else process.env.FLAIR_ADMIN_AUTH = savedCredential; rmSync(home, { recursive: true, force: true }); } - return { exitCode, stdout: stdout.join("\n"), stderr: stderr.join("\n") }; + return { exitCode, stdout: stdout.join("\n"), stderr: stderr.join("\n"), error }; } test("refused write and no Agent row: exits non-zero, prints no success, names agent, URL and remedy", async () => { @@ -152,7 +157,7 @@ test("the stored key differs from the generated one: exits non-zero", async () = } }); -test("the read-back fails: exits non-zero, never treating a failed read as absent", async () => { +test("the read-back fails: exits non-zero, never treating a failed read as absent, and names both remedy branches", async () => { const id = `${AGENT}-read-fail`; const fake = startFakeFlair(async (req, url) => { if (req.method === "PUT") return Response.json({}, { status: 200 }); @@ -164,6 +169,12 @@ test("the read-back fails: exits non-zero, never treating a failed read as absen expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); expect(res.stderr).toContain("the read-back failed"); + // cli#593: a non-404 read failure leaves the row's existence unknown, so the + // remedy names the remove branch as well as the add branch. + expect(res.stderr).toContain(`flair agent show ${id}`); + expect(res.stderr).toContain(`flair agent remove ${id}`); + expect(res.stderr).toContain(`flair agent add ${id} --keys-dir`); + expect(res.stderr).toContain("Memory and Soul rows"); } finally { fake.stop(); } @@ -275,3 +286,88 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( }, 30_000, ); + +/** + * cli#593 — create uses one operator credential (FLAIR_ADMIN_AUTH, sent as a + * Basic Authorization header on the Agent PUT and the read-back GET). Each create + * refusal path and the success path below runs with a unique marker embedded in + * that credential, and the marker is asserted absent from stdout, stderr and any + * thrown error. Paths: the missing-id usage refusal; a refused write over no Agent + * row; a refused write with --no-seed; a failed read-back (row existence unknown); + * a row holding no public key; a row holding a different key; the success path + * (the key reads back equal). + */ +test("every create refusal path and the success path keep the operator credential out of stdout, stderr and thrown errors", async () => { + const marker = `credential-593-${randomUUID()}-${"z".repeat(40)}`; + const credential = `operator:${marker}`; + const wire = Buffer.from(credential).toString("base64"); + + const fakes: FakeFlair[] = []; + const make = (basic: (req: Request, url: URL) => Promise): FakeFlair => { + const fake = startFakeFlair(basic); + fakes.push(fake); + return fake; + }; + + const noRowId = `${AGENT}-cred-no-row`; + const noRow = make(async (req, url) => (req.method === "PUT" ? refusePut(url, noRowId) : undefined)); + + const noSeedId = `${AGENT}-cred-no-seed`; + const noSeed = make(async (req, url) => (req.method === "PUT" ? refusePut(url, noSeedId) : undefined)); + + const readFailId = `${AGENT}-cred-read-fail`; + const readFail = make(async (req, url) => { + if (req.method === "PUT") return Response.json({}, { status: 200 }); + if (url.pathname === `/Agent/${readFailId}`) return new Response("boom", { status: 500 }); + return undefined; + }); + + const noKeyId = `${AGENT}-cred-no-key`; + const noKey = make(async (req, url) => { + if (req.method === "PUT") return refusePut(url, noKeyId); + if (url.pathname === `/Agent/${noKeyId}`) return Response.json({ id: noKeyId, name: noKeyId }); + return undefined; + }); + + const differId = `${AGENT}-cred-differ`; + const differ = make(async (req, url) => { + if (req.method === "PUT") return refusePut(url, differId); + if (url.pathname === `/Agent/${differId}`) return Response.json({ id: differId, name: differId, publicKey: "de".repeat(32) }); + return undefined; + }); + + const okId = `${AGENT}-cred-ok`; + let okStored: string | null = null; + const ok = make(async (req, url) => { + if (url.pathname === `/Agent/${okId}` && req.method === "PUT") { + const body = (await req.json()) as { publicKey?: string }; + okStored = body.publicKey ?? null; + return Response.json({}, { status: 200 }); + } + if (url.pathname === `/Agent/${okId}` && okStored !== null) return Response.json({ id: okId, name: okId, publicKey: okStored }); + return undefined; + }); + + const paths: Array<[string, () => Promise<{ exitCode: number | undefined; stdout: string; stderr: string; error: string | null }>]> = [ + ["missing id", () => runCreate("http://127.0.0.1:1", "", { credential })], + ["refused write over no row", () => runCreate(noRow.url, noRowId, { credential })], + ["refused write, --no-seed", () => runCreate(noSeed.url, noSeedId, { credential, noSeed: true })], + ["failed read-back", () => runCreate(readFail.url, readFailId, { credential })], + ["row holds no key", () => runCreate(noKey.url, noKeyId, { credential })], + ["row holds a different key", () => runCreate(differ.url, differId, { credential })], + ["success", () => runCreate(ok.url, okId, { credential })], + ]; + + try { + expect(paths.length).toBe(7); + for (const [name, run] of paths) { + const res = await run(); + for (const stream of [res.stdout, res.stderr, res.error ?? ""]) { + expect(`${name}: ${stream}`).not.toContain(marker); + expect(`${name}: ${stream}`).not.toContain(wire); + } + } + } finally { + for (const fake of fakes) fake.stop(); + } +}); From 3c097a36634c01beafad82741f3391942071c49e Mon Sep 17 00:00:00 2001 From: flint Date: Sat, 10 Oct 2026 21:58:52 -0700 Subject: [PATCH 2/3] test(cli): the credential test captures every output and only swallows its own exit Co-Authored-By: Claude Opus 5.5 --- .../test/agent-create-registration.test.ts | 147 +++++++++++++----- 1 file changed, 105 insertions(+), 42 deletions(-) diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index df5a131f..b889973a 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -5,16 +5,14 @@ const patchShared = createPatchShared(); * generated key back equal. These tests drive the create path against a fake * Flair: signed calls reach the shared verifying stub (cli#554), and the * operator-credentialed calls the create flow makes — the Agent PUT and the - * read-back GET, both Basic — reach a route this file controls. Each failure - * branch (refused write, no row, `pending` stored, mismatch, failed read) and - * the success branch is covered; the real-Harper case runs under - * TPS_TEST_REAL_FLAIR=1. + * read-back GET, both Basic — reach a route this file controls. */ import { afterEach, expect, mock, spyOn, test } from "bun:test"; import { randomUUID } from "node:crypto"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { format } from "node:util"; import { runAgent } from "../src/commands/agent.js"; import { stubFlairHandler } from "./helpers/stub-flair.js"; @@ -29,6 +27,7 @@ afterEach(() => { interface FakeFlair { url: string; stop(): void; + requests: Array<{ method: string; path: string; auth: string }>; } /** @@ -40,17 +39,19 @@ interface FakeFlair { */ function startFakeFlair(basic: (req: Request, url: URL) => Promise): FakeFlair { const signed = stubFlairHandler({}); + const requests: FakeFlair["requests"] = []; const server = Bun.serve({ port: 0, async fetch(req) { const url = new URL(req.url); if (url.pathname === "/Health") return new Response("ok"); const auth = req.headers.get("Authorization") ?? ""; + requests.push({ method: req.method, path: url.pathname, auth }); if (auth.startsWith("Basic ")) return (await basic(req, url)) ?? new Response("not found", { status: 404 }); return signed(req); }, }); - return { url: `http://127.0.0.1:${server.port}`, stop: () => server.stop(true) }; + return { url: `http://127.0.0.1:${server.port}`, stop: () => server.stop(true), requests }; } /** A Basic `PUT /Agent/` that Flair refuses (it drops `publicKey`, so the write cannot complete). */ @@ -63,8 +64,9 @@ function refusePut(url: URL, id: string): Response | undefined { async function runCreate( url: string, id: string, - flags: { noSeed?: boolean; credential?: string } = {}, -): Promise<{ exitCode: number | undefined; stdout: string; stderr: string; error: string | null }> { + flags: { noSeed?: boolean; credential?: string; inspectError?: boolean } = {}, + create: typeof runAgent = runAgent, +): Promise<{ exitCode: number | undefined; stdout: string; stderr: string; error: unknown; hasError: boolean }> { const home = mkdtempSync(join(tmpdir(), "create-registration-")); const savedHome = process.env.HOME; const savedCredential = process.env.FLAIR_ADMIN_AUTH; @@ -72,28 +74,49 @@ async function runCreate( if (flags.credential !== undefined) process.env.FLAIR_ADMIN_AUTH = flags.credential; const stdout: string[] = []; const stderr: string[] = []; - const logSpy = spyOn(console, "log").mockImplementation((...args: unknown[]) => { stdout.push(args.join(" ")); }); - const errSpy = spyOn(console, "error").mockImplementation((...args: unknown[]) => { stderr.push(args.join(" ")); }); + const consoleSpies = (["log", "info", "warn", "error"] as const).map((method) => + spyOn(console, method).mockImplementation((...args: unknown[]) => { + (method === "log" || method === "info" ? stdout : stderr).push(`${format(...args)}\n`); + }), + ); + const captureWrite = (output: string[]): typeof process.stdout.write => ( + chunk: string | Uint8Array, + encodingOrCallback?: BufferEncoding | ((error?: Error | null) => void), + callback?: (error?: Error | null) => void, + ) => { + output.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString()); + const done = typeof encodingOrCallback === "function" ? encodingOrCallback : callback; + if (done) queueMicrotask(() => done()); + return true; + }; + const stdoutSpy = spyOn(process.stdout, "write").mockImplementation(captureWrite(stdout)); + const stderrSpy = spyOn(process.stderr, "write").mockImplementation(captureWrite(stderr)); + const syntheticExit = Symbol("runCreate exit"); let exitCode: number | undefined; const exitSpy = spyOn(process, "exit").mockImplementation(((code?: number): never => { exitCode = code; - throw new Error(`exit:${code}`); + throw syntheticExit; }) as never); - let error: string | null = null; + let error: unknown = null; + let hasError = false; try { - await runAgent({ action: "create", id, name: id, flairUrl: url, noSeed: flags.noSeed }); + await create({ action: "create", id, name: id, flairUrl: url, noSeed: flags.noSeed }); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - if (!message.includes("exit:")) error = message; + if (err !== syntheticExit) { + if (!flags.inspectError) throw err; + error = err; + hasError = true; + } } finally { exitSpy.mockRestore(); - logSpy.mockRestore(); - errSpy.mockRestore(); + for (const spy of consoleSpies) spy.mockRestore(); + stdoutSpy.mockRestore(); + stderrSpy.mockRestore(); if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome; if (savedCredential === undefined) delete process.env.FLAIR_ADMIN_AUTH; else process.env.FLAIR_ADMIN_AUTH = savedCredential; rmSync(home, { recursive: true, force: true }); } - return { exitCode, stdout: stdout.join("\n"), stderr: stderr.join("\n"), error }; + return { exitCode, stdout: stdout.join(""), stderr: stderr.join(""), error, hasError }; } test("refused write and no Agent row: exits non-zero, prints no success, names agent, URL and remedy", async () => { @@ -287,17 +310,7 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( 30_000, ); -/** - * cli#593 — create uses one operator credential (FLAIR_ADMIN_AUTH, sent as a - * Basic Authorization header on the Agent PUT and the read-back GET). Each create - * refusal path and the success path below runs with a unique marker embedded in - * that credential, and the marker is asserted absent from stdout, stderr and any - * thrown error. Paths: the missing-id usage refusal; a refused write over no Agent - * row; a refused write with --no-seed; a failed read-back (row existence unknown); - * a row holding no public key; a row holding a different key; the success path - * (the key reads back equal). - */ -test("every create refusal path and the success path keep the operator credential out of stdout, stderr and thrown errors", async () => { +test("selected create cases omit the credential marker and Basic encoding from captured output and errors", async () => { const marker = `credential-593-${randomUUID()}-${"z".repeat(40)}`; const credential = `operator:${marker}`; const wire = Buffer.from(credential).toString("base64"); @@ -348,26 +361,76 @@ test("every create refusal path and the success path keep the operator credentia return undefined; }); - const paths: Array<[string, () => Promise<{ exitCode: number | undefined; stdout: string; stderr: string; error: string | null }>]> = [ - ["missing id", () => runCreate("http://127.0.0.1:1", "", { credential })], - ["refused write over no row", () => runCreate(noRow.url, noRowId, { credential })], - ["refused write, --no-seed", () => runCreate(noSeed.url, noSeedId, { credential, noSeed: true })], - ["failed read-back", () => runCreate(readFail.url, readFailId, { credential })], - ["row holds no key", () => runCreate(noKey.url, noKeyId, { credential })], - ["row holds a different key", () => runCreate(differ.url, differId, { credential })], - ["success", () => runCreate(ok.url, okId, { credential })], + const paths = [ + { name: "missing id", url: "http://127.0.0.1:1", id: "", exitCode: 1, message: "Usage: tps agent create" }, + { name: "refused write over no row", fake: noRow, id: noRowId, exitCode: 1, message: "no Agent row exists", seed: true }, + { name: "refused write, --no-seed", fake: noSeed, id: noSeedId, noSeed: true, exitCode: 1, message: "no Agent row exists", seed: false }, + { name: "failed read-back", fake: readFail, id: readFailId, exitCode: 1, message: "the read-back failed", seed: true }, + { name: "row holds no key", fake: noKey, id: noKeyId, exitCode: 1, message: "the row has no public key", seed: true }, + { name: "row holds a different key", fake: differ, id: differId, exitCode: 1, message: "not the generated key", seed: true }, + { name: "success", fake: ok, id: okId, exitCode: undefined, message: "registered in Flair (no seed", seed: true }, ]; try { - expect(paths.length).toBe(7); - for (const [name, run] of paths) { - const res = await run(); - for (const stream of [res.stdout, res.stderr, res.error ?? ""]) { - expect(`${name}: ${stream}`).not.toContain(marker); - expect(`${name}: ${stream}`).not.toContain(wire); + for (const path of paths) { + const res = await runCreate(path.fake?.url ?? path.url!, path.id, { credential, noSeed: path.noSeed, inspectError: true }); + for (const stream of [res.stdout, res.stderr, format(res.error)]) { + expect(`${path.name}: ${stream}`).not.toContain(marker); + expect(`${path.name}: ${stream}`).not.toContain(wire); + } + expect(res.hasError).toBe(false); + expect(res.error).toBeNull(); + expect(res.exitCode).toBe(path.exitCode); + expect(path.exitCode === 1 ? res.stderr : res.stdout).toContain(path.message); + if (path.exitCode === 1) expect(res.stdout).not.toContain("registered in Flair"); + else { + expect(res.stderr).toBe(""); + expect(res.stdout).toContain("Config written to"); + expect(okStored).toMatch(/^[a-f0-9]{64}$/); + } + if (path.fake) { + const basic = path.fake.requests.filter((req) => req.auth.startsWith("Basic ")); + expect(basic).toEqual([ + { method: "PUT", path: `/Agent/${path.id}`, auth: `Basic ${wire}` }, + { method: "GET", path: `/Agent/${path.id}`, auth: `Basic ${wire}` }, + ]); + expect(path.fake.requests.some((req) => req.method === "POST" && req.path === "/AgentSeed/")).toBe(path.seed); } } } finally { for (const fake of fakes) fake.stop(); } }); + +test("runCreate captures console methods and direct stream writes", async () => { + const res = await runCreate("", "", {}, async () => { + console.log("log"); + console.info("info"); + console.warn("warn"); + console.error("error"); + await new Promise((resolve) => process.stdout.write(Buffer.from("stdout"), resolve)); + await new Promise((resolve) => process.stderr.write("stderr", "utf8", resolve)); + }); + expect(res.stdout).toBe("log\ninfo\nstdout"); + expect(res.stderr).toBe("warn\nerror\nstderr"); + expect(res.exitCode).toBeUndefined(); + expect(res.error).toBeNull(); +}); + +test("runCreate retains non-exit thrown values by identity when inspected", async () => { + for (const error of [new Error("credential exit:1"), new Error("early failure"), null, undefined]) { + const res = await runCreate("", "", { inspectError: true }, async () => { throw error; }); + expect(res.hasError).toBe(true); + expect(res.error).toBe(error); + expect(res.exitCode).toBeUndefined(); + } +}); + +test("runCreate rejects unexpected errors for callers without inspection", async () => { + for (const error of [new Error("credential exit:1"), new Error("early failure"), null, undefined]) { + await expect(runCreate("", "", {}, async () => { throw error; })).rejects.toBe(error); + } + const res = await runCreate("", "", {}, async () => { process.exit(1); }); + expect(res.exitCode).toBe(1); + expect(res.error).toBeNull(); +}); From cd2d4fa728ab5e0a9fd60d999c341ec81bc640c8 Mon Sep 17 00:00:00 2001 From: flint Date: Sat, 10 Oct 2026 23:05:51 -0700 Subject: [PATCH 3/3] test(cli): the success case uses a real keypair and an existing agent, as real Flair allows Co-Authored-By: Claude Opus 5.5 --- .../test/agent-create-registration.test.ts | 152 ++++++++++++------ 1 file changed, 101 insertions(+), 51 deletions(-) diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index b889973a..da2d2278 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -2,18 +2,18 @@ import { createPatchShared } from "./helpers/patch-shared.js"; const patchShared = createPatchShared(); /** * cli#512 — `tps agent create` reports registration only when Flair reads the - * generated key back equal. These tests drive the create path against a fake + * local key back equal. These tests drive the create path against a fake * Flair: signed calls reach the shared verifying stub (cli#554), and the * operator-credentialed calls the create flow makes — the Agent PUT and the * read-back GET, both Basic — reach a route this file controls. */ import { afterEach, expect, mock, spyOn, test } from "bun:test"; import { randomUUID } from "node:crypto"; -import { mkdtempSync, rmSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { homedir } from "node:os"; import { join } from "node:path"; import { format } from "node:util"; import { runAgent } from "../src/commands/agent.js"; +import { generateKeyPair, saveKeyPair } from "../src/utils/identity.js"; import { stubFlairHandler } from "./helpers/stub-flair.js"; const AGENT = "create-registration-agent"; @@ -32,13 +32,16 @@ interface FakeFlair { /** * A fake Flair. A request carrying a TPS-Ed25519 header reaches the shared - * verifying stub with no registered signers — so a fresh agent's own signed + * verifying stub — so a fresh agent's own signed * read (getAgent) and seed (AgentSeed) are refused 401 `unknown_agent`, exactly * as a real Flair refuses an unregistered caller. A request carrying Basic * reaches `basic` (the Agent PUT and the read-back GET). */ -function startFakeFlair(basic: (req: Request, url: URL) => Promise): FakeFlair { - const signed = stubFlairHandler({}); +function startFakeFlair( + basic: (req: Request, url: URL) => Promise, + signers: Record = {}, +): FakeFlair { + const signed = stubFlairHandler(signers, basic); const requests: FakeFlair["requests"] = []; const server = Bun.serve({ port: 0, @@ -64,13 +67,11 @@ function refusePut(url: URL, id: string): Response | undefined { async function runCreate( url: string, id: string, - flags: { noSeed?: boolean; credential?: string; inspectError?: boolean } = {}, + flags: { noSeed?: boolean; credential?: string; inspectError?: boolean; keyPair?: ReturnType } = {}, create: typeof runAgent = runAgent, ): Promise<{ exitCode: number | undefined; stdout: string; stderr: string; error: unknown; hasError: boolean }> { - const home = mkdtempSync(join(tmpdir(), "create-registration-")); - const savedHome = process.env.HOME; + const home = homedir(); const savedCredential = process.env.FLAIR_ADMIN_AUTH; - process.env.HOME = home; if (flags.credential !== undefined) process.env.FLAIR_ADMIN_AUTH = flags.credential; const stdout: string[] = []; const stderr: string[] = []; @@ -100,6 +101,7 @@ async function runCreate( let error: unknown = null; let hasError = false; try { + if (flags.keyPair) saveKeyPair(flags.keyPair, join(home, ".tps", "identity"), id); await create({ action: "create", id, name: id, flairUrl: url, noSeed: flags.noSeed }); } catch (err) { if (err !== syntheticExit) { @@ -112,9 +114,7 @@ async function runCreate( for (const spy of consoleSpies) spy.mockRestore(); stdoutSpy.mockRestore(); stderrSpy.mockRestore(); - if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome; if (savedCredential === undefined) delete process.env.FLAIR_ADMIN_AUTH; else process.env.FLAIR_ADMIN_AUTH = savedCredential; - rmSync(home, { recursive: true, force: true }); } return { exitCode, stdout: stdout.join(""), stderr: stderr.join(""), error, hasError }; } @@ -183,7 +183,7 @@ test("the stored key differs from the generated one: exits non-zero", async () = test("the read-back fails: exits non-zero, never treating a failed read as absent, and names both remedy branches", async () => { const id = `${AGENT}-read-fail`; const fake = startFakeFlair(async (req, url) => { - if (req.method === "PUT") return Response.json({}, { status: 200 }); + if (req.method === "PUT") return refusePut(url, id); if (url.pathname === `/Agent/${id}`) return new Response("boom", { status: 500 }); return undefined; }); @@ -203,46 +203,48 @@ test("the read-back fails: exits non-zero, never treating a failed read as absen } }); -test("the key reads back equal: prints success and exits zero", async () => { +test("an existing Agent with the local key: prints success without a seed or write", async () => { const id = `${AGENT}-ok`; - let stored: string | null = null; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const row = { id, name: id, publicKey, createdAt: keyPair.createdAt }; const fake = startFakeFlair(async (req, url) => { - if (url.pathname === `/Agent/${id}` && req.method === "PUT") { - const body = (await req.json()) as { publicKey?: string }; - stored = body.publicKey ?? null; - return Response.json({}, { status: 200 }); - } - if (url.pathname === `/Agent/${id}` && stored !== null) return Response.json({ id, name: id, publicKey: stored }); + if (req.method === "PUT" || req.method === "PATCH") return refusePut(url, id); + if (req.method === "GET" && url.pathname === `/Agent/${id}`) return Response.json(row); return undefined; - }); + }, { [id]: publicKey }); try { - const res = await runCreate(fake.url, id); + const res = await runCreate(fake.url, id, { keyPair, inspectError: true }); expect(res.exitCode).toBeUndefined(); - expect(res.stdout).toContain("registered in Flair (no seed"); + expect(res.hasError).toBe(false); + expect(res.error).toBeNull(); + expect(res.stdout).toContain("already registered in Flair"); expect(res.stderr).toBe(""); - expect(stored).toMatch(/^[a-f0-9]{64}$/); + expect(fake.requests.map(({ method, path }) => ({ method, path }))).toEqual([ + { method: "GET", path: `/Agent/${id}` }, + { method: "GET", path: `/Agent/${id}` }, + ]); + expect(fake.requests[0]!.auth).toStartWith(`TPS-Ed25519 ${id}:`); + expect(row.publicKey).toBe(publicKey); } finally { fake.stop(); } }); -/** A fake whose Agent row holds `encode()` */ async function runWithStoredEncoding( id: string, encode: (hexKey: string) => string, ): Promise<{ exitCode: number | undefined; stdout: string; stderr: string }> { - let stored: string | null = null; + const keyPair = generateKeyPair(); + const publicKey = encode(Buffer.from(keyPair.signing.publicKey).toString("hex")); + const row = { id, name: id, publicKey, createdAt: keyPair.createdAt }; const fake = startFakeFlair(async (req, url) => { - if (url.pathname === `/Agent/${id}` && req.method === "PUT") { - const body = (await req.json()) as { publicKey?: string }; - stored = body.publicKey ? encode(body.publicKey) : null; - return Response.json({}, { status: 200 }); - } - if (url.pathname === `/Agent/${id}` && stored !== null) return Response.json({ id, name: id, publicKey: stored }); + if (req.method === "PUT" || req.method === "PATCH") return refusePut(url, id); + if (req.method === "GET" && url.pathname === `/Agent/${id}`) return Response.json(row); return undefined; - }); + }, { [id]: /^[a-f0-9]{64}$/.test(publicKey) ? Buffer.from(publicKey, "hex").toString("base64") : publicKey }); try { - return await runCreate(fake.url, id); + return await runCreate(fake.url, id, { keyPair }); } finally { fake.stop(); } @@ -250,7 +252,7 @@ async function runWithStoredEncoding( const b64url = (hex: string) => Buffer.from(hex, "hex").toString("base64url"); -test("the row holds the base64url of the generated key (what flair stores): exits zero", async () => { +test("the row holds the base64url of the local key: exits zero", async () => { const res = await runWithStoredEncoding(`${AGENT}-b64-equal`, b64url); expect(res.exitCode).toBeUndefined(); expect(res.stdout).toContain("registered in Flair"); @@ -263,7 +265,7 @@ test("the row holds the base64url of a different key: exits non-zero", async () expect(res.stderr).toContain("not the generated key"); }); -test("the row holds the hex of the generated key (flair accepts either): exits zero", async () => { +test("the row holds the hex of the local key: exits zero", async () => { const res = await runWithStoredEncoding(`${AGENT}-hex-equal`, (hex) => hex); expect(res.exitCode).toBeUndefined(); }); @@ -310,6 +312,50 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( 30_000, ); +test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( + "against a real Flair, create succeeds with an existing Agent and matching local key", + async () => { + const baseUrl = process.env.TPS_TEST_FLAIR_URL ?? "http://127.0.0.1:9926"; + const opsUrl = process.env.TPS_TEST_FLAIR_OPS_URL ?? "http://127.0.0.1:9925"; + const admin = process.env.TPS_TEST_FLAIR_ADMIN ?? "admin:test123"; + const id = `create-existing-${randomUUID()}`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const wire = Buffer.from(admin).toString("base64"); + const headers = { Authorization: `Basic ${wire}`, "Content-Type": "application/json" }; + const inserted = await fetch(opsUrl, { + method: "POST", headers, + body: JSON.stringify({ + operation: "insert", database: "flair", table: "Agent", + records: [{ id, name: id, publicKey, createdAt: keyPair.createdAt }], + }), + }); + try { + expect(inserted.ok).toBe(true); + const res = await runCreate(baseUrl, id, { keyPair, credential: admin, inspectError: true }); + expect(res.exitCode).toBeUndefined(); + expect(res.hasError).toBe(false); + expect(res.error).toBeNull(); + expect(res.stdout).toContain("already registered in Flair"); + expect(res.stderr).toBe(""); + for (const stream of [res.stdout, res.stderr, format(res.error)]) { + expect(stream).not.toContain(admin); + expect(stream).not.toContain(wire); + } + const record = await fetch(`${baseUrl}/Agent/${id}`, { headers }); + expect(record.ok).toBe(true); + expect((await record.json() as { publicKey: string }).publicKey).toBe(publicKey); + } finally { + const deleted = await fetch(opsUrl, { + method: "POST", headers, + body: JSON.stringify({ operation: "delete", database: "flair", table: "Agent", hash_values: [id] }), + }); + expect(deleted.ok).toBe(true); + } + }, + 30_000, +); + test("selected create cases omit the credential marker and Basic encoding from captured output and errors", async () => { const marker = `credential-593-${randomUUID()}-${"z".repeat(40)}`; const credential = `operator:${marker}`; @@ -330,7 +376,7 @@ test("selected create cases omit the credential marker and Basic encoding from c const readFailId = `${AGENT}-cred-read-fail`; const readFail = make(async (req, url) => { - if (req.method === "PUT") return Response.json({}, { status: 200 }); + if (req.method === "PUT") return refusePut(url, readFailId); if (url.pathname === `/Agent/${readFailId}`) return new Response("boom", { status: 500 }); return undefined; }); @@ -350,16 +396,15 @@ test("selected create cases omit the credential marker and Basic encoding from c }); const okId = `${AGENT}-cred-ok`; - let okStored: string | null = null; - const ok = make(async (req, url) => { - if (url.pathname === `/Agent/${okId}` && req.method === "PUT") { - const body = (await req.json()) as { publicKey?: string }; - okStored = body.publicKey ?? null; - return Response.json({}, { status: 200 }); - } - if (url.pathname === `/Agent/${okId}` && okStored !== null) return Response.json({ id: okId, name: okId, publicKey: okStored }); + const okKeyPair = generateKeyPair(); + const okPublicKey = Buffer.from(okKeyPair.signing.publicKey).toString("base64url"); + const okRow = { id: okId, name: okId, publicKey: okPublicKey, createdAt: okKeyPair.createdAt }; + const ok = startFakeFlair(async (req, url) => { + if (req.method === "PUT" || req.method === "PATCH") return refusePut(url, okId); + if (req.method === "GET" && url.pathname === `/Agent/${okId}`) return Response.json(okRow); return undefined; - }); + }, { [okId]: okPublicKey }); + fakes.push(ok); const paths = [ { name: "missing id", url: "http://127.0.0.1:1", id: "", exitCode: 1, message: "Usage: tps agent create" }, @@ -368,12 +413,12 @@ test("selected create cases omit the credential marker and Basic encoding from c { name: "failed read-back", fake: readFail, id: readFailId, exitCode: 1, message: "the read-back failed", seed: true }, { name: "row holds no key", fake: noKey, id: noKeyId, exitCode: 1, message: "the row has no public key", seed: true }, { name: "row holds a different key", fake: differ, id: differId, exitCode: 1, message: "not the generated key", seed: true }, - { name: "success", fake: ok, id: okId, exitCode: undefined, message: "registered in Flair (no seed", seed: true }, + { name: "success", fake: ok, id: okId, keyPair: okKeyPair, exitCode: undefined, message: "already registered in Flair", seed: false }, ]; try { for (const path of paths) { - const res = await runCreate(path.fake?.url ?? path.url!, path.id, { credential, noSeed: path.noSeed, inspectError: true }); + const res = await runCreate(path.fake?.url ?? path.url!, path.id, { credential, noSeed: path.noSeed, inspectError: true, keyPair: path.keyPair }); for (const stream of [res.stdout, res.stderr, format(res.error)]) { expect(`${path.name}: ${stream}`).not.toContain(marker); expect(`${path.name}: ${stream}`).not.toContain(wire); @@ -386,12 +431,17 @@ test("selected create cases omit the credential marker and Basic encoding from c else { expect(res.stderr).toBe(""); expect(res.stdout).toContain("Config written to"); - expect(okStored).toMatch(/^[a-f0-9]{64}$/); + expect(okRow.publicKey).toBe(okPublicKey); + expect(ok.requests.map(({ method, path }) => ({ method, path }))).toEqual([ + { method: "GET", path: `/Agent/${okId}` }, + { method: "GET", path: `/Agent/${okId}` }, + ]); + expect(ok.requests[0]!.auth).toStartWith(`TPS-Ed25519 ${okId}:`); } if (path.fake) { const basic = path.fake.requests.filter((req) => req.auth.startsWith("Basic ")); expect(basic).toEqual([ - { method: "PUT", path: `/Agent/${path.id}`, auth: `Basic ${wire}` }, + ...(path.keyPair ? [] : [{ method: "PUT", path: `/Agent/${path.id}`, auth: `Basic ${wire}` }]), { method: "GET", path: `/Agent/${path.id}`, auth: `Basic ${wire}` }, ]); expect(path.fake.requests.some((req) => req.method === "POST" && req.path === "/AgentSeed/")).toBe(path.seed);