From 68c4d8924151e2e1602e2cdd09288ea2220a95d4 Mon Sep 17 00:00:00 2001 From: troyeguo <13820674+troyeguo@users.noreply.github.com> Date: Sat, 6 Jun 2026 15:47:30 +0800 Subject: [PATCH] refactor: consolidate and enhance guidelines for Electron main process, Go server, i18n, and TypeScript/React --- .github/copilot-instructions.md | 134 +++++++----------- .../electron-main.instructions.md | 72 ++++++++++ .../instructions/go-server.instructions.md | 42 ++++++ .github/instructions/i18n.instructions.md | 32 +++++ .../typescript-react.instructions.md | 77 ++++++++++ 5 files changed, 274 insertions(+), 83 deletions(-) create mode 100644 .github/instructions/electron-main.instructions.md create mode 100644 .github/instructions/go-server.instructions.md create mode 100644 .github/instructions/i18n.instructions.md create mode 100644 .github/instructions/typescript-react.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index defa1fa8..f8a873f4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,92 +1,60 @@ -# Koodo Reader – Copilot Instructions +# Koodo Reader – Code Review Standards -## Architecture Overview +## Security Critical Issues + +- Validate all IPC arguments from the renderer before filesystem, DB, or shell operations +- Never log tokens, passwords, or full book-file paths at info level (use `electron-log` at debug) +- Flag any new `eval()` call; existing plugin TTS/dictionary paths in the main process are intentional +- Sanitize HTML injected into reader iframes (style/note utilities in `src/utils/reader/`) +- Check for hardcoded credentials, API keys, or secrets in code and config files +- Review authentication and authorization on any new OPDS or cloud-sync endpoints + +## Performance Red Flags + +- Flag synchronous file I/O on the Electron main thread (`fs.readFileSync` / `fs.writeFileSync`) +- Identify N+1 database query patterns or missing batch operations +- Ensure resize/scroll event handlers are throttled (see existing `RESIZE_THROTTLE_MS` pattern) +- Verify resource cleanup on window close and reader exit (downloads, DB connections, temp files) + +## Code Quality Essentials + +- All user-visible strings must use `react-i18next` (`t("key")`) — never hardcode English text +- Never open SQLite directly from the renderer — all DB work goes through the `database-command` IPC channel +- Avoid `any` in TypeScript; define interfaces in co-located `interface.tsx` files +- Remove dead code, commented-out blocks, and unused imports before merging + +## Review Style + +- Be specific and actionable; explain the "why" behind each recommendation +- Prioritize security vulnerabilities and main-thread-blocking issues above style comments +- Ask a clarifying question when intent around IPC channels or window lifecycle is ambiguous +- Acknowledge good patterns when you see them + +## Testing Standards + +- Changes to `src/utils/reader/` (live iframe rendering) require manual verification for layout regressions +- New IPC handlers must handle error paths; do not silently swallow failures +- New i18n keys in `en.json` must have corresponding `t()` call sites in `src/` + +--- + +## Architecture Context Koodo Reader is an **Electron + React (CRA) + Redux** cross-platform ebook reader. -| Layer | Location | Role | -| --------------------- | ------------------------------------- | ------------------------------------------------------------------------------ | -| Electron main process | `main.js` | Window management, IPC handlers, SQLite via `better-sqlite3`, sync/cloud utils | -| React renderer | `src/` | UI, Redux state, book rendering | -| Reader engine | `src/assets/lib/kookit-extra.min.mjs` | Closed-source ESM module – book parsing, SQL statements, sync utilities | -| HTTP server | `httpserver/` | Optional Go server for KOReader/OPDS integration | +| Layer | Location | Role | +| ----- | -------- | ---- | +| Electron main | `main.js` | IPC handlers, SQLite via `better-sqlite3`, cloud sync, native integrations | +| React renderer | `src/` | UI, Redux state, book rendering | +| Reader engine | `src/assets/lib/kookit-extra.min.mjs` | Closed-source ESM — book parsing, SQL statements, sync utilities | +| HTTP server | `httpserver/` | Optional Go server for KOReader / OPDS integration | -## IPC Communication Pattern +**Key IPC channels:** `open-book`, `new-tab`, `exit-tab`, `database-command`, `cloud-upload/download`, `discord-rpc-update`, `prompt-biometric-auth` -All renderer↔main communication uses Electron IPC. Main handlers are registered inside `createMainWin()` in `main.js`. +**Redux slices:** `book`, `reader`, `manager`, `viewArea`, `backupPage`, `sidebar`, `progressPanel` -- **Renderer → Main (async):** `ipcRenderer.invoke("channel-name", config)` / `ipcMain.handle(...)` -- **Renderer → Main (sync):** `ipcRenderer.sendSync("channel-name")` / `ipcMain.on(...)` with `event.returnValue` -- **Main → Renderer (push):** `mainWin.webContents.send("event-name", payload)` +**Container pattern:** `index.tsx` (Redux connect) → `component.tsx` → `interface.tsx` in `src/containers/` -Key channels: `open-book`, `new-tab`, `exit-tab`, `database-command`, `cloud-upload/download`, `discord-rpc-update`, `prompt-biometric-auth`. +**Window modes:** `new-tab` → `WebContentsView` overlay (in-app); `open-book` → separate `BrowserWindow`. Reader close is two-phase: `before-reader-close` → flush data → `reader-close-ready`. -## Redux State Structure - -State slices in `src/store/reducers/` — connected via `src/store/index.tsx`: - -- `book` – current book, notes, bookmarks, render functions -- `reader` – reader UI state (mode, scale, theme, nav lock) -- `manager` – library, plugins, auth, dialogs -- `viewArea` – menu open/mode state -- `backupPage` – sync/drive config -- `sidebar`, `progressPanel` – UI panels - -Containers follow the `index.tsx` (connect) → `component.tsx` (React class/func) → `interface.tsx` (prop types) pattern in `src/containers/`. - -## Database Access - -All DB operations go through the main process via the `database-command` IPC channel. SQL statements and schema are defined in `kookit-extra.min.mjs` (`SqlStatement`). Never open SQLite directly from the renderer. - -```ts -// Renderer example -ipcRenderer.invoke("database-command", { - statement: "saveBook", - statementType: "function", - executeType: "run", - dbName: "book", - data: bookObj, - storagePath: dirPath, -}); -``` - -## Reader Tab vs. Reader Window - -- **`new-tab`** – opens a `WebContentsView` overlaid on `mainWin` (in-app tab, used for book reading within the main window). -- **`open-book`** – opens a separate `BrowserWindow` (`readerWindow`). Supports fullscreen, merge-word (frameless transparent), and always-on-top. -- Reader close is two-phase: main sends `before-reader-close` → renderer flushes reading-time data → renderer replies `reader-close-ready` → main closes window. - -## Reader Utilities (`src/utils/reader/`) - -| File | Purpose | -| --------------- | -------------------------------------------- | -| `styleUtil.ts` | Font, layout, CSS injection into book iframe | -| `themeUtil.ts` | Background/text color themes | -| `noteUtil.ts` | Highlight/note rendering in book content | -| `ttsUtil.ts` | Text-to-speech coordination | -| `mouseEvent.ts` | Touch/mouse event handling in reader | -| `discordRPC.ts` | Discord Rich Presence update calls | -| `launchUtil.ts` | Book launch/open coordination | - -## Developer Workflows - -```bash -npm run dev # Start React dev server + Electron with hot reload (nodemon) -npm start # React dev server only (port 3000) -npm run ele # Electron only (needs built or running React server) -npm run build # Production React build → build/ -npm run release # Build + package Electron app (electron-builder) -npm run rebuild # Rebuild native modules (better-sqlite3) for current Electron -``` - -- Dev mode uses `http://localhost:3000`; production uses `file://build/index.html`. -- `electron-store` persists window positions, user preferences, and encrypted tokens between sessions. -- Logs go to `userData/debug.log` via `electron-log`; accessible via **Settings → Debug Logs**. - -## Key Conventions - -- **i18n:** All UI strings use `react-i18next`. Translation files are in `src/assets/locales/`. Run `node scripts/i18n-script.js` to manage translations. -- **Plugins:** Loaded as `PluginModel` objects from DB; plugin scripts are `eval()`'d in the main process for TTS (`generate-tts` IPC handler). -- **Cloud sync:** Instantiated lazily via `getSyncUtil(config)` in `main.js`; cached per service in `syncUtilCache`. Services: OneDrive, Google Drive, Dropbox, WebDAV, S3, SFTP, FTP, SMB, MEGA, etc. -- **Biometric auth:** macOS uses Touch ID (`systemPreferences.promptTouchID`); Windows uses Windows Hello via PowerShell WinRT bridging. -- **Book cover/style assets:** Stored under `userData/uploads/`; path exposed to renderer via `user-data` IPC sync channel. +Path-specific review rules are in `.github/instructions/` for `main.js`, `src/**/*.{ts,tsx}`, `httpserver/**`, and `src/assets/locales/**`. diff --git a/.github/instructions/electron-main.instructions.md b/.github/instructions/electron-main.instructions.md new file mode 100644 index 00000000..299b8aca --- /dev/null +++ b/.github/instructions/electron-main.instructions.md @@ -0,0 +1,72 @@ +--- +applyTo: "main.js" +--- + +# Electron Main Process Guidelines + +## Purpose + +Review rules for `main.js`—IPC handlers, windows, SQLite, cloud sync, and native integrations. + +## IPC Handler Safety + +- Validate all arguments from the renderer before filesystem, shell, or DB operations +- Prefer `ipcMain.handle` (async) over sync handlers unless sync is already established for that channel +- Return structured errors; avoid leaking stack traces or token values to the renderer + +```javascript +// Avoid +ipcMain.handle("save-path", (_, userPath) => { + fs.writeFileSync(userPath, data); +}); + +// Prefer +ipcMain.handle("save-path", (_, userPath) => { + if (typeof userPath !== "string" || userPath.includes("..")) { + throw new Error("Invalid path"); + } + fs.writeFileSync(userPath, data); +}); +``` + +## Database Access + +- Route all SQLite work through existing `database-command` handler patterns +- SQL schema and statements live in `kookit-extra.min.mjs`—do not duplicate schema in `main.js` +- Reuse `dbConnection` lifecycle; close connections on app quit + +## Window Lifecycle + +- **`new-tab`**: `WebContentsView` overlay on `mainWin` for in-app reading +- **`open-book`**: separate `BrowserWindow` (`readerWindow`); supports fullscreen and frameless modes +- Honor two-phase reader close: wait for `reader-close-ready` before destroying reader windows +- Throttle resize handlers (see existing `RESIZE_THROTTLE_MS` pattern) + +## Cloud Sync and Credentials + +- Use cached sync utils (`syncUtilCache`); avoid creating duplicate service instances +- Store tokens via `electron-store`; review new fields for encryption or sensitivity +- Ensure download/upload cancellation cleans up `downloadRequest` and temp files + +## Native and Platform Code + +- macOS biometric: `systemPreferences.promptTouchID` +- Windows Hello: existing PowerShell WinRT bridge—keep timeouts and error parsing +- Plugin TTS uses `eval(voiceFunc)` in main process—flag untrusted plugin source changes + +## Logging + +- Use `electron-log`; never log tokens, passwords, or full book file paths at info level +- Debug logs go to `userData/debug.log` + +## Testing Guidelines + +- New IPC handlers must handle error paths (invalid args, missing files, DB failures) +- Test cancel paths for cloud sync operations (`downloadRequest` cleanup) +- Biometric auth fallback paths must be exercised when hardware is unavailable + +## Performance + +- Avoid blocking the main thread with large synchronous file I/O +- Batch database operations where possible +- Reuse cached sync utils (`syncUtilCache`); creating duplicate instances wastes memory diff --git a/.github/instructions/go-server.instructions.md b/.github/instructions/go-server.instructions.md new file mode 100644 index 00000000..16f7fff8 --- /dev/null +++ b/.github/instructions/go-server.instructions.md @@ -0,0 +1,42 @@ +--- +applyTo: "httpserver/**" +--- + +# Go HTTP Server Guidelines + +## Purpose + +Review rules for the optional Go server used for KOReader and OPDS integration. + +## Code Style + +- Follow standard Go formatting (`gofmt`) +- Use clear handler and package names matching existing files (`koreader.go`, etc.) +- Keep HTTP handlers small; extract helpers for OPDS/KOReader logic + +## Security Considerations + +- Validate request paths and query parameters; prevent directory traversal +- Do not expose arbitrary filesystem access beyond configured library roots +- Review authentication on OPDS endpoints if added or changed +- Avoid logging client credentials or book content at default log levels + +## Error Handling + +- Return appropriate HTTP status codes with concise error bodies +- Handle server shutdown and connection cleanup gracefully + +## Testing Guidelines + +- New HTTP handlers should have unit tests covering at least one success and one error path +- OPDS/KOReader endpoint changes need integration-level verification with a real client if feasible + +## Performance + +- Keep handlers non-blocking; use goroutines for long-running operations +- Avoid loading entire book files into memory for metadata-only responses + +## Dependencies + +- Update `go.mod` / `go.sum` together when adding modules +- Prefer stdlib where the existing code does; match current dependency choices diff --git a/.github/instructions/i18n.instructions.md b/.github/instructions/i18n.instructions.md new file mode 100644 index 00000000..5a8ea51e --- /dev/null +++ b/.github/instructions/i18n.instructions.md @@ -0,0 +1,32 @@ +--- +applyTo: "src/assets/locales/**" +--- + +# i18n Locale Guidelines + +## Purpose + +Review rules for translation JSON files under `src/assets/locales/`. + +## Key Conventions + +- Preserve existing key naming style (camelCase or dot-separated keys used in the repo) +- Do not rename keys without updating all references in `src/` +- Keep JSON valid: double quotes, no trailing commas, UTF-8 encoding + +## Translation Changes + +- English (`en.json`) is typically the source locale for new keys +- Non-English locale edits should come from dedicated translation PRs or `scripts/i18n-script.js` workflows +- Flag PRs that only change English UI strings in code but omit matching locale keys + +## Review Focus + +- Check for missing placeholders (`{{name}}`, `%s`) compared to the English source string +- Flag broken interpolation or HTML entities that could break rendering +- Avoid embedding secrets or internal URLs in translation strings + +## Testing Guidelines + +- After adding a new key to `en.json`, verify the `t("key")` call site exists in `src/` +- Placeholder changes (e.g. `{{count}}` → `{{total}}`) must be updated everywhere the key is used diff --git a/.github/instructions/typescript-react.instructions.md b/.github/instructions/typescript-react.instructions.md new file mode 100644 index 00000000..ad9fbbba --- /dev/null +++ b/.github/instructions/typescript-react.instructions.md @@ -0,0 +1,77 @@ +--- +applyTo: "src/**/*.{ts,tsx}" +--- + +# TypeScript / React Guidelines + +## Purpose + +Review rules for the React renderer, Redux store, components, and reader utilities under `src/`. + +## Type Safety + +- Avoid `any`; define interfaces in co-located `interface.tsx` files +- Type Redux `mapStateToProps` with `stateType` from `src/store` +- Type IPC payloads when adding new renderer-side invoke calls + +```typescript +// Avoid +function handleBook(book: any) { + return book.key; +} + +// Prefer +interface BookRecord { + key: string; + name: string; +} + +function handleBook(book: BookRecord): string { + return book.key; +} +``` + +## Component Structure + +- Follow container pattern: `index.tsx` (Redux connect) → `component.tsx` → `interface.tsx` +- Keep components focused; extract reusable UI into `src/components/` +- Match existing class or function component style in the surrounding folder + +## Redux Conventions + +- Add actions in `src/store/actions/` and reducers in `src/store/reducers/` +- Avoid mutating state; follow existing reducer patterns in the target slice +- Do not access SQLite or filesystem directly from the renderer + +## i18n + +- All user-visible strings must use `react-i18next` (`t("key")`), not hardcoded English +- Add keys to locale JSON under `src/assets/locales/` when introducing new UI text +- Do not edit translated locale files for non-English languages unless the PR explicitly updates translations + +## Reader Utilities (`src/utils/reader/`) + +- Changes affect live book rendering inside iframes—review for layout regressions and XSS +- Coordinate with reader tab (`new-tab`) vs reader window (`open-book`) lifecycle +- Reader close is two-phase: `before-reader-close` → flush data → `reader-close-ready` + +## Error Handling + +- Surface user-facing errors via existing toast/dialog patterns +- Do not swallow IPC failures silently; log or notify the user + +## Testing Guidelines + +- Reader utility changes (`src/utils/reader/`) need manual regression testing in both tab and window reader modes +- New Redux actions should have corresponding reducer tests verifying state shape +- IPC failure paths must surface errors via toast/dialog, not silent swallows + +## Security Considerations + +- Flag new `eval()` in popup or plugin components; existing plugin TTS/dictionary paths are intentional +- Sanitize HTML injected into reader iframes in style/note utilities + +## Performance + +- Avoid re-rendering heavy components on every Redux state change—use `shouldComponentUpdate` or `React.memo` +- Prefer lazy imports for large reader libraries not needed on initial load