preview updates (mock electron api, wos checks)#2986
Conversation
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
WalkthroughThe PR introduces a preview mode across the frontend: adds a mock Electron API and installer, installs it in preview startup, and wires preview-specific global initialization via global-atoms/GlobalModel with a new optional GlobalInitOptions.isPreview flag. The WaveObj store gains preview-object mocking and exported mockObjectForPreview, and backend calls in callBackendService now guard for a missing endpoint. Onboarding InitPage accepts a new telemetryUpdateFn prop and preview examples pass it through. Miscellaneous: broadened ESLint no-unused-vars ignore patterns and a copyright year bump. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Deploying waveterm with
|
| Latest commit: |
ea20da0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://801cb61a.waveterm.pages.dev |
| Branch Preview URL: | https://sawka-preview-updates.waveterm.pages.dev |
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
MAJOR
Resolved IssuesThe following issue was identified in earlier reviews but has been fixed in subsequent commits:
Other Observations (not in diff)Other files using
Files Reviewed (10 files)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/preview/preview-electron-api.ts`:
- Around line 4-61: The previewElectronApi object is missing the required async
method setIsActive defined by the ElectronApi interface; add setIsActive: () =>
Promise<void> to previewElectronApi (e.g., implement a stub that returns
Promise.resolve() or an async no-op) so the object satisfies the type
contract—update the previewElectronApi declaration to include this method
alongside the other functions.
In `@frontend/preview/preview.tsx`:
- Line 138: Replace the unsafe cast "{} as FullConfigType" used in
globalStore.set(getAtoms().fullConfigAtom, {} as FullConfigType) with a
structurally complete default that matches FullConfigType (e.g., include default
presets array and default settings object or use an exported
DEFAULT_FULL_CONFIG). Update the initialization so fullConfig.presets and
fullConfig.settings are defined (either create and use a DEFAULT_FULL_CONFIG
constant or import an existing default) before calling globalStore.set to avoid
runtime property access errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e8547e9e-3db6-401b-ac3e-86e84238962c
📒 Files selected for processing (9)
eslint.config.jsfrontend/app/onboarding/onboarding.tsxfrontend/app/store/client-model.tsfrontend/app/store/global-atoms.tsfrontend/app/store/wos.tsfrontend/preview/preview-electron-api.tsfrontend/preview/preview.tsxfrontend/preview/previews/onboarding.preview.tsxfrontend/types/custom.d.ts
| isPreview: true, | ||
| } as GlobalInitOptions; | ||
| initGlobalAtoms(initOpts); | ||
| globalStore.set(getAtoms().fullConfigAtom, {} as FullConfigType); |
There was a problem hiding this comment.
Initialize fullConfigAtom with a structurally safe default, not {}.
Line 138 sets {} as FullConfigType; preview components that read fullConfig.presets/fullConfig.settings directly can throw at runtime.
🔧 Proposed fix
- globalStore.set(getAtoms().fullConfigAtom, {} as FullConfigType);
+ globalStore.set(
+ getAtoms().fullConfigAtom,
+ {
+ settings: {},
+ presets: {},
+ connections: {},
+ } as FullConfigType
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| globalStore.set(getAtoms().fullConfigAtom, {} as FullConfigType); | |
| globalStore.set( | |
| getAtoms().fullConfigAtom, | |
| { | |
| settings: {}, | |
| presets: {}, | |
| connections: {}, | |
| } as FullConfigType | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/preview/preview.tsx` at line 138, Replace the unsafe cast "{} as
FullConfigType" used in globalStore.set(getAtoms().fullConfigAtom, {} as
FullConfigType) with a structurally complete default that matches FullConfigType
(e.g., include default presets array and default settings object or use an
exported DEFAULT_FULL_CONFIG). Update the initialization so fullConfig.presets
and fullConfig.settings are defined (either create and use a DEFAULT_FULL_CONFIG
constant or import an existing default) before calling globalStore.set to avoid
runtime property access errors.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| usp.set("method", method); | ||
| const url = getWebServerEndpoint() + "/wave/service?" + usp.toString(); | ||
| const webEndpoint = getWebServerEndpoint(); | ||
| if (webEndpoint == null) throw new Error(`cannot call ${methodName}: no web endpoint`); |
There was a problem hiding this comment.
CRITICAL: Null check breaks preview mode - getWebServerEndpoint() intentionally returns null in preview windows (see frontend/util/endpoints.ts:12), but this null check throws an error. This will crash all backend service calls in preview mode.
The fix should either:
- Add an early return for preview windows:
if (isPreviewWindow()) return Promise.reject(new Error(...)); - Or remove the null check and let it fail with a more descriptive error downstream.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.kilocode/skills/electron-api/SKILL.md (1)
58-66: Documentation correctly demonstrates the preview stub pattern.The example properly shows a no-op stub that matches the async signature, and the section numbering is adjusted accordingly.
Optional: Consider adding preview stub examples for other patterns.
For consistency and completeness, you could add preview stub examples for the sync and fire-and-forget patterns shown later in the document. For example:
- Sync:
getUserName: () => "unknown"- Fire-and-forget:
openExternal: (_url: string) => {}This would help developers understand how to stub different return types and communication patterns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kilocode/skills/electron-api/SKILL.md around lines 58 - 66, Add optional preview-stub examples for the sync and fire-and-for-get patterns to the SKILL.md section (alongside the existing captureScreenshot async stub) so readers see all patterns; specifically, show a sync stub like getUserName that returns a default string and a fire-and-forget stub like openExternal that is an empty function, placing them near the "Add Preview Stub" example for consistency and clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.kilocode/skills/electron-api/SKILL.md:
- Around line 58-66: Add optional preview-stub examples for the sync and
fire-and-for-get patterns to the SKILL.md section (alongside the existing
captureScreenshot async stub) so readers see all patterns; specifically, show a
sync stub like getUserName that returns a default string and a fire-and-forget
stub like openExternal that is an empty function, placing them near the "Add
Preview Stub" example for consistency and clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3aa87407-6089-4e91-bb00-add589a4ff29
📒 Files selected for processing (1)
.kilocode/skills/electron-api/SKILL.md
No description provided.