Report what the docs-builder installer actually did

The upstream installer exits 0 whether it installs, skips an existing
binary, or is declined at its overwrite prompt, and docs-utils reported
every one of those as "Installed docs-builder." A user who answered "n"
saw Installation aborted immediately followed by [OK] Installed
docs-builder, contradicted two lines later by the status table still
offering the update.

The outcome now comes from the tool's reported version either side of the
run, not from the exit code: installed, updated, left unchanged, or no
binary on PATH. installerOutcome is a pure function so the four cases are
covered by a table test.

Also collapses the duplicate "Installing documentation tools" header.
update called installOptionalTools once per tool and the function prints
its own section heading, so a full update printed the heading twice and
read like two separate phases.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H3vJ764kbRQMFqEtZxh42b

Author: theletterf

cmd/elastic-docs-utils/main.go

@@ -228,16 +228,55 @@ func installOptionalTools(r *ui.Renderer, vale, docsBuilder, dryRun, force, assu
 			}
 			updateDownloadProgress(progress, "docs-builder", completed, total)
 		}
+		// The upstream installer exits 0 whether it installs, skips an existing
+		// binary, or is declined at its overwrite prompt, so its exit code alone
+		// cannot tell us what happened. Compare the version either side of it.
+		before := updates.BinaryVersion("docs-builder", "--version")
 		if err := bootstrap.InstallDocsBuilderWithProgress(force, assumeYes, callback); err != nil {
 			failures = append(failures, fmt.Errorf("install docs-builder: %w", err))
 		} else {
-			r.Success("Installed docs-builder.")
+			progress.Stop()
+			level, message := installerOutcome("docs-builder", before, updates.BinaryVersion("docs-builder", "--version"))
+			switch level {
+			case outcomeMissing:
+				r.Warn("%s", message)
+			case outcomeUnchanged:
+				r.Info("%s", message)
+			default:
+				r.Success("%s", message)
+			}
 		}
 		progress.Stop()
 	}
 	return failures
 }
 
+type installerOutcomeLevel int
+
+const (
+	outcomeInstalled installerOutcomeLevel = iota
+	outcomeUpdated
+	outcomeUnchanged
+	outcomeMissing
+)
+
+// installerOutcome decides what an upstream installer actually did, from the
+// tool's reported version either side of the run. The installers exit 0 whether
+// they install, skip an existing binary, or are declined at an overwrite
+// prompt, so a zero exit on its own must not be reported as an install.
+func installerOutcome(tool, before, after string) (installerOutcomeLevel, string) {
+	switch {
+	case after == "":
+		return outcomeMissing, fmt.Sprintf("The %s installer finished, but no %s binary is on PATH.", tool, tool)
+	case before != "" && after == before:
+		return outcomeUnchanged, fmt.Sprintf("%s left unchanged at %s; the installer did not replace it.", tool, after)
+	case before == "":
+		return outcomeInstalled, fmt.Sprintf("Installed %s %s.", tool, after)
+	default:
+		return outcomeUpdated, fmt.Sprintf("Updated %s %s to %s.", tool, before, after)
+	}
+}
+
 func reportToolFailures(r *ui.Renderer, failures []error) {
 	for _, failure := range failures {
 		r.Warn("%v", failure)
@@ -452,16 +491,16 @@ func commandUpdate(r *ui.Renderer, args []string) error {
 	// A failed component must not stop the remaining ones, and the refreshed
 	// status below is most useful precisely when something went wrong.
 	var toolFailures []error
-	if selected.vale {
-		toolFailures = append(toolFailures, installOptionalTools(r, true, false, *dryRun, *force, false)...)
-	} else {
+	if !selected.vale {
 		r.Info("Skipping Vale and Elastic Vale rules.")
 	}
-	if selected.docsBuilder {
-		toolFailures = append(toolFailures, installOptionalTools(r, false, true, *dryRun, *force, false)...)
-	} else {
+	if !selected.docsBuilder {
 		r.Info("Skipping docs-builder.")
 	}
+	// One call for both tools: installOptionalTools prints its own section
+	// header, so calling it per tool printed "Installing documentation tools"
+	// twice and read like two separate phases.
+	toolFailures = append(toolFailures, installOptionalTools(r, selected.vale, selected.docsBuilder, *dryRun, *force, false)...)
 	reportToolFailures(r, toolFailures)
 	if *dryRun {
 		r.Info("Would refresh documentation tool status after updates.")

cmd/elastic-docs-utils/main_test.go

@@ -130,3 +130,31 @@ func TestRenderUpdateStatusShowsHintsForActionableRows(t *testing.T) {
 		t.Fatalf("current component printed a hint:\n%s", out.String())
 	}
 }
+
+// The upstream installers exit 0 whether they install, skip, or are declined,
+// so the reported outcome has to come from the version either side of the run.
+func TestInstallerOutcome(t *testing.T) {
+	cases := []struct {
+		name          string
+		before, after string
+		want          installerOutcomeLevel
+		contains      string
+	}{
+		{"declined leaves version untouched", "1.57.1", "1.57.1", outcomeUnchanged, "left unchanged at 1.57.1"},
+		{"fresh install", "", "1.58.0", outcomeInstalled, "Installed docs-builder 1.58.0"},
+		{"real update", "1.57.1", "1.58.0", outcomeUpdated, "1.57.1 to 1.58.0"},
+		{"binary absent afterwards", "1.57.1", "", outcomeMissing, "no docs-builder binary is on PATH"},
+		{"absent before and after", "", "", outcomeMissing, "no docs-builder binary is on PATH"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			level, msg := installerOutcome("docs-builder", c.before, c.after)
+			if level != c.want {
+				t.Fatalf("level = %v, want %v (msg %q)", level, c.want, msg)
+			}
+			if !strings.Contains(msg, c.contains) {
+				t.Fatalf("message %q does not contain %q", msg, c.contains)
+			}
+		})
+	}
+}

internal/updates/updates.go

@@ -260,6 +260,14 @@ func lookupHint(err error) string {
 	return "Could not query GitHub to determine the latest version"
 }
 
+// BinaryVersion reports the version a locally installed tool prints, or an
+// empty string when the tool is absent or prints nothing usable. Callers use it
+// to confirm that an installer actually changed the binary, because the
+// upstream installers exit 0 whether they install, skip, or are declined.
+func BinaryVersion(command string, args ...string) string {
+	return binaryVersion(command, args...)
+}
+
 func binaryVersion(command string, args ...string) string {
 	path, err := exec.LookPath(command)
 	if err != nil {