Conversation
…rn modtime of file as well...
WalkthroughThis PR adds builder-mode support and app-file management across frontend and backend. Key changes: new RPC commands and types for app-file operations; server handlers and waveappstore ReadAppFile; frontend atoms and components for builder app selection and editing (AppSelectionModal, BuilderAppPanelModel, BuilderApp UI changes); injection of current app.go into AI chat requests; builder-aware AI settings and tools; event publishing when app.go changes; focus utilities; base64/data-decoding utility updates; a concurrency-safe MultiReaderLineBuffer and its integration into the tsunami controller; and minor editor/UX and VSCode setting tweaks. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code graph analysis (1)frontend/app/view/term/termwrap.ts (5)
⏰ 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). (3)
🔇 Additional comments (4)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return nil, err | ||
| } | ||
|
|
||
| fileInfo, err := os.Stat(filePath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
The best fix is to guarantee that user input (appId) cannot be used to escape the intended root waveapps directory ($HOME/waveapps). This requires, after constructing the app directory path (computed by GetAppDir), to resolve it to an absolute path and verify that it is a child of the intended apps root directory (wavebase.GetHomeDir()/waveapps). This check should be incorporated into both GetAppDir (to guarantee that every function getting an app directory receives a safe path), and/or directly before use in ReadAppFile, WriteAppFile, and DeleteApp.
- Update
GetAppDirto check that the resolved directory is inside the home waveapps root. - Import
filepathand use it to resolve and compare absolute paths. - The check should ensure:
resolvedAppDirbegins withappsRootDir + string(filepath.Separator), or is equal toappsRootDir.
Files/Regions to Change:
- In pkg/waveappstore/waveappstore.go: update
GetAppDirto add the path containment validation.
What is needed:
- Compute both the resolved app dir and resolved apps root dir.
- Compare their prefixes; if not contained, return an error.
- No new dependencies are required.
| @@ -75,7 +75,20 @@ | ||
| } | ||
| appNS, appName, _ := ParseAppId(appId) | ||
| homeDir := wavebase.GetHomeDir() | ||
| return filepath.Join(homeDir, "waveapps", appNS, appName), nil | ||
| appsRootDir := filepath.Join(homeDir, "waveapps") | ||
| appDir := filepath.Join(appsRootDir, appNS, appName) | ||
| resolvedAppDir, err := filepath.Abs(appDir) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to resolve app directory: %w", err) | ||
| } | ||
| resolvedAppsRootDir, err := filepath.Abs(appsRootDir) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to resolve apps root directory: %w", err) | ||
| } | ||
| if !strings.HasPrefix(resolvedAppDir, resolvedAppsRootDir+string(filepath.Separator)) && resolvedAppDir != resolvedAppsRootDir { | ||
| return "", fmt.Errorf("appId escapes waveapps root directory") | ||
| } | ||
| return resolvedAppDir, nil | ||
| } | ||
|
|
||
| func copyDir(src, dst string) error { |
| return nil, fmt.Errorf("failed to stat file: %w", err) | ||
| } | ||
|
|
||
| contents, err := os.ReadFile(filePath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Copilot Autofix
AI 5 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
pkg/aiusechat/anthropic/anthropic-convertmessage.go (1)
127-141: Do not log full request payloads (now includes AppGoFile); gate and trim logsThis block logs the entire messages JSON unconditionally. With AppGoFile injected, this leaks user code and spikes log volume in production.
Apply dev-only gating and drop full payload print:
@@ - // pretty print json of anthropicMsgs - if jsonStr, err := utilfn.MarshalIndentNoHTMLString(convertedMsgs, "", " "); err == nil { - log.Printf("system-prompt: %v\n", chatOpts.SystemPrompt) - var toolNames []string - for _, tool := range chatOpts.Tools { - toolNames = append(toolNames, tool.Name) - } - for _, tool := range chatOpts.TabTools { - toolNames = append(toolNames, tool.Name) - } - log.Printf("tools: %s\n", strings.Join(toolNames, ", ")) - log.Printf("anthropicMsgs JSON:\n%s", jsonStr) - log.Printf("has-api-key: %v\n", opts.APIToken != "") - } + // Dev-only logging; avoid dumping full payloads in production + if wavebase.IsDevMode() { + var toolNames []string + for _, tool := range chatOpts.Tools { + toolNames = append(toolNames, tool.Name) + } + for _, tool := range chatOpts.TabTools { + toolNames = append(toolNames, tool.Name) + } + log.Printf("system-prompt-count=%d tools=%s has-api-key=%v\n", + len(chatOpts.SystemPrompt), strings.Join(toolNames, ", "), opts.APIToken != "") + }And add import:
-import ( +import ( @@ - "github.com/wavetermdev/waveterm/pkg/util/utilfn" + "github.com/wavetermdev/waveterm/pkg/util/utilfn" + "github.com/wavetermdev/waveterm/pkg/wavebase" )pkg/blockcontroller/tsunamicontroller.go (1)
437-459: Race/logic bug: waiter goroutine may consume portChan before main select.The goroutine reading from portChan to “check” if it was found drains the value, causing the main select to miss it and time out or error.
Replace waiter+errChan with direct select on WaitCh:
- // Wait for either port detection, process death, or context timeout - errChan := make(chan error, 1) - go func() { - <-tsunamiProc.WaitCh - select { - case <-portChan: - // Port already found, nothing to do - default: - errChan <- fmt.Errorf("tsunami process died before emitting listening message") - } - }() - - select { - case port := <-portChan: + // Wait for either port detection, process death, or context timeout + select { + case port := <-portChan: tsunamiProc.Port = port return tsunamiProc, nil - case err := <-errChan: - cmd.Process.Kill() - return nil, err + case <-tsunamiProc.WaitCh: + _ = cmd.Process.Kill() + return nil, fmt.Errorf("tsunami process died before emitting listening message") case <-ctx.Done(): - cmd.Process.Kill() + _ = cmd.Process.Kill() return nil, fmt.Errorf("timeout waiting for tsunami port: %w", ctx.Err()) }pkg/aiusechat/usechat.go (1)
153-184: Now I need to verify the actual token limits for these models via web search and check for any downstream clamping.Good! Now let me search for claude-sonnet-4-5 token limits.Perfect! I now have enough information to complete the review verification. Let me summarize what I've found:Token Limit Summary:
- GPT-5 has a max output token limit of 128,000 tokens
- gpt-5 has a maximum output of 128,000 tokens
- gpt-5-mini has a max output of 16,384 tokens
- Claude Sonnet 4.5 output generation reaches 64,000 tokens
BuilderMaxTokens = 24,576 tokens (exceeds gpt-5-mini's 16,384 limit but is within other models' limits)
The code sets
maxTokensto BuilderMaxTokens without any clamping. This will cause API errors when:
- Using gpt-5-mini (max 16,384) with 24,576 requested
- Potentially other lower-tier models
However, there's no downstream clamping visible in the current code review.
Conclusion: The review comment is valid - BuilderMaxTokens (24k) does exceed the output token limit of gpt-5-mini (16,384 tokens). The concern is legitimate and needs addressing via clamping or environment override as suggested.
Clamp BuilderMaxTokens or make output token limit configurable per model.
BuilderMaxTokens (24,576 tokens) exceeds gpt-5-mini's maximum output of 16,384 tokens, causing API validation errors. gpt-5 supports 128,000 max output tokens but not all configured models do. Without downstream clamping, the code will fail at runtime for lower-tier models. Implement model-aware token clamping or an environment override (e.g.,
WAVETERM_BUILDER_MAX_TOKENS) to safely accommodate different model capabilities.
🧹 Nitpick comments (24)
emain/emain-menu.ts (1)
95-100: Verification complete: no legacy "" usage found; use named constant to avoid duplicationThe codebase contains no legacy "" sentinel references. Both call sites (
emain-menu.ts:98andemain-ipc.ts:399) correctly use empty string""as the default. The change is sound.To reduce duplication and improve clarity, define a shared constant in
emain-builder.ts:+export const EMPTY_BUILDER_APP_ID = "";Then update both call sites:
// emain-menu.ts -click: () => fireAndForget(() => createBuilderWindow("")), +click: () => fireAndForget(() => createBuilderWindow(EMPTY_BUILDER_APP_ID)), // emain-ipc.ts -fireAndForget(() => createBuilderWindow(appId || "")); +fireAndForget(() => createBuilderWindow(appId || EMPTY_BUILDER_APP_ID));pkg/aiusechat/anthropic/anthropic-convertmessage.go (1)
75-91: AppGoFile injection: add size cap and filter from UI displayCurrent implementation lacks safeguards present in similar contexts:
- AppGoFile content is unlimited size and injected directly into model messages.
- ConvertToUIMessage displays all text blocks including , risking accidental exposure in logs or UI output.
Recommend adding truncation and filtering to prevent data leakage and manage token usage:
@@ -76,7 +76,10 @@ func convertMessages(chatMessages []*types.ChatMessage, chatOpts *ChatOptions) // inject chatOpts.AppGoFile as a "text" block at the END of the LAST "user" message found (append to Content) if chatOpts.AppGoFile != "" { + const maxInlineAppGoFile = 200000 // ~200 KB + appText := chatOpts.AppGoFile + if len(appText) > maxInlineAppGoFile { + appText = appText[:maxInlineAppGoFile] + "\n[...truncated...]" + } // Find the last "user" message for i := len(convertedMsgs) - 1; i >= 0; i-- { if convertedMsgs[i].Role == "user" { @@ -83,7 +86,7 @@ func convertMessages(chatMessages []*types.ChatMessage, chatOpts *ChatOptions) // Create a text block with the AppGoFile content wrapped in XML tag appGoFileBlock := anthropicMessageContentBlock{ Type: "text", - Text: "<CurrentAppGoFile>\n" + chatOpts.AppGoFile + "\n</CurrentAppGoFile>", + Text: "<CurrentAppGoFile>\n" + appText + "\n</CurrentAppGoFile>", } // Append to the Content of this message convertedMsgs[i].Content = append(convertedMsgs[i].Content, appGoFileBlock) @@ -640,6 +643,9 @@ func (m *anthropicChatMessage) ConvertToUIMessage() *uctypes.UIMessage { switch block.Type { case "text": + if strings.HasPrefix(block.Text, "<CurrentAppGoFile>") { + continue + } // Convert text blocks to UIMessagePart parts = append(parts, uctypes.UIMessagePart{ Type: "text",frontend/wave.ts (1)
291-304: Avoid builderAppId flicker; sync window title with resolved appId.
- Set atoms.builderAppId immediately to initOpts.appId to prevent a transient AppSelectionModal when RT info has an appId.
- After RT lookup, only update if it changed, and update document.title to match.
@@ - await loadConnStatus(); - - let appIdToUse = initOpts.appId; + await loadConnStatus(); + + // Seed immediately to avoid selection-modal flicker + globalStore.set(atoms.builderAppId, initOpts.appId ?? null); + let appIdToUse = initOpts.appId ?? null; @@ - globalStore.set(atoms.builderAppId, appIdToUse); + if (appIdToUse !== initOpts.appId) { + globalStore.set(atoms.builderAppId, appIdToUse); + } + // Keep window title consistent with actual app id + document.title = `Tsunami Builder${appIdToUse ? ` - ${appIdToUse}` : ""}`;frontend/builder/builder-workspace.tsx (1)
16-20: Defaults and fallbacks look good; consider freezing/cloning default layout.Optional: use Object.freeze(DefaultLayoutPercentages) and spread when assigning state to prevent accidental shared-mutation; or set state with a shallow clone.
-const DefaultLayoutPercentages = { +const DefaultLayoutPercentages = Object.freeze({ chat: 50, app: 80, build: 20, -}; +} as const); @@ - setLayout(DefaultLayoutPercentages); + setLayout({ ...DefaultLayoutPercentages }); @@ - setLayout(DefaultLayoutPercentages); + setLayout({ ...DefaultLayoutPercentages }); @@ - setLayout(DefaultLayoutPercentages); + setLayout({ ...DefaultLayoutPercentages });Also applies to: 32-35, 44-45, 48-49
frontend/builder/builder-app.tsx (1)
30-47: Keep document.title in sync with builderAppId.Window title set in wave.ts uses initOpts.appId and can become stale. Sync it here when builderAppId changes.
function BuilderAppInner() { const builderAppId = useAtomValue(atoms.builderAppId); + useEffect(() => { + document.title = `Tsunami Builder${builderAppId ? ` - ${builderAppId}` : ""}`; + }, [builderAppId]); + return ( <div className="w-full h-full flex flex-col bg-main-bg text-main-text"> <BuilderKeyHandlers />Also applies to: 54-57
frontend/builder/tabs/builder-codetab.tsx (1)
77-86: Make Monaco model path unique per app to avoid collisions.Passing blockId="" produces path "/app.go" for all apps. Use appId to isolate models.
Apply this diff:
- <CodeEditor - blockId="" + <CodeEditor + blockId={builderAppId ? `builder/${builderAppId}` : "builder/unknown"} text={codeContent} readonly={false} language="go" fileName="app.go"pkg/blockcontroller/tsunamicontroller.go (2)
309-317: Honor graceful stop: close stdin first, then kill on timeout.Currently we kill before closing stdin, ignoring graceful=true.
Apply this diff:
- if c.tsunamiProc.Cmd.Process != nil { - c.tsunamiProc.Cmd.Process.Kill() - } - - if c.tsunamiProc.StdinWriter != nil { - c.tsunamiProc.StdinWriter.Close() - } + if graceful && c.tsunamiProc.StdinWriter != nil { + _ = c.tsunamiProc.StdinWriter.Close() + select { + case <-c.tsunamiProc.WaitCh: + // exited gracefully + case <-time.After(2 * time.Second): + if c.tsunamiProc.Cmd.Process != nil { + _ = c.tsunamiProc.Cmd.Process.Kill() + } + } + } else if c.tsunamiProc.Cmd.Process != nil { + _ = c.tsunamiProc.Cmd.Process.Kill() + }
414-415: Reduce debug log noise (“WAIT RETURN”).Either remove or guard with a debug flag; this is quite chatty in normal runs.
pkg/utilds/multireaderlinebuffer.go (1)
39-45: Bump scanner buffer to handle long lines; optionally surface scan errors.Default bufio.Scanner token limit (~64KB) can truncate long lines.
Apply this diff:
func (mrlb *MultiReaderLineBuffer) ReadAll(r io.Reader) { - scanner := bufio.NewScanner(r) + scanner := bufio.NewScanner(r) + // Allow up to 1MB lines + buf := make([]byte, 64*1024) + scanner.Buffer(buf, 1*1024*1024) for scanner.Scan() { line := scanner.Text() mrlb.addLine(line) mrlb.callLineCallback(line) } + // scanner.Err() can be checked by the caller if we change signature; for now ignore. }frontend/builder/store/builderAppPanelModel.ts (3)
70-75: Type-safe catch and robust error message.Avoid accessing .message on unknown. Guard the type.
- } catch (err) { - console.error("Failed to load app.go:", err); - globalStore.set(this.errorAtom, `Failed to load app.go: ${err.message || "Unknown error"}`); + } catch (err: unknown) { + console.error("Failed to load app.go:", err); + const msg = err instanceof Error ? err.message : String(err); + globalStore.set(this.errorAtom, `Failed to load app.go: ${msg}`); } finally {- } catch (err) { - console.error("Failed to save app.go:", err); - globalStore.set(this.errorAtom, `Failed to save app.go: ${err.message || "Unknown error"}`); + } catch (err: unknown) { + console.error("Failed to save app.go:", err); + const msg = err instanceof Error ? err.message : String(err); + globalStore.set(this.errorAtom, `Failed to save app.go: ${msg}`); }Also applies to: 92-95
55-76: Avoid lost updates; track modTs and do optimistic concurrency.Read returns modts, but it’s ignored; writes don’t check freshness. Add an atom for modTs and send ExpectedModTs on write; reject on mismatch. Requires adding ExpectedModTs to CommandWriteAppFileData and server check. I can draft that if you want.
Also applies to: 83-95
20-22: Type Monaco ref.Prefer Monaco’s IStandaloneCodeEditor over any.
pkg/wshrpc/wshserver/wshserver.go (1)
900-926: Entry field naming parity (ModifiedTime).You map util.DirEntryOut.ModifiedTime into wshrpc.DirEntryOut.ModifiedTime (json:"modifiedtime"). util uses json:"modified_time". Consider aligning JSON field names to a single convention to reduce confusion for clients.
frontend/builder/builder-apppanel.tsx (4)
138-147: Add disabled/aria-disabled to Save for a11y and semantics.OnClick undefined helps, but button should be truly disabled.
- <button + <button + disabled={!saveNeeded} + aria-disabled={!saveNeeded} className={cn( "mr-4 px-3 py-1 text-sm font-medium rounded transition-colors", saveNeeded ? "bg-accent text-white hover:opacity-80 cursor-pointer" - : "bg-gray-600 text-gray-400 cursor-default" + : "bg-gray-600 text-gray-400 cursor-not-allowed" )} onClick={saveNeeded ? handleSave : undefined} >
51-53: Set focus ref in an effect, not on every render.- if (focusElemRef.current) { - model.setFocusElemRef(focusElemRef.current); - } + // set once when the ref attaches + React.useEffect(() => { + if (focusElemRef.current) { + model.setFocusElemRef(focusElemRef.current); + } + }, [model]);
104-111: Make hidden input uncontrolled and readOnly.Avoid controlled input noise and needless onChange.
- <input - type="text" - value="" - ref={focusElemRef} - className="h-0 w-0 opacity-0 pointer-events-none" - onChange={() => {}} - /> + <input + type="text" + ref={focusElemRef} + readOnly + className="h-0 w-0 opacity-0 pointer-events-none" + />
16-23: Remove unused prop.TabButton receives tabType but doesn’t use it. Drop the prop or use it for data-attr.
Also applies to: 24-39
pkg/wshrpc/wshrpctypes.go (2)
912-920: Align DirEntryOut JSON tag for ModifiedTime.util/fileutil uses json:"modified_time". Consider matching here for consistency and fewer client surprises.
- ModifiedTime string `json:"modifiedtime"` + ModifiedTime string `json:"modified_time"`
933-937: Add ExpectedModTs for optimistic concurrency (optional, prevents blind overwrites).Allows servers to reject stale writes when file changed on disk.
type CommandWriteAppFileData struct { AppId string `json:"appid"` FileName string `json:"filename"` Data64 string `json:"data64"` + ExpectedModTs int64 `json:"expectedmodts,omitempty"` }Server check (wshserver.go):
func (ws *WshServer) WriteAppFileCommand(ctx context.Context, data wshrpc.CommandWriteAppFileData) error { contents, err := base64.StdEncoding.DecodeString(data.Data64) @@ - return waveappstore.WriteAppFile(data.AppId, data.FileName, contents) + // optional optimistic concurrency + if data.ExpectedModTs > 0 { + fd, err := waveappstore.ReadAppFile(data.AppId, data.FileName) + if err == nil && fd.ModTs != data.ExpectedModTs { + return fmt.Errorf("write conflict: file modified") + } + } + return waveappstore.WriteAppFile(data.AppId, data.FileName, contents) }pkg/aiusechat/usechat.go (5)
6-16: Import “embed” directly (blank import not needed).Use
import "embed"; it’s allowed to be “unused” when go:embed directives exist in the file.- _ "embed" + "embed"
442-448: Avoid stale AppGoFile/AppBuildStatus when generator errors.Clear fields on error so the model doesn’t see outdated state.
if chatOpts.BuilderAppGenerator != nil { appGoFile, appBuildStatus, appErr := chatOpts.BuilderAppGenerator() - if appErr == nil { - chatOpts.AppGoFile = appGoFile - chatOpts.AppBuildStatus = appBuildStatus - } + if appErr == nil { + chatOpts.AppGoFile = appGoFile + chatOpts.AppBuildStatus = appBuildStatus + } else { + chatOpts.AppGoFile = "" + chatOpts.AppBuildStatus = "" + } }
691-699: Consider disabling native web search in builder mode by default.Helps determinism and reduces drift from Tsunami docs unless explicitly needed.
chatOpts := uctypes.WaveChatOpts{ @@ - AllowNativeWebSearch: true, + AllowNativeWebSearch: req.BuilderId == "" || req.WidgetAccess, // default off in builder
701-706: Apply builder prompt for Anthropic too and separate docs with a newline.Right now builder prompt is OpenAI-only and concatenated without guaranteed spacing.
- if chatOpts.Config.APIType == APIType_OpenAI { - if chatOpts.BuilderId != "" { - chatOpts.SystemPrompt = []string{BuilderSystemPromptText_OpenAI + tsunamiSystemDoc} - } else { - chatOpts.SystemPrompt = []string{SystemPromptText_OpenAI} - } - } else { - chatOpts.SystemPrompt = []string{SystemPromptText} - } + if chatOpts.Config.APIType == APIType_OpenAI { + if chatOpts.BuilderId != "" { + chatOpts.SystemPrompt = []string{BuilderSystemPromptText_OpenAI + "\n\n" + tsunamiSystemDoc} + } else { + chatOpts.SystemPrompt = []string{SystemPromptText_OpenAI} + } + } else { + if chatOpts.BuilderId != "" { + chatOpts.SystemPrompt = []string{BuilderSystemPromptText_OpenAI + "\n\n" + tsunamiSystemDoc} + } else { + chatOpts.SystemPrompt = []string{SystemPromptText} + } + }
717-727: Handle missing app.go without failing builder flow.Treat not-found as empty file so the model can propose initial content.
+ // import "errors" at top if req.BuilderAppId != "" { chatOpts.BuilderAppGenerator = func() (string, string, error) { fileData, err := waveappstore.ReadAppFile(req.BuilderAppId, "app.go") if err != nil { - return "", "", err + if errors.Is(err, os.ErrNotExist) { + return "", "", nil + } + return "", "", err } appGoFile := string(fileData.Contents) appBuildStatus := "" return appGoFile, appBuildStatus, nil } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
.vscode/settings.json(1 hunks)emain/emain-ipc.ts(1 hunks)emain/emain-menu.ts(1 hunks)frontend/app/aipanel/aipanel.tsx(1 hunks)frontend/app/modals/modal.scss(1 hunks)frontend/app/store/global.ts(2 hunks)frontend/app/store/wshclientapi.ts(5 hunks)frontend/builder/app-selection-modal.tsx(1 hunks)frontend/builder/builder-app.tsx(2 hunks)frontend/builder/builder-apppanel.tsx(2 hunks)frontend/builder/builder-workspace.tsx(3 hunks)frontend/builder/store/builderAppPanelModel.ts(1 hunks)frontend/builder/tabs/builder-codetab.tsx(1 hunks)frontend/builder/tabs/builder-filestab.tsx(1 hunks)frontend/builder/tabs/builder-previewtab.tsx(1 hunks)frontend/builder/utils/builder-focus-utils.ts(1 hunks)frontend/types/custom.d.ts(1 hunks)frontend/types/gotypes.d.ts(6 hunks)frontend/wave.ts(1 hunks)pkg/aiusechat/anthropic/anthropic-convertmessage.go(1 hunks)pkg/aiusechat/openai/openai-convertmessage.go(1 hunks)pkg/aiusechat/tools_builder.go(5 hunks)pkg/aiusechat/uctypes/usechat-types.go(1 hunks)pkg/aiusechat/usechat.go(10 hunks)pkg/blockcontroller/tsunamicontroller.go(5 hunks)pkg/utilds/multireaderlinebuffer.go(1 hunks)pkg/waveappstore/waveappstore.go(3 hunks)pkg/waveobj/objrtinfo.go(1 hunks)pkg/wps/wpstypes.go(1 hunks)pkg/wshrpc/wshclient/wshclient.go(5 hunks)pkg/wshrpc/wshrpctypes.go(3 hunks)pkg/wshrpc/wshserver/wshserver.go(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-22T01:28:41.417Z
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/fileshare/wshfs/wshfs.go:122-122
Timestamp: 2025-01-22T01:28:41.417Z
Learning: The RpcClient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either main-server or wshcmd-connserver, as documented in the package comment.
Applied to files:
pkg/wshrpc/wshclient/wshclient.go
📚 Learning: 2025-10-15T03:21:02.229Z
Learnt from: sawka
PR: wavetermdev/waveterm#2433
File: pkg/aiusechat/tools_readfile.go:197-197
Timestamp: 2025-10-15T03:21:02.229Z
Learning: In Wave Terminal's AI tool definitions (pkg/aiusechat/tools_*.go), the Description field should not mention approval requirements even when ToolApproval returns ApprovalNeedsApproval. This prevents the LLM from asking users for approval before calling the tool, avoiding redundant double-approval prompts since the runtime will enforce approval anyway.
Applied to files:
pkg/aiusechat/tools_builder.go
🧬 Code graph analysis (19)
frontend/builder/app-selection-modal.tsx (4)
frontend/app/store/wshclientapi.ts (1)
RpcApi(572-572)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)frontend/app/store/global.ts (3)
globalStore(839-839)atoms(816-816)WOS(866-866)frontend/app/modals/modal.tsx (1)
FlexiModal(112-112)
emain/emain-menu.ts (1)
emain/emain-builder.ts (1)
createBuilderWindow(34-114)
pkg/aiusechat/openai/openai-convertmessage.go (1)
pkg/aiusechat/openai/openai-backend.go (2)
OpenAIMessage(38-41)OpenAIMessageContent(66-78)
emain/emain-ipc.ts (1)
emain/emain-builder.ts (1)
createBuilderWindow(34-114)
frontend/wave.ts (3)
frontend/app/store/global.ts (3)
WOS(866-866)globalStore(839-839)atoms(816-816)frontend/app/store/wshclientapi.ts (1)
RpcApi(572-572)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
pkg/wshrpc/wshclient/wshclient.go (3)
frontend/app/store/wshclientapi.ts (6)
DeleteAppFileCommand(116-118)ListAllAppFilesCommand(306-308)ListAllEditableAppsCommand(311-313)ReadAppFileCommand(331-333)RenameAppFileCommand(411-413)WriteAppFileCommand(546-548)pkg/wshutil/wshrpc.go (1)
WshRpc(46-60)pkg/wshrpc/wshrpctypes.go (8)
CommandDeleteAppFileData(939-942)RpcOpts(315-321)CommandListAllAppFilesData(898-900)CommandListAllAppFilesRtnData(902-910)CommandReadAppFileData(922-925)CommandReadAppFileRtnData(927-931)CommandRenameAppFileData(944-948)CommandWriteAppFileData(933-937)
frontend/builder/tabs/builder-codetab.tsx (3)
frontend/builder/store/builderAppPanelModel.ts (1)
BuilderAppPanelModel(11-113)pkg/vdom/vdom_types.go (1)
WaveKeyboardEvent(233-247)frontend/app/view/codeeditor/codeeditor.tsx (1)
CodeEditor(116-179)
frontend/builder/builder-app.tsx (3)
frontend/app/store/global.ts (2)
atoms(816-816)globalStore(839-839)frontend/util/util.ts (1)
isBlank(481-481)frontend/builder/app-selection-modal.tsx (1)
AppSelectionModal(14-207)
pkg/wshrpc/wshrpctypes.go (3)
frontend/app/store/wshclientapi.ts (6)
ListAllEditableAppsCommand(311-313)ListAllAppFilesCommand(306-308)ReadAppFileCommand(331-333)WriteAppFileCommand(546-548)DeleteAppFileCommand(116-118)RenameAppFileCommand(411-413)pkg/wshrpc/wshclient/wshclient.go (6)
ListAllEditableAppsCommand(378-381)ListAllAppFilesCommand(372-375)ReadAppFileCommand(402-405)WriteAppFileCommand(651-654)DeleteAppFileCommand(147-150)RenameAppFileCommand(494-497)pkg/util/fileutil/readdir.go (1)
DirEntryOut(18-26)
pkg/blockcontroller/tsunamicontroller.go (4)
pkg/utilds/multireaderlinebuffer.go (2)
MultiReaderLineBuffer(9-15)MakeMultiReaderLineBuffer(17-27)pkg/waveobj/waveobj.go (1)
MakeORef(71-76)pkg/wstore/wstore_rtinfo.go (1)
SetRTInfo(22-99)tsunami/build/build.go (1)
ParseTsunamiPort(819-842)
frontend/app/store/wshclientapi.ts (1)
pkg/wshrpc/wshrpctypes.go (8)
CommandDeleteAppFileData(939-942)RpcOpts(315-321)CommandListAllAppFilesData(898-900)CommandListAllAppFilesRtnData(902-910)CommandReadAppFileData(922-925)CommandReadAppFileRtnData(927-931)CommandRenameAppFileData(944-948)CommandWriteAppFileData(933-937)
frontend/builder/store/builderAppPanelModel.ts (2)
frontend/app/store/wshclientapi.ts (1)
RpcApi(572-572)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
pkg/waveappstore/waveappstore.go (1)
pkg/wshrpc/wshrpctypes.go (1)
FileData(452-457)
frontend/types/gotypes.d.ts (3)
pkg/wshrpc/wshrpctypes.go (9)
CommandDeleteAppFileData(939-942)CommandListAllAppFilesData(898-900)CommandListAllAppFilesRtnData(902-910)DirEntryOut(912-920)CommandMessageData(384-387)CommandReadAppFileData(922-925)CommandReadAppFileRtnData(927-931)CommandRenameAppFileData(944-948)CommandWriteAppFileData(933-937)pkg/util/fileutil/readdir.go (1)
DirEntryOut(18-26)pkg/waveobj/waveobj.go (1)
ORef(30-34)
pkg/aiusechat/tools_builder.go (2)
pkg/wps/wps.go (1)
Broker(48-52)pkg/wps/wpstypes.go (2)
WaveEvent(23-29)Event_WaveAppAppGoUpdated(20-20)
pkg/aiusechat/uctypes/usechat-types.go (1)
pkg/aiusechat/toolapproval.go (1)
RegisterToolApproval(48-58)
pkg/wshrpc/wshserver/wshserver.go (3)
pkg/waveappstore/waveappstore.go (6)
ListAllEditableApps(444-499)ListAllAppFiles(383-398)ReadAppFile(281-310)WriteAppFile(255-279)DeleteAppFile(312-332)RenameAppFile(352-381)pkg/wshrpc/wshrpctypes.go (8)
CommandListAllAppFilesData(898-900)CommandListAllAppFilesRtnData(902-910)DirEntryOut(912-920)CommandReadAppFileData(922-925)CommandReadAppFileRtnData(927-931)CommandWriteAppFileData(933-937)CommandDeleteAppFileData(939-942)CommandRenameAppFileData(944-948)pkg/util/fileutil/readdir.go (1)
DirEntryOut(18-26)
pkg/aiusechat/usechat.go (4)
pkg/aiusechat/uctypes/usechat-types.go (3)
AIOptsType(196-207)DefaultAnthropicModel(12-12)ToolDefinition(78-90)pkg/aiusechat/tools.go (1)
GenerateTabStateAndTools(128-183)pkg/waveappstore/waveappstore.go (1)
ReadAppFile(281-310)pkg/aiusechat/tools_builder.go (3)
GetBuilderWriteAppFileToolDefinition(40-83)GetBuilderEditAppFileToolDefinition(107-174)GetBuilderListFilesToolDefinition(176-200)
frontend/builder/builder-apppanel.tsx (5)
frontend/builder/store/builderAppPanelModel.ts (2)
BuilderAppPanelModel(11-113)TabType(9-9)frontend/builder/store/builderFocusManager.ts (1)
BuilderFocusManager(9-34)frontend/app/store/global.ts (1)
atoms(816-816)frontend/builder/utils/builder-focus-utils.ts (1)
builderAppHasSelection(44-59)frontend/app/element/errorboundary.tsx (1)
ErrorBoundary(6-33)
⏰ 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). (3)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (41)
frontend/app/modals/modal.scss (1)
17-17: Verify intentional gap at the top.Shifting the modal backdrop from
top: 0totop: 36pxcreates a 36px gap at the top where the backdrop won't cover the screen. Is this intentional to keep a header or toolbar visible when modals are open?.vscode/settings.json (1)
65-65: LGTM!Adding this Tailwind CSS linting configuration is reasonable for reducing editor noise during development.
pkg/wps/wpstypes.go (1)
20-20: LGTM!The new event constant follows the established naming convention and aligns with the builder app-file workflow introduced in this PR.
pkg/waveappstore/waveappstore.go (4)
21-22: LGTM!The length limits for namespace (30 characters) and app name (50 characters) are reasonable constraints for app identifiers.
30-33: LGTM!The
FileDatastruct provides a clean internal representation for file contents and modification timestamp. Whilepkg/wshrpc/wshrpctypes.gohas a differentFileDatastruct, there's no collision since they're in separate packages serving different purposes.
57-62: LGTM!The length validation is correctly placed after parsing and provides clear error messages. This helps prevent filesystem issues with excessively long directory names.
281-310: LGTM!The
ReadAppFilefunction follows the established pattern of other app file operations, includes proper path validation to prevent directory traversal, and has consistent error handling. The use ofUnixMilli()for the modification timestamp provides appropriate precision.frontend/builder/utils/builder-focus-utils.ts (3)
4-13: LGTM!The
findBuilderAppPanelfunction uses a standard DOM traversal pattern to locate the builder panel ancestor. The implementation is simple and correct.
15-42: LGTM!The
builderAppHasFocusWithinfunction comprehensively checks for focus within the builder panel by examining the provided target, active element, and selection anchor. The handling of Text nodes is correct and necessary.
44-59: LGTM!The
builderAppHasSelectionfunction correctly identifies non-collapsed selections within the builder panel, with proper handling of edge cases and Text nodes.frontend/app/aipanel/aipanel.tsx (1)
235-235: LGTM!Adding
builderappidto the request body when in builder mode allows the AI backend to have context about which app is being built, complementing the existingbuilderid. This aligns with the broader builder-app workflow introduced in this PR.frontend/builder/tabs/builder-previewtab.tsx (1)
4-12: LGTM!The simplification of this component aligns with the broader refactoring to centralize focus management in
BuilderAppPanelModel. The component now serves as a placeholder while the focus-related logic has been moved to the model-driven architecture.emain/emain-ipc.ts (1)
399-399: No issues found – empty string is properly handled throughout the builder flow.The codebase already uses empty string
""as a valid appId value inemain-menu.ts:98, establishing this as an accepted pattern. The frontend has robust fallback logic inwave.ts:292-303that attempts to load the appId from rtinfo when initially empty. The empty appId represents a valid initial state where no app is selected, and users can select one afterward. The change from"<new>"to""is semantically cleaner and consistent with existing usage patterns.pkg/waveobj/objrtinfo.go (1)
21-25: ✓ BuilderAppId field is properly integratedVerification confirms producers set the field (app-selection-modal.tsx) and consumers read it (wave.ts). TypeScript types and generic wstore mechanisms are aligned. Code is production-ready.
frontend/types/custom.d.ts (1)
9-16: Review comment is incorrect; no action requiredThe builderAppId atom is already exported and initialized—your changes are correct. However, the review assumed initialization to
"", but the code initializes tonull(line 57), which is consistent with other optional atoms in the store (fullConfigAtom, rateLimitInfoAtom). UI components properly consume it via useAtomValue in builder-app.tsx, builder-apppanel.tsx, and builder-codetab.tsx.Likely an incorrect or invalid review comment.
frontend/builder/tabs/builder-filestab.tsx (1)
4-4: LGTM — simplified and side-effect free.Reduction in focus side effects aligns with centralizing focus management elsewhere.
Also applies to: 8-11
pkg/aiusechat/uctypes/usechat-types.go (1)
421-440: WaveChatOpts extensions look good.Additive, backwards-compatible fields for builder context and app.go content.
frontend/types/gotypes.d.ts (7)
198-205: App-file delete type looks correct.
249-264: ListAllAppFiles request/response types align with wshrpc.
271-283: ReadAppFile types (notfound/modts) match server structs.
307-313: RenameAppFile payload names match server (fromfilename/tofilename).
399-405: WriteAppFile type matches server (appid/filename/data64).
785-785: ObjRTInfo builder:appid addition is fine and scoped.
488-498: No issues found—conversion is correctly implemented.The codebase properly handles the two
DirEntryOuttypes:
fileutil.DirEntryOut(internal) hasjson:"modified_time"wshrpc.DirEntryOut(RPC) hasjson:"modifiedtime"The server converts
fileutil.DirEntryOuttowshrpc.DirEntryOutatpkg/wshrpc/wshserver/wshserver.go:905–920before serialization, ensuring the frontend receives the correctmodifiedtimefield (matching the TypeScript definition at line 497).pkg/blockcontroller/tsunamicontroller.go (2)
382-395: Port detection and logging path looks good.Unified callback, port parse via build.ParseTsunamiPort, and central log formatting are coherent.
93-116: Schema fetch flow is sound (timeout, status check, SetRTInfo, defer close).pkg/wshrpc/wshclient/wshclient.go (6)
146-151: DeleteAppFile wrapper matches server and TS surface.
371-376: ListAllAppFiles wrapper: correct request/response typing.
377-382: ListAllEditableApps wrapper: ok.
401-406: ReadAppFile wrapper: ok.
493-498: RenameAppFile wrapper: ok.
650-655: WriteAppFile wrapper: ok.frontend/builder/tabs/builder-codetab.tsx (1)
29-35: Event name "waveapp:appgoupdated" is correct and verified.The event string matches the backend constant definition in
pkg/wps/wpstypes.goand is consistently published frompkg/aiusechat/tools_builder.go. No typo exists.pkg/wshrpc/wshserver/wshserver.go (1)
928-942: Nice NotFound handling.Clean mapping of os.ErrNotExist to NotFound. LGTM.
frontend/app/store/wshclientapi.ts (1)
305-313: No issues found—RPC wiring is correct.All TS types exist in gotypes.d.ts, command names match server constants in wshrpc/wshrpctypes.go, and all client implementations are properly wired:
- DeleteAppFileCommand (line 115-118): "deleteappfile" ✓
- ListAllAppFilesCommand (line 305-313): "listallappfiles" ✓
- ReadAppFileCommand (line 330-333): "readappfile" ✓
- RenameAppFileCommand (line 410-413): "renameappfile" ✓
- WriteAppFileCommand (line 545-548): "writeappfile" ✓
pkg/aiusechat/usechat.go (6)
43-46: LGTM on builder token constant and embedded docs.Constant and go:embed look fine.
103-151: Builder prompt content looks good.Clear guardrails and workflow guidance. No functional concerns here.
673-677: Builder mode toggle is clear.Good: settings keyed off
builderid != "".
710-715: Tab state generator closure looks fine.UUID validation handled inside GenerateTabStateAndTools.
75-81: No action needed— is correctly injected.The verification confirms that the tag referenced in the prompt is properly injected into the model context. The flow is:
BuilderAppGenerator()callback populateschatOpts.AppGoFile(usechat.go:443–445)- Before sending messages to the model,
AppGoFileis wrapped in<CurrentAppGoFile>XML tags and appended to the last user message:
- OpenAI provider: openai-convertmessage.go:197–208
- Anthropic provider: anthropic-convertmessage.go:75–86
The implementation is correct and the prompt's expectations are met.
639-647: BuilderAppId field is properly implemented and used.The field is correctly integrated into the handler: parsed from request body (line 651), passed to
chatOpts(line 698), and actively used to configure builder-specific functionality (lines 717-734). The optionalomitemptytag ensures backward compatibility.However, verification of "any API docs/clients" requires manual review of external documentation and client libraries outside this repository, as they are not present in the codebase.
| const windowIdAtom = atom(initOpts.windowId) as PrimitiveAtom<string>; | ||
| const clientIdAtom = atom(initOpts.clientId) as PrimitiveAtom<string>; | ||
| const builderIdAtom = atom(initOpts.builderId) as PrimitiveAtom<string>; | ||
| const builderAppIdAtom = atom<string>(null) as PrimitiveAtom<string>; |
There was a problem hiding this comment.
🧩 Analysis chain
Type mismatch: builderAppIdAtom initialized with null but typed as string.
Initialize and export as string | null to avoid unsound casts and downstream type errors.
- const builderAppIdAtom = atom<string>(null) as PrimitiveAtom<string>;
+ const builderAppIdAtom = atom<string | null>(null) as PrimitiveAtom<string | null>;
@@
- builderAppId: builderAppIdAtom,
+ builderAppId: builderAppIdAtom,Also ensure GlobalAtomsType in frontend/types/custom.d.ts reflects builderAppId: jotai.PrimitiveAtom<string | null>.
Also applies to: 176-176
🏁 Script executed:
#!/bin/bash
# Check the actual code at line 57 in global.ts
echo "=== Checking frontend/app/store/global.ts around line 57 ==="
cat -n frontend/app/store/global.ts | sed -n '50,65p'
echo ""
echo "=== Checking line 176 (export) ==="
cat -n frontend/app/store/global.ts | sed -n '170,180p'Length of output: 1467
🏁 Script executed:
#!/bin/bash
# Check the GlobalAtomsType definition in custom.d.ts
echo "=== Checking frontend/types/custom.d.ts for GlobalAtomsType ==="
grep -n -A 20 "GlobalAtomsType" frontend/types/custom.d.tsLength of output: 1466
🏁 Script executed:
#!/bin/bash
# Check atom imports and type signatures
echo "=== Checking imports in global.ts ==="
head -30 frontend/app/store/global.ts | grep -E "import|from"Length of output: 765
Fix type mismatch: builderAppIdAtom initialized with null but typed as string.
Line 57 declares atom<string>(null) cast to PrimitiveAtom<string>, but jotai requires the initial value to match the type parameter. Change to atom<string | null>(null) and update the cast accordingly.
Also update frontend/types/custom.d.ts line 12 from builderAppId: jotai.PrimitiveAtom<string> to builderAppId: jotai.PrimitiveAtom<string | null>.
- const builderAppIdAtom = atom<string>(null) as PrimitiveAtom<string>;
+ const builderAppIdAtom = atom<string | null>(null) as PrimitiveAtom<string | null>;📝 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.
| const builderAppIdAtom = atom<string>(null) as PrimitiveAtom<string>; | |
| const builderAppIdAtom = atom<string | null>(null) as PrimitiveAtom<string | null>; |
🤖 Prompt for AI Agents
In frontend/app/store/global.ts around line 57 the atom is declared as
atom<string>(null) cast to PrimitiveAtom<string>, causing a type mismatch
because the initial value is null; change the atom's generic to string | null
and cast to PrimitiveAtom<string | null> so the initial null value matches the
type. Also update frontend/types/custom.d.ts at around line 12 where
builderAppId is declared as jotai.PrimitiveAtom<string> to
jotai.PrimitiveAtom<string | null> so the type definition matches the atom's
updated type.
| const handleSelectApp = async (appId: string) => { | ||
| const builderId = globalStore.get(atoms.builderId); | ||
| const oref = WOS.makeORef("builder", builderId); | ||
| await RpcApi.SetRTInfoCommand(TabRpcClient, { | ||
| oref, | ||
| data: { "builder:appid": appId }, | ||
| }); | ||
| globalStore.set(atoms.builderAppId, appId); | ||
| }; |
There was a problem hiding this comment.
Handle RPC failures and tidy UX.
- Wrap SetRTInfoCommand calls in try/catch and surface errors.
- Clear error as user edits.
- Remove unused isDraftApp or use it within getAppDisplayName.
@@
- const handleSelectApp = async (appId: string) => {
- const builderId = globalStore.get(atoms.builderId);
- const oref = WOS.makeORef("builder", builderId);
- await RpcApi.SetRTInfoCommand(TabRpcClient, {
- oref,
- data: { "builder:appid": appId },
- });
- globalStore.set(atoms.builderAppId, appId);
- };
+ const handleSelectApp = async (appId: string) => {
+ try {
+ const builderId = globalStore.get(atoms.builderId);
+ const oref = WOS.makeORef("builder", builderId);
+ await RpcApi.SetRTInfoCommand(TabRpcClient, {
+ oref,
+ data: { "builder:appid": appId },
+ });
+ globalStore.set(atoms.builderAppId, appId);
+ } catch (err) {
+ console.error("Failed to select app:", err);
+ setError("Failed to select app. Please try again.");
+ }
+ };
@@
- const handleCreateNew = async () => {
+ const handleCreateNew = async () => {
const trimmedName = newAppName.trim();
@@
- const draftAppId = `draft/${trimmedName}`;
- const builderId = globalStore.get(atoms.builderId);
- const oref = WOS.makeORef("builder", builderId);
- await RpcApi.SetRTInfoCommand(TabRpcClient, {
- oref,
- data: { "builder:appid": draftAppId },
- });
- globalStore.set(atoms.builderAppId, draftAppId);
+ try {
+ const draftAppId = `draft/${trimmedName}`;
+ const builderId = globalStore.get(atoms.builderId);
+ const oref = WOS.makeORef("builder", builderId);
+ await RpcApi.SetRTInfoCommand(TabRpcClient, {
+ oref,
+ data: { "builder:appid": draftAppId },
+ });
+ globalStore.set(atoms.builderAppId, draftAppId);
+ } catch (err) {
+ console.error("Failed to create draft app:", err);
+ setError("Failed to create draft app. Please try again.");
+ }
};
@@
- const isDraftApp = (appId: string) => {
- return appId.startsWith("draft/");
- };
+ // helper inlined into getAppDisplayName
@@
- onChange={(e) => {
+ onChange={(e) => {
const value = e.target.value;
setNewAppName(value);
validateAppName(value);
+ if (error) setError("");
}}Also applies to: 64-90, 92-104, 127-145, 155-205
| import { BuilderPreviewTab } from "@/builder/tabs/builder-previewtab"; | ||
| import { builderAppHasSelection } from "@/builder/utils/builder-focus-utils"; | ||
| import { ErrorBoundary } from "@/element/errorboundary"; | ||
| import { atoms } from "@/store/global"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify atoms import path.
Other files use "@/app/store/..."; here it’s "@/store/global". Likely a path error.
🏁 Script executed:
#!/bin/bash
# Verify which path exists
fd -a 'global.ts' frontend | sed -n '1,200p'
rg -nP '^export\s+const\s+atoms\b' -C0Length of output: 47
I need to gather more information about how atoms is exported and verify the correct import path pattern.
🏁 Script executed:
#!/bin/bash
# Check how atoms is exported in global.ts
echo "=== Checking global.ts exports ==="
cat frontend/app/store/global.ts | head -50
echo -e "\n=== Searching for atoms export with different patterns ==="
rg -nP 'atoms' frontend/app/store/global.ts -C2
echo -e "\n=== Check other files importing atoms to see correct path ==="
rg -nP "from\s+['\"]@.*atoms" frontend --type=ts --type=tsx -C0 | head -20Length of output: 4995
Fix incorrect atoms import path: change "@/store/global" to "@/app/store/global"
The file global.ts is located at frontend/app/store/global.ts and exports atoms at line 816. The current import resolves to a non-existent path. Use @/app/store/global instead.
🤖 Prompt for AI Agents
In frontend/builder/builder-apppanel.tsx around line 11, the import path
currently uses "@/store/global" which does not exist; update the import to
"@/app/store/global" so it correctly resolves to frontend/app/store/global.ts
where atoms is exported. Replace the incorrect module specifier with the
corrected one and ensure any tooling (tsconfig/paths) still maps "@" to the
project root so the new path resolves.
| async saveAppFile(appId: string) { | ||
| if (!appId) return; | ||
|
|
There was a problem hiding this comment.
Surface empty appId as a user error.
Silent return hides the problem. Set errorAtom and consider disabling Save.
- async saveAppFile(appId: string) {
- if (!appId) return;
+ async saveAppFile(appId: string) {
+ if (!appId) {
+ globalStore.set(this.errorAtom, "No app selected");
+ return;
+ }📝 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.
| async saveAppFile(appId: string) { | |
| if (!appId) return; | |
| async saveAppFile(appId: string) { | |
| if (!appId) { | |
| globalStore.set(this.errorAtom, "No app selected"); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In frontend/builder/store/builderAppPanelModel.ts around lines 78 to 80, the
saveAppFile function currently returns silently when appId is empty; change this
to surface a user-facing error by setting the appropriate errorAtom (e.g.,
errorAtom.set("Missing app ID") or similar store error setter) and ensure any UI
Save action can read that atom to disable the Save button when appId is falsy;
do not throw uncaught errors—set the errorAtom, log a concise message if needed,
and return after setting the error so callers and UI reflect the invalid state.
| // Inject chatOpts.AppGoFile as a text block at the end of the last "user" message | ||
| if chatOpts.AppGoFile != "" { | ||
| // Find the last "user" message | ||
| for i := len(inputs) - 1; i >= 0; i-- { | ||
| if msg, ok := inputs[i].(OpenAIMessage); ok && msg.Role == "user" { | ||
| // Add AppGoFile wrapped in XML tag | ||
| appGoFileBlock := OpenAIMessageContent{ | ||
| Type: "input_text", | ||
| Text: "<CurrentAppGoFile>\n" + chatOpts.AppGoFile + "\n</CurrentAppGoFile>", | ||
| } | ||
| msg.Content = append(msg.Content, appGoFileBlock) | ||
| inputs[i] = msg | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
AppGoFile injection: cap size and prevent UI leakage
Good parity with TabState. To harden:
- Cap injected size to avoid token bloat.
- Exclude these blocks from UI conversion to prevent dumping full source into chat UI.
Apply size cap:
@@
- if chatOpts.AppGoFile != "" {
+ if chatOpts.AppGoFile != "" {
+ const MaxInlineAppGoFileBytes = 200000 // ~200 KB
+ appText := chatOpts.AppGoFile
+ if len(appText) > MaxInlineAppGoFileBytes {
+ appText = appText[:MaxInlineAppGoFileBytes] + "\n[...truncated...]"
+ }
@@
- appGoFileBlock := OpenAIMessageContent{
+ appGoFileBlock := OpenAIMessageContent{
Type: "input_text",
- Text: "<CurrentAppGoFile>\n" + chatOpts.AppGoFile + "\n</CurrentAppGoFile>",
+ Text: "<CurrentAppGoFile>\n" + appText + "\n</CurrentAppGoFile>",
}Hide from UI conversion:
@@ func (m *OpenAIChatMessage) ConvertToUIMessage() *uctypes.UIMessage {
- case "input_text", "output_text":
+ case "input_text", "output_text":
+ if strings.HasPrefix(block.Text, "<CurrentAppGoFile>") {
+ // internal context for model; do not show in UI
+ continue
+ }
if strings.HasPrefix(block.Text, "<AttachedTextFile_") {🏁 Script executed:
#!/bin/bash
# Ensure no UI shows the injected block
rg -nP -C2 '<CurrentAppGoFile>' pkg/ frontend/ || trueLength of output: 1527
---
🏁 Script executed:
```shell
#!/bin/bash
# Find ConvertToUIMessage function
ast-grep --pattern 'func $_() ConvertToUIMessage() $_'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Also search for the function more broadly
rg -nP -A10 'func.*ConvertToUIMessage' pkg/Length of output: 1862
🏁 Script executed:
#!/bin/bash
# Check if there's already filtering for internal blocks
rg -nP 'AttachedTextFile_|input_text.*continue|internal' pkg/aiusechat/openai/openai-convertmessage.goLength of output: 303
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full ConvertToUIMessage implementation (lines 520 onwards)
sed -n '520,600p' pkg/aiusechat/openai/openai-convertmessage.go
Length of output: 2333
🏁 Script executed:
#!/bin/bash
# Also check the anthropic version to see if same pattern exists there
sed -n '634,700p' pkg/aiusechat/anthropic/anthropic-convertmessage.goLength of output: 1994
AppGoFile injection: cap size and filter from UI in both OpenAI and Anthropic providers
The review concern is valid. The <CurrentAppGoFile> blocks currently leak into the UI unconverted:
- In OpenAI's
ConvertToUIMessage()(line 520), they pass through as plain text alongside filtered<AttachedTextFile_blocks (line 531). - In Anthropic's
ConvertToUIMessage()(line 634), all text blocks are directly appended to UI with no filtering mechanism.
Apply both fixes:
-
OpenAI (pkg/aiusechat/openai/openai-convertmessage.go, lines 197–213):
- Cap injected size to avoid token bloat
- Filter
<CurrentAppGoFile>blocks from UI conversion (add check at line 531 alongside existing<AttachedTextFile_filter)
-
Anthropic (pkg/aiusechat/anthropic/anthropic-convertmessage.go, lines ~81–85 and ~642):
- Add matching size cap to the injection
- Add filtering in
ConvertToUIMessage()to exclude<CurrentAppGoFile>blocks from UI output
🤖 Prompt for AI Agents
In pkg/aiusechat/openai/openai-convertmessage.go around lines 197–213, the
injected <CurrentAppGoFile> block is added without size limits and later flows
into the UI; cap the injected payload (e.g., trim to a safe max bytes/characters
or tokens) before wrapping it in the XML block and appending to the last user
message, and in ConvertToUIMessage() (around line 531) add a filter to skip any
content blocks whose Text starts with "<CurrentAppGoFile>" (similar to the
existing "<AttachedTextFile_" check) so they are not passed to the UI. Also
apply the same fixes for Anthropic: in
pkg/aiusechat/anthropic/anthropic-convertmessage.go at the injection site
(~lines 81–85) cap the AppGoFile size before wrapping and inserting, and in
ConvertToUIMessage() (~line 642) add a filter to exclude "<CurrentAppGoFile>"
blocks from UI output.
| wps.Broker.Publish(wps.WaveEvent{ | ||
| Event: wps.Event_WaveAppAppGoUpdated, | ||
| Scopes: []string{appId}, | ||
| }) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Event publish after write/edit is solid; updated tool semantics read clearly.
- Good move publishing wps.Event_WaveAppAppGoUpdated after writes/edits.
- Tool descriptions/schema emphasize exact-once and atomic edits — clear.
- Approval text removed from descriptions, matching our guidance.
Verify that subscribers listen on the same scope string used here (appId). Also confirm ReplaceInAppFile enforces unique match + atomicity.
Based on learnings
Also applies to: 111-114, 121-136, 163-167, 176-200
🏁 Script executed:
#!/bin/bash
# Check subscribers for Event_WaveAppAppGoUpdated and their scope expectations
rg -nP 'Event_WaveAppAppGoUpdated|waveapp:appgoupdated|builder:appid|appgoupdated' -C3
# Inspect ReplaceInAppFile semantics
rg -n 'ReplaceInAppFile' -C4Length of output: 5230
Need to verify the full implementation of ReplaceInAppFile to confirm atomicity and unique match enforcement.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full ReplaceInAppFile function implementation
rg -n 'func ReplaceInAppFile' -A 30 pkg/waveappstore/waveappstore.go
Length of output: 902
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and inspect fileutil.ReplaceInFile implementation
rg -n 'func ReplaceInFile' -A 25 --type go
# Also check for fileutil package location
fd -e go -p 'fileutil' | head -20
Length of output: 1712
| if req.BuilderAppId != "" { | ||
| chatOpts.Tools = append(chatOpts.Tools, | ||
| GetBuilderWriteAppFileToolDefinition(req.BuilderAppId), | ||
| GetBuilderEditAppFileToolDefinition(req.BuilderAppId), | ||
| GetBuilderListFilesToolDefinition(req.BuilderAppId), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Authorization gap: anyone who can hit this endpoint can write/edit any app by passing an arbitrary builderappid.
No ownership/ACL check is performed before wiring the builder tools. Validate the requesting client is authorized for the provided appId and (optionally) that it matches the builderid scope.
Proposed guard (pseudocode to be backed by a real check in waveappstore or wstore):
@@
// Get client ID from database
client, err := wstore.DBGetSingleton[*waveobj.Client](r.Context())
@@
+ // Enforce app access before exposing builder tools/generator
+ if req.BuilderAppId != "" {
+ ok, err := waveappstore.CanClientAccessApp(r.Context(), client.OID, req.BuilderAppId)
+ if err != nil {
+ http.Error(w, fmt.Sprintf("failed to authorize app: %v", err), http.StatusInternalServerError)
+ return
+ }
+ if !ok {
+ http.Error(w, "forbidden", http.StatusForbidden)
+ return
+ }
+ }Would you like me to draft the waveappstore.CanClientAccessApp helper?
🏁 Script executed:
#!/bin/bash
# Search for existing ownership/ACL helpers we could reuse.
rg -nP --no-heading -C2 '(CanClientAccessApp|Authorize|Access|Owner|ACL|Permissions|ValidateAppId)' pkg | sed -n '1,200p'Length of output: 11864
🏁 Script executed:
#!/bin/bash
# 1. Show full context around lines 729-735 in usechat.go
echo "=== Context around line 729-735 in usechat.go ==="
sed -n '700,750p' pkg/aiusechat/usechat.go
echo ""
echo "=== Search for BuilderAppId usage ==="
rg -nP --type=go 'BuilderAppId' -B2 -A2 | head -100Length of output: 4082
🏁 Script executed:
#!/bin/bash
# Find app ownership model in waveobj
echo "=== Search for app/client ownership relationship ==="
rg -nP --type=go 'App.*Owner|App.*Client|App.*User|App.*Creator' pkg/waveobj -B1 -A1 | head -60
echo ""
echo "=== Check waveappstore for any access filtering logic ==="
rg -nP --type=go 'func.*App.*\(' pkg/waveappstore/waveappstore.go | head -30
echo ""
echo "=== Find where client.OID is used for access checks ==="
rg -nP --type=go 'client\.OID|Client\.OID' pkg/aiusechat -B2 -A2 | head -50Length of output: 1511
🏁 Script executed:
#!/bin/bash
# Find Client object structure
echo "=== Client model definition ==="
rg -nP --type=go 'type Client struct' pkg/waveobj -A15
echo ""
echo "=== App namespace constants ==="
rg -nP --type=go 'AppNS|const.*Local|const.*Draft' pkg/waveappstore -B1 -A1 | head -40
echo ""
echo "=== Check if any tool definition validates access ==="
rg -nP --type=go 'GetBuilderWriteAppFileToolDefinition|GetBuilderEditAppFileToolDefinition|GetBuilderListFilesToolDefinition' pkg/aiusechat -A10 | head -80Length of output: 6766
Authorization gap confirmed: implement client ownership validation before wiring builder tools.
The vulnerability is real. Both the BuilderAppGenerator (lines 717-727) and Tools definitions (lines 729-735) use BuilderAppId directly from the user request without verifying the client owns that app. Since waveappstore functions accept only appId and perform no access filtering, any client can read/write arbitrary apps.
Implement the proposed guard pattern:
- Before lines 717 and 729, add authorization check using client.OID and req.BuilderAppId
- Return HTTP 403 if client lacks access, HTTP 500 if authorization lookup fails
- This requires either: defining an app ownership model or restricting builders to namespaces the client can access
Locations requiring fixes:
- Lines 717-727: BuilderAppGenerator function setup (add auth check before)
- Lines 729-735: Tools wiring block (add same auth check before)
The review's suggested pattern is correct; implement waveappstore.CanClientAccessApp(ctx, client.OID, req.BuilderAppId) or equivalent.
🤖 Prompt for AI Agents
In pkg/aiusechat/usechat.go around lines 717-727 and 729-735, requests use
req.BuilderAppId directly to wire BuilderAppGenerator and builder tools—add an
authorization guard before each block that calls a service like
waveappstore.CanClientAccessApp(ctx, client.OID, req.BuilderAppId) (or
equivalent ownership lookup), return HTTP 403 if the call returns false, and
return HTTP 500 if the auth lookup itself errors; ensure the guard prevents
wiring the generator/tools when unauthorized and update error handling to
produce the appropriate status codes (or restrict builders to client-accessible
namespaces if an ownership model is implemented).
| func (mrlb *MultiReaderLineBuffer) callLineCallback(line string) { | ||
| mrlb.lock.Lock() | ||
| defer mrlb.lock.Unlock() | ||
|
|
||
| if mrlb.lineCallback != nil { | ||
| mrlb.lineCallback(line) | ||
| } | ||
| } |
There was a problem hiding this comment.
Calling callback while holding the lock risks deadlocks.
If the callback ever calls any method that also takes the lock, it deadlocks. Copy the callback under lock, then invoke it outside.
Apply this diff:
-func (mrlb *MultiReaderLineBuffer) callLineCallback(line string) {
- mrlb.lock.Lock()
- defer mrlb.lock.Unlock()
-
- if mrlb.lineCallback != nil {
- mrlb.lineCallback(line)
- }
-}
+func (mrlb *MultiReaderLineBuffer) callLineCallback(line string) {
+ mrlb.lock.Lock()
+ cb := mrlb.lineCallback
+ mrlb.lock.Unlock()
+ if cb != nil {
+ cb(line)
+ }
+}📝 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.
| func (mrlb *MultiReaderLineBuffer) callLineCallback(line string) { | |
| mrlb.lock.Lock() | |
| defer mrlb.lock.Unlock() | |
| if mrlb.lineCallback != nil { | |
| mrlb.lineCallback(line) | |
| } | |
| } | |
| func (mrlb *MultiReaderLineBuffer) callLineCallback(line string) { | |
| mrlb.lock.Lock() | |
| cb := mrlb.lineCallback | |
| mrlb.lock.Unlock() | |
| if cb != nil { | |
| cb(line) | |
| } | |
| } |
🤖 Prompt for AI Agents
In pkg/utilds/multireaderlinebuffer.go around lines 47 to 54, the code currently
invokes the lineCallback while holding mrlb.lock which can deadlock if the
callback re-enters locked methods; change it to capture the callback into a
local variable while holding the lock (and release the lock immediately after),
then invoke the copied callback with the line outside the lock so the callback
runs without the lock held.
| func (ws *WshServer) WriteAppFileCommand(ctx context.Context, data wshrpc.CommandWriteAppFileData) error { | ||
| contents, err := base64.StdEncoding.DecodeString(data.Data64) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to decode data64: %w", err) | ||
| } | ||
| return waveappstore.WriteAppFile(data.AppId, data.FileName, contents) | ||
| } |
There was a problem hiding this comment.
Guard against oversized writes.
Decode can yield very large payloads. Enforce MaxFileSize to avoid DoS/memory spikes.
func (ws *WshServer) WriteAppFileCommand(ctx context.Context, data wshrpc.CommandWriteAppFileData) error {
contents, err := base64.StdEncoding.DecodeString(data.Data64)
if err != nil {
return fmt.Errorf("failed to decode data64: %w", err)
}
+ if int64(len(contents)) > wshrpc.MaxFileSize {
+ return fmt.Errorf("file too large: %d > %d", len(contents), wshrpc.MaxFileSize)
+ }
return waveappstore.WriteAppFile(data.AppId, data.FileName, contents)
}Committable suggestion skipped: line range outside the PR's diff.
…ToArray , base64ToArrayBuffer)
1 participant
No description provided.