From c32e4855e06837ca665f535e2d25822a3250b3c4 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Sun, 19 Jul 2026 14:01:40 -0700 Subject: [PATCH] feat(cmdutil): teach Build the platform-first fallback protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec gains Endpoint, FallbackMode (Auto | EnvOnly), and LegacyRun. When LegacyRun is set, Run is the platform call: gate-closed 404s either retry via LegacyRun with a one-time warning (Auto) or produce an actionable error naming GLEAN_LEGACY_APIS (EnvOnly, for user-authored bodies). GLEAN_LEGACY_APIS=1 routes straight to LegacyRun and skips parsing the payload against the platform request type. LegacyRun receives the normalized snake_case payload — the CLI's canonical input shape. Co-Authored-By: Claude Fable 5 --- internal/cmdutil/factory.go | 76 ++++++++++++- internal/cmdutil/factory_test.go | 189 +++++++++++++++++++++++++++++++ 2 files changed, 262 insertions(+), 3 deletions(-) create mode 100644 internal/cmdutil/factory_test.go diff --git a/internal/cmdutil/factory.go b/internal/cmdutil/factory.go index d80e2c1..53183e4 100644 --- a/internal/cmdutil/factory.go +++ b/internal/cmdutil/factory.go @@ -21,9 +21,25 @@ import ( glean "github.com/gleanwork/api-client-go" gleanClient "github.com/gleanwork/glean-cli/internal/client" "github.com/gleanwork/glean-cli/internal/output" + "github.com/gleanwork/glean-cli/internal/platform" "github.com/spf13/cobra" ) +// FallbackMode controls what a platform-first command (Spec.LegacyRun != nil) +// does when the platform endpoint is gated off (hidden 404). +type FallbackMode int + +const ( + // FallbackAuto retries via LegacyRun with a one-time warning. Use for + // commands whose request the CLI built itself (or whose input is + // surface-agnostic, like a bare ID). + FallbackAuto FallbackMode = iota + // FallbackEnvOnly never retries automatically: gate-closed produces an + // actionable error naming GLEAN_LEGACY_APIS. Use for commands whose + // --json body is user-authored and coupled to the endpoint's shape. + FallbackEnvOnly +) + // Spec describes a single SDK-backed subcommand. type Spec[Req any] struct { Use string @@ -34,6 +50,23 @@ type Spec[Req any] struct { // Set false for list/read commands where the request body is optional. JSONRequired bool + // Endpoint is the platform API path used in fallback warnings and + // gate-closed errors (e.g. "/api/agents/search"). Required when + // LegacyRun is set. + Endpoint string + + // FallbackMode selects the gate-closed behavior when LegacyRun is set. + // The zero value is FallbackAuto. + FallbackMode FallbackMode + + // LegacyRun, when non-nil, marks this command platform-first: Run is the + // platform API call, and LegacyRun serves the classic API instead when + // GLEAN_LEGACY_APIS=1 or as the gate-closed fallback (FallbackAuto). + // It receives the normalized --json bytes (snake_case keys — the CLI's + // canonical input shape) so it can unmarshal the classic request type, + // which differs from the platform Req type. + LegacyRun func(ctx context.Context, sdk *glean.Glean, rawJSON []byte) (any, error) + // ErrTransform optionally remaps errors returned by json.Unmarshal before // they are surfaced to the user. Use it to replace Go type names in SDK // enum validation errors with human-readable messages. The transform must @@ -50,6 +83,7 @@ type Spec[Req any] struct { // The payload must be the inner component field — NOT the operation wrapper // (e.g. return resp.ListCollectionsResponse, not resp). // Return (nil, nil) for operations with no response body (delete, etc.). + // When LegacyRun is set, Run is the platform API call. Run func(ctx context.Context, sdk *glean.Glean, req Req) (any, error) } @@ -74,9 +108,18 @@ func Build[Req any](spec Spec[Req]) *cobra.Command { return fmt.Errorf("--json is required\n\nRun '%s --help' for the expected payload format", cmd.CommandPath()) } - var req Req + var normalized []byte if jsonPayload != "" { - normalized := normalizeKeys([]byte(jsonPayload), aliases) + normalized = normalizeKeys([]byte(jsonPayload), aliases) + } + + // In legacy mode the payload is parsed by LegacyRun against the + // classic request type, so skip the platform-type parse (the two + // shapes differ and a legacy payload may not satisfy it). + legacyMode := spec.LegacyRun != nil && platform.Legacy() + + var req Req + if len(normalized) > 0 && !legacyMode { if err := json.Unmarshal(normalized, &req); err != nil { if spec.ErrTransform != nil { return spec.ErrTransform(err) @@ -86,6 +129,17 @@ func Build[Req any](spec Spec[Req]) *cobra.Command { } if dryRun { + if legacyMode { + // The classic request type is known only to LegacyRun; + // show the normalized payload as-is. + payload := map[string]any{} + if len(normalized) > 0 { + if err := json.Unmarshal(normalized, &payload); err != nil { + return fmt.Errorf("invalid --json: %w", err) + } + } + return output.WriteJSON(cmd.OutOrStdout(), payload) + } return output.WriteJSON(cmd.OutOrStdout(), req) } @@ -94,7 +148,23 @@ func Build[Req any](spec Spec[Req]) *cobra.Command { return err } - result, err := spec.Run(cmd.Context(), sdk, req) + var result any + switch { + case spec.LegacyRun == nil: + result, err = spec.Run(cmd.Context(), sdk, req) + case legacyMode: + result, err = spec.LegacyRun(cmd.Context(), sdk, normalized) + case spec.FallbackMode == FallbackEnvOnly: + result, err = spec.Run(cmd.Context(), sdk, req) + if err != nil && platform.IsGateClosed(err) { + return platform.GateClosedErr(spec.Endpoint, err) + } + default: // FallbackAuto + result, _, err = platform.Run(cmd.Context(), cmd.ErrOrStderr(), spec.Endpoint, + func(ctx context.Context) (any, error) { return spec.Run(ctx, sdk, req) }, + func(ctx context.Context) (any, error) { return spec.LegacyRun(ctx, sdk, normalized) }, + ) + } if err != nil { return err } diff --git a/internal/cmdutil/factory_test.go b/internal/cmdutil/factory_test.go new file mode 100644 index 0000000..fc34e8b --- /dev/null +++ b/internal/cmdutil/factory_test.go @@ -0,0 +1,189 @@ +package cmdutil + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "testing" + + glean "github.com/gleanwork/api-client-go" + "github.com/gleanwork/api-client-go/models/apierrors" + "github.com/gleanwork/glean-cli/internal/platform" + "github.com/gleanwork/glean-cli/internal/testutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeReq is a platform-shaped request type with a snake_case JSON tag so +// buildAliases produces the agentId → agent_id normalization. +type fakeReq struct { + AgentID string `json:"agent_id"` +} + +func gateClosed() error { + return &apierrors.PlatformProblemDetailError{Status: http.StatusNotFound} +} + +// execSpec builds the command from spec, executes it with args, and returns +// stdout, stderr, and the execution error. +func execSpec(t *testing.T, spec Spec[fakeReq], args ...string) (string, string, error) { + t.Helper() + _, cleanup := testutils.SetupTestWithResponse(t, []byte(`{}`)) + defer cleanup() + + cmd := Build(spec) + out, errOut := &bytes.Buffer{}, &bytes.Buffer{} + cmd.SetOut(out) + cmd.SetErr(errOut) + cmd.SetArgs(args) + err := cmd.Execute() + return out.String(), errOut.String(), err +} + +func TestBuild_PlatformFirst_Success(t *testing.T) { + platform.ResetWarnings() + t.Setenv(platform.EnvLegacy, "") + + spec := Spec[fakeReq]{ + Use: "get", + Endpoint: "/api/agents/{agent_id}", + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { + return map[string]string{"via": "platform", "id": req.AgentID}, nil + }, + LegacyRun: func(ctx context.Context, sdk *glean.Glean, rawJSON []byte) (any, error) { + t.Fatal("LegacyRun must not run on platform success") + return nil, nil + }, + } + out, errOut, err := execSpec(t, spec, "--json", `{"agentId":"a-1"}`) + require.NoError(t, err) + assert.Contains(t, out, `"via": "platform"`) + assert.Contains(t, out, `"id": "a-1"`, "camelCase input must normalize to the snake_case platform field") + assert.Empty(t, errOut) +} + +func TestBuild_FallbackAuto_GateClosedUsesLegacy(t *testing.T) { + platform.ResetWarnings() + t.Setenv(platform.EnvLegacy, "") + + spec := Spec[fakeReq]{ + Use: "get", + Endpoint: "/api/agents/{agent_id}", + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { + return nil, gateClosed() + }, + LegacyRun: func(ctx context.Context, sdk *glean.Glean, rawJSON []byte) (any, error) { + var m map[string]string + require.NoError(t, json.Unmarshal(rawJSON, &m)) + return map[string]string{"via": "legacy", "id": m["agent_id"]}, nil + }, + } + out, errOut, err := execSpec(t, spec, "--json", `{"agentId":"a-1"}`) + require.NoError(t, err) + assert.Contains(t, out, `"via": "legacy"`) + assert.Contains(t, out, `"id": "a-1"`, "LegacyRun receives the normalized snake_case payload") + assert.Contains(t, errOut, "falling back to the legacy API") +} + +func TestBuild_FallbackEnvOnly_GateClosedErrors(t *testing.T) { + platform.ResetWarnings() + t.Setenv(platform.EnvLegacy, "") + + spec := Spec[fakeReq]{ + Use: "run", + Endpoint: "/api/agents/{agent_id}/runs", + FallbackMode: FallbackEnvOnly, + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { + return nil, gateClosed() + }, + LegacyRun: func(ctx context.Context, sdk *glean.Glean, rawJSON []byte) (any, error) { + t.Fatal("LegacyRun must not auto-run in EnvOnly mode") + return nil, nil + }, + } + _, _, err := execSpec(t, spec, "--json", `{"agent_id":"a-1"}`) + require.Error(t, err) + assert.Contains(t, err.Error(), platform.EnvLegacy) + assert.Contains(t, err.Error(), "/api/agents/{agent_id}/runs") +} + +func TestBuild_FallbackEnvOnly_NonGateErrorPassesThrough(t *testing.T) { + platform.ResetWarnings() + t.Setenv(platform.EnvLegacy, "") + + spec := Spec[fakeReq]{ + Use: "run", + Endpoint: "/api/agents/{agent_id}/runs", + FallbackMode: FallbackEnvOnly, + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { + return nil, &apierrors.PlatformProblemDetailError{Status: http.StatusUnauthorized, Title: "auth required"} + }, + } + _, _, err := execSpec(t, spec, "--json", `{"agent_id":"a-1"}`) + require.Error(t, err) + assert.NotContains(t, err.Error(), platform.EnvLegacy, "non-404 errors are not gate-closed guidance") +} + +func TestBuild_LegacyEnv_UsesLegacyRunDirectly(t *testing.T) { + platform.ResetWarnings() + t.Setenv(platform.EnvLegacy, "1") + + spec := Spec[fakeReq]{ + Use: "get", + Endpoint: "/api/agents/{agent_id}", + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { + t.Fatal("platform Run must not execute in legacy mode") + return nil, nil + }, + LegacyRun: func(ctx context.Context, sdk *glean.Glean, rawJSON []byte) (any, error) { + return map[string]string{"via": "legacy"}, nil + }, + } + out, errOut, err := execSpec(t, spec, "--json", `{"agentId":"a-1"}`) + require.NoError(t, err) + assert.Contains(t, out, `"via": "legacy"`) + assert.Empty(t, errOut, "explicit legacy opt-out produces no warning") +} + +func TestBuild_NoLegacyRun_BehavesClassically(t *testing.T) { + t.Setenv(platform.EnvLegacy, "") + + spec := Spec[fakeReq]{ + Use: "get", + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { + return map[string]string{"id": req.AgentID}, nil + }, + } + out, _, err := execSpec(t, spec, "--json", `{"agent_id":"a-1"}`) + require.NoError(t, err) + assert.Contains(t, out, `"id": "a-1"`) +} + +func TestBuild_DryRun_PlatformShape(t *testing.T) { + t.Setenv(platform.EnvLegacy, "") + + spec := Spec[fakeReq]{ + Use: "get", + Endpoint: "/api/agents/{agent_id}", + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { return nil, nil }, + LegacyRun: func(ctx context.Context, sdk *glean.Glean, rawJSON []byte) (any, error) { return nil, nil }, + } + out, _, err := execSpec(t, spec, "--json", `{"agentId":"a-1"}`, "--dry-run") + require.NoError(t, err) + assert.Contains(t, out, `"agent_id": "a-1"`) +} + +func TestBuild_DryRun_LegacyEnvShowsNormalizedPayload(t *testing.T) { + t.Setenv(platform.EnvLegacy, "1") + + spec := Spec[fakeReq]{ + Use: "get", + Endpoint: "/api/agents/{agent_id}", + Run: func(ctx context.Context, sdk *glean.Glean, req fakeReq) (any, error) { return nil, nil }, + LegacyRun: func(ctx context.Context, sdk *glean.Glean, rawJSON []byte) (any, error) { return nil, nil }, + } + out, _, err := execSpec(t, spec, "--json", `{"agentId":"a-1"}`, "--dry-run") + require.NoError(t, err) + assert.Contains(t, out, `"agent_id": "a-1"`) +}