amadan

carlos / platform Public

cli: .carlos/config account key; rewrites preserve unknown fields

configAccount walks like configDefault but keys off the account field,
reporting the same file's console for the pairing rule. saveConfigFile
and the new saveConfigAccount rewrite through writeConfigKeys, which
keeps fields this binary doesn't know — the old drop-on-rewrite made
every future key lossy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell e0eaab84aa25a9b6f31eff05a962705f055d3352 parent 3b62160
2 files changed, +208 −23
  • config.go +112 −23
  • config_test.go +96 −0
diff --git a/config.go b/config.go
index b11c3171c..2b5c60370 100644
--- a/config.go
+++ b/config.go
@@ -34,38 +34,52 @@ import (
// only; no other verb ever assumes it.
const defaultConsole = "https://platform.carlosframework.com"
-// cliConfig is the whole file. Unknown fields are ignored on read and
-// dropped on rewrite: the file is additive, and the CLI is its only parser.
+// cliConfig is the whole file. Unknown fields are ignored on read and — since
+// the account key landed — PRESERVED on rewrite (writeConfigKeys): the file
+// is additive, other tools may learn to read it, and a rewrite that drops
+// what it doesn't understand makes every future key lossy against the binary
+// that predates it.
type cliConfig struct {
Console string `json:"console"`
+ Account string `json:"account"`
}
-// loadConfigFile reads one .carlos/config. A missing file, or one without a
-// console, is ("", nil): "no default at this level", and resolution
-// continues. A file that cannot be parsed, or names a console
+// loadConfig reads one .carlos/config whole. A missing file is a zero
+// config, not an error. A file that cannot be parsed, or names a console
// normalizeConsole refuses, is an ERROR — silently ignoring a typo'd
// config would ship to the wrong console, the exact mistake this file
-// exists to prevent.
-func loadConfigFile(path string) (string, error) {
+// exists to prevent. The console comes back normalized ("" when unset);
+// the account comes back trimmed.
+func loadConfig(path string) (cliConfig, error) {
raw, err := os.ReadFile(path)
if errors.Is(err, os.ErrNotExist) {
- return "", nil
+ return cliConfig{}, nil
}
if err != nil {
- return "", err
+ return cliConfig{}, err
}
var cfg cliConfig
if err := json.Unmarshal(raw, &cfg); err != nil {
- return "", fmt.Errorf("%s is not readable as a carlos config: %w", path, err)
+ return cliConfig{}, fmt.Errorf("%s is not readable as a carlos config: %w", path, err)
}
if strings.TrimSpace(cfg.Console) == "" {
- return "", nil
- }
- n, err := normalizeConsole(cfg.Console)
- if err != nil {
- return "", fmt.Errorf("%s: %w", path, err)
+ cfg.Console = ""
+ } else {
+ n, err := normalizeConsole(cfg.Console)
+ if err != nil {
+ return cliConfig{}, fmt.Errorf("%s: %w", path, err)
+ }
+ cfg.Console = n
}
- return n, nil
+ cfg.Account = strings.TrimSpace(cfg.Account)
+ return cfg, nil
+}
+
+// loadConfigFile is loadConfig's console-only view — the shape every
+// console-default caller predating the account key still reads.
+func loadConfigFile(path string) (string, error) {
+ cfg, err := loadConfig(path)
+ return cfg.Console, err
}
// configDefault resolves the configured default console: the nearest
@@ -111,6 +125,49 @@ func configDefault() (console, source string, err error) {
return c, path, nil
}
+// configAccount resolves the configured default ACCOUNT: the nearest
+// .carlos/config walking up from the working directory whose account key is
+// set, else ~/.carlos/config. A level that names only a console does not
+// stop this walk — console and account are separate defaults that happen to
+// share a file. Returns the account, the file it came from, and that SAME
+// file's console ("" when it names none): the pairing rule lives at the
+// caller (apimode.go defaultAccount), which honors a file's account only
+// for the console the file itself names — a config pair points at one
+// stack, and -console to a different stack must not drag the other stack's
+// account along.
+func configAccount() (account, source, fileConsole string, err error) {
+ if dir, werr := os.Getwd(); werr == nil {
+ for {
+ path := filepath.Join(dir, ".carlos", "config")
+ cfg, err := loadConfig(path)
+ if err != nil {
+ return "", "", "", err
+ }
+ if cfg.Account != "" {
+ return cfg.Account, path, cfg.Console, nil
+ }
+ parent := filepath.Dir(dir)
+ if parent == dir {
+ break
+ }
+ dir = parent
+ }
+ }
+ path, perr := globalConfigPath()
+ if perr != nil {
+ // No home directory means no global config — a state, not an error.
+ return "", "", "", nil
+ }
+ cfg, err := loadConfig(path)
+ if err != nil {
+ return "", "", "", err
+ }
+ if cfg.Account == "" {
+ return "", "", "", nil
+ }
+ return cfg.Account, path, cfg.Console, nil
+}
+
// globalConfigPath is ~/.carlos/config — the same directory that holds
// credentials, though this file is not a secret and never holds one.
func globalConfigPath() (string, error) {
@@ -121,22 +178,54 @@ func globalConfigPath() (string, error) {
return filepath.Join(home, ".carlos", "config"), nil
}
-// saveConfigFile validates the console and writes {"console": …} at path,
-// creating the .carlos directory (dirPerm: 0700 for ~/.carlos, which
-// MkdirAll leaves alone if it already exists; 0755 for a project directory
-// that is meant to be committed). Temp-file-then-rename like
-// credentialFile.save, so a crash cannot leave a truncated config — but
-// 0644, not 0600: this file is not a secret.
+// saveConfigFile validates the console and writes the "console" key at
+// path, preserving everything else the file holds (writeConfigKeys).
+// dirPerm: 0700 for ~/.carlos, which MkdirAll leaves alone if it already
+// exists; 0755 for a project directory that is meant to be committed.
func saveConfigFile(path, console string, dirPerm os.FileMode) error {
n, err := normalizeConsole(console)
if err != nil {
return err
}
+ return writeConfigKeys(path, map[string]any{"console": n}, dirPerm)
+}
+
+// saveConfigAccount writes a console+account pair — the chooser's "save as
+// this project's default" answer. The two keys land together deliberately:
+// configAccount's pairing rule reads them as one fact ("this checkout means
+// this account on this console"), so saving the account without pinning the
+// console would leave a default that silently stops applying the day the
+// directory's console default changes.
+func saveConfigAccount(path, console, account string, dirPerm os.FileMode) error {
+ n, err := normalizeConsole(console)
+ if err != nil {
+ return err
+ }
+ return writeConfigKeys(path, map[string]any{"console": n, "account": account}, dirPerm)
+}
+
+// writeConfigKeys rewrites path with the given keys set, keeping every
+// field it does not understand — a rewrite that drops unknown keys makes
+// every future config key lossy against the binary that predates it.
+// Temp-file-then-rename like credentialFile.save, so a crash cannot leave
+// a truncated config — but 0644, not 0600: this file is not a secret.
+func writeConfigKeys(path string, set map[string]any, dirPerm os.FileMode) error {
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, dirPerm); err != nil {
return err
}
- body, err := json.MarshalIndent(cliConfig{Console: n}, "", " ")
+ all := map[string]any{}
+ if raw, err := os.ReadFile(path); err == nil {
+ if err := json.Unmarshal(raw, &all); err != nil {
+ return fmt.Errorf("%s is not readable as a carlos config: %w", path, err)
+ }
+ } else if !errors.Is(err, os.ErrNotExist) {
+ return err
+ }
+ for k, v := range set {
+ all[k] = v
+ }
+ body, err := json.MarshalIndent(all, "", " ")
if err != nil {
return err
}
diff --git a/config_test.go b/config_test.go
index 4e835c085..54bf00f56 100644
--- a/config_test.go
+++ b/config_test.go
@@ -2,6 +2,7 @@ package main
import (
"bytes"
+ "encoding/json"
"os"
"path/filepath"
"strings"
@@ -325,3 +326,98 @@ func TestAuthDefaultVerb(t *testing.T) {
t.Fatal("two positional args must be a usage error")
}
}
+
+func TestConfigAccountWalk(t *testing.T) {
+ useTempCredentials(t)
+ home := os.Getenv("HOME")
+
+ // Nothing anywhere: no account default, no error.
+ if a, src, cons, err := configAccount(); err != nil || a != "" || src != "" || cons != "" {
+ t.Fatalf("bare configAccount = %q %q %q %v, want empty", a, src, cons, err)
+ }
+
+ // Only the global, and the file names no console: a pure account
+ // default, fileConsole empty.
+ globalPath := writeConfig(t, home, `{"account": "acme"}`)
+ a, src, cons, err := configAccount()
+ if err != nil || a != "acme" || src != globalPath || cons != "" {
+ t.Fatalf("global-only = %q %q %q %v", a, src, cons, err)
+ }
+
+ // A nearer file with only a console does not stop the ACCOUNT walk.
+ root := t.TempDir()
+ writeConfig(t, root, `{"console": "https://outer.example"}`)
+ nested := filepath.Join(root, "cmd", "deep")
+ if err := os.MkdirAll(nested, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ t.Chdir(nested)
+ if a, _, _, err = configAccount(); err != nil || a != "acme" {
+ t.Fatalf("console-only level = %q %v, want the walk to continue to acme", a, err)
+ }
+
+ // The nearest account-bearing file wins and reports ITS console —
+ // normalized, because the pairing rule compares it to a session console.
+ nearest := writeConfig(t, nested, `{"console": "https://inner.example/", "account": "tools"}`)
+ a, src, cons, err = configAccount()
+ if err != nil || a != "tools" || src != nearest || cons != "https://inner.example" {
+ t.Fatalf("nearest = %q %q %q %v", a, src, cons, err)
+ }
+
+ // Malformed config mid-walk is an error, not a skip.
+ os.WriteFile(nearest, []byte(`not json`), 0o644)
+ if _, _, _, err = configAccount(); err == nil {
+ t.Fatal("malformed project config must be an error, not a skip")
+ }
+}
+
+func TestConfigRewritesPreserveUnknownFields(t *testing.T) {
+ useTempCredentials(t)
+ dir := t.TempDir()
+ path := filepath.Join(dir, ".carlos", "config")
+
+ // A fresh saveConfigAccount writes both keys.
+ if err := saveConfigAccount(path, "https://tito.example/", "acme", 0o755); err != nil {
+ t.Fatal(err)
+ }
+ cfg, err := loadConfig(path)
+ if err != nil || cfg.Console != "https://tito.example" || cfg.Account != "acme" {
+ t.Fatalf("fresh save = %+v, %v", cfg, err)
+ }
+
+ // A file holding a key from the future keeps it across BOTH rewrites —
+ // the old drop-on-rewrite behavior made every new key lossy.
+ if err := os.WriteFile(path, []byte(`{"console":"https://tito.example","future_key":42}`), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := saveConfigAccount(path, "https://tito.example", "tools", 0o755); err != nil {
+ t.Fatal(err)
+ }
+ raw, err := os.ReadFile(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ var all map[string]any
+ if err := json.Unmarshal(raw, &all); err != nil {
+ t.Fatal(err)
+ }
+ if all["future_key"] != float64(42) || all["account"] != "tools" {
+ t.Fatalf("saveConfigAccount dropped fields: %v", all)
+ }
+ if err := saveConfigFile(path, "https://other.example/", 0o755); err != nil {
+ t.Fatal(err)
+ }
+ raw, _ = os.ReadFile(path)
+ all = map[string]any{}
+ if err := json.Unmarshal(raw, &all); err != nil {
+ t.Fatal(err)
+ }
+ if all["future_key"] != float64(42) || all["account"] != "tools" || all["console"] != "https://other.example" {
+ t.Fatalf("saveConfigFile dropped fields: %v", all)
+ }
+
+ // The console is still validated on write, account or not.
+ if err := saveConfigAccount(path, "ftp://nope", "x", 0o755); err == nil {
+ t.Fatal("saving an invalid console must fail")
+ }
+}