feat: add agent source tracking to user-agent header#280
Conversation
Generalize agent source tracking beyond Claude Code. Add an internal/agent package mirroring Hugging Face's public agent-harnesses registry, detecting ~20 harnesses (Claude Code, Codex, Cursor, Gemini CLI, Copilot, opencode, and more) plus the standard AI_AGENT/AGENT fallback, with the same first-match priority ordering. Both the REST client and the legacy GraphQL query path now share the single agent.Detect() helper, removing the duplicated inline CLAUDECODE checks so behavior can't drift. CON-85
bb84917 to
bb15158
Compare
TimPietruskyRunPod
left a comment
There was a problem hiding this comment.
Clean, well-tested feature and verified live against production across the REST, GraphQL, and legacy paths — every path emits the (via <agent>) tag and the API accepts it. Rebased onto main is green (go build/vet/test ./...).
Requesting changes on two points before merge, both small:
- Bare
AGENTfallback risks false attribution (medium) — it's a widely-used variable, so unrelated environments will silently tag traffic with arbitrary strings and pollute the very analytics this feature produces. - Env value flows unsanitized into the User-Agent (low) — not injectable (Go rejects CRLF at write time), but spaces/junk yield a malformed UA. A trim + charset/length clamp would harden it.
Non-blocking nit left inline on the duplicated UA assembly. Happy to fold the fixes in directly if you'd prefer.
|
|
||
| // standardEnvVars are generic variables any tool can set to identify itself. | ||
| // When set, the value is used directly as the agent id. | ||
| var standardEnvVars = []string{"AI_AGENT", "AGENT"} |
There was a problem hiding this comment.
medium — drop bare AGENT or namespace it. AGENT is a very common variable (CI runners, tmux/ssh setups, ad-hoc scripts), and its raw value is used directly as the agent id. Unrelated environments will silently tag traffic with arbitrary strings, polluting the analytics this feature exists to produce. AI_AGENT is specific enough to keep; I'd remove bare AGENT or gate it behind a RUNPOD_-namespaced opt-in.
| } | ||
| for _, env := range standardEnvVars { | ||
| if v := os.Getenv(env); v != "" { | ||
| return v |
There was a problem hiding this comment.
low — sanitize the fallback value before it reaches the header. This value is concatenated straight into the User-Agent. It isn't header injection (net/http rejects CRLF at write time), but a value with spaces/junk produces a malformed UA. Suggest strings.TrimSpace plus a charset/length clamp (e.g. keep [A-Za-z0-9._-], cap ~64) here before returning.
|
|
||
| sanitizedVersion := strings.TrimRight(Version, "\r\n") | ||
| userAgent := "RunPod-CLI/" + sanitizedVersion + " (" + runtime.GOOS + "; " + runtime.GOARCH + ")" | ||
| if a := agent.Detect(); a != "" { |
There was a problem hiding this comment.
nit (non-blocking) — the UA string is now assembled in two places: internal/api.buildUserAgent() (runpod-cli/...) and here inline (RunPod-CLI/..., different casing, empty version in non-release builds). The package split makes sharing awkward and the casing is pre-existing, so fine to leave — but worth a follow-up to centralize so the (via ...) tag can't drift between paths.
bare AGENT is too common in unrelated environments (ci runners, shell setups) and would attribute traffic to arbitrary values; only AI_AGENT is honored now. the standard value is trimmed and clamped to user-agent-safe chars ([A-Za-z0-9._-], 64 cap) so it can't produce a malformed header.
|
Pushed
added unit tests for both (bare-AGENT-ignored, injection-stripped, length cap). verified live against production: |
both internal/api and the legacy api/query.go built the agent tag inline, so the format could drift between paths. add agent.Suffix() as the single source for the fragment; callers keep their own (intentionally different) cli prefixes.
Appends coding agent source tags to the User-Agent header on all outgoing API requests when the corresponding environment variable is set (e.g.
CLAUDECODE=1adds(via claude-code)). Covers both the REST client and GraphQL client paths.CON-85