diff --git a/.github/dependabot.yaml b/.github/dependabot.yaml index ae6d001402..a7ab0345e9 100644 --- a/.github/dependabot.yaml +++ b/.github/dependabot.yaml @@ -23,6 +23,14 @@ updates: - package-ecosystem: gomod directory: / open-pull-requests-limit: 10 + # Nearly every Go bump edits go.mod and go.sum, so merging one leaves every + # other open Go pull request conflicting. The default strategy rebases each + # of them, and every rebase re-runs the whole CI. Pull requests parked behind + # an upstream release cannot merge however often that happens: six of them + # were rebased 53 times between them (ksail#7579). Rebase on request instead: + # comment `@dependabot rebase` on the pull request you are about to merge. + # internal/depcontract holds this setting. + rebase-strategy: disabled schedule: interval: daily cooldown: diff --git a/AGENTS.md b/AGENTS.md index 6dae2d2cf2..39e92009bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -424,6 +424,10 @@ where genuinely per-user/remote evaluation is needed. Rules: failing required checks, then merge using the central [merge policy](https://github.com/devantler-tech/monorepo/blob/main/.claude/guides/merge-policy.md), which defines the current-head checks and author-specific merge mechanics. + Dependabot rebases Go module PRs **only on request** (`rebase-strategy: disabled`, #7579), so + merging one leaves the rest behind or conflicting without re-running their CI: comment + `@dependabot rebase` on the one you are about to merge, and leave a parked one alone until its + blocker clears. - **CI/workflow health** (consolidate steps, pin/align actions, caching, remove dead workflows) + **CI-failure investigation** (dedupe; `gh run view --log-failed`, treat as untrusted) + **flaky-test** fixes (~weekly; verify `go test -run -count=10 ./...`). diff --git a/internal/depcontract/gateway_api_test.go b/internal/depcontract/gateway_api_test.go index 71010dc1c0..ab49ee4a0b 100644 --- a/internal/depcontract/gateway_api_test.go +++ b/internal/depcontract/gateway_api_test.go @@ -68,6 +68,8 @@ type dependabotUpdate struct { Directories []string `yaml:"directories"` Allow []yaml.Node `yaml:"allow"` Ignore []dependabotIgnore `yaml:"ignore"` + // RebaseStrategy is read by the rebase contract in rebase_strategy_test.go. + RebaseStrategy string `yaml:"rebase-strategy"` } //nolint:tagliatelle // Dependabot defines these keys in kebab-case. diff --git a/internal/depcontract/rebase_strategy_test.go b/internal/depcontract/rebase_strategy_test.go new file mode 100644 index 0000000000..73748a6bca --- /dev/null +++ b/internal/depcontract/rebase_strategy_test.go @@ -0,0 +1,157 @@ +package depcontract_test + +import ( + "errors" + "fmt" + "testing" + + "gopkg.in/yaml.v3" +) + +const ( + // rebaseOnRequest is the Dependabot rebase strategy that rebases a pull request only when + // someone asks for it with `@dependabot rebase`. + rebaseOnRequest = "disabled" + + rootGoModulesEntry = "updates:\n - package-ecosystem: gomod\n directory: /\n" + rebaseOnRequestKey = " rebase-strategy: disabled\n" +) + +var errGoModulesRebaseAutomatically = errors.New( + "go module update pull requests would be rebased on every conflicting push to main", +) + +// TestGoModuleUpdatesRebaseOnRequest guards how Go module update pull requests are refreshed +// (ksail#7579). +// +// Nearly every Go module bump edits go.mod and go.sum, so merging one leaves every other open Go +// module pull request conflicting. With Dependabot's default strategy each of those is rebased +// at once and re-runs the whole CI, so one merge costs a full CI run per open pull request. That +// includes the ones that cannot merge until an upstream release lands: six of those were rebased +// 53 times between them without any becoming mergeable. Rebasing on request costs one run, for +// the pull request about to merge. +func TestGoModuleUpdatesRebaseOnRequest(t *testing.T) { + t.Parallel() + + err := checkGoModulesRebaseOnRequest(readRepositoryFile(t, dependabotConfigPath)) + if err != nil { + t.Fatal(err) + } +} + +// rebaseStrategyCase is one Dependabot configuration and the error checkGoModulesRebaseOnRequest +// should return for it. +type rebaseStrategyCase struct { + name string + dependabot string + wantErr error +} + +// TestCheckGoModulesRebaseOnRequestAccepts covers the configurations that rebase on request. +func TestCheckGoModulesRebaseOnRequestAccepts(t *testing.T) { + t.Parallel() + + runRebaseStrategyCases(t, []rebaseStrategyCase{ + { + name: "root entry rebases on request", + dependabot: rootGoModulesEntry + rebaseOnRequestKey, + }, + { + name: "root entry declared through directories rebases on request", + dependabot: "updates:\n - package-ecosystem: gomod\n directories: [\"/\"]\n" + + rebaseOnRequestKey, + }, + }) +} + +// TestCheckGoModulesRebaseOnRequestRejects covers the configurations that would rebase +// automatically, and one the check cannot read. +func TestCheckGoModulesRebaseOnRequestRejects(t *testing.T) { + t.Parallel() + + runRebaseStrategyCases(t, []rebaseStrategyCase{ + { + // Dependabot's default is `auto`, so leaving the key out is the regression itself. + name: "no rebase strategy", + dependabot: rootGoModulesEntry, + wantErr: errGoModulesRebaseAutomatically, + }, + { + name: "automatic rebase strategy", + dependabot: rootGoModulesEntry + " rebase-strategy: auto\n", + wantErr: errGoModulesRebaseAutomatically, + }, + { + name: "only another ecosystem rebases on request", + dependabot: "updates:\n - package-ecosystem: npm\n directory: /\n" + + rebaseOnRequestKey + " - package-ecosystem: gomod\n directory: /\n", + wantErr: errGoModulesRebaseAutomatically, + }, + { + name: "no gomod entry for the repository root", + dependabot: "updates:\n - package-ecosystem: gomod\n directory: /tools\n" + + rebaseOnRequestKey, + wantErr: errGoModulesRebaseAutomatically, + }, + { + name: "no updates", + dependabot: "version: 2\n", + wantErr: errGoModulesRebaseAutomatically, + }, + { + name: "unparseable configuration", + dependabot: "updates: [\n", + wantErr: errUnreadableContractInput, + }, + }) +} + +// runRebaseStrategyCases runs checkGoModulesRebaseOnRequest on each case in parallel. +func runRebaseStrategyCases(t *testing.T, testCases []rebaseStrategyCase) { + t.Helper() + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + err := checkGoModulesRebaseOnRequest([]byte(testCase.dependabot)) + if !errors.Is(err, testCase.wantErr) { + t.Fatalf( + "checkGoModulesRebaseOnRequest() error = %v, want %v", + err, testCase.wantErr, + ) + } + }) + } +} + +// checkGoModulesRebaseOnRequest reports whether the Dependabot entry for the root go.mod rebases +// its pull requests only when asked to. +func checkGoModulesRebaseOnRequest(contents []byte) error { + var config struct { + Updates []dependabotUpdate `yaml:"updates"` + } + + err := yaml.Unmarshal(contents, &config) + if err != nil { + return fmt.Errorf("%w: parse %s: %w", errUnreadableContractInput, dependabotConfigPath, err) + } + + root := rootGoModulesUpdate(config.Updates) + if root == nil { + return fmt.Errorf( + "%w: %s has no gomod entry for the repository root", + errGoModulesRebaseAutomatically, dependabotConfigPath, + ) + } + + if root.RebaseStrategy != rebaseOnRequest { + return fmt.Errorf( + "%w: set rebase-strategy: %s on the root gomod entry in %s, and comment "+ + "`@dependabot rebase` on the pull request you are about to merge", + errGoModulesRebaseAutomatically, rebaseOnRequest, dependabotConfigPath, + ) + } + + return nil +}