From b19682ee78af70fe304c113e8e2560a5f0bbaa7d Mon Sep 17 00:00:00 2001 From: ydflow Date: Sun, 27 Sep 2026 09:47:11 +0800 Subject: [PATCH] fix(iwiki): stop fetchAllPages from rejecting every space walk before it starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'initial queue is empty' guard ran after tryDrain(), which had already dispatched the root request: tryDrain shifts the only queue entry, so the queue is always empty by then and the promise rejected with 'fetchAllPages: rootId 队列为空' on every call, before any response could arrive. Zero-latency mocks hid it — the first response beat the rejection and populated allPages, so the catch guard (allPages.length === 0) swallowed the error. Check the guard before tryDrain() and seed the queue only for a non-empty rootId, so it rejects exactly when there is nothing to walk. Also drop the dead '!stopped' from the drain loop (tryDrain already returns when stopped, and stopped cannot change inside the synchronous loop — oxlint no-unmodified-loop-condition, the hit #836 flags), and write the file's user-facing messages in English per the repo rule. --- src/__tests__/iwiki-client.test.ts | 118 +++++++++++++++++++++++++++++ src/utils/iwiki-client.ts | 27 ++++--- 2 files changed, 133 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/iwiki-client.test.ts diff --git a/src/__tests__/iwiki-client.test.ts b/src/__tests__/iwiki-client.test.ts new file mode 100644 index 000000000..992cba8a5 --- /dev/null +++ b/src/__tests__/iwiki-client.test.ts @@ -0,0 +1,118 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { IWikiClient, type IWikiPage } from '../utils/iwiki-client.js'; +import { log } from '../utils/logger.js'; + +/** + * Resolve on a later macrotask, the way a real MCP request over HTTPS does. A + * mock that resolves within the same microtask turn hides the ordering bug + * this suite guards against: the traversal promise must not be able to reject + * before the first response arrives, and with zero-latency mocks the first + * response beats that rejection, so the bug never shows. + */ +function remote(value: T): Promise { + return new Promise((resolve) => setTimeout(() => resolve(value), 5)); +} + +/** A client whose `getSpacePageTree` answers from `tree`; one node may be an Error. */ +function clientWithTree(tree: Record): { + client: IWikiClient; + calls: string[]; +} { + const client = new IWikiClient('token'); + const calls: string[] = []; + vi.spyOn(client, 'getSpacePageTree').mockImplementation((parentid: string) => { + calls.push(parentid); + const node = tree[parentid]; + if (node === undefined) return remote([]); + return node instanceof Error ? Promise.reject(node) : remote(node); + }); + return { client, calls }; +} + +describe('IWikiClient.fetchAllPages', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns the pages of a single-node space', async () => { + const { client, calls } = clientWithTree({ + root: [{ docid: 'root', title: 'Root' }], + }); + + const pages = await client.fetchAllPages('root'); + + expect(pages).toEqual([{ docid: 'root', title: 'Root' }]); + expect(calls).toEqual(['root']); + }); + + it('walks children breadth-first', async () => { + const { client, calls } = clientWithTree({ + root: [ + { docid: 'a', title: 'A', has_children: true }, + { docid: 'b', title: 'B', has_children: true }, + ], + a: [{ docid: 'c', title: 'C' }], + b: [], + }); + + const pages = await client.fetchAllPages('root'); + + expect(pages.map((page) => page.docid)).toEqual(['a', 'b', 'c']); + expect(calls).toEqual(['root', 'a', 'b']); + }); + + it('keeps going when one node fails and warns about it', async () => { + const warn = vi.spyOn(log, 'warn').mockImplementation(() => {}); + const { client, calls } = clientWithTree({ + root: [ + { docid: 'a', title: 'A', has_children: true }, + { docid: 'b', title: 'B', has_children: true }, + ], + a: new Error('boom'), + b: [{ docid: 'c', title: 'C' }], + }); + + const pages = await client.fetchAllPages('root'); + + expect(pages.map((page) => page.docid)).toEqual(['a', 'b', 'c']); + expect(calls).toEqual(['root', 'a', 'b']); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('parentid=a')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('boom')); + }); + + it('stops at maxPages and warns in English', async () => { + const warn = vi.spyOn(log, 'warn').mockImplementation(() => {}); + const { client, calls } = clientWithTree({ + root: [ + { docid: '1', title: 'One' }, + { docid: '2', title: 'Two' }, + { docid: '3', title: 'Three' }, + ], + }); + + const pages = await client.fetchAllPages('root', { maxPages: 2 }); + + expect(pages.map((page) => page.docid)).toEqual(['1', '2']); + expect(calls).toEqual(['root']); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('max page limit (2)')); + }); + + it('rejects for an empty rootId without any request', async () => { + const { client, calls } = clientWithTree({}); + + await expect(client.fetchAllPages('')).rejects.toThrow('fetchAllPages: rootId is empty'); + expect(calls).toEqual([]); + }); + + it('getSpacePageTree warns in English and returns [] when the request fails', async () => { + const warn = vi.spyOn(log, 'warn').mockImplementation(() => {}); + const client = new IWikiClient('token'); + vi.spyOn(client as unknown as { _callTool: () => Promise }, '_callTool').mockImplementation(() => + Promise.reject(new Error('connect ETIMEDOUT')), + ); + + await expect(client.getSpacePageTree('root')).resolves.toEqual([]); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('iWiki page tree request failed [parentid=root]')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('connect ETIMEDOUT')); + }); +}); diff --git a/src/utils/iwiki-client.ts b/src/utils/iwiki-client.ts index bdcda25d9..99d995c02 100644 --- a/src/utils/iwiki-client.ts +++ b/src/utils/iwiki-client.ts @@ -170,7 +170,7 @@ export class IWikiClient { try { response = JSON.parse(rawBody) as JsonRpcResponse; } catch (parseErr: unknown) { - throw new Error(`iWiki MCP 响应解析失败: ${String(parseErr)}，原始响应: ${rawBody.slice(0, 200)}`); + throw new Error(`iWiki MCP response could not be parsed: ${String(parseErr)}; raw response: ${rawBody.slice(0, 200)}`); } if (response.error) { @@ -223,7 +223,7 @@ export class IWikiClient { : Boolean(item['has_children']), })); } catch (err: unknown) { - log.warn(`获取页面树失败 [parentid=${parentid}]: ${String(err)}`); + log.warn(`iWiki page tree request failed [parentid=${parentid}]: ${String(err)}`); return []; } } @@ -291,8 +291,8 @@ export class IWikiClient { const maxPages = opts?.maxPages ?? DEFAULT_MAX_PAGES; const allPages: IWikiPage[] = []; - // BFS 队列：待获取子树的 parentid 列表 - const queue: string[] = [rootId]; + // BFS 队列：待获取子树的 parentid 列表；rootId 为空则保持为空 + const queue: string[] = rootId ? [rootId] : []; let running = 0; let stopped = false; @@ -304,8 +304,8 @@ export class IWikiClient { return; } - // 填满并发槽 - while (queue.length > 0 && running < concurrency && !stopped) { + // 填满并发槽（tryDrain 入口已拦截 stopped，同步循环体内它不会变化） + while (queue.length > 0 && running < concurrency) { const parentid = queue.shift()!; running++; @@ -318,7 +318,7 @@ export class IWikiClient { if (!stopped) { stopped = true; log.warn( - `已达到最大页数限制（${maxPages}），停止继续遍历。已收集: ${allPages.length} 页`, + `Reached the max page limit (${maxPages}); stopped traversing. Collected ${allPages.length} page(s).`, ); } break; @@ -334,7 +334,7 @@ export class IWikiClient { }) .catch((err: unknown) => { running--; - log.warn(`BFS 遍历节点失败 [parentid=${parentid}]: ${String(err)}`); + log.warn(`BFS traversal failed for node [parentid=${parentid}]: ${String(err)}`); // 单节点失败不中断整体，继续处理其他节点 tryDrain(); }); @@ -346,12 +346,15 @@ export class IWikiClient { } }; - tryDrain(); - - // 防止初始队列为空时直接结束 + // rootId 为空时 BFS 无处可走：必须在 tryDrain 之前判断。tryDrain 会把 + // 队列里的 rootId 派发出去，此后队列必然为空——晚于此处的判断会让每 + // 次调用在遍历开始前就以 "rootId 队列为空" 拒绝。 if (queue.length === 0) { - reject(new Error('fetchAllPages: rootId 队列为空')); + reject(new Error('fetchAllPages: rootId is empty')); + return; } + + tryDrain(); }).catch((err: unknown) => { // 仅 rootId 为空时抛出，其他错误已在 tryDrain 内处理 if (allPages.length === 0) {