Improve cross-tag move UX and safety; add MCP suggestions and CLI tips#1135
Conversation
…--with-dependencies/--ignore-dependencies - scripts/modules/ui.js: drop force tip in conflict resolution - scripts/modules/commands.js: remove force examples from move help - docs/cross-tag-task-movement.md: purge force mentions; add explicit with/ignore examples
…ignore deps behavior and current-tag fallback - CLI integration: remove force expectations, keep with/ignore, current-tag fallback - Integration: remove force-path test - Unit: add scoped traversal test, adjust fixtures to avoid id collision
…-dependencies filtering - resolveDependencies: traverse only sourceTag tasks to avoid cross-tag contamination - filter dependent IDs to those present in source tag, numeric only - ignore-dependencies: drop deps pointing to tasks from sourceTag; keep targetTag deps
…returns conflict suggestions - new test: tests/unit/mcp/tools/move-task-cross-tag-options.test.js
…dd integration test case
…t Next Steps tips when core returns them (e.g., after ignore-dependencies)\n- Add dedicated help block when an ID already exists in target tag
…clude suggestions on TASK_ALREADY_EXISTS errors\n- MCP: map ID collision to TASK_ALREADY_EXISTS with suggestions\n- Tests: add MCP unit test for ID collision suggestions
…ollision suggestions\n\n- CLI integration test: assert Next Steps tips printed when result.tips present\n- Integration test: assert TASK_ALREADY_EXISTS error includes suggestions payload
…LI/MCP/core/tests)
- Introduced a detailed help command for cross-tag task movement, enhancing user guidance on usage and options. - Updated validation logic in `validateCrossTagMove` to include checks for indirect dependencies, improving accuracy in conflict detection. - Refactored tests to ensure comprehensive coverage of new validation scenarios and error handling. - Cleaned up documentation to reflect the latest changes in task movement functionality.
- Eliminated duplicate printing of tips for next steps after the move operation, streamlining the output for users. - This change enhances clarity by ensuring tips are only displayed when relevant, improving overall user experience.
- Updated documentation to replace the deprecated "force move" concept with clear alternatives: `--with-dependencies` and `--ignore-dependencies`. - Enhanced Scenario 3 with explicit options and improved inline comments for better readability. - Removed confusing commented code in favor of a straightforward note in the Force Move section.
🦋 Changeset detectedLatest commit: ea547f8 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughDeprecates force-based cross-tag moves in docs/UI, replaces with explicit --with-dependencies / --ignore-dependencies, scopes dependency resolution to the source tag, surfaces advisory "Next Steps" tips for ignored-dependencies, and enriches ID-collision and dependency-conflict errors with structured suggestions. Tests and changesets updated. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI (commands.js)
participant TM as TaskManager (move-task.js)
participant DM as DependencyResolver
User->>CLI: move --from <tagA> --to <tagB> [--with-dependencies / --ignore-dependencies]
CLI->>TM: moveTasksBetweenTags(taskIds, from, to, opts)
TM->>DM: resolve dependencies (scoped to source tag)
DM-->>TM: deps / conflicts
alt ignore-dependencies with conflicts
TM-->>CLI: result { tips: [validate-dependencies, fix-dependencies] }
CLI->>User: print success + "Next Steps" tips
else with-dependencies or no conflicts
TM-->>CLI: success
CLI->>User: print success
end
sequenceDiagram
participant CLI
participant TM
CLI->>TM: moveTasksBetweenTags(...)
TM-->>CLI: error TASK_ALREADY_EXISTS { suggestions[...] }
CLI->>User: Conflict: ID already exists in target tag + suggestions
sequenceDiagram
participant Client
participant MCP as MCP direct fn
participant Core as moveTasksBetweenTags
Client->>MCP: cross-tag move request
MCP->>Core: invoke move
alt Dependency conflicts
Core-->>MCP: error CROSS_TAG_DEPENDENCY_CONFLICTS + metadata
MCP-->>Client: failure CROSS_TAG_DEPEND_CONFLICT + suggestions
else ID collision
Core-->>MCP: error TASK_ALREADY_EXISTS + suggestions
MCP-->>Client: failure TASK_ALREADY_EXISTS + suggestions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 17
🔭 Outside diff range comments (6)
.changeset/clarify-force-move-docs.md (1)
11-11: Remove stray trailing content.There’s an orphaned “11” at EOF that will render oddly and can break tooling.
Apply this diff to delete it:
-11mcp-server/src/core/direct-functions/move-task-cross-tag.js (4)
97-99: Preserve and restore the prior silent mode state.Current logic always enables and then disables silent mode, which can disrupt callers if silent mode was already enabled.
Proposed refactor (outside this hunk):
// before entering the try that calls core const wasSilent = isSilentMode(); if (!wasSilent) enableSilentMode(); try { // ... } finally { if (!wasSilent) { disableSilentMode(); } }Also import
isSilentModefromscripts/modules/utils.js.
110-118: Pass mcpLog wrapper, commandName, outputType, and outputFormat to core; include telemetryData in response.Direct functions must:
- Wrap and pass the FastMCP logger via a safe wrapper as
mcpLog.- Pass
{ commandName: 'move-task-cross-tag', outputType: 'mcp' }in context andoutputFormat: 'json'if supported.- Return
coreResult.telemetryDataindata.Example refactor (outside this hunk):
// Create a minimal logger wrapper to avoid "mcpLog[level] is not a function" const mcpLog = { info: (...a) => log.info?.(...a), warn: (...a) => log.warn?.(...a), error: (...a) => log.error?.(...a), debug: (...a) => log.debug?.(...a), }; const result = await moveTasksBetweenTags( tasksPath, sourceIds, args.sourceTag, args.targetTag, moveOptions, { projectRoot, mcpLog, commandName: 'move-task-cross-tag', outputType: 'mcp', outputFormat: 'json', } ); // later in success return data: { ...result, telemetryData: result.telemetryData, // ... }
6-6: Use the canonical path resolver per guidelines.Guidelines specify using
findTasksJsonPath. If that’s the established helper, switch to it for consistency.Apply this diff (if applicable in utils):
-import { findTasksPath } from '../utils/path-utils.js'; +import { findTasksJsonPath } from '../utils/path-utils.js';And update the call site:
- tasksPath = findTasksPath(args, log); + tasksPath = findTasksJsonPath(args, log);
33-41: Standardize input validation error codes.Direct wrappers should use the standard set: INPUT_VALIDATION_ERROR, FILE_NOT_FOUND_ERROR, CORE_FUNCTION_ERROR, UNEXPECTED_ERROR. Replace bespoke codes like MISSING_SOURCE_IDS with INPUT_VALIDATION_ERROR and include a details field for the missing parameter.
Example (outside this hunk):
return { success: false, error: { code: 'INPUT_VALIDATION_ERROR', message: 'Source IDs are required', details: { missing: 'sourceIds' } } };tests/integration/cli/move-cross-tag.test.js (1)
721-734: Test expectation includes a non-existent ‘force’ option; remove it.The test harness passes only withDependencies/ignoreDependencies; expecting force: undefined will cause brittle or failing assertions.
Apply:
{ withDependencies: undefined, - ignoreDependencies: undefined, - force: undefined + ignoreDependencies: undefined }
| feat(move): improve cross-tag move UX and safety | ||
|
|
||
| - CLI: print "Next Steps" tips after cross-tag moves that used --ignore-dependencies (validate/fix guidance) | ||
| - CLI: show dedicated help block on ID collisions (destination tag already has the ID) | ||
| - Core: add structured suggestions to TASK_ALREADY_EXISTS errors | ||
| - MCP: map ID collision errors to TASK_ALREADY_EXISTS and include suggestions | ||
| - Tests: cover MCP options, error suggestions, CLI tips printing, and integration error payload suggestions |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a one-line, user-facing changeset summary (minor).
The detailed bullets belong in PR description and docs. The changeset should have a single-line imperative summary for the CHANGELOG.
Apply this diff:
-feat(move): improve cross-tag move UX and safety
-
-- CLI: print "Next Steps" tips after cross-tag moves that used --ignore-dependencies (validate/fix guidance)
-- CLI: show dedicated help block on ID collisions (destination tag already has the ID)
-- Core: add structured suggestions to TASK_ALREADY_EXISTS errors
-- MCP: map ID collision errors to TASK_ALREADY_EXISTS and include suggestions
-- Tests: cover MCP options, error suggestions, CLI tips printing, and integration error payload suggestions
+feat(move): improve cross-tag move UX and safety; add ID-collision suggestions, scoped dependency traversal, CLI “Next Steps” tips, and MCP error mapping📝 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.
| feat(move): improve cross-tag move UX and safety | |
| - CLI: print "Next Steps" tips after cross-tag moves that used --ignore-dependencies (validate/fix guidance) | |
| - CLI: show dedicated help block on ID collisions (destination tag already has the ID) | |
| - Core: add structured suggestions to TASK_ALREADY_EXISTS errors | |
| - MCP: map ID collision errors to TASK_ALREADY_EXISTS and include suggestions | |
| - Tests: cover MCP options, error suggestions, CLI tips printing, and integration error payload suggestions | |
| feat(move): improve cross-tag move UX and safety; add ID-collision suggestions, scoped dependency traversal, CLI “Next Steps” tips, and MCP error mapping |
🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Use correct spacing
Context: ...e): improve cross-tag move UX and safety - CLI: print "Next Steps" tips after cross...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~7-~7: There might be a mistake here.
Context: ...ore-dependencies (validate/fix guidance) - CLI: show dedicated help block on ID col...
(QB_NEW_EN_OTHER)
[grammar] ~8-~8: There might be a mistake here.
Context: ...ons (destination tag already has the ID) - Core: add structured suggestions to TASK...
(QB_NEW_EN_OTHER)
[grammar] ~9-~9: There might be a mistake here.
Context: ...uggestions to TASK_ALREADY_EXISTS errors - MCP: map ID collision errors to TASK_ALR...
(QB_NEW_EN_OTHER)
[grammar] ~10-~10: There might be a mistake here.
Context: ...K_ALREADY_EXISTS and include suggestions - Tests: cover MCP options, error suggesti...
(QB_NEW_EN_OTHER)
[grammar] ~11-~11: There might be a mistake here.
Context: ...nd integration error payload suggestions ---
(QB_NEW_EN_OTHER)
🤖 Prompt for AI Agents
In .changeset/strong-eagles-vanish.md around lines 5 to 11 the changeset
contains a multi-line bulleted description; replace that block with a
single-line, user-facing imperative summary for the CHANGELOG such as "Improve
cross-tag move UX and safety." Remove the bullets (they belong in the PR
description/docs), keep the changeset type as minor, and ensure the file
contains only the one-line summary sentence.
| - Core: add structured suggestions to TASK_ALREADY_EXISTS errors | ||
| - MCP: map ID collision errors to TASK_ALREADY_EXISTS and include suggestions | ||
| - Tests: cover MCP options, error suggestions, CLI tips printing, and integration error payload suggestions | ||
| --- |
There was a problem hiding this comment.
Remove extra frontmatter delimiter.
A trailing --- after the summary isn’t part of Changesets format and may confuse parsers.
Apply this diff:
----Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .changeset/strong-eagles-vanish.md around line 12, there is an extra trailing
frontmatter delimiter line containing '---'; remove that trailing '---' so the
file matches Changesets format (only the required frontmatter and summary
remain) and save the file without the stray delimiter.
| Note: Force moves are no longer supported. Instead, use one of these options: | ||
|
|
||
| ```bash | ||
| task-master move --from=5 --from-tag=backlog --to-tag=in-progress --force | ||
| ``` | ||
| - `--with-dependencies` — move dependents together | ||
| - `--ignore-dependencies` — break cross-tag dependencies | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Tighten wording and clarify which option may break relationships.
Make it explicit that the warning applies to --ignore-dependencies, and simplify the lead-in sentence.
Apply this diff:
-Note: Force moves are no longer supported. Instead, use one of these options:
+Force moves are no longer supported. Choose one of the following options:
- `--with-dependencies` — move dependents together
- `--ignore-dependencies` — break cross-tag dependenciesOutside this hunk, consider changing the warning to:
“Warning: Using --ignore-dependencies may break dependency relationships and should be used with caution.”
📝 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.
| Note: Force moves are no longer supported. Instead, use one of these options: | |
| ```bash | |
| task-master move --from=5 --from-tag=backlog --to-tag=in-progress --force | |
| ``` | |
| - `--with-dependencies` — move dependents together | |
| - `--ignore-dependencies` — break cross-tag dependencies | |
| Force moves are no longer supported. Choose one of the following options: | |
| - `--with-dependencies` — move dependents together | |
| - `--ignore-dependencies` — break cross-tag dependencies |
🧰 Tools
🪛 LanguageTool
[grammar] ~68-~68: Use correct spacing
Context: ...rted. Instead, use one of these options: - --with-dependencies — move dependents together - `--ignore-...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~70-~70: There might be a mistake here.
Context: ...dependencies— move dependents together ---ignore-dependencies` — break cross-tag dependencies
(QB_NEW_EN_OTHER)
[grammar] ~71-~71: There might be a mistake here.
Context: ...ndencies` — break cross-tag dependencies
(QB_NEW_EN_OTHER)
🤖 Prompt for AI Agents
In docs/cross-tag-task-movement.md around lines 68 to 72, the current wording is
vague about which option can break relationships; change the lead-in to a
simpler sentence and explicitly state that the warning applies to
--ignore-dependencies (e.g., “Note: Force moves are no longer supported. Use one
of the following options.”), list the two flags with brief clarifying phrases,
and make the warning outside this hunk read: “Warning: Using
--ignore-dependencies may break dependency relationships and should be used with
caution.” Ensure the text clearly links the warning to --ignore-dependencies.
| // Validate suggestions on the error payload | ||
| try { | ||
| await moveTasksBetweenTags( | ||
| testDataPath, | ||
| taskIds, | ||
| sourceTag, | ||
| targetTag, | ||
| {}, | ||
| { projectRoot: '/test/project' } | ||
| ); | ||
| } catch (err) { | ||
| expect(err.code).toBe('TASK_ALREADY_EXISTS'); | ||
| expect(Array.isArray(err.data?.suggestions)).toBe(true); | ||
| const s = (err.data?.suggestions || []).join(' '); | ||
| expect(s).toContain('different target tag'); | ||
| expect(s).toContain('different set of IDs'); | ||
| expect(s).toContain('within-tag'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Assert structured error fields on ID collision.
Core now enriches the error payload with structured fields. Validate them to future-proof the test against message text changes.
Apply:
} catch (err) {
expect(err.code).toBe('TASK_ALREADY_EXISTS');
expect(Array.isArray(err.data?.suggestions)).toBe(true);
+ expect(err.data?.conflictingId).toBe(1);
+ expect(err.data?.targetTag).toBe('in-progress');
const s = (err.data?.suggestions || []).join(' ');
expect(s).toContain('different target tag');
expect(s).toContain('different set of IDs');
expect(s).toContain('within-tag');
}📝 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.
| // Validate suggestions on the error payload | |
| try { | |
| await moveTasksBetweenTags( | |
| testDataPath, | |
| taskIds, | |
| sourceTag, | |
| targetTag, | |
| {}, | |
| { projectRoot: '/test/project' } | |
| ); | |
| } catch (err) { | |
| expect(err.code).toBe('TASK_ALREADY_EXISTS'); | |
| expect(Array.isArray(err.data?.suggestions)).toBe(true); | |
| const s = (err.data?.suggestions || []).join(' '); | |
| expect(s).toContain('different target tag'); | |
| expect(s).toContain('different set of IDs'); | |
| expect(s).toContain('within-tag'); | |
| } | |
| // Validate suggestions on the error payload | |
| try { | |
| await moveTasksBetweenTags( | |
| testDataPath, | |
| taskIds, | |
| sourceTag, | |
| targetTag, | |
| {}, | |
| { projectRoot: '/test/project' } | |
| ); | |
| } catch (err) { | |
| expect(err.code).toBe('TASK_ALREADY_EXISTS'); | |
| expect(Array.isArray(err.data?.suggestions)).toBe(true); | |
| expect(err.data?.conflictingId).toBe(1); | |
| expect(err.data?.targetTag).toBe('in-progress'); | |
| const s = (err.data?.suggestions || []).join(' '); | |
| expect(s).toContain('different target tag'); | |
| expect(s).toContain('different set of IDs'); | |
| expect(s).toContain('within-tag'); | |
| } |
| import { jest } from '@jest/globals'; | ||
|
|
||
| // Mocks | ||
| const mockFindTasksPath = jest | ||
| .fn() | ||
| .mockReturnValue('/test/path/.taskmaster/tasks/tasks.json'); | ||
| jest.unstable_mockModule( | ||
| '../../../../mcp-server/src/core/utils/path-utils.js', | ||
| () => ({ | ||
| findTasksPath: mockFindTasksPath | ||
| }) | ||
| ); | ||
|
|
||
| const mockEnableSilentMode = jest.fn(); | ||
| const mockDisableSilentMode = jest.fn(); | ||
| jest.unstable_mockModule('../../../../scripts/modules/utils.js', () => ({ | ||
| enableSilentMode: mockEnableSilentMode, | ||
| disableSilentMode: mockDisableSilentMode | ||
| })); | ||
|
|
||
| // Spyable mock for moveTasksBetweenTags | ||
| const mockMoveTasksBetweenTags = jest.fn(); | ||
| jest.unstable_mockModule( | ||
| '../../../../scripts/modules/task-manager/move-task.js', | ||
| () => ({ | ||
| moveTasksBetweenTags: mockMoveTasksBetweenTags | ||
| }) | ||
| ); | ||
|
|
||
| // Import after mocks | ||
| const { moveTaskCrossTagDirect } = await import( | ||
| '../../../../mcp-server/src/core/direct-functions/move-task-cross-tag.js' | ||
| ); | ||
|
|
||
| describe('MCP Cross-Tag Move Direct Function - options & suggestions', () => { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Great mock-first setup; add assertions for silent mode toggling and path resolution.
To harden the contract of the direct function, verify silent mode and tasks path resolution are invoked as expected.
Apply:
beforeEach(() => {
jest.clearAllMocks();
});
it('passes only withDependencies/ignoreDependencies (no force) to core', async () => {
@@
await moveTaskCrossTagDirect(
{
sourceIds: '1,2',
sourceTag: 'backlog',
targetTag: 'in-progress',
withDependencies: true,
projectRoot: '/test'
},
mockLog
);
// Assert options argument (5th param)
expect(mockMoveTasksBetweenTags).toHaveBeenCalled();
const args = mockMoveTasksBetweenTags.mock.calls[0];
const moveOptions = args[4];
expect(moveOptions).toEqual({
withDependencies: true,
ignoreDependencies: false
});
expect('force' in moveOptions).toBe(false);
+
+ // Silent mode toggling happens around the core call
+ expect(mockEnableSilentMode).toHaveBeenCalledTimes(1);
+ expect(mockDisableSilentMode).toHaveBeenCalledTimes(1);
+ expect(mockEnableSilentMode.mock.invocationCallOrder[0]).toBeLessThan(
+ mockDisableSilentMode.mock.invocationCallOrder[0]
+ );
+
+ // Path resolution is used when only projectRoot is provided
+ expect(mockFindTasksPath).toHaveBeenCalled();
});📝 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.
| import { jest } from '@jest/globals'; | |
| // Mocks | |
| const mockFindTasksPath = jest | |
| .fn() | |
| .mockReturnValue('/test/path/.taskmaster/tasks/tasks.json'); | |
| jest.unstable_mockModule( | |
| '../../../../mcp-server/src/core/utils/path-utils.js', | |
| () => ({ | |
| findTasksPath: mockFindTasksPath | |
| }) | |
| ); | |
| const mockEnableSilentMode = jest.fn(); | |
| const mockDisableSilentMode = jest.fn(); | |
| jest.unstable_mockModule('../../../../scripts/modules/utils.js', () => ({ | |
| enableSilentMode: mockEnableSilentMode, | |
| disableSilentMode: mockDisableSilentMode | |
| })); | |
| // Spyable mock for moveTasksBetweenTags | |
| const mockMoveTasksBetweenTags = jest.fn(); | |
| jest.unstable_mockModule( | |
| '../../../../scripts/modules/task-manager/move-task.js', | |
| () => ({ | |
| moveTasksBetweenTags: mockMoveTasksBetweenTags | |
| }) | |
| ); | |
| // Import after mocks | |
| const { moveTaskCrossTagDirect } = await import( | |
| '../../../../mcp-server/src/core/direct-functions/move-task-cross-tag.js' | |
| ); | |
| describe('MCP Cross-Tag Move Direct Function - options & suggestions', () => { | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); | |
| it('passes only withDependencies/ignoreDependencies (no force) to core', async () => { | |
| await moveTaskCrossTagDirect( | |
| { | |
| sourceIds: '1,2', | |
| sourceTag: 'backlog', | |
| targetTag: 'in-progress', | |
| withDependencies: true, | |
| projectRoot: '/test' | |
| }, | |
| mockLog | |
| ); | |
| // Assert options argument (5th param) | |
| expect(mockMoveTasksBetweenTags).toHaveBeenCalled(); | |
| const args = mockMoveTasksBetweenTags.mock.calls[0]; | |
| const moveOptions = args[4]; | |
| expect(moveOptions).toEqual({ | |
| withDependencies: true, | |
| ignoreDependencies: false | |
| }); | |
| expect('force' in moveOptions).toBe(false); | |
| // Silent mode toggling happens around the core call | |
| expect(mockEnableSilentMode).toHaveBeenCalledTimes(1); | |
| expect(mockDisableSilentMode).toHaveBeenCalledTimes(1); | |
| expect(mockEnableSilentMode.mock.invocationCallOrder[0]).toBeLessThan( | |
| mockDisableSilentMode.mock.invocationCallOrder[0] | |
| ); | |
| // Path resolution is used when only projectRoot is provided | |
| expect(mockFindTasksPath).toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
In tests/unit/mcp/tools/move-task-cross-tag-options.test.js around lines 1 to
35, add assertions that verify silent mode toggling and tasks path resolution:
call moveTaskCrossTagDirect with options that set silent mode and ensure
mockEnableSilentMode is called before moveTasksBetweenTags and
mockDisableSilentMode is called after; assert mockFindTasksPath was invoked (and
with any expected args if the function should receive them) and that
mockMoveTasksBetweenTags was called using the path returned by mockFindTasksPath
('/test/path/.taskmaster/tasks/tasks.json') so the test confirms both path
resolution and silent-mode behavior.
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
Outdated
Show resolved
Hide resolved
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
mcp-server/src/core/direct-functions/move-task-cross-tag.js (3)
110-118: Pass standard MCP context to core and wrap FastMCP loggerDirect functions should pass mcp-specific context and a wrapped logger to core. Also surface telemetryData explicitly.
Apply this diff:
import { enableSilentMode, - disableSilentMode + disableSilentMode, + logWrapper } from '../../../../scripts/modules/utils.js'; @@ // Call the core moveTasksBetweenTags function const result = await moveTasksBetweenTags( tasksPath, sourceIds, args.sourceTag, args.targetTag, moveOptions, - { projectRoot } + { + projectRoot, + mcpLog: logWrapper(log), + commandName: 'move', + outputType: 'mcp', + outputFormat: 'json' + } ); @@ return { success: true, data: { ...result, + telemetryData: result?.telemetryData, message: `Successfully moved ${sourceIds.length} task(s) from "${args.sourceTag}" to "${args.targetTag}"`, moveOptions, sourceTag: args.sourceTag, targetTag: args.targetTag } };If your utils export a differently named helper (e.g., createLogWrapper), swap
logWrapperaccordingly.Also applies to: 8-11, 120-129
104-109: Disallow mutually exclusive flags simultaneouslyGuard against
--with-dependenciesand--ignore-dependenciesboth being true; they conflict semantically.Apply this diff:
// Prepare move options const moveOptions = { withDependencies: args.withDependencies || false, ignoreDependencies: args.ignoreDependencies || false }; + + // Validate mutually exclusive options + if (moveOptions.withDependencies && moveOptions.ignoreDependencies) { + return { + success: false, + error: { + message: + '--with-dependencies and --ignore-dependencies cannot be used together', + code: 'INVALID_DEPENDENCY_OPTIONS', + suggestions: [ + 'Choose exactly one: --with-dependencies or --ignore-dependencies', + 'Omit both flags to review tips and decide' + ] + } + }; + }
6-6: Use the canonicalfindTasksJsonPathalias in direct‐function wrappersTo align with our utility‐wrapper convention and the guideline “Call
findTasksJsonPath(args, log)from within direct function wrappers,” please update both wrappers to import and invoke the alias instead of the base function:
- In
mcp-server/src/core/direct-functions/show-task.jsand.../move-task-cross-tag.js:// at the top of the file - import { findTasksPath } from '../utils/path-utils.js'; + import { findTasksJsonPath } from '../utils/path-utils.js';- In each call site:
// show-task.js - tasksJsonPath = findTasksPath({ projectRoot, file }, log); + tasksJsonPath = findTasksJsonPath({ projectRoot, file }, log); // move-task-cross-tag.js (around line 94) - tasksPath = findTasksPath(args, log); + tasksPath = findTasksJsonPath(args, log);This ensures consistency across all direct functions and uses the explicitly named alias for resolving the
tasks.jsonpath.
♻️ Duplicate comments (4)
mcp-server/src/core/direct-functions/move-task-cross-tag.js (2)
205-212: Include conflict metadata in error payloadExpose
conflictingIdandtargetTagwhen available to help clients render targeted guidance.Apply this diff:
return { success: false, error: { message: error.message, code: errorCode, - suggestions + suggestions, + // Useful context for ID collisions if provided by core + conflictingId: error.conflictingId, + targetTag: error.targetTag } };
192-202: Pass through core-provided suggestions for ID collisions; fall back to defaultsPrefer
error.suggestionsfrom core when present to keep guidance consistent across surfaces.Apply this diff:
- } else if ( - error.code === 'TASK_ALREADY_EXISTS' || - error.message?.includes('already exists in target tag') - ) { - // Target tag has an ID collision - errorCode = 'TASK_ALREADY_EXISTS'; - suggestions = [ - 'Choose a different target tag without conflicting IDs', - 'Move a different set of IDs (avoid existing ones)', - 'If needed, move within-tag to a new ID first, then cross-tag move' - ]; + } else if ( + error.code === 'TASK_ALREADY_EXISTS' || + error.message?.includes('already exists in target tag') + ) { + // Target tag has an ID collision + errorCode = 'TASK_ALREADY_EXISTS'; + suggestions = Array.isArray(error.suggestions) && error.suggestions.length + ? error.suggestions + : [ + 'Choose a different target tag without conflicting IDs', + 'Move a different set of IDs (avoid existing ones)', + 'If needed, move within-tag to a new ID first, then cross-tag move' + ];docs/cross-tag-task-movement.md (2)
68-74: Tighten lead-in and make warning explicitly about --ignore-dependenciesClarify which option may break relationships and simplify the intro sentence.
Apply this diff:
-Note: Force moves are no longer supported. Instead, use one of these options: +Force moves are no longer supported. Choose one of the following options: - `--with-dependencies` — move dependents together - `--ignore-dependencies` — break cross-tag dependencies - -⚠️ **Warning**: This may break dependency relationships and should be used with caution. +⚠️ **Warning**: Using `--ignore-dependencies` may break dependency relationships and should be used with caution.
264-274: Rename scenario heading to remove “Force Move” and align copyAvoid referencing deprecated behavior in headings and streamline the intro.
Apply this diff:
-### Scenario 3: Force Move +### Scenario 3: Choose explicit dependency handling @@ -Choose one of these options explicitly: +Choose one of the following options:
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.changeset/clarify-force-move-docs.md(1 hunks)docs/cross-tag-task-movement.md(3 hunks)mcp-server/src/core/direct-functions/move-task-cross-tag.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
mcp-server/src/core/direct-functions/*.js
📄 CodeRabbit Inference Engine (.cursor/rules/architecture.mdc)
mcp-server/src/core/direct-functions/*.js: Direct function files in mcp-server/src/core/direct-functions/.js must implement silent mode using enableSilentMode and disableSilentMode from scripts/modules/utils.js, and always restore silent mode state using a try/finally block.
Direct function parameters in mcp-server/src/core/direct-functions/.js must match the corresponding core function signatures; do not add extra parameters not present in the core function.
Always use the isSilentMode() function to check silent mode state in mcp-server/src/core/direct-functions/*.js; do not access global variables like silentMode directly.
mcp-server/src/core/direct-functions/*.js: Check that all helper functions your direct function needs are properly exported from their source modules and import these dependencies explicitly at the top of your file in direct functions.
Verify the signature of core functions you're calling in direct functions and ensure all required parameters are provided, passing explicit values for required parameters rather than relying on defaults.
Use path.join() instead of string concatenation for file paths, and follow established file naming conventions (e.g., 'task_001.txt') in direct functions.
Wrap core function calls and AI calls in try/catch blocks in direct functions, log errors with appropriate severity and context, and return standardized error objects with code and message.
When calling a core function from a direct function, pass the log object provided by FastMCP wrapped in the standard logWrapper object as the mcpLog property in the options argument.
Wrap core function calls within direct functions using enableSilentMode() and disableSilentMode() in a try/finally block if the core function might produce console output that isn't reliably controlled by passing { mcpLog } or an outputFormat parameter.
Do not wrap calls to the unified AI service (generateTextService, generateObjectService) in silent mode; their logging is handled internal...
Files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
mcp-server/src/{tools,core/direct-functions}/*.js
📄 CodeRabbit Inference Engine (.cursor/rules/mcp.mdc)
mcp-server/src/{tools,core/direct-functions}/*.js: Use kebab-case for all file names in mcp-server/src/tools/ and mcp-server/src/core/direct-functions/.
Use helpers from mcp-server/src/tools/utils.js, mcp-server/src/core/utils/path-utils.js, and mcp-server/src/core/utils/ai-client-utils.js for centralized utilities.
Files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
mcp-server/src/core/direct-functions/**/*.js
📄 CodeRabbit Inference Engine (.cursor/rules/telemetry.mdc)
mcp-server/src/core/direct-functions/**/*.js: Direct function wrappers in mcp-server/src/core/direct-functions/ must call the corresponding core logic function, passing commandName and outputType: 'mcp' in the context, and outputFormat: 'json' if supported.
Direct function wrappers in mcp-server/src/core/direct-functions/ must include coreResult.telemetryData as a field within the data object of the successful response.In direct functions (
mcp-server/src/core/direct-functions/*) that call core functions (scripts/modules/*), ensure console output from the core function is suppressed to avoid breaking MCP JSON responses, preferably by passing anoutputFormatparameter or by wrapping the call with silent mode.
Files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
**/*.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:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
{scripts/modules/utils.js,mcp-server/src/core/direct-functions/**/*.js}
📄 CodeRabbit Inference Engine (.cursor/rules/utilities.mdc)
{scripts/modules/utils.js,mcp-server/src/core/direct-functions/**/*.js}: Use the logger wrapper pattern when passing loggers to preventmcpLog[level] is not a functionerrors; do not pass the FastMCPlogobject directly asmcpLogto core functions.
Ensure silent mode is disabled in afinallyblock to prevent it from staying enabled.
Files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
.changeset/*.md
📄 CodeRabbit Inference Engine (.cursor/rules/changeset.mdc)
.changeset/*.md: When runningnpm run changesetornpx changeset add, provide a concise summary of the changes for theCHANGELOG.mdin imperative mood, typically a single line, and not a detailed Git commit message.
The changeset summary should be user-facing, describing what changed in the released version that is relevant to users or consumers of the package.
Do not use your detailed Git commit message body as the changeset summary.
Files:
.changeset/clarify-force-move-docs.md
.changeset/*
📄 CodeRabbit Inference Engine (.cursor/rules/new_features.mdc)
Create appropriate changesets for new features, use semantic versioning, include tagged system information in release notes, and document breaking changes if any.
Files:
.changeset/clarify-force-move-docs.md
docs/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/new_features.mdc)
Add feature documentation to '/docs' folder, include tagged system usage examples, update command reference documentation, and provide migration notes if relevant.
Files:
docs/cross-tag-task-movement.md
🧠 Learnings (30)
📓 Common learnings
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Guidelines for integrating new features into the Task Master CLI with tagged system considerations (new_features.mdc).
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use AI to generate detailed subtasks within the current tag context, considering complexity analysis for subtask counts and ensuring proper IDs for newly created subtasks.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use tag resolution functions to maintain backward compatibility, returning legacy format to core functions and not exposing the tagged structure to existing core logic.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Assign unique sequential IDs to tasks within each tag context by calculating the next ID based on existing tasks in the current tag; do not hardcode or reuse IDs within the same tag.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Core functions must receive the legacy format for 100% backward compatibility, using tag resolution functions to abstract the tagged structure.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Tasks must be accessed and updated through the tag resolution layer using getTasksForTag(data, tagName) and setTasksForTag(data, tagName, tasks); direct manipulation of the tagged structure in core functions is not allowed.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Use tagged task system aware functions for task finding and manipulation, handle both task and subtask operations, and validate task IDs before operations.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Generate task files from the current tag context, include tag information in generated files, and do not mix tasks from different tags in file generation.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.jsdocs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Implement complete migration for all related files in the tagged task system, handle configuration and state file creation, and provide migration status tracking.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Implement silent migration from legacy to tagged format in the readJSON() function, detect legacy format, convert automatically, and preserve all existing task data during migration.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:07:39.336Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/architecture.mdc:0-0
Timestamp: 2025-07-18T17:07:39.336Z
Learning: Applies to scripts/modules/task-manager/*.js : Files in scripts/modules/task-manager/ should each handle a specific action related to task management (e.g., add-task.js, expand-task.js), supporting the tagged task lists system and backward compatibility.
Applied to files:
mcp-server/src/core/direct-functions/move-task-cross-tag.js
📚 Learning: 2025-07-18T17:07:53.100Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/changeset.mdc:0-0
Timestamp: 2025-07-18T17:07:53.100Z
Learning: Applies to .changeset/*.md : When running `npm run changeset` or `npx changeset add`, provide a concise summary of the changes for the `CHANGELOG.md` in imperative mood, typically a single line, and not a detailed Git commit message.
Applied to files:
.changeset/clarify-force-move-docs.md
📚 Learning: 2025-07-23T16:03:42.784Z
Learnt from: Crunchyman-ralph
PR: eyaltoledano/claude-task-master#1035
File: .changeset/quiet-rabbits-bathe.md:5-10
Timestamp: 2025-07-23T16:03:42.784Z
Learning: For changeset files (.changeset/*.md), avoid suggesting punctuation for bullet points as the project intentionally omits punctuation to make the generated changelog feel more natural.
Applied to files:
.changeset/clarify-force-move-docs.md
📚 Learning: 2025-07-18T17:07:53.100Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/changeset.mdc:0-0
Timestamp: 2025-07-18T17:07:53.100Z
Learning: Applies to .changeset/*.md : Do not use your detailed Git commit message body as the changeset summary.
Applied to files:
.changeset/clarify-force-move-docs.md
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Maintain valid dependency structure with `task-master fix-dependencies` when needed.
Applied to files:
.changeset/clarify-force-move-docs.mddocs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Use `task-master add-dependency --id=<id> --depends-on=<id>` and `task-master remove-dependency --id=<id> --depends-on=<id>` to manage task dependencies, avoiding circular dependencies and duplicates.
Applied to files:
.changeset/clarify-force-move-docs.mddocs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Mark outdated patterns as deprecated, remove rules that no longer apply, update references to deprecated rules, and document migration paths for old patterns.
Applied to files:
docs/cross-tag-task-movement.md
📚 Learning: 2025-07-31T22:07:49.716Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/commands.mdc:0-0
Timestamp: 2025-07-31T22:07:49.716Z
Learning: Applies to scripts/modules/commands.js : Suggest non-destructive alternatives when appropriate, explaining the difference between deletion and status changes, and include examples of alternative commands.
Applied to files:
docs/cross-tag-task-movement.md
📚 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: Use the `move_task` command to reorganize tasks and subtasks within the hierarchy, ensuring parent-child relationships and dependency integrity.
Applied to files:
docs/cross-tag-task-movement.md
📚 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: Use the Taskmaster command set (`task-master` CLI or MCP tools) for all task management operations: listing, expanding, updating, tagging, and status changes.
Applied to files:
docs/cross-tag-task-movement.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: Use the --tag <name> option to operate on specific task contexts (tags) when running Taskmaster commands. If the tag does not exist, it will be created automatically.
Applied to files:
docs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:10:53.657Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Guidelines for managing task dependencies and relationships across tagged task contexts (dependencies.mdc).
Applied to files:
docs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:19:27.365Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: assets/.windsurfrules:0-0
Timestamp: 2025-07-18T17:19:27.365Z
Learning: Use `task-master next` to show the next task to work on, ensuring tasks are completed in the appropriate sequence.
Applied to files:
docs/cross-tag-task-movement.md
📚 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: Organize tasks into tags (contexts) such as 'master', 'feature-branch', or 'v2.0' in Taskmaster to isolate workstreams and prevent conflicts.
Applied to files:
docs/cross-tag-task-movement.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: When removing tasks or subtasks, use the remove-task or remove-subtask commands with caution, as this operation cannot be undone. Consider using 'blocked', 'cancelled', or 'deferred' status instead if you want to keep the task for reference.
Applied to files:
docs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:10:53.657Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Guide for using Task Master to manage task-driven development workflows with tagged task lists support (dev_workflow.mdc).
Applied to files:
docs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Tasks must be organized into separate contexts (tags) within tasks.json, using the tagged format: {"master": {"tasks": [...]}, "feature-branch": {"tasks": [...]}}. Legacy format {"tasks": [...]} must be silently migrated to the tagged format on first use.
Applied to files:
docs/cross-tag-task-movement.md
📚 Learning: 2025-07-18T17:13:30.188Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-07-18T17:13:30.188Z
Learning: Applies to scripts/modules/* : Do not omit the --tag CLI option in commands that operate on tasks
Applied to files:
docs/cross-tag-task-movement.md
📚 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: Use the `add_dependency` and `remove_dependency` commands to manage task dependencies, ensuring no circular or duplicate dependencies.
Applied to files:
docs/cross-tag-task-movement.md
📚 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: When breaking down complex tasks, use the `expand_task` command with appropriate flags (`--force`, `--research`, `--num`, `--prompt`) and review generated subtasks for accuracy.
Applied to files:
docs/cross-tag-task-movement.md
🪛 LanguageTool
.changeset/clarify-force-move-docs.md
[grammar] ~4-~4: Insert the missing word
Context: --- "task-master-ai": patch --- docs(move): clarify cross-tag move docs; dep...
(QB_NEW_EN_OTHER_ERROR_IDS_32)
[grammar] ~5-~5: There might be a mistake here.
Context: ...endencies/--ignore-dependencies examples
(QB_NEW_EN)
docs/cross-tag-task-movement.md
[grammar] ~68-~68: Use correct spacing
Context: ...rted. Instead, use one of these options: - --with-dependencies — move dependents together - `--ignore-...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~70-~70: There might be a mistake here.
Context: ...dependencies— move dependents together ---ignore-dependencies` — break cross-tag dependencies
(QB_NEW_EN_OTHER)
[grammar] ~71-~71: There might be a mistake here.
Context: ...ndencies` — break cross-tag dependencies
(QB_NEW_EN_OTHER)
[grammar] ~266-~266: Use correct spacing
Context: ... Choose one of these options explicitly: bash # Move together with dependencies task-master move --from=5 --from-tag=backlog --to-tag=in-progress --with-dependencies # Or break dependencies task-master move --from=5 --from-tag=backlog --to-tag=in-progress --ignore-dependencies ### Scenario 4: Moving Subtasks ```bash # O...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.17.2)
.changeset/clarify-force-move-docs.md
5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (3)
.changeset/clarify-force-move-docs.md (1)
5-5: Good: concise, single-line, imperative summaryThe changeset body follows repo guidance: single, user-facing line in imperative mood. Nice.
mcp-server/src/core/direct-functions/move-task-cross-tag.js (1)
97-133: Good: silent mode enable/disable wrapped in finallyCorrectly prevents stray console output in MCP responses and reliably restores state.
docs/cross-tag-task-movement.md (1)
95-95: Nice improvement: actionable step without redundancyThe step now directs re-running with the chosen flag and avoids repeating earlier items.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "task-master-ai": patch | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a minor bump for this feature
This PR introduces user-facing functionality across CLI/MCP/Core and updated docs; per semver and your PR notes, this should be a minor release.
Apply this diff:
-"task-master-ai": patch
+"task-master-ai": minor📝 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.
| "task-master-ai": patch | |
| --- | |
| "task-master-ai": minor | |
| --- |
🤖 Prompt for AI Agents
.changeset/clarify-force-move-docs.md around line 2: the changeset currently
lists "task-master-ai": patch but the PR introduces user-facing CLI/MCP/Core
features and updated docs so it should be a minor release; update the value from
patch to minor (e.g. "task-master-ai": minor) to ensure a minor semver bump and
save the file.
…st.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…endencies option - Introduced a new test to ensure that the dependency traversal is limited to tasks from the source tag when using the --with-dependencies option, addressing potential ID collisions across tags.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js (1)
222-224: Rename test; no ID collision is configured (same feedback as earlier).Fixture has no target-tag ID collision (in-progress contains only id:4). The title and preface comment overstate the scenario. Rename to reflect the actual behavior under test.
Apply this diff:
- // New test: ensure with-dependencies only traverses sourceTag tasks even if IDs collide across tags - it('should scope dependency traversal to source tag when IDs collide across tags', async () => { + // New test: ensure with-dependencies traverses only the sourceTag's tasks (does not traverse target tag) + it('should scope dependency traversal to source tag only', async () => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
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/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.test.js
**/*.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:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.test.js
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Guidelines for integrating new features into the Task Master CLI with tagged system considerations (new_features.mdc).
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/*.js : Default to current tag when not specified, support explicit tag selection in advanced features, validate tag existence before operations, and provide clear messaging about tag context.
📚 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 core logic independently with both data formats, mock file system operations, test tag resolution behavior, and verify migration compatibility in unit tests.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 : Verify modifications on the in-memory task objects passed to writeJSON.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use AI to generate detailed subtasks within the current tag context, considering complexity analysis for subtask counts and ensuring proper IDs for newly created subtasks.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Assign unique sequential IDs to tasks within each tag context by calculating the next ID based on existing tasks in the current tag; do not hardcode or reuse IDs within the same tag.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Generate task files from the current tag context, include tag information in generated files, and do not mix tasks from different tags in file generation.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Tasks must be organized into separate contexts (tags) within tasks.json, using the tagged format: {"master": {"tasks": [...]}, "feature-branch": {"tasks": [...]}}. Legacy format {"tasks": [...]} must be silently migrated to the tagged format on first use.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 tests/{unit,integration,e2e}/**/*.test.js : Use sample task fixtures for consistent test data, mock file system operations, and test both success and error paths for task operations.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use consistent formatting for task files, include all task properties in text files, and format dependencies with status indicators.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 : Use different file paths for each test to avoid test interdependence.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Format task and dependency IDs consistently when adding dependencies
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 modify real task files (tasks.json) during tests.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:09:40.548Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:40.548Z
Learning: Applies to scripts/modules/dependency-manager.js : Do not create circular dependencies between subtasks
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.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/dependency-manager.js : Features that handle task relationships belong in 'scripts/modules/dependency-manager.js'.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Support both task and subtask dependencies in cycle detection
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
🧬 Code Graph Analysis (1)
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js (4)
mcp-server/src/core/direct-functions/move-task-cross-tag.js (1)
result(111-118)scripts/modules/task-manager/move-task.js (2)
result(135-141)result(189-189)scripts/modules/utils.js (1)
result(503-507)tests/unit/scripts/modules/commands/move-cross-tag.test.js (1)
result(157-164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
| const result = await moveTasksBetweenTags( | ||
| mockTasksPath, | ||
| [1], // backlog:1 depends on backlog:2 | ||
| 'backlog', | ||
| 'in-progress', | ||
| { withDependencies: true }, | ||
| mockContext | ||
| ); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Assert the result to verify move count and avoid an unused variable.
Validate that withDependencies moved both the task and its source-tag dependency.
Apply this diff:
const result = await moveTasksBetweenTags(
mockTasksPath,
[1], // backlog:1 depends on backlog:2
'backlog',
'in-progress',
{ withDependencies: true },
mockContext
);
+ expect(result.message).toContain('Successfully moved 2 tasks');📝 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.
| const result = await moveTasksBetweenTags( | |
| mockTasksPath, | |
| [1], // backlog:1 depends on backlog:2 | |
| 'backlog', | |
| 'in-progress', | |
| { withDependencies: true }, | |
| mockContext | |
| ); | |
| const result = await moveTasksBetweenTags( | |
| mockTasksPath, | |
| [1], // backlog:1 depends on backlog:2 | |
| 'backlog', | |
| 'in-progress', | |
| { withDependencies: true }, | |
| mockContext | |
| ); | |
| expect(result.message).toContain('Successfully moved 2 tasks'); |
🤖 Prompt for AI Agents
In tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js around
lines 227 to 235, the test stores the return value of moveTasksBetweenTags in an
unused variable; update the test to assert the function's result to verify the
move count so the test confirms that withDependencies moved both the requested
task and its source-tag dependency (e.g., assert the returned move count equals
2 or assert the returned object indicates two tasks moved), removing the unused
variable.
| // Write should include backlog:2 moved, and must NOT traverse or fetch dependencies from the target tag | ||
| expect(writeJSON).toHaveBeenCalledWith( | ||
| mockTasksPath, | ||
| expect.objectContaining({ | ||
| 'in-progress': expect.objectContaining({ | ||
| tasks: expect.arrayContaining([ | ||
| expect.objectContaining({ id: 1 }), | ||
| expect.objectContaining({ id: 2 }) // the backlog:2 now moved | ||
| // ensure existing in-progress:2 remains (by id) but we don't double-add or fetch deps from it | ||
| ]) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix inaccurate inline comment and strengthen verification.
The inline comment references a non-existent "in-progress:2". Remove it and add assertions to ensure:
- existing in-progress task (id:4) remains, and
- exactly two tasks were added to the target (ids 1 and 2), preventing accidental traversal of target-tag deps.
Apply this diff:
// Write should include backlog:2 moved, and must NOT traverse or fetch dependencies from the target tag
expect(writeJSON).toHaveBeenCalledWith(
mockTasksPath,
expect.objectContaining({
'in-progress': expect.objectContaining({
tasks: expect.arrayContaining([
expect.objectContaining({ id: 1 }),
- expect.objectContaining({ id: 2 }) // the backlog:2 now moved
- // ensure existing in-progress:2 remains (by id) but we don't double-add or fetch deps from it
+ expect.objectContaining({ id: 2 }) // the backlog:2 now moved
])
})
}),
mockContext.projectRoot,
null
);
+ // Additionally, ensure existing target-tag tasks remain and no extra tasks were traversed from the target
+ const written = writeJSON.mock.calls[0][1];
+ expect(written['in-progress'].tasks).toEqual(
+ expect.arrayContaining([expect.objectContaining({ id: 4 })])
+ );
+ expect(written['in-progress'].tasks).toHaveLength(3);📝 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.
| // Write should include backlog:2 moved, and must NOT traverse or fetch dependencies from the target tag | |
| expect(writeJSON).toHaveBeenCalledWith( | |
| mockTasksPath, | |
| expect.objectContaining({ | |
| 'in-progress': expect.objectContaining({ | |
| tasks: expect.arrayContaining([ | |
| expect.objectContaining({ id: 1 }), | |
| expect.objectContaining({ id: 2 }) // the backlog:2 now moved | |
| // ensure existing in-progress:2 remains (by id) but we don't double-add or fetch deps from it | |
| ]) | |
| }) | |
| // Write should include backlog:2 moved, and must NOT traverse or fetch dependencies from the target tag | |
| expect(writeJSON).toHaveBeenCalledWith( | |
| mockTasksPath, | |
| expect.objectContaining({ | |
| 'in-progress': expect.objectContaining({ | |
| tasks: expect.arrayContaining([ | |
| expect.objectContaining({ id: 1 }), | |
| expect.objectContaining({ id: 2 }) // the backlog:2 now moved | |
| ]) | |
| }) | |
| }), | |
| mockContext.projectRoot, | |
| null | |
| ); | |
| // Additionally, ensure existing target-tag tasks remain and no extra tasks were traversed from the target | |
| const written = writeJSON.mock.calls[0][1]; | |
| expect(written['in-progress'].tasks).toEqual( | |
| expect.arrayContaining([expect.objectContaining({ id: 4 })]) | |
| ); | |
| expect(written['in-progress'].tasks).toHaveLength(3); |
🤖 Prompt for AI Agents
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js around lines
236 to 246: remove the inaccurate inline comment about "in-progress:2" and
instead strengthen the assertions by verifying the writeJSON call contains the
existing in-progress task (id: 4) still present and that the target tag received
exactly the two moved tasks (ids 1 and 2) — e.g., assert the in-progress.tasks
array contains an object with id: 4, contains objects with id: 1 and id: 2, and
that the count of moved tasks added is exactly 2 to prevent accidental
traversal/fetch of target-tag dependencies.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js (2)
227-235: Assert the result to verify move count and avoid an unused variable.The returned result isn’t asserted; add a check to confirm that both the task and its dependency were moved.
Apply:
const result = await moveTasksBetweenTags( mockTasksPath, [1], // backlog:1 depends on backlog:2 'backlog', 'in-progress', { withDependencies: true }, mockContext ); + expect(result.message).toContain('Successfully moved 2 tasks');
236-250: Fix inaccurate inline comment and strengthen verification.Remove the misleading comment about “in-progress:2” and assert the target tag retains existing task id:4 and has exactly 3 tasks (ids 4, 1, 2).
Apply:
- // Write should include backlog:2 moved, and must NOT traverse or fetch dependencies from the target tag + // Write should include backlog:2 moved, and must NOT traverse or fetch dependencies from the target tag expect(writeJSON).toHaveBeenCalledWith( mockTasksPath, expect.objectContaining({ 'in-progress': expect.objectContaining({ tasks: expect.arrayContaining([ expect.objectContaining({ id: 1 }), - expect.objectContaining({ id: 2 }) // the backlog:2 now moved - // ensure existing in-progress:2 remains (by id) but we don't double-add or fetch deps from it + expect.objectContaining({ id: 2 }) // the backlog:2 now moved ]) }) }), mockContext.projectRoot, null ); + // Additionally ensure existing target-tag tasks remain and no extra tasks were traversed from the target + const written = writeJSON.mock.calls[0][1]; + expect(written['in-progress'].tasks).toEqual( + expect.arrayContaining([expect.objectContaining({ id: 4 })]) + ); + expect(written['in-progress'].tasks).toHaveLength(3);tests/integration/move-task-cross-tag.integration.test.js (1)
600-617: Also assert structured error fields on ID collision.Core now includes conflictingId and targetTag. Add checks to future-proof against message text changes.
Apply:
} catch (err) { expect(err.code).toBe('TASK_ALREADY_EXISTS'); expect(Array.isArray(err.data?.suggestions)).toBe(true); + expect(err.data?.conflictingId).toBe(1); + expect(err.data?.targetTag).toBe('in-progress'); const s = (err.data?.suggestions || []).join(' '); expect(s).toContain('different target tag'); expect(s).toContain('different set of IDs'); expect(s).toContain('within-tag'); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
tests/integration/move-task-cross-tag.integration.test.js(3 hunks)tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
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/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
tests/integration/*.js
📄 CodeRabbit Inference Engine (.cursor/rules/architecture.mdc)
Integration tests in tests/integration/ should test interactions between modules.
Files:
tests/integration/move-task-cross-tag.integration.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/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.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/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
tests/integration/**/*.test.js
📄 CodeRabbit Inference Engine (.cursor/rules/tests.mdc)
Integration tests must be located in tests/integration/, test interactions between modules, and focus on component interfaces rather than implementation details.
Files:
tests/integration/move-task-cross-tag.integration.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/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
tests/{integration,e2e}/**/*.test.js
📄 CodeRabbit Inference Engine (.cursor/rules/tests.mdc)
Properly mock session objects when required by functions, and reset environment variables between tests if modified.
Files:
tests/integration/move-task-cross-tag.integration.test.js
**/*.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:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.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/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.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/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.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/scripts/modules/task-manager/move-task-cross-tag.test.js
🧠 Learnings (28)
📓 Common learnings
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/glossary.mdc:0-0
Timestamp: 2025-07-18T17:10:53.657Z
Learning: Guidelines for integrating new features into the Task Master CLI with tagged system considerations (new_features.mdc).
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/*.js : Default to current tag when not specified, support explicit tag selection in advanced features, validate tag existence before operations, and provide clear messaging about tag context.
📚 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/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 tests/{unit,integration,e2e}/**/*.test.js : Use sample task fixtures for consistent test data, mock file system operations, and test both success and error paths for task operations.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.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 core logic independently with both data formats, mock file system operations, test tag resolution behavior, and verify migration compatibility in unit tests.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use AI to generate detailed subtasks within the current tag context, considering complexity analysis for subtask counts and ensuring proper IDs for newly created subtasks.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Use consistent formatting for task files, include all task properties in text files, and format dependencies with status indicators.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.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/dependency-manager.js : Features that handle task relationships belong in 'scripts/modules/dependency-manager.js'.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Generate task files from the current tag context, include tag information in generated files, and do not mix tasks from different tags in file generation.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:09:40.548Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:40.548Z
Learning: Applies to scripts/modules/dependency-manager.js : Remove references to non-existent tasks during validation
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.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/*.js : Ensure new features work with existing projects seamlessly, supporting both legacy and tagged task data formats, and support silent migration during feature usage.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 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 tests/{unit,integration,e2e}/**/*.test.js : Mock console output and verify correct formatting in UI function tests. Use flexible assertions like toContain() or toMatch() for formatted output.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 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 : Verify file operations use the correct paths in expect statements.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 tests/{unit,integration,e2e}/**/*.test.js : 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.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 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 : When verifying console assertions, assert against the actual arguments passed (single formatted string), not multiple arguments.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Subtasks must use consistent properties, maintain simple numeric IDs unique within the parent task, and must not duplicate parent task properties.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Support both task and subtask dependencies in cycle detection
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Do not create circular dependencies between subtasks
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Tasks must be organized into separate contexts (tags) within tasks.json, using the tagged format: {"master": {"tasks": [...]}, "feature-branch": {"tasks": [...]}}. Legacy format {"tasks": [...]} must be silently migrated to the tagged format on first use.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.jstests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Validate that referenced tasks exist before adding dependencies
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 Learning: 2025-07-18T17:18:17.759Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/utilities.mdc:0-0
Timestamp: 2025-07-18T17:18:17.759Z
Learning: Applies to scripts/modules/utils.js : Use tagged task system aware functions for task finding and manipulation, handle both task and subtask operations, and validate task IDs before operations.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Tasks must be accessed and updated through the tag resolution layer using getTasksForTag(data, tagName) and setTasksForTag(data, tagName, tasks); direct manipulation of the tagged structure in core functions is not allowed.
Applied to files:
tests/integration/move-task-cross-tag.integration.test.js
📚 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 : Verify modifications on the in-memory task objects passed to writeJSON.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Provide functions for updating task status within the current tag context, handling both individual tasks and subtasks, and considering subtask status when updating parent tasks.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:14:29.399Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tasks.mdc:0-0
Timestamp: 2025-07-18T17:14:29.399Z
Learning: Applies to scripts/modules/task-manager.js : Allow filtering tasks by status within the current tag context, handle subtask display in lists, and use consistent table formats.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 : Use different file paths for each test to avoid test interdependence.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:09:45.690Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/dependencies.mdc:0-0
Timestamp: 2025-07-18T17:09:45.690Z
Learning: Applies to scripts/modules/dependency-manager.js : Format task and dependency IDs consistently when adding dependencies
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 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 modify real task files (tasks.json) during tests.
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
📚 Learning: 2025-07-18T17:13:30.188Z
Learnt from: CR
PR: eyaltoledano/claude-task-master#0
File: .cursor/rules/tags.mdc:0-0
Timestamp: 2025-07-18T17:13:30.188Z
Learning: Applies to scripts/modules/* : Every command that reads or writes tasks.json must be tag-aware
Applied to files:
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js
🧬 Code Graph Analysis (2)
tests/integration/move-task-cross-tag.integration.test.js (2)
mcp-server/src/core/direct-functions/move-task-cross-tag.js (1)
result(111-118)tests/unit/scripts/modules/commands/move-cross-tag.test.js (1)
result(157-164)
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js (4)
mcp-server/src/core/direct-functions/move-task-cross-tag.js (1)
result(111-118)scripts/modules/task-manager/move-task.js (2)
result(135-141)result(189-189)scripts/modules/utils.js (1)
result(503-507)tests/unit/scripts/modules/commands/move-cross-tag.test.js (1)
result(157-164)
🔇 Additional comments (2)
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js (1)
222-224: Renamed test reads clearer and matches the scenario.Title now reflects source-tag-only traversal with --with-dependencies. Good improvement.
tests/integration/move-task-cross-tag.integration.test.js (1)
429-459: Good: advisory tips assertions are precise and bounded.Asserting array shape and exact tips reduces false positives and aligns with PR objectives around “Next Steps” tips.
| // New test: ensure with-dependencies only traverses tasks from the source tag | ||
| it('should scope dependency traversal to source tag when using --with-dependencies', async () => { | ||
| findCrossTagDependencies.mockReturnValue([]); | ||
| validateSubtaskMove.mockImplementation(() => {}); | ||
|
|
||
| const result = await moveTasksBetweenTags( | ||
| mockTasksPath, | ||
| [1], // backlog:1 depends on backlog:2 | ||
| 'backlog', | ||
| 'in-progress', | ||
| { withDependencies: true }, | ||
| mockContext | ||
| ); | ||
|
|
||
| // Write should include backlog:2 moved, and must NOT traverse or fetch dependencies from the target tag | ||
| expect(writeJSON).toHaveBeenCalledWith( | ||
| mockTasksPath, | ||
| expect.objectContaining({ | ||
| 'in-progress': expect.objectContaining({ | ||
| tasks: expect.arrayContaining([ | ||
| expect.objectContaining({ id: 1 }), | ||
| expect.objectContaining({ id: 2 }) // the backlog:2 now moved | ||
| // ensure existing in-progress:2 remains (by id) but we don't double-add or fetch deps from it | ||
| ]) | ||
| }) | ||
| }), | ||
| mockContext.projectRoot, | ||
| null | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make dependency traversal deterministic in this test.
Depending on implementation, dependency gathering may come from dependency-manager.getDependentTaskIds. Mock it to ensure id:2 is moved with id:1, keeping this test stable against internal refactors.
Apply:
it('should scope dependency traversal to source tag when using --with-dependencies', async () => {
findCrossTagDependencies.mockReturnValue([]);
validateSubtaskMove.mockImplementation(() => {});
+ getDependentTaskIds.mockReturnValue([2]);To verify whether moveTasksBetweenTags currently uses getDependentTaskIds, run:
🏁 Script executed:
#!/bin/bash
# Search for getDependentTaskIds usage in move-task.js
rg -n "getDependentTaskIds\\(" scripts/modules/task-manager/move-task.jsLength of output: 73
🏁 Script executed:
#!/bin/bash
# Search for usage of findCrossTagDependencies and dependency functions in move-task.js
rg -n "findCrossTagDependencies" -A3 scripts/modules/task-manager/move-task.js
# Search for getDependentTaskIds globally
rg -n "getDependentTaskIds" -C2 .
# Show import statements in move-task.js
rg -n "^import" scripts/modules/task-manager/move-task.jsLength of output: 11496
Ensure the test stubs the actual dependency resolver
It looks like moveTasksBetweenTags never calls getDependentTaskIds (it’s imported in scripts/modules/task-manager/move-task.js but not invoked), so adding
getDependentTaskIds.mockReturnValue([2]);won’t affect this test. You have two ways to restore determinism:
- Fix the implementation: in
scripts/modules/task-manager/move-task.js, invoke
getDependentTaskIds(sourceTasks, crossTagDependencies, allTasks)when{ withDependencies: true }.
Then in your test (tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js), before callingmoveTasksBetweenTags, add:findCrossTagDependencies.mockReturnValue([]); validateSubtaskMove.mockImplementation(() => {});
- getDependentTaskIds.mockReturnValue([2]);
- **Update the test to mock the real function used**: if you’ve switched to `findAllDependenciesRecursively` (per your integration tests), stub that instead:
```diff
findCrossTagDependencies.mockReturnValue([]);
validateSubtaskMove.mockImplementation(() => {});
+ findAllDependenciesRecursively.mockReturnValue([2]);
Either way, make sure the function your code actually calls is mocked to return [2] so the test consistently moves both id:1 and id:2.
🤖 Prompt for AI Agents
tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js lines
222-252: the test stubs getDependentTaskIds but the implementation under test
does not call that function (it calls a different dependency resolver), so the
test is non-deterministic; either update the implementation in
scripts/modules/task-manager/move-task.js to call
getDependentTaskIds(sourceTasks, crossTagDependencies, allTasks) when
options.withDependencies is true so the existing mock
getDependentTaskIds.mockReturnValue([2]) takes effect, or change this test to
mock the actual function the module calls (e.g., findAllDependenciesRecursively
or findCrossTagDependencies) by adding the appropriate mockReturnValue([2])
before invoking moveTasksBetweenTags so the resolver used by the code returns id
2 and the writeJSON assertion passes.
Crunchyman-ralph
left a comment
There was a problem hiding this comment.
lgtm, great PRs as always @mm-parthy
#1135) * docs: Auto-update and format models.md * docs(ui,cli): remove --force from cross-tag move guidance; recommend --with-dependencies/--ignore-dependencies - scripts/modules/ui.js: drop force tip in conflict resolution - scripts/modules/commands.js: remove force examples from move help - docs/cross-tag-task-movement.md: purge force mentions; add explicit with/ignore examples * test(move): update cross-tag move tests to drop --force; assert with/ignore deps behavior and current-tag fallback - CLI integration: remove force expectations, keep with/ignore, current-tag fallback - Integration: remove force-path test - Unit: add scoped traversal test, adjust fixtures to avoid id collision * fix(move): scope dependency traversal to source tag; tag-aware ignore-dependencies filtering - resolveDependencies: traverse only sourceTag tasks to avoid cross-tag contamination - filter dependent IDs to those present in source tag, numeric only - ignore-dependencies: drop deps pointing to tasks from sourceTag; keep targetTag deps * test(mcp): ensure cross-tag move passes only with/ignore options and returns conflict suggestions - new test: tests/unit/mcp/tools/move-task-cross-tag-options.test.js * feat(move): add advisory tips when ignoring cross-tag dependencies; add integration test case * feat(cli/move): improve ID collision UX for cross-tag moves\n\n- Print Next Steps tips when core returns them (e.g., after ignore-dependencies)\n- Add dedicated help block when an ID already exists in target tag * feat(move/mcp): improve ID collision UX and suggestions\n\n- Core: include suggestions on TASK_ALREADY_EXISTS errors\n- MCP: map ID collision to TASK_ALREADY_EXISTS with suggestions\n- Tests: add MCP unit test for ID collision suggestions * test(move/cli): print tips on ignore-dependencies results; print ID collision suggestions\n\n- CLI integration test: assert Next Steps tips printed when result.tips present\n- Integration test: assert TASK_ALREADY_EXISTS error includes suggestions payload * chore(changeset): add changeset for cross-tag move UX improvements (CLI/MCP/core/tests) * Add cross-tag task movement help and validation improvements - Introduced a detailed help command for cross-tag task movement, enhancing user guidance on usage and options. - Updated validation logic in `validateCrossTagMove` to include checks for indirect dependencies, improving accuracy in conflict detection. - Refactored tests to ensure comprehensive coverage of new validation scenarios and error handling. - Cleaned up documentation to reflect the latest changes in task movement functionality. * refactor(commands): remove redundant tips printing after move operation - Eliminated duplicate printing of tips for next steps after the move operation, streamlining the output for users. - This change enhances clarity by ensuring tips are only displayed when relevant, improving overall user experience. * docs(move): clarify "force move" options and improve examples - Updated documentation to replace the deprecated "force move" concept with clear alternatives: `--with-dependencies` and `--ignore-dependencies`. - Enhanced Scenario 3 with explicit options and improved inline comments for better readability. - Removed confusing commented code in favor of a straightforward note in the Force Move section. * chore: run formatter * Update .changeset/clarify-force-move-docs.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update docs/cross-tag-task-movement.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * test(move): add test for dependency traversal scoping with --with-dependencies option - Introduced a new test to ensure that the dependency traversal is limited to tasks from the source tag when using the --with-dependencies option, addressing potential ID collisions across tags. * test(move): enhance tips validation in cross-tag task movement integration test --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
eyaltoledano#1135) * docs: Auto-update and format models.md * docs(ui,cli): remove --force from cross-tag move guidance; recommend --with-dependencies/--ignore-dependencies - scripts/modules/ui.js: drop force tip in conflict resolution - scripts/modules/commands.js: remove force examples from move help - docs/cross-tag-task-movement.md: purge force mentions; add explicit with/ignore examples * test(move): update cross-tag move tests to drop --force; assert with/ignore deps behavior and current-tag fallback - CLI integration: remove force expectations, keep with/ignore, current-tag fallback - Integration: remove force-path test - Unit: add scoped traversal test, adjust fixtures to avoid id collision * fix(move): scope dependency traversal to source tag; tag-aware ignore-dependencies filtering - resolveDependencies: traverse only sourceTag tasks to avoid cross-tag contamination - filter dependent IDs to those present in source tag, numeric only - ignore-dependencies: drop deps pointing to tasks from sourceTag; keep targetTag deps * test(mcp): ensure cross-tag move passes only with/ignore options and returns conflict suggestions - new test: tests/unit/mcp/tools/move-task-cross-tag-options.test.js * feat(move): add advisory tips when ignoring cross-tag dependencies; add integration test case * feat(cli/move): improve ID collision UX for cross-tag moves\n\n- Print Next Steps tips when core returns them (e.g., after ignore-dependencies)\n- Add dedicated help block when an ID already exists in target tag * feat(move/mcp): improve ID collision UX and suggestions\n\n- Core: include suggestions on TASK_ALREADY_EXISTS errors\n- MCP: map ID collision to TASK_ALREADY_EXISTS with suggestions\n- Tests: add MCP unit test for ID collision suggestions * test(move/cli): print tips on ignore-dependencies results; print ID collision suggestions\n\n- CLI integration test: assert Next Steps tips printed when result.tips present\n- Integration test: assert TASK_ALREADY_EXISTS error includes suggestions payload * chore(changeset): add changeset for cross-tag move UX improvements (CLI/MCP/core/tests) * Add cross-tag task movement help and validation improvements - Introduced a detailed help command for cross-tag task movement, enhancing user guidance on usage and options. - Updated validation logic in `validateCrossTagMove` to include checks for indirect dependencies, improving accuracy in conflict detection. - Refactored tests to ensure comprehensive coverage of new validation scenarios and error handling. - Cleaned up documentation to reflect the latest changes in task movement functionality. * refactor(commands): remove redundant tips printing after move operation - Eliminated duplicate printing of tips for next steps after the move operation, streamlining the output for users. - This change enhances clarity by ensuring tips are only displayed when relevant, improving overall user experience. * docs(move): clarify "force move" options and improve examples - Updated documentation to replace the deprecated "force move" concept with clear alternatives: `--with-dependencies` and `--ignore-dependencies`. - Enhanced Scenario 3 with explicit options and improved inline comments for better readability. - Removed confusing commented code in favor of a straightforward note in the Force Move section. * chore: run formatter * Update .changeset/clarify-force-move-docs.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update docs/cross-tag-task-movement.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update tests/unit/scripts/modules/task-manager/move-task-cross-tag.test.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * test(move): add test for dependency traversal scoping with --with-dependencies option - Introduced a new test to ensure that the dependency traversal is limited to tasks from the source tag when using the --with-dependencies option, addressing potential ID collisions across tags. * test(move): enhance tips validation in cross-tag task movement integration test --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Improve cross-tag move UX and safety across Core, CLI, and MCP:
--with-dependencies. When--ignore-dependenciesis used and cross-tag deps are broken, return actionable “Next Steps” tips.TASK_ALREADY_EXISTSand include suggestions in the error payload; ensure onlywithDependencies/ignoreDependenciesare passed (no force).Type of Change
Testing
Highlights:
result.tips; prints ID collision help blockTASK_ALREADY_EXISTSincludes suggestions payloadChangeset
Package: task-master-ai (minor)
File:
.changeset/strong-eagles-vanish.mdSummary:
Additional Notes
--with-dependencies(future flag idea:--with-reverse-dependents)Summary by CodeRabbit
New Features
Documentation
Tests