Skip to content

refactor(architecture): tighten desktop boundaries#435

Draft
hiqiancheng wants to merge 14 commits into
mainfrom
codex/issue-434-architecture-boundaries
Draft

refactor(architecture): tighten desktop boundaries#435
hiqiancheng wants to merge 14 commits into
mainfrom
codex/issue-434-architecture-boundaries

Conversation

@hiqiancheng

@hiqiancheng hiqiancheng commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

First external contributors may need to complete the CLA Assistant check before merge.
For code changes, this PR must link the related issue or RFC in the section below.

Summary

Tightens the desktop frontend architecture boundaries for RFC #434.

  • Moves stable cross-layer DTOs into apps/desktop/src/contracts while keeping runtime policy in application or services.

  • Splits popup definitions so manifest/layout metadata is contract-owned and Vue component binding stays in PopupView.

  • Extracts attachment storage, HTTP fetch, database runtime, data backup, and provider config policy out of overloaded service internals.

  • Strengthens import-boundary tests so baseline violations and unexpected dependency cycles are caught explicitly.

  • Adds direct boundary characterization tests for popup manifest/component sync, DatabaseRuntimeService, HttpService Tauri fetch wrapping, and DataManagementService backup failure semantics.

  • Strengthens desktop E2E smoke coverage for startup, quick search results, model dropdown popup, session history popup, settings persistence, and settings section navigation.

  • Adds reusable E2E window helpers that identify Tauri windows by in-page state and include diagnostic snapshots on multi-window failures.

  • Adds popup-window smoke coverage for both registered popup manifests and the Vue component binding path.

  • Skips remote model metadata bootstrap in E2E mode so UI critical-path smoke tests are not gated on network metadata refresh.
    User-facing impact: no intended behavior change. This is an architecture-boundary refactor aimed at reducing future cross-layer coupling.

Related issue or RFC

Link the issue or RFC:

For code changes, link the tracking issue. Only documentation wording, link fixes, or comment-only cleanups may skip the issue-first flow.

AI assistance disclosure

If AI tools materially assisted this contribution, disclose that here or point to the relevant commit trailer.

  • Tool(s) used: Codex; Brooks-lint subagent (Volta) for architecture and test-quality review.
  • Scope of assistance: implementation planning, code changes, boundary-test hardening, and review of architecture/test-suite risks.
  • Human review or rewrite performed: changes were reviewed during preparation; maintainers should still review the full boundary model and risk areas.
  • Architecture or boundary impact: yes; this PR moves shared contracts and runtime policy across desktop frontend layers under accepted RFC [RFC]: Refactor desktop frontend architecture boundaries #434.

Testing evidence

List the commands you ran and the results you observed.

  • pnpm --filter @touchai/desktop test:e2e - passed locally; startup, quick search, model dropdown popup, session history popup, and settings smoke specs ran together with 5 passing.
  • pnpm --filter @touchai/desktop test:unit -- tests/i18n/main-bootstrap.test.ts - passed; Vitest executed the desktop frontend suite: 133 files / 771 tests.
  • pnpm --filter @touchai/desktop test:typecheck - passed.
  • pnpm --filter @touchai/desktop type:check - passed.
  • pnpm --filter @touchai/desktop lint:check - passed with 20 existing warnings and 0 errors.
  • pnpm --filter @touchai/desktop format:check - passed.
  • Commit hook verification also passed pnpm --filter @touchai/desktop check:rust, pnpm --filter @touchai/desktop type:check, staged ESLint, and staged Prettier; Rust emitted 2 existing unused warnings in src/core/search/manager.rs.
  • Earlier focused tests passed for architecture import boundaries, popup manifest/component sync, DatabaseRuntimeService, HttpService Tauri fetch wrapping, DataManagementService backup failure semantics, setting-tool i18n, AgentService error i18n, useAgent i18n, database backup i18n, and NativeService coverage touched by this refactor.
  • pnpm test:pr - check and test:unit passed, then test:rust stopped when build.rs timed out downloading https://github.com/rtk-ai/rtk/releases/download/v0.40.0/rtk-x86_64-pc-windows-msvc.zip with Windows socket error 10060.
  • Local test:rust retry could not proceed because this machine has C: full, no E:/G: drives, and L:\TouchAI cannot be created (The request is not supported). CI should provide the first complete Rust proof for this PR.

Did you follow TDD (test-first) for feature and fix work? Strongly recommended. See docs/testing/testing.md.

pnpm test:pr

Risk notes

  • AgentService, runtime, MCP, or schema impact: touches AgentService runtime boundaries and shared contracts; no MCP behavior change intended.
  • database baseline or migration impact: no schema, migration, or baseline changes intended.
  • release or packaging impact: only test timeout handling for Velopack release-asset normalization was adjusted; no packaging behavior change intended.

Screenshots or recordings

Not applicable; no intended UI behavior change.

Checklist

  • The PR title follows Conventional Commits and is valid for squash merge.
  • This PR is either ready for review or explicitly marked as a Draft PR.
  • I did not use [WIP] or similar title prefixes.
  • If AI materially assisted this PR, I disclosed the tools and scope and I personally reviewed every affected change.
  • I can explain the why, what, and how of this change without relying on an AI tool.
  • If this touches AgentService, runtime, MCP, or schema boundaries, there is an accepted RFC.
  • If this changes architecture or adds a new cross-boundary abstraction, there is an accepted RFC.
  • I ran pnpm test:pr for this code PR, or this is a docs-only change.
  • If I changed Rust behavior or tests, I reviewed pnpm test:coverage:rust or relied on CI coverage evidence.
  • If I changed desktop startup/window/search/popup/settings/E2E paths, I ran pnpm test:e2e locally or documented why CI is the first valid proof.
  • I added tests or explained why tests are not appropriate.
  • I updated docs when behavior changed.

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f99be53c-06c0-48f2-9d9c-832599aff1cb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-434-architecture-boundaries

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added area:frontend Frontend UI or view-layer changes area:database Schema, persistence, or migration changes area:agent-service AgentService and conversation runtime changes labels Jun 7, 2026
Strengthen startup, quick search, and settings smoke coverage.

Skip remote model metadata bootstrap in E2E mode to avoid network noise.

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

Labels

area:agent-service AgentService and conversation runtime changes area:database Schema, persistence, or migration changes area:frontend Frontend UI or view-layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant