From d9ed2ad3c5dda2c8032d8afb5b582aa6fef707e1 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Fri, 25 Sep 2026 12:12:11 -0500 Subject: [PATCH] fix: Keep plugins working after upgrading A user whose login predates the session config, and whose first command after upgrading is a plugin, got an empty API host, so the plugin could not reach Datum Cloud. Plugin setup now resolves the account the same way docs openapi and auth get-token do, which creates the missing session from the stored login. The switch test now stores credentials for the previous account, so it fails on the old code with that account's host rather than an empty one. Tests that build plugin settings start from an empty mock keyring so they never read a developer's real credentials. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/plugindispatch/dispatch.go | 17 ++++----- internal/plugindispatch/dispatch_test.go | 45 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/internal/plugindispatch/dispatch.go b/internal/plugindispatch/dispatch.go index 3918d56..5b8f0ed 100644 --- a/internal/plugindispatch/dispatch.go +++ b/internal/plugindispatch/dispatch.go @@ -154,16 +154,17 @@ func BuildEnv(factory *client.DatumCloudFactory) ([]string, error) { // DATUM_* variables always describe the same account — the one // CurrentContext/ActiveSession resolves to — rather than pairing a // current session name with a stale, unrelated keyring host. + // GetUserKeyForCurrentSession also creates the session for users whose + // login predates the session config, so their first command can be a + // plugin. sessionName := "" apiHost := "" - if cfg, cfgErr := datumconfig.LoadAuto(); cfgErr == nil && cfg != nil { - if session := cfg.ActiveSessionEntry(); session != nil { - sessionName = session.Name - if session.Endpoint.Server != "" { - apiHost = datumconfig.StripScheme(session.Endpoint.Server) - } else if host, hostErr := authutil.GetAPIHostnameForUser(session.UserKey); hostErr == nil { - apiHost = host - } + if userKey, session, err := authutil.GetUserKeyForCurrentSession(); err == nil && session != nil { + sessionName = session.Name + if session.Endpoint.Server != "" { + apiHost = datumconfig.StripScheme(session.Endpoint.Server) + } else if host, hostErr := authutil.GetAPIHostnameForUser(userKey); hostErr == nil { + apiHost = host } } diff --git a/internal/plugindispatch/dispatch_test.go b/internal/plugindispatch/dispatch_test.go index a9313fd..06e7901 100644 --- a/internal/plugindispatch/dispatch_test.go +++ b/internal/plugindispatch/dispatch_test.go @@ -25,6 +25,9 @@ func buildMinimalFactory(t *testing.T) *client.DatumCloudFactory { tmpHome := t.TempDir() t.Setenv("HOME", tmpHome) t.Setenv("USERPROFILE", tmpHome) // Windows compat + // BuildEnv reads the keyring; start from an empty mock so neither the + // developer's real credentials nor another test's entries leak in. + keyring.MockInit() f, err := client.NewDatumFactory(context.Background()) if err != nil { @@ -178,6 +181,7 @@ func TestBuildEnv_sessionPropagated(t *testing.T) { tmpHome := t.TempDir() t.Setenv("HOME", tmpHome) t.Setenv("USERPROFILE", tmpHome) + keyring.MockInit() cfgDir := filepath.Join(tmpHome, ".datumctl") if err := os.MkdirAll(cfgDir, 0o755); err != nil { @@ -232,6 +236,10 @@ func TestBuildEnv_FollowsSwitchedAccountNotLegacyKeyring(t *testing.T) { // Simulate the account that logged in last, before the switch — this is // what the legacy keyring marker names, and what a call site reading it // directly would still use. + oldCreds := `{"token":{"access_token":"x"},"hostname":"auth.datum.net","api_hostname":"api.datum.net","user_email":"old@example.com"}` + if err := keyring.Set(authutil.ServiceName, "old@example.com@auth.datum.net", oldCreds); err != nil { + t.Fatalf("seed old account credentials: %v", err) + } if err := keyring.Set(authutil.ServiceName, authutil.ActiveUserKey, "old@example.com@auth.datum.net"); err != nil { t.Fatalf("seed legacy active_user: %v", err) } @@ -280,6 +288,43 @@ func TestBuildEnv_FollowsSwitchedAccountNotLegacyKeyring(t *testing.T) { } } +// TestBuildEnv_LegacyLoginWithoutSessionConfig covers a user whose login +// predates the session config: credentials sit in the keyring and the config +// has no sessions. A plugin run as their first command must still receive the +// API host from those credentials. +func TestBuildEnv_LegacyLoginWithoutSessionConfig(t *testing.T) { + // Not parallel — uses t.Setenv and the mock keyring. + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("USERPROFILE", tmpHome) + keyring.MockInit() + + creds := `{"token":{"access_token":"x"},"hostname":"auth.datum.net","api_hostname":"api.datum.net","user_email":"user@example.com"}` + if err := keyring.Set(authutil.ServiceName, "user@example.com", creds); err != nil { + t.Fatalf("seed credentials: %v", err) + } + if err := keyring.Set(authutil.ServiceName, authutil.ActiveUserKey, "user@example.com"); err != nil { + t.Fatalf("seed legacy active_user: %v", err) + } + + f, err := client.NewDatumFactory(context.Background()) + if err != nil { + t.Fatalf("NewDatumFactory: %v", err) + } + + env, err := BuildEnv(f) + if err != nil { + t.Fatalf("BuildEnv: %v", err) + } + + if got := envValue(env, "DATUM_API_HOST"); got != "api.datum.net" { + t.Errorf("DATUM_API_HOST=%q, want %q", got, "api.datum.net") + } + if got := envValue(env, "DATUM_SESSION"); got == "" { + t.Error("DATUM_SESSION is empty, want the session created from the legacy login") + } +} + // TestBuildEnv_sessionEmptyWhenNone verifies that DATUM_SESSION is present in // the env slice but set to "" when no active session is configured. func TestBuildEnv_sessionEmptyWhenNone(t *testing.T) {