fix(sdk/go): address principal engineer review findings

- Remove dead boolCount function that would fail golangci-lint (#1)
- Emit EventAdded for the first watch event instead of EventModified,
  matching k8s watch semantics (#7)
- Add mutex locking to all mock server methods that access the shared
  sandboxes map, fixing latent race conditions (#12)
- Skip HealthCheck integration test that calls an unimplemented stub (#13)
- Scope doc.go examples: mark sections for sub-clients not yet available
  in this PR with "available in a future release" (#4)
- Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields
  as reserved for future use (#2, #6)

Signed-off-by: Roland Huß <rhuss@redhat.com>

Author: rhuss

sdk/go/openshell/v1/doc.go

@@ -34,7 +34,7 @@
 //	    log.Fatal(err)
 //	}
 //
-// # Command Execution
+// # Command Execution (available in a future release)
 //
 //	result, err := client.Exec().Run(ctx, sandbox.Name, []string{"echo", "hello"}, v1.ExecOptions{})
 //	if err != nil {
@@ -76,7 +76,7 @@
 //	}
 //	// channel closes automatically after Ready or Error
 //
-// # Service Exposure
+// # Service Exposure (available in a future release)
 //
 // Expose an HTTP service running inside a sandbox and retrieve its public URL:
 //
@@ -94,7 +94,7 @@
 //	    fmt.Printf("  %s → port %d (URL: %s)\n", ep.ServiceName, ep.TargetPort, ep.URL)
 //	}
 //
-// # Provider Profiles
+// # Provider Profiles (available in a future release)
 //
 // List available provider profiles and import new ones:
 //
@@ -119,7 +119,7 @@
 //	    fmt.Printf("[%s] %s: %s\n", d.Severity, d.Field, d.Message)
 //	}
 //
-// # Credential Refresh
+// # Credential Refresh (available in a future release)
 //
 // Configure gateway-owned credential refresh for a provider:
 //
@@ -192,7 +192,7 @@
 //	    "x-proxy-key": "proxy-secret",
 //	})
 //
-// # SSH Session Management
+// # SSH Session Management (available in a future release)
 //
 // Create an SSH session for a sandbox and use the returned connection details.
 // Note: CreateSession accepts a sandbox ID, not a name. For name-based access
@@ -213,7 +213,7 @@
 //	}
 //	fmt.Printf("Session revoked: %v\n", revoked)
 //
-// # TCP Port Forwarding
+// # TCP Port Forwarding (available in a future release)
 //
 // Forward a local connection to a port inside a sandbox:
 //
@@ -242,7 +242,7 @@
 //	    v1.WithForwardServiceID("billing-db"),
 //	)
 //
-// # SSH Tunneling
+// # SSH Tunneling (available in a future release)
 //
 // Create an SSH tunnel to a sandbox port in a single call. Tunnel combines
 // session creation, TCP forwarding with an SSH relay target, and automatic
@@ -300,7 +300,7 @@
 //	    },
 //	}, nil)
 //
-// Replace the full policy at runtime via configuration update:
+// Replace the full policy at runtime via configuration update (available in a future release):
 //
 //	result, err := client.Config().Update(ctx, &v1.ConfigUpdate{
 //	    Name: "secure-sandbox",
@@ -312,7 +312,7 @@
 //	    },
 //	})
 //
-// Read a policy back from revision history:
+// Read a policy back from revision history (available in a future release):
 //
 //	revisions, err := client.Policy().List(ctx, "secure-sandbox")
 //	if err != nil {
@@ -324,7 +324,7 @@
 //	    }
 //	}
 //
-// # Configuration Management
+// # Configuration Management (available in a future release)
 //
 // Read sandbox and gateway configuration, and update settings:
 //

sdk/go/openshell/v1/integration_test.go

@@ -29,6 +29,8 @@ func TestIntegration_HealthCheck(t *testing.T) {
 	require.NoError(t, err)
 	defer client.Close()
 
+	t.Skip("TODO: Health().Check() is a stub until PR B lands")
+
 	_, err = client.Health().Check(context.Background())
 	require.NoError(t, err)
 }

sdk/go/openshell/v1/internal/converter/copy.go

@@ -65,12 +65,3 @@ func mapToStruct(m map[string]any) *structpb.Struct {
 	return s
 }
 
-func boolCount(flags ...bool) int {
-	n := 0
-	for _, f := range flags {
-		if f {
-			n++
-		}
-	}
-	return n
-}

sdk/go/openshell/v1/sandbox_client.go

@@ -205,11 +205,15 @@ func (s *sandboxClient) Watch(ctx context.Context, name string, opts ...WatchOpt
 	go func() {
 		defer close(ch)
 		ev := first
+		isFirst := true
 		for {
 			if sbPayload, ok := ev.Payload.(*pb.SandboxStreamEvent_Sandbox); ok && sbPayload.Sandbox != nil {
 				sandbox := converter.SandboxFromProto(sbPayload.Sandbox)
 				eventType := EventModified
-				if sandbox.Status.Phase == SandboxDeleting {
+				if isFirst {
+					eventType = EventAdded
+					isFirst = false
+				} else if sandbox.Status.Phase == SandboxDeleting {
 					eventType = EventDeleted
 				}
 				select {

sdk/go/openshell/v1/sandbox_client_test.go

@@ -55,6 +55,8 @@ func newMockSandboxServer() *mockSandboxServer {
 }
 
 func (s *mockSandboxServer) CreateSandbox(_ context.Context, req *pb.CreateSandboxRequest) (*pb.SandboxResponse, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	if s.createErr != nil {
 		return nil, s.createErr
 	}
@@ -96,6 +98,8 @@ func (s *mockSandboxServer) setPhase(name string, phase pb.SandboxPhase) {
 }
 
 func (s *mockSandboxServer) ListSandboxes(_ context.Context, _ *pb.ListSandboxesRequest) (*pb.ListSandboxesResponse, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	if s.listErr != nil {
 		return nil, s.listErr
 	}
@@ -107,6 +111,8 @@ func (s *mockSandboxServer) ListSandboxes(_ context.Context, _ *pb.ListSandboxes
 }
 
 func (s *mockSandboxServer) DeleteSandbox(_ context.Context, req *pb.DeleteSandboxRequest) (*pb.DeleteSandboxResponse, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	if s.deleteErr != nil {
 		return nil, s.deleteErr
 	}
@@ -119,6 +125,8 @@ func (s *mockSandboxServer) DeleteSandbox(_ context.Context, req *pb.DeleteSandb
 }
 
 func (s *mockSandboxServer) AttachSandboxProvider(_ context.Context, req *pb.AttachSandboxProviderRequest) (*pb.AttachSandboxProviderResponse, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	if s.attachErr != nil {
 		return nil, s.attachErr
 	}
@@ -131,6 +139,8 @@ func (s *mockSandboxServer) AttachSandboxProvider(_ context.Context, req *pb.Att
 }
 
 func (s *mockSandboxServer) DetachSandboxProvider(_ context.Context, req *pb.DetachSandboxProviderRequest) (*pb.DetachSandboxProviderResponse, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	if s.detachErr != nil {
 		return nil, s.detachErr
 	}
@@ -567,7 +577,7 @@ func TestSandboxWatch_ReceivesEvents(t *testing.T) {
 	defer w.Stop()
 
 	ev1 := <-w.ResultChan()
-	assert.Equal(t, EventModified, ev1.Type)
+	assert.Equal(t, EventAdded, ev1.Type)
 	require.NotNil(t, ev1.Object)
 	assert.Equal(t, "sb-1", ev1.Object.Name)
 	assert.Equal(t, SandboxProvisioning, ev1.Object.Status.Phase)
@@ -599,7 +609,7 @@ func TestSandboxWatch_FiltersSandboxEventsOnly(t *testing.T) {
 	defer w.Stop()
 
 	ev := <-w.ResultChan()
-	assert.Equal(t, EventModified, ev.Type)
+	assert.Equal(t, EventAdded, ev.Type)
 	assert.Equal(t, "sb-1", ev.Object.Name)
 
 	// Stream ends after server sends all events; channel should close
@@ -738,7 +748,7 @@ func TestSandboxWatch_StopOnTerminal_Ready(t *testing.T) {
 
 	// Should receive the Provisioning event
 	ev1 := <-w.ResultChan()
-	assert.Equal(t, EventModified, ev1.Type)
+	assert.Equal(t, EventAdded, ev1.Type)
 	assert.Equal(t, SandboxProvisioning, ev1.Object.Status.Phase)
 
 	// Should receive the Ready event (terminal)
@@ -782,7 +792,7 @@ func TestSandboxWatch_StopOnTerminal_Error(t *testing.T) {
 
 	// Should receive the Provisioning event
 	ev1 := <-w.ResultChan()
-	assert.Equal(t, EventModified, ev1.Type)
+	assert.Equal(t, EventAdded, ev1.Type)
 	assert.Equal(t, SandboxProvisioning, ev1.Object.Status.Phase)
 
 	// Should receive the Error event (terminal)
@@ -820,7 +830,7 @@ func TestSandboxWatch_StopOnTerminal_False_DoesNotClose(t *testing.T) {
 	defer w.Stop()
 
 	ev := <-w.ResultChan()
-	assert.Equal(t, EventModified, ev.Type)
+	assert.Equal(t, EventAdded, ev.Type)
 	assert.Equal(t, SandboxReady, ev.Object.Status.Phase)
 
 	// Channel closes because mock stream ends (not because of StopOnTerminal)

sdk/go/openshell/v1/types/config.go

@@ -7,10 +7,13 @@ import "time"
 
 // Config holds all settings needed to create a Client.
 type Config struct {
-	Address     string
-	TLS         *TLSConfig
-	Auth        AuthProvider
-	Timeout     time.Duration
+	Address string
+	TLS     *TLSConfig
+	Auth    AuthProvider
+	// Timeout is reserved for future use. It is not yet applied.
+	Timeout time.Duration
+	// RetryPolicy is reserved for future use. It is not yet applied.
 	RetryPolicy *RetryPolicy
-	Logger      Logger
+	// Logger is reserved for future use. It is not yet applied.
+	Logger Logger
 }

sdk/go/openshell/v1/types/options.go

@@ -26,8 +26,10 @@ type UpdateOptions struct{}
 
 // WatchOptions configures watch behavior.
 type WatchOptions struct {
+	// TimeoutSeconds is reserved for future use. Use context for timeout control.
 	TimeoutSeconds int64
-	LabelSelector  string
+	// LabelSelector is reserved for future use.
+	LabelSelector string
 	// StopOnTerminal causes the watch to close automatically when the sandbox
 	// reaches a terminal phase (Ready or Error).
 	StopOnTerminal bool