|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #5856 — `callData` has no `batch` arm, and `batch` is refused like every |
| 5 | + * other action it does not serve. |
| 6 | + * |
| 7 | + * The removed code was three lines: |
| 8 | + * |
| 9 | + * ```ts |
| 10 | + * if (action === 'batch') { |
| 11 | + * // Batch operations — not yet supported via direct service dispatch |
| 12 | + * return { object: params.object, results: [] }; |
| 13 | + * } |
| 14 | + * ``` |
| 15 | + * |
| 16 | + * It was the ONLY arm in `callData` that answered an unimplemented action with |
| 17 | + * SUCCESS. Every other unhandled action throws `400 Unknown data action: …`, |
| 18 | + * and `aggregate` throws `503` when the engine cannot serve it — this one |
| 19 | + * returned an HTTP 200 whose body is shaped exactly like a batch that ran and |
| 20 | + * matched nothing, having opened no transaction and written nothing. Retry, |
| 21 | + * idempotency and audit all read that as one successful empty operation. |
| 22 | + * |
| 23 | + * ## Why deleting it changed no online behaviour — the enumeration |
| 24 | + * |
| 25 | + * Nothing could reach the arm, and its unreachability lived UPSTREAM of it |
| 26 | + * (ADR-0115 Evidence 5 / #4451: "the slot exists, nobody registers it"), which |
| 27 | + * is why removal is the fix rather than a comment. Every entry point into |
| 28 | + * `callData`, on `main` at the time of the fix: |
| 29 | + * |
| 30 | + * | entry point | what it passes as `action` | |
| 31 | + * |---|---| |
| 32 | + * | `domains/data.ts` (`/data`) | the literals `query` / `get` / `create` / `update` / `delete`; `parts[1]` is compared against `'query'` and otherwise read as a record **id**, never as an action | |
| 33 | + * | `domains/mcp.ts` (MCP bridge, `run_action`) | the literals `query` / `get` / `aggregate` / `create` / `update` / `delete` | |
| 34 | + * | `domains/actions.ts` + `invokeBusinessAction` | the literal `get` | |
| 35 | + * | `endpoint-executor.ts` (declarative endpoints, bound in `dispatcher-plugin.ts`) | one literal per `ObjectOperation`, and that type is `ApiEndpointSchema.objectParams.operation` — a CLOSED enum of find/get/create/update/delete | |
| 36 | + * | outside this package | nothing: `callData` is not re-exported from `packages/runtime/src/index.ts` | |
| 37 | + * |
| 38 | + * The two structural halves of that table are pinned below (the `/data` route |
| 39 | + * table, and the endpoint vocabulary) so a future re-wiring has to face them. |
| 40 | + * |
| 41 | + * ## What this suite pins |
| 42 | + * |
| 43 | + * 1. `batch` is refused with the SAME `{ statusCode: 400, message }` shape as |
| 44 | + * any other unknown action — on a deployment WITH the `protocol` slot and |
| 45 | + * on one WITHOUT it, since the removed arm sat past both paths; |
| 46 | + * 2. the actions `callData` really serves are untouched (positive control); |
| 47 | + * 3. the two upstream facts that made the arm unreachable. |
| 48 | + * |
| 49 | + * Reverse verification (direction predicted BEFORE running, then measured — |
| 50 | + * see the PR): restoring the three lines turns case 1 RED in the ordinary |
| 51 | + * direction — the call RESOLVES `{ object: 'task', results: [] }` instead of |
| 52 | + * rejecting, so every `rejects` assertion in `describe('batch is refused …')` |
| 53 | + * fails with "promise resolved instead of rejected". Cases 2 and 3 stay green |
| 54 | + * under the restore: they describe the paths the arm never sat on, which is |
| 55 | + * the same claim the enumeration above makes. |
| 56 | + */ |
| 57 | + |
| 58 | +import { describe, it, expect } from 'vitest'; |
| 59 | +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; |
| 60 | +import { ApiEndpointSchema } from '@objectstack/spec/api'; |
| 61 | + |
| 62 | +import { callData, type ActionExecutionDeps } from './action-execution.js'; |
| 63 | +import { HttpDispatcher, type HttpProtocolContext } from './http-dispatcher.js'; |
| 64 | + |
| 65 | +const EC = { userId: 'u1', isSystem: false, positions: [], permissions: [] } as any; |
| 66 | +/** [#5155] Every service lookup resolves off the REQUEST's kernel. */ |
| 67 | +const REQ = { request: {} } as HttpProtocolContext; |
| 68 | +const SCHEMA = { name: 'task', fields: { title: { name: 'title', type: 'text' } } }; |
| 69 | + |
| 70 | +// --------------------------------------------------------------------------- |
| 71 | +// Harnesses — the same row set behind both deployments |
| 72 | +// --------------------------------------------------------------------------- |
| 73 | + |
| 74 | +function rows() { |
| 75 | + return [{ id: 'r1', title: 'one' }]; |
| 76 | +} |
| 77 | + |
| 78 | +/** The read surface both harnesses share. No write verb is defined: this suite |
| 79 | + * never writes, and a double that declares one it does not need is a contract |
| 80 | + * to keep in sync for nothing (`check:engine-double-contract`'s subject). */ |
| 81 | +function engine(store = rows()) { |
| 82 | + return { |
| 83 | + // `registry` is what `HttpDispatcher.getObjectQLService` requires before |
| 84 | + // it will hand the service to `callData` — not decoration. |
| 85 | + registry: { getObject: (n: string) => (n === 'task' ? SCHEMA : undefined) }, |
| 86 | + find: async (_o: string, bag: any) => { |
| 87 | + const id = bag?.where?.id; |
| 88 | + return id == null ? [...store] : store.filter((r) => r.id === String(id)); |
| 89 | + }, |
| 90 | + findOne: async (_o: string, opts: any) => store.find((r) => r.id === String(opts?.where?.id)) ?? null, |
| 91 | + } as any; |
| 92 | +} |
| 93 | + |
| 94 | +/** No `protocol` slot — every verb takes `callData`'s ObjectQL fallback. */ |
| 95 | +function fallbackHarness() { |
| 96 | + const ql = engine(); |
| 97 | + const services: Record<string, any> = { |
| 98 | + metadata: { getObject: async () => ({ name: 'task', fields: {} }) }, |
| 99 | + objectql: ql, |
| 100 | + }; |
| 101 | + const deps: ActionExecutionDeps = { |
| 102 | + resolveService: (async (_c: HttpProtocolContext, name: string) => services[name]) as any, |
| 103 | + getObjectQL: async () => ql, |
| 104 | + }; |
| 105 | + return { deps, ql, services }; |
| 106 | +} |
| 107 | + |
| 108 | +/** Protocol-first, with the REAL `@objectstack/metadata-protocol` occupant. */ |
| 109 | +function protocolHarness() { |
| 110 | + const ql = engine(); |
| 111 | + const services: Record<string, any> = { |
| 112 | + metadata: { getObject: async () => ({ name: 'task', fields: {} }) }, |
| 113 | + protocol: new ObjectStackProtocolImplementation(ql), |
| 114 | + objectql: ql, |
| 115 | + }; |
| 116 | + const deps: ActionExecutionDeps = { |
| 117 | + resolveService: (async (_c: HttpProtocolContext, name: string) => services[name]) as any, |
| 118 | + getObjectQL: async () => ql, |
| 119 | + }; |
| 120 | + return { deps, ql, services }; |
| 121 | +} |
| 122 | + |
| 123 | +const DEPLOYMENTS: Array<[string, () => { deps: ActionExecutionDeps }]> = [ |
| 124 | + ['without the protocol slot (ObjectQL fallback)', fallbackHarness], |
| 125 | + ['with the protocol slot', protocolHarness], |
| 126 | +]; |
| 127 | + |
| 128 | +/** Capture a rejection as plain data so two of them can be compared. */ |
| 129 | +async function rejection(p: Promise<unknown>) { |
| 130 | + try { |
| 131 | + const resolved = await p; |
| 132 | + return { rejected: false as const, resolved }; |
| 133 | + } catch (e) { |
| 134 | + return { rejected: true as const, error: e as any }; |
| 135 | + } |
| 136 | +} |
| 137 | + |
| 138 | +// --------------------------------------------------------------------------- |
| 139 | +// 1. `batch` is refused, and refused like everything else unknown |
| 140 | +// --------------------------------------------------------------------------- |
| 141 | + |
| 142 | +describe('batch is refused with the unknown-action answer (#5856)', () => { |
| 143 | + it.each(DEPLOYMENTS)('%s → 400 Unknown data action: batch', async (_label, harness) => { |
| 144 | + const { deps } = harness(); |
| 145 | + await expect( |
| 146 | + callData(deps, REQ, 'batch', { object: 'task' }, undefined, undefined, EC), |
| 147 | + ).rejects.toEqual({ statusCode: 400, message: 'Unknown data action: batch' }); |
| 148 | + }, 60_000); |
| 149 | + |
| 150 | + it('answers `batch` in the SAME shape as any other unknown action', async () => { |
| 151 | + // The claim the issue is about, stated as an identity rather than as a |
| 152 | + // literal: `batch` is no longer a special case of anything. Only the |
| 153 | + // action name may differ between the two rejections. |
| 154 | + const { deps } = fallbackHarness(); |
| 155 | + const forBatch = await rejection(callData(deps, REQ, 'batch', { object: 'task' }, undefined, undefined, EC)); |
| 156 | + const forOther = await rejection(callData(deps, REQ, 'frobnicate', { object: 'task' }, undefined, undefined, EC)); |
| 157 | + |
| 158 | + expect(forBatch.rejected).toBe(true); |
| 159 | + expect(forOther.rejected).toBe(true); |
| 160 | + expect(Object.keys(forBatch.error).sort()).toEqual(Object.keys(forOther.error).sort()); |
| 161 | + expect(forBatch.error.statusCode).toBe(forOther.error.statusCode); |
| 162 | + expect(forBatch.error.message.replace('batch', 'X')).toBe(forOther.error.message.replace('frobnicate', 'X')); |
| 163 | + }, 60_000); |
| 164 | + |
| 165 | + it('never answers a 200 whose body reads as "the batch ran and matched nothing"', async () => { |
| 166 | + // The was-red assertion in its narrowest form. `{ results: [] }` is |
| 167 | + // indistinguishable from a real empty batch, which is what made this |
| 168 | + // worse than a 501: nothing downstream can tell the two apart. |
| 169 | + for (const [, harness] of DEPLOYMENTS) { |
| 170 | + const outcome = await rejection( |
| 171 | + callData(harness().deps, REQ, 'batch', { object: 'task' }, undefined, undefined, EC), |
| 172 | + ); |
| 173 | + expect(outcome.rejected).toBe(true); |
| 174 | + expect(outcome).not.toMatchObject({ resolved: { results: [] } }); |
| 175 | + } |
| 176 | + }, 60_000); |
| 177 | +}); |
| 178 | + |
| 179 | +// --------------------------------------------------------------------------- |
| 180 | +// 2. Positive control — the actions `callData` DOES serve are untouched |
| 181 | +// --------------------------------------------------------------------------- |
| 182 | + |
| 183 | +describe('the served actions still answer (positive control)', () => { |
| 184 | + it.each(DEPLOYMENTS)('%s → query lists, get reads', async (_label, harness) => { |
| 185 | + const { deps } = harness(); |
| 186 | + const list: any = await callData(deps, REQ, 'query', { object: 'task', query: {} }, undefined, undefined, EC); |
| 187 | + expect(list.object).toBe('task'); |
| 188 | + expect(list.records).toEqual([{ id: 'r1', title: 'one' }]); |
| 189 | + |
| 190 | + const one: any = await callData(deps, REQ, 'get', { object: 'task', id: 'r1' }, undefined, undefined, EC); |
| 191 | + expect(one).toMatchObject({ object: 'task', id: 'r1', record: { id: 'r1', title: 'one' } }); |
| 192 | + }, 60_000); |
| 193 | +}); |
| 194 | + |
| 195 | +// --------------------------------------------------------------------------- |
| 196 | +// 3. The two upstream facts that made the arm unreachable |
| 197 | +// --------------------------------------------------------------------------- |
| 198 | + |
| 199 | +describe('nothing upstream can spell `batch` (#5856 enumeration)', () => { |
| 200 | + it('the dispatcher’s `/data` domain declines `/data/:object/batch` — it routes only `query`', async () => { |
| 201 | + // `handleDataRequest` compares `parts[1]` against the literal 'query' |
| 202 | + // and otherwise reads it as a record id, so this POST matches no branch |
| 203 | + // and the domain DECLINES it (`handled: false`). This is the upstream |
| 204 | + // constraint the removed arm was relying on for its safety. |
| 205 | + // |
| 206 | + // Note what this does NOT say: `POST /data/:object/batch` is a real |
| 207 | + // endpoint — `@objectstack/rest` mounts it (`registerBatchEndpoints`, |
| 208 | + // `rest-server.ts`), together with the cross-object `POST /batch`. |
| 209 | + // That is the point of route-ownership rule 1: batching has one owner, |
| 210 | + // and a host that wants it mounts REST. What is pinned here is that |
| 211 | + // THIS domain is not a second owner of the same path. |
| 212 | + const h = fallbackHarness(); |
| 213 | + const resolve = (name: string) => |
| 214 | + name === 'objectql' ? h.ql |
| 215 | + : name === 'metadata' ? h.services.metadata |
| 216 | + : name === 'auth' ? { api: { getSession: async () => ({ user: { id: 'u1' } }) } } |
| 217 | + : undefined; |
| 218 | + const kernel: any = { getService: resolve, getServiceAsync: async (n: string) => resolve(n) }; |
| 219 | + const dispatcher = new HttpDispatcher(kernel); |
| 220 | + |
| 221 | + const res: any = await dispatcher.dispatch('POST', '/data/task/batch', { operations: [] }, {}, { request: {} } as HttpProtocolContext); |
| 222 | + expect(res.handled).toBe(false); |
| 223 | + |
| 224 | + // The sibling that IS routed, so the assertion above cannot pass by the |
| 225 | + // whole domain being broken. |
| 226 | + const served: any = await dispatcher.dispatch('POST', '/data/task/query', {}, {}, { request: {} } as HttpProtocolContext); |
| 227 | + expect(served.handled).toBe(true); |
| 228 | + expect(served.response.status).toBe(200); |
| 229 | + }, 60_000); |
| 230 | + |
| 231 | + it('a declared endpoint cannot ask for `batch` — the operation enum is closed', () => { |
| 232 | + // `endpoint-executor.ts`'s `ObjectOperation` is this enum, so the |
| 233 | + // declarative-endpoint path can only ever hand `callData` one of five |
| 234 | + // literals. Publish rejects the rest. |
| 235 | + const declare = (operation: string) => |
| 236 | + ApiEndpointSchema.safeParse({ |
| 237 | + name: 'task_batch', |
| 238 | + path: '/api/v1/apps/showcase/task', |
| 239 | + method: 'POST', |
| 240 | + type: 'object_operation', |
| 241 | + target: 'task', |
| 242 | + objectParams: { object: 'task', operation }, |
| 243 | + }); |
| 244 | + |
| 245 | + expect(declare('batch').success).toBe(false); |
| 246 | + expect(declare('create').success).toBe(true); |
| 247 | + }); |
| 248 | +}); |
0 commit comments