Skip to content

Commit 2bfdc42

Browse files
fix(runtime): an api action whose target names its record is refused 400 naming recordId when called without one, never forwarded to a path with a hole in it (#29005)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XL51wryBuAmngcxc4MhBYSV)_ Fixes #28932 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:`type: 'api'` 动作的 `target` 里带记录占位(`{id}`、`${ctx.recordId}`、`{字段}`)时,调用方没传 `recordId`(或传了空串),动作入口现在直接回 400 `VALIDATION_ERROR`,消息点名缺的是 `recordId`(记录上缺字段则点名那个字段),不再转发。REST 动作入口和 MCP `run_action` 共用同一处判断。 - **为什么改**:原来占位被清空或原样留在路径里转发出去,路由匹配不上,调用方收到 404 `ENDPOINT_NOT_FOUND`「端点不存在」,会以为路由坏了,而不是自己漏了参数。 - **风险与回滚**:低。只把原来就失败的调用(404)改成 400 并说清原因;带真实 id 的调用(200)和未知 id 的调用(目标自己的 404 `REQUEST_NOT_FOUND`)照旧转发。回滚即撤销本 PR。 - **先看**:`packages/runtime/src/action-execution.ts`(`buildApiActionRequest` 第 1 步路径拼装里新增的两处拒收和两个小函数);`packages/runtime/src/domains/api-action-forward.test.ts`(文件末尾新增的一组用例);`packages/verify/src/action-door-api-forward-denied-subject.test.ts`(真实启动栈上的两条新用例)。 - **两分钟验证**:`pnpm --filter @objectstack/runtime exec vitest run src/domains/api-action-forward.test.ts`,预期 73/73 通过(main 上新增用例 4 条红)。 - **需要你做的**:无,席位落地。 ## Why An integration calling a record-anchored `api` action over REST or MCP without its `recordId` was told the endpoint does not exist (404 `ENDPOINT_NOT_FOUND`), so the cost of finding a missing parameter landed on the caller. The door now says which input is missing. ## What changes **The seam.** `buildApiActionRequest` (`packages/runtime/src/action-execution.ts`) assembles the forward for a same-origin `type: 'api'` action, and both action doors call it: REST `POST /api/v1/actions/:object/:action` (`domains/actions.ts`) and the MCP `run_action` bridge (`invokeBusinessAction`). Step 1 of it, the path, now refuses a record token it cannot fill, before anything is forwarded: - **`${ctx.recordId}` left unresolved.** An empty `recordId` now counts as none (`input.recordId || record.id || undefined`), so `${ctx.recordId}` stays in place instead of being blanked to `''`. It is refused naming `recordId`, ahead of the browser-only token refusal that would otherwise claim it with a sentence saying the server *does* interpolate `${ctx.recordId}`. - **`{field}` left unresolved.** A field the row has no value for (`null`, `undefined` or `''`) is left in place, never blanked. After the browser-only refusal (so a `{{page.x}}` keeps that refusal), any `{field}` token still in the path is refused: naming `recordId` when the caller named no record, otherwise naming the field. Without a row, `{id}` is filled from the routed `recordId`, the same source `recordIdParam` already uses. Both doors always pass a row, so that branch is reachable only by a direct caller of the function. - **The refusal shape is the existing one.** The function returns `{ status: 400, message }`, as its other refusals do. The REST door answers it through `deps.error(message, 400)` and the MCP door throws it with `standardErrorCodeForHttpStatus(400)`, so both answer `VALIDATION_ERROR`. No new error code. No runtime string carries a tracker number. - **Changeset** `.changeset/api-action-missing-record-id.md`: `patch` on `@objectstack/runtime`, with both declaration lines. **Step 1 measurement (the order's ruling 2).** Every `api` action in this repo whose `target` carries a `{field}` token names `{id}` alone: the nine `sys_approval_request` decision actions in `packages/plugins/plugin-approvals/src/sys-approval-request.object.ts` (`approve`, `reject`, `reassign`, `revise`, `request-info`, `remind`, `recall`, `resubmit`, `comment`) and the two `sys_api_key` actions in `packages/platform-objects/src/identity/sys-api-key.object.ts`. `ui/apps/console/src/dev/DevRecordHeaderActions.tsx` has one more `{id}` target (a dev-only page). Two actions use `${ctx.recordId}` instead: `packages/services/service-datasource/src/datasource-admin-plugin.ts` (`/test`, `/migrate-credential`). `examples/` has no record-token api target; its only `{{categoryId}}` hit is a QA step in `examples/app-showcase/qa/platform-smoke.test.json`, not an action. hotcrm (`d4ffe99`) has none. ## Tests - **Unit, new block at the end of `packages/runtime/src/domains/api-action-forward.test.ts`** (5 tests): - REST: the approve shape (`/api/v1/approvals/requests/{id}/approve`) with `{ params: {} }` and with `{ recordId: '', params: {} }`: 400, code `VALIDATION_ERROR` (pinned against `standardErrorCodeForHttpStatus(400)` and literally), the message names `` `recordId` ``, and the in-process request is never called. - REST: a `${ctx.recordId}` target (a `DELETE`) and a `{email}` target, each with no id and with `''`: the same refusal, nothing forwarded. - REST controls: a real id is forwarded (200, the id in the URL); an unknown id is forwarded and the target's 404 `REQUEST_NOT_FOUND` is relayed; a target with no token is forwarded with no id (200). - MCP `run_action`: the approve shape with no id and with `''` is thrown 400 `VALIDATION_ERROR` naming `recordId`, nothing forwarded. - `buildApiActionRequest`: a row without `email`, with `email: null` and with `email: ''` is refused naming `` `email` ``; no row at all names `recordId`; `{id}` without a row is filled from the routed id, percent-encoded. - **Real stack, two new cases in `packages/verify/src/action-door-api-forward-denied-subject.test.ts`** (booted stack, real approvals routes, a non-admin pending approver): - `POST /api/v1/actions/sys_approval_request/approval_approve` with `{ params: {} }` and with `{ recordId: '' }`: 400 `VALIDATION_ERROR` naming `recordId`, and the request is untouched (no approve row, slate unchanged). - Control: an id that names no request answers the decision route's 404 `REQUEST_NOT_FOUND`. The existing case in the same file already pins the real-id 200. - **Red on base `f7c545ad`.** Unit file with `action-execution.ts` checked out from base: 4 failed, 69 passed (the four refusal tests; all controls green). Verify file with runtime `dist` rebuilt from base: 1 failed, 4 passed, the failure reading `{"code":"ENDPOINT_NOT_FOUND","message":"Not found","httpStatus":404}`, which is the card's repro. Both trees restored: `git status --porcelain` empty, blob equal to HEAD, runtime rebuilt. - **Green on head.** Unit file 73/73. Verify file 5/5. <details><summary>Ablation (one-off, from committed HEAD)</summary> Both guard lines replaced through `node scripts/ablation-replace.mjs` (anchor 1 → 0 each): the `${ctx.recordId}` check by one that tests for the marker `ABLATED-28932-A`, and the `{field}` check by one that tests for `ABLATED-28932-B`, so the helper stays referenced and the build still type-checks. - Unit file (reads `action-execution.ts` from source): 4 failed, 69 passed, the same four tests as on base. - Verify file (reads runtime `dist`): `pnpm --filter @objectstack/runtime build`, then `node scripts/ablation-dist-preflight.mjs @objectstack/runtime 'ABLATED-28932-B'` answered "marker present in 2 built files". 1 failed, 4 passed. With the guard ablated, the `{id}` token is no longer blanked, so the forward reaches the route with `{id}` literally and the route answers 400 `FILTER_TOKEN_UNKNOWN`; the pin still reds because the code is not `VALIDATION_ERROR`. - Restore: `git checkout HEAD -- PATH`, `git diff HEAD` 0 bytes, blob `12811782c09a` equal to HEAD; rebuilt, and `ablation-dist-preflight.mjs --absent` answered "marker absent from all 12 built files" and "working tree clean against HEAD". </details> ## Acceptance notes - **Accept set.** Every call refused now was already a failure on `main`: a forward to a path with a hole in it, answered 404 by the router (or, for an `{id}` the approvals route read literally, 400 `FILTER_TOKEN_UNKNOWN`). Hence `Clause-②: no`. One odd shape moves from forwarded to refused: a target written with a scope-less `${name}` or `{{name}}` whose `{name}` part matched no row field used to be blanked to `$` or `{}` and forwarded; it now stays in place and gets the existing browser-only refusal. No such target exists in this repo, `examples/` or hotcrm (measurement above). - **The console path is not touched.** `ui/` resolves its own click-time tokens; this PR changes only the server's forward. - **Ordering of refusals.** The record-token refusal runs before the same-origin check and the body checks, so a call that is missing its id is told that first. ## Verification <details><summary>Package tests and typecheck</summary> All readings at head `f71c6e23` (base `f7c545ad`), after `pnpm install --frozen-lockfile` and `pnpm --filter '@objectstack/verify^...' build` (which covers the runtime closure). - `pnpm --filter @objectstack/runtime test`: 513 files passed, 8272 tests passed, 19 skipped. Lock verdict `command-exit 0`. - `pnpm --filter @objectstack/verify exec vitest run src/action-door-api-forward-denied-subject.test.ts`: 5/5 passed. - `pnpm --filter @objectstack/runtime typecheck`: exit 0, `check:test-typecheck: OK` with its ledger held (27 files / 188 errors / 68 signatures, unchanged). - `pnpm --filter @objectstack/verify typecheck`: exit 0, `check:test-typecheck: OK` (0 / 0 / 0). - `eslint --no-inline-config packages/runtime packages/verify`: exit 0. </details> <details><summary>Derived gates</summary> - `node scripts/pm/dispatch-gates.mjs --commands` at `f71c6e23`: 61 commands. 60 ran and exited 0. - `dispatch-gates --ran`: 61 derived, 60 run, 1 NOT-MEASURED, 0 UNRUN. - NOT MEASURED: `check:dual-build-cjs-loads`, reason: exit 3, PREREQUISITE NOT MET (it needs a full repo build; only the verify closure was built). CI runs it. - `check-plugin-teardown-shape --self-test` first answered exit 3 on the shallow clone; after `git fetch --depth=1` of the commit it reads, it answered exit 0 (48 cases), and that is the code recorded. - Not run locally, as the os-dev scope says: `test:repo`, the repo-wide lint, and the families outside the derivation. The PR's CI check-runs carry them. - No dogfood battery: the title has no `!`, and `Clause-②` has no `(narrowing)` arm. </details> ## Human Minimum None: the seat reviews and lands it. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5nNCtq276yJtjqKq8B)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 462b4f9 commit 2bfdc42

4 files changed

Lines changed: 196 additions & 7 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): an `api` action whose `target` names its record is refused `400` naming `recordId` when called with no record id, instead of being forwarded to a path that reaches no route
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A same-origin `type: 'api'` action whose `target` carries a record token (`{id}`, `${ctx.recordId}`, or a `{field}` read from the record, for example `approval_approve` → `/api/v1/approvals/requests/{id}/approve`) used to be forwarded even when the caller gave no `recordId`, or an empty one: the token was blanked or left as written, the forwarded path matched no route, and the action door answered `404 ENDPOINT_NOT_FOUND`, telling the caller the endpoint did not exist.
11+
12+
Both action doors (REST `POST /api/v1/actions/:object/:action` and the MCP `run_action` tool) now refuse that call `400 VALIDATION_ERROR` before any forward, naming the missing input: `recordId` when no record was named, otherwise the field the record has no value for. A call with a real id, and a call with an id the target does not know, are forwarded and answered by the target as before.

