fix(metadata-protocol): auditMetaItem propagates a failed audit read instead of reporting an empty trail

.changeset/tidy-pandas-repeat.md

@@ -0,0 +1,23 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+`auditMetaItem` no longer reports a failed audit read as an empty audit trail
+
+The `catch` closing the audit read in `ObjectStackProtocolImplementation.auditMetaItem`
+was unqualified. Its comment named two benign causes — the `sys_metadata_audit` table not
+being provisioned (legacy environments) and a host engine that exposes no `find` — but the
+clause took every other cause with them: a connection drop, a permission denial, a
+timeout, a malformed row, a query bug. Each was reported to the caller as the well-formed
+statement `{ events: [] }`, i.e. "this item has no audit entries".
+
+This is the compliance surface behind `GET /api/v1/meta/:type/:name/audit`, which exists
+so Studio's audit-log tab can show who tried what and whether a lock blocked it, so an
+empty answer reads as *nobody touched this item*. Because the swallowed failures are
+transient, the same item could report a full trail one minute and a clean one the next.
+
+Both benign causes still answer `{ events: [] }` exactly as documented. Every other read
+failure now raises `SERVICE_UNAVAILABLE` / 503 carrying the driver error as `cause`, which
+the route's existing error handler turns into an honest 5xx — the same treatment the
+sibling `listCommits` and `getMetaItem` reads in this package already give (ADR-0110 D3: a
+miss and a fault are different facts).

packages/metadata-protocol/src/protocol.audit-read-failure-propagation.test.ts

@@ -0,0 +1,220 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// #9638 — `auditMetaItem`'s unqualified `catch` reported ANY failed audit read
+// as `{ events: [] }`.
+//
+// The catch named two benign causes in its comment ("table not provisioned
+// (legacy env) or driver doesn't expose `find`") and then took every OTHER
+// cause with them: a connection drop, a permission denial, a malformed row, a
+// query bug, a timeout. All of them reached the caller as the well-formed
+// statement "this item has no audit entries".
+//
+// ADR-0110 D3 — a miss and a fault are different facts. This is the compliance
+// surface: `auditMetaItem` is the read behind
+// `GET /api/v1/meta/:type/:name/audit`, which exists so Studio's 审计日志 tab
+// can show who tried what and whether a lock blocked it. An empty answer there
+// reads as *nobody touched this item*.
+//
+// This file pins BOTH directions, because a method that raised unconditionally
+// would satisfy the first half and destroy the documented feature:
+//
+// • a NON-benign failure now propagates as 503 / SERVICE_UNAVAILABLE, which
+// the `/audit` route's existing `handleRouteError` turns into an honest
+// 5xx (it reads `error.status`, `packages/rest/src/error-response.ts`);
+// • BOTH benign causes still answer `{ events: [] }`, verbatim.
+//
+// ⚠️ Anti-vacuity. An "it propagates" assertion is worthless if the assertions
+// cannot see the difference between a populated trail and an empty one in the
+// first place — this repo has been bitten by exactly that shape
+// (`body.item.fields` vs `body.data.item.fields`). The POSITIVE CONTROL below
+// reads a real row all the way through the mapping and asserts its fields, so
+// every "empty" assertion in this file is known to be a measurement rather
+// than a shape that could never have been non-empty.
+
+import { describe, it, expect, vi } from 'vitest';
+import { ObjectStackProtocolImplementation } from './protocol.js';
+
+/** A protocol whose engine read fails with `error`. */
+function protocolWhoseReadFails(error: unknown) {
+ const find = vi.fn(async () => { throw error; });
+ const engine = { registry: { getObject: () => undefined }, find };
+ return { p: new ObjectStackProtocolImplementation(engine as any), find };
+}
+
+/** A protocol whose engine read succeeds, returning `rows`. */
+function protocolReading(rows: any[]) {
+ const find = vi.fn(async () => rows);
+ const engine = { registry: { getObject: () => undefined }, find };
+ return { p: new ObjectStackProtocolImplementation(engine as any), find };
+}
+
+const ITEM = { type: 'views', name: 'shared_grid' } as const;
+
+/**
+ * Capture the rejection, or fail loudly naming what was RESOLVED instead.
+ *
+ * Deliberately not a bare `.rejects.toThrow()`: that cannot separate "answered
+ * with the wrong body" from "did not raise at all", and the wrong body — a
+ * well-formed empty trail — *is* the defect. It also would not print the
+ * `{ events: [] }` that makes a failure here self-explanatory.
+ */
+async function rejectionOf(promise: Promise<unknown>): Promise<any> {
+ let resolved: unknown;
+ try {
+ resolved = await promise;
+ } catch (error) {
+ return error;
+ }
+ throw new Error(
+ `expected the read failure to propagate, but it RESOLVED with `
+ + `${JSON.stringify(resolved)} — the defect: a fault disguised as an empty audit trail`,
+ );
+}
+
+describe('#9638 auditMetaItem: a failed audit read is a fault, not an empty trail', () => {
+ // ── The propagating half ────────────────────────────────────────────────
+ //
+ // Three flavours, because the old catch was unqualified and each of these
+ // means "the rows may well exist and simply were not seen".
+ const nonBenign: Array<[string, Error]> = [
+ ['a connection drop', new Error('connect ECONNREFUSED 127.0.0.1:5432')],
+ ['a permission denial', new Error('permission denied for table sys_metadata_audit')],
+ ['a timeout', new Error('query timeout after 30000ms')],
+ ];
+
+ it.each(nonBenign)(
+ '⭐ THE PIN — %s propagates as 503 SERVICE_UNAVAILABLE instead of `{ events: [] }`',
+ async (_label, driverError) => {
+ const { p } = protocolWhoseReadFails(driverError);
+
+ const error = await rejectionOf(p.auditMetaItem({ ...ITEM }));
+
+ // ADR-0112: `code` AND `status` together. `status` alone would pass
+ // for any 5xx and `code` alone carries no HTTP verdict, and it is
+ // the PAIR the REST boundary reads.
+ expect(error.code).toBe('SERVICE_UNAVAILABLE');
+ expect(error.status).toBe(503);
+ },
+ );
+
+ it('the driver error rides as `cause`, so the operator still sees what actually broke', async () => {
+ const driverError = new Error('connect ECONNREFUSED 127.0.0.1:5432');
+ const { p } = protocolWhoseReadFails(driverError);
+
+ const error = await rejectionOf(p.auditMetaItem({ ...ITEM }));
+
+ // Not the driver error itself: unwrapped it has no `status`, so the
+ // REST boundary would have to guess from message text — and
+ // `mapDataError` guesses `no such table` back into a 404 miss.
+ expect(error.cause).toBe(driverError);
+ });
+
+ it('503 is a status `handleRouteError` turns into a 5xx — not a 2xx and not a client error', async () => {
+ const { p } = protocolWhoseReadFails(new Error('query timeout after 30000ms'));
+
+ const error = await rejectionOf(p.auditMetaItem({ ...ITEM }));
+
+ // The route's catch passes this straight to `handleRouteError`, which
+ // reads `error.status` in the 400-599 band. Pinning the band is what
+ // makes "an honest 5xx" a checkable claim at this layer.
+ expect(error.status).toBeGreaterThanOrEqual(500);
+ expect(error.status).toBeLessThan(600);
+ });
+
+ // ── The benign half — both causes the old comment named ─────────────────
+
+ it.each([
+ ['sqlite', 'no such table: sys_metadata_audit'],
+ ['sqlite, driver-prefixed', 'SQLITE_ERROR: no such table: sys_metadata_audit'],
+ ['postgres', 'relation "sys_metadata_audit" does not exist'],
+ ['mysql', "Table 'db.sys_metadata_audit' doesn't exist"],
+ ])(
+ 'BENIGN 1/2 — an unprovisioned table (%s) still answers `{ events: [] }`',
+ async (_dialect, message) => {
+ const { p } = protocolWhoseReadFails(new Error(message));
+
+ // The documented promise, kept exactly as documented: a legacy
+ // install prior to ADR-0010 has genuinely no rows, so the empty
+ // answer IS the truth and a first boot must not explode.
+ await expect(p.auditMetaItem({ ...ITEM })).resolves.toEqual({ events: [] });
+ },
+ );
+
+ it('BENIGN 2/2 — a host engine exposing no `find` still answers `{ events: [] }`', async () => {
+ // `MetadataHostEngine` carries `[key: string]: any`, so a metadata-only
+ // store or a partial double with no `find` satisfies the type.
+ const engine = { registry: { getObject: () => undefined } };
+ const p = new ObjectStackProtocolImplementation(engine as any);
+
+ await expect(p.auditMetaItem({ ...ITEM })).resolves.toEqual({ events: [] });
+ });
+
+ it('the missing-`find` answer is decided BEFORE the read, not by classifying a TypeError', async () => {
+ // Why this matters: a missing method raises `TypeError: … is not a
+ // function`, and the ONLY thing separating that from a genuine
+ // TypeError raised INSIDE a real driver's `find` (a null deref on a
+ // malformed row — an actual fault) is the V8 message text. Classifying
+ // it in the catch would re-open the fail-open this card closes. So the
+ // capability is asked as a precondition, and a driver that DOES have
+ // `find` and throws a TypeError is a fault.
+ const { p } = protocolWhoseReadFails(
+ new TypeError("Cannot read properties of undefined (reading 'occurred_at')"),
+ );
+
+ const error = await rejectionOf(p.auditMetaItem({ ...ITEM }));
+
+ expect(error.code).toBe('SERVICE_UNAVAILABLE');
+ expect(error.status).toBe(503);
+ });
+
+ // ── Anti-vacuity ────────────────────────────────────────────────────────
+
+ it('POSITIVE CONTROL — a real row maps through, so "empty" above is a measurement', async () => {
+ const { p, find } = protocolReading([{
+ id: 'evt_1',
+ occurred_at: '2026-08-18T10:00:00.000Z',
+ actor: 'alice',
+ source: 'studio',
+ operation: 'save',
+ outcome: 'denied',
+ code: 'METADATA_LOCKED',
+ lock_state: 'locked',
+ lock_overridden: false,
+ request_id: 'req_7',
+ note: 'blocked by package lock',
+ }]);
+
+ const result = await p.auditMetaItem({ ...ITEM });
+
+ // If this file's assertions could not tell a populated trail from an
+ // empty one, THIS is the case that would fail — which is exactly why
+ // it is here rather than assumed.
+ expect(result.events).toHaveLength(1);
+ expect(result.events[0]).toMatchObject({
+ id: 'evt_1',
+ actor: 'alice',
+ operation: 'save',
+ outcome: 'denied',
+ code: 'METADATA_LOCKED',
+ lockState: 'locked',
+ lockOverridden: false,
+ requestId: 'req_7',
+ note: 'blocked by package lock',
+ });
+ expect(find).toHaveBeenCalledTimes(1);
+ });
+
+ it('⭐ a genuine zero-row read and a FAULT are no longer the same answer', async () => {
+ // The equivalence the defect created, stated as one assertion. A read
+ // that succeeded and found nothing is the empty trail; a read that
+ // failed is not an answer at all.
+ const { p: readEmpty } = protocolReading([]);
+ await expect(readEmpty.auditMetaItem({ ...ITEM })).resolves.toEqual({ events: [] });
+
+ const { p: readBroke } = protocolWhoseReadFails(
+ new Error('connect ECONNREFUSED 127.0.0.1:5432'),
+ );
+ const error = await rejectionOf(readBroke.auditMetaItem({ ...ITEM }));
+ expect(error.status).toBe(503);
+ });
+});

