|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #14486 — `FilesystemLoader.list()` reports only names the resolve trio can |
| 5 | + * open: one shared name-to-path derivation for `list()`, `findFile()` and |
| 6 | + * `loadManyKeyed()`. |
| 7 | + * |
| 8 | + * --------------------------------------------------------------------------- |
| 9 | + * The defect (measured on `origin/main` @ 23619f579, this fixture) |
| 10 | + * --------------------------------------------------------------------------- |
| 11 | + * `list()` reported `path.basename(file, ext)` for every file its glob found — |
| 12 | + * nested or not, with an extension or without — while `findFile()` resolved |
| 13 | + * `ROOT/TYPE/NAME` plus one of five hard-coded extensions. The two disagreed |
| 14 | + * for three shapes, and the disagreement reached consumers through |
| 15 | + * `MetadataManager.listNames()`, which unions loader `list()` output unfiltered: |
| 16 | + * |
| 17 | + * ROOT/object/crm/account.json -> listed as `account`; exists()=false |
| 18 | + * ROOT/object/noext -> listed as `noext`; exists()=false |
| 19 | + * ROOT/object/*.js (default set)-> listed and resolvable; never LOADED, and |
| 20 | + * `load()` threw `No serializer found` |
| 21 | + * |
| 22 | + * A name in the list that `get()` answers `null` for is a silent failure: the |
| 23 | + * author (human or AI) reads it as their own typo and retries the same word. |
| 24 | + * |
| 25 | + * --------------------------------------------------------------------------- |
| 26 | + * The rule this pins (maintainer ruling on #14486, via the director seat, |
| 27 | + * 2026-09-02 — option A, narrow) |
| 28 | + * --------------------------------------------------------------------------- |
| 29 | + * `list()` reports a file only where this loader's derivation is a bijection |
| 30 | + * for it: directly under `ROOT/TYPE/`, carrying an extension one of this |
| 31 | + * INSTANCE's registered serializers claims. Rejected as option B was the |
| 32 | + * reverse-unify — teach `list()` to report `crm/account` and `findFile()` to |
| 33 | + * accept path-shaped names — which makes a slash inside a metadata name every |
| 34 | + * consumer's permanent obligation, with no measured demand for it. |
| 35 | + * |
| 36 | + * The extension set is the REGISTERED serializer set, deliberately NOT |
| 37 | + * ADR-0008 §10's `.json`-only rule: §10 governs the `metadata-fs` store, and |
| 38 | + * applying it verbatim here would silently drop `.yaml` and `.ts` metadata from |
| 39 | + * `listNames()` — a breakage this card never asked for. Under the manager's |
| 40 | + * default set (`typescript` / `json` / `yaml`) that leaves `.js` out, closing |
| 41 | + * the card's row-4 membership mismatch for free. |
| 42 | + * |
| 43 | + * --------------------------------------------------------------------------- |
| 44 | + * What the RECORD cases are for |
| 45 | + * --------------------------------------------------------------------------- |
| 46 | + * `RECORD:` cases pin behaviour this repair deliberately LEAVES ALONE, so the |
| 47 | + * divergence that remains is visible in a diff rather than implicit. Both of |
| 48 | + * them are the same half of the ruling — "a file that does not fit is neither |
| 49 | + * listed NOR resolvable" — which could not be taken here: each would invert a |
| 50 | + * landed #14341 pin in `filesystem-loader-keyed-items.test.ts`, a file under a |
| 51 | + * concurrent claim (PR #14627) when this landed. |
| 52 | + * |
| 53 | + * `CONTROL:` cases pin the things that must NOT move, and one of them |
| 54 | + * (`javascript` registered) is the reverse verification that the narrowing |
| 55 | + * reads the registered set rather than a second hard-coded list. |
| 56 | + */ |
| 57 | + |
| 58 | +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; |
| 59 | +import * as fs from 'node:fs/promises'; |
| 60 | +import * as os from 'node:os'; |
| 61 | +import * as path from 'node:path'; |
| 62 | +import type { MetadataFormat } from '@objectstack/spec/system'; |
| 63 | +import { MetadataManager } from '../metadata-manager.js'; |
| 64 | +import { FilesystemLoader } from './filesystem-loader.js'; |
| 65 | +import { JSONSerializer } from '../serializers/json-serializer.js'; |
| 66 | +import { YAMLSerializer } from '../serializers/yaml-serializer.js'; |
| 67 | +import { TypeScriptSerializer } from '../serializers/typescript-serializer.js'; |
| 68 | +import type { MetadataSerializer } from '../serializers/serializer-interface.js'; |
| 69 | + |
| 70 | +const TYPE = 'object'; |
| 71 | + |
| 72 | +/** The card's probe fixture, verbatim, plus a well-formed `.ts` module. */ |
| 73 | +const FIXTURE: Record<string, string> = { |
| 74 | + 'flat.json': JSON.stringify({ name: 'flat', label: 'Flat' }), |
| 75 | + 'dotted.config.json': JSON.stringify({ name: 'dotted.config' }), |
| 76 | + 'yamlish.yaml': 'name: yamlish\n', |
| 77 | + 'yamlish2.yml': 'name: yamlish2\n', |
| 78 | + 'noext': JSON.stringify({ name: 'noext' }), |
| 79 | + // `.ts` carrying the `export const` pattern `TypeScriptSerializer` needs, and |
| 80 | + // a JSON-compatible object literal. The card's row 4 claimed `.ts` was never |
| 81 | + // loaded; that was the probe's fixture, not the loader — see the CONTROL. |
| 82 | + 'scripted.ts': 'export const scripted = { "name": "scripted", "label": "Scripted" };\n', |
| 83 | + // Same pattern, `.js`: resolvable and listed before this repair, loadable by |
| 84 | + // nothing under the default format set. |
| 85 | + 'jsonly.js': 'export const jsonly = { "name": "jsonly" };\n', |
| 86 | +}; |
| 87 | + |
| 88 | +const NESTED: Record<string, string> = { |
| 89 | + 'account.json': JSON.stringify({ name: 'account', label: 'Account' }), |
| 90 | + 'nameless-nested.json': JSON.stringify({ label: 'Nameless nested' }), |
| 91 | +}; |
| 92 | + |
| 93 | +/** Exactly the flat files carrying an extension the DEFAULT set registers. */ |
| 94 | +const LISTED_UNDER_DEFAULT_SET = ['dotted.config', 'flat', 'scripted', 'yamlish', 'yamlish2']; |
| 95 | + |
| 96 | +let root: string; |
| 97 | + |
| 98 | +beforeAll(async () => { |
| 99 | + root = await fs.mkdtemp(path.join(os.tmpdir(), 'fsloader-reach-')); |
| 100 | + const typeDir = path.join(root, TYPE); |
| 101 | + await fs.mkdir(path.join(typeDir, 'crm'), { recursive: true }); |
| 102 | + |
| 103 | + for (const [rel, body] of Object.entries(FIXTURE)) { |
| 104 | + await fs.writeFile(path.join(typeDir, rel), body, 'utf-8'); |
| 105 | + } |
| 106 | + for (const [rel, body] of Object.entries(NESTED)) { |
| 107 | + await fs.writeFile(path.join(typeDir, 'crm', rel), body, 'utf-8'); |
| 108 | + } |
| 109 | +}); |
| 110 | + |
| 111 | +afterAll(async () => { |
| 112 | + await fs.rm(root, { recursive: true, force: true }); |
| 113 | +}); |
| 114 | + |
| 115 | +/** The manager's DEFAULT format set, exactly as `MetadataManager` builds it. */ |
| 116 | +function defaultSerializers(): Map<MetadataFormat, MetadataSerializer> { |
| 117 | + return new Map<MetadataFormat, MetadataSerializer>([ |
| 118 | + ['json', new JSONSerializer()], |
| 119 | + ['yaml', new YAMLSerializer()], |
| 120 | + ['typescript', new TypeScriptSerializer('typescript')], |
| 121 | + ]); |
| 122 | +} |
| 123 | + |
| 124 | +function loader(): FilesystemLoader { |
| 125 | + return new FilesystemLoader(root, defaultSerializers()); |
| 126 | +} |
| 127 | + |
| 128 | +/** A cold manager — empty registry, one filesystem loader answering. */ |
| 129 | +function coldManager(): MetadataManager { |
| 130 | + const manager = new MetadataManager({ formats: ['typescript', 'json', 'yaml'], loaders: [] }); |
| 131 | + manager.registerLoader(loader()); |
| 132 | + return manager; |
| 133 | +} |
| 134 | + |
| 135 | +describe('#14486 FilesystemLoader.list() reports only names findFile() resolves', () => { |
| 136 | + it('lists exactly the flat files carrying a registered extension', async () => { |
| 137 | + expect((await loader().list(TYPE)).sort()).toEqual(LISTED_UNDER_DEFAULT_SET); |
| 138 | + }); |
| 139 | + |
| 140 | + it('EVERY listed name resolves through findFile(), stat() and load()', async () => { |
| 141 | + // The bijection claim itself. Before the repair `account`, `nameless-nested` |
| 142 | + // and `noext` were listed and answered `false` / `null` / `null` here. |
| 143 | + const fsLoader = loader(); |
| 144 | + |
| 145 | + for (const name of await fsLoader.list(TYPE)) { |
| 146 | + expect(await fsLoader.exists(TYPE, name)).toBe(true); |
| 147 | + expect(await fsLoader.stat(TYPE, name)).not.toBeNull(); |
| 148 | + expect((await fsLoader.load(TYPE, name)).data).not.toBeNull(); |
| 149 | + } |
| 150 | + }); |
| 151 | + |
| 152 | + it('a NESTED file is no longer listed under its bare basename', async () => { |
| 153 | + const listed = await loader().list(TYPE); |
| 154 | + |
| 155 | + expect(listed).not.toContain('account'); |
| 156 | + expect(listed).not.toContain('nameless-nested'); |
| 157 | + }); |
| 158 | + |
| 159 | + it('an EXTENSION-LESS file is no longer listed', async () => { |
| 160 | + expect(await loader().list(TYPE)).not.toContain('noext'); |
| 161 | + }); |
| 162 | + |
| 163 | + it('a .js file leaves list() under the default set — row 4, closed for free', async () => { |
| 164 | + // Not a rule about `.js`: it is the extension set following the REGISTERED |
| 165 | + // serializers. Under the default set nothing can deserialize `javascript`, |
| 166 | + // so the file is now neither listed nor resolvable instead of being listed, |
| 167 | + // resolvable, and unloadable. |
| 168 | + const fsLoader = loader(); |
| 169 | + |
| 170 | + expect(await fsLoader.list(TYPE)).not.toContain('jsonly'); |
| 171 | + expect(await fsLoader.exists(TYPE, 'jsonly')).toBe(false); |
| 172 | + }); |
| 173 | + |
| 174 | + it('CONTROL: registering `javascript` puts the .js file back, listed AND loadable', async () => { |
| 175 | + // The reverse verification: the narrowing reads this instance's serializer |
| 176 | + // map, not a second hard-coded extension list. |
| 177 | + const serializers = defaultSerializers(); |
| 178 | + serializers.set('javascript', new TypeScriptSerializer('javascript')); |
| 179 | + const fsLoader = new FilesystemLoader(root, serializers); |
| 180 | + |
| 181 | + expect(await fsLoader.list(TYPE)).toContain('jsonly'); |
| 182 | + expect(await fsLoader.exists(TYPE, 'jsonly')).toBe(true); |
| 183 | + expect((await fsLoader.load(TYPE, 'jsonly')).data).toEqual({ name: 'jsonly' }); |
| 184 | + }); |
| 185 | + |
| 186 | + it('CONTROL: .yaml AND .yml both survive — §10 governs metadata-fs, not this set', async () => { |
| 187 | + const listed = await loader().list(TYPE); |
| 188 | + |
| 189 | + expect(listed).toContain('yamlish'); |
| 190 | + expect(listed).toContain('yamlish2'); |
| 191 | + }); |
| 192 | + |
| 193 | + it('CONTROL: only the final extension is stripped, so dotted.config.json lists as dotted.config', async () => { |
| 194 | + expect(await loader().list(TYPE)).toContain('dotted.config'); |
| 195 | + }); |
| 196 | + |
| 197 | + it('CONTROL: a well-formed .ts module loads — the card row-4 `.ts` claim was its fixture', async () => { |
| 198 | + // Re-measured as triage required: `TypeScriptSerializer` is registered under |
| 199 | + // the default set and deserializes an `export const` module whose object |
| 200 | + // literal is JSON-compatible. The card's probe missed because its `.ts` |
| 201 | + // fixture was not that, and `load()` threw where `loadMany()` drops silently. |
| 202 | + expect((await loader().load(TYPE, 'scripted')).data).toEqual({ |
| 203 | + name: 'scripted', |
| 204 | + label: 'Scripted', |
| 205 | + }); |
| 206 | + }); |
| 207 | +}); |
| 208 | + |
| 209 | +describe('#14486 the repair reaches MetadataManager', () => { |
| 210 | + it('listNames() and get() give the same answer for every name', async () => { |
| 211 | + const manager = coldManager(); |
| 212 | + |
| 213 | + for (const name of await manager.listNames(TYPE)) { |
| 214 | + expect(await manager.get(TYPE, name)).toBeDefined(); |
| 215 | + } |
| 216 | + }); |
| 217 | + |
| 218 | + it('the unresolvable names are gone from listNames()', async () => { |
| 219 | + const names = await coldManager().listNames(TYPE); |
| 220 | + |
| 221 | + expect(names.sort()).toEqual(LISTED_UNDER_DEFAULT_SET); |
| 222 | + }); |
| 223 | + |
| 224 | + it('get() still answers undefined for the shapes that stopped being listed', async () => { |
| 225 | + const manager = coldManager(); |
| 226 | + |
| 227 | + expect(await manager.get(TYPE, 'account')).toBeUndefined(); |
| 228 | + expect(await manager.get(TYPE, 'noext')).toBeUndefined(); |
| 229 | + }); |
| 230 | +}); |
| 231 | + |
| 232 | +describe('#14486 RECORD: the half of the ruling this PR could not take', () => { |
| 233 | + it('RECORD: loadMany() still returns bodies for files list() no longer names', async () => { |
| 234 | + // The ruling also pinned "nothing unlisted is returned by `loadMany()` |
| 235 | + // either". Filtering the shared walk would invert three landed #14341 pins |
| 236 | + // — `filesystem-loader-keyed-items.test.ts:113`, `:167`, `:187` — and its |
| 237 | + // `loadMany()` CONTROL at `:196` ("with every file", length 7), in a file |
| 238 | + // held by a concurrent claim (PR #14627). Recorded, not repaired. |
| 239 | + const bodies = await loader().loadMany<{ name?: string; label?: string }>(TYPE); |
| 240 | + |
| 241 | + expect(bodies).toContainEqual({ name: 'account', label: 'Account' }); |
| 242 | + expect(bodies).toContainEqual({ name: 'noext' }); |
| 243 | + // ...and the `.js` file is still absent, for the row-4 reason: no serializer. |
| 244 | + expect(bodies.some(body => body.name === 'jsonly')).toBe(false); |
| 245 | + }); |
| 246 | + |
| 247 | + it('RECORD: a path-shaped name still resolves, though nothing lists it', async () => { |
| 248 | + // The other half of "neither listed NOR resolvable". Refusing a separator in |
| 249 | + // `findFile()` would invert `filesystem-loader-keyed-items.test.ts:175`, and |
| 250 | + // `save()` would still CREATE nested files (it mkdir -p's the name's |
| 251 | + // dirname), so the write side would have to narrow in the same stroke. |
| 252 | + const fsLoader = loader(); |
| 253 | + |
| 254 | + expect(await fsLoader.exists(TYPE, path.join('crm', 'account'))).toBe(true); |
| 255 | + expect(await fsLoader.list(TYPE)).not.toContain(path.join('crm', 'account')); |
| 256 | + }); |
| 257 | +}); |
0 commit comments