chore: fix telemetry env var for publish CI #230
Conversation
|
Claude finished @ymc9's task —— View job PR Review: Telemetry Environment Variable FixTodo List:
Review SummaryThe changes look good and address a clear need for telemetry configuration in the publish workflow. Here's my analysis: ✅ Positive Aspects
📋 Code Quality & Best Practices
🔒 Security Considerations
🐛 Potential IssuesMinor Consideration: The workflow doesn't validate if 🚀 Performance Impact
🧪 Test Coverage
📝 Minor Suggestions
Overall AssessmentThis is a clean, straightforward fix that follows GitHub Actions best practices. The changes are minimal, focused, and address the specific need for telemetry configuration in the publish workflow. No blocking issues identified. Recommendation: ✅ Approve - Ready to merge |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes telemetry configuration in the publish release CI workflow by adding proper environment variable settings.
- Adds telemetry tracking token from secrets
- Disables tracking with DO_NOT_TRACK flag
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds workflow-wide env vars to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/publish-release.yml (1)
8-10: Resolve telemetry env contradiction and scope secret to the single step that needs itSetting DO_NOT_TRACK='1' while also exporting TELEMETRY_TRACKING_TOKEN at workflow scope is contradictory and unnecessarily exposes the secret to all steps and 3rd‑party actions. Decide intent and adjust scoping:
Preferred: disable telemetry by default, enable only for the publish step.
- Keep DO_NOT_TRACK='1' at workflow level.
- Remove TELEMETRY_TRACKING_TOKEN from workflow env.
- In the "Publish packages" step, set TELEMETRY_TRACKING_TOKEN and override DO_NOT_TRACK='0'.
Apply this diff:
name: Publish and Release @@ -on: +on: workflow_dispatch: push: branches: - main -env: - TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }} - DO_NOT_TRACK: '1' +env: + DO_NOT_TRACK: '1' @@ - name: Publish packages run: pnpm run publish-all env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }} + DO_NOT_TRACK: '0'Verification (optional): confirm your tooling actually honors these vars.
#!/bin/bash # Where are these envs used? rg -n --hidden -S "DO_NOT_TRACK|TELEMETRY_TRACKING_TOKEN|TELEMETRY|TRACKING|NEXT_TELEMETRY_DISABLED|TURBO_TELEMETRY_DISABLED"
🧹 Nitpick comments (1)
.github/workflows/publish-release.yml (1)
8-10: If telemetry should be fully disabled, remove the token entirelyAlternate path: if the goal is to ensure no tracking during CI, drop the token and keep DO_NOT_TRACK='1'.
-env: - TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }} - DO_NOT_TRACK: '1' +env: + DO_NOT_TRACK: '1'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/publish-release.yml(1 hunks)
|
Claude finished @ymc9's task —— View job PR Review: Telemetry Environment Variable FixTodo List:
Review SummaryI've analyzed all the changes in this PR. Here's my comprehensive review: 🚨 Critical Issue: Contradictory Telemetry ConfigurationFile: The most significant issue is the contradictory telemetry configuration: env:
TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }}
DO_NOT_TRACK: '1'Problem: Setting both a tracking token AND Recommendations:
✅ Positive AspectsWorkflow Changes
Test Configuration Changes
📋 Other FindingsTest File Changes (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/test/init.test.ts (1)
7-8: Skip only on CI to keep the suite running locallyAvoid blanket skipping so regressions are still caught locally. Use conditional skipping, which Vitest supports via describe.skipIf / describe.runIf.
-// skipping due to timeout during CI -describe.skip('Cli init command tests', () => { +// skip in CI only; keep running locally +describe.skipIf(!!process.env.CI)('Cli init command tests', () => {Refs: Vitest API for describe.skipIf / runIf. (vitest.dev, v2.vitest.dev)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/cli/test/init.test.ts(1 hunks)vitest.base.config.ts(0 hunks)vitest.config.ts(1 hunks)vitest.workspace.ts(0 hunks)
💤 Files with no reviewable changes (2)
- vitest.base.config.ts
- vitest.workspace.ts
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/cli/test/init.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test (20.x)
🔇 Additional comments (1)
vitest.config.ts (1)
1-7: Migration to test.projects looks correctUsing test.projects is the recommended approach in Vitest ≥3.2; workspace is deprecated. This config shape aligns with current docs.
Refs: Vitest “Test Projects” guide. (vitest.dev)
|
Claude finished @ymc9's task —— View job PR Review: Telemetry Environment Variable Fix Todo List:
Comprehensive Code Review🚨 Critical Issue: Contradictory Telemetry ConfigurationFile: env:
TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }}
DO_NOT_TRACK: '1'Problem: This is the most significant issue in the PR. Setting both a telemetry tracking token AND Security Risk: The tracking token is unnecessarily exposed to all workflow steps and third-party actions when it may not be needed if tracking is disabled. Recommendations:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/cli/test/migrate.test.ts (1)
12-13: Make test suite skips conditional on CI, don’t permanently disable
- In packages/cli/test/migrate.test.ts, replace the hard
describe.skip('CLI migrate commands test', …)with a CI-gated skip (e.g.) and consider raising the per-test timeout.const inCI = process.env.CI === 'true' || process.env.CI === '1'; const d = inCI ? describe.skip : describe; d('CLI migrate commands test', () => { … });- In packages/cli/test/init.test.ts, apply the same CI‐gated skip for the
describe.skip('Cli init command tests', …)suite.Optionally open a follow-up issue to re-enable both suites once CI is stable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/cli/test/migrate.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/cli/test/migrate.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (20.x)
- GitHub Check: claude-review
|
Claude finished @ymc9's task —— View job PR Review: Telemetry Environment Variable Fix Todo List:
Comprehensive Code Review🚨 Critical Issue: Contradictory Telemetry ConfigurationFile: env:
TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }}
DO_NOT_TRACK: '1'Problem: This is the most significant issue in the PR. Setting both a telemetry tracking token AND Security Risk: The tracking token is unnecessarily exposed to all workflow steps and third-party actions when it may not be needed if tracking is disabled. Recommendations:
|
Summary by CodeRabbit
Chores
Tests