fix(management): retry transient Devin OAuth callback read errors

internal/api/handlers/management/auth_files_devin_oauth.go

@@ -136,6 +136,9 @@ func (h *Handler) completeDevinOAuth(ctx context.Context, authDir, state, codeVe
 	log.Info("Devin authentication successful")
 }
 
+// readDevinOAuthCallbackFile is a variable so tests can simulate read errors.
+var readDevinOAuthCallbackFile = os.ReadFile
+
 func waitDevinOAuthCallback(ctx context.Context, path, state string) (*oauthCallbackFilePayload, error) {
 	ticker := time.NewTicker(500 * time.Millisecond)
 	defer ticker.Stop()
@@ -146,17 +149,18 @@ func waitDevinOAuthCallback(ctx context.Context, path, state string) (*oauthCall
 		if ctx.Err() != nil {
 			return nil, errors.New("Timeout waiting for OAuth callback")
 		}
-		data, errRead := os.ReadFile(path)
+		data, errRead := readDevinOAuthCallbackFile(path)
 		if errRead == nil {
 			var result oauthCallbackFilePayload
 			if errDecode := json.Unmarshal(data, &result); errDecode != nil {
 				return nil, errors.New("Invalid OAuth callback")
 			}
 			return &result, nil
 		}
-		if !errors.Is(errRead, os.ErrNotExist) {
-			return nil, errors.New("Failed to read OAuth callback")
-		}
+		// Retry on any read error, like the other OAuth flows do. The file may not
+		// exist yet, or it may be briefly unreadable: on Windows a read that lands
+		// while the callback handler renames the file into place fails with a
+		// sharing violation. The context deadline and session state bound the wait.
 		select {
 		case <-ctx.Done():
 			return nil, errors.New("Timeout waiting for OAuth callback")

internal/api/handlers/management/auth_files_devin_oauth_test.go

@@ -21,6 +21,11 @@ import (
 	coreauth "github.com/router-for-me/CLIProxyAPI/v8/sdk/cliproxy/auth"
 )
 
+// devinOAuthTestWait is a failure bound, not an expected latency: the flow
+// checks for its callback file every 500ms and saves the credential to disk,
+// and both stall for seconds on a loaded host.
+const devinOAuthTestWait = 15 * time.Second
+
 type fakeDevinOAuthService struct {
 	exchange func(context.Context, string, string) (string, error)
 	create   func(context.Context, string) (*coreauth.Auth, error)
@@ -122,11 +127,14 @@ func TestDevinRemoteOAuthFlow(t *testing.T) {
 		if base64.RawURLEncoding.EncodeToString(digest[:]) != query.Get("code_challenge") {
 			t.Fatal("PKCE challenge does not match verifier")
 		}
-	case <-time.After(5 * time.Second):
-		t.Fatal("token exchange did not start")
+	case <-time.After(devinOAuthTestWait):
+		// Report the session status: it carries the reason when the flow gave up.
+		w = httptest.NewRecorder()
+		router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/get-auth-status?state="+start.State, nil))
+		t.Fatalf("token exchange did not start, session status: %s", w.Body.String())
 	}
 
-	deadline := time.NewTimer(5 * time.Second)
+	deadline := time.NewTimer(devinOAuthTestWait)
 	defer deadline.Stop()
 	ticker := time.NewTicker(10 * time.Millisecond)
 	defer ticker.Stop()
@@ -254,3 +262,33 @@ func TestWaitDevinOAuthCallbackExpiredContext(t *testing.T) {
 		t.Fatalf("error = %v", errWait)
 	}
 }
+
+func TestWaitDevinOAuthCallbackRetriesTransientReadError(t *testing.T) {
+	const state = "devin-transient-read-error"
+	RegisterOAuthSession(state, "devin")
+	defer CompleteOAuthSession(state)
+
+	// On Windows a read that lands while the callback handler renames the file
+	// into place fails with a sharing violation. The next read succeeds.
+	reads := 0
+	originalRead := readDevinOAuthCallbackFile
+	readDevinOAuthCallbackFile = func(string) ([]byte, error) {
+		reads++
+		if reads == 1 {
+			return nil, errors.New("the process cannot access the file because it is being used by another process")
+		}
+		return []byte(`{"state":"` + state + `","code":"code"}`), nil
+	}
+	t.Cleanup(func() { readDevinOAuthCallbackFile = originalRead })
+
+	result, errWait := waitDevinOAuthCallback(context.Background(), filepath.Join(t.TempDir(), "callback.oauth"), state)
+	if errWait != nil {
+		t.Fatalf("transient read error ended the login: %v", errWait)
+	}
+	if result.Code != "code" || result.State != state {
+		t.Fatalf("unexpected callback: %+v", result)
+	}
+	if reads != 2 {
+		t.Fatalf("reads = %d, want 2", reads)
+	}
+}