fix(sync-md): prevent crash when adding subtasks to markdown file directly

Fixes #6021

When users manually added subtasks to markdown files that referenced
non-existent parent tasks, the plugin would crash due to unsafe null
assertions and missing validation. This made the plugin permanently
disabled and required creating a new markdown file to recover.

Changes:
- Added parent ID validation before creating operations
- Orphaned subtasks are now converted to root tasks with warnings
- Added comprehensive error handling with user notifications
- Plugin stays enabled even after sync errors
- Added 15 new tests (689 lines) for orphaned subtask scenarios
- Fixed all existing tests to support new error notifications

The fix implements defense-in-depth:
1. Validation layer: Check parent IDs exist before operations
2. Error handling: Catch and report errors without crashing
3. User feedback: Clear notifications about issues
4. Data preservation: No data loss, orphans become root tasks
This commit is contained in:
Johannes Millan 2026-01-16 11:58:17 +01:00
parent ac929a5dbe
commit f9fd8454cc
12 changed files with 1130 additions and 94 deletions

View file

@ -0,0 +1,77 @@
import { mdToSp } from '../../sync/md-to-sp';
describe('Error Recovery Integration', () => {
beforeEach(() => {
// Mock PluginAPI
global.PluginAPI = {
showSnack: jest.fn(),
getTasks: jest.fn().mockResolvedValue([]),
getAllProjects: jest
.fn()
.mockResolvedValue([{ id: 'test-project', title: 'Test Project' }]),
batchUpdateForProject: jest.fn().mockResolvedValue({
success: true,
createdTaskIds: {},
}),
} as any;
});
afterEach(() => {
jest.clearAllMocks();
});
it('should not crash on valid markdown with subtasks', async () => {
const markdown = `- [ ] <!--parent--> Parent Task
- [ ] <!--child--> Child Task`;
// Should not throw and should sync successfully
await expect(mdToSp(markdown, 'test-project')).resolves.not.toThrow();
// Should NOT show error notifications for valid markdown
expect(global.PluginAPI.showSnack).not.toHaveBeenCalledWith(
expect.objectContaining({ type: 'ERROR' }),
);
});
it('should show error notification on batch operation failure', async () => {
(global.PluginAPI.batchUpdateForProject as jest.Mock).mockResolvedValue({
success: false,
errors: [{ operationIndex: 0, type: 'TASK_NOT_FOUND', message: 'Task not found' }],
});
const markdown = `- [ ] Task`;
await expect(mdToSp(markdown, 'test-project')).rejects.toThrow();
expect(global.PluginAPI.showSnack).toHaveBeenCalledWith(
expect.objectContaining({
type: 'ERROR',
msg: expect.stringContaining('Batch operations failed'),
}),
);
});
it('should continue plugin operation even after sync errors', async () => {
const markdown = `- [ ] <!--invalid--> Task with issue`;
// First call fails
(global.PluginAPI.batchUpdateForProject as jest.Mock).mockRejectedValueOnce(
new Error('Network error'),
);
await expect(mdToSp(markdown, 'test-project')).rejects.toThrow();
// Verify error was communicated to user
expect(global.PluginAPI.showSnack).toHaveBeenCalledWith(
expect.objectContaining({ type: 'ERROR' }),
);
// Plugin should still be functional - second call succeeds
(global.PluginAPI.batchUpdateForProject as jest.Mock).mockResolvedValueOnce({
success: true,
createdTaskIds: {},
});
await expect(mdToSp(markdown, 'test-project')).resolves.not.toThrow();
});
});

View file

