Unify the three coexisting styles for $inferSelect/$inferInsert type exports (XxxSelect/XxxInsert and Xxx/NewXxx) onto the dominant XxxRow / InsertXxxRow form, and document it as the convention.
- schemas: assistant, group, mcpServer, note, miniApp, pin, tagging, painting, userModel, userProvider -> *Row / Insert*Row
- update all consumers (services, migrators, mappings, seeders, tests)
- docs: add naming-conventions.md section 5.3 and a pointer in src/main/data/db/README.md
The Row suffix is chosen over Drizzle's docs-style SelectXxx/InsertXxx to keep the DB-row type visibly distinct from the API XxxEntity type. Table names, columns, indexes, and migrations are unchanged - this is a type-name-only change.
Apply the naming-conventions §6.1 acronym-casing rule (MCP -> Mcp) to the
MCP* PascalCase identifier family across the codebase (McpServer, McpTool,
McpToolResponse, BuiltinMcpServerNames, McpService, etc.) plus the local
identifiers boundMcp/enableMcp/disableMcp and the didiMcp registry key.
Regenerate the OpenAPI spec from the renamed schemas.
Deliberately left unchanged (not naming-convention identifiers): persisted
field keys read by migrators (enabledMCPs), v1 Redux selectors (selectMCP),
string values (ExaMCP, logger labels), and UPPER_SNAKE constants (MCP_*).
Also fix naming issues in the data reference docs that prompted this:
- JSONStreamReader -> JsonStreamReader (match the real class name)
- rowToMCPServer -> rowToMcpServer (match the real function name)
- replace the TopicService getInstance() skeleton with a direct singleton
- sync stale MCPServer/MCPTool/McpService references in affected docs
The former v2 branch has merged into main, making the old "main is frozen,
submit to v2" guidance backwards. Update all contributor-facing entry points
to the current model: main is the active v2 development line; v1 maintenance
fixes target the v1 branch.
- CLAUDE.md: add a v2 "Current state" callout and a branch-targeting
Operational Rule (the AI-facing context).
- gh-create-pr skill: route v1 maintenance PRs to base v1.
- PR template: replace the stale hidden comment with a visible branch
callout and add a branch checklist item.
- CONTRIBUTING.md / docs/guides/contributing.md: rewrite the branch strategy
section to the new model.
- docs/guides/branching-strategy.md: add a current-model note and fix the PR
target guideline.
The V2 migration disabled foreign keys ad-hoc in each migrator, which was fragile (a
one-shot PRAGMA is lost at @libsql/client's post-transaction connection swap) and produced
recurring FK constraint bugs.
Register foreign_keys = OFF once via the patched setPragma() in MigrationDbService so it
replays on every connection, and remove the scattered per-migrator PRAGMA toggling. Each
migrator now self-checks its own fully-resolved tables via the new
BaseMigrator.assertOwnedForeignKeys() (scoped PRAGMA foreign_key_check per table), with the
engine's final verifyForeignKeys() as the cross-domain backstop.
- BaseMigrator: add assertOwnedForeignKeys() helper + real-DB tests
- MigrationDbService: db.run(PRAGMA OFF) -> client.setPragma(PRAGMA OFF)
- Assistant/Chat/Agents/Knowledge migrators: drop FK toggling, add scoped self-checks
- remapAgentPrefixIds: drop FK toggling + whole-db check; keep manual tx (ATTACH window);
export AGENT_TABLES for AgentsMigrator self-check
- pragmaReplay: add foreign_keys replay tests
- docs: update v2-migration-guide + migration README FK conventions
Move useWindowInitData out of the single-file src/renderer/core/ tree into the canonical src/renderer/hooks/ bucket and remove the now-empty core/ directory. The renderer core/ tree was pre-created to mirror src/main/core/ but only ever held this one cross-window hook, violating the naming convention against pre-creating subdirectories for anticipated growth.
Update all three import sites plus the doc and comment references (windows/README, preload comment, window-manager README and usage guide) to the new path.
Standardize all renderer windows on a three-layer convention: a thin
entryPoint.tsx that only bootstraps and mounts a providers-root XxxApp.tsx,
with window UI kept in semantically-named content components. Co-locating a
component with a top-level createRoot().render() breaks the React Fast
Refresh boundary (full reload on every edit); splitting restores hot-swap
and makes all 8 windows uniform.
- trace: rename traceWindow.tsx -> TraceApp.tsx, add thin entryPoint.tsx,
update index.html script src
- settings: extract SettingsApp.tsx (providers + router + fatal error);
entryPoint keeps the async preload/init-data bootstrap
- subWindow: extract SubWindowApp.tsx (providers + preloadAll + queryClient)
- selection/toolbar: extract SelectionToolbarApp.tsx
- selection/action: rename feature SelectionActionApp.tsx -> ActionWindow.tsx,
add providers-root SelectionActionApp.tsx mirroring quickAssistant
- document the convention in src/renderer/windows/README.md and cross-link it
from the window-manager reference
Also fix a Tailwind regression introduced by the earlier renderer flatten:
the @source directive in assets/styles/tailwind.css resolved one level above
the repo root after the file moved up a directory, so @cherrystudio/ui utility
classes were no longer scanned/generated. Correct ../../../../../ to
../../../../, restoring ~211 UI-library utility classes.
Move all renderer source from src/renderer/src/* up one level to
src/renderer/*, removing the redundant nested src directory.
- Update path aliases (@renderer, @types, @logger, @data) and TanStack
Router paths in electron.vite.config.ts; update tsconfig.{json,web,node}
path mappings and include globs.
- Fix Vite root-relative script paths in the 8 renderer HTML entries.
- Update cross-process relative imports in main/preload (language,
apiServer models, preload index) to drop the /src segment.
- Switch renderer test imports of the logger mock to the @test-mocks alias.
- Update hardcoded renderer paths in scripts and their fixtures, lint
configs (eslint/oxlint/biome), CODEOWNERS, docs, and the data-classify tool.
- Convert deep (../../+) relative imports within the renderer to the
@renderer alias (69 files, 108 imports); keep single-level relatives.
- Fix doc links broken by the move and correct one pre-existing broken
link in naming-conventions.md.
packages/shared was never a real pnpm workspace package (no package.json); it was referenced only through the @shared TypeScript path alias. Relocate it under src/ via git mv (143 files, detected as pure renames).
Repoint the @shared alias and include globs to src/shared across electron.vite.config.ts, tsconfig.{json,node,web}.json and vitest.config.ts; update scripts/check-custom-exts.ts, scripts/update-languages.ts, the eslint.config.mjs generated-file globs, the data-classify generator output targets, .github/CODEOWNERS path rules, and CLAUDE.md/docs/source-comment references.
The @shared alias name is unchanged, so all 1403 @shared/* import sites resolve without modification. Verified with typecheck:node, typecheck:web and the full test suite (700 files, 9739 tests passing).
### What this PR does
Before this PR:
- Directory loading is split between two pipelines:
`FileStorage.getDirectoryStructure` (Notes) and
`FileStorage.listDirectory` (everything else). Same workspace, two
scans, two watchers.
- `FileStorage.listDirectory` defaults `maxEntries: 20`, silently
truncating list-mode callers (workspace trees rendered `tests/__mocks__`
as a file because it sorted out of the first 20 results).
- Notes' manual chokidar plumbing (`startFileWatcher` / `onFileChange`)
lives on legacy `file-change` IPC and crashes with `EMFILE: too many
open files` on large repos because the watcher opens one FD per
directory in `node_modules`.
After this PR:
- One primitive — `DirectoryTreeBuilder`
(`src/main/file/tree/builder.ts`, RFC §12) — owns the in-memory tree and
the chokidar watcher. It is the only directory-walking code on the main
side.
- Builder dedupe lives behind `TreeRegistry` (lifecycle `WhenReady`):
identical `(rootPath, options)` requests share one builder, with a 500ms
grace window so a remount inside one React commit reuses the warm scan +
watcher instead of paying for a rescan.
- `.gitignore` parsing (via `ignore@7`) drives BOTH ripgrep's
`--ignore-file` AND chokidar's `ignored` predicate, so EMFILE on
`node_modules`-heavy workspaces is gone. `.git` is always excluded even
when `.gitignore` doesn't list it.
- `search.listDirectory` is the now-real Phase 2 entry point (was a
stub). `maxEntries` default raised to `Number.MAX_SAFE_INTEGER`;
truncation is a search-mode concern and callers that want a cap pass it
explicitly. The 20-entry default was the bug.
- Renderer-side `useDirectoryTree(rootPath, options)` hook applies
mutation pushes to a JSON-mirrored `TreeDirRoot`. The tree DTO and
`TreeNode` class live in `packages/shared/file/types/tree.ts` so main
and renderer share one shape (no parent cycles in JSON; WeakMap parent
ref recovered in `FromJSON`).
- Notes (`NotesPage`) migrated off `getDirectoryStructure` + manual
watcher onto `useDirectoryTree` + `useNote` join. The Redux
`starredPaths` / `expandedPaths` path stays untouched in this PR —
`useNote` already covers that surface via `noteTable`.
Fixes #
### Why we need it and why it was done in this way
The following tradeoffs were made:
- **Builder dedupe on main, not renderer.** A renderer-side cache would
still pay one IPC round-trip per remount because the expensive thing is
the FS scan + watcher install, which only the main side owns. Sharing on
the main means one ripgrep + one chokidar regardless of how many panes
mount the same root.
- **500ms grace window for builder teardown.** Long enough to cover
React's "deletions before insertions" effect ordering (sub-millisecond
in practice), short enough that closing a workspace doesn't keep watcher
FDs alive noticeably.
- **`Tree_*` IPC owned by `TreeRegistry.onInit`.** The lifetime of the
handlers must match the lifetime of the chokidar watchers they
reference. Putting them in `FileManager` would leak handlers across
re-init.
- **`TreeNode` class hierarchy (not plain DTOs)** lets `rename` mutate
`path` once at the subtree root and have `adjustChildrenPaths` cascade —
the alternative is rebuilding the subtree, which throws away every
consumer's identity-based caching.
- **Hard ESLint isolation:** `src/main/file/tree/**` does not import
`@main/data/**`. Tree is a runtime concern; persistence is an orthogonal
one (`noteTable` is a sparse state overlay on top of FS paths, not a
tree mirror). RFC §12.6.
The following alternatives were considered:
- Keep the renderer-side cache (rejected — covered above; main-side
dedupe is strictly stronger).
- Add an `excludeGlobs` array option (rejected — the right source of
truth is `.gitignore`, and users editing `.gitignore` get free updates).
- One DTO without classes (rejected — rename cascade is the load-bearing
case).
Links to places where the discussion took place: RFC §12
(`v2-refactor-temp/docs/file-manager/rfc-file-manager.md`)
### Breaking changes
- Removed legacy IPC channels: `File_GetDirectoryStructure`,
`File_StartWatcher`, `File_StopWatcher`, `File_PauseWatcher`,
`File_ResumeWatcher`. Only Notes used them; the migration is in this PR.
No renderer outside Notes touched them.
- `search.listDirectory` (renamed from `FileStorage.listDirectory`) now
defaults to unbounded `maxEntries`. Existing callers that relied on the
implicit 20-cap (none I could find) would need to pass `maxEntries: 20`
explicitly.
### Special notes for your reviewer
- `src/main/file/tree/__tests__/builder.test.ts` covers initial scan,
`.gitignore` honoring, chokidar fan-out, `dispose` cleanup, and JSON
round-trip (no parent cycles).
- `src/main/file/tree/__tests__/registry.test.ts` covers builder dedupe,
grace-window reuse, multi-consumer mutation fan-out, and
`webContents`-destroyed cascade cleanup.
- `src/main/utils/file/__tests__/search.test.ts` locks in the
`maxEntries` default fix and the `--hidden` flag wiring.
- ArtifactPane / chat-page integration of this primitive lives on
`feat/chat-page` and is not part of this PR (that branch has the Shell
forceMount + chat-page Tree IPC integration on top).
### 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: A user-guide update was considered and is present
(link) or not required.
- [x] Self-review: I have reviewed my own code before requesting review
from others
### Release note
```release-note
NONE
```
---------
Signed-off-by: suyao <sy20010504@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add closed-form rule for stateful class suffix (Service / Manager) with
shape-based routing for non-stateful modules and cross-link to
lifecycle-decision-guide. Add file-vs-subdirectory organization (§4.4),
top-level directories closed by default (§4.8), bucket anti-patterns
(§6.7), and extend the acronym rule to filenames (§6.1).
Provide a decision rule for choosing the Service vs Manager class suffix: Service is the default capability provider, Manager is reserved for classes whose defining job is owning a pool/registry of homogeneous instances.
The rule applies to new classes; existing names are not renamed.
The per-queue dispatch gate counted active rows (pending+delayed+running) against the concurrency cap, so a queue whose backlog exceeded its cap blocked every claim forever — a permanent deadlock. Dispatch recursion only fires on a successful claim, the completion re-kick hit the same inflated count, and the delayed-promotion tick only dispatches when something was promoted. Count only running, matching the global gate.
Also fan out to all queues on completion when a dispatch was blocked solely by the global cap, fixing a cross-queue lost wakeup where a queue starved purely by the global cap waited until the next promotion tick.
Rename the concurrency counters to countRunningByQueueTx/countRunningGlobalTx and NON_TERMINAL_STATUSES to ACTIVE_STATUSES (matching existing knowledge-service convention), establishing a clear running (occupied slot) vs active (non-terminal) vocabulary; getStaleNonTerminal/getNonTerminalByType become getStaleActive/getActiveByType.
Add regression tests: a single queue with more jobs than its concurrency drains to completion; a globally-starved queue wakes when a global slot frees.
Document the app_state table as internal continuity markers: admission checklist, owner-held DB handle access (no dedicated service today), single-owner keys, scope-prefixed naming, disposability, and a key registry. Reference it from the data system-selection README and the seeding guide.
Add §6.6 noting that src/renderer/src/routes/ files are
kebab-case (TanStack Router file→URL mapping, per §2.3).
List TanStack's reserved tokens. Extend §1 Quick Reference
and §7 Decision Tree so the exception is discoverable.
### 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>
Migrate all JobManager state writes to the new serialized write channel so
they share a single libsql safety boundary, replacing the removed
JobManager.globalDispatchMutex.
Two-form DAO pattern in JobService / JobScheduleService:
- *Tx methods take a DbOrTx and compose inside withWriteTx.
- Non-Tx methods become thin wrappers — callsite shape unchanged so
consumers do not need to know about withWriteTx.
JobManager changes:
- Delete globalDispatchMutex; dispatch claim now runs inside withWriteTx,
with queue.mutex preserved as Layer 1 per-queue tick serialization
(lock acquisition order: Layer 1 first, Layer 0 via withWriteTx).
- All remaining writes (scheduleRetry, finalizeJob, patchMetadata, cancel,
cancelMany) route through withWriteTx.
- scheduleRetry now degrades to finalizeJob('failed', retryable=true) on
persistent non-BUSY tx failure. BUSY is already handled by withWriteTx;
this fallback defends against SQLITE_CORRUPT / FULL / driver bugs that
would otherwise leave a row stuck in 'running'. finalizeJob's existing
synthesizeFailedSnapshot still releases the in-memory slot when its own
tx fails.
- spawnExecute's fire-and-forget IIFE gets a two-stage .catch chain so
any leaked exception (logger errors, fallback finalize failures) cannot
surface as UnhandledPromiseRejection.
Stranded `running` rows from any double-failure path are reclaimed by
runStartupRecovery on the next process start.
Updates concurrency-and-locks.md to reflect Layer 0's new home in
DbService. Extends JobManager.integration.test.ts with two regression
tests: scheduleRetry fallback degrade and spawnExecute outer-catch
swallow.
libsql client-ts upstream issue #288 makes PRAGMA busy_timeout ineffective
for async transactions, so concurrent db.transaction() calls reliably surface
SQLITE_BUSY. Introduce DbService.withWriteTx as a serialized write helper:
- Process-wide FIFO mutex (async-mutex) serializes write transactions.
- libsql client's default BEGIN IMMEDIATE protects against read-then-write
tx upgrade failures (no override needed at the drizzle layer).
- Single 50ms BUSY retry guards against transient external locks.
Reads do NOT need this — WAL gives readers snapshot isolation that is never
blocked by writers.
Includes unit tests (FIFO ordering, finally release on throw, single BUSY
retry, persistent BUSY rethrow, non-BUSY passthrough) plus a real-libsql
integration test. Updates the DbService test mock with a passthrough
withWriteTx so dependent services do not throw "is not a function" in
tests. Documents the API in database-patterns.md and points
CLAUDE.md / data-api-overview.md at the new pattern.
`LifecycleManager.allReady()` was holding `await Promise.allSettled(_doAllReady)`,
making every `onAllReady` body a synchronous bootstrap dependency. This
contradicted the hook's JSDoc ("post-bootstrap supplement, not a critical
initialization gate") and gave any in-hook deferred work an oversize blast
radius — a 60 s wait inside one service stalled `ALL_SERVICES_READY` and
delayed bootstrap completion.
Align the implementation with the JSDoc:
- `allReady()` becomes `void`. It synchronously invokes every initialized
service's `_doAllReady()`, attaches an async `.catch` that re-emits
`SERVICE_ERROR`, then emits `ALL_SERVICES_READY` immediately.
- `Application.bootstrap()` drops its `await` on `allReady()`.
- `LifecycleManager` tests adjusted: drop redundant `await`s, rewrite
`resolves.toBeUndefined()` as `not.toThrow()`, drain microtasks before
asserting on the now-async `SERVICE_ERROR` emit, and add a test
exercising the fire-and-forget contract with a never-resolving hook.
`ALL_SERVICES_READY` now fires when hooks are *invoked*, not when they
complete. Docs reflect the contract change: a "Hook vs Event" comparison
in `lifecycle-overview.md`, two new Common Mistakes in
`lifecycle-decision-guide.md`, and an `onAllReady` business-work pattern
template in `lifecycle-usage.md` showing the \`setTimeout\` + signal +
\`onStop\` join model used by JobManager.
`onAllReady` is a post-bootstrap supplement, but JobManager was holding
a 60 s `await setTimeout` plus four IO steps inside it. The hook became
a 60 s blocking call and left `onStop` without a join point for the
deferred recovery.
Split the two concerns:
- `onAllReady` is now a synchronous `void` that schedules a `setTimeout`
(cleared via `registerDisposable`) and returns.
- The recovery body moves to a private `runStartupRecoveryFlow` whose
every IO step re-checks `_isShuttingDown`.
- A new `_recoveryDone: Promise<void> | undefined` lets `onStop` join
the flow before tearing down dependent resources.
- `_onAllReadyStopRequested` is renamed `_isShuttingDown` to reflect
its expanded role.
- `detectAndDispatchOverdue` gains a per-loop shutdown check so a slow
`onMissed` cannot starve `onStop`.
Test fixtures (smoke / schedule / integration) now access `_recoveryDone`
after fake-timer advancement instead of awaiting the lifecycle hook.
Docs synced: new "Startup Recovery" section in
`job-and-scheduler/overview.md`; "Registration timing" + recovery
internals + timeout-sentinel notes in `handler-authoring.md`; `once` /
`interval` trigger lifetime in `scheduler-usage.md`; lock acquisition
order in `concurrency-and-locks.md`. Module READMEs add pointers.
JobManager.onAllReady now walks distinct (queue, type) pairs for all
non-terminal rows and calls ensureQueue() for each. Previously
dispatchAll() iterated an empty this.queues Map on cold start, so rows
reset by runStartupRecovery stayed pending until the next enqueue
incidentally ensured the right queue. This bridges that gap so recovered
jobs dispatch on the same tick recovery finishes.
Additional changes bundled in:
- JobService.getDistinctActiveQueues: SQL groupBy(queue, type) over
pending/delayed/running, returned shape aligned with ensureQueue input
- Integration tests: retry/singleton cases extended to assert the full
recovery -> resurrect -> dispatch -> completed chain; new case for
resurrection of a pre-existing pending row; bootstrapManager drains
the dispatch chain under fake timers before switching back to real
timers so handler internal setTimeouts are not cancelled mid-flight
- useJob: add sibling useJobProgress hook (no DataApi fallback; reads
only the shared cache that JobManager publishes via reportProgress)
- handler-authoring docs: add organization convention (handlers live in
<module>/tasks/, file naming *JobHandler.ts)
- JobManager.makeError: replace repeated (err as Error & {...})
assignments with Object.assign for equivalent behaviour in one expr
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.
Documentation pass that wraps up Phase 1 by surfacing how to find and use
the Job/Scheduler system, plus bringing the public-API JSDoc in line with
the project convention (summary + @param/@returns/@throws tags).
- Add `src/main/core/job/README.md` and `src/main/core/scheduler/README.md`
as thin pointers into `docs/references/job-and-scheduler/`, matching the
Lifecycle/Application README style (file structure + Quick Links table,
no duplicated content).
- Add `docs/references/job-and-scheduler/scheduler-usage.md` documenting
the decision tree for `SchedulerService` vs `BaseService.registerInterval`
vs raw `setInterval`, the three judgment questions, common mistakes,
and reserved id prefixes used internally by JobManager.
- Tighten `overview.md` so the dispatch lock acquisition order matches the
code: Layer 1 (per-queue) first, then Layer 0 (global libsql tx). Make
the fixed order explicit to prevent future deadlock-introducing refactors.
- Upgrade public-API JSDoc on JobManager (registerHandler / enqueue / cancel
/ cancelMany / get / list / dispatch + full schedule registry: by-id and
by-name variants) with @param / @returns / @throws including error codes
(JOB_UNKNOWN_TYPE / JOB_PAYLOAD_TOO_LARGE / JOB_CANCEL_REASON_TOO_LONG /
JOB_SCHEDULE_NAME_REQUIRED / JOB_SCHEDULE_NOT_FOUND_BY_NAME /
JOB_SCHEDULE_NAME_CONFLICT / JOB_SCHEDULE_SINGLETON_EXISTS).
- Upgrade SchedulerService public-API JSDoc on registerSchedule / pause /
resume / unregister / triggerNow / getNextRun / has with @param /
@returns; tighten the class-level doc to describe `protect: true`
semantics and the chained-setTimeout interval implementation.
- Add descriptive block comments to private state-machine transition
helpers (finalizeJob / scheduleRetry / armSchedule / detectAndDispatch-
Overdue / runGC / handleFor / publishState / computeBackoff / dispatchAll)
to document non-obvious design decisions, and inline-document the fixed
lock acquisition order inside `dispatch` to prevent accidental reversal.
No runtime behavior change. lint / format / docs:check-links all clean.
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.
The rule that schema files in packages/shared/data/api/schemas/ are
organized by the entity's domain (not URL prefix) was followed in code
but never written down, so readers could reasonably misinfer "the URL
parent decides the file" from routes like /topics/:topicId/messages
living in messages.ts.
Add a Schema File Organization section to api-types.md, with a small
table comparing routes whose URL parent and returned entity disagree,
and a cross-reference note from api-design-guidelines.md so path-design
readers land on the right page.
- data-api-in-main: add Registry Sub-Resource Endpoints section -
GET-only for stateless reads, AIP-136 colon notation for derived
views, registry packages are main-only (bundle waste + merge
already in main)
- best-practice-layered-preset-pattern: preset-only static fields
must merge in rowToEntity rather than via parallel endpoint;
document acceptable exceptions for catalog and specialised
surfaces
- data-ordering-guide section 2: drop user_provider.isEnabled from
the Live partition list - the table is whole-table ordered
(already correct in section 7)
- database-patterns: flag boolean columns without .notNull() as a
common R3 offender, with concrete wrong/right example
All service methods accepting a Drizzle transaction now follow:
- tx is the first parameter
- method name ends with Tx
Renamed:
- pinService.purgeForEntity / purgeForEntities -> *Tx
- tagService.purgeForEntity / purgeForEntities -> *Tx
- TagService#assertTagsExist (private) -> assertTagsExistTx
- PromptService#assertPromptsExist (private) -> assertPromptsExistTx
- AssistantService#syncRelations (private) -> syncRelationsTx
- AgentService#syncSettingsToSessions (private) -> syncSettingsToSessionsTx
The convention is documented in docs/references/data/data-api-in-main.md
under the new "Transaction Method Naming" section. Behavior is unchanged.
Wraps setInterval with auto-unref, exception isolation, and cleanup
via the existing registerDisposable channel. Returns a Disposable.
Replaces 5 ad-hoc setInterval patterns (CacheService, PreferenceService,
WindowManager, ProxyManager, FileProcessingTaskService) — unifies
three pre-existing cleanup styles and fixes a missing unref() in
ProxyManager.
WindowManager: warmupGcTimer cleanup moves from onDestroy to onStop
(via registerDisposable) — acceptable for this singleton.
Skipped (YAGNI): registerTimeout, immediate/unref options,
activation-scoped variant. QuickAssistantService keeps bare
setInterval — its lifecycle is finer than activation.
### 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>
The "DB defaults are near-permanent" guidance previously claimed
drizzle-kit cannot auto-generate the SQLite table rebuild for DEFAULT
changes. That's incorrect: drizzle-kit emits the full PRAGMA / CREATE
__new_xxx / INSERT SELECT / DROP / RENAME / re-create-indexes sequence
automatically.
Rewrite the supporting argument from "tooling can't do it" to:
- every change forces a full-table rebuild at runtime (schema lock,
~2x temporary disk, slow on multi-GB tables);
- DEFAULT changes never touch existing rows;
- legacy NULL backfill must be hand-written into the rebuild's
INSERT SELECT line via COALESCE - drizzle-kit will not synthesize
that.
Conclusion (near-permanent) and Safe bias remain unchanged - only the
underlying mechanics are corrected.
Drop the drizzle-orm#2489 reference and the "drizzle-kit generate
--custom" workaround it implied.
Codify what was implicit: putting a value into a DB column DEFAULT for the first time costs nothing, but changing it later in SQLite is expensive and asymmetric (no ALTER COLUMN SET DEFAULT; drizzle-kit emits only an explanatory comment without naming the affected column — drizzle-orm#2489). So the first write is effectively the final one, and the placement bias should flip from "DB by default" to "service unless certain".
Verified via drizzle-orm maintainer (Andrii Sherman) Medium article, drizzle-orm GitHub issues #2489 / #5360 / #1313, and Drizzle docs via context7. Empirical: this repo's 7 existing migrations are all ALTER TABLE ADD COLUMN — zero ALTER COLUMN — confirming the team has so far avoided the manual rebuild path organically.
Spec changes in best-practice-default-values-and-nullability.md:
- New section "DB defaults are near-permanent" between DM2 and Quick chooser
- DM2 → DB DEFAULT row note tightened, links to new section
- Quick chooser flipped: "unsure" now → Service ?? (was "try DB first")
- Standard Layered Design: emoji moved from DB DEFAULT to service ?? (product-chosen value)
- Anti-patterns: emoji-mask row's Correct column updated; new row for "speculative DB DEFAULT thinking I can tune later"
- Case Study A's Fix description aligned with new bias
- Related References: drizzle-orm#2489 added
Companion update in database-patterns.md: same precision for the DB .default('X') cell in "Where the default value lives".
Establish team standards for placing default values across the data stack (DB DEFAULT / Drizzle $defaultFn / Zod .default() / service ??) and judging column nullability.
Originating context: PR #14689 fixed a PATCH leakage bug rooted in defaults living in three places at once (DB, Zod Create schema, row mapper) for the assistant entity. The follow-up discussion recovered general principles that other entities (agent, message) also violate; this doc captures them as a reference for future schema/service work.
- New: best-practice-default-values-and-nullability.md — Five rules (R1-R5), decision matrices for nullability and default placement, standard layered design example, anti-patterns table, case studies (assistant, modelId, agent.accessiblePaths)
- api-design-guidelines.md: refine Rule C Update derivation guidance; add Rule E discouraging Zod .default() on entity / Create / Update schemas
- data-api-in-main.md: upgrade row-mapper ?? fallbacks from "needs hand-write" tolerance to anti-pattern; add Write-path defaults section codifying R4
- database-patterns.md: add Column Nullability and Defaults section; add R3 no-fabricated-fallbacks bullet to Row → Entity Mapping
- README.md: index entry under Reference Guides
No code changes. Implementation follow-up will land in separate PRs that apply these rules entity by entity.
Mirror the existing single-row purgeForEntity on PinService and TagService
with a purgeForEntities(tx, entityType, ids[]) variant. Empty input is a
no-op; non-empty input collapses N round trips into one and emits a single
aggregated log line with a count, removing the "loop and log per id" smell
when consumers later cascade deletes for many entities of the same type.
Document a Cross-Service Table Access rule in data-api-in-main.md:
- Writes to a foreign table must go through the owner (pass tx into its
method) — owners are the single source of truth for invariants and the
emitter of mutation logs; foreign writes split that knowledge.
- Reads that inline-JOIN a foreign table are allowed when JOIN is the
simpler path (e.g. AssistantService.list pulling per-row tags).
The new bulk methods are intentionally landed without a consumer so that
the "add a bulk method on the owner instead of writing raw SQL from the
caller" path described by the rule is available the moment a cascading
delete appears, rather than nudging the first author to bypass the owner.
