fix: avoid build failures during CI

[ymc9]

Summary by CodeRabbit

• Bug Fixes
  • Fixed inconsistent build behavior for the VS Code extension when telemetry tracking configuration is unavailable. The build process now exits consistently with a warning message across all environments, rather than using different exit codes and logging levels depending on whether it runs in CI or local development.

[coderabbitai]

📝 Walkthrough

Walkthrough

The post-build.ts script was refactored to unify its exit behavior when the telemetry tracking token is unset. It now consistently exits with code 0 and logs a warning, removing the previous environment-dependent exit code logic that varied between CI and non-CI contexts.

Changes

Cohort / File(s)	Summary
Build Script Enhancement packages/ide/vscode/scripts/post-build.ts	Unified exit code behavior for unset telemetry token; now always exits with code 0 and emits a warning instead of conditional exit codes based on CI environment.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop, hop, the telemetry hops with grace,
No more CI paths splitting the trace,
One steady exit, a warning so clear,
Post-build scripts now unified here! ✨

🚥 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 'fix: avoid build failures during CI' directly relates to the main change, which modifies the build script to always exit with code 0 when the telemetry token is unset, preventing CI build failures.
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/env-build-error

---

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

[claude]

Claude finished @ymc9's task —— View job

---

Code Review for PR #2510

Reviewing the pull request "fix: avoid build failures during CI".

Todo List

• Read and analyze the modified file
• Provide code review feedback on quality, bugs, performance, security, and test coverage
• Update comment with comprehensive review

---

Review Summary

This PR simplifies the error handling in the VSCode extension's post-build script by removing the CI-specific error behavior that was causing build failures.

Changes Made

The change modifies packages/ide/vscode/scripts/post-build.ts:8-11 to remove the conditional CI error handling:

Before:

if (!telemetryToken) {
    if (process.env.CI) {
        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);
    }
}

After:

if (!telemetryToken) {
    console.warn('Warning: VSCODE_TELEMETRY_TRACKING_TOKEN environment variable is not set, skipping token injection');
    process.exit(0);
}

Code Quality Assessment

✅ Positive Aspects:

• Clear Intent: The change directly addresses the stated goal of avoiding build failures during CI
• Consistent Behavior: Now behaves the same way in all environments (CI and local)
• Graceful Degradation: Uses warning instead of error, allowing builds to continue
• Clean Implementation: Removes unnecessary conditional complexity

⚠️ Considerations:

1. Missing Telemetry in Production: This change means that if the telemetry token is missing in CI/production builds, the extension will be published without telemetry functionality. Consider if this is the intended behavior.

2. Silent Failures: The script now exits with code 0 (success) even when telemetry injection fails. This could make it harder to detect configuration issues.

3. Build Process Impact: Verify that downstream build steps handle the case where telemetry token injection is skipped.

Security Considerations

✅ No Security Issues Detected:

• The change doesn't introduce any security vulnerabilities
• Still properly handles environment variables
• No exposure of sensitive data

Performance Considerations

✅ Minimal Performance Impact:

• Removes one conditional check (minor improvement)
• File I/O operations remain unchanged when token is present
• Early exit when token is missing (good)

Test Coverage Recommendations

Consider adding tests for:

1. Missing Token Scenarios: Verify script behavior when VSCODE_TELEMETRY_TRACKING_TOKEN is undefined
2. Token Injection Success: Ensure token replacement works correctly
3. File Not Found: Handle cases where dist/extension.js doesn't exist

Questions for Consideration

1. Production Impact: Is it acceptable for production builds to skip telemetry injection without failing the build?
2. Monitoring: How will you detect when telemetry is not working due to missing tokens?
3. Alternative Approach: Should this script succeed but mark the build artifact in some way to indicate missing telemetry?

Overall Assessment

This is a pragmatic fix that resolves immediate CI build failures. The code quality is good, and the change is minimal and focused. However, consider the trade-offs between build reliability and telemetry functionality in production environments.

Recommendation: ✅ Approve - This change successfully addresses the build failure issue while maintaining code quality and security standards.

---

[coderabbitai]

🧹 Nitpick comments (1)

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

8-11: Consider stricter validation for publish workflows.

The change successfully prevents CI build failures when the telemetry token is unset, which is appropriate for development and testing. However, since vscode:publish depends on this build step (per package.json), a missing token during an actual release would now silently succeed, producing an extension without telemetry tracking.

Consider one of the following approaches to safeguard production releases:

1. Add a separate validation step in the vscode:publish script that requires the token
2. Introduce an environment variable (e.g., REQUIRE_TELEMETRY_TOKEN=true) that can enforce exit code 1 for release pipelines

💡 Optional: Add stricter mode for release pipelines

 const telemetryToken = process.env.VSCODE_TELEMETRY_TRACKING_TOKEN;
 if (!telemetryToken) {
+    if (process.env.REQUIRE_TELEMETRY_TOKEN === 'true') {
+        console.error('Error: VSCODE_TELEMETRY_TRACKING_TOKEN is required but not set');
+        process.exit(1);
+    }
     console.warn('Warning: VSCODE_TELEMETRY_TRACKING_TOKEN environment variable is not set, skipping token injection');
     process.exit(0);
 }

Then update package.json for the publish script:

"vscode:publish": "REQUIRE_TELEMETRY_TOKEN=true pnpm build && vsce publish --no-dependencies --follow-symlinks"

🤖 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` around lines 8 - 11, Update the
telemetry token check in post-build.ts so missing tokens fail CI only in
stricter mode: where currently the code checks telemetryToken and warns then
process.exit(0), change it to read process.env.REQUIRE_TELEMETRY_TOKEN and, if
REQUIRE_TELEMETRY_TOKEN === 'true' and telemetryToken is falsy, call
processLogger.error or console.error and process.exit(1); otherwise keep the
existing console.warn and process.exit(0). Reference the telemetryToken check in
post-build.ts and the new REQUIRE_TELEMETRY_TOKEN env variable to locate and
implement this conditional behavior.

🤖 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`:
- Around line 8-11: Update the telemetry token check in post-build.ts so missing
tokens fail CI only in stricter mode: where currently the code checks
telemetryToken and warns then process.exit(0), change it to read
process.env.REQUIRE_TELEMETRY_TOKEN and, if REQUIRE_TELEMETRY_TOKEN === 'true'
and telemetryToken is falsy, call processLogger.error or console.error and
process.exit(1); otherwise keep the existing console.warn and process.exit(0).
Reference the telemetryToken check in post-build.ts and the new
REQUIRE_TELEMETRY_TOKEN env variable to locate and implement this conditional
behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3ed50051-2779-420a-8e84-db5834b4b3ac

📥 Commits

Reviewing files that changed from the base of the PR and between bebb181 and 2908868.

📒 Files selected for processing (1)

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