Maintenance: Use std-env for AI agent detection in telemetry#34114
Maintenance: Use std-env for AI agent detection in telemetry#34114
Conversation
Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
|
View your CI Pipeline Execution ↗ for commit 2281e96
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThe changes refactor agent detection from explicit environment variable parsing to a simplified approach using the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches
📝 Coding Plan for PR comments
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/core/src/telemetry/detect-agent.test.ts (1)
5-64: Tests are well-structured but rely on std-env's internal behavior.The tests correctly use
vi.stubEnv()for environment variable stubbing and properly clean up withvi.unstubAllEnvs()inafterEach. However, these tests are effectively integration tests that depend on std-env's internal mapping of environment variables to agent names.If std-env changes how it maps environment variables (e.g.,
CLAUDECODE→claude), these tests would fail even thoughdetectAgent()itself works correctly. Consider whether adding a unit test that mocksstd-envdirectly would provide more stable coverage:vi.mock('std-env', { spy: true }); // Then in beforeEach: vi.mocked(stdEnvDetectAgent).mockReturnValue({ name: 'claude' });This is not blocking since the current approach validates the full integration path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/telemetry/detect-agent.test.ts` around lines 5 - 64, Tests rely on std-env's mapping of env vars to agent names causing brittle integration tests; change the tests in detect-agent.test.ts to mock the 'std-env' module directly instead of stubbing process envs: use vi.mock('std-env', { spy: true }) and in each test (or beforeEach) set vi.mocked(<imported std-env function>).mockReturnValue(...) to return the expected agent object (e.g., { name: 'claude' }) for each scenario, keep the existing detectAgent() assertions, and restore mocks/unmock in afterEach so tests no longer depend on std-env implementation details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/package.json`:
- Line 360: The project imports detectAgent from std-env but std-env v4 doesn't
export it; update detect-agent.ts to stop importing detectAgent from std-env and
provide your own implementation or switch to a package that actually exposes
that API. Specifically, in detect-agent.ts remove the failing import, implement
and export a local detectAgent function (or use a compatible library) that
inspects known environment indicators (e.g., process.env variables like VERCEL,
NETLIFY, GITHUB_ACTIONS, CI, etc.) and any runtime/provider info you need, and
then update any callers to use this local detectAgent; if you prefer to keep
std-env, change package.json to a std-env version that exports detectAgent and
adjust imports accordingly.
---
Nitpick comments:
In `@code/core/src/telemetry/detect-agent.test.ts`:
- Around line 5-64: Tests rely on std-env's mapping of env vars to agent names
causing brittle integration tests; change the tests in detect-agent.test.ts to
mock the 'std-env' module directly instead of stubbing process envs: use
vi.mock('std-env', { spy: true }) and in each test (or beforeEach) set
vi.mocked(<imported std-env function>).mockReturnValue(...) to return the
expected agent object (e.g., { name: 'claude' }) for each scenario, keep the
existing detectAgent() assertions, and restore mocks/unmock in afterEach so
tests no longer depend on std-env implementation details.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 134c1ffa-0b26-4772-b8f4-91e58155c56c
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
code/core/package.jsoncode/core/src/telemetry/detect-agent.test.tscode/core/src/telemetry/detect-agent.tscode/core/src/telemetry/telemetry.ts
| "sirv": "^2.0.4", | ||
| "slash": "^5.0.0", | ||
| "source-map": "^0.7.4", | ||
| "std-env": "^4.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
std-env detectAgent function API
💡 Result:
std-env (the UnJS npm package) does not expose a detectAgent() function. Its public API is primarily environment flags, plus CI/provider detection and runtime detection via exported constants like provider, providerInfo, runtime, and runtimeInfo. [1][2][3]
Example usage:
import { provider, providerInfo, runtime, runtimeInfo, isCI } from "std-env";
console.log({ isCI, provider, providerInfo, runtime, runtimeInfo });If you meant a different “std-env” library (or a different package that does have detectAgent), share a link/repo name and I’ll pull the exact API. [1][2]
Sources: [1] [2] [3]
🏁 Script executed:
rg "detectAgent" --type js --type ts -B 2 -A 2Repository: storybookjs/storybook
Length of output: 5940
🏁 Script executed:
cat -n code/core/src/telemetry/detect-agent.tsRepository: storybookjs/storybook
Length of output: 803
Critical: std-env v4.0.0 does not export detectAgent function.
The code imports detectAgent from std-env (line 1 of detect-agent.ts), but std-env v4's public API provides only provider, providerInfo, runtime, runtimeInfo, and isCI. This import will fail at runtime. Either use a different package that provides agent detection, or implement the agent detection logic directly without external dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/package.json` at line 360, The project imports detectAgent from
std-env but std-env v4 doesn't export it; update detect-agent.ts to stop
importing detectAgent from std-env and provide your own implementation or switch
to a package that actually exposes that API. Specifically, in detect-agent.ts
remove the failing import, implement and export a local detectAgent function (or
use a compatible library) that inspects known environment indicators (e.g.,
process.env variables like VERCEL, NETLIFY, GITHUB_ACTIONS, CI, etc.) and any
runtime/provider info you need, and then update any callers to use this local
detectAgent; if you prefer to keep std-env, change package.json to a std-env
version that exports detectAgent and adjust imports accordingly.
| // - cliVersion | ||
| const inCI = isCI(); | ||
| const agentDetection = detectAgent({ stdoutIsTTY: process.stdout.isTTY, env: process.env }); | ||
| const agentDetection = detectAgent(); |
There was a problem hiding this comment.
This may break former telemetry as name are not exactly the same as before. 'claude-code' for example is 'claude' in std-env
There was a problem hiding this comment.
Yes, this is an intentional breaking change that comes with adopting std-env's naming conventions. It's documented in the PR description's "Behavioural changes to note" table:
| Before | After |
|---|---|
claude-code |
claude |
gemini-cli |
gemini |
Since the goal is to use std-env as the canonical source for agent detection, these new names are the ones std-env uses and will continue to maintain. Any telemetry dashboards/queries that filter on the old names would need to be updated to account for the rename. Let me know if you'd prefer to add a name-mapping layer to preserve backward compatibility with historical telemetry data, and I'll implement that.
There was a problem hiding this comment.
This will break our telemetry. It is probably better to have a name-mapping.
Storybook had its own bespoke agent detection logic that was already drifting behind the ecosystem.
std-envv4 ships a well-maintaineddetectAgent()that covers more agents and supports the standardAI_AGENTenv var (used by Vitest 4.1's new agent reporter).Changes
detect-agent.ts— Replaces all custom env var checks and heuristics with a thin wrapper aroundstd-env'sdetectAgent():CLAUDECODE,GEMINI_CLI,CODEX_SANDBOX,CURSOR_AGENT,AGENTchecks, andTERM=dumb/GIT_PAGER=catheuristicsclaude,gemini,codex,opencode,cursor,replit,auggie,goose,devin,kiro,pi, and any agent viaAI_AGENTAgentInfo.namewidened from a closedKnownAgentNameunion tostringtelemetry.ts—detectAgent()call drops the{ stdoutIsTTY, env }options (std-env readsprocess.envdirectly)detect-agent.test.ts— Tests rewritten to usevi.stubEnvagainst the new std-env-backed detectionpackage.json—std-env@^4.0.0added as adevDependency(bundled, not a runtime external)Behavioural changes to note
claude-codeclaudegemini-cligeminiAGENT=ampdetectedAI_AGENT=amp(std-env explicit override)TERM=dumb/GIT_PAGER=catheuristicsWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
ofcncog2cu-dsn.algolia.net/opt/hostedtoolcache/node/22.22.1/x64/bin/node /opt/hostedtoolcache/node/22.22.1/x64/bin/node /home/REDACTED/work/storybook/storybook/.yarn/releases/yarn-4.10.3.cjs add --dev std-env@^4.0.0(dns block)If you need me to access, download, or install something from one of these locations, you can either:
View original Slack conversation
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.
Summary by CodeRabbit
Refactor
Tests
Chores