diff --git a/README.md b/README.md index d99ca17..27f485d 100644 --- a/README.md +++ b/README.md @@ -213,7 +213,7 @@ Commands with JSON output support: - `-H, --headless` - Launch browser without GUI access - `--kiosk` - Launch browser in kiosk mode - `--start-url ` - Initial page to open on launch - - `--name ` - Optional unique name for the session (set at creation; used to find it later by name) + - `--name ` - Optional unique name for the session (used to find it later by name; can be changed with `browsers update --name`) - `--tag ` - Set a tag on the session, repeatable; up to 50 pairs - `--pool-id ` - Acquire a browser from the specified pool (mutually exclusive with --pool-name; ignores other session flags). `--name`/`--tag` still apply to the acquired session. - `--pool-name ` - Acquire a browser from the pool name (mutually exclusive with --pool-id; ignores other session flags) @@ -230,6 +230,10 @@ Commands with JSON output support: - `kernel browsers get ` - Get detailed browser session info by ID or name - `--output json`, `-o json` - Output raw JSON object - `kernel browsers update ` - Update a running browser session by ID or name + - `--name ` - Set a new unique name for the session (mutually exclusive with `--clear-name`) + - `--clear-name` - Clear the session name + - `--tag ` - Set a tag, repeatable; up to 50 pairs. Replaces the entire tag set (not merged); mutually exclusive with `--clear-tags` + - `--clear-tags` - Remove all tags from the session - `--telemetry=all` - Enable telemetry for all categories - `--telemetry=off` - Disable telemetry - `--telemetry=` - Per-category config, e.g. `--telemetry=network=on,page=off` diff --git a/cmd/browsers.go b/cmd/browsers.go index feb7487..fde9229 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -298,6 +298,12 @@ type BrowsersUpdateInput struct { Viewport string Force bool Telemetry string + Name string + SetName bool + ClearName bool + Tags map[string]string + SetTags bool + ClearTags bool Output string } @@ -697,9 +703,37 @@ func (b BrowsersCmd) Update(ctx context.Context, in BrowsersUpdateInput) error { return fmt.Errorf("cannot specify both --proxy-id and --clear-proxy") } + // Cannot specify both --name and --clear-name + if in.SetName && in.ClearName { + return fmt.Errorf("cannot specify both --name and --clear-name") + } + + // Cannot specify both --tag and --clear-tags. Use SetTags (the raw flag + // signal) as well as the parsed map so the check still fires when every + // --tag value was malformed and dropped to an empty map. + if (in.SetTags || len(in.Tags) > 0) && in.ClearTags { + return fmt.Errorf("cannot specify both --tag and --clear-tags") + } + + // --tag was provided but parsed to zero valid pairs (every value malformed). + // Treat as a user error rather than silently leaving tags unchanged. + if in.SetTags && len(in.Tags) == 0 { + return fmt.Errorf("no valid --tag KEY=VALUE pairs provided") + } + + // --name must carry a value; clearing is done explicitly via --clear-name. + // (A set name combined with --clear-name is already rejected above, so the + // ClearName case cannot reach here.) + if in.SetName && in.Name == "" { + return fmt.Errorf("--name requires a non-empty value; use --clear-name to clear the name") + } + hasProxyChange := in.ProxyID != "" || in.ClearProxy hasProfileChange := in.ProfileID != "" || in.ProfileName != "" hasViewportChange := in.Viewport != "" + // By this point a set name is guaranteed non-empty (the guard above rejects --name ""). + hasNameChange := in.SetName || in.ClearName + hasTagsChange := len(in.Tags) > 0 || in.ClearTags // Validate --save-changes is only used with a profile if in.ProfileSaveChanges.Set && !hasProfileChange { @@ -712,12 +746,27 @@ func (b BrowsersCmd) Update(ctx context.Context, in BrowsersUpdateInput) error { } // Validate that at least one update option is provided - if !hasProxyChange && !hasProfileChange && !hasViewportChange && in.Telemetry == "" { - return fmt.Errorf("must specify at least one of: --proxy-id, --clear-proxy, --profile-id, --profile-name, --viewport, or --telemetry") + if !hasProxyChange && !hasProfileChange && !hasViewportChange && in.Telemetry == "" && !hasNameChange && !hasTagsChange { + return fmt.Errorf("must specify at least one of: --proxy-id, --clear-proxy, --profile-id, --profile-name, --viewport, --telemetry, --name, --clear-name, --tag, or --clear-tags") } params := kernel.BrowserUpdateParams{} + // Handle name changes + if in.ClearName { + params.Name = kernel.Opt("") + } else if in.SetName { + params.Name = kernel.Opt(in.Name) + } + + // Handle tag changes. Tags are a full replace, not a merge: providing --tag + // replaces the entire set, and --clear-tags removes all tags. + if in.ClearTags { + params.Tags = kernel.Tags{} + } else if len(in.Tags) > 0 { + params.Tags = kernel.Tags(in.Tags) + } + // Handle proxy changes if in.ClearProxy { params.ProxyID = kernel.Opt("") @@ -781,6 +830,12 @@ func (b BrowsersCmd) Update(ctx context.Context, in BrowsersUpdateInput) error { } pterm.Success.Printf("Updated browser %s\n", browser.SessionID) + if hasNameChange { + pterm.Info.Printf("Name: %s\n", util.OrDash(browser.Name)) + } + if hasTagsChange { + pterm.Info.Printf("Tags: %s\n", util.OrDash(formatTags(browser.Tags))) + } if in.Telemetry != "" { printTelemetrySummary(browser.Telemetry) } @@ -2338,8 +2393,12 @@ Supported operations: - Load a profile into a session that doesn't have one (--profile-id or --profile-name) - Change viewport dimensions (--viewport) - Force viewport resize during active live view or recording (--force with --viewport) + - Rename or clear the session name (--name or --clear-name) + - Replace or clear the session tags (--tag or --clear-tags) -Note: Profiles can only be loaded into sessions that don't already have a profile.`, +Notes: + - Profiles can only be loaded into sessions that don't already have a profile. + - --tag replaces the entire tag set (it is not merged with existing tags).`, Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { return fmt.Errorf("missing required argument: browser ID or name\n\nUsage: kernel browsers update [flags]") @@ -2379,6 +2438,10 @@ func init() { browsersUpdateCmd.Flags().String("viewport", "", "Browser viewport size (e.g., 1920x1080@25). Supported: 2560x1440@10, 1920x1080@25, 1920x1200@25, 1440x900@25, 1024x768@60, 1200x800@60, 1280x800@60") browsersUpdateCmd.Flags().Bool("force", false, "Force viewport resize even when a live view or recording/replay is active") browsersUpdateCmd.Flags().String("telemetry", "", "Update telemetry: --telemetry=all (reset to default set), --telemetry=off (disable), or --telemetry=console,network (merge those categories into the current selection)") + browsersUpdateCmd.Flags().String("name", "", "Set a new unique name for the browser session (mutually exclusive with --clear-name)") + browsersUpdateCmd.Flags().Bool("clear-name", false, "Clear the browser session name") + browsersUpdateCmd.Flags().StringArray("tag", nil, "Set a tag KEY=VALUE (repeatable; up to 50 pairs). Replaces the entire tag set; mutually exclusive with --clear-tags") + browsersUpdateCmd.Flags().Bool("clear-tags", false, "Remove all tags from the browser session") browsersCmd.AddCommand(browsersListCmd) browsersCmd.AddCommand(browsersCreateCmd) @@ -2645,7 +2708,7 @@ func init() { 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", "", "Configure telemetry (opt-in): --telemetry=all (default set), --telemetry=off (disable), or --telemetry=console,network (capture exactly those categories)") - browsersCreateCmd.Flags().String("name", "", "Optional unique name for the browser session (used to find it later; set at creation only)") + browsersCreateCmd.Flags().String("name", "", "Optional unique name for the browser session (used to find it later; can be changed with 'browsers update --name')") browsersCreateCmd.Flags().StringArray("tag", nil, "Set a tag KEY=VALUE on the session (repeatable; up to 50 pairs)") browsersCreateCmd.Flags().String("chrome-policy", "", "Custom Chrome enterprise policy as a JSON object") browsersCreateCmd.Flags().String("chrome-policy-file", "", "Read Chrome enterprise policy (JSON object) from a file (use '-' for stdin)") @@ -2921,6 +2984,10 @@ func runBrowsersUpdate(cmd *cobra.Command, args []string) error { viewport, _ := cmd.Flags().GetString("viewport") force, _ := cmd.Flags().GetBool("force") telemetry, _ := cmd.Flags().GetString("telemetry") + name, _ := cmd.Flags().GetString("name") + clearName, _ := cmd.Flags().GetBool("clear-name") + tags := tagsFromFlag(cmd, "tag") + clearTags, _ := cmd.Flags().GetBool("clear-tags") svc := client.Browsers b := BrowsersCmd{browsers: &svc} @@ -2934,6 +3001,12 @@ func runBrowsersUpdate(cmd *cobra.Command, args []string) error { Viewport: viewport, Force: force, Telemetry: telemetry, + Name: name, + SetName: cmd.Flags().Changed("name"), + ClearName: clearName, + Tags: tags, + SetTags: cmd.Flags().Changed("tag"), + ClearTags: clearTags, Output: out, }) } diff --git a/cmd/browsers_test.go b/cmd/browsers_test.go index f283130..aecf979 100644 --- a/cmd/browsers_test.go +++ b/cmd/browsers_test.go @@ -2086,3 +2086,263 @@ func TestBrowsersUpdate_ForceWithProxyButNoViewport_Errors(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "--force requires --viewport") } + +func captureUpdateParams(t *testing.T) (*FakeBrowsersService, *kernel.BrowserUpdateParams) { + t.Helper() + captured := &kernel.BrowserUpdateParams{} + fake := &FakeBrowsersService{UpdateFunc: func(ctx context.Context, id string, body kernel.BrowserUpdateParams, opts ...option.RequestOption) (*kernel.BrowserUpdateResponse, error) { + *captured = body + return &kernel.BrowserUpdateResponse{SessionID: "session123", Name: body.Name.Value, Tags: body.Tags}, nil + }} + return fake, captured +} + +func TestBrowsersUpdate_WithName_ForwardsParam(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + Name: "new-name", + SetName: true, + }) + + assert.NoError(t, err) + assert.True(t, captured.Name.Valid()) + assert.Equal(t, "new-name", captured.Name.Value) +} + +func TestBrowsersUpdate_ClearName_SendsEmptyName(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + ClearName: true, + }) + + assert.NoError(t, err) + raw, marshalErr := json.Marshal(*captured) + require.NoError(t, marshalErr) + assert.Contains(t, string(raw), `"name":""`) +} + +func TestBrowsersUpdate_WithTags_ReplacesTagSet(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + Tags: map[string]string{"team": "backend", "env": "staging"}, + }) + + assert.NoError(t, err) + assert.Equal(t, "backend", captured.Tags["team"]) + assert.Equal(t, "staging", captured.Tags["env"]) + assert.Len(t, captured.Tags, 2) +} + +func TestBrowsersUpdate_ClearTags_SendsEmptyObject(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + ClearTags: true, + }) + + assert.NoError(t, err) + raw, marshalErr := json.Marshal(*captured) + require.NoError(t, marshalErr) + assert.Contains(t, string(raw), `"tags":{}`) +} + +func TestBrowsersUpdate_NameAndClearName_Errors(t *testing.T) { + setupStdoutCapture(t) + fake := &FakeBrowsersService{} + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + Name: "x", + SetName: true, + ClearName: true, + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot specify both --name and --clear-name") +} + +func TestBrowsersUpdate_TagAndClearTags_Errors(t *testing.T) { + setupStdoutCapture(t) + fake := &FakeBrowsersService{} + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + Tags: map[string]string{"a": "1"}, + ClearTags: true, + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot specify both --tag and --clear-tags") +} + +func TestBrowsersUpdate_EmptyName_WithoutClear_Errors(t *testing.T) { + setupStdoutCapture(t) + fake := &FakeBrowsersService{} + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + Name: "", + SetName: true, + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "use --clear-name") +} + +func TestBrowsersUpdate_NameOnly_SatisfiesAtLeastOne(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + Name: "renamed", + SetName: true, + }) + + assert.NoError(t, err) + assert.Equal(t, "renamed", captured.Name.Value) +} + +func TestBrowsersUpdate_NoOptions_Errors(t *testing.T) { + setupStdoutCapture(t) + fake := &FakeBrowsersService{} + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{Identifier: "session123"}) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "must specify at least one") +} + +func TestBrowsersUpdate_NameAndTagsWithProxy_AllForwarded(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + ProxyID: "proxy-123", + Name: "combo", + SetName: true, + Tags: map[string]string{"k": "v"}, + SetTags: true, + }) + + assert.NoError(t, err) + assert.Equal(t, "combo", captured.Name.Value) + assert.Equal(t, "v", captured.Tags["k"]) + assert.Equal(t, "proxy-123", captured.ProxyID.Value) +} + +// Regression guard: a non-name/non-tags update must omit both fields entirely +// (omit = leave unchanged), never sending an accidental empty name or tags. +func TestBrowsersUpdate_OmitNameAndTags_NotSent(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + ProxyID: "proxy-123", + }) + + assert.NoError(t, err) + assert.False(t, captured.Name.Valid()) + assert.Nil(t, captured.Tags) + raw, marshalErr := json.Marshal(*captured) + require.NoError(t, marshalErr) + assert.NotContains(t, string(raw), `"name"`) + assert.NotContains(t, string(raw), `"tags"`) +} + +func TestBrowsersUpdate_ClearNameWithSetTags_BothForwarded(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + ClearName: true, + Tags: map[string]string{"env": "prod"}, + SetTags: true, + }) + + assert.NoError(t, err) + raw, marshalErr := json.Marshal(*captured) + require.NoError(t, marshalErr) + assert.Contains(t, string(raw), `"name":""`) + assert.Contains(t, string(raw), `"env":"prod"`) +} + +func TestBrowsersUpdate_SetNameWithClearTags_BothForwarded(t *testing.T) { + setupStdoutCapture(t) + fake, captured := captureUpdateParams(t) + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + Name: "renamed", + SetName: true, + ClearTags: true, + }) + + assert.NoError(t, err) + raw, marshalErr := json.Marshal(*captured) + require.NoError(t, marshalErr) + assert.Contains(t, string(raw), `"name":"renamed"`) + assert.Contains(t, string(raw), `"tags":{}`) +} + +// A malformed-only --tag (tagsFromFlag drops it to nil) combined with +// --clear-tags must still be rejected as contradictory, not silently clear. +func TestBrowsersUpdate_MalformedTagWithClearTags_Errors(t *testing.T) { + setupStdoutCapture(t) + fake := &FakeBrowsersService{} + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + SetTags: true, // --tag was provided... + Tags: nil, // ...but every value was malformed and dropped + ClearTags: true, + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot specify both --tag and --clear-tags") +} + +// --tag provided but every value malformed (parsed to zero pairs) is a user +// error, not a no-op or a misleading "must specify at least one" message. +func TestBrowsersUpdate_AllMalformedTags_Errors(t *testing.T) { + setupStdoutCapture(t) + fake := &FakeBrowsersService{} + b := BrowsersCmd{browsers: fake} + + err := b.Update(context.Background(), BrowsersUpdateInput{ + Identifier: "session123", + SetTags: true, + Tags: nil, + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "no valid --tag") +} diff --git a/go.mod b/go.mod index 838c1b7..a014ca3 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/charmbracelet/lipgloss/v2 v2.0.0-beta.1 github.com/golang-jwt/jwt/v5 v5.2.2 github.com/joho/godotenv v1.5.1 - github.com/kernel/kernel-go-sdk v0.65.0 + github.com/kernel/kernel-go-sdk v0.66.0 github.com/klauspost/compress v1.18.5 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c github.com/pterm/pterm v0.12.80 diff --git a/go.sum b/go.sum index 5591e94..0059b0f 100644 --- a/go.sum +++ b/go.sum @@ -64,8 +64,8 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= -github.com/kernel/kernel-go-sdk v0.65.0 h1:/ASWrUxqKjpvDd9LOQ++H6Bp4Cp/lyu2SUSO3K8VzRE= -github.com/kernel/kernel-go-sdk v0.65.0/go.mod h1:EeZzSuHZVeHKxKCPUzxou2bovNGhXaz0RXrSqKNf1AQ= +github.com/kernel/kernel-go-sdk v0.66.0 h1:pn+fSHHo4fJ4kYm8uOkF5J2rj6k1FC6NqlLzoxy2jy4= +github.com/kernel/kernel-go-sdk v0.66.0/go.mod h1:EeZzSuHZVeHKxKCPUzxou2bovNGhXaz0RXrSqKNf1AQ= github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg=