review: fix create/update --telemetry parity; fix append aliasing; gofmt

Author: archandatta

cmd/browsers.go

@@ -427,12 +427,14 @@ func (b BrowsersCmd) Create(ctx context.Context, in BrowsersCreateInput) error {
 
 	if in.Telemetry == "all" {
 		params.Telemetry = kernel.BrowserTelemetryRequestConfigParam{Enabled: kernel.Opt(true)}
+	} else if in.Telemetry == "off" {
+		params.Telemetry = kernel.BrowserTelemetryRequestConfigParam{Enabled: kernel.Opt(false)}
 	} else if in.Telemetry != "" {
 		p, err := parseTelemetryCategories(in.Telemetry)
 		if err != nil {
 			return err
 		}
-		params.Telemetry = kernel.BrowserTelemetryRequestConfigParam{Browser: p}
+		params.Telemetry = kernel.BrowserTelemetryRequestConfigParam{Enabled: kernel.Opt(true), Browser: p}
 	}
 
 	browser, err := b.browsers.New(ctx, params)
@@ -2535,7 +2537,7 @@ func init() {
 	browsersCreateCmd.Flags().Bool("viewport-interactive", false, "Interactively select viewport size from list")
 	browsersCreateCmd.Flags().String("pool-id", "", "Browser pool ID to acquire from (mutually exclusive with --pool-name)")
 	browsersCreateCmd.Flags().String("pool-name", "", "Browser pool name to acquire from (mutually exclusive with --pool-id)")
-	browsersCreateCmd.Flags().String("telemetry", "", "Enable telemetry: --telemetry=all to enable all, --telemetry=network=on,page=off for per-category")
+	browsersCreateCmd.Flags().String("telemetry", "", "Configure telemetry: --telemetry=all to enable, --telemetry=off to disable, --telemetry=network=on,page=off for per-category")
 
 	// curl
 	curlCmd := &cobra.Command{

cmd/browsers_telemetry.go

@@ -68,6 +68,9 @@ func parseTelemetryCategories(s string) (kernel.BrowserTelemetryCategoriesConfig
 // "system" is always-on and cannot be toggled, but is valid as a --categories stream filter.
 var settableCategories = []string{"console", "interaction", "network", "page"}
 
+// streamableCategories extends settableCategories with "system" for --categories stream filtering.
+var streamableCategories = append(settableCategories[:len(settableCategories):len(settableCategories)], "system")
+
 // eventCategory derives the category from the event type prefix.
 // "monitor_*" maps to "system"; all others use the prefix before the first "_".
 // TODO(sdk): kernel-go-sdk should surface Category directly on BrowserTelemetryEventUnion.
@@ -98,8 +101,8 @@ func (b BrowsersCmd) TelemetryStream(ctx context.Context, in BrowsersTelemetrySt
 		return fmt.Errorf("unsupported --output value: use 'json'")
 	}
 	for _, c := range in.Categories {
-		if c != "system" && !slices.Contains(settableCategories, c) {
-			return fmt.Errorf("unknown category %q: must be one of %s", c, strings.Join(append(settableCategories, "system"), ", "))
+		if !slices.Contains(streamableCategories, c) {
+			return fmt.Errorf("unknown category %q: must be one of %s", c, strings.Join(streamableCategories, ", "))
 		}
 	}
 	if b.telemetry == nil {

cmd/browsers_telemetry_test.go

@@ -29,7 +29,6 @@ func TestTelemetryStream_UnknownCategoryErrors(t *testing.T) {
 	assert.Contains(t, err.Error(), "unknown category")
 }
 
-
 func makeEvent(t *testing.T, raw string) kernel.BrowserTelemetryEventUnion {
 	t.Helper()
 	var ev kernel.BrowserTelemetryEventUnion

cmd/browsers_test.go

@@ -911,7 +911,6 @@ func (f *FakeProcessService) StdoutStreamStreaming(ctx context.Context, processI
 	return makeStream([]kernel.BrowserProcessStdoutStreamResponse{{Stream: kernel.BrowserProcessStdoutStreamResponseStreamStdout, DataB64: "aGVsbG8=", Event: ""}, {Event: "exit", ExitCode: 0}})
 }
 
-
 type FakeLogService struct {
 	StreamFunc func(ctx context.Context, id string, query kernel.BrowserLogStreamParams, opts ...option.RequestOption) *ssestream.Stream[shared.LogEvent]
 }
@@ -1672,7 +1671,8 @@ func TestBrowsersCreate_WithTelemetryCategories(t *testing.T) {
 	err := b.Create(context.Background(), BrowsersCreateInput{Telemetry: "network=on,page=off"})
 
 	assert.NoError(t, err)
-	assert.False(t, captured.Telemetry.Enabled.Valid())
+	assert.True(t, captured.Telemetry.Enabled.Valid())
+	assert.True(t, captured.Telemetry.Enabled.Value)
 	assert.True(t, captured.Telemetry.Browser.Network.Enabled.Valid())
 	assert.True(t, captured.Telemetry.Browser.Network.Enabled.Value)
 	assert.True(t, captured.Telemetry.Browser.Page.Enabled.Valid())
@@ -1681,6 +1681,22 @@ func TestBrowsersCreate_WithTelemetryCategories(t *testing.T) {
 	assert.False(t, captured.Telemetry.Browser.Interaction.Enabled.Valid())
 }
 
+func TestBrowsersCreate_WithTelemetryOff(t *testing.T) {
+	setupStdoutCapture(t)
+	var captured kernel.BrowserNewParams
+	fake := &FakeBrowsersService{NewFunc: func(ctx context.Context, body kernel.BrowserNewParams, opts ...option.RequestOption) (*kernel.BrowserNewResponse, error) {
+		captured = body
+		return &kernel.BrowserNewResponse{SessionID: "session123", CdpWsURL: "ws://example"}, nil
+	}}
+	b := BrowsersCmd{browsers: fake}
+
+	err := b.Create(context.Background(), BrowsersCreateInput{Telemetry: "off"})
+
+	assert.NoError(t, err)
+	assert.True(t, captured.Telemetry.Enabled.Valid())
+	assert.False(t, captured.Telemetry.Enabled.Value)
+}
+
 func TestBrowsersCreate_WithoutTelemetry(t *testing.T) {
 	setupStdoutCapture(t)
 	var captured kernel.BrowserNewParams
@@ -1783,4 +1799,3 @@ func TestBrowsersUpdate_ForceWithProxyButNoViewport_Errors(t *testing.T) {
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "--force requires --viewport")
 }
-