Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR introduces a new Tauri settings plugin ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
apps/desktop/src/components/settings/ai/llm/health.tsx (1)
106-132: Prefer eliminating theresult as ...type assertionYou can usually get the same union typing by returning
as constobjects from each branch (or by annotatingresult), avoiding the cast at Line 131.apps/desktop/src/components/settings/ai/shared/index.tsx (1)
15-68: Store swap looks consistent; avoid the hard cast if the hook can be typed.
The migration tosettings.UI.*+settings.STORE_IDis consistent. IfuseSetPartialRowCallbackcan be generically typed, prefer that overas (row: Partial<AIProvider>) => voidto avoid masking signature mismatches.plugins/settings/package.json (1)
1-10: Consider addingexportsfield for clarity in workspace consumption.While the current
"main": "./js/index.ts"works in this monorepo with Vite's native TypeScript support, adding anexportsfield would clarify module resolution:{ "name": "@hypr/plugin-settings", "private": true, "main": "./js/index.ts", + "exports": { + ".": "./js/index.ts" + }, "scripts": { "codegen": "cargo test -p tauri-plugin-settings" }, "dependencies": { "@tauri-apps/api": "^2.9.1" } }The
@tauri-apps/apiversion^2.9.1is correctly aligned with the rest of the codebase.plugins/settings/src/lib.rs (1)
30-34: Consider graceful error handling fordata_dir()call.The
unwrap()on line 31 will panic ifdata_dir()returnsNone. While this is unlikely in normal operation, consider propagating the error for more graceful handling:.setup(|app, _api| { - let base = app.path().data_dir().unwrap().join("hyprnote"); + let base = app.path().data_dir() + .ok_or_else(|| "Failed to resolve data directory".to_string())? + .join("hyprnote"); let state = SettingsState::new(base); assert!(app.manage(state)); Ok(()) })plugins/settings/src/commands.rs (1)
1-1: Unused import.The
tauri::Managertrait is not used in this file. Thestate()method comes fromtauri::AppHandledirectly.-use tauri::Manager;apps/desktop/src/store/tinybase/jsonPersister.test.ts (1)
133-135: Consider using a type guard or explicit interface.The inline type assertion is functional but verbose. If this pattern repeats, consider defining a shared type for partial settings results.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (24)
Cargo.lockis excluded by!**/*.lockplugins/deeplink2/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/deeplink2/permissions/autogenerated/commands/ping.tomlis excluded by!plugins/**/permissions/**plugins/deeplink2/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/deeplink2/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/deeplink2/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**plugins/importer/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/importer/permissions/autogenerated/commands/ping.tomlis excluded by!plugins/**/permissions/**plugins/importer/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/importer/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/importer/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**plugins/settings/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/settings/permissions/autogenerated/commands/load.tomlis excluded by!plugins/**/permissions/**plugins/settings/permissions/autogenerated/commands/path.tomlis excluded by!plugins/**/permissions/**plugins/settings/permissions/autogenerated/commands/save.tomlis excluded by!plugins/**/permissions/**plugins/settings/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/settings/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/settings/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**plugins/webhook/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/webhook/permissions/autogenerated/commands/ping.tomlis excluded by!plugins/**/permissions/**plugins/webhook/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/webhook/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/webhook/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (37)
Cargo.toml(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/capabilities/default.json(2 hunks)apps/desktop/src-tauri/src/lib.rs(1 hunks)apps/desktop/src/components/main/sidebar/banner/index.tsx(2 hunks)apps/desktop/src/components/settings/ai/llm/health.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/shared/index.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)apps/desktop/src/components/settings/general/index.tsx(3 hunks)apps/desktop/src/components/settings/general/notification.tsx(2 hunks)apps/desktop/src/config/use-config.ts(4 hunks)apps/desktop/src/hooks/useCurrentModelModalitySupport.ts(1 hunks)apps/desktop/src/hooks/useLLMConnection.ts(2 hunks)apps/desktop/src/hooks/useSTTConnection.ts(1 hunks)apps/desktop/src/main.tsx(2 hunks)apps/desktop/src/store/tinybase/jsonPersister.test.ts(2 hunks)apps/desktop/src/store/tinybase/jsonPersister.ts(3 hunks)apps/desktop/src/store/tinybase/keys.ts(0 hunks)apps/desktop/src/store/tinybase/settings.ts(1 hunks)apps/web/content/docs/faq/7.settings.mdx(1 hunks)plugins/importer/build.rs(1 hunks)plugins/settings/.gitignore(1 hunks)plugins/settings/Cargo.toml(1 hunks)plugins/settings/build.rs(1 hunks)plugins/settings/js/index.ts(1 hunks)plugins/settings/package.json(1 hunks)plugins/settings/src/commands.rs(1 hunks)plugins/settings/src/error.rs(1 hunks)plugins/settings/src/ext.rs(1 hunks)plugins/settings/src/lib.rs(1 hunks)plugins/settings/src/state.rs(1 hunks)plugins/settings/tsconfig.json(1 hunks)plugins/webhook/build.rs(1 hunks)plugins/webhook/openapi.gen.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/store/tinybase/keys.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/settings/general/notification.tsxplugins/settings/js/index.tsapps/desktop/src/components/settings/ai/stt/configure.tsxapps/desktop/src/config/use-config.tsapps/desktop/src/components/settings/ai/stt/select.tsxapps/desktop/src/components/main/sidebar/banner/index.tsxapps/desktop/src/components/settings/ai/llm/select.tsxapps/desktop/src/hooks/useCurrentModelModalitySupport.tsapps/desktop/src/store/tinybase/settings.tsapps/desktop/src/components/settings/ai/shared/index.tsxapps/desktop/src/components/settings/general/index.tsxapps/desktop/src/store/tinybase/jsonPersister.test.tsapps/desktop/src/main.tsxapps/desktop/src/hooks/useSTTConnection.tsapps/desktop/src/hooks/useLLMConnection.tsapps/desktop/src/components/settings/ai/llm/health.tsxapps/desktop/src/store/tinybase/jsonPersister.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
plugins/settings/js/index.tsapps/desktop/src/config/use-config.tsapps/desktop/src/hooks/useCurrentModelModalitySupport.tsapps/desktop/src/store/tinybase/settings.tsapps/desktop/src/store/tinybase/jsonPersister.test.tsapps/desktop/src/hooks/useSTTConnection.tsapps/desktop/src/hooks/useLLMConnection.tsapps/desktop/src/store/tinybase/jsonPersister.ts
plugins/*/src/lib.rs
📄 CodeRabbit inference engine (plugins/AGENTS.md)
After updating commands in
plugins/<NAME>/src/lib.rs, runcodegen, updateplugins/<NAME>/permissions/default.toml, andapps/desktop/src-tauri/capabilities/default.json
Files:
plugins/settings/src/lib.rs
🧠 Learnings (7)
📚 Learning: 2025-11-24T16:32:01.459Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:32:01.459Z
Learning: Applies to **/*.ts : Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Applied to files:
plugins/settings/tsconfig.json
📚 Learning: 2025-11-24T16:32:01.459Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:32:01.459Z
Learning: Applies to **/*.config.{ts,json} : Agent configuration should be centralized and externalized from implementation logic
Applied to files:
plugins/settings/tsconfig.json
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: After some amount of TypeScript changes, run `pnpm -r typecheck`.
Applied to files:
plugins/settings/tsconfig.json
📚 Learning: 2025-11-27T11:40:22.782Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: plugins/AGENTS.md:0-0
Timestamp: 2025-11-27T11:40:22.782Z
Learning: Applies to plugins/*/src/lib.rs : After updating commands in `plugins/<NAME>/src/lib.rs`, run `codegen`, update `plugins/<NAME>/permissions/default.toml`, and `apps/desktop/src-tauri/capabilities/default.json`
Applied to files:
plugins/settings/src/ext.rsplugins/settings/src/lib.rsplugins/importer/build.rsCargo.tomlapps/desktop/src-tauri/Cargo.tomlplugins/settings/Cargo.tomlplugins/settings/package.jsonplugins/settings/build.rsapps/desktop/src-tauri/src/lib.rsplugins/webhook/build.rsplugins/settings/src/commands.rs
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`), similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/main/sidebar/banner/index.tsx
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`). It is similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/main/sidebar/banner/index.tsx
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx,js,jsx,rs,json,toml,yaml,yml} : Format using `dprint fmt` from the root. Do not use `cargo fmt`.
Applied to files:
plugins/settings/.gitignore
🧬 Code graph analysis (10)
plugins/settings/src/state.rs (1)
plugins/settings/src/commands.rs (3)
path(5-8)load(12-17)save(21-27)
plugins/settings/src/lib.rs (3)
apps/desktop/src-tauri/src/lib.rs (3)
make_specta_builder(210-218)make_specta_builder(228-228)export_types(225-239)plugins/settings/src/state.rs (4)
new(10-16)path(18-20)load(22-30)save(32-54)plugins/settings/src/commands.rs (6)
path(5-8)load(12-17)save(21-27)app(6-6)app(15-15)app(25-25)
apps/desktop/src/hooks/useCurrentModelModalitySupport.ts (2)
apps/desktop/src/components/settings/ai/shared/list-common.ts (1)
InputModality(14-14)apps/desktop/src/auth.tsx (1)
useAuth(299-307)
apps/desktop/src/store/tinybase/settings.ts (3)
apps/desktop/src/store/tinybase/jsonPersister.ts (2)
migrateKeysJsonToSettings(139-183)createSettingsPersister(185-212)plugins/windows/js/index.ts (1)
getCurrentWebviewWindowLabel(9-12)apps/desktop/src/store/tinybase/save.ts (1)
registerSaveHandler(5-10)
apps/desktop/src/components/settings/ai/shared/index.tsx (1)
packages/store/src/schema-internal.ts (1)
AIProvider(43-43)
apps/desktop/src/store/tinybase/jsonPersister.test.ts (1)
apps/desktop/src/store/tinybase/jsonPersister.ts (2)
settingsToContent(111-117)storeToSettings(119-137)
plugins/settings/build.rs (4)
apps/desktop/src-tauri/src/lib.rs (1)
main(13-208)plugins/importer/build.rs (1)
main(3-5)plugins/webhook/build.rs (1)
main(3-5)plugins/settings/src/state.rs (1)
new(10-16)
apps/desktop/src-tauri/src/lib.rs (1)
plugins/settings/src/lib.rs (1)
init(25-37)
plugins/settings/src/commands.rs (2)
plugins/settings/src/state.rs (3)
path(18-20)load(22-30)save(32-54)owhisper/owhisper-config/src/lib.rs (1)
settings(73-73)
apps/desktop/src/store/tinybase/jsonPersister.ts (1)
apps/desktop/src/store/tinybase/settings.ts (1)
SETTINGS_MAPPING(23-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (macos, depot-macos-14)
- GitHub Check: ci
- GitHub Check: fmt
🔇 Additional comments (35)
apps/desktop/src-tauri/capabilities/default.json (1)
72-77: LGTM!Both changes follow the existing capability patterns. The
fs:allow-removepermission is appropriately scoped to$DATA/hyprnote/**/*, consistent with other file system operations, and thesettings:defaultpermission correctly enables the new settings plugin for the application.Also applies to: 118-118
plugins/webhook/build.rs (1)
1-4: Add "todo" to the COMMANDS array in build.rs.The webhook plugin defines a
#[tauri::command]functioncommands::todo()insrc/commands.rsbut theCOMMANDSarray inbuild.rsis empty. This will prevent the command from being properly registered. Updatebuild.rsline 1 to include the command name:const COMMANDS: &[&str] = &["todo"];⛔ Skipped due to learnings
Learnt from: CR Repo: fastrepl/hyprnote PR: 0 File: plugins/AGENTS.md:0-0 Timestamp: 2025-11-27T11:40:22.782Z Learning: Applies to plugins/*/src/lib.rs : After updating commands in `plugins/<NAME>/src/lib.rs`, run `codegen`, update `plugins/<NAME>/permissions/default.toml`, and `apps/desktop/src-tauri/capabilities/default.json`plugins/importer/build.rs (1)
1-4: Update build.rs COMMANDS array to include "ping".The importer plugin registers the
pingcommand in lib.rs viatauri_specta::collect_commands![commands::ping::<Wry>,], but build.rs showsCOMMANDS: &[]. Update toconst COMMANDS: &[&str] = &["ping"];to match the actual command being exposed, consistent with how other plugins declare their commands in build.rs.⛔ Skipped due to learnings
Learnt from: CR Repo: fastrepl/hyprnote PR: 0 File: plugins/AGENTS.md:0-0 Timestamp: 2025-11-27T11:40:22.782Z Learning: Applies to plugins/*/src/lib.rs : After updating commands in `plugins/<NAME>/src/lib.rs`, run `codegen`, update `plugins/<NAME>/permissions/default.toml`, and `apps/desktop/src-tauri/capabilities/default.json`plugins/webhook/openapi.gen.json (1)
407-407: LGTM!Cosmetic change only—adds trailing newline for cleaner formatting.
plugins/settings/tsconfig.json (1)
1-5: LGTM!Standard TypeScript configuration for the settings plugin. Correctly extends the base config and includes the generated bindings.
apps/web/content/docs/faq/7.settings.mdx (1)
1-5: LGTM!Documentation addition for the new settings.json file location, aligning with the settings migration.
plugins/settings/js/index.ts (1)
1-1: LGTM!Standard re-export pattern for generated Tauri plugin bindings.
apps/desktop/src/components/main/sidebar/banner/index.tsx (2)
7-7: LGTM!Import updated to use the new settings-based configuration hook.
23-35: Config keys are properly defined in the settings store.The config keys
current_llm_provider,current_llm_model,current_stt_provider, andcurrent_stt_modelare all correctly defined in the settings store schema (lines 45, 49, 50, and 54 inapps/desktop/src/store/tinybase/settings.ts). The migration touseConfigValuesis sound.plugins/settings/src/ext.rs (1)
1-3: LGTM!Standard Rust extension trait pattern. The empty trait with blanket implementation sets up an extension point for future methods on
tauri::Managertypes.plugins/settings/.gitignore (1)
1-17: LGTM!Standard ignore patterns for a Tauri plugin with Rust and TypeScript components.
apps/desktop/src/components/settings/ai/stt/configure.tsx (1)
26-298: LGTM: consistent migration tosettingsstore hooks/STORE_ID.
No behavioral change beyond selecting the new store.Also applies to: 552-557
plugins/settings/Cargo.toml (1)
1-26: The concerns in this review are not valid issues. Bothedition = "2024"and thetauridependency withfeatures = ["test"]are workspace-wide standards—all 25 plugins inplugins/consistently use the same configuration. This is clearly an intentional design pattern, not a misconfiguration or oversight in this file.apps/desktop/src-tauri/src/lib.rs (1)
85-91: No action needed—thetauri-plugin-settingsintegration is properly wired. The capability ("settings:default") is present inapps/desktop/src-tauri/capabilities/default.json, the plugin is initialized inlib.rs, and codegen via the test harness (cargo test -p tauri-plugin-settings) handles TypeScript binding generation. All three commands (path,load,save) are correctly exported viatauri_specta.apps/desktop/package.json (1)
50-50: LGTM! New settings plugin dependency added correctly.The addition of the
@hypr/plugin-settingsworkspace dependency aligns with the unified settings store migration described in the PR objectives.apps/desktop/src/components/settings/general/index.tsx (1)
8-42: LGTM! Store migration implemented correctly.The migration from the main store to the settings store is clean and consistent. The
useSetPartialValuesCallbackhook now correctly referencessettings.UIandsettings.STORE_ID, while preserving all data transformation logic.apps/desktop/src/hooks/useLLMConnection.ts (1)
23-68: LGTM! Store references updated consistently.The hook now correctly reads
current_llm_provider,current_llm_model, and provider configuration from the unified settings store. All connection resolution logic remains intact.apps/desktop/src/hooks/useSTTConnection.ts (1)
11-27: LGTM! STT connection hook migrated successfully.The migration to the settings store is consistent with other hooks. Provider and model configuration now correctly sources from
settings.UIusingsettings.STORE_ID.apps/desktop/src/components/settings/general/notification.tsx (1)
18-110: LGTM! All notification setting handlers migrated correctly.All five
useSetValueCallbackhandlers now correctly usesettings.UIandsettings.STORE_ID. The form submission and state management logic remains unchanged and continues to work with the migrated handlers.apps/desktop/src/hooks/useCurrentModelModalitySupport.ts (1)
20-32: LGTM! Modality support hook migrated correctly.The hook now sources current model and provider data from the settings store while maintaining all modality detection logic. The migration is consistent with other hooks in this PR.
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
16-53: LGTM! Provider/model selection migrated correctly.Both
useSetValueCallbackhandlers for provider and model selection now correctly usesettings.UIandsettings.STORE_ID. The form logic and submission handlers work seamlessly with the migrated callbacks.
191-194: LGTM! Query reference updated to settings store.The
useResultTablehook correctly usessettings.QUERIES.llmProviderswithsettings.STORE_ID. ThellmProvidersquery is properly defined in the settings module and queries theai_providerstable with the expected schema (type, base_url). The provider configuration mapping logic remains unchanged.Cargo.toml (1)
128-128: Plugin registration is complete.The
tauri-plugin-settingsdependency is correctly added to Cargo.toml. Verification confirms:
- ✓ Plugin implementation exists in
plugins/settings/src/lib.rswith init function- ✓ Capabilities registered in
apps/desktop/src-tauri/capabilities/default.jsonassettings:defaultNo additional action required.
apps/desktop/src/config/use-config.ts (1)
1-4: LGTM! Clean migration to settings store.The import update and subsequent hook changes correctly migrate the configuration hooks from the main store to the new unified settings store. The logic and behavior remain unchanged.
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
18-18: LGTM! Store migration looks correct.The import and callback updates properly migrate to the unified settings store.
Also applies to: 37-49
224-227: Thesettings.QUERIES.sttProvidersquery is properly defined.The query is defined in the settings module at line 111 and is correctly accessible via
settings.QUERIES.sttProviders. The code change is valid.apps/desktop/src/main.tsx (1)
27-27: LGTM! Settings store component properly integrated.The new
SettingsStoreComponentis correctly added alongside the existingStoreComponent, with both receiving the same persistence configuration. This enables the unified settings store while maintaining backward compatibility with the main store.Also applies to: 110-111
plugins/settings/src/commands.rs (1)
3-27: LGTM! Clean command implementations.The three Tauri commands (
path,load,save) are well-structured wrappers around theSettingsStatemethods. The async/sync distinction is appropriate, and error handling viaResult<_, String>follows Tauri conventions.apps/desktop/src/store/tinybase/jsonPersister.test.ts (2)
6-49: LGTM! Comprehensive round-trip testing.The test suite properly validates bidirectional conversion between settings format and store format. The full data test covers AI providers (both LLM and STT), notification settings, and general settings.
102-116: Good edge case coverage for empty data.The empty data test correctly verifies that the conversion creates the expected default structure with empty nested objects for
ai,notification, andgeneralsections.plugins/settings/src/lib.rs (1)
1-37: No action needed. Thesettings:defaultpermission is already configured inapps/desktop/src-tauri/capabilities/default.json. The repository does not useplugins/settings/permissions/default.tomlfiles; permissions are managed directly in the capabilities configuration file where the settings plugin is already listed.Likely an incorrect or invalid review comment.
apps/desktop/src/store/tinybase/settings.ts (1)
114-199: Well-structured store component with proper lifecycle management.The component correctly handles:
- Schema-based store creation
- Migration from legacy keys.json and main store
- Conditional persistence based on window label
- Broadcast channel synchronization for multi-window support
- Query definitions for provider filtering
apps/desktop/src/store/tinybase/jsonPersister.ts (3)
185-212: Clean persister implementation with proper error handling.The persister correctly:
- Returns
undefinedon load errors (allowing graceful degradation)- Logs save errors without throwing (prevents persister from breaking)
- Uses reasonable 1-second polling interval
- Properly integrates with
commands.loadandcommands.save
131-134: No action needed. The spread operation at lines 131-134 is safe.The keys mapped under the
aipath in SETTINGS_MAPPING arecurrent_llm_provider,current_llm_model,current_stt_provider, andcurrent_stt_model— all scalar config values. TheproviderRowsToSettingsfunction returns an object with keysllmandsttcontaining nested provider configurations. Since these key names do not overlap, spreadingprovidersintosettings.aiwill not overwrite any existing settings fromstoreValuesToSettings.
14-16: The circular dependency exists but is safe.jsonPersister.tsimportsSETTINGS_MAPPINGfrom./settings, andsettings.tsimportscreateSettingsPersisterandmigrateKeysJsonToSettingsfrom./jsonPersister. However, this does not cause runtime issues because the imported functions are only called inside theuseCreatePersisterhook callback (lines 116–119), which executes during component render after both modules are fully initialized.SETTINGS_MAPPINGis used only within function bodies, not at module initialization time. This pattern is safe and does not require refactoring.
| tauri-plugin-process = { workspace = true } | ||
| tauri-plugin-sentry = { workspace = true } | ||
| tauri-plugin-settings = { workspace = true } | ||
| tauri-plugin-shell = { workspace = true } | ||
| tauri-plugin-single-instance = { workspace = true } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Check workspace root Cargo.toml for tauri-plugin-settings definition
rg "tauri-plugin-settings" --type toml -B 2 -A 2Repository: fastrepl/hyprnote
Length of output: 1544
🏁 Script executed:
# Look for tauri-plugin-settings initialization in the codebase
rg "tauri_plugin_settings" --type rust -B 2 -A 2Repository: fastrepl/hyprnote
Length of output: 479
🏁 Script executed:
# Check if capabilities/default.json exists and what it contains
cat apps/desktop/src-tauri/capabilities/default.json 2>/dev/null | head -100Repository: fastrepl/hyprnote
Length of output: 2551
🏁 Script executed:
# Check permissions files in plugins
find . -name "default.toml" -path "*/permissions/*" -exec cat {} \;Repository: fastrepl/hyprnote
Length of output: 43
Add permissions to apps/desktop/src-tauri/capabilities/default.json.
The tauri-plugin-settings dependency addition requires a corresponding permissions entry. Add "settings:default" to the permissions array in apps/desktop/src-tauri/capabilities/default.json to complete the plugin integration.
🤖 Prompt for AI Agents
In apps/desktop/src-tauri/capabilities/default.json around the permissions array
(near lines 52-56 in Cargo.toml context), add the permission string
"settings:default" to the JSON permissions array so the tauri-plugin-settings
dependency is granted the required capability; update the array to include
"settings:default" alongside existing entries and ensure the JSON remains valid
(commas and quoting correct).
| const result = await commands.save(settings); | ||
| if (result.status === "error") { | ||
| console.error("[migrateKeysJsonToSettings] save error:", result.error); | ||
| return false; | ||
| } | ||
|
|
||
| await remove(keysPath, options); | ||
| console.info( | ||
| "[migrateKeysJsonToSettings] migrated keys.json to settings.json", | ||
| ); | ||
| return true; |
There was a problem hiding this comment.
File removal failure after successful save leaves inconsistent state.
If commands.save succeeds but remove(keysPath) fails, the migration will be marked as failed (returns false) but settings were already saved. On next app start, migrateKeysJsonToSettings will run again, potentially causing duplicate migrations.
Consider separating the concerns or handling removal failure differently:
const result = await commands.save(settings);
if (result.status === "error") {
console.error("[migrateKeysJsonToSettings] save error:", result.error);
return false;
}
- await remove(keysPath, options);
+ try {
+ await remove(keysPath, options);
+ } catch (removeError) {
+ console.warn("[migrateKeysJsonToSettings] remove failed, but migration succeeded:", removeError);
+ // Migration was successful even if file removal failed
+ }
console.info(
"[migrateKeysJsonToSettings] migrated keys.json to settings.json",
);
return true;🤖 Prompt for AI Agents
In apps/desktop/src/store/tinybase/jsonPersister.ts around lines 168-178, the
code treats a failed remove(keysPath) as a migration failure after successfully
saving settings, which leaves an inconsistent state and can cause duplicate
runs; change the flow so that after commands.save succeeds you attempt
remove(keysPath) inside a try/catch, and if removal fails do NOT mark the
migration as failed—log the removal error (and emit telemetry) and return true
so the app won’t retry the migration, or alternatively attempt a rollback by
deleting the newly saved settings (using the appropriate commands API) and
return false only if the rollback succeeds; ensure the removal error is logged
with context and surfaced for later retry/cleanup.
| ); | ||
|
|
||
| useProvideStore(STORE_ID, store); | ||
| useProvideQueries(STORE_ID, queries!); |
There was a problem hiding this comment.
Non-null assertion on queries may cause runtime errors.
useCreateQueries could return undefined before the store is ready. Using ! assertion bypasses TypeScript's safety check.
Consider adding a null check or using optional chaining:
- useProvideQueries(STORE_ID, queries!);
+ useProvideQueries(STORE_ID, queries ?? undefined);Or conditionally provide queries only when defined:
- useProvideQueries(STORE_ID, queries!);
+ if (queries) {
+ useProvideQueries(STORE_ID, queries);
+ }🤖 Prompt for AI Agents
In apps/desktop/src/store/tinybase/settings.ts around line 194, the code uses a
non-null assertion on `queries` (useProvideQueries(STORE_ID, queries!)), which
can cause runtime errors if `useCreateQueries` returns undefined; change this to
only call useProvideQueries when `queries` is defined (e.g., check `if (queries)
{ useProvideQueries(STORE_ID, queries) }` or otherwise guard/early-return until
the store is ready) so you avoid the `!` assertion and preserve type safety.
| for (const [rowId, row] of Object.entries(mainProviders)) { | ||
| if (row.api_key || row.base_url) { | ||
| settingsStore.setRow("ai_providers", rowId, { | ||
| type: row.type ?? "", | ||
| base_url: row.base_url ?? "", | ||
| api_key: row.api_key ?? "", | ||
| }); | ||
| } | ||
| mainStore.delRow("ai_providers", rowId); | ||
| } |
There was a problem hiding this comment.
Provider rows deleted from main store even when not migrated.
If a row has neither api_key nor base_url, it won't be copied to settings store (line 210-216), but will still be deleted from main store (line 217). This could cause unintended data loss for incomplete provider entries.
Consider moving the delete inside the condition block or removing the condition:
for (const [rowId, row] of Object.entries(mainProviders)) {
- if (row.api_key || row.base_url) {
- settingsStore.setRow("ai_providers", rowId, {
- type: row.type ?? "",
- base_url: row.base_url ?? "",
- api_key: row.api_key ?? "",
- });
- }
- mainStore.delRow("ai_providers", rowId);
+ settingsStore.setRow("ai_providers", rowId, {
+ type: row.type ?? "",
+ base_url: row.base_url ?? "",
+ api_key: row.api_key ?? "",
+ });
+ mainStore.delRow("ai_providers", rowId);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const [rowId, row] of Object.entries(mainProviders)) { | |
| if (row.api_key || row.base_url) { | |
| settingsStore.setRow("ai_providers", rowId, { | |
| type: row.type ?? "", | |
| base_url: row.base_url ?? "", | |
| api_key: row.api_key ?? "", | |
| }); | |
| } | |
| mainStore.delRow("ai_providers", rowId); | |
| } | |
| for (const [rowId, row] of Object.entries(mainProviders)) { | |
| settingsStore.setRow("ai_providers", rowId, { | |
| type: row.type ?? "", | |
| base_url: row.base_url ?? "", | |
| api_key: row.api_key ?? "", | |
| }); | |
| mainStore.delRow("ai_providers", rowId); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/store/tinybase/settings.ts around lines 209 to 218, provider
rows are always deleted from mainStore even when they are not migrated because
the mainStore.delRow call is outside the if that checks for api_key/base_url;
move the deletion into the if block (or otherwise only call delRow when you
actually setRow successfully) so that rows lacking api_key and base_url are not
removed, ensuring deletion happens only after a successful migration.
| const COMMANDS: &[&str] = &["path", "load", "save"]; | ||
|
|
||
| fn main() { | ||
| tauri_plugin::Builder::new(COMMANDS).build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify settings plugin permissions and capabilities are configured
echo "=== Checking permissions file ==="
if [ -f "plugins/settings/permissions/default.toml" ]; then
cat plugins/settings/permissions/default.toml
else
echo "WARNING: permissions/default.toml not found"
fi
echo -e "\n=== Checking capabilities file ==="
if [ -f "apps/desktop/src-tauri/capabilities/default.json" ]; then
rg -A 2 -B 2 "settings" apps/desktop/src-tauri/capabilities/default.json || echo "No 'settings' found in capabilities"
else
echo "WARNING: capabilities/default.json not found"
fi
echo -e "\n=== Checking lib.rs for command registration ==="
if [ -f "plugins/settings/src/lib.rs" ]; then
rg -n "invoke_handler|path|load|save" plugins/settings/src/lib.rs
else
echo "WARNING: src/lib.rs not found"
fi
echo -e "\n=== Listing plugins/settings directory ==="
ls -la plugins/settings/Repository: fastrepl/hyprnote
Length of output: 1162
Create plugins/settings/permissions/default.toml.
The commands are properly registered in src/lib.rs and the capabilities have been updated in apps/desktop/src-tauri/capabilities/default.json, but plugins/settings/permissions/default.toml is missing. This file should define the permission scopes for the plugin's commands (path, load, save).
🤖 Prompt for AI Agents
In plugins/settings/build.rs around lines 1 to 5, the plugin registers commands
("path", "load", "save") but the permissions file
plugins/settings/permissions/default.toml is missing; create that file and
declare permission scopes for each command (path, load, save) mapping each
command to an appropriate permission key and human-readable description so the
desktop capability JSON can map to these scopes—ensure the TOML has entries for
each command scope, a short description, and default access level so the
plugin's commands are covered by permissions.
| use serde::{Serialize, ser::Serializer}; | ||
|
|
||
| pub type Result<T> = std::result::Result<T, Error>; | ||
|
|
||
| #[derive(Debug, thiserror::Error)] | ||
| pub enum Error {} | ||
|
|
||
| impl Serialize for Error { | ||
| fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> | ||
| where | ||
| S: Serializer, | ||
| { | ||
| serializer.serialize_str(self.to_string().as_ref()) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Uninhabited enum Error {} is effectively unusable; add variants or remove the public API.
Right now Result<T, Error> suggests fallible APIs but you can’t ever produce an Error.
One option (minimal) is to define string-backed variants and keep the “serialize as string” behavior:
use serde::{Serialize, ser::Serializer};
pub type Result<T> = std::result::Result<T, Error>;
#[derive(Debug, thiserror::Error)]
-pub enum Error {}
+pub enum Error {
+ #[error("io: {0}")]
+ Io(String),
+ #[error("json: {0}")]
+ Json(String),
+ #[error("{0}")]
+ Other(String),
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use serde::{Serialize, ser::Serializer}; | |
| pub type Result<T> = std::result::Result<T, Error>; | |
| #[derive(Debug, thiserror::Error)] | |
| pub enum Error {} | |
| impl Serialize for Error { | |
| fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> | |
| where | |
| S: Serializer, | |
| { | |
| serializer.serialize_str(self.to_string().as_ref()) | |
| } | |
| } | |
| use serde::{Serialize, ser::Serializer}; | |
| pub type Result<T> = std::result::Result<T, Error>; | |
| #[derive(Debug, thiserror::Error)] | |
| pub enum Error { | |
| #[error("io: {0}")] | |
| Io(String), | |
| #[error("json: {0}")] | |
| Json(String), | |
| #[error("{0}")] | |
| Other(String), | |
| } | |
| impl Serialize for Error { | |
| fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> | |
| where | |
| S: Serializer, | |
| { | |
| serializer.serialize_str(self.to_string().as_ref()) | |
| } | |
| } |
🤖 Prompt for AI Agents
In plugins/settings/src/error.rs around lines 1 to 15, the public type alias
Result<T, Error> references an uninhabited enum Error which makes the API
unusable; update the file to make Error instantiable by either (a) adding
meaningful variants (e.g., String-backed or named variants for expected failure
cases) and derive/implement Display/thiserror so to_string() works with the
existing Serialize impl, or (b) if there are truly no errors, remove the public
Result alias and expose std::result::Result<T, Never> or use a non-fallible API;
prefer adding explicit variants (and annotate with #[derive(Debug,
thiserror::Error, Serialize)] or implement Display) to preserve the current
“serialize as string” behavior.
| pub async fn save(&self, settings: serde_json::Value) -> Result<(), String> { | ||
| let _guard = self.lock.write().await; | ||
|
|
||
| if let Some(parent) = self.path.parent() { | ||
| tokio::fs::create_dir_all(parent) | ||
| .await | ||
| .map_err(|e| format!("create dir: {}", e))?; | ||
| } | ||
|
|
||
| let tmp_path = self.path.with_extension("json.tmp"); | ||
| let content = | ||
| serde_json::to_string_pretty(&settings).map_err(|e| format!("serialize: {}", e))?; | ||
|
|
||
| tokio::fs::write(&tmp_path, &content) | ||
| .await | ||
| .map_err(|e| format!("write tmp: {}", e))?; | ||
|
|
||
| tokio::fs::rename(&tmp_path, &self.path) | ||
| .await | ||
| .map_err(|e| format!("rename: {}", e))?; | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Windows bug: rename(tmp, path) fails when path already exists → subsequent saves can error.
This will likely break “save settings twice” on Windows.
- tokio::fs::rename(&tmp_path, &self.path)
- .await
- .map_err(|e| format!("rename: {}", e))?;
+ // On Windows, rename fails if destination exists. Remove first and retry.
+ match tokio::fs::rename(&tmp_path, &self.path).await {
+ Ok(()) => {}
+ Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
+ tokio::fs::remove_file(&self.path)
+ .await
+ .map_err(|e| format!("remove existing: {}", e))?;
+ tokio::fs::rename(&tmp_path, &self.path)
+ .await
+ .map_err(|e| format!("rename: {}", e))?;
+ }
+ Err(e) => return Err(format!("rename: {}", e)),
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub async fn save(&self, settings: serde_json::Value) -> Result<(), String> { | |
| let _guard = self.lock.write().await; | |
| if let Some(parent) = self.path.parent() { | |
| tokio::fs::create_dir_all(parent) | |
| .await | |
| .map_err(|e| format!("create dir: {}", e))?; | |
| } | |
| let tmp_path = self.path.with_extension("json.tmp"); | |
| let content = | |
| serde_json::to_string_pretty(&settings).map_err(|e| format!("serialize: {}", e))?; | |
| tokio::fs::write(&tmp_path, &content) | |
| .await | |
| .map_err(|e| format!("write tmp: {}", e))?; | |
| tokio::fs::rename(&tmp_path, &self.path) | |
| .await | |
| .map_err(|e| format!("rename: {}", e))?; | |
| Ok(()) | |
| } | |
| pub async fn save(&self, settings: serde_json::Value) -> Result<(), String> { | |
| let _guard = self.lock.write().await; | |
| if let Some(parent) = self.path.parent() { | |
| tokio::fs::create_dir_all(parent) | |
| .await | |
| .map_err(|e| format!("create dir: {}", e))?; | |
| } | |
| let tmp_path = self.path.with_extension("json.tmp"); | |
| let content = | |
| serde_json::to_string_pretty(&settings).map_err(|e| format!("serialize: {}", e))?; | |
| tokio::fs::write(&tmp_path, &content) | |
| .await | |
| .map_err(|e| format!("write tmp: {}", e))?; | |
| // On Windows, rename fails if destination exists. Remove first and retry. | |
| match tokio::fs::rename(&tmp_path, &self.path).await { | |
| Ok(()) => {} | |
| Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { | |
| tokio::fs::remove_file(&self.path) | |
| .await | |
| .map_err(|e| format!("remove existing: {}", e))?; | |
| tokio::fs::rename(&tmp_path, &self.path) | |
| .await | |
| .map_err(|e| format!("rename: {}", e))?; | |
| } | |
| Err(e) => return Err(format!("rename: {}", e)), | |
| } | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In plugins/settings/src/state.rs around lines 32 to 54, the atomic save uses
tokio::fs::rename(tmp, path) which fails on Windows if the destination already
exists; update save to handle that by attempting to rename and, on failure due
to existing destination, remove the existing self.path (ignoring NotFound) and
retry the rename once, propagating any other errors; ensure removal and retry
are awaited and mapped to appropriate error messages so the save succeeds when
overwriting an existing file on Windows.
1 participant
No description provided.