diff --git a/.changeset/metadata-get-diagnosed-outage-vs-miss.md b/.changeset/metadata-get-diagnosed-outage-vs-miss.md new file mode 100644 index 0000000000..5b2e2c20b0 --- /dev/null +++ b/.changeset/metadata-get-diagnosed-outage-vs-miss.md @@ -0,0 +1,43 @@ +--- +'@objectstack/metadata': patch +'@objectstack/metadata-protocol': patch +'@objectstack/objectql': patch +'@objectstack/spec': patch +--- + +metadata: `getDiagnosed` — a metadata read that FAILED stops arriving as "nobody declared this" + +`MetadataManager.loadDiagnosed` computes the ADR-0110 D3 verdict (a MISS and an OUTAGE +are different facts with opposite security meanings) and `get()` discarded it two hops +later: `load()` kept only `.data`, `get()` turned that `null` into `undefined`. Every +consumer of `get()` therefore received one `undefined` for two opposite facts and could +not have told them apart even if it had wanted to. + +**New read.** `MetadataManager.getDiagnosed(type, name)` returns +`{ data, degraded, errors }` — the registry-first counterpart of `loadDiagnosed`, declared +as an optional member of `IMetadataService`. A registry hit is never degraded (it +consulted no loader); a clean miss is never degraded (every loader answered). + +**`get()` is unchanged — zero breaking.** Same signature, same answer, same behaviour for +every existing caller, including the microtask-level ordering `register()`'s watchers +depend on. Only callers that ASK for the verdict pay for it. Making `get()` throw on +`degraded` was deliberately not done: the boot path degrades on purpose. + +**Consumers switched**, each with a disposition argued for its own context rather than one +blanket rule: + +- `getMetaItem` / `getMetaItemCached` — a degraded MetadataService read with nothing in + the registry now raises `503 SERVICE_UNAVAILABLE` instead of falling through to + `404 RESOURCE_NOT_FOUND`. This is the half that made the existing `#5532` comment (" + reaching here now means a real miss") untrue. +- `getMetaItemLayered` — the `code` layer joins the rule its `overlay` layer already + followed. `code: null` is a positive claim, and `lockSource = code ?? overlay ?? {}` + derives from it, so an outage could render an item the packager locked + (`_lock: 'full'`) as `editable: true, deletable: true`. +- `ObjectQLPlugin`'s `object` metadata-event refresh — logs `warn` naming the consequence + (the registry keeps the previous definition; nothing retries) and the fix, instead of + `debug` "metadata service has no fresh body". `warn` and not `error` because the write + already landed; only a re-read failed. + +Hosts whose `metadata` slot is a shim that predates `getDiagnosed` are read as +"not degraded" — exactly what they could express before — so their behaviour is unchanged. diff --git a/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts b/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts index 5188c2f4b7..bb89ca52b1 100644 --- a/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts +++ b/packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts @@ -406,3 +406,197 @@ describe('[#5707] the layered read stops painting an outage as "nothing was cust function p_layered(engine: any, request: Record): Promise { return new ObjectStackProtocolImplementation(engine).getMetaItemLayered(request as any); } + +// --------------------------------------------------------------------------- +// [#5840] The OTHER read in these same two methods — the MetadataService one +// --------------------------------------------------------------------------- +// Everything above is about the `sys_metadata` overlay read, whose failure the +// protocol can see because it arrives as a throw. The second source each of +// these methods consults — the `metadata` SERVICE, i.e. the loader chain +// (filesystem, database, attached repository) — failed silently: a loader that +// throws is warn-logged and skipped inside `MetadataManager`, and its `get()` +// dropped the `degraded` verdict `loadDiagnosed` had already computed. So an +// unreachable metadata database arrived here as the ordinary `undefined` of a +// name nobody declared, and the SAME two methods that now refuse to guess on +// their overlay half went on guessing on this one: +// +// getMetaItem → falls through, item stays undefined → 404 "not found" +// getMetaItemLayered → `code: null` → `lockSource = code ?? overlay ?? {}` +// → `editable: true, deletable: true` on an item whose +// code layer may declare `_lock: 'full'` +// +// The second is the sharper one, and it is the shape ADR-0110 D3 names +// outright: an availability failure widening an affordance. `getDiagnosed` +// (#5840) is the seam that makes the failure visible at all; these cases pin +// what each method does with it. +// +// Reverse verification, direction predicted BEFORE running: ordinary red, and +// it must be taken on the CONSUMER, not the producer. These doubles feed the +// return contract directly, so reverting `MetadataManager.getDiagnosed` cannot +// turn them red — only deleting the two `if (… degraded)` branches in +// `protocol.ts` can. Two laps therefore prove two different halves, and +// neither substitutes for the other. +// +// Predicted for the consumer lap: 5 red / 3 green across the two describes +// below — every case that expects a 503, and only those, with all three +// narrowness/back-compat cases green (they assert the branch does NOT fire). +// Measured: exactly that, and the 18 #5532/#5707 cases above stayed green, +// which is what shows this is the third read joining the rule rather than a +// blanket "these methods now throw". + +/** A services registry holding one `metadata` service — what the protocol probes. */ +function servicesWith(metadata: unknown): () => Map { + const registry = new Map([['metadata', metadata]]); + return () => registry; +} + +const LOADER_FAILURE = 'database: connect ECONNREFUSED 10.0.0.5:5432'; + +/** + * A `metadata` service whose loader chain is DOWN. `get()` answers exactly what + * it answered before this issue — `undefined`, indistinguishable from a miss — + * and `getDiagnosed()` reports the verdict that was being computed and thrown + * away all along. + */ +const metadataServiceInOutage = () => ({ + get: vi.fn(async () => undefined), + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: true, errors: [LOADER_FAILURE] })), +}); + +/** A `metadata` service that answered, and simply does not hold the item. */ +const metadataServiceWithMiss = () => ({ + get: vi.fn(async () => undefined), + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: false, errors: [] })), +}); + +/** A `metadata` service that holds `body`. */ +const metadataServiceHolding = (body: unknown) => ({ + get: vi.fn(async () => body), + getDiagnosed: vi.fn(async () => ({ data: body, degraded: false, errors: [] })), +}); + +/** A service that predates #5840: `get` only, no way to report the difference. */ +const legacyMetadataService = (body?: unknown) => ({ get: vi.fn(async () => body) }); + +/** The outage envelope, for a cause built from loader messages rather than a driver error. */ +function expectLoaderOutage(caught: any) { + expect(caught?.status).toBe(503); + expect(caught?.code).toBe('SERVICE_UNAVAILABLE'); + expect(ErrorCode.safeParse(caught?.code).success).toBe(true); + expect(caught.message).toContain('unknown'); + expect(caught.message.toLowerCase()).not.toContain('not found'); + // The failing loaders' own words reach the operator on `cause`, which is + // what `logWithheldServerFault` prints (#5437) — the protocol never sees a + // driver error here, because `MetadataManager` already absorbed it. + expect(String((caught.cause as Error)?.message)).toContain(LOADER_FAILURE); +} + +/** A protocol whose overlay store is healthy and empty — only the SERVICE half varies. */ +function protocolWithService(metadata: unknown, registryItems: Record = {}) { + return new ObjectStackProtocolImplementation( + engineWithRows([], registryItems), + servicesWith(metadata), + ); +} + +describe('[#5840] a MetadataService outage stops arriving as "nobody declared this"', () => { + it('the singular read throws 503 instead of falling through to a 404', async () => { + const p = protocolWithService(metadataServiceInOutage()); + + const caught = await rejection(() => p.getMetaItem({ type: 'object', name: 'acct' } as any)); + expectLoaderOutage(caught); + }); + + it('getMetaItemCached stops relabelling that outage "Metadata item object/acct not found"', async () => { + const p = protocolWithService(metadataServiceInOutage()); + + const caught = await rejection(() => p.getMetaItemCached({ type: 'object', name: 'acct' } as any)); + expectLoaderOutage(caught); + // The comment above that 404 claims "reaching here now means a real + // miss". This is the half that used to make it untrue. + expect(caught.message).not.toContain('Metadata item object/acct not found'); + }); + + it('a miss and an outage are told apart by code alone — the whole point', async () => { + const missP = protocolWithService(metadataServiceWithMiss()); + const outageP = protocolWithService(metadataServiceInOutage()); + + const miss = await rejection(() => missP.getMetaItemCached({ type: 'object', name: 'ghost' } as any)); + const outage = await rejection(() => outageP.getMetaItemCached({ type: 'object', name: 'ghost' } as any)); + + expect([miss.status, miss.code]).toEqual([404, 'RESOURCE_NOT_FOUND']); + expect([outage.status, outage.code]).toEqual([503, 'SERVICE_UNAVAILABLE']); + }); + + it('the layered read refuses to publish a `code: null` it never verified', async () => { + const p = protocolWithService(metadataServiceInOutage()); + + const caught = await rejection( + () => p.getMetaItemLayered({ type: 'object', name: 'acct' } as any), + ); + expectLoaderOutage(caught); + }); + + it('and that is what stops an outage from unlocking a locked artifact', async () => { + // The concrete widening. `lockSource = code ?? overlay ?? {}`, so a + // code layer that never arrived resolves the protection envelope from + // `{}` — `editable: true, deletable: true` on an item the packager + // locked. Left column: what the truth looks like. Right column: what + // the outage used to render, and now cannot. + const locked = { name: 'acct', label: 'Account', _lock: 'full' }; + + const healthy: any = await protocolWithService( + metadataServiceHolding(locked), + ).getMetaItemLayered({ type: 'object', name: 'acct' } as any); + expect(healthy.lock).toBe('full'); + expect([healthy.editable, healthy.deletable]).toEqual([false, false]); + + const caught = await rejection( + () => protocolWithService(metadataServiceInOutage()) + .getMetaItemLayered({ type: 'object', name: 'acct' } as any), + ); + expectLoaderOutage(caught); + // Never the silently-permissive envelope. + expect(caught.editable).toBeUndefined(); + }); +}); + +describe('[#5840] the narrowness is the design — three things it deliberately does not do', () => { + it('a registry hit still answers, degraded service or not', async () => { + // A registry item IS a real declaration, so the answer contains no + // unfounded claim and is served exactly as before. The 503 fires only + // where the alternative would have been "this does not exist". + const p = protocolWithService(metadataServiceInOutage(), { + acct: { name: 'acct', label: 'Account (packaged)' }, + }); + + const res: any = await p.getMetaItem({ type: 'object', name: 'acct' } as any); + expect(res.item?.label).toBe('Account (packaged)'); + + const layered: any = await p.getMetaItemLayered({ type: 'object', name: 'acct' } as any); + expect(layered.code).toMatchObject({ label: 'Account (packaged)' }); + }); + + it('a clean MISS still renders the all-null layered envelope, not a 503', async () => { + const p = protocolWithService(metadataServiceWithMiss()); + + const res: any = await p.getMetaItemLayered({ type: 'object', name: 'ghost' } as any); + expect(res.code).toBeNull(); + expect(res.overlay).toBeNull(); + expect(res.effective).toBeNull(); + }); + + it('a service that predates `getDiagnosed` behaves exactly as it did', async () => { + // It cannot report the distinction, so it is read as "not degraded" — + // precisely what it could express before, unchanged. The alternative + // (treating an un-probeable service as suspect) would 503 every host + // whose `metadata` slot is a shim. + const holding = protocolWithService(legacyMetadataService({ name: 'acct', label: 'From shim' })); + const res: any = await holding.getMetaItem({ type: 'object', name: 'acct' } as any); + expect(res.item?.label).toBe('From shim'); + + const empty = protocolWithService(legacyMetadataService(undefined)); + const caught = await rejection(() => empty.getMetaItemCached({ type: 'object', name: 'ghost' } as any)); + expect([caught.status, caught.code]).toEqual([404, 'RESOURCE_NOT_FOUND']); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 36a923769c..d4387d41c3 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -3163,6 +3163,88 @@ export class ObjectStackProtocolImplementation implements throw metadataStoreUnavailableError(error); } + /** + * [#5840] Read ONE item from the `metadata` service, keeping the ADR-0110 + * D3 verdict instead of flattening it into `undefined`. + * + * The `sys_metadata` overlay reads in this file already refuse to answer an + * outage as an absence (#5532 / #5707) — they see a throw and rethrow it as + * a 503. The MetadataService reads could not do the same, and not because + * anyone decided they should not: `MetadataManager.get()` swallows a loader + * failure internally, so an unreachable metadata database arrived here as + * the very same `undefined` a name that was never declared produces. The + * verdict existed one layer down (`loadDiagnosed`) and was discarded two + * hops before this call site. `getDiagnosed` (#5840) hands it over. + * + * Deliberately does NOT throw: the two callers want the same fact and + * dispose of it differently — see each call site. A service that predates + * `getDiagnosed` reports nothing degraded, which is exactly what it could + * express before, so its behaviour is unchanged. + * + * The singular/plural retry is folded in because both callers do it, and + * `degraded` must be the verdict of the WHOLE lookup: a first read that + * failed is not made trustworthy by an alternate spelling that cleanly + * missed. + */ + private async readItemFromMetadataService( + type: string, + name: string, + packageId?: string, + ): Promise<{ data: unknown; degraded: boolean; errors: string[] }> { + const services = this.getServicesRegistry?.(); + const metadataService: any = services?.get('metadata'); + if (!metadataService || typeof metadataService.get !== 'function') { + return { data: undefined, degraded: false, errors: [] }; + } + // ADR-0048 — thread the caller's package id so a single-item fetch is + // package-scoped. Passed positionally to BOTH reads, so whatever the + // occupant of the `metadata` slot makes of a third argument today is + // unchanged by which of the two methods answers. + const read = async (t: string): Promise<{ data: unknown; degraded: boolean; errors: string[] }> => { + if (typeof metadataService.getDiagnosed === 'function') { + const diagnosed = await metadataService.getDiagnosed(t, name, packageId); + return { + data: diagnosed?.data, + degraded: diagnosed?.degraded === true, + errors: Array.isArray(diagnosed?.errors) ? diagnosed.errors : [], + }; + } + return { data: await metadataService.get(t, name, packageId), degraded: false, errors: [] }; + }; + + const primary = await read(type); + if (primary.data !== undefined && primary.data !== null) return primary; + const alt = PLURAL_TO_SINGULAR[type] ?? SINGULAR_TO_PLURAL[type]; + if (!alt) return { data: undefined, degraded: primary.degraded, errors: primary.errors }; + const secondary = await read(alt); + if (secondary.data !== undefined && secondary.data !== null) return secondary; + return { + data: undefined, + degraded: primary.degraded || secondary.degraded, + errors: [...primary.errors, ...secondary.errors], + }; + } + + /** + * [#5840] The MetadataService counterpart of + * {@link rethrowUnlessMetadataStoreUnprovisioned}: turn a degraded read + * into the same 503 the overlay half of these methods already throws. + * + * There is no driver error to carry here — `MetadataManager` warn-logs and + * skips each failing loader — so `cause` is built from the messages it + * collected, which is what reaches the operator through + * `handleRouteError` / `logWithheldServerFault`. + */ + private throwMetadataServiceUnavailable(errors: string[]): never { + throw metadataStoreUnavailableError( + new Error( + `The metadata service could not read every loader: ${ + errors.length > 0 ? errors.join('; ') : 'no loader detail reported' + }`, + ), + ); + } + async getMetaItems(request: { type: string; packageId?: string; organizationId?: string; previewDrafts?: boolean }) { // #4432 — CANONICAL TYPE KEY. See {@link canonicalMetaType}. This one // is load-bearing twice over: the SchemaRegistry indexes code-authored @@ -3625,26 +3707,24 @@ export class ObjectStackProtocolImplementation implements // running server). Without this ordering, edits to `*.view.ts` // source files appear to take effect (MetadataManager learns the // new value) but reads continue to return the stale registry copy. + // [#5840] `serviceDegraded` survives past the registry step below on + // purpose — see the branch that reads it after step 3 for why the + // verdict cannot be acted on here. + let serviceDegraded: { degraded: boolean; errors: string[] } | undefined; if (item === undefined) { try { - const services = this.getServicesRegistry?.(); - const metadataService = services?.get('metadata'); - if (metadataService && typeof metadataService.get === 'function') { - // Thread the caller's package id (ADR-0048) so a single-item - // fetch is package-scoped: when two installed packages ship the - // same type/name, the facade prefers the requester's own item. - const fromService = await metadataService.get(request.type, request.name, request.packageId); - if (fromService !== undefined && fromService !== null) { - item = fromService; - } else { - const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) { - const altFromService = await metadataService.get(alt, request.name, request.packageId); - if (altFromService !== undefined && altFromService !== null) { - item = altFromService; - } - } - } + // Threads the caller's package id (ADR-0048) so a single-item + // fetch is package-scoped: when two installed packages ship the + // same type/name, the facade prefers the requester's own item. + const fromService = await this.readItemFromMetadataService( + request.type, + request.name, + request.packageId, + ); + if (fromService.data !== undefined && fromService.data !== null) { + item = fromService.data; + } else if (fromService.degraded) { + serviceDegraded = fromService; } } catch { // MetadataService not available — fall through @@ -3671,6 +3751,26 @@ export class ObjectStackProtocolImplementation implements } } + // [#5840] The MetadataService half of the #5532 rule, and the last + // moment it can be applied. The cached-read wrapper below this method + // documents its 404 as "reaching here now means a real miss — + // `getMetaItem` throws 503 rather than answering `undefined` when the + // store could not be read". That was true of the overlay read only: a + // metadata database the LOADERS could not reach was warn-logged inside + // `MetadataManager` and arrived at step 2 as a plain `undefined`, so + // the outage was served as `404 RESOURCE_NOT_FOUND` — a claim about + // what the author declared, made from a read that never happened. + // + // Deliberately narrow, and deliberately after step 3: a registry hit is + // a real declaration, so the answer contains no false claim and is + // served exactly as before (possibly staler than the service copy — the + // pre-existing ordering trade-off, not this issue's). Only when the + // WHOLE chain resolved nothing does the degraded read change anything, + // because only then would this method answer "no such item". + if (item === undefined && serviceDegraded?.degraded) { + this.throwMetadataServiceUnavailable(serviceDegraded.errors); + } + // Merge registered navigation contributions into a served app // (ADR-0029 D7) — parity with the getMetaItems list path so a // single-app fetch (GET /meta/app/) also sees the contributed @@ -3768,11 +3868,21 @@ export class ObjectStackProtocolImplementation implements * that would have to stand in for it already means "not customised". So * an overlay read that failed is reported as a failure, never as a layer. * + * [#5840] That rule now holds on BOTH halves. It could not before: the + * code layer's failure is a loader `MetadataManager` warn-logs and skips, + * so it reached this method as an ordinary `undefined` and became + * `code: null` — the same unfounded assertion, one column to the left, and + * the one the lock/affordance flags are derived from. + * * @throws {@link metadataStoreUnavailableError} — 503 / - * `SERVICE_UNAVAILABLE`, driver error on `cause`, when the - * `sys_metadata` overlay read fails for any reason other than the - * table not being provisioned yet (which genuinely means "no - * overlay row" and still returns normally). + * `SERVICE_UNAVAILABLE` when a read that would decide a layer did + * not happen: the `sys_metadata` overlay read failing for any + * reason other than the table not being provisioned yet (which + * genuinely means "no overlay row" and still returns normally), + * carrying the driver error on `cause`; or (#5840) the code + * layer's MetadataService read reporting `degraded` with nothing + * in the registry to answer instead, carrying the failing loaders' + * messages on `cause`. */ async getMetaItemLayered(request: { type: string; @@ -3815,18 +3925,21 @@ export class ObjectStackProtocolImplementation implements request = canonicalizeMetaRequestType(request); // ── code layer: MetadataService.get + registry, BYPASSING overlay ── let code: unknown | null = null; + let codeDegraded: { degraded: boolean; errors: string[] } | undefined; try { - const services = this.getServicesRegistry?.(); - const metadataService = services?.get('metadata'); - if (metadataService && typeof metadataService.get === 'function') { - // ADR-0048 — package-scope the code layer so a same-name - // collision resolves to the requested package's artifact. - let fromService = await metadataService.get(request.type, request.name, request.packageId); - if (fromService === undefined || fromService === null) { - const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) fromService = await metadataService.get(alt, request.name, request.packageId); - } - if (fromService !== undefined && fromService !== null) code = fromService; + // ADR-0048 — package-scope the code layer so a same-name + // collision resolves to the requested package's artifact. + const fromService = await this.readItemFromMetadataService( + request.type, + request.name, + request.packageId, + ); + if (fromService.data !== undefined && fromService.data !== null) { + code = fromService.data; + } else if (fromService.degraded) { + // [#5840] Kept, not swallowed — acted on after the registry + // fallback below, which may still produce a real code layer. + codeDegraded = fromService; } } catch { // ignore @@ -3844,6 +3957,26 @@ export class ObjectStackProtocolImplementation implements if (regItem !== undefined) code = regItem; } + // [#5840] The code half of the rule #5707 wrote for the overlay half, + // eleven lines below. `code: null` is not a shrug — this method states + // it positively ("no packaged/code-layer definition exists"), and the + // response then DERIVES from it: `lockSource = code ?? overlay ?? {}` + // feeds `resolveLockState`, so an item whose code layer declares + // `_lock: 'full'` is rendered `editable: true, deletable: true` when + // the read that would have found that lock simply failed. An + // availability failure widening an affordance is precisely what + // ADR-0110 D3 forbids, and the overlay half of this very method + // already refuses to do it — the two halves were asymmetric only + // because the loader failure was invisible on this side. + // + // Same narrow shape as the overlay half: the benign "nothing there" + // still returns `code: null` normally (a clean miss is not degraded), + // and a registry hit above is a real code layer, so this fires only + // when the null would otherwise be an unfounded authorship claim. + if (code === null && codeDegraded?.degraded) { + this.throwMetadataServiceUnavailable(codeDegraded.errors); + } + // ── overlay layer: sys_metadata row (org-scoped wins, then env-wide) ── let overlay: unknown | null = null; let overlayScope: 'org' | 'env' | null = null; @@ -5910,6 +6043,15 @@ export class ObjectStackProtocolImplementation implements // not be read (see // {@link rethrowUnlessMetadataStoreUnprovisioned}) — so the // 404 is a claim this layer is finally entitled to make. + // + // [#5840] That entitlement was only three-quarters earned when + // it was written: it held for the `sys_metadata` overlay read, + // whose failure arrives as a throw, and NOT for the + // MetadataService read, whose failure `MetadataManager` + // warn-logs and skips — so a loader outage still reached this + // line as a plain missing `item` and was answered 404. Both + // halves now refuse to guess (see + // {@link readItemFromMetadataService}). throw metadataItemNotFoundError(request.type, request.name); } @@ -7346,6 +7488,15 @@ export class ObjectStackProtocolImplementation implements const services = this.getServicesRegistry?.(); const metadataService = services?.get('metadata'); if (metadataService && typeof metadataService.get === 'function') { + // [#5840] Measured and deliberately left on plain `get`. This + // read decides nothing and asserts nothing: it returns void, + // its `undefined` produces no answer to any caller, and the + // method's own contract above is "best-effort, the next full + // reload fixes the registry anyway". Routing it through + // `getDiagnosed` could only add a log line to a path that is + // already documented as silent — over-applying the rule, which + // is how `error`/`warn` become unreadable (AGENTS.md + // "Degradation log levels", the do-not-over-apply half). const artifactItem = await metadataService.get(type, name); if (artifactItem !== undefined) { this.engine.registry.registerItem(type, artifactItem, 'name'); diff --git a/packages/metadata/src/metadata-manager-get-diagnosed.test.ts b/packages/metadata/src/metadata-manager-get-diagnosed.test.ts new file mode 100644 index 0000000000..9cacb84aa6 --- /dev/null +++ b/packages/metadata/src/metadata-manager-get-diagnosed.test.ts @@ -0,0 +1,228 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5840 — `MetadataManager.get()` discarded the `degraded` verdict + * `loadDiagnosed` had already computed, two hops before any consumer could see + * it. + * + * --------------------------------------------------------------------------- + * The defect + * --------------------------------------------------------------------------- + * `loadDiagnosed` exists for ADR-0110 D3: a MISS and an OUTAGE are different + * facts with opposite security meanings, and its own TSDoc says so — "callers + * that gate on a declaration MUST NOT read that `null` as 'the author declared + * no gate'". But the chain flattened it immediately: + * + * load() = (await loadDiagnosed(...)).data // keeps `.data` only + * get() = (await load(...)) ?? undefined // and `null` → `undefined` + * + * So every consumer of `get()` — six of them at the time this was filed, in + * `metadata-protocol`, `objectql`, `plugin-security` and `mcp` — received one + * `undefined` for two opposite facts and could not have told them apart even + * if it had wanted to: the verdict was computed, then dropped, inside the + * method it would have had to ask. + * + * --------------------------------------------------------------------------- + * What these tests pin + * --------------------------------------------------------------------------- + * 1. The two facts are now SEPARATELY observable, and separable ONLY by + * `degraded` — `data` is `undefined` in both, so no downstream consumer can + * reconstruct the distinction from the payload and grow a second, weaker + * way of asking. + * 2. A registry hit is never degraded: it consulted no loader, so there is + * nothing to be unsure about — this is what makes `getDiagnosed` the + * counterpart of `get` rather than of `loadDiagnosed`, and it is why the + * consumers could not simply switch to `loadDiagnosed` (that one skips the + * in-memory registry and would resolve different items). + * 3. `get()` itself is byte-identical in behaviour — same signature, same + * answer for every one of these cases. Callers pay nothing; only callers + * that ASK for the verdict get it. + * + * --------------------------------------------------------------------------- + * Reverse verification, direction predicted BEFORE running + * --------------------------------------------------------------------------- + * Ordinary red, and deliberately PARTIAL. The lap is `getDiagnosed` reading + * through the discarding chain again — `const data = await this.load(type, + * name); return { data: data ?? undefined, degraded: false, errors: [] }`, + * which is exactly what a consumer could observe before this issue. + * + * Predicted 4 red / 5 green; measured 4 red / 5 green, the same four: + * "a loader that THREW", "separable ONLY by `degraded`", "a registry hit is + * never degraded" (its closing assertion reads a NON-registry name back), and + * "errors from several failing loaders". + * + * The five that stay green are the point of the split. Every assertion about + * a MISS survives the revert, because a miss was never the broken half — read + * as a whole, the pair proves the new verdict changed the outage answer and + * left the miss answer alone, which is the ADR-0110 D3 shape. The `get()` + * parity describe stays green for the same reason: `get()` is untouched. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { MetadataLoadOptions, MetadataLoadResult } from '@objectstack/spec/system'; +import { MetadataManager } from './metadata-manager.js'; +import type { MetadataLoader } from './loaders/loader-interface.js'; + +vi.mock('@objectstack/core', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }), +})); + +/** An outage: the row may well be there and simply was not seen. */ +const connectionRefused = () => + Object.assign(new Error('connect ECONNREFUSED 10.0.0.5:5432'), { code: 'ECONNREFUSED' }); + +/** + * A read-only loader whose `load()` either throws (outage) or answers `null` + * (clean miss) or answers a body. Deliberately minimal: `getDiagnosed` walks + * `load()` and nothing else. + */ +function loader( + name: string, + load: (type: string, itemName: string) => Promise, +): MetadataLoader { + return { + contract: { + name, + protocol: `${name}:`, + capabilities: { read: true, write: false, watch: false, list: true }, + }, + load: async (type: string, itemName: string, _options?: MetadataLoadOptions) => + load(type, itemName), + loadMany: async () => [], + list: async () => [], + exists: async () => false, + stat: async () => null, + } as unknown as MetadataLoader; +} + +/** A loader that cannot read its store — #5108's posture, one layer down. */ +const brokenLoader = (name = 'broken') => + loader(name, async () => { throw connectionRefused(); }); + +/** A loader that answers, and simply does not hold the item. */ +const emptyLoader = (name = 'empty') => + loader(name, async () => ({ data: null })); + +/** A loader that holds `body` under every name it is asked for. */ +const holdingLoader = (body: unknown, name = 'holding') => + loader(name, async () => ({ data: body, source: 'memory', format: 'json', loadTime: 0 })); + +function managerWith(...loaders: MetadataLoader[]): MetadataManager { + const mgr = new MetadataManager({}); + for (const l of loaders) mgr.registerLoader(l); + return mgr; +} + +describe('[#5840] getDiagnosed — an outage and a miss stop being the same answer', () => { + it('a loader that THREW is reported degraded, with the failure text', async () => { + const mgr = managerWith(brokenLoader()); + + const read = await mgr.getDiagnosed('permission', 'admin_all'); + + expect(read.degraded).toBe(true); + expect(read.data).toBeUndefined(); + expect(read.errors).toHaveLength(1); + expect(read.errors[0]).toContain('broken'); + expect(read.errors[0]).toContain('ECONNREFUSED'); + }); + + it('a name nobody declared is NOT degraded — a clean miss is a fact', async () => { + const mgr = managerWith(emptyLoader()); + + const read = await mgr.getDiagnosed('permission', 'never_declared'); + + expect(read.degraded).toBe(false); + expect(read.data).toBeUndefined(); + expect(read.errors).toEqual([]); + }); + + it('the two are separable ONLY by `degraded` — `data` cannot rebuild it', async () => { + const outage = await managerWith(brokenLoader()).getDiagnosed('permission', 'admin_all'); + const miss = await managerWith(emptyLoader()).getDiagnosed('permission', 'admin_all'); + + // The payload halves are indistinguishable, deliberately: if `data` + // could carry the difference, a consumer would soon read the weaker + // signal instead of the verdict. + expect(outage.data).toBe(miss.data); + expect(outage.data).toBeUndefined(); + + // The verdict is the whole difference. + expect([outage.degraded, miss.degraded]).toEqual([true, false]); + }); + + it('a healthy loader answering AFTER a broken one is not degraded', async () => { + // `degraded` means "nothing answered AND something failed" — a store + // that produced the item is a complete answer no matter what happened + // beside it. Same posture as `loadDiagnosed`, inherited not re-decided. + const mgr = managerWith(brokenLoader(), holdingLoader({ name: 'admin_all' })); + + const read = await mgr.getDiagnosed('permission', 'admin_all'); + + expect(read.data).toEqual({ name: 'admin_all' }); + expect(read.degraded).toBe(false); + }); + + it('an in-memory registry hit is never degraded — it consulted no loader', async () => { + // This is the half `loadDiagnosed` cannot express, and the reason the + // consumers of `get()` needed their OWN diagnosed read: switching them + // to `loadDiagnosed` would have skipped this registry entirely and + // changed which items they resolve. + const mgr = managerWith(brokenLoader()); + mgr.registerInMemory('view', 'accounts_grid', { name: 'accounts_grid' }); + + const read = await mgr.getDiagnosed('view', 'accounts_grid'); + + expect(read.data).toEqual({ name: 'accounts_grid' }); + expect(read.degraded).toBe(false); + expect(read.errors).toEqual([]); + + // And the loader really is broken for anything the registry lacks. + expect((await mgr.getDiagnosed('view', 'other')).degraded).toBe(true); + }); + + it('errors from several failing loaders are all reported', async () => { + const mgr = managerWith(brokenLoader('db'), brokenLoader('remote')); + + const read = await mgr.getDiagnosed('permission', 'admin_all'); + + expect(read.degraded).toBe(true); + expect(read.errors).toHaveLength(2); + expect(read.errors.join(' ')).toContain('db'); + expect(read.errors.join(' ')).toContain('remote'); + }); +}); + +describe('[#5840] `get()` is unchanged — zero breaking, by construction', () => { + it('answers exactly `getDiagnosed().data` in all four cases', async () => { + const cases: Array<[string, MetadataManager, string, unknown]> = [ + ['outage', managerWith(brokenLoader()), 'admin_all', undefined], + ['miss', managerWith(emptyLoader()), 'admin_all', undefined], + ['hit', managerWith(holdingLoader({ name: 'admin_all' })), 'admin_all', { name: 'admin_all' }], + ['no loaders at all', managerWith(), 'admin_all', undefined], + ]; + + for (const [label, mgr, name, expected] of cases) { + const viaGet = await mgr.get('permission', name); + const viaDiagnosed = await mgr.getDiagnosed('permission', name); + expect(viaGet, label).toEqual(expected); + expect(viaGet, label).toEqual(viaDiagnosed.data); + } + }); + + it('still prefers the in-memory registry over every loader', async () => { + const mgr = managerWith(holdingLoader({ name: 'from_loader' })); + mgr.registerInMemory('view', 'accounts_grid', { name: 'from_registry' }); + + expect(await mgr.get('view', 'accounts_grid')).toEqual({ name: 'from_registry' }); + }); + + it('an outage still resolves rather than throwing — the contract callers rely on', async () => { + // Direction (b) of the issue — making `get()` throw on `degraded` — was + // deliberately NOT taken: it changes a public contract and the boot + // path degrades on purpose (see the TSDoc on `restoreMetadataFromDb`, + // #5897). The diagnosis is offered, never imposed. + const mgr = managerWith(brokenLoader()); + + await expect(mgr.get('permission', 'admin_all')).resolves.toBeUndefined(); + }); +}); diff --git a/packages/metadata/src/metadata-manager.ts b/packages/metadata/src/metadata-manager.ts index 7e70e8b2f7..40dd19a731 100644 --- a/packages/metadata/src/metadata-manager.ts +++ b/packages/metadata/src/metadata-manager.ts @@ -780,6 +780,23 @@ export class MetadataManager implements IMetadataService { /** * Get a metadata item by type and name. * Checks in-memory registry first, then falls back to loaders. + * + * Returns `undefined` both when nothing declares the item and when every + * loader that could have held it FAILED — see {@link getDiagnosed} when the + * caller must tell those apart. This is the same relationship {@link load} + * has with {@link loadDiagnosed}, so every existing caller keeps its exact + * behaviour and only callers that ASK for the verdict pay for it. + * + * [#5840] Deliberately NOT expressed as `(await getDiagnosed(…)).data`, + * although that is what it computes. The obvious delegation adds one + * `await` hop, and a registry hit here is observed one microtask sooner than + * it would be through a second async frame — which `register()`'s watchers + * depend on, because `notifyWatchers` does not await its handlers and + * ObjectQL's bridge re-reads through `get()` on the event rather than + * trusting the payload (`register-notifies-watchers.test.ts` pins it, and + * went red on the delegating version). The duplication is three lines and is + * pinned from the other side: `get()` and `getDiagnosed().data` are asserted + * to agree on every case in `metadata-manager-get-diagnosed.test.ts`. */ async get(type: string, name: string): Promise { // Check in-memory registry first @@ -793,6 +810,45 @@ export class MetadataManager implements IMetadataService { return result ?? undefined; } + /** + * `get`, plus whether the answer can be trusted as complete. + * + * [#5840] {@link loadDiagnosed} already computes this verdict — and `get()` + * threw it away two hops later (`load` kept only `.data`, `get` turned that + * `null` into `undefined`), so no caller of `get` could reach the one fact + * ADR-0110 D3 exists to preserve: **a miss and an outage are different facts + * with opposite security meanings.** A consumer that gates on a declaration + * MUST NOT read `undefined` as "the author declared nothing" — an + * availability failure would silently widen access (the REST `/actions` + * fail-open branch, #3935) or make a positive claim about authorship from a + * read that never happened (`code: null` in the layered read, #5707/#5532). + * + * This is the registry-first counterpart of {@link loadDiagnosed}, and that + * difference is why callers of `get` cannot simply switch to `loadDiagnosed`: + * doing so would skip the in-memory registry and change what they resolve. + * + * `degraded` is true when at least one loader threw AND nothing answered with + * the item — never when the in-memory registry answered, because that answer + * needed no loader. A clean miss (every loader answered, none had it) is NOT + * degraded. The posture is deliberately conservative: with a loader down we + * cannot prove the item is absent, so we decline to claim it is. + */ + async getDiagnosed( + type: string, + name: string + ): Promise<{ data: unknown | undefined; degraded: boolean; errors: string[] }> { + // Check in-memory registry first — a hit here consulted no loader, so + // there is nothing to be degraded about. + const typeStore = this.registry.get(type); + if (typeStore?.has(name)) { + return { data: typeStore.get(name), degraded: false, errors: [] }; + } + + // Fallback to loaders, keeping the verdict this time. + const { data, degraded, errors } = await this.loadDiagnosed(type, name); + return { data: data ?? undefined, degraded, errors }; + } + /** * List all metadata items of a given type. * diff --git a/packages/objectql/src/plugin-metadata-event-outage.test.ts b/packages/objectql/src/plugin-metadata-event-outage.test.ts new file mode 100644 index 0000000000..fd04634686 --- /dev/null +++ b/packages/objectql/src/plugin-metadata-event-outage.test.ts @@ -0,0 +1,220 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5840, ADR-0110 D3 — event side] An `object` metadata event whose re-read + * FAILED is not an object that no longer has a body. + * + * --------------------------------------------------------------------------- + * The defect + * --------------------------------------------------------------------------- + * `subscribeToMetadataEvents` re-reads the changed object through the metadata + * service ("the loader chain (FS, DB, attached repository)", per its own + * comment) and re-registers it. The read went through `MetadataManager.get()`, + * which answered an unreachable loader chain with the same `undefined` a + * deleted object produces — `loadDiagnosed` computed the `degraded` verdict and + * `load()`/`get()` dropped it two hops before this call site (#5840). + * + * So the `else` branch logged, at `debug`, that the "metadata service has no + * fresh body" — an assertion about what is declared, made from a read that + * never happened — and the registry silently kept the PREVIOUS definition. + * + * --------------------------------------------------------------------------- + * Why `warn` here and `error` in `restoreMetadataFromDb` (#5897) + * --------------------------------------------------------------------------- + * The sibling file next to this one pins an `error` for the boot-side outage, + * and copying that by analogy would be the mirror-image mistake AGENTS.md + * "Degradation log levels" warns about. The level is decided by that section's + * own question, asked honestly: does something this code CLAIMS IS PERSISTED + * fail to land while the system looks normal? No. The write already landed in + * the metadata store — the event is what announces it. What failed is a + * re-READ, and the registry keeps serving the definition it already holds, so + * this kernel's copy is behind rather than lost. Functional degradation → + * `warn`. It would also fire once per event during an outage, where the boot + * line fires once per process. + * + * What the level does NOT excuse is silence about the cost: the line owes the + * consequence (stale schema served, nothing retries) and the fix, neither of + * which the `debug` line it replaces carried. + * + * --------------------------------------------------------------------------- + * Reverse verification, direction predicted BEFORE running + * --------------------------------------------------------------------------- + * Ordinary red, taken on the CONSUMER: this file feeds the service double's + * return contract directly, so reverting `MetadataManager.getDiagnosed` cannot + * move it — only restoring the plain `metadataService.get(...)` read (and with + * it the single `else`) can. Predicted: the three outage cases go red, on the + * assertion that the `debug` "no fresh body" line did NOT fire, and the two + * unaffected cases (a real body, a genuine miss) stay green — they never + * depended on the verdict. + * + * The service double declares `subscribe` / `get` / `getDiagnosed` and no + * engine write verb, so there is no `delete`/`update` dispatch for + * `check:engine-double-contract` to scan and no guard to hand-mirror. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectQLPlugin } from './plugin.js'; +import type { ObjectQL } from './engine.js'; + +type AnyRecord = Record; + +const LOADER_FAILURE = 'database: connect ECONNREFUSED 10.0.0.5:5432'; + +function makeCtx() { + const logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + return { logger, getService: vi.fn(), hook: vi.fn() } as AnyRecord; +} + +/** A registry that records what was re-registered, so "kept the old body" is observable. */ +function makeRegistry() { + return { + invalidate: vi.fn(), + registerObject: vi.fn(), + }; +} + +function makePlugin(registry: AnyRecord) { + return new ObjectQLPlugin({ ql: { registry } as unknown as ObjectQL }); +} + +/** + * Wire the subscription and hand back the handler the plugin registered — the + * seam under test is what that handler does with the read's verdict. + */ +function subscribeAndCapture(metadataService: AnyRecord, plugin: AnyRecord, ctx: AnyRecord) { + let handler: ((evt: AnyRecord) => Promise) | undefined; + metadataService.subscribe = vi.fn((_type: string, h: any) => { + handler = h; + return () => {}; + }); + plugin.subscribeToMetadataEvents(metadataService, ctx); + if (!handler) throw new Error('plugin did not register a handler'); + return handler; +} + +/** A metadata service whose loader chain is DOWN. */ +const serviceInOutage = () => ({ + get: vi.fn(async () => undefined), + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: true, errors: [LOADER_FAILURE] })), +}); + +/** A metadata service that answered, and the object genuinely has no body. */ +const serviceWithMiss = () => ({ + get: vi.fn(async () => undefined), + getDiagnosed: vi.fn(async () => ({ data: undefined, degraded: false, errors: [] })), +}); + +/** A healthy service holding `body`. */ +const serviceHolding = (body: unknown) => ({ + get: vi.fn(async () => body), + getDiagnosed: vi.fn(async () => ({ data: body, degraded: false, errors: [] })), +}); + +/** A service that predates `getDiagnosed`. */ +const legacyService = (body?: unknown) => ({ get: vi.fn(async () => body) }); + +const linesAt = (ctx: AnyRecord, level: 'debug' | 'info' | 'warn' | 'error'): string[] => + ctx.logger[level].mock.calls.map((c: unknown[]) => String(c[0])); + +describe('ObjectQLPlugin metadata events — an unreadable loader chain is not "no body" (#5840)', () => { + it('stops claiming the service "has no fresh body" when the read never happened', async () => { + const ctx = makeCtx(); + const registry = makeRegistry(); + const handler = subscribeAndCapture(serviceInOutage(), makePlugin(registry) as any, ctx); + + await handler({ type: 'changed', name: 'acct' }); + + // The pre-fix line, verbatim — the assertion that fails first if the + // branch is ever dropped. + expect(linesAt(ctx, 'debug').join('\n')).not.toMatch(/has no fresh body/); + expect(linesAt(ctx, 'warn')).toHaveLength(1); + }); + + it('the warn line owes the consequence and the fix, like any degradation line', async () => { + const ctx = makeCtx(); + const handler = subscribeAndCapture(serviceInOutage(), makePlugin(makeRegistry()) as any, ctx); + + await handler({ type: 'changed', name: 'acct' }); + + const line = linesAt(ctx, 'warn')[0]; + // What was lost: the registry is behind, and nothing retries on its own. + expect(line).toMatch(/PREVIOUS definition/); + expect(line).toMatch(/stale/); + expect(line).toMatch(/nothing retries/); + // …and the remedy, actionable without reading the source. + expect(line).toMatch(/connection|credentials|loaders/); + // The failing loaders' own words ride in the structured meta slot. + expect(ctx.logger.warn.mock.calls[0][1]).toMatchObject({ + name: 'acct', + errors: [LOADER_FAILURE], + }); + }); + + it('is a WARN, not an error — the write already landed; only a re-read failed', async () => { + // Deliberately asserted, not assumed: `restoreMetadataFromDb` (#5897) logs + // `error` for its outage, and the difference between the two is the + // AGENTS.md judgment question, not the subsystem. + const ctx = makeCtx(); + const handler = subscribeAndCapture(serviceInOutage(), makePlugin(makeRegistry()) as any, ctx); + + await handler({ type: 'changed', name: 'acct' }); + + expect(linesAt(ctx, 'error')).toEqual([]); + expect(linesAt(ctx, 'warn')).toHaveLength(1); + }); + + it('a genuine miss still takes the debug branch — the benign case is untouched', async () => { + const ctx = makeCtx(); + const handler = subscribeAndCapture(serviceWithMiss(), makePlugin(makeRegistry()) as any, ctx); + + await handler({ type: 'changed', name: 'acct' }); + + expect(linesAt(ctx, 'debug').join('\n')).toMatch(/has no fresh body/); + expect(linesAt(ctx, 'warn')).toEqual([]); + }); + + it('a healthy read still re-registers the fresh body, unchanged', async () => { + const ctx = makeCtx(); + const registry = makeRegistry(); + const handler = subscribeAndCapture( + serviceHolding({ name: 'acct', label: 'Account', _packageId: 'crm' }), + makePlugin(registry) as any, + ctx, + ); + + await handler({ type: 'changed', name: 'acct' }); + + expect(registry.invalidate).toHaveBeenCalledWith('acct'); + expect(registry.registerObject).toHaveBeenCalledWith( + expect.objectContaining({ name: 'acct' }), + 'crm', + undefined, + 'own', + ); + expect(linesAt(ctx, 'warn')).toEqual([]); + }); + + it('a service that predates `getDiagnosed` behaves exactly as it did', async () => { + // It cannot report the distinction, so it is read as "not degraded" — + // precisely what it could express before. Both of its outcomes are pinned. + const held = makeCtx(); + const registry = makeRegistry(); + const heldHandler = subscribeAndCapture( + legacyService({ name: 'acct' }), + makePlugin(registry) as any, + held, + ); + await heldHandler({ type: 'changed', name: 'acct' }); + expect(registry.registerObject).toHaveBeenCalledTimes(1); + + const empty = makeCtx(); + const emptyHandler = subscribeAndCapture( + legacyService(undefined), + makePlugin(makeRegistry()) as any, + empty, + ); + await emptyHandler({ type: 'changed', name: 'acct' }); + expect(linesAt(empty, 'debug').join('\n')).toMatch(/has no fresh body/); + expect(linesAt(empty, 'warn')).toEqual([]); + }); +}); diff --git a/packages/objectql/src/plugin.ts b/packages/objectql/src/plugin.ts index 4e7d214318..a332d82f6b 100644 --- a/packages/objectql/src/plugin.ts +++ b/packages/objectql/src/plugin.ts @@ -708,9 +708,21 @@ export class ObjectQLPlugin implements Plugin { // Re-fetch the canonical definition from the metadata service. // The metadata service goes through its loader chain (FS, DB, // attached repository), so this picks up edits from any source. - const fresh = typeof metadataService.get === 'function' - ? await metadataService.get('object', name) - : undefined; + // + // [#5840, ADR-0110 D3] Through `getDiagnosed` when the service offers + // it: the loader chain named above is exactly what can be DOWN, and + // `get()` reported an unreachable metadata database as the same + // `undefined` a deleted object produces. The `else` branch below then + // said, in the log, that the service "has no fresh body" — an + // assertion about what is declared, made from a read that never + // happened. A service that predates `getDiagnosed` reports nothing + // degraded, which is precisely what it could express before. + const read = typeof metadataService.getDiagnosed === 'function' + ? await metadataService.getDiagnosed('object', name) + : typeof metadataService.get === 'function' + ? { data: await metadataService.get('object', name), degraded: false, errors: [] } + : { data: undefined, degraded: false, errors: [] }; + const fresh = read?.data; if (fresh && typeof fresh === 'object') { // Re-register with the original contributor metadata. We use // 'metadata-service' as packageId to match how the initial @@ -727,7 +739,31 @@ export class ObjectQLPlugin implements Plugin { name, packageId, }); + } else if (read?.degraded) { + // #5840 — `warn`, not `error`, and the choice is made with the + // AGENTS.md "Degradation log levels" question rather than by + // analogy to `restoreMetadataFromDb`'s `error` below. Ask it + // honestly: does something this code CLAIMS IS PERSISTED fail to + // land, while the system looks normal? No — the write already + // landed in the metadata store; what failed is a re-READ, and the + // registry keeps serving the definition it already holds. That is a + // functional degradation (this kernel's copy is behind), not a + // durability one. Escalating it would be the mirror-image failure + // that rule warns about, and it would fire once per event during an + // outage rather than once per boot. + // + // What it still owes the reader is the consequence and the fix, + // which the `debug` line it replaces gave neither of. + ctx.logger.warn( + '[ObjectQLPlugin] object metadata changed but the metadata service could not be read — ' + + 'the registry keeps the PREVIOUS definition for this object and nothing retries: reads serve the stale ' + + 'schema until a later event for it succeeds or the process restarts. ' + + 'Fix: check the loaders behind the metadata service (datasource connection, credentials, table).', + { name, errors: read.errors }, + ); } else { + // A read that HAPPENED and found nothing — the object really is gone + // from every loader (deleted between the event and this re-read). ctx.logger.debug('[ObjectQLPlugin] object event received but metadata service has no fresh body', { name }); } } catch (e: any) { diff --git a/packages/spec/src/contracts/metadata-service.ts b/packages/spec/src/contracts/metadata-service.ts index ec79cf28ea..a1980a6ec2 100644 --- a/packages/spec/src/contracts/metadata-service.ts +++ b/packages/spec/src/contracts/metadata-service.ts @@ -254,12 +254,43 @@ export interface IMetadataService { /** * Get a metadata item by type and name + * + * `undefined` is AMBIGUOUS by construction — it means "not found" *and* + * "every loader that could hold it failed". Prefer {@link getDiagnosed} + * wherever the difference could change a decision (#5840). + * * @param type - Metadata type * @param name - Item name/identifier * @returns The metadata definition, or undefined if not found */ get(type: string, name: string): Promise; + /** + * Get a metadata item, and say whether the answer can be trusted as + * complete. Same distinction {@link loadDiagnosed} draws, on the read that + * consults the in-memory registry first (ADR-0110 D3). + * + * [#5840] Declared because the verdict was already being computed and then + * discarded: `MetadataManager` defines `get` as + * `(await getDiagnosed(…)).data`, so a MISS and an OUTAGE arrive at every + * consumer as the same `undefined`. A caller deciding whether something is + * ABSENT must check `degraded` — treating a degraded read as an absence is + * how an outage becomes an authorization answer, or a positive claim about + * what an author declared. + * + * Callers of `get` cannot substitute `loadDiagnosed`: that one walks only + * the loaders, so it would skip the in-memory registry and resolve + * different items. + * + * Optional: implementations that predate it simply cannot report the + * distinction, and a consumer that probes for it must keep reading `get` + * when it is absent. + */ + getDiagnosed?( + type: string, + name: string, + ): Promise<{ data: unknown | undefined; degraded: boolean; errors: string[] }>; + /** * List all metadata items of a given type * @param type - Metadata type