Refactor `pc config` to support key-based configuration and commands by austin-denoble · Pull Request #90 · pinecone-io/cli

austin-denoble · 2026-05-28T17:14:23Z

Problem

The pc config command exposed configuration through a flat set of verb-noun subcommands — get-api-key, set-api-key, set-color, set-environment — with one Cobra command file per setting. The pattern had a few problems:

Adding a new setting required adding a new command file and registering it in two places (the config command tree and the root-level auth-skip list).
No way to view all current settings at once — no list equivalent.
No way to describe a setting (its purpose, valid values, sensitivity).
The naming diverged from how most modern CLIs (gh, aws, npm, stripe) handle config, raising the learning curve for new users.

This becomes more painful as the set of configurable values grows.

Solution

Refactor pc config to a generic, registry-driven interface modeled after gh config. A single keyDescriptor in internal/pkg/cli/command/config/registry.go holds each setting's getter, setter, clear function, optional onChange side-effect hook, short and long descriptions, sensitivity flag, and valid values. The get / set / unset / list / describe commands are generic and dispatch through the registry — adding a new config key is now a single registry entry, no new command file required.

New commands

$ pc config list
KEY           VALUE        DESCRIPTION
api-key       <not set>    Default API key for authenticating with Pinecone
environment   production   Pinecone environment to target (production or staging)
color         true         Enable or disable colored terminal output

$ pc config get api-key
[INFO] api-key: <not set>

$ pc config set api-key pcsk_...
[SUCCESS] api-key updated

$ pc config set environment staging
[SUCCESS] environment updated
[INFO] You have been logged out; to login again, run `pc login`
[INFO] To set a new API key, run `pc config set api-key <value>`

$ pc config unset api-key
[SUCCESS] api-key cleared

$ pc config describe environment
KEY            environment
VALUE          production
SENSITIVE      false
VALID VALUES   production, staging
DESCRIPTION    Pinecone environment to target (production or staging)

Select which Pinecone environment the CLI talks to. Most users should leave this set to 'production'; 'staging' is intended for Pinecone internal development.

Changing the environment clears your existing authentication state: any OAuth session is logged out, the default API key is cleared, and the target organization and project are reset. You will need to re-authenticate
and re-target after switching.

Registry-driven extensibility

Each setting is one entry in configRegistry. Adding a new key — say, a default output format — would look like:

"output-format": {
    Description: "Default output format for commands",
    ValidValues: []string{"text", "json"},
    getStr: func() string { return conf.OutputFormat.Get() },
    setStr: func(value string) error {
        switch value {
        case "text", "json":
            conf.OutputFormat.Set(value)
            return nil
        default:
            return fmt.Errorf("invalid value %q; must be one of: text, json", value)
        }
    },
    clearStr: func() { conf.OutputFormat.Clear() },
},

No new command file. No changes to root.go's skip-auth list. It just works across get, set, unset, list, and describe.

Side-effect hooks

The environment key needs to clear OAuth state, the API key, and the target org/project when it changes. That used to live inline in set-environment. It now lives in an onChange hook on the registry entry and fires consistently from both set and unset:

onChange: func(ctx context.Context, _, _ string) []string {
    // Logout OAuth, clear API key, clear target org/project,
    // return human-readable info lines for the user.
}

Flags

--json on get, list, describe for machine-readable output.
--reveal on get, list, describe to unmask sensitive values (defaults to masking head/tail for api-key).

Backwards compatibility

The four legacy commands (get-api-key, set-api-key, set-color, set-environment) are preserved as hidden, deprecated aliases. They continue to function and print Cobra's standard deprecation notice
pointing at the new equivalent:

$ pc config set-color true
Command "set-color" is deprecated, use 'pc config set color <true|false>' instead
[SUCCESS] Config property color updated to true

Existing scripts and docs keep working; new users land on the new pattern.

Type of Change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

The legacy commands are kept as hidden aliases, so this is non-breaking for existing users. Docs referencing pc config set-api-key etc. should be updated to use pc config set api-key <value> going forward.

Test Plan

Note

Medium Risk
Environment onChange clears OAuth, API keys, and targets on set/unset—behavior change vs legacy paths for unset; API key handling remains sensitive but follows existing masking patterns.

Overview
Replaces per-setting pc config subcommands with generic get, set, unset, list, and describe that dispatch through a central configRegistry (keyDescriptor entries for api-key, color, environment). New keys are added via registry entries instead of new command files.

