Skip to content

Commit 875d007

Browse files
fix(rest): GET /meta/diagnostics refuses an unknown type, severity or package, and keys an alias by its canonical type (#27514) (#27620)
domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F <!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4Uu5mq5spkb2GMJ5SN6jEWmDk)_ Fixes #27514 Clause-②: no (narrowing — owes one contract-review-tier review before the queue) Metadata-protocol: none Skills-docs: follow-up #27632 ## 维护者速读 - **改了什么**:诊断接口 `GET /meta/diagnostics` 遇到不认识的类型、严重级别或包,不再回 200「全部正常」,而是回 400 并说明原因;用别名(如 `views`)查询时统计按标准类型名(`view`)给出;该接口所有拒绝统一成一种错误信封。 - **为什么改**:这个接口是作者、AI 和 CI 判断元数据是否健康的依据。拼错类型或包名时它回「扫描 1 个类型,0 个问题」,调用方会误以为一切正常。 - **风险与回滚**:只收窄这一个只读接口的请求面,合法请求的响应形状和键集不变(别名请求的统计键从别名改为标准名,是本卡要求的变化)。回滚即还原本 PR。 - **先看**:`packages/rest/src/meta-collection-routes.ts`(诊断路由:severity 校验、类型拒绝、统一信封);`packages/metadata-protocol/src/protocol-meta-diagnostics.ts`(未知包拒绝、标准类型键);`packages/rest/src/rest-meta-diagnostics-unknown-params.test.ts`。 - **两分钟验证**:`pnpm --filter @objectstack/rest exec vitest run src/rest-meta-diagnostics-unknown-params.test.ts`,预期 11 个用例全部通过。 - **需要你做的**:无,席位落地(落地前需一次 contract-review-tier 评审)。 ## What changes Measured on `main` @ `556f3230` through the real route (a real `ObjectStackProtocolImplementation` over a real `SchemaRegistry`), every premise on the card held: | query | before | after | | :-- | :-- | :-- | | `?type=zzz_unknown` | 200, `scannedTypes: 1`, `stats.zzz_unknown` zeros | 400 `INVALID_REQUEST` | | `?severity=bogus` | 200, read as the error sweep | 400 `INVALID_REQUEST` | | `?package=com.nope.missing` | 200, zeros for all 30 swept types | 400 `INVALID_REQUEST` | | `?type=views` | 200, `stats` and `entries` keyed `views` | 200, keyed `view`, byte-equal to `?type=view` | | `?type=viewes` (#8924) | 400 flat `{ error: '…', code }` | 400 nested `{ error: { code, message } }`, same message | | `?type=view&type=object` (#6877) | 400 nested `VALIDATION_ERROR` | unchanged | - **severity** (route): `error` or `warning`; an empty value keeps the default, as an empty `type` keeps the full sweep. - **type** (route): after the sweep, the route asks the list door's own verdict, `refuseUnknownMetaListType` (the static spelling contract, then the live type set from `getMetaTypes`), so `GET /meta/:type` and the diagnostics door agree on which names are types, and a plugin type the deployment carries live still answers. Asked after the sweep so the #8924 near-miss refusal, which names the spellings the caller reached for, keeps answering first. - **package** (sweep): refused only when the sweep found no item bound to it AND the installed-package listing (`getMetaItems` of type `package`, the registry's records) does not carry it. An installed package with no items still answers its zeros, and rows bound to an uninstalled package are still swept. Fails open when the listing cannot be read. - **alias key** (sweep): a named type is swept and keyed under `canonicalMetaUrlType(type)`, the fold `canonicalMetaType` applies at every `/meta` entry point. A spelling the fold does not map (`viewes`, `fieldz`) is unchanged, so the #8924 refusal still sees what the caller wrote. - **one envelope** (route): every `400 INVALID_REQUEST` the route throws or receives now leaves in the nested `{ error: { code, message } }` its repeated-parameter refusal already used (message bounded by `truncateClientMessage`). Everything else keeps `handleRouteError`. Every code is an existing ledgered one (`INVALID_REQUEST`, `VALIDATION_ERROR`); no new code, nothing in `packages/spec`. `metadata-protocol/src/protocol.ts` is untouched: the package check uses `getMetaItems`, which `MetaDiagnosticsHost` already carries. ## Tests - New `packages/rest/src/rest-meta-diagnostics-unknown-params.test.ts` (11 cases, through the HTTP route): the three refusals, both envelope cases, the alias case, and six preservation cases (`severity` warning/error/empty/absent, an installed package, `package=all`, an installed package with no items, items bound to an uninstalled package, a live plugin type). On `main`: 5 failed, 6 passed. On this branch: 11 passed. - Two existing pins moved, both the alias key this card retires: `protocol.diagnostics-store-outage.test.ts` (`stats.fields` to `stats.field`) and `rest-server-meta-read-org-scope.test.ts` (`stats.views` to `stats.view`, plus `stats.views` absent). - `pnpm --filter @objectstack/metadata-protocol test`: 279 files passed, 21781 tests passed. `pnpm --filter @objectstack/rest test`: 335 files passed, 6589 tests passed. `typecheck` on both packages: exit 0. - Ablation, one leg per pin, each through `scripts/ablation-replace.mjs` (anchor hit 1 to 0, restore blob == HEAD, `git diff HEAD` empty): severity check, type verdict, envelope writer (all three in `rest` src, read by the test from source), and the package refusal and canonical key (in `metadata-protocol`, which the test reads from `dist/`: rebuilt, `ablation-dist-preflight` marker present exit 0, then rebuilt again and `--absent` exit 0). Each leg turned exactly its own pins red: severity 1, type 1, envelope 4, package 1, alias 1. ## Acceptance notes - `scripts/engine-double-contract.pinned.json` gains the new rig's `findOne` double (written by `check-engine-double-contract.mjs --write`); the rig opens `findOne` with `assertEngineFindOnePredicate`. That ledger is outside the claimed file surface and is the gate's own bookkeeping for the new test. - With `?type=X&package=P`, the package refusal reads the swept types only, so an uninstalled `P` whose orphan rows are all of other types is refused with that type filter. Without the filter the full sweep finds them. - The branch name is the card thread's harness-pinned `claude/project-thread-nx5qkr`, not `claude/issue-27514-…`; the claim records it. ## Verification All readings on head `59ef3dbe`. - `node scripts/pm/dispatch-gates.mjs --commands`: 69 derived families, each run with its exit code captured before any pipe, all exit 0. `--ran`: 69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN. - `pnpm lint` (repo-wide, `eslint . --no-inline-config`): exit 0. eslint on `packages/rest` and `packages/metadata-protocol`: exit 0. - Dogfood battery (`pnpm --filter @objectstack/dogfood test` under `os-verify-lock.sh`, no shard, no path, closure built): the lock's `VERDICT command-exit 0`; Test Files 350 passed, 3 skipped (353); Tests 2817 passed, 20 skipped (2837); no failure. - Open PRs on the touched files: 36 open PR heads (every open PR but the release and mailbox branches) fetched and diffed against their merge bases; none touches the four source and test files. Two (#27535, #27474) also add rows to `scripts/engine-double-contract.pinned.json`; `git merge-tree --write-tree` of this head with each answers exit 0 (no conflict). --- _Generated by [Claude Code](https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a552e86 commit 875d007

7 files changed

Lines changed: 325 additions & 5 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/rest': patch
3+
'@objectstack/metadata-protocol': patch
4+
---
5+
6+
fix(rest): `GET /meta/diagnostics` refuses an unknown type, severity or package instead of answering an all-clear, and keys an alias's stats by its canonical type (#27514)
7+
8+
Clause-②: no (narrowing — owes one contract-review-tier review before the queue)
9+
Metadata-protocol: none
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) The narrowed face is a REQUEST to one read door: query values that were never honoured (an unknown type, severity or package) are now refused instead of answered with an empty sweep, and an alias's stats move to the canonical key the sweep already listed it under. No stored metadata, package source or object definition holds a diagnostics request or response, no schema key or export changes, and the remedy is the caller's own query string, so there is nothing for `objectstack migrate meta` to rewrite. -->
12+
13+
`/meta/diagnostics` is the door an author, an agent or CI reads to learn whether metadata is healthy, and three query values it could not honour were answered `200` with zero issues.
14+
15+
- **`?type=`** naming no metadata type (`zzz_unknown`) is refused with `400 INVALID_REQUEST`, by the same verdict `GET /meta/:type` uses: the platform's static type spelling, then the types this deployment carries live. A near-miss spelling of a declared type (`viewes`) keeps its existing refusal, which names the spellings it was reaching for.
16+
- **`?severity=`** other than `error` or `warning` is refused with `400 INVALID_REQUEST`. An empty value still means the default, `error`.
17+
- **`?package=`** naming a package that is not installed, and that no swept item is bound to, is refused with `400 INVALID_REQUEST`. An installed package with no items still answers its zeros, items bound to an uninstalled package are still swept, and `package=all` still sweeps every package.
18+
- **An alias** (`?type=views`) is swept and keyed in `stats` and `entries` under its canonical type (`view`), so it answers exactly what `?type=view` answers.
19+
- **One envelope.** Every refusal of this route now answers the nested `{ error: { code, message } }` the repeated-parameter refusal already used. The near-miss type refusal used to answer `{ error: '…', code }`.

‎packages/metadata-protocol/src/protocol-meta-diagnostics.ts‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import type { ObjectStackProtocolImplementation, ShippingPackages } from './protocol.js';
3737
import { resolveItemLock } from './item-lock.js';
3838
import { DEFAULT_METADATA_TYPE_REGISTRY, getMetadataTypeSchema } from '@objectstack/spec/kernel';
39+
import { canonicalMetaUrlType } from '@objectstack/spec/shared';
3940
import { computeMetadataDiagnostics } from './metadata-diagnostics.js';
4041
import type { MetadataDiagnostics } from './metadata-diagnostics.js';
4142

@@ -77,8 +78,15 @@ export async function getMetaDiagnostics(
7778
stats: Record<string, { count: number; locked: number; packages: string[] }>;
7879
}> {
7980
const includeWarnings = request.severity === 'warning';
81+
// [#27514] A named type is swept, and keyed in `stats` and `entries`, under
82+
// its CANONICAL spelling: the fold `canonicalMetaType` applies at every
83+
// `/meta` entry point. `getMetaItems` already listed `views` as `view`, but
84+
// the answer was keyed by the raw alias, so a consumer reading `stats.view`
85+
// missed an alias request's counts. A spelling the fold does not map
86+
// (`viewes`, `fieldz`) comes back unchanged, so the [#8924] refusal below
87+
// still sees what the caller wrote.
8088
const targetTypes = request.type
81-
? [request.type]
89+
? [canonicalMetaUrlType(request.type)]
8290
: DEFAULT_METADATA_TYPE_REGISTRY
8391
.filter((e) => getMetadataTypeSchema(e.type))
8492
.map((e) => e.type);
@@ -240,6 +248,29 @@ export async function getMetaDiagnostics(
240248
stats[t] = { count: items.length, locked: lockedCount, packages: [...pkgSet].sort() };
241249
}
242250

251+
// [#27514] A package that is not installed, and that no swept item is bound
252+
// to, scoped the sweep to nothing: every type answered `count: 0` and the
253+
// whole answer read as a clean package. Refused instead, with the [#8924]
254+
// refusal's status and code. Both halves are read, in this order: an item
255+
// bound to the package is proof enough (rows of an uninstalled package are
256+
// real, and this sweep is how an author finds them), and only an empty
257+
// sweep asks the installed-package listing (`getMetaItems` of type
258+
// `package`, the registry's records). An installed package with no items
259+
// still answers its honest zeros. Fails OPEN: a listing that cannot be read
260+
// keeps the answer, because "no such package" is an existence claim and an
261+
// unreadable authority cannot make it.
262+
if (request.packageId !== undefined && scannedItems === 0
263+
&& !(await packageIsInstalled(this, request.packageId))) {
264+
const err = new Error(
265+
`'${request.packageId}' is not an installed package, and no metadata item is bound to it, so a sweep `
266+
+ `scoped to it would report a clean package that does not exist. GET /api/v1/packages lists the `
267+
+ `installed packages; omit package (or pass package=all) to sweep every package.`,
268+
);
269+
(err as any).code = 'INVALID_REQUEST';
270+
(err as any).status = 400;
271+
throw err;
272+
}
273+
243274
return {
244275
entries,
245276
total: entries.length,
@@ -248,3 +279,22 @@ export async function getMetaDiagnostics(
248279
stats,
249280
};
250281
}
282+
283+
/**
284+
* [#27514] Does the installed-package listing carry this id? `true` ("cannot
285+
* disprove, so do not refuse") when the listing cannot be read at all.
286+
*/
287+
async function packageIsInstalled(host: Pick<MetaDiagnosticsHost, 'getMetaItems'>, packageId: string): Promise<boolean> {
288+
let listed: unknown;
289+
try {
290+
listed = await host.getMetaItems({ type: 'package' });
291+
} catch {
292+
return true;
293+
}
294+
const items = (listed as { items?: unknown } | null | undefined)?.items;
295+
if (!Array.isArray(items)) return true;
296+
return items.some((record) => {
297+
const r = record as { manifest?: { id?: unknown }; id?: unknown } | null | undefined;
298+
return (r?.manifest?.id ?? r?.id) === packageId;
299+
});
300+
}

‎packages/metadata-protocol/src/protocol.diagnostics-store-outage.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,9 @@ describe('[#8924] a caller error the producer classified is refused loudly, neve
306306
const res: any = await p.getMetaDiagnostics({ type: 'fields' });
307307
expect(res.total).toBe(0);
308308
expect(res.scannedTypes).toBe(1);
309-
expect(res.stats.fields).toEqual({ count: 0, locked: 0, packages: [] });
309+
// [#27514] Keyed by the canonical type the plural folds to, not the plural.
310+
expect(Object.keys(res.stats)).toEqual(['field']);
311+
expect(res.stats.field).toEqual({ count: 0, locked: 0, packages: [] });
310312
});
311313

312314
it('a name that is a plural of NOTHING still answers benignly — the second verdict (#8421) lives on the mint door, not here', async () => {

‎packages/rest/src/meta-collection-routes.ts‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888

8989
import { rethrowAuthzStoreUnavailable } from '@objectstack/core';
9090
import type { GetMetaItemsRequest } from '@objectstack/spec/api';
91+
import { truncateClientMessage } from '@objectstack/types';
9192
import { handleRouteError, sendFieldVisibilityFault } from './error-response.js';
9293
import * as metaReadGate from './meta-item-read-gate.js';
9394
import { refuseRepeatedQueryParams } from './query-multiplicity.js';
@@ -189,6 +190,29 @@ export interface MetaCollectionRouteHost {
189190
metaTypeSingular: (typeof RestServer)['metaTypeSingular'];
190191
}
191192

193+
/**
194+
* [#27514] A `GET {metaPath}/diagnostics` refusal of a query value the sweep
195+
* cannot honour: `400 INVALID_REQUEST`, the status and code of the [#8924]
196+
* near-miss type refusal it extends. Thrown, so it leaves through the route's
197+
* one refusal writer ({@link isDiagnosticsRefusal}) with the refusals the
198+
* list door and the sweep throw.
199+
*/
200+
function diagnosticsRefusal(message: string): Error {
201+
return Object.assign(new Error(message), { code: 'INVALID_REQUEST', status: 400 });
202+
}
203+
204+
/**
205+
* [#27514] Is this a classified `400 INVALID_REQUEST` refusal of the
206+
* diagnostics request: {@link diagnosticsRefusal}, the list door's
207+
* unknown-type verdict, or the sweep's near-miss spelling and unknown package?
208+
* Those are the refusals the route answers in its nested envelope; anything
209+
* else keeps `handleRouteError`'s classification.
210+
*/
211+
function isDiagnosticsRefusal(error: unknown): error is Error {
212+
const e = error as { status?: unknown; code?: unknown } | null | undefined;
213+
return error instanceof Error && e?.status === 400 && e?.code === 'INVALID_REQUEST';
214+
}
215+
192216
/**
193217
* Register the `/meta` type list, the whole-store maintenance family and the
194218
* per-type list — the first three groups of
@@ -291,8 +315,19 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop
291315
// value each; the `as string` casts are exactly the
292316
// laundering that kept `tsc` silent about the array arm.
293317
if (refuseRepeatedQueryParams(req, res, ['severity', 'type', 'package'])) return;
294-
const severityParam = (req.query?.severity as string | undefined) ?? 'error';
295-
const severity = severityParam === 'warning' ? 'warning' : 'error';
318+
// [#27514] The sweep reports one of two severities, and any
319+
// other value used to be read as `error` and answered 200:
320+
// a caller asking for something this sweep does not have
321+
// got the error sweep with nothing saying so. An empty value
322+
// states nothing and keeps the default, as an empty `type`
323+
// keeps the full sweep.
324+
const severity = (req.query?.severity as string | undefined) || 'error';
325+
if (severity !== 'error' && severity !== 'warning') {
326+
throw diagnosticsRefusal(
327+
`'${severity}' is not a diagnostics severity. Use 'error' (the default: the entries that `
328+
+ `fail validation) or 'warning' (those, and the entries that carry warnings).`,
329+
);
330+
}
296331
const diagnosticsType = (req.query?.type as string | undefined) || undefined;
297332
// [ADR-0131 D6] No organization: the sweep reads each
298333
// type environment → code, the partition every `/meta`
@@ -305,8 +340,34 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop
305340
// each swept type is a list read, so `all` names no package.
306341
packageId: metaItemPackageBinding(req.query?.package),
307342
});
343+
// [#27514] A named type that is not a metadata type at all
344+
// was swept as one empty type: `200 {scannedTypes: 1}` with
345+
// zero issues, an all-clear over nothing. Refused by the
346+
// list door's own verdict ({@link refuseUnknownMetaListType}:
347+
// the static spelling contract, then the live type set), so
348+
// the two doors cannot disagree about which names are types.
349+
// Asked AFTER the sweep on purpose: the sweep's [#8924]
350+
// refusal of a near-miss spelling (`viewes`) names the
351+
// spellings the caller was reaching for, and it keeps
352+
// answering first.
353+
if (diagnosticsType !== undefined) await this.refuseUnknownMetaListType(p, diagnosticsType);
308354
res.json(result);
309355
} catch (error: any) {
356+
// [#27514] ONE envelope for every refusal this route
357+
// answers. The repeated-parameter refusal above writes the
358+
// ADR-0112 nested `{ error: { code, message } }`, and the
359+
// classified 400s that are THROWN (the severity check above,
360+
// the list door's unknown-type verdict, and the sweep's
361+
// [#8924] near-miss spelling and unknown package) used to
362+
// leave through `handleRouteError` as `{ error: '…', code }`,
363+
// so a client reading `body.error.code` found it on one
364+
// refusal and `undefined` on the next.
365+
if (isDiagnosticsRefusal(error)) {
366+
res.status(400).json({
367+
error: { code: 'INVALID_REQUEST', message: truncateClientMessage(error.message) },
368+
});
369+
return;
370+
}
310371
handleRouteError(res, error);
311372
}
312373
},

0 commit comments

Comments
 (0)