From fa101f4ae2235c6a7bdb10732f488454acbc828d Mon Sep 17 00:00:00 2001 From: os-support-ai Date: Thu, 17 Sep 2026 21:54:07 +0000 Subject: [PATCH] fix(rest): the batch door reaches the engine slot through `wiredEngineOrLoud`, so a wired-and-failing engine answers 503 on every wiring (#18559) `objectQLProvider`'s third consumer, `POST /api/v1/batch`, read the field directly, so a rejection escaped the read, missed the adjacent 501 arm and landed in the handler's generic outer catch as 500 INTERNAL_ERROR. Its two sibling consumers answer 503 SERVICE_UNAVAILABLE for the identical fact. Measured at the door on a real RestServer over a real ObjectKernel: the 500 was reachable only on the MULTI-KERNEL wiring. On the single-kernel composition the open core boots, `computeExecCtx` resolves the engine through its own `wiredEngineOrLoud` branch and raises first, so this door already answered 503. The repair removes a wiring-dependent divergence rather than choosing a new wire answer. Both ABSENCE shapes still answer 501 NOT_IMPLEMENTED on both wirings, and the fault message is still withheld; only the status and the machine code move. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 --- .../18559-batch-door-wired-failing-engine.md | 38 ++++ .../src/objectql-slot-consumer-census.test.ts | 187 ++++++++++++++---- packages/rest/src/rest-server.ts | 59 +++++- 3 files changed, 245 insertions(+), 39 deletions(-) create mode 100644 .changeset/18559-batch-door-wired-failing-engine.md diff --git a/.changeset/18559-batch-door-wired-failing-engine.md b/.changeset/18559-batch-door-wired-failing-engine.md new file mode 100644 index 00000000000..915eec2cecb --- /dev/null +++ b/.changeset/18559-batch-door-wired-failing-engine.md @@ -0,0 +1,38 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): `POST /api/v1/batch` answers the same thing for a wired-and-failing engine on every wiring — 503, the answer this slot's two other consumers already give (#18559) + +`objectQLProvider` has three consumers in `packages/rest/src/rest-server.ts`. Two reach the +seam through `wiredEngineOrLoud`, which keeps "no engine is wired" and "the engine WAS wired +and could not be resolved" as two facts. The cross-object batch door read the field directly, +so a rejection escaped the read, missed the adjacent `501 NOT_IMPLEMENTED` arm (it tests +`!ql || typeof ql.transaction !== 'function'`, which a rejection never reaches) and landed in +the handler's generic outer `catch`. + +⛔ **Not a re-collapse and not a regression.** The two facts always differed on the wire, so +the decidable test #14251 tightened was already satisfied at this consumer. What was wrong is +that they differed *through a catch-all that knows nothing about this seam*. + +**What moves, measured on a real `RestServer` over a real `ObjectKernel`, driven at the door:** + +| wiring, engine wired and FAILING | before | after | +|:--|:--|:--| +| single-kernel (the composition the open core boots) | 503 `SERVICE_UNAVAILABLE` | 503 — unchanged | +| multi-kernel (a `kernelManager` is wired) | **500 `INTERNAL_ERROR`** | **503 `SERVICE_UNAVAILABLE`** | + +⭐ The single-kernel row is why this is a de-divergence rather than a new wire ruling: there +`computeExecCtx` resolves the engine through its own `wiredEngineOrLoud` branch and raises +before the batch handler's engine line runs, so this door already answered 503. The 500 was +reachable only where that gate's kernel branch absorbs by design and hands the engine question +down. An operator got one of two answers for one fact depending on which composition was +running — and 500 and 503 are not synonyms to a client: one says "I am broken", the other says +"I am temporarily unavailable, retry". + +**Unchanged, and pinned:** both ABSENCE shapes still answer `501 NOT_IMPLEMENTED` on both +wirings — no provider wired at all, and a provider that RESOLVES `undefined`, which is the +seam contract declaring absence rather than failing. The fault MESSAGE is still withheld +(`Internal server error`); only the status and the machine code move. `SERVICE_UNAVAILABLE` is +an existing `StandardErrorCode` already emitted by the sibling `/meta/object/:name/state/:field` +door for this same fact — no new code, no new payload key, no new export. diff --git a/packages/rest/src/objectql-slot-consumer-census.test.ts b/packages/rest/src/objectql-slot-consumer-census.test.ts index e43b098ff2f..40271feaa75 100644 --- a/packages/rest/src/objectql-slot-consumer-census.test.ts +++ b/packages/rest/src/objectql-slot-consumer-census.test.ts @@ -32,29 +32,34 @@ * ## What is asserted about the answers * * §2 drives the third consumer on the wiring where it decides anything, and - * asserts the DISTINCTION — not a status code. Measured here, on - * `POST /api/v1/batch`: + * asserts the DISTINCTION — not only a status code. Measured here, on + * `POST /api/v1/batch`, MULTI-KERNEL wiring: * - * | engine fact | wire answer | - * |:-------------------------------------------|:---------------------------| - * | no provider wired at all | 501 NOT_IMPLEMENTED | - * | provider RESOLVES `undefined` (absence) | 501 NOT_IMPLEMENTED | - * | provider REJECTS (wired, failed to build) | 500 INTERNAL_ERROR | - * | provider THROWS SYNCHRONOUSLY (#13280) | 500 INTERNAL_ERROR | + * | engine fact | before #18559 | now | + * |:------------------------------------------|:-----------------------|:----------------------------| + * | no provider wired at all | 501 NOT_IMPLEMENTED | 501 — unchanged | + * | provider RESOLVES `undefined` (absence) | 501 NOT_IMPLEMENTED | 501 — unchanged | + * | provider REJECTS (wired, failed to build) | **500 INTERNAL_ERROR** | **503 SERVICE_UNAVAILABLE** | + * | provider THROWS SYNCHRONOUSLY (#13280) | **500 INTERNAL_ERROR** | **503 SERVICE_UNAVAILABLE** | * - * ⇒ this consumer does **not** re-collapse: a rejection and a resolved - * `undefined` reach two different answers, which is the whole of the decidable - * test. It reaches them through the handler's own outer `catch` - * (`handleRouteError`) rather than through the `wiredEngineOrLoud` seam its two - * siblings use. + * ⇒ this consumer never re-collapsed: a rejection and a resolved `undefined` + * always reached two different answers, which is the whole of the decidable + * test, and #18559 is therefore ⛔ not a regression repair. ⭐ What changed is + * HOW. It used to reach them through the handler's own outer `catch` + * (`handleRouteError`) — a catch-all that knows nothing about this seam — and + * now reaches them through the `wiredEngineOrLoud` seam its two siblings + * already use. `expect(rejecting).not.toEqual(absent)` stays the load-bearing + * assertion either way; the status pin is what says which mechanism answered. * - * ⛔ **500 is RECORDED here, not ruled.** The other two consumers of this same - * slot answer `503 SERVICE_UNAVAILABLE` for the identical fact. Whether this - * door should join them is a public-door wire change and needs the per-consumer - * ruling #14251 reserves for exactly this; ⛔ nothing here asserts that 500 is - * correct, and ⛔ nothing here moves it. What is load-bearing is - * `expect(rejecting).not.toEqual(absent)` — if a later edit collapses the two, - * this file says so. + * ⚠️ §2b is why the 500 was a DIVERGENCE rather than this door's answer. On the + * SINGLE-KERNEL wiring — the composition the open core boots — this door already + * answered `503` before #18559, because `computeExecCtx` takes its PROVIDER + * branch there (`wiredEngineOrLoud`) and raises before the batch handler's own + * engine line runs. The 500 was reachable only on the MULTI-KERNEL wiring, where + * that gate's kernel branch absorbs by design (`wiredEngineOrLoud`'s RESIDUE + * note) and hands the engine question down. ⇒ the repair did not pick a NEW wire + * answer for this door; it removed a wiring-dependent divergence. §2b drives + * both wirings side by side so that sentence is a reading and not an argument. * * ## Controls * @@ -62,9 +67,18 @@ * symbol known to be present (it must find sites) and against one known to be * absent (it must find none), so "3 sites" is a reading rather than the only * sentence the instrument can produce. §2 serves a REAL answer — 200 on a - * healthy engine, 400 on a declared refusal — so no 501/500 below comes from an + * healthy engine, 400 on a declared refusal — so no 501/503 below comes from an * instrument that can only report faults. * + * ⚠️ The healthy control is deliberately driven with `{ operations: [] }` and + * not with a real op. On the MULTI-KERNEL wiring a real op continues past this + * door's engine line into `resolveProtocol`/`loadObjectItems`, which this + * harness's auth-only kernel cannot serve — measured, that answers 500 + * INTERNAL_ERROR for a HEALTHY engine too, which would make a 500 read here + * ambiguous between "the seam answered" and "the harness ran out of kernel". + * The empty-operations body returns inside the door, after the engine line and + * before the protocol is touched, so the engine fact is the only variable. + * * ## What this file deliberately does NOT re-assert * * The other two consumers already have behavioural pins of their own and are @@ -80,7 +94,12 @@ import { describe, it, expect, vi } from 'vitest'; import { readFileSync, existsSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname, resolve } from 'node:path'; -import { ObjectKernel } from '@objectstack/core'; +import { + AUTHZ_STORE_UNAVAILABLE_CODE, + AUTHZ_STORE_UNAVAILABLE_STATUS, + ObjectKernel, +} from '@objectstack/core'; +import { INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; // `.js` on purpose — NodeNext resolution requires the extension. import { RestServer } from './rest-server.js'; @@ -141,22 +160,41 @@ describe('[#18546] §1 the `objectQLProvider` consumer census is mechanical', () expect(sites).toHaveLength(3); }); - it('two consumers reach the slot through `wiredEngineOrLoud`; the third does not', () => { + it('⭐ [#18559] ALL THREE consumers now reach the slot through `wiredEngineOrLoud` — none reads it directly', () => { const sites = invocationSites(SOURCE, 'objectQLProvider'); + const lines = SOURCE.split('\n'); const viaSeam = sites.filter((s) => { - const lines = SOURCE.split('\n'); // The helper is named on the line that OPENS the call, two lines up - // from the invocation in both existing spellings. + // from the invocation in all three spellings. return lines.slice(Math.max(0, s.line - 4), s.line).join('\n').includes('wiredEngineOrLoud('); }); - expect(viaSeam).toHaveLength(2); - const direct = sites.filter((s) => !viaSeam.includes(s)); - expect(direct).toHaveLength(1); - // …and the one that does not is the cross-object batch door, identified - // by the answer its `undefined` path produces twelve characters later. - const after = SOURCE.split('\n').slice(direct[0]!.line, direct[0]!.line + 6).join('\n'); - expect(after).toContain('Transactional batch not supported by this runtime'); + // The message carries the offenders, so a failure hands the next reader + // the site rather than a bare number. + expect(direct.map((s) => `L${s.line}: ${s.text}`).join('\n')).toBe(''); + expect(direct).toHaveLength(0); + expect(viaSeam).toHaveLength(3); + + // ⭐ CONTROL for the window predicate itself, on BOTH axes. It reads a + // WINDOW of source text, so "3 of 3" is only a reading if the same + // predicate can also say something else. `emailServiceProvider` is the + // sibling slot whose one call site #15405 routed through the SWALLOWING + // helper instead, so it is the natural negative: + const wrappedIn = (field: string, helper: string) => + invocationSites(SOURCE, field).filter((s) => + lines.slice(Math.max(0, s.line - 4), s.line).join('\n').includes(helper)); + // - the predicate must NOT claim that slot for `wiredEngineOrLoud`… + expect(wrappedIn('emailServiceProvider', 'wiredEngineOrLoud(')).toHaveLength(0); + // - …and it must still FIRE for the helper that slot really uses, so + // the zero above is a discrimination and not a dead predicate. + expect(wrappedIn('emailServiceProvider', 'seamOrUndefined(')).toHaveLength(1); + + // …and the batch door is one of the three, identified by the answer its + // `undefined` path produces a few lines later — so "3 via the seam" is + // pinned to include the consumer #18559 moved, not just its two siblings. + const batch = viaSeam.filter((s) => + lines.slice(s.line, s.line + 6).join('\n').includes('Transactional batch not supported by this runtime')); + expect(batch).toHaveLength(1); }); it('⛔ the field is read only as `this.objectQLProvider` — no alias keeps a consumer out of this census', () => { @@ -299,7 +337,12 @@ const engineHealthy = async () => ({ transaction: async (fn: any) => fn({}) }); // --------------------------------------------------------------------------- describe('[#18546] §2 `POST /batch` — the slot\'s third consumer does not re-collapse', () => { - it('CONTROL: a healthy engine is SERVED, so 501/500 is not all this instrument can say', async () => { + it('CONTROL: a healthy engine is SERVED, so 501/503 is not all this instrument can say', async () => { + // ⚠️ `{ operations: [] }` on purpose — see the file docblock's control + // note. A real op continues past this door's engine line into + // `resolveProtocol`, which this harness's auth-only kernel cannot serve, + // and a HEALTHY engine then answers 500 too. The empty body returns + // inside the door, so the engine fact stays the only variable. const served = await driveBatch({ ...kernelHost(), objectQLProvider: engineHealthy }, { operations: [] }); expect(served.status).toBe(200); expect(served.body).toEqual({ results: [] }); @@ -332,11 +375,21 @@ describe('[#18546] §2 `POST /batch` — the slot\'s third consumer does not re- // value the `undefined` path produces. Side by side, and unequal. expect(rejecting.status).not.toBe(absent.status); expect(rejecting.body?.code).not.toBe(absent.body?.code); - // Today's answer, RECORDED so a change is legible — ⛔ not ruled correct. - // Its two sibling consumers answer 503 for this same fact; whether this - // door joins them is the per-consumer ruling #14251 reserves. - expect(rejecting.status).toBe(500); - expect(rejecting.body?.code).toBe('INTERNAL_ERROR'); + // ⭐ [#18559] THE PIN THAT MOVED. This read `500` / `INTERNAL_ERROR` + // before the repair — the answer the handler's generic outer catch + // produced — and reads the branded outage its two sibling consumers + // already answer now that this door reaches the slot through + // `wiredEngineOrLoud`. The two constants are `@objectstack/core`'s, so + // the pin cannot drift from the helper that raises them. + expect(rejecting.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); + expect(rejecting.status).toBe(503); + expect(rejecting.body?.code).toBe(AUTHZ_STORE_UNAVAILABLE_CODE); + expect(rejecting.body?.code).toBe('SERVICE_UNAVAILABLE'); + // ⛔ And the fault MESSAGE is still withheld: what moved is the status + // and the code, ⛔ never the prose. The driver's own sentence + // ("driver handshake failed") must not appear on the wire. + expect(rejecting.body?.error).toBe(INTERNAL_ERROR_MESSAGE); + expect(JSON.stringify(rejecting.body)).not.toContain('driver handshake failed'); }); it('a NON-`async` provider throwing synchronously reaches that same answer, not the `undefined` path', async () => { @@ -351,6 +404,64 @@ describe('[#18546] §2 `POST /batch` — the slot\'s third consumer does not re- }); }); +// --------------------------------------------------------------------------- +// §2b — [#18559] the SAME door on BOTH wirings. This is the section that says +// what the repair actually was: not a new wire answer for this door, but the +// removal of a divergence between two compositions of the same server. +// --------------------------------------------------------------------------- + +/** + * The SINGLE-KERNEL host — no `kernelManager`, auth reached through the + * provider seam. This is the composition `pnpm dev:crm` and the open core boot, + * and on it `computeExecCtx` resolves the engine through its PROVIDER branch, + * which is `wiredEngineOrLoud`. A wired-and-failing engine therefore raises at + * the GATE, before this door's own engine line is reached. + */ +function singleKernelHost(): Wiring { + return { authServiceProvider: authService, defaultEnvironmentIdProvider: () => 'env_1' }; +} + +describe('[#18559] §2b the batch door answers ONE thing for one fact, on both wirings', () => { + it('CONTROL: the single-kernel harness SERVES a real operation, so its 5xx below is a discrimination', async () => { + // ⭐ This is the control the multi-kernel harness cannot give: here a + // HEALTHY engine and a REAL op answer 200, so the 503 in the next case + // is the engine fact and ⛔ not the harness running out of kernel. + const served = await driveBatch({ ...singleKernelHost(), objectQLProvider: engineHealthy }, ONE_OP); + expect(served.status).toBe(200); + expect(served.body).toEqual({ results: [{ id: '1' }] }); + }); + + it('⭐ single-kernel: a wired-and-failing engine answered 503 BEFORE this card too — the gate raises first', async () => { + // ⛔ Not a claim this repair introduced: it is the reading that makes the + // repair a de-divergence rather than a new ruling on a public door. The + // gate's `wiredEngineOrLoud` raises `AuthzStoreUnavailableError` and the + // handler's outer catch serves it with the branded status. + const rejecting = await driveBatch({ ...singleKernelHost(), objectQLProvider: providerRejecting }, ONE_OP); + expect(rejecting.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); + expect(rejecting.body?.code).toBe(AUTHZ_STORE_UNAVAILABLE_CODE); + }); + + it('⭐ the two wirings now AGREE for the identical engine fact', async () => { + const single = await driveBatch({ ...singleKernelHost(), objectQLProvider: providerRejecting }, ONE_OP); + const multi = await driveBatch({ ...kernelHost(), objectQLProvider: providerRejecting }, ONE_OP); + // The whole of #18559, in one line: before the repair these two were + // 503 and 500 for one fact, decided by which composition the operator + // happened to be running. + expect(multi.status).toBe(single.status); + expect(multi.body?.code).toBe(single.body?.code); + }); + + it('⛔ and BOTH absence shapes still answer 501 on both wirings — no accept set moved', async () => { + for (const host of [singleKernelHost(), kernelHost()]) { + for (const provider of [undefined, providerAbsent]) { + const answer = await driveBatch({ ...host, objectQLProvider: provider }, ONE_OP); + expect(answer.status).toBe(501); + expect(answer.body?.code).toBe('NOT_IMPLEMENTED'); + } + } + }); +}); + // --------------------------------------------------------------------------- // §3 — the other two consumers are covered elsewhere, by name // --------------------------------------------------------------------------- diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 781a0e4ae21..4571e491bae 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -13373,7 +13373,64 @@ export class RestServer { const environmentId = isScoped ? req.params?.environmentId : undefined; const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; - const ql = this.objectQLProvider ? await this.objectQLProvider(environmentId) : undefined; + // [#18559] The engine seam, reached the way its two SIBLING + // consumers of this same slot already reach it — + // `wiredEngineOrLoud` — so "no engine is wired" and "the + // engine WAS wired and could not be resolved" stay two facts + // instead of being told apart only by accident. The retired + // spelling this replaces was a plain read: + // + // this.objectQLProvider ? await this.objectQLProvider(environmentId) : undefined + // + // ⚠️ It did NOT re-collapse them — that is why this is not a + // regression and was not a blocker for #14251's decidable + // test. A rejection escaped the plain read, missed the 501 + // arm below (which tests `!ql || typeof ql.transaction !== + // 'function'`, and a rejection never reaches it), and was + // caught by this handler's GENERIC outer catch + // (`handleRouteError`). So the two facts did differ on the + // wire — 500 INTERNAL_ERROR against 501 NOT_IMPLEMENTED — + // but through a catch-all that knows nothing about this + // seam, at a status this slot's other two consumers do not + // use for the same fact. + // + // ⭐ What decided it, measured on a real `RestServer` over a + // real `ObjectKernel` rather than argued: THIS DOOR ALREADY + // ANSWERS 503 on the single-kernel wiring. There + // `computeExecCtx` takes its PROVIDER branch, which is + // `wiredEngineOrLoud`, and raises before this line runs. The + // 500 was reachable only on the MULTI-KERNEL wiring, where + // the gate's kernel branch absorbs by design (see + // `wiredEngineOrLoud`'s RESIDUE note) and hands the engine + // question down to this line: + // + // | wiring, engine wired and FAILING | before | after | + // |:--|:--|:--| + // | single-kernel (gate raises first)| 503 | 503 — unchanged | + // | multi-kernel (gate absorbs) | **500**| **503** | + // + // ⇒ the repair does not choose a new wire answer for this + // door; it removes a WIRING-DEPENDENT divergence, leaving + // the answer this door already gave on the composition the + // open core boots. + // + // ⛔ NOT `seamOrUndefined`. That helper SWALLOWS, and its own + // docblock forbids routing the data-engine seam back through + // it "to make the seams uniform". + // + // The wiring fact is the provider's PRESENCE, asked once and + // never inferred from what it returned, so both ABSENCE + // shapes reach the 501 below byte-for-byte as before: no + // provider wired at all, and a provider that RESOLVES + // `undefined`, which is the seam contract declaring absence + // rather than failing. `wiredEngineOrLoud` also invokes the + // provider SYNCHRONOUSLY, so a host wiring a non-`async` + // provider — which the seam's declared type cannot prevent — + // reaches the same answer as one that rejects (#13280). + const ql = await wiredEngineOrLoud( + Boolean(this.objectQLProvider), + () => this.objectQLProvider!(environmentId), + ); if (!ql || typeof ql.transaction !== 'function') { // Typed like every other 501 on this server (clone/search, // #4067) so a client can key on the code, not the prose.