Commit Graph

8 Commits

Author SHA1 Message Date
Jason
7b1a68ee4e refactor(backend): phase 5 - optimize concurrency with RwLock and async IO
Replace Mutex with RwLock for AppState.config to enable concurrent reads,
improving performance for tray menu building and query operations that
previously blocked each other unnecessarily.

Key changes:
- Migrate AppState.config from Mutex<MultiAppConfig> to RwLock<MultiAppConfig>
- Distinguish read-only operations (read()) from mutations (write()) across
  all command handlers and service layers
- Offload blocking file I/O in import/export commands to spawn_blocking threads,
  minimizing lock hold time and preventing main thread blocking
- Extract load_config_for_import() to separate I/O logic from state updates
- Update all integration tests to use RwLock semantics

Performance impact:
- Concurrent reads: Multiple threads can now query config simultaneously
  (tray menu, provider list, MCP config)
- Reduced contention: Write locks only acquired during actual mutations
- Non-blocking I/O: Config import/export no longer freezes UI thread

All existing tests pass with new locking semantics.
2025-10-28 12:23:44 +08:00
Jason
7e27f88154 refactor(backend): phase 4 - add test hooks and extend service layer
- Extract internal functions in commands/mcp.rs and commands/provider.rs
  to enable unit testing without Tauri context
- Add test hooks: set_mcp_enabled_test_hook, import_mcp_from_claude_test_hook,
  import_mcp_from_codex_test_hook, import_default_config_test_hook
- Migrate error types from String to AppError for precise error matching in tests
- Extend ProviderService with delete() method to unify Codex/Claude cleanup logic
- Add comprehensive test coverage:
  - tests/mcp_commands.rs: command-level tests for MCP operations
  - tests/provider_service.rs: service-level tests for switch/delete operations
- Run cargo fmt to fix formatting issues (EOF newlines)
- Update BACKEND_REFACTOR_PLAN.md to mark phase 3 complete
2025-10-28 11:58:57 +08:00
Jason
c2e8855a0f refactor(backend): phase 4 - extract provider service layer
Architecture improvements:
- Extract ProviderService with switch/backfill/write methods
- Reduce command layer from 160 to 13 lines via delegation
- Separate business logic (services) from state management (commands)
- Introduce precise error handling with structured validation

Refactoring details:
- Split Codex/Claude switching into symmetric private methods
- Add multi-layer validation for Codex auth field (existence + type)
- Extract import_config_from_path for command and test reuse
- Expose export_config_to_file and ProviderService in public API

Test coverage:
- Add 10+ integration tests for Claude/Codex switching flows
- Cover import/export success and failure scenarios (JSON parse, missing file)
- Verify state consistency on error paths (current remains unchanged)
- Test snapshot backfill for both old and new providers after switching
2025-10-28 10:47:48 +08:00
Jason
8e980e6974 refactor(backend): phase 3 - unify error handling and fix backup timestamp bug
Key improvements:
- Extract switch_provider_internal() returning AppError for better testability
- Fix backup mtime inheritance: use read+write instead of fs::copy to ensure latest backup survives cleanup
- Add 15+ integration tests covering provider commands, atomic writes, and rollback scenarios
- Expose write_codex_live_atomic, AppState, and test hooks in public API
- Extract tests/support.rs with isolated HOME and mutex utilities

Test coverage:
- Provider switching with live config backfill and MCP sync
- Codex atomic write success and failure rollback
- Backup retention policy with proper mtime ordering
- Negative cases: missing auth field, invalid provider ID
2025-10-28 09:55:10 +08:00
Jason
10abdfa096 refactor(backend): phase 3 - expand integration tests for Codex and MCP sync
Expand test suite from 3 to 11 integration tests, adding comprehensive coverage
for Codex dual-file atomicity and bidirectional MCP synchronization:

New Codex sync tests:
- sync_codex_provider_writes_auth_and_config: validates atomic write of auth.json
  and config.toml, plus SSOT backfill of latest toml content
- sync_enabled_to_codex_writes_enabled_servers: MCP projection to config.toml
- sync_enabled_to_codex_removes_servers_when_none_enabled: cleanup when all disabled
- sync_enabled_to_codex_returns_error_on_invalid_toml: error handling for malformed TOML

New Codex MCP import tests:
- import_from_codex_adds_servers_from_mcp_servers_table: imports new servers from live config
- import_from_codex_merges_into_existing_entries: smart merge preserving SSOT server configs

New Claude MCP tests:
- sync_claude_enabled_mcp_projects_to_user_config: enabled/disabled filtering for .claude.json
- import_from_claude_merges_into_config: intelligent merge preserving existing configurations

Expand lib.rs API exports:
- Codex paths: get_codex_auth_path, get_codex_config_path
- Claude MCP: get_claude_mcp_path
- MCP sync: sync_enabled_to_claude, sync_enabled_to_codex
- MCP import: import_from_claude, import_from_codex
- Error type: AppError (for test assertions)

Test infrastructure improvements:
- Enhanced reset_test_fs() to clean .claude.json
- All tests use isolated HOME directory with sequential execution via mutex

Test results: 11/11 passed
Files changed: 3 (+394/-6 lines)

Next steps: Command layer integration tests and error recovery scenarios
2025-10-27 23:26:42 +08:00
Jason
6a9aa7aeb5 refactor(backend): phase 3 - add integration tests for config sync (partial)
Add integration test suite with isolated test environment:
- New test file: tests/import_export_sync.rs (149 lines, 3 test cases)
  * sync_claude_provider_writes_live_settings: validates SSOT sync to live settings
  * create_backup_skips_missing_file: edge case handling for missing config
  * create_backup_generates_snapshot_file: verifies backup snapshot creation
- Test infrastructure:
  * OnceLock-based isolated HOME directory in temp folder
  * Mutex guard to ensure sequential test execution (avoid file system race)
  * Automatic cleanup between test runs

Export core APIs for testing (lib.rs):
- AppType, MultiAppConfig, Provider (data structures)
- get_claude_settings_path, read_json_file (config utilities)
- create_backup, sync_current_providers_to_live (sync operations)
- update_settings, AppSettings (settings management)

Adjust visibility:
- import_export::sync_current_providers_to_live: fn -> pub fn

Update documentation:
- Mark Phase 3 as in-progress (🚧) in BACKEND_REFACTOR_PLAN.md
- Document current test coverage scope and pending scenarios

Test results: 7/7 passed (4 unit + 3 integration)
Build time: 0.16s

Next steps:
- Add Codex sync tests (auth.json + config.toml atomic writes)
- Add MCP sync integration tests
- Add end-to-end provider switching tests
2025-10-27 22:30:57 +08:00
Jason
9f5c2b427f refactor(backend): phase 2 - split commands.rs by domain (100%)
Split monolithic commands.rs (1525 lines) into 7 domain-focused modules
to improve maintainability and readability while preserving the external API.

## Changes

### Module Structure

Created `commands/` directory with domain-based organization:

- **provider.rs** (946 lines, 15 commands)
  - Provider CRUD operations (get, add, update, delete, switch)
  - Usage query integration
  - Endpoint speed testing and custom endpoint management
  - Sort order management
  - Largest file but highly cohesive (all provider-related)

- **mcp.rs** (235 lines, 13 commands)
  - Claude MCP management (~/.claude.json)
  - SSOT MCP config management (config.json)
  - Sync operations (Claude ↔ Codex)
  - Import/export functionality

- **config.rs** (153 lines, 8 commands)
  - Config path queries (Claude/Codex)
  - Directory operations (open, pick)
  - Config status checks
  - Parameter compatibility layer (app_type/app/appType)

- **settings.rs** (40 lines, 5 commands)
  - App settings management
  - App restart functionality
  - app_config_dir override (Store integration)

