refactor(ipc): add typed IPC registry, migrate aiKeyHandlers as proof#272
Conversation
Scoped slice of PR-F. This adds the foundation; the rest of PR-F
(keychain, HMAC license) and the migration of the remaining 12 handler
modules become follow-up PRs once the pattern is reviewed.
What this PR introduces:
- apps/desktop/src/main/ipc/registry.ts — defineIpcHandler({channel, args, handler})
wraps ipcMain.handle with Zod tuple validation. Renderer-supplied args
are validated BEFORE business logic runs. An invalid payload throws
IpcValidationError at the boundary, surfacing as a structured error
to the renderer instead of a confusing downstream crash.
- apps/desktop/src/main/handlers/aiKeyHandlers.ts — migrated as the
proof. Provider names are now constrained to alphanumeric (with _ or -),
max 64 chars. API keys are bounded at 4096 chars. These are
conservative defenses against accidental large payloads and obviously
bogus inputs, not a substitute for the credential-at-rest hardening
that's coming separately (keychain integration is the next slice of
PR-F).
- apps/desktop/package.json — adds zod ^4.4.3 as a direct dep (it was
already in 4 workspace packages but not exposed to desktop main).
Audit linkage:
- B2 (IPC handlers accept raw strings without validation): partially
addressed for the AI-key surface. The 12 other handler modules will
follow the same defineIpcHandler({channel, args, handler}) shape in
subsequent PRs so each migration is small enough to review.
- B3 (AI keys stored unencrypted via keytar) and B4 (license HMAC) are
deliberately NOT in this PR. They each need keytar integration with
electron-builder, a one-time migration on first launch, and reviewer
attention — separate PRs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
549fd11
into
refactor/db-sync-indexes-and-fts5-hardening
## Summary Applies the typed IPC registry pattern from #272 to six handler modules. ~30 IPC channels are now validated at the boundary with Zod tuple schemas before the business logic runs. The remaining five "heavy" modules (\`note\`, \`notebook\`, \`data\`, \`git\`, \`authSync\` — ~96 channels) will land in **PR-F5** as a separate PR for reviewability. ## Files migrated | Module | Channels | Notes | |---|---|---| | \`logHandlers.ts\` | 2 | LogLevel enum, message ≤16 KiB | | \`shareHandlers.ts\` | 2 | Note content ≤1 MiB, tags/backlinks count-capped | | \`updateHandlers.ts\` | 2 (+1 raw) | \`updates:installNow\` kept raw — synchronous quit path | | \`licenseHandlers.ts\` | 4 | Optional plan enum on \`openSubscribe\` | | \`localServerHandlers.ts\` | 4 | Port range 1–65535 (was inside-handler check) | | \`pluginHandlers.ts\` | 11 (+1 raw) | \`PluginIdSchema\` matches install-time regex | \`plugins:requestReload\` uses \`ipcMain.on\` (fire-and-forget, not invoke), so it's not subject to \`defineIpcHandler\` — left raw. ## Schema highlights - \`PluginIdSchema\` is \`z.string().min(1).max(128).regex(/^[a-zA-Z0-9_-]+$/)\` — mirrors the regex enforced at install time on \`manifest.id\`. The boundary now catches the same shape that the install-time check catches. - Where the original code had defensive in-handler validation (e.g. \`if (port < 1 || port > 65535)\`), the schema now handles it. Defensive checks that produce nicer error strings (e.g. the \"https only\" guard inside \`installFromUrl\`) are kept as belt-and-suspenders. ## Test plan - [x] \`pnpm --filter @readied/desktop typecheck:main\` — green. - [x] \`pnpm test\` — 17/17 packages. - [ ] Manual: each affected setting/feature in the app continues to work for valid inputs. - [ ] Manual: try a malformed payload via DevTools (e.g. \`window.readied.plugins.setEnabled('not-a-valid-id!@#', true)\`). Expect \`IpcValidationError\`. ## Stack context **PR-F4** in the audit stack. Stacked on top of #272 (PR-F1) → #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). ## Follow-ups - **PR-F5**: heavy data modules (note, notebook, data, git, authSync) — same pattern, ~96 channels. - **PR-F2**: keychain storage for AI keys (Electron safeStorage is the leaning choice). - **PR-F3**: license verification (asymmetric — server signs, client verifies; **not** HMAC). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#274) ## Summary Completes the IPC registry migration started in #272 (registry) → #273 (light handlers). Five remaining handler modules (~97 channels) now run through \`defineIpcHandler\` with Zod tuple validation at the boundary. **The full IPC surface of the desktop app is now validated.** ## Files | Module | Channels | Notes | |---|---|---| | \`notebookHandlers.ts\` | 15 | Notebook CRUD + per-notebook git settings. Common \`serialize(nb)\` helper avoids 6 copies of the same object spread. | | \`gitHandlers.ts\` | 9 | Short-SHA accepted (≥7 hex), commit message ≤8 KiB, note content ≤1 MiB. | | \`dataHandlers.ts\` | 7 of 8 | backup/list/export/exportNote/import/paths/openFolder migrated. **\`data:backup:restore\`** stays raw — the integrity-check + rollback state machine from #271 doesn't fit the registry's wrapper cleanly. | | \`noteHandlers.ts\` | 32 | Notes CRUD, tags, links, embeds, stats. \`EmbedTargetSchema\` regex-constrained to filename-safe characters (no slashes, no path traversal) at the boundary in addition to the existing \`join()\` guard. \`ArrayBuffer\` validated via \`z.instanceof(ArrayBuffer)\` on \`embeds:saveAsset\`. | | \`authSyncHandlers.ts\` | 33 | Auth/magic-link, sync (pull/push/syncNow/conflicts/auto-sync/history), E2EE key management, subscription portal, devices. \`SyncChangeSchema\` is the most complex shape here. | ## Exceptions (still raw \`ipcMain\`) After this PR every \`ipcMain.handle\` in \`apps/desktop/src/main/handlers/\` either goes through \`defineIpcHandler\` or has an explanatory comment: | Channel | Why raw | |---|---| | \`updates:installNow\` | Synchronous quit + window destruction path; registry's async wrapper would interfere | | \`plugins:requestReload\` | Uses \`ipcMain.on\` — fire-and-forget event, not invoke | | \`data:backup:restore\` | Integrity-check rollback state machine from PR #271 | These are documented in code as well as listed here. ## Schema highlights - \`SyncChangeSchema\`: \`{ noteId, operation: enum, content?: ≤10 MiB, localVersion?: int }\` — the boundary now catches malformed push payloads instead of letting them flow into the sync engine. - \`EmbedTargetSchema\`: \`/^[a-zA-Z0-9._-]+$/\` — defense in depth on top of \`join()\` for path traversal. - \`KeyHexSchema\` for encryption export/import — strict hex format, length 32–256 chars. - All \`IdSchema\`s capped at 128 chars; \`TagSchema\` at 64; \`ContentSchema\` at 10 MiB; \`CommitMessageSchema\` at 8 KiB. ## Test plan - [x] \`pnpm -r typecheck\` — green. - [x] \`pnpm test\` — 17/17 packages. - [ ] Manual: exercise notes CRUD, tag operations, sync cycle, encryption setup, device management. - [ ] Manual: send malformed payloads via DevTools (e.g. \`window.readied.sync.push('not-an-array')\`) and expect \`IpcValidationError\`. ## Stack context **PR-F5** in the audit stack. Stacked on top of #273 (PR-F4) → #272 (PR-F1) → #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). ## Follow-ups (per Tomy's review) - **PR-F2** — keychain storage for AI keys via Electron \`safeStorage\` (not keytar). - **PR-F3** — license verification with **asymmetric** crypto (server signs, client verifies with embedded public key). The original audit's HMAC suggestion is wrong: a client that can verify can also forge. - **PR-E** — Playwright Electron E2E (deferred until architecture settles). - **PR-G** — split god files (deferred — too risky to mix with this stack). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…errors (#275) ## Summary The audit listed **B3 (\"AI keys stored unencrypted\")** as a gap, but \`apps/desktop/src/main/services/aiKeyStorage.ts\` has been using Electron \`safeStorage\` since the file was introduced. AI keys are already OS-encrypted: macOS Keychain / Windows DPAPI / Linux libsecret. **No new dependency needed — \`safeStorage\` is the right call here, exactly as you said.** However, while reviewing the file for the audit, I found a **real bug** worth shipping in this PR. ## The real bug \`readKeys()\` used a catch-all that treated any decryption / parse failure as corruption and deleted the encrypted file: \`\`\`ts } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') { return {}; } // Decryption or parsing failed - clear corrupted file await this.clearAll(); // ← silent data loss return {}; } \`\`\` On macOS the keychain becomes **temporarily** inaccessible after sleep/wake (and on Linux when libsecret hasn't unlocked yet). \`safeStorage.decryptString()\` throws during that window. The previous code interpreted that as corruption and wiped the file → user's AI keys gone, with no error surfaced. ## New error contract | Condition | Behavior | |---|---| | ENOENT on read | Return \`{}\` (no keys yet) | | \`isEncryptionAvailable()\` → false | Throw \`AiKeyEncryptionUnavailableError\` | | \`safeStorage.decryptString\` throws | Throw \`AiKeyDecryptionError\` (cause preserved) | | \`JSON.parse\` fails | Throw \`AiKeyDecryptionError\` | | Top-level shape is not an object | Throw \`AiKeyDecryptionError\` | **The encrypted file is never auto-deleted on a read path now.** The only delete sites are: - Explicit \`removeKey()\` that empties the map → \`unlinkFile()\` - (No other paths.) Errors propagate to the IPC layer, where \`defineIpcHandler\` from #272 wraps them in a structured response for the renderer. ## Why this matters Before this fix, the failure mode for a user whose keychain happened to be locked at the wrong moment was: open the AI panel → see \"no keys\" → re-enter all their provider keys → next sleep/wake cycle: repeat. The fix surfaces a real error (\"keychain locked, retry\") instead of silently destroying state. ## Test plan - [x] \`pnpm -r typecheck\` — green. - [x] \`pnpm test\` — 17/17 packages. - [ ] Manual on macOS: save an AI key, sleep the machine for a few minutes, wake up, immediately open AI panel. Before this fix: keys may be gone. After: either keys load normally OR a clear error surfaces and a retry works. - [ ] Manual on Linux without libsecret: expect \`AiKeyEncryptionUnavailableError\` with the message pointing at libsecret/gnome-keyring/kwallet. ## Stack context **PR-F2** in the audit stack. Stacked on top of #274 (PR-F5) → #273 (PR-F4) → #272 (PR-F1) → #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). ## Up next **PR-F3**: Ed25519 license verification — server signs license payload with private key, desktop verifies with embedded public key. No symmetric secrets on the client. Coming as the next PR in the stack. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…F3) (#276) ## Summary Adds the asymmetric verification primitive the desktop needs so the license server can sign subscription state and the client can verify it **without holding any signing secret**. This is the foundation; no live wiring yet — those follow once the server emits signed envelopes. Per your review: **Ed25519, not HMAC** — a client that can verify a symmetric MAC can also forge it. ## Wire format \`\`\`ts { payload: { payloadVersion: 1, subscription: { /* SubscriptionInfo */ }, issuedAt: "2026-06-08T12:00:00.000Z", ttlSeconds?: 3600 // optional, server-suggested max age }, signature: "<base64 Ed25519 signature>" } \`\`\` Signature is computed over \`canonicalJson(payload)\` — a deterministic sorted-key JSON encoding. \`JSON.stringify\` is **not** deterministic across runtimes, so the canonical encoder is mandatory. ## Files | File | Change | |---|---| | \`packages/licensing/src/types.ts\` | Add \`SignedSubscriptionPayload\` + \`SignedSubscriptionEnvelope\` types | | \`packages/licensing/src/validator.ts\` | Add \`canonicalJson\`, \`signSubscriptionPayload\` (server), \`verifySubscriptionSignature\` (client). \`DEFAULT_SUBSCRIPTION_PUBLIC_KEY\` is an all-zeros placeholder with a REPLACE BEFORE SHIPPING note | | \`packages/licensing/src/index.ts\` | Re-export the new types + functions | | \`packages/licensing/__tests__/subscriptionSignature.test.ts\` | **18 new tests** (103 total in the package now) | | \`packages/licensing/README.md\` | Wire format, server flow, embedded public key, key rotation, why trial.json is **not** signed | ## What \`verifySubscriptionSignature\` checks 1. Envelope shape (\`payload\` + \`signature\` strings present). 2. Payload shape (\`payloadVersion: 1\`, \`issuedAt\`, \`subscription\`). 3. Ed25519 signature against the public key (or \`DEFAULT_SUBSCRIPTION_PUBLIC_KEY\` if the caller doesn't pass one — and the default will fail until replaced). 4. Replay window — rejects envelopes older than \`maxAgeSeconds\` (defaults to \`payload.ttlSeconds\`, else 7 days). 5. Future-timestamp window — tolerates 60s clock skew but rejects obviously-future \`issuedAt\` values. 6. Subscription field/period validation (delegates to existing \`verifySubscription\`). Injectable clock (\`config.nowMs\`) makes timing tests deterministic. ## What is NOT in this PR (intentional) - **Live wiring** into \`FileLicenseStorage\` / \`licenseHandlers.ts\`. Wiring would lock out every user before the server emits signed envelopes. Wiring lands as a follow-up once the server is ready. - **Real public key.** \`DEFAULT_SUBSCRIPTION_PUBLIC_KEY\` is all zeros. Replacing it is a deliberate, separate change — coordinated with whichever release first ships signed envelopes. - **Trial signing.** Trial state remains unsigned by design (no server-side trial registration; subscription is the real boundary). The README explains. ## Test plan - [x] \`pnpm --filter @readied/licensing test\` — **103/103 tests** pass (5 files; 18 new in this PR). - [x] \`pnpm -r typecheck\` — green across the monorepo. - [ ] Server team uses \`signSubscriptionPayload\` to start signing. After enough clients have the new code, the embedded public key is replaced and live verification is enabled in a follow-up PR. ## Stack context **PR-F3** in the audit stack. Stacked on top of #275 (PR-F2) → #274 (PR-F5) → #273 (PR-F4) → #272 (PR-F1) → #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Lands the **infrastructure** for end-to-end testing the desktop app: a Playwright + Electron setup with per-test isolation, two initial specs, and a Linux CI job. The CI job ships as **\`continue-on-error: true\`** while we stabilize — flip it off in a follow-up once it's reliably green on develop. ## Files | File | Purpose | |---|---| | \`apps/desktop/playwright.config.ts\` | Serial workers (Electron app state is shared), generous timeouts, retain-on-failure trace/video | | \`apps/desktop/e2e/fixtures.ts\` | \`launchApp()\` helper — fresh Electron instance with isolated \`userData\` per test via \`mkdtemp\`; \`_electron.launch\` API | | \`apps/desktop/e2e/tsconfig.json\` | Dedicated tsconfig so specs don't pull in renderer/main types | | \`apps/desktop/e2e/smoke.spec.ts\` | App launches, window renders, IPC bridge present, no uncaught console errors during mount | | \`apps/desktop/e2e/notes.spec.ts\` | Notes IPC contract — create/list/get roundtrip, FTS5 search | | \`apps/desktop/e2e/README.md\` | How to run locally, what's tested, what's out of scope | | \`apps/desktop/package.json\` | \`@playwright/test\` devDep, \`e2e\` + \`e2e:headed\` scripts, \`typecheck:e2e\` folded into \`typecheck\` | | \`.github/workflows/ci.yml\` | New \`e2e\` job, ubuntu + xvfb, uploads playwright-report on failure | | \`.gitignore\` | Excludes \`test-results/\`, \`playwright-report/\`, \`playwright/.cache/\` | | \`knip.json\` | Registers \`playwright.config.ts\` + \`e2e/**/*.ts\` | ## Why drive the preload bridge, not the editor UI \`notes.spec.ts\` calls \`window.readied.notes.*\` directly via \`page.evaluate\` instead of typing in the CodeMirror surface. Reasons: 1. **Selectors churn**, contracts are stable. 2. The preload bridge is the same surface the renderer uses — anything that breaks here breaks the renderer too. 3. CodeMirror flake on hotkeys / IME / focus is a class of pain we don't need until the editor is split. Future UI-level specs land once \`MarkdownEditor.tsx\` is split (PR-G follow-up). ## Specs in this PR ### \`smoke.spec.ts\` 1. App launches, main window has non-zero size, IPC bridge present. 2. Console produces no uncaught errors in the first 3 seconds (ignoring known noise like \"Sentry: No DSN configured\"). This is the regression catch for #266 — the editor mount crashes that produced blank windows. ### \`notes.spec.ts\` 1. \`create → list → read\` roundtrip works. 2. \`search\` via FTS5 finds a freshly-created note via a unique marker. ## What's deliberately NOT here - Editor UI interactions (typing, formatting, hotkeys) — too prone to flake without per-spec selectors. Revisit after editor split. - AI panel streaming — better as a vitest test against \`@readied/ai-core\`. - Sync flows — need a fake server. ## Honest disclaimer **I scaffolded all of this but could NOT execute the suite end-to-end against a real Electron build in this session** (no display attached, no built bundle in this branch). The CI's \`continue-on-error\` gate and the local README acknowledge that the first verifier on a real machine may need to tweak the specs once they meet a real renderer. ## Test plan - [x] \`pnpm --filter @readied/desktop typecheck\` — green (includes new \`typecheck:e2e\`). - [x] \`pnpm test\` — 17/17 packages. - [ ] Run \`pnpm --filter @readied/desktop build && pnpm --filter @readied/desktop e2e\` locally on macOS to confirm both specs pass against the real bundle. Adjust selectors / bridge shape if reality drifts from the assumed types. - [ ] Push to a feature branch and watch the CI \`e2e\` job. Confirm artifacts upload on intentional failure. ## Stack context **PR-E** in the audit stack. Stacked on top of #276 (PR-F3) → #275 (PR-F2) → #274 (PR-F5) → #273 (PR-F4) → #272 (PR-F1) → #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). 13 PRs deep. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es (PR-G) (#278) ## Summary **First scoped slice of the PR-G \"split god files\" effort.** \`apps/desktop/src/main/index.ts\` goes from **1065 → 950 lines** (-11%). No behavior change. All extractions are self-contained and surface-area preserving — no API changes, no runtime touch. ## What moved | New file | Was at | Notes | |---|---|---| | \`services/fileLicenseStorage.ts\` | \`index.ts:146–216\` | Three \`readJsonOrNull\` helpers fold the repeated try/catch/return-null pattern into one private function. Header comment points at PR-F3 so the next step (move to signed envelopes) is unambiguous. | | \`services/windowState.ts\` | \`index.ts:218–254\` | Header comment now documents *why* the file I/O is synchronous (called during \`BrowserWindow\` construction before the renderer mounts, and during close where handlers don't await). | ## What this PR DELIBERATELY does NOT touch Per your earlier guidance about merge-conflict risk: | File | Why deferred | |---|---| | \`packages/storage-sqlite/src/repositories/SQLiteNoteRepository.ts\` (1121 L) | Splitting into \`NoteCrudRepository\` + \`NoteTagRepository\` + \`NoteArchiveRepository\` + \`NoteSyncRepository\` needs runtime verification against a real DB. Risk of merge conflicts with every other in-flight PR. Separate effort. | | \`apps/desktop/src/renderer/components/MarkdownEditor.tsx\` (724 L) | The CodeMirror surface is too entangled to split without an E2E suite to catch regressions. Defer until PR-E (#277) is stabilized; then refactor under test coverage. | The principle is established here: surgical extractions of self-contained pieces, each verifying typecheck+tests, no behavior change. Future PRs can keep slicing under the same discipline. ## Test plan - [x] \`pnpm -r typecheck\` — green - [x] \`pnpm test\` — 17/17 packages - [x] \`pnpm --filter @readied/desktop typecheck\` — green (includes the e2e tsconfig from #277) - [ ] Manual smoke: launch the app, confirm window remembers its position after close+reopen, confirm license/trial files still read/write correctly ## Stack context **PR-G** in the audit stack. Stacked on top of #277 (PR-E) → #276 (PR-F3) → #275 (PR-F2) → #274 (PR-F5) → #273 (PR-F4) → #272 (PR-F1) → #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). **14 PRs deep.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary First slice of the knip cleanup. Knip surfaced 28 unused files in #267. After per-file verification (greps, import tracing, barrel resolution), **12 were genuinely orphaned** — those are deleted here. The rest stay for reasons documented below. ## Deleted (12 files, 659 lines) ### \`apps/desktop\` | File | Why safe | |---|---| | \`src/renderer/analytics.ts\` | No imports anywhere | | \`src/renderer/hooks/useTheme.ts\` | No imports anywhere | | \`src/renderer/settings.tsx\` | No imports anywhere (the real settings entry is \`pages/settings/SettingsApp.tsx\`, loaded via dynamic import in \`main.tsx\`) | | \`src/renderer/ui/patterns/Modal.tsx\` + \`index.ts\` + \`Modal.module.css\` + \`.gitkeep\` | Only the barrel imported \`Modal\`, and the barrel itself was unused — both go together | ### \`packages\` | File | Why safe | |---|---| | \`plugin-api/src/editor/types.ts\` | Not re-exported by \`plugin-api/src/index.ts\` | | \`storage-core/src/{interfaces,migrations,repositories,types}/index.ts\` | Main package index imports directly from concrete files, not these barrels | | \`storage-sqlite/src/repositories/index.ts\` | Same — concrete imports, no barrel use | ### \`scripts\` | File | Why safe | |---|---| | \`scripts/bump-version.js\` | Not referenced by any \`package.json\` script or CI workflow | ## Kept (Knip false positives) ### Auto-discovered files Knip can't see | File | Why kept | |---|---| | \`apps/desktop/src/renderer/vite-env.d.ts\` | \`/// <reference types="vite/client" />\` — required by Vite | | \`apps/desktop/src/renderer/css-modules.d.ts\` | TypeScript module shim for CSS module imports | | \`apps/desktop/src/renderer/turndown-plugin-gfm.d.ts\` | TypeScript module shim for an untyped npm dep | | \`apps/web/mdx-components.tsx\` | Next.js convention — auto-discovered, never imported | ### Knip wrong about being unused | File | Reality | |---|---| | \`renderer/pages/settings/components/controls/{NumberInput,Select,TextInput,Toggle}.tsx\` | Imported by EditorSection / AiSection / UpdatesSection **through** the \`controls/index.ts\` barrel. Knip flagged both the components and the barrel as unused because it doesn't follow the chain. | ## Skipped (out of audit scope) apps/web cleanup: \`magicui/*\`, \`NavDropdown\`, \`ui/separator\`. The audit excluded apps/web; leaving them for a separate marketing-site pass. ## Test plan - [x] \`pnpm -r typecheck\` — green - [x] \`pnpm test\` — 17/17 packages - [ ] Manual smoke after merge ## What's next in this cleanup track - **PR-Knip-2**: remove unused production + dev dependencies (the \`Unused dependencies (9)\` block from knip) - **PR-Knip-3**: remove unused exports (the \`Unused exports (~100)\` block) — much more careful, since some exports may be public API contracts for plugins or external consumers ## Stack context Stacked on top of #278 (PR-G) → #277 (PR-E) → #276 (PR-F3) → #275 (PR-F2) → #274 (PR-F5) → #273 (PR-F4) → #272 (PR-F1) → #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). **15 PRs deep.** 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Why this PR The audit stack (#266 through #284) was structured as 19 stacked PRs, each one's base pointing at its predecessor in the chain. Each merged into **its own parent branch**, not into \`develop\`. Net result: \`develop\` only contains #265, and the other 19 PRs are stranded on their branch tips. This PR ships the head of the stack (\`chore/set-subscription-public-key\`) into \`develop\` so the work actually lands. It's a single PR with **21 commits** — the linear chain of the stack — preserving each individual PR's conventional commit message so semantic-release can categorise them for the next release. ## What's in this PR (in merge order) | # | Commit | Title | |---|---|---| | 1 | \`3412e4e\` | fix(typecheck): unblock pnpm -r typecheck after TS 6.x bump (already merged as squash in develop — no-op overlap) | | 2 | \`03da9cb\` | fix(editor): stop runtime crashes in MarkdownEditor | | 3 | \`07a76d6\` | chore(tooling): replace husky with lefthook, add knip, tighten lint-staged | | 4 | \`e27f1ba\` | refactor(stores): use named selectors instead of destructuring full state | | 5 | \`c519fc3\` | chore(test): add coverage baseline + smoke tests for @readied/commands | | 6 | \`5b1cc55\` | chore(mcp-server): migrate to registerTool API + FTS5 for read_note | | 7 | \`0eb49a5\` | fix(backup): integrity-check restored db and roll back on failure | | 8 | \`402e280\` | refactor(ipc): add typed IPC registry, migrate aiKeyHandlers as proof | | 9 | \`dd4823d\` | refactor(ipc): migrate light handlers to defineIpcHandler | | 10 | \`5607100\` | refactor(ipc): migrate heavy data handlers to defineIpcHandler | | 11 | \`c51c4d3\` | fix(aiKeyStorage): stop deleting encrypted keys on transient decrypt errors | | 12 | \`d5f33ef\` | feat(licensing): add Ed25519 subscription envelope sign + verify | | 13 | \`5ba96d4\` | feat(e2e): scaffold Playwright Electron suite + CI job | | 14 | \`2bd1396\` | refactor(main): extract FileLicenseStorage and window state to services | | 15 | \`fd9b809\` | chore(knip): delete verified-unused files (phase 1) | | 16 | \`a3d7c1b\` | chore(knip): remove unused dependencies (phase 2) | | 17 | \`0ec48b3\` | feat(license): wire Ed25519 signed-envelope verification at the storage layer | | 18 | \`cca7e04\` | refactor(storage-sqlite): extract noteMapping helpers from SQLiteNoteRepository | | 19 | \`1a0df95\` | refactor(editor): extract theme + highlight from MarkdownEditor | | 20 | \`9034c71\` | chore(license): set real SUBSCRIPTION_PUBLIC_KEY | | 21 | \`390503c\` | docs(api): document LICENSE_SIGNING_PRIVATE_KEY secret requirement | Each is already individually reviewed and merged on GitHub (#266–#284). They appear here as their original commits because the stack used rebase-based stacking, not merge commits. ## Pre-merge verification (local, this branch) - ✅ \`pnpm -r typecheck\` — green across 18 workspace projects - ✅ \`pnpm test\` — 17/17 packages - ✅ \`pnpm build\` — 6/6 packages ## After this PR merges Per the release flow in \`CLAUDE.md\`: 1. Open \`develop → main\` PR 2. Click \"Run workflow\" on Release action — \`semantic-release\` analyses these conventional commits and bumps the version 3. Tag push triggers Build workflow — mac/win/linux in parallel 4. All builds green → release un-drafts → electron-updater serves the update ## Notable behaviour changes for users - **Editor**: no more blank-window crash on notes with tables (#266) — root cause was \`Decoration.replace\` over multi-line ranges from a ViewPlugin instead of a StateField - **AI keys**: no more silent deletion of keys when keychain is temporarily locked after sleep/wake (#275) - **Backups**: corrupt restored DB is now refused and rolled back to a safety copy (#271) - **Subscriptions**: client now verifies Ed25519 server signatures (#281, #284); server-side signing rollout still needed for full effect - **Tooling**: lefthook replaces husky, knip available for dead-code audits (#267) - **Infrastructure**: full IPC surface now validated with Zod at the boundary (#272 + #273 + #274), Playwright scaffold in place (#277) ## Notable for reviewers - The placeholder \`SUBSCRIPTION_PUBLIC_KEY\` in #281 was replaced with a real key in #284. The matching private key is set in Cloudflare as \`LICENSE_SIGNING_PRIVATE_KEY\` (prod + staging). This keypair was generated in a Claude session and is dev/staging-grade — rotate before serving real paid customers. - 50% of audit findings were Knip false positives or already-fixed (documented per PR). Stack reflects real debt, not the audit verbatim. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added end-to-end testing for the desktop application with comprehensive smoke and notes testing. * Implemented IPC handler validation using Zod schemas for improved type safety. * Added subscription envelope signing and verification using Ed25519 cryptography. * **Bug Fixes** * Improved encryption error handling and recovery logic. * **Tests** * Established centralized test coverage configuration across all packages. * Expanded test suites for markdown commands and licensing functionality. * **Chores** * Transitioned from Husky to Lefthook for Git hooks management. * Refactored internal IPC architecture and service modules. * Updated build and deployment configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Promotes `develop` to `main` for **v0.15.0**. Originally opened 2026-04-24; now refreshed with the 19-PR tech-debt audit shipped via #285 plus the accumulated dependabot bumps reconciled. `semantic-release` will pick the version bump. Expected: **minor (v0.14.x → v0.15.0)** because of multiple \`feat:\` commits. ## What ships (audit highlights, from #285) ### Runtime fixes (user-facing) - **Editor no longer crashes on table-containing notes** (#266) — \`Decoration.replace\` over multi-line ranges moved from a ViewPlugin to a StateField, plus an EditorView.exceptionSink so any future plugin error no longer tears down the EditorView. - **AI keys survive sleep/wake** (#275) — \`aiKeyStorage\` stopped silently deleting the encrypted store when the keychain was temporarily locked after macOS sleep. - **Backup restore is now safe** (#271) — restored DBs go through \`PRAGMA integrity_check\` before being swapped in; corrupt backups roll back to the safety copy. - **MCP server runs without electron-builder rebuilds** (#264 → #270) — migrated from native \`better-sqlite3\` to built-in \`node:sqlite\` (Node 22.5+), updated to the new \`registerTool\` MCP SDK API. ### Security - **Typed IPC boundary** (#272 + #273 + #274) — 130+ IPC channels now validated with Zod tuples at the main↔renderer boundary. Garbage in fails fast with \`IpcValidationError\` instead of corrupting downstream code. - **Ed25519 license verification scaffolding** (#276 + #281 + #284) — \`signSubscriptionPayload\` / \`verifySubscriptionSignature\` helpers ship in \`@readied/licensing\`, wired into \`FileLicenseStorage.readSubscriptionData\` with lenient fallthrough during migration. Real public key embedded (\`d04901…\`). Server-side \`LICENSE_SIGNING_PRIVATE_KEY\` already set in Cloudflare staging + production. ### Developer experience - **Husky → Lefthook** (#267) plus lint-staged that now runs ESLint, not just Prettier. - **\`knip\` added** (#267) + 12 unused files deleted (#279) + 6 unused deps dropped (#280). - **Playwright Electron E2E scaffold** (#277) with smoke + notes-IPC specs and a Linux+xvfb CI job (\`continue-on-error: true\` while it stabilises). - **Vitest coverage baseline** (#269) — 12 packages share a coverage config; smoke tests added for \`@readied/commands\`. ### Refactor (no behavior change) - **Zustand selectors migration** (#268) — 3 components stopped destructuring entire stores. - **God-file extractions**: - \`main/index.ts\` 1065 → 950 lines (#278) — \`FileLicenseStorage\`, \`windowState\` extracted to services - \`SQLiteNoteRepository.ts\` 1121 → 1038 lines (#282) — pure helpers extracted to \`noteMapping.ts\` - \`MarkdownEditor.tsx\` 737 → 612 lines (#283) — theme + markdownHighlighting extracted to \`editorTheme.ts\` ## Deploys triggered | Workflow | Trigger | What happens | |---|---|---| | \`deploy-api.yml\` | Auto on \`push\` to main affecting \`packages/api/**\` | Tests + deploys \`@readied/api\` to Cloudflare Workers (\`readied-api-production\`). This stack only touched \`wrangler.toml\` + \`.dev.vars\` docs, no production code change. | | \`release.yml\` | Manual \`workflow_dispatch\` post-merge | \`semantic-release\` analyses conventional commits, bumps version, creates GitHub Release draft + tag | | \`build.yml\` | Auto on tag push from release.yml | mac / windows / linux parallel builds, artefacts attached to the GitHub Release | ## Pre-merge verification (local, this branch) - ✅ \`pnpm -r typecheck\` — green across 18 workspace projects - ✅ \`pnpm test\` — 17/17 packages - ✅ Merge resolved: take develop versions for 19 conflicted package.jsons (develop has equal or newer deps than main's dependabot bumps) ## Post-merge action items (operator) 1. **Deploy API to staging first** (smoke test): \`\`\` gh workflow run deploy-api.yml -f environment=staging \`\`\` 2. Confirm staging API responds correctly (subscription endpoint with new \`LICENSE_SIGNING_PRIVATE_KEY\` secret already set in CF). 3. Merge this PR → auto-deploys API to production. 4. Trigger Release workflow: GitHub → Actions → Release → "Run workflow" → main. 5. Watch Build workflow for mac/win/linux completion. 6. Confirm the release un-drafts itself. ## Known risks / follow-ups - **Pre-existing Vercel preview failure for \`apps/web\`** — marketing site, scheduled to be extracted to its own repo (P3 in the roadmap). - **\`SUBSCRIPTION_PUBLIC_KEY\` is dev-grade** — generated in a Claude session. Before the licensing server emits envelopes for real paid users, rotate the keypair from a trusted machine and ship a follow-up release. - **Branch protection should require CodeRabbit completion before automerge** — added to the roadmap as a process item; this very PR was BLOCKED correctly because of that policy gap being closed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
Scoped first slice of PR-F. The audit's PR-F bundled IPC validation + keychain (keytar) + HMAC license files into one PR — that's three independent security efforts, each with their own review surface. This PR ships just the typed IPC registry plus a single handler migration to prove the pattern.
The other two slices (keychain credential storage, HMAC license signing) will land as their own PRs.
What this PR introduces
1. `apps/desktop/src/main/ipc/registry.ts`
A small `defineIpcHandler({channel, args, handler})` wrapper around `ipcMain.handle()` with Zod tuple validation at the boundary:
```ts
defineIpcHandler({
channel: 'ai:saveKey',
args: z.tuple([ProviderSchema, ApiKeySchema]),
handler: (provider, apiKey) => aiKeyStorage.saveKey(provider, apiKey),
});
```
Renderer-supplied args are validated before business logic runs. An invalid payload throws `IpcValidationError` at the boundary, so the renderer sees a structured error instead of a downstream crash deep inside storage code.
2. `aiKeyHandlers.ts` migrated as the proof
5 channels migrated. Schemas:
These are bounds-checks, not credential-at-rest hardening (that's the keychain slice).
3. `apps/desktop/package.json`
Adds `zod ^4.4.3` as a direct dep (it was already used in 4 workspace packages but not exposed to desktop main).
What this PR does NOT include
Test plan
Audit linkage
Stack context
PR-F1 in the audit stack. Stacked on top of #271 (PR-I) → #270 (PR-J) → #269 (PR-D) → #268 (PR-H) → #267 (PR-C) → #266 (PR-A) → #265 (PR-B). Retargets to `develop` automatically as predecessors merge.
🤖 Generated with Claude Code