From 69b216f9bf9cf6983679d8bc5c98ee3a5f24dcae Mon Sep 17 00:00:00 2001 From: Corey Quinn Date: Fri, 26 Jun 2026 15:06:43 -0700 Subject: [PATCH] feat(facts): make update --text optional and add facts confirm CLI `facts update` no longer forces --text. When --text is omitted it reuses the existing fetchText path (the same one the MCP surface uses): the current fact text is fetched and echoed back so only the provided fields change. This lets `bee facts update --confirmed true` flip the confirmed flag without a manual get-then-update dance and without risking a text rewrite. Add `bee facts confirm ` as sugar for the common confirm-and-preserve-text case. It is CLI-only: bee_update_fact already lets MCP callers set confirmed without supplying text, so a separate MCP tool would churn the tool snapshot for no new capability. Update USAGE, the top-of-file divergence comment, tests, and the README facts section to match. --- README.md | 3 +- sources/resources/facts/index.test.ts | 70 +++++++++++++++++++++++-- sources/resources/facts/index.ts | 73 ++++++++++++++++++++++----- 3 files changed, 128 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 8481f8c..bdcd3e4 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,8 @@ By default, data commands return markdown. Use `--json` to print raw JSON. - `facts list` - List facts. Options: `--limit N`, `--cursor `, `--unconfirmed`, `--json`. - `facts get ` - Get a specific fact. Options: `--json`. - `facts create --text ` - Create a new fact. Options: `--json`. - - `facts update --text ` - Update a fact. Options: `--confirmed `, `--json`. + - `facts update ` - Update a fact. Options: `--text ` (when omitted, the existing text is preserved), `--confirmed `, `--json`. + - `facts confirm ` - Confirm a fact (sets `confirmed=true`) while preserving its existing text. Options: `--json`. - `facts delete ` - Delete a fact. Options: `--json`. - `facts search --query ` - Search saved facts. Options: `--limit N`, `--json`. diff --git a/sources/resources/facts/index.test.ts b/sources/resources/facts/index.test.ts index 9e71aaa..2fa4c0e 100644 --- a/sources/resources/facts/index.test.ts +++ b/sources/resources/facts/index.test.ts @@ -275,9 +275,31 @@ describe("facts command (registry-derived)", () => { await expectError(factsCommand.run(["update", "--text", "x"], ctx), "Missing fact id."); }); - it("rejects update without --text", async () => { - const ctx = proxyContext(() => Response.json({})); - await expectError(factsCommand.run(["update", "4"], ctx), "Missing fact text. Provide --text."); + it("updates --confirmed without --text by fetching and preserving existing text", async () => { + let getCalls = 0; + let putBody: unknown; + const ctx = proxyContext(async (request) => { + const url = new URL(request.url); + expect(url.pathname).toBe("/v1/facts/4"); + if (request.method === "GET") { + getCalls += 1; + return Response.json({ id: 4, text: "Stored", tags: [], created_at: 0, confirmed: false }); + } + expect(request.method).toBe("PUT"); + putBody = await request.json(); + return Response.json({ id: 4, text: "Stored", tags: [], created_at: 0, confirmed: true }); + }); + const { logs, restore } = captureLogs(); + try { + await factsCommand.run(["update", "4", "--confirmed", "true"], ctx); + } finally { + restore(); + } + // The existing text is fetched (GET) and echoed back in the PUT so it is not + // rewritten; only the provided field changes. + expect(getCalls).toBe(1); + expect(putBody).toEqual({ text: "Stored", confirmed: true }); + expect(logs.join("\n")).toContain("- text: Stored"); }); it("rejects update with bad --confirmed", async () => { @@ -288,6 +310,48 @@ describe("facts command (registry-derived)", () => { ); }); + // ---- confirm -------------------------------------------------------------- + + it("confirm fetches existing text and PUTs confirmed=true without rewriting text", async () => { + let getCalls = 0; + let putBody: unknown; + const ctx = proxyContext(async (request) => { + const url = new URL(request.url); + expect(url.pathname).toBe("/v1/facts/7"); + if (request.method === "GET") { + getCalls += 1; + return Response.json({ id: 7, text: "Keep me", tags: [], created_at: 0, confirmed: false }); + } + expect(request.method).toBe("PUT"); + putBody = await request.json(); + return Response.json({ id: 7, text: "Keep me", tags: [], created_at: 0, confirmed: true }); + }); + const { logs, restore } = captureLogs(); + try { + await factsCommand.run(["confirm", "7"], ctx); + } finally { + restore(); + } + expect(getCalls).toBe(1); + expect(putBody).toEqual({ text: "Keep me", confirmed: true }); + expect(logs.join("\n")).toContain("- confirmed: true"); + }); + + it("rejects confirm without an id", async () => { + const ctx = proxyContext(() => Response.json({})); + await expectError(factsCommand.run(["confirm"], ctx), "Missing fact id."); + }); + + it("rejects a non-numeric id on confirm", async () => { + const ctx = proxyContext(() => Response.json({})); + await expectError(factsCommand.run(["confirm", "abc"], ctx), "Fact id must be a positive integer."); + }); + + it("rejects extra positionals on confirm", async () => { + const ctx = proxyContext(() => Response.json({})); + await expectError(factsCommand.run(["confirm", "1", "2"], ctx), "Unexpected arguments: 1 2"); + }); + // ---- delete --------------------------------------------------------------- it("deletes a fact via DELETE", async () => { diff --git a/sources/resources/facts/index.ts b/sources/resources/facts/index.ts index 701f757..bb18c0a 100644 --- a/sources/resources/facts/index.ts +++ b/sources/resources/facts/index.ts @@ -7,8 +7,11 @@ // confirmed when includeUnconfirmed is true. // - facts search and bee_search_facts both query the BM25 facts index via // POST /v1/search/conversations with filter=facts. -// - bee_update_fact (MCP) fetches the existing text when text is omitted; the -// CLI `facts update` requires --text and never fetches. +// - bee_update_fact (MCP) and the CLI `facts update` both fetch the existing +// text when text is omitted, reusing the same fetchText path so a bare +// `facts update --confirmed true` preserves the stored text. The CLI +// `facts confirm ` is sugar over that path (sets confirmed=true) and is +// CLI-only: bee_update_fact already covers the confirm-by-flag case for MCP. import { printJson } from "@/client/clientApi"; import { printToolData } from "@/commands/mcpToolOutput"; import type { JsonObject } from "@/mcp/types"; @@ -42,7 +45,8 @@ const USAGE = [ "bee facts search --query [--limit N] [--json]", "bee facts get [--json]", "bee facts create --text [--json]", - "bee facts update --text [--confirmed ] [--json]", + "bee facts update [--text ] [--confirmed ] [--json]", + "bee facts confirm [--json]", "bee facts delete [--json]", ].join("\n"); @@ -419,9 +423,9 @@ const createFact: ActionDefinition = { // ---- update (= bee_update_fact) -------------------------------------------- -// `text` is optional only on the MCP surface: when absent, run() fetches the -// existing fact text. The CLI always supplies text. fetchText distinguishes -// "no text provided" from an empty string. +// `text` is optional on both surfaces: when absent, run() fetches the existing +// fact text via the fetchText path so only the provided fields (e.g. confirmed) +// change. fetchText distinguishes "no text provided" from an empty string. type FactUpdateInput = { id: number; text: string | undefined; @@ -458,17 +462,19 @@ const updateFact: ActionDefinition = { }, coerceInput: (raw, surface) => { if (surface === "cli") { - // CLI `facts update` validation order: missing id -> missing text -> bad id - // (numeric) check. --text is required (no fetch like MCP). The shared parser - // already emitted "Unexpected arguments:" for >1 positionals. + // CLI `facts update` validation order: missing id -> bad id (numeric) check. + // The shared parser already emitted "Unexpected arguments:" for >1 + // positionals. --text is now optional: when omitted we reuse the fetchText + // path (same as MCP) so only the provided fields (e.g. --confirmed) change + // and the stored text is preserved. if (raw["id"] === undefined) { throw new Error("Missing fact id."); } - if (coerceOptionalString(raw["text"]) === undefined) { - throw new Error("Missing fact text. Provide --text."); - } const id = coerceCliFactId(raw["id"]); - const input: FactUpdateInput = { id, text: stringArg(raw["text"], "text"), fetchText: false }; + const hasText = coerceOptionalString(raw["text"]) !== undefined; + const input: FactUpdateInput = hasText + ? { id, text: stringArg(raw["text"], "text"), fetchText: false } + : { id, text: undefined, fetchText: true }; if (typeof raw["confirmed"] === "boolean") { input.confirmed = raw["confirmed"]; } @@ -509,6 +515,37 @@ const updateFact: ActionDefinition = { }, }; +// ---- confirm (CLI only) ----------------------------------------------------- + +// Sugar for the common "flip confirmed on, leave the text alone" case so callers +// don't hand-write `update --confirmed true`. CLI-only: bee_update_fact +// already lets MCP callers set confirmed without supplying text, so adding a +// separate MCP tool would only churn the tool snapshot for no new capability. +// Reuses updateFact.run by producing the same fetchText input with confirmed +// pinned to true. +type FactConfirmInput = FactUpdateInput; + +const confirmFact: ActionDefinition = { + cli: { + subcommand: "confirm", + positionals: [{ name: "id", required: false }], + flags: [], + render: (result, format) => { + if (result.kind !== "json") { + return; + } + renderFactDocument(result.data, format); + }, + }, + coerceInput: (raw) => ({ + id: coerceCliFactId(raw["id"]), + text: undefined, + fetchText: true, + confirmed: true, + }), + run: updateFact.run, +}; + // ---- delete (= bee_delete_fact) -------------------------------------------- const deleteFact: ActionDefinition = { @@ -548,5 +585,13 @@ export const factsResource: ResourceModule = { missingSubcommandMessage: "Missing subcommand. Use list.", unknownSubcommandPrefix: "Unknown facts subcommand: ", }, - actions: [listFacts, searchFacts, getFact, createFact, updateFact, deleteFact], + actions: [ + listFacts, + searchFacts, + getFact, + createFact, + updateFact, + confirmFact, + deleteFact, + ], };