Add a Code Organization subsection that points to the main, renderer, and shared-layer architecture references so contributors learn where code goes before adding it.
main-process-architecture.md §3: state the no-renderer-imports rule (src/main + src/preload must not import renderer code; cross-process types → @shared, main-only → src/main), now enforced by the ESLint no-restricted-imports rule; clarify that the existing 'no automated enforcement yet' note refers to the internal direction edges. §7: track the lone remaining exception — main/utils/language.ts's relative renderer i18n import, deferred to the i18n migration PR.
renderer-architecture.md §7: add the anti-pattern of importing a shared bucket root (@renderer/utils, @renderer/types) instead of the specific file/topic, or giving types/ or utils/ a re-export root index.ts.
The node build aliased @types to src/renderer/types, letting main and preload import renderer code — a layering violation. Remove that alias from tsconfig.node.json and the electron.vite main config, and relocate every type main needs by its real cross-process usage.
Cross-process (both main and renderer) → @shared: utils/{systemProviderId,reasoning,mcp,ocr}, types/{ocr,notification,backup,apiGateway,note,image,plugin,skill}, types/mcp (McpTool/McpPrompt/McpResource), data/types/file/legacyFileMetadata (ImageFileMetadata). Main-only → src/main: services/remotefile/types, markdownParser PluginError, ai/mcp/types (McpCallToolResponse/McpToolResultContent/GetResourceResponse). v1 legacy shapes (Provider/Model/Shortcut, read by migrators + legacy services) → data/migration/v2/legacyTypes.ts, kept out of @shared as throwaway.
Renderer keeps its own @types alias; renderer/types re-exports the moved symbols so its barrel is unchanged for renderer consumers. Add an ESLint no-restricted-imports boundary rule banning @types/@renderer from src/main and src/preload. The relative renderer i18n import in main/utils/language.ts is left with a TODO and deferred to a dedicated i18n PR.
Mirror Shared Layer Architecture §3.1 onto the renderer top-level type buckets: types/ and utils/ are categories, not modules, so they carry no root index.ts. Consumers import @renderer/<bucket>/<topic>; a multi-file topic exposes a single curated index.ts with named exports and no wildcard re-export (export *).
Record the current deviations as §8 migration items: the types/ and utils/ root barrels, and the redundant @types alias (dissolve in favor of @renderer/types/<topic>).
Add main-process-architecture.md — the main-process peer of the renderer and shared-layer docs: a charter per top-level directory, placement rules, dependency direction, governance, and current deviations.
Refocus architecture-overview.md on the cross-process picture: refresh the process model and data flow to v2 (drop Redux), slim main-only sections to pointers, and replace the duplicated subsystem table with a reference map.
Cross-reference the new doc from naming-conventions §4.8 (per-root closed top-level) and §4.10 (feature vs type-bucket placement).
Remove the unused `LoaderReturn` from `@shared/types/codeTools.ts` (no consumers on main or the feat/chat-page truth branch). Its `status` field was the sole reason `@shared` imported `ProcessingStatus` from `@types` (src/renderer/types) — a Layer-4 layering violation now eliminated.
Record the remaining types/utils single-process residue (searchSnippet, pdf, EXTERNAL_APPS) and the error/serializable duplication cluster as a migration backlog in shared-layer-architecture.md; keywordSearch and SerializableSchema were verified cross-process via feat/chat-page and correctly stay.
`.ts` files under `src/shared` must use camelCase (naming-conventions §3.2);
kebab-case is only sanctioned under `packages/ui/` and `src/renderer/routes/`.
The `presets/` kebab naming came from best-practice-layered-preset-pattern.md,
which predated and conflicted with the authoritative spec.
- Rename presets/{code-cli,default-assistant,file-processing,mini-apps,
translate-languages,web-search-providers}.ts and utils/code-languages.ts
(plus the two matching __tests__ files) to camelCase, and update all importers
- Fix the upstream generator scripts/update-languages.ts to emit
codeLanguages.ts; otherwise `pnpm update:languages` would recreate the
kebab-named file
- Correct best-practice-layered-preset-pattern.md (kebab -> camelCase) and link
it to naming-conventions §3.2 so it cannot drift again
- Fix two stale `types/file` path references in file/architecture.md
Dissolve @shared/utils/index.ts (bucket-root barrel) into topic files per the shared-layer governance (buckets have no index.ts), then route each split-out topic by its actual consumer process: dataUrl.ts stays in @shared (cross-process: main + renderer); http.ts and jsonc.ts move to src/main/utils (main-only, generic); env redaction folds into the CodeCli service (main-only, consumer-specific).
Carve CodeCliService into a src/main/services/codeCli/ service directory (mirrors webSearch/): move CodeCliService.ts, terminals.ts, and tests in, add an index.ts barrel, and house the env-redaction helper as envRedaction.ts. Rename the mislabelled CodeToolsService.test.ts to codeCliService.helpers.test.ts and merge the redaction tests into it. Rewrite all consumer imports accordingly; no behavior change.
Replace the standalone HOME_CHERRY_DIR constant with central application.getPath() lookups across OvmsManager, OvOcrService, CodeCliService, and the rtk/process utils. This removes the duplicate of CHERRY_HOME_DIRNAME (core/paths/constants.ts) left over from the v1 to v2 migration and routes every ~/.cherrystudio path through the registry. Register feature.cli.install_global for the bun global install dir.
Fix getBinaryPath to gate its system-PATH fallback on the managed binary's existence rather than the bin directory's. The bin dir is created early by unrelated bootstrap flows (rtk extraction, CLI install), so the old dir-existence check returned a non-existent absolute path whenever a binary was absent, breaking spawn callers instead of falling back to PATH. Add cross-platform tests for getBinaryPath, which was previously uncovered.
The generator template emitted `export const languages`, but the data file and its three consumers expect `codeLanguages` (diverged at #12803 when the linguist data file was forked and renamed without updating the generator). Emit the matching name so `pnpm update:languages` is usable again.
Also refresh stale "languages.ts" references in comments and logs to the current "code-languages.ts" filename.
@shared/utils/serialize.ts (safeSerialize, consumed by both main's McpRuntimeService and the renderer) reached the runtime isSerializable guard through the @types alias, which resolves to the renderer types barrel — a shared->renderer layering inversion that pulled renderer code into the main bundle. Point it at @shared/utils/serializable instead, the canonical home for the guard and SerializableSchema.
Delete the byte-identical renderer copy src/renderer/types/serialize.ts and its barrel re-export, and repoint renderer/types/error.ts at the single @shared/types/serializable type so Serializable has one definition. Move the guard's test alongside the canonical module. No runtime behavior change.
Dissolve the by-kind @shared/config junk drawer per shared-layer governance: route each member by shape and actual consumer process — cross-process slices into types//utils//ai/, single-process code back into main/renderer (Invariant 1.1). Confirm each item's real consumer process rather than trusting the directional plan (API_SERVER_DEFAULTS is renderer-only, MIN_WINDOW_* is cross-process, providers.ts is renderer-only), and drop dead consts (ZOOM_LEVELS/ZOOM_OPTIONS, bookExts, thirdPartyApplicationExts).
Purge runtime logic from types/ so the bucket holds only declarations: move serializeError + AI-SDK error guards to utils/error.ts, the FileHandle factories/guards to utils/file/handle.ts, isSerializable + SerializableSchema to utils/serializable.ts, and the tab-instance guard/normalizer to utils/tabInstanceMetadata.ts. Tests follow the logic to utils/__tests__; the type-level ipc contract test is retired (its invariants kept as a breadcrumb for the future IpcApi Zod schema). types/ is now logic-free and test-free.
Also: remove the dead @shared mock from the packages/ui code-editor test so packages/ui no longer references production code; fix the data-classify preference generator prompts import and the update-languages output path to the relocated modules.
Update shared-layer-architecture (3.1 type/util test rule, 5/6 config dissolution) and renderer-architecture cross-references.
<!-- Template from
https://github.com/kubevirt/kubevirt/blob/main/.github/PULL_REQUEST_TEMPLATE.md?-->
<!-- Thanks for sending a pull request! Here are some tips for you:
1. Consider creating this PR as draft:
https://github.com/CherryHQ/cherry-studio/blob/main/CONTRIBUTING.md
-->
> ### 🚨 Branch strategy — read before opening this PR
>
> The v2 refactor has merged into `main`, so **`main` is the default
branch for active development** (v1 and v2 code currently coexist there
— expect large, breaking changes).
>
> - **Active development** (features, refactors, optimizations, fixes
for the current codebase) → target **`main`** (the default base).
> - **v1 maintenance** (hotfixes and subsequent v1 releases) → branch
from and target **`v1`**, _not_ `main`.
>
> A v1 fix does **not** auto-carry to `main`: if the same bug exists on
`main`, open a separate forward-port PR targeting `main`. Before
touching subsystems being replaced, read `docs/references/data/` and
watch for `@deprecated` markers — they flag code being deleted.
### What this PR does
Before this PR:
Developers could install dependencies with Node.js 24.16.0 or newer even
though those versions can trigger incomplete Electron binary extraction
through the Electron installer dependency chain.
After this PR:
The project declares `>=24.11.1 <24.16.0` as the supported Node.js range
and enables `engine-strict=true` so pnpm fails early when the active
Node.js runtime is outside that range.
<!-- (optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)`
format, will close the issue(s) when PR gets merged)*: -->
Fixes #N/A
### Why we need it and why it was done in this way
The following tradeoffs were made:
This pins the supported Node.js 24 range below 24.16.0 instead of
changing Electron, `extract-zip`, or transitive dependency resolution.
That keeps the change small and focused on protecting local installs
until the upstream extractor issue is resolved in the Electron
dependency chain.
The following alternatives were considered:
- Overriding `yauzl` to a newer version was considered, but that changes
transitive dependency behavior and is broader than an engine guard.
- Manually extracting Electron with system `unzip` works as a local
workaround, but it is not suitable as a project setup requirement.
- Upgrading Electron may eventually be the right long-term fix, but this
PR only prevents known-bad Node runtimes from producing broken installs.
Links to places where the discussion took place: <!-- optional: slack,
other GH issue, mailinglist, ... -->
- https://github.com/electron/electron/issues/51619
-
https://community.getmailspring.com/t/building-is-broken-with-node-js-26-1-0-and-24-16-0/14475
- https://github.com/nodejs/node/issues/63487
### Breaking changes
<!-- optional -->
Developers using Node.js 24.16.0 or newer must switch to a supported
Node.js version before running pnpm install. This affects development
tooling only and does not change runtime behavior for app users.
### Special notes for your reviewer
This is a development environment guard for an upstream Node.js/Electron
installer incompatibility. It intentionally does not change application
code.
### Checklist
This checklist is not enforcing, but it's a reminder of items that could
be relevant to every PR.
Approvers are expected to review this list.
- [x] Branch: This PR targets the correct branch — `main` for active
development, `v1` for v1 maintenance fixes
- [x] PR: The PR description is expressive enough and will help future
contributors
- [x] Code: [Write code that humans can
understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans)
and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle)
- [x] Refactor: You have [left the code cleaner than you found it (Boy
Scout
Rule)](https://learning.oreilly.com/library/view/97-things-every/9780596809515/ch08.html)
- [x] Upgrade: Impact of this change on upgrade flows was considered and
addressed if required
- [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com)
was considered and is present (link) or not required. Check this only
when the PR introduces or changes a user-facing feature or behavior.
- [ ] Self-review: I have reviewed my own code (e.g., via
[`/gh-pr-review`](/.claude/skills/gh-pr-review/SKILL.md), `gh pr diff`,
or GitHub UI) before requesting review from others
### Release note
<!-- Write your release note:
1. Enter your extended release note in the below block. If the PR
requires additional action from users switching to the new release,
include the string "action required".
2. If no release note is required, just write "NONE".
3. Only include user-facing changes (new features, bug fixes visible to
users, UI changes, behavior changes). For CI, maintenance, internal
refactoring, build tooling, or other non-user-facing work, write "NONE".
-->
```release-note
NONE
```
Move the four ad-hoc top-level @shared dirs into the closed top-level set, routed by shape: pure logic + class blueprints to utils/, type/contract declarations to types/.
command -> utils/command + types/command; file -> utils/file + types/file; shortcuts -> utils/shortcut + types/shortcut; externalApp -> utils/externalApp + types/externalApp.
Replace the exported menuRegistry singleton with a pure resolveMenu over a frozen contribution set (Invariant 1.2: no exported instance); keep MenuRegistry as a per-process blueprint sharing the same resolve algorithm.
Rewrite all consumer imports per symbol origin and align shared-layer-architecture and renderer-architecture docs.
Add docs/references/shared-layer-architecture.md as the authoritative reference for src/shared: two invariants (cross-process; no mutable runtime state), the closed top-level set {ai, data, ipc, types, utils} by category, types/utils shape rules (single-file vs subdirectory, barrels, constants guardrail), the placement decision, anti-patterns, and a deferred migration section (config dissolution with constant.ts itemized).
Relocate @shared-internal rules out of renderer-architecture.md and architecture-overview.md into the new doc (leaving pointers); repoint command's cross-process cell to @shared/utils/command + @shared/types/command in renderer-architecture sections 6 and 8; link the per-root applications of the closed-top-level rule from naming-conventions section 4.8.
- Emphasize cross-process as the entry gate for @shared; single-process logic stays in its own layer (plain shared types excepted). Mirror as a summary in architecture-overview.
- Refine the command-capability example to decompose by shape across @shared/command, utils/command, hooks/command, and components/command; update the §8 alignment/coupling notes for the resolved component/hook → feature edges.
The command capability was a cross-cutting capability mis-modeled as a domain feature, leaving component/hook -> feature reverse edges. Decompose it by shape into the shared type buckets: React contexts + hooks -> src/renderer/hooks/command/; React components -> src/renderer/components/command/ (CommandProvider, CommandContextKeyProvider, CommandMenus, CommandControls); renderer-only pure helpers (shortcut label, KeyboardEvent -> binding) plus a new resolveCommandDisplayState -> src/renderer/utils/command.ts; @shared/command keeps only cross-process resolution logic.
Dedupe the per-command display computation into resolveCommandDisplayState (used by useResolvedCommand and useResolvedCommandMenu), drop the private useCommandShortcutLabel, and route CommandShortcut through useResolvedCommand.
Rename useAllShortcuts -> useCommandShortcuts, ContextKeyProvider -> CommandContextKeyProvider, menus -> CommandMenus, presentation -> CommandControls, and repoint all consumers by symbol kind. Remove the now-empty src/renderer/features/.
Add docs/references/renderer-architecture.md as the canonical reference for src/renderer organization: the two-axis model (type x domain), four-layer downward dependency direction, per-directory responsibilities, the features/ definition, curated-barrel public API with import/no-restricted-paths enforcement, closed top-level governance, anti-patterns, a target-vs-current-state section, and industry references.
Align the existing docs: collapse the renderer subtree in architecture-overview.md to a pointer, and in naming-conventions.md list renderer services/ in the type-bucket set and link features/ placement to the new doc.
### What this PR does
Before this PR:
Deleting a global MCP server via the UI removed the row from
`mcp_server` and (via FK cascade) the `assistant_mcp_server` junction
table, but each agent stored its MCP servers as an `agent.mcps` JSON
`string[]` column with no foreign key. The deleted MCP's ID lingered in
that JSON array, so agents kept trying to connect to a non-existent MCP
during startup/heartbeat — generating thousands of "Failed to connect to
MCP" error logs daily.
After this PR:
The agent↔MCP relation is **normalized**. The `agent.mcps` JSON column
is dropped and replaced by an `agent_mcp_server` junction table with `ON
DELETE CASCADE` on both sides (mirroring the existing
`assistant_mcp_server` table). Deleting an MCP server — or an agent —
now cascades structurally instead of leaving dangling JSON references.
`McpServerService.delete()` removes the server inside a single
`withWriteTx` transaction; `AgentService.removeMcpFromAllAgentsTx()`
first captures the affected agent IDs and explicitly deletes their
junction rows (the FK cascade is a safety net), so that after the
transaction commits `AgentService.emitAgentUpdatedForIds()` fires
`onAgentUpdated` for each affected agent and lets warm sessions refresh
their tool policy live.
The v1→v2 migration backfills the new table: `migrateAgentMcps()` reads
the legacy `agents.mcps` JSON arrays, remaps the old MCP IDs to their
new UUIDs (via `McpServerMigrator`'s `mcpServerIdMapping`), drops
dangling references, and writes `agent_mcp_server` rows.
Fixes#15986
### Why we need it and why it was done in this way
`agent.mcps` stored MCP server IDs as an unconstrained JSON `string[]`,
so SQLite could not cascade-delete them and the data model allowed
agents to reference MCP servers that no longer existed. Normalizing the
relation into a junction table with FK `ON DELETE CASCADE` is the
root-cause fix: referential integrity is now enforced by the database,
deletes cascade structurally, and no application-side JSON rewriting is
needed on every MCP delete.
All agent↔MCP writes (create / update / delete) go through
`AgentService` inside `withWriteTx` and roll back atomically;
`onAgentUpdated` events fire only after commit so sessions never refresh
against an uncommitted state. Reads project the junction rows back into
the agent DTO's `mcps` field, so the public API shape is unchanged for
consumers.
The following alternatives were considered:
- **Keep the JSON column and filter it in JS / `json_remove()` on
delete**: leaves the unconstrained column (and the dangling-reference
class of bugs) in place, and requires rewriting every agent row on each
MCP delete. Rejected in favor of normalizing the relation, which fixes
the invariant at the schema level.
### Breaking changes
None for users. Internally, the `agent.mcps` column is dropped in favor
of the `agent_mcp_server` junction table; the v1→v2 migrator
(`migrateAgentMcps`) carries existing associations forward, remapping
legacy MCP IDs to their new UUIDs.
### Special notes for your reviewer
- `agent_mcp_server` mirrors `assistant_mcp_server` exactly (schema,
naming, FK cascade); the `assistant` side was already covered by FK
cascade and is unchanged.
- `AgentService.emitAgentUpdatedForIds()` uses a batch `inArray` query
(not N+1 `getAgent()` calls); events fire only after the write
transaction commits, and the emission is wrapped so a post-commit
refresh failure cannot misreport a successful delete.
- `removeMcpFromAllAgentsTx()` captures affected agent IDs before
deleting the junction rows so events can be emitted post-commit; the
MCP-server delete's FK cascade is a redundant safety net.
- v1→v2: `migrateAgentMcps()` runs while `agents_legacy` is attached and
before `remapAgentPrefixIds()`, following the same `mcpServerIdMapping`
remap + dangling-drop pattern as `AssistantMigrator`.
### Checklist
- [x] Branch: This PR targets the correct branch — `main` for active
development
- [x] PR: The PR description is expressive enough and will help future
contributors
- [x] Code: Write code that humans can understand and Keep it simple
- [x] Refactor: You have left the code cleaner than you found it (Boy
Scout Rule)
- [x] Upgrade: Impact of this change on upgrade flows was considered and
addressed if required
- [ ] Documentation: A user-guide update was considered and is present
(link) or not required. Check this only when the PR introduces or
changes a user-facing feature or behavior.
- [x] Self-review: I have reviewed my own code (e.g., via gh-pr-review,
gh pr diff, or GitHub UI) before requesting review from others
### Release note
```release-note
Bug fix: Deleting a global MCP server now cascade-removes its references from all agents, preventing stale "Failed to connect to MCP" errors.
```
---------
Signed-off-by: suyao <sy20010504@gmail.com>
Co-authored-by: SuYao <sy20010504@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: fullex <106392080+0xfullex@users.noreply.github.com>
Add `src/shared/ipc/schemas/**/*.ts` to the `data-schema-key/valid-key`
rule's files glob so route/event keys for every current and future IPC
domain are enforced automatically, replacing the hard-coded file list that
silently missed each migrated domain (selection, window, knowledge,
fileProcessing, webSearch). Add a guard that skips zod data-field names —
keys inside a `z.*(...)` object literal — so only the route/event strings
are constrained while `Object.freeze(...)` registries stay validated.
Drop the stale "global registry" type assertions in schema.types.test.ts
that manually enumerated every migrated route/event union; that positive
contract is already proven at compile time by the production handler maps,
leaving the @ts-expect-error negative assertions as the test's real value.
Remove an unused eslint-disable directive surfaced by the linter.
The V2 migration gate's fallback labeled every migrate() failure as a 'database connectivity issue' and, in dev, only offered the reset-DB guidance for the unambiguous 'object already exists' case. A SQLITE_CONSTRAINT_* thrown from migrate() (e.g. existing rows violating a newly-added CHECK) fell through to that misleading message.
Make the fallback accurate and dev-aware: in dev it surfaces the raw error plus BOTH likely causes — incompatible local data (reset the DB at the shown path) OR a migration bug (fix it; do not blanket-delete, or the bug resurfaces for users with real data) — and in production it stays neutral and never tells a real user to delete data. Constraint failures during migration are ambiguous (incompatible legacy data vs migration bug), so they are deliberately NOT auto-classified as 'delete the DB'. The unambiguous schema-out-of-sync ('already exists') dev reset dialog is unchanged.
Add docs/references/data/database-construction.md as the single home for how the SQLite DB is built and evolved: boot init order, drizzle migrations (regenerate-never-rename, CI gates, additive-vs-rebuild), the CUSTOM_SQL_STATEMENTS every-boot replay (~0.1ms O(1)), and the FTS5 fts_rowid rowid-stability rule, plus a gotchas table. Move the Migrations and Custom SQL sections out of database-patterns.md into it (left as pointers), and index it from data/README.md and src/main/data/db/README.md.
Fix stale references found while consolidating: wrong generate command, customSql.ts vs customSqls.ts, columnHelpers.ts vs _columnHelpers.ts, a nonexistent messageFts.ts, yarn vs pnpm, the v2-todo single-0000 claim, the generated-column wording, and v1 data.blocks vocabulary in the testing doc.
message_fts and agent_session_message_fts keyed their FTS5 external-content index on SQLite's implicit rowid. A drizzle table rebuild (INSERT...SELECT drops the rowid) or VACUUM reshuffles it, silently desyncing the index (wrong/missing hits, no error; the default integrity-check does not detect it).
Add a real fts_rowid integer-unique column to each table, set content_rowid to it, and assign it in the AFTER INSERT trigger (MAX+1, O(log N) via the unique index, race-free under withWriteTx). A real column is carried verbatim through rebuilds and untouched by VACUUM, so the index stays aligned by construction. Update both search joins to base.fts_rowid = fts.rowid, convert the agent-session triggers to DROP+CREATE, and add a regression test reproducing a rowid-reshuffling rebuild that asserts integrity-check,1 stays clean (with a NULL fts_rowid negative control). Note the same latent hazard on knowledge search_text_fts (deferred; no rebuild/VACUUM path today).
