From de139d907ff0ec31e8daa8dba62ab08296966edc Mon Sep 17 00:00:00 2001 From: Anton Arapov Date: Fri, 29 May 2026 10:06:51 +0200 Subject: [PATCH] feat(tags): add delete_tag_definition + document track/tag probe findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the one genuinely-buildable gap from the post-v1.6.6 connector audit, and records why the other headline gap isn't buildable. Both gated on endpoint existence, settled by scripts/wire-trace-v167.ts before any code was written: - GET /tracks → 405 Method not allowed. No tenant-wide track- instance list exists (the route only accepts POST/create). Orphan tracks (NOTES §25) stay reachable only by known id, so a `list_tracks` tool is NOT buildable — declined on evidence, not assumption. Documented in NOTES §33. - DELETE //tags/{id} → 204, verified gone tenant-wide on a follow-up read. Tag definitions ARE deletable, so this ships `delete_tag_definition`. delete_tag_definition (write, destructive, confirm-gated): - Schema {entity, tagId, confirm: true}; entity selects the tag namespace (parties/opportunities/kases), same as add_tag/list_tags. - Handler: DELETE //tags/{tagId}, idempotent on 404 (alreadyDeleted), invalidates the list_tags cache, explicit confirm guard for direct callers (mirrors defineDelete). - `delete_` prefix auto-applies destructiveHint via inferAnnotations — no register-tool.ts change needed. - Distinct from remove_tag_by_id (DETACH from one record); this removes the definition from every record that shared it. Catalog 87 → 88 tools; destructive-hinted 7 → 8. 529 → 536 tests (+7). Bundle 167.37 KB stdio / 195.24 KB http. Tool-count + bundle figures synced across README / DEPLOY / DESIGN / HOWTO / bundle-shape canary. scripts/wire-trace-v167.ts committed for re-runnability. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 21 ++++ DEPLOY.md | 2 +- DESIGN.md | 2 +- HOWTO.md | 4 +- NOTES-ON-CAPSULE-API.md | 51 ++++++++ README.md | 4 +- scripts/wire-trace-v167.ts | 216 +++++++++++++++++++++++++++++++++ src/server.ts | 10 ++ src/tools/tags.ts | 50 +++++++- tests/bundle-shape.test.ts | 2 +- tests/mcp-integration.test.ts | 3 +- tests/tags.test.ts | 63 ++++++++++ tests/tool-annotations.test.ts | 16 +-- 13 files changed, 427 insertions(+), 17 deletions(-) create mode 100644 scripts/wire-trace-v167.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a7dfd9..79dd3f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,19 @@ versions adhere to [Semantic Versioning](https://semver.org). ### Added +- **`delete_tag_definition` (write, destructive, confirm-gated).** + Deletes a tag DEFINITION from an entity type's tag namespace + (parties / opportunities / kases), removing it from every record + that shared it — distinct from `remove_tag_by_id`, which only + detaches a tag from one record and leaves the definition intact. + Closes the "stranded test / typo tag definitions can't be cleaned + up through the connector" gap. Endpoint verified empirically + (`scripts/wire-trace-v167.ts`): `DELETE //tags/{id}` → 204, + with a follow-up read confirming tenant-wide removal. Idempotent on + 404; mirrors the other `delete_*` tools' confirm gate and + destructive annotation. Tool catalog 87 → 88; destructive-hinted + tools 7 → 8. + - **`GET /` now serves a small HTML landing page** — a short human-readable body pointing at `/mcp` and the GitHub repo, plus `` and @@ -110,6 +123,14 @@ versions adhere to [Semantic Versioning](https://semver.org). person-partyId no-op. Same `ZZZ-V166-*` test record tagging and full cleanup pattern as v164 / v165. +- **NOTES-ON-CAPSULE-API.md §33 (new section)** records two + endpoint-existence findings from `scripts/wire-trace-v167.ts`: + `GET /tracks` → 405 (no tenant-wide track-instance list, so a + `list_tracks` tool is NOT buildable — orphan tracks per §25 stay + reachable only by known id), and `DELETE //tags/{id}` → 204 + (tag definitions ARE deletable tenant-wide, now exposed as + `delete_tag_definition`). The probe ships for re-runnability. + ## [1.6.5] — 2026-05-25 Patch release on top of v1.6.3. Combines two waves of consistency diff --git a/DEPLOY.md b/DEPLOY.md index 793c934..976b12e 100644 --- a/DEPLOY.md +++ b/DEPLOY.md @@ -202,7 +202,7 @@ In Claude.ai admin → **Settings → Connectors → Custom Connectors → Add c | Client ID | `MCP_OAUTH_CLIENT_ID` you set during deploy | | Client Secret | `MCP_OAUTH_CLIENT_SECRET` you set during deploy | -Save. Anthropic walks the OAuth dance silently; on success the connector page shows the tools (49 if `CAPSULE_MCP_READONLY=1`, 87 if not). +Save. Anthropic walks the OAuth dance silently; on success the connector page shows the tools (49 if `CAPSULE_MCP_READONLY=1`, 88 if not). ## Wire up a shared Project diff --git a/DESIGN.md b/DESIGN.md index ccc23cf..4a8298b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -70,7 +70,7 @@ where the bytes flow; the semantics are identical. ▼ ┌────────────────────────┐ │ src/tools/*.ts │ - │ (87 tools across the │ + │ (88 tools across the │ │ Capsule resource │ │ graph — see README) │ └────────────────────────┘ diff --git a/HOWTO.md b/HOWTO.md index dd89c61..1c70d27 100644 --- a/HOWTO.md +++ b/HOWTO.md @@ -11,7 +11,7 @@ npm install npm test ``` -529 tests, all mocked — no Capsule API calls happen, no token needed. The suite has three layers: +536 tests, all mocked — no Capsule API calls happen, no token needed. The suite has three layers: - **Per-tool unit tests** (e.g. `tests/parties.test.ts`): import the tool function, mock `undici.fetch`, assert on the URL, method, body, and response handling. Most tests live here. - **MCP-protocol integration tests** (`tests/mcp-integration.test.ts`): drive a real `McpServer` through the wire protocol via the SDK's in-memory transport pair, with `undici.fetch` still mocked. Catches the layer between "tool function works" and "MCP correctly registers and dispatches the tool". Includes the `get_attachment` content-type routing logic (which lives in `server.ts`, not the tool function). @@ -46,7 +46,7 @@ for the contributor-facing summary. npm run build ``` -Produces `dist/index.js` (stdio entry, ~165 KB, with `#!/usr/bin/env node` shebang and the executable bit set) and `dist/http.js` (HTTP entry, ~193 KB, no shebang). Each is fully self-contained — tsup runs as two separate configs so the stdio entry can be invoked directly via npx while the HTTP entry isn't a CLI. tsup target is Node 22 (undici 8 requires Node 22+ for the `webidl.util.markAsUncloneable` runtime API). +Produces `dist/index.js` (stdio entry, ~167 KB, with `#!/usr/bin/env node` shebang and the executable bit set) and `dist/http.js` (HTTP entry, ~195 KB, no shebang). Each is fully self-contained — tsup runs as two separate configs so the stdio entry can be invoked directly via npx while the HTTP entry isn't a CLI. tsup target is Node 22 (undici 8 requires Node 22+ for the `webidl.util.markAsUncloneable` runtime API). `npm run build` also chains `npm run build:icon` (`scripts/build-icon.mjs`), which regenerates `src/icon.ts` from the canonical `assets/icon.svg`. The TypeScript file is committed (so typecheck works without a build step) but is **generated** — edit the SVG, then run the build. A drift-guard test (`tests/icon-source.test.ts`) fails CI if the two ever fall out of sync. diff --git a/NOTES-ON-CAPSULE-API.md b/NOTES-ON-CAPSULE-API.md index 8137c64..ade17f2 100644 --- a/NOTES-ON-CAPSULE-API.md +++ b/NOTES-ON-CAPSULE-API.md @@ -1382,6 +1382,57 @@ default-on in callers without conditional logic. --- +## 33. No tenant-wide track-instance list; tag definitions ARE deletable + +Two endpoint-existence questions, settled empirically by +[`scripts/wire-trace-v167.ts`](scripts/wire-trace-v167.ts) so that two +proposed tools could be graded buildable-or-not before any code was +written. + +### `GET /tracks` → 405 — there is NO tenant-wide track-instance list + +`GET /tracks` returns **`405 Method not allowed`** (the route exists +but only accepts `POST`, which creates an instance). Track instances +are therefore reachable only: + +- entity-scoped: `GET //{id}/tracks` (the `list_entity_tracks` + tool), or +- by known id: `GET /tracks/{id}` (the `show_track` tool). + +**Consequence:** the orphan track instances from §25 (which survive +parent deletion) **cannot be enumerated tenant-wide** — if you don't +already hold the orphan's id, there is no read path to it. A proposed +`list_tracks` tool is therefore **not buildable** against Capsule's +v2 API; it was declined on this basis rather than the assumption it +could be added. The only mitigations for orphan accumulation remain +(a) capture the track-instance id at `apply_track` time and +`remove_track` it explicitly before deleting the parent, or (b) clean +up in Capsule's web UI. + +### `DELETE //tags/{id}` → 204 — tag DEFINITIONS are deletable + +A tag definition minted via `add_tag` was removed with +**`DELETE /parties/tags/{id}` → 204`**, and a follow-up +`GET /parties/tags` confirmed it was gone tenant-wide (not merely +detached from one record). Tags are entity-namespaced (separate +`/parties/tags`, `/opportunities/tags`, `/kases/tags` lists), so the +delete path carries the entity prefix. + +This is distinct from the tag DETACH path used by `remove_tag_by_id` +(`PUT //{id}` with `{tags: [{id, _delete: true}]}`), which +removes a tag from ONE record and leaves the definition intact. +Definition-delete removes the definition from the namespace and from +every record that shared it. + +**Where in our code:** [`src/tools/tags.ts`](src/tools/tags.ts) +`deleteTagDefinition` (the `delete_tag_definition` tool, confirm-gated, +idempotent on 404). Verified by `scripts/wire-trace-v167.ts`. + +**No Capsule docs page** documents either behaviour explicitly; +both come from the live probe. + +--- + ## How to add to this file When you discover a new Capsule API quirk: diff --git a/README.md b/README.md index f39e949..e2bd43e 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,10 @@ A [Model Context Protocol](https://modelcontextprotocol.io) server for [Capsule CRM](https://capsulecrm.com). Connect Claude (Desktop, Code, or web Projects via Custom Connector) to your CRM and let it answer natural-language questions across the full record graph: contacts, organisations, opportunities, projects, tasks, and timeline activity. Beyond the basics it covers structured filters with field/operator conditions, saved searches with sort, workflow tracks (templates and instances), file attachments (read + write), audit of deleted records, and batch fetches up to 50 records per call. -- **87 tools** across the Capsule resource graph (49 in read-only mode) — full read coverage plus careful, confirm-gated writes; 6 batched-write tools (`batch_*`) for mass-update workflows +- **88 tools** across the Capsule resource graph (49 in read-only mode) — full read coverage plus careful, confirm-gated writes; 6 batched-write tools (`batch_*`) for mass-update workflows - **Two transports**: stdio for local installs (Claude Desktop / Code), HTTP+OAuth for hosted Custom Connectors - **Read-only mode** as a one-env-var flag; works alongside read-scoped Capsule tokens -- **MCP tool annotations**: 49 read tools carry `readOnlyHint: true`, 7 destructive ones carry `destructiveHint: true` — clients that honor these hints can auto-approve safe reads while still prompting for writes/destructive calls +- **MCP tool annotations**: 49 read tools carry `readOnlyHint: true`, 8 destructive ones carry `destructiveHint: true` — clients that honor these hints can auto-approve safe reads while still prompting for writes/destructive calls - **Apache 2.0** ## Pick your install diff --git a/scripts/wire-trace-v167.ts b/scripts/wire-trace-v167.ts new file mode 100644 index 0000000..8929540 --- /dev/null +++ b/scripts/wire-trace-v167.ts @@ -0,0 +1,216 @@ +/** + * Wire-trace probes for the connector-gap audit (post-v1.6.6). Answers + * the two questions that gate whether the proposed `list_tracks` and + * `delete_tag_definition` tools are even BUILDABLE — i.e. whether + * Capsule's v2 API exposes the upstream endpoints they'd need. The + * connector's own notes only document entity-scoped + by-id track + * reads and tag DETACH (never definition-delete), so both are + * unverified assumptions until probed. + * + * PROBE 1 — tenant-wide track-instance enumeration. + * Does `GET /tracks` list all track instances across the tenant? + * If yes (200 + a `tracks` array), a `list_tracks` tool is + * buildable and orphan track instances (which survive parent + * deletion — NOTES §25) become sweepable. If 404/405, there is + * no read-side path to orphans by-anything-but-known-id, and + * `list_tracks` is NOT buildable. + * + * PROBE 2 — tag-definition deletion. + * `add_tag` auto-creates a tenant-global tag definition; the + * connector can DETACH a tag from an entity (`remove_tag_by_id`) + * but never DELETE the definition. Does Capsule expose a + * definition-delete endpoint? Probes `DELETE /parties/tags/{id}` + * then `DELETE /tags/{id}`. If either 200/204s, a + * `delete_tag_definition` tool is buildable. + * + * CAVEAT: this probe must create a throwaway tag definition to + * attempt deleting it. If NO delete endpoint works, that + * definition is stranded in the tenant (there's no API path to + * remove it — which is precisely the gap being measured). It is + * labelled `ZZZ-V167-DELME-*` so it's trivially findable for + * manual web-UI cleanup. The probe reports whether it stranded. + * + * Pattern mirrors scripts/wire-trace-v164.ts … -v166.ts: ZZZ-V167-* + * labelled test records, full cleanup on exit, no tenant-specific + * strings or IDs committed (everything discovered at runtime). Run + * with: + * + * CAPSULE_API_TOKEN= npx tsx scripts/wire-trace-v167.ts + */ + +import { fetch } from "undici"; + +const TOKEN = process.env["CAPSULE_API_TOKEN"]; +if (!TOKEN) { + console.error("CAPSULE_API_TOKEN env var required (write-scoped)"); + process.exit(1); +} + +const BASE = "https://api.capsulecrm.com/api/v2"; +const HEADERS = { + Authorization: `Bearer ${TOKEN}`, + "Content-Type": "application/json", + Accept: "application/json", +}; + +interface ApiResult { + status: number; + body: unknown; +} + +async function call(method: string, path: string, body?: unknown): Promise { + const res = await fetch(`${BASE}${path}`, { + method, + headers: HEADERS, + body: body !== undefined ? JSON.stringify(body) : undefined, + }); + let parsed: unknown; + try { + parsed = await res.json(); + } catch { + parsed = await res.text().catch(() => null); + } + return { status: res.status, body: parsed }; +} + +/** Compact one-line view of a response body, capped. */ +function bodyPreview(body: unknown): string { + return JSON.stringify(body)?.slice(0, 300) ?? "null"; +} + +/** Pull the array under a top-level key (e.g. `tracks`, `tags`), or []. */ +function arrayUnder(body: unknown, key: string): Array> { + if (body && typeof body === "object" && key in (body as Record)) { + const arr = (body as Record)[key]; + if (Array.isArray(arr)) return arr as Array>; + } + return []; +} + +async function probeTenantWideTracks(): Promise { + console.log("\n========================================="); + console.log("PROBE 1: GET /tracks — is there a tenant-wide track-instance list?"); + console.log("========================================="); + + const res = await call("GET", "/tracks?page=1&perPage=2"); + console.log(` GET /tracks → status ${res.status}`); + if (res.status === 200) { + const tracks = arrayUnder(res.body, "tracks"); + const keys = Object.keys((res.body as Record) ?? {}); + console.log(` top-level keys: ${JSON.stringify(keys)}`); + console.log(` tracks[] present? ${tracks.length > 0 || keys.includes("tracks")}`); + console.log(` → BUILDABLE: a list_tracks tool can enumerate tenant-wide instances.`); + console.log(` sample: ${bodyPreview(res.body)}`); + } else if (res.status === 404 || res.status === 405) { + console.log(` → NOT BUILDABLE: no tenant-wide track-instance endpoint.`); + console.log(` Orphan tracks are reachable only by known id (GET /tracks/{id})`); + console.log(` or entity-scoped (GET //{id}/tracks). list_tracks is moot.`); + console.log(` body: ${bodyPreview(res.body)}`); + } else { + console.log(` → INCONCLUSIVE (unexpected status). body: ${bodyPreview(res.body)}`); + } + + // Contrast: confirm the entity-scoped + definitions endpoints behave + // as the connector already assumes (sanity that the token can read + // tracks at all, so a 404 above is "no endpoint" not "no access"). + const defs = await call("GET", "/tracks/definitions?perPage=1"); + console.log( + ` contrast GET /tracks/definitions → status ${defs.status} (definitions list the connector already uses)`, + ); +} + +async function probeTagDefinitionDelete(): Promise { + console.log("\n========================================="); + console.log("PROBE 2: tag-definition deletion — DELETE /parties/tags/{id} then /tags/{id}"); + console.log("========================================="); + + const tag = `ZZZ-V167-${Date.now()}`; + const tagName = `ZZZ-V167-DELME-${Date.now()}`; + let hostPartyId: number | undefined; + let tagId: number | undefined; + let stranded = true; + + try { + // Host party to attach the throwaway tag to (add_tag's path is a + // PUT on an entity; you can't mint a definition without one). + const host = await call("POST", "/parties", { + party: { type: "organisation", name: `${tag}-TAGHOST` }, + }); + if (host.status !== 201) { + console.log(` setup failed (create host party → ${host.status}); aborting probe 2`); + return; + } + hostPartyId = (host.body as { party: { id: number } }).party.id; + console.log(` host party ${hostPartyId}`); + + // Attach a fresh-named tag → Capsule auto-creates the definition. + // The PUT response does NOT echo the tags array, so read the party + // back with embed=tags to get the tag object (which carries the + // tenant-global definition id — NOTES §20). + await call("PUT", `/parties/${hostPartyId}`, { + party: { tags: [{ name: tagName }] }, + }); + const read = await call("GET", `/parties/${hostPartyId}?embed=tags`); + const tags = arrayUnder((read.body as { party?: unknown })?.party ?? {}, "tags"); + const created = tags.find((t) => t["name"] === tagName); + tagId = typeof created?.["id"] === "number" ? (created["id"] as number) : undefined; + console.log(` created tag definition "${tagName}" → id ${tagId ?? "UNKNOWN"}`); + if (tagId === undefined) { + console.log(` could not resolve new tag id; party+tags body: ${bodyPreview(read.body)}`); + return; + } + + // Candidate delete endpoints, in order. Stop at first 2xx. + const candidates = [`/parties/tags/${tagId}`, `/tags/${tagId}`]; + for (const path of candidates) { + const del = await call("DELETE", path); + console.log(` DELETE ${path} → status ${del.status}`); + if (del.status === 200 || del.status === 204) { + console.log(` → BUILDABLE: delete_tag_definition can use DELETE ${path}`); + stranded = false; + break; + } + if (del.status !== 404 && del.status !== 405) { + console.log(` (non-404/405 body: ${bodyPreview(del.body)})`); + } + } + if (stranded) { + console.log(` → NOT BUILDABLE: no tag-definition delete endpoint responded 2xx.`); + console.log(` Tag definitions can only be removed via Capsule's web UI.`); + } + } finally { + if (hostPartyId !== undefined) { + const d = await call("DELETE", `/parties/${hostPartyId}`); + console.log(` cleanup: delete host party ${hostPartyId} → ${d.status}`); + } + // Verify whether the definition outlived the host party (it will, + // if no delete endpoint worked — definitions are tenant-global, + // independent of any entity). + if (tagId !== undefined) { + const check = await call("GET", "/parties/tags?perPage=100"); + const present = arrayUnder(check.body, "tags").some((t) => t["id"] === tagId); + console.log( + ` post-cleanup: tag definition ${tagId} still in tenant? ${present ? "YES" : "no"}`, + ); + if (present) { + console.log(` ⚠ STRANDED: search Capsule's web UI for "${tagName}" and delete manually.`); + } + } + } +} + +async function main() { + try { + await probeTenantWideTracks(); + await probeTagDefinitionDelete(); + console.log("\nAll probes complete."); + } catch (err) { + console.error("\n!!! probe run crashed:", err); + process.exit(1); + } +} + +main().catch((err) => { + console.error("fatal:", err); + process.exit(1); +}); diff --git a/src/server.ts b/src/server.ts index 6ed57f3..425cdba 100644 --- a/src/server.ts +++ b/src/server.ts @@ -127,6 +127,8 @@ import { addTag, removeTagByIdSchema, removeTagById, + deleteTagDefinitionSchema, + deleteTagDefinition, batchAddTagSchema, batchAddTag, batchRemoveTagByIdSchema, @@ -1097,6 +1099,14 @@ export function createCapsuleMcpServer(opts?: { clientId?: string }): McpServer removeTagById, ); + registerTool( + server, + "delete_tag_definition", + "DESTRUCTIVE & TENANT-WIDE: permanently delete a tag DEFINITION from an entity type's tag namespace (parties / opportunities / kases). Unlike remove_tag_by_id — which detaches a tag from ONE record and leaves the definition intact for others — this removes the definition itself, so the tag disappears from EVERY record that shared it. Use it to clean up stray / mistyped / test tag definitions polluting the tenant-global list. Requires confirm=true. Always read the affected tag first via list_tags and confirm with the user; if you only want to untag one record, use remove_tag_by_id instead. Irreversible (re-creating by name via add_tag mints a brand-new id). Idempotent on retry: `{deleted: true, alreadyDeleted: false, entity, tagId}` on a fresh delete, or `{deleted: true, alreadyDeleted: true, entity, tagId}` if the definition was already gone (Capsule's 404 is caught). Endpoint verified empirically (DELETE //tags/{id} → 204).", + deleteTagDefinitionSchema, + deleteTagDefinition, + ); + registerBatchTool( server, "batch_add_tag", diff --git a/src/tools/tags.ts b/src/tools/tags.ts index 4a5f5a6..d687366 100644 --- a/src/tools/tags.ts +++ b/src/tools/tags.ts @@ -30,11 +30,12 @@ */ import { z } from "zod"; +import { confirmFlag } from "./confirm-flag.js"; import { defineBatch } from "./define-batch.js"; import { positiveId } from "./shared-schemas.js"; -import { capsuleGetCached, capsulePut } from "../capsule/client.js"; +import { capsuleDelete, capsuleGetCached, capsulePut } from "../capsule/client.js"; import { invalidateByPrefix } from "../capsule/cache.js"; -import { idempotentWithResult, isCapsuleTagNotFound } from "../capsule/idempotent.js"; +import { idempotent, idempotentWithResult, isCapsuleTagNotFound } from "../capsule/idempotent.js"; const TAG_LIST_PATH = { parties: "/parties/tags", @@ -138,6 +139,51 @@ export async function removeTagById(input: z.infer) return result; } +// ── delete_tag_definition (write, DESTRUCTIVE) ───────────────────────────── +// +// remove_tag_by_id (above) DETACHES a tag from one entity — the tag +// definition survives in the tenant for every other record that uses +// it. This tool DELETES the definition itself from the tenant: the tag +// disappears from the entity-type's tag namespace and from every +// record that shared it. That blast radius is why it's confirm-gated +// and destructive-hinted (the `delete_` prefix auto-applies the hint). +// +// Endpoint verified empirically (scripts/wire-trace-v167.ts): a fresh +// definition created via add_tag was removed with +// `DELETE /parties/tags/{id}` → 204, and a follow-up read confirmed it +// was gone tenant-wide. Tags are entity-namespaced (separate +// /parties/tags, /opportunities/tags, /kases/tags lists), so the tool +// takes the same `entity` selector as add_tag / list_tags. + +export const deleteTagDefinitionSchema = z.object({ + entity: TagEntity, + tagId: positiveId.describe( + "The tag definition's id (from list_tags, or embed='tags' on a record). NOT an entity id.", + ), + confirm: confirmFlag().describe( + "Must be set to true. DESTRUCTIVE & tenant-wide: permanently deletes the tag DEFINITION from this entity type's tag namespace, removing it from EVERY record that shares it — not just one. To detach a tag from a single record while keeping the definition, use remove_tag_by_id instead. Irreversible (the definition is gone; re-creating by name via add_tag mints a new id). Idempotent on retry.", + ), +}); + +export async function deleteTagDefinition(input: z.infer) { + const { entity, tagId, confirm } = input; + // The schema's confirmFlag() already rejects non-true at the MCP + // validation layer; this guard mirrors defineDelete for callers that + // invoke the handler directly (tests, future internal callers). + if (confirm !== true) { + throw new Error("delete_tag_definition requires confirm: true"); + } + const result = await idempotent( + () => capsuleDelete(`/${entity}/tags/${tagId}`), + () => ({ deleted: true as const, alreadyDeleted: false, entity, tagId }), + () => ({ deleted: true as const, alreadyDeleted: true, entity, tagId }), + ); + // The definition just left the tenant-global list for this entity + // type — drop the cached list so the next list_tags reads fresh. + invalidateByPrefix(TAG_LIST_PATH[entity], "delete_tag_definition"); + return result; +} + // ── batch_add_tag (write, fan-out) ──────────────────────────────────────── export const { schema: batchAddTagSchema, handler: batchAddTag } = defineBatch({ diff --git a/tests/bundle-shape.test.ts b/tests/bundle-shape.test.ts index 36beb54..c1d00b0 100644 --- a/tests/bundle-shape.test.ts +++ b/tests/bundle-shape.test.ts @@ -54,7 +54,7 @@ describe.skipIf(!distExists)("bundle shape (post-build canary)", () => { const httpKb = statSync(HTTP_PATH).size / 1024; // Floor catches "the bundler produced an empty file"; ceiling // catches "we accidentally inlined a giant dependency". The - // current values (~165 / ~193 KB) sit comfortably in the band. + // current values (~167 / ~195 KB) sit comfortably in the band. expect(stdioKb).toBeGreaterThan(MIN_KB); expect(stdioKb).toBeLessThan(MAX_KB); expect(httpKb).toBeGreaterThan(MIN_KB); diff --git a/tests/mcp-integration.test.ts b/tests/mcp-integration.test.ts index 663940e..470b5a8 100644 --- a/tests/mcp-integration.test.ts +++ b/tests/mcp-integration.test.ts @@ -63,7 +63,7 @@ describe("tools/list", () => { // README advertises the exact catalogue size; keep it locked here // so docs drift is caught before release. - expect(tools.length).toBe(87); + expect(tools.length).toBe(88); // Reads we always expect for (const r of [ @@ -106,6 +106,7 @@ describe("tools/list", () => { "remove_additional_party", "add_tag", "remove_tag_by_id", + "delete_tag_definition", ]) { expect(names).toContain(w); } diff --git a/tests/tags.test.ts b/tests/tags.test.ts index 637199d..baff998 100644 --- a/tests/tags.test.ts +++ b/tests/tags.test.ts @@ -118,3 +118,66 @@ describe("removeTagById — atomic detach by per-entity link id", () => { expect(body[wrapper].tags).toEqual([{ id: 42, _delete: true }]); }); }); + +describe("deleteTagDefinition — tenant-wide definition delete (DESTRUCTIVE)", () => { + it.each([ + ["parties", "/parties/tags/42"], + ["opportunities", "/opportunities/tags/42"], + ["kases", "/kases/tags/42"], + ] as const)("DELETEs /%s/tags/{id} when confirm=true", async (entity, expectedPath) => { + mockFetch(204, {}); + const { deleteTagDefinition } = await import("../src/tools/tags.js"); + + const result = await deleteTagDefinition({ entity, tagId: 42, confirm: true }); + + const [url, opts] = vi.mocked(fetch).mock.calls[0]!; + expect(url).toContain(expectedPath); + expect((opts as RequestInit).method).toBe("DELETE"); + expect(result).toEqual({ deleted: true, alreadyDeleted: false, entity, tagId: 42 }); + }); + + it("is idempotent: Capsule 404 → alreadyDeleted: true", async () => { + // A re-issued delete (definition already gone) 404s; the connector + // converts that to a success-shaped alreadyDeleted result so + // reconciliation / retry loops are safe. + mockFetch(404, { message: "Could not find resource" }); + const { deleteTagDefinition } = await import("../src/tools/tags.js"); + + const result = await deleteTagDefinition({ entity: "parties", tagId: 42, confirm: true }); + expect(result).toEqual({ deleted: true, alreadyDeleted: true, entity: "parties", tagId: 42 }); + }); + + it("rejects confirm=false / missing confirm at the schema layer", async () => { + const { deleteTagDefinitionSchema } = await import("../src/tools/tags.js"); + expect( + deleteTagDefinitionSchema.safeParse({ entity: "parties", tagId: 42, confirm: false }).success, + ).toBe(false); + expect(deleteTagDefinitionSchema.safeParse({ entity: "parties", tagId: 42 }).success).toBe( + false, + ); + expect( + deleteTagDefinitionSchema.safeParse({ entity: "parties", tagId: 42, confirm: true }).success, + ).toBe(true); + }); + + it("rejects unknown entity at the schema layer", async () => { + const { deleteTagDefinitionSchema } = await import("../src/tools/tags.js"); + expect( + deleteTagDefinitionSchema.safeParse({ entity: "widgets", tagId: 42, confirm: true }).success, + ).toBe(false); + }); + + it("handler guards confirm even when called directly (bypassing schema)", async () => { + // Tests + internal callers invoke the handler function directly, + // skipping the MCP-layer Zod validation. The handler's own guard + // must still reject a non-true confirm so the destructive op can't + // fire from a type-violating direct call. + const { deleteTagDefinition } = await import("../src/tools/tags.js"); + await expect( + // @ts-expect-error — deliberately violating the `confirm: true` type + deleteTagDefinition({ entity: "parties", tagId: 42, confirm: false }), + ).rejects.toThrow(/requires confirm: true/); + // No HTTP call should have been attempted. + expect(vi.mocked(fetch).mock.calls).toHaveLength(0); + }); +}); diff --git a/tests/tool-annotations.test.ts b/tests/tool-annotations.test.ts index d4a5a00..544ad08 100644 --- a/tests/tool-annotations.test.ts +++ b/tests/tool-annotations.test.ts @@ -1,7 +1,7 @@ /** * Tests for MCP tool annotations inference. * - * Annotations are name-inferred at registration time so all 87 tools + * Annotations are name-inferred at registration time so all 88 tools * get accurate hints without per-call-site annotation declarations. * These tests pin three contracts: * @@ -20,7 +20,7 @@ * we make every hint explicit and never rely on spec defaults. * * 3. Aggregate counts match the catalog: 49 read-only-and-not- - * destructive, 7 not-read-and-destructive, 31 not-read-and-not- + * destructive, 8 not-read-and-destructive, 31 not-read-and-not- * destructive (creates / updates / additive writes). Drift in * any direction means a new tool is going to surprise users * with an unexpected pre-call prompt (or, worse, an auto- @@ -54,13 +54,14 @@ describe("inferAnnotations (pure helper)", () => { } }); - it("marks the 7 destructive tools as not-read-only AND destructive (explicit)", () => { + it("marks the 8 destructive tools as not-read-only AND destructive (explicit)", () => { for (const name of [ "delete_party", "delete_opportunity", "delete_project", "delete_task", "delete_entry", + "delete_tag_definition", "remove_track", "remove_additional_party", ]) { @@ -161,13 +162,14 @@ describe("tools/list response carries inferred annotations", () => { expect(byName.get(r)?.annotations?.destructiveHint).toBe(false); } - // All 7 destructive tools. + // All 8 destructive tools. for (const dest of [ "delete_party", "delete_opportunity", "delete_project", "delete_task", "delete_entry", + "delete_tag_definition", "remove_track", "remove_additional_party", ]) { @@ -205,7 +207,7 @@ describe("tools/list response carries inferred annotations", () => { // Aggregate counts — the catalogue invariants we want CI to defend. // Three mutually-exclusive buckets: // - readOnly+nondestructive (reads): 49 - // - notRead+destructive (delete_*, remove_track, remove_additional_party): 7 + // - notRead+destructive (delete_*, remove_track, remove_additional_party): 8 // - notRead+notDestructive (creates/updates/additive writes): 31 let readOnly = 0; let destructive = 0; @@ -220,9 +222,9 @@ describe("tools/list response carries inferred annotations", () => { // any tool landing there means inferAnnotations regressed. } expect(readOnly).toBe(49); - expect(destructive).toBe(7); + expect(destructive).toBe(8); expect(routineWrite).toBe(31); expect(readOnly + destructive + routineWrite).toBe(result.tools.length); - expect(result.tools.length).toBe(87); + expect(result.tools.length).toBe(88); }); });