Skip to content

Commit ef0ae42

Browse files
tcconnallytcconnallyclaude
authored
fix(cli): honor top-level --db when a subcommand follows (#313) (#314)
`mimir --db PATH serve` silently ignored the flag and used the subcommand's default db path — a serious footgun for MCP host configs (`args: ["--db", P, "serve"]` connected to the wrong database with no warning). The top-level `--db` is documented for `mimir --db PATH` but was never propagated once a subcommand was present; each subcommand's own defaulted `--db` won. Propagate the top-level `--db` into the subcommand when the user didn't pass a subcommand-level one (i.e. it still equals the default). An explicit subcommand-level `--db` always wins; ObsidianSync's Option<String> db is filled when None. Added a `Commands::db_field_mut()` accessor + `apply_top_level_db()` called once in main(), plus tests for the propagate / explicit-wins / obsidian cases. Verified e2e: `mimir --db P stats` now creates/opens the DB at P. Closes #313. Co-authored-by: tcconnally <hermes@perseus.observer> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 86ecbca commit ef0ae42

1 file changed

Lines changed: 85 additions & 1 deletion

File tree

src/main.rs

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,54 @@ enum Commands {
365365
},
366366
}
367367

368+
impl Commands {
369+
/// Mutable handle to this subcommand's defaulted `--db String` field, if it
370+
/// has one. `Migrate`/`Keygen` have no database; `ObsidianSync` uses an
371+
/// `Option<String>` and is handled separately (#313).
372+
fn db_field_mut(&mut self) -> Option<&mut String> {
373+
match self {
374+
Commands::Write { db, .. }
375+
| Commands::Serve { db, .. }
376+
| Commands::Forget { db, .. }
377+
| Commands::Prune { db, .. }
378+
| Commands::Decay { db, .. }
379+
| Commands::Reindex { db, .. }
380+
| Commands::Stats { db, .. }
381+
| Commands::StateDigest { db, .. }
382+
| Commands::VaultExport { db, .. }
383+
| Commands::VaultImport { db, .. }
384+
| Commands::Purge { db, .. }
385+
| Commands::Doctor { db, .. } => Some(db),
386+
Commands::ObsidianSync { .. } | Commands::Migrate { .. } | Commands::Keygen { .. } => {
387+
None
388+
}
389+
}
390+
}
391+
}
392+
393+
/// #313: honor the documented top-level `--db` even when a subcommand follows
394+
/// (`mimir --db PATH serve`). Each subcommand carries its own `--db` defaulted to
395+
/// `default_db_path()`; when the user did not pass a subcommand-level `--db` (it
396+
/// still equals the default), the top-level flag fills it in so it is no longer
397+
/// silently ignored. An explicit subcommand-level `--db` always wins.
398+
fn apply_top_level_db(cli: &mut Cli) {
399+
let Some(top_db) = cli.db.clone() else {
400+
return;
401+
};
402+
let Some(cmd) = cli.command.as_mut() else {
403+
return;
404+
};
405+
if let Commands::ObsidianSync { db, .. } = cmd {
406+
if db.is_none() {
407+
*db = Some(top_db);
408+
}
409+
} else if let Some(db) = cmd.db_field_mut() {
410+
if *db == default_db_path() {
411+
*db = top_db;
412+
}
413+
}
414+
}
415+
368416
fn default_db_path() -> String {
369417
std::env::var("MIMIR_DB_PATH").unwrap_or_else(|_| {
370418
let home = std::env::var("HOME")
@@ -475,7 +523,8 @@ fn run_doctor(db_path: &str) {
475523
}
476524

477525
fn main() {
478-
let cli = Cli::parse();
526+
let mut cli = Cli::parse();
527+
apply_top_level_db(&mut cli); // #313: `mimir --db PATH serve` must honor --db
479528

480529
match cli.command {
481530
Some(Commands::Keygen { key_file }) => {
@@ -1284,6 +1333,41 @@ mod tests {
12841333
}
12851334
}
12861335

1336+
#[test]
1337+
fn top_level_db_propagates_to_serve_subcommand() {
1338+
// #313: `mimir --db PATH serve` must NOT silently fall back to the
1339+
// subcommand's default db — the documented top-level flag fills it in.
1340+
let mut cli = Cli::parse_from(["mimir", "--db", "/tmp/top.db", "serve"]);
1341+
apply_top_level_db(&mut cli);
1342+
match cli.command {
1343+
Some(Commands::Serve { db, .. }) => assert_eq!(db, "/tmp/top.db"),
1344+
_ => panic!("expected serve subcommand"),
1345+
}
1346+
}
1347+
1348+
#[test]
1349+
fn explicit_subcommand_db_wins_over_top_level() {
1350+
// #313: an explicit subcommand-level `--db` always beats the top-level one.
1351+
let mut cli =
1352+
Cli::parse_from(["mimir", "--db", "/tmp/top.db", "serve", "--db", "/tmp/sub.db"]);
1353+
apply_top_level_db(&mut cli);
1354+
match cli.command {
1355+
Some(Commands::Serve { db, .. }) => assert_eq!(db, "/tmp/sub.db"),
1356+
_ => panic!("expected serve subcommand"),
1357+
}
1358+
}
1359+
1360+
#[test]
1361+
fn top_level_db_propagates_to_obsidian_sync() {
1362+
// #313: ObsidianSync uses an Option<String> db; the top-level flag fills it.
1363+
let mut cli = Cli::parse_from(["mimir", "--db", "/tmp/top.db", "obsidian-sync", "/tmp/v"]);
1364+
apply_top_level_db(&mut cli);
1365+
match cli.command {
1366+
Some(Commands::ObsidianSync { db, .. }) => assert_eq!(db.as_deref(), Some("/tmp/top.db")),
1367+
_ => panic!("expected obsidian-sync subcommand"),
1368+
}
1369+
}
1370+
12871371
#[test]
12881372
fn parses_migrate_subcommand() {
12891373
let cli = Cli::parse_from([

0 commit comments

Comments
 (0)