From 46ecea84f473578f87fcc286043ed71b4f777322 Mon Sep 17 00:00:00 2001
From: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Date: Fri, 18 Sep 2026 18:04:49 +0100
Subject: [PATCH 1/6] telemetry: add best-effort usage telemetry recorder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add the internal/telemetry package: a Recorder that records src-cli usage
events to the authenticated Sourcegraph instance via the Telemetry V2
recordEvents GraphQL mutation. Recording is strictly best-effort — network,
GraphQL, timeout, and old-instance (<5.2) failures are all swallowed and never
affect the command the user ran. Command identity is encoded in feature/action
with numeric-only metadata (no user content, no privateMetadata), and names are
validated against Sourcegraph's server-side naming rules.
---
 internal/telemetry/telemetry.go      | 186 +++++++++++++++++++++++++++
 internal/telemetry/telemetry_test.go | 158 +++++++++++++++++++++++
 internal/telemetry/validate.go       |  39 ++++++
 internal/telemetry/validate_test.go  |  44 +++++++
 4 files changed, 427 insertions(+)
 create mode 100644 internal/telemetry/telemetry.go
 create mode 100644 internal/telemetry/telemetry_test.go
 create mode 100644 internal/telemetry/validate.go
 create mode 100644 internal/telemetry/validate_test.go

diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go
new file mode 100644
index 0000000000..74d372b2a3
--- /dev/null
+++ b/internal/telemetry/telemetry.go
@@ -0,0 +1,186 @@
+// Package telemetry records src-cli usage events to the Sourcegraph instance
+// the CLI is authenticated to, using the Telemetry V2 `recordEvents` GraphQL
+// mutation.
+//
+// Recording is strictly best-effort: a failure to record — whether a network
+// error, a GraphQL error, a timeout, or an instance too old to support the
+// mutation — must never affect the command the user actually ran. See
+// .context/TELEMETRY.md for the design and event schema.
+package telemetry
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"sort"
+	"time"
+
+	"github.com/sourcegraph/src-cli/internal/api"
+
+	"github.com/sourcegraph/sourcegraph/lib/errors"
+)
+
+const (
+	// ClientName identifies src-cli as the source of telemetry events.
+	ClientName = "SRC_CLI"
+
+	// eventParametersVersion is the schema version of the metadata we attach to
+	// each event. Bump it when the shape of the metadata changes.
+	eventParametersVersion = 1
+
+	// defaultTimeout bounds how long a single Record call may spend recording.
+	// It is deliberately short: telemetry is sent synchronously right before the
+	// process exits, so it must not add meaningful latency.
+	defaultTimeout = 2 * time.Second
+)
+
+// recordEventsMutation mirrors the mutation used by Sourcegraph's own clients.
+// The `telemetry` mutation only exists on Sourcegraph 5.2+, so on older
+// instances this returns GraphQL errors, which Record silently drops.
+const recordEventsMutation = `mutation RecordTelemetryEvents($events: [TelemetryEventInput!]!) {
+    telemetry {
+        recordEvents(events: $events) {
+            alwaysNil
+        }
+    }
+}`
+
+// Source identifies the client emitting events. It is constant for the lifetime
+// of a process.
+type Source struct {
+	// Client is the source client name, e.g. ClientName.
+	Client string
+	// ClientVersion is the src-cli version, e.g. "6.1.0" or "dev".
+	ClientVersion string
+}
+
+// Event is a single telemetry event.
+//
+// Feature and Action carry the event's identity and are always exported by
+// Sourcegraph, so command identity lives here (e.g. Feature "srcCli.search",
+// Action "succeeded"). Metadata values are numeric-only and are also always
+// exported; they must never contain user content. See .context/TELEMETRY.md.
+type Event struct {
+	// Feature is a noun describing what the event is about, e.g. "srcCli.search".
+	Feature string
+	// Action is a verb describing what happened, e.g. "succeeded" or "failed".
+	Action string
+	// Metadata holds numeric-only, PII-free facts about the event.
+	Metadata map[string]float64
+}
+
+// Recorder records events for a single Source through an api.Client.
+type Recorder struct {
+	client  api.Client
+	source  Source
+	timeout time.Duration
+	debug   io.Writer
+}
+
+// Option customizes a Recorder.
+type Option func(*Recorder)
+
+// WithTimeout overrides the default per-Record timeout.
+func WithTimeout(d time.Duration) Option {
+	return func(r *Recorder) {
+		if d > 0 {
+			r.timeout = d
+		}
+	}
+}
+
+// WithDebug sets a writer that receives a diagnostic line whenever an event is
+// dropped. Intended to be wired to verbose (-v) output; leave unset for silence.
+func WithDebug(w io.Writer) Option {
+	return func(r *Recorder) { r.debug = w }
+}
+
+// NewRecorder returns a Recorder that records events for source through client.
+func NewRecorder(client api.Client, source Source, opts ...Option) *Recorder {
+	r := &Recorder{
+		client:  client,
+		source:  source,
+		timeout: defaultTimeout,
+	}
+	for _, opt := range opts {
+		opt(r)
+	}
+	return r
+}
+
+// Record sends event on a best-effort basis. It never returns an error and
+// never panics: validation, network, GraphQL, timeout, and old-instance
+// failures are all silently dropped (written to the debug writer if one was set
+// via WithDebug). It applies its own timeout, so the caller's context need not
+// carry a deadline.
+func (r *Recorder) Record(ctx context.Context, event Event) {
+	if err := r.record(ctx, event); err != nil && r.debug != nil {
+		fmt.Fprintf(r.debug, "telemetry: dropping event %q/%q: %v\n", event.Feature, event.Action, err)
+	}
+}
+
+// record does the work behind Record and returns any error, so it can be tested
+// directly. Callers outside tests should use Record.
+func (r *Recorder) record(ctx context.Context, event Event) error {
+	if r.client == nil {
+		return errors.New("nil api client")
+	}
+	if err := Validate(event.Feature, event.Action); err != nil {
+		return err
+	}
+
+	ctx, cancel := context.WithTimeout(ctx, r.timeout)
+	defer cancel()
+
+	vars := map[string]any{
+		"events": []any{buildEventInput(r.source, event)},
+	}
+
+	// The recordEvents payload has no fields we care about; we only need to
+	// know whether the request succeeded.
+	var result struct {
+		Telemetry struct {
+			RecordEvents struct {
+				AlwaysNil *string
+			}
+		}
+	}
+	if _, err := r.client.NewRequest(recordEventsMutation, vars).Do(ctx, &result); err != nil {
+		return err
+	}
+	return nil
+}
+
+// buildEventInput builds a single TelemetryEventInput as a JSON-serializable map.
+func buildEventInput(source Source, event Event) map[string]any {
+	return map[string]any{
+		"feature": event.Feature,
+		"action":  event.Action,
+		"source": map[string]any{
+			"client":        source.Client,
+			"clientVersion": source.ClientVersion,
+		},
+		"parameters": map[string]any{
+			"version":  eventParametersVersion,
+			"metadata": buildMetadata(event.Metadata),
+		},
+	}
+}
+
+// buildMetadata converts numeric metadata into the list of {key, value} inputs
+// the API expects, sorted by key for deterministic output.
+func buildMetadata(metadata map[string]float64) []any {
+	out := make([]any, 0, len(metadata))
+	if len(metadata) == 0 {
+		return out
+	}
+	keys := make([]string, 0, len(metadata))
+	for k := range metadata {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	for _, k := range keys {
+		out = append(out, map[string]any{"key": k, "value": metadata[k]})
+	}
+	return out
+}
diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go
new file mode 100644
index 0000000000..c17e603594
--- /dev/null
+++ b/internal/telemetry/telemetry_test.go
@@ -0,0 +1,158 @@
+package telemetry
+
+import (
+	"bytes"
+	"context"
+	"testing"
+	"time"
+
+	"github.com/sourcegraph/src-cli/internal/api"
+	apimock "github.com/sourcegraph/src-cli/internal/api/mock"
+
+	"github.com/sourcegraph/sourcegraph/lib/errors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+)
+
+func testSource() Source {
+	return Source{Client: ClientName, ClientVersion: "6.1.0"}
+}
+
+func TestRecord_SendsWellFormedMutation(t *testing.T) {
+	client := &apimock.Client{}
+	req := &apimock.Request{}
+
+	var gotQuery string
+	var gotVars map[string]any
+	client.On("NewRequest", mock.Anything, mock.Anything).
+		Run(func(args mock.Arguments) {
+			gotQuery = args.Get(0).(string)
+			gotVars = args.Get(1).(map[string]any)
+		}).
+		Return(req)
+	req.On("Do", mock.Anything, mock.Anything).Return(true, nil)
+
+	rec := NewRecorder(client, testSource())
+	rec.Record(context.Background(), Event{
+		Feature:  "srcCli.search",
+		Action:   "succeeded",
+		Metadata: map[string]float64{"durationMs": 12, "exitCode": 0},
+	})
+
+	assert.Equal(t, recordEventsMutation, gotQuery)
+
+	events, ok := gotVars["events"].([]any)
+	if !ok || len(events) != 1 {
+		t.Fatalf("expected 1 event, got %#v", gotVars["events"])
+	}
+	event := events[0].(map[string]any)
+	assert.Equal(t, "srcCli.search", event["feature"])
+	assert.Equal(t, "succeeded", event["action"])
+
+	source := event["source"].(map[string]any)
+	assert.Equal(t, ClientName, source["client"])
+	assert.Equal(t, "6.1.0", source["clientVersion"])
+
+	params := event["parameters"].(map[string]any)
+	assert.Equal(t, eventParametersVersion, params["version"])
+
+	metadata := params["metadata"].([]any)
+	// sorted by key: durationMs, exitCode
+	assert.Equal(t, []any{
+		map[string]any{"key": "durationMs", "value": float64(12)},
+		map[string]any{"key": "exitCode", "value": float64(0)},
+	}, metadata)
+
+	client.AssertExpectations(t)
+	req.AssertExpectations(t)
+}
+
+func TestRecord_EmptyMetadataSendsEmptyList(t *testing.T) {
+	client := &apimock.Client{}
+	req := &apimock.Request{}
+
+	var gotVars map[string]any
+	client.On("NewRequest", mock.Anything, mock.Anything).
+		Run(func(args mock.Arguments) { gotVars = args.Get(1).(map[string]any) }).
+		Return(req)
+	req.On("Do", mock.Anything, mock.Anything).Return(true, nil)
+
+	rec := NewRecorder(client, testSource())
+	rec.Record(context.Background(), Event{Feature: "srcCli.version", Action: "succeeded"})
+
+	event := gotVars["events"].([]any)[0].(map[string]any)
+	params := event["parameters"].(map[string]any)
+	assert.Equal(t, []any{}, params["metadata"])
+}
+
+func TestRecord_ValidationFailsBeforeSending(t *testing.T) {
+	client := &apimock.Client{}
+	// No expectations set: NewRequest must never be called.
+
+	rec := NewRecorder(client, testSource())
+	err := rec.record(context.Background(), Event{Feature: "Bad_Feature", Action: "succeeded"})
+
+	assert.Error(t, err)
+	client.AssertNotCalled(t, "NewRequest", mock.Anything, mock.Anything)
+}
+
+func TestRecord_NetworkErrorSwallowed(t *testing.T) {
+	client := &apimock.Client{}
+	req := &apimock.Request{}
+	client.On("NewRequest", mock.Anything, mock.Anything).Return(req)
+	req.On("Do", mock.Anything, mock.Anything).Return(false, errors.New("connection refused"))
+
+	var debug bytes.Buffer
+	rec := NewRecorder(client, testSource(), WithDebug(&debug))
+
+	// Must not panic and must not surface the error.
+	assert.NotPanics(t, func() {
+		rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "failed"})
+	})
+	assert.Contains(t, debug.String(), "connection refused")
+
+	// record itself reports the error for callers that want it.
+	err := rec.record(context.Background(), Event{Feature: "srcCli.search", Action: "failed"})
+	assert.Error(t, err)
+}
+
+func TestRecord_GraphQLErrorSwallowed(t *testing.T) {
+	// Simulates an instance too old to have the telemetry mutation: the server
+	// returns GraphQL errors, which must be dropped silently.
+	client := &apimock.Client{}
+	req := &apimock.Request{}
+	client.On("NewRequest", mock.Anything, mock.Anything).Return(req)
+	req.On("Do", mock.Anything, mock.Anything).
+		Return(false, api.GraphQlErrors{})
+
+	rec := NewRecorder(client, testSource())
+	assert.NotPanics(t, func() {
+		rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+	})
+}
+
+func TestRecord_NilClientDoesNotPanic(t *testing.T) {
+	rec := NewRecorder(nil, testSource())
+	assert.NotPanics(t, func() {
+		rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+	})
+}
+
+func TestRecord_AppliesTimeout(t *testing.T) {
+	client := &apimock.Client{}
+	req := &apimock.Request{}
+
+	var hadDeadline bool
+	client.On("NewRequest", mock.Anything, mock.Anything).Return(req)
+	req.On("Do", mock.Anything, mock.Anything).
+		Run(func(args mock.Arguments) {
+			ctx := args.Get(0).(context.Context)
+			_, hadDeadline = ctx.Deadline()
+		}).
+		Return(true, nil)
+
+	rec := NewRecorder(client, testSource(), WithTimeout(50*time.Millisecond))
+	rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+
+	assert.True(t, hadDeadline, "expected Record to apply a context deadline")
+}
diff --git a/internal/telemetry/validate.go b/internal/telemetry/validate.go
new file mode 100644
index 0000000000..b71f3acf9e
--- /dev/null
+++ b/internal/telemetry/validate.go
@@ -0,0 +1,39 @@
+package telemetry
+
+import (
+	"github.com/sourcegraph/src-cli/internal/lazyregexp"
+
+	"github.com/sourcegraph/sourcegraph/lib/errors"
+)
+
+// maxNameLength is the maximum length Sourcegraph accepts for a feature or
+// action name.
+const maxNameLength = 64
+
+// featureActionRegex matches the names Sourcegraph accepts for feature and
+// action: they must start with a lowercase letter and contain only letters,
+// dashes, and dots (no digits, underscores, or whitespace). It mirrors the
+// server-side validation in the Sourcegraph monorepo.
+var featureActionRegex = lazyregexp.New(`^[a-z][a-zA-Z\-.]+$`)
+
+// Validate reports whether feature and action satisfy Sourcegraph's naming
+// rules. Events that fail validation are rejected before any request is made.
+func Validate(feature, action string) error {
+	if err := validateName("feature", feature); err != nil {
+		return err
+	}
+	return validateName("action", action)
+}
+
+func validateName(kind, name string) error {
+	if name == "" {
+		return errors.Newf("telemetry %s must not be empty", kind)
+	}
+	if len(name) > maxNameLength {
+		return errors.Newf("telemetry %s %q exceeds %d characters", kind, name, maxNameLength)
+	}
+	if !featureActionRegex.MatchString(name) {
+		return errors.Newf("telemetry %s %q must match %s", kind, name, featureActionRegex.Re().String())
+	}
+	return nil
+}
diff --git a/internal/telemetry/validate_test.go b/internal/telemetry/validate_test.go
new file mode 100644
index 0000000000..bf73648dba
--- /dev/null
+++ b/internal/telemetry/validate_test.go
@@ -0,0 +1,44 @@
+package telemetry
+
+import "testing"
+
+func TestValidate(t *testing.T) {
+	tests := []struct {
+		name    string
+		feature string
+		action  string
+		wantErr bool
+	}{
+		{name: "valid simple", feature: "srcCli", action: "succeeded"},
+		{name: "valid dotted feature", feature: "srcCli.batch.apply", action: "failed"},
+		{name: "valid dashed feature", feature: "srcCli.code-intel", action: "succeeded"},
+		{name: "empty feature", feature: "", action: "succeeded", wantErr: true},
+		{name: "empty action", feature: "srcCli", action: "", wantErr: true},
+		{name: "digit in feature", feature: "srcCli2", action: "succeeded", wantErr: true},
+		{name: "underscore in action", feature: "srcCli", action: "did_it", wantErr: true},
+		{name: "leading uppercase", feature: "SrcCli", action: "succeeded", wantErr: true},
+		{name: "whitespace", feature: "srcCli search", action: "succeeded", wantErr: true},
+		{name: "single char feature (too short for +)", feature: "s", action: "succeeded", wantErr: true},
+		{name: "too long", feature: longName(70), action: "ok", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := Validate(tt.feature, tt.action)
+			if tt.wantErr && err == nil {
+				t.Fatalf("Validate(%q, %q) = nil, want error", tt.feature, tt.action)
+			}
+			if !tt.wantErr && err != nil {
+				t.Fatalf("Validate(%q, %q) = %v, want nil", tt.feature, tt.action, err)
+			}
+		})
+	}
+}
+
+func longName(n int) string {
+	b := make([]byte, n)
+	for i := range b {
+		b[i] = 'a'
+	}
+	return string(b)
+}

From 745a8a0fa9a9f70341ad87634983d2baac663135 Mon Sep 17 00:00:00 2001
From: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Date: Sat, 19 Sep 2026 16:29:06 +0100
Subject: [PATCH 2/6] fix/telemetry: keep recording failures silent

Telemetry must never contaminate command output, but GraphQL request execution can print OAuth reauthorization guidance on HTTP 401 responses. Send telemetry through the authenticated HTTP client directly so failures remain best-effort without interactive output, while preserving authentication and cancellation.

Add regression coverage for OAuth failures, request shape, GraphQL errors, and timeout cancellation.

## Test Plan

- go test ./...
- go test ./internal/telemetry -run 'TestRecord_(AppliesTimeout|TimeoutCancelsHTTPRequest)' -count=20
---
 internal/telemetry/telemetry.go      |  38 ++++--
 internal/telemetry/telemetry_test.go | 190 ++++++++++++++++++++++-----
 2 files changed, 187 insertions(+), 41 deletions(-)

diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go
index 74d372b2a3..b364412dfc 100644
--- a/internal/telemetry/telemetry.go
+++ b/internal/telemetry/telemetry.go
@@ -9,9 +9,12 @@
 package telemetry
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
+	"net/http"
 	"sort"
 	"time"
 
@@ -136,18 +139,37 @@ func (r *Recorder) record(ctx context.Context, event Event) error {
 		"events": []any{buildEventInput(r.source, event)},
 	}
 
-	// The recordEvents payload has no fields we care about; we only need to
-	// know whether the request succeeded.
+	payload, err := json.Marshal(map[string]any{
+		"query":     recordEventsMutation,
+		"variables": vars,
+	})
+	if err != nil {
+		return err
+	}
+	// Use the HTTP-level client because GraphQL Request.Do may print interactive
+	// authentication guidance; telemetry must never affect command output.
+	req, err := r.client.NewHTTPRequest(ctx, http.MethodPost, ".api/graphql", bytes.NewReader(payload))
+	if err != nil {
+		return err
+	}
+	resp, err := r.client.Do(req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		return errors.Newf("telemetry request failed: %s", resp.Status)
+	}
 	var result struct {
-		Telemetry struct {
-			RecordEvents struct {
-				AlwaysNil *string
-			}
-		}
+		Errors []json.RawMessage `json:"errors"`
 	}
-	if _, err := r.client.NewRequest(recordEventsMutation, vars).Do(ctx, &result); err != nil {
+	if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
 		return err
 	}
+	if len(result.Errors) > 0 {
+		return api.NewGraphQlErrors(result.Errors)
+	}
 	return nil
 }
 
diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go
index c17e603594..a8dd6dc7d7 100644
--- a/internal/telemetry/telemetry_test.go
+++ b/internal/telemetry/telemetry_test.go
@@ -3,11 +3,20 @@ package telemetry
 import (
 	"bytes"
 	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/sourcegraph/src-cli/internal/api"
 	apimock "github.com/sourcegraph/src-cli/internal/api/mock"
+	"github.com/sourcegraph/src-cli/internal/oauth"
 
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/stretchr/testify/assert"
@@ -18,19 +27,50 @@ func testSource() Source {
 	return Source{Client: ClientName, ClientVersion: "6.1.0"}
 }
 
+func response(statusCode int, body string) *http.Response {
+	return &http.Response{
+		StatusCode: statusCode,
+		Status:     http.StatusText(statusCode),
+		Body:       io.NopCloser(strings.NewReader(body)),
+	}
+}
+
+type cancellationClient struct {
+	api.Client
+	canceled chan struct{}
+	release  chan struct{}
+}
+
+func (c *cancellationClient) NewHTTPRequest(ctx context.Context, method, _ string, body io.Reader) (*http.Request, error) {
+	return http.NewRequestWithContext(ctx, method, "http://example.com/.api/graphql", body)
+}
+
+func (c *cancellationClient) Do(req *http.Request) (*http.Response, error) {
+	select {
+	case <-req.Context().Done():
+		close(c.canceled)
+		return nil, req.Context().Err()
+	case <-c.release:
+		return nil, errors.New("test client released")
+	}
+}
+
 func TestRecord_SendsWellFormedMutation(t *testing.T) {
 	client := &apimock.Client{}
-	req := &apimock.Request{}
+	req := httptest.NewRequest(http.MethodPost, "/.api/graphql", nil)
 
-	var gotQuery string
-	var gotVars map[string]any
-	client.On("NewRequest", mock.Anything, mock.Anything).
+	var gotPayload struct {
+		Query     string         `json:"query"`
+		Variables map[string]any `json:"variables"`
+	}
+	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).
 		Run(func(args mock.Arguments) {
-			gotQuery = args.Get(0).(string)
-			gotVars = args.Get(1).(map[string]any)
+			if err := json.NewDecoder(args.Get(3).(io.Reader)).Decode(&gotPayload); err != nil {
+				t.Fatal(err)
+			}
 		}).
-		Return(req)
-	req.On("Do", mock.Anything, mock.Anything).Return(true, nil)
+		Return(req, nil)
+	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
 	rec := NewRecorder(client, testSource())
 	rec.Record(context.Background(), Event{
@@ -39,11 +79,11 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 		Metadata: map[string]float64{"durationMs": 12, "exitCode": 0},
 	})
 
-	assert.Equal(t, recordEventsMutation, gotQuery)
+	assert.Equal(t, recordEventsMutation, gotPayload.Query)
 
-	events, ok := gotVars["events"].([]any)
+	events, ok := gotPayload.Variables["events"].([]any)
 	if !ok || len(events) != 1 {
-		t.Fatalf("expected 1 event, got %#v", gotVars["events"])
+		t.Fatalf("expected 1 event, got %#v", gotPayload.Variables["events"])
 	}
 	event := events[0].(map[string]any)
 	assert.Equal(t, "srcCli.search", event["feature"])
@@ -54,7 +94,7 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 	assert.Equal(t, "6.1.0", source["clientVersion"])
 
 	params := event["parameters"].(map[string]any)
-	assert.Equal(t, eventParametersVersion, params["version"])
+	assert.Equal(t, float64(eventParametersVersion), params["version"])
 
 	metadata := params["metadata"].([]any)
 	// sorted by key: durationMs, exitCode
@@ -64,43 +104,48 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 	}, metadata)
 
 	client.AssertExpectations(t)
-	req.AssertExpectations(t)
 }
 
 func TestRecord_EmptyMetadataSendsEmptyList(t *testing.T) {
 	client := &apimock.Client{}
-	req := &apimock.Request{}
+	req := httptest.NewRequest(http.MethodPost, "/.api/graphql", nil)
 
-	var gotVars map[string]any
-	client.On("NewRequest", mock.Anything, mock.Anything).
-		Run(func(args mock.Arguments) { gotVars = args.Get(1).(map[string]any) }).
-		Return(req)
-	req.On("Do", mock.Anything, mock.Anything).Return(true, nil)
+	var gotPayload struct {
+		Variables map[string]any `json:"variables"`
+	}
+	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).
+		Run(func(args mock.Arguments) {
+			if err := json.NewDecoder(args.Get(3).(io.Reader)).Decode(&gotPayload); err != nil {
+				t.Fatal(err)
+			}
+		}).
+		Return(req, nil)
+	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
 	rec := NewRecorder(client, testSource())
 	rec.Record(context.Background(), Event{Feature: "srcCli.version", Action: "succeeded"})
 
