feat(sync): surface save conflicts detected by negotiate

[sdornan]

Description
^{Explain the changes or enhancements you are proposing with this pull request.}

Save conflicts were half-wired. The SSH push-pull path already resolves them server-side and calls emit_sync_conflict, but the API negotiate path only reported action: "conflict" in its JSON response. On top of that, nothing in frontend/src subscribed to any sync:* event, so a conflict was invisible to the user either way.

This closes both halves:

• negotiate_sync emits sync:conflict once per conflicting operation, after the session is updated so session_id is valid. The route itself stays a sync def, so its handler calls keep running in the threadpool; only the emit is async, reached through asyncio.run because _get_socket_manager() builds a write-only manager per call rather than holding the app's. @protected_route branches on is_async_callable, so the signature decides where the body runs.
• The emit is fail-open and bounded: a negotiation must not fail, or stall a client's emulator launch, because the user could not be notified. The events go out together behind a 2s deadline and a cap on concurrent emits, so neither the wait nor the number of Redis connections grows with the number of conflicts.
• A new v2 composable, useSyncConflictToast, consumes the event and shows a warning toast naming the game. It resolves the name from a loaded cache (galleryRoms, currentRom, recentRoms) with no network request, and falls back to a generic string when the game is not on any loaded surface.

The toast dedupes per page session and per device, because a conflict is re-reported on every negotiation until it is resolved; without that, the user would be told twice per play session, at launch and at exit. Keying on the device as well keeps a second device's conflict on the same save from being swallowed.

This is the first piece of save sync between RomM and the romm-desktop shell, which rides this REST negotiate protocol. The shell itself, states sync, and any conflict resolution flow are follow-ups.

flowchart LR
    shell[romm-desktop shell] -->|"POST /api/sync/negotiate"| negotiate["negotiate_sync<br/>(sync route, async emit)"]
    negotiate -->|"action: conflict<br/>asyncio.run"| emit["emit_sync_conflict"]
    emit -->|"sync:conflict, room user:{id}"| socket[Socket.IO]
    socket --> consumer["useSyncConflictToast<br/>(new)"]
    consumer -->|snackbarShow| host[NotificationHost]

    subgraph existing["already existed"]
        emit
        socket
        host
    end

Loading

Checklist
^{Please check all that apply.}

• I've tested the changes locally
• I've updated relevant comments
• I've assigned reviewers for this PR
• I've added unit tests that cover the changes

Unit tests cover the emit (payload, silence on a no_op, the fail-open contract when the emitter raises, the deadline that keeps a slow notification from stalling a launch, and the cap on concurrent emits), and the frontend test covers the named toast, the generic fallback, and the per-device dedupe.

Local checks: pytest tests/endpoints/test_sync.py 46 passed; npm run test 1366 passed; npm run typecheck clean; npm run build clean; check_i18n_locales.py and check_i18n_sorted.py clean; trunk fmt && trunk check clean.

Screenshots (if applicable)

The warning toast naming the game:

---

AI assistance: This change was written with Claude Code (Anthropic). I directed the design (transport, conflict ownership, toast-only surface) and reviewed every change; the code, tests, and translations were drafted by the agent.

[greptile-apps]

Greptile Summary

This PR surfaces save-sync conflicts from REST negotiation through Socket.IO and installs a global v2 toast consumer.

• Converts negotiation to async and emits one fail-open notification for each conflicting operation.
• Resolves toast labels from loaded ROM caches, with localized generic fallbacks.
• Adds backend and frontend tests plus translations across supported locales.
• The current frontend dedupe identity can suppress distinct conflicts from different devices, and the Redis notification remains on the response-critical path.

Confidence Score: 4/5

The PR should not merge until distinct conflicts from different devices can each reach the user; the Redis latency concern is additional non-blocking hardening.

Conflict state is tracked and emitted per device, but the frontend dedupes only by ROM and filename, so one device's event can suppress another device's only user-visible notification.

Files Needing Attention: frontend/src/v2/composables/useSyncConflictToast/index.ts, frontend/src/v2/composables/useSyncConflictToast/index.test.ts, backend/endpoints/sync.py

Important Files Changed

Filename	Overview
backend/endpoints/sync.py	Emits conflict events after session persistence, but awaits unbounded sequential Redis operations before returning negotiation results.
backend/tests/endpoints/test_sync.py	Covers conflict emission, silent no-op behavior, and raised emitter errors, but not slow notification infrastructure.
frontend/src/v2/composables/useSyncConflictToast/index.ts	Adds cache-backed warning toasts, but dedupes distinct device-specific conflicts under the same ROM/file key.
frontend/src/v2/composables/useSyncConflictToast/index.test.ts	Covers named, generic, and repeated-event behavior but omits two devices conflicting on the same save.
frontend/src/v2/layouts/AppLayout.vue	Installs the conflict subscriber using the established application-layout lifecycle pattern.
frontend/src/locales/en_US/rom.json	Adds representative named and generic save-conflict messages, mirrored across the other changed locale files.

Prompt To Fix All With AI

### Issue 1
frontend/src/v2/composables/useSyncConflictToast/index.ts:36-38
**Device conflicts share dedupe key**

Conflict detection is device-specific, but this page-session dedupe key omits `device_id`. If two devices conflict on the same ROM and file, the first event adds this key and the second device's distinct conflict is silently discarded. Because the toast is currently the user's only conflict surface, the second conflict receives no notification. Include the device identity in the key and cover this case with a test.

### Issue 2
backend/endpoints/sync.py:344-355
**Redis can delay negotiation**

The notification is described as fail-open, but every conflict is emitted sequentially through a newly created Redis manager and awaited before the negotiation response, without an explicit timeout. A slow or unavailable Redis connection can therefore delay negotiation once per conflict, even though exceptions are caught. Bound the notification attempt or move it outside the response-critical path so notification infrastructure cannot block emulator launch.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "feat(sync): surface save conflicts detec..." | Re-trigger Greptile}

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect event-loop safety, notification latency, and conflict-toast correctness.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds end-to-end visibility for save conflicts detected during REST sync negotiation, from backend Socket.IO events to frontend warning toasts.

Changes:

• Emits fail-open sync:conflict events during negotiation.
• Adds cached ROM-name resolution and per-session toast deduplication.
• Adds backend/frontend tests and translations across supported locales.

Review findings:

• backend/endpoints/sync.py — Critical (3 votes): Blocking database work now runs on the async event loop; retain threadpool execution or offload the blocking work.
• backend/endpoints/sync.py — Moderate (3 votes): Redis notification timeouts can repeat per conflict; reuse the manager or apply a bounded total timeout.
• frontend/src/v2/composables/useSyncConflictToast/index.ts — Moderate (1 vote): Empty names bypass the cached fs_name fallback.
• frontend/src/v2/composables/useSyncConflictToast/index.ts — Moderate (1 vote): Deduplication omits save slots and can suppress distinct or later conflicts.

File summaries

File	Summary
frontend/src/v2/layouts/AppLayout.vue	Installs global conflict-toast handling.
frontend/src/v2/composables/useSyncConflictToast/index.ts	Subscribes to conflict events, resolves cached names, and displays deduplicated warnings.
frontend/src/v2/composables/useSyncConflictToast/index.test.ts	Tests naming, fallback, and deduplication.
frontend/src/locales/zh_TW/rom.json	Adds Traditional Chinese conflict messages.
frontend/src/locales/zh_CN/rom.json	Adds Simplified Chinese conflict messages.
frontend/src/locales/tr_TR/rom.json	Adds Turkish conflict messages.
frontend/src/locales/ru_RU/rom.json	Adds Russian conflict messages.
frontend/src/locales/ro_RO/rom.json	Adds Romanian conflict messages.
frontend/src/locales/pt_BR/rom.json	Adds Brazilian Portuguese conflict messages.
frontend/src/locales/pl_PL/rom.json	Adds Polish conflict messages.
frontend/src/locales/ko_KR/rom.json	Adds Korean conflict messages.
frontend/src/locales/ja_JP/rom.json	Adds Japanese conflict messages.
frontend/src/locales/it_IT/rom.json	Adds Italian conflict messages.
frontend/src/locales/hu_HU/rom.json	Adds Hungarian conflict messages.
frontend/src/locales/fr_FR/rom.json	Adds French conflict messages.
frontend/src/locales/es_ES/rom.json	Adds Spanish conflict messages.
frontend/src/locales/en_US/rom.json	Adds source English conflict messages.
frontend/src/locales/en_GB/rom.json	Adds English conflict messages.
frontend/src/locales/de_DE/rom.json	Adds German conflict messages.
frontend/src/locales/cs_CZ/rom.json	Adds Czech conflict messages.
frontend/src/locales/bg_BG/rom.json	Adds Bulgarian conflict messages.
backend/tests/endpoints/test_sync.py	Tests conflict emission, no-op silence, and fail-open behavior.
backend/endpoints/sync.py	Emits conflict notifications during negotiation.

Review details

Suppressed comments (2)

frontend/src/v2/composables/useSyncConflictToast/index.ts:32

• name may be an empty string as well as null; with ?? this returns "", and the caller's name ? ... : generic branch then hides the loaded ROM's fs_name. Use a truthy fallback so cached games with blank metadata still get a useful name.

    return rom ? (rom.name ?? rom.fs_name) : null;

frontend/src/v2/composables/useSyncConflictToast/index.ts:37

• seen is keyed only by rom_id:file_name, but negotiation distinguishes saves by (rom_id, slot) and the conflict event omits slot. Two conflicting slots with the same filename, or a later conflict after the original is resolved, are therefore suppressed for the rest of the page. Include a stable conflict/version identity in the event and use it here.

    const key = `${payload.rom_id}:${payload.file_name}`;
    if (seen.has(key)) return;

• Files reviewed: 23/23 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sdornan]

Triaged all five findings. Two fixed in ee0cd1d1b, three declined. Reasoning below, with the evidence, so the declines are on the record rather than left hanging.

Fixed: Redis can delay negotiation

backend/endpoints/sync.py (Copilot moderate x3, Greptile P2)

Valid, and the sharper half is the scaling rather than the raw latency. The emits ran sequentially, one awaited call per conflict, and emit_sync_conflict dials a fresh AsyncRedisManager on every call, so the pre-response wait grew without bound with the conflict count.

The emits now go out concurrently under a single asyncio.wait_for (CONFLICT_NOTIFY_TIMEOUT_S = 2.0), which makes the added wait constant rather than proportional. return_exceptions=True plus a zip(..., strict=True) inspection loop preserves the per-file warning, and the broad except preserves fail-open: an asyncio.CancelledError is a BaseException, so a genuine cancellation still propagates instead of being swallowed.

test_slow_notification_does_not_stall_the_negotiation asserts wall-clock. I checked it actually discriminates: with the bound removed it fails at 30.05s, with the bound in place it passes at 0.55s.

Fixed: empty name bypasses the fs_name fallback

Frontend (Copilot suppressed, moderate)

Valid. ?? only caught null, so a cached ROM with name: "" rendered "Save conflict detected for ". Now a truthy fallback, with a test.

Declined: blocking database work on the event loop

backend/endpoints/sync.py:124 (Copilot critical x3)

The mechanism is real and I traced it: protected_route branches on is_async_callable (backend/decorators/auth.py:107), so an async def handler does run its synchronous SQLAlchemy calls on the event-loop thread.

What I do not think holds is that this change introduces the hazard. The route is async def because it has to await the socket emit, and the alternative suggested here (keep it sync and bridge only the notification) means awaiting the emit from a threadpool thread, which needs an event loop of its own. The surrounding code already has this shape: add_state (backend/endpoints/states.py:54) and add_save (backend/endpoints/saves.py:194) are both async def and call synchronous db_*_handler functions, and 99 async handlers across the 21 modules in backend/endpoints/ do the same.

Moving the handler layer off the loop is worth doing, but as a pass over the endpoint and handler layers rather than inside a PR that adds a notification. Happy to file an issue to track it if that would be useful.

Declined: dedupe key omits device_id

Frontend (Greptile P1)

The toast names a game, not a device: "Save conflict detected for {game}". Two devices conflicting on the same (rom_id, file_name) would render two byte-identical toasts, so adding device_id to the key would add a duplicate rather than recover information that was being lost. The conflict is a property of the save, and the toast is informational by design, with resolution deliberately left as later work, so nothing actionable is missed by informing once per game.

Declined: dedupe omits save slots

Frontend (Copilot suppressed, moderate)

The first half does not apply. Slotted uploads are datetime-tagged server-side (endpoints/saves.py:239), so two slots for one game have distinct file_names and the existing key already separates them.

The second half is accurate: a conflict that is re-reported after it has been resolved stays suppressed for the rest of the page session. That is intended. The server re-emits the event on every negotiation for as long as the conflict is unresolved, so re-toasting on each poll would be noise rather than signal.

[Copilot]

🔵 Needs a closer look

The negotiation path can block the event loop and create unbounded concurrent Redis managers.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

backend/endpoints/sync.py:124

• Changing this route to async def moves all of its synchronous SQLAlchemy handler calls onto the server event-loop thread. A negotiation performs several queries and commits before its first await, so a slow database or concurrent negotiations can block unrelated async requests and Socket.IO work. Keep the database-heavy negotiation on FastAPI's worker thread, or extract/offload the synchronous phase before awaiting notifications.

backend/endpoints/sync.py:356

• This launches one task per conflict, and each emit_sync_conflict constructs its own AsyncRedisManager. Since payload.saves has no length bound, a large conflict set can create an unbounded burst of Redis managers/connections even though the timeout limits duration. Reuse one manager and emit with bounded concurrency, or add a batch emitter that publishes all conflict payloads through one manager.

                    server_updated_at=save.updated_at,
                    server_content_hash=save.content_hash,
                )
            )

• Files reviewed: 23/23 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Conflict deduplication can suppress distinct push-pull conflicts that share a filename.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

frontend/src/v2/composables/useSyncConflictToast/index.ts:37

• This key is not unique for the existing push-pull producer: backend/tasks/sync_push_pull_task.py:156 emits every conflict with rom_id=0, while its file_name is only a basename and can repeat across configured platform directories. Two distinct conflicts such as save.dat on two platforms therefore produce the same key and the second warning is suppressed. Include a stable conflict identifier, such as platform plus save or slot identity, in the socket payload and key.
  backend/endpoints/sync.py:158
• When the deadline expires, TimeoutError has an empty message, so this logs Failed to emit ... events: without explaining that notification delivery timed out. Handle the timeout explicitly and include the configured deadline; retaining the exception type also keeps the generic failure log informative.

• Files reviewed: 23/23 changed files
• Comments generated: 0 new
• Review effort level: Balanced