mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-08 22:21:45 +00:00
Merge pull request #837 from fabro-sh/codex/reject-local-auto-pr-admission
Reject automatic pull requests for Local runs
This commit is contained in:
commit
bdb7b29877
6 changed files with 113 additions and 3 deletions
|
|
@ -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`).
|
||||
|
|
|
|||
|
|
@ -555,7 +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.
|
||||
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]
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,11 @@ async fn validate_intent_environment(
|
|||
if image_incompatible || target_incompatible {
|
||||
return Err(EnvironmentSelectionError::TargetUnsupported { detail });
|
||||
}
|
||||
// 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) =
|
||||
run_manifest::sandbox_provider_policy_error(&state.server_settings(), effective_provider)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -4251,6 +4251,101 @@ 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;
|
||||
|
||||
// `post_run_manifest` asserts the `201 Created` admission outcome.
|
||||
post_run_manifest(&app, folder_intent(workflow_version_id, target.path())).await;
|
||||
}
|
||||
|
||||
#[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::<RunId>().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());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn post_runs_run_intent_observes_folder_git_metadata_without_a_remote_call() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue