From 87b22197ff868e518a9fbcc43dc6c2582c8d62ff Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 14:10:05 +0000 Subject: [PATCH 1/3] test: Fill unit-test gaps for MCP task functionality Task behavior is covered end-to-end at the integration level but the small pure helpers were uncovered at the unit level. Adds: - isTaskCancelled (src/mcp/utils.ts): return values for each task status and not-found case, plus that taskId/mcpSessionId propagate to getTask. - ProgressTracker: related-task _meta is included when taskId is supplied and absent otherwise. - createProgressTracker factory: null/instance branches for each combination of progressToken, sendNotification, and onStatusMessage. - taskSupport contract across tool categories: only default-mode call-actor declares execution.taskSupport, value is in ALLOWED_TASK_TOOL_EXECUTION_MODES, and no openai-mode tool declares it. https://claude.ai/code/session_01SBaWhEW1vE35bTTivv1JNs --- tests/unit/mcp.utils.test.ts | 41 +++++++++++++++++- tests/unit/tools.mode_contract.test.ts | 36 +++++++++++++++- tests/unit/utils.progress.test.ts | 60 +++++++++++++++++++++++++- 3 files changed, 134 insertions(+), 3 deletions(-) diff --git a/tests/unit/mcp.utils.test.ts b/tests/unit/mcp.utils.test.ts index 1227847e..ba4e9dcf 100644 --- a/tests/unit/mcp.utils.test.ts +++ b/tests/unit/mcp.utils.test.ts @@ -1,7 +1,8 @@ +import type { TaskStore } from '@modelcontextprotocol/sdk/experimental/tasks/interfaces.js'; import { describe, expect, it, vi } from 'vitest'; import { SKYFIRE_README_CONTENT } from '../../src/const.js'; -import { parseInputParamsFromUrl } from '../../src/mcp/utils.js'; +import { isTaskCancelled, parseInputParamsFromUrl } from '../../src/mcp/utils.js'; import { resolvePaymentProvider } from '../../src/payments/index.js'; import { createResourceService } from '../../src/resources/resource_service.js'; import type { AvailableWidget } from '../../src/resources/widgets.js'; @@ -61,6 +62,44 @@ describe('parseInputParamsFromUrl', () => { }); }); +describe('isTaskCancelled', () => { + const makeTaskStore = (getTaskReturn: unknown) => ({ + getTask: vi.fn().mockResolvedValue(getTaskReturn), + } as unknown as TaskStore); + + it('should return true when task status is cancelled', async () => { + const taskStore = makeTaskStore({ status: 'cancelled' }); + + const result = await isTaskCancelled('task-1', 'session-1', taskStore); + + expect(result).toBe(true); + }); + + it('should return false when task status is not cancelled', async () => { + const taskStore = makeTaskStore({ status: 'working' }); + + const result = await isTaskCancelled('task-1', 'session-1', taskStore); + + expect(result).toBe(false); + }); + + it('should return false when task is not found (getTask returns undefined)', async () => { + const taskStore = makeTaskStore(undefined); + + const result = await isTaskCancelled('task-1', 'session-1', taskStore); + + expect(result).toBe(false); + }); + + it('should pass taskId and mcpSessionId through to taskStore.getTask', async () => { + const taskStore = makeTaskStore({ status: 'working' }); + + await isTaskCancelled('task-42', 'session-xyz', taskStore); + + expect(taskStore.getTask).toHaveBeenCalledWith('task-42', 'session-xyz'); + }); +}); + describe('MCP resources', () => { const buildAvailableWidget = (uri: string, exists: boolean): AvailableWidget => ({ ...WIDGET_REGISTRY[uri], diff --git a/tests/unit/tools.mode_contract.test.ts b/tests/unit/tools.mode_contract.test.ts index a0a85fdf..cefb16e3 100644 --- a/tests/unit/tools.mode_contract.test.ts +++ b/tests/unit/tools.mode_contract.test.ts @@ -9,7 +9,7 @@ */ import { describe, expect, it } from 'vitest'; -import { HelperTools } from '../../src/const.js'; +import { ALLOWED_TASK_TOOL_EXECUTION_MODES, HelperTools } from '../../src/const.js'; import { searchApifyDocsTool } from '../../src/tools/common/search_apify_docs.js'; import { CATEGORY_NAMES, getCategoryTools } from '../../src/tools/index.js'; import type { ToolBase, ToolEntry } from '../../src/types.js'; @@ -174,6 +174,40 @@ describe('getCategoryTools mode contract (tool-mode separation)', () => { }); }); +describe('taskSupport contract across tool categories', () => { + it('should declare taskSupport only on call-actor in default mode, with an allowed value', () => { + const defaultCategories = getCategoryTools('default'); + const toolsWithTaskSupport: { name: string; value: unknown }[] = []; + + for (const categoryName of CATEGORY_NAMES) { + for (const tool of defaultCategories[categoryName]) { + if (tool.execution?.taskSupport !== undefined) { + toolsWithTaskSupport.push({ name: tool.name, value: tool.execution.taskSupport }); + } + } + } + + // Only default-mode call-actor is expected to declare taskSupport among static internal tools. + // (Dynamically-created Actor tools from actor_tools_factory also declare it, but those are not + // returned by getCategoryTools.) + expect(toolsWithTaskSupport.map((t) => t.name)).toEqual([HelperTools.ACTOR_CALL]); + + for (const { value } of toolsWithTaskSupport) { + expect(ALLOWED_TASK_TOOL_EXECUTION_MODES).toContain(value); + } + }); + + it('should not declare taskSupport on any tool in openai mode', () => { + const openaiCategories = getCategoryTools('openai'); + + for (const categoryName of CATEGORY_NAMES) { + for (const tool of openaiCategories[categoryName]) { + expect(tool.execution?.taskSupport).toBeUndefined(); + } + } + }); +}); + describe('getToolPublicFieldOnly _meta filtering', () => { const toolWithOpenAiMeta = { name: 'test-tool', diff --git a/tests/unit/utils.progress.test.ts b/tests/unit/utils.progress.test.ts index e5c3fb89..fa168cfe 100644 --- a/tests/unit/utils.progress.test.ts +++ b/tests/unit/utils.progress.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; -import { ProgressTracker } from '../../src/utils/progress.js'; +import { createProgressTracker, ProgressTracker } from '../../src/utils/progress.js'; describe('ProgressTracker', () => { it('should send progress notifications correctly', async () => { @@ -83,4 +83,62 @@ describe('ProgressTracker', () => { await expect(tracker.updateProgress('Test')).resolves.toBeUndefined(); expect(mockOnStatusMessage).toHaveBeenCalledWith('Test'); }); + + it('should include related-task metadata with taskId in progress notifications', async () => { + const mockSendNotification = vi.fn(); + const tracker = new ProgressTracker({ + progressToken: 'tok', + sendNotification: mockSendNotification, + taskId: 'task-abc', + }); + + await tracker.updateProgress('running'); + + expect(mockSendNotification).toHaveBeenCalledWith({ + method: 'notifications/progress', + params: { + progressToken: 'tok', + progress: 1, + message: 'running', + }, + _meta: { + 'io.modelcontextprotocol/related-task': { + taskId: 'task-abc', + }, + }, + }); + }); + + it('should not include _meta when taskId is not provided', async () => { + const mockSendNotification = vi.fn(); + const tracker = new ProgressTracker({ + progressToken: 'tok', + sendNotification: mockSendNotification, + }); + + await tracker.updateProgress('running'); + + const notification = mockSendNotification.mock.calls[0][0]; + expect(notification).not.toHaveProperty('_meta'); + }); +}); + +describe('createProgressTracker', () => { + it('should return null when no progressToken, no sendNotification, and no onStatusMessage', () => { + expect(createProgressTracker(undefined, undefined)).toBeNull(); + }); + + it('should return ProgressTracker when progressToken and sendNotification are provided', () => { + const tracker = createProgressTracker('tok', vi.fn()); + expect(tracker).toBeInstanceOf(ProgressTracker); + }); + + it('should return ProgressTracker when only onStatusMessage is provided', () => { + const tracker = createProgressTracker(undefined, undefined, undefined, vi.fn()); + expect(tracker).toBeInstanceOf(ProgressTracker); + }); + + it('should return null when progressToken is provided but sendNotification is not', () => { + expect(createProgressTracker('tok', undefined)).toBeNull(); + }); }); From c44bdbd6881638a27a27c46fd06819fc7e6cd20f Mon Sep 17 00:00:00 2001 From: Jiri Spilka Date: Thu, 16 Apr 2026 22:26:39 +0200 Subject: [PATCH 2/3] refactor: Extract RELATED_TASK_META_KEY constant; trim createProgressTracker tests Co-Authored-By: Claude Opus 4.7 (1M context) --- src/const.ts | 4 ++++ src/utils/progress.ts | 4 ++-- tests/unit/mcp.utils.test.ts | 4 ---- tests/unit/utils.progress.test.ts | 12 ++---------- 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/const.ts b/src/const.ts index 5e0715b7..732457ef 100644 --- a/src/const.ts +++ b/src/const.ts @@ -208,3 +208,7 @@ export const HTTP_NOT_FOUND = 404; // Modes that allow long running task tool executions export const ALLOWED_TASK_TOOL_EXECUTION_MODES = ['optional', 'required'] as const; + +// MCP _meta key for associating messages with a task. +// TODO: replace with RELATED_TASK_META_KEY from @modelcontextprotocol/sdk once the installed SDK exports it. +export const RELATED_TASK_META_KEY = 'io.modelcontextprotocol/related-task'; diff --git a/src/utils/progress.ts b/src/utils/progress.ts index 5d0f7268..c0a24375 100644 --- a/src/utils/progress.ts +++ b/src/utils/progress.ts @@ -1,7 +1,7 @@ import type { ProgressNotification } from '@modelcontextprotocol/sdk/types.js'; import type { ApifyClient } from '../apify_client.js'; -import { PROGRESS_NOTIFICATION_INTERVAL_MS } from '../const.js'; +import { PROGRESS_NOTIFICATION_INTERVAL_MS, RELATED_TASK_META_KEY } from '../const.js'; export class ProgressTracker { private progressToken?: string | number; @@ -39,7 +39,7 @@ export class ProgressTracker { // Per MCP spec: progress notifications during task execution should include related-task metadata ...(this.taskId && { _meta: { - 'io.modelcontextprotocol/related-task': { + [RELATED_TASK_META_KEY]: { taskId: this.taskId, }, }, diff --git a/tests/unit/mcp.utils.test.ts b/tests/unit/mcp.utils.test.ts index ba4e9dcf..194068e1 100644 --- a/tests/unit/mcp.utils.test.ts +++ b/tests/unit/mcp.utils.test.ts @@ -69,7 +69,6 @@ describe('isTaskCancelled', () => { it('should return true when task status is cancelled', async () => { const taskStore = makeTaskStore({ status: 'cancelled' }); - const result = await isTaskCancelled('task-1', 'session-1', taskStore); expect(result).toBe(true); @@ -77,7 +76,6 @@ describe('isTaskCancelled', () => { it('should return false when task status is not cancelled', async () => { const taskStore = makeTaskStore({ status: 'working' }); - const result = await isTaskCancelled('task-1', 'session-1', taskStore); expect(result).toBe(false); @@ -85,7 +83,6 @@ describe('isTaskCancelled', () => { it('should return false when task is not found (getTask returns undefined)', async () => { const taskStore = makeTaskStore(undefined); - const result = await isTaskCancelled('task-1', 'session-1', taskStore); expect(result).toBe(false); @@ -93,7 +90,6 @@ describe('isTaskCancelled', () => { it('should pass taskId and mcpSessionId through to taskStore.getTask', async () => { const taskStore = makeTaskStore({ status: 'working' }); - await isTaskCancelled('task-42', 'session-xyz', taskStore); expect(taskStore.getTask).toHaveBeenCalledWith('task-42', 'session-xyz'); diff --git a/tests/unit/utils.progress.test.ts b/tests/unit/utils.progress.test.ts index fa168cfe..c32de601 100644 --- a/tests/unit/utils.progress.test.ts +++ b/tests/unit/utils.progress.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; +import { RELATED_TASK_META_KEY } from '../../src/const.js'; import { createProgressTracker, ProgressTracker } from '../../src/utils/progress.js'; describe('ProgressTracker', () => { @@ -102,7 +103,7 @@ describe('ProgressTracker', () => { message: 'running', }, _meta: { - 'io.modelcontextprotocol/related-task': { + [RELATED_TASK_META_KEY]: { taskId: 'task-abc', }, }, @@ -128,17 +129,8 @@ describe('createProgressTracker', () => { expect(createProgressTracker(undefined, undefined)).toBeNull(); }); - it('should return ProgressTracker when progressToken and sendNotification are provided', () => { - const tracker = createProgressTracker('tok', vi.fn()); - expect(tracker).toBeInstanceOf(ProgressTracker); - }); - it('should return ProgressTracker when only onStatusMessage is provided', () => { const tracker = createProgressTracker(undefined, undefined, undefined, vi.fn()); expect(tracker).toBeInstanceOf(ProgressTracker); }); - - it('should return null when progressToken is provided but sendNotification is not', () => { - expect(createProgressTracker('tok', undefined)).toBeNull(); - }); }); From 734d36e6af8a36693ad94c112a7f3692629a711e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Apr 2026 09:48:55 +0000 Subject: [PATCH 3/3] fix: handle progressToken=0 in progress tracking Agent-Logs-Url: https://github.com/apify/apify-mcp-server/sessions/aed4b4c5-420c-469d-a96d-0bf1cf8915b1 Co-authored-by: jirispilka <19406805+jirispilka@users.noreply.github.com> --- src/mcp/server.ts | 2 +- src/utils/progress.ts | 5 +++-- tests/unit/utils.progress.test.ts | 17 +++++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 051b3f08..e1ec65a4 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -871,7 +871,7 @@ export class ActorsMcpServer { } // Only set up notification handlers if progressToken is provided by the client - if (progressToken) { + if (progressToken !== undefined && progressToken !== null) { // Set up notification handlers for the client for (const schema of ServerNotificationSchema.options) { const method = schema.shape.method.value; diff --git a/src/utils/progress.ts b/src/utils/progress.ts index c0a24375..dde27600 100644 --- a/src/utils/progress.ts +++ b/src/utils/progress.ts @@ -27,7 +27,7 @@ export class ProgressTracker { this.currentProgress += 1; // Send progress notification only if progressToken and sendNotification are available - if (this.progressToken && this.sendNotification) { + if (this.progressToken !== undefined && this.progressToken !== null && this.sendNotification) { try { const notification: ProgressNotification = { method: 'notifications/progress' as const, @@ -111,7 +111,8 @@ export function createProgressTracker( onStatusMessage?: (message: string) => Promise, ): ProgressTracker | null { // Create tracker if we have either progress notification support or a status message callback - if ((!progressToken || !sendNotification) && !onStatusMessage) { + const hasProgressNotificationSupport = progressToken !== undefined && progressToken !== null && !!sendNotification; + if (!hasProgressNotificationSupport && !onStatusMessage) { return null; } diff --git a/tests/unit/utils.progress.test.ts b/tests/unit/utils.progress.test.ts index c32de601..78f4f569 100644 --- a/tests/unit/utils.progress.test.ts +++ b/tests/unit/utils.progress.test.ts @@ -133,4 +133,21 @@ describe('createProgressTracker', () => { const tracker = createProgressTracker(undefined, undefined, undefined, vi.fn()); expect(tracker).toBeInstanceOf(ProgressTracker); }); + + it('should return ProgressTracker and send notifications for progressToken = 0', async () => { + const mockSendNotification = vi.fn(); + const tracker = createProgressTracker(0, mockSendNotification); + + expect(tracker).toBeInstanceOf(ProgressTracker); + await tracker?.updateProgress('Started'); + + expect(mockSendNotification).toHaveBeenCalledWith({ + method: 'notifications/progress', + params: { + progressToken: 0, + progress: 1, + message: 'Started', + }, + }); + }); });