refactor(mcp): убрать мёртвый runtime-слой page-id, оставить бренд PageId
Ревью #516: isPageId/asPageId/isSlugId/asSlugId/SLUG_ID_RE + типы SlugId/PageRef имели НОЛЬ вызовов в монорепе — только барные ре-экспорты в client.ts и кейсы теста. Несущий смысл — только compile-time бренд PageId (минтится as PageId в resolvePageId, единственном узле канонизации; asPageId туда намеренно не звался). Докстринг заявлял «asPageId() guards the untrusted PUBLIC boundary» — но никто не звал: спекулятивный вес. Удалил мёртвые runtime-символы + типы SlugId/PageRef + ре-экспорты + их тест-файл. Оставил тип PageId + касты. Поправил врущий коммент в resolvePageId (бренд — чистый compile-time маркер, гарантия — что этот узел единственный производитель). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -33,18 +33,11 @@ import { TransformsMixin, type ITransformsMixin } from "./client/transforms.js";
|
||||
export type { DocmostMcpConfig, SandboxPut } from "./client/context.js";
|
||||
export { formatDocmostAxiosError, assertFullUuid } from "./client/errors.js";
|
||||
|
||||
// Branded page-identity types + validating constructors/guards (#435): a page's
|
||||
// internal UUID (`PageId`) and public slug (`SlugId`) are distinct nominal types
|
||||
// so the two can't be swapped as bare strings. Re-exported on the package
|
||||
// surface so hosts/tests can validate + brand identities at their boundaries.
|
||||
export type { PageId, SlugId, PageRef } from "./lib/page-id.js";
|
||||
export {
|
||||
asPageId,
|
||||
asSlugId,
|
||||
isPageId,
|
||||
isSlugId,
|
||||
SLUG_ID_RE,
|
||||
} from "./lib/page-id.js";
|
||||
// Branded canonical page-identity type (#435): the internal page UUID is a
|
||||
// distinct nominal type so an unresolved raw/slug string can't be swapped into
|
||||
// the seams that require the canonical id (see lib/page-id.ts). Re-exported on
|
||||
// the package surface for hosts that type against the resolved id.
|
||||
export type { PageId } from "./lib/page-id.js";
|
||||
|
||||
// The full public + shared instance surface of the assembled client. Built by
|
||||
// INTERSECTING each domain mixin's public interface (each DERIVED from its class
|
||||
|
||||
@@ -721,9 +721,10 @@ export abstract class DocmostClientContext {
|
||||
// is minted (#435). The value is validated here — a UUID input by isUuid, a
|
||||
// resolved id as the server's own page.id — so the downstream write path
|
||||
// (withPageLock / mutatePageContent) can require the brand and reject any
|
||||
// unresolved raw id at compile time. The brand is applied by cast, not the
|
||||
// asPageId() validator, to avoid rejecting the fake ids the mock tests feed
|
||||
// a server stub; asPageId() guards the untrusted PUBLIC boundary instead.
|
||||
// unresolved raw id at compile time. The brand is a pure compile-time marker
|
||||
// applied by cast (no runtime guard): the guarantee is that this seam is the
|
||||
// only place a `PageId` is produced, so every branded value went through the
|
||||
// UUID/resolve check above.
|
||||
if (isUuid(pageId)) return pageId as PageId;
|
||||
const cached = this.pageIdCache.get(pageId);
|
||||
if (cached) return cached as PageId;
|
||||
|
||||
@@ -1,81 +1,18 @@
|
||||
/**
|
||||
* Branded page-identity types for the MCP client (incident family #435).
|
||||
* Branded canonical page-identity type for the MCP client (incident family #435).
|
||||
*
|
||||
* A Docmost page has TWO identities that are BOTH plain strings:
|
||||
* - the internal `page.id` — a canonical UUID (the server generates UUIDv7),
|
||||
* - the public `slugId` — a 10-char nanoid over [0-9A-Za-z] used in URLs.
|
||||
* Because both are bare `string`s, they were passed around interchangeably and
|
||||
* silently swapped (e.g. locking/keying a collab doc by the slugId instead of
|
||||
* the UUID — the #260 data-loss). Branding them as distinct nominal types makes
|
||||
* a swap a COMPILE error at the seams that matter, and the validating
|
||||
* constructors reject a malformed / cross-wired identity at runtime too.
|
||||
* A Docmost page has TWO identities that are both plain strings: the internal
|
||||
* `page.id` (a canonical UUID the server generates as UUIDv7) and the public
|
||||
* `slugId` (a 10-char nanoid used in URLs). Because both are bare `string`s they
|
||||
* were passed around interchangeably and silently swapped — e.g. locking/keying a
|
||||
* collab doc by the slugId instead of the UUID (the #260 data-loss).
|
||||
*
|
||||
* These are type-level + format-validation helpers ONLY: a `PageId`/`SlugId` is
|
||||
* still a `string` at runtime (assignable INTO any `string` parameter with no
|
||||
* change), so branding a value flows outward for free; only the few seams that
|
||||
* REQUIRE a canonical id (resolvePageId's result, the per-page lock key, the
|
||||
* collab write entrypoints) demand the brand and so catch an unresolved raw id.
|
||||
* `PageId` brands the CANONICAL id as a distinct nominal type so a raw/unresolved
|
||||
* string cannot flow into the seams that REQUIRE the canonical id (resolvePageId's
|
||||
* result, the per-page lock key, the collab write entrypoints) — those become a
|
||||
* COMPILE error, catching a swap at build time. It is still a `string` at runtime
|
||||
* (assignable INTO any `string` parameter unchanged), so branding flows outward
|
||||
* for free; the brand is minted at the single canonicalization seam
|
||||
* (`resolvePageId`, via `as PageId`).
|
||||
*/
|
||||
import { UUID_RE } from "./page-lock.js";
|
||||
|
||||
/** The internal canonical page id (`page.id`), a canonical UUID. */
|
||||
export type PageId = string & { readonly __brand: "PageId" };
|
||||
|
||||
/** The public page slug id (`slugId`), a 10-char nanoid used in URLs. */
|
||||
export type SlugId = string & { readonly __brand: "SlugId" };
|
||||
|
||||
/**
|
||||
* A page REFERENCE an agent may supply: EITHER the canonical UUID `PageId` or
|
||||
* the public `SlugId`. The server's page lookup accepts both (a non-UUID is
|
||||
* matched as a slugId), so the public read/tool boundary legitimately takes
|
||||
* either form; only the resolved-to-canonical value is a strict `PageId`.
|
||||
*/
|
||||
export type PageRef = PageId | SlugId;
|
||||
|
||||
// A slugId is exactly 10 chars from the nanoid alphabet the server uses for
|
||||
// generateSlugId: [0-9A-Za-z] (see apps/server .../common/helpers/nanoid.utils).
|
||||
// Disjoint from a UUID (which is 36 chars WITH dashes), so the two formats can
|
||||
// never be confused for one another.
|
||||
export const SLUG_ID_RE = /^[0-9A-Za-z]{10}$/;
|
||||
|
||||
/** Type guard: is `value` a canonical page UUID (`PageId`)? */
|
||||
export function isPageId(value: unknown): value is PageId {
|
||||
return typeof value === "string" && UUID_RE.test(value);
|
||||
}
|
||||
|
||||
/** Type guard: is `value` a public slug id (`SlugId`)? */
|
||||
export function isSlugId(value: unknown): value is SlugId {
|
||||
return typeof value === "string" && SLUG_ID_RE.test(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate + brand a canonical page id. Throws an actionable error (BEFORE any
|
||||
* network use) when `value` is not a canonical UUID, so a slugId or garbage
|
||||
* cross-wired where the canonical id is required fails fast and loud instead of
|
||||
* silently splitting a lock/doc key (#260). `label` names the offending param.
|
||||
*/
|
||||
export function asPageId(value: string, label = "pageId"): PageId {
|
||||
if (!isPageId(value)) {
|
||||
throw new Error(
|
||||
`${label}: expected a canonical page UUID (36 chars, e.g. ` +
|
||||
`019f499a-9f8c-7d68-b7be-ce100d7c6c56), got '${value}'. A slugId or ` +
|
||||
`other identity must be resolved to the page UUID first.`,
|
||||
);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate + brand a public slug id. Throws when `value` is not the 10-char
|
||||
* slugId format, so a UUID or garbage cross-wired where a slugId is required is
|
||||
* rejected at the boundary.
|
||||
*/
|
||||
export function asSlugId(value: string, label = "slugId"): SlugId {
|
||||
if (!isSlugId(value)) {
|
||||
throw new Error(
|
||||
`${label}: expected a 10-char page slugId (e.g. 'aB3xQ7kR2p'), got ` +
|
||||
`'${value}'.`,
|
||||
);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
@@ -1,57 +0,0 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
asPageId,
|
||||
asSlugId,
|
||||
isPageId,
|
||||
isSlugId,
|
||||
SLUG_ID_RE,
|
||||
} from "../../build/lib/page-id.js";
|
||||
|
||||
// A real canonical page UUID (UUIDv7-shaped) and a real 10-char slugId.
|
||||
const UUID = "019f499a-9f8c-7d68-b7be-ce100d7c6c56";
|
||||
const SLUG = "aB3xQ7kR2p";
|
||||
|
||||
test("isPageId accepts a canonical UUID and rejects a slugId / garbage", () => {
|
||||
assert.equal(isPageId(UUID), true);
|
||||
assert.equal(isPageId(SLUG), false);
|
||||
assert.equal(isPageId("not-a-uuid"), false);
|
||||
assert.equal(isPageId(""), false);
|
||||
assert.equal(isPageId(undefined), false);
|
||||
assert.equal(isPageId(123), false);
|
||||
});
|
||||
|
||||
test("isSlugId accepts the 10-char slug format and rejects a UUID / wrong length", () => {
|
||||
assert.equal(isSlugId(SLUG), true);
|
||||
assert.equal(isSlugId(UUID), false);
|
||||
assert.equal(isSlugId("short"), false); // < 10 chars
|
||||
assert.equal(isSlugId("aB3xQ7kR2pX"), false); // 11 chars
|
||||
assert.equal(isSlugId("aB3xQ7kR2!"), false); // illegal char
|
||||
assert.equal(isSlugId(""), false);
|
||||
assert.equal(isSlugId(null), false);
|
||||
});
|
||||
|
||||
test("SLUG_ID_RE is anchored (no substring match inside a longer string)", () => {
|
||||
assert.equal(SLUG_ID_RE.test(`prefix-${SLUG}`), false);
|
||||
assert.equal(SLUG_ID_RE.test(`${SLUG}-suffix`), false);
|
||||
});
|
||||
|
||||
test("asPageId returns the branded value unchanged for a valid UUID", () => {
|
||||
assert.equal(asPageId(UUID), UUID);
|
||||
});
|
||||
|
||||
test("asPageId throws an actionable error for a slugId (the #260 swap)", () => {
|
||||
assert.throws(() => asPageId(SLUG), /canonical page UUID/);
|
||||
// The offending value and a custom label are surfaced for self-correction.
|
||||
assert.throws(() => asPageId("garbage", "targetPageId"), /targetPageId/);
|
||||
assert.throws(() => asPageId("garbage", "targetPageId"), /garbage/);
|
||||
});
|
||||
|
||||
test("asSlugId returns the branded value unchanged for a valid slugId", () => {
|
||||
assert.equal(asSlugId(SLUG), SLUG);
|
||||
});
|
||||
|
||||
test("asSlugId throws for a UUID cross-wired where a slugId is required", () => {
|
||||
assert.throws(() => asSlugId(UUID), /10-char page slugId/);
|
||||
assert.throws(() => asSlugId("nope"), /nope/);
|
||||
});
|
||||
Reference in New Issue
Block a user