mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
ci: lock Cargo resolution and fix cancellation flake (#461)
Some checks failed
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
Some checks failed
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
## Summary
Enforce Cargo lockfile use across CI and release automation so jobs fail
on stale `Cargo.lock` state instead of resolving dependencies
implicitly. This adds `--locked` to Rust CI, release builds/tests,
nightly release tagging, the TypeScript workflow's embedded Rust build,
and helper-owned Cargo calls in `fabro-dev`.
This also fixes the Linux CI flake exposed by the PR: canceling a
durably blocked in-process run could take the abort path while the
workflow was still unwinding a human-input gate, causing
`run.failed(cancelled)` to be followed by `run.unblocked`. That invalid
event order broke projection rebuilds and made `GET /runs/{id}` return
404. Cancellation now uses the durable lifecycle status when selecting
the in-process blocked-run path, so the pending interview is cancelled
before the terminal event is emitted.
The release command's intentional `cargo update --workspace` step is
unchanged, because that step updates `Cargo.lock` after bumping the
workspace version.
## Testing
- `cargo nextest run --locked -p fabro-dev --features dev -E
'test(dry_run_computes_stable_version_from_date) |
test(dry_run_prints_equivalent_build_commands)'`
- `cargo --locked dev release --dry-run --skip-tests --release-date
2026-01-01`
- `cargo --locked dev docs check`
- `cargo nextest run --locked -p fabro-server --features test-support
cancel_durably_blocked_in_process_run_cancels_pending_interview_without_abort_signal
--status-level fail --final-status-level fail --show-progress none`
- `cargo nextest run --locked -p fabro-server --features test-support
--test it scenario::lifecycle --profile ci --status-level fail
--final-status-level fail --show-progress none --no-fail-fast`
- Linux Docker stress reproduction: `cargo nextest run --locked -p
fabro-server --features test-support --test it
scenario::lifecycle::full_http_lifecycle_cancel --profile ci
--stress-count 200 --status-level fail --final-status-level fail
--show-progress none`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --locked -p fabro-dev --features dev
--all-targets -- -D warnings`
- `cargo +nightly-2026-04-14 clippy --locked -p fabro-server --features
test-support --all-targets -- -D warnings`
- `git diff --check`
Full `cargo nextest run --locked -p fabro-dev --features dev` currently
has two unrelated policy-test failures:
`policy::catalog_builtin_references_stay_in_allowlist` and
`policy::workflow_template_rendering_call_sites_stay_in_allowlist`.
---
[](https://github.com/EveryInc/compound-engineering-plugin)
🤖 Generated with GPT-5 via [Codex](https://openai.com/codex)
---------
Co-authored-by: Release Repro <release-repro@example.com>
This commit is contained in:
parent
1184e25eab
commit
b5de404354
11 changed files with 138 additions and 45 deletions
2
.github/workflows/nightly.yml
vendored
2
.github/workflows/nightly.yml
vendored
|
|
@ -67,4 +67,4 @@ jobs:
|
|||
git remote set-url origin \
|
||||
"https://x-access-token:${release_token}@github.com/${GITHUB_REPOSITORY}.git"
|
||||
unset release_token
|
||||
cargo dev release --nightly
|
||||
cargo --locked dev release --nightly
|
||||
|
|
|
|||
10
.github/workflows/release.yml
vendored
10
.github/workflows/release.yml
vendored
|
|
@ -84,7 +84,7 @@ jobs:
|
|||
- uses: taiki-e/install-action@773334c0e05d7e699e4d78234494308223f3a2cf # nextest
|
||||
|
||||
- name: Refresh embedded SPA
|
||||
run: cargo dev spa refresh
|
||||
run: cargo --locked dev spa refresh
|
||||
|
||||
- name: Test (x86_64-musl)
|
||||
# nextest still shells through cargo test for this target, so
|
||||
|
|
@ -93,22 +93,22 @@ jobs:
|
|||
env:
|
||||
CC_x86_64_unknown_linux_musl: musl-gcc
|
||||
CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER: musl-gcc
|
||||
run: cargo nextest run --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci
|
||||
run: cargo nextest run --locked --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci
|
||||
|
||||
- name: Test
|
||||
# aarch64-musl test runs have not been validated on the compile
|
||||
# runner yet; shipping binary is exercised via Docker smoke tests.
|
||||
# Re-enable after verifying the workspace passes on this target.
|
||||
if: matrix.target != 'aarch64-unknown-linux-musl' && matrix.target != 'x86_64-unknown-linux-musl'
|
||||
run: cargo nextest run --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci
|
||||
run: cargo nextest run --locked --workspace --target ${{ matrix.target }} --release --status-level slow --profile ci
|
||||
|
||||
- name: Build (musl via cargo-zigbuild)
|
||||
if: matrix.musl
|
||||
run: cargo zigbuild --target ${{ matrix.target }} --release -p fabro-cli
|
||||
run: cargo zigbuild --locked --target ${{ matrix.target }} --release -p fabro-cli
|
||||
|
||||
- name: Build
|
||||
if: ${{ !matrix.musl }}
|
||||
run: cargo build --target ${{ matrix.target }} --release -p fabro-cli
|
||||
run: cargo build --locked --target ${{ matrix.target }} --release -p fabro-cli
|
||||
|
||||
- name: Package
|
||||
run: |
|
||||
|
|
|
|||
8
.github/workflows/rust.yml
vendored
8
.github/workflows/rust.yml
vendored
|
|
@ -76,7 +76,7 @@ jobs:
|
|||
run: |
|
||||
! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*"disabled"' \
|
||||
lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml
|
||||
- run: cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
|
||||
- run: cargo +nightly-2026-04-14 clippy --locked --workspace --all-targets -- -D warnings
|
||||
|
||||
generated-docs:
|
||||
name: Generated Docs
|
||||
|
|
@ -91,7 +91,7 @@ jobs:
|
|||
- uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2
|
||||
with:
|
||||
cache-on-failure: true
|
||||
- run: cargo dev docs check
|
||||
- run: cargo --locked dev docs check
|
||||
|
||||
test:
|
||||
name: Test (Linux)
|
||||
|
|
@ -107,7 +107,7 @@ jobs:
|
|||
with:
|
||||
cache-on-failure: true
|
||||
- uses: taiki-e/install-action@773334c0e05d7e699e4d78234494308223f3a2cf # nextest
|
||||
- run: cargo nextest run --workspace --status-level slow --profile ci
|
||||
- run: cargo nextest run --locked --workspace --status-level slow --profile ci
|
||||
|
||||
test-macos:
|
||||
name: Test (macOS)
|
||||
|
|
@ -124,4 +124,4 @@ jobs:
|
|||
with:
|
||||
cache-on-failure: true
|
||||
- uses: taiki-e/install-action@773334c0e05d7e699e4d78234494308223f3a2cf # nextest
|
||||
- run: cargo nextest run --workspace --status-level slow --profile ci
|
||||
- run: cargo nextest run --locked --workspace --status-level slow --profile ci
|
||||
|
|
|
|||
2
.github/workflows/typescript.yml
vendored
2
.github/workflows/typescript.yml
vendored
|
|
@ -73,5 +73,5 @@ jobs:
|
|||
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
|
||||
- run: bun install --frozen-lockfile
|
||||
- uses: dtolnay/rust-toolchain@631a55b12751854ce901bb631d5902ceb48146f7 # stable
|
||||
- run: cargo dev build -- -p fabro-cli --release
|
||||
- run: cargo --locked dev build -- --locked -p fabro-cli --release
|
||||
- run: wc -c < target/release/fabro
|
||||
|
|
|
|||
|
|
@ -148,6 +148,7 @@ impl DockerBuildPlan {
|
|||
|
||||
fn spa_refresh_command() -> PlannedCommand {
|
||||
PlannedCommand::new("cargo")
|
||||
.arg("--locked")
|
||||
.arg("dev")
|
||||
.arg("spa")
|
||||
.arg("refresh")
|
||||
|
|
@ -235,6 +236,6 @@ fn build_script(target: &str, zig_arch: &str) -> String {
|
|||
cargo install --locked --root /opt/cargo-tools cargo-zigbuild; \
|
||||
fi; \
|
||||
rustup target add {target}; \
|
||||
cargo zigbuild --release -p fabro-cli --target {target}"
|
||||
cargo zigbuild --locked --release -p fabro-cli --target {target}"
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ pub(crate) fn docs_cli_reference_root(root: &Path, check: bool) -> Result<()> {
|
|||
fn render_cli_reference() -> Result<String> {
|
||||
let command = PlannedCommand::new("cargo")
|
||||
.arg("run")
|
||||
.arg("--locked")
|
||||
.arg("-p")
|
||||
.arg("fabro-cli")
|
||||
.arg("--")
|
||||
|
|
|
|||
|
|
@ -237,6 +237,7 @@ impl ReleasePlan {
|
|||
|
||||
fn spa_refresh_command() -> PlannedCommand {
|
||||
PlannedCommand::new("cargo")
|
||||
.arg("--locked")
|
||||
.arg("dev")
|
||||
.arg("spa")
|
||||
.arg("refresh")
|
||||
|
|
@ -249,6 +250,7 @@ impl ReleasePlan {
|
|||
.env("SEGMENT_WRITE_KEY", RELEASE_TEST_SEGMENT_WRITE_KEY)
|
||||
.arg("nextest")
|
||||
.arg("run")
|
||||
.arg("--locked")
|
||||
.arg("--workspace")
|
||||
.arg("--release")
|
||||
.arg("--profile")
|
||||
|
|
|
|||
|
|
@ -53,7 +53,7 @@ fn dry_run_prints_equivalent_build_commands() {
|
|||
let stdout = output_text(&output.stdout);
|
||||
|
||||
assert!(
|
||||
stdout.contains("cargo dev spa refresh"),
|
||||
stdout.contains("cargo --locked dev spa refresh"),
|
||||
"dry-run should print SPA refresh command:\n{stdout}"
|
||||
);
|
||||
assert!(
|
||||
|
|
@ -61,7 +61,9 @@ fn dry_run_prints_equivalent_build_commands() {
|
|||
"dry-run should print builder docker run:\n{stdout}"
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("cargo zigbuild --release -p fabro-cli --target x86_64-unknown-linux-musl"),
|
||||
stdout.contains(
|
||||
"cargo zigbuild --locked --release -p fabro-cli --target x86_64-unknown-linux-musl"
|
||||
),
|
||||
"dry-run should print cargo-zigbuild target:\n{stdout}"
|
||||
);
|
||||
assert!(
|
||||
|
|
|
|||
|
|
@ -96,7 +96,7 @@ fn dry_run_computes_stable_version_from_date() {
|
|||
"dry-run should compute base version from date:\n{stdout}"
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("cargo dev spa refresh"),
|
||||
stdout.contains("cargo --locked dev spa refresh"),
|
||||
"dry-run should print one SPA refresh command:\n{stdout}"
|
||||
);
|
||||
assert!(
|
||||
|
|
@ -109,7 +109,7 @@ fn dry_run_computes_stable_version_from_date() {
|
|||
);
|
||||
assert!(
|
||||
stdout.contains(
|
||||
"unset GH_TOKEN GITHUB_TOKEN && SEGMENT_WRITE_KEY=fake-for-local-smoke cargo nextest run"
|
||||
"unset GH_TOKEN GITHUB_TOKEN && SEGMENT_WRITE_KEY=fake-for-local-smoke cargo nextest run --locked"
|
||||
),
|
||||
"dry-run should show release tests without inherited GitHub tokens:\n{stdout}"
|
||||
);
|
||||
|
|
|
|||
|
|
@ -371,46 +371,67 @@ async fn cancel_run(
|
|||
if let Some(response) = reject_if_archived(state.as_ref(), &id).await {
|
||||
return response;
|
||||
}
|
||||
let pending_control = match load_pending_control(state.as_ref(), id).await {
|
||||
Ok(pending_control) => pending_control,
|
||||
let durable_summary = match state.store.runs().find(&id).await {
|
||||
Ok(summary) => summary,
|
||||
Err(err) => {
|
||||
return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string())
|
||||
.into_response();
|
||||
}
|
||||
};
|
||||
let pending_control = durable_summary
|
||||
.as_ref()
|
||||
.and_then(|summary| summary.lifecycle.pending_control);
|
||||
let durable_status = durable_summary
|
||||
.as_ref()
|
||||
.map(|summary| summary.lifecycle.status);
|
||||
let cancel_target = {
|
||||
let mut runs = state.runs.lock().expect("runs lock poisoned");
|
||||
match runs.get_mut(&id) {
|
||||
Some(managed_run) => match managed_run.status {
|
||||
RunStatus::Submitted
|
||||
| RunStatus::Pending { .. }
|
||||
| RunStatus::Runnable
|
||||
| RunStatus::Starting
|
||||
| RunStatus::Running
|
||||
| RunStatus::Blocked { .. }
|
||||
| RunStatus::Paused { .. } => {
|
||||
let persist_cancelled_status = matches!(
|
||||
managed_run.status,
|
||||
RunStatus::Submitted | RunStatus::Pending { .. } | RunStatus::Runnable
|
||||
);
|
||||
if persist_cancelled_status {
|
||||
managed_run.status = RunStatus::Failed {
|
||||
reason: FailureReason::Cancelled,
|
||||
Some(managed_run) => {
|
||||
let managed_status = managed_run.status;
|
||||
match managed_status {
|
||||
RunStatus::Submitted
|
||||
| RunStatus::Pending { .. }
|
||||
| RunStatus::Runnable
|
||||
| RunStatus::Starting
|
||||
| RunStatus::Running
|
||||
| RunStatus::Blocked { .. }
|
||||
| RunStatus::Paused { .. } => {
|
||||
let answer_transport = managed_run.answer_transport.clone();
|
||||
let should_cancel_pending_interview =
|
||||
matches!(
|
||||
&answer_transport,
|
||||
Some(RunAnswerTransport::InProcess { .. })
|
||||
) && (matches!(managed_status, RunStatus::Blocked { .. })
|
||||
|| matches!(durable_status, Some(RunStatus::Blocked { .. })));
|
||||
let persist_cancelled_status = matches!(
|
||||
managed_status,
|
||||
RunStatus::Submitted | RunStatus::Pending { .. } | RunStatus::Runnable
|
||||
) && !should_cancel_pending_interview;
|
||||
if persist_cancelled_status {
|
||||
managed_run.status = RunStatus::Failed {
|
||||
reason: FailureReason::Cancelled,
|
||||
};
|
||||
}
|
||||
let cancel_tx = if should_cancel_pending_interview {
|
||||
None
|
||||
} else {
|
||||
managed_run.cancel_tx.take()
|
||||
};
|
||||
Some((
|
||||
persist_cancelled_status,
|
||||
answer_transport,
|
||||
managed_run.cancel_token.clone(),
|
||||
cancel_tx,
|
||||
managed_run.worker_ref.clone(),
|
||||
))
|
||||
}
|
||||
_ => {
|
||||
return ApiError::new(StatusCode::CONFLICT, "Run is not cancellable.")
|
||||
.into_response();
|
||||
}
|
||||
Some((
|
||||
persist_cancelled_status,
|
||||
managed_run.answer_transport.clone(),
|
||||
managed_run.cancel_token.clone(),
|
||||
managed_run.cancel_tx.take(),
|
||||
managed_run.worker_ref.clone(),
|
||||
))
|
||||
}
|
||||
_ => {
|
||||
return ApiError::new(StatusCode::CONFLICT, "Run is not cancellable.")
|
||||
.into_response();
|
||||
}
|
||||
},
|
||||
}
|
||||
None => None,
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -13626,6 +13626,72 @@ async fn cancel_run_requests_worker_runtime_stop_when_control_unavailable() {
|
|||
assert_eq!(runtime.requested_refs(), vec![worker_ref]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cancel_durably_blocked_in_process_run_cancels_pending_interview_without_abort_signal() {
|
||||
let state = test_app_state();
|
||||
let app = crate::test_support::build_test_router(Arc::clone(&state));
|
||||
let run_id = fixtures::RUN_1;
|
||||
create_durable_run_with_events(&state, run_id, &[
|
||||
workflow_event::Event::RunRunning,
|
||||
workflow_event::Event::RunBlocked {
|
||||
blocked_reason: BlockedReason::HumanInputRequired,
|
||||
},
|
||||
])
|
||||
.await;
|
||||
|
||||
let interviewer = Arc::new(ControlInterviewer::new());
|
||||
let mut question = Question::new("approve?", QuestionType::YesNo);
|
||||
question.id = "q-1".to_string();
|
||||
let ask_interviewer = Arc::clone(&interviewer);
|
||||
let ask = tokio::spawn(async move { ask_interviewer.ask(question).await });
|
||||
tokio::task::yield_now().await;
|
||||
|
||||
let (cancel_tx, mut cancel_rx) = oneshot::channel();
|
||||
let cancel_token = CancellationToken::new();
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
let mut run = managed_run(
|
||||
MINIMAL_DOT.to_string(),
|
||||
RunStatus::Running,
|
||||
Utc::now(),
|
||||
temp_dir.path().join(run_id.to_string()),
|
||||
RunExecutionMode::Start,
|
||||
);
|
||||
run.answer_transport = Some(RunAnswerTransport::InProcess {
|
||||
interviewer,
|
||||
steering_hub: Arc::new(fabro_workflow::SteeringHub::new(Arc::new(
|
||||
fabro_workflow::event::Emitter::new(run_id),
|
||||
))),
|
||||
});
|
||||
run.cancel_token = Some(cancel_token);
|
||||
run.cancel_tx = Some(cancel_tx);
|
||||
state
|
||||
.runs
|
||||
.lock()
|
||||
.expect("runs lock poisoned")
|
||||
.insert(run_id, run);
|
||||
|
||||
let req = Request::builder()
|
||||
.method("POST")
|
||||
.uri(api(&format!("/runs/{run_id}/cancel")))
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
let response = app.oneshot(req).await.unwrap();
|
||||
assert_status!(response, StatusCode::OK).await;
|
||||
|
||||
let submission = tokio::time::timeout(std::time::Duration::from_millis(100), ask)
|
||||
.await
|
||||
.expect("cancel should resolve the pending in-process interview")
|
||||
.expect("interview task should not panic");
|
||||
assert_eq!(submission.answer.value, AnswerValue::Cancelled);
|
||||
assert!(
|
||||
matches!(
|
||||
cancel_rx.try_recv(),
|
||||
Err(tokio::sync::oneshot::error::TryRecvError::Empty)
|
||||
),
|
||||
"blocked in-process cancellation should let the workflow unwind instead of aborting it"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pause_run_rejects_when_control_is_already_pending() {
|
||||
let state = test_app_state();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue