From 575125a5dc6741cdc83efddee8edecf4edca5fb3 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 02:20:12 +0300 Subject: [PATCH] fix(mcp): enrich bad/inaccessible spaceId 404 into an actionable error (#534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A well-formed but non-existent/inaccessible spaceId made the space-permissions check answer with an opaque 404 ("Space permissions not found"), which the agent could not self-correct from. Add an enrich-on-404 wrapper at the client-method level (Variant A — no backend change) that, ONLY on that 404, replaces the server text with a factual message naming the bad spaceId and the spaces the token can actually see, pointing at listSpaces. Mechanism (context.ts): - getAccessibleSpaceIndex(): the token's accessible spaces from the single source of truth (/spaces), with a per-instance short-TTL cache (MCP_SPACES_CACHE_TTL_MS, default 60s; 0 disables) and single-flight dedup. Only a COMPLETE (untruncated) result is cached and only a complete result may drive the rewrite. The in-flight promise is nulled on BOTH resolve and reject so a transient /spaces blip is never memoized. Cache invalidated on every identity change, mirroring collabTokenCache (login() + the 401/403 reauth interceptor). - paginateAllWithMeta(): surfaces the `truncated` flag paginateAll swallows; paginateAll now delegates to it (unchanged contract, getSpaces untouched). - withSpaceAccessDiagnostics(spaceId, mcpName, fn): abort/cap wins FIRST (by the toolAbortSignal flag, NOT e.name — a cap may be TimeoutError/custom); only a 404 is enrichable; FAILS OPEN (rethrows the original server error) on every source of uncertainty (fetch failed / !complete / spaceId present / aborted). Wrapped only the paths where the sole 404 cause is the space membership check: getTree, listPages (tree + recent-with-spaceId), search (with spaceId), checkNewComments. createPage/getPageContext are deliberately NOT wrapped. formatSpaceNotAccessible (errors.ts) composes the message (<=10 spaces + "(+N ещё)" tail; distinct zero-spaces variant). Tests: 11 new unit tests (stub client) covering all 9 acceptance criteria + message shape; full mcp unit suite green (701 tests). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/mcp/src/client.ts | 6 +- packages/mcp/src/client/comments.ts | 13 +- packages/mcp/src/client/context.ts | 216 ++++++++++- packages/mcp/src/client/errors.ts | 34 ++ packages/mcp/src/client/read.ts | 62 ++- .../unit/space-access-diagnostics.test.mjs | 354 ++++++++++++++++++ 6 files changed, 661 insertions(+), 24 deletions(-) create mode 100644 packages/mcp/test/unit/space-access-diagnostics.test.mjs diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index 75c62871..e33b8211 100644 --- a/packages/mcp/src/client.ts +++ b/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"; // The full public + shared instance surface of the assembled client. Built by // INTERSECTING each domain mixin's public interface (each DERIVED from its class diff --git a/packages/mcp/src/client/comments.ts b/packages/mcp/src/client/comments.ts index c1fd557d..da12e606 100644 --- a/packages/mcp/src/client/comments.ts +++ b/packages/mcp/src/client/comments.ts @@ -650,10 +650,15 @@ export function CommentsMixin>( // 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 const results: any[] = []; diff --git a/packages/mcp/src/client/context.ts b/packages/mcp/src/client/context.ts index 8c784f53..c615ce5e 100644 --- a/packages/mcp/src/client/context.ts +++ b/packages/mcp/src/client/context.ts @@ -25,7 +25,10 @@ import { } from "../lib/collab-session.js"; import { withPageLock, isUuid } from "../lib/page-lock.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 @@ -116,6 +119,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; + spaces: { id: string; name: string }[]; + complete: boolean; +}; + export abstract class DocmostClientContext { protected client: AxiosInstance; protected token: string | null = null; @@ -163,6 +196,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 | 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 @@ -280,6 +334,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(); @@ -412,6 +469,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}`; }) @@ -622,13 +681,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( endpoint: string, basePayload: Record = {}, limit: number = 100, ): Promise { + return (await this.paginateAllWithMeta(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( + endpoint: string, + basePayload: Record = {}, + limit: number = 100, + ): Promise<{ items: T[]; truncated: boolean }> { await this.ensureAuthenticated(); const clampedLimit = Math.max(1, Math.min(100, limit)); @@ -689,7 +769,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 { + 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 => { + 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( + spaceId: string, + mcpName: string, + fn: () => Promise, + ): Promise { + 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)); + } } diff --git a/packages/mcp/src/client/errors.ts b/packages/mcp/src/client/errors.ts index 19c3d4aa..aa9ada95 100644 --- a/packages/mcp/src/client/errors.ts +++ b/packages/mcp/src/client/errors.ts @@ -46,6 +46,40 @@ 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 listed = shown.map((s) => `${s.id} (${s.name})`).join(", "); + const remaining = spaces.length - shown.length; + const tail = + remaining > 0 ? ` (+${remaining} ещё, см. listSpaces)` : ""; + return `${mcpName}: spaceId "${spaceId}" не найден среди доступных тебе спейсов. Доступные: ${listed}${tail} — скопируй нужный id дословно из listSpaces.`; +} + // 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. diff --git a/packages/mcp/src/client/read.ts b/packages/mcp/src/client/read.ts index 0587ab47..85fb9fa6 100644 --- a/packages/mcp/src/client/read.ts +++ b/packages/mcp/src/client/read.ts @@ -132,17 +132,29 @@ export function ReadMixin>(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 = { 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>(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>(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(); } } diff --git a/packages/mcp/test/unit/space-access-diagnostics.test.mjs b/packages/mcp/test/unit/space-access-diagnostics.test.mjs new file mode 100644 index 00000000..1d91355b --- /dev/null +++ b/packages/mcp/test/unit/space-access-diagnostics.test.mjs @@ -0,0 +1,354 @@ +// 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 make404(url = "/pages/tree") { + const config = { method: "post", url, baseURL: "http://host.example/api" }; + const response = { + status: 404, + statusText: "Not Found", + data: { message: "Space permissions not found" }, + headers: {}, + config, + }; + return new AxiosError( + "Request failed with status code 404", + "ERR_BAD_REQUEST", + config, + {}, + response, + ); +} + +// 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"); +}); + +// --- 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", () => { + const many = Array.from({ length: 13 }, (_, i) => ({ + id: `id-${i}`, + name: `Space ${i}`, + })); + const msg = formatSpaceNotAccessible("getTree", BAD, many); + assert.ok(msg.includes("id-0 (Space 0)")); + assert.ok(msg.includes("id-9 (Space 9)"), "10th entry (index 9) is shown"); + assert.ok(!msg.includes("id-10 (Space 10)"), "the 11th is collapsed"); + assert.ok(msg.includes("(+3 ещё, см. listSpaces)"), "tail counts the remainder"); +});