Skip to content

Commit b9b22b9

Browse files
fix(metadata-protocol): leave a denied audit row for every documented refusal code at the save, publish, rollback and delete doors (#28393)
<!-- claude-projects-attribution: {"github_login":"os-zhuang"} --> _Requested by **Sam** · [project thread](https://claude.ai/code/project/chan_01LjZwNRFiG2fkp8SNf6rHNx?thread=cmsg_01LjZwNRFiG2fkp8SNf6rHNx7AAmZNhrNzszS8chcmhkgm)_ Fixes #28355 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:元数据的保存、发布、回滚、删除四个入口,遇到 `NOT_OVERRIDABLE` / `NOT_CREATABLE` / `INVALID_METADATA` / `DESTRUCTIVE_CHANGE` 拒绝时,会在 `sys_metadata_audit` 里写一条 `denied` 记录。 - 为什么改:审计表文档一直写着这四个拒绝码,但从没有任何代码写过,读 `/audit` 的人会以为被拒的尝试从未发生。 - 风险与回滚:调用方拿到的错误(码、状态、文案)完全不变;审计写入是尽力而为,失败不影响请求。回滚即 revert 本 PR。 - 先看:`packages/metadata-protocol/src/protocol-refusal-audit.ts`(映射表与去重标记),`protocol.ts` 四个入口的包装段,`protocol.refusal-audit-rows.test.ts`。 - 两分钟验证:`pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2 src/protocol.refusal-audit-rows.test.ts`,预期 12 个用例全绿。 - 需要你做的:无,席位落地。 ## What changes - **New module `protocol-refusal-audit.ts`.** A closed map from the four catalogued refusal codes to the lower-case values `sys_metadata_audit.code` already documents (`not_overridable`, `not_creatable`, `invalid_metadata`, `destructive_change`), `refusalAuditEntry` (the row a refusal owes at a door, or none), and `withRefusalAudit` (run the door, record the row on such a refusal, rethrow the same error). - **The four single-item doors are wrapped.** `saveMetaItem`, `publishMetaItem`, `rollbackMetaItem` and `deleteMetaItem` keep their names and signatures; each is now a short wrapper over a `protected …Unaudited` body holding the unchanged code. The row is written where the door hands the refusal back, so a refusal raised by any gate inside the door (including shared helpers such as the ADR-0131 D6 organization-scoped refusal) is covered, and so is one added later. - **The row.** `outcome: 'denied'`, `operation` the door, `actor` / `source` as the door's allowed row records them, `note` = `clientFacingFailureText` (the sentence the caller was already shown; the column is wire-visible), and `type` folded to the canonical singular (the writer refuses anything else). - **What still writes no row.** Any other code (`INVALID_REQUEST`, `RESOURCE_CONFLICT`, driver faults, 5xx) writes nothing; the column's set stays closed. `item_locked` and `metadata_conflict` are not in the map, so their own gates' rows are not duplicated. A refusal handed back through two wrapped doors is recorded once (a marker on the error). The batch publish keeps its `batch_aborted` row. - **Nothing removed from the vocabulary.** All five codes the card lists are refused by a real door; `item_locked` was already stamped (see Acceptance notes), so nothing is trimmed. ## Why `sys_metadata_audit` promised denial rows that never appeared, so an operator or agent reading `/audit` for a metadata name concluded that a refused authoring attempt never happened. Triage's lean (enforce at the refusing door, beside `metadata_conflict`) is followed. ## Tests - `protocol.refusal-audit-rows.test.ts` (new, 12 cases), driven through the real doors with a fake engine that persists audit inserts: one denied row per enforced code on the save door (`not_creatable`, `not_overridable`, `destructive_change`, `invalid_metadata`), the plural spelling filed under the canonical type, the publish / rollback / delete doors each stamping their own `operation`, plus an allowed control (one `allowed` / `ok` row, no denied row), an out-of-map refusal (`INVALID_REQUEST`) writing nothing, the closed map, and a refusal through two doors recorded once. - Five existing pins asserted that a refusal touches the engine not at all, or scanned the door's source by member name. Each is reconciled to the new truth rather than loosened: - `protocol.code-only-types.test.ts` (#5264 block), `protocol.lock-org-axis-agree.test.ts`, `protocol.tenant-authored-write.test.ts`: the only engine call a refusal makes is now its own `denied` audit row, asserted exactly (table, operation, code), and still no `sys_metadata` write. - `protocol.read-verb-canonical-fold.test.ts`, `protocol.view-expansion-reconcile-call-sites.test.ts`: the fold and the stored-row writes now live in the `…Unaudited` bodies; the rosters name those members (and the method scan admits `protected`, which only these four use). - `scripts/engine-double-contract.pinned.json`: the new test's fake engine is recorded, as `check:engine-double-contract` requires. ## Acceptance notes - **`item_locked` (the dispatch asked):** it is already stamped. `assertLockAllowsWrite` / `assertLockAllowsDelete` / `lockWriteRefusal` in `protocol-lock-version-gates.ts` write `code: 'item_locked'` with `lock_state` on all four doors, and the batch publish re-records it outside its transaction (`recordPendingDenialAudit`). Not measured: whether the repository's own read-only-base refusal (`SysMetadataRepository.readOnlyBaseOverrideError`, `ITEM_LOCKED` with `lockSource: 'package'`) is ever reached past that gate without a row; it is deliberately not in the map, so a lock-gate refusal cannot be written twice. - `sys-metadata-audit.object.ts` is unchanged: its documented vocabulary is now true, so no trim was needed. - One file outside the claimed surface: `scripts/engine-double-contract.pinned.json`, the gate's own ledger entry for the new test double. ## Verification All readings are on head `7e9960c8da` (merged with `origin/main` at `fc5d5093b`), in a dedicated worktree. - **Package suite:** `pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2` (under the verify lock) → `Test Files 285 passed | 4 skipped (289)`, `Tests 21872 passed | 26 skipped (21898)`. Before the five pins were reconciled the same run was 15 red in exactly those five files. - **Typecheck:** `pnpm --filter @objectstack/metadata-protocol typecheck` → exit 0 (its `tsconfig.json` includes `src/**/*`, so the test files are compiled). - **Gates:** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 69 commands; all 69 ran and exited 0. `--ran` reconciliation: `69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN`, every family carrying its exit code. The two real reds on the first pass were fixed in this PR (`check:objectql-double-limit`: the new `find` double now applies `limit`; `check:engine-double-contract`: the double is recorded in the ledger). - **Lint:** `pnpm lint` → exit 0. - `check:platform-checklist` not run: no exported symbol is renamed or removed (the four doors keep their names and signatures). - Dogfood battery: n/a (no `!` in the title, no `(narrowing)` arm). <details><summary>Ablation of the audit write (one-off, not kept)</summary> Predicted before running: the nine denied-row cases red, the allowed control, the `INVALID_REQUEST` no-row case and the closed-map case green. `node scripts/ablation-replace.mjs` on `packages/metadata-protocol/src/protocol-refusal-audit.ts`, anchor `if (entry) {`, replaced by a guard that is always false (`entry && Date.now() is negative`), running `vitest run src/protocol.refusal-audit-rows.test.ts` under it. - mutation landed: anchor 1 → 0, blob `4b5d1b1c9c20` → `35ebe7c00ab9` - `Tests 9 failed | 3 passed (12)`, exactly as predicted - restored: blob after restore `4b5d1b1c9c20` == HEAD, `git diff HEAD` empty The unit suite imports `./protocol.js` from source, so no rebuild was involved. </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: n/a, not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the 维护者速读 block is the repo's required Chinese maintainer summary). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01QUs3Bs2hELyx9Go8Efyui5)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 125ad06 commit b9b22b9

12 files changed

Lines changed: 706 additions & 119 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): a metadata write refused with `NOT_OVERRIDABLE`, `NOT_CREATABLE`, `INVALID_METADATA` or `DESTRUCTIVE_CHANGE` now leaves its `denied` row in `sys_metadata_audit`
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`sys_metadata_audit.code` has always documented `not_overridable`, `not_creatable`, `invalid_metadata` and `destructive_change` as `denied` values, and the save, publish, rollback and delete doors refuse with exactly those codes. No audit write stamped them, though: only the lock gate (`item_locked`) and the stale-version check (`metadata_conflict`) left a row. A reader of `GET /api/v1/meta/:type/:name/audit` concluded that a refused attempt never happened.
11+
12+
- **The row.** `outcome: 'denied'`, `code` the documented lower-case value, `operation` the door (`save`, `publish`, `rollback` or `delete`), `actor` and `source` as the door's allowed row records them, and `note` the sentence the caller was refused with. The type is filed under its canonical singular, as every other row is.
13+
- **Where it is written.** Where the door hands the refusal back to its caller, so a refusal raised by any of the gates inside the door is covered, including one added later.
14+
- **What still writes no row.** A refusal with any other code (`INVALID_REQUEST`, `RESOURCE_CONFLICT`, a driver fault, any 5xx) writes nothing, as before; the column's set of values stays closed. `item_locked` and `metadata_conflict` keep the rows their own gates write and are not written twice. A refusal one door hands back through another is recorded once. The batch package publish keeps its `batch_aborted` row.
15+
- **No caller change.** The refusal the caller receives is unchanged: the same code, status and message. The audit write is best-effort, as it always was.
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#28355] The `denied` audit row a single-item metadata door owes for a
5+
* refusal that writes no row of its own.
6+
*
7+
* `sys_metadata_audit.code` documents `not_overridable`, `not_creatable`,
8+
* `invalid_metadata` and `destructive_change` as `denied` values, and the four
9+
* single-item doors (`saveMetaItem`, `publishMetaItem`, `rollbackMetaItem`,
10+
* `deleteMetaItem`) do refuse with exactly those catalogued codes — but only
11+
* the lock gate (`item_locked`) and the optimistic-concurrency check
12+
* (`metadata_conflict`) ever wrote a row. Every other refusal left the trail
13+
* silent, so a reader of `/audit` concluded that a refused attempt never
14+
* happened.
15+
*
16+
* The row is stamped where the door hands the refusal back to its caller,
17+
* not at each of the many sites inside the door that can raise one: those
18+
* codes are thrown by shared helpers the doors reach in several places
19+
* (`refuseOrganizationScopedWrite`, the overlay and create gates, the
20+
* authoring-rule and spec gates, the destructive-change gate), so a per-site
21+
* write would leave any site added later silent again. One seam per door
22+
* reads the code the refusal carries and nothing else.
23+
*
24+
* What it deliberately does NOT do:
25+
*
26+
* - It mints no new value. The map below is closed, and each value is one
27+
* the column already documents. A refusal whose code is not in it — a
28+
* driver fault, `INVALID_REQUEST`, `RESOURCE_CONFLICT`, any 5xx — leaves
29+
* no row, exactly as before; lower-casing whatever `code` arrived would
30+
* turn the column's closed set into an open one (the `batch_aborted`
31+
* argument in the object file).
32+
* - It does not re-stamp `item_locked` or `metadata_conflict`: their gates
33+
* already write their own row, and neither code is in the map.
34+
* - It writes one row per refusal even when one door hands it back through
35+
* another: the first seam to record a refusal marks it, and every outer
36+
* seam it passes through after that leaves it alone.
37+
* - The batch publish keeps its own `batch_aborted` row with the causal code
38+
* in `note`; it is not one of these doors.
39+
*
40+
* `note` carries {@link clientFacingFailureText}: it is wire-visible through
41+
* `GET /meta/:type/:name/audit`, and these refusals declare a 4xx, so their
42+
* sentence is the one the caller was already shown.
43+
*/
44+
45+
import type { MetadataAuditEntry } from './protocol.js';
46+
import { canonicalMetaType, clientFacingFailureText } from './protocol.js';
47+
48+
/**
49+
* The catalogued refusal codes (ADR-0112, SCREAMING) whose door stamps a
50+
* `denied` row, and the persisted audit value each one is stored as (D6b —
51+
* the column keeps its own lower-case vocabulary).
52+
*/
53+
export const REFUSAL_AUDIT_CODES: Readonly<Record<string, string>> = Object.freeze({
54+
NOT_OVERRIDABLE: 'not_overridable',
55+
NOT_CREATABLE: 'not_creatable',
56+
INVALID_METADATA: 'invalid_metadata',
57+
DESTRUCTIVE_CHANGE: 'destructive_change',
58+
});
59+
60+
/** The door a refusal left, and the request fields its row is keyed on. */
61+
export interface RefusalAuditDoor {
62+
operation: 'save' | 'publish' | 'rollback' | 'delete';
63+
type: unknown;
64+
name: unknown;
65+
actor: unknown;
66+
source: string;
67+
}
68+
69+
/**
70+
* Name-mangled like `__pendingAudit`: an internal hand-off between two seams,
71+
* never a wire field. `clientFacingFailureText` / `clientFacingFailureCode`
72+
* read `message` / `code` only.
73+
*/
74+
type RefusalWithAuditMark = { code?: unknown; __refusalAudited?: true };
75+
76+
/**
77+
* The row `err` owes at `door`, or `null` when it owes none: its code is not
78+
* one of {@link REFUSAL_AUDIT_CODES}, an earlier seam already recorded it, or
79+
* the request names no item to key the row on.
80+
*/
81+
export function refusalAuditEntry(err: unknown, door: RefusalAuditDoor): MetadataAuditEntry | null {
82+
if (err === null || typeof err !== 'object') return null;
83+
const refusal = err as RefusalWithAuditMark;
84+
if (refusal.__refusalAudited) return null;
85+
if (typeof refusal.code !== 'string' || !Object.prototype.hasOwnProperty.call(REFUSAL_AUDIT_CODES, refusal.code)) return null;
86+
// No row without an identity: the trail is keyed on `(type, name)`, and
87+
// minting one for a request that names no item would file a fictional
88+
// item in a compliance ledger.
89+
if (typeof door.type !== 'string' || door.type === '') return null;
90+
if (typeof door.name !== 'string' || door.name === '') return null;
91+
return {
92+
// The writer refuses a non-canonical type; the door may be handed a
93+
// plural spelling it has not folded yet when the refusal comes first.
94+
type: canonicalMetaType(door.type),
95+
name: door.name,
96+
organizationId: null,
97+
operation: door.operation,
98+
outcome: 'denied',
99+
// adr0112-ok: D6b — persisted audit column, its own vocabulary
100+
code: REFUSAL_AUDIT_CODES[refusal.code]!,
101+
...(typeof door.actor === 'string' && door.actor !== '' ? { actor: door.actor } : {}),
102+
source: door.source,
103+
note: clientFacingFailureText(err, refusal.code),
104+
};
105+
}
106+
107+
/**
108+
* Run a door's body; when it refuses with a code that owes a row, record the
109+
* row and rethrow the same refusal untouched.
110+
*
111+
* `record` is the protocol's own `recordMetadataAudit`, handed in as a
112+
* closure so a spy set on the instance is the one reached. It is best-effort
113+
* by contract (ADR-0010 §3.6) and never replaces the refusal the caller gets.
114+
*/
115+
export async function withRefusalAudit<T>(
116+
record: (entry: MetadataAuditEntry) => Promise<void>,
117+
door: RefusalAuditDoor,
118+
run: () => Promise<T>,
119+
): Promise<T> {
120+
try {
121+
return await run();
122+
} catch (err) {
123+
const entry = refusalAuditEntry(err, door);
124+
if (entry) {
125+
(err as RefusalWithAuditMark).__refusalAudited = true;
126+
await record(entry);
127+
}
128+
throw err;
129+
}
130+
}

‎packages/metadata-protocol/src/protocol.code-only-types.test.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,9 +536,11 @@ describe('code-only metadata types are refused on every kernel (#5086)', () => {
536536

537537
expect(err?.status).toBe(403);
538538
expect(metaRows(rows)).toEqual([]);
539-
// Stronger than "no row landed": no write was even attempted,
540-
// so there is nothing for a raw-engine path to have done.
541-
expect(writes).toEqual([]);
539+
// Stronger than "no row landed": no metadata write was even
540+
// attempted, so there is nothing for a raw-engine path to have
541+
// done. [#28355] The one engine call the refusal makes is its
542+
// own `denied` row in the audit trail, and nothing else.
543+
expect(writes).toEqual([{ op: 'insert', table: 'sys_metadata_audit' }]);
542544
});
543545
}
544546

‎packages/metadata-protocol/src/protocol.lock-org-axis-agree.test.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,13 @@ describe('[#21716] pin 2 — an env-wide _lock: full row binds an organization w
171171
expect({ code: err.code, status: err.status }, operation)
172172
.toEqual({ code: 'NOT_OVERRIDABLE', status: 403 });
173173
}
174-
// [ADR-0131 D6] Refused before the `_lock` gate: nothing is written.
175-
expect(inserted).toEqual([]);
174+
// [ADR-0131 D6] Refused before the `_lock` gate: no metadata row and
175+
// no `item_locked` row is written. [#28355] Each refusal leaves its
176+
// own `not_overridable` row in the audit trail, and nothing else.
177+
expect(inserted.map((i) => [i.table, i.values.operation, i.values.code])).toEqual([
178+
['sys_metadata_audit', 'save', 'not_overridable'],
179+
['sys_metadata_audit', 'delete', 'not_overridable'],
180+
]);
176181
});
177182

178183
it(`${kernel} kernel: an org-scoped publish and rollback are refused the same way (the shared write gate)`, async () => {

‎packages/metadata-protocol/src/protocol.read-verb-canonical-fold.test.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,41 @@ describe('#9157 — the population, re-derived from the code rather than from th
235235
// the failure mode this whole describe block exists to prevent.
236236
expect(starts.length).toBeGreaterThan(20);
237237

238+
// [#28355] The four single-item doors are public wrappers (the refusal
239+
// audit) over a private `…Unaudited` body, and both read their request
240+
// shape from a module-level alias (`type SaveMetaItemRequest = {…}`): a
241+
// private member's type is not published. So a signature naming such an
242+
// alias is read from the alias, and a wrapper is judged by the body it
243+
// delegates to — that is where the fold is.
244+
const memberSlice = (line: number) => {
245+
const k = starts.findIndex((s) => s.line === line);
246+
const end = k >= 0 ? starts[k + 1]?.line : lines.findIndex((l, i) => i > line && METHOD_OR_PRIVATE.test(l));
247+
return lines.slice(line, end === undefined || end < 0 ? lines.length : end).join('\n');
248+
};
249+
const METHOD_OR_PRIVATE = /^ {4}(?:private |protected )?(?:static )?(?:async )?[A-Za-z_][A-Za-z0-9_]*\s*\(/;
250+
const aliasLiteral = (name: string): string | undefined => {
251+
const at = src.indexOf(`\ntype ${name} = {`);
252+
if (at < 0) return undefined;
253+
const open = src.indexOf('{', at);
254+
let depth = 0;
255+
for (let i = open; i < src.length; i++) {
256+
if (src[i] === '{') depth++;
257+
else if (src[i] === '}' && --depth === 0) return src.slice(open, i + 1);
258+
}
259+
return undefined;
260+
};
261+
const delegateBody = (body: string): string => {
262+
const to = /this\.([A-Za-z_][A-Za-z0-9_]*Unaudited)\(request\)/.exec(body)?.[1];
263+
if (to === undefined) return body;
264+
const line = lines.findIndex((l) => l.startsWith(` private async ${to}(`));
265+
expect(line, `the wrapper delegates to '${to}', which the scan cannot find`).toBeGreaterThanOrEqual(0);
266+
return memberSlice(line);
267+
};
268+
238269
const unfolded: string[] = [];
239270
const folded: string[] = [];
240271
for (let k = 0; k < starts.length; k++) {
241-
const body = lines.slice(starts[k]!.line, starts[k + 1]?.line ?? lines.length).join('\n');
272+
const body = delegateBody(lines.slice(starts[k]!.line, starts[k + 1]?.line ?? lines.length).join('\n'));
242273
// The PARAMETER LIST exactly — walked by paren balance rather than
243274
// guessed at with a character budget, so a long request type cannot
244275
// push `type: string` outside the window and drop a verb silently.
@@ -249,7 +280,9 @@ describe('#9157 — the population, re-derived from the code rather than from th
249280
if (body[i] === '(') depth++;
250281
else if (body[i] === ')' && --depth === 0) { close = i; break; }
251282
}
252-
const sig = body.slice(open, close + 1);
283+
const declared = body.slice(open, close + 1);
284+
const alias = /^\(\s*request\??\s*:\s*([A-Z][A-Za-z0-9_]*)\s*\)$/.exec(declared)?.[1];
285+
const sig = alias === undefined ? declared : (aliasLiteral(alias) ?? declared);
253286
// A REQUIRED `type: string` in the request object — the shape a
254287
// `/meta/:type` path segment arrives as. `type?:` is a query-string
255288
// FILTER (`/meta/diagnostics?type=`, `/meta/_drafts?type=`), a

0 commit comments

Comments
 (0)