Skip to content

fix(telemetry): the token lookup and the profile store use two different keys, so outcomes silently spool #552

Description

@saadqbal

Bugbot flagged this as High on the develop -> staging promotion #540,
and it is the finding that held the 2026-08-21 staging hop. Filing it so the
finding is recorded before that thread is resolved — an unresolved thread
blocks the merge regardless of severity, and the ticket comes first.

Fix on develop, not on #540: every push to a promotion re-rolls its review.

The finding

Telemetry token lookup misses profiles
internal/cli/telemetry_transport.go#L414-L424,
with internal/cli/telemetry.go#L192-L194

telemetryToken looks up the bearer with cfg.Profile(env) using the
telemetry label from telemetryEnv(signedInEnv()), but profiles are keyed by
the raw CurrentEnv string. When that label is remapped (unknown env →
prod) or only matches after sessionEnv normalisation (case/whitespace),
the lookup creates an empty profile and returns no token. Delivery then always
takes the no-token spool path, so outcomes never POST even though
authedClient still uses the real session token via cfg.Current().

Bugbot bug id: 0e5c3db8-98d2-47f9-96b7-522132ec1808

Why it is worth a High

Two keys for one concept. cfg.Profile is keyed on the rawCurrentEnv;
the telemetry label has been through telemetryEnv, which deliberately
remaps anything unrecognised onto prod (#531) and normalises case and
whitespace. Whenever those two disagree the lookup does not miss loudly — it
creates an empty profile and returns no token.

The failure is silent by construction, and that is the part that matters:

  • delivery falls through to the no-token spool path every time, so outcomes
    are written locally and never POSTed
  • authedClient keeps working, because it reads cfg.Current() and gets the
    real session token — so the CLI looks healthy and signed in
  • so there is no error, no retry, no warning: telemetry just silently stops
    arriving, and the only symptom is an absence somewhere else

An empty profile created as a side effect of a read is also worth a look on
its own — a lookup that mutates config to record its own miss makes the second
call look like a hit.

Acceptance criteria

  • telemetryToken resolves the bearer through the same key cfg.Profile
    is keyed on, so a remapped or normalised label cannot miss
  • A remapped env (unknown → prod) and a label needing sessionEnv
    normalisation (case, surrounding whitespace) both still find the token
  • Looking up a token does not create a profile as a side effect
  • A test pins that a signed-in CLI whose env label was remapped POSTs
    rather than spools — the current tests do not distinguish the two, which
    is why this reached a promotion
  • The no-token spool path is still taken when there genuinely is no token

Context

Adjacent, already closed — worth reading before starting, since the remap this
depends on came from the first one:

Not covered here

#540's other three findings are already resolved on that PR and are not part
of this ticket: Same-ceiling set now requires --yes (Medium,
internal/cli/resources_set.go), Shellcheck not on runner image (High,
.github/workflows/build.yml), and Telemetry env resolved twice (Low,
internal/cli/telemetry.go). Verified resolved on 2026-08-21; only the
telemetry-token thread is open.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions