feature: task-dnd-ux (3/3) - #1129
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.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a versioned task-organization data model shared between the extension host and webview, a persistent ChangesTask organization feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant HistoryView
participant ExtensionStateContext
participant ClineProvider
participant TaskOrganizationStore
HistoryView->>ExtensionStateContext: mutateTaskOrganization
ExtensionStateContext->>ClineProvider: taskOrganizationMutation
ClineProvider->>TaskOrganizationStore: mutate with base revision
TaskOrganizationStore-->>ClineProvider: typed mutation result
ClineProvider-->>ExtensionStateContext: taskOrganizationMutationResult
ExtensionStateContext-->>HistoryView: resolved mutation promise and snapshot
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (29)
docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md-58-58 (1)
58-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the wording in the failure analysis.
Use
terminal, shell, and command-execution testsat Line 58. Use1 msinstead of1msat Lines 66 and 228.Also applies to: 66-66, 228-228
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md` at line 58, Update the failure-analysis wording to say “terminal, shell, and command-execution tests” instead of “terminal/shell/command execution related tests,” and format both occurrences of the duration as “1 ms” rather than “1ms” at the referenced lines.Source: Linters/SAST tools
docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md-124-134 (1)
124-134: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the short-range breakdown finding to match the later fix.
docs/260730_0002_session_dashboard-crash-debug/context-save-260730-2333.mdLine 11 records commit0769ccea7, and Line 51 records the daily-rollup fix as completed. This report still presents the monthly-rollup problem as unresolved and retains it as a release condition. Mark the finding as resolved or clearly label this report as a pre-fix snapshot.Also applies to: 171-183
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md` around lines 124 - 134, Update the short-range breakdown finding in the report, including the repeated section around the later referenced lines, to reflect that the daily-rollup fix is completed. Mark the monthly-rollup issue as resolved and remove it as an outstanding release condition, or clearly label the report as a pre-fix snapshot while preserving the recorded commit references.docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md-136-144 (1)
136-144: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winQualify the claim that the crash is eliminated.
Lines 138-144 state that
cacheRatio > 0still uses the full event scan and can retain the crash vector. Lines 165-167 then state that the crash is eliminated. Replace the absolute claim with a statement limited to the default fast-path query.Also applies to: 163-167
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md` around lines 136 - 144, The document states in lines 138-144 that cacheRatio > 0 scenarios still use full event scans and retain the crash vector, but then makes an absolute claim in lines 165-167 that the crash is eliminated. Update the crash-elimination claim in lines 165-167 to qualify it as only applying to the default fast-path query configuration (where cacheRatio is 0 or undefined), making clear that the limitation described in Inquiry 2 means the crash vector persists for users who enable cacheRatio.docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md-80-80 (1)
80-80: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to the code fences.
Use
textfor the three branch-list fences at Lines 80, 125, and 160. This resolves the reported Markdownlint MD040 warnings.Also applies to: 125-125, 160-160
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md` at line 80, The three branch-list code fences in this markdown file are missing language identifiers, which triggers Markdownlint MD040 warnings. Add the language identifier `text` to each of the three code fence opening markers for the branch-list sections. This ensures each code fence declaration includes a language specifier, resolving the linting violations.Source: Linters/SAST tools
docs/260730_0002_session_dashboard-crash-debug/test-mimo-parallel.txt-1-133 (1)
1-133: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStore a normalized, reviewable test report.
As committed, this file contains NUL-padded terminal output and ANSI escape sequences. Common tools can treat it as binary, and the report is difficult to read or search. Re-export it as UTF-8 plain text with terminal control codes removed, or commit a concise report with the command, exit status, platform, and test counts.
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/test-mimo-parallel.txt` around lines 1 - 133, Replace the raw terminal capture in the test report with UTF-8 plain text by removing NUL padding and ANSI escape sequences. Prefer a concise, reviewable report that preserves the test command, exit status, platform, and final test counts.docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt-1-2 (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winNormalize the captured output before committing it.
The file contains NUL bytes and terminal ANSI escape sequences. Standard viewers and repository search display corrupted content. Re-capture or convert the output to UTF-8, strip terminal control codes, and retain only readable log content.
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt` around lines 1 - 2, Normalize the captured output in test-strict-reasoning.txt before committing it: convert the file to UTF-8, remove NUL bytes and terminal ANSI escape sequences, and retain only readable log content.docs/260803_0002_session_6-branch-bug-fix-verification/170924_code-report.md-3-7 (1)
3-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the relative links to source files.
This report is under
docs/260803_0002_session_6-branch-bug-fix-verification/. Therefore,../src/...resolves todocs/src/..., not the repositorysrc/...directory. Change the affected links on Line 3, Line 6, Line 7, Line 15, Line 16, Line 26, and Line 27 to use../../src/....Also applies to: 15-16, 26-27
🤖 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 `@docs/260803_0002_session_6-branch-bug-fix-verification/170924_code-report.md` around lines 3 - 7, Update the affected relative source links in the report, including references to TaskOrganizationStore.ts, TaskOrganizationStore.spec.ts, withLock(), mutate(), resolveUnit(), and resolveTaskClosure(), from ../src/... to ../../src/... so they resolve from the document’s directory to the repository src directory.webview-ui/src/components/history/TaskOrganizationDndSurface.tsx-72-74 (1)
72-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not discard a typed folder name on every revision change.
This effect clears
pendingFolderDraftwheneverorganization.revisionchanges. The drag that opened the dialog is not the only source of revision changes: a concurrentmoveToFolder, a pin toggle, or a mutation from another view also bumps it. The folder-name dialog then closes and the typed name is lost with no message. Restrict the cancellation to the case where the draft source or destination no longer exists, or keep the dialog open and revalidate on confirm.🤖 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 `@webview-ui/src/components/history/TaskOrganizationDndSurface.tsx` around lines 72 - 74, Update the useEffect watching organization.revision so it does not unconditionally clear pendingFolderDraft on unrelated revisions. Only cancel the draft when its source or destination folder no longer exists, or otherwise keep the dialog open and revalidate those references during confirmation.webview-ui/src/components/history/ManualFolderItem.tsx-316-319 (1)
316-319: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable the member drop target during selection mode.
ManualFolderItemdisables its folder drop target whenisSelectionModeis true, butManualFolderMemberItemregisters a member drop target withoutdisabled. Pass the current mode throughHistoryPreviewInnerand setdisabled: isSelectionModeon the member droppable so drops do not create folders during selection mode.🤖 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 `@webview-ui/src/components/history/ManualFolderItem.tsx` around lines 316 - 319, Update ManualFolderMemberItem’s useDroppable configuration to accept the isSelectionMode value passed through HistoryPreviewInner and set disabled to that value, matching the existing folder drop-target behavior so member drops cannot create folders during selection mode.webview-ui/src/components/history/TaskOrganizationInteractionContext.tsx-60-69 (1)
60-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReconcile the two
targetKeyimplementations.
buildGroupDndData()emitsautoGrouptargets from pinned projection rows, but the UI callsisPinned()/togglePin()with equivalenttasktargets for task groups. This localtargetKeymaps those same units to different keys, so a task group can render as pinned and remain incanPinafter the task-unit pin is removed. Use one shared canonical helper for task andautoGrouppins.🤖 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 `@webview-ui/src/components/history/TaskOrganizationInteractionContext.tsx` around lines 60 - 69, Update the targetKey logic used by buildGroupDndData, isPinned, and togglePin so task and autoGroup targets for the same task unit resolve to the same canonical key; reuse the existing shared helper if available rather than maintaining a separate local mapping. Preserve distinct keys for folder targets and ensure pin removal updates canPin consistently.webview-ui/src/components/history/taskOrganizationModel.ts-635-647 (1)
635-647: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a single child-relationship source for workspace filtering.
buildFlattenedVirtualEntriesusesparentTaskIdthroughchildrenMap, butbuildGroupedOrganizationProjectionusestask.childIdsinisVisibleInWorkspace.childIdsis optional onHistoryItemand is not reliably written alongsideparentTaskId, so a group can hide even when one of its descendants belongs to the current workload. Switch this path to the sameparentTaskId/children-map source.🤖 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 `@webview-ui/src/components/history/taskOrganizationModel.ts` around lines 635 - 647, Update isVisibleInWorkspace to collect descendant task IDs using the parentTaskId-derived childrenMap, matching buildFlattenedVirtualEntries, instead of relying on task.childIds. Preserve the existing root fallback and taskBelongsToWorkspace checks so descendants in the current workspace keep the group visible.webview-ui/src/components/history/SubtaskRow.tsx-113-116 (1)
113-116: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUnwired pin props in two leaf components. Both files gained pin props and a
PinButtonbranch, but their parents never pass those props, so both branches are unreachable in the current tree. Decide one owner for the pin control per card and wire or remove accordingly.
webview-ui/src/components/history/SubtaskRow.tsx#L113-L116: passshowPin,isPinned,canPin, andonTogglePinfromTaskGroupItem.tsxLine 101, or delete the props and thePinButtonbranch at Lines 76-84.webview-ui/src/components/history/TaskItemFooter.tsx#L71-L79: pass the pin props fromTaskItem.tsxLines 136-142, or delete this branch and keep the header pin control inTaskItem.🤖 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 `@webview-ui/src/components/history/SubtaskRow.tsx` around lines 113 - 116, Choose one pin-control owner per card and ensure the other leaf branch is removed or wired. In webview-ui/src/components/history/SubtaskRow.tsx:113-116, update TaskGroupItem.tsx:101 to pass showPin, isPinned, canPin, and onTogglePin, or remove those props and the PinButton branch at SubtaskRow.tsx:76-84. In webview-ui/src/components/history/TaskItemFooter.tsx:71-79, either pass the pin props from TaskItem.tsx:136-142 or remove this branch while retaining TaskItem’s header pin control.webview-ui/src/components/history/TaskOrganizationErrorBoundary.tsx-13-20 (1)
13-20: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the doc comment.
Lines 18-19 state that the boundary "renders children as-is".
renderat Lines 39-41 returnsthis.props.fallback ?? nullafter an error, so the children are unmounted. Align the comment with the behavior.♻️ Proposed change
- * On error the boundary logs a warning and renders children as-is (i.e. the - * new feature is silently disabled rather than crashing the whole view). + * On error the boundary logs the error, unmounts the failing subtree, and + * renders the provided fallback (or nothing) instead of crashing the view.🤖 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 `@webview-ui/src/components/history/TaskOrganizationErrorBoundary.tsx` around lines 13 - 20, Update the documentation comment for TaskOrganizationErrorBoundary to state that it renders the configured fallback, or null when no fallback is provided, after an error; remove the claim that it renders children as-is or leaves the existing view mounted.webview-ui/src/components/history/HistoryView.tsx-231-252 (1)
231-252: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe folder targets in
handleConfirmSelectionFolderNameare unreachable.
canCreateFolderFromSelectionat Line 231 requiresselectedFolderIds.length === 0.handleCreateFolderFromSelectionreturns early when that flag is false, so the dialog only opens with zero selected folders. TheselectedFolderIds.map(...)spread at Line 242 therefore always produces an empty list, and the comment at Line 228 ("tasks/groups and/or folders combined") does not match the gate.Decide the intended behavior. If folders must never join a new folder, remove the dead spread and correct the comment. If folders may join, relax the gate.
♻️ Proposed change if folders must be excluded
- // Create Folder is enabled when at least two distinct canonical units are - // selected (tasks/groups and/or folders combined). + // Create Folder is enabled when at least two distinct canonical task units + // are selected. Folder selection disables it. // Architect spec Section 1.6: create-folder requires at least two canonical // task units and is disabled while any folder is selected. const canCreateFolderFromSelection = selectedTaskTargets.length >= 2 && selectedFolderIds.length === 0const handleConfirmSelectionFolderName = useCallback( (name: string) => { - const targets: TaskOrganizationTargetV1[] = [ - ...selectedTaskTargets, - ...selectedFolderIds.map((folderId) => ({ kind: "folder", folderId }) as TaskOrganizationTargetV1), - ] - void createFolderFromSelection(name, targets).then((result) => { + void createFolderFromSelection(name, selectedTaskTargets).then((result) => { if (result.success) { setSelectedTaskIds([]) setSelectedFolderIds([]) } }) }, - [selectedTaskTargets, selectedFolderIds, createFolderFromSelection], + [selectedTaskTargets, createFolderFromSelection], )🤖 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 `@webview-ui/src/components/history/HistoryView.tsx` around lines 231 - 252, Resolve the inconsistency between canCreateFolderFromSelection and handleConfirmSelectionFolderName: if selected folders are not allowed, remove the selectedFolderIds target mapping and update the nearby comment to describe task/group-only selection; otherwise, relax the canCreateFolderFromSelection guard so folder selections can reach the dialog and retain their targets.src/core/task-persistence/TaskOrganizationStore.ts-842-875 (1)
842-875: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winThe watcher never recovers if the tasks directory is missing.
fsSync.watchthrowsENOENTwhentasksDirdoes not exist. On a fresh profile the store loads an empty state and writes nothing until the first mutation, so the directory can be absent atinitialize()time. Thecatchat Line 869 logs the failure, and no later attempt starts the watcher. Cross-instance reloads then stay disabled for the whole session.Create the directory before watching, or retry after the first successful save.
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 842 - 875, The watcher setup around getTasksDir and fsSync.watch must handle a missing tasks directory instead of permanently stopping after ENOENT. Ensure the directory is created before calling fsSync.watch, or trigger a retry after the first successful save, while preserving the existing disposed checks and watcher behavior.webview-ui/src/components/history/__tests__/DeleteFoldersDialog.spec.tsx-4-41 (1)
4-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the localized count and cancel close behavior.
The translation mock returns raw keys. The rendering test does not verify the folder count. The cancel test does not verify
onOpenChange(false).Return representative localized strings from
t. Assert the interpolated count. Pass a spy toonOpenChangein the cancel test and assert that it receivesfalse.🤖 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 `@webview-ui/src/components/history/__tests__/DeleteFoldersDialog.spec.tsx` around lines 4 - 41, Update the useAppTranslation mock in DeleteFoldersDialog tests to return representative localized strings with count interpolation, then assert the rendered confirmation text includes the folder count. In the cancel test, pass a spy as onOpenChange and verify it is called with false while preserving the existing onConfirm assertion.webview-ui/src/i18n/locales/pl/history.json-51-64 (1)
51-64: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new task-organization strings in each locale.
These values render in English for non-English users.
webview-ui/src/i18n/locales/pl/history.json#L51-L64: Replace the English values with Polish translations.webview-ui/src/i18n/locales/pt-BR/history.json#L51-L64: Replace the English values with Brazilian Portuguese translations.webview-ui/src/i18n/locales/ru/history.json#L51-L64: Replace the English values with Russian translations.webview-ui/src/i18n/locales/tr/history.json#L51-L64: Replace the English values with Turkish translations.🤖 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 `@webview-ui/src/i18n/locales/pl/history.json` around lines 51 - 64, Translate the new task-organization strings, preserving the existing keys and interpolation syntax, in webview-ui/src/i18n/locales/pl/history.json lines 51-64 (Polish), webview-ui/src/i18n/locales/pt-BR/history.json lines 51-64 (Brazilian Portuguese), webview-ui/src/i18n/locales/ru/history.json lines 51-64 (Russian), and webview-ui/src/i18n/locales/tr/history.json lines 51-64 (Turkish); replace each English value with its appropriate locale translation.webview-ui/src/i18n/locales/it/history.json-51-64 (1)
51-64: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the English history strings.
These locale bundles display English folder controls to Italian, Japanese, and Dutch users.
webview-ui/src/i18n/locales/it/history.json#L51-L64: Replace the English values with Italian translations.webview-ui/src/i18n/locales/ja/history.json#L51-L64: Replace the English values with Japanese translations.webview-ui/src/i18n/locales/nl/history.json#L51-L64: Replace the English values with Dutch translations.🤖 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 `@webview-ui/src/i18n/locales/it/history.json` around lines 51 - 64, The locale bundle files contain English strings instead of translations for Italian, Japanese, and Dutch users. Update webview-ui/src/i18n/locales/it/history.json lines 51-64 to replace all English string values (newFolder, folderNamePlaceholder, renameFolder, removeFromFolder, deleteEmptyFolder, pin, unpin, pinLimitReached, pinned, folder, tasks, unfiled, dragToOrganize, dropHereToRemove) with Italian translations. Apply the same transformation at webview-ui/src/i18n/locales/ja/history.json lines 51-64 with Japanese translations. Apply the same transformation at webview-ui/src/i18n/locales/nl/history.json lines 51-64 with Dutch translations. Keep all JSON keys unchanged; only update the string values to their target-language equivalents.webview-ui/src/i18n/locales/es/history.json-58-71 (1)
58-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new history labels.
These locale bundles still show English for new folder, pin, and drag-and-drop labels. Translate all added English values so the new workflow remains localized.
webview-ui/src/i18n/locales/es/history.json#L58-L71: Translate the English history labels to Spanish.webview-ui/src/i18n/locales/fr/history.json#L58-L71: Translate the English history labels to French.webview-ui/src/i18n/locales/hi/history.json#L51-L64: Translate the English history labels to Hindi.webview-ui/src/i18n/locales/id/history.json#L60-L73: Translate the English history labels to Indonesian.🤖 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 `@webview-ui/src/i18n/locales/es/history.json` around lines 58 - 71, Translate every English value for the new history labels in webview-ui/src/i18n/locales/es/history.json lines 58-71, webview-ui/src/i18n/locales/fr/history.json lines 58-71, webview-ui/src/i18n/locales/hi/history.json lines 51-64, and webview-ui/src/i18n/locales/id/history.json lines 60-73 into the respective locale languages, preserving all translation keys and interpolation placeholders.webview-ui/src/i18n/locales/vi/history.json-51-64 (1)
51-64: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe same English folder and pin strings were added to three non-English locale files. Keys
newFolderthroughdropHereToRemovehold English values in all three files, while the keys that follow in the same block are translated. The shared root cause is one untranslated block copied into each locale. Two of these keys,dropHereToRemoveanddragToOrganize, also duplicate the translateddropToRemoveFromFolderanddragTask; remove whichever key of each pair the components do not use.
webview-ui/src/i18n/locales/vi/history.json#L51-L64: translate the 14 English values to Vietnamese.webview-ui/src/i18n/locales/zh-CN/history.json#L51-L64: translate the 14 English values to Simplified Chinese.webview-ui/src/i18n/locales/zh-TW/history.json#L51-L64: translate the 14 English values to Traditional Chinese.🤖 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 `@webview-ui/src/i18n/locales/vi/history.json` around lines 51 - 64, Translate the 14 English values from newFolder through dropHereToRemove in webview-ui/src/i18n/locales/vi/history.json#L51-L64 into Vietnamese, in webview-ui/src/i18n/locales/zh-CN/history.json#L51-L64 into Simplified Chinese, and in webview-ui/src/i18n/locales/zh-TW/history.json#L51-L64 into Traditional Chinese. In each file, remove the unused duplicate between dragToOrganize and the translated dragTask key, and between dropHereToRemove and the translated dropToRemoveFromFolder key, preserving whichever key the components use.webview-ui/src/components/history/__tests__/TaskOrganizationDndSurface.spec.tsx-196-212 (1)
196-212: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winConfigure
mockUseExtensionStatebefore the first render in this test.
renderruns at line 199, butmockUseExtensionState.mockReturnValue(...)runs at line 206.beforeEachonly callsvi.clearAllMocks(), which clears recorded calls and keeps implementations. The first render therefore uses whatever return value an earlier test installed. If this test runs alone, with.only, or after a reorder,useExtensionState()returnsundefinedand the surface throws while destructuringtaskOrganization.Move the mock setup above
render.💚 Suggested change
const capture = installDndCapture() const mutateSpy = vi.fn().mockResolvedValue(createSuccessResult()) + mockUseExtensionState.mockReturnValue({ + taskOrganization: createEmptyOrganizationState(), + mutateTaskOrganization: mutateSpy, + }) const { rerender } = render( <TaskOrganizationInteractionProvider> <TaskOrganizationDndSurface enabled resolveDragLabel={() => "label"}> <div /> </TaskOrganizationDndSurface> </TaskOrganizationInteractionProvider>, ) - mockUseExtensionState.mockReturnValue({ - taskOrganization: createEmptyOrganizationState(), - mutateTaskOrganization: mutateSpy, - })🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationDndSurface.spec.tsx` around lines 196 - 212, Move the mockUseExtensionState.mockReturnValue setup above the initial render in the “cancels a pending draft when disabled” test, ensuring TaskOrganizationDndSurface receives taskOrganization and mutateTaskOrganization during rendering. Keep the existing mock values and test flow unchanged.webview-ui/src/components/history/__tests__/TaskOrganizationErrorBoundary.spec.tsx-110-115 (1)
110-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis assertion cannot fail.
document.body.textContentreturns text nodes only. It never contains thedata-testidattribute value"safe-child", so line 114 always passes and proves nothing about the throwing subtree. Assert on the rendered element instead. Note that this test then overlaps the test at lines 40-50, so consider merging the two.💚 Suggested change
expect(screen.getByText("Fallback content")).toBeInTheDocument() // The throwing child should not be in the DOM - expect(document.body.textContent).not.toContain("safe-child") + expect(screen.queryByTestId("safe-child")).not.toBeInTheDocument()🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationErrorBoundary.spec.tsx` around lines 110 - 115, Replace the ineffective document.body.textContent check in the TaskOrganizationErrorBoundary test with an assertion that queries the rendered element identified by the throwing child’s data-testid and verifies it is absent. Since this duplicates the existing coverage near the earlier fallback test, merge the assertions or remove the redundant test while preserving verification that the fallback renders and the throwing subtree does not.webview-ui/src/i18n/locales/vi/chat.json-20-20 (1)
20-20: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore the missing UI translation for child tasks.
TaskHeader.tsxstill renders{t("chat:task.waitingOnSubtask")}, but only theenlocale containstask.waitingOnSubtask; add it back for every locale or update the call to use the newchat:subtasks.goToSubtaskkey.🤖 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 `@webview-ui/src/i18n/locales/vi/chat.json` at line 20, Update the translation lookup in TaskHeader.tsx to use the existing chat:subtasks.goToSubtask key instead of chat:task.waitingOnSubtask. The entries at webview-ui/src/i18n/locales/vi/chat.json:20-20, webview-ui/src/i18n/locales/zh-CN/chat.json:20-20, and webview-ui/src/i18n/locales/zh-TW/chat.json:20-20 require no direct changes because they are corrected by reusing the existing key.webview-ui/src/components/history/__tests__/HistoryView.taskOrganization.spec.tsx-578-619 (1)
578-619: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe test does not verify canonical-root resolution.
The test name states that a child drop resolves to its canonical root. The body only asserts that
draggable-entry-unfiled-unit-parent-1is present. It installs the DnD harness but never triggers a drop, and it never inspects the drag data for anautoGrouptarget. As written, the test passes even if canonical resolution is broken.Drive a drop through the harness and assert the resolved source target.
💚 Suggested assertion using the installed harness
render(<HistoryView onDone={vi.fn()} />) - // The parent group draggable must carry the autoGroup target. - const parentEntry = screen.getByTestId("draggable-entry-unfiled-unit-parent-1") - expect(parentEntry).toBeInTheDocument() + expect(screen.getByTestId("draggable-entry-unfiled-unit-parent-1")).toBeInTheDocument() + + // A drag that starts from the group must carry the canonical autoGroup target. + getHarness().triggerDrop( + { kind: "task", target: { kind: "autoGroup", rootTaskId: "parent-1" } }, + { id: "drop-unfiled-unit-solo-1", data: { kind: "task", target: { kind: "task", taskId: "solo-1" } } }, + ) + expect(spies.onRequestCreateFolder).toHaveBeenCalledWith( + { kind: "autoGroup", rootTaskId: "parent-1" }, + { kind: "task", taskId: "solo-1" }, + )🤖 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 `@webview-ui/src/components/history/__tests__/HistoryView.taskOrganization.spec.tsx` around lines 578 - 619, Update the test “resolves an automatic-group child drop to its canonical root” to trigger a child drop through the installed DnD harness after rendering. Inspect the resulting drag data or move callback and assert that the autoGroup source target resolves to the canonical parent root (“parent-1”), rather than only asserting the parent entry is present.webview-ui/src/i18n/locales/ca/history.json-58-71 (1)
58-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the visible history labels for Catalan and German.
webview-ui/src/i18n/locales/ca/history.json#L58-L71: replace the English fallback values with Catalan translations.webview-ui/src/i18n/locales/de/history.json#L58-L71: replace the English fallback values with German translations.Users who select either locale receive a mixed-language history UI.
🤖 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 `@webview-ui/src/i18n/locales/ca/history.json` around lines 58 - 71, Replace the English fallback values for the history labels from newFolder through dropHereToRemove in webview-ui/src/i18n/locales/ca/history.json lines 58-71 with Catalan translations, and apply the corresponding German translations to webview-ui/src/i18n/locales/de/history.json lines 58-71. Preserve all translation keys and interpolation syntax, including {{count}} in tasks.webview-ui/src/i18n/__tests__/translation-parity.spec.ts-10-42 (1)
10-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd every new history key to
REQUIRED_HISTORY_KEYS.The list omits
dragTask,dragFolder,createFolder,createFolderDescription,folderNameLabel,folderNameRequired,folderNameTooLong,folderNameInvalidChars,deleteFolder,folderOptions,expandFolder,collapseFolder,create,openTask, andopenFolder. A locale can omit any of these new UI keys and still pass both parity tests. Add them to the required list.🤖 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 `@webview-ui/src/i18n/__tests__/translation-parity.spec.ts` around lines 10 - 42, Extend REQUIRED_HISTORY_KEYS with all omitted history UI keys: dragTask, dragFolder, createFolder, createFolderDescription, folderNameLabel, folderNameRequired, folderNameTooLong, folderNameInvalidChars, deleteFolder, folderOptions, expandFolder, collapseFolder, create, openTask, and openFolder, so parity tests require every new key.webview-ui/src/components/history/__tests__/useTaskOrganizationDnd.spec.tsx-45-48 (1)
45-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCentralize the partial DnD event fixtures.
The suite repeats undocumented
as unknown as Drag*Eventcasts through line 221. Move the partial fixtures into typeddragStart,dragOver, anddragEndhelpers. If a double assertion is still needed, document the fieldsuseTaskOrganizationDndreads at the helper.🤖 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 `@webview-ui/src/components/history/__tests__/useTaskOrganizationDnd.spec.tsx` around lines 45 - 48, Centralize the partial DnD event construction used by the useTaskOrganizationDnd tests by adding typed dragStart, dragOver, and dragEnd helpers, then replace the repeated inline as unknown as Drag*Event casts through the suite with those helpers. Document within each helper the event fields read by useTaskOrganizationDnd, retaining any required double assertion only there.Source: Coding guidelines
webview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsx-37-37 (1)
37-37: 📐 Maintainability & Code Quality | 🟡 MinorReplace
window as anyresult storage with typed test state.Move the test result store into a test-scoped, typed variable and update the assignment, reset, and assertion references. This applies to both
__lastResult__and__lastMutationResult__usage.[low_effort_and_medium_reward]
🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsx` at line 37, Replace the window-cast result stores with test-scoped typed variables in both webview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsx:37-37 and webview-ui/src/context/__tests__/ExtensionStateContext.taskOrganization.spec.tsx:34-34. Update all __lastResult__ and __lastMutationResult__ assignments, resets, and assertions to use the typed variables instead of window state.Source: Coding guidelines
webview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.ts-10-13 (1)
10-13: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert delegation with a complete pointerdown fixture.
makePointerEventonly suppliestarget, so the delegatedPointerSensoractivator can fail due to missingisPrimary,button, orownerDocumentfields. Add a primary-left-button pointerdown fixture and assert the delegated result istrue; keep the unavoidable event-shape cast in a documented helper.🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.ts` around lines 10 - 13, Update makePointerEvent in the TaskOrganizationPointerSensor tests to provide a complete primary left-button pointerdown fixture, including isPrimary, button, and ownerDocument on the native event target. Add an assertion that the delegated PointerSensor activator returns true, and retain the unavoidable event-shape cast only within this documented helper.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c7df8fe-79dd-4506-a430-34e028c18dee
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (103)
docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.mddocs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.mddocs/260730_0002_session_dashboard-crash-debug/context-save-260730-2333.mddocs/260730_0002_session_dashboard-crash-debug/test-error-interception.txtdocs/260730_0002_session_dashboard-crash-debug/test-mimo-parallel.txtdocs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txtdocs/260730_0002_session_dashboard-crash-debug/test-task-dnd-ux.txtdocs/260730_0002_session_dashboard-crash-debug/test-unified-shell-resolution.txtdocs/260803_0002_session_6-branch-bug-fix-verification/170635_code-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/170715_code-vitest-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/170924_code-report.mddocs/260803_0002_session_6-branch-bug-fix-verification/171100_code-report.mdknip.jsonpackages/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.tswebview-ui/package.jsonwebview-ui/src/components/history/DeleteFoldersDialog.tsxwebview-ui/src/components/history/DraggableTaskEntry.tsxwebview-ui/src/components/history/FolderNameDialog.tsxwebview-ui/src/components/history/HistoryPreview.tsxwebview-ui/src/components/history/HistoryView.tsxwebview-ui/src/components/history/ManualFolderItem.tsxwebview-ui/src/components/history/PinButton.tsxwebview-ui/src/components/history/PinnedHistoryItem.tsxwebview-ui/src/components/history/SubtaskRow.tsxwebview-ui/src/components/history/TaskGroupItem.tsxwebview-ui/src/components/history/TaskItem.tsxwebview-ui/src/components/history/TaskItemFooter.tsxwebview-ui/src/components/history/TaskOrganizationDndSurface.tsxwebview-ui/src/components/history/TaskOrganizationErrorBoundary.tsxwebview-ui/src/components/history/TaskOrganizationInteractionContext.tsxwebview-ui/src/components/history/TaskOrganizationPointerSensor.tswebview-ui/src/components/history/__tests__/DeleteFoldersDialog.spec.tsxwebview-ui/src/components/history/__tests__/DraggableTaskEntry.spec.tsxwebview-ui/src/components/history/__tests__/HistoryPreview.spec.tsxwebview-ui/src/components/history/__tests__/HistoryPreview.taskOrganization.spec.tsxwebview-ui/src/components/history/__tests__/HistoryView.taskOrganization.spec.tsxwebview-ui/src/components/history/__tests__/ManualFolderItem.spec.tsxwebview-ui/src/components/history/__tests__/PinButton.spec.tsxwebview-ui/src/components/history/__tests__/TaskItemFooter.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationDndSurface.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationErrorBoundary.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.setup.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.spec.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.vitest.config.tswebview-ui/src/components/history/__tests__/useTaskOrganizationDnd.spec.tsxwebview-ui/src/components/history/taskOrganizationModel.tswebview-ui/src/components/history/types.tswebview-ui/src/components/history/useTaskOrganizationDnd.tswebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/context/__tests__/ExtensionStateContext.taskOrganization.spec.tsxwebview-ui/src/i18n/__tests__/translation-parity.spec.tswebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/history.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/history.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/history.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/history.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/history.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/history.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/history.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/history.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/history.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/history.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/history.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/history.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/history.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/history.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/history.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/history.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/history.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/history.jsonwebview-ui/vitest.setup.ts
💤 Files with no reviewable changes (1)
- webview-ui/src/components/history/tests/TaskItemFooter.spec.tsx
| schemaVersion: z.number().int().min(1), | ||
| revision: z.number().int().min(0), | ||
| folders: z.array(manualTaskFolderSchema), | ||
| pins: z.array(pinnedItemSchema).max(MAX_PINNED_TARGETS), | ||
| updatedAt: z.number(), | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The pins cap makes forward-compatible reads fail.
The comment at Line 69 states that schemaVersion accepts any positive integer so the store can detect future versions instead of quarantining them. The pins cap defeats that intent. If a newer version raises MAX_PINNED_TARGETS and writes four pins, taskOrganizationStateSchema.safeParse fails in TaskOrganizationStore.load (src/core/task-persistence/TaskOrganizationStore.ts Lines 288-294) before the data.schemaVersion > 1 branch runs. The store then quarantines the file and loads an empty state, so the user loses folders and pins written by the newer version.
Enforce the pin limit at mutation time only, where the store already does so (setPinned, Line 568).
♻️ Proposed change
schemaVersion: z.number().int().min(1),
revision: z.number().int().min(0),
folders: z.array(manualTaskFolderSchema),
- pins: z.array(pinnedItemSchema).max(MAX_PINNED_TARGETS),
+ // No max here: the read schema must stay forward compatible so that a file
+ // written by a newer version is detected via `schemaVersion` instead of
+ // being quarantined. The pin limit is enforced on mutation.
+ pins: z.array(pinnedItemSchema),
updatedAt: z.number(),📝 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.
| schemaVersion: z.number().int().min(1), | |
| revision: z.number().int().min(0), | |
| folders: z.array(manualTaskFolderSchema), | |
| pins: z.array(pinnedItemSchema).max(MAX_PINNED_TARGETS), | |
| updatedAt: z.number(), | |
| }) | |
| schemaVersion: z.number().int().min(1), | |
| revision: z.number().int().min(0), | |
| folders: z.array(manualTaskFolderSchema), | |
| // No max here: the read schema must stay forward compatible so that a file | |
| // written by a newer version is detected via `schemaVersion` instead of | |
| // being quarantined. The pin limit is enforced on mutation. | |
| pins: z.array(pinnedItemSchema), | |
| updatedAt: z.number(), | |
| }) |
🤖 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 `@packages/types/src/task-organization.ts` around lines 72 - 77, Remove the
MAX_PINNED_TARGETS-based .max() constraint from the pins field in
taskOrganizationStateSchema so schema validation remains forward-compatible with
newer pin counts. Keep the pin limit enforced by the existing setPinned mutation
path rather than during persisted-state parsing.
| const requestId = | ||
| "requestId" in mutation && typeof (mutation as Record<string, unknown>).requestId === "string" | ||
| ? ((mutation as Record<string, unknown>).requestId as string) | ||
| : "" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace requestId through the task organization mutation path.
set -euo pipefail
fd -t f 'taskOrganizationMessageHandler' -x rg -n -C 6 'requestId|mutate\(|committedRevision' {}
# Webview-side correlation of mutation results.
rg -n -C 4 'requestId' --glob 'webview-ui/src/**/*.{ts,tsx}' | rg -n -C 4 'taskOrganization|Mutation' || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 8527
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
fd -t f 'TaskOrganizationStore|task-organization' .
echo "== TaskOrganizationStore relevant section =="
store=$(fd -t f 'TaskOrganizationStore' . | head -n1)
if [ -n "${store:-}" ]; then
wc -l "$store"
sed -n '130,210p' "$store" | cat -n
fi
echo "== task-organization schemas relevant section =="
types=$(fd -t f 'task-organization' . | head -n1)
if [ -n "${types:-}" ]; then
wc -l "$types"
sed -n '1,180p' "$types" | cat -n
fi
echo "== mutated result usages in store =="
rg -n -C 4 'taskOrganizationMutationResult|requestId|mutate\(' --glob '**/*.{ts,tsx}' .Repository: Zoo-Code-Org/Zoo-Code
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast, json, pathlib
paths = list(pathlib.Path('.').rglob('*'))
store_path = next(p for p in paths if p.name == 'TaskOrganizationStore.ts')
types_path = next(p for p in paths if p.name == 'task-organization.ts')
store_src = store_path.read_text()
types_src = types_path.read_text()
checks = {
"store_mentions_requestId_property_on_mutation_literal": 'requestId' in store_src
and '.requestId' in store_src
and 'requestId' in store_src,
"store_has_requestId_local_derivation": 'const requestId =' in store_src and 'requestId' in store_src,
"mutation_request_schema_has_requestId": 'requestId' in types_src and 'taskOrganizationMutationRequestV1' in types_src,
}
for k, v in checks.items():
print(f"{k}={v}")
# Behavioral probe: evaluate the store's expression for each mutation variant extracted from source.
import re
mut_names = re.findall(r'kind:\s*"([^"]+)"', types_src)
print("mutation_kinds=" + json.dumps(mut_names))
for kind in mut_names:
expr = f'({"requestId" in {"kind": kind} and isinstance({"requestId" in {"kind": kind}.get("requestId", None)}, str) and str({"requestId" in {"kind": kind}.get("requestId", None)}, str)} else "")'
print("requestId_property_found_on_extracted_mutation_objects=" + json.dumps(not any('requestId' in kind for kind in mut_names)))
PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
store_path = next(p for p in pathlib.Path('.').rglob('*') if p.name == 'TaskOrganizationStore.ts')
types_path = next(p for p in pathlib.Path('.').rglob('*') if p.name == 'task-organization.ts')
store_src = store_path.read_text()
types_src = types_path.read_text()
print("== store requestId extraction source ==")
for i, line in enumerate(store_src.splitlines(), 1):
if 'requestId' in line or 'const requestId' in line:
print(f"{store_path}:{i}: {line.strip()}")
print("\n== mutation kinds in task-organization.ts ==")
for i, line in enumerate(types_src.splitlines(), 1):
m = re.search(r'kind:\s*"([^"]+)"', line)
if m:
print(f"{types_path}:{i}: kind={m.group(1)} line={line.strip()}")
print("\n== requestId in task-organization.ts nearby definitions ==")
for i, line in enumerate(types_src.splitlines(), 1):
if 'requestId' in line or 'taskOrganizationMutationRequestV1' in line or 'TaskOrganizationMutationResultV1' in line:
print(f"{types_path}:{i}: {line.strip()}")
print("\n== taskOrganizationMutation message/result correlation in TypeScript source ==")
for p in pathlib.Path('.').rglob('*'):
if not (p.name.endswith(('.ts', '.tsx')) or p.is_file()):
continue
if p.name in {'TaskOrganizationStore.ts', 'task-organization.ts'}:
continue
src = p.read_text(errors='ignore')
if 'taskOrganizationMutationResult' in src or 'taskOrganizationMutation:' in src:
hits = []
for i, line in enumerate(src.splitlines(), 1):
if 'taskOrganizationMutationResult' in line or 'type: "taskOrganizationMutationResult"' in line or 'requestId' in line:
hits.append((i, line.strip()))
if hits:
print(f"\n--- {p} ---")
for idx, (line_no, line) in enumerate(hits):
start = max(1, line_no - 3)
end = min(len(src.splitlines()), line_no + 3)
for n in range(start, end + 1):
print(f"{n}: {src.splitlines()[n-1].strip()}")
if idx < len(hits) - 1:
print()
PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 21048
Keep requestId off the mutation and forward the request wrapper’s requestId.
TaskOrganizationMutationV1 does not carry requestId, so store.mutate(...) returns requestId: "". The message handler then posts requestId: "" in taskOrganizationMutationResult, but the webview indexes pending mutations by the request wrapper requestId, so the store path can break mutation result correlation. Pass the request requestId into the store and return it on every result.
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 177 - 180,
Update the TaskOrganizationStore mutation flow to stop reading requestId from
the TaskOrganizationMutationV1 payload. Accept the request wrapper’s requestId
at the store entry point, propagate it through store.mutate(...) and every
taskOrganizationMutationResult response, and keep requestId excluded from the
mutation object.
| const surviving = missing.flatMap((id) => { | ||
| const descendants: string[] = [] | ||
| const stack = childMap.get(id) ?? [] | ||
| while (stack.length > 0) { | ||
| const child = stack.pop()! | ||
| if (visibleIds.has(child)) { | ||
| descendants.push(child) | ||
| } | ||
| stack.push(...(childMap.get(child) ?? [])) | ||
| } | ||
| return descendants | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
surviving mutates the shared childMap arrays and drops descendants.
childMap.get(id) returns the array stored in the map, not a copy. Line 701 assigns that array to stack, and Lines 703 and 707 then pop() and push() on it. The loop therefore empties the map entry for id and appends unrelated grandchildren to it.
Two effects follow:
- If a second missing member shares the same parent entry, the second lookup sees a drained or polluted array, so surviving descendants are lost from the folder.
- The polluted
childMapalso feeds the laternext.foldersiterations in the samerecomputeFromHistorycall.
The result is silent folder-membership loss during reconciliation after a parent task is deleted.
🐛 Proposed fix
const surviving = missing.flatMap((id) => {
const descendants: string[] = []
- const stack = childMap.get(id) ?? []
+ const stack = [...(childMap.get(id) ?? [])]
while (stack.length > 0) {
const child = stack.pop()!
if (visibleIds.has(child)) {
descendants.push(child)
}
stack.push(...(childMap.get(child) ?? []))
}
return descendants
})A cycle in parentTaskId data would also make this while loop run forever. Consider tracking visited IDs:
const descendants: string[] = []
const stack = [...(childMap.get(id) ?? [])]
+ const seen = new Set<string>()
while (stack.length > 0) {
const child = stack.pop()!
+ if (seen.has(child)) continue
+ seen.add(child)📝 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.
| const surviving = missing.flatMap((id) => { | |
| const descendants: string[] = [] | |
| const stack = childMap.get(id) ?? [] | |
| while (stack.length > 0) { | |
| const child = stack.pop()! | |
| if (visibleIds.has(child)) { | |
| descendants.push(child) | |
| } | |
| stack.push(...(childMap.get(child) ?? [])) | |
| } | |
| return descendants | |
| }) | |
| const surviving = missing.flatMap((id) => { | |
| const descendants: string[] = [] | |
| const stack = [...(childMap.get(id) ?? [])] | |
| const seen = new Set<string>() | |
| while (stack.length > 0) { | |
| const child = stack.pop()! | |
| if (seen.has(child)) continue | |
| seen.add(child) | |
| if (visibleIds.has(child)) { | |
| descendants.push(child) | |
| } | |
| stack.push(...(childMap.get(child) ?? [])) | |
| } | |
| return descendants | |
| }) |
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 699 - 710,
Update the descendant traversal in recomputeFromHistory’s surviving calculation
so stack is initialized from a copy of childMap.get(id), preventing pop/push
operations from mutating shared childMap arrays. Also track visited task IDs
during traversal to prevent infinite loops when parentTaskId data contains
cycles, while preserving descendant collection for visible IDs.
| const createMockProvider = (mutateResult: TaskOrganizationMutationResultV1): ClineProvider => { | ||
| const mockLog = vi.fn() | ||
| const mockPostMessageToWebview = vi.fn() | ||
| const mockMutate = vi.fn().mockResolvedValue(mutateResult) | ||
| const mockState = createEmptyTaskOrganizationState() | ||
|
|
||
| const store = { | ||
| mutate: mockMutate, | ||
| getState: vi.fn(() => mockState), | ||
| } | ||
|
|
||
| return { | ||
| log: mockLog, | ||
| postMessageToWebview: mockPostMessageToWebview, | ||
| getTaskOrganizationStore: vi.fn(() => store), | ||
| } as unknown as ClineProvider |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace the broad casts and lint suppression with precise test doubles.
The mock factory casts a partial object to the complete ClineProvider class. The malformed-request test also adds an as any cast and suppresses the resulting lint violation.
Define a narrow provider interface for handleTaskOrganizationMessage. Type both provider mocks against that interface. Test malformed input with an undefined payload or a documented unknown boundary fixture.
As per coding guidelines, new TypeScript code must fix lint violations instead of suppressing them, avoid as any, and use precise test doubles.
Also applies to: 86-97, 237-245
🤖 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 11 - 26, Replace the broad ClineProvider cast in createMockProvider and
the other provider mock with a narrow interface containing only the members
required by handleTaskOrganizationMessage. Type both test doubles against that
interface, remove the malformed-request as any cast and lint suppression, and
pass an undefined payload or documented unknown boundary fixture instead.
Source: Coding guidelines
| <Button | ||
| variant="ghost" | ||
| className="flex-1 min-w-0 justify-start h-auto px-0 py-0 font-normal text-left truncate" | ||
| onClick={onClick} | ||
| aria-label={isFolder ? t("history:openFolder", { name: folderName }) : t("history:openTask")}> | ||
| <span className="truncate" data-testid="pinned-item-label"> | ||
| {isFolder ? folderName : (label ?? unit.rootTaskId)} | ||
| </span> | ||
| </Button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The pinned shortcut button does nothing for the current callers.
onClick is optional. HistoryView.tsx (Lines 334-357) and HistoryPreview.tsx (Lines 173-198) render PinnedHistoryItem without onClick. The button stays focusable and announces history:openTask or history:openFolder, but a click and an Enter key press have no effect. A pinned shortcut that cannot be opened defeats the purpose of the pinned section.
Either make onClick required, or add a default action. For pinned units, post showTaskWithId with the unit root id; for pinned folders, expand the folder.
🤖 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 `@webview-ui/src/components/history/PinnedHistoryItem.tsx` around lines 64 -
72, Update PinnedHistoryItem so its focusable shortcut always performs an
action: either require callers to provide onClick and update HistoryView and
HistoryPreview, or implement the default behavior of posting showTaskWithId for
pinned units and expanding pinned folders. Preserve the existing folder/task
labels and ensure both click and keyboard activation use the working action.
| const handleRequestMoveToFolder = useCallback( | ||
| (source: TaskOrganizationTargetV1, folderId: string) => { | ||
| if (!enabled) return | ||
| void moveToFolder(source, folderId) | ||
| }, | ||
| [enabled, moveToFolder], | ||
| ) | ||
|
|
||
| const handleRequestRemoveFromFolder = useCallback( | ||
| (source: TaskOrganizationTargetV1, folderId: string) => { | ||
| if (!enabled) return | ||
| void removeFromFolder(source, folderId) | ||
| }, | ||
| [enabled, removeFromFolder], | ||
| ) | ||
|
|
||
| const { sensors, activeDrag, handleDragStart, handleDragOver, handleDragEnd, handleDragCancel } = | ||
| useTaskOrganizationDnd({ | ||
| onRequestCreateFolder: handleRequestCreateFolder, | ||
| onRequestMoveToFolder: handleRequestMoveToFolder, | ||
| onRequestRemoveFromFolder: handleRequestRemoveFromFolder, | ||
| }) | ||
|
|
||
| const handleConfirmFolderName = useCallback( | ||
| (name: string) => { | ||
| if (!pendingFolderDraft) return | ||
| void createFolder(name, pendingFolderDraft.source, pendingFolderDraft.destination) | ||
| setPendingFolderDraft(null) | ||
| }, | ||
| [createFolder, pendingFolderDraft], | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle the mutation results instead of discarding them.
moveToFolder, removeFromFolder, and createFolder resolve with a TaskOrganizationMutationResultV1 and do not throw. TaskOrganizationStore.mutate returns success: false with codes such as TASK_ORG/CONFLICT/002 when the revision is stale. All three call sites use void and drop that result, so a rejected drag or a failed folder creation produces no message and no retry. The user sees the drop do nothing. Inspect success and surface the error, for example through the existing toast or error state.
🛡️ Sketch for surfacing failures
const handleRequestMoveToFolder = useCallback(
(source: TaskOrganizationTargetV1, folderId: string) => {
if (!enabled) return
- void moveToFolder(source, folderId)
+ moveToFolder(source, folderId)
+ .then((result) => {
+ if (!result.success) onMutationError?.(result.error)
+ })
+ .catch(() => onMutationError?.())
},
- [enabled, moveToFolder],
+ [enabled, moveToFolder, onMutationError],
)🤖 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 `@webview-ui/src/components/history/TaskOrganizationDndSurface.tsx` around
lines 84 - 114, Update handleRequestMoveToFolder, handleRequestRemoveFromFolder,
and handleConfirmFolderName to await their mutation results instead of
discarding them with void. Inspect each TaskOrganizationMutationResultV1 success
value and surface failed mutations through the component’s existing toast or
error-state mechanism, while preserving the current enabled and pending-folder
guards.
| const mutateTaskOrganization = useCallback( | ||
| async (mutation: TaskOrganizationMutationRequestV1["mutation"]): Promise<TaskOrganizationMutationResultV1> => { | ||
| const requestId = `task-org-${Date.now()}-${Math.random().toString(36).slice(2)}` | ||
| const currentRevision = taskOrgRevisionRef.current | ||
|
|
||
| vscode.postMessage({ | ||
| type: "taskOrganizationMutation", | ||
| taskOrganizationMutation: { | ||
| requestId, | ||
| baseRevision: currentRevision, | ||
| mutation, | ||
| }, | ||
| } as WebviewMessage) | ||
|
|
||
| return new Promise((resolve) => { | ||
| pendingTaskOrgMutations.current.set(requestId, resolve) | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add timeout and unmount cleanup for pending mutations.
A pending entry is removed only when a matching result arrives. If the host drops the response or the provider is disposed, the promise never settles and the resolver remains in the map.
Add a timeout for each request. Remove the entry when the timeout expires. Also settle and clear all pending requests when the context provider unmounts.
🤖 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 `@webview-ui/src/context/ExtensionStateContext.tsx` around lines 541 - 557,
Update mutateTaskOrganization to associate each pending request with a timeout
that removes its requestId from pendingTaskOrgMutations and settles the promise
when no response arrives. Add provider-unmount cleanup that clears all remaining
entries and settles their promises, while cancelling any request timers to avoid
callbacks after cleanup.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (29)
docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md-58-58 (1)
58-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the wording in the failure analysis.
Use
terminal, shell, and command-execution testsat Line 58. Use1 msinstead of1msat Lines 66 and 228.Also applies to: 66-66, 228-228
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md` at line 58, Update the failure-analysis wording to say “terminal, shell, and command-execution tests” instead of “terminal/shell/command execution related tests,” and format both occurrences of the duration as “1 ms” rather than “1ms” at the referenced lines.Source: Linters/SAST tools
docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md-124-134 (1)
124-134: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the short-range breakdown finding to match the later fix.
docs/260730_0002_session_dashboard-crash-debug/context-save-260730-2333.mdLine 11 records commit0769ccea7, and Line 51 records the daily-rollup fix as completed. This report still presents the monthly-rollup problem as unresolved and retains it as a release condition. Mark the finding as resolved or clearly label this report as a pre-fix snapshot.Also applies to: 171-183
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md` around lines 124 - 134, Update the short-range breakdown finding in the report, including the repeated section around the later referenced lines, to reflect that the daily-rollup fix is completed. Mark the monthly-rollup issue as resolved and remove it as an outstanding release condition, or clearly label the report as a pre-fix snapshot while preserving the recorded commit references.docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md-136-144 (1)
136-144: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winQualify the claim that the crash is eliminated.
Lines 138-144 state that
cacheRatio > 0still uses the full event scan and can retain the crash vector. Lines 165-167 then state that the crash is eliminated. Replace the absolute claim with a statement limited to the default fast-path query.Also applies to: 163-167
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.md` around lines 136 - 144, The document states in lines 138-144 that cacheRatio > 0 scenarios still use full event scans and retain the crash vector, but then makes an absolute claim in lines 165-167 that the crash is eliminated. Update the crash-elimination claim in lines 165-167 to qualify it as only applying to the default fast-path query configuration (where cacheRatio is 0 or undefined), making clear that the limitation described in Inquiry 2 means the crash vector persists for users who enable cacheRatio.docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md-80-80 (1)
80-80: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to the code fences.
Use
textfor the three branch-list fences at Lines 80, 125, and 160. This resolves the reported Markdownlint MD040 warnings.Also applies to: 125-125, 160-160
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.md` at line 80, The three branch-list code fences in this markdown file are missing language identifiers, which triggers Markdownlint MD040 warnings. Add the language identifier `text` to each of the three code fence opening markers for the branch-list sections. This ensures each code fence declaration includes a language specifier, resolving the linting violations.Source: Linters/SAST tools
docs/260730_0002_session_dashboard-crash-debug/test-mimo-parallel.txt-1-133 (1)
1-133: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStore a normalized, reviewable test report.
As committed, this file contains NUL-padded terminal output and ANSI escape sequences. Common tools can treat it as binary, and the report is difficult to read or search. Re-export it as UTF-8 plain text with terminal control codes removed, or commit a concise report with the command, exit status, platform, and test counts.
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/test-mimo-parallel.txt` around lines 1 - 133, Replace the raw terminal capture in the test report with UTF-8 plain text by removing NUL padding and ANSI escape sequences. Prefer a concise, reviewable report that preserves the test command, exit status, platform, and final test counts.docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt-1-2 (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winNormalize the captured output before committing it.
The file contains NUL bytes and terminal ANSI escape sequences. Standard viewers and repository search display corrupted content. Re-capture or convert the output to UTF-8, strip terminal control codes, and retain only readable log content.
🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt` around lines 1 - 2, Normalize the captured output in test-strict-reasoning.txt before committing it: convert the file to UTF-8, remove NUL bytes and terminal ANSI escape sequences, and retain only readable log content.docs/260803_0002_session_6-branch-bug-fix-verification/170924_code-report.md-3-7 (1)
3-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the relative links to source files.
This report is under
docs/260803_0002_session_6-branch-bug-fix-verification/. Therefore,../src/...resolves todocs/src/..., not the repositorysrc/...directory. Change the affected links on Line 3, Line 6, Line 7, Line 15, Line 16, Line 26, and Line 27 to use../../src/....Also applies to: 15-16, 26-27
🤖 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 `@docs/260803_0002_session_6-branch-bug-fix-verification/170924_code-report.md` around lines 3 - 7, Update the affected relative source links in the report, including references to TaskOrganizationStore.ts, TaskOrganizationStore.spec.ts, withLock(), mutate(), resolveUnit(), and resolveTaskClosure(), from ../src/... to ../../src/... so they resolve from the document’s directory to the repository src directory.webview-ui/src/components/history/TaskOrganizationDndSurface.tsx-72-74 (1)
72-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not discard a typed folder name on every revision change.
This effect clears
pendingFolderDraftwheneverorganization.revisionchanges. The drag that opened the dialog is not the only source of revision changes: a concurrentmoveToFolder, a pin toggle, or a mutation from another view also bumps it. The folder-name dialog then closes and the typed name is lost with no message. Restrict the cancellation to the case where the draft source or destination no longer exists, or keep the dialog open and revalidate on confirm.🤖 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 `@webview-ui/src/components/history/TaskOrganizationDndSurface.tsx` around lines 72 - 74, Update the useEffect watching organization.revision so it does not unconditionally clear pendingFolderDraft on unrelated revisions. Only cancel the draft when its source or destination folder no longer exists, or otherwise keep the dialog open and revalidate those references during confirmation.webview-ui/src/components/history/ManualFolderItem.tsx-316-319 (1)
316-319: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable the member drop target during selection mode.
ManualFolderItemdisables its folder drop target whenisSelectionModeis true, butManualFolderMemberItemregisters a member drop target withoutdisabled. Pass the current mode throughHistoryPreviewInnerand setdisabled: isSelectionModeon the member droppable so drops do not create folders during selection mode.🤖 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 `@webview-ui/src/components/history/ManualFolderItem.tsx` around lines 316 - 319, Update ManualFolderMemberItem’s useDroppable configuration to accept the isSelectionMode value passed through HistoryPreviewInner and set disabled to that value, matching the existing folder drop-target behavior so member drops cannot create folders during selection mode.webview-ui/src/components/history/TaskOrganizationInteractionContext.tsx-60-69 (1)
60-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReconcile the two
targetKeyimplementations.
buildGroupDndData()emitsautoGrouptargets from pinned projection rows, but the UI callsisPinned()/togglePin()with equivalenttasktargets for task groups. This localtargetKeymaps those same units to different keys, so a task group can render as pinned and remain incanPinafter the task-unit pin is removed. Use one shared canonical helper for task andautoGrouppins.🤖 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 `@webview-ui/src/components/history/TaskOrganizationInteractionContext.tsx` around lines 60 - 69, Update the targetKey logic used by buildGroupDndData, isPinned, and togglePin so task and autoGroup targets for the same task unit resolve to the same canonical key; reuse the existing shared helper if available rather than maintaining a separate local mapping. Preserve distinct keys for folder targets and ensure pin removal updates canPin consistently.webview-ui/src/components/history/taskOrganizationModel.ts-635-647 (1)
635-647: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a single child-relationship source for workspace filtering.
buildFlattenedVirtualEntriesusesparentTaskIdthroughchildrenMap, butbuildGroupedOrganizationProjectionusestask.childIdsinisVisibleInWorkspace.childIdsis optional onHistoryItemand is not reliably written alongsideparentTaskId, so a group can hide even when one of its descendants belongs to the current workload. Switch this path to the sameparentTaskId/children-map source.🤖 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 `@webview-ui/src/components/history/taskOrganizationModel.ts` around lines 635 - 647, Update isVisibleInWorkspace to collect descendant task IDs using the parentTaskId-derived childrenMap, matching buildFlattenedVirtualEntries, instead of relying on task.childIds. Preserve the existing root fallback and taskBelongsToWorkspace checks so descendants in the current workspace keep the group visible.webview-ui/src/components/history/SubtaskRow.tsx-113-116 (1)
113-116: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUnwired pin props in two leaf components. Both files gained pin props and a
PinButtonbranch, but their parents never pass those props, so both branches are unreachable in the current tree. Decide one owner for the pin control per card and wire or remove accordingly.
webview-ui/src/components/history/SubtaskRow.tsx#L113-L116: passshowPin,isPinned,canPin, andonTogglePinfromTaskGroupItem.tsxLine 101, or delete the props and thePinButtonbranch at Lines 76-84.webview-ui/src/components/history/TaskItemFooter.tsx#L71-L79: pass the pin props fromTaskItem.tsxLines 136-142, or delete this branch and keep the header pin control inTaskItem.🤖 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 `@webview-ui/src/components/history/SubtaskRow.tsx` around lines 113 - 116, Choose one pin-control owner per card and ensure the other leaf branch is removed or wired. In webview-ui/src/components/history/SubtaskRow.tsx:113-116, update TaskGroupItem.tsx:101 to pass showPin, isPinned, canPin, and onTogglePin, or remove those props and the PinButton branch at SubtaskRow.tsx:76-84. In webview-ui/src/components/history/TaskItemFooter.tsx:71-79, either pass the pin props from TaskItem.tsx:136-142 or remove this branch while retaining TaskItem’s header pin control.webview-ui/src/components/history/TaskOrganizationErrorBoundary.tsx-13-20 (1)
13-20: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the doc comment.
Lines 18-19 state that the boundary "renders children as-is".
renderat Lines 39-41 returnsthis.props.fallback ?? nullafter an error, so the children are unmounted. Align the comment with the behavior.♻️ Proposed change
- * On error the boundary logs a warning and renders children as-is (i.e. the - * new feature is silently disabled rather than crashing the whole view). + * On error the boundary logs the error, unmounts the failing subtree, and + * renders the provided fallback (or nothing) instead of crashing the view.🤖 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 `@webview-ui/src/components/history/TaskOrganizationErrorBoundary.tsx` around lines 13 - 20, Update the documentation comment for TaskOrganizationErrorBoundary to state that it renders the configured fallback, or null when no fallback is provided, after an error; remove the claim that it renders children as-is or leaves the existing view mounted.webview-ui/src/components/history/HistoryView.tsx-231-252 (1)
231-252: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe folder targets in
handleConfirmSelectionFolderNameare unreachable.
canCreateFolderFromSelectionat Line 231 requiresselectedFolderIds.length === 0.handleCreateFolderFromSelectionreturns early when that flag is false, so the dialog only opens with zero selected folders. TheselectedFolderIds.map(...)spread at Line 242 therefore always produces an empty list, and the comment at Line 228 ("tasks/groups and/or folders combined") does not match the gate.Decide the intended behavior. If folders must never join a new folder, remove the dead spread and correct the comment. If folders may join, relax the gate.
♻️ Proposed change if folders must be excluded
- // Create Folder is enabled when at least two distinct canonical units are - // selected (tasks/groups and/or folders combined). + // Create Folder is enabled when at least two distinct canonical task units + // are selected. Folder selection disables it. // Architect spec Section 1.6: create-folder requires at least two canonical // task units and is disabled while any folder is selected. const canCreateFolderFromSelection = selectedTaskTargets.length >= 2 && selectedFolderIds.length === 0const handleConfirmSelectionFolderName = useCallback( (name: string) => { - const targets: TaskOrganizationTargetV1[] = [ - ...selectedTaskTargets, - ...selectedFolderIds.map((folderId) => ({ kind: "folder", folderId }) as TaskOrganizationTargetV1), - ] - void createFolderFromSelection(name, targets).then((result) => { + void createFolderFromSelection(name, selectedTaskTargets).then((result) => { if (result.success) { setSelectedTaskIds([]) setSelectedFolderIds([]) } }) }, - [selectedTaskTargets, selectedFolderIds, createFolderFromSelection], + [selectedTaskTargets, createFolderFromSelection], )🤖 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 `@webview-ui/src/components/history/HistoryView.tsx` around lines 231 - 252, Resolve the inconsistency between canCreateFolderFromSelection and handleConfirmSelectionFolderName: if selected folders are not allowed, remove the selectedFolderIds target mapping and update the nearby comment to describe task/group-only selection; otherwise, relax the canCreateFolderFromSelection guard so folder selections can reach the dialog and retain their targets.src/core/task-persistence/TaskOrganizationStore.ts-842-875 (1)
842-875: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winThe watcher never recovers if the tasks directory is missing.
fsSync.watchthrowsENOENTwhentasksDirdoes not exist. On a fresh profile the store loads an empty state and writes nothing until the first mutation, so the directory can be absent atinitialize()time. Thecatchat Line 869 logs the failure, and no later attempt starts the watcher. Cross-instance reloads then stay disabled for the whole session.Create the directory before watching, or retry after the first successful save.
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 842 - 875, The watcher setup around getTasksDir and fsSync.watch must handle a missing tasks directory instead of permanently stopping after ENOENT. Ensure the directory is created before calling fsSync.watch, or trigger a retry after the first successful save, while preserving the existing disposed checks and watcher behavior.webview-ui/src/components/history/__tests__/DeleteFoldersDialog.spec.tsx-4-41 (1)
4-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the localized count and cancel close behavior.
The translation mock returns raw keys. The rendering test does not verify the folder count. The cancel test does not verify
onOpenChange(false).Return representative localized strings from
t. Assert the interpolated count. Pass a spy toonOpenChangein the cancel test and assert that it receivesfalse.🤖 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 `@webview-ui/src/components/history/__tests__/DeleteFoldersDialog.spec.tsx` around lines 4 - 41, Update the useAppTranslation mock in DeleteFoldersDialog tests to return representative localized strings with count interpolation, then assert the rendered confirmation text includes the folder count. In the cancel test, pass a spy as onOpenChange and verify it is called with false while preserving the existing onConfirm assertion.webview-ui/src/i18n/locales/pl/history.json-51-64 (1)
51-64: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new task-organization strings in each locale.
These values render in English for non-English users.
webview-ui/src/i18n/locales/pl/history.json#L51-L64: Replace the English values with Polish translations.webview-ui/src/i18n/locales/pt-BR/history.json#L51-L64: Replace the English values with Brazilian Portuguese translations.webview-ui/src/i18n/locales/ru/history.json#L51-L64: Replace the English values with Russian translations.webview-ui/src/i18n/locales/tr/history.json#L51-L64: Replace the English values with Turkish translations.🤖 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 `@webview-ui/src/i18n/locales/pl/history.json` around lines 51 - 64, Translate the new task-organization strings, preserving the existing keys and interpolation syntax, in webview-ui/src/i18n/locales/pl/history.json lines 51-64 (Polish), webview-ui/src/i18n/locales/pt-BR/history.json lines 51-64 (Brazilian Portuguese), webview-ui/src/i18n/locales/ru/history.json lines 51-64 (Russian), and webview-ui/src/i18n/locales/tr/history.json lines 51-64 (Turkish); replace each English value with its appropriate locale translation.webview-ui/src/i18n/locales/it/history.json-51-64 (1)
51-64: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the English history strings.
These locale bundles display English folder controls to Italian, Japanese, and Dutch users.
webview-ui/src/i18n/locales/it/history.json#L51-L64: Replace the English values with Italian translations.webview-ui/src/i18n/locales/ja/history.json#L51-L64: Replace the English values with Japanese translations.webview-ui/src/i18n/locales/nl/history.json#L51-L64: Replace the English values with Dutch translations.🤖 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 `@webview-ui/src/i18n/locales/it/history.json` around lines 51 - 64, The locale bundle files contain English strings instead of translations for Italian, Japanese, and Dutch users. Update webview-ui/src/i18n/locales/it/history.json lines 51-64 to replace all English string values (newFolder, folderNamePlaceholder, renameFolder, removeFromFolder, deleteEmptyFolder, pin, unpin, pinLimitReached, pinned, folder, tasks, unfiled, dragToOrganize, dropHereToRemove) with Italian translations. Apply the same transformation at webview-ui/src/i18n/locales/ja/history.json lines 51-64 with Japanese translations. Apply the same transformation at webview-ui/src/i18n/locales/nl/history.json lines 51-64 with Dutch translations. Keep all JSON keys unchanged; only update the string values to their target-language equivalents.webview-ui/src/i18n/locales/es/history.json-58-71 (1)
58-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new history labels.
These locale bundles still show English for new folder, pin, and drag-and-drop labels. Translate all added English values so the new workflow remains localized.
webview-ui/src/i18n/locales/es/history.json#L58-L71: Translate the English history labels to Spanish.webview-ui/src/i18n/locales/fr/history.json#L58-L71: Translate the English history labels to French.webview-ui/src/i18n/locales/hi/history.json#L51-L64: Translate the English history labels to Hindi.webview-ui/src/i18n/locales/id/history.json#L60-L73: Translate the English history labels to Indonesian.🤖 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 `@webview-ui/src/i18n/locales/es/history.json` around lines 58 - 71, Translate every English value for the new history labels in webview-ui/src/i18n/locales/es/history.json lines 58-71, webview-ui/src/i18n/locales/fr/history.json lines 58-71, webview-ui/src/i18n/locales/hi/history.json lines 51-64, and webview-ui/src/i18n/locales/id/history.json lines 60-73 into the respective locale languages, preserving all translation keys and interpolation placeholders.webview-ui/src/i18n/locales/vi/history.json-51-64 (1)
51-64: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe same English folder and pin strings were added to three non-English locale files. Keys
newFolderthroughdropHereToRemovehold English values in all three files, while the keys that follow in the same block are translated. The shared root cause is one untranslated block copied into each locale. Two of these keys,dropHereToRemoveanddragToOrganize, also duplicate the translateddropToRemoveFromFolderanddragTask; remove whichever key of each pair the components do not use.
webview-ui/src/i18n/locales/vi/history.json#L51-L64: translate the 14 English values to Vietnamese.webview-ui/src/i18n/locales/zh-CN/history.json#L51-L64: translate the 14 English values to Simplified Chinese.webview-ui/src/i18n/locales/zh-TW/history.json#L51-L64: translate the 14 English values to Traditional Chinese.🤖 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 `@webview-ui/src/i18n/locales/vi/history.json` around lines 51 - 64, Translate the 14 English values from newFolder through dropHereToRemove in webview-ui/src/i18n/locales/vi/history.json#L51-L64 into Vietnamese, in webview-ui/src/i18n/locales/zh-CN/history.json#L51-L64 into Simplified Chinese, and in webview-ui/src/i18n/locales/zh-TW/history.json#L51-L64 into Traditional Chinese. In each file, remove the unused duplicate between dragToOrganize and the translated dragTask key, and between dropHereToRemove and the translated dropToRemoveFromFolder key, preserving whichever key the components use.webview-ui/src/components/history/__tests__/TaskOrganizationDndSurface.spec.tsx-196-212 (1)
196-212: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winConfigure
mockUseExtensionStatebefore the first render in this test.
renderruns at line 199, butmockUseExtensionState.mockReturnValue(...)runs at line 206.beforeEachonly callsvi.clearAllMocks(), which clears recorded calls and keeps implementations. The first render therefore uses whatever return value an earlier test installed. If this test runs alone, with.only, or after a reorder,useExtensionState()returnsundefinedand the surface throws while destructuringtaskOrganization.Move the mock setup above
render.💚 Suggested change
const capture = installDndCapture() const mutateSpy = vi.fn().mockResolvedValue(createSuccessResult()) + mockUseExtensionState.mockReturnValue({ + taskOrganization: createEmptyOrganizationState(), + mutateTaskOrganization: mutateSpy, + }) const { rerender } = render( <TaskOrganizationInteractionProvider> <TaskOrganizationDndSurface enabled resolveDragLabel={() => "label"}> <div /> </TaskOrganizationDndSurface> </TaskOrganizationInteractionProvider>, ) - mockUseExtensionState.mockReturnValue({ - taskOrganization: createEmptyOrganizationState(), - mutateTaskOrganization: mutateSpy, - })🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationDndSurface.spec.tsx` around lines 196 - 212, Move the mockUseExtensionState.mockReturnValue setup above the initial render in the “cancels a pending draft when disabled” test, ensuring TaskOrganizationDndSurface receives taskOrganization and mutateTaskOrganization during rendering. Keep the existing mock values and test flow unchanged.webview-ui/src/components/history/__tests__/TaskOrganizationErrorBoundary.spec.tsx-110-115 (1)
110-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis assertion cannot fail.
document.body.textContentreturns text nodes only. It never contains thedata-testidattribute value"safe-child", so line 114 always passes and proves nothing about the throwing subtree. Assert on the rendered element instead. Note that this test then overlaps the test at lines 40-50, so consider merging the two.💚 Suggested change
expect(screen.getByText("Fallback content")).toBeInTheDocument() // The throwing child should not be in the DOM - expect(document.body.textContent).not.toContain("safe-child") + expect(screen.queryByTestId("safe-child")).not.toBeInTheDocument()🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationErrorBoundary.spec.tsx` around lines 110 - 115, Replace the ineffective document.body.textContent check in the TaskOrganizationErrorBoundary test with an assertion that queries the rendered element identified by the throwing child’s data-testid and verifies it is absent. Since this duplicates the existing coverage near the earlier fallback test, merge the assertions or remove the redundant test while preserving verification that the fallback renders and the throwing subtree does not.webview-ui/src/i18n/locales/vi/chat.json-20-20 (1)
20-20: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore the missing UI translation for child tasks.
TaskHeader.tsxstill renders{t("chat:task.waitingOnSubtask")}, but only theenlocale containstask.waitingOnSubtask; add it back for every locale or update the call to use the newchat:subtasks.goToSubtaskkey.🤖 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 `@webview-ui/src/i18n/locales/vi/chat.json` at line 20, Update the translation lookup in TaskHeader.tsx to use the existing chat:subtasks.goToSubtask key instead of chat:task.waitingOnSubtask. The entries at webview-ui/src/i18n/locales/vi/chat.json:20-20, webview-ui/src/i18n/locales/zh-CN/chat.json:20-20, and webview-ui/src/i18n/locales/zh-TW/chat.json:20-20 require no direct changes because they are corrected by reusing the existing key.webview-ui/src/components/history/__tests__/HistoryView.taskOrganization.spec.tsx-578-619 (1)
578-619: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe test does not verify canonical-root resolution.
The test name states that a child drop resolves to its canonical root. The body only asserts that
draggable-entry-unfiled-unit-parent-1is present. It installs the DnD harness but never triggers a drop, and it never inspects the drag data for anautoGrouptarget. As written, the test passes even if canonical resolution is broken.Drive a drop through the harness and assert the resolved source target.
💚 Suggested assertion using the installed harness
render(<HistoryView onDone={vi.fn()} />) - // The parent group draggable must carry the autoGroup target. - const parentEntry = screen.getByTestId("draggable-entry-unfiled-unit-parent-1") - expect(parentEntry).toBeInTheDocument() + expect(screen.getByTestId("draggable-entry-unfiled-unit-parent-1")).toBeInTheDocument() + + // A drag that starts from the group must carry the canonical autoGroup target. + getHarness().triggerDrop( + { kind: "task", target: { kind: "autoGroup", rootTaskId: "parent-1" } }, + { id: "drop-unfiled-unit-solo-1", data: { kind: "task", target: { kind: "task", taskId: "solo-1" } } }, + ) + expect(spies.onRequestCreateFolder).toHaveBeenCalledWith( + { kind: "autoGroup", rootTaskId: "parent-1" }, + { kind: "task", taskId: "solo-1" }, + )🤖 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 `@webview-ui/src/components/history/__tests__/HistoryView.taskOrganization.spec.tsx` around lines 578 - 619, Update the test “resolves an automatic-group child drop to its canonical root” to trigger a child drop through the installed DnD harness after rendering. Inspect the resulting drag data or move callback and assert that the autoGroup source target resolves to the canonical parent root (“parent-1”), rather than only asserting the parent entry is present.webview-ui/src/i18n/locales/ca/history.json-58-71 (1)
58-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the visible history labels for Catalan and German.
webview-ui/src/i18n/locales/ca/history.json#L58-L71: replace the English fallback values with Catalan translations.webview-ui/src/i18n/locales/de/history.json#L58-L71: replace the English fallback values with German translations.Users who select either locale receive a mixed-language history UI.
🤖 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 `@webview-ui/src/i18n/locales/ca/history.json` around lines 58 - 71, Replace the English fallback values for the history labels from newFolder through dropHereToRemove in webview-ui/src/i18n/locales/ca/history.json lines 58-71 with Catalan translations, and apply the corresponding German translations to webview-ui/src/i18n/locales/de/history.json lines 58-71. Preserve all translation keys and interpolation syntax, including {{count}} in tasks.webview-ui/src/i18n/__tests__/translation-parity.spec.ts-10-42 (1)
10-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd every new history key to
REQUIRED_HISTORY_KEYS.The list omits
dragTask,dragFolder,createFolder,createFolderDescription,folderNameLabel,folderNameRequired,folderNameTooLong,folderNameInvalidChars,deleteFolder,folderOptions,expandFolder,collapseFolder,create,openTask, andopenFolder. A locale can omit any of these new UI keys and still pass both parity tests. Add them to the required list.🤖 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 `@webview-ui/src/i18n/__tests__/translation-parity.spec.ts` around lines 10 - 42, Extend REQUIRED_HISTORY_KEYS with all omitted history UI keys: dragTask, dragFolder, createFolder, createFolderDescription, folderNameLabel, folderNameRequired, folderNameTooLong, folderNameInvalidChars, deleteFolder, folderOptions, expandFolder, collapseFolder, create, openTask, and openFolder, so parity tests require every new key.webview-ui/src/components/history/__tests__/useTaskOrganizationDnd.spec.tsx-45-48 (1)
45-48: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCentralize the partial DnD event fixtures.
The suite repeats undocumented
as unknown as Drag*Eventcasts through line 221. Move the partial fixtures into typeddragStart,dragOver, anddragEndhelpers. If a double assertion is still needed, document the fieldsuseTaskOrganizationDndreads at the helper.🤖 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 `@webview-ui/src/components/history/__tests__/useTaskOrganizationDnd.spec.tsx` around lines 45 - 48, Centralize the partial DnD event construction used by the useTaskOrganizationDnd tests by adding typed dragStart, dragOver, and dragEnd helpers, then replace the repeated inline as unknown as Drag*Event casts through the suite with those helpers. Document within each helper the event fields read by useTaskOrganizationDnd, retaining any required double assertion only there.Source: Coding guidelines
webview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsx-37-37 (1)
37-37: 📐 Maintainability & Code Quality | 🟡 MinorReplace
window as anyresult storage with typed test state.Move the test result store into a test-scoped, typed variable and update the assignment, reset, and assertion references. This applies to both
__lastResult__and__lastMutationResult__usage.[low_effort_and_medium_reward]
🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsx` at line 37, Replace the window-cast result stores with test-scoped typed variables in both webview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsx:37-37 and webview-ui/src/context/__tests__/ExtensionStateContext.taskOrganization.spec.tsx:34-34. Update all __lastResult__ and __lastMutationResult__ assignments, resets, and assertions to use the typed variables instead of window state.Source: Coding guidelines
webview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.ts-10-13 (1)
10-13: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert delegation with a complete pointerdown fixture.
makePointerEventonly suppliestarget, so the delegatedPointerSensoractivator can fail due to missingisPrimary,button, orownerDocumentfields. Add a primary-left-button pointerdown fixture and assert the delegated result istrue; keep the unavoidable event-shape cast in a documented helper.🤖 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 `@webview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.ts` around lines 10 - 13, Update makePointerEvent in the TaskOrganizationPointerSensor tests to provide a complete primary left-button pointerdown fixture, including isPrimary, button, and ownerDocument on the native event target. Add an assertion that the delegated PointerSensor activator returns true, and retain the unavoidable event-shape cast only within this documented helper.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c7df8fe-79dd-4506-a430-34e028c18dee
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (103)
docs/260730_0002_session_dashboard-crash-debug/232600_ask-full-audit-report.mddocs/260730_0002_session_dashboard-crash-debug/233600_code-multi-branch-report.mddocs/260730_0002_session_dashboard-crash-debug/context-save-260730-2333.mddocs/260730_0002_session_dashboard-crash-debug/test-error-interception.txtdocs/260730_0002_session_dashboard-crash-debug/test-mimo-parallel.txtdocs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txtdocs/260730_0002_session_dashboard-crash-debug/test-task-dnd-ux.txtdocs/260730_0002_session_dashboard-crash-debug/test-unified-shell-resolution.txtdocs/260803_0002_session_6-branch-bug-fix-verification/170635_code-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/170715_code-vitest-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/170924_code-report.mddocs/260803_0002_session_6-branch-bug-fix-verification/171100_code-report.mdknip.jsonpackages/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.tswebview-ui/package.jsonwebview-ui/src/components/history/DeleteFoldersDialog.tsxwebview-ui/src/components/history/DraggableTaskEntry.tsxwebview-ui/src/components/history/FolderNameDialog.tsxwebview-ui/src/components/history/HistoryPreview.tsxwebview-ui/src/components/history/HistoryView.tsxwebview-ui/src/components/history/ManualFolderItem.tsxwebview-ui/src/components/history/PinButton.tsxwebview-ui/src/components/history/PinnedHistoryItem.tsxwebview-ui/src/components/history/SubtaskRow.tsxwebview-ui/src/components/history/TaskGroupItem.tsxwebview-ui/src/components/history/TaskItem.tsxwebview-ui/src/components/history/TaskItemFooter.tsxwebview-ui/src/components/history/TaskOrganizationDndSurface.tsxwebview-ui/src/components/history/TaskOrganizationErrorBoundary.tsxwebview-ui/src/components/history/TaskOrganizationInteractionContext.tsxwebview-ui/src/components/history/TaskOrganizationPointerSensor.tswebview-ui/src/components/history/__tests__/DeleteFoldersDialog.spec.tsxwebview-ui/src/components/history/__tests__/DraggableTaskEntry.spec.tsxwebview-ui/src/components/history/__tests__/HistoryPreview.spec.tsxwebview-ui/src/components/history/__tests__/HistoryPreview.taskOrganization.spec.tsxwebview-ui/src/components/history/__tests__/HistoryView.taskOrganization.spec.tsxwebview-ui/src/components/history/__tests__/ManualFolderItem.spec.tsxwebview-ui/src/components/history/__tests__/PinButton.spec.tsxwebview-ui/src/components/history/__tests__/TaskItemFooter.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationDndSurface.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationErrorBoundary.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.setup.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.spec.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.vitest.config.tswebview-ui/src/components/history/__tests__/useTaskOrganizationDnd.spec.tsxwebview-ui/src/components/history/taskOrganizationModel.tswebview-ui/src/components/history/types.tswebview-ui/src/components/history/useTaskOrganizationDnd.tswebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/context/__tests__/ExtensionStateContext.taskOrganization.spec.tsxwebview-ui/src/i18n/__tests__/translation-parity.spec.tswebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/history.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/history.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/history.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/history.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/history.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/history.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/history.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/history.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/history.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/history.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/history.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/history.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/history.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/history.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/history.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/history.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/history.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/history.jsonwebview-ui/vitest.setup.ts
💤 Files with no reviewable changes (1)
- webview-ui/src/components/history/tests/TaskItemFooter.spec.tsx
🛑 Comments failed to post (1)
docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt (1)
2-7: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail target='docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt' if [ ! -f "$target" ]; then echo "missing: $target" exit 0 fi echo "== file stats ==" wc -l "$target" echo "== matches for Windows user/project paths ==" rg -n -i -E 'C:\\Users\\k1yt|OneDrive\\Projects|file://[^\s\r\n]+' "$target" || true echo "== matches for C:/Users/k1yt ==" rg -n -i -E 'C:/Users/k1yt|file://[^\s\r\n]+' "$target" || true echo "== decoded snippet around local path ==" python3 - <<'PY' from pathlib import Path p=Path('docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt') text=p.read_text(encoding='utf-8-sig') for i,line in enumerate(text.splitlines(),1): if 'k1yt' in line or 'OneDrive' in line: print(f"{i}: {line[:240]!r}") PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1099
🏁 Script executed:
#!/bin/bash set -euo pipefail target='docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt' if [ ! -f "$target" ]; then echo "missing: $target" exit 0 fi echo "== file stats ==" wc -l "$target" echo "== matches for Windows user/project paths ==" rg -n -i -E 'C\\Users\\k1yt|OneDrive\\Projects|C:/Users/k1yt|file://[^\s\r\n]+' "$target" || true echo "== decoded snippet around local path ==" python3 - <<'PY' from pathlib import Path p=Path('docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt') text=p.read_text(encoding='utf-8-sig', errors='replace') for i,line in enumerate(text.splitlines(),1): if 'k1yt' in line or 'OneDrive' in line: print(f"{i}: {line[:260]!r}") PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 468
🏁 Script executed:
#!/bin/bash set -euo pipefail target='docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt' echo "== hex bytes around file start ==" xxd -g 1 -n 16 "$target" echo "== decoded lines containing k1yt/OneDrive/file:// ==" python3 - <<'PY' from pathlib import Path p=Path('docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt') raw=p.read_bytes() for seq in [b'\xff\xfe', b'\xfe\xff', b'\xef\xbb\xbf']: if raw.startswith(seq): enc='utf-16-le' if seq==b'\xff\xfe' else 'utf-16-be' text=raw.decode(enc) break else: text=raw.decode('utf-8', errors='replace') for i,line in enumerate(text.splitlines(),1): if ('k1yt' in line) or ('OneDrive' in line) or ('file://' in line): print(f'{i}: {line[:300]!r}') PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 234
Sensitive Data Exposure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Remove developer-specific filesystem paths from the crash log.
C:\Users\k1yt\OneDrive\Projects\ZooCode\srcand localfile://path text disclose the Windows account name and filesystem layout. Replace those paths with<repo-root>or sanitize the artifact before merging.🤖 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 `@docs/260730_0002_session_dashboard-crash-debug/test-strict-reasoning.txt` around lines 2 - 7, Sanitize the crash-log artifact in test-strict-reasoning.txt by removing the developer-specific Windows path, account name, and local file:// path details. Replace repository-root references with <repo-root> and remove or generalize any remaining machine-specific filesystem text while preserving the relevant error output.
5dc3461 to
ec9d71e
Compare
ec9d71e to
402f670
Compare
Patch coverage checks were blocking 10+ PRs with 80%/70% thresholds. Changed to informational: true so patch coverage is reported but not a required status check.
…union of both sides)
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (2)
src/core/task-persistence/TaskOrganizationStore.ts (2)
699-710: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not mutate
childMapduring descendant traversal.
stackaliases the array stored inchildMap. Calls topop()andpush()drain and pollute that shared array, so later missing members can lose descendants during the same reconciliation.Initialize
stackfrom a copy and track visited IDs.🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 699 - 710, Update the descendant traversal in the missing-ID reconciliation around the surviving calculation so the traversal stack is a copy of each childMap entry rather than the stored array, preventing pop/push operations from mutating childMap. Track visited IDs during traversal to avoid revisiting nodes while still collecting visible descendants for each missing ID.
53-53: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDeclare optional
getAll()ontaskHistory.The option and private-field types still omit
getAll(), although both reconciliation paths probe and invoke it. Declare the optional method with itsHistoryItem[]return type.Also applies to: 77-77, 621-622
🤖 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/task-persistence/TaskOrganizationStore.ts` at line 53, Update the taskHistory type declarations, including the option and private-field types used by TaskOrganizationStore, to declare an optional getAll() method returning HistoryItem[]. Keep the existing optional get(taskId) declaration unchanged and ensure both reconciliation paths can type-check their getAll() usage.
🧹 Nitpick comments (6)
webview-ui/src/i18n/__tests__/translation-parity.spec.ts (1)
65-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe second test cannot fail independently of the first.
localeRequiredKeysat lines 79-81 is the intersection of the locale's keys withREQUIRED_HISTORY_KEYS. It can only differ fromexpectedShapeby missing an entry, and the first test already fails in that case. The second test therefore adds no coverage.Both tests also accept an empty string, because
toBeDefined()passes for"". An empty value produces the same broken UI that the comment at lines 5-9 describes. Convert the second test into a value check.♻️ Proposed replacement
- it("has identical key shape across all locales for the required task-organization keys", () => { - // Locales may carry additional legacy keys not present in en. The shape - // contract that matters for this feature is that every locale exposes - // the SAME set of required task-organization keys. Sort the required - // list once and assert every locale's filtered shape equals it. - const locales = fs - .readdirSync(LOCALES_DIR) - .filter((name) => fs.statSync(path.join(LOCALES_DIR, name)).isDirectory()) - - const expectedShape = [...REQUIRED_HISTORY_KEYS].sort() - - for (const locale of locales) { - const filePath = path.join(LOCALES_DIR, locale, "history.json") - const history = JSON.parse(fs.readFileSync(filePath, "utf-8")) - const localeRequiredKeys = Object.keys(history) - .filter((k) => REQUIRED_HISTORY_KEYS.includes(k)) - .sort() - - expect( - localeRequiredKeys, - `Key shape mismatch in ${locale}/history.json: missing=${expectedShape.filter( - (k) => !localeRequiredKeys.includes(k), - )}`, - ).toEqual(expectedShape) - } - }) + it("resolves every required key to a non-empty string in every locale", () => { + for (const locale of readLocales()) { + const history = JSON.parse(fs.readFileSync(path.join(LOCALES_DIR, locale, "history.json"), "utf-8")) + + for (const key of REQUIRED_HISTORY_KEYS) { + expect(history[key], `Empty value for "${key}" in ${locale}/history.json`).toEqual( + expect.stringMatching(/\S/), + ) + } + } + })Add the shared helper so both tests reuse one directory scan:
function readLocales(): string[] { return fs.readdirSync(LOCALES_DIR).filter((name) => fs.statSync(path.join(LOCALES_DIR, name)).isDirectory()) }🤖 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 `@webview-ui/src/i18n/__tests__/translation-parity.spec.ts` around lines 65 - 90, Replace the redundant key-shape test around REQUIRED_HISTORY_KEYS with a value validation that asserts each required locale entry is non-empty, not merely defined. Add a shared readLocales helper for the directory scan and update both tests to reuse it, preserving the existing required-key coverage while ensuring empty translations fail.src/core/task-persistence/TaskOrganizationStore.ts (1)
298-301: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplain the double assertion at the cast site.
Document why a future-schema value must be stored as
TaskOrganizationStateV1, or replace the cast with a typed read-only future-state representation. The nearby behavior comment does not explain this double assertion.As per coding guidelines, “Use double assertions only as a last resort and explain them with a comment next to the cast.”
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 298 - 301, Update the future-schema branch in the task organization loading method around the data cast: either replace the double assertion with a typed read-only future-state representation, or retain it only with a nearby comment explaining why the future-schema value must be stored as TaskOrganizationStateV1. Keep the existing warning, assignment behavior, and early return unchanged.Source: Coding guidelines
codecov.yml (1)
15-22: 📐 Maintainability & Code Quality | 🔵 TrivialPatch coverage no longer blocks.
Both patch statuses are now
informational: true, so new untested code cannot fail the check. The project statuses at Lines 5-14 still ratchet total coverage, which limits the risk. Confirm this relaxation is intended for the long term, since this PR adds a large amount of new webview code.🤖 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` around lines 15 - 22, Confirm whether making both patch coverage statuses informational in the codecov configuration is an intentional long-term policy; if not, restore blocking patch coverage for default and webview-patch while preserving the existing project-level coverage thresholds.webview-ui/src/components/history/HistoryView.tsx (3)
238-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe folder-target branch is unreachable.
canCreateFolderFromSelectionrequiresselectedFolderIds.length === 0, andhandleCreateFolderFromSelectionreturns early otherwise. The dialog therefore only opens with an empty folder selection, so thefolderIdtargets built here are always absent. Either allow folder selection incanCreateFolderFromSelectionor delete this branch.🤖 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 `@webview-ui/src/components/history/HistoryView.tsx` around lines 238 - 252, The folder-target construction in handleConfirmSelectionFolderName is unreachable because canCreateFolderFromSelection and handleCreateFolderFromSelection reject nonempty selectedFolderIds. Update the folder-creation flow to allow folder selections, preserving the existing selectedFolderIds-to-TaskOrganizationTargetV1 mapping and clearing behavior after success.
831-1149: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider extracting the shared header and list markup.
HistoryViewBaselineFallbackduplicates roughly 270 lines of search controls, sort controls, selection header, and Virtuoso rendering fromHistoryViewInner. Future changes must be applied twice. Extract the shared header and the baseline list into small components that both renderers use.🤖 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 `@webview-ui/src/components/history/HistoryView.tsx` around lines 831 - 1149, Extract the duplicated search/sort controls, workspace selector, selection header, and Virtuoso task/group rendering from HistoryViewBaselineFallback and HistoryViewInner into shared components. Reuse those components in both renderers while preserving their existing props, callbacks, selection behavior, and rendering differences; keep renderer-specific dialogs and layout concerns in the parent components.
57-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
groupsparameter.
buildGroupDndDataignoresgroupsand discards it withvoid groups. Drop the parameter and update both call sites (Line 393 and Line 677).♻️ Proposed fix
-function buildGroupDndData(group: TaskGroup, groups: TaskGroup[], folderId?: string): DndItemData { +function buildGroupDndData(group: TaskGroup, folderId?: string): DndItemData { const rootId = group.parent.id const hasChildren = group.subtasks.length > 0 const target: TaskOrganizationTargetV1 = hasChildren ? { kind: "autoGroup", rootTaskId: rootId } : { kind: "task", taskId: rootId } - void groups return {🤖 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 `@webview-ui/src/components/history/HistoryView.tsx` around lines 57 - 69, Remove the unused groups parameter and the void groups statement from buildGroupDndData, then update both callers around the HistoryView usages to pass only the remaining required arguments.
🤖 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: Convert codecov.yml from CRLF to LF line endings while preserving its
coverage configuration. Add a .gitattributes rule for the file only if needed to
prevent editors from reintroducing CRLF.
In `@src/core/task-persistence/TaskOrganizationStore.ts`:
- Around line 281-292: Update the recovery paths in the task-organization
loading method, including the catch block and the result.success validation
branch, so the live file is removed or moved only after quarantine succeeds.
Perform this cleanup under the same locking protocol used by safeUpdateJson,
ensuring the next mutation can recreate valid state from the empty state.
- Around line 320-324: Update the concurrent-modification branch in
TaskOrganizationStore’s mutate flow to reload the committed state after
detecting current.revision >= next.revision, then return TASK_ORG/CONFLICT/002
using the reloaded current revision instead of propagating
TASK_ORG/PERSISTENCE/005 with the stale local revision. Preserve the existing
same-or-newer revision check and reconciliation behavior.
- Around line 584-612: Update resolveTarget and resolveUnit to validate target
existence before returning canonical targets or member IDs: when taskHistory is
available, reject unknown task IDs and autoGroup rootTaskIds; always reject
folder targets whose folderId is absent from state.folders. Ensure invalid
targets cannot proceed to state mutation, using the existing invalid-target
handling convention.
In `@src/eslint-suppressions.json`:
- Around line 187-190: Replace the newly introduced any usages in the Mimo,
OpenCode Go, and Qwen Code native-tools tests with precise test doubles or
unknown plus appropriate type guards, then remove the added suppression entries
from src/eslint-suppressions.json at lines 187-190, 242-245, and 257-260,
restoring each prior count without increasing any suppression count.
In `@src/utils/safeWriteJson.ts`:
- Around line 313-395: Extract the duplicated atomic temp-write, backup, commit,
cleanup, and rollback logic into a private helper named _atomicCommitJson that
accepts the resolved file path, data, and prettyPrint option. Move the shared
implementation from safeWriteJson into this helper, excluding lock acquisition
and release, then replace the inline block in both safeWriteJson and
safeUpdateJson with calls to _atomicCommitJson while preserving existing error
and rollback behavior.
- Around line 301-311: Update the read-error handling in safeUpdateJson to
rethrow every failure whose error code is not ENOENT, regardless of whether the
thrown value is an Error instance. Preserve the existing missing-file behavior
for ENOENT and ensure non-Error read failures cannot leave fileExisted false and
continue to updater(current).
In
`@webview-ui/src/components/history/__tests__/HistoryPreview.taskOrganization.spec.tsx`:
- Around line 438-451: Align the test named “opens the folder-name dialog when
card A is dropped on card B” with its actual behavior: either rename it to
describe validating draggable/droppable metadata and the dialog’s closed initial
state, or update the test to perform a real drop through the DnD surface and
assert the folder-name dialog opens.
- Around line 453-458: Update the “cancel posts nothing” test to use the
createFolder mock initialized in beforeEach instead of the optional call on the
fallback org object. Perform the component’s cancel interaction before asserting
that createFolder was not called, so the test verifies cancellation behavior
rather than only the initial render.
In
`@webview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsx`:
- Line 37: Replace the window-based __lastResult__ channel in the test harness
with a module-level variable typed to the mutation result, and update all four
assignments and assertion reads to use it. Remove every window as any cast while
preserving the existing result assertions.
In
`@webview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.ts`:
- Around line 114-122: Strengthen the assertion in the “delegates to
PointerSensor for non-interactive targets” test by verifying the concrete result
expected from PointerSensor for the valid primary-button event, rather than only
checking that the result is boolean. Alternatively, spy on the base
PointerSensor handler and assert it receives the event; keep the existing
non-interactive target setup unchanged.
In `@webview-ui/src/components/history/DeleteFoldersDialog.tsx`:
- Around line 42-47: Update the AlertDialogDescription usage in
DeleteFoldersDialog so it no longer renders the two div blocks inside Radix’s
default p element; either move those blocks outside the description or configure
the description with asChild and a div wrapper while preserving both translated
messages and their styling.
In `@webview-ui/src/components/history/HistoryView.tsx`:
- Around line 416-419: Use the canonical pin target shape for both history
pin-button sections: in webview-ui/src/components/history/HistoryView.tsx lines
416-419, derive the target from memberGroup.subtasks.length and reuse it for
isPinned and togglePin; at lines 696-698, derive the unfiled target from
group.subtasks.length, preferably reusing dndData.target. Update the relevant
pin call sites without changing unrelated behavior.
- Around line 649-654: Update the onTogglePin callback in HistoryView to prefix
the togglePin call with void, matching other call sites and satisfying the
no-floating-promise rule.
In `@webview-ui/src/i18n/locales/ca/history.json`:
- Around line 58-71: Translate the English values for the referenced history
keys in the Catalan locale, including newFolder, folderNamePlaceholder,
renameFolder, removeFromFolder, deleteEmptyFolder, pin, unpin, pinLimitReached,
pinned, folder, unfiled, tasks, dragToOrganize, and dropHereToRemove. Preserve
all keys and the {{count}} interpolation while leaving unrelated entries
unchanged.
- Line 68: Add Catalan plural variants for the history tasks translation by
replacing the single tasks entry with tasks_one and tasks_other, following the
nearby count-key convention and ensuring singular counts render “1 task” while
plural counts render “{{count}} tasks”.
---
Duplicate comments:
In `@src/core/task-persistence/TaskOrganizationStore.ts`:
- Around line 699-710: Update the descendant traversal in the missing-ID
reconciliation around the surviving calculation so the traversal stack is a copy
of each childMap entry rather than the stored array, preventing pop/push
operations from mutating childMap. Track visited IDs during traversal to avoid
revisiting nodes while still collecting visible descendants for each missing ID.
- Line 53: Update the taskHistory type declarations, including the option and
private-field types used by TaskOrganizationStore, to declare an optional
getAll() method returning HistoryItem[]. Keep the existing optional get(taskId)
declaration unchanged and ensure both reconciliation paths can type-check their
getAll() usage.
---
Nitpick comments:
In `@codecov.yml`:
- Around line 15-22: Confirm whether making both patch coverage statuses
informational in the codecov configuration is an intentional long-term policy;
if not, restore blocking patch coverage for default and webview-patch while
preserving the existing project-level coverage thresholds.
In `@src/core/task-persistence/TaskOrganizationStore.ts`:
- Around line 298-301: Update the future-schema branch in the task organization
loading method around the data cast: either replace the double assertion with a
typed read-only future-state representation, or retain it only with a nearby
comment explaining why the future-schema value must be stored as
TaskOrganizationStateV1. Keep the existing warning, assignment behavior, and
early return unchanged.
In `@webview-ui/src/components/history/HistoryView.tsx`:
- Around line 238-252: The folder-target construction in
handleConfirmSelectionFolderName is unreachable because
canCreateFolderFromSelection and handleCreateFolderFromSelection reject nonempty
selectedFolderIds. Update the folder-creation flow to allow folder selections,
preserving the existing selectedFolderIds-to-TaskOrganizationTargetV1 mapping
and clearing behavior after success.
- Around line 831-1149: Extract the duplicated search/sort controls, workspace
selector, selection header, and Virtuoso task/group rendering from
HistoryViewBaselineFallback and HistoryViewInner into shared components. Reuse
those components in both renderers while preserving their existing props,
callbacks, selection behavior, and rendering differences; keep renderer-specific
dialogs and layout concerns in the parent components.
- Around line 57-69: Remove the unused groups parameter and the void groups
statement from buildGroupDndData, then update both callers around the
HistoryView usages to pass only the remaining required arguments.
In `@webview-ui/src/i18n/__tests__/translation-parity.spec.ts`:
- Around line 65-90: Replace the redundant key-shape test around
REQUIRED_HISTORY_KEYS with a value validation that asserts each required locale
entry is non-empty, not merely defined. Add a shared readLocales helper for the
directory scan and update both tests to reuse it, preserving the existing
required-key coverage while ensuring empty translations fail.
🪄 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: d8ce4f7d-1db7-4d42-b668-45eadc8deb97
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (92)
codecov.ymlknip.jsonpackages/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.tswebview-ui/package.jsonwebview-ui/src/components/history/DeleteFoldersDialog.tsxwebview-ui/src/components/history/DraggableTaskEntry.tsxwebview-ui/src/components/history/FolderNameDialog.tsxwebview-ui/src/components/history/HistoryPreview.tsxwebview-ui/src/components/history/HistoryView.tsxwebview-ui/src/components/history/ManualFolderItem.tsxwebview-ui/src/components/history/PinButton.tsxwebview-ui/src/components/history/PinnedHistoryItem.tsxwebview-ui/src/components/history/SubtaskRow.tsxwebview-ui/src/components/history/TaskGroupItem.tsxwebview-ui/src/components/history/TaskItem.tsxwebview-ui/src/components/history/TaskItemFooter.tsxwebview-ui/src/components/history/TaskOrganizationDndSurface.tsxwebview-ui/src/components/history/TaskOrganizationErrorBoundary.tsxwebview-ui/src/components/history/TaskOrganizationInteractionContext.tsxwebview-ui/src/components/history/TaskOrganizationPointerSensor.tswebview-ui/src/components/history/__tests__/DeleteFoldersDialog.spec.tsxwebview-ui/src/components/history/__tests__/DraggableTaskEntry.spec.tsxwebview-ui/src/components/history/__tests__/HistoryPreview.spec.tsxwebview-ui/src/components/history/__tests__/HistoryPreview.taskOrganization.spec.tsxwebview-ui/src/components/history/__tests__/HistoryView.taskOrganization.spec.tsxwebview-ui/src/components/history/__tests__/ManualFolderItem.spec.tsxwebview-ui/src/components/history/__tests__/PinButton.spec.tsxwebview-ui/src/components/history/__tests__/TaskItemFooter.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationDndSurface.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationErrorBoundary.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationInteractionContext.spec.tsxwebview-ui/src/components/history/__tests__/TaskOrganizationPointerSensor.spec.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.setup.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.spec.tswebview-ui/src/components/history/__tests__/taskOrganizationModel.vitest.config.tswebview-ui/src/components/history/__tests__/useTaskOrganizationDnd.spec.tsxwebview-ui/src/components/history/taskOrganizationModel.tswebview-ui/src/components/history/types.tswebview-ui/src/components/history/useTaskOrganizationDnd.tswebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/context/__tests__/ExtensionStateContext.taskOrganization.spec.tsxwebview-ui/src/i18n/__tests__/translation-parity.spec.tswebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/history.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/history.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/history.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/history.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/history.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/history.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/history.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/history.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/history.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/history.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/history.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/history.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/history.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/history.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/history.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/history.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/history.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/history.jsonwebview-ui/vitest.setup.ts
💤 Files with no reviewable changes (1)
- webview-ui/src/components/history/tests/TaskItemFooter.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (79)
- webview-ui/src/i18n/locales/ja/chat.json
- webview-ui/src/i18n/locales/ca/chat.json
- webview-ui/src/i18n/locales/vi/chat.json
- webview-ui/src/components/history/tests/taskOrganizationModel.vitest.config.ts
- webview-ui/package.json
- src/core/task-persistence/index.ts
- webview-ui/src/i18n/locales/es/chat.json
- webview-ui/src/i18n/locales/en/chat.json
- webview-ui/src/i18n/locales/nl/chat.json
- webview-ui/src/i18n/locales/fr/chat.json
- webview-ui/src/i18n/locales/pt-BR/chat.json
- webview-ui/src/i18n/locales/ru/chat.json
- webview-ui/src/i18n/locales/ko/chat.json
- packages/types/src/index.ts
- webview-ui/src/i18n/locales/hi/chat.json
- src/core/webview/taskOrganizationMessageHandler.ts
- knip.json
- webview-ui/src/components/history/TaskOrganizationErrorBoundary.tsx
- webview-ui/src/i18n/locales/pl/chat.json
- webview-ui/src/i18n/locales/zh-CN/chat.json
- webview-ui/src/i18n/locales/es/history.json
- webview-ui/src/i18n/locales/hi/history.json
- webview-ui/src/i18n/locales/ja/history.json
- webview-ui/src/components/history/FolderNameDialog.tsx
- webview-ui/src/i18n/locales/nl/history.json
- webview-ui/src/components/history/tests/PinButton.spec.tsx
- webview-ui/src/i18n/locales/id/chat.json
- webview-ui/src/i18n/locales/zh-TW/chat.json
- webview-ui/src/components/history/tests/ManualFolderItem.spec.tsx
- webview-ui/src/i18n/locales/pl/history.json
- webview-ui/src/components/history/TaskItem.tsx
- src/core/webview/webviewMessageHandler.ts
- webview-ui/src/components/history/TaskGroupItem.tsx
- packages/types/src/vscode-extension-host.ts
- webview-ui/src/components/history/DraggableTaskEntry.tsx
- webview-ui/src/components/history/tests/taskOrganizationModel.setup.ts
- webview-ui/src/i18n/locales/zh-TW/history.json
- webview-ui/src/components/history/TaskOrganizationDndSurface.tsx
- webview-ui/src/i18n/locales/en/history.json
- webview-ui/src/i18n/locales/vi/history.json
- webview-ui/src/components/history/tests/HistoryPreview.spec.tsx
- webview-ui/src/i18n/locales/de/history.json
- webview-ui/src/i18n/locales/ru/history.json
- webview-ui/src/components/history/TaskItemFooter.tsx
- webview-ui/src/i18n/locales/ko/history.json
- webview-ui/src/components/history/tests/DraggableTaskEntry.spec.tsx
- webview-ui/src/i18n/locales/id/history.json
- src/core/webview/tests/taskOrganizationMessageHandler.spec.ts
- webview-ui/src/components/history/tests/useTaskOrganizationDnd.spec.tsx
- webview-ui/src/i18n/locales/fr/history.json
- webview-ui/src/i18n/locales/it/history.json
- src/shared/globalFileNames.ts
- src/core/webview/tests/ClineProvider.taskHistory.spec.ts
- webview-ui/src/i18n/locales/de/chat.json
- webview-ui/src/components/history/tests/DeleteFoldersDialog.spec.tsx
- webview-ui/src/i18n/locales/tr/chat.json
- webview-ui/src/i18n/locales/it/chat.json
- webview-ui/src/components/history/TaskOrganizationPointerSensor.ts
- webview-ui/src/components/history/SubtaskRow.tsx
- webview-ui/src/components/history/PinnedHistoryItem.tsx
- webview-ui/vitest.setup.ts
- webview-ui/src/i18n/locales/pt-BR/history.json
- packages/types/src/task-organization.ts
- webview-ui/src/components/history/tests/HistoryView.taskOrganization.spec.tsx
- webview-ui/src/i18n/locales/zh-CN/history.json
- webview-ui/src/components/history/PinButton.tsx
- webview-ui/src/components/history/TaskOrganizationInteractionContext.tsx
- webview-ui/src/components/history/tests/TaskOrganizationErrorBoundary.spec.tsx
- webview-ui/src/context/tests/ExtensionStateContext.taskOrganization.spec.tsx
- webview-ui/src/components/history/useTaskOrganizationDnd.ts
- webview-ui/src/components/history/taskOrganizationModel.ts
- webview-ui/src/context/ExtensionStateContext.tsx
- webview-ui/src/components/history/tests/taskOrganizationModel.spec.ts
- webview-ui/src/i18n/locales/tr/history.json
- webview-ui/src/components/history/ManualFolderItem.tsx
- webview-ui/src/components/history/tests/TaskOrganizationDndSurface.spec.tsx
- webview-ui/src/components/history/types.ts
- src/core/webview/ClineProvider.ts
- webview-ui/src/components/history/HistoryPreview.tsx
| comment: | ||
| layout: "diff, flags, components" | ||
| behavior: default | ||
| coverage: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Convert the file to LF line endings.
YAMLlint reports wrong new line character: expected \n. The file uses CRLF. Rewrite it with LF endings, and add a .gitattributes rule if editors keep reintroducing CRLF.
🧰 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, Convert codecov.yml from CRLF to LF line endings
while preserving its coverage configuration. Add a .gitattributes rule for the
file only if needed to prevent editors from reintroducing CRLF.
Source: Linters/SAST tools
| } catch (err) { | ||
| await this.quarantine(filePath, raw) | ||
| console.warn("[TaskOrganizationStore] Organization file was malformed and has been quarantined.") | ||
| this.state = createEmptyTaskOrganizationState(this.now) | ||
| return | ||
| } | ||
|
|
||
| const result = taskOrganizationStateSchema.safeParse(parsed) | ||
| if (!result.success) { | ||
| await this.quarantine(filePath, raw) | ||
| console.warn("[TaskOrganizationStore] Organization file failed validation and has been quarantined.") | ||
| this.state = createEmptyTaskOrganizationState(this.now) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove the malformed live file after quarantine.
quarantine() only copies the malformed content. The next mutation calls safeUpdateJson(), which parses the same malformed live file and fails before it can write the empty replacement state.
Move the live file into quarantine, or remove it only after a successful quarantine copy. Use the same locking protocol for this recovery path. This lets the next mutation recreate valid state.
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 281 - 292,
Update the recovery paths in the task-organization loading method, including the
catch block and the result.success validation branch, so the live file is
removed or moved only after quarantine succeeds. Perform this cleanup under the
same locking protocol used by safeUpdateJson, ensuring the next mutation can
recreate valid state from the empty state.
| if (current && current.revision >= next.revision) { | ||
| // Another process wrote the same or a newer revision while we | ||
| // held the lock. Same-revision writes lose: two processes that | ||
| // both computed `next` from the same base must not both commit. | ||
| throw this.createError("TASK_ORG/PERSISTENCE/005", "Concurrent modification detected.") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return a revision conflict with current state.
If another instance commits first, this branch throws a persistence error. mutate() then returns TASK_ORG/PERSISTENCE/005 and the stale local this.state.revision.
Reload the committed state after this condition and return TASK_ORG/CONFLICT/002 with the current revision. The webview can then reconcile its optimistic mutation.
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 320 - 324,
Update the concurrent-modification branch in TaskOrganizationStore’s mutate flow
to reload the committed state after detecting current.revision >= next.revision,
then return TASK_ORG/CONFLICT/002 using the reloaded current revision instead of
propagating TASK_ORG/PERSISTENCE/005 with the stale local revision. Preserve the
existing same-or-newer revision check and reconciliation behavior.
| private resolveTarget(target: TaskOrganizationTargetV1): TaskOrganizationTargetV1 { | ||
| if (target.kind === "task" || target.kind === "folder") { | ||
| return target | ||
| } | ||
| // autoGroup: resolve closure and return canonical root target. | ||
| const closure = this.resolveTaskClosure(target.rootTaskId) | ||
| return { kind: "autoGroup", rootTaskId: closure.rootId } | ||
| } | ||
|
|
||
| private resolveUnit(target: TaskOrganizationTargetV1): string[] { | ||
| switch (target.kind) { | ||
| case "task": { | ||
| // Resolve any known task through its closure. This covers both | ||
| // children and roots that have children, so dragging any group | ||
| // member moves the whole group together. | ||
| if (this.taskHistory?.get(target.taskId)) { | ||
| return this.resolveTaskClosure(target.taskId).ids | ||
| } | ||
| return [target.taskId] | ||
| } | ||
| case "folder": { | ||
| const folder = this.state.folders.find((f) => f.folderId === target.folderId) | ||
| return folder ? [...folder.taskIds] : [] | ||
| } | ||
| case "autoGroup": | ||
| return this.resolveTaskClosure(target.rootTaskId).ids | ||
| default: | ||
| return [] | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject targets that do not exist.
resolveTarget() accepts every task and folder target. resolveUnit() also converts an unknown task into a persisted ID. A valid-shape IPC mutation can therefore create phantom pins or folder members until a later reconciliation happens.
When taskHistory is available, reject unknown task and auto-group roots. Always reject unknown folder IDs before mutating state.
🤖 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/task-persistence/TaskOrganizationStore.ts` around lines 584 - 612,
Update resolveTarget and resolveUnit to validate target existence before
returning canonical targets or member IDs: when taskHistory is available, reject
unknown task IDs and autoGroup rootTaskIds; always reject folder targets whose
folderId is absent from state.folders. Ensure invalid targets cannot proceed to
state mutation, using the existing invalid-target handling convention.
| "api/providers/__tests__/mimo.spec.ts": { | ||
| "@typescript-eslint/no-explicit-any": { | ||
| "count": 29 | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not increase ESLint suppression counts.
Replace the new any usages with precise test doubles or unknown plus type guards. Restore the prior counts.
src/eslint-suppressions.json#L187-L190: remove the added Mimo test suppression.src/eslint-suppressions.json#L242-L245: remove the added OpenCode Go test suppression.src/eslint-suppressions.json#L257-L260: remove the added Qwen Code native-tools test suppression.
As per coding guidelines, “Suppression counts in src/eslint-suppressions.json must never increase.”
📍 Affects 1 file
src/eslint-suppressions.json#L187-L190(this comment)src/eslint-suppressions.json#L242-L245src/eslint-suppressions.json#L257-L260
🤖 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/eslint-suppressions.json` around lines 187 - 190, Replace the newly
introduced any usages in the Mimo, OpenCode Go, and Qwen Code native-tools tests
with precise test doubles or unknown plus appropriate type guards, then remove
the added suppression entries from src/eslint-suppressions.json at lines
187-190, 242-245, and 257-260, restoring each prior count without increasing any
suppression count.
Source: Coding guidelines
| <AlertDialogDescription className="text-vscode-foreground"> | ||
| <div className="mb-2">{t("history:confirmDeleteFolders", { count: folderCount })}</div> | ||
| <div className="text-vscode-editor-foreground bg-vscode-editor-background p-2 rounded text-sm"> | ||
| {t("history:deleteFoldersTasksPreserved")} | ||
| </div> | ||
| </AlertDialogDescription> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the rendered element for AlertDialogDescription in the shared UI wrapper.
fd -a 'alert-dialog.tsx$' webview-ui/src | xargs -r rg -n -C4 'AlertDialogDescription'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 513
🏁 Script executed:
#!/bin/bash
set -e
echo "== candidate files =="
fd -a 'alert-dialog.tsx$|DeleteFoldersDialog.tsx$' webview-ui/src
echo
echo "== dialog implementation around Description =="
fd -a 'alert-dialog.tsx$' webview-ui/src | xargs -r sed -n '60,90p'
echo
echo "== DeleteFoldersDialog around lines 30-80 =="
fd -a 'DeleteFoldersDialog.tsx$' webview-ui/src | xargs -r sed -n '30,80p'
echo
echo "== package metadata versions =="
for f in package.json webview-ui/package.json pnpm-lock.yaml; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n '"`@radix-ui/react-alert-dialog`"|version|`@radix-ui/react`' "$f" | head -40 || true
fi
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 4954
🌐 Web query:
React validateDOMNesting p div block elements table caption tbody
💡 Result:
The React validateDOMNesting warning is a developer-facing tool designed to flag HTML structure violations that can cause reconciliation errors or inconsistencies between how browsers interpret markup and how React expects the DOM to behave [1][2][3]. For nesting block elements like div inside p tags: The HTML5 specification restricts the p element to contain only phrasing content (e.g., span, em, text), prohibiting block-level elements like div, h1-h6, table, or ul [1][4][5]. React issues this warning because placing a block element inside a p tag creates invalid DOM nesting [1][5]. Fix: Replace the offending block-level tag (e.g., div) with an inline tag (e.g., span) or refactor your component hierarchy so that the div is no longer a descendant of the p element [1][5]. If you are using a third-party library component (like MUI's Typography) that defaults to a p tag, use its component prop (e.g., component="div") to change the rendered HTML element [6][5]. For table, caption, and tbody structure: React warns about missing tbody or incorrect table children (like tr directly under table) because browsers automatically inject these tags during HTML parsing [2][7][3]. If React does not explicitly include them in its virtual DOM, the mismatch between the rendered React tree and the actual browser-modified DOM can break updates or cause hydration failures [7][3][8]. Fix: Explicitly add the necessary tags (e.g., wrap your tr elements in a tbody) to your JSX [2][9][3]. This ensures the React tree perfectly matches the structure the browser will generate, preventing reconciliation issues [7][8]. These warnings are intentional and help maintain DOM stability, especially when using server-side rendering (SSR) or hydration, where DOM mismatches are more critical [7][10][8]. Top results: [1][2][7][9][5][3]
Citations:
- 1: https://stackabuse.com/bytes/fixing-div-cannot-appear-as-a-descendant-of-p-warning-in-react/
- 2: Warning: validateDOMNesting(...): <tr> cannot appear as a child of <table> react/react#5652
- 3: Warning: validateDOMNesting(...): <tr> cannot appear as a child of <table> react/react#5652
- 4: https://stackoverflow.com/questions/41928567/div-cannot-appear-as-a-descendant-of-p
- 5: https://bobbyhadz.com/blog/warning-validatedomnesting-div-cannot-appear-as-descendant-of-p-in-react
- 6: https://stackoverflow.com/questions/73291309/warning-validatedomnesting-p-cannot-appear-as-a-descendant-of-p
- 7: validateDOMNesting incorrectly validates table children react/react#12674
- 8: validateDOMNesting warns when optional tags are missing react/react#4464
- 9: https://stackoverflow.com/questions/55820297/how-to-fix-warning-validatedomnesting-div-cannot-appear-as-a-child-of
- 10: Bug:
validateDOMNestingHydration failed react/react#24519
Avoid block elements inside AlertDialogDescription.
AlertDialogDescription passes through to Radix’s <p> element, so the two <div> children create invalid <p> nesting. Move the blocks outside the description, or render the description as a <div> with asChild.
🤖 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 `@webview-ui/src/components/history/DeleteFoldersDialog.tsx` around lines 42 -
47, Update the AlertDialogDescription usage in DeleteFoldersDialog so it no
longer renders the two div blocks inside Radix’s default p element; either move
those blocks outside the description or configure the description with asChild
and a div wrapper while preserving both translated messages and their styling.
| showPin | ||
| isPinned={isPinned({ kind: "task", taskId: rootId })} | ||
| canPin={canPin} | ||
| onTogglePin={() => void togglePin({ kind: "task", taskId: rootId })} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pin targets ignore the canonical unit kind in both list sections. buildGroupDndData (Line 60) and selectedTaskTargets (Line 218) map a group with subtasks to { kind: "autoGroup", rootTaskId }, but both pin call sites always build { kind: "task", taskId }. Pin state then disagrees between the drag/selection paths and the pin button, and a unit can hold two differently shaped pins. Build the target once from the group shape and reuse it.
webview-ui/src/components/history/HistoryView.tsx#L416-L419: derive the folder-member pin target frommemberGroup.subtasks.lengthand pass it to bothisPinnedandtogglePin.webview-ui/src/components/history/HistoryView.tsx#L696-L698: derive the unfiled pin target fromgroup.subtasks.length, for example by reusing thetargetfield ofdndDatacomputed at Line 677.
📍 Affects 1 file
webview-ui/src/components/history/HistoryView.tsx#L416-L419(this comment)webview-ui/src/components/history/HistoryView.tsx#L696-L698
🤖 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 `@webview-ui/src/components/history/HistoryView.tsx` around lines 416 - 419,
Use the canonical pin target shape for both history pin-button sections: in
webview-ui/src/components/history/HistoryView.tsx lines 416-419, derive the
target from memberGroup.subtasks.length and reuse it for isPinned and togglePin;
at lines 696-698, derive the unfiled target from group.subtasks.length,
preferably reusing dndData.target. Update the relevant pin call sites without
changing unrelated behavior.
| showPin | ||
| isPinned={isPinned({ kind: "task", taskId: item.id })} | ||
| canPin={canPin} | ||
| onTogglePin={() => togglePin({ kind: "task", taskId: item.id })} | ||
| className="m-2" | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not leave a floating promise on this pin toggle.
Every other call site uses void togglePin(...). This call site omits it. Add void for consistency and to satisfy the no-floating-promise rule.
♻️ Proposed fix
- onTogglePin={() => togglePin({ kind: "task", taskId: item.id })}
+ onTogglePin={() => void togglePin({ kind: "task", taskId: item.id })}As per coding guidelines: "Do not leave floating promises; use void, await, or .catch() as appropriate."
📝 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.
| showPin | |
| isPinned={isPinned({ kind: "task", taskId: item.id })} | |
| canPin={canPin} | |
| onTogglePin={() => togglePin({ kind: "task", taskId: item.id })} | |
| className="m-2" | |
| /> | |
| showPin | |
| isPinned={isPinned({ kind: "task", taskId: item.id })} | |
| canPin={canPin} | |
| onTogglePin={() => void togglePin({ kind: "task", taskId: item.id })} | |
| className="m-2" | |
| /> |
🤖 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 `@webview-ui/src/components/history/HistoryView.tsx` around lines 649 - 654,
Update the onTogglePin callback in HistoryView to prefix the togglePin call with
void, matching other call sites and satisfying the no-floating-promise rule.
Source: Coding guidelines
| "newFolder": "New Folder", | ||
| "folderNamePlaceholder": "Enter folder name...", | ||
| "renameFolder": "Rename", | ||
| "removeFromFolder": "Remove from Folder", | ||
| "deleteEmptyFolder": "Delete Folder", | ||
| "pin": "Pin", | ||
| "unpin": "Unpin", | ||
| "pinLimitReached": "Maximum 3 pinned items allowed", | ||
| "pinned": "Pinned", | ||
| "folder": "Folder", | ||
| "tasks": "{{count}} tasks", | ||
| "unfiled": "Unfiled", | ||
| "dragToOrganize": "Drag to organize", | ||
| "dropHereToRemove": "Drop here to remove from folder", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which of these keys the webview actually references, and compare locales.
for key in newFolder folderNamePlaceholder renameFolder removeFromFolder deleteEmptyFolder pin unpin pinLimitReached pinned folder unfiled dragToOrganize dropHereToRemove dropToRemoveFromFolder dragTask deleteFolder; do
printf '\n=== %s ===\n' "$key"
rg -n --type=ts --type=tsx "history:$key\b" webview-ui/src || echo "(no usage found)"
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Language type support:"
rg --type-list | sed -n '/tsx/p;/typescript/p' || true
echo "\nLocate history locales and files with target keys:"
fd 'history\.json$' webview-ui/src/i18n/locales || true
echo "\nUsage references (TS/TSX-friendly patterns):"
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' "history:($|newFolder|folderNamePlaceholder|renameFolder|removeFromFolder|deleteEmptyFolder|pin|unpin|pinLimitReached|pinned|folder|unfiled|dragToOrganize|dropHereToRemove|dropToRemoveFromFolder|dragTask|deleteFolder)\b" webview-ui/src || echo "(no usage found in webview-ui/src)"
echo "\nKey count/importance in ca/history.json:"
python3 - <<'PY'
import json
from pathlib import Path
p = Path("webview-ui/src/i18n/locales/ca/history.json")
data = json.loads(p.read_text())
print(f"path exists={p.exists()} lines={sum(1 for _ in open(p))}")
# crude JSON line extraction for keys mentioned
keys = ["newFolder","folderNamePlaceholder","renameFolder","removeFromFolder","deleteEmptyFolder","pin","unpin","pinLimitReached","pinned","folder","unfiled","dragToOrganize","dropHereToRemove","dropToRemoveFromFolder","dragTask","deleteFolder"]
for key in keys:
val = data.get(key)
print(f"{key}: {val}")
PY
echo "\nInspect relevant locale sections:"
sed -n '50,110p' webview-ui/src/i18n/locales/ca/history.jsonRepository: Zoo-Code-Org/Zoo-Code
Length of output: 5582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for key in dragToOrganize dropHereToRemove deleteEmptyFolder; do
printf '\n=== %s ===\n' "$key"
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' "history:${key}\b" webview-ui/src || echo "(no usage found)"
done
echo "\nReferenced duplicate-adjacent values in components:"
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' "history:(dropToRemoveFromFolder|dragTask|deleteFolder|dragCardToOrganize|dragFolder)\b" webview-ui/src || echo "(no usage found)"Repository: Zoo-Code-Org/Zoo-Code
Length of output: 616
Translate the remaining Catalan values for referenced history keys.
newFolder, folderNamePlaceholder, renameFolder, removeFromFolder, deleteEmptyFolder, pin, unpin, pinLimitReached, pinned, folder, unfiled, and the drop/organize copy are still English in webview-ui/src/i18n/locales/ca/history.json and are rendered by the History UI. Translate them; removal comments do not create a code issue here.
🤖 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 `@webview-ui/src/i18n/locales/ca/history.json` around lines 58 - 71, Translate
the English values for the referenced history keys in the Catalan locale,
including newFolder, folderNamePlaceholder, renameFolder, removeFromFolder,
deleteEmptyFolder, pin, unpin, pinLimitReached, pinned, folder, unfiled, tasks,
dragToOrganize, and dropHereToRemove. Preserve all keys and the {{count}}
interpolation while leaving unrelated entries unchanged.
| "pinLimitReached": "Maximum 3 pinned items allowed", | ||
| "pinned": "Pinned", | ||
| "folder": "Folder", | ||
| "tasks": "{{count}} tasks", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the en locale key shape and the parity test expectations for "tasks".
fd -a 'history.json$' webview-ui/src/i18n/locales | xargs -r rg -n '"tasks'
fd -a 'translation-parity.spec.ts$' webview-ui/src | xargs -r rg -n -C3 'plural|_one|_other'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 2393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
fd -a 'history.json$|translation-parity.spec.ts$|ManualFolderItem' webview-ui/src/i18n webview-ui/src | sed 's#/home/jailuser/git/##' | sort
echo
echo "Catalan history.json lines 55-75:"
fd -a 'history.json$' webview-ui/src/i18n/locales/ca | xargs sed -n '55,75p'
echo
echo "English history.json lines 65-85:"
fd -a 'history.json$' webview-ui/src/i18n/locales/en | xargs sed -n '65,85p'
echo
echo "Translations parity tests around history tasks:"
fd -a 'translation-parity.spec.ts$' webview-ui/src | xargs rg -n -C4 'history|tasks|selectedFolders|deleteFoldersTitle|plural|one|other|locale' || true
echo
echo "ManualFolderItem references:"
fd -a 'ManualFolderItem' . -t f -e ts -e tsx -e svelte -e vue -e react | while read -r f; do
echo "--- $f"
rg -n -C4 'ManualFolderItem|tasks|selectedFolders|count:' "$f" || true
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 11854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Translation setup files:"
fd -a 'TranslationContext\.(tsx|ts)$|index\.(ts|tsx|js)$|i18n.*\.(json|ts|tsx)$' webview-ui/src | sed 's#/home/jailuser/git/##' | sort
echo
echo "TranslationContext outline/imports/usages:"
for f in $(fd -a 'TranslationContext\.(tsx|ts)$' webview-ui/src); do
echo "--- $f"
sed -n '1,160p' "$f"
echo
done
echo
echo "Package/i18n dependencies:"
if [ -f webview-ui/package.json ]; then
jq '{dependencies, devDependencies} | to_entries[] | select(.key|contains("i18n") or contains("react") or contains("format"))' webview-ui/package.json
else
rg -n '"i18next"|"react-i18next"|"react-i18next-versioned"|"i18next-fs-backend"|"i18next-http-backend"' webview-ui/package.json webview-ui -g 'package.json' -g 'vite.config.*' -g 'i18n*' || true
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 4246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "i18n setup files:"
fd -a 'setup\.(ts|tsx)$|index\.(ts|tsx)$' webview-ui/src/i18n | sed 's#/home/jailuser/git/##' | sort | while read -r f; do
echo "--- $f"
sed -n '1,220p' "$f"
done
echo
echo "ManualFolderItem imports:"
sed -n '1,40p' webview-ui/src/components/history/ManualFolderItem.tsx
echo
echo "Package files containing i18next:"
fd -a 'package.json$' . | while read -r f; do
echo "--- ${f#/home/jailuser/git/}"
jq '.dependencies, .devDependencies' "$f" 2>/dev/null | rg -n '"(i18next|react-i18next|react-i18next-versioned|language-tag|locale-utils)"' || true
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 4165
Add Catalan plurals for tasks.
ManualFolderItem calls t("history:tasks", { count: unitCount }). Use _one/_other like the nearby count keys so the singular does not render as "1 tasks".
🌐 Proposed fix
- "tasks": "{{count}} tasks",
+ "tasks_one": "{{count}} tasca",
+ "tasks_other": "{{count}} tasques",📝 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.
| "tasks": "{{count}} tasks", | |
| "tasks_one": "{{count}} tasca", | |
| "tasks_other": "{{count}} tasques", |
🤖 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 `@webview-ui/src/i18n/locales/ca/history.json` at line 68, Add Catalan plural
variants for the history tasks translation by replacing the single tasks entry
with tasks_one and tasks_other, following the nearby count-key convention and
ensuring singular counts render “1 task” while plural counts render “{{count}}
tasks”.
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
Adds history folder/group/pin UI, pointer/keyboard DnD, dialog, optimistic reconciliation/rollback, empty/error state, locale, accessibility, and visual snapshot. Does not duplicate store/handler.
Included Files
webview-ui/src/components/history/HistoryView.tsxwebview-ui/src/components/history/TaskOrganizationDndSurface.tsxwebview-ui/src/components/history/useTaskOrganizationDnd.tswebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/components/history/HistoryView.task-organization.visual.tsxExclusion Scope
Summary by CodeRabbit
New Features
Reliability