fix: ensure consistent task ID serialization as numbers (#1583)#1618
Open
SiriusA7 wants to merge 4 commits intoeyaltoledano:nextfrom
Open
fix: ensure consistent task ID serialization as numbers (#1583)#1618SiriusA7 wants to merge 4 commits intoeyaltoledano:nextfrom
SiriusA7 wants to merge 4 commits intoeyaltoledano:nextfrom
Conversation
…#1583) **Summary** Task IDs were inconsistently saved as strings or numbers in tasks.json depending on which command was used (e.g., set-status vs update-task), causing unnecessary git diffs and file instability. **Root Cause** The codebase has two storage layers writing to tasks.json: - Legacy CLI layer (scripts/modules/utils.js): converted IDs to numbers - New tm-core layer (packages/tm-core): converted IDs to strings The tm-core file storage adapter was incorrectly using string normalization intended for API storage (which supports prefixed IDs like "HAM-123"), even though file storage only needs simple numeric IDs. **Solution** Updated both storage layers to consistently use numbers for file storage: - Task IDs: saved as numbers (1, 2, 3) - Subtask IDs: saved as numbers (1, 2, 3) - Parent IDs: saved as numbers - Dependencies: subtask references (e.g., "1.1") kept as strings, task references converted to numbers **Implementation** - Updated file-storage.ts and format-handler.ts normalization logic - Enhanced scripts/modules/utils.js with comprehensive dependency and parentId handling - Updated TypeScript types to support number | string for IDs throughout the system - Fixed type compatibility in tasks-domain, executor services, and CLI commands - Added changeset documenting the user-facing fix
🦋 Changeset detectedLatest commit: 983b850 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📝 WalkthroughWalkthroughCentralizes task ID normalization to consistently store numeric IDs (preserving dotted subtask strings), expands ID types to Changes
Sequence Diagram(s)sequenceDiagram
participant User as rgba(52,128,235,0.5)
participant CLI as rgba(88,166,255,0.5)
participant ScriptsUtils as rgba(76,201,129,0.5)
participant TMCore as rgba(255,193,7,0.5)
participant FileStorage as rgba(255,87,34,0.5)
participant Disk as rgba(158,158,158,0.5)
participant Executor as rgba(156,39,176,0.5)
User->>CLI: run command (set/update/start)
CLI->>ScriptsUtils: pass raw task(s) / taskId
ScriptsUtils->>TMCore: normalized payload (ids numeric where applicable, dotted subtask refs preserved)
TMCore->>FileStorage: saveTasks / convertToSaveFormat
FileStorage->>Disk: write tasks.json (IDs serialized consistently)
TMCore->>Executor: start/execute task (uses taskIdStr for logs)
Executor->>TMCore: return ExecutionResult (taskIdStr)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tm-core/src/common/types/index.ts (1)
296-328:⚠️ Potential issue | 🟠 MajorType guards have mismatched constraints with their interface definitions.
The
Taskinterface (line 132) definesid: number | string, butisTask()on line 301 requirestypeof task.id === 'string', rejecting valid numeric IDs. Similarly, theSubtaskinterface (lines 173–174) allowsid: number | stringandparentId: number | string, butisSubtask()checks onlytypeof subtask.id === 'number'andtypeof subtask.parentId === 'string', rejecting valid string IDs and numeric parent IDs respectively.Update the guards to accept both types:
🛠️ Proposed fix
return ( - typeof task.id === 'string' && + (typeof task.id === 'string' || typeof task.id === 'number') && typeof task.title === 'string' && typeof task.description === 'string' && isTaskStatus(task.status) && isTaskPriority(task.priority) && Array.isArray(task.dependencies) && typeof task.details === 'string' && typeof task.testStrategy === 'string' && Array.isArray(task.subtasks) ); } @@ return ( - typeof subtask.id === 'number' && - typeof subtask.parentId === 'string' && + (typeof subtask.id === 'string' || typeof subtask.id === 'number') && + (typeof subtask.parentId === 'string' || typeof subtask.parentId === 'number') && typeof subtask.title === 'string' && typeof subtask.description === 'string' && isTaskStatus(subtask.status) && isTaskPriority(subtask.priority) && !('subtasks' in subtask) ); }
🤖 Fix all issues with AI agents
In `@packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts`:
- Around line 285-310: The normalizeTaskIds implementation in file-storage.ts
(and the similar normalizeTasks in format-handler.ts) uses bare Number(...)
coercions which can produce NaN and corrupt IDs; extract a shared utility (e.g.,
create a new function parseValidId(value) in a central utils module) that uses
parseInt(value, 10) with validation (!isNaN(parsed) && parsed > 0) and returns
either the parsed number or the original value, then replace all Number(...)
uses in normalizeTaskIds and normalizeTasks (including subtask id, parentId and
dependency parsing) to call parseValidId and preserve string dependency refs
when appropriate, removing the duplicated logic and centralizing validation.
🧹 Nitpick comments (1)
packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts (1)
219-245: Consider centralizing task-ID normalization to avoid drift.
This logic mirrorsFileStorage.normalizeTaskIds; extracting a shared helper in tm-core would reduce duplication and keep behavior consistent across adapters.
As per coding guidelines: Do not duplicate task ID formatting logic across modules - centralize formatting utilities.
packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts
Outdated
Show resolved
Hide resolved
Summary: Fixed four critical issues with task ID handling in tm-core: type guard mismatches, strict equality comparison failures, duplicated normalization logic, and missing NaN validation. Root Cause: 1. Type guards (isTask/isSubtask) checked for specific ID types (string/number) but interfaces allowed number | string, causing valid normalized tasks to be rejected 2. checkInProgressConflicts compared numeric task.id with string targetTaskId using strict equality (===), which always failed (e.g., 1 === "1") 3. Normalization logic was duplicated in file-storage.ts and format-handler.ts without validation 4. Number() coercions could produce NaN, corrupting task IDs without detection Solution: - Updated type guards to accept both number and string ID types using new isValidTaskIdType() helper - Fixed all ID comparisons in checkInProgressConflicts to use String() coercion - Created shared task-id-normalizer.ts utility with proper parseInt and NaN validation - Refactored both file-storage and format-handler to use the shared utility Implementation: - packages/tm-core/src/common/types/index.ts: Added isValidTaskIdType(), updated isTask() and isSubtask() - packages/tm-core/src/modules/tasks/services/task-execution-service.ts: String coercion for ID comparisons - packages/tm-core/src/common/utils/task-id-normalizer.ts: New shared utility with validation (normalizeTaskId, normalizeSubtaskId, normalizeDependency, normalizeTaskIds) - packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts: Removed duplicate normalizeTaskIds method - packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts: Removed duplicate normalizeTasks method - Added comprehensive test coverage: 54 tests (24 for normalizer, 30 for type guards)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tm-core/src/modules/tasks/services/task-execution-service.ts (1)
149-189:⚠️ Potential issue | 🟠 MajorNormalize
targetTaskIdonce to avoid false conflicts.
Right now onlytask.idis coerced, so numerictargetTaskIdvalues won’t match and can be incorrectly treated as conflicts.🛠️ Proposed fix
- // If the target task is already in-progress, that's fine - // Use String() coercion to handle both numeric and string IDs - const targetTaskInProgress = inProgressTasks.find( - (task) => String(task.id) === targetTaskId - ); + // If the target task is already in-progress, that's fine + // Use String() coercion to handle both numeric and string IDs + const targetTaskIdStr = String(targetTaskId); + const targetTaskInProgress = inProgressTasks.find( + (task) => String(task.id) === targetTaskIdStr + ); if (targetTaskInProgress) { return { canProceed: true, conflictingTasks: [] }; } // Check if target is a subtask and its parent is in-progress - const isSubtask = targetTaskId.includes('.'); + const isSubtask = targetTaskIdStr.includes('.'); if (isSubtask) { - const parentTaskId = targetTaskId.split('.')[0]; + const parentTaskId = targetTaskIdStr.split('.')[0]; const parentInProgress = inProgressTasks.find( (task) => String(task.id) === parentTaskId ); if (parentInProgress) { return { canProceed: true, conflictingTasks: [] }; // Allow subtasks when parent is in-progress @@ - const conflictingTasks = inProgressTasks.filter((task) => { + const conflictingTasks = inProgressTasks.filter((task) => { const taskIdStr = String(task.id); - if (taskIdStr === targetTaskId) return false; + if (taskIdStr === targetTaskIdStr) return false; // If target is a subtask, exclude its parent from conflicts if (isSubtask) { - const parentTaskId = targetTaskId.split('.')[0]; + const parentTaskId = targetTaskIdStr.split('.')[0]; if (taskIdStr === parentTaskId) return false; } // If the in-progress task is a subtask of our target parent, exclude it if (taskIdStr.includes('.')) { const taskParentId = taskIdStr.split('.')[0]; - if (isSubtask && taskParentId === targetTaskId.split('.')[0]) { + if (isSubtask && taskParentId === targetTaskIdStr.split('.')[0]) { return false; } } return true; });
🤖 Fix all issues with AI agents
In `@packages/tm-core/src/common/utils/task-id-normalizer.ts`:
- Around line 167-177: The function normalizeTask currently coerces
invalid/non-numeric task.id values to 0; change the fallback so we preserve the
original ID instead of forcing 0: replace the assignment of taskIdNum in
normalizeTask to use normalizedId ?? task.id (or undefined if task.id is absent)
and ensure calls to normalizeSubtask(subtask, taskIdNum) and any other places
(similar logic around lines 146-157) accept and propagate the original ID type
rather than a forced 0; update any type expectations or validations around
normalizeTaskId/normalizeSubtask to explicitly handle string IDs or missing IDs
instead of silently converting to 0.
🧹 Nitpick comments (2)
packages/tm-core/src/common/utils/index.ts (1)
60-69: Consider trimming exports to only what’s consumed. If these helpers aren’t imported by other packages, keepingindex.tsminimal avoids expanding the public surface unnecessarily.Based on learnings: For the tm-core package in the eyaltoledano/claude-task-master repository, the team prefers a minimal, need-based export strategy in index files rather than exposing all internal utilities.
packages/tm-core/src/common/types/index.ts (1)
305-318: Consider validating dependency element types in the guard.
Right now any array passes; checking each entry avoids accepting invalid dependency values.♻️ Suggested refinement
return ( isValidTaskIdType(task.id) && typeof task.title === 'string' && typeof task.description === 'string' && isTaskStatus(task.status) && isTaskPriority(task.priority) && Array.isArray(task.dependencies) && + task.dependencies.every(isValidTaskIdType) && typeof task.details === 'string' && typeof task.testStrategy === 'string' && Array.isArray(task.subtasks) );
…ormalizers
**Summary**
Fixed three critical issues in task ID normalization that could corrupt task data:
1. Non-numeric string IDs (e.g., "HAM-123", "abc") were silently coerced to 0
2. Negative numeric IDs bypassed validation in both normalizeTask() and normalizeSubtask()
3. Duplicated dependency normalization logic in scripts/modules/utils.js
**Root Cause**
The fallback logic in normalizeTask() and normalizeSubtask() had a fundamental flaw:
- `normalizedId ?? (typeof task.id === 'number' ? task.id : 0)` would accept ANY numeric value including negatives
- Non-numeric strings would fall through to 0, corrupting API IDs like "HAM-123"
- The typeof check didn't validate that numbers were positive, finite integers
**Solution**
1. Added `isValidPositiveInteger()` helper that strictly validates:
- Must be a number type
- Must be finite (rejects ±Infinity and NaN)
- Must be an integer (rejects decimals)
- Must be positive (rejects zero and negatives)
2. Updated normalizeTask() and normalizeSubtask() with explicit validation cascade:
- Use normalized ID if successful (positive integer)
- Preserve non-empty string IDs (API IDs like "HAM-123")
- Only accept valid positive integers (reject negative/zero/Infinity)
- Fall back to '0' only for truly undefined/null cases
3. Extracted normalizeDependencyArray() helper in utils.js to DRY up duplicate code
**Implementation**
- packages/tm-core/src/common/utils/task-id-normalizer.ts
- Added isValidPositiveInteger() validation helper
- Refactored normalizeTask() with proper validation cascade
- Refactored normalizeSubtask() with proper validation cascade
- Changed parentTaskId parameter to accept number | string for API ID support
- IDs now stored as strings for consistency with file-storage patterns
- packages/tm-core/src/common/utils/task-id-normalizer.spec.ts
- Added 12 new test cases covering:
- Negative ID rejection for tasks and subtasks
- Infinity rejection for tasks and subtasks
- Non-numeric string ID preservation (API IDs)
- Zero, NaN, null, undefined, empty string handling
- Subtask parentId preservation with string parent IDs
- scripts/modules/utils.js
- Extracted normalizeDependencyArray() helper (lines 540-558)
- Updated normalizeTaskIds() to use helper in both locations (lines 578, 607)
- Eliminated 24 lines of duplicated code
**Tests**: 37/37 passing (100%)
**TypeCheck**: ✅ Pass
**Linting**: ✅ Pass
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/tm-core/src/common/utils/task-id-normalizer.ts`:
- Around line 246-266: The JSDoc example for normalizeTaskIds is incorrect
relative to the implementation: IDs, parentId and numeric dependency strings
should be returned as numbers when they represent integers. Update
normalizeTaskIds (and any helper like parseTaskId/normalizeId in
task-id-normalizer.ts) to coerce string IDs that match an integer pattern (e.g.,
/^\d+$/) into numbers for id and parentId and convert dependency entries
similarly while leaving non-integer strings (like "3.1") unchanged; then ensure
subtasks inherit parentId as the coerced type. Also update any internal places
that construct dependencies/subtasks to use the normalized numeric form so the
returned structure matches the documented example.
- Around line 210-223: The code assigns string values to taskIdValue when
normalizedId is non-null, when task.id is a numeric valid integer, or in
fallback, which contradicts the requirement that numeric IDs remain numbers;
update the logic in task-id-normalizer.ts (symbols: taskIdValue, normalizedId,
isValidPositiveInteger, and the branch that mirrors normalizeSubtask) so that
when an ID is a numeric positive integer you assign a number (e.g.,
Number(normalizedId) or +task.id) instead of String(...), preserve non-empty
string IDs as strings, and only use '0' (string) if the fallback must be a
string per storage contract or otherwise use 0 as a numeric fallback to match
the numeric ID requirement.
- Around line 156-170: The code currently converts numeric IDs to strings (using
String(...)) and the fallback uses '0', which causes numeric IDs to be stored as
JSON strings; change the assignments so numeric branches produce numbers
instead: in task-id-normalizer.ts update subtaskIdValue (referenced as
subtaskIdValue, normalizedId, isValidPositiveInteger, and subtask.id) to assign
numeric values for numeric cases (e.g., use Number(normalizedId) or +subtask.id)
and set the fallback to 0 (number) while keeping the branch that preserves
non-empty non-numeric string IDs as strings; this ensures numeric IDs are
written as numbers in tasks.json.
🧹 Nitpick comments (1)
scripts/modules/utils.js (1)
564-612: Consider aligning mutation behavior with tm-core implementation.This
normalizeTaskIdsmutates tasks in-place, while the tm-core TypeScript version (normalizeTaskIdsintask-id-normalizer.ts) returns new objects via.map(). This behavioral difference could cause subtle bugs if callers expect one behavior but get the other.For now this works correctly, but consider aligning the approaches when refactoring.
…sks.json
Summary:
Fixed inconsistent task ID serialization where numeric IDs were being converted to strings in tasks.json. The tm-core file storage adapter now correctly stores numeric IDs as numbers (e.g., {"id": 1} instead of {"id": "1"}), while preserving string IDs for API identifiers like "HAM-123".
Root Cause:
The `normalizeSubtask` and `normalizeTask` functions in task-id-normalizer.ts were explicitly converting all normalized numeric IDs to strings via `String(normalizedId)` and using string fallback values ('0'), contradicting the PR objective to store numeric IDs as numbers in file storage.
Solution:
- Modified `normalizeSubtask` to return numeric IDs as numbers instead of strings
- Modified `normalizeTask` to return numeric IDs as numbers instead of strings
- Updated fallback value from string '0' to numeric 0
- Fixed `file-storage.ts` updateTask method to use `normalizeTaskId()` instead of `String(taskId)`
- Updated all related test expectations to assert numeric types for numeric IDs
Implementation:
- packages/tm-core/src/common/utils/task-id-normalizer.ts: Changed ID assignment from String(normalizedId) to normalizedId, fallback from '0' to 0
- packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts: Added normalizeTaskId import and fixed updateTask to preserve numeric IDs
- packages/tm-core/src/common/utils/task-id-normalizer.spec.ts: Updated 18 test assertions from string expectations to numeric expectations
- packages/tm-core/tests/integration/storage/file-storage-metadata.test.ts: Fixed ID comparison to handle numeric IDs
Fixes inconsistent serialization identified in code review for PR eyaltoledano#1583.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/tm-core/tests/integration/storage/file-storage-metadata.test.ts`:
- Around line 234-242: The test currently stringifies IDs (String(t.id) === '1')
which allows string IDs to pass; update the assertions to compare numeric IDs
directly (e.g., use t.id === 1 and t.id === 2 or add an explicit type assertion)
so the test enforces the numeric-serialization contract for loadedTasks and
their metadata (reference: loadedTasks and t.id in the
file-storage-metadata.test).
Comment on lines
+234
to
+242
| // Use String() for comparison since IDs are normalized to numbers after save/load | ||
| expect( | ||
| loadedTasks.find((t) => String(t.id) === '1')?.metadata | ||
| ).toEqual({ | ||
| existing: true | ||
| }); | ||
| expect(loadedTasks.find((t) => t.id === '2')?.metadata).toEqual({ | ||
| expect( | ||
| loadedTasks.find((t) => String(t.id) === '2')?.metadata | ||
| ).toEqual({ |
Contributor
There was a problem hiding this comment.
Keep numeric ID assertions to enforce the new contract.
Line 234-242 stringifies IDs, so string IDs would still pass and the test no longer guards the numeric-serialization requirement. Prefer numeric comparisons (and optionally a type assertion) to lock the behavior.
🔧 Suggested change
- // Use String() for comparison since IDs are normalized to numbers after save/load
- expect(
- loadedTasks.find((t) => String(t.id) === '1')?.metadata
- ).toEqual({
+ const task1 = loadedTasks.find((t) => t.id === 1);
+ const task2 = loadedTasks.find((t) => t.id === 2);
+ expect(task1?.id).toBe(1);
+ expect(task2?.id).toBe(2);
+ expect(task1?.metadata).toEqual({
existing: true
});
- expect(
- loadedTasks.find((t) => String(t.id) === '2')?.metadata
- ).toEqual({
+ expect(task2?.metadata).toEqual({
newTask: true
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Use String() for comparison since IDs are normalized to numbers after save/load | |
| expect( | |
| loadedTasks.find((t) => String(t.id) === '1')?.metadata | |
| ).toEqual({ | |
| existing: true | |
| }); | |
| expect(loadedTasks.find((t) => t.id === '2')?.metadata).toEqual({ | |
| expect( | |
| loadedTasks.find((t) => String(t.id) === '2')?.metadata | |
| ).toEqual({ | |
| const task1 = loadedTasks.find((t) => t.id === 1); | |
| const task2 = loadedTasks.find((t) => t.id === 2); | |
| expect(task1?.id).toBe(1); | |
| expect(task2?.id).toBe(2); | |
| expect(task1?.metadata).toEqual({ | |
| existing: true | |
| }); | |
| expect(task2?.metadata).toEqual({ | |
| newTask: true | |
| }); |
🤖 Prompt for AI Agents
In `@packages/tm-core/tests/integration/storage/file-storage-metadata.test.ts`
around lines 234 - 242, The test currently stringifies IDs (String(t.id) ===
'1') which allows string IDs to pass; update the assertions to compare numeric
IDs directly (e.g., use t.id === 1 and t.id === 2 or add an explicit type
assertion) so the test enforces the numeric-serialization contract for
loadedTasks and their metadata (reference: loadedTasks and t.id in the
file-storage-metadata.test).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| if (Array.isArray(task.dependencies)) { | ||
| task.dependencies = normalizeDependencyArray(task.dependencies); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing NaN validation corrupts subtask IDs to null
Medium Severity
The dot-notation path in normalizeTaskIds assigns parseInt result directly without validating for NaN. For malformed subtask IDs like "5.abc", parseInt("abc", 10) returns NaN, which gets assigned to subtask.id. When serialized to JSON, NaN becomes null, corrupting the task data. The non-dot-notation path at lines 591-594 correctly validates with !isNaN(parsedSubtaskId) && parsedSubtaskId > 0, and the TypeScript equivalent in task-id-normalizer.ts also properly validates before returning.
| // Convert subtask dependencies using helper function | ||
| if (Array.isArray(subtask.dependencies)) { | ||
| subtask.dependencies = normalizeDependencyArray(subtask.dependencies); | ||
| } |
There was a problem hiding this comment.
Duplicated normalization logic between TypeScript and JavaScript
Medium Severity
The new normalizeDependencyArray and updated normalizeTaskIds functions duplicate the logic from task-id-normalizer.ts. Since utils.js already imports from @tm/core (line 18: import { FileOperations } from '@tm/core';), it could import normalizeTaskIds and normalizeDependencies from @tm/core instead of maintaining a parallel implementation.
Crunchyman-ralph
requested changes
Feb 4, 2026
Collaborator
Crunchyman-ralph
left a comment
Crunchyman-ralph
left a comment
There was a problem hiding this comment.
this PR is great, one comment, I think we should pick either number or string, not either or.
So I guess lets stick to string, and enforce that everywhere in the codebase instead of trying to play nice on both, what do you think ?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes inconsistent task ID serialization in tasks.json. Task IDs are now consistently stored as numbers (without quotes) in both "set task" and "update task" operations.
Background: The codebase has two storage layers writing to tasks.json - the legacy CLI layer and the new tm-core layer. The tm-core file storage adapter was incorrectly using string normalization intended for API storage (which supports prefixed IDs like "HAM-123"), causing IDs to flip between strings and numbers depending on which command was used.
Changes:
Result: Task and subtask IDs are now consistently saved as numbers in tasks.json, eliminating unnecessary git diffs and file instability.
Type of Change
Testing
Changeset
Additional Notes
Closes #1583
Note
Medium Risk
Moderate risk because it changes core ID typing and serialization across storage, execution, and CLI paths; could surface edge cases where string IDs or subtask/dependency references are assumed to be strings.
Overview
Fixes inconsistent
tasks.jsonchurn by making file storage serialize task/subtask IDs as numbers (while still allowing API-style string IDs where needed).Introduces a shared
task-id-normalizerutility intm-core(with new unit tests) and replaces ad-hoc normalization in file storage/format handling; also updates legacy script writes to normalize IDs and dependency/parentId fields before persisting.Updates core types and several call sites (CLI
start, task lookup/execution, API storage operations, and UI dependency typing) to acceptnumber | stringIDs and explicitly coerce to strings where APIs/logging require it.Written by Cursor Bugbot for commit 983b850. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.