tsunami / builder updates (jsfuncs, devtools, gpt-5.4, etc)#3226
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughMain process and BrowserWindow handlers now close DevTools for the builder window and any embedded webview guests on close. IPC handler for "close-builder-window" also closes DevTools before destroying the window. Frontend builder adds a webviewRef, an Open DevTools kebab menu action, openPreviewDevTools(), and a persistent preview webview with lastKnownUrl plus overlay states. New builder-specific AI modes ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: |
d63a1d7
|
| Status: | ⚡️ Build in progress... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
emain/emain-window.ts (1)
295-332:⚠️ Potential issue | 🟡 MinorDevTools are closed even when the user cancels the close confirmation.
closeAllDevTools()is invoked unconditionally at the top of theclosehandler, before thewindow:confirmclosedialog runs insidefireAndForget. If the user clicks "Cancel" in the unsaved-tabs confirmation dialog, the window stays open but all of their DevTools instances have already been dismissed and their inspector state/breakpoints/console history is lost. Consider deferringthis.closeAllDevTools()until after the confirmation resolves andthis.canClose = trueis about to be set (i.e., right before the finalthis.close()), or to the"closed"handler if the timing still works for avoiding the underlying crash.Proposed restructuring
this.on("close", (e) => { if (this.canClose) { return; } if (this.isDestroyed()) { return; } - this.closeAllDevTools(); console.log("win 'close' handler fired", this.waveWindowId); if (getGlobalIsQuitting() || updater?.status == "installing" || getGlobalIsRelaunching()) { + this.closeAllDevTools(); return; } e.preventDefault(); fireAndForget(async () => { const numWindows = waveWindowMap.size; const fullConfig = await RpcApi.GetFullConfigCommand(ElectronWshClient); if (numWindows > 1 || !fullConfig.settings["window:savelastwindow"]) { if (fullConfig.settings["window:confirmclose"]) { const workspace = await WorkspaceService.GetWorkspace(this.workspaceId); if (isNonEmptyUnsavedWorkspace(workspace)) { const choice = dialog.showMessageBoxSync(this, { ... }); if (choice === 0) { return; } } } this.deleteAllowed = true; } + this.closeAllDevTools(); this.canClose = true; this.close(); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emain/emain-window.ts` around lines 295 - 332, The handler currently calls this.closeAllDevTools() unconditionally at the top which dismisses DevTools even when the user cancels the confirmation; move the this.closeAllDevTools() call out of the start of the "close" handler and invoke it only after the confirmation has resolved and just before calling this.close() (i.e., inside the fireAndForget flow immediately before setting this.canClose = true and this.close()), or alternatively invoke it from the "closed" handler — adjust references to this.canClose, this.deleteAllowed, dialog.showMessageBoxSync, isNonEmptyUnsavedWorkspace and fireAndForget accordingly so DevTools are only closed when the window is actually going to be closed.
🧹 Nitpick comments (5)
frontend/app/aipanel/waveai-model.tsx (1)
44-63: FrontendBuilderAIModeConfigsduplicates backend metadata — keep in sync.This hard-coded map mirrors
builderModeConfigsdefined inpkg/aiusechat/usechat-mode.go(display name/order/icon/description, provider, switchcompat, premium). Any future change to the backend builder mode metadata (e.g., description, adding a third mode, renaming) will need to be mirrored here manually, otherwise builder UI and runtime will drift. Consider either:
- Sourcing this via the same config broadcast mechanism (
atoms.waveaiModeConfigAtom) by having the backend also publish builder modes, or- Adding a small comment pointing at
pkg/aiusechat/usechat-mode.goso future edits are paired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/aipanel/waveai-model.tsx` around lines 44 - 63, BuilderAIModeConfigs is a hard-coded duplicate of backend builderModeConfigs which will drift; fix by replacing the static map with a single source of truth (subscribe to atoms.waveaiModeConfigAtom or equivalent published config from the backend) or at minimum add a comment referencing the backend builderModeConfigs location so maintainers update both; update the code that currently exports/uses BuilderAIModeConfigs to read from atoms.waveaiModeConfigAtom (or call into the published config provider) and remove/replace the hard-coded entries ("waveaibuilder@default", "waveaibuilder@deep") so UI derives display:name, display:order, display:icon, display:description, ai:provider, ai:switchcompat, and waveai:premium from the backend-published config.frontend/app/view/webview/webview.tsx (1)
954-961: LGTM — butuseEffectwould suffice.Cleanup doesn't read layout, so
useEffectis semantically equivalent here and avoids blocking paint. Not a correctness issue;useLayoutEffect's cleanup runs at the same point during unmount asuseEffect's.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/view/webview/webview.tsx` around lines 954 - 961, Replace useLayoutEffect with useEffect for the devtools cleanup: in the component where useLayoutEffect(() => { return () => { const webview = model.webviewRef.current; if (webview?.isDevToolsOpened()) { webview.closeDevTools(); } }; }, []); is used, change the hook to useEffect and keep the same cleanup logic (ensure React's useEffect is imported/available). This keeps the same unmount cleanup behavior while avoiding layout-effect usage.frontend/builder/tabs/builder-previewtab.tsx (2)
182-182: Nit: widenuseStatetype to includenull.
useState<string>(null)is inconsistent with the initial value. Minor, but it propagates a misleading non-nullable type.♻️ Suggested change
- const [lastKnownUrl, setLastKnownUrl] = useState<string>(null); + const [lastKnownUrl, setLastKnownUrl] = useState<string | null>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/builder/tabs/builder-previewtab.tsx` at line 182, The state declaration uses useState<string>(null) but initializes with null; change the generic to allow null to match the initial value by updating the hook call to useState<string | null>(null) so lastKnownUrl and setLastKnownUrl have the correct nullable type; locate the useState usage for lastKnownUrl in builder-previewtab.tsx and adjust the generic accordingly.
185-192:isWebViewActivereturns a truthy number, not a boolean, and setState-in-render pattern noted.Two small observations:
status === "running" && builderStatus?.port && builderStatus.port !== 0evaluates tonumber | boolean | undefined. It works in boolean contexts, but the downstreamvisibility: isWebViewActive ? "visible" : "hidden"etc. read cleaner if coerced. ConsiderBoolean(...)or!!builderStatus?.portwith port > 0.- Calling
setLastKnownUrlduring render is fine since it's guarded by inequality (React allows this), but ifbuilderStatus.portever flips back to a previous value, the webview will happily reload mid-session. Once the first URL is captured, a stableuseRef+useEffectmight express the "sticky last URL" intent more clearly. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/builder/tabs/builder-previewtab.tsx` around lines 185 - 192, The boolean expression assigned to isWebViewActive can be a number/undefined; change it to an explicit boolean (e.g., !!builderStatus?.port && builderStatus.port > 0) so usages like visibility: isWebViewActive ? "visible" : "hidden" are clear, and avoid calling setLastKnownUrl during render by making the "sticky last URL" stable: move the preview URL capture into a useEffect that reads builderStatus.port and builderId and writes to either a useRef (for truly sticky behavior) or setLastKnownUrl only when lastKnownUrl is empty, referencing isWebViewActive, lastKnownUrl, setLastKnownUrl, builderStatus.port, previewUrl, and useEffect/useRef to locate the code.frontend/builder/store/builder-apppanel-model.ts (1)
10-10: Prefer a type-only import forWebviewTag.
WebviewTagis used purely as a type here (webviewRef: { current: WebviewTag | null }). Using a value import from"electron"risks the bundler attempting to resolve the electron module in the renderer bundle depending on tsconfigimportsNotUsedAsValues/verbatimModuleSyntaxsettings. Make the import type-only for safety.♻️ Suggested change
-import { WebviewTag } from "electron"; +import type { WebviewTag } from "electron";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/builder/store/builder-apppanel-model.ts` at line 10, The import of WebviewTag should be type-only to avoid bundlers resolving the electron module at runtime; change the import to a type import (replace the current import of WebviewTag with a type-only import) and keep all usages like webviewRef: { current: WebviewTag | null } unchanged so it only affects the import; ensure you update the import statement that references WebviewTag accordingly.
🤖 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/app/aipanel/waveai-model.tsx`:
- Around line 143-147: Change the sentinel returned for the
builder+telemetry-disabled case in defaultModeAtom from "invalid" to "unknown"
so it matches the non-builder path (update the branch inside
this.defaultModeAtom where it currently returns "invalid"). Also add an explicit
guard inside setAIModeToDefault() to validate the value from
globalStore.get(this.defaultModeAtom) (use isValidMode) and, if invalid,
fallback to "unknown" (or another safe default) instead of propagating an
invalid mode to downstream code.
In `@pkg/aiusechat/usechat.go`:
- Around line 673-674: The current logic sets premium := shouldUsePremium() ||
builderMode which forces premium for any request with req.BuilderId, bypassing
resolveAIMode and rate-limit fallback; change this to only force premium when
the request is both from a builder (req.BuilderId set) and the requested AIMode
is a builder-only cloud mode (e.g., aiMode startsWith "waveaibuilder@" or use a
helper isBuilderOnlyMode(req.AIMode)), otherwise leave premium based solely on
shouldUsePremium() so non-builder WaveAI cloud modes (like "waveai@balanced")
still go through resolveAIMode/rate-limit fallback; update usages in
resolveAIMode/any downstream logic that relies on premium.
In `@tsunami/frontend/src/vdom.tsx`:
- Around line 200-212: The JS handler call is missing the second `elem`
parameter described by the Go-side `JSFunc`/`CombinedFunc` contract; update the
invocation in this block so the evaluated function receives both event and
element (e.g., pass `elem` alongside `...args`) or centralize the fix in
`vdom.go` as noted in the comment; ensure you adjust the call site that uses
`fnDecl.jscode` (the `fn(...args)` invocation) so the `elem` is provided, and
keep the `model.callVDomFunc(fnDecl, e, compId, propName)` behavior unchanged;
optionally consider replacing `eval(fnDecl.jscode)` with `new
Function("e","elem", body)` if the `jscode` format is changed to a bare function
body.
In `@tsunami/vdom/vdom.go`:
- Around line 102-112: The docstrings claim the JS handler receives (e, elem)
but runtime calls fn(...args) without supplying elem; fix this by wiring the
element/ref into the args before invoking the JS function: modify
convertVDomFunc (in tsunami/frontend/src/vdom.tsx) so when it builds the args
for fn(...args) it appends the DOM element or waveid/ref (the same value
described as "elem" in the comments) for event handlers and non-event handlers
as appropriate, ensuring JSFunc and CombinedFunc users get (e, elem) as
advertised.
---
Outside diff comments:
In `@emain/emain-window.ts`:
- Around line 295-332: The handler currently calls this.closeAllDevTools()
unconditionally at the top which dismisses DevTools even when the user cancels
the confirmation; move the this.closeAllDevTools() call out of the start of the
"close" handler and invoke it only after the confirmation has resolved and just
before calling this.close() (i.e., inside the fireAndForget flow immediately
before setting this.canClose = true and this.close()), or alternatively invoke
it from the "closed" handler — adjust references to this.canClose,
this.deleteAllowed, dialog.showMessageBoxSync, isNonEmptyUnsavedWorkspace and
fireAndForget accordingly so DevTools are only closed when the window is
actually going to be closed.
---
Nitpick comments:
In `@frontend/app/aipanel/waveai-model.tsx`:
- Around line 44-63: BuilderAIModeConfigs is a hard-coded duplicate of backend
builderModeConfigs which will drift; fix by replacing the static map with a
single source of truth (subscribe to atoms.waveaiModeConfigAtom or equivalent
published config from the backend) or at minimum add a comment referencing the
backend builderModeConfigs location so maintainers update both; update the code
that currently exports/uses BuilderAIModeConfigs to read from
atoms.waveaiModeConfigAtom (or call into the published config provider) and
remove/replace the hard-coded entries ("waveaibuilder@default",
"waveaibuilder@deep") so UI derives display:name, display:order, display:icon,
display:description, ai:provider, ai:switchcompat, and waveai:premium from the
backend-published config.
In `@frontend/app/view/webview/webview.tsx`:
- Around line 954-961: Replace useLayoutEffect with useEffect for the devtools
cleanup: in the component where useLayoutEffect(() => { return () => { const
webview = model.webviewRef.current; if (webview?.isDevToolsOpened()) {
webview.closeDevTools(); } }; }, []); is used, change the hook to useEffect and
keep the same cleanup logic (ensure React's useEffect is imported/available).
This keeps the same unmount cleanup behavior while avoiding layout-effect usage.
In `@frontend/builder/store/builder-apppanel-model.ts`:
- Line 10: The import of WebviewTag should be type-only to avoid bundlers
resolving the electron module at runtime; change the import to a type import
(replace the current import of WebviewTag with a type-only import) and keep all
usages like webviewRef: { current: WebviewTag | null } unchanged so it only
affects the import; ensure you update the import statement that references
WebviewTag accordingly.
In `@frontend/builder/tabs/builder-previewtab.tsx`:
- Line 182: The state declaration uses useState<string>(null) but initializes
with null; change the generic to allow null to match the initial value by
updating the hook call to useState<string | null>(null) so lastKnownUrl and
setLastKnownUrl have the correct nullable type; locate the useState usage for
lastKnownUrl in builder-previewtab.tsx and adjust the generic accordingly.
- Around line 185-192: The boolean expression assigned to isWebViewActive can be
a number/undefined; change it to an explicit boolean (e.g.,
!!builderStatus?.port && builderStatus.port > 0) so usages like visibility:
isWebViewActive ? "visible" : "hidden" are clear, and avoid calling
setLastKnownUrl during render by making the "sticky last URL" stable: move the
preview URL capture into a useEffect that reads builderStatus.port and builderId
and writes to either a useRef (for truly sticky behavior) or setLastKnownUrl
only when lastKnownUrl is empty, referencing isWebViewActive, lastKnownUrl,
setLastKnownUrl, builderStatus.port, previewUrl, and useEffect/useRef to locate
the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 433f9b32-9f01-485b-b925-9cedaa07b95e
📒 Files selected for processing (19)
emain/emain-builder.tsemain/emain-ipc.tsemain/emain-window.tsfrontend/app/aipanel/waveai-model.tsxfrontend/app/view/webview/webview.tsxfrontend/builder/builder-apppanel.tsxfrontend/builder/store/builder-apppanel-model.tsfrontend/builder/tabs/builder-previewtab.tsxpkg/aiusechat/uctypes/uctypes.gopkg/aiusechat/usechat-mode.gopkg/aiusechat/usechat.gotsunami/frontend/src/app.tsxtsunami/frontend/src/tailwind.csstsunami/frontend/src/types/custom.d.tstsunami/frontend/src/types/vdom.d.tstsunami/frontend/src/util/keyutil.tstsunami/frontend/src/vdom.tsxtsunami/vdom/vdom.gotsunami/vdom/vdom_types.go
| this.defaultModeAtom = jotai.atom((get) => { | ||
| const telemetryEnabled = get(getSettingsKeyAtom("telemetry:enabled")) ?? false; | ||
| if (this.inBuilder) { | ||
| return telemetryEnabled ? "waveai@balanced" : "invalid"; | ||
| return telemetryEnabled ? "waveaibuilder@default" : "invalid"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | head -20Repository: wavetermdev/waveterm
Length of output: 662
🏁 Script executed:
find frontend/app/aipanel -name "*.tsx" -o -name "*.ts" | head -20Repository: wavetermdev/waveterm
Length of output: 760
🏁 Script executed:
cat -n frontend/app/aipanel/waveai-model.tsx | head -160 | tail -50Repository: wavetermdev/waveterm
Length of output: 2515
🏁 Script executed:
wc -l frontend/app/aipanel/waveai-model.tsxRepository: wavetermdev/waveterm
Length of output: 105
🏁 Script executed:
rg -n "isValidMode|BuilderAIModeConfigs" frontend/app/aipanel/ -A 3Repository: wavetermdev/waveterm
Length of output: 2113
🏁 Script executed:
rg -n "const.*isValidMode|function.*isValidMode|export.*isValidMode" frontend/ -A 5Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
cat -n frontend/app/aipanel/aimode.tsx | head -100Repository: wavetermdev/waveterm
Length of output: 4536
🏁 Script executed:
cat -n frontend/app/aipanel/waveai-model.tsx | sed -n '44,55p'Repository: wavetermdev/waveterm
Length of output: 629
🏁 Script executed:
cat -n frontend/app/aipanel/waveai-model.tsx | sed -n '421,435p'Repository: wavetermdev/waveterm
Length of output: 636
🏁 Script executed:
rg -n "return.*isValidMode|const.*modes" frontend/app/aipanel/waveai-model.tsx -A 2 -B 2Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
cat -n frontend/app/aipanel/waveai-model.tsx | sed -n '100,110p'Repository: wavetermdev/waveterm
Length of output: 633
🏁 Script executed:
rg -n "defaultModeAtom" frontend/app/aipanel/waveai-model.tsxRepository: wavetermdev/waveterm
Length of output: 382
🏁 Script executed:
rg -n "setAIModeToDefault|currentAIMode" frontend/app/aipanel/waveai-model.tsx -A 3 | head -50Repository: wavetermdev/waveterm
Length of output: 1353
🏁 Script executed:
cat -n frontend/app/aipanel/waveai-model.tsx | sed -n '165,175p'Repository: wavetermdev/waveterm
Length of output: 413
🏁 Script executed:
rg -n "telemetryrequired|TelemetryRequired" frontend/app/aipanel/ -i -A 2Repository: wavetermdev/waveterm
Length of output: 1232
🏁 Script executed:
cat -n frontend/app/aipanel/telemetryrequired.tsx | head -50Repository: wavetermdev/waveterm
Length of output: 2718
🏁 Script executed:
cat -n frontend/app/aipanel/aipanel.tsx | sed -n '580,610p'Repository: wavetermdev/waveterm
Length of output: 1736
🏁 Script executed:
cat -n frontend/app/aipanel/aipanel.tsx | sed -n '1,50p'Repository: wavetermdev/waveterm
Length of output: 2760
🏁 Script executed:
rg -n "telemetry.*builder|builder.*telemetry" frontend/app/aipanel/ -i -B 2 -A 2Repository: wavetermdev/waveterm
Length of output: 551
🏁 Script executed:
rg -n "allowAccess" frontend/app/aipanel/aipanel.tsx -B 5 -A 2 | head -60Repository: wavetermdev/waveterm
Length of output: 1886
🏁 Script executed:
cat -n frontend/app/aipanel/waveai-model.tsx | sed -n '460,500p'Repository: wavetermdev/waveterm
Length of output: 1814
🏁 Script executed:
rg -n "SetRTInfoCommand|req\.AIMode|invalid" frontend/app/aipanel/waveai-model.tsx -B 2 -A 2Repository: wavetermdev/waveterm
Length of output: 1330
🏁 Script executed:
rg -n "setMessage.*mode|mode.*backend|AIMode" frontend/app/aipanel/ -A 3 | head -80Repository: wavetermdev/waveterm
Length of output: 5799
🏁 Script executed:
rg -n "currentAIMode|useChat|AIMode" frontend/app/aipanel/aipanel.tsx -B 2 -A 2 | head -100Repository: wavetermdev/waveterm
Length of output: 2451
🏁 Script executed:
rg -n "getUseChatEndpointUrl|reloadChatFromBackend" frontend/app/aipanel/waveai-model.tsx -A 10 | head -80Repository: wavetermdev/waveterm
Length of output: 1806
🏁 Script executed:
cat -n frontend/app/aipanel/waveai-model.tsx | sed -n '275,290p'Repository: wavetermdev/waveterm
Length of output: 690
🏁 Script executed:
rg -n "unknown" frontend/app/aipanel/waveai-model.tsx -B 2 -A 2Repository: wavetermdev/waveterm
Length of output: 552
🏁 Script executed:
find . -name "*.go" -o -name "*.ts" | xargs grep -l "invalid.*AI.*mode|AIMode.*invalid" 2>/dev/null | head -5Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
rg -n "aimode" frontend/app/aipanel/aipanel.tsx -B 3 -A 3Repository: wavetermdev/waveterm
Length of output: 837
🏁 Script executed:
find . -path ./node_modules -prune -o -name "*.go" -type f -exec grep -l "invalid.*mode\|AIMode" {} \; | head -10Repository: wavetermdev/waveterm
Length of output: 815
🏁 Script executed:
rg -n "post-chat-message" . --type go -A 20 | head -100Repository: wavetermdev/waveterm
Length of output: 1658
🏁 Script executed:
rg -n "WaveAIPostMessageHandler" pkg/aiusechat/ -A 50 | head -150Repository: wavetermdev/waveterm
Length of output: 3101
🏁 Script executed:
rg -n "aimode\|AIMode" pkg/aiusechat/usechat.go -B 3 -A 3 | head -100Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
rg -n "getWaveAISettings" pkg/aiusechat/ -A 30 | head -100Repository: wavetermdev/waveterm
Length of output: 4099
🏁 Script executed:
rg -n "invalid.*mode|AIMode.*invalid" pkg/aiusechat/ -B 3 -A 3Repository: wavetermdev/waveterm
Length of output: 501
🏁 Script executed:
cat -n pkg/aiusechat/usechat-mode.go | sed -n '280,295p'Repository: wavetermdev/waveterm
Length of output: 605
🏁 Script executed:
rg -n "resolveAIMode" pkg/aiusechat/usechat-mode.go -B 5 -A 20 | head -80Repository: wavetermdev/waveterm
Length of output: 788
Return "unknown" instead of hard-coded "invalid" for telemetry-disabled builder mode, and consider an explicit guard in setAIModeToDefault().
When inBuilder && !telemetryEnabled, defaultModeAtom returns "invalid" (line 146). While the frontend UI is blocked via allowAccess = false (line 274) and shows TelemetryRequiredMessage, the hard-coded sentinel creates two issues:
-
Inconsistency: Non-builder telemetry-disabled mode returns
"unknown"(line 152), but builder returns"invalid". If any code path bypasses the UI block or if a race condition occurs,isValidMode("invalid")returnsfalseand the backend rejects with"invalid AI mode: invalid"(pkg/aiusechat/usechat-mode.go:291). -
Circular fallback logic:
setAIModeToDefault()(line 447) callsglobalStore.get(this.defaultModeAtom), which can return"invalid"in the builder case. If called, this creates a scenario where the default itself is invalid.
Align with the non-builder pattern by returning "unknown" instead, and consider adding an explicit guard in setAIModeToDefault() to prevent propagating an invalid default.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/aipanel/waveai-model.tsx` around lines 143 - 147, Change the
sentinel returned for the builder+telemetry-disabled case in defaultModeAtom
from "invalid" to "unknown" so it matches the non-builder path (update the
branch inside this.defaultModeAtom where it currently returns "invalid"). Also
add an explicit guard inside setAIModeToDefault() to validate the value from
globalStore.get(this.defaultModeAtom) (use isValidMode) and, if invalid,
fallback to "unknown" (or another safe default) instead of propagating an
invalid mode to downstream code.
| builderMode := req.BuilderId != "" | ||
| premium := shouldUsePremium() || builderMode |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and understand the context around lines 673-674
head -n 700 pkg/aiusechat/usechat.go | tail -n 50Repository: wavetermdev/waveterm
Length of output: 1898
🏁 Script executed:
# Get more context around the critical lines
sed -n '650,700p' pkg/aiusechat/usechat.goRepository: wavetermdev/waveterm
Length of output: 1921
🏁 Script executed:
# Search for the resolveAIMode function to understand the fallback logic
rg -n "resolveAIMode" pkg/aiusechat/usechat.go -A 10 -B 2Repository: wavetermdev/waveterm
Length of output: 565
🏁 Script executed:
# Search for mode constants and builder mode definitions
rg -n "waveaibuilder|WaveAIBuilder" pkg/aiusechat/ -B 2 -A 2Repository: wavetermdev/waveterm
Length of output: 461
🏁 Script executed:
# Look for any validation of builder modes
rg -n "BuilderId|builderMode" pkg/aiusechat/usechat.go -B 2 -A 2Repository: wavetermdev/waveterm
Length of output: 2131
🏁 Script executed:
# Find the resolveAIMode function implementation
rg -n "func resolveAIMode" pkg/aiusechat/ -A 50Repository: wavetermdev/waveterm
Length of output: 3228
🏁 Script executed:
# Search for AIModeQuick to understand the fallback
rg -n "AIModeQuick|WaveAIQuick" pkg/aiusechat/uctypes/Repository: wavetermdev/waveterm
Length of output: 139
🏁 Script executed:
# Check if there's any validation preventing builders from using non-builder modes
rg -n "req.AIMode" pkg/aiusechat/usechat.go -B 3 -A 3Repository: wavetermdev/waveterm
Length of output: 528
🏁 Script executed:
# Look for the shouldUsePremium function to understand the premium check
rg -n "func shouldUsePremium" pkg/aiusechat/ -A 15Repository: wavetermdev/waveterm
Length of output: 827
🏁 Script executed:
# Check if getWaveAISettings does any mode validation for builders
rg -n "func getWaveAISettings" pkg/aiusechat/ -A 30Repository: wavetermdev/waveterm
Length of output: 2007
🏁 Script executed:
# Look for any validation that restricts builder requests to builder-only modes
rg -n "AIMode.*Builder|builder.*AIMode" pkg/aiusechat/ -B 3 -A 3Repository: wavetermdev/waveterm
Length of output: 2050
🏁 Script executed:
# Check if there's any request validation for builder mode before calling getWaveAISettings
sed -n '650,690p' pkg/aiusechat/usechat.goRepository: wavetermdev/waveterm
Length of output: 1345
Confirm intent: builder requests always bypass the premium rate-limit fallback.
premium := shouldUsePremium() || builderMode means that whenever BuilderId is present, premium is unconditionally true, regardless of the user's current premium rate-limit state (PReq/ResetEpoch). This prevents the resolveAIMode fallback from waveai@* cloud modes to AIModeQuick when premium is exhausted. For the new waveaibuilder@* modes this is fine (they use WaveAIPremium: true and are the only builder-visible cloud modes), but note that if a builder context ever submits req.AIMode == "waveai@balanced" (or any other WaveAICloud mode), this code will also skip the rate-limit fallback and count against premium quota without the usual guardrail. There is no validation preventing builders from requesting non-builder modes. If that's not desired, consider gating the override on the mode name (e.g., only when aiMode is a known builder mode like waveaibuilder@*).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/aiusechat/usechat.go` around lines 673 - 674, The current logic sets
premium := shouldUsePremium() || builderMode which forces premium for any
request with req.BuilderId, bypassing resolveAIMode and rate-limit fallback;
change this to only force premium when the request is both from a builder
(req.BuilderId set) and the requested AIMode is a builder-only cloud mode (e.g.,
aiMode startsWith "waveaibuilder@" or use a helper
isBuilderOnlyMode(req.AIMode)), otherwise leave premium based solely on
shouldUsePremium() so non-builder WaveAI cloud modes (like "waveai@balanced")
still go through resolveAIMode/rate-limit fallback; update usages in
resolveAIMode/any downstream logic that relies on premium.
| let retVal: any; | ||
| if (fnDecl.jscode) { | ||
| try { | ||
| const fn = eval(fnDecl.jscode); | ||
| if (typeof fn === "function") retVal = fn(...args); | ||
| } catch (err) { | ||
| console.error("vdom jscode error:", err); | ||
| } | ||
| } | ||
| if (!fnDecl.preventbackend) { | ||
| model.callVDomFunc(fnDecl, e, compId, propName); | ||
| } | ||
| return retVal; |
There was a problem hiding this comment.
jscode execution: downstream effect of the (e, elem) contract.
fn(...args) forwards only the underlying handler's arguments, so the second elem parameter documented on the Go side (JSFunc / CombinedFunc in tsunami/vdom/vdom.go) is never supplied. See the root-cause comment on vdom.go; the fix (either injecting elem here or correcting the docs) should be centralized there.
Minor, secondary note: eval was flagged by biome (lint/security/noGlobalEval). Given jscode originates from the trusted backend this is a deliberate choice, but consider using new Function("e", "elem", body) if jscode is migrated to a bare function body — it would be slightly more sandboxable (no access to the enclosing lexical scope) and also lets you pass elem explicitly, killing two birds.
🧰 Tools
🪛 Biome (2.4.11)
[error] 203-203: eval() exposes to security risks and performance issues.
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsunami/frontend/src/vdom.tsx` around lines 200 - 212, The JS handler call is
missing the second `elem` parameter described by the Go-side
`JSFunc`/`CombinedFunc` contract; update the invocation in this block so the
evaluated function receives both event and element (e.g., pass `elem` alongside
`...args`) or centralize the fix in `vdom.go` as noted in the comment; ensure
you adjust the call site that uses `fnDecl.jscode` (the `fn(...args)`
invocation) so the `elem` is provided, and keep the `model.callVDomFunc(fnDecl,
e, compId, propName)` behavior unchanged; optionally consider replacing
`eval(fnDecl.jscode)` with `new Function("e","elem", body)` if the `jscode`
format is changed to a bare function body.
1 participant
lots of updates for tsunami and builder window: