Skip to content

Commit ec201f6

Browse files
feat: add the upgrade command (#24)
* feat(upgrade): add hand-rolled semver comparison * feat(upgrade): detect the installation method from the binary path * feat(upgrade): derive release asset names and parse checksums.txt * feat(upgrade): add the GitHub Releases client * feat(upgrade): add atomic binary replacement for unix and windows * fix(upgrade): harden windows binary replacement rollback * feat(upgrade): download, verify and swap the binary for manual installs * test(upgrade): cover the zip extraction path and missing checksum entry extractZip had no test coverage even though it is the only extraction path a real Windows manual install exercises, and a stale skip comment claimed otherwise. Also add coverage for checksums.txt downloading successfully but omitting an entry for our asset, which must fail closed rather than pass silently. * feat(upgrade): delegate package-managed installs to their own manager * feat(cli): add the upgrade command * fix(cli): emit a JSON report for upgrade --json on unparseable versions Previously a dev/unparseable build under --json fell into the dev-build branch, printed human text to stderr, and returned nil with empty stdout and exit 0 — leaving a parsing script unable to distinguish 'no update available' from 'the command did nothing'. --json now always emits a report object, even when the current version can't be parsed. * docs: document the upgrade command * fix(cli): verify a manual upgrade's binary reports the new version After a manual replacement, nothing re-ran the new binary to confirm the swap took effect. If another commitbrief comes earlier on the user's PATH, the file swap succeeds but the command they type is still the old binary, and 'Upgraded to vX.Y.Z' would report success regardless. verifyReplacement re-runs <exe> --version and warns (never fails) on a mismatch; manual path only, since a package manager may relocate its own binary. reportsVersion trims the tag's leading v before comparing, since goreleaser injects the version without one. * fix(upgrade): correct timeout, verify, cleanup, and error-mapping bugs Final review fix wave on the upgrade command before merge: - Download used the 30s whole-request API client timeout, which counts time spent reading the body; a slow connection on a ~12 MiB archive failed permanently. Downloads now use a separate client with no whole-request timeout, bounded by a 30s ResponseHeaderTimeout instead. The API client's own timeout is restored to the originally specified 10s. - verifyReplacement's mismatch warning blamed PATH shadowing, but it execs the resolved binary directly and cannot observe PATH. Split into two independent checks: a version-mismatch warning that says only what it knows, and a new PATH-shadow check (LookPath + EvalSymlinks) that can actually detect a shadowing commitbrief and names both paths. - CleanupStale only swept the Windows .old file; interrupted manual installs left .commitbrief-dl-*/.commitbrief-bin-* scratch files behind forever. It's now shared across platforms and also sweeps those temps. - --check skipped the write-permission gate on the manual path, so it could report an install that a real run would refuse; it now runs the same preflight and warns without failing. - A malformed API response was reported as a network failure; it now maps to a distinct ErrBadResponse sentinel and message. - Fixed a tautological mode-preservation test and removed a dead no-op string replacement in path normalization. Docs (.ssot, wiki) corrected to match the accurate two-check verify rationale and the --check permission-gate behavior. * fix(upgrade): clone DefaultTransport for downloads, fix Windows path shadow check Second review round on the timeout/verify fixes from the previous commit: - Assets' transport was a bare &http.Transport{}, not DefaultTransport with ResponseHeaderTimeout changed. A zero-valued Transport drops Proxy (so HTTPS_PROXY/HTTP_PROXY was silently ignored for downloads, while the API client still honored it — a proxied user would see an update detected and then fail to download it) and DialContext/TLSHandshakeTimeout (so a blackholed connect or stalled handshake had no bound at all, since ResponseHeaderTimeout only starts after connect+TLS finish). Now built via http.DefaultTransport.(*http.Transport).Clone() with only ResponseHeaderTimeout overridden. - The download test asserted nothing about NewClient's actual client — it overrode Assets with its own and used a delay far short of any timeout it claimed to exercise, so it passed even against a broken Assets config. Replaced with a structural test that reads NewClient's fields directly; kept a corrected functional test alongside it. - shadowingPath compared paths with ==, which can false-positive on Windows over letter case or separator style that EvalSymlinks doesn't normalize. Exported detect.go's existing normalizePath logic as SamePath and used it instead — normalizePath itself is untouched.
1 parent 6b3521e commit ec201f6

59 files changed

Lines changed: 3609 additions & 126 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,20 @@ and the project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/v
1010
1111
## [Unreleased]
1212

13+
### Added
14+
- **`commitbrief upgrade` — in-tool updates across every install method (ADR-0034).**
15+
Detects whether the running binary came from Homebrew, Scoop, `go install`
16+
or a GitHub Releases tarball. Package-managed installs are delegated to
17+
their own manager (`brew upgrade` / `scoop update` / `go install …@latest`)
18+
because overwriting a manager-owned binary desynchronizes its metadata;
19+
only a manual install is replaced in place, after its SHA-256 is verified
20+
against the release `checksums.txt`. An unwritable target aborts *before*
21+
anything is downloaded and prints the exact command to run — CommitBrief
22+
never invokes `sudo` itself. `--check` reports without installing and
23+
always exits 0; `--json` implies `--check`.
24+
The version check runs **only** when you invoke the command: there is no
25+
automatic update check and no telemetry.
26+
1327
## [1.14.0] - 2026-07-25
1428

1529
### Added

README.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,24 @@ Pre-built binaries for Linux, macOS, and Windows on amd64 and arm64 are
104104
attached to each tagged release at
105105
[github.com/CommitBrief/commitbrief/releases](https://github.com/CommitBrief/commitbrief/releases).
106106

107+
### Upgrade
108+
109+
```sh
110+
commitbrief upgrade # check, confirm, install
111+
commitbrief upgrade --check # report only; install nothing
112+
```
113+
114+
`upgrade` detects how the binary was installed and does the right thing for
115+
it: Homebrew, Scoop and `go install` are handed to their own package
116+
manager, while a manually installed binary is downloaded from GitHub
117+
Releases, SHA-256 verified against the release checksums, and swapped in
118+
place. If the binary's directory is not writable, nothing is downloaded and
119+
the exact command you need is printed — CommitBrief never runs `sudo`
120+
itself. Only the binary is replaced; bundled man pages are not installed.
121+
122+
This is the only network request CommitBrief makes on its own behalf, and
123+
only when you run this command. There is no automatic update check.
124+
107125
## Stability
108126

109127
The v1.0.0 line is an **API freeze**. CLI flag surface, the JSON
@@ -222,6 +240,7 @@ commitbrief init [--force] # write COMMITBRIEF.md + OUTPUT.md
222240
commitbrief compress [--level=balanced] [--dry-run] # shrink COMMITBRIEF.md (preview first if you want)
223241
commitbrief doctor # health-check the pipeline
224242
commitbrief install-hook [--hook=...] # install a git hook that runs commitbrief
243+
commitbrief upgrade [--check] # check GitHub Releases and install a newer CommitBrief
225244
commitbrief dry-run # pipeline preview; no API call
226245
commitbrief list # command reference
227246
commitbrief mcp # run an MCP server over stdio (agent review gate; see "MCP server")

internal/cli/cli_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111

1212
func TestRootCommandHasSubcommands(t *testing.T) {
1313
root := newRootCmd()
14-
want := []string{"cache", "commit", "compress", "config", "diff", "doctor", "dry-run", "guard", "init", "install-hook", "list", "mcp", "providers", "remote", "setup", "summary"}
14+
want := []string{"cache", "commit", "compress", "config", "diff", "doctor", "dry-run", "guard", "init", "install-hook", "list", "mcp", "providers", "remote", "setup", "summary", "upgrade"}
1515
got := []string{}
1616
for _, c := range root.Commands() {
1717
// cobra adds `help` and `completion` automatically; filter to ours.

internal/cli/root.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ func newRootCmd() *cobra.Command {
169169
newSummaryCmd(),
170170
newMCPCmd(),
171171
newGuardCmd(),
172+
newUpgradeCmd(),
172173
)
173174
return cmd
174175
}

internal/cli/upgrade.go

Lines changed: 306 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,306 @@
1+
// SPDX-License-Identifier: GPL-3.0-or-later
2+
3+
package cli
4+
5+
import (
6+
"encoding/json"
7+
"errors"
8+
"fmt"
9+
"io"
10+
"os"
11+
"os/exec"
12+
"path/filepath"
13+
"runtime"
14+
"strings"
15+
16+
"github.com/spf13/cobra"
17+
18+
"github.com/CommitBrief/commitbrief/internal/ui"
19+
"github.com/CommitBrief/commitbrief/internal/upgrade"
20+
"github.com/CommitBrief/commitbrief/internal/version"
21+
)
22+
23+
// upgradeReport is the --json payload. It is intentionally NOT review
24+
// schema v1 — it describes an installation, not a review, and nothing
25+
// in the findings contract changes because of it.
26+
type upgradeReport struct {
27+
Current string `json:"current"`
28+
Latest string `json:"latest"`
29+
Method string `json:"method"`
30+
UpdateAvailable bool `json:"update_available"`
31+
Action string `json:"action"`
32+
}
33+
34+
func writeUpgradeJSON(w io.Writer, rep upgradeReport) error {
35+
enc := json.NewEncoder(w)
36+
enc.SetIndent("", " ")
37+
return enc.Encode(rep)
38+
}
39+
40+
func newUpgradeCmd() *cobra.Command {
41+
var checkOnly bool
42+
cmd := &cobra.Command{
43+
Use: "upgrade",
44+
Short: "Check for a newer CommitBrief and install it",
45+
Long: `Checks GitHub Releases for a newer CommitBrief and installs it.
46+
47+
How the binary was installed decides what happens. A Homebrew, Scoop or
48+
'go install' install is upgraded by its own package manager, because
49+
overwriting a manager-owned binary desynchronizes its metadata. Only a
50+
manually installed binary (a GitHub Releases tarball) is downloaded,
51+
SHA-256 verified against the release checksums, and replaced in place.
52+
53+
Nothing is installed without confirmation, and nothing is downloaded if
54+
the target cannot be written — CommitBrief never invokes sudo itself.
55+
56+
--check reports what would happen and installs nothing. --json implies
57+
--check: it prints a single report object and exits.
58+
59+
This is the only network request CommitBrief makes on its own behalf,
60+
and only when you run this command. There is no automatic update check
61+
and no telemetry.`,
62+
Args: cobra.NoArgs,
63+
RunE: func(cmd *cobra.Command, args []string) error {
64+
return runUpgrade(cmd, checkOnly || global.json)
65+
},
66+
}
67+
cmd.Flags().BoolVar(&checkOnly, "check", false, "only report whether a newer version exists; install nothing")
68+
return cmd
69+
}
70+
71+
func runUpgrade(cmd *cobra.Command, checkOnly bool) error {
72+
app, err := resolveContext(false)
73+
if err != nil {
74+
return err
75+
}
76+
cat := app.Catalog
77+
out := cmd.OutOrStdout()
78+
msg := cmd.ErrOrStderr() // human chatter goes to stderr so --json stdout stays clean
79+
80+
exe, err := upgrade.ResolveExe()
81+
if err != nil {
82+
return err
83+
}
84+
// Sweep the moved-aside binary a previous Windows upgrade could not
85+
// delete while it was still executing.
86+
upgrade.CleanupStale(exe)
87+
88+
method := upgrade.Detect(upgrade.CurrentEnv(exe))
89+
90+
current, ok := upgrade.ParseVersion(version.Version)
91+
if !ok {
92+
// --json promises a report object on every non-error run. A dev
93+
// build must therefore still emit one: returning nil with empty
94+
// stdout would hand a parsing script silence and exit 0, with no
95+
// way to tell "no update" from "the command did nothing".
96+
if global.json {
97+
return writeUpgradeJSON(out, upgradeReport{
98+
Current: version.Version,
99+
Method: string(method),
100+
UpdateAvailable: false,
101+
})
102+
}
103+
if checkOnly {
104+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.dev_build", version.Version))
105+
return nil
106+
}
107+
return errors.New(cat.T("upgrade.dev_build", version.Version))
108+
}
109+
110+
client := upgrade.NewClient(version.Version)
111+
rel, err := client.Latest(cmd.Context())
112+
if err != nil {
113+
switch {
114+
case errors.Is(err, upgrade.ErrRateLimited):
115+
return errors.New(cat.T("upgrade.err_rate_limited"))
116+
case errors.Is(err, upgrade.ErrNoRelease):
117+
return errors.New(cat.T("upgrade.err_no_release"))
118+
case errors.Is(err, upgrade.ErrBadResponse):
119+
// The server was reached and answered; the payload just
120+
// didn't parse. That is distinct from a network failure and
121+
// must not be reported as one.
122+
return errors.New(cat.T("upgrade.err_bad_response", err))
123+
default:
124+
return errors.New(cat.T("upgrade.err_network", err))
125+
}
126+
}
127+
128+
latest, ok := upgrade.ParseVersion(rel.TagName)
129+
if !ok {
130+
return errors.New(cat.T("upgrade.err_no_release"))
131+
}
132+
133+
argv := upgrade.Command(method)
134+
action := strings.Join(argv, " ")
135+
if method == upgrade.MethodManual {
136+
action = upgrade.AssetName(rel.TagName, runtime.GOOS, runtime.GOARCH)
137+
}
138+
139+
rep := upgradeReport{
140+
Current: current.String(),
141+
Latest: latest.String(),
142+
Method: string(method),
143+
UpdateAvailable: current.Compare(latest) < 0,
144+
Action: action,
145+
}
146+
147+
if !rep.UpdateAvailable {
148+
if global.json {
149+
rep.Action = ""
150+
return writeUpgradeJSON(out, rep)
151+
}
152+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.up_to_date", current.String()))
153+
return nil
154+
}
155+
156+
if global.json {
157+
return writeUpgradeJSON(out, rep)
158+
}
159+
160+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.available", current.String(), latest.String()))
161+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.method", string(method)))
162+
if method == upgrade.MethodManual {
163+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.action_manual", action))
164+
} else {
165+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.action_managed", action))
166+
}
167+
168+
if checkOnly {
169+
// Surface the same permission gate a real run would hit, so a
170+
// root-owned or read-only manual install is reported honestly
171+
// instead of promising an install that would actually be
172+
// refused. Never reached when --json implied checkOnly: that
173+
// path already returned above, and --json prints only the
174+
// report object. A warning only — --check always exits 0.
175+
if method == upgrade.MethodManual {
176+
if err := upgrade.PreflightWritable(exe); err != nil {
177+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.err_not_writable", exe))
178+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.hint_manual"))
179+
_, _ = fmt.Fprintf(msg, " sudo %s upgrade\n", exe)
180+
_, _ = fmt.Fprintf(msg, " %s\n", upgrade.ReleasesPage)
181+
}
182+
}
183+
return nil
184+
}
185+
186+
// NonInteractive mirrors internal/cli/cache.go: without a TTY and
187+
// without --yes the answer is a deterministic "no", so an unattended
188+
// run aborts instead of hanging or half-consuming stdin.
189+
confirmed, err := ui.Confirm(cmd.InOrStdin(), msg, cat.T("upgrade.confirm"), ui.AskOptions{
190+
AssumeYes: global.yes,
191+
Interactive: ui.IsStdinTTY(os.Stdin),
192+
NonInteractive: !ui.IsStdinTTY(os.Stdin),
193+
Catalog: app.Catalog,
194+
})
195+
if err != nil {
196+
return err
197+
}
198+
if !confirmed {
199+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.cancelled"))
200+
return nil
201+
}
202+
203+
if method == upgrade.MethodManual {
204+
if err := upgrade.PreflightWritable(exe); err != nil {
205+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.hint_manual"))
206+
_, _ = fmt.Fprintf(msg, " sudo %s upgrade\n", exe)
207+
_, _ = fmt.Fprintf(msg, " %s\n", upgrade.ReleasesPage)
208+
return errors.New(cat.T("upgrade.err_not_writable", exe))
209+
}
210+
if err := upgrade.InstallManual(cmd.Context(), upgrade.ManualOptions{
211+
Client: client,
212+
Release: rel,
213+
Target: exe,
214+
GOOS: runtime.GOOS,
215+
GOARCH: runtime.GOARCH,
216+
}); err != nil {
217+
switch {
218+
case errors.Is(err, upgrade.ErrChecksumMismatch):
219+
return errors.New(cat.T("upgrade.err_checksum", action))
220+
case errors.Is(err, upgrade.ErrAssetMissing):
221+
return errors.New(cat.T("upgrade.err_asset_missing", runtime.GOOS, runtime.GOARCH, upgrade.ReleasesPage))
222+
case errors.Is(err, upgrade.ErrNotWritable):
223+
return errors.New(cat.T("upgrade.err_not_writable", exe))
224+
default:
225+
return err
226+
}
227+
}
228+
verifyReplacement(cmd, cat, msg, exe, latest)
229+
} else {
230+
if err := upgrade.Run(cmd.Context(), argv, msg, cmd.ErrOrStderr()); err != nil {
231+
if errors.Is(err, upgrade.ErrToolMissing) {
232+
return errors.New(cat.T("upgrade.err_tool_missing", string(method), argv[0]))
233+
}
234+
return err
235+
}
236+
}
237+
238+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.success", latest.String()))
239+
return nil
240+
}
241+
242+
// verifyReplacement re-runs the binary we just swapped and checks two
243+
// independent things, each with its own warning: whether it reports the
244+
// expected version, and whether some other "commitbrief" resolves
245+
// earlier on PATH than the file that was just upgraded. Neither implies
246+
// the other — a version mismatch after execing the resolved `exe`
247+
// directly cannot be explained by PATH shadowing, since exec bypasses
248+
// PATH entirely; the shadow check exists to catch the separate, real
249+
// problem that the command the user types next may still resolve to
250+
// the old binary even though this exact file was upgraded correctly.
251+
//
252+
// Never fatal — the upgrade already happened, and neither a failure to
253+
// re-exec (a sandbox, a hardened mount) nor a shadowing PATH entry is a
254+
// reason to report failure. Applies to the manual path only; a package
255+
// manager may relocate its binary, so `exe` is not necessarily the new
256+
// file after delegation.
257+
func verifyReplacement(cmd *cobra.Command, cat catalog, msg io.Writer, exe string, latest upgrade.Version) {
258+
out, err := exec.CommandContext(cmd.Context(), exe, "--version").Output()
259+
if err != nil {
260+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.verify_failed", err))
261+
} else if reported := strings.TrimSpace(string(out)); !reportsVersion(reported, latest) {
262+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.verify_mismatch", reported, latest.String()))
263+
}
264+
265+
if shadow, ok := shadowingPath(exe); ok {
266+
_, _ = fmt.Fprintln(msg, cat.T("upgrade.verify_shadowed", exe, shadow))
267+
}
268+
}
269+
270+
// shadowingPath reports whatever "commitbrief" the user's shell would
271+
// actually resolve on PATH, if it differs from exe (the binary just
272+
// upgraded). ok is false when PATH has no commitbrief, or it resolves
273+
// to exe itself — neither is worth reporting, and a LookPath failure is
274+
// not an error in its own right, just the common case of a manual
275+
// install that was never put on PATH.
276+
//
277+
// Compares with upgrade.SamePath rather than == : on Windows, exe (from
278+
// os.Executable + EvalSymlinks) and the PATH lookup can differ in
279+
// letter case or `\`-vs-`/` without naming a different file — EvalSymlinks
280+
// normalizes neither — and a plain == would warn about a shadow that
281+
// does not exist.
282+
func shadowingPath(exe string) (shadow string, ok bool) {
283+
found, err := exec.LookPath("commitbrief")
284+
if err != nil {
285+
return "", false
286+
}
287+
resolved, err := filepath.EvalSymlinks(found)
288+
if err != nil {
289+
// A broken or unreadable link is not fatal to the check —
290+
// compare what LookPath found, unresolved.
291+
resolved = found
292+
}
293+
if upgrade.SamePath(resolved, exe, runtime.GOOS) {
294+
return "", false
295+
}
296+
return resolved, true
297+
}
298+
299+
// reportsVersion checks whether --version output names the expected
300+
// release. The leading "v" is trimmed because goreleaser injects the tag
301+
// without it (`-X …version.Version={{.Version}}`), so a released binary
302+
// prints "commitbrief 1.15.0 (…)" while the tag reads "v1.15.0".
303+
// Comparing them verbatim would warn on every successful upgrade.
304+
func reportsVersion(output string, v upgrade.Version) bool {
305+
return strings.Contains(output, strings.TrimPrefix(v.String(), "v"))
306+
}

0 commit comments

Comments
 (0)