feat: 看板体验优化 - #2346
Merged
Merged
Conversation
…055424 # Conflicts: # wework/scripts/prepare-dws-binary.mjs # wework/src/components/layout/DesktopWorkbenchLayout.test.tsx # wework/src/components/layout/workspace-panels/TemporaryChatPanel.tsx # wework/src/i18n/locales/en/common.json # wework/src/i18n/locales/zh-CN/common.json
Contributor
📝 WalkthroughWalkthroughThis change adds configurable cloud-project boards, custom statuses, recursive archival, AITable views and field mapping, DingTalk runtime routing, project-scoped chat, and updated desktop chat behavior. ChangesCloud board and runtime integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wework/src/features/todo/AITableView.tsx (1)
409-439: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe mount effect never sets
loadingback totrue.
loadingstarts astrue, so the first run is correct. This effect also re-runs whenapiorprojectchanges. On a re-run,loadingis alreadyfalse, so the grid keeps rendering the previous project's records and view until the new request resolves. SetsetLoading(true)at the start of the effect, asloadalready does.🐛 Proposed fix
useEffect(() => { let cancelled = false ;(async () => { + setLoading(true) setError(null) try {🤖 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 `@wework/src/features/todo/AITableView.tsx` around lines 409 - 439, Update the mount effect in AITableView’s useEffect to call setLoading(true) at the beginning of each run, before resetting the error and starting the async request, so api or project changes show the loading state while new data is fetched.
🟡 Minor comments (19)
wework/src/features/todo/ProjectSpaceChatSidebar.tsx-106-115 (1)
106-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove the draft copy into the i18n namespaces.
taskConversationDrafthardcodes Chinese user-visible text ([$任务:...],任务描述：,补充说明：). The rest of this component resolves copy throught(). Add these strings tosrc/i18n/locales/en/common.jsonandsrc/i18n/locales/zh-CN/common.json, then passtinto the helper.♻️ Proposed change
function taskConversationDraft( projectId: string | number, - request: ProjectSpaceChatLaunchRequest | null + request: ProjectSpaceChatLaunchRequest | null, + t: (key: string, options?: Record<string, unknown>) => string ): string { if (!request) return '' - const reference = `[$任务:${request.item.id}](cloud://projects/${projectId}/todos/${request.item.id})` + const reference = `[$${t('workbench.project_space_chat.task_mention_label')}:${request.item.id}](cloud://projects/${projectId}/todos/${request.item.id})` return request.item.description.trim() - ? `${reference}\n\n任务描述：\n${request.item.description.trim()}\n\n补充说明：\n` + ? `${reference}\n\n${t('workbench.project_space_chat.task_description_label')}\n${request.item.description.trim()}\n\n${t('workbench.project_space_chat.task_notes_label')}\n` : `${reference}\n\n` }As per coding guidelines: "Add new copy to the appropriate Wework namespace in both
src/i18n/locales/en/andsrc/i18n/locales/zh-CN/".🤖 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 `@wework/src/features/todo/ProjectSpaceChatSidebar.tsx` around lines 106 - 115, Update taskConversationDraft to accept the component’s t translator and replace all hardcoded user-visible text with i18n keys. Add the corresponding English and Simplified Chinese strings to common.json in both locale namespaces, then pass t from the component when calling the helper while preserving the existing draft formatting and conditional description behavior.Source: Coding guidelines
wework/src/features/todo/TodoEditor.tsx-721-723 (1)
721-723: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the configured status color for the status dot.
columnDotClassesonly contains the five built-in statuses. Whenproject.board_config.statusessupplies custom statuses, every dot rendersbg-zinc-400.todoShared.tsaddsboardStatusColorClassesfor this case, andCloudTodoWorkspace.tsxalready uses it. Resolve the color from the selected option first.♻️ Proposed change
+ {/* resolve the dot color from the configured status, then fall back */} <span - className={cn('h-2 w-2 rounded-full', columnDotClasses[status] ?? 'bg-zinc-400')} + className={cn( + 'h-2 w-2 rounded-full', + boardStatusColorClasses[ + statusOptions.find(option => option.id === status)?.color ?? '' + ] ?? + columnDotClasses[status] ?? + 'bg-zinc-400' + )} />Import
boardStatusColorClassesfrom./todoShared.🤖 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 `@wework/src/features/todo/TodoEditor.tsx` around lines 721 - 723, Update the status-dot rendering in TodoEditor to import and use boardStatusColorClasses from todoShared, resolving the selected status option’s configured color before falling back to columnDotClasses and then the existing default class. Ensure custom project.board_config.statuses colors are preserved.wework/src/i18n/locales/en/common.json-1991-1995 (1)
1991-1995: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse one public-project access scope in both locales.
The public-project description says all WeWork users can view a project. The management description says only users connected to the current Backend can view it. These descriptions must state the same authorization boundary.
wework/src/i18n/locales/en/common.json#L1991-L1995: Updateproject_visibility_manage_descriptionto match the intended public-project audience.wework/src/i18n/locales/zh-CN/common.json#L1990-L1994: Updateproject_visibility_manage_descriptionwith the same audience as the English string.🤖 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 `@wework/src/i18n/locales/en/common.json` around lines 1991 - 1995, Align project_visibility_manage_description in wework/src/i18n/locales/en/common.json (lines 1991-1995) and wework/src/i18n/locales/zh-CN/common.json (lines 1990-1994) so both describe the same intended public-project audience as project_visibility_public_description, removing the conflicting current-Backend restriction while preserving each locale’s language.backend/app/services/loop_items/service.py-587-603 (1)
587-603: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the descendant traversal against parent cycles.
The loop re-queries children of each newly found level and never tracks visited identifiers. If any stored
parent_idchain forms a cycle, the loop never terminates andarchived_itemsgrows without bound._validate_parent_changeblocks new cycles, but it does not protect rows written before that validation existed or rows written by other paths.Track visited identifiers and exclude them from the next query.
🛡️ Proposed fix
pending_parent_ids = [item.id] archived_items = [item] + visited = {item.id} while pending_parent_ids: children = ( db.query(LoopItem) .filter( LoopItem.cloud_project_id == item.cloud_project_id, LoopItem.parent_id.in_(pending_parent_ids), loop_datetime_is_unset(LoopItem.deleted_at), ) .all() ) - pending_parent_ids = [child.id for child in children] - archived_items.extend(children) + children = [child for child in children if child.id not in visited] + visited.update(child.id for child in children) + pending_parent_ids = [child.id for child in children] + archived_items.extend(children)🤖 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 `@backend/app/services/loop_items/service.py` around lines 587 - 603, Update the descendant traversal in the archival logic to track visited LoopItem identifiers, initialize it with the starting item, and exclude already visited IDs from each children query. Add newly discovered child IDs to the visited set before continuing, while preserving the existing archived_items collection and deletion/version updates.executor/src/agents/claude_code.rs-675-686 (1)
675-686: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse the real skill metadata for placeholder
SkillRefs.
resolve_skillreturns only(skill_id, namespace), but the deployed skill path uses the placeholder"is_public": falseand"content_hash": None. This is safe for most task skills, but a public skill with a cached manifest will only remain cached while a different backend route and hash state keep the manifest up-to-date; otherwise, every successful deploy still looks like a fresh cache miss. Passis_public/content_hashfrom the skill metadata instead of hard-coding the defaults.🤖 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 `@executor/src/agents/claude_code.rs` around lines 675 - 686, Update the successful resolve_skill handling to obtain and use the resolved skill’s actual metadata, including is_public and content_hash, when constructing the SkillRef inserted into plan.resolved_skill_map. Extend the resolved result or reuse the available skill metadata so these fields are no longer hard-coded to false and None, while preserving the existing skill_id and namespace values.wework/src/features/todo/CloudTodoWorkspace.tsx-914-922 (1)
914-922: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPriority column labels do not match the priority labels on the cards.
Line 917 renders
普通fornoneand the raw value for every other priority, so the columns readlow,medium,high, andurgent.CloudTodoBoardCard.tsxlines 16-22 render the same values as低,中,高, and紧急. The board therefore shows two different labels for one priority. Export the label map fromCloudTodoBoardCard.tsxand reuse it 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 914 - 922, Update the priority column construction around nativeGroupBy to reuse the exported label map from CloudTodoBoardCard.tsx for every priority, including none, instead of the inline 普通/raw-value labels. Import and apply that shared map while preserving the existing priority keys and metadata.wework/src/features/todo/CloudTodoWorkspace.tsx-1077-1080 (1)
1077-1080: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAn unavailable project-space API fails without user feedback. All three handlers resolve the API through
apiForProjectIdand then abandon the operation, so the dialog stays open with no message. The rename handler additionally throws outside itstryblock; because line 2389 calls it asvoid renameSelectedProject(), that throw becomes an unhandled promise rejection and never reachessetRenameError. Set the corresponding error state at each site instead.
wework/src/features/todo/CloudTodoWorkspace.tsx#L1077-L1080: replacethrow new Error('项目空间接口当前不可用')withsetRenameError(...)followed byreturn.wework/src/features/todo/CloudTodoWorkspace.tsx#L1101-L1104: replace the bareif (!api) returnwithsetArchiveError(...)followed byreturn.wework/src/features/todo/CloudTodoWorkspace.tsx#L1124-L1127: replace the bareif (!api) returnwithsetArchiveError(...)followed byreturn.🤖 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 1077 - 1080, Update renameSelectedProject at wework/src/features/todo/CloudTodoWorkspace.tsx#L1077-L1080 to setRenameError and return when apiForProjectId returns no API, rather than throwing. Update the handlers at `#L1101-L1104` and `#L1124-L1127` to setArchiveError and return instead of silently returning, so all unavailable project-space API cases provide user feedback.wework/src/features/todo/CloudTodoBoardCard.tsx-160-196 (1)
160-196: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTwo new popup surfaces lack complete dismissal handling. Both open an overlay from a trigger button, and neither implements the full dismissal pair that
CloudTodoWorkspace.tsxlines 805-826 already establish for the project menu. Keyboard users cannot close either overlay.
wework/src/features/todo/CloudTodoBoardCard.tsx#L160-L196: add an outside-click handler and an Escape handler formenuOpen, and addrole="menu"plusrole="menuitem".wework/src/features/todo/CloudTodoWorkspace.tsx#L138-L145: extend the existingmousedowneffect inAITableGroupFieldPickerwith akeydownhandler that closes the picker on Escape.🤖 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 `@wework/src/features/todo/CloudTodoBoardCard.tsx` around lines 160 - 196, The CloudTodoBoardCard menu lacks complete dismissal and accessibility behavior: add outside-click and Escape handling tied to menuOpen, mark its container as role="menu", and its archive button as role="menuitem". In wework/src/features/todo/CloudTodoWorkspace.tsx lines 138-145, extend the AITableGroupFieldPicker mousedown effect with a keydown listener that closes the picker on Escape.wework/src/features/todo/CloudTodoWorkspace.tsx-905-913 (1)
905-913: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNon-status columns inherit
status: 'inbox', so every empty column shows the inbox hint.The AITable, priority, assignee, and tag columns all set
statusto'inbox'. Line 2197 then looks upcolumnEmptyHints[column.status]and renders the inbox hint inside every empty column, including a DingTalk group column and the无标签column. Key the empty hint on the grouping mode, or only look it up whennativeGroupBy === 'status'.🐛 Proposed fix
- {columnItems.length === 0 && columnEmptyHints[column.status] && ( + {columnItems.length === 0 && + !isAITableProject && + nativeGroupBy === 'status' && + columnEmptyHints[column.status] && ( <div className="rounded-xl border border-dashed border-border px-3 py-6 text-center text-xs text-text-muted"> {columnEmptyHints[column.status]} </div> )}Also applies to: 953-971
🤖 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 905 - 913, Update the empty-column rendering near the columnEmptyHints lookup and the boardColumns definitions for AITable, priority, assignee, and tag grouping so non-status columns do not use their inherited status value to select an inbox hint. Only resolve and render columnEmptyHints when nativeGroupBy is 'status', or otherwise key the hint by the active grouping mode, while preserving status-based hints for actual status columns.wework/src/features/todo/CloudTodoBoardCard.tsx-78-83 (1)
78-83: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe date badge shows
updated_atwhen no due date exists.Line 81 falls back from
item.due_attoitem.updated_at, and both render behind the sameCalendarDaysicon with the sameMM-DDslice. A reader cannot tell a deadline from a last-modified date. Show the date only whendue_atis set, or label the fallback so the two meanings stay distinct.🐛 Proposed fix to distinguish the two dates
- {display.showDate ? ( - <span className="inline-flex items-center gap-1"> + {display.showDate ? ( + <span + className="inline-flex items-center gap-1" + title={item.due_at ? '截止日期' : '最近更新'} + > <CalendarDays className="h-3 w-3" /> {(item.due_at ?? item.updated_at).slice(5, 10)} </span> ) : null}🤖 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 `@wework/src/features/todo/CloudTodoBoardCard.tsx` around lines 78 - 83, Update the date badge in the component’s showDate rendering to avoid presenting updated_at as a due date: render the CalendarDays date only when item.due_at is set, or explicitly label any updated_at fallback so its meaning is distinct. Preserve the existing MM-DD formatting for due dates.wework/src/features/todo/CloudTodoWorkspace.tsx-1066-1075 (1)
1066-1075: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
selectProjectdoes not reset the AITable filter and search state.The function clears
nativeGroupFilterandnativeBoardQuery, but leavesaitableGroupFilterandaitableBoardQueryuntouched. The column predicate at lines 2139-2143 applies both values without anisAITableProjectguard. After a user filters a DingTalk board and switches to another project, every column filters against the stale value and the board renders empty. Reset the AITable state in the same function.🐛 Proposed fix
function selectProject(projectId: string | null) { setSelectedProjectId(projectId) setProjectView('board') setBoardParentId(null) setNativeGroupFilter('') setNativeBoardQuery('') + setAitableGroupFilter('') + setAitableBoardQuery('') setProjectSearchOpen(false) setProjectSearchQuery('') setProjectSearchFilters(emptyTaskSearchFilters) }🤖 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 1066 - 1075, Update selectProject to also reset the AITable filter and search state by clearing aitableGroupFilter and aitableBoardQuery alongside the existing native filter resets, so switching projects cannot retain stale AITable values.wework/src/features/todo/AITableView.tsx-134-142 (1)
134-142: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDate normalization uses UTC, while the grid value uses local time.
Line 137 converts a millisecond timestamp with
toISOString().slice(0, 10), which yields the UTC calendar date.gridValueat line 122 builds aDatethat AG Grid sorts and filters in local time. For a timestamp that falls on a different calendar day in UTC than in the user's timezone, the displayed and edited date differs from the sorted and filtered date by one day. Derive the date from local components so both paths agree.🐛 Proposed fix to use local date components
function editorText(field: AITableField, value: unknown): string { if (field.type !== 'date') return cellText(value) if (typeof value === 'number' && Number.isFinite(value)) { - return new Date(value).toISOString().slice(0, 10) + const date = new Date(value) + const month = String(date.getMonth() + 1).padStart(2, '0') + const day = String(date.getDate()).padStart(2, '0') + return `${date.getFullYear()}-${month}-${day}` } const text = cellText(value) const match = text.match(/^\d{4}-\d{2}-\d{2}/) return match?.[0] ?? text }🤖 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 `@wework/src/features/todo/AITableView.tsx` around lines 134 - 142, Update editorText for numeric date values to derive the YYYY-MM-DD string from the Date’s local year, month, and day components instead of toISOString().slice(0, 10). Keep the existing handling for non-numeric values unchanged so displayed and edited dates match gridValue’s local-time behavior.wework/src/features/todo/CloudTodoWorkspace.tsx-2022-2037 (1)
2022-2037: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReading
localStorageduring render makes the button state impure.Line 2022 calls
localStorage.getItem(personalGroupKey)inside the render body. React cannot re-render when that external value changes, so the应用到全局button appears and disappears only when some other state change happens to trigger a render.savePersonalGroupByandsaveGlobalGroupBycurrently force that render as a side effect, which makes the behavior incidental. Track the override in component state and keeplocalStoragewrites in the handlers.♻️ Proposed fix to move the flag into state
+ const [hasPersonalGroupOverride, setHasPersonalGroupOverride] = useState(false)function savePersonalGroupBy(groupBy: NativeBoardGroupBy) { setNativeGroupBy(groupBy) setNativeGroupFilter('') - if (personalGroupKey) localStorage.setItem(personalGroupKey, groupBy) + if (personalGroupKey) { + localStorage.setItem(personalGroupKey, groupBy) + setHasPersonalGroupOverride(true) + } }- if (personalGroupKey) localStorage.removeItem(personalGroupKey) + if (personalGroupKey) { + localStorage.removeItem(personalGroupKey) + setHasPersonalGroupOverride(false) + }- {personalGroupKey && localStorage.getItem(personalGroupKey) ? ( + {personalGroupKey && hasPersonalGroupOverride ? (Set the initial value inside the existing effect at lines 872-885, which already reads the same key.
🤖 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 2022 - 2037, Replace the render-time localStorage.getItem check in CloudTodoWorkspace with component state for the personal-group override flag. Initialize or synchronize that state in the existing effect that reads personalGroupKey, and update it explicitly in savePersonalGroupBy and saveGlobalGroupBy after their localStorage writes so rendering depends only on React state.wework/src/features/todo/BoardLayoutEditor.tsx-216-226 (1)
216-226: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Date.now()alone can produce duplicate status ids.Two
addStatuscalls inside the same millisecond generate the sameid. That id is the dnd-kit sortable id, the React key, and the persisted status id, so duplicates break sorting and rename or delete targeting. Derive the id from a value that cannot repeat, for examplecrypto.randomUUID(), or reject an id that already exists instatuses.🤖 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 `@wework/src/features/todo/BoardLayoutEditor.tsx` around lines 216 - 226, Update addStatus so each new status receives an id that cannot collide with existing statuses, replacing the Date.now()-based generation with crypto.randomUUID() or an equivalent uniqueness check against statuses. Preserve the existing status creation and onStatusesChange behavior.executor/src/task_runtime/mcp.rs-531-542 (1)
531-542: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe redirect can emit
nullids while instructing the agent to use them.
project.metadata["provider_config"]["base_id"]returnsValue::Nullwhen the project has no stored binding, so the payload can contain"base_id": nulltogether with "Use these bound IDs directly". The fallback branch only covers a missing project, not a project withoutbase_idortable_id.Treat a missing
base_idortable_idas unbound and return the fallback instruction instead.🤖 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 `@executor/src/task_runtime/mcp.rs` around lines 531 - 542, Update the project mapping around the redirect payload to validate that provider_config contains non-null base_id and table_id before emitting the bound-resource instruction. For projects missing either binding, return the existing fallback instruction instead; only include the direct-use payload when both IDs are present, while preserving the optional view_id behavior.wework/src/features/todo/CloudProjectManageView.tsx-658-687 (1)
658-687: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
aitableBusydrives two unrelated buttons.
waitForDwsAuthenticationpolls for up to 120 seconds (160 attempts × 750 ms). During that wait,aitableBusyalso disables "保存连接" at Line 681, and the connect button shows "等待浏览器授权…" while a plain save is in progress. Use a separate flag for the DingTalk login wait.The polling loop also continues after unmount, because nothing cancels it. Add a cancellation flag or an
AbortSignalso the view stops polling when the user leaves the page.🤖 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 `@wework/src/features/todo/CloudProjectManageView.tsx` around lines 658 - 687, Separate the DingTalk login-wait state from the save state: update the handlers and button rendering around waitForDwsAuthentication so only the connect button uses a dedicated login-wait flag, while the save button uses its existing save-busy state. Add cancellation tied to the view lifecycle for waitForDwsAuthentication, using an AbortSignal or equivalent cleanup flag, and stop the polling promptly when the component unmounts or the login flow is cancelled.executor/src/task_runtime/store.rs-297-302 (1)
297-302: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse project
board_config.statuseswhen validating local tasks.
update_projectstores anyboard_config, butcreate_task,update_task, andreorder_taskscallvalidate_statusagainst the fixed list"inbox" | "pending" | "in_progress" | "in_review" | "completed". This rejects task status values valid for a custom local board. Pass the project status ids to these local validation paths or add project-aware validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/task_runtime/store.rs` around lines 297 - 302, Update the local task validation used by create_task, update_task, and reorder_tasks to load the project’s board_config.statuses and validate against those status IDs instead of the fixed default list. Ensure update_project’s stored board_config is reused for project-aware validation while preserving the existing default statuses when no custom board configuration exists.wework/src/features/todo/BoardLayoutEditor.tsx-198-205 (1)
198-205: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClose the display menu on
Escape.The menu closes only on outside
pointerdown. A keyboard user who openscloud-board-display-menucannot dismiss it without a pointer, and focus is not returned to the trigger. Add an Escape handler in the same effect.♿ Proposed fix
useEffect(() => { if (!fieldsOpen) return const close = (event: PointerEvent) => { if (!fieldsRef.current?.contains(event.target as Node)) setFieldsOpen(false) } + const closeOnEscape = (event: KeyboardEvent) => { + if (event.key === 'Escape') setFieldsOpen(false) + } document.addEventListener('pointerdown', close) - return () => document.removeEventListener('pointerdown', close) + document.addEventListener('keydown', closeOnEscape) + return () => { + document.removeEventListener('pointerdown', close) + document.removeEventListener('keydown', closeOnEscape) + } }, [fieldsOpen])🤖 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 `@wework/src/features/todo/BoardLayoutEditor.tsx` around lines 198 - 205, Update the useEffect managing fieldsOpen and the fieldsRef outside-click handler to also listen for keydown events and close the display menu when Escape is pressed. Remove the keydown listener during cleanup, and return focus to the menu trigger after closing.executor/src/task_runtime/store.rs-329-343 (1)
329-343: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDistinguish unknown or already archived projects from stale archives.
archive_projectmaps every zero-row update toVersionConflict, so an archive call for a missingloop_itemsproject id or an already archived project also sendsversion_conflict; this can surface to the desktop as a false “project changed” result. Map the0-update case toProjectNotFound, and cleardeleted_atvialist_projects()/get_project()into account for the already archived case before returningVersionConflictfor a stale version.🤖 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 `@executor/src/task_runtime/store.rs` around lines 329 - 343, Update archive_project so a zero-row update first checks list_projects()/get_project() for the project and its deleted_at state: return ProjectNotFound when the project is missing or already archived, and return VersionConflict only when an existing active project has a stale version. Preserve the successful archive path and version update behavior.
🤖 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 `@backend/app/services/loop_items/service.py`:
- Around line 123-125: Update the list response paths that repeatedly call
loop_item_service.response_values(), especially the LoopItemListResponse and
MyWorkListResponse construction in deliveries.py, to batch-load or eager-load
all assignee users before mapping items. Refactor response_values() and its
callers to reuse the preloaded user data instead of executing db.get(User,
item.assignee_user_id) for each item, while preserving assignee_name behavior
for missing or unassigned users.
In `@executor/src/task_runtime/aitable_provider.rs`:
- Around line 501-579: The enrich_user_cells method should resolve user
identifiers through a bounded concurrent stream rather than awaiting each
contact user search sequentially, while preserving the existing per-request
limit behavior as appropriate. Add provider-level caching keyed by user
identifier, reuse cached names across calls, and ensure identifiers beyond the
current 30-item processing limit are not silently left unresolved; update the
name-enrichment pass to use cached or newly resolved results.
- Around line 768-807: Update infer_board_mapping so title candidates are
evaluated in declared priority order, preferring exact field-name matches before
substring matches, and exclude fields whose IDs are already assigned to other
mapping keys. Apply this selection specifically to title_field_id while
preserving the existing fallback to the first field when no suitable title
remains.
In `@executor/src/task_runtime/store.rs`:
- Around line 504-528: Update archive_task to accept the target task version,
start a TransactionBehavior::Immediate transaction before the task check, and
perform both get_task validation and the recursive archive update through that
transaction. Add the matching version predicate to the target-row update, return
VersionConflict when no row is updated due to a version mismatch, and commit the
transaction on success. Update the todos.archive IPC route to receive and
forward the version argument.
In `@wework/scripts/prepare-dws-binary.mjs`:
- Around line 53-66: Update extractZip to resolve the destination and each
archive entry path, then reject entries whose resolved output is outside the
destination boundary before creating directories or writing files. Use the
node:path separator as needed to enforce a proper descendant check while
allowing paths within the destination.
In `@wework/src/api/deliveries.ts`:
- Line 56: Replace hard-coded status switch mappings in DesktopWorkbenchMain and
other CloudLoopItem status consumers with lookups against
project.board_config.statuses, using the matching custom status id to resolve
its label. Preserve existing labels for built-in statuses and provide the
established fallback behavior when no configured status matches.
In `@wework/src/components/layout/workspace-panels/TemporaryChatPanel.tsx`:
- Around line 217-219: Update both lifecycleStore.syncTranscript calls in
TemporaryChatPanel to set preserveActiveTurn from
lifecycleStore.getTask(address)?.derived.isTurnActive, rather than
derived.isRunning, so submitting, awaiting, and running turns remain preserved
during transcript synchronization.
In `@wework/src/features/todo/CloudProjectManageView.tsx`:
- Around line 390-393: Replace hardcoded user-facing copy with the local
useTranslation('common') wrapper: in
wework/src/features/todo/CloudProjectManageView.tsx:390-393 and throughout the
view, restore translation usage for headings, controls, prompts, and error
fallbacks; in wework/src/features/todo/BoardLayoutEditor.tsx:243-248 and
throughout the editor, translate headings, helper text, menu labels,
aria-labels, and the default 新状态 name. Add matching keys to both
wework/src/i18n/locales/en/ and wework/src/i18n/locales/zh-CN/, register any new
namespace in wework/src/i18n/index.ts, and update BoardLayoutEditor.test.tsx to
assert translated output rather than raw copy.
In `@wework/src/features/todo/CloudTodoWorkspace.tsx`:
- Around line 82-201: Split CloudTodoWorkspace into cohesive modules to keep it
under the 1000-line guideline: move AITableGroupFieldPicker into its own module
following CloudTodoBoardCard.tsx’s pattern, move nativeBoardGroupFields,
nativeBoardStatusColors, and aitableCellLabels into todoShared.ts, and extract
the boardColumns construction into a memoized helper module while preserving its
existing behavior and interfaces.
- Around line 1328-1344: Update the tag-handling branches in the item move
logic, including the mapping callback and the corresponding code around the
alternate path, so dragging between tag columns replaces only the tag owned by
the current grouping while preserving all other tags. Treat an empty
column.groupValue as removing only the grouping tag rather than clearing
candidate.tags, and leave priority and assignee behavior unchanged.
- Around line 2223-2231: Update the DragOverlay rendering in CloudTodoWorkspace
to resolve the active item from items and render CloudTodoCardContent only when
that lookup returns an item; remove the non-null assertion from items.find(...).
Preserve the existing overlay styling and null rendering when there is no active
drag item or the item has disappeared.
In `@wework/src/features/todo/TodoEditor.tsx`:
- Around line 733-739: Update the status select rendering in TodoEditor so that
when the current status is non-empty and absent from statusOptions, it adds an
option for that status, while preserving the existing placeholder and mapped
options. Use the current status value and a suitable label, and ensure normal
statuses already present in statusOptions are not duplicated.
---
Outside diff comments:
In `@wework/src/features/todo/AITableView.tsx`:
- Around line 409-439: Update the mount effect in AITableView’s useEffect to
call setLoading(true) at the beginning of each run, before resetting the error
and starting the async request, so api or project changes show the loading state
while new data is fetched.
---
Minor comments:
In `@backend/app/services/loop_items/service.py`:
- Around line 587-603: Update the descendant traversal in the archival logic to
track visited LoopItem identifiers, initialize it with the starting item, and
exclude already visited IDs from each children query. Add newly discovered child
IDs to the visited set before continuing, while preserving the existing
archived_items collection and deletion/version updates.
In `@executor/src/agents/claude_code.rs`:
- Around line 675-686: Update the successful resolve_skill handling to obtain
and use the resolved skill’s actual metadata, including is_public and
content_hash, when constructing the SkillRef inserted into
plan.resolved_skill_map. Extend the resolved result or reuse the available skill
metadata so these fields are no longer hard-coded to false and None, while
preserving the existing skill_id and namespace values.
In `@executor/src/task_runtime/mcp.rs`:
- Around line 531-542: Update the project mapping around the redirect payload to
validate that provider_config contains non-null base_id and table_id before
emitting the bound-resource instruction. For projects missing either binding,
return the existing fallback instruction instead; only include the direct-use
payload when both IDs are present, while preserving the optional view_id
behavior.
In `@executor/src/task_runtime/store.rs`:
- Around line 297-302: Update the local task validation used by create_task,
update_task, and reorder_tasks to load the project’s board_config.statuses and
validate against those status IDs instead of the fixed default list. Ensure
update_project’s stored board_config is reused for project-aware validation
while preserving the existing default statuses when no custom board
configuration exists.
- Around line 329-343: Update archive_project so a zero-row update first checks
list_projects()/get_project() for the project and its deleted_at state: return
ProjectNotFound when the project is missing or already archived, and return
VersionConflict only when an existing active project has a stale version.
Preserve the successful archive path and version update behavior.
In `@wework/src/features/todo/AITableView.tsx`:
- Around line 134-142: Update editorText for numeric date values to derive the
YYYY-MM-DD string from the Date’s local year, month, and day components instead
of toISOString().slice(0, 10). Keep the existing handling for non-numeric values
unchanged so displayed and edited dates match gridValue’s local-time behavior.
In `@wework/src/features/todo/BoardLayoutEditor.tsx`:
- Around line 216-226: Update addStatus so each new status receives an id that
cannot collide with existing statuses, replacing the Date.now()-based generation
with crypto.randomUUID() or an equivalent uniqueness check against statuses.
Preserve the existing status creation and onStatusesChange behavior.
- Around line 198-205: Update the useEffect managing fieldsOpen and the
fieldsRef outside-click handler to also listen for keydown events and close the
display menu when Escape is pressed. Remove the keydown listener during cleanup,
and return focus to the menu trigger after closing.
In `@wework/src/features/todo/CloudProjectManageView.tsx`:
- Around line 658-687: Separate the DingTalk login-wait state from the save
state: update the handlers and button rendering around waitForDwsAuthentication
so only the connect button uses a dedicated login-wait flag, while the save
button uses its existing save-busy state. Add cancellation tied to the view
lifecycle for waitForDwsAuthentication, using an AbortSignal or equivalent
cleanup flag, and stop the polling promptly when the component unmounts or the
login flow is cancelled.
In `@wework/src/features/todo/CloudTodoBoardCard.tsx`:
- Around line 160-196: The CloudTodoBoardCard menu lacks complete dismissal and
accessibility behavior: add outside-click and Escape handling tied to menuOpen,
mark its container as role="menu", and its archive button as role="menuitem". In
wework/src/features/todo/CloudTodoWorkspace.tsx lines 138-145, extend the
AITableGroupFieldPicker mousedown effect with a keydown listener that closes the
picker on Escape.
- Around line 78-83: Update the date badge in the component’s showDate rendering
to avoid presenting updated_at as a due date: render the CalendarDays date only
when item.due_at is set, or explicitly label any updated_at fallback so its
meaning is distinct. Preserve the existing MM-DD formatting for due dates.
In `@wework/src/features/todo/CloudTodoWorkspace.tsx`:
- Around line 914-922: Update the priority column construction around
nativeGroupBy to reuse the exported label map from CloudTodoBoardCard.tsx for
every priority, including none, instead of the inline 普通/raw-value labels.
Import and apply that shared map while preserving the existing priority keys and
metadata.
- Around line 1077-1080: Update renameSelectedProject at
wework/src/features/todo/CloudTodoWorkspace.tsx#L1077-L1080 to setRenameError
and return when apiForProjectId returns no API, rather than throwing. Update the
handlers at `#L1101-L1104` and `#L1124-L1127` to setArchiveError and return instead
of silently returning, so all unavailable project-space API cases provide user
feedback.
- Around line 905-913: Update the empty-column rendering near the
columnEmptyHints lookup and the boardColumns definitions for AITable, priority,
assignee, and tag grouping so non-status columns do not use their inherited
status value to select an inbox hint. Only resolve and render columnEmptyHints
when nativeGroupBy is 'status', or otherwise key the hint by the active grouping
mode, while preserving status-based hints for actual status columns.
- Around line 1066-1075: Update selectProject to also reset the AITable filter
and search state by clearing aitableGroupFilter and aitableBoardQuery alongside
the existing native filter resets, so switching projects cannot retain stale
AITable values.
- Around line 2022-2037: Replace the render-time localStorage.getItem check in
CloudTodoWorkspace with component state for the personal-group override flag.
Initialize or synchronize that state in the existing effect that reads
personalGroupKey, and update it explicitly in savePersonalGroupBy and
saveGlobalGroupBy after their localStorage writes so rendering depends only on
React state.
In `@wework/src/features/todo/ProjectSpaceChatSidebar.tsx`:
- Around line 106-115: Update taskConversationDraft to accept the component’s t
translator and replace all hardcoded user-visible text with i18n keys. Add the
corresponding English and Simplified Chinese strings to common.json in both
locale namespaces, then pass t from the component when calling the helper while
preserving the existing draft formatting and conditional description behavior.
In `@wework/src/features/todo/TodoEditor.tsx`:
- Around line 721-723: Update the status-dot rendering in TodoEditor to import
and use boardStatusColorClasses from todoShared, resolving the selected status
option’s configured color before falling back to columnDotClasses and then the
existing default class. Ensure custom project.board_config.statuses colors are
preserved.
In `@wework/src/i18n/locales/en/common.json`:
- Around line 1991-1995: Align project_visibility_manage_description in
wework/src/i18n/locales/en/common.json (lines 1991-1995) and
wework/src/i18n/locales/zh-CN/common.json (lines 1990-1994) so both describe the
same intended public-project audience as project_visibility_public_description,
removing the conflicting current-Backend restriction while preserving each
locale’s language.
---
Nitpick comments:
In `@backend/app/services/loop_items/service.py`:
- Around line 52-66: Update _project_status_ids to import and reuse the
default_board_statuses helper from the cloud project schema when board statuses
are missing or invalid, returning the helper’s status identifiers instead of
duplicating the hardcoded fallback list.
In `@executor/src/agents/claude_code.rs`:
- Around line 670-699: Replace the sequential resolution loop around
resolve_skill with a bounded concurrent stream using
skill_download_concurrency() and buffer_unordered. Collect each skill’s
resolution result, then sequentially update plan.resolved_skill_map and emit the
existing not-found or failure events, preserving already-resolved skill skipping
and per-skill error handling.
In `@executor/src/local/app_ipc.rs`:
- Around line 715-725: Update the "projects.archive" handler to obtain the
version through the existing required_task_i64 helper, matching the files.move
pattern, and remove the duplicated inline parameter parsing while preserving the
existing archive_project call and error propagation.
In `@executor/src/task_runtime/aitable_provider.rs`:
- Around line 57-72: Update the detached login process in the task runtime
provider to retain stderr instead of suppressing it, then have the tokio::spawn
task await the child and log both the exit status and captured stderr. Preserve
the existing successful response and spawn-error handling while ensuring
failures after spawn are recorded.
- Around line 477-499: Cache the table schema and view configuration for the
duration of each board operation to avoid repeated CLI lookups. Update
board_config and the create_board/update_board/get_board/list_board call chain
to reuse one field result per base/table, and update the view lookup in
list_records to reuse one view configuration across all paginated requests when
view_id is supplied. Keep cache scope limited to the current board operation and
preserve existing mapping and lookup behavior.
In `@executor/src/task_runtime/mcp.rs`:
- Around line 505-512: Remove the unused _tool_name parameter from
is_locally_routed_project and update every production and test call site to pass
only runtime and project_id. Do not add a compatibility wrapper or retain the
dead argument.
- Around line 145-153: Update call_tool to resolve the requested project’s
LoopItem once per invocation, then pass that resolved project to
is_dingtalk_aitable_project, is_locally_routed_project, and
dingtalk_route_redirect instead of letting each helper reload the project list.
Apply the same reuse pattern in visible_tools so each tool-list request performs
only one project lookup.
In `@executor/src/task_runtime/model.rs`:
- Around line 64-65: Replace the arbitrary serde_json::Value types for
board_config and card_display with typed serde-deserializable structs matching
CloudProjectBoardConfig and its status entries. Enforce unique status
identifiers, the identifier pattern, valid color values, and a maximum of 50
statuses at IPC deserialization so LocalTaskStore::update_project only receives
valid configurations.
In `@executor/src/task_runtime/store.rs`:
- Around line 108-117: The default board statuses are duplicated between the
configuration in the store initialization and backend’s
default_board_statuses(); consolidate them through one shared definition, or add
a test asserting the status IDs, names, and colors remain identical across both
paths.
In `@executor/tests/local_app_ipc_contract.rs`:
- Around line 255-265: Add a stale-version dispatch for projects.archive before
the existing successful archive in the test, using an outdated version and
asserting the returned conflict code is "version_conflict". Keep the
current-project version for the subsequent successful archive and preserve the
final empty-project assertion.
In `@wework/src/api/aitable.ts`:
- Around line 67-70: Remove the unused createView method from the AITable
interface and delete its local implementation, including related dead code. Keep
the AITable contract minimal and preserve the existing behavior where kanban
creation is unavailable and view tabs remain hidden.
In `@wework/src/features/todo/AITableView.tsx`:
- Line 335: Replace the hardcoded user-visible strings `删除字段`, `删除记录`, and
`暂无记录` in `AITableView` with keys from the local `@/hooks/useTranslation`
wrapper, and add matching English and Chinese entries to the appropriate Wework
locale namespace under `src/i18n/locales/en/` and `src/i18n/locales/zh-CN/`.
- Around line 13-50: Replace AllCommunityModule registration with imports and
registration of only the AG Grid modules required by the features used in
AITableView and its AgGridReact configuration. Update
ModuleRegistry.registerModules accordingly, while preserving the existing grid
behavior and aitableGridTheme setup.
In `@wework/src/features/todo/BoardLayoutEditor.test.tsx`:
- Around line 33-34: Remove the literal sample-text assertion from the
BoardLayoutEditor test. Replace the getByText check for “补充项目管理页的空状态” with an
assertion targeting the preview container or a stable test id, while keeping the
existing layout-settings assertion unchanged.
- Around line 42-62: Extend the BoardLayoutEditor tests with a render using
canEditStatuses={false}, then assert that the cloud-board-status-add element is
absent. Keep the existing editable-status test unchanged and reuse the current
fixture props and query utilities.
In `@wework/src/features/todo/BoardLayoutEditor.tsx`:
- Around line 311-336: In BoardLayoutEditor, extract the preview-column
calculation from the statuses.map render into a named previewIndex constant
above the map, preserving the second-column-or-first-when-only-one behavior, and
use previewIndex for the SortableStatus previewDisplay condition.
In `@wework/src/features/todo/CloudProjectManageView.tsx`:
- Around line 294-341: Refresh the items from the server in the catch blocks of
both renameTag and deleteTag after any partial update failure, before clearing
the busy state, so local item tags and versions match completed server updates.
Keep the existing error reporting and success-path persistence behavior
unchanged.
- Around line 170-175: Remove the explicit version arguments from the
updateProject call sites around the affected handlers, while leaving
updateProject’s internal merge of the current version unchanged. Ensure each
caller passes only its update values so updateProject remains the single source
of truth for version.
In `@wework/src/features/todo/CloudTodoBoardCard.tsx`:
- Around line 16-22: Move all user-visible strings in CloudTodoBoardCard,
including priorityLabels and the labels referenced in the indicated ranges, into
the appropriate Wework translation namespace in both English and zh-CN locale
files. Use the local `@/hooks/useTranslation` wrapper within CloudTodoBoardCard to
read these translation keys, preserving the current Chinese text as zh-CN values
and adding corresponding English translations.
In `@wework/src/features/todo/CloudTodoWorkspace.test.tsx`:
- Around line 204-259: Add an automated E2E scenario covering project archival,
task archival, and launching project chat through the real application flow,
rather than the mocked component paths in the tests around the project and task
archival cases. Update the GitHub Actions workflow to invoke this scenario and
verify it runs as part of CI.
In `@wework/src/features/todo/CloudTodoWorkspace.tsx`:
- Around line 737-790: Update the useLayoutEffect measurement logic around
compute so it no longer depends on projectHeaderLevel, since that value is set
by the effect. Change setProjectHeaderLevel to use its previous state and return
the previous level when next is unchanged, preventing redundant commits;
preserve the existing level calculation and ResizeObserver behavior.
- Around line 1382-1394: Extract the shared column-membership predicate into an
itemMatchesColumn(item, column) helper, covering AITable and all native grouping
modes consistently. Replace the inline targetColumn predicate in finishBoardDrop
and the duplicated filter logic around the column rendering path with this
helper, preserving each caller’s existing behavior while ensuring both paths use
the same grouping rules.
- Around line 1592-1630: Replace the listed literal Chinese UI strings in
CloudTodoWorkspace, including the project action buttons and the additional
affected sections, with keys resolved through `@/hooks/useTranslation`. Add the
corresponding English and zh-CN entries to the appropriate Wework locale
namespace, and register that namespace in src/i18n/index.ts if it is new;
preserve the existing todo namespace usage and provide equivalent translations
for every new key.
In `@wework/src/features/todo/projectProviderConfig.test.ts`:
- Around line 78-86: Add a test alongside the existing
dingtalkAITableRuntimeContext cases covering a DingTalk project whose
provider_config omits base_id and table_id or contains blank values, and assert
the function returns undefined. Keep the existing non-DingTalk provider test
unchanged.
In `@wework/src/features/todo/projectProviderConfig.ts`:
- Around line 123-132: Move the static rules array out of the function
containing dingtalkAITableRuntimeContext into a module-level constant, then have
the function reuse that constant. Preserve the existing rule text and runtime
behavior while reducing per-call array allocation and keeping the function under
the 50-line guideline.
In `@wework/src/features/todo/ProjectSpaceChatSidebar.tsx`:
- Around line 63-66: Update runtimeCloudProjectId to read only
task.runtimeHandle?.cloudProjectId, removing the cloud_project_id fallback while
preserving the existing string/number normalization and null behavior.
- Around line 203-212: Update startNewConversation to read creatingNew directly
instead of performing side effects inside the setCreatingNew updater. Return
early when creatingNew is already true; otherwise reset selectedConversationKey,
draft, and draftItemId, increment chatInstance, and then set creatingNew to
true, keeping the updater pure or removing it.
🪄 Autofix (Beta)
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: 21ee4d5e-e622-44c6-b193-1622f2158729
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (51)
backend/app/api/endpoints/cloud_projects.pybackend/app/api/endpoints/deliveries.pybackend/app/schemas/cloud_project.pybackend/app/schemas/delivery.pybackend/app/services/cloud_projects/service.pybackend/app/services/loop_items/service.pybackend/tests/api/test_cloud_projects_api.pyexecutor/src/agents/claude_code.rsexecutor/src/agents/runtime_capabilities.rsexecutor/src/local/app_ipc.rsexecutor/src/task_runtime/aitable_provider.rsexecutor/src/task_runtime/aitable_provider_tests.rsexecutor/src/task_runtime/mcp.rsexecutor/src/task_runtime/model.rsexecutor/src/task_runtime/router.rsexecutor/src/task_runtime/store.rsexecutor/tests/local_app_ipc_contract.rspnpm-workspace.yamlwework/package.jsonwework/scripts/prepare-dws-binary.mjswework/src-tauri/src/system_sleep.rswework/src/api/aitable.tswework/src/api/deliveries.tswework/src/api/dws.tswework/src/api/local/localDelivery.test.tswework/src/api/local/localDelivery.tswework/src/api/local/localServices.test.tswework/src/api/local/localServices.tswework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/desktopChatLayout.tswework/src/components/layout/workspace-panels/TemporaryChatPanel.tsxwework/src/features/todo/AITableView.test.tsxwework/src/features/todo/AITableView.tsxwework/src/features/todo/BoardLayoutEditor.test.tsxwework/src/features/todo/BoardLayoutEditor.tsxwework/src/features/todo/CloudProjectManageView.tsxwework/src/features/todo/CloudTodoBoardCard.tsxwework/src/features/todo/CloudTodoWorkspace.test.tsxwework/src/features/todo/CloudTodoWorkspace.tsxwework/src/features/todo/ProjectSpaceChatSidebar.test.tsxwework/src/features/todo/ProjectSpaceChatSidebar.tsxwework/src/features/todo/TodoEditor.tsxwework/src/features/todo/projectProviderConfig.test.tswework/src/features/todo/projectProviderConfig.tswework/src/features/todo/todoShared.tswework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchContextTypes.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
💤 Files with no reviewable changes (1)
- wework/src/components/layout/DesktopWorkbenchLayout.tsx
Comment on lines
+123
to
+125
| if item.assignee_user_id: | ||
| assignee = db.get(User, item.assignee_user_id) | ||
| values["assignee_name"] = assignee.user_name if assignee else None |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find every caller of response_values and check for per-item loops.
set -euo pipefail
rg -n -C8 'response_values' --glob '*.py'Repository: wecode-ai/Wegent
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked Python paths under backend/app/services/loop_items/service.py:\n'
git ls-files 'backend/app/services/loop_items/service.py' 'backend/**/*.py' | sed -n '1,120p'
printf '\nFile exists/stat:\n'
if [ -f backend/app/services/loop_items/service.py ]; then
wc -l backend/app/services/loop_items/service.py
ast-grep outline backend/app/services/loop_items/service.py --view expanded || true
fiRepository: wecode-ai/Wegent
Length of output: 8595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'LoopItemService relevant sections:\n'
cat -n backend/app/services/loop_items/service.py | sed -n '1,340p'
printf '\nSearch for response_values in tracked Python files:\n'
rg -n -C6 'response_values|list_my_work|\\.response_values|list\\(' --glob '*.py' backend/app | sed -n '1,240p'Repository: wecode-ai/Wegent
Length of output: 14395
🏁 Script executed:
#!/bin/bash
set -xeu
printf 'Remaining LoopItemService relevant section:\n'
cat -n backend/app/services/loop_items/service.py | sed -n '340,900p'
printf '\nSearch for response_values / list_my_work / loop_item_service in tracked Python files:\n'
rg -n -C6 'response_values|list_my_work|loop_item_service|service\.list\(|service\.get\(' --glob '*.py' backend/appRepository: wecode-ai/Wegent
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Response schema definitions:\n'
rg -n -C5 'class (LoopItemResponse|LoopItemListResponse|MyWorkItemResponse|MyWorkListResponse)|assignee_name|assignee_user_id' backend/app/schemas --glob '*.py'Repository: wecode-ai/Wegent
Length of output: 5803
Batch assignee lookups on list response paths.
backend/app/api/endpoints/deliveries.py builds LoopItemListResponse and MyWorkListResponse by calling loop_item_service.response_values() for each item. Since response_values() runs one db.get(User, item.assignee_user_id) per task, use a batch user lookup or eager-load assignees before building repeated item responses.
🤖 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 `@backend/app/services/loop_items/service.py` around lines 123 - 125, Update
the list response paths that repeatedly call
loop_item_service.response_values(), especially the LoopItemListResponse and
MyWorkListResponse construction in deliveries.py, to batch-load or eager-load
all assignee users before mapping items. Refactor response_values() and its
callers to reuse the preloaded user data instead of executing db.get(User,
item.assignee_user_id) for each item, while preserving assignee_name behavior
for missing or unassigned users.
Comment on lines
+501
to
+579
| async fn enrich_user_cells(&self, fields: &[Value], records: &mut [Value]) { | ||
| let user_fields = fields | ||
| .iter() | ||
| .filter(|field| field.get("type").and_then(Value::as_str) == Some("user")) | ||
| .filter_map(|field| field.get("id").and_then(Value::as_str)) | ||
| .collect::<Vec<_>>(); | ||
| let mut names = HashMap::new(); | ||
| let mut user_ids = Vec::new(); | ||
| for record in records.iter() { | ||
| for field_id in &user_fields { | ||
| let Some(users) = record | ||
| .get("cells") | ||
| .and_then(|cells| cells.get(*field_id)) | ||
| .and_then(Value::as_array) | ||
| else { | ||
| continue; | ||
| }; | ||
| for user_id in users.iter().filter_map(|user| { | ||
| user.get("userId") | ||
| .or_else(|| user.get("user_id")) | ||
| .and_then(Value::as_str) | ||
| }) { | ||
| if !user_ids.iter().any(|candidate| candidate == user_id) { | ||
| user_ids.push(user_id.to_owned()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| for user_id in user_ids.into_iter().take(30) { | ||
| let Ok(response) = self | ||
| .run(&["contact", "user", "search", "--query", &user_id]) | ||
| .await | ||
| else { | ||
| continue; | ||
| }; | ||
| let user = list_from(&response, &["result", "users", "items", "data"]) | ||
| .into_iter() | ||
| .find(|user| { | ||
| user.get("userId") | ||
| .or_else(|| user.get("user_id")) | ||
| .and_then(Value::as_str) | ||
| == Some(user_id.as_str()) | ||
| }); | ||
| if let Some(name) = user.as_ref().and_then(|user| { | ||
| user.get("name") | ||
| .or_else(|| user.get("nick")) | ||
| .and_then(Value::as_str) | ||
| }) { | ||
| names.insert(user_id, name.to_owned()); | ||
| } | ||
| } | ||
| for record in records { | ||
| for field_id in &user_fields { | ||
| let Some(users) = record | ||
| .get_mut("cells") | ||
| .and_then(|cells| cells.get_mut(*field_id)) | ||
| .and_then(Value::as_array_mut) | ||
| else { | ||
| continue; | ||
| }; | ||
| for user in users { | ||
| let Some(object) = user.as_object_mut() else { | ||
| continue; | ||
| }; | ||
| let user_id = object | ||
| .get("userId") | ||
| .or_else(|| object.get("user_id")) | ||
| .and_then(Value::as_str) | ||
| .map(ToOwned::to_owned); | ||
| if let Some(user_id) = user_id { | ||
| object.insert( | ||
| "name".to_owned(), | ||
| json!(names.get(&user_id).cloned().unwrap_or(user_id)), | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Reduce the per-user process spawns in enrich_user_cells.
The loop awaits contact user search once per distinct user identifier, in sequence. Each run call spawns a DWS CLI process. A board with 30 assignees therefore launches 30 processes in series on every list_board call, and get_board calls list_board. No result is cached between calls.
Two changes limit the cost:
- Resolve the identifiers with a bounded concurrent stream instead of a sequential loop.
- Cache resolved names on the provider, keyed by user identifier.
Also note that take(30) silently drops the remaining identifiers, so those cells keep the raw user identifier as the displayed name.
🤖 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 `@executor/src/task_runtime/aitable_provider.rs` around lines 501 - 579, The
enrich_user_cells method should resolve user identifiers through a bounded
concurrent stream rather than awaiting each contact user search sequentially,
while preserving the existing per-request limit behavior as appropriate. Add
provider-level caching keyed by user identifier, reuse cached names across
calls, and ensure identifiers beyond the current 30-item processing limit are
not silently left unresolved; update the name-enrichment pass to use cached or
newly resolved results.
Comment on lines
+768
to
+807
| fn infer_board_mapping(mapping: &mut Map<String, Value>, fields: &[Value]) { | ||
| let candidates = [ | ||
| ("title_field_id", &["标题", "任务名称", "任务", "名称"][..]), | ||
| ("description_field_id", &["描述", "备注", "详情"][..]), | ||
| ("status_field_id", &["状态", "进度"][..]), | ||
| ("parent_field_id", &["父记录", "父任务"][..]), | ||
| ("priority_field_id", &["优先级"][..]), | ||
| ("assignee_field_id", &["负责人", "执行人"][..]), | ||
| ( | ||
| "due_field_id", | ||
| &["截止时间", "计划结束日期", "截止日期"][..], | ||
| ), | ||
| ]; | ||
| for (key, names) in candidates { | ||
| if mapping_get(mapping, key).is_some() { | ||
| continue; | ||
| } | ||
| let field = fields.iter().find(|field| { | ||
| field | ||
| .get("name") | ||
| .and_then(Value::as_str) | ||
| .is_some_and(|name| names.iter().any(|candidate| name.contains(candidate))) | ||
| }); | ||
| if let Some(field_id) = field | ||
| .and_then(|field| field.get("id")) | ||
| .and_then(Value::as_str) | ||
| { | ||
| mapping.insert(key.to_owned(), json!(field_id)); | ||
| } | ||
| } | ||
| if mapping_get(mapping, "title_field_id").is_none() { | ||
| if let Some(field_id) = fields | ||
| .first() | ||
| .and_then(|field| field.get("id")) | ||
| .and_then(Value::as_str) | ||
| { | ||
| mapping.insert("title_field_id".to_owned(), json!(field_id)); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The title inference can select the wrong field.
The closure accepts a field when its name contains any candidate string. The title candidates include "任务". That substring also appears in "父任务" and "任务状态". fields.iter().find returns the first field in schema order, so a board whose parent or status field precedes the real title field maps title_field_id to that field. The title then renders the wrong cell, and create_board writes the title into the wrong column.
Match the candidates in priority order, prefer an exact name match, and exclude fields already assigned to another mapping key.
🛠️ Proposed direction
+ let mut assigned = std::collections::HashSet::new();
for (key, names) in candidates {
if mapping_get(mapping, key).is_some() {
+ if let Some(id) = mapping_get(mapping, key) {
+ assigned.insert(id.to_owned());
+ }
continue;
}
- let field = fields.iter().find(|field| {
- field
- .get("name")
- .and_then(Value::as_str)
- .is_some_and(|name| names.iter().any(|candidate| name.contains(candidate)))
- });
+ // Prefer an exact match, then fall back to a substring match, and
+ // never reuse a field that another mapping key already claims.
+ let matches = |exact: bool| {
+ names.iter().find_map(|candidate| {
+ fields.iter().find(|field| {
+ let Some(name) = field.get("name").and_then(Value::as_str) else {
+ return false;
+ };
+ let id = field.get("id").and_then(Value::as_str).unwrap_or_default();
+ !assigned.contains(id)
+ && if exact { name == *candidate } else { name.contains(candidate) }
+ })
+ })
+ };
+ let field = matches(true).or_else(|| matches(false));
if let Some(field_id) = field
.and_then(|field| field.get("id"))
.and_then(Value::as_str)
{
mapping.insert(key.to_owned(), json!(field_id));
+ assigned.insert(field_id.to_owned());
}
}🤖 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 `@executor/src/task_runtime/aitable_provider.rs` around lines 768 - 807, Update
infer_board_mapping so title candidates are evaluated in declared priority
order, preferring exact field-name matches before substring matches, and exclude
fields whose IDs are already assigned to other mapping keys. Apply this
selection specifically to title_field_id while preserving the existing fallback
to the first field when no suitable title remains.
Comment on lines
+504
to
+528
| pub fn archive_task(&self, project_id: &str, task_id: &str) -> Result<(), TaskRuntimeError> { | ||
| self.get_task(project_id, task_id)?; | ||
| let connection = self.connection()?; | ||
| let archived_at = now(); | ||
| let updated = connection.execute( | ||
| "WITH RECURSIVE task_tree(id) AS ( | ||
| SELECT id FROM loop_items | ||
| WHERE id = ?1 AND resource_type = 'task' | ||
| AND cloud_project_id = ?2 AND deleted_at IS NULL | ||
| UNION ALL | ||
| SELECT child.id FROM loop_items child | ||
| JOIN task_tree parent ON child.parent_id = parent.id | ||
| WHERE child.resource_type = 'task' | ||
| AND child.cloud_project_id = ?2 AND child.deleted_at IS NULL | ||
| ) | ||
| UPDATE loop_items | ||
| SET deleted_at = ?3, updated_at = ?3, version = version + 1 | ||
| WHERE id IN (SELECT id FROM task_tree)", | ||
| params![task_id, project_id, archived_at], | ||
| )?; | ||
| if updated == 0 { | ||
| return Err(TaskRuntimeError::TaskNotFound); | ||
| } | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Add a version check and one transaction to archive_task.
archive_task archives the task and every descendant with no optimistic-concurrency check. update_task requires a matching version and returns VersionConflict. Archiving is more destructive than an update, so it needs at least the same protection.
The membership check with self.get_task and the recursive UPDATE also run on two separate statements without a transaction. Another writer can reparent or archive rows between the two steps, so the archived subtree can differ from the verified subtree.
Take a version for the target task, and run the check plus the UPDATE inside one TransactionBehavior::Immediate transaction, as update_task does at Lines 440-441. The IPC route todos.archive in executor/src/local/app_ipc.rs (Lines 1013-1021) must then forward the version.
🤖 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 `@executor/src/task_runtime/store.rs` around lines 504 - 528, Update
archive_task to accept the target task version, start a
TransactionBehavior::Immediate transaction before the task check, and perform
both get_task validation and the recursive archive update through that
transaction. Add the matching version predicate to the target-row update, return
VersionConflict when no row is updated due to a version mismatch, and commit the
transaction on success. Update the todos.archive IPC route to receive and
forward the version argument.
Comment on lines
+53
to
66
| async function extractZip(archive, destination) { | ||
| const zip = await JSZip.loadAsync(await readFile(archive)) | ||
| await Promise.all( | ||
| Object.values(zip.files).map(async entry => { | ||
| const output = join(destination, entry.name) | ||
| if (entry.dir) { | ||
| await mkdir(output, { recursive: true }) | ||
| return | ||
| } | ||
| await mkdir(dirname(output), { recursive: true }) | ||
| await writeFile(output, await entry.async('nodebuffer')) | ||
| }) | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject archive entries that escape the destination.
join(destination, entry.name) follows .. segments. A crafted entry name such as ../../evil.js writes outside temporaryDirectory. The previous implementation delegated to an external extractor; this in-process extractor must perform the check itself.
Resolve each output path and confirm that it stays inside the destination.
🛡️ Proposed fix
async function extractZip(archive, destination) {
const zip = await JSZip.loadAsync(await readFile(archive))
+ const root = resolve(destination)
await Promise.all(
Object.values(zip.files).map(async entry => {
- const output = join(destination, entry.name)
+ const output = resolve(destination, entry.name)
+ if (output !== root && !output.startsWith(`${root}${sep}`)) {
+ throw new Error(`Refusing to extract entry outside the destination: ${entry.name}`)
+ }
if (entry.dir) {
await mkdir(output, { recursive: true })
return
}
await mkdir(dirname(output), { recursive: true })
await writeFile(output, await entry.async('nodebuffer'))
})
)
}Add sep to the node:path import:
-import { dirname, join, resolve } from 'node:path'
+import { dirname, join, resolve, sep } from 'node:path'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function extractZip(archive, destination) { | |
| const zip = await JSZip.loadAsync(await readFile(archive)) | |
| await Promise.all( | |
| Object.values(zip.files).map(async entry => { | |
| const output = join(destination, entry.name) | |
| if (entry.dir) { | |
| await mkdir(output, { recursive: true }) | |
| return | |
| } | |
| await mkdir(dirname(output), { recursive: true }) | |
| await writeFile(output, await entry.async('nodebuffer')) | |
| }) | |
| ) | |
| } | |
| async function extractZip(archive, destination) { | |
| const zip = await JSZip.loadAsync(await readFile(archive)) | |
| const root = resolve(destination) | |
| await Promise.all( | |
| Object.values(zip.files).map(async entry => { | |
| const output = resolve(destination, entry.name) | |
| if (output !== root && !output.startsWith(`${root}${sep}`)) { | |
| throw new Error(`Refusing to extract entry outside the destination: ${entry.name}`) | |
| } | |
| if (entry.dir) { | |
| await mkdir(output, { recursive: true }) | |
| return | |
| } | |
| await mkdir(dirname(output), { recursive: true }) | |
| await writeFile(output, await entry.async('nodebuffer')) | |
| }) | |
| ) | |
| } |
🤖 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 `@wework/scripts/prepare-dws-binary.mjs` around lines 53 - 66, Update
extractZip to resolve the destination and each archive entry path, then reject
entries whose resolved output is outside the destination boundary before
creating directories or writing files. Use the node:path separator as needed to
enforce a proper descendant check while allowing paths within the destination.
Comment on lines
+390
to
+393
| <header className="pb-7"> | ||
| <h1 className="text-heading-lg font-semibold">管理项目</h1> | ||
| <p className="mt-1 text-sm text-text-muted">管理项目成员、标签和看板布局。</p> | ||
| </header> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Board settings UI bypasses the Wework i18n pipeline. Both files render user-facing copy as hardcoded Chinese literals instead of translation keys, and the manage view dropped its existing translation usage. The shared root cause is missing i18n wiring for the new board-settings surface.
wework/src/features/todo/CloudProjectManageView.tsx#L390-L393: restoreuseTranslation('common')and replace every literal in this view, including headings, member and tag controls, confirmation prompts, and error fallbacks, with translation keys.wework/src/features/todo/BoardLayoutEditor.tsx#L243-L248: use the same wrapper for the section heading, helper text, display-menu labels, status menu labels,aria-labelvalues, and the default新状态name.
Add the new keys to both wework/src/i18n/locales/en/ and wework/src/i18n/locales/zh-CN/, and update BoardLayoutEditor.test.tsx so it does not assert raw copy.
As per coding guidelines: "Use the local @/hooks/useTranslation wrapper for new Wework code" and "Add new copy to the appropriate Wework namespace in both src/i18n/locales/en/ and src/i18n/locales/zh-CN/; register new namespaces in src/i18n/index.ts".
📍 Affects 2 files
wework/src/features/todo/CloudProjectManageView.tsx#L390-L393(this comment)wework/src/features/todo/BoardLayoutEditor.tsx#L243-L248
🤖 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 `@wework/src/features/todo/CloudProjectManageView.tsx` around lines 390 - 393,
Replace hardcoded user-facing copy with the local useTranslation('common')
wrapper: in wework/src/features/todo/CloudProjectManageView.tsx:390-393 and
throughout the view, restore translation usage for headings, controls, prompts,
and error fallbacks; in wework/src/features/todo/BoardLayoutEditor.tsx:243-248
and throughout the editor, translate headings, helper text, menu labels,
aria-labels, and the default 新状态 name. Add matching keys to both
wework/src/i18n/locales/en/ and wework/src/i18n/locales/zh-CN/, register any new
namespace in wework/src/i18n/index.ts, and update BoardLayoutEditor.test.tsx to
assert translated output rather than raw copy.
Source: Coding guidelines
Comment on lines
+82
to
+201
| const nativeBoardGroupFields: AITableField[] = [ | ||
| { id: 'status', name: '状态', type: 'status', config: null, raw: {} }, | ||
| { id: 'priority', name: '优先级', type: 'singleSelect', config: null, raw: {} }, | ||
| { id: 'assignee', name: '负责人', type: 'user', config: null, raw: {} }, | ||
| { id: 'tag', name: '标签', type: 'tag', config: null, raw: {} }, | ||
| ] | ||
|
|
||
| const nativeBoardStatusColors: Record< | ||
| CloudLoopItem['status'], | ||
| 'gray' | 'blue' | 'orange' | 'purple' | 'green' | ||
| > = { | ||
| inbox: 'gray', | ||
| pending: 'blue', | ||
| in_progress: 'orange', | ||
| in_review: 'purple', | ||
| completed: 'green', | ||
| } | ||
|
|
||
| function aitableCellLabels(value: unknown): string[] { | ||
| if (value === null || value === undefined || value === '') return [] | ||
| return (Array.isArray(value) ? value : [value]) | ||
| .map(entry => { | ||
| if (typeof entry === 'object' && entry !== null) { | ||
| const object = entry as Record<string, unknown> | ||
| return String(object.name ?? object.title ?? object.text ?? '') | ||
| } | ||
| return String(entry) | ||
| }) | ||
| .filter(Boolean) | ||
| } | ||
|
|
||
| function AITableGroupFieldPicker({ | ||
| fields, | ||
| value, | ||
| onChange, | ||
| testIdPrefix = 'dingtalk-board-group', | ||
| searchPlaceholder = '搜索表格字段', | ||
| }: { | ||
| fields: AITableField[] | ||
| value: string | ||
| onChange: (fieldId: string) => void | ||
| testIdPrefix?: string | ||
| searchPlaceholder?: string | ||
| }) { | ||
| const rootRef = useRef<HTMLDivElement>(null) | ||
| const [open, setOpen] = useState(false) | ||
| const [query, setQuery] = useState('') | ||
| const selected = fields.find(field => field.id === value) | ||
| const visibleFields = fields | ||
| .filter(field => `${field.name} ${field.type}`.toLowerCase().includes(query.toLowerCase())) | ||
| .sort((left, right) => { | ||
| const recommended = (field: AITableField) => | ||
| /状态|负责人|优先级|所属项目/.test(field.name) ? 0 : 1 | ||
| return recommended(left) - recommended(right) | ||
| }) | ||
|
|
||
| useEffect(() => { | ||
| if (!open) return | ||
| const close = (event: MouseEvent) => { | ||
| if (!rootRef.current?.contains(event.target as Node)) setOpen(false) | ||
| } | ||
| document.addEventListener('mousedown', close) | ||
| return () => document.removeEventListener('mousedown', close) | ||
| }, [open]) | ||
|
|
||
| return ( | ||
| <div ref={rootRef} className="relative"> | ||
| <button | ||
| type="button" | ||
| data-testid={`${testIdPrefix}-by`} | ||
| onClick={() => setOpen(current => !current)} | ||
| className="flex h-8 min-w-32 items-center justify-between gap-2 rounded-lg border border-border bg-background px-3 text-xs text-text-secondary hover:bg-muted" | ||
| aria-expanded={open} | ||
| > | ||
| <span className="max-w-32 truncate">{selected?.name ?? '选择分组字段'}</span> | ||
| <ChevronDown className="h-3 w-3 shrink-0" /> | ||
| </button> | ||
| {open ? ( | ||
| <div className="absolute left-0 top-9 z-40 w-64 overflow-hidden rounded-xl border border-border bg-background p-1.5 shadow-lg"> | ||
| <label className="flex h-8 items-center gap-2 rounded-lg bg-muted px-2.5 text-text-muted"> | ||
| <Search className="h-3.5 w-3.5" /> | ||
| <input | ||
| autoFocus | ||
| data-testid={`${testIdPrefix}-search`} | ||
| value={query} | ||
| onChange={event => setQuery(event.target.value)} | ||
| placeholder={searchPlaceholder} | ||
| className="min-w-0 flex-1 bg-transparent text-xs text-text-primary outline-none" | ||
| /> | ||
| </label> | ||
| <div className="mt-1 max-h-72 overflow-y-auto overscroll-contain"> | ||
| {visibleFields.map(field => ( | ||
| <button | ||
| key={field.id} | ||
| type="button" | ||
| data-testid={`${testIdPrefix}-option-${field.id}`} | ||
| onClick={() => { | ||
| onChange(field.id) | ||
| setOpen(false) | ||
| setQuery('') | ||
| }} | ||
| className={cn( | ||
| 'flex h-9 w-full items-center rounded-lg px-2.5 text-left text-sm hover:bg-muted', | ||
| field.id === value && 'bg-muted font-medium' | ||
| )} | ||
| > | ||
| <span className="min-w-0 flex-1 truncate">{field.name}</span> | ||
| <span className="ml-2 shrink-0 text-xs text-text-muted">{field.type}</span> | ||
| {field.id === value ? <Check className="ml-2 h-3.5 w-3.5" /> : null} | ||
| </button> | ||
| ))} | ||
| {visibleFields.length === 0 ? ( | ||
| <p className="px-3 py-6 text-center text-xs text-text-muted">没有匹配字段</p> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Split this file; it is now 2449 lines.
The repository guideline sets a 1000-line limit, and this change grows the file further. Three extractions are self-contained and reduce the component body directly:
- Move
AITableGroupFieldPicker(lines 113-201) into its own module.CloudTodoBoardCard.tsxalready shows this pattern. - Move
nativeBoardGroupFields,nativeBoardStatusColors, andaitableCellLabels(lines 82-111) intotodoShared.ts. - Move the
boardColumnsconstruction (lines 905-982), which is a five-level nested ternary of about 78 lines recomputed on every render, into a memoized helper module.
As per coding guidelines: "Favor cohesive modules, explicit interfaces, and standard practices; split files over 1000 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 82 - 201, Split
CloudTodoWorkspace into cohesive modules to keep it under the 1000-line
guideline: move AITableGroupFieldPicker into its own module following
CloudTodoBoardCard.tsx’s pattern, move nativeBoardGroupFields,
nativeBoardStatusColors, and aitableCellLabels into todoShared.ts, and extract
the boardColumns construction into a memoized helper module while preserving its
existing behavior and interfaces.
Source: Coding guidelines
Comment on lines
+1328
to
1344
| } else { | ||
| setItems(current => | ||
| current.map(candidate => | ||
| candidate.id === item.id ? { ...candidate, source_status: sourceStatus } : candidate | ||
| ) | ||
| ) | ||
| try { | ||
| const itemApi = apiForProjectId(item.cloud_project_id) | ||
| if (!itemApi) throw new Error('项目空间当前不可用') | ||
| const updated = await itemApi.updateLoopItem(item.id, { | ||
| version: item.version, | ||
| status: sourceStatus as CloudLoopItem['status'], | ||
| current.map(candidate => { | ||
| if (candidate.id !== itemId) return candidate | ||
| if (nativeGroupBy === 'priority') { | ||
| return { ...candidate, priority: column.groupValue as CloudLoopItem['priority'] } | ||
| } | ||
| if (nativeGroupBy === 'assignee') { | ||
| return { | ||
| ...candidate, | ||
| assignee_user_id: column.groupValue ? Number(column.groupValue) : null, | ||
| } | ||
| } | ||
| return { ...candidate, tags: column.groupValue ? [column.groupValue] : [] } | ||
| }) | ||
| setItems(current => | ||
| current.map(candidate => (candidate.id === updated.id ? updated : candidate)) | ||
| ) | ||
| } catch (cause) { | ||
| setItems(previousItems) | ||
| setBoardError(cause instanceof Error ? cause.message : '移动任务失败') | ||
| } | ||
| return | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Dragging a card between tag columns deletes its other tags.
For nativeGroupBy === 'tag', line 1341 and line 1356 both set tags to a single-element array built from column.groupValue. A task with ['发布', '线上问题'] dragged into the 发布 column keeps only 发布. An empty groupValue, which is the 无标签 column, clears every tag. The gesture therefore destroys data that the user did not intend to change, and there is no confirmation. Replace only the tag that the current grouping owns, and keep the remaining tags.
🐛 Proposed fix to preserve tags outside the grouping
+ const retagItem = (candidate: CloudLoopItem): string[] => {
+ const groupTags = new Set(availableTags)
+ const preserved = (candidate.tags ?? []).filter(tag => !groupTags.has(tag))
+ return column.groupValue ? [...preserved, column.groupValue] : preserved
+ }- return { ...candidate, tags: column.groupValue ? [column.groupValue] : [] }
+ return { ...candidate, tags: retagItem(candidate) }- : { tags: column.groupValue ? [column.groupValue] : [] }
+ : { tags: retagItem(item) }Also applies to: 1349-1356
🤖 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 1328 - 1344,
Update the tag-handling branches in the item move logic, including the mapping
callback and the corresponding code around the alternate path, so dragging
between tag columns replaces only the tag owned by the current grouping while
preserving all other tags. Treat an empty column.groupValue as removing only the
grouping tag rather than clearing candidate.tags, and leave priority and
assignee behavior unchanged.
Comment on lines
2223
to
2231
| <DragOverlay dropAnimation={null}> | ||
| {activeDragItemId ? ( | ||
| <div className="w-[272px] rotate-1 rounded-xl border border-border bg-background p-3 text-left shadow-lg"> | ||
| <TodoCardContent | ||
| <CloudTodoCardContent | ||
| item={items.find(item => item.id === activeDragItemId)!} | ||
| display={boardCardDisplay} | ||
| /> | ||
| </div> | ||
| ) : null} |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The drag overlay dereferences a possibly missing item.
Line 2227 uses a non-null assertion on items.find(...). The board refresh interval at lines 1272-1274 replaces items every 15 seconds, and confirmArchiveItem removes items too. If the dragged item disappears from items while a drag is active, find returns undefined and CloudTodoCardContent throws on item.title, which unmounts the workspace. Resolve the item first and render the overlay only when it exists.
🐛 Proposed fix to remove the non-null assertion
<DragOverlay dropAnimation={null}>
- {activeDragItemId ? (
- <div className="w-[272px] rotate-1 rounded-xl border border-border bg-background p-3 text-left shadow-lg">
- <CloudTodoCardContent
- item={items.find(item => item.id === activeDragItemId)!}
- display={boardCardDisplay}
- />
- </div>
- ) : null}
+ {(() => {
+ const draggedItem = activeDragItemId
+ ? (items.find(item => item.id === activeDragItemId) ?? null)
+ : null
+ if (!draggedItem) return null
+ return (
+ <div className="w-[272px] rotate-1 rounded-xl border border-border bg-background p-3 text-left shadow-lg">
+ <CloudTodoCardContent item={draggedItem} display={boardCardDisplay} />
+ </div>
+ )
+ })()}
</DragOverlay>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <DragOverlay dropAnimation={null}> | |
| {activeDragItemId ? ( | |
| <div className="w-[272px] rotate-1 rounded-xl border border-border bg-background p-3 text-left shadow-lg"> | |
| <TodoCardContent | |
| <CloudTodoCardContent | |
| item={items.find(item => item.id === activeDragItemId)!} | |
| display={boardCardDisplay} | |
| /> | |
| </div> | |
| ) : null} | |
| <DragOverlay dropAnimation={null}> | |
| {(() => { | |
| const draggedItem = activeDragItemId | |
| ? (items.find(item => item.id === activeDragItemId) ?? null) | |
| : null | |
| if (!draggedItem) return null | |
| return ( | |
| <div className="w-[272px] rotate-1 rounded-xl border border-border bg-background p-3 text-left shadow-lg"> | |
| <CloudTodoCardContent item={draggedItem} display={boardCardDisplay} /> | |
| </div> | |
| ) | |
| })()} | |
| </DragOverlay> |
🤖 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 `@wework/src/features/todo/CloudTodoWorkspace.tsx` around lines 2223 - 2231,
Update the DragOverlay rendering in CloudTodoWorkspace to resolve the active
item from items and render CloudTodoCardContent only when that lookup returns an
item; remove the non-null assertion from items.find(...). Preserve the existing
overlay styling and null rendering when there is no active drag item or the item
has disappeared.
Comment on lines
+733
to
739
| {status === '' ? <option value="">未设置</option> : null} | ||
| {statusOptions.map(option => ( | ||
| <option key={option.id} value={option.id}> | ||
| {option.name} | ||
| </option> | ||
| ))} | ||
| </select> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the CloudLoopItem status type definition.
rg -nP -C 6 "status:\s*" wework/src/api/deliveries.ts | head -60
ast-grep run --pattern 'export interface CloudLoopItem { $$$ }' --lang typescript wework/src/api/deliveries.tsRepository: wecode-ai/Wegent
Length of output: 3410
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate TodoEditor and inspect relevant sections"
fd -a 'TodoEditor\.tsx$' . | sed 's#^\./##'
echo
echo "Outline TodoEditor"
ast-grep outline wework/src/features/todo/TodoEditor.tsx --match 'TodoEditor' --view expanded 2>/dev/null || true
echo
echo "Status-related code around select and save"
rg -n "statusOptions|status === ''|onChange|statusOptions|board_config|statusOptions|save|reorder|update" wework/src/features/todo/TodoEditor.tsx | sed -n '1,220p'
echo
echo "File ranges 700-770 and status handling area"
nl -ba wework/src/features/todo/TodoEditor.tsx | sed -n '680,770p'
echo "Status and statusOptions definitions/assignments"
rg -n -C 4 "set\s*status|statusOptions|use.*status|board_config|getStatus" wework/src/features/todo/TodoEditor.tsxRepository: wecode-ai/Wegent
Length of output: 2525
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n wework/src/features/todo/TodoEditor.tsx | sed -n '200,480p'
echo
cat -n wework/src/features/todo/TodoEditor.tsx | sed -n '700,745p'
echo
cat -n wework/src/features/todo/TodoEditor.tsx | sed -n '1080,1125p'Repository: wecode-ai/Wegent
Length of output: 17343
Render the current status when it is not in statusOptions.
In edit mode, status is initialized from item.status, but the <select> only creates the placeholder option when status === ''. If an old item has a status not present in project.board_config.statuses, the element selects the first option while status stays unchanged; the subsequent save then sends that stale status. Add the missing current status as an option when needed.
🤖 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 `@wework/src/features/todo/TodoEditor.tsx` around lines 733 - 739, Update the
status select rendering in TodoEditor so that when the current status is
non-empty and absent from statusOptions, it adds an option for that status,
while preserving the existing placeholder and mapped options. Use the current
status value and a suitable label, and ensure normal statuses already present in
statusOptions are not duplicated.
qdaxb
enabled auto-merge
July 31, 2026 03:54
auto-merge was automatically disabled
July 31, 2026 06:24
Head branch was pushed to by a user without write access
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wework/src-tauri/src/system_sleep.rs (2)
257-260: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle
SetThreadExecutionStatefailures when clearing inhibition.
SetThreadExecutionStatereturns zero on failure, but the Windows cleanup path discards the result. A failedES_CONTINUOUSreset can leave sleep inhibition active without logging. Capture the return value and log a warning when it is zero.🤖 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 `@wework/src-tauri/src/system_sleep.rs` around lines 257 - 260, Update the cleanup path around SetThreadExecutionState when clearing inhibition to capture its return value and log a warning if it is zero, while preserving the existing ES_CONTINUOUS reset behavior.
166-169: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftKeep Windows sleep inhibition on the same OS thread.
SetThreadExecutionStateis per-thread under Windows, andES_CONTINUOUSremains active until the acquiring thread calls it again with onlyES_CONTINUOUS.SystemSleepStateis shared across Tauri task threads, soSleepInhibitor::acquire()andDropcan run on different threads; the mismatch branch only logs instead of releasing the power request. Use a dedicated stable OS thread or thread-affine ownership, and add real-Tauri Windows coverage for task start, task settlement, and inhibitor release.🤖 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 `@wework/src-tauri/src/system_sleep.rs` around lines 166 - 169, Update the Windows-specific SleepInhibitor acquire/drop flow and shared SystemSleepState ownership so SetThreadExecutionState is always acquired and released on the same stable OS thread; do not merely log when thread IDs differ. Use dedicated thread-affine ownership, and add real-Tauri Windows coverage covering task start, task settlement, and inhibitor release.Source: Coding guidelines
🧹 Nitpick comments (1)
wework/src-tauri/src/system_sleep.rs (1)
6-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused Windows process-spawn declarations from Windows sleep inhibition.
system_sleep.rsonly usesSetThreadExecutionStateon Windows, soCommandExtandCREATE_NO_WINDOWare unused here. Remove these declarations with the obsolete PowerShell path to keep the Windows implementation simpler.Proposed cleanup
-#[cfg(target_os = "windows")] -use std::os::windows::process::CommandExt; - -#[cfg(target_os = "windows")] -const CREATE_NO_WINDOW: u32 = 0x0800_0000;🤖 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 `@wework/src-tauri/src/system_sleep.rs` around lines 6 - 11, Remove the Windows-only std::os::windows::process::CommandExt import and CREATE_NO_WINDOW constant from system_sleep.rs, along with the obsolete PowerShell process-spawn path that depends on them. Preserve the existing SetThreadExecutionState-based sleep inhibition implementation and unrelated task ID handling.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@wework/src-tauri/src/system_sleep.rs`:
- Around line 257-260: Update the cleanup path around SetThreadExecutionState
when clearing inhibition to capture its return value and log a warning if it is
zero, while preserving the existing ES_CONTINUOUS reset behavior.
- Around line 166-169: Update the Windows-specific SleepInhibitor acquire/drop
flow and shared SystemSleepState ownership so SetThreadExecutionState is always
acquired and released on the same stable OS thread; do not merely log when
thread IDs differ. Use dedicated thread-affine ownership, and add real-Tauri
Windows coverage covering task start, task settlement, and inhibitor release.
---
Nitpick comments:
In `@wework/src-tauri/src/system_sleep.rs`:
- Around line 6-11: Remove the Windows-only
std::os::windows::process::CommandExt import and CREATE_NO_WINDOW constant from
system_sleep.rs, along with the obsolete PowerShell process-spawn path that
depends on them. Preserve the existing SetThreadExecutionState-based sleep
inhibition implementation and unrelated task ID handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d45c1d5-3f8e-4094-96fd-5f97ec2cda98
📒 Files selected for processing (3)
wework/package.jsonwework/src-tauri/src/system_sleep.rswework/src/features/todo/AITableView.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- wework/src/features/todo/AITableView.test.tsx
- wework/package.json
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit