feature: task-dnd-ux (2/3) - #1127
Open
myk1yt wants to merge 19 commits into
Open
Conversation
…ence - Add Zod-based type contracts in packages/types/src/task-organization.ts - Add TaskOrganizationStore with atomic read-modify-write via safeUpdateJson - Add safeUpdateJson helper to src/utils/safeWriteJson.ts - Add taskOrganization to GlobalFileNames - Export TaskOrganizationStore types from @roo-code/types - Add ExtensionMessage/WebviewMessage fields for task organization - 29 tests covering CRUD, folder management, pinning, and concurrency - Fix all no-explicit-any lint errors with proper type narrowing
…vider state assembly - Add taskOrganizationMessageHandler.ts: validates mutation requests via Zod, applies through TaskOrganizationStore, posts typed results to webview - Add taskOrganizationMessageHandler.spec.ts: 6 tests covering validation, success, store rejection, and unexpected error paths - Wire taskOrganizationMutation case in webviewMessageHandler.ts - Integrate TaskOrganizationStore into ClineProvider: constructor init, dispose, getTaskOrganizationStore() getter, reconcile on history writes, and taskOrganization state in getStateToPostToWebview()
- Add TaskOrganizationStore for atomic persistence - Add DnD controller and UI components with dnd-kit - Add folder creation and drag-drop composition - Add pin buttons with ErrorBoundary protection - Add selection mode folder actions and DeleteFoldersDialog - Convert to whole-card drag with interactive control guard - Add localization for DnD UX redesign features - Stabilize DnD components and Welcome screen integration
…nd folders Three bugs caused workspace A's tasks/pins/folders to leak into workspace B: 1. HistoryPreview passed undefined as cwd to buildGroupedOrganizationProjection, disabling workspace filtering entirely in the preview. 2. HistoryView's renderPinnedHeader iterated ALL organization.pins (global state) without workspace filtering. Pinned tasks from other workspaces displayed raw task IDs as labels (the 'encrypted numbers' symptom). 3. buildGroupedOrganizationProjection always included folder projections even when all members belonged to other workspaces, causing empty folders from workspace A to appear in workspace B. Fix: pass cwd to the projection in HistoryView, filter pins by workspace when showAllWorkspaces is false, and skip folders with no visible members when cwd is provided. Genuinely empty folders (zero taskIds) are preserved.
Distinguish cwd === undefined (show all workspaces) from cwd === empty string (no workspace open). Previously !cwd treated both identically, causing workspace-specific folders and pins to appear when no workspace was open. - isVisibleInWorkspace: !cwd → cwd === undefined - folder skip condition: cwd && ... → cwd !== undefined && ...
… role=button to SubtaskRow - DraggableTaskEntry deliberately strips role from dnd-kit attributes so the wrapper is not matched by interactive selectors; update the two tests to assert the actual contract (no role/aria-pressed, tabindex=0, aria-roledescription=draggable) instead of role=button. - SubtaskRow's keyboard-interactive row (tabIndex + Enter/Space handler) lacked role=button; add it for a11y correctness. Safe for TaskOrganizationPointerSensor since [role=button] is not in its INTERACTIVE_SELECTOR. Fixes 4 failing platform-unit-test specs on PR #31 CI (ubuntu+windows).
…r reloads - save(): reject writes whose base revision is already on disk (>= instead of >) so two processes computing next=N+1 from the same base cannot both commit; the second now fails with TASK_ORG/PERSISTENCE/005 instead of silently overwriting the first. - load(): keep the in-memory state on transient read errors (e.g. the directory watcher firing mid temp+rename) instead of resetting to empty, which previously made the next mutation compute from an empty aggregate. - reloadFromWatcher(): fire onChange whenever the reloaded aggregate differs in content, not only when the revision increases, so the victim of a same-revision lost update still gets its webview notified.
The TaskHistoryStore.onWrite closure dereferenced this.taskOrganizationStore, which is only assigned a few lines after the history store is constructed. A history write landing in that window threw a TypeError (caught and logged, reconcile skipped). Guard the dereference so the reconcile is skipped cleanly until the store exists.
# Conflicts: # src/core/task-persistence/__tests__/TaskOrganizationStore.spec.ts
… merges The dedicated taskOrganizationUpdated handler drops stale revisions, but the full-state merge path spread newRest unconditionally, so a state push assembled before a mutation commit could arrive after the broadcast and regress the webview to an older revision (folder/pin UI flickers back and the next DnD mutation then gets a spurious TASK_ORG/CONFLICT/002). Apply the same revision guard to the taskOrganization field in mergeExtensionState.
…ty-cwd semantics - HistoryView: folder pins were exempt from workspace filtering, so a folder whose members all belong to another workspace still rendered as a pinned shortcut in Current Workspace mode. Keep a folder pin only when the folder is visible in the workspace-scoped projection (at least one visible member, or genuinely empty), matching buildGroupedOrganizationProjection. - taskOrganizationModel: filterByWorkspace treated cwd === "" as unfiltered, contradicting the documented "no workspace open" semantics. cwd === undefined is now the only unfiltered mode; "" filters to tasks without a workspace, matching buildGroupedOrganizationProjection.
myk1yt
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
August 4, 2026 11:47
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesTask organization contracts, persistence, folder and pin mutations, history reconciliation, file watching, and webview integration are added. Task organization contracts
Persistence and mutations
Provider and webview integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Webview
participant WebviewMessageHandler
participant ClineProvider
participant TaskOrganizationStore
participant JSONFile
Webview->>WebviewMessageHandler: taskOrganizationMutation
WebviewMessageHandler->>ClineProvider: handleTaskOrganizationMessage
ClineProvider->>TaskOrganizationStore: mutate(request)
TaskOrganizationStore->>JSONFile: locked read-modify-write
JSONFile-->>TaskOrganizationStore: committed state and revision
TaskOrganizationStore-->>ClineProvider: mutation result
ClineProvider-->>Webview: taskOrganizationMutationResult
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![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: 8
🧹 Nitpick comments (1)
src/core/webview/taskOrganizationMessageHandler.ts (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a narrow provider contract for the handler.
handleTaskOrganizationMessageuses onlylog,postMessageToWebview, andgetTaskOrganizationStore. Its concreteClineProviderparameter forces the tests to hide incomplete doubles withas unknown as ClineProvider.
src/core/webview/taskOrganizationMessageHandler.ts#L19-L19: accept a handler-specific provider interface with only the required provider and store methods.src/core/webview/__tests__/taskOrganizationMessageHandler.spec.ts#L11-L26: return a typed provider double instead of using a double assertion.src/core/webview/__tests__/taskOrganizationMessageHandler.spec.ts#L237-L245: type the rejecting-store fixture against the same narrow interface.As per coding guidelines: “Use double assertions only as a last resort and explain them with a comment.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/webview/taskOrganizationMessageHandler.ts` at line 19, The handler is coupled to the concrete ClineProvider despite using only a small subset of its API. In src/core/webview/taskOrganizationMessageHandler.ts lines 19-19, define and accept a narrow handler-specific provider interface containing log, postMessageToWebview, and getTaskOrganizationStore; in src/core/webview/__tests__/taskOrganizationMessageHandler.spec.ts lines 11-26, type the provider double against that interface without a double assertion; and in lines 237-245, type the rejecting-store fixture against the same interface.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/types/src/task-organization.ts`:
- Around line 68-76: Detect future schema versions using a minimal
schema-version envelope before applying version-1 validation. In
packages/types/src/task-organization.ts#L68-L76, expose the envelope separately
from taskOrganizationStateSchema; in
src/core/task-persistence/TaskOrganizationStore.ts#L278-L303, parse that
envelope first and handle schemaVersion > 1 before validating the full
aggregate; in
src/core/task-persistence/__tests__/TaskOrganizationStore.spec.ts#L128-L152, add
a fixture containing a future version without version-1 fields and verify it
follows the future-version path rather than being classified as corrupt.
In `@src/core/task-persistence/TaskOrganizationStore.ts`:
- Around line 171-180: Preserve a non-empty request ID in every mutation result:
update TaskOrganizationStore.mutate and its handler call path to receive and
propagate the request ID separately from TaskOrganizationMutationV1, then update
the request ID schema in packages/types/src/task-organization.ts (lines 134-138)
to require at least one character with z.string().min(1).
- Around line 635-641: The parent traversal at
src/core/task-persistence/TaskOrganizationStore.ts lines 635-641 must track
visited parent IDs and stop or reject traversal when an ID repeats. The
descendant traversal at src/core/task-persistence/TaskOrganizationStore.ts lines
699-708 must track visited descendant IDs before pushing children onto the
stack, preventing cyclic task history from causing infinite synchronous loops.
- Around line 842-875: In TaskOrganizationStore.ts lines 842-875, update the
watcher startup flow to await fs.mkdir(tasksDir, { recursive: true }) before
calling fsSync.watch, while preserving the existing disposed checks and error
handling. In src/core/task-persistence/__tests__/TaskOrganizationStore.spec.ts
lines 762-849, add coverage using an empty temporary directory that verifies a
real watcher notification is received after initialization.
- Around line 345-351: In src/core/task-persistence/TaskOrganizationStore.ts at
lines 345-351, after successfully writing the quarantine copy, delete or unlink
the original malformed file at the filePath to prevent subsequent safeUpdateJson
calls from attempting to parse the corrupted JSON. In
src/core/task-persistence/__tests__/TaskOrganizationStore.spec.ts at lines
114-126, add a test case that mutates the recovered store after quarantine and
verifies that the persistence succeeds, ensuring the corruption recovery path
does not block future mutations.
In `@src/core/webview/__tests__/taskOrganizationMessageHandler.spec.ts`:
- Around line 86-97: Update the malformed-input fixture in
taskOrganizationMessageHandler tests to construct a properly typed
WebviewMessage with a valid createFolder mutation, then remove a required member
such as destination before invoking the handler. Eliminate the any cast and
eslint suppression while preserving the runtime-validation scenario.
In `@src/core/webview/taskOrganizationMessageHandler.ts`:
- Around line 48-56: Ensure the nested
TaskOrganizationMutationResultV1.requestId matches request.requestId in the
handler’s mutate-and-post flow. Since TaskOrganizationStore.mutate currently
derives the ID from request.mutation, pass the validated request ID through that
mutation input or explicitly assign it to the returned result before
constructing the webview message, and add coverage for a mutate result with an
empty requestId.
In `@src/utils/safeWriteJson.ts`:
- Around line 360-390: The code currently unlinks actualTempBackupFilePath in
the final cleanup block even when the rollback (fs.rename) fails, destroying the
only remaining copy of the file. Track whether the rollback attempt failed by
introducing a flag, and skip the unlink of actualTempBackupFilePath at lines
382-390 if the rollback failed. This preserves the backup file as a recoverable
copy when the restore operation fails.
---
Nitpick comments:
In `@src/core/webview/taskOrganizationMessageHandler.ts`:
- Line 19: The handler is coupled to the concrete ClineProvider despite using
only a small subset of its API. In
src/core/webview/taskOrganizationMessageHandler.ts lines 19-19, define and
accept a narrow handler-specific provider interface containing log,
postMessageToWebview, and getTaskOrganizationStore; in
src/core/webview/__tests__/taskOrganizationMessageHandler.spec.ts lines 11-26,
type the provider double against that interface without a double assertion; and
in lines 237-245, type the rejecting-store fixture against the same interface.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ae66a04-f5d9-4876-82a7-3eef43d1c584
📒 Files selected for processing (14)
packages/types/src/index.tspackages/types/src/task-organization.tspackages/types/src/vscode-extension-host.tssrc/core/task-persistence/TaskOrganizationStore.tssrc/core/task-persistence/__tests__/TaskOrganizationStore.spec.tssrc/core/task-persistence/index.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.taskHistory.spec.tssrc/core/webview/__tests__/taskOrganizationMessageHandler.spec.tssrc/core/webview/taskOrganizationMessageHandler.tssrc/core/webview/webviewMessageHandler.tssrc/eslint-suppressions.jsonsrc/shared/globalFileNames.tssrc/utils/safeWriteJson.ts
Comment on lines
+86
to
+97
| const message: WebviewMessage = { | ||
| type: "taskOrganizationMutation", | ||
| taskOrganizationMutation: { | ||
| requestId: "req-bad", | ||
| baseRevision: 0, | ||
| mutation: { | ||
| kind: "createFolder", | ||
| // Missing required fields | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| } as any, | ||
| }, | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant test file and type definitions/usages without running repository code.
fd -a 'taskOrganizationMessageHandler.spec.ts' . | sed 's#^\./##'
fd -a 'taskOrganizationMessageHandler' . | sed 's#^\./##'
file="$(fd 'taskOrganizationMessageHandler.spec.ts' . | head -n 1)"
echo "FILE=$file"
wc -l "$file"
sed -n '1,140p' "$file" | nl -ba
echo "--- handler outline ---"
handler="$(fd 'taskOrganizationMessageHandler' . | head -n 1)"
echo "HANDLER=$handler"
ast-grep outline "$handler" --view expanded 2>/dev/null | sed -n '1,180p' || true
echo "--- relevant source lines ---"
sed -n '1,180p' "$handler" | nl -ba
echo "--- WebviewMessage/createFolder usages ---"
rg -n "type WebviewMessage|interface WebviewMessage|createFolder|TaskOrganization|mutation" . -g '*.ts' -g '*.tsx' | sed -n '1,240p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 585
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Use available tools to inspect context while avoiding dependency on unavailable commands.
file="./src/core/webview/__tests__/taskOrganizationMessageHandler.spec.ts"
handler="./src/core/webview/taskOrganizationMessageHandler.ts"
echo "TEST_FILE=$file"
wc -l "$file" "$handler"
echo
echo "--- test file lines 1-140 ---"
awk 'NR>=1 && NR<=140 { printf("%6d %s\n", NR, $0) }' "$file"
echo
echo "--- handler outline ---"
ast-grep outline "$handler" --view expanded 2>/dev/null | sed -n '1,180p' || true
echo
echo "--- handler lines 1-180 ---"
awk 'NR>=1 && NR<=180 { printf("%6d %s\n", NR, $0) }' "$handler"
echo
echo "--- type/message/case usages ---"
rg -n "type WebviewMessage|interface WebviewMessage|createFolder|TaskOrganization|Task-Organization|taskOrganization|mutation" . -g '*.ts' -g '*.tsx' | sed -n '1,320p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 45992
Create malformed input without suppressing ESLint.
Lines 94-95 suppress @typescript-eslint/no-explicit-any and cast the mutation to any. Build a valid WebviewMessage, then remove a required createFolder member such as destination before calling the handler. This keeps the fixture typed and still exercises Zod runtime validation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/webview/__tests__/taskOrganizationMessageHandler.spec.ts` around
lines 86 - 97, Update the malformed-input fixture in
taskOrganizationMessageHandler tests to construct a properly typed
WebviewMessage with a valid createFolder mutation, then remove a required member
such as destination before invoking the handler. Eliminate the any cast and
eslint suppression while preserving the runtime-validation scenario.
Source: Coding guidelines
Comment on lines
+48
to
+56
| try { | ||
| const store = provider.getTaskOrganizationStore() | ||
| const result: TaskOrganizationMutationResultV1 = await store.mutate(request.mutation, request.baseRevision) | ||
|
|
||
| await provider.postMessageToWebview({ | ||
| type: "taskOrganizationMutationResult", | ||
| requestId: request.requestId, | ||
| taskOrganizationMutationResult: result, | ||
| } satisfies Partial<ExtensionMessage>) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the nested result requestId equal to the request ID.
Line 50 passes only request.mutation to TaskOrganizationStore.mutate(). The store derives TaskOrganizationMutationResultV1.requestId from that argument, but the validated request keeps requestId outside mutation. Real store results therefore contain requestId: "", while only the outer message has the correct ID. A consumer of taskOrganizationMutationResult.requestId cannot correlate concurrent responses.
Set the nested result ID at this boundary, or add it to the store method contract. Add a test where mutate() returns an empty result ID.
Based on the supplied src/core/task-persistence/TaskOrganizationStore.ts context, mutate() reads the result ID only from its mutation parameter.
Proposed fix
const store = provider.getTaskOrganizationStore()
- const result: TaskOrganizationMutationResultV1 = await store.mutate(request.mutation, request.baseRevision)
+ const result: TaskOrganizationMutationResultV1 = {
+ ...(await store.mutate(request.mutation, request.baseRevision)),
+ requestId: request.requestId,
+ }📝 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
| try { | |
| const store = provider.getTaskOrganizationStore() | |
| const result: TaskOrganizationMutationResultV1 = await store.mutate(request.mutation, request.baseRevision) | |
| await provider.postMessageToWebview({ | |
| type: "taskOrganizationMutationResult", | |
| requestId: request.requestId, | |
| taskOrganizationMutationResult: result, | |
| } satisfies Partial<ExtensionMessage>) | |
| try { | |
| const store = provider.getTaskOrganizationStore() | |
| const result: TaskOrganizationMutationResultV1 = { | |
| ...(await store.mutate(request.mutation, request.baseRevision)), | |
| requestId: request.requestId, | |
| } | |
| await provider.postMessageToWebview({ | |
| type: "taskOrganizationMutationResult", | |
| requestId: request.requestId, | |
| taskOrganizationMutationResult: result, | |
| } satisfies Partial<ExtensionMessage>) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/webview/taskOrganizationMessageHandler.ts` around lines 48 - 56,
Ensure the nested TaskOrganizationMutationResultV1.requestId matches
request.requestId in the handler’s mutate-and-post flow. Since
TaskOrganizationStore.mutate currently derives the ID from request.mutation,
pass the validated request ID through that mutation input or explicitly assign
it to the returned result before constructing the webview message, and add
coverage for a mutate result with an empty requestId.
Comment on lines
+360
to
+390
| if (backupFileToRollbackOrCleanupWithinCatch) { | ||
| try { | ||
| await fs.rename(backupFileToRollbackOrCleanupWithinCatch, absoluteFilePath) | ||
| actualTempBackupFilePath = null | ||
| } catch (rollbackError) { | ||
| console.error( | ||
| `[Catch] Failed to restore backup ${backupFileToRollbackOrCleanupWithinCatch} to ${absoluteFilePath}:`, | ||
| rollbackError, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| if (newFileToCleanupWithinCatch) { | ||
| try { | ||
| await fs.unlink(newFileToCleanupWithinCatch) | ||
| } catch (cleanupError) { | ||
| console.error( | ||
| `[Catch] Failed to clean up temporary new file ${newFileToCleanupWithinCatch}:`, | ||
| cleanupError, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| if (actualTempBackupFilePath) { | ||
| try { | ||
| await fs.unlink(actualTempBackupFilePath) | ||
| } catch (cleanupError) { | ||
| console.error( | ||
| `[Catch] Failed to clean up temporary backup file ${actualTempBackupFilePath}:`, | ||
| cleanupError, | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Keep the backup when rollback fails.
If fs.rename at Line 362 fails, actualTempBackupFilePath remains the only known-good copy. Lines 383-390 then delete that backup. The target file can remain absent with no recoverable copy.
Do not unlink the backup after a failed rollback.
Proposed fix
if (actualTempBackupFilePath) {
- try {
- await fs.unlink(actualTempBackupFilePath)
- } catch (cleanupError) {
- console.error(
- `[Catch] Failed to clean up temporary backup file ${actualTempBackupFilePath}:`,
- cleanupError,
- )
- }
+ console.error(
+ `[Catch] Preserving backup after rollback failure: ${actualTempBackupFilePath}`,
+ )
}📝 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
| if (backupFileToRollbackOrCleanupWithinCatch) { | |
| try { | |
| await fs.rename(backupFileToRollbackOrCleanupWithinCatch, absoluteFilePath) | |
| actualTempBackupFilePath = null | |
| } catch (rollbackError) { | |
| console.error( | |
| `[Catch] Failed to restore backup ${backupFileToRollbackOrCleanupWithinCatch} to ${absoluteFilePath}:`, | |
| rollbackError, | |
| ) | |
| } | |
| } | |
| if (newFileToCleanupWithinCatch) { | |
| try { | |
| await fs.unlink(newFileToCleanupWithinCatch) | |
| } catch (cleanupError) { | |
| console.error( | |
| `[Catch] Failed to clean up temporary new file ${newFileToCleanupWithinCatch}:`, | |
| cleanupError, | |
| ) | |
| } | |
| } | |
| if (actualTempBackupFilePath) { | |
| try { | |
| await fs.unlink(actualTempBackupFilePath) | |
| } catch (cleanupError) { | |
| console.error( | |
| `[Catch] Failed to clean up temporary backup file ${actualTempBackupFilePath}:`, | |
| cleanupError, | |
| ) | |
| if (backupFileToRollbackOrCleanupWithinCatch) { | |
| try { | |
| await fs.rename(backupFileToRollbackOrCleanupWithinCatch, absoluteFilePath) | |
| actualTempBackupFilePath = null | |
| } catch (rollbackError) { | |
| console.error( | |
| `[Catch] Failed to restore backup ${backupFileToRollbackOrCleanupWithinCatch} to ${absoluteFilePath}:`, | |
| rollbackError, | |
| ) | |
| } | |
| } | |
| if (newFileToCleanupWithinCatch) { | |
| try { | |
| await fs.unlink(newFileToCleanupWithinCatch) | |
| } catch (cleanupError) { | |
| console.error( | |
| `[Catch] Failed to clean up temporary new file ${newFileToCleanupWithinCatch}:`, | |
| cleanupError, | |
| ) | |
| } | |
| } | |
| if (actualTempBackupFilePath) { | |
| console.error( | |
| `[Catch] Preserving backup after rollback failure: ${actualTempBackupFilePath}`, | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/safeWriteJson.ts` around lines 360 - 390, The code currently
unlinks actualTempBackupFilePath in the final cleanup block even when the
rollback (fs.rename) fails, destroying the only remaining copy of the file.
Track whether the rollback attempt failed by introducing a flag, and skip the
unlink of actualTempBackupFilePath at lines 382-390 if the rollback failed. This
preserves the backup file as a recoverable copy when the restore operation
fails.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@codecov.yml`:
- Line 1: Normalize the line endings of codecov.yml to LF throughout the file,
without changing its YAML content.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| comment: | ||
| layout: "diff, flags, components" | ||
| behavior: default | ||
| coverage: |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use LF line endings.
YAMLlint rejects this file because it uses CRLF line endings. Normalize codecov.yml to LF before merge.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@codecov.yml` at line 1, Normalize the line endings of codecov.yml to LF
throughout the file, without changing its YAML content.
Source: Linters/SAST tools
github-actions
Bot
added
the
awaiting-review
myk1yt
force-pushed
the
pr/b09-task-org-ipc-v2
branch
from
August 5, 2026 02:34
77e7207 to
e482208
Compare
github-actions
Bot
removed
the
awaiting-review
myk1yt
force-pushed
the
pr/b09-task-org-ipc-v2
branch
from
August 5, 2026 08:42
bf84cc1 to
044753a
Compare
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.
Stack Position
feature/task-dnd-uxDescription
https://youtube.com/shorts/6kx-bNScYew?feature=share
Full Feature Description
feature/task-dnd-uxtask-organization.ts,TaskOrganizationStore.ts,safeWriteJson.ts,taskOrganizationMessageHandler.ts,ClineProvider.ts,HistoryView.tsx,ExtensionStateContext.tsx.Why Split Into 17 PRs
Instead of submitting this feature as a single unified PR, it was split into individual PRs because as code size grows, safely reviewing a PR becomes very difficult. The feature was broken into mutually exclusive individual PRs so that each can be reviewed independently.
What This PR Specifically Changes
Validates typed create/rename/move/pin/reorder/delete requests, passes them to the store, and publishes success/recoverable error and authoritative extension state to the webview. Does not include UI components.
Included Files
src/core/webview/taskOrganizationMessageHandler.tssrc/core/webview/webviewMessageHandler.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/taskOrganizationMessageHandler.spec.tsExclusion Scope
Summary by CodeRabbit
New Features
Reliability