Skip to content
32 changes: 32 additions & 0 deletions .changeset/mcp-diagnose-empty-read-tsdoc-correction.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
"@objectstack/mcp": patch
---

docs(mcp): `diagnoseEmptyRead` 的 TSDoc 更正一句被证伪的事实 (#6724)

`packages/mcp/src/mcp-server-runtime.ts` 里 `diagnoseEmptyRead` 的 TSDoc(#6055
由 PR #6051 落地)为"在空答案之后再跑一次仅取结论的探针,而不是把
`getObject` 换成 `getDiagnosed('object', name)`"这个设计选择给出了两条理由。
其中一条是事实陈述,而它是**错的**:

> `MetadataFacade.getObject`(objectql)返回 `registry.getObject(name)` —— a
> different shape from its own `get()`,因此等价关系在一般情况下不成立。

`SchemaRegistry.getItem` 对 `'object'` / `'objects'` 类型直接特判回
`getObject`,所以 facade 的 `get('object', n)` 走的是同一次查找;其后的
`item?.content ?? item` 解包是空操作 —— 合并后的 `ServiceObject` 根本没有
`content` 键。实测:命中时两个成员交回**同一个对象引用**,未命中时双方都是
`undefined`。三个已发布实现由 `packages/objectql/src/
metadata-service-getobject-equivalence.test.ts`(PR #6839)钉住,契约侧的
`IMetadataService.getObject` 自 PR #6723(#6505)起也写明了这条等价关系。

同一句话在 `mcp-server-runtime.metadata-outage.test.ts` 里被复述过一次,一并
更正。

仍然成立的那半条理由被保留:`getObject` 是 `IMetadataService` 自己的成员,
#6055 当时它并**没有**被文档化的等价关系,在消费端擅自假定一条正是 Prime
Directive #12 禁止的私有方言 —— 所以解析器当初没有被换掉。

**纯注释,零行为变化。** 这次更正**不**主张把解析器换成
`getDiagnosed('object', name)`:那是一次独立的判断,由接手的人按其自身利弊
去做,本次改动既不作出也不预设。
15 changes: 12 additions & 3 deletions packages/mcp/src/mcp-server-runtime.metadata-outage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,18 @@ describe('object_schema resource — a metadata outage is not "Object not found"
const svc = holding(OBJECT);
await buildObjectSchemaResource(svc, 'acct');

// `getObject` stays the resolver (it is its own contract member, and
// `MetadataFacade.getObject` is NOT `get('object', name)`); the diagnosed
// read is a verdict probe on the MISS path only.
// `getObject` stays the resolver: it is its own contract member, and #6055
// declined to presume an equivalence the contract did not then document
// (Prime Directive #12).
//
// [#6724] The parenthetical that used to sit here claimed that the facade's
// `getObject` is NOT `get('object', name)`. That was false: the two hand
// back the identical object on every implementation this repo ships (pinned by
// `packages/objectql/src/metadata-service-getobject-equivalence.test.ts`,
// PR #6839; documented on `IMetadataService.getObject` by PR #6723).
// Whether the resolver should change is a separate call this correction
// does not make. What this case pins is unchanged either way: the
// diagnosed read is a verdict probe on the MISS path only.
expect((svc as AnyRecord).getObject).toHaveBeenCalledWith('acct');
expect((svc as AnyRecord).getDiagnosed).not.toHaveBeenCalled();
});
Expand Down
40 changes: 31 additions & 9 deletions packages/mcp/src/mcp-server-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,34 @@ async function diagnosedGet(
* `object_schema` resource, whose resolver is `getObject(name)`.
*
* Deliberately a **verdict-only probe run after the empty answer**, rather than
* swapping `getObject` out for `getDiagnosed('object', name)`. `getObject` is
* its own member of `IMetadataService` with no documented equivalence to
* `get('object', name)`, and the equivalence does not hold in general:
* `MetadataManager.getObject` delegates to `get('object', name)`, but
* `MetadataFacade.getObject` (objectql) returns `registry.getObject(name)` — a
* different shape from its own `get()`. Presuming the equivalence at a consumer
* is exactly the private dialect Prime Directive #12 forbids, so the resolver
* is left untouched and only the *question* "could this answer be trusted as
* complete?" is asked of the contract member that is declared to answer it.
* swapping `getObject` out for `getDiagnosed('object', name)`. The ground is
* the *contract*, not the runtime: `getObject` is its own member of
* `IMetadataService`, and at the time of #6055 that member carried **no
* documented equivalence** to `get('object', name)`. Presuming an undocumented
* equivalence at a consumer is exactly the private dialect Prime Directive #12
* forbids, so the resolver is left untouched and only the *question* "could
* this answer be trusted as complete?" is asked of the contract member that is
* declared to answer it.
*
* [#6724] This TSDoc used to offer a second, factual ground — that the
* equivalence "does not hold in general", `MetadataFacade.getObject` (objectql)
* returning "a different shape from its own `get()`". That claim is **false**,
* and it was asserted rather than measured. `SchemaRegistry.getItem`
* special-cases `'object'`/`'objects'` straight back to `getObject`, so the
* facade's `get('object', n)` resolves through the very same lookup, and the
* `item?.content ?? item` unwrap that follows is a no-op — a merged
* `ServiceObject` has no `content` key. Measured: the two members hand back the
* **identical object reference** on a hit, and both answer `undefined` on a
* miss. All three shipped implementations are pinned that way by
* `packages/objectql/src/metadata-service-getobject-equivalence.test.ts`
* (PR #6839 for #6745), and `IMetadataService.getObject` has documented the
* equivalence since PR #6723 (#6505) — so the "no documented equivalence" half
* above is a fact about #6055's repo, not today's.
*
* Correcting the record does not decide the design question, and this note
* deliberately does not make that call: whether the resolver should become
* `getDiagnosed('object', name)` and shed the extra miss-path read is a
* separate judgement, to be made on its own merits by whoever takes it up.
*
* Consequences of that choice, both acceptable and both deliberate:
* - one extra read on the MISS path only (never on a hit, never on success);
Expand All @@ -174,6 +193,9 @@ async function diagnosedGet(
* object that is genuinely absent. That is the conservative direction — it
* withholds, it never admits — and no such host exists today
* (`MetadataManager` is the only `getDiagnosed` implementation on `main`).
* Since PR #6723 the contract also rules such a host out by declaration, so
* this reads as residual risk against a contract violation, not as a live
* divergence anyone can point at.
*/
async function diagnoseEmptyRead(
metadataService: IMetadataService,
Expand Down
Loading