diff --git a/package-lock.json b/package-lock.json index c67037e..151fff5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "bookstack-mcp", - "version": "5.1.0", + "version": "5.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "bookstack-mcp", - "version": "5.1.0", + "version": "5.2.0", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.25.3", diff --git a/package.json b/package.json index 161e551..69d16fe 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bookstack-mcp", - "version": "5.1.0", + "version": "5.2.0", "description": "MCP server for BookStack wiki — search, read, create, and manage documentation via AI assistants", "type": "module", "bin": { diff --git a/src/bookstack-client.ts b/src/bookstack-client.ts index dad5ada..1e25a5a 100644 --- a/src/bookstack-client.ts +++ b/src/bookstack-client.ts @@ -34,8 +34,13 @@ export interface BookStackConfig { tokenSecret: string; enableWrite?: boolean; insecureSkipTlsVerify?: boolean; + /** Per-request HTTP timeout in ms. Bounds a slow/hung BookStack response so a + * call fails cleanly instead of hanging until the MCP client gives up. */ + timeoutMs?: number; } +const DEFAULT_TIMEOUT_MS = 30000; + export interface Book { id: number; name: string; @@ -144,12 +149,18 @@ export class BookStackClient { private enableWrite: boolean; private baseUrl: string; private bookSlugCache: Map = new Map(); + // In-flight single-book lookups, keyed by id, so concurrent callers for the + // same book share one request instead of stampeding BookStack. + private inflightBookSlug: Map> = new Map(); + // Memoized one-shot bulk warm of the slug cache (see warmBookSlugCache). + private bookSlugWarm?: Promise; constructor(config: BookStackConfig) { this.enableWrite = config.enableWrite || false; this.baseUrl = config.baseUrl; this.client = axios.create({ baseURL: `${config.baseUrl}/api`, + timeout: config.timeoutMs ?? DEFAULT_TIMEOUT_MS, headers: { 'Authorization': `Token ${config.tokenId}:${config.tokenSecret}`, 'Content-Type': 'application/json' @@ -181,21 +192,67 @@ export class BookStackClient { } } + /** + * Populate the book-slug cache from a single bulk `/books` listing, once per + * client lifetime. Enhancing a result set resolves a slug per item; without a + * warm cache a search whose results span (or even share) many books fires a + * concurrent `GET /books/{id}` per item — a cache stampede that BookStack + * rate-limits (429), and the retrying requests pile up in memory until the + * process OOMs. Memoized on `bookSlugWarm` so concurrent callers await one + * warm. Failure is non-fatal: getBookSlug falls back to a lazy single fetch. + */ + private async warmBookSlugCache(): Promise { + if (this.bookSlugWarm) return this.bookSlugWarm; + this.bookSlugWarm = (async () => { + const pageSize = 500; + const maxPages = 20; // safety cap (10k books) — huge wikis fall back to lazy fills + try { + for (let page = 0; page < maxPages; page++) { + const response = await this.client.get('/books', { + params: { count: pageSize, offset: page * pageSize } + }); + const books: Book[] = response.data?.data ?? []; + for (const book of books) { + if (book?.id != null) this.bookSlugCache.set(book.id, book.slug || String(book.id)); + } + if (books.length < pageSize) break; + } + } catch { + // Non-fatal: leave the cache partially warm; getBookSlug lazily fills gaps. + } + })(); + return this.bookSlugWarm; + } + private async getBookSlug(bookId: number): Promise { - // Check cache first - if (this.bookSlugCache.has(bookId)) { - return this.bookSlugCache.get(bookId)!; - } + // A single bulk warm turns the per-result N+1 into +1; after it, every book + // that existed at warm time is a cache hit. + await this.warmBookSlugCache(); - try { - const response = await this.client.get(`/books/${bookId}`); - const slug = response.data.slug || String(bookId); - this.bookSlugCache.set(bookId, slug); - return slug; - } catch (error) { - // Fallback to ID if book fetch fails - return String(bookId); - } + const cached = this.bookSlugCache.get(bookId); + if (cached !== undefined) return cached; + + // Gap (book created after warm, or beyond the warm cap): lazily fetch it, but + // share one in-flight request across concurrent callers so a batch of results + // referencing the same new book can't stampede. + const inflight = this.inflightBookSlug.get(bookId); + if (inflight) return inflight; + + const request = (async () => { + try { + const response = await this.client.get(`/books/${bookId}`); + const slug = response.data.slug || String(bookId); + this.bookSlugCache.set(bookId, slug); + return slug; + } catch (error) { + // Fallback to ID if book fetch fails + return String(bookId); + } finally { + this.inflightBookSlug.delete(bookId); + } + })(); + this.inflightBookSlug.set(bookId, request); + return request; } // URL generation utilities @@ -705,8 +762,10 @@ export class BookStackClient { const response = await this.client.get('/search', { params }); const results = response.data.data || response.data; - // Use the search response data as-is — no per-item BookStack fetch (was N+1). - // Callers can fetch full details via get_page/get_book/get_chapter if needed. + // Use the search response data as-is — callers fetch full details via + // get_page/get_book/get_chapter if needed. URL enhancement resolves a book + // slug per item, but getBookSlug bulk-warms its cache once, so this is +1 + // request, not the per-item N+1 it would otherwise be. const enhancedResults = await Promise.all( results.map(async (result: SearchResult) => { const url = await this.generateContentUrl(result); diff --git a/src/index.ts b/src/index.ts index fbab8ff..91ec217 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1179,6 +1179,9 @@ async function main() { const baseUrl = getRequiredEnvVar('BOOKSTACK_BASE_URL'); const insecureSkipTlsVerify = process.env.BOOKSTACK_INSECURE_SKIP_TLS_VERIFY?.toLowerCase() === 'true'; const envEnableWrite = process.env.BOOKSTACK_ENABLE_WRITE?.toLowerCase() === 'true'; + const timeoutRaw = process.env.BOOKSTACK_TIMEOUT_MS; + const timeoutParsed = timeoutRaw ? parseInt(timeoutRaw, 10) : NaN; + const timeoutMs = Number.isFinite(timeoutParsed) && timeoutParsed > 0 ? timeoutParsed : undefined; const oauth = loadOAuthConfig(process.env); // Initialize the OAuth broker-state store (Redis when REDIS_URL is set) before @@ -1195,7 +1198,8 @@ async function main() { tokenId: getRequiredEnvVar('BOOKSTACK_TOKEN_ID'), tokenSecret: getRequiredEnvVar('BOOKSTACK_TOKEN_SECRET'), enableWrite: oauth ? false : envEnableWrite, - insecureSkipTlsVerify + insecureSkipTlsVerify, + timeoutMs }; // The write credential is optional and only used by the OAuth proxy for sessions whose @@ -1203,7 +1207,7 @@ async function main() { const writeTokenId = process.env.BOOKSTACK_WRITE_TOKEN_ID; const writeTokenSecret = process.env.BOOKSTACK_WRITE_TOKEN_SECRET; const write: BookStackConfig | null = (writeTokenId && writeTokenSecret) - ? { baseUrl, tokenId: writeTokenId, tokenSecret: writeTokenSecret, enableWrite: true, insecureSkipTlsVerify } + ? { baseUrl, tokenId: writeTokenId, tokenSecret: writeTokenSecret, enableWrite: true, insecureSkipTlsVerify, timeoutMs } : null; const config: AppConfig = { read, write, oauth };