mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-10 22:43:37 +00:00
fabro doctor: check Docker daemon when Docker sandbox is enabled (#525)
## Summary Fixes #501. Adds a Docker sandbox diagnostics check so `fabro doctor` verifies the Docker daemon when the Docker sandbox provider is enabled. Disabled Docker providers are reported as disabled without touching the local daemon. ## What changed - Added `DockerSandboxProvider::check_daemon()` using Bollard `ping()` only, with no container/image side effects. - Added a `Docker Sandbox` check to server diagnostics with pass/error/timeout handling and operator remediation. - Updated demo diagnostics and doctor/server test fixtures so tests that do not exercise Docker explicitly disable the provider. - Added deterministic tests for enabled success, enabled failure, enabled timeout, and disabled skip paths. ## Verification - `cargo check -p fabro-server -p fabro-sandbox -p fabro-cli` - `cargo test -p fabro-server docker_sandbox --lib` - `cargo test -p fabro-server --features test-support diagnostics_reports_under_scoped_daytona_api_key --lib` - `cargo test -p fabro-cli --test it cmd::doctor` - `git diff --check` Not run locally: pinned nightly `fmt`/`clippy` because this environment has Homebrew Rust only and no `rustup` for `nightly-2026-04-14`. --------- Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
This commit is contained in:
parent
2fb2d93735
commit
94df98bb34
6 changed files with 258 additions and 28 deletions
|
|
@ -6,6 +6,7 @@ import {
|
|||
RouterProvider,
|
||||
useParams,
|
||||
} from "react-router";
|
||||
import { toast as sonnerToast } from "sonner";
|
||||
import {
|
||||
AskFabroUnavailableReasonEnum,
|
||||
QuestionType,
|
||||
|
|
@ -379,6 +380,13 @@ function textFromNode(
|
|||
return (node.children ?? []).map(textFromNode).join(" ");
|
||||
}
|
||||
|
||||
async function flushSonnerUpdates() {
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
});
|
||||
}
|
||||
|
||||
function textFromTestNode(node: TestRenderer.ReactTestInstance): string {
|
||||
return node.children.map((child) => {
|
||||
if (typeof child === "string") return child;
|
||||
|
|
@ -593,6 +601,7 @@ describe("RunDetail full-height child routes", () => {
|
|||
renderer.unmount();
|
||||
}
|
||||
});
|
||||
sonnerToast.dismiss();
|
||||
currentRunSummary = null;
|
||||
currentRunState = null;
|
||||
currentQuestions = [];
|
||||
|
|
@ -887,9 +896,7 @@ describe("RunDetail full-height child routes", () => {
|
|||
await deletion.promise;
|
||||
await Promise.resolve();
|
||||
});
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
await flushSonnerUpdates();
|
||||
|
||||
expect(deleteRunApiMock).toHaveBeenCalledTimes(1);
|
||||
expect(deleteRunApiMock.mock.calls[0]?.[0]).toBe("run_1");
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
use std::process::Output;
|
||||
|
||||
use fabro_config::Storage;
|
||||
use fabro_test::{fabro_snapshot, test_context, twin_openai};
|
||||
use fabro_test::{fabro_snapshot, require_env, test_context, twin_openai};
|
||||
use fabro_vault::{SecretType, Vault};
|
||||
|
||||
async fn run_success_output(mut cmd: assert_cmd::Command) -> Output {
|
||||
|
|
@ -22,12 +22,16 @@ fn toml_path(path: &std::path::Path) -> String {
|
|||
.replace('"', "\\\"")
|
||||
}
|
||||
|
||||
fn seed_openai_vault(storage_dir: &std::path::Path, api_key: &str) {
|
||||
fn seed_vault_secret(storage_dir: &std::path::Path, name: &str, value: &str) {
|
||||
let mut vault =
|
||||
Vault::load(Storage::new(storage_dir).secrets_path()).expect("test vault should load");
|
||||
vault
|
||||
.set("OPENAI_API_KEY", api_key, SecretType::Token, None)
|
||||
.expect("OpenAI credential should store in test vault");
|
||||
.set(name, value, SecretType::Token, None)
|
||||
.expect("credential should store in test vault");
|
||||
}
|
||||
|
||||
fn seed_openai_vault(storage_dir: &std::path::Path, api_key: &str) {
|
||||
seed_vault_secret(storage_dir, "OPENAI_API_KEY", api_key);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -75,7 +79,27 @@ fn dry_run_flag_is_rejected() {
|
|||
|
||||
#[fabro_macros::e2e_test(live("ANTHROPIC_API_KEY"))]
|
||||
fn live_doctor() {
|
||||
let context = test_context!();
|
||||
let mut context = test_context!();
|
||||
let api_key = require_env("ANTHROPIC_API_KEY")
|
||||
.expect("e2e_test live guard should require ANTHROPIC_API_KEY");
|
||||
let storage_dir = context.temp_dir.join("doctor-live-server-storage");
|
||||
context.write_home(
|
||||
".fabro/settings.toml",
|
||||
format!(
|
||||
r#"[server.storage]
|
||||
root = "{}"
|
||||
|
||||
[server.auth]
|
||||
methods = ["dev-token"]
|
||||
|
||||
[server.sandbox.providers.docker]
|
||||
enabled = false
|
||||
"#,
|
||||
toml_path(&storage_dir)
|
||||
),
|
||||
);
|
||||
seed_vault_secret(&storage_dir, "ANTHROPIC_API_KEY", &api_key);
|
||||
context.isolated_server();
|
||||
context.doctor().assert().success();
|
||||
}
|
||||
|
||||
|
|
@ -96,6 +120,9 @@ methods = ["dev-token"]
|
|||
|
||||
[server.integrations.github]
|
||||
strategy = "app"
|
||||
|
||||
[server.sandbox.providers.docker]
|
||||
enabled = false
|
||||
"#,
|
||||
toml_path(&storage_dir)
|
||||
),
|
||||
|
|
|
|||
|
|
@ -21,6 +21,15 @@ impl DockerSandboxProvider {
|
|||
Self
|
||||
}
|
||||
|
||||
pub async fn check_daemon() -> crate::Result<()> {
|
||||
let docker = Self::docker_client()?;
|
||||
docker
|
||||
.ping()
|
||||
.await
|
||||
.map_err(|err| crate::Error::context("Failed to reach Docker daemon", err))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn docker_client() -> crate::Result<Docker> {
|
||||
Docker::connect_with_local_defaults().map_err(crate::Error::docker_connect)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -670,7 +670,8 @@ pub(crate) async fn run_diagnostics(
|
|||
"checks": [
|
||||
{ "name": "LLM Providers", "status": "pass", "summary": "demo configured", "details": [], "remediation": null },
|
||||
{ "name": "GitHub App", "status": "pass", "summary": "demo configured", "details": [], "remediation": null },
|
||||
{ "name": "Sandbox", "status": "warning", "summary": "not configured", "details": [], "remediation": "Set DAYTONA_API_KEY to enable cloud sandbox execution" },
|
||||
{ "name": "Docker Sandbox", "status": "pass", "summary": "disabled", "details": [{ "text": "server.sandbox.providers.docker.enabled = false", "warn": false }], "remediation": null },
|
||||
{ "name": "Cloud Sandbox", "status": "warning", "summary": "not configured", "details": [], "remediation": "Set DAYTONA_API_KEY to enable cloud sandbox execution" },
|
||||
{ "name": "Brave Search", "status": "warning", "summary": "not configured", "details": [], "remediation": "Set BRAVE_SEARCH_API_KEY to enable web search" }
|
||||
]
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
use std::future::Future;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
|
|
@ -8,7 +9,7 @@ use fabro_llm::client::Client as LlmClient;
|
|||
use fabro_llm::model_test::{ModelTestStatus, run_basic_model_probe};
|
||||
use fabro_model::{Catalog, ProviderId};
|
||||
use fabro_redact::redact_string;
|
||||
use fabro_sandbox::daytona;
|
||||
use fabro_sandbox::{DockerSandboxProvider, daytona};
|
||||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::ServerAuthMethod;
|
||||
use fabro_types::settings::server::GithubIntegrationStrategy;
|
||||
|
|
@ -88,10 +89,11 @@ fn validate_session_secret(value: &str) -> Result<(), String> {
|
|||
}
|
||||
|
||||
pub async fn run_all(state: &AppState) -> DiagnosticsReport {
|
||||
let (llm, github, sandbox, brave) = tokio::join!(
|
||||
let (llm, github, docker_sandbox, cloud_sandbox, brave) = tokio::join!(
|
||||
check_llm_providers(state),
|
||||
check_github_app(state),
|
||||
check_sandbox(state),
|
||||
check_docker_sandbox(state),
|
||||
check_cloud_sandbox(state),
|
||||
check_brave_search(state),
|
||||
);
|
||||
let crypto = check_crypto(state);
|
||||
|
|
@ -101,7 +103,7 @@ pub async fn run_all(state: &AppState) -> DiagnosticsReport {
|
|||
sections: vec![
|
||||
CheckSection {
|
||||
title: "Credentials".to_string(),
|
||||
checks: vec![llm, github, sandbox, brave],
|
||||
checks: vec![llm, github, docker_sandbox, cloud_sandbox, brave],
|
||||
},
|
||||
CheckSection {
|
||||
title: "Configuration".to_string(),
|
||||
|
|
@ -548,10 +550,81 @@ async fn check_github_app(state: &AppState) -> CheckResult {
|
|||
}
|
||||
}
|
||||
|
||||
async fn check_sandbox(state: &AppState) -> CheckResult {
|
||||
async fn check_docker_sandbox(state: &AppState) -> CheckResult {
|
||||
check_docker_sandbox_with_probe(
|
||||
state
|
||||
.server_settings()
|
||||
.server
|
||||
.sandbox
|
||||
.providers
|
||||
.docker
|
||||
.enabled,
|
||||
|| async {
|
||||
DockerSandboxProvider::check_daemon()
|
||||
.await
|
||||
.map_err(|err| err.display_with_causes())
|
||||
},
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn check_docker_sandbox_with_probe<F, Fut>(
|
||||
enabled: bool,
|
||||
probe: F,
|
||||
probe_timeout: Duration,
|
||||
) -> CheckResult
|
||||
where
|
||||
F: FnOnce() -> Fut,
|
||||
Fut: Future<Output = Result<(), String>>,
|
||||
{
|
||||
if !enabled {
|
||||
return CheckResult {
|
||||
name: "Docker Sandbox".to_string(),
|
||||
status: CheckStatus::Pass,
|
||||
summary: "disabled".to_string(),
|
||||
details: vec![CheckDetail::new(
|
||||
"server.sandbox.providers.docker.enabled = false".to_string(),
|
||||
)],
|
||||
remediation: None,
|
||||
};
|
||||
}
|
||||
|
||||
let probe = timeout(probe_timeout, probe()).await;
|
||||
match probe {
|
||||
Ok(result) => docker_sandbox_probe_check(result),
|
||||
Err(_) => docker_sandbox_probe_check(Err("Docker daemon probe timed out".to_string())),
|
||||
}
|
||||
}
|
||||
|
||||
fn docker_sandbox_probe_check(probe: Result<(), String>) -> CheckResult {
|
||||
match probe {
|
||||
Ok(()) => CheckResult {
|
||||
name: "Docker Sandbox".to_string(),
|
||||
status: CheckStatus::Pass,
|
||||
summary: "daemon reachable".to_string(),
|
||||
details: vec![CheckDetail::new(
|
||||
"Docker daemon responded to ping".to_string(),
|
||||
)],
|
||||
remediation: None,
|
||||
},
|
||||
Err(err) => CheckResult {
|
||||
name: "Docker Sandbox".to_string(),
|
||||
status: CheckStatus::Error,
|
||||
summary: "daemon unavailable".to_string(),
|
||||
details: vec![CheckDetail::new(err)],
|
||||
remediation: Some(
|
||||
"Start Docker Desktop or the Docker daemon, fix Docker socket permissions, or disable Docker with `server.sandbox.providers.docker.enabled = false`."
|
||||
.to_string(),
|
||||
),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
async fn check_cloud_sandbox(state: &AppState) -> CheckResult {
|
||||
let Some(api_key) = state.vault_secret(EnvVars::DAYTONA_API_KEY) else {
|
||||
return CheckResult {
|
||||
name: "Sandbox".to_string(),
|
||||
name: "Cloud Sandbox".to_string(),
|
||||
status: CheckStatus::Warning,
|
||||
summary: "recommended, not configured".to_string(),
|
||||
details: Vec::new(),
|
||||
|
|
@ -564,14 +637,14 @@ async fn check_sandbox(state: &AppState) -> CheckResult {
|
|||
|
||||
match state.check_daytona_api_key(api_key).await {
|
||||
Ok(check) if check.ok() => CheckResult {
|
||||
name: "Sandbox".to_string(),
|
||||
name: "Cloud Sandbox".to_string(),
|
||||
status: CheckStatus::Pass,
|
||||
summary: format!("Daytona configured ({})", check.key_name),
|
||||
details: Vec::new(),
|
||||
remediation: None,
|
||||
},
|
||||
Ok(check) => CheckResult {
|
||||
name: "Sandbox".to_string(),
|
||||
name: "Cloud Sandbox".to_string(),
|
||||
status: CheckStatus::Error,
|
||||
summary: "Daytona API key is missing required scopes".to_string(),
|
||||
details: vec![CheckDetail::new(format!(
|
||||
|
|
@ -585,7 +658,7 @@ async fn check_sandbox(state: &AppState) -> CheckResult {
|
|||
)),
|
||||
},
|
||||
Err(err) => CheckResult {
|
||||
name: "Sandbox".to_string(),
|
||||
name: "Cloud Sandbox".to_string(),
|
||||
status: CheckStatus::Error,
|
||||
summary: "Daytona credential rejected".to_string(),
|
||||
details: vec![CheckDetail::new(format!("{err:#}"))],
|
||||
|
|
@ -920,16 +993,117 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn docker_sandbox_probe_passes_when_daemon_responds() {
|
||||
let result = docker_sandbox_probe_check(Ok(()));
|
||||
|
||||
assert_eq!(result.name, "Docker Sandbox");
|
||||
assert_eq!(result.status, CheckStatus::Pass);
|
||||
assert_eq!(result.summary, "daemon reachable");
|
||||
assert_eq!(result.remediation, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn docker_sandbox_probe_errors_when_daemon_is_unavailable() {
|
||||
let result = docker_sandbox_probe_check(Err("connection refused".to_string()));
|
||||
|
||||
assert_eq!(result.name, "Docker Sandbox");
|
||||
assert_eq!(result.status, CheckStatus::Error);
|
||||
assert_eq!(result.summary, "daemon unavailable");
|
||||
assert_eq!(result.details[0].text, "connection refused");
|
||||
assert_eq!(
|
||||
result.remediation.as_deref(),
|
||||
Some(
|
||||
"Start Docker Desktop or the Docker daemon, fix Docker socket permissions, or disable Docker with `server.sandbox.providers.docker.enabled = false`."
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn check_sandbox_ignores_env_backed_daytona_api_key() {
|
||||
async fn check_docker_sandbox_reports_pass_when_enabled_probe_succeeds() {
|
||||
let result = check_docker_sandbox_with_probe(
|
||||
true,
|
||||
|| async { Ok::<(), String>(()) },
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(result.name, "Docker Sandbox");
|
||||
assert_eq!(result.status, CheckStatus::Pass);
|
||||
assert_eq!(result.summary, "daemon reachable");
|
||||
assert_eq!(result.details[0].text, "Docker daemon responded to ping");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn check_docker_sandbox_reports_error_when_enabled_probe_fails() {
|
||||
let result = check_docker_sandbox_with_probe(
|
||||
true,
|
||||
|| async { Err::<(), String>("socket permission denied".to_string()) },
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(result.name, "Docker Sandbox");
|
||||
assert_eq!(result.status, CheckStatus::Error);
|
||||
assert_eq!(result.summary, "daemon unavailable");
|
||||
assert_eq!(result.details[0].text, "socket permission denied");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn check_docker_sandbox_reports_error_when_enabled_probe_times_out() {
|
||||
let result = check_docker_sandbox_with_probe(
|
||||
true,
|
||||
std::future::pending::<Result<(), String>>,
|
||||
Duration::from_millis(1),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(result.name, "Docker Sandbox");
|
||||
assert_eq!(result.status, CheckStatus::Error);
|
||||
assert_eq!(result.summary, "daemon unavailable");
|
||||
assert_eq!(result.details[0].text, "Docker daemon probe timed out");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn check_docker_sandbox_skips_probe_when_provider_is_disabled() {
|
||||
let settings = fabro_config::ServerSettingsBuilder::from_toml(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[server.auth]
|
||||
methods = ["dev-token"]
|
||||
|
||||
[server.sandbox.providers.docker]
|
||||
enabled = false
|
||||
"#,
|
||||
)
|
||||
.expect("settings should parse");
|
||||
let state = TestAppStateBuilder::new()
|
||||
.runtime_settings(settings, RunLayer::default())
|
||||
.build();
|
||||
|
||||
let result = check_docker_sandbox(&state).await;
|
||||
|
||||
assert_eq!(result.name, "Docker Sandbox");
|
||||
assert_eq!(result.status, CheckStatus::Pass);
|
||||
assert_eq!(result.summary, "disabled");
|
||||
assert_eq!(
|
||||
result.details[0].text,
|
||||
"server.sandbox.providers.docker.enabled = false"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn check_cloud_sandbox_ignores_env_backed_daytona_api_key() {
|
||||
let state = TestAppStateBuilder::new()
|
||||
.env_lookup(|name| {
|
||||
(name == EnvVars::DAYTONA_API_KEY).then(|| "dtn_from_env".to_string())
|
||||
})
|
||||
.build();
|
||||
|
||||
let result = check_sandbox(&state).await;
|
||||
let result = check_cloud_sandbox(&state).await;
|
||||
|
||||
assert_eq!(result.name, "Cloud Sandbox");
|
||||
assert_eq!(result.status, CheckStatus::Warning);
|
||||
assert_eq!(result.summary, "recommended, not configured");
|
||||
assert_eq!(
|
||||
|
|
|
|||
|
|
@ -1586,8 +1586,20 @@ async fn diagnostics_reports_under_scoped_daytona_api_key() {
|
|||
])
|
||||
.await;
|
||||
let base_url = server.base_url();
|
||||
let settings = fabro_config::ServerSettingsBuilder::from_toml(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[server.auth]
|
||||
methods = ["dev-token"]
|
||||
|
||||
[server.sandbox.providers.docker]
|
||||
enabled = false
|
||||
"#,
|
||||
)
|
||||
.expect("settings should parse");
|
||||
let state = test_app_state_with_env_lookup(
|
||||
default_test_server_settings(),
|
||||
settings,
|
||||
fabro_config::RunLayer::default(),
|
||||
5,
|
||||
move |name| match name {
|
||||
|
|
@ -1608,24 +1620,24 @@ async fn diagnostics_reports_under_scoped_daytona_api_key() {
|
|||
.unwrap();
|
||||
|
||||
let report = crate::diagnostics::run_all(&state).await;
|
||||
let sandbox = report
|
||||
let cloud_sandbox = report
|
||||
.sections
|
||||
.iter()
|
||||
.flat_map(|section| §ion.checks)
|
||||
.find(|check| check.name == "Sandbox")
|
||||
.expect("sandbox check should be present");
|
||||
.find(|check| check.name == "Cloud Sandbox")
|
||||
.expect("cloud sandbox check should be present");
|
||||
|
||||
assert_eq!(sandbox.status, CheckStatus::Error);
|
||||
assert_eq!(cloud_sandbox.status, CheckStatus::Error);
|
||||
assert_eq!(
|
||||
sandbox.summary,
|
||||
cloud_sandbox.summary,
|
||||
"Daytona API key is missing required scopes"
|
||||
);
|
||||
assert_eq!(
|
||||
sandbox.details[0].text,
|
||||
cloud_sandbox.details[0].text,
|
||||
"missing: write:snapshots, write:sandboxes"
|
||||
);
|
||||
assert_eq!(
|
||||
sandbox.remediation.as_deref(),
|
||||
cloud_sandbox.remediation.as_deref(),
|
||||
Some(
|
||||
"Regenerate the Daytona API key with scopes: write:snapshots, \
|
||||
delete:snapshots, write:sandboxes, delete:sandboxes, then \
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue