erlinerdandClaude Code
94008ce66f
* update: say so when the `magpie` command is a copy the app left behind install.sh links the terminal's magpie into ~/.local/bin to the app's own binary, so it follows every GUI update for free. A command that arrived as a copied file — an older installer, a hand cp — keeps running its own build once magpie.app moves on, and a command behind the app is a stale build: one that predates the WebDAV short-write check (#531) truncated a remote backup on `magpie webdav now`, wedging the sync on every tick after. The gap is macOS-only: on Linux the app and the command are one file, and its update replaces it in place. magpie doesn't rewrite the user's PATH itself — that's the installer's job, and a background update has no business reaching there. What the command *is* says whether it can follow the app, so update.StaleCLI is a plain Lstat of the installer's own convention (MAGPIE_BIN_DIR, else ~/.local/bin): a regular file is behind, a link isn't, and nothing runs the old binary to ask its version. When it's behind, magpie says so where the user can see it: /api/state carries cliBehind and the window's status line says it once per run (the panel stays quiet, too small for advice), and `magpie update` adds the line after updating the app it found through the same copied command. TestStaleCLI pins the three file states over a temp bin dir; cli-behind.test.cjs (WebKit and Chromium, en and zh) fakes a state with cliBehind and checks the window says it, the panel doesn't, and a following command is never mentioned. go vet, and go test -tags nogui ./internal/update ./internal/gui under a temp HOME pass. * update: only say the command is behind when it is, and how to fix it Two things the review caught about the notice. A copy that isn't actually behind the app shouldn't nag. The standalone magpie-cli build is a legitimate regular file in ~/.local/bin beside the app, and the first cut warned about it on every window load however current it was. StaleCLI now takes the app's version and stays quiet unless the copy is a release that trails it — a copy at or ahead of the app, a build from source, and one that won't run are all left be. Reading the version runs the old binary, whose start-up migrates things, but that's the only way to tell a stale release from a current one, and it's reached only for a file already known by Lstat to be a copy, once per run. The advice named a step that didn't fix it. `magpie update` run from a copied command has no bundle to update (the copy isn't inside a .app), so it replaces the copy with another copy and the warning comes back. Both the window's line and `magpie update`'s now point at what actually works: re-run the installer, or link the command by hand — `ln -sf <app>/Contents/MacOS/magpie <cli>`, the link install.sh makes. TestStaleCLI covers the version decision (behind → say, current, ahead, source, unreadable → quiet) over a temp bin dir; the browser test's wording follows. Verified by mutation: dropping the version guard turns the stale case red, and Stat for Lstat turns the link case red. go vet, and go test -tags nogui ./internal/update ./internal/gui under a temp HOME pass. * gui: the window's copied-command advice is an Lstat with a per-version dismiss (yetone's review on #623) What: the window no longer runs the old binary's `version` to tell a stale copy from a current one. update.CopiedCLI is a bare Lstat, reached on every launch; the version read stays in update.StaleCLI, which only `magpie update` calls. The advice moved from a 9s status line to a small dialog with "Hide until the next version", kept in settings' cli-behind-quiet file as the version it was dismissed at, so the next app update asks once more. POST /api/cli-behind/quiet writes it. Why: `magpie version` isn't read-only — run() migrates settings before the command answers, so an old copy rewrote the user's real config with the old build's migrations (old MoveOffAccountIDs could drop fields), and the read on the first /api/state cost up to 2s on a cold start. An Lstat can't tell whether the copy trails, so what the window says is advice, not a warning that knows — dismissable, and worded as a copy rather than a stale one. Tests: TestCLIBehindIsLstatOnlyAndDismissible (a non-executable file is reported — nothing ran; dismiss quiets it and the file holds this version; the installer's symlink is quiet). cli-behind.test.cjs covers the dialog, its dismiss POST and the quiet panel, chromium+webkit, en+zh, 4/4. go vet, go build -tags nogui (darwin, GOOS=linux, GOOS=windows) and go test -tags nogui -count=1 ./... under a temp HOME pass (netproxy's TestBunThroughBridge fails the same way on the branch without this change). Co-Authored-By: Claude Code <noreply@anthropic.com> * gui: the dismiss survives a restart, and cliBehindVal is behind a lock (yetone's review on #623) What: the POST handler wrote cliBehindVal = "" while state() read it on other goroutines with no lock — a page dismissing while another view polled /api/state raced. cliBehindVal now sits behind cliBehindMu on both sides. The test also proves the promise the handler's one-line clear couldn't: after a simulated restart (fresh Once, empty value) the dismissed advice stays quiet, and the next version asks once more. Why: the restart case was untested — short-circuiting cliQuiet() left the suite green, because the handler's in-memory clear alone satisfied it. The quiet file is what actually carries the dismiss across runs. Tests: TestCLIBehindIsLstatOnlyAndDismissible extended with the restart and next-version legs; `if false && cliQuiet()` now fails it (mutation-checked). go vet, go test -tags nogui -race ./internal/gui ./internal/update pass; TestGatewayTakenOver fails the same way with -race on the unmodified branch (pre-existing). Co-Authored-By: Claude Code <noreply@anthropic.com> --------- Co-authored-by: Claude Code <noreply@anthropic.com>
89 lines
2.9 KiB
Go
89 lines
2.9 KiB
Go
package gui
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"runtime"
|
|
"sync"
|
|
"testing"
|
|
)
|
|
|
|
// The window's advice about a copied `magpie` command is a bare Lstat — no
|
|
// version read, because `magpie version` isn't read-only: the old binary
|
|
// runs its old migrations over the user's real settings first. Only
|
|
// `magpie update`, which the user started, reads versions. The advice can
|
|
// be dismissed, and the dismiss is kept per version: the next app update
|
|
// asks once more.
|
|
func TestCLIBehindIsLstatOnlyAndDismissible(t *testing.T) {
|
|
if runtime.GOOS != "darwin" {
|
|
t.Skip("only the Mac keeps the command apart from the app")
|
|
}
|
|
home := t.TempDir()
|
|
t.Setenv("XDG_CONFIG_HOME", home)
|
|
bin := t.TempDir()
|
|
t.Setenv("MAGPIE_BIN_DIR", bin)
|
|
wasV, wasOnce, wasVal := Version, cliBehindOnce, cliBehindVal
|
|
t.Cleanup(func() { Version, cliBehindOnce, cliBehindVal = wasV, wasOnce, wasVal })
|
|
Version = "0.1.500"
|
|
cliBehindOnce = new(sync.Once) // another test may have asked already
|
|
cliBehindVal = ""
|
|
|
|
// a copy: the window says so without running anything (nothing here is
|
|
// executable — a binary would have to be run to report a version)
|
|
cli := filepath.Join(bin, "magpie")
|
|
if err := os.WriteFile(cli, []byte("not executable"), 0o644); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got := cliBehind(); got == "" {
|
|
t.Fatal("cliBehind = empty, want the copy's path: a copied command can't follow updates")
|
|
}
|
|
|
|
// the link the installer makes: quiet
|
|
mux := http.NewServeMux()
|
|
cliBehindRoutes(mux)
|
|
post := func() int {
|
|
rec := httptest.NewRecorder()
|
|
mux.ServeHTTP(rec, httptest.NewRequest("POST", "/api/cli-behind/quiet", nil))
|
|
return rec.Code
|
|
}
|
|
if c := post(); c != http.StatusNoContent {
|
|
t.Fatalf("dismiss: %d", c)
|
|
}
|
|
if got := cliBehind(); got != "" {
|
|
t.Fatalf("cliBehind = %q after dismiss, want empty", got)
|
|
}
|
|
b, err := os.ReadFile(filepath.Join(home, "magpie", cliQuietFile))
|
|
if err != nil || string(b) != Version+"\n" {
|
|
t.Fatalf("dismiss file = %q, %v; want this version, so the next one asks again", b, err)
|
|
}
|
|
|
|
// the dismiss outlives a restart: the copy is still there, but the
|
|
// quiet file still names this version, so a fresh run stays quiet
|
|
cliBehindOnce = new(sync.Once)
|
|
cliBehindVal = ""
|
|
if got := cliBehind(); got != "" {
|
|
t.Fatalf("cliBehind = %q after a restart, want empty: the dismiss is kept per version", got)
|
|
}
|
|
|
|
// the next version asks once more
|
|
Version = "0.1.501"
|
|
cliBehindOnce = new(sync.Once)
|
|
cliBehindVal = ""
|
|
if got := cliBehind(); got != cli {
|
|
t.Fatalf("cliBehind = %q on the next version, want %q: the dismiss only covers the version it was made at", got, tilde(cli))
|
|
}
|
|
|
|
// a link, not a copy: quiet even undismissed
|
|
os.Remove(cli)
|
|
if err := os.Symlink("/Applications/magpie.app/Contents/MacOS/magpie", cli); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
cliBehindOnce = new(sync.Once)
|
|
cliBehindVal = ""
|
|
if got := cliBehind(); got != "" {
|
|
t.Fatalf("cliBehind = %q for the installer's link, want empty", got)
|
|
}
|
|
}
|