From 12e6815f8d60068645ab4c2bdefee322fa06a589 Mon Sep 17 00:00:00 2001 From: Eric Wendland Date: Sun, 5 Jul 2026 22:39:15 +0200 Subject: [PATCH] feat: add local doctor checks --- README.md | 9 + crates/geth-cli/src/lib.rs | 34 +++ crates/geth-node/src/doctor.rs | 338 +++++++++++++++++++++++++++ crates/geth-node/src/lib.rs | 1 + crates/geth/tests/bootstrap.rs | 19 ++ docs/command-stability.md | 1 + docs/production-readiness-roadmap.md | 14 +- 7 files changed, 409 insertions(+), 7 deletions(-) create mode 100644 crates/geth-node/src/doctor.rs diff --git a/README.md b/README.md index 6f90cef..17fa515 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,7 @@ The bootstrap implementation provides: - `geth daemon service install|uninstall|start|stop|status|print` - `geth status` - `geth wait daemon|peer|sync --timeout-ms ` +- `geth doctor` - `geth backup create --out ` - `geth backup restore --target-home ` - `geth node id` @@ -434,6 +435,14 @@ the private SSH keys. `geth backup restore --target-home ` restores into a separate empty target home for validation. It refuses to overwrite a non-empty target. +## Doctor + +`geth doctor` is a local operational check that works even when the daemon is +not reachable. It checks config parsing, local daemon socket health, +`ssh-keygen` availability, metadata-store readability, imported peer-card +validity, and representative peer grants when local metadata is available. Use +`--json` for scripts. + ## Owner And Node Management The intended owner setup is SSH-admin-rooted: diff --git a/crates/geth-cli/src/lib.rs b/crates/geth-cli/src/lib.rs index 79eeaf2..74f269f 100644 --- a/crates/geth-cli/src/lib.rs +++ b/crates/geth-cli/src/lib.rs @@ -372,6 +372,7 @@ pub enum Command { #[command(subcommand)] command: BackupCommand, }, + Doctor, Node { #[command(subcommand)] command: NodeCommand, @@ -1341,6 +1342,15 @@ async fn run_inner(cli: Cli) -> Result<()> { run_backup_command(&paths, command, cli.json || cli.jsonl) .context("run backup command")?; } + Command::Doctor => { + let report = geth_node::doctor::run_doctor(&paths) + .await + .context("run doctor")?; + print_doctor_report(&report, cli.json || cli.jsonl)?; + if !report.ok { + std::process::exit(1); + } + } Command::Ssh { command: SshCommand::Proxy { @@ -2241,6 +2251,7 @@ fn request_for_command(command: Command) -> Result { | Command::Init { .. } | Command::Daemon { .. } | Command::Backup { .. } + | Command::Doctor | Command::Wait { .. } => { bail!("command is handled directly") } @@ -2316,6 +2327,29 @@ fn run_backup_command(paths: &GethPaths, command: BackupCommand, json: bool) -> Ok(()) } +fn print_doctor_report(report: &geth_node::doctor::DoctorReport, json: bool) -> Result<()> { + if json { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "type": "doctor", + "ok": report.ok, + "checks": report.checks, + }))? + ); + return Ok(()); + } + + println!("doctor: {}", if report.ok { "ok" } else { "failed" }); + for check in &report.checks { + println!("{:?}\t{}\t{}", check.status, check.code, check.message); + if let Some(hint) = &check.hint { + println!("hint\t{}\t{}", check.code, hint); + } + } + Ok(()) +} + async fn run_wait_command(paths: &GethPaths, command: WaitCommand) -> Result { match command { WaitCommand::Daemon { diff --git a/crates/geth-node/src/doctor.rs b/crates/geth-node/src/doctor.rs new file mode 100644 index 0000000..d8aaf7c --- /dev/null +++ b/crates/geth-node/src/doctor.rs @@ -0,0 +1,338 @@ +use crate::{NodeError, send_control}; +use geth_auth::AuthOp; +use geth_config::{GethConfig, GethPaths}; +use geth_control::{ControlRequest, ControlResponse}; +use geth_discovery::PeerCard; +use geth_store::Store; +use geth_types::PrincipalId; +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct DoctorReport { + pub ok: bool, + pub checks: Vec, +} + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct DoctorCheck { + pub code: String, + pub status: DoctorStatus, + pub message: String, + pub hint: Option, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum DoctorStatus { + Ok, + Warn, + Fail, +} + +pub async fn run_doctor(paths: &GethPaths) -> Result { + let mut checks = Vec::new(); + check_config(paths, &mut checks); + check_daemon(paths, &mut checks).await; + check_ssh_keygen(&mut checks); + check_store_metadata(paths, &mut checks); + let ok = checks + .iter() + .all(|check| check.status != DoctorStatus::Fail); + Ok(DoctorReport { ok, checks }) +} + +fn check_config(paths: &GethPaths, checks: &mut Vec) { + match GethConfig::load(&paths.config_file()) { + Ok(_) => checks.push(ok( + "config-ok", + format!("config is readable: {}", paths.config_file().display()), + )), + Err(error) => checks.push(fail( + "config-invalid", + format!("config is invalid: {error}"), + "fix config.toml or move it aside and rerun `geth init` to regenerate defaults", + )), + } +} + +async fn check_daemon(paths: &GethPaths, checks: &mut Vec) { + if !paths.socket_path().exists() { + checks.push(fail( + "daemon-not-running", + format!("daemon socket is absent: {}", paths.socket_path().display()), + "start the daemon with `geth daemon run` or the user service command for your platform", + )); + return; + } + + match send_control(paths, ControlRequest::Status).await { + Ok(ControlResponse::Status(status)) => checks.push(ok( + "daemon-ok", + format!( + "daemon responded as {} with endpoint {}", + status.node_id, + status.endpoint_id.as_deref().unwrap_or("not-started") + ), + )), + Ok(other) => checks.push(fail( + "daemon-unexpected-response", + format!("daemon returned unexpected response: {other:?}"), + "restart the daemon and rerun `geth doctor`", + )), + Err(error) => checks.push(fail( + "daemon-stale-socket", + format!( + "daemon socket exists but control connection failed: {}", + error + ), + "stop any stale daemon, remove the socket if no daemon is running, then start `geth daemon run`", + )), + } +} + +fn check_ssh_keygen(checks: &mut Vec) { + if std::process::Command::new("ssh-keygen") + .arg("-?") + .output() + .is_ok() + { + checks.push(ok( + "ssh-keygen-ok", + "ssh-keygen is available for SSH signature and certificate workflows", + )); + } else { + checks.push(fail( + "ssh-keygen-missing", + "ssh-keygen is not available on PATH", + "install OpenSSH client tools before using admin signing, cert approval, or KRL export workflows", + )); + } +} + +fn check_store_metadata(paths: &GethPaths, checks: &mut Vec) { + if !paths.metadata_db().exists() { + checks.push(fail( + "store-missing", + format!( + "metadata store is absent: {}", + paths.metadata_db().display() + ), + "run `geth init` before starting the daemon or creating resources", + )); + return; + } + + let store = match Store::open(&paths.metadata_db()) { + Ok(store) => store, + Err(error) => { + checks.push(fail( + "store-open-failed", + format!("metadata store could not be opened: {error}"), + "restore from backup or inspect the SQLite file before continuing", + )); + return; + } + }; + checks.push(ok( + "store-ok", + format!( + "metadata store is readable: {}", + paths.metadata_db().display() + ), + )); + + check_peer_cards(&store, checks); + check_peer_grants(&store, checks); +} + +fn check_peer_cards(store: &Store, checks: &mut Vec) { + let peers = match store.list_peer_cards() { + Ok(peers) => peers, + Err(error) => { + checks.push(fail( + "peer-card-read-failed", + format!("could not read peer cards: {error}"), + "inspect the metadata store or restore from backup", + )); + return; + } + }; + if peers.is_empty() { + checks.push(warn( + "peer-card-none", + "no imported peer cards are available", + "exchange peer cards with `geth peer export` and `geth peer import` before remote operations", + )); + return; + } + + let mut invalid = Vec::new(); + for peer in peers { + match serde_json::from_str::(&peer.card_json) + .map_err(NodeError::from) + .and_then(|card| card.validate_candidate().map_err(NodeError::from)) + { + Ok(()) => {} + Err(error) => invalid.push(format!("{}: {error}", peer.peer_id)), + } + } + if invalid.is_empty() { + checks.push(ok( + "peer-cards-ok", + "imported peer cards are valid candidates", + )); + } else { + checks.push(warn( + "peer-card-invalid", + format!( + "some imported peer cards are invalid: {}", + invalid.join("; ") + ), + "re-import a fresh signed peer card from the affected peer", + )); + } +} + +fn check_peer_grants(store: &Store, checks: &mut Vec) { + let peers = match store.list_peer_cards() { + Ok(peers) if !peers.is_empty() => peers, + _ => return, + }; + let resources = match store.list_resources() { + Ok(resources) if !resources.is_empty() => resources, + _ => return, + }; + let auth_ops = match store.list_auth_ops() { + Ok(ops) => ops + .into_iter() + .filter_map(|stored| serde_json::from_str::(&stored.op_json).ok()) + .collect::>(), + Err(error) => { + checks.push(fail( + "auth-op-read-failed", + format!("could not read auth operations: {error}"), + "inspect the metadata store or restore from backup", + )); + return; + } + }; + + let mut missing = Vec::new(); + for peer in peers.iter().take(4) { + for resource in resources.iter().take(6) { + let Some(capability) = representative_capability(&resource.kind) else { + continue; + }; + let explanation = geth_auth::explain_auth_ops( + &auth_ops + .iter() + .filter(|op| op.resource.as_str() == resource.resource_id) + .cloned() + .collect::>(), + PrincipalId::new(peer.peer_id.clone()), + resource.resource_id.clone().into(), + capability.into(), + ); + if !explanation.allowed { + missing.push(format!( + "{} lacks {} on {}", + peer.peer_id, capability, resource.resource_id + )); + } + } + } + + if missing.is_empty() { + checks.push(ok( + "peer-grants-ok", + "imported peers have representative grants for known resources", + )); + } else { + checks.push(warn( + "peer-grants-missing", + format!("missing representative grants: {}", missing.join("; ")), + "use `geth node grant` or `geth auth grant` for the resource/capability needed by the peer", + )); + } +} + +fn representative_capability(kind: &str) -> Option<&'static str> { + match kind { + "cas" => Some("cas.fetch"), + "kv" => Some("kv.sync"), + "db" => Some("db.sync"), + "document" => Some("document.sync"), + "pubsub" => Some("pubsub.subscribe"), + "pipe" => Some("pipe.connect"), + "ssh-proxy" => Some("ssh_proxy.connect"), + "overlay" => Some("overlay.route"), + _ => None, + } +} + +fn ok(code: impl Into, message: impl Into) -> DoctorCheck { + DoctorCheck { + code: code.into(), + status: DoctorStatus::Ok, + message: message.into(), + hint: None, + } +} + +fn warn( + code: impl Into, + message: impl Into, + hint: impl Into, +) -> DoctorCheck { + DoctorCheck { + code: code.into(), + status: DoctorStatus::Warn, + message: message.into(), + hint: Some(hint.into()), + } +} + +fn fail( + code: impl Into, + message: impl Into, + hint: impl Into, +) -> DoctorCheck { + DoctorCheck { + code: code.into(), + status: DoctorStatus::Fail, + message: message.into(), + hint: Some(hint.into()), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn doctor_reports_missing_daemon_and_store_before_init() { + let home = tempfile::tempdir().expect("home"); + let paths = GethPaths::from_home(home.path()); + let report = run_doctor(&paths).await.expect("doctor"); + assert!(!report.ok); + assert!(has_code(&report, "daemon-not-running")); + assert!(has_code(&report, "store-missing")); + } + + #[tokio::test] + async fn doctor_reports_bad_config_and_stale_socket() { + let home = tempfile::tempdir().expect("home"); + let paths = GethPaths::from_home(home.path()); + paths.ensure_base_dirs().expect("dirs"); + std::fs::write(paths.config_file(), "[iroh]\nrelay_mode = 1").expect("bad config"); + std::fs::write(paths.socket_path(), "stale").expect("socket placeholder"); + let report = run_doctor(&paths).await.expect("doctor"); + assert!(!report.ok); + assert!(has_code(&report, "config-invalid")); + assert!(has_code(&report, "daemon-stale-socket")); + } + + fn has_code(report: &DoctorReport, code: &str) -> bool { + report.checks.iter().any(|check| check.code == code) + } +} diff --git a/crates/geth-node/src/lib.rs b/crates/geth-node/src/lib.rs index 98049f6..91280cf 100644 --- a/crates/geth-node/src/lib.rs +++ b/crates/geth-node/src/lib.rs @@ -1,5 +1,6 @@ pub mod backup; mod daemon; +pub mod doctor; mod peer_client; mod runtime; pub mod service; diff --git a/crates/geth/tests/bootstrap.rs b/crates/geth/tests/bootstrap.rs index 3b93e65..440e7ee 100644 --- a/crates/geth/tests/bootstrap.rs +++ b/crates/geth/tests/bootstrap.rs @@ -462,6 +462,25 @@ fn wait_daemon_reports_json_timeout_without_daemon() { assert!(report["attempts"].as_u64().expect("attempts") >= 1); } +#[test] +fn doctor_reports_json_failures_without_daemon() { + let home = tempfile::tempdir().expect("tempdir"); + let output = run_geth(home.path(), &["--json", "doctor"]); + assert!(!output.status.success()); + let report = + serde_json::from_slice::(&output.stdout).expect("decode doctor report"); + assert_eq!(report["type"], "doctor"); + assert_eq!(report["ok"], false); + let codes = report["checks"] + .as_array() + .expect("checks") + .iter() + .map(|check| check["code"].as_str().expect("code")) + .collect::>(); + assert!(codes.contains(&"daemon-not-running")); + assert!(codes.contains(&"store-missing")); +} + #[test] fn peer_ping_uses_daemon_owned_iroh_endpoint() { if skip_iroh_integration_tests() { diff --git a/docs/command-stability.md b/docs/command-stability.md index 3126614..cc3a0c7 100644 --- a/docs/command-stability.md +++ b/docs/command-stability.md @@ -23,6 +23,7 @@ The following command families are intended to be stable automation surfaces: - `geth daemon run` - `geth daemon service install|uninstall|start|stop|status|print` - `geth status` +- `geth doctor` - `geth node id` - `geth backup create|restore` - `geth node list|rename|revoke|grant|revoke-grant|endpoint-add|endpoint-revoke` diff --git a/docs/production-readiness-roadmap.md b/docs/production-readiness-roadmap.md index 1443c0c..9e6e6ed 100644 --- a/docs/production-readiness-roadmap.md +++ b/docs/production-readiness-roadmap.md @@ -283,15 +283,15 @@ Goal: make production failures diagnosable from the CLI and logs. - `[x]` Iroh relay/local-discovery state is visible without exposing unrelated config secrets. -- `[ ]` Add `geth doctor`. +- `[x]` Add `geth doctor`. Acceptance criteria: - - `[ ]` Doctor detects daemon not running. - - `[ ]` Doctor detects stale local socket. - - `[ ]` Doctor detects bad config. - - `[ ]` Doctor detects missing `ssh-keygen`. - - `[ ]` Doctor explains missing grants and endpoint mismatches when enough + - `[x]` Doctor detects daemon not running. + - `[x]` Doctor detects stale local socket. + - `[x]` Doctor detects bad config. + - `[x]` Doctor detects missing `ssh-keygen`. + - `[x]` Doctor explains missing grants and endpoint mismatches when enough local metadata exists. - - `[ ]` Doctor has JSON output for scripts. + - `[x]` Doctor has JSON output for scripts. ## Phase 9: Packaging, Upgrade, And Release Discipline