fix(tags): Resolve critical tag deletion and migration notice bugs

Major Issues Fixed:

1. Tag Deletion Bug: Fixed critical issue where creating subtasks would delete other tags

   - Root cause: writeJSON function wasn't accepting projectRoot/tag parameters

   - Fixed writeJSON signature and logic to handle tagged data structure

   - Added proper merging of resolved tag data back into full tagged structure

2. Persistent Migration Notice: Fixed FYI notice showing after every command

   - Root cause: markMigrationForNotice was resetting migrationNoticeShown to false

   - Fixed migration logic to only trigger on actual legacy->tagged migrations

   - Added proper _rawTaggedData checks to prevent false migration detection

3. Data Corruption Prevention: Enhanced data integrity safeguards

   - Fixed writeJSON to filter out internal properties

   - Added automatic cleanup of rogue properties

   - Improved hasTaggedStructure detection logic

Commands Fixed: add-subtask, remove-subtask, and all commands now preserve tags correctly
This commit is contained in:
Eyal Toledano
2025-06-12 23:43:48 -04:00
parent b3ec151b27
commit 4585a6bbc7
9 changed files with 205 additions and 75 deletions

View File

@@ -2095,11 +2095,19 @@ ${result.result}
)
.option('-s, --status <status>', 'Status for the new subtask', 'pending')
.option('--skip-generate', 'Skip regenerating task files')
.option('--tag <tag>', 'Specify tag context for task operations')
.action(async (options) => {
const projectRoot = findProjectRoot();
if (!projectRoot) {
console.error(chalk.red('Error: Could not find project root.'));
process.exit(1);
}
const tasksPath = options.file || TASKMASTER_TASKS_FILE;
const parentId = options.parent;
const existingTaskId = options.taskId;
const generateFiles = !options.skipGenerate;
const tag = options.tag;
if (!parentId) {
console.error(
@@ -2133,7 +2141,8 @@ ${result.result}
parentId,
existingTaskId,
null,
generateFiles
generateFiles,
{ projectRoot, tag }
);
console.log(
chalk.green(
@@ -2159,7 +2168,8 @@ ${result.result}
parentId,
null,
newSubtaskData,
generateFiles
generateFiles,
{ projectRoot, tag }
);
console.log(
chalk.green(
@@ -3791,7 +3801,9 @@ async function runCLI(argv = process.argv) {
// Migration has occurred, check if we've shown the notice
let stateData = { migrationNoticeShown: false };
if (fs.existsSync(statePath)) {
stateData = readJSON(statePath) || stateData;
// Read state.json directly without tag resolution since it's not a tagged file
const rawStateData = fs.readFileSync(statePath, 'utf8');
stateData = JSON.parse(rawStateData) || stateData;
}
if (!stateData.migrationNoticeShown) {
@@ -3799,7 +3811,8 @@ async function runCLI(argv = process.argv) {
// Mark as shown
stateData.migrationNoticeShown = true;
writeJSON(statePath, stateData);
// Write state.json directly without tag resolution since it's not a tagged file
fs.writeFileSync(statePath, JSON.stringify(stateData, null, 2));
}
}
}

View File

@@ -1120,9 +1120,16 @@ function ensureAtLeastOneIndependentSubtask(tasksData) {
* This function is designed to be called after any task modification
* @param {Object} tasksData - The tasks data object with tasks array
* @param {string} tasksPath - Optional path to save the changes
* @param {string} projectRoot - Optional project root for tag context
* @param {string} tag - Optional tag for tag context
* @returns {boolean} - True if any changes were made
*/
function validateAndFixDependencies(tasksData, tasksPath = null) {
function validateAndFixDependencies(
tasksData,
tasksPath = null,
projectRoot = null,
tag = null
) {
if (!tasksData || !tasksData.tasks || !Array.isArray(tasksData.tasks)) {
log('error', 'Invalid tasks data');
return false;
@@ -1209,7 +1216,7 @@ function validateAndFixDependencies(tasksData, tasksPath = null) {
// Save changes if needed
if (tasksPath && changesDetected) {
try {
writeJSON(tasksPath, tasksData);
writeJSON(tasksPath, tasksData, projectRoot, tag);
log('debug', 'Saved dependency fixes to tasks.json');
} catch (error) {
log('error', 'Failed to save dependency fixes to tasks.json', error);

View File

@@ -11,6 +11,7 @@ import generateTaskFiles from './generate-task-files.js';
* @param {number|string|null} existingTaskId - ID of an existing task to convert to subtask (optional)
* @param {Object} newSubtaskData - Data for creating a new subtask (used if existingTaskId is null)
* @param {boolean} generateFiles - Whether to regenerate task files after adding the subtask
* @param {Object} context - Context object containing projectRoot and tag information
* @returns {Object} The newly created or converted subtask
*/
async function addSubtask(
@@ -18,13 +19,14 @@ async function addSubtask(
parentId,
existingTaskId = null,
newSubtaskData = null,
generateFiles = true
generateFiles = true,
context = {}
) {
try {
log('info', `Adding subtask to parent task ${parentId}...`);
// Read the existing tasks
const data = readJSON(tasksPath);
// Read the existing tasks with proper context
const data = readJSON(tasksPath, context.projectRoot, context.tag);
if (!data || !data.tasks) {
throw new Error(`Invalid or missing tasks file at ${tasksPath}`);
}
@@ -134,13 +136,13 @@ async function addSubtask(
);
}
// Write the updated tasks back to the file
writeJSON(tasksPath, data);
// Write the updated tasks back to the file with proper context
writeJSON(tasksPath, data, context.projectRoot, context.tag);
// Generate task files if requested
if (generateFiles) {
log('info', 'Regenerating task files...');
await generateTaskFiles(tasksPath, path.dirname(tasksPath));
await generateTaskFiles(tasksPath, path.dirname(tasksPath), context);
}
return newSubtask;

View File

@@ -226,12 +226,18 @@ async function addTask(
}
// Handle legacy format migration using utilities
if (rawData && Array.isArray(rawData.tasks)) {
report('Migrating legacy tasks.json format to tagged format...', 'info');
const legacyTasks = rawData.tasks;
if (rawData && Array.isArray(rawData.tasks) && !rawData._rawTaggedData) {
report('Legacy format detected. Migrating to tagged format...', 'info');
// This is legacy format - migrate it to tagged format
rawData = {
master: {
tasks: legacyTasks
tasks: rawData.tasks,
metadata: rawData.metadata || {
created: new Date().toISOString(),
updated: new Date().toISOString(),
description: 'Tasks for master context'
}
}
};
// Ensure proper metadata using utility

View File

@@ -11,7 +11,7 @@ import { getDebugFlag } from '../config-manager.js';
* Generate individual task files from tasks.json
* @param {string} tasksPath - Path to the tasks.json file
* @param {string} outputDir - Output directory for task files
* @param {Object} options - Additional options (mcpLog for MCP mode)
* @param {Object} options - Additional options (mcpLog for MCP mode, projectRoot, tag)
* @returns {Object|undefined} Result object in MCP mode, undefined in CLI mode
*/
function generateTaskFiles(tasksPath, outputDir, options = {}) {
@@ -19,7 +19,7 @@ function generateTaskFiles(tasksPath, outputDir, options = {}) {
// Determine if we're in MCP mode by checking for mcpLog
const isMcpMode = !!options?.mcpLog;
const data = readJSON(tasksPath);
const data = readJSON(tasksPath, options.projectRoot, options.tag);
if (!data || !data.tasks) {
throw new Error(`No valid tasks found in ${tasksPath}`);
}
@@ -33,7 +33,12 @@ function generateTaskFiles(tasksPath, outputDir, options = {}) {
// Validate and fix dependencies before generating files
log('info', `Validating and fixing dependencies`);
validateAndFixDependencies(data, tasksPath);
validateAndFixDependencies(
data,
tasksPath,
options.projectRoot,
options.tag
);
// Get valid task IDs from tasks.json
const validTaskIds = data.tasks.map((task) => task.id);

View File

@@ -566,10 +566,10 @@ async function tags(
tagList.push({
name: tagName,
isCurrent: tagName === currentTag,
taskCount: tasks.length,
completedTasks: tasks.filter(
(t) => t.status === 'done' || t.status === 'completed'
).length,
tasks: tasks || [],
created: metadata.created || 'Unknown',
description: metadata.description || 'No description'
});
@@ -634,7 +634,7 @@ async function tags(
row.push(tagDisplay);
if (showTaskCounts) {
row.push(chalk.white(tag.taskCount.toString()));
row.push(chalk.white(tag.tasks.length.toString()));
row.push(chalk.green(tag.completedTasks.toString()));
}
@@ -1034,8 +1034,7 @@ async function copyTag(
`Copy of "${sourceName}" created on ${new Date().toLocaleDateString()}`,
copiedFrom: {
tag: sourceName,
date: new Date().toISOString(),
taskCount: sourceTasks.length
date: new Date().toISOString()
}
}
};
@@ -1061,7 +1060,6 @@ async function copyTag(
sourceName,
targetName,
copied: true,
taskCount: sourceTasks.length,
description:
description ||
`Copy of "${sourceName}" created on ${new Date().toLocaleDateString()}`
@@ -1091,7 +1089,6 @@ async function copyTag(
sourceName,
targetName,
copied: true,
taskCount: sourceTasks.length,
description:
description ||
`Copy of "${sourceName}" created on ${new Date().toLocaleDateString()}`

View File

@@ -193,6 +193,29 @@ function log(level, ...args) {
}
}
/**
* Checks if the data object has a tagged structure (contains tag objects with tasks arrays)
* @param {Object} data - The data object to check
* @returns {boolean} True if the data has a tagged structure
*/
function hasTaggedStructure(data) {
if (!data || typeof data !== 'object') {
return false;
}
// Check if any top-level properties are objects with tasks arrays
for (const key in data) {
if (
data.hasOwnProperty(key) &&
typeof data[key] === 'object' &&
Array.isArray(data[key].tasks)
) {
return true;
}
}
return false;
}
/**
* Reads and parses a JSON file
* @param {string} filepath - Path to the JSON file
@@ -243,7 +266,12 @@ function readJSON(filepath, projectRoot = null, tag = null) {
}
// Check if this is legacy format that needs migration
if (Array.isArray(data.tasks)) {
// Only migrate if we have tasks at the ROOT level AND no tag-like structure
if (
Array.isArray(data.tasks) &&
!data._rawTaggedData &&
!hasTaggedStructure(data)
) {
if (isDebug) {
console.log(`File is in legacy format, performing migration...`);
}
@@ -523,13 +551,11 @@ function createStateJson(statePath) {
/**
* Marks in state.json that migration occurred and notice should be shown
* @param {string} tasksJsonPath - Path to the tasks.json file that was migrated
* @param {string} tasksJsonPath - Path to the tasks.json file
*/
function markMigrationForNotice(tasksJsonPath) {
try {
const projectRoot =
findProjectRoot(path.dirname(tasksJsonPath)) ||
path.dirname(tasksJsonPath);
const projectRoot = path.dirname(path.dirname(tasksJsonPath));
const statePath = path.join(projectRoot, '.taskmaster', 'state.json');
// Ensure state.json exists
@@ -541,8 +567,8 @@ function markMigrationForNotice(tasksJsonPath) {
try {
const rawState = fs.readFileSync(statePath, 'utf8');
const stateData = JSON.parse(rawState) || {};
if (stateData.migrationNoticeShown !== false) {
// Set to false to trigger notice display
// Only set to false if it's not already set (i.e., first time migration)
if (stateData.migrationNoticeShown === undefined) {
stateData.migrationNoticeShown = false;
fs.writeFileSync(statePath, JSON.stringify(stateData, null, 2), 'utf8');
}
@@ -563,43 +589,51 @@ function markMigrationForNotice(tasksJsonPath) {
}
/**
* Writes data to a JSON file
* Writes and saves a JSON file. Handles tagged task lists properly.
* @param {string} filepath - Path to the JSON file
* @param {Object} data - Data to write
* @param {Object} data - Data to write (can be resolved tag data or raw tagged data)
* @param {string} projectRoot - Optional project root for tag context
* @param {string} tag - Optional tag for tag context
*/
function writeJSON(filepath, data) {
// GUARD: Prevent circular dependency during config loading
let isDebug = false; // Default fallback
try {
// Only try to get debug flag if we're not in the middle of config loading
isDebug = getDebugFlag();
} catch (error) {
// If getDebugFlag() fails (likely due to circular dependency),
// use default false and continue
isDebug = false;
}
function writeJSON(filepath, data, projectRoot = null, tag = null) {
const isDebug = process.env.TASKMASTER_DEBUG === 'true';
try {
const dir = path.dirname(filepath);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true });
let finalData = data;
// If we have _rawTaggedData, this means we're working with resolved tag data
// and need to merge it back into the full tagged structure
if (data && data._rawTaggedData && projectRoot) {
const resolvedTag = tag || getCurrentTag(projectRoot);
// Get the original tagged data
const originalTaggedData = data._rawTaggedData;
// Create a clean copy of the current resolved data (without internal properties)
const { _rawTaggedData, tag: _, ...cleanResolvedData } = data;
// Update the specific tag with the resolved data
finalData = {
...originalTaggedData,
[resolvedTag]: cleanResolvedData
};
if (isDebug) {
console.log(
`writeJSON: Merging resolved data back into tag '${resolvedTag}'`
);
}
}
// Clean the data before writing - remove internal properties that should not be persisted
let cleanData = data;
if (data && typeof data === 'object') {
// First, filter out top-level internal properties
if (data._rawTaggedData !== undefined || data.tag !== undefined) {
const { _rawTaggedData, tag, ...cleanedData } = data;
cleanData = cleanedData;
}
// Clean up any internal properties that shouldn't be persisted
let cleanData = finalData;
if (cleanData && typeof cleanData === 'object') {
// Remove any _rawTaggedData or tag properties from root level
const { _rawTaggedData, tag: tagProp, ...rootCleanData } = cleanData;
cleanData = rootCleanData;
// For tagged task data, also clean up any rogue properties in tag objects
if (
filepath.includes('tasks.json') &&
cleanData &&
typeof cleanData === 'object'
) {
// Additional cleanup for tag objects
if (typeof cleanData === 'object' && !Array.isArray(cleanData)) {
const finalCleanData = {};
for (const [key, value] of Object.entries(cleanData)) {
if (
@@ -628,11 +662,13 @@ function writeJSON(filepath, data) {
}
fs.writeFileSync(filepath, JSON.stringify(cleanData, null, 2), 'utf8');
if (isDebug) {
console.log(`writeJSON: Successfully wrote to ${filepath}`);
}
} catch (error) {
log('error', `Error writing JSON file ${filepath}:`, error.message);
if (isDebug) {
// Use dynamic debug flag
// Use log utility for debug output too
log('error', 'Full error details:', error);
}
}