Skip to content

refactor(ipc): migrate heavy data handlers to defineIpcHandler (PR-F5)#274

Merged
github-actions[bot] merged 1 commit into
refactor/migrate-remaining-handlersfrom
refactor/migrate-heavy-handlers
Jun 8, 2026
Merged

refactor(ipc): migrate heavy data handlers to defineIpcHandler (PR-F5)#274
github-actions[bot] merged 1 commit into
refactor/migrate-remaining-handlersfrom
refactor/migrate-heavy-handlers

Conversation

@tomymaritano

@tomymaritano tomymaritano commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

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

  • `pnpm -r typecheck` — green.
  • `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

Summary by CodeRabbit

  • Refactor
    • Enhanced internal data validation across IPC handlers for authentication, synchronization, data management, git operations, notes, and notebooks with centralized validation mechanisms.
    • Reorganized handler registrations to ensure consistent response structures and improved runtime argument validation throughout the application.

PR-F5 — completes the migration started in PR-F1 (#272) and continued in
PR-F4 (#273). Five remaining handler modules, ~97 channels, now run
through the typed IPC registry. The full IPC surface of the desktop app
is now validated at the boundary.

Files:
- notebookHandlers.ts (15 channels): notebooks CRUD + git-per-notebook
  settings. Common `serialize(nb)` helper extracted to avoid 6 copies
  of the same object spread.
- gitHandlers.ts (9 channels): all git ops bounded — short-SHA accepted
  (>=7 hex chars), commit message ≤8 KiB, note file content ≤1 MiB.
- dataHandlers.ts (7 of 8 channels): backup/list/export/exportNote/
  import/paths/openFolder migrated. `data:backup:restore` stays raw
  ipcMain.handle because the integrity-check + rollback state machine
  from PR-I doesn't fit the registry's async wrapper cleanly.
- noteHandlers.ts (32 channels): notes CRUD, tags, links, embeds, stats.
  EmbedTargetSchema is regex-constrained to filename-safe characters
  (no slashes, no path traversal) at the boundary in addition to the
  `join(dataPaths.assets, noteId, target)` guard. ArrayBuffer validated
  via z.instanceof(ArrayBuffer) for embeds:saveAsset.
- authSyncHandlers.ts (33 channels): auth/magic-link, sync (pull/push/
  syncNow/conflicts/auto-sync/history), E2EE key management (setup/
  unlock/migrate/changePassphrase/export/import), subscription portal,
  device list/rename/revoke. SyncChangeSchema is a discriminator-free
  z.object with the operation enum — it's the most complex schema in
  this PR.

After this PR every ipcMain.handle in apps/desktop/src/main/handlers/
either goes through defineIpcHandler with a Zod tuple guarding the args,
or has an explanatory comment for the few exceptions:
- `updates:installNow` (sync quit + window destruction path)
- `plugins:requestReload` (uses ipcMain.on — fire-and-forget, not invoke)
- `data:backup:restore` (integrity-check rollback state machine from #271)

Test plan:
- pnpm -r typecheck — green.
- pnpm test — 17/17 packages.
- Manual: exercise each affected flow for valid inputs; try a malformed
  payload via DevTools (e.g. window.readied.sync.push('not-an-array'))
  and expect an IpcValidationError.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
readide Building Building Preview, Comment Jun 8, 2026 1:37pm

Request Review

@github-actions github-actions Bot merged commit 8f93532 into refactor/migrate-remaining-handlers Jun 8, 2026
4 of 6 checks passed
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f81c67b3-4644-4ca7-8e28-d58695ff0c6a

📥 Commits

Reviewing files that changed from the base of the PR and between dd4823d and 5607100.

📒 Files selected for processing (5)
  • apps/desktop/src/main/handlers/authSyncHandlers.ts
  • apps/desktop/src/main/handlers/dataHandlers.ts
  • apps/desktop/src/main/handlers/gitHandlers.ts
  • apps/desktop/src/main/handlers/noteHandlers.ts
  • apps/desktop/src/main/handlers/notebookHandlers.ts

📝 Walkthrough

Walkthrough

This PR refactors IPC handler registration across five Electron main-process handler modules—auth, sync, data, git, note, and notebook—from direct ipcMain.handle calls to a centralized defineIpcHandler abstraction with Zod-based runtime argument validation. All IPC channels retain their original behavior and response structures while gaining consistent schema validation.

Changes

IPC Handler Abstraction Refactor

Layer / File(s) Summary
Import and Schema Foundation
apps/desktop/src/main/handlers/authSyncHandlers.ts, dataHandlers.ts, gitHandlers.ts, noteHandlers.ts, notebookHandlers.ts
Updated imports to use defineIpcHandler instead of ipcMain.handle, added Zod imports, and defined reusable schemas for IDs, names, content sizes, email/token, crypto values, and git metadata across all modules.
Auth and Session Management
apps/desktop/src/main/handlers/authSyncHandlers.ts
Migrated magic link request/verify, session retrieval (clearing tokens on failure), logout (stopping auto-sync), and token refresh handlers to defineIpcHandler with validated inputs and consistent response shapes.
Sync and Real-time Operations
apps/desktop/src/main/handlers/authSyncHandlers.ts
Converted pull/push (with payload validation), full sync, status, conflict resolution, auto-sync start/stop, tag pull/push, and history handlers to defineIpcHandler with Zod-validated arguments while preserving per-channel result structures.
Encryption Key Management and Devices
apps/desktop/src/main/handlers/authSyncHandlers.ts
Refactored E2EE handlers (key setup, passphrase/recovery unlock with error detection, migration, passphrase change, export/import), subscription handlers (portal session, checkout), and device handlers (list, rename, revoke) to defineIpcHandler with validated inputs and consistent error responses.
Data Backup and Export/Import
apps/desktop/src/main/handlers/dataHandlers.ts
Migrated backup, export (with save dialog and note snapshots), exportNote (with Zod constraints), import (with per-note operation loop), and path handlers to defineIpcHandler while maintaining existing import/export workflows.
Git Handler Refactor
apps/desktop/src/main/handlers/gitHandlers.ts
Converted init, isRepo, commit, log, status, checkout, writeNote, readNote, and deleteNote handlers from ipcMain.handle to defineIpcHandler with Zod-validated notebook/note IDs and commit data, preserving success/error patterns.
Note CRUD and Tag Management
apps/desktop/src/main/handlers/noteHandlers.ts
Refactored note CRUD (create/read/update/delete, pin/unpin, soft delete, status updates, listing/search) and tag management (list/set/get/delete/rename) handlers to defineIpcHandler with Zod validation and manual tag normalization.
Note Links, Graph, and Embed Assets
apps/desktop/src/main/handlers/noteHandlers.ts
Migrated wikilink sync, backlinks, outgoing links, and graph handlers to defineIpcHandler; refactored embed file operations with target resolution, filename restrictions, max size validation, and per-note directory asset storage.
Note Activity Stats and Counts
apps/desktop/src/main/handlers/noteHandlers.ts
Converted notes:activityStats and notes:count endpoints to defineIpcHandler with Zod tuple validation while preserving activity computation, streak logic, and count structure.
Notebook CRUD and Hierarchy Management
apps/desktop/src/main/handlers/notebookHandlers.ts
Refactored notebook CRUD (list/tree/get/create/rename/move/delete/reorder) handlers to defineIpcHandler, introduced serialize() helper for consistent payload formatting, and maintained circular-reference prevention and descendant depth adjustment on move.
Notebook Git Integration
apps/desktop/src/main/handlers/notebookHandlers.ts
Migrated notebook Git handlers (enable/disable, check enabled, retrieve settings, set auto-commit) to defineIpcHandler with Zod validation and standardized { success, ... } responses using serialize() helper.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • tomymaritano/readide#214: This PR refactors the same IPC handler modules introduced in #214, replacing the original ipcMain.handle registration pattern with defineIpcHandler and adding Zod validation across auth, sync, data, git, note, and notebook handlers.

Suggested labels

size/XL, app:desktop

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/migrate-heavy-handlers

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5607100f56

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

async (_event: Electron.IpcMainInvokeEvent, content: string, suggestedName: string) => {
defineIpcHandler({
channel: 'data:exportNote',
args: z.tuple([z.string().max(1024 * 1024), z.string().max(512)]),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Raise exportNote content limit to note maximum

For notes larger than 1 MiB, this new IPC schema rejects data:exportNote before the handler runs, even though the core note limit is 10 MiB and the export plugin passes the full note content through this channel. This means a valid note between 1 MiB and 10 MiB can no longer be exported as Markdown; align this cap with the note content limit or leave size validation to the existing domain/export path.

Useful? React with 👍 / 👎.

// Commit messages can be long but not absurd.
const CommitMessageSchema = z.string().min(1).max(8192);
// Note file content cap matches the share payload cap.
const NoteContentSchema = z.string().max(1024 * 1024);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Raise git writeNote content limit to note maximum

When a git-backed notebook has auto-commit enabled, the renderer calls git:writeNote with the full updated note content, but this new schema rejects any note over 1 MiB while notes are valid up to 10 MiB. In that scenario saving a valid large note still succeeds, but the git write/commit path fails at IPC validation, so the repository is no longer updated for that note.

Useful? React with 👍 / 👎.

github-actions Bot pushed a commit that referenced this pull request Jun 8, 2026
…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>
github-actions Bot pushed a commit that referenced this pull request Jun 8, 2026
…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>
github-actions Bot pushed a commit that referenced this pull request Jun 8, 2026
## 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>
github-actions Bot pushed a commit that referenced this pull request Jun 8, 2026
…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>
github-actions Bot pushed a commit that referenced this pull request Jun 9, 2026
## 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>
github-actions Bot pushed a commit that referenced this pull request Jun 9, 2026
## 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>
github-actions Bot pushed a commit that referenced this pull request Jun 9, 2026
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant