fix(ide): only error on missing telemetry token in CI

[ymc9]

Summary

• The VSCode post-build script now only fails on a missing VSCODE_TELEMETRY_TRACKING_TOKEN when running in CI (process.env.CI)
• In local development, it warns and exits cleanly (code 0), skipping token injection

Test plan

• Verify local build succeeds without the env var set
• Verify CI build still fails if the env var is missing

🤖 Generated with Claude Code

Summary by CodeRabbit

• Chores
  • Build process now succeeds in local development environments when telemetry configuration is unavailable, while maintaining strict validation in continuous integration pipelines.

[claude]

Claude finished @ymc9's task —— View job

---

Code Review Complete ✅

I've analyzed the changes in packages/ide/vscode/scripts/post-build.ts and here's my comprehensive feedback:

Todo List:

• Read and analyze the modified file
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage implications
• Provide comprehensive feedback

Code Quality & Best Practices ✅

The code is clean, readable, and follows good practices:

• Clear logic flow: The conditional check for process.env.CI is straightforward and easy to understand
• Appropriate exit codes: Uses process.exit(1) for CI failures and process.exit(0) for local development skips
• Good error messaging: Clear distinction between error and warning messages
• Consistent style: Follows the existing codebase patterns

Potential Bugs/Issues ✅

No significant bugs detected. The implementation correctly:

• ✅ Checks for telemetry token availability before attempting to use it
• ✅ Handles both CI and local development environments appropriately
• ✅ Prevents the script from attempting file operations when the token is missing
• ✅ Uses proper exit codes for different scenarios

Performance Considerations ✅

Excellent performance improvement:

• Early exit optimization: The script now exits immediately in local development when no token is present, avoiding unnecessary file I/O operations (lines 18-21)
• Minimal overhead: The process.env.CI check is a simple boolean evaluation with negligible performance impact

Security Concerns ⚠️

Minor considerations (not critical issues):

1. Token exposure in logs: The token replacement happens at line 19, but this appears safe as:

   • No logging of the actual token value
   • File operations are local to the build process
   • Token is only used for placeholder replacement

2. File validation: Consider adding basic validation that the target file exists before attempting to read it:

   if (!fs.existsSync(file)) {
       console.error(`Error: Target file ${file} not found`);
       process.exit(1);
   }

Test Coverage Implications ✅

The PR includes appropriate test cases in the description:

• ✅ Local build succeeds without env var
• ✅ CI build fails if env var is missing

Recommendations for testing:

• Consider adding unit tests for this script to verify the conditional behavior
• Test that file operations are skipped when token is missing in local development

Overall Assessment

This is a well-implemented fix that addresses the original problem effectively:

✅ Solves the issue: Prevents local development builds from failing due to missing telemetry tokens
✅ Maintains CI integrity: Still enforces token requirements in CI environments
✅ Clean implementation: Simple, readable code with appropriate error handling
✅ Performance positive: Reduces unnecessary operations in local development

Minor Enhancement Suggestion

Consider adding file existence validation for robustness:

const file = 'dist/extension.js';
if (!fs.existsSync(file)) {
    console.error(`Error: Target file ${file} not found`);
    process.exit(1);
}

Verdict: This PR is ready to merge. The implementation is solid, addresses the stated requirements, and follows good coding practices.

---

[coderabbitai]

📝 Walkthrough

Walkthrough

The post-build script's error handling for a missing telemetry token is now conditional on the CI environment. In CI environments, it fails with an error as before. Outside CI, it logs a warning, skips token injection, and exits successfully.

Changes

Cohort / File(s)	Summary
Telemetry Token Handling packages/ide/vscode/scripts/post-build.ts	Modified error handling when VSCODE_TELEMETRY_TRACKING_TOKEN is unset to branch on process.env.CI. Maintains failure in CI while allowing graceful skip outside CI environments with a warning log.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A token that may not appear,
No longer brings a developer's tear.
In CI we must stand firm and tall,
But locally, we simply skip it all. 🎯

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: modifying error handling for a missing telemetry token to only fail in CI environments, which aligns directly with the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/vscode-post-build-ci-only

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/ide/vscode/scripts/post-build.ts (1)

9-9: Normalize CI parsing instead of relying on string truthiness.

Line 9 uses if (process.env.CI), which treats string values like "false" as truthy. While most CI systems set CI=true, this is fragile and could incorrectly trigger the error path if CI is explicitly set to any non-empty value.

Proposed fix

+const isCi = ['1', 'true'].includes((process.env.CI ?? '').toLowerCase());
 if (!telemetryToken) {
-    if (process.env.CI) {
+    if (isCi) {
         console.error('Error: VSCODE_TELEMETRY_TRACKING_TOKEN environment variable is not set');
         process.exit(1);
     } else {
         console.warn('Warning: VSCODE_TELEMETRY_TRACKING_TOKEN environment variable is not set, skipping token injection');
         process.exit(0);
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/ide/vscode/scripts/post-build.ts` at line 9, Replace the current
truthiness check on process.env.CI with a normalized boolean parse: create a
local flag (e.g., isCI) by lowercasing process.env.CI and checking against
explicit true-like values ("1", "true", "yes") and then use that flag in the
existing if (process.env.CI) conditional; update the conditional to if (isCI) so
values like "false" or empty strings don’t mistakenly evaluate as true while
preserving CI detection logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/ide/vscode/scripts/post-build.ts`:
- Line 9: Replace the current truthiness check on process.env.CI with a
normalized boolean parse: create a local flag (e.g., isCI) by lowercasing
process.env.CI and checking against explicit true-like values ("1", "true",
"yes") and then use that flag in the existing if (process.env.CI) conditional;
update the conditional to if (isCI) so values like "false" or empty strings
don’t mistakenly evaluate as true while preserving CI detection logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 070bc687-cfa6-4ea3-9b77-a84998109e64

📥 Commits

Reviewing files that changed from the base of the PR and between cb49667 and 490d87b.

📒 Files selected for processing (1)

• packages/ide/vscode/scripts/post-build.ts