Repository navigation
Conversation
AI review: 5 P0-P2 finding(s)Adds default-on CLI telemetry, agent attribution, and persistent telemetry settings across logout. The main risks are unintended disclosure of local data and incorrect project attribution. 🟠 P1
🟡 P2
⚪ P3
Not checked: The AI review read the diff, the code, and the PR's text; it ran no tests or builds and read nothing outside this repository.
Push a fix, or have a collaborator with write access (including the PR author) add the Human review: needed
Decide the points above: approve to accept them, or request changes. Suggested reviewer: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| | Claude Code | `CLAUDECODE`, `CLAUDE_CODE_ENTRYPOINT` | | ||
| | Codex | `CODEX_THREAD_ID`, `CODEX_SANDBOX`, `CODEX_CI` | | ||
| | Gemini CLI | `GEMINI_CLI` | | ||
| | Cursor | `CURSOR_EXTENSION_HOST_ROLE=agent-exec` | |
There was a problem hiding this comment.
⚪ P3: Docs list Cursor before Cursor CLI, but the code checks Cursor CLI first
The doc says "If several are set, the first row in this table wins" and lists Cursor (CURSOR_EXTENSION_HOST_ROLE=agent-exec) above Cursor CLI (CURSOR_AGENT). In internal/agent/agent.go:92-93, markers puts CURSOR_AGENT → cursor-cli first. Running cursor-agent from a Cursor agent-exec terminal sets both variables, so the CLI reports cursor-cli while the published doc says cursor. Fix: swap the two table rows so they match the code.
Found by claude-opus-5-5
| {env: "ANTIGRAVITY_AGENT", agent: "antigravity"}, | ||
| // COPILOT_GITHUB_TOKEN is left out on purpose: people export it in their | ||
| // own shell profile, so it does not show an agent ran the command. | ||
| {env: "COPILOT_MODEL", agent: "github-copilot"}, |
There was a problem hiding this comment.
⚪ P3: COPILOT_MODEL and COPILOT_ALLOW_ALL may be shell-profile settings, not proof an agent ran
The code skips COPILOT_GITHUB_TOKEN because people export it in their shell profile. As far as I know, Copilot CLI also documents COPILOT_MODEL (choose the model) and COPILOT_ALLOW_ALL (auto-approve tools) as settings users export themselves. If so, a user with export COPILOT_MODEL=... in .zshrc gets every command they type tagged github-copilot with source marker. Because step-2 markers outrank VOLCANO_AGENT, an agent that identifies itself only through VOLCANO_AGENT would also be reported as Copilot. Consider dropping these two markers, or treating them as a host-level signal, unless Copilot sets them in its own command shells.
Found by claude-opus-5-5
|
|
||
| ## Agent detection | ||
|
|
||
| Once set up, the CLI reports which agent ran each command to the Volcano API. |
There was a problem hiding this comment.
⚪ P3: "Once set up" suggests reporting depends on volcano setup
The new section says "Once set up, the CLI reports which agent ran each command." Detection actually runs on every API request whatever the setup state: detectAgent in versionProtocolDoer reads only environment variables and never checks whether volcano setup ran. A user who skips setup could wrongly think nothing is reported. Fix: say the CLI reports the agent on every API request whether or not volcano setup has run.
Found by claude-opus-5-5
| | Claude Code | `CLAUDECODE`, `CLAUDE_CODE_ENTRYPOINT` | | ||
| | Codex | `CODEX_THREAD_ID`, `CODEX_SANDBOX`, `CODEX_CI` | | ||
| | Gemini CLI | `GEMINI_CLI` | | ||
| | Cursor | `CURSOR_EXTENSION_HOST_ROLE=agent-exec` | |
There was a problem hiding this comment.
🟡 P2: Cursor marker precedence is documented in the wrong order
When both CURSOR_EXTENSION_HOST_ROLE=agent-exec and CURSOR_AGENT=1 are set without an earlier match, this table promises cursor because the first row wins. However, internal/agent/agent.go checks CURSOR_AGENT first and reports cursor-cli. Swap these rows to match the implemented precedence.
Found by gpt-6.1-sol
| | `X-Volcano-Agent-Raw` | `claude-code_2-1-299_agent` | The original value of `AI_AGENT`, `AGENT`, or `VOLCANO_AGENT`, when it differs from the name | | ||
| | `X-Volcano-Agent-Markers` | `claude-code,cursor` | Every agent whose step 2 or step 5 variables are set. More than one means one agent ran inside another, for example Claude Code run from a Cursor terminal | | ||
|
|
||
| Values are limited to letters, digits, and `._-/@+:`, and to 64 characters. |
There was a problem hiding this comment.
🟡 P2: The stated header limits do not apply to the markers header
setAgentHeaders in internal/api/version_protocol.go joins marker names with commas without sanitizing or truncating the combined value. Consequently, even the documented claude-code,cursor example contains an excluded character, and sufficiently many markers can exceed 64 characters. Scope this statement to the environment-derived name and raw fields, and describe the marker list separately.
Found by gpt-6.1-sol
| | Augment | `AUGMENT_AGENT` | | ||
| | Antigravity | `ANTIGRAVITY_AGENT` | | ||
| | GitHub Copilot | `COPILOT_MODEL`, `COPILOT_ALLOW_ALL` | | ||
|
|
There was a problem hiding this comment.
⚪ P3: Docs table lists Cursor before Cursor CLI, but the code checks them the other way round
The doc says "If several are set, the first row in this table wins" and puts CURSOR_EXTENSION_HOST_ROLE=agent-exec (Cursor) above CURSOR_AGENT (Cursor CLI). In internal/agent/agent.go the markers slice puts CURSOR_AGENT first. So when both are set, the CLI reports cursor-cli, while the published doc says cursor. Fix: swap the two table rows, or swap the two marker entries, so the doc and code agree.
Found by claude-opus-5-5
tkkhq
changed the title
|
|
||
| func printNotice(w io.Writer) { | ||
| fmt.Fprintln(w, "Note: the Volcano CLI collects usage data: the command and flag names you use (never their values), CLI version, platform, and which coding agent ran it. This helps us improve the CLI.") | ||
| fmt.Fprintf(w, "Turn it off with `volcano telemetry disable` or %s=1. Details: `volcano docs get telemetry`.\n", EnvDisabled) |
There was a problem hiding this comment.
🟡 P2: First-run notice points users to volcano docs get telemetry, which cannot resolve
docs.Service.Get reads the cached snapshot path exactly as given (snap.readFile(docPath) after safeRelPath). It adds no .md extension and does no fuzzy lookup, and doc IDs look like cli/deploy.md. AGENTS.md says this repo's docs/ syncs under /cli, so the new page will be at cli/telemetry.md. Running the exact command the notice prints fails with a doc-not-found error, at the one moment a user is told where to read about data collection. Change the hint to volcano docs get cli/telemetry.md, or point to the published URL.
Found by claude-opus-5-5
| // swallowed, since telemetry must not change what the CLI does. | ||
| func (r Recorder) Record(inv Invocation) { | ||
| r = r.withDefaults() | ||
| if inv.Command != nil && inv.Command.Annotations[SkipAnnotation] != "" { |
There was a problem hiding this comment.
🟡 P2: Every shell-completion request is recorded, spawns a flush process, and can silently use up the first-run notice
Only commands annotated with SkipAnnotation are skipped. Cobra's hidden __complete command (added by ExecuteC because the root does not disable completion) is returned as cmd, so every Tab press records an event. Each one also starts a detached volcano telemetry flush process that makes an HTTP POST. Cobra's bash, zsh, and fish completion scripts run __complete with 2>/dev/null. If a user's first interaction is a Tab completion, printNotice writes to a discarded stderr, NoticeShown is saved as true, and the user never sees the notice. Skip cobra.ShellCompRequestCmd / ShellCompNoDescRequestCmd (and possibly the completion command) in Record.
Found by claude-opus-5-5
| if state.DeviceID == "" { | ||
| state.DeviceID = uuid.NewString() | ||
| } | ||
| if !state.NoticeShown && inv.Stderr != nil { |
There was a problem hiding this comment.
🟡 P2: Notice is printed on every command when ~/.volcano/telemetry.json cannot be written
Record prints the notice and sets NoticeShown, then returns if State.Save fails, so the flag is never saved. When HOME is readable but not writable, every invocation prints the two-line notice to stderr again and sends no event. That covers sandboxed agents (Codex's default workspace-write sandbox, Claude Code's sandbox), read-only containers, and Nix-style builds. These are the agent runs this feature targets, and every command's output picks up the noise. Save the state before printing, and print only when the save succeeds (or skip the notice when state cannot be saved).
Found by claude-opus-5-5
| failure (`api`, `network`, `local_not_running`, `usage`, or `other`) and, for an | ||
| API error, its HTTP status and error code. | ||
|
|
||
| The CLI never sends flag values, command arguments, file contents, project or |
There was a problem hiding this comment.
⚪ P3: Docs say environment variable values are never sent, but free-text agent values are
For an unrecognized AI_AGENT, AGENT, or VOLCANO_AGENT value, canonical returns the sanitized, lowercased value itself. That value goes out as agent in the telemetry event and as X-Volcano-Agent/X-Volcano-Agent-Raw on every API request. AGENT in particular is a generic name a user may have set for something else. The privacy statement "never sends ... environment variable values" is therefore not accurate. Qualify it (e.g. "other than the agent name read from AI_AGENT/AGENT/VOLCANO_AGENT").
Found by claude-opus-5-5
| } | ||
| return | ||
| } | ||
| _ = r.Send(payload) |
There was a problem hiding this comment.
🟡 P2: Honor the docs command's offline guarantee
With telemetry enabled, volcano docs list --offline still starts a background POST to /cli/telemetry, even when the docs command fails because its cache is missing. This breaks the explicit “Never touch the network” contract in internal/cmd/docs/docs.go and docs/docs-search.md. Suppress network telemetry for invocations using this offline mode.
Found by gpt-6.1-sol
| printNotice(inv.Stderr) | ||
| state.NoticeShown = true | ||
| } | ||
| if err := r.State.Save(state); err != nil { |
There was a problem hiding this comment.
🟡 P2: Prevent initialization from overwriting a concurrent opt-out
A first-run recorder can load enabled state, overlap with volcano telemetry disable saving disabled:true, then overwrite that preference with its stale state here. Subsequent invocations resume telemetry despite the successful opt-out. Atomic rename prevents partial JSON but does not protect this read-modify-write sequence; serialize initialization and preference updates with a cross-process lock.
Found by gpt-6.1-sol
| case "opencode": | ||
| return "opencode" | ||
| } | ||
| return "" |
There was a problem hiding this comment.
🟡 P2: Map the supported pi harness before checking plugin status
After volcano setup --agent pi installs skills into ~/.pi/agent/skills, an invocation with VOLCANO_AGENT=pi reaches this fallback and checks ~/.volcano/skills instead. It therefore reports plugin_installed:false for a valid Pi installation, or reports an unrelated manual installation. Add the pi mapping supported by internal/setup/harness.go.
Found by gpt-6.1-sol
|
|
||
| Resource names come from the arguments that name a resource, such as | ||
| `<database>` in `volcano databases get <database>`, and from the `--name`, | ||
| `--function`, `--database`, `--project`, `--topic`, and `--bucket` flags. Other |
There was a problem hiding this comment.
🟡 P2: Telemetry doc says flag values and file paths are never sent, but the error message field carries them
The doc says "Other arguments and flag values, such as file paths, search queries, payloads, and commands run in a sandbox, are never sent." It also says the CLI never sends environment variable values except the agent name. But every failed run sends error_message, and that text often contains exactly those things:
- pflag errors quote the flag value (
invalid argument "..." for "--timeout" flag). unknown command "..."quotes the typed argument.- File errors quote the path.
internal/telemetry/fields.goleaves/out of the opaque pattern on purpose "so file paths survive", and the tests expectread ~/app/x.yaml: bad indentto be sent. project_idcan come straight fromVOLCANO_PROJECT_ID.
This page is the user-facing privacy disclosure, so the absolute claims should be narrowed. For example: "except as they appear in an error message, after the redaction above." The project ID from VOLCANO_PROJECT_ID should also be listed.
Found by claude-opus-5-5
There was a problem hiding this comment.
| return | ||
| } | ||
| if state.DeviceID == "" || !state.NoticeShown { | ||
| var firstRun bool |
There was a problem hiding this comment.
⚪ P3: A process that started before the notice was shown can still send an event during parallel first runs
Reporting() uses the state read at process start, so agent headers stay off when NoticeShown was false. Record does not follow that read: it calls initialize() and sends whenever firstRun comes back false.
When an agent runs two volcano commands at once on a fresh machine, process A marks the notice and prints it. Process B started before the notice existed, finds NoticeShown already true, and sends a full event at about the same moment. The user never had a chance to opt out, which breaks the "sends nothing ... before any event is sent" promise in docs/telemetry.md.
Fix: if the start-of-process state() had NoticeShown == false, still call initialize(), but never send from this process.
Found by claude-opus-5-5
There was a problem hiding this comment.
Fixed in ff11e45, same change as the P2 above: a process that started before the notice was shown sends nothing.
| } | ||
| found[key] = value | ||
| } | ||
| args := cmd.Flags().Args() |
There was a problem hiding this comment.
⚪ P3: sandboxes exec <cmd> without -- records the sandbox command as session-id
resources() trims positional args at ArgsLenAtDash() only when a -- is present. placeholders() for exec [session-id] -- <command> [args...] gives session-id.
So a common mistake like volcano sandboxes exec "echo hunter2" (no --) sends resources: {"session-id": "echo hunter2"}. The value passes resourceValue, which allows spaces, and redact. The command fails with a usage error, but the event still carries the text. That contradicts the doc's promise that commands run in a sandbox are never sent.
Fix: for usage lines that contain --, skip positional extraction unless ArgsLenAtDash() >= 0, or require ID-shaped placeholders (*-id) to parse as UUIDs.
Found by claude-opus-5-5
There was a problem hiding this comment.
Fixed in ff11e45. When the usage line has -- and the user didn't pass one, no positional arguments are read as resources.
| func ClearLogin() error { | ||
| return withLock(func() error { | ||
| configPath, err := Path() | ||
| cfg, err := load() |
There was a problem hiding this comment.
⚪ P3: volcano logout now fails on an unparseable config.json and leaves the token on disk
Before, logout called config.Delete(), which removed the file without reading it. So volcano logout was also the way to recover from a corrupt or hand-mangled config.
ClearLogin() now calls load() first. A JSON parse error (from a manual edit, or a crash during the non-atomic os.WriteFile in save()) makes logout fail with failed to clear credentials: failed to parse config file, and user_token stays in the file. Consider falling back to remove() when the config cannot be parsed.
Found by claude-opus-5-5
There was a problem hiding this comment.
Fixed in e3ab28d. ClearLogin removes the file when it can't be parsed, so logout still removes the token from a damaged config. Covered by TestLogoutRemovesUnparseableConfig.
| h.recorder().Record(Invocation{}) | ||
| assert.NotContains(t, string(h.sent[0]), "error_") | ||
| assert.NotContains(t, string(h.sent[0]), "resources") | ||
| assert.NotContains(t, string(h.sent[0]), "project_id") |
There was a problem hiding this comment.
⚪ P3: TestRecord_SuccessSendsNoErrorFields fails when VOLCANO_PROJECT_ID is set in the environment
build() fills ProjectID from r.cfg.ProjectID(), which reads the real VOLCANO_PROJECT_ID (the in-memory config has IgnoreEnv false). This test asserts the payload has no project_id, so it fails for any developer or CI job that has the variable exported, which is likely for people who use the CLI. TestRecord_ProjectAndResources already calls t.Setenv("VOLCANO_PROJECT_ID", ""); this test should do the same.
Found by claude-opus-5-5
There was a problem hiding this comment.
Fixed in ff11e45. The in-memory test config sets IgnoreEnv, so no harness test reads the real VOLCANO_PROJECT_ID.
| event.ErrorStatus = api.Status(inv.Err) | ||
| event.ErrorCode = api.Code(inv.Err) | ||
| home, _ := r.HomeDir() | ||
| event.ErrorMessage = errorMessage(inv.Err, home) |
There was a problem hiding this comment.
🟠 P1: Manifest parse errors can upload plaintext secrets
After the notice has been shown, running volcano config deploy with a manifest containing an SMTP password: 'hunter2${' makes internal/projectconfig/interpolate.go return an error quoting that entire value. None of the redaction patterns removes it, so telemetry uploads the plaintext password despite the documented exclusion of file contents and secrets. Suppress secret-bearing parser errors or send allowlisted error summaries instead of arbitrary error text.
Found by gpt-6.1-sol
There was a problem hiding this comment.
Fixed in d49c59f. Added internal/sensitive to mark errors whose text can quote file contents. Manifest YAML, interpolation, and decode errors and env file parse errors are marked, and telemetry replaces the marked part with [redacted]. The terminal still shows the full error. feecd6e applies the same marker to a function's passthrough error body in functions invoke.
| if err != nil || state.Disabled { | ||
| return | ||
| } | ||
| if firstRun { |
There was a problem hiding this comment.
🟡 P2: Concurrent first runs can report before the notice
When two processes start without telemetry state, both initially return false from Reporting(). Once the first persists NoticeShown, the second process's initialize() returns firstRun=false, allowing it to upload its event, potentially before the first process prints the notice. This contradicts the startup snapshot and “nothing until your next command” guarantee. Keep invocations that started before the notice from sending, even when initialization finds another process has saved the state.
Found by gpt-6.1-sol
There was a problem hiding this comment.
Fixed in ff11e45. A process whose start-of-process state had no notice shown never sends, even when a concurrent process marked the notice first. Covered by TestRecord_ConcurrentFirstRunSendsNothing.
…rs quote from the file
…otice, and skip positional resources when the expected -- is missing
…_PROJECT_ID is sent
| awsKeyPattern = regexp.MustCompile(`\b(?:AKIA|ASIA)[0-9A-Z]{16}\b`) | ||
| userPattern = regexp.MustCompile(`([a-zA-Z][a-zA-Z0-9+.-]*://)[^\s/@]+@`) | ||
| queryPattern = regexp.MustCompile(`(https?://[^\s?#]+)\?[^\s#]*`) | ||
| pairPattern = regexp.MustCompile(`(?i)\b(password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key)=\S+`) |
There was a problem hiding this comment.
🟡 P2: Secret-pair redaction misses prefixed names like DB_PASSWORD= and client_secret=
pairPattern puts \b before the key, and _ counts as a word character, so DB_PASSWORD=hunter2, SMTP_PASSWORD=…, MY_API_KEY=… and client_secret=… never match. Only the bare password= / API_KEY= forms in the test match. Short values also slip past opaquePattern (40+ chars). Concrete trigger: an agent or user runs volcano variables deploy DB_PASSWORD=hunter2. deploy is cobra.NoArgs, so the error is unknown command "DB_PASSWORD=hunter2" for "volcano variables deploy", and errorMessage sends it unredacted. That contradicts docs/telemetry.md, which promises to remove values of password=, token=, and similar pairs. The comment says the server applies the same rules, so it would miss this too. Fix: allow an identifier prefix, e.g. (?i)\b[A-Za-z0-9_]*?(password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key)=\S+ (keep the prefix in the replacement), add tests for DB_PASSWORD= and client_secret=, and mirror the change in volcano-hosting.
Found by claude-opus-5-5
| Volcano team which agents people build with. It never changes what a command | ||
| does, and it is never used to allow or block a request. | ||
|
|
||
| The CLI reads environment variables only. It does not read files, inspect other |
There was a problem hiding this comment.
⚪ P3: Published doc says the CLI does not read files or send command arguments, but this PR's telemetry does both
agent-detection.md says "The CLI reads environment variables only. It does not read files, inspect other processes, or send command arguments." In this same PR, the usage event reads harness plugin files to report plugin_installed/plugin_version, and it sends resource names taken from positional arguments and flag values (docs/telemetry.md says so). This is a published, privacy-relevant page, so the blanket statement will mislead readers. Scope it to the request headers, e.g. "Detection reads environment variables only, and the headers below carry no command arguments", and point to telemetry.md for what the usage event collects.
Found by claude-opus-5-5
| return nil, fmt.Errorf("invalid manifest: %w", sensitive.Wrap(err)) | ||
| } | ||
|
|
||
| if err := manifest.Validate(); err != nil { |
There was a problem hiding this comment.
🟡 P2: Mark manifest validation failures as sensitive
Parse expands environment references before validation, and validateFunctionKind and validateFunctionVisibility quote rejected values. A misconfigured visibility: ${SMTP_PASSWORD} therefore sends the expanded password in telemetry when it does not match a redaction pattern. Wrap validation errors with sensitive.Wrap, as the surrounding parsing and decoding errors already do.
Found by gpt-6.1-sol
| Resources: resources(inv.Command), | ||
| } | ||
| if r.cfg != nil { | ||
| event.ProjectID = r.cfg.ProjectID() |
There was a problem hiding this comment.
🟡 P2: Use the command's effective project for telemetry
After selecting a cloud project, running volcano functions get hello targets the local project through internal/cmd/local/local.go's withLocalConfig. The recorder instead reads the saved cloud configuration, attaching that cloud project's ID to local resource names and failures. Pass the effective session project into the invocation, or omit the project ID for local commands.
Found by gpt-6.1-sol
…etry never sends it
…when it is a version
| secrets in command arguments. | ||
|
|
||
| Resource names come from the arguments that name a resource, such as | ||
| `<database>` in `volcano databases get <database>`, and from the `--name`, |
There was a problem hiding this comment.
⚪ P3: Resource example uses a placeholder that volcano databases get does not have
The doc says resource names come from arguments "such as <database> in volcano databases get <database>". The real usage line is get <name> (internal/cmd/databases/get.go), so the event key for that command is name, not database. <database> is the first argument of commands such as databases branch get <database> <branch> and databases backup list <database>. Use one of those, or volcano databases get <name>, so the published example matches what the event actually contains.
Found by claude-opus-5-5
| if inv.Err != nil { | ||
| event.ErrorKind = errorKind(inv.Err) | ||
| event.ErrorStatus = api.Status(inv.Err) | ||
| event.ErrorCode = api.Code(inv.Err) |
There was a problem hiding this comment.
🟡 P2: Suppress error codes carried by sensitive errors
A function returning HTTP 400 with {"error":"failed","code":"private_customer_4411"} produces a sensitive error, but api.Code unwraps it and copies the application's code into telemetry unchanged. Function response content consequently leaves the machine despite the documented promise to retain only its HTTP status. Omit error codes originating inside sensitive errors.
Found by gpt-6.1-sol
| break | ||
| } | ||
| if resourcePlaceholders[placeholder] { | ||
| add(placeholder, args[i]) |
There was a problem hiding this comment.
🟡 P2: Exclude function paths from collected resource names
volcano functions invoke /Users/alice/private-project/hello.js is supported: internal/function/function.go resolves it using the basename. Telemetry instead collects the complete argument under resources.name, because the placeholder is name and the value pattern accepts slashes. This uploads local paths even on successful invocations, contrary to the documented collection policy. Normalize path-valued function identifiers or omit them.
Found by gpt-6.1-sol
marckong
left a comment
marckong
left a comment
There was a problem hiding this comment.
Review of the CLI side (head 04c6e6b), read alongside Kong/volcano-hosting#1919 and Kong/volcano-skills#62. No blocking findings. I read the core paths (recorder, redaction, agent detection, sender, opt-out commands, logout change) but did not run tests.
[P2] Quoted secret pairs leak part of the value — internal/telemetry/fields.go:159-161
pairPattern ends at \S+, so password='a b' becomes password=[redacted] b' and the tail still leaves the machine. password: x and JSON "token": "x" don't match. The opaque-run rule excludes / and ., so a long base64 secret containing / can split into runs under 40 chars. The API copy is identical, so it won't catch these. Suggest table tests for these shapes in error messages.
[P2] Redaction/allowlists are hand-synced across repos — internal/telemetry/fields.go:155-162
Eight regexes are duplicated with volcano-hosting and the agent list lives in three repos; only prose enforces they match. A shared golden test file would make drift a CI failure.
[P2] First-run notice omits account linkage — internal/telemetry/telemetry.go:452-455
The notice lists what is collected but not that events are attributed to the logged-in account (only volcano telemetry --help and the docs say so), and the device ID survives logout. Since the notice is the only consent point, consider adding account linkage and that error messages can contain typed input.
[P3] Recorder reads the cloud config, not the command's config — internal/telemetry/telemetry.go:143, :346
LoadConfig defaults to config.Load and ProjectID comes from it, while main.go honors deps.ConfigLoader elsewhere. If local-mode commands reach Record, they would report the cloud config's project and post to the cloud API URL. I didn't confirm they do.
Question: does versionProtocolDoer only wrap calls to the Volcano API? X-Volcano-Agent* headers shouldn't reach a customer's function endpoint via functions invoke.
Strengths: conservative consent flow (nothing sent on first run, no send from a process started before the notice), sensitive parser errors blanked, opt-out survives logout with a test.
Tracking
POST /cli/telemetryand agent properties on PostHog events)VOLCANO_AGENT)Why
Summary
internal/agent: detect the coding agent from env vars in this order: namedAI_AGENT/AGENT, harness markers (CLAUDECODE,CODEX_THREAD_ID,GEMINI_CLI,CURSOR_AGENT,OPENCODE_CLIENT, ...),VOLCANO_AGENT, bare truthyAI_AGENT/AGENT(unknown), then host-terminal markers (CURSOR_TRACE_ID,REPL_ID); exact name matches only, canonical names, values sanitized to a header-safe charset capped at 64 charsX-Volcano-Agent,-Source,-Raw,-Markerson every API request fromversionProtocolDoer, and nothing when no agent is detected, telemetry is off, or the first-run notice has not been shown yetinternal/telemetry: onecli_command_invokedevent per run with command path, names of flags set, selected project ID, resource names and IDs, CLI version, OS, arch, CPU count, CI vendor, stdin TTY, agent name/source/markers, Volcano plugin installed + version for that agent (version sent only when it is semver-shaped), exit code, duration; every failed run adds error kind, HTTP status, API error code, and the error message<database>,<name-or-id>,<bucket>, ...) and from thename,function,database,project,topic,bucketflags, keyed by placeholder or flag name; paths, queries, payloads, and sandbox commands are never sent, and values that don't look like a name or look like a secret are dropped~, strip ANSI escapes and control characters, collapse whitespace, then redact bearer tokens, JWTs,pk-/pt-/sk-/ak-keys, AWS key IDs, URL user info, URL query strings,password=-style pairs (including prefixed keys such asDB_PASSWORD=), and 40+ char opaque runs, and cut to 500 runes; the API applies the same steps again, and itsdocs/internal/guides/cli-telemetry.mdis the wire contractinternal/sensitiveto mark errors whose text can carry file contents or application data; telemetry replaces that part with[redacted]while the terminal still shows it; used forvolcano-config.yamlYAML/interpolation/decode errors, env file parse errors (godotenv quotes the file), and a function's passthrough error body involcano functions invokevolcano telemetry flushprocess toPOST {apiURL}/cli/telemetrywith the saved login token when present, so commands never wait on the networkvolcano telemetry --help, which ships in the binary, since docs caches synced before this release lackcli/telemetry.mdsetup.PluginStatusto answer "is the Volcano plugin installed for this harness" from local files, without the harness presence probevolcano telemetry disable|enable|statusandVOLCANO_TELEMETRY_DISABLED=1;VOLCANO_TELEMETRY_DEBUG=1prints the event instead of sending (off unless set);volcano telemetrycommands are never recorded, so an opt-out isn't itself reportedtelemetryblock of~/.volcano/config.json, updated throughconfig.Updateso a first run can't overwrite a concurrentvolcano telemetry disablevolcano logoutto clear the token, user ID, current project, and function aliases instead of deletingconfig.json, so the telemetry setting and docs source survive a logout; the file is still removed when nothing else is in it, or when it can't be parsed (so logout still removes the token from a damaged file)__complete, which runs on every Tab with stderr discarded, andvolcano completion <shell>, which rc files run on every new shell) or--offlineruns, checking raw args too so a flag parse error can't bypass itrun()after the exit code is known, usingExecuteCfor the executed command; telemetry never changes the exit code or output beyond the one-time noticeCOPILOT_MODEL/COPILOT_ALLOW_ALLas weak signals, since people export them in their own profile; mappito itsvolcano setupharnessdocs/telemetry.mdanddocs/agent-detection.md, linked fromdocs/README.mdanddocs/setup.mdinternal/agentaliases and markers here,allowedAgentsin volcano-hostinginternal/productanalytics/allowlist.go, and the harness list in volcano-skillsAGENTS.md/cli/telemetry, which needs Kong/volcano-hosting#1919 deployed first; before that the flush gets a 404 in the background and nothing breaksVerification
make lintgo test ./...go test -race ./internal/agent/ ./internal/api/HOMEagainst a local sink: first run printed the notice once and sent nothing, each later run posted one event to/cli/telemetrywith the expected command, flags, agent, exit code, and error kind; a failingcloud functions get hellocarried the project ID,{"name-or-id": "hello"}, and the error message;volcano telemetry status/disablesent nothing;DO_NOT_TRACK=1no longer opts out; aftervolcano telemetry disablenothing was sent;volcano logoutkept thetelemetryblock inconfig.jsonHOME(no notice on repeated runs) and__complete(no state written)volcano projects listtwice with a freshHOMEagainst a local server: the first run sent noX-Volcano-Agentheader and no event; the second sent bothCODEX_THREAD_ID+CURSOR_TRACE_ID,VOLCANO_AGENTonly, a CRLF injection attempt inAI_AGENT, and a clean env (no headers)