merge dev to main (v2.19.1) #2256
Conversation
📝 WalkthroughWalkthroughAdds VS Code telemetry integration (new module, activation/sign-in instrumentation), introduces a save workflow for ZModel preview output, updates release notes text, and augments the schema build to inject a telemetry token and copy resources post-build. Also bumps the JetBrains IDE plugin version to 2.19.1. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant VSCode as VS Code
participant Extension as Extension (activation)
participant Telemetry as Telemetry
Note over VSCode,Extension: Activation
VSCode->>Extension: activate()
Extension->>Telemetry: track("extension:activate")
Note over Extension: Extension ready
Note over User,Extension: Sign-in flow
User->>Extension: Trigger sign-in
Extension->>Telemetry: track("auth:signin:show")
Extension->>Telemetry: track("auth:signin:start")
Extension->>Extension: Perform auth
alt success
Extension->>Telemetry: track("auth:signin:complete")
Extension-->>User: Signed in
else error
Extension->>Telemetry: track("auth:signin:error",{message})
Extension-->>User: Error message
end
sequenceDiagram
autonumber
actor User
participant Preview as ZModelPreview
participant FS as File System
participant Telemetry as Telemetry
participant VSCode as VS Code UI
Note over User,Preview: Generate preview
User->>Preview: Generate documentation
Preview->>Telemetry: track("zmodel:preview")
Preview->>Preview: Store lastGeneratedMarkdown
Preview->>VSCode: openMarkdownPreview(temp "zmodel-preview.md")
Note over User,Preview: Save documentation
User->>Preview: Command zenstack.save-zmodel-documentation
Preview->>Telemetry: track("zmodel:save:start")
Preview->>VSCode: Show Save Dialog
alt user cancels
Preview-->>User: Cancelled
Preview->>Telemetry: track("zmodel:save:cancel")
else path selected
Preview->>FS: Write lastGeneratedMarkdown to chosen path
alt write ok
Preview->>Telemetry: track("zmodel:save:complete")
Preview->>VSCode: Refresh UI / notifications
else write error
Preview-->>User: Error message
Preview->>Telemetry: track("zmodel:save:error",{message})
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/schema/src/vscode/zenstack-auth-provider.ts (2)
199-217: JWT decode usesatob(not available in Node) and doesn’t handle base64urlIn VS Code’s extension host (Node),
atobis undefined and JWT payloads are base64url-encoded. Decode withBuffer+ base64url normalization; also keep padding logic.Apply:
- const paddedPayload = payload + '='.repeat((4 - (payload.length % 4)) % 4); - const decoded = atob(paddedPayload); - - return JSON.parse(decoded); + const base64url = payload.replace(/-/g, '+').replace(/_/g, '/'); + const padded = base64url + '='.repeat((4 - (base64url.length % 4)) % 4); + const decoded = Buffer.from(padded, 'base64').toString('utf8'); + return JSON.parse(decoded);
184-194: Refactor identify call to avoid PII and guard undefined
Replace the raw-email identify and non-null assertion in createSessionFromAccessToken with a stable non-PII id (sub or jti), falling back to a SHA-256 hashed email. The telemetry module already skips all tracking whenvscode.env.isTelemetryEnabledis false.--- a/packages/schema/src/vscode/zenstack-auth-provider.ts +++ b/packages/schema/src/vscode/zenstack-auth-provider.ts @@ private async createSessionFromAccessToken(accessToken: string): Promise<vscode.AuthenticationSession> { - telemetry.identify(claims.email!); + // Identify a stable, non-PII user id; fallback to hashed email if no sub/jti + const distinctId = claims.sub ?? claims.jti; + if (distinctId) { + telemetry.identify(distinctId); + } else if (claims.email) { + const { createHash } = await import('crypto'); + const hashedEmail = createHash('sha256').update(claims.email).digest('hex'); + telemetry.identify(hashedEmail); + }packages/schema/src/vscode/zmodel-preview.ts (2)
194-201: Windows path bug: use fsPath when converting URIsUsing
vscode.Uri.file(uri.path)will mis-handle Windows paths (e.g.,/c:/...). Use the filesystem path fromvscode-uri’sfsPathto construct a propervscode.Uri.Apply:
- const fileUri = vscode.Uri.file(uri.path); - const fileContent = await vscode.workspace.fs.readFile(fileUri); - const filePath = fileUri.path; + const fileUri = vscode.Uri.file(uri.fsPath); + const fileContent = await vscode.workspace.fs.readFile(fileUri); + const filePath = fileUri.fsPath;
1-11: Import TextEncoder from 'util' to avoid TS/runtime issuesIn VS Code extensions,
TextEncodertyping may be missing unlessdomlib is enabled; importing fromutilis the safe, cross‑Node choice.import * as os from 'os'; +import { TextEncoder } from 'util';Also applies to: 272-279
🧹 Nitpick comments (14)
packages/schema/src/vscode/zenstack-auth-provider.ts (3)
168-173: Preserve requested scopes on session creationScopes collected in
this.pendingAuthare dropped; session objects always get[]. Pass scopes through for correctness.Apply:
- private async createSessionFromAccessToken(accessToken: string): Promise<vscode.AuthenticationSession> { + private async createSessionFromAccessToken( + accessToken: string, + scopes: readonly string[] = [] + ): Promise<vscode.AuthenticationSession> { ... - return { + return { id: claims.jti || Math.random().toString(36), accessToken: accessToken, account: { id: claims.sub || 'unknown', label: claims.email || 'unknown@zenstack.dev', }, - scopes: [], + scopes: Array.from(scopes), };And in the callback:
- const session = await this.createSessionFromAccessToken(accessToken); + const session = await this.createSessionFromAccessToken(accessToken, this.pendingAuth?.scopes ?? []);Also applies to: 192-193
221-236: Logout awaits aren’t honored; show message after removals complete
forEach(async ...)isn’t awaited, so the info message can show before sessions are removed. UsePromise.all.Apply:
async logoutAllSessions(): Promise<void> { if (this._sessions.length === 0) { return; } - (await this.getSessions()).forEach(async (s) => await this.removeSession(s.id)); - vscode.window.showInformationMessage('Successfully logged out of ZenStack.'); + const sessions = await this.getSessions(); + await Promise.all(sessions.map((s) => this.removeSession(s.id))); + vscode.window.showInformationMessage('Successfully logged out of ZenStack.'); }Also applies to: 87-94
199-206: Consider validating token expiry (exp) before creating a sessionPrevent creating unusable sessions with expired tokens; fail fast with a clear message.
Example:
const parts = token.split('.'); ... - return JSON.parse(decoded); + const claims = JSON.parse(decoded) as JWTClaims; + if (typeof claims.exp === 'number' && Date.now() / 1000 >= claims.exp) { + throw new Error('Access token expired'); + } + return claims;packages/schema/src/extension.ts (2)
21-26: Emit “signin:show” before awaiting the modalTrack the “show” event before
showWarningMessageto reflect actual prompt display timing.- const selection = await vscode.window.showWarningMessage('Please sign in to use this feature', signIn); - telemetry.track('extension:signin:show'); + telemetry.track('extension:signin:show'); + const selection = await vscode.window.showWarningMessage('Please sign in to use this feature', signIn);
45-47: Add useful context to activation telemetryInclude extension and VS Code versions for diagnostics.
- telemetry.track('extension:activate'); + telemetry.track('extension:activate', { + extensionVersion: context.extension.packageJSON.version, + vscodeVersion: vscode.version, + platform: process.platform, + arch: process.arch, + });packages/schema/src/vscode/res/zmodel-preview-release-notes.html (1)
66-75: Verify codicon names and add accessible labels
- Confirm
codicon-previewandcodicon-saveexist in the referenced codicons version; otherwise icons won’t render.- Add
aria-label/titlefor screen readers.Example:
Click (<span class="codicon codicon-save" aria-label="Save" title="Save"></span>) ...To check rendering locally, open the preview and confirm icons appear.
packages/schema/build/bundle.js (2)
16-17: Replace all placeholder occurrences, not just the first
String.replacereplaces only the first match. UsereplaceAllor a global regex.- content = content.replace('<VSCODE_TELEMETRY_TRACKING_TOKEN>', telemetryToken); + content = content.replaceAll('<VSCODE_TELEMETRY_TRACKING_TOKEN>', telemetryToken);
5-5:pathis unusedRemove unused import.
-const path = require('path');packages/schema/src/vscode/zmodel-preview.ts (3)
324-335: Avoid auto-closing the editor after save; provide actionable toast insteadOpening and immediately closing the saved file is surprising and can close the wrong editor. Offer “Open” / “Reveal” actions instead.
- // Open and close the saved file to refresh the shown markdown preview - await vscode.commands.executeCommand('vscode.open', saveUri); - await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); + const choice = await vscode.window.showInformationMessage( + `Documentation saved: ${saveUri.fsPath}`, + 'Open', + 'Reveal in Explorer' + ); + if (choice === 'Open') { + await vscode.commands.executeCommand('vscode.open', saveUri); + } else if (choice === 'Reveal in Explorer') { + await vscode.commands.executeCommand('revealFileInOS', saveUri); + }
53-64: Initialize markdown-preview context on activationContext is only updated on tab changes; initialize it once so keybindings/UI dependent on
zenstack.isMarkdownPreviewwork immediately.context.subscriptions.push( vscode.window.tabGroups.onDidChangeTabs(() => { - const activeTabLabels = vscode.window.tabGroups.all.filter((group) => + const activeTabLabels = vscode.window.tabGroups.all.filter((group) => group.activeTab?.label?.endsWith(this.previewZModelFileName) ); if (activeTabLabels.length > 0) { vscode.commands.executeCommand('setContext', 'zenstack.isMarkdownPreview', true); } else { vscode.commands.executeCommand('setContext', 'zenstack.isMarkdownPreview', false); } }) ); + // initialize once + vscode.commands.executeCommand( + 'setContext', + 'zenstack.isMarkdownPreview', + vscode.window.tabGroups.all.some((g) => g.activeTab?.label?.endsWith(this.previewZModelFileName)) + );
8-8: Potential circular dependency withrequireAuthfrom../extensionImporting from the activation module can create cycles (activation ⇒ preview ⇒ extension). Consider extracting
requireAuthto a shared auth module.Could you confirm there’s no load/activation cycle here?
packages/schema/src/vscode/vscode-telemetry.ts (3)
31-35: Remove unsupportedgeolocateinit option
mixpanelNode client doesn’t use ageolocateinit flag. Remove it to avoid confusion.- this.mixpanel = init(VSCODE_TELEMETRY_TRACKING_TOKEN, { - geolocate: true, - }); + this.mixpanel = init(VSCODE_TELEMETRY_TRACKING_TOKEN);
18-29: Avoid heavy work at module import; lazily init the client/device idConstructing the singleton computes machine ID at import time. Consider lazy init to reduce activation latency and potential I/O during extension load.
If you keep the singleton, defer
getDeviceId()to firsttrack/identifycall and cache the result.Also applies to: 75-75
44-63: Addip: 0to payload for IP suppression
Per Mixpanel Node.js SDK, settingip: 0in the event payload skips GeoIP lookup for that event.@@ packages/schema/src/vscode/vscode-telemetry.ts:44-63 const payload = { distinct_id: this.deviceId, time: new Date(), $os: this._os_type, osType: this._os_type, osRelease: this._os_release, osPlatform: this._os_platform, osArch: this._os_arch, osVersion: this._os_version, nodeVersion: process.version, vscodeAppName: this.vscodeAppName, vscodeVersion: this.vscodeVersion, vscodeAppHost: this.vscodeAppHost, + ip: 0, ...properties, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
package.jsonis excluded by!**/*.jsonpackages/ide/jetbrains/package.jsonis excluded by!**/*.jsonpackages/language/package.jsonis excluded by!**/*.jsonpackages/misc/redwood/package.jsonis excluded by!**/*.jsonpackages/plugins/openapi/package.jsonis excluded by!**/*.jsonpackages/plugins/swr/package.jsonis excluded by!**/*.jsonpackages/plugins/tanstack-query/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/package.jsonis excluded by!**/*.jsonpackages/runtime/package.jsonis excluded by!**/*.jsonpackages/schema/package.jsonis excluded by!**/*.jsonpackages/schema/tsconfig.jsonis excluded by!**/*.jsonpackages/sdk/package.jsonis excluded by!**/*.jsonpackages/server/package.jsonis excluded by!**/*.jsonpackages/testtools/package.jsonis excluded by!**/*.json
📒 Files selected for processing (7)
packages/ide/jetbrains/build.gradle.kts(1 hunks)packages/schema/build/bundle.js(2 hunks)packages/schema/src/extension.ts(3 hunks)packages/schema/src/vscode/res/zmodel-preview-release-notes.html(1 hunks)packages/schema/src/vscode/vscode-telemetry.ts(1 hunks)packages/schema/src/vscode/zenstack-auth-provider.ts(2 hunks)packages/schema/src/vscode/zmodel-preview.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/schema/src/extension.ts (1)
packages/schema/src/vscode/zenstack-auth-provider.ts (1)
AUTH_PROVIDER_ID(11-11)
packages/schema/src/vscode/zmodel-preview.ts (1)
packages/schema/src/vscode/documentation-cache.ts (1)
DocumentationCache(15-152)
packages/schema/src/vscode/vscode-telemetry.ts (3)
packages/schema/bin/post-install.js (3)
Mixpanel(6-6)os(8-8)payload(16-28)packages/schema/src/utils/machine-id-utils.ts (1)
getMachineId(63-74)packages/schema/src/global.d.ts (1)
TELEMETRY_TRACKING_TOKEN(2-2)
⏰ 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). (5)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
🔇 Additional comments (5)
packages/ide/jetbrains/build.gradle.kts (1)
12-12: Version bump to 2.19.1 looks goodMatches the PR title and no behavioral changes here.
packages/schema/src/extension.ts (1)
5-10: Telemetry module correctly respects VS Code telemetry settings
VSCodeTelemetryonly initializes Mixpanel whenvscode.env.isTelemetryEnabledis true, and bothtrackandidentifyguard onthis.mixpanel, making them no-ops when telemetry is disabled.packages/schema/src/vscode/zenstack-auth-provider.ts (1)
32-41: Manual verification required: ensure extension manifest includes onUri activation and uriHandler
I wasn’t able to locate anypackage.jsonin the workspace. Please confirm that your VS Code extension’s manifest declares
"activationEvents": ["onUri"]- a
"uriHandler"contribution
so that the/auth-callbackroute is correctly wired up.packages/schema/build/bundle.js (1)
36-37: No resource collisions detected betweensrc/resandsrc/vscode/res– I compared the relative paths in both directories and found no overlapping files, so the current copy operations intobundle/reswon’t overwrite each other.packages/schema/src/vscode/zmodel-preview.ts (1)
239-247: Confirm global fetch support in the extension host
Your code callsfetch, which relies on Node’s global fetch (available only in Node ≥ 18). Verify that your extension’spackage.jsondefines anengines.vscoderange targeting a VS Code/Electron build with built-infetchsupport, or add a fallback import (e.g. dynamic import ofnode-fetch) inzmodel-preview.ts.
No description provided.