From abadf0fc321bf9f3b53fb33bc53307b1b8679fa3 Mon Sep 17 00:00:00 2001 From: "ccf-lisa[bot]" <286799724+ccf-lisa[bot]@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:39:35 -0300 Subject: [PATCH] fix(deploy): install.sh's yes/no prompts take the answer typed Co-Authored-By: Claude Opus 5.5 --- cmd/factory-agent/installsh_test.go | 102 +++++++++++++++++++++++++++- deploy/install.sh | 31 ++++++--- 2 files changed, 121 insertions(+), 12 deletions(-) diff --git a/cmd/factory-agent/installsh_test.go b/cmd/factory-agent/installsh_test.go index 16295129..1e719571 100644 --- a/cmd/factory-agent/installsh_test.go +++ b/cmd/factory-agent/installsh_test.go @@ -6,7 +6,8 @@ package main // before changing anything. It is never run for real: in dry runs and refusals every command // that changes something is a stub that records the call and fails, and HOME stays empty; runs // that write get a curl stub serving fake factory-agent binaries (which only record their -// arguments) from a local dir, and --no-service. Only --quiet runs: the default mode reads /dev/tty. +// arguments) from a local dir, and --no-service. The default mode's runs answer from a file +// (FACTORY_INSTALL_TTY, the script's test seam for /dev/tty). import ( "crypto/sha256" @@ -46,13 +47,14 @@ var forbidden = []string{"curl", "wget", "install", "mkdir", "launchctl", "syste var forbiddenWriting = []string{"wget", "launchctl", "systemctl", "loginctl", "sudo", "tart", "factory-agent"} // installRun is one install.sh run; writes lets it write into the temp HOME, which home (when -// set) prepares. +// set) prepares. answers, when set, are what the terminal answers, one per line. type installRun struct { sysName, machine string env map[string]string args []string writes bool home func(t *testing.T, home string) + answers []string } // installResult is a run's exit code, output and HOME, and what the fake binaries were asked. @@ -115,6 +117,13 @@ case "$url" in "$STUB_BASE"/*) cp "$STUB_ASSETS/${url##*/}" "$out" ;; *) echo "c for k, v := range r.env { cmd.Env = append(cmd.Env, k+"="+v) } + if r.answers != nil { + tty := filepath.Join(t.TempDir(), "tty") + if err := os.WriteFile(tty, []byte(strings.Join(r.answers, "\n")+"\n"), 0o600); err != nil { + t.Fatal(err) + } + cmd.Env = append(cmd.Env, "FACTORY_INSTALL_TTY="+tty) + } out, err := cmd.CombinedOutput() code := 0 if err != nil { @@ -529,3 +538,92 @@ func TestInstallScriptRefuses(t *testing.T) { }) } } + +// TestInstallScriptAsks: the default mode, on a host that is enrolled and has an agent.yaml, +// takes the y/n answers typed (the terminal is a file of answers). +func TestInstallScriptAsks(t *testing.T) { + skipWithoutShasum(t) + const mine = "# my own agent.yaml\n" + dir := func(home string) string { return filepath.Join(home, ".config/factory-agent") } + existing := func(t *testing.T, home string) { + state := filepath.Join(home, "Library/Application Support/factory-agent") + for _, d := range []string{dir(home), state} { + if err := os.MkdirAll(d, 0o700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(dir(home), "agent.yaml"), []byte(mine), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(state, "agent-refresh-token"), []byte("r\n"), 0o600); err != nil { + t.Fatal(err) + } + } + // settings answers the dispatcher's prompt and takes the default (Enter) for the 13 others. + settings := append([]string{"https://factory.example.dev"}, make([]string, 13)...) + answers := func(parts ...[]string) []string { + var all []string + for _, p := range parts { + all = append(all, p...) + } + return all + } + tests := []struct { + name string + args []string + answers []string + wantCode int + want string + replaced bool + }{ + { + // Replace: y; the settings; Write it: y; both model logins: Enter (no). + name: "replaced", answers: answers([]string{"y"}, settings, []string{"y", "", ""}), + want: "writing ", replaced: true, + }, + { + name: "kept", args: []string{"--dry-run"}, answers: []string{"n"}, + want: "keeping it", + }, + { + name: "kept on Enter", args: []string{"--dry-run"}, answers: []string{""}, + want: "keeping it", + }, + { + name: "not written", answers: answers([]string{"y"}, settings, []string{"n"}), + wantCode: 1, want: "not written; nothing was changed", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := installRun{ + sysName: "Darwin", machine: "arm64", writes: true, home: existing, answers: tt.answers, + args: append([]string{"--no-service"}, tt.args...), + }.do(t) + if res.code != tt.wantCode || !strings.Contains(res.out, tt.want) { + t.Fatalf("exit %d, want %d and %q\n%s", res.code, tt.wantCode, tt.want, res.out) + } + baks, _ := filepath.Glob(filepath.Join(dir(res.home), "agent.yaml.bak-*")) + if !tt.replaced { + if b, _ := os.ReadFile(filepath.Join(dir(res.home), "agent.yaml")); string(b) != mine || len(baks) != 0 { + t.Errorf("agent.yaml is now %q, backups %v", b, baks) + } + return + } + cfg := written(t, res.home) + if cfg.Dispatcher != "https://factory.example.dev" || cfg.Pool != "local" || cfg.Host != "my-host" { + t.Errorf("dispatcher %q, pool %q, host %q", cfg.Dispatcher, cfg.Pool, cfg.Host) + } + checkDefaults(t, cfg) + if len(baks) != 1 { + t.Fatalf("backups %v", baks) + } + if b, _ := os.ReadFile(baks[0]); string(b) != mine { + t.Errorf("the backup holds %q", b) + } + if !strings.Contains(res.agentRan, "host init") || strings.Contains(res.agentRan, "--yes") || strings.Contains(res.agentRan, "login") { + t.Errorf("the agent ran %q, want host init without --yes and no login", res.agentRan) + } + }) + } +} diff --git a/deploy/install.sh b/deploy/install.sh index 180219de..71c36482 100755 --- a/deploy/install.sh +++ b/deploy/install.sh @@ -162,21 +162,32 @@ run() { [ "$dry_run" = 1 ] || "$@" } +# The terminal the prompts read: /dev/tty. FACTORY_INSTALL_TTY, a file of answers (one per +# line), is for installsh_test.go. ask opens it once, on fd 9, so a file is read line by line. +tty="${FACTORY_INSTALL_TTY:-/dev/tty}" +tty_open=0 + # have_tty reports whether there is a terminal to ask on. -have_tty() { { : /dev/null; } +have_tty() { [ "$tty_open" = 1 ] || { : <"$tty"; } 2>/dev/null; } -# ask VAR PROMPT [silent]: reads a line from the terminal into VAR. +# ask VAR PROMPT [silent]: reads a line from the terminal into VAR. Its locals are _ask_*, names +# no caller's VAR uses: printf -v on a name ask declares would set ask's local, not the caller's. ask() { - local var="$1" prompt="$2" silent="${3:-}" answer="" - [ "$quiet" = 0 ] || die "--quiet: nothing may be asked ($prompt)" + local _ask_var="$1" _ask_prompt="$2" _ask_silent="${3:-}" _ask_reply="" + [ "$quiet" = 0 ] || die "--quiet: nothing may be asked ($_ask_prompt)" have_tty || die "no terminal to ask on: pass --quiet, with the settings as flags or environment variables" - if [ -n "$silent" ]; then - read -rs -p "$prompt" answer /dev/tty + if [ "$tty_open" = 0 ]; then + exec 9<"$tty" + tty_open=1 + fi + if [ -n "$_ask_silent" ]; then + read -rs -p "$_ask_prompt" _ask_reply <&9 || die "no answer (the terminal closed)" + # The newline the silent read didn't echo, where read printed the prompt. + echo >&2 else - read -r -p "$prompt" answer