From 9df26f2f5af894782cbed05b18dbc2ede23f780b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 14 Sep 2026 18:07:39 -0700 Subject: [PATCH 1/2] improvement(analytics): attribute every request, credential, and queued run --- apps/docs/content/docs/cli/usage-data.mdx | 2 +- apps/sim/app/api/v1/auth.ts | 9 +- apps/sim/background/schedule-execution.ts | 9 +- apps/sim/background/webhook-execution.ts | 9 +- apps/sim/background/workflow-execution.ts | 5 +- apps/sim/lib/api-key/service.test.ts | 22 +++++ apps/sim/lib/api-key/service.ts | 6 +- apps/sim/lib/auth/auth.ts | 22 ++++- apps/sim/lib/auth/hybrid.ts | 43 +------- apps/sim/lib/auth/internal.test.ts | 14 +++ apps/sim/lib/auth/internal.ts | 3 +- apps/sim/lib/auth/oauth-access-token.test.ts | 6 ++ apps/sim/lib/auth/oauth-access-token.ts | 3 +- .../core/utils/request-attribution.test.ts | 43 ++++++++ .../sim/lib/core/utils/request-attribution.ts | 29 ++++++ apps/sim/lib/posthog/server.test.ts | 18 ++++ apps/sim/lib/posthog/server.ts | 1 + .../executor/enqueue-execution.test.ts | 60 ++++++++++++ .../workflows/executor/enqueue-execution.ts | 2 + packages/logger/src/index.ts | 3 +- packages/logger/src/request-context.test.ts | 61 ++++++++++++ packages/logger/src/request-context.ts | 24 ++++- .../sim-cli/src/telemetry/client-info.test.ts | 4 + packages/sim-cli/src/telemetry/client-info.ts | 6 +- .../sim-cli/src/telemetry/coding-agent.ts | 7 ++ .../sim-cli/src/telemetry/invocation.test.ts | 9 ++ packages/sim-cli/src/telemetry/invocation.ts | 9 +- packages/utils/src/client-info.test.ts | 74 ++++++++++++-- packages/utils/src/client-info.ts | 97 +++++++++++++++++-- 29 files changed, 513 insertions(+), 87 deletions(-) create mode 100644 apps/sim/lib/core/utils/request-attribution.test.ts create mode 100644 apps/sim/lib/core/utils/request-attribution.ts create mode 100644 apps/sim/lib/workflows/executor/enqueue-execution.test.ts create mode 100644 packages/logger/src/request-context.test.ts diff --git a/apps/docs/content/docs/cli/usage-data.mdx b/apps/docs/content/docs/cli/usage-data.mdx index 229bfa374e5..9c21bd5f092 100644 --- a/apps/docs/content/docs/cli/usage-data.mdx +++ b/apps/docs/content/docs/cli/usage-data.mdx @@ -20,7 +20,7 @@ One event per command, after the command finishes: | Duration | `1432` ms | From process start to completion | | CLI, Node, OS, CPU | `2.1.2`, `22.14.0`, `darwin`, `arm64` | | | Terminal and CI | `is_tty`, `is_ci` | Whether stdout is a terminal, whether a CI variable is set | -| Coding agent | `claude-code` | When the CLI runs inside an AI coding agent's shell | +| Coding agent | `claude-code`, or `none` | The AI coding agent whose shell the CLI runs in, if any | | Deployment kind | `hosted` or `self_hosted` | Never the address | | Device and session ids | random UUIDs | See below | diff --git a/apps/sim/app/api/v1/auth.ts b/apps/sim/app/api/v1/auth.ts index 770dbfee8e9..78c68e1f9dd 100644 --- a/apps/sim/app/api/v1/auth.ts +++ b/apps/sim/app/api/v1/auth.ts @@ -1,9 +1,5 @@ -import { - describePrincipalAuth, - type PersonalApiKeyPrincipal, - type WorkspaceApiKeyPrincipal, -} from '@sim/auth/principal' -import { createLogger, setRequestAuth } from '@sim/logger' +import type { PersonalApiKeyPrincipal, WorkspaceApiKeyPrincipal } from '@sim/auth/principal' +import { createLogger } from '@sim/logger' import type { NextRequest } from 'next/server' import { authenticateApiKeyFromHeader, updateApiKeyLastUsed } from '@/lib/api-key/service' import { ANONYMOUS_USER_ID } from '@/lib/auth/constants' @@ -76,7 +72,6 @@ export async function authenticateV1Request(request: NextRequest): Promise { + /** A trigger, not a client, started this run. */ + const requestContext: RequestContext = { + requestId, + client: { surface: 'schedule', source: 'trigger' }, + } + return await runWithRequestContext(requestContext, async () => { logger.info(`[${requestId}] Starting schedule execution`, { scheduleId: payload.scheduleId, workflowId: payload.workflowId, diff --git a/apps/sim/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index 5240bc500c9..78a2f5aa7b0 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -6,7 +6,7 @@ import { } from '@sim/auth/principal' import { db } from '@sim/db' import { account, webhook } from '@sim/db/schema' -import { createLogger, runWithRequestContext } from '@sim/logger' +import { createLogger, type RequestContext, runWithRequestContext } from '@sim/logger' import { toError } from '@sim/utils/errors' import { interruptibleSleep } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' @@ -556,7 +556,12 @@ export async function executeWebhookJob( }) } - return await runWithRequestContext({ requestId }, async () => { + /** A trigger, not a client, started this run. */ + const requestContext: RequestContext = { + requestId, + client: { surface: 'webhook', source: 'trigger' }, + } + return await runWithRequestContext(requestContext, async () => { logger.info(`[${requestId}] Starting webhook execution`, { webhookId: authenticatedPayload.webhookId, workflowId: authenticatedPayload.workflowId, diff --git a/apps/sim/background/workflow-execution.ts b/apps/sim/background/workflow-execution.ts index e4b82ca8034..b04e5a0610c 100644 --- a/apps/sim/background/workflow-execution.ts +++ b/apps/sim/background/workflow-execution.ts @@ -25,6 +25,7 @@ import { getTimeoutErrorMessage, RESERVATION_TTL_BUFFER_MS, } from '@/lib/core/execution-limits' +import type { RequestAttribution } from '@/lib/core/utils/request-attribution' import { preprocessExecution } from '@/lib/execution/preprocessing' import { LoggingSession } from '@/lib/logs/execution/logging-session' import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' @@ -76,6 +77,8 @@ export type WorkflowExecutionPayload = { correlation?: AsyncExecutionCorrelation metadata?: Record callChain?: string[] + /** Who queued the run, restored into the job's context so its events stay attributed. */ + attribution?: RequestAttribution executionMode?: 'sync' | 'stream' | 'async' /** Upstream preprocessing already consumed rate-limit quota and owns the usage reservation. */ admissionCompleted?: boolean @@ -177,7 +180,7 @@ export async function executeWorkflowJob( } } - return await runWithRequestContext({ requestId }, async () => { + return await runWithRequestContext({ requestId, ...payload.attribution }, async () => { logger.info(`[${requestId}] Starting workflow execution job: ${workflowId}`, { userId: payload.userId, triggerType: payload.triggerType, diff --git a/apps/sim/lib/api-key/service.test.ts b/apps/sim/lib/api-key/service.test.ts index 0c78eca1391..07a5a83b04e 100644 --- a/apps/sim/lib/api-key/service.test.ts +++ b/apps/sim/lib/api-key/service.test.ts @@ -7,6 +7,7 @@ * * @vitest-environment node */ +import { setRequestAuth } from '@sim/logger' import { dbChainMockFns } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -95,6 +96,27 @@ describe('authenticateApiKeyFromHeader', () => { expect(dbChainMockFns.where).toHaveBeenCalledTimes(1) }) + it('records the key kind on the request without replacing a more specific principal', async () => { + dbChainMockFns.where.mockResolvedValueOnce([ + personalKeyRecord({ type: 'workspace', workspaceId: 'workspace-1' }), + ]) + + await authenticateApiKeyFromHeader('sk-sim-plain-key') + + expect(vi.mocked(setRequestAuth)).toHaveBeenCalledWith( + { kind: 'workspace_api_key' }, + { preserveExisting: true } + ) + }) + + it('records nothing for a key that fails its checks', async () => { + dbChainMockFns.where.mockResolvedValueOnce([personalKeyRecord({ userId: 'other-user' })]) + + await authenticateApiKeyFromHeader('sk-sim-plain-key', { userId: 'user-1' }) + + expect(vi.mocked(setRequestAuth)).not.toHaveBeenCalled() + }) + it('returns invalid when the hash lookup finds a row that fails scope checks', async () => { const record = personalKeyRecord({ userId: 'other-user' }) dbChainMockFns.where.mockResolvedValueOnce([record]) diff --git a/apps/sim/lib/api-key/service.ts b/apps/sim/lib/api-key/service.ts index edcee7df2f1..79baf152f56 100644 --- a/apps/sim/lib/api-key/service.ts +++ b/apps/sim/lib/api-key/service.ts @@ -1,6 +1,6 @@ import { db } from '@sim/db' import { apiKey as apiKeyTable, user as userTable } from '@sim/db/schema' -import { createLogger } from '@sim/logger' +import { createLogger, setRequestAuth } from '@sim/logger' import { and, eq, isNull, lt, or } from 'drizzle-orm' import { hashApiKey } from '@/lib/api-key/crypto' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' @@ -132,6 +132,10 @@ export async function authenticateApiKeyFromHeader( } logger.debug('API key matched via hash lookup', { keyId: record.id, keyType }) + setRequestAuth( + { kind: keyType === 'personal' ? 'personal_api_key' : 'workspace_api_key' }, + { preserveExisting: true } + ) return { success: true, diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index f5c839f6c8e..db651381b50 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -4,7 +4,7 @@ import { sso } from '@better-auth/sso' import { stripe } from '@better-auth/stripe' import { db } from '@sim/db' import * as schema from '@sim/db/schema' -import { createLogger } from '@sim/logger' +import { createLogger, setRequestAuth } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { type BetterAuthOptions, betterAuth, type User } from 'better-auth' import { @@ -1758,13 +1758,25 @@ export const auth = betterAuth({ async function getSessionImpl() { if (isAuthDisabled) { await ensureAnonymousUserExists() - return createAnonymousSession() + return recordSessionAuth(createAnonymousSession()) } const hdrs = await headers() - return await auth.api.getSession({ - headers: hdrs, - }) + return recordSessionAuth( + await auth.api.getSession({ + headers: hdrs, + }) + ) +} + +/** + * Records a resolved session as the request's auth kind. Stamped here, where + * every session is resolved, so the many routes that authenticate by calling + * `getSession` directly are attributed without each one remembering to. + */ +function recordSessionAuth(session: T): T { + if (session?.user?.id) setRequestAuth({ kind: 'session' }, { preserveExisting: true }) + return session } export const getSession = cache(getSessionImpl) diff --git a/apps/sim/lib/auth/hybrid.ts b/apps/sim/lib/auth/hybrid.ts index eac476540ca..3a5b18bb2c1 100644 --- a/apps/sim/lib/auth/hybrid.ts +++ b/apps/sim/lib/auth/hybrid.ts @@ -1,5 +1,5 @@ -import { describePrincipalAuth, type WorkflowExecutionPrincipal } from '@sim/auth/principal' -import { createLogger, setRequestAuth } from '@sim/logger' +import type { WorkflowExecutionPrincipal } from '@sim/auth/principal' +import { createLogger } from '@sim/logger' import type { NextRequest } from 'next/server' import { API_KEY_HEADER, BEARER_PREFIX } from '@/lib/api/server/credential-headers' import { authenticateApiKeyFromHeader, updateApiKeyLastUsed } from '@/lib/api-key/service' @@ -83,7 +83,7 @@ function resolveUserFromJwt( * @param options - Optional configuration * @param options.requireWorkflowId - Whether workflowId/userId is required (default: true) */ -async function resolveInternalAuth( +export async function checkInternalAuth( request: NextRequest, options: { requireWorkflowId?: boolean } = {} ): Promise { @@ -131,7 +131,7 @@ async function resolveInternalAuth( * @param options - Optional configuration * @param options.requireWorkflowId - Whether workflowId/userId is required for JWT (default: true) */ -async function resolveSessionOrInternalAuth( +export async function checkSessionOrInternalAuth( request: NextRequest, options: { requireWorkflowId?: boolean } = {} ): Promise { @@ -195,7 +195,7 @@ async function resolveSessionOrInternalAuth( * * For internal JWT calls, requires workflowId to determine user context */ -async function resolveHybridAuth( +export async function checkHybridAuth( request: NextRequest, options: { requireWorkflowId?: boolean } = {} ): Promise { @@ -277,36 +277,3 @@ async function resolveHybridAuth( } } } - -type AuthCheck = ( - request: NextRequest, - options?: { requireWorkflowId?: boolean } -) => Promise - -/** - * Records how a request authenticated on the request context, so the logs and - * analytics of a route that authenticates through these helpers rather than a - * route builder carry the same `auth` attribution. A principal describes - * itself; an internal JWT that produced none is recorded by its auth type. - */ -function recordingAuth(resolve: AuthCheck): AuthCheck { - return async (request, options) => { - const result = await resolve(request, options) - if (!result.success) return result - if (result.principal) { - setRequestAuth(describePrincipalAuth(result.principal)) - } else if (result.authType) { - setRequestAuth({ kind: result.authType }) - } - return result - } -} - -/** Internal JWT authentication only. See {@link resolveInternalAuth}. */ -export const checkInternalAuth = recordingAuth(resolveInternalAuth) - -/** Session or internal JWT authentication, never an API key. See {@link resolveSessionOrInternalAuth}. */ -export const checkSessionOrInternalAuth = recordingAuth(resolveSessionOrInternalAuth) - -/** Any of the three supported credentials. See {@link resolveHybridAuth}. */ -export const checkHybridAuth = recordingAuth(resolveHybridAuth) diff --git a/apps/sim/lib/auth/internal.test.ts b/apps/sim/lib/auth/internal.test.ts index 9bcbbd86f01..e971359a5fe 100644 --- a/apps/sim/lib/auth/internal.test.ts +++ b/apps/sim/lib/auth/internal.test.ts @@ -3,6 +3,7 @@ */ import { serializePrincipal } from '@sim/auth/principal' +import { setRequestAuth } from '@sim/logger' import { resetEnvMock } from '@sim/testing' import { decodeJwt, SignJWT } from 'jose' import { afterAll, describe, expect, it, vi } from 'vitest' @@ -40,6 +41,19 @@ describe('internal JWT claims', () => { }) }) + it('records a verified internal token as the request auth kind, and a refused one not at all', async () => { + vi.mocked(setRequestAuth).mockClear() + + await verifyInternalToken('not-a-jwt') + expect(vi.mocked(setRequestAuth)).not.toHaveBeenCalled() + + await verifyInternalToken(await generateInternalToken('user-1')) + expect(vi.mocked(setRequestAuth)).toHaveBeenCalledWith( + { kind: 'internal_jwt' }, + { preserveExisting: true } + ) + }) + it('rejects unknown sandbox profiles instead of falling back to another image', async () => { const token = await generateInternalToken('user-1', { sandboxProfile: 'unknown-profile' as never, diff --git a/apps/sim/lib/auth/internal.ts b/apps/sim/lib/auth/internal.ts index c0e54f2b5a5..ec2b1a38e19 100644 --- a/apps/sim/lib/auth/internal.ts +++ b/apps/sim/lib/auth/internal.ts @@ -5,7 +5,7 @@ import { type WorkflowExecutionAuthority, type WorkflowExecutionPrincipal, } from '@sim/auth/principal' -import { createLogger } from '@sim/logger' +import { createLogger, setRequestAuth } from '@sim/logger' import { safeCompare } from '@sim/security/compare' import { generateId } from '@sim/utils/id' import { type JWTPayload, jwtVerify, SignJWT } from 'jose' @@ -295,6 +295,7 @@ export async function verifyInternalToken( if (payload.sandboxProfile !== undefined && payload.sandboxProfile !== 'mothership') { return { valid: false } } + setRequestAuth({ kind: 'internal_jwt' }, { preserveExisting: true }) return { valid: true, userId: typeof payload.userId === 'string' ? payload.userId : undefined, diff --git a/apps/sim/lib/auth/oauth-access-token.test.ts b/apps/sim/lib/auth/oauth-access-token.test.ts index fc54b1abf6c..48d7bdb668c 100644 --- a/apps/sim/lib/auth/oauth-access-token.test.ts +++ b/apps/sim/lib/auth/oauth-access-token.test.ts @@ -1,6 +1,7 @@ /** * @vitest-environment node */ +import { setRequestAuth } from '@sim/logger' import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -85,12 +86,17 @@ describe('verifyOAuthAccessToken', () => { }) expect(dbChainMockFns.where).toHaveBeenCalledOnce() expect(JSON.stringify(dbChainMockFns.where.mock.calls[0])).toContain('hash:secret') + expect(vi.mocked(setRequestAuth)).toHaveBeenCalledWith( + { kind: 'oauth_access_token', clientId: 'sim-cli' }, + { preserveExisting: true } + ) }) it('refuses a credential that is not one of ours without a database read', async () => { expect(await reason('sim_abc')).toBe('malformed') expect(await reason('sim_oat_')).toBe('malformed') expect(dbChainMockFns.where).not.toHaveBeenCalled() + expect(vi.mocked(setRequestAuth)).not.toHaveBeenCalled() }) it('refuses an unknown, expired, disabled-client, orphaned, or banned token', async () => { diff --git a/apps/sim/lib/auth/oauth-access-token.ts b/apps/sim/lib/auth/oauth-access-token.ts index 584bf617325..87441af091d 100644 --- a/apps/sim/lib/auth/oauth-access-token.ts +++ b/apps/sim/lib/auth/oauth-access-token.ts @@ -1,7 +1,7 @@ import type { OAuthAccessTokenPrincipal } from '@sim/auth/principal' import { db } from '@sim/db' import { oauthAccessToken, oauthClient, user } from '@sim/db/schema' -import { createLogger } from '@sim/logger' +import { createLogger, setRequestAuth } from '@sim/logger' import { sha256Hex } from '@sim/security/hash' import { eq } from 'drizzle-orm' import { isAccountBlocked } from '@/lib/auth/ban' @@ -137,6 +137,7 @@ export async function verifyOAuthAccessToken( } logger.debug('Authenticated OAuth access token', { tokenId: row.id, clientId: row.clientId }) + setRequestAuth({ kind: 'oauth_access_token', clientId: row.clientId }, { preserveExisting: true }) return { kind: 'oauth_access_token', userId: row.userId, diff --git a/apps/sim/lib/core/utils/request-attribution.test.ts b/apps/sim/lib/core/utils/request-attribution.test.ts new file mode 100644 index 00000000000..d875a96da14 --- /dev/null +++ b/apps/sim/lib/core/utils/request-attribution.test.ts @@ -0,0 +1,43 @@ +/** + * @vitest-environment node + */ +import { loggerMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { captureRequestAttribution } from '@/lib/core/utils/request-attribution' + +describe('captureRequestAttribution', () => { + beforeEach(() => { + vi.mocked(loggerMock.getRequestContext).mockReset() + }) + + it('carries the client and auth of the request that queues the work', () => { + vi.mocked(loggerMock.getRequestContext).mockReturnValue({ + requestId: 'req-1', + path: '/api/workflows/wf-1/execute', + client: { surface: 'cli', version: '2.1.2', agent: 'none', source: 'header' }, + auth: { kind: 'oauth_access_token', clientId: 'sim-cli' }, + }) + + expect(captureRequestAttribution()).toEqual({ + client: { surface: 'cli', version: '2.1.2', agent: 'none', source: 'header' }, + auth: { kind: 'oauth_access_token', clientId: 'sim-cli' }, + }) + }) + + it('omits what the request did not establish', () => { + vi.mocked(loggerMock.getRequestContext).mockReturnValue({ + requestId: 'req-1', + client: { surface: 'api', source: 'credential' }, + }) + + expect(captureRequestAttribution()).toEqual({ + client: { surface: 'api', source: 'credential' }, + }) + }) + + it('carries nothing outside a request', () => { + vi.mocked(loggerMock.getRequestContext).mockReturnValue(undefined) + + expect(captureRequestAttribution()).toBeUndefined() + }) +}) diff --git a/apps/sim/lib/core/utils/request-attribution.ts b/apps/sim/lib/core/utils/request-attribution.ts new file mode 100644 index 00000000000..3efb1da2fd8 --- /dev/null +++ b/apps/sim/lib/core/utils/request-attribution.ts @@ -0,0 +1,29 @@ +import { getRequestContext, type RequestAuth } from '@sim/logger' +import type { ResolvedClientInfo } from '@sim/utils/client-info' + +/** + * Who started a piece of work: the client a request came from and the + * credential it authenticated with. The request context holds these only in + * memory, so work that leaves the request — a queued job, possibly on another + * machine — would lose them. Carrying this snapshot on the job and restoring it + * into the job's own context keeps its logs and analytics attributed to the + * request that queued it, the way OpenTelemetry baggage rides a message. + */ +export interface RequestAttribution { + client?: ResolvedClientInfo + auth?: RequestAuth +} + +/** + * The current request's attribution, for a job it is about to queue. Returns + * `undefined` outside a request, so a job queued by a trigger carries nothing + * and is attributed by its trigger instead. + */ +export function captureRequestAttribution(): RequestAttribution | undefined { + const context = getRequestContext() + if (!context?.client && !context?.auth) return undefined + return { + ...(context.client ? { client: context.client } : {}), + ...(context.auth ? { auth: context.auth } : {}), + } +} diff --git a/apps/sim/lib/posthog/server.test.ts b/apps/sim/lib/posthog/server.test.ts index 169f2e0c713..286310328d6 100644 --- a/apps/sim/lib/posthog/server.test.ts +++ b/apps/sim/lib/posthog/server.test.ts @@ -58,6 +58,24 @@ describe('captureServerEvent', () => { ) }) + it('stamps the user agent product of an undeclared client', () => { + vi.mocked(loggerMock.getRequestContext).mockReturnValue({ + requestId: 'req-1', + client: { surface: 'api', source: 'credential', name: 'python-requests' }, + }) + + captureServerEvent('user-1', 'workflow_deployed', { + workflow_id: 'workflow-1', + workspace_id: 'workspace-1', + }) + + expect(captureSpy).toHaveBeenCalledWith( + expect.objectContaining({ + properties: expect.objectContaining({ surface: 'api', client_name: 'python-requests' }), + }) + ) + }) + it('stamps the request, its authentication, and the workflow call chain', () => { vi.mocked(loggerMock.getRequestContext).mockReturnValue({ requestId: 'req-1', diff --git a/apps/sim/lib/posthog/server.ts b/apps/sim/lib/posthog/server.ts index 022c760990d..a9cb0603907 100644 --- a/apps/sim/lib/posthog/server.ts +++ b/apps/sim/lib/posthog/server.ts @@ -78,6 +78,7 @@ function contextProperties(explicit: Record): Record ({ mockEnqueue: vi.fn() })) + +vi.mock('@/lib/core/async-jobs', () => ({ + getJobQueue: vi.fn(async () => ({ enqueue: mockEnqueue })), + shouldExecuteInline: vi.fn(() => false), +})) +vi.mock('@/lib/billing/calculations/usage-reservation', () => ({ + releaseExecutionSlot: vi.fn(), +})) +vi.mock('@/background/workflow-execution', () => ({ executeWorkflowJob: vi.fn() })) + +import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' +import { enqueueWorkflowExecution } from '@/lib/workflows/executor/enqueue-execution' + +const params = { + requestId: 'req-1', + workflowId: 'wf-1', + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + userId: 'user-1', + billingAttribution: {} as BillingAttributionSnapshot, + workspaceId: 'workspace-1', + input: {}, + triggerType: 'api', + executionId: 'exec-1', + executionTimeoutMs: 60_000, +} as const + +describe('enqueueWorkflowExecution', () => { + beforeEach(() => { + vi.clearAllMocks() + mockEnqueue.mockResolvedValue('job-1') + }) + + it('carries the queuing request attribution on the job payload', async () => { + vi.mocked(loggerMock.getRequestContext).mockReturnValue({ + requestId: 'req-1', + client: { surface: 'cli', version: '2.1.2', source: 'header' }, + auth: { kind: 'personal_api_key' }, + }) + + await enqueueWorkflowExecution(params) + + expect(mockEnqueue).toHaveBeenCalledWith( + 'workflow-execution', + expect.objectContaining({ + attribution: { + client: { surface: 'cli', version: '2.1.2', source: 'header' }, + auth: { kind: 'personal_api_key' }, + }, + }), + expect.anything() + ) + }) +}) diff --git a/apps/sim/lib/workflows/executor/enqueue-execution.ts b/apps/sim/lib/workflows/executor/enqueue-execution.ts index be3200d8ab6..0136fcbe08b 100644 --- a/apps/sim/lib/workflows/executor/enqueue-execution.ts +++ b/apps/sim/lib/workflows/executor/enqueue-execution.ts @@ -6,6 +6,7 @@ import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attr import { getJobQueue, shouldExecuteInline } from '@/lib/core/async-jobs' import { isAsyncJobEnqueueError } from '@/lib/core/async-jobs/types' import { toTriggerMaxDurationSeconds } from '@/lib/core/execution-limits' +import { captureRequestAttribution } from '@/lib/core/utils/request-attribution' import { WORKFLOW_EXECUTION_JOB_ID_PREFIX } from '@/lib/workflows/executor/execution-job-ids' import { executeWorkflowJob, type WorkflowExecutionPayload } from '@/background/workflow-execution' import type { ResolvedSecretTraceProvenanceV1 } from '@/executor/utils/resolved-secret-trace-registry' @@ -116,6 +117,7 @@ export async function enqueueWorkflowExecution( requestId, correlation, callChain, + attribution: captureRequestAttribution(), enforceCredentialAccess, isPublicApiAccess, executionMode: 'async', diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index 728153d9b0d..a60099b86ba 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -318,6 +318,7 @@ const requestContextMetadata = (context: RequestContext): LoggerMetadata => { if (context.client) { metadata.surface = context.client.surface if (context.client.version) metadata.clientVersion = context.client.version + if (context.client.name) metadata.clientName = context.client.name if (context.client.agent) metadata.codingAgent = context.client.agent } if (context.auth) { @@ -543,7 +544,7 @@ export function createLogger(module: string, config?: LoggerConfig): Logger { return new Logger(module, config) } -export type { RequestAuth, RequestContext } from './request-context' +export type { RequestAuth, RequestContext, SetRequestAuthOptions } from './request-context' export { getRequestContext, runWithRequestContext, diff --git a/packages/logger/src/request-context.test.ts b/packages/logger/src/request-context.test.ts new file mode 100644 index 00000000000..48f145c7316 --- /dev/null +++ b/packages/logger/src/request-context.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import { getRequestContext, runWithRequestContext, setRequestAuth } from './request-context' + +describe('setRequestAuth', () => { + it('replaces the recorded auth by default', () => { + runWithRequestContext({ requestId: 'req-1' }, () => { + setRequestAuth({ kind: 'internal_jwt' }) + setRequestAuth({ kind: 'delegated', service: 'executor' }) + + expect(getRequestContext()?.auth).toEqual({ kind: 'delegated', service: 'executor' }) + }) + }) + + it('keeps an auth already recorded when asked to preserve it', () => { + runWithRequestContext({ requestId: 'req-1' }, () => { + setRequestAuth({ kind: 'delegated', service: 'executor' }) + setRequestAuth({ kind: 'session' }, { preserveExisting: true }) + + expect(getRequestContext()?.auth).toEqual({ kind: 'delegated', service: 'executor' }) + }) + }) + + it('fills an empty slot even when asked to preserve', () => { + runWithRequestContext({ requestId: 'req-1' }, () => { + setRequestAuth({ kind: 'session' }, { preserveExisting: true }) + + expect(getRequestContext()?.auth).toEqual({ kind: 'session' }) + }) + }) + + it('attributes a client that did not identify itself by the credential', () => { + runWithRequestContext( + { requestId: 'req-1', client: { surface: 'unknown', source: 'unidentified', name: 'curl' } }, + () => { + setRequestAuth({ kind: 'personal_api_key' }, { preserveExisting: true }) + + expect(getRequestContext()?.client).toEqual({ + surface: 'api', + source: 'credential', + name: 'curl', + }) + } + ) + }) + + it('leaves a client that identified itself as it declared', () => { + runWithRequestContext( + { requestId: 'req-1', client: { surface: 'cli', version: '2.1.2', source: 'header' } }, + () => { + setRequestAuth({ kind: 'oauth_access_token', clientId: 'sim-cli' }) + + expect(getRequestContext()?.client?.surface).toBe('cli') + } + ) + }) + + it('does nothing outside a request', () => { + expect(() => setRequestAuth({ kind: 'session' })).not.toThrow() + expect(getRequestContext()).toBeUndefined() + }) +}) diff --git a/packages/logger/src/request-context.ts b/packages/logger/src/request-context.ts index ae07c2fd16b..918ab3460a4 100644 --- a/packages/logger/src/request-context.ts +++ b/packages/logger/src/request-context.ts @@ -1,4 +1,4 @@ -import type { ResolvedClientInfo } from '@sim/utils/client-info' +import { attributeUndeclaredClient, type ResolvedClientInfo } from '@sim/utils/client-info' export interface RequestContext { requestId: string @@ -98,14 +98,28 @@ export function setRequestTraceId(traceId: string): void { if (store && traceId) store.traceId = traceId } +export interface SetRequestAuthOptions { + /** + * Keep an auth kind already recorded. Credential verifiers pass this: they + * know only the credential, and must not replace the principal a route + * builder described from it (an internal JWT that turned out to carry a + * delegated principal), nor be replaced by a later incidental check. + */ + preserveExisting?: boolean +} + /** * Records how the current request authenticated so every later log line and * analytics event in it can say so. Authentication runs inside the handler, * after the route context exists, hence a mutation of the live store rather - * than a field supplied at `runWithRequestContext` time. No-op outside a - * request context. + * than a field supplied at `runWithRequestContext` time. A client that did not + * identify itself is attributed by the credential here, the first point the + * request's origin is known. No-op outside a request context. */ -export function setRequestAuth(auth: RequestAuth): void { +export function setRequestAuth(auth: RequestAuth, options: SetRequestAuthOptions = {}): void { const store = storage.getStore() - if (store) store.auth = auth + if (!store) return + if (options.preserveExisting && store.auth) return + store.auth = auth + if (store.client) store.client = attributeUndeclaredClient(store.client, auth.kind) } diff --git a/packages/sim-cli/src/telemetry/client-info.test.ts b/packages/sim-cli/src/telemetry/client-info.test.ts index c61ef331c8b..e00b4ceb358 100644 --- a/packages/sim-cli/src/telemetry/client-info.test.ts +++ b/packages/sim-cli/src/telemetry/client-info.test.ts @@ -24,6 +24,10 @@ describe('clientInfoHeader', () => { ) }) + it('reports that no agent was detected rather than leaving it out', () => { + expect(clientInfoHeader({})).toContain('; agent/none') + }) + it('withholds the agent when usage reporting is opted out', () => { const header = clientInfoHeader({ CLAUDECODE: '1', DO_NOT_TRACK: '1' }) expect(header).not.toContain('agent/') diff --git a/packages/sim-cli/src/telemetry/client-info.ts b/packages/sim-cli/src/telemetry/client-info.ts index 996a8b0dd7e..342659a7b98 100644 --- a/packages/sim-cli/src/telemetry/client-info.ts +++ b/packages/sim-cli/src/telemetry/client-info.ts @@ -1,6 +1,6 @@ import { CLIENT_INFO_HEADER, formatClientInfo } from '@sim/utils/client-info' import { CLI_VERSION, USER_AGENT } from '../version' -import { detectCodingAgent } from './coding-agent' +import { detectCodingAgent, NO_CODING_AGENT } from './coding-agent' import { telemetryStatus } from './policy' import { loadTelemetryState } from './state' @@ -10,7 +10,7 @@ let cached: string | undefined /** * The `X-Sim-Client-Info` value: the same facts as the user agent, in the * header every official client sends, plus the AI coding agent driving this - * shell when one can be detected. The server reads this header, not the user + * shell (`none` when none is detected). The server reads this header, not the user * agent, so a request from the CLI is attributed to the CLI on every log line * and analytics event it produces. * @@ -42,7 +42,7 @@ function buildClientInfoHeader(env: NodeJS.ProcessEnv): string { runtime: { name: 'node', version: process.versions.node }, os: process.platform, arch: process.arch, - ...(reportingAllowed(env) ? { agent: detectCodingAgent(env) } : {}), + ...(reportingAllowed(env) ? { agent: detectCodingAgent(env) ?? NO_CODING_AGENT } : {}), }) } diff --git a/packages/sim-cli/src/telemetry/coding-agent.ts b/packages/sim-cli/src/telemetry/coding-agent.ts index 79605e9097f..9e12e32e15e 100644 --- a/packages/sim-cli/src/telemetry/coding-agent.ts +++ b/packages/sim-cli/src/telemetry/coding-agent.ts @@ -81,6 +81,13 @@ function declaredAgentName(value: string | undefined): string | undefined { return VERSIONED_DECLARATION.exec(trimmed)?.[1] ?? trimmed } +/** + * What the CLI reports when no agent is detected: a person at a terminal, or a + * script. Sent explicitly so that an absent value means only that the client + * did not report one — an older release, or reporting turned off. + */ +export const NO_CODING_AGENT = 'none' + /** * The agent driving this shell, or `undefined` for a person at a terminal. * diff --git a/packages/sim-cli/src/telemetry/invocation.test.ts b/packages/sim-cli/src/telemetry/invocation.test.ts index 81a803f31ba..56672a1b667 100644 --- a/packages/sim-cli/src/telemetry/invocation.test.ts +++ b/packages/sim-cli/src/telemetry/invocation.test.ts @@ -209,6 +209,15 @@ describe('command telemetry', () => { expect(sentBy(send).properties.coding_agent).toBe('claude-code') }) + it('reports none when no coding agent drives the shell', async () => { + const { telemetry, program, send } = harness() + + await run(program, ['workflows', 'list']) + telemetry.complete({ exitCode: 0 }) + + expect(sentBy(send).properties.coding_agent).toBe('none') + }) + it('shows the first-run notice on a terminal and does not report that run', async () => { const { telemetry, program, send, write } = harness({ stderrIsTty: true }) diff --git a/packages/sim-cli/src/telemetry/invocation.ts b/packages/sim-cli/src/telemetry/invocation.ts index 1370cf73aa7..de11504143c 100644 --- a/packages/sim-cli/src/telemetry/invocation.ts +++ b/packages/sim-cli/src/telemetry/invocation.ts @@ -3,7 +3,7 @@ import { profileFrom } from '../context' import { isCi } from '../environment' import { SimApiError } from '../http/client' import { CLI_VERSION } from '../version' -import { detectCodingAgent } from './coding-agent' +import { detectCodingAgent, NO_CODING_AGENT } from './coding-agent' import { telemetryStatus } from './policy' import { loadTelemetryState, nextSession, type TelemetryState, writeTelemetryState } from './state' import { @@ -86,8 +86,8 @@ export interface CommandEventProperties { /** Whether stdout was a terminal, which separates people from scripts. */ is_tty: boolean is_ci: boolean - /** The AI coding agent driving this shell, when one could be detected. */ - coding_agent?: string + /** The AI coding agent driving this shell, or `none` when none was detected. */ + coding_agent: string /** Whether the profile targets Sim's hosted deployment or a self-hosted one; never the address. */ endpoint_kind?: 'hosted' | 'self_hosted' } @@ -262,11 +262,10 @@ export function createCommandTelemetry(options: CommandTelemetryOptions = {}): C arch: process.arch, is_tty: stdoutIsTty, is_ci: isCi(env), + coding_agent: detectCodingAgent(env) ?? NO_CODING_AGENT, } const kind = endpointKind(invocation.action) if (kind) properties.endpoint_kind = kind - const agent = detectCodingAgent(env) - if (agent) properties.coding_agent = agent send(target, { api_key: target.key, diff --git a/packages/utils/src/client-info.test.ts b/packages/utils/src/client-info.test.ts index 516d2b778e5..df4c71a2087 100644 --- a/packages/utils/src/client-info.test.ts +++ b/packages/utils/src/client-info.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + attributeUndeclaredClient, CLIENT_INFO_HEADER, formatClientInfo, parseClientInfo, @@ -121,15 +122,76 @@ describe('resolveClientInfo', () => { expect(resolved).toEqual({ surface: 'web', source: 'fetch_metadata' }) }) - it('leaves a credentialed browser request unattributed', () => { + it('leaves a credentialed browser request to be attributed by its credential', () => { expect( - resolveClientInfo(headers({ 'sec-fetch-mode': 'cors' }), { hasExternalCredentials: true }) - ).toBeUndefined() + resolveClientInfo( + headers({ + 'sec-fetch-mode': 'cors', + 'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36', + }), + { hasExternalCredentials: true } + ) + ).toEqual({ surface: 'unknown', source: 'unidentified', name: 'browser' }) }) - it('leaves a bare request unattributed', () => { + it('names an undeclared client by its user agent product', () => { expect( - resolveClientInfo(headers({ 'user-agent': 'curl/8.0.0' }), { hasExternalCredentials: true }) - ).toBeUndefined() + resolveClientInfo(headers({ 'user-agent': 'python-requests/2.32.3' }), { + hasExternalCredentials: true, + }) + ).toEqual({ surface: 'unknown', source: 'unidentified', name: 'python-requests' }) + expect( + resolveClientInfo(headers({ 'user-agent': 'Go-http-client/1.1' }), { + hasExternalCredentials: false, + }) + ).toEqual({ surface: 'unknown', source: 'unidentified', name: 'go-http-client' }) + }) + + it('omits the name when there is no readable user agent', () => { + expect(resolveClientInfo(headers({}), { hasExternalCredentials: false })).toEqual({ + surface: 'unknown', + source: 'unidentified', + }) + expect( + resolveClientInfo(headers({ 'user-agent': '(bad)' }), { hasExternalCredentials: true }) + ).toEqual({ surface: 'unknown', source: 'unidentified' }) + }) +}) + +describe('attributeUndeclaredClient', () => { + const undeclared = { surface: 'unknown', source: 'unidentified', name: 'curl' } as const + + it('attributes a customer credential to the API', () => { + for (const kind of ['personal_api_key', 'workspace_api_key', 'oauth_access_token']) { + expect(attributeUndeclaredClient(undeclared, kind)).toEqual({ + surface: 'api', + source: 'credential', + name: 'curl', + }) + } + }) + + it("attributes Sim's own service credentials to internal traffic", () => { + for (const kind of ['internal_jwt', 'delegated', 'organization_delegated', 'system']) { + expect(attributeUndeclaredClient(undeclared, kind).surface).toBe('internal') + } + }) + + it('keeps a session without browser headers unknown', () => { + expect(attributeUndeclaredClient(undeclared, 'session')).toEqual(undeclared) + }) + + it('re-attributes when a more specific principal replaces the credential', () => { + const asApi = attributeUndeclaredClient(undeclared, 'personal_api_key') + expect(attributeUndeclaredClient(asApi, 'delegated').surface).toBe('internal') + }) + + it('never changes a client that identified itself or a trigger that started a run', () => { + const cli = { surface: 'cli', version: '2.1.2', source: 'header' } as const + const web = { surface: 'web', source: 'fetch_metadata' } as const + const schedule = { surface: 'schedule', source: 'trigger' } as const + expect(attributeUndeclaredClient(cli, 'personal_api_key')).toBe(cli) + expect(attributeUndeclaredClient(web, 'session')).toBe(web) + expect(attributeUndeclaredClient(schedule, 'system')).toBe(schedule) }) }) diff --git a/packages/utils/src/client-info.ts b/packages/utils/src/client-info.ts index 09537a4fafa..cc990d9e5cc 100644 --- a/packages/utils/src/client-info.ts +++ b/packages/utils/src/client-info.ts @@ -51,11 +51,42 @@ export interface ClientInfo { agent?: string } +/** + * The surfaces the server assigns to work no official client declared, so every + * request and run is attributed to something rather than left blank: + * + * - `api`: direct API traffic, authenticated by a customer's credential. + * - `internal`: Sim's own services calling each other, authenticated by a + * service credential. + * - `webhook`, `schedule`: a run a trigger started rather than a client. + * - `unknown`: none of the above, such as an unauthenticated webhook delivery. + */ +export const UNDECLARED_SURFACES = ['api', 'internal', 'webhook', 'schedule', 'unknown'] as const + +export type UndeclaredSurface = (typeof UNDECLARED_SURFACES)[number] + +/** Every surface a request or run can be attributed to. */ +export type RequestSurface = SimSurface | UndeclaredSurface + /** How the server established a request's client. */ -export type ClientInfoSource = 'header' | 'user_agent' | 'fetch_metadata' +export type ClientInfoSource = + | 'header' + | 'user_agent' + | 'fetch_metadata' + | 'credential' + | 'trigger' + | 'unidentified' -export interface ResolvedClientInfo extends ClientInfo { +export interface ResolvedClientInfo extends Omit { + surface: RequestSurface source: ClientInfoSource + /** + * The product an undeclared client names first in its `User-Agent` + * (`python-requests`, `curl`, `node`, or `browser` for any browser), in the + * sense of OpenTelemetry's `user_agent.name`. Present only when the client + * did not declare itself, since a declared client already says what it is. + */ + name?: string } /** Bounds a caller-controlled header before it is parsed or logged. */ @@ -75,6 +106,12 @@ const LEGACY_CLI_USER_AGENT = /^sim-cli\/([A-Za-z0-9._+-]+)/ /** The header browsers attach to every request and non-browser clients never do. */ const FETCH_METADATA_HEADER = 'sec-fetch-mode' +/** The leading product name of a `User-Agent`, bounded so a hostile value stays cheap to read. */ +const USER_AGENT_PRODUCT = /^([A-Za-z0-9._+-]{1,64})(?:[/\s]|$)/ + +/** The product every browser, and many libraries imitating one, names first. */ +const BROWSER_PRODUCT = 'mozilla' + const SURFACE_SET: ReadonlySet = new Set(SIM_SURFACES) function isSurface(value: string): value is SimSurface { @@ -179,26 +216,70 @@ export interface ResolveClientInfoOptions { * else does, so a browser request that carries no external credentials can * only have come from a page Sim served — the web app, or a public surface * such as a shared chat. A browser request that does carry an API key is a - * third-party integration and is deliberately left unattributed. The web app + * third-party integration, and falls through to the credential. The web app * declares itself on its contract-bound calls; this covers the raw-`fetch` * exceptions and stale bundles that do not. * - * Returns `undefined` when none of these apply: direct API traffic from an - * unofficial client, or a server-to-server call. + * 4. Anything else is `unknown` until it authenticates, when + * {@link attributeUndeclaredClient} refines it by the credential it used. + * Headers alone cannot separate a customer's API key from one of Sim's own + * services, so the refinement waits for the credential to verify. + * + * An undeclared client records its `User-Agent` product name, so it still says + * which library or tool sent it. */ export function resolveClientInfo( headers: HeaderReader, options: ResolveClientInfoOptions -): ResolvedClientInfo | undefined { +): ResolvedClientInfo { const declared = parseClientInfo(headers.get(CLIENT_INFO_HEADER)) if (declared) return { ...declared, source: 'header' } - const legacyCli = LEGACY_CLI_USER_AGENT.exec(headers.get('user-agent') ?? '') + const userAgent = headers.get('user-agent') ?? '' + const legacyCli = LEGACY_CLI_USER_AGENT.exec(userAgent) if (legacyCli) return { surface: 'cli', version: legacyCli[1], source: 'user_agent' } if (headers.get(FETCH_METADATA_HEADER) !== null && !options.hasExternalCredentials) { return { surface: 'web', source: 'fetch_metadata' } } - return undefined + const name = userAgentName(userAgent) + return { surface: 'unknown', source: 'unidentified', ...(name ? { name } : {}) } +} + +/** Auth kinds only Sim's own services hold: the executor, Copilot, and system jobs. */ +const INTERNAL_AUTH_KINDS: ReadonlySet = new Set([ + 'internal_jwt', + 'delegated', + 'organization_delegated', + 'system', +]) + +/** + * Refines an undeclared client by the credential its request authenticated + * with: a customer's credential (an API key, an OAuth token, a SCIM or Slack + * connection) makes it `api`, a Sim service credential makes it `internal`. A + * session stays `unknown`, since a cookie without browser headers says nothing + * about the client. A client that identified itself is returned unchanged, and + * so is one a trigger started. + */ +export function attributeUndeclaredClient( + client: ResolvedClientInfo, + authKind: string +): ResolvedClientInfo { + if (client.source !== 'unidentified' && client.source !== 'credential') return client + if (authKind === 'session') return { ...client, surface: 'unknown', source: 'unidentified' } + const surface = INTERNAL_AUTH_KINDS.has(authKind) ? 'internal' : 'api' + return { ...client, surface, source: 'credential' } +} + +/** + * The first product a `User-Agent` names, lowercased so one library groups as + * one value, with every browser collapsed to `browser` since they all claim to + * be Mozilla. + */ +function userAgentName(userAgent: string): string | undefined { + const product = USER_AGENT_PRODUCT.exec(userAgent)?.[1]?.toLowerCase() + if (!product) return undefined + return product === BROWSER_PRODUCT ? 'browser' : product } From 05218219748c81ee7fce1da69181128fda692d86 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 14 Sep 2026 18:14:25 -0700 Subject: [PATCH 2/2] fix(cli): match the GitHub CLI's coding agent detection table --- .../src/telemetry/coding-agent.test.ts | 43 +++++++++--- .../sim-cli/src/telemetry/coding-agent.ts | 69 ++++++++----------- 2 files changed, 60 insertions(+), 52 deletions(-) diff --git a/packages/sim-cli/src/telemetry/coding-agent.test.ts b/packages/sim-cli/src/telemetry/coding-agent.test.ts index 4312ab8df83..0c12bdbf57e 100644 --- a/packages/sim-cli/src/telemetry/coding-agent.test.ts +++ b/packages/sim-cli/src/telemetry/coding-agent.test.ts @@ -12,21 +12,35 @@ describe('detectCodingAgent', () => { [{ CLAUDECODE: '1', CLAUDE_CODE_IS_COWORK: '1' }, 'cowork'], [{ CODEX_THREAD_ID: 'thr_1' }, 'codex'], [{ CODEX_SANDBOX: 'seatbelt' }, 'codex'], + [{ CODEX_CI: '1' }, 'codex'], [{ GEMINI_CLI: '1' }, 'gemini-cli'], - [{ CURSOR_AGENT: '1' }, 'cursor'], + [{ COPILOT_CLI: '1' }, 'copilot-cli'], + [{ OPENCODE: '1' }, 'opencode'], + [{ ANTIGRAVITY_AGENT: '1' }, 'antigravity'], + [{ AUGMENT_AGENT: '1' }, 'augment-cli'], [{ CURSOR_TRACE_ID: 'abc' }, 'cursor'], - [{ CURSOR_EXTENSION_HOST_ROLE: 'agent-exec' }, 'cursor'], - [{ OPENCODE: '1', AGENT: '1' }, 'opencode'], - [{ CLINE_ACTIVE: 'true' }, 'cline'], - [{ OZ_RUN_ID: 'run_1' }, 'warp'], - [{ PI_CODING_AGENT: 'true' }, 'pi'], + [{ CURSOR_AGENT: '1' }, 'cursor-cli'], + [{ CURSOR_EXTENSION_HOST_ROLE: 'agent-exec' }, 'cursor-cli'], ])('recognises %o as %s', (env, expected) => { expect(detectCodingAgent(env)).toBe(expected) }) it('names Amp rather than the Claude Code marker it also sets', () => { expect(detectCodingAgent({ AGENT: 'amp', CLAUDECODE: '1' })).toBe('amp') - expect(detectCodingAgent({ AMP_CURRENT_THREAD_ID: 'T-1', CLAUDECODE: '1' })).toBe('amp') + }) + + it('names the Cursor IDE over the Cursor CLI signal', () => { + expect(detectCodingAgent({ CURSOR_TRACE_ID: 'abc', CURSOR_AGENT: '1' })).toBe('cursor') + }) + + it('reads the declaration Claude Code sets on its shells as claude-code', () => { + expect( + detectCodingAgent({ + AI_AGENT: 'claude-code_2-1-270_agent', + CLAUDECODE: '1', + CLAUDE_CODE_ENTRYPOINT: 'cli', + }) + ).toBe('claude-code') }) it('lets an agent declare its own name over every vendor marker', () => { @@ -52,10 +66,17 @@ describe('detectCodingAgent', () => { expect(detectCodingAgent({ AI_AGENT: 'x'.repeat(65) })).toBeUndefined() }) - it('ignores markers that only mean an agent is installed', () => { - expect(detectCodingAgent({ REPL_ID: 'abc', GOOSE_PROVIDER: 'x', AIDER_API_KEY: 'k' })).toBe( - undefined - ) + it('ignores signals that describe where a person works rather than an agent driving it', () => { + expect( + detectCodingAgent({ + REPL_ID: 'abc', + GOOSE_PROVIDER: 'x', + TERM_PROGRAM: 'kiro', + PATH: '/home/me/.pi/agent/bin:/usr/bin', + AGENT: 'goose', + OPENCODE_CLIENT: 'vscode', + }) + ).toBeUndefined() }) it('ignores a cursor role that is not the agent executor', () => { diff --git a/packages/sim-cli/src/telemetry/coding-agent.ts b/packages/sim-cli/src/telemetry/coding-agent.ts index 9e12e32e15e..46c3ceaf9db 100644 --- a/packages/sim-cli/src/telemetry/coding-agent.ts +++ b/packages/sim-cli/src/telemetry/coding-agent.ts @@ -3,19 +3,21 @@ * * Agents mark the shells they spawn with an environment variable, and the CLI * reports that mark so usage driven by an agent can be told apart from a person - * at a terminal. The checks, their order, and the names follow the GitHub CLI - * (`internal/agents/detect.go`), which is the most complete verified table: - * generic conventions first, then vendor markers, with the more specific - * marker ahead of a broader one it implies (Amp sets `CLAUDECODE` too; Cowork - * is Claude Code plus its own flag). + * at a terminal. The checks, their order, and the names follow the GitHub CLI's + * `internal/agents/detect.go` exactly: the generic `AI_AGENT` declaration + * first, then vendor markers, with a more specific agent ahead of a broader + * marker it also sets (Amp and Cowork both set `CLAUDECODE`). * - * Only markers an agent sets on the shells it drives are consulted. Variables - * that merely mean an agent is installed or configured — `REPL_ID`, - * `GOOSE_PROVIDER`, `AIDER_*`, `COPILOT_*` — are deliberately absent, because - * they would attribute a person's own command to an agent. + * Four of that table's signals are deliberately left out, each one the GitHub + * CLI itself marks as low confidence: `REPL_ID` (present in every Replit + * environment), `GOOSE_PROVIDER` (Goose is merely configured), + * `TERM_PROGRAM=kiro` (Kiro's terminal, which a person uses too), and a + * `.pi/agent` entry on `PATH`. Each describes where a person is working rather + * than an agent driving the command, so reporting it would attribute that + * person's own commands to an agent. */ -/** The value an agent may declare itself with under the generic conventions. */ +/** The value an agent may declare itself with under `AI_AGENT`. */ const AGENT_NAME_PATTERN = /^[a-z0-9_-]+$/i const MAX_AGENT_NAME_LENGTH = 64 @@ -29,34 +31,25 @@ const anyOf = (env: NodeJS.ProcessEnv) => variables.some((variable) => Boolean(env[variable])) -/** Vendor markers, most specific first. */ +/** Vendor markers, in the GitHub CLI's order. */ const AGENT_MARKERS: readonly AgentMarker[] = [ - { name: 'amp', matches: (env) => env.AGENT === 'amp' || Boolean(env.AMP_CURRENT_THREAD_ID) }, - { - name: 'codex', - matches: anyOf( - 'CODEX_THREAD_ID', - 'CODEX_SANDBOX', - 'CODEX_CI', - 'CODEX_SANDBOX_NETWORK_DISABLED' - ), - }, + { name: 'amp', matches: (env) => env.AGENT === 'amp' }, + /** Set by Codex on the commands it runs (`codex-rs/core`: `spawn.rs`, `exec_env.rs`, `unified_exec`). */ + { name: 'codex', matches: anyOf('CODEX_SANDBOX', 'CODEX_CI', 'CODEX_THREAD_ID') }, { name: 'gemini-cli', matches: anyOf('GEMINI_CLI') }, + { name: 'copilot-cli', matches: anyOf('COPILOT_CLI') }, + /** Not `OPENCODE_CALLER` or `OPENCODE_CLIENT`, which name what launched OpenCode. */ { name: 'opencode', matches: anyOf('OPENCODE') }, { name: 'antigravity', matches: anyOf('ANTIGRAVITY_AGENT') }, - { name: 'augment', matches: anyOf('AUGMENT_AGENT') }, - { name: 'cline', matches: anyOf('CLINE_ACTIVE') }, + { name: 'augment-cli', matches: anyOf('AUGMENT_AGENT') }, { name: 'cowork', matches: anyOf('CLAUDE_CODE_IS_COWORK') }, + /** `CLAUDECODE` is documented in Claude Code's environment variable reference. */ { name: 'claude-code', matches: anyOf('CLAUDECODE', 'CLAUDE_CODE') }, + { name: 'cursor', matches: anyOf('CURSOR_TRACE_ID') }, { - name: 'cursor', - matches: (env) => - anyOf('CURSOR_AGENT', 'CURSOR_TRACE_ID')(env) || - env.CURSOR_EXTENSION_HOST_ROLE === 'agent-exec', + name: 'cursor-cli', + matches: (env) => Boolean(env.CURSOR_AGENT) || env.CURSOR_EXTENSION_HOST_ROLE === 'agent-exec', }, - { name: 'warp', matches: anyOf('OZ_RUN_ID') }, - { name: 'pi', matches: anyOf('PI_CODING_AGENT') }, - { name: 'crush', matches: anyOf('CRUSH') }, ] /** @@ -91,17 +84,11 @@ export const NO_CODING_AGENT = 'none' /** * The agent driving this shell, or `undefined` for a person at a terminal. * - * `AI_AGENT` and `AGENT` are the two generic conventions agents have converged - * on for naming themselves and win over vendor markers when set. `AGENT` is - * consulted only when it carries a name: OpenCode sets it to `1`, which names - * nothing, and its own marker handles it. + * `AI_AGENT` is the generic convention agents use to name themselves, and wins + * over every vendor marker when it holds a well-formed name. */ export function detectCodingAgent(env: NodeJS.ProcessEnv = process.env): string | undefined { - const declared = declaredAgentName(env.AI_AGENT) - if (declared) return declared - - const generic = declaredAgentName(env.AGENT) - if (generic && generic !== '1') return generic - - return AGENT_MARKERS.find((marker) => marker.matches(env))?.name + return ( + declaredAgentName(env.AI_AGENT) ?? AGENT_MARKERS.find((marker) => marker.matches(env))?.name + ) }