fix: batch E — remaining bugs + test improvements (BUG-21,27,30 + test fixes)

Author: DeliciousBuding

internal/client/browser.go

@@ -636,7 +636,9 @@ func (c *Client) ClaimUserTab(tabID string) (Tab, error) {
 	}
 	result.normalize()
 	// Auto-attach debugger so CDP commands work immediately
-	_ = c.attachTab(tabIDInt)
+	if err := c.attachTab(tabIDInt); err != nil && c.log != nil {
+		c.log.Printf("claim tab %d: auto-attach failed: %v", tabIDInt, err)
+	}
 	return result, nil
 }
 

internal/client/browser_rpc_test.go

@@ -413,9 +413,10 @@ func TestNavigateBackUsesHistoryEntry(t *testing.T) {
 	}
 
 	var navCall *recordedCall
-	for i, call := range rec.snapshot() {
+	snap := rec.snapshot()
+	for _, call := range snap {
 		if call.method == "executeCdp" && call.params["method"] == "Page.navigateToHistoryEntry" {
-			c := rec.snapshot()[i]
+			c := call
 			navCall = &c
 			break
 		}
@@ -631,6 +632,9 @@ func TestDomCUAClickComputesBoxCenter(t *testing.T) {
 		}
 		params, _ := req.Params.(map[string]interface{})
 		switch params["method"] {
+		case "DOM.resolveNode":
+			// DomCUAClick resolves node before getting box model
+			return map[string]interface{}{"object": map[string]interface{}{"objectId": "test"}}
 		case "DOM.getBoxModel":
 			return map[string]interface{}{
 				"model": map[string]interface{}{

internal/client/browser_test.go

@@ -35,7 +35,10 @@ func TestUserTabNormalize(t *testing.T) {
 }
 
 func TestNewUUIDFormat(t *testing.T) {
-	u := newUUID()
+	u, err := newUUID()
+	if err != nil {
+		t.Fatalf("newUUID: %v", err)
+	}
 	if len(u) != 36 {
 		t.Fatalf("UUID length = %d, want 36: %q", len(u), u)
 	}
@@ -56,7 +59,10 @@ func TestNewUUIDFormat(t *testing.T) {
 func TestNewUUIDUnique(t *testing.T) {
 	seen := make(map[string]struct{}, 1000)
 	for i := 0; i < 1000; i++ {
-		u := newUUID()
+		u, err := newUUID()
+		if err != nil {
+			t.Fatalf("newUUID iteration %d: %v", i, err)
+		}
 		if _, dup := seen[u]; dup {
 			t.Fatalf("duplicate UUID after %d iterations: %s", i, u)
 		}

internal/client/client.go

@@ -3,7 +3,7 @@ package client
 import (
 	"bufio"
 	"context"
-	"crypto/rand"
+	cryptoRand "crypto/rand"
 	"encoding/json"
 	"fmt"
 	"log"
@@ -12,6 +12,8 @@ import (
 	"sync/atomic"
 	"time"
 
+	"math/rand"
+
 	"github.com/DeliciousBuding/codex-browser-bridge/internal/discovery"
 	"github.com/DeliciousBuding/codex-browser-bridge/internal/protocol"
 )
@@ -114,12 +116,28 @@ func Connect(pipeName string, logger *log.Logger) (*Client, error) {
 // that establish the transport themselves.
 func NewFromConn(conn net.Conn, logger *log.Logger) *Client {
 	ctx, cancel := context.WithCancel(context.Background())
+
+	sessionID, err := newUUID()
+	if err != nil {
+		if logger != nil {
+			logger.Printf("newUUID failed, using fallback: %v", err)
+		}
+		sessionID = fallbackUUID()
+	}
+	turnID, err := newUUID()
+	if err != nil {
+		if logger != nil {
+			logger.Printf("newUUID failed, using fallback: %v", err)
+		}
+		turnID = fallbackUUID()
+	}
+
 	c := &Client{
 		conn:   conn,
 		reader: bufio.NewReaderSize(conn, 256*1024),
 		session: protocol.SessionParams{
-			SessionID: newUUID(),
-			TurnID:    newUUID(),
+			SessionID: sessionID,
+			TurnID:    turnID,
 		},
 		pending: make(map[int]chan *protocol.Response),
 		ctx:     ctx,
@@ -255,10 +273,22 @@ func truncate(s string, n int) string {
 }
 
 // newUUID generates a UUID v4 string without external dependencies.
-func newUUID() string {
+func newUUID() (string, error) {
+	var b [16]byte
+	if _, err := cryptoRand.Read(b[:]); err != nil {
+		return "", fmt.Errorf("crypto/rand failed: %w", err)
+	}
+	b[6] = (b[6] & 0x0f) | 0x40 // version 4
+	b[8] = (b[8] & 0x3f) | 0x80 // variant 10
+	return fmt.Sprintf("%08x-%04x-%04x-%04x-%012x",
+		b[0:4], b[4:6], b[6:8], b[8:10], b[10:16]), nil
+}
+
+// fallbackUUID generates a UUID v4 using math/rand when crypto/rand is unavailable.
+func fallbackUUID() string {
 	var b [16]byte
-	if _, err := rand.Read(b[:]); err != nil {
-		panic(fmt.Sprintf("crypto/rand failed: %v", err))
+	for i := range b {
+		b[i] = byte(rand.Intn(256))
 	}
 	b[6] = (b[6] & 0x0f) | 0x40 // version 4
 	b[8] = (b[8] & 0x3f) | 0x80 // variant 10

internal/client/e2e_test.go

@@ -1,6 +1,8 @@
 package client
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/DeliciousBuding/codex-browser-bridge/internal/protocol"
@@ -15,8 +17,14 @@ func TestCdpWithAttachSequence(t *testing.T) {
 	}{
 		{"Navigate", func(c *Client) error { return c.Navigate("1", "https://test.com") }},
 		{"Screenshot", func(c *Client) error {
-			_, err := c.Screenshot("1", false)
-			return err
+			b64, err := c.Screenshot("1", false)
+			if err != nil {
+				return err
+			}
+			if b64 == "" {
+				return fmt.Errorf("expected non-empty screenshot data")
+			}
+			return nil
 		}},
 		{"Evaluate", func(c *Client) error {
 			_, err := c.Evaluate("1", "1+1")
@@ -30,6 +38,7 @@ func TestCdpWithAttachSequence(t *testing.T) {
 				if req.Method == "executeCdp" {
 					return map[string]interface{}{
 						"result": map[string]interface{}{"value": "ok"},
+						"data":   "iVBORw0KGgo...",
 					}
 				}
 				return map[string]bool{"ok": true}
@@ -99,7 +108,7 @@ func TestWaitForLoadTimeout(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected timeout error, got state=%q", state)
 	}
-	if err.Error() == "" || err.Error()[:7] != "timed o" {
+	if err.Error() == "" || !strings.HasPrefix(err.Error(), "timed o") {
 		t.Errorf("error = %v, should mention timeout", err)
 	}
 }

internal/client/rpc_test.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"bufio"
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"net"
@@ -228,3 +229,65 @@ func contains(s, substr string) bool {
 	}
 	return false
 }
+
+// TestSendNotificationFrame verifies that SendNotification encodes a
+// valid JSON-RPC notification frame: no "id" field, correct length prefix,
+// method and params properly encoded.
+func TestSendNotificationFrame(t *testing.T) {
+	clientConn, serverConn := net.Pipe()
+	defer clientConn.Close()
+	defer serverConn.Close()
+
+	c := NewFromConn(clientConn, nil)
+	defer c.Close()
+
+	serverReader := bufio.NewReader(serverConn)
+
+	type notifMsg struct {
+		JSONRPC string      `json:"jsonrpc"`
+		Method  string      `json:"method"`
+		Params  interface{} `json:"params,omitempty"`
+	}
+
+	var got notifMsg
+	errCh := make(chan error, 1)
+	go func() {
+		raw, err := protocol.DecodeFrame(serverReader)
+		if err != nil {
+			errCh <- fmt.Errorf("decode frame: %w", err)
+			return
+		}
+		// Verify no "id" field in raw JSON (JSON-RPC notification)
+		if bytes.Contains(raw, []byte(`"id"`)) {
+			errCh <- fmt.Errorf("notification frame contains 'id' field: %s", raw)
+			return
+		}
+		if err := json.Unmarshal(raw, &got); err != nil {
+			errCh <- fmt.Errorf("unmarshal: %w", err)
+			return
+		}
+		errCh <- nil
+	}()
+
+	err := c.SendNotification("test.event", map[string]string{"hello": "world"})
+	if err != nil {
+		t.Fatalf("SendNotification: %v", err)
+	}
+
+	if err := <-errCh; err != nil {
+		t.Fatal(err)
+	}
+	if got.JSONRPC != "2.0" {
+		t.Errorf("jsonrpc = %q, want \"2.0\"", got.JSONRPC)
+	}
+	if got.Method != "test.event" {
+		t.Errorf("method = %q, want \"test.event\"", got.Method)
+	}
+	paramsMap, ok := got.Params.(map[string]interface{})
+	if !ok {
+		t.Fatalf("params is %T, want map", got.Params)
+	}
+	if v, ok := paramsMap["hello"].(string); !ok || v != "world" {
+		t.Errorf("params[hello] = %v, want \"world\"", paramsMap["hello"])
+	}
+}