feat: auto-slim completed tasks to reduce tasks.json size#1666
feat: auto-slim completed tasks to reduce tasks.json size#1666withsivram wants to merge 4 commits intoeyaltoledano:mainfrom
Conversation
…ixes eyaltoledano#1647) When moving tasks between tags, if a task's ID already exists in the target tag, the task is now automatically renumbered to the next available ID instead of throwing an error. Dependencies within the moved batch are updated to reflect the new IDs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ltoledano#1642) When a task transitions to "done" status, automatically slim it by: - Clearing `details` and `testStrategy` fields (set to empty string) - Truncating `description` to 200 characters with "..." suffix - Slimming all subtasks when a parent task is marked done This is controlled by the `slimDoneTasks` config option (default: true). Git history preserves pre-slim content. Only triggers on transition TO done, not on already-done tasks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds configurable automatic slimming of tasks when transitioned to done and implements automatic ID renumbering during cross‑tag moves (updates dependencies, records original→new ID mappings, and surfaces renumbering in MCP success messages). Tests and config defaults updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SetTaskStatus
participant ConfigManager
participant UpdateSingleTask
participant SlimTask
participant Storage
User->>SetTaskStatus: setTaskStatus(id, "done")
SetTaskStatus->>ConfigManager: isSlimDoneTasksEnabled(explicitRoot)
ConfigManager-->>SetTaskStatus: true/false
SetTaskStatus->>UpdateSingleTask: updateSingleTaskStatus(..., { slimOnDone })
UpdateSingleTask->>UpdateSingleTask: Check status transition
alt Transitioning to done/completed
UpdateSingleTask->>SlimTask: slimTaskOnComplete(task, oldStatus, newStatus)
SlimTask->>SlimTask: Clear details, testStrategy
SlimTask->>SlimTask: Truncate description to 200 chars
SlimTask->>SlimTask: Slim subtasks when parent completes
end
UpdateSingleTask->>Storage: Write updated task(s)
Storage-->>User: Updated task saved
sequenceDiagram
participant User
participant MoveTasksAPI
participant MoveLogic
participant IdResolver
participant DependencyUpdater
participant Storage
User->>MoveTasksAPI: moveTasksBetweenTags(source, target, ids)
MoveTasksAPI->>MoveLogic: executeMoveOperation()
MoveLogic->>IdResolver: Check for ID collisions in target
alt ID collision detected
IdResolver->>IdResolver: getNextAvailableId(existingIds, idRemapping)
IdResolver-->>MoveLogic: newId
MoveLogic->>MoveLogic: Update task.id = newId
MoveLogic->>MoveLogic: Record originalId → newId in idRemapping
end
MoveLogic->>DependencyUpdater: Remap dependencies within moved batch using idRemapping
DependencyUpdater->>DependencyUpdater: Update parent.subId and subtask dependency strings
MoveLogic->>Storage: Write moved tasks to target tag
MoveLogic-->>User: Success response with renumbering details (originalId → newId)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Pull request overview
Adds an “auto-slim on completion” behavior to reduce tasks.json bloat by clearing verbose task fields when tasks transition to done, controlled via a new global config flag. The PR also includes a separate behavioral change to cross-tag moves that auto-renumbers tasks on ID collisions.
Changes:
- Add
global.slimDoneTasks(defaulttrue) and plumb it intoset-task-status→update-single-task-statusto slim tasks/subtasks on transition todone. - Introduce
slim-task.jsutilities to cleardetails/testStrategyand truncatedescription. - Change cross-tag move behavior to auto-renumber tasks on ID collisions (and update related integration/MCP messaging).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/scripts/modules/task-manager/set-task-status.test.js | Updates mocks/expectations for the new slimming option passed into status updates. |
| tests/unit/config-manager.test.js | Extends default-config fixture with slimDoneTasks. |
| tests/integration/move-task-cross-tag.integration.test.js | Updates expectations to reflect auto-renumbering on cross-tag ID collisions. |
| scripts/modules/task-manager/update-single-task-status.js | Adds optional statusOptions.slimOnDone and calls slim helpers during status transitions. |
| scripts/modules/task-manager/slim-task.js | New module implementing task/subtask slimming behavior on completion. |
| scripts/modules/task-manager/set-task-status.js | Reads slimDoneTasks config and passes slimOnDone into updateSingleTaskStatus. |
| scripts/modules/task-manager/move-task.js | Implements auto-renumbering on ID collisions and attempts to remap dependencies post-move. |
| scripts/modules/config-manager.js | Adds default global.slimDoneTasks and isSlimDoneTasksEnabled() getter export. |
| mcp-server/src/core/direct-functions/move-task-cross-tag.js | Updates returned message to include renumbering details when applicable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (existingTaskIndex !== -1) { | ||
| throw new MoveTaskError( | ||
| MOVE_ERROR_CODES.TASK_ALREADY_EXISTS, | ||
| `Task ${taskId} already exists in target tag "${targetTag}"`, | ||
| { | ||
| conflictingId: normalizedTaskId, | ||
| targetTag, | ||
| suggestions: [ | ||
| 'Choose a different target tag without conflicting IDs', | ||
| 'Move a different set of IDs (avoid existing ones)', | ||
| 'If needed, move within-tag to a new ID first, then cross-tag move' | ||
| ] | ||
| } | ||
| // ID collision detected — auto-renumber to the next available ID | ||
| assignedId = getNextAvailableId(rawData, targetTag, idRemapping); | ||
| idRemapping.set(normalizedTaskId, assignedId); | ||
| log( | ||
| 'info', | ||
| `Task ${normalizedTaskId} conflicts with existing ID in "${targetTag}", renumbered to ${assignedId}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
This auto-renumbering behavior on ID conflicts is a significant semantic change (previously a collision threw TASK_ALREADY_EXISTS). It’s also unrelated to the PR’s stated purpose (auto-slimming done tasks). Either split this change into a separate PR or update the PR description/issues to reflect the cross-tag move behavior change and its implications for users.
| if (movedTask && Array.isArray(movedTask.dependencies)) { | ||
| movedTask.dependencies = movedTask.dependencies.map((dep) => { | ||
| const normalizedDep = normalizeDependency(dep); | ||
| if ( | ||
| Number.isFinite(normalizedDep) && | ||
| idRemapping.has(normalizedDep) | ||
| ) { | ||
| return idRemapping.get(normalizedDep); | ||
| } | ||
| return dep; | ||
| }); |
There was a problem hiding this comment.
The dependency remapping uses normalizeDependency(dep), which collapses dotted subtask dependencies like "2.1" to the parent task ID (2). If an ID is remapped, this code will replace a subtask dependency string with a numeric task ID, losing the .subtask portion and breaking dependency semantics. Preserve dotted dependency strings by only remapping the parent portion (and keep the original type/format) when applying idRemapping.
| if (movedTask && Array.isArray(movedTask.subtasks)) { | ||
| movedTask.subtasks.forEach((subtask) => { | ||
| if (Array.isArray(subtask.dependencies)) { | ||
| subtask.dependencies = subtask.dependencies.map((dep) => { | ||
| const normalizedDep = normalizeDependency(dep); | ||
| if ( | ||
| Number.isFinite(normalizedDep) && | ||
| idRemapping.has(normalizedDep) | ||
| ) { | ||
| return idRemapping.get(normalizedDep); | ||
| } | ||
| return dep; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Same issue for subtask dependencies: normalizeDependency(dep) will turn strings like "5.1" into 5, so when an ID is remapped this can overwrite a subtask dependency with a parent task ID and drop the subtask suffix. When remapping, detect dotted dependency strings and rewrite only the parent part (e.g., ${newParent}.${subId}) rather than replacing the whole value with a number.
|
|
||
| /** | ||
| * Slim a completed task by removing verbose fields that are no longer actionable. | ||
| * Removes `details` and `testStrategy`, truncates `description` to 200 chars. |
There was a problem hiding this comment.
The docstring says "truncates description to 200 chars", but the implementation keeps the first 200 characters and then appends "..." (resulting in a string longer than 200). Update the comment (or the truncation logic) so the documented behavior matches the actual output length.
| * Removes `details` and `testStrategy`, truncates `description` to 200 chars. | |
| * Removes `details` and `testStrategy`, and if `description` exceeds 200 characters, | |
| * keeps the first 200 characters and appends an ellipsis ("..."). |
| function slimTask(task) { | ||
| if (!task) return task; | ||
|
|
||
| // Clear verbose fields | ||
| if (task.details) { | ||
| task.details = ''; | ||
| } | ||
|
|
||
| if (task.testStrategy) { | ||
| task.testStrategy = ''; | ||
| } | ||
|
|
||
| // Truncate description to max length | ||
| if ( | ||
| task.description && | ||
| task.description.length > DESCRIPTION_TRUNCATE_LENGTH | ||
| ) { | ||
| task.description = | ||
| task.description.substring(0, DESCRIPTION_TRUNCATE_LENGTH) + '...'; | ||
| } |
There was a problem hiding this comment.
This PR introduces new behavior (slimming tasks/subtasks on transition to done) but there are no unit tests asserting the actual slimming results (e.g., details/testStrategy cleared, description truncation, and that slimming only happens on the transition-to-done). Adding focused unit tests around slimTaskOnComplete/slimSubtaskOnComplete (or via updateSingleTaskStatus) would help prevent regressions.
| @@ -2,6 +2,7 @@ import chalk from 'chalk'; | |||
|
|
|||
| import { isValidTaskStatus } from '../../../src/constants/task-status.js'; | |||
There was a problem hiding this comment.
TASK_STATUS_OPTIONS is referenced in the invalid-status error message, but it is not imported in this module. As-is, passing an invalid status will throw a ReferenceError instead of the intended error. Import TASK_STATUS_OPTIONS from src/constants/task-status.js (alongside isValidTaskStatus) or change the message to avoid referencing an undefined symbol.
| import { isValidTaskStatus } from '../../../src/constants/task-status.js'; | |
| import { isValidTaskStatus, TASK_STATUS_OPTIONS } from '../../../src/constants/task-status.js'; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
scripts/modules/task-manager/move-task.js (2)
828-834: Potential undefined access if target tag was just created.The function assumes
rawData[targetTag].tasksexists. WhilevalidateMovecreates the target tag structure (line 647-648), consider adding a defensive check.🛡️ Optional defensive fix
function getNextAvailableId(rawData, targetTag, idRemapping) { - const existingIds = rawData[targetTag].tasks.map((t) => t.id); + const existingIds = (rawData[targetTag]?.tasks || []).map((t) => t.id); const remappedIds = Array.from(idRemapping.values()); const allIds = [...existingIds, ...remappedIds]; if (allIds.length === 0) return 1; return Math.max(...allIds) + 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/modules/task-manager/move-task.js` around lines 828 - 834, getNextAvailableId assumes rawData[targetTag].tasks exists and can throw if the tag was just created without a tasks array; update getNextAvailableId to defensively read existingIds from rawData[targetTag]?.tasks or an empty array (e.g., use optional chaining or an explicit existence/Array.isArray check) so mapping an empty array is safe, then continue computing remappedIds and the max; refer to getNextAvailableId and validateMove to ensure the target tag creation won't cause undefined access.
943-985: Dependency remapping only updates the moved batch, leaving potential dangling references in the source tag.The remapping logic correctly updates dependencies within the moved tasks when another moved task gets renumbered. However, tasks remaining in the source tag that depended on moved tasks will have dangling references. This is expected behavior (addressed by the tip at line 1043-1045), but documenting this limitation in a code comment would help future maintainers.
📝 Suggested documentation comment
// After all tasks are moved, update cross-references within the moved batch. // If task A depended on task B and both were moved but B got renumbered, // update A's dependency to point to B's new ID. + // NOTE: Tasks left behind in the source tag are NOT updated here. If they + // depended on moved tasks, they will have dangling references. Users are + // advised to run validate-dependencies (see tip added in finalizeMove). if (idRemapping.size > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/modules/task-manager/move-task.js` around lines 943 - 985, Add a brief code comment above the idRemapping handling block that documents the limitation: explain that the current remapping loop (referencing idRemapping, tasksToMove, rawData[targetTag], movedTask and normalizeDependency) only updates dependencies inside the moved batch and its subtasks, and that tasks remaining in the source tag which depended on moved tasks may keep dangling references; suggest callers should handle cleaning up source-tag references if needed.mcp-server/src/core/direct-functions/move-task-cross-tag.js (1)
204-215: Dead code:TASK_ALREADY_EXISTSerror handling is unreachable.Since
moveTasksBetweenTagsnow auto-renumbers on ID collisions instead of throwingTASK_ALREADY_EXISTS, this error handling block will never be executed. Consider removing it or adding a comment explaining it's kept for potential future changes.♻️ Option 1: Remove dead code
- } else if ( - error.code === 'TASK_ALREADY_EXISTS' || - error.message?.includes('already exists in target tag') - ) { - // Target tag has an ID collision - errorCode = 'TASK_ALREADY_EXISTS'; - suggestions = [ - 'Choose a different target tag without conflicting IDs', - 'Move a different set of IDs (avoid existing ones)', - 'If needed, move within-tag to a new ID first, then cross-tag move' - ]; - }📝 Option 2: Add explanatory comment
} else if ( error.code === 'TASK_ALREADY_EXISTS' || error.message?.includes('already exists in target tag') ) { - // Target tag has an ID collision + // NOTE: As of auto-renumbering feature, cross-tag moves no longer throw + // TASK_ALREADY_EXISTS. Kept for backward compatibility with potential + // future changes or edge cases. errorCode = 'TASK_ALREADY_EXISTS';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp-server/src/core/direct-functions/move-task-cross-tag.js` around lines 204 - 215, The else-if branch handling the 'TASK_ALREADY_EXISTS' case in move-task-cross-tag.js is now dead because moveTasksBetweenTags auto-renumbers on ID collisions; remove the entire else-if block that checks error.code === 'TASK_ALREADY_EXISTS' || error.message?.includes('already exists in target tag') along with its assignments to errorCode and suggestions (and any related comment), or if you prefer to keep it, replace it with a short explanatory comment referencing moveTasksBetweenTags to indicate why the block is intentionally unreachable; update any tests or callers that assumed that errorCode/suggestions branch if present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/modules/task-manager/slim-task.js`:
- Around line 115-130: slimSubtaskOnComplete can throw if oldStatus or newStatus
is null/undefined because the code calls toLowerCase() directly; update the
function (referencing slimSubtaskOnComplete and slimSubtask) to defensively
normalize statuses before comparing (e.g., coerce oldStatus/newStatus to empty
string or check for falsy first) and then perform the lowercase comparisons,
returning early as before and calling slimSubtask(subtask) only when the safe
comparisons indicate a transition to done/completed.
- Around line 80-91: slimTaskOnComplete can throw when oldStatus (or newStatus)
is null/undefined because code calls
oldStatus.toLowerCase()/newStatus.toLowerCase(); fix by normalizing both
statuses before calling toLowerCase (e.g., coalesce to '' or 'pending') and use
those normalized variables in the isDoneStatus and wasDone checks so
slimTaskOnComplete, and any callers of oldStatus/newStatus, safely handle
missing values.
---
Nitpick comments:
In `@mcp-server/src/core/direct-functions/move-task-cross-tag.js`:
- Around line 204-215: The else-if branch handling the 'TASK_ALREADY_EXISTS'
case in move-task-cross-tag.js is now dead because moveTasksBetweenTags
auto-renumbers on ID collisions; remove the entire else-if block that checks
error.code === 'TASK_ALREADY_EXISTS' || error.message?.includes('already exists
in target tag') along with its assignments to errorCode and suggestions (and any
related comment), or if you prefer to keep it, replace it with a short
explanatory comment referencing moveTasksBetweenTags to indicate why the block
is intentionally unreachable; update any tests or callers that assumed that
errorCode/suggestions branch if present.
In `@scripts/modules/task-manager/move-task.js`:
- Around line 828-834: getNextAvailableId assumes rawData[targetTag].tasks
exists and can throw if the tag was just created without a tasks array; update
getNextAvailableId to defensively read existingIds from
rawData[targetTag]?.tasks or an empty array (e.g., use optional chaining or an
explicit existence/Array.isArray check) so mapping an empty array is safe, then
continue computing remappedIds and the max; refer to getNextAvailableId and
validateMove to ensure the target tag creation won't cause undefined access.
- Around line 943-985: Add a brief code comment above the idRemapping handling
block that documents the limitation: explain that the current remapping loop
(referencing idRemapping, tasksToMove, rawData[targetTag], movedTask and
normalizeDependency) only updates dependencies inside the moved batch and its
subtasks, and that tasks remaining in the source tag which depended on moved
tasks may keep dangling references; suggest callers should handle cleaning up
source-tag references if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8ebc69c-a6bf-4b85-823e-d96b47837757
📒 Files selected for processing (9)
mcp-server/src/core/direct-functions/move-task-cross-tag.jsscripts/modules/config-manager.jsscripts/modules/task-manager/move-task.jsscripts/modules/task-manager/set-task-status.jsscripts/modules/task-manager/slim-task.jsscripts/modules/task-manager/update-single-task-status.jstests/integration/move-task-cross-tag.integration.test.jstests/unit/config-manager.test.jstests/unit/scripts/modules/task-manager/set-task-status.test.js
| function slimTaskOnComplete(task, oldStatus, newStatus) { | ||
| const isDoneStatus = | ||
| newStatus.toLowerCase() === 'done' || | ||
| newStatus.toLowerCase() === 'completed'; | ||
| const wasDone = | ||
| oldStatus.toLowerCase() === 'done' || | ||
| oldStatus.toLowerCase() === 'completed'; | ||
|
|
||
| // Only slim on transition TO done, not if already done | ||
| if (!isDoneStatus || wasDone) { | ||
| return task; | ||
| } |
There was a problem hiding this comment.
Potential TypeError if oldStatus is null/undefined.
oldStatus.toLowerCase() at lines 85-86 will throw if oldStatus is null or undefined. While callers currently pass oldStatus from subtask.status || 'pending', defensive handling here would be safer.
🛡️ Proposed defensive fix
function slimTaskOnComplete(task, oldStatus, newStatus) {
+ if (!oldStatus || !newStatus) return task;
+
const isDoneStatus =
newStatus.toLowerCase() === 'done' ||
newStatus.toLowerCase() === 'completed';
const wasDone =
- oldStatus.toLowerCase() === 'done' ||
- oldStatus.toLowerCase() === 'completed';
+ (oldStatus || '').toLowerCase() === 'done' ||
+ (oldStatus || '').toLowerCase() === 'completed';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/modules/task-manager/slim-task.js` around lines 80 - 91,
slimTaskOnComplete can throw when oldStatus (or newStatus) is null/undefined
because code calls oldStatus.toLowerCase()/newStatus.toLowerCase(); fix by
normalizing both statuses before calling toLowerCase (e.g., coalesce to '' or
'pending') and use those normalized variables in the isDoneStatus and wasDone
checks so slimTaskOnComplete, and any callers of oldStatus/newStatus, safely
handle missing values.
| function slimSubtaskOnComplete(subtask, oldStatus, newStatus) { | ||
| const isDoneStatus = | ||
| newStatus.toLowerCase() === 'done' || | ||
| newStatus.toLowerCase() === 'completed'; | ||
| const wasDone = | ||
| oldStatus.toLowerCase() === 'done' || | ||
| oldStatus.toLowerCase() === 'completed'; | ||
|
|
||
| if (!isDoneStatus || wasDone) { | ||
| return subtask; | ||
| } | ||
|
|
||
| log('info', `Slimming completed subtask ${subtask.id}: "${subtask.title}"`); | ||
|
|
||
| return slimSubtask(subtask); | ||
| } |
There was a problem hiding this comment.
Same null safety concern for oldStatus parameter.
Apply the same defensive handling as suggested for slimTaskOnComplete.
🛡️ Proposed defensive fix
function slimSubtaskOnComplete(subtask, oldStatus, newStatus) {
+ if (!oldStatus || !newStatus) return subtask;
+
const isDoneStatus =
newStatus.toLowerCase() === 'done' ||
newStatus.toLowerCase() === 'completed';
const wasDone =
- oldStatus.toLowerCase() === 'done' ||
- oldStatus.toLowerCase() === 'completed';
+ (oldStatus || '').toLowerCase() === 'done' ||
+ (oldStatus || '').toLowerCase() === 'completed';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/modules/task-manager/slim-task.js` around lines 115 - 130,
slimSubtaskOnComplete can throw if oldStatus or newStatus is null/undefined
because the code calls toLowerCase() directly; update the function (referencing
slimSubtaskOnComplete and slimSubtask) to defensively normalize statuses before
comparing (e.g., coerce oldStatus/newStatus to empty string or check for falsy
first) and then perform the lowercase comparisons, returning early as before and
calling slimSubtask(subtask) only when the safe comparisons indicate a
transition to done/completed.
… unit tests - Import TASK_STATUS_OPTIONS in update-single-task-status.js to fix ReferenceError when validating invalid status values - Update slimTask docstring to say "200 chars + ellipsis" (actual output can be 203 chars due to appended "...") - Add 24 unit tests for slim-task module covering: description truncation, details/testStrategy clearing, transition-only slimming, subtask slimming, and edge cases (null/undefined inputs) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/modules/task-manager/update-single-task-status.js (1)
25-28: Consider defensive handling for nullstatusOptions.If a caller explicitly passes
nullforstatusOptions, line 28's destructuring will throw a TypeError. While the current caller inset-task-status.jsalways passes a valid object, adding a guard would make this more robust for future callers.🛡️ Defensive fix
- statusOptions = {} + statusOptions = null ) { - const { slimOnDone = false } = statusOptions; + const { slimOnDone = false } = statusOptions || {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/modules/task-manager/update-single-task-status.js` around lines 25 - 28, The destructuring "const { slimOnDone = false } = statusOptions;" will throw if a caller passes null for statusOptions; make it defensive by falling back to an empty object before destructuring (e.g., use statusOptions || {} in the assignment) so slimOnDone is safely defaulted. Update the code in update-single-task-status.js where statusOptions and slimOnDone are used to use this guarded destructuring to avoid a TypeError when null is passed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/modules/task-manager/update-single-task-status.js`:
- Around line 25-28: The destructuring "const { slimOnDone = false } =
statusOptions;" will throw if a caller passes null for statusOptions; make it
defensive by falling back to an empty object before destructuring (e.g., use
statusOptions || {} in the assignment) so slimOnDone is safely defaulted. Update
the code in update-single-task-status.js where statusOptions and slimOnDone are
used to use this guarded destructuring to avoid a TypeError when null is passed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b0968dc-fa51-40e2-a7b1-82c984f115f3
📒 Files selected for processing (3)
scripts/modules/task-manager/slim-task.jsscripts/modules/task-manager/update-single-task-status.jstests/unit/scripts/modules/task-manager/slim-task.test.js
✅ Files skipped from review due to trivial changes (1)
- scripts/modules/task-manager/slim-task.js
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
donestatus, automatically slim it by clearingdetailsandtestStrategyfields and truncatingdescriptionto 200 charactersslimDoneTasksconfig option inglobalsettings (default:true, can be disabled by setting tofalse)Test plan
set-task-statustests pass (12/12) with updated mock for new config optionupdate-single-task-statustests pass (5/5)config-managertests pass (35/35) withslimDoneTasksadded to DEFAULT_CONFIG fixturedetails/testStrategyare cleared in tasks.jsonslimDoneTasks: falsein configCloses #1642
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests