refactor vault&migration&gemini, add context module#1157
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors vault, migration, and Gemini modules to use a unified VaultCore interface, reorganizes modules under a new integration crate, and updates various service commands to accept an AppContext with storage and vault dependencies.
- Extracted and implemented
VaultCoreInterfaceandVaultCorefor tenant‐agnostic vault operations. - Updated PKI, PGP, and Nostr modules to call vault through
VaultCoremethods instead of direct async static access. - Switched mono/mega/gateway commands and HTTP/SSH servers to use
AppContextandjupiter::storage::Storage.
Reviewed Changes
Copilot reviewed 80 out of 84 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vault/src/integration/vault_core.rs | Introduced VaultCore and VaultCoreInterface, consolidated vault API calls. |
| vault/src/pki.rs | Refactored PKI module to call vault via VaultCore and converted async logic. |
| vault/src/pgp.rs | Updated PGP key operations (gen, save, load, delete) to use VaultCore. |
| vault/src/nostr.rs | Changed Nostr ID handling to go through VaultCore and store/retrieve via vault. |
| vault/src/lib.rs | Defined Vault trait to unify vault usage; removed legacy static functions. |
| monobean/src/config.rs | Revised monobean base/cache path resolution to use common::config::mega_*. |
| mono/src/server/{ssh,https}_server.rs | Shifted from Context to AppContext, wired in vault and storage dependencies. |
| jupiter/src/storage/init.rs | Replaced inline Migrator::up with new apply_migrations helper. |
| gemini/src/util.rs | Adjusted P2P URI functions to accept peer ID parameters rather than static lookups. |
Comments suppressed due to low confidence (1)
vault/src/lib.rs:37
- The
save_to_vaultmethod currently swallows errors silently by ignoring the result. Consider returning aResult<(), MegaError>so callers can handle failures appropriately.
fn save_to_vault(&self, key: impl AsRef<str>, value: impl AsRef<str>) {
| let guard = self.core.read().unwrap(); | ||
| guard | ||
| .handle_request(&mut req) | ||
| .map_err(|_| MegaError::with_message("Failed to write to vault API")) |
There was a problem hiding this comment.
This error mapping discards the underlying error. Consider capturing and including the original error message, e.g., .map_err(|e| MegaError::with_message(format!("Failed to write to vault API: {}", e))).
| .map_err(|_| MegaError::with_message("Failed to write to vault API")) | |
| .map_err(|e| MegaError::with_message(format!("Failed to write to vault API: {}", e))) |
| let mut req = Request::new(path.as_ref()); | ||
| req.operation = Operation::Read; | ||
| req.client_token = self.token().to_string(); | ||
| let guard = self.core.read().unwrap(); |
There was a problem hiding this comment.
Using unwrap() on a poisoned RwLock can panic in production. Consider using expect("...") with context or propagating the error to avoid unexpected panics.
| let guard = self.core.read().unwrap(); | |
| let guard = self.core.read().expect("Failed to acquire read lock for vault core during read operation"); |
Massive changes applied due to the api change. I also tried to re-organize the modules like take
contextout of jupiter and make it a top-level crate.For now, monobean doesn't adapt to the api changes and I'll make it a task in ospp-2025 for starting up.