Skip to content

refactor: consolidate duplicated shared logic - #780

Open
beruro wants to merge 9 commits into
developfrom
junyu/consolidate-shared-logic-refactors
Open

refactor: consolidate duplicated shared logic#780
beruro wants to merge 9 commits into
developfrom
junyu/consolidate-shared-logic-refactors

Conversation

@beruro

@beruro beruro commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

Several mature code paths independently reimplemented the same domain decisions, lifecycle shells, projections, provider commands, or search utilities. That duplicated ownership made fixes prone to drift and left three uncompiled imported-history modules as dead maintenance surface. The cleanup was initially split across #771#779, which made one systematic reuse pass unnecessarily expensive to review and run through CI.

Solution

Consolidate the reuse cleanup into one PR while preserving nine focused commits so each boundary can still be reviewed or reverted independently:

  1. Remove three uncompiled imported-history modules.
  2. Centralize simulator event classification.
  3. Reuse the canonical Cloud session-scope resolver.
  4. Centralize external MCP config loading.
  5. Centralize LSP command detection.
  6. Share Activity Group render projection and lazy tool usage.
  7. Share the Claude/Codex OAuth session-setup shell while retaining provider-specific capture logic.
  8. Reuse the canonical fuzzy-search utilities in Spotlight symbol search.
  9. Share the sqlx-backed Tauri lifecycle between PostgreSQL and MySQL providers while keeping typed dialect differences explicit.

Public imports, IPC/wire commands, persisted formats, provider-specific token maps, database SQL dialect behavior, and existing user-facing flows remain compatible. The two UI extractions include their required frontend UI audit reports and regression coverage.

This PR supersedes #771, #772, #773, #774, #775, #776, #777, #778, and #779.

Potential risks

The combined diff spans independent runtime areas, so a reviewer must consider more surface than in a narrowly scoped PR. The commit boundaries, focused regression tests, and unchanged public contracts limit coupling and allow individual rollback by reverting the relevant commit. OAuth behavior still depends on native WebView/browser integration, and database behavior still depends on real PostgreSQL/MySQL drivers; those live credential paths were not exercised locally. There are no migrations, destructive data changes, dependency updates, or persistence/wire-format changes.

Audit

Architecture review covered ownership, type boundaries, state transitions, Tauri/wire compatibility, error mapping, initialization parity, naming, dead code, and test topology. UI audit verdicts were 1 abstract / 3 keep-with-reason for Activity Groups and 2 abstract / 3 fix / 2 keep-with-reason for OAuth Session Setup.

Effects: the OAuth shell retains only external browser/WebView lifecycle synchronization. The completion timer is cleared, external close is idempotent, and duplicate retries are coalesced. No polling, worker, cache, stream, or retained background loop was added.

Verification

  • pnpm typecheck && pnpm lint — executed on the consolidated branch; passed.
  • pnpm run test — executed on the consolidated branch; full Vitest suite passed.
  • node scripts/tauri/prepare-sidecars.cjs --profile debug — passed and staged the local debug sidecar.
  • cargo clippy --workspace --all-targets -- -D warnings — local Rust 1.91 stopped on the unchanged src-tauri/crates/orgtrack-core/src/sources/imported_history/window.rs:189 collapsible_else_if baseline warning. This PR does not modify that file or line.
  • Consolidated GitHub CI — Frontend (typecheck · lint · test), Rust (clippy), and AI attribution all passed.
  • All three GitHub checks were also successful on each source PR before consolidation.
  • The repository snapshot does not expose verify:quick / verify:final; the full commands above were used instead.
  • Live native OAuth and credentialed PostgreSQL/MySQL connectivity were not run locally.
  • No screenshot is included because the UI work consolidates existing presentation; it does not intentionally redesign the rendered states.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant