From 89f29ced09ec65ca56783e64f06d4dd8ec48499a Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 24 Jun 2026 12:59:26 -0400 Subject: [PATCH 1/3] fix(e2e): make hotfix-conflict scenario anchor deterministic executeHotfixApply anchored env/ at the recorded state SHA and silently fell back to trunk HEAD when that SHA was momentarily empty under a gitea state-sync race. The fallback seeded the env branch at the just-patched tip, turning the scenario's engineered cherry-pick conflict into an empty clean apply and flipping the resulting PR label run-to-run. resolveEnvAnchor now resolves an explicit base_ref then the recorded SHA, and errors loudly instead of guessing; the conflict scenario pins base_ref and asserts the conflict label it actually produces. Signed-off-by: Joshua Temple --- e2e/harness/hotfix_actions.go | 37 +++++++++-- e2e/harness/hotfix_actions_test.go | 65 +++++++++++++++++++ e2e/harness/multistep.go | 9 +++ .../hotfix/hotfix-conflict-resolution.yaml | 32 +++++---- 4 files changed, 124 insertions(+), 19 deletions(-) diff --git a/e2e/harness/hotfix_actions.go b/e2e/harness/hotfix_actions.go index 8c95ef1d..e96c8aa7 100644 --- a/e2e/harness/hotfix_actions.go +++ b/e2e/harness/hotfix_actions.go @@ -135,6 +135,32 @@ func (r *Runner) executeHotfixPlan(ctx context.Context, step *HotfixPlanStep) er return nil } +// resolveEnvAnchor determines the commit env/ must be created at when it +// does not yet exist. The anchor's tree content fully determines whether a later +// cherry-pick applies cleanly or conflicts, so it is resolved deterministically: +// +// 1. an explicit baseRef (resolved via the execution context, falling back to +// literal) when the scenario pins the base, then +// 2. the env's recorded state SHA. +// +// A silent fallback to trunk HEAD is deliberately NOT used. When the recorded +// SHA is momentarily empty (a gitea state-propagation race), anchoring on trunk +// HEAD would seed env/ with the just-patched tip, turning an engineered +// conflict into an empty (clean) cherry-pick and flipping the resulting PR label +// run-to-run. An empty resolution returns an error so the race surfaces loudly +// instead of being masked by a non-deterministic anchor. +func (r *Runner) resolveEnvAnchor(env, baseRef string) (string, error) { + if baseRef != "" { + if anchor := r.resolveCommit(baseRef); anchor != "" { + return anchor, nil + } + } + if anchor := r.ctx.GetState(env).SHA; anchor != "" { + return anchor, nil + } + return "", fmt.Errorf("hotfix_apply: cannot anchor env/%s: no base_ref given and recorded state SHA for %q is empty (likely a gitea state sync race); pin the scenario step's base_ref to make the cherry-pick outcome deterministic", env, env) +} + // executeHotfixApply performs a harness-driven cherry-pick of a trunk commit onto // env/, pushing a hotfix branch and opening a labeled PR. It mirrors the // product workflow's apply recipe (internal/generate/hotfix.go) but runs the git @@ -159,18 +185,15 @@ func (r *Runner) executeHotfixApply(ctx context.Context, step *HotfixApplyStep) hotfixBranch := "hotfix/" + env + "/" + short r.t.Logf(" HotfixApply: commits=%s env=%s branch=%s", commitList, env, hotfixBranch) - // Ensure env/ exists, anchored at the env's recorded state SHA (or HEAD). + // Ensure env/ exists, anchored at the env's true base (resolveEnvAnchor). branches, err := r.harness.gitea.ListBranches(ctx, r.harness.repo) if err != nil { return fmt.Errorf("list branches: %w", err) } if !containsString(branches, envBranch) { - anchor := r.ctx.GetState(env).SHA - if anchor == "" { - anchor, err = r.harness.gitea.getHeadSHA(ctx, r.harness.repo) - if err != nil { - return fmt.Errorf("get HEAD SHA for env branch anchor: %w", err) - } + anchor, err := r.resolveEnvAnchor(env, step.BaseRef) + if err != nil { + return err } if err := r.harness.gitea.CreateBranch(ctx, r.harness.repo, envBranch, anchor); err != nil { return fmt.Errorf("create env branch %s: %w", envBranch, err) diff --git a/e2e/harness/hotfix_actions_test.go b/e2e/harness/hotfix_actions_test.go index 1455ae8d..09f3c0ae 100644 --- a/e2e/harness/hotfix_actions_test.go +++ b/e2e/harness/hotfix_actions_test.go @@ -298,6 +298,71 @@ func TestAssertStateDivergence(t *testing.T) { } } +// TestResolveEnvAnchor verifies the deterministic env-branch anchor resolution +// that fixes the hotfix-conflict-resolution flake: an explicit base_ref wins, a +// recorded state SHA is the fallback, and an unresolvable anchor errors loudly +// rather than silently anchoring on trunk HEAD (which would flip an engineered +// conflict into an empty clean cherry-pick run-to-run). +func TestResolveEnvAnchor(t *testing.T) { + t.Run("base_ref resolved via execution context wins", func(t *testing.T) { + r := NewRunner(t, nil) + r.ctx.RecordCommit("commit1", "base1111aaaa") + r.ctx.RecordState("test", "tip2222bbbb", "v0.1.0") + + anchor, err := r.resolveEnvAnchor("test", "commit1") + require.NoError(t, err) + assert.Equal(t, "base1111aaaa", anchor, "base_ref must take precedence over the recorded state SHA") + }) + + t.Run("base_ref literal SHA when not a known reference", func(t *testing.T) { + r := NewRunner(t, nil) + anchor, err := r.resolveEnvAnchor("test", "literalsha9999") + require.NoError(t, err) + assert.Equal(t, "literalsha9999", anchor) + }) + + t.Run("falls back to recorded state SHA when no base_ref", func(t *testing.T) { + r := NewRunner(t, nil) + r.ctx.RecordState("test", "tip2222bbbb", "v0.1.0") + anchor, err := r.resolveEnvAnchor("test", "") + require.NoError(t, err) + assert.Equal(t, "tip2222bbbb", anchor) + }) + + t.Run("errors when no base_ref and state SHA empty (no trunk-HEAD fallback)", func(t *testing.T) { + r := NewRunner(t, nil) + anchor, err := r.resolveEnvAnchor("test", "") + require.Error(t, err) + assert.Empty(t, anchor) + assert.Contains(t, err.Error(), "base_ref") + assert.Contains(t, err.Error(), "deterministic") + }) +} + +// TestParseHotfixApplyBaseRef verifies the hotfix_apply base_ref field unmarshals +// so a scenario can pin the env anchor for a deterministic cherry-pick outcome. +func TestParseHotfixApplyBaseRef(t *testing.T) { + yamlDoc := ` +name: "Hotfix apply base_ref" +config: + environments: [test] +steps: + - name: "Apply with pinned base" + action: hotfix_apply + hotfix_apply: + target_env: test + commit_ref: commit3 + base_ref: commit1 +` + s, err := ParseMultiStepScenario([]byte(yamlDoc)) + require.NoError(t, err) + require.Len(t, s.Steps, 1) + require.NotNil(t, s.Steps[0].HotfixApply) + assert.Equal(t, "test", s.Steps[0].HotfixApply.TargetEnv) + assert.Equal(t, "commit3", s.Steps[0].HotfixApply.CommitRef) + assert.Equal(t, "commit1", s.Steps[0].HotfixApply.BaseRef) +} + // TestRunnerHotfixActionsNoHarness proves the dispatch wiring for each new // action returns nil (the no-harness guard path) without containers. func TestRunnerHotfixActionsNoHarness(t *testing.T) { diff --git a/e2e/harness/multistep.go b/e2e/harness/multistep.go index 31dd3490..1a7b1e19 100644 --- a/e2e/harness/multistep.go +++ b/e2e/harness/multistep.go @@ -115,9 +115,18 @@ type HotfixPlanStep struct { // HotfixApplyStep defines a hotfix_apply action: a harness-driven cherry-pick of // CommitRef onto env/, pushing a hotfix branch and opening a labeled // PR. CommitRef is resolved via the execution context (falling back to literal). +// +// BaseRef, when set, pins the env/ anchor to a specific commit +// (resolved via the execution context, falling back to literal) when the env +// branch does not yet exist. This makes the cherry-pick outcome deterministic: +// whether commit3's diff applies cleanly or conflicts depends entirely on the +// content at the env anchor, so a scenario that engineers a conflict must pin +// the anchor rather than depend on the synced state SHA, which a gitea +// state-propagation race can momentarily report empty. type HotfixApplyStep struct { TargetEnv string `yaml:"target_env"` CommitRef string `yaml:"commit_ref"` + BaseRef string `yaml:"base_ref,omitempty"` } // MergePRStep defines a merge_pr action. Index identifies the PR directly; if diff --git a/e2e/scenarios/hotfix/hotfix-conflict-resolution.yaml b/e2e/scenarios/hotfix/hotfix-conflict-resolution.yaml index 6a68575a..89ef3085 100644 --- a/e2e/scenarios/hotfix/hotfix-conflict-resolution.yaml +++ b/e2e/scenarios/hotfix/hotfix-conflict-resolution.yaml @@ -118,19 +118,26 @@ steps: hotfix_apply: target_env: test commit_ref: commit3 + # Pin the env/test anchor to commit1 (Version "1"). The cherry-pick outcome + # is fully determined by the anchor content: commit3's diff rewrites the + # Version line from "2" to "2-patched", so applying it onto a tree where + # that line reads "1" produces a textual conflict every run. Without this + # pin the harness anchored on the recorded test SHA, which a gitea state + # sync race could momentarily report empty and silently flip the anchor to + # the just-patched trunk tip, turning the conflict into an empty (clean) + # cherry-pick and the PR label from -conflict to plain cascade-hotfix. + base_ref: commit1 expect: branches: exist: ["env/test"] - # Whether the cherry-pick of commit3 onto env/test surfaces a textual - # conflict is a host-side mechanic: real GitHub's server-side merge flags - # the overlapping Version edit, while the gitea-backed cherry-pick here - # applies it cleanly and opens a cascade-hotfix PR. The e2e backend is - # gitea, so this step asserts the label gitea actually produces; the - # conflict-labeled PR path is exercised by the real-GitHub validation - # fleet. The resolve step below still pushes resolved content onto the PR - # head and replays the check, so the resolution path stays covered. + # The cherry-pick conflicts deterministically here (see base_ref above): + # commit3's Version edit overlaps the "1" still on env/test, so the apply + # job opens the conflict-resolution PR labeled cascade-hotfix-conflict. + # The resolve step below pushes resolved content onto the PR head and + # replays the check, exercising the full conflict-resolution path the + # scenario name promises. prs: - open_with_label: "cascade-hotfix" + open_with_label: "cascade-hotfix-conflict" # The gitea harness creates PR labels on demand, so it cannot reproduce the # real-GitHub behavior where "gh pr create --label X" fails hard when X does # not yet exist. That gap let a missing label seed ship undetected: the @@ -157,9 +164,10 @@ steps: - name: "Merge hotfix PR" action: merge_pr merge_pr: - # Matches the label gitea applied at apply time (see the note on the apply - # step): the cherry-pick lands clean here, so the PR carries cascade-hotfix. - label: "cascade-hotfix" + # Matches the label the apply step recorded: the cherry-pick conflicts here + # (see the base_ref note on the apply step), so the resolution PR carries + # cascade-hotfix-conflict. + label: "cascade-hotfix-conflict" - name: "Finalize hotfix for test" action: hotfix_merged From 7f2561830309bbe782c3d760853d28fd0d819d4a Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 24 Jun 2026 13:56:15 -0400 Subject: [PATCH 2/3] fix(e2e): seed hotfix env branch via git push by SHA gitea's branches API resolves old_ref_name as a branch or tag name, not a raw commit SHA, so seeding env/ from an anchor SHA silently branched from HEAD instead. At the conflict scenario's apply step HEAD already carried the patched content, so the cherry-pick applied cleanly and never produced the engineered conflict. Seed the env branch with a git push of the anchor SHA from inside the act container, which resolves the object by hash precisely, so the anchor content is exactly what the SHA guarantees. Signed-off-by: Joshua Temple --- e2e/harness/hotfix_actions.go | 76 ++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 20 deletions(-) diff --git a/e2e/harness/hotfix_actions.go b/e2e/harness/hotfix_actions.go index e96c8aa7..6e0db3ff 100644 --- a/e2e/harness/hotfix_actions.go +++ b/e2e/harness/hotfix_actions.go @@ -185,31 +185,23 @@ func (r *Runner) executeHotfixApply(ctx context.Context, step *HotfixApplyStep) hotfixBranch := "hotfix/" + env + "/" + short r.t.Logf(" HotfixApply: commits=%s env=%s branch=%s", commitList, env, hotfixBranch) - // Ensure env/ exists, anchored at the env's true base (resolveEnvAnchor). + // Determine whether env/ already exists so we know whether to seed it. branches, err := r.harness.gitea.ListBranches(ctx, r.harness.repo) if err != nil { return fmt.Errorf("list branches: %w", err) } - if !containsString(branches, envBranch) { - anchor, err := r.resolveEnvAnchor(env, step.BaseRef) + needsSeedEnvBranch := !containsString(branches, envBranch) + + // Resolve the anchor SHA for env branch seeding. The anchor's tree content + // determines the cherry-pick outcome: only needed when the branch is absent. + // resolveEnvAnchor errors (rather than silently using trunk HEAD) when the + // anchor is unresolvable, surfacing sync races loudly. + var anchorSHA string + if needsSeedEnvBranch { + anchorSHA, err = r.resolveEnvAnchor(env, step.BaseRef) if err != nil { return err } - if err := r.harness.gitea.CreateBranch(ctx, r.harness.repo, envBranch, anchor); err != nil { - return fmt.Errorf("create env branch %s: %w", envBranch, err) - } - r.t.Logf(" HotfixApply: created %s at %s", envBranch, truncateSHA(anchor)) - // Gitea's branch-list endpoint lags a create: wait until the new branch - // is listed so a later branches.exist assertion (which lists branches) - // observes it rather than racing the create. - if err := r.waitForBranchListed(ctx, envBranch, 30*time.Second); err != nil { - return fmt.Errorf("waiting for env branch %s to be listed: %w", envBranch, err) - } - } - - baseSHA, err := r.harness.gitea.GetBranchSHA(ctx, r.harness.repo, envBranch) - if err != nil { - return fmt.Errorf("get base SHA for %s: %w", envBranch, err) } if err := r.harness.SyncRepoToActContainer(ctx); err != nil { @@ -221,12 +213,35 @@ func (r *Runner) executeHotfixApply(ctx context.Context, step *HotfixApplyStep) // conflict PR body. The push uses the admin-credentialed origin URL. pushURL := r.authedRepoURL() short8 := short - script := strings.Join([]string{ + + // Env branch seed snippet: when env/ does not yet exist, push the anchor + // SHA directly via git rather than the gitea HTTP branches API. The HTTP API + // accepts old_ref_name as a branch or tag name; passing a raw commit SHA is + // not reliable across gitea versions (some ignore it and branch from HEAD + // instead). A git push from inside the act container is precise: git resolves + // the object by its SHA and the push creates the remote ref at exactly that + // commit. The act container has the full object database (SyncRepoToActContainer + // fetches all of main including the anchor commit) so the push always finds the + // object. + seedEnvBranchLines := []string{} + if needsSeedEnvBranch { + seedEnvBranchLines = []string{ + // Create env/ at anchorSHA via git push. --force handles the case + // where a prior scenario retry created a stale copy. + fmt.Sprintf("git push --force %q %q", pushURL, anchorSHA+":refs/heads/"+envBranch), + "echo \"SEED_ENV_EXIT=$?\"", + } + } + + scriptLines := []string{ "set +e", // Abort any half-finished cherry-pick left in the shared /tmp/repo by a // prior apply in this scenario, then drop any stale local hotfix branch so // the re-create below always anchors on the freshly-fetched remote tip. "git cherry-pick --abort >/dev/null 2>&1 || true", + } + scriptLines = append(scriptLines, seedEnvBranchLines...) + scriptLines = append(scriptLines, // Force-update the env tracking ref so a second apply onto an env branch // the first finalize already advanced (squash-merge) anchors on the // current tip rather than a stale ref, otherwise the cherry-pick replays @@ -256,12 +271,26 @@ func (r *Runner) executeHotfixApply(ctx context.Context, step *HotfixApplyStep) // shared branch. fmt.Sprintf("git push --force %q %q", pushURL, hotfixBranch+":"+hotfixBranch), "echo \"PUSH_EXIT=$?\"", - }, "\n") + ) + script := strings.Join(scriptLines, "\n") _, out, err := r.execInRepo(ctx, script) if err != nil { return fmt.Errorf("cherry-pick exec: %w", err) } + if needsSeedEnvBranch { + if strings.Contains(out, "SEED_ENV_EXIT=") && !strings.Contains(out, "SEED_ENV_EXIT=0") { + r.t.Logf(" HotfixApply seed-env push output:\n%s", out) + return fmt.Errorf("failed to seed env branch %s at %s", envBranch, truncateSHA(anchorSHA)) + } + r.t.Logf(" HotfixApply: seeded %s at %s via git push", envBranch, truncateSHA(anchorSHA)) + // Gitea's branch-list endpoint lags a push: wait until the new branch + // is listed so a later branches.exist assertion (which lists branches) + // observes it rather than racing the push. + if err := r.waitForBranchListed(ctx, envBranch, 30*time.Second); err != nil { + return fmt.Errorf("waiting for seeded env branch %s to be listed: %w", envBranch, err) + } + } conflictFiles := parseSentinel(out, "CONFLICT_FILES=") conflict := strings.TrimSpace(conflictFiles) != "" if strings.Contains(out, "PUSH_EXIT=") && !strings.Contains(out, "PUSH_EXIT=0") { @@ -275,6 +304,13 @@ func (r *Runner) executeHotfixApply(ctx context.Context, step *HotfixApplyStep) return fmt.Errorf("waiting for pushed branch %s: %w", hotfixBranch, err) } + // Read the env branch's current tip as the base for the PR trailers. This is + // read after the script so it reflects the seeded-or-pre-existing branch head. + baseSHA, err := r.harness.gitea.GetBranchSHA(ctx, r.harness.repo, envBranch) + if err != nil { + return fmt.Errorf("get base SHA for %s: %w", envBranch, err) + } + // Build the PR body with the three product trailers; append the conflict file // list on the conflict path. body := fmt.Sprintf("Cascade-Hotfix-Target: %s\nCascade-Hotfix-Source: %s\nCascade-Hotfix-Base: %s\n", env, commitList, baseSHA) From 6f0f960eb818646cce394ac2dbfce300f39830d2 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 24 Jun 2026 18:42:12 -0400 Subject: [PATCH 3/3] fix(e2e): demux docker exec stream so the hotfix conflict sentinel parses execInRepo read the container exec output with a plain io.Copy, but Docker's exec stream is multiplexed with 8-byte per-frame headers. Those binary header bytes landed between newlines, so parseSentinel split lines that began with a frame header instead of CONFLICT_FILES=, the match never hit, and conflict was always parsed as false (wrong cascade-hotfix label on a genuine conflict). Read through the existing readDemuxedStream (stdcopy.StdCopy) helper instead. The cherry-pick was always conflicting correctly; the sentinel is now parseable. Verified locally: the conflict scenario passes deterministically with conflict=true across repeated runs, and the clean-apply path is unaffected. Signed-off-by: Joshua Temple --- e2e/harness/hotfix_actions.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/e2e/harness/hotfix_actions.go b/e2e/harness/hotfix_actions.go index 6e0db3ff..9d7a546d 100644 --- a/e2e/harness/hotfix_actions.go +++ b/e2e/harness/hotfix_actions.go @@ -1,11 +1,9 @@ package harness import ( - "bytes" "context" "encoding/json" "fmt" - "io" "strings" "time" ) @@ -60,11 +58,16 @@ func (r *Runner) execInRepo(ctx context.Context, script string) (int, string, er if err != nil { return exitCode, "", err } - var out bytes.Buffer - if reader != nil { - _, _ = io.Copy(&out, reader) + // readDemuxedStream strips Docker's per-frame multiplexing headers (stream + // type + big-endian length prefix on each chunk). A plain io.Copy leaves those + // 8-byte binary headers interspersed in the output, corrupting sentinel lines + // like "CONFLICT_FILES=..." so that HasPrefix never matches, silently turning + // an engineered cherry-pick conflict into a clean apply. + out, demuxErr := readDemuxedStream(reader) + if demuxErr != nil { + return exitCode, "", fmt.Errorf("exec output: %w", demuxErr) } - return exitCode, out.String(), nil + return exitCode, out, nil } // repoEnv builds the standard workflow environment (GITHUB_REPOSITORY) shared by