feat: core features#1
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR establishes the initial OpenPaste monorepo workspace, shared Rust crates, daemon and desktop applications, IPC and platform layers, and the first set of project documentation. ChangesOpenPaste Project Scaffold
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant DaemonMain
participant IpcServer
participant PlatformProvider
participant Database
participant EventBus
DaemonMain->>IpcServer: start(handler)
DaemonMain->>PlatformProvider: get_content()
PlatformProvider-->>DaemonMain: ClipboardItem
DaemonMain->>Database: insert_item(item)
DaemonMain->>EventBus: publish(ClipboardAdded)
IpcServer-->>DaemonMain: handle client request
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
apps/desktop/src-tauri/src/main.rs-19-31 (1)
19-31: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFallback to current directory for socket path is unreliable.
PathBuf::from(".")falls back to the process's current working directory, which is unpredictable for a desktop app (could be anywhere). Ifdata_local_dir()returnsNone, the IPC connection will silently fail or create a socket in an unexpected location.Return an error or use a more deterministic fallback like the temp directory.
🔒 Proposed fix: use temp dir fallback
fn get_ipc_socket_path() -> PathBuf { let data_dir = dirs::data_local_dir() - .unwrap_or_else(|| PathBuf::from(".")) + .or_else(|| dirs::temp_dir()) + .unwrap_or_else(|| PathBuf::from("/tmp")) .join("openpaste");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src-tauri/src/main.rs` around lines 19 - 31, The fallback in get_ipc_socket_path() is using PathBuf::from(".") when dirs::data_local_dir() returns None, which can place the IPC socket in an unpredictable working directory. Update get_ipc_socket_path() to use a deterministic fallback such as the temp directory (or return an error instead of silently defaulting), and keep the platform-specific socket naming for openpaste.sock/openpaste.pipe unchanged.apps/desktop/src/App.tsx-69-71 (1)
69-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError is cast to
stringbut Tauri errors are objects.
e as stringwill render as[object Object]in the error UI. Use(e as Error).messageorString(e)to display a meaningful message.🐛 Proposed fix
- setError(e as string) + setError(e instanceof Error ? e.message : String(e))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/App.tsx` around lines 69 - 71, The catch block in App should not cast the Tauri error to string because the thrown value is usually an object, which breaks the UI display. Update the error handling in the App component’s catch path to extract a meaningful message from the caught value, using e.message when it is an Error or falling back to String(e), and pass that to setError while keeping the existing console.error logging.docs/ARCHITECTURE.md-597-642 (1)
597-642: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the dependency graph with the actual workspace.
This still lists
clipboard-utilsandclipboard-index, but the workspace manifest only definesclipboard-coreandclipboard-search. Please rename/remove the stale nodes so the doc matches what contributors will actually build.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ARCHITECTURE.md` around lines 597 - 642, The dependency graph in ARCHITECTURE.md is out of sync with the workspace: it still includes stale nodes like clipboard-utils and clipboard-index that are no longer defined. Update the diagram to match the actual packages by removing or renaming those entries and adjusting the downstream relationships in the Dependency Graph section so only current workspace symbols such as clipboard-core and clipboard-search remain.docs/CONTRIBUTING.md-425-436 (1)
425-436: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSwap out the Vue-specific editor recommendation.
Volaris a Vue plugin, but this repo's desktop UI is React/TypeScript. Replace it with a React-oriented extension or drop the item to avoid sending contributors down the wrong path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/CONTRIBUTING.md` around lines 425 - 436, The VS Code extension list includes a Vue-specific recommendation that doesn’t match this repository’s React/TypeScript desktop UI. Update the section in CONTRIBUTING.md by replacing the TypeScript extension item that mentions TypeScript Vue Plugin (Volar) with a React-appropriate recommendation or removing it entirely, keeping the rest of the editor setup guidance unchanged.docs/SECURITY.md-151-158 (1)
151-158: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winMake the encryption default explicit.
This section says encryption is "optional but recommended", but the release checklist later requires "encryption enabled by default". Please reconcile those statements so contributors know the intended default posture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/SECURITY.md` around lines 151 - 158, The Secure Defaults section in SECURITY.md conflicts with the release checklist because it describes encryption as optional while the intended default is encryption enabled. Update the wording in the relevant security guidance to make the default encryption posture explicit and consistent across the document, especially in the Secure Defaults list and any nearby references to configuration defaults.docs/ROADMAP.md-20-25 (1)
20-25: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRefresh the timeline before merging.
These phase dates are already hard-coded across 2024–2026, so the roadmap reads as stale immediately. Consider relative milestones or updated dates so the document reflects the current plan.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ROADMAP.md` around lines 20 - 25, The roadmap timeline in the Phase 1 section is stale and hard-coded to old dates, so update the timeline wording in ROADMAP.md to reflect the current plan before merging. Refresh the phase milestone dates (or replace them with relative milestones) wherever the roadmap content is anchored, keeping the Phase 1/Foundation structure and related phase entries consistent across the document.crates/clipboard-core/src/lib.rs-44-55 (1)
44-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winJPEG detection is unnecessarily guarded behind the 8-byte length check.
The JPEG signature (
0xFF, 0xD8, 0xFF) only requires 3 bytes, but it's inside thedata.len() >= 8guard meant for the PNG 8-byte signature. A JPEG payload of 3-7 bytes would be misclassified asText.🐛 Proposed fix
- // Check for image signatures - if data.len() >= 8 { - let header = &data[..8]; - // PNG - if header.starts_with(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]) { - return ContentType::Image; - } - // JPEG - if header.starts_with(&[0xFF, 0xD8, 0xFF]) { - return ContentType::Image; - } + // Check for PNG signature (8 bytes) + if data.len() >= 8 && data.starts_with(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]) { + return ContentType::Image; + } + + // Check for JPEG signature (3 bytes) + if data.len() >= 3 && data.starts_with(&[0xFF, 0xD8, 0xFF]) { + return ContentType::Image; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-core/src/lib.rs` around lines 44 - 55, The JPEG signature check in the content-type detection logic is incorrectly nested under the 8-byte PNG guard in the `lib.rs` image signature block. Update the `ContentType` detection path so `header.starts_with(&[0xFF, 0xD8, 0xFF])` can run when the input is at least 3 bytes, while keeping the PNG check under the 8-byte requirement. Use the existing `data`/`header` handling in `clipboard-core` to split the length guard appropriately without changing the overall `ContentType::Image` behavior.crates/clipboard-core/src/item.rs-21-34 (1)
21-34: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Defaultimpl produces an empty hash that could violate the UNIQUE constraint.
ClipboardItem::default()setshash: String::new()(empty string). If a default item is ever persisted to the database, all default instances would share the same empty hash, violating theUNIQUEconstraint on the hash column. Consider computing the hash from the empty content inDefault, or makingDefaultprivate to prevent accidental persistence.🛡️ Proposed fix
impl Default for ClipboardItem { fn default() -> Self { + let content = Vec::new(); + let hash = Self::compute_hash(&content); Self { id: Uuid::new_v4(), content_type: ContentType::Text, - content: Vec::new(), - hash: String::new(), + content, + hash, created_at: Utc::now(), accessed_at: None, pinned: false, favorite: false, } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-core/src/item.rs` around lines 21 - 34, ClipboardItem::default currently assigns an empty String to hash, which can create duplicate values if a default item is persisted and conflict with the UNIQUE constraint. Update the Default impl for ClipboardItem so hash is derived the same way as real items would be (for example from the empty content), or otherwise ensure default instances cannot be persisted; use the ClipboardItem::default constructor and hash field initialization as the main place to fix this.crates/clipboard-platform/src/provider.rs-127-138 (1)
127-138: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
set_contentsilently succeeds for non-text content types.When
item.content_type != ContentType::Text, the method returnsOk(())without setting anything or signaling an error. Callers have no way to distinguish "content set successfully" from "content type not supported, nothing happened."Consider returning an error (e.g.,
PlatformError::AccessFailed("unsupported content type".into())) or at minimum logging a warning.🛡️ Proposed fix
async fn set_content(&self, item: &ClipboardItem) -> Result<(), PlatformError> { if item.content_type == ContentType::Text { let text = String::from_utf8_lossy(&item.content).to_string(); let mut ctx = Clipboard::new().map_err(|e| PlatformError::AccessFailed(e.to_string()))?; ctx.set_text(&text) .map_err(|e| PlatformError::AccessFailed(e.to_string()))?; + } else { + return Err(PlatformError::AccessFailed( + format!("Unsupported content type: {:?}", item.content_type), + )); } - // TODO: Handle other content types Ok(()) }Also applies to: 178-189, 229-240
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-platform/src/provider.rs` around lines 127 - 138, `set_content` currently returns success for unsupported clipboard item types without doing anything, so callers can’t detect the no-op. Update the `set_content` implementation in `provider.rs` (and the matching `set_content` paths in the other affected providers) to explicitly handle `ContentType` mismatches by returning a `PlatformError::AccessFailed` or logging a warning and then returning an error. Keep the existing text path using `Clipboard::new` and `set_text`, but ensure non-text content does not fall through to `Ok(())`.crates/clipboard-ai/src/ai.rs-29-32 (1)
29-32: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInconsistent stub behavior across
AiEnginemethods.
categorizeandsummarizereturnErr(AiError::ProviderNotConfigured), butsuggestionsreturnsOk(Vec::new()). Callers cannot distinguish "provider not configured" from "no suggestions available." Return the same error for consistency.💡 Suggested fix
pub async fn suggestions(&self, _content: &str) -> Result<Vec<String>, AiError> { // TODO: Implement smart suggestions - Ok(Vec::new()) + Err(AiError::ProviderNotConfigured) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-ai/src/ai.rs` around lines 29 - 32, The `suggestions` method in `AiEngine` is behaving differently from `categorize` and `summarize` by returning an empty success result instead of `AiError::ProviderNotConfigured`. Update `AiEngine::suggestions` to match the other stubbed methods and return the same error so callers can consistently detect when the provider is not configured.crates/clipboard-api/src/api.rs-50-76 (1)
50-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStub handlers return HTTP 200 for "Not implemented" errors.
All three non-status handlers return
Json(json!({ "success": false, "error": "Not implemented" }))which defaults to200 OK. This is semantically incorrect — clients cannot distinguish success from failure by status code. Return501 Not Implemented(or503 Service Unavailable) until these are implemented.💡 Suggested fix using StatusCode
mod handlers { - use axum::{response::IntoResponse, Json}; + use axum::{http::StatusCode, response::IntoResponse, Json}; use serde_json::json; pub async fn status() -> impl IntoResponse { Json(json!({ "status": "ok", "version": "0.1.0" })) } pub async fn get_clipboard() -> impl IntoResponse { - Json(json!({ - "success": false, - "error": "Not implemented" - })) + (StatusCode::NOT_IMPLEMENTED, Json(json!({ + "success": false, + "error": "Not implemented" + }))) } pub async fn set_clipboard() -> impl IntoResponse { - Json(json!({ - "success": false, - "error": "Not implemented" - })) + (StatusCode::NOT_IMPLEMENTED, Json(json!({ + "success": false, + "error": "Not implemented" + }))) } pub async fn search() -> impl IntoResponse { - Json(json!({ - "success": false, - "error": "Not implemented" - })) + (StatusCode::NOT_IMPLEMENTED, Json(json!({ + "success": false, + "error": "Not implemented" + }))) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-api/src/api.rs` around lines 50 - 76, The stub handlers in api.rs currently return Json responses with "success": false and "Not implemented" but still default to 200 OK. Update get_clipboard, set_clipboard, and search to return an explicit non-200 status such as StatusCode::NOT_IMPLEMENTED (or SERVICE_UNAVAILABLE) alongside the JSON body, while leaving status() unchanged; use the handler names to locate the three affected responses.crates/clipboard-encryption/src/encryption.rs-43-52 (1)
43-52: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Nonce::from_slicewill panic on invalid nonce length.
Nonce::from_slice(&encrypted.nonce)panics if the slice is not exactly 12 bytes (the AES-256-GCM nonce size). In normal operation the nonce is always 12 bytes, but ifEncryptedDatais deserialized from corrupted or tampered storage, this will crash the process instead of returning a graceful error.🛡️ Proposed fix: validate nonce length before conversion
let nonce = Nonce::from_slice(&encrypted.nonce); + if encrypted.nonce.len() != 12 { + return Err(EncryptionError::DecryptionFailed( + "invalid nonce length".to_string(), + )); + } + let nonce = Nonce::from_slice(&encrypted.nonce);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-encryption/src/encryption.rs` around lines 43 - 52, The decrypt path in EncryptionError handling can panic because EncryptData::nonce is converted with Nonce::from_slice without validating its length first. Update decrypt in the encryption.rs module to check encrypted.nonce has the expected 12-byte AES-GCM nonce size before constructing the Nonce, and return an EncryptionError instead of panicking when the nonce is invalid. Use the decrypt method and the Nonce conversion site as the main fix point.
🧹 Nitpick comments (20)
apps/cli/src/main.rs (2)
17-32: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider using integer IDs to match the database layer.
The
idfields forGet,Copy,Pin,Favorite, andDeleteareString. The database layer (SQLite) likely uses integer row IDs (i64). When these commands are implemented, theStringIDs will require parsing/conversion at the boundary. Usingi64(or a dedicatedItemIdnewtype) here would align the CLI contract with the database and IPC layers and surface invalid input earlier via clap's built-in parsing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/main.rs` around lines 17 - 32, The CLI command variants in main.rs use String for item IDs, but the database layer expects integer row IDs. Update the Get, Copy, Pin, Favorite, and Delete variants to use i64 or a dedicated ItemId newtype so clap parses invalid input up front and the CLI contract matches the SQLite/IPC layer; adjust any downstream matching or handlers that consume these IDs accordingly.
44-85: 📐 Maintainability & Code Quality | 🔵 TrivialStubbed commands need implementation.
All command handlers are currently
println!stubs with TODO comments. Per the PR objectives, these are expected for Phase 1/2 scaffolding. Would you like me to help implement any of these commands (e.g., wiringSearchto the IPC client orListto the database)?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/main.rs` around lines 44 - 85, The command dispatch in main is still stubbed with println! and TODOs instead of real behavior. Replace the placeholder handling in the cli command match for Commands::Search, Get, Copy, List, Pin, Favorite, Delete, Status, DaemonStart, and DaemonStop with the actual implementations or wiring to the existing IPC/database/daemon logic, using the command variants and main entrypoint as the primary locations to update.apps/cli/Cargo.toml (1)
14-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
clipboard-clidependency.The
clipboard-clilibrary crate exists with relevant dependencies (clap,anyhow,clipboard-core,tokio, etc.) but is not referenced by the binary. The binary defines all CLI logic inline inmain.rsinstead of delegating to the library. Consider addingclipboard-clias a dependency and moving the command definitions and dispatch logic there.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/Cargo.toml` around lines 14 - 16, The CLI binary is missing a dependency on the existing clipboard-cli crate, so the command parsing and dispatch logic stays duplicated in main.rs. Add clipboard-cli to the apps/cli Cargo.toml dependencies, then move the CLI definitions and execution flow into the clipboard-cli library entry points so the binary just delegates to that crate. Use the existing clipboard-cli module and its command/dispatch types to keep the binary thin and reuse the shared logic.apps/desktop/index.html (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the page title from "desktop" to "OpenPaste".
The
<title>is set to the generic template name "desktop". This will appear in the window title and browser tab.✏️ Suggested change
- <title>desktop</title> + <title>OpenPaste</title>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/index.html` at line 7, The page title is still set to the generic placeholder in the desktop HTML entry point. Update the title value in the index.html document so the browser tab and window title use OpenPaste instead of the current desktop label.apps/desktop/src-tauri/src/main.rs (1)
33-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract common IPC client creation and error handling to eliminate duplication.
Every command repeats the same pattern: create
IpcClient,senda message, and matchError/Err/_toErr(String). Extracting a helper reduces ~120 lines of boilerplate to a single call per command.As per path instructions, this Tauri backend exposes IPC-backed commands — the duplication spans all 7 commands in this file.
♻️ Proposed refactor: extract IPC helper
+async fn send_ipc<T>( + message: IpcMessage, + extract: impl Fn(IpcMessage) -> Result<T, String>, +) -> Result<T, String> { + let socket_path = get_ipc_socket_path(); + let client = IpcClient::new(socket_path.to_string_lossy().to_string()); + match client.send(message).await { + Ok(msg) => extract(msg), + Err(e) => Err(format!("IPC error: {}", e)), + } +} + #[tauri::command] async fn get_clipboard_history() -> Result<Vec<ClipboardItem>, String> { - let socket_path = get_ipc_socket_path(); - let client = IpcClient::new(socket_path.to_string_lossy().to_string()); - - match client.send(IpcMessage::GetHistory).await { - Ok(IpcMessage::ClipboardHistory { items }) => { - let clipboard_items: Vec<ClipboardItem> = items - .into_iter() - .map(|item| ClipboardItem { - id: item.id, - content_type: item.content_type, - content: String::from_utf8_lossy(&item.content).to_string(), - hash: item.hash, - created_at: item.created_at, - accessed_at: item.accessed_at, - pinned: item.pinned, - favorite: item.favorite, - }) - .collect(); - Ok(clipboard_items) - } - Ok(IpcMessage::Error { message }) => Err(format!("Daemon error: {}", message)), - Err(e) => Err(format!("IPC error: {}", e)), - _ => Err("Unexpected response from daemon".to_string()), - } + send_ipc(IpcMessage::GetHistory, |msg| match msg { + IpcMessage::ClipboardHistory { items } => { + Ok(items.into_iter().map(map_clipboard_item).collect()) + } + IpcMessage::Error { message } => Err(format!("Daemon error: {}", message)), + _ => Err("Unexpected response from daemon".to_string()), + }).await }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src-tauri/src/main.rs` around lines 33 - 160, All Tauri IPC commands repeat the same client setup and response/error mapping, so extract that boilerplate into a shared helper around IpcClient::new and send. Add a reusable function for the common send-and-map flow, then update get_clipboard_history, search_clipboard_items, set_clipboard_content, get_current_clipboard, delete_clipboard_item, toggle_pin_item, and toggle_favorite_item to call it with their specific IpcMessage and success handling. Keep the helper responsible for converting IpcMessage::Error, transport Err, and unexpected responses into the String errors so each command becomes a thin wrapper.Source: Path instructions
apps/desktop/src-tauri/Cargo.toml (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVerify the
shell-openfeature is needed.The
shell-openfeature is enabled on thetauridependency, but no shell-open API usage is visible inmain.rs. If unused, removing it reduces the attack surface and build complexity.As per path instructions, this file is part of the Tauri desktop application layer that exposes IPC-backed commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src-tauri/Cargo.toml` at line 14, The tauri dependency in the desktop app manifest enables the shell-open feature, but there is no visible use of the shell-open API in main.rs or related Tauri command code. Verify whether any shell-open functionality is actually required by the desktop app layer; if it is not, remove the shell-open feature from the tauri dependency and keep the manifest limited to only the features used by the IPC-backed commands.Source: Path instructions
apps/desktop/src/App.tsx (1)
550-620: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winModal lacks keyboard dismiss and focus management.
The detail modal has no Escape key handler to close it and no focus trap. Users who open it via keyboard cannot dismiss it without tabbing to the
✕button. Add anonKeyDownhandler for Escape and basic focus management.♿ Proposed fix — add Escape handler
{selectedItem && ( <div style={{ position: 'fixed', inset: 0, backgroundColor: 'rgba(0, 0, 0, 0.7)', display: 'flex', alignItems: 'center', justifyContent: 'center', padding: '1rem', zIndex: 1000 + }} + onKeyDown={(e) => { + if (e.key === 'Escape') setSelectedItem(null) }}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/App.tsx` around lines 550 - 620, The Clipboard Item modal rendered from selectedItem in App.tsx needs keyboard dismissal and basic focus handling. Add an Escape key handler to close the modal via setSelectedItem(null), and make the modal container manage focus when it opens so keyboard users start inside the dialog. Also ensure focus is restored to the previously focused element when the modal closes, using the modal wrapper/button elements in this selectedItem block as the entry points.clippy.toml (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMisleading comment: config warns on wildcard imports, not all warnings.
The comment
# Deny warningssuggests all warnings are denied, butwarn-on-all-wildcard-imports = trueonly enables warnings for wildcard imports (e.g.,use foo::*). If the intent is to deny all warnings in CI, that should be configured viaRUSTFLAGS="-D warnings"in the CI workflow or#![deny(warnings)]in crate roots.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clippy.toml` around lines 3 - 4, The comment in clippy.toml is misleading because warn-on-all-wildcard-imports only enables warnings for wildcard imports, not all warnings. Update the comment near warn-on-all-wildcard-imports to describe wildcard-import warnings accurately, or move any intended “deny warnings” behavior to the CI workflow or crate roots using the appropriate warning-deny mechanism.crates/clipboard-core/src/lib.rs (2)
68-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpand test coverage for content type detection.
Tests only cover
TextandHtml. Missing tests for PNG, JPEG, empty data, and the edge case where JPEG data is shorter than 8 bytes.✅ Suggested additional tests
#[test] fn test_content_type_detection() { assert_eq!(ContentType::detect(b"Hello, World!"), ContentType::Text); assert_eq!(ContentType::detect(b"<!DOCTYPE html>"), ContentType::Html); + assert_eq!(ContentType::detect(b""), ContentType::Text); + assert_eq!( + ContentType::detect(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]), + ContentType::Image + ); + assert_eq!( + ContentType::detect(&[0xFF, 0xD8, 0xFF, 0xE0]), + ContentType::Image + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-core/src/lib.rs` around lines 68 - 77, Expand the coverage in the tests module for ContentType::detect by adding cases for PNG and JPEG signatures, empty input, and the short-JPEG edge case where the byte slice is fewer than 8 bytes. Update the existing test_content_type_detection test (or add focused tests alongside it) to assert the correct ContentType variants for these inputs so the detection logic in ContentType::detect is fully exercised.
57-65: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHTML detection misses lowercase
<!doctype.The check covers
<!DOCTYPE,<html, and<HTMLbut not<!doctype(lowercase). Real-world HTML documents may use any casing. Consider a case-insensitive prefix check.🔧 Proposed fix
- let text = String::from_utf8_lossy(data); - if text.starts_with("<!DOCTYPE") || text.starts_with("<html") || text.starts_with("<HTML") { + let text = String::from_utf8_lossy(data); + let lower = text.to_lowercase(); + if lower.starts_with("<!doctype") || lower.starts_with("<html") { return ContentType::Html; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-core/src/lib.rs` around lines 57 - 65, The HTML detection in clipboard-core’s content type check is case-sensitive and misses lowercase doctype input. Update the prefix logic in the ContentType detection path around the existing String::from_utf8_lossy(data) check to treat HTML markers case-insensitively, so <!doctype and other casing variants are recognized alongside the current <!DOCTYPE and <html forms.README.md (1)
37-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winREADME project structure and tech stack are incomplete.
The project structure tree (lines 70-82) lists only 4 crates (
clipboard-core,clipboard-db,clipboard-ipc,clipboard-daemon), but the workspace includes many more:clipboard-storage,clipboard-encryption,clipboard-events,clipboard-platform,clipboard-api,clipboard-search,clipboard-sync,clipboard-plugin, andclipboard-ai. Similarly, the technology stack (line 41) says "Unix domain sockets (IPC)" but this PR also adds Windows named pipe IPC support, and Phase 1 features (line 16) say "Core clipboard capture (macOS)" while Windows and Linux support was also added.📝 Suggested updates
**Backend:** - Rust - SQLite (with sqlx) - Tokio async runtime -- Unix domain sockets (IPC) +- Unix domain sockets / Windows named pipes (IPC)├── crates/ │ ├── clipboard-core/ # Core clipboard types +│ ├── clipboard-db/ # Database operations +│ ├── clipboard-storage/ # Compressed file storage +│ ├── clipboard-encryption/ # AES-256-GCM encryption +│ ├── clipboard-events/ # Event bus │ ├── clipboard-ipc/ # IPC communication +│ ├── clipboard-platform/ # Platform clipboard providers +│ ├── clipboard-daemon/ # Background daemon +│ ├── clipboard-api/ # REST API server +│ ├── clipboard-search/ # FTS5 search engine +│ ├── clipboard-sync/ # Cloud sync +│ ├── clipboard-plugin/ # WASM plugin system +│ └── clipboard-ai/ # AI features └── docs/ # DocumentationAlso applies to: 70-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 37 - 41, Update the README’s project structure and tech stack sections to reflect the full workspace and new platform support. Expand the structure tree referenced by the crates section to include the additional packages such as clipboard-storage, clipboard-encryption, clipboard-events, clipboard-platform, clipboard-api, clipboard-search, clipboard-sync, clipboard-plugin, and clipboard-ai, and revise the Phase 1 and backend/platform bullets so they no longer describe macOS-only or Unix-domain-socket-only support. Make the wording in the README consistent with the actual multi-platform IPC support and the broader crate layout exposed by the workspace.crates/clipboard-daemon/src/main.rs (2)
135-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated DB model creation — extract a helper or
Fromimpl.The identical
clipboard_db::models::ClipboardItemstruct construction appears in both theSetClipboardhandler (lines 135–144) and the watch loop (lines 218–227). Extract this to avoid divergence.♻️ Suggested extraction
fn to_db_item(item: &ClipboardItem) -> clipboard_db::models::ClipboardItem { clipboard_db::models::ClipboardItem { id: 0, content_type: item.content_type.to_string(), content: item.content.clone(), hash: item.hash.clone(), created_at: item.created_at, accessed_at: item.accessed_at, pinned: item.pinned, favorite: item.favorite, } }Also applies to: 218-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-daemon/src/main.rs` around lines 135 - 144, The same clipboard_db::models::ClipboardItem construction is duplicated in both the SetClipboard path and the watch loop, so extract it into a shared helper or a From impl to keep the mapping consistent. Add a reusable conversion near the ClipboardItem/clipboard_db::models::ClipboardItem boundary (for example a to_db_item helper or impl From<&ClipboardItem>), then replace both inline struct literals with that shared conversion so future field changes only need one update.
84-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
ClipboardHistoryItemmapping — extract a helper orFromimpl.The identical 10-line mapping from
clipboard_db::models::ClipboardItemtoclipboard_ipc::ClipboardHistoryItemappears in bothGetHistory(lines 84–95) andSearchItems(lines 107–118). Extract this into a helper function or aFromimplementation to keep the two paths in sync.♻️ Suggested extraction
fn map_to_history_item(item: clipboard_db::models::ClipboardItem) -> clipboard_ipc::ClipboardHistoryItem { clipboard_ipc::ClipboardHistoryItem { id: item.id.to_string(), content_type: item.content_type, content: item.content, hash: item.hash, created_at: item.created_at.to_rfc3339(), accessed_at: item.accessed_at.map(|t| t.to_rfc3339()), pinned: item.pinned, favorite: item.favorite, } } // Then at both call sites: let history_items: Vec<_> = items.into_iter().map(map_to_history_item).collect();Or as a
Fromimpl in the IPC crate:impl From<clipboard_db::models::ClipboardItem> for ClipboardHistoryItem { fn from(item: clipboard_db::models::ClipboardItem) -> Self { // ... same mapping } }Also applies to: 107-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-daemon/src/main.rs` around lines 84 - 95, The `ClipboardHistoryItem` conversion is duplicated in both `GetHistory` and `SearchItems`, so extract the shared `clipboard_db::models::ClipboardItem` -> `clipboard_ipc::ClipboardHistoryItem` mapping into a helper function or a `From` implementation. Update both call sites in `main.rs` to use that shared converter so the fields stay in sync and future changes only happen in one place.crates/clipboard-ipc/Cargo.toml (1)
18-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant target-specific
tokiodependencies.The
[target.'cfg(unix)'.dependencies]and[target.'cfg(windows)'.dependencies]sections re-declaretokio = { workspace = true }identically to the general dependency at line 11. Without platform-specific feature flags, these add no value and are dead configuration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-ipc/Cargo.toml` around lines 18 - 22, Remove the redundant target-specific tokio entries from the Cargo.toml dependency sections and keep the shared workspace dependency only; the duplicated [target.'cfg(unix)'.dependencies] and [target.'cfg(windows)'.dependencies] declarations in crates/clipboard-ipc/Cargo.toml can be deleted since they don’t add any platform-specific behavior. Preserve any other target-specific dependencies in those sections if present, but eliminate the repeated tokio = { workspace = true } lines.crates/clipboard-platform/Cargo.toml (1)
21-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlatform-specific dependencies are unused in the current implementation.
clipboard-win,wl-clipboard-rs,x11-clipboard,cocoa, andobjcare declared but never referenced inprovider.rs— all three platform implementations use onlyarboard. These add unnecessary compile-time dependencies and build surface for platforms that may not even be the build target.Consider removing them until the
watch_changesTODOs are actually implemented, or gate them behind a feature flag.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-platform/Cargo.toml` around lines 21 - 30, The Cargo.toml platform-specific dependencies are currently unused because provider.rs only relies on arboard, so remove clipboard-win, wl-clipboard-rs, x11-clipboard, cocoa, and objc from the target-specific dependency sections or gate them behind a feature flag. Update the clipboard-platform crate configuration so only symbols actually referenced by the current Provider implementations remain, and keep the TODO watch_changes work isolated until those platform backends are implemented.crates/clipboard-platform/src/provider.rs (1)
96-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree platform implementations are near-identical — consolidate to eliminate duplication.
The
windows,linux, andmacosmodules are functionally identical: all usearboard::Clipboardthe same way forget_contentandset_content, and all return the same "Not implemented" error forwatch_changes. The only differences are the module/type names and TODO comments.Since
arboardalready abstracts platform differences, a singleArboardProviderimplementation could replace all three. Platform-specific modules would only be needed oncewatch_changesis actually implemented with native APIs.♻️ Proposed consolidation
+/// Single arboard-based provider for all platforms +#[derive(Clone)] +pub struct ArboardProvider; + +impl ArboardProvider { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ClipboardProvider for ArboardProvider { + async fn get_content(&self) -> Result<ClipboardItem, PlatformError> { + let content_bytes = tokio::task::spawn_blocking(|| { + let mut ctx = Clipboard::new() + .map_err(|e| PlatformError::AccessFailed(e.to_string()))?; + let text = ctx + .get_text() + .map_err(|e| PlatformError::AccessFailed(e.to_string()))?; + Ok::<_, PlatformError>(text.into_bytes()) + }) + .await + .map_err(|e| PlatformError::AccessFailed(e.to_string()))??; + + let content_type = ContentType::detect(&content_bytes); + Ok(ClipboardItem::new(content_type, content_bytes)) + } + + async fn set_content(&self, item: &ClipboardItem) -> Result<(), PlatformError> { + if item.content_type != ContentType::Text { + return Ok(()); + } + let text = String::from_utf8_lossy(&item.content).to_string(); + tokio::task::spawn_blocking(move || { + let mut ctx = Clipboard::new() + .map_err(|e| PlatformError::AccessFailed(e.to_string()))?; + ctx.set_text(&text) + .map_err(|e| PlatformError::AccessFailed(e.to_string()))?; + Ok::<_, PlatformError>(()) + }) + .await + .map_err(|e| PlatformError::AccessFailed(e.to_string()))??; + Ok(()) + } + + async fn watch_changes(&self) -> Result<(), PlatformError> { + Err(PlatformError::WatchFailed("Not implemented".to_string())) + } +}Then simplify
PlatformProviderandget_provider:+pub type PlatformProvider = ArboardProvider; + +pub fn get_provider() -> Result<PlatformProvider, PlatformError> { + Ok(ArboardProvider::new()) +}Also applies to: 147-196, 198-247
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-platform/src/provider.rs` around lines 96 - 145, The three platform-specific providers are duplicated, with WindowsProvider matching the linux and macos implementations for get_content, set_content, and watch_changes. Consolidate the shared arboard-based logic into a single provider implementation (for example, an ArboardProvider) and have the platform modules reuse it, keeping only truly platform-specific code where native clipboard watching will eventually differ. Update the provider selection logic so the unique symbols WindowsProvider, LinuxProvider, and MacOSProvider no longer each carry the same copy of these methods.crates/clipboard-api/src/api.rs (1)
23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tower-httpCORS and trace layers are declared but not applied.
Cargo.tomlincludestower-httpwithcorsandtracefeatures, but the router inapi.rsdoesn't applyTraceLayerorCorsLayer. Either apply them or remove the dependency to avoid pulling unused code.💡 Suggested fix to apply middleware
use crate::ApiError; use axum::{ routing::{get, post}, Router, }; use std::net::SocketAddr; use tokio::net::TcpListener; +use tower_http::trace::TraceLayer; impl ApiServer { fn router() -> Router { Router::new() .route("/api/v1/status", get(handlers::status)) .route( "/api/v1/clipboard", get(handlers::get_clipboard).post(handlers::set_clipboard), ) .route("/api/v1/search", post(handlers::search)) + .layer(TraceLayer::new_for_http()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-api/src/api.rs` around lines 23 - 31, The router in router() currently defines only the API routes and never applies the tower-http middleware that Cargo.toml enables. Update the Router setup in api.rs to wrap the route tree with TraceLayer and CorsLayer (or remove the unused tower-http features if middleware is not needed), making sure the middleware is applied to the handlers like handlers::status, handlers::get_clipboard, handlers::set_clipboard, and handlers::search.crates/clipboard-search/src/lib.rs (1)
11-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SearchError::InvalidSyntaxis defined but never used.All sqlx errors in
search.rsare mapped toQueryFailed, including FTS5 syntax errors from malformed queries. Consider detecting FTS5 syntax errors (e.g., by inspecting the sqlx error string) and mapping them toInvalidSyntaxfor more actionable error reporting, or remove the variant if it's not needed in the current phase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-search/src/lib.rs` around lines 11 - 17, The SearchError enum currently defines InvalidSyntax but search.rs maps every sqlx failure to QueryFailed, so update the error handling in the search flow to distinguish FTS5 malformed-query/syntax failures and return SearchError::InvalidSyntax from the relevant search function path, using the existing SearchError variants and the sqlx error inspection logic; if that distinction is not needed, remove InvalidSyntax from SearchError instead so the enum matches actual usage.crates/clipboard-storage/src/storage.rs (1)
8-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unnecessary
#[allow(dead_code)]
base_pathis used instore,retrieve, anddeleteviaself.base_path.join(...). The#[allow(dead_code)]attribute is misleading and should be removed.🧹 Proposed fix
pub struct Storage { - #[allow(dead_code)] base_path: std::path::PathBuf, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-storage/src/storage.rs` around lines 8 - 11, The Storage struct’s base_path field is already used by store, retrieve, and delete through self.base_path.join(...), so the #[allow(dead_code)] attribute is unnecessary and misleading. Remove that attribute from the Storage definition and keep the field as-is so the existing methods continue to reference it normally.crates/clipboard-encryption/src/key_derivation.rs (1)
39-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider using
hash_password_intofor direct key derivation instead of PHC string + SHA-256.The current
derive_keymethod hashes the password viahash_password, converts the resulting PHC string (which includes algorithm, version, parameters, salt, and hash) to bytes, and then SHA-256 hashes that entire string to produce a 32-byte key. This is non-standard and unnecessarily complex — the comment on line 48 says "Extract the hash bytes" but actually extracts the full PHC metadata string.
Argon2::hash_password_intocan directly produce a 32-byte key from the password and raw salt bytes, eliminating the SHA-256 dependency and the misleading intermediate step.♻️ Proposed refactor using hash_password_into
/// Derive a key from password using Argon2id pub fn derive_key(&self, password: &str, salt: &str) -> Result<[u8; 32], EncryptionError> { - let salt = SaltString::from_b64(salt) - .map_err(|e| EncryptionError::KeyDerivationFailed(e.to_string()))?; - - let password_hash = self - .argon2 - .hash_password(password.as_bytes(), &salt) - .map_err(|e| EncryptionError::KeyDerivationFailed(e.to_string()))?; - - // Extract the hash bytes and use SHA-256 to derive a 32-byte key - let hash_str = password_hash.to_string(); - let hash_bytes = hash_str.as_bytes(); - let mut key = [0u8; 32]; - - // Use SHA-256 to derive a 32-byte key from the hash string - let digest = sha2::Sha256::digest(hash_bytes); - key.copy_from_slice(&digest); - - Ok(key) + // Decode the base64 salt to raw bytes for hash_password_into + let salt = SaltString::from_b64(salt) + .map_err(|e| EncryptionError::KeyDerivationFailed(e.to_string()))?; + let raw_salt = salt + .decode_b64() + .map_err(|e| EncryptionError::KeyDerivationFailed(e.to_string()))?; + + let mut key = [0u8; 32]; + self.argon2 + .hash_password_into(password.as_bytes(), raw_salt.as_bytes(), &mut key) + .map_err(|e| EncryptionError::KeyDerivationFailed(e.to_string()))?; + + Ok(key) }Note: Verify
SaltString::decode_b64availability in thepassword-hashversion resolved by the workspace. If unavailable, generate raw salt bytes directly ingenerate_saltand encode to base64 for storage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/clipboard-encryption/src/key_derivation.rs` around lines 39 - 58, The derive_key method in KeyDerivation currently turns an Argon2 PHC string into bytes and then SHA-256 hashes that full metadata string, which is overly indirect and misleading. Update KeyDerivation::derive_key to use Argon2::hash_password_into for direct 32-byte key derivation from the password and raw salt bytes, and remove the intermediate password_hash/string/SHA-256 steps. If SaltString decoding is needed, use the existing salt handling in this module to obtain raw bytes or adjust generate_salt accordingly, and keep EncryptionError mapping consistent for derivation failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72496c25-11cc-4788-b70a-95fe3bfef519
⛔ Files ignored due to path filters (7)
apps/desktop/package-lock.jsonis excluded by!**/package-lock.jsonapps/desktop/public/favicon.svgis excluded by!**/*.svgapps/desktop/public/icons.svgis excluded by!**/*.svgapps/desktop/src-tauri/icons/icon.pngis excluded by!**/*.pngapps/desktop/src/assets/hero.pngis excluded by!**/*.pngapps/desktop/src/assets/react.svgis excluded by!**/*.svgapps/desktop/src/assets/vite.svgis excluded by!**/*.svg
📒 Files selected for processing (80)
.github/workflows/ci.yml.gitignoreCargo.tomlREADME.mdapps/cli/Cargo.tomlapps/cli/src/main.rsapps/desktop/.gitignoreapps/desktop/README.mdapps/desktop/eslint.config.jsapps/desktop/index.htmlapps/desktop/package.jsonapps/desktop/postcss.config.jsapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/build.rsapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.cssapps/desktop/src/App.tsxapps/desktop/src/index.cssapps/desktop/src/main.tsxapps/desktop/tailwind.config.jsapps/desktop/tsconfig.app.jsonapps/desktop/tsconfig.jsonapps/desktop/tsconfig.node.jsonapps/desktop/vite.config.tsclippy.tomlcrates/clipboard-ai/Cargo.tomlcrates/clipboard-ai/src/ai.rscrates/clipboard-ai/src/lib.rscrates/clipboard-api/Cargo.tomlcrates/clipboard-api/src/api.rscrates/clipboard-api/src/handlers.rscrates/clipboard-api/src/lib.rscrates/clipboard-cli/Cargo.tomlcrates/clipboard-cli/src/lib.rscrates/clipboard-core/Cargo.tomlcrates/clipboard-core/src/clipboard.rscrates/clipboard-core/src/item.rscrates/clipboard-core/src/lib.rscrates/clipboard-daemon/Cargo.tomlcrates/clipboard-daemon/src/main.rscrates/clipboard-db/Cargo.tomlcrates/clipboard-db/migrations/001_initial.sqlcrates/clipboard-db/src/database.rscrates/clipboard-db/src/lib.rscrates/clipboard-db/src/models.rscrates/clipboard-db/src/schema.rscrates/clipboard-encryption/Cargo.tomlcrates/clipboard-encryption/src/encryption.rscrates/clipboard-encryption/src/key_derivation.rscrates/clipboard-encryption/src/lib.rscrates/clipboard-events/Cargo.tomlcrates/clipboard-events/src/bus.rscrates/clipboard-events/src/event.rscrates/clipboard-events/src/lib.rscrates/clipboard-ipc/Cargo.tomlcrates/clipboard-ipc/src/error.rscrates/clipboard-ipc/src/ipc.rscrates/clipboard-ipc/src/lib.rscrates/clipboard-platform/Cargo.tomlcrates/clipboard-platform/src/lib.rscrates/clipboard-platform/src/provider.rscrates/clipboard-plugin/Cargo.tomlcrates/clipboard-plugin/src/lib.rscrates/clipboard-plugin/src/plugin.rscrates/clipboard-plugin/src/sandbox.rscrates/clipboard-search/Cargo.tomlcrates/clipboard-search/src/lib.rscrates/clipboard-search/src/search.rscrates/clipboard-storage/Cargo.tomlcrates/clipboard-storage/src/lib.rscrates/clipboard-storage/src/storage.rscrates/clipboard-sync/Cargo.tomlcrates/clipboard-sync/src/lib.rscrates/clipboard-sync/src/sync.rsdocs/ARCHITECTURE.mddocs/CONTRIBUTING.mddocs/ROADMAP.mddocs/SECURITY.mdrustfmt.toml
…vements - Fix critical FTS5 prefix query (use proper wildcard syntax) - Remove Cargo.lock from .gitignore for reproducible builds - Unify ID type to String across ClipboardItem and Tauri commands - Fix binary content corruption using base64 encoding for non-text content - Set CSP in tauri.conf.json for security - Fix stale closure in useEffect using ref to prevent unnecessary re-renders - Remove redundant index on hash column (UNIQUE already creates index) - Fix blocking I/O in storage (use tokio::fs instead of std::fs) - Fix filename collision using UUID instead of timestamp - Add path traversal protection in storage operations - Fix CI to run on all PRs (not just main/develop branches) - Update base64 encoding to use non-deprecated API
- Install libgtk-3-dev, libwebkit2gtk-4.0-dev, libappindicator3-dev, librsvg2-dev, patchelf on Ubuntu - These are required for Tauri/GTK dependencies on Linux
- Change libwebkit2gtk-4.0-dev to libwebkit2gtk-4.1-dev - Ubuntu 24.04 uses webkit2gtk 4.1 instead of 4.0
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/DATABASE.md`:
- Around line 366-369: The seed inserts for collections are missing the required
timestamp fields, so the block will fail against the current schema. Update the
seed data to include values for collections.created_at and
collections.updated_at, or adjust the collections table definition to provide
defaults for those NOT NULL columns; use the existing INSERT INTO collections
block as the place to make the change.
- Around line 309-322: The FTS maintenance triggers for clipboard_items_fts are
using plain UPDATE and DELETE, which won’t correctly sync an external-content
FTS5 table. Update the clipboard_items_fts_update and clipboard_items_fts_delete
triggers to use FTS5’s delete/reinsert maintenance pattern instead, making sure
the trigger logic around clipboard_items_fts preserves index consistency for
updates and deletions.
In `@docs/SEARCH.md`:
- Around line 205-241: The weighting is being applied twice in the search
scoring logic, so pinned and favorite boosts are far larger than intended.
Update the scoring in the combined_score documentation/example to use the boost
values from pinned_boost and favorite_boost only once, and remove the extra
multipliers applied to those fields so the helpers remain the single source of
truth for their weights.
- Around line 265-269: The tag filter in the clipboard search query is binding
`tags.name IN (?)` as a single parameter, so multi-tag lookups will not match
all values. Update the query-building logic around the clipboard/tag search to
expand the `Vec<String>` into one placeholder per tag and bind them all, or use
an equivalent normalization approach such as a temp table, so the
`clipboard_item_tags`/`tags` join correctly returns results for every requested
tag.
- Around line 198-203: The frequency_boost helper uses log10(access_count)
directly, which breaks the default zero-access case in ranking. Update
frequency_boost to use a zero-safe transform such as log10(access_count + 1)
while keeping the same return shape. Use the existing frequency_boost function
in the search docs snippet as the target for the fix.
- Around line 284-289: The SQL example under the Pinned/Favorite Filter section
uses two separate WHERE clauses, which is invalid; update the example in
SEARCH.md so it either combines the pinned and favorite predicates into one
WHERE clause using AND, or splits them into two distinct query snippets. Keep
the change localized to the markdown example block and ensure the query shown is
valid SQL.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b79debd5-a560-46b2-b108-f6f7ae0ac4d5
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
.github/workflows/ci.yml.gitignoreapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.tsxcrates/clipboard-db/migrations/001_initial.sqlcrates/clipboard-search/src/search.rscrates/clipboard-storage/Cargo.tomlcrates/clipboard-storage/src/storage.rsdocs/AI.mddocs/CLI.mddocs/DATABASE.mddocs/DESIGN_SYSTEM.mddocs/ENCRYPTION.mddocs/EVENTS.mddocs/IPC.mddocs/PLATFORM.mddocs/PLUGIN_SDK.mddocs/PROJECT.mddocs/RELEASE.mddocs/REPOSITORY_STRUCTURE.mddocs/REST_API.mddocs/SEARCH.mddocs/STACK.mddocs/STORAGE.mddocs/TASKS.mddocs/TESTING.mddocs/UI.md
💤 Files with no reviewable changes (3)
- .gitignore
- .github/workflows/ci.yml
- crates/clipboard-db/migrations/001_initial.sql
✅ Files skipped from review due to trivial changes (15)
- docs/PROJECT.md
- docs/REPOSITORY_STRUCTURE.md
- docs/TASKS.md
- docs/STACK.md
- docs/AI.md
- docs/RELEASE.md
- docs/EVENTS.md
- docs/IPC.md
- docs/PLUGIN_SDK.md
- docs/PLATFORM.md
- docs/STORAGE.md
- docs/DESIGN_SYSTEM.md
- docs/REST_API.md
- docs/CLI.md
- docs/ENCRYPTION.md
🚧 Files skipped from review as they are similar to previous changes (6)
- crates/clipboard-storage/Cargo.toml
- apps/desktop/src-tauri/tauri.conf.json
- apps/desktop/src-tauri/Cargo.toml
- crates/clipboard-search/src/search.rs
- apps/desktop/src-tauri/src/main.rs
- apps/desktop/src/App.tsx
| -- Update trigger | ||
| CREATE TRIGGER clipboard_items_fts_update AFTER UPDATE ON clipboard_items BEGIN | ||
| UPDATE clipboard_items_fts SET | ||
| content_preview = NEW.content_preview, | ||
| source_app = COALESCE(NEW.source_app, ''), | ||
| source_window = COALESCE(NEW.source_window, ''), | ||
| metadata = COALESCE(NEW.metadata, '{}') | ||
| WHERE rowid = NEW.id; | ||
| END; | ||
|
|
||
| -- Delete trigger | ||
| CREATE TRIGGER clipboard_items_fts_delete AFTER DELETE ON clipboard_items BEGIN | ||
| DELETE FROM clipboard_items_fts WHERE rowid = OLD.id; | ||
| END; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the documented schema and surrounding trigger definitions.
sed -n '260,360p' docs/DATABASE.md
# Find all FTS-related trigger definitions in the document.
rg -n "clipboard_items_fts_(update|delete|insert)|content='clipboard_items'|content_rowid|fts5" docs/DATABASE.mdRepository: anonfedora/openpaste
Length of output: 1963
🌐 Web query:
SQLite FTS5 external content triggers UPDATE DELETE maintenance pattern documentation
💡 Result:
In SQLite, FTS5 tables configured with the external content option (using the content='...' option) do not automatically update when the source table changes [1][2]. Maintaining consistency is the user's responsibility, and the standard maintenance pattern is to use database triggers on the source table to synchronize changes [1][2]. Because FTS5 does not support standard UPDATE or DELETE operations on external content tables in the same way as traditional SQL tables, synchronization is achieved by using a special command syntax that simulates these operations via INSERT statements into the FTS5 virtual table [3][1]. The standard trigger maintenance pattern is: 1. AFTER INSERT: Insert the new row into the FTS5 table [3][2]. 2. AFTER DELETE: Use the FTS5 'delete' command to remove the old row data [3][1]. 3. AFTER UPDATE: Use the FTS5 'delete' command to remove the old row data, followed by an 'insert' of the updated row data [3][4]. Example Implementation: Assuming a source table ITEMS and an FTS5 table ITEMS_FTS: -- Insert trigger CREATE TRIGGER ITEMS_ai AFTER INSERT ON ITEMS BEGIN INSERT INTO ITEMS_FTS(rowid, title, body) VALUES (new.id, new.title, new.body); END; -- Delete trigger CREATE TRIGGER ITEMS_ad AFTER DELETE ON ITEMS BEGIN INSERT INTO ITEMS_FTS(ITEMS_FTS, rowid, title, body) VALUES ('delete', old.id, old.title, old.body); END; -- Update trigger CREATE TRIGGER ITEMS_au AFTER UPDATE ON ITEMS BEGIN INSERT INTO ITEMS_FTS(ITEMS_FTS, rowid, title, body) VALUES ('delete', old.id, old.title, old.body); INSERT INTO ITEMS_FTS(rowid, title, body) VALUES (new.id, new.title, new.body); END; Key Considerations: - Command Syntax: The INSERT INTO FTS5_TABLE(FTS5_TABLE, ...) syntax is a control message [3]. Passing the string 'delete' as the first column tells the FTS5 engine to perform a deletion based on the provided rowid and content [3][1]. - Update Order: In the update trigger, it is critical to perform the 'delete' operation before the 'insert' operation [3][1]. - Initial Population: Triggers only capture changes made after they are created [1][2]. You must manually populate the FTS5 table from the existing content table when first setting up the index [1][2]. - Performance: Triggers are optimized for individual row operations [3]. For large bulk imports, it is often more efficient to drop the triggers, perform the bulk import, and then rebuild the FTS5 index [3].
Citations:
- 1: https://www2.sqlite.org/draft/fts5.html
- 2: https://docs.w3cub.com/sqlite/fts5
- 3: https://wiki.r-that.com/snippets/sqlite-fts5-triggers-verbatim/
- 4: https://stackoverflow.com/questions/59095651/sqlite-fts-table-is-not-being-populated
Use FTS5 delete/insert maintenance triggers.
clipboard_items_fts is an external-content table, so plain UPDATE/DELETE here won’t keep the index in sync. Switch the update trigger to delete + reinsert, and the delete trigger to the FTS5 'delete' command.
Example shape
-CREATE TRIGGER clipboard_items_fts_update AFTER UPDATE ON clipboard_items BEGIN
- UPDATE clipboard_items_fts SET
- content_preview = NEW.content_preview,
- source_app = COALESCE(NEW.source_app, ''),
- source_window = COALESCE(NEW.source_window, ''),
- metadata = COALESCE(NEW.metadata, '{}')
- WHERE rowid = NEW.id;
-END;
+CREATE TRIGGER clipboard_items_fts_update AFTER UPDATE ON clipboard_items BEGIN
+ INSERT INTO clipboard_items_fts(clipboard_items_fts, rowid, content_preview, source_app, source_window, metadata)
+ VALUES('delete', OLD.id, OLD.content_preview, OLD.source_app, OLD.source_window, OLD.metadata);
+ INSERT INTO clipboard_items_fts(rowid, content_preview, source_app, source_window, metadata)
+ VALUES(NEW.id, NEW.content_preview, COALESCE(NEW.source_app, ''), COALESCE(NEW.source_window, ''), COALESCE(NEW.metadata, '{}'));
+END;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/DATABASE.md` around lines 309 - 322, The FTS maintenance triggers for
clipboard_items_fts are using plain UPDATE and DELETE, which won’t correctly
sync an external-content FTS5 table. Update the clipboard_items_fts_update and
clipboard_items_fts_delete triggers to use FTS5’s delete/reinsert maintenance
pattern instead, making sure the trigger logic around clipboard_items_fts
preserves index consistency for updates and deletions.
| INSERT INTO collections (name, description, is_system, sort_order) VALUES | ||
| ('All Items', 'All clipboard items', 1, 0), | ||
| ('Pinned', 'Pinned clipboard items', 1, 1), | ||
| ('Favorites', 'Favorite clipboard items', 1, 2); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Populate the required timestamps in the seed rows.
collections.created_at and collections.updated_at are NOT NULL, so this seed block will fail as written. Either pass both values in the inserts or add defaults to the table definition.
Suggested fix
-INSERT INTO collections (name, description, is_system, sort_order) VALUES
- ('All Items', 'All clipboard items', 1, 0),
- ('Pinned', 'Pinned clipboard items', 1, 1),
- ('Favorites', 'Favorite clipboard items', 1, 2);
+INSERT INTO collections (name, description, is_system, sort_order, created_at, updated_at) VALUES
+ ('All Items', 'All clipboard items', 1, 0, strftime('%s', 'now') * 1000, strftime('%s', 'now') * 1000),
+ ('Pinned', 'Pinned clipboard items', 1, 1, strftime('%s', 'now') * 1000, strftime('%s', 'now') * 1000),
+ ('Favorites', 'Favorite clipboard items', 1, 2, strftime('%s', 'now') * 1000, strftime('%s', 'now') * 1000);📝 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.
| INSERT INTO collections (name, description, is_system, sort_order) VALUES | |
| ('All Items', 'All clipboard items', 1, 0), | |
| ('Pinned', 'Pinned clipboard items', 1, 1), | |
| ('Favorites', 'Favorite clipboard items', 1, 2); | |
| INSERT INTO collections (name, description, is_system, sort_order, created_at, updated_at) VALUES | |
| ('All Items', 'All clipboard items', 1, 0, strftime('%s', 'now') * 1000, strftime('%s', 'now') * 1000), | |
| ('Pinned', 'Pinned clipboard items', 1, 1, strftime('%s', 'now') * 1000, strftime('%s', 'now') * 1000), | |
| ('Favorites', 'Favorite clipboard items', 1, 2, strftime('%s', 'now') * 1000, strftime('%s', 'now') * 1000); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/DATABASE.md` around lines 366 - 369, The seed inserts for collections
are missing the required timestamp fields, so the block will fail against the
current schema. Update the seed data to include values for
collections.created_at and collections.updated_at, or adjust the collections
table definition to provide defaults for those NOT NULL columns; use the
existing INSERT INTO collections block as the place to make the change.
| **Frequency Boost:** | ||
| ```rust | ||
| fn frequency_boost(access_count: i32) -> f64 { | ||
| 1.0 + (access_count as f64).log10() | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the zero-access case safe.
log10(0) yields -inf, so the default access_count = 0 can poison ranking for new items. Use a zero-safe transform such as log10(access_count + 1).
Suggested fix
-fn frequency_boost(access_count: i32) -> f64 {
- 1.0 + (access_count as f64).log10()
-}
+fn frequency_boost(access_count: i32) -> f64 {
+ 1.0 + ((access_count as f64) + 1.0).log10()
+}📝 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.
| **Frequency Boost:** | |
| ```rust | |
| fn frequency_boost(access_count: i32) -> f64 { | |
| 1.0 + (access_count as f64).log10() | |
| } | |
| ``` | |
| **Frequency Boost:** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/SEARCH.md` around lines 198 - 203, The frequency_boost helper uses
log10(access_count) directly, which breaks the default zero-access case in
ranking. Update frequency_boost to use a zero-safe transform such as
log10(access_count + 1) while keeping the same return shape. Use the existing
frequency_boost function in the search docs snippet as the target for the fix.
| **Pinned Boost:** | ||
| ```rust | ||
| fn pinned_boost(pinned: bool) -> f64 { | ||
| if pinned { 10.0 } else { 0.0 } | ||
| } | ||
| ``` | ||
|
|
||
| **Favorite Boost:** | ||
| ```rust | ||
| fn favorite_boost(favorite: bool) -> f64 { | ||
| if favorite { 5.0 } else { 0.0 } | ||
| } | ||
| ``` | ||
|
|
||
| **Source App Boost:** (Optional, configurable) | ||
| ```rust | ||
| fn source_app_boost(source_app: &str, preferred_apps: &[&str]) -> f64 { | ||
| if preferred_apps.contains(&source_app) { 2.0 } else { 0.0 } | ||
| } | ||
| ``` | ||
|
|
||
| ### Combined Score | ||
|
|
||
| ```rust | ||
| fn combined_score( | ||
| bm25: f64, | ||
| recency: f64, | ||
| frequency: f64, | ||
| pinned: f64, | ||
| favorite: f64, | ||
| ) -> f64 { | ||
| bm25 * 1.0 + | ||
| recency * 0.5 + | ||
| frequency * 0.3 + | ||
| pinned * 10.0 + | ||
| favorite * 5.0 | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply pinned/favorite weighting in one place only.
The helpers already encode the boost value, and combined_score multiplies them again. That makes pinned items 100x and favorites 25x heavier than the config implies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/SEARCH.md` around lines 205 - 241, The weighting is being applied twice
in the search scoring logic, so pinned and favorite boosts are far larger than
intended. Update the scoring in the combined_score documentation/example to use
the boost values from pinned_boost and favorite_boost only once, and remove the
extra multipliers applied to those fields so the helpers remain the single
source of truth for their weights.
| WHERE id IN ( | ||
| SELECT clipboard_item_id FROM clipboard_item_tags | ||
| JOIN tags ON clipboard_item_tags.tag_id = tags.id | ||
| WHERE tags.name IN (?) | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Expand tag placeholders dynamically.
tags is a Vec<String>, but IN (?) only binds a single value. Multi-tag searches will silently miss results unless you generate one placeholder per tag or normalize through a temp table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/SEARCH.md` around lines 265 - 269, The tag filter in the clipboard
search query is binding `tags.name IN (?)` as a single parameter, so multi-tag
lookups will not match all values. Update the query-building logic around the
clipboard/tag search to expand the `Vec<String>` into one placeholder per tag
and bind them all, or use an equivalent normalization approach such as a temp
table, so the `clipboard_item_tags`/`tags` join correctly returns results for
every requested tag.
| ### Pinned/Favorite Filter | ||
|
|
||
| ```sql | ||
| WHERE pinned = 1 | ||
| WHERE favorite = 1 | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a single WHERE clause here.
Two WHERE clauses in one query is invalid SQL; combine the predicates with AND or split the examples into separate snippets.
Suggested fix
-WHERE pinned = 1
-WHERE favorite = 1
+WHERE pinned = 1 AND favorite = 1📝 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.
| ### Pinned/Favorite Filter | |
| ```sql | |
| WHERE pinned = 1 | |
| WHERE favorite = 1 | |
| ``` | |
| ### Pinned/Favorite Filter | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/SEARCH.md` around lines 284 - 289, The SQL example under the
Pinned/Favorite Filter section uses two separate WHERE clauses, which is
invalid; update the example in SEARCH.md so it either combines the pinned and
favorite predicates into one WHERE clause using AND, or splits them into two
distinct query snippets. Keep the change localized to the markdown example block
and ensure the query shown is valid SQL.
- Configure icon paths in tauri.conf.json for all platforms - Add 32x32, 128x128, 128x128@2x PNG icons - Add macOS .icns and Windows .ico icons
- Required by soup2-sys crate for webkit2gtk support - Fixes build failure on Ubuntu 24.04
- Change from ubuntu-latest (24.04) to ubuntu-22.04 - Use webkit2gtk-4.0-dev (stable in 22.04) instead of 4.1-dev - Remove libsoup dependency (not needed with 4.0) - Ubuntu 22.04 LTS has better library compatibility for Tauri
1 participant
Phase 1 & 2 Core Features Implementation
Summary
This PR implements the foundational encryption, platform support, and core features for OpenPaste as outlined in Phase 1 and Phase 2 of the roadmap.
Phase 1: Foundation Improvements
Encryption Foundation:
Platform Support:
Phase 2: Core Features
FTS5 Full-Text Search:
SearchItemsIPC message typeZstd Compression:
Clipboard Actions:
Database Improvements:
Code Quality
cargo check,cargo test,cargo clippy,cargo fmtFiles Changed
Testing
Known Issues
Next Steps
Summary by CodeRabbit