From eb6b875b167362cf2231ab8afffe67d08a6770ca Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 20:26:55 +0300 Subject: [PATCH 1/2] Fix installer upgrades, config validation, and CLI deadlines --- cmd/pilotctl/appstore.go | 4 +- cmd/pilotctl/config_values.go | 163 +++++++++++++++++++++++ cmd/pilotctl/main.go | 91 ++++++++----- cmd/pilotctl/updates.go | 1 - cmd/pilotctl/zz_installer_issues_test.go | 125 +++++++++++++++++ cmd/pilotctl/zz_issue_review_test.go | 117 ++++++++++++++++ install.sh | 39 ++++-- 7 files changed, 494 insertions(+), 46 deletions(-) create mode 100644 cmd/pilotctl/config_values.go create mode 100644 cmd/pilotctl/zz_installer_issues_test.go create mode 100644 cmd/pilotctl/zz_issue_review_test.go diff --git a/cmd/pilotctl/appstore.go b/cmd/pilotctl/appstore.go index 13067a44..d42e3e21 100644 --- a/cmd/pilotctl/appstore.go +++ b/cmd/pilotctl/appstore.go @@ -193,13 +193,13 @@ func cmdAppStoreList(_ []string) { _ = json.NewEncoder(os.Stdout).Encode([]appListEntry{}) return } - fmt.Fprintf(os.Stderr, "no install root at %s — install an app first\n", root) + fmt.Printf("no apps installed under %s\n", root) return } fatalHint("io_error", "check the install root permissions", "read %s: %v", root, err) } - var apps []appListEntry + apps := make([]appListEntry, 0) for _, e := range entries { if !e.IsDir() { continue diff --git a/cmd/pilotctl/config_values.go b/cmd/pilotctl/config_values.go new file mode 100644 index 00000000..b564a5a0 --- /dev/null +++ b/cmd/pilotctl/config_values.go @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package main + +import ( + "encoding/hex" + "fmt" + "net" + "net/url" + "strconv" + "strings" + "time" + + "github.com/pilot-protocol/pilotprotocol/internal/proxyconf" +) + +// Config keys mirror the daemon flags, using the underscore spelling read +// by pilotctl as well as the daemon. Unknown keys must never look successful. +var configValueKinds = map[string]string{ + "admin_token": "String", + "advertise_endpoint": "String", + "beacon": "String", + "beacon_rtt_probe": "Bool", + "compat_beacon": "String", + "dataexchange_b64": "Bool", + "email": "String", + "encrypt": "Bool", + "endpoint": "String", + "enterprise_control": "String", + "hostname": "String", + "identity": "String", + "idle_timeout": "Duration", + "keepalive": "Duration", + "listen": "String", + "log_format": "String", + "log_level": "String", + "log_max_backups": "Int", + "log_max_size": "Int", + "max_conns_per_port": "Int", + "max_conns_total": "Int", + "motd_feed_url": "String", + "motd_interval": "Duration", + "networks": "String", + "no_dataexchange": "Bool", + "no_echo": "Bool", + "no_eventstream": "Bool", + "no_path_watch": "Bool", + "no_rx_watchdog": "Bool", + "no_skillinject": "Bool", + "owner": "String", + "proxy": "String", + "proxy_cmd": "String", + "public": "Bool", + "registry": "String", + "registry_fingerprint": "String", + "registry_tls": "Bool", + "registry_trust": "String", + "rekey_whitelist": "String", + "relay_only": "Bool", + "reply_whitelist": "String", + "sandbox": "Bool", + "sandbox_dir": "String", + "security_profile": "String", + "socket": "String", + "strict_dataplane_trust": "Bool", + "syn_rate_limit": "Int", + "syn_whitelist": "String", + "telemetry_url": "String", + "time_wait": "Duration", + "tls_trust": "String", + "transport": "String", + "trust_auto_approve": "Bool", + "webhook": "String", + "webhook_secret": "String", +} + +func validateConfigValue(key, raw string) (interface{}, error) { + kind, ok := configValueKinds[key] + if !ok { + return nil, fmt.Errorf("unknown key %q", key) + } + if raw == "" && clearableConfigKeys[key] { + return "", nil + } + value := strings.TrimSpace(raw) + if value == "" { + return nil, fmt.Errorf("%s must not be empty", key) + } + switch kind { + case "Bool": + if value != "true" && value != "false" { + return nil, fmt.Errorf("%s must be true or false", key) + } + return value == "true", nil + case "Int": + n, err := strconv.Atoi(value) + if err != nil || n < 0 { + return nil, fmt.Errorf("%s must be a non-negative integer", key) + } + return n, nil + case "Duration": + d, err := time.ParseDuration(value) + if err != nil || d < 0 { + return nil, fmt.Errorf("%s must be a non-negative duration", key) + } + return value, nil + } + switch key { + case "transport": + t, err := normalizeTransport(value) + if err == nil && t == "" { + err = validateTransport(value) + } + return t, err + case "proxy": + return proxyconf.Normalize(value) + case "registry", "beacon", "endpoint", "advertise_endpoint", "listen": + host, port, err := net.SplitHostPort(value) + n, portErr := strconv.Atoi(port) + if err != nil || portErr != nil || n < 0 || n > 65535 || + (key != "listen" && (host == "" || n == 0)) || + strings.ContainsAny(host, " /\\?#@\t\r\n") { + return nil, fmt.Errorf("%s must be a host:port address", key) + } + case "socket", "identity", "enterprise_control", "sandbox_dir": + if strings.ContainsRune(raw, 0) { + return nil, fmt.Errorf("%s contains a NUL byte", key) + } + case "hostname": + if len(value) > 63 || strings.Trim(value, "abcdefghijklmnopqrstuvwxyz0123456789-") != "" || strings.HasPrefix(value, "-") || strings.HasSuffix(value, "-") { + return nil, fmt.Errorf("hostname must be 1-63 lowercase letters, digits or internal hyphens") + } + case "webhook", "motd_feed_url", "telemetry_url", "compat_beacon": + u, err := url.Parse(value) + if err != nil || u.Hostname() == "" || u.User != nil || + (key == "compat_beacon" && u.Scheme != "wss") || + (key != "compat_beacon" && u.Scheme != "http" && u.Scheme != "https") { + return nil, fmt.Errorf("invalid URL for %s", key) + } + case "registry_trust", "tls_trust": + if value != "pinned" && value != "system" { + return nil, fmt.Errorf("%s must be pinned or system", key) + } + case "registry_fingerprint": + b, err := hex.DecodeString(value) + if err != nil || len(b) != 32 { + return nil, fmt.Errorf("registry_fingerprint must be a SHA-256 hex digest") + } + case "log_level": + if value != "debug" && value != "info" && value != "warn" && value != "error" { + return nil, fmt.Errorf("invalid log_level") + } + case "log_format": + if value != "text" && value != "json" { + return nil, fmt.Errorf("log_format must be text or json") + } + case "security_profile": + if value != "compatible" && value != "enterprise" { + return nil, fmt.Errorf("security_profile must be compatible or enterprise") + } + } + return raw, nil +} diff --git a/cmd/pilotctl/main.go b/cmd/pilotctl/main.go index 7968cd98..6b71e409 100644 --- a/cmd/pilotctl/main.go +++ b/cmd/pilotctl/main.go @@ -884,7 +884,8 @@ Flags: --type text|json|binary payload encoding (default: text) --count send N times (default: 1) --reuse-conn reuse the connection across --count sends (saves ~1 RTT) - --wait [] wait for a reply in the inbox (default timeout: 30s) + --wait [] reply wait only, after sending (default: 30s) + --timeout total wall time, including connect, handshake and reply --trace print per-step timing breakdown to stderr --no-auto-handshake skip automatic trust handshake with known agents --enterprise-control request a signed enterprise decision before sending @@ -1570,7 +1571,12 @@ func printCommandHelp(cmd string, cmdArgs []string) { // --- Usage --- func usage() { - fmt.Fprintf(os.Stderr, `pilotctl — Pilot Protocol CLI + printUsage(os.Stderr) + os.Exit(2) +} + +func printUsage(w io.Writer) { + fmt.Fprintf(w, `pilotctl — Pilot Protocol CLI Global flags: --json Output structured JSON (for agent/programmatic use) @@ -1697,7 +1703,6 @@ Companion binaries: $PILOT_DAEMON_BIN / $PILOT_GATEWAY_BIN, next to the pilotctl executable, then $PATH. `) - os.Exit(2) } // --- Main --- @@ -1733,8 +1738,9 @@ func main() { cmdArgs := args[1:] // Top-level help - if cmd == "-h" || cmd == "--help" { - usage() + if cmd == "help" || cmd == "-h" || cmd == "--help" { + printUsage(os.Stdout) + return } // Per-command help: pilotctl -h / --help if hasHelpFlag(cmdArgs) { @@ -1756,7 +1762,7 @@ dispatch: cmdArgs = cmdArgs[1:] goto dispatch - case "version": + case "version", "--version": fmt.Println(version) return @@ -2159,28 +2165,10 @@ func cmdConfig(args []string) { if len(parts) != 2 { fatalCode("invalid_argument", "usage: pilotctl config --set key=value") } - // Validate the keys daemon start (and pilot-daemon, which reads - // config.json itself) interprets, so a typo fails here rather than - // on the next daemon start. Empty clears the key. - value := parts[1] - if value != "" { - switch parts[0] { - case "transport": - t, err := normalizeTransport(value) - if err == nil && t == "" { - err = validateTransport(value) - } - if err != nil { - fatalCode("invalid_argument", "config: %v", err) - } - value = t - case "proxy": - p, err := proxyconf.Normalize(value) - if err != nil { - fatalCode("invalid_argument", "config: %v", err) - } - value = p - } + parts[0] = strings.ReplaceAll(parts[0], "-", "_") + value, err := validateConfigValue(parts[0], parts[1]) + if err != nil { + fatalCode("invalid_argument", "config: %v", err) } cfg := loadConfig() cfg[parts[0]] = value @@ -2206,7 +2194,7 @@ func cmdConfig(args []string) { fatalCode("internal", "save config: %v", err) } if parts[0] == "proxy" { - result["value"] = redactProxyURL(value) + result["value"] = redactProxyURL(value.(string)) } outputOK(result) return @@ -2488,7 +2476,7 @@ func contextCatalog() map[string]interface{} { // Messaging "send-message": map[string]interface{}{ - "args": []string{"", "--data ", "[--type text|json|binary]", "[--count ]", "[--reuse-conn]"}, + "args": []string{"", "--data ", "[--type text|json|binary]", "[--count ]", "[--reuse-conn]", "[--wait ]", "[--timeout ]"}, "description": "Send a typed message to a node via data exchange (port 1001). --count N sends N messages; --reuse-conn shares one connection across all N (env: PILOT_SENDMSG_REUSE_CONN=1). Default type: text", "returns": "target, to, type, bytes, ack, reuse_conn", }, @@ -2859,7 +2847,17 @@ func planDaemonLaunch(args []string) daemonLaunchPlan { hostname = h } } - encrypt := !flagBool(flags, "no-encrypt") + encrypt := true + if value, ok := cfg["encrypt"]; ok { + parsed, err := strconv.ParseBool(fmt.Sprint(value)) + if err != nil { + fatalCode("invalid_argument", "config: encrypt must be true or false") + } + encrypt = parsed + } + if flagBool(flags, "no-encrypt") { + encrypt = false + } identityPath := flagString(flags, "identity", "") if identityPath == "" { identityPath = configDir() + "/identity.json" @@ -2923,7 +2921,7 @@ func planDaemonLaunch(args []string) daemonLaunchPlan { "--log-format", logFormat, ) // pilot-daemon's encrypt flag defaults to true; pass `=false` - // only when --no-encrypt was supplied. + // when disabled through config or --no-encrypt. if !encrypt { daemonArgs = append(daemonArgs, "--encrypt=false") } @@ -4822,7 +4820,28 @@ func streamSendFile(d *driver.Driver, target protocol.Addr, filePath, filename s func cmdSendMessage(args []string) { flags, pos := parseFlags(args) - if len(pos) < 1 { + for name := range flags { + switch name { + case "data", "type", "count", "reuse-conn", "wait", "timeout", "trace", "no-auto-handshake", "enterprise-control", "governed-resource": + default: + fatalCode("invalid_argument", "send-message: unknown flag --%s", name) + } + } + // The CLI owns a hard wall-clock budget: driver and registry requests, + // handshakes, policy decisions, ACKs and inbox polling can all block. + // Exiting closes the IPC session, cancelling its outstanding daemon work. + // --wait retains its reply-only meaning; --timeout caps the entire command. + if raw, ok := flags["timeout"]; ok { + timeout, err := time.ParseDuration(raw) + if err != nil || timeout <= 0 { + fatalCode("invalid_argument", "--timeout must be a positive duration") + } + timer := time.AfterFunc(timeout, func() { + fatalHint("timeout", "delivery may already have occurred; check the inbox before retrying", "send-message exceeded total timeout %s", timeout) + }) + defer timer.Stop() + } + if len(pos) != 1 { fatalCode("invalid_argument", "usage: pilotctl send-message --data [--type text|json|binary] [--trace] [--count ] [--reuse-conn] [--wait ] [--enterprise-control --governed-resource ]") } @@ -4836,7 +4855,11 @@ func cmdSendMessage(args []string) { if raw == "true" { waitDur = 30 * time.Second } else { - waitDur = flagDuration(flags, "wait", 30*time.Second) + var err error + waitDur, err = time.ParseDuration(raw) + if err != nil || waitDur <= 0 { + fatalCode("invalid_argument", "--wait must be a positive duration") + } } } diff --git a/cmd/pilotctl/updates.go b/cmd/pilotctl/updates.go index 7661dd6a..c824b491 100644 --- a/cmd/pilotctl/updates.go +++ b/cmd/pilotctl/updates.go @@ -192,7 +192,6 @@ func printUpdateState(st updater.Status, restart daemonRestart) { case restart.recorded == "": case restart.resolved(): fmt.Printf("Daemon restart: not needed — the daemon runs the installed %s\n", restart.daemonVersion) - fmt.Printf("%s(the recorded restart error is out of date: %s)\n", indent, restart.recorded) case restart.daemonDown(): fmt.Printf("Daemon restart: daemon not running — %s runs when it starts\n", orDash(restart.installed)) fmt.Printf("%slast restart attempt: %s\n", indent, restart.recorded) diff --git a/cmd/pilotctl/zz_installer_issues_test.go b/cmd/pilotctl/zz_installer_issues_test.go new file mode 100644 index 00000000..5cdfd5dd --- /dev/null +++ b/cmd/pilotctl/zz_installer_issues_test.go @@ -0,0 +1,125 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func installerSection(t *testing.T, start, end string) string { + t.Helper() + b, err := os.ReadFile("../../install.sh") + if err != nil { + t.Fatal(err) + } + s := string(b) + a := strings.Index(s, start) + if a < 0 { + t.Fatal("missing", start) + } + s = s[a:] + bidx := strings.Index(s, end) + if bidx < 0 { + t.Fatal("missing", end) + } + return s[:bidx] +} + +func TestIssue475LegacyLaunchdMigration(t *testing.T) { + section := installerSection(t, "if [ \"$OS\" = \"darwin\" ]; then\n for _label in network.pilotprotocol.pilot-daemon", "# install_bin") + for _, tc := range []struct { + name string + loaded, fail bool + }{{"loaded", true, false}, {"unloaded", false, false}, {"bootout fails", true, true}} { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + dir := filepath.Join(home, "Library", "LaunchAgents") + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + old := filepath.Join(dir, "com.vulturelabs.pilot-daemon.plist") + original := "legacy operator flags" + os.WriteFile(old, []byte(original), 0600) + script := `set -eu +OS=darwin +PILOT_MANAGED_MODE=0 +PILOT_MANAGED_NO_START=0 +RESTART_LAUNCHD="" +launchctl() { + case "$1:$2" in + print:*/com.vulturelabs.pilot-daemon) [ "$LOADED" = 1 ] ;; + print:*) return 1 ;; + bootout:*) [ "$FAIL_BOOTOUT" != 1 ] ;; + *) return 99 ;; + esac +} +` + section + `printf 'RESTART=%s\n' "$RESTART_LAUNCHD"` + cmd := exec.Command("sh", "-c", script) + cmd.Env = append(os.Environ(), "HOME="+home, "LOADED=0", "FAIL_BOOTOUT=0") + if tc.loaded { + cmd.Env = append(cmd.Env, "LOADED=1") + } + if tc.fail { + cmd.Env = append(cmd.Env, "FAIL_BOOTOUT=1") + } + out, err := cmd.CombinedOutput() + if tc.fail { + if err == nil { + t.Fatal("must abort") + } + if _, err := os.Stat(old); err != nil { + t.Fatal("legacy plist removed on failure") + } + return + } + if err != nil { + t.Fatalf("%v: %s", err, out) + } + if _, err := os.Stat(old); !os.IsNotExist(err) { + t.Fatal("legacy plist still active") + } + b, err := os.ReadFile(filepath.Join(dir, "network.pilotprotocol.pilot-daemon.plist")) + if err != nil || string(b) != original { + t.Fatalf("lost operator settings: %s %v", b, err) + } + want := "RESTART=\n" + if tc.loaded { + want = "RESTART=network.pilotprotocol.pilot-daemon\n" + } + if !strings.Contains(string(out), want) { + t.Fatalf("want %q: %s", want, out) + } + }) + } +} + +func TestIssue476UpgradeAutoUpdateState(t *testing.T) { + section := installerSection(t, "# Enable background auto-updates by default", "# --- Set up system service ---") + for _, existing := range []string{"", `{"enabled":false}`, `{"enabled":true}`} { + t.Run(existing, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "auto-update.json") + if existing != "" { + os.WriteFile(path, []byte(existing), 0600) + } + cmd := exec.Command("sh", "-c", "set -eu\n"+section) + cmd.Env = append(os.Environ(), "PILOT_DIR="+dir, "PILOT_MANAGED_MODE=0", "UPDATING=true") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%v: %s", err, out) + } + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if existing != "" && string(b) != existing { + t.Fatalf("operator setting changed: %s", b) + } + if existing == "" && !strings.Contains(string(b), `"enabled": true`) { + t.Fatalf("upgrade not enabled: %s", b) + } + }) + } +} diff --git a/cmd/pilotctl/zz_issue_review_test.go b/cmd/pilotctl/zz_issue_review_test.go new file mode 100644 index 00000000..612d4aa0 --- /dev/null +++ b/cmd/pilotctl/zz_issue_review_test.go @@ -0,0 +1,117 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestIssue477RejectInvalidConfig(t *testing.T) { + for _, kv := range []string{"transport=bogus", "proxy=notaurl", "encrypt=maybe", "encrypt=", "socket=", "registry=notanaddr", "registry=host:99999", "beacon=:9001", "nonsense_key=1", "log_max_size=-1"} { + t.Run(kv, func(t *testing.T) { + home := t.TempDir() + out, errOut, code := runCLI(t, []string{"--json", "config", "--set", kv}, map[string]string{"HOME": home, "PILOT_HOME": home}) + if code == 0 || !strings.Contains(out+errOut, "invalid_argument") { + t.Fatalf("exit %d: %s %s", code, out, errOut) + } + if _, err := os.Stat(filepath.Join(home, ".pilot", "config.json")); !os.IsNotExist(err) { + t.Fatalf("invalid setting wrote config: %v", err) + } + }) + } +} + +func TestIssue477EncryptBooleanAndLaunch(t *testing.T) { + withTempHomeFull(t) + for _, val := range []string{"false", "true"} { + captureStdout(t, func() { cmdConfig([]string{"--set", "encrypt=" + val}) }) + cfg := loadConfig() + if got, ok := cfg["encrypt"].(bool); !ok || got != (val == "true") { + t.Fatalf("encrypt not a JSON boolean: %#v", cfg["encrypt"]) + } + args, _, _ := buildDaemonArgs(nil) + if got := strings.Contains(strings.Join(args, " "), "--encrypt=false"); got != (val == "false") { + t.Fatalf("encrypt %s: %v", val, args) + } + } + args, _, _ := buildDaemonArgs([]string{"--no-encrypt"}) + if !strings.Contains(strings.Join(args, " "), "--encrypt=false") { + t.Fatal(args) + } +} + +func TestIssue481HelpAndVersion(t *testing.T) { + for _, args := range [][]string{{"help"}, {"--help"}, {"-h"}, {"--version"}, {"version"}} { + out, stderr, code := runCLI(t, args, nil) + if code != 0 || out == "" || strings.Contains(stderr, "unknown command") { + t.Fatalf("%v: exit=%d out=%q err=%q", args, code, out, stderr) + } + } +} + +func TestIssue481EmptyAppList(t *testing.T) { + for _, exists := range []bool{true, false} { + root := filepath.Join(t.TempDir(), "apps") + if exists { + if err := os.Mkdir(root, 0700); err != nil { + t.Fatal(err) + } + } + for _, jsonMode := range []bool{true, false} { + args := []string{"appstore", "list"} + if jsonMode { + args = append([]string{"--json"}, args...) + } + out, stderr, code := runCLI(t, args, map[string]string{"PILOT_APPSTORE_ROOT": root}) + if code != 0 || stderr != "" { + t.Fatalf("exit=%d out=%s err=%s", code, out, stderr) + } + if jsonMode && strings.TrimSpace(out) != "[]" { + t.Fatalf("want [], got %s", out) + } + } + } +} + +func TestIssue480RejectUnknownAndInvalidFlags(t *testing.T) { + for _, flags := range [][]string{{"--bogusflag"}, {"--timeout", "nonsense"}, {"--timeout", "0s"}, {"--wait", "-1s"}} { + args := append([]string{"--json", "send-message", "0:0000.0000.002A", "--data", "hi"}, flags...) + out, stderr, code := runCLI(t, args, map[string]string{"PILOT_SOCKET": "/tmp/nonexistent-issue480.sock"}) + if code == 0 || !strings.Contains(out+stderr, "invalid_argument") { + t.Fatalf("%v: %d %s %s", flags, code, out, stderr) + } + } +} + +func TestIssue480TotalTimeout(t *testing.T) { + for _, phase := range []string{"resolve", "handshake", "dial", "ack", "reply"} { + t.Run(phase, func(t *testing.T) { + d := newStreamDaemon(t) + target := "0:0000.0000.002A" + switch phase { + case "resolve": + target = "dead-peer" + d.on(tdCmdResolveHostname, func([]byte) [][]byte { return nil }) + case "handshake": + d.on(tdCmdHandshake, func([]byte) [][]byte { return nil }) + case "dial": + d.on(tdCmdDial, func([]byte) [][]byte { return nil }) + case "ack": + d.on(tdCmdSend, func([]byte) [][]byte { return nil }) + } + start := time.Now() + out, stderr, code := runCLI(t, []string{"--json", "send-message", target, "--data", "hi", "--wait", "5s", "--timeout", "200ms"}, map[string]string{"PILOT_SOCKET": d.path}) + elapsed := time.Since(start) + var result map[string]interface{} + if err := json.Unmarshal([]byte(stderr), &result); err != nil { + t.Fatalf("invalid JSON %q: %v; stderr=%s", out, err, stderr) + } + if code == 0 || !strings.Contains(stderr, "exceeded total timeout") || elapsed > 2*time.Second { + t.Fatalf("exit=%d elapsed=%v out=%s stderr=%s", code, elapsed, out, stderr) + } + }) + } +} diff --git a/install.sh b/install.sh index 7dc168ca..fea6d263 100755 --- a/install.sh +++ b/install.sh @@ -1218,15 +1218,36 @@ if [ "$OS" = "linux" ] && [ "$CAN_PRIV" = true ] \ done fi if [ "$OS" = "darwin" ]; then - for _label in network.pilotprotocol.pilot-daemon network.pilotprotocol.pilot-updater; do + for _label in network.pilotprotocol.pilot-daemon network.pilotprotocol.pilot-updater com.vulturelabs.pilot-daemon com.vulturelabs.pilot-updater; do _lp="$HOME/Library/LaunchAgents/${_label}.plist" - if [ -f "$_lp" ] && launchctl list 2>/dev/null | grep -q "$_label"; then + _new_label="$_label" + case "$_label" in + com.vulturelabs.*) _new_label="network.pilotprotocol.${_label#com.vulturelabs.}" ;; + esac + if launchctl print "gui/$(id -u)/${_label}" >/dev/null 2>&1; then + # Do not replace binaries or install a duplicate service if the + # old job cannot be stopped. bootout also handles a missing plist. + if ! launchctl bootout "gui/$(id -u)/${_label}" 2>/dev/null; then + echo "Error: could not unload ${_label}; aborting upgrade" >&2 + exit 1 + fi if { [ "$PILOT_MANAGED_MODE" != "1" ] || [ "$PILOT_MANAGED_NO_START" != "1" ]; } \ - && { [ "$PILOT_MANAGED_MODE" != "1" ] || [ "$_label" != "network.pilotprotocol.pilot-updater" ]; }; then - RESTART_LAUNCHD="${RESTART_LAUNCHD}${RESTART_LAUNCHD:+ }${_label}" + && { [ "$PILOT_MANAGED_MODE" != "1" ] || [ "$_new_label" != "network.pilotprotocol.pilot-updater" ]; }; then + case " $RESTART_LAUNCHD " in + *" ${_new_label} "*) ;; + *) RESTART_LAUNCHD="${RESTART_LAUNCHD}${RESTART_LAUNCHD:+ }${_new_label}" ;; + esac fi - launchctl unload "$_lp" 2>/dev/null || true - echo " Unloaded ${_label} (will reload after upgrade)" + echo " Unloaded ${_label} (will reload as ${_new_label} after upgrade)" + fi + if [ "$_new_label" != "$_label" ] && [ -f "$_lp" ]; then + # Preserve operator arguments for the plist-regeneration code, + # then retire the legacy RunAtLoad job even if it was not loaded. + _new_lp="$HOME/Library/LaunchAgents/${_new_label}.plist" + if [ ! -f "$_new_lp" ]; then + cp "$_lp" "$_new_lp" + fi + mv "$_lp" "${_lp}.migrated" fi done fi @@ -1718,7 +1739,7 @@ if [ "$PILOT_MANAGED_MODE" = "1" ]; then # a background downgrade to silently remove enforcement. printf '{\n "enabled": false,\n "reason": "managed-runtime-pinned-by-authority"\n}\n' > "$PILOT_DIR/auto-update.json" echo "Auto-updates pinned to the management authority runtime channel" -elif [ "$UPDATING" != true ] && [ ! -f "$PILOT_DIR/auto-update.json" ]; then +elif [ ! -f "$PILOT_DIR/auto-update.json" ]; then printf '{\n "enabled": true\n}\n' > "$PILOT_DIR/auto-update.json" echo "Auto-updates ENABLED (opt-out) — disable with: pilotctl update disable" fi @@ -1845,7 +1866,7 @@ USVC if [ "$PILOT_MANAGED_MODE" != "1" ] && [ -f "$BIN_DIR/pilot-updater" ]; then # shellcheck disable=SC2086 if $PILOT_SUDO systemctl enable --now pilot-updater; then - echo " Started: pilot-updater (auto-updates enabled)" + echo " Started: pilot-updater (check automatic-update state: pilotctl update status)" else echo " Note: could not enable pilot-updater via systemd (non-fatal)." fi @@ -2047,7 +2068,7 @@ UPLIST if [ "$PILOT_MANAGED_MODE" != "1" ] && [ -f "$BIN_DIR/pilot-updater" ] && [ -f "$UPLIST" ]; then launchctl unload "$UPLIST" 2>/dev/null || true launchctl load -w "$UPLIST" - echo " Started: pilot-updater (auto-updates enabled)" + echo " Started: pilot-updater (check automatic-update state: pilotctl update status)" fi # Reload the daemon agent if it was loaded before we swapped its binary, From 172db21515315235863684e3cb80d2daace8f5fc Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 21:06:40 +0300 Subject: [PATCH 2/2] pilotctl: retry the daemon -help probe on ETXTBSY; don't cache a failed probe The probe ran pilot-daemon -help once; ETXTBSY (binary still open for writing: an updater swapping it, or a concurrently forked process holding the write fd) made it report the daemon's flags as unknown, and the empty result was cached. Seen as TestDaemonFlagUsageParsesUsage failing in CI under parallel tests. Retry up to 5x with 50 ms spacing within the probe timeout, and only cache successful probes. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/pilotctl/daemon_transport.go | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/cmd/pilotctl/daemon_transport.go b/cmd/pilotctl/daemon_transport.go index 6f937697..7731572e 100644 --- a/cmd/pilotctl/daemon_transport.go +++ b/cmd/pilotctl/daemon_transport.go @@ -6,6 +6,7 @@ import ( "bufio" "context" "encoding/json" + "errors" "fmt" "os" "os/exec" @@ -14,6 +15,7 @@ import ( "runtime" "strings" "sync" + "syscall" "time" "github.com/pilot-protocol/pilotprotocol/internal/proxyconf" @@ -350,12 +352,23 @@ func daemonFlagUsage(bin string) map[string]string { } ctx, cancel := context.WithTimeout(context.Background(), daemonFlagProbeTimeout) defer cancel() - cmd := exec.CommandContext(ctx, bin, "-help") - // Minimal environment: -help prints each flag's default, and an older - // daemon with env-backed defaults would echo e.g. a credential-bearing - // $PILOT_PROXY into the captured output. - cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + os.Getenv("HOME")} - out, _ := cmd.CombinedOutput() // -help exits 0 (flag.ExitOnError) or 2 on older builds + var out []byte + for attempt := 0; ; attempt++ { + cmd := exec.CommandContext(ctx, bin, "-help") + // Minimal environment: -help prints each flag's default, and an older + // daemon with env-backed defaults would echo e.g. a credential-bearing + // $PILOT_PROXY into the captured output. + cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + os.Getenv("HOME")} + var err error + out, err = cmd.CombinedOutput() // -help exits 0 (flag.ExitOnError) or 2 on older builds + // ETXTBSY: the binary is still open for writing (an updater or + // installer swapping it, or a concurrently forked process that + // inherited the write fd). It clears within milliseconds. + if !errors.Is(err, syscall.ETXTBSY) || attempt >= 4 || ctx.Err() != nil { + break + } + time.Sleep(50 * time.Millisecond) + } var set map[string]string current := "" sc := bufio.NewScanner(strings.NewReader(string(out))) @@ -381,7 +394,10 @@ func daemonFlagUsage(bin string) map[string]string { set[current] += strings.TrimSpace(line) + " " } } - daemonFlagCache[bin] = set + if set != nil { + // A failed probe is not cached, so a later call can still learn the flags. + daemonFlagCache[bin] = set + } return set }