mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-11 03:40:05 +00:00
Some checks failed
Rust / Clippy (push) Waiting to run
Rust / Format (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
## Summary
Makes LLM setup explicitly skippable in both the web installer and
`fabro install`, without making omission accidental. A skipped LLM step
lets install complete with zero LLM credentials; later LLM-dependent
workflows keep using the existing provider-not-configured behavior.
`fabro doctor` is intentionally unchanged.
Plan: `docs/superpowers/plans/2026-05-14-optional-llm-install.md`
## Key changes
**Server + API**
- `PUT /install/llm` now accepts `{"providers":[]}` as "LLM step
completed, skipped" — the empty-list rejection is removed; per-provider
validation for non-empty lists is retained.
- OpenAPI: dropped `minItems: 1` from
`InstallLlmProvidersInput.providers`, updated schema descriptions so
empty = skipped and `llm: null` = incomplete. TypeScript client
regenerated.
- `/install/finish` still requires the LLM step to be present, but
tolerates zero credentials — it writes settings, runtime auth secrets,
and GitHub secrets normally and writes no LLM vault entries.
**Web installer**
- New "Skip LLM setup" secondary action on the LLM step (via a
`secondaryAction` prop on `StepPanel`) that records an empty provider
list and advances to GitHub.
- Review screen shows `LLM providers: Skipped` (step completed, empty)
vs `Not configured` (step never completed), via a new
`describeLlmSummary` helper.
- Continue with no API keys still shows the existing validation error —
skipping is only reachable through the explicit skip action.
**CLI**
- Interactive `fabro install` asks "Configure LLM providers now?"
(default yes) before provider selection; declining returns an empty
selection and continues to GitHub.
- Hidden non-interactive `--skip-llm` flag, mutually exclusive with
`--llm-provider` / `--llm-api-key-stdin` / `--llm-api-key-env` via clap
`conflicts_with_all`. Missing LLM flags are still validation errors
unless `--skip-llm` is present. Non-interactive usage text updated with
a skip example.
## Code review
Ran a 12-reviewer `ce:review` pass (correctness, testing,
maintainability, project-standards, agent-native, learnings, security,
api-contract, reliability, adversarial, cli-readiness,
kieran-typescript). No P0/P1 findings; agent-native parity PASS. Applied
fixes in `40a29c591`:
- Re-entrancy guard on `runStepSubmit` so a fast double-click on "Skip
LLM setup" can't fire two requests.
- `validate()` only suggests `--skip-llm` in the missing-provider error
when no credential flag is set (it conflicts with those flags).
- Added tests: all three `--skip-llm` conflict arms, the review screen's
"Not configured" branch, and the skip-button failure path.
One advisory finding left as report-only: an empty `PUT /install/llm`
overwrites previously-saved credentials if a user navigates Back and
clicks Skip — judged acceptable since the button is explicitly labeled
and clicking it is deliberate.
## Testing
- `cargo nextest run -p fabro-server -p fabro-cli -p fabro-install` —
1521 passed
- `cargo build -p fabro-api`, `cargo fmt --check`, `cargo clippy`
(changed crates) — clean
- `bun test` (install-app) — 14 passed; `bun run typecheck` — clean
- New coverage: server accepts empty providers + session shows `llm`
complete with `providers:[]`; finish with skipped LLM persists no LLM
vault credentials but keeps GitHub secrets; web skip button PUTs
`providers:[]` and navigates to GitHub; review renders Skipped / Not
configured; CLI `--skip-llm` requires `--non-interactive`, conflicts
with all credential flags, `validate()` succeeds with `--skip-llm`,
usage text documents `--skip-llm`.
Not added (out of plan scope): an automated test for the interactive
`InstallInputSource` skip branch — `InteractiveInstallInputSource` is
TTY-coupled and has no existing tests; the non-interactive `--skip-llm`
path is fully covered.
## Post-Deploy Monitoring & Validation
This change is install-time only; there is no continuous runtime impact.
Validate during the next install/release smoke:
- **Web installer:** run a fresh browser install, click "Skip LLM setup"
on the LLM step, confirm it advances to GitHub and the review screen
reads `LLM providers: Skipped`. Finish the install and confirm the
server restarts into normal mode with no LLM credentials in the vault
(`secrets.json` has no credential entries) and
GitHub/server/object-store/sandbox settings written normally.
- **CLI:** run `fabro install --non-interactive --skip-llm
--github-strategy token --github-username <user>` and confirm it
completes; run interactive `fabro install` and confirm declining
"Configure LLM providers now?" continues to GitHub.
- **Healthy signals:** install completes (web `/install/finish` → 202;
CLI exits 0), server boots in normal mode, `fabro doctor` runs and
reports no LLM providers configured (expected, unchanged behavior).
- **Failure signals / rollback trigger:** install fails to finish,
server fails to boot after a skipped install, or `/install/finish`
rejects a completed-but-empty LLM step. Rollback = revert this PR;
install behavior returns to requiring at least one LLM provider.
- **Validation window/owner:** next install smoke / release
verification, owned by whoever runs the release.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
456 lines
15 KiB
Rust
456 lines
15 KiB
Rust
#![expect(
|
|
clippy::disallowed_methods,
|
|
reason = "integration tests stage fixtures and subprocess env with sync test infrastructure"
|
|
)]
|
|
|
|
use fabro_config::{Storage, envfile};
|
|
use fabro_test::{EnvVars, fabro_snapshot, test_context};
|
|
use fabro_vault::{SecretType, Vault};
|
|
|
|
#[test]
|
|
fn help() {
|
|
let context = test_context!();
|
|
let mut cmd = context.install();
|
|
cmd.arg("--help");
|
|
fabro_snapshot!(context.filters(), cmd, @"
|
|
success: true
|
|
exit_code: 0
|
|
----- stdout -----
|
|
Set up the Fabro environment (LLMs, certs, GitHub)
|
|
|
|
Usage: fabro install [OPTIONS] [COMMAND]
|
|
|
|
Commands:
|
|
github Configure GitHub integration (token or GitHub App)
|
|
help Print this message or the help of the given subcommand(s)
|
|
|
|
Options:
|
|
--json Output as JSON [env: FABRO_JSON=]
|
|
--storage-dir <STORAGE_DIR> Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=]
|
|
--debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=]
|
|
--web-url <WEB_URL> Base URL for the web UI (used for OAuth callback URLs and generated settings) [default: http://127.0.0.1:32276]
|
|
--no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true]
|
|
--non-interactive Run install without prompts; use hidden scripted flags for inputs
|
|
--quiet Suppress non-essential output [env: FABRO_QUIET=]
|
|
--verbose Enable verbose output [env: FABRO_VERBOSE=]
|
|
-h, --help Print help
|
|
----- stderr -----
|
|
");
|
|
}
|
|
|
|
#[test]
|
|
fn github_help() {
|
|
let context = test_context!();
|
|
let mut cmd = context.install();
|
|
cmd.args(["github", "--help"]);
|
|
fabro_snapshot!(context.filters(), cmd, @"
|
|
success: true
|
|
exit_code: 0
|
|
----- stdout -----
|
|
Configure GitHub integration (token or GitHub App)
|
|
|
|
Usage: fabro install github [OPTIONS]
|
|
|
|
Options:
|
|
--json Output as JSON [env: FABRO_JSON=]
|
|
--strategy <STRATEGY> GitHub authentication strategy (requires --non-interactive) [possible values: token, app]
|
|
--debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=]
|
|
--owner <OWNER> GitHub App owner: `personal` or `org:<slug>` (app only, requires --non-interactive)
|
|
--no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true]
|
|
--non-interactive Run install without prompts; use hidden scripted flags for inputs
|
|
--quiet Suppress non-essential output [env: FABRO_QUIET=]
|
|
--verbose Enable verbose output [env: FABRO_VERBOSE=]
|
|
-h, --help Print help
|
|
----- stderr -----
|
|
");
|
|
}
|
|
|
|
#[test]
|
|
fn install_json_requires_non_interactive() {
|
|
let context = test_context!();
|
|
let output = context
|
|
.command()
|
|
.args(["--json", "install"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("--json is only supported for install with --non-interactive"));
|
|
}
|
|
|
|
#[test]
|
|
fn install_json_non_interactive_is_not_rejected_as_unsupported() {
|
|
let context = test_context!();
|
|
let output = context
|
|
.command()
|
|
.args(["--json", "install", "--non-interactive"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("Non-interactive install requires additional flags"));
|
|
assert!(!stderr.contains("--json is not supported for this command"));
|
|
}
|
|
|
|
#[test]
|
|
fn install_json_non_interactive_allows_github_app_strategy() {
|
|
let context = test_context!();
|
|
let output = context
|
|
.command()
|
|
.env_remove("MISSING_ANTHROPIC_API_KEY")
|
|
.args([
|
|
"--json",
|
|
"install",
|
|
"--non-interactive",
|
|
"--llm-provider",
|
|
"anthropic",
|
|
"--llm-api-key-env",
|
|
"MISSING_ANTHROPIC_API_KEY",
|
|
"--github-strategy",
|
|
"app",
|
|
"--github-owner",
|
|
"personal",
|
|
])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(!stderr.contains("GitHub App setup is not supported with --non-interactive"));
|
|
assert!(!stderr.contains("requires --github-username"));
|
|
let stdout = String::from_utf8(output.stdout).unwrap();
|
|
let value: serde_json::Value =
|
|
serde_json::from_str(stdout.trim()).expect("install JSON error should parse");
|
|
assert_eq!(value["event"], "install_error");
|
|
assert_eq!(value["status"], "error");
|
|
}
|
|
|
|
#[test]
|
|
fn non_interactive_without_inputs_prints_scripted_usage_and_fails() {
|
|
let context = test_context!();
|
|
let output = context
|
|
.command()
|
|
.args(["install", "--non-interactive"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("Non-interactive install requires additional flags"));
|
|
assert!(stderr.contains("--llm-provider"));
|
|
assert!(stderr.contains("--github-strategy"));
|
|
}
|
|
|
|
#[test]
|
|
fn install_rejects_wildcard_web_url_before_collecting_inputs() {
|
|
let context = test_context!();
|
|
let output = context
|
|
.command()
|
|
.args([
|
|
"install",
|
|
"--web-url",
|
|
"http://0.0.0.0:32276",
|
|
"--non-interactive",
|
|
])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("--web-url must not use a wildcard host"));
|
|
assert!(
|
|
!stderr.contains("Non-interactive install requires additional flags"),
|
|
"wildcard web URL should be rejected before scripted input validation: {stderr}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn hidden_non_interactive_args_require_non_interactive() {
|
|
let context = test_context!();
|
|
let output = context
|
|
.command()
|
|
.args(["install", "--llm-provider", "anthropic"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("requires --non-interactive"));
|
|
}
|
|
|
|
#[test]
|
|
fn skip_llm_requires_non_interactive() {
|
|
let context = test_context!();
|
|
let output = context
|
|
.command()
|
|
.args(["install", "--skip-llm"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("--skip-llm requires --non-interactive"));
|
|
}
|
|
|
|
#[test]
|
|
fn skip_llm_conflicts_with_llm_credential_flags() {
|
|
let context = test_context!();
|
|
|
|
let provider_conflict = context
|
|
.command()
|
|
.args([
|
|
"install",
|
|
"--non-interactive",
|
|
"--skip-llm",
|
|
"--llm-provider",
|
|
"anthropic",
|
|
])
|
|
.output()
|
|
.expect("command should run");
|
|
assert!(!provider_conflict.status.success());
|
|
let stderr = String::from_utf8(provider_conflict.stderr).unwrap();
|
|
assert!(
|
|
stderr.contains("--skip-llm") && stderr.contains("--llm-provider"),
|
|
"expected a conflict error between --skip-llm and --llm-provider: {stderr}"
|
|
);
|
|
|
|
let stdin_conflict = context
|
|
.command()
|
|
.args([
|
|
"install",
|
|
"--non-interactive",
|
|
"--skip-llm",
|
|
"--llm-api-key-stdin",
|
|
])
|
|
.output()
|
|
.expect("command should run");
|
|
assert!(!stdin_conflict.status.success());
|
|
let stderr = String::from_utf8(stdin_conflict.stderr).unwrap();
|
|
assert!(
|
|
stderr.contains("--skip-llm") && stderr.contains("--llm-api-key-stdin"),
|
|
"expected a conflict error between --skip-llm and --llm-api-key-stdin: {stderr}"
|
|
);
|
|
|
|
let env_conflict = context
|
|
.command()
|
|
.args([
|
|
"install",
|
|
"--non-interactive",
|
|
"--skip-llm",
|
|
"--llm-api-key-env",
|
|
"ANTHROPIC_API_KEY",
|
|
])
|
|
.output()
|
|
.expect("command should run");
|
|
assert!(!env_conflict.status.success());
|
|
let stderr = String::from_utf8(env_conflict.stderr).unwrap();
|
|
assert!(
|
|
stderr.contains("--skip-llm") && stderr.contains("--llm-api-key-env"),
|
|
"expected a conflict error between --skip-llm and --llm-api-key-env: {stderr}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn github_requires_prior_install() {
|
|
let context = test_context!();
|
|
std::fs::remove_file(context.home_dir.join(".fabro/settings.toml")).unwrap();
|
|
let output = context
|
|
.command()
|
|
.args(["install", "github"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("No settings.toml found. Run `fabro install` first."));
|
|
}
|
|
|
|
#[test]
|
|
fn github_scripted_flags_require_non_interactive() {
|
|
let context = test_context!();
|
|
context.write_home(".fabro/settings.toml", "_version = 1\n");
|
|
|
|
let output = context
|
|
.command()
|
|
.args(["install", "github", "--strategy", "token"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("--strategy requires --non-interactive"));
|
|
}
|
|
|
|
#[test]
|
|
fn github_non_interactive_requires_strategy() {
|
|
let context = test_context!();
|
|
context.write_home(".fabro/settings.toml", "_version = 1\n");
|
|
|
|
let output = context
|
|
.command()
|
|
.args(["install", "github", "--non-interactive"])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(!output.status.success());
|
|
let stderr = String::from_utf8(output.stderr).unwrap();
|
|
assert!(stderr.contains("install github --non-interactive requires --strategy"));
|
|
}
|
|
|
|
#[test]
|
|
fn github_non_interactive_token_reconfigures_existing_app_install() {
|
|
let mut context = test_context!();
|
|
let storage_dir = context.home_dir.join("install-storage");
|
|
context.manage_storage_dir(&storage_dir);
|
|
context.write_home(
|
|
".fabro/settings.toml",
|
|
format!(
|
|
r#"
|
|
_version = 1
|
|
|
|
[server.storage]
|
|
root = "{}"
|
|
|
|
[server.auth]
|
|
methods = ["dev-token", "github"]
|
|
|
|
[server.auth.github]
|
|
allowed_usernames = ["alice"]
|
|
|
|
[server.integrations.github]
|
|
strategy = "app"
|
|
app_id = "123"
|
|
slug = "alice-fabro"
|
|
client_id = "client-id"
|
|
|
|
[project.metadata]
|
|
mode = "keep-me"
|
|
"#,
|
|
storage_dir.display()
|
|
),
|
|
);
|
|
|
|
let server_env_path = Storage::new(&storage_dir).runtime_directory().env_path();
|
|
envfile::write_env_file(
|
|
&server_env_path,
|
|
&std::collections::HashMap::from([
|
|
("GITHUB_APP_PRIVATE_KEY".to_string(), "private".to_string()),
|
|
(
|
|
"GITHUB_APP_CLIENT_SECRET".to_string(),
|
|
"client-secret".to_string(),
|
|
),
|
|
(
|
|
"GITHUB_APP_WEBHOOK_SECRET".to_string(),
|
|
"webhook-secret".to_string(),
|
|
),
|
|
("KEEP_ME".to_string(), "1".to_string()),
|
|
]),
|
|
)
|
|
.unwrap();
|
|
|
|
let fake_bin = context.temp_dir.join("fake-bin");
|
|
std::fs::create_dir_all(&fake_bin).unwrap();
|
|
let fake_gh = fake_bin.join("gh");
|
|
std::fs::write(
|
|
&fake_gh,
|
|
"#!/bin/sh\nif [ \"$1\" = \"auth\" ] && [ \"$2\" = \"token\" ]; then\n printf 'token-from-gh\\n'\n exit 0\nfi\nexit 1\n",
|
|
)
|
|
.unwrap();
|
|
#[cfg(unix)]
|
|
{
|
|
use std::os::unix::fs::PermissionsExt;
|
|
|
|
std::fs::set_permissions(&fake_gh, std::fs::Permissions::from_mode(0o755)).unwrap();
|
|
}
|
|
|
|
let path = format!(
|
|
"{}:{}",
|
|
fake_bin.display(),
|
|
std::env::var(EnvVars::PATH).unwrap()
|
|
);
|
|
let output = context
|
|
.command()
|
|
.env(EnvVars::PATH, path)
|
|
.args([
|
|
"install",
|
|
"github",
|
|
"--non-interactive",
|
|
"--strategy",
|
|
"token",
|
|
])
|
|
.output()
|
|
.expect("command should run");
|
|
|
|
assert!(output.status.success(), "{output:?}");
|
|
|
|
let settings = std::fs::read_to_string(context.home_dir.join(".fabro/settings.toml")).unwrap();
|
|
let parsed: toml::Value = toml::from_str(&settings).unwrap();
|
|
let github = parsed
|
|
.get("server")
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|server| server.get("integrations"))
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|integrations| integrations.get("github"))
|
|
.and_then(toml::Value::as_table)
|
|
.expect("server.integrations.github should exist");
|
|
assert_eq!(
|
|
github.get("strategy").and_then(toml::Value::as_str),
|
|
Some("token")
|
|
);
|
|
assert!(!github.contains_key("app_id"));
|
|
assert!(!github.contains_key("slug"));
|
|
assert!(!github.contains_key("client_id"));
|
|
|
|
let methods = parsed
|
|
.get("server")
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|server| server.get("auth"))
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|auth| auth.get("methods"))
|
|
.and_then(toml::Value::as_array)
|
|
.expect("server.auth.methods should exist");
|
|
assert_eq!(
|
|
methods
|
|
.iter()
|
|
.map(|value| value.as_str().expect("auth method should be a string"))
|
|
.collect::<Vec<_>>(),
|
|
vec!["dev-token"]
|
|
);
|
|
assert!(
|
|
parsed
|
|
.get("server")
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|server| server.get("auth"))
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|auth| auth.get("github"))
|
|
.is_none(),
|
|
"server.auth.github should be removed"
|
|
);
|
|
assert_eq!(
|
|
parsed
|
|
.get("project")
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|project| project.get("metadata"))
|
|
.and_then(toml::Value::as_table)
|
|
.and_then(|metadata| metadata.get("mode"))
|
|
.and_then(toml::Value::as_str),
|
|
Some("keep-me")
|
|
);
|
|
|
|
let server_env = envfile::read_env_file(&server_env_path).unwrap();
|
|
assert!(!server_env.contains_key("GITHUB_APP_PRIVATE_KEY"));
|
|
assert!(!server_env.contains_key("GITHUB_APP_CLIENT_SECRET"));
|
|
assert!(!server_env.contains_key("GITHUB_APP_WEBHOOK_SECRET"));
|
|
assert_eq!(server_env.get("KEEP_ME").map(String::as_str), Some("1"));
|
|
|
|
let vault = Vault::load(Storage::new(&storage_dir).secrets_path()).unwrap();
|
|
assert_eq!(vault.get("GITHUB_TOKEN"), Some("token-from-gh"));
|
|
assert_eq!(
|
|
vault
|
|
.get_entry("GITHUB_TOKEN")
|
|
.map(|entry| entry.secret_type),
|
|
Some(SecretType::Environment)
|
|
);
|
|
}
|