ConfigService backs the handlers with validation, ErrNoChange idempotency, and onChange hooks (environment changes now clear OAuth, API key, and target org/project on both set and unset). get / list / describe support --json and --reveal for sensitive values; set / unset support --json.

Legacy get-api-key, set-api-key, set-color, and set-environment remain as hidden, deprecated aliases; help text points users to pc config set api-key. root.go adds the new subcommands to the auth-skip list. Broad unit tests cover masking, JSON output, and registry behavior.

^{Reviewed by Cursor Bugbot for commit 8936497. Bugbot is set up for automated code reviews on this repo. Configure here.}

…mands Replaces the per-setting subcommands (get-api-key, set-api-key, set-color, set-environment) with a generic, registry-driven interface modeled after `gh config` and `aws configure`. A new keyDescriptor in registry.go holds each setting's getter, setter, validator, side-effect hook, short/long descriptions, and sensitivity flag. Adding a new key is supported through a registry entry rather than a command file. Adds `pc config get|set|unset|list|describe <key>`, with --json and --reveal flag support for sensitive values like API keys. The four legacy commands are maintained as hidden, deprecated aliases to maintain functionality for existing users.

`pc config unset` now invokes the onChange hook when clearing a key changes its value, fixing a bug where `unset environment` on staging silently resets to production without clearing the staging-tied OAuth session, API key, or target org/project. `pc config set` now passes the canonical stored value to onChange via a post-set getStr() read, rather than the raw user input. Hooks like environment's now see "production" instead of "prod" when the input is normalized during setStr.

Extract business logic from cobra Run blocks into run* functions that accept a ConfigService interface, enabling unit testing without touching the real viper config store. Add --json output to set and unset commands for machine-readable use. Add a Hidden field to keyDescriptor to suppress internal keys (environment) from list output and tab completion while keeping them accessible by name.

… onChange: add specific fields to keyDescriptor for validateStr, persistStr, and onChange. make sure config set|unset call validateStr, onChange, and then persistStr to avoid onChange failing after persistence has already occurred

…ually checked in ValidateStr

cursor · 2026-05-31T21:46:09Z

+			case "production", "staging":
+				// canonical values
+			default:
+				return "", fmt.Errorf("invalid environment %q; must be one of: production, staging", value)


Environment prod alias dropped

Medium Severity

pc config set environment rejects prod, while the hidden legacy set-environment command still maps prod to production. The new registry path only accepts the literals production and staging, so shorthand that worked before fails on the primary command.

^{Reviewed by Cursor Bugbot for commit 1a6a4f9. Configure here.}

cursor · 2026-05-31T21:46:09Z

+		}
+	}
+	desc.persistStr(normalizedVal)
+	return lines, nil


Environment side effects before persist

Medium Severity

For environment, Set and Unset run onChange (OAuth logout, API key and target clears with immediate disk writes) before persistStr updates the environment field. If the final environment write fails or the process exits afterward, auth and targets can be wiped while the stored environment never changes.

Additional Locations (1)

internal/pkg/cli/command/config/registry.go#L267-L276

^{Reviewed by Cursor Bugbot for commit 1a6a4f9. Configure here.}

cursor · 2026-05-31T21:46:09Z

+		},
+		persistStr: func(value string) {
+			secrets.DefaultAPIKey.Set(value)
+		},


Empty api-key allowed on set

Low Severity

The api-key registry entry has no validateStr, so pc config set api-key accepts an empty value and persists it. The deprecated set-api-key command rejects empty input, and the registry comment implies nil ValidValues means any non-empty string.

^{Reviewed by Cursor Bugbot for commit 1a6a4f9. Configure here.}

cursor · 2026-05-31T21:46:09Z

+		if lines, err = desc.onChange(ctx, oldVal, newVal); err != nil {
+			return nil, err
+		}
+	}


Env binding retriggers environment hook

Medium Severity

environment uses conf.Environment.Get() for unset no-op checks and ErrNoChange, but that getter includes the bound PINECONE_ENVIRONMENT variable. When the env var overrides the file, Unset/Set toward production can keep seeing a non-default effective value, so the destructive onChange hook runs on every attempt even after the file already holds the target value.

Additional Locations (1)

internal/pkg/cli/command/config/registry.go#L114-L124

^{Reviewed by Cursor Bugbot for commit 1a6a4f9. Configure here.}

