Skip to content

Refactor pc config to support key-based configuration and commands#90

Merged
austin-denoble merged 20 commits into
mainfrom
adenoble/improve-configuration-path
Jun 3, 2026
Merged

Refactor pc config to support key-based configuration and commands#90
austin-denoble merged 20 commits into
mainfrom
adenoble/improve-configuration-path

Conversation

@austin-denoble
Copy link
Copy Markdown
Collaborator

@austin-denoble austin-denoble commented May 28, 2026

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, validator, persister, optional onChange side-effect hook, short and long descriptions, sensitivity flag, valid values, and env var binding. 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       ENV VAR NAME        ENV VAR OVERRIDE   DESCRIPTION
api-key   <not set>   PINECONE_API_KEY     false              Default API key for authenticating with Pinecone
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 to pcsk_***...***

$ pc config set environment staging
[SUCCESS] environment updated to staging
[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
ENV VAR NAME       $PINECONE_ENVIRONMENT
ENV VAR OVERRIDE   false
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.
...

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"},
    defaultVal:  "text",
    getStr: func() string { return conf.OutputFormat.GetStored() },
    validateStr: func(value string) (string, error) {
        switch value {
        case "text", "json":
            return value, nil
        default:
            return "", fmt.Errorf("invalid value %q; must be one of: text, json", value)
        }
    },
    persistStr: func(value string) { conf.OutputFormat.Set(value) },
},

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

Both environment and api-key carry onChange hooks:

  • environment clears OAuth state, the API key, and the target org/project when it changes, then returns human-readable info lines surfaced to the user (and in --json output).
  • api-key reconciles state.AuthedUser.AuthContext when a key is set or cleared — switching to AuthDefaultAPIKey, falling back through service-account or OAuth, or clearing to AuthNone.

Hooks fire from both set and unset. They are skipped when an env var is actively overriding the key — writing the config file doesn't change the runtime value in that case, so auth side effects would be incorrect.

Env var override visibility

Keys with an env var binding (PINECONE_API_KEY, PINECONE_ENVIRONMENT) now surface override state in all read commands. get, list, and describe report the env var name and whether it's currently overriding the stored value. describe --json includes env_var_name and env_var_override fields; long_description is omitted from JSON output (it's help text for human display only).

Flags

  • --json on get, set, unset, list, describe for machine-readable output. For set and unset, JSON output includes any onChange side-effect messages so machine consumers can observe auth state changes.
  • --reveal on get, list, describe to unmask sensitive values (defaults to masking head/tail for api-key).
  • --all on list to include hidden keys (e.g. environment).

Color aliases

color accepts true/false, on/off, and 1/0 — all normalized to true/false on write.

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

  • just test-unit passes
  • pc config list — confirms all keys render with descriptions and env var columns
  • pc config list --all — confirms hidden keys (environment) appear
  • pc config get api-key — confirms <not set> placeholder for empty values
  • pc config get api-key --reveal — confirms full value shown when revealed
  • pc config set environment staging — confirms OAuth/API key/target wipe fires
  • pc config unset environment (from staging) — confirms same wipe fires
  • pc config set environment prod — confirms canonical production is stored and reported
  • pc config set color on / off / 1 / 0 — confirms aliases normalize correctly
  • pc config set api-key <value> — confirms AuthContext flips to AuthDefaultAPIKey
  • pc config unset api-key — confirms AuthContext falls back correctly
  • pc config describe api-key / environment / color — confirms ENV VAR rows present for bound keys, long description rendered below table
  • PINECONE_API_KEY=x pc config get api-key — confirms ENV VAR OVERRIDE: true shown
  • PINECONE_ENVIRONMENT=staging pc config set environment staging — confirms config file updated, no auth side effects fired
  • pc config set api-key <value> --json / pc config set environment staging --json — confirms structured output includes onChange messages
  • pc config describe api-key --json — confirms long_description absent from JSON output
  • Legacy pc config get-api-key / set-api-key / set-color / set-environment — confirms each still works and prints deprecation notice
  • --json on get, list, describe — confirms structured output

…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.
Comment thread internal/pkg/cli/command/config/unset.go Outdated
Comment thread internal/pkg/cli/command/config/set.go Outdated
`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.
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
… 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
Comment thread internal/pkg/cli/command/config/unset.go
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
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
Comment thread internal/pkg/cli/command/config/registry.go Outdated
Comment thread internal/pkg/cli/command/config/set.go Outdated
Comment thread internal/pkg/cli/command/config/set.go
…t reading from viper and bypassing any configured env setting
… state.AuthedUser appropriately if configuring an API key
- Output the normalized stored value rather than the raw user input in pc config set --json (e.g. true instead of on for color aliases)
- Include onChange side-effect messages in JSON output for both set and unset so environment changes (logout, key/target clear) are visible to machine consumers, not silently dropped
- Treat an unreadable OAuth token record as no active session in the environment onChange hook rather than blocking the operation entirely, unblocking users who authenticate via API key
Comment thread internal/pkg/cli/command/config/set.go
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
Comment thread internal/pkg/cli/command/config/set.go Outdated
Comment thread internal/pkg/cli/command/config/registry.go
…e them in output

When PINECONE_API_KEY or PINECONE_ENVIRONMENT is active, config set and unset now write the stored preference without running onChange side effects (auth logout, credential clears), since the env var controls the runtime value regardless of what is in the config file.

Config output (get, list, describe) now exposes env_var_name and env_var_override on all keys with an env var binding so users can see when a setting is being overridden and by which variable.
@austin-denoble
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d96200e. Configure here.

@austin-denoble austin-denoble merged commit 9fa3ef2 into main Jun 3, 2026
8 checks passed
@austin-denoble austin-denoble deleted the adenoble/improve-configuration-path branch June 3, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants