From e96766dc4595b6551605d4c846895fcad79ffa98 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Tue, 28 Jul 2026 14:53:33 -0700 Subject: [PATCH] feat(runway): git-backed merger with REBASE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? Runway's `merger` extension has had exactly one implementation — `noop`, which always succeeds. Nothing actually merges anything. This lands the first real backend: a `Merger` driven by the `git` CLI against a local checkout. It ships REBASE only. The strategy-specific apply paths are small and independent, but the machinery underneath them — the pinned git runtime, object resolution, the reset/apply/push cycle, contention retry, dry-run discard, conflict classification — is shared and is the bulk of what needs review. Landing it with one strategy keeps that review separable from the per-strategy mechanics that follow in this stack. ### What? Adds `runway/extension/merger/git`. **A change is a range of commits, not a commit.** A change URI pins a pull request to a single head SHA, but a pull request is routinely several commits. Applying the head alone applies only that commit's diff against its own parent: it conflicts against context its predecessors would have established, or — when the commits touch different files — succeeds while silently dropping everything before the head. So the unit replayed is the range from the change's merge base with the target up to its head. A change already contained in the target has an empty range and is a no-op, which is what keeps redelivery idempotent. A range runs through git's sequencer, which changes the control flow versus a single pick: the operation stops on a commit it declines and `--skip` advances to the next rather than ending. Two kinds of commit stop it harmlessly — one already present on the target, and one that was empty to begin with — and both are skipped. Anything else is a real conflict, and the in-progress pick is aborted so the checkout stays usable. The commits an apply produced are read back off the checkout rather than tracked per-invocation, which stays correct when the sequencer drops some. **Referenced commits are guaranteed present before anything is applied.** The default fetch refspec is `+refs/heads/*`, which does not cover a provider's change refs: a pull request head never also pushed as a branch — the normal case for a fork — is simply absent, and the apply then fails with git's "bad object", indistinguishable from a conflict. Every commit the request names is now fetched and verified up front, for all steps, so an unusable request fails without having mutated the checkout. Commits are requested by SHA (relying on the server serving a want for a reachable-but-unadvertised object, which github.com allows), falling back to the provider's canonical ref, with deployment-supplied refspecs as a last resort. Neither fetch is shallow — the range needs ancestry. A commit a *reachable* remote cannot supply is terminal; a remote that will not answer stays retryable. The first is a property of the request, the second a property of the moment. **One seam for change providers.** Every URI is reduced to the only three things the merger needs — the commit to apply, the ref the provider publishes it under, and a label for synthesized messages. `github://` and `git://` are supported; an unrecognized scheme is terminal. Adding a provider is one case in that mapping rather than a change to any apply path. **Optional staleness check.** Fetching by SHA guarantees the merger applies exactly the commit the URI names, not that it is still the change's head — a force-push leaves the superseded commit fetchable on most hosts. When enabled, each change's canonical ref is read (one ref advertisement, no object transfer) and a mismatch is terminal. **Atomicity, contention, dry run.** Nothing reaches the remote until the final push. If the push is rejected because the remote tip moved, the whole reset/apply/push cycle retries up to a bounded number of attempts. `CheckMergeability` runs the identical apply path but never pushes, then resets and reports empty outputs, committing intermediate steps locally so a multi-step check sees the same conflict surface a real merge would. **Runtime hygiene.** Every invocation uses an explicitly pinned git runtime and a scrubbed environment — no system or global config, no interactive prompts. That leaves no ambient identity, so the committer is injected per-invocation. `isConcreteStrategy` currently admits only REBASE; SQUASH_REBASE, MERGE and PROMOTE are rejected as invalid requests until their apply paths land later in this stack. The merger is not wired into the server yet, so this adds no production behavior. ## Test Plan ✅ `bazel test //runway/...` — 6/6 targets pass, including the new `//runway/extension/merger/git` suite (40s) The suite drives a real git binary against throwaway repositories. Beyond the single-commit cases (stacked URIs, already-landed changes, multi-step requests, conflicts, checkout recovery, contention retry, give-up after max attempts, DEFAULT resolution, dry runs), it covers what this change is actually about: a multi-commit change expressed as **one** URI for both disjoint and overlapping edits, a partially-landed change, an empty commit inside a range, a conflict inside a range leaving no sequencer state behind, stacked multi-commit changes not duplicating each other's commits, a head reachable only via its pull-request ref, an unavailable commit classified as invalid rather than conflicting, the staleness check on and off, and provider resolution across schemes. The regression tests were verified to fail against head-only picking: reverting just that line fails 5 of them, and they pass with the range applied. --- runway/extension/merger/git/BUILD.bazel | 52 + runway/extension/merger/git/README.md | 63 + runway/extension/merger/git/changeref.go | 96 ++ runway/extension/merger/git/git_merger.go | 796 ++++++++++++ .../extension/merger/git/git_merger_test.go | 1125 +++++++++++++++++ runway/extension/merger/git/objects.go | 116 ++ 6 files changed, 2248 insertions(+) create mode 100644 runway/extension/merger/git/BUILD.bazel create mode 100644 runway/extension/merger/git/README.md create mode 100644 runway/extension/merger/git/changeref.go create mode 100644 runway/extension/merger/git/git_merger.go create mode 100644 runway/extension/merger/git/git_merger_test.go create mode 100644 runway/extension/merger/git/objects.go diff --git a/runway/extension/merger/git/BUILD.bazel b/runway/extension/merger/git/BUILD.bazel new file mode 100644 index 00000000..a173bd6b --- /dev/null +++ b/runway/extension/merger/git/BUILD.bazel @@ -0,0 +1,52 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = [ + "changeref.go", + "git_merger.go", + "objects.go", + ], + importpath = "github.com/uber/submitqueue/runway/extension/merger/git", + visibility = ["//visibility:public"], + deps = [ + "//api/base/mergestrategy/protopb:go_default_library", + "//api/runway/messagequeue:go_default_library", + "//api/runway/messagequeue/protopb:go_default_library", + "//platform/base/change/git:go_default_library", + "//platform/base/change/github:go_default_library", + "//platform/metrics:go_default_library", + "//runway/extension/merger:go_default_library", + "@com_github_uber_go_tally//:go_default_library", + "@org_uber_go_zap//:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["git_merger_test.go"], + data = [ + "@git", + "@git//:git_receive_pack", + "@git//:git_upload_archive", + "@git//:git_upload_pack", + "@git//:templates", + "@git//:templates/description", + ], + embed = [":go_default_library"], + env = { + "SUBMITQUEUE_TEST_GIT": "$(location @git//:git)", + "SUBMITQUEUE_TEST_GIT_TEMPLATE_DESCRIPTION": "$(location @git//:templates/description)", + }, + deps = [ + "//api/base/change/protopb:go_default_library", + "//api/base/mergestrategy/protopb:go_default_library", + "//api/runway/messagequeue:go_default_library", + "//api/runway/messagequeue/protopb:go_default_library", + "//runway/extension/merger:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + "@com_github_uber_go_tally//:go_default_library", + "@org_uber_go_zap//zaptest:go_default_library", + ], +) diff --git a/runway/extension/merger/git/README.md b/runway/extension/merger/git/README.md new file mode 100644 index 00000000..1f119c90 --- /dev/null +++ b/runway/extension/merger/git/README.md @@ -0,0 +1,63 @@ +# git merger + +A `merger.Merger` backed by the `git` CLI operating on a local checkout. It applies a merge request's ordered steps onto a target branch, honoring each step's strategy, and — for a committing merge — pushes the result. It is constructed by the wiring layer (see [`service/runway`](../../../../service/runway)) with the checkout path, remote, target branch, pinned git runtime, committer identity, and the default strategy; the request itself carries only a queue name and the changes, so the merger reads change URIs straight from the payload (no store, no resolver). + +## Model + +A request is an ordered list of steps; each step names a change (a set of provider URIs, each ending in a full head commit SHA) and a strategy. Steps are applied in order on top of the target tip — earlier steps are the in-flight base, the last step is the candidate. Each step yields one `StepResult`; the revisions a step produces on the target are its outputs, in application order. + +A URI pins a change to one head commit, but a change is routinely several commits. The full set is recovered locally rather than from the wire: the commits to replay are the range from the change's merge base with the target up to its head. Applying the head commit alone would apply only that commit's diff against its own parent — conflicting against context its predecessors would have established, or silently dropping them when they touch different files. + +## Change providers + +Every URI is reduced to three things: the commit to apply, the ref the provider publishes that commit under, and a short label for synthesized commit messages. Adding a provider is one case in that mapping, not a change to any apply path. + +| Scheme | Commit | Canonical ref | Label | +|---|---|---|---| +| `github://` | head commit SHA | `refs/pull/{n}/head` | `org/repo#n` | +| `git://` | the URI's commit SHA | the URI's own ref | `repo@ref` | + +An unrecognized scheme is a terminal invalid request. + +## Object availability + +The default fetch refspec is `+refs/heads/*`, which does not cover a provider's change refs — a pull request head never also pushed as a branch, the normal case for a fork, is simply absent locally. Every referenced commit is therefore fetched and verified before any step is applied, so a request naming an unreachable commit fails without having touched the checkout. + +Commits are requested by SHA, which relies on the server serving a want for an object that is reachable but not advertised (`uploadpack.allowReachableSHA1InWant`, which github.com enables); the provider's canonical ref is the fallback. Neither fetch is shallow — the apply paths need ancestry, not just the commit. A remote that supports neither can supply explicit refspecs via configuration. + +A commit that cannot be fetched from a reachable remote is a terminal invalid request, not a conflict. If the remote itself is unreachable the error stays retryable. + +The same line is drawn inside an apply: a non-zero exit from git is not by itself a conflict. The index is consulted for conflicted entries before the operation is aborted, so a missing object, an unreadable repository or a killed process stays a retryable infrastructure error instead of permanently failing a change that never actually collided. + +## Staleness + +Fetching by SHA guarantees the merger applies exactly the commit a URI names — not that the commit is still the change's head. A force-push leaves the superseded commit fetchable for some time on most hosts, so a successful fetch says nothing about freshness. When enabled, each change's canonical ref is read (one ref advertisement, no object transfer) and a mismatch is terminal. A ref that no longer exists yields no verdict. + +## Strategies + +| Strategy | What it does | Outputs | +|---|---|---| +| `REBASE` | Cherry-picks every commit each change introduces onto the tip, in order. A commit already present on the target is skipped (no output), as is one that was empty to begin with. | one revision per newly-created commit | +| `DEFAULT` | Resolved to the instance's configured default strategy before any step runs. | per the resolved strategy | + +`REBASE` is the only strategy implemented so far. `SQUASH_REBASE`, `MERGE`, and `PROMOTE` are defined by the wire contract but not yet applied here — a step naming one is rejected as an invalid request. + +## Committing, dry-run, atomicity, contention + +`Merge` commits and reports outputs; `CheckMergeability` runs the identical apply but never pushes, then resets the checkout to discard the local commits and reports empty outputs. A multi-step check commits its intermediate steps locally so it sees the same conflict surface a real merge would. + +For a committing merge nothing reaches the remote until the final push. A step that fails to apply aborts its in-progress git operation and returns without pushing. If the push fails because the remote tip moved between reset and push, the whole reset/apply/push cycle is retried up to a bounded number of attempts; detection re-fetches the tip and compares it to the SHA the cycle was based on. + +## Failure classification + +A merge conflict surfaces as `merger.ErrConflict`. An unusable request surfaces as `merger.ErrInvalidRequest`: an unsupported strategy or URI scheme, a malformed URI, a commit a reachable remote cannot supply, a change whose head has moved on, or a change sharing no history with the target under a picking strategy. Both are terminal — the controller publishes a `FAILED` result rather than retrying. Everything else (network/auth/push faults, and an unreachable remote) is returned as a plain error for the consumer to retry. + +The distinction between the last two matters operationally: a commit that is missing while the remote answers is a property of the request, whereas a remote that will not answer is a property of the moment. + +## Runtime and identity + +Every git invocation uses the pinned runtime (explicit executable, exec-path, and template dir) and a scrubbed environment: no ambient configuration, no system or global git config, no interactive prompts. Because that leaves no ambient identity, the committer name and email are injected per-invocation, which the commit-creating `REBASE` strategy requires. + +Scrubbing denies git ambient *configuration* — anything that could change what a merge produces. It deliberately does not deny it the means to reach the remote, so the agent socket, `PATH`, ssh-command, TLS and proxy variables are inherited when set. Without them an SSH remote cannot authenticate and git cannot even exec `ssh`; none of them can influence merge semantics. A deployment needing more can name additional variables on the runtime. + +See [Object availability](#object-availability) for how referenced commits are obtained. diff --git a/runway/extension/merger/git/changeref.go b/runway/extension/merger/git/changeref.go new file mode 100644 index 00000000..3dc213e2 --- /dev/null +++ b/runway/extension/merger/git/changeref.go @@ -0,0 +1,96 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package git + +import ( + "fmt" + "strings" + + entitygit "github.com/uber/submitqueue/platform/base/change/git" + entitygithub "github.com/uber/submitqueue/platform/base/change/github" + "github.com/uber/submitqueue/runway/extension/merger" +) + +// changeRef is everything the merger needs from a change URI, reduced to a +// form that does not depend on which provider minted it. Adding a provider +// means adding one case to resolveChange, not touching the apply paths. +type changeRef struct { + // SHA is the full commit hash the URI pins the change to. This is the + // commit that gets fetched and applied. + SHA string + // Ref is the fully-qualified ref the provider publishes this change's head + // under (e.g. "refs/pull/12/head"). Used as a fetch fallback and as the + // staleness comparison point. Empty when the scheme has no such ref, in + // which case both are skipped. + Ref string + // Label is a short human-readable identifier for the change, used in + // commit messages the merger synthesizes. + Label string +} + +// resolveChange maps a change URI onto the merger's provider-neutral view of +// it. An unrecognized scheme is terminal: no retry produces a parser for it. +func resolveChange(uri string) (changeRef, error) { + scheme, _, ok := strings.Cut(uri, "://") + if !ok { + return changeRef{}, fmt.Errorf("%w: change URI %q has no scheme", merger.ErrInvalidRequest, uri) + } + + switch scheme { + case "github": + cid, err := entitygithub.ParseChangeID(uri) + if err != nil { + return changeRef{}, fmt.Errorf("%w: invalid change URI %q: %v", merger.ErrInvalidRequest, uri, err) + } + return changeRef{ + SHA: cid.HeadCommitSHA, + // GitHub publishes every PR's head under refs/pull//head in the + // base repository, including PRs opened from a fork. + Ref: fmt.Sprintf("refs/pull/%d/head", cid.PRNumber), + Label: fmt.Sprintf("%s#%d", cid.OwnerRepo(), cid.PRNumber), + }, nil + + case "git": + cid, err := entitygit.ParseChangeID(uri) + if err != nil { + return changeRef{}, fmt.Errorf("%w: invalid change URI %q: %v", merger.ErrInvalidRequest, uri, err) + } + // A git:// URI already names its own fully-qualified ref, so the + // staleness check reads exactly the ref the caller pinned. + return changeRef{ + SHA: cid.CommitSHA, + Ref: cid.Ref, + Label: fmt.Sprintf("%s@%s", cid.Repo, cid.Ref), + }, nil + + default: + return changeRef{}, fmt.Errorf("%w: unsupported change URI scheme %q in %q", merger.ErrInvalidRequest, scheme, uri) + } +} + +// resolveStepChanges resolves every URI of every step, in application order. +func resolveStepChanges(steps []resolvedStep) ([]changeRef, error) { + var refs []changeRef + for _, rs := range steps { + for _, uri := range rs.step.GetChange().GetUris() { + ref, err := resolveChange(uri) + if err != nil { + return nil, err + } + refs = append(refs, ref) + } + } + return refs, nil +} diff --git a/runway/extension/merger/git/git_merger.go b/runway/extension/merger/git/git_merger.go new file mode 100644 index 00000000..cbd2d063 --- /dev/null +++ b/runway/extension/merger/git/git_merger.go @@ -0,0 +1,796 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package git is a Merger implementation backed by a local git checkout. It +// applies a MergeRequest's ordered steps onto a target branch, honoring each +// step's merge strategy, and (for a committing Merge) pushes the result. +// +// Strategy → git operation: +// +// - REBASE: cherry-pick each URI's head commit onto the target tip. +// - DEFAULT: resolved to the instance's configured DefaultStrategy +// before any step runs. +// +// REBASE is the only strategy implemented so far. A step naming any other +// strategy is rejected as merger.ErrInvalidRequest until its apply path lands. +// +// Atomicity: for a committing merge nothing reaches the remote until the final +// push. A step that fails to apply aborts the in-progress git operation and +// returns without pushing. +// +// Contention: if the push fails because the remote tip moved between reset and +// push, the whole reset/apply/push cycle is retried up to Params.MaxPushAttempts +// (default 10). Detection re-fetches the remote tip after a push failure and +// compares it to the SHA reset to at the start of the cycle. +// +// Dry-run (CheckMergeability) applies the exact same steps but never pushes; +// intermediate steps are committed locally so a cumulative multi-step check +// sees the same conflict surface, then the checkout is reset to discard them. +// Reported StepResults carry no Outputs. +// +// Change URIs are resolved through resolveChange (see changeref.go), which +// reduces any supported provider URI to the commit to apply, the ref that +// commit canonically lives under, and a label. github:// and git:// are +// supported; an unrecognized scheme is a terminal invalid request. +// +// Object availability: every referenced commit is fetched and verified before +// any step is applied. A change is replayed as the full range from its merge +// base with the target up to its head, because a URI pins a pull request to a +// single head SHA while the pull request itself is routinely several commits. +package git + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + + "github.com/uber-go/tally" + "go.uber.org/zap" + + mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" + runwaymq "github.com/uber/submitqueue/api/runway/messagequeue" + runwaypb "github.com/uber/submitqueue/api/runway/messagequeue/protopb" + coremetrics "github.com/uber/submitqueue/platform/metrics" + "github.com/uber/submitqueue/runway/extension/merger" +) + +// defaultMaxPushAttempts caps the retry loop when the remote tip keeps moving +// under us. Bounded to prevent an infinite loop against a busy remote. +const defaultMaxPushAttempts = 10 + +// Default committer identity used when Params leaves it unset. The scrubbed +// environment (GIT_CONFIG_NOSYSTEM, GIT_CONFIG_GLOBAL=/dev/null) leaves no +// ambient identity, so the commit-creating REBASE strategy needs one supplied +// explicitly. +const ( + defaultCommitterName = "SubmitQueue Runway" + defaultCommitterEmail = "runway@submitqueue.invalid" +) + +// GitRuntime identifies the explicitly provided Git runtime used by the Merger. +type GitRuntime struct { + // Executable is the absolute path to the Git executable. + Executable string + // ExecPath is the absolute directory containing Git's helper executables. + ExecPath string + // TemplateDir is the absolute directory containing Git's repository + // templates. + TemplateDir string + // PassthroughEnv names additional environment variables to inherit from + // the parent process, on top of the auth and transport ones always passed + // through. For a deployment whose remote needs something unusual; leave + // empty otherwise. Names that could alter merge semantics do not belong + // here — the scrubbed environment is what keeps a merge reproducible. + PassthroughEnv []string +} + +// Params holds the dependencies for the git Merger. +type Params struct { + // CheckoutPath is the absolute path to an existing git checkout that the + // Merger will operate against. The Merger owns this working tree. + CheckoutPath string + // Remote is the name of the git remote to fetch from and push to + // (e.g. "origin"). + Remote string + // Target is the destination branch ref on the remote (e.g. "main"). + Target string + // DefaultStrategy resolves a step whose strategy is DEFAULT. Must be a + // concrete strategy (currently only REBASE). + DefaultStrategy mergestrategypb.Strategy + // Runtime is the pinned Git runtime used for every invocation. + Runtime GitRuntime + // MaxPushAttempts caps how many times a committing merge retries the full + // reset/apply/push cycle when the remote tip moves under it. Defaults to + // defaultMaxPushAttempts when zero or negative. + MaxPushAttempts int + // FetchRefspecs are extra refspecs fetched on every cycle. Only needed for + // a remote that refuses to serve an unadvertised object by SHA; leave empty + // otherwise, since fetching broad patterns such as + // "+refs/pull/*/head:refs/remotes/origin/pr/*" costs a full ref + // advertisement on every attempt. + FetchRefspecs []string + // CheckStaleness enables verifying, before applying, that each change's + // canonical ref still points at the commit its URI names. + CheckStaleness bool + // CommitterName / CommitterEmail identify the author/committer of + // service-created commits. Defaults are used when empty. + CommitterName string + CommitterEmail string + // Logger is the structured logger. + Logger *zap.SugaredLogger + // MetricsScope is the metrics scope for instrumentation. + MetricsScope tally.Scope +} + +// gitMerger implements merger.Merger by shelling out to the `git` CLI against a +// local checkout. +type gitMerger struct { + checkoutPath string + remote string + target string + defaultStrategy mergestrategypb.Strategy + runtime GitRuntime + maxPushAttempts int + fetchRefspecs []string + checkStaleness bool + committerName string + committerEmail string + logger *zap.SugaredLogger + metricsScope tally.Scope + + // mu serializes concurrent operations — the underlying checkout cannot be + // safely shared between operations. + mu sync.Mutex +} + +// Verify gitMerger implements merger.Merger at compile time. +var _ merger.Merger = (*gitMerger)(nil) + +// resolvedStep pairs a request step with its resolved (non-DEFAULT) strategy. +type resolvedStep struct { + step *runwaymq.MergeStep + strategy mergestrategypb.Strategy +} + +// NewMerger constructs a git-backed Merger operating against the given checkout. +// The checkout must already exist and have the configured remote. Runtime paths +// must be absolute and DefaultStrategy must be a concrete strategy. +func NewMerger(params Params) (merger.Merger, error) { + if err := params.Runtime.validate(); err != nil { + return nil, err + } + if !isConcreteStrategy(params.DefaultStrategy) { + return nil, fmt.Errorf("default strategy must be concrete (currently only REBASE), got %v", params.DefaultStrategy) + } + maxAttempts := params.MaxPushAttempts + if maxAttempts <= 0 { + maxAttempts = defaultMaxPushAttempts + } + committerName := params.CommitterName + if committerName == "" { + committerName = defaultCommitterName + } + committerEmail := params.CommitterEmail + if committerEmail == "" { + committerEmail = defaultCommitterEmail + } + return &gitMerger{ + checkoutPath: params.CheckoutPath, + remote: params.Remote, + target: params.Target, + defaultStrategy: params.DefaultStrategy, + runtime: params.Runtime, + maxPushAttempts: maxAttempts, + fetchRefspecs: params.FetchRefspecs, + checkStaleness: params.CheckStaleness, + committerName: committerName, + committerEmail: committerEmail, + logger: params.Logger.Named("git_merger"), + metricsScope: params.MetricsScope.SubScope("git_merger"), + }, nil +} + +func (r GitRuntime) validate() error { + for name, path := range map[string]string{ + "executable": r.Executable, + "exec path": r.ExecPath, + "template dir": r.TemplateDir, + } { + if path == "" { + return fmt.Errorf("git runtime %s is required", name) + } + if !filepath.IsAbs(path) { + return fmt.Errorf("git runtime %s must be absolute: %q", name, path) + } + } + return nil +} + +// CheckMergeability applies the request's steps as a dry run: it verifies each +// step applies cleanly without committing to the remote, and returns per-step +// results with empty Outputs. +func (m *gitMerger) CheckMergeability(ctx context.Context, req *runwaymq.MergeRequest) (*runwaymq.MergeResult, error) { + return m.process(ctx, req, false) +} + +// Merge applies the request's steps and commits the result to the remote, +// returning per-step Outputs (the revisions produced on the target). +func (m *gitMerger) Merge(ctx context.Context, req *runwaymq.MergeRequest) (*runwaymq.MergeResult, error) { + return m.process(ctx, req, true) +} + +// process is the shared entry point for both Merge and CheckMergeability. The +// commit flag selects whether the result is pushed to the remote (Merge) or +// applied locally and discarded (CheckMergeability). +func (m *gitMerger) process(ctx context.Context, req *runwaymq.MergeRequest, commit bool) (ret *runwaymq.MergeResult, retErr error) { + opName := "check" + if commit { + opName = "merge" + } + op := coremetrics.Begin(m.metricsScope, opName, coremetrics.LongLatencyBuckets) + defer func() { op.Complete(retErr) }() + + steps, err := m.resolveAndValidate(req) + if err != nil { + return nil, err + } + + m.mu.Lock() + defer m.mu.Unlock() + + m.logger.Debugw("starting merge", + "id", req.GetId(), + "target", m.target, + "remote", m.remote, + "step_count", len(steps), + "commit", commit, + ) + + return m.applyTransforming(ctx, req, steps, commit) +} + +// resolveAndValidate normalizes DEFAULT strategies to the configured default +// and validates every change URI parses. All failures here are terminal (merger.ErrInvalidRequest): retrying never +// succeeds, so the controller publishes a FAILED result rather than nacking. +func (m *gitMerger) resolveAndValidate(req *runwaymq.MergeRequest) ([]resolvedStep, error) { + if len(req.GetSteps()) == 0 { + return nil, fmt.Errorf("%w: request has no steps", merger.ErrInvalidRequest) + } + + resolved := make([]resolvedStep, 0, len(req.GetSteps())) + for _, step := range req.GetSteps() { + strategy := step.GetStrategy() + if strategy == mergestrategypb.Strategy_DEFAULT { + strategy = m.defaultStrategy + } + if !isConcreteStrategy(strategy) { + return nil, fmt.Errorf("%w: unsupported strategy %v", merger.ErrInvalidRequest, step.GetStrategy()) + } + ch := step.GetChange() + if ch == nil || len(ch.GetUris()) == 0 { + return nil, fmt.Errorf("%w: step %q has no change URIs", merger.ErrInvalidRequest, step.GetStepId()) + } + for _, uri := range ch.GetUris() { + if _, err := resolveChange(uri); err != nil { + return nil, err + } + } + resolved = append(resolved, resolvedStep{step: step, strategy: strategy}) + } + + return resolved, nil +} + +// applyTransforming runs the reset/apply/push cycle for the transforming +// strategies, retrying on remote contention when committing. For a dry run it applies the steps locally then discards them. +func (m *gitMerger) applyTransforming(ctx context.Context, req *runwaymq.MergeRequest, steps []resolvedStep, commit bool) (*runwaymq.MergeResult, error) { + var lastErr error + for attempt := 1; attempt <= m.maxPushAttempts; attempt++ { + baseSHA, stepResults, err := m.tryApply(ctx, steps, commit) + if err == nil { + if !commit { + // Discard the local commits the dry run created so the checkout + // is clean for the next operation, and report empty Outputs. + if derr := m.resetToRemote(ctx); derr != nil { + return nil, fmt.Errorf("discard after dry-run: %w", derr) + } + stripOutputs(stepResults) + } + m.logger.Debugw("merge complete", "id", req.GetId(), "target", m.target, "commit", commit) + return successResult(req, stepResults), nil + } + + // A conflict is terminal — no retry. Discard any partial dry-run state. + if errors.Is(err, merger.ErrConflict) { + if !commit { + _ = m.resetToRemote(ctx) + } + return nil, err + } + + // Only a push failure caused by the remote tip moving under us (between + // reset and push) is worth retrying; everything else is fatal. baseSHA + // is empty when the failure happened before reset captured a base. + if !commit || baseSHA == "" { + return nil, err + } + currentSHA, refetchErr := m.refetchTipSHA(ctx) + if refetchErr != nil { + return nil, fmt.Errorf("refetch after push failure failed: %v (original push error: %w)", refetchErr, err) + } + if currentSHA == baseSHA { + return nil, err + } + + coremetrics.NamedCounter(m.metricsScope, "merge", "stale_base_retries", 1) + m.logger.Warnw("remote tip moved during merge, retrying", + "attempt", attempt, + "max_attempts", m.maxPushAttempts, + "base_sha", baseSHA, + "current_sha", currentSHA, + "err", err, + ) + lastErr = err + } + + coremetrics.NamedCounter(m.metricsScope, "merge", "stale_base_giveup", 1) + return nil, fmt.Errorf("exceeded %d merge attempts due to remote contention: %w", m.maxPushAttempts, lastErr) +} + +// tryApply runs one full reset+apply(+push) cycle. The returned baseSHA is the +// SHA the cycle was based on (set as soon as resetToRemote completes) so the +// caller can distinguish concurrent-push contention from other failures. +func (m *gitMerger) tryApply(ctx context.Context, steps []resolvedStep, commit bool) (string, []*runwaymq.StepResult, error) { + if err := m.resetToRemote(ctx); err != nil { + coremetrics.NamedCounter(m.metricsScope, "merge", "reset_errors", 1) + return "", nil, err + } + baseSHA, err := m.headSHA(ctx) + if err != nil { + return "", nil, err + } + + // Resolve, fetch and vet every change up front, before a single step is + // applied, so an unusable request cannot leave the checkout half-built. + refs, err := resolveStepChanges(steps) + if err != nil { + return baseSHA, nil, err + } + if err := m.ensureObjects(ctx, refs); err != nil { + return baseSHA, nil, err + } + if err := m.checkStale(ctx, refs); err != nil { + return baseSHA, nil, err + } + + stepResults, err := m.applySteps(ctx, steps) + if err != nil { + // The failing apply function aborts its own in-progress git operation; + // the next attempt starts with resetToRemote regardless. + return baseSHA, nil, err + } + + if commit { + if err := m.push(ctx); err != nil { + coremetrics.NamedCounter(m.metricsScope, "merge", "git_push_errors", 1) + return baseSHA, nil, err + } + } + return baseSHA, stepResults, nil +} + +// applySteps dispatches each step by its resolved strategy, in order, building +// up local HEAD and collecting one StepResult per step. +func (m *gitMerger) applySteps(ctx context.Context, steps []resolvedStep) ([]*runwaymq.StepResult, error) { + results := make([]*runwaymq.StepResult, 0, len(steps)) + for _, rs := range steps { + var ( + outputs []*runwaymq.StepOutput + err error + ) + switch rs.strategy { + case mergestrategypb.Strategy_REBASE: + outputs, err = m.applyRebase(ctx, rs.step) + default: + // resolveAndValidate rejects anything else; defensive. + return nil, fmt.Errorf("%w: unsupported strategy %v", merger.ErrInvalidRequest, rs.strategy) + } + if err != nil { + return nil, err + } + results = append(results, &runwaymq.StepResult{StepId: rs.step.GetStepId(), Outputs: outputs}) + } + return results, nil +} + +// applyRebase cherry-picks the head SHA of every URI of every change in the +// step, in order, returning one StepOutput per newly-created commit. +func (m *gitMerger) applyRebase(ctx context.Context, step *runwaymq.MergeStep) ([]*runwaymq.StepOutput, error) { + picked, err := m.pickStepChanges(ctx, step) + if err != nil { + return nil, err + } + return toOutputs(picked), nil +} + +// pickStepChanges applies every change in the step, in order, returning the +// SHAs of the commits created on the target (empty for a change whose content +// was already present). +func (m *gitMerger) pickStepChanges(ctx context.Context, step *runwaymq.MergeStep) ([]string, error) { + var picked []string + for _, uri := range step.GetChange().GetUris() { + ref, err := resolveChange(uri) + if err != nil { + return nil, err + } + created, err := m.pickRange(ctx, ref) + if err != nil { + return nil, err + } + picked = append(picked, created...) + } + return picked, nil +} + +// pickRange replays every commit the change introduces, not just its head. +// +// A change URI pins a pull request to its head commit, and a pull request is +// routinely more than one commit. Cherry-picking the head alone applies only +// that commit's diff against its own parent, which either conflicts against +// context the earlier commits would have established, or — worse, when the +// commits touch different files — succeeds while silently dropping everything +// before the head. The unit to replay is therefore the range from the change's +// merge base with the target up to its head. +func (m *gitMerger) pickRange(ctx context.Context, ref changeRef) ([]string, error) { + base, err := m.mergeBase(ctx, "HEAD", ref.SHA) + if err != nil { + return nil, err + } + if base == "" { + // No common ancestor: there is no range to compute, and replaying an + // imported history commit-by-commit would rewrite every SHA in it, + // which is the opposite of what such a request wants. + return nil, fmt.Errorf("%w: change %s shares no history with the merge target; use the MERGE strategy to integrate an unrelated history", + merger.ErrInvalidRequest, ref.Label) + } + + // A change wholly contained in the target has an empty range — its merge + // base with HEAD is its own head. That is the already-landed case, and it + // is a no-op rather than an error. + pending, err := m.revList(ctx, base, ref.SHA) + if err != nil { + return nil, err + } + if len(pending) == 0 { + return nil, nil + } + + preSHA, err := m.headSHA(ctx) + if err != nil { + return nil, err + } + if err := m.cherryPickRange(ctx, base, ref.SHA); err != nil { + return nil, err + } + return m.revList(ctx, preSHA, "HEAD") +} + +// cherryPickRange cherry-picks base..head, resuming past commits git declines +// to apply because they would be empty. +// +// A multi-commit range runs through git's sequencer, which changes the control +// flow versus a single pick: the operation stops on the offending commit and +// --skip advances to the next one rather than ending the operation. Two kinds +// of commit stop it harmlessly — one whose content is already on the target +// (the change was partially landed) and one that was empty to begin with — and +// both should be skipped rather than treated as failures. Anything else ends +// the attempt, and the in-progress pick is aborted so the checkout stays +// usable for the next request. +func (m *gitMerger) cherryPickRange(ctx context.Context, base, head string) error { + out, err := m.runCombined(ctx, nil, "cherry-pick", base+".."+head) + for err != nil { + if isRedundantCherryPick(out) { + out, err = m.runCombined(ctx, nil, "cherry-pick", "--skip") + continue + } + // Ask the index what happened before aborting clears it. A non-zero + // exit alone does not mean the change conflicts — a missing object, an + // unreadable repository or a killed process all land here too, and + // calling those conflicts tells the client its change collides with + // the target and permanently fails a request that a retry might well + // have completed. + conflicted := m.hasUnmergedPaths(ctx) + _, _ = m.run(ctx, nil, "cherry-pick", "--abort") + detail := strings.TrimSpace(string(out)) + if !conflicted { + coremetrics.NamedCounter(m.metricsScope, "merge", "cherry_pick_errors", 1) + return fmt.Errorf("git cherry-pick %s..%s: %w: %s", base, head, err, detail) + } + coremetrics.NamedCounter(m.metricsScope, "merge", "cherry_pick_conflicts", 1) + return fmt.Errorf("%w: git cherry-pick %s..%s: %s", merger.ErrConflict, base, head, detail) + } + return nil +} + +// hasUnmergedPaths reports whether the index holds conflicted entries. That is +// what separates a genuine content conflict from any other reason a git +// invocation exited non-zero, and it has to be read before the operation is +// aborted, since aborting clears the index. +func (m *gitMerger) hasUnmergedPaths(ctx context.Context) bool { + out, err := m.run(ctx, nil, "ls-files", "--unmerged") + return err == nil && len(bytes.TrimSpace(out)) > 0 +} + +// mergeBase returns the best common ancestor of two commits, or "" when they +// share no history. git exits 1 for the latter, which is an answer rather than +// a failure. +func (m *gitMerger) mergeBase(ctx context.Context, a, b string) (string, error) { + out, err := m.run(ctx, nil, "merge-base", a, b) + if err == nil { + return strings.TrimSpace(string(out)), nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return "", nil + } + return "", fmt.Errorf("git merge-base %s %s: %w", a, b, err) +} + +// revList lists the commits reachable from `to` but not from `from`, oldest +// first. The commits an apply produced are derived from the checkout this way +// rather than tracked per-invocation, which stays correct when the sequencer +// drops some of them. +func (m *gitMerger) revList(ctx context.Context, from, to string) ([]string, error) { + out, err := m.run(ctx, nil, "rev-list", "--reverse", from+".."+to) + if err != nil { + return nil, fmt.Errorf("git rev-list %s..%s: %w", from, to, err) + } + return strings.Fields(string(out)), nil +} + +// resetToRemote fetches the configured remote and hard-resets the checkout's +// HEAD to refs/remotes//, producing a clean working tree pinned +// to the latest remote tip. +func (m *gitMerger) resetToRemote(ctx context.Context) error { + if _, err := m.run(ctx, nil, "fetch", m.remote); err != nil { + return fmt.Errorf("git fetch %s: %w", m.remote, err) + } + // Deployment-supplied refspecs, for a server that refuses a fetch for an + // unadvertised object and so cannot satisfy the by-SHA fetch in + // ensureObject. Normally empty. + for _, refspec := range m.fetchRefspecs { + if _, err := m.run(ctx, nil, "fetch", m.remote, refspec); err != nil { + return fmt.Errorf("git fetch %s %s: %w", m.remote, refspec, err) + } + } + remoteRef := m.remote + "/" + m.target + if _, err := m.run(ctx, nil, "reset", "--hard", remoteRef); err != nil { + return fmt.Errorf("git reset --hard %s: %w", remoteRef, err) + } + if _, err := m.run(ctx, nil, "clean", "-fdx"); err != nil { + return fmt.Errorf("git clean: %w", err) + } + return nil +} + +// headSHA returns the SHA at HEAD in the local checkout. +func (m *gitMerger) headSHA(ctx context.Context) (string, error) { + out, err := m.run(ctx, nil, "rev-parse", "HEAD") + if err != nil { + return "", fmt.Errorf("git rev-parse HEAD: %w", err) + } + return strings.TrimSpace(string(out)), nil +} + +// refetchTipSHA fetches the remote and returns the current SHA at +// refs/remotes//. Used after a push failure to detect whether +// the remote tip moved under us. +func (m *gitMerger) refetchTipSHA(ctx context.Context) (string, error) { + if _, err := m.run(ctx, nil, "fetch", m.remote); err != nil { + return "", fmt.Errorf("git fetch %s: %w", m.remote, err) + } + remoteRef := m.remote + "/" + m.target + out, err := m.run(ctx, nil, "rev-parse", remoteRef) + if err != nil { + return "", fmt.Errorf("git rev-parse %s: %w", remoteRef, err) + } + return strings.TrimSpace(string(out)), nil +} + +// push pushes the current HEAD to refs/heads/ on the remote. +func (m *gitMerger) push(ctx context.Context) error { + refspec := "HEAD:refs/heads/" + m.target + if _, err := m.run(ctx, nil, "push", m.remote, refspec); err != nil { + return fmt.Errorf("git push %s %s: %w", m.remote, refspec, err) + } + return nil +} + +// run executes a `git` command in the checkout. Returns captured stdout and an +// error that includes captured stderr for diagnostics. +func (m *gitMerger) run(ctx context.Context, stdin []byte, args ...string) ([]byte, error) { + cmd := m.command(ctx, args...) + if stdin != nil { + cmd.Stdin = bytes.NewReader(stdin) + } + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String())) + } + return stdout.Bytes(), nil +} + +// runCombined is like run but returns combined stdout+stderr both on success +// and failure. Used when the caller needs to inspect git's diagnostic output. +func (m *gitMerger) runCombined(ctx context.Context, stdin []byte, args ...string) ([]byte, error) { + cmd := m.command(ctx, args...) + if stdin != nil { + cmd.Stdin = bytes.NewReader(stdin) + } + return cmd.CombinedOutput() +} + +// command builds a git command with the committer identity injected via -c +// flags, on top of the pinned runtime and scrubbed environment. +func (m *gitMerger) command(ctx context.Context, args ...string) *exec.Cmd { + withIdentity := make([]string, 0, len(args)+6) + withIdentity = append(withIdentity, + "-c", "user.name="+m.committerName, + "-c", "user.email="+m.committerEmail, + "-c", "commit.gpgsign=false", + ) + withIdentity = append(withIdentity, args...) + return newGitCommand(ctx, m.runtime, m.checkoutPath, withIdentity...) +} + +// newGitCommand constructs a Git command without inheriting the caller's +// environment. The executable and helper paths come from the pinned runtime; +// repository-local configuration remains an intentional input. +func newGitCommand(ctx context.Context, runtime GitRuntime, dir string, args ...string) *exec.Cmd { + gitArgs := make([]string, 0, len(args)+3) + gitArgs = append(gitArgs, + "--exec-path="+runtime.ExecPath, + "-c", "init.templateDir="+runtime.TemplateDir, + ) + gitArgs = append(gitArgs, args...) + + cmd := exec.CommandContext(ctx, runtime.Executable, gitArgs...) + cmd.Dir = dir + cmd.Env = []string{ + "HOME=" + filepath.Join(dir, ".submitqueue-git-home"), + "XDG_CONFIG_HOME=" + filepath.Join(dir, ".submitqueue-git-home", "xdg"), + "GIT_CONFIG_NOSYSTEM=1", + "GIT_CONFIG_GLOBAL=" + os.DevNull, + "GIT_ATTR_NOSYSTEM=1", + "GIT_TERMINAL_PROMPT=0", + "GIT_PAGER=cat", + "GIT_EDITOR=:", + "GIT_EXEC_PATH=" + runtime.ExecPath, + "GIT_TEMPLATE_DIR=" + runtime.TemplateDir, + "LC_ALL=C", + "LANG=C", + } + cmd.Env = append(cmd.Env, passthroughEnv(runtime.PassthroughEnv)...) + return cmd +} + +// authEnvNames are the variables inherited from the parent process when set. +// +// Scrubbing the environment is about denying git ambient *configuration* that +// could change what a merge produces. Reaching the remote is a separate +// concern, and these carry it: an SSH remote authenticates through the agent +// socket, git locates its ssh and credential helpers through PATH, and TLS and +// proxy settings decide whether an HTTPS remote is reachable at all. Dropping +// them does not make the merge more hermetic, it just makes fetch and push +// fail — and at Uber, where a uSSH certificate lives in the agent, it fails as +// an opaque authentication error rather than an obviously missing variable. +// +// None of these can influence merge semantics, which is what keeps them +// compatible with the scrubbing above. +var authEnvNames = []string{ + "SSH_AUTH_SOCK", + "SSH_AGENT_PID", + "PATH", + "GIT_SSH", + "GIT_SSH_COMMAND", + "GIT_SSH_VARIANT", + "GIT_SSL_CAINFO", + "GIT_SSL_CAPATH", + "SSL_CERT_DIR", + "SSL_CERT_FILE", + "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", + "http_proxy", "https_proxy", "no_proxy", +} + +// passthroughEnv returns "NAME=value" entries for the auth and transport +// variables that are actually set, plus any extra names the deployment asked +// for. An unset variable is omitted rather than exported empty, which for +// SSH_AUTH_SOCK is the difference between "use the agent" and "there is no +// agent". +func passthroughEnv(extra []string) []string { + names := make([]string, 0, len(authEnvNames)+len(extra)) + names = append(names, authEnvNames...) + names = append(names, extra...) + + seen := make(map[string]bool, len(names)) + env := make([]string, 0, len(names)) + for _, name := range names { + if name == "" || seen[name] { + continue + } + seen[name] = true + if v, ok := os.LookupEnv(name); ok { + env = append(env, name+"="+v) + } + } + return env +} + +// isConcreteStrategy reports whether s names a concrete integration strategy +// (i.e. not DEFAULT and not an unknown value). Only REBASE is implemented so +// far; the remaining strategies are rejected as invalid requests until their +// apply paths land. +func isConcreteStrategy(s mergestrategypb.Strategy) bool { + switch s { + case mergestrategypb.Strategy_REBASE: + return true + default: + return false + } +} + +// isRedundantCherryPick reports whether git's cherry-pick output indicates the +// pick was rejected because the change is already present on target. +func isRedundantCherryPick(out []byte) bool { + s := string(out) + return strings.Contains(s, "previous cherry-pick is now empty") || + strings.Contains(s, "nothing to commit") +} + +// toOutputs wraps commit SHAs as StepOutputs in order. +func toOutputs(shas []string) []*runwaymq.StepOutput { + if len(shas) == 0 { + return nil + } + outputs := make([]*runwaymq.StepOutput, 0, len(shas)) + for _, sha := range shas { + outputs = append(outputs, &runwaymq.StepOutput{Id: sha}) + } + return outputs +} + +// stripOutputs clears the Outputs of every step result — used for a dry-run +// check, which reports mergeability only. +func stripOutputs(steps []*runwaymq.StepResult) { + for _, s := range steps { + s.Outputs = nil + } +} + +// successResult builds a SUCCEEDED MergeResult echoing the request id. +func successResult(req *runwaymq.MergeRequest, steps []*runwaymq.StepResult) *runwaymq.MergeResult { + return &runwaymq.MergeResult{ + Id: req.GetId(), + Outcome: runwaypb.Outcome_SUCCEEDED, + Steps: steps, + } +} diff --git a/runway/extension/merger/git/git_merger_test.go b/runway/extension/merger/git/git_merger_test.go new file mode 100644 index 00000000..1c81cee1 --- /dev/null +++ b/runway/extension/merger/git/git_merger_test.go @@ -0,0 +1,1125 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package git + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber-go/tally" + "go.uber.org/zap/zaptest" + + changepb "github.com/uber/submitqueue/api/base/change/protopb" + mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" + runwaymq "github.com/uber/submitqueue/api/runway/messagequeue" + runwaypb "github.com/uber/submitqueue/api/runway/messagequeue/protopb" + "github.com/uber/submitqueue/runway/extension/merger" +) + +const pinnedGitVersion = "2.55.0" + +// fakeSHA is a syntactically valid 40-char hex SHA used by request-validation +// tests that never reach git (the request is rejected before any command runs). +const fakeSHA = "0123456789abcdef0123456789abcdef01234567" + +// gitFixture provides a bare "remote" repository plus a working checkout that +// the Merger operates on. Tests run real `git` commands so they exercise the +// same code path as production. +// +// The fixture also exposes helpers for pushing additional commits to side +// branches on the remote, so each test can build the SHAs it needs the Merger +// to apply. +type gitFixture struct { + root string + remoteDir string + checkoutDir string + authorDir string // a separate working clone used to author "PR" commits +} + +func setupGitFixture(t *testing.T) gitFixture { + t.Helper() + + root := t.TempDir() + remoteDir := filepath.Join(root, "remote.git") + checkoutDir := filepath.Join(root, "checkout") + authorDir := filepath.Join(root, "author") + + mustGit(t, root, "init", "--bare", "-b", "main", remoteDir) + + // Seed main with one initial commit so the Merger's reset/fetch flow has + // something to land on. + mustGit(t, root, "init", "-b", "main", authorDir) + configRepo(t, authorDir, "author", "author@example.com") + require.NoError(t, writeFile(filepath.Join(authorDir, "hello.txt"), "hello\nworld\n")) + mustGit(t, authorDir, "add", ".") + mustGit(t, authorDir, "commit", "-m", "seed") + mustGit(t, authorDir, "remote", "add", "origin", remoteDir) + mustGit(t, authorDir, "push", "origin", "main") + + mustGit(t, root, "clone", remoteDir, checkoutDir) + configRepo(t, checkoutDir, "checkout", "checkout@example.com") + + return gitFixture{ + root: root, + remoteDir: remoteDir, + checkoutDir: checkoutDir, + authorDir: authorDir, + } +} + +func TestGitRuntimeValidate(t *testing.T) { + abs, err := filepath.Abs("git") + require.NoError(t, err) + tests := []struct { + name string + runtime GitRuntime + wantErr bool + }{ + { + name: "valid", + runtime: GitRuntime{ + Executable: abs, + ExecPath: abs, + TemplateDir: abs, + }, + }, + { + name: "missing executable", + runtime: GitRuntime{ + ExecPath: abs, + TemplateDir: abs, + }, + wantErr: true, + }, + { + name: "relative exec path", + runtime: GitRuntime{ + Executable: abs, + ExecPath: "git-core", + TemplateDir: abs, + }, + wantErr: true, + }, + { + name: "relative template dir", + runtime: GitRuntime{ + Executable: abs, + ExecPath: abs, + TemplateDir: "templates", + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.runtime.validate() + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} + +func TestNewGitCommandDoesNotInheritEnvironment(t *testing.T) { + t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient") + runtime := testGitRuntime(t) + + cmd := newGitCommand(context.Background(), runtime, t.TempDir(), "--version") + + assert.Equal(t, runtime.Executable, cmd.Path) + assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient") + assert.Contains(t, cmd.Env, "GIT_CONFIG_NOSYSTEM=1") + assert.Contains(t, cmd.Env, "GIT_EXEC_PATH="+runtime.ExecPath) +} + +func TestNewGitCommandPreservesAuthEnvironment(t *testing.T) { + // Scrubbing denies git ambient configuration; it must not also deny it the + // means to reach the remote. Without the agent socket an SSH remote cannot + // authenticate, and without PATH git cannot even exec ssh. + t.Setenv("SSH_AUTH_SOCK", "/tmp/agent.sock") + t.Setenv("PATH", "/usr/bin:/bin") + t.Setenv("HTTPS_PROXY", "http://proxy.example.com:3128") + t.Setenv("SUBMITQUEUE_GIT_AMBIENT", "ambient") + + cmd := newGitCommand(context.Background(), testGitRuntime(t), t.TempDir(), "--version") + + assert.Contains(t, cmd.Env, "SSH_AUTH_SOCK=/tmp/agent.sock") + assert.Contains(t, cmd.Env, "PATH=/usr/bin:/bin") + assert.Contains(t, cmd.Env, "HTTPS_PROXY=http://proxy.example.com:3128") + assert.NotContains(t, cmd.Env, "SUBMITQUEUE_GIT_AMBIENT=ambient") +} + +func TestNewGitCommandOmitsUnsetAuthEnvironment(t *testing.T) { + // An unset variable must be omitted rather than exported empty: an empty + // SSH_AUTH_SOCK tells ssh there is no agent instead of letting it look. + t.Setenv("SSH_AUTH_SOCK", "") + require.NoError(t, os.Unsetenv("SSH_AUTH_SOCK")) + + cmd := newGitCommand(context.Background(), testGitRuntime(t), t.TempDir(), "--version") + + for _, entry := range cmd.Env { + assert.False(t, strings.HasPrefix(entry, "SSH_AUTH_SOCK="), "unset variable leaked as %q", entry) + } +} + +func TestNewGitCommandPassesThroughExtraEnvironment(t *testing.T) { + t.Setenv("SUBMITQUEUE_CUSTOM_TRANSPORT", "value") + runtime := testGitRuntime(t) + runtime.PassthroughEnv = []string{"SUBMITQUEUE_CUSTOM_TRANSPORT"} + + cmd := newGitCommand(context.Background(), runtime, t.TempDir(), "--version") + + assert.Contains(t, cmd.Env, "SUBMITQUEUE_CUSTOM_TRANSPORT=value") +} + +func TestCherryPickRange_NonConflictFailureIsRetryable(t *testing.T) { + // A cherry-pick can exit non-zero for reasons that have nothing to do with + // the change colliding — a bad revision here, standing in for a missing + // object or an unreadable repository. Reporting those as ErrConflict would + // permanently fail a request that a retry could complete. + f := setupGitFixture(t) + m := f.newMerger(t, mergestrategypb.Strategy_REBASE).(*gitMerger) + + err := m.cherryPickRange(context.Background(), "notarevision", "alsonotarevision") + require.Error(t, err) + assert.False(t, errors.Is(err, merger.ErrConflict), "a non-conflict failure must stay retryable") + assert.False(t, errors.Is(err, merger.ErrInvalidRequest)) +} + +func TestCherryPickRange_RealConflictIsErrConflict(t *testing.T) { + f := setupGitFixture(t) + mainBefore, conflictingSHA := f.setupConflict(t) + require.NotEmpty(t, mainBefore) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE).(*gitMerger) + require.NoError(t, m.resetToRemote(context.Background())) + require.NoError(t, m.ensureObjects(context.Background(), []changeRef{{SHA: conflictingSHA}})) + + err := m.cherryPickRange(context.Background(), conflictingSHA+"^", conflictingSHA) + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrConflict)) + assert.True(t, f.checkoutClean(t), "the aborted pick must leave no conflicted index behind") +} + +// --- REBASE --- + +func TestMerge_Rebase_SingleChangeSingleURI(t *testing.T) { + f := setupGitFixture(t) + sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(sha)))) + require.NoError(t, err) + assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome()) + require.Len(t, res.GetSteps(), 1) + require.Len(t, res.GetSteps()[0].GetOutputs(), 1) + + out := res.GetSteps()[0].GetOutputs()[0] + assert.Equal(t, []string{out.GetId()}, f.remoteCommitsSinceSeed(t)) + assert.Equal(t, "hello\nearth\n", f.remoteFile(t, "hello.txt")) +} + +func TestMerge_Rebase_StackedURIsInOneChange(t *testing.T) { + f := setupGitFixture(t) + sha1, sha2 := f.pushStack(t) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", + stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(sha1), uri(sha2)), + )) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 1) + outputs := res.GetSteps()[0].GetOutputs() + require.Len(t, outputs, 2) + + assert.Equal(t, []string{outputs[0].GetId(), outputs[1].GetId()}, f.remoteCommitsSinceSeed(t)) + assert.Equal(t, "hello\nearth\ngoodbye\n", f.remoteFile(t, "hello.txt")) +} + +func TestMerge_Rebase_AlreadyLandedChange(t *testing.T) { + f := setupGitFixture(t) + sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + + // Land the same content on main outside the Merger so the cherry-pick + // finds nothing new to add. + f.landOnMain(t, sha) + mainBefore := f.remoteHEAD(t) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(sha)))) + require.NoError(t, err) + assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome()) + require.Len(t, res.GetSteps(), 1) + assert.Empty(t, res.GetSteps()[0].GetOutputs()) + assert.Equal(t, mainBefore, f.remoteHEAD(t), + "rebased-out merge should not advance the remote tip") +} + +func TestMerge_Rebase_MixedChanges(t *testing.T) { + t.Run("two steps", func(t *testing.T) { + f := setupGitFixture(t) + subsumedSHA := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + f.landOnMain(t, subsumedSHA) + freshSHA := f.pushPRCommit(t, "feature/b", "extra.txt", "extra\n", "add extra") + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", + stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(subsumedSHA)), + stepOf(mergestrategypb.Strategy_REBASE, "s2", uri(freshSHA)), + )) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 2) + assert.Empty(t, res.GetSteps()[0].GetOutputs()) + require.Len(t, res.GetSteps()[1].GetOutputs(), 1) + assert.Equal(t, "extra\n", f.remoteFile(t, "extra.txt")) + }) + + t.Run("one change two URIs", func(t *testing.T) { + f := setupGitFixture(t) + subsumedSHA := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + f.landOnMain(t, subsumedSHA) + freshSHA := f.pushPRCommit(t, "feature/b", "extra.txt", "extra\n", "add extra") + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", + stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(subsumedSHA), uri(freshSHA)), + )) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 1) + // Only the fresh URI produced a commit; the subsumed one is a no-op. + require.Len(t, res.GetSteps()[0].GetOutputs(), 1) + assert.Equal(t, "extra\n", f.remoteFile(t, "extra.txt")) + }) +} + +func TestMerge_Rebase_Conflict(t *testing.T) { + f := setupGitFixture(t) + mainBefore, conflictingSHA := f.setupConflict(t) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + _, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(conflictingSHA)))) + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrConflict)) + assert.Equal(t, mainBefore, f.remoteHEAD(t), "on conflict the remote tip must not move") +} + +func TestMerge_Rebase_ResetsBetweenCalls(t *testing.T) { + f := setupGitFixture(t) + sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + + // Dirty the checkout so that, without a reset, subsequent operations would + // fail or include unrelated changes. + require.NoError(t, writeFile(filepath.Join(f.checkoutDir, "stray.txt"), "leftover\n")) + + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(sha)))) + require.NoError(t, err) + require.Len(t, res.GetSteps()[0].GetOutputs(), 1) + + landed := res.GetSteps()[0].GetOutputs()[0].GetId() + out := mustGitOutput(t, f.remoteDir, "ls-tree", "--name-only", landed) + assert.NotContains(t, string(out), "stray.txt", "unrelated file should not have landed") + assert.Contains(t, string(out), "hello.txt") +} + +func TestMerge_Rebase_RecoversAfterPriorConflict(t *testing.T) { + f := setupGitFixture(t) + _, conflictingSHA := f.setupConflict(t) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + _, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(conflictingSHA)))) + require.Error(t, err) + + // A subsequent, clean merge must succeed even though the prior call left a + // cherry-pick in progress before its rollback. + freshSHA := f.pushPRCommit(t, "feature/c", "extra.txt", "extra\n", "add extra") + res, err := m.Merge(context.Background(), req("b2", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(freshSHA)))) + require.NoError(t, err) + assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome()) + assert.Equal(t, "extra\n", f.remoteFile(t, "extra.txt")) +} + +func TestMerge_Rebase_RetriesWhenRemoteMovesUnderUs(t *testing.T) { + f := setupGitFixture(t) + // Pre-stage one race commit, install the hook, then build the feature + // commit. Order matters: pushPRCommit also goes through the hook, so race + + // feature must be on the remote before the hook is armed. + raceSHA := f.pushPRCommit(t, "race", "race.txt", "race\n", "race commit") + featureSHA := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + f.installRaceHook(t, []string{raceSHA}) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(featureSHA)))) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 1) + require.Len(t, res.GetSteps()[0].GetOutputs(), 1) + + assert.Equal(t, 2, f.hookInvocations(t), + "first attempt rejected by hook, second attempt allowed through") + + commits := f.remoteCommitsSinceSeed(t) + require.Len(t, commits, 2) + assert.Equal(t, raceSHA, commits[0], "race commit landed first via the hook") + assert.Equal(t, res.GetSteps()[0].GetOutputs()[0].GetId(), commits[1], + "our cherry-pick landed on top after the retry") + assert.Equal(t, "hello\nearth\n", f.remoteFile(t, "hello.txt")) +} + +func TestMerge_Rebase_GivesUpAfterMaxAttempts(t *testing.T) { + f := setupGitFixture(t) + raceSHAs := []string{ + f.pushPRCommit(t, "race1", "r1.txt", "1\n", "r1"), + f.pushPRCommit(t, "race2", "r2.txt", "2\n", "r2"), + } + featureSHA := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + f.installRaceHook(t, raceSHAs) + + m, err := NewMerger(Params{ + CheckoutPath: f.checkoutDir, + Remote: "origin", + Target: "main", + DefaultStrategy: mergestrategypb.Strategy_REBASE, + Runtime: testGitRuntime(t), + MaxPushAttempts: 2, + Logger: zaptest.NewLogger(t).Sugar(), + MetricsScope: tally.NoopScope, + CommitterName: "Test", + CommitterEmail: "test@example.com", + }) + require.NoError(t, err) + + _, err = m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(featureSHA)))) + require.Error(t, err) + assert.False(t, errors.Is(err, merger.ErrConflict)) + assert.Equal(t, 2, f.hookInvocations(t), "both attempts hit the hook before the cap kicked in") + // The remote ended up at race2 (the last hook injection), and our feature + // commit never landed. + assert.Equal(t, raceSHAs[1], f.remoteHEAD(t)) +} + +func TestMerge_Default_ResolvesToRebase(t *testing.T) { + f := setupGitFixture(t) + sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_DEFAULT, "s1", uri(sha)))) + require.NoError(t, err) + assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome()) + require.Len(t, res.GetSteps(), 1) + require.Len(t, res.GetSteps()[0].GetOutputs(), 1, "DEFAULT resolves to REBASE: one output per fresh change") + assert.Equal(t, []string{res.GetSteps()[0].GetOutputs()[0].GetId()}, f.remoteCommitsSinceSeed(t)) +} + +// --- multi-commit changes (one URI, many commits) --- +// +// A change URI pins a pull request to a single head SHA, so these are the tests +// that actually reflect the wire contract: however many commits the change +// contains, the request names one. Passing the intermediate SHAs as extra URIs +// would be testing a shape production cannot produce. + +func TestMerge_Rebase_MultiCommitChangeSingleURI(t *testing.T) { + f := setupGitFixture(t) + head := f.pushMultiCommitPR(t, "feature/multi", + commitSpec{"a.txt", "a\n", "add a"}, + commitSpec{"b.txt", "b\n", "add b"}, + commitSpec{"c.txt", "c\n", "add c"}, + ) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(head)))) + require.NoError(t, err) + assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome()) + + // Every commit of the change lands, not just the one the URI names. + require.Len(t, res.GetSteps(), 1) + assert.Len(t, res.GetSteps()[0].GetOutputs(), 3) + assert.Equal(t, "a\n", f.remoteFile(t, "a.txt")) + assert.Equal(t, "b\n", f.remoteFile(t, "b.txt")) + assert.Equal(t, "c\n", f.remoteFile(t, "c.txt")) +} + +func TestMerge_Rebase_MultiCommitChangeSharedFile(t *testing.T) { + // Commits that build on each other in the same region: applying only the + // head would conflict against context its predecessors establish. + f := setupGitFixture(t) + head := f.pushMultiCommitPR(t, "feature/shared", + commitSpec{"hello.txt", "hello\nworld\nearth\n", "add earth"}, + commitSpec{"hello.txt", "hello\nworld\nearth\nmars\n", "add mars"}, + ) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(head)))) + require.NoError(t, err) + assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome()) + assert.Equal(t, "hello\nworld\nearth\nmars\n", f.remoteFile(t, "hello.txt")) +} + +func TestMerge_Rebase_PartiallyLandedChange(t *testing.T) { + // The change's first commit is already on the target. The range still + // covers it; git declines it as redundant and the rest is applied. + f := setupGitFixture(t) + head := f.pushMultiCommitPR(t, "feature/partial", + commitSpec{"a.txt", "a\n", "add a"}, + commitSpec{"b.txt", "b\n", "add b"}, + ) + first := strings.TrimSpace(string(mustGitOutput(t, f.authorDir, "rev-parse", head+"^"))) + f.landOnMain(t, first) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(head)))) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 1) + assert.Len(t, res.GetSteps()[0].GetOutputs(), 1, "only the not-yet-landed commit produces output") + assert.Equal(t, "a\n", f.remoteFile(t, "a.txt")) + assert.Equal(t, "b\n", f.remoteFile(t, "b.txt")) +} + +func TestMerge_Rebase_EmptyCommitInRange(t *testing.T) { + f := setupGitFixture(t) + head := f.pushMultiCommitPR(t, "feature/empty", + commitSpec{"a.txt", "a\n", "add a"}, + commitSpec{message: "an empty commit"}, + commitSpec{"b.txt", "b\n", "add b"}, + ) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(head)))) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 1) + assert.Len(t, res.GetSteps()[0].GetOutputs(), 2, "the empty commit is skipped, not landed") + assert.Equal(t, "a\n", f.remoteFile(t, "a.txt")) + assert.Equal(t, "b\n", f.remoteFile(t, "b.txt")) +} + +func TestMerge_Rebase_ConflictInRangeLeavesCheckoutUsable(t *testing.T) { + f := setupGitFixture(t) + head := f.pushMultiCommitPR(t, "feature/conflicting", + commitSpec{"a.txt", "a\n", "add a"}, + commitSpec{"hello.txt", "hello\nmars\n", "diverge hello"}, + ) + // Land a conflicting edit to the same file outside the merger. + other := f.pushPRCommit(t, "feature/other", "hello.txt", "hello\nvenus\n", "venus") + f.landOnMain(t, other) + mainBefore := f.remoteHEAD(t) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + _, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(head)))) + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrConflict)) + assert.Equal(t, mainBefore, f.remoteHEAD(t), "a conflicting range must not advance the remote") + assert.True(t, f.checkoutClean(t), "the aborted range must leave no sequencer state behind") + + // The checkout is still usable for the next request. + clean := f.pushPRCommit(t, "feature/clean", "z.txt", "z\n", "add z") + _, err = m.Merge(context.Background(), req("c", stepOf(mergestrategypb.Strategy_REBASE, "s2", uri(clean)))) + require.NoError(t, err) +} + +func TestMerge_Rebase_StackedMultiCommitChanges(t *testing.T) { + // Change B is authored on top of change A, and both carry several commits. + // B's range reaches back over A's commits; they are redundant by the time B + // applies, so they must be skipped rather than duplicated. + f := setupGitFixture(t) + aHead := f.pushMultiCommitPR(t, "feature/a", + commitSpec{"a1.txt", "a1\n", "a1"}, + commitSpec{"a2.txt", "a2\n", "a2"}, + ) + bHead := f.pushMultiCommitPRFrom(t, aHead, "feature/b", + commitSpec{"b1.txt", "b1\n", "b1"}, + commitSpec{"b2.txt", "b2\n", "b2"}, + ) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", + stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(aHead)), + stepOf(mergestrategypb.Strategy_REBASE, "s2", uri(bHead)), + )) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 2) + assert.Len(t, res.GetSteps()[0].GetOutputs(), 2) + assert.Len(t, res.GetSteps()[1].GetOutputs(), 2, "A's commits must not be replayed inside B's range") + assert.Len(t, f.remoteCommitsSinceSeed(t), 4) + for _, name := range []string{"a1.txt", "a2.txt", "b1.txt", "b2.txt"} { + assert.NotEmpty(t, f.remoteFile(t, name), name) + } +} + +// --- object availability and staleness --- + +func TestMerge_UnavailableCommitIsInvalidRequestNotConflict(t *testing.T) { + // A commit the remote cannot supply is a request we can never satisfy. It + // must not be reported as a merge conflict: that would tell the client its + // change conflicts when nothing was ever applied. + f := setupGitFixture(t) + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + + _, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(fakeSHA)))) + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrInvalidRequest)) + assert.False(t, errors.Is(err, merger.ErrConflict)) +} + +func TestMerge_ChangeReachableOnlyViaPRRef(t *testing.T) { + // The head is published as a pull-request ref and never as a branch, the + // normal shape for a fork PR. The default fetch refspec does not cover it. + f := setupGitFixture(t) + head := f.pushMultiCommitPR(t, "feature/forked", + commitSpec{"a.txt", "a\n", "add a"}, + commitSpec{"b.txt", "b\n", "add b"}, + ) + f.publishPRRef(t, 1, head) + mustGit(t, f.authorDir, "push", "origin", "--delete", "feature/forked") + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(head)))) + require.NoError(t, err) + require.Len(t, res.GetSteps(), 1) + assert.Len(t, res.GetSteps()[0].GetOutputs(), 2) +} + +func TestMerge_StaleChangeRejected(t *testing.T) { + f := setupGitFixture(t) + stale := f.pushPRCommit(t, "feature/s", "s.txt", "v1\n", "v1") + f.publishPRRef(t, 1, stale) + // The author force-pushes: the PR head moves on, but the old commit is + // still fetchable, so only an explicit check catches it. + current := f.pushPRCommit(t, "feature/s2", "s.txt", "v2\n", "v2") + f.publishPRRef(t, 1, current) + + m := f.newMergerWith(t, func(p *Params) { p.CheckStaleness = true }) + _, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(stale)))) + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrInvalidRequest)) + + // The same request is accepted when the URI names the current head. + _, err = m.Merge(context.Background(), req("c", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(current)))) + require.NoError(t, err) +} + +func TestMerge_StalenessCheckOffByDefault(t *testing.T) { + f := setupGitFixture(t) + stale := f.pushPRCommit(t, "feature/s", "s.txt", "v1\n", "v1") + f.publishPRRef(t, 1, stale) + current := f.pushPRCommit(t, "feature/s2", "t.txt", "v2\n", "v2") + f.publishPRRef(t, 1, current) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + _, err := m.Merge(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(stale)))) + require.NoError(t, err) +} + +// --- change provider resolution --- + +func TestResolveChange(t *testing.T) { + tests := []struct { + name string + uri string + wantSHA string + wantRef string + wantLabel string + wantErr bool + }{ + { + name: "github pull request", + uri: "github://github.example.com/uber/submitqueue/pull/42/" + fakeSHA, + wantSHA: fakeSHA, + wantRef: "refs/pull/42/head", + wantLabel: "uber/submitqueue#42", + }, + { + name: "git ref", + uri: "git://git.example.com/uber/monorepo/refs%2Fheads%2Fmain/" + fakeSHA, + wantSHA: fakeSHA, + wantRef: "refs/heads/main", + wantLabel: "uber/monorepo@refs/heads/main", + }, + {name: "unsupported scheme", uri: "phab://phab.example.com/D123/456", wantErr: true}, + {name: "no scheme", uri: "not-a-uri", wantErr: true}, + {name: "malformed github", uri: "github://github.example.com/nope", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := resolveChange(tt.uri) + if tt.wantErr { + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrInvalidRequest)) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantSHA, got.SHA) + assert.Equal(t, tt.wantRef, got.Ref) + assert.Equal(t, tt.wantLabel, got.Label) + }) + } +} + +// --- invalid requests --- + +func TestMerge_InvalidRequests(t *testing.T) { + tests := []struct { + name string + req *runwaymq.MergeRequest + }{ + { + name: "no steps", + req: req("b"), + }, + { + name: "unsupported strategy", + req: req("b", stepOf(mergestrategypb.Strategy_MERGE, "s1", uri(fakeSHA))), + }, + { + name: "malformed URI", + req: req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", "not a uri")), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := setupGitFixture(t) + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + _, err := m.Merge(context.Background(), tt.req) + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrInvalidRequest)) + }) + } +} + +// --- CheckMergeability (dry-run) --- + +func TestCheckMergeability_RebaseMergeable(t *testing.T) { + f := setupGitFixture(t) + sha := f.pushPRCommit(t, "feature/a", "hello.txt", "hello\nearth\n", "tweak hello") + mainBefore := f.remoteHEAD(t) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + res, err := m.CheckMergeability(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(sha)))) + require.NoError(t, err) + assert.Equal(t, runwaypb.Outcome_SUCCEEDED, res.GetOutcome()) + require.Len(t, res.GetSteps(), 1) + assert.Empty(t, res.GetSteps()[0].GetOutputs(), "a dry run reports no outputs") + assert.Equal(t, mainBefore, f.remoteHEAD(t), "a dry run does not advance the remote tip") +} + +func TestCheckMergeability_Conflict(t *testing.T) { + f := setupGitFixture(t) + mainBefore, conflictingSHA := f.setupConflict(t) + + m := f.newMerger(t, mergestrategypb.Strategy_REBASE) + _, err := m.CheckMergeability(context.Background(), req("b", stepOf(mergestrategypb.Strategy_REBASE, "s1", uri(conflictingSHA)))) + require.Error(t, err) + assert.True(t, errors.Is(err, merger.ErrConflict)) + assert.Equal(t, mainBefore, f.remoteHEAD(t)) +} + +func TestPinnedGitVersion(t *testing.T) { + out := mustGitOutput(t, t.TempDir(), "--version") + assert.Equal(t, "git version "+pinnedGitVersion, strings.TrimSpace(string(out))) +} + +// --- request builders --- + +// req builds a MergeRequest with the given correlation id and ordered steps. +func req(id string, steps ...*runwaymq.MergeStep) *runwaymq.MergeRequest { + return &runwaymq.MergeRequest{Id: id, Steps: steps} +} + +// stepOf builds a step whose change carries the given URIs. +func stepOf(strategy mergestrategypb.Strategy, stepID string, uris ...string) *runwaymq.MergeStep { + return &runwaymq.MergeStep{ + StepId: stepID, + Strategy: strategy, + Change: &changepb.Change{Uris: uris}, + } +} + +// --- merger + fixture helpers --- + +// commitSpec describes one commit to build on a change branch. +type commitSpec struct { + path string + contents string + message string +} + +// pushMultiCommitPR builds a branch of several commits off the current +// origin/main and returns ONLY the head SHA — which is all a change URI ever +// carries, however many commits the change actually contains. Tests that pass +// the intermediate SHAs separately are not exercising the real contract. +func (f gitFixture) pushMultiCommitPR(t *testing.T, branch string, commits ...commitSpec) string { + t.Helper() + return f.pushMultiCommitPRFrom(t, "origin/main", branch, commits...) +} + +// pushMultiCommitPRFrom is pushMultiCommitPR based at an explicit start point, +// for building a change that stacks on another change rather than on trunk. +func (f gitFixture) pushMultiCommitPRFrom(t *testing.T, base, branch string, commits ...commitSpec) string { + t.Helper() + mustGit(t, f.authorDir, "fetch", "origin") + mustGit(t, f.authorDir, "checkout", "-B", branch, base) + for _, c := range commits { + if c.path == "" { + mustGit(t, f.authorDir, "commit", "--allow-empty", "-m", c.message) + continue + } + require.NoError(t, writeFile(filepath.Join(f.authorDir, c.path), c.contents)) + mustGit(t, f.authorDir, "add", ".") + mustGit(t, f.authorDir, "commit", "-m", c.message) + } + mustGit(t, f.authorDir, "push", "-f", "origin", branch) + return strings.TrimSpace(string(mustGitOutput(t, f.authorDir, "rev-parse", "HEAD"))) +} + +// publishPRRef points refs/pull//head at sha on the remote, mirroring how a +// host publishes a pull request head. Needed by anything exercising the +// canonical-ref fallback or the staleness check. +func (f gitFixture) publishPRRef(t *testing.T, n int, sha string) { + t.Helper() + mustGit(t, f.authorDir, "push", "-f", "origin", + fmt.Sprintf("%s:refs/pull/%d/head", sha, n)) +} + +// newMergerWith builds a Merger with non-default Params, for the options that +// only some tests exercise. +func (f gitFixture) newMergerWith(t *testing.T, mutate func(*Params)) merger.Merger { + t.Helper() + p := Params{ + CheckoutPath: f.checkoutDir, + Remote: "origin", + Target: "main", + DefaultStrategy: mergestrategypb.Strategy_REBASE, + Runtime: testGitRuntime(t), + Logger: zaptest.NewLogger(t).Sugar(), + MetricsScope: tally.NoopScope, + CommitterName: "Test", + CommitterEmail: "test@example.com", + } + mutate(&p) + m, err := NewMerger(p) + require.NoError(t, err) + return m +} + +// checkoutClean reports whether the merger's checkout has no in-progress +// operation and no dirty state — what must hold after any failed apply. +func (f gitFixture) checkoutClean(t *testing.T) bool { + t.Helper() + out := mustGitOutput(t, f.checkoutDir, "status", "--porcelain") + if strings.TrimSpace(string(out)) != "" { + return false + } + for _, marker := range []string{"CHERRY_PICK_HEAD", "MERGE_HEAD", "sequencer"} { + if _, err := os.Stat(filepath.Join(f.checkoutDir, ".git", marker)); err == nil { + return false + } + } + return true +} + +func (f gitFixture) newMerger(t *testing.T, defaultStrategy mergestrategypb.Strategy) merger.Merger { + t.Helper() + m, err := NewMerger(Params{ + CheckoutPath: f.checkoutDir, + Remote: "origin", + Target: "main", + DefaultStrategy: defaultStrategy, + Runtime: testGitRuntime(t), + Logger: zaptest.NewLogger(t).Sugar(), + MetricsScope: tally.NoopScope, + CommitterName: "Test", + CommitterEmail: "test@example.com", + }) + require.NoError(t, err) + return m +} + +// configRepo applies the test-only config that lets git commit work in a +// sandbox without GPG signing, system identity, or system git hooks. The devpod +// environment installs hooks via the system git config (core.hooksPath = +// /etc/git-hooks) that interfere with test commits — point each test repo at an +// empty hooks dir to disarm them without resorting to --no-verify. +func configRepo(t *testing.T, dir, name, email string) { + mustGit(t, dir, "config", "user.name", name) + mustGit(t, dir, "config", "user.email", email) + mustGit(t, dir, "config", "commit.gpgsign", "false") + mustGit(t, dir, "config", "tag.gpgsign", "false") + + hooksDir := filepath.Join(dir, ".no-hooks") + require.NoError(t, os.MkdirAll(hooksDir, 0o755)) + mustGit(t, dir, "config", "core.hooksPath", hooksDir) +} + +// pushPRCommit creates a single commit on a feature branch on the remote +// branched off the *current* origin/main, returning the resulting SHA. +func (f gitFixture) pushPRCommit(t *testing.T, branch, path, contents, message string) string { + t.Helper() + mustGit(t, f.authorDir, "fetch", "origin") + return f.pushPRCommitFrom(t, "origin/main", branch, path, contents, message) +} + +// pushPRCommitFrom creates a single commit on `branch` based on the given base +// ref, returning the resulting SHA. Use this to create branches that diverge +// from a specific ancestor (so they conflict when cherry-picked onto a newer +// main). +func (f gitFixture) pushPRCommitFrom(t *testing.T, base, branch, path, contents, message string) string { + t.Helper() + mustGit(t, f.authorDir, "checkout", "-B", branch, base) + require.NoError(t, writeFile(filepath.Join(f.authorDir, path), contents)) + mustGit(t, f.authorDir, "add", ".") + mustGit(t, f.authorDir, "commit", "-m", message) + mustGit(t, f.authorDir, "push", "-f", "origin", branch) + out := mustGitOutput(t, f.authorDir, "rev-parse", "HEAD") + return strings.TrimSpace(string(out)) +} + +// pushStack builds a two-commit stack on a single branch (so the second SHA's +// parent is the first SHA) and returns both SHAs in order. +func (f gitFixture) pushStack(t *testing.T) (string, string) { + t.Helper() + mustGit(t, f.authorDir, "fetch", "origin") + mustGit(t, f.authorDir, "checkout", "-B", "feature/stack", "origin/main") + + require.NoError(t, writeFile(filepath.Join(f.authorDir, "hello.txt"), "hello\nearth\n")) + mustGit(t, f.authorDir, "add", ".") + mustGit(t, f.authorDir, "commit", "-m", "step 1") + sha1 := strings.TrimSpace(string(mustGitOutput(t, f.authorDir, "rev-parse", "HEAD"))) + + require.NoError(t, writeFile(filepath.Join(f.authorDir, "hello.txt"), "hello\nearth\ngoodbye\n")) + mustGit(t, f.authorDir, "add", ".") + mustGit(t, f.authorDir, "commit", "-m", "step 2") + sha2 := strings.TrimSpace(string(mustGitOutput(t, f.authorDir, "rev-parse", "HEAD"))) + + mustGit(t, f.authorDir, "push", "-f", "origin", "feature/stack") + return sha1, sha2 +} + +// setupConflict lands "earth" on main and returns the pre-merge main SHA plus a +// divergent "mars" SHA that conflicts when applied onto the new main. +func (f gitFixture) setupConflict(t *testing.T) (mainBefore, conflictingSHA string) { + t.Helper() + seedSHA := f.remoteSHA(t, "main") + earthSHA := f.pushPRCommitFrom(t, seedSHA, "feature/a", "hello.txt", "hello\nearth\n", "earth") + f.landOnMain(t, earthSHA) + mainBefore = f.remoteHEAD(t) + conflictingSHA = f.pushPRCommitFrom(t, seedSHA, "feature/b", "hello.txt", "hello\nmars\n", "mars") + return mainBefore, conflictingSHA +} + +// remoteSHA returns the SHA at refs/heads/ on the bare remote. +func (f gitFixture) remoteSHA(t *testing.T, branch string) string { + t.Helper() + out := mustGitOutput(t, f.remoteDir, "rev-parse", branch) + return strings.TrimSpace(string(out)) +} + +// landOnMain cherry-picks an arbitrary SHA directly onto main on the remote, +// simulating "this content is already on the target branch" for rebased-out +// tests. +func (f gitFixture) landOnMain(t *testing.T, sha string) { + t.Helper() + mustGit(t, f.authorDir, "fetch", "origin") + mustGit(t, f.authorDir, "checkout", "-B", "land", "origin/main") + mustGit(t, f.authorDir, "cherry-pick", sha) + mustGit(t, f.authorDir, "push", "origin", "land:main") +} + +// advanceMain fast-forwards main on the remote directly to sha (which must be a +// fast-forward of the current tip), used to pre-place a commit on the target. +func (f gitFixture) advanceMain(t *testing.T, sha string) { + t.Helper() + mustGit(t, f.authorDir, "fetch", "origin") + mustGit(t, f.authorDir, "push", "origin", sha+":main") +} + +// uri builds a github-format URI ending in `sha` so the Merger's parser resolves +// it to that SHA. +func uri(sha string) string { + return fmt.Sprintf("github://github.example.com/uber/submitqueue/pull/1/%s", sha) +} + +// installRaceHook writes a pre-receive hook on the bare remote that simulates +// concurrent pushes. On its Nth invocation it reads the Nth line of race-shas, +// points refs/heads/main at that SHA via update-ref, and exits 1 (rejecting the +// current push). Once race-shas is exhausted it exits 0 and the push goes +// through. +// +// Combined with the gitMerger's contention-detection (refetch + compare base +// SHA), this lets a test drive the full retry loop using only real git +// mechanics — the second-attempt's reset picks up the moved tip and proceeds +// from there. +func (f gitFixture) installRaceHook(t *testing.T, raceSHAs []string) { + t.Helper() + hookDir := filepath.Join(f.remoteDir, "hooks") + require.NoError(t, os.MkdirAll(hookDir, 0o755)) + // Override the system-wide core.hooksPath so the hook we just wrote actually + // fires on the bare remote (the devpod sets a global /etc/git-hooks + // directory that would otherwise win). + mustGit(t, f.remoteDir, "config", "core.hooksPath", hookDir) + require.NoError(t, writeFile( + filepath.Join(hookDir, "race-shas"), + strings.Join(raceSHAs, "\n")+"\n", + )) + const script = `#!/bin/sh +counter_file="$GIT_DIR/hooks/race-counter" +race_sha_file="$GIT_DIR/hooks/race-shas" +count=$(cat "$counter_file" 2>/dev/null || echo 0) +count=$((count + 1)) +echo "$count" > "$counter_file" +next_sha=$(sed -n "${count}p" "$race_sha_file") +if [ -z "$next_sha" ]; then + exit 0 +fi +# Pre-receive runs in git's quarantine env; unset its markers so update-ref is +# allowed to mutate the live ref store. +unset GIT_QUARANTINE_PATH GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES +"$GIT_EXEC_PATH/git" update-ref refs/heads/main "$next_sha" +echo "race hook moved main to $next_sha and rejected push" >&2 +exit 1 +` + hookPath := filepath.Join(hookDir, "pre-receive") + require.NoError(t, os.WriteFile(hookPath, []byte(script), 0o755)) +} + +// hookInvocations returns the number of times the pre-receive race hook has +// fired. Used by retry tests to verify the loop ran the expected number of +// attempts. +func (f gitFixture) hookInvocations(t *testing.T) int { + t.Helper() + data, err := os.ReadFile(filepath.Join(f.remoteDir, "hooks", "race-counter")) + if errors.Is(err, os.ErrNotExist) { + return 0 + } + require.NoError(t, err) + n, err := strconv.Atoi(strings.TrimSpace(string(data))) + require.NoError(t, err) + return n +} + +// remoteHEAD returns the SHA that origin/main currently points to. +func (f gitFixture) remoteHEAD(t *testing.T) string { + out := mustGitOutput(t, f.remoteDir, "rev-parse", "main") + return strings.TrimSpace(string(out)) +} + +// remoteFile returns the contents of `path` at origin/main on the bare remote. +func (f gitFixture) remoteFile(t *testing.T, path string) string { + out := mustGitOutput(t, f.remoteDir, "show", "main:"+path) + return string(out) +} + +// remoteCommitsSinceSeed returns commit SHAs reachable from origin/main newer +// than the first (seed) commit, in chronological order. +func (f gitFixture) remoteCommitsSinceSeed(t *testing.T) []string { + out := mustGitOutput(t, f.remoteDir, "log", "--reverse", "--format=%H", "main") + all := strings.Split(strings.TrimSpace(string(out)), "\n") + if len(all) <= 1 { + return nil + } + return all[1:] +} + +// parents returns the parent SHAs recorded on the commit at sha on the remote. +func (f gitFixture) parents(t *testing.T, sha string) []string { + t.Helper() + out := mustGitOutput(t, f.remoteDir, "rev-list", "--parents", "-n", "1", sha) + fields := strings.Fields(strings.TrimSpace(string(out))) + require.NotEmpty(t, fields) + return fields[1:] +} + +func mustGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := newGitCommand(context.Background(), testGitRuntime(t), dir, args...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + require.NoError(t, cmd.Run(), "git %s: %s", strings.Join(args, " "), stderr.String()) +} + +func mustGitOutput(t *testing.T, dir string, args ...string) []byte { + t.Helper() + cmd := newGitCommand(context.Background(), testGitRuntime(t), dir, args...) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + require.NoError(t, cmd.Run(), "git %s: %s", strings.Join(args, " "), stderr.String()) + return stdout.Bytes() +} + +func testGitRuntime(t *testing.T) GitRuntime { + t.Helper() + executable := absoluteTestPath(t, "SUBMITQUEUE_TEST_GIT") + templateDescription := absoluteTestPath(t, "SUBMITQUEUE_TEST_GIT_TEMPLATE_DESCRIPTION") + return GitRuntime{ + Executable: executable, + ExecPath: filepath.Dir(executable), + TemplateDir: filepath.Dir(templateDescription), + } +} + +func absoluteTestPath(t *testing.T, name string) string { + t.Helper() + path := os.Getenv(name) + require.NotEmpty(t, path) + if filepath.IsAbs(path) { + _, err := os.Stat(path) + require.NoError(t, err) + return path + } + if absolute, err := filepath.Abs(path); err == nil { + if _, err := os.Stat(absolute); err == nil { + return absolute + } + } + + // rules_go expands $(location) to an execroot-relative path. Tests run from + // their main-repository runfiles directory, so translate an external output + // to its canonical repository path under TEST_SRCDIR. + const externalMarker = "/external/" + slashed := filepath.ToSlash(path) + externalPath := "" + if strings.HasPrefix(slashed, "external/") { + externalPath = strings.TrimPrefix(slashed, "external/") + } else if i := strings.Index(slashed, externalMarker); i >= 0 { + externalPath = slashed[i+len(externalMarker):] + } + if externalPath != "" { + runfilesRoot := os.Getenv("TEST_SRCDIR") + require.NotEmpty(t, runfilesRoot) + candidate := filepath.Join(runfilesRoot, filepath.FromSlash(externalPath)) + _, err := os.Stat(candidate) + require.NoError(t, err) + return candidate + } + + require.FailNowf(t, "resolve test path", "%s=%q is not a runfile", name, path) + return "" +} + +func writeFile(path, contents string) error { + return os.WriteFile(path, []byte(contents), 0o644) +} diff --git a/runway/extension/merger/git/objects.go b/runway/extension/merger/git/objects.go new file mode 100644 index 00000000..8fcb95e6 --- /dev/null +++ b/runway/extension/merger/git/objects.go @@ -0,0 +1,116 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package git + +import ( + "context" + "fmt" + "strings" + + coremetrics "github.com/uber/submitqueue/platform/metrics" + "github.com/uber/submitqueue/runway/extension/merger" +) + +// ensureObjects makes every commit the request references available locally, +// for all steps, before any step is applied. Front-loading it means a request +// naming an unreachable commit fails without having mutated the checkout. +// +// It matters because the default fetch refspec is +refs/heads/*, which does not +// cover a provider's change refs: a pull request head that was never also +// pushed as a branch (the normal case for a fork) is simply absent locally, and +// the apply would then fail with git's "bad object" — indistinguishable, to the +// apply paths, from a merge conflict. +func (m *gitMerger) ensureObjects(ctx context.Context, refs []changeRef) error { + for _, ref := range refs { + if err := m.ensureObject(ctx, ref); err != nil { + return err + } + } + return nil +} + +// ensureObject guarantees a single commit is present, fetching it if needed. +// +// The commit is requested by SHA, which relies on the server allowing a want +// for an object that is reachable but not advertised (uploadpack +// .allowReachableSHA1InWant, which github.com enables). The provider's +// canonical ref is the fallback for a server that does not. Neither fetch is +// shallow: the apply paths need the commit's ancestry, not just the commit. +func (m *gitMerger) ensureObject(ctx context.Context, ref changeRef) error { + if m.hasCommit(ctx, ref.SHA) { + return nil + } + + if _, err := m.run(ctx, nil, "fetch", m.remote, ref.SHA); err == nil && m.hasCommit(ctx, ref.SHA) { + return nil + } + if ref.Ref != "" { + if _, err := m.run(ctx, nil, "fetch", m.remote, ref.Ref); err == nil && m.hasCommit(ctx, ref.SHA) { + return nil + } + } + + // Both fetches failed. Distinguish "the remote is unreachable right now" + // (infrastructure, worth retrying) from "the remote is fine and simply does + // not have this commit" (terminal — no number of retries conjures it). + if _, err := m.run(ctx, nil, "ls-remote", "--exit-code", m.remote, "HEAD"); err != nil { + return fmt.Errorf("remote %s unreachable while resolving commit %s: %w", m.remote, ref.SHA, err) + } + + coremetrics.NamedCounter(m.metricsScope, "merge", "object_unavailable", 1) + return fmt.Errorf("%w: commit %s is not available from remote %s (tried by SHA and via %q)", + merger.ErrInvalidRequest, ref.SHA, m.remote, ref.Ref) +} + +// hasCommit reports whether sha names a commit object in the local checkout. +func (m *gitMerger) hasCommit(ctx context.Context, sha string) bool { + _, err := m.run(ctx, nil, "cat-file", "-e", sha+"^{commit}") + return err == nil +} + +// checkStale reports whether any change has been superseded since its URI was +// minted. Fetching by SHA guarantees the merger applies exactly the commit the +// URI names, but not that the commit is still the change's head — a force-push +// leaves the old commit fetchable for some time on most hosts, so a successful +// fetch says nothing about freshness. +// +// The comparison costs one ref advertisement and transfers no objects. A ref +// that no longer exists yields no verdict rather than a failure: the change may +// legitimately have been closed, and ensureObjects has already established that +// the commit itself is present. +func (m *gitMerger) checkStale(ctx context.Context, refs []changeRef) error { + if !m.checkStaleness { + return nil + } + for _, ref := range refs { + if ref.Ref == "" { + continue + } + out, err := m.run(ctx, nil, "ls-remote", m.remote, ref.Ref) + if err != nil { + return fmt.Errorf("git ls-remote %s %s: %w", m.remote, ref.Ref, err) + } + fields := strings.Fields(string(out)) + if len(fields) == 0 { + continue + } + if current := fields[0]; current != ref.SHA { + coremetrics.NamedCounter(m.metricsScope, "merge", "stale_changes", 1) + return fmt.Errorf("%w: change is stale: %s names commit %s but %s now points at %s", + merger.ErrInvalidRequest, ref.Label, ref.SHA, ref.Ref, current) + } + } + return nil +}