@ -191,6 +191,52 @@ describe('Error Scenarios and Boundary Conditions', () => {
expect(createOps.length).toBeGreaterThanOrEqual(1);
});
it('should convert orphaned subtasks to root tasks and warn', () => {
const mdTasks: ParsedTask[] = [
{
id: 'orphan',
title: 'Orphan Subtask',
parentId: 'non-existent-parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Orphan Subtask',
},
{
id: 'normal',
title: 'Normal Task',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 2,
originalLine: '- [ ] Normal Task',
},
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
const operations = generateTaskOperations(mdTasks, [], projectId);
// Orphan should be created as root task (no parentId in create operation)
const createOps = operations.filter((op) => op.type === 'create');
const orphanCreate = createOps.find(
(op) => op.type === 'create' && op.data.title === 'Orphan Subtask',
);
expect(orphanCreate).toBeDefined();
expect(orphanCreate?.data.parentId).toBeUndefined();
// Should have logged warning
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Orphaned subtask detected'),
);
consoleWarnSpy.mockRestore();
});
it('should handle duplicate operations gracefully', () => {
const mdTasks = [
{ id: 'dup', title: 'Task 1', completed: false },
@ -320,4 +366,78 @@ describe('Error Scenarios and Boundary Conditions', () => {
});
});
});
describe('Operation Validation', () => {
it('should validate operations and catch invalid parent references', () => {
const mdTasks: ParsedTask[] = [
{
id: 'child',
title: 'Child Task',
parentId: 'invalid-parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 0,
originalLine: ' - [ ] Child',
},
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
// Should not throw, but should log warnings
expect(() => {
generateTaskOperations(mdTasks, [], 'test-project');
}).not.toThrow();
expect(consoleWarnSpy).toHaveBeenCalled();
consoleWarnSpy.mockRestore();
});
it('should handle null parent IDs safely', () => {
const mdTasks: ParsedTask[] = [
{
id: 'task1',
title: 'Task 1',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Task 1',
},
];
expect(() => {
generateTaskOperations(mdTasks, [], 'test-project');
}).not.toThrow();
});
it('should handle subtasks with missing parent gracefully', () => {
const mdTasks: ParsedTask[] = [
{
id: 'subtask-without-parent',
title: 'Orphaned Subtask',
parentId: 'missing-parent-id',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 5,
originalLine: ' - [ ] Orphaned Subtask',
},
];
// Should create as root task, not crash
const operations = generateTaskOperations(mdTasks, [], 'test-project');
const createOp = operations.find(
(op) => op.type === 'create' && op.data.title === 'Orphaned Subtask',
);
expect(createOp).toBeDefined();
expect(createOp?.data.parentId).toBeUndefined();
});
});
});

View file

@ -25,6 +25,7 @@ const mockPluginAPI = {
},
persistDataSynced: jest.fn(),
loadSyncedData: jest.fn(),
showSnack: jest.fn(),
};
(global as any).PluginAPI = mockPluginAPI;
@ -40,7 +41,10 @@ describe('MD to SP Sync - Complete Tests', () => {
// Setup default mocks
mockPluginAPI.getTasks.mockResolvedValue([]);
mockPluginAPI.batchUpdateForProject.mockResolvedValue(undefined);
mockPluginAPI.batchUpdateForProject.mockResolvedValue({
success: true,
createdTaskIds: {},
});
mockPluginAPI.getAllProjects.mockResolvedValue([
{ id: mockProjectId, title: 'Test Project' },
]);
@ -182,18 +186,19 @@ describe('MD to SP Sync - Complete Tests', () => {
});
it('should handle project not found', async () => {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
mockPluginAPI.getAllProjects.mockResolvedValue([]);
await mdToSp(mockMarkdownContent, mockProjectId);
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Project test-project not found'),
await expect(mdToSp(mockMarkdownContent, mockProjectId)).rejects.toThrow(
'Project test-project not found',
);
expect(mockPluginAPI.batchUpdateForProject).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
expect(mockPluginAPI.batchUpdateForProject).not.toHaveBeenCalled();
expect(mockPluginAPI.showSnack).toHaveBeenCalledWith(
expect.objectContaining({
type: 'ERROR',
msg: expect.stringContaining('Project test-project not found'),
}),
);
});
it('should handle getTasks error', async () => {
@ -405,29 +410,31 @@ describe('MD to SP Sync - Complete Tests', () => {
});
it('should handle null projectId gracefully', async () => {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
await mdToSp(mockMarkdownContent, null as any);
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Project null not found'),
await expect(mdToSp(mockMarkdownContent, null as any)).rejects.toThrow(
'Project null not found',
);
expect(mockPluginAPI.batchUpdateForProject).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
expect(mockPluginAPI.batchUpdateForProject).not.toHaveBeenCalled();
expect(mockPluginAPI.showSnack).toHaveBeenCalledWith(
expect.objectContaining({
type: 'ERROR',
msg: expect.stringContaining('Project null not found'),
}),
);
});
it('should handle undefined projectId gracefully', async () => {
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
await mdToSp(mockMarkdownContent, undefined as any);
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Project undefined not found'),
await expect(mdToSp(mockMarkdownContent, undefined as any)).rejects.toThrow(
'Project undefined not found',
);
expect(mockPluginAPI.batchUpdateForProject).not.toHaveBeenCalled();
consoleWarnSpy.mockRestore();
expect(mockPluginAPI.batchUpdateForProject).not.toHaveBeenCalled();
expect(mockPluginAPI.showSnack).toHaveBeenCalledWith(
expect.objectContaining({
type: 'ERROR',
msg: expect.stringContaining('Project undefined not found'),
}),
);
});
});
});

View file

