diff --git a/.changelog/unreleased/fixed-512-create-reports-real-registration.md b/.changelog/unreleased/fixed-512-create-reports-real-registration.md new file mode 100644 index 00000000..cee05f2e --- /dev/null +++ b/.changelog/unreleased/fixed-512-create-reports-real-registration.md @@ -0,0 +1 @@ +- **`tps agent create` prints success only after the generated key reads back equal from Flair.** It re-reads the agent's stored `publicKey` over the operator credential; when Flair is reachable, it exits non-zero unless the stored key reads back equal to the generated key, naming the agent, the Flair URL and the remedy. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index 73cd471b..8777fcbd 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -241,6 +241,46 @@ async function loadSoulFile(filePath: string): Promise> { return result; } +/** Flair stores base64url (`flair agent add`) and accepts hex or base64/base64url; only a 32-byte decode is a key. */ +function storedKeyAsHex(stored: string): string | null { + let bytes: Buffer; + if (/^[0-9a-fA-F]{64}$/.test(stored)) bytes = Buffer.from(stored, "hex"); + else if (/^[A-Za-z0-9+/_-]+={0,2}$/.test(stored)) bytes = Buffer.from(stored, "base64"); + else return null; + return bytes.length === 32 ? bytes.toString("hex") : null; +} + +/** + * cli#512 — when Flair is reachable, `create` exits non-zero here unless the + * stored key reads back equal to the generated key. + */ +function refuseRegistration( + id: string, + flairUrl: string, + identityDir: string, + detail: string, + writeError: string | null, + rowExists: boolean, +): never { + const cause = writeError ? `${detail}; the registration write failed: ${writeError}` : detail; + console.error(`❌ Agent '${id}' is not registered in Flair at ${flairUrl} — ${cause}.`); + console.error( + ` 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) { + 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 { + console.error(` Until then: ${copy}.`); + } + process.exit(1); +} + async function createAgent(args: AgentArgs): Promise { const id = args.id; if (!id) { @@ -274,7 +314,12 @@ async function createAgent(args: AgentArgs): Promise { console.log(` Keys saved to ${identityDir}/`); } - // 2. Register in Flair + // 2. Register in Flair, then verify the key actually landed. + // + // cli#512: registration is reported only when Flair reads the key back equal + // to the key just generated. Current Flair drops `publicKey` on Agent PUT and + // PATCH, so neither can store it. The exit decision rests on the read-back; + // the write errors recorded in writeError are reported when it fails. const flair = createFlairClient(id, flairUrl, keyPath); const online = await flair.ping(); @@ -282,40 +327,62 @@ async function createAgent(args: AgentArgs): Promise { console.warn(` ⚠️ Flair not reachable at ${flairUrl} — skipping registration.`); console.warn(` Run setup-harper.sh and retry: tps agent create --id ${id}`); } else { - const existing = await flair.getAgent(id); - if (existing) { - console.log(` Agent '${id}' already registered in Flair.`); - // Still register real public key if record has placeholder - if (existing.publicKey === "pending") { - await flair.updateAgent(id, { publicKey: pubKeyHex }).catch(() => {}); - } - } else if (!args.noSeed) { - // Seed agent with soul + starter memories - const soulTemplate = args.soulFile - ? await loadSoulFile(args.soulFile) - : undefined; - const starterMemories = args.starterMemories; - try { - const seeded = await flair.seedAgent({ - agentId: id, - displayName: name, - role: "agent", - soulTemplate, - starterMemories, - }); - // Update the public key on the agent record that AgentSeed created - await flair.updateAgent(id, { publicKey: pubKeyHex }).catch(() => {}); - console.log(` Agent seeded: ${seeded.soulEntries.length} soul entries, ${seeded.memories.length} memories.`); - } catch (_e: any) { - // AgentSeed requires admin auth — fall back to direct registration - await flair.registerAgent(name, pubKeyHex).catch(() => {}); - console.log(` Agent '${id}' registered in Flair (no seed — not admin).`); + let writeError: string | null = null; + let successLine: string | null = null; + try { + const existing = await flair.getAgent(id); + if (existing) { + if (storedKeyAsHex(existing.publicKey ?? "") !== pubKeyHex) { + await flair.updateAgent(id, { publicKey: pubKeyHex }); + } + successLine = ` Agent '${id}' already registered in Flair.`; + } else if (!args.noSeed) { + // Seed agent with soul + starter memories + const soulTemplate = args.soulFile + ? await loadSoulFile(args.soulFile) + : undefined; + const starterMemories = args.starterMemories; + try { + const seeded = await flair.seedAgent({ + agentId: id, + displayName: name, + role: "agent", + soulTemplate, + starterMemories, + }); + // Update the public key on the agent record that AgentSeed created + await flair.updateAgent(id, { publicKey: pubKeyHex }); + successLine = ` Agent seeded: ${seeded.soulEntries.length} soul entries, ${seeded.memories.length} memories.`; + } catch (seedErr) { + // AgentSeed requires admin auth — fall back to direct registration + writeError = `seed or key update failed: ${seedErr instanceof Error ? seedErr.message : String(seedErr)}`; + await flair.registerAgent(name, pubKeyHex); + successLine = ` Agent '${id}' registered in Flair (no seed — not admin).`; + } + } else { + // --no-seed: just register + await flair.registerAgent(name, pubKeyHex); + successLine = ` Agent '${id}' registered in Flair (seeding skipped).`; } - } else { - // --no-seed: just register - await flair.registerAgent(name, pubKeyHex); - console.log(` Agent '${id}' registered in Flair (seeding skipped).`); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + writeError = writeError ? `${writeError}; ${msg}` : msg; + } + + let stored: { found: boolean; publicKey: string | null }; + 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); + } + if (!stored.found) { + refuseRegistration(id, flairUrl, identityDir, "no Agent row exists", writeError, false); + } 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); } + console.log(successLine ?? ` Agent '${id}' registered in Flair.`); } // 3. Write agent config diff --git a/packages/cli/src/utils/flair-client.ts b/packages/cli/src/utils/flair-client.ts index 2503b1d7..99a14460 100644 --- a/packages/cli/src/utils/flair-client.ts +++ b/packages/cli/src/utils/flair-client.ts @@ -287,6 +287,31 @@ export class FlairClient { } } + /** + * Read the public key Flair actually stores for `agentId`, over the operator + * (Basic admin) credential. cli#512: the read-back that decides whether a + * registration happened must not use the key it is verifying — an agent + * whose key is not registered yet cannot read its own row. A 404 is the only + * "no such row"; every other non-OK response throws, so a failed read is + * never read as "absent". + */ + async readStoredPublicKey( + agentId: string = this.agentId, + adminAuth?: string, + ): Promise<{ found: boolean; publicKey: string | null }> { + const auth = adminAuth ?? process.env.FLAIR_ADMIN_AUTH ?? "admin:admin123"; + const res = await fetch(`${this.baseUrl}/Agent/${encodeURIComponent(agentId)}`, { + headers: { Authorization: `Basic ${Buffer.from(auth).toString("base64")}` }, + }); + if (res.status === 404) return { found: false, publicKey: null }; + if (!res.ok) { + const text = await res.text().catch(() => ""); + throw new FlairRequestError(`Flair GET /Agent/${agentId} → ${res.status}: ${text}`, res.status); + } + const record = (await res.json()) as FlairAgent | null; + return { found: true, publicKey: typeof record?.publicKey === "string" ? record.publicKey : null }; + } + async listAgents(): Promise { try { return await this.request("GET", "/Agent/"); diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts new file mode 100644 index 00000000..76cf6788 --- /dev/null +++ b/packages/cli/test/agent-create-registration.test.ts @@ -0,0 +1,277 @@ +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 + * 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. + */ +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 { runAgent } from "../src/commands/agent.js"; +import { stubFlairHandler } from "./helpers/stub-flair.js"; + +const AGENT = "create-registration-agent"; + +// cli#555: a file that names `spyOn` carries the top-level teardown; each +// runCreate restores its own spies in a finally as well. +afterEach(() => { + mock.restore(); +}); + +interface FakeFlair { + url: string; + stop(): void; +} + +/** + * 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 + * 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({}); + 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") ?? ""; + 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) }; +} + +/** A Basic `PUT /Agent/` that Flair refuses (it drops `publicKey`, so the write cannot complete). */ +function refusePut(url: URL, id: string): Response | undefined { + if (url.pathname === `/Agent/${id}`) return Response.json({ type: "error:ValidationError", code: "ValidationError", status: 400 }, { status: 400 }); + return undefined; +} + +/** Run `tps agent create` for `id` against `url`, capturing its exit code and output. */ +async function runCreate( + url: string, + id: string, + flags: { noSeed?: boolean } = {}, +): Promise<{ exitCode: number | undefined; stdout: string; stderr: string }> { + const home = mkdtempSync(join(tmpdir(), "create-registration-")); + const savedHome = process.env.HOME; + process.env.HOME = home; + 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(" ")); }); + let exitCode: number | undefined; + const exitSpy = spyOn(process, "exit").mockImplementation(((code?: number): never => { + exitCode = code; + throw new Error(`exit:${code}`); + }) as never); + try { + await runAgent({ action: "create", id, name: id, flairUrl: url, noSeed: flags.noSeed }); + } catch (err) { + if (!String(err).includes("exit:")) throw err; + } finally { + exitSpy.mockRestore(); + logSpy.mockRestore(); + errSpy.mockRestore(); + if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome; + rmSync(home, { recursive: true, force: true }); + } + return { exitCode, stdout: stdout.join("\n"), stderr: stderr.join("\n") }; +} + +test("refused write and no Agent row: exits non-zero, prints no success, names agent, URL and remedy", async () => { + const id = `${AGENT}-no-row`; + const fake = startFakeFlair(async (req, url) => (req.method === "PUT" ? refusePut(url, id) : undefined)); + try { + const res = await runCreate(fake.url, id); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(id); + expect(res.stderr).toContain(fake.url); + expect(res.stderr).toContain("no Agent row exists"); + expect(res.stderr).toContain("seed or key update failed"); + expect(res.stderr).toContain("flair#2266"); + expect(res.stderr).toContain(`flair agent add ${id} --keys-dir`); + expect(res.stderr).not.toContain("remove"); + expect(res.stderr).toContain(join(".tps", "identity", `${id}.key`)); + expect(res.stderr).toContain(join(".tps", "identity", `${id}.pub`)); + expect(res.stderr).toContain(join(".tps", "identity")); + } finally { + fake.stop(); + } +}); + +test("the stored key is still `pending`: exits non-zero naming the stored value", async () => { + const id = `${AGENT}-pending`; + const fake = startFakeFlair(async (req, url) => { + if (req.method === "PUT") return refusePut(url, id); + if (url.pathname === `/Agent/${id}`) return Response.json({ id, name: id, publicKey: "pending" }); + return undefined; + }); + try { + const res = await runCreate(fake.url, id); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain("the stored public key is 'pending'"); + const remove = res.stderr.indexOf(`flair agent remove ${id}`); + expect(remove).toBeGreaterThan(-1); + expect(remove).toBeLessThan(res.stderr.indexOf(`flair agent add ${id}`)); + expect(res.stderr).toContain("Memory and Soul rows"); + } finally { + fake.stop(); + } +}); + +test("the stored key differs from the generated one: exits non-zero", async () => { + const id = `${AGENT}-mismatch`; + const other = "de".repeat(32); + const fake = startFakeFlair(async (req, url) => { + if (req.method === "PUT") return refusePut(url, id); + if (url.pathname === `/Agent/${id}`) return Response.json({ id, name: id, publicKey: other }); + return undefined; + }); + try { + const res = await runCreate(fake.url, id); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain("not the generated key"); + } finally { + fake.stop(); + } +}); + +test("the read-back fails: exits non-zero, never treating a failed read as absent", async () => { + const id = `${AGENT}-read-fail`; + const fake = startFakeFlair(async (req, url) => { + if (req.method === "PUT") return Response.json({}, { status: 200 }); + if (url.pathname === `/Agent/${id}`) return new Response("boom", { status: 500 }); + return undefined; + }); + try { + const res = await runCreate(fake.url, id); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain("the read-back failed"); + } finally { + fake.stop(); + } +}); + +test("the key reads back equal: prints success and exits zero", async () => { + const id = `${AGENT}-ok`; + let stored: string | null = null; + 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 }); + return undefined; + }); + try { + const res = await runCreate(fake.url, id); + expect(res.exitCode).toBeUndefined(); + expect(res.stdout).toContain("registered in Flair (no seed"); + expect(res.stderr).toBe(""); + expect(stored).toMatch(/^[a-f0-9]{64}$/); + } 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 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 }); + return undefined; + }); + try { + return await runCreate(fake.url, id); + } finally { + fake.stop(); + } +} + +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 () => { + const res = await runWithStoredEncoding(`${AGENT}-b64-equal`, b64url); + expect(res.exitCode).toBeUndefined(); + expect(res.stdout).toContain("registered in Flair"); +}); + +test("the row holds the base64url of a different key: exits non-zero", async () => { + const res = await runWithStoredEncoding(`${AGENT}-b64-other`, () => b64url("de".repeat(32))); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain("not the generated key"); +}); + +test("the row holds the hex of the generated key (flair accepts either): exits zero", async () => { + const res = await runWithStoredEncoding(`${AGENT}-hex-equal`, (hex) => hex); + expect(res.exitCode).toBeUndefined(); +}); + +test("the row holds a value that decodes to the wrong length: exits non-zero", async () => { + const res = await runWithStoredEncoding(`${AGENT}-short`, (hex) => Buffer.from(hex, "hex").subarray(0, 31).toString("base64url")); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); +}); + +/** + * The seam through the REAL component: a real Harper running Flair. Skips + * everywhere one is not reachable (CI has no Flair lane); run it with + * TPS_TEST_REAL_FLAIR=1 and TPS_TEST_FLAIR_URL pointing at an instance whose + * admin credential is TPS_TEST_FLAIR_ADMIN (default admin:test123). Current + * Flair drops `publicKey` on Agent PUT/PATCH, so a fresh `create` cannot + * register: it must exit non-zero and print no success line. + */ +test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( + "against a real Flair, create refuses when the generated key cannot be registered", + async () => { + const baseUrl = process.env.TPS_TEST_FLAIR_URL ?? "http://127.0.0.1:9926"; + const admin = process.env.TPS_TEST_FLAIR_ADMIN ?? "admin:test123"; + const id = `create-refusal-${randomUUID()}`; + const savedAdmin = process.env.FLAIR_ADMIN_AUTH; + process.env.FLAIR_ADMIN_AUTH = admin; + try { + const res = await runCreate(baseUrl, id); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stdout).not.toContain("✅"); + expect(res.stderr).toContain(id); + expect(res.stderr).toContain(baseUrl); + expect(res.stderr).toContain("flair#2266"); + // Independently of the CLI's own read-back: the real Flair stored no row. + const record = await fetch(`${baseUrl}/Agent/${encodeURIComponent(id)}`, { + headers: { Authorization: `Basic ${Buffer.from(admin).toString("base64")}` }, + }); + expect(record.status).toBe(404); + } finally { + if (savedAdmin === undefined) delete process.env.FLAIR_ADMIN_AUTH; else process.env.FLAIR_ADMIN_AUTH = savedAdmin; + } + }, + 30_000, +);