From 69e39553381e7b3c340b50a94abb311533aa7de7 Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 16:14:07 +0000 Subject: [PATCH 1/8] feat(agent): create registers a pending agent's key through POST /AgentKeyRegister (cli#512) `tps agent create` completes registration when Flair's row for the agent stores `publicKey: "pending"`: it sends the generated key through POST /AgentKeyRegister over its operator credential, then runs the unchanged read-back, so success is printed only on an equal read-back. Every refusal from the endpoint exits non-zero naming the agent, the Flair URL, the refusal name and a remedy; a Flair without the route (404 or 405) exits non-zero naming the minimum version. Flair's Agent PUT and PATCH store no key, so create no longer sends the key that way. --- .../changed-512-create-registers-key.md | 7 + packages/cli/src/commands/agent.ts | 81 +++++- packages/cli/src/utils/flair-client.ts | 92 +++++++ .../test/agent-create-registration.test.ts | 241 +++++++++++++++++- packages/cli/test/helpers/scratch-flair.ts | 9 +- scripts/real-flair-lane.ts | 1 + 6 files changed, 416 insertions(+), 15 deletions(-) create mode 100644 .changelog/unreleased/changed-512-create-registers-key.md diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md new file mode 100644 index 00000000..771aae9a --- /dev/null +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -0,0 +1,7 @@ +- **`tps agent create` completes registration when Flair's row for the agent is `pending`.** + It sends the generated key through `POST /AgentKeyRegister` over its operator + credential and reads the stored key back: success is printed only when the key + reads back equal. Each refusal from the endpoint exits non-zero with the agent, + the Flair URL, the refusal name and a remedy; a Flair that does not serve the + route (404 or 405) exits non-zero and names the minimum Flair version that does. + Flair's `Agent` PUT and PATCH do not store `publicKey`, so neither is used. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index ff3b9e7a..afccc53f 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -13,7 +13,7 @@ import { AgentRuntime, loadAgentConfig } from "@tpsdev-ai/agent"; import yaml from "js-yaml"; import { generateKeyPair, saveKeyPair, loadKeyPair, verify } from "../utils/identity.js"; -import { createFlairClient, isEncryptedPrivateKeyPem, loadFlairSigningKey } from "../utils/flair-client.js"; +import { createFlairClient, FlairAgentKeyRegisterError, isEncryptedPrivateKeyPem, loadFlairSigningKey } from "../utils/flair-client.js"; import { applyRole, loadPlugins } from "../plugins/index.js"; import { createInterface as createPromptInterface } from "node:readline/promises"; import { accessSync, constants, createReadStream, existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, renameSync, statSync, watch, writeFileSync } from "node:fs"; @@ -398,6 +398,54 @@ function refuseUnloadableIdentity(id: string, identityDir: string, err: unknown) process.exit(1); } +/** + * cli#512 — the first Flair release that serves POST /AgentKeyRegister, the + * operation `create` uses to store a locally generated key on a `pending` row. + * The route merged into Flair main after its 0.61.0 entry, so no published + * release has it (the newest published is 0.59.0). + */ +const MIN_FLAIR_VERSION_FOR_KEY_REGISTER = "0.62.0"; + +/** Flair's stored `publicKey` value for an Agent row seeded but not yet registered (flair#2266). */ +const PENDING_PUBLIC_KEY = "pending"; + +/** + * cli#512 — `create` refuses when POST /AgentKeyRegister does not store the + * generated key. `status`/`refusal` are the endpoint's own response: the HTTP + * status and its refusal name, or null when the body named none this client + * knows. A 404 or 405 with no refusal is a Flair without the route. + * + * Every branch exits non-zero and none falls back to a write the old server + * would mishandle; the slice-1 read-back remains the only source of success. + */ +function refuseKeyRegistration(id: string, flairUrl: string, status: number, refusal: string | null): never { + if (refusal === null && (status === 404 || status === 405)) { + console.error( + `❌ Agent '${id}' is not registered in Flair at ${flairUrl} — this Flair has no POST /AgentKeyRegister (${status}), so a key generated here cannot be stored.`, + ); + console.error( + ` Remedy: upgrade this Flair to ${MIN_FLAIR_VERSION_FOR_KEY_REGISTER} or newer, then re-run \`tps agent create --id ${id}\`.`, + ); + process.exit(1); + } + const remedies: Record = { + "authentication required": "the admin credential is empty or invalid; set FLAIR_ADMIN_AUTH to this Flair's admin credential", + forbidden: "the credential is not this Flair's admin; set FLAIR_ADMIN_AUTH to its admin credential", + registration_request_invalid: "the generated agent id or public key was refused as malformed", + agent_not_found: `no Agent row exists for '${id}'; create the agent's row on this Flair instance, then re-run \`tps agent create --id ${id}\``, + agent_not_local: `'${id}' did not originate on this instance; register its key on the agent's home instance`, + agent_inactive: `'${id}' is deactivated; reactivate it before registering a key`, + agent_key_not_pending: `'${id}' already holds a key; replacing it is the key-rotation path, not create`, + registration_conflict: "another write to this agent is in progress; re-run `tps agent create`", + registration_unavailable: "the registration store was unavailable or the committed key did not read back; re-run `tps agent create`", + }; + const name = refusal ?? `no refusal named (${status})`; + const remedy = refusal !== null ? remedies[refusal] : "the response named no refusal this client knows; check that Flair is reachable and its version"; + console.error(`❌ Agent '${id}' is not registered in Flair at ${flairUrl} — POST /AgentKeyRegister refused the registration (${name}).`); + console.error(` Remedy: ${remedy}.`); + process.exit(1); +} + async function createAgent(args: AgentArgs): Promise { const id = args.id; if (!id) { @@ -471,8 +519,9 @@ async function createAgent(args: AgentArgs): Promise { // // 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. + // PATCH, so neither can store it; a `pending` row is completed through + // POST /AgentKeyRegister. 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(); @@ -503,12 +552,14 @@ async function createAgent(args: AgentArgs): Promise { soulTemplate, starterMemories, }); - // Update the public key on the agent record that AgentSeed created - await flair.updateAgent(id, { publicKey: pubKeyHex }); + // cli#512: the seed leaves (or keeps) a row whose stored key is + // `pending`; the block after the write registers the generated key + // through Flair's supported operation. Flair's Agent PUT does not + // store `publicKey`, so it is not sent here. 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)}`; + writeError = `seed failed: ${seedErr instanceof Error ? seedErr.message : String(seedErr)}`; await flair.registerAgent(name, pubKeyHex); successLine = ` Agent '${id}' registered in Flair (no seed — not admin).`; } @@ -530,6 +581,24 @@ async function createAgent(args: AgentArgs): Promise { // 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 && stored.publicKey === PENDING_PUBLIC_KEY) { + // cli#512 — the row is `pending`: register the generated key through + // Flair's supported operation, then read again; the slice-1 check below + // decides, so success is printed only on an equal read-back. This covers a + // row seeded earlier and one this run seeded. + const publicKeyB64url = Buffer.from(kp.signing.publicKey).toString("base64url"); + try { + await flair.registerAgentKey(id, publicKeyB64url); + } catch (e) { + if (e instanceof FlairAgentKeyRegisterError) refuseKeyRegistration(id, flairUrl, e.status, e.refusal); + refuseKeyRegistration(id, flairUrl, 0, 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, "unknown"); + } + } if (!stored.found) { refuseRegistration(id, flairUrl, identityDir, "no Agent row exists", writeError, "absent"); } else if (stored.publicKey === null || storedKeyAsHex(stored.publicKey) !== pubKeyHex) { diff --git a/packages/cli/src/utils/flair-client.ts b/packages/cli/src/utils/flair-client.ts index d84c455a..c41bdf09 100644 --- a/packages/cli/src/utils/flair-client.ts +++ b/packages/cli/src/utils/flair-client.ts @@ -299,6 +299,56 @@ function responseError(entry: string, method: string, path: string, status: numb return `Flair ${entry} ${method} ${path} → ${status}: ${code}`; } +/** + * The refusal names POST /AgentKeyRegister is documented to answer in its + * `error` field (flair#2266): the authentication denials and the registration + * refusals. A body naming none of these yields `null` — the response text is + * never echoed, so a route that is not this endpoint (a Flair without it + * answers 404 or 405 with no such field) is recognised as naming no refusal. + */ +const AGENT_KEY_REGISTER_REFUSALS: readonly string[] = [ + "authentication required", + "forbidden", + "registration_request_invalid", + "agent_not_found", + "agent_not_local", + "agent_inactive", + "agent_key_not_pending", + "registration_conflict", + "registration_unavailable", +]; + +/** Membership — module-private, so nothing outside this module can change how a refusal is named. */ +const AGENT_KEY_REGISTER_REFUSAL_SET: ReadonlySet = new Set(AGENT_KEY_REGISTER_REFUSALS); + +/** The refusal name on a POST /AgentKeyRegister response, or null when the body named none this client knows. */ +function agentKeyRegisterRefusal(body: string): string | null { + let parsed: unknown; + try { + parsed = JSON.parse(body); + } catch { + return null; + } + const code = (parsed as { error?: unknown } | null)?.error; + return typeof code === "string" && AGENT_KEY_REGISTER_REFUSAL_SET.has(code) ? code : null; +} + +/** + * A refusal from POST /AgentKeyRegister: the HTTP status and the endpoint's own + * refusal name, or null when the response named none this client knows. A 404 + * or 405 with a null refusal is a Flair without the route. The message carries + * the status and the allow-listed name only, never response text. + */ +export class FlairAgentKeyRegisterError extends Error { + constructor( + public readonly status: number, + public readonly refusal: string | null, + ) { + super(`Flair registerAgentKey POST /AgentKeyRegister → ${status}: ${refusal ?? UNRECOGNIZED_ERROR}`); + this.name = "FlairAgentKeyRegisterError"; + } +} + /** The `code` of an object-valued failure or cause; other candidates, including callables, are ignored. */ function errorCode(err: unknown): string { try { @@ -563,6 +613,48 @@ export class FlairClient { } } + /** + * POST /AgentKeyRegister — register `publicKey` (the canonical base64url + * encoding of 32 bytes) for the `pending` Agent row `agentId`, over the + * operator (Basic admin) credential. cli#512: this is Flair's supported + * operation for a row whose key Flair's Agent PUT and PATCH do not store. A + * non-OK response throws a FlairAgentKeyRegisterError carrying the status and + * the endpoint's refusal name (null when the body named none this client + * knows, which is how a Flair without the route is recognised). A transport + * failure throws a FlairRequestError (status 0). + */ + async registerAgentKey( + agentId: string, + publicKey: string, + adminAuth?: string, + ): Promise<{ agentId: string; publicKey: string; updatedAt: string; registrationId: string }> { + const url = `${this.baseUrl}/AgentKeyRegister`; + const route = requestedPath(url); + const auth = adminAuth ?? process.env.FLAIR_ADMIN_AUTH ?? "admin:admin123"; + let res: Response; + try { + res = await fetch(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Basic ${Buffer.from(auth).toString("base64")}`, + }, + body: JSON.stringify({ agentId, publicKey }), + }); + } catch (err) { + throw new FlairRequestError(transportError("registerAgentKey", "POST", route, err), 0); + } + if (!res.ok) { + const text = await res.text().catch(() => ""); + throw new FlairAgentKeyRegisterError(res.status, agentKeyRegisterRefusal(text)); + } + try { + return (await res.json()) as { agentId: string; publicKey: string; updatedAt: string; registrationId: string }; + } catch { + throw new FlairRequestError(parseError("registerAgentKey", "POST", route, res.status), res.status); + } + } + async getAgent(id?: string): Promise { try { return await this.request("GET", `/Agent/${encodeURIComponent(id ?? this.agentId)}`); diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index 9c9f6ae8..62fd943b 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -146,7 +146,7 @@ test("refused write and no Agent row: exits non-zero, prints no success, names a 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("seed failed"); expect(res.stderr).toContain("flair#2266"); expect(res.stderr).toContain(`flair agent add ${id} --keys-dir`); expect(res.stderr).not.toContain("remove"); @@ -158,8 +158,8 @@ test("refused write and no Agent row: exits non-zero, prints no success, names a } }); -test("the stored key is still `pending`: exits non-zero naming the stored value", async () => { - const id = `${AGENT}-pending`; +test("a pending row on a Flair without POST /AgentKeyRegister: exits non-zero naming the minimum version, prints no success", async () => { + const id = `${AGENT}-pending-no-route`; 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" }); @@ -169,16 +169,181 @@ test("the stored key is still `pending`: exits non-zero naming the stored value" 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"); + expect(res.stderr).toContain(`Agent '${id}' is not registered in Flair at ${fake.url}`); + expect(res.stderr).toContain("this Flair has no POST /AgentKeyRegister (404)"); + expect(res.stderr).toContain("upgrade this Flair to 0.62.0 or newer"); + // no fallback write after the missing route: the registration POST is last + expect(fake.requests.at(-1)).toEqual({ method: "POST", path: "/AgentKeyRegister", auth: expect.stringContaining("Basic ") }); } finally { fake.stop(); } }); +test("a 405 on the registration route: exits non-zero naming the minimum version", async () => { + const id = `${AGENT}-pending-method`; + const fake = startFakeFlair(async (req, url) => { + if (req.method === "PUT") return refusePut(url, id); + if (req.method === "POST" && url.pathname === "/AgentKeyRegister") return new Response("method not allowed", { status: 405 }); + 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("this Flair has no POST /AgentKeyRegister (405)"); + expect(res.stderr).toContain("upgrade this Flair to 0.62.0 or newer"); + } finally { + fake.stop(); + } +}); + +interface RegistrarFlair { + url: string; + stop(): void; + readonly row: { id: string; name: string; publicKey: string }; + readonly requests: Array<{ method: string; path: string; auth: string }>; + readonly registerBodies: Array>; +} + +/** + * A fake Flair that serves POST /AgentKeyRegister over Basic admin for a + * `pending` Agent row, modeled on flair resources/AgentKeyRegister.ts: it admits + * Basic admin, stores the body's publicKey on the row and answers the documented + * body on 200, and answers a signed GET for a still-`pending` row as + * `unknown_agent` (a pending row cannot authenticate as itself). `route` omits + * the route (404) or answers 405; `refusal` answers a named refusal; `store: + * false` answers 200 without storing (the read-back then differs). + */ +function startRegistrarFlair( + id: string, + publicKey: string, + opts: { + route?: "serve" | "missing" | "method"; + refusal?: { status: number; error: string }; + store?: boolean; + } = {}, +): RegistrarFlair { + const route = opts.route ?? "serve"; + const row = { id, name: id, publicKey: "pending" }; + const requests: RegistrarFlair["requests"] = []; + const registerBodies: Array> = []; + let registrations = 0; + const routes = async (req: Request, url: URL): Promise => { + const auth = req.headers.get("Authorization") ?? ""; + const signed = auth.startsWith("TPS-Ed25519 "); + if (req.method === "POST" && url.pathname === "/AgentKeyRegister") { + if (route === "missing") return undefined; + if (route === "method") return new Response("method not allowed", { status: 405 }); + let body: Record = {}; + try { + body = (await req.json()) as Record; + } catch { + /* no body */ + } + registerBodies.push(body); + if (opts.refusal) return Response.json({ error: opts.refusal.error }, { status: opts.refusal.status }); + registrations += 1; + if (opts.store === false) { + return Response.json({ agentId: id, publicKey: String(body.publicKey), updatedAt: new Date().toISOString(), registrationId: `reg-${registrations}` }); + } + row.publicKey = String(body.publicKey); + return Response.json({ agentId: id, publicKey: row.publicKey, updatedAt: new Date().toISOString(), registrationId: `reg-${registrations}` }); + } + if (req.method === "GET" && url.pathname === `/Agent/${id}`) { + if (signed && row.publicKey === "pending") return Response.json({ error: "unknown_agent" }, { status: 401 }); + return Response.json({ ...row }); + } + if (req.method === "PUT" || req.method === "PATCH") return refusePut(url, id); + return undefined; + }; + const signedStub = stubFlairHandler({ [id]: publicKey }, routes); + 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 routes(req, url)) ?? new Response("not found", { status: 404 }); + return signedStub(req); + }, + }); + return { url: `http://127.0.0.1:${server.port}`, stop: () => server.stop(true), row, requests, registerBodies }; +} + +test("a pending row and a Flair that serves POST /AgentKeyRegister: the generated key is registered and read back equal, then success", async () => { + const id = `${AGENT}-registers`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey); + try { + const res = await runCreate(fake.url, id, { keyPair, inspectError: true }); + expect(res.hasError).toBe(false); + expect(res.exitCode).toBeUndefined(); + expect(res.stdout).toContain("registered in Flair"); + // the key registered is the base64url of the generated public key + expect(fake.registerBodies).toEqual([{ agentId: id, publicKey }]); + expect(fake.row.publicKey).toBe(publicKey); + // the registration is over Basic admin; the signed verification follows it + const registration = fake.requests.findIndex((req) => req.method === "POST" && req.path === "/AgentKeyRegister"); + expect(registration).toBeGreaterThan(-1); + expect(fake.requests[registration]!.auth).toStartWith("Basic "); + expect(fake.requests.at(-1)).toEqual({ method: "GET", path: `/Agent/${id}`, auth: expect.stringContaining("TPS-Ed25519 ") }); + } finally { + fake.stop(); + } +}); + +test("the endpoint answers 200 but the row does not read the key back: exits non-zero, prints no success", async () => { + const id = `${AGENT}-accepted-unstored`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { store: false }); + try { + const res = await runCreate(fake.url, id, { keyPair }); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain("the stored public key is 'pending', not the generated key"); + } finally { + fake.stop(); + } +}); + +/** A refusal from POST /AgentKeyRegister: the status the endpoint answers and the remedy its refusal names. */ +const KEY_REGISTRATION_REFUSALS: ReadonlyArray<{ status: number; error: string; status404?: boolean; remedy: string }> = [ + { status: 400, error: "registration_request_invalid", remedy: "the generated agent id or public key was refused as malformed" }, + { status: 401, error: "authentication required", remedy: "set FLAIR_ADMIN_AUTH" }, + { status: 403, error: "forbidden", remedy: "set FLAIR_ADMIN_AUTH to its admin credential" }, + { status: 403, error: "agent_not_local", remedy: "register its key on the agent's home instance" }, + { status: 403, error: "agent_inactive", remedy: "reactivate it before registering a key" }, + { status: 404, error: "agent_not_found", status404: true, remedy: "no Agent row exists" }, + { status: 409, error: "agent_key_not_pending", remedy: "key-rotation path" }, + { status: 409, error: "registration_conflict", remedy: "another write to this agent is in progress" }, + { status: 503, error: "registration_unavailable", remedy: "re-run `tps agent create`" }, +]; + +for (const refusal of KEY_REGISTRATION_REFUSALS) { + test(`POST /AgentKeyRegister refusal ${refusal.error}: exits non-zero naming the agent, URL, refusal and remedy`, async () => { + const id = `${AGENT}-refuse-${refusal.error.replace(/\W/g, "-")}`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { refusal: { status: refusal.status, error: refusal.error } }); + try { + const res = await runCreate(fake.url, id, { keyPair }); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(`Agent '${id}' is not registered in Flair at ${fake.url}`); + expect(res.stderr).toContain(`POST /AgentKeyRegister refused the registration (${refusal.error})`); + expect(res.stderr).toContain(refusal.remedy); + // agent_not_found is the endpoint's own refusal, not the missing-route case + if (!refusal.status404) expect(res.stderr).not.toContain("this Flair has no POST /AgentKeyRegister"); + } 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); @@ -749,6 +914,66 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( 240_000, ); +test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( + "against a real Flair, create on a pending row completes registration when the instance serves POST /AgentKeyRegister, and refuses otherwise", + async () => { + const { baseUrl, opsUrl, admin } = flairTarget(); + const id = `create-pending-${randomUUID()}`; + 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: "pending", createdAt: new Date().toISOString() }], + }), + }); + expect(inserted.ok).toBe(true); + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + try { + // Whether this Flair serves POST /AgentKeyRegister: a missing route answers + // 404, a present one refuses an empty body 400. Measured once, before the run. + const probe = await fetch(`${baseUrl}/AgentKeyRegister`, { + method: "POST", + headers, + body: JSON.stringify({}), + }); + const capable = probe.status !== 404 && probe.status !== 405; + + const res = await runCreate(baseUrl, id, { keyPair, credential: admin, inspectError: true }); + if (capable) { + expect(res.exitCode).toBeUndefined(); + expect(res.hasError).toBe(false); + expect(res.stdout).toContain("registered in Flair"); + const record = await fetch(`${baseUrl}/Agent/${id}`, { headers }); + expect(record.ok).toBe(true); + expect((await record.json() as { publicKey: string }).publicKey).toBe(publicKey); + } else { + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain("this Flair has no POST /AgentKeyRegister"); + expect(res.stderr).toContain("0.62.0"); + } + for (const stream of [res.stdout, res.stderr, format(res.error)]) { + expect(stream).not.toContain(admin); + expect(stream).not.toContain(wire); + } + } 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); + } + }, + 240_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}`; diff --git a/packages/cli/test/helpers/scratch-flair.ts b/packages/cli/test/helpers/scratch-flair.ts index 315a2a91..676f77c7 100644 --- a/packages/cli/test/helpers/scratch-flair.ts +++ b/packages/cli/test/helpers/scratch-flair.ts @@ -14,6 +14,13 @@ import { generateKeyPair } from "../../src/utils/identity.js"; export const FLAIR_PACKAGE = "@tpsdev-ai/flair"; export const FLAIR_VERSION = "0.58.0"; +/** + * The npm spec the scratch instance installs. Defaults to the pinned release; + * `TPS_TEST_FLAIR_SPEC` overrides it, so the lane can run against a Flair build + * that is not on npm yet (a tarball packed from a checkout, say). + */ +const FLAIR_INSTALL_SPEC = process.env.TPS_TEST_FLAIR_SPEC ?? `${FLAIR_PACKAGE}@${FLAIR_VERSION}`; + /** The operator (Harper admin) credential the helper boots and seeds with. */ export const ADMIN_AUTH = "admin:test123"; @@ -226,7 +233,7 @@ async function bootScratchFlair(instance: ScratchProcess, signal: AbortSignal) { // npm cache (HOME is the scratch home). await execFileAsync( "npm", - ["install", "--prefix", prefix, "--no-audit", "--no-fund", "--loglevel", "error", `${FLAIR_PACKAGE}@${FLAIR_VERSION}`], + ["install", "--prefix", prefix, "--no-audit", "--no-fund", "--loglevel", "error", FLAIR_INSTALL_SPEC], { env: { PATH: process.env.PATH ?? "", HOME: home, npm_config_cache: join(home, ".npm") }, signal, timeout: 300_000 }, ); diff --git a/scripts/real-flair-lane.ts b/scripts/real-flair-lane.ts index 3d887a59..d0b7ccb5 100644 --- a/scripts/real-flair-lane.ts +++ b/scripts/real-flair-lane.ts @@ -18,6 +18,7 @@ export const EXPECTED_CASES: Record = { "agent-create-registration.test.ts": [ "against a real Flair, create refuses when the generated key cannot be registered", "against a real Flair, create succeeds with an existing Agent and matching local key", + "against a real Flair, create on a pending row completes registration when the instance serves POST /AgentKeyRegister, and refuses otherwise", ], "mail-flair-read-auth.test.ts": ["real Flair Agent read guards and refused-read mail recovery"], }; From d0781ed73f92145b8588807a684e5cb6b2b353bc Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 17:50:15 +0000 Subject: [PATCH 2/8] fix(agent): scope the registration claim, cause-accurate refusals, no-fallback asserts (cli#512) - Limit the success claim to the Flair registration line (offline create still prints its ready line without a read-back); describe the existing-row Agent PUT that still carries the key. - Make each refusal remedy cause-accurate; give a transport failure and an unparseable success body their own bounded diagnostic instead of a refusal with an invented status. - Assert no Agent PUT/PATCH follows the registration POST in the 405 and every refusal case. - A 404/405 with no recognized refusal names the minimum Flair version as the likely cause, not a certainty. - Point the read-back remedy at the operation that stores a key. --- .../changed-512-create-registers-key.md | 13 ++- packages/cli/src/commands/agent.ts | 59 ++++++---- packages/cli/src/utils/flair-client.ts | 13 +-- .../test/agent-create-registration.test.ts | 102 +++++++++++++++--- 4 files changed, 138 insertions(+), 49 deletions(-) diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md index 771aae9a..3bfefeff 100644 --- a/.changelog/unreleased/changed-512-create-registers-key.md +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -1,7 +1,10 @@ - **`tps agent create` completes registration when Flair's row for the agent is `pending`.** It sends the generated key through `POST /AgentKeyRegister` over its operator - credential and reads the stored key back: success is printed only when the key - reads back equal. Each refusal from the endpoint exits non-zero with the agent, - the Flair URL, the refusal name and a remedy; a Flair that does not serve the - route (404 or 405) exits non-zero and names the minimum Flair version that does. - Flair's `Agent` PUT and PATCH do not store `publicKey`, so neither is used. + credential and reads the stored key back; the Flair registration line is + printed only when the key reads back equal. Each refusal from the endpoint + exits non-zero with the agent, the Flair URL, the refusal name and a remedy; a + response this client cannot recognize as that endpoint's refusal (404 or 405) + exits non-zero naming the minimum Flair version that serves the route as the + likely cause. Flair drops `publicKey` on `Agent` PUT and PATCH: an existing row + whose stored key differs still gets an `Agent` PUT carrying the key, which + stores nothing. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index afccc53f..c3f49eda 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -13,7 +13,7 @@ import { AgentRuntime, loadAgentConfig } from "@tpsdev-ai/agent"; import yaml from "js-yaml"; import { generateKeyPair, saveKeyPair, loadKeyPair, verify } from "../utils/identity.js"; -import { createFlairClient, FlairAgentKeyRegisterError, isEncryptedPrivateKeyPem, loadFlairSigningKey } from "../utils/flair-client.js"; +import { createFlairClient, FlairAgentKeyRegisterError, FlairRequestError, isEncryptedPrivateKeyPem, loadFlairSigningKey } from "../utils/flair-client.js"; import { applyRole, loadPlugins } from "../plugins/index.js"; import { createInterface as createPromptInterface } from "node:readline/promises"; import { accessSync, constants, createReadStream, existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, renameSync, statSync, watch, writeFileSync } from "node:fs"; @@ -274,7 +274,7 @@ function refuseRegistration( 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).`, + ` Flair drops publicKey on Agent PUT/PATCH; the operation that stores one is POST /AgentKeyRegister (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 (rowState === "exists") { @@ -282,7 +282,7 @@ function refuseRegistration( ` 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.`, + ` ⚠️ \`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 uses POST /AgentKeyRegister (flair#2266).`, ); } else if (rowState === "unknown") { console.error( @@ -413,7 +413,8 @@ const PENDING_PUBLIC_KEY = "pending"; * cli#512 — `create` refuses when POST /AgentKeyRegister does not store the * generated key. `status`/`refusal` are the endpoint's own response: the HTTP * status and its refusal name, or null when the body named none this client - * knows. A 404 or 405 with no refusal is a Flair without the route. + * knows. A 404 or 405 with no refusal names no response this client recognizes + * as the endpoint, so the route may be unavailable on this Flair. * * Every branch exits non-zero and none falls back to a write the old server * would mishandle; the slice-1 read-back remains the only source of success. @@ -421,31 +422,43 @@ const PENDING_PUBLIC_KEY = "pending"; function refuseKeyRegistration(id: string, flairUrl: string, status: number, refusal: string | null): never { if (refusal === null && (status === 404 || status === 405)) { console.error( - `❌ Agent '${id}' is not registered in Flair at ${flairUrl} — this Flair has no POST /AgentKeyRegister (${status}), so a key generated here cannot be stored.`, + `❌ Agent '${id}' is not registered in Flair at ${flairUrl} — POST /AgentKeyRegister answered ${status} with no refusal this client recognizes, so a key generated here was not stored; this Flair may not serve the route.`, ); console.error( - ` Remedy: upgrade this Flair to ${MIN_FLAIR_VERSION_FOR_KEY_REGISTER} or newer, then re-run \`tps agent create --id ${id}\`.`, + ` Remedy: if this Flair predates the route, upgrade it to ${MIN_FLAIR_VERSION_FOR_KEY_REGISTER} or newer, then re-run \`tps agent create --id ${id}\`.`, ); process.exit(1); } const remedies: Record = { "authentication required": "the admin credential is empty or invalid; set FLAIR_ADMIN_AUTH to this Flair's admin credential", forbidden: "the credential is not this Flair's admin; set FLAIR_ADMIN_AUTH to its admin credential", - registration_request_invalid: "the generated agent id or public key was refused as malformed", + registration_request_invalid: "the agent id or public key was refused as malformed", agent_not_found: `no Agent row exists for '${id}'; create the agent's row on this Flair instance, then re-run \`tps agent create --id ${id}\``, agent_not_local: `'${id}' did not originate on this instance; register its key on the agent's home instance`, agent_inactive: `'${id}' is deactivated; reactivate it before registering a key`, - agent_key_not_pending: `'${id}' already holds a key; replacing it is the key-rotation path, not create`, - registration_conflict: "another write to this agent is in progress; re-run `tps agent create`", - registration_unavailable: "the registration store was unavailable or the committed key did not read back; re-run `tps agent create`", + agent_key_not_pending: `the stored key for '${id}' is not 'pending', so this operation does not apply; replacing a key is the key-rotation path, not create`, + registration_conflict: "another write to this agent is in progress or already committed; re-run `tps agent create`", + registration_unavailable: "the registration did not complete (its transaction failed or its committed key did not read back); re-run `tps agent create`", }; const name = refusal ?? `no refusal named (${status})`; - const remedy = refusal !== null ? remedies[refusal] : "the response named no refusal this client knows; check that Flair is reachable and its version"; + const remedy = refusal !== null ? remedies[refusal] : "the response named no refusal this client recognizes; check that Flair serves this route and its version"; console.error(`❌ Agent '${id}' is not registered in Flair at ${flairUrl} — POST /AgentKeyRegister refused the registration (${name}).`); console.error(` Remedy: ${remedy}.`); process.exit(1); } +/** + * cli#512 — the POST /AgentKeyRegister call produced no endpoint response: a + * transport failure (status 0) or a success body that was not JSON. No refusal + * was received, so this is never reported as a refusal and no HTTP status is + * invented for it; the diagnostic carries the client's own bounded message. + */ +function refuseKeyRegistrationFailure(id: string, flairUrl: string, detail: string): never { + console.error(`❌ Agent '${id}' could not be registered in Flair at ${flairUrl} — POST /AgentKeyRegister did not complete: ${detail}.`); + console.error(` Remedy: check that this Flair is reachable and serves the route, then re-run \`tps agent create --id ${id}\`.`); + process.exit(1); +} + async function createAgent(args: AgentArgs): Promise { const id = args.id; if (!id) { @@ -517,11 +530,11 @@ async function createAgent(args: AgentArgs): Promise { // 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; a `pending` row is completed through - // POST /AgentKeyRegister. The exit decision rests on the read-back; the write - // errors recorded in writeError are reported when it fails. + // cli#512: the Flair registration line is printed 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; a `pending` row is completed + // through POST /AgentKeyRegister. 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(); @@ -535,6 +548,9 @@ async function createAgent(args: AgentArgs): Promise { const existing = await flair.getAgent(id); if (existing) { if (storedKeyAsHex(existing.publicKey ?? "") !== pubKeyHex) { + // cli#512: an existing row whose stored key differs still gets an Agent + // PUT carrying the key; Flair drops `publicKey` on that write, so it + // stores nothing, and the read-back below decides. await flair.updateAgent(id, { publicKey: pubKeyHex }); } successLine = ` Agent '${id}' already registered in Flair.`; @@ -554,8 +570,8 @@ async function createAgent(args: AgentArgs): Promise { }); // cli#512: the seed leaves (or keeps) a row whose stored key is // `pending`; the block after the write registers the generated key - // through Flair's supported operation. Flair's Agent PUT does not - // store `publicKey`, so it is not sent here. + // through Flair's supported operation. This path sends no `publicKey`: + // Flair's Agent PUT does not store it. successLine = ` Agent seeded: ${seeded.soulEntries.length} soul entries, ${seeded.memories.length} memories.`; } catch (seedErr) { // AgentSeed requires admin auth — fall back to direct registration @@ -584,14 +600,15 @@ async function createAgent(args: AgentArgs): Promise { if (stored.found && stored.publicKey === PENDING_PUBLIC_KEY) { // cli#512 — the row is `pending`: register the generated key through // Flair's supported operation, then read again; the slice-1 check below - // decides, so success is printed only on an equal read-back. This covers a - // row seeded earlier and one this run seeded. + // decides, so the Flair registration line is printed only on an equal + // read-back. This covers a row seeded earlier. const publicKeyB64url = Buffer.from(kp.signing.publicKey).toString("base64url"); try { await flair.registerAgentKey(id, publicKeyB64url); } catch (e) { if (e instanceof FlairAgentKeyRegisterError) refuseKeyRegistration(id, flairUrl, e.status, e.refusal); - refuseKeyRegistration(id, flairUrl, 0, null); + const detail = e instanceof FlairRequestError ? e.message : "the request produced no response"; + refuseKeyRegistrationFailure(id, flairUrl, detail); } try { stored = await flair.readStoredPublicKey(id); diff --git a/packages/cli/src/utils/flair-client.ts b/packages/cli/src/utils/flair-client.ts index c41bdf09..9bef929b 100644 --- a/packages/cli/src/utils/flair-client.ts +++ b/packages/cli/src/utils/flair-client.ts @@ -303,8 +303,8 @@ function responseError(entry: string, method: string, path: string, status: numb * The refusal names POST /AgentKeyRegister is documented to answer in its * `error` field (flair#2266): the authentication denials and the registration * refusals. A body naming none of these yields `null` — the response text is - * never echoed, so a route that is not this endpoint (a Flair without it - * answers 404 or 405 with no such field) is recognised as naming no refusal. + * never echoed, so a response that is not this endpoint's refusal names no + * refusal (a Flair without the route answers 404 or 405 with no such field). */ const AGENT_KEY_REGISTER_REFUSALS: readonly string[] = [ "authentication required", @@ -336,8 +336,9 @@ function agentKeyRegisterRefusal(body: string): string | null { /** * A refusal from POST /AgentKeyRegister: the HTTP status and the endpoint's own * refusal name, or null when the response named none this client knows. A 404 - * or 405 with a null refusal is a Flair without the route. The message carries - * the status and the allow-listed name only, never response text. + * or 405 with a null refusal names no response this client recognizes as the + * endpoint, so it is not proof the route is absent. The message carries the + * status and the allow-listed name only, never response text. */ export class FlairAgentKeyRegisterError extends Error { constructor( @@ -620,8 +621,8 @@ export class FlairClient { * operation for a row whose key Flair's Agent PUT and PATCH do not store. A * non-OK response throws a FlairAgentKeyRegisterError carrying the status and * the endpoint's refusal name (null when the body named none this client - * knows, which is how a Flair without the route is recognised). A transport - * failure throws a FlairRequestError (status 0). + * knows, which is how a response this client cannot recognize as this route's + * refusal is named). A transport failure throws a FlairRequestError (status 0). */ async registerAgentKey( agentId: string, diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index 62fd943b..1361c2fd 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -136,6 +136,17 @@ function expectNoKeyBytes(output: string, bytes: Uint8Array): void { } } +/** + * The registration POST happened, and no Agent PUT/PATCH for `id` follows it: the + * refusal branches send no fallback write. The registered request list is the + * server's own record, so a fallback write that lands after the POST shows here. + */ +function expectNoAgentWriteAfterRegistration(requests: Array<{ method: string; path: string }>, id: string): void { + const post = requests.findIndex((req) => req.method === "POST" && req.path === "/AgentKeyRegister"); + expect(post).toBeGreaterThan(-1); + expect(requests.slice(post + 1).filter((req) => (req.method === "PUT" || req.method === "PATCH") && req.path === `/Agent/${id}`)).toEqual([]); +} + 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)); @@ -158,7 +169,7 @@ test("refused write and no Agent row: exits non-zero, prints no success, names a } }); -test("a pending row on a Flair without POST /AgentKeyRegister: exits non-zero naming the minimum version, prints no success", async () => { +test("a pending row and an unrecognized 404 from POST /AgentKeyRegister: exits non-zero naming the minimum version as the likely cause, prints no success", async () => { const id = `${AGENT}-pending-no-route`; const fake = startFakeFlair(async (req, url) => { if (req.method === "PUT") return refusePut(url, id); @@ -170,16 +181,21 @@ test("a pending row on a Flair without POST /AgentKeyRegister: exits non-zero na expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); expect(res.stderr).toContain(`Agent '${id}' is not registered in Flair at ${fake.url}`); - expect(res.stderr).toContain("this Flair has no POST /AgentKeyRegister (404)"); - expect(res.stderr).toContain("upgrade this Flair to 0.62.0 or newer"); - // no fallback write after the missing route: the registration POST is last + // the response is not asserted to have a cause: only that this client cannot + // recognize it as the endpoint's refusal + expect(res.stderr).toContain("POST /AgentKeyRegister answered 404 with no refusal this client recognizes"); + expect(res.stderr).toContain("this Flair may not serve the route"); + expect(res.stderr).toContain("upgrade it to 0.62.0 or newer"); + // no fallback write after the registration POST: the POST is last, and no + // Agent PUT/PATCH follows it + expectNoAgentWriteAfterRegistration(fake.requests, id); expect(fake.requests.at(-1)).toEqual({ method: "POST", path: "/AgentKeyRegister", auth: expect.stringContaining("Basic ") }); } finally { fake.stop(); } }); -test("a 405 on the registration route: exits non-zero naming the minimum version", async () => { +test("a 405 on the registration route: exits non-zero naming the minimum version as the likely cause, and sends no fallback write", async () => { const id = `${AGENT}-pending-method`; const fake = startFakeFlair(async (req, url) => { if (req.method === "PUT") return refusePut(url, id); @@ -191,8 +207,10 @@ test("a 405 on the registration route: exits non-zero naming the minimum version const res = await runCreate(fake.url, id); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain("this Flair has no POST /AgentKeyRegister (405)"); - expect(res.stderr).toContain("upgrade this Flair to 0.62.0 or newer"); + expect(res.stderr).toContain("POST /AgentKeyRegister answered 405 with no refusal this client recognizes"); + expect(res.stderr).toContain("this Flair may not serve the route"); + expect(res.stderr).toContain("upgrade it to 0.62.0 or newer"); + expectNoAgentWriteAfterRegistration(fake.requests, id); } finally { fake.stop(); } @@ -212,14 +230,17 @@ interface RegistrarFlair { * Basic admin, stores the body's publicKey on the row and answers the documented * body on 200, and answers a signed GET for a still-`pending` row as * `unknown_agent` (a pending row cannot authenticate as itself). `route` omits - * the route (404) or answers 405; `refusal` answers a named refusal; `store: - * false` answers 200 without storing (the read-back then differs). + * the route (an unrecognized 404) or answers 405; `refusal` answers a named + * refusal; `store: false` answers 200 without storing (the read-back then + * differs). `route: "transport"` answers a redirect the client follows to a + * closed port, so its fetch fails before any response; `route: "unparseable"` + * answers 200 with a body that is not JSON. */ function startRegistrarFlair( id: string, publicKey: string, opts: { - route?: "serve" | "missing" | "method"; + route?: "serve" | "missing" | "method" | "transport" | "unparseable"; refusal?: { status: number; error: string }; store?: boolean; } = {}, @@ -235,6 +256,11 @@ function startRegistrarFlair( if (req.method === "POST" && url.pathname === "/AgentKeyRegister") { if (route === "missing") return undefined; if (route === "method") return new Response("method not allowed", { status: 405 }); + // A transport failure: the client follows this redirect to a closed port and + // its fetch rejects, so no response body reaches the caller. + if (route === "transport") return new Response(null, { status: 302, headers: { Location: "http://127.0.0.1:1/nope" } }); + // A success status whose body is not JSON: the parse path, not a refusal. + if (route === "unparseable") return new Response("not json", { status: 200 }); let body: Record = {}; try { body = (await req.json()) as Record; @@ -311,20 +337,20 @@ test("the endpoint answers 200 but the row does not read the key back: exits non }); /** A refusal from POST /AgentKeyRegister: the status the endpoint answers and the remedy its refusal names. */ -const KEY_REGISTRATION_REFUSALS: ReadonlyArray<{ status: number; error: string; status404?: boolean; remedy: string }> = [ - { status: 400, error: "registration_request_invalid", remedy: "the generated agent id or public key was refused as malformed" }, +const KEY_REGISTRATION_REFUSALS: ReadonlyArray<{ status: number; error: string; remedy: string }> = [ + { status: 400, error: "registration_request_invalid", remedy: "the agent id or public key was refused as malformed" }, { status: 401, error: "authentication required", remedy: "set FLAIR_ADMIN_AUTH" }, { status: 403, error: "forbidden", remedy: "set FLAIR_ADMIN_AUTH to its admin credential" }, { status: 403, error: "agent_not_local", remedy: "register its key on the agent's home instance" }, { status: 403, error: "agent_inactive", remedy: "reactivate it before registering a key" }, - { status: 404, error: "agent_not_found", status404: true, remedy: "no Agent row exists" }, + { status: 404, error: "agent_not_found", remedy: "no Agent row exists" }, { status: 409, error: "agent_key_not_pending", remedy: "key-rotation path" }, { status: 409, error: "registration_conflict", remedy: "another write to this agent is in progress" }, { status: 503, error: "registration_unavailable", remedy: "re-run `tps agent create`" }, ]; for (const refusal of KEY_REGISTRATION_REFUSALS) { - test(`POST /AgentKeyRegister refusal ${refusal.error}: exits non-zero naming the agent, URL, refusal and remedy`, async () => { + test(`POST /AgentKeyRegister refusal ${refusal.error}: exits non-zero naming the agent, URL, refusal and remedy, and sends no fallback write`, async () => { const id = `${AGENT}-refuse-${refusal.error.replace(/\W/g, "-")}`; const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); @@ -336,14 +362,56 @@ for (const refusal of KEY_REGISTRATION_REFUSALS) { expect(res.stderr).toContain(`Agent '${id}' is not registered in Flair at ${fake.url}`); expect(res.stderr).toContain(`POST /AgentKeyRegister refused the registration (${refusal.error})`); expect(res.stderr).toContain(refusal.remedy); - // agent_not_found is the endpoint's own refusal, not the missing-route case - if (!refusal.status404) expect(res.stderr).not.toContain("this Flair has no POST /AgentKeyRegister"); + // the endpoint's own refusal, never the unrecognized-response case + expect(res.stderr).not.toContain("this Flair may not serve the route"); + // no Agent PUT/PATCH follows the refused registration POST + expectNoAgentWriteAfterRegistration(fake.requests, id); } finally { fake.stop(); } }); } +test("the registration POST fails in transport: exits non-zero with its own diagnostic, never a refusal and no invented status", async () => { + const id = `${AGENT}-register-transport`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { route: "transport" }); + try { + const res = await runCreate(fake.url, id, { keyPair }); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(`Agent '${id}' could not be registered in Flair at ${fake.url}`); + expect(res.stderr).toContain("POST /AgentKeyRegister did not complete"); + expect(res.stderr).toContain("connection refused"); + // not called a refusal, and no status is invented for it + expect(res.stderr).not.toContain("refused the registration"); + expect(res.stderr).not.toContain("no refusal named"); + expectNoAgentWriteAfterRegistration(fake.requests, id); + } finally { + fake.stop(); + } +}); + +test("the registration POST answers 200 with a body that is not JSON: exits non-zero with its own diagnostic, not a refusal", async () => { + const id = `${AGENT}-register-unparseable`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { route: "unparseable" }); + try { + const res = await runCreate(fake.url, id, { keyPair }); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(`Agent '${id}' could not be registered in Flair at ${fake.url}`); + expect(res.stderr).toContain("malformed response body"); + expect(res.stderr).not.toContain("refused the registration"); + expect(res.stderr).not.toContain("no refusal named"); + expectNoAgentWriteAfterRegistration(fake.requests, id); + } 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); @@ -955,7 +1023,7 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( } else { expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain("this Flair has no POST /AgentKeyRegister"); + expect(res.stderr).toContain("this Flair may not serve the route"); expect(res.stderr).toContain("0.62.0"); } for (const stream of [res.stdout, res.stderr, format(res.error)]) { From bcbd84f5ac59641002b6001d9b4cb1a9de00568f Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 21:56:23 +0000 Subject: [PATCH 3/8] fix(agent): report an unconfirmed key registration when Flair cannot confirm (cli#512) A `registration_unavailable` answer, a transport failure or an unreadable success body left the key's fate unknown; `create` now says the registration is unconfirmed rather than naming the agent unregistered, gives a transport failure a distinct diagnostic from a response-parsing failure, and re-reads the key after `registration_unavailable` so an equal read-back still reports the registration. Also: Flair's `Agent` PUT cannot store the key, and the pending claim is conditioned on the route being served and accepted and the key reading back equal. --- .../changed-512-create-registers-key.md | 21 ++++--- packages/cli/src/commands/agent.ts | 59 +++++++++++++------ packages/cli/src/utils/flair-client.ts | 5 +- .../test/agent-create-registration.test.ts | 57 ++++++++++++++---- 4 files changed, 102 insertions(+), 40 deletions(-) diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md index 3bfefeff..90d2b8ed 100644 --- a/.changelog/unreleased/changed-512-create-registers-key.md +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -1,10 +1,13 @@ -- **`tps agent create` completes registration when Flair's row for the agent is `pending`.** - It sends the generated key through `POST /AgentKeyRegister` over its operator - credential and reads the stored key back; the Flair registration line is - printed only when the key reads back equal. Each refusal from the endpoint - exits non-zero with the agent, the Flair URL, the refusal name and a remedy; a - response this client cannot recognize as that endpoint's refusal (404 or 405) - exits non-zero naming the minimum Flair version that serves the route as the - likely cause. Flair drops `publicKey` on `Agent` PUT and PATCH: an existing row +- **`tps agent create` completes a `pending` agent's key registration through + Flair's `POST /AgentKeyRegister`.** It sends the generated key through that + route over its operator credential and reads the stored key back; the Flair + registration line is printed only when Flair serves and accepts the route and + the key reads back equal. Each refusal from the endpoint exits non-zero with + the agent, the Flair URL, the refusal name and a remedy; a response this client + cannot recognize as that endpoint's refusal (404 or 405) exits non-zero naming + the minimum Flair version that serves the route as the likely cause; and a + transport failure, a success body that is not JSON, or a + `registration_unavailable` reports the registration as unconfirmed rather than + unregistered. Flair drops `publicKey` on `Agent` PUT and PATCH: an existing row whose stored key differs still gets an `Agent` PUT carrying the key, which - stores nothing. + cannot store it. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index c3f49eda..2d0b7243 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -274,7 +274,7 @@ function refuseRegistration( 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 drops publicKey on Agent PUT/PATCH; the operation that stores one is POST /AgentKeyRegister (flair#2266).`, + ` Flair drops publicKey on Agent PUT/PATCH; the operation that stores a key on an existing pending row is POST /AgentKeyRegister (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 (rowState === "exists") { @@ -410,14 +410,18 @@ const MIN_FLAIR_VERSION_FOR_KEY_REGISTER = "0.62.0"; const PENDING_PUBLIC_KEY = "pending"; /** - * cli#512 — `create` refuses when POST /AgentKeyRegister does not store the - * generated key. `status`/`refusal` are the endpoint's own response: the HTTP - * status and its refusal name, or null when the body named none this client - * knows. A 404 or 405 with no refusal names no response this client recognizes - * as the endpoint, so the route may be unavailable on this Flair. + * cli#512 — `create` refuses when POST /AgentKeyRegister names a refusal other + * than `registration_unavailable`. `status`/`refusal` are the endpoint's own + * response: the HTTP status and its refusal name, or null when the body named + * none this client knows. A 404 or 405 with no refusal names no response this + * client recognizes as the endpoint, so it is not proof the route is absent — + * the route may be unavailable on this Flair. * * Every branch exits non-zero and none falls back to a write the old server * would mishandle; the slice-1 read-back remains the only source of success. + * `registration_unavailable` does not reach here: Flair returns it both before + * committing (nothing changed) and after committing when its own read-back + * fails, so the caller reports it unconfirmed instead. */ function refuseKeyRegistration(id: string, flairUrl: string, status: number, refusal: string | null): never { if (refusal === null && (status === 404 || status === 405)) { @@ -438,24 +442,32 @@ function refuseKeyRegistration(id: string, flairUrl: string, status: number, ref agent_inactive: `'${id}' is deactivated; reactivate it before registering a key`, agent_key_not_pending: `the stored key for '${id}' is not 'pending', so this operation does not apply; replacing a key is the key-rotation path, not create`, registration_conflict: "another write to this agent is in progress or already committed; re-run `tps agent create`", - registration_unavailable: "the registration did not complete (its transaction failed or its committed key did not read back); re-run `tps agent create`", }; const name = refusal ?? `no refusal named (${status})`; - const remedy = refusal !== null ? remedies[refusal] : "the response named no refusal this client recognizes; check that Flair serves this route and its version"; + const remedy = refusal !== null + ? (remedies[refusal] ?? "the endpoint refused the registration; re-run `tps agent create`") + : "the response named no refusal this client recognizes; check that Flair serves this route and its version"; console.error(`❌ Agent '${id}' is not registered in Flair at ${flairUrl} — POST /AgentKeyRegister refused the registration (${name}).`); console.error(` Remedy: ${remedy}.`); process.exit(1); } /** - * cli#512 — the POST /AgentKeyRegister call produced no endpoint response: a - * transport failure (status 0) or a success body that was not JSON. No refusal - * was received, so this is never reported as a refusal and no HTTP status is - * invented for it; the diagnostic carries the client's own bounded message. + * cli#512 — the POST /AgentKeyRegister call left the generated key's fate + * unknown, so the registration is reported UNCONFIRMED, never as a refusal and + * never as unregistered. The paths: no response arrived (a transport failure), + * a success status carried a body that was not JSON (a response-parsing + * failure), or Flair answered `registration_unavailable` — which Flair returns + * both before committing (nothing changed) and after committing when its own + * read-back fails. The diagnostic + * carries the client's own bounded cause; a refusal name is never invented and, + * for a response that arrived, only its own status travels in the cause. */ -function refuseKeyRegistrationFailure(id: string, flairUrl: string, detail: string): never { - console.error(`❌ Agent '${id}' could not be registered in Flair at ${flairUrl} — POST /AgentKeyRegister did not complete: ${detail}.`); - console.error(` Remedy: check that this Flair is reachable and serves the route, then re-run \`tps agent create --id ${id}\`.`); +function refuseKeyRegistrationUnconfirmed(id: string, flairUrl: string, cause: string): never { + console.error(`❌ Agent '${id}' registration in Flair at ${flairUrl} is unconfirmed — ${cause}.`); + console.error( + ` Remedy: re-run \`tps agent create --id ${id}\`; it reads the stored key back and reports it.`, + ); process.exit(1); } @@ -606,9 +618,20 @@ async function createAgent(args: AgentArgs): Promise { try { await flair.registerAgentKey(id, publicKeyB64url); } catch (e) { - if (e instanceof FlairAgentKeyRegisterError) refuseKeyRegistration(id, flairUrl, e.status, e.refusal); - const detail = e instanceof FlairRequestError ? e.message : "the request produced no response"; - refuseKeyRegistrationFailure(id, flairUrl, detail); + if (e instanceof FlairAgentKeyRegisterError) { + // Flair returns `registration_unavailable` both before committing + // (nothing changed) and after committing when its own read-back + // fails, so the key's fate is unknown here: report it unconfirmed, + // never as a refusal and never as unregistered. + if (e.refusal === "registration_unavailable") { + refuseKeyRegistrationUnconfirmed(id, flairUrl, "POST /AgentKeyRegister answered registration_unavailable, which Flair returns both before committing (nothing changed) and after committing when its own read-back fails, so whether the key is stored is unconfirmed"); + } + refuseKeyRegistration(id, flairUrl, e.status, e.refusal); + } else if (e instanceof FlairRequestError && e.status !== 0) { + refuseKeyRegistrationUnconfirmed(id, flairUrl, `the registration response carried status ${e.status} but no readable JSON body — a response-parsing failure (${e.message})`); + } else { + refuseKeyRegistrationUnconfirmed(id, flairUrl, `the registration request received no response — a transport failure (${e instanceof Error ? e.message : String(e)})`); + } } try { stored = await flair.readStoredPublicKey(id); diff --git a/packages/cli/src/utils/flair-client.ts b/packages/cli/src/utils/flair-client.ts index 9bef929b..394ca369 100644 --- a/packages/cli/src/utils/flair-client.ts +++ b/packages/cli/src/utils/flair-client.ts @@ -303,8 +303,9 @@ function responseError(entry: string, method: string, path: string, status: numb * The refusal names POST /AgentKeyRegister is documented to answer in its * `error` field (flair#2266): the authentication denials and the registration * refusals. A body naming none of these yields `null` — the response text is - * never echoed, so a response that is not this endpoint's refusal names no - * refusal (a Flair without the route answers 404 or 405 with no such field). + * never echoed, so a response this client cannot recognize as this endpoint + * names no refusal, and such a response (a 404 or 405 without the field, say) + * is not proof the route is absent. */ const AGENT_KEY_REGISTER_REFUSALS: readonly string[] = [ "authentication required", diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index 1361c2fd..dfaa232e 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -219,7 +219,7 @@ test("a 405 on the registration route: exits non-zero naming the minimum version interface RegistrarFlair { url: string; stop(): void; - readonly row: { id: string; name: string; publicKey: string }; + readonly row: { id: string; name: string; publicKey: string; updatedAt?: string }; readonly requests: Array<{ method: string; path: string; auth: string }>; readonly registerBodies: Array>; } @@ -234,19 +234,21 @@ interface RegistrarFlair { * refusal; `store: false` answers 200 without storing (the read-back then * differs). `route: "transport"` answers a redirect the client follows to a * closed port, so its fetch fails before any response; `route: "unparseable"` - * answers 200 with a body that is not JSON. + * answers 200 with a body that is not JSON; `route: "unavailable"` commits a + * write to the row and then answers 503 `registration_unavailable` (the status + * Flair returns after committing when its own read-back fails). */ function startRegistrarFlair( id: string, publicKey: string, opts: { - route?: "serve" | "missing" | "method" | "transport" | "unparseable"; + route?: "serve" | "missing" | "method" | "transport" | "unparseable" | "unavailable"; refusal?: { status: number; error: string }; store?: boolean; } = {}, ): RegistrarFlair { const route = opts.route ?? "serve"; - const row = { id, name: id, publicKey: "pending" }; + const row: RegistrarFlair["row"] = { id, name: id, publicKey: "pending" }; const requests: RegistrarFlair["requests"] = []; const registerBodies: Array> = []; let registrations = 0; @@ -268,6 +270,12 @@ function startRegistrarFlair( /* no body */ } registerBodies.push(body); + // The committed write, then the endpoint reports the registration + // unavailable (as Flair does after committing when its read-back fails). + if (route === "unavailable") { + row.publicKey = String(body.publicKey); + return Response.json({ error: "registration_unavailable" }, { status: 503 }); + } if (opts.refusal) return Response.json({ error: opts.refusal.error }, { status: opts.refusal.status }); registrations += 1; if (opts.store === false) { @@ -346,7 +354,6 @@ const KEY_REGISTRATION_REFUSALS: ReadonlyArray<{ status: number; error: string; { status: 404, error: "agent_not_found", remedy: "no Agent row exists" }, { status: 409, error: "agent_key_not_pending", remedy: "key-rotation path" }, { status: 409, error: "registration_conflict", remedy: "another write to this agent is in progress" }, - { status: 503, error: "registration_unavailable", remedy: "re-run `tps agent create`" }, ]; for (const refusal of KEY_REGISTRATION_REFUSALS) { @@ -372,7 +379,7 @@ for (const refusal of KEY_REGISTRATION_REFUSALS) { }); } -test("the registration POST fails in transport: exits non-zero with its own diagnostic, never a refusal and no invented status", async () => { +test("the registration POST receives no response: exits non-zero reporting the registration unconfirmed, never 'not registered' and never a refusal", async () => { const id = `${AGENT}-register-transport`; const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); @@ -381,19 +388,20 @@ test("the registration POST fails in transport: exits non-zero with its own diag const res = await runCreate(fake.url, id, { keyPair }); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain(`Agent '${id}' could not be registered in Flair at ${fake.url}`); - expect(res.stderr).toContain("POST /AgentKeyRegister did not complete"); + expect(res.stderr).toContain(`Agent '${id}' registration in Flair at ${fake.url} is unconfirmed`); + expect(res.stderr).toContain("transport failure"); + // the transport failure's own bounded reason, not a refusal and not the word expect(res.stderr).toContain("connection refused"); - // not called a refusal, and no status is invented for it expect(res.stderr).not.toContain("refused the registration"); expect(res.stderr).not.toContain("no refusal named"); + expect(res.stderr).not.toContain("not registered"); expectNoAgentWriteAfterRegistration(fake.requests, id); } finally { fake.stop(); } }); -test("the registration POST answers 200 with a body that is not JSON: exits non-zero with its own diagnostic, not a refusal", async () => { +test("the registration POST answers 200 with a body that is not JSON: exits non-zero reporting the registration unconfirmed, a parsing failure distinct from a transport one", async () => { const id = `${AGENT}-register-unparseable`; const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); @@ -402,10 +410,37 @@ test("the registration POST answers 200 with a body that is not JSON: exits non- const res = await runCreate(fake.url, id, { keyPair }); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain(`Agent '${id}' could not be registered in Flair at ${fake.url}`); + expect(res.stderr).toContain(`Agent '${id}' registration in Flair at ${fake.url} is unconfirmed`); + expect(res.stderr).toContain("response-parsing failure"); expect(res.stderr).toContain("malformed response body"); expect(res.stderr).not.toContain("refused the registration"); expect(res.stderr).not.toContain("no refusal named"); + expect(res.stderr).not.toContain("not registered"); + expectNoAgentWriteAfterRegistration(fake.requests, id); + } finally { + fake.stop(); + } +}); + +test("a registration_unavailable response after a committed write: exits non-zero reporting the registration unconfirmed, never 'not registered', and the read-back check ran", async () => { + const id = `${AGENT}-register-unavailable`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { route: "unavailable" }); + try { + const res = await runCreate(fake.url, id, { keyPair }); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(`Agent '${id}' registration in Flair at ${fake.url} is unconfirmed`); + expect(res.stderr).toContain("registration_unavailable"); + // Flair returns this after a commit too, so it never names the agent unregistered + expect(res.stderr).not.toContain("not registered"); + expect(res.stderr).not.toContain("refused the registration"); + // the read-back check ran (the operator read of the row before the POST), + // and no fallback write followed the registration POST + const registration = fake.requests.findIndex((req) => req.method === "POST" && req.path === "/AgentKeyRegister"); + expect(registration).toBeGreaterThan(-1); + expect(fake.requests.slice(0, registration).some((req) => req.method === "GET" && req.path === `/Agent/${id}`)).toBe(true); expectNoAgentWriteAfterRegistration(fake.requests, id); } finally { fake.stop(); From 102b519c5f3152de696e3b8d646b89d672d8a169 Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 23:04:59 +0000 Subject: [PATCH 4/8] fix(agent): read the stored key back after an unconfirmed registration (cli#512) --- .../changed-512-create-registers-key.md | 27 ++-- packages/cli/src/commands/agent.ts | 68 +++++---- .../test/agent-create-registration.test.ts | 131 +++++++++++++----- scripts/real-flair-lane.ts | 2 +- 4 files changed, 158 insertions(+), 70 deletions(-) diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md index 90d2b8ed..1cb6bccb 100644 --- a/.changelog/unreleased/changed-512-create-registers-key.md +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -1,13 +1,14 @@ -- **`tps agent create` completes a `pending` agent's key registration through - Flair's `POST /AgentKeyRegister`.** It sends the generated key through that - route over its operator credential and reads the stored key back; the Flair - registration line is printed only when Flair serves and accepts the route and - the key reads back equal. Each refusal from the endpoint exits non-zero with - the agent, the Flair URL, the refusal name and a remedy; a response this client - cannot recognize as that endpoint's refusal (404 or 405) exits non-zero naming - the minimum Flair version that serves the route as the likely cause; and a - transport failure, a success body that is not JSON, or a - `registration_unavailable` reports the registration as unconfirmed rather than - unregistered. Flair drops `publicKey` on `Agent` PUT and PATCH: an existing row - whose stored key differs still gets an `Agent` PUT carrying the key, which - cannot store it. +- **`tps agent create` registers a `pending` agent's key through Flair's `POST + /AgentKeyRegister` when the route accepts it and the stored key reads back + equal.** It sends the generated key through that route over its operator + credential and reads the stored key back; the Flair registration line is + printed only when Flair serves and accepts the route and the key reads back + equal. Each refusal from the endpoint exits non-zero with the agent, the Flair + URL, the refusal name and a remedy; a response this client cannot recognize as + that endpoint's refusal (404 or 405) exits non-zero naming the minimum Flair + version that serves the route as the likely cause; and a transport failure, a + success body that is not JSON, or a `registration_unavailable` is answered by + that read-back: `create` reports success only when the key reads back equal, + and otherwise reports this attempt unconfirmed with the observed state. Flair + drops `publicKey` on `Agent` PUT and PATCH: an existing row whose stored key + differs still gets an `Agent` PUT carrying the key, which cannot store it. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index 2d0b7243..d951ec6b 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -426,7 +426,7 @@ const PENDING_PUBLIC_KEY = "pending"; function refuseKeyRegistration(id: string, flairUrl: string, status: number, refusal: string | null): never { if (refusal === null && (status === 404 || status === 405)) { console.error( - `❌ Agent '${id}' is not registered in Flair at ${flairUrl} — POST /AgentKeyRegister answered ${status} with no refusal this client recognizes, so a key generated here was not stored; this Flair may not serve the route.`, + `❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: POST /AgentKeyRegister answered ${status} with no refusal this client recognizes, so whether a key was stored is unknown; this Flair may not serve the route.`, ); console.error( ` Remedy: if this Flair predates the route, upgrade it to ${MIN_FLAIR_VERSION_FOR_KEY_REGISTER} or newer, then re-run \`tps agent create --id ${id}\`.`, @@ -443,30 +443,32 @@ function refuseKeyRegistration(id: string, flairUrl: string, status: number, ref agent_key_not_pending: `the stored key for '${id}' is not 'pending', so this operation does not apply; replacing a key is the key-rotation path, not create`, registration_conflict: "another write to this agent is in progress or already committed; re-run `tps agent create`", }; - const name = refusal ?? `no refusal named (${status})`; + const name = refusal ?? "no refusal named"; const remedy = refusal !== null ? (remedies[refusal] ?? "the endpoint refused the registration; re-run `tps agent create`") : "the response named no refusal this client recognizes; check that Flair serves this route and its version"; - console.error(`❌ Agent '${id}' is not registered in Flair at ${flairUrl} — POST /AgentKeyRegister refused the registration (${name}).`); + console.error(`❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: POST /AgentKeyRegister refused the registration (${name}) with HTTP ${status}.`); console.error(` Remedy: ${remedy}.`); process.exit(1); } /** * cli#512 — the POST /AgentKeyRegister call left the generated key's fate - * unknown, so the registration is reported UNCONFIRMED, never as a refusal and - * never as unregistered. The paths: no response arrived (a transport failure), - * a success status carried a body that was not JSON (a response-parsing - * failure), or Flair answered `registration_unavailable` — which Flair returns - * both before committing (nothing changed) and after committing when its own - * read-back fails. The diagnostic - * carries the client's own bounded cause; a refusal name is never invented and, - * for a response that arrived, only its own status travels in the cause. + * unknown and the read-back after the POST did not show the key stored, so this + * attempt is reported UNCONFIRMED, never as a refusal and never as unregistered. + * The paths: no response arrived (a transport failure), a success status carried + * a body that was not JSON (a response-parsing failure), or Flair answered + * `registration_unavailable` — which Flair returns both before committing + * (nothing changed) and after committing when its own read-back fails. `cause` + * is the client's own bounded account of the POST request; a refusal name is + * never invented and, for a response that arrived, only its own status travels + * in the cause. `observed` is what the read-back after the POST found. */ -function refuseKeyRegistrationUnconfirmed(id: string, flairUrl: string, cause: string): never { - console.error(`❌ Agent '${id}' registration in Flair at ${flairUrl} is unconfirmed — ${cause}.`); +function refuseKeyRegistrationUnconfirmed(id: string, flairUrl: string, cause: string, observed: string): never { + console.error(`❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: ${cause}.`); + console.error(` The read-back after the POST ${observed}.`); console.error( - ` Remedy: re-run \`tps agent create --id ${id}\`; it reads the stored key back and reports it.`, + ` Remedy: re-run \`tps agent create --id ${id}\`; it registers the key again and reads the stored key back.`, ); process.exit(1); } @@ -562,7 +564,7 @@ async function createAgent(args: AgentArgs): Promise { if (storedKeyAsHex(existing.publicKey ?? "") !== pubKeyHex) { // cli#512: an existing row whose stored key differs still gets an Agent // PUT carrying the key; Flair drops `publicKey` on that write, so it - // stores nothing, and the read-back below decides. + // cannot store the key, and the read-back below decides. await flair.updateAgent(id, { publicKey: pubKeyHex }); } successLine = ` Agent '${id}' already registered in Flair.`; @@ -611,33 +613,51 @@ async function createAgent(args: AgentArgs): Promise { } if (stored.found && stored.publicKey === PENDING_PUBLIC_KEY) { // cli#512 — the row is `pending`: register the generated key through - // Flair's supported operation, then read again; the slice-1 check below - // decides, so the Flair registration line is printed only on an equal - // read-back. This covers a row seeded earlier. + // Flair's supported operation, then read the stored key back after the + // POST on every path. The read-back decides: a key that reads back equal + // is success, and a POST whose response was unconfirmed — a transport + // failure, a success body that is not JSON, or `registration_unavailable` + // — is resolved by what the row now holds. This covers a row seeded + // earlier. const publicKeyB64url = Buffer.from(kp.signing.publicKey).toString("base64url"); + let unconfirmed: string | null = null; try { await flair.registerAgentKey(id, publicKeyB64url); } catch (e) { if (e instanceof FlairAgentKeyRegisterError) { // Flair returns `registration_unavailable` both before committing // (nothing changed) and after committing when its own read-back - // fails, so the key's fate is unknown here: report it unconfirmed, - // never as a refusal and never as unregistered. + // fails, so whether the key is stored is unknown here: the read-back + // below decides, never as a refusal and never as unregistered. if (e.refusal === "registration_unavailable") { - refuseKeyRegistrationUnconfirmed(id, flairUrl, "POST /AgentKeyRegister answered registration_unavailable, which Flair returns both before committing (nothing changed) and after committing when its own read-back fails, so whether the key is stored is unconfirmed"); + unconfirmed = "POST /AgentKeyRegister answered registration_unavailable, which Flair returns both before committing (nothing changed) and after committing when its own read-back fails"; + } else { + refuseKeyRegistration(id, flairUrl, e.status, e.refusal); } - refuseKeyRegistration(id, flairUrl, e.status, e.refusal); } else if (e instanceof FlairRequestError && e.status !== 0) { - refuseKeyRegistrationUnconfirmed(id, flairUrl, `the registration response carried status ${e.status} but no readable JSON body — a response-parsing failure (${e.message})`); + unconfirmed = `the registration response carried status ${e.status} but no readable JSON body — a response-parsing failure (${e.message})`; } else { - refuseKeyRegistrationUnconfirmed(id, flairUrl, `the registration request received no response — a transport failure (${e instanceof Error ? e.message : String(e)})`); + unconfirmed = `the registration request received no response — a transport failure (${e instanceof Error ? e.message : String(e)})`; } } + // The read-back runs after the POST whether or not the response was + // confirmed: only the stored key shows whether this attempt landed. try { stored = await flair.readStoredPublicKey(id); } catch (e) { + if (unconfirmed !== null) { + refuseKeyRegistrationUnconfirmed(id, flairUrl, unconfirmed, `failed (${e instanceof Error ? e.message : String(e)})`); + } refuseRegistration(id, flairUrl, identityDir, `the read-back failed (${e instanceof Error ? e.message : String(e)})`, writeError, "unknown"); } + if (unconfirmed !== null && !(stored.found && storedKeyAsHex(stored.publicKey ?? "") === pubKeyHex)) { + const observed = !stored.found + ? "found no Agent row" + : stored.publicKey === null + ? "found no stored public key" + : `found the stored public key '${stored.publicKey}', not the generated key`; + refuseKeyRegistrationUnconfirmed(id, flairUrl, unconfirmed, observed); + } } if (!stored.found) { refuseRegistration(id, flairUrl, identityDir, "no Agent row exists", writeError, "absent"); diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index dfaa232e..604000cd 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -147,6 +147,13 @@ function expectNoAgentWriteAfterRegistration(requests: Array<{ method: string; p expect(requests.slice(post + 1).filter((req) => (req.method === "PUT" || req.method === "PATCH") && req.path === `/Agent/${id}`)).toEqual([]); } +/** The registration POST happened, and a read-back `GET /Agent/` follows it. */ +function expectReadBackAfterRegistration(requests: Array<{ method: string; path: string }>, id: string): void { + const post = requests.findIndex((req) => req.method === "POST" && req.path === "/AgentKeyRegister"); + expect(post).toBeGreaterThan(-1); + expect(requests.slice(post + 1).some((req) => req.method === "GET" && req.path === `/Agent/${id}`)).toBe(true); +} + 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)); @@ -180,7 +187,8 @@ test("a pending row and an unrecognized 404 from POST /AgentKeyRegister: exits n const res = await runCreate(fake.url, id); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain(`Agent '${id}' is not registered in Flair at ${fake.url}`); + expect(res.stderr).toContain(`Agent '${id}'`); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); // the response is not asserted to have a cause: only that this client cannot // recognize it as the endpoint's refusal expect(res.stderr).toContain("POST /AgentKeyRegister answered 404 with no refusal this client recognizes"); @@ -232,11 +240,12 @@ interface RegistrarFlair { * `unknown_agent` (a pending row cannot authenticate as itself). `route` omits * the route (an unrecognized 404) or answers 405; `refusal` answers a named * refusal; `store: false` answers 200 without storing (the read-back then - * differs). `route: "transport"` answers a redirect the client follows to a - * closed port, so its fetch fails before any response; `route: "unparseable"` - * answers 200 with a body that is not JSON; `route: "unavailable"` commits a - * write to the row and then answers 503 `registration_unavailable` (the status - * Flair returns after committing when its own read-back fails). + * differs). For the three unconfirmed routes — `transport` (a redirect the + * client follows to a closed port, so its fetch fails before any response), + * `unparseable` (200 with a body that is not JSON), and `unavailable` (503 + * `registration_unavailable`, the status Flair returns after committing when + * its own read-back fails) — `commit: true` writes the key to the row before + * the unconfirmed response, so the read-back after the POST reads it equal. */ function startRegistrarFlair( id: string, @@ -245,6 +254,7 @@ function startRegistrarFlair( route?: "serve" | "missing" | "method" | "transport" | "unparseable" | "unavailable"; refusal?: { status: number; error: string }; store?: boolean; + commit?: boolean; } = {}, ): RegistrarFlair { const route = opts.route ?? "serve"; @@ -258,11 +268,6 @@ function startRegistrarFlair( if (req.method === "POST" && url.pathname === "/AgentKeyRegister") { if (route === "missing") return undefined; if (route === "method") return new Response("method not allowed", { status: 405 }); - // A transport failure: the client follows this redirect to a closed port and - // its fetch rejects, so no response body reaches the caller. - if (route === "transport") return new Response(null, { status: 302, headers: { Location: "http://127.0.0.1:1/nope" } }); - // A success status whose body is not JSON: the parse path, not a refusal. - if (route === "unparseable") return new Response("not json", { status: 200 }); let body: Record = {}; try { body = (await req.json()) as Record; @@ -270,10 +275,25 @@ function startRegistrarFlair( /* no body */ } registerBodies.push(body); + // `commit: true` models a write Flair made before the unconfirmed + // response: the row then holds the key although the client cannot confirm + // the response, and the read-back after the POST reads it equal. + const commit = opts.commit === true; + // A transport failure: the client follows this redirect to a closed port and + // its fetch rejects, so no response body reaches the caller. + if (route === "transport") { + if (commit) row.publicKey = String(body.publicKey); + return new Response(null, { status: 302, headers: { Location: "http://127.0.0.1:1/nope" } }); + } + // A success status whose body is not JSON: the parse path, not a refusal. + if (route === "unparseable") { + if (commit) row.publicKey = String(body.publicKey); + return new Response("not json", { status: 200 }); + } // The committed write, then the endpoint reports the registration // unavailable (as Flair does after committing when its read-back fails). if (route === "unavailable") { - row.publicKey = String(body.publicKey); + if (commit) row.publicKey = String(body.publicKey); return Response.json({ error: "registration_unavailable" }, { status: 503 }); } if (opts.refusal) return Response.json({ error: opts.refusal.error }, { status: opts.refusal.status }); @@ -366,7 +386,8 @@ for (const refusal of KEY_REGISTRATION_REFUSALS) { const res = await runCreate(fake.url, id, { keyPair }); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain(`Agent '${id}' is not registered in Flair at ${fake.url}`); + expect(res.stderr).toContain(`Agent '${id}'`); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); expect(res.stderr).toContain(`POST /AgentKeyRegister refused the registration (${refusal.error})`); expect(res.stderr).toContain(refusal.remedy); // the endpoint's own refusal, never the unrecognized-response case @@ -379,8 +400,24 @@ for (const refusal of KEY_REGISTRATION_REFUSALS) { }); } -test("the registration POST receives no response: exits non-zero reporting the registration unconfirmed, never 'not registered' and never a refusal", async () => { - const id = `${AGENT}-register-transport`; +test("a transport failure after the POST was sent, but the key reads back equal: success, and the read-back runs after the POST", async () => { + const id = `${AGENT}-transport-equal`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { route: "transport", commit: true }); + try { + const res = await runCreate(fake.url, id, { keyPair, inspectError: true }); + expect(res.hasError).toBe(false); + expect(res.exitCode).toBeUndefined(); + expect(res.stdout).toContain("registered in Flair"); + expectReadBackAfterRegistration(fake.requests, id); + } finally { + fake.stop(); + } +}); + +test("a transport failure after the POST and a pending read-back: this attempt was not confirmed, naming the observed state", async () => { + const id = `${AGENT}-transport-pending`; const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); const fake = startRegistrarFlair(id, publicKey, { route: "transport" }); @@ -388,21 +425,38 @@ test("the registration POST receives no response: exits non-zero reporting the r const res = await runCreate(fake.url, id, { keyPair }); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain(`Agent '${id}' registration in Flair at ${fake.url} is unconfirmed`); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); expect(res.stderr).toContain("transport failure"); - // the transport failure's own bounded reason, not a refusal and not the word + // the transport failure's own bounded reason, not a refusal name expect(res.stderr).toContain("connection refused"); + expect(res.stderr).toContain("The read-back after the POST found the stored public key 'pending', not the generated key"); expect(res.stderr).not.toContain("refused the registration"); - expect(res.stderr).not.toContain("no refusal named"); expect(res.stderr).not.toContain("not registered"); + expectReadBackAfterRegistration(fake.requests, id); expectNoAgentWriteAfterRegistration(fake.requests, id); } finally { fake.stop(); } }); -test("the registration POST answers 200 with a body that is not JSON: exits non-zero reporting the registration unconfirmed, a parsing failure distinct from a transport one", async () => { - const id = `${AGENT}-register-unparseable`; +test("an unparseable success body, but the key reads back equal: success, and the read-back runs after the POST", async () => { + const id = `${AGENT}-unparseable-equal`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { route: "unparseable", commit: true }); + try { + const res = await runCreate(fake.url, id, { keyPair, inspectError: true }); + expect(res.hasError).toBe(false); + expect(res.exitCode).toBeUndefined(); + expect(res.stdout).toContain("registered in Flair"); + expectReadBackAfterRegistration(fake.requests, id); + } finally { + fake.stop(); + } +}); + +test("an unparseable success body and a pending read-back: this attempt was not confirmed, a parsing failure distinct from a transport one", async () => { + const id = `${AGENT}-unparseable-pending`; const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); const fake = startRegistrarFlair(id, publicKey, { route: "unparseable" }); @@ -410,20 +464,37 @@ test("the registration POST answers 200 with a body that is not JSON: exits non- const res = await runCreate(fake.url, id, { keyPair }); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain(`Agent '${id}' registration in Flair at ${fake.url} is unconfirmed`); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); expect(res.stderr).toContain("response-parsing failure"); expect(res.stderr).toContain("malformed response body"); + expect(res.stderr).toContain("The read-back after the POST found the stored public key 'pending', not the generated key"); expect(res.stderr).not.toContain("refused the registration"); - expect(res.stderr).not.toContain("no refusal named"); expect(res.stderr).not.toContain("not registered"); + expectReadBackAfterRegistration(fake.requests, id); expectNoAgentWriteAfterRegistration(fake.requests, id); } finally { fake.stop(); } }); -test("a registration_unavailable response after a committed write: exits non-zero reporting the registration unconfirmed, never 'not registered', and the read-back check ran", async () => { - const id = `${AGENT}-register-unavailable`; +test("registration_unavailable after a committed write and an equal read-back: success, and the read-back runs after the POST", async () => { + const id = `${AGENT}-unavailable-equal`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { route: "unavailable", commit: true }); + try { + const res = await runCreate(fake.url, id, { keyPair, inspectError: true }); + expect(res.hasError).toBe(false); + expect(res.exitCode).toBeUndefined(); + expect(res.stdout).toContain("registered in Flair"); + expectReadBackAfterRegistration(fake.requests, id); + } finally { + fake.stop(); + } +}); + +test("registration_unavailable without a committed write and a pending read-back: this attempt was not confirmed, naming the observed state", async () => { + const id = `${AGENT}-unavailable-pending`; const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); const fake = startRegistrarFlair(id, publicKey, { route: "unavailable" }); @@ -431,16 +502,12 @@ test("a registration_unavailable response after a committed write: exits non-zer const res = await runCreate(fake.url, id, { keyPair }); expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain(`Agent '${id}' registration in Flair at ${fake.url} is unconfirmed`); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); expect(res.stderr).toContain("registration_unavailable"); - // Flair returns this after a commit too, so it never names the agent unregistered + expect(res.stderr).toContain("The read-back after the POST found the stored public key 'pending', not the generated key"); expect(res.stderr).not.toContain("not registered"); expect(res.stderr).not.toContain("refused the registration"); - // the read-back check ran (the operator read of the row before the POST), - // and no fallback write followed the registration POST - const registration = fake.requests.findIndex((req) => req.method === "POST" && req.path === "/AgentKeyRegister"); - expect(registration).toBeGreaterThan(-1); - expect(fake.requests.slice(0, registration).some((req) => req.method === "GET" && req.path === `/Agent/${id}`)).toBe(true); + expectReadBackAfterRegistration(fake.requests, id); expectNoAgentWriteAfterRegistration(fake.requests, id); } finally { fake.stop(); @@ -1018,7 +1085,7 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( ); test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( - "against a real Flair, create on a pending row completes registration when the instance serves POST /AgentKeyRegister, and refuses otherwise", + "against a real Flair, create on a pending row completes registration when the instance serves POST /AgentKeyRegister and the stored key reads back equal; when the route is absent it refuses naming the minimum version", async () => { const { baseUrl, opsUrl, admin } = flairTarget(); const id = `create-pending-${randomUUID()}`; diff --git a/scripts/real-flair-lane.ts b/scripts/real-flair-lane.ts index 14c60321..8021ad61 100644 --- a/scripts/real-flair-lane.ts +++ b/scripts/real-flair-lane.ts @@ -18,7 +18,7 @@ export const EXPECTED_CASES: Record = { "agent-create-registration.test.ts": [ "against a real Flair, create refuses when the generated key cannot be registered", "against a real Flair, create succeeds with an existing Agent and matching local key", - "against a real Flair, create on a pending row completes registration when the instance serves POST /AgentKeyRegister, and refuses otherwise", + "against a real Flair, create on a pending row completes registration when the instance serves POST /AgentKeyRegister and the stored key reads back equal; when the route is absent it refuses naming the minimum version", ], "mail-flair-read-auth.test.ts": ["real Flair Agent read guards and refused-read mail recovery"], "presence-flair-contract.test.ts": [ From 7489b820171381e116e946947c66074db96f3c82 Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 18:15:30 -0700 Subject: [PATCH 5/8] round: address the review verdict (Codex fix round) Round (Codex), wording only: non-zero claim limited to definitive refusals; post-POST read named for success and the three ambiguous outcomes; seed keeps an existing row; the retry remedy checks the stored key Co-Authored-By: Claude Opus 5.5 --- .../changed-512-create-registers-key.md | 6 ++--- packages/cli/src/commands/agent.ts | 27 +++++++------------ 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md index 1cb6bccb..43e621f3 100644 --- a/.changelog/unreleased/changed-512-create-registers-key.md +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -1,10 +1,8 @@ - **`tps agent create` registers a `pending` agent's key through Flair's `POST /AgentKeyRegister` when the route accepts it and the stored key reads back equal.** It sends the generated key through that route over its operator - credential and reads the stored key back; the Flair registration line is - printed only when Flair serves and accepts the route and the key reads back - equal. Each refusal from the endpoint exits non-zero with the agent, the Flair - URL, the refusal name and a remedy; a response this client cannot recognize as + credential. Definitive refusals exit non-zero with the agent, the Flair URL, + the refusal name and a remedy; a response this client cannot recognize as that endpoint's refusal (404 or 405) exits non-zero naming the minimum Flair version that serves the route as the likely cause; and a transport failure, a success body that is not JSON, or a `registration_unavailable` is answered by diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index d951ec6b..b64fe6ee 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -461,14 +461,13 @@ function refuseKeyRegistration(id: string, flairUrl: string, status: number, ref * `registration_unavailable` — which Flair returns both before committing * (nothing changed) and after committing when its own read-back fails. `cause` * is the client's own bounded account of the POST request; a refusal name is - * never invented and, for a response that arrived, only its own status travels - * in the cause. `observed` is what the read-back after the POST found. + * never invented. `observed` is what the read-back after the POST found. */ function refuseKeyRegistrationUnconfirmed(id: string, flairUrl: string, cause: string, observed: string): never { console.error(`❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: ${cause}.`); console.error(` The read-back after the POST ${observed}.`); console.error( - ` Remedy: re-run \`tps agent create --id ${id}\`; it registers the key again and reads the stored key back.`, + ` Remedy: re-run \`tps agent create --id ${id}\`; it retries registration if the row is still pending and checks the stored key.`, ); process.exit(1); } @@ -547,8 +546,7 @@ async function createAgent(args: AgentArgs): Promise { // cli#512: the Flair registration line is printed 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; a `pending` row is completed - // through POST /AgentKeyRegister. The exit decision rests on the read-back; - // the write errors recorded in writeError are reported when it fails. + // through POST /AgentKeyRegister. const flair = createFlairClient(id, flairUrl, keyPath); const online = await flair.ping(); @@ -582,10 +580,8 @@ async function createAgent(args: AgentArgs): Promise { soulTemplate, starterMemories, }); - // cli#512: the seed leaves (or keeps) a row whose stored key is - // `pending`; the block after the write registers the generated key - // through Flair's supported operation. This path sends no `publicKey`: - // Flair's Agent PUT does not store it. + // cli#512: the seed creates a `pending` row if absent; an existing + // row stays unchanged. The read below determines the stored key. successLine = ` Agent seeded: ${seeded.soulEntries.length} soul entries, ${seeded.memories.length} memories.`; } catch (seedErr) { // AgentSeed requires admin auth — fall back to direct registration @@ -612,13 +608,9 @@ async function createAgent(args: AgentArgs): Promise { refuseRegistration(id, flairUrl, identityDir, `the read-back failed (${e instanceof Error ? e.message : String(e)})`, writeError, "unknown"); } if (stored.found && stored.publicKey === PENDING_PUBLIC_KEY) { - // cli#512 — the row is `pending`: register the generated key through - // Flair's supported operation, then read the stored key back after the - // POST on every path. The read-back decides: a key that reads back equal - // is success, and a POST whose response was unconfirmed — a transport - // failure, a success body that is not JSON, or `registration_unavailable` - // — is resolved by what the row now holds. This covers a row seeded - // earlier. + // cli#512 — register the `pending` row's key, then read it back after + // success, a transport failure, a success body that is not JSON, or + // `registration_unavailable`. Definitive refusals exit before that read. const publicKeyB64url = Buffer.from(kp.signing.publicKey).toString("base64url"); let unconfirmed: string | null = null; try { @@ -640,8 +632,7 @@ async function createAgent(args: AgentArgs): Promise { unconfirmed = `the registration request received no response — a transport failure (${e instanceof Error ? e.message : String(e)})`; } } - // The read-back runs after the POST whether or not the response was - // confirmed: only the stored key shows whether this attempt landed. + // Read after success or an ambiguous outcome; definitive refusals exited above. try { stored = await flair.readStoredPublicKey(id); } catch (e) { From 6a16efacdb9ce63131f7140ce1e1930209aa57f4 Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 18:33:44 -0700 Subject: [PATCH 6/8] round: address the review verdict (Codex fix round) Round (Codex): the Flair fake matches the real auth boundary (admin credential checked; pending-key refusal); an unrecognized or unreadable error response is unconfirmed and keeps its status Co-Authored-By: Claude Opus 5.5 --- .../changed-512-create-registers-key.md | 5 +- packages/cli/src/commands/agent.ts | 27 ++-- packages/cli/src/utils/flair-client.ts | 24 +--- .../test/agent-create-registration.test.ts | 123 +++++++++++++----- 4 files changed, 105 insertions(+), 74 deletions(-) diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md index 43e621f3..d25862c9 100644 --- a/.changelog/unreleased/changed-512-create-registers-key.md +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -2,8 +2,9 @@ /AgentKeyRegister` when the route accepts it and the stored key reads back equal.** It sends the generated key through that route over its operator credential. Definitive refusals exit non-zero with the agent, the Flair URL, - the refusal name and a remedy; a response this client cannot recognize as - that endpoint's refusal (404 or 405) exits non-zero naming the minimum Flair + the refusal name and a remedy; an error response with no recognized refusal, + or an unreadable error body, remains unconfirmed with its HTTP status; an + unrecognized 404 or 405 names the minimum Flair version that serves the route as the likely cause; and a transport failure, a success body that is not JSON, or a `registration_unavailable` is answered by that read-back: `create` reports success only when the key reads back equal, diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index b64fe6ee..2cb88356 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -409,20 +409,7 @@ const MIN_FLAIR_VERSION_FOR_KEY_REGISTER = "0.62.0"; /** Flair's stored `publicKey` value for an Agent row seeded but not yet registered (flair#2266). */ const PENDING_PUBLIC_KEY = "pending"; -/** - * cli#512 — `create` refuses when POST /AgentKeyRegister names a refusal other - * than `registration_unavailable`. `status`/`refusal` are the endpoint's own - * response: the HTTP status and its refusal name, or null when the body named - * none this client knows. A 404 or 405 with no refusal names no response this - * client recognizes as the endpoint, so it is not proof the route is absent — - * the route may be unavailable on this Flair. - * - * Every branch exits non-zero and none falls back to a write the old server - * would mishandle; the slice-1 read-back remains the only source of success. - * `registration_unavailable` does not reach here: Flair returns it both before - * committing (nothing changed) and after committing when its own read-back - * fails, so the caller reports it unconfirmed instead. - */ +/** `refusal` is null when the body is unreadable or names no recognized refusal. */ function refuseKeyRegistration(id: string, flairUrl: string, status: number, refusal: string | null): never { if (refusal === null && (status === 404 || status === 405)) { console.error( @@ -443,11 +430,13 @@ function refuseKeyRegistration(id: string, flairUrl: string, status: number, ref agent_key_not_pending: `the stored key for '${id}' is not 'pending', so this operation does not apply; replacing a key is the key-rotation path, not create`, registration_conflict: "another write to this agent is in progress or already committed; re-run `tps agent create`", }; - const name = refusal ?? "no refusal named"; - const remedy = refusal !== null - ? (remedies[refusal] ?? "the endpoint refused the registration; re-run `tps agent create`") - : "the response named no refusal this client recognizes; check that Flair serves this route and its version"; - console.error(`❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: POST /AgentKeyRegister refused the registration (${name}) with HTTP ${status}.`); + if (refusal === null) { + console.error(`❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: POST /AgentKeyRegister answered HTTP ${status} with no refusal this client recognizes; whether a key was stored is unknown.`); + console.error(" Remedy: check that Flair serves this route and its version."); + process.exit(1); + } + const remedy = remedies[refusal] ?? "the endpoint refused the registration; re-run `tps agent create`"; + console.error(`❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: POST /AgentKeyRegister refused the registration (${refusal}) with HTTP ${status}.`); console.error(` Remedy: ${remedy}.`); process.exit(1); } diff --git a/packages/cli/src/utils/flair-client.ts b/packages/cli/src/utils/flair-client.ts index 394ca369..e1bd2bfc 100644 --- a/packages/cli/src/utils/flair-client.ts +++ b/packages/cli/src/utils/flair-client.ts @@ -299,14 +299,7 @@ function responseError(entry: string, method: string, path: string, status: numb return `Flair ${entry} ${method} ${path} → ${status}: ${code}`; } -/** - * The refusal names POST /AgentKeyRegister is documented to answer in its - * `error` field (flair#2266): the authentication denials and the registration - * refusals. A body naming none of these yields `null` — the response text is - * never echoed, so a response this client cannot recognize as this endpoint - * names no refusal, and such a response (a 404 or 405 without the field, say) - * is not proof the route is absent. - */ +/** Recognized POST /AgentKeyRegister refusal names. */ const AGENT_KEY_REGISTER_REFUSALS: readonly string[] = [ "authentication required", "forbidden", @@ -322,7 +315,7 @@ const AGENT_KEY_REGISTER_REFUSALS: readonly string[] = [ /** Membership — module-private, so nothing outside this module can change how a refusal is named. */ const AGENT_KEY_REGISTER_REFUSAL_SET: ReadonlySet = new Set(AGENT_KEY_REGISTER_REFUSALS); -/** The refusal name on a POST /AgentKeyRegister response, or null when the body named none this client knows. */ +/** Parse a recognized refusal name from the body. */ function agentKeyRegisterRefusal(body: string): string | null { let parsed: unknown; try { @@ -334,13 +327,7 @@ function agentKeyRegisterRefusal(body: string): string | null { return typeof code === "string" && AGENT_KEY_REGISTER_REFUSAL_SET.has(code) ? code : null; } -/** - * A refusal from POST /AgentKeyRegister: the HTTP status and the endpoint's own - * refusal name, or null when the response named none this client knows. A 404 - * or 405 with a null refusal names no response this client recognizes as the - * endpoint, so it is not proof the route is absent. The message carries the - * status and the allow-listed name only, never response text. - */ +/** An HTTP error from POST /AgentKeyRegister; null means an unreadable body or no recognized refusal. */ export class FlairAgentKeyRegisterError extends Error { constructor( public readonly status: number, @@ -621,9 +608,8 @@ export class FlairClient { * operator (Basic admin) credential. cli#512: this is Flair's supported * operation for a row whose key Flair's Agent PUT and PATCH do not store. A * non-OK response throws a FlairAgentKeyRegisterError carrying the status and - * the endpoint's refusal name (null when the body named none this client - * knows, which is how a response this client cannot recognize as this route's - * refusal is named). A transport failure throws a FlairRequestError (status 0). + * the endpoint's refusal name (null for an unreadable body or no recognized + * refusal). A transport failure throws a FlairRequestError (status 0). */ async registerAgentKey( agentId: string, diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index 604000cd..618695c8 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -4,10 +4,7 @@ const patchShared = createPatchShared(); * cli#512 — `tps agent create` reports registration only when Flair reads the * local key back equal. cli#607 — and only when the local key pair is * consistent and Flair accepts a request signed with the private key. 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. Every fixture's Agent PUT obeys Flair: it never + * tests drive the create path against a fake Flair. Every fixture's Agent PUT obeys Flair: it never * stores `publicKey` (see `refusePut`). */ import { afterEach, expect, mock, spyOn, test } from "bun:test"; @@ -19,9 +16,11 @@ import { format } from "node:util"; import { runAgent } from "../src/commands/agent.js"; import { generateKeyPair, saveKeyPair } from "../src/utils/identity.js"; import { flairTarget } from "./helpers/scratch-flair.js"; -import { stubFlairHandler } from "./helpers/stub-flair.js"; +import { requestSigner, stubFlairHandler } from "./helpers/stub-flair.js"; const AGENT = "create-registration-agent"; +const ADMIN_AUTH = "admin:admin123"; +const ADMIN_HEADER = `Basic ${Buffer.from(ADMIN_AUTH).toString("base64")}`; // cli#555: a file that names `spyOn` carries the top-level teardown; each // runCreate restores its own spies in a finally as well. @@ -230,27 +229,14 @@ interface RegistrarFlair { readonly row: { id: string; name: string; publicKey: string; updatedAt?: string }; readonly requests: Array<{ method: string; path: string; auth: string }>; readonly registerBodies: Array>; + fetch(req: Request): Response | Promise; } -/** - * A fake Flair that serves POST /AgentKeyRegister over Basic admin for a - * `pending` Agent row, modeled on flair resources/AgentKeyRegister.ts: it admits - * Basic admin, stores the body's publicKey on the row and answers the documented - * body on 200, and answers a signed GET for a still-`pending` row as - * `unknown_agent` (a pending row cannot authenticate as itself). `route` omits - * the route (an unrecognized 404) or answers 405; `refusal` answers a named - * refusal; `store: false` answers 200 without storing (the read-back then - * differs). For the three unconfirmed routes — `transport` (a redirect the - * client follows to a closed port, so its fetch fails before any response), - * `unparseable` (200 with a body that is not JSON), and `unavailable` (503 - * `registration_unavailable`, the status Flair returns after committing when - * its own read-back fails) — `commit: true` writes the key to the row before - * the unconfirmed response, so the read-back after the POST reads it equal. - */ function startRegistrarFlair( id: string, publicKey: string, opts: { + fetchOnly?: boolean; route?: "serve" | "missing" | "method" | "transport" | "unparseable" | "unavailable"; refusal?: { status: number; error: string }; store?: boolean; @@ -263,8 +249,6 @@ function startRegistrarFlair( const registerBodies: Array> = []; let registrations = 0; const routes = async (req: Request, url: URL): Promise => { - const auth = req.headers.get("Authorization") ?? ""; - const signed = auth.startsWith("TPS-Ed25519 "); if (req.method === "POST" && url.pathname === "/AgentKeyRegister") { if (route === "missing") return undefined; if (route === "method") return new Response("method not allowed", { status: 405 }); @@ -305,25 +289,83 @@ function startRegistrarFlair( return Response.json({ agentId: id, publicKey: row.publicKey, updatedAt: new Date().toISOString(), registrationId: `reg-${registrations}` }); } if (req.method === "GET" && url.pathname === `/Agent/${id}`) { - if (signed && row.publicKey === "pending") return Response.json({ error: "unknown_agent" }, { status: 401 }); return Response.json({ ...row }); } if (req.method === "PUT" || req.method === "PATCH") return refusePut(url, id); return undefined; }; const signedStub = stubFlairHandler({ [id]: publicKey }, routes); - 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 routes(req, url)) ?? new Response("not found", { status: 404 }); - return signedStub(req); - }, + const pendingStub = stubFlairHandler({ [id]: "pending" }, routes); + const handler = async (req: Request): Promise => { + 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 ")) { + if (auth !== ADMIN_HEADER) return Response.json({ error: "authentication required" }, { status: 401 }); + return (await routes(req, url)) ?? new Response("not found", { status: 404 }); + } + return (row.publicKey === "pending" ? pendingStub : signedStub)(req); + }; + const server = opts.fetchOnly ? undefined : Bun.serve({ port: 0, fetch: handler }); + return { url: server ? `http://127.0.0.1:${server.port}` : "http://flair.test", stop: () => server?.stop(true), row, requests, registerBodies, fetch: handler }; +} + +test("the registrar rejects other Basic credentials and a signed pending row", async () => { + const id = `${AGENT}-auth-boundary`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { fetchOnly: true }); + try { + const path = `/Agent/${id}`; + const admin = await fake.fetch(new Request(`${fake.url}${path}`, { headers: { Authorization: ADMIN_HEADER } })); + expect(admin.status).toBe(200); + expect((await admin.json()).publicKey).toBe("pending"); + const wrong = await fake.fetch(new Request(`${fake.url}/AgentKeyRegister`, { + method: "POST", + headers: { Authorization: `Basic ${Buffer.from("admin:wrong").toString("base64")}`, "Content-Type": "application/json" }, + body: JSON.stringify({ agentId: id, publicKey }), + })); + expect(wrong.status).toBe(401); + expect(await wrong.json()).toEqual({ error: "authentication required" }); + expect(fake.registerBodies).toEqual([]); + const sign = requestSigner(id, Buffer.from(keyPair.signing.privateKey)); + const pending = await fake.fetch(new Request(`${fake.url}${path}`, { headers: { Authorization: sign("GET", path) } })); + expect(pending.status).toBe(401); + expect(await pending.json()).toEqual({ error: "signature_verification_failed" }); + } finally { + fake.stop(); + } +}); + +for (const body of ["unrecognized", "unreadable"] as const) { + test(`an ${body} registration error body leaves the outcome unconfirmed with its HTTP status`, async () => { + const id = `${AGENT}-${body}-error`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { fetchOnly: true }); + const restore = patchShared(globalThis, "fetch", (async (input, init) => { + const req = new Request(input, init); + if (req.method === "POST" && new URL(req.url).pathname === "/AgentKeyRegister") { + const response = Response.json({ error: "unexpected_server_error" }, { status: 502 }); + if (body === "unreadable") response.text = async () => { throw new Error("body read failed"); }; + return response; + } + return fake.fetch(req); + }) as typeof fetch); + try { + const res = await runCreate(fake.url, id, { keyPair, credential: ADMIN_AUTH }); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain("was not confirmed"); + expect(res.stderr).toContain("POST /AgentKeyRegister answered HTTP 502"); + expect(res.stderr).toContain("whether a key was stored is unknown"); + expect(res.stderr).not.toContain("refused the registration"); + } finally { + restore(); + fake.stop(); + } }); - return { url: `http://127.0.0.1:${server.port}`, stop: () => server.stop(true), row, requests, registerBodies }; } test("a pending row and a Flair that serves POST /AgentKeyRegister: the generated key is registered and read back equal, then success", async () => { @@ -1105,6 +1147,19 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); try { + const path = `/Agent/${id}`; + const adminRead = await fetch(`${baseUrl}${path}`, { headers }); + expect(adminRead.status).toBe(200); + expect((await adminRead.json()).publicKey).toBe("pending"); + const wrong = await fetch(`${baseUrl}${path}`, { + headers: { Authorization: `Basic ${Buffer.from("admin:wrong").toString("base64")}` }, + }); + expect(wrong.ok).toBe(false); + const sign = requestSigner(id, Buffer.from(keyPair.signing.privateKey)); + const pending = await fetch(`${baseUrl}${path}`, { headers: { Authorization: sign("GET", path) } }); + expect(pending.status).toBe(401); + expect((await pending.json()).error).toBe("signature_verification_failed"); + // Whether this Flair serves POST /AgentKeyRegister: a missing route answers // 404, a present one refuses an empty body 400. Measured once, before the run. const probe = await fetch(`${baseUrl}/AgentKeyRegister`, { From 04d214e5bca4f6f01978b829eea49f7221e2182a Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 19:53:32 -0700 Subject: [PATCH 7/8] round: address the review verdict (Codex fix round) CodeRabbit round (Codex): a read-back failure after a successful or ambiguous registration POST is reported unconfirmed with the re-run remedy, not the remove remedy Co-Authored-By: Claude Opus 5.5 --- .../changed-512-create-registers-key.md | 3 +- packages/cli/src/commands/agent.ts | 19 ++++------ .../test/agent-create-registration.test.ts | 36 +++++++++++++++++++ 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md index d25862c9..685ed841 100644 --- a/.changelog/unreleased/changed-512-create-registers-key.md +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -9,5 +9,4 @@ success body that is not JSON, or a `registration_unavailable` is answered by that read-back: `create` reports success only when the key reads back equal, and otherwise reports this attempt unconfirmed with the observed state. Flair - drops `publicKey` on `Agent` PUT and PATCH: an existing row whose stored key - differs still gets an `Agent` PUT carrying the key, which cannot store it. + drops `publicKey` on `Agent` PUT and PATCH. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index 2cb88356..ac274e5e 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -445,11 +445,7 @@ function refuseKeyRegistration(id: string, flairUrl: string, status: number, ref * cli#512 — the POST /AgentKeyRegister call left the generated key's fate * unknown and the read-back after the POST did not show the key stored, so this * attempt is reported UNCONFIRMED, never as a refusal and never as unregistered. - * The paths: no response arrived (a transport failure), a success status carried - * a body that was not JSON (a response-parsing failure), or Flair answered - * `registration_unavailable` — which Flair returns both before committing - * (nothing changed) and after committing when its own read-back fails. `cause` - * is the client's own bounded account of the POST request; a refusal name is + * `cause` is the client's own bounded account of the POST request; a refusal name is * never invented. `observed` is what the read-back after the POST found. */ function refuseKeyRegistrationUnconfirmed(id: string, flairUrl: string, cause: string, observed: string): never { @@ -549,9 +545,6 @@ async function createAgent(args: AgentArgs): Promise { const existing = await flair.getAgent(id); if (existing) { if (storedKeyAsHex(existing.publicKey ?? "") !== pubKeyHex) { - // cli#512: an existing row whose stored key differs still gets an Agent - // PUT carrying the key; Flair drops `publicKey` on that write, so it - // cannot store the key, and the read-back below decides. await flair.updateAgent(id, { publicKey: pubKeyHex }); } successLine = ` Agent '${id}' already registered in Flair.`; @@ -625,10 +618,12 @@ async function createAgent(args: AgentArgs): Promise { try { stored = await flair.readStoredPublicKey(id); } catch (e) { - if (unconfirmed !== null) { - refuseKeyRegistrationUnconfirmed(id, flairUrl, unconfirmed, `failed (${e instanceof Error ? e.message : String(e)})`); - } - refuseRegistration(id, flairUrl, identityDir, `the read-back failed (${e instanceof Error ? e.message : String(e)})`, writeError, "unknown"); + refuseKeyRegistrationUnconfirmed( + id, + flairUrl, + unconfirmed ?? "POST /AgentKeyRegister succeeded", + `failed (${e instanceof Error ? e.message : String(e)})`, + ); } if (unconfirmed !== null && !(stored.found && storedKeyAsHex(stored.publicKey ?? "") === pubKeyHex)) { const observed = !stored.found diff --git a/packages/cli/test/agent-create-registration.test.ts b/packages/cli/test/agent-create-registration.test.ts index 618695c8..1b960fc3 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -391,6 +391,42 @@ test("a pending row and a Flair that serves POST /AgentKeyRegister: the generate } }); +for (const route of ["serve", "unavailable"] as const) { + test(`a ${route === "serve" ? "successful" : "registration_unavailable"} POST followed by a read-back exception: unconfirmed with a re-run remedy and no removal advice`, async () => { + const id = `${AGENT}-${route}-read-fail`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { fetchOnly: true, route, commit: true }); + const restore = patchShared(globalThis, "fetch", (async (input, init) => { + const req = new Request(input, init); + const response = await fake.fetch(req); + if (req.method === "GET" && new URL(req.url).pathname === `/Agent/${id}` && fake.registerBodies.length > 0) { + throw new Error("post-registration read failed"); + } + return response; + }) as typeof fetch); + try { + const res = await runCreate(fake.url, id, { keyPair, credential: ADMIN_AUTH }); + expect(fake.registerBodies).toEqual([{ agentId: id, publicKey }]); + expect(fake.row.publicKey).toBe(publicKey); + expectReadBackAfterRegistration(fake.requests, id); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); + expect(res.stderr).toContain(route === "serve" ? "POST /AgentKeyRegister succeeded" : "POST /AgentKeyRegister answered registration_unavailable"); + expect(res.stderr).toContain("The read-back after the POST failed ("); + expect(res.stderr).toContain("transport error: network failure"); + expect(res.stderr).toContain(`Remedy: re-run \`tps agent create --id ${id}\``); + expect(res.stderr).not.toContain("flair agent remove"); + expect(res.stderr).not.toContain("is not registered in Flair"); + expectNoAgentWriteAfterRegistration(fake.requests, id); + } finally { + restore(); + fake.stop(); + } + }); +} + test("the endpoint answers 200 but the row does not read the key back: exits non-zero, prints no success", async () => { const id = `${AGENT}-accepted-unstored`; const keyPair = generateKeyPair(); From 66a4cf7d8ee53b0f461f40760d817f4897a35506 Mon Sep 17 00:00:00 2001 From: flint Date: Fri, 16 Oct 2026 20:20:30 -0700 Subject: [PATCH 8/8] round: address the review verdict (Codex fix round) Round (Codex): every post-POST unequal or failed read-back reports unconfirmed (no remove advice on any post-POST path); the fake refuses a non-admin registration POST; removal wording says tries Co-Authored-By: Claude Opus 5.5 --- .../changed-512-create-registers-key.md | 7 +- ...ed-512-create-reports-real-registration.md | 2 +- packages/cli/src/commands/agent.ts | 24 ++-- .../test/agent-create-registration.test.ts | 133 ++++++++++++++---- 4 files changed, 126 insertions(+), 40 deletions(-) diff --git a/.changelog/unreleased/changed-512-create-registers-key.md b/.changelog/unreleased/changed-512-create-registers-key.md index 685ed841..f4182ca7 100644 --- a/.changelog/unreleased/changed-512-create-registers-key.md +++ b/.changelog/unreleased/changed-512-create-registers-key.md @@ -5,8 +5,7 @@ the refusal name and a remedy; an error response with no recognized refusal, or an unreadable error body, remains unconfirmed with its HTTP status; an unrecognized 404 or 405 names the minimum Flair - version that serves the route as the likely cause; and a transport failure, a - success body that is not JSON, or a `registration_unavailable` is answered by - that read-back: `create` reports success only when the key reads back equal, - and otherwise reports this attempt unconfirmed with the observed state. Flair + version that serves the route as the likely cause. After a successful or + ambiguous POST, a read-back that fails or does not return the generated key + reports this attempt unconfirmed with a re-run-create remedy. Flair drops `publicKey` on `Agent` PUT and PATCH. diff --git a/.changelog/unreleased/fixed-512-create-reports-real-registration.md b/.changelog/unreleased/fixed-512-create-reports-real-registration.md index cee05f2e..b15df982 100644 --- a/.changelog/unreleased/fixed-512-create-reports-real-registration.md +++ b/.changelog/unreleased/fixed-512-create-reports-real-registration.md @@ -1 +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. +- **When Flair is reachable, `tps agent create` reports registration only after the generated key reads back equal.** It re-reads the agent's stored `publicKey` over the operator credential; 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 ac274e5e..ba716b5f 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -282,11 +282,11 @@ function refuseRegistration( ` 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 uses POST /AgentKeyRegister (flair#2266).`, + ` ⚠️ \`flair agent remove\` tries to delete the agent's Agent, Memory and Soul rows and its key files in the keys dir (unless --keep-keys); it can stop on a failure.`, ); } 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}.`, + ` Until then: if \`flair agent show ${id}\` finds the agent, it already exists: run \`flair agent remove ${id}\` on the Flair host first (this tries to delete that agent's Memory and Soul rows and can stop on a failure), then ${copy}.`, ); console.error(` Otherwise: ${copy}.`); } else { @@ -401,8 +401,6 @@ function refuseUnloadableIdentity(id: string, identityDir: string, err: unknown) /** * cli#512 — the first Flair release that serves POST /AgentKeyRegister, the * operation `create` uses to store a locally generated key on a `pending` row. - * The route merged into Flair main after its 0.61.0 entry, so no published - * release has it (the newest published is 0.59.0). */ const MIN_FLAIR_VERSION_FOR_KEY_REGISTER = "0.62.0"; @@ -442,11 +440,9 @@ function refuseKeyRegistration(id: string, flairUrl: string, status: number, ref } /** - * cli#512 — the POST /AgentKeyRegister call left the generated key's fate - * unknown and the read-back after the POST did not show the key stored, so this - * attempt is reported UNCONFIRMED, never as a refusal and never as unregistered. + * cli#512 — a read-back after POST /AgentKeyRegister did not confirm the key. * `cause` is the client's own bounded account of the POST request; a refusal name is - * never invented. `observed` is what the read-back after the POST found. + * never invented. `observed` is the read-back result or failure. */ function refuseKeyRegistrationUnconfirmed(id: string, flairUrl: string, cause: string, observed: string): never { console.error(`❌ Agent '${id}' — this attempt to register a key in Flair at ${flairUrl} was not confirmed: ${cause}.`); @@ -581,6 +577,7 @@ async function createAgent(args: AgentArgs): Promise { writeError = writeError ? `${writeError}; ${msg}` : msg; } + let registrationPostCause: string | null = null; let stored: { found: boolean; publicKey: string | null }; try { stored = await flair.readStoredPublicKey(id); @@ -625,13 +622,14 @@ async function createAgent(args: AgentArgs): Promise { `failed (${e instanceof Error ? e.message : String(e)})`, ); } - if (unconfirmed !== null && !(stored.found && storedKeyAsHex(stored.publicKey ?? "") === pubKeyHex)) { + registrationPostCause = unconfirmed ?? "POST /AgentKeyRegister succeeded"; + if (!(stored.found && storedKeyAsHex(stored.publicKey ?? "") === pubKeyHex)) { const observed = !stored.found ? "found no Agent row" : stored.publicKey === null ? "found no stored public key" : `found the stored public key '${stored.publicKey}', not the generated key`; - refuseKeyRegistrationUnconfirmed(id, flairUrl, unconfirmed, observed); + refuseKeyRegistrationUnconfirmed(id, flairUrl, registrationPostCause, observed); } } if (!stored.found) { @@ -650,9 +648,15 @@ async function createAgent(args: AgentArgs): Promise { try { confirmed = await flair.request<{ publicKey?: string }>("GET", `/Agent/${encodeURIComponent(id)}`); } catch (e) { + if (registrationPostCause !== null) { + refuseKeyRegistrationUnconfirmed(id, flairUrl, registrationPostCause, `failed on the signed request (${e instanceof Error ? e.message : String(e)})`); + } refuseRegistration(id, flairUrl, identityDir, `a request signed with the private key was refused (${e instanceof Error ? e.message : String(e)})`, writeError, "exists"); } if (storedKeyAsHex(confirmed?.publicKey ?? "") !== pubKeyHex) { + if (registrationPostCause !== null) { + refuseKeyRegistrationUnconfirmed(id, flairUrl, registrationPostCause, "did not confirm the registered key on the signed request"); + } refuseRegistration(id, flairUrl, identityDir, "the signed request was accepted but its response did not confirm the registered 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 1b960fc3..0d330f04 100644 --- a/packages/cli/test/agent-create-registration.test.ts +++ b/packages/cli/test/agent-create-registration.test.ts @@ -8,7 +8,7 @@ const patchShared = createPatchShared(); * stores `publicKey` (see `refusePut`). */ import { afterEach, expect, mock, spyOn, test } from "bun:test"; -import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, readlinkSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, lstatSync, mkdirSync, readFileSync, readlinkSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; import { createHash, createPrivateKey, createPublicKey, randomBytes, randomUUID } from "node:crypto"; import { homedir } from "node:os"; import { join } from "node:path"; @@ -294,8 +294,12 @@ function startRegistrarFlair( if (req.method === "PUT" || req.method === "PATCH") return refusePut(url, id); return undefined; }; - const signedStub = stubFlairHandler({ [id]: publicKey }, routes); - const pendingStub = stubFlairHandler({ [id]: "pending" }, routes); + const signedRoutes = (req: Request, url: URL) => + req.method === "POST" && url.pathname === "/AgentKeyRegister" + ? Response.json({ error: "forbidden" }, { status: 403 }) + : routes(req, url); + const signedStub = stubFlairHandler({ [id]: publicKey }, signedRoutes); + const pendingStub = stubFlairHandler({ [id]: "pending" }, signedRoutes); const handler = async (req: Request): Promise => { const url = new URL(req.url); if (url.pathname === "/Health") return new Response("ok"); @@ -311,7 +315,7 @@ function startRegistrarFlair( return { url: server ? `http://127.0.0.1:${server.port}` : "http://flair.test", stop: () => server?.stop(true), row, requests, registerBodies, fetch: handler }; } -test("the registrar rejects other Basic credentials and a signed pending row", async () => { +test("the registrar rejects other Basic credentials, a signed pending GET and a non-admin signed registration POST", async () => { const id = `${AGENT}-auth-boundary`; const keyPair = generateKeyPair(); const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); @@ -333,6 +337,23 @@ test("the registrar rejects other Basic credentials and a signed pending row", a const pending = await fake.fetch(new Request(`${fake.url}${path}`, { headers: { Authorization: sign("GET", path) } })); expect(pending.status).toBe(401); expect(await pending.json()).toEqual({ error: "signature_verification_failed" }); + expect(fake.row.publicKey).toBe("pending"); + const register = await fake.fetch(new Request(`${fake.url}/AgentKeyRegister`, { + method: "POST", + headers: { Authorization: ADMIN_HEADER, "Content-Type": "application/json" }, + body: JSON.stringify({ agentId: id, publicKey }), + })); + expect(register.status).toBe(200); + expect(fake.row.publicKey).toBe(publicKey); + const signedRegister = await fake.fetch(new Request(`${fake.url}/AgentKeyRegister`, { + method: "POST", + headers: { Authorization: sign("POST", "/AgentKeyRegister"), "Content-Type": "application/json" }, + body: JSON.stringify({ agentId: id, publicKey: Buffer.alloc(32, 7).toString("base64url") }), + })); + expect(signedRegister.status).toBe(403); + expect(await signedRegister.json()).toEqual({ error: "forbidden" }); + expect(fake.row.publicKey).toBe(publicKey); + expect(fake.registerBodies).toEqual([{ agentId: id, publicKey }]); } finally { fake.stop(); } @@ -427,20 +448,73 @@ for (const route of ["serve", "unavailable"] as const) { }); } -test("the endpoint answers 200 but the row does not read the key back: exits non-zero, prints no success", async () => { - const id = `${AGENT}-accepted-unstored`; - const keyPair = generateKeyPair(); - const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); - const fake = startRegistrarFlair(id, publicKey, { store: false }); - try { - const res = await runCreate(fake.url, id, { keyPair }); - expect(res.exitCode).toBe(1); - expect(res.stdout).not.toContain("registered in Flair"); - expect(res.stderr).toContain("the stored public key is 'pending', not the generated key"); - } finally { - fake.stop(); - } -}); +for (const observed of ["pending", "different", "absent", "no-key"] as const) { + test(`a successful POST followed by a ${observed} read-back: unconfirmed with a re-run remedy and no removal advice`, async () => { + const id = `${AGENT}-accepted-${observed}`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { fetchOnly: true }); + const storedKey = observed === "pending" ? "pending" : "de".repeat(32); + const restore = patchShared(globalThis, "fetch", (async (input, init) => { + const req = new Request(input, init); + const response = await fake.fetch(req); + if (req.method === "GET" && new URL(req.url).pathname === `/Agent/${id}` && fake.registerBodies.length > 0) { + if (observed === "absent") return new Response("not found", { status: 404 }); + return Response.json({ id, name: id, ...(observed === "no-key" ? {} : { publicKey: storedKey }) }); + } + return response; + }) as typeof fetch); + try { + const res = await runCreate(fake.url, id, { keyPair, credential: ADMIN_AUTH }); + expect(fake.registerBodies).toEqual([{ agentId: id, publicKey }]); + expectReadBackAfterRegistration(fake.requests, id); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); + expect(res.stderr).toContain("POST /AgentKeyRegister succeeded"); + expect(res.stderr).toContain(observed === "absent" ? "found no Agent row" : observed === "no-key" ? "found no stored public key" : `found the stored public key '${storedKey}', not the generated key`); + expect(res.stderr).toContain(`Remedy: re-run \`tps agent create --id ${id}\``); + expect(res.stderr).not.toContain("flair agent remove"); + expect(res.stderr).not.toContain("is not registered in Flair"); + expectNoAgentWriteAfterRegistration(fake.requests, id); + } finally { + restore(); + fake.stop(); + } + }); +} + +for (const observed of ["failure", "different", "absent"] as const) { + test(`a successful POST and equal admin read-back followed by a signed ${observed}: unconfirmed with no removal advice`, async () => { + const id = `${AGENT}-signed-${observed}`; + const keyPair = generateKeyPair(); + const publicKey = Buffer.from(keyPair.signing.publicKey).toString("base64url"); + const fake = startRegistrarFlair(id, publicKey, { fetchOnly: true }); + const restore = patchShared(globalThis, "fetch", (async (input, init) => { + const req = new Request(input, init); + const response = await fake.fetch(req); + if (req.method === "GET" && new URL(req.url).pathname === `/Agent/${id}` && fake.registerBodies.length > 0 && req.headers.get("Authorization")?.startsWith("TPS-Ed25519 ")) { + if (observed === "failure") throw new Error("signed read failed"); + if (observed === "absent") return new Response("not found", { status: 404 }); + return Response.json({ id, name: id, publicKey: "de".repeat(32) }); + } + return response; + }) as typeof fetch); + try { + const res = await runCreate(fake.url, id, { keyPair, credential: ADMIN_AUTH }); + expect(fake.registerBodies).toEqual([{ agentId: id, publicKey }]); + expect(fake.requests.at(-1)?.auth).toStartWith("TPS-Ed25519 "); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain("registered in Flair"); + expect(res.stderr).toContain(`this attempt to register a key in Flair at ${fake.url} was not confirmed`); + expect(res.stderr).toContain(`Remedy: re-run \`tps agent create --id ${id}\``); + expect(res.stderr).not.toContain("flair agent remove"); + } finally { + restore(); + fake.stop(); + } + }); +} /** A refusal from POST /AgentKeyRegister: the status the endpoint answers and the remedy its refusal names. */ const KEY_REGISTRATION_REFUSALS: ReadonlyArray<{ status: number; error: string; remedy: string }> = [ @@ -790,13 +864,14 @@ for (const suffix of [".seed", ".key"]) { }); } -// chmod 000 does not stop root from reading, so the case cannot be built as root. -test.skipIf(process.getuid?.() === 0)("an existing private key file that cannot be read: a named refusal before any Flair request, not a throw", async () => { +test("a directory at the private key path: refusal before any Flair request, not a throw", async () => { const id = `${AGENT}-unreadable-key`; const keyPair = generateKeyPair(); - const fake = startFakeFlair(async (req, url) => (req.method === "PUT" ? refusePut(url, id) : undefined)); + const fake = startRegistrarFlair(id, Buffer.from(keyPair.signing.publicKey).toString("base64url"), { fetchOnly: true }); + const restore = patchShared(globalThis, "fetch", (async (input, init) => fake.fetch(new Request(input, init))) as typeof fetch); const { keyPath } = writeIdentity(id, keyPair); - chmodSync(keyPath, 0o000); + unlinkSync(keyPath); + mkdirSync(keyPath); try { const res = await runCreate(fake.url, id, { inspectError: true }); expect(res.hasError).toBe(false); @@ -804,13 +879,12 @@ test.skipIf(process.getuid?.() === 0)("an existing private key file that cannot expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered"); expect(res.stderr).toContain(`Agent '${id}' cannot load its identity at`); - expect(res.stderr).toContain(`${keyPath} cannot be read (EACCES)`); - expect(res.stderr).toContain("make that file readable by the user running tps"); + expect(res.stderr).toMatch(/cannot be read \(EISDIR\)|loading the identity files failed/); expect(res.stderr).toContain(`tps agent create --id ${id}`); expectNoKeyBytes(res.stdout + res.stderr, keyPair.signing.privateKey); expect(fake.requests).toEqual([]); } finally { - chmodSync(keyPath, 0o600); + restore(); fake.stop(); } }); @@ -1213,6 +1287,15 @@ test.skipIf(process.env.TPS_TEST_REAL_FLAIR !== "1")( const record = await fetch(`${baseUrl}/Agent/${id}`, { headers }); expect(record.ok).toBe(true); expect((await record.json() as { publicKey: string }).publicKey).toBe(publicKey); + const signedRegister = await fetch(`${baseUrl}/AgentKeyRegister`, { + method: "POST", + headers: { Authorization: sign("POST", "/AgentKeyRegister"), "Content-Type": "application/json" }, + body: JSON.stringify({ agentId: id, publicKey: Buffer.alloc(32, 7).toString("base64url") }), + }); + expect(signedRegister.status).toBe(403); + const unchanged = await fetch(`${baseUrl}${path}`, { headers }); + expect(unchanged.status).toBe(200); + expect((await unchanged.json()).publicKey).toBe(publicKey); } else { expect(res.exitCode).toBe(1); expect(res.stdout).not.toContain("registered in Flair");