diff --git a/internal/git/git.go b/internal/git/git.go index 1aab0897..3b993d27 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -336,6 +336,15 @@ func backoffForAttempt(base time.Duration, attempt int) time.Duration { return d + time.Duration(rand.Int64N(int64(base)+1)) } +// RefetchAndReset re-fetches trunk and hard-resets the working tree to the +// upstream tracking tip, so the working manifest holds the fresh trunk bytes +// (including any concurrent sibling leaf). It is exported for a state writer that +// wants to re-derive its owned leaf onto fresh trunk before its first commit, so +// that write never rests on a stale checkout base rather than relying solely on a +// push rejection to surface the staleness. It is the same operation the push +// retry loop performs on a rejected push. +func RefetchAndReset(dir string) error { return refetchAndReset(dir) } + // refetchAndReset re-fetches trunk and hard-resets the working tree to the // upstream tracking tip, dropping any local commit whose bytes were derived from // a stale trunk. It is the pre-step of a re-apply retry: after it returns the diff --git a/internal/orchestrate/orchestrator.go b/internal/orchestrate/orchestrator.go index 25a80841..715aa2ed 100644 --- a/internal/orchestrate/orchestrator.go +++ b/internal/orchestrate/orchestrator.go @@ -685,6 +685,26 @@ func (o *Orchestrator) componentStateWrites() []config.StateWrite { // commitAndPush commits and pushes state changes. func (o *Orchestrator) commitAndPush(version string) error { + // For a component-scoped run, re-derive the owned leaf onto the CURRENT trunk + // manifest before the first commit: re-fetch trunk, hard-reset the working tree + // to the upstream tip, and node-patch only state.components.. + // back in through WriteScopedState. This makes the very first attempt rest on + // fresh trunk bytes rather than the checkout-time base, so a concurrent + // sibling's leaf is preserved even when the push then fast-forwards with no + // rejection to trigger the retry-loop re-apply. It mirrors the Contents-API + // write path, which re-reads fresh trunk on every attempt including the first. + // The single-component path (component == "") keeps its historical behavior + // byte-identical: it commits the checkout-base bytes and rebases only on a + // rejected push. + if o.component != "" { + if err := git.RefetchAndReset(o.baseDir); err != nil { + return err + } + if err := o.writeConfig(); err != nil { + return err + } + } + // Check if there are changes to commit status, _ := o.gitOutput("status", "--porcelain", o.configPath) if strings.TrimSpace(status) == "" { diff --git a/internal/orchestrate/shared_parent_clobber_test.go b/internal/orchestrate/shared_parent_clobber_test.go new file mode 100644 index 00000000..66b8170a --- /dev/null +++ b/internal/orchestrate/shared_parent_clobber_test.go @@ -0,0 +1,284 @@ +package orchestrate + +import ( + "fmt" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/stablekernel/cascade/internal/config" +) + +// seedSharedParentTwoComponents builds a bare remote seeded at ONE shared parent +// commit that already carries BOTH component leaves (api + web) at v0.1.0, then +// returns two working clones checked out at that shared parent. This models the +// shared_paths wave: one commit fires two component orchestrate lanes, each of +// which finalizes its own leaf into the single shared manifest on trunk. +func seedSharedParentTwoComponents(t *testing.T) (remote, cloneAPI, cloneWEB, statePathAPI, statePathWEB string) { + t.Helper() + + remote = t.TempDir() + runGit(t, remote, "init", "--bare", "-b", "main") + + const seed = "ci:\n" + + " state:\n" + + " components:\n" + + " api:\n" + + " dev:\n" + + " version: api-0.1.0-rc.0\n" + + " sha: aaaa000\n" + + " web:\n" + + " dev:\n" + + " version: web-0.1.0-rc.0\n" + + " sha: wwww000\n" + + seeder := t.TempDir() + runGit(t, seeder, "clone", remote, ".") + runGit(t, seeder, "checkout", "-b", "main") + writeFile(t, seeder, ".github/manifest.yaml", seed) + runGit(t, seeder, "add", ".github/manifest.yaml") + runGit(t, seeder, "commit", "-m", "chore: seed shared parent") + runGit(t, seeder, "push", "-u", "origin", "main") + + cloneAPI = t.TempDir() + runGit(t, cloneAPI, "clone", remote, ".") + statePathAPI = filepath.Join(cloneAPI, ".github", "manifest.yaml") + + cloneWEB = t.TempDir() + runGit(t, cloneWEB, "clone", remote, ".") + statePathWEB = filepath.Join(cloneWEB, ".github", "manifest.yaml") + + return remote, cloneAPI, cloneWEB, statePathAPI, statePathWEB +} + +func newComponentOrchestrator(statePath, component, baseDir, version string) *Orchestrator { + return &Orchestrator{ + configPath: statePath, + environment: "dev", + component: component, + baseDir: baseDir, + pushBackoff: time.Millisecond, + cicdFile: &config.CICDFile{ + State: map[string]*config.EnvState{ + "dev": {Version: version, SHA: component + "-new"}, + }, + }, + } +} + +// finalizeLane mirrors Finalize's write-then-commit-and-push sequence for one +// component lane. +func finalizeLane(t *testing.T, o *Orchestrator) { + t.Helper() + if err := o.writeConfig(); err != nil { + t.Fatalf("%s writeConfig: %v", o.component, err) + } + if err := o.commitAndPush(o.cicdFile.State["dev"].Version); err != nil { + t.Fatalf("%s commitAndPush: %v", o.component, err) + } +} + +func assertBothLeavesSurvive(t *testing.T, clone string, wantAPI, wantWEB string) { + t.Helper() + runGit(t, clone, "fetch", "origin", "main") + got := runGit(t, clone, "show", "origin/main:.github/manifest.yaml") + for _, want := range []string{wantAPI, wantWEB} { + if !strings.Contains(got, want) { + t.Errorf("converged trunk manifest missing %q; got:\n%s", want, got) + } + } +} + +// TestSharedParent_SequentialLanes_ApiFirst: both lanes writeConfig on the shared +// parent, then api pushes first (ff) and web pushes second (reject -> reapply). +func TestSharedParent_SequentialLanes_ApiFirst(t *testing.T) { + _, cloneAPI, cloneWEB, spAPI, spWEB := seedSharedParentTwoComponents(t) + + api := newComponentOrchestrator(spAPI, "api", cloneAPI, "api-0.2.0-rc.0") + web := newComponentOrchestrator(spWEB, "web", cloneWEB, "web-0.2.0-rc.0") + + // Both derive their leaf against the shared-parent bytes BEFORE either pushes. + if err := api.writeConfig(); err != nil { + t.Fatalf("api writeConfig: %v", err) + } + if err := web.writeConfig(); err != nil { + t.Fatalf("web writeConfig: %v", err) + } + if err := api.commitAndPush("api-0.2.0-rc.0"); err != nil { + t.Fatalf("api commitAndPush: %v", err) + } + if err := web.commitAndPush("web-0.2.0-rc.0"); err != nil { + t.Fatalf("web commitAndPush: %v", err) + } + + assertBothLeavesSurvive(t, cloneAPI, "api-0.2.0-rc.0", "web-0.2.0-rc.0") +} + +// TestSharedParent_SequentialLanes_WebFirst is the reverse ordering. +func TestSharedParent_SequentialLanes_WebFirst(t *testing.T) { + _, cloneAPI, cloneWEB, spAPI, spWEB := seedSharedParentTwoComponents(t) + + api := newComponentOrchestrator(spAPI, "api", cloneAPI, "api-0.2.0-rc.0") + web := newComponentOrchestrator(spWEB, "web", cloneWEB, "web-0.2.0-rc.0") + + if err := web.writeConfig(); err != nil { + t.Fatalf("web writeConfig: %v", err) + } + if err := api.writeConfig(); err != nil { + t.Fatalf("api writeConfig: %v", err) + } + if err := web.commitAndPush("web-0.2.0-rc.0"); err != nil { + t.Fatalf("web commitAndPush: %v", err) + } + if err := api.commitAndPush("api-0.2.0-rc.0"); err != nil { + t.Fatalf("api commitAndPush: %v", err) + } + + assertBothLeavesSurvive(t, cloneAPI, "api-0.2.0-rc.0", "web-0.2.0-rc.0") +} + +// TestSharedParent_FullLane_ApiFirst drives the full write+push per lane in +// strict sequence (api lane fully finalizes, then web lane fully finalizes), +// matching the empirical "two sequential state commits" observation. +func TestSharedParent_FullLane_ApiFirst(t *testing.T) { + _, cloneAPI, cloneWEB, spAPI, spWEB := seedSharedParentTwoComponents(t) + + api := newComponentOrchestrator(spAPI, "api", cloneAPI, "api-0.2.0-rc.0") + web := newComponentOrchestrator(spWEB, "web", cloneWEB, "web-0.2.0-rc.0") + + finalizeLane(t, api) + finalizeLane(t, web) + + assertBothLeavesSurvive(t, cloneAPI, "api-0.2.0-rc.0", "web-0.2.0-rc.0") +} + +// TestSharedParent_ConcurrentWave races N component lanes finalizing off one +// shared parent through real goroutines, mirroring a monorepo's shared_paths +// wave. Every owned leaf must survive on trunk regardless of the OS scheduler's +// interleaving. +func TestSharedParent_ConcurrentWave(t *testing.T) { + remote := t.TempDir() + runGit(t, remote, "init", "--bare", "-b", "main") + + comps := []string{"api", "web", "worker", "cron", "gateway"} + + // Seed a shared parent carrying every component's old leaf. + var seed strings.Builder + seed.WriteString("ci:\n state:\n components:\n") + for _, c := range comps { + fmt.Fprintf(&seed, " %s:\n dev:\n version: %s-0.1.0-rc.0\n sha: %s000\n", c, c, c) + } + seeder := t.TempDir() + runGit(t, seeder, "clone", remote, ".") + runGit(t, seeder, "checkout", "-b", "main") + writeFile(t, seeder, ".github/manifest.yaml", seed.String()) + runGit(t, seeder, "add", ".github/manifest.yaml") + runGit(t, seeder, "commit", "-m", "chore: seed shared parent") + runGit(t, seeder, "push", "-u", "origin", "main") + + var wg sync.WaitGroup + errs := make(chan error, len(comps)) + for _, c := range comps { + clone := t.TempDir() + runGit(t, clone, "clone", remote, ".") + sp := filepath.Join(clone, ".github", "manifest.yaml") + o := newComponentOrchestrator(sp, c, clone, c+"-0.2.0-rc.0") + wg.Add(1) + go func(o *Orchestrator) { + defer wg.Done() + if err := o.writeConfig(); err != nil { + errs <- err + return + } + if err := o.commitAndPush(o.cicdFile.State["dev"].Version); err != nil { + errs <- err + } + }(o) + } + wg.Wait() + close(errs) + for err := range errs { + if err != nil { + t.Fatalf("lane failed: %v", err) + } + } + + verify := t.TempDir() + runGit(t, verify, "clone", remote, ".") + got := runGit(t, verify, "show", "origin/main:.github/manifest.yaml") + for _, c := range comps { + want := c + "-0.2.0-rc.0" + if !strings.Contains(got, want) { + t.Errorf("converged trunk missing %q (clobbered); got:\n%s", want, got) + } + } +} + +// TestSharedParent_LaneBehindTrunk models a lane whose local checkout is behind +// trunk: the sibling already landed its NEW leaf on trunk before this lane +// pushes. The lane's first push must reject and the re-apply must preserve the +// sibling's new value rather than reverting it to the checkout-time value. +func TestSharedParent_LaneBehindTrunk(t *testing.T) { + remote, cloneAPI, cloneWEB, spAPI, spWEB := seedSharedParentTwoComponents(t) + _ = remote + + // api lands its NEW leaf on trunk first (fully finalizes). + api := newComponentOrchestrator(spAPI, "api", cloneAPI, "api-0.2.0-rc.0") + finalizeLane(t, api) + + // web's clone is still at the shared parent (behind trunk): its on-disk + // manifest shows api-0.1.0. It derives its leaf against that stale base. + web := newComponentOrchestrator(spWEB, "web", cloneWEB, "web-0.2.0-rc.0") + finalizeLane(t, web) + + // api's NEW leaf must NOT have been reverted by web's finalize. + assertBothLeavesSurvive(t, cloneWEB, "api-0.2.0-rc.0", "web-0.2.0-rc.0") + got := runGit(t, cloneWEB, "show", "origin/main:.github/manifest.yaml") + if strings.Contains(got, "api-0.1.0-rc.0") { + t.Errorf("web finalize reverted api to the stale checkout value; got:\n%s", got) + } +} + +// TestSharedParent_FirstAttemptReDerivesOntoFreshTrunk is the direct regression +// guard for the first-attempt clobber: a lane whose working manifest is stale +// (still shows the sibling's OLD value) while its local HEAD already sits at the +// trunk tip. A plain first push then fast-forwards with NO rejection, so the +// rejection-driven re-apply never runs and the stale sibling value lands, +// reverting the sibling. commitAndPush must re-derive the owned leaf onto fresh +// trunk before the first commit so the sibling survives. Fails before the fix, +// passes after. +func TestSharedParent_FirstAttemptReDerivesOntoFreshTrunk(t *testing.T) { + _, cloneAPI, cloneWEB, spAPI, spWEB := seedSharedParentTwoComponents(t) + + // api fully finalizes first: trunk is now api-0.2.0, web-0.1.0. + api := newComponentOrchestrator(spAPI, "api", cloneAPI, "api-0.2.0-rc.0") + finalizeLane(t, api) + + // Sync web's clone so its local HEAD is the trunk tip (a plain push here would + // fast-forward), then leave a stale working manifest showing api's OLD value. + // This is the state a rejection can never surface. + runGit(t, cloneWEB, "fetch", "origin", "main") + runGit(t, cloneWEB, "reset", "--hard", "origin/main") + stale := "ci:\n state:\n components:\n" + + " api:\n dev:\n version: api-0.1.0-rc.0\n sha: aaaa000\n" + + " web:\n dev:\n version: web-0.2.0-rc.0\n sha: web-new\n" + writeFile(t, cloneWEB, ".github/manifest.yaml", stale) + + web := newComponentOrchestrator(spWEB, "web", cloneWEB, "web-0.2.0-rc.0") + if err := web.commitAndPush("web-0.2.0-rc.0"); err != nil { + t.Fatalf("web commitAndPush: %v", err) + } + + runGit(t, cloneWEB, "fetch", "origin", "main") + got := runGit(t, cloneWEB, "show", "origin/main:.github/manifest.yaml") + if strings.Contains(got, "api-0.1.0-rc.0") { + t.Fatalf("first-attempt clobber: api reverted to 0.1.0 with no reject to catch it:\n%s", got) + } + for _, want := range []string{"api-0.2.0-rc.0", "web-0.2.0-rc.0"} { + if !strings.Contains(got, want) { + t.Errorf("converged trunk missing %q:\n%s", want, got) + } + } +}