### 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>
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).
The boot reconcile of crash-orphaned pending assistant turns (findPendingAssistantMessageIds) full-scanned the message and agent_session_message tables. Add a plain status index on each so the lookup is a SEARCH, not a SCAN.
Plain, not partial: Drizzle binds status = ?, which SQLite cannot match against a partial (literal-predicate) index. Also select only id, since the reconcile just flips matched rows to error.
### What this PR does
Before this PR:
- `file_entry` table used `trashed_at` for the soft-delete timestamp,
diverging from every other soft-deletable table in the schema (`agent`,
`assistant`, `message`, `topic`), which all use `deleted_at`.
After this PR:
- `file_entry.deleted_at` (and BO field `deletedAt`) — naming is
consistent across the entire schema.
- Renamed identifiers:
- Schema field: `trashedAt` → `deletedAt`
- SQL column: `trashed_at` → `deleted_at`
- Index: `fe_trashed_at_idx` → `fe_deleted_at_idx`
- CHECK constraint: `fe_external_no_trash` → `fe_external_no_delete`
- Updated all source files, tests, and architecture docs (including
`v2-refactor-temp/docs/file-manager/`).
- **Intentionally NOT renamed** (out of scope — these are API surface /
concept names, not the column name): `moveToTrash`, `restoreFromTrash`,
`inTrash` (query flag), `isTrashed`, `batchTrash`, `internalTrash`, and
"Trash" as a concept in comments/docs.
Fixes #
### Why we need it and why it was done in this way
The following tradeoffs were made:
- **Scope discipline**: kept the rename strictly at the
column-identifier layer (4 identifiers). Did not change API names or
concept words — switching the "Trash" concept to "Delete" is a larger
semantic change that deserves its own PR.
- **Migration 0026 contains a manual SQL patch.**
drizzle-orm/drizzle-kit issue
[#3653](https://github.com/drizzle-team/drizzle-orm/issues/3653) causes
the SQLite rebuild-table path to drop the leading `ALTER TABLE … RENAME
COLUMN` statement. The generated `INSERT … SELECT "deleted_at" FROM
file_entry` would fail because the source table still has `trashed_at`.
The migration manually prepends an explicit `ALTER TABLE file_entry
RENAME COLUMN trashed_at TO deleted_at;` before the rebuild. Upstream
fix landed in `drizzle-kit@1.0.0-beta`/`rc` but is not backported to the
`0.31.x` stable line we depend on.
- **Why keeping the manual patch is acceptable**: per `CLAUDE.md` § v2
Refactoring, `migrations/sqlite-drizzle/` is throwaway during v2 — it
will be wiped and regenerated as a single clean initial migration from
the final schemas before release. Mid-development DB drift is explicitly
acceptable, and the manual SQL only needs to survive until that
regeneration.
The following alternatives were considered:
- Selecting `create column` in `drizzle-kit generate` instead of `rename
column`: also produces invalid SQL (same root cause — the rebuild path
puts the new column name in the `SELECT` list regardless of the rename
mapping). Rejected.
- Skipping the `0026` migration entirely and relying on `db:push` / DB
reset during dev: pollutes `_journal.json` divergence and makes the next
schema change confusing. Rejected.
- Upgrading to `drizzle-kit@1.0.0-beta`/`rc` to get the fix: v1 is a
major rewrite with significant breaking changes (alternation engine
rewrite, ORM type system rewrite, migration folder layout change). Out
of scope for this PR. Rejected.
Links to places where the discussion took place: N/A
### Breaking changes
None. Dev-only DB column rename during v2 refactor. No user-visible
behavior change. No public API surface change. v1 data never reaches
this branch except through migrators in `src/main/data/migration/v2/`.
### Special notes for your reviewer
- The single manual edit to drizzle-generated SQL is in
`migrations/sqlite-drizzle/0026_sturdy_aqueduct.sql` — look for the
`MANUAL PATCH` comment block at the top. Without it the migration will
fail to apply.
- "Trash" concept words still appear throughout the file-manager
codebase by design (function names, comments, docs section headings). If
we later want to migrate the whole concept to "Delete", that should be a
follow-up PR.
### 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] 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.
- [x] 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
```release-note
NONE
```
---------
Signed-off-by: icarus <eurfelux@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four backbone changes that close known gaps in the schedule subsystem before the next round of business handler work:
* DB-enforced singleton: drop the app-layer Mutex in JobScheduleService and let UNIQUE(type, name) enforce "one singleton per type" via a '' sentinel. rowToSnapshot maps '' back to null so the external snapshot contract stays `string | null`.
* updateJobSchedule public API: writes the DB row and re-arms the in-process cron entry when trigger or enabled changes. Field-presence check avoids JSON.stringify brittleness; the one-turn race is an accepted last-writer-wins limitation.
* Startup recovery gating: move runStartupRecovery + armSchedule from onReady to a new onAllReady behind a 60s delay so business services have a window to register handlers before recovery runs. onStop flips a stop flag for clean teardown; per-step try/catch so one failure does not zero the session.
* Schedule control API tests: cover pause/resume/triggerNow/unregister × by-id/by-name plus updateJobSchedule branch matrix; JobScheduleService gets its own unit suite for singleton/sentinel behavior.
Side effects: add JOB_SCHEDULE_NAME_INVALID code; fix pre-existing arg order on DataApiErrorFactory.conflict at unique handlers (message, then resource); tighten getByTypeAndName signature to (type, name: string); listNamesForType filters the singleton sentinel and returns string[]; sync existing integration/smoke fixtures to await _doAllReady() with fake timers; add "Schedule identity: (type, name) model" section in handler-authoring.md.
cherry-studio v2 had 6+ ad-hoc queue/scheduler implementations (Knowledge,
FileProcessing, agent SchedulerService, TopicQueue, NotificationQueue,
protocol heartbeats) with no shared registry, inconsistent cancel and
progress semantics, and no cross-restart recovery outside agent_task.
This commit lands the unified replacement: JobManager owns Job lifecycle
and SchedulerService owns time-only scheduling, both reusable independently.
Phase 1 ships the backbone only: jobTable + jobScheduleTable, entity
services, 6-state machine with per-handler recovery (abandon/retry/
singleton), catch-up policy (skip-missed/after-startup), retry backoff,
GC, idempotencyKey dedup, useJob renderer hook, and 4 reference docs.
Four-layer lock model addresses libsql client-ts issue #288 (Layer 0
global dispatch mutex serializes all transactions).
croner@^10 is introduced for cron expressions (zero-dep, Electron-friendly).
Application.SHUTDOWN_TIMEOUT_MS is promoted to public for JobManager reuse.
cron-parser stays until Phase 2 agent task migration.
No business migrations in this commit — Knowledge/FileProcessing/agent
SchedulerService remain untouched and migrate in Phase 2-4 per
docs/references/job-and-scheduler/migration-checklist.md.
Follow-ups for Phase 1 completion: DataApi GET /jobs handler, dummy.echo
smoke test, integration tests, migration feasibility report.
Boolean columns without .notNull() infer as boolean | null, forcing every
reader into the row.x ?? default fabricated-fallback pattern that R3
forbids. Sweep all schemas and pair .notNull() with the existing default
on columns whose NULL carries no domain meaning:
- user_provider.isEnabled (NOT NULL DEFAULT true)
- user_model.{isEnabled, isHidden, isDeprecated} (NOT NULL with existing
defaults)
- mini_app.bordered (NOT NULL DEFAULT true)
Booleans kept nullable on purpose, because NULL carries a real third-state
meaning: mcp_server.{longRunning, shouldConfig, isTrusted} and
user_model.supportsStreaming.
Drop the now-dead fallbacks:
- ProviderService.rowToRuntimeProvider: row.isEnabled ?? true
- ModelService.rowToRuntimeModel: row.isEnabled ?? true, row.isHidden ?? false
- modelMerger.mergeModelWithUser: userModel.isEnabled ?? !(catalogOverride?.disabled ?? false)
and userModel.isHidden ?? false
The merger change is a design adjustment, not just a refactor:
user_model.isEnabled no longer falls back to catalogOverride.disabled.
Each model row now carries its own authoritative isEnabled, and the
create path (mergePresetModel + mergedModelToNewUserModel) writes the
catalog-derived value at INSERT time.
Tighten modelMerger.UserModelRowSchema to require these fields; use
satisfies NewUserModel in dtoToNewUserModel so the constructed object
type narrows to a concrete shape without a manual intersection patch.
Regenerate drizzle migration; tables rebuilt in-place under v2 throwaway
policy.
### What this PR does
Migrates the MiniApp feature from v1 (Redux + sidecar
`custom-minapps.json`) to the v2 data architecture (DataApi + Preference
+ Cache), and integrates it into the v2 AppShell tab system.
**Before this PR**
- App lists lived in three Redux arrays (`enabled` / `disabled` /
`pinned`); custom-app logos were stripped before persistence and
recovered at runtime from `{userData}/Data/Files/custom-minapps.json`.
- Settings (`region`, `max_keep_alive`, `open_link_external`,
`show_opened_in_sidebar`) lived in legacy redux/electron-store.
- Runtime keep-alive used a module-level `lru-cache` singleton, mirrored
into v2 cache via `onInsert` / `disposeAfter` (two sources of truth —
already a known race).
- Routes were `/app/minapp/*`; sidebar icon literal was `'minapp'`.
- Sidebar mode used the legacy popup container; top-navbar mode was
non-functional.
**After this PR**
- A single `mini_app` SQLite table owns every row (preset + custom).
Preset rows are seeded by `MiniAppSeeder` from `PRESETS_MINI_APPS` on
every boot; custom rows come in via `POST /mini-apps`. The seeder uses
`setWhere isNotNull(presetMiniappId)` so refreshing preset display
fields can never overwrite a custom row whose `appId` happens to collide
with a preset.
- `MiniAppMigrator` imports v1 Redux state and reads
`custom-minapps.json` (path resolved through
`MigrationPaths.customMiniAppsFile`) to recover stripped logos.
- Settings live under typed Preference keys
(`feature.mini_app.{region,max_keep_alive,open_link_external}`); sidebar
icon literal renamed `'minapp'` → `'mini_app'` with a complex preference
transform that rewrites existing user arrays in-place.
- API: `GET/POST/PATCH/DELETE /mini-apps` + `POST
/mini-apps/order:batch`, Zod-validated, fractional-indexing ordering
scoped by `status` (cross-status batches are rejected with
`VALIDATION_ERROR` per the data-ordering-guide contract). Status
transitions reassign `orderKey` to the tail of the target partition
inside a transaction.
- Renderer hook `useMiniApps` exposes **command-style** writes only:
`updateAppStatus(id, status)` and `setAppStatusBulk([{id, status}])`.
The legacy declarative `updateMiniApps(list)` /
`updateDisabledMiniApps(list)` / `updatePinnedMiniApps(list)` are gone —
they took region-filtered subsets and silently disabled rows the caller
never saw.
- Keep-alive list is stored solely in
`useCache('mini_app.opened_keep_alive')`. Cap eviction respects AppShell
pin status: `useMiniAppPopup` reads pinned mini-app routes from
`useTabs` and skips them in eviction. `MiniAppTabsPool` renders webviews
in a stable `appId`-sorted order so LRU reorders never move `<webview>`
DOM nodes (Electron `<webview>` loses its guest WebContents on
detach/reattach).
- **Unified launch path**: clicking any miniapp (from the launcher grid
or a top tab bar entry) calls `openTab('/app/mini-app/<id>', { title,
icon: app.logo })`. A globally-mounted `<MiniAppTabsPool>` in `AppShell`
keeps a `<webview>` alive per opened app, regardless of sidebar vs
top-navbar layout.
- Settings UI rewritten as a `PageSidePanel` drawer composed of
`MiniAppListPair` (visible / hidden columns with drag-drop) and
`MiniAppDisplaySettings` (region / cache slider). New custom-app form is
a separate `NewMiniAppPanel` drawer.
- Sidebar's running-mini-apps strip removed — opened apps live
exclusively in the top tab bar (per #3198804265). Companion preference
`feature.mini_app.show_opened_in_sidebar` deleted from the schema.
### Why we need it and why it was done in this way
Part of the broader v2 data-layer migration (Redux/Dexie/ElectronStore →
DataApi + Preference + Cache).
**Architecture**
- DataApi for entity rows (preserves user content); Preference for
atomic settings; Cache (Memory tier) for runtime ephemera.
- Layered preset pattern (`best-practice-layered-preset-pattern.md`):
preset and custom rows share the same table, discriminated by
`presetMiniappId`. Seeder refreshes preset display fields on re-run;
custom rows are immutable to the seeder.
- Region filtering is a **view-only** concern (read path); the write
path is command-style and never references region. This eliminated a
class of bugs where editing the visible (filtered) list caused
region-hidden rows to drift.
- AppShell tab pinning is the canonical "keep this loaded" signal. The
keep-alive cap respects it; pinned mini-app tabs never get evicted
regardless of cap. Render-order independence in `MiniAppTabsPool`
ensures LRU touches don't move `<webview>` nodes around.
- Per-app icon resolution: `app.logo` is a `CompoundIcon` id (e.g.
`"Moonshot"`) for presets and a URL for custom apps. UI consumers (tab
bar, sidebar entry, settings list) call `getMiniAppsLogo` to resolve the
id to a `CompoundIcon` before rendering, with `<img>` fallback for URL
strings.
- Per-entity tab icons are cleared on internal navigation, sidebar
reuse, and the top-bar settings button — three call sites that all flip
the active tab's URL now consistently reset `icon: undefined` so a
mini-app logo never sticks onto an unrelated route.
**Tradeoffs**
- `useMiniApps` still exposes `miniapps` (region-filtered
enabled+pinned) and `disabled` (region-filtered). These are display-only
views. Renamed/typed wrappers were considered but deferred — the
refactor to command-style writes already eliminated the bug class that
motivated the rename.
- The `applyReorderedList` integration test for
`reorderMiniAppsByStatus` was dropped — `MockUseDataApiUtils` doesn't
fill the SWR cache that `useReorder.readCurrent` reads. Splice logic is
straightforward and the server-side `applyScopedMoves` test covers the
contract.
- Sidebar primitives in `@cherrystudio/ui`-adjacent layout still accept
`miniAppTabs` / `onMiniAppTabClick` props (defensive defaults — render
nothing without a producer). Removing these from the primitive's API is
a separate refactor not in scope.
### Breaking changes
User-visible changes are auto-migrated by the v2 migration framework —
no manual user action required:
- Sidebar icon literal `'minapp'` → `'mini_app'` (rewritten by the
`sidebar_icons_rename` complex preference transform)
- Preference key rename `feature.minapp.*` → `feature.mini_app.*`
(auto-migrated via `classification.json`)
- Custom-app logos stripped from v1 Redux are recovered from
`custom-minapps.json` during migration
One product-shape change is documented under
`v2-refactor-temp/docs/breaking-changes/`:
- `2026-05-07-miniapp-sidebar-running-list-removed.md` — the sidebar no
longer surfaces opened mini-apps under the mini-app entry. Open apps are
accessed exclusively via the top tab bar; pin a tab to keep its state
across switches.
The legacy v1 preference `showOpenedMinappsInSidebar` is reclassified as
`status: deleted` in the migration pipeline; v1 values are dropped
during v1→v2 migration with no v2 destination.
### Special notes for your reviewer
**Verified end-to-end on a real dev profile**: v1 Redux state +
`custom-minapps.json` → v2 SQLite, including pinned-app cross-group
dedup (a v1 pinned app appears in both `pinned` and `enabled` Redux
arrays; the migrator counts duplicates as skipped so the engine's
`targetCount >= sourceCount - skippedCount` invariant holds — without
this, any user with pinned miniapps was blocked from migrating).
**Drizzle migrations** are throwaway in dev per `CLAUDE.md`.
`migrations/sqlite-drizzle/0020_even_hulk.sql` is the single regenerated
migration; it will be wiped to a clean initial migration before release.
**Review history**: 28 line-comments across multiple formal review
rounds. All resolved. The most consequential fixes:
- `applyScopedMoves` in `MiniAppService.reorder` — rejects cross-status
batches with `VALIDATION_ERROR` instead of silently splitting them.
- `update()` reassigns `orderKey` to a fresh tail in the target
partition on status change.
- Empty-string substitution in migrator mappings is now caught by the
post-transform validity check; bad rows are skipped + warned, never
inserted.
- Migrator validation switched from `limit(5)` sample to full `count(*)
WHERE empty-fields` — bad rows can no longer pass validation by virtue
of being beyond the sample window.
- Keep-alive cap exempts pinned tabs (#3198809321 + the kangfenmao
keepalive review); render order in `MiniAppTabsPool` is `appId`-stable
so LRU touches don't move `<webview>` nodes (this was the root cause of
"switching tabs reloads the webview").
**Out of scope**:
- The remaining `@renderer/store/tabs` import in
`PaintingsRoutePage.tsx` is pre-existing v1 residual (not introduced or
touched by this PR).
### Checklist
- [x] PR: description rewritten to reflect the final architecture +
integration with the AppShell tab system
- [x] Code: command-style writes (`updateAppStatus` /
`setAppStatusBulk`); see `useMiniApps`, `MiniAppService`,
`MiniAppMigrator`, `MiniAppTabsPool`, `useMiniAppPopup` for the main
entry points
- [x] Refactor: ~1500 lines of dead/legacy code removed
(`Tab/TabContainer`, `TabsService`, `MiniAppPopupContainer`,
`TopViewMiniAppContainer`, legacy LRU singleton, `PinnedMiniApps`, dead
`userOverrides` / `MiniAppRegistryService`, unused `Signal.ts`)
- [x] Upgrade: v1 → v2 migration verified end-to-end on a real dev
instance
- [x] Documentation: architecture covered by `docs/references/data/`;
one user-visible behavior change documented in
`v2-refactor-temp/docs/breaking-changes/`
- [x] Self-review: multi-agent review via `/gh-pr-review` (twice); all
28 review comments resolved
### Release note
```release-note
NONE - Internal v2 data refactor. User-facing renames (route, sidebar icon, preference keys) are auto-migrated. The sidebar no longer shows a running-mini-apps strip; opened apps live in the top tab bar.
```
---------
Signed-off-by: suyao <sy20010504@gmail.com>
Signed-off-by: chengcheng84 <hello_world0000@outlook.com>
Co-authored-by: suyao <sy20010504@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: fullex <106392080+0xfullex@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
Apply NOT NULL constraints where NULL has no domain meaning:
- agent / agentSession: description, instructions, mcps, allowedTools,
configuration, accessiblePaths, slashCommands (session only)
- agentGlobalSkill.tags
- message.searchableText, message.siblingsGroupId
- topic.name, topic.{isNameManuallyEdited, sortOrder, isPinned, pinnedOrder}
- miniapp.sortOrder
Drop rowMapper "?? fallback" patterns; preserve genuine T|null contracts
(agentSessionMessage.agentSessionId now passes NULL through, with the Zod
entity tightened to .nullable() to match).
Migrate product-chosen DB DEFAULTs to the service layer:
- agentTask.status DB DEFAULT removed; service was already supplying 'active'
- agentGlobalSkill.isEnabled DB DEFAULT flipped from true to false to match
SkillService.install behavior
Drop Zod .default([]) from CreateAgentSchema.accessiblePaths so the
service-layer computeWorkspacePaths() is the single runtime default source.
Update FTS5 triggers to COALESCE the group_concat result to '' so
messages with no main_text blocks don't violate the new NOT NULL on
searchable_text.
Refs: docs/references/data/best-practice-default-values-and-nullability.md
Two sibling resources land together because their migration SQL is
generated as one unit and their API-schema / handler wiring has to go
in the same commit to keep the repo compiling.
group: upgrade the existing group table from `sort_order INT` to
`order_key TEXT` (`scopedOrderKeyIndex('group', 'entityType')`) and
expose it as a first-class resource under /groups. Each entityType
owns an independent orderKey sequence. Reorder delegates to the new
applyScopedMoves helper.
pin: brand-new polymorphic table `(id, entityType, entityId, orderKey,
timestamps)` with UNIQUE(entityType, entityId) enforcing idempotency
at the DB layer. One table serves arbitrarily many consumers — same
precedent as entity_tag. No FK to consumer tables; every consumer
service's delete path must call `pinService.purgeForEntity(tx,
entityType, entityId)` to keep the two tables in sync (signature is
tx-first, the mainstream ORM convention; tagService.removeEntityTags
is the project's historical tx-last outlier and is left as-is).
PinService.pin is idempotent and concurrent-safe: a fast-path SELECT
returns existing rows, and a UNIQUE collision under concurrent INSERT
is caught, classified, and re-SELECTed so the caller never sees a
constraint error.
Handler layer is a thin Zod-parse shell — all scope inference, row
lookup, and orderKey computation live in the services.
The previous glob `./src/main/data/db/schemas/*` matched the
`__tests__` directory, and drizzle-kit's `prepareFilenames` expands any
matched directory by reading its immediate files regardless of
extension. That pulled `_columnHelpers.test.ts` into the schema load
path, and its `import ... from 'vitest'` blew up because drizzle-kit
loads schemas via CJS `require()` (vitest is ESM-only).
Switch to a recursive pattern that also excludes `*.test.ts` by name,
so the config tolerates future subdirectory-organised schemas without
re-introducing the same class of bug.
Add `.notNull()` to `createdAt` / `updatedAt` in the shared
`createUpdateTimestamps` helper so Drizzle `$inferSelect` produces
`number` instead of the misleading `number | null`. `deletedAt` in
`createUpdateDeleteTimestamps` stays nullable (soft-delete semantics).
Generated migration 0013 rebuilds 26 affected tables via the standard
SQLite table-recreation pattern; FK / CHECK / INDEX constraints are
preserved across rebuild. No backfill is added (project is in the
development phase; null pre-existing rows are accepted as a "wipe DB"
signal rather than engineered around).
Fix upstream in `KnowledgeMappings.toTimestamp` so it returns a
`Date.now()` fallback instead of `undefined` — otherwise future Dexie
-> v2 migrator runs would try to insert undefined into NOT NULL
columns. Three test assertions updated from `undefined` to
`expect.any(Number)`.
Sweep 18 downstream call sites across 9 functions that were carrying a
dead `?? new Date().toISOString()` fallback:
- AssistantService, KnowledgeBaseService, KnowledgeItemService,
MessageService, TopicService, TranslateHistoryService,
TranslateLanguageService (the original Pattern A set)
- McpServerService and MiniAppService.rowToMiniApp (reclassified from
Pattern B: the domain types stay `optional` to accommodate builtin
literals in the renderer, but `string` assigns legally into
`string | undefined`, so the switch is safe)
Keep `MiniAppService.builtinToMiniApp` on `timestampToISOOrUndefined`
— its `dbRow?: MiniAppSelect` semantics ("the preference row may not
exist at all") is genuinely optional, not a disguised "nullable column".
Also remove a Pattern C that neither the plan nor the grep audit
caught: `TagService.ensureTagTimestamp` was a self-rolled defense
layer that threw INTERNAL_SERVER_ERROR on null timestamps. The DB
now refuses to produce such rows, so the defense — and the test
named "should surface timestamp anomalies instead of masking them" —
are dead code. Removed both.
Update three docs to reflect the new defaults:
- `services/utils/README.md` — drop the "DB still nullable" table row
and the predictive paragraph; reframe Pattern B around
"whole row may not exist"
- `services/utils/rowMappers.ts` JSDoc — same reframing
- `docs/references/data/data-api-in-main.md` — delete the fallback
code samples and simplify Convention §3
### What this PR does
Before this PR:
- v2 migration did not include provider/model data migration from legacy
`llm` state.
- Provider/model data APIs and handlers were incomplete.
- `@cherrystudio/provider-registry` (formerly provider-catalog) package
was not integrated into the data layer.
After this PR:
- Add provider/model migration path (`ProviderModelMigrator` + mappings)
and register it in v2 migrator flow.
- Add `@cherrystudio/provider-registry` package with JSON-based registry
data, Zod-validated schemas, and lifecycle-managed
`ProviderRegistryService`.
- Complete provider/model schemas, services, handlers, shared API
schemas/types, and model merger utility.
- Complete provider API endpoints (`registry-models`, `auth-config`,
`api-keys`) aligned with lifecycle DI patterns.
**Note:** This PR is intentionally scoped to backend/data-layer only.
Renderer consumer migration will be submitted in a separate PR to
maintain domain separation.
Fixes #
N/A
### Why we need it and why it was done in this way
The following tradeoffs were made:
- Kept migration and data API implementation within current v2
architecture (handler -> service -> db schema) instead of adding
temporary compatibility layers.
- Replaced protobuf toolchain with JSON + Zod validation for simpler
data pipeline and better debuggability.
- Converted all numeric enums to string-valued `as-const` objects
(EndpointType, ModelCapability, Modality, etc.) for runtime
debuggability.
- Unified separate `baseUrls`, `modelsApiUrls`, `reasoningFormatTypes`
fields into a single `endpointConfigs` map keyed by EndpointType.
The following alternatives were considered:
- Keep protobuf-based registry data; rejected due to complexity of proto
toolchain and poor debuggability of binary data.
- Include renderer consumer migration in same PR; deferred to separate
PR for cleaner domain boundaries.
Links to places where the discussion took place:
- Original combined PR: #14034
### Breaking changes
None.
### Special notes for your reviewer
- This is a backend-only extraction from #14034, which contained both
backend and renderer consumer code. The renderer migration will follow
in a separate PR.
- Please focus review on migration flow (`ProviderModelMigrator`),
provider/model service contracts, and the registry package design.
- The `@cherrystudio/provider-registry` package was renamed from
`provider-catalog` and uses JSON data files instead of protobuf.
### Checklist
- [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)
- [ ] Upgrade: Impact of this change on upgrade flows was considered and
addressed if required
- [ ] Documentation: Not required (internal data layer, no user-facing
changes)
- [x] Self-review: I have reviewed my own code
### Release note
```release-note
NONE
```
---------
Signed-off-by: jidan745le <420511176@qq.com>
Signed-off-by: suyao <sy20010504@gmail.com>
Co-authored-by: suyao <sy20010504@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: fullex <106392080+0xfullex@users.noreply.github.com>
- Introduced a new method `runCustomMigrations` in `DbService` to execute custom SQL statements that Drizzle cannot manage, such as triggers and virtual tables.
- Updated `database-patterns.md` and `README.md` to document the handling of custom SQL and its importance in maintaining database integrity during migrations.
- Refactored `messageFts.ts` to define FTS5 virtual table and associated triggers as idempotent SQL statements for better migration management.
