diff --git a/internal/commands/jobs.go b/internal/commands/jobs.go index eac6089..02cb098 100644 --- a/internal/commands/jobs.go +++ b/internal/commands/jobs.go @@ -68,6 +68,17 @@ func addJobsCustomCommands(root *cobra.Command, cfg config.Config, runtimeSpec * Short: "Custom job-runs workflows", SilenceUsage: true, SilenceErrors: true, + // Curated commands reuse shared api-subtree operations whose CommandPath + // is the api-tree name (e.g. runs.create). Stamp the invoked user-facing + // command name onto the context so BuildRequest reports it (e.g. + // job-runs.create) in the advisory X-Wherobots-Client header instead. + // PersistentPreRunE runs for the invoked subcommand, so `cmd` is the leaf + // (create/logs/list/metrics), and the context propagates to every request + // those commands issue, including internal helper calls. + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + cmd.SetContext(executor.WithCommand(cmd.Context(), curatedCommandName(cmd))) + return nil + }, RunE: func(cmd *cobra.Command, _ []string) error { return cmd.Help() }, @@ -111,6 +122,25 @@ func newJobsRunner(cfg config.Config, runtimeSpec *spec.RuntimeSpec, client *htt return r, true } +// curatedCommandName returns the invoked command's user-facing dotted path, +// excluding the root application name — e.g. `wherobots job-runs create` +// becomes "job-runs.create". It walks the parent chain, so the value reflects +// the actually-invoked command regardless of which shared operation it reuses. +func curatedCommandName(cmd *cobra.Command) string { + if cmd == nil { + return "" + } + var segments []string + for c := cmd; c != nil && c.Parent() != nil; c = c.Parent() { + segments = append(segments, c.Name()) + } + // Reverse into root-to-leaf order. + for i, j := 0, len(segments)-1; i < j; i, j = i+1, j-1 { + segments[i], segments[j] = segments[j], segments[i] + } + return strings.Join(segments, ".") +} + func findOperation(runtimeSpec *spec.RuntimeSpec, method, path string) *spec.Operation { if runtimeSpec == nil { return nil diff --git a/internal/commands/jobs_test.go b/internal/commands/jobs_test.go index 7eb92d5..2b0236b 100644 --- a/internal/commands/jobs_test.go +++ b/internal/commands/jobs_test.go @@ -644,6 +644,112 @@ func TestJobsListDefaultsToText(t *testing.T) { } } +func TestJobsListEmitsCuratedCommandInClientHeader(t *testing.T) { + t.Parallel() + + var gotClientHeader string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/runs" { + gotClientHeader = r.Header.Get("X-Wherobots-Client") + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"items":[],"total":0,"next_page":null}`) + return + } + http.NotFound(w, r) + })) + defer server.Close() + + root := buildJobsTestRoot(server.URL) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{"job-runs", "list"}) + + if err := root.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + + // The curated `job-runs list` command reuses the shared `GET /runs` + // operation, whose api-subtree CommandPath is `runs.`. The client + // header must report the user-facing curated command, not the shared op. + if !strings.HasSuffix(gotClientHeader, "cmd=job-runs.list") { + t.Fatalf("X-Wherobots-Client = %q, want cmd=job-runs.list", gotClientHeader) + } +} + +func TestJobsCreateEmitsCuratedCommandInClientHeader(t *testing.T) { + t.Parallel() + + var runsClientHeader string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/organization": + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"fileStore":{"bucketName":"managed-bucket"}}`) + case r.Method == http.MethodPost && r.URL.Path == "/files/upload-url": + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"uploadUrl":"https://example.com/upload"}`) + case r.Method == http.MethodGet && r.URL.Path == "/files/dir": + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"name":"root","path":"s3://managed-bucket/customer/root"}`) + case r.Method == http.MethodPost && r.URL.Path == "/runs": + runsClientHeader = r.Header.Get("X-Wherobots-Client") + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"id":"run-123","name":"test-job-001","status":"PENDING","createTime":"2026-01-01T00:00:00Z","payload":{}}`) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + root := buildJobsTestRoot(server.URL) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{ + "job-runs", "create", "s3://bucket/script.py", + "--name", "test-job-001", + "--upload-path", "s3://override-bucket/custom/prefix", + }) + + if err := root.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if !strings.HasSuffix(runsClientHeader, "cmd=job-runs.create") { + t.Fatalf("X-Wherobots-Client on POST /runs = %q, want cmd=job-runs.create", runsClientHeader) + } +} + +func TestApiSubtreeEmitsDottedResourceCommandInClientHeader(t *testing.T) { + t.Parallel() + + var gotClientHeader string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && r.URL.Path == "/runs" { + gotClientHeader = r.Header.Get("X-Wherobots-Client") + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"items":[],"total":0,"next_page":null}`) + return + } + http.NotFound(w, r) + })) + defer server.Close() + + root := buildJobsTestRoot(server.URL) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + // The dynamic api subtree keeps its own operation CommandPath; invoking + // it directly must still report the api-tree command name. + root.SetArgs([]string{"api", "runs", "list"}) + + if err := root.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if !strings.HasSuffix(gotClientHeader, "cmd=runs.list") { + t.Fatalf("X-Wherobots-Client = %q, want cmd=runs.list", gotClientHeader) + } +} + func TestJobsRunningAliasFiltersStatus(t *testing.T) { t.Parallel() diff --git a/internal/executor/request.go b/internal/executor/request.go index 5927515..7a27bba 100644 --- a/internal/executor/request.go +++ b/internal/executor/request.go @@ -15,11 +15,73 @@ import ( "wherobots/cli/internal/spec" ) +// Version identifies the CLI build for the advisory X-Wherobots-Client header. +// main.go overwrites it with the ldflags-injected build version; the "dev" +// default keeps local and test builds working without wiring. +var Version = "dev" + type QueryPair struct { Key string Value string } +// commandContextKey carries the user-facing command name for the +// X-Wherobots-Client header's cmd= field. Curated commands (e.g. `job-runs`) +// reuse shared api-subtree operations whose CommandPath is the api-tree name +// (e.g. runs.create), so they inject the invoked command name here to override +// it. Dynamic api commands leave it unset and fall back to op.CommandPath. +type commandContextKey struct{} + +// WithCommand returns a context that carries the user-facing command name +// (dotted, e.g. "job-runs.create") for the X-Wherobots-Client header. An empty +// name leaves the context unchanged so the op.CommandPath fallback applies. +func WithCommand(ctx context.Context, command string) context.Context { + if command == "" { + return ctx + } + return context.WithValue(ctx, commandContextKey{}, command) +} + +// commandFromContext returns the command name set by WithCommand, or "". +func commandFromContext(ctx context.Context) string { + if ctx == nil { + return "" + } + name, _ := ctx.Value(commandContextKey{}).(string) + return name +} + +// clientHeaderName is the ordered, append-only client-identification header. +// The CLI is an ORIGIN client, so it emits a single hop and never appends to +// an existing value. The header is advisory only and never affects auth. +const clientHeaderName = "X-Wherobots-Client" + +// clientHeaderSanitizer strips characters that would break the hop grammar +// (commas separate hops, semicolons separate fields) so they can never appear +// inside a value. +var clientHeaderSanitizer = strings.NewReplacer(",", "_", ";", "_") + +// buildClientHeader renders this CLI's single origin hop: +// +// client=cli;ver=;cmd= +// +// The cmd field is omitted when command is empty. Values are sanitized so no +// comma or semicolon leaks into the grammar. An empty version falls back to +// "dev" to match the package default. +func buildClientHeader(version, command string) string { + if version == "" { + version = "dev" + } + var b strings.Builder + b.WriteString("client=cli;ver=") + b.WriteString(clientHeaderSanitizer.Replace(version)) + if command != "" { + b.WriteString(";cmd=") + b.WriteString(clientHeaderSanitizer.Replace(command)) + } + return b.String() +} + type HTTPError struct { StatusCode int Body []byte @@ -243,6 +305,14 @@ func BuildRequest( req.Header.Set("Content-Type", contentType) } req.Header.Set("x-api-key", cfg.APIKey) + // Prefer the invoked command name carried on the context (set by curated + // commands whose shared op.CommandPath is the api-tree name); fall back to + // the operation's own CommandPath for dynamic api commands. + command := commandFromContext(ctx) + if command == "" { + command = strings.Join(op.CommandPath, ".") + } + req.Header.Set(clientHeaderName, buildClientHeader(Version, command)) return req, nil } diff --git a/internal/executor/request_test.go b/internal/executor/request_test.go index ebd02b5..b104f94 100644 --- a/internal/executor/request_test.go +++ b/internal/executor/request_test.go @@ -184,6 +184,143 @@ func TestBuildRequestInjectsPathQueryBodyAndAuth(t *testing.T) { } } +func TestBuildRequestInjectsWherobotsClientHeader(t *testing.T) { + // Not parallel: mutates the package-level Version var. + prev := Version + Version = "1.2.3" + t.Cleanup(func() { Version = prev }) + + cfg := config.Config{APIKey: "abc123"} + runtimeSpec := &spec.RuntimeSpec{BaseURL: "https://api.example.com"} + op := &spec.Operation{ + Method: "GET", + Path: "/job-runs", + CommandPath: []string{"job-runs", "list"}, + } + + req, err := BuildRequest(context.Background(), cfg, runtimeSpec, op, nil, nil, "") + if err != nil { + t.Fatalf("BuildRequest() error = %v", err) + } + if got, want := req.Header.Get("X-Wherobots-Client"), "client=cli;ver=1.2.3;cmd=job-runs.list"; got != want { + t.Fatalf("X-Wherobots-Client = %q, want %q", got, want) + } +} + +func TestBuildRequestClientHeaderPrefersContextCommand(t *testing.T) { + // Not parallel: mutates the package-level Version var. + prev := Version + Version = "1.2.3" + t.Cleanup(func() { Version = prev }) + + cfg := config.Config{APIKey: "abc123"} + runtimeSpec := &spec.RuntimeSpec{BaseURL: "https://api.example.com"} + // op.CommandPath is the shared api-tree name; the context override must win. + op := &spec.Operation{ + Method: "POST", + Path: "/runs", + CommandPath: []string{"runs", "create"}, + } + + ctx := WithCommand(context.Background(), "job-runs.create") + req, err := BuildRequest(ctx, cfg, runtimeSpec, op, nil, nil, `{}`) + if err != nil { + t.Fatalf("BuildRequest() error = %v", err) + } + if got, want := req.Header.Get("X-Wherobots-Client"), "client=cli;ver=1.2.3;cmd=job-runs.create"; got != want { + t.Fatalf("X-Wherobots-Client = %q, want %q", got, want) + } +} + +func TestBuildRequestClientHeaderFallsBackToCommandPathWithoutContext(t *testing.T) { + // Not parallel: mutates the package-level Version var. + prev := Version + Version = "1.2.3" + t.Cleanup(func() { Version = prev }) + + cfg := config.Config{APIKey: "abc123"} + runtimeSpec := &spec.RuntimeSpec{BaseURL: "https://api.example.com"} + op := &spec.Operation{ + Method: "POST", + Path: "/runs", + CommandPath: []string{"runs", "create"}, + } + + // No context command set: fall back to op.CommandPath. + req, err := BuildRequest(context.Background(), cfg, runtimeSpec, op, nil, nil, `{}`) + if err != nil { + t.Fatalf("BuildRequest() error = %v", err) + } + if got, want := req.Header.Get("X-Wherobots-Client"), "client=cli;ver=1.2.3;cmd=runs.create"; got != want { + t.Fatalf("X-Wherobots-Client = %q, want %q", got, want) + } +} + +func TestBuildRequestWherobotsClientHeaderOmitsCommandWhenEmpty(t *testing.T) { + // Not parallel: mutates the package-level Version var. + prev := Version + Version = "4.5.6" + t.Cleanup(func() { Version = prev }) + + cfg := config.Config{APIKey: "abc123"} + runtimeSpec := &spec.RuntimeSpec{BaseURL: "https://api.example.com"} + op := &spec.Operation{Method: "GET", Path: "/job-runs"} + + req, err := BuildRequest(context.Background(), cfg, runtimeSpec, op, nil, nil, "") + if err != nil { + t.Fatalf("BuildRequest() error = %v", err) + } + if got, want := req.Header.Get("X-Wherobots-Client"), "client=cli;ver=4.5.6"; got != want { + t.Fatalf("X-Wherobots-Client = %q, want %q", got, want) + } +} + +func TestBuildClientHeader(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + version string + command string + want string + }{ + { + name: "version and command", + version: "1.2.3", + command: "job-runs.list", + want: "client=cli;ver=1.2.3;cmd=job-runs.list", + }, + { + name: "empty command omits cmd", + version: "1.2.3", + command: "", + want: "client=cli;ver=1.2.3", + }, + { + name: "empty version falls back to dev", + version: "", + command: "job-runs.list", + want: "client=cli;ver=dev;cmd=job-runs.list", + }, + { + name: "sanitizes separators out of values", + version: "1,2;3", + command: "a;b,c.list", + want: "client=cli;ver=1_2_3;cmd=a_b_c.list", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := buildClientHeader(tc.version, tc.command); got != tc.want { + t.Fatalf("buildClientHeader(%q, %q) = %q, want %q", tc.version, tc.command, got, tc.want) + } + }) + } +} + func TestBuildRequestMissingRequiredQueryReturnsError(t *testing.T) { t.Parallel() diff --git a/main.go b/main.go index 35136c3..f08d6f3 100644 --- a/main.go +++ b/main.go @@ -8,6 +8,7 @@ import ( "wherobots/cli/internal/commands" "wherobots/cli/internal/config" + "wherobots/cli/internal/executor" "wherobots/cli/internal/spec" "wherobots/cli/internal/version" ) @@ -26,6 +27,11 @@ func main() { } func run(ctx context.Context) error { + // Surface the build version to the executor so outgoing requests carry the + // advisory X-Wherobots-Client header. Set here (not via ldflags directly on + // the executor var) to keep version wiring in one place. + executor.Version = buildVersion + // Start a background update check early so it runs in parallel with setup. updateCh := version.CheckInBackground(ctx, buildVersion)