packages/metadata-protocol/src/protocol.ts

@@ -6334,6 +6334,21 @@ export class ObjectStackProtocolImplementation implements
  * prior to ADR-0010) the call returns `{ events: [] }` instead of
  * raising, keeping the Studio tab harmless.
  *
+ * [#9638] `{ events: [] }` means "the audit trail was read and this item
+ * has no entries" and NOTHING else. Exactly two causes answer it without a
+ * read: the unprovisioned table above, and a host engine that exposes no
+ * `find` (the `typeof` probe before the read). Every other failure — a
+ * connection drop, a permission denial, a timeout, a malformed row, a query
+ * bug — RAISES, because the rows may well exist and simply were not seen,
+ * and a compliance reader must never be handed "nobody touched this item"
+ * on those terms (ADR-0110 D3). The unqualified `catch` that used to report
+ * all of them as an empty trail is what this closes.
+ *
+ * @throws {@link metadataStoreUnavailableError} — a 503 carrying the driver
+ * error as `cause`, for every read failure that is not an
+ * unprovisioned table. The `/audit` route's existing
+ * `handleRouteError` turns it into an honest 5xx.
+ *
  * `organizationId` SCOPES the read and is enforced in the query below:
  * rows for that organization plus env-wide (`organization_id IS NULL`)
  * rows, and nothing else. Omitted (or `null`) reads the env-wide rows
@@ -6437,6 +6452,35 @@ export class ObjectStackProtocolImplementation implements
  // organization reads exactly the env-wide rows an org-less write
  // produces. Fail-closed, and symmetric with the write path.
  const organizationId = request.organizationId ?? null;
+ // [#9638] The FIRST of the two benign causes the catch below used to
+ // name, asked as a PRECONDITION rather than as an error shape.
+ //
+ // `MetadataHostEngine` carries `[key: string]: any`, so a metadata-only
+ // store or a partial test double with no `find` satisfies the type and
+ // reaches here. That is a real, documented deployment shape and it must
+ // keep answering `{ events: [] }`.
+ //
+ // ⚠️ Asked HERE, before the `try`, because it cannot be asked soundly
+ // INSIDE the catch. Measured: a missing method raises
+ // `TypeError: this.engine.find is not a function`, which
+ // `isMissingTableError` correctly reports as NOT benign — but the only
+ // signal separating it from a genuine `TypeError` raised *inside* a
+ // real driver's `find` (a malformed row, a null deref — actual faults)
+ // is the V8 message text. Sniffing that text would re-open exactly the
+ // fail-open this card closes, one error class narrower. A `typeof`
+ // probe is a fact about the engine, not a guess about an error, so it
+ // cannot misclassify a fault as a capability gap.
+ //
+ // Same shape as the sibling limb one layer up: the `/audit` route's own
+ // capability probe (`typeof p.auditMetaItem !== 'function'`, #9426)
+ // likewise decides BEFORE the call rather than classifying its failure.
+ if (typeof (this.engine as { find?: unknown }).find !== 'function') {
+ console.warn(
+ `[Protocol] auditMetaItem: host engine exposes no \`find\`; `
+ + `reporting no audit entries for ${request.type}/${request.name}`,
+ );
+ return { events: [] };
+ }
  try {
  // Org-scoped lookup: include rows for the specific org AND
  // env-wide (organization_id IS NULL) rows so the editor
@@ -6496,8 +6540,31 @@ export class ObjectStackProtocolImplementation implements
  }));
  return { events };
  } catch (err: any) {
- // Table not provisioned (legacy env) or driver doesn't
- // expose `find` — return empty rather than 500ing the tab.
+ // [#9638] Benign (the table has not been provisioned in this legacy
+ // env) falls through to the empty answer; everything else is a read
+ // that DID NOT HAPPEN and leaves as a 503. Byte-for-byte the shape
+ // the sibling {@link listCommits} carries (#5980), and the same
+ // {@link isMissingTableError} predicate `DatabaseLoader` (#5108) and
+ // `SysMetadataRepository` (#4867) ask — a driver quirk is taught to
+ // the platform once rather than re-spelled per seam.
+ //
+ // This `catch` used to be UNQUALIFIED. A connection drop, a
+ // permission denial, a malformed row, a query bug or a timeout was
+ // reported to the caller as the well-formed statement "this item has
+ // no audit entries" — ADR-0110 D3 broken (a miss and a fault are
+ // different facts) on the COMPLIANCE surface. `auditMetaItem` is the
+ // read behind `GET /api/v1/meta/:type/:name/audit`, which exists so
+ // Studio's 审计日志 tab can show who tried what and whether a lock
+ // blocked it; an empty answer there reads as *nobody touched this
+ // item*. Worse than the static capability gap #9426 fixed one layer
+ // up, because a transient read failure makes the same item report a
+ // full trail one minute and a clean one the next. The `console.warn`
+ // below is on the SERVER; it was never an answer to the reader.
+ //
+ // The second cause the old comment named — a host engine with no
+ // `find` — is decided by the precondition probe above the `try`, so
+ // it never reaches here and this arm has exactly ONE benign cause.
+ this.rethrowUnlessMetadataStoreUnprovisioned(err);
  console.warn(
  `[Protocol] auditMetaItem read failed for ${request.type}/${request.name}: ${err?.message ?? err}`,
  );