mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
Fix: Add OAuth callback URLs to CLI-generated GitHub App manifest (#105)
This PR fixes GitHub issue #97 where `fabro install` creates a GitHub App that passes `fabro doctor` but fails during OAuth login in `fabro-web` because the CLI-generated manifest is missing `callback_urls` and `setup_url` fields. Without these fields, GitHub rejects the OAuth flow with a "must be configured with a callback URL" error, even though the web setup flow (`setup.tsx`) already includes them correctly. The fix extracts manifest construction into a standalone `build_github_app_manifest` helper and adds the missing `callback_urls` and `setup_url` fields, mirroring what the web setup flow already provides. A new `--web-url` flag (defaulting to `http://localhost:5173`) is added to the `Install` command so users can specify their web UI base URL, which is then threaded through `run_install` and `setup_github_app` to construct the correct OAuth callback endpoints. A unit test is included to verify that the generated manifest contains the expected `callback_urls` and `setup_url` values for a given `web_url`. ### Fabro Details <details> <summary>Ran 9 stages in 17m 47s for $1.56</summary> | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 0s | – | 0 | | preflight_compile | 1m 11s | – | 0 | | preflight_lint | 14s | – | 0 | | implement | 6m 32s | $0.64 | 1 | | simplify_opus | 0s | – | 3 | | simplify_gpt | 5m 42s | $0.92 | 0 | | verify | 1m 21s | – | 0 | | fmt | 1s | – | 0 | | **Total** | **17m 47s** | **$1.56** | **4** | </details> <details> <summary>Ran <code>ImplementAndSimplify.fabro</code> (12 nodes and 15 edges)</summary> ```dot digraph ImplementAndSimplify { graph [ goal="Implement and simplify", model_stylesheet=" * { backend: api; model: claude-opus-4-6;} " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo clippy -q --workspace -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD."] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-54)", prompt="@prompts/simplify.md", model="gpt-54"] verify [label="Verify", shape=parallelogram, script="cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all clippy lint warnings and test failures.", max_visits=3] fmt [label="Format", shape=parallelogram, script="cargo fmt --all 2>&1", max_retries=0] start -> toolchain toolchain -> preflight_compile [condition="outcome=success"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=success"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=success"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> fmt [condition="outcome=success"] verify -> fixup fixup -> verify fmt -> exit } ``` </details> ⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro <noreply@fabro.sh> Co-authored-by: Bryan Helmkamp <bryan@brynary.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
brynary-fabro[bot]
GitHub
parent
523044226f
commit
fd5cde2719
2 changed files with 54 additions and 23 deletions
|
|
@ -258,9 +258,33 @@ struct CallbackParams {
|
|||
code: String,
|
||||
}
|
||||
|
||||
fn build_github_app_manifest(app_name: &str, port: u16, web_url: &str) -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"name": app_name,
|
||||
"url": "https://github.com/apps/arc",
|
||||
"redirect_url": format!("http://127.0.0.1:{port}/callback"),
|
||||
"callback_urls": [format!("{web_url}/auth/callback")],
|
||||
"setup_url": format!("{web_url}/setup/callback"),
|
||||
"public": false,
|
||||
"default_permissions": {
|
||||
"contents": "write",
|
||||
"metadata": "read",
|
||||
"pull_requests": "write",
|
||||
"checks": "write",
|
||||
"issues": "write",
|
||||
"emails": "read"
|
||||
},
|
||||
"default_events": []
|
||||
})
|
||||
}
|
||||
|
||||
/// Run the GitHub App manifest registration flow via a temporary local server.
|
||||
/// Returns env var pairs (key, value) for secrets to merge into `.env`.
|
||||
async fn setup_github_app(arc_dir: &Path, s: &Styles) -> Result<Vec<(String, String)>> {
|
||||
async fn setup_github_app(
|
||||
arc_dir: &Path,
|
||||
s: &Styles,
|
||||
web_url: &str,
|
||||
) -> Result<Vec<(String, String)>> {
|
||||
// Random suffix so app names don't collide
|
||||
let mut rng = rand::thread_rng();
|
||||
let suffix: String = (0..6)
|
||||
|
|
@ -275,21 +299,7 @@ async fn setup_github_app(arc_dir: &Path, s: &Styles) -> Result<Vec<(String, Str
|
|||
let addr: SocketAddr = listener.local_addr()?;
|
||||
let port = addr.port();
|
||||
|
||||
let manifest = serde_json::json!({
|
||||
"name": app_name,
|
||||
"url": "https://github.com/apps/arc",
|
||||
"redirect_url": format!("http://127.0.0.1:{port}/callback"),
|
||||
"public": false,
|
||||
"default_permissions": {
|
||||
"contents": "write",
|
||||
"metadata": "read",
|
||||
"pull_requests": "write",
|
||||
"checks": "write",
|
||||
"issues": "write",
|
||||
"emails": "read"
|
||||
},
|
||||
"default_events": []
|
||||
});
|
||||
let manifest = build_github_app_manifest(&app_name, port, web_url);
|
||||
let manifest_json = serde_json::to_string(&manifest)?;
|
||||
let escaped_manifest = manifest_json
|
||||
.replace('&', "&")
|
||||
|
|
@ -472,7 +482,7 @@ async fn setup_github_app(arc_dir: &Path, s: &Styles) -> Result<Vec<(String, Str
|
|||
Ok(env_pairs)
|
||||
}
|
||||
|
||||
pub async fn run_install() -> Result<()> {
|
||||
pub async fn run_install(web_url: &str) -> Result<()> {
|
||||
let s = Styles::detect_stderr();
|
||||
let emoji = console::Emoji("⚒️ ", "");
|
||||
|
||||
|
|
@ -640,7 +650,7 @@ pub async fn run_install() -> Result<()> {
|
|||
.await??;
|
||||
|
||||
if setup_github {
|
||||
let github_env_pairs = setup_github_app(&arc_dir, &s).await?;
|
||||
let github_env_pairs = setup_github_app(&arc_dir, &s, web_url).await?;
|
||||
let slug = {
|
||||
let cli_toml_path = arc_dir.join("cli.toml");
|
||||
let toml_content = std::fs::read_to_string(&cli_toml_path).unwrap_or_default();
|
||||
|
|
@ -951,4 +961,21 @@ mod tests {
|
|||
assert_eq!(tls.key, PathBuf::from("~/.fabro/certs/server.key"));
|
||||
assert_eq!(tls.ca, PathBuf::from("~/.fabro/certs/ca.crt"));
|
||||
}
|
||||
|
||||
// -- GitHub App manifest --
|
||||
|
||||
#[test]
|
||||
fn manifest_includes_callback_urls_and_setup_url() {
|
||||
let web_url = "https://app.example.com";
|
||||
let manifest = build_github_app_manifest("Arc-test", 12345, web_url);
|
||||
|
||||
assert_eq!(
|
||||
manifest["callback_urls"],
|
||||
serde_json::json!(["https://app.example.com/auth/callback"]),
|
||||
);
|
||||
assert_eq!(
|
||||
manifest["setup_url"],
|
||||
serde_json::json!("https://app.example.com/setup/callback"),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -119,7 +119,11 @@ enum Command {
|
|||
#[command(hide = true)]
|
||||
Init,
|
||||
/// Set up the Fabro environment (LLMs, certs, GitHub)
|
||||
Install,
|
||||
Install {
|
||||
/// Base URL for the web UI (used for OAuth callback URLs)
|
||||
#[arg(long, default_value = "http://localhost:5173")]
|
||||
web_url: String,
|
||||
},
|
||||
/// List workflow runs
|
||||
#[command(hide = true)]
|
||||
Ps(commands::runs::RunsListArgs),
|
||||
|
|
@ -490,7 +494,7 @@ async fn main_inner() -> (String, Result<()>) {
|
|||
RepoCommand::Deinit => "repo deinit",
|
||||
},
|
||||
Command::Init => "init",
|
||||
Command::Install => "install",
|
||||
Command::Install { .. } => "install",
|
||||
Command::Ps(_) => "ps",
|
||||
Command::Rm(_) => "rm",
|
||||
Command::Pr { command } => match command {
|
||||
|
|
@ -572,7 +576,7 @@ async fn main_inner() -> (String, Result<()>) {
|
|||
| Command::Exec(_)
|
||||
| Command::Repo { .. }
|
||||
| Command::Init
|
||||
| Command::Install
|
||||
| Command::Install { .. }
|
||||
) {
|
||||
upgrade::spawn_upgrade_check(cli.no_upgrade_check, upgrade_check_enabled)
|
||||
} else {
|
||||
|
|
@ -845,8 +849,8 @@ async fn main_inner() -> (String, Result<()>) {
|
|||
);
|
||||
init::run_init().await?;
|
||||
}
|
||||
Command::Install => {
|
||||
install::run_install().await?;
|
||||
Command::Install { web_url } => {
|
||||
install::run_install(&web_url).await?;
|
||||
}
|
||||
Command::Ps(args) => {
|
||||
let styles = fabro_util::terminal::Styles::detect_stdout();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue