fix: escape angle brackets in markdown code blocks#1667
Open
withsivram wants to merge 2 commits intoeyaltoledano:mainfrom
Open
fix: escape angle brackets in markdown code blocks#1667withsivram wants to merge 2 commits intoeyaltoledano:mainfrom
withsivram wants to merge 2 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>
…o#1562) Angle brackets inside fenced code blocks (e.g. JSX like <Navigate />) were incorrectly detected as HTML tags, causing the entire content to be run through TurndownService which corrupted the markdown rendering. The fix adds a hasHtmlOutsideCodeBlocks() helper that strips fenced code blocks and inline code spans before testing for HTML tags, so only actual HTML outside code triggers the Turndown conversion. Includes regression tests covering the exact scenario from the issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Contributor
📝 WalkthroughWalkthroughThis pull request fixes markdown rendering issues with angle brackets in code blocks (issue Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant moveTasksBetweenTags as moveTasksBetweenTags
participant executeMoveOperation as executeMoveOperation
participant getNextAvailableId as getNextAvailableId
participant Database as Database/rawData
participant finalizeMove as finalizeMove
Client->>moveTasksBetweenTags: Move tasks between tags
moveTasksBetweenTags->>executeMoveOperation: Execute move operation
loop For each task
executeMoveOperation->>Database: Check if ID exists in target tag
alt ID Conflict Detected
executeMoveOperation->>getNextAvailableId: Get next available ID
getNextAvailableId->>Database: Query target tag IDs + remapping
getNextAvailableId-->>executeMoveOperation: Return assignedId
executeMoveOperation->>executeMoveOperation: Record originalId → newId mapping
executeMoveOperation->>executeMoveOperation: Update task.id to assignedId
else No Conflict
executeMoveOperation->>executeMoveOperation: Keep original ID
end
executeMoveOperation->>executeMoveOperation: Update dependency references
end
executeMoveOperation->>Database: Persist updated tasks
executeMoveOperation->>finalizeMove: Finalize with idRemapping
finalizeMove->>finalizeMove: Build success message with renumbering details
finalizeMove-->>Client: Return result with movedTasks & message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
This PR aims to prevent markdown rendering corruption in the CLI when angle brackets appear inside fenced/inline code by avoiding HTML-to-Markdown conversion unless HTML is detected outside code. It also introduces a separate behavioral change to cross-tag task moves: ID collisions in the target tag are now auto-renumbered instead of erroring.
Changes:
- Add
hasHtmlOutsideCodeBlocks()to avoid treating<...>inside code as HTML, preventing Turndown from corrupting markdown output. - Add CLI regression tests for markdown/code-block angle bracket scenarios.
- Change cross-tag move behavior to auto-renumber on ID collisions and update associated messaging/tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/cli/src/utils/content-renderer.ts | Improves HTML detection by ignoring fenced/inline code before deciding to run Turndown. |
| apps/cli/src/utils/content-renderer.spec.ts | Adds regression coverage for angle brackets in code blocks/inline code and HTML handling. |
| scripts/modules/task-manager/move-task.js | Auto-renumbers IDs on cross-tag collision and attempts to remap dependencies after renumbering. |
| mcp-server/src/core/direct-functions/move-task-cross-tag.js | Adjusts returned success message to include renumbering details. |
| tests/integration/move-task-cross-tag.integration.test.js | Updates integration expectation from “throw on collision” to “renumber and succeed”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+956
to
+965
| 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.
When updating dependencies after renumbering, dotted dependency strings like "3.1" (subtask references) will be corrupted: normalizeDependency() returns only the parent ID (3), so the current logic replaces the entire dependency with the new parent ID and drops the .1 suffix. This breaks cross-task subtask dependencies when a referenced task is renumbered. Preserve the subtask suffix when remapping (e.g., "3.1" -> "7.1"), and avoid changing the dependency value type unnecessarily.
Comment on lines
+971
to
+980
| 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.
The subtask dependency remapping loop also uses normalizeDependency(dep) and, on a match, replaces the full dependency entry with the new parent task ID. For dependencies expressed as dotted strings (e.g., "2.1"), this drops the subtask portion and changes semantics. Adjust the remap to preserve dotted-format dependencies (and their type) when the parent ID is renumbered.
Comment on lines
+828
to
+833
| function getNextAvailableId(rawData, targetTag, idRemapping) { | ||
| 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; |
There was a problem hiding this comment.
getNextAvailableId() assumes rawData[targetTag].tasks is always an array. If the target tag exists but has a malformed shape (e.g., missing tasks), this will throw before you can recover. Consider normalizing/initializing rawData[targetTag].tasks (or guarding with Array.isArray) inside this helper to make renumbering resilient.
Comment on lines
885
to
893
| 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 PR is titled/described as a markdown rendering fix (#1562), but this hunk changes cross-tag move semantics from throwing TASK_ALREADY_EXISTS on ID collision to auto-renumbering and continuing. That’s a user-visible behavior/API change and likely deserves mention in the PR description/title (or to be split into a separate PR) so it can be reviewed/released independently.
Comment on lines
+120
to
+130
| // Check if any tasks were renumbered during the move | ||
| const renumberedTasks = (result.movedTasks || []).filter( | ||
| (t) => t.newId !== undefined | ||
| ); | ||
| let message = `Successfully moved ${sourceIds.length} task(s) from "${args.sourceTag}" to "${args.targetTag}"`; | ||
| if (renumberedTasks.length > 0) { | ||
| const renumberDetails = renumberedTasks | ||
| .map((t) => `${t.originalId} → ${t.newId}`) | ||
| .join(', '); | ||
| message += `. Renumbered to avoid ID collisions: ${renumberDetails}`; | ||
| } |
There was a problem hiding this comment.
The success message here is built from sourceIds.length, but moveTasksBetweenTags() may move additional tasks when withDependencies is true. In that case the message will report the wrong count (and may diverge from result.movedTasks.length). Prefer deriving the count from result.movedTasks.length/result.message so it stays accurate across move options.
![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: 2
🧹 Nitpick comments (4)
mcp-server/src/core/direct-functions/move-task-cross-tag.js (1)
120-136: Consider usingresult.messagedirectly instead of reconstructing.The core function
finalizeMovealready constructs a message with renumbering details. This direct function reconstructs a similar message but usessourceIds.length(original request count) instead ofmovedTasks.length(actual moved count, which may include dependencies). This could cause inconsistency when--with-dependenciespulls in additional tasks.♻️ Proposed simplification
- // Check if any tasks were renumbered during the move - const renumberedTasks = (result.movedTasks || []).filter( - (t) => t.newId !== undefined - ); - let message = `Successfully moved ${sourceIds.length} task(s) from "${args.sourceTag}" to "${args.targetTag}"`; - if (renumberedTasks.length > 0) { - const renumberDetails = renumberedTasks - .map((t) => `${t.originalId} → ${t.newId}`) - .join(', '); - message += `. Renumbered to avoid ID collisions: ${renumberDetails}`; - } - return { success: true, data: { ...result, - message, + // result.message from core already includes renumbering details moveOptions, sourceTag: args.sourceTag, targetTag: args.targetTag } };🤖 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 120 - 136, The code rebuilds a success message using sourceIds.length which can differ from the actual moved set when dependencies are included; instead use the message already produced by finalizeMove (i.e., use result.message) or, if you need to compute the count here, derive it from result.movedTasks.length (or result.movedTasks || []).length to reflect the actual moved tasks; update the block in move-task-cross-tag.js to stop reconstructing the message and return result.message (or use result.movedTasks.length) so the response is consistent with finalizeMove’s logic.tests/integration/move-task-cross-tag.integration.test.js (1)
553-610: Consider adding test coverage for dependency remapping during renumbering.The current test validates basic ID renumbering but doesn't verify that dependencies referencing renumbered tasks are correctly updated. Consider adding a test case where:
- Task A depends on Task B
- Both are moved, and Task B gets renumbered
- Verify Task A's dependency is updated to reference Task B's new ID
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/move-task-cross-tag.integration.test.js` around lines 553 - 610, Add a new integration test that exercises dependency remapping when renumbering by using moveTasksBetweenTags: create mock readJSON data where Task A (id 1) depends on Task B (id 2), move both from sourceTag to targetTag where id 2 conflicts and will be renumbered, call moveTasksBetweenTags, then assert the result reports renumbering and the writtenData (from mockUtils.writeJSON) shows Task B has the new ID and Task A's dependencies array has been updated to reference Task B's new ID; reference moveTasksBetweenTags, mockUtils.readJSON and mockUtils.writeJSON to locate where to inject this scenario and validate dependency updates.apps/cli/src/utils/content-renderer.ts (2)
57-61: Add explicit JSDoc contract tags for this utility helper.Please add
@paramand@returns(and a brief example) to make the behavior contract explicit for this non-trivial regex logic.Suggested doc-only update
/** * Check if content contains HTML tags outside of markdown code blocks and inline code. * Strips fenced code blocks (```...```) and inline code (`...`) before testing, * so angle brackets inside code (e.g. JSX like <Navigate />) are not mistaken for HTML. + * `@param` text Raw content that may contain markdown, inline code, and/or HTML. + * `@returns` True when an HTML-like tag is detected outside code contexts. + * `@example` + * hasHtmlOutsideCodeBlocks('```tsx\n<Navigate />\n```') // false */As per coding guidelines: "Document all parameters and return values in JSDoc format, include descriptions for complex logic, and add examples for non-obvious usage".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/utils/content-renderer.ts` around lines 57 - 61, Add explicit JSDoc tags to the hasHtmlOutsideCodeBlocks utility: add `@param` describing the text argument (e.g. "text: Raw content that may contain markdown, inline code, and/or HTML"), add `@returns` describing the boolean result ("True when an HTML-like tag is detected outside code contexts"), and include a brief `@example` showing usage (e.g. hasHtmlOutsideCodeBlocks('```tsx\n<Navigate />\n```') // false). Place these tags in the existing comment block above the function in content-renderer.ts so the regex behavior contract is clear.
62-69: Regex stripping is still fragile for some valid markdown code-span/fence forms.Current stripping can miss edge cases (like multi-backtick inline spans), which may still allow false HTML detection in rare inputs and re-route through Turndown unexpectedly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/utils/content-renderer.ts` around lines 62 - 69, The current hasHtmlOutsideCodeBlocks function uses fragile regex that misses edge cases (multi-backtick inline spans, complex fences) and causes false HTML detection; replace the fragile stripping with a proper Markdown parse-and-inspect approach: parse the input with a Markdown parser (e.g., remark/unified or any project-preferred markdown tokenizer), walk the AST and ignore/skip nodes representing code (code, inlineCode, codeFence, etc.), then search only the remaining text/HTML nodes for tags, or alternatively enhance the logic in hasHtmlOutsideCodeBlocks to correctly handle multi-backtick inline code spans and variable-length fences by using the parser’s token stream to detect code spans rather than regex.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/utils/content-renderer.spec.ts`:
- Around line 23-39: The regression tests for renderContent are too weak because
they only check keyword presence; update the two tests ("should not treat angle
brackets inside fenced code blocks as HTML (issue `#1562`)" and "should not treat
angle brackets inside inline code as HTML") to assert preserved structure: call
renderContent(input) and assert that the output contains a code block or
pre/code wrapper (e.g., check for '<pre', '<code', or a newline-delimited fenced
block surrounding 'export function Component' and the '<Navigate' text) and that
"Implementation" appears followed by a newline or heading markup (e.g.,
'Implementation' then '\n' or '<h2' before the code), and for the inline case
assert that the inline code formatting is preserved (e.g., '`<Navigate>`'
rendered as '<code' or backticks preserved) so the tests fail if content is
collapsed into a single mangled line.
In `@scripts/modules/task-manager/move-task.js`:
- Around line 943-985: The dependency remapping loses subtask suffixes because
normalizeDependency returns the numeric parent ID and you replace the whole dep
with idRemapping.get(normalizedDep); update the remapping logic inside the
movedTask.dependencies and movedTask.subtasks[].dependencies map callbacks to:
extract the original dep string, detect and preserve any dotted suffix (e.g.,
".2"), look up the new parent ID via idRemapping.get(normalizedDep), and return
either the numeric new ID (if original was numeric) or the reconstructed dotted
string `${newParent}${suffix}` (if original had a suffix); apply this same
reconstruction for both dependency arrays referenced in the movedTask processing
(functions/variables: normalizeDependency, idRemapping, movedTask,
movedTask.subtasks).
---
Nitpick comments:
In `@apps/cli/src/utils/content-renderer.ts`:
- Around line 57-61: Add explicit JSDoc tags to the hasHtmlOutsideCodeBlocks
utility: add `@param` describing the text argument (e.g. "text: Raw content that
may contain markdown, inline code, and/or HTML"), add `@returns` describing the
boolean result ("True when an HTML-like tag is detected outside code contexts"),
and include a brief `@example` showing usage (e.g.
hasHtmlOutsideCodeBlocks('```tsx\n<Navigate />\n```') // false). Place these
tags in the existing comment block above the function in content-renderer.ts so
the regex behavior contract is clear.
- Around line 62-69: The current hasHtmlOutsideCodeBlocks function uses fragile
regex that misses edge cases (multi-backtick inline spans, complex fences) and
causes false HTML detection; replace the fragile stripping with a proper
Markdown parse-and-inspect approach: parse the input with a Markdown parser
(e.g., remark/unified or any project-preferred markdown tokenizer), walk the AST
and ignore/skip nodes representing code (code, inlineCode, codeFence, etc.),
then search only the remaining text/HTML nodes for tags, or alternatively
enhance the logic in hasHtmlOutsideCodeBlocks to correctly handle multi-backtick
inline code spans and variable-length fences by using the parser’s token stream
to detect code spans rather than regex.
In `@mcp-server/src/core/direct-functions/move-task-cross-tag.js`:
- Around line 120-136: The code rebuilds a success message using
sourceIds.length which can differ from the actual moved set when dependencies
are included; instead use the message already produced by finalizeMove (i.e.,
use result.message) or, if you need to compute the count here, derive it from
result.movedTasks.length (or result.movedTasks || []).length to reflect the
actual moved tasks; update the block in move-task-cross-tag.js to stop
reconstructing the message and return result.message (or use
result.movedTasks.length) so the response is consistent with finalizeMove’s
logic.
In `@tests/integration/move-task-cross-tag.integration.test.js`:
- Around line 553-610: Add a new integration test that exercises dependency
remapping when renumbering by using moveTasksBetweenTags: create mock readJSON
data where Task A (id 1) depends on Task B (id 2), move both from sourceTag to
targetTag where id 2 conflicts and will be renumbered, call
moveTasksBetweenTags, then assert the result reports renumbering and the
writtenData (from mockUtils.writeJSON) shows Task B has the new ID and Task A's
dependencies array has been updated to reference Task B's new ID; reference
moveTasksBetweenTags, mockUtils.readJSON and mockUtils.writeJSON to locate where
to inject this scenario and validate dependency updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 227570ee-2c5e-4bd5-a634-d254db6dea80
📒 Files selected for processing (5)
apps/cli/src/utils/content-renderer.spec.tsapps/cli/src/utils/content-renderer.tsmcp-server/src/core/direct-functions/move-task-cross-tag.jsscripts/modules/task-manager/move-task.jstests/integration/move-task-cross-tag.integration.test.js
Comment on lines
+23
to
+39
| it('should not treat angle brackets inside fenced code blocks as HTML (issue #1562)', () => { | ||
| const input = | ||
| '## Implementation\\n\\n```tsx\\nexport function Component() {\\n return <Navigate to="/" replace />\\n}\\n```'; | ||
| const result = renderContent(input); | ||
| // The key assertion: the output should still contain "Implementation" as a | ||
| // recognizable section and should NOT collapse into a single mangled line. | ||
| expect(result).toContain('Implementation'); | ||
| // The code content should be preserved (Navigate component reference) | ||
| expect(result).toContain('Navigate'); | ||
| }); | ||
|
|
||
| it('should not treat angle brackets inside inline code as HTML', () => { | ||
| const input = 'Use the `<Navigate>` component for redirects.'; | ||
| const result = renderContent(input); | ||
| expect(result).toContain('Navigate'); | ||
| expect(result).toContain('redirects'); | ||
| }); |
Contributor
There was a problem hiding this comment.
Regression assertions are too weak to catch the original one-line-collapse bug.
These tests can still pass if output is mangled into one line, because they only check keyword presence. Please assert preserved structure and raw code content more directly.
Suggested assertion hardening
it('should not treat angle brackets inside fenced code blocks as HTML (issue `#1562`)', () => {
@@
const result = renderContent(input);
@@
expect(result).toContain('Implementation');
- // The code content should be preserved (Navigate component reference)
- expect(result).toContain('Navigate');
+ // Preserve code line content and multi-line structure
+ expect(result).toContain('<Navigate to="/" replace />');
+ expect(result.split('\n').length).toBeGreaterThan(4);
});
it('should not treat angle brackets inside inline code as HTML', () => {
@@
const result = renderContent(input);
- expect(result).toContain('Navigate');
+ expect(result).toContain('<Navigate>');
expect(result).toContain('redirects');
});
@@
it('should handle mixed markdown with code blocks containing multiple angle brackets', () => {
@@
const result = renderContent(input);
@@
expect(result).toContain('Current State');
expect(result).toContain('RequireGuest');
+ expect(result).toContain('<BrowserRouter>');
+ expect(result).toContain('<Route path="/" element={<Home />} />');
+ expect(result.split('\n').length).toBeGreaterThan(6);
});Also applies to: 49-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cli/src/utils/content-renderer.spec.ts` around lines 23 - 39, The
regression tests for renderContent are too weak because they only check keyword
presence; update the two tests ("should not treat angle brackets inside fenced
code blocks as HTML (issue `#1562`)" and "should not treat angle brackets inside
inline code as HTML") to assert preserved structure: call renderContent(input)
and assert that the output contains a code block or pre/code wrapper (e.g.,
check for '<pre', '<code', or a newline-delimited fenced block surrounding
'export function Component' and the '<Navigate' text) and that "Implementation"
appears followed by a newline or heading markup (e.g., 'Implementation' then
'\n' or '<h2' before the code), and for the inline case assert that the inline
code formatting is preserved (e.g., '`<Navigate>`' rendered as '<code' or
backticks preserved) so the tests fail if content is collapsed into a single
mangled line.
Comment on lines
+943
to
985
| // 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. | ||
| if (idRemapping.size > 0) { | ||
| for (const taskId of tasksToMove) { | ||
| const normalizedTaskId = | ||
| typeof taskId === 'string' ? parseInt(taskId, 10) : taskId; | ||
| // Find the task in the target tag (it may have been renumbered) | ||
| const finalId = idRemapping.get(normalizedTaskId) || normalizedTaskId; | ||
| const movedTask = rawData[targetTag].tasks.find( | ||
| (t) => t.id === finalId | ||
| ); | ||
| 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; | ||
| }); | ||
| } | ||
| // Also update subtask dependencies that reference remapped IDs | ||
| 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; | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Subtask dependency references may be incorrectly remapped.
When a dependency is in dotted format (e.g., "5.2" referencing subtask 2 of task 5), normalizeDependency extracts the parent ID (5). If task 5 was remapped to 6, the current code replaces the entire dependency with 6 instead of "6.2", losing the subtask reference.
🐛 Proposed fix to preserve subtask references
if (movedTask && Array.isArray(movedTask.dependencies)) {
movedTask.dependencies = movedTask.dependencies.map((dep) => {
const normalizedDep = normalizeDependency(dep);
if (
Number.isFinite(normalizedDep) &&
idRemapping.has(normalizedDep)
) {
+ // Preserve subtask reference if present
+ if (typeof dep === 'string' && dep.includes('.')) {
+ const [, subId] = dep.split('.');
+ return `${idRemapping.get(normalizedDep)}.${subId}`;
+ }
return idRemapping.get(normalizedDep);
}
return dep;
});
}
// Also update subtask dependencies that reference remapped IDs
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)
) {
+ // Preserve subtask reference if present
+ if (typeof dep === 'string' && dep.includes('.')) {
+ const [, subId] = dep.split('.');
+ return `${idRemapping.get(normalizedDep)}.${subId}`;
+ }
return idRemapping.get(normalizedDep);
}
return dep;
});
}
});
}🤖 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, The
dependency remapping loses subtask suffixes because normalizeDependency returns
the numeric parent ID and you replace the whole dep with
idRemapping.get(normalizedDep); update the remapping logic inside the
movedTask.dependencies and movedTask.subtasks[].dependencies map callbacks to:
extract the original dep string, detect and preserve any dotted suffix (e.g.,
".2"), look up the new parent ID via idRemapping.get(normalizedDep), and return
either the numeric new ID (if original was numeric) or the reconstructed dotted
string `${newParent}${suffix}` (if original had a suffix); apply this same
reconstruction for both dependency arrays referenced in the movedTask processing
(functions/variables: normalizeDependency, idRemapping, movedTask,
movedTask.subtasks).
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
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.
Summary
detailsfield contains angle brackets (<,>) inside fenced code blocks (e.g. JSX/TSX like<Navigate to="/" replace />)renderContent()used a naive/<[^>]+>/regex to detect HTML, which also matched angle brackets inside code blocks, incorrectly routing the content through TurndownService (HTML-to-Markdown converter) and corrupting the outputhasHtmlOutsideCodeBlocks()helper that strips fenced code blocks and inline code spans before testing for HTML tagsFixes #1562
Test plan
content-renderer.spec.tscovering:task-master show <id>on a task with JSX/TSX in thedetailsfield and confirm proper formatting🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features