- **plugin.rs** (36 lines, 4 commands)
  - Claude plugin management (~/.claude/config.json)
  - Plugin status and config operations

- **misc.rs** (45 lines, 3 commands)
  - External link handling
  - Update checks
  - Portable mode detection

- **mod.rs** (15 lines)
  - Module exports via `pub use`
  - Preserves flat API structure

### API Preservation

- Used `pub use` pattern to maintain external API
- All commands still accessible as `commands::function_name`
- Zero breaking changes for frontend code
- lib.rs invoke_handler unchanged (48 commands registered)

## Statistics

- Files: 1 → 7 (modular organization)
- Lines: 1525 → 1470 (net -55 lines, -3.6%)
- Commands: 48 → 48 (all preserved)
- Average file size: 210 lines (excluding provider.rs)
- Compilation:  Success (6.92s, 0 warnings)
- Tests:  4/4 passed

## Benefits

- **Maintainability**: Easier to locate and modify domain-specific code
- **Readability**: Smaller files (~200 lines) vs monolithic 1500+ lines
- **Testability**: Can unit test individual modules in isolation
- **Scalability**: Clear pattern for adding new command groups
- **Zero Risk**: No API changes, all tests passing

## Design Decisions

1. **Domain-based split**: Organized by business domain (provider, mcp, config)
   rather than technical layers (crud, query, sync)

2. **Preserved provider.rs size**: Kept at 946 lines to maintain high cohesion
   (all provider-related operations together). Can be further split in Phase 2.1
   if needed.

3. **Parameter compatibility**: Retained multiple parameter names (app_type, app,
   appType) for backward compatibility with different frontend call styles

## Phase 2 Status:  100% Complete

Ready for Phase 3: Adding integration tests.

Co-authored-by: Claude <noreply@anthropic.com>
2025-10-27 22:18:05 +08:00
Jason
c01e495eea refactor(backend): phase 1 - unified error handling with thiserror
Introduce AppError enum to replace Result<T, String> pattern across
the codebase, improving error context preservation and type safety.

## Changes

### Core Infrastructure
- Add src/error.rs with AppError enum using thiserror
- Add thiserror dependency to Cargo.toml
- Implement helper functions: io(), json(), toml() for ergonomic error creation
- Implement From<PoisonError> for automatic lock error conversion
- Implement From<AppError> for String to maintain Tauri command compatibility

### Module Migrations (60% complete)
- config.rs: Full migration to AppError
  - read_json_file, write_json_file, atomic_write
  - archive_file, copy_file, delete_file
- claude_mcp.rs: Full migration to AppError
  - get_mcp_status, read_mcp_json, upsert_mcp_server
  - delete_mcp_server, validate_command_in_path
  - set_mcp_servers_map
- codex_config.rs: Full migration to AppError
  - write_codex_live_atomic with rollback support
  - read_and_validate_codex_config_text
  - validate_config_toml
- app_config.rs: Partial migration
  - MultiAppConfig::load, MultiAppConfig::save
- store.rs: Partial migration
  - AppState::save now returns Result<(), AppError>
- commands.rs: Minimal changes
  - Use .map_err(Into::into) for compatibility
- mcp.rs: Minimal changes
  - sync_enabled_to_claude uses Into::into conversion

### Documentation
- Add docs/BACKEND_REFACTOR_PLAN.md with detailed refactoring roadmap

## Benefits
- Type-safe error handling with preserved error chains
- Better error messages with file paths and context
- Reduced boilerplate code (118 Result<T, String> instances to migrate)
- Automatic error conversion for seamless integration

## Testing
- All existing tests pass (4/4)
- Compilation successful with no warnings
- Build time: 0.61s (no performance regression)

## Remaining Work
- claude_plugin.rs (7 functions)
- migration.rs, import_export.rs
- Add unit tests for error.rs
- Complete commands.rs migration after dependent modules

Co-authored-by: Claude <claude@anthropic.com>
2025-10-27 16:29:11 +08:00