|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #15037 — `RemoteLoader.list()` declares `Promise<string[]>` and maps a |
| 5 | + * nameless body straight through, so `listNames()` can hand a caller a literal |
| 6 | + * `undefined` where the type says a `string`. |
| 7 | + * |
| 8 | + * --------------------------------------------------------------------------- |
| 9 | + * The defect (measured on `origin/main` @ 9c3fda5fb, this fixture) |
| 10 | + * --------------------------------------------------------------------------- |
| 11 | + * async list(type: string): Promise<string[]> { |
| 12 | + * const items = await this.loadMany<{ name: string }>(type); |
| 13 | + * return items.map(i => i.name); |
| 14 | + * } |
| 15 | + * |
| 16 | + * The type argument `{ name: string }` is an ASSERTION about bodies that |
| 17 | + * arrived over HTTP; nothing checks it. A body with no top-level `name` yields |
| 18 | + * `i.name === undefined`, and that `undefined` is pushed into an array the |
| 19 | + * signature declares as `string[]` — a runtime violation of a declared type, |
| 20 | + * not merely an untidy entry. `MetadataManager.listNames()` unions loader |
| 21 | + * `list()` output unfiltered (`result.forEach(item => names.add(item))`), so |
| 22 | + * the violation reaches consumers, which then use the value as an object key, |
| 23 | + * lower-case it, or feed it back to a by-name `load()`. |
| 24 | + * |
| 25 | + * --------------------------------------------------------------------------- |
| 26 | + * The direction, and why it was not this seat's to choose |
| 27 | + * --------------------------------------------------------------------------- |
| 28 | + * Three of the four sibling loaders in this directory had already answered it, |
| 29 | + * and `RemoteLoader` was the only one with no guard at all: |
| 30 | + * |
| 31 | + * DatabaseLoader `rows.map(row => row.name as string)` |
| 32 | + * `.filter(name => typeof name === 'string')` — guarded |
| 33 | + * MemoryLoader `Array.from(typeStore.keys())` — store keys |
| 34 | + * FilesystemLoader narrowed by #14486 to names `findFile()` resolves |
| 35 | + * RemoteLoader `items.map(i => i.name)` — unguarded |
| 36 | + * |
| 37 | + * So the repair is `DatabaseLoader`'s guard, one file away: same directory, |
| 38 | + * same method name, same "cast then map" spelling, one `.filter()` behind it. |
| 39 | + * "Refuse loudly" was NOT taken, and that is a landed decision rather than a |
| 40 | + * preference — `DatabaseLoader`'s guard is a silent `.filter()`, and |
| 41 | + * `FilesystemLoader`'s narrowing carries a maintainer ruling (via the director |
| 42 | + * seat on #14486, 2026-09-02, direction A, with B explicitly refused) whose |
| 43 | + * own reasoning is this card's: |
| 44 | + * |
| 45 | + * 「A name in the list that `get()` answers `null` for is the silent failure |
| 46 | + * an author reads as their own typo, so they retry the same word: the list |
| 47 | + * and the door now agree instead.」 |
| 48 | + * |
| 49 | + * An `undefined` in `listNames()` is the extreme form of a name the door can |
| 50 | + * never answer. |
| 51 | + * |
| 52 | + * ⛔ What is deliberately NOT copied: `MemoryLoader`'s structural fix (return |
| 53 | + * the store key) and #14205's keying rule ("identity is the key the store |
| 54 | + * holds an item under, not `body.name`"). This loader reads over HTTP and has |
| 55 | + * no store key to fall back on, so `body.name` is all it has. The guard SHAPE |
| 56 | + * is what transfers; the family's keying rule cannot be satisfied here. |
| 57 | + * |
| 58 | + * --------------------------------------------------------------------------- |
| 59 | + * Why the double is a `fetch`, not a hand-written loader |
| 60 | + * --------------------------------------------------------------------------- |
| 61 | + * Every case below drives the REAL `RemoteLoader` and, on the manager face, a |
| 62 | + * REAL `MetadataManager`. The only thing stubbed is the wire — a `fetch` that |
| 63 | + * serves the collection and answers the by-name door — so the code under test |
| 64 | + * is this loader's own `list()`/`loadMany()`/`load()`, not a verdict handed in |
| 65 | + * by a mock. |
| 66 | + * |
| 67 | + * `CONTROL:` cases pin what must NOT move: the well-formed body stays listed |
| 68 | + * and stays loadable, so a guard that dropped everything would fail here too. |
| 69 | + * `RECORD:` pins behaviour this repair deliberately leaves alone. |
| 70 | + */ |
| 71 | + |
| 72 | +import { describe, it, expect, vi, afterEach } from 'vitest'; |
| 73 | +import { MetadataManager } from '../metadata-manager.js'; |
| 74 | +import { RemoteLoader } from './remote-loader.js'; |
| 75 | + |
| 76 | +const BASE = 'https://metadata.invalid/api'; |
| 77 | +const TYPE = 'object'; |
| 78 | + |
| 79 | +/** One well-formed body, and the two shapes that violate the asserted cast. */ |
| 80 | +const NAMED = { name: 'account', label: 'Account' }; |
| 81 | +const NAMELESS = { label: 'Nameless' }; |
| 82 | +const NUMERIC_NAME = { name: 42, label: 'Numeric' }; |
| 83 | + |
| 84 | +const BODIES: Array<Record<string, unknown>> = [NAMED, NAMELESS, NUMERIC_NAME]; |
| 85 | + |
| 86 | +/** Exactly the bodies whose top-level `name` is a string. */ |
| 87 | +const LISTABLE = ['account']; |
| 88 | + |
| 89 | +/** |
| 90 | + * The remote, minimally: `GET /{type}` serves the collection, `GET|HEAD |
| 91 | + * /{type}/{name}` is the by-name door and answers only for a body whose |
| 92 | + * top-level `name` equals that segment. That door is what makes |
| 93 | + * `listNames()`/`get()` comparable — it is the remote analogue of |
| 94 | + * `findFile()`. |
| 95 | + */ |
| 96 | +function serveRemote(bodies: Array<Record<string, unknown>>) { |
| 97 | + const collection = `${BASE}/${TYPE}`; |
| 98 | + |
| 99 | + return vi.fn(async (input: unknown, init?: { method?: string }) => { |
| 100 | + const url = String(input); |
| 101 | + const method = (init?.method ?? 'GET').toUpperCase(); |
| 102 | + const json = (body: unknown) => |
| 103 | + new Response(JSON.stringify(body), { |
| 104 | + status: 200, |
| 105 | + headers: { 'content-type': 'application/json' }, |
| 106 | + }); |
| 107 | + |
| 108 | + if (url === collection) { |
| 109 | + return method === 'HEAD' ? new Response(null, { status: 200 }) : json(bodies); |
| 110 | + } |
| 111 | + |
| 112 | + if (url.startsWith(`${collection}/`)) { |
| 113 | + const segment = url.slice(collection.length + 1); |
| 114 | + const hit = bodies.find(body => body.name === segment); |
| 115 | + if (!hit) return new Response(null, { status: 404 }); |
| 116 | + return method === 'HEAD' ? new Response(null, { status: 200 }) : json(hit); |
| 117 | + } |
| 118 | + |
| 119 | + return new Response(null, { status: 404 }); |
| 120 | + }); |
| 121 | +} |
| 122 | + |
| 123 | +function loader(bodies: Array<Record<string, unknown>> = BODIES): RemoteLoader { |
| 124 | + vi.stubGlobal('fetch', serveRemote(bodies)); |
| 125 | + return new RemoteLoader(BASE); |
| 126 | +} |
| 127 | + |
| 128 | +/** A cold manager — empty registry, one remote loader answering. */ |
| 129 | +function coldManager(bodies: Array<Record<string, unknown>> = BODIES): MetadataManager { |
| 130 | + const manager = new MetadataManager({ formats: ['json'], loaders: [] }); |
| 131 | + manager.registerLoader(loader(bodies)); |
| 132 | + return manager; |
| 133 | +} |
| 134 | + |
| 135 | +afterEach(() => { |
| 136 | + vi.unstubAllGlobals(); |
| 137 | +}); |
| 138 | + |
| 139 | +describe('#15037 RemoteLoader.list() keeps the `string[]` its signature declares', () => { |
| 140 | + it('EVERY listed name is a string — the declared-type violation itself', async () => { |
| 141 | + // The claim the signature makes, asserted directly. Before the guard this |
| 142 | + // answered `['account', undefined, 42]` against a `Promise<string[]>`. |
| 143 | + for (const name of await loader().list(TYPE)) { |
| 144 | + expect(typeof name).toBe('string'); |
| 145 | + } |
| 146 | + }); |
| 147 | + |
| 148 | + it('a body with no top-level `name` is no longer listed as `undefined`', async () => { |
| 149 | + const listed = await loader().list(TYPE); |
| 150 | + |
| 151 | + expect(listed).not.toContain(undefined); |
| 152 | + expect(listed.sort()).toEqual(LISTABLE); |
| 153 | + }); |
| 154 | + |
| 155 | + it('a non-string `name` is dropped too — the guard is `typeof`, not truthiness', async () => { |
| 156 | + // `DatabaseLoader`'s predicate is `typeof name === 'string'`, so it rejects |
| 157 | + // every shape the cast lied about, not just the missing one. A body whose |
| 158 | + // `name` is a number violates `Promise<string[]>` exactly as hard. |
| 159 | + expect(await loader().list(TYPE)).not.toContain(42 as unknown as string); |
| 160 | + }); |
| 161 | + |
| 162 | + it('EVERY listed name resolves through exists(), stat() and load()', async () => { |
| 163 | + // The list-and-door agreement, on this loader's own face. |
| 164 | + const remote = loader(); |
| 165 | + |
| 166 | + for (const name of await remote.list(TYPE)) { |
| 167 | + expect(await remote.exists(TYPE, name)).toBe(true); |
| 168 | + expect(await remote.stat(TYPE, name)).not.toBeNull(); |
| 169 | + expect((await remote.load(TYPE, name)).data).not.toBeNull(); |
| 170 | + } |
| 171 | + }); |
| 172 | + |
| 173 | + it('CONTROL: the well-formed body is still listed and still loadable', async () => { |
| 174 | + // A guard that dropped everything would satisfy every case above. This is |
| 175 | + // what stops that: the named body must survive untouched. |
| 176 | + const remote = loader(); |
| 177 | + |
| 178 | + expect(await remote.list(TYPE)).toContain('account'); |
| 179 | + expect((await remote.load(TYPE, 'account')).data).toEqual(NAMED); |
| 180 | + }); |
| 181 | + |
| 182 | + it('CONTROL: a collection of only well-formed bodies is unchanged by the guard', async () => { |
| 183 | + expect((await loader([NAMED, { name: 'contact' }]).list(TYPE)).sort()).toEqual([ |
| 184 | + 'account', |
| 185 | + 'contact', |
| 186 | + ]); |
| 187 | + }); |
| 188 | + |
| 189 | + it('CONTROL: an empty collection still lists nothing', async () => { |
| 190 | + expect(await loader([]).list(TYPE)).toEqual([]); |
| 191 | + }); |
| 192 | +}); |
| 193 | + |
| 194 | +describe('#15037 the repair reaches MetadataManager', () => { |
| 195 | + it('listNames() and get() give the same answer for every name', async () => { |
| 196 | + // The #14486 shape, reused on the `RemoteLoader` face as triage asked. |
| 197 | + // Before the guard, `listNames()` carried `undefined`, and `get()` for it |
| 198 | + // fetched `.../object/undefined` and answered `undefined` — the list and |
| 199 | + // the door disagreeing, which is the failure an author reads as their own |
| 200 | + // typo. |
| 201 | + const manager = coldManager(); |
| 202 | + |
| 203 | + for (const name of await manager.listNames(TYPE)) { |
| 204 | + expect(await manager.get(TYPE, name)).toBeDefined(); |
| 205 | + } |
| 206 | + }); |
| 207 | + |
| 208 | + it('listNames() no longer forwards a literal `undefined` to consumers', async () => { |
| 209 | + const names = await coldManager().listNames(TYPE); |
| 210 | + |
| 211 | + expect(names).not.toContain(undefined); |
| 212 | + expect(names.every(name => typeof name === 'string')).toBe(true); |
| 213 | + expect(names.sort()).toEqual(LISTABLE); |
| 214 | + }); |
| 215 | + |
| 216 | + it('CONTROL: listNames() still reports the well-formed body, and get() resolves it', async () => { |
| 217 | + const manager = coldManager(); |
| 218 | + |
| 219 | + expect(await manager.listNames(TYPE)).toContain('account'); |
| 220 | + expect(await manager.get(TYPE, 'account')).toEqual(NAMED); |
| 221 | + }); |
| 222 | +}); |
| 223 | + |
| 224 | +describe('#15037 RECORD: what this repair deliberately leaves alone', () => { |
| 225 | + it('RECORD: loadMany() still returns the bodies list() no longer names', async () => { |
| 226 | + // The guard narrows `list()` only. Filtering the body read would change |
| 227 | + // what `MetadataManager.loadMany()` aggregates — a different direction |
| 228 | + // (#14341/#14205 fixed items going MISSING; this card is about one |
| 229 | + // APPEARING as `undefined`) and not this card's. `loadMany` keys nothing, |
| 230 | + // so a nameless body is still legitimately served there. |
| 231 | + const bodies = await loader().loadMany<Record<string, unknown>>(TYPE); |
| 232 | + |
| 233 | + expect(bodies).toHaveLength(3); |
| 234 | + expect(bodies).toContainEqual(NAMELESS); |
| 235 | + }); |
| 236 | + |
| 237 | + it('RECORD: the by-name door cannot reach a nameless body, before or after', async () => { |
| 238 | + // `RemoteLoader` reads over HTTP and holds no store key, so there is no |
| 239 | + // identity to list a nameless body under. #14205's keying rule cannot be |
| 240 | + // satisfied here; the list is narrowed to agree with the door instead. |
| 241 | + expect((await loader().load(TYPE, 'Nameless')).data).toBeNull(); |
| 242 | + }); |
| 243 | +}); |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?v=4&size=40){kind=avatar}
0 commit comments