fix(orchestrate): re-derive component state leaf onto fresh trunk before the first push - #549
Merged
Merged
Conversation
…rst state commit The component-scoped finalize committed its owned state leaf onto the checkout-time manifest and relied on a push rejection to surface staleness. That defends only when local HEAD is behind trunk. When HEAD already sits at the trunk tip but the working manifest is stale, the first push fast-forwards with no rejection, so the re-apply never runs and the stale sibling leaf lands, reverting a concurrent component's already-committed value. The Contents-API write path avoids this by re-reading fresh trunk on every attempt including the first. Make commitAndPush re-fetch trunk, hard-reset the working tree to the upstream tip, and node-patch only the owned leaf back in before the first commit, so no attempt ever rests on a stale base. Gate it on the component-scoped path; the single-component path keeps its historical commit-then-rebase behavior byte-identical. Export RefetchAndReset for the shared re-sync. Add shared-parent convergence tests covering sequential orderings, a concurrent goroutine wave, a lane behind trunk, and the first-attempt stale-working-tree case that fails before this change. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
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.
Problem
A shared-path commit can fire two component orchestrate lanes concurrently. Each finalizes its own
state.components.<component>.<env>leaf into the one trunk manifest. The component write is re-appliable on a rejected push (it re-fetches trunk and node-patches only its own leaf, preserving siblings), but that guard only runs when the local branch is behind trunk.The first commit in
commitAndPushrested on the checkout-time manifest bytes. In the interleaving where the local HEAD is already at the fresh trunk tip but the working manifest is stale, the firstgit pushfast-forwards with no rejection, so the re-apply never runs and the commit writes back a stale sibling leaf, reverting a value another component already committed. Observed live: a shared-path wave reverted one component's dev version while advancing the other. The Contents-API finalizers (promote, hotfix, rollback) were already immune because they re-read fresh trunk on every attempt including the first; the git orchestrate path did not.Fix
internal/orchestrate/orchestrator.gocommitAndPush(component-scoped path only): re-fetch and reset to the trunk tip, then re-derive the owned leaf viawriteConfigbefore the first commit, so no attempt ever rests on a stale base. This mirrors the Contents-API path. The single-component path is unchanged and byte-identical (it writes the whole state node and has no sibling to converge against). Exposedgit.RefetchAndResetfor reuse; the retry ceiling and thecascade-state-write: attempt=N/Minstrumentation are unchanged.Verification
Regression test
TestSharedParent_FirstAttemptReDerivesOntoFreshTrunk(red before, green after): two components finalizing off one shared parent, assert both leaves survive. Plus a 5-lane concurrent goroutine wave and both sequential orderings.go build ./...,go test ./...(2706 pass),go test ./... -race,golangci-lint run ./...all clean; single-component byte-identical baseline passes.Caught by the v1-readiness fleet's shared-path concurrency wave.