Scope the in-flight reattach store to the dispatch unit, not the machine
Summary
- A parallel
StartWithPreviousbatch dispatches its steps concurrently (Task.WhenAll). When two steps share a target machine, both callDispatchOrReattachAsyncfor the same(task, machine)at once. The in-flight reattach store (InFlightScriptStore) was keyed by(serverTaskId, machineId)only, so the second dispatch's reattach probe found the first's just-recorded ticket and "skipped duplicate dispatch" — silently running the first step's script instead of its own, with the losing observer returning exit-1and failing the deployment. Record-before-RPC (#431) widened the window by persisting the ticket beforeStartScript. - Fix: key the in-flight slot by the dispatch unit via a new
DispatchSlot(MachineId, StepId, ActionId). It keys on the stable, process-unique step/action ids — NOT display names, which carry no uniqueness constraint (the schema enforces uniquestep_order/action_orderonly, so two identically-named steps would otherwise re-collapse to one slot). The ids come from the frozen process snapshot, so they are identical across a crash→resume; threaded ontoScriptExecutionRequestand attached post-render at the single dispatch site (alongside the existing masker attach). - Concurrent dispatches to one machine now stay independent — live and on resume each re-attaches only to its OWN ticket. The checkpoint JSON moves from a machine-keyed object to a
{m,s,a,t}list; older/interim shapes deserialise to empty (the documented re-dispatch fail-safe), so it is non-breaking. - This is the real bug behind the flaky
StepBatcherParallelE2ETests(twoStartWithPrevioussteps on one agent): with the slot fix both steps run their own script, the deployment succeeds, and the agent-side execution intervals overlap deterministically. The E2E also now proves overlap from agent-clock timestamps emitted via adateformat string (immune to the JSON action-property round-trip) instead of a flaky total-wall-clock ceiling.
Test plan
-
Unit (5986/5986): InFlightScriptMapTests— same-machine/different-action-id independence, sibling-slot non-match, legacy machine-keyed + interim name-keyed shapes tolerated as empty -
Integration (real Postgres): InFlightScriptStoreTestsdispatch-scoped independence + concurrent same-machine record;HalibutResumeReattachTests.ConcurrentDispatch_SameMachineIdenticallyNamedSteps_…— deterministic two-concurrent-dispatch regression with TWO identically-named steps differing only by id (a gate parks step A's RPC until step B has dispatched), proving no cross-reattach,StartScriptCalls == 2 -
E2E (K8s Pipeline): StepBatcherParallelE2ETeststwoStartWithPrevioussteps on one agent → Success + overlapping intervals -
Full unit suite green; non-breaking (transient checkpoint column, fail-safe handles old shapes) -
Adversarial multi-agent review (concurrency / resume / non-breaking / completeness) — the name-vs-id finding drove the id-keying