Conversation
…-sync - Fix broken path construction in FSSyncHandler: build_* methods now return relative paths; sync_watcher uses paths relative to sync base instead of CWD (was completely non-functional in production) - Fix SSH connection leak in push-pull task: conn.close() now in finally - Add log.warning for disabled SSH host key verification - Fix race condition in session operation counter: use atomic SQL increment instead of read-then-write - Extract _increment_session_counter helper, add exc_info to warnings - Replace legacy session.query() with select() in sync_sessions_handler - Fix orphaned session: trigger_push_pull now passes session_id to job - Fix wasteful SSH download when no matched_save exists - Fix BaseModel import collision in sync.py (pydantic -> project base) - Fix ORM mutation in UserSchema.from_orm_with_request: set field on schema instance instead of mutating live ORM object - Mask ssh_password and ssh_key_path in DeviceSchema API response - Fix migration PostgreSQL compatibility: condition ON UPDATE clause on MySQL, drop enum in downgrade - Rename copy-paste artifact rom_user_status_enum
- Restore NoResultFound behavior on update_session, complete_session, fail_session when row is missing (scalar returns None, old .one() raised -- silent None is a semantic regression) - Remove redundant get_session call from _increment_session_counter; the atomic SQL increment is already a no-op on missing rows - Log warning when passed session_id is not found in _sync_device instead of silently creating an orphan session
…king, and auth utils - test_sync_sessions_handler: increment_operations_completed (atomic counter, no-op on missing), NoResultFound on update/complete/fail with nonexistent session - test_sync_watcher: _extract_device_and_platform path parsing (valid, non-incoming, too few parts, nested, outside base), _ensure_conflicts_dir creation and idempotency, process_sync_changes empty/disabled - test_sync (endpoints): negotiate with untracked saves (no_op), server saves not mentioned by client (download), deleted-by-client detection (skip), complete on FAILED/CANCELLED session (400), trigger_push_pull passes session_id in enqueue kwargs - test_device (endpoints): sync_config SSH credential masking (ssh_password/ssh_key_path -> "********"), null config passthrough, config without sensitive fields - test_utils_auth: _get_device_name UA parsing (browser+OS, browser only, OS only, neither), create_or_find_web_device (creates new, returns existing on fingerprint match, updates last_seen)
uv sync installs sqlalchemy[mariadb-connector] regardless of which database is being tested, so libmariadb-dev must be present on all runners. The postgres migration job and pytest postgresql matrix were missing this step.
The syncsessionstatus enum creation used checkfirst=False which fails with DuplicateObject if the type already exists (e.g., test reruns or partial migrations). Matches the pattern used in the downgrade.
… PostgreSQL The sa.Enum() inline in create_table tried to CREATE TYPE again after the explicit ENUM.create() call. Use the pre-created enum variable with create_type=False for PostgreSQL to avoid DuplicateObject error. Verified locally: upgrade, downgrade, re-upgrade all clean on PostgreSQL.
fix: address bugs and add test coverage for save-sync
Contributor
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
Contributor
Contributor
This was referenced Mar 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a multi-device save synchronization subsystem (API negotiation, file-transfer watcher, and SSH/SFTP push-pull), auto-registers a “web” device on login, and modernizes backend response models for Pydantic v2 (plus assorted deprecation/status-code updates and test refactors).
Changes:
- Introduces device-aware save syncing: sync sessions, negotiation endpoint, push/pull background task, and file-transfer folder watcher.
- Auto-creates (or reuses) a fingerprinted web device on login/OIDC and surfaces
current_device_idto the frontend. - Updates backend response modeling to Pydantic v2 patterns (UTCDatetime + ConfigDict), plus various deprecation/test cleanups.
Reviewed changes
Copilot reviewed 73 out of 88 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds asyncssh + ua-parser dependencies to the lockfile. |
| pyproject.toml | Declares asyncssh and ua-parser runtime dependencies. |
| frontend/src/views/Player/EmulatorJS/utils.ts | Passes deviceId through EmulatorJS save upload/update calls. |
| frontend/src/views/Player/EmulatorJS/Player.vue | Reads current_device_id and forwards it into save callbacks. |
| frontend/src/utils/tasks.ts | Adds UI mapping for new sync task type. |
| frontend/src/services/api/save.ts | Adds device_id query param support for save upload/update APIs. |
| frontend/src/generated/models/UserSchema.ts | Adds current_device_id to generated user schema. |
| frontend/src/generated/models/TaskType.ts | Adds 'sync' to generated task-type union. |
| frontend/src/generated/models/SyncSessionSchema.ts | New generated model for sync sessions. |
| frontend/src/generated/models/SyncOperationSchema.ts | New generated model for sync operations. |
| frontend/src/generated/models/SyncNegotiateResponse.ts | New generated model for negotiate response. |
| frontend/src/generated/models/SyncNegotiatePayload.ts | New generated model for negotiate payload. |
| frontend/src/generated/models/SyncCompletePayload.ts | New generated model for session completion payload. |
| frontend/src/generated/models/DeviceUpdatePayload.ts | Adds sync fields to device update payload model. |
| frontend/src/generated/models/DeviceSchema.ts | Adds sync_config to device schema model. |
| frontend/src/generated/models/DeviceCreatePayload.ts | Adds sync fields to device create payload model. |
| frontend/src/generated/models/ClientSaveState.ts | New generated model for client save state. |
| frontend/src/generated/index.ts | Exports new sync-related generated models. |
| entrypoint.sh | Optionally starts the sync folder watcher in dev. |
| docker/init_scripts/init | Adds ENABLE_SYNC_FOLDER_WATCHER wiring + watchdog support. |
| backend/utils/client_tokens.py | Updates deprecated HTTP 422 constant usage. |
| backend/utils/auth.py | Implements auto-create/find “web” device based on UA + IP fingerprint. |
| backend/tests/test_utils_auth.py | Adds unit tests for device name parsing and web-device upsert behavior. |
| backend/tests/test_sync_watcher.py | Adds tests for watcher path parsing + early-return behavior. |
| backend/tests/tasks/test_sync_push_pull.py | Adds tests for push/pull task initialization + run() behavior. |
| backend/tests/tasks/test_scan_library.py | Fixes unawaited coroutine mocking in scan tests. |
| backend/tests/tasks/test_prevent_requeue.py | Renames test helper class to DummyTask. |
| backend/tests/handler/sync/test_comparison.py | Adds tests for save-state comparison algorithm. |
| backend/tests/handler/sync/init.py | Adds handler sync test package marker. |
| backend/tests/handler/filesystem/test_sync_handler.py | Adds tests for filesystem sync handler utilities. |
| backend/tests/handler/database/test_sync_sessions_handler.py | Adds tests for sync session DB handler behavior. |
| backend/tests/handler/database/test_device_save_sync_handler.py | Adds tests for device-save sync junction handler behavior. |
| backend/tests/handler/auth/test_session_middleware.py | Updates cookie usage to avoid Starlette deprecation. |
| backend/tests/endpoints/test_tasks.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/test_sync.py | Adds endpoint tests for negotiate/sessions/push-pull trigger + isolation. |
| backend/tests/endpoints/test_saves.py | Updates tests for new HTTP 422 constant + fixture consolidation. |
| backend/tests/endpoints/test_raw.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/test_platform.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/test_oauth.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/test_identity.py | Updates cookie usage + fixture consolidation; removes sync_cache usage. |
| backend/tests/endpoints/test_heartbeat.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/test_device.py | Adds tests around sync_config masking in device responses. |
| backend/tests/endpoints/test_config.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/test_client_tokens.py | Consolidates client fixture usage; updates HTTP 422 constant. |
| backend/tests/endpoints/test_assets.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/roms/test_upload.py | Updates deprecated HTTP 413 constant usage; fixture cleanup. |
| backend/tests/endpoints/roms/test_rom.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/feeds.py | Consolidates client fixture usage via endpoints conftest. |
| backend/tests/endpoints/conftest.py | Centralizes TestClient + redis cache flush + access token fixtures. |
| backend/tests/conftest.py | Ensures SyncSession rows are cleared between tests. |
| backend/tasks/tasks.py | Adds SYNC to TaskType enum. |
| backend/tasks/sync_push_pull_task.py | Implements async SSH/SFTP push-pull synchronization task. |
| backend/tasks/sync_folder_task.py | Adds sync folder scan task (manual fallback) for file-transfer mode. |
| backend/sync_watcher.py | Adds watchfiles-driven incoming sync folder processor + socket emits. |
| backend/startup.py | Starts scheduled push-pull sync task when enabled. |
| backend/models/sync_session.py | Adds SyncSession SQLAlchemy model + status enum. |
| backend/models/device.py | Adds KNOWN_DEVICES registry + sync_config JSON column. |
| backend/main.py | Registers sync router + sync socket module. |
| backend/handler/sync/ssh_handler.py | Adds AsyncSSH-based SSH/SFTP helper for push/pull mode. |
| backend/handler/sync/comparison.py | Adds deterministic save-state comparison logic. |
| backend/handler/sync/init.py | Adds sync handler package marker. |
| backend/handler/metadata/launchbox_handler/local_source.py | Fixes XML root truthiness check (is not None). |
| backend/handler/filesystem/sync_handler.py | Adds filesystem helper for sync folder paths + hashing + cleanup. |
| backend/handler/filesystem/init.py | Exposes fs_sync_handler singleton. |
| backend/handler/database/sync_sessions_handler.py | Adds DB handler for sync sessions lifecycle. |
| backend/handler/database/devices_handler.py | Adds get_device_by_id and get_all_devices_by_sync_mode; expands fingerprint lookup. |
| backend/handler/database/init.py | Exposes db_sync_session_handler. |
| backend/endpoints/user.py | Returns user via UserSchema.from_orm_with_request() (adds device context). |
| backend/endpoints/sync.py | Adds sync negotiate/session endpoints + push-pull trigger endpoint. |
| backend/endpoints/sockets/sync.py | Adds websocket emit helpers for sync progress/events via Redis manager. |
| backend/endpoints/saves.py | Adds session counter increments + device-aware sync upserts on save update. |
| backend/endpoints/roms/upload.py | Updates deprecated HTTP 413 constant usage. |
| backend/endpoints/responses/sync.py | Adds response schemas for sync endpoints (Pydantic v2 style). |
| backend/endpoints/responses/rom.py | Migrates response models to UTCDatetime + ConfigDict. |
| backend/endpoints/responses/platform.py | Migrates response models to UTCDatetime + ConfigDict. |
| backend/endpoints/responses/identity.py | Adds current_device_id + request-aware user schema constructor. |
| backend/endpoints/responses/firmware.py | Migrates response models to UTCDatetime + ConfigDict. |
| backend/endpoints/responses/device.py | Adds sync_config masking serializer + UTCDatetime conversions. |
| backend/endpoints/responses/collection.py | Migrates response models to UTCDatetime + ConfigDict. |
| backend/endpoints/responses/client_token.py | Migrates response models to UTCDatetime + ConfigDict. |
| backend/endpoints/responses/base.py | Introduces UTCDatetime serializer and removes deprecated json_encoders config. |
| backend/endpoints/responses/assets.py | Migrates base asset timestamps to UTCDatetime + ConfigDict. |
| backend/endpoints/device.py | Adds sync fields to device create/update payloads + auto-folder creation for file_transfer. |
| backend/endpoints/auth.py | Auto-registers web device on login/OIDC and stores device_id in session. |
| backend/config/init.py | Adds sync-related configuration knobs (paths, toggles, cron). |
| backend/alembic/versions/0073_sync_sessions.py | Adds sync_sessions table + devices.sync_config column migration. |
| .github/workflows/pytest.yml | Adjusts MariaDB connector install step logic. |
| .github/workflows/migrations.yml | Ensures MariaDB connectors are installed for migrations CI job. |
Files not reviewed (12)
- frontend/src/generated/index.ts: Language not supported
- frontend/src/generated/models/ClientSaveState.ts: Language not supported
- frontend/src/generated/models/DeviceCreatePayload.ts: Language not supported
- frontend/src/generated/models/DeviceSchema.ts: Language not supported
- frontend/src/generated/models/DeviceUpdatePayload.ts: Language not supported
- frontend/src/generated/models/SyncCompletePayload.ts: Language not supported
- frontend/src/generated/models/SyncNegotiatePayload.ts: Language not supported
- frontend/src/generated/models/SyncNegotiateResponse.ts: Language not supported
- frontend/src/generated/models/SyncOperationSchema.ts: Language not supported
- frontend/src/generated/models/SyncSessionSchema.ts: Language not supported
- frontend/src/generated/models/TaskType.ts: Language not supported
- frontend/src/generated/models/UserSchema.ts: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+90
to
+115
| connect_kwargs: dict[str, Any] = { | ||
| "host": host, | ||
| "port": port, | ||
| "username": username, | ||
| "known_hosts": None, # Accept all host keys (TODO: make configurable) | ||
| } | ||
|
|
||
| # Resolve key path (explicit or convention-based) | ||
| key_path = self._resolve_key_path(device_id or "", sync_config) | ||
| if key_path: | ||
| connect_kwargs["client_keys"] = [key_path] | ||
| elif sync_config.get("ssh_password"): | ||
| connect_kwargs["password"] = sync_config["ssh_password"] | ||
| else: | ||
| raise ValueError( | ||
| f"No SSH authentication method available for {host}. " | ||
| f"Mount a key at {self.keys_path}/{{device_id}}.pem or " | ||
| "provide ssh_key_path/ssh_password in sync_config." | ||
| ) | ||
|
|
||
| log.warning( | ||
| f"SSH host key verification disabled for {host} -- " | ||
| "connection is vulnerable to MITM attacks" | ||
| ) | ||
| log.info(f"Connecting to {username}@{host}:{port}") | ||
| return await asyncssh.connect(**connect_kwargs) |
There was a problem hiding this comment.
known_hosts=None disables SSH host key verification for all push/pull sync connections, making the sync channel vulnerable to MITM. Consider making host key verification configurable (e.g., optional known_hosts file/fingerprint pinning) and defaulting to verification enabled in production deployments.
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
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.
This PR introduces a multi-device save synchronization system, automatic web device registration on login, and a batch of Pydantic V2 / deprecation cleanups across the backend.
Device Registration
Auto-create web devices on login: When a user authenticates via the login or OIDC endpoint, a web device is automatically created (or found) in the backend — no frontend registration flow required.
Fingerprinting: Web devices are identified by client IP + parsed browser/OS family. UA parsing uses
ua-parserto extract version-stable identifiers (e.g."Chrome on Mac OS X") so browser updates don't create duplicate devices.KNOWN_DEVICESregistry (models/device.py): A frozenDeviceTypedataclass defines each supported client with itsplatform,clientidentifier, and defaultsync_mode. The current registry:webAPIgroutAPIargosy-launcherPUSH_PULLutils/auth.py:create_or_find_web_device()looks up the"web"device type, fingerprints the request, and upserts the device — updatinglast_seenif it already exists.Save Synchronization
The save sync system ties saves to specific devices using a
DeviceSaveSyncjunction table, enabling multi-device conflict detection and resolution across three distinct sync modes.Data Model
DeviceSaveSynclinksDevicetoSavewith:last_synced_at— when this device last synced the save (used as the baseline for conflict detection)is_untracked— opt-out flag so a device can intentionally ignore a saveSave responses are enriched with per-device context:
is_current(whether the device has the latest version),last_synced_at, andis_untracked.Conflict Detection
POST /sync/negotiateaccepts a list of the device's local saves (withcontent_hash,updated_at,file_size_bytes) and runs each throughcompare_save_state(), which produces one of four actions:upload— client has a newer save, server should accept itdownload— server has a newer save, client should pull itconflict— both sides changed sincelast_synced_at, manual resolution requiredno_op— already in syncThe session is recorded in
SyncSessionand the client is responsible for executing the returned operations before callingPOST /sync/sessions/{id}/complete.Sync Modes
API(Manual) — Client-driven: the device negotiates, executes uploads/downloads, and marks the session complete. This is how the web client and muOS (grout) operate.FILE_TRANSFER— A background file watcher monitors{SYNC_BASE_PATH}/{device_id}/incoming/{platform_slug}/. When a file appears, it's compared against the server copy and routed tooutgoing/(server is newer), accepted as an upload (client is newer), or moved toconflicts/with a socket notification.PUSH_PULL— A periodic background task SSH/SFTPs into the device using credentials indevice.sync_config, lists remote save directories, and performs bidirectional sync: pulling newer remote saves to the server and pushing newer server saves back to the device. Progress is streamed via WebSocket events (sync:started,sync:progress,sync:completed,sync:conflict,sync:error), all scoped to the user's socket room.EmulatorJS Integration
The
EJS_onSaveSavecallback inPlayer.vueis wired to the save sync system: when the user saves from within the emulator,saveSave()is called with the registered web device ID. This either uploads a new save (POST /saves) or updates an existing one (PUT /saves/{id}), both passingdevice_idso aDeviceSaveSyncrecord is created or updated immediately. A toast confirms the result. States follow the same pattern viaEJS_onSaveState.Pydantic V2 / Deprecation Fixes
UTCDatetimeannotated type (Annotated[datetime, PlainSerializer(...)]) replaces the deprecatedjson_encodersdict across all response models — idiomatic Pydantic V2 serialization without changing field types.model_config = ConfigDict(...)replaces innerclass Configin 9 response model files (device,sync,client_token,platform,assets,collection,identity,firmware,rom).HTTP_413_REQUEST_ENTITY_TOO_LARGE→HTTP_413_CONTENT_TOO_LARGE,HTTP_422_UNPROCESSABLE_ENTITY→HTTP_422_UNPROCESSABLE_CONTENT.TestTask→DummyTask(naming convention), per-requestcookies=→Cookieheader (Starlette deprecation),if root:→if root is not None:(element truth value), unawaited coroutine in scan library tests fixed withside_effect=AsyncMock(...).