From ee432d25071e3544b0c40eb20a1914237039aa1c Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Thu, 25 Jun 2026 09:36:35 -0400 Subject: [PATCH 1/2] fix(orchestrate): skip unchanged builds via build-state base ladder Signed-off-by: Joshua Temple --- e2e/scenarios/06-no-change-skip.yaml | 59 ++++++ internal/orchestrate/nochange_skip_test.go | 197 +++++++++++++++++++++ internal/orchestrate/orchestrator.go | 60 ++++++- 3 files changed, 308 insertions(+), 8 deletions(-) create mode 100644 e2e/scenarios/06-no-change-skip.yaml create mode 100644 internal/orchestrate/nochange_skip_test.go diff --git a/e2e/scenarios/06-no-change-skip.yaml b/e2e/scenarios/06-no-change-skip.yaml new file mode 100644 index 00000000..799f041a --- /dev/null +++ b/e2e/scenarios/06-no-change-skip.yaml @@ -0,0 +1,59 @@ +name: "No Change Skip" +description: "Re-orchestrating with no intervening commit skips the build instead of re-running it" + +config: + trunk_branch: main + environments: [] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: [] + +steps: + # Step 1: Push a src commit to trunk -> build runs, draft release v0.1.0-rc.0 + - name: "Initial feature commit" + action: commit + commit: + message: "feat: add initial feature" + files: + src/app.go: | + package main + func main() { + println("Hello v0.1.0") + } + + - name: "Orchestrate after first commit" + action: orchestrate + expect: + state: + prerelease: + sha: commit1 + version: "v0.1.0-rc.0" + jobs: + build-app: success + releases: + - tag: "v0.1.0-rc.0" + prerelease: true + draft: true + tags: + exist: ["v0.1.0-rc.0"] + + # Step 2: Re-orchestrate with NO new source commit. The only thing that moved + # HEAD since step 1 is cascade's own "chore: update state" finalize commit, + # which touches .github/manifest.yaml and never matches the src/** trigger. + # Change detection now anchors on the build's recorded SHA (and the env-level + # SHA), so the build has nothing new to do and must be SKIPPED rather than + # re-running on every dispatch. + # + # The prerelease SHA and version are intentionally not asserted here: the + # finalize state commit advances HEAD and the rc number recomputes, both of + # which are orthogonal to the no-change-skip fix. The rc tag from step 1 still + # exists, and build-app: skipped is the load-bearing assertion. + - name: "Orchestrate again with no new commit" + action: orchestrate + expect: + jobs: + build-app: skipped + tags: + exist: ["v0.1.0-rc.0"] diff --git a/internal/orchestrate/nochange_skip_test.go b/internal/orchestrate/nochange_skip_test.go new file mode 100644 index 00000000..f5d606e9 --- /dev/null +++ b/internal/orchestrate/nochange_skip_test.go @@ -0,0 +1,197 @@ +package orchestrate + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stablekernel/cascade/internal/config" +) + +// gitEnv returns a deterministic git identity so commits work in CI runners +// that have no global git config. +func gitEnv() []string { + return append(os.Environ(), + "GIT_AUTHOR_NAME=cascade-test", + "GIT_AUTHOR_EMAIL=cascade-test@example.com", + "GIT_COMMITTER_NAME=cascade-test", + "GIT_COMMITTER_EMAIL=cascade-test@example.com", + ) +} + +// runGit runs a git command in dir and fails the test on error. +func runGit(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = gitEnv() + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, out) + } + return strings.TrimSpace(string(out)) +} + +// writeFile writes a file under dir, creating parent directories. +func writeFile(t *testing.T, dir, rel, content string) { + t.Helper() + full := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir for %s: %v", rel, err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", rel, err) + } +} + +// initRepo creates a real git repo with an initial non-src commit (so HEAD~1 +// exists) followed by a src/** commit (HEAD). It returns the repo root and the +// HEAD SHA of the src commit. +func initRepo(t *testing.T) (repoDir, headSHA string) { + t.Helper() + repoDir = t.TempDir() + runGit(t, repoDir, "init", "-b", "main") + + // Initial, non-src commit so HEAD~1 exists. + writeFile(t, repoDir, "README.md", "# fixture\n") + runGit(t, repoDir, "add", "README.md") + runGit(t, repoDir, "commit", "-m", "chore: init") + + // A src/** commit becomes HEAD. + writeFile(t, repoDir, "src/app.go", "package main\n\nfunc main() {}\n") + runGit(t, repoDir, "add", "src/app.go") + runGit(t, repoDir, "commit", "-m", "feat: add app") + + headSHA = runGit(t, repoDir, "rev-parse", "HEAD") + return repoDir, headSHA +} + +// writeManifest writes a no-environment manifest under /.github/manifest.yaml +// with a single build on src/** triggers and records prerelease state at the +// given SHA. It returns the manifest path. +func writeManifest(t *testing.T, repoDir, stateSHA string) string { + t.Helper() + manifestPath := filepath.Join(repoDir, ".github", "manifest.yaml") + manifest := `ci: + config: + project: nochange-test + trunk_branch: main + environments: [] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: [] + state: + prerelease: + sha: ` + stateSHA + ` + version: v0.1.0-rc.0 +` + writeFile(t, repoDir, ".github/manifest.yaml", manifest) + return manifestPath +} + +// TestSetup_NoChangeReDispatch_SkipsBuild verifies that when HEAD is unchanged +// since the last orchestrate (envState.SHA == HEAD), a build with no dependent +// deploy is SKIPPED rather than re-run. This is the no-change-skip bug: the old +// base ladder fell back to HEAD~1, which still contains the last src commit, so +// the build re-ran on every dispatch. +func TestSetup_NoChangeReDispatch_SkipsBuild(t *testing.T) { + repoDir, headSHA := initRepo(t) + + // State records the build was already produced at HEAD (env-level SHA). + manifestPath := writeManifest(t, repoDir, headSHA) + + orch, err := NewOrchestrator(manifestPath, "ci", "prerelease") + if err != nil { + t.Fatalf("NewOrchestrator: %v", err) + } + + res, err := orch.Setup(headSHA) + if err != nil { + t.Fatalf("Setup: %v", err) + } + + if res.RunBuilds["app"] { + t.Errorf("RunBuilds[app] = true, want false (no-change re-dispatch should skip the build)") + } +} + +// TestSetup_NewCommit_RunsBuild is the positive counterpart: once HEAD advances +// past the recorded state SHA with a src change, the build must run again. +func TestSetup_NewCommit_RunsBuild(t *testing.T) { + repoDir, oldHead := initRepo(t) + + // State still points at the previous HEAD. + manifestPath := writeManifest(t, repoDir, oldHead) + + // Advance HEAD with a new src commit. + writeFile(t, repoDir, "src/feature.go", "package main\n\nfunc Feature() {}\n") + runGit(t, repoDir, "add", "src/feature.go") + runGit(t, repoDir, "commit", "-m", "feat: add feature") + newHead := runGit(t, repoDir, "rev-parse", "HEAD") + + orch, err := NewOrchestrator(manifestPath, "ci", "prerelease") + if err != nil { + t.Fatalf("NewOrchestrator: %v", err) + } + + res, err := orch.Setup(newHead) + if err != nil { + t.Fatalf("Setup: %v", err) + } + + if !res.RunBuilds["app"] { + t.Errorf("RunBuilds[app] = false, want true (a new src commit must re-run the build)") + } +} + +// TestFinalize_RecordsPerBuildSHA verifies that a successful build's SHA is +// recorded into envState.Builds so the build base ladder can consult it on the +// next dispatch. +func TestFinalize_RecordsPerBuildSHA(t *testing.T) { + repoDir, headSHA := initRepo(t) + manifestPath := writeManifest(t, repoDir, "") + + // Finalize commits and pushes the updated manifest. Wire up a bare remote + // and an upstream so the push has a destination in this hermetic fixture. + remoteDir := t.TempDir() + runGit(t, remoteDir, "init", "--bare", "-b", "main") + runGit(t, repoDir, "remote", "add", "origin", remoteDir) + runGit(t, repoDir, "push", "-u", "origin", "main") + + orch, err := NewOrchestrator(manifestPath, "ci", "prerelease") + if err != nil { + t.Fatalf("NewOrchestrator: %v", err) + } + + // Run Finalize in dry-run-free mode but avoid the commit/push by checking + // state in-memory before the write side effects matter. Finalize writes the + // config file and attempts a commit; in a fresh repo with the manifest + // staged this still records state in memory, which is what we assert. + err = orch.Finalize(headSHA, "v0.1.0-rc.0", + map[string]string{}, + map[string]string{"app": "success"}, + ) + if err != nil { + t.Fatalf("Finalize: %v", err) + } + + envState := orch.cicdFile.State["prerelease"] + if envState == nil { + t.Fatalf("prerelease state is nil after Finalize") + } + bs := envState.Builds["app"] + if bs == nil { + t.Fatalf("envState.Builds[app] is nil; expected a recorded build state") + } + if bs.SHA != headSHA { + t.Errorf("envState.Builds[app].SHA = %q, want %q", bs.SHA, headSHA) + } + if bs.BuiltAt == "" { + t.Errorf("envState.Builds[app].BuiltAt is empty; expected a timestamp") + } + _ = config.BuildState{} // keep config import meaningful if assertions change +} diff --git a/internal/orchestrate/orchestrator.go b/internal/orchestrate/orchestrator.go index 3b2291de..c77fcecd 100644 --- a/internal/orchestrate/orchestrator.go +++ b/internal/orchestrate/orchestrator.go @@ -189,6 +189,28 @@ func (o *Orchestrator) Finalize(headSHA, version string, deployResults, buildRes } } + // Update per-build state for successful builds. Recording the build SHA here + // is what lets the next dispatch's base-SHA ladder anchor change detection on + // the last successful build, so an unchanged HEAD skips the build instead of + // re-running it. + if envState.Builds == nil { + envState.Builds = make(map[string]*config.BuildState) + } + + for name, result := range buildResults { + if result == "success" { + if envState.Builds[name] == nil { + envState.Builds[name] = &config.BuildState{} + } + envState.Builds[name].SHA = headSHA + envState.Builds[name].BuiltAt = timestamp + envState.Builds[name].BuiltBy = actor + log.Info("Updated %s.builds.%s state", o.environment, name) + } else { + log.Debug("Skipping %s build state update (result=%s)", name, result) + } + } + // Write updated config if err := o.writeConfig(); err != nil { return fmt.Errorf("failed to write config: %w", err) @@ -213,20 +235,42 @@ func (o *Orchestrator) calculateBaseSHAs(envState *config.EnvState) map[string]s defaultBase, _ = o.gitOutput("rev-list", "--max-parents=0", "HEAD") } - // Set base SHAs from per-deployable state + // Set base SHAs from per-deployable state. The build base-SHA ladder is, in + // priority order: + // + // 1. The build's own recorded state SHA (envState.Builds[name].SHA). This is + // the precise "last time this build ran" anchor, recorded by Finalize. + // 2. The dependent deploy's state SHA. Preserves the original behavior for + // builds whose artifact is carried forward by a deploy that depends on it. + // 3. The env-level last-orchestrated SHA (envState.SHA). This is the fallback + // that fixes no-environment / no-dependent-deploy builds: after the first + // orchestrate at HEAD, envState.SHA == HEAD, so a no-change re-dispatch + // diffs HEAD..HEAD (empty) and the build is skipped instead of re-running. + // 4. defaultBase (HEAD~1, or the initial commit) as the first-run fallback. for _, build := range o.cicdFile.Config.Builds { key := "build_" + build.Name - if envState != nil && envState.Deploys != nil { - // For builds, use the deploy state of the dependent deploy - for _, deploy := range o.cicdFile.Config.Deploys { - if contains(deploy.DependsOn, build.Name) { - if ds := envState.Deploys[deploy.Name]; ds != nil && ds.SHA != "" { - baseSHAs[key] = ds.SHA - break + if envState != nil { + // 1. The build's own recorded state SHA. + if bs := envState.Builds[build.Name]; bs != nil && bs.SHA != "" { + baseSHAs[key] = bs.SHA + } + // 2. The dependent deploy's state SHA. + if baseSHAs[key] == "" && envState.Deploys != nil { + for _, deploy := range o.cicdFile.Config.Deploys { + if contains(deploy.DependsOn, build.Name) { + if ds := envState.Deploys[deploy.Name]; ds != nil && ds.SHA != "" { + baseSHAs[key] = ds.SHA + break + } } } } + // 3. The env-level last-orchestrated SHA. + if baseSHAs[key] == "" && envState.SHA != "" { + baseSHAs[key] = envState.SHA + } } + // 4. First-run fallback. if baseSHAs[key] == "" { baseSHAs[key] = defaultBase } From 6bf6007596535bff3f8deb302eb6cd3bef4c2f0f Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Thu, 25 Jun 2026 09:44:23 -0400 Subject: [PATCH 2/2] test(orchestrate): add empirical proof tests for no-change-skip bug and masking condition Signed-off-by: Joshua Temple --- internal/orchestrate/nochange_proof_test.go | 111 ++++++++++++++++++ .../orchestrate/nochange_reconcile_test.go | 67 +++++++++++ 2 files changed, 178 insertions(+) create mode 100644 internal/orchestrate/nochange_proof_test.go create mode 100644 internal/orchestrate/nochange_reconcile_test.go diff --git a/internal/orchestrate/nochange_proof_test.go b/internal/orchestrate/nochange_proof_test.go new file mode 100644 index 00000000..421ce2dd --- /dev/null +++ b/internal/orchestrate/nochange_proof_test.go @@ -0,0 +1,111 @@ +package orchestrate + +import ( + "testing" + + "github.com/stablekernel/cascade/internal/config" +) + +// unfixedBuildBaseSHA reproduces the PRE-FIX build base-SHA ladder verbatim, as +// it existed before commit ee432d2 (see `git diff HEAD~1`). The old ladder had +// exactly two rungs for a build: (1) the dependent deploy's state SHA, then +// (2) defaultBase (HEAD~1). There was no per-build state lookup and no +// env-level envState.SHA fallback. This function lets us run the unfixed +// algorithm against the same real git fixture the fixed code runs against, +// proving empirically whether the no-change-skip bug was real. +func (o *Orchestrator) unfixedBuildBaseSHA(envState *config.EnvState, buildName string) string { + defaultBase, _ := o.gitOutput("rev-parse", "HEAD~1") + if defaultBase == "" { + defaultBase, _ = o.gitOutput("rev-list", "--max-parents=0", "HEAD") + } + + base := "" + if envState != nil && envState.Deploys != nil { + for _, deploy := range o.cicdFile.Config.Deploys { + if contains(deploy.DependsOn, buildName) { + if ds := envState.Deploys[deploy.Name]; ds != nil && ds.SHA != "" { + base = ds.SHA + break + } + } + } + } + if base == "" { + base = defaultBase + } + return base +} + +// TestNoChangeReDispatch_ProvesUnfixedBugAndFix runs BOTH algorithms against one +// real git fixture representing a genuine no-change re-dispatch: +// +// - Repo: HEAD~1 = chore:init (no src), HEAD = feat:add app (src/app.go). +// - State: prerelease.sha == HEAD (the env was already orchestrated at HEAD). +// - There is NO dependent deploy (deploys: []), and NO prior finalize state +// commit sits at HEAD: HEAD is the src commit itself. +// +// This is the exact "same-HEAD no-change dispatch with no intervening state +// commit" scenario. We assert the OBSERVED values: +// +// UNFIXED: base = HEAD~1 (chore:init) -> diff HEAD~1..HEAD touches src/app.go +// -> detectChanges == true -> build RE-RUNS (THE BUG) +// FIXED: base = envState.SHA == HEAD -> diff HEAD..HEAD empty +// -> detectChanges == false -> build SKIPS (THE FIX) +func TestNoChangeReDispatch_ProvesUnfixedBugAndFix(t *testing.T) { + repoDir, headSHA := initRepo(t) + manifestPath := writeManifest(t, repoDir, headSHA) // state.sha == HEAD + + orch, err := NewOrchestrator(manifestPath, "ci", "prerelease") + if err != nil { + t.Fatalf("NewOrchestrator: %v", err) + } + + envState := orch.cicdFile.State["prerelease"] + if envState == nil || envState.SHA != headSHA { + t.Fatalf("fixture invariant broken: envState.SHA=%q want %q", func() string { + if envState == nil { + return "" + } + return envState.SHA + }(), headSHA) + } + + // --- Sanity: confirm HEAD~1 carries no src/** but HEAD~1..HEAD does. --- + headParent, _ := orch.gitOutput("rev-parse", "HEAD~1") + t.Logf("OBSERVED headSHA = %s", headSHA) + t.Logf("OBSERVED HEAD~1 = %s", headParent) + t.Logf("OBSERVED envState.SHA = %s", envState.SHA) + + triggers := orch.cicdFile.Config.Builds[0].Triggers // ["src/**"] + + // --- UNFIXED algorithm --- + unfixedBase := orch.unfixedBuildBaseSHA(envState, "app") + unfixedRuns := orch.detectChanges(unfixedBase, headSHA, triggers) + t.Logf("UNFIXED base=%s detectChanges(base..HEAD)=%v", unfixedBase, unfixedRuns) + + if unfixedBase != headParent { + t.Errorf("UNFIXED base = %s, expected HEAD~1 (%s): the bug requires the ladder to fall to HEAD~1", unfixedBase, headParent) + } + if !unfixedRuns { + t.Errorf("UNFIXED RunBuilds[app] = false, expected TRUE: the unfixed ladder anchors on HEAD~1 which still contains the src commit, so the build re-runs on a no-change dispatch (the bug was NOT reproduced)") + } else { + t.Logf("PROVEN: unfixed code RE-RUNS the build on a genuine no-change dispatch (RunBuilds[app]=true)") + } + + // --- FIXED algorithm (the real Setup path) --- + res, err := orch.Setup(headSHA) + if err != nil { + t.Fatalf("Setup: %v", err) + } + fixedBase := res.BaseSHAs["build_app"] + t.Logf("FIXED base=%s RunBuilds[app]=%v", fixedBase, res.RunBuilds["app"]) + + if fixedBase != headSHA { + t.Errorf("FIXED base = %s, expected envState.SHA == HEAD (%s)", fixedBase, headSHA) + } + if res.RunBuilds["app"] { + t.Errorf("FIXED RunBuilds[app] = true, expected FALSE: the fix should skip the build on a no-change dispatch") + } else { + t.Logf("PROVEN: fixed code SKIPS the build on a genuine no-change dispatch (RunBuilds[app]=false)") + } +} diff --git a/internal/orchestrate/nochange_reconcile_test.go b/internal/orchestrate/nochange_reconcile_test.go new file mode 100644 index 00000000..024eb759 --- /dev/null +++ b/internal/orchestrate/nochange_reconcile_test.go @@ -0,0 +1,67 @@ +package orchestrate + +import ( + "path/filepath" + "testing" +) + +// TestReconcile_StateCommitAtHead_UnfixedAlsoSkips proves the masking case that +// explains why the bug was invisible in the normal harness/live-fleet flow. +// +// Scenario: a prior finalize HAS run and pushed a "chore: update state" commit, +// so the commit graph on the next dispatch is: +// +// HEAD = chore: update state (manifest.yaml only, no src/**) +// HEAD~1 = feat: add app (src/app.go) +// +// dispatch fires at GITHUB_SHA == HEAD (the state commit). With the UNFIXED +// ladder, base = HEAD~1 = the src commit, and diff(HEAD~1..HEAD) touches only +// manifest.yaml, which does NOT match src/** -> build SKIPS even without the +// fix. This is why a same-graph re-dispatch through finalize never reproduced +// the bug: the intervening state commit at HEAD masks it. +// +// Therefore the live-fleet failure ("a no-change orchestrate cut a prerelease +// and Build(app) succeeded") can only be a dispatch where NO state commit sits +// at HEAD - i.e. GITHUB_SHA was the src commit itself (first-ever dispatch, a +// failed/never-run prior finalize, or a manual re-dispatch at the src SHA). +func TestReconcile_StateCommitAtHead_UnfixedAlsoSkips(t *testing.T) { + repoDir, srcHead := initRepo(t) // HEAD = feat:add app (src/app.go) + + // Simulate a prior finalize: write the manifest with state.sha = srcHead and + // commit it as a manifest-only "chore: update state" commit. That commit is + // now HEAD; the src commit becomes HEAD~1. + writeManifest(t, repoDir, srcHead) + runGit(t, repoDir, "add", ".github/manifest.yaml") + runGit(t, repoDir, "commit", "-m", "chore: update state for prerelease [skip ci]") + stateHead := runGit(t, repoDir, "rev-parse", "HEAD") + + manifestPath := filepath.Join(repoDir, ".github", "manifest.yaml") + orch, err := NewOrchestrator(manifestPath, "ci", "prerelease") + if err != nil { + t.Fatalf("NewOrchestrator: %v", err) + } + envState := orch.cicdFile.State["prerelease"] + triggers := orch.cicdFile.Config.Builds[0].Triggers + + headParent, _ := orch.gitOutput("rev-parse", "HEAD~1") + t.Logf("OBSERVED stateHead (HEAD) = %s", stateHead) + t.Logf("OBSERVED HEAD~1 (srcHead) = %s", headParent) + + // Confirm HEAD is manifest-only. + changed, _ := orch.gitOutput("diff", "--name-only", headParent, stateHead) + t.Logf("OBSERVED diff(HEAD~1..HEAD) files = %q", changed) + + // UNFIXED ladder: base = HEAD~1 = srcHead. Dispatch at GITHUB_SHA = stateHead. + unfixedBase := orch.unfixedBuildBaseSHA(envState, "app") + unfixedRuns := orch.detectChanges(unfixedBase, stateHead, triggers) + t.Logf("UNFIXED base=%s detectChanges(base..stateHead)=%v", unfixedBase, unfixedRuns) + + if unfixedBase != headParent { + t.Errorf("UNFIXED base = %s, want HEAD~1 (%s)", unfixedBase, headParent) + } + if unfixedRuns { + t.Errorf("UNFIXED RunBuilds[app] = true; expected FALSE: with a state commit at HEAD the masking should make even the unfixed code skip") + } else { + t.Logf("RECONCILED: with a state commit at HEAD, the UNFIXED code ALSO skips (diff is manifest-only). The bug is masked whenever finalize pushed a state commit before re-dispatch.") + } +}