Merge remote-tracking branch 'gitea/develop' into fix/534-mcp-spaceid

The list-only cap assumed a well-formed prefix; a pathologically long
agent-supplied spaceId (unvalidated for length) lives in the prefix and bypassed
the cap. Add the same unconditional final slice formatDocmostAxiosError uses, so
the WHOLE message is bounded regardless of spaceId length. No-op for normal
inputs (36-char UUID) where the list cap already keeps it <= 300.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -392,6 +392,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `@docmost/token-estimate` package) so they can never diverge. Deferred-tool
   activation is also cached in the chat metadata to avoid re-resolving it each
   turn. (#490)
+- **Cyrillic (and any non-ASCII) draw.io labels no longer turn into mojibake
+  when a diagram is opened in the draw.io editor.** Agent-created diagrams
+  (`drawioCreate`) and Confluence-imported diagrams stored their model in the
+  SVG's `content=` attribute as base64; the draw.io editor decodes that via
+  Latin-1 `atob` (no UTF-8 step), so every non-ASCII char (e.g. `Старт-бит`,
+  `ё`, `—`) split into garbage and the editor's autosave then persisted the
+  corrupted model, breaking the page preview too. Both write paths
+  (`buildDrawioSvg`, the import service's `createDrawioSvg`) now write `content=`
+  as XML-entity-escaped mxfile XML — draw.io's own native form, decoded by the
+  DOM as UTF-8 — so labels open intact. The decoder reads both the new
+  entity-encoded form and the old base64 form, so existing diagrams still open.
+  *Healing pre-fix diagrams:* only a diagram that still holds its original
+  (correct-UTF-8) base64 — i.e. one not yet opened/autosaved in the draw.io
+  editor — can be repaired in place by `drawioGet` → `drawioUpdate` with the
+  same XML (rewrites the attachment in the new form); no migration script is
+  needed. A diagram that was already opened in the editor persisted the
+  mojibake at rest, so `drawioGet` reads the already-corrupted text and
+  `drawioUpdate` faithfully rewrites it — that text is lost and is not
+  recoverable by a rewrite. (#507)
 - **A chat with one malformed message part no longer 500s on every turn, and a
   failed send no longer duplicates the user's message.** Incoming client parts
   are now whitelisted to `text` (a forged tool-result part can no longer reach

apps/server/src/core/ai-chat/ai-chat.service.ts

@@ -189,10 +189,11 @@ export function stepBudgetWarning(stepNumber: number): string {
 //
 // `system` is the in-scope system prompt; we CONCATENATE so the original
 // persona/context is preserved — a bare `system` override would REPLACE the
-// whole system prompt for the step. `activatedTools` is PER-TURN mutable state
-// owned by the streaming loop (a closure Set grown by loadTools); it is passed
-// in (not module-global, not persisted) so this stays a pure function of its
-// arguments.
+// whole system prompt for the step. `activatedTools` is a closure Set grown by
+// loadTools and owned by the streaming loop; the caller seeds it from and
+// persists it to the chat's metadata across turns (#490), but this function only
+// READS the Set it is handed, so it stays a pure function of its arguments (not
+// module-global).
 //
 // NOTE: at AI SDK v7 the per-step `system` field is renamed to `instructions`.
 // On v6 (`^6.0.134`) `system` is the correct field — adjust when bumping.
@@ -1410,10 +1411,11 @@ export class AiChatService implements OnModuleInit, OnModuleDestroy {
       const baseTools = { ...external.tools, ...docmostTools };
 
       // Deferred tool loading state (#332), scoped to THIS streaming loop:
-      //  - `activatedTools` is per-TURN mutable state — a fresh closure Set created
-      //    per streamText call, NOT module-global and NOT persisted, so a new turn
-      //    starts cold. loadTools.execute adds to it; prepareAgentStep reads it to
-      //    widen `activeTools` on the NEXT step.
+      //  - `activatedTools` is a fresh closure Set per streamText call (not
+      //    module-global), SEEDED from the chat's persisted metadata.activatedTools
+      //    (#490, just below) so activation carries across turns. loadTools.execute
+      //    adds to it; prepareAgentStep reads it to widen `activeTools` on the NEXT
+      //    step; turn end persists it back.
       //  - `validDeferredNames` = every tool that is NOT core (the in-app deferred
       //    tools + ALL external MCP tools), computed from the ACTUAL toolset so an
       //    external tool is loadable by its namespaced name. loadTools rejects any

apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts

@@ -0,0 +1,131 @@
+// p-limit and @sindresorhus/slugify are ESM-only and not in jest's transform
+// allowlist; both are irrelevant to createDrawioSvg (a pure fs + string method),
+// so they are mocked out to keep the module graph loadable under ts-jest.
+jest.mock('p-limit', () => ({
+  __esModule: true,
+  default: () => (fn: () => unknown) => fn(),
+}));
+jest.mock('@sindresorhus/slugify', () => ({
+  __esModule: true,
+  default: (input: string) => String(input),
+}));
+
+import { promises as fs } from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { ImportAttachmentService } from './import-attachment.service';
+
+/**
+ * Unit test for ImportAttachmentService.createDrawioSvg (issue #507).
+ *
+ * The Confluence import wraps a `.drawio` file into a `.drawio.svg` attachment.
+ * The `content=` payload MUST be the mxfile XML entity-escaped (draw.io's native
+ * form), NOT base64 — draw.io's editor decodes a base64 content= via Latin-1
+ * atob, mangling every non-ASCII char into mojibake. createDrawioSvg touches no
+ * injected dependency, so the service is built with placeholder deps.
+ */
+describe('ImportAttachmentService.createDrawioSvg (#507)', () => {
+  const service = new ImportAttachmentService(
+    {} as any,
+    {} as any,
+    {} as any,
+  );
+  const call = (p: string): Promise<Buffer> =>
+    (service as any).createDrawioSvg(p);
+
+  let tmpDir: string;
+  beforeAll(async () => {
+    tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'drawio-507-'));
+  });
+  afterAll(async () => {
+    await fs.rm(tmpDir, { recursive: true, force: true });
+  });
+
+  const writeDrawio = async (name: string, xml: string): Promise<string> => {
+    const p = path.join(tmpDir, name);
+    await fs.writeFile(p, xml, 'utf-8');
+    return p;
+  };
+
+  it('writes content= as entity-encoded XML, not base64', async () => {
+    const drawio =
+      '<mxfile host="Confluence"><diagram name="Схема — ёж">' +
+      '<mxGraphModel><root><mxCell id="0"/>' +
+      '<mxCell id="2" value="Старт-бит" vertex="1" parent="0"/>' +
+      '</root></mxGraphModel></diagram></mxfile>';
+    const p = await writeDrawio('cyrillic.drawio', drawio);
+
+    const svg = (await call(p)).toString('utf-8');
+    const content = /content="([^"]*)"/.exec(svg)?.[1];
+    expect(content).toBeDefined();
+
+    // Entity-encoded XML form, starting with &lt;mxfile — never a base64 blob.
+    expect(content).toMatch(/^&lt;mxfile/);
+    expect(content).toContain('&lt;');
+    // Non-ASCII survives as raw UTF-8, with no Latin-1 mojibake.
+    expect(content).toContain('Старт-бит');
+    expect(content).toContain('Схема — ёж');
+    expect(content).not.toContain('Ð');
+
+    // Decoding the attribute (un-escaping) yields the original drawio file.
+    const decoded = content!
+      .replace(/&lt;/g, '<')
+      .replace(/&gt;/g, '>')
+      .replace(/&quot;/g, '"')
+      .replace(/&amp;/g, '&');
+    expect(decoded).toBe(drawio);
+  });
+
+  it('encodes literal tab/newline/CR as numeric char-refs, not literal control chars (#507 F1)', async () => {
+    // A literal tab/newline/CR inside the mxfile XML would be collapsed to a
+    // single space by XML attribute-value normalization when the draw.io editor
+    // reads content=, silently flattening multi-line labels and tab-bearing
+    // values. They must be emitted as numeric char-refs instead.
+    const drawio =
+      '<mxfile><diagram name="p">' +
+      '<mxGraphModel><root><mxCell id="0"/>' +
+      '<mxCell id="2" value="col1\tcol2" style="html=1;\nshadow=0" vertex="1" parent="0"/>' +
+      '<mxCell id="3" value="Line1\nLine2\rLine3" vertex="1" parent="0"/>' +
+      '</root></mxGraphModel></diagram></mxfile>';
+    const p = await writeDrawio('ctrl.drawio', drawio);
+
+    const svg = (await call(p)).toString('utf-8');
+    const content = /content="([^"]*)"/.exec(svg)?.[1];
+    expect(content).toBeDefined();
+    // No literal control chars survive in the attribute value.
+    expect(content).not.toMatch(/[\t\n\r]/);
+    // They round-trip as numeric char-refs.
+    expect(content).toContain('&#x9;');
+    expect(content).toContain('&#xa;');
+    expect(content).toContain('&#xd;');
+    // Decoding (char-refs back to literal, entities back) recovers the file.
+    const decoded = content!
+      .replace(/&lt;/g, '<')
+      .replace(/&gt;/g, '>')
+      .replace(/&quot;/g, '"')
+      .replace(/&#39;/g, "'")
+      .replace(/&#x9;/gi, '\t')
+      .replace(/&#xa;/gi, '\n')
+      .replace(/&#xd;/gi, '\r')
+      .replace(/&amp;/g, '&');
+    expect(decoded).toBe(drawio);
+  });
+
+  it('escapes XML metacharacters in the drawio payload', async () => {
+    const drawio = '<mxfile><diagram name="a &amp; b">"q" &lt;x&gt;</diagram></mxfile>';
+    const p = await writeDrawio('meta.drawio', drawio);
+
+    const svg = (await call(p)).toString('utf-8');
+    const content = /content="([^"]*)"/.exec(svg)?.[1];
+    expect(content).toBeDefined();
+    // The attribute value must contain no bare `<`, `>` or `"` that would break
+    // out of the content="..." attribute or the SVG element.
+    expect(content).not.toMatch(/[<>"]/);
+    const decoded = content!
+      .replace(/&lt;/g, '<')
+      .replace(/&gt;/g, '>')
+      .replace(/&quot;/g, '"')
+      .replace(/&amp;/g, '&');
+    expect(decoded).toBe(drawio);
+  });
+});

apps/server/src/integrations/import/services/import-attachment.service.ts

@@ -8,6 +8,7 @@ import { createReadStream } from 'node:fs';
 import { promises as fs } from 'fs';
 import { Readable } from 'stream';
 import { getMimeType, sanitizeFileName } from '../../../common/helpers';
+import { htmlEscape } from '../../../common/helpers/html-escaper';
 import { v7 } from 'uuid';
 import { FileTask } from '@docmost/db/types/entity.types';
 import { getAttachmentFolderPath } from '../../../core/attachment/attachment.utils';
@@ -849,7 +850,12 @@ export class ImportAttachmentService {
   ): Promise<Buffer> {
     try {
       const drawioContent = await fs.readFile(drawioPath, 'utf-8');
-      const drawioBase64 = Buffer.from(drawioContent).toString('base64');
+      // Write the mxfile XML XML-entity-escaped (draw.io's native content= form),
+      // NOT base64. draw.io's editor decodes a base64 content= via Latin-1 atob
+      // (no UTF-8 step), turning every non-ASCII char (Cyrillic, ё, —) into
+      // mojibake; the entity-encoded form is decoded by the DOM as UTF-8 and
+      // opens intact. Docmost's own decoder reads both forms.
+      const drawioEscaped = this.xmlEscapeContent(drawioContent);
 
       let imageElement = '';
       // If we have a PNG, include it in the SVG
@@ -875,7 +881,7 @@ export class ImportAttachmentService {
       width="600"
       height="400"
       viewBox="0 0 600 400"
-      content="${drawioBase64}">${imageElement}</svg>`;
+      content="${drawioEscaped}">${imageElement}</svg>`;
 
       return Buffer.from(svgContent, 'utf-8');
     } catch (error) {
@@ -884,6 +890,24 @@ export class ImportAttachmentService {
     }
   }
 
+  /**
+   * Escape a string so it is safe as the value of a double-quoted XML attribute
+   * (the `content=` payload of a `.drawio.svg`). The shared `htmlEscape` covers
+   * `& < > " '` (a strict superset of what this attribute needs; the extra `'`
+   * escape is harmless in a `"`-delimited value). On top of that, the numeric
+   * char-refs for tab/newline/CR are required: a literal tab/newline/CR inside
+   * an attribute value is collapsed to a single space by XML attribute-value
+   * normalization on DOM read (both our decoder and the real draw.io editor),
+   * silently flattening multi-line labels and tab-bearing values. Char-refs
+   * survive that normalization (#507).
+   */
+  private xmlEscapeContent(s: string): string {
+    return htmlEscape(s)
+      .replace(/\t/g, '&#x9;')
+      .replace(/\n/g, '&#xa;')
+      .replace(/\r/g, '&#xd;');
+  }
+
   private async uploadWithRetry(opts: {
     abs: string;
     storageFilePath: string;

apps/server/test/integration/ai-chat-stream.int-spec.ts

@@ -322,20 +322,21 @@ describe('AiChatService.stream [integration]', () => {
   });
 
   /**
-   * #332 deferred tool loading, the ON path. The riskiest property is that the
-   * per-turn `activatedTools` Set is created FRESH inside each stream() call, so a
-   * tool a previous turn activated via loadTools is NOT still active when the next
-   * turn starts — the new turn begins "cold" (CORE + loadTools only). The unit
-   * tests only exercise pure prepareAgentStep with hand-fed Sets; this pins the
-   * real wiring end-to-end (loadTools.execute -> activatedTools -> prepareStep ->
-   * per-step activeTools) against the real streamText loop, and proves there is no
-   * cross-turn leak. We drive a MockLanguageModelV3 whose step 1 calls
-   * loadTools(['createPage']) and assert, via the model's recorded per-step
-   * CallOptions.tools (the AI SDK filters the provider tool list by activeTools),
-   * that the deferred tool becomes active on the SAME turn's next step but NOT on a
-   * fresh turn's first step.
+   * #332 + #490 deferred tool loading, the ON path. Turn 1 starts COLD (CORE +
+   * loadTools only) and activates a deferred tool via loadTools; that activation
+   * is PERSISTED into the chat's metadata.activatedTools (#490) so the NEXT turn
+   * SEEDS from it and the tool is active from the fresh turn's FIRST step — the
+   * model never re-runs loadTools to re-activate the same tool. The unit tests
+   * only exercise pure prepareAgentStep with hand-fed Sets; this pins the real
+   * wiring end-to-end (loadTools.execute -> activatedTools -> persist -> next-turn
+   * seed -> prepareStep -> per-step activeTools) against the real streamText loop.
+   * We drive a MockLanguageModelV3 whose step 1 calls loadTools(['createPage'])
+   * and assert, via the model's recorded per-step CallOptions.tools (the AI SDK
+   * filters the provider tool list by activeTools), that the deferred tool becomes
+   * active on the SAME turn's next step AND, seeded from metadata, on the next
+   * turn's first step.
    */
-  describe('deferred tool loading ON — per-turn activation, no leak (#332)', () => {
+  describe('deferred tool loading ON — cross-turn activation persistence (#332 + #490)', () => {
     // A stub deferred (non-core) tool the agent can activate. Its execute is never
     // called — the model only needs to SEE it become active — but it must be a
     // valid AI-SDK tool so the SDK includes it in a step's tool list once active.
@@ -451,7 +452,7 @@ describe('AiChatService.stream [integration]', () => {
       } as any);
     }
 
-    it('activates a deferred tool for the SAME turn, and a NEW turn starts cold (no leak)', async () => {
+    it('activates a deferred tool for the SAME turn, and a NEW turn SEEDS it from persisted chat metadata (#490)', async () => {
       const chatId = (await createChat(db, { workspaceId, creatorId: userId })).id;
 
       // --- Turn 1: loadTools(createPage) on step 1, then answer on step 2. ---
@@ -474,7 +475,7 @@ describe('AiChatService.stream [integration]', () => {
       // Step 2 of the SAME turn sees the just-activated deferred tool.
       expect(step2Tools).toContain('createPage');
 
-      // --- Turn 2 on the SAME chat: must start cold again. ---
+      // --- Turn 2 on the SAME chat: seeds the persisted activation (#490). ---
       const model2 = new MockLanguageModelV3({
         doStream: async () => ({ stream: successStream() }),
       } as any);
@@ -485,9 +486,10 @@ describe('AiChatService.stream [integration]', () => {
 
       const nextTurnFirstStep = toolNames(model2.doStreamCalls[0]);
       expect(nextTurnFirstStep).toContain('loadTools');
-      // The activated set is per-turn: the prior turn's createPage did NOT leak,
-      // so the fresh turn's first step sees it deferred again.
-      expect(nextTurnFirstStep).not.toContain('createPage');
+      // #490: activation PERSISTS across turns — turn 1 wrote createPage into the
+      // chat's metadata.activatedTools, so the next turn seeds from it and the
+      // deferred tool is active from the FIRST step (no need to re-run loadTools).
+      expect(nextTurnFirstStep).toContain('createPage');
     });
   });
 });

packages/mcp/src/client.ts

@@ -31,7 +31,11 @@ import { TransformsMixin, type ITransformsMixin } from "./client/transforms.js";
 // existing importer (index.ts, http.ts, stdio.ts, the in-app host) keeps working
 // with ZERO changes.
 export type { DocmostMcpConfig, SandboxPut } from "./client/context.js";
-export { formatDocmostAxiosError, assertFullUuid } from "./client/errors.js";
+export {
+  formatDocmostAxiosError,
+  assertFullUuid,
+  formatSpaceNotAccessible,
+} from "./client/errors.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

packages/mcp/src/client/comments.ts

@@ -725,10 +725,15 @@ export function CommentsMixin<TBase extends GConstructor<DocmostClientContext>>(
     // The subtree scope (parentPageId given) already INCLUDES the root node
     // itself: /pages/tree seeds getPageAndDescendants with id = parentPageId, so
     // no separate getPageRaw fetch for the parent is needed.
-    const { pages: pagesInScope, truncated } = await this.enumerateSpacePages(
-      spaceId,
-      parentPageId,
-    );
+    // #534: the enumerateSpacePages seed (`/pages/tree`) 404s for a bad or
+    // inaccessible spaceId; wrap it so that 404 becomes an actionable "spaceId
+    // not accessible" hint instead of the opaque "Space permissions not found".
+    // Only the whole enumeration is wrapped (the only 404 source here) — see the
+    // wrap-allowlist invariant on withSpaceAccessDiagnostics.
+    const { pages: pagesInScope, truncated } =
+      await this.withSpaceAccessDiagnostics(spaceId, "checkNewComments", () =>
+        this.enumerateSpacePages(spaceId, parentPageId),
+      );
 
     // 2. Fetch comments for each page, keep ones created after since. Runs with
     // bounded concurrency (#490) instead of one-at-a-time — the per-page reads are

packages/mcp/src/client/context.ts

@@ -26,7 +26,10 @@ import {
 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 {
+  formatDocmostAxiosError,
+  formatSpaceNotAccessible,
+} from "./errors.js";
 import { GetPageConversionCache } from "./getpage-cache.js";
 
 // A generic mixin base constructor (issue #450). Each domain mixin is a factory
@@ -117,6 +120,36 @@ function readCollabTokenTtlMs(): number {
   return Number.isFinite(raw) ? Math.max(0, raw) : 5 * 60 * 1000;
 }
 
+/**
+ * Accessible-space index cache TTL in milliseconds (issue #534). Read fresh from
+ * the environment on every access — mirroring readCollabTokenTtlMs above — so a
+ * test or a live rollback can change it without reloading the module.
+ *
+ * The index (see getAccessibleSpaceIndex) is fetched ONLY on the enrich-on-404
+ * slow path to turn an opaque "Space permissions not found" 404 into a factual
+ * "spaceId X is not among your accessible spaces" hint; a short TTL keeps a burst
+ * of failing tool calls from re-sweeping /spaces each time while never widening
+ * the permission-staleness window meaningfully. Default 60s. An EXPLICIT 0 (or
+ * negative) DISABLES the cache (exact fetch-per-enrichment). Unset/unparseable
+ * (NaN) falls back to the 60s default with the cache ON.
+ */
+function readSpacesCacheTtlMs(): number {
+  const raw = parseInt(process.env.MCP_SPACES_CACHE_TTL_MS ?? "", 10);
+  return Number.isFinite(raw) ? Math.max(0, raw) : 60000;
+}
+
+/**
+ * The set of spaces the current token can see, plus a `complete` flag that is
+ * false when the /spaces listing was truncated at the pagination ceiling. Used
+ * by the enrich-on-404 diagnostics: an authoritative membership test is only
+ * possible when `complete` is true (see withSpaceAccessDiagnostics).
+ */
+export type AccessibleSpaceIndex = {
+  ids: Set<string>;
+  spaces: { id: string; name: string }[];
+  complete: boolean;
+};
+
 export abstract class DocmostClientContext {
   protected client: AxiosInstance;
   protected token: string | null = null;
@@ -164,6 +197,27 @@ export abstract class DocmostClientContext {
   // bypassed on a forced refresh (the 401/403 reauth path). null = no token yet.
   protected collabTokenCache: { token: string; mintedAt: number } | null = null;
 
+  // Accessible-space index cache + single-flight (issue #534). TWO separate
+  // fields, mirroring loginPromise (in-flight dedup) vs collabTokenCache
+  // (persistent value):
+  //   - spaceIndexCache: the last SUCCESSFULLY-FETCHED, COMPLETE index plus the
+  //     wall-clock time it was fetched. Written ONLY from a resolved /spaces
+  //     sweep whose result was complete (a truncated list is never cached, since
+  //     it cannot answer "is this spaceId missing?"). Per-instance (a
+  //     DocmostClient is built per user / per chat) so it can never leak across
+  //     identities; invalidated on every identity change exactly like
+  //     collabTokenCache (login() + the 401/403 reauth interceptor).
+  //   - spaceIndexInFlight: dedups concurrent enrich-on-404 fetches into ONE
+  //     /spaces sweep. CRITICAL INVARIANT: this promise is nulled in `.finally`
+  //     on BOTH resolve AND reject — a rejected/settled promise is NEVER
+  //     memoized, so a transient /spaces blip during one failed tool call cannot
+  //     poison the diagnostics for the rest of the session.
+  protected spaceIndexCache: {
+    index: AccessibleSpaceIndex;
+    fetchedAt: number;
+  } | null = null;
+  protected spaceIndexInFlight: Promise<AccessibleSpaceIndex> | null = null;
+
   // Content-addressed conversion cache for getPage (issue #479). Keyed on
   // (canonical pageId, updatedAt, optionsHash) -> the converted Markdown, so a
   // re-read of an UNCHANGED page skips the expensive convertProseMirrorToMarkdown
@@ -281,6 +335,9 @@ export abstract class DocmostClientContext {
           // keep serving a collab token minted under the old one.
           this.token = null;
           this.collabTokenCache = null;
+          // #534: a new identity/login must not keep serving a space index
+          // computed under the old token (same reasoning as collabTokenCache).
+          this.spaceIndexCache = null;
           delete this.client.defaults.headers.common["Authorization"];
           try {
             await this.login();
@@ -413,6 +470,8 @@ export abstract class DocmostClientContext {
           // Identity (re)established: drop any collab token minted under a
           // previous identity so the #435 cache can never outlive it.
           this.collabTokenCache = null;
+          // #534: likewise drop the accessible-space index of the old identity.
+          this.spaceIndexCache = null;
           this.client.defaults.headers.common["Authorization"] =
             `Bearer ${token}`;
         })
@@ -630,13 +689,34 @@ export abstract class DocmostClientContext {
   }
 
   /**
-   * Generic pagination handler for Docmost API endpoints
+   * Generic pagination handler for Docmost API endpoints. Thin wrapper over
+   * paginateAllWithMeta that discards the `truncated` flag — the historical
+   * contract every caller (getSpaces, etc.) relies on. Callers that need to KNOW
+   * whether the result set was complete (e.g. #534's getAccessibleSpaceIndex,
+   * which must not assert "spaceId missing" against a truncated list) call
+   * paginateAllWithMeta directly.
    */
   async paginateAll<T = any>(
     endpoint: string,
     basePayload: Record<string, any> = {},
     limit: number = 100,
   ): Promise<T[]> {
+    return (await this.paginateAllWithMeta<T>(endpoint, basePayload, limit))
+      .items;
+  }
+
+  /**
+   * Generic pagination handler that ALSO surfaces whether the result was
+   * truncated at the MAX_PAGES ceiling. `paginateAll` swallows this flag (it only
+   * warns); callers that must distinguish "complete listing" from "gave up at the
+   * cap" use this overload. `truncated` is true iff the loop stopped at the
+   * ceiling while the server still reported more pages.
+   */
+  async paginateAllWithMeta<T = any>(
+    endpoint: string,
+    basePayload: Record<string, any> = {},
+    limit: number = 100,
+  ): Promise<{ items: T[]; truncated: boolean }> {
     await this.ensureAuthenticated();
 
     const clampedLimit = Math.max(1, Math.min(100, limit));
@@ -697,7 +777,137 @@ export abstract class DocmostClientContext {
       );
     }
 
-    return allItems;
+    return { items: allItems, truncated };
+  }
+
+  /**
+   * The set of spaces the current token can access (issue #534), fetched from the
+   * single source of truth — the `/spaces` listing — with a per-instance
+   * short-TTL cache and single-flight dedup. Used ONLY by the enrich-on-404 slow
+   * path (withSpaceAccessDiagnostics), so the happy path incurs ZERO extra
+   * requests.
+   *
+   * `complete` is `!truncated`: it is false when the /spaces listing was cut at
+   * the pagination ceiling. A truncated index can never authoritatively answer
+   * "is this spaceId missing?", so only a complete result is cached AND only a
+   * complete result is allowed to drive the "not accessible" rewrite.
+   *
+   * Cache/single-flight discipline (see the spaceIndexCache / spaceIndexInFlight
+   * field docs):
+   *   - serve a fresh, complete cached index without any request;
+   *   - otherwise collapse concurrent callers onto ONE in-flight /spaces sweep;
+   *   - write the persistent cache ONLY from a resolved, complete fetch;
+   *   - null the in-flight promise on BOTH resolve and reject (never memoize a
+   *     rejected promise — a transient /spaces failure must be retried fresh).
+   */
+  async getAccessibleSpaceIndex(): Promise<AccessibleSpaceIndex> {
+    const ttl = readSpacesCacheTtlMs();
+
+    // Fast path: a still-fresh, complete cached index needs no request at all.
+    if (
+      ttl > 0 &&
+      this.spaceIndexCache &&
+      Date.now() - this.spaceIndexCache.fetchedAt < ttl
+    ) {
+      return this.spaceIndexCache.index;
+    }
+
+    // Single-flight: a concurrent enrichment joins the in-flight sweep instead of
+    // issuing its own. (A settled/rejected promise is never left here — see the
+    // `.finally` below — so this only ever joins a genuinely in-progress fetch.)
+    if (this.spaceIndexInFlight) return this.spaceIndexInFlight;
+
+    const fetchPromise = (async (): Promise<AccessibleSpaceIndex> => {
+      const { items, truncated } = await this.paginateAllWithMeta("/spaces", {});
+      const spaces = items.map((s: any) => ({
+        id: s?.id,
+        name: s?.name,
+      }));
+      return {
+        ids: new Set(spaces.map((s) => s.id)),
+        spaces,
+        complete: !truncated,
+      };
+    })();
+
+    this.spaceIndexInFlight = fetchPromise
+      .then((index) => {
+        // Cache ONLY a complete result, and only while the cache is enabled.
+        if (ttl > 0 && index.complete) {
+          this.spaceIndexCache = { index, fetchedAt: Date.now() };
+        }
+        return index;
+      })
+      .finally(() => {
+        // CRITICAL (#534): clear the in-flight slot on BOTH resolve and reject.
+        // Nulling on reject too means a transient /spaces error is retried by the
+        // NEXT enrichment with a fresh fetch, never re-serving the rejection.
+        this.spaceIndexInFlight = null;
+      });
+
+    return this.spaceIndexInFlight;
+  }
+
+  /**
+   * Wrap a client method whose 404 means "the supplied spaceId is not accessible"
+   * and, ONLY on that 404, replace the opaque server text ("Space permissions not
+   * found") with a factual, actionable message naming the spaceId and the spaces
+   * the token can actually see (issue #534). A HINT layered on top of the
+   * backend, which stays authoritative — so it FAILS OPEN on ANY uncertainty:
+   * every branch below that is not a confident "this spaceId is genuinely
+   * missing" rethrows the ORIGINAL server error unchanged. The happy path returns
+   * fn()'s value with zero extra requests.
+   *
+   * WRAP-ALLOWLIST INVARIANT (load-bearing — read before wrapping a new method):
+   * among the currently wrapped tools a 404 comes ONLY from the spaceId
+   * membership / space-permissions check — their pageId / rootPageId /
+   * parentPageId branches resolve to 403 or 200, NEVER 404. If a future change
+   * adds a `NotFoundException` to `/pages/tree`, `/pages/recent`,
+   * `/pages/sidebar-pages` or `/search` (e.g. "page not found"), this enrichment
+   * would MISATTRIBUTE that 404 to the spaceId. Re-audit the wrapped call before
+   * relying on this, and only wrap paths where the sole 404 cause is the space.
+   */
+  protected async withSpaceAccessDiagnostics<T>(
+    spaceId: string,
+    mcpName: string,
+    fn: () => Promise<T>,
+  ): Promise<T> {
+    try {
+      return await fn();
+    } catch (e) {
+      // Abort/cap wins FIRST and is detected by the SIGNAL FLAG, not e.name: a
+      // per-call cap may be an AbortSignal.timeout() (reason name "TimeoutError")
+      // or a custom reason, so `e.name === 'AbortError'` is NOT reliable (#534
+      // hole B). A stopped/capped turn must propagate its reason, never trigger a
+      // /spaces sweep or a rewrite.
+      if (this.toolAbortSignal?.aborted) throw e;
+
+      // Only a 404 is enrichable; any other status/shape is a different failure.
+      if (!(axios.isAxiosError(e) && e.response?.status === 404)) throw e;
+
+      let idx: AccessibleSpaceIndex;
+      try {
+        idx = await this.getAccessibleSpaceIndex();
+      } catch (fetchErr) {
+        // The /spaces sweep itself failed. If we were aborted mid-sweep,
+        // propagate the abort reason; otherwise FAIL OPEN with the ORIGINAL
+        // server error rather than a misleading "not found".
+        if (this.toolAbortSignal?.aborted) throw fetchErr;
+        if (process.env.DEBUG) {
+          console.error("space-diag: /spaces fetch failed:", fetchErr);
+        }
+        throw e;
+      }
+
+      // Fail open when the listing is incomplete (can't assert "missing") or when
+      // the spaceId IS present (the 404 is about something else, not the space).
+      if (!idx.complete) throw e;
+      if (idx.ids.has(spaceId)) throw e;
+
+      // Confident: the spaceId is well-formed but not among the accessible
+      // spaces. Replace the opaque server text with the actionable fact.
+      throw new Error(formatSpaceNotAccessible(mcpName, spaceId, idx.spaces));
+    }
   }
 
 

packages/mcp/src/client/errors.ts

@@ -46,6 +46,60 @@ export function assertFullUuid(
   }
 }
 
+// Max number of accessible spaces to enumerate inline in the "space not
+// accessible" message (issue #534) before collapsing the rest into a "(+N ещё)"
+// tail, so a workspace with many spaces cannot blow up the model context.
+const SPACE_LIST_CAP = 10;
+
+/**
+ * Compose the model-facing "spaceId is not accessible" message (issue #534).
+ * This is FACT text about the supplied spaceId that REPLACES the opaque server
+ * string ("Space permissions not found") on the enrich-on-404 path — it names
+ * the exact bad id, lists the spaces the token can actually see (id + name, so
+ * the agent can copy the right id verbatim), and points at `listSpaces`.
+ *
+ * Deliberately Russian: like the other agent-facing tool guidance in this repo,
+ * this is the message the acting agent reads to self-correct.
+ */
+export function formatSpaceNotAccessible(
+  mcpName: string,
+  spaceId: string,
+  spaces: { id: string; name: string }[],
+): string {
+  // No accessible spaces at all — a distinct diagnosis (token has no space
+  // access), not "you picked the wrong one from this list".
+  if (!Array.isArray(spaces) || spaces.length === 0) {
+    return `${mcpName}: spaceId "${spaceId}" недоступен, и доступных тебе спейсов нет — проверь доступ токена / вызови listSpaces.`;
+  }
+
+  const shown = spaces.slice(0, SPACE_LIST_CAP);
+  const remaining = spaces.length - shown.length;
+  const tail =
+    remaining > 0 ? ` (+${remaining} ещё, см. listSpaces)` : "";
+
+  // Cap the whole message at ERROR_MESSAGE_CAP (same budget as
+  // formatDocmostAxiosError) so ~10 long space names cannot blow up the model
+  // context. Truncate ONLY the interpolated space LIST — the fixed prefix (which
+  // carries the bad spaceId) and the fixed suffix (the "…из listSpaces"
+  // instruction) are always kept intact, so the actionable parts survive even
+  // when the list is trimmed.
+  const prefix = `${mcpName}: spaceId "${spaceId}" не найден среди доступных тебе спейсов. Доступные: `;
+  const suffix = ` — скопируй нужный id дословно из listSpaces.`;
+  let listed = shown.map((s) => `${s.id} (${s.name})`).join(", ") + tail;
+  const budget = ERROR_MESSAGE_CAP - prefix.length - suffix.length;
+  if (listed.length > budget) {
+    listed = listed.slice(0, Math.max(0, budget - 1)) + "…";
+  }
+  const message = `${prefix}${listed}${suffix}`;
+  // Unconditional final backstop (mirrors formatDocmostAxiosError): the list
+  // cap above assumes a well-formed prefix, but a pathologically long
+  // agent-supplied spaceId lives in the prefix and would otherwise blow past the
+  // budget. Cap the WHOLE message so nothing bloats the model context.
+  return message.length > ERROR_MESSAGE_CAP
+    ? message.slice(0, ERROR_MESSAGE_CAP - 1) + "…"
+    : message;
+}
+
 // Keep ONLY the pathname of a request (no host, no query string, no fragment)
 // so the message never leaks a host or query params. Resolves a relative
 // config.url against config.baseURL, then discards everything but the path.

packages/mcp/src/client/read.ts

@@ -132,17 +132,29 @@ export function ReadMixin<TBase extends GConstructor<DocmostClientContext>>(Base
       // BFS hit its node cap) had no way to know pages were missing. Return the
       // tree alongside the flag; the primary /pages/tree path is uncapped so this
       // is false there.
-      const { pages, truncated } = await this.enumerateSpacePages(spaceId);
-      return { tree: buildPageTree(pages), truncated };
+      // #534: spaceId is required here; wrap so a bad-spaceId 404 (from the
+      // /pages/tree seed inside enumerateSpacePages) becomes an actionable hint.
+      return this.withSpaceAccessDiagnostics(spaceId, "listPages", async () => {
+        const { pages, truncated } = await this.enumerateSpacePages(spaceId);
+        return { tree: buildPageTree(pages), truncated };
+      });
     }
 
     const clampedLimit = Math.max(1, Math.min(100, limit));
     const payload: Record<string, any> = { limit: clampedLimit, page: 1 };
     if (spaceId) payload.spaceId = spaceId;
-    const response = await this.client.post("/pages/recent", payload);
-    const data = response.data;
-    const items = data.data?.items || data.items || [];
-    return items.map((page: any) => filterPage(page));
+    // #534: only the WITH-spaceId recent path can 404 on space access; wrap it so
+    // that 404 is rewritten. Without a spaceId there is no space to diagnose, so
+    // the wrapper is inert (run the request directly).
+    const runRecent = async () => {
+      const response = await this.client.post("/pages/recent", payload);
+      const data = response.data;
+      const items = data.data?.items || data.items || [];
+      return items.map((page: any) => filterPage(page));
+    };
+    return spaceId
+      ? this.withSpaceAccessDiagnostics(spaceId, "listPages", runRecent)
+      : runRecent();
   }
 
   /**
@@ -174,8 +186,16 @@ export function ReadMixin<TBase extends GConstructor<DocmostClientContext>>(Base
         "getTree: spaceId is required (a page tree is scoped to one space).",
       );
     }
-    const { pages } = await this.enumerateSpacePages(spaceId, rootPageId);
-    return buildPageTree(pages, { shape: "getTree", maxDepth });
+    // #534: the 404 for a bad/inaccessible spaceId surfaces from the
+    // `/pages/tree` seeding step inside enumerateSpacePages (which is NOT in a
+    // try/catch of its own for that case) — wrap the whole body so it is caught
+    // and rewritten into an actionable "spaceId not accessible" message. Only the
+    // space membership check can 404 here (rootPageId 403/200), see the
+    // wrap-allowlist invariant on withSpaceAccessDiagnostics.
+    return this.withSpaceAccessDiagnostics(spaceId, "getTree", async () => {
+      const { pages } = await this.enumerateSpacePages(spaceId, rootPageId);
+      return buildPageTree(pages, { shape: "getTree", maxDepth });
+    });
   }
 
   /**
@@ -700,17 +720,27 @@ export function ReadMixin<TBase extends GConstructor<DocmostClientContext>>(Base
     if (limit !== undefined) {
       payload.limit = Math.max(1, Math.min(50, limit));
     }
-    const response = await this.client.post("/search", payload);
 
-    // Normalize both response shapes: bare array and paginated { items: [...] }
-    const data = response.data?.data;
-    const items = Array.isArray(data) ? data : data?.items || [];
-    const filteredItems = items.map((item: any) => filterSearchResult(item));
+    const runSearch = async () => {
+      const response = await this.client.post("/search", payload);
 
-    return {
-      items: filteredItems,
-      success: response.data?.success || false,
+      // Normalize both response shapes: bare array and paginated { items: [...] }
+      const data = response.data?.data;
+      const items = Array.isArray(data) ? data : data?.items || [];
+      const filteredItems = items.map((item: any) => filterSearchResult(item));
+
+      return {
+        items: filteredItems,
+        success: response.data?.success || false,
+      };
     };
+
+    // #534: a search scoped to a spaceId 404s when that space is inaccessible;
+    // wrap only that case so the 404 becomes an actionable hint. A workspace-wide
+    // search (no spaceId) has no space to diagnose — run it directly (inert).
+    return spaceId
+      ? this.withSpaceAccessDiagnostics(spaceId, "search", runSearch)
+      : runSearch();
   }
 
   }

packages/mcp/src/lib/drawio-xml.ts

@@ -178,10 +178,11 @@ function sliceModel(xml: string): string | null {
 // --- decode chain ----------------------------------------------------------
 
 /**
- * Read the `content=` attribute out of a `.drawio.svg` string. Docmost stores a
- * base64 payload there (createDrawioSvg); draw.io's own SVG export may store the
- * XML entity-encoded instead. The DOM decodes entities for us, so the caller
- * only has to distinguish "starts with '<'" (raw XML) from base64.
+ * Read the `content=` attribute out of a `.drawio.svg` string. Docmost writes
+ * the mxfile XML entity-encoded there (buildDrawioSvg / createDrawioSvg), which
+ * is also how draw.io's own SVG export stores it; older attachments stored a
+ * base64 payload instead. The DOM decodes entities for us, so the caller only
+ * has to distinguish "starts with '<'" (raw XML) from base64.
  */
 export function extractContentAttr(svg: string): string {
   const { doc, error } = parseXml(svg);
@@ -195,12 +196,23 @@ export function extractContentAttr(svg: string): string {
   // value itself never contains a double-quote (base64 / entity-encoded XML).
   const m = /content="([^"]*)"/.exec(svg);
   if (m) {
-    // Decode the handful of XML entities a raw regex would leave encoded.
+    // Decode the handful of XML entities a raw regex would leave encoded. The
+    // numeric char-refs for tab/newline/CR MUST be decoded here too: the DOM
+    // path above turns them back into the literal control chars, so this
+    // regex fallback has to agree or the two decode paths diverge (#507).
+    // `&amp;` is decoded last so an escaped `&amp;#x9;` reads back as the
+    // literal text `&#x9;`, not a tab.
     return m[1]
       .replace(/&lt;/g, "<")
       .replace(/&gt;/g, ">")
       .replace(/&quot;/g, '"')
       .replace(/&#39;/g, "'")
+      .replace(/&#x9;/gi, "\t")
+      .replace(/&#9;/g, "\t")
+      .replace(/&#xa;/gi, "\n")
+      .replace(/&#10;/g, "\n")
+      .replace(/&#xd;/gi, "\r")
+      .replace(/&#13;/g, "\r")
       .replace(/&amp;/g, "&");
   }
   throw new Error("drawio: SVG has no content= attribute to decode");
@@ -307,9 +319,16 @@ export function encodeDrawioFile(modelXml: string, title = "Page-1"): string {
 /**
  * Build the `diagram.drawio.svg` attachment. Mirrors the import service's
  * createDrawioSvg contract exactly:
- *   <svg xmlns=… xmlns:xlink=… content="${base64(drawioFile)}">${inner}</svg>
+ *   <svg xmlns=… xmlns:xlink=… content="${xmlEscape(drawioFile)}">${inner}</svg>
  * plus width/height/viewBox from the diagram bounding box and the schematic
  * preview as the visible children (`inner`).
+ *
+ * The `content=` value is the mxfile XML XML-entity-escaped (draw.io's own
+ * native form), NOT base64. draw.io's editor decodes a base64 content= via
+ * Latin-1 atob (no UTF-8 step), turning every non-ASCII char (e.g. Cyrillic,
+ * ё, —) into mojibake; the entity-encoded form is decoded by the DOM as UTF-8
+ * and opens intact. Our decoder (decodeDrawioSvg) reads both forms, so old
+ * base64 attachments still round-trip.
  */
 export function buildDrawioSvg(
   modelXml: string,
@@ -318,14 +337,14 @@ export function buildDrawioSvg(
   title = "Page-1",
 ): string {
   const file = encodeDrawioFile(modelXml, title);
-  const base64 = Buffer.from(file, "utf-8").toString("base64");
+  const content = xmlEscape(file);
   const w = Math.max(1, Math.round(bbox.width));
   const h = Math.max(1, Math.round(bbox.height));
   return (
     `<svg xmlns="http://www.w3.org/2000/svg" ` +
     `xmlns:xlink="http://www.w3.org/1999/xlink" ` +
     `width="${w}" height="${h}" viewBox="0 0 ${w} ${h}" ` +
-    `content="${base64}">${inner}</svg>`
+    `content="${content}">${inner}</svg>`
   );
 }
 
@@ -334,7 +353,15 @@ function xmlEscape(s: string): string {
     .replace(/&/g, "&amp;")
     .replace(/</g, "&lt;")
     .replace(/>/g, "&gt;")
-    .replace(/"/g, "&quot;");
+    .replace(/"/g, "&quot;")
+    // A literal tab/newline/CR inside an attribute value is collapsed to a
+    // single space by XML attribute-value normalization on DOM read (both jsdom
+    // here and the real draw.io editor), silently flattening multi-line labels
+    // and tab-bearing values. Numeric char-refs survive that normalization, so
+    // emit them the way draw.io's own native export does (#507).
+    .replace(/\t/g, "&#x9;")
+    .replace(/\n/g, "&#xa;")
+    .replace(/\r/g, "&#xd;");
 }
 
 // --- normalization + hash --------------------------------------------------

packages/mcp/test/unit/drawio-xml.test.mjs

@@ -51,6 +51,14 @@ const hasRule = (issues, rule, cellId) =>
     (i) => i.rule === rule && (cellId === undefined || i.cellId === cellId),
   );
 
+// Reverse the attribute-value XML escaping used in the `content=` payload.
+const unescapeAttr = (s) =>
+  s
+    .replace(/&lt;/g, "<")
+    .replace(/&gt;/g, ">")
+    .replace(/&quot;/g, '"')
+    .replace(/&amp;/g, "&");
+
 // --- style parsing ---------------------------------------------------------
 
 test("parseStyle: base stylename + key=value pairs", () => {
@@ -335,16 +343,20 @@ test("encode/build: a title with < > \" & round-trips without corrupting the SVG
 
   // The outer content="..." attribute must not be broken by the title: the raw
   // title metacharacters never appear literally in the SVG markup (they are
-  // base64-encoded inside content=, and escaped inside the file XML).
+  // entity-escaped inside content=, doubly so where the file XML already escaped
+  // them inside name="...").
   const contentMatch = /content="([^"]*)"/.exec(svg);
   assert.ok(contentMatch, "SVG has a single well-formed content= attribute");
 
   // The diagram model still decodes losslessly despite the exotic title.
   assert.equal(decodeDrawioSvg(svg), model);
 
+  // content= is now entity-encoded XML (draw.io's native form), never base64.
+  assert.match(contentMatch[1], /^&lt;mxfile/);
+
   // The file XML is well-formed: the title lives in name="..." as escaped
   // entities, so unescaping recovers the original title byte-for-byte.
-  const fileXml = Buffer.from(contentMatch[1], "base64").toString("utf-8");
+  const fileXml = unescapeAttr(contentMatch[1]);
   const nameMatch = /<diagram id="[^"]*" name="([^"]*)">/.exec(fileXml);
   assert.ok(nameMatch, "the diagram name attribute is intact and quote-safe");
   const decodedTitle = nameMatch[1]
@@ -358,3 +370,112 @@ test("encode/build: a title with < > \" & round-trips without corrupting the SVG
   const file = encodeDrawioFile(model, title);
   assert.match(file, /name="A &lt; B &gt; C &quot; D &amp; E">/);
 });
+
+// --- #507: content= is entity-encoded XML, never base64 --------------------
+
+// A model whose cell values carry Cyrillic, ё and an em dash — exactly the
+// characters draw.io's Latin-1 atob mangles when content= is base64.
+const CYRILLIC_MODEL =
+  "<mxGraphModel><root>" +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="Старт-бит — ёж" style="rounded=1;html=1;" vertex="1" parent="1">' +
+  '<mxGeometry x="100" y="100" width="120" height="60" as="geometry"/></mxCell>' +
+  "</root></mxGraphModel>";
+
+test("#507: buildDrawioSvg writes content= as entity-encoded XML (not base64)", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, "Диаграмма");
+
+  const contentMatch = /content="([^"]*)"/.exec(svg);
+  assert.ok(contentMatch, "SVG has a single well-formed content= attribute");
+  const content = contentMatch[1];
+
+  // The content= value is the entity-encoded mxfile XML — starts with `&lt;mxfile`.
+  assert.match(content, /^&lt;mxfile/, "content= is entity-encoded mxfile XML");
+  // It must NOT be a base64 blob: base64 has no XML entities and no literal `<`.
+  assert.ok(content.includes("&lt;"), "content= carries XML entities, not base64");
+
+  // Cyrillic / ё / — survive verbatim in the attribute (raw UTF-8, not atob-mangled).
+  assert.ok(content.includes("Старт-бит — ёж"), "non-ASCII value is raw UTF-8 in content=");
+  assert.ok(content.includes("Диаграмма"), "non-ASCII title is raw UTF-8 in content=");
+  // The mojibake that base64+atob would have produced must be absent.
+  assert.ok(!content.includes("Ð"), "no Latin-1 mojibake in content=");
+});
+
+test("#507: Cyrillic model round-trips byte-stable through buildDrawioSvg -> decodeDrawioSvg", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, "Заголовок — ё");
+  assert.equal(decodeDrawioSvg(svg), model);
+});
+
+test("#507 back-compat: an OLD base64-form .drawio.svg still decodes losslessly", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  // Reproduce the pre-fix write path: encodeDrawioFile -> base64 in content=.
+  const file = encodeDrawioFile(model, "Старая диаграмма");
+  const base64 = Buffer.from(file, "utf-8").toString("base64");
+  const svg =
+    `<svg xmlns="http://www.w3.org/2000/svg" content="${base64}"><g/></svg>`;
+  // No XML entities, purely base64 alphabet — this is the legacy form.
+  assert.ok(!base64.includes("<") && !base64.includes("&"));
+  assert.equal(decodeDrawioSvg(svg), model);
+});
+
+test("#507 negative: non-ASCII in a cell value AND in the title round-trip clean", () => {
+  const model = normalizeXml(CYRILLIC_MODEL);
+  const title = "Тест — ёмкость № 5";
+  const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, title);
+
+  // Model recovered byte-for-byte.
+  assert.equal(decodeDrawioSvg(svg), model);
+
+  // Title lands in the <diagram name="..."> of the decoded file XML, intact.
+  const content = /content="([^"]*)"/.exec(svg)[1];
+  const fileXml = unescapeAttr(content);
+  const nameMatch = /<diagram id="[^"]*" name="([^"]*)">/.exec(fileXml);
+  assert.ok(nameMatch, "diagram name attribute present");
+  assert.equal(unescapeAttr(nameMatch[1]), title);
+});
+
+// --- #507 F1: literal tab/newline/CR in an attribute value survive the
+// content= round-trip. The whole mxfile XML lives in one content="..." attr;
+// XML attribute-value normalization collapses a LITERAL tab/newline/CR to a
+// single space on DOM read, so the escape must emit numeric char-refs instead.
+const CTRL_MODEL =
+  "<mxGraphModel><root>" +
+  '<mxCell id="0"/><mxCell id="1" parent="0"/>' +
+  '<mxCell id="2" value="col1\tcol2" style="html=1;\nshadow=0" vertex="1" parent="1">' +
+  '<mxGeometry x="10" y="10" width="120" height="60" as="geometry"/></mxCell>' +
+  '<mxCell id="3" value="Line1\nLine2\rLine3" vertex="1" parent="1">' +
+  '<mxGeometry x="10" y="100" width="120" height="60" as="geometry"/></mxCell>' +
+  "</root></mxGraphModel>";
+
+test("#507 F1: literal tab/newline/CR in a value round-trip byte-stable (DOM decode)", () => {
+  const svg = buildDrawioSvg(CTRL_MODEL, "<g/>", { width: 200, height: 200 });
+  const content = /content="([^"]*)"/.exec(svg)[1];
+  // The tab/newline/CR must be emitted as numeric char-refs, never as literal
+  // control chars (which DOM attribute-value normalization would eat).
+  assert.ok(
+    !/[\t\n\r]/.test(content),
+    "no literal tab/newline/CR survive in the content= attribute",
+  );
+  assert.ok(
+    content.includes("&#x9;") &&
+      content.includes("&#xa;") &&
+      content.includes("&#xd;"),
+    "tab/newline/CR are emitted as numeric char-refs",
+  );
+  // Full DOM decode recovers the model byte-for-byte, control chars intact.
+  assert.equal(decodeDrawioSvg(svg), CTRL_MODEL);
+});
+
+test("#507 F1: regex fallback decodes tab/newline/CR char-refs (agrees with DOM path)", () => {
+  const svg = buildDrawioSvg(CTRL_MODEL, "<g/>", { width: 200, height: 200 });
+  const content = /content="([^"]*)"/.exec(svg)[1];
+  // A bare `&` makes the SVG wrapper malformed, forcing extractContentAttr onto
+  // its regex fallback branch. That branch must decode the tab/newline/CR
+  // char-refs exactly like the DOM path, or the two decoders diverge.
+  const malformedSvg = `<svg content="${content}">&</svg>`;
+  assert.equal(decodeDrawioSvg(malformedSvg), CTRL_MODEL);
+  // The well-formed (DOM) path yields the identical result.
+  assert.equal(decodeDrawioSvg(svg), CTRL_MODEL);
+});

packages/mcp/test/unit/space-access-diagnostics.test.mjs

@@ -0,0 +1,423 @@
+// Issue #534: enrich-on-404 space-access diagnostics.
+//
+// When a tool is handed a well-formed but non-existent/inaccessible spaceId, the
+// server answers the space-permissions check with an opaque 404 ("Space
+// permissions not found"). The client wrapper (withSpaceAccessDiagnostics) turns
+// ONLY that 404 into an actionable message naming the bad spaceId and the spaces
+// the token can actually see — while FAILING OPEN (rethrowing the original
+// server error unchanged) on every source of uncertainty.
+//
+// These tests drive the assembled DocmostClient with its inner seams
+// (enumerateSpacePages / client.post / paginateAllWithMeta) stubbed at runtime,
+// mirroring the stub-client style of error-diagnostics.test.mjs. Only criterion
+// 9 (createPage is NOT wrapped) uses a real offline http server, because
+// createPage's 404 arrives over a bare-axios multipart path.
+import { test, after } from "node:test";
+import assert from "node:assert/strict";
+import http from "node:http";
+import axios, { AxiosError } from "axios";
+import {
+  DocmostClient,
+  formatSpaceNotAccessible,
+} from "../../build/client.js";
+
+// Two accessible spaces (id + name is all getAccessibleSpaceIndex maps/uses).
+const SPACES = [
+  { id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", name: "Engineering" },
+  { id: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb", name: "Design" },
+];
+// A well-formed UUID that is NOT among the accessible spaces.
+const BAD = "99999999-9999-4999-8999-999999999999";
+
+// Build an AxiosError shaped exactly as the response interceptor would hand it
+// on a 404 — status readable, axios.isAxiosError() true.
+function makeAxiosErr(status, url = "/pages/tree", message = "boom") {
+  const config = { method: "post", url, baseURL: "http://host.example/api" };
+  const response = {
+    status,
+    statusText: String(status),
+    data: { message },
+    headers: {},
+    config,
+  };
+  return new AxiosError(
+    `Request failed with status code ${status}`,
+    "ERR_BAD_REQUEST",
+    config,
+    {},
+    response,
+  );
+}
+function make404(url = "/pages/tree") {
+  return makeAxiosErr(404, url, "Space permissions not found");
+}
+
+// A client whose token is pre-set (so ensureAuthenticated never hits the
+// network) and whose /spaces sweep is a counted stub. Individual tests override
+// enumerateSpacePages / client.post to shape the method-under-test's outcome.
+function makeClient({ spaces = SPACES, truncated = false } = {}) {
+  const c = new DocmostClient("http://127.0.0.1:1/api", "u@example.com", "pw");
+  c.token = "t";
+  c.client.defaults.headers.common["Authorization"] = "Bearer t";
+  c._spacesFetches = 0;
+  c.paginateAllWithMeta = async (endpoint) => {
+    if (endpoint === "/spaces") {
+      c._spacesFetches++;
+      return { items: spaces, truncated };
+    }
+    throw new Error(`unexpected paginate endpoint ${endpoint}`);
+  };
+  return c;
+}
+
+// --- Criterion 1: bad spaceId on getTree -> actionable rewrite --------------
+test("getTree with a non-existent spaceId is rewritten into an actionable hint", async () => {
+  const c = makeClient();
+  c.enumerateSpacePages = async () => {
+    throw make404();
+  };
+
+  await assert.rejects(
+    () => c.getTree(BAD),
+    (e) => {
+      assert.ok(e.message.includes(BAD), "names the passed spaceId");
+      // At least one valid "id (name)" pair.
+      assert.ok(
+        e.message.includes(`${SPACES[0].id} (${SPACES[0].name})`),
+        "lists an accessible id (name)",
+      );
+      assert.ok(e.message.includes("listSpaces"), "points at listSpaces");
+      assert.ok(
+        !e.message.includes("Space permissions not found"),
+        "the opaque server text is replaced",
+      );
+      return true;
+    },
+  );
+  assert.equal(c._spacesFetches, 1, "one /spaces sweep on the enrichment path");
+});
+
+// --- Criterion 2: happy path -> no /spaces request --------------------------
+test("getTree with a valid spaceId returns the tree and makes NO /spaces request", async () => {
+  const c = makeClient();
+  // Spy client.post to prove no /spaces POST is issued on the happy path.
+  const posted = [];
+  c.client.post = async (url) => {
+    posted.push(url);
+    throw new Error(`unexpected post ${url}`);
+  };
+  c.enumerateSpacePages = async () => ({ pages: [], truncated: false });
+
+  const res = await c.getTree(SPACES[0].id);
+  assert.ok(Array.isArray(res), "tree returned as before");
+  assert.equal(c._spacesFetches, 0, "no /spaces sweep on the happy path");
+  assert.ok(
+    !posted.includes("/spaces"),
+    "no /spaces POST on the happy path",
+  );
+});
+
+// --- Criterion 3: short-TTL cache + refetch after expiry --------------------
+test("two bad getTree within TTL share ONE /spaces fetch; a refetch happens after TTL", async () => {
+  const c = makeClient();
+  c.enumerateSpacePages = async () => {
+    throw make404();
+  };
+
+  await assert.rejects(() => c.getTree(BAD), /не найден среди/);
+  await assert.rejects(() => c.getTree(BAD), /не найден среди/);
+  assert.equal(c._spacesFetches, 1, "second call served from cache");
+
+  // Age the cache past the default 60s TTL -> exactly one refetch.
+  assert.ok(c.spaceIndexCache, "a complete result was cached");
+  c.spaceIndexCache.fetchedAt = Date.now() - 10 * 60 * 1000;
+  await assert.rejects(() => c.getTree(BAD), /не найден среди/);
+  assert.equal(c._spacesFetches, 2, "exactly one refetch after TTL");
+});
+
+// --- Criterion 4: no spaceId -> wrapper inert -------------------------------
+test("listPages WITHOUT spaceId is inert (raw error propagates, no /spaces sweep)", async () => {
+  const c = makeClient();
+  c.client.post = async () => {
+    throw make404("/pages/recent");
+  };
+  await assert.rejects(
+    () => c.listPages(),
+    (e) => {
+      assert.equal(e.response?.status, 404, "raw server 404 propagates");
+      assert.ok(!/не найден среди/.test(e.message ?? ""), "not rewritten");
+      return true;
+    },
+  );
+  assert.equal(c._spacesFetches, 0, "no /spaces sweep without a spaceId");
+});
+
+test("search WITHOUT spaceId is inert (raw error propagates, no /spaces sweep)", async () => {
+  const c = makeClient();
+  c.client.post = async () => {
+    throw make404("/search");
+  };
+  await assert.rejects(
+    () => c.search("query"),
+    (e) => {
+      assert.equal(e.response?.status, 404, "raw server 404 propagates");
+      assert.ok(!/не найден среди/.test(e.message ?? ""), "not rewritten");
+      return true;
+    },
+  );
+  assert.equal(c._spacesFetches, 0, "no /spaces sweep without a spaceId");
+});
+
+// --- Fail-open: a NON-404 error is never enriched (regression guard) --------
+test("a non-404 error (500) on a wrapped call propagates unchanged, with NO /spaces sweep", async () => {
+  const c = makeClient();
+  // A real server failure — must surface as-is, never be swallowed by the
+  // enrichment path or reformatted into a "not found among your spaces" message.
+  c.enumerateSpacePages = async () => {
+    throw makeAxiosErr(500, "/pages/tree", "Internal Server Error");
+  };
+  await assert.rejects(
+    () => c.getTree(BAD),
+    (e) => {
+      assert.equal(e.response?.status, 500, "the original 500 propagates");
+      assert.ok(
+        !/не найден среди/.test(e.message ?? ""),
+        "a non-404 is NOT rewritten",
+      );
+      return true;
+    },
+  );
+  assert.equal(c._spacesFetches, 0, "no /spaces sweep for a non-404");
+});
+
+// --- Fail-open: 404 when the spaceId IS accessible -> not about the space ----
+test("a 404 when the spaceId IS in the accessible index fails open (the 404 is about something else)", async () => {
+  const c = makeClient();
+  // The wrapped call 404s, but the spaceId is genuinely accessible — the 404
+  // must be about some OTHER resource, so the original error propagates and is
+  // NOT falsely rewritten to "spaceId not found among your spaces".
+  c.enumerateSpacePages = async () => {
+    throw make404();
+  };
+  await assert.rejects(
+    () => c.getTree(SPACES[0].id),
+    (e) => {
+      assert.equal(e.response?.status, 404, "original 404 preserved");
+      assert.ok(
+        !/не найден среди/.test(e.message ?? ""),
+        "no false 'not found' when the space is accessible",
+      );
+      return true;
+    },
+  );
+  assert.equal(c._spacesFetches, 1, "the index WAS consulted to make this call");
+});
+
+// --- Criterion 5: incomplete listing -> fail open ---------------------------
+test("a truncated /spaces listing (!complete) fails OPEN with the original 404", async () => {
+  const c = makeClient({ truncated: true });
+  c.enumerateSpacePages = async () => {
+    throw make404();
+  };
+  await assert.rejects(
+    () => c.getTree(BAD),
+    (e) => {
+      assert.equal(e.response?.status, 404, "original server error preserved");
+      assert.ok(!/не найден среди/.test(e.message ?? ""), "no false rewrite");
+      return true;
+    },
+  );
+  assert.equal(c.spaceIndexCache, null, "a truncated result is never cached");
+});
+
+// --- Criterion 6: /spaces fetch fails -> fail open; in-flight nulled on reject
+test("a /spaces fetch failure fails OPEN, logs under DEBUG, and never memoizes the rejected in-flight promise", async () => {
+  const c = makeClient();
+  let fetchCount = 0;
+  c.paginateAllWithMeta = async () => {
+    fetchCount++;
+    throw new Error("network boom");
+  };
+  c.enumerateSpacePages = async () => {
+    throw make404();
+  };
+
+  const prevDebug = process.env.DEBUG;
+  process.env.DEBUG = "1";
+  const errs = [];
+  const origErr = console.error;
+  console.error = (...a) => errs.push(a.map(String).join(" "));
+  try {
+    await assert.rejects(
+      () => c.getTree(BAD),
+      (e) => {
+        assert.equal(e.response?.status, 404, "original 404 rethrown");
+        return true;
+      },
+    );
+  } finally {
+    console.error = origErr;
+    if (prevDebug === undefined) delete process.env.DEBUG;
+    else process.env.DEBUG = prevDebug;
+  }
+
+  assert.ok(
+    errs.some((l) => l.includes("space-diag: /spaces fetch failed")),
+    "a DEBUG stderr line is emitted",
+  );
+  assert.equal(c.spaceIndexInFlight, null, "in-flight promise nulled on reject");
+
+  // The rejected in-flight promise must NOT be reused: a second enrichment does
+  // a genuinely fresh fetch (fetchCount increments to 2).
+  await assert.rejects(
+    () => c.getTree(BAD),
+    (e) => e.response?.status === 404,
+  );
+  assert.equal(fetchCount, 2, "fresh fetch — rejected promise not memoized");
+});
+
+// --- Criterion 7: zero accessible spaces ------------------------------------
+test("zero accessible spaces yields the dedicated 'no accessible spaces' message", async () => {
+  const c = makeClient({ spaces: [] });
+  c.enumerateSpacePages = async () => {
+    throw make404();
+  };
+  await assert.rejects(
+    () => c.getTree(BAD),
+    (e) => {
+      assert.ok(
+        e.message.includes("доступных тебе спейсов нет"),
+        "the zero-spaces branch fired",
+      );
+      assert.ok(e.message.includes(BAD), "still names the bad spaceId");
+      return true;
+    },
+  );
+});
+
+// --- Criterion 8: abort/cap during enrichment -------------------------------
+test("an aborted signal (custom/TimeoutError reason) propagates its reason, not a rewrite, and skips the sweep", async () => {
+  const c = makeClient();
+  const ac = new AbortController();
+  const reason = new Error("per-call cap exceeded");
+  reason.name = "TimeoutError"; // NOT 'AbortError' — hole B
+  ac.abort(reason);
+  c.setToolAbortSignal(ac.signal);
+  // Simulate paginateAll's throwIfAborted(): the inner op rejects with the
+  // signal's reason (not an AxiosError).
+  c.enumerateSpacePages = async () => {
+    throw ac.signal.reason;
+  };
+
+  await assert.rejects(
+    () => c.getTree(BAD),
+    (e) => {
+      assert.equal(e, reason, "the abort/cap reason itself propagates");
+      assert.equal(e.name, "TimeoutError");
+      assert.ok(!/не найден среди/.test(e.message ?? ""), "no rewrite");
+      return true;
+    },
+  );
+  assert.equal(c._spacesFetches, 0, "abort short-circuits before any sweep");
+});
+
+// --- Criterion 9: createPage is NOT wrapped (real offline server) -----------
+function readBody(req) {
+  return new Promise((resolve) => {
+    let raw = "";
+    req.on("data", (c) => (raw += c));
+    req.on("end", () => resolve(raw));
+  });
+}
+function sendJson(res, status, obj, extra = {}) {
+  res.writeHead(status, { "Content-Type": "application/json", ...extra });
+  res.end(JSON.stringify(obj));
+}
+const openServers = [];
+function spawn(handler) {
+  return new Promise((resolve) => {
+    const server = http.createServer(handler);
+    server.listen(0, "127.0.0.1", () => {
+      openServers.push(server);
+      const { port } = server.address();
+      resolve({ baseURL: `http://127.0.0.1:${port}/api` });
+    });
+  });
+}
+after(async () => {
+  await Promise.all(openServers.map((s) => new Promise((r) => s.close(r))));
+});
+
+test("createPage with an inaccessible spaceId returns the server error as-is (NOT wrapped)", async () => {
+  const { baseURL } = await spawn(async (req, res) => {
+    await readBody(req);
+    if (req.url === "/api/auth/login") {
+      sendJson(res, 200, { success: true }, {
+        "Set-Cookie": "authToken=t; Path=/; HttpOnly",
+      });
+      return;
+    }
+    if (req.url === "/api/pages/import") {
+      // The space-permissions 404 createPage would see for a bad spaceId.
+      sendJson(res, 404, { message: "Space permissions not found" });
+      return;
+    }
+    sendJson(res, 404, {});
+  });
+
+  const client = new DocmostClient(baseURL, "u@example.com", "pw");
+  // Prove the diagnostics path is never entered from createPage.
+  let idxCalls = 0;
+  const realIdx = client.getAccessibleSpaceIndex.bind(client);
+  client.getAccessibleSpaceIndex = async () => {
+    idxCalls++;
+    return realIdx();
+  };
+
+  await assert.rejects(
+    () => client.createPage("Title", "body", BAD),
+    (e) => {
+      assert.equal(e.response?.status, 404, "the raw server 404 surfaces");
+      assert.ok(
+        !/не найден среди/.test(e.message ?? ""),
+        "createPage's 404 is NOT rewritten (multi-cause path)",
+      );
+      return true;
+    },
+  );
+  assert.equal(idxCalls, 0, "createPage never invokes the space diagnostics");
+});
+
+// --- formatSpaceNotAccessible unit shape ------------------------------------
+test("formatSpaceNotAccessible caps the inline list at 10 and appends a (+N ещё) tail", () => {
+  // Short ids/names so the whole message stays under the length cap and the full
+  // list-cap behaviour (first 10 shown, rest collapsed) is observable intact.
+  const many = Array.from({ length: 13 }, (_, i) => ({
+    id: `s${i}`,
+    name: `${i}`,
+  }));
+  const msg = formatSpaceNotAccessible("getTree", BAD, many);
+  assert.ok(msg.includes("s0 (0)"));
+  assert.ok(msg.includes("s9 (9)"), "10th entry (index 9) is shown");
+  assert.ok(!msg.includes("s10 (10)"), "the 11th is collapsed");
+  assert.ok(msg.includes("(+3 ещё, см. listSpaces)"), "tail counts the remainder");
+  assert.ok(msg.length <= 300, `short-name message stays under the cap (${msg.length})`);
+});
+
+test("formatSpaceNotAccessible caps the assembled message at ERROR_MESSAGE_CAP (300)", () => {
+  // 10 spaces with long names would, uncapped, produce a message several times
+  // over the 300-char budget. The cap must keep it compact while still carrying
+  // the bad spaceId and the listSpaces pointer.
+  const longName = "X".repeat(120);
+  const spaces = Array.from({ length: 10 }, (_, i) => ({
+    id: `id-${i}`,
+    name: `${longName}-${i}`,
+  }));
+  const msg = formatSpaceNotAccessible("getTree", BAD, spaces);
+  assert.ok(
+    msg.length <= 300,
+    `message must be <= 300 chars, got ${msg.length}`,
+  );
+  assert.ok(msg.includes(BAD), "the bad spaceId survives the cap");
+  assert.ok(msg.includes("listSpaces"), "the listSpaces pointer survives the cap");
+});