Add AI agent detection to CLI analytics

[amcaplan]

WHY are these changes introduced?

We can't currently distinguish CLI runs made by AI coding agents from runs made by humans or scripts. The Shopify AI toolkit has agents declare themselves through SHOPIFY_CLI_AGENT_INFO and SHOPIFY_CLI_AGENT_IDS, but agents outside that toolkit stay invisible in analytics.

https://github.com/shop/issues-develop/issues/23864

WHAT is this pull request doing?

Detect the agent with @vercel/detect-agent and report it through the existing sensitive env_shopify_variables field, so there is no Monorail schema change.

Detected names use the packed format producers already send — SHOPIFY_CLI_AGENT_INFO="n:<name>" — so the agent column resolves the same way whether a name was declared or detected. Only the n: tag is set: detection cannot resolve version, provider or model, so those tags are omitted rather than padded, leaving those columns null instead of a literal none.

Where the package's vocabulary differs from the toolkit's, it is mapped (claude → claude-code, gemini → gemini-cli); every other name passes through verbatim. The map keys come from the package's exported KNOWN_AGENTS constant rather than string literals, so if the package renames an agent this fails the build instead of quietly reporting a stale name.

Detection is a fallback. It is skipped when SHOPIFY_CLI_AGENT_INFO or SHOPIFY_CLI_AGENT_IDS is declared, because writing SHOPIFY_CLI_AGENT_INFO ourselves would clobber the producer's whole packed value, not just the name. The merge spreads detected variables last, so it is this guard — not merge order — that protects a declared value. SHOPIFY_CLI_AGENT_DETECTED=true records that a name was derived rather than declared.

Detected names have | replaced with _. It is the tag separator with no escape sequence, and the AI_AGENT convention passes arbitrary values through verbatim, so a name containing one could otherwise fabricate tags such as v:. Substituting rather than deleting avoids squashing two words into one. A name that is nothing but separators or whitespace reports nothing at all.

A detection failure is swallowed to a debug line: telemetry must not break a command.

How this plugs into analytics

In private/node/analytics.ts, getShopifyEnvironmentVariables is now async, takes its environment as a parameter (defaulting to process.env) so it can be tested without stubbing globals, and merges detected variables on top of the allowlisted declared ones. getSensitiveEnvironmentData awaits it.

The allowedShopifyEnvironmentVariableNames allowlist is unchanged, and two useful properties fall out of it:

• The allowlist filter runs before the merge, and SHOPIFY_CLI_AGENT_DETECTED is not on it. So a SHOPIFY_CLI_AGENT_DETECTED set in the environment is dropped, and the CLI itself is the only thing that can put that key in the payload.
• SHOPIFY_CLI_AGENT_INFO is on it, which is why a declared value reaches the payload untouched.

monorailAnalyticsSkipped() and metricAnalyticsSkipped() are extracted from two expressions that already existed inline in reportAnalyticsEvent, so the detection short-circuit is guaranteed to use the same condition that decides delivery. public/node/analytics.ts now calls the two predicates; its behaviour is unchanged.

Detection is gated on monorailAnalyticsSkipped() alone, not on both transports, because the sensitive payload only ever reaches Monorail — recordMetrics receives just public scalars (cliVersion, owningPlugin, command, exitMode, timings). Gating on both would mean detecting an agent in the metrics-only configuration and then discarding the result. Note this is deliberately not analyticsDisabled(), since an alwaysLogAnalytics override still sends the Monorail event.

Net effect: detection runs only when its output can actually be delivered, so an opted-out user pays nothing for it.

How to manually test your changes?

bin/dev.js runs the bundle rather than the source, so bundle first:

pnpm nx bundle --skip-nx-cache

A detected agent, with nothing declared:

env -u SHOPIFY_CLI_AGENT_INFO -u SHOPIFY_CLI_AGENT_IDS AI_AGENT=codex SHOPIFY_CLI_ALWAYS_LOG_ANALYTICS=1 node packages/cli/bin/dev.js version --verbose | grep env_shopify_variables

Reports {"SHOPIFY_CLI_AGENT_INFO":"n:codex","SHOPIFY_CLI_AGENT_DETECTED":"true"}.

A declared agent, which detection must leave alone:

env -u SHOPIFY_CLI_AGENT_IDS AI_AGENT=codex SHOPIFY_CLI_AGENT_INFO='n:cursor|v:2.1.0|p:openai|m:gpt-5' SHOPIFY_CLI_ALWAYS_LOG_ANALYTICS=1 node packages/cli/bin/dev.js version --verbose | grep env_shopify_variables

Reports the declared value unchanged, with no SHOPIFY_CLI_AGENT_DETECTED — note the declared agent (cursor) and the detected one (codex) are deliberately different, so you can see which one won.

An opted-out user, where detection must not run at all:

env -u SHOPIFY_CLI_AGENT_INFO -u SHOPIFY_CLI_AGENT_IDS AI_AGENT=codex SHOPIFY_CLI_NO_ANALYTICS=1 node packages/cli/bin/dev.js version --verbose | grep env_shopify_variables

