From f91923892c0fc333b6011fbbc9cf271e9f411fc0 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 13 Apr 2026 20:46:11 -0400 Subject: [PATCH] refactor(github): rename gh_cli strategy to token, GITHUB_CLI_TOKEN to GITHUB_TOKEN The gh_cli strategy was named after its bootstrap mechanism, not what it actually is at runtime: a stored token. This rename makes the abstraction honest and decouples runtime behavior from the gh CLI. - Rename GithubIntegrationStrategy::GhCli to Token (serialized as "token") - Rename vault/env secret from GITHUB_CLI_TOKEN to GITHUB_TOKEN - Accept GH_TOKEN as a fallback in both CLI and server - CLI no longer shells out to `gh auth token` at runtime; reads from vault/env like the server already did - fabro install still bootstraps from `gh auth token` as a one-time op Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/administration/server-configuration.mdx | 6 +-- docs/integrations/daytona.mdx | 4 +- docs/integrations/github.mdx | 16 ++++--- docs/reference/cli.mdx | 2 +- lib/crates/fabro-cli/src/args.rs | 4 +- lib/crates/fabro-cli/src/commands/install.rs | 48 ++++++++++--------- lib/crates/fabro-cli/src/commands/pr/close.rs | 2 +- .../fabro-cli/src/commands/pr/create.rs | 2 +- lib/crates/fabro-cli/src/commands/pr/list.rs | 2 +- lib/crates/fabro-cli/src/commands/pr/merge.rs | 2 +- lib/crates/fabro-cli/src/commands/pr/mod.rs | 12 +++-- lib/crates/fabro-cli/src/commands/pr/view.rs | 2 +- .../fabro-cli/src/commands/run/runner.rs | 16 +++++-- lib/crates/fabro-cli/src/main.rs | 6 +-- lib/crates/fabro-cli/src/shared/github.rs | 31 ++++++++++-- lib/crates/fabro-cli/tests/it/cmd/pr_view.rs | 2 +- .../fabro-config/tests/resolve_server.rs | 4 +- lib/crates/fabro-server/src/demo/mod.rs | 2 +- lib/crates/fabro-server/src/diagnostics.rs | 36 ++++++-------- lib/crates/fabro-server/src/serve.rs | 4 +- lib/crates/fabro-server/src/server.rs | 19 ++++---- lib/crates/fabro-types/src/settings/server.rs | 2 +- .../tests/server_settings_serde.rs | 2 +- 23 files changed, 130 insertions(+), 96 deletions(-) diff --git a/docs/administration/server-configuration.mdx b/docs/administration/server-configuration.mdx index 7f9107e1b..c3509820f 100644 --- a/docs/administration/server-configuration.mdx +++ b/docs/administration/server-configuration.mdx @@ -179,11 +179,11 @@ Customize the git author identity used for checkpoint commits. When not set, def ### `[server.integrations.github]` section -Configure GitHub integration auth. `strategy = "gh_cli"` is the default and uses a stored `GITHUB_CLI_TOKEN` from the vault. `strategy = "app"` enables the GitHub App flow, browser OAuth, and webhooks. +Configure GitHub integration auth. `strategy = "token"` is the default and uses a stored `GITHUB_TOKEN` from the vault (with `GH_TOKEN` as a fallback). `strategy = "app"` enables the GitHub App flow, browser OAuth, and webhooks. ```toml title="settings.toml" [server.integrations.github] -strategy = "gh_cli" +strategy = "token" ``` For GitHub App mode, set `strategy = "app"` and include `app_id`, `client_id`, and `slug`. Webhook delivery is configured under `[server.integrations.github.webhooks]`: @@ -272,7 +272,7 @@ Fabro resolves these from `process env -> server.env`. | Variable | Description | |---|---| -| `GITHUB_CLI_TOKEN` | Token captured from `gh auth token` and stored by `fabro install` when `strategy = "gh_cli"` | +| `GITHUB_TOKEN` | GitHub personal access token, stored by `fabro install` when `strategy = "token"`. Also accepts `GH_TOKEN` as a fallback. | ### GitHub App extras (optional) diff --git a/docs/integrations/daytona.mdx b/docs/integrations/daytona.mdx index 51a4b8bad..0d2d42cb0 100644 --- a/docs/integrations/daytona.mdx +++ b/docs/integrations/daytona.mdx @@ -18,7 +18,7 @@ description: "Run Fabro workflows in sandboxed Daytona cloud environments" ## Prerequisites - A `DAYTONA_API_KEY` environment variable (get one from [app.daytona.io](https://app.daytona.io)) -- GitHub access configured via the default `gh_cli` strategy or a [GitHub App](/integrations/github) (required for private repository cloning and checkpoint pushing) +- GitHub access configured via the default `token` strategy or a [GitHub App](/integrations/github) (required for private repository cloning and checkpoint pushing) ## Configuration @@ -99,7 +99,7 @@ If a snapshot is configured by name but doesn't exist and no `dockerfile` is pro ## Private repositories -Fabro automatically clones the current repository into the sandbox at `/home/daytona/workspace`. Public repositories work without extra configuration. Private repositories require GitHub access. In `gh_cli` mode, Fabro uses the stored CLI token directly. In `app` mode, Fabro uses short-lived Installation Access Tokens scoped to the specific repository. +Fabro automatically clones the current repository into the sandbox at `/home/daytona/workspace`. Public repositories work without extra configuration. Private repositories require GitHub access. In `token` mode, Fabro uses the stored token directly. In `app` mode, Fabro uses short-lived Installation Access Tokens scoped to the specific repository. If the clone fails without GitHub access configured, Fabro suggests running the setup flow: diff --git a/docs/integrations/github.mdx b/docs/integrations/github.mdx index 039d67d8e..b5b4b3577 100644 --- a/docs/integrations/github.mdx +++ b/docs/integrations/github.mdx @@ -5,18 +5,18 @@ description: "Integrate Fabro with GitHub for repository access and OAuth login" Fabro supports two GitHub integration strategies: -- `gh_cli` — the default for local and individual use. Fabro captures `gh auth token` during `fabro install`, stores it as `GITHUB_CLI_TOKEN`, and uses that token directly for repo access, pull requests, and sandbox `GITHUB_TOKEN` injection. +- `token` — the default for local and individual use. Fabro captures `gh auth token` during `fabro install`, stores it as `GITHUB_TOKEN`, and uses that token directly for repo access, pull requests, and sandbox `GITHUB_TOKEN` injection. - `app` — the team-oriented option. Fabro registers a [GitHub App](https://docs.github.com/en/apps/overview), uses installation tokens for repo access, and enables browser OAuth and webhooks. -`gh_cli` changes GitHub integration auth only. It does not provide browser sign-in, so the embedded web UI is disabled when `strategy = "gh_cli"`. +`token` changes GitHub integration auth only. It does not provide browser sign-in, so the embedded web UI is disabled when `strategy = "token"`. ## Strategy matrix -| Capability | `gh_cli` | `app` | +| Capability | `token` | `app` | |---|---|---| | CLI pull requests | Yes | Yes | | Private repo cloning | Yes | Yes | -| Sandbox `GITHUB_TOKEN` | Direct CLI token | Scoped installation token | +| Sandbox `GITHUB_TOKEN` | Direct token | Scoped installation token | | Browser sign-in | No | Yes | | Web UI routes | Disabled | Enabled | | Webhooks | No | Yes | @@ -125,13 +125,15 @@ Fabro stores the GitHub App secrets in `/server.env` under these keys: The private key is stored as base64-encoded PEM. Fabro also accepts raw PEM format (starting with `-----BEGIN`). -## `gh_cli` mode +## `token` mode Choose **GitHub CLI** in `fabro install` to use the default local-user flow. The installer: 1. Runs `gh auth token` -2. Stores the token as `GITHUB_CLI_TOKEN` -3. Writes `strategy = "gh_cli"` under `[server.integrations.github]` +2. Stores the token as `GITHUB_TOKEN` +3. Writes `strategy = "token"` under `[server.integrations.github]` + +After install, Fabro reads `GITHUB_TOKEN` from the vault or environment (with `GH_TOKEN` as a fallback). Token updates after install are the user's responsibility. In this mode, Fabro disables the embedded web UI and browser auth routes. Machine API routes and `/health` continue to work. diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 5fa3a6288..77f56d2a3 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -405,7 +405,7 @@ Run IDs support prefix matching — you can use the first few characters instead ## `fabro pr` -Manage GitHub pull requests created by workflow runs. Requires GitHub access to be configured via the default `gh_cli` strategy or a [GitHub App](/integrations/github). +Manage GitHub pull requests created by workflow runs. Requires GitHub access to be configured via the default `token` strategy or a [GitHub App](/integrations/github). ### `fabro pr create` diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index 7c459e344..f51a56311 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -1278,8 +1278,8 @@ pub(crate) struct DoctorArgs { #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] pub(crate) enum InstallGitHubStrategyArg { - #[value(name = "gh_cli", alias = "gh-cli")] - GhCli, + #[value(name = "token")] + Token, App, } diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index c0faefc24..f12478e96 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -188,9 +188,9 @@ fn github_integration_table(doc: &mut toml::Value) -> Result<&mut toml::Table> { .context("settings.toml [server.integrations.github] is not a table") } -fn write_github_cli_settings(doc: &mut toml::Value) -> Result<()> { +fn write_token_settings(doc: &mut toml::Value) -> Result<()> { let github = github_integration_table(doc)?; - github.insert("strategy".into(), toml::Value::String("gh_cli".to_string())); + github.insert("strategy".into(), toml::Value::String("token".to_string())); github.remove("app_id"); github.remove("slug"); github.remove("client_id"); @@ -303,20 +303,20 @@ Non-interactive usage: fabro install --non-interactive \ --llm-provider anthropic \ --llm-api-key-env ANTHROPIC_API_KEY \ - --github-strategy gh_cli \ + --github-strategy token \ --github-username brynary printf '%s\n' "$ANTHROPIC_API_KEY" | fabro install --non-interactive \ --llm-provider anthropic \ --llm-api-key-stdin \ - --github-strategy gh_cli \ + --github-strategy token \ --github-username brynary Hidden non-interactive flags: --llm-provider --llm-api-key-stdin --llm-api-key-env - --github-strategy + --github-strategy --github-username --overwrite-settings --keep-existing-settings @@ -339,7 +339,7 @@ struct LlmInstallSelection { #[derive(Debug)] enum GitHubInstallSelection { - GhCli, + Token, App { owner: GitHubAppOwner, username: Option, @@ -487,7 +487,7 @@ impl InstallInputSource for InteractiveInstallInputSource { .await??; match strategy { - 0 => Ok(GitHubInstallSelection::GhCli), + 0 => Ok(GitHubInstallSelection::Token), 1 => { let (owner, username) = prompt_github_app_owner(s).await?; Ok(GitHubInstallSelection::App { owner, username }) @@ -557,7 +557,7 @@ impl NonInteractiveInstallInputSource { ); match self.args.github_strategy { - Some(InstallGitHubStrategyArg::GhCli) => {} + Some(InstallGitHubStrategyArg::Token) => {} Some(InstallGitHubStrategyArg::App) => { bail!("GitHub App setup is not supported with --non-interactive") } @@ -626,7 +626,7 @@ impl InstallInputSource for NonInteractiveInstallInputSource { _printer: Printer, ) -> Result { match self.args.github_strategy { - Some(InstallGitHubStrategyArg::GhCli) => Ok(GitHubInstallSelection::GhCli), + Some(InstallGitHubStrategyArg::Token) => Ok(GitHubInstallSelection::Token), Some(InstallGitHubStrategyArg::App) => { bail!("GitHub App setup is not supported with --non-interactive") } @@ -1190,7 +1190,7 @@ pub(crate) async fn run_install( fabro_util::printerr!(printer, ""); match input_source.choose_github_install(&s, printer).await? { - GitHubInstallSelection::GhCli => { + GitHubInstallSelection::Token => { let token = fabro_github::gh_auth_token() .await .map_err(|err| anyhow!("{err}. Run `gh auth login` and rerun `fabro install`."))?; @@ -1201,11 +1201,15 @@ pub(crate) async fn run_install( } else { toml::from_str(&existing).context("failed to parse existing settings.toml")? }; - write_github_cli_settings(&mut doc)?; + write_token_settings(&mut doc)?; std::fs::write(&user_toml_path, toml::to_string_pretty(&doc)?)?; - fabro_util::printerr!(printer, " {} GitHub CLI configured", s.green.apply_to("✔")); + fabro_util::printerr!( + printer, + " {} GitHub token configured", + s.green.apply_to("✔") + ); vault_secrets.push(CreateSecretRequest { - name: "GITHUB_CLI_TOKEN".to_string(), + name: "GITHUB_TOKEN".to_string(), value: token, type_: ApiSecretType::Environment, description: None, @@ -1566,7 +1570,7 @@ name = "custom" } #[test] - fn write_github_cli_settings_uses_server_integrations_github() { + fn write_token_settings_uses_server_integrations_github() { let mut doc: toml::Value = toml::from_str( r#" _version = 1 @@ -1580,7 +1584,7 @@ client_id = "client-id" ) .unwrap(); - write_github_cli_settings(&mut doc).unwrap(); + write_token_settings(&mut doc).unwrap(); let github = doc .get("server") @@ -1593,7 +1597,7 @@ client_id = "client-id" assert_eq!( github.get("strategy").and_then(toml::Value::as_str), - Some("gh_cli") + Some("token") ); assert!(!github.contains_key("app_id")); assert!(!github.contains_key("slug")); @@ -1700,7 +1704,7 @@ client_id = "client-id" ]; let vault_secrets = vec![ CreateSecretRequest { - name: "GITHUB_CLI_TOKEN".to_string(), + name: "GITHUB_TOKEN".to_string(), value: "gh-token".to_string(), type_: ApiSecretType::Environment, description: None, @@ -1793,7 +1797,7 @@ client_id = "client-id" llm_provider: Some(Provider::Anthropic), llm_api_key_stdin: true, llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), - github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_strategy: Some(InstallGitHubStrategyArg::Token), github_username: Some("brynary".to_string()), ..InstallNonInteractiveArgs::default() }); @@ -1809,7 +1813,7 @@ client_id = "client-id" let source = NonInteractiveInstallInputSource { args: InstallNonInteractiveArgs { llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), - github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_strategy: Some(InstallGitHubStrategyArg::Token), github_username: Some("brynary".to_string()), ..InstallNonInteractiveArgs::default() }, @@ -1846,7 +1850,7 @@ client_id = "client-id" args: InstallNonInteractiveArgs { llm_provider: Some(Provider::Anthropic), llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), - github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_strategy: Some(InstallGitHubStrategyArg::Token), ..InstallNonInteractiveArgs::default() }, }; @@ -1864,7 +1868,7 @@ client_id = "client-id" args: InstallNonInteractiveArgs { llm_provider: Some(Provider::Anthropic), llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), - github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_strategy: Some(InstallGitHubStrategyArg::Token), keep_existing_settings: true, ..InstallNonInteractiveArgs::default() }, @@ -1898,7 +1902,7 @@ client_id = "client-id" args: InstallNonInteractiveArgs { llm_provider: Some(Provider::Anthropic), llm_api_key_env: Some("ANTHROPIC_API_KEY".to_string()), - github_strategy: Some(InstallGitHubStrategyArg::GhCli), + github_strategy: Some(InstallGitHubStrategyArg::Token), github_username: Some("brynary".to_string()), ..InstallNonInteractiveArgs::default() }, diff --git a/lib/crates/fabro-cli/src/commands/pr/close.rs b/lib/crates/fabro-cli/src/commands/pr/close.rs index 8dc9523f8..a1e63aacf 100644 --- a/lib/crates/fabro-cli/src/commands/pr/close.rs +++ b/lib/crates/fabro-cli/src/commands/pr/close.rs @@ -12,7 +12,7 @@ pub(super) async fn close_command( ) -> Result<()> { let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, printer).await?; - let creds = super::load_github_credentials_required(printer).await?; + let creds = super::load_github_credentials_required(printer)?; fabro_github::close_pull_request( &creds, diff --git a/lib/crates/fabro-cli/src/commands/pr/create.rs b/lib/crates/fabro-cli/src/commands/pr/create.rs index 52cae606c..1da9a3e50 100644 --- a/lib/crates/fabro-cli/src/commands/pr/create.rs +++ b/lib/crates/fabro-cli/src/commands/pr/create.rs @@ -74,7 +74,7 @@ pub(super) async fn create_command( let (owner, repo) = fabro_github::parse_github_owner_repo(&https_url) .map_err(|err| anyhow::anyhow!("{err}"))?; - let creds = super::load_github_credentials_required(printer).await?; + let creds = super::load_github_credentials_required(printer)?; let branch_found = fabro_github::branch_exists( &creds, diff --git a/lib/crates/fabro-cli/src/commands/pr/list.rs b/lib/crates/fabro-cli/src/commands/pr/list.rs index 53dae4201..051703887 100644 --- a/lib/crates/fabro-cli/src/commands/pr/list.rs +++ b/lib/crates/fabro-cli/src/commands/pr/list.rs @@ -44,7 +44,7 @@ pub(super) async fn list_command( return Ok(()); } - let creds = super::load_github_credentials_required(printer).await?; + let creds = super::load_github_credentials_required(printer)?; let futures: Vec<_> = entries .iter() diff --git a/lib/crates/fabro-cli/src/commands/pr/merge.rs b/lib/crates/fabro-cli/src/commands/pr/merge.rs index 4e8f35f4f..34ac28d3b 100644 --- a/lib/crates/fabro-cli/src/commands/pr/merge.rs +++ b/lib/crates/fabro-cli/src/commands/pr/merge.rs @@ -12,7 +12,7 @@ pub(super) async fn merge_command( ) -> Result<()> { let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, printer).await?; - let creds = super::load_github_credentials_required(printer).await?; + let creds = super::load_github_credentials_required(printer)?; fabro_github::merge_pull_request( &creds, diff --git a/lib/crates/fabro-cli/src/commands/pr/mod.rs b/lib/crates/fabro-cli/src/commands/pr/mod.rs index 1a2a7b355..58f008d18 100644 --- a/lib/crates/fabro-cli/src/commands/pr/mod.rs +++ b/lib/crates/fabro-cli/src/commands/pr/mod.rs @@ -5,6 +5,7 @@ mod merge; mod view; use anyhow::{Context, Result, anyhow}; +use fabro_config::Storage; use fabro_github::GitHubCredentials; use fabro_types::PullRequestRecord; use fabro_types::settings::InterpString; @@ -14,8 +15,10 @@ use crate::args::{GlobalArgs, PrCommand, PrNamespace, ServerTargetArgs}; use crate::command_context::CommandContext; use crate::server_runs::ServerSummaryLookup; use crate::shared::github::build_github_credentials; +use crate::user_config; -const GITHUB_CREDENTIALS_REQUIRED: &str = "GitHub credentials required — run `gh auth login` or configure a GitHub App with `fabro install`"; +const GITHUB_CREDENTIALS_REQUIRED: &str = + "GitHub credentials required — run `fabro install` or set GITHUB_TOKEN"; pub(crate) async fn dispatch( ns: PrNamespace, @@ -31,7 +34,7 @@ pub(crate) async fn dispatch( } } -async fn load_github_credentials_required(printer: Printer) -> Result { +fn load_github_credentials_required(printer: Printer) -> Result { let ctx = CommandContext::base(printer)?; let server_settings = fabro_config::resolve_server_from_file(ctx.machine_settings()).map_err(|errors| { @@ -44,6 +47,9 @@ async fn load_github_credentials_required(printer: Printer) -> Result Result Result<()> { let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, printer).await?; - let creds = super::load_github_credentials_required(printer).await?; + let creds = super::load_github_credentials_required(printer)?; let detail = fabro_github::get_pull_request( &creds, diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index 177514d8c..88a7a1f9d 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -78,8 +78,14 @@ pub(crate) async fn execute( spawn_worker_control_stream(Arc::clone(&interviewer), Arc::clone(&cancel_token))?; let run_control = RunControlState::new(); install_signal_handlers(Arc::clone(&run_control), Arc::clone(&cancel_token))?; - let github_app = maybe_build_github_credentials(&run_record.settings).await?; let vault = load_worker_vault(storage_dir.as_deref())?; + let github_app = { + let vault_guard = match &vault { + Some(arc) => Some(arc.read().await), + None => None, + }; + maybe_build_github_credentials(&run_record.settings, vault_guard.as_deref())? + }; let services = StartServices { run_id, cancel_token: Some(Arc::clone(&cancel_token)), @@ -490,8 +496,9 @@ fn update_worker_title_from_event(event: &RunEvent) { } } -async fn maybe_build_github_credentials( +fn maybe_build_github_credentials( settings: &SettingsLayer, + vault: Option<&fabro_vault::Vault>, ) -> Result> { let resolved_run = fabro_config::resolve_run_from_file(settings).ok(); let resolved_server = fabro_config::resolve_server_from_file(settings).ok(); @@ -513,12 +520,11 @@ async fn maybe_build_github_credentials( .map(InterpString::as_source); if required_github_credentials { - return build_github_credentials(strategy, app_id.as_deref()).await; + return build_github_credentials(strategy, app_id.as_deref(), vault); } if pull_request_enabled { - return Ok(build_github_credentials(strategy, app_id.as_deref()) - .await + return Ok(build_github_credentials(strategy, app_id.as_deref(), vault) .ok() .flatten()); } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 5d12dbb57..6f1a86d13 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -380,7 +380,7 @@ mod tests { } #[test] - fn parse_install_non_interactive_accepts_gh_cli_strategy() { + fn parse_install_non_interactive_accepts_token_strategy() { let cli = Cli::try_parse_from([ "fabro", "install", @@ -390,7 +390,7 @@ mod tests { "--llm-api-key-env", "ANTHROPIC_API_KEY", "--github-strategy", - "gh_cli", + "token", "--github-username", "brynary", ]) @@ -400,7 +400,7 @@ mod tests { assert!(args.non_interactive); assert_eq!( args.scripted.github_strategy, - Some(InstallGitHubStrategyArg::GhCli) + Some(InstallGitHubStrategyArg::Token) ); } _ => panic!("unexpected command variant"), diff --git a/lib/crates/fabro-cli/src/shared/github.rs b/lib/crates/fabro-cli/src/shared/github.rs index 72fa8d2c0..aebb119b3 100644 --- a/lib/crates/fabro-cli/src/shared/github.rs +++ b/lib/crates/fabro-cli/src/shared/github.rs @@ -1,18 +1,39 @@ use anyhow::anyhow; use fabro_github::GitHubCredentials; use fabro_types::settings::server::GithubIntegrationStrategy; +use fabro_vault::Vault; -pub(crate) async fn build_github_credentials( +pub(crate) fn build_github_credentials( strategy: GithubIntegrationStrategy, app_id: Option<&str>, + vault: Option<&Vault>, ) -> anyhow::Result> { match strategy { GithubIntegrationStrategy::App => { GitHubCredentials::from_env(app_id).map_err(|err| anyhow!(err)) } - GithubIntegrationStrategy::GhCli => fabro_github::gh_auth_token() - .await - .map(|token| Some(GitHubCredentials::Token(token))) - .map_err(|err| anyhow!(err)), + GithubIntegrationStrategy::Token => { + let token = lookup_github_token(vault); + match token { + Some(t) => Ok(Some(GitHubCredentials::Token(t))), + None => Err(anyhow!( + "GITHUB_TOKEN not configured — run fabro install or set GITHUB_TOKEN" + )), + } + } } } + +/// Look up GitHub token: GITHUB_TOKEN env -> vault GITHUB_TOKEN -> GH_TOKEN env +/// -> vault GH_TOKEN +fn lookup_github_token(vault: Option<&Vault>) -> Option { + lookup_env_or_vault("GITHUB_TOKEN", vault).or_else(|| lookup_env_or_vault("GH_TOKEN", vault)) +} + +fn lookup_env_or_vault(name: &str, vault: Option<&Vault>) -> Option { + std::env::var(name) + .ok() + .or_else(|| vault.and_then(|v| v.get(name).map(str::to_string))) + .map(|t| t.trim().to_string()) + .filter(|t| !t.is_empty()) +} diff --git a/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs b/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs index eb168643a..1f1338816 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/pr_view.rs @@ -102,6 +102,6 @@ fn pr_view_reads_pull_request_from_store_without_pull_request_json() { exit_code: 1 ----- stdout ----- ----- stderr ----- - error: GitHub credentials required — run `gh auth login` or configure a GitHub App with `fabro install` + error: GitHub credentials required — run `fabro install` or set GITHUB_TOKEN "); } diff --git a/lib/crates/fabro-config/tests/resolve_server.rs b/lib/crates/fabro-config/tests/resolve_server.rs index a53817bbe..f4563387e 100644 --- a/lib/crates/fabro-config/tests/resolve_server.rs +++ b/lib/crates/fabro-config/tests/resolve_server.rs @@ -159,7 +159,7 @@ strategy = "app" } #[test] -fn defaults_github_integration_strategy_to_gh_cli() { +fn defaults_github_integration_strategy_to_token() { let file = parse( r" _version = 1 @@ -174,6 +174,6 @@ enabled = true assert_eq!( settings.integrations.github.strategy, - GithubIntegrationStrategy::GhCli + GithubIntegrationStrategy::Token ); } diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 060a48f6f..837b79ded 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -1677,7 +1677,7 @@ mod settings { "integrations": { "github": { "enabled": false, - "strategy": "gh_cli", + "strategy": "token", "app_id": "12345", "client_id": "Iv1.abc123", "slug": "fabro-dev", diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index 4d0a8db8d..88da93991 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -283,25 +283,22 @@ async fn probe_llm_provider(client: &LlmClient, provider: Provider) -> Result<() async fn check_github_app(state: &AppState) -> CheckResult { let settings = state.server_settings(); - if settings.integrations.github.strategy == GithubIntegrationStrategy::GhCli { + if settings.integrations.github.strategy == GithubIntegrationStrategy::Token { let token = match state.github_credentials(&settings.integrations.github) { Ok(Some(fabro_github::GitHubCredentials::Token(token))) => token, - Ok(Some(_)) => unreachable!("gh_cli strategy should not return app credentials"), + Ok(Some(_)) => unreachable!("token strategy should not return app credentials"), Ok(None) => { return CheckResult { - name: "GitHub CLI".to_string(), + name: "GitHub Token".to_string(), status: CheckStatus::Warning, summary: "not configured".to_string(), details: Vec::new(), - remediation: Some( - "Run fabro install on the server host to store GITHUB_CLI_TOKEN" - .to_string(), - ), + remediation: Some("Run fabro install or set GITHUB_TOKEN".to_string()), }; } Err(err) => { return CheckResult { - name: "GitHub CLI".to_string(), + name: "GitHub Token".to_string(), status: CheckStatus::Error, summary: "missing token".to_string(), details: vec![CheckDetail::new(err.clone())], @@ -310,7 +307,7 @@ async fn check_github_app(state: &AppState) -> CheckResult { } }; - let http = match http_client_or_check("GitHub CLI", CheckStatus::Error) { + let http = match http_client_or_check("GitHub Token", CheckStatus::Error) { Ok(http) => http, Err(result) => return result, }; @@ -326,7 +323,7 @@ async fn check_github_app(state: &AppState) -> CheckResult { return match probe { Ok(Ok(response)) if response.status().is_success() => CheckResult { - name: "GitHub CLI".to_string(), + name: "GitHub Token".to_string(), status: CheckStatus::Pass, summary: "configured".to_string(), details: Vec::new(), @@ -334,42 +331,39 @@ async fn check_github_app(state: &AppState) -> CheckResult { }, Ok(Ok(response)) if response.status() == fabro_http::StatusCode::UNAUTHORIZED => { CheckResult { - name: "GitHub CLI".to_string(), + name: "GitHub Token".to_string(), status: CheckStatus::Error, summary: "token invalid".to_string(), details: vec![CheckDetail::new(format!( "GitHub returned {}", response.status() ))], - remediation: Some( - "Run fabro install on the server host to refresh GITHUB_CLI_TOKEN" - .to_string(), - ), + remediation: Some("Run fabro install or update GITHUB_TOKEN".to_string()), } } Ok(Ok(response)) => CheckResult { - name: "GitHub CLI".to_string(), + name: "GitHub Token".to_string(), status: CheckStatus::Error, summary: "connectivity error".to_string(), details: vec![CheckDetail::new(format!( "GitHub returned {}", response.status() ))], - remediation: Some("Check GitHub connectivity and the stored CLI token".to_string()), + remediation: Some("Check GitHub connectivity and GITHUB_TOKEN".to_string()), }, Ok(Err(err)) => CheckResult { - name: "GitHub CLI".to_string(), + name: "GitHub Token".to_string(), status: CheckStatus::Error, summary: "connectivity error".to_string(), details: vec![CheckDetail::new(err.to_string())], - remediation: Some("Check GitHub connectivity and the stored CLI token".to_string()), + remediation: Some("Check GitHub connectivity and GITHUB_TOKEN".to_string()), }, Err(_) => CheckResult { - name: "GitHub CLI".to_string(), + name: "GitHub Token".to_string(), status: CheckStatus::Error, summary: "timeout".to_string(), details: vec![CheckDetail::new("GitHub probe timed out".to_string())], - remediation: Some("Check GitHub connectivity and the stored CLI token".to_string()), + remediation: Some("Check GitHub connectivity and GITHUB_TOKEN".to_string()), }, }; } diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 5b4c6c6e9..421ffa711 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -347,7 +347,7 @@ where // Optionally start webhook listener let webhook_manager = match resolved_server_settings.integrations.github.strategy { - GithubIntegrationStrategy::GhCli => None, + GithubIntegrationStrategy::Token => None, GithubIntegrationStrategy::App => { let webhook_app_id = resolved_server_settings .integrations @@ -773,7 +773,7 @@ _version = 1 enabled = true [server.integrations.github] -strategy = "gh_cli" +strategy = "token" "#, ); diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index aaa0d0725..60f2df098 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -640,9 +640,10 @@ impl AppState { }, ))) } - GithubIntegrationStrategy::GhCli => { + GithubIntegrationStrategy::Token => { let token = self - .vault_or_env("GITHUB_CLI_TOKEN") + .vault_or_env("GITHUB_TOKEN") + .or_else(|| self.vault_or_env("GH_TOKEN")) .as_deref() .map(str::trim) .filter(|token| !token.is_empty()) @@ -650,7 +651,7 @@ impl AppState { match token { Some(token) => Ok(Some(fabro_github::GitHubCredentials::Token(token))), None => Err( - "gh_cli strategy requires a stored token — run fabro install on the server host or manually provision GITHUB_CLI_TOKEN" + "GITHUB_TOKEN not configured — run fabro install or set GITHUB_TOKEN" .to_string(), ), } @@ -1832,13 +1833,13 @@ async fn get_github_repo( Err(err) => return ApiError::new(StatusCode::BAD_GATEWAY, err).into_response(), } } - GithubIntegrationStrategy::GhCli => match state.github_credentials(github_settings) { + GithubIntegrationStrategy::Token => match state.github_credentials(github_settings) { Ok(Some(fabro_github::GitHubCredentials::Token(token))) => token, - Ok(Some(_)) => unreachable!("gh_cli strategy should not return app credentials"), + Ok(Some(_)) => unreachable!("token strategy should not return app credentials"), Ok(None) => { return ApiError::new( StatusCode::SERVICE_UNAVAILABLE, - "GITHUB_CLI_TOKEN is not configured", + "GITHUB_TOKEN is not configured", ) .into_response(); } @@ -1868,7 +1869,7 @@ async fn get_github_repo( { Ok(response) if response.status().is_success() => response, Ok(response) - if github_settings.strategy == GithubIntegrationStrategy::GhCli + if github_settings.strategy == GithubIntegrationStrategy::Token && matches!( response.status(), fabro_http::StatusCode::FORBIDDEN | fabro_http::StatusCode::NOT_FOUND @@ -1889,12 +1890,12 @@ async fn get_github_repo( .into_response(); } Ok(response) - if github_settings.strategy == GithubIntegrationStrategy::GhCli + if github_settings.strategy == GithubIntegrationStrategy::Token && response.status() == fabro_http::StatusCode::UNAUTHORIZED => { return ApiError::new( StatusCode::SERVICE_UNAVAILABLE, - "Stored GitHub CLI token is invalid — run fabro install or update GITHUB_CLI_TOKEN", + "Stored GitHub token is invalid — run fabro install or update GITHUB_TOKEN", ) .into_response(); } diff --git a/lib/crates/fabro-types/src/settings/server.rs b/lib/crates/fabro-types/src/settings/server.rs index cedf04208..29ec55931 100644 --- a/lib/crates/fabro-types/src/settings/server.rs +++ b/lib/crates/fabro-types/src/settings/server.rs @@ -497,7 +497,7 @@ pub struct IntegrationWebhooksLayer { #[serde(rename_all = "snake_case")] pub enum GithubIntegrationStrategy { #[default] - GhCli, + Token, App, } diff --git a/lib/crates/fabro-types/tests/server_settings_serde.rs b/lib/crates/fabro-types/tests/server_settings_serde.rs index dc40eb272..fafc9fc4d 100644 --- a/lib/crates/fabro-types/tests/server_settings_serde.rs +++ b/lib/crates/fabro-types/tests/server_settings_serde.rs @@ -8,7 +8,7 @@ fn settings_layer_round_trips_github_integration_strategy() { "server": { "integrations": { "github": { - "strategy": "gh_cli", + "strategy": "token", "app_id": "{{ env.GITHUB_APP_ID }}" } }