Skip to content

Commit 0aac0bf

Browse files
test(runtime): pin the api action door's 2xx reading of a { success: false } answer (#28007)
domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F <!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuPMZ8BHWtPf6nBTp51qSM64)_ Fixes #27918 Clause-②: no Metadata-protocol: none Skills-docs: none — no behaviour changes; no skill or doc describes how this door reads a 2xx body ## 维护者速读 - **改了什么**:`type: 'api'` 动作转发门对目标 2xx 答复的读法保持原样,只把它钉住:docblock 写明"2xx 一律按成功读,不看 body 的 success",并加了单元测试 + 一个门级测试。 - **为什么改**:分诊要求先实测。逐一核对仓内每个同源 `type: 'api'` 动作目标后,没有任何一个会以 2xx 答顶层 `success: false`(拒绝全部是非 2xx),所以按分诊结论"钉住现状并关卡"。 - **风险与回滚**:零运行时风险:生产代码只改了注释,其余是测试。回滚即 revert 本 PR。 - **先看**:`packages/runtime/src/action-execution.ts`(`apiActionResultOf` 的 docblock,实测清单的结论在这里)→ `packages/runtime/src/domains/api-action-forward.test.ts`(新增 4 个用例)。 - **两分钟验证**:`pnpm --filter @objectstack/runtime exec vitest run src/domains/api-action-forward.test.ts`,预期 29 passed。 - **需要你做的**:无,席位落地。 ## What changes Measure-first, per triage 6515335719. **No same-origin `type: 'api'` target in this repository answers HTTP 2xx with a top-level `success: false`**, so today's reading is pinned rather than changed: - `apiActionResultOf` docblock: a 2xx is read as success whatever its body says, and why (the measured fact, plus "a target that starts answering that shape has the defect itself; the remedy goes in that target, never a body check here"). - `api-action-forward.test.ts`: a door-level case through `handleActions` (200 `{ success: false, data }` → 200 `data`; 200 `{ success: false, message }` → 200 with the body as `data`) and three unit cases through the real `apiActionResultOf` export. No production behaviour changes. `skip-changeset`: the only `src` edit is a comment (fast lane). ## Acceptance notes <details><summary>The measured census (tree: objectstack-ai/objectstack @ 5f6dc96, re-read after merging main @ 4a5396b)</summary> Declarations: `git grep -nE "type:\s*['\"]api['\"]"` outside tests/dist/docs = 244 hits; the action declarations among them name **77 targets** across `packages/platform-objects` (58, all `sys_*` identity actions), `plugin-approvals` (9), `service-datasource` (2, metadata-type actions), `examples/app-showcase` (4), and `ui/` (1 dev harness + 3 grid demos on `/api/demo/*`, which no server serves). JSON/YAML metadata: 0 action declarations. Dogfood fixtures: one test-injected target, `/api/v1/cloud/organizations/{id}/change-slug`, served by the cloud repo (out of this tree). Routes those targets resolve to, and what each answers on a refusal: | Target family | Route owner | `success: false` sites | 2xx `success: false`? | |:--|:--|:--|:--| | `/api/v1/auth/**` objectstack mounts (admin/*, sso/*, sys-session/revoke, sys-oauth-application/register, organization add-member, send-verification-email …) | `packages/plugins/plugin-auth/src/*` | 100 hits; every one carries a literal 4xx/5xx, or a status derived from a vendor `!resp.ok` / a thrown error's status (`mapAuthApiError`, `resolveThrownHttpError`) | none | | `/api/v1/auth/**` stock better-auth routes | `better-auth@1.7.12` dist | non-`true` `success` only on `/organization/has-permission` and `/admin/has-permission` (`{ error: null, success: bool }` at 200) | none targeted: no declared action names either route | | `/api/v1/approvals/requests/{id}/*` (9) | `packages/rest/src/approvals-routes.ts` | `res.json(out)` relays the service result; `ApprovalDecisionResult` / `Recall` / `SendBack` / `Resubmit` / `{ request }` carry no `success` key; refusals go through `handleApprovalError` / 4xx-5xx | none | | `/api/v1/datasources/:name/test`, `/migrate-credential` | `packages/services/service-datasource/src/admin-routes.ts:762,786` | `sendOk` → `{ success: true, data }` always | none | | `/api/v1/keys` | `packages/runtime/src/domains/keys.ts` | 1 (the 405) | none | | `/api/v1/data/sys_api_key/{id}` (PATCH) | `packages/runtime/src/domains/data.ts` | `deps.success` / `deps.error` | none | | `/api/v1/apps/showcase/recalc` (flow endpoint) | `packages/runtime/src/endpoint-executor.ts` | classified refusals → 4xx; the unclassified residual answers 200 `{ success: true, data: { success: false } }`, and the engine's one unclassified exit (flow not found) is pre-probed to 404 | none at top level | | `/api/v1/print/records`, `/api/v1/forms/:slug/submit` | `service-print/src/print-routes.ts`, `rest/src/form-routes.ts` | 0; `sendError` / mapped status | none | Nested `{ success: true, data: { success: false } }` answers (flow residual, a zero-row `DELETE /data`, a package rollback) come back through this door as the same bytes: unwrap `data`, re-wrap with `deps.success`. No live probe was run: the code read is not ambiguous for any row. </details> - **Ride-along not made (deviation).** The card asked to reword the `actions.ts` ledger note in `scripts/check-route-envelope.mjs` ("the non-2xx relay (`deps.error` spread)") "after PR #27915". #27915 is an open draft that has not merged, and on `main` the non-2xx relay still goes through `deps.error` spread (`actions.ts:851`), so the note is accurate today. Rewording it here would make it wrong on `main`. The rewording belongs to #27915, which changes that code. - `git merge-tree` against the three open PRs that touch these files (#27964, #27940, #27915): all merge clean. ## Verification <details><summary>Ablation (scripts/ablation-replace.mjs; the test imports `../action-execution.js`, so src, no dist)</summary> 1. Door reads a 2xx `success: false` as a refusal: `actions.ts` `if (answer.status >= 200 && answer.status < 300) {` → `… && (answer.body as any)?.success !== false) {`. Mutation landed (anchor 1→0, blob da0ea34→0967d680dd2b). **Red**: `Tests 1 failed | 28 passed (29)`, `relays a 2xx { success: false, … } answer as a 200 result` → `expected false to be true`. Restored: blob after == HEAD (da0ea34). 2. Unwrap skips a `success: false` body: `'data' in body && 'success' in body` → `'data' in body && (body as any).success !== false`. Landed (a809a3f→ec5627410e23). **Red**: `Tests 2 failed | 27 passed (29)`: the door case and the `{ success: false, data }` unit case, `expected { success: false, …(1) } to deeply equal { reason: 'already_banned' }`. Restored: blob after == HEAD (a809a3f). </details> <details><summary>Local gates @ f442344</summary> - `pnpm --filter @objectstack/runtime test`: Test Files 482 passed (482) · Tests 7844 passed, 19 skipped · VERDICT command-exit 0 - `pnpm --filter @objectstack/runtime typecheck` (tsc + check:test-typecheck): VERDICT command-exit 0 - `eslint --no-inline-config packages/runtime`: exit 0 - `dispatch-gates --commands` derived 52 families; all run, each exit captured before any pipe; `--ran`: "52 derived famil(ies) accounted for — 51 run, 1 NOT-MEASURED". The one: `pnpm check:dual-build-cjs-loads` exit 3, PREREQUISITE NOT MET (it reads every package's `dist/`, which needs a full `pnpm build`), so NOT MEASURED; CI runs it. - Dogfood battery: n/a (no `!` in the title, no `(narrowing)` arm). </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F Co-authored-by: Claude <noreply@anthropic.com>
1 parent 35fc9b9 commit 0aac0bf

2 files changed

Lines changed: 57 additions & 0 deletions

File tree

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,6 +1553,22 @@ export async function forwardApiAction(
15531553
* envelope (the console's `apiHandler` unwraps the same way, so a
15541554
* `resultDialog` path reads identically on both paths), else the body itself
15551555
* (a stock better-auth answer is not enveloped).
1556+
*
1557+
* [#27918] A 2xx is read as success whatever its body says: `success: false`
1558+
* on a 2xx is not a refusal here, so `{ success: false, data }` answers `data`
1559+
* and `{ success: false, message }` answers the body itself. The status is
1560+
* the target's verdict and this door relays it rather than sniffing a body.
1561+
* Measured 2026-10-30 against every same-origin `type: 'api'` target declared
1562+
* in this repository (the `sys_*` identity actions over better-auth and the
1563+
* plugin-auth mounts, the approvals routes, the datasource admin routes, the
1564+
* data door, the showcase's flow endpoint, print and form routes, and the
1565+
* objectui declarations): none answers a 2xx with a top-level
1566+
* `success: false`. Each refusal is non-2xx, and the nested
1567+
* `{ success: true, data: { success: false } }` that a flow endpoint's
1568+
* unclassified result or a zero-row delete can answer comes back through
1569+
* this door as the same bytes. A target that starts answering 2xx
1570+
* `success: false` has the defect itself (failures speak HTTP, #3962), and
1571+
* the fix goes in that target, never in a body check here (PD #12).
15561572
*/
15571573
export function apiActionResultOf(answer: ApiActionForwardAnswer): unknown {
15581574
const body = answer.body;

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { HttpDispatcher } from '../http-dispatcher.js';
3131
import type { HttpProtocolContext } from '../http-dispatcher.js';
3232
import {
3333
invokeBusinessAction,
34+
apiActionResultOf,
3435
buildApiActionRequest,
3536
isHeadlessInvokableAction,
3637
headlessActionTypeError,
@@ -298,6 +299,29 @@ describe('door 1 — REST `POST /actions/:object/:action` forwards a same-origin
298299
expect(messageOf(res)).toBe('Platform admin only');
299300
});
300301

302+
// [#27918] Today's reading, pinned: the STATUS is the target's verdict, so a
303+
// 2xx is relayed as a result even when its body says `success: false`. No
304+
// same-origin target in the repository answers that shape (the measured
305+
// census is on `apiActionResultOf`'s docblock); one that starts to is fixed
306+
// at the target, so a change here is a decision, not a cleanup.
307+
it('relays a 2xx `{ success: false, … }` answer as a 200 result, never as a refusal', async () => {
308+
const enveloped = boot({ answer: { status: 200, body: { success: false, data: { reason: 'already_banned' } } } });
309+
const viaData = await enveloped.dispatcher.handleActions(
310+
`/${OBJECT}/${BAN_LEAD.name}`, 'POST', { recordId: 'u_target' }, CTX(),
311+
);
312+
expect(statusOf(viaData)).toBe(200);
313+
expect(viaData?.response?.body?.success).toBe(true);
314+
expect(viaData?.response?.body?.data).toEqual({ reason: 'already_banned' });
315+
316+
const bare = boot({ answer: { status: 200, body: { success: false, message: 'not applied' } } });
317+
const viaMessage = await bare.dispatcher.handleActions(
318+
`/${OBJECT}/${BAN_LEAD.name}`, 'POST', { recordId: 'u_target' }, CTX(),
319+
);
320+
expect(statusOf(viaMessage)).toBe(200);
321+
expect(viaMessage?.response?.body?.success).toBe(true);
322+
expect(viaMessage?.response?.body?.data).toEqual({ success: false, message: 'not applied' });
323+
});
324+
301325
it('an OFF-ORIGIN api action is refused 400 before any forward — the browser-only case', async () => {
302326
const { dispatcher, request } = boot();
303327

@@ -342,6 +366,23 @@ describe('door 1 — REST `POST /actions/:object/:action` forwards a same-origin
342366
});
343367
});
344368

369+
describe('apiActionResultOf — a 2xx answer read as the action\'s result', () => {
370+
const at200 = (body: unknown) => ({ status: 200, body, headers: {} });
371+
372+
it('unwraps `data` from a `{ success: false, data }` answer: the 2xx decides, not the flag', () => {
373+
expect(apiActionResultOf(at200({ success: false, data: { reason: 'already_banned' } }))).toEqual({ reason: 'already_banned' });
374+
});
375+
376+
it('answers a `{ success: false, message }` answer (no `data`) as the body itself', () => {
377+
expect(apiActionResultOf(at200({ success: false, message: 'not applied' }))).toEqual({ success: false, message: 'not applied' });
378+
});
379+
380+
it('still unwraps the ordinary success envelope and passes an unenveloped body through', () => {
381+
expect(apiActionResultOf(at200({ success: true, data: { ok: true } }))).toEqual({ ok: true });
382+
expect(apiActionResultOf(at200({ status: true }))).toEqual({ status: true });
383+
});
384+
});
385+
345386
describe('door 2 — the MCP `run_action` bridge forwards a same-origin api action', () => {
346387
const runViaMcp = async (h: ReturnType<typeof boot>, name: string, input: any = {}) => {
347388
const deps: any = {

0 commit comments

Comments
 (0)