fix(runway): ISS-011 fail fast without Git configuration

service/runway/README.md

@@ -15,7 +15,7 @@ These topic keys and their wire contracts are owned by the queue's producer side
 
 ### Merger backend
 
-The merge work is done by the [`merger`](../../runway/extension/merger) extension, resolved **per queue** — so one Runway can serve several repositories by giving each queue its own merge target. By default every queue gets the **noop** merger (always succeeds — for local dev and compose). Point `MERGE_CONFIG_PATH` at a merge configuration file to wire real **git** merge targets, or set `MERGE_CHECKOUT_PATH` to configure a single one from the environment (see Configuration).
+The merge work is done by the [`merger`](../../runway/extension/merger) extension, resolved **per queue** — so one Runway can serve several repositories by giving each queue its own merge target. By default every queue gets the **noop** merger (always succeeds — for local dev and compose). Point `MERGE_CONFIG_PATH` at a merge configuration file to wire real **git** merge targets, or set `MERGE_CHECKOUT_PATH` to configure a single one from the environment (see Configuration). Setting `MERGER=git` makes Git configuration mandatory: startup fails unless one of those sources defines at least one Git target.
 
 Two queues naming the same checkout resolve to the *same* merger instance, which is what serializes them against each other: a git merger locks the working tree it owns, and two instances over one tree would reset it out from under each other mid-merge. Naming one checkout for two *different* targets is rejected at startup.
 
@@ -71,6 +71,7 @@ The Runway controllers themselves live under [`runway/controller/`](../../runway
 | `QUEUE_MYSQL_DSN` | yes      | Queue database DSN                            | —                        |
 | `PORT`            | no       | gRPC listen address                           | `:8086`                  |
 | `HOSTNAME`        | no       | Subscriber name for the queue consumer        | `runway-<unix_ts>`       |
+| `MERGER`          | no       | Explicit merger selection. `git` requires `MERGE_CHECKOUT_PATH` or a `MERGE_CONFIG_PATH` file containing at least one Git target; `noop` forces synthetic success; `fake` is test-only. Unset resolves from the merge configuration and retains the noop fallback. | — |
 | `MERGE_CONFIG_PATH` | no | Path to the per-queue merge configuration file (see above). Takes precedence over the `MERGE_*` variables below, which configure a single target. | — |
 | `MERGE_CHECKOUT_PATH` | no   | Absolute path to the git checkout the merger owns. When unset (and no config file), the noop merger is used. | — (noop) |
 | `MERGE_REMOTE`    | no       | Git remote to fetch/push                       | `origin`                 |

service/runway/server/config_test.go

@@ -15,13 +15,15 @@
 package main
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/zap/zaptest"
 
 	mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb"
 )
@@ -113,6 +115,133 @@ func TestLoadMergeConfig_EmptyFileIsNoop(t *testing.T) {
 	assert.False(t, cfg.usesGit())
 }
 
+func TestResolveMergerStartupConfig(t *testing.T) {
+	tests := []struct {
+		name             string
+		merger           string
+		checkoutPath     string
+		configPath       string
+		configContents   string
+		hasConfigFile    bool
+		wantSelection    string
+		wantDefaultType  string
+		wantUsesGit      bool
+		wantErr          error
+		wantUnclassified bool
+	}{
+		{
+			name:            "unset without config uses automatic noop fallback",
+			wantDefaultType: mergerTypeNoop,
+		},
+		{
+			name:    "explicit git without config fails",
+			merger:  mergerTypeGit,
+			wantErr: errExplicitGitConfigurationRequired,
+		},
+		{
+			name:            "explicit git with checkout environment is configured",
+			merger:          mergerTypeGit,
+			checkoutPath:    "/var/checkouts/r",
+			wantSelection:   mergerTypeGit,
+			wantDefaultType: mergerTypeGit,
+			wantUsesGit:     true,
+		},
+		{
+			name:            "explicit git with config file containing a git target is configured",
+			merger:          mergerTypeGit,
+			configContents:  gitTargetConfig(),
+			hasConfigFile:   true,
+			wantSelection:   mergerTypeGit,
+			wantDefaultType: mergerTypeNoop,
+			wantUsesGit:     true,
+		},
+		{
+			name:           "explicit git with config file containing no git target fails",
+			merger:         mergerTypeGit,
+			configContents: noopTargetConfig(),
+			hasConfigFile:  true,
+			wantErr:        errExplicitGitConfigurationRequired,
+		},
+		{
+			name:          "explicit noop bypasses merge configuration",
+			merger:        mergerTypeNoop,
+			configPath:    "/missing/merge.yaml",
+			wantSelection: mergerTypeNoop,
+		},
+		{
+			name:          "explicit fake preserves test behavior",
+			merger:        mergerOverrideFake,
+			configPath:    "/missing/merge.yaml",
+			wantSelection: mergerOverrideFake,
+		},
+		{
+			name:             "invalid merger fails",
+			merger:           "magic",
+			wantUnclassified: true,
+		},
+		{
+			name:             "invalid merge config still fails",
+			configContents:   "defaults:\n  merger: {type: magic}\n",
+			hasConfigFile:    true,
+			wantUnclassified: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			setMergerStartupEnv(t, tt.merger, tt.checkoutPath)
+			switch {
+			case tt.hasConfigFile:
+				t.Setenv("MERGE_CONFIG_PATH", writeConfig(t, tt.configContents))
+			case tt.configPath != "":
+				t.Setenv("MERGE_CONFIG_PATH", tt.configPath)
+			}
+
+			startup, err := resolveMergerStartupConfig(zaptest.NewLogger(t))
+			if tt.wantErr != nil {
+				require.ErrorIs(t, err, tt.wantErr)
+				return
+			}
+			if tt.wantUnclassified {
+				require.Error(t, err)
+				assert.False(t, errors.Is(err, errExplicitGitConfigurationRequired))
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantSelection, startup.selection)
+			assert.Equal(t, tt.wantDefaultType, startup.targets.Defaults.Merger.Type)
+			assert.Equal(t, tt.wantUsesGit, startup.targets.usesGit())
+		})
+	}
+}
+
+func setMergerStartupEnv(t *testing.T, merger, checkoutPath string) {
+	t.Helper()
+	t.Setenv("MERGER", merger)
+	t.Setenv("MERGE_CONFIG_PATH", "")
+	t.Setenv("MERGE_CHECKOUT_PATH", checkoutPath)
+}
+
+func gitTargetConfig() string {
+	return `
+defaults:
+  merger: {type: noop}
+queues:
+  - name: demo
+    merger: {type: git, checkoutPath: /var/checkouts/r}
+`
+}
+
+func noopTargetConfig() string {
+	return `
+defaults:
+  merger: {type: noop}
+queues:
+  - name: demo
+    merger: {type: noop}
+`
+}
+
 func TestLoadMergeConfig_SharedCheckoutForSameTargetIsAllowed(t *testing.T) {
 	// Two queues landing on one target must resolve to one merger instance, so
 	// sharing a checkout is the correct configuration rather than a mistake.

service/runway/server/docker-compose.yml

@@ -45,6 +45,7 @@ services:
       - PORT=:8080
       # Merger implementation. Empty (the default) resolves from the merge
       # environment: git when MERGE_CHECKOUT_PATH is set, noop otherwise. The
+      # explicit git value fails startup without a configured Git target. The
       # e2e suite sets SQ_RUNWAY_MERGER=fake to drive outcomes from the payload.
       - MERGER=${SQ_RUNWAY_MERGER:-}
       # Queue infrastructure connection

service/runway/server/main.go

@@ -62,6 +62,10 @@ type RunwayServer struct {
 	pingController *controller.PingController
 }
 
+const mergerOverrideFake = "fake"
+
+var errExplicitGitConfigurationRequired = errors.New(`MERGER="git" requires usable Git configuration: set MERGE_CHECKOUT_PATH or set MERGE_CONFIG_PATH to a config containing at least one git merger`)
+
 // Ping delegates to the controller.
 func (s *RunwayServer) Ping(ctx context.Context, req *pb.PingRequest) (*pb.PingResponse, error) {
 	return s.pingController.Ping(ctx, req)
@@ -307,34 +311,31 @@ func run() error {
 
 // newMergerFactory builds the mergers for the server.
 //
-// MERGER pins every queue to one implementation explicitly, which is how a test
-// holds the service to a fake without a git checkout. Left unset, each queue
-// resolves its own merge target through the merge configuration, so a
-// deployment can serve several repositories from one Runway.
+// MERGER=fake and MERGER=noop pin every queue to one implementation.
+// MERGER=git requires the resolved merge configuration to contain a Git target.
+// Left unset, each queue resolves its own merge target through configuration.
 //
 // The fake is reachable only through MERGER, never through the configuration
 // file: an implementation whose outcomes are steered by markers in a change URI
 // has no business being selectable by a production config.
 func newMergerFactory(ctx context.Context, logger *zap.Logger, scope tally.Scope) (merger.Factory, error) {
-	switch impl := strings.ToLower(strings.TrimSpace(os.Getenv("MERGER"))); impl {
-	case "fake":
+	startup, err := resolveMergerStartupConfig(logger)
+	if err != nil {
+		return nil, err
+	}
+
+	switch startup.selection {
+	case mergerOverrideFake:
 		// Marker-driven outcomes, for e2e tests that need Runway to fail on
 		// demand without a git checkout. Never production.
 		logger.Info("MERGER=fake; using marker-driven fake merger for every queue")
 		return &fakeMergerFactory{seq: new(atomic.Uint64)}, nil
 	case "noop":
 		logger.Info("MERGER=noop; using noop merger for every queue")
 		return &noopMergerFactory{seq: new(atomic.Uint64)}, nil
-	case "", "git":
-		// Fall through to the configured per-queue merge targets.
-	default:
-		return nil, fmt.Errorf("invalid MERGER %q", impl)
 	}
 
-	cfg, err := loadMergeConfigFromEnv(logger)
-	if err != nil {
-		return nil, err
-	}
+	cfg := startup.targets
 
 	// The git runtime is resolved only when something actually needs it, so a
 	// deployment running nothing but the noop merger does not require git to be
@@ -385,6 +386,31 @@ func newMergerFactory(ctx context.Context, logger *zap.Logger, scope tally.Scope
 	return mergerRegistry{byQueue: byQueue, fallback: fallback}, nil
 }
 
+type mergerStartupConfig struct {
+	selection string
+	targets   mergeConfig
+}
+
+func resolveMergerStartupConfig(logger *zap.Logger) (mergerStartupConfig, error) {
+	selection := strings.ToLower(strings.TrimSpace(os.Getenv("MERGER")))
+	switch selection {
+	case mergerOverrideFake, mergerTypeNoop:
+		return mergerStartupConfig{selection: selection}, nil
+	case "", mergerTypeGit:
+	default:
+		return mergerStartupConfig{}, fmt.Errorf("invalid MERGER %q", selection)
+	}
+
+	cfg, err := loadMergeConfigFromEnv(logger)
+	if err != nil {
+		return mergerStartupConfig{}, err
+	}
+	if selection == mergerTypeGit && !cfg.usesGit() {
+		return mergerStartupConfig{}, errExplicitGitConfigurationRequired
+	}
+	return mergerStartupConfig{selection: selection, targets: cfg}, nil
+}
+
 // loadMergeConfigFromEnv reads the merge configuration file when one is
 // configured, and otherwise reconstructs the equivalent single-queue
 // configuration from the MERGE_* environment.