From fa68ce39ba55925d6db0ad22fdda8c1bdd1e5822 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 04:26:09 +0000 Subject: [PATCH] feat(client): add approvals namespace; remove dead workflow surface (ADR-0019) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0019 collapsed approval into Flow — approval is a first-class flow node, not a workflow step. The server exposes a dedicated `/api/v1/approvals` surface, but the client SDK still carried the old approval-on-`workflow` methods pointing at routes that never existed. - `@objectstack/client`: add a `client.approvals` namespace backed by the real REST surface — `listRequests` (the "my approvals" inbox), `getRequest`, `approve` / `reject` (record a decision, resume the owning flow), and `listActions` (audit trail). Re-export the approval runtime types. - Remove the dead workflow approve/reject surface: `client.workflow.approve` / `reject`, the `WorkflowApprove*` / `WorkflowReject*` protocol schemas, types, `IProtocolService` methods, and the `/approve` / `/reject` entries in `DEFAULT_WORKFLOW_ROUTES`. `workflow` is reclaimed for state machines, so `getConfig` / `getState` / `transition` are unchanged. - Discovery advertises the new `approvals` route key. --- .changeset/client-approvals-namespace.md | 38 ++++++ packages/client/src/client.test.ts | 80 +++++++++--- packages/client/src/index.ts | 115 ++++++++++++++---- packages/spec/src/api/discovery.zod.ts | 3 + packages/spec/src/api/plugin-rest-api.test.ts | 7 +- packages/spec/src/api/plugin-rest-api.zod.ts | 33 +---- packages/spec/src/api/protocol.test.ts | 8 -- packages/spec/src/api/protocol.zod.ts | 42 +------ 8 files changed, 203 insertions(+), 123 deletions(-) create mode 100644 .changeset/client-approvals-namespace.md diff --git a/.changeset/client-approvals-namespace.md b/.changeset/client-approvals-namespace.md new file mode 100644 index 000000000..842d25e20 --- /dev/null +++ b/.changeset/client-approvals-namespace.md @@ -0,0 +1,38 @@ +--- +'@objectstack/client': minor +'@objectstack/spec': minor +--- + +client SDK: add `approvals` namespace; remove dead workflow approve/reject surface (ADR-0019) + +ADR-0019 collapsed approval into Flow: approval is no longer a workflow step but +a first-class **flow node** that opens a request and suspends the run, with a +human decision resuming the flow down the matching `approve` / `reject` edge. +The server already exposes this as a dedicated `/api/v1/approvals` surface +(`registerApprovalsEndpoints`), but the client SDK still carried the old +approval-on-`workflow` methods, which pointed at routes that never existed. + +- **`@objectstack/client`** gains a `client.approvals` namespace backed by the + real REST surface: + - `listRequests(filter?)` → `GET /approvals/requests` (the "my approvals" + inbox; filter by `status` (single or array), `object`, `recordId`, + `approverId`, `submitterId`). + - `getRequest(id)` → `GET /approvals/requests/:id`. + - `approve(id, { actorId?, comment? })` / `reject(id, …)` → + `POST /approvals/requests/:id/{approve,reject}` (records a decision and + resumes the owning flow run). + - `listActions(id)` → `GET /approvals/requests/:id/actions` (audit trail). + + The approval runtime types (`ApprovalRequestRow`, `ApprovalActionRow`, + `ApprovalStatus`, `ApprovalDecisionInput`, `ApprovalDecisionResult`) are + re-exported so consumers can type the namespace without reaching into + `@objectstack/spec`. + +- **Removed the dead workflow approve/reject surface.** `client.workflow.approve` + / `client.workflow.reject` and the backing `WorkflowApprove*` / `WorkflowReject*` + protocol schemas, types, `IProtocolService` methods, and the `/approve` / + `/reject` entries in `DEFAULT_WORKFLOW_ROUTES` are gone — approval decisions + are no longer recorded on a workflow record. `workflow` is reclaimed for state + machines, so `getConfig` / `getState` / `transition` are unchanged. + +- Discovery advertises the new route key: `ApiRoutesSchema.approvals`. diff --git a/packages/client/src/client.test.ts b/packages/client/src/client.test.ts index 45ee3d231..e583cd549 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -272,36 +272,76 @@ describe('Workflow namespace', () => { expect(body.comment).toBe('Ready for review'); }); - it('should approve workflow', async () => { + // ADR-0019: approve/reject left the workflow namespace — they now live on + // `client.approvals` (approval is a flow node, not a workflow step). +}); + +describe('Approvals namespace (ADR-0019)', () => { + it('should list approval requests with filters', async () => { const { client, fetchMock } = createMockClient({ - success: true, - data: { success: true, newState: 'approved' } + data: [{ id: 'req-1', status: 'pending', object_name: 'order', record_id: 'rec-1', process_name: 'flow:approve' }] }); - const result = await client.workflow.approve({ - object: 'order', - recordId: 'rec-1', - comment: 'Looks good' + const result = await client.approvals.listRequests({ status: 'pending', approverId: 'user-1' }); + expect(result).toHaveLength(1); + expect(result[0].id).toBe('req-1'); + const url = fetchMock.mock.calls[0][0] as string; + expect(url).toContain('/api/v1/approvals/requests'); + expect(url).toContain('status=pending'); + expect(url).toContain('approverId=user-1'); + }); + + it('should join array status filters', async () => { + const { client, fetchMock } = createMockClient({ data: [] }); + await client.approvals.listRequests({ status: ['approved', 'rejected'] }); + const url = decodeURIComponent(fetchMock.mock.calls[0][0] as string); + expect(url).toContain('status=approved,rejected'); + }); + + it('should get a single approval request', async () => { + const { client, fetchMock } = createMockClient({ + id: 'req-1', status: 'pending', object_name: 'order', record_id: 'rec-1', process_name: 'flow:approve' }); - expect(result.newState).toBe('approved'); + const result = await client.approvals.getRequest('req-1'); + expect(result.id).toBe('req-1'); + const url = fetchMock.mock.calls[0][0] as string; + expect(url).toContain('/api/v1/approvals/requests/req-1'); + }); + + it('should record an approve decision', async () => { + const { client, fetchMock } = createMockClient({ + request: { id: 'req-1', status: 'approved' }, finalized: true, decision: 'approve', resumed: true + }); + const result = await client.approvals.approve('req-1', { actorId: 'user-1', comment: 'Looks good' }); + expect(result.finalized).toBe(true); + expect(result.decision).toBe('approve'); const [url, opts] = fetchMock.mock.calls[0]; - expect(url).toContain('/api/v1/workflow/order/rec-1/approve'); + expect(url).toContain('/api/v1/approvals/requests/req-1/approve'); expect(opts.method).toBe('POST'); + const body = JSON.parse(opts.body); + expect(body.actorId).toBe('user-1'); + expect(body.comment).toBe('Looks good'); }); - it('should reject workflow', async () => { + it('should record a reject decision', async () => { const { client, fetchMock } = createMockClient({ - success: true, - data: { success: true, newState: 'rejected' } + request: { id: 'req-1', status: 'rejected' }, finalized: true, decision: 'reject' }); - const result = await client.workflow.reject({ - object: 'order', - recordId: 'rec-1', - reason: 'Incomplete data', - comment: 'Missing fields' + const result = await client.approvals.reject('req-1', { comment: 'Missing fields' }); + expect(result.decision).toBe('reject'); + const [url, opts] = fetchMock.mock.calls[0]; + expect(url).toContain('/api/v1/approvals/requests/req-1/reject'); + expect(opts.method).toBe('POST'); + }); + + it('should list the action audit trail', async () => { + const { client, fetchMock } = createMockClient({ + data: [{ id: 'act-1', request_id: 'req-1', action: 'approve', actor_id: 'user-1' }] }); - expect(result.newState).toBe('rejected'); - const body = JSON.parse(fetchMock.mock.calls[0][1].body); - expect(body.reason).toBe('Incomplete data'); + const result = await client.approvals.listActions('req-1'); + expect(result).toHaveLength(1); + expect(result[0].action).toBe('approve'); + const url = fetchMock.mock.calls[0][0] as string; + expect(url).toContain('/api/v1/approvals/requests/req-1/actions'); }); }); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index ff2b49822..22b5e3efc 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -40,10 +40,6 @@ import { GetWorkflowStateResponse, WorkflowTransitionRequest, WorkflowTransitionResponse, - WorkflowApproveRequest, - WorkflowApproveResponse, - WorkflowRejectRequest, - WorkflowRejectResponse, ListViewsResponse, GetViewResponse, CreateViewRequest, @@ -87,6 +83,12 @@ import { WellKnownCapabilities, ApiRoutes, } from '@objectstack/spec/api'; +import type { + ApprovalRequestRow, + ApprovalActionRow, + ApprovalStatus, + ApprovalDecisionResult, +} from '@objectstack/spec/contracts'; import { Logger, createLogger } from '@objectstack/core/logger'; import { RealtimeAPI } from './realtime-api'; @@ -2494,36 +2496,90 @@ export class ObjectStackClient { }) }); return this.unwrapResponse(res); + } + // ADR-0019: approve/reject are no longer workflow operations. Approval is a + // flow node — see the `approvals` namespace below for recording decisions. + }; + + /** + * Approval Services (ADR-0019) + * + * Approval is a first-class flow node, not a workflow step: a flow's + * Approval node opens a request and suspends the run; recording a decision + * here finalises the request and resumes the owning flow down the matching + * `approve` / `reject` edge. This namespace drives the "my approvals" inbox + * and the decision API exposed under `/api/v1/approvals`. + */ + approvals = { + /** + * List approval requests ("my approvals" inbox). Filter by status, target + * object / record, the user expected to act next, or the submitter. + */ + listRequests: async (filter?: { + object?: string; + recordId?: string; + status?: ApprovalStatus | ApprovalStatus[]; + approverId?: string; + submitterId?: string; + }): Promise => { + const route = this.getRoute('approvals'); + const params = new URLSearchParams(); + if (filter?.object) params.set('object', filter.object); + if (filter?.recordId) params.set('recordId', filter.recordId); + if (filter?.status) { + params.set('status', Array.isArray(filter.status) ? filter.status.join(',') : filter.status); + } + if (filter?.approverId) params.set('approverId', filter.approverId); + if (filter?.submitterId) params.set('submitterId', filter.submitterId); + const qs = params.toString(); + const res = await this.fetch(`${this.baseUrl}${route}/requests${qs ? `?${qs}` : ''}`); + const body = await this.unwrapResponse<{ data?: ApprovalRequestRow[] } | ApprovalRequestRow[]>(res); + return Array.isArray(body) ? body : (body?.data ?? []); }, /** - * Approve a workflow step + * Get a single approval request by id. */ - approve: async (request: WorkflowApproveRequest): Promise => { - const route = this.getRoute('workflow'); - const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(request.object)}/${encodeURIComponent(request.recordId)}/approve`, { + getRequest: async (requestId: string): Promise => { + const route = this.getRoute('approvals'); + const res = await this.fetch(`${this.baseUrl}${route}/requests/${encodeURIComponent(requestId)}`); + return this.unwrapResponse(res); + }, + + /** + * Record an approve decision on a request. Finalises the request when the + * node's behaviour is satisfied and resumes the owning flow run. + */ + approve: async (requestId: string, decision?: { actorId?: string; comment?: string }): Promise => { + const route = this.getRoute('approvals'); + const res = await this.fetch(`${this.baseUrl}${route}/requests/${encodeURIComponent(requestId)}/approve`, { method: 'POST', - body: JSON.stringify({ - comment: request.comment, - data: request.data - }) + body: JSON.stringify({ actorId: decision?.actorId, comment: decision?.comment }) }); - return this.unwrapResponse(res); + return this.unwrapResponse(res); }, /** - * Reject a workflow step + * Record a reject decision on a request. Resumes the owning flow run down + * the `reject` edge. */ - reject: async (request: WorkflowRejectRequest): Promise => { - const route = this.getRoute('workflow'); - const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(request.object)}/${encodeURIComponent(request.recordId)}/reject`, { + reject: async (requestId: string, decision?: { actorId?: string; comment?: string }): Promise => { + const route = this.getRoute('approvals'); + const res = await this.fetch(`${this.baseUrl}${route}/requests/${encodeURIComponent(requestId)}/reject`, { method: 'POST', - body: JSON.stringify({ - reason: request.reason, - comment: request.comment - }) + body: JSON.stringify({ actorId: decision?.actorId, comment: decision?.comment }) }); - return this.unwrapResponse(res); + return this.unwrapResponse(res); + }, + + /** + * Audit trail (the immutable action log) for an approval request. + */ + listActions: async (requestId: string): Promise => { + const route = this.getRoute('approvals'); + const res = await this.fetch(`${this.baseUrl}${route}/requests/${encodeURIComponent(requestId)}/actions`); + const body = await this.unwrapResponse<{ data?: ApprovalActionRow[] } | ApprovalActionRow[]>(res); + return Array.isArray(body) ? body : (body?.data ?? []); } }; @@ -3286,6 +3342,7 @@ export class ObjectStackClient { permissions: '/api/v1/permissions', realtime: '/api/v1/realtime', workflow: '/api/v1/workflow', + approvals: '/api/v1/approvals', views: '/api/v1/ui/views', notifications: '/api/v1/notifications', ai: '/api/v1/ai', @@ -3611,10 +3668,6 @@ export type { GetWorkflowStateResponse, WorkflowTransitionRequest, WorkflowTransitionResponse, - WorkflowApproveRequest, - WorkflowApproveResponse, - WorkflowRejectRequest, - WorkflowRejectResponse, ListViewsResponse, GetViewResponse, CreateViewResponse, @@ -3654,3 +3707,13 @@ export type { EmailPasswordConfigPublic, AuthFeaturesConfig, } from '@objectstack/spec/api'; + +// Approval runtime types (ADR-0019) — surfaced so SDK consumers can type the +// `client.approvals` namespace without reaching into `@objectstack/spec`. +export type { + ApprovalRequestRow, + ApprovalActionRow, + ApprovalStatus, + ApprovalDecisionInput, + ApprovalDecisionResult, +} from '@objectstack/spec/contracts'; diff --git a/packages/spec/src/api/discovery.zod.ts b/packages/spec/src/api/discovery.zod.ts index 1d063fe46..6381e9479 100644 --- a/packages/spec/src/api/discovery.zod.ts +++ b/packages/spec/src/api/discovery.zod.ts @@ -109,6 +109,9 @@ export const ApiRoutesSchema = lazySchema(() => z.object({ /** Base URL for Workflow Engine */ workflow: z.string().optional().describe('e.g. /api/v1/workflow'), + /** Base URL for Approvals (ADR-0019: approval as a flow node) */ + approvals: z.string().optional().describe('e.g. /api/v1/approvals'), + /** Base URL for Realtime (WebSocket/SSE) */ realtime: z.string().optional().describe('e.g. /api/v1/realtime'), diff --git a/packages/spec/src/api/plugin-rest-api.test.ts b/packages/spec/src/api/plugin-rest-api.test.ts index ca68f7aab..a4b6d4461 100644 --- a/packages/spec/src/api/plugin-rest-api.test.ts +++ b/packages/spec/src/api/plugin-rest-api.test.ts @@ -537,9 +537,10 @@ describe('plugin-rest-api.zod', () => { expect(DEFAULT_WORKFLOW_ROUTES.methods).toContain('getWorkflowConfig'); expect(DEFAULT_WORKFLOW_ROUTES.methods).toContain('getWorkflowState'); expect(DEFAULT_WORKFLOW_ROUTES.methods).toContain('workflowTransition'); - expect(DEFAULT_WORKFLOW_ROUTES.methods).toContain('workflowApprove'); - expect(DEFAULT_WORKFLOW_ROUTES.methods).toContain('workflowReject'); - expect(DEFAULT_WORKFLOW_ROUTES.endpoints).toHaveLength(5); + // ADR-0019: approve/reject are no longer workflow routes (moved to /approvals). + expect(DEFAULT_WORKFLOW_ROUTES.methods).not.toContain('workflowApprove'); + expect(DEFAULT_WORKFLOW_ROUTES.methods).not.toContain('workflowReject'); + expect(DEFAULT_WORKFLOW_ROUTES.endpoints).toHaveLength(3); }); it('should validate DEFAULT_REALTIME_ROUTES', () => { diff --git a/packages/spec/src/api/plugin-rest-api.zod.ts b/packages/spec/src/api/plugin-rest-api.zod.ts index bd73a19d0..b8d82c32a 100644 --- a/packages/spec/src/api/plugin-rest-api.zod.ts +++ b/packages/spec/src/api/plugin-rest-api.zod.ts @@ -1119,7 +1119,7 @@ export const DEFAULT_WORKFLOW_ROUTES: RestApiRouteRegistration = { prefix: '/api/v1/workflow', service: 'workflow', category: 'workflow', - methods: ['getWorkflowConfig', 'getWorkflowState', 'workflowTransition', 'workflowApprove', 'workflowReject'], + methods: ['getWorkflowConfig', 'getWorkflowState', 'workflowTransition'], authRequired: true, endpoints: [ { @@ -1161,34 +1161,9 @@ export const DEFAULT_WORKFLOW_ROUTES: RestApiRouteRegistration = { permissions: ['workflow.transition'], cacheable: false, }, - { - method: 'POST', - path: '/:object/:recordId/approve', - handler: 'workflowApprove', - category: 'workflow', - public: false, - summary: 'Approve workflow step', - description: 'Approves a pending workflow approval step', - tags: ['Workflow'], - requestSchema: 'WorkflowApproveRequestSchema', - responseSchema: 'WorkflowApproveResponseSchema', - permissions: ['workflow.approve'], - cacheable: false, - }, - { - method: 'POST', - path: '/:object/:recordId/reject', - handler: 'workflowReject', - category: 'workflow', - public: false, - summary: 'Reject workflow step', - description: 'Rejects a pending workflow approval step', - tags: ['Workflow'], - requestSchema: 'WorkflowRejectRequestSchema', - responseSchema: 'WorkflowRejectResponseSchema', - permissions: ['workflow.reject'], - cacheable: false, - }, + // ADR-0019: approve/reject are no longer workflow routes. Approval is a + // flow node; decisions are recorded on the approvals runtime via + // `POST /api/v1/approvals/requests/:id/{approve,reject}`. ], middleware: [ { name: 'auth', type: 'authentication', enabled: true, order: 10 }, diff --git a/packages/spec/src/api/protocol.test.ts b/packages/spec/src/api/protocol.test.ts index 193e2e4c2..62e257aec 100644 --- a/packages/spec/src/api/protocol.test.ts +++ b/packages/spec/src/api/protocol.test.ts @@ -34,8 +34,6 @@ import { GetWorkflowStateRequestSchema, WorkflowTransitionRequestSchema, WorkflowTransitionResponseSchema, - WorkflowApproveRequestSchema, - WorkflowRejectRequestSchema, // Realtime RealtimeConnectRequestSchema, RealtimeConnectResponseSchema, @@ -225,12 +223,6 @@ describe('ObjectStack Protocol', () => { expect(WorkflowTransitionResponseSchema.safeParse({ object: 'lead', recordId: 'l1', success: true, state, }).success).toBe(true); - expect(WorkflowApproveRequestSchema.safeParse({ - object: 'lead', recordId: 'l1', comment: 'Approved', - }).success).toBe(true); - expect(WorkflowRejectRequestSchema.safeParse({ - object: 'lead', recordId: 'l1', reason: 'Missing info', - }).success).toBe(true); }); it('validates Realtime operations', () => { diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index 6f276774c..be60b01aa 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -726,33 +726,11 @@ export const WorkflowTransitionResponseSchema = lazySchema(() => z.object({ state: WorkflowStateSchema.describe('New workflow state after transition'), })); -export const WorkflowApproveRequestSchema = lazySchema(() => z.object({ - object: z.string().describe('Object name'), - recordId: z.string().describe('Record ID'), - comment: z.string().optional().describe('Approval comment'), - data: z.record(z.string(), z.unknown()).optional().describe('Additional data'), -})); - -export const WorkflowApproveResponseSchema = lazySchema(() => z.object({ - object: z.string().describe('Object name'), - recordId: z.string().describe('Record ID'), - success: z.boolean().describe('Whether the approval succeeded'), - state: WorkflowStateSchema.describe('New workflow state after approval'), -})); - -export const WorkflowRejectRequestSchema = lazySchema(() => z.object({ - object: z.string().describe('Object name'), - recordId: z.string().describe('Record ID'), - reason: z.string().describe('Rejection reason'), - comment: z.string().optional().describe('Additional comment'), -})); - -export const WorkflowRejectResponseSchema = lazySchema(() => z.object({ - object: z.string().describe('Object name'), - recordId: z.string().describe('Record ID'), - success: z.boolean().describe('Whether the rejection succeeded'), - state: WorkflowStateSchema.describe('New workflow state after rejection'), -})); +// ADR-0019: approval is no longer a workflow step. The approve/reject surface +// moved off `workflow` onto the dedicated approvals runtime (a flow's Approval +// node opens a request and suspends; a decision resumes it). Decisions are +// recorded via `POST /approvals/requests/:id/{approve,reject}`, not on a +// workflow record. `workflow` is reclaimed for state machines (transitions). // ========================================== // Realtime Operations @@ -1125,10 +1103,6 @@ export const ObjectStackProtocolSchema = lazySchema(() => z.object({ .describe('Get workflow state for a record'), workflowTransition: z.function() .describe('Execute a workflow state transition'), - workflowApprove: z.function() - .describe('Approve a workflow step'), - workflowReject: z.function() - .describe('Reject a workflow step'), // Realtime Operations realtimeConnect: z.function() @@ -1287,10 +1261,6 @@ export type GetWorkflowStateRequest = z.input; export type WorkflowTransitionRequest = z.input; export type WorkflowTransitionResponse = z.infer; -export type WorkflowApproveRequest = z.input; -export type WorkflowApproveResponse = z.infer; -export type WorkflowRejectRequest = z.input; -export type WorkflowRejectResponse = z.infer; // Realtime Types export type RealtimeConnectRequest = z.input; @@ -1466,8 +1436,6 @@ export interface ObjectStackProtocol { getWorkflowConfig?(request: GetWorkflowConfigRequest): Promise; getWorkflowState?(request: GetWorkflowStateRequest): Promise; workflowTransition?(request: WorkflowTransitionRequest): Promise; - workflowApprove?(request: WorkflowApproveRequest): Promise; - workflowReject?(request: WorkflowRejectRequest): Promise; // Realtime (optional) realtimeConnect?(request: RealtimeConnectRequest): Promise;