diff --git a/CLAUDE.md b/CLAUDE.md index bb7e6ae..cdcef96 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -278,13 +278,20 @@ Full list of tools exposed by this server (88 total). Each is implemented in `sr | `get_remote_agent_setup_command` | Build a bash command to register an agent on a remote machine | | `check_remote_agent_freshness` | Check if a remote agent's heartbeat is recent | -### Approvals Tools (4) -| Tool | Description | -|------|-------------| -| `list_pending_approvals` | List all pending approval requests across workspaces | -| `decide_approval` | Approve or deny a pending approval request | -| `create_approval` | Create an approval request for a workspace | -| `get_workspace_approvals` | List approval requests for a specific workspace | +### Approvals Tools (4) — DEPRECATED shims over the unified requests subsystem +These keep their original names and signatures (backward-compatible) but now +route to the unified `/requests` endpoints with `kind=approval` (RFC +"unified-requests-inbox", P5). New approvals land in the unified `requests` +table and surface in the unified inbox/Approvals tab. Prefer the Requests / +Inbox tools (`create_request` / `respond_request` / `list_inbox` / +`check_requests`) for new work. + +| Tool | Description | Routes to | +|------|-------------|-----------| +| `list_pending_approvals` | List all pending approval requests across workspaces | `GET /requests/pending?kind=approval` | +| `decide_approval` | Approve or deny a pending approval request (legacy `denied` -> `rejected`) | `POST /workspaces/:id/requests/:id/respond` | +| `create_approval` | Create an approval request for a workspace | `POST /workspaces/:id/requests` (kind=approval) | +| `get_workspace_approvals` | List requests raised by a specific workspace | `GET /workspaces/:id/requests` | ## MCP Transport Gotchas diff --git a/README.md b/README.md index ccd5cd9..017b632 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,8 @@ See the [full tool registry](CLAUDE.md#mcp-tool-registry) for all tools. Highlig | Channels | list adapters, list, add, update, remove, send, test, discover chats | | Schedules | list, create, update, delete, run, get history | | Discovery | list peers, discover, check_access, list events, import/export, canvas viewport | -| Approvals | list pending, decide, create, get workspace approvals | +| Requests / Inbox | `create_request`, `list_inbox`, `check_requests`, `get_request`, `respond_request`, `add_request_message`, `cancel_request` (unified Tasks + Approvals) | +| Approvals *(deprecated)* | `list_pending_approvals`, `decide_approval`, `create_approval`, `get_workspace_approvals` — backward-compatible shims that route to the unified requests system (`kind='approval'`); prefer the Requests / Inbox tools | | Remote Agents | list (runtime=external), get state, setup command, check freshness | ## Setup @@ -121,7 +122,7 @@ because several tool names overlap). | Tokens | `mint_org_token`, `list_org_tokens`, `revoke_org_token`, `mint_workspace_token` | | Plugin governance | `get_org_plugin_allowlist`, `set_org_plugin_allowlist` | | Bundles | `export_bundle`, `import_bundle` | -| Audit | `list_org_events`, `list_pending_approvals` | +| Audit | `list_org_events`, `list_pending_approvals` *(deprecated shim → `/requests/pending?kind=approval`)* | | **CP-tier (gated)** | `list_orgs`, `get_org` | Each tool's input schema, endpoint, and request body are derived from the diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index f3d81b7..66dcc60 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -872,15 +872,15 @@ describe("handleCollapseTeam()", () => { }); // ============================================================ -// Approvals +// Approvals (DEPRECATED shims over the unified /requests subsystem, RFC P5) // ============================================================ describe("handleListPendingApprovals()", () => { - test("GETs /approvals/pending", async () => { + test("GETs /requests/pending?kind=approval (unified shim)", async () => { global.fetch = mockFetch([{ id: "ap-1" }]); const result = await handleListPendingApprovals(); expect(global.fetch).toHaveBeenCalledWith( - `${PLATFORM_URL}/approvals/pending`, + `${PLATFORM_URL}/requests/pending?kind=approval`, expect.objectContaining({ method: "GET" }) ); expectJsonContent(result, [{ id: "ap-1" }]); @@ -888,48 +888,55 @@ describe("handleListPendingApprovals()", () => { }); describe("handleDecideApproval()", () => { - test("POSTs to /workspaces/:id/approvals/:ap_id/decide with approved decision", async () => { - global.fetch = mockFetch({ decided: true }); + test("POSTs to /workspaces/:id/requests/:id/respond with action=approved", async () => { + global.fetch = mockFetch({ status: "approved", request_id: "ap-42" }); const result = await handleDecideApproval({ workspace_id: "ws-1", approval_id: "ap-42", decision: "approved", }); const callArgs = (global.fetch as jest.Mock).mock.calls[0]; - expect(callArgs[0]).toBe(`${PLATFORM_URL}/workspaces/ws-1/approvals/ap-42/decide`); + expect(callArgs[0]).toBe(`${PLATFORM_URL}/workspaces/ws-1/requests/ap-42/respond`); expect(callArgs[1].method).toBe("POST"); const sent = JSON.parse(callArgs[1].body); - expect(sent.decision).toBe("approved"); - expect(sent.decided_by).toBe("mcp-client"); - expectJsonContent(result, { decided: true }); + expect(sent.action).toBe("approved"); + expect(sent.responder_type).toBe("user"); + expect(sent.responder_id).toBe("admin"); + expectJsonContent(result, { status: "approved", request_id: "ap-42" }); }); - test("POSTs with denied decision", async () => { - global.fetch = mockFetch({ decided: true }); + test("maps legacy decision=denied to action=rejected", async () => { + global.fetch = mockFetch({ status: "rejected", request_id: "ap-99" }); await handleDecideApproval({ workspace_id: "ws-1", approval_id: "ap-99", decision: "denied" }); - const sent = JSON.parse((global.fetch as jest.Mock).mock.calls[0][1].body); - expect(sent.decision).toBe("denied"); + const callArgs = (global.fetch as jest.Mock).mock.calls[0]; + expect(callArgs[0]).toBe(`${PLATFORM_URL}/workspaces/ws-1/requests/ap-99/respond`); + const sent = JSON.parse(callArgs[1].body); + expect(sent.action).toBe("rejected"); }); }); describe("handleCreateApproval()", () => { - test("POSTs to /workspaces/:id/approvals with action and reason", async () => { - global.fetch = mockFetch({ id: "ap-new" }); + test("POSTs to /workspaces/:id/requests with kind=approval, action->title, reason->detail", async () => { + global.fetch = mockFetch({ request_id: "ap-new", status: "pending" }); await handleCreateApproval({ workspace_id: "ws-1", action: "deploy", reason: "prod release" }); const callArgs = (global.fetch as jest.Mock).mock.calls[0]; - expect(callArgs[0]).toBe(`${PLATFORM_URL}/workspaces/ws-1/approvals`); + expect(callArgs[0]).toBe(`${PLATFORM_URL}/workspaces/ws-1/requests`); + expect(callArgs[1].method).toBe("POST"); const sent = JSON.parse(callArgs[1].body); - expect(sent.action).toBe("deploy"); - expect(sent.reason).toBe("prod release"); + expect(sent.kind).toBe("approval"); + expect(sent.recipient_type).toBe("user"); + expect(sent.recipient_id).toBe(""); + expect(sent.title).toBe("deploy"); + expect(sent.detail).toBe("prod release"); }); }); describe("handleGetWorkspaceApprovals()", () => { - test("GETs /workspaces/:id/approvals", async () => { + test("GETs /workspaces/:id/requests (unified outgoing shim)", async () => { global.fetch = mockFetch([{ id: "ap-1" }]); await handleGetWorkspaceApprovals({ workspace_id: "ws-1" }); expect(global.fetch).toHaveBeenCalledWith( - `${PLATFORM_URL}/workspaces/ws-1/approvals`, + `${PLATFORM_URL}/workspaces/ws-1/requests`, expect.objectContaining({ method: "GET" }) ); }); diff --git a/src/tools/approvals.ts b/src/tools/approvals.ts index cdceb90..aafd125 100644 --- a/src/tools/approvals.ts +++ b/src/tools/approvals.ts @@ -3,6 +3,33 @@ import { z } from "zod"; import { apiCall, platformGet, toMcpResult } from "../api.js"; import { validate } from "../utils/validation.js"; +// --------------------------------------------------------------------------- +// Approval tools — DEPRECATED SHIMS over the unified requests subsystem +// (RFC "unified-requests-inbox", Phase 5). +// +// These four tools keep their original NAMES and PARAMETER SIGNATURES for +// backward compatibility, but their handlers now route to the unified +// `/requests` endpoints with kind='approval' instead of the legacy +// `/approvals` endpoints. New approvals therefore land in the unified +// `requests` table and surface in the unified inbox/Approvals tab alongside +// requests created via create_request. +// +// Prefer the new tools (create_request / respond_request / list_inbox / +// check_requests) for new work — these shims exist only so existing callers +// do not break. +// +// Endpoint contract (molecule-core workspace-server, RFC P1): +// POST /workspaces/:id/requests (Create) +// POST /workspaces/:id/requests/:requestId/respond (Respond) +// GET /requests/pending?kind=approval (ListPending — cross-org) +// GET /workspaces/:id/requests (ListOutgoing) +// NOTE: the per-workspace reads (ListOutgoing / ListInbox) take only a +// `status` filter — P1 has NO `kind` query param on those reads, so +// get_workspace_approvals cannot filter to approvals server-side; it returns +// this workspace's outgoing requests (tasks + approvals). The cross-org +// /requests/pending endpoint DOES support ?kind=approval. +// --------------------------------------------------------------------------- + // --------------------------------------------------------------------------- // Schemas // --------------------------------------------------------------------------- @@ -27,37 +54,57 @@ const GetWorkspaceApprovalsSchema = z.object({ export type GetWorkspaceApprovalsParams = z.infer; // --------------------------------------------------------------------------- -// Handlers +// Handlers (shims → unified /requests, kind='approval') // --------------------------------------------------------------------------- export async function handleListPendingApprovals(): Promise> { - const data = await platformGet("/approvals/pending"); + // Cross-org pending view, filtered to the approval slice. P1's + // /requests/pending supports ?kind=task|approval (validated server-side). + const data = await platformGet("/requests/pending?kind=approval"); return toMcpResult(data); } export async function handleDecideApproval(args: unknown): Promise> { const params = validate(args, DecideApprovalSchema); + // Map the legacy decision enum to the unified respond action. For an + // approval-kind request the valid terminal actions are approved | rejected; + // legacy "denied" maps to "rejected". responder identity = user/admin (the + // canvas/admin path default). + const action = params.decision === "approved" ? "approved" : "rejected"; const data = await apiCall( "POST", - `/workspaces/${params.workspace_id}/approvals/${params.approval_id}/decide`, - { decision: params.decision, decided_by: "mcp-client" } + `/workspaces/${params.workspace_id}/requests/${params.approval_id}/respond`, + { action, responder_type: "user", responder_id: "admin" } ); return toMcpResult(data); } export async function handleCreateApproval(args: unknown): Promise> { const params = validate(args, CreateApprovalSchema); + // Raise an approval-kind request addressed to a user. The action becomes the + // title and the reason becomes the detail. recipient_id is left empty (P1 + // does not require it for a user recipient). const data = await apiCall( "POST", - `/workspaces/${params.workspace_id}/approvals`, - { action: params.action, reason: params.reason } + `/workspaces/${params.workspace_id}/requests`, + { + kind: "approval", + recipient_type: "user", + recipient_id: "", + title: params.action, + detail: params.reason, + } ); return toMcpResult(data); } export async function handleGetWorkspaceApprovals(args: unknown): Promise> { const params = validate(args, GetWorkspaceApprovalsSchema); - const data = await platformGet(`/workspaces/${params.workspace_id}/approvals`); + // The unified equivalent of "approvals raised by this workspace" is its + // outgoing requests. P1 has NO kind filter on this read, so the result + // includes any tasks the workspace also raised; clients that need only + // approvals can filter client-side on kind, or use list_inbox / check_requests. + const data = await platformGet(`/workspaces/${params.workspace_id}/requests`); return toMcpResult(data); } @@ -65,17 +112,20 @@ export async function handleGetWorkspaceApprovals(args: unknown): Promise