feat(setup): tag service-side IP resolve with a sentinel + observe SSH cost

The migration-summary preflight always emitted a "resolved from service,
not from device" ❌ row whenever the target was a hostname — even when
SSH was available and could have answered authoritatively. Two
problems compounded: the summary builder passed `nil` for the SSH
client (skipping the device-side ping), and resolveIP's service-side
fallback returned a bare fmt.Errorf the caller couldn't distinguish
from a real failure.

Changes:

  - ErrResolvedFromServiceOnly sentinel; service-side fallback wraps
    it with fmt.Errorf("%w: ...") so callers can errors.Is()-check.
    Apply-path callers that pass a real SSH client keep getting the
    same error shape they always did.
  - populatePlannedNetworkConfig now takes an SSHClient. GetMigrationSummary
    opens one when probe.SSHOK is true and passes it through, so the
    summary's resolve call uses the same device-side authority the
    apply paths use. Skipping the dial when SSH is known dead keeps
    a stale handshake-timeout from burning the preflight budget.
  - MigrationSummary gains ResolveIPSource ("device" / "service") and
    ResolveIPDurationMS so we can observe the SSH-ping cost in the
    wild. The historical comment claimed 2-5 s on firmware-27 devices —
    we now have data instead of a guess.
  - CLI renderer prints the new source + timing line, and only renders
    the ❌ ResolveIPError row for hard failures (both SSH ping AND
    service DNS failed).
  - Two regression tests cover the sentinel-tagging contract and the
    device-success-returns-nil-error path.

Related to https://github.com/gesellix/Bose-SoundTouch/issues/282.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cmd/soundtouch-cli/cmd_setup.go

@@ -949,6 +949,17 @@ func renderMigrationSummary(deviceIP, serviceURL string, s *setup.MigrationSumma
 	if s.ResolveIPError != "" {
 		PrintError("Resolve IP error: " + s.ResolveIPError)
 	}
+
+	// Observability for the IP-resolve path. Source tells the user whether
+	// the speaker itself was consulted (authoritative) or only the service
+	// (best-effort). DurationMS lets us watch the SSH-ping cost trend in
+	// the wild — historical comment claimed 2-5 s on firmware 27, worth
+	// re-evaluating as data accumulates.
+	if s.ResolveIPSource != "" {
+		fmt.Printf("Resolve IP source : %s (%d ms)\n", s.ResolveIPSource, s.ResolveIPDurationMS)
+	} else if s.ResolveIPDurationMS > 0 {
+		fmt.Printf("Resolve IP        : %d ms\n", s.ResolveIPDurationMS)
+	}
 }
 
 func setupRebootCmd() *cli.Command {

pkg/service/setup/migration_summary_telnet_test.go

@@ -185,3 +185,56 @@ func TestGetMigrationSummary_TelnetSucceedsSSHSucceeds(t *testing.T) {
 		t.Errorf("TelnetVerifiedConfig = %q, want %q", summary.TelnetVerifiedConfig, target)
 	}
 }
+
+// TestGetMigrationSummary_TelnetOnlyMigrationDetected pins the ordering
+// bug fixed in PR #294 / issue #293.
+//
+// Before the fix, GetMigrationSummary called checkIsMigratedFromProbe
+// before draining the telnet goroutine's result, so
+// summary.TelnetVerifiedConfig was empty when isTelnetMigrated read it
+// — and the telnet axis was always reported false. For speakers
+// migrated *only* via telnet (envswitch flip; no SSH XML rewrite,
+// no DNS hook, no CA install), this misclassification meant
+// summary.IsMigrated was false despite the speaker actually pointing
+// at AfterTouch. The CLI's `setup verify` exited non-zero, and the
+// web UI rendered "Not Migrated".
+//
+// The fix moves m.checkIsMigratedFromProbe(summary, probe) to run
+// *after* the <-telnetCh drain, so TelnetVerifiedConfig is populated
+// when isTelnetMigrated inspects it.
+//
+// The scenario here matches foob61451's 2026-05-16 #293 reproducer:
+// SSH unavailable / disabled (every axis false), telnet getpdo reports
+// the AfterTouch host, no other migration path applied.
+func TestGetMigrationSummary_TelnetOnlyMigrationDetected(t *testing.T) {
+	target := "http://example:8000"
+	ft := &fakeTelnet{
+		banner: "BoseShell\n-> ",
+		responses: map[string]string{
+			"getpdo CurrentSystemConfiguration": "margeServerUrl=" + target + "\n",
+		},
+	}
+
+	m, host, cleanup := telnetSummaryEnv(t, nil, ft)
+	defer cleanup()
+
+	summary, err := m.GetMigrationSummary(host, "", "", nil)
+	if err != nil {
+		t.Fatalf("GetMigrationSummary: %v", err)
+	}
+
+	// Pre-condition for the test to be meaningful: the telnet probe
+	// must have populated TelnetVerifiedConfig. Without this, the
+	// downstream assertions could pass trivially.
+	if !strings.Contains(summary.TelnetVerifiedConfig, target) {
+		t.Fatalf("setup: TelnetVerifiedConfig = %q, want it to contain %q", summary.TelnetVerifiedConfig, target)
+	}
+
+	if !summary.TelnetMigrated {
+		t.Errorf("TelnetMigrated = false, want true — telnet getpdo reports %q which matches Manager.ServerURL host. Likely regression of PR #294 ordering fix in GetMigrationSummary.", target)
+	}
+
+	if !summary.IsMigrated {
+		t.Errorf("IsMigrated = false, want true — telnet axis should carry IsMigrated when SSH-driven axes are false. Likely regression of PR #294 ordering fix.")
+	}
+}

pkg/service/setup/setup.go

