feat: improve mcp.json with better timeout#1238
feat: improve mcp.json with better timeout#1238Crunchyman-ralph wants to merge 0 commit intonextfrom
Conversation
|
WalkthroughAdds timeout: 300 to the task-master-ai MCP server configuration across code, config, tests, and docs. The builder in src/utils/create-mcp-config.js and root .mcp.json now include timeout. Documentation examples and a unit test fixture are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Client as MCP Host Client
participant MCP as task-master-ai Process
User->>Client: Trigger task-master-ai action
Client->>MCP: Spawn with command/args/env
Note over Client,MCP: Timeout set to 300s
alt Completes before 300s
MCP-->>Client: Result
Client-->>User: Return output
else Exceeds 300s
Client-->>MCP: Terminate process (timeout)
Client-->>User: Report timeout
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
llms-install.md (1)
16-34: Replace hard tabs with spaces in code block (MD010).markdownlint flagged hard tabs. Convert to spaces.
Apply this diff:
-{ - "mcpServers": { - "taskmaster-ai": { - "command": "npx", - "args": ["-y", "task-master-ai"], - "timeout": 300, - "env": { - "ANTHROPIC_API_KEY": "user_will_add_their_key_here", - "PERPLEXITY_API_KEY": "user_will_add_their_key_here", - "OPENAI_API_KEY": "user_will_add_their_key_here", - "GOOGLE_API_KEY": "user_will_add_their_key_here", - "MISTRAL_API_KEY": "user_will_add_their_key_here", - "OPENROUTER_API_KEY": "user_will_add_their_key_here", - "XAI_API_KEY": "user_will_add_their_key_here" - } - } - } -} +{ + "mcpServers": { + "taskmaster-ai": { + "command": "npx", + "args": ["-y", "task-master-ai"], + "timeout": 300, + "env": { + "ANTHROPIC_API_KEY": "user_will_add_their_key_here", + "PERPLEXITY_API_KEY": "user_will_add_their_key_here", + "OPENAI_API_KEY": "user_will_add_their_key_here", + "GOOGLE_API_KEY": "user_will_add_their_key_here", + "MISTRAL_API_KEY": "user_will_add_their_key_here", + "OPENROUTER_API_KEY": "user_will_add_their_key_here", + "XAI_API_KEY": "user_will_add_their_key_here" + } + } + } +}src/utils/create-mcp-config.js (1)
69-109: Existing mcp.json entries won’t get timeout due to early return. Add timeout if missing on existing servers.Today, if any server already references task-master-ai in args, the function returns and never adds timeout, missing the PR’s objective for existing configs. Align with prior behavior (only set server.timeout; don’t overwrite env).
Apply this diff:
// Check if any existing server configuration already has task-master-ai in its args const hasMCPString = Object.values(mcpConfig.mcpServers).some( (server) => server.args && Array.isArray(server.args) && server.args.some( (arg) => typeof arg === 'string' && arg.includes('task-master-ai') ) ); - if (hasMCPString) { - log( - 'info', - 'Found existing task-master-ai MCP configuration in mcp.json, leaving untouched' - ); - return; // Exit early, don't modify the existing configuration - } + if (hasMCPString) { + // Add timeout to any server that references task-master-ai if missing + let updatedCount = 0; + Object.values(mcpConfig.mcpServers).forEach((server) => { + if ( + server?.args && + Array.isArray(server.args) && + server.args.some( + (arg) => typeof arg === 'string' && arg.includes('task-master-ai') + ) + ) { + if (typeof server.timeout !== 'number') { + server.timeout = 300; + updatedCount++; + } + } + }); + if (updatedCount > 0) { + fs.writeFileSync(mcpPath, formatJSONWithTabs(mcpConfig) + '\n'); + log( + 'info', + `Added timeout: 300 to ${updatedCount} MCP server(s) referencing task-master-ai` + ); + } else { + log( + 'info', + 'Existing task-master-ai MCP configuration already has timeout; no changes' + ); + } + return; + } // Add the task-master-ai server if it doesn't exist if (!mcpConfig.mcpServers['task-master-ai']) { mcpConfig.mcpServers['task-master-ai'] = newMCPServer['task-master-ai']; log( 'info', 'Added task-master-ai server to existing MCP configuration' ); } else { - log('info', 'task-master-ai server already configured in mcp.json'); + // Ensure timeout exists on the direct task-master-ai server without altering env + const existing = mcpConfig.mcpServers['task-master-ai']; + if (typeof existing.timeout !== 'number') { + existing.timeout = 300; + fs.writeFileSync(mcpPath, formatJSONWithTabs(mcpConfig) + '\n'); + log('info', 'Added timeout: 300 to existing task-master-ai server'); + } else { + log('info', 'task-master-ai server already configured with timeout'); + } } // Write the updated configuration - fs.writeFileSync(mcpPath, formatJSONWithTabs(mcpConfig) + '\n'); - log('success', 'Updated MCP configuration file'); + if (fs.existsSync(mcpPath)) { + // When we actually added/modified something above, the write already happened. + // If we added the server brand new, persist here. + if (!hasMCPString && mcpConfig.mcpServers['task-master-ai'] === newMCPServer['task-master-ai']) { + fs.writeFileSync(mcpPath, formatJSONWithTabs(mcpConfig) + '\n'); + log('success', 'Updated MCP configuration file'); + } + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.mcp.json(1 hunks).taskmaster/CLAUDE.md(1 hunks)apps/docs/archive/Installation.mdx(1 hunks)apps/docs/getting-started/quick-start/configuration-quick.mdx(1 hunks)apps/docs/getting-started/quick-start/installation.mdx(2 hunks)llms-install.md(1 hunks)public/assets/AGENTS.md(1 hunks)src/utils/create-mcp-config.js(1 hunks)tests/unit/profiles/selective-profile-removal.test.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.js: Declare and initialize global variables at the top of modules to avoid hoisting issues.
Use proper function declarations to avoid hoisting issues and initialize variables before they are referenced.
Do not reference variables before their declaration in module scope.
Use dynamic imports (import()) to avoid initialization order issues in modules.
Files:
src/utils/create-mcp-config.jstests/unit/profiles/selective-profile-removal.test.js
{src/utils/**,src/middleware/**}
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Test coverage for all code should meet or exceed 80% lines/functions and 70% branches globally; critical code (utils, middleware) should meet higher thresholds (90% utils, 85% middleware)
Files:
src/utils/create-mcp-config.js
tests/{unit,integration,e2e,fixtures}/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Test files must be organized as follows: unit tests in tests/unit/, integration tests in tests/integration/, end-to-end tests in tests/e2e/, and test fixtures in tests/fixtures/.
Files:
tests/unit/profiles/selective-profile-removal.test.js
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/git_workflow.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Create a test file and ensure all tests pass when all subtasks are complete; commit tests if added or modified
When all subtasks are complete, run final testing using the appropriate test runner (e.g., npm test, jest, or manual testing)
Files:
tests/unit/profiles/selective-profile-removal.test.js
**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.test.js: Never use asynchronous operations in tests. Make all mocks return synchronous values when possible.
Always mock tests properly based on the way the tested functions are defined and used.
Follow the test file organization: mocks must be set up before importing modules under test, and spies on mocked modules should be set up after imports.
Use fixtures from tests/fixtures/ for consistent sample data across tests.
Always declare mocks before importing the modules being tested in Jest test files.
Use jest.spyOn() after imports to create spies on mock functions and reference these spies in test assertions.
When testing functions with callbacks, get the callback from your mock's call arguments, execute it directly with test inputs, and verify the results.
For ES modules, use jest.mock() before static imports and jest.unstable_mockModule() before dynamic imports to mock dependencies.
Reset mock functions (mockFn.mockReset()) before dynamic imports if they might have been called previously.
When verifying console assertions, assert against the actual arguments passed (single formatted string), not multiple arguments.
Use mock-fs to mock file system operations in tests, and restore the file system after each test.
Mock API calls (e.g., Anthropic/Claude) by mocking the entire module and providing predictable responses.
Set mock environment variables in test setup and restore them after each test.
Maintain test fixtures separate from test logic.
Follow the mock-first-then-import pattern for all Jest mocks.
Do not define mock variables before jest.mock() calls (they won't be accessible due to hoisting).
Use test-specific file paths (e.g., 'test-tasks.json') for all file operations in tests.
Mock readJSON and writeJSON to avoid real file system interactions in tests.
Verify file operations use the correct paths in expect statements.
Use different file paths for each test to avoid test interdependence.
Verify modifications on the in-memory task objects passed to w...
Files:
tests/unit/profiles/selective-profile-removal.test.js
tests/unit/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
tests/unit/**/*.test.js: Unit tests must be located in tests/unit/, test individual functions and utilities in isolation, mock all external dependencies, and keep tests small, focused, and fast.
Do not include actual command execution in unit tests.
Files:
tests/unit/profiles/selective-profile-removal.test.js
tests/{unit,integration,e2e}/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
tests/{unit,integration,e2e}/**/*.test.js: When testing CLI commands built with Commander.js, test the command action handlers directly rather than trying to mock the entire Commander.js chain.
When mocking the Commander.js chain, mock ALL chainable methods (option, argument, action, on, etc.) and return this (or the mock object) from all chainable method mocks.
Explicitly handle all options, including defaults and shorthand flags (e.g., -p for --prompt), and include null/undefined checks in test implementations for parameters that might be optional.
Do not try to use the real action implementation without proper mocking, and do not mock Commander partially—either mock it completely or test the action directly.
Mock the action handlers for CLI commands and verify they're called with correct arguments.
Use sample task fixtures for consistent test data, mock file system operations, and test both success and error paths for task operations.
Mock console output and verify correct formatting in UI function tests. Use flexible assertions like toContain() or toMatch() for formatted output.
Mock chalk functions to return the input text to make testing easier while still verifying correct function calls.
Files:
tests/unit/profiles/selective-profile-removal.test.js
**/*.{test,spec}.*
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Test files should follow naming conventions: .test., .spec., or _test. depending on the language
Files:
tests/unit/profiles/selective-profile-removal.test.js
tests/{unit,integration,e2e}/**
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Organize test directories by test type (unit, integration, e2e) and mirror source structure where possible
Files:
tests/unit/profiles/selective-profile-removal.test.js
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: public/assets/AGENTS.md:0-0
Timestamp: 2025-09-22T16:42:54.285Z
Learning: Applies to public/assets/**/.mcp.json : Configure the Task Master MCP server in .mcp.json under mcpServers.task-master-ai with command npx and the listed env keys
Learnt from: olssonsten
PR: eyaltoledano/claude-task-master#1112
File: src/profiles/roo.js:0-0
Timestamp: 2025-08-12T01:22:48.873Z
Learning: In src/profiles/roo.js, the enhanceRooMCPConfiguration function was simplified to only set server.timeout = 300, removing problematic overwrites of server.env and alwaysAllow configurations that could interfere with user settings.
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1178
File: packages/tm-core/src/auth/config.ts:5-7
Timestamp: 2025-09-02T21:51:27.921Z
Learning: The user Crunchyman-ralph prefers not to use node: scheme imports (e.g., 'node:os', 'node:path') for Node.js core modules and considers suggestions to change bare imports to node: scheme as too nitpicky.
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: For CLI usage, install Taskmaster globally with `npm install -g task-master-ai` or use locally via `npx task-master-ai ...`.
Applied to files:
.taskmaster/CLAUDE.mdllms-install.mdsrc/utils/create-mcp-config.js.mcp.jsonapps/docs/archive/Installation.mdxpublic/assets/AGENTS.mdapps/docs/getting-started/quick-start/configuration-quick.mdxapps/docs/getting-started/quick-start/installation.mdx
📚 Learning: 2025-09-22T16:42:54.285Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: public/assets/AGENTS.md:0-0
Timestamp: 2025-09-22T16:42:54.285Z
Learning: Applies to public/assets/**/.mcp.json : Configure the Task Master MCP server in .mcp.json under mcpServers.task-master-ai with command npx and the listed env keys
Applied to files:
.taskmaster/CLAUDE.mdllms-install.mdsrc/utils/create-mcp-config.js.mcp.jsonapps/docs/archive/Installation.mdxpublic/assets/AGENTS.mdapps/docs/getting-started/quick-start/configuration-quick.mdxtests/unit/profiles/selective-profile-removal.test.jsapps/docs/getting-started/quick-start/installation.mdx
📚 Learning: 2025-07-18T17:10:12.881Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:12.881Z
Learning: Applies to .taskmaster/config.json : Store Taskmaster configuration settings (AI model selections, parameters, logging level, default subtasks/priority, project name, etc.) in the `.taskmaster/config.json` file located in the project root directory. Do not configure non-API key settings via environment variables.
Applied to files:
llms-install.md
📚 Learning: 2025-07-31T22:08:16.039Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/taskmaster.mdc:0-0
Timestamp: 2025-07-31T22:08:16.039Z
Learning: Applies to .taskmaster/config.json : All other Taskmaster settings (model choice, max tokens, temperature, log level, custom endpoints) are managed in .taskmaster/config.json via the task-master models command or models MCP tool.
Applied to files:
llms-install.md
📚 Learning: 2025-07-18T17:10:02.683Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-07-18T17:10:02.683Z
Learning: Applies to .taskmaster/config.json : Store Taskmaster configuration settings (AI model selections, parameters, logging level, default subtasks/priority, project name, tag management) in `.taskmaster/config.json` in the project root. Do not configure these via environment variables.
Applied to files:
llms-install.md.mcp.json
📚 Learning: 2025-07-31T22:08:16.039Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/taskmaster.mdc:0-0
Timestamp: 2025-07-31T22:08:16.039Z
Learning: Applies to .taskmaster/config.json : Do not manually edit the .taskmaster/config.json file; always use the provided CLI or MCP tools for configuration changes.
Applied to files:
src/utils/create-mcp-config.jsapps/docs/getting-started/quick-start/configuration-quick.mdx
📚 Learning: 2025-07-17T21:33:57.585Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1002
File: .changeset/puny-friends-give.md:2-3
Timestamp: 2025-07-17T21:33:57.585Z
Learning: In the eyaltoledano/claude-task-master repository, the MCP server code in mcp-server/src/ is part of the main "task-master-ai" package, not a separate "mcp-server" package. When creating changesets for MCP server changes, use "task-master-ai" as the package name.
Applied to files:
src/utils/create-mcp-config.js
📚 Learning: 2025-09-22T16:42:10.070Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: public/assets/.windsurfrules:0-0
Timestamp: 2025-09-22T16:42:10.070Z
Learning: Configure environment variables: ANTHROPIC_API_KEY (required), MODEL, MAX_TOKENS, TEMPERATURE, DEBUG, TASKMASTER_LOG_LEVEL, DEFAULT_SUBTASKS, DEFAULT_PRIORITY, PROJECT_NAME, PROJECT_VERSION, PERPLEXITY_API_KEY, PERPLEXITY_MODEL
Applied to files:
src/utils/create-mcp-config.jsapps/docs/archive/Installation.mdxapps/docs/getting-started/quick-start/installation.mdx
📚 Learning: 2025-08-12T01:22:48.873Z
Learnt from: olssonsten
PR: eyaltoledano/claude-task-master#1112
File: src/profiles/roo.js:0-0
Timestamp: 2025-08-12T01:22:48.873Z
Learning: In src/profiles/roo.js, the enhanceRooMCPConfiguration function was simplified to only set server.timeout = 300, removing problematic overwrites of server.env and alwaysAllow configurations that could interfere with user settings.
Applied to files:
tests/unit/profiles/selective-profile-removal.test.jsapps/docs/getting-started/quick-start/installation.mdx
📚 Learning: 2025-07-18T17:16:13.793Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-07-18T17:16:13.793Z
Learning: Applies to **/*.test.js : Do not import real AI service clients in tests; create fully mocked versions that return predictable responses.
Applied to files:
tests/unit/profiles/selective-profile-removal.test.js
📚 Learning: 2025-07-18T17:12:57.903Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-07-18T17:12:57.903Z
Learning: Applies to scripts/modules/**/*.test.js : Test CLI and MCP interfaces with real task data, verify end-to-end workflows across tag contexts, and test error scenarios and recovery in integration tests.
Applied to files:
tests/unit/profiles/selective-profile-removal.test.js
📚 Learning: 2025-07-18T17:11:36.732Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-07-18T17:11:36.732Z
Learning: Applies to mcp-server/src/tools/*.js : MCP tools should always call *Direct wrappers instead of executeTaskMasterCommand, except as a fallback if a direct function is not yet implemented.
Applied to files:
tests/unit/profiles/selective-profile-removal.test.js
🪛 markdownlint-cli2 (0.18.1)
llms-install.md
21-21: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 GitHub Actions: CI
tests/unit/profiles/selective-profile-removal.test.js
[error] 629-634: Formatter would reformat code. format-check detected changes: 1 error reported by the formatter (biome format). Please run 'biome format .' to fix formatting issues.
🔇 Additional comments (12)
apps/docs/archive/Installation.mdx (1)
23-23: LGTM: Added timeout to MCP config.Consistent with repo-wide change.
.taskmaster/CLAUDE.md (1)
89-89: LGTM: Documenting timeout is correct.Matches code and other docs.
tests/unit/profiles/selective-profile-removal.test.js (2)
630-633: LGTM: Include timeout in test MCP mock.Keeps mocks in sync with generated config.
1-1: Run biome format and commit the changes.
CI reported pending biome formatting;npm run formatexecuted (biome format . --write) and only printed npm deprecation warnings — commit the formatted files and re-run CI.public/assets/AGENTS.md (1)
89-89: LGTM: timeout documented in MCP example.Consistent with other examples.
llms-install.md (1)
21-21: LGTM: timeout added to MCP configuration.Docs match generator behavior.
apps/docs/getting-started/quick-start/installation.mdx (3)
73-74: LGTM: Cursor/Windsurf config includes timeout.Matches generator and PR intent.
102-104: LGTM: VS Code config includes timeout.Consistent with other examples.
73-74: Sanity-check alias consistency — unify "taskmaster-ai" vs "task-master-ai"Automated repo search couldn't be completed in this review environment; verify repo-wide and consolidate to a single alias (update deeplinks, MCP keys, and docs).
src/utils/create-mcp-config.js (1)
49-60: LGTM: Default timeout=300 in generated server config.Good default without touching env.
.mcp.json (1)
6-8: LGTM: Adds timeout to local MCP config.Matches generated config and docs.
apps/docs/getting-started/quick-start/configuration-quick.mdx (1)
23-23: Clarify timeout units — specify secondsTimeout values are in seconds across MCP clients (Cursor, Claude Desktop). Add a one-line note under the snippet in apps/docs/getting-started/quick-start/configuration-quick.mdx (line 23): "timeout is in seconds". Repo search for other "task-master-ai" examples couldn't complete in the sandbox — confirm and add units where missing.
| "taskmaster-ai": { | ||
| "command": "npx", | ||
| "args": ["-y", "task-master-ai"], | ||
| "timeout": 300, | ||
| "env": { |
There was a problem hiding this comment.
🧹 Nitpick
Unify MCP server alias across docs (“taskmaster-ai” vs “task-master-ai”).
Most docs use “task-master-ai”. Recommend aligning this example to avoid confusion.
Apply this diff:
- "mcpServers": {
- "taskmaster-ai": {
+ "mcpServers": {
+ "task-master-ai": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "taskmaster-ai": { | |
| "command": "npx", | |
| "args": ["-y", "task-master-ai"], | |
| "timeout": 300, | |
| "env": { | |
| "mcpServers": { | |
| "task-master-ai": { | |
| "command": "npx", | |
| "args": ["-y", "task-master-ai"], | |
| "timeout": 300, | |
| "env": { |
🤖 Prompt for AI Agents
In apps/docs/archive/Installation.mdx around lines 20 to 24, the MCP server
alias is inconsistent with the rest of the docs ("taskmaster-ai" vs
"task-master-ai"); update the key name "taskmaster-ai" to "task-master-ai" in
this JSON example so it matches the canonical alias used elsewhere in the
documentation.
Crunchyman-ralph
force-pushed
the
ralph/fix/mcp.timeout.issues
branch
from
548b6de to
c2fc61d
Compare
1 participant
What type of PR is this?
Description
Related Issues
How to Test This
# Example commands or stepsExpected result:
Contributor Checklist
npm run changesetnpm testnpm run format-check(ornpm run formatto fix)Changelog Entry
For Maintainers
Summary by CodeRabbit
New Features
Documentation
Tests
Chores