mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
feat(runs): allow retrying succeeded runs
Broaden manual retry eligibility to all unarchived terminal runs while preserving active and archived precondition failures.
This commit is contained in:
parent
2d78f96107
commit
a992a7d76b
6 changed files with 151 additions and 34 deletions
|
|
@ -427,13 +427,14 @@ describe("run lifecycle actions", () => {
|
|||
expect(canApprove(makeRun({ kind: "runnable" }))).toBe(false);
|
||||
});
|
||||
|
||||
test("canRetry allows failed (including cancelled) and dead runs except archived runs", () => {
|
||||
test("canRetry allows terminal runs except archived runs", () => {
|
||||
expect(canRetry(makeRun({ kind: "failed", reason: "workflow_error" }))).toBe(true);
|
||||
expect(canRetry(makeRun({ kind: "dead" }))).toBe(true);
|
||||
expect(canRetry(makeRun({ kind: "failed", reason: "cancelled" }))).toBe(true);
|
||||
expect(canRetry(makeRun({ kind: "succeeded", reason: "completed" }))).toBe(false);
|
||||
expect(canRetry(makeRun({ kind: "succeeded", reason: "completed" }))).toBe(true);
|
||||
expect(canRetry(makeRun({ kind: "running" }))).toBe(false);
|
||||
expect(canRetry(makeRun({ kind: "failed", reason: "workflow_error" }, true))).toBe(false);
|
||||
expect(canRetry(makeRun({ kind: "succeeded", reason: "completed" }, true))).toBe(false);
|
||||
});
|
||||
|
||||
test("isTerminalCancelledRun distinguishes immediate cancel success from in-flight cancellation", () => {
|
||||
|
|
|
|||
|
|
@ -152,7 +152,7 @@ export function canUnarchive(status: string | null | undefined): boolean {
|
|||
export function canRetry(run: Pick<Run, "lifecycle"> | null | undefined): boolean {
|
||||
if (!run || run.lifecycle.archived) return false;
|
||||
const status = run.lifecycle.status;
|
||||
return status.kind === "failed" || status.kind === "dead";
|
||||
return status.kind === "succeeded" || status.kind === "failed" || status.kind === "dead";
|
||||
}
|
||||
|
||||
export function canDelete(status: string | null | undefined): boolean {
|
||||
|
|
|
|||
|
|
@ -2076,10 +2076,10 @@ paths:
|
|||
tags: [Runs]
|
||||
summary: Retry Run
|
||||
description: >
|
||||
Creates a fresh run from the failed or dead source run's captured
|
||||
Creates a fresh run from the terminal source run's captured
|
||||
durable definition, records `retried_from` on the new run, and schedules
|
||||
it for execution. The source run is left unchanged. Active, succeeded,
|
||||
and archived runs are not retryable.
|
||||
it for execution. The source run is left unchanged. Active and archived
|
||||
runs are not retryable.
|
||||
parameters:
|
||||
- $ref: "#/components/parameters/RunId"
|
||||
responses:
|
||||
|
|
|
|||
|
|
@ -10417,8 +10417,8 @@ async fn retry_missing_run_returns_not_found() {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn retry_non_retryable_run_returns_conflict() {
|
||||
let state = test_app_state();
|
||||
async fn retry_succeeded_run_creates_and_queues_new_run() {
|
||||
let state = test_app_state_with_isolated_storage();
|
||||
let app = crate::test_support::build_test_router(Arc::clone(&state));
|
||||
let source_run_id = RunId::new();
|
||||
create_durable_run_with_events(&state, source_run_id, &[
|
||||
|
|
@ -10435,6 +10435,68 @@ async fn retry_non_retryable_run_returns_conflict() {
|
|||
},
|
||||
])
|
||||
.await;
|
||||
let source_events_before = state
|
||||
.store
|
||||
.open_run(&source_run_id)
|
||||
.await
|
||||
.unwrap()
|
||||
.list_events()
|
||||
.await
|
||||
.unwrap()
|
||||
.len();
|
||||
|
||||
let response = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri(api(&format!("/runs/{source_run_id}/retry")))
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let body = response_json!(response, StatusCode::CREATED).await;
|
||||
let new_run_id = body["id"].as_str().unwrap().parse::<RunId>().unwrap();
|
||||
|
||||
assert_ne!(new_run_id, source_run_id);
|
||||
assert_eq!(body["retried_from"], source_run_id.to_string());
|
||||
assert_eq!(run_json_status(&body)["kind"], "runnable");
|
||||
|
||||
let source_store = state.store.open_run(&source_run_id).await.unwrap();
|
||||
assert_eq!(
|
||||
source_store.list_events().await.unwrap().len(),
|
||||
source_events_before
|
||||
);
|
||||
assert_eq!(
|
||||
source_store.state().await.unwrap().status,
|
||||
RunStatus::Succeeded {
|
||||
reason: SuccessReason::Completed,
|
||||
}
|
||||
);
|
||||
|
||||
let new_state = state
|
||||
.store
|
||||
.open_run(&new_run_id)
|
||||
.await
|
||||
.unwrap()
|
||||
.state()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(new_state.retried_from, Some(source_run_id));
|
||||
assert_eq!(new_state.status, RunStatus::Runnable);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn retry_active_run_returns_conflict() {
|
||||
let state = test_app_state();
|
||||
let app = crate::test_support::build_test_router(Arc::clone(&state));
|
||||
let source_run_id = RunId::new();
|
||||
create_durable_run_with_events(&state, source_run_id, &[
|
||||
workflow_event::Event::RunSubmitted {
|
||||
definition_blob: None,
|
||||
},
|
||||
])
|
||||
.await;
|
||||
|
||||
let response = app
|
||||
.oneshot(
|
||||
|
|
|
|||
|
|
@ -103,11 +103,12 @@ pub async fn retry_run(
|
|||
}
|
||||
|
||||
fn ensure_retryable(status: RunStatus, run_id: &RunId) -> std::result::Result<(), Error> {
|
||||
match status {
|
||||
RunStatus::Failed { .. } | RunStatus::Dead => Ok(()),
|
||||
other => Err(Error::Precondition(format!(
|
||||
"run {run_id} cannot be retried from status {other}; expected failed or dead"
|
||||
))),
|
||||
if status.is_terminal() {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(Error::Precondition(format!(
|
||||
"run {run_id} cannot be retried from status {status}; expected terminal"
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -235,6 +236,23 @@ mod tests {
|
|||
event::append_event(store, &run_id, &event).await.unwrap();
|
||||
}
|
||||
|
||||
async fn append_succeeded(store: &fabro_store::RunDatabase, run_id: RunId) {
|
||||
append_started(store, run_id).await;
|
||||
event::append_event(store, &run_id, &Event::WorkflowRunCompleted {
|
||||
timing: RunTiming::wall_only(10),
|
||||
artifact_count: 0,
|
||||
status: "succeeded".to_string(),
|
||||
reason: fabro_types::SuccessReason::Completed,
|
||||
total_usd_micros: None,
|
||||
final_git_commit_sha: None,
|
||||
final_patch: None,
|
||||
diff_summary: None,
|
||||
billing: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
async fn seed_retryable_failed_source(
|
||||
store: &Database,
|
||||
source_run_id: RunId,
|
||||
|
|
@ -424,26 +442,51 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn retry_rejects_non_retryable_sources() {
|
||||
async fn retry_creates_fresh_run_from_succeeded_source() {
|
||||
let store = memory_store();
|
||||
|
||||
let succeeded = fixtures::RUN_1;
|
||||
let succeeded_store = store.create_run(&succeeded).await.unwrap();
|
||||
append_created(&succeeded_store, succeeded, None, None).await;
|
||||
append_started(&succeeded_store, succeeded).await;
|
||||
event::append_event(&succeeded_store, &succeeded, &Event::WorkflowRunCompleted {
|
||||
timing: RunTiming::wall_only(10),
|
||||
artifact_count: 0,
|
||||
status: "succeeded".to_string(),
|
||||
reason: fabro_types::SuccessReason::Completed,
|
||||
total_usd_micros: None,
|
||||
final_git_commit_sha: None,
|
||||
final_patch: None,
|
||||
diff_summary: None,
|
||||
billing: None,
|
||||
let source_run_id = fixtures::RUN_1;
|
||||
let source_store = store.create_run(&source_run_id).await.unwrap();
|
||||
append_created(&source_store, source_run_id, None, None).await;
|
||||
let definition_blob = Some(
|
||||
source_store
|
||||
.write_blob(br#"{\"definition\":true}"#)
|
||||
.await
|
||||
.unwrap(),
|
||||
);
|
||||
event::append_event(&source_store, &source_run_id, &Event::RunSubmitted {
|
||||
definition_blob,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
append_succeeded(&source_store, source_run_id).await;
|
||||
|
||||
let outcome = retry_run(&store, &RetryRunInput {
|
||||
source_run_id,
|
||||
new_run_id: RunId::new(),
|
||||
provenance: Some(provenance("retry-user")),
|
||||
web_url: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let retry_store = store.open_run(&outcome.new_run_id).await.unwrap();
|
||||
let retry_events = retry_store.list_events().await.unwrap();
|
||||
let retry_state = fabro_store::RunProjection::apply_events(&retry_events).unwrap();
|
||||
assert_eq!(retry_events.len(), 2);
|
||||
assert_eq!(retry_state.status, RunStatus::Submitted);
|
||||
assert_eq!(retry_state.retried_from, Some(source_run_id));
|
||||
assert_eq!(retry_state.spec.definition_blob, definition_blob);
|
||||
assert_eq!(
|
||||
source_store.state().await.unwrap().status,
|
||||
RunStatus::Succeeded {
|
||||
reason: fabro_types::SuccessReason::Completed,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn retry_rejects_active_and_archived_sources() {
|
||||
let store = memory_store();
|
||||
|
||||
let active = fixtures::RUN_2;
|
||||
let active_store = store.create_run(&active).await.unwrap();
|
||||
|
|
@ -470,7 +513,7 @@ mod tests {
|
|||
.await
|
||||
.unwrap();
|
||||
|
||||
for run_id in [succeeded, active, archived] {
|
||||
for run_id in [active, archived] {
|
||||
let err = retry_run(&store, &RetryRunInput {
|
||||
source_run_id: run_id,
|
||||
new_run_id: RunId::new(),
|
||||
|
|
@ -509,6 +552,17 @@ mod tests {
|
|||
ensure_retryable(RunStatus::Dead, &fixtures::RUN_1).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn succeeded_status_is_retryable() {
|
||||
ensure_retryable(
|
||||
RunStatus::Succeeded {
|
||||
reason: fabro_types::SuccessReason::Completed,
|
||||
},
|
||||
&fixtures::RUN_1,
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cancelled_status_is_retryable() {
|
||||
ensure_retryable(
|
||||
|
|
|
|||
|
|
@ -1120,7 +1120,7 @@ export const RunsApiAxiosParamCreator = function (configuration?: Configuration)
|
|||
};
|
||||
},
|
||||
/**
|
||||
* Creates a fresh run from the failed or dead source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active, succeeded, and archived runs are not retryable.
|
||||
* Creates a fresh run from the terminal source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active and archived runs are not retryable.
|
||||
* @summary Retry Run
|
||||
* @param {string} id Unique run identifier (ULID).
|
||||
* @param {*} [options] Override http request option.
|
||||
|
|
@ -1868,7 +1868,7 @@ export const RunsApiFp = function(configuration?: Configuration) {
|
|||
return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath);
|
||||
},
|
||||
/**
|
||||
* Creates a fresh run from the failed or dead source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active, succeeded, and archived runs are not retryable.
|
||||
* Creates a fresh run from the terminal source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active and archived runs are not retryable.
|
||||
* @summary Retry Run
|
||||
* @param {string} id Unique run identifier (ULID).
|
||||
* @param {*} [options] Override http request option.
|
||||
|
|
@ -2264,7 +2264,7 @@ export const RunsApiFactory = function (configuration?: Configuration, basePath?
|
|||
return localVarFp.retrieveRunGraphSource(id, options).then((request) => request(axios, basePath));
|
||||
},
|
||||
/**
|
||||
* Creates a fresh run from the failed or dead source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active, succeeded, and archived runs are not retryable.
|
||||
* Creates a fresh run from the terminal source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active and archived runs are not retryable.
|
||||
* @summary Retry Run
|
||||
* @param {string} id Unique run identifier (ULID).
|
||||
* @param {*} [options] Override http request option.
|
||||
|
|
@ -2652,7 +2652,7 @@ export class RunsApi extends BaseAPI {
|
|||
}
|
||||
|
||||
/**
|
||||
* Creates a fresh run from the failed or dead source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active, succeeded, and archived runs are not retryable.
|
||||
* Creates a fresh run from the terminal source run\'s captured durable definition, records `retried_from` on the new run, and schedules it for execution. The source run is left unchanged. Active and archived runs are not retryable.
|
||||
* @summary Retry Run
|
||||
* @param {string} id Unique run identifier (ULID).
|
||||
* @param {*} [options] Override http request option.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue