chore: apply nitpicks of 0.36.0 post-merge#1459
chore: apply nitpicks of 0.36.0 post-merge#1459Crunchyman-ralph wants to merge 1 commit intonextfrom
Conversation
|
WalkthroughThis pull request introduces timeout handling for npm install operations, adds query parameter support to tarball download requests, enhances MCP response validation, improves task service initialization flow, adds a new "review" status for tasks, extends error handling tests, and refactors integration tests to use parameterization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (4)
packages/ai-sdk-provider-grok-cli/src/errors.test.ts (1)
53-60: LGTM! Test adds valuable coverage for promptExcerpt behavior.The new test correctly verifies that
requestBodyValuesis populated whenpromptExcerptis provided.Consider enhancing the "minimal parameters" test (lines 43-51) to also verify the
requestBodyValuesbehavior when nopromptExcerptis provided, ensuring complete coverage of both positive and negative cases:it('should create APICallError with minimal parameters', () => { const error = createAPICallError({ message: 'Simple error' }); expect(error).toBeInstanceOf(APICallError); expect(error.message).toBe('Simple error'); expect(error.isRetryable).toBe(false); + expect(error.requestBodyValues).toEqual({}); });tests/integration/profiles/hamster-rules-distribution.test.js (1)
114-127: Consider using synchronous profile access for consistency.The parameterization is correctly implemented. However, the dynamic import could be replaced with the synchronous pattern already used in
getExpectedHamsterPath(line 34), making the test consistent with the coding guidelines preference for synchronous operations in tests.Apply this diff to use synchronous access:
- test.each(PROFILES_WITHOUT_DEFAULT_RULES)( - '%s profile does not use defaultFileMap (has custom mechanism)', - async (profile) => { - // Dynamically import the profile to check its configuration - const profileModule = await import( - `../../../src/profiles/${profile}.js` - ); - const profileExport = profileModule[`${profile}Profile`]; + test.each(PROFILES_WITHOUT_DEFAULT_RULES)( + '%s profile does not use defaultFileMap (has custom mechanism)', + (profile) => { + // Access the profile configuration from the already-imported module + const profileExport = profilesModule[`${profile}Profile`]; // These profiles should have includeDefaultRules: false // which means they won't automatically get hamster files via defaultFileMap expect(profileExport.includeDefaultRules).toBe(false); } );packages/tm-core/src/modules/tasks/services/task-service.ts (1)
812-818: Consider resetting the initialized flag even when storage is null.The early return on line 812 prevents the initialized flag from being reset when storage is null. While this edge case is unlikely under normal operation, defensive programming suggests always resetting the flag in
close()to ensure the service can recover from partial initialization failures.Apply this diff to always reset the initialized flag:
async close(): Promise<void> { - if (!this.storage) return; - try { - await this.storage.close(); + if (this.storage) { + await this.storage.close(); + } } finally { this.initialized = false; } }apps/cli/src/utils/auto-update/install.ts (1)
29-31: Timeout +completedguards make installs robust against hangs and double resolution.This is a solid pattern:
- A single
INSTALL_TIMEOUT_MSconstant shared across both install paths keeps behavior consistent.- In both
installFromTarballandperformDirectNpmInstall, thecompletedflag plus timeout ensures:
- Long‑running or stuck npm installs are terminated after 5 minutes with clear user feedback.
close/errorhandlers and the timeout handler never race into multipleresolve(...)calls.- Intervals, timeouts, progress bar, and spinner are all cleaned up in every exit path.
Two optional follow‑ups you might consider:
- Factor the timeout/
completedpattern into a small helper.
The logic in both functions is nearly identical; extracting a tiny utility (e.g., wrapping a child process with a timeout andcompletedguard) would reduce duplication and make future tweaks safer.- Align new logging with the central log utility and silent mode.
Newconsole.log/console.errorcalls in the timeout branches follow the existing style in this file, but project guidelines prefer going through the shared logger so silent mode and log levels are respected. Not a blocker for this PR, but worth a later cleanup pass. As per coding guidelines, this would centralize behavior and avoid direct console usage in CLI utilities.Also applies to: 158-173, 208-212, 250-253, 297-312, 323-327, 349-353
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
apps/cli/src/utils/auto-update/changelog.ts(1 hunks)apps/cli/src/utils/auto-update/download.ts(1 hunks)apps/cli/src/utils/auto-update/install.ts(7 hunks)apps/mcp/tests/integration/tools/generate.tool.test.ts(1 hunks)packages/ai-sdk-provider-grok-cli/src/errors.test.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts(1 hunks)packages/tm-core/src/modules/tasks/services/task-service.ts(1 hunks)tests/integration/profiles/hamster-rules-distribution.test.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
TypeScript test files must achieve minimum code coverage thresholds: 80% lines/functions and 70% branches globally, 90% for utilities, and 85% for middleware; new features must meet or exceed these thresholds
Files:
packages/tm-core/src/modules/tasks/services/task-file-generator.service.tspackages/ai-sdk-provider-grok-cli/src/errors.test.tsapps/cli/src/utils/auto-update/changelog.tspackages/tm-core/src/modules/tasks/services/task-service.tsapps/cli/src/utils/auto-update/download.tsapps/cli/src/utils/auto-update/install.tsapps/mcp/tests/integration/tools/generate.tool.test.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts}: Import and use specific getters from config-manager.js (e.g., getMainProvider(), getLogLevel(), getMainMaxTokens()) to access configuration values needed for application logic
Use isApiKeySet(providerName, session) from config-manager.js to check if a provider's key is available before potentially attempting an AI call
Do not add direct console.log calls outside the logging utility - use the central log function instead
Ensure silent mode is disabled in a finally block to prevent it from staying enabled
Do not access the global silentMode variable directly - use the exported silent mode control functions instead
Do not duplicate task ID formatting logic across modules - centralize formatting utilities
Use ContextGatherer class from utils/contextGatherer.js for AI-powered commands that need project context, supporting tasks, files, custom text, and project tree context
Use FuzzyTaskSearch class from utils/fuzzyTaskSearch.js for automatic task relevance detection with configurable search parameters
Use fuzzy search to supplement user-provided task IDs and display discovered task IDs to users for transparency
Do not replace explicit user task selections with fuzzy results - fuzzy search should supplement, not replace user selections
Use readJSON and writeJSON utilities for all JSON file operations instead of raw fs.readFileSync or fs.writeFileSync
Include error handling for JSON file operations and validate JSON structure after reading
Use path.join() for cross-platform path construction and path.resolve() for absolute paths, validating paths before file operations
Support both .env files and MCP session environment for environment variable resolution with fallbacks for missing values
Prefer updating the core function to accept an outputFormat parameter and check outputFormat === 'json' before displaying UI elements
Files:
packages/tm-core/src/modules/tasks/services/task-file-generator.service.tspackages/ai-sdk-provider-grok-cli/src/errors.test.tsapps/cli/src/utils/auto-update/changelog.tspackages/tm-core/src/modules/tasks/services/task-service.tsapps/cli/src/utils/auto-update/download.tsapps/cli/src/utils/auto-update/install.tstests/integration/profiles/hamster-rules-distribution.test.jsapps/mcp/tests/integration/tools/generate.tool.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import modules with
.jsextension even in TypeScript source files for ESM compatibility
Files:
packages/tm-core/src/modules/tasks/services/task-file-generator.service.tspackages/ai-sdk-provider-grok-cli/src/errors.test.tsapps/cli/src/utils/auto-update/changelog.tspackages/tm-core/src/modules/tasks/services/task-service.tsapps/cli/src/utils/auto-update/download.tsapps/cli/src/utils/auto-update/install.tsapps/mcp/tests/integration/tools/generate.tool.test.ts
**/*.test.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/new_features.mdc)
**/*.test.{js,ts}: Follow the mock-first-then-import pattern for Jest mocking; use jest.spyOn() for spy functions; clear mocks between tests; verify mocks with the pattern described intests.mdc
Test new features with both legacy and tagged task data formats; verify tag resolution behavior; test migration compatibility; ensure pre-migration projects are handled correctly
Files:
packages/ai-sdk-provider-grok-cli/src/errors.test.tstests/integration/profiles/hamster-rules-distribution.test.jsapps/mcp/tests/integration/tools/generate.tool.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
Unit tests must follow the describe/it pattern, use beforeEach for mock setup with jest.clearAllMocks(), include specific assertions with expect(), and test both success and error scenarios including edge cases
Files:
packages/ai-sdk-provider-grok-cli/src/errors.test.tsapps/mcp/tests/integration/tools/generate.tool.test.ts
**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,test}.ts: Always use.tsfor TypeScript tests, never.js
NEVER use async/await in test functions unless testing actual asynchronous operations; use synchronous top-level imports instead of dynamicawait import()
Files:
packages/ai-sdk-provider-grok-cli/src/errors.test.tsapps/mcp/tests/integration/tools/generate.tool.test.ts
**/{utils,utilities,helpers}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities,helpers}/**/*.{js,ts}: Document all parameters and return values in JSDoc format, include descriptions for complex logic, and add examples for non-obvious usage
Support multiple log levels (debug, info, warn, error) with appropriate icons for different log levels and respect the configured log level
Files:
apps/cli/src/utils/auto-update/changelog.tsapps/cli/src/utils/auto-update/download.tsapps/cli/src/utils/auto-update/install.ts
**/{utils,utilities}/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/{utils,utilities}/**/*.{js,ts}: Use try/catch blocks for all file operations and return null or a default value on failure rather than allowing exceptions to propagate unhandled
Log detailed error information using the log utility in catch blocks for file operations
Create utilities for consistent task ID handling that support different ID formats (numeric, string, dot notation)
Implement reusable task finding utilities that support both task and subtask lookups and add context to subtask results
Implement cycle detection using graph traversal by tracking visited nodes and recursion stack, returning specific information about cycles
Detect circular dependencies using DFS and validate task references before operations
Export all utility functions explicitly in logical groups and include configuration constants from utility modules
Do not use default exports in utility modules - use named exports only
Group related exports together in utility modules and avoid creating circular dependencies
Make the log function respect silent mode by skipping logging when silent mode is enabled
Files:
apps/cli/src/utils/auto-update/changelog.tsapps/cli/src/utils/auto-update/download.tsapps/cli/src/utils/auto-update/install.ts
apps/cli/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
CLI (@tm/cli) should be a thin presentation layer that calls tm-core methods and displays results; handle only CLI-specific concerns like argument parsing, output formatting, and user prompts
Files:
apps/cli/src/utils/auto-update/changelog.tsapps/cli/src/utils/auto-update/download.tsapps/cli/src/utils/auto-update/install.ts
**/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
**/*.js: Always use isSilentMode() function to check current silent mode status instead of directly accessing the global silentMode variable or global.silentMode
Use try/finally block pattern when wrapping core function calls with enableSilentMode/disableSilentMode to ensure silent mode is always restored, even if errors occur
For functions that need to handle both a passed silentMode parameter and check global state, check both the function parameter and global state: const isSilent = options.silentMode || (typeof options.silentMode === 'undefined' && isSilentMode())
Functions should accept their dependencies as parameters rather than using globals to promote testability and explicit dependency injection
Define callbacks as separate functions for easier testing rather than inline functions
Files:
tests/integration/profiles/hamster-rules-distribution.test.js
tests/integration/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Integration tests should be located in tests/integration/ and test interactions between modules
Files:
tests/integration/profiles/hamster-rules-distribution.test.js
**/*.{js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/test_workflow.mdc)
JavaScript test files using Jest must follow the same testing patterns as TypeScript files, include proper mocking of external dependencies, and achieve the same coverage thresholds
Files:
tests/integration/profiles/hamster-rules-distribution.test.js
**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
**/*.test.js: Never use asynchronous operations in tests. Always mock tests properly based on the way the tested functions are defined and used.
Follow Jest test file structure: 1) Imports, 2) Mock setup before importing modules under test, 3) Import modules after mocks, 4) Set up spies on mocked modules, 5) Describe suite with descriptive name, 6) Setup/teardown hooks, 7) Grouped tests for related functionality, 8) Individual test cases with clear descriptions using Arrange-Act-Assert pattern.
When testing CLI commands built with Commander.js: test command action handlers directly rather than mocking the entire Commander chain; create simplified test-specific implementations of command handlers; explicitly handle all options including defaults and shorthand flags; include null/undefined checks for optional parameters; use fixtures for consistent sample data.
Usejest.mock()before any imports. Jest hoists mock calls to the top of the file. Always declare mocks before importing modules being tested. Use factory pattern for complex mocks that need access to other variables.
When testing ES modules with dynamic imports, usejest.unstable_mockModule()beforeawait import(). Include__esModule: truein mock factories. Reset mock functions before dynamic import. Mock named and default exports as needed.
Mock file system operations usingmock-fslibrary. Mock API calls by providing jest.fn() implementations that return expected structures. Mock environment variables in test setup.
When testing functions with callbacks: get the callback from mock's call arguments usingmock.calls[index][argIndex]; execute it directly with test inputs; verify results match expectations.
For task file operations in tests: use test-specific file paths (e.g., 'test-tasks.json'); mockreadJSONandwriteJSONto avoid real file system interactions; verify file operations use correct paths; use different paths for each test; verify modifications on in-memory task objects passed t...
Files:
tests/integration/profiles/hamster-rules-distribution.test.js
tests/integration/**/*.test.js
📄 CodeRabbit inference engine (.cursor/rules/tests.mdc)
Locate integration tests in
tests/integration/directory. Test interactions between modules, focus on component interfaces rather than implementation details, use more realistic but controlled test environments.
Files:
tests/integration/profiles/hamster-rules-distribution.test.js
**/tests/integration/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place package and app integration tests in
packages/<package-name>/tests/integration/<module>/<file>.test.tsorapps/<app-name>/tests/integration/<module>/<file>.test.ts
Files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
apps/mcp/**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In unit tests for apps/mcp, mock tm-core responses but use real MCP framework to test response formatting
Files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
**/tests/integration/**/*.{spec,test}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In integration tests, use real tm-core and mock only external boundaries (APIs, DB, filesystem)
Files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
🧠 Learnings (39)
📓 Common learnings
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/git_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:23.000Z
Learning: Pull Request descriptions must include: Task Overview, Subtasks Completed (checklist), Implementation Details, Testing approach, Breaking Changes (if any), and Related Tasks.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1069
File: .changeset/fix-tag-complexity-detection.md:0-0
Timestamp: 2025-08-02T15:33:22.656Z
Learning: For changeset files (.changeset/*.md), Crunchyman-ralph prefers to ignore formatting nitpicks about blank lines between frontmatter and descriptions, as he doesn't mind having them and wants to avoid such comments in future reviews.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.805Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 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.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1132
File: .github/workflows/weekly-metrics-discord.yml:81-93
Timestamp: 2025-08-13T22:10:46.958Z
Learning: Crunchyman-ralph ignores YAML formatting nitpicks about trailing spaces when there's no project-specific YAML formatter configured, preferring to focus on functionality over cosmetic formatting issues.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1200
File: src/ai-providers/custom-sdk/grok-cli/language-model.js:96-100
Timestamp: 2025-09-19T16:06:42.182Z
Learning: The user Crunchyman-ralph prefers to keep environment variable names explicit (like GROK_CLI_API_KEY) rather than supporting multiple aliases, to avoid overlap and ensure clear separation between different CLI implementations.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1105
File: scripts/modules/supported-models.json:242-254
Timestamp: 2025-08-08T11:33:15.297Z
Learning: Preference: In scripts/modules/supported-models.json, the "name" field is optional. For OpenAI entries (e.g., "gpt-5"), Crunchyman-ralph prefers omitting "name" when the id is explicit enough; avoid nitpicks requesting a "name" in such cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1178
File: packages/tm-core/src/subpath-exports.test.ts:6-9
Timestamp: 2025-09-03T12:45:30.724Z
Learning: The user Crunchyman-ralph prefers to avoid overly nitpicky or detailed suggestions in code reviews, especially for test coverage of minor import paths. Focus on more substantial issues rather than comprehensive coverage of all possible edge cases.
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1217
File: apps/cli/src/index.ts:16-21
Timestamp: 2025-09-18T16:35:35.147Z
Learning: The user Crunchyman-ralph considers suggestions to export types for better ergonomics (like exporting UpdateInfo type alongside related functions) as nitpicky and prefers not to implement such suggestions.
📚 Learning: 2025-11-24T18:02:36.361Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-11-24T18:02:36.361Z
Learning: Applies to scripts/modules/task-manager.js : Format task files with consistent structure including task metadata (ID, title, status), dependencies with status indicators, and tag context information in the file header
Applied to files:
packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts
📚 Learning: 2025-11-24T18:04:43.949Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.949Z
Learning: Applies to scripts/modules/**/*.{js,ts} : Implement silent migration for tasks.json files that transforms old format to tagged format, marking global flag and performing complete migration
Applied to files:
packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts
📚 Learning: 2025-11-24T17:57:14.728Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-11-24T17:57:14.728Z
Learning: Applies to tests/unit/ai-providers/*.test.js : Create unit tests in `tests/unit/ai-providers/<provider-name>.test.js` that mock the provider's AI SDK module and test each exported function for correct client instantiation, parameter passing, result handling, and error handling
Applied to files:
packages/ai-sdk-provider-grok-cli/src/errors.test.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : When testing exact error messages, don't assert on exact error message text that might change. Instead test for error presence and general properties using patterns like `toContain` or `toMatch`.
Applied to files:
packages/ai-sdk-provider-grok-cli/src/errors.test.ts
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Do not import or instantiate real AI service clients. Create fully mocked versions that return predictable responses. Mock the entire module with controlled behavior.
Applied to files:
packages/ai-sdk-provider-grok-cli/src/errors.test.ts
📚 Learning: 2025-11-24T17:57:14.728Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/ai_providers.mdc:0-0
Timestamp: 2025-11-24T17:57:14.728Z
Learning: Applies to src/ai-providers/*.js : Provider functions must include basic validation and try/catch error handling
Applied to files:
packages/ai-sdk-provider-grok-cli/src/errors.test.ts
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to **/*.test.ts : Unit tests must follow the describe/it pattern, use beforeEach for mock setup with jest.clearAllMocks(), include specific assertions with expect(), and test both success and error scenarios including edge cases
Applied to files:
packages/ai-sdk-provider-grok-cli/src/errors.test.tstests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-25T18:32:29.805Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1444
File: apps/cli/src/utils/auto-update/changelog.ts:103-111
Timestamp: 2025-11-25T18:32:29.805Z
Learning: The claude-task-master project uses a custom changelog format with PR numbers and author acknowledgements in the pattern `- [#PR](...) Thanks [author]! - Description`, which is parsed by the regex in apps/cli/src/utils/auto-update/changelog.ts.
Applied to files:
apps/cli/src/utils/auto-update/changelog.ts
📚 Learning: 2025-11-24T18:00:32.587Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-11-24T18:00:32.587Z
Learning: Refer to changeset.mdc for guidelines on using Changesets (npm run changeset) to manage versioning and changelogs
Applied to files:
apps/cli/src/utils/auto-update/changelog.ts
📚 Learning: 2025-08-06T21:14:23.071Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1091
File: .changeset/wide-actors-report.md:0-0
Timestamp: 2025-08-06T21:14:23.071Z
Learning: For changeset files (.changeset/*.md), the first line after the frontmatter must be a plain, unstyled summary line that gets integrated directly into the changelog. Do not add markdown headings or styling as this would interfere with the changelog generation process. Ignore markdownlint MD041 rule for these files.
Applied to files:
apps/cli/src/utils/auto-update/changelog.ts
📚 Learning: 2025-11-24T17:58:47.001Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-11-24T17:58:47.001Z
Learning: Applies to scripts/modules/commands.js : Implement version checking to notify users of available updates using non-blocking checks that don't delay command execution, displaying notifications after command completion
Applied to files:
apps/cli/src/utils/auto-update/install.ts
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to **/*.test.js : When testing CLI commands built with Commander.js: test command action handlers directly rather than mocking the entire Commander chain; create simplified test-specific implementations of command handlers; explicitly handle all options including defaults and shorthand flags; include null/undefined checks for optional parameters; use fixtures for consistent sample data.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Create simplified test functions instead of complex setups: create simplified versions of complex functions focusing on core logic; remove file system operations, API calls, and external dependencies; pass all dependencies as parameters.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:01:44.137Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.137Z
Learning: Applies to **/*.test.{js,ts} : Test new features with both legacy and tagged task data formats; verify tag resolution behavior; test migration compatibility; ensure pre-migration projects are handled correctly
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to **/*.test.js : For task file operations in tests: use test-specific file paths (e.g., 'test-tasks.json'); mock `readJSON` and `writeJSON` to avoid real file system interactions; verify file operations use correct paths; use different paths for each test; verify modifications on in-memory task objects passed to `writeJSON`.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to tests/integration/**/*.test.js : Locate integration tests in `tests/integration/` directory. Test interactions between modules, focus on component interfaces rather than implementation details, use more realistic but controlled test environments.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to tests/fixtures/**/* : Provide reusable test data in fixtures located in `tests/fixtures/`. Keep fixtures small and representative, export fixtures as named exports for reuse.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Make async operations synchronous in tests. Mock async functions to return synchronous values when possible. Don't use real async/await or Promise resolution that might fail unpredictably.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:46.699Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.699Z
Learning: Applies to tests/unit/**/*.test.js : Locate unit tests in `tests/unit/` directory. Test individual functions and utilities in isolation, mock all external dependencies, keep tests small and focused.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:00:06.781Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/dev_workflow.mdc:0-0
Timestamp: 2025-11-24T18:00:06.781Z
Learning: Applies to {.cursor/rules/**/*,.roo/rules/**/*} : Create rule files in profile-specific directories following internal guidelines (e.g., `.cursor/rules`, `.roo/rules`)
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T22:09:45.426Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.426Z
Learning: Applies to **/*.{spec,test}.ts : NEVER use async/await in test functions unless testing actual asynchronous operations; use synchronous top-level imports instead of dynamic `await import()`
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:13.408Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/test_workflow.mdc:0-0
Timestamp: 2025-11-24T18:03:13.408Z
Learning: Applies to tests/teardown.ts : Global teardown file must be created to handle cleanup after all tests complete and prevent worker process leaks
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T18:03:46.700Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/tests.mdc:0-0
Timestamp: 2025-11-24T18:03:46.700Z
Learning: Applies to **/*.test.js : Use dynamic imports (`import()`) to avoid initialization order issues. Structure modules to avoid circular dependencies. Consider factory functions for modules with complex state.
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T17:58:07.977Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-11-24T17:58:07.977Z
Learning: Each module should have well-defined exports that can be mocked in tests
Applied to files:
tests/integration/profiles/hamster-rules-distribution.test.js
📚 Learning: 2025-11-24T22:09:45.426Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.426Z
Learning: Applies to apps/mcp/src/**/*.{ts,tsx} : MCP (tm/mcp) should be a thin presentation layer that calls tm-core methods and returns MCP-formatted responses; handle only MCP-specific concerns like tool schemas, parameter validation, and response formatting
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T22:09:45.426Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T22:09:45.426Z
Learning: Applies to apps/mcp/**/*.{spec,test}.ts : In unit tests for apps/mcp, mock tm-core responses but use real MCP framework to test response formatting
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-26T21:57:48.917Z
Learnt from: Crunchyman-ralph
Repo: eyaltoledano/claude-task-master PR: 1451
File: apps/mcp/src/tools/tasks/set-task-status.tool.ts:23-25
Timestamp: 2025-11-26T21:57:48.917Z
Learning: Applies to apps/mcp/src/tools/**/*.ts : In the new apps/mcp MCP tool architecture, projectRoot is a required parameter in Zod schemas (not optional like in the legacy mcp-server structure)
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:01:44.137Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/new_features.mdc:0-0
Timestamp: 2025-11-24T18:01:44.137Z
Learning: Applies to mcp-server/src/tools/*.js : Create MCP tool definitions in `mcp-server/src/tools/` using kebab-case naming; use zod for parameter validation; make projectRoot optional as the HOF handles fallback; wrap execute method with `withNormalizedProjectRoot`
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:04:43.949Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.949Z
Learning: Applies to mcp-server/src/tools/**/*.{js,ts} : Use processMCPResponseData(taskOrData, fieldsToRemove) utility to filter potentially sensitive or large fields (like details, testStrategy) from task objects before sending responses
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T17:58:07.977Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-11-24T17:58:07.977Z
Learning: New MCP tools should be imported and registered in mcp-server/src/tools/index.js and tool definitions should be added to mcp.json
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:01:06.046Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-11-24T18:01:06.046Z
Learning: Applies to mcp-server/src/tools/*.js : Use createErrorResponse and createContentResponse utilities from tools/utils.js for formatting MCP responses
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:01:06.046Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-11-24T18:01:06.046Z
Learning: Applies to mcp-server/src/tools/*.js : Include projectRoot as an optional parameter in tool definitions using zod schema
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:02:49.769Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/telemetry.mdc:0-0
Timestamp: 2025-11-24T18:02:49.769Z
Learning: Applies to mcp-server/src/tools/**/*.js : MCP tools in mcp-server/src/tools/ must call the corresponding direct function wrapper and pass the result object to handleApiResult(result, log) from mcp-server/src/tools/utils.js
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:04:43.949Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.949Z
Learning: Applies to mcp-server/src/tools/**/*.{js,ts} : Use handleApiResult(result, log, errorPrefix, processFunction) to standardize the formatting of responses returned by direct functions into the MCP response format
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:01:06.046Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-11-24T18:01:06.046Z
Learning: Applies to mcp-server/src/tools/*.js : Use handleApiResult utility to format the result from a *Direct function into a standard MCP response, which automatically handles data processing via processMCPResponseData
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:04:43.949Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-11-24T18:04:43.949Z
Learning: Applies to mcp-server/src/**/*.{js,ts} : Use createContentResponse(content) and createErrorResponse(errorMessage) helper functions to create basic MCP response structures
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-11-24T18:01:06.046Z
Learnt from: CR
Repo: eyaltoledano/claude-task-master PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-11-24T18:01:06.046Z
Learning: Applies to mcp-server/src/core/direct-functions/*.js : Wrap core function calls and AI calls in try/catch blocks, log errors with appropriate severity and context, and return standardized error objects with code and message
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
📚 Learning: 2025-10-12T11:06:36.374Z
Learnt from: rtmcrc
Repo: eyaltoledano/claude-task-master PR: 933
File: mcp-server/src/tools/agent-llm.js:1-3
Timestamp: 2025-10-12T11:06:36.374Z
Learning: The agent_llm MCP tool in mcp-server/src/tools/agent-llm.js is an exception to the standard MCP tool pattern. It should NOT use handleApiResult because it handles delegated LLM calls rather than direct API calls. Using createErrorResponse for error messages is the correct approach for this tool.
Applied to files:
apps/mcp/tests/integration/tools/generate.tool.test.ts
🪛 ast-grep (0.40.0)
apps/cli/src/utils/auto-update/changelog.ts
[warning] 73-76: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
## ${version.replace(/\./g, '\\.')}\\s*\\n,
'i'
)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (5)
packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts (1)
325-341: Newreviewstatus mapping looks consistent; keep type and docs in syncThe added
'review'case with the eye symbol fits the existing status→symbol pattern and will correctly surface into dependency status display. Please just confirm that:
TaskStatusincludes'review', and- any user-facing docs or status filters that enumerate statuses are updated accordingly.
apps/mcp/tests/integration/tools/generate.tool.test.ts (1)
80-84: LGTM! Good defensive validation.This guard prevents runtime errors if the MCP inspector returns an unexpected response format, and the serialized error message will aid debugging.
tests/integration/profiles/hamster-rules-distribution.test.js (1)
73-108: LGTM! Parameterization correctly implemented.The refactoring to use
test.each()is well done. The test maintains all the original logic while reducing code duplication, and the cleanup is properly handled in the finally block.apps/cli/src/utils/auto-update/changelog.ts (1)
73-77: Semver pre‑validation makes the dynamic regex safe (static warning is a false positive).Given the strict semver check above (
^\d+\.\d+\.\d+(-[a-zA-Z0-9.-]+)?$), theversionused inversionRegexis limited to digits, dots, hyphen, and alphanumerics, so there are no user‑supplied metacharacters to drive a ReDoS. The new comment correctly documents this mitigation and keeps the code aligned with the static-analysis hint. Just make sure any future change to the semver pattern keeps that guarantee in mind.apps/cli/src/utils/auto-update/download.ts (1)
102-105: Includingurl.searchin the request path correctly preserves query params.Using
url.pathname + url.searchensures signed/CDN tarball URLs with query strings keep working, while remaining backward compatible when there is no query string.
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
Bug Fixes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.