a73x

3243b2c9

fix(server): a hand-deployed agent still orders before the next release

a73x   2026-08-06 11:56

Commit message
fix(server): a hand-deployed agent still orders before the next release

Release versions order by the tuple (X, Y, Z, N), reading both a plain tag
"vX.Y.Z" and the git-describe build derived from it, "vX.Y.Z-N-g<hex>", where
N is the commit count past the tag and a plain tag sits at N=0. An agent
deployed from a non-tag commit — "v0.0.2-2-g68f804d" — therefore orders before
"v0.0.3" and is offered the upgrade, while a build ahead of the latest release
— "v0.0.3-5-gabc1234" — never orders before it and is never offered a
downgrade.

The describe suffix is exact: a bare decimal commit count and a "g"-prefixed
non-empty hex abbrev, nothing trailing. A dirty working tree, an unstamped
"dev" build, or any other malformed version stays unparsable and never orders
before anything, so it never sees an upgrade.
internal/server/release/release.go
Old New
@@ -117,15 +117,21 @@ func (c *Client) Poll(ctx context.Context, every time.Duration, onErr func(error
117 } 117 }
118 118
119 // Less reports whether version a orders strictly before b. Versions are 119 // Less reports whether version a orders strictly before b. Versions are
120 // eitri's own tags ("vX.Y.Z"); anything unparsable (e.g. "dev") never orders 120 // eitri's own tags ("vX.Y.Z") and the git-describe builds derived from them
121 // before anything — an unstamped build never sees an upgrade. 121 // ("vX.Y.Z-N-g<hex>", N commits past the tag), ordered by (X, Y, Z, N) with a
122 // plain tag at N=0. So a hand-deployed "v0.0.2-2-g68f804d" still orders before
123 // the "v0.0.3" release and takes the upgrade, while a build ahead of the
124 // latest release ("v0.0.3-5-gabc1234") never orders before it and is never
125 // offered a downgrade. Anything unparsable — "dev", a "-dirty" working tree,
126 // a malformed tag — never orders before anything, so an unstamped build never
127 // sees an upgrade.
122 func Less(a, b string) bool { 128 func Less(a, b string) bool {
123 pa, oka := parse(a) 129 pa, oka := parse(a)
124 pb, okb := parse(b) 130 pb, okb := parse(b)
125 if !oka || !okb { 131 if !oka || !okb {
126 return false 132 return false
127 } 133 }
128 for i := range 3 { 134 for i := range pa {
129 if pa[i] != pb[i] { 135 if pa[i] != pb[i] {
130 return pa[i] < pb[i] 136 return pa[i] < pb[i]
131 } 137 }
@@ -133,18 +139,69 @@ func Less(a, b string) bool {
133 return false 139 return false
134 } 140 }
135 141
136 func parse(v string) ([3]int, bool) { 142 // parse reads "vX.Y.Z" or "vX.Y.Z-N-g<hex>" into the ordering tuple
137 var out [3]int 143 // (X, Y, Z, N); a plain tag carries N=0. The describe suffix must be exactly a
138 parts := strings.SplitN(strings.TrimPrefix(v, "v"), ".", 3) 144 // non-negative decimal commit count and a "g"-prefixed non-empty hex abbrev —
145 // a "-dirty" marker or any other trailing text makes the version unparsable.
146 func parse(v string) ([4]int, bool) {
147 var out [4]int
148 core := strings.TrimPrefix(v, "v")
149 if rel, desc, found := strings.Cut(core, "-"); found {
150 core = rel
151 count, abbrev, ok := strings.Cut(desc, "-")
152 if !ok || !isDecimal(count) || !isGitAbbrev(abbrev) {
153 return out, false
154 }
155 n, err := strconv.Atoi(count)
156 if err != nil {
157 return out, false
158 }
159 out[3] = n
160 }
161 parts := strings.SplitN(core, ".", 3)
139 if len(parts) != 3 { 162 if len(parts) != 3 {
140 return out, false 163 return out, false
141 } 164 }
142 for i, p := range parts { 165 for i, p := range parts {
166 if !isDecimal(p) {
167 return out, false
168 }
143 n, err := strconv.Atoi(p) 169 n, err := strconv.Atoi(p)
144 if err != nil || n < 0 { 170 if err != nil {
145 return out, false 171 return out, false
146 } 172 }
147 out[i] = n 173 out[i] = n
148 } 174 }
149 return out, true 175 return out, true
150 } 176 }
177
178 // isDecimal reports whether s is a non-empty run of decimal digits — the sign
179 // Atoi would otherwise accept is not part of a version component.
180 func isDecimal(s string) bool {
181 if s == "" {
182 return false
183 }
184 for _, r := range s {
185 if r < '0' || r > '9' {
186 return false
187 }
188 }
189 return true
190 }
191
192 // isGitAbbrev reports whether s is a "g"-prefixed non-empty hex object abbrev,
193 // the shape git describe appends after the commit count.
194 func isGitAbbrev(s string) bool {
195 hex, ok := strings.CutPrefix(s, "g")
196 if !ok || hex == "" {
197 return false
198 }
199 for _, r := range hex {
200 switch {
201 case r >= '0' && r <= '9', r >= 'a' && r <= 'f', r >= 'A' && r <= 'F':
202 default:
203 return false
204 }
205 }
206 return true
207 }
internal/server/release/release_test.go
Old New
@@ -21,6 +21,8 @@ func TestLess(t *testing.T) {
21 {"v0.9.0", "v0.10.0", true}, // numeric, not lexicographic 21 {"v0.9.0", "v0.10.0", true}, // numeric, not lexicographic
22 {"dev", "v0.0.2", false}, // unparsable never upgrades 22 {"dev", "v0.0.2", false}, // unparsable never upgrades
23 {"v0.0.1", "dev", false}, 23 {"v0.0.1", "dev", false},
24 {"", "v0.0.2", false},
25 {"v0.0.2", "", false},
24 } 26 }
25 for _, c := range cases { 27 for _, c := range cases {
26 if got := Less(c.a, c.b); got != c.want { 28 if got := Less(c.a, c.b); got != c.want {
@@ -29,6 +31,75 @@ func TestLess(t *testing.T) {
29 } 31 }
30 } 32 }
31 33
34 // TestLessOrdersGitDescribeBuilds pins the ordering of a git-describe build
35 // ("vX.Y.Z-N-g<hex>", N commits past tag vX.Y.Z) against a release tag: it
36 // sorts by (X, Y, Z, N), so a build derived from an older tag takes the
37 // upgrade while a build ahead of the latest release is never offered one.
38 func TestLessOrdersGitDescribeBuilds(t *testing.T) {
39 cases := []struct {
40 a, b string
41 want bool
42 }{
43 // A hand-deployed agent two commits past v0.0.2 upgrades to v0.0.3.
44 {"v0.0.2-2-g68f804d", "v0.0.3", true},
45 {"v0.0.3", "v0.0.2-2-g68f804d", false},
46 // A dev build ahead of the latest release is never downgraded onto it.
47 {"v0.0.3-5-gabc1234", "v0.0.3", false},
48 {"v0.0.3", "v0.0.3-5-gabc1234", true},
49 // The commit count breaks ties within one tag.
50 {"v0.0.3-2-gabc1234", "v0.0.3-5-gabc1234", true},
51 {"v0.0.3-5-gabc1234", "v0.0.3-2-gabc1234", false},
52 {"v0.0.3-5-gabc1234", "v0.0.3-5-gabc1234", false},
53 // A plain tag is the same build as zero commits past it.
54 {"v0.0.3-0-gabc1234", "v0.0.3", false},
55 {"v0.0.3", "v0.0.3-0-gabc1234", false},
56 // Release components still outrank the commit count.
57 {"v0.0.2-99-gabc1234", "v0.1.0", true},
58 {"v0.1.0-1-gabc1234", "v0.0.2-99-gabc1234", false},
59 // Uppercase hex is still a hex abbrev.
60 {"v0.0.2-2-gABC1234", "v0.0.3", true},
61 }
62 for _, c := range cases {
63 if got := Less(c.a, c.b); got != c.want {
64 t.Errorf("Less(%q,%q) = %v, want %v", c.a, c.b, got, c.want)
65 }
66 }
67 }
68
69 // TestLessLeavesUnstampedBuildsUnordered pins the shapes that stay
70 // unparsable: a dirty working tree or a malformed version never orders before
71 // anything, in either direction, so it is never offered an upgrade.
72 func TestLessLeavesUnstampedBuildsUnordered(t *testing.T) {
73 unstamped := []string{
74 "",
75 "dev",
76 "v0.0.3-dirty",
77 "v0.0.2-2-g68f804d-dirty",
78 "v0.0.2-x-g123", // commit count is not a number
79 "v0.0.2--2-g68f804d", // negative commit count
80 "v0.0.2-2", // no g-abbrev
81 "v0.0.2-2-g", // empty abbrev
82 "v0.0.2-2-gZZZ", // abbrev is not hex
83 "v0.0.2-2-68f804d", // abbrev missing its g
84 "v0.0.2-2-g68f804d-3", // trailing junk
85 "v-1.0.0", // negative major
86 "v0.-1.2", // negative minor
87 "v0.0", // too few components
88 "v0.0.2.1", // too many components
89 "v0.0.+2", // signed component
90 "v0.0.99999999999999999999", // component out of range
91 "v0.0.2-99999999999999999999-gabc", // commit count out of range
92 }
93 for _, u := range unstamped {
94 if Less(u, "v0.0.3") {
95 t.Errorf("Less(%q, %q) = true, want false", u, "v0.0.3")
96 }
97 if Less("v0.0.3", u) {
98 t.Errorf("Less(%q, %q) = true, want false", "v0.0.3", u)
99 }
100 }
101 }
102
32 func TestRefreshParsesManifest(t *testing.T) { 103 func TestRefreshParsesManifest(t *testing.T) {
33 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { 104 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
34 w.Write([]byte(`{"version":"v0.0.2","artifacts":{"eitri-agent":{"linux/amd64":{"url":"https://eitri.sh/dl/v0.0.2/a.tar.gz","sha256":"ab"}}}}`)) 105 w.Write([]byte(`{"version":"v0.0.2","artifacts":{"eitri-agent":{"linux/amd64":{"url":"https://eitri.sh/dl/v0.0.2/a.tar.gz","sha256":"ab"}}}}`))