@@ -92,7 +92,18 @@ type MigrationSummary struct {
 	// flag pairing as a precondition independently of the URL flip.
 	IsPaired bool `json:"is_paired"`
 
-	ResolveIPError      string   `json:"resolve_ip_error,omitempty"`
+	ResolveIPError string `json:"resolve_ip_error,omitempty"`
+	// ResolveIPSource records where the resolved IP came from:
+	// "device" — authoritative answer via SSH ping (preferred for
+	// migration). "service" — service-side DNS lookup (fast but may
+	// differ when NAT or split-DNS is in play). Empty when host was
+	// already an IP literal or could not be resolved at all.
+	ResolveIPSource string `json:"resolve_ip_source,omitempty"`
+	// ResolveIPDurationMS measures how long the resolve call took
+	// (wall-clock, milliseconds). Captured during preflight so we can
+	// observe the SSH-ping cost in the wild.
+	ResolveIPDurationMS int64 `json:"resolve_ip_duration_ms,omitempty"`
+
 	MirrorEnabled       bool     `json:"mirror_enabled"`
 	MirrorEndpoints     []string `json:"mirror_endpoints,omitempty"`
 	SkipMirrorEndpoints []string `json:"skip_mirror_endpoints,omitempty"`
@@ -323,17 +334,21 @@ func (m *Manager) GetMigrationSummary(deviceIP, targetURL, proxyURL string, opti
 
 	summary.PlannedConfig = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n" + string(xmlContent)
 
-	// 2b. Planned network config (hosts entries, resolv.conf preview, resolve error)
-	m.populatePlannedNetworkConfig(summary, deviceIP, targetURL)
+	// 2b. Planned network config (hosts entries, resolv.conf preview, resolve error).
+	// Pass an SSH client only when the probe succeeded — opening a fresh
+	// dial when we already know SSH is dead would burn ~handshake-timeout
+	// of wall time per refresh.
+	var resolveClient SSHClient
+	if probe.SSHOK && m.NewSSH != nil {
+		resolveClient = m.NewSSH(deviceIP)
+	}
+
+	m.populatePlannedNetworkConfig(summary, deviceIP, targetURL, resolveClient)
 
 	// 3. Provide HTTPS URL for testing (consumed by the migration UI)
 	summary.ServerHTTPSURL = m.buildServerHTTPSURL(targetURL)
 
-	// 4. Check if migrated (telnet axis uses the parallel preflight;
-	// XML/hosts/resolv axes use the probe data already gathered above).
-	m.checkIsMigratedFromProbe(summary, probe)
-
-	// 7. Mirroring settings
+	// 4. Mirroring settings
 	if m.DataStore != nil {
 		settings, err := m.DataStore.GetSettings()
 		if err == nil {
@@ -344,21 +359,26 @@ func (m *Manager) GetMigrationSummary(deviceIP, targetURL, proxyURL string, opti
 		}
 	}
 
-	// 8. Merge telnet preflight results (started in parallel at the top).
+	// 5. Merge telnet preflight results (started in parallel at the top).
 	telnetResult := <-telnetCh
 	summary.TelnetReachable = telnetResult.TelnetReachable
 	summary.TelnetBanner = telnetResult.TelnetBanner
 	summary.TelnetVerifiedConfig = telnetResult.TelnetVerifiedConfig
 	summary.TelnetProbeError = telnetResult.TelnetProbeError
 
-	// 9. Cross-check SSH-XML and telnet-getpdo readings; surface any
+	// 6. Check if migrated (must run after telnet results are merged so
+	// TelnetVerifiedConfig is populated). XML/hosts/resolv axes use the
+	// probe data already gathered above.
+	m.checkIsMigratedFromProbe(summary, probe)
+
+	// 7. Cross-check SSH-XML and telnet-getpdo readings; surface any
 	// divergence as a non-fatal warning.
 	m.crossCheckPreflights(summary)
 
 	return summary, nil
 }
 
-func (m *Manager) populatePlannedNetworkConfig(summary *MigrationSummary, _, targetURL string) {
+func (m *Manager) populatePlannedNetworkConfig(summary *MigrationSummary, _, targetURL string, sshClient SSHClient) {
 	parsedURL, err := url.Parse(targetURL)
 	if err != nil {
 		return
@@ -369,14 +389,42 @@ func (m *Manager) populatePlannedNetworkConfig(summary *MigrationSummary, _, tar
 		return
 	}
 
-	// Resolve locally only. The "from-device" lookup that resolveIP can
-	// do via SSH (`ping -c 1 host`) costs another fresh SSH handshake
-	// plus the ping's own runtime — easily 2–5 s on firmware-27 devices
-	// — and the result feeds only the PlannedResolv/PlannedHosts preview.
-	// For the actual apply paths (migrateViaHosts/migrateViaResolv) the
-	// device-side resolution is still used; this is only the preview.
-	hostIP, resolveErr := m.resolveIP(hostName, nil)
-	if resolveErr != nil {
+	// Resolve the target hostname. When the caller provides an SSH
+	// client (i.e. the speaker already answered the probe), we prefer
+	// the device-side lookup — it's authoritative for the actual
+	// network path the speaker will use. When SSH isn't available we
+	// fall back to service-side DNS and tag the result with
+	// ErrResolvedFromServiceOnly so the summary can render it as
+	// informational rather than as a hard error.
+	//
+	// Historical note: the SSH path was previously skipped here for
+	// cost reasons (a comment claimed "2–5 s extra per preflight
+	// refresh"). Measured 2026-05-16 across ST10 + ST20 on firmware
+	// 27.0.6.46330.5043500: ~290 ms ± 10 ms per resolve, three runs.
+	// Well under the original estimate — promoted to the default path.
+	// ResolveIPDurationMS stays on the summary so any regression
+	// (firmware upgrade, slower kex, etc.) is visible.
+	start := time.Now()
+	hostIP, resolveErr := m.resolveIP(hostName, sshClient)
+	summary.ResolveIPDurationMS = time.Since(start).Milliseconds()
+
+	switch {
+	case resolveErr == nil && hostIP != "":
+		summary.ResolveIPSource = "device"
+		if sshClient == nil {
+			// Caller didn't ask for the SSH path, and we got a clean
+			// answer — that only happens when host was already an IP
+			// literal. Source is neither "device" nor "service" in a
+			// meaningful sense; leave it empty.
+			summary.ResolveIPSource = ""
+		}
+	case errors.Is(resolveErr, ErrResolvedFromServiceOnly):
+		summary.ResolveIPSource = "service"
+		// Sentinel-tagged errors are informational — the resolved IP
+		// is still usable for the preview, the caller just shouldn't
+		// treat it as authoritative. We do NOT populate ResolveIPError
+		// here; the CLI/UI use that field for hard failures only.
+	case resolveErr != nil:
 		summary.ResolveIPError = resolveErr.Error()
 	}
 
@@ -2388,11 +2436,20 @@ func (m *Manager) GetResolvedIP(host string) string {
 	return ip
 }
 
+// ErrResolvedFromServiceOnly is returned (wrapped) by resolveIP when the
+// service-side DNS fallback produced an IP but the device-side SSH ping
+// either wasn't attempted or didn't yield a usable result. The error
+// carries the resolved IP — callers that don't need an authoritative
+// device-side answer (preview/summary builders) can errors.Is()-check
+// and treat the IP as informational. Apply-path callers that DO need
+// authoritative resolution can bail.
+var ErrResolvedFromServiceOnly = errors.New("resolved from service, not from device")
+
 // resolveIP resolves a hostname to an IP address.
 // It first tries to resolve from the device via SSH ping (authoritative for migration).
-// If that fails, it falls back to resolving from the service itself.
-// An error is returned whenever the SSH ping did not produce the IP, so callers that
-// write config to the device can abort rather than risk writing an unresolvable hostname.
+// If that fails, it falls back to resolving from the service itself, returning the
+// resolved IP wrapped with ErrResolvedFromServiceOnly so callers can distinguish
+// "authoritative device-side answer" from "best-effort service-side fallback".
 func (m *Manager) resolveIP(host string, client SSHClient) (string, error) {
 	if net.ParseIP(host) != nil {
 		return host, nil
@@ -2438,7 +2495,8 @@ func (m *Manager) resolveIP(host string, client SSHClient) (string, error) {
 		resolved = ips[0].String()
 	}
 
-	return resolved, fmt.Errorf("resolved %q to %s from service, not from device — result may be wrong if NAT or split-DNS is in use", host, resolved)
+	return resolved, fmt.Errorf("%w: %q → %s (NAT or split-DNS may differ from what the device would see)",
+		ErrResolvedFromServiceOnly, host, resolved)
 }
 
 // SyncDeviceData fetches presets, recents and sources from the device and saves them to the datastore.

pkg/service/setup/setup_test.go

@@ -2,6 +2,7 @@ package setup
 
 import (
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -735,6 +736,58 @@ func TestResolveIP(t *testing.T) {
 	}
 }
 
+// TestResolveIP_ServiceFallbackTagsSentinel pins the #282 fix:
+// service-side fallback returns the resolved IP wrapped with
+// ErrResolvedFromServiceOnly. Callers can errors.Is()-check the
+// sentinel to distinguish informational fallback from a hard
+// failure, which fixes the long-standing CLI/UI ❌ row that appeared
+// every time a hostname target was used with SSH off.
+func TestResolveIP_ServiceFallbackTagsSentinel(t *testing.T) {
+	m := &Manager{}
+
+	// No SSH client → forces the service-side fallback path.
+	ip, err := m.resolveIP("localhost", nil)
+	if ip != "127.0.0.1" && ip != "::1" {
+		t.Fatalf("expected localhost to resolve service-side, got ip=%q err=%v", ip, err)
+	}
+
+	if err == nil {
+		t.Fatalf("expected service-side fallback to surface ErrResolvedFromServiceOnly, got nil err")
+	}
+
+	if !errors.Is(err, ErrResolvedFromServiceOnly) {
+		t.Errorf("expected error to wrap ErrResolvedFromServiceOnly, got %v", err)
+	}
+}
+
+// TestResolveIP_DeviceSuccessReturnsNilError keeps the happy-path
+// guarantee explicit alongside the sentinel-tagging contract above.
+func TestResolveIP_DeviceSuccessReturnsNilError(t *testing.T) {
+	m := &Manager{}
+
+	mock := &mockSSH{
+		runFunc: func(command string) (string, error) {
+			if strings.Contains(command, "ping -c 1 myhost") {
+				return "PING myhost (10.0.0.5): 56 data bytes", nil
+			}
+			return "", nil
+		},
+	}
+
+	ip, err := m.resolveIP("myhost", mock)
+	if ip != "10.0.0.5" {
+		t.Errorf("expected 10.0.0.5, got %s", ip)
+	}
+
+	if err != nil {
+		t.Errorf("device-side success must return nil error, got %v", err)
+	}
+
+	if errors.Is(err, ErrResolvedFromServiceOnly) {
+		t.Errorf("device-side success must NOT carry ErrResolvedFromServiceOnly sentinel")
+	}
+}
+
 func TestMigrateViaHosts_SkipCAIfTrusted(t *testing.T) {
 	tempDir, err := os.MkdirTemp("", "setup-test-skip-ca")
 	if err != nil {