Reports no agent variables.

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows)
• I've considered possible documentation changes
• I've considered analytics changes to measure impact
• The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds fallback AI-agent detection to CLI analytics using @vercel/detect-agent, while preserving declared agent metadata.

Changes:

• Adds and locks the detection dependency.
• Detects, sanitizes, and reports agent names through sensitive analytics fields.
• Adds detection and analytics integration tests.

Review findings include two critical issues in analytics gating and test fixtures, four moderate issues involving send eligibility and name handling, and one documentation nit.

File summaries

File	Description
pnpm-lock.yaml	Locks the new detection dependency.
packages/cli-kit/src/public/node/analytics.ts	Provides analytics skip-check behavior.
packages/cli-kit/src/public/node/analytics.test.ts	Tests analytics integration and opt-out behavior.
packages/cli-kit/src/private/node/context/agent.ts	Implements agent detection, mapping, and sanitization.
packages/cli-kit/src/private/node/context/agent.test.ts	Tests detection behavior and normalization.
packages/cli-kit/src/private/node/analytics.ts	Integrates detected variables into analytics payloads.
packages/cli-kit/package.json	Adds the detection dependency.

Review details

Files not reviewed (1)

• pnpm-lock.yaml: Generated file

Suppressed comments (2)

packages/cli-kit/src/private/node/analytics.ts:136

• env_shopify_variables is only delivered in the Monorail payload, but this gate treats alwaysLogMetrics as sufficient to run detection. With analytics disabled and only the metrics override enabled, monorailAnalyticsSkipped() remains true, so sendAnalyticsEvent drops the Monorail payload and the detected values are discarded after the detection work. Gate this enrichment on Monorail delivery instead; the related test should not require detection for a metrics-only send.
  packages/cli-kit/src/private/node/context/agent.ts:33
• The lookup happens after replacing |, so an arbitrary AI_AGENT value can collide with a canonical detector name. With the pinned detector names, AI_AGENT=claude|code becomes claude_code and is then remapped to claude-code, falsely attributing it to the Shopify toolkit instead of preserving the sanitized custom name. Apply the known-agent mapping to rawDetectedName before sanitizing the fallback.

    const detectedName = rawDetectedName.replaceAll('|', '_').trim()

    return {
      SHOPIFY_CLI_AGENT_INFO: `n:${toolkitAgentNamesByDetectedName[detectedName] ?? detectedName}`,

• Files reviewed: 6/7 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[amcaplan]

The predicted failure doesn't occur, but the mechanism behind it was real and has been fixed — just not the way suggested.

The suite is green. does nothing when analytics are disabled asserts only on publishMonorailEvent, which the default background-send path never calls directly, so the test passed either way.

That weak assertion was hiding a real problem, which is the useful half of this comment. buildPayload() does run before the skip gate, so the single queued true from mockReturnValueOnce was being consumed during payload construction; the gate then saw false and the code went on to spawn a background analytics process. The test claimed "does nothing" while something was in fact happening.

Fixed by making the test mean what it says: mockReturnValueOnce(true) → mockReturnValue(true), plus expect(execMock).not.toHaveBeenCalled() so the background-spawn path is actually covered.

Not adopting the suggested restructuring — threading a precomputed decision into payload construction would widen getSensitiveEnvironmentData's signature to carry gating state, and the predicates are pure reads of environment state, so evaluating them more than once is cheap and side-effect free. One of the two extra calls is gone anyway, since the enrichment now gates on monorailAnalyticsSkipped() alone.

— 🤖 AI-generated reply (Claude Code), posted on @amcaplan's behalf.

[amcaplan]

Correct, and we're knowingly accepting it for now.

Confirmed the ordering: buildPayload() runs before runWithRateLimit in reportAnalyticsEvent, so once the daily limit is exhausted determineAgent() still does its env/filesystem work and the payload is then discarded.

Not changing it in this PR because both available fixes reach beyond its scope: moving detection after send-eligibility means reordering reportAnalyticsEvent, and making the enrichment lazy changes the payload-construction contract for every caller. Neither produces incorrect data — the cost is wasted work on a path that is already being dropped.

The closely related gating problem is fixed, though: detection is now gated on monorailAnalyticsSkipped() alone, since the sensitive payload only ever reaches Monorail (recordMetrics receives just public scalars). So detection no longer runs when the Monorail event won't be sent at all, which was the larger of the two wasted-work cases.

— 🤖 AI-generated reply (Claude Code), posted on @amcaplan's behalf.

[github-actions]

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

• Some seemingly private modules might be re-exported through public modules.
• If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/private/node/context/agent.d.ts

export declare function detectedAgentEnvironmentVariables(env?: NodeJS.ProcessEnv): Promise<NodeJS.ProcessEnv>;

Existing type declarations

packages/cli-kit/dist/private/node/analytics.d.ts

@@ -31,4 +31,6 @@ export declare function getSensitiveEnvironmentData(config: Interfaces.Config):
     env_plugin_installed_all: string;
     env_shopify_variables: string;
 }>;
+export declare function monorailAnalyticsSkipped(): boolean;
+export declare function metricAnalyticsSkipped(): boolean;
 export {};
\ No newline at end of file