fix(submit): release the singleflight claim when a submit fails

[maharajamihir]

The bug

submit_recipe_inner claims the per-cache_key singleflight slot at try_claim_or_follow (src/runner.rs:442) but doesn't reach sbatch until ~120 lines later — after the output-marker check, the output mkdirs, copy_dir_filtered, the context.json write and render_script. Every error in that window returned straight to the caller and left the claim row sitting at status = 'created', job_id = NULL, which is exactly the predicate runs_cache_key_inflight_unique (migration 0003) treats as in-flight.

Nothing ages that row out. Every later submission of the same recipe finds it, attaches as a follower of a leader that will never run, and returns follower_of: run_... with no job. No CLI command clears it.

Hit for real today: a recipe whose command contained ${LABCTL_RUN_ID} failed at render_script; the next two submissions came back as followers and it took a hand-written UPDATE runs SET status='failed' to unblock.

The fix

The post-claim stretch now runs inside an async block; its error arm calls a new release_claim_on_failed_submit, which

• flips the row to failed via the existing update_status helper (which supplies the finished_at that the runs_terminal_finished_at biconditional requires for any terminal status),
• keeps the row — it's the record of what went wrong — but it no longer owns the key,
• returns the original error unchanged, so the cleanup never masks the failure that triggered it,
• logs a cleanup that itself fails with tracing::error! and swallows it, for the same reason.

Same per-run failure isolation agent.rs and try_submit_pending_children already apply. The follower and cache_hit paths both return before the claim window and are untouched; so is the pending-pipeline-children sweep, whose own update_status(child, "failed") on a failed complete_pending_submission is now redundant but harmless.

The runner.rs diff looks large because the wrapped body is re-indented; git diff -w is 167 insertions, 1 deletion.

Test

runner::claim_tests::failed_submit_releases_singleflight_claim (PG-gated, #[ignore]d like the rest) drives a real submit_recipe_inner against the live registry with runs_base, the output root and the repo mapping all redirected into a tempdir, so nothing lands in the shared filesystem. The injected failure is the marker-collision bail! — the first thing that can fail after the claim — so the test never reaches source-copy or sbatch. It then asserts the row is failed with finished_at set and job_id still NULL, and that find_inflight_for_cache_key no longer returns it.

It observes, then cleans up, then asserts, so a failing assertion can't strand rows in the shared registry (which matters for the negative control below).

The pg_store::tests fixtures it reuses (live_cluster, dummy_recipe, insert_test_user, cleanup_user) are now pub(crate).

Run on hai-login2 (where the PG socket lives)

$ cargo test --all-features failed_submit_releases_singleflight_claim -- --ignored --nocapture
running 1 test
test runner::claim_tests::failed_submit_releases_singleflight_claim ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 44 filtered out; finished in 0.76s

Negative control — same test with the production fix neutered

release_claim_on_failed_submit reduced to err (no update_status), everything else identical:

thread 'runner::claim_tests::failed_submit_releases_singleflight_claim' panicked at src/runner.rs:2358:9:
assertion `left == right` failed: claim row must be terminal after a failed submit
  left: "created"
 right: "failed"
test runner::claim_tests::failed_submit_releases_singleflight_claim ... FAILED

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 44 filtered out; finished in 0.67s

i.e. the stranded created claim reproduces exactly, and the test catches it.

Full suite (local, non-PG)

test result: ok. 36 passed; 0 failed; 9 ignored; 0 measured; 0 filtered out   # unit (was 8 ignored; +1 is the new test)
test result: ok. 9 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out    # tests/smoke.rs

cargo fmt --check and cargo clippy --all-features --all-targets are clean. No test rows remain in the registry after either run.

🤖 Generated with Claude Code