feat(ai,mcp): идентичность прогона реиндекса + брендированные PageId/SlugId (#495 item 14)

Аспект A — идентичность прогона реиндекса эмбеддингов. У статуса/поллинга реиндекса не было идентичности КОНКРЕТНОГО прогона, из-за чего класс багов «это тот же прогон или новый?» чинили дважды. Теперь каждый прогон получает свой runId (crypto.randomUUID в start()), он хранится в Redis- хэше рядом с total/done/startedAt и возвращается в ReindexProgress. Эндпоинт статуса (getMasked -> MaskedAiSettings) отдаёт runId и reindexStartedAt. Клиент кеит поллинг на (runId, startedAt): смена runId = НОВЫЙ прогон (сбрасываем залатанное per-run состояние поллинга), тот же runId = тот же прогон. Всё best-effort/косметика как и остальной код прогресса: пустой/отсутствующий runId деградирует мягко и никогда не ломает реиндекс. Аспект B — брендированные PageId/SlugId в MCP-клиенте + валидация формата в серверных DTO (семейство инцидентов #435: двойная идентичность страницы — внутренний id и slugId — гонялась как голая строка и молча путалась). - packages/mcp/src/lib/page-id.ts: номинальные типы PageId/SlugId + валидирующие конструкторы asPageId/asSlugId и гварды isPageId/isSlugId (формат UUID и 10-символьного slugId). PageId протянут через единственный узел канонизации и записи: resolvePageId() теперь возвращает PageId, а ключ per-page лока (withPageLock) и точки записи в collab (mutatePageContent/replacePageContent/ updatePageContentRealtime) требуют бренд — сырой slugId/непроверенный id больше не проходит проверку типов (инвариант #260 «resolve-then-lock» теперь на уровне компилятора). Публичный вход методов остаётся строкой (это legitimно UUID ИЛИ slugId), брендируется каноническое значение. - apps/server core/page/dto: PageIdDto.pageId получает валидацию формата (@Matches, UUID или 10-символьный slugId), так что кривая/подменённая идентичность отклоняется на границе, а не падает в repo голой строкой. Тесты (все зелёные, с mutation-verify каждого): - server: getMasked отдаёт runId/reindexStartedAt; стор пишет/читает runId и мягко деградирует его до ''; DTO отклоняет кривой pageId/slugId и принимает валидный. - client: чистый хелпер reindexRunKey/isNewReindexRun (кеинг поллинга на runId). - mcp: конструкторы/гварды PageId/SlugId (валидация формата). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 19:33:18 +03:00
parent bf087c0a10
commit d50295dc2a
17 changed files with 555 additions and 17 deletions
@@ -6,6 +6,8 @@ import {
  nextReindexPollInterval,
  isReindexComplete,
  isReindexButtonLoading,
+  reindexRunKey,
+  isNewReindexRun,
 } from './ai-provider-settings';

 describe('resolveCardStatus', () => {
@@ -221,6 +223,128 @@ describe('isReindexComplete', () => {
  });
 });

+describe('reindexRunKey', () => {
+  it('is null when the status carries no run identity', () => {
+    expect(reindexRunKey(undefined)).toBeNull();
+    expect(
+      reindexRunKey({ reindexing: false, indexedPages: 5, totalPages: 5 }),
+    ).toBeNull();
+  });
+
+  it('is null for a legacy/degraded record with an empty runId', () => {
+    // The server sends runId='' for a record written before the field existed;
+    // the client must treat that as "no identity" (fall back to prior behaviour).
+    expect(
+      reindexRunKey({
+        reindexing: true,
+        indexedPages: 0,
+        totalPages: 10,
+        runId: '',
+        reindexStartedAt: 1000,
+      }),
+    ).toBeNull();
+  });
+
+  it('folds runId and startedAt into one stable key', () => {
+    expect(
+      reindexRunKey({
+        reindexing: true,
+        indexedPages: 0,
+        totalPages: 10,
+        runId: 'run-a',
+        reindexStartedAt: 1000,
+      }),
+    ).toBe('run-a:1000');
+  });
+
+  it('changes when the runId changes for the same startedAt', () => {
+    const a = reindexRunKey({
+      reindexing: true,
+      indexedPages: 0,
+      totalPages: 10,
+      runId: 'run-a',
+      reindexStartedAt: 1000,
+    });
+    const b = reindexRunKey({
+      reindexing: true,
+      indexedPages: 0,
+      totalPages: 10,
+      runId: 'run-b',
+      reindexStartedAt: 1000,
+    });
+    expect(a).not.toBe(b);
+  });
+
+  it('changes when the same runId restarts at a new startedAt', () => {
+    const a = reindexRunKey({
+      reindexing: true,
+      indexedPages: 0,
+      totalPages: 10,
+      runId: 'run-a',
+      reindexStartedAt: 1000,
+    });
+    const b = reindexRunKey({
+      reindexing: true,
+      indexedPages: 0,
+      totalPages: 10,
+      runId: 'run-a',
+      reindexStartedAt: 2000,
+    });
+    expect(a).not.toBe(b);
+  });
+});
+
+describe('isNewReindexRun (poll keying on runId)', () => {
+  // Derive the status shape from the helper itself so the test needs no export
+  // of the component-internal ReindexStatus type.
+  type ReindexStatusLike = NonNullable<Parameters<typeof reindexRunKey>[0]>;
+  const run = (runId: string, startedAt: number): ReindexStatusLike => ({
+    reindexing: true,
+    indexedPages: 0,
+    totalPages: 10,
+    runId,
+    reindexStartedAt: startedAt,
+  });
+
+  it('first identity after none latched is a NEW run', () => {
+    expect(isNewReindexRun(null, run('run-a', 1000))).toBe(true);
+  });
+
+  it('the SAME identity is not a new run (same run being watched)', () => {
+    const key = reindexRunKey(run('run-a', 1000));
+    expect(isNewReindexRun(key, run('run-a', 1000))).toBe(false);
+  });
+
+  it('a DIFFERENT runId is a new run (reset per-run poll state)', () => {
+    const key = reindexRunKey(run('run-a', 1000));
+    expect(isNewReindexRun(key, run('run-b', 1000))).toBe(true);
+  });
+
+  it('an identity-less poll (no runId / cleared record) is never a new run', () => {
+    const key = reindexRunKey(run('run-a', 1000));
+    expect(
+      isNewReindexRun(key, {
+        reindexing: false,
+        indexedPages: 10,
+        totalPages: 10,
+      }),
+    ).toBe(false);
+  });
+
+  it('a legacy empty-runId poll does not spuriously reset a latched run', () => {
+    const key = reindexRunKey(run('run-a', 1000));
+    expect(
+      isNewReindexRun(key, {
+        reindexing: true,
+        indexedPages: 3,
+        totalPages: 10,
+        runId: '',
+        reindexStartedAt: 1000,
+      }),
+    ).toBe(false);
+  });
+});
+
 describe('isReindexButtonLoading', () => {
  it('loads while the POST mutation is pending', () => {
    expect(
@@ -173,9 +173,43 @@ export function resolveKeyField(
 // Subset of the status payload that drives the reindex poll decisions.
 type ReindexStatus = Pick<
  IAiSettings,
-  "reindexing" | "indexedPages" | "totalPages"
+  "reindexing" | "indexedPages" | "totalPages" | "runId" | "reindexStartedAt"
 >;

+/**
+ * A stable per-RUN key for the reindex poll: `runId:startedAt`, or `null` when
+ * the status carries no run identity (no active run, or a legacy/degraded
+ * server record with an empty runId). Two polls of the SAME run share a key; a
+ * new run mints a fresh runId and so a different key.
+ *
+ * This is the single place the client turns the server's run identity into the
+ * value it keys on — it removes the "is this the same run I've been watching or
+ * a brand-new one?" ambiguity that made a class of reindex-status bugs (a stale
+ * pre-reindex snapshot vs a fresh run) get fixed twice (#262). `startedAt` is
+ * folded in so a run that somehow reuses a runId but restarted is still new.
+ */
+export function reindexRunKey(status: ReindexStatus | undefined): string | null {
+  const runId = status?.runId;
+  if (!runId) return null;
+  return `${runId}:${status?.reindexStartedAt ?? ""}`;
+}
+
+/**
+ * Decide whether the latest poll represents a NEW reindex run relative to the
+ * run key the client last latched (`prevKey`, `null` if none yet). True only
+ * when the status carries an identity AND it differs from the latched one — the
+ * signal to reset any per-run poll state (the "seen active" latch / progress the
+ * UI held). The same identity (or no identity) is NOT a new run, so an unchanged
+ * or identity-less poll never resets mid-run.
+ */
+export function isNewReindexRun(
+  prevKey: string | null,
+  status: ReindexStatus | undefined,
+): boolean {
+  const key = reindexRunKey(status);
+  return key !== null && key !== prevKey;
+}
+
 /**
 * Decide the TanStack Query `refetchInterval` while a reindex may be running.
 * Returns the poll interval (ms) to keep polling, or `false` to stop.
@@ -320,6 +354,13 @@ export default function AiProviderSettings() {
  // counter at 0 until a manual reload. A ref (not state) because it must not
  // trigger a render and is only ever read where `reindexing` is already false.
  const reindexSeenActiveRef = useRef(false);
+  // The run identity (runId:startedAt) the current poll window is keyed on. When
+  // a poll reports a DIFFERENT runId the server has started a NEW run, so we
+  // re-latch to it and reset `reindexSeenActiveRef` — a fresh run must never
+  // inherit the previous run's "seen active"/completion state (which would stop
+  // polling immediately or read the old run's counters as this run's). null =
+  // no run keyed yet (steady state, or a legacy record without a runId).
+  const reindexRunKeyRef = useRef<string | null>(null);

  // Only admins may read the (masked) AI settings; the server enforces this too.
  const { data: settings, isLoading } = useAiSettingsQuery(isAdmin, (query) =>
@@ -336,6 +377,14 @@ export default function AiProviderSettings() {
  // unmount because the deadline state goes away with the component.
  useEffect(() => {
    if (reindexDeadline === null) return;
+    // Key the poll on the run identity: if this poll carries a runId different
+    // from the one we latched, the server started a NEW run, so adopt it and
+    // drop the per-run "seen active" latch (a fresh run must not inherit the
+    // previous run's completion state). Same runId => same run, leave it alone.
+    if (isNewReindexRun(reindexRunKeyRef.current, settings)) {
+      reindexRunKeyRef.current = reindexRunKey(settings);
+      reindexSeenActiveRef.current = false;
+    }
    // Latch "we have seen the active run" the moment a poll reports it, so the
    // completion check below (and the refetchInterval's) only fires once the run
    // has genuinely started — never on the stale pre-reindex snapshot.
@@ -1220,6 +1269,10 @@ export default function AiProviderSettings() {
                  // immediately.
                  onSuccess: () => {
                    reindexSeenActiveRef.current = false;
+                    // Forget the previous run's identity so the first poll of
+                    // this window (carrying the new run's runId) is recognized
+                    // as a new run and keyed afresh.
+                    reindexRunKeyRef.current = null;
                    setReindexDeadline(Date.now() + REINDEX_POLL_CAP_MS);
                  },
                })
@@ -51,6 +51,14 @@ export interface IAiSettings {
  // True while a full workspace reindex is actively running; the counts above
  // then reflect the live run progress (done climbs 0 -> total).
  reindexing?: boolean;
+  // Identity of the ACTIVE reindex run (present only while `reindexing`). The
+  // poll keys on `runId`: a changed value means a NEW run (reset the per-run
+  // poll state the UI latched), the same value is the run already being watched.
+  // Absent/empty ('') => no identity available; the client keeps prior behaviour.
+  runId?: string;
+  // Epoch-ms the active run started; paired with `runId` so a restart with a
+  // recycled id is still detected as a new run.
+  reindexStartedAt?: number;
 }

 // Update payload. Key semantics (same for `apiKey` and `embeddingApiKey`):
@@ -0,0 +1,49 @@
+import 'reflect-metadata';
+import { plainToInstance } from 'class-transformer';
+import { validate } from 'class-validator';
+import { PageIdDto } from './page.dto';
+
+// #435: PageIdDto.pageId carries a page's DOUBLE identity (internal UUID OR
+// public 10-char slugId), both as bare strings. The DTO must accept exactly
+// those two FORMATS and reject a malformed / swapped identity at the boundary.
+async function pageIdErrors(pageId: unknown) {
+  const dto = plainToInstance(PageIdDto, { pageId });
+  const errors = await validate(dto as object);
+  return errors.some((e) => e.property === 'pageId');
+}
+
+const UUID = '019f499a-9f8c-7d68-b7be-ce100d7c6c56';
+const SLUG = 'aB3xQ7kR2p';
+
+describe('PageIdDto pageId format validation', () => {
+  it('accepts a canonical page UUID', async () => {
+    expect(await pageIdErrors(UUID)).toBe(false);
+  });
+
+  it('accepts a 10-char slugId', async () => {
+    expect(await pageIdErrors(SLUG)).toBe(false);
+  });
+
+  it('rejects a truncated / wrong-length slug', async () => {
+    expect(await pageIdErrors('aB3xQ7kR2')).toBe(true); // 9 chars
+    expect(await pageIdErrors('aB3xQ7kR2pX')).toBe(true); // 11 chars
+  });
+
+  it('rejects a slug with an illegal character', async () => {
+    expect(await pageIdErrors('aB3xQ7kR2!')).toBe(true);
+  });
+
+  it('rejects a full URL / path-shaped identity (not the bare id)', async () => {
+    expect(await pageIdErrors(`my-page-title-${SLUG}`)).toBe(true);
+    expect(await pageIdErrors(`https://x/p/${SLUG}`)).toBe(true);
+  });
+
+  it('rejects a malformed UUID', async () => {
+    expect(await pageIdErrors('019f499a-9f8c-7d68-b7be')).toBe(true);
+    expect(await pageIdErrors('not-a-uuid-at-all-really')).toBe(true);
+  });
+
+  it('rejects an empty string', async () => {
+    expect(await pageIdErrors('')).toBe(true);
+  });
+});
@@ -0,0 +1,39 @@
+import { applyDecorators } from '@nestjs/common';
+import { Matches, ValidationOptions } from 'class-validator';
+
+/**
+ * A page identity at the API boundary is EITHER the internal page UUID or the
+ * public 10-char slugId (page.repo.findById matches a non-UUID input as a
+ * slugId). Both arrive as bare strings, which is exactly how the two got swapped
+ * silently (incident family #435). This regex pins the two accepted FORMATS so a
+ * malformed / cross-wired identity (a truncated slug, a full URL, an email, an
+ * id from another entity kind that isn't even shaped like either) is rejected at
+ * the boundary instead of falling through to a confusing 404.
+ *
+ *   - UUID:    canonical 8-4-4-4-12 hex (version-agnostic — page ids are UUIDv7,
+ *              so only the shape/length is enforced, matching the MCP's UUID_RE
+ *              and the server's isValidUUID acceptance).
+ *   - slugId:  exactly 10 chars over [0-9A-Za-z] (nanoid `generateSlugId`).
+ *
+ * The two are disjoint (a UUID is 36 chars WITH dashes, a slugId 10 chars
+ * WITHOUT), so a value can only satisfy one branch.
+ */
+export const PAGE_ID_OR_SLUG_ID_REGEX =
+  /^(?:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|[0-9A-Za-z]{10})$/i;
+
+/**
+ * Validate that a DTO string field is a well-formed page identity (page UUID OR
+ * 10-char slugId). Composed decorator so the same format rule is applied
+ * consistently wherever a DTO accepts a `pageId` that may be either form.
+ */
+export function IsPageIdOrSlugId(
+  validationOptions?: ValidationOptions,
+): PropertyDecorator {
+  return applyDecorators(
+    Matches(PAGE_ID_OR_SLUG_ID_REGEX, {
+      message:
+        'pageId must be a page UUID or a 10-character slugId',
+      ...validationOptions,
+    }),
+  );
+}
@@ -9,10 +9,16 @@ import {
 import { Transform } from 'class-transformer';

 import { ContentFormat } from './create-page.dto';
+import { IsPageIdOrSlugId } from './page-identity.validator';

 export class PageIdDto {
  @IsString()
  @IsNotEmpty()
+  // Format-validate the double identity (#435): accept only a page UUID or a
+  // 10-char slugId so a malformed / swapped identity is rejected at the boundary
+  // rather than passed to the repo as a bare string. Base for PageInfoDto,
+  // DeletePageDto, BacklinksListDto, AddLabelsDto/RemoveLabelDto, etc.
+  @IsPageIdOrSlugId()
  pageId: string;
 }

@@ -99,10 +99,12 @@ describe('AiSettingsService.getMasked reindex progress', () => {
    // actually pins the progress.total branch rather than coincidentally
    // matching the DB fallback. With fix #1 the two sources agree in practice,
    // but getMasked must still return progress.total when a record is active.
+    const startedAt = Date.now();
    reindexProgress.get.mockResolvedValue({
      total: 500,
      done: 120,
-      startedAt: Date.now(),
+      startedAt,
+      runId: 'run-abc',
    });

    const masked = await service.getMasked(WORKSPACE_ID);
@@ -110,6 +112,10 @@ describe('AiSettingsService.getMasked reindex progress', () => {
    expect(masked.indexedPages).toBe(120); // progress.done, not DB 478
    expect(masked.totalPages).toBe(500); // progress.total, not DB 478
    expect(masked.reindexing).toBe(true);
+    // The status payload must carry the run identity so the client can key its
+    // poll on it (a changed runId => a NEW run).
+    expect(masked.runId).toBe('run-abc');
+    expect(masked.reindexStartedAt).toBe(startedAt);
  });

  it('falls back to countIndexedPages when no reindex is active', async () => {
@@ -121,6 +127,10 @@ describe('AiSettingsService.getMasked reindex progress', () => {
    expect(masked.indexedPages).toBe(478);
    expect(masked.totalPages).toBe(478);
    expect(masked.reindexing).toBe(false);
+    // No active run -> no run identity surfaced (the client keeps its prior
+    // steady-state behaviour).
+    expect(masked.runId).toBeUndefined();
+    expect(masked.reindexStartedAt).toBeUndefined();
  });
 });

@@ -371,6 +371,12 @@ export class AiSettingsService {
      totalPages,
      // Optional hint for the client: a reindex run is currently in progress.
      reindexing: progress != null,
+      // Per-run identity so the client can key its poll on a stable run id and
+      // reset its per-run state when a NEW run starts. Present only while a run
+      // is active; `runId` may be '' for a legacy/degraded record (the client
+      // treats that as "no identity").
+      runId: progress?.runId,
+      reindexStartedAt: progress?.startedAt,
    };
  }

@@ -149,4 +149,14 @@ export interface MaskedAiSettings {
  // True while a full workspace reindex is actively running (the counts above
  // then reflect the live run progress rather than the steady-state DB count).
  reindexing?: boolean;
+  // Identity of the ACTIVE reindex run (present only while `reindexing`). The
+  // client keys its poll on `runId`: a changed value means a NEW run (reset the
+  // per-run poll state it latched), the same value means the run it is already
+  // watching — removing the "same run or a fresh one?" ambiguity a stale
+  // pre-reindex snapshot otherwise causes. Absent/empty degrades gracefully.
+  runId?: string;
+  // Epoch-ms the active run started (present only while `reindexing`). Paired
+  // with `runId` so a run that restarts with the same (recycled) id is still
+  // seen as new.
+  reindexStartedAt?: number;
 }
@@ -48,19 +48,38 @@ describe('EmbeddingReindexProgressService', () => {
  }

  describe('get', () => {
-    it('maps a valid hash to a ReindexProgress object', async () => {
+    it('maps a valid hash to a ReindexProgress object (incl. the run identity)', async () => {
      const { redis, hgetall } = makeRedis();
-      hgetall.mockResolvedValue({ total: '478', done: '120', startedAt: '1000' });
+      hgetall.mockResolvedValue({
+        total: '478',
+        done: '120',
+        startedAt: '1000',
+        runId: 'run-xyz',
+      });
      const service = makeService(redis);

      await expect(service.get(WORKSPACE_ID)).resolves.toEqual({
        total: 478,
        done: 120,
        startedAt: 1000,
+        runId: 'run-xyz',
      });
      expect(hgetall).toHaveBeenCalledWith(KEY);
    });

+    it('degrades a missing runId to an empty string (legacy/partial record)', async () => {
+      const { redis, hgetall } = makeRedis();
+      // A record written before runId existed: get() must still succeed and
+      // report runId='' so the client treats it as "no identity", never breaks.
+      hgetall.mockResolvedValue({ total: '10', done: '3', startedAt: '5' });
+      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toEqual({
+        total: 10,
+        done: 3,
+        startedAt: 5,
+        runId: '',
+      });
+    });
+
    it('returns null for an empty hash (no record)', async () => {
      const { redis, hgetall } = makeRedis();
      hgetall.mockResolvedValue({});
@@ -87,11 +106,17 @@ describe('EmbeddingReindexProgressService', () => {

    it('coerces a non-finite startedAt to 0', async () => {
      const { redis, hgetall } = makeRedis();
-      hgetall.mockResolvedValue({ total: '10', done: '2', startedAt: 'nope' });
+      hgetall.mockResolvedValue({
+        total: '10',
+        done: '2',
+        startedAt: 'nope',
+        runId: 'run-1',
+      });
      await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toEqual({
        total: 10,
        done: 2,
        startedAt: 0,
+        runId: 'run-1',
      });
    });

@@ -115,6 +140,21 @@ describe('EmbeddingReindexProgressService', () => {
      expect(multiObj.exec).toHaveBeenCalledTimes(1);
    });

+    it('mints a fresh non-empty runId into the record on each start', async () => {
+      const { redis, multiObj } = makeRedis();
+      const service = makeService(redis);
+      await service.start(WORKSPACE_ID, 1);
+      await service.start(WORKSPACE_ID, 1);
+
+      const firstRunId = multiObj.hset.mock.calls[0][1].runId;
+      const secondRunId = multiObj.hset.mock.calls[1][1].runId;
+      expect(typeof firstRunId).toBe('string');
+      expect(firstRunId).not.toBe('');
+      // Each run gets its OWN identity so the client can tell a re-trigger apart
+      // from the run it is already watching.
+      expect(secondRunId).not.toBe(firstRunId);
+    });
+
    it('defaults the expire TTL to the full 1h record TTL', async () => {
      const { redis, multiObj } = makeRedis();
      await makeService(redis).start(WORKSPACE_ID, 478);
@@ -1,17 +1,28 @@
 import { Injectable, Logger } from '@nestjs/common';
 import { RedisService } from '@nestjs-labs/nestjs-ioredis';
+import { randomUUID } from 'node:crypto';
 import type { Redis } from 'ioredis';

 /**
 * Live progress of an in-flight workspace embeddings reindex run.
 * `total` is the number of pages the run will process, `done` how many it has
 * already processed (success OR handled failure), `startedAt` the epoch-ms the
- * record was created.
+ * record was created, and `runId` a per-run identity minted at `start()`.
+ *
+ * `runId` gives each reindex run a stable identity so a poller can tell "same
+ * run I've been watching" from "a NEW run started" WITHOUT guessing from the
+ * progress counters (the ambiguity that a stale pre-reindex snapshot vs a fresh
+ * run otherwise causes — the bug class fixed twice under #262). It is best-
+ * effort like the rest of this record: a record written before this field
+ * existed (or a Redis hiccup) yields an empty `runId`, which the client must
+ * treat as "no identity available" and degrade to its prior behaviour, never
+ * break.
 */
 export interface ReindexProgress {
  total: number;
  done: number;
  startedAt: number;
+  runId: string;
 }

 /** Redis key namespace for the per-workspace reindex-progress record. */
@@ -86,12 +97,18 @@ export class EmbeddingReindexProgressService {
  ): Promise<void> {
    const key = this.key(workspaceId);
    try {
+      // A fresh identity per run so the client poll can key on it: a changed
+      // runId means a genuinely NEW run (reset any latched per-run poll state),
+      // the same runId means the run the client is already watching. Best-effort
+      // like the counters — never surfaced to the user, only used to disambiguate.
+      const runId = randomUUID();
      await this.redis
        .multi()
        .hset(key, {
          total: String(total),
          done: '0',
          startedAt: String(Date.now()),
+          runId,
        })
        .expire(key, ttlSeconds)
        .exec();
@@ -150,7 +167,15 @@ export class EmbeddingReindexProgressService {
      const done = Number(data.done);
      const startedAt = Number(data.startedAt);
      if (!Number.isFinite(total) || !Number.isFinite(done)) return null;
-      return { total, done, startedAt: Number.isFinite(startedAt) ? startedAt : 0 };
+      // `runId` degrades gracefully: a pre-existing record (written before this
+      // field) or a stripped value reads as '' — the client treats that as "no
+      // identity" and keeps its prior behaviour rather than breaking the poll.
+      return {
+        total,
+        done,
+        startedAt: Number.isFinite(startedAt) ? startedAt : 0,
+        runId: typeof data.runId === 'string' ? data.runId : '',
+      };
    } catch (err) {
      this.logger.warn(
        `reindex-progress read failed for workspace ${workspaceId}; ` +
@@ -33,6 +33,19 @@ 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";
+
 // The full public + shared instance surface of the assembled client. Built by
 // INTERSECTING each domain mixin's public interface (each DERIVED from its class
 // and enforced by that class's `implements` clause — issue #446, no hand-mirror)
@@ -24,6 +24,7 @@ import {
  isCollabAuthFailedError,
 } from "../lib/collab-session.js";
 import { withPageLock, isUuid } from "../lib/page-lock.js";
+import type { PageId } from "../lib/page-id.js";
 import { getCollabToken, performLogin } from "../lib/auth-utils.js";
 import { formatDocmostAxiosError } from "./errors.js";
 import { GetPageConversionCache } from "./getpage-cache.js";
@@ -669,10 +670,17 @@ export abstract class DocmostClientContext {
   * once via getPageRaw and cached (both slugId->uuid and uuid->uuid), so
   * repeated edits on the same page add no extra request.
   */
-  protected async resolvePageId(pageId: string): Promise<string> {
-    if (isUuid(pageId)) return pageId;
+  protected async resolvePageId(pageId: string): Promise<PageId> {
+    // This is the ONE canonicalization seam, so it is where the `PageId` brand
+    // 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.
+    if (isUuid(pageId)) return pageId as PageId;
    const cached = this.pageIdCache.get(pageId);
-    if (cached) return cached;
+    if (cached) return cached as PageId;
    const data = await this.getPageRaw(pageId);
    const uuid = data?.id;
    if (typeof uuid !== "string" || !uuid) {
@@ -681,7 +689,7 @@ export abstract class DocmostClientContext {
      );
    }
    this.pageIdCache.set(pageId, uuid);
-    return uuid;
+    return uuid as PageId;
  }


@@ -13,6 +13,7 @@ import { JSDOM } from "jsdom";
 import { markdownToProseMirror } from "@docmost/prosemirror-markdown";
 import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
 import { withPageLock } from "./page-lock.js";
+import type { PageId } from "./page-id.js";
 import {
  sanitizeForYjs,
  findUnstorableAttr,
@@ -250,7 +251,10 @@ export function assertYjsEncodable(doc: any): void {
 * read->write window, and it never throws (it can NEVER break a write).
 */
 export async function mutatePageContent(
-  pageId: string,
+  // Canonical UUID only (#260/#435): the brand forces every caller to
+  // resolvePageId() BEFORE this seam so the lock + CollabSession key can never
+  // be a raw slugId.
+  pageId: PageId,
  collabToken: string,
  baseUrl: string,
  transform: (liveDoc: any) => any | null,
@@ -287,7 +291,7 @@ export async function mutatePageContent(
 * mutatePageContent.
 */
 export async function replacePageContent(
-  pageId: string,
+  pageId: PageId,
  prosemirrorDoc: any,
  collabToken: string,
  baseUrl: string,
@@ -316,7 +320,7 @@ export async function replacePageContent(
 * Tables and :::callout::: blocks survive thanks to the full schema.
 */
 export async function updatePageContentRealtime(
-  pageId: string,
+  pageId: PageId,
  markdownContent: string,
  collabToken: string,
  baseUrl: string,
@@ -0,0 +1,81 @@
+/**
+ * Branded page-identity types 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.
+ *
+ * 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.
+ */
+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;
+}
@@ -8,6 +8,8 @@
 * failure) before it runs. Different pages never block each other.
 */

+import type { PageId } from "./page-id.js";
+
 const chains = new Map<string, Promise<unknown>>();

 // Canonical UUID shape (versions 1–8, matching the `uuid` package's `validate`
@@ -28,11 +30,14 @@ export function isUuid(value: string): boolean {
 // awaited/handled by the caller; only the internal chaining tail swallows
 // errors (purely to gate ordering).
 export function withPageLock<T>(
-  pageId: string,
+  pageId: PageId,
  fn: () => Promise<T>,
 ): Promise<T> {
-  // STRUCTURAL INVARIANT (issue #449, "resolve-then-lock"): the mutex key MUST
-  // be the canonical page UUID, never a raw slugId. The whole write path relies
+  // STRUCTURAL INVARIANT (issue #449/#435, "resolve-then-lock"): the mutex key
+  // MUST be the canonical page UUID, never a raw slugId. The `PageId` brand now
+  // enforces this at COMPILE time (a raw string / slugId no longer type-checks
+  // as a key); the runtime assert below stays as a backstop for untyped (JS)
+  // callers and the http/stdio transports. The whole write path relies
  // on the lock key AND the CollabSession cache key being the resolved UUID
  // (#260) — if a future write method forgot to call resolvePageId and locked
  // under a slugId, two writes to the same page would take DIFFERENT mutex keys
@@ -0,0 +1,57 @@
+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/);
+});