ros/claude-task-master
1
0
Fork 0
mirror of https://github.com/eyaltoledano/claude-task-master.git synced 2026-01-29 22:02:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: adopt modifyJson pattern for atomic read-modify-write operations (#1569)

Browse Source 
Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
Co-authored-by: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com>
Resolves issue #1568.
This commit is contained in:
Ben Coombs
2026-01-14 18:20:55 +00:00
committed by GitHub
parent e762e4f646
commit 4cfde1c3d5
3 changed files with 154 additions and 147 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
.changeset/adopt-modifyjson-pattern.md Normal file
View File

@@ -0,0 +1,9 @@
---
"task-master-ai": patch
---
Improve concurrency safety by adopting modifyJson pattern in file-storage
- Refactor saveTasks, createTag, deleteTag, renameTag to use modifyJson for atomic read-modify-write operations
- This prevents lost updates when multiple processes concurrently modify tasks.json
- Complements the cross-process file locking added in PR #1566

281
packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts
View File

@@ -220,6 +220,7 @@ export class FileStorage implements IStorage {
/**
* Save tasks for a specific tag in the single tasks.json file
* Uses modifyJson for atomic read-modify-write to prevent lost updates
*/
async saveTasks(tasks: Task[], tag?: string): Promise<void> {
const filePath = this.pathResolver.getTasksPath();
@@ -228,17 +229,6 @@ export class FileStorage implements IStorage {
// Ensure directory exists
await this.fileOps.ensureDir(this.pathResolver.getTasksDir());
// Get existing data from the file
let existingData: any = {};
try {
existingData = await this.fileOps.readJson(filePath);
} catch (error: any) {
if (error.code !== 'ENOENT') {
throw new Error(`Failed to read existing tasks: ${error.message}`);
}
// File doesn't exist, start with empty data
}
// Create metadata for this tag
const metadata: TaskMetadata = {
version: '1.0.0',
@@ -251,43 +241,44 @@ export class FileStorage implements IStorage {
// Normalize tasks
const normalizedTasks = this.normalizeTaskIds(tasks);
// Update the specific tag in the existing data structure
if (
this.formatHandler.detectFormat(existingData) === 'legacy' ||
Object.keys(existingData).some(
(key) => key !== 'tasks' && key !== 'metadata'
)
) {
// Legacy format - update/add the tag
existingData[resolvedTag] = {
tasks: normalizedTasks,
metadata
};
} else if (resolvedTag === 'master') {
// Standard format for master tag
existingData = {
tasks: normalizedTasks,
metadata
};
} else {
// Convert to legacy format when adding non-master tags
const masterTasks = existingData.tasks || [];
const masterMetadata = existingData.metadata || metadata;
existingData = {
master: {
tasks: masterTasks,
metadata: masterMetadata
},
[resolvedTag]: {
// Use modifyJson for atomic read-modify-write
await this.fileOps.modifyJson(filePath, (existingData: any) => {
// Update the specific tag in the existing data structure
if (
this.formatHandler.detectFormat(existingData) === 'legacy' ||
Object.keys(existingData).some(
(key) => key !== 'tasks' && key !== 'metadata'
)
) {
// Legacy format - update/add the tag
existingData[resolvedTag] = {
tasks: normalizedTasks,
metadata
}
};
}
};
return existingData;
} else if (resolvedTag === 'master') {
// Standard format for master tag
return {
tasks: normalizedTasks,
metadata
};
} else {
// Convert to legacy format when adding non-master tags
const masterTasks = existingData.tasks || [];
const masterMetadata = existingData.metadata || metadata;
// Write the updated file
await this.fileOps.writeJson(filePath, existingData);
return {
master: {
tasks: masterTasks,
metadata: masterMetadata
},
[resolvedTag]: {
tasks: normalizedTasks,
metadata
}
};
}
});
}
/**
@@ -594,6 +585,7 @@ export class FileStorage implements IStorage {
/**
* Create a new tag in the tasks.json file
* Uses modifyJson for atomic read-modify-write to prevent lost updates
*/
async createTag(
tagName: string,
@@ -602,62 +594,33 @@ export class FileStorage implements IStorage {
const filePath = this.pathResolver.getTasksPath();
try {
const existingData = await this.fileOps.readJson(filePath);
const format = this.formatHandler.detectFormat(existingData);
await this.fileOps.modifyJson(filePath, (existingData: any) => {
const format = this.formatHandler.detectFormat(existingData);
if (format === 'legacy') {
// Legacy format - add new tag key
if (tagName in existingData) {
throw new TaskMasterError(
`Tag ${tagName} already exists`,
ERROR_CODES.VALIDATION_ERROR
);
}
// Get tasks to copy if specified
let tasksToCopy = [];
if (options?.copyFrom) {
if (
options.copyFrom in existingData &&
existingData[options.copyFrom].tasks
) {
tasksToCopy = JSON.parse(
JSON.stringify(existingData[options.copyFrom].tasks)
if (format === 'legacy') {
// Legacy format - add new tag key
if (tagName in existingData) {
throw new TaskMasterError(
`Tag ${tagName} already exists`,
ERROR_CODES.VALIDATION_ERROR
);
}
}
// Create new tag structure
existingData[tagName] = {
tasks: tasksToCopy,
metadata: {
created: new Date().toISOString(),
updatedAt: new Date().toISOString(),
description:
options?.description ||
`Tag created on ${new Date().toLocaleDateString()}`,
tags: [tagName]
// Get tasks to copy if specified
let tasksToCopy: any[] = [];
if (options?.copyFrom) {
if (
options.copyFrom in existingData &&
existingData[options.copyFrom].tasks
) {
tasksToCopy = JSON.parse(
JSON.stringify(existingData[options.copyFrom].tasks)
);
}
}
};
await this.fileOps.writeJson(filePath, existingData);
} else {
// Standard format - need to convert to legacy format first
const masterTasks = existingData.tasks || [];
const masterMetadata = existingData.metadata || {};
// Get tasks to copy (from master in this case)
let tasksToCopy = [];
if (options?.copyFrom === 'master' || !options?.copyFrom) {
tasksToCopy = JSON.parse(JSON.stringify(masterTasks));
}
const newData = {
master: {
tasks: masterTasks,
metadata: { ...masterMetadata, tags: ['master'] }
},
[tagName]: {
// Create new tag structure
existingData[tagName] = {
tasks: tasksToCopy,
metadata: {
created: new Date().toISOString(),
@@ -667,11 +630,39 @@ export class FileStorage implements IStorage {
`Tag created on ${new Date().toLocaleDateString()}`,
tags: [tagName]
}
}
};
};
await this.fileOps.writeJson(filePath, newData);
}
return existingData;
} else {
// Standard format - need to convert to legacy format first
const masterTasks = existingData.tasks || [];
const masterMetadata = existingData.metadata || {};
// Get tasks to copy (from master in this case)
let tasksToCopy: any[] = [];
if (options?.copyFrom === 'master' || !options?.copyFrom) {
tasksToCopy = JSON.parse(JSON.stringify(masterTasks));
}
return {
master: {
tasks: masterTasks,
metadata: { ...masterMetadata, tags: ['master'] }
},
[tagName]: {
tasks: tasksToCopy,
metadata: {
created: new Date().toISOString(),
updatedAt: new Date().toISOString(),
description:
options?.description ||
`Tag created on ${new Date().toLocaleDateString()}`,
tags: [tagName]
}
}
};
}
});
} catch (error: any) {
if (error.code === 'ENOENT') {
throw new Error('Tasks file not found - initialize project first');
@@ -682,26 +673,41 @@ export class FileStorage implements IStorage {
/**
* Delete a tag from the single tasks.json file
* Uses modifyJson for atomic read-modify-write to prevent lost updates
*/
async deleteTag(tag: string): Promise<void> {
const filePath = this.pathResolver.getTasksPath();
try {
const existingData = await this.fileOps.readJson(filePath);
// Use modifyJson to handle all cases atomically
let shouldDeleteFile = false;
if (this.formatHandler.detectFormat(existingData) === 'legacy') {
// Legacy format - remove the tag key
if (tag in existingData) {
delete existingData[tag];
await this.fileOps.writeJson(filePath, existingData);
} else {
throw new Error(`Tag ${tag} not found`);
await this.fileOps.modifyJson(filePath, (data: any) => {
if (
this.formatHandler.detectFormat(data) !== 'legacy' &&
tag === 'master'
) {
// Standard format - mark for file deletion after lock release
shouldDeleteFile = true;
return data; // Return unchanged, we'll delete the file after
}
} else if (tag === 'master') {
// Standard format - delete the entire file for master tag
if (this.formatHandler.detectFormat(data) === 'legacy') {
// Legacy format - remove the tag key
if (tag in data) {
delete data[tag];
return data;
} else {
throw new Error(`Tag ${tag} not found`);
}
} else {
throw new Error(`Tag ${tag} not found in standard format`);
}
});
// Delete the file if we're removing master tag from standard format
if (shouldDeleteFile) {
await this.fileOps.deleteFile(filePath);
} else {
throw new Error(`Tag ${tag} not found in standard format`);
}
} catch (error: any) {
if (error.code === 'ENOENT') {
@@ -713,44 +719,43 @@ export class FileStorage implements IStorage {
/**
* Rename a tag within the single tasks.json file
* Uses modifyJson for atomic read-modify-write to prevent lost updates
*/
async renameTag(oldTag: string, newTag: string): Promise<void> {
const filePath = this.pathResolver.getTasksPath();
try {
const existingData = await this.fileOps.readJson(filePath);
await this.fileOps.modifyJson(filePath, (existingData: any) => {
if (this.formatHandler.detectFormat(existingData) === 'legacy') {
// Legacy format - rename the tag key
if (oldTag in existingData) {
existingData[newTag] = existingData[oldTag];
delete existingData[oldTag];
if (this.formatHandler.detectFormat(existingData) === 'legacy') {
// Legacy format - rename the tag key
if (oldTag in existingData) {
existingData[newTag] = existingData[oldTag];
delete existingData[oldTag];
// Update metadata tags array
if (existingData[newTag].metadata) {
existingData[newTag].metadata.tags = [newTag];
}
// Update metadata tags array
if (existingData[newTag].metadata) {
existingData[newTag].metadata.tags = [newTag];
return existingData;
} else {
throw new Error(`Tag ${oldTag} not found`);
}
} else if (oldTag === 'master') {
// Convert standard format to legacy when renaming master
const masterTasks = existingData.tasks || [];
const masterMetadata = existingData.metadata || {};
await this.fileOps.writeJson(filePath, existingData);
return {
[newTag]: {
tasks: masterTasks,
metadata: { ...masterMetadata, tags: [newTag] }
}
};
} else {
throw new Error(`Tag ${oldTag} not found`);
throw new Error(`Tag ${oldTag} not found in standard format`);
}
} else if (oldTag === 'master') {
// Convert standard format to legacy when renaming master
const masterTasks = existingData.tasks || [];
const masterMetadata = existingData.metadata || {};
const newData = {
[newTag]: {
tasks: masterTasks,
metadata: { ...masterMetadata, tags: [newTag] }
}
};
await this.fileOps.writeJson(filePath, newData);
} else {
throw new Error(`Tag ${oldTag} not found in standard format`);
}
});
} catch (error: any) {
if (error.code === 'ENOENT') {
throw new Error(`Tag ${oldTag} not found - file doesn't exist`);

11
tests/unit/file-locking.test.js
View File

@@ -3,18 +3,11 @@
* Verifies that concurrent access to tasks.json is properly serialized
*/
import {
jest,
describe,
it,
expect,
beforeEach,
afterEach
} from '@jest/globals';
import fs from 'fs';
import path from 'path';
import os from 'os';
import path from 'path';
import { fileURLToPath } from 'url';
import { afterEach, beforeEach, describe, expect, it } from '@jest/globals';
const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);