From c2b2d543d484418739cf3fae3183848e54fc23f4 Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Wed, 8 Oct 2025 15:19:38 +0200 Subject: [PATCH] chore: fix CI and apply requested changes --- scripts/modules/dependency-manager.js | 25 ++++--- tests/unit/dependency-manager.test.js | 97 ++++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 11 deletions(-) diff --git a/scripts/modules/dependency-manager.js b/scripts/modules/dependency-manager.js index e29c4387..e7c618a2 100644 --- a/scripts/modules/dependency-manager.js +++ b/scripts/modules/dependency-manager.js @@ -312,18 +312,23 @@ async function removeDependency(tasksPath, taskId, dependencyId, context = {}) { // Check if the dependency exists by comparing string representations const dependencyIndex = targetTask.dependencies.findIndex((dep) => { - // Convert both to strings for comparison - let depStr = String(dep); - - // Special handling for numeric IDs that might be subtask references - if (typeof dep === 'number' && dep < 100 && isSubtask) { - // It's likely a reference to another subtask in the same parent task - // Convert to full format for comparison (e.g., 2 -> "1.2" for a subtask in task 1) - const [parentId] = formattedTaskId.split('.'); - depStr = `${parentId}.${dep}`; + // Direct string comparison (handles both numeric IDs and dot notation) + const depStr = String(dep); + if (depStr === normalizedDependencyId) { + return true; } - return depStr === normalizedDependencyId; + // For subtasks: handle numeric dependencies that might be references to other subtasks + // in the same parent (e.g., subtask 1.2 depending on subtask 1.1 stored as just "1") + if (typeof dep === 'number' && dep < 100 && isSubtask) { + const [parentId] = formattedTaskId.split('.'); + const fullSubtaskRef = `${parentId}.${dep}`; + if (fullSubtaskRef === normalizedDependencyId) { + return true; + } + } + + return false; }); if (dependencyIndex === -1) { diff --git a/tests/unit/dependency-manager.test.js b/tests/unit/dependency-manager.test.js index a584ce26..deda7cf4 100644 --- a/tests/unit/dependency-manager.test.js +++ b/tests/unit/dependency-manager.test.js @@ -1061,7 +1061,12 @@ describe('Dependency Manager Module', () => { // Verify the dependency was actually added to subtask 2.2 expect(subtask22.dependencies).toContain(11); // Also verify a success log was emitted - expect(mockLog).toHaveBeenCalled(); + const successCall = mockLog.mock.calls.find( + ([level]) => level === 'success' + ); + expect(successCall).toBeDefined(); + expect(successCall[1]).toContain('2.2'); + expect(successCall[1]).toContain('11'); }); test('should allow top-level task to depend on subtask', async () => { @@ -1144,5 +1149,95 @@ describe('Dependency Manager Module', () => { // Verify that an error was reported to the user expect(mockLog).toHaveBeenCalled(); }); + + test('should remove top-level task dependency from a subtask', async () => { + const { addDependency, removeDependency } = await import( + '../../scripts/modules/dependency-manager.js' + ); + + // Start with cloned data and add 11 to 2.2 + await addDependency(TEST_TASKS_PATH, '2.2', 11, { projectRoot: '/test' }); + + // Get the saved data from the add operation + const addWriteCall = mockWriteJSON.mock.calls.find( + ([p]) => p === TEST_TASKS_PATH + ); + expect(addWriteCall).toBeDefined(); + const dataWithDep = addWriteCall[1]; + + // Verify the dependency was added + const subtask22AfterAdd = dataWithDep.tasks + .find((t) => t.id === 2) + .subtasks.find((st) => st.id === 2); + expect(subtask22AfterAdd.dependencies).toContain(11); + + // Clear mocks and re-setup mockReadJSON with the modified data + jest.clearAllMocks(); + mockReadJSON.mockImplementation(() => structuredClone(dataWithDep)); + + await removeDependency(TEST_TASKS_PATH, '2.2', 11, { + projectRoot: '/test' + }); + + const writeCall = mockWriteJSON.mock.calls.find( + ([p]) => p === TEST_TASKS_PATH + ); + expect(writeCall).toBeDefined(); + const saved = writeCall[1]; + const subtask22 = saved.tasks + .find((t) => t.id === 2) + .subtasks.find((st) => st.id === 2); + expect(subtask22.dependencies).not.toContain(11); + // Verify success log was emitted + const successCall = mockLog.mock.calls.find( + ([level]) => level === 'success' + ); + expect(successCall).toBeDefined(); + expect(successCall[1]).toContain('2.2'); + expect(successCall[1]).toContain('11'); + }); + + test('should remove subtask dependency from a top-level task', async () => { + const { addDependency, removeDependency } = await import( + '../../scripts/modules/dependency-manager.js' + ); + + // Add subtask dependency to task 11 + await addDependency(TEST_TASKS_PATH, 11, '2.1', { projectRoot: '/test' }); + + // Get the saved data from the add operation + const addWriteCall = mockWriteJSON.mock.calls.find( + ([p]) => p === TEST_TASKS_PATH + ); + expect(addWriteCall).toBeDefined(); + const dataWithDep = addWriteCall[1]; + + // Verify the dependency was added + const task11AfterAdd = dataWithDep.tasks.find((t) => t.id === 11); + expect(task11AfterAdd.dependencies).toContain('2.1'); + + // Clear mocks and re-setup mockReadJSON with the modified data + jest.clearAllMocks(); + mockReadJSON.mockImplementation(() => structuredClone(dataWithDep)); + + await removeDependency(TEST_TASKS_PATH, 11, '2.1', { + projectRoot: '/test' + }); + + const writeCall = mockWriteJSON.mock.calls.find( + ([p]) => p === TEST_TASKS_PATH + ); + expect(writeCall).toBeDefined(); + const saved = writeCall[1]; + const task11 = saved.tasks.find((t) => t.id === 11); + expect(task11.dependencies).not.toContain('2.1'); + // Verify success log was emitted + const successCall = mockLog.mock.calls.find( + ([level]) => level === 'success' + ); + expect(successCall).toBeDefined(); + expect(successCall[1]).toContain('11'); + expect(successCall[1]).toContain('2.1'); + }); }); });