Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions integration-tests/sdk-typescript/sdk-mcp-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
SDKTestHelper,
extractText,
findToolUseBlocks,
findToolResults,
createSharedTestOptions,
} from './test-helper.js';

Expand Down Expand Up @@ -431,7 +432,6 @@ describe('SDK MCP Server Integration (E2E)', () => {
});

const messages: SDKMessage[] = [];
let assistantText = '';
let foundToolUse = false;

try {
Expand All @@ -446,15 +446,19 @@ describe('SDK MCP Server Integration (E2E)', () => {
if (toolUseBlocks.length > 0) {
foundToolUse = true;
}
assistantText += extractText(message.message.content);
}
}

// Validate tool was called
expect(foundToolUse).toBe(true);

// Validate result contains the delayed response
expect(assistantText.toLowerCase()).toMatch(/test_async/i);
// Assert on the deterministic tool result rather than the model's
// paraphrased final text, which does not reliably echo the value
// verbatim and made this test flaky on main.
const toolResults = findToolResults(messages, MCP_DELAYED_RESPONSE);
expect(toolResults.length).toBeGreaterThan(0);
expect(toolResults[0]?.isError).toBe(false);
Comment on lines +458 to +460

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This stabilized test is not exercised by any PR-gating CI check. integration-tests/ is outside every npm workspace, so the workspace npm test never collects it; the only workflow that runs it (e2e.yml) has no pull_request trigger and runs post-merge on main, nightly, and on-demand. (The skipped Integration Tests (CLI, No Sandbox) check runs the cli suite via test:integration:cli:sandbox:none, not the sdk-typescript suite this file lives in, so it would not collect this test even when it runs.)

Failure scenario: if the new assertion were defective — e.g. MCP_DELAYED_RESPONSE not matching the real tool name so toolResults is empty and toBeGreaterThan(0) should fail — no PR check would catch it; the PR merges green and the failure surfaces only post-merge on main or at the nightly run. (The assertion is in fact type-correct and the tool name matches — this is a verification gap, not a known defect.)

Concrete step before merge: run the SDK suite on-demand against a live model — npm run test:integration:sdk:sandbox:none (or target this file: QWEN_SANDBOX=false npx vitest run --root ./integration-tests sdk-typescript/sdk-mcp-server.test.ts).

中文说明

[建议] 本次稳定化后的测试不会被任何 PR 门禁 CI 检查执行。integration-tests/ 不在任何 npm workspace 内,因此 workspace 的 npm test 永远不会收集它;唯一运行它的 workflow(e2e.yml)没有 pull_request 触发器,仅在合并到 main 后、每夜定时以及手动触发时运行。(被跳过的 Integration Tests (CLI, No Sandbox) 检查通过 test:integration:cli:sandbox:none 运行的是 cli 套件,而非本文件所在的 sdk-typescript 套件,因此即便它运行也不会收集本测试。)

失败场景:如果新断言存在缺陷——例如 MCP_DELAYED_RESPONSE 与真实工具名不匹配,导致 toolResults 为空、toBeGreaterThan(0) 本应失败——没有任何 PR 检查会捕获它;PR 会以绿色合并,失败只会在合并到 main 后或每夜运行时才显现。(事实上该断言类型正确、工具名也匹配——这是一个验证缺口,而非已知缺陷。)

合并前的具体步骤:针对真实模型按需运行 SDK 套件——npm run test:integration:sdk:sandbox:none(或只运行本文件:QWEN_SANDBOX=false npx vitest run --root ./integration-tests sdk-typescript/sdk-mcp-server.test.ts)。

— qwen3.8-max-preview via Qwen Code /review

expect(toolResults[0]?.content.toLowerCase()).toMatch(/test_async/i);

// Validate successful completion
const lastMessage = messages[messages.length - 1];
Expand Down
Loading