From 3bee6edc0cdecfcc07d4c3adee03a23c0ab94b4a Mon Sep 17 00:00:00 2001 From: dvcdsys Date: Mon, 10 Aug 2026 18:51:07 +0100 Subject: [PATCH] fix(mac): make Start say why it failed, and copy the password without a flash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems from testing the previous change. **Start did nothing after the database was deleted.** Removing the bootstrap password was right, but it turned a silent recreation into a silent refusal: with the email still present and the password gone, an empty database is the half-configured case bootstrap.go rejects outright ("CIX_BOOTSTRAP_ADMIN_EMAIL is set but CIX_BOOTSTRAP_ADMIN_PASSWORD is empty"). The server exited in milliseconds and the menu went back to "Stopped" — indistinguishable from a button that does nothing. needsFirstRun already knew, but only at launch; a running app never asked again. Start now checks before starting, and offers to set up again rather than spawning a process that cannot survive. It also verifies the server outlived the start: launchctl reports having spawned it, not having kept it, so a process that rejects its own configuration used to disappear without a word. When that happens the server's own message is shown, trimmed to the last few lines with a pointer to the log. Worth stating for the record: the refusal is guarded by `count == 0`, so retiring the password never affects a start against an intact database. **Copy Password flashed the window.** AppleScript's `display dialog` is modal and returns only when it closes, so a copy button meant closing the window and opening it again — on screen, a blink. The password is now on the clipboard before the window appears and the message says so, which removes the flash and the click together. There is nothing to be coy about: this is a password the app generated seconds ago and is showing on purpose. Co-Authored-By: Claude Opus 5 --- cli/launcher/dialog_darwin.go | 50 +++++++---------------- cli/launcher/firstrun_darwin.go | 4 +- cli/launcher/firstrun_darwin_test.go | 56 +++++++++++++++++++++++++ cli/launcher/launchd_darwin.go | 61 ++++++++++++++++++++++++++++ cli/launcher/menu_darwin.go | 58 ++++++++++++++++++++------ cli/launcher/resetpw_darwin.go | 4 +- 6 files changed, 182 insertions(+), 51 deletions(-) diff --git a/cli/launcher/dialog_darwin.go b/cli/launcher/dialog_darwin.go index ea680c5..6f9d682 100644 --- a/cli/launcher/dialog_darwin.go +++ b/cli/launcher/dialog_darwin.go @@ -65,43 +65,23 @@ func alert(title, message string) error { return runOsascript(2*time.Minute, script) } -// alertWithCopy is alert() plus a button that puts secret on the clipboard. +// alertWithSecret shows a credential and puts it on the clipboard. // -// AppleScript cannot make a run of text clickable — `display dialog` draws one -// static string — so the click target has to be a button. That is the whole -// reason this is not simply "click the password". -// -// It re-shows the dialog after copying rather than dismissing. A password -// displayed once is exactly the thing someone reaches for a second time, and a -// window that disappears on the first attempt is how people end up reading -// credentials off a screenshot. The loop always terminates: every turn of it -// waits for a click. -func alertWithCopy(title, message, secret, copyLabel string) error { - body := message - for { - script := fmt.Sprintf( - `display dialog %s with title %s %s buttons {%s, "Done"} default button "Done"`, - quoteAS(body), quoteAS(title), iconClause(), quoteAS(copyLabel), - ) - out, err := outputOsascript(5*time.Minute, script) - if err != nil { - // Dismissing the window is a perfectly good way to say "I have it". - if errors.Is(err, errCancelled) { - return nil - } - return err - } - if !strings.Contains(out, copyLabel) { - return nil - } - - if err := copyToClipboard(secret); err != nil { - logf("could not copy to the clipboard: %v", err) - body = message + "\n\nCould not copy to the clipboard." - continue - } - body = message + "\n\nCopied to the clipboard." +// The copying happens before the window opens, not on a button, and the message +// says so. A button was tried and was worse: AppleScript cannot make a run of +// text clickable — `display dialog` is modal and returns only when it closes — +// so "copy" meant closing the window and opening it again, which on screen is +// a flash. Copying up front removes the flash and the click at once, and there +// is nothing to be coy about: this is a password the app generated seconds ago +// and is showing on purpose, and reaching for the clipboard is the next thing +// anyone does with it. +func alertWithSecret(title, message, secret, secretName string) error { + note := fmt.Sprintf("\n\nThe %s is on your clipboard.", secretName) + if err := copyToClipboard(secret); err != nil { + logf("could not copy the %s to the clipboard: %v", secretName, err) + note = fmt.Sprintf("\n\nThe %s could not be copied to your clipboard — select it above.", secretName) } + return alert(title, message+note) } // copyToClipboard pipes a value to pbcopy. diff --git a/cli/launcher/firstrun_darwin.go b/cli/launcher/firstrun_darwin.go index c9f403a..dcb41e8 100644 --- a/cli/launcher/firstrun_darwin.go +++ b/cli/launcher/firstrun_darwin.go @@ -226,11 +226,11 @@ func runFirstRun(u *updater) error { retireBootstrapPassword() } - return alertWithCopy("cix is set up", fmt.Sprintf( + return alertWithSecret("cix is set up", fmt.Sprintf( "Sign in at %s\n\nEmail:\n%s\n\nTemporary password:\n%s\n\n"+ "You will be asked to change this password on first login.%s%s", dashboardURL(vars), email, password, waitNote, cliNote), - password, "Copy Password") + password, "password") } // registerWithCLI adds (or updates) the local server in ~/.cix/config.yaml. diff --git a/cli/launcher/firstrun_darwin_test.go b/cli/launcher/firstrun_darwin_test.go index f78753d..0caa5c9 100644 --- a/cli/launcher/firstrun_darwin_test.go +++ b/cli/launcher/firstrun_darwin_test.go @@ -1,8 +1,10 @@ package main import ( + "fmt" "os" "path/filepath" + "strings" "testing" ) @@ -158,3 +160,57 @@ func TestSetDefaultKeepsExistingChoices(t *testing.T) { t.Errorf("version check = %q, want the default filled in", vars["CIX_VERSION_CHECK_ENABLED"]) } } + +// When the server exits on a configuration it will not accept, its own words +// are the only useful thing to show — the menu otherwise says "Stopped" and +// nothing more, which is how a deleted database looked like a broken button. +func TestLastServerError(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + logs := filepath.Join(home, ".cix", "logs") + if err := os.MkdirAll(logs, 0o700); err != nil { + t.Fatal(err) + } + + t.Run("reports the tail", func(t *testing.T) { + body := "cix-server is ready\n\n\nbootstrap auth: incomplete bootstrap configuration:\n" + + " CIX_BOOTSTRAP_ADMIN_EMAIL is set but CIX_BOOTSTRAP_ADMIN_PASSWORD is empty.\n" + if err := os.WriteFile(filepath.Join(logs, "cix-server.err"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + got := lastServerError() + if !strings.Contains(got, "incomplete bootstrap configuration") { + t.Errorf("lastServerError() = %q, want the refusal in it", got) + } + // Blank lines carry nothing into a dialog. + if strings.Contains(got, "\n\n") { + t.Errorf("lastServerError() kept blank lines: %q", got) + } + }) + + t.Run("bounded", func(t *testing.T) { + var sb strings.Builder + for i := range 500 { + fmt.Fprintf(&sb, "line %d with a good deal of text after it so the cap is reached\n", i) + } + if err := os.WriteFile(filepath.Join(logs, "cix-server.err"), []byte(sb.String()), 0o644); err != nil { + t.Fatal(err) + } + got := lastServerError() + if n := len([]rune(got)); n > 901 { + t.Errorf("lastServerError() returned %d runes, want it capped", n) + } + if !strings.Contains(got, "line 499") { + t.Error("lastServerError() dropped the most recent line") + } + }) + + t.Run("no log at all", func(t *testing.T) { + if err := os.Remove(filepath.Join(logs, "cix-server.err")); err != nil { + t.Fatal(err) + } + if got := lastServerError(); got == "" { + t.Error("lastServerError() = \"\", want something to show the user") + } + }) +} diff --git a/cli/launcher/launchd_darwin.go b/cli/launcher/launchd_darwin.go index 7d92be5..ec79e06 100644 --- a/cli/launcher/launchd_darwin.go +++ b/cli/launcher/launchd_darwin.go @@ -331,3 +331,64 @@ func foreignAgent() bool { } return !strings.Contains(string(data), managedByMarker) } + +// serverDiedOnStart reports why the server is already gone after a Start we +// asked for, or "" when it is still alive. +// +// launchctl answers for having spawned the process, not for the process +// surviving it. A server that rejects its own configuration exits in +// milliseconds and leaves the menu reading "Stopped" with no explanation — +// indistinguishable from a Start button that does nothing. +// +// The wait is two-sided on purpose: a pid takes a moment to appear, and a +// process that is going to die does it almost at once. Neither half is a +// deadline for anything the user waits on — a healthy server returns from here +// as soon as it has a pid. +func serverDiedOnStart() string { + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) && launchdPID() == 0 { + time.Sleep(200 * time.Millisecond) + } + time.Sleep(2 * time.Second) + if launchdPID() != 0 { + return "" + } + return lastServerError() +} + +// lastServerError pulls the tail of cix-server.err, for a dialog. +// +// The server writes its refusals there as plain prose across several lines, so +// the last few non-empty ones are the message. Capped: an unbounded log tail in +// a modal dialog is its own failure. +func lastServerError() string { + dir, err := logDir() + if err != nil { + return "The server exited immediately." + } + data, err := os.ReadFile(filepath.Join(dir, "cix-server.err")) + if err != nil { + return "The server exited immediately." + } + + var lines []string + for line := range strings.SplitSeq(string(data), "\n") { + if strings.TrimSpace(line) != "" { + lines = append(lines, strings.TrimRight(line, " \t")) + } + } + if len(lines) == 0 { + return "The server exited immediately, without logging anything." + } + const keep = 8 + if len(lines) > keep { + lines = lines[len(lines)-keep:] + } + + out := strings.Join(lines, "\n") + const maxRunes = 900 + if r := []rune(out); len(r) > maxRunes { + out = "…" + string(r[len(r)-maxRunes:]) + } + return out +} diff --git a/cli/launcher/menu_darwin.go b/cli/launcher/menu_darwin.go index 76e92ee..cb3deed 100644 --- a/cli/launcher/menu_darwin.go +++ b/cli/launcher/menu_darwin.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "os" "os/exec" @@ -446,22 +447,55 @@ func (m *menu) toggleServer() { } defer m.endBusy() - var err error if s.State == stateRunning { - err = stopServer() - } else { - // Rewrite the wrapper before starting. It is generated, not edited, and - // something else may have replaced it — install-server.sh most obviously. - if err = writeLaunchdFiles(autostartEnabled()); err == nil { - err = startServer() + if err := stopServer(); err != nil { + _ = alert("cix", fmt.Sprintf("Could not stop the server.\n\n%v", err)) } + m.poll.refresh() + return } - if err != nil { - verb := "start" - if s.State == stateRunning { - verb = "stop" + + // A missing database is not something Start can fix. The server refuses to + // boot without an admin account to create — correctly — and says so in a + // log nobody has open, so the button appears to do nothing at all. Offer + // the thing that would actually help. + if needsFirstRun() { + ok, err := confirm("Set cix up again?", + "There is no cix database. If you deleted it, the server cannot start until an "+ + "administrator account is created again.\n\n"+ + "Setting up again creates a new account and a new, empty index. Anything that "+ + "was indexed before is already gone with the database.", + "Set Up") + if err != nil || !ok { + return } - _ = alert("cix", fmt.Sprintf("Could not %s the server.\n\n%v", verb, err)) + if err := runFirstRun(m.updater); err != nil && !errors.Is(err, errCancelled) { + logf("re-running setup failed: %v", err) + _ = alert("Setup failed", fmt.Sprintf("cix could not set itself up again.\n\n%v", err)) + } + m.poll.refresh() + return + } + + // Rewrite the wrapper before starting. It is generated, not edited, and + // something else may have replaced it — install-server.sh most obviously. + err := writeLaunchdFiles(autostartEnabled()) + if err == nil { + err = startServer() + } + if err != nil { + _ = alert("cix", fmt.Sprintf("Could not start the server.\n\n%v", err)) + return + } + + // launchctl reports success for having spawned the process, not for the + // process surviving. A server that exits on a configuration it cannot + // accept leaves the menu saying "Stopped" and nothing else — which is how + // a deleted database looked like a broken Start button. + if detail := serverDiedOnStart(); detail != "" { + logf("the server exited immediately after Start: %s", detail) + _ = alert("The server stopped straight away", detail+ + "\n\nThe full log is in ~/.cix/logs/cix-server.err.") } m.poll.refresh() } diff --git a/cli/launcher/resetpw_darwin.go b/cli/launcher/resetpw_darwin.go index f37ce3c..578d558 100644 --- a/cli/launcher/resetpw_darwin.go +++ b/cli/launcher/resetpw_darwin.go @@ -70,11 +70,11 @@ func (m *menu) resetPasswordFlow() { return } - _ = alertWithCopy("Password reset", fmt.Sprintf( + _ = alertWithSecret("Password reset", fmt.Sprintf( "Account:\n%s\n\nTemporary password:\n%s\n\n"+ "You will be asked to change it at the next sign-in. Other sessions for this "+ "account have been signed out.", email, password), - password, "Copy Password") + password, "password") } // runResetPassword executes the reset and returns the generated password.