Skip to content

Commit a6f2d18

Browse files
committed
fix: use cached state for version check notification
The previous non-blocking select pattern raced against the GitHub API goroutine and the notification never appeared. Now reads cached state from the previous run to display immediately, and refreshes the cache in a fire-and-forget background goroutine for the next run.
1 parent 36cac5a commit a6f2d18

3 files changed

Lines changed: 89 additions & 24 deletions

File tree

internal/cli/root.go

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ var (
9191
flagBaseURL string
9292
)
9393

94-
var updateResultCh chan *update.CheckResult
94+
var updateNotice *update.CheckResult
9595

9696
var rootCmd = &cobra.Command{
9797
Use: "flashduty",
@@ -100,38 +100,26 @@ var rootCmd = &cobra.Command{
100100
SilenceUsage: true,
101101
SilenceErrors: true,
102102
PersistentPreRun: func(cmd *cobra.Command, _ []string) {
103-
path := cmd.CommandPath()
104-
if path == "flashduty update" || path == "flashduty version" {
105-
return
106-
}
107-
if !update.ShouldCheck(versionStr) {
103+
if cmd.CommandPath() == "flashduty update" {
108104
return
109105
}
110106
if !term.IsTerminal(int(os.Stderr.Fd())) {
111107
return
112108
}
113-
updateResultCh = make(chan *update.CheckResult, 1)
114-
go func() {
115-
result, err := update.CheckForUpdate(versionStr)
116-
if err != nil {
117-
return
118-
}
119-
updateResultCh <- result
120-
}()
109+
updateNotice = update.StateHasUpdate(versionStr)
110+
if update.ShouldCheck(versionStr) {
111+
go func() {
112+
_, _ = update.CheckForUpdate(versionStr)
113+
}()
114+
}
121115
},
122116
PersistentPostRun: func(_ *cobra.Command, _ []string) {
123-
if updateResultCh == nil {
117+
if updateNotice == nil {
124118
return
125119
}
126-
select {
127-
case result := <-updateResultCh:
128-
if result != nil && result.UpdateAvailable {
129-
fmt.Fprintf(os.Stderr, "\nA new version of flashduty is available: v%s -> %s\n",
130-
update.StripV(result.CurrentVersion), result.LatestVersion)
131-
fmt.Fprintf(os.Stderr, "To update, run: flashduty update\n")
132-
}
133-
default:
134-
}
120+
_, _ = fmt.Fprintf(os.Stderr, "\nA new version of flashduty is available: v%s -> %s\n",
121+
update.StripV(updateNotice.CurrentVersion), updateNotice.LatestVersion)
122+
_, _ = fmt.Fprintf(os.Stderr, "To update, run: flashduty update\n")
135123
},
136124
}
137125

internal/update/check.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,22 @@ func CheckForUpdate(currentVersion string) (*CheckResult, error) {
197197
UpdateAvailable: IsNewer(tag, currentVersion),
198198
}, nil
199199
}
200+
201+
func StateHasUpdate(currentVersion string) *CheckResult {
202+
if currentVersion == "dev" || currentVersion == "(devel)" {
203+
return nil
204+
}
205+
state := loadState()
206+
if state.LatestVersion == "" {
207+
return nil
208+
}
209+
if !IsNewer(state.LatestVersion, currentVersion) {
210+
return nil
211+
}
212+
return &CheckResult{
213+
CurrentVersion: currentVersion,
214+
LatestVersion: state.LatestVersion,
215+
LatestURL: state.LatestURL,
216+
UpdateAvailable: true,
217+
}
218+
}

internal/update/check_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,3 +306,61 @@ func TestCheckForUpdate(t *testing.T) {
306306
t.Errorf("state.LatestVersion = %q, want %q", state.LatestVersion, "v0.7.0")
307307
}
308308
}
309+
310+
func TestStateHasUpdate(t *testing.T) {
311+
tmp := t.TempDir()
312+
setTestHome(t, tmp)
313+
314+
if got := StateHasUpdate("0.6.0"); got != nil {
315+
t.Error("StateHasUpdate should return nil when no state file exists")
316+
}
317+
318+
_ = saveState(&State{
319+
CheckedAt: time.Now(),
320+
LatestVersion: "v0.7.0",
321+
LatestURL: "https://example.com/v0.7.0",
322+
})
323+
324+
got := StateHasUpdate("0.6.0")
325+
if got == nil {
326+
t.Fatal("StateHasUpdate should return non-nil when update is available")
327+
}
328+
if !got.UpdateAvailable {
329+
t.Error("UpdateAvailable = false, want true")
330+
}
331+
if got.LatestVersion != "v0.7.0" {
332+
t.Errorf("LatestVersion = %q, want %q", got.LatestVersion, "v0.7.0")
333+
}
334+
}
335+
336+
func TestStateHasUpdate_AlreadyCurrent(t *testing.T) {
337+
tmp := t.TempDir()
338+
setTestHome(t, tmp)
339+
340+
_ = saveState(&State{
341+
CheckedAt: time.Now(),
342+
LatestVersion: "v0.6.0",
343+
LatestURL: "https://example.com/v0.6.0",
344+
})
345+
346+
if got := StateHasUpdate("0.6.0"); got != nil {
347+
t.Error("StateHasUpdate should return nil when already up to date")
348+
}
349+
}
350+
351+
func TestStateHasUpdate_DevVersion(t *testing.T) {
352+
tmp := t.TempDir()
353+
setTestHome(t, tmp)
354+
355+
_ = saveState(&State{
356+
CheckedAt: time.Now(),
357+
LatestVersion: "v1.0.0",
358+
})
359+
360+
if got := StateHasUpdate("dev"); got != nil {
361+
t.Error("StateHasUpdate should return nil for dev version")
362+
}
363+
if got := StateHasUpdate("(devel)"); got != nil {
364+
t.Error("StateHasUpdate should return nil for (devel) version")
365+
}
366+
}

0 commit comments

Comments
 (0)