Prevent Dask from executing recipes and probes more than once per invocation

[EiffL]

Dask may rerun a task after losing its worker or its completed result, even with retries=0. For recipes that write project files, that can repeat side effects while the original command is still running.

This PR adds one atomic claim in the existing Dask scheduler before each recipe or probe executes. A second attempt is refused. Missing invocation state, an unavailable scheduler, or a disconnected driver also refuses new execution. Claims are scoped to one invocation and removed on exit; later invocations prune abandoned records.

The implementation is a 91-line module and small integrations in run and materialize. There are no leases, heartbeats, completion receipts, process supervisors, sandbox changes, or automatic restoration after interruption. It does not stop an already running command or prevent independent invocations from writing the same project. Existing interruption behavior retains unreported partial outputs for inspection.

CPU, memory, and GPU resource management remains in the separate stacked PR #230.

Validation: all 892 tests pass locally, plus Ruff, strict mypy, and a strict documentation build. Tests exercise real Dask worker loss during and after execution, task forgetting and resubmission, missing scheduler state, lost claim replies, and ordinary task failure. No new Slurm job was submitted. All automated GitHub checks pass at 0eb27e9: Linux/Python 3.11–3.13, macOS/Python 3.13, lint, agent evaluation, automated review, and sign-off validation. The external DCO gate awaits PR approval.

[github-actions]

✅ Eval

Metric	Value
Outputs check	success
Agent run	success
Turns	9
Tool calls	7
Cost	$0.14
Agent wall time	0m50s
Model	claude-sonnet-5-5

lc status

  mode:    direct
  sandbox: landlock (fs: declared, network: allowed)
  crate:   up to date with the outputs

  · current  baseline/best_fit        e54414d
  · current  baseline/hubble_diagram  f753373
  · current  baseline/residuals       e54414d

3 current

Confusion & pain points (Claude analysis)

Confusion & pain points

• The run was essentially clean. All tool calls succeeded on the first attempt except one build failure, and no lc or astra verb was misunderstood. The friction was small, and none of it was a product gap.
• The first lc materialize failed on a Python SyntaxError, which cost a fix, a commit and a rerun. The agent generated plot_hubble.py from a nested bash heredoc and a sed-style template. That put \" inside an f-string expression (f"H0={b[\"H0\"]:.1f}"). It also ran git add and lc compute launch in the same chained command as the script writes. It never ran the scripts locally before materializing.
  • The cause was agent-side quoting, not lc.
  • lc did surface the traceback correctly, and the tree stayed clean for the retry.
  • A cheap lc run <cluster> python scripts/plot_hubble.py --help probe, or python -m py_compile, would have caught the error before a commit and an allocation.
• The agent left a no-op mv scripts/plot_hubble.py scripts/plot_hubble.py, which errored with "same file". This is agent slop from an abandoned rename, not a product issue. The chain ran on because it wasn't under set -e. The risk is that a real failure in a long chained command is easy to miss.
• The agent spelled the astra CLI as uvx astra-tools@0.2.18 validate. It had to pin a version and use uvx because astra isn't on the PATH of the project or the tool install. The skill and the environment probably should say which invocation to use.
  • No error occurred, but the pinned 0.2.18 had to come from somewhere (probably the skill).
  • If the eval harness should provide astra directly, putting it on the PATH would remove the guess.
• Minor: the agent added license = "CC-BY-4.0" with a sed insertion after requires-python in pyproject.toml. It did this to switch on crate maintenance, which lc derives from [project].license. This worked, but only because the agent knew that lc materialize maintains the crate when a license is set.
  • The task and skill didn't obviously state that connection.
  • If a crate is expected in every eval, the scaffold or a lc init report line could make the license requirement more discoverable.

Full trace: agent-trace artifact on this run.

[EiffL]

Code review: bugs, overengineering and reuse

Automated review of main...c1a1b67 (3 commits, 36 files), focused on correctness and on overengineering and poor code reuse. Line references were checked against this commit, but the findings themselves have not each been reproduced.

The most serious issue is in the lease: a single failed scheduler call ends it, so one hiccup can abort a whole run. Bugs come first, most severe first, then the design issues.

Bugs

1. One failed call ends the lease (execution.py:193, execution.py:128). The driver's heartbeat thread returns on its first exception and never renews again, and each task's monitor treats one failed active call as revocation. A scheduler stall over 5 s (GC pause, a large graph submission, slow TLS on an HPC interconnect) makes one _rpc hit callback_timeout. Every running recipe is then killed with ExecutionCancelled, even though the lease is meant to tolerate 15 s and the scheduler recovered within a second.
2. A fully successful run can end as ExecutionUncertain (execution.py:223). If the cleanup revoke/forget call fails after every task was consumed, the except Exception raises ExecutionUncertain. The made/failed report is lost, and the user is told partial outputs were retained and to stop the allocation. lc run loses a successful probe's outcome the same way.
3. Exit-0 recipes can be reported as failed (sandbox/processes.py:308). The process group is scanned the moment the leader becomes a zombie, with no grace period. A multiprocessing or joblib resource_tracker that is still exiting counts as "left background processes running", so a recipe that exited 0 reports exit code 1. Its output is restored instead of committed, and whether this happens is random.
4. Supervisor cleanup timeouts are far below the 15 s budget (processes.py:229, processes.py:81). Every runtime call (inspect/stop/kill/rm) gets timeout=2, and _drain gives SIGKILL 1 s. A slow podman inspect under storage-lock contention, or a timed-out recipe stuck in uninterruptible Lustre I/O, then turns into a false "uncertain" abort.
5. The walltime SIGKILL can be skipped (compute/local_runtime.py:48). _stop_session now runs a lazy import and psutil calls before os.kill(owner, SIGKILL), with no try/finally, and catches only NoSuchProcess. An ImportError or psutil.AccessDenied raised inside the SIGALRM handler skips the kill, so a recipe that ignores SIGTERM outlives the allocation's time limit. The old os.killpg could not be skipped.
6. The UNSTOPPED message is usually false (materialize.py:739, run.py:70). "Tasks that did not report may still be running, and any files they wrote remain" is added before the invocation drains. With this change the drain usually confirms the stop and restores those outputs, yet the user is still told to run lc compute down and look for leftover files.
7. Resource parsing breaks read-only verbs (plan.py:337). TaskResources.parse runs while the graph is built. A spec astra validate accepts but lc cannot execute (gpus, disk, fractional cpus, cpus: 0, memory: null) now makes lc status and lc materialize --check refuse, although nothing is being executed. The refusal belongs at submission (_Dask.validate).
8. A needless 40 s hang (execution.py:213). Once any task is recorded uncertain, which nothing can change, the drain still polls for the full _STOP_TIMEOUT before raising. revoke/pending should return immediately when an uncertain entry exists.

Overengineering and reuse

• Stopping a session is written twice.
  • compute/local.py:395 repeats the scan processes.members(session=…) already does, and imports that same helper 40 lines later (local.py:439).
  • local_runtime.py _stop_session repeats the terminate → grace → kill sequence.
  • The custodian grace is spelled 16, 15 and _CLEANUP_TIMEOUT = 15.0, and the plain grace is 3.0 in one path and 2.5 in the other.
  • The has_custodian command-line match only picks a grace, which the wait loops don't need because they exit early once the session is empty. One processes.stop_session(sid) would serve both callers.
• Parsers written twice (execution_resources.py:147). _duration/_memory repeat compute.model.duration/memory_bytes (the same exact-bytes arithmetic) with a different grammar. For example, time_limit: 1h30m is accepted in astra.yaml but refused in compute.yaml. TaskResources(cpus, memory_bytes, time_seconds) also duplicates compute.model.Request(cpus, memory_bytes, seconds), against the "one shared Pydantic model family" rule. The shared parsers should be extended instead.
• String-dispatched scheduler state (execution.py:49). _state is a ten-operation function selected by a string and stored as a raw dict in dask_scheduler.extensions, which Dask iterates at teardown, and revoke falls through into pending. A mistyped operation only fails at runtime. _request (execution.py:104) is a one-line wrapper with one caller, and the client.sync(client.run_on_scheduler, …, callback_timeout=…) idiom used for client.cancel could replace it. A small SchedulerPlugin with named handlers would be clearer.
• The exceptions live in the wrong layer. ExecutionCancelled/ExecutionUncertain are defined in execution.py but raised in sandbox/processes.py and caught in sandbox/boundary.py. That takes five function-local imports to avoid a cycle (processes.py:134, 152, 163, 203; boundary.py:120), and makes the exec boundary depend on the Dask invocation layer above it. They belong in sandbox, re-exported by execution.
• An untyped flag on a builtin exception (execution.py:222). execution_stopped is set on a KeyboardInterrupt under type: ignore, then read with getattr in two places in cli/commands.py (192, 386). mypy cannot check it, and a KeyboardInterrupt from any other layer silently reads as "not stopped". _interrupted could take the flag, or the engine could raise a dedicated exception.
• OCI details in the generic supervisor (processes.py:285). --cidfile is spliced in at the fixed position argv[2:2], selected by contains_prefix, which builds OCIBackend's [runtime, "run", …] layout into generic code. A future apptainer backend (contains_prefix=True) would get apptainer exec --cidfile … and apptainer inspect. CLAUDE.md already says this kind of fact should become mechanism-keyed. The flag belongs in OCIBackend.wrap, with cleanup keyed on the backend.
• _Dask.submit reserves too much (materialize.py:712).
  • It finds the task by position (args[1]), so reordering worker.materialize's arguments breaks every submit.
  • It recomputes requirements(), which validate just computed.
  • It reserves recipe resources even for tasks that will only classify as current, behind or blocked. With no memory declared, that is the worker's whole memory, so a fully up-to-date graph is classified one task at a time instead of task_slots_per_node at once.
  • The resources should be passed explicitly.

🤖 Generated with Claude Code