cursor

Cursor Bugbot has reviewed your changes and found 5 potential issues.

There are 9 total unresolved issues (including 4 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8936497. Configure here.}

cursor · 2026-05-31T23:11:16Z

+			target organization and project are reset. You will need to re-authenticate
+			and re-target after switching.
+		`),
+		Hidden:      true,


environment omitted from list

Medium Severity

The environment setting is marked Hidden, and List only includes visibleKeys(), so pc config list omits environment even though the PR describes listing all three settings including environment.

Additional Locations (1)

internal/pkg/cli/command/config/registry.go#L278-L291

^{Reviewed by Cursor Bugbot for commit 8936497. Configure here.}

cursor · 2026-05-31T23:11:16Z

+		}
+	}
+	desc.persistStr(newVal)
+	return lines, nil


unset api-key skips auth state

Medium Severity

pc config unset api-key only clears the stored API key via persistStr and never reconciles state.AuthedUser, unlike pc auth clear --api-key, so the in-process auth context can stay on API-key mode after the key was removed.

^{Reviewed by Cursor Bugbot for commit 8936497. Configure here.}

cursor · 2026-05-31T23:11:16Z

+			token, err := oauth.Token(ctx)
+			if err != nil {
+				return nil, fmt.Errorf("error retrieving oauth token: %w", err)
+			}


Token error blocks env change

Medium Severity

The environment onChange hook requires oauth.Token to succeed before set or unset can finish, so a stale or broken OAuth record can prevent changing or resetting environment even when the user only relies on an API key.

^{Reviewed by Cursor Bugbot for commit 8936497. Configure here.}

cursor · 2026-05-31T23:11:16Z

+	}
+
+	if opts.json {
+		fmt.Fprintln(os.Stdout, text.IndentJSON(setOutput{Key: keyName, Value: value}))


JSON set shows raw value

Low Severity

With --json, pc config set prints the user-supplied value from the CLI args instead of the normalized value persisted by the registry (for example on vs true for color).

^{Reviewed by Cursor Bugbot for commit 8936497. Configure here.}

cursor · 2026-05-31T23:11:17Z

+
+	if opts.json {
+		fmt.Fprintln(os.Stdout, text.IndentJSON(setOutput{Key: keyName, Value: value}))
+		return nil


JSON hides auth side effects

High Severity

With --json, successful pc config set and pc config unset return only key metadata and exit before printing onChange lines, so environment changes that log out OAuth, clear the API key, and reset targets complete silently for JSON consumers.

Additional Locations (1)

internal/pkg/cli/command/config/unset.go#L66-L69

^{Reviewed by Cursor Bugbot for commit 8936497. Configure here.}

cursor Bot reviewed May 28, 2026

View reviewed changes

Comment thread internal/pkg/cli/command/config/unset.go Outdated

Comment thread internal/pkg/cli/command/config/set.go Outdated

pinecone-projects Bot assigned austin-denoble May 28, 2026

cursor Bot reviewed May 31, 2026

View reviewed changes

Comment thread internal/pkg/cli/command/config/registry.go

Comment thread internal/pkg/cli/command/config/registry.go

Comment thread internal/pkg/cli/command/config/registry.go Outdated

Comment thread internal/pkg/cli/command/config/get.go

austin-denoble added 4 commits May 31, 2026 17:27

use msg.FailJSON in get, describe, and list config commands

5bfe55c

make sure visibleKeys() preserves the ordering of the listed keys

f499f0a

clean up confusing aliasing around ValidValues and how things are act…

1a6a4f9

…ually checked in ValidateStr

cursor Bot reviewed May 31, 2026

View reviewed changes

clean up error message when using unset and the value is unchanged

8936497

cursor Bot reviewed May 31, 2026

View reviewed changes

Conversation

austin-denoble commented May 28, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

New commands

Registry-driven extensibility

Side-effect hooks

Flags

Backwards compatibility

Type of Change

Test Plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

Environment prod alias dropped

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

Environment side effects before persist

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

Empty api-key allowed on set

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

Env binding retriggers environment hook

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

environment omitted from list

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

unset api-key skips auth state

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

Token error blocks env change

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

JSON set shows raw value

Uh oh!

cursor Bot May 31, 2026

Choose a reason for hiding this comment

JSON hides auth side effects

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

austin-denoble commented May 28, 2026 •

edited by cursor Bot

Loading