fix(setup): read the URLs back when a telnet command is not confirmed

A rejected or unrecognised command response aborted the sequence with
"read back and reconcile all four URL fields before rebooting", leaving the
user to do by hand what the service can do in one read-only command.

That advice also assumes the write failed, which the reply shape does not
prove. A telnet console is a shared stream and firmware echoes vary: an
interleaved log line, a normalised URL, or a banner arriving late all produce
a response the parser does not recognise, for a write that landed. This is the
same class as the earlier parser reporting HTTP 500 when all four writes had
succeeded, just narrower.

The abort itself is kept, so no further write is sent and a rejected sequence
cannot spread. Before returning, `getpdo CurrentSystemConfiguration` now runs
and the reported error carries what the device actually holds. Nothing is
claimed to have succeeded on ambiguous evidence; the user simply gets the
evidence.

The two tests asserting that nothing at all follows a rejection now assert the
property that matters, that no further command CHANGES the device, since a
read-only read-back does not.

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

pkg/service/setup/telnet_migration.go

@@ -312,8 +312,20 @@ func (m *Manager) migrateViaTelnet(deviceIP string, urls telnetURLs) (string, er
 		fmt.Fprintf(&logs, "→ %s\n%s\n", cmd, strings.TrimRight(resp, "\r\n"))
 
 		if err := validateTelnetMutationResponse(cmd, resp, persistenceCommand, urls); err != nil {
-			return logs.String(), fmt.Errorf("%w; %s", err,
-				telnetWriteFailureContext(runtimeWrites, persistenceCommand))
+			// Aborting here is deliberate: no further writes are sent, so a
+			// rejected sequence cannot spread. But the advice this produces is
+			// "read back and reconcile all four URL fields", which the service
+			// can simply do, and must, because an unrecognised reply is not
+			// proof the write failed. A telnet console is a shared stream and
+			// firmware echoes vary, so the readback is better evidence than
+			// the reply shape. It is read-only and changes nothing.
+			// Read back before snapshotting logs: the return values are
+			// evaluated left to right, so logs.String() inline would capture
+			// the builder before the read-back appends to it.
+			readBack := telnetReadBackSummary(t, &logs)
+
+			return logs.String(), fmt.Errorf("%w; %s%s", err,
+				telnetWriteFailureContext(runtimeWrites, persistenceCommand), readBack)
 		}
 
 		if !persistenceCommand {
@@ -401,6 +413,40 @@ func hasTelnetPersistenceConfirmation(response, expected string) bool {
 		len(meaningful) == 2 && meaningful[0] == expected && meaningful[1] == "OK"
 }
 
+// telnetReadBackSummary reads the live URL configuration so a failure report
+// says what the device actually holds now, instead of asking the user to go
+// and find out. Returns an empty string when the readback itself fails, since
+// there is then nothing trustworthy to add.
+func telnetReadBackSummary(t TelnetClient, logs *strings.Builder) string {
+	verify, err := t.SendCommand("getpdo CurrentSystemConfiguration")
+	if err != nil {
+		fmt.Fprintf(logs, "Read-back after the failure also failed: %v\n", err)
+
+		return ""
+	}
+
+	fmt.Fprintf(logs, "→ getpdo CurrentSystemConfiguration (read-back after the failure)\n%s\n",
+		strings.TrimRight(verify, "\r\n"))
+
+	current := parseGetpdoConfig(verify)
+	if len(current) == 0 {
+		return ""
+	}
+
+	fields := make([]string, 0, len(current))
+	for _, field := range canonicalBoseTelnetURLs().fields() {
+		if value, ok := current[field.configName]; ok && value != "" {
+			fields = append(fields, field.configName+"="+value)
+		}
+	}
+
+	if len(fields) == 0 {
+		return ""
+	}
+
+	return "; the device currently reports " + strings.Join(fields, ", ")
+}
+
 func telnetWriteFailureContext(runtimeWrites int, persistenceAttempted bool) string {
 	if persistenceAttempted {
 		return "all four runtime URL writes were confirmed, but the persistence outcome is uncertain; " +

pkg/service/setup/telnet_migration_test.go

@@ -270,13 +270,7 @@ func TestMigrateViaTelnet_CommandNotFoundAborts(t *testing.T) {
 		t.Errorf("err = %v, want to mention the rejected command", err)
 	}
 
-	// The verification command must NOT have been sent — the run aborts on
-	// the first rejection.
-	for _, c := range f.commands {
-		if c == "getpdo CurrentSystemConfiguration" {
-			t.Errorf("verification was sent after a rejected command: %v", f.commands)
-		}
-	}
+	assertNoTelnetWritesAfterRejection(t, f.commands, "envswitch")
 }
 
 func TestMigrateViaTelnet_GenericRuntimeRejectionReportsPartialState(t *testing.T) {
@@ -322,9 +316,30 @@ func TestMigrateViaTelnet_EnvswitchRejectionReportsUncertainPersistence(t *testi
 		t.Errorf("err = %v, want uncertain-persistence classification", err)
 	}
 
-	for _, command := range f.commands {
-		if command == "getpdo CurrentSystemConfiguration" {
-			t.Errorf("verification was sent after unconfirmed envswitch response: %v", f.commands)
+	assertNoTelnetWritesAfterRejection(t, f.commands, "envswitch")
+}
+
+// assertNoTelnetWritesAfterRejection checks the property that matters once a
+// command is rejected: no further command may CHANGE the device. The
+// read-only getpdo read-back is expected, since the failure report says what
+// the device actually holds rather than telling the user to go and find out.
+func assertNoTelnetWritesAfterRejection(t *testing.T, commands []string, rejected string) {
+	t.Helper()
+
+	seenRejected := false
+	for _, command := range commands {
+		if strings.Contains(command, rejected) {
+			seenRejected = true
+
+			continue
+		}
+
+		if !seenRejected {
+			continue
+		}
+
+		if strings.HasPrefix(command, "sys configuration") || strings.HasPrefix(command, "envswitch") {
+			t.Errorf("a write was sent after the rejected command: %v", commands)
 		}
 	}
 }
@@ -499,3 +514,32 @@ func TestTelnetRevertNotOfferedOnUnmigratedSpeakers(t *testing.T) {
 		})
 	}
 }
+
+// TestMigrateViaTelnet_FailureReportsWhatTheDeviceHolds: the failure advice is
+// "read back and reconcile all four URL fields", which the service can do
+// itself. An unrecognised reply is not proof the write failed, so the readback
+// is better evidence than the reply shape.
+func TestMigrateViaTelnet_FailureReportsWhatTheDeviceHolds(t *testing.T) {
+	target := "http://example:8000"
+	urls := defaultTelnetURLs(target)
+	resp := happyResponses(target)
+	resp["envswitch boseurls set "+urls.Marge+" "+urls.SwUpdate] = "something unfamiliar\n"
+
+	f := &fakeTelnet{responses: resp}
+	m := newFakeTelnetManager(f)
+
+	logs, err := m.migrateViaTelnet("192.0.2.1", urls)
+	if err == nil {
+		t.Fatal("expected an error for an unrecognised envswitch response")
+	}
+
+	if !strings.Contains(err.Error(), "the device currently reports") {
+		t.Errorf("err = %v, want it to carry the live URL state", err)
+	}
+	if !strings.Contains(err.Error(), "margeServerUrl="+target) {
+		t.Errorf("err = %v, want the actual margeServerUrl value", err)
+	}
+	if !strings.Contains(logs, "read-back after the failure") {
+		t.Errorf("logs did not record the read-back:\n%s", logs)
+	}
+}