diff --git a/.changeset/four-bears-listen.md b/.changeset/four-bears-listen.md new file mode 100644 index 0000000000..d81de2b559 --- /dev/null +++ b/.changeset/four-bears-listen.md @@ -0,0 +1,5 @@ +--- +"claude-dev": patch +--- + +Dev: Supports using secrets as PostHog API keys at build time diff --git a/.github/workflows/publish-nightly.yml b/.github/workflows/publish-nightly.yml index b155a2bd37..148604ac89 100644 --- a/.github/workflows/publish-nightly.yml +++ b/.github/workflows/publish-nightly.yml @@ -69,5 +69,7 @@ jobs: env: VSCE_PAT: ${{ secrets.VSCE_PAT }} OVSX_PAT: ${{ secrets.OVSX_PAT }} + TELEMETRY_SERVICE_API_KEY: ${{ secrets.TELEMETRY_SERVICE_API_KEY }} + ERROR_SERVICE_API_KEY: ${{ secrets.ERROR_SERVICE_API_KEY }} CLINE_ENVIRONMENT: production run: npm run publish:marketplace:nightly \ No newline at end of file diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4c65352024..71c7743998 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -95,6 +95,8 @@ jobs: VSCE_PAT: ${{ secrets.VSCE_PAT }} OVSX_PAT: ${{ secrets.OVSX_PAT }} CLINE_ENVIRONMENT: production + TELEMETRY_SERVICE_API_KEY: ${{ secrets.TELEMETRY_SERVICE_API_KEY }} + ERROR_SERVICE_API_KEY: ${{ secrets.ERROR_SERVICE_API_KEY }} run: | # Required to generate the .vsix vsce package --allow-package-secrets sendgrid --out "cline-${{ steps.get_version.outputs.version }}.vsix" diff --git a/src/services/error/ClineError.ts b/src/services/error/ClineError.ts index a67d8af5e1..92dfeb56de 100644 --- a/src/services/error/ClineError.ts +++ b/src/services/error/ClineError.ts @@ -115,6 +115,10 @@ export class ClineError extends Error { */ static transform(error: any, modelId?: string, providerId?: string): ClineError { try { + // If already a ClineError, return it directly to prevent infinite recursion + if (error instanceof ClineError) { + return error + } return new ClineError(JSON.parse(error), modelId, providerId) } catch { return new ClineError(error, modelId, providerId) diff --git a/src/services/error/ErrorProviderFactory.ts b/src/services/error/ErrorProviderFactory.ts index ce040154e5..6fcb67dd07 100644 --- a/src/services/error/ErrorProviderFactory.ts +++ b/src/services/error/ErrorProviderFactory.ts @@ -1,11 +1,12 @@ -import { PostHogClientConfig, posthogConfig } from "@/shared/services/config/posthog-config" +import { isPostHogConfigValid, PostHogClientConfig, posthogConfig } from "@/shared/services/config/posthog-config" +import { ClineError } from "./ClineError" import { IErrorProvider } from "./providers/IErrorProvider" import { PostHogErrorProvider } from "./providers/PostHogErrorProvider" /** * Supported error provider types */ -export type ErrorProviderType = "posthog" | "none" +export type ErrorProviderType = "posthog" | "no-op" /** * Configuration for error providers @@ -27,18 +28,19 @@ export class ErrorProviderFactory { */ public static async createProvider(config: ErrorProviderConfig): Promise { switch (config.type) { - case "posthog": - if (config.config.apiKey !== undefined && config.config.errorTrackingApiKey !== undefined) { - return await new PostHogErrorProvider({ - apiKey: config.config.apiKey, - errorTrackingApiKey: config.config.errorTrackingApiKey, - host: config.config.host, - uiHost: config.config.uiHost, - }).initialize() - } - return new NoOpErrorProvider() + case "posthog": { + const hasValidPostHogConfig = isPostHogConfigValid(config.config) + const errorTrackingApiKey = config.config.errorTrackingApiKey + return hasValidPostHogConfig && errorTrackingApiKey + ? await new PostHogErrorProvider({ + apiKey: errorTrackingApiKey, + errorTrackingApiKey: errorTrackingApiKey, + host: config.config.host, + uiHost: config.config.uiHost, + }).initialize() + : new NoOpErrorProvider() // Fallback to no-op provider + } default: - console.error(`Unsupported error provider type: ${config.type}`) return new NoOpErrorProvider() } } @@ -60,31 +62,32 @@ export class ErrorProviderFactory { * or for testing purposes */ class NoOpErrorProvider implements IErrorProvider { - public logException(_error: Error, _properties?: Record): void { - // No-op + public logException(error: Error | ClineError, _properties?: Record): void { + // Use console.error directly to avoid potential infinite recursion through Logger + console.error("[NoOpErrorProvider]", error.message || String(error)) } public logMessage( - _message: string, - _level?: "error" | "warning" | "log" | "debug" | "info", - _properties?: Record, + message: string, + level?: "error" | "warning" | "log" | "debug" | "info", + properties?: Record, ): void { - // No-op + console.log("[NoOpErrorProvider]", { message, level, properties }) } public isEnabled(): boolean { - return false + return true } public getSettings() { return { - enabled: false, - hostEnabled: false, - level: "off" as const, + enabled: true, + hostEnabled: true, + level: "all" as const, } } public async dispose(): Promise { - // No-op + console.info("[NoOpErrorProvider] Disposing") } } diff --git a/src/services/error/providers/PostHogErrorProvider.ts b/src/services/error/providers/PostHogErrorProvider.ts index 561f6cb15b..7808920bac 100644 --- a/src/services/error/providers/PostHogErrorProvider.ts +++ b/src/services/error/providers/PostHogErrorProvider.ts @@ -23,7 +23,7 @@ export class PostHogErrorProvider implements IErrorProvider { constructor(clientConfig: PostHogClientValidConfig) { // Use shared PostHog client if provided, otherwise create a new one - this.client = new PostHog(clientConfig.apiKey, { + this.client = new PostHog(clientConfig.errorTrackingApiKey, { host: clientConfig.host, enableExceptionAutocapture: false, // NOTE: Re-enable it once the api key is set to env var before_send: (event) => PostHogClientProvider.eventFilter(event), diff --git a/src/services/feature-flags/FeatureFlagsProviderFactory.ts b/src/services/feature-flags/FeatureFlagsProviderFactory.ts index 18855e1ee4..09a1471d17 100644 --- a/src/services/feature-flags/FeatureFlagsProviderFactory.ts +++ b/src/services/feature-flags/FeatureFlagsProviderFactory.ts @@ -1,3 +1,5 @@ +import { isPostHogConfigValid, posthogConfig } from "@/shared/services/config/posthog-config" +import { Logger } from "../logging/Logger" import { PostHogClientProvider } from "../posthog/PostHogClientProvider" import type { IFeatureFlagsProvider } from "./providers/IFeatureFlagsProvider" import { PostHogFeatureFlagsProvider } from "./providers/PostHogFeatureFlagsProvider" @@ -5,7 +7,7 @@ import { PostHogFeatureFlagsProvider } from "./providers/PostHogFeatureFlagsProv /** * Supported feature flags provider types */ -export type FeatureFlagsProviderType = "posthog" | "none" +export type FeatureFlagsProviderType = "posthog" | "no-op" /** * Configuration for feature flags providers @@ -26,18 +28,17 @@ export class FeatureFlagsProviderFactory { */ public static createProvider(config: FeatureFlagsProviderConfig): IFeatureFlagsProvider { switch (config.type) { - case "posthog": + case "posthog": { // Get the shared PostHog client from PostHogClientProvider - const client = PostHogClientProvider.getClient() - if (client) { - return new PostHogFeatureFlagsProvider(client) + const sharedClient = PostHogClientProvider.getClient() + if (sharedClient) { + return new PostHogFeatureFlagsProvider(sharedClient) } // Fall back to NoOp provider if no client is available return new NoOpFeatureFlagsProvider() - case "none": - return new NoOpFeatureFlagsProvider() + } default: - throw new Error(`Unsupported feature flags provider type: ${config.type}`) + return new NoOpFeatureFlagsProvider() } } @@ -46,8 +47,9 @@ export class FeatureFlagsProviderFactory { * @returns Default configuration using PostHog */ public static getDefaultConfig(): FeatureFlagsProviderConfig { + const hasValidConfig = isPostHogConfigValid(posthogConfig) return { - type: "posthog", + type: hasValidConfig ? "posthog" : "no-op", } } } @@ -57,26 +59,28 @@ export class FeatureFlagsProviderFactory { * or for testing purposes */ class NoOpFeatureFlagsProvider implements IFeatureFlagsProvider { - public async getFeatureFlag(_flagName: string): Promise { + public async getFeatureFlag(flagName: string): Promise { + Logger.info(`[NoOpFeatureFlagsProvider] getFeatureFlag called with flagName=${flagName}`) return undefined } - public async getFeatureFlagPayload(_flagName: string): Promise { + public async getFeatureFlagPayload(flagName: string): Promise { + Logger.info(`[NoOpFeatureFlagsProvider] getFeatureFlagPayload called with flagName=${flagName}`) return null } public isEnabled(): boolean { - return false + return true } public getSettings() { return { - enabled: false, - timeout: 0, + enabled: true, + timeout: 1000, } } public async dispose(): Promise { - // No-op + Logger.info("[NoOpFeatureFlagsProvider] Disposing") } } diff --git a/src/services/telemetry/TelemetryProviderFactory.ts b/src/services/telemetry/TelemetryProviderFactory.ts index 2ff0bf1473..5969fc2c57 100644 --- a/src/services/telemetry/TelemetryProviderFactory.ts +++ b/src/services/telemetry/TelemetryProviderFactory.ts @@ -1,3 +1,5 @@ +import { isPostHogConfigValid, posthogConfig } from "@/shared/services/config/posthog-config" +import { Logger } from "../logging/Logger" import { PostHogClientProvider } from "../posthog/PostHogClientProvider" import type { ITelemetryProvider } from "./providers/ITelemetryProvider" import { PostHogTelemetryProvider } from "./providers/PostHogTelemetryProvider" @@ -5,7 +7,7 @@ import { PostHogTelemetryProvider } from "./providers/PostHogTelemetryProvider" /** * Supported telemetry provider types */ -export type TelemetryProviderType = "posthog" | "none" +export type TelemetryProviderType = "posthog" | "no-op" /** * Configuration for telemetry providers @@ -26,15 +28,15 @@ export class TelemetryProviderFactory { */ public static async createProvider(config: TelemetryProviderConfig): Promise { // Get the shared PostHog client from PostHogClientProvider - const sharedClient = PostHogClientProvider.getClient() switch (config.type) { - case "posthog": + case "posthog": { + // Get the shared PostHog client from PostHogClientProvider + const sharedClient = PostHogClientProvider.getClient() if (sharedClient) { return await new PostHogTelemetryProvider(sharedClient).initialize() } return new NoOpTelemetryProvider() - case "none": - return new NoOpTelemetryProvider() + } default: console.error(`Unsupported telemetry provider type: ${config.type}`) return new NoOpTelemetryProvider() @@ -46,8 +48,9 @@ export class TelemetryProviderFactory { * @returns Default configuration using PostHog */ public static getDefaultConfig(): TelemetryProviderConfig { + const hasValidConfig = isPostHogConfigValid(posthogConfig) return { - type: "posthog", + type: hasValidConfig ? "posthog" : "no-op", } } } @@ -57,16 +60,19 @@ export class TelemetryProviderFactory { * or for testing purposes */ export class NoOpTelemetryProvider implements ITelemetryProvider { - public log(_event: string, _properties?: Record): void { - // No-op + public isOptIn = true + + public log(event: string, properties?: Record): void { + Logger.log(`[NoOpTelemetryProvider] ${event}: ${JSON.stringify(properties)}`) } - public identifyUser(_userInfo: any, _properties?: Record): void { - // No-op + public identifyUser(userInfo: any, properties?: Record): void { + Logger.info(`[NoOpTelemetryProvider] identifyUser - ${JSON.stringify(userInfo)} - ${JSON.stringify(properties)}`) } - public setOptIn(_optIn: boolean): void { - // No-op + public setOptIn(optIn: boolean): void { + Logger.info(`[NoOpTelemetryProvider] setOptIn(${optIn})`) + this.isOptIn = optIn } public isEnabled(): boolean { @@ -82,6 +88,6 @@ export class NoOpTelemetryProvider implements ITelemetryProvider { } public async dispose(): Promise { - // No-op + Logger.info("[NoOpTelemetryProvider] Disposing") } } diff --git a/src/services/telemetry/TelemetryService.test.ts b/src/services/telemetry/TelemetryService.test.ts index 45c136c7db..6e0374e54f 100644 --- a/src/services/telemetry/TelemetryService.test.ts +++ b/src/services/telemetry/TelemetryService.test.ts @@ -6,15 +6,22 @@ import * as assert from "assert" import * as sinon from "sinon" +import { HostProvider } from "@/hosts/host-provider" +import * as posthogConfigModule from "@/shared/services/config/posthog-config" import { setVscodeHostProviderMock } from "@/test/host-provider-test-utils" -import { NoOpTelemetryProvider, TelemetryProviderFactory, type TelemetryProviderType } from "./TelemetryProviderFactory" +import { NoOpTelemetryProvider, TelemetryProviderFactory, TelemetryProviderType } from "./TelemetryProviderFactory" import { TelemetryService } from "./TelemetryService" describe("Telemetry system is abstracted and can easily switch between providers", () => { + // Setup and teardown for HostProvider mocking before(() => { setVscodeHostProviderMock() }) + after(() => { + // Reset HostProvider after tests + HostProvider.reset() + }) const MOCK_USER_INFO = { id: "test-user-123", email: "test@example.com", @@ -34,7 +41,7 @@ describe("Telemetry system is abstracted and can easily switch between providers describe("Telemetry Service", () => { it("should include correct metadata with telemetry events", async () => { const noOpProvider = await TelemetryProviderFactory.createProvider({ - type: "none", + type: "no-op", }) // Spy on the provider's log method to verify metadata @@ -117,7 +124,7 @@ describe("Telemetry system is abstracted and can easily switch between providers it("should create No-Op provider and handle all operations safely", async () => { console.log("\n=== Testing No-Op Provider ===") const noOpProvider = await TelemetryProviderFactory.createProvider({ - type: "none", + type: "no-op", }) const noOpTelemetryService = new TelemetryService(noOpProvider, MOCK_METADATA) @@ -193,6 +200,9 @@ describe("Telemetry system is abstracted and can easily switch between providers describe("Factory Configuration", () => { it("should return default configuration", () => { + // Mock PostHog config validation to return true for this test + const isPostHogConfigValidStub = sinon.stub(posthogConfigModule, "isPostHogConfigValid").returns(true) + const defaultConfig = TelemetryProviderFactory.getDefaultConfig() assert.deepStrictEqual( @@ -202,6 +212,9 @@ describe("Telemetry system is abstracted and can easily switch between providers }, "Should return PostHog as default configuration", ) + + // Restore the stub + isPostHogConfigValidStub.restore() }) it("should handle provider switching seamlessly", async () => { @@ -220,7 +233,7 @@ describe("Telemetry system is abstracted and can easily switch between providers // Switch to No-Op provider const noOpProvider = await TelemetryProviderFactory.createProvider({ - type: "none", + type: "no-op", }) telemetryService = new TelemetryService(noOpProvider, MOCK_METADATA) diff --git a/src/shared/services/config/posthog-config.ts b/src/shared/services/config/posthog-config.ts index 131fff0dd5..6e9a455404 100644 --- a/src/shared/services/config/posthog-config.ts +++ b/src/shared/services/config/posthog-config.ts @@ -1,30 +1,57 @@ export interface PostHogClientConfig { + /** + * The main API key for PostHog telemetry service. + */ apiKey?: string | undefined + /** + * The API key for PostHog used only for error tracking service. + */ errorTrackingApiKey?: string | undefined host: string uiHost: string } +/** + * Helper type for a valid PostHog client configuration. + * Must contains api keys for both telemetry and error tracking. + */ export interface PostHogClientValidConfig extends PostHogClientConfig { apiKey: string errorTrackingApiKey: string } -// Public PostHog key (safe for open source) -const posthogProdConfig = { - apiKey: "phc_qfOAGxZw2TL5O8p9KYd9ak3bPBFzfjC8fy5L6jNWY7K", - errorTrackingApiKey: "phc_qfOAGxZw2TL5O8p9KYd9ak3bPBFzfjC8fy5L6jNWY7K", - host: "https://data.cline.bot", - uiHost: "https://us.posthog.com", -} satisfies PostHogClientConfig +/** + * NOTE: Ensure that dev environment is not used in production. + * process.env.CI will always be true in the CI environment, during both testing and publishing step, + * so it is not a reliable indicator of the environment. + */ +const useDevEnv = process?.env?.IS_DEV === "true" || process?.env?.CLINE_ENVIRONMENT === "local" -// Public PostHog key for Development Environment project -const posthogDevEnvConfig = { - apiKey: "phc_uY24EJXNBcc9kwO1K8TJUl5hPQntGM6LL1Mtrz0CBD4", - errorTrackingApiKey: "phc_uY24EJXNBcc9kwO1K8TJUl5hPQntGM6LL1Mtrz0CBD4", +/** + * PostHog configuration for Production Environment. + * NOTE: The production environment variables will be injected at build time in CI/CD pipeline. + * IMPORTANT: The secrets must be added to the GitHub Secrets and matched with the environment variables names + * defined in the .github/workflows/publish.yml workflow. + * NOTE: The development environment variables should be retrieved from 1password shared vault. + */ +export const posthogConfig: PostHogClientConfig = { + apiKey: process?.env?.TELEMETRY_SERVICE_API_KEY, + errorTrackingApiKey: process?.env?.ERROR_SERVICE_API_KEY, host: "https://data.cline.bot", - uiHost: "https://us.i.posthog.com", -} satisfies PostHogClientConfig + uiHost: useDevEnv ? "https://us.i.posthog.com" : "https://us.posthog.com", +} -// NOTE: Ensure that dev environment is used when process.env.IS_DEV is "true" -export const posthogConfig = process.env.IS_DEV === "true" ? posthogDevEnvConfig : posthogProdConfig +const isTestEnv = process?.env?.E2E_TEST === "true" || process?.env?.IS_TEST === "true" + +export function isPostHogConfigValid(config: PostHogClientConfig): config is PostHogClientValidConfig { + // Allow invalid config in test environment to enable mocking and stubbing + if (isTestEnv) { + return false + } + return ( + typeof config.apiKey === "string" && + typeof config.errorTrackingApiKey === "string" && + typeof config.host === "string" && + typeof config.uiHost === "string" + ) +} diff --git a/webview-ui/src/config/platform-configs.json b/webview-ui/src/config/platform-configs.json index 3c0f43cd7e..b3a4494d40 100644 --- a/webview-ui/src/config/platform-configs.json +++ b/webview-ui/src/config/platform-configs.json @@ -4,13 +4,13 @@ "showNavbar": false, "postMessageHandler": "vscode", "togglePlanActKeys": "Meta+Shift+a", - "supportsTerminalMentions": true + "supportsTerminalMentions": true }, "standalone": { "messageEncoding": "json", "showNavbar": true, "postMessageHandler": "standalone", "togglePlanActKeys": "Meta+Shift+p", - "supportsTerminalMentions": false + "supportsTerminalMentions": false } } diff --git a/webview-ui/vite.config.ts b/webview-ui/vite.config.ts index c9ea1c6d29..bfb9f34b6c 100644 --- a/webview-ui/vite.config.ts +++ b/webview-ui/vite.config.ts @@ -89,11 +89,18 @@ export default defineConfig({ }, define: { __PLATFORM__: JSON.stringify(platform), - "process.env": { - NODE_ENV: JSON.stringify(process.env.IS_DEV ? "development" : "production"), - IS_DEV: JSON.stringify(process.env.IS_DEV), - IS_TEST: JSON.stringify(process.env.IS_TEST), - }, + process: JSON.stringify({ + env: { + NODE_ENV: process?.env?.IS_DEV ? "development" : "production", + CLINE_ENVIRONMENT: process?.env?.CLINE_ENVIRONMENT ?? "production", + IS_DEV: process?.env?.IS_DEV === "true", + IS_TEST: process?.env?.IS_TEST === "true", + CI: process?.env?.CI === "true", + // PostHog environment variables + TELEMETRY_SERVICE_API_KEY: process?.env?.TELEMETRY_SERVICE_API_KEY, + ERROR_SERVICE_API_KEY: process?.env?.ERROR_SERVICE_API_KEY, + }, + }), }, resolve: { alias: {