fix(wework): archive tasks without triggering a cloud sync - #2421
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe workbench adds optional cloud-sync control to work-list and device refreshes. Archive and stream-event paths now update local state without automatic cloud requests. Cloud sync cleanup handles cancelled and superseded refreshes. Tests cover these flows. ChangesWork-list refresh control
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant archiveRuntimeConversations
participant refreshWorkLists
participant cloudRuntimeSync
archiveRuntimeConversations->>refreshWorkLists: refreshWorkLists({ syncCloud: false })
refreshWorkLists-->>archiveRuntimeConversations: update local work lists
refreshWorkLists-xcloudRuntimeSync: skip cloud synchronization
refreshWorkLists->>cloudRuntimeSync: synchronize when syncCloud is not false
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wework/src/features/workbench/useWorkbenchDataRefresh.ts (1)
626-627: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared
RefreshWorkListstype.The implementation repeats
{ syncCloud?: boolean }instead of using the exportedRefreshWorkListstype. Annotate the callback withRefreshWorkListsand import it as a type. This keeps the implementation and context contracts synchronized.As per coding guidelines, reuse existing abstractions instead of duplicating the refresh contract.
Proposed change
+import type { RefreshWorkLists } from './workbenchContextTypes' + - const refreshWorkLists = useCallback( - async (options?: { syncCloud?: boolean }) => { + const refreshWorkLists: RefreshWorkLists = useCallback( + async options => {🤖 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/workbench/useWorkbenchDataRefresh.ts` around lines 626 - 627, Update refreshWorkLists in useWorkbenchDataRefresh.ts to use the exported RefreshWorkLists type instead of the inline { syncCloud?: boolean } signature, and import RefreshWorkLists as a type. Preserve the callback’s existing behavior while keeping its contract aligned with the shared context type.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/features/workbench/useWorkbenchDataRefresh.ts`:
- Around line 646-654: Update the fallback runtime-work selection in the refresh
flow around filteredRuntimeWorkResult so that when listRuntimeWork fails and the
result is undefined, state.runtimeWork is passed through removeRuntimeTasks
using archivedRuntimeTaskAddressesRef.current before being assigned to
runtimeWork. Preserve the existing successful-result filtering and add a
regression test covering a rejected listRuntimeWork request during archive
refresh, verifying the archived task remains removed.
---
Nitpick comments:
In `@wework/src/features/workbench/useWorkbenchDataRefresh.ts`:
- Around line 626-627: Update refreshWorkLists in useWorkbenchDataRefresh.ts to
use the exported RefreshWorkLists type instead of the inline { syncCloud?:
boolean } signature, and import RefreshWorkLists as a type. Preserve the
callback’s existing behavior while keeping its contract aligned with the shared
context type.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69a64390-07aa-4690-af3c-8fc0cde7d6a6
📒 Files selected for processing (5)
wework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/useWorkbenchDataRefresh.tswework/src/features/workbench/useWorkbenchRuntimeTasks.tswework/src/features/workbench/workbenchContextTypes.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 6587-6618: Update the test around ArchiveRemoteRuntimeTaskProbe to
clear the mocked runtimeWorkApi.listRuntimeWork call history after the initial
render completes, then assert after the archive action that
runtimeWorkApi.listRuntimeWork was called exactly once. Keep the existing
optimistic-removal assertions and configure the mock rejection so the test
verifies the failed local refresh path.
- Around line 6651-6701: Strengthen the `onRuntimeGoalCleared` test in the
archive-remote-task flow by asserting that `streamHandlers.onRuntimeGoalCleared`
is registered as a function before dispatching the event, then invoke it
directly without optional chaining. Preserve the existing assertions verifying
the archived task remains absent and `cloudListRuntimeWork` is not called again.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6dc7caab-74e4-4b77-b2e1-36c10d1f0a9d
📒 Files selected for processing (3)
wework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/WorkbenchProvider.tsxwework/src/features/workbench/useWorkbenchDataRefresh.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- wework/src/features/workbench/WorkbenchProvider.tsx
- wework/src/features/workbench/useWorkbenchDataRefresh.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wework/src/features/workbench/useWorkbenchDeviceUpgrades.ts (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the exported
RefreshDevicestype.
wework/src/features/workbench/workbenchContextTypes.tsalready exports the sameuseCacheFallbackandsyncCloudoptions. Use that type forUseWorkbenchDeviceUpgradesOptions.refreshDevicesinstead of duplicating the inline signature.As per coding guidelines, reuse existing abstractions instead of duplicating type contracts.
Proposed type reuse
+import type { RefreshDevices } from './workbenchContextTypes' + interface UseWorkbenchDeviceUpgradesOptions { - refreshDevices: (options?: { useCacheFallback?: boolean; syncCloud?: boolean }) => Promise<void> + refreshDevices: RefreshDevices }🤖 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/workbench/useWorkbenchDeviceUpgrades.ts` at line 27, Update UseWorkbenchDeviceUpgradesOptions.refreshDevices in useWorkbenchDeviceUpgrades.ts to reuse the exported RefreshDevices type from workbenchContextTypes.ts instead of repeating the inline options signature. Keep the same refreshDevices contract and parameter shape, and adjust the type reference so the shared type remains the single source of truth.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 2836-2840: Update the socket-event test around onDeviceSlotUpdate
to assert the handler is registered, then invoke it directly without optional
chaining. Await a positive local refresh signal, such as the device-list mock
completing, before asserting cloudListRuntimeWork was called once, ensuring any
asynchronous refresh has finished.
---
Nitpick comments:
In `@wework/src/features/workbench/useWorkbenchDeviceUpgrades.ts`:
- Line 27: Update UseWorkbenchDeviceUpgradesOptions.refreshDevices in
useWorkbenchDeviceUpgrades.ts to reuse the exported RefreshDevices type from
workbenchContextTypes.ts instead of repeating the inline options signature. Keep
the same refreshDevices contract and parameter shape, and adjust the type
reference so the shared type remains the single source of truth.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: da596b26-ccd6-4d85-83b2-77c1b312a61f
📒 Files selected for processing (4)
wework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/useWorkbenchDataRefresh.tswework/src/features/workbench/useWorkbenchDeviceUpgrades.tswework/src/features/workbench/workbenchContextTypes.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- wework/src/features/workbench/useWorkbenchDataRefresh.ts
| await act(async () => { | ||
| streamHandlers.onDeviceSlotUpdate?.({ device_id: 'device-1' }) | ||
| }) | ||
|
|
||
| expect(cloudListRuntimeWork).toHaveBeenCalledTimes(1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the socket-event test exercise and await the refresh.
Line 2837 uses optional chaining, so the test passes when onDeviceSlotUpdate is not registered. The callback also starts void refreshDevices(...), so the test does not explicitly wait for the local refresh to complete. A cloud request from a regression can occur after Line 2840.
Assert that the handler is registered, invoke it without optional chaining, and wait for a positive local-refresh signal, such as the device-list mock, before checking the cloud call count.
Based on the previous review comment, this repeats the same optional-handler assertion gap.
Proposed assertion
+ expect(streamHandlers.onDeviceSlotUpdate).toEqual(expect.any(Function))
+
await act(async () => {
- streamHandlers.onDeviceSlotUpdate?.({ device_id: 'device-1' })
+ streamHandlers.onDeviceSlotUpdate!({ device_id: 'device-1' })
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx` around lines 2836 -
2840, Update the socket-event test around onDeviceSlotUpdate to assert the
handler is registered, then invoke it directly without optional chaining. Await
a positive local refresh signal, such as the device-list mock completing, before
asserting cloudListRuntimeWork was called once, ensuring any asynchronous
refresh has finished.
1 participant
背景
在 Wework 桌面端归档任务后，侧边栏“云端工作”会变成“同步中”，且长时间不恢复。
根因（日志定位）
归档本身不会主动触发云端同步，真正的问题是状态机被归档打断后卡死：
removeRuntimeTasksFromCloudState会把inFlightRevision置空——如果当时有一个云端同步正在进行（例如启动引导同步），它的完成校验就会失效，finishCloudRuntimeSync提前返回，导致同步的syncing检查状态永远不更新。refreshCloudBackgroundData在同步被后续刷新取代（请求被 abort）时同样提前返回，不清理inFlightRevision、不恢复syncing状态。云端工作的可用性检查永久停留在syncing，即“同步中”卡死，直到下一次完整同步成功才覆盖。用户每次归档后看到的“同步中”正是被归档打断后卡住的旧状态。修改
refreshWorkLists({ syncCloud: false })，不再触发云端后台同步。clearCloudRuntimeSync/abandonCloudRuntimeSync：任何打断/取代同步的路径（归档、被新刷新取代）都会清空inFlightRevision，并把syncing检查状态恢复为上一次已知值（有 lastGood 时显示“可用”，否则回到“未检查”），避免永久卡在“同步中”。archivedRuntimeTaskAddressesRef保持隐藏（本地剔除的可靠性）。inFlightRevision保持不变，避免被清理逻辑误伤。影响
验证
cloudWorkStatus.availability不再停留在syncing；tsc -b、eslint、prettier、pre-push 质量门禁全部通过。