Repository navigation
Add Ratzilla integration to enable Glues in web browsers - #215
Conversation
WalkthroughAdds a wasm/web frontend and CI wasm build job, splits backend implementations by target (wasm vs native) with a BackendBox abstraction, removes CSV/JSON backends, introduces a cross-platform input layer, refactors TUI for dual CLI/web targets, and reorganizes server and workspace manifests for unified CLI and server crates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Browser as User (Browser)
participant Host as WASM Host
participant Web as tui::web
participant Term as DomTerminal
participant App as Glues App
participant Backend as BackendBox
Browser->>Host: load index.html
Host->>Web: wasm_bindgen start()
Web->>Term: create Dom terminal + key handler
Term-->>Web: KeyEvent
Web->>App: consume(Input)
App->>Backend: dispatch operation (Instant or Proxy)
Backend-->>App: Result / Transition
App-->>Term: render frame
sequenceDiagram
autonumber
actor Client as TUI/Web
participant ClientProxy as ProxyClient
participant Server as ProxyServer
participant DB as BackendBox
Client->>ClientProxy: Proxy request (fetch_notes)
ClientProxy->>Server: HTTP POST
Server->>DB: CoreBackend call
DB-->>Server: Result
Server-->>ClientProxy: Response
ClientProxy-->>Client: Data
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 7
🧹 Nitpick comments (35)
tui/web/index.html (2)
9-12: Add SRI to CDN stylesheetInclude integrity and crossorigin on the Fira Code CDN link for supply-chain hardening.
- <link - rel="stylesheet" - href="https://cdnjs.cloudflare.com/ajax/libs/firacode/6.2.0/fira_code.min.css" - /> + <link + rel="stylesheet" + href="https://cdnjs.cloudflare.com/ajax/libs/firacode/6.2.0/fira_code.min.css" + integrity="<!-- fill SRI hash here -->" + crossorigin="anonymous" + referrerpolicy="no-referrer" + />
6-8: Remove user-scalable=no for accessibilityDisabling zoom impairs accessibility; omit user-scalable.
- content="width=device-width, initial-scale=1.0, user-scalable=no" + content="width=device-width, initial-scale=1.0".github/workflows/rust.yml (1)
50-67: Wasm build in CI: LGTM; minor hardening ideas
- Consider installing trunk via a pinned action/binary for reproducibility rather than cargo install latest.
- Optionally upload tui/web/dist as an artifact for preview.
Example pin using taiki-e/install-action:
- - name: Install trunk - run: cargo install trunk --locked --force + - name: Install trunk + uses: taiki-e/install-action@v2 + with: + tool: trunk@0.20.3tui/src/web.rs (3)
13-24: Consider better error reporting beyond console logging.The current implementation logs errors to the console but doesn't provide structured error handling or recovery mechanisms. Consider implementing a more robust error reporting system that could notify users of initialization failures through the UI.
36-49: Verify error handling in event callbacks.The keyboard event handler silently discards errors with
let _on Lines 45-46. Critical errors during input processing or state saving could go unnoticed.Consider logging errors or implementing a user-visible error notification system:
- let action = guard.context.consume(&input).await; - let _ = guard.handle_action(action, input).await; - guard.save().await; + let action = guard.context.consume(&input).await; + if let Err(e) = guard.handle_action(action, input).await { + console::error_1(&JsValue::from_str(&format!("Error handling action: {}", e))); + } + guard.save().await;
51-55: Consider handling lock contention more gracefully.Using
try_lock()prevents blocking but silently skips render frames when the lock is held. This could lead to UI inconsistencies during heavy input processing.Consider implementing a frame-skip counter or warning mechanism to detect rendering issues:
terminal.draw_web(move |frame| { if let Ok(mut guard) = draw_app.try_lock() { guard.draw(frame); + } else { + // Log frame skip for debugging purposes + console::warn_1(&JsValue::from_str("Frame skipped due to lock contention")); } });tui/src/config.rs (3)
38-45: Add error handling for missing HOME directory.The fallback to current directory when HOME is unavailable is reasonable, but silently falling back with
expect()could crash in restricted environments.Consider logging the fallback:
pub(crate) fn get_glue() -> Glue<CsvStorage> { let path = home_dir() - .unwrap_or(std::env::current_dir().expect("failed to get current directory")) + .unwrap_or_else(|| { + let cwd = std::env::current_dir().expect("failed to get current directory"); + eprintln!("Warning: HOME directory not found, using current directory: {:?}", cwd); + cwd + }) .join(PATH);
47-138: Significant code duplication between native and WASM implementations.Lines 47-99 and 119-171 contain nearly identical logic for
init(),update(), andget()functions, with only the storage type differing. This violates DRY principles and makes maintenance harder.Consider extracting shared logic using a trait or macro:
// Define a trait for storage operations trait ConfigStorage: Execute { fn new_glue() -> Self; } // Implement shared logic generically async fn init_generic<S: ConfigStorage>() { let mut glue = S::new_glue(); // ... rest of the shared logic } // Platform-specific implementations just provide the glue creation #[cfg(not(target_arch = "wasm32"))] impl ConfigStorage for Glue<CsvStorage> { fn new_glue() -> Self { // existing get_glue() logic } } #[cfg(target_arch = "wasm32")] impl ConfigStorage for Glue<WebStorage> { fn new_glue() -> Self { Glue::new(WebStorage::new(WebStorageType::Local)) } }
58-65: Silent error handling in config initialization.The
let _ =pattern on Lines 59 and 131 silently ignores insertion errors. If default values fail to insert, the application might behave unexpectedly.Consider at least logging failures:
for (key, value) in DEFAULTS { - let _ = table("config") + if let Err(e) = table("config") .insert() .columns(vec!["key", "value"]) .values(vec![vec![text(*key), text(*value)]]) .execute(&mut glue) - .await; + .await { + eprintln!("Warning: Failed to insert default config {}: {}", key, e); + } }Also applies to: 130-137
core/src/backend/local/wasm.rs (1)
25-27: No-op sync method may cause confusion.The
sync()method always returnsOk(())without performing any actual synchronization. This could lead to data consistency issues if callers expect synchronization to occur.Consider either:
- Document clearly that sync is not supported in WASM
- Return an error indicating sync is unsupported
- Remove the method if it's not required by the interface
pub fn sync(&self) -> Result<()> { + // Sync is a no-op for in-memory storage in WASM environment Ok(()) }tui/Cargo.toml (1)
26-29: Empty feature flags may cause confusion.The
cliandwebfeatures are defined but don't actually enable any dependencies or conditional compilation beyond the binary target gating.Consider documenting the purpose of these features or removing them if they're only used for binary gating:
[features] default = ["cli"] +# Feature flag to enable CLI binary compilation cli = [] +# Feature flag reserved for future web-specific functionality web = []tui/src/input.rs (2)
86-87: Unexpected Input variant for unhandled events.Line 86 maps unhandled event types to
Input::Resize(0, 0), which could be misleading. A resize event with zero dimensions might be processed incorrectly by downstream code.Consider adding a dedicated variant for unhandled events or documenting this behavior:
pub enum Input { Key(KeyEvent), Paste(String), Resize(u16, u16), + Unknown, } // In the From implementation: - _ => Input::Resize(0, 0), + _ => Input::Unknown,
162-162: Hardcoded KeyEventKind::Press for WASM events.Line 162 always sets
kind: KeyEventKind::Pressregardless of the actual event type. This could cause issues if release or repeat events need to be handled differently in the future.Consider documenting this limitation or checking if ratzilla provides event kind information:
KeyEvent { code, modifiers: KeyModifiers::new(event.ctrl, event.alt, event.shift), + // TODO: ratzilla doesn't currently expose event kind, defaulting to Press kind: KeyEventKind::Press, }tui/src/context.rs (1)
147-153: Forward paste/text events (Input::Paste) to TextAreato_textarea_input currently only matches Input::Key while enum Input defines Paste(String); update fn to_textarea_input (tui/src/input.rs) to map Input::Paste(s) → the equivalent tui_textarea::Input carrying s so the TextArea used in tui/src/context.rs (lines 147–153) receives pasted/IME/text input.
core/src/backend/proxy/client.rs (1)
18-43: Add client timeouts on native builds; preserve wasm behavior.core/Cargo.toml already configures reqwest per target, so cfg-gating a Client::builder timeout is safe — apply the patch below.
use async_trait::async_trait; +#[cfg(not(target_arch = "wasm32"))] +use std::time::Duration; @@ pub async fn connect<U: Into<String>>(url: U) -> Result<Self> { let url = url.into(); - let client = Client::new(); + #[cfg(not(target_arch = "wasm32"))] + let client = Client::builder() + .timeout(Duration::from_secs(10)) + .build()?; + #[cfg(target_arch = "wasm32")] + let client = Client::new();core/src/backend/proxy/server.rs (3)
3-3: Drop unused CoreBackend import.CoreBackend isn't referenced in this file; keep the import surface minimal.
-use crate::backend::{BackendBox, CoreBackend}; +use crate::backend::BackendBox;
14-14: Relax receiver mutability on handle (enables concurrency).handle doesn't mutate self; switching to &self removes an unnecessary exclusive borrow and allows concurrent calls.
- pub async fn handle(&mut self, req: ProxyRequest) -> ProxyResponse { + pub async fn handle(&self, req: ProxyRequest) -> ProxyResponse {Please confirm that BackendBox methods only need &self.
17-88: Deduplicate Ok/Err response mapping.Many match arms repeat the same Result → ProxyResponse boilerplate. Extract a small helper to reduce repetition and future drift.
impl ProxyServer { + fn map<T, E, F>(res: Result<T, E>, f: F) -> ProxyResponse + where + E: std::fmt::Display, + F: FnOnce(T) -> ResultPayload, + { + match res { + Ok(v) => ProxyResponse::Ok(f(v)), + Err(e) => ProxyResponse::Err(e.to_string()), + } + } @@ - FetchDirectory { directory_id } => match self.db.fetch_directory(directory_id).await { - Ok(dir) => ProxyResponse::Ok(ResultPayload::Directory(dir)), - Err(e) => ProxyResponse::Err(e.to_string()), - }, + FetchDirectory { directory_id } => { + Self::map(self.db.fetch_directory(directory_id).await, ResultPayload::Directory) + }, @@ - FetchDirectories { parent_id } => match self.db.fetch_directories(parent_id).await { - Ok(dirs) => ProxyResponse::Ok(ResultPayload::Directories(dirs)), - Err(e) => ProxyResponse::Err(e.to_string()), - }, + FetchDirectories { parent_id } => { + Self::map(self.db.fetch_directories(parent_id).await, ResultPayload::Directories) + },(Apply similarly to other arms.)
tui/tests/entry.rs (1)
74-83: Avoid hardcoded per-arch hotkeys in tests.Tie the test to a single source of truth (e.g., re-export a PROXY_HOTKEY constant from tui/src/context/entry.rs for tests) to prevent drift when keymaps change.
If no public constant exists, consider adding one behind cfg(test) or a test-only re-export.
tui/tests/tester.rs (1)
38-41: Remove unnecessary unsafe block.std::env::set_var is safe; the unsafe block is misleading and may trip linters.
- unsafe { - std::env::set_var("HOME", &cwd); - } + std::env::set_var("HOME", &cwd);README.md (2)
53-57: Add a brief CORS note for browser → proxy usage.Wasm builds require the proxy to send CORS headers for cross-origin access; calling this out helps first‑time users.
- - In the TUI entry menu choose `Proxy`, enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally. + - In the TUI entry menu choose `Proxy`, enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally. + - For the web build, ensure the proxy sends appropriate CORS headers (e.g., Access-Control-Allow-Origin) so the browser can reach it.
121-126: Fix markdownlint MD004: use asterisks for unordered list.Align bullet style to the rest of the doc to satisfy markdownlint.
-- `cargo coverage` - run the workspace under coverage and emit an HTML report at `target/llvm-cov/html/index.html` -- `cargo coverage-report` - print a text summary (run after `cargo coverage --no-report`; append `--fail-under-lines <N>` to guard CI) -- `cargo coverage-lcov` - export `target/llvm-cov/lcov.info` for external services -- `cargo coverage-clean` - remove coverage artifacts +* `cargo coverage` - run the workspace under coverage and emit an HTML report at `target/llvm-cov/html/index.html` +* `cargo coverage-report` - print a text summary (run after `cargo coverage --no-report`; append `--fail-under-lines <N>` to guard CI) +* `cargo coverage-lcov` - export `target/llvm-cov/lcov.info` for external services +* `cargo coverage-clean` - remove coverage artifactstui/src/app.rs (2)
17-20: Dead import due to contradictory cfg; remove.#[cfg(target_arch = "wasm32")] and #[cfg(not(target_arch = "wasm32"))] stacked together make this use unreachable.
-#[cfg(target_arch = "wasm32")] -#[cfg(not(target_arch = "wasm32"))] -use crate::logger::*;
56-72: Throttle autosaves on idle.Saving on every idle tick can cause needless I/O. Gate on a dirty flag or minimum interval.
Example: track last_save: Instant and only save if state is dirty and elapsed > N secs.
core/src/backend.rs (2)
62-196: Deduplicate Box<...> forwarding impls with a single generic implThree nearly identical impls can drift. Replace them with cfg’d generic impls for Box.
Apply this diff to replace the two wasm impls and the non-wasm impl with generic ones:
-#[cfg(target_arch = "wasm32")] -#[async_trait(?Send)] -impl CoreBackend for Box<dyn CoreBackend> { … all fns forwarding … } - -#[cfg(target_arch = "wasm32")] -#[async_trait(?Send)] -impl CoreBackend for Box<dyn CoreBackend + Send> { … all fns forwarding … } - -#[cfg(not(target_arch = "wasm32"))] -#[async_trait] -impl CoreBackend for Box<dyn CoreBackend + Send> { … all fns forwarding … } +#[cfg(target_arch = "wasm32")] +#[async_trait(?Send)] +impl<T: ?Sized + CoreBackend> CoreBackend for Box<T> { + fn root_id(&self) -> DirectoryId { (**self).root_id() } + async fn fetch_directory(&mut self, directory_id: DirectoryId) -> Result<Directory> { (**self).fetch_directory(directory_id).await } + async fn fetch_directories(&mut self, parent_id: DirectoryId) -> Result<Vec<Directory>> { (**self).fetch_directories(parent_id).await } + async fn add_directory(&mut self, parent_id: DirectoryId, name: String) -> Result<Directory> { (**self).add_directory(parent_id, name).await } + async fn remove_directory(&mut self, directory_id: DirectoryId) -> Result<()> { (**self).remove_directory(directory_id).await } + async fn move_directory(&mut self, directory_id: DirectoryId, parent_id: DirectoryId) -> Result<()> { (**self).move_directory(directory_id, parent_id).await } + async fn rename_directory(&mut self, directory_id: DirectoryId, name: String) -> Result<()> { (**self).rename_directory(directory_id, name).await } + async fn fetch_notes(&mut self, directory_id: DirectoryId) -> Result<Vec<Note>> { (**self).fetch_notes(directory_id).await } + async fn fetch_note_content(&mut self, note_id: NoteId) -> Result<String> { (**self).fetch_note_content(note_id).await } + async fn add_note(&mut self, directory_id: DirectoryId, name: String) -> Result<Note> { (**self).add_note(directory_id, name).await } + async fn remove_note(&mut self, note_id: NoteId) -> Result<()> { (**self).remove_note(note_id).await } + async fn rename_note(&mut self, note_id: NoteId, name: String) -> Result<()> { (**self).rename_note(note_id, name).await } + async fn update_note_content(&mut self, note_id: NoteId, content: String) -> Result<()> { (**self).update_note_content(note_id, content).await } + async fn move_note(&mut self, note_id: NoteId, directory_id: DirectoryId) -> Result<()> { (**self).move_note(note_id, directory_id).await } + async fn log(&mut self, category: String, message: String) -> Result<()> { (**self).log(category, message).await } +} + +#[cfg(not(target_arch = "wasm32"))] +#[async_trait] +impl<T: ?Sized + CoreBackend> CoreBackend for Box<T> { + fn root_id(&self) -> DirectoryId { (**self).root_id() } + async fn fetch_directory(&mut self, directory_id: DirectoryId) -> Result<Directory> { (**self).fetch_directory(directory_id).await } + async fn fetch_directories(&mut self, parent_id: DirectoryId) -> Result<Vec<Directory>> { (**self).fetch_directories(parent_id).await } + async fn add_directory(&mut self, parent_id: DirectoryId, name: String) -> Result<Directory> { (**self).add_directory(parent_id, name).await } + async fn remove_directory(&mut self, directory_id: DirectoryId) -> Result<()> { (**self).remove_directory(directory_id).await } + async fn move_directory(&mut self, directory_id: DirectoryId, parent_id: DirectoryId) -> Result<()> { (**self).move_directory(directory_id, parent_id).await } + async fn rename_directory(&mut self, directory_id: DirectoryId, name: String) -> Result<()> { (**self).rename_directory(directory_id, name).await } + async fn fetch_notes(&mut self, directory_id: DirectoryId) -> Result<Vec<Note>> { (**self).fetch_notes(directory_id).await } + async fn fetch_note_content(&mut self, note_id: NoteId) -> Result<String> { (**self).fetch_note_content(note_id).await } + async fn add_note(&mut self, directory_id: DirectoryId, name: String) -> Result<Note> { (**self).add_note(directory_id, name).await } + async fn remove_note(&mut self, note_id: NoteId) -> Result<()> { (**self).remove_note(note_id).await } + async fn rename_note(&mut self, note_id: NoteId, name: String) -> Result<()> { (**self).rename_note(note_id, name).await } + async fn update_note_content(&mut self, note_id: NoteId, content: String) -> Result<()> { (**self).update_note_content(note_id, content).await } + async fn move_note(&mut self, note_id: NoteId, directory_id: DirectoryId) -> Result<()> { (**self).move_note(note_id, directory_id).await } + async fn log(&mut self, category: String, message: String) -> Result<()> { (**self).log(category, message).await } +}Also applies to: 198-264
32-33: Ensure log() persistence semantics are consistentDirectory/note operations call sync() after writes; log() may also need sync to persist. Consider updating core/src/backend/local/log.rs to call self.sync(), matching others.
Suggested change in core/src/backend/local/log.rs:
pub async fn log(&mut self, category: String, message: String) -> Result<()> { table("Log") .insert() .columns(vec!["category", "message"]) .values(vec![vec![text(category), text(message)]]) .execute(&mut self.storage) .await?; - - Ok(()) + self.sync() }Also applies to: 59-60
tui/src/main.rs (1)
29-51: Initialize color-eyre before other subsystemsInstall error handlers first to capture early failures.
- config::init().await; - logger::init().await; - color_eyre::install()?; + color_eyre::install()?; + config::init().await; + logger::init().await;tui/src/context/entry.rs (1)
65-77: Minor copy edit in Git promptGrammar tweak for clarity.
- Line::from("The path must contain an existing .git repository.".fg(THEME.hint)), - Line::from("otherwise, an error will occur.".fg(THEME.hint)), + Line::from("The path must contain an existing .git repository;".fg(THEME.hint)), + Line::from("otherwise, an error will occur.".fg(THEME.hint)),tui/src/action.rs (2)
13-17: Consider using#[cfg(not(target_arch = "wasm32"))]for conditional imports instead ofallow(unused_imports).Using
cfgdirectly would be more idiomatic and clearer about the intent:-#[cfg_attr(target_arch = "wasm32", allow(unused_imports))] -use ratatui::style::Stylize; - -#[cfg_attr(target_arch = "wasm32", allow(unused_imports))] -use super::theme::THEME; +#[cfg(not(target_arch = "wasm32"))] +use ratatui::style::Stylize; + +#[cfg(not(target_arch = "wasm32"))] +use super::theme::THEME;
226-229: Consider logging or providing feedback for unsupported operations on wasm32.Currently, these actions silently do nothing on wasm32. Consider adding a log message or user feedback to indicate that these operations are not supported in the web environment:
#[cfg(target_arch = "wasm32")] Action::Tui(TuiAction::OpenFile) | Action::Tui(TuiAction::OpenGit(_)) -| Action::Tui(TuiAction::OpenMongo(_)) => {} +| Action::Tui(TuiAction::OpenMongo(_)) => { + log_warn("File, Git, and MongoDB operations are not supported in the web environment"); +}core/src/backend/local/native.rs (4)
27-38: Consider adding connection validation for the memory backend.While the memory backend is simple, consider adding a basic validation or initialization check to ensure the storage is properly set up before returning:
pub async fn memory(task_tx: Sender<Task>) -> Result<Self> { let glue = Glue::new(MemoryStorage::default()); let mut storage = Storage::Memory(glue); let root_id = setup(&mut storage).await?; + + // Validate that the root directory was properly created + if root_id.0.is_nil() { + return Err(Error::BackendError("Failed to initialize root directory".to_string())); + } Ok(Self { storage, root_id, task_tx, }) }
40-50: Add path validation for file backend.Consider validating the file path before creating the storage to provide better error messages:
pub async fn file(task_tx: Sender<Task>, path: &str) -> Result<Self> { + if path.is_empty() { + return Err(Error::BackendError("File path cannot be empty".to_string())); + } + let mut storage = FileStorage::new(path).map(Glue::new).map(Storage::File)?; let root_id = setup(&mut storage).await?;
52-70: Consider validating Git remote and branch parameters.The function accepts remote and branch as parameters but doesn't validate them. Consider adding basic validation:
pub async fn git( task_tx: Sender<Task>, path: &str, remote: String, branch: String, ) -> Result<Self> { + if path.is_empty() { + return Err(Error::BackendError("Git path cannot be empty".to_string())); + } + if remote.is_empty() { + return Err(Error::BackendError("Git remote cannot be empty".to_string())); + } + if branch.is_empty() { + return Err(Error::BackendError("Git branch cannot be empty".to_string())); + } + let mut storage = GitStorage::open(path, StorageType::File)?; storage.set_remote(remote); storage.set_branch(branch);
107-110: Consider using a more specific error variant for channel send failures.The error conversion loses context about what type of operation failed:
self.task_tx .clone() .send(task) - .map_err(|e| Error::BackendError(e.to_string()))?; + .map_err(|e| Error::BackendError(format!("Failed to send Git sync task: {}", e)))?;core/src/glues.rs (1)
37-47: Consider adding cleanup handling for the task system.The native implementation spawns a background thread but doesn't provide a way to cleanly shut it down. Consider adding a shutdown mechanism:
impl Glues { pub fn new() -> Self { let transition_queue = Arc::new(Mutex::new(VecDeque::new())); let (task_tx, task_rx) = channel(); let task_handle = handle_tasks(task_rx, &transition_queue); Self { db: None, state: EntryState.into(), task_tx, task_handle, transition_queue, } } + + pub fn shutdown(self) -> Result<()> { + // Send a shutdown signal to the task handler + drop(self.task_tx); // This will cause recv() to return Err in the task handler + self.task_handle.join() + .map_err(|_| Error::BackendError("Task handler thread panicked".to_string()))?; + Ok(()) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.locktui/tests/snapshots/entry__csv_alert.snapis excluded by!**/*.snaptui/tests/snapshots/entry__help_closed.snapis excluded by!**/*.snaptui/tests/snapshots/entry__help_open.snapis excluded by!**/*.snaptui/tests/snapshots/entry__local_prompt.snapis excluded by!**/*.snaptui/tests/snapshots/entry__proxy_prompt_open.snapis excluded by!**/*.snap
📒 Files selected for processing (45)
.github/workflows/rust.yml(1 hunks).gitignore(1 hunks)Cargo.toml(0 hunks)README.md(3 hunks)core/Cargo.toml(1 hunks)core/src/backend.rs(3 hunks)core/src/backend/local.rs(1 hunks)core/src/backend/local/core_backend.rs(1 hunks)core/src/backend/local/directory.rs(1 hunks)core/src/backend/local/native.rs(1 hunks)core/src/backend/local/wasm.rs(1 hunks)core/src/backend/proxy/client.rs(1 hunks)core/src/backend/proxy/server.rs(1 hunks)core/src/event.rs(0 hunks)core/src/glues.rs(1 hunks)core/src/lib.rs(2 hunks)core/src/state/entry.rs(4 hunks)core/tests/backend.rs(1 hunks)core/tests/proxy.rs(1 hunks)server/Cargo.toml(1 hunks)server/src/main.rs(2 hunks)tui/Cargo.toml(1 hunks)tui/src/action.rs(8 hunks)tui/src/app.rs(4 hunks)tui/src/config.rs(1 hunks)tui/src/context.rs(2 hunks)tui/src/context/entry.rs(6 hunks)tui/src/context/notebook.rs(3 hunks)tui/src/input.rs(1 hunks)tui/src/lib.rs(1 hunks)tui/src/logger.rs(2 hunks)tui/src/main.rs(3 hunks)tui/src/transitions.rs(1 hunks)tui/src/views/body/entry.rs(1 hunks)tui/src/views/dialog/help.rs(1 hunks)tui/src/web.rs(1 hunks)tui/tests/editor_insert.rs(1 hunks)tui/tests/editor_normal.rs(1 hunks)tui/tests/editor_visual.rs(1 hunks)tui/tests/entry.rs(3 hunks)tui/tests/notebook_tree.rs(1 hunks)tui/tests/notebook_views.rs(1 hunks)tui/tests/tester.rs(4 hunks)tui/web/Trunk.toml(1 hunks)tui/web/index.html(1 hunks)
💤 Files with no reviewable changes (2)
- core/src/event.rs
- Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T06:56:17.849Z
Learnt from: panarch
PR: gluesql/glues#213
File: server/src/main.rs:118-125
Timestamp: 2025-09-19T06:56:17.849Z
Learning: The gluesql backends that CoreBackend implementations are built on top of rely on &mut self access patterns and aren't designed for concurrent callers.
Applied to files:
core/tests/backend.rscore/src/state/entry.rscore/src/backend/local.rs
🧬 Code graph analysis (14)
tui/src/web.rs (4)
tui/src/app.rs (2)
run(41-105)new(33-38)tui/src/config.rs (2)
init(47-66)init(119-138)tui/src/logger.rs (1)
init(14-26)tui/src/theme.rs (1)
set_theme(64-66)
core/src/state/entry.rs (3)
core/src/backend/local/native.rs (2)
file(40-50)memory(27-38)core/src/glues.rs (2)
new(36-48)new(78-83)core/src/backend/local/wasm.rs (1)
memory(18-23)
tui/tests/entry.rs (1)
tui/tests/tester.rs (1)
key(67-69)
core/src/backend.rs (7)
core/src/backend/local/core_backend.rs (15)
root_id(14-16)fetch_directory(18-20)fetch_directories(22-24)add_directory(26-28)remove_directory(30-32)move_directory(34-40)rename_directory(42-44)fetch_notes(46-48)fetch_note_content(50-52)add_note(54-56)remove_note(58-60)rename_note(62-64)update_note_content(66-68)move_note(70-72)log(74-76)core/src/backend/proxy/client.rs (15)
root_id(49-51)fetch_directory(53-62)fetch_directories(64-73)add_directory(75-84)remove_directory(86-95)move_directory(97-113)rename_directory(115-124)fetch_notes(126-132)fetch_note_content(134-140)add_note(142-151)remove_note(153-159)rename_note(161-167)update_note_content(169-178)move_note(180-192)log(194-200)core/src/backend/local/directory.rs (6)
fetch_directory(11-44)fetch_directories(46-77)add_directory(79-99)remove_directory(103-122)move_directory(124-138)rename_directory(140-154)core/src/state/notebook/consume/directory.rs (1)
move_directory(230-257)core/src/backend/local/note.rs (7)
fetch_notes(33-64)fetch_note_content(14-31)add_note(66-82)remove_note(84-92)rename_note(106-116)update_note_content(94-104)move_note(118-128)core/src/state/notebook/consume/note.rs (1)
move_note(200-226)core/src/backend/local/log.rs (1)
log(8-17)
tui/src/context.rs (1)
tui/src/input.rs (1)
to_textarea_input(168-173)
server/src/main.rs (2)
core/src/backend/proxy/server.rs (1)
new(10-12)core/src/glues.rs (2)
new(36-48)new(78-83)
core/src/backend/local/native.rs (3)
core/src/schema.rs (1)
setup(11-105)core/src/backend/local/wasm.rs (4)
sync(25-27)memory(18-23)execute(35-35)execute(43-46)core/src/glues.rs (4)
new(36-48)new(78-83)default(30-32)default(72-74)
tui/src/config.rs (2)
tui/tests/tester.rs (1)
new(35-48)tui/src/logger.rs (1)
init(14-26)
core/src/backend/local/wasm.rs (4)
core/src/schema.rs (1)
setup(11-105)core/src/backend/local/native.rs (4)
memory(27-38)sync(95-114)execute(122-122)execute(130-140)core/src/backend.rs (5)
root_id(11-11)root_id(38-38)root_id(65-67)root_id(133-135)root_id(201-203)core/src/backend/local/core_backend.rs (1)
root_id(14-16)
tui/src/logger.rs (1)
tui/src/config.rs (2)
get_glue(38-45)get_glue(115-117)
tui/src/action.rs (1)
tui/tests/tester.rs (2)
ctrl(61-64)key(67-69)
tui/src/context/notebook.rs (1)
tui/src/input.rs (1)
to_textarea_input(168-173)
tui/src/main.rs (1)
server/src/main.rs (1)
main(61-97)
core/src/glues.rs (5)
core/src/task.rs (1)
handle_tasks(21-42)core/src/backend/proxy/server.rs (1)
new(10-12)core/src/state/notebook.rs (2)
new(44-74)consume(300-308)core/src/state/entry.rs (1)
consume(11-65)core/src/state.rs (1)
consume(22-41)
🪛 markdownlint-cli2 (0.17.2)
README.md
53-53: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
54-54: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
🔇 Additional comments (52)
.gitignore (1)
2-4: Ignore web dist: LGTMAdding tui/web/dist/ is correct for Trunk output; reorder is harmless.
server/src/main.rs (1)
86-87: Layer ordering is finewith_state before layer(cors) is OK; the layer applies router‑wide.
core/src/backend/local/directory.rs (1)
101-103: Correct use of async_recursion(?Send) on wasm32This avoids unnecessary Send bounds on single‑threaded wasm. Nice.
tui/src/views/dialog/help.rs (1)
53-101: Help content looks aligned per-targetNon‑wasm section correctly documents memory/file/git/mongo + proxy; no action needed.
tui/web/Trunk.toml (1)
6-9: Serve section key naming — 'addresses' is correct in current TrunkCurrent Trunk docs use addresses (list) — older releases used address (string); no change required.
tui/web/index.html (1)
18-21: Confirm data-target-name="glues" maps to a cdylib crate when --features web is enabledVerification script returned no matches in the repository; cannot confirm. Action items:
- Ensure a crate named "glues" (or a lib that produces an artifact named "glues") declares crate-type = ["cdylib"] in its Cargo.toml.
- Ensure that crate is built when
--features webis passed (feature gating or workspace membership).Location: tui/web/index.html Lines 18-21
data-target-name="glues" data-target-kind="cdylib" data-cargo-args="--lib --no-default-features --features web"server/Cargo.toml (1)
15-16: Remove unused tower = "0.5.2" from server/Cargo.toml; verify dependency treeFile: server/Cargo.toml (lines 15–16). rg found no direct
tower::usage in server/;cargo treefailed in the sandbox (error: "no /proc/self/exe available") so dependency-tree verification must be run locally.Apply this diff:
- tower = "0.5.2" - tower-http = { version = "0.5", features = ["cors"] } + tower-http = { version = "0.5", features = ["cors"] }Run locally to confirm no mixed tower majors:
cargo tree -p glues-proxy-server | rg -nP '\btower v'
If you see both tower v0.4 and v0.5 in the graph, remove the directtower = "0.5.2"or align versions; otherwise apply the diff.tui/src/web.rs (1)
60-62: LGTM!The error conversion utility is well-implemented and follows standard patterns for WASM error handling.
core/Cargo.toml (1)
25-41: Platform-specific dependencies look good.The separation of dependencies by target architecture is well-structured. The WASM target appropriately excludes file-based storage backends and uses
default-features = falsefor reqwest to avoid incompatible features.tui/src/input.rs (1)
175-203: LGTM! Well-structured input conversion utilities.The conversion functions provide clean abstractions for converting between different input representations, maintaining consistency across the codebase.
tui/Cargo.toml (1)
14-14: cdylib is required for wasm-bindgen/wasm-pack — keep it.
The "cdylib" crate-type is needed to emit the .wasm artifact that wasm-bindgen/wasm-pack expect; keeping crate-type = ["rlib", "cdylib"] is correct.core/src/backend/local/wasm.rs (1)
30-36: Confirmed: keep #[async_trait(?Send)] for WASM — no change requiredRepo intentionally disables the Send bound for wasm targets (cfg_attr(target_arch = "wasm32", async_trait(?Send)) is used elsewhere such as core/src/backend/local/core_backend.rs and core/src/backend/proxy/client.rs); the impl still requiring T: Send mirrors the native implementation and is not a blocker.
core/tests/proxy.rs (1)
13-13: Appropriate wasm32 target gating for proxy tests.The conditional compilation attribute
#[cfg(not(target_arch = "wasm32"))]correctly excludes this test from wasm32 builds, which is appropriate since proxy backend operations likely require native networking capabilities not available in WebAssembly environments.tui/tests/notebook_tree.rs (1)
6-6: Import path updated for cross-platform input handling.The import has been correctly updated from
ratatui::crossterm::event::KeyCodetoglues::input::KeyCodeto align with the new cross-platform input module. This ensures consistent input handling across native and WebAssembly targets.tui/tests/editor_visual.rs (1)
6-6: Import path updated for cross-platform input handling.The import has been correctly updated from
ratatui::crossterm::event::KeyCodetoglues::input::KeyCodeto align with the new cross-platform input module. This ensures consistent input handling across native and WebAssembly targets.tui/tests/editor_normal.rs (1)
6-6: Import path updated for cross-platform input handling.The import has been correctly updated from
ratatui::crossterm::event::KeyCodetoglues::input::KeyCodeto align with the new cross-platform input module. This ensures consistent input handling across native and WebAssembly targets.tui/src/lib.rs (2)
6-6: Cross-platform input module added.The new
inputmodule provides a unified abstraction for input handling across native and WebAssembly targets. This is well-architected for supporting multiple platforms.
13-15: WebAssembly-specific web module appropriately gated.The conditional compilation
#[cfg(target_arch = "wasm32")]correctly limits the web module to WebAssembly builds only. This ensures the web frontend code is only compiled for the appropriate target.core/tests/backend.rs (1)
4-4: Appropriate wasm32 target gating for backend tests.The conditional compilation attribute
#[cfg(not(target_arch = "wasm32"))]correctly excludes this test from wasm32 builds. This is appropriate since backend operations involving channels and native database functionality are not suitable for WebAssembly environments.tui/tests/editor_insert.rs (1)
6-6: Import path updated for cross-platform input handling.The import has been correctly updated from
ratatui::crossterm::event::KeyCodetoglues::input::KeyCodeto align with the new cross-platform input module. This ensures consistent input handling across native and WebAssembly targets.tui/src/views/body/entry.rs (1)
35-35: Simplified uniform coloring for menu items.The code now applies uniform coloring (
THEME.menu) to all menu items, removing the conditional logic that previously applied different colors to CSV/JSON items. This simplification aligns with the removal of CSV/JSON menu options from the entry context.core/src/backend/proxy/client.rs (1)
46-47: Correct: async_trait gating for wasm vs. native.The cfg_attr split avoids unnecessary Send bounds on wasm while preserving native behavior. Good alignment with the CoreBackend split.
core/src/lib.rs (1)
19-19: Public re-exports gated consistently.Re-exporting Task/handle_tasks only for non‑wasm keeps the public API clean across targets.
tui/src/context.rs (1)
5-11: Good: centralized input abstraction.Using crate::input::{...} decouples TUI from crossterm and aligns with wasm builds.
core/src/backend/local/core_backend.rs (1)
11-12: Correct: async_trait(?Send) on wasm, default on native.Matches the trait’s threading model and the prior learning that CoreBackend impls are not designed for concurrent callers.
core/src/state/entry.rs (3)
17-17: Nice: platform-aware memory backend helper.Centralizing Db::memory selection by target reduces cfg noise at call sites.
94-102: WASM vs native Db::memory split is clean.Signatures line up; wasm path doesn’t depend on task_tx, native path does.
26-53: Non‑wasm storage gates mostly correct — EntryEvent missing OpenMongocore/src/event.rs defines EntryEvent::{OpenMemory, OpenFile(String), OpenGit{...}} (no OpenMongo); core/src/state/entry.rs has a #[cfg(not(target_arch = "wasm32"))] match arm for Entry(OpenMongo{...}). Add a cfg‑gated EntryEvent::OpenMongo in core/src/event.rs or gate/remove the Entry::OpenMongo arm so event and state variants stay consistent.
tui/src/transitions.rs (1)
6-6: No action required —log!is #[macro_export].
#[macro_export]is declared on thelog!macro in tui/src/logger.rs, so the macro is globally exported and nouse crate::logger::*;is necessary.tui/tests/notebook_views.rs (1)
6-8: Approve — Tester already uses glues::input::KeyCode.
tui/tests/tester.rs imports and uses KeyCode from glues::input (Tester::key and helpers), so no change required.tui/tests/entry.rs (1)
55-67: Good wasm gating for native-only test.open_local_prompt is correctly hidden on wasm32.
tui/tests/tester.rs (1)
16-22: Nice: unified key_press helper.Keeps tests concise and aligned with the new input API.
tui/src/app.rs (1)
74-76: Good: centralizing crossterm → Input conversion.Keeps platform abstraction in one place.
core/src/backend.rs (3)
8-33: WASM trait split looks correctNon-Send futures via async_trait(?Send) are appropriate for wasm32 and match the rest of the codebase’s &mut self model.
35-60: Native trait bound on Send is appropriateRequiring Send on non-wasm aligns with server/task usage and BackendBox aliasing.
266-270: BackendBox aliases look goodThis unifies types across targets and simplifies callers.
tui/src/logger.rs (4)
66-81: panic formatting change is fineSwitch to panic!("{e}") is okay; retains Display.
8-12: Fix import: use crate::config::get_glue (not crate::config::platform::get_glue)
get_glue is defined in tui/src/config.rs (lines 38 and 115); update the use to crate::config::get_glue or add a config::platform module.Likely an incorrect or invalid review comment.
3-6: Confirm ratzilla re-export of web_sys::console; keep wasm_bindgen::JsValue import.
ratzilla::web_sys::console is referenced in tui/src/web.rs:6 and tui/src/logger.rs:4 (so ratzilla provides console); there is no ratzilla::JsValue in the repo, so importing JsValue from wasm_bindgen is correct.
45-49: Macro awaits require async context — ensure call sites are async or add a sync varianttui/src/logger.rs (lines 45–49) defines log! which expands to .await; this forces every call site to be inside an async fn/async block. Verify all call sites are async, or apply one of:
- remove the .await so the macro returns the Future (callers await), or
- add a sync variant (e.g., log_now!) for non-async contexts.
tui/src/main.rs (1)
53-54: WASM no-op main is correctKeeps wasm build happy without pulling in tokio/clap.
tui/src/context/entry.rs (1)
33-37: Menu gating and key mappings look consistentProxy at [5] for native and [2] for wasm matches MENU_ITEMS.
Also applies to: 115-124
core/src/backend/local.rs (1)
1-10: Arch split via re-exports — looks good; files present, verify target builds.
core/src/backend/local/native.rs and core/src/backend/local/wasm.rs exist; re-exports match the cfgs. Run CI or locally: cargo check (native) and cargo check --target wasm32-unknown-unknown (wasm).tui/src/action.rs (2)
8-8: LGTM! Clean migration to the unified input module.The import change from separate ratatui imports to the consolidated
crate::inputmodule aligns well with the cross-platform input handling approach.
376-376: LGTM! Proper use of the new field-based modifier check.The change from comparing against
KeyModifiers::CONTROLto usingkey.modifiers.ctrlis correct and aligns with the new input handling approach.tui/src/context/notebook.rs (4)
6-6: LGTM! Clean integration with the unified input module.The import properly leverages the new cross-platform input abstraction, including the
to_textarea_inputhelper function for mapping inputs to textarea commands.
20-21: Good platform-specific handling of clipboard functionality.The conditional compilation properly gates the clipboard dependency for non-wasm32 targets, acknowledging that clipboard operations aren't available in web environments.
451-454: LGTM! Correct modifier check pattern.The change to use
modifiers.ctrldirectly is consistent with the new input handling approach.
461-463: Good use of the input mapping helper function.The integration with
to_textarea_inputproperly filters inputs before forwarding them to the editor, ensuring only valid textarea inputs are processed.Also applies to: 467-469
core/src/backend/local/native.rs (1)
117-141: LGTM! Clean async trait implementation with proper error handling.The
Executetrait and its blanket implementation provide a clean abstraction over the different storage backends. The pattern matching on storage variants and consistent error handling is well done.core/src/glues.rs (2)
1-96: LGTM! Well-structured platform-specific module organization.The separation of native and wasm implementations with a unified public interface through conditional compilation is clean and maintainable. The use of type aliases (
Inner) and re-exports provides a seamless API regardless of the target platform.
21-22: Good abstraction with BackendBox for unified backend handling.The change from
Box<dyn CoreBackend + Send>toBackendBoxsimplifies the type signatures and provides better abstraction across both native and wasm targets.Also applies to: 67-68
| let cors = CorsLayer::new() | ||
| .allow_origin(Any) | ||
| .allow_methods(Any) | ||
| .allow_headers(Any); |
There was a problem hiding this comment.
❓ Verification inconclusive
CORS is fully open; restrict via env and cache preflights
allow_origin(Any)/methods(Any)/headers(Any) is too permissive for production. Gate via CORS_ALLOWED_ORIGIN and set a sane max_age.
- let cors = CorsLayer::new()
- .allow_origin(Any)
- .allow_methods(Any)
- .allow_headers(Any);
+ use axum::http::HeaderValue;
+ let cors = if let Ok(origin) = std::env::var("CORS_ALLOWED_ORIGIN") {
+ if let Ok(val) = HeaderValue::from_str(&origin) {
+ CorsLayer::new()
+ .allow_origin(val)
+ .allow_methods(Any)
+ .allow_headers(Any)
+ .max_age(Duration::from_secs(300))
+ } else {
+ CorsLayer::new()
+ .allow_origin(Any)
+ .allow_methods(Any)
+ .allow_headers(Any)
+ .max_age(Duration::from_secs(300))
+ }
+ } else {
+ // Dev default; lock down via env in CI/prod.
+ CorsLayer::new()
+ .allow_origin(Any)
+ .allow_methods(Any)
+ .allow_headers(Any)
+ .max_age(Duration::from_secs(300))
+ };If the web client uses credentials (cookies/authorization headers), add:
// .allow_credentials(true)and ensure allowed origin is not wildcard.
Restrict CORS in production (server/src/main.rs:78–81)
allow_origin(Any)/allow_methods(Any)/allow_headers(Any) is too permissive — require a CORS_ALLOWED_ORIGIN whitelist (parse to HeaderValue or use AllowedOrigins), set CorsLayer::max_age (e.g., 300–3600s) to cache preflights, and only enable .allow_credentials(true) when using a specific origin (not Any). (docs.rs)
🤖 Prompt for AI Agents
In server/src/main.rs around lines 78–81 the CORS layer is currently fully
permissive; replace the Any origin/methods/headers with a whitelist sourced from
a CORS_ALLOWED_ORIGIN env var (parse into a HeaderValue or use
tower_http::cors::AllowOrigin/AllowedOrigins) and validate/fallback to a safe
default, restrict allowed methods and headers to the app's required set instead
of Any, set CorsLayer::max_age to e.g. 300–3600 seconds to cache preflight
responses, and only call .allow_credentials(true) when the configured origin is
a specific origin (not Any); ensure errors parsing the env var are handled and
logged and that the production build uses the whitelist.
| KeyCode::Char('a') => TuiAction::Help.into(), | ||
|
|
There was a problem hiding this comment.
Keybinding bug: Help is on 'h' but code listens to 'a'
Displayed as “[h] Help” but handler uses KeyCode::Char('a'). Fix to 'h'.
- KeyCode::Char('a') => TuiAction::Help.into(),
+ KeyCode::Char('h') => TuiAction::Help.into(),📝 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.
| KeyCode::Char('a') => TuiAction::Help.into(), | |
| KeyCode::Char('h') => TuiAction::Help.into(), |
🤖 Prompt for AI Agents
In tui/src/context/entry.rs around lines 125-126, the keybinding for Help is
incorrect: the code matches KeyCode::Char('a') while the UI displays “[h] Help”.
Change the pattern to KeyCode::Char('h') (and optionally also handle
KeyCode::Char('H') if you want case-insensitive input) so the handler matches
the displayed keybinding.
| let message = { | ||
| #[cfg(target_arch = "wasm32")] | ||
| { | ||
| vec![ | ||
| Line::from("Glues keeps storage simple:"), | ||
| Line::raw(""), | ||
| Line::from("Instant".fg(THEME.accent_text).bg(THEME.accent)), | ||
| Line::raw("Volatile in-memory storage that resets when Glues exits."), | ||
| Line::raw("Great for quick scratch notes or testing."), | ||
| Line::raw(""), | ||
| Line::from("Proxy".fg(THEME.accent_text).bg(THEME.accent)), | ||
| Line::raw("Bridge to a Glues proxy server for persistence."), | ||
| Line::raw( | ||
| "Start it with `cargo run -p glues-proxy-server` (use `--listen` to change the address).", | ||
| ), | ||
| Line::raw("Both the web and TUI clients can connect to the same proxy."), | ||
| Line::raw(""), | ||
| ] | ||
| } |
There was a problem hiding this comment.
Fix proxy start command (missing required storage subcommand)
server/src/main.rs requires a StorageCommand; the wasm help text omits it.
- Line::raw(
- "Start it with `cargo run -p glues-proxy-server` (use `--listen` to change the address).",
- ),
+ Line::raw(
+ "Start it with `cargo run -p glues-proxy-server -- memory` (use `--listen` to change the address).",
+ ),📝 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.
| let message = { | |
| #[cfg(target_arch = "wasm32")] | |
| { | |
| vec![ | |
| Line::from("Glues keeps storage simple:"), | |
| Line::raw(""), | |
| Line::from("Instant".fg(THEME.accent_text).bg(THEME.accent)), | |
| Line::raw("Volatile in-memory storage that resets when Glues exits."), | |
| Line::raw("Great for quick scratch notes or testing."), | |
| Line::raw(""), | |
| Line::from("Proxy".fg(THEME.accent_text).bg(THEME.accent)), | |
| Line::raw("Bridge to a Glues proxy server for persistence."), | |
| Line::raw( | |
| "Start it with `cargo run -p glues-proxy-server` (use `--listen` to change the address).", | |
| ), | |
| Line::raw("Both the web and TUI clients can connect to the same proxy."), | |
| Line::raw(""), | |
| ] | |
| } | |
| let message = { | |
| #[cfg(target_arch = "wasm32")] | |
| { | |
| vec![ | |
| Line::from("Glues keeps storage simple:"), | |
| Line::raw(""), | |
| Line::from("Instant".fg(THEME.accent_text).bg(THEME.accent)), | |
| Line::raw("Volatile in-memory storage that resets when Glues exits."), | |
| Line::raw("Great for quick scratch notes or testing."), | |
| Line::raw(""), | |
| Line::from("Proxy".fg(THEME.accent_text).bg(THEME.accent)), | |
| Line::raw("Bridge to a Glues proxy server for persistence."), | |
| Line::raw( | |
| "Start it with `cargo run -p glues-proxy-server -- memory` (use `--listen` to change the address).", | |
| ), | |
| Line::raw("Both the web and TUI clients can connect to the same proxy."), | |
| Line::raw(""), | |
| ] | |
| } |
🤖 Prompt for AI Agents
In tui/src/views/dialog/help.rs around lines 32 to 50, the wasm help text shows
an incorrect start command for the proxy server missing the required storage
subcommand; update the string to include the storage subcommand (e.g., change
the instruction to "Start it with `cargo run -p glues-proxy-server -- storage
proxy` (use `--listen` to change the address).") so the example matches
server/src/main.rs' required command structure.
| [build] | ||
| release = true | ||
| manifest = "../Cargo.toml" | ||
| dist = "dist" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Double-check Trunk [build] keys: ‘manifest’ name may be off
Trunk’s config has evolved; some docs use workspace_manifest_path (or cargo-manifest attr in HTML). Using an unknown key is silently ignored. Ensure the key matches your Trunk version.
🌐 Web query:
What are the valid keys under the [build] section of Trunk.toml as of the latest Trunk release? Is 'workspace_manifest_path' supported, and is 'manifest' a valid key?
💡 Result:
Short answer
- Valid [build] keys include: target, html_output, release, dist, public_url, filehash, inject_scripts, offline, frozen, locked, minify, no_sri (as shown in the official Trunk configuration docs). [1]
- Neither workspace_manifest_path nor manifest appear as supported keys in the documented Trunk.toml build schema — they are not listed in the official config/examples. Use the documented keys above (or generate the current JSON schema via
trunk config generate-schemato confirm your installed Trunk version). [1][2]
Sources
- Trunk configuration / Trunk.toml documentation. [1]
- Example Trunk.toml in the Trunk repo (config/schema reference). [2]
Trunk [build] key 'manifest' is not supported — remove or replace it
Trunk.toml’s [build] keys do not include "manifest" or "workspace_manifest_path" and unknown keys are silently ignored; remove/replace this key or use Trunk’s cargo-manifest HTML attribute. Verify against your Trunk version with trunk config generate-schema.
File: tui/web/Trunk.toml (lines 1–4)
🤖 Prompt for AI Agents
In tui/web/Trunk.toml around lines 1 to 4, the [build] key "manifest" is
unsupported by Trunk and should be removed or replaced; delete the manifest =
"../Cargo.toml" line from this file and, if you need to point Trunk at a
non-default Cargo manifest, move that setting into your HTML via Trunk's
cargo-manifest attribute (or run `trunk config generate-schema` to confirm
supported keys and use the correct Trunk configuration key). Ensure the
remaining [build] keys are valid (e.g., release and dist) and validate the file
against your Trunk version.
# Conflicts: # Cargo.toml # README.md # core/src/backend/local.rs # core/src/state/entry.rs # tui/Cargo.toml # tui/src/action.rs # tui/src/context/entry.rs # tui/src/views/dialog/help.rs # tui/tests/entry.rs
Glues Coverage Report
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (4)
54-54: Avoid hardcoding menu index; fix list marker to satisfy markdownlint (MD004).Hardcoded “[5]” is brittle as menu ordering can change across builds. Also, switch “-” to “*” to match the repo’s unordered list style.
- - In the TUI entry menu choose `Proxy` (option `[5]` on the native build), enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally. + * In the TUI entry menu choose `Proxy`, enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally.
56-56: Clarify browser persistence scope and available backends.Make it explicit that Instant is ephemeral in the browser and that Local/Git/Mongo aren’t available on web.
- > **Web build:** The browser version of Glues persists configuration through GlueSQL WebStorage (LocalStorage) and currently exposes the **Instant** and **Proxy** backends. Point the Proxy option at a running Glues proxy server to keep data outside the browser sandbox. + > **Web build:** Configuration is stored in WebStorage (LocalStorage). **Instant** notes are in-memory and ephemeral; use the **Proxy** backend to persist data to a running Glues proxy server. Local/Git/Mongo are not available in the browser build.
74-74: Be explicit about the output path of the static bundle.Small clarity boost by showing the repo-relative path.
- To produce a static bundle instead, run `trunk build --release` from the same directory; the optimized artifacts will be emitted under `dist/`. + To produce a static bundle instead, run `trunk build --release` from the same directory; the optimized artifacts will be emitted under `tui/web/dist/`.
29-29: Typo: “user” → “use”.Minor copy fix.
-paru -S glues # user your favorite AUR helper +paru -S glues # use your favorite AUR helper
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: coverage
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
README.md (6)
54-54: Fix markdownlint MD004: use asterisks for list bullets.Line 54 uses "-" while the rest of the doc prefers "". Switch to "" to satisfy MD004 and keep style consistent. Also consider updating lines 52–53 for consistency.
- - In the TUI entry menu choose `Proxy` (option `[5]` on the native build), enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally. + * In the TUI entry menu choose `Proxy` (option `[5]` on the native build), enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally.
56-57: Add a brief security note for browser storage and remote proxy.Make risks explicit: LocalStorage isn’t suitable for secrets; prefer Proxy for persistence and HTTPS when not on localhost.
- > **Web build:** The browser version of Glues persists configuration through GlueSQL WebStorage (LocalStorage) and currently exposes the **Instant** and **Proxy** backends. Point the Proxy option at a running Glues proxy server to keep data outside the browser sandbox. + > **Web build:** The browser version of Glues persists configuration through GlueSQL WebStorage (LocalStorage) and currently exposes the **Instant** and **Proxy** backends. Point the Proxy option at a running Glues proxy server to keep data outside the browser sandbox. Avoid storing sensitive data in the browser; use Proxy for persistence, and prefer HTTPS when the proxy is not localhost.
60-66: Consider pinning/documenting Trunk version for reproducible builds.Add a known-good Trunk version (or note CI’s version) to avoid local/CI mismatches.
69-71: Tiny UX hint: tell users how to change the serve port.Useful when 8080 is busy.
- trunk serve --release + trunk serve --release # use --port <N> if 8080 is occupied
72-72: Mention CORS when proxying to another origin.Users will otherwise hit blocked requests.
-3. Open [http://localhost:8080](http://localhost:8080) in your browser. Use **Instant** for ephemeral notes, or connect to a running proxy server via the **Proxy** option for persistence. +3. Open [http://localhost:8080](http://localhost:8080) in your browser. Use **Instant** for ephemeral notes, or connect to a running proxy server via the **Proxy** option for persistence. If the proxy is on another origin, ensure CORS is allowed on the server.
74-74: Clarify exact output path.State “tui/web/dist/” to match the project layout.
-To produce a static bundle instead, run `trunk build --release` from the same directory; the optimized artifacts will be emitted under `dist/`. +To produce a static bundle instead, run `trunk build --release` from the same directory; the optimized artifacts will be emitted under `tui/web/dist/`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/rust.yml(1 hunks)README.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/rust.yml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Wasm Build
- GitHub Check: coverage
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
server/Cargo.toml (1)
25-26: Verify tower-http CORS security configuration.The addition of
tower-httpwith CORS features is necessary for the web client integration, but based on the existing review comment, the CORS configuration inserver/src/lib.rsappears to use overly permissive settings that could pose security risks in production.Based on the past review comments and security best practices, the current CORS configuration using
allow_origin(Any),allow_methods(Any), andallow_headers(Any)is too permissive and should be restricted via environment variables, with explicit origin whitelisting, method restrictions, andmax_ageconfiguration for preflight caching.The existing review comment provides a comprehensive solution - please implement the suggested CORS restrictions before production deployment.
🧹 Nitpick comments (8)
.github/workflows/rust.yml (3)
59-60: Remove redundant wasm32 target additionLine 60 duplicates the target installation that's already performed by
dtolnay/rust-toolchain@stableon lines 56-58.Apply this diff to remove the redundant step:
- - name: Ensure wasm32 target - run: rustup target add wasm32-unknown-unknown
50-54: Consider making wasm_build independent of rust_testThe
wasm_buildjob depends onrust_test, which could unnecessarily delay feedback if tests fail. Since wasm compilation checks different aspects (target compatibility) than functional tests, these could run in parallel.Consider removing the dependency or making it conditional:
wasm_build: name: Wasm Build runs-on: ubuntu-latest - needs: [rust_test]
65-66: Consider usingcargo-binstallfor faster trunk installationInstalling trunk from source with
--forcecan be slow. Consider usingcargo-binstallfirst to download a pre-built binary, falling back to source only if necessary.- name: Install trunk - run: cargo install trunk --locked --force + run: | + cargo install cargo-binstall + cargo binstall trunk -y --locked || cargo install trunk --lockedREADME.md (1)
53-55: Fix inconsistent unordered list styleThe markdown uses dashes instead of asterisks for list items, which is inconsistent with the rest of the document.
Apply this diff to maintain consistency:
- - Run the bundled proxy server with `cargo run -p glues-server -- memory` (replace `memory` with `file`, `git`, or `mongo` as needed). The server listens on `127.0.0.1:4000` by default; use `--listen` to change the address. - - In the TUI entry menu choose `Proxy` (option `[5]` on the native build), enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally. - - After installing via `cargo install glues`, start the proxy with `glues server memory` (or `file`, `git`, `mongo`) to use the same executable for both client and server flows. + * Run the bundled proxy server with `cargo run -p glues-server -- memory` (replace `memory` with `file`, `git`, or `mongo` as needed). The server listens on `127.0.0.1:4000` by default; use `--listen` to change the address. + * In the TUI entry menu choose `Proxy` (option `[5]` on the native build), enter the proxy URL (e.g. `http://127.0.0.1:4000`), and Glues will talk to the remote backend just like it does locally. + * After installing via `cargo install glues`, start the proxy with `glues server memory` (or `file`, `git`, `mongo`) to use the same executable for both client and server flows.server/src/lib.rs (3)
110-128: Consider extracting backend creation to a factory patternThe
build_backendfunction mixes async I/O operations with factory logic. Consider extracting this into a proper factory pattern for better testability and separation of concerns.Additionally, ensure all backends handle connection failures gracefully. The MongoDB backend might benefit from connection retry logic.
85-88: CORS configuration may be too permissive for productionUsing
Anyfor all CORS settings allows any origin to access the server. While this may be intentional for development, consider making this configurable for production deployments.Would you like me to implement configurable CORS settings based on environment variables or CLI arguments?
143-153: Avoid silently clearing the transition queue — process or bound it.core/src/task.rs pushes transitions; tui/src/app.rs consumes them by popping (tui/src/app.rs:59–61). server/src/lib.rs currently clears the queue every 500ms in spawn_transition_drain (server/src/lib.rs:143–153), which silently drops transitions in server/headless mode instead of processing them. Replace clear() with draining+processing/forwarding, or use a bounded queue/channel to prevent unbounded growth and lost events.
tui/Cargo.toml (1)
27-30: Consider documenting the feature flagsThe
cliandwebfeatures are defined but not documented. Consider adding doc comments to explain their purpose and when to use each.[features] default = ["cli"] +## Enables CLI-specific functionality for terminal-based usage cli = [] +## Enables web-specific functionality for browser-based WASM deployment web = []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktui/tests/snapshots/entry__help_open.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
.github/workflows/rust.yml(1 hunks)Cargo.toml(1 hunks)README.md(1 hunks)bin/glues/Cargo.toml(1 hunks)bin/glues/src/main.rs(1 hunks)server/Cargo.toml(1 hunks)server/src/lib.rs(1 hunks)server/src/main.rs(1 hunks)tui/Cargo.toml(1 hunks)tui/src/cli.rs(1 hunks)tui/src/lib.rs(1 hunks)tui/src/main.rs(1 hunks)tui/src/views/dialog/help.rs(1 hunks)tui/tests/editor_insert.rs(1 hunks)tui/tests/editor_normal.rs(1 hunks)tui/tests/editor_visual.rs(1 hunks)tui/tests/entry.rs(3 hunks)tui/tests/notebook_tree.rs(1 hunks)tui/tests/notebook_views.rs(1 hunks)tui/tests/tester.rs(4 hunks)tui/web/index.html(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tui/tests/editor_normal.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- tui/src/lib.rs
- tui/tests/editor_insert.rs
- tui/tests/notebook_views.rs
- tui/src/views/dialog/help.rs
- tui/tests/entry.rs
- tui/tests/tester.rs
- tui/web/index.html
- Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (5)
bin/glues/src/main.rs (4)
server/src/main.rs (1)
main(4-7)tui/src/main.rs (2)
main(6-8)main(11-11)server/src/lib.rs (1)
run(70-104)tui/src/cli.rs (1)
run(44-62)
tui/src/cli.rs (3)
server/src/lib.rs (3)
parse_args(66-68)run(70-104)run_cli(106-108)tui/src/theme.rs (1)
set_theme(64-66)tui/tests/tester.rs (1)
new(35-48)
server/src/lib.rs (2)
core/src/task.rs (1)
handle_tasks(21-42)core/src/backend/local/native.rs (3)
file(40-50)git(52-70)mongo(72-85)
server/src/main.rs (1)
server/src/lib.rs (2)
parse_args(66-68)run(70-104)
tui/src/main.rs (2)
server/src/lib.rs (1)
run_cli(106-108)tui/src/cli.rs (1)
run_cli(64-66)
🪛 markdownlint-cli2 (0.17.2)
README.md
53-53: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
54-54: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
55-55: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Wasm Build
🔇 Additional comments (18)
tui/tests/editor_visual.rs (1)
6-6: Unified KeyCode import — LGTM; double‑check variant namingSwitch to
glues_tui::input::KeyCodeis consistent with the new input API. Please confirmKeyCode::Escexists (notEscape) to match the calls below.You can reuse the verification script from the other comment; it will also cover this file.
tui/tests/notebook_tree.rs (1)
6-6: Approve KeyCode import migration — verifiedNo legacy ratatui/crossterm KeyCode imports remain; KeyCode variants (Char, Esc, Enter, Backspace, etc.) are defined and mapped in tui/src/input.rs; [lib] name is "glues_tui" in tui/Cargo.toml.
README.md (1)
59-75: Excellent documentation for the web build featureThe Web Build section provides clear prerequisites, step-by-step instructions, and helpful context about the browser-based version's capabilities and limitations. The documentation effectively guides users through both development (
trunk serve) and production (trunk build) workflows.server/src/lib.rs (1)
1-32: Well-structured imports and module organizationThe imports are properly organized with clear separation between external and internal dependencies. The use of type aliases for AsyncMutex helps disambiguate between sync and async mutex types.
tui/Cargo.toml (2)
14-15: Good choice of crate types for dual-target supportIncluding both
rlibandcdylibcrate types enables the library to work as a regular Rust dependency and as a WASM module, which aligns well with the dual CLI/web architecture.
40-63: Well-structured target-specific dependenciesThe separation of dependencies by target architecture is clean and appropriate. The native target gets file system and database backends while WASM gets web storage and browser-specific utilities.
bin/glues/src/main.rs (2)
8-28: Clean and intuitive CLI structureThe CLI design with optional subcommands is well-structured, allowing
gluesto run the TUI by default whileglues serverruns the proxy server. This provides a good user experience.
30-38: Approve — error propagation and color_eyre initialization verifiedmain() propagates errors and both subcommands call color_eyre::install() (server/src/lib.rs:71, tui/src/cli.rs:56).
server/src/main.rs (1)
1-7: Clean refactoring to thin main entry point.The refactoring successfully extracts server startup logic to
glues_serverlibrary, creating a clean separation between the binary entry point and core server functionality. This aligns well with the PR's goal of creating a unified CLI and server architecture.server/Cargo.toml (2)
2-2: Package name change looks appropriate.The renaming from
glues-proxy-servertoglues-serveris more concise and better reflects the server's broader functionality beyond just proxying.
11-17: Well-structured library and binary targets.The dual lib/bin configuration is correctly structured with appropriate naming conventions (
glues_serverlibrary,glues-serverbinary) and proper path specification.bin/glues/Cargo.toml (1)
1-19: Well-organized unified CLI manifest.The new CLI entry point effectively consolidates access to both TUI and server functionality. The dependency structure is appropriate, using local path dependencies with proper feature gating (
glues-tuiwithdefault-features = falseandfeatures = ["cli"]).tui/src/main.rs (2)
1-8: Proper platform-specific conditional compilation.The
#[cfg(not(target_arch = "wasm32"))]guards appropriately isolate CLI functionality from WASM builds, delegating to the centralizedcli::run_cli()function.
10-11: WASM compatibility handled correctly.The no-op
main()for WASM targets satisfies the entry point requirement while avoiding unsupported functionality.tui/src/cli.rs (4)
1-7: Proper platform gating and imports.The file-level
#[cfg(not(target_arch = "wasm32"))]guard and imports are correctly structured for non-WASM builds only.
9-31: Well-designed CLI argument structure.The
ThemeArgenum covers comprehensive theme options, and theTuiArgsstruct properly integrates with Clap's derive macros. TheDefaultimplementation is consistent with the Clap default.
44-62: Comprehensive theme initialization and terminal setup.The
runfunction properly maps theme arguments to constants, initializes necessary subsystems (config,logger,color_eyre), and handles Ratatui terminal lifecycle with proper cleanup viaratatui::restore().
64-66: Consistent API pattern with server module.The
run_cli()convenience function matches the pattern used inserver/src/lib.rs, providing a unified interface for CLI execution across modules.
panarch
changed the title
Summary by CodeRabbit
New Features
Changes
Documentation
Tests
Chores