Improve cross-tag move UX and safety; add MCP suggestions and CLI tips (#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>
This commit is contained in:
Parthy
@@ -61,6 +61,43 @@ const MOVE_ERROR_CODES = {
|
||||
INVALID_TARGET_TAG: 'INVALID_TARGET_TAG'
|
||||
};
|
||||
|
||||
/**
|
||||
* Normalize a dependency value to its numeric parent task ID.
|
||||
* - Numbers are returned as-is (if finite)
|
||||
* - Numeric strings are parsed ("5" -> 5)
|
||||
* - Dotted strings return the parent portion ("5.2" -> 5)
|
||||
* - Empty/invalid values return null
|
||||
* - null/undefined are preserved
|
||||
* @param {number|string|null|undefined} dep
|
||||
* @returns {number|null|undefined}
|
||||
*/
|
||||
function normalizeDependency(dep) {
|
||||
if (dep === null || dep === undefined) return dep;
|
||||
if (typeof dep === 'number') return Number.isFinite(dep) ? dep : null;
|
||||
if (typeof dep === 'string') {
|
||||
const trimmed = dep.trim();
|
||||
if (trimmed === '') return null;
|
||||
const parentPart = trimmed.includes('.') ? trimmed.split('.')[0] : trimmed;
|
||||
const parsed = parseInt(parentPart, 10);
|
||||
return Number.isFinite(parsed) ? parsed : null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize an array of dependency values to numeric IDs.
|
||||
* Preserves null/undefined input (returns as-is) and filters out invalid entries.
|
||||
* @param {Array<any>|null|undefined} deps
|
||||
* @returns {Array<number>|null|undefined}
|
||||
*/
|
||||
function normalizeDependencies(deps) {
|
||||
if (deps === null || deps === undefined) return deps;
|
||||
if (!Array.isArray(deps)) return deps;
|
||||
return deps
|
||||
.map((d) => normalizeDependency(d))
|
||||
.filter((n) => Number.isFinite(n));
|
||||
}
|
||||
|
||||
/**
|
||||
* Move one or more tasks/subtasks to new positions
|
||||
* @param {string} tasksPath - Path to tasks.json file
|
||||
@@ -674,15 +711,34 @@ async function resolveDependencies(
|
||||
) {
|
||||
const { withDependencies = false, ignoreDependencies = false } = options;
|
||||
|
||||
// Scope allTasks to the source tag to avoid cross-tag contamination when
|
||||
// computing dependency chains for --with-dependencies
|
||||
const tasksInSourceTag = Array.isArray(allTasks)
|
||||
? allTasks.filter((t) => t && t.tag === sourceTag)
|
||||
: [];
|
||||
|
||||
// Handle --with-dependencies flag first (regardless of cross-tag dependencies)
|
||||
if (withDependencies) {
|
||||
// Move dependent tasks along with main tasks
|
||||
// Find ALL dependencies recursively within the same tag
|
||||
const allDependentTaskIds = findAllDependenciesRecursively(
|
||||
// Find ALL dependencies recursively, but only using tasks from the source tag
|
||||
const allDependentTaskIdsRaw = findAllDependenciesRecursively(
|
||||
sourceTasks,
|
||||
allTasks,
|
||||
tasksInSourceTag,
|
||||
{ maxDepth: 100, includeSelf: false }
|
||||
);
|
||||
|
||||
// Filter dependent IDs to those that actually exist in the source tag
|
||||
const sourceTagIds = new Set(
|
||||
tasksInSourceTag.map((t) =>
|
||||
typeof t.id === 'string' ? parseInt(t.id, 10) : t.id
|
||||
)
|
||||
);
|
||||
const allDependentTaskIds = allDependentTaskIdsRaw.filter((depId) => {
|
||||
// Only numeric task IDs are eligible to be moved (subtasks cannot be moved cross-tag)
|
||||
const normalizedId = normalizeDependency(depId);
|
||||
return Number.isFinite(normalizedId) && sourceTagIds.has(normalizedId);
|
||||
});
|
||||
|
||||
const allTaskIdsToMove = [...new Set([...taskIds, ...allDependentTaskIds])];
|
||||
|
||||
log(
|
||||
@@ -711,22 +767,31 @@ async function resolveDependencies(
|
||||
if (ignoreDependencies) {
|
||||
// Break cross-tag dependencies (edge case - shouldn't normally happen)
|
||||
sourceTasks.forEach((task) => {
|
||||
const sourceTagTasks = tasksInSourceTag;
|
||||
const targetTagTasks = Array.isArray(allTasks)
|
||||
? allTasks.filter((t) => t && t.tag === targetTag)
|
||||
: [];
|
||||
task.dependencies = task.dependencies.filter((depId) => {
|
||||
// Handle both task IDs and subtask IDs (e.g., "1.2")
|
||||
let depTask = null;
|
||||
if (typeof depId === 'string' && depId.includes('.')) {
|
||||
// It's a subtask ID - extract parent task ID and find the parent task
|
||||
const [parentId, subtaskId] = depId
|
||||
.split('.')
|
||||
.map((id) => parseInt(id, 10));
|
||||
depTask = allTasks.find((t) => t.id === parentId);
|
||||
} else {
|
||||
// It's a regular task ID - normalize to number for comparison
|
||||
const normalizedDepId =
|
||||
typeof depId === 'string' ? parseInt(depId, 10) : depId;
|
||||
depTask = allTasks.find((t) => t.id === normalizedDepId);
|
||||
const parentTaskId = normalizeDependency(depId);
|
||||
|
||||
// If dependency resolves to a task in the source tag, drop it (would be cross-tag after move)
|
||||
if (
|
||||
Number.isFinite(parentTaskId) &&
|
||||
sourceTagTasks.some((t) => t.id === parentTaskId)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return !depTask || depTask.tag === targetTag;
|
||||
|
||||
// If dependency resolves to a task in the target tag, keep it
|
||||
if (
|
||||
Number.isFinite(parentTaskId) &&
|
||||
targetTagTasks.some((t) => t.id === parentTaskId)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Otherwise, keep as-is (unknown/unresolved dependency)
|
||||
return true;
|
||||
});
|
||||
});
|
||||
|
||||
@@ -810,7 +875,16 @@ async function executeMoveOperation(
|
||||
if (existingTaskIndex !== -1) {
|
||||
throw new MoveTaskError(
|
||||
MOVE_ERROR_CODES.TASK_ALREADY_EXISTS,
|
||||
`Task ${taskId} already exists in target tag "${targetTag}"`
|
||||
`Task ${taskId} already exists in target tag "${targetTag}"`,
|
||||
{
|
||||
conflictingId: normalizedTaskId,
|
||||
targetTag,
|
||||
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'
|
||||
]
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -851,7 +925,8 @@ async function finalizeMove(
|
||||
tasksPath,
|
||||
context,
|
||||
sourceTag,
|
||||
targetTag
|
||||
targetTag,
|
||||
dependencyResolution
|
||||
) {
|
||||
const { projectRoot } = context;
|
||||
const { rawData, movedTasks } = moveResult;
|
||||
@@ -859,10 +934,23 @@ async function finalizeMove(
|
||||
// Write the updated data
|
||||
writeJSON(tasksPath, rawData, projectRoot, null);
|
||||
|
||||
return {
|
||||
const response = {
|
||||
message: `Successfully moved ${movedTasks.length} tasks from "${sourceTag}" to "${targetTag}"`,
|
||||
movedTasks
|
||||
};
|
||||
|
||||
// If we intentionally broke cross-tag dependencies, provide tips to validate & fix
|
||||
if (
|
||||
dependencyResolution &&
|
||||
dependencyResolution.type === 'ignored-dependencies'
|
||||
) {
|
||||
response.tips = [
|
||||
'Run "task-master validate-dependencies" to check for dependency issues.',
|
||||
'Run "task-master fix-dependencies" to automatically repair dangling dependencies.'
|
||||
];
|
||||
}
|
||||
|
||||
return response;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -898,7 +986,7 @@ async function moveTasksBetweenTags(
|
||||
const { rawData, sourceTasks, allTasks } = await prepareTaskData(validation);
|
||||
|
||||
// 3. Handle dependencies
|
||||
const { tasksToMove } = await resolveDependencies(
|
||||
const { tasksToMove, dependencyResolution } = await resolveDependencies(
|
||||
sourceTasks,
|
||||
allTasks,
|
||||
options,
|
||||
@@ -923,7 +1011,8 @@ async function moveTasksBetweenTags(
|
||||
tasksPath,
|
||||
context,
|
||||
sourceTag,
|
||||
targetTag
|
||||
targetTag,
|
||||
dependencyResolution
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user