@ -0,0 +1,614 @@
/**
* Comprehensive tests for orphaned subtask fix (Issue #6021)
* Tests the validation logic that prevents crashes when markdown files
* contain subtasks referencing non-existent parents
*/
import { generateTaskOperations } from '../../sync/generate-task-operations';
import { ParsedTask } from '../../sync/markdown-parser';
import { Task } from '@super-productivity/plugin-api';
describe('Orphaned Subtask Fix - Issue #6021', () => {
const projectId = 'test-project';
describe('Parent ID Validation', () => {
it('should convert orphaned subtask to root task when parent ID does not exist', () => {
const mdTasks: ParsedTask[] = [
{
id: 'orphan-subtask',
title: 'Orphaned Subtask',
parentId: 'non-existent-parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Orphaned Subtask',
},
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
const operations = generateTaskOperations(mdTasks, [], projectId);
// Should create the task without parentId (as root task)
const createOps = operations.filter((op) => op.type === 'create');
expect(createOps).toHaveLength(1);
expect(createOps[0].data.title).toBe('Orphaned Subtask');
expect(createOps[0].data.parentId).toBeUndefined();
// Should log warning
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Orphaned subtask detected'),
);
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Orphaned Subtask'),
);
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('non-existent-parent'),
);
consoleWarnSpy.mockRestore();
});
it('should handle multiple orphaned subtasks', () => {
const mdTasks: ParsedTask[] = [
{
id: 'orphan1',
title: 'Orphan 1',
parentId: 'missing-parent-1',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 0,
originalLine: ' - [ ] Orphan 1',
},
{
id: 'orphan2',
title: 'Orphan 2',
parentId: 'missing-parent-2',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Orphan 2',
},
{
id: 'orphan3',
title: 'Orphan 3',
parentId: 'missing-parent-1', // Same missing parent as orphan1
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 2,
originalLine: ' - [ ] Orphan 3',
},
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
const operations = generateTaskOperations(mdTasks, [], projectId);
// All three should be created as root tasks
const createOps = operations.filter((op) => op.type === 'create');
expect(createOps).toHaveLength(3);
createOps.forEach((op) => {
expect(op.data.parentId).toBeUndefined();
});
// Should log warnings (at least once per orphan)
expect(consoleWarnSpy).toHaveBeenCalled();
expect(
consoleWarnSpy.mock.calls.some((call) =>
call[0]?.includes('Orphaned subtask detected'),
),
).toBe(true);
consoleWarnSpy.mockRestore();
});
it('should preserve valid parent-child relationships', () => {
const mdTasks: ParsedTask[] = [
{
id: 'parent',
title: 'Parent Task',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Parent Task',
},
{
id: 'valid-child',
title: 'Valid Child',
parentId: 'parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Valid Child',
},
];
const operations = generateTaskOperations(mdTasks, [], projectId);
const createOps = operations.filter((op) => op.type === 'create');
const childOp = createOps.find((op) => op.data.title === 'Valid Child');
// Valid child should keep its parentId
expect(childOp).toBeDefined();
expect(childOp!.data.parentId).toBe('parent');
});
it('should handle mix of valid and orphaned subtasks', () => {
const mdTasks: ParsedTask[] = [
{
id: 'parent',
title: 'Parent Task',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Parent Task',
},
{
id: 'valid-child',
title: 'Valid Child',
parentId: 'parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Valid Child',
},
{
id: 'orphan',
title: 'Orphaned Subtask',
parentId: 'deleted-parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 2,
originalLine: ' - [ ] Orphaned Subtask',
},
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
const operations = generateTaskOperations(mdTasks, [], projectId);
const createOps = operations.filter((op) => op.type === 'create');
// Valid child should have parent
const validChild = createOps.find((op) => op.data.title === 'Valid Child');
expect(validChild!.data.parentId).toBe('parent');
// Orphan should not have parent
const orphan = createOps.find((op) => op.data.title === 'Orphaned Subtask');
expect(orphan!.data.parentId).toBeUndefined();
// Should warn about the orphan
expect(consoleWarnSpy).toHaveBeenCalled();
const orphanWarnings = consoleWarnSpy.mock.calls.filter((call) =>
call[0]?.includes('Orphaned subtask detected'),
);
expect(orphanWarnings.length).toBeGreaterThanOrEqual(1);
consoleWarnSpy.mockRestore();
});
});
describe('Temp ID Handling', () => {
it('should recognize temp IDs as valid parent IDs', () => {
const mdTasks: ParsedTask[] = [
{
id: null,
title: 'Parent Without ID',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Parent Without ID',
},
{
id: null,
title: 'Child of Parent',
parentId: 'temp_0', // References temp ID
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Child of Parent',
},
];
const operations = generateTaskOperations(mdTasks, [], projectId);
const createOps = operations.filter((op) => op.type === 'create');
const childOp = createOps.find((op) => op.data.title === 'Child of Parent');
// Child should have temp_0 as parent
expect(childOp!.data.parentId).toBe('temp_0');
});
it('should handle multiple levels with temp IDs', () => {
const mdTasks: ParsedTask[] = [
{
id: null,
title: 'Parent 1',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Parent 1',
},
{
id: null,
title: 'Child 1',
parentId: 'temp_0',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Child 1',
},
{
id: null,
title: 'Parent 2',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 2,
originalLine: '- [ ] Parent 2',
},
{
id: null,
title: 'Child 2',
parentId: 'temp_2',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 3,
originalLine: ' - [ ] Child 2',
},
];
const operations = generateTaskOperations(mdTasks, [], projectId);
const createOps = operations.filter((op) => op.type === 'create');
const child1 = createOps.find((op) => op.data.title === 'Child 1');
const child2 = createOps.find((op) => op.data.title === 'Child 2');
expect(child1!.data.parentId).toBe('temp_0');
expect(child2!.data.parentId).toBe('temp_2');
});
});
describe('SP Task Parent Validation', () => {
it('should validate parent exists in SP tasks', () => {
const mdTasks: ParsedTask[] = [
{
id: 'child',
title: 'Child Task',
parentId: 'sp-parent', // References SP task
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 0,
originalLine: ' - [ ] Child Task',
},
];
const spTasks: Task[] = [
{
id: 'sp-parent',
title: 'SP Parent Task',
isDone: false,
parentId: null,
projectId,
} as Task,
];
const operations = generateTaskOperations(mdTasks, spTasks, projectId);
const createOps = operations.filter((op) => op.type === 'create');
const childOp = createOps.find((op) => op.data.title === 'Child Task');
// Should keep parent reference since it exists in SP
expect(childOp!.data.parentId).toBe('sp-parent');
});
it('should reject parent ID that exists in SP but is itself a subtask', () => {
const mdTasks: ParsedTask[] = [
{
id: 'child',
title: 'Child Task',
parentId: 'sp-subtask',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 0,
originalLine: ' - [ ] Child Task',
},
];
const spTasks: Task[] = [
{
id: 'sp-parent',
title: 'SP Parent',
isDone: false,
parentId: null,
projectId,
} as Task,
{
id: 'sp-subtask',
title: 'SP Subtask',
isDone: false,
parentId: 'sp-parent', // This is a subtask, not a root task
projectId,
} as Task,
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
const operations = generateTaskOperations(mdTasks, spTasks, projectId);
const createOps = operations.filter((op) => op.type === 'create');
const childOp = createOps.find((op) => op.data.title === 'Child Task');
// Should convert to root task since parent is not a root task
expect(childOp!.data.parentId).toBeUndefined();
expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Orphaned subtask detected'),
);
consoleWarnSpy.mockRestore();
});
});
describe('Update Operations', () => {
it('should validate parent ID when updating existing task', () => {
const mdTasks: ParsedTask[] = [
{
id: 'existing-child',
title: 'Existing Child',
parentId: 'non-existent-parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 0,
originalLine: ' - [ ] Existing Child',
},
];
const spTasks: Task[] = [
{
id: 'existing-child',
title: 'Existing Child',
isDone: false,
parentId: 'old-parent',
projectId,
} as Task,
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
const operations = generateTaskOperations(mdTasks, spTasks, projectId);
const updateOps = operations.filter((op) => op.type === 'update');
const childUpdate = updateOps.find((op) => op.taskId === 'existing-child');
// Should update parentId to null (remove invalid parent)
expect(childUpdate).toBeDefined();
expect(childUpdate!.updates.parentId).toBeNull();
expect(consoleWarnSpy).toHaveBeenCalled();
consoleWarnSpy.mockRestore();
});
});
describe('Operation Validation Warnings', () => {
it('should log validation warnings for operations with invalid parents', () => {
// This test uses the validation function that runs after operations are generated
const mdTasks: ParsedTask[] = [
{
id: 'task1',
title: 'Task 1',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Task 1',
},
];
const consoleWarnSpy = jest.spyOn(console, 'warn');
generateTaskOperations(mdTasks, [], projectId);
// Should not have any validation warnings for valid operations
const validationWarnings = consoleWarnSpy.mock.calls.filter((call) =>
call[0]?.includes('Operation warnings'),
);
expect(validationWarnings).toHaveLength(0);
consoleWarnSpy.mockRestore();
});
});
describe('Edge Cases', () => {
it('should handle null parent ID safely', () => {
const mdTasks: ParsedTask[] = [
{
id: 'task1',
title: 'Task with null parent',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Task',
},
];
expect(() => {
generateTaskOperations(mdTasks, [], projectId);
}).not.toThrow();
});
it('should handle undefined parent ID safely', () => {
const mdTasks: ParsedTask[] = [
{
id: 'task1',
title: 'Task with undefined parent',
parentId: undefined as any,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Task',
},
];
expect(() => {
generateTaskOperations(mdTasks, [], projectId);
}).not.toThrow();
});
it('should handle empty string parent ID as invalid', () => {
const mdTasks: ParsedTask[] = [
{
id: 'child',
title: 'Child with empty parent',
parentId: '',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 0,
originalLine: ' - [ ] Child',
},
];
const operations = generateTaskOperations(mdTasks, [], projectId);
const createOps = operations.filter((op) => op.type === 'create');
expect(createOps[0].data.parentId).toBeUndefined();
});
it('should not crash with large number of orphaned subtasks', () => {
const mdTasks: ParsedTask[] = Array.from({ length: 100 }, (_, i) => ({
id: `orphan-${i}`,
title: `Orphan ${i}`,
parentId: `missing-parent-${i}`,
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: i,
originalLine: ` - [ ] Orphan ${i}`,
}));
expect(() => {
const operations = generateTaskOperations(mdTasks, [], projectId);
expect(operations.length).toBeGreaterThan(0);
}).not.toThrow();
});
});
describe('Subtask Ordering with Orphans', () => {
it('should not include orphaned subtasks in parent subtask ordering', () => {
const mdTasks: ParsedTask[] = [
{
id: 'parent',
title: 'Parent',
parentId: null,
completed: false,
isSubtask: false,
depth: 0,
indent: 0,
line: 0,
originalLine: '- [ ] Parent',
},
{
id: 'valid-child',
title: 'Valid Child',
parentId: 'parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 1,
originalLine: ' - [ ] Valid Child',
},
{
id: 'orphan',
title: 'Orphan',
parentId: 'missing-parent',
completed: false,
isSubtask: true,
depth: 1,
indent: 2,
line: 2,
originalLine: ' - [ ] Orphan',
},
];
const spTasks: Task[] = [
{
id: 'parent',
title: 'Parent',
isDone: false,
parentId: null,
projectId,
subTaskIds: [],
} as Task,
];
const operations = generateTaskOperations(mdTasks, spTasks, projectId);
// Find the update operation for parent's subTaskIds
const updateOps = operations.filter((op) => op.type === 'update');
const parentUpdate = updateOps.find((op) => op.taskId === 'parent');
// Parent should have the valid child in subTaskIds
expect(parentUpdate).toBeDefined();
if (parentUpdate && parentUpdate.updates.subTaskIds) {
const subTaskIds = parentUpdate.updates.subTaskIds as string[];
// The valid child should be in the list (either by ID or temp ID)
expect(subTaskIds.length).toBe(1);
// The orphan should definitely not be included
expect(subTaskIds).not.toContain('orphan');
expect(subTaskIds).not.toContain('temp_2'); // orphan's potential temp ID
}
});
});
});

View file

@ -49,6 +49,7 @@ import { verifySyncState, logSyncVerification } from '../../sync/verify-sync';
},
persistDataSynced: jest.fn(),
loadSyncedData: jest.fn(),
showSnack: jest.fn(),
};
// Mock console to reduce noise
@ -109,6 +110,7 @@ describe('Sync Manager Debounce Behavior', () => {
.fn()
.mockResolvedValue([{ id: 'test-project', title: 'Test Project' }]),
batchUpdateForProject: jest.fn().mockResolvedValue(undefined),
showSnack: jest.fn(),
};
});

View file

@ -43,6 +43,7 @@ jest.mock('../../../shared/logger', () => ({
},
persistDataSynced: jest.fn(),
loadSyncedData: jest.fn(),
showSnack: jest.fn(),
};
describe('Sync Manager - Edge Cases', () => {
@ -75,6 +76,7 @@ describe('Sync Manager - Edge Cases', () => {
.fn()
.mockResolvedValue([{ id: 'test-project', title: 'Test Project' }]),
batchUpdateForProject: jest.fn().mockResolvedValue(undefined),
showSnack: jest.fn(),
};
});

View file

@ -175,7 +175,10 @@ export const createMockPluginAPI = (): any => {
getAllProjects: jest
.fn()
.mockResolvedValue([{ id: 'test-project', title: 'Test Project' }]),
batchUpdateForProject: jest.fn().mockResolvedValue(undefined),
batchUpdateForProject: jest.fn().mockResolvedValue({
success: true,
createdTaskIds: {},
}),
log: {
critical: jest.fn(),
err: jest.fn(),
@ -189,6 +192,7 @@ export const createMockPluginAPI = (): any => {
},
persistDataSynced: jest.fn(),
loadSyncedData: jest.fn(),
showSnack: jest.fn(),
};
(global as any).PluginAPI = mockAPI;

View file

@ -8,6 +8,77 @@ import {
} from '@super-productivity/plugin-api';
import { ParsedTask } from './markdown-parser';
interface ValidationResult {
isValid: boolean;
errors: string[];
warnings: string[];
}
/**
* Validate that operations are safe to execute
* Catches invalid parent references and missing task IDs
*/
const validateOperations = (
operations: BatchOperation[],
mdTasks: ParsedTask[],
spTasks: Task[],
): ValidationResult => {
const errors: string[] = [];
const warnings: string[] = [];
const spTaskIds = new Set(spTasks.map((t) => t.id));
const tempIds = new Set<string>();
operations.forEach((op, index) => {
switch (op.type) {
case 'create':
tempIds.add(op.tempId);
if (op.data.parentId) {
const parentExists =
spTaskIds.has(op.data.parentId) || tempIds.has(op.data.parentId);
if (!parentExists) {
warnings.push(
`Operation ${index}: Create task "${op.data.title}" references ` +
`non-existent parent "${op.data.parentId}"`,
);
}
}
break;
case 'update':
if (!spTaskIds.has(op.taskId)) {
errors.push(
`Operation ${index}: Update references non-existent task "${op.taskId}"`,
);
}
if (op.updates.parentId !== undefined && op.updates.parentId !== null) {
const parentExists =
spTaskIds.has(op.updates.parentId) || tempIds.has(op.updates.parentId);
if (!parentExists) {
warnings.push(
`Operation ${index}: Update task "${op.taskId}" to non-existent parent "${op.updates.parentId}"`,
);
}
}
break;
case 'delete':
if (!spTaskIds.has(op.taskId)) {
warnings.push(
`Operation ${index}: Delete references non-existent task "${op.taskId}"`,
);
}
break;
}
});
return {
isValid: errors.length === 0,
errors,
warnings,
};
};
/**
* Generate batch operations to sync markdown tasks to Super Productivity
*/
@ -65,6 +136,41 @@ export const generateTaskOperations = (
newParentId: string | null;
}> = [];
// Build set of valid parent IDs BEFORE processing tasks
const validParentIds = new Set<string>();
// Add all root-level MD tasks as valid parents (including temp IDs)
mdTasks
.filter((t) => !t.isSubtask)
.forEach((t) => {
if (t.id && !duplicateIds.has(t.id)) {
validParentIds.add(t.id);
} else {
// Tasks without IDs will get temp IDs - add them too
validParentIds.add(`temp_${t.line}`);
}
});
// Add all root-level SP tasks as valid parents
spTasks.forEach((t) => {
if (!t.parentId && t.id) validParentIds.add(t.id);
});
// Helper function to get validated parent ID
const getValidatedParentId = (mdTask: ParsedTask): string | null => {
if (!mdTask.parentId) return null;
if (!validParentIds.has(mdTask.parentId)) {
console.warn(
`[sync-md] Orphaned subtask detected: "${mdTask.title}" (line ${mdTask.line}) ` +
`references non-existent parent "${mdTask.parentId}". Converting to root task.`,
);
return null;
}
return mdTask.parentId;
};
// First pass: process MD tasks
mdTasks.forEach((mdTask) => {
// Check if this task has a duplicate ID
@ -93,14 +199,14 @@ export const generateTaskOperations = (
// Handle parent relationship
// Normalize undefined and null to null for comparison
const spParentId = spTask.parentId || null;
const mdParentId = mdTask.parentId || null;
const mdParentId = getValidatedParentId(mdTask);
if (spParentId !== mdParentId) {
updates.parentId = mdTask.parentId;
updates.parentId = mdParentId;
// Track this parent change for cleanup
tasksWithChangedParents.push({
taskId: spTask.id!,
oldParentId: spTask.parentId || null,
newParentId: mdTask.parentId || null,
newParentId: mdParentId,
});
}
@ -119,9 +225,10 @@ export const generateTaskOperations = (
notes: mdTask.notes,
};
// Only include parentId if it's not null
if (mdTask.parentId) {
createData.parentId = mdTask.parentId;
// Only include parentId if it's valid (not null and parent exists)
const validatedParentId = getValidatedParentId(mdTask);
if (validatedParentId) {
createData.parentId = validatedParentId;
}
operations.push({
@ -151,14 +258,14 @@ export const generateTaskOperations = (
// Handle parent relationship
// Normalize undefined and null to null for comparison
const spParentId = spTask.parentId || null;
const mdParentId = mdTask.parentId || null;
const mdParentId = getValidatedParentId(mdTask);
if (spParentId !== mdParentId) {
updates.parentId = mdTask.parentId;
updates.parentId = mdParentId;
// Track this parent change for cleanup
tasksWithChangedParents.push({
taskId: spTask.id!,
oldParentId: spTask.parentId || null,
newParentId: mdTask.parentId || null,
newParentId: mdParentId,
});
}
@ -177,9 +284,10 @@ export const generateTaskOperations = (
notes: mdTask.notes,
};
// Only include parentId if it's not null
if (mdTask.parentId) {
createData.parentId = mdTask.parentId;
// Only include parentId if it's valid (not null and parent exists)
const validatedParentId = getValidatedParentId(mdTask);
if (validatedParentId) {
createData.parentId = validatedParentId;
}
operations.push({
@ -309,14 +417,22 @@ export const generateTaskOperations = (
// Fourth pass: handle subtask ordering
// Group subtasks by parent, maintaining their order from the markdown file
// Note: validParentIds and getValidatedParentId() already filter out invalid parents
const subtasksByParent = new Map<string, ParsedTask[]>();
mdTasks
.filter((t) => t.isSubtask && t.parentId)
.forEach((subtask) => {
if (!subtasksByParent.has(subtask.parentId!)) {
subtasksByParent.set(subtask.parentId!, []);
const validatedParentId = getValidatedParentId(subtask);
// Skip if parent is invalid (warning already logged by getValidatedParentId)
if (!validatedParentId) {
return;
}
subtasksByParent.get(subtask.parentId!)!.push(subtask);
if (!subtasksByParent.has(validatedParentId)) {
subtasksByParent.set(validatedParentId, []);
}
subtasksByParent.get(validatedParentId)!.push(subtask);
});
// Sort subtasks by line number to maintain order from file
@ -378,5 +494,16 @@ export const generateTaskOperations = (
const reorderOps = operations.filter((op) => op.type === 'reorder');
const otherOps = operations.filter((op) => op.type !== 'reorder');
// Validate operations before returning
const validation = validateOperations(operations, mdTasks, spTasks);
if (!validation.isValid) {
console.error('[sync-md] Invalid operations generated:', validation.errors);
validation.errors.forEach((err) => console.error(` - ${err}`));
}
if (validation.warnings.length > 0) {
console.warn('[sync-md] Operation warnings:', validation.warnings);
validation.warnings.forEach((warn) => console.warn(` - ${warn}`));
}
return [...otherOps, ...reorderOps];
};

View file

@ -52,13 +52,17 @@ describe('Header Preservation Integration', () => {
}
}
});
return Promise.resolve();
return Promise.resolve({
success: true,
createdTaskIds: {},
});
}),
persistDataSynced: jest.fn().mockImplementation((data) => {
Object.assign(mockSyncedData, data);
return Promise.resolve();
}),
loadSyncedData: jest.fn().mockResolvedValue(mockSyncedData),
showSnack: jest.fn(),
};
});

View file

@ -10,36 +10,70 @@ export const mdToSp = async (
markdownContent: string,
projectId: string,
): Promise<void> => {
// Use parseMarkdownWithHeader to get header, but handle backward compatibility
const parseResult = parseMarkdownWithHeader(markdownContent);
const parsedTasks = parseResult.tasks;
try {
// Parse markdown with error tracking
const parseResult = parseMarkdownWithHeader(markdownContent);
const parsedTasks = parseResult.tasks;
// Get current state
const currentTasks = await PluginAPI.getTasks();
const currentProjects = await PluginAPI.getAllProjects();
// Report parse errors to user if any
if (parseResult.errors && parseResult.errors.length > 0) {
const errorMsg = `Found ${parseResult.errors.length} issue(s) in markdown:\n${parseResult.errors.slice(0, 3).join('\n')}`;
console.warn('[sync-md]', errorMsg);
if (!currentProjects.find((p) => p.id === projectId)) {
console.warn(`[sync-md] Project ${projectId} not found, skipping sync`);
return;
}
PluginAPI.showSnack({
msg: `Sync.md: ${parseResult.errors.length} parsing issue(s). Check console for details.`,
type: 'WARNING',
});
}
// Filter tasks for the specific project
const projectTasks = currentTasks.filter((task) => task.projectId === projectId);
// Get current state
const currentTasks = await PluginAPI.getTasks();
const currentProjects = await PluginAPI.getAllProjects();
// Generate operations using the new sync logic
const operations = generateTaskOperations(parsedTasks, projectTasks, projectId);
if (!currentProjects.find((p) => p.id === projectId)) {
throw new Error(`Project ${projectId} not found`);
}
// Execute batch operations
if (operations.length > 0) {
console.log(
`[sync-md] Executing ${operations.length} sync operations for project ${projectId}`,
operations,
);
// Filter tasks for the specific project
const projectTasks = currentTasks.filter((task) => task.projectId === projectId);
// Use the operations directly - they already match the expected BatchOperation format
await PluginAPI.batchUpdateForProject({
projectId,
operations,
// Generate operations with validation
const operations = generateTaskOperations(parsedTasks, projectTasks, projectId);
// Execute batch operations
if (operations.length > 0) {
console.log(
`[sync-md] Executing ${operations.length} sync operations for project ${projectId}`,
operations,
);
const result = await PluginAPI.batchUpdateForProject({
projectId,
operations,
});
// Handle batch operation errors
if (!result.success && result.errors && result.errors.length > 0) {
const errorSummary = result.errors
.map((e) => `Op ${e.operationIndex}: ${e.message}`)
.slice(0, 3)
.join('; ');
throw new Error(`Batch operations failed: ${errorSummary}`);
}
console.log('[sync-md] Sync operations completed successfully');
}
} catch (error) {
console.error('[sync-md] Error in mdToSp:', error);
// Show user-friendly error notification
PluginAPI.showSnack({
msg: `Sync.md: Failed to sync markdown to SP. ${error instanceof Error ? error.message : 'Unknown error'}`,
type: 'ERROR',
});
// Re-throw to allow caller to handle if needed
throw error;
}
};

View file

@ -44,22 +44,37 @@ const performInitialSync = async (config: LocalUserCfg): Promise<void> => {
// Determine sync direction
if (!fileStats) {
// No file exists, create from SP
await spToMd(config);
} else {
// File is newer, sync to SP
const content = await readTasksFile(config.filePath);
if (content) {
const projectId = config.projectId;
await mdToSp(content, projectId);
try {
await mdToSp(content, projectId);
} catch (syncError) {
console.error('[sync-md] Initial sync failed:', syncError);
PluginAPI.showSnack({
msg: 'Sync.md: Initial sync failed. Plugin is still active.',
type: 'WARNING',
});
// Don't throw - allow plugin to continue running
}
}
}
// TODO there is no proper way to check for a newer state from SP
// TODO we should persist the last modified timestamp from the file when syncing and check if it changed. if not we always update from SP to MD
// Verify sync state after initial sync
const verificationResult = await verifySyncState(config);
logSyncVerification(verificationResult, 'initial sync');
} catch (outerError) {
console.error('[sync-md] Fatal error in initial sync:', outerError);
PluginAPI.showSnack({
msg: 'Sync.md: Plugin started but sync failed. Check console.',
type: 'ERROR',
});
} finally {
syncInProgress = false;
}
@ -106,38 +121,63 @@ const handleMdToSpSync = async (config: LocalUserCfg): Promise<void> => {
try {
const content = await readTasksFile(config.filePath);
if (content) {
// Use the project ID from config, fallback to default
const projectId = config.projectId;
console.log(`[sync-md] Executing mdToSp for project: ${projectId}`);
await mdToSp(content, projectId);
// Verify sync state after file change sync
const verificationResult = await verifySyncState(config);
logSyncVerification(verificationResult, 'MD to SP sync (file change)');
try {
await mdToSp(content, projectId);
// If there are still differences after MD→SP sync, trigger SP→MD sync to resolve them
if (!verificationResult.isInSync) {
console.log(
'[sync-md] MD to SP sync incomplete, triggering SP to MD sync to resolve remaining differences',
);
// Show success notification (optional, can be removed if too noisy)
PluginAPI.showSnack({
msg: 'Sync.md: Synced successfully',
type: 'SUCCESS',
});
// Temporarily disable file watcher to prevent triggering another MD→SP sync
stopFileWatcher();
// Verify sync state after file change sync
const verificationResult = await verifySyncState(config);
logSyncVerification(verificationResult, 'MD to SP sync (file change)');
try {
await spToMd(config);
// If there are still differences, trigger SP→MD sync
if (!verificationResult.isInSync) {
console.log(
'[sync-md] MD to SP sync incomplete, triggering SP to MD sync to resolve remaining differences',
);
// Verify again after SP→MD sync
const finalVerification = await verifySyncState(config);
logSyncVerification(finalVerification, 'SP to MD sync (resolving differences)');
} finally {
// Re-enable file watcher
startFileWatcher(config.filePath, () => {
handleFileChange(config);
});
stopFileWatcher();
try {
await spToMd(config);
const finalVerification = await verifySyncState(config);
logSyncVerification(
finalVerification,
'SP to MD sync (resolving differences)',
);
} finally {
startFileWatcher(config.filePath, () => {
handleFileChange(config);
});
}
}
} catch (syncError) {
console.error('[sync-md] Sync failed:', syncError);
// Error already shown by mdToSp, but add guidance
PluginAPI.showSnack({
msg: 'Sync.md: Check markdown file for orphaned subtasks or formatting issues',
type: 'WARNING',
});
// Don't re-throw - keep plugin running
}
}
} catch (fileError) {
console.error('[sync-md] Failed to read markdown file:', fileError);
PluginAPI.showSnack({
msg: `Sync.md: Cannot read file ${config.filePath}. Check permissions.`,
type: 'ERROR',
});
} finally {
syncInProgress = false;
}

View file

@ -29,7 +29,12 @@ const bridgeConfig: UiBridgeConfig = {
console.log('[sync-md] Sync manager initialized successfully');
} catch (error) {
console.error('[sync-md] Failed to initialize sync manager:', error);
throw error;
// Show error but DON'T throw - keep plugin running
PluginAPI.showSnack({
msg: 'Sync.md: Failed to initialize. Check file path and permissions.',
type: 'ERROR',
});
}
}
},