From d26ed27c58db4bf687c36c3df210c0b266d5186a Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:05:52 +0000 Subject: [PATCH 1/8] docs: add design spec for platform-aware updater fix (GH #18) Co-Authored-By: Claude Opus 4.6 (1M context) --- ...5-updater-platform-aware-install-design.md | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-05-updater-platform-aware-install-design.md diff --git a/docs/superpowers/specs/2026-04-05-updater-platform-aware-install-design.md b/docs/superpowers/specs/2026-04-05-updater-platform-aware-install-design.md new file mode 100644 index 0000000..8cf420e --- /dev/null +++ b/docs/superpowers/specs/2026-04-05-updater-platform-aware-install-design.md @@ -0,0 +1,106 @@ +# Platform-Aware Update Installation + +**Date:** 2026-04-05 +**Issue:** [#18](https://github.com/0xMMA/KeyLint/issues/18) — App update via "Download & Install" button fails on Windows +**Status:** Approved + +## Problem + +`DownloadAndInstall()` uses `minio/selfupdate.Apply()` to replace the running executable in-place. This fails on Windows for two reasons: + +1. **Wrong asset type**: The Windows release asset (`*-windows-amd64-setup`) is an NSIS installer, not a raw binary. Piping it through `selfupdate.Apply()` would produce a corrupt executable even if it succeeded. +2. **No elevation**: The app installs to `C:\Program Files\KeyLint\KeyLint\`. Writing to Program Files requires admin privileges. `selfupdate.Apply()` creates `.KeyLint.exe.new` in the same directory without UAC elevation, triggering "Access is denied." + +## Solution + +Split `DownloadAndInstall()` into platform-specific install strategies. Both platforms share the download-to-temp phase; the install phase diverges. + +### Flow + +``` +DownloadAndInstall() + 1. CheckForUpdate() // unchanged + 2. HTTP GET release asset → temp file // shared + 3. Platform-specific install: + ├── [windows] os.StartProcess(temp.exe) → return ErrRestartRequired + └── [linux] selfupdate.Apply(tempFile) → return nil +``` + +### Windows Strategy + +1. Download the NSIS installer to `os.CreateTemp("", "KeyLint-update-*.exe")`. +2. Launch the installer via `os.StartProcess` (detached — survives parent exit). The NSIS installer's own manifest requests UAC elevation. +3. Return sentinel `ErrRestartRequired`. The caller (frontend bridge or main.go) uses this to quit the app gracefully so the installer can replace the locked executable. + +### Linux Strategy + +1. Download the raw binary to `os.CreateTemp("", "KeyLint-update-*")`. +2. Apply via `selfupdate.Apply(file, selfupdate.Options{})`. +3. If permission error, return a descriptive message: `"update downloaded to — install manually or run with appropriate permissions"`. + +### Sentinel Error + +```go +var ErrRestartRequired = errors.New("update requires application restart") +``` + +Callers check `errors.Is(err, ErrRestartRequired)` to distinguish "quit now, installer is running" from actual failures. + +## File Changes + +| File | Change | +|------|--------| +| `internal/features/updater/service.go` | Refactor `DownloadAndInstall()` to download to temp, delegate to `applyPlatformUpdate()` | +| `internal/features/updater/install_windows.go` | `applyPlatformUpdate()`: launch NSIS installer, return `ErrRestartRequired` | +| `internal/features/updater/install_linux.go` | `applyPlatformUpdate()`: `selfupdate.Apply()` from temp file | +| `internal/features/updater/install_test.go` | Regression test (prove bug) + new behavior tests | +| `frontend/.../settings.component.ts` | Handle restart-required response (show "closing" message, trigger app quit) | +| `frontend/.../settings.component.spec.ts` | Test for restart-required UI path | +| `frontend/.../wails.service.ts` | No change — `downloadAndInstall()` RPC signature unchanged | + +## Testing Plan + +### Regression Tests (prove the bug) + +1. **Test selfupdate fails on read-only directory**: Create a temp executable in a read-only dir, attempt `selfupdate.Apply()` targeting it, assert permission error. Documents WHY the old approach was replaced. +2. **Test Windows asset is an installer, not a raw binary**: Assert `matchPlatformAsset()` returns a `*-setup*` URL on Windows — proving the downloaded file is an NSIS installer that cannot be used as a binary replacement. + +### Fix Tests (prove it works) + +3. **Test download-to-temp**: httptest server serves fake bytes → assert temp file exists with correct content and size. +4. **Test Windows install path**: Provide a mock/stub process launcher → assert it receives the temp `.exe` path, assert `ErrRestartRequired` is returned. +5. **Test Linux install path**: `selfupdate.Apply()` against a writable temp target → assert success, assert no `ErrRestartRequired`. +6. **Test error paths**: Download returns HTTP 500 → error. Download returns empty body → error. Temp file creation fails → error. + +### Frontend Tests + +7. **Test restart-required UI**: Mock `downloadAndInstall()` to return restart-required → assert "closing" message shown. +8. **Test error UI**: Mock `downloadAndInstall()` to throw → assert error message shown (existing test, verify still passes). + +### Manual Verification + +- Build Windows installer, install to Program Files, trigger update → NSIS wizard launches, app closes, update completes. +- This step cannot be automated in CI (requires Windows GUI + real GitHub release). + +## Frontend UX Change + +Current behavior after successful install: +> "Update installed! Restart the app to use the new version." + +New behavior (Windows): +> "Installing update... the app will close shortly." + +Then the app quits after a brief delay (1-2s) to let the user read the message. The NSIS installer takes over. + +Linux behavior unchanged — same "Restart" message as before. + +## Dependencies + +- No new dependencies. `minio/selfupdate` stays for Linux. `os.StartProcess` is stdlib. +- NSIS installer is already built by CI (`release.yml`). + +## Out of Scope + +- Delta updates, silent background updates (future: Velopack when Go SDK matures). +- Automatic restart after update on Linux. +- macOS support. From 32fc27b18b062109e2cfaa6bfa49d95216cd7a2e Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:16:29 +0000 Subject: [PATCH 2/8] docs: add implementation plan for platform-aware updater fix (GH #18) Co-Authored-By: Claude Opus 4.6 (1M context) --- ...26-04-05-updater-platform-aware-install.md | 687 ++++++++++++++++++ 1 file changed, 687 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-05-updater-platform-aware-install.md diff --git a/docs/superpowers/plans/2026-04-05-updater-platform-aware-install.md b/docs/superpowers/plans/2026-04-05-updater-platform-aware-install.md new file mode 100644 index 0000000..bfe7a6a --- /dev/null +++ b/docs/superpowers/plans/2026-04-05-updater-platform-aware-install.md @@ -0,0 +1,687 @@ +# Platform-Aware Updater Fix — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix GH #18 — the "Download & Install" button fails on Windows because `selfupdate.Apply()` cannot write to `C:\Program Files\` and the downloaded asset is an NSIS installer, not a raw binary. + +**Architecture:** Split `DownloadAndInstall()` into a shared download-to-temp phase and platform-specific install phase. Windows launches the NSIS installer and quits the app. Linux applies the raw binary via `selfupdate.Apply()`. A `quitFunc` callback lets the service trigger app shutdown without importing Wails directly. + +**Tech Stack:** Go 1.26, minio/selfupdate v0.6.0, Wails v3, Angular v21, Vitest + +**Spec:** `docs/superpowers/specs/2026-04-05-updater-platform-aware-install-design.md` + +--- + +## File Structure + +| File | Purpose | +|------|---------| +| `internal/features/updater/model.go` | Add `InstallResult` type | +| `internal/features/updater/service.go` | Refactor `DownloadAndInstall()` — download to temp, delegate to `applyPlatformUpdate()`, add `quitFunc` | +| `internal/features/updater/install_windows.go` | `applyPlatformUpdate()`: launch NSIS installer via `exec.Command` | +| `internal/features/updater/install_linux.go` | `applyPlatformUpdate()`: `selfupdate.Apply()` from temp file | +| `internal/features/updater/install_test.go` | Regression test + all new behavior tests | +| `main.go` | Set `quitFunc` on updater service after Wails app is created | +| `frontend/src/app/core/wails.service.ts` | Update `downloadAndInstall()` return type to `Promise` | +| `frontend/src/testing/wails-mock.ts` | Update mock to return `InstallResult` | +| `frontend/src/app/features/settings/settings.component.ts` | Handle `restart_required` in UI | +| `frontend/src/app/features/settings/settings.component.spec.ts` | Test restart-required UI path | + +--- + +### Task 1: Regression Test — Prove the Bug + +**Files:** +- Create: `internal/features/updater/install_test.go` + +This test documents WHY the old `selfupdate.Apply()` approach was replaced: it fails with a permission error when the executable lives in a read-only directory (like `C:\Program Files\`). + +- [ ] **Step 1: Write the regression test** + +Create `internal/features/updater/install_test.go`: + +```go +package updater + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/minio/selfupdate" +) + +// TestSelfupdateFailsOnReadOnlyDirectory documents the root cause of GH #18: +// selfupdate.Apply() cannot write to directories that require elevated permissions +// (e.g. C:\Program Files\ on Windows). This test ensures we never regress to the +// old in-place replacement approach for directories that aren't user-writable. +func TestSelfupdateFailsOnReadOnlyDirectory(t *testing.T) { + dir := t.TempDir() + fakeBin := filepath.Join(dir, "KeyLint.exe") + if err := os.WriteFile(fakeBin, []byte("old binary"), 0o755); err != nil { + t.Fatal(err) + } + // Make the directory read-only so selfupdate cannot create .KeyLint.exe.new + if err := os.Chmod(dir, 0o555); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(dir, 0o755) }) + + err := selfupdate.Apply(strings.NewReader("new binary"), selfupdate.Options{ + TargetPath: fakeBin, + }) + + if err == nil { + t.Fatal("expected permission error when applying update to read-only directory, got nil — " + + "this means the old selfupdate.Apply() approach would silently corrupt the install") + } + + // The error message varies by OS: "permission denied" (Linux) or "Access is denied" (Windows). + errMsg := strings.ToLower(err.Error()) + if !strings.Contains(errMsg, "permission denied") && !strings.Contains(errMsg, "access is denied") { + t.Errorf("expected permission-related error, got: %v", err) + } +} +``` + +- [ ] **Step 2: Run the test to confirm it passes (proving the bug exists)** + +Run: `go test ./internal/features/updater/ -run TestSelfupdateFailsOnReadOnlyDirectory -v` + +Expected: PASS — the test confirms that `selfupdate.Apply()` fails on a read-only directory, which is exactly the bug from GH #18. + +- [ ] **Step 3: Commit** + +```bash +git add internal/features/updater/install_test.go +git commit -m "test(updater): regression test proving GH #18 selfupdate permission failure" +``` + +--- + +### Task 2: Backend — Add InstallResult Type, applyFunc Hook, and Platform Files + +**Files:** +- Modify: `internal/features/updater/model.go` — add `InstallResult` +- Modify: `internal/features/updater/service.go` — add `quitFunc`, `SetQuitFunc()`, `applyFunc` test hook, refactor `DownloadAndInstall()` +- Create: `internal/features/updater/install_linux.go` — Linux `applyPlatformUpdate()` +- Create: `internal/features/updater/install_windows.go` — Windows `applyPlatformUpdate()` +- Modify: `internal/features/updater/install_test.go` — tests for new download + install flow + +**Design note:** `selfupdate.Apply()` with default options replaces `os.Executable()` — the test runner itself. To avoid corrupting the test binary, the Service has an `applyFunc` field that overrides the platform function in tests. This is only used in tests; production code always uses the real `applyPlatformUpdate`. + +- [ ] **Step 1: Write tests for the new download + install flow** + +Append to `internal/features/updater/install_test.go`: + +```go +import ( + "bytes" + "net/http" + "net/http/httptest" + "os" + // ... existing imports plus these +) + +// serveAsset creates an httptest server that serves the given payload for any request. +func serveAsset(payload []byte) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(payload) + })) +} + +// makeReleaseWithURL creates a release whose asset URL points to a custom server. +func makeReleaseWithURL(tag string, assetName string, assetURL string) githubRelease { + return githubRelease{ + TagName: tag, + Name: tag, + Body: "Release " + tag, + Draft: false, + Prerelease: false, + Assets: []githubAsset{{ + Name: assetName, + BrowserDownloadURL: assetURL, + }}, + } +} + +func TestDownloadAndInstall_DownloadsToTempAndCallsApply(t *testing.T) { + payload := []byte("fake-installer-payload-12345") + assetSrv := serveAsset(payload) + defer assetSrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", assetSrv.URL+"/dl"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + + // Override the platform apply function to capture and verify the temp file + // instead of calling selfupdate.Apply (which would corrupt the test binary). + var capturedPath string + var capturedContent []byte + svc.applyFunc = func(_ *Service, tmpPath string) (InstallResult, error) { + capturedPath = tmpPath + data, err := os.ReadFile(tmpPath) + if err != nil { + t.Fatalf("applyFunc: failed to read temp file: %v", err) + } + capturedContent = data + return InstallResult{RestartRequired: false}, nil + } + + result, err := svc.DownloadAndInstall() + if err != nil { + t.Fatalf("DownloadAndInstall() error: %v", err) + } + if result.RestartRequired { + t.Error("expected RestartRequired=false from test applyFunc") + } + if capturedPath == "" { + t.Fatal("applyFunc was never called — download phase may have failed") + } + if !bytes.Equal(capturedContent, payload) { + t.Errorf("temp file content = %q, want %q", capturedContent, payload) + } +} + +func TestDownloadAndInstall_ApplyFuncRestartRequired(t *testing.T) { + assetSrv := serveAsset([]byte("installer")) + defer assetSrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", assetSrv.URL+"/dl"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + svc.applyFunc = func(_ *Service, _ string) (InstallResult, error) { + return InstallResult{RestartRequired: true}, nil + } + + result, err := svc.DownloadAndInstall() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !result.RestartRequired { + t.Error("expected RestartRequired=true (simulating Windows path)") + } +} + +func TestDownloadAndInstall_NoUpdateAvailable(t *testing.T) { + releases := []githubRelease{ + makeRelease("v1.0.0", false, false, "KeyLint-v1.0.0-linux-amd64"), + } + srv := serveGitHubReleases(t, releases) + defer srv.Close() + + svc := newTestService("1.0.0", srv) + _, err := svc.DownloadAndInstall() + if err == nil { + t.Fatal("expected error for no-update-available") + } + if !strings.Contains(err.Error(), "no update available") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestDownloadAndInstall_DownloadHTTPError(t *testing.T) { + errorSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "not found", http.StatusNotFound) + })) + defer errorSrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", errorSrv.URL+"/missing"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + _, err := svc.DownloadAndInstall() + if err == nil { + t.Fatal("expected error for HTTP 404 download") + } + if !strings.Contains(err.Error(), "404") { + t.Errorf("expected status code in error, got: %v", err) + } +} + +func TestDownloadAndInstall_EmptyBody(t *testing.T) { + emptySrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // 200 OK but empty body + })) + defer emptySrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", emptySrv.URL+"/empty"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + _, err := svc.DownloadAndInstall() + if err == nil { + t.Fatal("expected error for empty download body") + } + if !strings.Contains(err.Error(), "empty") { + t.Errorf("expected 'empty' in error, got: %v", err) + } +} +``` + +- [ ] **Step 2: Run tests to confirm they fail (functions don't exist yet or signatures mismatch)** + +Run: `go test ./internal/features/updater/ -run "TestDownloadAndInstall" -v` + +Expected: FAIL — `DownloadAndInstall()` still returns `error` (not `InstallResult, error`), and `applyFunc` field doesn't exist. + +- [ ] **Step 3: Add InstallResult to model.go** + +Add to `internal/features/updater/model.go`, after the `UpdateInfo` struct: + +```go +// InstallResult is returned by DownloadAndInstall to indicate the outcome. +// On Windows, RestartRequired is true because the NSIS installer needs the app to exit. +type InstallResult struct { + RestartRequired bool `json:"restart_required"` +} +``` + +- [ ] **Step 4: Add quitFunc, applyFunc, and refactor DownloadAndInstall in service.go** + +In `internal/features/updater/service.go`, update the imports, Service struct, add `SetQuitFunc`, and replace `DownloadAndInstall`: + +```go +import ( + "errors" + // ... existing imports plus: + "io" + "os" +) + +type Service struct { + currentVersion string + releasesAPIURL string + client *http.Client + settingsSvc *settings.Service + quitFunc func() // called after launching installer on Windows; set via SetQuitFunc + applyFunc func(svc *Service, tmpPath string) (InstallResult, error) // override for testing; nil uses applyPlatformUpdate +} + +// SetQuitFunc sets the callback invoked after launching the installer on Windows. +// The callback should wait briefly (for the frontend to display a message) then quit the app. +func (s *Service) SetQuitFunc(fn func()) { + s.quitFunc = fn +} +``` + +Replace the `DownloadAndInstall` method entirely: + +```go +// DownloadAndInstall fetches the release asset for the current platform, saves it +// to a temp file, and delegates to the platform-specific installer. +// On Windows this launches the NSIS setup and returns InstallResult{RestartRequired: true}. +// On Linux this applies the binary in-place via selfupdate. +func (s *Service) DownloadAndInstall() (InstallResult, error) { + updateInfo, err := s.CheckForUpdate() + if err != nil { + return InstallResult{}, fmt.Errorf("checking for update: %w", err) + } + if !updateInfo.IsAvailable { + return InstallResult{}, fmt.Errorf("no update available") + } + if updateInfo.ReleaseURL == "" { + return InstallResult{}, fmt.Errorf("no download URL for current platform") + } + + resp, err := s.client.Get(updateInfo.ReleaseURL) + if err != nil { + return InstallResult{}, fmt.Errorf("downloading update: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return InstallResult{}, fmt.Errorf("download returned status %d", resp.StatusCode) + } + + // Write to a temp file so platform-specific code can work with a path on disk. + tmpFile, err := os.CreateTemp("", "KeyLint-update-*.exe") + if err != nil { + return InstallResult{}, fmt.Errorf("creating temp file: %w", err) + } + + n, err := io.Copy(tmpFile, resp.Body) + if err != nil { + tmpFile.Close() + os.Remove(tmpFile.Name()) + return InstallResult{}, fmt.Errorf("writing update to temp file: %w", err) + } + tmpFile.Close() + + if n == 0 { + os.Remove(tmpFile.Name()) + return InstallResult{}, errors.New("downloaded update is empty") + } + + applyFn := applyPlatformUpdate + if s.applyFunc != nil { + applyFn = s.applyFunc + } + return applyFn(s, tmpFile.Name()) +} +``` + +Remove the old unused imports (`"github.com/minio/selfupdate"`) from `service.go` — selfupdate is now only used in `install_linux.go`. + +- [ ] **Step 5: Create the Linux install implementation** + +Create `internal/features/updater/install_linux.go`: + +```go +//go:build !windows + +package updater + +import ( + "fmt" + "os" + + "github.com/minio/selfupdate" +) + +// applyPlatformUpdate applies the downloaded binary in-place using selfupdate. +// On Linux the release asset is a raw executable, so direct replacement works +// as long as the target directory is writable. +func applyPlatformUpdate(svc *Service, tmpPath string) (InstallResult, error) { + f, err := os.Open(tmpPath) + if err != nil { + return InstallResult{}, fmt.Errorf("opening downloaded update: %w", err) + } + defer f.Close() + defer os.Remove(tmpPath) + + if err := selfupdate.Apply(f, selfupdate.Options{}); err != nil { + return InstallResult{}, fmt.Errorf("applying update: %w — if this is a permission error, "+ + "the downloaded file is at %s — install manually or run with appropriate permissions", + err, tmpPath) + } + + return InstallResult{RestartRequired: false}, nil +} +``` + +- [ ] **Step 6: Create the Windows install implementation** + +Create `internal/features/updater/install_windows.go`: + +```go +//go:build windows + +package updater + +import ( + "fmt" + "os" + "os/exec" +) + +// applyPlatformUpdate launches the downloaded NSIS installer and signals the app to quit. +// The NSIS installer's own manifest requests UAC elevation, so no special privileges +// are needed here — Windows will show the UAC prompt to the user. +func applyPlatformUpdate(svc *Service, tmpPath string) (InstallResult, error) { + cmd := exec.Command(tmpPath) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + if err := cmd.Start(); err != nil { + os.Remove(tmpPath) + return InstallResult{}, fmt.Errorf("launching installer: %w", err) + } + + // Schedule app quit so the installer can replace the locked executable. + // The quitFunc is set by main.go and includes a short delay for the + // frontend to display the "closing" message before the app exits. + if svc.quitFunc != nil { + go svc.quitFunc() + } + + return InstallResult{RestartRequired: true}, nil +} +``` + +- [ ] **Step 7: Run all updater tests** + +Run: `go test ./internal/features/updater/ -v` + +Expected: ALL PASS — regression test still passes (proving the old bug), new tests pass with the refactored code. + +- [ ] **Step 8: Commit** + +```bash +git add internal/features/updater/model.go internal/features/updater/service.go \ + internal/features/updater/install_linux.go internal/features/updater/install_windows.go \ + internal/features/updater/install_test.go +git commit -m "fix(updater): platform-aware install — launch NSIS on Windows, selfupdate on Linux + +Fixes #18. The old approach used selfupdate.Apply() which fails on Windows +because (a) the release asset is an NSIS installer, not a raw binary, and +(b) Program Files requires UAC elevation for writes." +``` + +--- + +### Task 3: Wire quitFunc in main.go + +**Files:** +- Modify: `main.go:88-89` — store updater service in a variable, call `SetQuitFunc` + +- [ ] **Step 1: Update main.go to set quitFunc on the updater service** + +In `main.go`, replace line 88–89: + +```go +// OLD: +wailsApp.RegisterService(application.NewService(updater.NewService(AppVersion, services.Settings))) + +// NEW: +updaterSvc := updater.NewService(AppVersion, services.Settings) +updaterSvc.SetQuitFunc(func() { + // Brief delay so the frontend can display the "closing" message. + time.Sleep(2 * time.Second) + wailsApp.Quit() +}) +wailsApp.RegisterService(application.NewService(updaterSvc)) +``` + +Add `"time"` to the imports in `main.go`. + +- [ ] **Step 2: Build to verify compilation** + +Run: `go build -o bin/KeyLint .` + +Expected: Compiles cleanly. + +- [ ] **Step 3: Regenerate Wails bindings (return type changed)** + +Run: `wails3 generate bindings` + +Note: This updates the auto-generated TypeScript bindings in `frontend/bindings/` to reflect the new `(InstallResult, error)` return type. + +- [ ] **Step 4: Commit** + +```bash +git add main.go frontend/bindings/ +git commit -m "fix(updater): wire quitFunc in main.go, regenerate bindings" +``` + +--- + +### Task 4: Frontend — Handle InstallResult + +**Files:** +- Modify: `frontend/src/app/core/wails.service.ts:163-165` — update return type +- Modify: `frontend/src/testing/wails-mock.ts:59` — update mock +- Modify: `frontend/src/app/features/settings/settings.component.ts:524-537,262-268` — handle `restart_required` + +- [ ] **Step 1: Write the frontend test for restart-required** + +Append to the `describe('About tab', ...)` block in `frontend/src/app/features/settings/settings.component.spec.ts`, after the existing `installUpdate()` test: + +```typescript + it('installUpdate() shows restart message when restart_required is true', async () => { + wailsMock.downloadAndInstall.mockResolvedValue({ restart_required: true }); + component.updateInfo = { ...defaultUpdateInfo, is_available: true, latest_version: '3.7.0' }; + await component.installUpdate(); + expect(component.updateSuccess).toBe(true); + expect(component.updateRestartRequired).toBe(true); + }); + + it('installUpdate() shows standard success when restart_required is false', async () => { + wailsMock.downloadAndInstall.mockResolvedValue({ restart_required: false }); + component.updateInfo = { ...defaultUpdateInfo, is_available: true, latest_version: '3.7.0' }; + await component.installUpdate(); + expect(component.updateSuccess).toBe(true); + expect(component.updateRestartRequired).toBe(false); + }); +``` + +- [ ] **Step 2: Run tests to confirm they fail** + +Run: `cd frontend && npm test` + +Expected: FAIL — `updateRestartRequired` property doesn't exist yet. + +- [ ] **Step 3: Update wails.service.ts** + +In `frontend/src/app/core/wails.service.ts`, change the `downloadAndInstall` method. First check the generated bindings to see the exact return type name, then update: + +```typescript +downloadAndInstall(): Promise { + return UpdaterService.DownloadAndInstall(); +} +``` + +Add the `InstallResult` type to the imports from the bindings (or define it in the `wails.service.ts` types section if that's where types live). The type shape is: + +```typescript +export interface InstallResult { + restart_required: boolean; +} +``` + +- [ ] **Step 4: Update wails-mock.ts** + +In `frontend/src/testing/wails-mock.ts`, change line 59: + +```typescript +// OLD: +downloadAndInstall: vi.fn().mockResolvedValue(undefined), + +// NEW: +downloadAndInstall: vi.fn().mockResolvedValue({ restart_required: false }), +``` + +- [ ] **Step 5: Update settings.component.ts — add state and handle result** + +In `frontend/src/app/features/settings/settings.component.ts`, add a new property after `updateSuccess`: + +```typescript +updateRestartRequired = false; +``` + +Update the `installUpdate()` method: + +```typescript +async installUpdate(): Promise { + this.updateInstalling = true; + this.updateError = ''; + this.updateSuccess = false; + this.updateRestartRequired = false; + try { + const result = await this.wails.downloadAndInstall(); + this.updateSuccess = true; + this.updateRestartRequired = result.restart_required; + this.updateInfo = null; + } catch (e) { + this.updateError = `Install failed: ${e instanceof Error ? e.message : String(e)}`; + } finally { + this.updateInstalling = false; + this.cdr.detectChanges(); + } + } +``` + +- [ ] **Step 6: Update the template success message** + +In `frontend/src/app/features/settings/settings.component.ts`, replace the `updateSuccess` message block (lines 262-268): + +```html + @if (updateSuccess) { + + } +``` + +- [ ] **Step 7: Run frontend tests** + +Run: `cd frontend && npm test` + +Expected: ALL PASS — new tests pass, existing `installUpdate()` test passes because the mock now returns `{ restart_required: false }`. + +- [ ] **Step 8: Commit** + +```bash +git add frontend/src/app/core/wails.service.ts \ + frontend/src/testing/wails-mock.ts \ + frontend/src/app/features/settings/settings.component.ts \ + frontend/src/app/features/settings/settings.component.spec.ts +git commit -m "feat(updater): frontend handles restart_required from updater" +``` + +--- + +### Task 5: Final Verification + +- [ ] **Step 1: Run all Go tests** + +Run: `go test ./internal/... -v` + +Expected: ALL PASS. + +- [ ] **Step 2: Run all frontend tests** + +Run: `cd frontend && npm test` + +Expected: ALL PASS, 0 failures. + +- [ ] **Step 3: Build the project** + +Run: `cd frontend && npm run build && cd .. && go build -o bin/KeyLint .` + +Expected: Compiles cleanly. + +- [ ] **Step 4: Review the regression test one more time** + +Run: `go test ./internal/features/updater/ -run TestSelfupdateFailsOnReadOnlyDirectory -v` + +Expected: PASS — the regression test still documents that the old approach would fail, proving the bug existed and our new code path avoids it. + +- [ ] **Step 5: Commit any remaining changes** + +If the bindings regeneration or build produced changes not yet committed: + +```bash +git add -A # review what's staged first +git commit -m "chore(updater): final verification — all tests pass, build clean" +``` From cae5deb6bf80b4502063223e8b96000d5f87b563 Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:18:18 +0000 Subject: [PATCH 3/8] test(updater): regression test proving GH #18 selfupdate permission failure Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/features/updater/install_test.go | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 internal/features/updater/install_test.go diff --git a/internal/features/updater/install_test.go b/internal/features/updater/install_test.go new file mode 100644 index 0000000..4979b43 --- /dev/null +++ b/internal/features/updater/install_test.go @@ -0,0 +1,42 @@ +package updater + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/minio/selfupdate" +) + +// TestSelfupdateFailsOnReadOnlyDirectory documents the root cause of GH #18: +// selfupdate.Apply() cannot write to directories that require elevated permissions +// (e.g. C:\Program Files\ on Windows). This test ensures we never regress to the +// old in-place replacement approach for directories that aren't user-writable. +func TestSelfupdateFailsOnReadOnlyDirectory(t *testing.T) { + dir := t.TempDir() + fakeBin := filepath.Join(dir, "KeyLint.exe") + if err := os.WriteFile(fakeBin, []byte("old binary"), 0o755); err != nil { + t.Fatal(err) + } + // Make the directory read-only so selfupdate cannot create .KeyLint.exe.new + if err := os.Chmod(dir, 0o555); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(dir, 0o755) }) + + err := selfupdate.Apply(strings.NewReader("new binary"), selfupdate.Options{ + TargetPath: fakeBin, + }) + + if err == nil { + t.Fatal("expected permission error when applying update to read-only directory, got nil — " + + "this means the old selfupdate.Apply() approach would silently corrupt the install") + } + + // The error message varies by OS: "permission denied" (Linux) or "Access is denied" (Windows). + errMsg := strings.ToLower(err.Error()) + if !strings.Contains(errMsg, "permission denied") && !strings.Contains(errMsg, "access is denied") { + t.Errorf("expected permission-related error, got: %v", err) + } +} From a1331d540828f68a1bf2b890045ca5d8427efe31 Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:21:46 +0000 Subject: [PATCH 4/8] =?UTF-8?q?fix(updater):=20platform-aware=20install=20?= =?UTF-8?q?=E2=80=94=20launch=20NSIS=20on=20Windows,=20selfupdate=20on=20L?= =?UTF-8?q?inux?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #18. The old approach used selfupdate.Apply() which fails on Windows because (a) the release asset is an NSIS installer, not a raw binary, and (b) Program Files requires UAC elevation for writes. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/features/updater/install_linux.go | 30 ++++ internal/features/updater/install_test.go | 152 +++++++++++++++++++ internal/features/updater/install_windows.go | 32 ++++ internal/features/updater/model.go | 6 + internal/features/updater/service.go | 66 +++++--- 5 files changed, 265 insertions(+), 21 deletions(-) create mode 100644 internal/features/updater/install_linux.go create mode 100644 internal/features/updater/install_windows.go diff --git a/internal/features/updater/install_linux.go b/internal/features/updater/install_linux.go new file mode 100644 index 0000000..797d194 --- /dev/null +++ b/internal/features/updater/install_linux.go @@ -0,0 +1,30 @@ +//go:build !windows + +package updater + +import ( + "fmt" + "os" + + "github.com/minio/selfupdate" +) + +// applyPlatformUpdate applies the downloaded binary in-place using selfupdate. +// On Linux the release asset is a raw executable, so direct replacement works +// as long as the target directory is writable. +func applyPlatformUpdate(svc *Service, tmpPath string) (InstallResult, error) { + f, err := os.Open(tmpPath) + if err != nil { + return InstallResult{}, fmt.Errorf("opening downloaded update: %w", err) + } + defer f.Close() + defer os.Remove(tmpPath) + + if err := selfupdate.Apply(f, selfupdate.Options{}); err != nil { + return InstallResult{}, fmt.Errorf("applying update: %w — if this is a permission error, "+ + "the downloaded file is at %s — install manually or run with appropriate permissions", + err, tmpPath) + } + + return InstallResult{RestartRequired: false}, nil +} diff --git a/internal/features/updater/install_test.go b/internal/features/updater/install_test.go index 4979b43..ac1ebdd 100644 --- a/internal/features/updater/install_test.go +++ b/internal/features/updater/install_test.go @@ -1,6 +1,9 @@ package updater import ( + "bytes" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" @@ -40,3 +43,152 @@ func TestSelfupdateFailsOnReadOnlyDirectory(t *testing.T) { t.Errorf("expected permission-related error, got: %v", err) } } + +// serveAsset creates an httptest server that serves the given payload for any request. +func serveAsset(payload []byte) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(payload) + })) +} + +// makeReleaseWithURL creates a release whose asset URL points to a custom server. +func makeReleaseWithURL(tag string, assetName string, assetURL string) githubRelease { + return githubRelease{ + TagName: tag, + Name: tag, + Body: "Release " + tag, + Draft: false, + Prerelease: false, + Assets: []githubAsset{{ + Name: assetName, + BrowserDownloadURL: assetURL, + }}, + } +} + +func TestDownloadAndInstall_DownloadsToTempAndCallsApply(t *testing.T) { + payload := []byte("fake-installer-payload-12345") + assetSrv := serveAsset(payload) + defer assetSrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", assetSrv.URL+"/dl"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + + // Override the platform apply function to capture and verify the temp file + // instead of calling selfupdate.Apply (which would corrupt the test binary). + var capturedPath string + var capturedContent []byte + svc.applyFunc = func(_ *Service, tmpPath string) (InstallResult, error) { + capturedPath = tmpPath + data, err := os.ReadFile(tmpPath) + if err != nil { + t.Fatalf("applyFunc: failed to read temp file: %v", err) + } + capturedContent = data + return InstallResult{RestartRequired: false}, nil + } + + result, err := svc.DownloadAndInstall() + if err != nil { + t.Fatalf("DownloadAndInstall() error: %v", err) + } + if result.RestartRequired { + t.Error("expected RestartRequired=false from test applyFunc") + } + if capturedPath == "" { + t.Fatal("applyFunc was never called — download phase may have failed") + } + if !bytes.Equal(capturedContent, payload) { + t.Errorf("temp file content = %q, want %q", capturedContent, payload) + } +} + +func TestDownloadAndInstall_ApplyFuncRestartRequired(t *testing.T) { + assetSrv := serveAsset([]byte("installer")) + defer assetSrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", assetSrv.URL+"/dl"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + svc.applyFunc = func(_ *Service, _ string) (InstallResult, error) { + return InstallResult{RestartRequired: true}, nil + } + + result, err := svc.DownloadAndInstall() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !result.RestartRequired { + t.Error("expected RestartRequired=true (simulating Windows path)") + } +} + +func TestDownloadAndInstall_NoUpdateAvailable(t *testing.T) { + releases := []githubRelease{ + makeRelease("v1.0.0", false, false, "KeyLint-v1.0.0-linux-amd64"), + } + srv := serveGitHubReleases(t, releases) + defer srv.Close() + + svc := newTestService("1.0.0", srv) + _, err := svc.DownloadAndInstall() + if err == nil { + t.Fatal("expected error for no-update-available") + } + if !strings.Contains(err.Error(), "no update available") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestDownloadAndInstall_DownloadHTTPError(t *testing.T) { + errorSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "not found", http.StatusNotFound) + })) + defer errorSrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", errorSrv.URL+"/missing"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + _, err := svc.DownloadAndInstall() + if err == nil { + t.Fatal("expected error for HTTP 404 download") + } + if !strings.Contains(err.Error(), "404") { + t.Errorf("expected status code in error, got: %v", err) + } +} + +func TestDownloadAndInstall_EmptyBody(t *testing.T) { + emptySrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // 200 OK but empty body + })) + defer emptySrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", emptySrv.URL+"/empty"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + _, err := svc.DownloadAndInstall() + if err == nil { + t.Fatal("expected error for empty download body") + } + if !strings.Contains(err.Error(), "empty") { + t.Errorf("expected 'empty' in error, got: %v", err) + } +} diff --git a/internal/features/updater/install_windows.go b/internal/features/updater/install_windows.go new file mode 100644 index 0000000..4503c3c --- /dev/null +++ b/internal/features/updater/install_windows.go @@ -0,0 +1,32 @@ +//go:build windows + +package updater + +import ( + "fmt" + "os" + "os/exec" +) + +// applyPlatformUpdate launches the downloaded NSIS installer and signals the app to quit. +// The NSIS installer's own manifest requests UAC elevation, so no special privileges +// are needed here — Windows will show the UAC prompt to the user. +func applyPlatformUpdate(svc *Service, tmpPath string) (InstallResult, error) { + cmd := exec.Command(tmpPath) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + if err := cmd.Start(); err != nil { + os.Remove(tmpPath) + return InstallResult{}, fmt.Errorf("launching installer: %w", err) + } + + // Schedule app quit so the installer can replace the locked executable. + // The quitFunc is set by main.go and includes a short delay for the + // frontend to display the "closing" message before the app exits. + if svc.quitFunc != nil { + go svc.quitFunc() + } + + return InstallResult{RestartRequired: true}, nil +} diff --git a/internal/features/updater/model.go b/internal/features/updater/model.go index 24757bc..9ef8ec0 100644 --- a/internal/features/updater/model.go +++ b/internal/features/updater/model.go @@ -24,6 +24,12 @@ type PlatformAsset struct { Signature string `json:"signature"` } +// InstallResult is returned by DownloadAndInstall to indicate the outcome. +// On Windows, RestartRequired is true because the NSIS installer needs the app to exit. +type InstallResult struct { + RestartRequired bool `json:"restart_required"` +} + // githubRelease represents a single release from the GitHub Releases API. type githubRelease struct { TagName string `json:"tag_name"` diff --git a/internal/features/updater/service.go b/internal/features/updater/service.go index b7c637d..5fa344d 100644 --- a/internal/features/updater/service.go +++ b/internal/features/updater/service.go @@ -2,7 +2,9 @@ package updater import ( "encoding/json" + "errors" "fmt" + "io" "net/http" "os" "runtime" @@ -10,18 +12,18 @@ import ( "strings" "keylint/internal/features/settings" - - "github.com/minio/selfupdate" ) const defaultReleasesAPIURL = "https://api.github.com/repos/0xMMA/KeyLint/releases?per_page=20" -// Service checks for updates and can apply them in-place using minio/selfupdate. +// Service checks for updates and can apply them using platform-specific strategies. type Service struct { - currentVersion string - releasesAPIURL string - client *http.Client - settingsSvc *settings.Service + currentVersion string + releasesAPIURL string + client *http.Client + settingsSvc *settings.Service + quitFunc func() // called after launching installer on Windows; set via SetQuitFunc + applyFunc func(svc *Service, tmpPath string) (InstallResult, error) // override for testing; nil uses applyPlatformUpdate } // NewService creates an updater Service with the given current version string. @@ -40,6 +42,12 @@ func (s *Service) GetVersion() string { return s.currentVersion } +// SetQuitFunc sets the callback invoked after launching the installer on Windows. +// The callback should wait briefly (for the frontend to display a message) then quit the app. +func (s *Service) SetQuitFunc(fn func()) { + s.quitFunc = fn +} + // CheckForUpdate fetches the GitHub Releases API and finds the best available update. func (s *Service) CheckForUpdate() (UpdateInfo, error) { info := UpdateInfo{CurrentVersion: s.currentVersion} @@ -141,40 +149,56 @@ func matchPlatformAsset(assets []githubAsset) string { return "" } -// DownloadAndInstall fetches the binary for the current platform and replaces the running exe. -func (s *Service) DownloadAndInstall() error { +// DownloadAndInstall fetches the release asset for the current platform, saves it +// to a temp file, and delegates to the platform-specific installer. +// On Windows this launches the NSIS setup and returns InstallResult{RestartRequired: true}. +// On Linux this applies the binary in-place via selfupdate. +func (s *Service) DownloadAndInstall() (InstallResult, error) { updateInfo, err := s.CheckForUpdate() if err != nil { - return fmt.Errorf("checking for update: %w", err) + return InstallResult{}, fmt.Errorf("checking for update: %w", err) } if !updateInfo.IsAvailable { - return fmt.Errorf("no update available") + return InstallResult{}, fmt.Errorf("no update available") } if updateInfo.ReleaseURL == "" { - return fmt.Errorf("no download URL for current platform") + return InstallResult{}, fmt.Errorf("no download URL for current platform") } resp, err := s.client.Get(updateInfo.ReleaseURL) if err != nil { - return fmt.Errorf("downloading update: %w", err) + return InstallResult{}, fmt.Errorf("downloading update: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return fmt.Errorf("download returned status %d", resp.StatusCode) + return InstallResult{}, fmt.Errorf("download returned status %d", resp.StatusCode) } - if err := selfupdate.Apply(resp.Body, selfupdate.Options{}); err != nil { - return fmt.Errorf("applying update: %w", err) + // Write to a temp file so platform-specific code can work with a path on disk. + tmpFile, err := os.CreateTemp("", "KeyLint-update-*.exe") + if err != nil { + return InstallResult{}, fmt.Errorf("creating temp file: %w", err) } - // Restart the process so the new binary takes effect immediately. - executable, err := os.Executable() + n, err := io.Copy(tmpFile, resp.Body) if err != nil { - return fmt.Errorf("finding executable path: %w", err) + tmpFile.Close() + os.Remove(tmpFile.Name()) + return InstallResult{}, fmt.Errorf("writing update to temp file: %w", err) + } + tmpFile.Close() + + if n == 0 { + os.Remove(tmpFile.Name()) + return InstallResult{}, errors.New("downloaded update is empty") + } + + applyFn := applyPlatformUpdate + if s.applyFunc != nil { + applyFn = s.applyFunc } - _ = executable // exec.Command would be used here for a restart; omitted for simplicity. - return nil + return applyFn(s, tmpFile.Name()) } // parsedVersion holds the decomposed parts of a semver string with optional pre-release suffix. From 37e0794d798613de35bf2edd7e75d0050ece8e3f Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:23:31 +0000 Subject: [PATCH 5/8] fix(updater): wire quitFunc in main.go, regenerate bindings Co-Authored-By: Claude Opus 4.6 (1M context) --- .../internal/features/logger/service.js | 2 ++ .../internal/features/settings/models.js | 2 +- .../internal/features/updater/index.js | 1 + .../internal/features/updater/models.js | 32 +++++++++++++++++++ .../internal/features/updater/service.js | 24 +++++++++++--- main.go | 9 +++++- 6 files changed, 64 insertions(+), 6 deletions(-) diff --git a/frontend/bindings/keylint/internal/features/logger/service.js b/frontend/bindings/keylint/internal/features/logger/service.js index 4a07df1..3ced463 100644 --- a/frontend/bindings/keylint/internal/features/logger/service.js +++ b/frontend/bindings/keylint/internal/features/logger/service.js @@ -13,6 +13,8 @@ import { Call as $Call, CancellablePromise as $CancellablePromise, Create as $Cr /** * Log writes a frontend message at the given level into debug.log. + * The msg is wrapped in Redact() because frontend messages may contain user text. + * Error and warn levels log the msg directly (operational). * @param {string} level * @param {string} msg * @returns {$CancellablePromise} diff --git a/frontend/bindings/keylint/internal/features/settings/models.js b/frontend/bindings/keylint/internal/features/settings/models.js index 9ab2949..1fea0c2 100644 --- a/frontend/bindings/keylint/internal/features/settings/models.js +++ b/frontend/bindings/keylint/internal/features/settings/models.js @@ -179,7 +179,7 @@ export class Settings { } if (!("log_level" in $$source)) { /** - * "off" | "trace" | "debug" | "info" | "warning" | "error" + * "off"|"trace"|"debug"|"info"|"warning"|"error" * @member * @type {string} */ diff --git a/frontend/bindings/keylint/internal/features/updater/index.js b/frontend/bindings/keylint/internal/features/updater/index.js index e5c5c4d..54b3643 100644 --- a/frontend/bindings/keylint/internal/features/updater/index.js +++ b/frontend/bindings/keylint/internal/features/updater/index.js @@ -8,5 +8,6 @@ export { }; export { + InstallResult, UpdateInfo } from "./models.js"; diff --git a/frontend/bindings/keylint/internal/features/updater/models.js b/frontend/bindings/keylint/internal/features/updater/models.js index dafb4e7..675f1ba 100644 --- a/frontend/bindings/keylint/internal/features/updater/models.js +++ b/frontend/bindings/keylint/internal/features/updater/models.js @@ -6,6 +6,38 @@ // @ts-ignore: Unused imports import { Create as $Create } from "@wailsio/runtime"; +/** + * InstallResult is returned by DownloadAndInstall to indicate the outcome. + * On Windows, RestartRequired is true because the NSIS installer needs the app to exit. + */ +export class InstallResult { + /** + * Creates a new InstallResult instance. + * @param {Partial} [$$source = {}] - The source object to create the InstallResult. + */ + constructor($$source = {}) { + if (!("restart_required" in $$source)) { + /** + * @member + * @type {boolean} + */ + this["restart_required"] = false; + } + + Object.assign(this, $$source); + } + + /** + * Creates a new InstallResult instance from a string or object. + * @param {any} [$$source = {}] + * @returns {InstallResult} + */ + static createFrom($$source = {}) { + let $$parsedSource = typeof $$source === 'string' ? JSON.parse($$source) : $$source; + return new InstallResult(/** @type {Partial} */($$parsedSource)); + } +} + /** * UpdateInfo is returned to the frontend to describe whether an update is available. */ diff --git a/frontend/bindings/keylint/internal/features/updater/service.js b/frontend/bindings/keylint/internal/features/updater/service.js index 4a69e23..1fc46a5 100644 --- a/frontend/bindings/keylint/internal/features/updater/service.js +++ b/frontend/bindings/keylint/internal/features/updater/service.js @@ -3,7 +3,7 @@ // This file is automatically generated. DO NOT EDIT /** - * Service checks for updates and can apply them in-place using minio/selfupdate. + * Service checks for updates and can apply them using platform-specific strategies. * @module */ @@ -26,11 +26,16 @@ export function CheckForUpdate() { } /** - * DownloadAndInstall fetches the binary for the current platform and replaces the running exe. - * @returns {$CancellablePromise} + * DownloadAndInstall fetches the release asset for the current platform, saves it + * to a temp file, and delegates to the platform-specific installer. + * On Windows this launches the NSIS setup and returns InstallResult{RestartRequired: true}. + * On Linux this applies the binary in-place via selfupdate. + * @returns {$CancellablePromise<$models.InstallResult>} */ export function DownloadAndInstall() { - return $Call.ByID(1125161478); + return $Call.ByID(1125161478).then(/** @type {($result: any) => any} */(($result) => { + return $$createType1($result); + })); } /** @@ -41,5 +46,16 @@ export function GetVersion() { return $Call.ByID(4045591078); } +/** + * SetQuitFunc sets the callback invoked after launching the installer on Windows. + * The callback should wait briefly (for the frontend to display a message) then quit the app. + * @param {any} fn + * @returns {$CancellablePromise} + */ +export function SetQuitFunc(fn) { + return $Call.ByID(1352960079, fn); +} + // Private type creation functions const $$createType0 = $models.UpdateInfo.createFrom; +const $$createType1 = $models.InstallResult.createFrom; diff --git a/main.go b/main.go index f0945bf..33f9e50 100644 --- a/main.go +++ b/main.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "os" + "time" "keylint/internal/app" "keylint/internal/cli" @@ -86,7 +87,13 @@ func main() { wailsApp.RegisterService(application.NewService(featurelogger.NewService())) // Updater service — AppVersion injected at build time via ldflags. - wailsApp.RegisterService(application.NewService(updater.NewService(AppVersion, services.Settings))) + updaterSvc := updater.NewService(AppVersion, services.Settings) + updaterSvc.SetQuitFunc(func() { + // Brief delay so the frontend can display the "closing" message. + time.Sleep(2 * time.Second) + wailsApp.Quit() + }) + wailsApp.RegisterService(application.NewService(updaterSvc)) // Dev-tools shortcut simulation service. sim := &simulateService{shortcut: services.Shortcut} From 89312c0fa29bfe3e61330246d67a1ed87f35237b Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:25:59 +0000 Subject: [PATCH 6/8] feat(updater): frontend handles restart_required from updater Co-Authored-By: Claude Opus 4.6 (1M context) --- frontend/src/app/core/wails.service.ts | 6 +++--- .../settings/settings.component.spec.ts | 18 +++++++++++++++++- .../features/settings/settings.component.ts | 9 +++++++-- frontend/src/testing/wails-mock.ts | 4 ++-- 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/core/wails.service.ts b/frontend/src/app/core/wails.service.ts index f9ba151..f2de1ed 100644 --- a/frontend/src/app/core/wails.service.ts +++ b/frontend/src/app/core/wails.service.ts @@ -12,10 +12,10 @@ import * as UpdaterService from '../../../bindings/keylint/internal/features/upd import * as LoggerService from '../../../bindings/keylint/internal/features/logger/service.js'; import * as PyramidizeService from '../../../bindings/keylint/internal/features/pyramidize/service.js'; import { Settings, KeyStatus } from '../../../bindings/keylint/internal/features/settings/models.js'; -import { UpdateInfo } from '../../../bindings/keylint/internal/features/updater/models.js'; +import { UpdateInfo, InstallResult } from '../../../bindings/keylint/internal/features/updater/models.js'; import type { PyramidizeRequest, PyramidizeResult, RefineGlobalRequest, RefineGlobalResult, SpliceRequest, SpliceResult, AppPreset } from '../../../bindings/keylint/internal/features/pyramidize/models.js'; -export type { Settings, KeyStatus, UpdateInfo }; +export type { Settings, KeyStatus, UpdateInfo, InstallResult }; export type { PyramidizeRequest, PyramidizeResult, RefineGlobalRequest, RefineGlobalResult, SpliceRequest, SpliceResult, AppPreset }; @@ -160,7 +160,7 @@ export class WailsService implements OnDestroy { return UpdaterService.CheckForUpdate(); } - downloadAndInstall(): Promise { + downloadAndInstall(): Promise { return UpdaterService.DownloadAndInstall(); } diff --git a/frontend/src/app/features/settings/settings.component.spec.ts b/frontend/src/app/features/settings/settings.component.spec.ts index ac8e2ed..79a4e12 100644 --- a/frontend/src/app/features/settings/settings.component.spec.ts +++ b/frontend/src/app/features/settings/settings.component.spec.ts @@ -241,11 +241,27 @@ describe('SettingsComponent', () => { }); it('installUpdate() sets updateSuccess on success', async () => { - wailsMock.downloadAndInstall.mockResolvedValue(undefined); + wailsMock.downloadAndInstall.mockResolvedValue({ restart_required: false }); component.updateInfo = { ...defaultUpdateInfo, is_available: true, latest_version: '3.7.0' }; await component.installUpdate(); expect(component.updateSuccess).toBe(true); expect(wailsMock.downloadAndInstall).toHaveBeenCalled(); }); + + it('installUpdate() shows restart message when restart_required is true', async () => { + wailsMock.downloadAndInstall.mockResolvedValue({ restart_required: true }); + component.updateInfo = { ...defaultUpdateInfo, is_available: true, latest_version: '3.7.0' }; + await component.installUpdate(); + expect(component.updateSuccess).toBe(true); + expect(component.updateRestartRequired).toBe(true); + }); + + it('installUpdate() shows standard success when restart_required is false', async () => { + wailsMock.downloadAndInstall.mockResolvedValue({ restart_required: false }); + component.updateInfo = { ...defaultUpdateInfo, is_available: true, latest_version: '3.7.0' }; + await component.installUpdate(); + expect(component.updateSuccess).toBe(true); + expect(component.updateRestartRequired).toBe(false); + }); }); }); diff --git a/frontend/src/app/features/settings/settings.component.ts b/frontend/src/app/features/settings/settings.component.ts index 2f523a9..8b004e9 100644 --- a/frontend/src/app/features/settings/settings.component.ts +++ b/frontend/src/app/features/settings/settings.component.ts @@ -263,7 +263,9 @@ interface ProviderKey { } @@ -383,6 +385,7 @@ export class SettingsComponent implements OnInit { updateInstalling = false; updateError = ''; updateSuccess = false; + updateRestartRequired = false; // App Defaults tab state presets: AppPreset[] = []; @@ -525,9 +528,11 @@ export class SettingsComponent implements OnInit { this.updateInstalling = true; this.updateError = ''; this.updateSuccess = false; + this.updateRestartRequired = false; try { - await this.wails.downloadAndInstall(); + const result = await this.wails.downloadAndInstall(); this.updateSuccess = true; + this.updateRestartRequired = result.restart_required; this.updateInfo = null; } catch (e) { this.updateError = `Install failed: ${e instanceof Error ? e.message : String(e)}`; diff --git a/frontend/src/testing/wails-mock.ts b/frontend/src/testing/wails-mock.ts index 719f5db..d2403e1 100644 --- a/frontend/src/testing/wails-mock.ts +++ b/frontend/src/testing/wails-mock.ts @@ -1,6 +1,6 @@ import { Subject } from 'rxjs'; import { vi } from 'vitest'; -import type { Settings, KeyStatus, UpdateInfo } from '../app/core/wails.service'; +import type { Settings, KeyStatus, UpdateInfo, InstallResult } from '../app/core/wails.service'; export const defaultSettings: Settings = { active_provider: 'openai', @@ -56,7 +56,7 @@ export function createWailsMock() { resetSettings: vi.fn().mockResolvedValue(undefined), getVersion: vi.fn().mockResolvedValue('3.6.0'), checkForUpdate: vi.fn().mockResolvedValue({ ...defaultUpdateInfo }), - downloadAndInstall: vi.fn().mockResolvedValue(undefined), + downloadAndInstall: vi.fn().mockResolvedValue({ restart_required: false }), log: vi.fn().mockResolvedValue(undefined), pasteToForeground: vi.fn().mockResolvedValue(undefined), ngOnDestroy: vi.fn(), From a22c84317684e41c05112fac1b356b7e787b79b8 Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:29:45 +0000 Subject: [PATCH 7/8] fix(updater): preserve temp file on Linux install failure, add error propagation test The defer os.Remove was deleting the temp file before the user could act on the "install manually" error message. Now only removed on success. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/features/updater/install_linux.go | 2 +- internal/features/updater/install_test.go | 25 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/internal/features/updater/install_linux.go b/internal/features/updater/install_linux.go index 797d194..5084b6e 100644 --- a/internal/features/updater/install_linux.go +++ b/internal/features/updater/install_linux.go @@ -18,7 +18,6 @@ func applyPlatformUpdate(svc *Service, tmpPath string) (InstallResult, error) { return InstallResult{}, fmt.Errorf("opening downloaded update: %w", err) } defer f.Close() - defer os.Remove(tmpPath) if err := selfupdate.Apply(f, selfupdate.Options{}); err != nil { return InstallResult{}, fmt.Errorf("applying update: %w — if this is a permission error, "+ @@ -26,5 +25,6 @@ func applyPlatformUpdate(svc *Service, tmpPath string) (InstallResult, error) { err, tmpPath) } + os.Remove(tmpPath) return InstallResult{RestartRequired: false}, nil } diff --git a/internal/features/updater/install_test.go b/internal/features/updater/install_test.go index ac1ebdd..206e49b 100644 --- a/internal/features/updater/install_test.go +++ b/internal/features/updater/install_test.go @@ -2,6 +2,7 @@ package updater import ( "bytes" + "fmt" "net/http" "net/http/httptest" "os" @@ -192,3 +193,27 @@ func TestDownloadAndInstall_EmptyBody(t *testing.T) { t.Errorf("expected 'empty' in error, got: %v", err) } } + +func TestDownloadAndInstall_ApplyFuncError(t *testing.T) { + assetSrv := serveAsset([]byte("binary")) + defer assetSrv.Close() + + releases := []githubRelease{ + makeReleaseWithURL("v9.0.0", "KeyLint-v9.0.0-linux-amd64", assetSrv.URL+"/dl"), + } + releaseSrv := serveGitHubReleases(t, releases) + defer releaseSrv.Close() + + svc := newTestService("1.0.0", releaseSrv) + svc.applyFunc = func(_ *Service, _ string) (InstallResult, error) { + return InstallResult{}, fmt.Errorf("simulated install failure") + } + + _, err := svc.DownloadAndInstall() + if err == nil { + t.Fatal("expected error from applyFunc") + } + if !strings.Contains(err.Error(), "simulated install failure") { + t.Errorf("expected applyFunc error to propagate, got: %v", err) + } +} From 8ab605345d53f754fe4a9660f5df82d82cb0ea3f Mon Sep 17 00:00:00 2001 From: Michael <0xMMA@users.noreply.github.com> Date: Sun, 5 Apr 2026 22:38:00 +0000 Subject: [PATCH 8/8] ci: add Windows build + NSIS installer to PR workflow Both Linux and Windows builds now run on every PR, with 3-day artifact retention. Tests run first in a separate job; builds only start after tests pass. Windows artifacts (binary + NSIS setup) are downloadable from the Actions tab for local testing. Co-Authored-By: Claude Opus 4.6 (1M context) --- .claude/commands/ship.md | 4 +- .claude/docs/versioning.md | 5 +- .github/workflows/build-linux.yml | 132 +++++++++++++++++++++++++++++- 3 files changed, 135 insertions(+), 6 deletions(-) diff --git a/.claude/commands/ship.md b/.claude/commands/ship.md index 7fa1591..bf9530d 100644 --- a/.claude/commands/ship.md +++ b/.claude/commands/ship.md @@ -6,7 +6,7 @@ Perform a complete git workflow to ship the current changes in this repo. - **Branch convention:** `feat/`, `fix/`, `chore/` - **Target branch:** `main` -- **CI:** `build-linux.yml` runs automatically on PR open (tests + compile) +- **CI:** `build-linux.yml (Build CI)` runs automatically on PR open (tests + compile) - **Release:** done separately via `git tag` — do NOT tag here ## Steps @@ -43,7 +43,7 @@ Perform a complete git workflow to ship the current changes in this repo. - Otherwise: `gh pr create` with: - Title: same as commit subject - Body: what changed, why, and any manual verification steps - - Include note that `build-linux.yml` CI will run automatically + - Include note that `build-linux.yml (Build CI)` CI will run automatically - If $ARGUMENTS is provided, use it as extra context for the PR body ## Notes diff --git a/.claude/docs/versioning.md b/.claude/docs/versioning.md index 955a4fd..acf0337 100644 --- a/.claude/docs/versioning.md +++ b/.claude/docs/versioning.md @@ -14,7 +14,7 @@ Suffix is optional; use `-alpha` / `-beta` / `-rc` for pre-releases. ``` feat/ branch → PR opened - └─ build-linux.yml runs automatically (tests + compile check) + └─ build-linux.yml runs automatically (tests + Linux/Windows builds) → PR merged to main → git tag vX.Y.Z[-suffix] ← release trigger → git push origin vX.Y.Z[-suffix] @@ -41,7 +41,8 @@ Then go to **GitHub → Releases** and publish the draft once the workflow compl **`build-linux.yml`** (PR + push to main): - Runs frontend tests (Vitest) + Go tests -- Builds Linux binary (artifact only, not released) +- Builds Linux binary + Windows binary + NSIS installer (artifacts only, not released) +- PR artifacts auto-expire after 3 days **`release.yml`** (tag push): - Builds `KeyLint-vX.Y.Z-linux-amd64` and `KeyLint-vX.Y.Z-windows-amd64.exe` diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 0d2dc4b..9f24e0c 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -1,4 +1,4 @@ -name: Build Linux +name: Build on: pull_request: @@ -6,7 +6,7 @@ on: branches: [main] jobs: - build: + test: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -50,6 +50,44 @@ jobs: - name: Run Go tests run: go test ./internal/... + build-linux: + needs: test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.26' + cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '24' + cache: 'npm' + cache-dependency-path: frontend/package-lock.json + + - name: Install system deps + uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: pkg-config libgtk-3-dev libwebkit2gtk-4.1-dev gcc + version: 1.0 + + - name: Install Wails v3 CLI + run: go install github.com/wailsapp/wails/v3/cmd/wails3@v3.0.0-alpha.72 + + - name: Install frontend dependencies + working-directory: frontend + run: npm ci + + - name: Build frontend + working-directory: frontend + run: npm run build + - name: Build Linux binary run: | export PATH=$PATH:$HOME/go/bin @@ -61,3 +99,93 @@ jobs: with: name: KeyLint-linux-amd64 path: bin/KeyLint + retention-days: 3 + + build-windows: + needs: test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.26' + cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '24' + cache: 'npm' + cache-dependency-path: frontend/package-lock.json + + - name: Install system deps + uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: pkg-config libgtk-3-dev libwebkit2gtk-4.1-dev gcc gcc-mingw-w64 + version: 1.0 + + - name: Install Wails v3 CLI + run: go install github.com/wailsapp/wails/v3/cmd/wails3@v3.0.0-alpha.72 + + - name: Install frontend dependencies + working-directory: frontend + run: npm ci + + - name: Build frontend + working-directory: frontend + run: npm run build + + - name: Generate Windows resource file + run: | + export PATH=$PATH:$HOME/go/bin + wails3 generate syso \ + -arch amd64 \ + -icon build/windows/icon.ico \ + -manifest build/windows/wails.exe.manifest \ + -info build/windows/info.json \ + -out wails_windows_amd64.syso + + - name: Build Windows binary + run: | + export PATH=$PATH:$HOME/go/bin + VERSION=$(git describe --tags --always --dirty 2>/dev/null || echo "dev") + GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc \ + go build -tags production -trimpath \ + -ldflags="-w -s -H windowsgui -X main.AppVersion=${VERSION}" \ + -o bin/KeyLint.exe . + + - name: Install NSIS + uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: nsis + version: 1.0 + + - name: Download WebView2 bootstrapper + run: | + curl -sSL "https://go.microsoft.com/fwlink/p/?LinkId=2124703" \ + -o build/windows/nsis/MicrosoftEdgeWebview2Setup.exe + + - name: Build NSIS installer + run: | + VERSION=$(git describe --tags --always --dirty 2>/dev/null || echo "dev") + makensis -DARG_WAILS_AMD64_BINARY=../../../bin/KeyLint.exe \ + -DINFO_PRODUCTVERSION="${VERSION#v}" \ + build/windows/nsis/project.nsi + + - name: Upload Windows binary + uses: actions/upload-artifact@v4 + with: + name: KeyLint-windows-amd64 + path: bin/KeyLint.exe + retention-days: 3 + + - name: Upload Windows installer + uses: actions/upload-artifact@v4 + with: + name: KeyLint-windows-amd64-setup + path: bin/KeyLint-amd64-installer.exe + retention-days: 3