From 18cb752b61ce8cfb901e9e0a0467d47c8ec94b90 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Wed, 2 Sep 2026 17:15:52 -0400 Subject: [PATCH 1/3] Reject automatic pull requests for Local runs --- docs/public/execution/run-configuration.mdx | 2 + lib/apps/fabro-server/src/run_intent.rs | 4 + .../fabro-server/src/server/handler/runs.rs | 14 +++ lib/apps/fabro-server/src/server/tests.rs | 117 ++++++++++++++++++ 4 files changed, 137 insertions(+) diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index 7db919dba..e3eaa93cc 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -557,6 +557,8 @@ The `sandbox` transport runs the MCP server inside the workflow's sandbox. This Automatically open a GitHub pull request when the workflow run completes successfully. Requires a [GitHub App](/integrations/github) to be configured. +Automatic pull requests require a clone-based Docker or Daytona environment. For in-place Local runs, leave pull requests disabled; run creation rejects a Local environment when `enabled = true`. + ```toml title="run.toml" [run.pull_request] enabled = true diff --git a/lib/apps/fabro-server/src/run_intent.rs b/lib/apps/fabro-server/src/run_intent.rs index 489306828..68aa7c7a8 100644 --- a/lib/apps/fabro-server/src/run_intent.rs +++ b/lib/apps/fabro-server/src/run_intent.rs @@ -133,6 +133,10 @@ pub(crate) enum EnvironmentSelectionError { NotFound { id: EnvironmentId }, #[error("{detail}")] TargetUnsupported { detail: &'static str }, + #[error( + "automatic pull requests require a clone-based Docker or Daytona environment; disable run.pull_request.enabled for Local execution" + )] + AutomaticPullRequestUnsupported, #[error("{detail}")] ProviderDisabled { provider: SandboxProviderKind, diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 236e0ec63..decd392fd 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -1041,6 +1041,11 @@ fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response { error.to_string(), "target_environment_unsupported", ), + EnvironmentSelectionError::AutomaticPullRequestUnsupported => intent_error( + StatusCode::UNPROCESSABLE_ENTITY, + error.to_string(), + "pull_request_environment_unsupported", + ), EnvironmentSelectionError::ProviderDisabled { .. } | EnvironmentSelectionError::MissingCredential { .. } => intent_error( StatusCode::SERVICE_UNAVAILABLE, @@ -1118,6 +1123,15 @@ async fn validate_intent_environment( if image_incompatible || target_incompatible { return Err(EnvironmentSelectionError::TargetUnsupported { detail }); } + if configured_provider == SandboxProviderKind::Local + && settings + .run + .pull_request + .as_ref() + .is_some_and(|pull_request| pull_request.enabled) + { + return Err(EnvironmentSelectionError::AutomaticPullRequestUnsupported); + } if let Some(detail) = run_manifest::sandbox_provider_policy_error(&state.server_settings(), effective_provider) { diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 7f23d1989..887f0c7da 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -4251,6 +4251,123 @@ async fn post_runs_run_intent_canonicalizes_and_persists_a_local_folder_target() assert!(projection.spec.definition_blob.is_some()); } +#[tokio::test] +async fn post_runs_run_intent_rejects_automatic_pull_requests_for_local_environment() { + let target = tempfile::tempdir().unwrap(); + let state = local_test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let workflow_version_id = store_workflow_version( + &state, + MINIMAL_DOT, + Some("_version = 1\n[run.pull_request]\nenabled = true\n"), + ) + .await; + + let response = + post_run_intent_response(&app, folder_intent(workflow_version_id, target.path())).await; + let body = response_json!(response, StatusCode::UNPROCESSABLE_ENTITY).await; + + assert_eq!( + body["errors"][0]["code"], + "pull_request_environment_unsupported" + ); + assert_eq!( + body["errors"][0]["detail"], + "automatic pull requests require a clone-based Docker or Daytona environment; disable run.pull_request.enabled for Local execution" + ); + assert!(state.runs.lock().expect("runs lock poisoned").is_empty()); + assert!( + state + .stores + .run_summaries + .list_identities() + .await + .unwrap() + .is_empty() + ); +} + +#[tokio::test] +async fn post_runs_run_intent_accepts_disabled_pull_requests_for_local_environment() { + let target = tempfile::tempdir().unwrap(); + let state = local_test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let workflow_version_id = store_workflow_version( + &state, + MINIMAL_DOT, + Some("_version = 1\n[run.pull_request]\nenabled = false\n"), + ) + .await; + + let body = post_run_manifest(&app, folder_intent(workflow_version_id, target.path())).await; + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); + let projection = state + .stores + .runs + .open_run_reader(&run_id) + .await + .unwrap() + .state() + .await + .unwrap(); + + assert_eq!( + projection.spec.settings.run.environment.provider, + EnvironmentProvider::Local + ); + assert!(projection.spec.settings.run.pull_request.is_none()); +} + +#[tokio::test] +async fn post_runs_run_intent_accepts_automatic_pull_requests_for_configured_docker_dry_run() { + let state = test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let workflow_version_id = store_workflow_version( + &state, + MINIMAL_DOT, + Some("_version = 1\n[run.pull_request]\nenabled = true\n"), + ) + .await; + + let body = post_run_manifest( + &app, + json!({ + "workflow_version_id": workflow_version_id, + "target": { + "kind": "git", + "repo": "fabro-sh/fabro", + "branch": "main" + }, + "args": { "dry_run": true } + }), + ) + .await; + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); + let projection = state + .stores + .runs + .open_run_reader(&run_id) + .await + .unwrap() + .state() + .await + .unwrap(); + + assert_eq!( + projection.spec.settings.run.environment.provider, + EnvironmentProvider::Docker + ); + assert_eq!(projection.spec.settings.run.execution.mode, RunMode::DryRun); + assert!( + projection + .spec + .settings + .run + .pull_request + .is_some_and(|pull_request| pull_request.enabled) + ); +} + #[tokio::test] async fn post_runs_run_intent_observes_folder_git_metadata_without_a_remote_call() { let dir = tempfile::tempdir().unwrap(); From cd20e453ec3d132f3547919efc26c5047712e1d5 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Wed, 2 Sep 2026 17:24:22 -0400 Subject: [PATCH 2/3] Use a compatible environment in pull request settings test --- lib/apps/fabro-cli/tests/it/cmd/create.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/apps/fabro-cli/tests/it/cmd/create.rs b/lib/apps/fabro-cli/tests/it/cmd/create.rs index 655ed1570..d8125f278 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/create.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/create.rs @@ -1560,7 +1560,7 @@ draft = false let workflow_owned = context .create_cmd() .current_dir(project.path()) - .args(["--environment", "local", workflow.to_str().unwrap()]) + .args(["--environment", "default", workflow.to_str().unwrap()]) .assert() .success(); let workflow_owned_id = created_run_id(workflow_owned.get_output()); From c5010c0b330020037f14d23b996a5a1629e2bbb2 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Thu, 3 Sep 2026 11:43:29 -0400 Subject: [PATCH 3/3] Simplify the Local pull-request admission check Use the shared clone-based provider predicate and rely on the settings resolver dropping disabled pull-request settings instead of re-checking the enabled flag. List the new intent-lane error code in the OpenAPI description, trim the acceptance tests to what they actually prove, and fold the docs note into the existing requirements sentence. Co-Authored-By: Claude Fable 5.1 --- docs/public/api-reference/fabro-api.yaml | 3 +- docs/public/execution/run-configuration.mdx | 4 +-- .../fabro-server/src/server/handler/runs.rs | 10 ++----- lib/apps/fabro-server/src/server/tests.rs | 28 ++----------------- 4 files changed, 9 insertions(+), 36 deletions(-) diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 5c294ed86..aa5938af4 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -1181,7 +1181,8 @@ paths: Failures return the standard error body. The intent lane responds `404` (`workflow_version_not_found`, `environment_not_found`), `422` (`run_intent_invalid`, `target_invalid`, - `target_environment_unsupported`, `workflow_version_unusable`, + `target_environment_unsupported`, + `pull_request_environment_unsupported`, `workflow_version_unusable`, `run_compile_invalid`), `503` (`integration_unavailable`), or `500` (`workflow_version_store_error`, `credential_store_error`, `variable_store_error`, `run_persistence_failed`). diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index e3eaa93cc..699c3b3c4 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -555,9 +555,7 @@ The `sandbox` transport runs the MCP server inside the workflow's sandbox. This ### `[run.pull_request]` -Automatically open a GitHub pull request when the workflow run completes successfully. Requires a [GitHub App](/integrations/github) to be configured. - -Automatic pull requests require a clone-based Docker or Daytona environment. For in-place Local runs, leave pull requests disabled; run creation rejects a Local environment when `enabled = true`. +Automatically open a GitHub pull request when the workflow run completes successfully. Requires a [GitHub App](/integrations/github) to be configured and a clone-based Docker or Daytona environment; run creation rejects `enabled = true` on a Local environment. ```toml title="run.toml" [run.pull_request] diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index decd392fd..2c9076e89 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -1123,13 +1123,9 @@ async fn validate_intent_environment( if image_incompatible || target_incompatible { return Err(EnvironmentSelectionError::TargetUnsupported { detail }); } - if configured_provider == SandboxProviderKind::Local - && settings - .run - .pull_request - .as_ref() - .is_some_and(|pull_request| pull_request.enabled) - { + // Settings resolution drops `run.pull_request` unless it is enabled, so + // `Some` means automatic pull requests were requested. + if !configured_provider.is_clone_based() && settings.run.pull_request.is_some() { return Err(EnvironmentSelectionError::AutomaticPullRequestUnsupported); } if let Some(detail) = diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 887f0c7da..c70c92486 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -4299,23 +4299,8 @@ async fn post_runs_run_intent_accepts_disabled_pull_requests_for_local_environme ) .await; - let body = post_run_manifest(&app, folder_intent(workflow_version_id, target.path())).await; - let run_id = body["id"].as_str().unwrap().parse::().unwrap(); - let projection = state - .stores - .runs - .open_run_reader(&run_id) - .await - .unwrap() - .state() - .await - .unwrap(); - - assert_eq!( - projection.spec.settings.run.environment.provider, - EnvironmentProvider::Local - ); - assert!(projection.spec.settings.run.pull_request.is_none()); + // `post_run_manifest` asserts the `201 Created` admission outcome. + post_run_manifest(&app, folder_intent(workflow_version_id, target.path())).await; } #[tokio::test] @@ -4358,14 +4343,7 @@ async fn post_runs_run_intent_accepts_automatic_pull_requests_for_configured_doc EnvironmentProvider::Docker ); assert_eq!(projection.spec.settings.run.execution.mode, RunMode::DryRun); - assert!( - projection - .spec - .settings - .run - .pull_request - .is_some_and(|pull_request| pull_request.enabled) - ); + assert!(projection.spec.settings.run.pull_request.is_some()); } #[tokio::test]