Skip to content

Add AI agent detection to CLI analytics - #8523

Merged
amcaplan merged 1 commit into
mainfrom
detect-agents
Sep 15, 2026
Merged

amcaplan merged 1 commit into
mainfrom
detect-agents

Conversation

@amcaplan

@amcaplan amcaplan commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

CLI analytics can't distinguish AI-agent runs from human or scripted ones. The AI toolkit has agents declare themselves through SHOPIFY_CLI_AGENT_INFO / SHOPIFY_CLI_AGENT_IDS; agents outside it stay invisible.

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's no Monorail schema change. Detected names reuse the packed format producers already send — SHOPIFY_CLI_AGENT_INFO="n:<name>" — so the agent column resolves identically whether declared or detected. Only n: is set; detection can't resolve version, provider or model.

Decisions worth review:

  • Detection is skipped when either declared variable is present: writing SHOPIFY_CLI_AGENT_INFO ourselves would clobber the producer's packed value, not just the name.
  • SHOPIFY_CLI_AGENT_DETECTION reports the outcome on every run — detected, none, unusable_name, skipped, failed — so a detection error is measurable rather than indistinguishable from finding nothing. Deliberately off the allowlist, which filters before the merge, so only the CLI can set it. It describes the detector's own outcome, so it is not a filter for agent vs non-agent traffic: legacy SHOPIFY_CLI_AGENT is allowlisted and coalesced first but doesn't suppress detection, so a legacy-only producer can yield that variable alongside none.
  • | becomes _ in detected names — it separates tags with no escape sequence, so one could otherwise fabricate tags like v:.
  • Not gated on analytics delivery: determineAgent() measures 15µs worst case, against an event that does a network POST.

How to manually test your changes?

bin/dev.js runs the bundle, so pnpm nx bundle --skip-nx-cache first. Detected, nothing declared — reports "n:codex" with "SHOPIFY_CLI_AGENT_DETECTION":"detected":

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

Declared, which detection must leave untouched — reports the declared value with "SHOPIFY_CLI_AGENT_DETECTION":"skipped". The declared (cursor) and detected (codex) names differ so you can see which won:

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

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

@amcaplan amcaplan added the claudeception Pull request created by Claudeception agents label Sep 10, 2026
@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Sep 10, 2026
@amcaplan
amcaplan force-pushed the detect-agents branch 7 times, most recently from e5604a9 to 7e369c5 Compare September 14, 2026 14:01
@amcaplan
amcaplan marked this pull request as ready for review September 14, 2026 18:01
@amcaplan
amcaplan requested a review from a team as a code owner September 14, 2026 18:01
Copilot AI lite review requested due to automatic review settings September 14, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment thread packages/cli-kit/src/private/node/analytics.ts Outdated
Comment thread packages/cli-kit/src/private/node/context/agent.test.ts
Comment thread packages/cli-kit/src/private/node/analytics.ts
Comment thread packages/cli-kit/src/private/node/context/agent.ts Outdated
Comment thread packages/cli-kit/src/private/node/context/agent.ts Outdated
@amcaplan
amcaplan force-pushed the detect-agents branch 4 times, most recently from 2665e2c to 140c363 Compare September 15, 2026 07:51
@gonzaloriestra

Copy link
Copy Markdown
Contributor

/snapit

@github-actions

Copy link
Copy Markdown
Contributor

🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260915103618

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@gonzaloriestra gonzaloriestra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work well 👍

I just added a suggestion


try {
const detection = await determineAgent()
if (!detection.isAgent) return {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we better return SHOPIFY_CLI_AGENT_DETECTED: 'false' in this case to distinguish from skipped or failed detection?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, and it pushed the design somewhere better than the original suggestion.

Returning 'false' here would separate "no agent" from the other two states, but it puts the marker on ~every run — no-agent is the overwhelming majority — while leaving the rare, interesting case, determineAgent() actually throwing, still silent and inferable only from total absence. And "absence" is ambiguous: it also matches an older CLI with no detection code at all.

So SHOPIFY_CLI_AGENT_DETECTED is retired in favour of one always-present SHOPIFY_CLI_AGENT_DETECTION:

state value
a declaration was already present, detection skipped skipped
detection produced a usable name detected
detection ran and found no agent none
an agent was found under an unusable name unusable_name
determineAgent() threw failed

The unusable_name case is an agent reported under a name that is nothing but tag separators or whitespace — reachable by setting AI_AGENT to such a value. It started out folded into none, but that misreports a real agent run as a non-agent one and buries an anomaly in the bucket holding ~99% of traffic, which defeats the point of adding states at all.

One caveat worth carrying into the warehouse column: this field describes only the fallback detector's own outcome, and shouldn't be used by itself to filter agent from non-agent traffic. The guard deliberately ignores the legacy singular SHOPIFY_CLI_AGENT (which the warehouse coalesces first, and which is itself on the allowlist), so a legacy-only producer can yield a single row carrying both SHOPIFY_CLI_AGENT: <their value> and SHOPIFY_CLI_AGENT_DETECTION: none. That row does have an agent — the fallback detector just didn't find it. Same applies in reverse: detected means detection found an agent, not that the resolved agent column came from detection.

Like the key it replaces, SHOPIFY_CLI_AGENT_DETECTION is deliberately kept off allowedShopifyEnvironmentVariableNames. That filter runs before the merge, so the CLI stays the only thing that can set it.

The cost is one short key on every analytics event — the price of a denominator that doesn't depend on reasoning about CLI versions.

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

Detect the agent running the CLI with @vercel/detect-agent and report it as
n:<name> inside SHOPIFY_CLI_AGENT_INFO, the packed format the Shopify AI
toolkit already uses, so a detected name resolves through the same field as
a declared one.

Detection only fills the gap. It is skipped when a producer declared
SHOPIFY_CLI_AGENT_INFO or SHOPIFY_CLI_AGENT_IDS, because writing INFO
ourselves would clobber their whole packed value, not just the name.

SHOPIFY_CLI_AGENT_DETECTION reports the outcome on every run, as one of
detected, none, unusable_name, skipped or failed, so a detection error is
measurable rather than indistinguishable from finding no agent. It is
deliberately absent from the allowlist, which filters before the merge, so
the CLI is the only thing that can set it. It describes the detector's own
outcome, not whether the run had an agent: legacy SHOPIFY_CLI_AGENT does not
suppress detection, so a legacy-only producer can yield that variable
alongside none.

Assisted-By: devx/105d6a35-ec6d-462e-9e36-a9fde4ec06fc
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

We found no diffs with existing type declarations

@amcaplan
amcaplan added this pull request to the merge queue Sep 15, 2026
Merged via the queue into main with commit 8829ed5 Sep 15, 2026
42 of 55 checks passed
@amcaplan
amcaplan deleted the detect-agents branch September 15, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claudeception Pull request created by Claudeception agents no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants