From c105ace89d94df16c6aaa3e4e7cd7e49e39d8b96 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 23:04:00 +0000 Subject: [PATCH 1/5] wip(metadata): refuse an ambiguous metadata stem at list time (#14921) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- packages/metadata/src/index.ts | 13 + .../src/loaders/ambiguous-metadata-stem.ts | 118 ++++++ .../filesystem-loader-ambiguous-stem.test.ts | 365 ++++++++++++++++++ .../metadata/src/loaders/filesystem-loader.ts | 128 ++++-- packages/metadata/src/metadata-manager.ts | 18 + 5 files changed, 610 insertions(+), 32 deletions(-) create mode 100644 packages/metadata/src/loaders/ambiguous-metadata-stem.ts create mode 100644 packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts diff --git a/packages/metadata/src/index.ts b/packages/metadata/src/index.ts index 9033eb6525..bd7532ce54 100644 --- a/packages/metadata/src/index.ts +++ b/packages/metadata/src/index.ts @@ -19,6 +19,19 @@ export { MemoryLoader } from './loaders/memory-loader.js'; export { RemoteLoader } from './loaders/remote-loader.js'; export { DatabaseLoader, type DatabaseLoaderOptions } from './loaders/database-loader.js'; +// [#14921] The ambiguous-stem refusal. Published from the ROOT entry, not only +// from `./node` beside `FilesystemLoader`: the error reaches consumers through +// `MetadataManager.listNames()` / `list()`, which live here, and a caller that +// wants to tell "this deployment's metadata tree names one item twice" apart +// from a storage outage needs the predicate wherever it catches — not only +// where the loader is constructed. +export { + AmbiguousMetadataStemError, + isAmbiguousMetadataStemError, + AMBIGUOUS_METADATA_STEM_CODE, + AMBIGUOUS_METADATA_STEM_STATUS, +} from './loaders/ambiguous-metadata-stem.js'; + // Objects export { SysMetadataObject, SysMetadataHistoryObject } from '@objectstack/metadata-core'; diff --git a/packages/metadata/src/loaders/ambiguous-metadata-stem.ts b/packages/metadata/src/loaders/ambiguous-metadata-stem.ts new file mode 100644 index 0000000000..0b5b3f3696 --- /dev/null +++ b/packages/metadata/src/loaders/ambiguous-metadata-stem.ts @@ -0,0 +1,118 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14921] The refusal a metadata source tree earns by naming one item twice. + * + * ## The invariant this restores + * + * *What is listed is what is loadable.* `FilesystemLoader` derives a metadata + * name by stripping the extension from a flat file's basename, and resolves a + * name back to a file under a FIXED extension precedence (`.json` → `.yaml` → + * `.yml` → `.ts` → `.js`). Two files sharing a stem therefore produced one name + * TWICE in `list()` while only the first-precedence file was reachable through + * any name at all: the listed set and the addressable set stopped being the + * same set, and `loadMany()` kept returning both bodies. The loser was + * invisible — not missing, not reported, just never served. + * + * The failure is silent in the direction that matters for authoring, and the + * trigger is a move authors (human and AI) make constantly: convert + * `twin.json` to `twin.yaml` and leave the old file behind, or land one from + * each of two packages. Today the JSON one is served forever with no + * diagnostic anywhere, and `MetadataManager.admitLoaderItems()`'s documented + * "keep the first and say nothing" absorbs the collision a second time. + * + * ## The ruling (maintainer, via the director seat on #14921, 2026-09-05) + * + * Option 1 of three: **refuse the ambiguous stem loudly at list time.** Two + * files sharing a stem across the registered extensions is an AUTHORING ERROR, + * reported with both paths named, never resolved by precedence. Not taken: + * option 2 (keep the precedence and log at `warn` — with zero instances in any + * measured tree, nobody reads that log, and the invariant stays broken) and + * option 3 (make the extension part of the name for the non-first file — a + * naming rule invented for an error state, grown into the contract). + * + * The narrowing is cheap for the reason the grade records: no measured + * production or example tree carries two files with one stem, so no existing + * tree goes red. It is a narrowing with almost no migration account. + * + * ## Why a brand and a predicate rather than bare `instanceof` + * + * `MetadataManager`'s plural reads catch per loader on purpose (#5108/#14423): + * a storage outage must degrade to a short-but-served list rather than take the + * whole enumeration down. This refusal is the opposite kind of fact — an + * author's tree is malformed and no retry fixes it — so those seams have to + * re-raise THIS error while still absorbing every other one. A predicate over + * a `Symbol.for` brand is the discrimination that survives duplicate copies of + * this module in a consumer's dependency graph, where `instanceof` does not. + * Same shape, and for the same reason, as `@objectstack/core`'s + * `isAuthzStoreUnavailableError`. + */ + +/** ADR-0112 wire code for the refusal. */ +export const AMBIGUOUS_METADATA_STEM_CODE = 'AMBIGUOUS_METADATA_STEM' as const; + +/** + * HTTP status a transport should answer. + * + * 500, deliberately: the REQUEST is well formed and no caller can fix it by + * sending something else — the deployment's own metadata source tree is + * ambiguous. Not 503 (nothing is transient here; a retry answers identically + * until a file is deleted or renamed) and not 4xx (the caller did nothing + * wrong). + */ +export const AMBIGUOUS_METADATA_STEM_STATUS = 500 as const; + +const AMBIGUOUS_METADATA_STEM_BRAND = Symbol.for('objectstack.metadata.ambiguousStem'); + +/** + * Thrown when one metadata name is derived from more than one file among a + * loader's REGISTERED extensions. + * + * The message names every colliding path and the metadata type, because those + * are exactly the two things an author needs and neither is recoverable from + * the name alone: a bare "duplicate `twin`" sends them looking through a tree + * for something they already believe they deleted. + */ +export class AmbiguousMetadataStemError extends Error { + /** Brand — see the module doc on why this is not `instanceof`. */ + readonly [AMBIGUOUS_METADATA_STEM_BRAND] = true as const; + /** ADR-0112 wire code. */ + readonly code = AMBIGUOUS_METADATA_STEM_CODE; + /** HTTP status a transport should answer. */ + readonly status = AMBIGUOUS_METADATA_STEM_STATUS; + /** The metadata type whose directory holds the collision (e.g. `object`). */ + readonly type: string; + /** The one name both files derive to. */ + readonly stem: string; + /** Every colliding file, absolute, sorted — never just the winner. */ + readonly paths: readonly string[]; + + constructor(type: string, stem: string, paths: readonly string[]) { + const sorted = [...paths].sort(); + super( + `Ambiguous metadata name \`${stem}\` for type \`${type}\`: ${sorted.length} files ` + + `resolve to the same name — ${sorted.map(p => `\`${p}\``).join(', ')}. ` + + `Only the first would ever be served (extension precedence: .json, .yaml, .yml, .ts, .js), ` + + `so the others are listed and unreachable. Delete or rename all but one.`, + ); + this.name = 'AmbiguousMetadataStemError'; + this.type = type; + this.stem = stem; + this.paths = sorted; + } +} + +/** + * True when `err` is the ambiguous-stem refusal above. + * + * The predicate every catch-and-degrade seam uses to re-raise THIS one without + * loosening its handling of anything else — a storage outage still degrades, an + * author's malformed tree does not. + */ +export function isAmbiguousMetadataStemError(err: unknown): err is AmbiguousMetadataStemError { + return ( + typeof err === 'object' + && err !== null + && (err as Record)[AMBIGUOUS_METADATA_STEM_BRAND] === true + ); +} diff --git a/packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts b/packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts new file mode 100644 index 0000000000..5be499e9f0 --- /dev/null +++ b/packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts @@ -0,0 +1,365 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #14921 — two files sharing one stem are REFUSED, never resolved by + * extension precedence. + * + * --------------------------------------------------------------------------- + * The defect (measured on `origin/main` @ c463d03e0, this fixture) + * --------------------------------------------------------------------------- + * `FilesystemLoader` derives a name by stripping a flat file's extension, and + * resolves a name back under a FIXED precedence (`.json` → `.yaml` → `.yml` → + * `.ts` → `.js`). With `object/twin.json` and `object/twin.yaml` both present: + * + * twin occurrences in list(): 2 <- one name, listed twice + * twin resolves to : twin.json + * + * So `twin.yaml` was counted in the list and addressable through no name at + * all, and `loadMany()` returned BOTH bodies. `MetadataManager.listNames()` + * unions loader output into a `Set`, which collapses the duplicate and takes + * the count discrepancy with it — the file stayed unreachable either way, so a + * clean-looking `listNames()` was never evidence the problem was absorbed. + * + * The invariant that broke, in one line: **what is listed is what is + * loadable.** The listed set and the addressable set stopped being the same + * set. + * + * --------------------------------------------------------------------------- + * The rule this pins (maintainer ruling on #14921, via the director seat, + * 2026-09-05 — option 1 of three, verbatim 「同意」) + * --------------------------------------------------------------------------- + * Two files sharing a stem across the REGISTERED extensions is an authoring + * error, refused at list time with both paths and the type named, never served + * by precedence. Not taken: option 2 (keep the precedence, log at `warn` — with + * zero instances in any measured tree nobody reads that log, and the invariant + * stays broken) and option 3 (make the extension part of the name for the + * non-first file — a naming rule invented for an error state). + * + * --------------------------------------------------------------------------- + * The three pins the ruling requires + * --------------------------------------------------------------------------- + * PIN 1 the duplicate is refused, with BOTH paths named + * PIN 2 a single file per stem is still listed AND loadable + * PIN 3 the `loadMany()` two-body symptom is gone + * + * `CONTROL:` cases pin what must NOT move — chiefly that the refusal reads + * THIS instance's registered serializer set rather than a second hard-coded + * extension list, that it covers exactly the flat/resolvable domain `list()` + * reports, and that a storage OUTAGE still degrades at the manager seams + * instead of being swept up by the new rethrow. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import type { MetadataFormat, MetadataLoaderContract, MetadataStats } from '@objectstack/spec/system'; +import { MetadataManager } from '../metadata-manager.js'; +import { FilesystemLoader } from './filesystem-loader.js'; +import type { MetadataLoader } from './loader-interface.js'; +import { + AmbiguousMetadataStemError, + isAmbiguousMetadataStemError, + AMBIGUOUS_METADATA_STEM_CODE, + AMBIGUOUS_METADATA_STEM_STATUS, +} from './ambiguous-metadata-stem.js'; +import { JSONSerializer } from '../serializers/json-serializer.js'; +import { YAMLSerializer } from '../serializers/yaml-serializer.js'; +import { TypeScriptSerializer } from '../serializers/typescript-serializer.js'; +import type { MetadataSerializer } from '../serializers/serializer-interface.js'; + +const TYPE = 'object'; + +/** The card's probe fixture, verbatim, plus the shapes the CONTROLs need. */ +const AMBIGUOUS: Record = { + 'twin.json': JSON.stringify({ name: 'twin-json' }), + 'twin.yaml': 'name: twin-yaml\n', +}; + +/** + * One file per stem, three registered extensions — the tree PIN 2 proves is + * untouched. Kept in its own root: the refusal is per type directory, so a + * collision anywhere in `object/` would take these down with it and PIN 2 would + * be testing nothing. + */ +const CLEAN: Record = { + 'solo.json': JSON.stringify({ name: 'solo', label: 'Solo' }), + 'alpha.yaml': 'name: alpha\n', + 'beta.ts': 'export const beta = { "name": "beta" };\n', +}; + +/** `.js` is NOT in the manager's default format set — see the CONTROL pair. */ +const REGISTERED_SET_PROBE: Record = { + 'dual.json': JSON.stringify({ name: 'dual-json' }), + 'dual.js': 'export const dual = { "name": "dual-js" };\n', +}; + +let ambiguousRoot: string; +let cleanRoot: string; +let probeRoot: string; + +beforeAll(async () => { + ambiguousRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'fsloader-ambig-')); + cleanRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'fsloader-clean-')); + probeRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'fsloader-probe-')); + + const ambiguousDir = path.join(ambiguousRoot, TYPE); + await fs.mkdir(ambiguousDir, { recursive: true }); + for (const [rel, body] of Object.entries(AMBIGUOUS)) { + await fs.writeFile(path.join(ambiguousDir, rel), body, 'utf-8'); + } + // A SECOND type directory under the same root, deliberately clean: the + // refusal is scoped to the directory that holds the collision. + await fs.mkdir(path.join(ambiguousRoot, 'view'), { recursive: true }); + await fs.writeFile( + path.join(ambiguousRoot, 'view', 'ok.json'), + JSON.stringify({ name: 'ok' }), + 'utf-8', + ); + + const cleanDir = path.join(cleanRoot, TYPE); + await fs.mkdir(path.join(cleanDir, 'crm'), { recursive: true }); + for (const [rel, body] of Object.entries(CLEAN)) { + await fs.writeFile(path.join(cleanDir, rel), body, 'utf-8'); + } + // Nested twin of a FLAT name. Not a collision: a nested file is neither + // listed nor resolvable (#14486), so it derives no name to collide with. + await fs.writeFile( + path.join(cleanDir, 'crm', 'solo.json'), + JSON.stringify({ name: 'nested-solo' }), + 'utf-8', + ); + + const probeDir = path.join(probeRoot, TYPE); + await fs.mkdir(probeDir, { recursive: true }); + for (const [rel, body] of Object.entries(REGISTERED_SET_PROBE)) { + await fs.writeFile(path.join(probeDir, rel), body, 'utf-8'); + } +}); + +afterAll(async () => { + for (const root of [ambiguousRoot, cleanRoot, probeRoot]) { + await fs.rm(root, { recursive: true, force: true }); + } +}); + +/** The manager's DEFAULT format set, exactly as `MetadataManager` builds it. */ +function defaultSerializers(): Map { + return new Map([ + ['json', new JSONSerializer()], + ['yaml', new YAMLSerializer()], + ['typescript', new TypeScriptSerializer('typescript')], + ]); +} + +const loaderFor = (root: string): FilesystemLoader => + new FilesystemLoader(root, defaultSerializers()); + +/** A cold manager — empty registry, one filesystem loader answering. */ +function coldManager(root: string): MetadataManager { + const manager = new MetadataManager({ formats: ['typescript', 'json', 'yaml'], loaders: [] }); + manager.registerLoader(loaderFor(root)); + return manager; +} + +/** Reject-or-null, so the thrown value itself can be asserted on. */ +const caught = async (run: () => Promise): Promise => + run().then(() => null, (e: unknown) => e); + +const twinJson = (): string => path.join(ambiguousRoot, TYPE, 'twin.json'); +const twinYaml = (): string => path.join(ambiguousRoot, TYPE, 'twin.yaml'); + +describe('#14921 PIN 1 — the ambiguous stem is refused, with BOTH paths named', () => { + it('list() throws the ADR-0112 envelope: code, status, and both paths + the type in the message', async () => { + const error = await caught(() => loaderFor(ambiguousRoot).list(TYPE)); + + // The envelope, not the throw: a bare `.toThrow()` would stay green against + // any `Error` at all, including one thrown for a different reason. + expect(isAmbiguousMetadataStemError(error)).toBe(true); + expect((error as AmbiguousMetadataStemError).code).toBe(AMBIGUOUS_METADATA_STEM_CODE); + expect((error as AmbiguousMetadataStemError).status).toBe(AMBIGUOUS_METADATA_STEM_STATUS); + + // The ruling's own words: "both paths and the type in the message". + const message = (error as Error).message; + expect(message).toContain(twinJson()); + expect(message).toContain(twinYaml()); + expect(message).toContain(TYPE); + }); + + it('the error carries both paths structurally, sorted — never just the precedence winner', async () => { + const error = (await caught(() => + loaderFor(ambiguousRoot).list(TYPE), + )) as AmbiguousMetadataStemError; + + expect(error.type).toBe(TYPE); + expect(error.stem).toBe('twin'); + expect([...error.paths]).toEqual([twinJson(), twinYaml()].sort()); + }); + + it('MetadataManager.listNames() PROPAGATES rather than absorbing', async () => { + // The `Set` at this layer hides the DUPLICATE, never the unreachability — + // so a clean-looking `listNames()` was the misreading the ruling names. + const error = await caught(() => coldManager(ambiguousRoot).listNames(TYPE)); + + expect(isAmbiguousMetadataStemError(error)).toBe(true); + }); + + it('MetadataManager.list() propagates too — the sibling plural read', async () => { + // `admitLoaderItems()` refuses before contributing anything, so absorbing + // here would answer with a `degraded` set missing EVERY item this loader + // holds: an authoring error rendered as a storage outage. + const error = await caught(() => coldManager(ambiguousRoot).list(TYPE)); + + expect(isAmbiguousMetadataStemError(error)).toBe(true); + }); + + it('the refusal is scoped to the type directory that holds the collision', async () => { + expect(await loaderFor(ambiguousRoot).list('view')).toEqual(['ok']); + }); +}); + +describe('#14921 PIN 2 — one file per stem is still listed AND loadable', () => { + it('lists every flat file carrying a registered extension', async () => { + expect((await loaderFor(cleanRoot).list(TYPE)).sort()).toEqual(['alpha', 'beta', 'solo']); + }); + + it('EVERY listed name still resolves through exists(), stat() and load()', async () => { + const loader = loaderFor(cleanRoot); + + for (const name of await loader.list(TYPE)) { + expect(await loader.exists(TYPE, name)).toBe(true); + expect(await loader.stat(TYPE, name)).not.toBeNull(); + expect((await loader.load(TYPE, name)).data).not.toBeNull(); + } + }); + + it('the manager still lists and gets them', async () => { + const manager = coldManager(cleanRoot); + + expect((await manager.listNames(TYPE)).sort()).toEqual(['alpha', 'beta', 'solo']); + expect(await manager.get(TYPE, 'solo')).toEqual({ name: 'solo', label: 'Solo' }); + }); + + it('CONTROL: a NESTED file sharing a flat name is not a collision', async () => { + // `crm/solo.json` sits beside a flat `solo.json`. A nested file derives no + // resolvable name at all (#14486), so there is nothing for it to collide + // with — and treating basenames as the unit would refuse this good tree. + const loader = loaderFor(cleanRoot); + + expect(await loader.list(TYPE)).toContain('solo'); + expect((await loader.load(TYPE, 'solo')).data).toEqual({ name: 'solo', label: 'Solo' }); + }); +}); + +describe('#14921 PIN 3 — the loadMany() two-body symptom is gone', () => { + it('loadMany() no longer answers with two bodies for one name', async () => { + const bodies = await caught(() => loaderFor(ambiguousRoot).loadMany<{ name?: string }>(TYPE)); + + // Refused, not silently de-duplicated: picking a winner here is option 2, + // which the ruling declined — the loser would stay unreachable and the + // listed set would stay different from the addressable one. + expect(isAmbiguousMetadataStemError(bodies)).toBe(true); + expect(Array.isArray(bodies)).toBe(false); + }); + + it('loadManyKeyed() refuses on the same walk', async () => { + const keyed = await caught(() => loaderFor(ambiguousRoot).loadManyKeyed(TYPE)); + + expect(isAmbiguousMetadataStemError(keyed)).toBe(true); + }); + + it('a `limit` cannot buy a caller past the refusal', async () => { + // The check runs over the whole matched set, before `limit` truncates, so + // whether a tree is refused never depends on how much was asked for. + const bodies = await caught(() => loaderFor(ambiguousRoot).loadMany(TYPE, { limit: 1 })); + + expect(isAmbiguousMetadataStemError(bodies)).toBe(true); + }); + + it('CONTROL: loadMany() on the clean tree is unchanged', async () => { + const bodies = await loaderFor(cleanRoot).loadMany<{ name?: string }>(TYPE); + + // Flat AND nested, exactly as before: this walk was never narrowed to the + // listed set (#14486 RECORD), and this card does not narrow it either. + expect(bodies.map(b => b.name).sort()).toEqual(['alpha', 'beta', 'nested-solo', 'solo']); + }); +}); + +describe('#14921 CONTROL — the refusal reads the REGISTERED extension set', () => { + it('`dual.json` + `dual.js` is NOT ambiguous under the default set', async () => { + // `.js` carries no registered serializer there, so it derives no name — + // the same reason #14486 stopped listing it. + const loader = loaderFor(probeRoot); + + expect(await loader.list(TYPE)).toEqual(['dual']); + expect((await loader.load(TYPE, 'dual')).data).toEqual({ name: 'dual-json' }); + }); + + it('registering `javascript` makes the SAME tree ambiguous — the reverse verification', async () => { + // Nothing on disk changed. If the refusal read a second hard-coded + // extension list instead of this instance's serializer map, this case + // would answer identically to the one above. + const serializers = defaultSerializers(); + serializers.set('javascript', new TypeScriptSerializer('javascript')); + const loader = new FilesystemLoader(probeRoot, serializers); + + const error = await caught(() => loader.list(TYPE)); + + expect(isAmbiguousMetadataStemError(error)).toBe(true); + expect((error as AmbiguousMetadataStemError).stem).toBe('dual'); + expect([...(error as AmbiguousMetadataStemError).paths]).toEqual( + [path.join(probeRoot, TYPE, 'dual.js'), path.join(probeRoot, TYPE, 'dual.json')].sort(), + ); + }); +}); + +/** + * A loader whose reads fail the way a datasource fails: an `ECONNRESET` with no + * ambiguous-stem brand on it. + * + * Hand-rolled rather than driven through `DatabaseLoader`, because the thing + * under test is the DISCRIMINATION in the manager's two `catch` blocks, and + * what makes a case sharp here is only that the thrown value is un-branded. A + * real driver harness would add machinery without adding evidence. + */ +class OutageLoader implements MetadataLoader { + readonly contract: MetadataLoaderContract = { + name: 'outage', + protocol: 'test:', + capabilities: { read: true, write: false, watch: false, list: true }, + supportedFormats: ['json'], + supportsWatch: false, + supportsWrite: false, + supportsCache: false, + }; + + private fail(): never { + throw Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' }); + } + + async load(): Promise { this.fail(); } + async loadMany(): Promise { this.fail(); } + async exists(): Promise { return false; } + async stat(): Promise { return null; } + async list(): Promise { this.fail(); } +} + +describe('#14921 CONTROL — a storage OUTAGE still degrades, it is not swept up', () => { + it('listNames() still absorbs an un-branded loader failure', async () => { + const manager = new MetadataManager({ formats: ['json'], loaders: [] }); + manager.registerLoader(new OutageLoader()); + + // Degraded, exactly as #14423 left it: an array, short and served. + expect(await manager.listNames(TYPE)).toEqual([]); + }); + + it('list() still absorbs it too, and still reports the read as degraded', async () => { + const manager = new MetadataManager({ formats: ['json'], loaders: [] }); + manager.registerLoader(new OutageLoader()); + + const diagnosed = await manager.listDiagnosed(TYPE); + + expect(diagnosed.items).toEqual([]); + expect(diagnosed.degraded).toBe(true); + }); +}); diff --git a/packages/metadata/src/loaders/filesystem-loader.ts b/packages/metadata/src/loaders/filesystem-loader.ts index 561d7cb916..138de82cec 100644 --- a/packages/metadata/src/loaders/filesystem-loader.ts +++ b/packages/metadata/src/loaders/filesystem-loader.ts @@ -22,6 +22,7 @@ import type { import type { Logger } from '@objectstack/core'; import type { MetadataLoader, MetadataKeyedItem } from './loader-interface.js'; import type { MetadataSerializer } from '../serializers/serializer-interface.js'; +import { AmbiguousMetadataStemError } from './ambiguous-metadata-stem.js'; /** * The pre-#14205 key: a body's own top-level `name`, when it has one. Kept for @@ -259,37 +260,46 @@ export class FilesystemLoader implements MetadataLoader { path.join(typeDir, pattern) ); + // [#14921] Matched first, read second. The two-body answer this card + // names is produced HERE, not by `list()`: the walk reads both files of + // a colliding stem and hands back both bodies while only one of them can + // ever be addressed. The refusal therefore has to sit in front of the + // whole matched set — not per pattern, and not after `limit` truncates — + // so whether a tree is refused cannot depend on how many items the + // caller happened to ask for. An absolute pattern makes `glob` answer + // with absolute paths, which is what `resolvableNames()` measures. + const files: string[] = []; for (const pattern of globPatterns) { - const files = await glob(pattern, { - ignore: ['**/node_modules/**', '**/*.test.*', '**/*.spec.*', '**/*[*]*'], - nodir: true, - }); - - for (const file of files) { - if (limit && items.length >= limit) { - break; - } + files.push( + ...(await glob(pattern, { + ignore: ['**/node_modules/**', '**/*.test.*', '**/*.spec.*', '**/*[*]*'], + nodir: true, + })), + ); + } - try { - const content = await fs.readFile(file, 'utf-8'); - const format = this.detectFormat(file); - const serializer = this.getSerializer(format); - - if (serializer) { - const data = serializer.deserialize(content); - items.push({ file, data }); - } - } catch (error) { - this.logger?.warn('Failed to load file', { - file, - error: error instanceof Error ? error.message : String(error), - }); - } - } + this.resolvableNames(type, typeDir, files); + for (const file of files) { if (limit && items.length >= limit) { break; } + + try { + const content = await fs.readFile(file, 'utf-8'); + const format = this.detectFormat(file); + const serializer = this.getSerializer(format); + + if (serializer) { + const data = serializer.deserialize(content); + items.push({ file, data }); + } + } catch (error) { + this.logger?.warn('Failed to load file', { + file, + error: error instanceof Error ? error.message : String(error), + }); + } } return items; @@ -366,18 +376,13 @@ export class FilesystemLoader implements MetadataLoader { async list(type: string): Promise { const typeDir = path.join(this.rootDir, type); + let files: string[]; try { - const files = await glob('**/*', { + files = await glob('**/*', { cwd: typeDir, ignore: ['**/node_modules/**', '**/*.test.*', '**/*.spec.*'], nodir: true, }); - - // `cwd` makes these relative; `resolvableNameForPath()` measures against - // the type directory, so hand it the absolute path it expects. - return files - .map(file => this.resolvableNameForPath(typeDir, path.join(typeDir, file))) - .filter((name): name is string => name !== null); } catch (error) { this.logger?.error('Failed to list', undefined, { type, @@ -385,6 +390,15 @@ export class FilesystemLoader implements MetadataLoader { }); return []; } + + // [#14921] Derived OUTSIDE that `catch`, deliberately: the walk failing is + // a degradation this method has always swallowed into `[]`, while an + // ambiguous stem is an authoring error the caller must see. Left inside, + // this method's own diagnostic would eat the refusal and answer `[]` — + // the same silence the refusal exists to end. + // `cwd` makes these relative; `resolvableNameForPath()` measures against + // the type directory, so hand it the absolute path it expects. + return this.resolvableNames(type, typeDir, files.map(file => path.join(typeDir, file))); } async save( @@ -562,6 +576,56 @@ export class FilesystemLoader implements MetadataLoader { return FilesystemLoader.nameFromFilename(rel); } + /** + * [#14921] The names this loader reports for `files` — and the ONE place an + * ambiguous stem is refused. + * + * Shared by {@link list} and {@link loadManyEntries} so the two can never + * disagree about which trees are admissible: a stem that `list()` refuses + * must not still be walked and returned as two bodies by `loadMany()`, which + * is exactly the split this card measured. + * + * Refuses on the FIRST colliding name in sorted order, so a tree holding more + * than one collision always names the same one — a refusal that moves + * between runs reads as flakiness rather than as the fixed authoring error it + * is. Paths are deduplicated because two overlapping `patterns` legitimately + * match one file twice, and counting that as a collision would refuse a + * perfectly good tree. + * + * ⛔ Not a precedence resolver. Picking a winner here is what the ruling + * declined (option 2, keep the precedence and log): the loser would stay + * unreachable and the listed set would stay different from the addressable + * one. + */ + private resolvableNames(type: string, typeDir: string, files: readonly string[]): string[] { + const byName = new Map>(); + + for (const file of files) { + const name = this.resolvableNameForPath(typeDir, file); + + if (name === null) { + continue; + } + + let paths = byName.get(name); + if (!paths) { + paths = new Set(); + byName.set(name, paths); + } + paths.add(file); + } + + for (const name of [...byName.keys()].sort()) { + const paths = byName.get(name)!; + + if (paths.size > 1) { + throw new AmbiguousMetadataStemError(type, name, [...paths]); + } + } + + return [...byName.keys()]; + } + /** * Find file for a given type and name */ diff --git a/packages/metadata/src/metadata-manager.ts b/packages/metadata/src/metadata-manager.ts index 894a2cd491..e774ce9e11 100644 --- a/packages/metadata/src/metadata-manager.ts +++ b/packages/metadata/src/metadata-manager.ts @@ -68,6 +68,7 @@ import type { MetadataSerializer } from './serializers/serializer-interface.js'; import type { IDataDriver, IDataEngine } from '@objectstack/spec/contracts'; import type { MetadataLoader, MetadataKeyedItem } from './loaders/loader-interface.js'; import { DatabaseLoader } from './loaders/database-loader.js'; +import { isAmbiguousMetadataStemError } from './loaders/ambiguous-metadata-stem.js'; import { generateSimpleDiff, generateDiffSummary } from './utils/metadata-history-utils.js'; import type { MetadataRepository, @@ -1173,6 +1174,15 @@ export class MetadataManager implements IMetadataService { await this.admitLoaderItems(loader, type, items); this.reportLoaderReadRecovered(loader.contract.name); } catch (e) { + // [#14921] Same discrimination as `listNames`, and needed for the same + // reason: `admitLoaderItems` refuses an ambiguous stem BEFORE it + // contributes anything, so absorbing it here would drop every item + // this loader holds into a `degraded` partial set — an authoring error + // rendered as a storage outage, which is the one reading it must never + // get. A real outage still degrades exactly as before. + if (isAmbiguousMetadataStemError(e)) { + throw e; + } degraded = true; errors.push(`${loader.contract.name}: ${e instanceof Error ? e.message : String(e)}`); this.reportLoaderReadFailure(loader.contract.name, type, e); @@ -1620,6 +1630,14 @@ export class MetadataManager implements IMetadataService { result.forEach(item => names.add(item)); this.reportLoaderReadRecovered(loader.contract.name); } catch (e) { + // [#14921] PROPAGATE, never absorb: an ambiguous metadata stem is an + // authoring error, not an outage. Degrading it here would answer with + // a set that silently omits every name this loader holds while the + // server keeps reporting healthy — precisely the silence the refusal + // exists to end, moved one layer up. Everything else still degrades. + if (isAmbiguousMetadataStemError(e)) { + throw e; + } // [#14423] Parity with `loadMany` and `list()` — see this method's // docblock. Same seam, same verdict, same helper. this.reportLoaderReadFailure(loader.contract.name, type, e); From f8f02d2be7a115e609dae7de977bda2d3bb23420 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 23:36:36 +0000 Subject: [PATCH 2/5] fix(metadata): refuse an ambiguous metadata stem instead of serving it by extension precedence (#14921) Two files sharing one stem across the registered extensions produced one name twice in FilesystemLoader.list() while only the first-precedence file was reachable by any name, and loadMany() returned both bodies. list() and the shared loadMany()/loadManyKeyed() walk now refuse with an ADR-0112 envelope naming both paths and the type; MetadataManager.listNames() and list() propagate it rather than absorbing it into their per-loader degradation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- .changeset/metadata-ambiguous-stem-refused.md | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 .changeset/metadata-ambiguous-stem-refused.md diff --git a/.changeset/metadata-ambiguous-stem-refused.md b/.changeset/metadata-ambiguous-stem-refused.md new file mode 100644 index 0000000000..33443247db --- /dev/null +++ b/.changeset/metadata-ambiguous-stem-refused.md @@ -0,0 +1,67 @@ +--- +"@objectstack/metadata": minor +--- + +fix(metadata): two files sharing one stem are refused with both paths named, instead of one being listed twice and served by extension precedence (#14921) + +**BREAKING** accept-set narrowing on `FilesystemLoader`, shipped as `minor` +under the repo's launch-window convention for breaking changes. Ruled on +#14921 (2026-09-05, option 1 of three). + +**Remedy: delete or rename the duplicate file.** The refusal names every +colliding path and the metadata type, so the fix is visible at the point of +failure. + +`FilesystemLoader` derives a metadata name by stripping a flat file's +extension, and resolves a name back to a file under a FIXED extension +precedence (`.json` → `.yaml` → `.yml` → `.ts` → `.js`). Two files sharing a +stem therefore produced one name **twice** in `list()` while only the +first-precedence file was reachable through any name at all. With +`object/twin.json` and `object/twin.yaml` both present, `list()` answered +`['twin', 'twin']`, `twin.yaml` was addressable through nothing, and +`loadMany()` returned both bodies. `MetadataManager.listNames()` unions loader +output into a `Set`, which collapsed the duplicate and took the count +discrepancy with it — the file stayed unreachable either way, so a clean +`listNames()` was never evidence the collision had been absorbed. + +The invariant that broke: **what is listed is what is loadable.** The listed +set and the addressable set stopped being the same set. The failure was silent +in the direction that matters for authoring — convert `twin.json` to +`twin.yaml` and leave the old file behind, or land one from each of two +packages, and the JSON one is served forever with no diagnostic anywhere, +while `admitLoaderItems()`'s documented "keep the first and say nothing" +absorbs the collision a second time. + +`FilesystemLoader.list()` now throws `AmbiguousMetadataStemError` +(`AMBIGUOUS_METADATA_STEM`, HTTP 500) naming both paths and the type, and the +same refusal fronts the shared `loadMany()` / `loadManyKeyed()` walk, so the +two-body answer is gone rather than de-duplicated. `MetadataManager.listNames()` +and `list()` **propagate** it rather than absorbing it into their per-loader +degradation: an ambiguous stem is an authoring error no retry fixes, and +degrading it would drop every item the loader holds into a short-but-served +list while the server keeps reporting healthy. A real storage outage still +degrades exactly as before — the seams discriminate on a branded predicate, +`isAmbiguousMetadataStemError`, not on a blanket rethrow. + +**Refused shape**, precisely: two or more files **directly under +`ROOT/TYPE/`** whose basenames differ only by an extension belonging to one of +**this instance's registered serializers**. Register `javascript` and +`dual.json` + `dual.js` becomes ambiguous; under the manager's default format +set (`typescript` / `json` / `yaml`) it is not, because `.js` derives no name. +Nested files are untouched — they are neither listed nor resolvable (#14486), +so `crm/solo.json` beside a flat `solo.json` is not a collision. The refusal is +scoped to the type directory that holds it: a clean `view/` still lists while +`object/` refuses. + +New exports from the package root entry: `AmbiguousMetadataStemError`, +`isAmbiguousMetadataStemError`, `AMBIGUOUS_METADATA_STEM_CODE`, +`AMBIGUOUS_METADATA_STEM_STATUS`. + +Measured migration cost, which is what makes this narrowing cheap: **no tree in +this repository carries the shape.** A walk of all 7,770 tracked files across +526 directories found zero stem collisions among `.json` / `.yaml` / `.yml` / +`.ts` / `.js`, confirmed independently by a `git ls-files` pass, and the repo +holds no `.yaml`/`.yml` metadata file at all outside CI and workspace config. +No existing tree goes red. + + From a05742ec8d8a755a279e2c687163337e0e7243a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 23:55:39 +0000 Subject: [PATCH 3/5] chore(runtime): classify the #14921 AMBIGUOUS_METADATA_STEM site in the dispatcher error-code vocabulary table The ADR-0112 envelope added in packages/metadata is a new code-stamping site. Reachability measured from the call graph: MetadataManager.listNames()/list() re-raise it, listObjects() is list('object'), and domains/mcp.ts awaits meta.listObjects() with no try on the MCP request path, so it reaches the dispatcher door. Verdict pending-registration; the ledger entry is the packages/spec lane's call and is not made here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- .../src/dispatcher-error-vocabulary.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index 03da276c25..016add74d3 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -649,6 +649,42 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ 'and registering the code is what ratchets the row out again.', }, + // ── pending registration [#14921]: a metadata-tree refusal that reaches a + // ── dispatcher-door read ─────────────────────────────────────────────── + // Not a widened scan and not a demotion: this producer is NEW. #14921 made + // `FilesystemLoader.list()` (and the shared `loadMany()` walk behind it) + // refuse a metadata name derived from more than one file, where before it + // reported the name twice and served the first by extension precedence. + // The refusal carries the ADR-0112 envelope the ruling ordered, which is + // what puts a site here to classify. + { + code: 'AMBIGUOUS_METADATA_STEM', + file: 'packages/metadata/src/loaders/ambiguous-metadata-stem.ts', + shape: 'classconst', + door: 'dispatcher', + verdict: 'pending-registration', + why: + "The #14921 refusal: two files under one `ROOT/TYPE/` deriving one metadata name across this " + + 'loader\'s REGISTERED extensions is an authoring error, refused with both paths and the type ' + + 'named rather than resolved by the `.json` → `.yaml` → `.yml` → `.ts` → `.js` precedence ' + + '(maintainer ruling, director seat, 2026-09-05, option 1 of three). ⭐ Reachability, which is ' + + 'the only question this table answers, is DERIVED FROM THE CALL GRAPH and is a single ' + + 'uncaught hop: `MetadataManager.listNames()` and `list()` re-raise this error rather than ' + + 'absorbing it into their per-loader degradation (that discrimination is the point — a storage ' + + 'outage still degrades), `listObjects()` is `list(\'object\')`, and ' + + '`packages/runtime/src/domains/mcp.ts` `listObjectSummaries` awaits `meta.listObjects()` with ' + + 'NO `try` around it, on the MCP bridge\'s request path. So the throw leaves the domain handler ' + + 'and reaches `HttpDispatcher.errorFromThrown` — the dispatcher door. ⚠️ The measured in-repo ' + + 'population of trees that can raise it is ZERO (a walk of 7,770 tracked files over 526 ' + + 'directories found no stem collision among the registered extensions), so no existing tree ' + + 'reaches the door today; the row records the PRODUCER, which is what this table is for, and ' + + 'a producer nobody triggers yet is exactly the one that hides. Since #9106 the door narrows, ' + + 'so the body parses and `AMBIGUOUS_METADATA_STEM` rides `declaredCode` while `error.code` ' + + 'takes the member the 500 derives — the demote this verdict names. ⛔ Registering it widens ' + + '`ApiErrorSchema.code` and is the `packages/spec` lane\'s call, NOT made here: this row is ' + + 'that batch\'s input, and the registration is what ratchets it out again.', + }, + // ── boot refusals: no HTTP boundary exists yet ───────────────────────── // [#9460] The four `MigrationJournalRefusal` codes below arrive through the // same code-carrying-helper shape as `owd_widening_forbidden` — a class From 457f305319730f20c757d832f63ffbb217f3a83c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 00:01:11 +0000 Subject: [PATCH 4/5] chore(runtime): keep tracker ids out of the vocabulary row's runtime string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check:doc-authoring — a runtime string reaches authors and operators who cannot resolve a tracker id. The anchors stay in the adjacent // comment. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- packages/runtime/src/dispatcher-error-vocabulary.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index 016add74d3..e6cc03efe5 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -664,7 +664,7 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ door: 'dispatcher', verdict: 'pending-registration', why: - "The #14921 refusal: two files under one `ROOT/TYPE/` deriving one metadata name across this " + "The ambiguous-metadata-stem refusal: two files under one `ROOT/TYPE/` deriving one metadata name across this " + 'loader\'s REGISTERED extensions is an authoring error, refused with both paths and the type ' + 'named rather than resolved by the `.json` → `.yaml` → `.yml` → `.ts` → `.js` precedence ' + '(maintainer ruling, director seat, 2026-09-05, option 1 of three). ⭐ Reachability, which is ' @@ -678,8 +678,9 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ + 'population of trees that can raise it is ZERO (a walk of 7,770 tracked files over 526 ' + 'directories found no stem collision among the registered extensions), so no existing tree ' + 'reaches the door today; the row records the PRODUCER, which is what this table is for, and ' - + 'a producer nobody triggers yet is exactly the one that hides. Since #9106 the door narrows, ' - + 'so the body parses and `AMBIGUOUS_METADATA_STEM` rides `declaredCode` while `error.code` ' + + 'a producer nobody triggers yet is exactly the one that hides. The door narrows (see this ' + + "file's header), so the body parses and `AMBIGUOUS_METADATA_STEM` rides `declaredCode` " + + 'while `error.code` ' + 'takes the member the 500 derives — the demote this verdict names. ⛔ Registering it widens ' + '`ApiErrorSchema.code` and is the `packages/spec` lane\'s call, NOT made here: this row is ' + 'that batch\'s input, and the registration is what ratchets it out again.', From 2aa6373d1330a5c3321acbfd532eadb6ec026e97 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 00:03:56 +0000 Subject: [PATCH 5/5] test(metadata): the fake loader's contract protocol must be a declared member MetadataLoaderContract.protocol is a closed union; 'test:' is not in it. The outage CONTROL only needs a loader whose failure carries no ambiguous-stem brand, so it declares 'memory:'. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- .../src/loaders/filesystem-loader-ambiguous-stem.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts b/packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts index 5be499e9f0..b93ede0dd4 100644 --- a/packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts +++ b/packages/metadata/src/loaders/filesystem-loader-ambiguous-stem.test.ts @@ -325,7 +325,7 @@ describe('#14921 CONTROL — the refusal reads the REGISTERED extension set', () class OutageLoader implements MetadataLoader { readonly contract: MetadataLoaderContract = { name: 'outage', - protocol: 'test:', + protocol: 'memory:', capabilities: { read: true, write: false, watch: false, list: true }, supportedFormats: ['json'], supportsWatch: false,