‎packages/runtime/src/action-execution.ts‎

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,31 @@ export function foldsApiDecisionOutputs(action: any): boolean {
13311331
return !isDeclarativeUpdateAction(action) && action?.type === 'api' && apiBodyWrap(action) === undefined;
13321332
}
13331333

1334+
/**
1335+
* Which record input does this partly built forward `path` still lack, by its
1336+
* `{field}` tokens? `recordId` when the caller named no record, otherwise the
1337+
* field the record has no value for; `undefined` when every token resolved.
1338+
* Read after `${…}` and `{{…}}` are refused, so a brace pair left here is a
1339+
* `{field}` token.
1340+
*/
1341+
function unresolvedApiTargetRecordInput(path: string, routedRecordId: unknown): string | undefined {
1342+
const match = /\{([a-z_][a-z0-9_]*)\}/i.exec(path);
1343+
if (!match) return undefined;
1344+
return routedRecordId === undefined ? 'recordId' : match[1];
1345+
}
1346+
1347+
/** The refusal for a `target` record token the dispatch inputs left unresolved: nothing is forwarded. */
1348+
function unresolvedApiTargetRecordRefusal(name: string, target: string, missing: string): ApiActionRequestRefusal {
1349+
return {
1350+
status: 400,
1351+
message: missing === 'recordId'
1352+
? `Action '${name}' has a \`target\` ('${target}') that names its record, and no \`recordId\` was given. ` +
1353+
`Pass the id of the record to run it on as \`recordId\`. The request was not sent.`
1354+
: `Action '${name}' has a \`target\` ('${target}') that reads \`${missing}\` from its record, and this ` +
1355+
`record has no value for it. The request was not sent.`,
1356+
};
1357+
}
1358+
13341359
/**
13351360
* Assemble the request a same-origin `type: 'api'` action makes — method,
13361361
* path and JSON body — from its DECLARATION and the dispatch inputs, by the
@@ -1341,6 +1366,10 @@ export function foldsApiDecisionOutputs(action: any): boolean {
13411366
* the caller (`ctx.recordId`, `ctx.user.id`, `ctx.org.id`; `ctx.origin` /
13421367
* `ctx.apiBase` are `''` on the server, as they are for a same-origin
13431368
* console), values percent-encoded; then `{field}` from the subject row.
1369+
* A record token neither can fill — no `recordId` given (an empty one is
1370+
* none), or a row with no value for the field — is REFUSED naming the
1371+
* missing input, never blanked or forwarded as written: that path reaches
1372+
* no route, and its 404 would say the endpoint does not exist.
13441373
* 2. The body: the params bag, flat or under `bodyShape.wrap`; the `outputs.*`
13451374
* fold (#3447); `recordIdParam` seeded from the subject row's
13461375
* `recordIdField` (default `id`) — or REFUSED when the row cannot supply it
@@ -1373,23 +1402,37 @@ export function buildApiActionRequest(
13731402
: undefined;
13741403

13751404
// ── 1. the path ──
1405+
// An empty id is no id: `${ctx.recordId}` and `{id}` stay unresolved, and
1406+
// are refused below by name, rather than blanked into a path naming no record.
1407+
const routedRecordId: unknown = input.recordId || (input.record as any)?.id || undefined;
13761408
let path = substituteApiTargetTokens(target, {
13771409
param: input.params,
13781410
ctx: {
13791411
origin: '',
13801412
apiBase: '',
1381-
recordId: input.recordId ?? (input.record as any)?.id,
1413+
recordId: routedRecordId,
13821414
user: { id: ec.userId, email: ec.email },
13831415
org: { id: organizationId },
13841416
},
13851417
});
1386-
if (input.record && /\{[a-z_][a-z0-9_]*\}/i.test(path)) {
1387-
const row = input.record;
1388-
path = path.replace(/\{([a-z_][a-z0-9_]*)\}/gi, (_m, k: string) => {
1389-
const v = row[k];
1390-
return v == null ? '' : encodeURIComponent(String(v));
1418+
if (/\{[a-z_][a-z0-9_]*\}/i.test(path)) {
1419+
// Without a row only `{id}` has a source: the routed `recordId`, as
1420+
// for `recordIdParam` below. A field with no value is LEFT IN PLACE,
1421+
// never blanked: the refusal below names it, and nothing is forwarded.
1422+
const row: Record<string, unknown> = input.record && typeof input.record === 'object' ? input.record : {};
1423+
path = path.replace(/\{([a-z_][a-z0-9_]*)\}/gi, (m, k: string) => {
1424+
const v = row[k] ?? (k === 'id' ? routedRecordId : undefined);
1425+
return v == null || v === '' ? m : encodeURIComponent(String(v));
13911426
});
13921427
}
1428+
// A record token left unresolved — the caller named no record, or the
1429+
// record has no value for the field the path reads — is refused BEFORE the
1430+
// forward, naming the missing input: a forward with a hole in its path
1431+
// reaches no route, and its 404 would tell the caller the endpoint does
1432+
// not exist instead of what they did not send. `${ctx.recordId}` is read
1433+
// here, ahead of the browser-only refusal below, which would otherwise
1434+
// claim it; a `{field}` token after it, so a `{{page.x}}` keeps that one.
1435+
if (path.includes('${ctx.recordId}')) return unresolvedApiTargetRecordRefusal(name, target, 'recordId');
13931436
if (/\$\{[\w.]+\}|\{\{/.test(path)) {
13941437
return {
13951438
status: 400,
@@ -1400,6 +1443,8 @@ export function buildApiActionRequest(
14001443
`exists only in the browser). Pass the value as a param, or call the target from the console.`,
14011444
};
14021445
}
1446+
const missingRecordInput = unresolvedApiTargetRecordInput(path, routedRecordId);
1447+
if (missingRecordInput) return unresolvedApiTargetRecordRefusal(name, target, missingRecordInput);
14031448
if (!isSameOriginApiTarget(path)) {
14041449
return { status: 400, message: `Action '${name}' interpolated its \`target\` to '${path}', which is not a same-origin path.` };
14051450
}

‎packages/runtime/src/domains/api-action-forward.test.ts‎

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ const FLAG_BY_EMAIL = {
9999
ai: { exposed: true, description: 'Flag the lead by email.' },
100100
};
101101

102+
/** An api action whose `target` carries no record token. */
103+
const PING_LOCAL = {
104+
name: 'ping_local', label: 'Ping', objectName: OBJECT, type: 'api', method: 'POST',
105+
target: '/api/v1/apps/showcase/ping', ai: { exposed: true, description: 'Ping the app.' },
106+
};
107+
102108
type Injected = { url: string; init: RequestInit };
103109

104110
function boot(opts: {
@@ -122,7 +128,7 @@ function boot(opts: {
122128
});
123129
const server = { getRawApp: () => ({ request }) };
124130
const executeAction = vi.fn(async () => ({ ran: 'script' }));
125-
const objectDef = { name: OBJECT, actions: [BAN_LEAD, REMOTE_PING, DELETE_LEAD, RECALC_LEAD, APPROVE_LEAD, NOTE_OWNER, FLAG_BY_EMAIL] };
131+
const objectDef = { name: OBJECT, actions: [BAN_LEAD, REMOTE_PING, DELETE_LEAD, RECALC_LEAD, APPROVE_LEAD, NOTE_OWNER, FLAG_BY_EMAIL, PING_LOCAL] };
126132
const ql: any = {
127133
executeAction,
128134
getSchema: (name: string) => (name === OBJECT ? objectDef : undefined),
@@ -1342,3 +1348,104 @@ describe('[#28657] a denied subject load forwards an api action whose request ne
13421348
expect(apiForwardSubjectRecord('u_target', delivered)).toBe(delivered.record);
13431349
});
13441350
});
1351+
1352+
// An api action whose `target` names its record (`{id}`, `${ctx.recordId}`,
1353+
// `{field}`) called with no record id, or an empty one, used to be forwarded
1354+
// with the token blanked or left as written: the path reached no route and the
1355+
// door answered the router's 404 ENDPOINT_NOT_FOUND, telling the caller the
1356+
// endpoint does not exist. It is refused 400 VALIDATION_ERROR naming the
1357+
// missing input, before any forward. The real-stack half, on the real
1358+
// approvals routes, is `packages/verify/src/action-door-api-forward-denied-subject.test.ts`.
1359+
describe('an api action whose target names its record, called with no record id, is refused naming `recordId`', () => {
1360+
const runViaMcp = async (h: ReturnType<typeof boot>, name: string, input: any = {}) => {
1361+
const deps: any = {
1362+
resolveService: async (_ctx: unknown, service: string) =>
1363+
service === 'metadata' ? h.metadata
1364+
: service === 'automation' ? null
1365+
: service === 'http.server' ? { getRawApp: () => ({ request: h.request }) }
1366+
: h.ql,
1367+
getObjectQL: async () => h.ql,
1368+
};
1369+
return invokeBusinessAction(deps, CTX() as any, name, input, {
1370+
driver: h.ql,
1371+
ec: { userId: 'u_agent', tenantId: 'org_1', systemPermissions: [] },
1372+
getMeta: () => h.metadata,
1373+
callData: async (_a: string, p: any) => ({ object: p?.object, id: p?.id, record: { id: p?.id, email: 'target@example.test' } }),
1374+
});
1375+
};
1376+
1377+
it('REST: the approve shape with no `recordId`, or an empty one, answers 400 VALIDATION_ERROR naming it, and nothing is forwarded', async () => {
1378+
for (const body of [{ params: {} }, { recordId: '', params: {} }]) {
1379+
const { dispatcher, request } = boot();
1380+
1381+
const res = await dispatcher.handleActions(`/${OBJECT}/${APPROVE_LEAD.name}`, 'POST', body, CTX());
1382+
1383+
expect(statusOf(res), JSON.stringify(body)).toBe(400);
1384+
expect(codeOf(res), JSON.stringify(body)).toBe(standardErrorCodeForHttpStatus(400));
1385+
expect(codeOf(res)).toBe('VALIDATION_ERROR');
1386+
expect(messageOf(res)).toContain('`recordId`');
1387+
expect(request, JSON.stringify(body)).not.toHaveBeenCalled();
1388+
}
1389+
});
1390+
1391+
it('REST: a `${ctx.recordId}` target and a `{field}` target are refused the same way with no record id', async () => {
1392+
for (const action of [DELETE_LEAD, NOTE_OWNER]) {
1393+
for (const body of [{}, { recordId: '' }]) {
1394+
const { dispatcher, request } = boot();
1395+
1396+
const res = await dispatcher.handleActions(`/${OBJECT}/${action.name}`, 'POST', body, CTX());
1397+
1398+
expect(statusOf(res), `${action.name} ${JSON.stringify(body)}`).toBe(400);
1399+
expect(codeOf(res)).toBe('VALIDATION_ERROR');
1400+
expect(messageOf(res)).toContain('`recordId`');
1401+
expect(request).not.toHaveBeenCalled();
1402+
}
1403+
}
1404+
});
1405+
1406+
it('REST, the controls: a real id is forwarded (200), an unknown id gets the target\'s 404 REQUEST_NOT_FOUND, a tokenless target is forwarded with no id', async () => {
1407+
const real = boot();
1408+
const ok = await real.dispatcher.handleActions(`/${OBJECT}/${APPROVE_LEAD.name}`, 'POST', { recordId: 'u_target', params: {} }, CTX());
1409+
expect(statusOf(ok), JSON.stringify(ok?.response?.body)).toBe(200);
1410+
expect(real.injected[0].url).toBe('http://app.example.test/api/v1/approvals/requests/u_target/approve');
1411+
1412+
const unknown = boot({ answer: { status: 404, body: { code: 'REQUEST_NOT_FOUND', error: "request 'areq_none' not found" } } });
1413+
const missing = await unknown.dispatcher.handleActions(`/${OBJECT}/${APPROVE_LEAD.name}`, 'POST', { recordId: 'areq_none', params: {} }, CTX());
1414+
expect(statusOf(missing)).toBe(404);
1415+
expect(codeOf(missing)).toBe('REQUEST_NOT_FOUND');
1416+
expect(unknown.injected[0].url).toBe('http://app.example.test/api/v1/approvals/requests/areq_none/approve');
1417+
1418+
const ping = boot();
1419+
const pinged = await ping.dispatcher.handleActions(`/${OBJECT}/${PING_LOCAL.name}`, 'POST', { params: {} }, CTX());
1420+
expect(statusOf(pinged), JSON.stringify(pinged?.response?.body)).toBe(200);
1421+
expect(ping.injected[0].url).toBe('http://app.example.test/api/v1/apps/showcase/ping');
1422+
});
1423+
1424+
it('MCP `run_action`: the approve shape with no `recordId`, or an empty one, is thrown 400 VALIDATION_ERROR naming it, and nothing is forwarded', async () => {
1425+
for (const input of [{ params: {} }, { recordId: '', params: {} }]) {
1426+
const h = boot();
1427+
1428+
const thrown = await runViaMcp(h, APPROVE_LEAD.name, input).catch((e) => e);
1429+
1430+
expect((thrown as any).status, JSON.stringify(input)).toBe(400);
1431+
expect((thrown as any).code).toBe('VALIDATION_ERROR');
1432+
expect(String((thrown as any).message)).toContain('`recordId`');
1433+
expect(h.request).not.toHaveBeenCalled();
1434+
}
1435+
});
1436+
1437+
it('buildApiActionRequest: a row with no value for a `{field}` token is refused naming that field, never blanked', () => {
1438+
const ec = { userId: 'u1', tenantId: 'org_9' };
1439+
for (const record of [{ id: 'r1' }, { id: 'r1', email: null }, { id: 'r1', email: '' }]) {
1440+
const refused = buildApiActionRequest(NOTE_OWNER, { recordId: 'r1', record, params: {}, ec });
1441+
expect((refused as any).status, JSON.stringify(record)).toBe(400);
1442+
expect((refused as any).message).toContain('`email`');
1443+
}
1444+
// No record at all: the caller named none, so the missing input is `recordId`.
1445+
const none = buildApiActionRequest(NOTE_OWNER, { record: {}, params: {}, ec });
1446+
expect((none as any).message).toContain('`recordId`');
1447+
// `{id}` without a row is filled from the routed id, as `recordIdParam` is.
1448+
const rowless = buildApiActionRequest(APPROVE_LEAD, { recordId: 'areq 1', params: {}, ec });
1449+
expect((rowless as any).path).toBe('/api/v1/approvals/requests/areq%201/approve');
1450+
});
1451+
});

‎packages/verify/src/action-door-api-forward-denied-subject.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,4 +219,29 @@ describe('a pending approver decides through the action door — the forward aut
219219
expect(await approveRows(requestId)).toEqual([]);
220220
expect(pendingOf(await requestRow(requestId))).toEqual([FIRST, SECOND]);
221221
});
222+
223+
it('Approve with no record id, or an empty one, is refused 400 naming `recordId` — never forwarded to a path with a hole in it', async () => {
224+
const requestId = await openRequest('INV-3');
225+
226+
for (const body of [{ params: {} }, { recordId: '', params: {} }]) {
227+
const answer = await call(first, 'POST', '/actions/sys_approval_request/approval_approve', body);
228+
229+
// The route's 404 ENDPOINT_NOT_FOUND would say the endpoint does not exist.
230+
expect(answer.status, answer.text).toBe(400);
231+
expect(answer.json?.error?.code, answer.text).toBe('VALIDATION_ERROR');
232+
expect(answer.json?.error?.message, answer.text).toContain('`recordId`');
233+
}
234+
// Nothing reached the decision route.
235+
expect(await approveRows(requestId)).toEqual([]);
236+
expect(pendingOf(await requestRow(requestId))).toEqual([FIRST, SECOND]);
237+
});
238+
239+
it('the control: an id that names no request is still forwarded, and the decision route\'s 404 REQUEST_NOT_FOUND is the answer', async () => {
240+
const answer = await call(first, 'POST', '/actions/sys_approval_request/approval_approve', {
241+
recordId: 'areq_does_not_exist', params: {},
242+
});
243+
244+
expect(answer.status, answer.text).toBe(404);
245+
expect(answer.json?.error?.code, answer.text).toBe('REQUEST_NOT_FOUND');
246+
});
222247
});

0 commit comments

Comments
 (0)