feat: emit X-Wherobots-Client header with command name (WBC-182)

[ClayMav]

Summary

Implements WBC-182. The CLI now sets an advisory X-Wherobots-Client header on every outgoing API request so the backend can attribute traffic to the CLI and to the specific command that generated it.

The CLI is an ORIGIN client, so it emits a single hop and never appends to an existing value:

X-Wherobots-Client: client=cli;ver=<buildVersion>;cmd=<command>

• cmd is the operation's CommandPath joined with . (e.g. job-runs.list). Populated in internal/commands/builder.go as append(PathToResourceSegments(op.Path), verb).
• The ;cmd= field is omitted when CommandPath is empty.
• Values are sanitized so commas (hop separators) and semicolons (field separators) can never leak into and break the shared-header grammar.
• The header is advisory only and never affects auth.

Implementation

• internal/executor/request.go
  • Added exported Version package var (default "dev").
  • Added a pure buildClientHeader(version, command string) string helper (empty version falls back to dev; empty command omits ;cmd=; sanitizes separators).
  • BuildRequest(...) now sets the header right after x-api-key.
• main.go sets executor.Version = buildVersion at the top of run().

Version wiring approach

buildVersion is a package-level var in main.go, injected via ldflags at build time (default "dev"). It reaches the executor package through an exported executor.Version var that main.go assigns during run() setup. This keeps all version wiring in one place, requires no ldflags changes to the build, and introduces no import cycle (main already depends on internal/executor; internal/executor does not depend on main or internal/commands).

Testing (TDD)

Followed RED -> GREEN:

• RED: added failing tests asserting the header value for a populated CommandPath (client=cli;ver=<version>;cmd=job-runs.list) and the empty-CommandPath case (no ;cmd=); confirmed compile failure (undefined Version / buildClientHeader).
• GREEN: implemented; go test ./... passes; go build ./... and go vet ./... clean.

New tests in internal/executor/request_test.go:

• TestBuildRequestInjectsWherobotsClientHeader
• TestBuildRequestWherobotsClientHeaderOmitsCommandWhenEmpty
• TestBuildClientHeader (table: version+command, empty command, empty version, separator sanitization)

The two BuildRequest tests mutate the package-level Version and are deliberately non-parallel (verified clean under go test -race).

Rubric self-assessment

1. Correctness — Header value matches the spec grammar exactly; command joined with .; ;cmd= omitted when empty. Verified by tests.
2. Scope — Touches only request.go, its test, and one line + import in main.go. No other submodules or staging/prod.
3. Testing — Pure helper unit-tested across 4 cases; integration asserted via BuildRequest on the returned *http.Request; full suite + -race green.
4. Readability — Small pure helper, documented package vars/consts explaining the origin-hop/advisory-only semantics.
5. Safety / auth — Header is advisory only; does not touch or depend on the x-api-key path; cannot affect auth.
6. Robustness — Sanitizes commas/semicolons out of values so malformed input can't break the shared grammar; empty version falls back to dev.
7. Architecture — Version threaded via an exported executor.Version var set from main.go; no import cycle; wiring centralized.
8. Conventions — Conventional commit, go fmt clean, matches existing request_test.go style and repo CLAUDE.md.

Caveats

• Version is threaded by assigning executor.Version = buildVersion in run() rather than injecting ldflags directly onto the executor var. If a future entrypoint calls BuildRequest without going through run(), the header would report dev until executor.Version is set.

Linear: WBC-182

https://claude.ai/code/session_01QP5saxNQTUTJpMRRxZXhzR

[ClayMav]

@salty-hambot rubric

[salty-hambot]

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ⚠️ concerns

Dimension	Verdict	Notes
correctness	⚠️ concerns	Confirmed via jobs.go/builder.go: curated job-runs commands reuse the api-subtree ops from findOperation, whose CommandPath is set to PathToResourceSegments(op.Path)+verb (builder.go:109) — so cmd= misreports job-runs.* invocations as runs..
security	✅ pass	Header is advisory-only, sanitized against grammar-breaking chars, and never touches the x-api-key auth path.
privacy	✅ pass	Only CLI version and command name are emitted; no credentials or PII enter the header.
reliability	⚠️ concerns	executor.Version is a package-level var set in run(); any future entrypoint or test calling BuildRequest before run() silently emits 'dev' — acknowledged but unguarded.
scalability	✅ pass	strings.Builder with a few small writes; NewReplacer allocated once at init — no concerns.
observability	✅ pass	Header propagates command attribution and appears automatically in --dry-run curl output — though the cmd= value is wrong for curated commands (see correctness).
clarity/maintainability	✅ pass	Well-documented pure helper; change confined to request.go plus one assignment in main.go; consistent with surrounding style.
test quality	⚠️ concerns	Unit and integration tests are solid, but they hand-set CommandPath to []string{"job-runs","list"} and never exercise the curated job-runs command path end-to-end — masking the real misattribution bug.

Tidy, well-tested header PR — but the reviewer waved through the one real bug: curated job-runs commands share the API-subtree ops, so cmd= emits runs.<verb> instead of the user-facing job-runs.create. The two 'questions' were self-answering nitpicks; dropped them.

_{0 finding(s) posted · 2 filtered as false positives · 1 skipped as already raised.
💬 To request a re-review, comment @salty-hambot review}

[ClayMav]

@salty-hambot rubric

[salty-hambot]

Reviewed by Salty Hambot 🤖🧂 — rubric mode

Verdict: ⚠️ concerns

Dimension	Verdict	Notes
correctness	✅ pass	Prior misattribution bug (runs.list vs job-runs.list) is fully fixed via PersistentPreRunE + context override; tests confirm both curated and dynamic paths.
security	✅ pass	Header is advisory-only, values sanitized against comma/semicolon, and the x-api-key auth path is untouched.
privacy	✅ pass	Only CLI version and user-invoked command name are emitted — no credentials or PII.
reliability	⚠️ concerns	executor.Version is a package-level var set in run(); a future entrypoint calling BuildRequest before run() would silently emit 'dev' — acknowledged, unguarded, low impact.
scalability	✅ pass	strings.Builder with a few small writes; NewReplacer allocated once at init — no concerns.
observability	✅ pass	Header now propagates correct user-facing attribution on every request and in --dry-run curl output.
clarity/maintainability	✅ pass	curatedCommandName is a clean pure helper, context-key pattern is idiomatic, and the change is tightly scoped with clear comments.
test quality	✅ pass	Three integration tests cover curated list, curated create, and dynamic api-subtree end-to-end; unit tests cover buildClientHeader and context-preference/fallback — prior masking gap closed.

Round two and the one real bug is properly dead — curated job-runs commands now stamp their user-facing name onto the context, BuildRequest honors it, and three end-to-end tests actually exercise the routing this time instead of hand-faking CommandPath. The dev-version caveat lingers but it's acknowledged and low-stakes. Ship it.

Prior findings: ✅ 2 resolved · ⚠️ 1 still open

_{0 finding(s) posted.
💬 To request a re-review, comment @salty-hambot review}