fix(modem): prevent AT command response desynchronization

internal/device/cell_lock.go

@@ -226,18 +226,13 @@ func (manager *Manager) prepareCellLock(ctx context.Context, id string) (*manage
 	return state, queries, nil
 }
 
-// Called with opMu held; discard the client on missing finals to isolate late replies.
 func (manager *Manager) cellLockCommand(ctx context.Context, state *managedDevice, command string) (modem.Response, error) {
 	if err := ctx.Err(); err != nil {
 		return modem.Response{}, err
 	}
 	response, err := manager.command(ctx, state.client, command)
-	if response.Final == "" {
-		_ = state.client.Close()
-		state.client = nil
-		if err == nil {
-			err = fmt.Errorf("%s: modem command completed without a final result", command)
-		}
+	if response.Final == "" && err == nil {
+		err = fmt.Errorf("%s: modem command completed without a final result", command)
 	}
 	return response, err
 }

internal/device/cell_lock_test.go

@@ -84,10 +84,9 @@ func TestSetCellLockFailures(t *testing.T) {
 	for _, test := range []struct {
 		name   string
 		setter clientStep
-		closes int
 	}{
 		{name: "setter rejected", setter: rejectedCommand(command)},
-		{name: "missing final", setter: clientStep{command: command, err: context.DeadlineExceeded}, closes: 1},
+		{name: "missing final", setter: clientStep{command: command, err: context.DeadlineExceeded}},
 		{name: "readback mismatch", setter: clientStep{command: command, response: okResponse()}},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -108,8 +107,11 @@ func TestSetCellLockFailures(t *testing.T) {
 			if test.setter.err != nil && !errors.Is(err, test.setter.err) {
 				t.Fatalf("lost modem error: %v", err)
 			}
-			if client.closeCount != test.closes {
-				t.Fatalf("client closed %d times, want %d", client.closeCount, test.closes)
+			if client.closeCount != 0 {
+				t.Fatalf("client closed %d times, want 0", client.closeCount)
+			}
+			if manager.devices[id].client != client {
+				t.Fatal("client not retained after unconfirmed write")
 			}
 			client.assertDone(t)
 		})

internal/device/cellular_ims.go

@@ -186,10 +186,12 @@ func (manager *Manager) SetCellularIMS(ctx context.Context, id string, mode Cell
 	rebootCtx, cancel := manager.withTimeout(ctx, manager.longTimeout)
 	_, err = client.Execute(rebootCtx, "AT+CFUN=1,1")
 	cancel()
-	if closeErr := client.Close(); err == nil {
-		err = closeErr
+	if !isCommandInterrupted(err) {
+		if closeErr := client.Close(); err == nil {
+			err = closeErr
+		}
+		state.client = nil
 	}
-	state.client = nil
 	state.preFlightMode = nil
 	manager.clearSnapshot(id, state)
 	manager.setResult(id, state, nil, err)

internal/device/cellular_ims_test.go

@@ -2,6 +2,7 @@ package device
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"vocat/internal/modem"
@@ -49,22 +50,33 @@ func TestParseCellularCSRegistration(t *testing.T) {
 }
 
 func TestSetCellularIMSEnablesAndRebootsOnlyOnce(t *testing.T) {
-	client := &transcriptClient{steps: []clientStep{
-		{command: `AT+QCFG="ims"`, response: modem.Response{Lines: []string{`+QCFG: "ims",0,0`}, Final: "OK"}},
-		{command: "AT+CFUN?", response: okResponse("+CFUN: 1")},
-		{command: `AT+QCFG="ims",1`, response: modem.Response{Final: "OK"}},
-		{command: `AT+QCFG="ims"`, response: modem.Response{Lines: []string{`+QCFG: "ims",1,0`}, Final: "OK"}},
-		{command: "AT+CFUN=1,1", response: modem.Response{Final: "OK"}},
-	}}
-	manager, id := newStartedTestManager(t, client)
-	status, err := manager.SetCellularIMS(context.Background(), id, CellularIMSModeForceEnabled)
-	if err != nil || !status.Configured || !status.Changed || !status.Rebooting {
-		t.Fatalf("SetCellularIMS = %+v, %v", status, err)
+	for name, rebootErr := range map[string]error{"success": nil, "timeout": modem.ErrCommandTimeout, "canceled": context.Canceled} {
+		t.Run(name, func(t *testing.T) {
+			client := &transcriptClient{steps: []clientStep{
+				{command: `AT+QCFG="ims"`, response: modem.Response{Lines: []string{`+QCFG: "ims",0,0`}, Final: "OK"}},
+				{command: "AT+CFUN?", response: okResponse("+CFUN: 1")},
+				{command: `AT+QCFG="ims",1`, response: modem.Response{Final: "OK"}},
+				{command: `AT+QCFG="ims"`, response: modem.Response{Lines: []string{`+QCFG: "ims",1,0`}, Final: "OK"}},
+				{command: "AT+CFUN=1,1", response: modem.Response{Final: "OK"}, err: rebootErr},
+			}}
+			manager, id := newStartedTestManager(t, client)
+			status, err := manager.SetCellularIMS(context.Background(), id, CellularIMSModeForceEnabled)
+			if !errors.Is(err, rebootErr) || !status.Configured || !status.Changed || !status.Rebooting {
+				t.Fatalf("SetCellularIMS = %+v, %v", status, err)
+			}
+			wantCloses := 1
+			if rebootErr != nil {
+				wantCloses = 0
+				if manager.devices[id].client != client {
+					t.Fatal("IMS restart discarded the session before its final")
+				}
+			}
+			if client.closeCount != wantCloses {
+				t.Fatalf("close count = %d, want %d", client.closeCount, wantCloses)
+			}
+			client.assertDone(t)
+		})
 	}
-	if client.closeCount != 1 {
-		t.Fatalf("close count = %d, want 1", client.closeCount)
-	}
-	client.assertDone(t)
 }
 
 func TestSetCellularIMSNoopDoesNotReboot(t *testing.T) {

internal/device/manager.go

@@ -880,16 +880,24 @@ func (manager *Manager) Reboot(ctx context.Context, id string) error {
 	commandCtx, cancel := manager.withTimeout(ctx, manager.longTimeout)
 	defer cancel()
 	_, err = client.Execute(commandCtx, "AT+CFUN=1,1")
-	if closeErr := client.Close(); err == nil {
-		err = closeErr
+	if !isCommandInterrupted(err) {
+		if closeErr := client.Close(); err == nil {
+			err = closeErr
+		}
+		state.client = nil
 	}
-	state.client = nil
 	state.preFlightMode = nil
 	manager.clearSnapshot(id, state)
 	manager.setResult(id, state, nil, err)
 	return err
 }
 
+// Interrupted commands still own their late replies; reopening the port would
+// let those replies complete an unrelated command in the new session.
+func isCommandInterrupted(err error) bool {
+	return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, modem.ErrCommandTimeout)
+}
+
 // softResetForProfileSwitch resets the baseband SIM stack using a soft CFUN sequence
 // (AT+CFUN=0 -> AT+CFUN=4) instead of rebooting the entire hardware module (AT+CFUN=1,1).
 // This causes the baseband to reload the new eSIM profile files within ~1-2 seconds
@@ -927,7 +935,7 @@ func (manager *Manager) softResetSIMLocked(ctx context.Context, id string, state
 	}
 	if _, err := client.Execute(commandCtx, "AT+CFUN=0"); err != nil {
 		// 超时或取消时不能确认 CFUN=0 是否已生效，仍尝试恢复到 RF 关闭的工作模式。
-		if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, modem.ErrCommandTimeout) {
+		if isCommandInterrupted(err) {
 			reloadCtx, cancelReload := context.WithTimeout(context.WithoutCancel(ctx), manager.commandTimeout)
 			_, reloadErr := client.Execute(reloadCtx, "AT+CFUN=4")
 			cancelReload()

internal/device/mbn.go

@@ -429,8 +429,10 @@ func (manager *Manager) ReconcileEC20MBNAfterProfileSwitch(ctx context.Context,
 	rebootContext, cancelReboot := context.WithTimeout(ctx, manager.longTimeout)
 	_, rebootErr := client.Execute(rebootContext, "AT+CFUN=1,1")
 	cancelReboot()
-	_ = client.Close()
-	state.client = nil
+	if !isCommandInterrupted(rebootErr) {
+		_ = client.Close()
+		state.client = nil
+	}
 	state.preFlightMode = nil
 	manager.clearSnapshot(id, state)
 	state.opMu.Unlock()

internal/device/sms.go

@@ -131,15 +131,6 @@ func (manager *Manager) SendSMS(
 		default:
 			result.SubmissionStatus = "unknown"
 		}
-		if errors.Is(submitErr, modem.ErrCommandTimeout) ||
-			errors.Is(submitErr, context.Canceled) ||
-			errors.Is(submitErr, context.DeadlineExceeded) {
-			// A timeout after Ctrl-Z has an inherently uncertain outcome. Close
-			// this session so a late +CMGS/OK cannot corrupt the next command;
-			// callers must decide whether it is safe to retry.
-			_ = client.Close()
-			state.client = nil
-		}
 		partErr := fmt.Errorf(
 			"submit SMS part %d/%d: %w",
 			part.part,

internal/device/sms_test.go

@@ -260,8 +260,11 @@ func TestManagerSendSMSTimeoutNeverClaimsAcceptanceOrDelivery(t *testing.T) {
 	client.mu.Lock()
 	closeCount := client.closeCount
 	client.mu.Unlock()
-	if closeCount != 1 {
-		t.Fatalf("close count = %d, want 1 after uncertain timeout", closeCount)
+	if closeCount != 0 {
+		t.Fatalf("close count = %d, want 0 after uncertain timeout", closeCount)
+	}
+	if manager.devices[id].client != client {
+		t.Fatal("client not retained after uncertain timeout")
 	}
 	client.assertDone(t)
 }

internal/device/snapshot.go

@@ -50,8 +50,8 @@ func (manager *Manager) readSnapshot(
 	// Native MHI/QMI devices expose their immutable modem identity through DMS.
 	// Read it before any SIM-dependent AT probes: a missing/bad card can make
 	// those commands slow or fail, but must never prevent IMEI from appearing.
-	if strings.EqualFold(strings.TrimSpace(backend), "qmi") && isNativeQMICandidate(candidate) {
-		qmiContext, cancelQMI := manager.withTimeout(ctx, manager.commandTimeout*5)
+	if isNativeQMICandidate(candidate) {
+		qmiContext, cancelQMI := context.WithTimeout(ctx, manager.commandTimeout)
 		qmiIMEI, qmiErr := manager.readNativeQMIIMEI(qmiContext, candidate)
 		cancelQMI()
 		if qmiErr == nil {
@@ -252,12 +252,9 @@ func (manager *Manager) readSnapshot(
 		snapshot.RegistrationStatus = 1
 		snapshot.RegistrationSource = "COPS"
 	}
-	if snapshot.IMEI == "" {
-		// AT+CGSN on some MHI modems (the UFI dongle behind the OpenStick 410)
-		// returns the IMEI line but never a final OK, so it would block until the
-		// caller's deadline (30s during a periodic refresh) and starve every other
-		// device operation behind the lock. Give it an independent short timeout
-		// and let the WWAN transport's drain discard the trailing stale bytes.
+	// Some native modems never finish CGSN. Do not leave the AT session
+	// waiting for its final; native QMI identity is read through DMS instead.
+	if snapshot.IMEI == "" && !isNativeQMICandidate(candidate) {
 		for _, command := range []string{"AT+CGSN", "AT+CGSN=1"} {
 			cgsnCtx, cancelCGSN := context.WithTimeout(ctx, manager.commandTimeout)
 			cgsnResponse, cgsnErr := manager.command(cgsnCtx, client, command)
@@ -270,16 +267,6 @@ func (manager *Manager) readSnapshot(
 			}
 		}
 	}
-	if snapshot.IMEI == "" && strings.EqualFold(strings.TrimSpace(backend), "qmi") && isNativeQMICandidate(candidate) {
-		qmiContext, cancelQMI := manager.withTimeout(ctx, manager.commandTimeout*5)
-		qmiIMEI, qmiErr := manager.readNativeQMIIMEI(qmiContext, candidate)
-		cancelQMI()
-		if qmiErr == nil {
-			snapshot.IMEI = qmiIMEI
-		} else {
-			snapshot.Warnings = append(snapshot.Warnings, "read IMEI via QMI DMS: "+qmiErr.Error())
-		}
-	}
 	if snapshot.IMEI == "" && previousSnapshot != nil {
 		// IMEI is hardware identity and does not change with the inserted card.
 		// Preserve a prior successful read across a transient QMI/AT failure.

internal/device/snapshot_guard_test.go

@@ -57,10 +57,66 @@ func (c *lenientATClient) saw(command string) bool {
 	return false
 }
 
-// AT+CGSN on some MHI modems returns the IMEI line but never a final OK, so it
-// would block until the caller's deadline and hold the device lock for the
-// whole periodic refresh. The snapshot must bound CGSN with its own short
-// timeout instead of inheriting the refresh deadline.
+func TestSnapshotSkipsCGSNOnlyForNativeQMI(t *testing.T) {
+	const commandTimeout = 20 * time.Millisecond
+	for _, test := range []struct {
+		name, id, backend string
+		qmiErr            error
+	}{
+		{name: "native AT", id: "mhi-wwan0", backend: "at"},
+		{name: "native QMI", id: "mhi-wwan0", backend: "qmi"},
+		{name: "native AT failure", id: "mhi-wwan0", backend: "at", qmiErr: errors.New("DMS unavailable")},
+		{name: "native QMI failure", id: "mhi-wwan0", backend: "qmi", qmiErr: errors.New("DMS unavailable")},
+		{name: "native AT timeout", id: "mhi-wwan0", backend: "at", qmiErr: context.DeadlineExceeded},
+		{name: "native QMI timeout", id: "mhi-wwan0", backend: "qmi", qmiErr: context.DeadlineExceeded},
+		{name: "USB AT", id: "usb-ec20", backend: "at"},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+			defer cancel()
+			candidate := modem.Candidate{ID: test.id, QMIControl: "/dev/wwan0qmi0"}
+			client := &lenientATClient{}
+			qmiCalls := 0
+			manager := &Manager{commandTimeout: commandTimeout, qmiRadioOpener: func(ctx context.Context, _ string) (qmiRadioSession, error) {
+				qmiCalls++
+				if deadline, ok := ctx.Deadline(); !ok || time.Until(deadline) > commandTimeout {
+					t.Fatal("DMS probe deadline exceeds CommandTimeout")
+				}
+				if test.qmiErr == context.DeadlineExceeded {
+					<-ctx.Done()
+					return nil, ctx.Err()
+				}
+				return &fakeQMIRadioSession{imei: "866241014372802", imeiErr: test.qmiErr}, nil
+			}}
+			previous := &Snapshot{IMEI: "866241014372801"}
+			snapshot, err := manager.readSnapshot(ctx, test.id, candidate, test.backend, "", previous, client)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !client.saw("AT+CPIN?") || ctx.Err() != nil {
+				t.Fatalf("CPIN did not continue with a live parent: commands = %v, error = %v", client.commands, ctx.Err())
+			}
+			if test.qmiErr != nil && !strings.Contains(strings.Join(snapshot.Warnings, "\n"), test.qmiErr.Error()) {
+				t.Fatalf("warnings = %v, want DMS error %v", snapshot.Warnings, test.qmiErr)
+			}
+			wantCGSN := test.id == "usb-ec20"
+			if client.saw("AT+CGSN") != wantCGSN || client.saw("AT+CGSN=1") != wantCGSN {
+				t.Fatalf("commands = %v, want CGSN probes = %t", client.commands, wantCGSN)
+			}
+			wantCalls, wantIMEI := 0, previous.IMEI
+			if !wantCGSN {
+				wantCalls = 1
+				if test.qmiErr == nil {
+					wantIMEI = "866241014372802"
+				}
+			}
+			if qmiCalls != wantCalls || snapshot.IMEI != wantIMEI {
+				t.Fatalf("DMS calls = %d, IMEI = %q; want %d, %q", qmiCalls, snapshot.IMEI, wantCalls, wantIMEI)
+			}
+		})
+	}
+}
+
 func TestManagerRefreshBoundsCGSNTimeout(t *testing.T) {
 	client := &lenientATClient{cgsnDelay: 5 * time.Second}
 	manager, id := newStartedTestManager(t, client)
@@ -92,9 +148,6 @@ func TestManagerRefreshBoundsCGSNTimeout(t *testing.T) {
 // card that call blocks until its long timeout and starves the AT terminal
 // behind the device lock.
 func TestManagerRefreshSkipsQMIICCIDWithoutReadySIM(t *testing.T) {
-	// CGSN succeeds so the snapshot does not fall back to the QMI DMS IMEI
-	// read either; the test focuses on the UIM ICCID fallback being skipped
-	// without a READY card.
 	client := &lenientATClient{cgsnIMEI: "866241014372802"}
 	manager, err := NewManager(Options{
 		Discoverer: staticDiscoverer{candidates: []modem.Candidate{{
@@ -127,12 +180,13 @@ func TestManagerRefreshSkipsQMIICCIDWithoutReadySIM(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Refresh: %v", err)
 	}
-	// Exactly one QMI open is expected: the immutable DMS IMEI read runs
-	// unconditionally for native QMI candidates (IMEI is hardware identity,
-	// independent of the card). The UIM ICCID fallback, which would block
-	// without a READY SIM, must be skipped.
+	// Only the SIM-independent DMS read is attempted; the UIM ICCID fallback
+	// must still be skipped without a READY SIM.
 	if qmiCalls != 1 {
-		t.Fatalf("qmiRadioOpener called %d times, want 1 (DMS IMEI only, UIM ICCID must be skipped without a READY SIM)", qmiCalls)
+		t.Fatalf("qmiRadioOpener called %d times, want 1 DMS attempt and no UIM ICCID read", qmiCalls)
+	}
+	if client.saw("AT+CGSN") || client.saw("AT+CGSN=1") || snapshot.IMEI != "" {
+		t.Fatalf("native IMEI fell back to AT: %q, commands = %v", snapshot.IMEI, client.commands)
 	}
 	for _, warning := range snapshot.Warnings {
 		if strings.Contains(warning, "QMI UIM") {