Skip to content

feat: prevent stale refresh overwrites and add ordering conflict recovery - #996

Open
devin-ai-integration[bot] wants to merge 2 commits into
devin/1785276696-reorder-coordinatorfrom
devin/1785279143-stale-refresh
Open

feat: prevent stale refresh overwrites and add ordering conflict recovery#996
devin-ai-integration[bot] wants to merge 2 commits into
devin/1785276696-reorder-coordinatorfrom
devin/1785279143-stale-refresh

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Closes #980. Part of epic #975. Stacked on #995.

Summary

Makes Browse's category/uncategorized-image reads stale-proof and gives ordering conflicts a deliberate resolution path.

  • Latest-request-wins sequencing (useBrowseData): every category-tree and uncategorized-image read claims a generation (categoriesReadGen/uncategorizedReadGen); only the newest read may commit state, so a slow older response (foreground or background) can never overwrite newer data regardless of completion order. A separate visibleCategoriesLoadGen (claimed only by non-silent loads) owns the loading flag, so a silent poll finishing later cannot strand the spinner.
  • Cancellation: foreground loads and authoritative refreshes (refreshCategories/refreshUncategorizedImages) abort the previous read for the same data via per-data AbortControllers; AbortError is treated as expected control flow — superseded refreshes resolve with the freshest committed data (mirrored via refs) instead of rejecting, so no false "Could not refresh" warnings or unhandled rejections.
  • Pending-order protection: the 30 s background poll skips its reads while tileOrderingCoordinator.hasUnsavedChanges() (dirty / saving / queued / conflicted / awaiting retry), so a polling response can never replace local pending order; polling resumes on the next tick once clean.
  • Conflict recovery: 409 now offers two explicit paths in ReorderStatusIndicator — existing Refresh (acceptServerOrder) plus new Keep my order (TileOrderingCoordinator.reapplyLocalOrder), which resubmits the newest local intent against the authoritative revision from the conflict body. Never silent last-write-wins.
  • Scope-specific reads: conflict refreshes continue to use GET /api/tile-order for the affected scope; successful reorders still avoid full-tree reloads (from feat: add navigation-safe frontend reorder coordinator and save-state UX #995).
  • Docs: new "Stale-refresh prevention and conflict recovery (Prevent stale refresh overwrites and add ordering conflict recovery #980)" section in docs/tile-ordering.md.

Testing

  • New tests in tests/useBrowseData.test.ts: older response cannot overwrite newer (categories + images), foreground load aborts prior read and its response is discarded, superseded refresh resolves with freshest committed data, silent read cannot strand the visible spinner, aborts are control flow (no console error), polling pauses while unsaved and resumes after resolution.
  • New coordinator test: reapplyLocalOrder re-PUTs the newest local intent with the conflict revision; new indicator test for "Keep my order".
  • Full frontend suite: 88 files / 1366 passed (3 expected fail); npm run build clean.
  • Note: npm run lint has one pre-existing error in src/components/CanvasOverlay.tsx (react-refresh/only-export-components, introduced by fix: correct Fabric text box resizing #984 on main) — present on the base branch, untouched here.

Needs human verification

Link to Devin session: https://app.devin.ai/sessions/414754bd88374dc693f47100ac269e78

…very

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

Open in Devin Review

Comment thread frontend/src/useBrowseData.ts
Comment thread frontend/src/useBrowseData.ts Outdated
Comment thread frontend/src/useBrowseData.ts
Comment thread frontend/src/tileOrdering.ts
…g owned by visible loads

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread frontend/src/useBrowseData.ts
Comment thread frontend/src/useBrowseData.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants