diff --git a/docs/content/docs/architecture/library-decisions.md b/docs/content/docs/architecture/library-decisions.md index cf07242..cf25aa8 100644 --- a/docs/content/docs/architecture/library-decisions.md +++ b/docs/content/docs/architecture/library-decisions.md @@ -128,8 +128,10 @@ configDir := filepath.Join(xdg.ConfigHome, "specs") **Decision:** `github.com/Masterminds/semver/v3` for semver-aware version comparison. **Rationale:** -- `specs template upgrade` compares local and remote tag versions to find the highest - available semver tag greater than the currently installed version. +- `specs template update`/`upgrade` compare local and remote tag versions to find the highest + available semver tag greater than the currently installed version. This applies both to + tag-tracked templates and to branch-tracked templates whose checkout sits on a semver tag, so + a lower-numbered tag published on a later commit is never mistaken for an update (issue #83). - `semver.NewVersion()` + `GreaterThan()` replaces hand-rolled string comparison. --- diff --git a/docs/content/docs/architecture/template-engine.md b/docs/content/docs/architecture/template-engine.md index 1575d23..049d003 100644 --- a/docs/content/docs/architecture/template-engine.md +++ b/docs/content/docs/architecture/template-engine.md @@ -434,7 +434,7 @@ The `update` command forces an immediate refresh for one or all templates. ```go type TemplateStatus struct { CheckedAt JSONTime // time of last remote check - IsUpToDate bool // true when local HEAD matches remote + IsUpToDate bool // true when no newer version is available (see below) LatestVersion string // set when a newer semver tag is available ErrorKind pkggit.CheckErrorKind // "network", "auth", "not-found", "unknown", or "" } @@ -442,3 +442,24 @@ type TemplateStatus struct { `specs template list` displays a `Status` column with labels: `up-to-date`, `update: `, `update available`, `unknown (offline?)`, `auth error`, `not found`. + +### How a newer version is determined + +`resolveStatus` compares the remote refs against the local checkout using two modes: + +- **Tag-tracked** — the tracked ref is itself a semver tag (e.g. the template was downloaded + with `github:owner/repo:1.1.0`). A newer version is the **highest semver tag strictly + greater** than the current one. A lower-numbered tag published later (e.g. `1.0.1` after + `1.1.0`) is never treated as an update. +- **Branch-tracked** — the tracked ref is a branch (the default when no ref is given). If the + local checkout sits exactly on a released semver tag, the same semver rule as above applies: + an update requires a strictly-greater semver tag, so `1.0.1` pushed on a commit newer than + the installed `1.1.0` is **not** reported as an update. When the checkout is not on a semver + tag (a rolling branch), the check falls back to comparing the branch-tip commit against the + local `HEAD`, so any new commit counts as an update. + +This ensures "newer" always follows semantic-versioning rules for tagged templates, regardless +of the order in which tags were pushed (issue #83). Determining whether the checkout is on a +released version uses the tag that points exactly at `HEAD` (dereferencing annotated tags) +rather than parsing git-describe output, whose `--g` suffix would otherwise be +misread as a semver pre-release. diff --git a/internal/util/git/git.go b/internal/util/git/git.go index 5063ade..c159d03 100644 --- a/internal/util/git/git.go +++ b/internal/util/git/git.go @@ -370,7 +370,46 @@ func CheckRemoteContext(ctx context.Context, dir, url, branch string) (result Re return RemoteCheckResult{ErrorKind: CheckErrorUnknown} } - return resolveStatus(refs, head.Hash(), branch) + // currentVersion is the semver tag the local checkout sits exactly on, or "" when + // HEAD is on an untagged branch commit. resolveStatus uses it to decide whether a + // branch-tracked template should be compared by semver or by branch-tip commit. + currentVersion := semverTagAtCommit(repo, head.Hash()) + + return resolveStatus(refs, head.Hash(), branch, currentVersion) +} + +// semverTagAtCommit returns the highest valid semver tag that points directly at +// hash (dereferencing annotated tags), or "" when no semver tag is exactly on that +// commit. This tells CheckRemoteContext whether the local checkout is pinned to a +// released version — as opposed to sitting on an arbitrary branch commit — without +// relying on git-describe output, whose "--g" suffix would otherwise be +// misread as a semver pre-release. +func semverTagAtCommit(repo *gogit.Repository, hash plumbing.Hash) string { + tags, err := repo.Tags() + if err != nil { + return "" + } + var best *semver.Version + var bestOrig string + _ = tags.ForEach(func(ref *plumbing.Reference) error { + h := ref.Hash() + if obj, err := repo.TagObject(h); err == nil { + h = obj.Target // annotated tag: resolve to the commit it points at + } + if h != hash { + return nil + } + v, err := semver.NewVersion(ref.Name().Short()) + if err != nil { + return nil + } + if best == nil || v.GreaterThan(best) { + best = v + bestOrig = v.Original() + } + return nil + }) + return bestOrig } // CheckRemote is a context-free convenience wrapper around CheckRemoteContext. @@ -396,7 +435,11 @@ func classifyRemoteError(err error) CheckErrorKind { // resolveStatus compares remote refs against the local HEAD for the given ref. // Tag-first resolution is used, consistent with Clone behaviour. -func resolveStatus(refs []*plumbing.Reference, localHead plumbing.Hash, ref string) RemoteCheckResult { +// +// currentVersion is the semver tag the local checkout sits exactly on (or ""). +// It is only consulted for branch-tracked templates: when it is a valid semver +// version the check is resolved by semver rather than by branch-tip commit (see below). +func resolveStatus(refs []*plumbing.Reference, localHead plumbing.Hash, ref, currentVersion string) RemoteCheckResult { tagRef := plumbing.NewTagReferenceName(ref) branchRef := plumbing.NewBranchReferenceName(ref) @@ -421,6 +464,18 @@ func resolveStatus(refs []*plumbing.Reference, localHead plumbing.Hash, ref stri // Branch fallback. for _, r := range refs { if r.Name() == branchRef { + // When the local checkout sits exactly on a released semver version, a newer + // version must be a strictly-greater semver tag — not merely a newer commit on + // the branch. This stops a lower-numbered tag pushed on a later commit (e.g. + // 1.0.1 after 1.1.0) from being reported as an update (issue #83). Rolling + // branches with no semver version fall back to comparing the branch-tip commit. + if _, err := semver.NewVersion(currentVersion); err == nil { + latest := latestSemverTag(remoteTags, currentVersion) + if latest == "" || latest == currentVersion { + return RemoteCheckResult{IsUpToDate: true} + } + return RemoteCheckResult{IsUpToDate: false, LatestVersion: latest} + } return RemoteCheckResult{IsUpToDate: r.Hash() == localHead} } } diff --git a/internal/util/git/remote_check_test.go b/internal/util/git/remote_check_test.go index 66adb1d..2fc07e7 100644 --- a/internal/util/git/remote_check_test.go +++ b/internal/util/git/remote_check_test.go @@ -4,9 +4,14 @@ import ( "errors" "fmt" "net" + "os" + "path/filepath" "testing" + "time" + gogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/plumbing/transport" ) @@ -55,7 +60,7 @@ func TestResolveStatus_BranchUpToDate(t *testing.T) { refs := []*plumbing.Reference{ plumbing.NewHashReference(plumbing.NewBranchReferenceName("main"), hashA), } - result := resolveStatus(refs, hashA, "main") + result := resolveStatus(refs, hashA, "main", "") if !result.IsUpToDate { t.Error("expected IsUpToDate = true when branch hash matches local HEAD") } @@ -68,7 +73,7 @@ func TestResolveStatus_BranchBehind(t *testing.T) { refs := []*plumbing.Reference{ plumbing.NewHashReference(plumbing.NewBranchReferenceName("main"), hashB), } - result := resolveStatus(refs, hashA, "main") + result := resolveStatus(refs, hashA, "main", "") if result.IsUpToDate { t.Error("expected IsUpToDate = false when branch hash differs from local HEAD") } @@ -77,11 +82,59 @@ func TestResolveStatus_BranchBehind(t *testing.T) { } } +// TestResolveStatus_BranchOnSemverTagNotOutdatedByLowerTag reproduces issue #83: +// a branch-tracked template checked out on 1.1.0 must not be reported as outdated +// when the branch advances to a commit tagged with a lower version (1.0.1). +func TestResolveStatus_BranchOnSemverTagNotOutdatedByLowerTag(t *testing.T) { + refs := []*plumbing.Reference{ + // Branch tip moved to the 1.0.1 commit (hashB), local HEAD is still on 1.1.0 (hashA). + plumbing.NewHashReference(plumbing.NewBranchReferenceName("main"), hashB), + plumbing.NewHashReference(plumbing.NewTagReferenceName("1.1.0"), hashA), + plumbing.NewHashReference(plumbing.NewTagReferenceName("1.0.1"), hashB), + } + result := resolveStatus(refs, hashA, "main", "1.1.0") + if !result.IsUpToDate { + t.Error("expected IsUpToDate = true: 1.0.1 is not a semver upgrade over 1.1.0") + } + if result.LatestVersion != "" { + t.Errorf("expected empty LatestVersion, got %q", result.LatestVersion) + } +} + +// TestResolveStatus_BranchOnSemverTagUpgradesToHigherTag verifies the branch path +// still surfaces a genuinely higher semver tag as an update. +func TestResolveStatus_BranchOnSemverTagUpgradesToHigherTag(t *testing.T) { + refs := []*plumbing.Reference{ + plumbing.NewHashReference(plumbing.NewBranchReferenceName("main"), hashB), + plumbing.NewHashReference(plumbing.NewTagReferenceName("1.1.0"), hashA), + plumbing.NewHashReference(plumbing.NewTagReferenceName("1.2.0"), hashB), + } + result := resolveStatus(refs, hashA, "main", "1.1.0") + if result.IsUpToDate { + t.Error("expected IsUpToDate = false when a higher semver tag exists") + } + if result.LatestVersion != "1.2.0" { + t.Errorf("LatestVersion: got %q, want %q", result.LatestVersion, "1.2.0") + } +} + +// TestResolveStatus_BranchNonSemverFallsBackToCommit verifies that a rolling branch +// whose checkout is not on a released version still tracks the branch tip by commit. +func TestResolveStatus_BranchNonSemverFallsBackToCommit(t *testing.T) { + refs := []*plumbing.Reference{ + plumbing.NewHashReference(plumbing.NewBranchReferenceName("main"), hashB), + } + result := resolveStatus(refs, hashA, "main", "") + if result.IsUpToDate { + t.Error("expected IsUpToDate = false: non-semver checkout falls back to commit comparison") + } +} + func TestResolveStatus_TagAlreadyLatest(t *testing.T) { refs := []*plumbing.Reference{ plumbing.NewHashReference(plumbing.NewTagReferenceName("v1.0.0"), hashA), } - result := resolveStatus(refs, hashA, "v1.0.0") + result := resolveStatus(refs, hashA, "v1.0.0", "v1.0.0") if !result.IsUpToDate { t.Error("expected IsUpToDate = true when on latest semver tag") } @@ -95,7 +148,7 @@ func TestResolveStatus_TagNewerExists(t *testing.T) { plumbing.NewHashReference(plumbing.NewTagReferenceName("v1.0.0"), hashA), plumbing.NewHashReference(plumbing.NewTagReferenceName("v2.0.0"), hashB), } - result := resolveStatus(refs, hashA, "v1.0.0") + result := resolveStatus(refs, hashA, "v1.0.0", "v1.0.0") if result.IsUpToDate { t.Error("expected IsUpToDate = false when newer tag exists") } @@ -105,17 +158,67 @@ func TestResolveStatus_TagNewerExists(t *testing.T) { } func TestResolveStatus_RefNotFound(t *testing.T) { - result := resolveStatus(nil, plumbing.ZeroHash, "main") + result := resolveStatus(nil, plumbing.ZeroHash, "main", "") if result.ErrorKind != CheckErrorNotFound { t.Errorf("expected CheckErrorNotFound, got %q", result.ErrorKind) } } +// commitFile stages a file and commits it, returning the new commit hash. +func commitFile(t *testing.T, repo *gogit.Repository, dir, name, msg string) plumbing.Hash { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(name), 0644); err != nil { + t.Fatalf("write file: %v", err) + } + wt, err := repo.Worktree() + if err != nil { + t.Fatalf("worktree: %v", err) + } + if _, err := wt.Add(name); err != nil { + t.Fatalf("add: %v", err) + } + sig := &object.Signature{Name: "T", Email: "t@example.com", When: time.Unix(0, 0).UTC()} + h, err := wt.Commit(msg, &gogit.CommitOptions{Author: sig, Committer: sig}) + if err != nil { + t.Fatalf("commit: %v", err) + } + return h +} + +func TestSemverTagAtCommit(t *testing.T) { + dir := t.TempDir() + repo, err := gogit.PlainInit(dir, false) + if err != nil { + t.Fatalf("init: %v", err) + } + + tagged := commitFile(t, repo, dir, "a", "first") + // A lightweight and an (higher) annotated tag both on the same commit. + if _, err := repo.CreateTag("1.1.0", tagged, nil); err != nil { + t.Fatalf("lightweight tag: %v", err) + } + sig := &object.Signature{Name: "T", Email: "t@example.com", When: time.Unix(0, 0).UTC()} + if _, err := repo.CreateTag("v1.2.0", tagged, &gogit.CreateTagOptions{Message: "release", Tagger: sig}); err != nil { + t.Fatalf("annotated tag: %v", err) + } + + untagged := commitFile(t, repo, dir, "b", "second") + + // Highest semver tag at the commit wins, and annotated tags are dereferenced. + if got := semverTagAtCommit(repo, tagged); got != "v1.2.0" { + t.Errorf("semverTagAtCommit(tagged): got %q, want %q", got, "v1.2.0") + } + // A commit with no tag on it yields no version. + if got := semverTagAtCommit(repo, untagged); got != "" { + t.Errorf("semverTagAtCommit(untagged): got %q, want empty string", got) + } +} + func TestLatestSemverTag_NewerExists(t *testing.T) { tags := map[string]struct{}{ - "v1.0.0": {}, - "v1.1.0": {}, - "v2.0.0": {}, + "v1.0.0": {}, + "v1.1.0": {}, + "v2.0.0": {}, "not-semver": {}, } got := latestSemverTag(tags, "v1.1.0")