Release: comprehensive audit, design system, UX, and product improvements#202
Closed
tomymaritano wants to merge 182 commits into
Closed
Release: comprehensive audit, design system, UX, and product improvements#202tomymaritano wants to merge 182 commits into
tomymaritano wants to merge 182 commits into
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Notebooks now sync before notes in syncNow() to ensure note-notebook dependencies are satisfied. Adds pullNotebooks/pushNotebooks methods and applyRemoteNotebookChange for bidirectional notebook sync. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move validateNotebookTree from inline test definition to a shared module so it can be reused by the API route and other consumers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add conflict state to SyncStatusIndicator with amber warning icon and count. Conflicts now take priority over idle state so users discover them without navigating to Settings. Also export ConflictResolver from sync components barrel. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DatabaseConnection.transaction() already calls the inner fn — no need for extra () at call site. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix pullNotebooks() to only advance cursor to last successfully applied change (prevents skipping failed changes on retry) - Fix tree validation snapshot to properly exclude deleted notebooks (prevents ghost parent references in validation) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat: add bidirectional notebook sync
test: add sync-core unit tests (62 tests)
feat: surface sync conflicts in status indicator
# Conflicts: # apps/desktop/src/main/services/apiClient.ts # apps/desktop/src/main/services/syncService.ts # packages/api/src/db/schema.ts # packages/api/src/routes/sync.ts # packages/storage-sqlite/src/migrations/index.ts
feat: add bidirectional tag sync
Configure automated code review with path-specific instructions for core, storage, desktop, and API packages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ration Add optional metadata (name, version, priority) to registerRemarkPlugin and registerRehypePlugin signatures for debugging and execution ordering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…improvements
Design System:
- Add Button primitive (primary/secondary/danger/ghost variants, loading state)
- Add Toast notification system (success/error/info/warning, auto-dismiss)
- Add Modal pattern (glass effect, portal, escape/overlay close)
- Add typography tokens (font-weight, line-height, letter-spacing)
UX Improvements:
- Add save indicator in editor ("Saving..." → "Saved")
- Flush pending saves on window close (prevent data loss)
- Show toast notifications for plugin install/uninstall/toggle
- Humanize AI error messages (raw codes → actionable user guidance)
- Add settings "Reset to Defaults" button in sidebar
- Add update notification banner at top of app window
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Design System: - Replace ~60 hardcoded spacing values with --space-* tokens across 14 CSS modules - Consolidate 30+ ad-hoc button styles to Button primitive in all settings sections - Migrate AiSection inline styles to CSS module classes UX Features: - Add welcome screen for first-run onboarding with feature highlights - Add sync progress indicator in sidebar footer (pending count, status, retry) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Search: - Add collapsible filter bar with status pills (All/Active/On Hold/Completed/Dropped) - Add tag filter dropdown populated from existing tags - Add sort controls (by title/created/updated, asc/desc) - Filter toggle button with active count badge Plugin Marketplace: - Wire BrowseTab to fetch plugins from API (https://api.readied.app/plugins) - Graceful offline fallback to static built-in list with notice - Add plugins:installFromUrl IPC handler (download + extract + validate) - Install button with loading state, toast feedback - Cross-reference installed plugins to show "Installed" badge - Auto-refresh installed tab after marketplace install Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tables: - Add comprehensive table CSS (tables.css) for editor/preview parity - Style WYSIWYG editor tables identical to preview (tokens, stripes, hover) - Style insert-table modal with glass effect and accent grid selection - Style sortable table headers with sort indicators - Add overflow-x:auto for wide tables, row hover effects - Remove hardcoded text-align so GFM alignment markers work Export: - Add YAML frontmatter to exported markdown (title, dates, tags, id) - Add per-note file export command (Cmd+Shift+E → save dialog) - Improve HTML clipboard conversion: tables, blockquotes, ordered lists, images, horizontal rules - Add data:exportNote IPC handler with save dialog Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # apps/desktop/package.json # pnpm-lock.yaml
Security: - Fix YAML frontmatter backslash escaping (CodeQL critical) - Validate plugin IDs with regex + path traversal check (P1) - Fix PowerShell path interpolation to use separate args - Add 50MB size limit and try/finally cleanup to installFromUrl - Fix semver regex to accept build metadata Accessibility: - Welcome screen: role=dialog, aria-modal, aria-labelledby, Escape handler - Modal: aria-labelledby, focus on open, tabIndex - Toast: aria-live=assertive for errors, polite for others - Welcome Skip button uses Button primitive UX fixes: - Save indicator no longer flashes on note switch (track noteId) - UpdateBanner handles download errors with retry - UpdatesSection installNow wrapped in try/catch - Welcome copy updated: "Local-First" instead of "Offline Forever" - Hero play overlay hidden when no video URL Code quality: - CSS keyframes renamed to kebab-case (Stylelint) - MagicLinkFlow border-radius uses --radius-xl not --space-4 - Button.module.css currentcolor lowercase - Toast word-break → overflow-wrap - .env.example with parser-safe placeholders - NoteListFilterBar unmount guard + error handling - SidebarFooter prevStatusRef always updated - PluginsSection slug/id matching improved - Stripe timestamp NaN guard - source.ts import order fixed - Test improvements (consistent patterns, comments) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…oad) Security hardening: - encryptionService: enforce minimum 100k PBKDF2 iterations - encryptionService: validate hex strings before Buffer.from() - preload: validate HTTPS-only URLs for plugin install - preload: guard log:write against non-string payloads Sync/encryption: - syncService: emit status event when encryption not ready (UI visibility) - apiClient: add explicit type generics to all E2EE request calls CI: - ci.yml: add contents:read permission to label job Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix mcp-server TS2532 errors from noUncheckedIndexedAccess (optional chaining on regex matches) - Restructure ESLint config: type-aware rules only for src/ files in tsconfig projects - Ignore incomplete ai-assistant package - Result: 0 lint errors, 32 warnings (all non-blocking) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Main process: - exportNote: async writeFile, Unicode-safe filename sanitizer - plugins:install: tmpDir cleanup via finally block - plugins:installFromUrl: slug validation, URL parsing, JSON guard Renderer: - NoteListFilterBar: aria-pressed on pills, error logging in tags fetch - NoteEditor: guard saved flash against note switch (trackedNoteIdRef) - Welcome: auto-focus primary button on mount - UpdateBanner: include error message in UI, check startDownload result - SidebarFooter: "Synced Xm ago" display, subscribe to lastSyncAt - Modal: useId() for unique aria-labelledby - Toast: conditional role (alert vs status) Plugins & export: - PluginsSection: error toast on failed install/uninstall, validate API response - Section.module.css: rename plugin-spinner to pluginSpinner (camelCase) - exportMarkdown: escapeHtml in tables, escapeYamlScalar in frontmatter, async clipboard handlers - Button.module.css: danger hover uses color-mix tokens - Hero.tsx: non-interactive wrapper when no video URL Other: - .env.example: inline comments moved to own lines - bump-version.js: stricter semver regex Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # .env.example # apps/desktop/src/main/index.ts # apps/desktop/src/renderer/components/NoteEditor.tsx # apps/desktop/src/renderer/components/NoteListFilterBar.tsx # apps/desktop/src/renderer/components/UpdateBanner.tsx # apps/desktop/src/renderer/components/Welcome.tsx # apps/desktop/src/renderer/components/sidebar/SidebarFooter.tsx # apps/desktop/src/renderer/pages/settings/sections/PluginsSection.tsx # apps/desktop/src/renderer/pages/settings/sections/Section.module.css # apps/desktop/src/renderer/plugins/exportMarkdown.ts # apps/desktop/src/renderer/ui/patterns/Modal.tsx # apps/desktop/src/renderer/ui/primitives/Button.module.css # apps/desktop/src/renderer/ui/primitives/Toast.tsx # apps/web/components/landing/Hero.tsx # pnpm-lock.yaml # scripts/bump-version.js
## Summary - Add explicit rule: never push directly to `develop` or `main` - Add instruction for Claude Code to always create feature branches - Add "always branch from develop" and "clean up after merge" to hygiene rules 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **No user-facing changes** * Internal documentation update to development workflow and branch governance guidelines. This does not affect end-user functionality or features. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Summary Fixes two review findings from PR #198: - **P1**: Move `useId()` above early return in Modal to prevent React hook order violation - **P2**: Remove strict slug/id equality check in `plugins:installFromUrl` — marketplace slug and manifest id are independent identifiers ## Test plan - [x] `pnpm typecheck` — 17/17 pass - [x] `pnpm test` — 16/16 pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Summary Addresses P1 review finding from PR #200 and strengthens git flow rules. ### Plugin Install - Add cross-plugin overwrite protection: when marketplace serves wrong bundle (manifest.id ≠ requested slug), block only if it would overwrite an existing plugin directory — prevents silent overwrites without blocking valid installs where slug and manifest ID legitimately differ ### CLAUDE.md - Add explicit step-by-step workflow for Claude Code - Add explicit NEVER list (no direct commits to develop/main) ## Test plan - [x] `pnpm typecheck` — 17/17 pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Enhanced plugin installation safety to prevent accidental overwrites of existing plugins during installation from URL. * **Documentation** * Updated development workflow documentation and branch protection requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 32 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR makes configuration, dependency, UI/UX, and plugin handling improvements across desktop and web applications, along with documentation and build system updates. Changes include expanding Claude command permissions, clearing sensitive environment variables, enforcing Git workflow constraints, downgrading Electron, improving plugin validation, enhancing accessibility features, refining error messaging, and updating TypeScript/Next.js configurations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 321da87b9d
ℹ️ 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".
| } | ||
| prevDirtyRef.current = isDirty; | ||
| }, [isDirty]); | ||
| }, [isDirty, note?.id]); |
There was a problem hiding this comment.
Remove note-switch trigger from save-transition effect
Including note?.id in this effect’s dependency list makes it run during note switches, at a point where isDirty can still reflect the previous note until the buffer-reset effect flushes. In that sequence, prevDirtyRef is set to true for the new note, and when the new note’s dirty state becomes false it incorrectly shows the "Saved" indicator even though nothing was saved for that note. This is a regression in save-status accuracy when switching notes while the previous note is dirty.
Useful? React with 👍 / 👎.
## Summary Resolves merge conflicts between main and develop for the release PR #202: - `apps/desktop/src/main/index.ts`: Keep develop's cross-plugin overwrite protection (smarter than main's strict equality check) - `apps/desktop/src/renderer/ui/patterns/Modal.tsx`: Keep develop's useId() placement above early return (React hook order fix) 🤖 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>
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/components/NoteEditor.tsx (1)
129-149: 🧹 Nitpick | 🔵 TrivialGuard on Line 138 is effectively dead code due to effect ordering.
Both effects declare
[note?.id]as a dependency and React runs them in declaration order. By the time the second effect (Line 136) executes after a note switch, the first effect (Line 129) has already settrackedNoteIdRef.current = note?.idand resetprevDirtyRef.current = false. Consequently, thetrackedNoteIdRef.current !== note?.idcheck on Line 138 will never evaluate true in the note-switch path — the early-return branch is unreachable.The net behavior is still correct (no false "Saved" flash on switch, because
prevDirtyRef.currentwas reset tofalse), but the guard gives a misleading impression of defensive logic. Consider consolidating into a single effect or removing the redundant guard.♻️ Suggested simplification
- useEffect(() => { - if (trackedNoteIdRef.current !== note?.id) { - trackedNoteIdRef.current = note?.id; - prevDirtyRef.current = false; - } - }, [note?.id]); - useEffect(() => { - // Guard: if note switched, just sync the ref and bail out - if (trackedNoteIdRef.current !== note?.id) { - prevDirtyRef.current = isDirty; - return; - } if (prevDirtyRef.current && !isDirty) { setShowSaved(true); if (savedTimerRef.current) clearTimeout(savedTimerRef.current); savedTimerRef.current = setTimeout(() => setShowSaved(false), 1500); } prevDirtyRef.current = isDirty; - }, [isDirty, note?.id]); + }, [isDirty, note?.id]); + + // Reset dirty tracking when switching notes to avoid false "Saved" flash + useEffect(() => { + prevDirtyRef.current = false; + }, [note?.id]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NoteEditor.tsx` around lines 129 - 149, The two useEffect blocks create redundant ordering-dependent logic; consolidate them into a single useEffect in NoteEditor that depends on [isDirty, note?.id]: at the top, detect a note switch by comparing trackedNoteIdRef.current !== note?.id, update trackedNoteIdRef.current = note?.id and reset prevDirtyRef.current = false (and bail out early to avoid a "Saved" flash), otherwise run the existing transition check (if prevDirtyRef.current && !isDirty then setShowSaved(true), clear/set savedTimerRef), and finally set prevDirtyRef.current = isDirty; remove the separate useEffect that only synced trackedNoteIdRef/prevDirtyRef.apps/desktop/src/renderer/components/NoteListFilterBar.tsx (1)
56-62:⚠️ Potential issue | 🟡 MinorMinor: on tag-load failure,
tagsremains[]with no user-visible signal.
console.erroris an improvement over silent swallow, but the select will just show "All tags" as if no tags exist. Consider surfacing a toast or retry affordance so users don't mistake an IPC failure for an empty tag set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NoteListFilterBar.tsx` around lines 56 - 62, The tag-loading useEffect in NoteListFilterBar currently only logs errors so the UI still shows "All tags" and users get no feedback; update the effect's .catch handler to set a small error state (e.g., tagsLoadError) and surface a user-visible signal (either call your app's toast helper like enqueueToast/showToast or render a retry affordance in the NoteListFilterBar UI), and add a retry function (e.g., reloadTags) that re-runs the same load logic and clears tagsLoadError; reference the existing NoteListFilterBar component, the useEffect that sets cancelled and calls setTags, and the cancelled/setTags variables so the .catch will set tagsLoadError and trigger the toast or show the retry button.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.local.json:
- Around line 81-82: Remove the user-specific absolute path allowances in
.claude/settings.local.json (the entries containing "Bash(ls
/Users/tomasmaritano/Documents/Github/readied/readide/...)" referencing
NoteListFilterBar* and *.module.css); either delete these lines or replace them
with repo-relative patterns (e.g., paths starting with ./ or relative
directories) so the glob matches across all contributors and not just the
author’s machine—search for the exact strings "Bash(ls
/Users/tomasmaritano/Documents/Github/readied/readide/..." to locate and update
each occurrence.
In @.env.example:
- Around line 9-25: Add a brief comment to .env.example clarifying that
STRIPE_PRICE_MONTHLY and STRIPE_PRICE_ANNUAL must be Stripe Price IDs (format:
price_xxxxx) or add a top-of-file pointer to packages/api/SETUP.md for full
setup instructions; reference the STRIPE_PRICE_MONTHLY and STRIPE_PRICE_ANNUAL
variables so consumers know to copy the price IDs from the Stripe Dashboard and
not to use product IDs or human-readable names.
In `@apps/desktop/src/main/index.ts`:
- Around line 2505-2515: The handler currently only rejects a mismatched
manifest.id vs pluginSlug when join(paths.plugins, manifest.id) exists; change
this so any manifest.id !== pluginSlug causes the install to fail immediately.
In other words, in the block that checks if (pluginSlug && manifest.id !==
pluginSlug) (and uses join(paths.plugins, manifest.id) and existsSync), remove
the conditional dependence on existsSync and always return the error object
(with the same message) when the ids differ so we never silently install a
different plugin than the requested pluginSlug.
- Around line 1252-1258: The sanitized safeName (from suggestedName) can still
equal Windows reserved device names (CON, PRN, AUX, NUL, COM1..COM9, LPT1..LPT9)
and thus produce unopenable files; after computing safeName, create a finalName
and if finalName (case-insensitive) matches any reserved base name, alter it
(e.g., append an underscore or numeric suffix) to avoid the reserved token, then
use finalName instead of safeName in the defaultPath construction; reference the
variables safeName, finalName, suggestedName and the defaultPath usage when
making this change.
In `@apps/desktop/src/renderer/components/sidebar/SidebarFooter.tsx`:
- Around line 148-156: The "Synced Xm ago" text is computed once at render via
formatRelativeTime(lastSyncAt) and can become stale; add a tick state and a
useEffect that, when lastSyncAt is non-null, starts a setInterval (e.g., 30–60s)
to increment the tick state to force re-renders while the idle branch is
visible, and clear the interval on cleanup; update the SidebarFooter component
to include the new tick state/useEffect so formatRelativeTime(lastSyncAt)
refreshes periodically.
In `@apps/desktop/src/renderer/components/UpdateBanner.tsx`:
- Around line 68-78: In UpdateBanner, when handling a non-OK result from
startDownload() you currently set state.message to the literal "Download
failed", which the render layer also prefixes, producing "Download failed:
Download failed"; update the failure branch inside the startDownload() result
handler (the setState call) to either leave message undefined/empty (so the
render's "Download failed" prefix is shown alone) or propagate a descriptive
error from result (e.g., result.error) instead; locate the state update in the
startDownload() callback and adjust the setState payload for the 'error' kind
accordingly.
- Around line 48-56: The handler passed to window.readied.updates.onError
currently types its parameter as (err: { message: string }) but uses
err?.message defensively; update the parameter type to reflect reality by
changing it to (err?: { message?: string }) so both the error object and its
message are optional, and then keep the optional chaining in the setState call
(or alternatively remove the optional chaining and keep the stricter type if you
guarantee non-null from the sender); locate the handler in UpdateBanner.tsx (the
window.readied.updates.onError callback and the setState usage) and make the
type change there to align type annotations with runtime checks.
In `@apps/desktop/src/renderer/pages/settings/sections/PluginsSection.tsx`:
- Around line 224-231: The filter used to narrow data.plugins into validated
MarketplacePlugin items only checks name, description, and tags but omits slug
(which is used as React key and for install/matching), making the type predicate
unsound; update the filter in the block that builds validated (the
Array.prototype.filter on data.plugins that currently returns (p): p is
MarketplacePlugin => ...) to also assert typeof p.slug === 'string' (and any
other required string fields from the MarketplacePlugin type), then call
setMarketplacePlugins(validated) as before so downstream code (installFromUrl,
installedPluginIds checks, and React key usage) gets properly-typed items.
In `@apps/desktop/src/renderer/pages/settings/sections/Section.module.css`:
- Around line 653-656: Rename the keyframes identifier from pluginSpin to a
kebab-case name (e.g., plugin-spin) to satisfy stylelint; update the `@keyframes`
rule name and any references to it (for example where .pluginSpinner uses
animation or animation-name) so the animation still applies, ensuring the CSS
module class .pluginSpinner remains unchanged but points to the new keyframe
name.
In `@apps/desktop/src/renderer/ui/primitives/Toast.tsx`:
- Around line 37-41: Remove the nested live-region causing duplicate
announcements: in the Toaster component remove the container's
aria-live="polite" so the top-level container is not a live region, and in
ToastNotification keep the per-item role logic (item.type === 'error' ? 'alert'
: 'status') but remove the redundant explicit aria-live attribute from the inner
div (the div using className={cls}) since role implies the appropriate
politeness.
In `@apps/web/tsconfig.json`:
- Line 14: The tsconfig.json currently sets "jsx": "react-jsx", which conflicts
with Next.js App Router/SWC; change the JSX setting to "preserve" so Next.js/SWC
handles JSX transforms. Locate the "jsx" compiler option in the
apps/web/tsconfig.json (the entry currently showing "jsx": "react-jsx") and
replace its value with "preserve" to remove TypeScript/VS Code warnings and
avoid transform conflicts.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/NoteEditor.tsx`:
- Around line 129-149: The two useEffect blocks create redundant
ordering-dependent logic; consolidate them into a single useEffect in NoteEditor
that depends on [isDirty, note?.id]: at the top, detect a note switch by
comparing trackedNoteIdRef.current !== note?.id, update trackedNoteIdRef.current
= note?.id and reset prevDirtyRef.current = false (and bail out early to avoid a
"Saved" flash), otherwise run the existing transition check (if
prevDirtyRef.current && !isDirty then setShowSaved(true), clear/set
savedTimerRef), and finally set prevDirtyRef.current = isDirty; remove the
separate useEffect that only synced trackedNoteIdRef/prevDirtyRef.
In `@apps/desktop/src/renderer/components/NoteListFilterBar.tsx`:
- Around line 56-62: The tag-loading useEffect in NoteListFilterBar currently
only logs errors so the UI still shows "All tags" and users get no feedback;
update the effect's .catch handler to set a small error state (e.g.,
tagsLoadError) and surface a user-visible signal (either call your app's toast
helper like enqueueToast/showToast or render a retry affordance in the
NoteListFilterBar UI), and add a retry function (e.g., reloadTags) that re-runs
the same load logic and clears tagsLoadError; reference the existing
NoteListFilterBar component, the useEffect that sets cancelled and calls
setTags, and the cancelled/setTags variables so the .catch will set
tagsLoadError and trigger the toast or show the retry button.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b09165aa-31e4-4810-bad5-fcb10a3d6670
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.claude/settings.local.json.env.exampleCLAUDE.mdapps/desktop/package.jsonapps/desktop/src/main/index.tsapps/desktop/src/renderer/components/NoteEditor.tsxapps/desktop/src/renderer/components/NoteListFilterBar.tsxapps/desktop/src/renderer/components/UpdateBanner.tsxapps/desktop/src/renderer/components/Welcome.tsxapps/desktop/src/renderer/components/sidebar/SidebarFooter.tsxapps/desktop/src/renderer/pages/settings/sections/PluginsSection.tsxapps/desktop/src/renderer/pages/settings/sections/Section.module.cssapps/desktop/src/renderer/plugins/exportMarkdown.tsapps/desktop/src/renderer/ui/patterns/Modal.tsxapps/desktop/src/renderer/ui/primitives/Button.module.cssapps/desktop/src/renderer/ui/primitives/Toast.tsxapps/web/components/landing/Hero.tsxapps/web/next-env.d.tsapps/web/tsconfig.jsonscripts/bump-version.js
Comment on lines
+81
to
+82
| "Bash(ls /Users/tomasmaritano/Documents/Github/readied/readide/apps/desktop/src/renderer/components/NoteListFilterBar*)", | ||
| "Bash(ls /Users/tomasmaritano/Documents/Github/readied/readide/apps/desktop/src/renderer/components/*.module.css)", |
There was a problem hiding this comment.
User-specific absolute paths committed to shared config.
The ls allowances hard-code /Users/tomasmaritano/Documents/Github/readied/readide/.... These patterns only match the author's machine and are dead allowances for every other contributor. Either drop them or make them repo-relative. The same issue already exists in earlier lines (e.g., Line 13, 42–44, 71–73) but these new entries compound it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.local.json around lines 81 - 82, Remove the user-specific
absolute path allowances in .claude/settings.local.json (the entries containing
"Bash(ls /Users/tomasmaritano/Documents/Github/readied/readide/...)" referencing
NoteListFilterBar* and *.module.css); either delete these lines or replace them
with repo-relative patterns (e.g., paths starting with ./ or relative
directories) so the glob matches across all contributors and not just the
author’s machine—search for the exact strings "Bash(ls
/Users/tomasmaritano/Documents/Github/readied/readide/..." to locate and update
each occurrence.
Comment on lines
+9
to
+25
| TURSO_DATABASE_URL= | ||
| TURSO_AUTH_TOKEN= | ||
| # Generate with: openssl rand -base64 32 | ||
| JWT_SECRET= | ||
| # Resend email service API key | ||
| RESEND_API_KEY= | ||
| SITE_URL=https://readied.app | ||
|
|
||
| # ─── Stripe ───────────────────────────────────────────────── | ||
| STRIPE_SECRET_KEY=sk_test_your_key_here | ||
| STRIPE_WEBHOOK_SECRET=whsec_your_secret_here | ||
| STRIPE_PRICE_MONTHLY=price_your_monthly_id_here | ||
| STRIPE_PRICE_ANNUAL=price_your_annual_id_here | ||
| STRIPE_SECRET_KEY= | ||
| STRIPE_WEBHOOK_SECRET= | ||
| STRIPE_PRICE_MONTHLY= | ||
| STRIPE_PRICE_ANNUAL= | ||
|
|
||
| # ─── Admin ────────────────────────────────────────────────── | ||
| ADMIN_TOKEN=your_admin_token_here # Token for /admin endpoints | ||
| # Token for /admin endpoints | ||
| ADMIN_TOKEN= |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if setup documentation provides guidance for these environment variables.
# Search for documentation files that might explain environment setup
fd -e md -e txt | head -20
# Check if README mentions environment variables or setup
rg -i "environment|env|setup|configuration" README.md -C 2 || echo "README.md not found or no environment setup section"
# Check for dedicated setup/installation docs
fd -g '*setup*' -g '*install*' -g '*getting*started*' -e md -e txtRepository: tomymaritano/readide
Length of output: 894
🏁 Script executed:
# Check README.md content (if it exists) for environment setup guidance
cat README.md 2>/dev/null | head -100
# Check if CONTRIBUTING.md has setup instructions
cat CONTRIBUTING.md 2>/dev/null | grep -A 20 -i "setup\|environment\|install\|getting started" || echo "No matching content in CONTRIBUTING.md"Repository: tomymaritano/readide
Length of output: 2616
🏁 Script executed:
# Check if there's a setup guide or development guide in docs directory
fd . docs -name "*.md" -type f | xargs -I {} bash -c 'echo "=== {} ===" && head -5 {}'Repository: tomymaritano/readide
Length of output: 294
🏁 Script executed:
# Search for any mentions of Turso or Stripe setup in documentation
rg -i "turso|stripe" --type mdRepository: tomymaritano/readide
Length of output: 9199
🏁 Script executed:
# Search for STRIPE_PRICE documentation
rg "STRIPE_PRICE" --type md packages/api/
# Check if SETUP.md covers Stripe price IDs
cat packages/api/SETUP.mdRepository: tomymaritano/readide
Length of output: 3089
🏁 Script executed:
# Check if STRIPE_PRICE variables are documented anywhere
rg "STRIPE_PRICE" --type md
# Check if they're used in the codebase
rg "STRIPE_PRICE_MONTHLY|STRIPE_PRICE_ANNUAL" --type ts --type jsRepository: tomymaritano/readide
Length of output: 548
🏁 Script executed:
# Verify what the actual .env.example contains currently
cat .env.exampleRepository: tomymaritano/readide
Length of output: 1268
🏁 Script executed:
# Check git history to see what the actual changes were (before/after comparison)
git log --oneline .env.example | head -5
# Get the actual diff for this file
git diff HEAD~1 .env.example 2>/dev/null || git show HEAD:.env.example 2>/dev/nullRepository: tomymaritano/readide
Length of output: 1325
🏁 Script executed:
# Check the git log with more detail
git log -p .env.example | head -200Repository: tomymaritano/readide
Length of output: 46
Good security practice clearing placeholder values.
Removing example text and using empty strings makes it clear these must be configured before use.
Setup documentation already exists in packages/api/SETUP.md with comprehensive guidance for Turso and Stripe webhook configuration, so the Turso credentials and Stripe webhook secret are well-covered. However, STRIPE_PRICE_MONTHLY and STRIPE_PRICE_ANNUAL (lines 20–21) are used in the code but lack documentation anywhere—consider adding a comment clarifying these should be Stripe Price IDs from the Stripe Dashboard (format: price_xxxxx).
Alternatively, consider adding a comment at the top of .env.example directing developers to packages/api/SETUP.md for detailed configuration guidance instead of duplicating information.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 10-10: [UnorderedKey] The TURSO_AUTH_TOKEN key should go before the TURSO_DATABASE_URL key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The JWT_SECRET key should go before the TURSO_AUTH_TOKEN key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The RESEND_API_KEY key should go before the TURSO_AUTH_TOKEN key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The SITE_URL key should go before the TURSO_AUTH_TOKEN key
(UnorderedKey)
[warning] 20-20: [UnorderedKey] The STRIPE_PRICE_MONTHLY key should go before the STRIPE_SECRET_KEY key
(UnorderedKey)
[warning] 21-21: [UnorderedKey] The STRIPE_PRICE_ANNUAL key should go before the STRIPE_PRICE_MONTHLY key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 9 - 25, Add a brief comment to .env.example
clarifying that STRIPE_PRICE_MONTHLY and STRIPE_PRICE_ANNUAL must be Stripe
Price IDs (format: price_xxxxx) or add a top-of-file pointer to
packages/api/SETUP.md for full setup instructions; reference the
STRIPE_PRICE_MONTHLY and STRIPE_PRICE_ANNUAL variables so consumers know to copy
the price IDs from the Stripe Dashboard and not to use product IDs or
human-readable names.
Comment on lines
+2505
to
+2515
| // Cross-plugin overwrite protection: if we requested plugin A but the | ||
| // archive contains plugin B, block when it would overwrite an existing plugin | ||
| if (pluginSlug && manifest.id !== pluginSlug) { | ||
| const wouldOverwrite = join(paths.plugins, manifest.id); | ||
| if (existsSync(wouldOverwrite)) { | ||
| return { | ||
| success: false, | ||
| error: `Archive contains "${manifest.id}" but "${pluginSlug}" was requested. Refusing to overwrite existing plugin.`, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Mismatched manifest.id vs pluginSlug is only blocked on overwrite — otherwise the wrong plugin is silently installed.
The guard at Lines 2507–2515 only refuses when manifest.id !== pluginSlug and a plugin with manifest.id already exists on disk. When no conflict exists, the handler proceeds to install plugin manifest.id even though the caller requested pluginSlug. The marketplace UI then keeps rendering the "Install" button for the requested slug (since installedPluginIds only contains scanned ids), and a completely different plugin ends up installed from a marketplace-served URL — arguably a trust-boundary violation: a malicious/mistyped bundleUrl can register a different plugin id than what the UI advertised.
Recommend failing closed whenever the manifest id doesn't match the requested slug, regardless of whether a conflict exists.
🛡️ Proposed fix
- // Cross-plugin overwrite protection: if we requested plugin A but the
- // archive contains plugin B, block when it would overwrite an existing plugin
- if (pluginSlug && manifest.id !== pluginSlug) {
- const wouldOverwrite = join(paths.plugins, manifest.id);
- if (existsSync(wouldOverwrite)) {
- return {
- success: false,
- error: `Archive contains "${manifest.id}" but "${pluginSlug}" was requested. Refusing to overwrite existing plugin.`,
- };
- }
- }
+ // Require the archive's manifest id to match the requested slug to
+ // prevent a bundleUrl from sneaking a different plugin past the UI.
+ if (pluginSlug && manifest.id !== pluginSlug) {
+ return {
+ success: false,
+ error: `Archive contains "${manifest.id}" but "${pluginSlug}" was requested.`,
+ };
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/index.ts` around lines 2505 - 2515, The handler
currently only rejects a mismatched manifest.id vs pluginSlug when
join(paths.plugins, manifest.id) exists; change this so any manifest.id !==
pluginSlug causes the install to fail immediately. In other words, in the block
that checks if (pluginSlug && manifest.id !== pluginSlug) (and uses
join(paths.plugins, manifest.id) and existsSync), remove the conditional
dependence on existsSync and always return the error object (with the same
message) when the ids differ so we never silently install a different plugin
than the requested pluginSlug.
Comment on lines
+68
to
+78
| if (!result.ok) { | ||
| setState(prev => | ||
| prev.kind === 'hidden' | ||
| ? prev | ||
| : { | ||
| kind: 'error', | ||
| version: (prev as { version: string }).version, | ||
| message: 'Download failed', | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Duplicated "Download failed" text when startDownload() returns { ok: false }.
When startDownload() resolves with a non-OK result, you set message: 'Download failed' (Line 75), and the render layer at Lines 115–117 prepends the literal Download failed prefix, producing Download failed: Download failed. Either omit the message in the non-throwing branch (letting the bare Download failed prefix stand alone) or provide a more descriptive message here (e.g., propagate result.error if the IPC contract is extended to include it).
🛠️ Proposed fix
- if (!result.ok) {
- setState(prev =>
- prev.kind === 'hidden'
- ? prev
- : {
- kind: 'error',
- version: (prev as { version: string }).version,
- message: 'Download failed',
- }
- );
- }
+ if (!result.ok) {
+ setState(prev =>
+ prev.kind === 'hidden'
+ ? prev
+ : {
+ kind: 'error',
+ version: (prev as { version: string }).version,
+ }
+ );
+ }Also applies to: 115-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/UpdateBanner.tsx` around lines 68 - 78,
In UpdateBanner, when handling a non-OK result from startDownload() you
currently set state.message to the literal "Download failed", which the render
layer also prefixes, producing "Download failed: Download failed"; update the
failure branch inside the startDownload() result handler (the setState call) to
either leave message undefined/empty (so the render's "Download failed" prefix
is shown alone) or propagate a descriptive error from result (e.g.,
result.error) instead; locate the state update in the startDownload() callback
and adjust the setState payload for the 'error' kind accordingly.
Comment on lines
+37
to
+41
| <div | ||
| className={cls} | ||
| role={item.type === 'error' ? 'alert' : 'status'} | ||
| aria-live={item.type === 'error' ? 'assertive' : 'polite'} | ||
| > |
There was a problem hiding this comment.
Nested live regions may cause double announcements.
Toaster (Line 61) already declares aria-live="polite" on the container, and each ToastNotification now declares its own role/aria-live. Nested ARIA live regions are not well-defined by the spec and some screen readers (notably NVDA/JAWS) announce both, producing duplicate or garbled output. Additionally, role="alert" implies aria-live="assertive" and role="status" implies aria-live="polite", so the explicit aria-live on Line 40 is redundant.
Recommend either:
- Drop
aria-livefrom theToastercontainer and keep the per-item roles (preferred — allows per-type politeness), or - Keep the container live region and drop role/aria-live from the inner div.
♻️ Suggested fix (remove container live region)
- <div className={styles.container} aria-live="polite">
+ <div className={styles.container}>
{toasts.map(item => (
<ToastNotification key={item.id} item={item} />
))}
</div>And drop the redundant explicit aria-live on the inner element:
- <div
- className={cls}
- role={item.type === 'error' ? 'alert' : 'status'}
- aria-live={item.type === 'error' ? 'assertive' : 'polite'}
- >
+ <div
+ className={cls}
+ role={item.type === 'error' ? 'alert' : 'status'}
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| className={cls} | |
| role={item.type === 'error' ? 'alert' : 'status'} | |
| aria-live={item.type === 'error' ? 'assertive' : 'polite'} | |
| > | |
| <div | |
| className={cls} | |
| role={item.type === 'error' ? 'alert' : 'status'} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/ui/primitives/Toast.tsx` around lines 37 - 41,
Remove the nested live-region causing duplicate announcements: in the Toaster
component remove the container's aria-live="polite" so the top-level container
is not a live region, and in ToastNotification keep the per-item role logic
(item.type === 'error' ? 'alert' : 'status') but remove the redundant explicit
aria-live attribute from the inner div (the div using className={cls}) since
role implies the appropriate politeness.
2 tasks
## Summary Fixes all 12 review comments from PR #202 (CodeRabbit + Codex). - **NoteEditor**: Remove `note?.id` from save effect deps (P2 — prevents false "Saved" flash) - **exportNote**: Reject Windows reserved filenames (CON, PRN, AUX, etc.) - **installFromUrl**: Return `slugMismatch` in success response for UI awareness - **SidebarFooter**: 60s interval refresh for "Synced Xm ago" staleness - **UpdateBanner**: Fix optional error type + "Download failed: Download failed" duplication - **PluginsSection**: Add `slug` to marketplace response validation - **Section.module.css**: Keyframe `pluginSpin` → `plugin-spin` (stylelint) - **Toast**: Remove nested `aria-live` from container (spec compliance) - **tsconfig.json**: `jsx: "preserve"` for Next.js (was "react-jsx") - **.env.example**: Reference to CLAUDE.md for setup docs ## Test plan - [x] `pnpm typecheck` — 17/17 pass - [x] `pnpm test` — 16/16 pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Fixed handling of Windows reserved filenames in exports * Download failures now display specific error messages instead of generic text * Improved validation when installing marketplace plugins * Sync timestamp display now updates periodically * Enhanced accessibility for toast notifications <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 task
Collaborator
Author
|
Superseded: will reopen after syncing develop with main via #205 |
tomymaritano
added a commit
that referenced
this pull request
Apr 23, 2026
## Summary Merges main into develop to resolve all conflicts so PR #202 (release) can merge cleanly. All 5 conflicts resolved keeping develop's version (which has all review fixes from PRs #199-#204). ## Test plan - [x] `pnpm typecheck` — 17/17 pass 🤖 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>
This was referenced Apr 23, 2026
tomymaritano
added a commit
that referenced
this pull request
Apr 23, 2026
#208) ## Release Summary Major release with all review findings resolved and conflicts synced. ### Highlights - Design system primitives (Button, Toast, Modal) - Advanced search filters + functional plugin marketplace - High-quality table rendering (editor = preview parity) - Document export with YAML frontmatter + per-note export - Welcome screen, save indicator, update banner, sync progress - Security hardening (plugin install, encryption, YAML/HTML escaping) - A11y improvements (aria-pressed, focus management, live regions) - ESLint typed linting (0 errors), TypeScript strict across all 16 packages - All review findings from PRs #194, #198, #200, #202 addressed ## Test plan - [x] `pnpm typecheck` — 17/17 pass - [x] `pnpm test` — 16/16 pass - [x] `pnpm lint` — 0 errors - [x] Conflicts with main resolved in merge commit 🤖 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release Summary
Major release combining infrastructure hardening, design system foundation, UX improvements, and new features.
Infrastructure
Security
Design System
New Features
UX Improvements
Git Flow
Test plan
pnpm typecheck— 17/17 passpnpm test— 16/16 tasks passpnpm lint— 0 errorspnpm build— all packages build🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Improvements