bridget: use standard settings path and migrate userdata

[nicoschmdt]

updated version of #3524
fix: #3523

[github-actions]

Automated PR Review

0. Summary

• Verdict: DOES NOT SAIL 🪨
• Critical items to address: 1.1

Moves bridget's settings out of USERDATA/settings/bridget/bridget/ and into the default appdirs.user_config_dir("bridget") path used by every other service, and adds an on_settings_created hook that migrates the previous settings-*.json files from the old userdata location the first time the service starts on the new path.

1. Correctness & Implementation Bugs

• 1.1 [major] core/services/bridget/settings.py — SettingsV2.on_settings_created only migrates from OLD_SETTINGS_DIR/settings-2.json; if a user has only settings-1.json at the old location (i.e. they configured bridges under bridget V1 and never modified them after the V1→V2 code shipped), that data is not migrated and is silently lost on first boot after this PR ships. Trace: bridget.py init on startup only calls add_bridge for specs already loaded, and add_bridge skips save() when the spec is already in specsv2, so V1-only users never get a settings-2.json written to the old dir. Recommend falling back to settings-1.json when the V2 file is absent — either by extending migrate_from_old_settings to run V1→V2 conversion (e.g. use PydanticManager.load_from_file(SettingsV2, old_v1_path) which does the migration internally), or by checking the V1 file explicitly inside SettingsV2.on_settings_created.
• 1.2 [minor] migrate_from_old_settings swallows every exception with except Exception: pass and only annotates it with # If migration fails, continue with empty settings. A corrupt or partially-written source file is exactly the case where users need to be told what happened; at minimum, log the failure with loguru.logger.warning(...) so the incident is discoverable in service logs.

6. Code Quality & Style

• 6.1 [minor] core/services/bridget/settings.py — SettingsV1.on_settings_created is unreachable: only SettingsV2 is ever passed to PydanticManager (see bridget.py:46), and SettingsV2 overrides on_settings_created, so Python's MRO never routes to SettingsV1's version. Either delete it, or restructure so that SettingsV2 inherits the callback the way cable_guy/api/settings.py does (put the whole migration on SettingsV1, don't override in V2, and select the source file inside the hook). Right now it reads like a fallback that will kick in but never does.
• 6.2 [minor] SettingsV2.on_settings_created guards with if (file_path.parent / "settings-2.json").exists(): return. Per PydanticSettings.save, on_settings_created is only invoked when file_path itself does not exist, and file_path for a SettingsV2 instance is precisely settings-2.json in that same directory — so this branch is dead. The analogous guard in SettingsV1.on_settings_created has the same problem (it checks settings-1.json, which is the file being created). The cable_guy pattern this is modelled on works because that guard checks a different file from the one being created; here the file names collide.
• 6.3 [nit] OLD_SETTINGS_DIR is a module-level constant used only inside migrate_from_old_settings, and the function already accepts an old_dir override. Consider making OLD_SETTINGS_DIR a default parameter value or a private constant closer to the function; as-is the module top is a bit noisier than needed.
• 6.4 [nit] settings.py imports pathlib as a module while every other module in bridget uses from pathlib import Path. Minor consistency nit.

7. Tests

• 7.1 [minor] No pytest added for migrate_from_old_settings or the two on_settings_created overrides, even though the entire PR is a data-migration change. Given the correctness concern in 1.1, a small test that seeds a temp directory with a V1 file and a V2 file (separately) and asserts the resulting SettingsV2 contains the expected specs would catch the regression and document the intended behaviour.

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.