Record the in-flight ScriptTicket before the StartScript RPC#431
Merged
Conversation
DispatchOrReattachAsync recorded the in-flight ticket only AFTER StartScriptAsync returned. If the server crashed (or the RPC response was lost) in the window between the agent launching the script and that record committing, the resumed run found no pointer and dispatched a fresh ticket — running the (non-idempotent) deployment script a second time. Record the ticket BEFORE firing StartScript so a resume always knows a ticket to re-probe: TryReattachAsync re-attaches if the agent still has the script, and falls through to a clean fresh dispatch (via the Complete+UnknownResult sentinel) if it never started. The agent is idempotent for a known ticket, so this only ever avoids a duplicate, and it composes with the existing reattach/checkpoint/circuit-breaker design rather than changing it. Adds an integration regression test that captures, at the instant StartScript fires, what the store has durably recorded: red before the reorder, green after.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DispatchOrReattachAsyncrecorded the in-flight ScriptTicket afterStartScriptAsyncreturned (HalibutMachineExecutionStrategy.cs). A server crash — or a lost RPC response — in the window between the agent launching the script and the server committing the ticket left no durable pointer, so the resumed run dispatched a fresh ticket and ran the (non-idempotent) deployment script a second time.TryReattachAsyncre-attaches if the agent still holds the script, or falls through to a clean fresh dispatch (via the agent'sComplete+UnknownResultunknown-ticket sentinel) if it never started.This is the StartScript half of the SOTA-audit P1 (server-side idempotency, aligning with Octopus ScriptServiceV2). The RPC-retry + observe-loop pointer-preservation half (
GetStatus/CompleteScript) follows in a separate PR.Why it's non-breaking + generic
IInFlightScriptStoreand checkpoint column. No new state, no signature change.LocalScriptService/ScriptPodServicededupe by ticket), so a retried/known StartScript returns status, not a second launch.RecordDispatchedAsyncitself fails, the dispatch now aborts cleanly before any agent-side launch (strictly safer than the old order).Test plan
HalibutResumeReattachTests.FreshDispatch_RecordsInFlightTicket_BeforeStartScriptRpc— a hook captures, at the instant StartScript fires, what the store has durably committed (read through a fresh DI scope, as a resumed server would). Verified red→green: fails with "record-before-RPC violated" before the reorder, passes after.dotnet build0 errors.