Skip to content

Commit 0352942

Browse files
bugerclaude
andauthored
fix: preserve user-provided task IDs instead of replacing with auto-generated ones (#542)
When creating tasks in batch with user-provided IDs (e.g., id: "tui-static-gen"), the ID was only used for dependency resolution then discarded. Tasks were stored under auto-generated IDs (task-1, task-2). When the AI later tried to complete by the original ID, it got "Task not found." Now user-provided IDs are preserved as the actual stored task IDs: - createTask() accepts optional id field, uses it when provided - createTasks() no longer maps user IDs to auto-generated ones - Dependencies reference user IDs directly - Collision detection for duplicate IDs in both single and batch creation Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5a3b932 commit 0352942

File tree

2 files changed

+69
-34
lines changed

2 files changed

+69
-34
lines changed

npm/src/agent/tasks/TaskManager.js

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ export class TaskManager {
6363
* @throws {Error} If dependencies are invalid or create a cycle
6464
*/
6565
createTask(taskData) {
66-
const id = this._generateId();
66+
const id = taskData.id || this._generateId();
67+
// If a user-provided ID collides with an existing task, throw
68+
if (taskData.id && this.tasks.has(taskData.id)) {
69+
throw new Error(`Task ID "${taskData.id}" already exists. Choose a different ID. Available tasks: ${this._getAvailableTaskIds()}`);
70+
}
6771
const now = this._now();
6872

6973
// Validate dependencies exist
@@ -148,46 +152,42 @@ export class TaskManager {
148152
}
149153
}
150154

151-
// Build a mapping of user-provided IDs to future auto-generated IDs
152-
const idMap = new Map();
153-
const batchAutoIds = new Set();
154-
let nextCounter = this.taskCounter;
155+
// Collect all IDs that will exist after this batch (for dependency validation)
156+
const batchIds = new Set();
155157
for (const taskData of tasksData) {
156-
nextCounter++;
157-
const autoId = `task-${nextCounter}`;
158-
batchAutoIds.add(autoId);
159158
if (taskData.id) {
160-
idMap.set(taskData.id, autoId);
159+
if (this.tasks.has(taskData.id)) {
160+
throw new Error(`Task ID "${taskData.id}" already exists. Choose a different ID. Available tasks: ${this._getAvailableTaskIds()}`);
161+
}
162+
if (batchIds.has(taskData.id)) {
163+
throw new Error(`Duplicate task ID "${taskData.id}" in batch. Each task must have a unique ID.`);
164+
}
165+
batchIds.add(taskData.id);
161166
}
162167
}
163168

164-
// Resolve dependencies and "after" references, then validate before creating anything
169+
// Validate dependencies and "after" references before creating anything
165170
const resolvedTasksData = tasksData.map(taskData => {
166171
const resolved = { ...taskData };
167-
delete resolved.id; // Don't pass user ID to createTask
168172

169173
if (resolved.dependencies) {
170174
resolved.dependencies = resolved.dependencies.map(depId => {
171-
if (idMap.has(depId)) return idMap.get(depId);
172-
// Check if it's already an existing task ID or a batch auto-generated ID
173-
if (this.tasks.has(depId) || batchAutoIds.has(depId)) return depId;
174-
const batchIds = idMap.size > 0 ? Array.from(idMap.keys()).join(', ') : '(none provided)';
175-
throw new Error(`Dependency "${depId}" does not exist. Each task in the batch must have an "id" field, and dependencies must reference those IDs. Current batch IDs: ${batchIds}. Existing tasks: ${this._getAvailableTaskIds()}`);
175+
if (batchIds.has(depId) || this.tasks.has(depId)) return depId;
176+
const knownIds = batchIds.size > 0 ? Array.from(batchIds).join(', ') : '(none provided)';
177+
throw new Error(`Dependency "${depId}" does not exist. Each task in the batch must have an "id" field, and dependencies must reference those IDs. Current batch IDs: ${knownIds}. Existing tasks: ${this._getAvailableTaskIds()}`);
176178
});
177179
}
178180

179181
if (resolved.after) {
180-
if (idMap.has(resolved.after)) {
181-
resolved.after = idMap.get(resolved.after);
182-
} else if (!this.tasks.has(resolved.after) && !batchAutoIds.has(resolved.after)) {
183-
throw new Error(`Task "${resolved.after}" does not exist. Cannot insert after non-existent task. Available tasks: ${this._getAvailableTaskIds()}${idMap.size > 0 ? `, batch IDs: ${Array.from(idMap.keys()).join(', ')}` : ''}`);
182+
if (!batchIds.has(resolved.after) && !this.tasks.has(resolved.after)) {
183+
throw new Error(`Task "${resolved.after}" does not exist. Cannot insert after non-existent task. Available tasks: ${this._getAvailableTaskIds()}${batchIds.size > 0 ? `, batch IDs: ${Array.from(batchIds).join(', ')}` : ''}`);
184184
}
185185
}
186186

187187
return resolved;
188188
});
189189

190-
// All validation passed — create tasks
190+
// All validation passed — create tasks (user-provided IDs are preserved by createTask)
191191
const createdTasks = [];
192192
for (const taskData of resolvedTasksData) {
193193
const task = this.createTask(taskData);

npm/tests/unit/task-manager.test.js

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,27 +88,43 @@ describe('TaskManager', () => {
8888
{ id: 'third', title: 'Task 3', dependencies: ['first', 'second'] }
8989
]);
9090

91-
expect(tasks[1].dependencies).toEqual(['task-1']);
92-
expect(tasks[2].dependencies).toEqual(['task-1', 'task-2']);
91+
expect(tasks[1].dependencies).toEqual(['first']);
92+
expect(tasks[2].dependencies).toEqual(['first', 'second']);
9393
});
9494

95-
test('should resolve user-provided IDs to auto-generated IDs in batch dependencies', () => {
95+
test('should preserve user-provided IDs in batch creation', () => {
9696
const tasks = manager.createTasks([
9797
{ id: 'auth', title: 'Authenticate with API' },
9898
{ id: 'list-projects', title: 'List Projects', dependencies: ['auth'] },
9999
{ id: 'list-clusters', title: 'List Clusters', dependencies: ['list-projects'] }
100100
]);
101101

102102
expect(tasks).toHaveLength(3);
103-
// Dependencies should be remapped to auto-generated IDs
104-
expect(tasks[0].id).toBe('task-1');
105-
expect(tasks[1].id).toBe('task-2');
106-
expect(tasks[1].dependencies).toEqual(['task-1']);
107-
expect(tasks[2].id).toBe('task-3');
108-
expect(tasks[2].dependencies).toEqual(['task-2']);
103+
// User-provided IDs are preserved as-is
104+
expect(tasks[0].id).toBe('auth');
105+
expect(tasks[1].id).toBe('list-projects');
106+
expect(tasks[1].dependencies).toEqual(['auth']);
107+
expect(tasks[2].id).toBe('list-clusters');
108+
expect(tasks[2].dependencies).toEqual(['list-projects']);
109+
});
110+
111+
test('should allow completing tasks by user-provided ID', () => {
112+
manager.createTasks([
113+
{ id: 'tui-static-gen', title: 'Generate TUI statics' },
114+
{ id: 'tui-render', title: 'Render TUI', dependencies: ['tui-static-gen'] }
115+
]);
116+
117+
// Should be able to complete using the user-provided ID
118+
const completed = manager.completeTask('tui-static-gen');
119+
expect(completed.id).toBe('tui-static-gen');
120+
expect(completed.status).toBe('completed');
121+
122+
// Dependent task should now be unblocked
123+
const render = manager.getTask('tui-render');
124+
expect(render).toBeTruthy();
109125
});
110126

111-
test('should resolve user-provided IDs with multiple dependencies', () => {
127+
test('should preserve user-provided IDs with multiple dependencies', () => {
112128
const tasks = manager.createTasks([
113129
{ id: 'setup', title: 'Setup' },
114130
{ id: 'build', title: 'Build', dependencies: ['setup'] },
@@ -117,10 +133,11 @@ describe('TaskManager', () => {
117133
]);
118134

119135
expect(tasks).toHaveLength(4);
120-
expect(tasks[3].dependencies).toEqual(['task-2', 'task-3']);
136+
expect(tasks[3].id).toBe('deploy');
137+
expect(tasks[3].dependencies).toEqual(['build', 'test']);
121138
});
122139

123-
test('should resolve user-provided IDs in "after" parameter', () => {
140+
test('should preserve user-provided IDs in "after" parameter', () => {
124141
const tasks = manager.createTasks([
125142
{ id: 'first', title: 'First task' },
126143
{ id: 'third', title: 'Third task' },
@@ -130,7 +147,25 @@ describe('TaskManager', () => {
130147
expect(tasks).toHaveLength(3);
131148
const allTasks = manager.listTasks();
132149
const taskIds = allTasks.map(t => t.id);
133-
expect(taskIds).toEqual(['task-1', 'task-3', 'task-2']);
150+
expect(taskIds).toEqual(['first', 'second', 'third']);
151+
});
152+
153+
test('should reject duplicate IDs in batch', () => {
154+
expect(() => {
155+
manager.createTasks([
156+
{ id: 'dup', title: 'First' },
157+
{ id: 'dup', title: 'Second' }
158+
]);
159+
}).toThrow(/Duplicate task ID "dup"/);
160+
});
161+
162+
test('should reject IDs that collide with existing tasks', () => {
163+
manager.createTask({ title: 'Existing' }); // creates task-1
164+
expect(() => {
165+
manager.createTasks([
166+
{ id: 'task-1', title: 'Collision' }
167+
]);
168+
}).toThrow(/already exists/);
134169
});
135170

136171
test('should not create any tasks if batch validation fails', () => {

0 commit comments

Comments
 (0)