-	event := gotVars["events"].([]any)[0].(map[string]any)
+	event := gotPayload.Variables["events"].([]any)[0].(map[string]any)
 	params := event["parameters"].(map[string]any)
 	assert.Equal(t, []any{}, params["metadata"])
 }
 
 func TestRecord_ValidationFailsBeforeSending(t *testing.T) {
 	client := &apimock.Client{}
-	// No expectations set: NewRequest must never be called.
+	// No expectations set: NewHTTPRequest must never be called.
 
 	rec := NewRecorder(client, testSource())
 	err := rec.record(context.Background(), Event{Feature: "Bad_Feature", Action: "succeeded"})
 
 	assert.Error(t, err)
-	client.AssertNotCalled(t, "NewRequest", mock.Anything, mock.Anything)
+	client.AssertNotCalled(t, "NewHTTPRequest", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
 func TestRecord_NetworkErrorSwallowed(t *testing.T) {
 	client := &apimock.Client{}
-	req := &apimock.Request{}
-	client.On("NewRequest", mock.Anything, mock.Anything).Return(req)
-	req.On("Do", mock.Anything, mock.Anything).Return(false, errors.New("connection refused"))
+	req := httptest.NewRequest(http.MethodPost, "/.api/graphql", nil)
+	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).Return(req, nil)
+	client.On("Do", req).Return(nil, errors.New("connection refused"))
 
 	var debug bytes.Buffer
 	rec := NewRecorder(client, testSource(), WithDebug(&debug))
@@ -120,10 +165,9 @@ func TestRecord_GraphQLErrorSwallowed(t *testing.T) {
 	// Simulates an instance too old to have the telemetry mutation: the server
 	// returns GraphQL errors, which must be dropped silently.
 	client := &apimock.Client{}
-	req := &apimock.Request{}
-	client.On("NewRequest", mock.Anything, mock.Anything).Return(req)
-	req.On("Do", mock.Anything, mock.Anything).
-		Return(false, api.GraphQlErrors{})
+	req := httptest.NewRequest(http.MethodPost, "/.api/graphql", nil)
+	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).Return(req, nil)
+	client.On("Do", req).Return(response(http.StatusOK, "{\"errors\":[{\"message\":\"unknown field telemetry\"}]}"), nil)
 
 	rec := NewRecorder(client, testSource())
 	assert.NotPanics(t, func() {
@@ -138,21 +182,101 @@ func TestRecord_NilClientDoesNotPanic(t *testing.T) {
 	})
 }
 
+func TestRecord_OAuthUnauthorizedDoesNotWriteToStdout(t *testing.T) {
+	var requests atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requests.Add(1)
+		assert.Equal(t, "Bearer oauth-token", r.Header.Get("Authorization"))
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	defer server.Close()
+
+	endpointURL, err := url.Parse(server.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var clientOutput bytes.Buffer
+	client := api.NewClient(api.ClientOpts{
+		EndpointURL: endpointURL,
+		Out:         &clientOutput,
+		OAuthToken: &oauth.Token{
+			Endpoint:    server.URL,
+			AccessToken: "oauth-token",
+			ExpiresAt:   time.Now().Add(time.Hour),
+		},
+	})
+
+	oldStdout := os.Stdout
+	stdoutReader, stdoutWriter, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	os.Stdout = stdoutWriter
+	t.Cleanup(func() { os.Stdout = oldStdout })
+
+	rec := NewRecorder(client, testSource())
+	rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+
+	if err := stdoutWriter.Close(); err != nil {
+		t.Fatal(err)
+	}
+	os.Stdout = oldStdout
+	stdout, err := io.ReadAll(stdoutReader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := stdoutReader.Close(); err != nil {
+		t.Fatal(err)
+	}
+	assert.Empty(t, stdout)
+	assert.Empty(t, clientOutput.String())
+	assert.Equal(t, int32(1), requests.Load())
+}
+
 func TestRecord_AppliesTimeout(t *testing.T) {
 	client := &apimock.Client{}
-	req := &apimock.Request{}
+	req := httptest.NewRequest(http.MethodPost, "/.api/graphql", nil)
 
 	var hadDeadline bool
-	client.On("NewRequest", mock.Anything, mock.Anything).Return(req)
-	req.On("Do", mock.Anything, mock.Anything).
+	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).
 		Run(func(args mock.Arguments) {
 			ctx := args.Get(0).(context.Context)
 			_, hadDeadline = ctx.Deadline()
 		}).
-		Return(true, nil)
+		Return(req, nil)
+	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
 	rec := NewRecorder(client, testSource(), WithTimeout(50*time.Millisecond))
 	rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
 
 	assert.True(t, hadDeadline, "expected Record to apply a context deadline")
 }
+
+func TestRecord_TimeoutCancelsHTTPRequest(t *testing.T) {
+	requestCanceled := make(chan struct{})
+	client := &cancellationClient{canceled: requestCanceled, release: make(chan struct{})}
+	rec := NewRecorder(client, testSource(), WithTimeout(20*time.Millisecond))
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	done := make(chan struct{})
+	go func() {
+		rec.Record(ctx, Event{Feature: "srcCli.search", Action: "succeeded"})
+		close(done)
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		cancel()
+		close(client.release)
+		<-done
+		t.Fatal("Record did not return after telemetry timeout")
+	}
+
+	select {
+	case <-requestCanceled:
+	default:
+		t.Fatal("HTTP request was not canceled after telemetry timeout")
+	}
+}

From 3b460b9973fea0ec634de847b76e793575d3e8fb Mon Sep 17 00:00:00 2001
From: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Date: Sat, 19 Sep 2026 19:54:40 +0100
Subject: [PATCH 3/6] refactor/telemetry: rely on server validation

The telemetry API already validates feature and action names authoritatively, so maintaining a second validator in src-cli adds drift risk without improving correctness. Remove the duplicate validation and allow rejected events to follow the recorder's existing best-effort failure path.

## Test Plan

- go test ./...
---
 internal/telemetry/telemetry.go      | 10 ++-----
 internal/telemetry/telemetry_test.go | 11 -------
 internal/telemetry/validate.go       | 39 ------------------------
 internal/telemetry/validate_test.go  | 44 ----------------------------
 4 files changed, 3 insertions(+), 101 deletions(-)
 delete mode 100644 internal/telemetry/validate.go
 delete mode 100644 internal/telemetry/validate_test.go

diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go
index b364412dfc..8d9b6a80a7 100644
--- a/internal/telemetry/telemetry.go
+++ b/internal/telemetry/telemetry.go
@@ -112,10 +112,9 @@ func NewRecorder(client api.Client, source Source, opts ...Option) *Recorder {
 }
 
 // Record sends event on a best-effort basis. It never returns an error and
-// never panics: validation, network, GraphQL, timeout, and old-instance
-// failures are all silently dropped (written to the debug writer if one was set
-// via WithDebug). It applies its own timeout, so the caller's context need not
-// carry a deadline.
+// never panics: network, GraphQL, timeout, and old-instance failures are all
+// silently dropped (written to the debug writer if one was set via WithDebug).
+// It applies its own timeout, so the caller's context need not carry a deadline.
 func (r *Recorder) Record(ctx context.Context, event Event) {
 	if err := r.record(ctx, event); err != nil && r.debug != nil {
 		fmt.Fprintf(r.debug, "telemetry: dropping event %q/%q: %v\n", event.Feature, event.Action, err)
@@ -128,9 +127,6 @@ func (r *Recorder) record(ctx context.Context, event Event) error {
 	if r.client == nil {
 		return errors.New("nil api client")
 	}
-	if err := Validate(event.Feature, event.Action); err != nil {
-		return err
-	}
 
 	ctx, cancel := context.WithTimeout(ctx, r.timeout)
 	defer cancel()
diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go
index a8dd6dc7d7..e902e1c4bb 100644
--- a/internal/telemetry/telemetry_test.go
+++ b/internal/telemetry/telemetry_test.go
@@ -130,17 +130,6 @@ func TestRecord_EmptyMetadataSendsEmptyList(t *testing.T) {
 	assert.Equal(t, []any{}, params["metadata"])
 }
 
-func TestRecord_ValidationFailsBeforeSending(t *testing.T) {
-	client := &apimock.Client{}
-	// No expectations set: NewHTTPRequest must never be called.
-
-	rec := NewRecorder(client, testSource())
-	err := rec.record(context.Background(), Event{Feature: "Bad_Feature", Action: "succeeded"})
-
-	assert.Error(t, err)
-	client.AssertNotCalled(t, "NewHTTPRequest", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
-}
-
 func TestRecord_NetworkErrorSwallowed(t *testing.T) {
 	client := &apimock.Client{}
 	req := httptest.NewRequest(http.MethodPost, "/.api/graphql", nil)
diff --git a/internal/telemetry/validate.go b/internal/telemetry/validate.go
deleted file mode 100644
index b71f3acf9e..0000000000
--- a/internal/telemetry/validate.go
+++ /dev/null
@@ -1,39 +0,0 @@
-package telemetry
-
-import (
-	"github.com/sourcegraph/src-cli/internal/lazyregexp"
-
-	"github.com/sourcegraph/sourcegraph/lib/errors"
-)
-
-// maxNameLength is the maximum length Sourcegraph accepts for a feature or
-// action name.
-const maxNameLength = 64
-
-// featureActionRegex matches the names Sourcegraph accepts for feature and
-// action: they must start with a lowercase letter and contain only letters,
-// dashes, and dots (no digits, underscores, or whitespace). It mirrors the
-// server-side validation in the Sourcegraph monorepo.
-var featureActionRegex = lazyregexp.New(`^[a-z][a-zA-Z\-.]+$`)
-
-// Validate reports whether feature and action satisfy Sourcegraph's naming
-// rules. Events that fail validation are rejected before any request is made.
-func Validate(feature, action string) error {
-	if err := validateName("feature", feature); err != nil {
-		return err
-	}
-	return validateName("action", action)
-}
-
-func validateName(kind, name string) error {
-	if name == "" {
-		return errors.Newf("telemetry %s must not be empty", kind)
-	}
-	if len(name) > maxNameLength {
-		return errors.Newf("telemetry %s %q exceeds %d characters", kind, name, maxNameLength)
-	}
-	if !featureActionRegex.MatchString(name) {
-		return errors.Newf("telemetry %s %q must match %s", kind, name, featureActionRegex.Re().String())
-	}
-	return nil
-}
diff --git a/internal/telemetry/validate_test.go b/internal/telemetry/validate_test.go
deleted file mode 100644
index bf73648dba..0000000000
--- a/internal/telemetry/validate_test.go
+++ /dev/null
@@ -1,44 +0,0 @@
-package telemetry
-
-import "testing"
-
-func TestValidate(t *testing.T) {
-	tests := []struct {
-		name    string
-		feature string
-		action  string
-		wantErr bool
-	}{
-		{name: "valid simple", feature: "srcCli", action: "succeeded"},
-		{name: "valid dotted feature", feature: "srcCli.batch.apply", action: "failed"},
-		{name: "valid dashed feature", feature: "srcCli.code-intel", action: "succeeded"},
-		{name: "empty feature", feature: "", action: "succeeded", wantErr: true},
-		{name: "empty action", feature: "srcCli", action: "", wantErr: true},
-		{name: "digit in feature", feature: "srcCli2", action: "succeeded", wantErr: true},
-		{name: "underscore in action", feature: "srcCli", action: "did_it", wantErr: true},
-		{name: "leading uppercase", feature: "SrcCli", action: "succeeded", wantErr: true},
-		{name: "whitespace", feature: "srcCli search", action: "succeeded", wantErr: true},
-		{name: "single char feature (too short for +)", feature: "s", action: "succeeded", wantErr: true},
-		{name: "too long", feature: longName(70), action: "ok", wantErr: true},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			err := Validate(tt.feature, tt.action)
-			if tt.wantErr && err == nil {
-				t.Fatalf("Validate(%q, %q) = nil, want error", tt.feature, tt.action)
-			}
-			if !tt.wantErr && err != nil {
-				t.Fatalf("Validate(%q, %q) = %v, want nil", tt.feature, tt.action, err)
-			}
-		})
-	}
-}
-
-func longName(n int) string {
-	b := make([]byte, n)
-	for i := range b {
-		b[i] = 'a'
-	}
-	return string(b)
-}

From 12e947e2034526166efafe6696479fc0d932811b Mon Sep 17 00:00:00 2001
From: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Date: Sat, 19 Sep 2026 20:40:35 +0100
Subject: [PATCH 4/6] refactor/telemetry: minimize recorder API

The telemetry package has no production callers yet, so exporting event models, source configuration, options, and test-only controls commits src-cli to an API before its integration requirements are known. Keep only the recorder construction and recording operations public, fix the client identity internally, and pass event values directly.

## Test Plan

- go test ./...
---
 internal/telemetry/telemetry.go      | 104 +++++++--------------------
 internal/telemetry/telemetry_test.go |  51 +++++++------
 2 files changed, 51 insertions(+), 104 deletions(-)

diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go
index 8d9b6a80a7..18ad68348e 100644
--- a/internal/telemetry/telemetry.go
+++ b/internal/telemetry/telemetry.go
@@ -12,8 +12,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"fmt"
-	"io"
 	"net/http"
 	"sort"
 	"time"
@@ -24,8 +22,8 @@ import (
 )
 
 const (
-	// ClientName identifies src-cli as the source of telemetry events.
-	ClientName = "SRC_CLI"
+	// clientName identifies src-cli as the source of telemetry events.
+	clientName = "SRC_CLI"
 
 	// eventParametersVersion is the schema version of the metadata we attach to
 	// each event. Bump it when the shape of the metadata changes.
@@ -48,82 +46,34 @@ const recordEventsMutation = `mutation RecordTelemetryEvents($events: [Telemetry
     }
 }`
 
-// Source identifies the client emitting events. It is constant for the lifetime
-// of a process.
-type Source struct {
-	// Client is the source client name, e.g. ClientName.
-	Client string
-	// ClientVersion is the src-cli version, e.g. "6.1.0" or "dev".
-	ClientVersion string
-}
-
-// Event is a single telemetry event.
-//
-// Feature and Action carry the event's identity and are always exported by
-// Sourcegraph, so command identity lives here (e.g. Feature "srcCli.search",
-// Action "succeeded"). Metadata values are numeric-only and are also always
-// exported; they must never contain user content. See .context/TELEMETRY.md.
-type Event struct {
-	// Feature is a noun describing what the event is about, e.g. "srcCli.search".
-	Feature string
-	// Action is a verb describing what happened, e.g. "succeeded" or "failed".
-	Action string
-	// Metadata holds numeric-only, PII-free facts about the event.
-	Metadata map[string]float64
-}
-
-// Recorder records events for a single Source through an api.Client.
+// Recorder records events through an api.Client.
 type Recorder struct {
-	client  api.Client
-	source  Source
-	timeout time.Duration
-	debug   io.Writer
+	client        api.Client
+	clientVersion string
+	timeout       time.Duration
 }
 
-// Option customizes a Recorder.
-type Option func(*Recorder)
-
-// WithTimeout overrides the default per-Record timeout.
-func WithTimeout(d time.Duration) Option {
-	return func(r *Recorder) {
-		if d > 0 {
-			r.timeout = d
-		}
+// NewRecorder returns a Recorder for the given src-cli version.
+func NewRecorder(client api.Client, clientVersion string) *Recorder {
+	return &Recorder{
+		client:        client,
+		clientVersion: clientVersion,
+		timeout:       defaultTimeout,
 	}
 }
 
-// WithDebug sets a writer that receives a diagnostic line whenever an event is
-// dropped. Intended to be wired to verbose (-v) output; leave unset for silence.
-func WithDebug(w io.Writer) Option {
-	return func(r *Recorder) { r.debug = w }
-}
-
-// NewRecorder returns a Recorder that records events for source through client.
-func NewRecorder(client api.Client, source Source, opts ...Option) *Recorder {
-	r := &Recorder{
-		client:  client,
-		source:  source,
-		timeout: defaultTimeout,
-	}
-	for _, opt := range opts {
-		opt(r)
-	}
-	return r
-}
-
-// Record sends event on a best-effort basis. It never returns an error and
-// never panics: network, GraphQL, timeout, and old-instance failures are all
-// silently dropped (written to the debug writer if one was set via WithDebug).
-// It applies its own timeout, so the caller's context need not carry a deadline.
-func (r *Recorder) Record(ctx context.Context, event Event) {
-	if err := r.record(ctx, event); err != nil && r.debug != nil {
-		fmt.Fprintf(r.debug, "telemetry: dropping event %q/%q: %v\n", event.Feature, event.Action, err)
-	}
+// Record sends an event on a best-effort basis. Feature and action identify the
+// event (for example, "srcCli.search" and "succeeded"). Metadata must contain
+// only numeric, PII-free facts. Record never returns an error or panics: network,
+// GraphQL, timeout, and old-instance failures are silently dropped. It applies
+// its own timeout, so the caller's context need not carry a deadline.
+func (r *Recorder) Record(ctx context.Context, feature, action string, metadata map[string]float64) {
+	_ = r.record(ctx, feature, action, metadata)
 }
 
 // record does the work behind Record and returns any error, so it can be tested
 // directly. Callers outside tests should use Record.
-func (r *Recorder) record(ctx context.Context, event Event) error {
+func (r *Recorder) record(ctx context.Context, feature, action string, metadata map[string]float64) error {
 	if r.client == nil {
 		return errors.New("nil api client")
 	}
@@ -132,7 +82,7 @@ func (r *Recorder) record(ctx context.Context, event Event) error {
 	defer cancel()
 
 	vars := map[string]any{
-		"events": []any{buildEventInput(r.source, event)},
+		"events": []any{buildEventInput(r.clientVersion, feature, action, metadata)},
 	}
 
 	payload, err := json.Marshal(map[string]any{
@@ -170,17 +120,17 @@ func (r *Recorder) record(ctx context.Context, event Event) error {
 }
 
 // buildEventInput builds a single TelemetryEventInput as a JSON-serializable map.
-func buildEventInput(source Source, event Event) map[string]any {
+func buildEventInput(clientVersion, feature, action string, metadata map[string]float64) map[string]any {
 	return map[string]any{
-		"feature": event.Feature,
-		"action":  event.Action,
+		"feature": feature,
+		"action":  action,
 		"source": map[string]any{
-			"client":        source.Client,
-			"clientVersion": source.ClientVersion,
+			"client":        clientName,
+			"clientVersion": clientVersion,
 		},
 		"parameters": map[string]any{
 			"version":  eventParametersVersion,
-			"metadata": buildMetadata(event.Metadata),
+			"metadata": buildMetadata(metadata),
 		},
 	}
 }
diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go
index e902e1c4bb..3e7957df35 100644
--- a/internal/telemetry/telemetry_test.go
+++ b/internal/telemetry/telemetry_test.go
@@ -23,9 +23,7 @@ import (
 	"github.com/stretchr/testify/mock"
 )
 
-func testSource() Source {
-	return Source{Client: ClientName, ClientVersion: "6.1.0"}
-}
+const testClientVersion = "6.1.0"
 
 func response(statusCode int, body string) *http.Response {
 	return &http.Response{
@@ -72,11 +70,10 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 		Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
-	rec := NewRecorder(client, testSource())
-	rec.Record(context.Background(), Event{
-		Feature:  "srcCli.search",
-		Action:   "succeeded",
-		Metadata: map[string]float64{"durationMs": 12, "exitCode": 0},
+	rec := NewRecorder(client, testClientVersion)
+	rec.Record(context.Background(), "srcCli.search", "succeeded", map[string]float64{
+		"durationMs": 12,
+		"exitCode":   0,
 	})
 
 	assert.Equal(t, recordEventsMutation, gotPayload.Query)
@@ -90,8 +87,8 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 	assert.Equal(t, "succeeded", event["action"])
 
 	source := event["source"].(map[string]any)
-	assert.Equal(t, ClientName, source["client"])
-	assert.Equal(t, "6.1.0", source["clientVersion"])
+	assert.Equal(t, clientName, source["client"])
+	assert.Equal(t, testClientVersion, source["clientVersion"])
 
 	params := event["parameters"].(map[string]any)
 	assert.Equal(t, float64(eventParametersVersion), params["version"])
@@ -122,8 +119,8 @@ func TestRecord_EmptyMetadataSendsEmptyList(t *testing.T) {
 		Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
-	rec := NewRecorder(client, testSource())
-	rec.Record(context.Background(), Event{Feature: "srcCli.version", Action: "succeeded"})
+	rec := NewRecorder(client, testClientVersion)
+	rec.Record(context.Background(), "srcCli.version", "succeeded", nil)
 
 	event := gotPayload.Variables["events"].([]any)[0].(map[string]any)
 	params := event["parameters"].(map[string]any)
@@ -136,17 +133,15 @@ func TestRecord_NetworkErrorSwallowed(t *testing.T) {
 	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).Return(req, nil)
 	client.On("Do", req).Return(nil, errors.New("connection refused"))
 
-	var debug bytes.Buffer
-	rec := NewRecorder(client, testSource(), WithDebug(&debug))
+	rec := NewRecorder(client, testClientVersion)
 
 	// Must not panic and must not surface the error.
 	assert.NotPanics(t, func() {
-		rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "failed"})
+		rec.Record(context.Background(), "srcCli.search", "failed", nil)
 	})
-	assert.Contains(t, debug.String(), "connection refused")
 
 	// record itself reports the error for callers that want it.
-	err := rec.record(context.Background(), Event{Feature: "srcCli.search", Action: "failed"})
+	err := rec.record(context.Background(), "srcCli.search", "failed", nil)
 	assert.Error(t, err)
 }
 
@@ -158,16 +153,16 @@ func TestRecord_GraphQLErrorSwallowed(t *testing.T) {
 	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{\"errors\":[{\"message\":\"unknown field telemetry\"}]}"), nil)
 
-	rec := NewRecorder(client, testSource())
+	rec := NewRecorder(client, testClientVersion)
 	assert.NotPanics(t, func() {
-		rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+		rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
 	})
 }
 
 func TestRecord_NilClientDoesNotPanic(t *testing.T) {
-	rec := NewRecorder(nil, testSource())
+	rec := NewRecorder(nil, testClientVersion)
 	assert.NotPanics(t, func() {
-		rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+		rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
 	})
 }
 
@@ -203,8 +198,8 @@ func TestRecord_OAuthUnauthorizedDoesNotWriteToStdout(t *testing.T) {
 	os.Stdout = stdoutWriter
 	t.Cleanup(func() { os.Stdout = oldStdout })
 
-	rec := NewRecorder(client, testSource())
-	rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+	rec := NewRecorder(client, testClientVersion)
+	rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
 
 	if err := stdoutWriter.Close(); err != nil {
 		t.Fatal(err)
@@ -235,8 +230,9 @@ func TestRecord_AppliesTimeout(t *testing.T) {
 		Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
-	rec := NewRecorder(client, testSource(), WithTimeout(50*time.Millisecond))
-	rec.Record(context.Background(), Event{Feature: "srcCli.search", Action: "succeeded"})
+	rec := NewRecorder(client, testClientVersion)
+	rec.timeout = 50 * time.Millisecond
+	rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
 
 	assert.True(t, hadDeadline, "expected Record to apply a context deadline")
 }
@@ -244,13 +240,14 @@ func TestRecord_AppliesTimeout(t *testing.T) {
 func TestRecord_TimeoutCancelsHTTPRequest(t *testing.T) {
 	requestCanceled := make(chan struct{})
 	client := &cancellationClient{canceled: requestCanceled, release: make(chan struct{})}
-	rec := NewRecorder(client, testSource(), WithTimeout(20*time.Millisecond))
+	rec := NewRecorder(client, testClientVersion)
+	rec.timeout = 20 * time.Millisecond
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	done := make(chan struct{})
 	go func() {
-		rec.Record(ctx, Event{Feature: "srcCli.search", Action: "succeeded"})
+		rec.Record(ctx, "srcCli.search", "succeeded", nil)
 		close(done)
 	}()
 

From 1574f384138df68ffb2e573d57173f321dc84004 Mon Sep 17 00:00:00 2001
From: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Date: Tue, 22 Sep 2026 15:10:03 +0100
Subject: [PATCH 5/6] refactor/telemetry: add debug diagnostics

Keep expected telemetry failures out of normal output while making them available through debug logging with the underlying error. Narrow the concrete recorder type, require its logger dependency, and retain the server-required parameters and numeric metadata wire format.

## Test Plan

- go test ./...
---
 internal/telemetry/telemetry.go      | 42 +++++++++++-----------
 internal/telemetry/telemetry_test.go | 52 ++++++++++++++++------------
 2 files changed, 49 insertions(+), 45 deletions(-)

diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go
index 18ad68348e..3946002d53 100644
--- a/internal/telemetry/telemetry.go
+++ b/internal/telemetry/telemetry.go
@@ -18,12 +18,13 @@ import (
 
 	"github.com/sourcegraph/src-cli/internal/api"
 
+	"github.com/sourcegraph/log"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
 const (
 	// clientName identifies src-cli as the source of telemetry events.
-	clientName = "SRC_CLI"
+	clientName = "src.cli"
 
 	// eventParametersVersion is the schema version of the metadata we attach to
 	// each event. Bump it when the shape of the metadata changes.
@@ -47,37 +48,37 @@ const recordEventsMutation = `mutation RecordTelemetryEvents($events: [Telemetry
 }`
 
 // Recorder records events through an api.Client.
-type Recorder struct {
+type recorder struct {
 	client        api.Client
 	clientVersion string
 	timeout       time.Duration
+	logger        log.Logger
 }
 
 // NewRecorder returns a Recorder for the given src-cli version.
-func NewRecorder(client api.Client, clientVersion string) *Recorder {
-	return &Recorder{
+func NewRecorder(client api.Client, logger log.Logger, clientVersion string) *recorder {
+	return &recorder{
 		client:        client,
 		clientVersion: clientVersion,
 		timeout:       defaultTimeout,
+		logger:        logger,
 	}
 }
 
 // Record sends an event on a best-effort basis. Feature and action identify the
 // event (for example, "srcCli.search" and "succeeded"). Metadata must contain
-// only numeric, PII-free facts. Record never returns an error or panics: network,
-// GraphQL, timeout, and old-instance failures are silently dropped. It applies
-// its own timeout, so the caller's context need not carry a deadline.
-func (r *Recorder) Record(ctx context.Context, feature, action string, metadata map[string]float64) {
-	_ = r.record(ctx, feature, action, metadata)
+// only numeric, PII-free facts. Network, GraphQL, timeout, and old-instance
+// failures are logged at debug level. Record applies its own timeout, so the
+// caller's context need not carry a deadline.
+func (r *recorder) Record(ctx context.Context, feature, action string, metadata map[string]float64) {
+	if err := r.record(ctx, feature, action, metadata); err != nil {
+		r.logger.Debug("recording telemetry event", log.String("feature", feature), log.String("action", action), log.Error(err))
+	}
 }
 
 // record does the work behind Record and returns any error, so it can be tested
 // directly. Callers outside tests should use Record.
-func (r *Recorder) record(ctx context.Context, feature, action string, metadata map[string]float64) error {
-	if r.client == nil {
-		return errors.New("nil api client")
-	}
-
+func (r *recorder) record(ctx context.Context, feature, action string, metadata map[string]float64) error {
 	ctx, cancel := context.WithTimeout(ctx, r.timeout)
 	defer cancel()
 
@@ -124,7 +125,7 @@ func buildEventInput(clientVersion, feature, action string, metadata map[string]
 	return map[string]any{
 		"feature": feature,
 		"action":  action,
-		"source": map[string]any{
+		"source": map[string]string{
 			"client":        clientName,
 			"clientVersion": clientVersion,
 		},
@@ -139,16 +140,13 @@ func buildEventInput(clientVersion, feature, action string, metadata map[string]
 // the API expects, sorted by key for deterministic output.
 func buildMetadata(metadata map[string]float64) []any {
 	out := make([]any, 0, len(metadata))
-	if len(metadata) == 0 {
-		return out
-	}
 	keys := make([]string, 0, len(metadata))
-	for k := range metadata {
-		keys = append(keys, k)
+	for key := range metadata {
+		keys = append(keys, key)
 	}
 	sort.Strings(keys)
-	for _, k := range keys {
-		out = append(out, map[string]any{"key": k, "value": metadata[k]})
+	for _, key := range keys {
+		out = append(out, map[string]any{"key": key, "value": metadata[key]})
 	}
 	return out
 }
diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go
index 3e7957df35..e510c40aff 100644
--- a/internal/telemetry/telemetry_test.go
+++ b/internal/telemetry/telemetry_test.go
@@ -18,6 +18,8 @@ import (
 	apimock "github.com/sourcegraph/src-cli/internal/api/mock"
 	"github.com/sourcegraph/src-cli/internal/oauth"
 
+	"github.com/sourcegraph/log"
+	"github.com/sourcegraph/log/logtest"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -70,7 +72,8 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 		Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
-	rec := NewRecorder(client, testClientVersion)
+	logger := log.NoOp()
+	rec := NewRecorder(client, logger, testClientVersion)
 	rec.Record(context.Background(), "srcCli.search", "succeeded", map[string]float64{
 		"durationMs": 12,
 		"exitCode":   0,
@@ -90,20 +93,17 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 	assert.Equal(t, clientName, source["client"])
 	assert.Equal(t, testClientVersion, source["clientVersion"])
 
-	params := event["parameters"].(map[string]any)
-	assert.Equal(t, float64(eventParametersVersion), params["version"])
-
-	metadata := params["metadata"].([]any)
-	// sorted by key: durationMs, exitCode
+	parameters := event["parameters"].(map[string]any)
+	assert.Equal(t, float64(eventParametersVersion), parameters["version"])
 	assert.Equal(t, []any{
 		map[string]any{"key": "durationMs", "value": float64(12)},
 		map[string]any{"key": "exitCode", "value": float64(0)},
-	}, metadata)
+	}, parameters["metadata"])
 
 	client.AssertExpectations(t)
 }
 
-func TestRecord_EmptyMetadataSendsEmptyList(t *testing.T) {
+func TestRecord_NilMetadataSendsEmptyList(t *testing.T) {
 	client := &apimock.Client{}
 	req := httptest.NewRequest(http.MethodPost, "/.api/graphql", nil)
 
@@ -119,12 +119,14 @@ func TestRecord_EmptyMetadataSendsEmptyList(t *testing.T) {
 		Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
-	rec := NewRecorder(client, testClientVersion)
+	logger := log.NoOp()
+	rec := NewRecorder(client, logger, testClientVersion)
 	rec.Record(context.Background(), "srcCli.version", "succeeded", nil)
 
 	event := gotPayload.Variables["events"].([]any)[0].(map[string]any)
-	params := event["parameters"].(map[string]any)
-	assert.Equal(t, []any{}, params["metadata"])
+	parameters := event["parameters"].(map[string]any)
+	assert.Equal(t, float64(eventParametersVersion), parameters["version"])
+	assert.Equal(t, []any{}, parameters["metadata"])
 }
 
 func TestRecord_NetworkErrorSwallowed(t *testing.T) {
@@ -133,12 +135,19 @@ func TestRecord_NetworkErrorSwallowed(t *testing.T) {
 	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).Return(req, nil)
 	client.On("Do", req).Return(nil, errors.New("connection refused"))
 
-	rec := NewRecorder(client, testClientVersion)
+	logger, exportLogs := logtest.CapturedWith(t, logtest.LoggerOptions{Level: log.LevelNone})
+	rec := NewRecorder(client, logger, testClientVersion)
 
 	// Must not panic and must not surface the error.
 	assert.NotPanics(t, func() {
 		rec.Record(context.Background(), "srcCli.search", "failed", nil)
 	})
+	logs := exportLogs()
+	if assert.Len(t, logs, 1) {
+		assert.Equal(t, log.LevelDebug, logs[0].Level)
+		assert.Equal(t, "recording telemetry event", logs[0].Message)
+		assert.Equal(t, "connection refused", logs[0].Fields["error"])
+	}
 
 	// record itself reports the error for callers that want it.
 	err := rec.record(context.Background(), "srcCli.search", "failed", nil)
@@ -153,14 +162,8 @@ func TestRecord_GraphQLErrorSwallowed(t *testing.T) {
 	client.On("NewHTTPRequest", mock.Anything, http.MethodPost, ".api/graphql", mock.Anything).Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{\"errors\":[{\"message\":\"unknown field telemetry\"}]}"), nil)
 
-	rec := NewRecorder(client, testClientVersion)
-	assert.NotPanics(t, func() {
-		rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
-	})
-}
-
-func TestRecord_NilClientDoesNotPanic(t *testing.T) {
-	rec := NewRecorder(nil, testClientVersion)
+	logger := log.NoOp()
+	rec := NewRecorder(client, logger, testClientVersion)
 	assert.NotPanics(t, func() {
 		rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
 	})
@@ -198,7 +201,8 @@ func TestRecord_OAuthUnauthorizedDoesNotWriteToStdout(t *testing.T) {
 	os.Stdout = stdoutWriter
 	t.Cleanup(func() { os.Stdout = oldStdout })
 
-	rec := NewRecorder(client, testClientVersion)
+	logger := log.NoOp()
+	rec := NewRecorder(client, logger, testClientVersion)
 	rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
 
 	if err := stdoutWriter.Close(); err != nil {
@@ -230,7 +234,8 @@ func TestRecord_AppliesTimeout(t *testing.T) {
 		Return(req, nil)
 	client.On("Do", req).Return(response(http.StatusOK, "{}"), nil)
 
-	rec := NewRecorder(client, testClientVersion)
+	logger := log.NoOp()
+	rec := NewRecorder(client, logger, testClientVersion)
 	rec.timeout = 50 * time.Millisecond
 	rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
 
@@ -240,7 +245,8 @@ func TestRecord_AppliesTimeout(t *testing.T) {
 func TestRecord_TimeoutCancelsHTTPRequest(t *testing.T) {
 	requestCanceled := make(chan struct{})
 	client := &cancellationClient{canceled: requestCanceled, release: make(chan struct{})}
-	rec := NewRecorder(client, testClientVersion)
+	logger := log.NoOp()
+	rec := NewRecorder(client, logger, testClientVersion)
 	rec.timeout = 20 * time.Millisecond
 
 	ctx, cancel := context.WithCancel(context.Background())

From 3637d1ca68cff82e482b63ad766a9664f4dd3bfe Mon Sep 17 00:00:00 2001
From: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Date: Tue, 22 Sep 2026 16:28:15 +0100
Subject: [PATCH 6/6] add privateMetadata fields

---
 internal/telemetry/telemetry.go      | 29 ++++++++++++++++------------
 internal/telemetry/telemetry_test.go | 22 ++++++++++++++-------
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go
index 3946002d53..c7b7d1ede1 100644
--- a/internal/telemetry/telemetry.go
+++ b/internal/telemetry/telemetry.go
@@ -67,23 +67,24 @@ func NewRecorder(client api.Client, logger log.Logger, clientVersion string) *re
 
 // Record sends an event on a best-effort basis. Feature and action identify the
 // event (for example, "srcCli.search" and "succeeded"). Metadata must contain
-// only numeric, PII-free facts. Network, GraphQL, timeout, and old-instance
-// failures are logged at debug level. Record applies its own timeout, so the
-// caller's context need not carry a deadline.
-func (r *recorder) Record(ctx context.Context, feature, action string, metadata map[string]float64) {
-	if err := r.record(ctx, feature, action, metadata); err != nil {
+// only numeric, PII-free facts. Private metadata may contain arbitrary JSON and
+// is not exported from Sourcegraph instances by default. Network, GraphQL,
+// timeout, and old-instance failures are logged at debug level. Record applies
+// its own timeout, so the caller's context need not carry a deadline.
+func (r *recorder) Record(ctx context.Context, feature, action string, metadata map[string]float64, privateMetadata map[string]any) {
+	if err := r.record(ctx, feature, action, metadata, privateMetadata); err != nil {
 		r.logger.Debug("recording telemetry event", log.String("feature", feature), log.String("action", action), log.Error(err))
 	}
 }
 
 // record does the work behind Record and returns any error, so it can be tested
 // directly. Callers outside tests should use Record.
-func (r *recorder) record(ctx context.Context, feature, action string, metadata map[string]float64) error {
+func (r *recorder) record(ctx context.Context, feature, action string, metadata map[string]float64, privateMetadata map[string]any) error {
 	ctx, cancel := context.WithTimeout(ctx, r.timeout)
 	defer cancel()
 
 	vars := map[string]any{
-		"events": []any{buildEventInput(r.clientVersion, feature, action, metadata)},
+		"events": []any{buildEventInput(r.clientVersion, feature, action, metadata, privateMetadata)},
 	}
 
 	payload, err := json.Marshal(map[string]any{
@@ -121,7 +122,14 @@ func (r *recorder) record(ctx context.Context, feature, action string, metadata
 }
 
 // buildEventInput builds a single TelemetryEventInput as a JSON-serializable map.
-func buildEventInput(clientVersion, feature, action string, metadata map[string]float64) map[string]any {
+func buildEventInput(clientVersion, feature, action string, metadata map[string]float64, privateMetadata map[string]any) map[string]any {
+	parameters := map[string]any{
+		"version":  eventParametersVersion,
+		"metadata": buildMetadata(metadata),
+	}
+	if privateMetadata != nil {
+		parameters["privateMetadata"] = privateMetadata
+	}
 	return map[string]any{
 		"feature": feature,
 		"action":  action,
@@ -129,10 +137,7 @@ func buildEventInput(clientVersion, feature, action string, metadata map[string]
 			"client":        clientName,
 			"clientVersion": clientVersion,
 		},
-		"parameters": map[string]any{
-			"version":  eventParametersVersion,
-			"metadata": buildMetadata(metadata),
-		},
+		"parameters": parameters,
 	}
 }
 
diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go
index e510c40aff..0303fd4ccf 100644
--- a/internal/telemetry/telemetry_test.go
+++ b/internal/telemetry/telemetry_test.go
@@ -77,6 +77,9 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 	rec.Record(context.Background(), "srcCli.search", "succeeded", map[string]float64{
 		"durationMs": 12,
 		"exitCode":   0,
+	}, map[string]any{
+		"queryType": "literal",
+		"streamed":  true,
 	})
 
 	assert.Equal(t, recordEventsMutation, gotPayload.Query)
@@ -99,6 +102,10 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
 		map[string]any{"key": "durationMs", "value": float64(12)},
 		map[string]any{"key": "exitCode", "value": float64(0)},
 	}, parameters["metadata"])
+	assert.Equal(t, map[string]any{
+		"queryType": "literal",
+		"streamed":  true,
+	}, parameters["privateMetadata"])
 
 	client.AssertExpectations(t)
 }
@@ -121,12 +128,13 @@ func TestRecord_NilMetadataSendsEmptyList(t *testing.T) {
 
 	logger := log.NoOp()
 	rec := NewRecorder(client, logger, testClientVersion)
-	rec.Record(context.Background(), "srcCli.version", "succeeded", nil)
+	rec.Record(context.Background(), "srcCli.version", "succeeded", nil, nil)
 
 	event := gotPayload.Variables["events"].([]any)[0].(map[string]any)
 	parameters := event["parameters"].(map[string]any)
 	assert.Equal(t, float64(eventParametersVersion), parameters["version"])
 	assert.Equal(t, []any{}, parameters["metadata"])
+	assert.NotContains(t, parameters, "privateMetadata")
 }
 
 func TestRecord_NetworkErrorSwallowed(t *testing.T) {
@@ -140,7 +148,7 @@ func TestRecord_NetworkErrorSwallowed(t *testing.T) {
 
 	// Must not panic and must not surface the error.
 	assert.NotPanics(t, func() {
-		rec.Record(context.Background(), "srcCli.search", "failed", nil)
+		rec.Record(context.Background(), "srcCli.search", "failed", nil, nil)
 	})
 	logs := exportLogs()
 	if assert.Len(t, logs, 1) {
@@ -150,7 +158,7 @@ func TestRecord_NetworkErrorSwallowed(t *testing.T) {
 	}
 
 	// record itself reports the error for callers that want it.
-	err := rec.record(context.Background(), "srcCli.search", "failed", nil)
+	err := rec.record(context.Background(), "srcCli.search", "failed", nil, nil)
 	assert.Error(t, err)
 }
 
@@ -165,7 +173,7 @@ func TestRecord_GraphQLErrorSwallowed(t *testing.T) {
 	logger := log.NoOp()
 	rec := NewRecorder(client, logger, testClientVersion)
 	assert.NotPanics(t, func() {
-		rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
+		rec.Record(context.Background(), "srcCli.search", "succeeded", nil, nil)
 	})
 }
 
@@ -203,7 +211,7 @@ func TestRecord_OAuthUnauthorizedDoesNotWriteToStdout(t *testing.T) {
 
 	logger := log.NoOp()
 	rec := NewRecorder(client, logger, testClientVersion)
-	rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
+	rec.Record(context.Background(), "srcCli.search", "succeeded", nil, nil)
 
 	if err := stdoutWriter.Close(); err != nil {
 		t.Fatal(err)
@@ -237,7 +245,7 @@ func TestRecord_AppliesTimeout(t *testing.T) {
 	logger := log.NoOp()
 	rec := NewRecorder(client, logger, testClientVersion)
 	rec.timeout = 50 * time.Millisecond
-	rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
+	rec.Record(context.Background(), "srcCli.search", "succeeded", nil, nil)
 
 	assert.True(t, hadDeadline, "expected Record to apply a context deadline")
 }
@@ -253,7 +261,7 @@ func TestRecord_TimeoutCancelsHTTPRequest(t *testing.T) {
 	defer cancel()
 	done := make(chan struct{})
 	go func() {
-		rec.Record(ctx, "srcCli.search", "succeeded", nil)
+		rec.Record(ctx, "srcCli.search", "succeeded", nil, nil)
 		close(done)
 	}()