From da0cbdc93bbf161b15175d7e09fcd43df2bf363f Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:49:58 -0400 Subject: [PATCH 1/2] fix browser pool update clear semantics Preserve omitted values while allowing explicit zero fill rates and durable proxy, profile, extension, and Chrome policy clears. Verify the serialized SDK request bodies and reject conflicting or invalid inputs before making API calls. --- README.md | 2 +- cmd/browser_pools.go | 76 +++++++++++++--- cmd/browser_pools_test.go | 182 +++++++++++++++++++++++++++++++------- cmd/browsers.go | 6 +- 4 files changed, 218 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index c54f458..ea57950 100644 --- a/README.md +++ b/README.md @@ -275,7 +275,7 @@ Commands with JSON output support: - `kernel browser-pools get ` - Get pool details - `--output json`, `-o json` - Output raw JSON object - `kernel browser-pools update ` - Update pool configuration - - Same flags as create plus `--clear-start-url` (remove the pool's start URL) and `--discard-all-idle` (discard all idle browsers and refill). An empty `--chrome-policy '{}'` is ignored and does not clear an existing policy; recreate the pool to remove one. `--telemetry` updates only apply to browsers warmed after the update. + - Same flags as create plus `--clear-profile`, `--clear-proxy`, `--clear-start-url`, `--clear-extensions`, and `--clear-chrome-policy` for removing durable configuration. `--fill-rate 0` pauses automatic filling. `--discard-all-idle` discards all idle browsers and refills the pool. `--telemetry` updates only apply to browsers warmed after the update. - `--output json`, `-o json` - Output raw JSON object - `kernel browser-pools delete ` - Delete a pool - `--force` - Force delete even if browsers are leased diff --git a/cmd/browser_pools.go b/cmd/browser_pools.go index ca753bd..c80bcdc 100644 --- a/cmd/browser_pools.go +++ b/cmd/browser_pools.go @@ -291,7 +291,7 @@ type BrowserPoolsUpdateInput struct { IDOrName string Name string Size int64 - FillRate int64 + FillRate Int64Flag TimeoutSeconds int64 Stealth BoolFlag Headless BoolFlag @@ -299,18 +299,44 @@ type BrowserPoolsUpdateInput struct { RefreshOnProfileUpdate BoolFlag ProfileID string ProfileName string + ClearProfile bool ProxyID string + ClearProxy bool StartURL string ClearStartURL bool Extensions []string + ClearExtensions bool Viewport string ChromePolicy string ChromePolicyFile string + ClearChromePolicy bool Telemetry string DiscardAllIdle BoolFlag Output string } +func validateBrowserPoolUpdateInput(in BrowserPoolsUpdateInput) error { + if in.StartURL != "" && in.ClearStartURL { + return fmt.Errorf("cannot specify both --start-url and --clear-start-url") + } + if in.FillRate.Set && in.FillRate.Value < 0 { + return fmt.Errorf("--fill-rate must be zero or greater") + } + if in.ProxyID != "" && in.ClearProxy { + return fmt.Errorf("cannot specify both --proxy-id and --clear-proxy") + } + if (in.ProfileID != "" || in.ProfileName != "") && in.ClearProfile { + return fmt.Errorf("cannot specify --clear-profile with --profile-id or --profile-name") + } + if len(in.Extensions) > 0 && in.ClearExtensions { + return fmt.Errorf("cannot specify both --extension and --clear-extensions") + } + if (in.ChromePolicy != "" || in.ChromePolicyFile != "") && in.ClearChromePolicy { + return fmt.Errorf("cannot specify --clear-chrome-policy with --chrome-policy or --chrome-policy-file") + } + return nil +} + func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) error { if err := validateJSONOutput(in.Output); err != nil { return err @@ -318,8 +344,8 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) if err := validateStartURLFlag(in.StartURL); err != nil { return err } - if in.StartURL != "" && in.ClearStartURL { - return fmt.Errorf("cannot specify both --start-url and --clear-start-url") + if err := validateBrowserPoolUpdateInput(in); err != nil { + return err } params := kernel.BrowserPoolUpdateParams{} @@ -330,8 +356,8 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) if in.Size > 0 { params.Size = kernel.Int(in.Size) } - if in.FillRate > 0 { - params.FillRatePerMinute = kernel.Int(in.FillRate) + if in.FillRate.Set { + params.FillRatePerMinute = kernel.Int(in.FillRate.Value) } if in.TimeoutSeconds > 0 { params.TimeoutSeconds = kernel.Int(in.TimeoutSeconds) @@ -357,7 +383,9 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) pterm.Error.Println(err.Error()) return nil } - if profileSet { + if in.ClearProfile { + params.Profile.ID = kernel.String("") + } else if profileSet { if profileID != "" { params.Profile.ID = kernel.String(profileID) } else { @@ -365,7 +393,9 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) } } - if in.ProxyID != "" { + if in.ClearProxy { + params.ProxyID = kernel.String("") + } else if in.ProxyID != "" { params.ProxyID = kernel.String(in.ProxyID) } if in.ClearStartURL { @@ -391,11 +421,19 @@ func (c BrowserPoolsCmd) Update(ctx context.Context, in BrowserPoolsUpdateInput) } if len(chromePolicy) > 0 { params.ChromePolicy = chromePolicy - } else if (in.ChromePolicy != "" || in.ChromePolicyFile != "") && in.Output != "json" { - // An empty policy ({}) cannot clear an existing one: omitzero drops it before it - // reaches the server. Warn instead of silently doing nothing, but stay quiet on the - // json path so stdout remains valid JSON. - pterm.Warning.Println("An empty chrome policy is ignored and does not clear the pool's existing policy; recreate the pool to remove a policy.") + } + + extraFields := map[string]any{} + // The SDK's omitzero encoder drops empty collections, so explicit clears use + // its extra-fields escape hatch to preserve {} and [] on the wire. + if in.ClearExtensions { + extraFields["extensions"] = []kernel.BrowserExtensionParam{} + } + if in.ClearChromePolicy || (chromePolicy != nil && len(chromePolicy) == 0) { + extraFields["chrome_policy"] = map[string]any{} + } + if len(extraFields) > 0 { + params.SetExtraFields(extraFields) } if in.Telemetry != "" { @@ -663,13 +701,17 @@ func init() { browserPoolsUpdateCmd.Flags().Bool("refresh-on-profile-update", false, "Flush idle browsers when the pool's profile is updated") browserPoolsUpdateCmd.Flags().String("profile-id", "", "Profile ID") browserPoolsUpdateCmd.Flags().String("profile-name", "", "Profile name") + browserPoolsUpdateCmd.Flags().Bool("clear-profile", false, "Remove the pool profile") browserPoolsUpdateCmd.Flags().String("proxy-id", "", "Proxy ID") + browserPoolsUpdateCmd.Flags().Bool("clear-proxy", false, "Remove the pool proxy") browserPoolsUpdateCmd.Flags().String("start-url", "", "Initial page to open for new browsers") browserPoolsUpdateCmd.Flags().Bool("clear-start-url", false, "Clear the pool start URL") browserPoolsUpdateCmd.Flags().StringSlice("extension", []string{}, "Extension IDs or names") + browserPoolsUpdateCmd.Flags().Bool("clear-extensions", false, "Remove all pool extensions") browserPoolsUpdateCmd.Flags().String("viewport", "", "Viewport size (e.g. 1280x800)") browserPoolsUpdateCmd.Flags().String("chrome-policy", "", "Custom Chrome enterprise policy as a JSON object") browserPoolsUpdateCmd.Flags().String("chrome-policy-file", "", "Read Chrome enterprise policy (JSON object) from a file (use '-' for stdin)") + browserPoolsUpdateCmd.Flags().Bool("clear-chrome-policy", false, "Remove the pool's custom Chrome enterprise policy") browserPoolsUpdateCmd.MarkFlagsMutuallyExclusive("chrome-policy", "chrome-policy-file") browserPoolsUpdateCmd.Flags().String("telemetry", "", "Update pool telemetry: --telemetry=all (reset to default set), --telemetry=off (disable), or --telemetry=console,network (merge those categories into the current selection). Applies only to browsers warmed after the update.") browserPoolsUpdateCmd.Flags().Bool("discard-all-idle", false, "Discard all idle browsers") @@ -779,13 +821,17 @@ func runBrowserPoolsUpdate(cmd *cobra.Command, args []string) error { refreshOnProfileUpdate, _ := cmd.Flags().GetBool("refresh-on-profile-update") profileID, _ := cmd.Flags().GetString("profile-id") profileName, _ := cmd.Flags().GetString("profile-name") + clearProfile, _ := cmd.Flags().GetBool("clear-profile") proxyID, _ := cmd.Flags().GetString("proxy-id") + clearProxy, _ := cmd.Flags().GetBool("clear-proxy") startURL, _ := cmd.Flags().GetString("start-url") clearStartURL, _ := cmd.Flags().GetBool("clear-start-url") extensions, _ := cmd.Flags().GetStringSlice("extension") + clearExtensions, _ := cmd.Flags().GetBool("clear-extensions") viewport, _ := cmd.Flags().GetString("viewport") chromePolicy, _ := cmd.Flags().GetString("chrome-policy") chromePolicyFile, _ := cmd.Flags().GetString("chrome-policy-file") + clearChromePolicy, _ := cmd.Flags().GetBool("clear-chrome-policy") telemetry, _ := cmd.Flags().GetString("telemetry") discardIdle, _ := cmd.Flags().GetBool("discard-all-idle") output, _ := cmd.Flags().GetString("output") @@ -794,7 +840,7 @@ func runBrowserPoolsUpdate(cmd *cobra.Command, args []string) error { IDOrName: args[0], Name: name, Size: size, - FillRate: fillRate, + FillRate: Int64Flag{Set: cmd.Flags().Changed("fill-rate"), Value: fillRate}, TimeoutSeconds: timeout, Stealth: BoolFlag{Set: cmd.Flags().Changed("stealth"), Value: stealth}, Headless: BoolFlag{Set: cmd.Flags().Changed("headless"), Value: headless}, @@ -802,13 +848,17 @@ func runBrowserPoolsUpdate(cmd *cobra.Command, args []string) error { RefreshOnProfileUpdate: BoolFlag{Set: cmd.Flags().Changed("refresh-on-profile-update"), Value: refreshOnProfileUpdate}, ProfileID: profileID, ProfileName: profileName, + ClearProfile: clearProfile, ProxyID: proxyID, + ClearProxy: clearProxy, StartURL: startURL, ClearStartURL: clearStartURL, Extensions: extensions, + ClearExtensions: clearExtensions, Viewport: viewport, ChromePolicy: chromePolicy, ChromePolicyFile: chromePolicyFile, + ClearChromePolicy: clearChromePolicy, Telemetry: telemetry, DiscardAllIdle: BoolFlag{Set: cmd.Flags().Changed("discard-all-idle"), Value: discardIdle}, Output: output, diff --git a/cmd/browser_pools_test.go b/cmd/browser_pools_test.go index 5a1ab9e..17e952c 100644 --- a/cmd/browser_pools_test.go +++ b/cmd/browser_pools_test.go @@ -2,12 +2,16 @@ package cmd import ( "context" + "encoding/json" + "os" + "path/filepath" "testing" "github.com/kernel/kernel-go-sdk" "github.com/kernel/kernel-go-sdk/option" "github.com/kernel/kernel-go-sdk/packages/pagination" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // FakeBrowserPoolsService is a configurable fake implementing BrowserPoolsService. @@ -244,45 +248,161 @@ func TestBrowserPoolsUpdate_WithChromePolicy(t *testing.T) { assert.Equal(t, map[string]any{"BookmarkBarEnabled": false}, captured.ChromePolicy) } -func TestBrowserPoolsUpdate_EmptyChromePolicyWarnsAndDoesNotClear(t *testing.T) { - setupStdoutCapture(t) - - var captured kernel.BrowserPoolUpdateParams - fake := &FakeBrowserPoolsService{ - UpdateFunc: func(ctx context.Context, id string, body kernel.BrowserPoolUpdateParams, opts ...option.RequestOption) (*kernel.BrowserPool, error) { - captured = body - return &kernel.BrowserPool{ID: id}, nil +func TestBrowserPoolsUpdate_DurableClearAndZeroStates(t *testing.T) { + policyDir := t.TempDir() + emptyObjectPolicyFile := filepath.Join(policyDir, "empty-object.json") + require.NoError(t, os.WriteFile(emptyObjectPolicyFile, []byte(`{}`), 0o600)) + blankPolicyFile := filepath.Join(policyDir, "blank.json") + require.NoError(t, os.WriteFile(blankPolicyFile, []byte("\n"), 0o600)) + + tests := []struct { + name string + input BrowserPoolsUpdateInput + wantJSON string + }{ + { + name: "durable fields omitted", + input: BrowserPoolsUpdateInput{}, + wantJSON: `{}`, + }, + { + name: "fill rate zero", + input: BrowserPoolsUpdateInput{ + FillRate: Int64Flag{Set: true, Value: 0}, + }, + wantJSON: `{"fill_rate_per_minute":0}`, + }, + { + name: "clear proxy", + input: BrowserPoolsUpdateInput{ + ClearProxy: true, + }, + wantJSON: `{"proxy_id":""}`, + }, + { + name: "clear profile", + input: BrowserPoolsUpdateInput{ + ClearProfile: true, + }, + wantJSON: `{"profile":{"id":""}}`, + }, + { + name: "clear extensions", + input: BrowserPoolsUpdateInput{ + ClearExtensions: true, + }, + wantJSON: `{"extensions":[]}`, + }, + { + name: "clear Chrome policy", + input: BrowserPoolsUpdateInput{ + ClearChromePolicy: true, + }, + wantJSON: `{"chrome_policy":{}}`, + }, + { + name: "empty inline Chrome policy", + input: BrowserPoolsUpdateInput{ + ChromePolicy: `{}`, + }, + wantJSON: `{"chrome_policy":{}}`, + }, + { + name: "empty object Chrome policy file", + input: BrowserPoolsUpdateInput{ + ChromePolicyFile: emptyObjectPolicyFile, + }, + wantJSON: `{"chrome_policy":{}}`, + }, + { + name: "blank Chrome policy file", + input: BrowserPoolsUpdateInput{ + ChromePolicyFile: blankPolicyFile, + }, + wantJSON: `{}`, + }, + { + name: "all durable clear states", + input: BrowserPoolsUpdateInput{ + FillRate: Int64Flag{Set: true, Value: 0}, + ClearProfile: true, + ClearProxy: true, + ClearExtensions: true, + ClearChromePolicy: true, + }, + wantJSON: `{"fill_rate_per_minute":0,"profile":{"id":""},"proxy_id":"","extensions":[],"chrome_policy":{}}`, }, } - c := BrowserPoolsCmd{client: fake} - err := c.Update(context.Background(), BrowserPoolsUpdateInput{ - IDOrName: "pool-1", - ChromePolicy: "{}", - }) - assert.NoError(t, err) - assert.Nil(t, captured.ChromePolicy) - assert.Contains(t, outBuf.String(), "does not clear") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupStdoutCapture(t) + + var gotJSON []byte + var marshalErr error + fake := &FakeBrowserPoolsService{ + UpdateFunc: func(ctx context.Context, id string, body kernel.BrowserPoolUpdateParams, opts ...option.RequestOption) (*kernel.BrowserPool, error) { + gotJSON, marshalErr = json.Marshal(body) + return &kernel.BrowserPool{ID: id}, nil + }, + } + + tt.input.IDOrName = "pool-1" + err := (BrowserPoolsCmd{client: fake}).Update(context.Background(), tt.input) + require.NoError(t, err) + require.NoError(t, marshalErr) + assert.JSONEq(t, tt.wantJSON, string(gotJSON)) + }) + } } -func TestBrowserPoolsUpdate_EmptyChromePolicyQuietInJSONMode(t *testing.T) { - setupStdoutCapture(t) - - fake := &FakeBrowserPoolsService{ - UpdateFunc: func(ctx context.Context, id string, body kernel.BrowserPoolUpdateParams, opts ...option.RequestOption) (*kernel.BrowserPool, error) { - return &kernel.BrowserPool{ID: id}, nil +func TestBrowserPoolsUpdate_RejectsInvalidDurableInputs(t *testing.T) { + tests := []struct { + name string + input BrowserPoolsUpdateInput + wantErr string + }{ + { + name: "conflicting proxy flags", + input: BrowserPoolsUpdateInput{ProxyID: "proxy-1", ClearProxy: true}, + wantErr: "cannot specify both --proxy-id and --clear-proxy", + }, + { + name: "conflicting profile flags", + input: BrowserPoolsUpdateInput{ProfileID: "profile-1", ClearProfile: true}, + wantErr: "cannot specify --clear-profile with --profile-id or --profile-name", + }, + { + name: "conflicting extension flags", + input: BrowserPoolsUpdateInput{Extensions: []string{"extension-1"}, ClearExtensions: true}, + wantErr: "cannot specify both --extension and --clear-extensions", + }, + { + name: "conflicting Chrome policy flags", + input: BrowserPoolsUpdateInput{ChromePolicy: `{}`, ClearChromePolicy: true}, + wantErr: "cannot specify --clear-chrome-policy with --chrome-policy or --chrome-policy-file", + }, + { + name: "negative fill rate", + input: BrowserPoolsUpdateInput{FillRate: Int64Flag{Set: true, Value: -1}}, + wantErr: "--fill-rate must be zero or greater", }, } - c := BrowserPoolsCmd{client: fake} - err := c.Update(context.Background(), BrowserPoolsUpdateInput{ - IDOrName: "pool-1", - ChromePolicy: "{}", - Output: "json", - }) - assert.NoError(t, err) - // The warning must not leak onto stdout in json mode, where it would corrupt the payload. - assert.NotContains(t, outBuf.String(), "does not clear") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fake := &FakeBrowserPoolsService{ + UpdateFunc: func(context.Context, string, kernel.BrowserPoolUpdateParams, ...option.RequestOption) (*kernel.BrowserPool, error) { + t.Fatal("Update should not be called for invalid input") + return nil, nil + }, + } + + tt.input.IDOrName = "pool-1" + err := (BrowserPoolsCmd{client: fake}).Update(context.Background(), tt.input) + require.EqualError(t, err, tt.wantErr) + }) + } } func TestBrowserPoolsCreate_WithTelemetry(t *testing.T) { diff --git a/cmd/browsers.go b/cmd/browsers.go index 0561184..1d27d91 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -185,9 +185,9 @@ func parseStringMapFlag(values []string, flagName string) (map[string]string, er // parseChromePolicy resolves the --chrome-policy / --chrome-policy-file inputs into a // custom Chrome enterprise policy object. The two inputs are mutually exclusive (enforced // by cobra); a file path of "-" reads stdin. It returns a nil map when neither input is -// set or the content is empty. An explicit empty object ("{}") yields a non-nil empty map, -// so callers must guard the SDK assignment with len>0: chrome_policy uses omitzero, which -// drops only a nil map, not an empty one. +// set or the content is empty. An explicit empty object ("{}") yields a non-nil empty map. +// Generated chrome_policy params use omitzero, which drops empty maps; update callers that +// need to send {} must use the SDK's extra-fields escape hatch. func parseChromePolicy(inline, file string) (map[string]any, error) { data := strings.TrimSpace(inline) if file != "" { From 324e1033d8c0acd581071aa5e12afee6e8d6c005 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Fri, 7 Aug 2026 10:28:28 -0400 Subject: [PATCH 2/2] test(cli): cover browser pool start URL clears --- cmd/browser_pools_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/cmd/browser_pools_test.go b/cmd/browser_pools_test.go index 17e952c..921220e 100644 --- a/cmd/browser_pools_test.go +++ b/cmd/browser_pools_test.go @@ -286,6 +286,13 @@ func TestBrowserPoolsUpdate_DurableClearAndZeroStates(t *testing.T) { }, wantJSON: `{"profile":{"id":""}}`, }, + { + name: "clear start URL", + input: BrowserPoolsUpdateInput{ + ClearStartURL: true, + }, + wantJSON: `{"start_url":""}`, + }, { name: "clear extensions", input: BrowserPoolsUpdateInput{ @@ -327,10 +334,11 @@ func TestBrowserPoolsUpdate_DurableClearAndZeroStates(t *testing.T) { FillRate: Int64Flag{Set: true, Value: 0}, ClearProfile: true, ClearProxy: true, + ClearStartURL: true, ClearExtensions: true, ClearChromePolicy: true, }, - wantJSON: `{"fill_rate_per_minute":0,"profile":{"id":""},"proxy_id":"","extensions":[],"chrome_policy":{}}`, + wantJSON: `{"fill_rate_per_minute":0,"profile":{"id":""},"proxy_id":"","start_url":"","extensions":[],"chrome_policy":{}}`, }, } @@ -367,6 +375,11 @@ func TestBrowserPoolsUpdate_RejectsInvalidDurableInputs(t *testing.T) { input: BrowserPoolsUpdateInput{ProxyID: "proxy-1", ClearProxy: true}, wantErr: "cannot specify both --proxy-id and --clear-proxy", }, + { + name: "conflicting start URL flags", + input: BrowserPoolsUpdateInput{StartURL: "https://example.com", ClearStartURL: true}, + wantErr: "cannot specify both --start-url and --clear-start-url", + }, { name: "conflicting profile flags", input: BrowserPoolsUpdateInput{ProfileID: "profile-1", ClearProfile: true},