From bfdfaf7fefaee5e605ff846878b22aaf740abc76 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:06:56 +0200 Subject: [PATCH] fix(release): refuse to tag unless HEAD is at the remote default tip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release tool tags whatever HEAD points at and pushes the tag, but the downstream "chore: bump" commit it creates afterwards is only committed locally (gitPushTags pushes tags, never the branch). When the tool is re-run while HEAD sits on such a local-only bump commit, the new tag lands on a commit that is not on the remote default branch. That is how store/v0.0.28 ended up pinned to the "chore: bump store/v0.0.27" commit, shipping a store release that predated — and therefore did not contain — a Linux keychain fix that had already merged via PRs. Before creating any tag, fetch and verify HEAD equals the tip of the remote default branch (origin/HEAD, falling back to main); refuse otherwise. The check is skipped in --dry and --skip-git modes since they create no tags. Adds an integration test (temp repo + bare remote) covering the in-sync, local-only-commit, and pushed cases, and documents the release workflow and guard in docs/releasing.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/releasing.md | 61 ++++++++++++++++++++ x/release/command.go | 60 ++++++++++++++++++++ x/release/command_test.go | 113 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 docs/releasing.md create mode 100644 x/release/command_test.go diff --git a/docs/releasing.md b/docs/releasing.md new file mode 100644 index 00000000..4ad86916 --- /dev/null +++ b/docs/releasing.md @@ -0,0 +1,61 @@ +# Releasing modules + +This repository is a multi-module Go repository. Each module is released with +its own tag of the form `/vX.Y.Z` (for example `store/v0.0.29`). + +Releases are cut with the helper in [`x/release`](../x/release): + +```sh +# from the repository root +go run ./x/release bump +``` + +By default this performs a patch release and propagates the bump to internal +downstream modules. Useful flags: + +- `--release ` — choose the bump level for `` + (downstreams are always propagated as a patch). +- `--dry` — log every step without changing anything. +- `--skip-git` — preview only the `go.mod` edits, skipping git operations. +- `--no-propagate` — release only ``; do not bump downstreams. + +## How a release is applied + +For the released module the tool: + +1. Creates an annotated tag on the current `HEAD` and pushes the tag. +2. Bumps the module version in each downstream `go.mod`. +3. Commits those edits as `chore: bump /vX.Y.Z`. + +> [!IMPORTANT] +> The tag is created on `HEAD`, and the `chore: bump` commit is committed +> **locally** — the tool pushes tags, not the branch. Land that commit on the +> default branch via a PR. + +## HEAD must match the remote default branch + +Before creating any tag, the tool fetches and verifies that `HEAD` is exactly +the tip of the remote default branch (`origin/HEAD`, falling back to `main`). +If it is not, the release is refused: + +``` +refusing to tag: HEAD () is not at the tip of origin/main (); ... +``` + +This guard exists because the tool tags whatever `HEAD` points at. If it is run +while `HEAD` sits on a local-only `chore: bump` commit (or any commit not yet on +the default branch), the tag lands on a commit that does not contain changes +merged afterwards. That is exactly how `store/v0.0.28` was pinned to an older +`chore: bump store/v0.0.27` commit and shipped without a keychain fix that had +already merged via PRs. + +Always release from an up-to-date checkout of the default branch: + +```sh +git checkout main +git pull --ff-only +go run ./x/release bump +``` + +The `--dry` and `--skip-git` modes skip this check, since they do not create +tags. diff --git a/x/release/command.go b/x/release/command.go index ebea0e5f..0823aca3 100644 --- a/x/release/command.go +++ b/x/release/command.go @@ -56,6 +56,11 @@ func ReleaseCommand(cfg Config) (*cobra.Command, error) { Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { mod := args[0] + if !opts.dryRun && !opts.skipGit { + if err := verifyReleaseRef(cmd.Context()); err != nil { + return err + } + } data, err := newRepoData(cfg.EnableModulesWithPreReleaseVersion) if err != nil { return err @@ -284,3 +289,58 @@ func gitCommit(ctx context.Context, commit string) error { } return nil } + +// verifyReleaseRef ensures HEAD is exactly the tip of the remote default branch +// before any tags are created. +// +// The tool tags the current HEAD and pushes the tag immediately, but the +// downstream "chore: bump" commit it creates afterwards is only committed +// locally — gitPushTags pushes tags, never the branch. If the tool is re-run +// while HEAD sits on such a local-only bump commit, the new tag lands on a +// commit that is not on the remote default branch. That is how store/v0.0.28 +// ended up pinned to a "chore: bump store/v0.0.27" commit that predated later +// fixes merged via PRs. Refuse to tag unless HEAD matches the fetched remote +// default branch tip. +func verifyReleaseRef(ctx context.Context) error { + branch := remoteDefaultBranch(ctx) + if out, err := runGit(ctx, "fetch", "origin", branch); err != nil { + return fmt.Errorf("git fetch origin %s (%s): %s", branch, err, out) + } + head, err := revParse(ctx, "HEAD") + if err != nil { + return err + } + remote, err := revParse(ctx, "refs/remotes/origin/"+branch) + if err != nil { + return err + } + if head != remote { + return fmt.Errorf("refusing to tag: HEAD (%s) is not at the tip of origin/%s (%s); "+ + "merge/pull the remote default branch and ensure any local 'chore: bump' commit is pushed before releasing", + head, branch, remote) + } + return nil +} + +// remoteDefaultBranch resolves origin's default branch, falling back to "main" +// when origin/HEAD is not configured locally. +func remoteDefaultBranch(ctx context.Context) string { + out, err := runGit(ctx, "symbolic-ref", "--short", "refs/remotes/origin/HEAD") + if err != nil { + return "main" + } + return strings.TrimPrefix(strings.TrimSpace(out), "origin/") +} + +func revParse(ctx context.Context, ref string) (string, error) { + out, err := runGit(ctx, "rev-parse", ref) + if err != nil { + return "", fmt.Errorf("git rev-parse %s (%s): %s", ref, err, out) + } + return strings.TrimSpace(out), nil +} + +func runGit(ctx context.Context, args ...string) (string, error) { + out, err := exec.CommandContext(ctx, "git", args...).CombinedOutput() + return string(out), err +} diff --git a/x/release/command_test.go b/x/release/command_test.go new file mode 100644 index 00000000..92690eb9 --- /dev/null +++ b/x/release/command_test.go @@ -0,0 +1,113 @@ +// Copyright 2026 Docker, 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 main + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Not parallel: the git helpers shell out in the process working directory, so +// the test switches cwd, which is process-global. +func Test_verifyReleaseRef(t *testing.T) { + repo := newGitRepoWithRemote(t) + + t.Run("passes when HEAD is at remote default tip", func(t *testing.T) { + assert.NoError(t, verifyReleaseRef(repo.ctxAt(t))) + }) + + t.Run("fails on a local-only commit", func(t *testing.T) { + repo.commit(t, "chore: bump local/v0.0.1") + err := verifyReleaseRef(repo.ctxAt(t)) + require.Error(t, err) + assert.Contains(t, err.Error(), "refusing to tag") + }) + + t.Run("passes again once the commit is pushed", func(t *testing.T) { + repo.run(t, "push", "origin", "HEAD:main") + assert.NoError(t, verifyReleaseRef(repo.ctxAt(t))) + }) +} + +type gitRepo struct { + dir string +} + +// newGitRepoWithRemote creates a working clone whose origin is a local bare +// repo, with origin/HEAD pointing at main, and HEAD at the remote tip. +func newGitRepoWithRemote(t *testing.T) *gitRepo { + t.Helper() + root := t.TempDir() + bare := filepath.Join(root, "origin.git") + work := filepath.Join(root, "work") + + runGitIn(t, root, "init", "--bare", "--initial-branch=main", bare) + + r := &gitRepo{dir: work} + runGitIn(t, root, "clone", bare, work) + r.config(t) + require.NoError(t, os.WriteFile(filepath.Join(work, "README.md"), []byte("seed\n"), 0o644)) + r.run(t, "add", "README.md") + r.run(t, "commit", "-m", "seed") + r.run(t, "push", "-u", "origin", "main") + // Make origin/HEAD resolvable so remoteDefaultBranch finds it. + r.run(t, "remote", "set-head", "origin", "main") + return r +} + +func (r *gitRepo) config(t *testing.T) { + t.Helper() + r.run(t, "config", "user.email", "test@example.com") + r.run(t, "config", "user.name", "test") + r.run(t, "config", "commit.gpgsign", "false") + r.run(t, "config", "tag.gpgsign", "false") +} + +func (r *gitRepo) commit(t *testing.T, msg string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(r.dir, "file.txt"), []byte(msg), 0o644)) + r.run(t, "add", "file.txt") + r.run(t, "commit", "-m", msg) +} + +// ctxAt returns a context after switching the process into the repo dir, so the +// git helpers (which shell out in the cwd) operate on this repo. +func (r *gitRepo) ctxAt(t *testing.T) context.Context { + t.Helper() + cwd, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(r.dir)) + t.Cleanup(func() { _ = os.Chdir(cwd) }) + return t.Context() +} + +func (r *gitRepo) run(t *testing.T, args ...string) { + t.Helper() + runGitIn(t, r.dir, args...) +} + +func runGitIn(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %v: %s", args, out) +}