Skip to content

Commit f147dcd

Browse files
committed
fix: report curated command name in X-Wherobots-Client (WBC-182)
Curated `job-runs` commands reuse the shared api-subtree operations returned by findOperation (e.g. `POST /runs`), whose CommandPath was assigned the api-tree name (`runs.<verb>`) in builder.go. Deriving the header's cmd= field from op.CommandPath therefore misattributed curated traffic as `runs.create` / `runs.list` instead of the user-facing `job-runs.create` / `job-runs.list`. Thread the invoked command name through the request context: the `job-runs` parent's PersistentPreRunE stamps the leaf command's dotted user-facing path (curatedCommandName) onto the context, and BuildRequest prefers that over op.CommandPath (falling back to op.CommandPath for the dynamic api subtree, which is already correct). Context propagation covers every request a curated command issues, including internal helper calls (org lookup, upload-url, run polling) without touching their signatures, and avoids mutating the shared operation pointers. Add end-to-end tests exercising the curated path through BuildRootCommand + httptest asserting `cmd=job-runs.create` / `cmd=job-runs.list` on the wire, a guard that `api runs list` still reports `runs.list`, and executor unit tests for the context override and its fallback. Claude-Session: https://claude.ai/code/session_01QP5saxNQTUTJpMRRxZXhzR
1 parent bde1e8b commit f147dcd

4 files changed

Lines changed: 219 additions & 1 deletion

File tree

internal/commands/jobs.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,17 @@ func addJobsCustomCommands(root *cobra.Command, cfg config.Config, runtimeSpec *
6868
Short: "Custom job-runs workflows",
6969
SilenceUsage: true,
7070
SilenceErrors: true,
71+
// Curated commands reuse shared api-subtree operations whose CommandPath
72+
// is the api-tree name (e.g. runs.create). Stamp the invoked user-facing
73+
// command name onto the context so BuildRequest reports it (e.g.
74+
// job-runs.create) in the advisory X-Wherobots-Client header instead.
75+
// PersistentPreRunE runs for the invoked subcommand, so `cmd` is the leaf
76+
// (create/logs/list/metrics), and the context propagates to every request
77+
// those commands issue, including internal helper calls.
78+
PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
79+
cmd.SetContext(executor.WithCommand(cmd.Context(), curatedCommandName(cmd)))
80+
return nil
81+
},
7182
RunE: func(cmd *cobra.Command, _ []string) error {
7283
return cmd.Help()
7384
},
@@ -111,6 +122,25 @@ func newJobsRunner(cfg config.Config, runtimeSpec *spec.RuntimeSpec, client *htt
111122
return r, true
112123
}
113124

125+
// curatedCommandName returns the invoked command's user-facing dotted path,
126+
// excluding the root application name — e.g. `wherobots job-runs create`
127+
// becomes "job-runs.create". It walks the parent chain, so the value reflects
128+
// the actually-invoked command regardless of which shared operation it reuses.
129+
func curatedCommandName(cmd *cobra.Command) string {
130+
if cmd == nil {
131+
return ""
132+
}
133+
var segments []string
134+
for c := cmd; c != nil && c.Parent() != nil; c = c.Parent() {
135+
segments = append(segments, c.Name())
136+
}
137+
// Reverse into root-to-leaf order.
138+
for i, j := 0, len(segments)-1; i < j; i, j = i+1, j-1 {
139+
segments[i], segments[j] = segments[j], segments[i]
140+
}
141+
return strings.Join(segments, ".")
142+
}
143+
114144
func findOperation(runtimeSpec *spec.RuntimeSpec, method, path string) *spec.Operation {
115145
if runtimeSpec == nil {
116146
return nil

internal/commands/jobs_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,112 @@ func TestJobsListDefaultsToText(t *testing.T) {
644644
}
645645
}
646646

647+
func TestJobsListEmitsCuratedCommandInClientHeader(t *testing.T) {
648+
t.Parallel()
649+
650+
var gotClientHeader string
651+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
652+
if r.Method == http.MethodGet && r.URL.Path == "/runs" {
653+
gotClientHeader = r.Header.Get("X-Wherobots-Client")
654+
w.Header().Set("Content-Type", "application/json")
655+
_, _ = io.WriteString(w, `{"items":[],"total":0,"next_page":null}`)
656+
return
657+
}
658+
http.NotFound(w, r)
659+
}))
660+
defer server.Close()
661+
662+
root := buildJobsTestRoot(server.URL)
663+
root.SetOut(&bytes.Buffer{})
664+
root.SetErr(&bytes.Buffer{})
665+
root.SetArgs([]string{"job-runs", "list"})
666+
667+
if err := root.Execute(); err != nil {
668+
t.Fatalf("Execute() error = %v", err)
669+
}
670+
671+
// The curated `job-runs list` command reuses the shared `GET /runs`
672+
// operation, whose api-subtree CommandPath is `runs.<verb>`. The client
673+
// header must report the user-facing curated command, not the shared op.
674+
if !strings.HasSuffix(gotClientHeader, "cmd=job-runs.list") {
675+
t.Fatalf("X-Wherobots-Client = %q, want cmd=job-runs.list", gotClientHeader)
676+
}
677+
}
678+
679+
func TestJobsCreateEmitsCuratedCommandInClientHeader(t *testing.T) {
680+
t.Parallel()
681+
682+
var runsClientHeader string
683+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
684+
switch {
685+
case r.Method == http.MethodGet && r.URL.Path == "/organization":
686+
w.Header().Set("Content-Type", "application/json")
687+
_, _ = io.WriteString(w, `{"fileStore":{"bucketName":"managed-bucket"}}`)
688+
case r.Method == http.MethodPost && r.URL.Path == "/files/upload-url":
689+
w.Header().Set("Content-Type", "application/json")
690+
_, _ = io.WriteString(w, `{"uploadUrl":"https://example.com/upload"}`)
691+
case r.Method == http.MethodGet && r.URL.Path == "/files/dir":
692+
w.Header().Set("Content-Type", "application/json")
693+
_, _ = io.WriteString(w, `{"name":"root","path":"s3://managed-bucket/customer/root"}`)
694+
case r.Method == http.MethodPost && r.URL.Path == "/runs":
695+
runsClientHeader = r.Header.Get("X-Wherobots-Client")
696+
w.Header().Set("Content-Type", "application/json")
697+
_, _ = io.WriteString(w, `{"id":"run-123","name":"test-job-001","status":"PENDING","createTime":"2026-01-01T00:00:00Z","payload":{}}`)
698+
default:
699+
http.NotFound(w, r)
700+
}
701+
}))
702+
defer server.Close()
703+
704+
root := buildJobsTestRoot(server.URL)
705+
root.SetOut(&bytes.Buffer{})
706+
root.SetErr(&bytes.Buffer{})
707+
root.SetArgs([]string{
708+
"job-runs", "create", "s3://bucket/script.py",
709+
"--name", "test-job-001",
710+
"--upload-path", "s3://override-bucket/custom/prefix",
711+
})
712+
713+
if err := root.Execute(); err != nil {
714+
t.Fatalf("Execute() error = %v", err)
715+
}
716+
717+
if !strings.HasSuffix(runsClientHeader, "cmd=job-runs.create") {
718+
t.Fatalf("X-Wherobots-Client on POST /runs = %q, want cmd=job-runs.create", runsClientHeader)
719+
}
720+
}
721+
722+
func TestApiSubtreeEmitsDottedResourceCommandInClientHeader(t *testing.T) {
723+
t.Parallel()
724+
725+
var gotClientHeader string
726+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
727+
if r.Method == http.MethodGet && r.URL.Path == "/runs" {
728+
gotClientHeader = r.Header.Get("X-Wherobots-Client")
729+
w.Header().Set("Content-Type", "application/json")
730+
_, _ = io.WriteString(w, `{"items":[],"total":0,"next_page":null}`)
731+
return
732+
}
733+
http.NotFound(w, r)
734+
}))
735+
defer server.Close()
736+
737+
root := buildJobsTestRoot(server.URL)
738+
root.SetOut(&bytes.Buffer{})
739+
root.SetErr(&bytes.Buffer{})
740+
// The dynamic api subtree keeps its own operation CommandPath; invoking
741+
// it directly must still report the api-tree command name.
742+
root.SetArgs([]string{"api", "runs", "list"})
743+
744+
if err := root.Execute(); err != nil {
745+
t.Fatalf("Execute() error = %v", err)
746+
}
747+
748+
if !strings.HasSuffix(gotClientHeader, "cmd=runs.list") {
749+
t.Fatalf("X-Wherobots-Client = %q, want cmd=runs.list", gotClientHeader)
750+
}
751+
}
752+
647753
func TestJobsRunningAliasFiltersStatus(t *testing.T) {
648754
t.Parallel()
649755

internal/executor/request.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,32 @@ type QueryPair struct {
2525
Value string
2626
}
2727

28+
// commandContextKey carries the user-facing command name for the
29+
// X-Wherobots-Client header's cmd= field. Curated commands (e.g. `job-runs`)
30+
// reuse shared api-subtree operations whose CommandPath is the api-tree name
31+
// (e.g. runs.create), so they inject the invoked command name here to override
32+
// it. Dynamic api commands leave it unset and fall back to op.CommandPath.
33+
type commandContextKey struct{}
34+
35+
// WithCommand returns a context that carries the user-facing command name
36+
// (dotted, e.g. "job-runs.create") for the X-Wherobots-Client header. An empty
37+
// name leaves the context unchanged so the op.CommandPath fallback applies.
38+
func WithCommand(ctx context.Context, command string) context.Context {
39+
if command == "" {
40+
return ctx
41+
}
42+
return context.WithValue(ctx, commandContextKey{}, command)
43+
}
44+
45+
// commandFromContext returns the command name set by WithCommand, or "".
46+
func commandFromContext(ctx context.Context) string {
47+
if ctx == nil {
48+
return ""
49+
}
50+
name, _ := ctx.Value(commandContextKey{}).(string)
51+
return name
52+
}
53+
2854
// clientHeaderName is the ordered, append-only client-identification header.
2955
// The CLI is an ORIGIN client, so it emits a single hop and never appends to
3056
// an existing value. The header is advisory only and never affects auth.
@@ -279,7 +305,14 @@ func BuildRequest(
279305
req.Header.Set("Content-Type", contentType)
280306
}
281307
req.Header.Set("x-api-key", cfg.APIKey)
282-
req.Header.Set(clientHeaderName, buildClientHeader(Version, strings.Join(op.CommandPath, ".")))
308+
// Prefer the invoked command name carried on the context (set by curated
309+
// commands whose shared op.CommandPath is the api-tree name); fall back to
310+
// the operation's own CommandPath for dynamic api commands.
311+
command := commandFromContext(ctx)
312+
if command == "" {
313+
command = strings.Join(op.CommandPath, ".")
314+
}
315+
req.Header.Set(clientHeaderName, buildClientHeader(Version, command))
283316

284317
return req, nil
285318
}

internal/executor/request_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,55 @@ func TestBuildRequestInjectsWherobotsClientHeader(t *testing.T) {
207207
}
208208
}
209209

210+
func TestBuildRequestClientHeaderPrefersContextCommand(t *testing.T) {
211+
// Not parallel: mutates the package-level Version var.
212+
prev := Version
213+
Version = "1.2.3"
214+
t.Cleanup(func() { Version = prev })
215+
216+
cfg := config.Config{APIKey: "abc123"}
217+
runtimeSpec := &spec.RuntimeSpec{BaseURL: "https://api.example.com"}
218+
// op.CommandPath is the shared api-tree name; the context override must win.
219+
op := &spec.Operation{
220+
Method: "POST",
221+
Path: "/runs",
222+
CommandPath: []string{"runs", "create"},
223+
}
224+
225+
ctx := WithCommand(context.Background(), "job-runs.create")
226+
req, err := BuildRequest(ctx, cfg, runtimeSpec, op, nil, nil, `{}`)
227+
if err != nil {
228+
t.Fatalf("BuildRequest() error = %v", err)
229+
}
230+
if got, want := req.Header.Get("X-Wherobots-Client"), "client=cli;ver=1.2.3;cmd=job-runs.create"; got != want {
231+
t.Fatalf("X-Wherobots-Client = %q, want %q", got, want)
232+
}
233+
}
234+
235+
func TestBuildRequestClientHeaderFallsBackToCommandPathWithoutContext(t *testing.T) {
236+
// Not parallel: mutates the package-level Version var.
237+
prev := Version
238+
Version = "1.2.3"
239+
t.Cleanup(func() { Version = prev })
240+
241+
cfg := config.Config{APIKey: "abc123"}
242+
runtimeSpec := &spec.RuntimeSpec{BaseURL: "https://api.example.com"}
243+
op := &spec.Operation{
244+
Method: "POST",
245+
Path: "/runs",
246+
CommandPath: []string{"runs", "create"},
247+
}
248+
249+
// No context command set: fall back to op.CommandPath.
250+
req, err := BuildRequest(context.Background(), cfg, runtimeSpec, op, nil, nil, `{}`)
251+
if err != nil {
252+
t.Fatalf("BuildRequest() error = %v", err)
253+
}
254+
if got, want := req.Header.Get("X-Wherobots-Client"), "client=cli;ver=1.2.3;cmd=runs.create"; got != want {
255+
t.Fatalf("X-Wherobots-Client = %q, want %q", got, want)
256+
}
257+
}
258+
210259
func TestBuildRequestWherobotsClientHeaderOmitsCommandWhenEmpty(t *testing.T) {
211260
// Not parallel: mutates the package-level Version var.
212261
prev := Version

0 commit comments

Comments
 (0)