fix: 修復定期同步與防抖計時器競態問題

[GJRobert]

1. syncFromCloud 執行時清除 debounce timer，避免同步完成後又重複同步
2. 週期性背景同步執行前清除 debounce timer，避免與用戶觸發的同步衝突

https://claude.ai/code/session_01VVpR7srpxsjmWHHUGi4Sgt

[gemini-code-assist]

Summary of Changes

Hello @GJRobert, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

此拉取請求透過實作強健的防抖計時器清除機制，解決了雲端同步相關的競態問題。它確保了使用者觸發和週期性背景同步操作都能正確管理待處理的同步請求，從而防止冗餘或衝突的同步，並提升雲端同步功能的整體穩定性。

Highlights

• syncFromCloud 執行時的防抖處理: 在 syncFromCloud 函數開始執行時，會清除任何待處理的 syncDebounceTimer 並將 hasPendingSync 設為 false，以防止在同步完成後重複觸發同步。
• 週期性背景同步的防抖處理: 在週期性背景同步執行 syncFromCloud 之前，會清除任何待處理的 syncDebounceTimer 並將 hasPendingSync 設為 false，以避免與用戶觸發的同步操作產生衝突。

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[claude]

PR Review: 修復定期同步與防抖計時器競態問題

Summary

This PR addresses a race condition between periodic background sync and debounced user-triggered sync by clearing the debounce timer when sync operations begin. The changes are focused and appropriate for the issue being fixed.

Code Quality ✅

Strengths:

• Clear intent: The changes directly address the race condition described in the PR
• Consistent pattern: The same cleanup logic is applied in both locations (js/cloud-sync.js:128-133 and js/cloud-sync.js:453-458)
• Well-documented: Chinese comments clearly explain why the timer is being cleared
• Minimal scope: Only adds necessary code without over-engineering

Observations:

• The fix is defensive and prevents duplicate sync operations
• Code follows existing patterns in the file (similar cleanup in signOut() at lines 95-100)

Potential Issues ⚠️

1. Code Duplication

The same cleanup logic appears in 3 places:

• syncFromCloud() (lines 128-133) - NEW
• startPeriodicSync() callback (lines 453-458) - NEW
• signOut() (lines 95-100) - existing

Suggestion: Consider extracting this into a helper function:

function clearPendingSync() {
  if (syncDebounceTimer) {
    clearTimeout(syncDebounceTimer);
    syncDebounceTimer = null;
  }
  hasPendingSync = false;
}

Then call it in all three locations. This improves maintainability and reduces the chance of bugs if the cleanup logic needs to change.

2. Potential Race Condition (Edge Case)

In syncFromCloud(), the debounce timer is cleared BEFORE checking cloudSyncState.isSyncing. Consider this sequence:

1. User triggers sync → triggerCloudSync() sets 30s timer
2. Immediately after, periodic sync fires → clears the timer
3. Periodic sync exits early due to isSyncing guard
4. User's intended sync never happens (timer was cleared)

Current code:

if (cloudSyncState.isSyncing) return; // Line 126

// Clear timer (lines 128-133)
if (syncDebounceTimer) {
  clearTimeout(syncDebounceTimer);
  syncDebounceTimer = null;
}

Suggested fix: Only clear the timer if sync will actually proceed:

// 競合保護：避免同時執行多個同步
if (cloudSyncState.isSyncing) return;

cloudSyncState.isSyncing = true;

// 清除待處理的 debounce timer，避免同步完成後又重複同步
if (syncDebounceTimer) {
  clearTimeout(syncDebounceTimer);
  syncDebounceTimer = null;
}
hasPendingSync = false;

Move the timer clearing AFTER setting isSyncing = true to ensure the sync will actually execute.

3. Missing Edge Case in setupPageUnloadSync()

The visibility change handler (lines 482-507) clears the debounce timer when page is hidden, but the periodic sync timer clearing in startPeriodicSync() might interfere with this logic if timing is unlucky.

Performance Considerations ✅

• Positive impact: Reduces unnecessary sync API calls
• No performance regressions: Clearing timers is a lightweight operation
• Debounce window: 30 seconds is appropriate for this use case

Security Concerns ✅

No security issues identified. The changes only affect sync timing, not data handling or authentication.

Test Coverage ⚠️

Missing test scenarios:

1. User triggers sync → periodic sync fires before completion → verify no duplicate sync
2. Periodic sync running → user triggers sync → verify debounce timer is properly cleared
3. Multiple rapid user actions → periodic sync → verify correct behavior

Recommendation: Add integration tests for these race conditions, possibly using jest.useFakeTimers() to control timing.

Additional Observations

Good Practices Already in Place:

• Proper use of guards (if (cloudSyncState.isSyncing) return)
• Consistent state management with hasPendingSync flag
• Visibility API usage to save resources (line 452)

Documentation:

The Chinese comments are clear, but consider adding a brief explanation in the commit message or code comments about:

• Why this race condition matters (data consistency, API quota, user experience)
• The specific scenario that could occur without this fix

Recommendations

Priority: High

1. Fix the race condition edge case in syncFromCloud() by moving timer cleanup after isSyncing = true

Priority: Medium

2. Extract duplicate cleanup logic into a helper function
3. Add integration tests for the race condition scenarios

Priority: Low

4. Consider adding debug logging when timer is cleared to help with future debugging:

   if (syncDebounceTimer) {
     console.log('[CloudSync] 清除待處理的同步計時器');
     clearTimeout(syncDebounceTimer);
     syncDebounceTimer = null;
   }

Conclusion

This is a solid bug fix that addresses a real race condition issue. The implementation is straightforward and follows existing patterns in the codebase. With the suggested refinements above (especially the placement of the timer cleanup), this will be even more robust.

Recommendation: Request changes for the race condition edge case, then approve.

---

Review generated by Claude Code

[gemini-code-assist]

Code Review

這份 PR 透過在 syncFromCloud 和 startPeriodicSync 函式中加入清除防抖計時器 (debounce timer) 的邏輯，成功修復了定期同步與使用者觸發同步之間的競態問題。程式碼變更方向正確，能有效避免不必要的重複同步。

我唯一的建議是關於程式碼的重複性。新加入的清除計時器邏輯在兩個地方是相同的，可以考慮將其提取到一個共用的輔助函式中，以提高程式碼的可讀性和可維護性。我在程式碼中留下了具體的重構建議。

[gemini-code-assist]

這段用來清除待處理同步的邏輯，在 startPeriodicSync 函式中也重複出現了 (L454-458)。為了提高程式碼的可維護性並遵循 DRY (Don't Repeat Yourself) 原則，建議將這段邏輯提取到一個獨立的輔助函式中。

例如，你可以建立一個 cancelPendingSync 函式：

function cancelPendingSync() {
  if (syncDebounceTimer) {
    clearTimeout(syncDebounceTimer);
    syncDebounceTimer = null;
  }
  hasPendingSync = false;
}

然後在 syncFromCloud 和 startPeriodicSync 中直接呼叫 cancelPendingSync() 即可。這樣做也能讓程式碼的意圖更清晰。

順帶一提，在 signOut (L96) 和 setupPageUnloadSync (L486) 中也有類似的邏輯，也可以考慮一併重構。