From 8839ba31a4cd407b3b04cf796e3f198d270d37f5 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 28 Jul 2026 21:12:51 +0100 Subject: [PATCH 01/10] feat: `flagsmith evaluate` shows what an SDK resolves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Evaluation runs through the Flagsmith Go SDK against the SDK API, so the answer is the one an application would get: segment overrides applied, variants allocated, an identity and its traits overlaid. The SDK evaluates over the CLI's own transport, which brings retries, credential-stripping redirects and FLAGSMITH_DEBUG tracing with it. Its User-Agent is now stamped over whatever a request carries rather than only filling in a missing one, so a library that sets its own still reaches the API as the CLI. Its logger is discarded — it writes debug lines straight to os.Stderr, past the command's writers. Identity evaluation is a read-only what-if: `transient: true` unless --persist says otherwise, so neither the identity nor the traits are stored. Traits without an identity evaluate the anonymous identity. The environment context may name the environment rather than carry its key, for `evaluate` and `api --sdk` alike, as it does for every Admin command. The local name cache turns a name into a key for free, so the common case reaches the SDK API with no Admin credential at all; only a name the cache has never seen costs a lookup, and that lookup seeds the cache. A ref of 22 alphanumerics is taken for a key without consulting anything — that is what the backend mints, and a key nothing has cached yet must not need credentials. The cache spans an instance rather than a project, so only a unique name match short-circuits; anything else defers to the Admin API, which scopes the name properly. beep boop --- go.mod | 10 +- go.sum | 20 +- internal/cmd/api.go | 17 +- internal/cmd/cmd_test.go | 684 +++++++++++++++++++++++++++++++++- internal/cmd/errors.go | 4 + internal/cmd/evaluate.go | 212 +++++++++++ internal/cmd/resolve.go | 52 +++ internal/httpx/client.go | 9 +- internal/httpx/client_test.go | 14 +- 9 files changed, 985 insertions(+), 37 deletions(-) create mode 100644 internal/cmd/evaluate.go diff --git a/go.mod b/go.mod index 0bb0e35..3e8ab87 100644 --- a/go.mod +++ b/go.mod @@ -10,13 +10,15 @@ require ( github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.9 github.com/zalando/go-keyring v0.2.8 - golang.org/x/sync v0.15.0 + golang.org/x/sync v0.16.0 golang.org/x/term v0.45.0 ) require ( + github.com/Flagsmith/flagsmith-go-client/v5 v5.1.0 // indirect github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect + github.com/blang/semver/v4 v4.0.0 // indirect github.com/catppuccin/go v0.3.0 // indirect github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 // indirect github.com/charmbracelet/bubbletea v1.3.6 // indirect @@ -31,9 +33,11 @@ require ( github.com/danieljoos/wincred v1.2.3 // indirect github.com/dustin/go-humanize v1.0.1 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect + github.com/go-resty/resty/v2 v2.17.2 // indirect github.com/godbus/dbus/v5 v5.2.2 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/itchyny/timefmt-go v0.1.8 // indirect + github.com/itlightning/dateparse v0.2.1 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-colorable v0.1.14 // indirect github.com/mattn/go-isatty v0.0.20 // indirect @@ -43,8 +47,10 @@ require ( github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect github.com/muesli/cancelreader v0.2.2 // indirect github.com/muesli/termenv v0.16.0 // indirect + github.com/ohler55/ojg v1.28.1 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect + golang.org/x/net v0.43.0 // indirect golang.org/x/sys v0.47.0 // indirect - golang.org/x/text v0.23.0 // indirect + golang.org/x/text v0.28.0 // indirect ) diff --git a/go.sum b/go.sum index a17cbe6..5128a21 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/Flagsmith/flagsmith-go-client/v5 v5.1.0 h1:/1KMxWHfzo8I7FnZ0mnMw+ICM4HVZvCEYTJ4idM2VEU= +github.com/Flagsmith/flagsmith-go-client/v5 v5.1.0/go.mod h1:CjTs0XAIJXG+08lf4h51W9cjvrNaK92TLVYtjPrnI3w= github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/atotto/clipboard v0.1.4 h1:EH0zSVneZPSuFR11BlR9YppQTVDbh5+16AmcJi4g1z4= @@ -6,6 +8,8 @@ github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiE github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= github.com/aymanbagabas/go-udiff v0.3.1 h1:LV+qyBQ2pqe0u42ZsUEtPiCaUoqgA9gYRDs3vj1nolY= github.com/aymanbagabas/go-udiff v0.3.1/go.mod h1:G0fsKmG+P6ylD0r6N/KgQD/nWzgfnl8ZBcNLgcbrw8E= +github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= +github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/catppuccin/go v0.3.0 h1:d+0/YicIq+hSTo5oPuRi5kOpqkVA5tAsU6dNhvRu+aY= github.com/catppuccin/go v0.3.0/go.mod h1:8IHJuMGaUUjQM82qBrGNBv7LFq6JI3NnQCF6MOlZjpc= github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 h1:JFgG/xnwFfbezlUnFMJy0nusZvytYysV4SCS2cYbvws= @@ -53,6 +57,8 @@ github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6 github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/fatih/color v1.19.0 h1:Zp3PiM21/9Ld6FzSKyL5c/BULoe/ONr9KlbYVOfG8+w= github.com/fatih/color v1.19.0/go.mod h1:zNk67I0ZUT1bEGsSGyCZYZNrHuTkJJB+r6Q9VuMi0LE= +github.com/go-resty/resty/v2 v2.17.2 h1:FQW5oHYcIlkCNrMD2lloGScxcHJ0gkjshV3qcQAyHQk= +github.com/go-resty/resty/v2 v2.17.2/go.mod h1:kCKZ3wWmwJaNc7S29BRtUhJwy7iqmn+2mLtQrOyQlVA= github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -61,6 +67,8 @@ github.com/itchyny/gojq v0.12.19 h1:ttXA0XCLEMoaLOz5lSeFOZ6u6Q3QxmG46vfgI4O0DEs= github.com/itchyny/gojq v0.12.19/go.mod h1:5galtVPDywX8SPSOrqjGxkBeDhSxEW1gSxoy7tn1iZY= github.com/itchyny/timefmt-go v0.1.8 h1:1YEo1JvfXeAHKdjelbYr/uCuhkybaHCeTkH8Bo791OI= github.com/itchyny/timefmt-go v0.1.8/go.mod h1:5E46Q+zj7vbTgWY8o5YkMeYb4I6GeWLFnetPy5oBrAI= +github.com/itlightning/dateparse v0.2.1 h1:AB0NJTyI0HYcerEUMovKZOiQVBg1mBPxgAnWQwzLP6g= +github.com/itlightning/dateparse v0.2.1/go.mod h1:xHlmL8lT0L9JIBlaKotRwsoDYpKJskXpiU9ZwbbSkNA= github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= @@ -79,6 +87,8 @@ github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELU github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= +github.com/ohler55/ojg v1.28.1 h1:Xy93DelhLSZNeWv8GPKtP6qMqkUlZlAxBP/AQcC5RfY= +github.com/ohler55/ojg v1.28.1/go.mod h1:/Y5dGWkekv9ocnUixuETqiL58f+5pAsUfg5P8e7Pa2o= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -101,8 +111,10 @@ github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cma go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= -golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= -golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/net v0.43.0 h1:lat02VYK2j4aLzMzecihNvTlJNQUq316m2Mr9rnM6YE= +golang.org/x/net v0.43.0/go.mod h1:vhO1fvI4dGsIjh73sWfUVjj3N7CA9WkKJNQm2svM6Jg= +golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= +golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -110,8 +122,8 @@ golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= -golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY= -golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4= +golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= +golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/cmd/api.go b/internal/cmd/api.go index f1b33aa..dabe225 100644 --- a/internal/cmd/api.go +++ b/internal/cmd/api.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -97,7 +96,7 @@ func runAPI(cmd *cobra.Command, args []string) error { // apiTarget resolves the base URL and the auth applier for the chosen surface. func apiTarget(cmd *cobra.Command, pc *projectContext) (string, func(*http.Request), error) { if apiSDKFlag { - key, err := sdkEnvironmentKey(pc) + key, err := sdkEnvironmentKey(cmd, pc) if err != nil { return "", nil, err } @@ -112,20 +111,6 @@ func apiTarget(cmd *cobra.Command, pc *projectContext) (string, func(*http.Reque return apiURL, cred.auth.Apply, nil } -// sdkEnvironmentKey returns the client- or server-side key for --sdk calls, -// taken verbatim from the environment context (no Admin name resolution). -func sdkEnvironmentKey(pc *projectContext) (string, error) { - sdkURL, _ := pc.SDKAPIURL.Value.(string) - if _, v := envCredential(envEnvironmentKey, sdkURL, defaultSDKAPIURL); v != "" { - return v, nil - } - if k, ok := pc.Environment.Value.(string); ok && k != "" { - return k, nil - } - return "", withHint(errors.New("no environment key"), - "Set FLAGSMITH_ENVIRONMENT_KEY, or pass -e.") -} - // apiRequestBody builds the request body (or query params) and resolves the // method: explicit --method, else POST when a body/field is present, else GET. func apiRequestBody(cmd *cobra.Command) (body []byte, contentType string, query url.Values, method string, err error) { diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 53a02cd..d7ece49 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -85,6 +85,7 @@ func resetFlags() { apiHeaderFlags = nil apiFieldFlags = nil apiRawFields = nil + evalTraitFlags = nil } // setEnvCred exports a credential variable host-scoped to url, as a @@ -162,6 +163,7 @@ type fakeInstance struct { idLookupCalls int // count of GET /identities/ (identifier lookups) edgeLookups int // count of GET /edge-identities/ (uuid lookups) envGetCalls int // count of GET /environments/{key}/ (retrieve) + envListCalls int // count of GET /environments/ list calls projGetCalls int // count of GET /projects/{id}/ (retrieve) orgListCalls int // count of GET /organisations/ list calls tokenPosts int // count of POST /o/token/ (refresh) calls @@ -187,6 +189,19 @@ type fakeInstance struct { featureSegments map[string][]map[string]any // feature id -> feature-segment rows (priority order) featureStates map[string][]map[string]any // feature id -> admin featurestates rows + // The SDK API surface `flagsmith evaluate` reads. sdkEnvFlags is keyed by + // environment key — an unrecognised key gets a 401, as the real SDK API + // does; sdkIdentityFlags overrides it per identifier ("" is the anonymous + // identity), so an identity evaluation can be told apart from the + // environment defaults. + sdkEnvFlags map[string][]map[string]any + sdkIdentityFlags map[string][]map[string]any + lastIdentify map[string]any // last POST /api/v1/identities/ body + sdkUserAgents []string // User-Agent of every SDK API request + sdkKeys []string // X-Environment-Key of every SDK API request + sdkStatus int // when non-zero, the SDK endpoints answer with it + sdkDelay time.Duration // artificial latency for the SDK endpoints + nextFeatureID int lastFeatureBody map[string]any // last feature create/update body nextMVID int @@ -295,12 +310,16 @@ func newFakeInstance(t *testing.T) *fakeInstance { }}}, }, }, - nextSegmentID: 100, - nextFeatureID: 900, - nextMVID: 300, - nextOrgID: 20, - serverKeys: map[string][]map[string]any{}, - nextServerKeyID: 500, + sdkEnvFlags: map[string][]map[string]any{ + "WqXhZk8sVY3dGgTqZ9pJmN": sdkFlagsFrom(defaultFeatures()), + }, + sdkIdentityFlags: map[string][]map[string]any{}, + nextSegmentID: 100, + nextFeatureID: 900, + nextMVID: 300, + nextOrgID: 20, + serverKeys: map[string][]map[string]any{}, + nextServerKeyID: 500, } mux := http.NewServeMux() mux.HandleFunc("GET /.well-known/oauth-authorization-server", func(w http.ResponseWriter, r *http.Request) { @@ -393,6 +412,9 @@ func newFakeInstance(t *testing.T) *fakeInstance { json.NewEncoder(w).Encode(proj) }) mux.HandleFunc("GET /api/v1/environments/", func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + f.envListCalls++ + f.mu.Unlock() if !authorized(r) { w.WriteHeader(http.StatusUnauthorized) return @@ -1283,6 +1305,49 @@ func newFakeInstance(t *testing.T) *fakeInstance { f.mu.Unlock() json.NewEncoder(w).Encode(map[string]any{"results": results}) }) + // The SDK API: the two endpoints the Flagsmith SDK evaluates flags over. + mux.HandleFunc("GET /api/v1/flags/", func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + f.sdkUserAgents = append(f.sdkUserAgents, r.Header.Get("User-Agent")) + f.sdkKeys = append(f.sdkKeys, r.Header.Get("X-Environment-Key")) + status, flags, delay := f.sdkStatus, f.sdkEnvFlags[r.Header.Get("X-Environment-Key")], f.sdkDelay + f.mu.Unlock() + time.Sleep(delay) + if status != 0 { + w.WriteHeader(status) + return + } + if flags == nil { + w.WriteHeader(http.StatusUnauthorized) + return + } + json.NewEncoder(w).Encode(flags) + }) + mux.HandleFunc("POST /api/v1/identities/", func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + identifier, _ := body["identifier"].(string) + f.mu.Lock() + f.sdkUserAgents = append(f.sdkUserAgents, r.Header.Get("User-Agent")) + f.sdkKeys = append(f.sdkKeys, r.Header.Get("X-Environment-Key")) + f.lastIdentify = body + status, flags := f.sdkStatus, f.sdkEnvFlags[r.Header.Get("X-Environment-Key")] + if override, ok := f.sdkIdentityFlags[identifier]; ok { + flags = override + } + f.mu.Unlock() + if status != 0 { + w.WriteHeader(status) + return + } + if flags == nil { + w.WriteHeader(http.StatusUnauthorized) + return + } + json.NewEncoder(w).Encode(map[string]any{ + "identifier": identifier, "traits": body["traits"], "flags": flags, + }) + }) // echo reflects the request back, for exercising `flagsmith api`. mux.HandleFunc("/api/v1/echo/", func(w http.ResponseWriter, r *http.Request) { if !authorized(r) && r.Header.Get("X-Environment-Key") == "" { @@ -1493,6 +1558,45 @@ func defaultFeatures() []map[string]any { } } +// sdkFlagsFrom renders admin feature fixtures as the SDK API's flags payload — +// the shape `flagsmith evaluate` reads. +func sdkFlagsFrom(features []map[string]any) []map[string]any { + flags := make([]map[string]any, 0, len(features)) + for _, item := range features { + state, _ := item["environment_feature_state"].(map[string]any) + flags = append(flags, map[string]any{ + "enabled": state["enabled"], + "feature_state_value": state["feature_state_value"], + "feature": map[string]any{"id": item["id"], "name": item["name"]}, + }) + } + return flags +} + +func (f *fakeInstance) identifyBody() map[string]any { + f.mu.Lock() + defer f.mu.Unlock() + return f.lastIdentify +} + +func (f *fakeInstance) sdkAgents() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.sdkUserAgents...) +} + +func (f *fakeInstance) sdkSentKeys() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.sdkKeys...) +} + +func (f *fakeInstance) environmentLists() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.envListCalls +} + func (f *fakeInstance) refreshCount() int { f.mu.Lock() defer f.mu.Unlock() @@ -1530,6 +1634,26 @@ func TestServerSideKeyNeverEchoed(t *testing.T) { } }) + t.Run("a key the SDK API rejects is not echoed", func(t *testing.T) { + // Given a key the SDK surface does not know, so evaluation gets a 401. + isolateStorage(t) + f := newFakeInstance(t) + root := tempRepo(t) + writeConfig(t, root, `{"apiUrl": "`+f.srv.URL+`"}`) + setEnvCred(t, envEnvironmentKey, f.srv.URL, "ser.SuperSecret123") + + // When + out, err := run("", "evaluate") + + // Then + if err == nil { + t.Fatal("err = nil, want an error") + } + if strings.Contains(out, "SuperSecret123") || strings.Contains(hintFor(err), "SuperSecret123") { + t.Errorf("output = %q — the server-side key leaked", out) + } + }) + t.Run("a positional ser. ref is refused without echoing it", func(t *testing.T) { // Given flagUpdateEnv(t) @@ -5781,6 +5905,554 @@ func TestEnvironmentKey(t *testing.T) { }) } +// evalEnv writes a config naming Development's client-side key, which doubles as +// the SDK credential. No Admin credential and no project are set: `evaluate` +// talks only to the SDK API, so every test here also proves it needs neither. +func evalEnv(t *testing.T) *fakeInstance { + t.Helper() + isolateStorage(t) + f := newFakeInstance(t) + root := tempRepo(t) + writeConfig(t, root, `{"environment": "WqXhZk8sVY3dGgTqZ9pJmN", "apiUrl": "`+f.srv.URL+`"}`) + return f +} + +// captureOSStderr collects what run writes to the process's stderr — where a +// library's own logger lands, bypassing the command's writers entirely. +func captureOSStderr(t *testing.T, run func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + saved := os.Stderr + os.Stderr = w + run() + os.Stderr = saved + w.Close() + logged, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + r.Close() + return string(logged) +} + +func TestEvaluate(t *testing.T) { + t.Run("human table with count", func(t *testing.T) { + // Given + evalEnv(t) + + // When + out, err := run("", "evaluate") + + // Then + if err != nil { + t.Fatalf("evaluate: %v\noutput: %s", err, out) + } + for _, want := range []string{ + "FEATURE", "ENABLED", "VALUE", + "onboarding_banner", "on", "-", + "max_items", "off", "25", "2 flags", + } { + if !strings.Contains(out, want) { + t.Errorf("output = %q, want it to contain %q", out, want) + } + } + }) + + t.Run("the SDK's own logging never reaches stderr", func(t *testing.T) { + // Given + evalEnv(t) + + // When + var err error + logged := captureOSStderr(t, func() { _, err = run("", "evaluate") }) + + // Then + if err != nil { + t.Fatalf("evaluate: %v", err) + } + if logged != "" { + t.Errorf("stderr = %q, want the SDK's logger silenced", logged) + } + }) + + t.Run("the invocation deadline bounds evaluation", func(t *testing.T) { + // Given an SDK API slower than the deadline the user asked for. + f := evalEnv(t) + f.sdkDelay = 1200 * time.Millisecond + t.Setenv("FLAGSMITH_TIMEOUT", "1") + + // When + _, err := run("", "evaluate") + + // Then + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("err = %v, want the deadline to bound the SDK's request", err) + } + }) + + t.Run("json is a curated array", func(t *testing.T) { + // Given + evalEnv(t) + + // When + out, err := run("", "evaluate", "--json") + + // Then + if err != nil { + t.Fatalf("evaluate --json: %v\noutput: %s", err, out) + } + var flags []map[string]any + if err := json.Unmarshal([]byte(out), &flags); err != nil { + t.Fatalf("parsing %q: %v", out, err) + } + if len(flags) != 2 { + t.Fatalf("flags = %+v", flags) + } + if flags[0]["feature"] != "onboarding_banner" || flags[0]["enabled"] != true || flags[0]["value"] != nil { + t.Errorf("item = %+v, want the resolved flag hoisted", flags[0]) + } + if flags[1]["value"] != float64(25) { + t.Errorf("item = %+v, want the resolved value", flags[1]) + } + if len(flags[0]) != 3 { + t.Errorf("item = %+v, want feature/enabled/value only", flags[0]) + } + }) + + t.Run("eval is an accepted alias", func(t *testing.T) { + // Given + evalEnv(t) + + // When + out, err := run("", "eval") + + // Then + if err != nil { + t.Fatalf("eval: %v\noutput: %s", err, out) + } + if !strings.Contains(out, "2 flags") { + t.Errorf("output = %q", out) + } + }) + + t.Run("a single feature renders a detail view", func(t *testing.T) { + // Given + evalEnv(t) + + // When + out, err := run("", "evaluate", "max_items") + + // Then + if err != nil { + t.Fatalf("evaluate max_items: %v\noutput: %s", err, out) + } + for _, want := range []string{"Feature", "max_items", "Enabled", "off", "Value", "25"} { + if !strings.Contains(out, want) { + t.Errorf("output = %q, want %q", out, want) + } + } + if strings.Contains(out, "onboarding_banner") { + t.Errorf("output = %q, want only the named feature", out) + } + }) + + t.Run("an unknown feature errors with a hint", func(t *testing.T) { + // Given + evalEnv(t) + + // When + _, err := run("", "evaluate", "nope") + + // Then + if err == nil { + t.Fatal("err = nil, want an error") + } + if !strings.Contains(err.Error(), `"nope"`) { + t.Errorf("err = %v, want it to name the feature", err) + } + if !strings.Contains(hintFor(err), "flagsmith eval") { + t.Errorf("hint = %q, want it to point at the listing", hintFor(err)) + } + }) + + t.Run("no flags", func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkEnvFlags["WqXhZk8sVY3dGgTqZ9pJmN"] = []map[string]any{} + + // When + out, err := run("", "evaluate") + + // Then + if err != nil { + t.Fatalf("evaluate: %v\noutput: %s", err, out) + } + if !strings.Contains(out, "No flags.") { + t.Errorf("output = %q", out) + } + }) + + t.Run("without an environment key it names the way in", func(t *testing.T) { + // Given + isolateStorage(t) + f := newFakeInstance(t) + root := tempRepo(t) + writeConfig(t, root, `{"apiUrl": "`+f.srv.URL+`"}`) + + // When + _, err := run("", "evaluate") + + // Then + if err == nil { + t.Fatal("err = nil, want an error") + } + if !strings.Contains(hintFor(err), "FLAGSMITH_ENVIRONMENT_KEY") { + t.Errorf("hint = %q, want it to name the variable", hintFor(err)) + } + }) + + t.Run("FLAGSMITH_ENVIRONMENT_KEY evaluates on its own", func(t *testing.T) { + // Given a config with no environment at all — the SDK credential is the + // only environment reference, as it is in CI. + isolateStorage(t) + f := newFakeInstance(t) + root := tempRepo(t) + writeConfig(t, root, `{"apiUrl": "`+f.srv.URL+`"}`) + setEnvCred(t, envEnvironmentKey, f.srv.URL, "ser.serverSideSecret") + f.sdkEnvFlags["ser.serverSideSecret"] = sdkFlagsFrom(defaultFeatures()) + + // When + out, err := run("", "evaluate") + + // Then + if err != nil { + t.Fatalf("evaluate: %v\noutput: %s", err, out) + } + if !strings.Contains(out, "2 flags") { + t.Errorf("output = %q", out) + } + }) + + t.Run("the CLI's User-Agent replaces the SDK's", func(t *testing.T) { + // Given + f := evalEnv(t) + + // When + if _, err := run("", "evaluate"); err != nil { + t.Fatalf("evaluate: %v", err) + } + + // Then + agents := f.sdkAgents() + if len(agents) != 1 || agents[0] != version.UserAgent() { + t.Errorf("User-Agent = %q, want the CLI's %q", agents, version.UserAgent()) + } + }) + + t.Run("an unexpected status is reported without a stack of SDK prefixes", func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkStatus = http.StatusInternalServerError + + // When + _, err := run("", "evaluate") + + // Then + if err == nil { + t.Fatal("err = nil, want an error") + } + if strings.Contains(err.Error(), "flagsmith:") { + t.Errorf("err = %v, want the SDK's own wording dropped", err) + } + if !strings.Contains(err.Error(), "500") { + t.Errorf("err = %v, want it to carry the status", err) + } + }) +} + +// The environment context is a client-side key or a name, as it is everywhere +// else. Evaluation needs the key, and a name only costs an Admin lookup when the +// local name cache cannot answer — so the common case stays credential-free. +func TestEvaluateResolvesEnvironmentName(t *testing.T) { + const productionKey = "K2mVsGdXhZ8kQqZ9pJmNbJ" + + // nameEnv points the context at an environment by name, with no Admin + // credential; production is the only environment the SDK API answers for. + nameEnv := func(t *testing.T, ref string) *fakeInstance { + t.Helper() + isolateStorage(t) + f := newFakeInstance(t) + root := tempRepo(t) + writeConfig(t, root, `{"project": 101, "environment": "`+ref+`", "apiUrl": "`+f.srv.URL+`"}`) + delete(f.sdkEnvFlags, "WqXhZk8sVY3dGgTqZ9pJmN") + f.sdkEnvFlags[productionKey] = sdkFlagsFrom(defaultFeatures()) + return f + } + + t.Run("a cached name resolves without an Admin credential", func(t *testing.T) { + // Given + f := nameEnv(t, "Production") + cache.Merge(f.srv.URL, &cache.Names{Environments: map[string]string{productionKey: "Production"}}) + + // When + out, err := run("", "evaluate") + + // Then + if err != nil { + t.Fatalf("evaluate: %v\noutput: %s", err, out) + } + if got := f.sdkSentKeys(); len(got) != 1 || got[0] != productionKey { + t.Errorf("X-Environment-Key = %q, want the named environment's key", got) + } + if got := f.environmentLists(); got != 0 { + t.Errorf("environment list calls = %d, want the cache to answer for free", got) + } + }) + + t.Run("-e takes a name too", func(t *testing.T) { + // Given + f := nameEnv(t, "WqXhZk8sVY3dGgTqZ9pJmN") + cache.Merge(f.srv.URL, &cache.Names{Environments: map[string]string{productionKey: "Production"}}) + + // When + out, err := run("", "evaluate", "-e", "Production") + + // Then + if err != nil { + t.Fatalf("evaluate -e: %v\noutput: %s", err, out) + } + if got := f.sdkSentKeys(); len(got) != 1 || got[0] != productionKey { + t.Errorf("X-Environment-Key = %q, want the named environment's key", got) + } + }) + + t.Run("an uncached name costs one Admin lookup, and is remembered", func(t *testing.T) { + // Given + f := nameEnv(t, "Production") + setMasterKey(t, f.srv.URL) + withEnvironments(f) + + // When + out, err := run("", "evaluate") + + // Then + if err != nil { + t.Fatalf("evaluate: %v\noutput: %s", err, out) + } + if got := f.sdkSentKeys(); len(got) != 1 || got[0] != productionKey { + t.Errorf("X-Environment-Key = %q, want the resolved key", got) + } + if got := f.environmentLists(); got != 1 { + t.Errorf("environment list calls = %d, want exactly one", got) + } + if got := cache.Load(f.srv.URL).Environments[productionKey]; got != "Production" { + t.Errorf("cache = %q, want the name remembered for next time", got) + } + }) + + t.Run("an ambiguous cached name defers to the Admin API", func(t *testing.T) { + // Given two projects' environments sharing a name in one instance-wide + // cache. Only the Admin API can scope the name to this project. + f := nameEnv(t, "Development") + setMasterKey(t, f.srv.URL) + withEnvironments(f) + f.sdkEnvFlags["WqXhZk8sVY3dGgTqZ9pJmN"] = sdkFlagsFrom(defaultFeatures()) + cache.Merge(f.srv.URL, &cache.Names{Environments: map[string]string{ + "WqXhZk8sVY3dGgTqZ9pJmN": "Development", + "OtherProjectDevKey1234": "Development", + }}) + + // When + out, err := run("", "evaluate") + + // Then + if err != nil { + t.Fatalf("evaluate: %v\noutput: %s", err, out) + } + if got := f.sdkSentKeys(); len(got) != 1 || got[0] != "WqXhZk8sVY3dGgTqZ9pJmN" { + t.Errorf("X-Environment-Key = %q, want this project's Development", got) + } + }) + + t.Run("an uncached name without credentials says how to authenticate", func(t *testing.T) { + // Given + nameEnv(t, "Production") + + // When + _, err := run("", "evaluate") + + // Then + if !errors.Is(err, auth.ErrNotLoggedIn) { + t.Errorf("err = %v, want the login error — a name cannot be resolved without one", err) + } + }) + + t.Run("a key is never resolved, cache or no cache", func(t *testing.T) { + // Given a key the cache has never seen and no credentials at all. + f := nameEnv(t, productionKey) + + // When + out, err := run("", "evaluate") + + // Then + if err != nil { + t.Fatalf("evaluate: %v\noutput: %s", err, out) + } + if got := f.environmentLists(); got != 0 { + t.Errorf("environment list calls = %d, want a key used as-is", got) + } + }) +} + +func TestEvaluateIdentity(t *testing.T) { + // The identity's own flags, distinguishable from the environment defaults. + overridden := func() []map[string]any { + flags := sdkFlagsFrom(defaultFeatures()) + flags[1]["enabled"], flags[1]["feature_state_value"] = true, 99 + return flags + } + + t.Run("resolves for an identity, transiently by default", func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkIdentityFlags["user-123"] = overridden() + + // When + out, err := run("", "evaluate", "--identity", "user-123") + + // Then + if err != nil { + t.Fatalf("evaluate --identity: %v\noutput: %s", err, out) + } + if !strings.Contains(out, "99") { + t.Errorf("output = %q, want the identity's resolved value", out) + } + body := f.identifyBody() + if body["identifier"] != "user-123" || body["transient"] != true { + t.Errorf("body = %+v, want a transient identity", body) + } + }) + + t.Run("traits are typed by inference", func(t *testing.T) { + // Given + f := evalEnv(t) + + // When + out, err := run("", "evaluate", "--identity", "user-123", + "--trait", "plan=premium", "--trait", "age=42", "--trait", "score=1.5", "--trait", "beta=true") + + // Then + if err != nil { + t.Fatalf("evaluate --trait: %v\noutput: %s", err, out) + } + want := map[string]any{"plan": "premium", "age": float64(42), "score": 1.5, "beta": true} + got := map[string]any{} + for _, item := range f.identifyBody()["traits"].([]any) { + trait := item.(map[string]any) + got[trait["trait_key"].(string)] = trait["trait_value"] + } + if len(got) != len(want) { + t.Fatalf("traits = %+v, want %+v", got, want) + } + for k, v := range want { + if got[k] != v { + t.Errorf("trait %s = %#v, want %#v", k, got[k], v) + } + } + }) + + t.Run("--persist opts into persistence", func(t *testing.T) { + // Given + f := evalEnv(t) + + // When + if _, err := run("", "eval", "--identity", "user-123", "--trait", "plan=premium", "--persist"); err != nil { + t.Fatalf("evaluate --persist: %v", err) + } + + // Then + if body := f.identifyBody(); body["transient"] != false { + t.Errorf("body = %+v, want transient dropped", body) + } + }) + + t.Run("traits alone evaluate an anonymous identity", func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkIdentityFlags[""] = overridden() + + // When + out, err := run("", "evaluate", "--trait", "country=US") + + // Then + if err != nil { + t.Fatalf("evaluate --trait: %v\noutput: %s", err, out) + } + body := f.identifyBody() + if body["identifier"] != "" || body["transient"] != true { + t.Errorf("body = %+v, want an anonymous transient identity", body) + } + if !strings.Contains(out, "99") { + t.Errorf("output = %q, want the anonymous identity's flags", out) + } + }) + + t.Run("--persist without an identity is a usage error", func(t *testing.T) { + // Given + evalEnv(t) + + // When + _, err := run("", "evaluate", "--trait", "country=US", "--persist") + + // Then + var usage *usageError + if !errors.As(err, &usage) { + t.Fatalf("err = %v, want a usage error", err) + } + if !strings.Contains(err.Error(), "--identity") { + t.Errorf("err = %v, want it to name the flag", err) + } + }) + + t.Run("a malformed trait is a usage error", func(t *testing.T) { + // Given + evalEnv(t) + + // When + _, err := run("", "evaluate", "--trait", "plan") + + // Then + var usage *usageError + if !errors.As(err, &usage) { + t.Fatalf("err = %v, want a usage error", err) + } + }) + + t.Run("a single feature resolves for the identity", func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkIdentityFlags["user-123"] = overridden() + + // When + out, err := run("", "evaluate", "max_items", "--identity", "user-123", "--json") + + // Then + if err != nil { + t.Fatalf("evaluate max_items --identity: %v\noutput: %s", err, out) + } + var flag map[string]any + if err := json.Unmarshal([]byte(out), &flag); err != nil { + t.Fatalf("parsing %q: %v", out, err) + } + if flag["feature"] != "max_items" || flag["enabled"] != true || flag["value"] != float64(99) { + t.Errorf("flag = %+v", flag) + } + }) +} + func TestProject(t *testing.T) { t.Run("list shows the organisation name", func(t *testing.T) { f := flagUpdateEnv(t) diff --git a/internal/cmd/errors.go b/internal/cmd/errors.go index c8446cf..1f47d35 100644 --- a/internal/cmd/errors.go +++ b/internal/cmd/errors.go @@ -27,10 +27,14 @@ const ( hintAccessToken = "For an OAuth access token, set FLAGSMITH_ACCESS_TOKEN instead." hintServerSideKey = "Server-side keys are secrets — provide them via FLAGSMITH_ENVIRONMENT_KEY instead." + hintEnvironmentKey = "Check FLAGSMITH_ENVIRONMENT_KEY, or the environment name/key passed with -e." + hintSDKAPIURL = "Check --sdk-api-url (or `sdkApiUrl`) — it must point at a Flagsmith SDK API." + hintEnvironmentList = "Run `flagsmith environment list` to see the environments in this project." hintProjectList = "Run `flagsmith project list` to see the projects you can access." hintOrganisationList = "Run `flagsmith organisation list` to see the organisations you can access." hintFlagList = "Run `flagsmith flag list` to see the flags in this environment." + hintEvaluate = "Run `flagsmith eval` to see the flags this environment resolves to." hintFeatureList = "Run `flagsmith feature list` to see the features in this project." hintSegmentList = "Run `flagsmith segment list` to see the segments in this project." diff --git a/internal/cmd/evaluate.go b/internal/cmd/evaluate.go new file mode 100644 index 0000000..4c7bbf6 --- /dev/null +++ b/internal/cmd/evaluate.go @@ -0,0 +1,212 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "strings" + + flagsmith "github.com/Flagsmith/flagsmith-go-client/v5" + "github.com/spf13/cobra" + + "github.com/Flagsmith/flagsmith-cli/internal/bug" + "github.com/Flagsmith/flagsmith-cli/internal/httpx" + "github.com/Flagsmith/flagsmith-cli/internal/output" +) + +// evalView is the curated JSON/detail shape for one resolved flag: what an SDK +// hands the application, and nothing else. Human output and JSON stay in +// lockstep. +type evalView struct { + Feature string `json:"feature"` + Enabled bool `json:"enabled"` + Value any `json:"value"` +} + +func newEvalView(f flagsmith.Flag) evalView { + return evalView{Feature: f.FeatureName, Enabled: f.Enabled, Value: f.Value} +} + +var ( + evalIdentityFlag string + evalTraitFlags []string + evalPersistFlag bool +) + +var evaluateCmd = &cobra.Command{ + Use: "evaluate [feature]", + Aliases: []string{"eval"}, + Short: "Show the flags an SDK resolves for the current environment", + Example: ` # the flags a fresh SDK sees + flagsmith evaluate + + # resolved for one user: segment overrides applied, variants allocated + flagsmith evaluate --identity user-123 + + # a what-if: overlay traits without persisting the identity + flagsmith evaluate --identity user-123 --trait plan=premium --trait age=42 + + # one feature, for scripting + flagsmith eval onboarding --identity user-123 --jq .value`, + Args: cobra.MaximumNArgs(1), + RunE: runEvaluate, +} + +func runEvaluate(cmd *cobra.Command, args []string) error { + if evalPersistFlag && evalIdentityFlag == "" { + return usageErrorf("there is no identity to persist — pass --identity, or drop --persist") + } + traits, err := parseTraits(evalTraitFlags) + if err != nil { + return err + } + pc, err := applyContext(cmd) + if err != nil { + return err + } + key, err := sdkEnvironmentKey(cmd, pc) + if err != nil { + return err + } + sdkURL, _ := pc.SDKAPIURL.Value.(string) + + flags, err := evaluateFlags(cmd.Context(), sdkURL, key, evalIdentityFlag, traits, evalPersistFlag) + if err != nil { + return evalError(sdkURL, err) + } + all := flags.AllFlags() + + if len(args) == 1 { + flag := findEvaluated(all, args[0]) + if flag == nil { + return withHint(fmt.Errorf("no flag named %q was resolved", args[0]), hintEvaluate) + } + return renderEvalDetail(cmd, newEvalView(*flag)) + } + views := make([]evalView, len(all)) + for i, flag := range all { + views[i] = newEvalView(flag) + } + return renderList(cmd, views, "No flags.", + []string{"FEATURE", "ENABLED", "VALUE"}, + func(_ int, v evalView) []string { + return []string{v.Feature, boolState(v.Enabled), truncateValue(valueDisplay(v.Value))} + }, "flag", "flags") +} + +// evaluateFlags resolves the environment's flags over the SDK API: the +// environment defaults on their own, or an identity evaluation as soon as an +// identity or a trait is in play. An empty identifier is the anonymous identity +// the SDK API evaluates without ever storing. +// +// The evaluation is transient unless --persist was given, so a what-if leaves +// neither the identity nor its traits behind. The SDK reads that choice from the +// context, and the environment key it was built with authenticates the call. +func evaluateFlags(ctx context.Context, sdkURL, key, identity string, traits []*flagsmith.Trait, persist bool) (flagsmith.Flags, error) { + client := newSDKClient(sdkURL, key) + if identity == "" && len(traits) == 0 { + return client.GetEnvironmentFlagsFromAPI(ctx) + } + transient := !persist + ctx = flagsmith.WithEvaluationContext(ctx, flagsmith.EvaluationContext{ + Identity: &flagsmith.IdentityEvaluationContext{Transient: &transient}, + }) + return client.GetIdentityFlagsFromAPI(ctx, identity, traits) +} + +// newSDKClient builds the Flagsmith SDK client for the resolved SDK surface. +// +// Evaluation goes over the CLI's own transport. +// The SDK's own logger is discarded. +// +// The http.Client is this invocation's alone rather than the shared one, because +// NewClient installs a 10s Timeout on any client that carries none — a cap that +// would otherwise leak into every other request in the process. Clearing it +// again leaves the request context as the only bound, as everywhere else, so +// FLAGSMITH_TIMEOUT still has the last word. +func newSDKClient(sdkURL, key string) *flagsmith.Client { + hc := httpx.New(userAgent()) + client := flagsmith.NewClient(key, + flagsmith.WithBaseURL(strings.TrimRight(sdkURL, "/")+"/api/v1/"), + flagsmith.WithHTTPClient(hc), + flagsmith.WithSlogLogger(slog.New(slog.DiscardHandler)), + ) + hc.Timeout = 0 + return client +} + +// findEvaluated returns the resolved flag for the named feature, or nil. The SDK +// API answers with every flag at once, so one feature is picked out here — by +// exact name, case-insensitively, as feature references are matched elsewhere. +func findEvaluated(flags []flagsmith.Flag, ref string) *flagsmith.Flag { + for i := range flags { + if strings.EqualFold(flags[i].FeatureName, ref) { + return &flags[i] + } + } + return nil +} + +// parseTraits turns repeated --trait key=value flags into SDK traits. Values are +// typed by the same inference as `flagsmith api -F`: true/false, a number, else +// a string. +func parseTraits(raw []string) ([]*flagsmith.Trait, error) { + traits := make([]*flagsmith.Trait, 0, len(raw)) + for _, item := range raw { + k, v, ok := strings.Cut(item, "=") + if !ok { + return nil, usageErrorf("invalid trait %q (want key=value)", item) + } + traits = append(traits, &flagsmith.Trait{TraitKey: k, TraitValue: typedFieldValue(v)}) + } + return traits, nil +} + +// evalError restates an SDK failure in the CLI's own terms: the SDK's errors +// carry a "flagsmith:" prefix and stack their own wording, and its message is +// all that survives of the response — never the key it was sent with, which may +// be a secret. +func evalError(sdkURL string, err error) error { + var apiErr *flagsmith.FlagsmithAPIError + if !errors.As(err, &apiErr) { + return err + } + switch apiErr.ResponseStatusCode { + case 0: + // No response at all: a transport failure, whose own error is the useful + // one — it is what says "context deadline exceeded" or "no such host". + if apiErr.Err != nil { + return fmt.Errorf("evaluating flags on %s: %w", sdkURL, apiErr.Err) + } + return bug.Mark(fmt.Errorf("evaluating flags on %s failed", sdkURL)) + case http.StatusUnauthorized, http.StatusForbidden: + return withHint(fmt.Errorf("%s rejected the environment key (%s)", sdkURL, apiErr.ResponseStatus), + hintEnvironmentKey) + case http.StatusNotFound: + return withHint(fmt.Errorf("%s has no SDK API to evaluate against (%s)", sdkURL, apiErr.ResponseStatus), + hintSDKAPIURL) + } + return bug.Mark(fmt.Errorf("evaluating flags on %s returned %s", sdkURL, apiErr.ResponseStatus)) +} + +// renderEvalDetail prints one resolved flag's detail view (or its JSON). +func renderEvalDetail(cmd *cobra.Command, v evalView) error { + return output.Render(cmd.OutOrStdout(), v, outputOpts(), func(w io.Writer) error { + return output.Detail(w, []output.Field{ + {Label: "Feature", Value: v.Feature}, + {Label: "Enabled", Value: boolState(v.Enabled)}, + {Label: "Value", Value: valueDisplay(v.Value)}, + }) + }) +} + +func init() { + f := evaluateCmd.Flags() + f.StringVar(&evalIdentityFlag, "identity", "", "resolve for this identifier") + f.StringArrayVar(&evalTraitFlags, "trait", nil, "trait key=value to evaluate with (repeatable)") + f.BoolVar(&evalPersistFlag, "persist", false, "persist the identity and its traits instead of evaluating transiently") + rootCmd.AddCommand(evaluateCmd) +} diff --git a/internal/cmd/resolve.go b/internal/cmd/resolve.go index a562c86..c1ab608 100644 --- a/internal/cmd/resolve.go +++ b/internal/cmd/resolve.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "regexp" "sort" "strconv" "strings" @@ -102,6 +103,57 @@ func resolveEnvironment(cmd *cobra.Command, pc *projectContext, cred *activeCred return *e, nil } +// environmentKeyShape matches a client-side SDK key: the backend mints them as +// shortuuids, which are always 22 alphanumerics. It tells a key apart from a +// name, so a key nothing has cached yet still reaches the SDK API without an +// Admin credential. A 22-character name with no separators is the false +// positive, and even that resolves once the cache has seen the environment. +var environmentKeyShape = regexp.MustCompile(`^[0-9A-Za-z]{22}$`) + +// sdkEnvironmentKey returns the environment key the SDK API authenticates with. +// +// FLAGSMITH_ENVIRONMENT_KEY wins: it is the only home for a server-side key, and +// it names its environment outright. Otherwise the environment context is a key +// or a name, as it is everywhere else — and a name is turned into a key by the +// local name cache wherever it can be, so evaluation stays free of the Admin API +// and of any credential for it. Falling through to the Admin API is what makes a +// name work at all on a cold cache; it is also the only way to scope a name that +// the instance-wide cache holds more than one of. +func sdkEnvironmentKey(cmd *cobra.Command, pc *projectContext) (string, error) { + sdkURL, _ := pc.SDKAPIURL.Value.(string) + if _, v := envCredential(envEnvironmentKey, sdkURL, defaultSDKAPIURL); v != "" { + return v, nil + } + ref, _ := pc.Environment.Value.(string) + if ref == "" { + return "", withHint(errors.New("no environment key"), + "Set FLAGSMITH_ENVIRONMENT_KEY, or pass -e.") + } + names := cache.Load(pc.apiURL()).Environments + if _, cached := names[ref]; cached { + return ref, nil + } + if hits := matchByName(names, ref); len(hits) == 1 { + return hits[0], nil + } + if environmentKeyShape.MatchString(ref) { + return ref, nil + } + cred, err := resolveCredential(cmd.Context()) + if err != nil { + return "", err + } + projectID, err := resolveProjectID(cmd, pc, cred) + if err != nil { + return "", err + } + env, err := resolveEnvironmentRef(cmd, cred, projectID, ref) + if err != nil { + return "", err + } + return env.APIKey, nil +} + // resolveEnvironmentRef turns an environment reference (an api_key or a name) // into its full record, scoped to the project, seeding the name cache on the // way. Environments are canonically addressed by key, so an exact key match diff --git a/internal/httpx/client.go b/internal/httpx/client.go index b43cdee..27287b2 100644 --- a/internal/httpx/client.go +++ b/internal/httpx/client.go @@ -30,8 +30,8 @@ const ( ) // New returns an *http.Client whose transport applies sane per-connection -// timeouts, sets a default User-Agent when the request carries none, and -// retries idempotent read requests on transient failures. +// timeouts, identifies the CLI with userAgent, and retries idempotent read +// requests on transient failures. // // It deliberately sets no overall Client.Timeout: the bound comes from the // request context, so cancellation works and a per-request deadline does not @@ -142,9 +142,8 @@ type transport struct { } func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) { - // A RoundTripper must not mutate the caller's request, so clone before - // stamping the default User-Agent. - if t.userAgent != "" && req.Header.Get("User-Agent") == "" { + // User-Agent is not negotiable + if t.userAgent != "" { req = req.Clone(req.Context()) req.Header.Set("User-Agent", t.userAgent) } diff --git a/internal/httpx/client_test.go b/internal/httpx/client_test.go index 8e07e09..4913c24 100644 --- a/internal/httpx/client_test.go +++ b/internal/httpx/client_test.go @@ -92,7 +92,7 @@ func TestRetriesIdempotentReads(t *testing.T) { }) } -func TestSetsUserAgentWhenAbsent(t *testing.T) { +func TestSetsUserAgent(t *testing.T) { t.Run("stamps the configured agent", func(t *testing.T) { var got string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -110,7 +110,9 @@ func TestSetsUserAgentWhenAbsent(t *testing.T) { } }) - t.Run("does not override a caller-set agent", func(t *testing.T) { + // The CLI's identity wins over a caller-set one — including the one the + // Flagsmith SDK sets on every request of its own. + t.Run("overrides a caller-set agent", func(t *testing.T) { var got string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { got = r.Header.Get("User-Agent") @@ -124,8 +126,12 @@ func TestSetsUserAgentWhenAbsent(t *testing.T) { t.Fatal(err) } resp.Body.Close() - if got != "custom/1.0" { - t.Errorf("User-Agent = %q, want custom/1.0", got) + if got != "flagsmith-cli/test" { + t.Errorf("User-Agent = %q, want flagsmith-cli/test", got) + } + // The caller's own request must come back untouched. + if req.Header.Get("User-Agent") != "custom/1.0" { + t.Errorf("caller's request was mutated: %q", req.Header.Get("User-Agent")) } }) } From 1e06b5beb9382bb089ec73ed753d60d671fac500 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 28 Jul 2026 22:40:00 +0100 Subject: [PATCH 02/10] feat: `--json` is an EvaluationResult, `--js` is frontend SDK state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `evaluate --json` now emits the result contract every Flagsmith SDK generates its result type from — sdk/evaluation-result.json — with a `$schema` naming it, so what the CLI prints and what an SDK resolves are the same document. It is a subset for now: `segments` and per-flag `reason` are required by the schema but returned by no SDK API endpoint, and `variant` is on the wire for an identity evaluation but discarded by the Go SDK before we see it. All three are absent rather than invented, and arrive as they become available. `--js` writes what a frontend SDK hydrates from instead: `api` and `flags`, and deliberately nothing else. An `evaluationContext` there would have to carry the environment key — a secret when it is a server-side one, and an app passes its own anyway — and `evaluationEvent`, `ts` and `identity` are bookkeeping an SDK fills in for itself. An empty environment still writes its state, with a warning: an SDK given no flags waits for a fetch a hydrating app never makes. Naming a feature prints that flag alone, so `--jq .value` reads it. `--js` describes a whole environment and refuses one: a single-flag state would tell an app the environment has a single flag. RFC §12 updated to match. beep boop --- internal/cmd/cmd_test.go | 166 +++++++++++++++++++++++++++++++++++---- internal/cmd/evaluate.go | 143 +++++++++++++++++++++++++++------ 2 files changed, 269 insertions(+), 40 deletions(-) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index d7ece49..7d67dc4 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -5917,6 +5917,16 @@ func evalEnv(t *testing.T) *fakeInstance { return f } +// evalDoc parses an evaluate command's machine-readable output. +func evalDoc(t *testing.T, out string) map[string]any { + t.Helper() + var doc map[string]any + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("parsing %q: %v", out, err) + } + return doc +} + // captureOSStderr collects what run writes to the process's stderr — where a // library's own logger lands, bypassing the command's writers entirely. func captureOSStderr(t *testing.T, run func()) string { @@ -5993,7 +6003,7 @@ func TestEvaluate(t *testing.T) { } }) - t.Run("json is a curated array", func(t *testing.T) { + t.Run("json is an EvaluationResult", func(t *testing.T) { // Given evalEnv(t) @@ -6004,21 +6014,141 @@ func TestEvaluate(t *testing.T) { if err != nil { t.Fatalf("evaluate --json: %v\noutput: %s", err, out) } - var flags []map[string]any - if err := json.Unmarshal([]byte(out), &flags); err != nil { - t.Fatalf("parsing %q: %v", out, err) + doc := evalDoc(t, out) + if got, _ := doc["$schema"].(string); !strings.HasSuffix(got, "/sdk/evaluation-result.json") { + t.Errorf("$schema = %q, want the SDK's evaluation-result schema", got) } + flags, _ := doc["flags"].(map[string]any) if len(flags) != 2 { - t.Fatalf("flags = %+v", flags) + t.Fatalf("flags = %+v, want one entry per feature, keyed by name", flags) + } + banner, _ := flags["onboarding_banner"].(map[string]any) + if banner["name"] != "onboarding_banner" || banner["enabled"] != true { + t.Errorf("flag = %+v, want the resolved state", banner) + } + if v, ok := banner["value"]; !ok || v != nil { + t.Errorf("flag = %+v, want an explicit null value — the schema requires the field", banner) + } + items, _ := flags["max_items"].(map[string]any) + if items["value"] != float64(25) || items["enabled"] != false { + t.Errorf("flag = %+v, want the resolved value", items) + } + // Omitted until the SDK API returns them: absent, never faked. + for _, absent := range []string{"reason", "variant"} { + if _, ok := items[absent]; ok { + t.Errorf("flag = %+v, want %q omitted rather than invented", items, absent) + } + } + if _, ok := doc["segments"]; ok { + t.Errorf("doc = %+v, want segments omitted rather than invented", doc) + } + }) + + t.Run("--js is the frontend SDK's hydration state", func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkEnvFlags["WqXhZk8sVY3dGgTqZ9pJmN"] = []map[string]any{ + {"enabled": true, "feature_state_value": "Welcome!", + "feature": map[string]any{"id": 1, "name": "banner-text"}}, + } + + // When + out, err := run("", "evaluate", "--js") + + // Then + if err != nil { + t.Fatalf("evaluate --js: %v\noutput: %s", err, out) } - if flags[0]["feature"] != "onboarding_banner" || flags[0]["enabled"] != true || flags[0]["value"] != nil { - t.Errorf("item = %+v, want the resolved flag hoisted", flags[0]) + doc := evalDoc(t, out) + api, _ := doc["api"].(string) + if !strings.HasSuffix(api, "/api/v1/") { + t.Errorf("api = %q, want the SDK API base with its trailing slash", api) } - if flags[1]["value"] != float64(25) { - t.Errorf("item = %+v, want the resolved value", flags[1]) + flags, _ := doc["flags"].(map[string]any) + flag, ok := flags["banner-text"].(map[string]any) + if !ok { + t.Fatalf("flags = %+v, want the feature name as the key, verbatim", flags) } - if len(flags[0]) != 3 { - t.Errorf("item = %+v, want feature/enabled/value only", flags[0]) + if flag["enabled"] != true || flag["value"] != "Welcome!" { + t.Errorf("flag = %+v", flag) + } + if _, ok := flag["name"]; ok { + t.Errorf("flag = %+v, want no name — the key is the name", flag) + } + if _, ok := doc["$schema"]; ok { + t.Errorf("doc = %+v, want no $schema — this is not an EvaluationResult", doc) + } + }) + + t.Run("--js and --json are mutually exclusive", func(t *testing.T) { + // Given + evalEnv(t) + + // When + _, err := run("", "evaluate", "--js", "--json") + + // Then + var usage *usageError + if !errors.As(err, &usage) { + t.Fatalf("err = %v, want a usage error", err) + } + }) + + t.Run("--js refuses a named feature", func(t *testing.T) { + // Given + evalEnv(t) + + // When + _, err := run("", "evaluate", "max_items", "--js") + + // Then + var usage *usageError + if !errors.As(err, &usage) { + t.Fatalf("err = %v, want a usage error — a one-flag state is a dud", err) + } + }) + + t.Run("--jq filters whichever shape was asked for", func(t *testing.T) { + // Given + evalEnv(t) + + // When + jsOut, err := run("", "evaluate", "--js", "--jq", ".flags.max_items.value") + if err != nil { + t.Fatalf("evaluate --js --jq: %v", err) + } + jsonOut, err := run("", "evaluate", "--jq", ".flags.max_items.name") + if err != nil { + t.Fatalf("evaluate --jq: %v", err) + } + + // Then + if strings.TrimSpace(jsOut) != "25" { + t.Errorf("--js --jq = %q, want 25", jsOut) + } + if strings.TrimSpace(jsonOut) != "max_items" { + t.Errorf("--jq = %q, want max_items", jsonOut) + } + }) + + t.Run("--js warns rather than ship a state nothing can hydrate", func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkEnvFlags["WqXhZk8sVY3dGgTqZ9pJmN"] = []map[string]any{} + + // When + out, errOut, err := runSplit("", "evaluate", "--js") + + // Then + if err != nil { + t.Fatalf("evaluate --js: %v", err) + } + if !strings.Contains(errOut, "Warning:") { + t.Errorf("stderr = %q, want a warning that the state is empty", errOut) + } + flags, _ := evalDoc(t, out)["flags"].(map[string]any) + if len(flags) != 0 { + t.Errorf("flags = %+v, want an empty map", flags) } }) @@ -6443,13 +6573,17 @@ func TestEvaluateIdentity(t *testing.T) { if err != nil { t.Fatalf("evaluate max_items --identity: %v\noutput: %s", err, out) } - var flag map[string]any - if err := json.Unmarshal([]byte(out), &flag); err != nil { - t.Fatalf("parsing %q: %v", out, err) - } - if flag["feature"] != "max_items" || flag["enabled"] != true || flag["value"] != float64(99) { + // Naming a feature prints the flag on its own, with no document around it, + // so `--jq .value` reads it. + flag := evalDoc(t, out) + if flag["name"] != "max_items" || flag["enabled"] != true || flag["value"] != float64(99) { t.Errorf("flag = %+v", flag) } + for _, absent := range []string{"$schema", "flags"} { + if _, ok := flag[absent]; ok { + t.Errorf("flag = %+v, want no %q — one flag is not an EvaluationResult", flag, absent) + } + } }) } diff --git a/internal/cmd/evaluate.go b/internal/cmd/evaluate.go index 4c7bbf6..1356217 100644 --- a/internal/cmd/evaluate.go +++ b/internal/cmd/evaluate.go @@ -17,23 +17,78 @@ import ( "github.com/Flagsmith/flagsmith-cli/internal/output" ) -// evalView is the curated JSON/detail shape for one resolved flag: what an SDK -// hands the application, and nothing else. Human output and JSON stay in -// lockstep. +// evalView is one resolved flag as the human view shows it — what an SDK hands +// the application, and nothing else. type evalView struct { - Feature string `json:"feature"` - Enabled bool `json:"enabled"` - Value any `json:"value"` + Feature string + Enabled bool + Value any } func newEvalView(f flagsmith.Flag) evalView { return evalView{Feature: f.FeatureName, Enabled: f.Enabled, Value: f.Value} } +const evaluationResultSchema = "https://raw.githubusercontent.com/Flagsmith/flagsmith/main/sdk/evaluation-result.json" + +// evalResult is `--json`: an EvaluationResult, as much of it as a remote +// evaluation can fill. +type evalResult struct { + Schema string `json:"$schema"` + Flags map[string]evalFlag `json:"flags"` +} + +// evalFlag is one flag in an EvaluationResult, and is also what naming a feature +// prints on its own. +type evalFlag struct { + Name string `json:"name"` + Enabled bool `json:"enabled"` + Value any `json:"value"` +} + +func newEvalFlag(v evalView) evalFlag { + return evalFlag{Name: v.Feature, Enabled: v.Enabled, Value: v.Value} +} + +func newEvalResult(views []evalView) evalResult { + flags := make(map[string]evalFlag, len(views)) + for _, v := range views { + flags[v.Feature] = newEvalFlag(v) + } + return evalResult{Schema: evaluationResultSchema, Flags: flags} +} + +// sdkState is `--js`: the state a Flagsmith frontend SDK hydrates from. +type sdkState struct { + API string `json:"api"` + Flags map[string]jsFlag `json:"flags"` +} + +// jsFlag is one flag in the hydration state. +type jsFlag struct { + Enabled bool `json:"enabled"` + Value any `json:"value"` +} + +// newSDKState keys flags by feature name, as the frontend SDKs look them up. +func newSDKState(sdkURL string, views []evalView) sdkState { + flags := make(map[string]jsFlag, len(views)) + for _, v := range views { + flags[v.Feature] = jsFlag{Enabled: v.Enabled, Value: v.Value} + } + return sdkState{API: sdkAPIBase(sdkURL), Flags: flags} +} + +// sdkAPIBase is the SDK API base URL as the SDKs write it, trailing slash and all. +func sdkAPIBase(sdkURL string) string { + return strings.TrimRight(sdkURL, "/") + "/api/v1/" +} + var ( evalIdentityFlag string evalTraitFlags []string evalPersistFlag bool + evalJSFlag bool ) var evaluateCmd = &cobra.Command{ @@ -50,7 +105,10 @@ var evaluateCmd = &cobra.Command{ flagsmith evaluate --identity user-123 --trait plan=premium --trait age=42 # one feature, for scripting - flagsmith eval onboarding --identity user-123 --jq .value`, + flagsmith eval onboarding --identity user-123 --jq .value + + # bootstrap a frontend SDK (can be provided as state) + flagsmith eval --js > state.json`, Args: cobra.MaximumNArgs(1), RunE: runEvaluate, } @@ -59,6 +117,13 @@ func runEvaluate(cmd *cobra.Command, args []string) error { if evalPersistFlag && evalIdentityFlag == "" { return usageErrorf("there is no identity to persist — pass --identity, or drop --persist") } + if evalJSFlag && jsonFlag { + return usageErrorf("--js and --json are different shapes — pass one or the other") + } + // --js takes the environment whole or not at all. + if evalJSFlag && len(args) == 1 { + return usageErrorf("--js describes a whole environment — drop %q, or drop --js", args[0]) + } traits, err := parseTraits(evalTraitFlags) if err != nil { return err @@ -84,17 +149,57 @@ func runEvaluate(cmd *cobra.Command, args []string) error { if flag == nil { return withHint(fmt.Errorf("no flag named %q was resolved", args[0]), hintEvaluate) } - return renderEvalDetail(cmd, newEvalView(*flag)) + return renderEvaluation(cmd, sdkURL, []evalView{newEvalView(*flag)}, true) } views := make([]evalView, len(all)) for i, flag := range all { views[i] = newEvalView(flag) } - return renderList(cmd, views, "No flags.", - []string{"FEATURE", "ENABLED", "VALUE"}, - func(_ int, v evalView) []string { - return []string{v.Feature, boolState(v.Enabled), truncateValue(valueDisplay(v.Value))} - }, "flag", "flags") + return renderEvaluation(cmd, sdkURL, views, false) +} + +// renderEvaluation writes the resolved flags: an EvaluationResult, or the +// frontend SDK's hydration state for --js, or the human table. +func renderEvaluation(cmd *cobra.Command, sdkURL string, views []evalView, single bool) error { + var doc any = newEvalResult(views) + if single { + doc = newEvalFlag(views[0]) + } + if evalJSFlag { + doc = newSDKState(sdkURL, views) + // An SDK handed a state with no flags has nothing to hydrate and waits + // for a fetch a serverState-only app never makes. The file is still + // written: with options of its own, an SDK fetches and is fine. + if len(views) == 0 { + fmt.Fprintln(cmd.ErrOrStderr(), + "Warning: no flags to hydrate from — an SDK given this state will wait for a fetch") + } + } + opts := outputOpts() + opts.JSON = opts.JSON || evalJSFlag + return output.Render(cmd.OutOrStdout(), doc, opts, func(w io.Writer) error { + if single { + v := views[0] + return output.Detail(w, []output.Field{ + {Label: "Feature", Value: v.Feature}, + {Label: "Enabled", Value: boolState(v.Enabled)}, + {Label: "Value", Value: valueDisplay(v.Value)}, + }) + } + if len(views) == 0 { + fmt.Fprintln(w, "No flags.") + return nil + } + rows := make([][]string, len(views)) + for i, v := range views { + rows[i] = []string{v.Feature, boolState(v.Enabled), truncateValue(valueDisplay(v.Value))} + } + if err := output.Table(w, []string{"FEATURE", "ENABLED", "VALUE"}, rows); err != nil { + return err + } + fmt.Fprintf(w, "\n%d %s\n", len(views), plural(len(views), "flag", "flags")) + return nil + }) } // evaluateFlags resolves the environment's flags over the SDK API: the @@ -192,21 +297,11 @@ func evalError(sdkURL string, err error) error { return bug.Mark(fmt.Errorf("evaluating flags on %s returned %s", sdkURL, apiErr.ResponseStatus)) } -// renderEvalDetail prints one resolved flag's detail view (or its JSON). -func renderEvalDetail(cmd *cobra.Command, v evalView) error { - return output.Render(cmd.OutOrStdout(), v, outputOpts(), func(w io.Writer) error { - return output.Detail(w, []output.Field{ - {Label: "Feature", Value: v.Feature}, - {Label: "Enabled", Value: boolState(v.Enabled)}, - {Label: "Value", Value: valueDisplay(v.Value)}, - }) - }) -} - func init() { f := evaluateCmd.Flags() f.StringVar(&evalIdentityFlag, "identity", "", "resolve for this identifier") f.StringArrayVar(&evalTraitFlags, "trait", nil, "trait key=value to evaluate with (repeatable)") f.BoolVar(&evalPersistFlag, "persist", false, "persist the identity and its traits instead of evaluating transiently") + f.BoolVar(&evalJSFlag, "js", false, "output the state a Flagsmith frontend SDK hydrates from") rootCmd.AddCommand(evaluateCmd) } From a5e5c3bff8f4d30a89720a4b5a2fec1fb7e26646 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 28 Jul 2026 22:55:06 +0100 Subject: [PATCH 03/10] feat: `--test` fails when a named flag is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `flagsmith eval --test` turns a lookup into a check: a disabled flag becomes a failure like any other — exit 1, reason on stderr — so a script can gate on it with `&& `. The flag still prints first, so whoever ran the check can see the state that failed it. The error names the flag and nothing else. Everywhere else the CLI would name the environment too, but eval never resolves one: it holds only the key, which is a secret when it is a server-side one and must stay out of stderr and CI logs. It carries no hint either — a disabled flag is the answer, not something to recover from. RFC §12 updated to match. beep boop --- internal/cmd/cmd_test.go | 80 ++++++++++++++++++++++++++++++++++++++++ internal/cmd/evaluate.go | 20 +++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 7d67dc4..981ba82 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -6094,6 +6094,86 @@ func TestEvaluate(t *testing.T) { } }) + t.Run("--test makes a disabled flag a failure", func(t *testing.T) { + // Given + evalEnv(t) + + // When + out, errOut, err := runSplit("", "evaluate", "max_items", "--test") + + // Then + if err == nil { + t.Fatal("err = nil, want the disabled flag to fail") + } + if !strings.Contains(err.Error(), "max_items is disabled") { + t.Errorf("err = %v, want it to say the flag is disabled", err) + } + // A failure like any other: exit 1, reason on stderr + if code := reportError(evaluateCmd, err); code != 1 { + t.Errorf("exit code = %d, want 1", code) + } + if h := hintFor(err); h != "" { + t.Errorf("hint = %q, want none", h) + } + // The environment is not named + if strings.Contains(errOut, "WqXhZk8sVY3dGgTqZ9pJmN") { + t.Errorf("stderr = %q, want no environment key", errOut) + } + // The flag itself still prints. + if !strings.Contains(out, "max_items") || !strings.Contains(out, "off") { + t.Errorf("stdout = %q, want the flag printed anyway", out) + } + }) + + t.Run("--test passes an enabled flag through", func(t *testing.T) { + // Given onboarding_banner is on. + evalEnv(t) + + // When + out, err := run("", "evaluate", "onboarding_banner", "--test") + + // Then + if err != nil { + t.Fatalf("evaluate --test: %v\noutput: %s", err, out) + } + if !strings.Contains(out, "onboarding_banner") { + t.Errorf("output = %q", out) + } + }) + + t.Run("--test composes with --json", func(t *testing.T) { + // Given + evalEnv(t) + + // When + out, _, err := runSplit("", "evaluate", "max_items", "--test", "--json") + + // Then + if err == nil { + t.Fatal("err = nil, want the disabled flag to fail") + } + if flag := evalDoc(t, out); flag["name"] != "max_items" || flag["enabled"] != false { + t.Errorf("flag = %+v, want the entry printed as well", flag) + } + }) + + t.Run("--test needs a named feature", func(t *testing.T) { + // Given + evalEnv(t) + + // When + _, err := run("", "evaluate", "--test") + + // Then + var usage *usageError + if !errors.As(err, &usage) { + t.Fatalf("err = %v, want a usage error", err) + } + if !strings.Contains(err.Error(), "--test") { + t.Errorf("err = %v, want it to name the flag", err) + } + }) + t.Run("--js refuses a named feature", func(t *testing.T) { // Given evalEnv(t) diff --git a/internal/cmd/evaluate.go b/internal/cmd/evaluate.go index 1356217..5b0fa7c 100644 --- a/internal/cmd/evaluate.go +++ b/internal/cmd/evaluate.go @@ -89,6 +89,7 @@ var ( evalTraitFlags []string evalPersistFlag bool evalJSFlag bool + evalTestFlag bool ) var evaluateCmd = &cobra.Command{ @@ -108,7 +109,10 @@ var evaluateCmd = &cobra.Command{ flagsmith eval onboarding --identity user-123 --jq .value # bootstrap a frontend SDK (can be provided as state) - flagsmith eval --js > state.json`, + flagsmith eval --js > state.json + + # gate a script on a flag: a disabled flag fails + flagsmith eval new-pipeline --test && ./deploy.sh`, Args: cobra.MaximumNArgs(1), RunE: runEvaluate, } @@ -124,6 +128,9 @@ func runEvaluate(cmd *cobra.Command, args []string) error { if evalJSFlag && len(args) == 1 { return usageErrorf("--js describes a whole environment — drop %q, or drop --js", args[0]) } + if evalTestFlag && len(args) != 1 { + return usageErrorf("--test checks one flag — name the feature to check") + } traits, err := parseTraits(evalTraitFlags) if err != nil { return err @@ -149,7 +156,15 @@ func runEvaluate(cmd *cobra.Command, args []string) error { if flag == nil { return withHint(fmt.Errorf("no flag named %q was resolved", args[0]), hintEvaluate) } - return renderEvaluation(cmd, sdkURL, []evalView{newEvalView(*flag)}, true) + if err := renderEvaluation(cmd, sdkURL, []evalView{newEvalView(*flag)}, true); err != nil { + return err + } + // --test adds one failure mode: a disabled flag. It reports after the flag + // has printed, so the caller sees the state that failed the check. + if evalTestFlag && !flag.Enabled { + return fmt.Errorf("%s is disabled", flag.FeatureName) + } + return nil } views := make([]evalView, len(all)) for i, flag := range all { @@ -303,5 +318,6 @@ func init() { f.StringArrayVar(&evalTraitFlags, "trait", nil, "trait key=value to evaluate with (repeatable)") f.BoolVar(&evalPersistFlag, "persist", false, "persist the identity and its traits instead of evaluating transiently") f.BoolVar(&evalJSFlag, "js", false, "output the state a Flagsmith frontend SDK hydrates from") + f.BoolVar(&evalTestFlag, "test", false, "with a named feature: fail when the flag is disabled") rootCmd.AddCommand(evaluateCmd) } From 55000adcdc902e9db80e36f86ae7683f6d8efc5a Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Wed, 29 Jul 2026 10:12:44 +0100 Subject: [PATCH 04/10] docs: README lists `flagsmith evaluate` Also drops the Design section: it linked twelve docs/design/*.md files that 92c69d6 deleted when the Notion RFC became the single source of truth. beep boop --- README.md | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/README.md b/README.md index e5c9f88..d4a62cc 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,7 @@ flagsmith flag list # list the flags in the current environment - `flagsmith environment list|get|create|update|delete|clone` (alias `env`) — manage environments (by name or API key). - `flagsmith environment key list|create|delete ` — manage server-side SDK keys. - `flagsmith environment document [environment]` — output the environment document (local-evaluation JSON). +- `flagsmith evaluate [feature]` (alias `eval`) — the flags an SDK resolves for the current environment (`--identity`, `--trait key=value`, `--persist`); `--js` writes the state a frontend SDK hydrates from, `--test` fails when a named flag is disabled. - `flagsmith config` — show the resolved context and where each value comes from. - `flagsmith login` / `logout` — browser OAuth (PKCE, loopback); also `auth login`/`auth logout`. - `flagsmith auth status` — identity, organisations, credential source, token expiry. @@ -50,18 +51,3 @@ flagsmith flag list # list the flags in the current environment - `--json` (or `FLAGSMITH_JSON_OUTPUT`) for machine-readable output; `--jq ` to filter it. - Static credentials: `FLAGSMITH_API_KEY` (Admin API), `FLAGSMITH_ENVIRONMENT_KEY` (SDK). - Self-hosted: `--api-url` or `FLAGSMITH_API_URL`. - -## Design - -[Installation](docs/design/01-installation.md) · -[Output & interactivity](docs/design/02-output-and-interactivity.md) · -[Authentication](docs/design/03-authentication.md) · -[Project config](docs/design/04-project-config.md) · -[CRUD conventions](docs/design/05-crud.md) · -[API](docs/design/06-api.md) · -[Flags](docs/design/07-flags.md) · -[Segments](docs/design/08-segments.md) · -[Features](docs/design/09-features.md) · -[Projects & organisations](docs/design/10-projects-organisations.md) · -[Environments](docs/design/11-environments.md) · -[Evaluate](docs/design/12-evaluate.md) From 1c089722a50b5111f1992ffa6b621faefc089d1f Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Wed, 29 Jul 2026 10:12:45 +0100 Subject: [PATCH 05/10] feat!: `-i` names the identity, and `flag list` takes `--identities` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every command that addresses one identity now takes `-i` — `flag get`, `flag update`, `flag delete`, `flag enable`, `flag disable`, and `eval`. `eval` also accepts `--identifier` as a hidden alias, because the flag commands spell it that way and nobody should have to remember which command uses which. `flag list --identity` becomes `--identities`. It is a switch selecting a dimension to list, not a value naming one, and the plural says so; `--segment ` stays singular because it does name one. The old spelling taught the wrong shape — `flag list --identity user-1` parses `--identity` as the boolean it is and leaves `user-1` behind as a positional. Which `flag list` then silently ignored, listing every flag in the environment: a command with no Args validator inherits cobra's ArbitraryArgs. It now refuses a stray argument and exits 2. RFC §7.3 updated to match. beep boop --- internal/cmd/cmd_test.go | 64 +++++++++++++++++++++++++++++++++---- internal/cmd/evaluate.go | 7 +++- internal/cmd/flag_update.go | 6 ++-- internal/cmd/flags.go | 19 +++++------ 4 files changed, 76 insertions(+), 20 deletions(-) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 981ba82..5cf7bd7 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -4288,9 +4288,9 @@ func TestFlagListIdentityOverrides(t *testing.T) { } f.mu.Unlock() - out, err := run("", "flag", "list", "--feature", "max_items", "--identity") + out, err := run("", "flag", "list", "--feature", "max_items", "--identities") if err != nil { - t.Fatalf("flag list --feature --identity: %v\noutput: %s", err, out) + t.Fatalf("flag list --feature --identities: %v\noutput: %s", err, out) } for _, want := range []string{ "IDENTIFIER", "STATE", "VALUE", @@ -4310,7 +4310,7 @@ func TestFlagListIdentityOverrides(t *testing.T) { f.coreOverrides = map[int]map[int]*fakeFS{501: {2: {id: 9100, enabled: true, value: "hero"}}} f.mu.Unlock() - out, err := run("", "flag", "list", "--feature", "max_items", "--identity", "--json") + out, err := run("", "flag", "list", "--feature", "max_items", "--identities", "--json") if err != nil { t.Fatal(err) } @@ -4335,18 +4335,18 @@ func TestFlagListIdentityOverrides(t *testing.T) { } f.mu.Unlock() - out, err := run("", "flag", "list", "--feature", "max_items", "--identity") + out, err := run("", "flag", "list", "--feature", "max_items", "--identities") if err != nil { - t.Fatalf("flag list --identity (edge): %v\noutput: %s", err, out) + t.Fatalf("flag list --identities (edge): %v\noutput: %s", err, out) } if !strings.Contains(out, "edge-user") || !strings.Contains(out, "1 override") { t.Errorf("output = %q, want the edge override listed", out) } }) - t.Run("--identity without --feature exits 2", func(t *testing.T) { + t.Run("--identities without --feature exits 2", func(t *testing.T) { f := flagUpdateEnv(t) - _, err := run("", "flag", "list", "--identity") + _, err := run("", "flag", "list", "--identities") var ue *usageError if !errors.As(err, &ue) || !strings.Contains(err.Error(), "--feature") { t.Errorf("err = %v, want a usage error naming --feature", err) @@ -4737,6 +4737,35 @@ func TestUsageIsSingleLine(t *testing.T) { func TestFlagIdentity(t *testing.T) { // max_items is feature id 2 in defaultFeatures; user-1 is core identity 501. + // -i is the identity everywhere it appears, `eval` included. + t.Run("-i is shorthand for --identifier", func(t *testing.T) { + f := flagUpdateEnv(t) + + out, err := run("", "flag", "update", "max_items", "-i", "user-1", "--value", "42", "--yes") + if err != nil { + t.Fatalf("flag update -i: %v\noutput: %s", err, out) + } + if w := f.lastIdentityWrite; w["feature"] != float64(2) || w["feature_state_value"] != float64(42) { + t.Errorf("core write = %+v, want -i to target the identity", w) + } + }) + + t.Run("-i works for get and delete too", func(t *testing.T) { + f := flagUpdateEnv(t) + f.coreOverrides[501] = map[int]*fakeFS{2: {id: 9001, enabled: true, value: 7}} + + out, err := run("", "flag", "get", "max_items", "-i", "user-1") + if err != nil { + t.Fatalf("flag get -i: %v\noutput: %s", err, out) + } + if !strings.Contains(out, "user-1") || !strings.Contains(out, "7") { + t.Errorf("output = %q, want the identity's override", out) + } + if out, err = run("", "flag", "delete", "max_items", "-i", "user-1", "--yes"); err != nil { + t.Fatalf("flag delete -i: %v\noutput: %s", err, out) + } + }) + t.Run("core: update creates an override via the core endpoint", func(t *testing.T) { f := flagUpdateEnv(t) // useEdge defaults to false @@ -6546,6 +6575,27 @@ func TestEvaluateIdentity(t *testing.T) { } }) + // The flag commands name this --identifier; eval names it --identity. Both + // spellings work here, so nobody has to remember which command uses which. + for _, spelling := range []string{"--identity", "--identifier", "-i"} { + t.Run("the identity is addressable as "+spelling, func(t *testing.T) { + // Given + f := evalEnv(t) + f.sdkIdentityFlags["user-123"] = overridden() + + // When + out, err := run("", "evaluate", spelling, "user-123") + + // Then + if err != nil { + t.Fatalf("evaluate %s: %v\noutput: %s", spelling, err, out) + } + if body := f.identifyBody(); body["identifier"] != "user-123" { + t.Errorf("body = %+v, want the identity resolved", body) + } + }) + } + t.Run("traits are typed by inference", func(t *testing.T) { // Given f := evalEnv(t) diff --git a/internal/cmd/evaluate.go b/internal/cmd/evaluate.go index 5b0fa7c..314b9d1 100644 --- a/internal/cmd/evaluate.go +++ b/internal/cmd/evaluate.go @@ -314,7 +314,12 @@ func evalError(sdkURL string, err error) error { func init() { f := evaluateCmd.Flags() - f.StringVar(&evalIdentityFlag, "identity", "", "resolve for this identifier") + f.StringVarP(&evalIdentityFlag, "identity", "i", "", "resolve for this identifier") + // The flag commands call this --identifier; accept that spelling too rather + // than making anyone remember which command uses which. Hidden, not + // deprecated: both names are correct, one is advertised. + f.StringVar(&evalIdentityFlag, "identifier", "", "alias for --identity") + f.Lookup("identifier").Hidden = true f.StringArrayVar(&evalTraitFlags, "trait", nil, "trait key=value to evaluate with (repeatable)") f.BoolVar(&evalPersistFlag, "persist", false, "persist the identity and its traits instead of evaluating transiently") f.BoolVar(&evalJSFlag, "js", false, "output the state a Flagsmith frontend SDK hydrates from") diff --git a/internal/cmd/flag_update.go b/internal/cmd/flag_update.go index d25af99..1c89390 100644 --- a/internal/cmd/flag_update.go +++ b/internal/cmd/flag_update.go @@ -405,12 +405,12 @@ func init() { flagUpdateCmd.Flags().StringVar(&flagValueFlag, "value", "", "set the flag value") flagUpdateCmd.Flags().StringVar(&flagTypeFlag, "type", "", "force the value type: string, integer, or boolean") flagUpdateCmd.Flags().StringVar(&flagUpdateSegment, "segment", "", "target this segment's override (id or name) instead of the environment default") - flagUpdateCmd.Flags().StringVar(&flagUpdateIdentifier, "identifier", "", "target this identity's override instead of the environment default") + flagUpdateCmd.Flags().StringVarP(&flagUpdateIdentifier, "identifier", "i", "", "target this identity's override instead of the environment default") flagUpdateCmd.Flags().IntVar(&flagUpdatePriority, "priority", 0, "move the segment override to this priority (0 is evaluated first)") flagDeleteCmd.Flags().StringVar(&flagDeleteSegment, "segment", "", "the segment (id or name) whose override to delete") - flagDeleteCmd.Flags().StringVar(&flagDeleteIdentifier, "identifier", "", "the identity whose override to delete") + flagDeleteCmd.Flags().StringVarP(&flagDeleteIdentifier, "identifier", "i", "", "the identity whose override to delete") for _, c := range []*cobra.Command{flagEnableCmd, flagDisableCmd} { c.Flags().StringVar(&flagToggleSegment, "segment", "", "target this segment's override (id or name) instead of the environment default") - c.Flags().StringVar(&flagToggleIdentifier, "identifier", "", "target this identity's override instead of the environment default") + c.Flags().StringVarP(&flagToggleIdentifier, "identifier", "i", "", "target this identity's override instead of the environment default") } } diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index 30328ab..01b2325 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -186,14 +186,15 @@ func flagContext(cmd *cobra.Command) (*projectContext, *activeCredential, int, a } var ( - flagListSegmentFlag string - flagListFeatureFlag string - flagListIdentityFlag bool + flagListSegmentFlag string + flagListFeatureFlag string + flagListIdentitiesFlag bool ) var flagListCmd = &cobra.Command{ Use: "list", Short: "List every flag in the current environment", + Args: cobra.NoArgs, // a stray positional would otherwise be silently ignored Example: ` # flags in the current environment flagsmith flag list @@ -202,13 +203,13 @@ var flagListCmd = &cobra.Command{ # one feature's segment or identity overrides flagsmith flag list --feature onboarding - flagsmith flag list --feature onboarding --identity`, + flagsmith flag list --feature onboarding --identities`, RunE: func(cmd *cobra.Command, args []string) error { switch { case flagListSegmentFlag != "" && flagListFeatureFlag != "": return usageErrorf("--segment and --feature are mutually exclusive") - case flagListIdentityFlag && flagListFeatureFlag == "": - return usageErrorf("--identity only applies together with --feature") + case flagListIdentitiesFlag && flagListFeatureFlag == "": + return usageErrorf("--identities only applies together with --feature") } _, cred, projectID, env, err := flagContext(cmd) if err != nil { @@ -223,7 +224,7 @@ var flagListCmd = &cobra.Command{ if err != nil { return err } - if flagListIdentityFlag { + if flagListIdentitiesFlag { return listFeatureIdentityOverrides(cmd, cred, env, projectID, feature) } return listFeatureSegmentOverrides(cmd, cred, env, feature) @@ -551,9 +552,9 @@ func plural(n int, one, many string) string { func init() { flagListCmd.Flags().StringVar(&flagListSegmentFlag, "segment", "", "list overrides for this segment (id or name)") flagListCmd.Flags().StringVar(&flagListFeatureFlag, "feature", "", "list this feature's segment overrides (id or name), in priority order") - flagListCmd.Flags().BoolVar(&flagListIdentityFlag, "identity", false, "with --feature: list its identity overrides instead") + flagListCmd.Flags().BoolVar(&flagListIdentitiesFlag, "identities", false, "with --feature: list its identity overrides instead") flagGetCmd.Flags().StringVar(&flagGetSegmentFlag, "segment", "", "show the override for this segment (id or name)") - flagGetCmd.Flags().StringVar(&flagGetIdentifierFlag, "identifier", "", "show the override for this identity") + flagGetCmd.Flags().StringVarP(&flagGetIdentifierFlag, "identifier", "i", "", "show the override for this identity") flagCmd.AddCommand(flagListCmd, flagGetCmd, flagUpdateCmd, flagEnableCmd, flagDisableCmd, flagReorderCmd, flagDeleteCmd, flagCreateCmd) rootCmd.AddCommand(flagCmd) } From 4d5429dee8c12bb67c4e0ffd699f129aff2d583e Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Wed, 29 Jul 2026 10:24:00 +0100 Subject: [PATCH 06/10] chore: tidy go.mod and go.sum The SDK was left recorded as an indirect dependency: it was added with `go get` after a `go mod tidy` had dropped it, at a point when nothing imported it yet, and never re-tidied once `internal/cmd/evaluate.go` did. Tidying also records the three transitive checksums that were missing from go.sum, which is what CI's `go mod tidy -diff` was failing on. beep boop --- go.mod | 2 +- go.sum | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 3e8ab87..73b745f 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/Flagsmith/flagsmith-cli go 1.26 require ( + github.com/Flagsmith/flagsmith-go-client/v5 v5.1.0 github.com/charmbracelet/huh v1.0.0 github.com/fatih/color v1.19.0 github.com/itchyny/gojq v0.12.19 @@ -15,7 +16,6 @@ require ( ) require ( - github.com/Flagsmith/flagsmith-go-client/v5 v5.1.0 // indirect github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect diff --git a/go.sum b/go.sum index 5128a21..521a9a2 100644 --- a/go.sum +++ b/go.sum @@ -61,6 +61,8 @@ github.com/go-resty/resty/v2 v2.17.2 h1:FQW5oHYcIlkCNrMD2lloGScxcHJ0gkjshV3qcQAy github.com/go-resty/resty/v2 v2.17.2/go.mod h1:kCKZ3wWmwJaNc7S29BRtUhJwy7iqmn+2mLtQrOyQlVA= github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/itchyny/gojq v0.12.19 h1:ttXA0XCLEMoaLOz5lSeFOZ6u6Q3QxmG46vfgI4O0DEs= @@ -104,6 +106,8 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/tailscale/hujson v0.0.0-20250605163823-992244df8c5a h1:a6TNDN9CgG+cYjaeN8l2mc4kSz2iMiCDQxPEyltUV/I= +github.com/tailscale/hujson v0.0.0-20250605163823-992244df8c5a/go.mod h1:EbW0wDK/qEUYI0A5bqq0C2kF8JTQwWONmGDBbzsxxHo= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs= @@ -124,6 +128,8 @@ golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= +golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= +golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From a7f01802e133bb1f9b6d4e2371da468dbffe76cf Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Wed, 29 Jul 2026 10:29:45 +0100 Subject: [PATCH 07/10] refactor: one definition of the SDK API base URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit newSDKClient rebuilt the string sdkAPIBase already returns, so the URL the SDK evaluates against and the `api` field `--js` writes could have drifted apart — including the trailing slash, which the JS SDK concatenates onto without normalising. beep boop --- internal/cmd/evaluate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/evaluate.go b/internal/cmd/evaluate.go index 314b9d1..f4dd9d1 100644 --- a/internal/cmd/evaluate.go +++ b/internal/cmd/evaluate.go @@ -250,7 +250,7 @@ func evaluateFlags(ctx context.Context, sdkURL, key, identity string, traits []* func newSDKClient(sdkURL, key string) *flagsmith.Client { hc := httpx.New(userAgent()) client := flagsmith.NewClient(key, - flagsmith.WithBaseURL(strings.TrimRight(sdkURL, "/")+"/api/v1/"), + flagsmith.WithBaseURL(sdkAPIBase(sdkURL)), flagsmith.WithHTTPClient(hc), flagsmith.WithSlogLogger(slog.New(slog.DiscardHandler)), ) From 24ebf07502aeb45d1e407adc06efcea29434d37f Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Wed, 29 Jul 2026 10:29:59 +0100 Subject: [PATCH 08/10] test: restore os.Stderr even if the captured call does not return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit captureOSStderr swapped os.Stderr back on the happy path only. A t.Fatal inside the captured func — which is exactly what the callers do on error — leaves the process's stderr pointing at a closed pipe for the rest of the package run, hiding the output of every test after it. beep boop --- internal/cmd/cmd_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 5cf7bd7..80202a6 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -5966,8 +5966,10 @@ func captureOSStderr(t *testing.T, run func()) string { } saved := os.Stderr os.Stderr = w + // Deferred: run may not return — t.Fatal inside it would otherwise leave + // os.Stderr pointing at the pipe, swallowing every later test's output. + defer func() { os.Stderr = saved }() run() - os.Stderr = saved w.Close() logged, err := io.ReadAll(r) if err != nil { From 5baaaf18327ac46a0dae0b659dd67f3517b141f0 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Wed, 29 Jul 2026 10:30:19 +0100 Subject: [PATCH 09/10] test: assert the rejected key is absent from the error, not just the output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subtest checked the command's output and the hint. The output does contain the rendered error — cobra prints it to the same buffer — but asserting on err.Error() directly stops the guarantee resting on that. beep boop --- internal/cmd/cmd_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 80202a6..f412eb3 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -1649,7 +1649,11 @@ func TestServerSideKeyNeverEchoed(t *testing.T) { if err == nil { t.Fatal("err = nil, want an error") } - if strings.Contains(out, "SuperSecret123") || strings.Contains(hintFor(err), "SuperSecret123") { + // out already carries the rendered error, but assert on the error itself + // too rather than lean on cobra printing it into the same buffer. + if strings.Contains(out, "SuperSecret123") || + strings.Contains(err.Error(), "SuperSecret123") || + strings.Contains(hintFor(err), "SuperSecret123") { t.Errorf("output = %q — the server-side key leaked", out) } }) From b076909f49bb547a2376b8589d6b6d0ed0c4494f Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Wed, 29 Jul 2026 10:30:38 +0100 Subject: [PATCH 10/10] test: the fake's identities endpoint honours sdkDelay too Only the flags endpoint slept, so a deadline test written against the identity path would have passed without ever exercising the delay it set. beep boop --- internal/cmd/cmd_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index f412eb3..92c4814 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -1331,11 +1331,12 @@ func newFakeInstance(t *testing.T) *fakeInstance { f.sdkUserAgents = append(f.sdkUserAgents, r.Header.Get("User-Agent")) f.sdkKeys = append(f.sdkKeys, r.Header.Get("X-Environment-Key")) f.lastIdentify = body - status, flags := f.sdkStatus, f.sdkEnvFlags[r.Header.Get("X-Environment-Key")] + status, flags, delay := f.sdkStatus, f.sdkEnvFlags[r.Header.Get("X-Environment-Key")], f.sdkDelay if override, ok := f.sdkIdentityFlags[identifier]; ok { flags = override } f.mu.Unlock() + time.Sleep(delay) if status != 0 { w.WriteHeader(status) return