|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #8818 — `saveMetaItem`'s opening guard was the ONE refusal in the method |
| 5 | + * that declared no ADR-0112 envelope, so consumers applying the rule withheld |
| 6 | + * its sentence and the REST boundary served it as a server fault. |
| 7 | + * |
| 8 | + * ## What was measured, end to end, before the fix |
| 9 | + * |
| 10 | + * The card was filed as an observation with an explicitly unverified premise |
| 11 | + * ("read from source, no victim measured"), so the premise was probed against |
| 12 | + * a real server (`pnpm dev:crm -- --fresh`, `PUT /api/v1/meta/view/:name`, |
| 13 | + * authenticated as the seeded platform admin) before a line was changed: |
| 14 | + * |
| 15 | + * | request body | reaches this guard? | answered (before) | |
| 16 | + * |---|---|---| |
| 17 | + * | *(no body at all)* | no | `422 INVALID_METADATA` | |
| 18 | + * | `null` | no | `422 INVALID_METADATA` | |
| 19 | + * | `{}` | no | `422 INVALID_METADATA` | |
| 20 | + * | `{"item": null}` | **YES** | **`500 INTERNAL_ERROR`** | |
| 21 | + * | `{"metadata": null}` | **YES** | **`500 INTERNAL_ERROR`** | |
| 22 | + * |
| 23 | + * So the honest outcomes the card itself listed — "unreachable, leave it |
| 24 | + * alone" and "a programming-error guard, not an authoring refusal" — are both |
| 25 | + * FALSE, and the reason is precise: `PUT /meta/:type/:name` unwraps the |
| 26 | + * `{ item }` / `{ metadata }` envelope shapes before calling, so an |
| 27 | + * explicitly-null envelope arrives as `item: null` and lands here, while a |
| 28 | + * missing/empty/`null` BODY folds to `{}` (truthy) and is refused downstream |
| 29 | + * by the per-type Zod parse. The reachable population is caller-authored JSON |
| 30 | + * only; every internal caller passes a concrete document. |
| 31 | + * |
| 32 | + * The cost was also LARGER than filed. The card predicted the sentence would |
| 33 | + * degrade to a consumer's generic fallback; what the wire actually did was |
| 34 | + * answer **500 `INTERNAL_ERROR`** — because `handleRouteError` has no status |
| 35 | + * to read and defaults to a server fault. A 500 does not merely tell the |
| 36 | + * author less, it tells them something FALSE: that the server broke and the |
| 37 | + * request is worth retrying, when it can never succeed unchanged. |
| 38 | + * |
| 39 | + * ## Why the `clientFacingFailureText` assertion is the point of this file |
| 40 | + * |
| 41 | + * A test asserting only `code`/`status` on the thrown error would pass without |
| 42 | + * demonstrating the thing the card is about. `declaresClientRefusal` is a |
| 43 | + * POSITIVE list keyed on a 4xx `status`, so the assertion that matters is that |
| 44 | + * the sentence now SURVIVES the rule rather than being replaced by the |
| 45 | + * caller's fallback — and that assertion is only evidence next to its control: |
| 46 | + * the same helper, handed the bare `Error` this guard used to throw, still |
| 47 | + * withholds. Both directions are asserted below; drop the control and the pin |
| 48 | + * would stay green against a `clientFacingFailureText` that had stopped |
| 49 | + * withholding anything at all. |
| 50 | + */ |
| 51 | + |
| 52 | +import { describe, it, expect, vi } from 'vitest'; |
| 53 | +import { ObjectStackProtocolImplementation, clientFacingFailureText } from './protocol.js'; |
| 54 | + |
| 55 | +/** |
| 56 | + * A protocol over an engine whose every verb is a tripwire: this guard is the |
| 57 | + * FIRST statement of `saveMetaItem`, so a refusal that touched the engine at |
| 58 | + * all would mean the check had moved behind something with side effects. |
| 59 | + */ |
| 60 | +function makeProtocol() { |
| 61 | + const findOne = vi.fn(async () => null); |
| 62 | + const find = vi.fn(async () => [] as unknown[]); |
| 63 | + const engine = { |
| 64 | + registry: { getObject: () => undefined }, |
| 65 | + findOne, |
| 66 | + find, |
| 67 | + }; |
| 68 | + return { p: new ObjectStackProtocolImplementation(engine as any), findOne, find }; |
| 69 | +} |
| 70 | + |
| 71 | +/** The refusal a rejected request produced, plus proof the engine was untouched. */ |
| 72 | +async function refusalFor(request: Record<string, unknown>) { |
| 73 | + const { p, findOne, find } = makeProtocol(); |
| 74 | + let answered: unknown; |
| 75 | + try { |
| 76 | + answered = await (p as any).saveMetaItem(request); |
| 77 | + } catch (e) { |
| 78 | + expect(findOne, 'the engine was reached before the refusal').not.toHaveBeenCalled(); |
| 79 | + expect(find, 'the engine was reached before the refusal').not.toHaveBeenCalled(); |
| 80 | + return e as Error & { code?: string; status?: number }; |
| 81 | + } |
| 82 | + throw new Error( |
| 83 | + `${JSON.stringify(request)} was ACCEPTED (answered ${JSON.stringify(answered)}) instead of refused`, |
| 84 | + ); |
| 85 | +} |
| 86 | + |
| 87 | +describe('#8818 — a save with no item declares the ADR-0112 envelope', () => { |
| 88 | + // The three spellings that reach this guard. `item: null` is the one the |
| 89 | + // REST route actually produces (from `{"item": null}` / `{"metadata": |
| 90 | + // null}`); the other two are the same condition reached through the SDK |
| 91 | + // and the protocol interface, where `item` is an optional parameter. |
| 92 | + it.each<[string, Record<string, unknown>]>([ |
| 93 | + ['an explicitly null item (what the wire produces)', { type: 'app', name: 'a', item: null }], |
| 94 | + ['an absent item', { type: 'app', name: 'a' }], |
| 95 | + ['an explicitly undefined item', { type: 'app', name: 'a', item: undefined }], |
| 96 | + ])('refuses %s with 400 INVALID_REQUEST', async (_label, request) => { |
| 97 | + const err = await refusalFor(request); |
| 98 | + |
| 99 | + // The envelope, not merely the throw. A bare `toThrow()` here would be |
| 100 | + // permanently green: the UNFIXED guard threw too — that was the whole |
| 101 | + // defect — so the throw carries no information and only the |
| 102 | + // declaration does. |
| 103 | + expect(err.code).toBe('INVALID_REQUEST'); |
| 104 | + expect(err.status).toBe(400); |
| 105 | + }); |
| 106 | + |
| 107 | + it('names the remedy in the message, so the refusal is self-correcting', async () => { |
| 108 | + const err = await refusalFor({ type: 'view', name: 'my_view', item: null }); |
| 109 | + |
| 110 | + expect(err.message).toContain("requires an 'item' body"); |
| 111 | + // The address the author got wrong is echoed back to them. |
| 112 | + expect(err.message).toContain('view/my_view'); |
| 113 | + }); |
| 114 | + |
| 115 | + it('does not refuse a request that DOES carry an item', async () => { |
| 116 | + // What a refusal is cheapest to break. Green in both directions on its |
| 117 | + // own — it is a guard, not evidence — but an over-broad guard (say, one |
| 118 | + // testing `'item' in request`) would turn it red. |
| 119 | + const { p } = makeProtocol(); |
| 120 | + await expect( |
| 121 | + (p as any).saveMetaItem({ type: 'app', name: 'a', item: { name: 'a' } }), |
| 122 | + ).rejects.not.toMatchObject({ code: 'INVALID_REQUEST' }); |
| 123 | + }); |
| 124 | +}); |
| 125 | + |
| 126 | +describe('#8818 — the refusal now SURVIVES the ADR-0112 disclosure rule', () => { |
| 127 | + it('is quoted back to the author instead of degrading to the fallback', async () => { |
| 128 | + const err = await refusalFor({ type: 'app', name: 'test_app', item: null }); |
| 129 | + |
| 130 | + const shown = clientFacingFailureText(err, 'save failed'); |
| 131 | + |
| 132 | + // THE POINT OF THE CARD: a consumer applying the #8086/#8136 rule now |
| 133 | + // shows the producer's own sentence. |
| 134 | + expect(shown).not.toBe('save failed'); |
| 135 | + expect(shown).toBe(err.message); |
| 136 | + expect(shown).toContain("requires an 'item' body"); |
| 137 | + }); |
| 138 | + |
| 139 | + it('CONTROL — the bare Error this guard used to throw is still withheld', () => { |
| 140 | + // Byte-for-byte what `origin/main` threw at this site. Without this |
| 141 | + // control the assertion above would also pass against a |
| 142 | + // `clientFacingFailureText` that had stopped withholding ANYTHING, |
| 143 | + // which is the regression that would silently re-open #8086. |
| 144 | + const undeclared = new Error('Item data is required'); |
| 145 | + |
| 146 | + expect(clientFacingFailureText(undeclared, 'save failed')).toBe('save failed'); |
| 147 | + }); |
| 148 | +}); |
0 commit comments