more builder updates #2553
more builder updates #2553
Conversation
sawka
commented
Nov 14, 2025
sawka
commented
- load manifest metadata into the FE
- builder only edits draft/ apps (convert local => draft)
- gofmt app.go after saving (AI tools and manual user save)
- dont open duplicate builder windows
- remix app context menu in waveapp
- add icon/iconcolor in appmeta and implement in the wave block frame
…f the FE... (pull from manifest)
…ext menu, move save button into code editor
WalkthroughThis pull request introduces a comprehensive feature set for enhanced app metadata handling, builder window management, and draft app support. It refactors the AppManifest structure to nest metadata fields into a new AppMeta type (replacing flat title/description fields with a structured object including icon and color properties), adds new RPC commands for creating draft apps from local applications and writing Go files with automatic formatting, extends the Electron IPC layer with builder window management handlers, enhances the Tsunami view with dynamic metadata loading and a remix-to-builder capability, updates the builder interface to convert local apps to drafts, and introduces Go code formatting utilities that integrate gofmt. The changes span TypeScript frontend components, Go backend services, RPC type definitions, and Electron main process utilities. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Key areas requiring additional scrutiny:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
pkg/wshrpc/wshserver/wshserver.go (1)
1017-1035: Formatting errors are silently ignored.
waveapputil.FormatGoCodereturns the original contents ifgofmtfails (as shown in the relevant code snippets), which means formatting errors are silently swallowed. The user receives back unformatted code without any indication that formatting failed. Consider either:
- Logging when formatting fails so developers can diagnose issues
- Returning a warning field in the response to inform the frontend
- Making formatting failure a non-fatal error that's reported but doesn't block the save
Example with logging:
func (ws *WshServer) WriteAppGoFileCommand(ctx context.Context, data wshrpc.CommandWriteAppGoFileData) (*wshrpc.CommandWriteAppGoFileRtnData, error) { if data.AppId == "" { return nil, fmt.Errorf("must provide an appId to WriteAppGoFileCommand") } contents, err := base64.StdEncoding.DecodeString(data.Data64) if err != nil { return nil, fmt.Errorf("failed to decode data64: %w", err) } + originalLen := len(contents) formattedOutput := waveapputil.FormatGoCode(contents) + if len(formattedOutput) == originalLen && bytes.Equal(formattedOutput, contents) { + // Formatting may have failed, log for diagnostics + log.Printf("gofmt may have failed for appId %s, returning original contents", data.AppId) + } err = waveappstore.WriteAppFile(data.AppId, "app.go", formattedOutput) if err != nil { return nil, err } encoded := base64.StdEncoding.EncodeToString(formattedOutput) return &wshrpc.CommandWriteAppGoFileRtnData{Data64: encoded}, nil }pkg/waveappstore/waveappstore.go (1)
410-440: Verify gofmt version compatibility and error handling.The
FormatGoFilefunction executesgofmt -wdirectly on the file. Consider these potential issues:
Version compatibility: The function relies on
gofmtbeing available at the resolved path, but doesn't verify the version. Different Go versions may have different formatting rules.Partial writes: If
gofmt -wfails midway (crash, disk full, permissions issue), the file could be left in a partially formatted or corrupted state. Consider using a temp file approach:
- Write to temp file
- Verify formatting succeeded
- Rename temp over original
Concurrent modification: No locking prevents concurrent calls to
FormatGoFileorWriteAppFileon the same file, which could lead to race conditions.Safer implementation using atomic writes:
func FormatGoFile(appId string, fileName string) error { if err := ValidateAppId(appId); err != nil { return fmt.Errorf("invalid appId: %w", err) } appDir, err := GetAppDir(appId) if err != nil { return err } filePath, err := validateAndResolveFilePath(appDir, fileName) if err != nil { return err } if filepath.Ext(filePath) != ".go" { return fmt.Errorf("file is not a Go file: %s", fileName) } gofmtPath, err := waveapputil.ResolveGoFmtPath() if err != nil { return fmt.Errorf("failed to resolve gofmt path: %w", err) } - cmd := exec.Command(gofmtPath, "-w", filePath) - if output, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("gofmt failed: %w\nOutput: %s", err, string(output)) - } + // Format to stdout instead of in-place to avoid partial writes + cmd := exec.Command(gofmtPath, filePath) + formattedOutput, err := cmd.Output() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok { + return fmt.Errorf("gofmt failed: %w\nOutput: %s", err, string(ee.Stderr)) + } + return fmt.Errorf("gofmt failed: %w", err) + } + + // Atomically write formatted output + tempFile := filePath + ".fmt.tmp" + if err := os.WriteFile(tempFile, formattedOutput, 0644); err != nil { + return fmt.Errorf("failed to write formatted output: %w", err) + } + if err := os.Rename(tempFile, filePath); err != nil { + os.Remove(tempFile) + return fmt.Errorf("failed to replace original file: %w", err) + } return nil }pkg/blockcontroller/tsunamicontroller.go (1)
137-186: Graceful manifest loading with potential for error logging.The manifest metadata loading is implemented with good defensive programming—errors are silently ignored, allowing the app to start even if manifest reading fails. However, consider logging manifest read failures for debugging purposes.
Add logging for diagnostics:
// Read and set app metadata from manifest if appId is available if appId != "" { - if manifest, err := waveappstore.ReadAppManifest(appId); err == nil { + manifest, err := waveappstore.ReadAppManifest(appId) + if err != nil { + log.Printf("TsunamiController: failed to read manifest for appId %s: %v", appId, err) + } else { blockRef := waveobj.MakeORef(waveobj.OType_Block, c.blockId) rtInfo := make(map[string]any) if manifest.AppMeta.Title != "" { rtInfo["tsunami:title"] = manifest.AppMeta.Title } // ... rest of metadata population ... } }frontend/builder/builder-apppanel.tsx (1)
254-258: Consider making thedraft/stripping more explicit
builderAppId.replace("draft/", "")works given the expecteddraft/<name>pattern, butreplacewill also modify ids where"draft/"appears later in the string. If you ever relax the invariant, a safer variant would be:-const appName = builderAppId.replace("draft/", ""); +const appName = builderAppId.startsWith("draft/") + ? builderAppId.slice("draft/".length) + : builderAppId;pkg/buildercontroller/buildercontroller.go (2)
212-220: Centralizing scaffold and SDK version config is good; minor duplication onlyUsing
waveapputil.GetTsunamiScaffoldPath()andDefaultTsunamiSdkVersionkeeps builder config consistent with other callers. Note this now callsGetFullConfig()both insideGetTsunamiScaffoldPathand again forsettingshere; if config fetching is ever expensive, you could pass settings into a helper instead of re-reading them, but this is non-blocking.
509-538: Propagate icon metadata when constructing BuilderStatus manifest
AppMetanow includesIconandIconColor, butGetStatusonly forwardsTitleandShortDesc. This means UIs that depend on builder status (e.g., Tsunami app frame icons) won’t see icon metadata even if it’s present in the manifest.You can fill these fields to keep the manifest round‑trip complete:
- wshrpcManifest := &wshrpc.AppManifest{ - AppMeta: wshrpc.AppMeta{ - Title: manifest.AppMeta.Title, - ShortDesc: manifest.AppMeta.ShortDesc, - }, + wshrpcManifest := &wshrpc.AppManifest{ + AppMeta: wshrpc.AppMeta{ + Title: manifest.AppMeta.Title, + ShortDesc: manifest.AppMeta.ShortDesc, + Icon: manifest.AppMeta.Icon, + IconColor: manifest.AppMeta.IconColor, + },pkg/waveapputil/waveapputil.go (2)
30-63: ResolveGoFmtPath is reasonable; consider a fallback for non-standard layoutsThe resolution logic (configured
TsunamiGoPath→FindGoExecutable()→gofmtin the same dir, with sanity checks) is solid. For uncommon installations wheregofmtisn’t colocated withgo, you might want a final fallback toexec.LookPath("gofmt")before failing, but that’s an optional robustness improvement.
65-79: FormatGoCode behavior is safe; minor efficiency tweak possibleGracefully returning the original contents when gofmt can’t be resolved or errors out avoids breaking the save path. To reduce allocations you could switch to
bytes.NewReader(contents)instead ofstrings.NewReader(string(contents)), but functionally this is fine.-import "strings" +import "bytes" ... - cmd := exec.Command(gofmtPath) - cmd.Stdin = strings.NewReader(string(contents)) + cmd := exec.Command(gofmtPath) + cmd.Stdin = bytes.NewReader(contents)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
.roo/rules/rules.md(1 hunks)emain/emain-ipc.ts(3 hunks)emain/emain-menu.ts(2 hunks)emain/preload.ts(1 hunks)frontend/app/block/blockframe.tsx(3 hunks)frontend/app/store/wshclientapi.ts(2 hunks)frontend/app/view/tsunami/tsunami.tsx(6 hunks)frontend/builder/app-selection-modal.tsx(3 hunks)frontend/builder/builder-app.tsx(2 hunks)frontend/builder/builder-apppanel.tsx(2 hunks)frontend/builder/store/builder-apppanel-model.ts(1 hunks)frontend/builder/tabs/builder-codetab.tsx(4 hunks)frontend/types/custom.d.ts(2 hunks)frontend/types/gotypes.d.ts(4 hunks)frontend/wave.ts(1 hunks)pkg/aiusechat/tools_builder.go(3 hunks)pkg/blockcontroller/tsunamicontroller.go(3 hunks)pkg/buildercontroller/buildercontroller.go(3 hunks)pkg/waveappstore/waveappstore.go(2 hunks)pkg/waveapputil/waveapputil.go(1 hunks)pkg/waveobj/objrtinfo.go(2 hunks)pkg/wps/wpstypes.go(1 hunks)pkg/wshrpc/wshclient/wshclient.go(2 hunks)pkg/wshrpc/wshrpctypes.go(7 hunks)pkg/wshrpc/wshserver/wshserver.go(3 hunks)tsunami/engine/clientimpl.go(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-10-15T03:21:02.229Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 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
📚 Learning: 2025-01-22T01:28:41.417Z
Learnt from: esimkowitz
Repo: wavetermdev/waveterm PR: 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/wshserver/wshserver.gopkg/wshrpc/wshclient/wshclient.go
📚 Learning: 2025-10-15T03:18:52.647Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: emain/emain-window.ts:811-828
Timestamp: 2025-10-15T03:18:52.647Z
Learning: In emain/emain-window.ts, within the relaunchBrowserWindows function, ClientService.GetClientData() is guaranteed to return a valid client object and never null/undefined. The backend ensures a client is initialized before startup, so no null-guard is needed when accessing clientData.windowids.
Applied to files:
emain/emain-ipc.ts
📚 Learning: 2025-11-01T00:57:23.025Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2504
File: frontend/app/aipanel/aipanel-contextmenu.ts:15-16
Timestamp: 2025-11-01T00:57:23.025Z
Learning: In the waveterm codebase, types defined in custom.d.ts are globally available and do not require explicit imports. Backend types defined in gotypes.d.ts are also globally available.
Applied to files:
.roo/rules/rules.md
📚 Learning: 2025-10-21T05:09:26.916Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2465
File: frontend/app/onboarding/onboarding-upgrade.tsx:13-21
Timestamp: 2025-10-21T05:09:26.916Z
Learning: In the waveterm codebase, clientData is loaded and awaited in wave.ts before React runs, ensuring it is always available when components mount. This means atoms.client will have data on first render.
Applied to files:
frontend/app/view/tsunami/tsunami.tsx
🧬 Code graph analysis (21)
pkg/aiusechat/tools_builder.go (2)
pkg/waveapputil/waveapputil.go (1)
FormatGoCode(65-79)pkg/waveappstore/waveappstore.go (2)
WriteAppFile(264-288)FormatGoFile(410-440)
frontend/builder/tabs/builder-codetab.tsx (3)
frontend/app/view/webview/webview.tsx (1)
handleKeyDown(336-347)pkg/vdom/vdom_types.go (1)
WaveKeyboardEvent(233-247)frontend/util/util.ts (1)
cn(485-485)
frontend/app/block/blockframe.tsx (2)
pkg/waveobj/wtype.go (2)
Block(282-290)Block(292-294)frontend/app/block/blockutil.tsx (1)
getBlockHeaderIcon(131-149)
pkg/waveapputil/waveapputil.go (3)
pkg/wconfig/filewatcher.go (1)
GetWatcher(33-57)pkg/wavebase/wavebase.go (1)
GetWaveAppPath(113-115)tsunami/build/build.go (1)
FindGoExecutable(130-165)
pkg/wshrpc/wshserver/wshserver.go (5)
frontend/app/store/wshclientapi.ts (2)
WriteAppGoFileCommand(636-638)MakeDraftFromLocalCommand(371-373)pkg/wshrpc/wshclient/wshclient.go (2)
WriteAppGoFileCommand(759-762)MakeDraftFromLocalCommand(450-453)pkg/wshrpc/wshrpctypes.go (4)
CommandWriteAppGoFileData(1015-1018)CommandWriteAppGoFileRtnData(1020-1022)CommandMakeDraftFromLocalData(1099-1101)CommandMakeDraftFromLocalRtnData(1103-1105)pkg/waveapputil/waveapputil.go (1)
FormatGoCode(65-79)pkg/waveappstore/waveappstore.go (2)
WriteAppFile(264-288)MakeDraftFromLocal(180-217)
pkg/wshrpc/wshclient/wshclient.go (3)
frontend/app/store/wshclientapi.ts (2)
MakeDraftFromLocalCommand(371-373)WriteAppGoFileCommand(636-638)pkg/wshutil/wshrpc.go (1)
WshRpc(47-61)pkg/wshrpc/wshrpctypes.go (5)
CommandMakeDraftFromLocalData(1099-1101)RpcOpts(361-367)CommandMakeDraftFromLocalRtnData(1103-1105)CommandWriteAppGoFileData(1015-1018)CommandWriteAppGoFileRtnData(1020-1022)
emain/emain-menu.ts (1)
emain/emain-ipc.ts (1)
openBuilderWindow(29-40)
frontend/app/store/wshclientapi.ts (1)
pkg/wshrpc/wshrpctypes.go (5)
CommandMakeDraftFromLocalData(1099-1101)RpcOpts(361-367)CommandMakeDraftFromLocalRtnData(1103-1105)CommandWriteAppGoFileData(1015-1018)CommandWriteAppGoFileRtnData(1020-1022)
emain/emain-ipc.ts (1)
emain/emain-builder.ts (3)
getAllBuilderWindows(30-32)createBuilderWindow(34-112)getBuilderWindowByWebContentsId(26-28)
frontend/builder/app-selection-modal.tsx (3)
frontend/app/store/wshclientapi.ts (1)
RpcApi(672-672)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)frontend/app/store/global.ts (3)
globalStore(839-839)atoms(816-816)getApi(826-826)
pkg/blockcontroller/tsunamicontroller.go (8)
pkg/waveapputil/waveapputil.go (2)
GetTsunamiScaffoldPath(21-28)DefaultTsunamiSdkVersion(19-19)pkg/waveappstore/waveappstore.go (1)
ReadAppManifest(706-728)pkg/waveobj/waveobj.go (1)
MakeORef(71-76)pkg/waveobj/wtype.go (1)
OType_Block(30-30)tsunami/engine/clientimpl.go (1)
AppMeta(43-48)pkg/wstore/wstore_rtinfo.go (1)
SetRTInfo(84-125)pkg/wps/wps.go (1)
Broker(48-52)pkg/wps/wpstypes.go (2)
WaveEvent(26-32)Event_TsunamiUpdateMeta(23-23)
tsunami/engine/clientimpl.go (2)
pkg/wshrpc/wshrpctypes.go (3)
AppMeta(1054-1059)SecretMeta(1061-1064)AppManifest(1066-1071)tsunami/app/defaultclient.go (1)
AppMeta(22-22)
pkg/waveappstore/waveappstore.go (1)
pkg/waveapputil/waveapputil.go (1)
ResolveGoFmtPath(30-63)
frontend/types/gotypes.d.ts (3)
pkg/wshrpc/wshrpctypes.go (6)
AppMeta(1054-1059)SecretMeta(1061-1064)CommandMakeDraftFromLocalData(1099-1101)CommandMakeDraftFromLocalRtnData(1103-1105)CommandWriteAppGoFileData(1015-1018)CommandWriteAppGoFileRtnData(1020-1022)tsunami/engine/clientimpl.go (2)
AppMeta(43-48)SecretMeta(50-53)tsunami/app/atom.go (1)
SecretMeta(26-29)
frontend/builder/builder-app.tsx (2)
frontend/util/util.ts (1)
isBlank(494-494)frontend/builder/app-selection-modal.tsx (1)
AppSelectionModal(100-233)
pkg/wshrpc/wshrpctypes.go (3)
frontend/app/store/wshclientapi.ts (2)
WriteAppGoFileCommand(636-638)MakeDraftFromLocalCommand(371-373)pkg/wshrpc/wshclient/wshclient.go (2)
WriteAppGoFileCommand(759-762)MakeDraftFromLocalCommand(450-453)tsunami/engine/clientimpl.go (1)
AppMeta(43-48)
pkg/buildercontroller/buildercontroller.go (2)
pkg/waveapputil/waveapputil.go (2)
GetTsunamiScaffoldPath(21-28)DefaultTsunamiSdkVersion(19-19)pkg/wshrpc/wshrpctypes.go (1)
AppMeta(1054-1059)
frontend/builder/store/builder-apppanel-model.ts (2)
frontend/app/store/wshclientapi.ts (1)
RpcApi(672-672)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
frontend/wave.ts (2)
frontend/app/store/wshclientapi.ts (1)
RpcApi(672-672)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
frontend/builder/builder-apppanel.tsx (4)
frontend/app/store/wshclientapi.ts (1)
RpcApi(672-672)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)frontend/app/store/modalmodel.ts (1)
modalsModel(45-45)frontend/app/modals/modal.tsx (1)
Modal(116-116)
frontend/app/view/tsunami/tsunami.tsx (7)
pkg/wshrpc/wshrpctypes.go (1)
AppMeta(1054-1059)tsunami/engine/clientimpl.go (1)
AppMeta(43-48)tsunami/app/defaultclient.go (1)
AppMeta(22-22)frontend/app/store/wshclientapi.ts (1)
RpcApi(672-672)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)frontend/app/store/global.ts (2)
WOS(866-866)getApi(826-826)frontend/app/store/wps.ts (1)
waveEventSubscribe(146-146)
🪛 markdownlint-cli2 (0.18.1)
.roo/rules/rules.md
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (43)
pkg/waveobj/objrtinfo.go (1)
9-10: LGTM! New metadata fields follow existing conventions.The addition of
TsunamiIconandTsunamiIconColorfields is consistent with the existing struct patterns and aligns with the PR objectives for enhanced app metadata support.pkg/wps/wpstypes.go (1)
23-23: LGTM!The new event constant follows the established naming convention and is appropriately placed with other event type declarations.
emain/emain-ipc.ts (2)
29-40: LGTM!The
openBuilderWindowfunction effectively prevents duplicate builder windows by reusing existing windows for the sameappId. The normalization ofappIdensures consistent comparison, and focusing the existing window provides good UX.
424-433: LGTM!The IPC handler properly updates the builder window's saved initialization options, enabling the frontend to propagate the app ID to an existing builder window.
emain/emain-menu.ts (1)
8-9: LGTM!The refactoring correctly replaces direct builder window creation with the new centralized
openBuilderWindowfunction, which provides window deduplication functionality.Also applies to: 132-132
frontend/app/view/tsunami/tsunami.tsx (4)
48-82: LGTM! Well-structured metadata initialization.The implementation correctly:
- Initializes app metadata from runtime info
- Subscribes to
tsunami:updatemetaevents for dynamic updates- Uses derived atoms for icon, iconColor, and name with sensible defaults
The pattern follows established practices in the codebase for event subscriptions and atom management.
161-177: LGTM! Remix functionality is properly guarded.The
remixInBuildermethod correctly:
- Guards against non-local apps with an early return
- Handles errors gracefully with logging
- Uses the new
MakeDraftFromLocalCommandRPC call- Opens the builder with the draft app ID via the Electron API
183-186: LGTM! Cleanup includes new subscription.The
disposemethod properly unsubscribes from the app metadata event subscription, preventing memory leaks.
225-235: LGTM! Conditional menu item rendering is correct.The "Remix WaveApp in Builder" option is appropriately shown only for local apps and includes proper separator placement.
frontend/types/custom.d.ts (2)
129-130: LGTM! API surface extensions are well-typed.The new
openBuilderandsetBuilderWindowAppIdmethods align with the IPC handlers added inemain-ipc.tsand support the builder window management workflow.
297-298: LGTM! ViewModel extension supports dynamic icon coloring.The optional
viewIconColoratom enables views to specify custom icon colors, as demonstrated in the tsunami view implementation.pkg/aiusechat/tools_builder.go (2)
123-124: LGTM! Automatic formatting improves code quality.Formatting the Go code before writing ensures consistent style, regardless of whether the content was generated by AI or manually edited. The
FormatGoCodefunction returns the original content if formatting fails, preventing errors from blocking the write operation.
258-259: LGTM! Post-edit formatting with appropriate error handling.Formatting after edits is beneficial, and the comment correctly explains that
gofmterrors are ignored because they may result from compilation errors that will be caught during the build step. This prevents formatting issues from blocking edits unnecessarily.frontend/builder/builder-app.tsx (1)
35-35: LGTM! Enforces draft-only editing workflow.The new
hasDraftApplogic correctly ensures that only draft apps display theBuilderWorkspace, enforcing the workflow where local apps must first be converted to drafts before editing. This aligns with the PR objective to "only edit draft/apps."Also applies to: 49-49
pkg/wshrpc/wshserver/wshserver.go (1)
1152-1160: LGTM!The implementation is clean and follows the established pattern for RPC command handlers. Error handling is appropriate, and the function correctly delegates to the waveappstore layer.
frontend/builder/tabs/builder-codetab.tsx (2)
62-73: Verify Save button doesn't obstruct editor content.The Save button is absolutely positioned at
top-1 right-4withz-50, which places it over the CodeEditor. Ensure this doesn't:
- Obstruct important editor UI elements (like the minimap or scrollbar on the right)
- Interfere with user interaction when editing near the top-right
- Remain visible and accessible at different viewport sizes
Consider testing these scenarios:
- Editor with minimap enabled (Monaco default)
- Long lines of code near the top
- Small window sizes
- When the button is disabled (grayed out) but still visible
If issues arise, consider alternative placements such as:
- Below the tab bar in a dedicated toolbar area
- In the block header (if this builder view has one)
- As a floating button with better margin/padding
31-35: LGTM! Clean save handler.The
handleSaveimplementation correctly checks forbuilderAppIdbefore callingmodel.saveAppFile, preventing errors when no app is loaded.frontend/wave.ts (1)
268-273: LGTM! Appropriate state persistence.The write-back logic correctly persists the
appIdToUseto RTInfo when it's not already present. The error handling is appropriate—logging the error without failing initialization allows the builder to continue functioning even if the write fails.frontend/builder/app-selection-modal.tsx (2)
122-146: LGTM! Solid draft conversion flow with good error handling.The implementation correctly converts local apps to drafts before editing, which aligns with the PR's objective of "Builder now only edits draft/apps." Key strengths:
- Idempotent: Based on
MakeDraftFromLocalimplementation (seen in relevant snippets), if a draft already exists, it returns the existing draft ID without overwriting- Error handling: Catches and displays conversion errors to the user, preventing silent failures
- State consistency: Updates all relevant state (RTInfo, global atom, document title, builder window) with the draft ID
148-159: LGTM! Consistent state propagation.The new app creation flow correctly uses
draft/namespace and consistently propagates the app ID to all necessary locations.emain/preload.ts (1)
68-69: LGTM! Clean IPC method additions.The new
openBuilderandsetBuilderWindowAppIdmethods follow the established pattern for IPC exposure and integrate cleanly with the existing API surface.pkg/blockcontroller/tsunamicontroller.go (1)
128-134: LGTM! Cleaner configuration resolution.Centralizing scaffold path and SDK version resolution through
waveapputilimproves maintainability and consistency across the codebase.frontend/app/block/blockframe.tsx (2)
88-104: LGTM! Clean icon color implementation.The icon color support is well-integrated:
- Optional parameter with safe defaults
- Style only applied when
iconColoris provided- Opacity set to 1.0 ensures color is fully visible
- Maintains existing behavior when no color is specified
184-184: LGTM! Proper atom usage for view icon color.The
viewIconColoris correctly retrieved from theviewModelusinguseAtomValueSafeand passed through to the rendering logic, maintaining consistency with other view metadata handling.Also applies to: 221-221
pkg/wshrpc/wshclient/wshclient.go (2)
449-453: New MakeDraftFromLocal RPC wrapper looks correctSignature, command string, and return type are consistent with
CommandMakeDraftFromLocal*definitions and other helpers in this file.
758-762: WriteAppGoFile RPC wrapper is consistent with RPC typesWrapper matches
CommandWriteAppGoFile*types and uses the same helper pattern as neighboring commands; no issues spotted.frontend/builder/builder-apppanel.tsx (1)
103-162: PublishAppModal state handling is solidThe three‑state flow (confirm/success/error) cleanly separates concerns, correctly handles missing
builderAppId, and surfaces backend error messages. WiringonOk/onClosethroughhandleCloselooks consistent with the modal API.frontend/app/store/wshclientapi.ts (2)
370-373: MakeDraftFromLocal TS wrapper matches backend contractMethod name, types, and
"makedraftfromlocal"command string align with the GoCommandMakeDraftFromLocal*definitions and client wrapper.
635-638: WriteAppGoFile TS wrapper is wired correctlyThe TS signature mirrors
CommandWriteAppGoFileData/RtnData, and the"writeappgofile"command string matches the Go side; no issues here.frontend/builder/store/builder-apppanel-model.ts (1)
244-261: Using WriteAppGoFile and updating with formatted content is correctSwitching to
WriteAppGoFileCommandand then decodingresult.data64back intoformattedContentensures the editor shows the gofmt’ed source and thatsaveNeededAtomresets as expected. Error handling and restart behavior are consistent with the rest of the model.frontend/types/gotypes.d.ts (4)
65-79: AppManifest/AppMeta typings align with Go metadata model
AppManifest.appmetaandAppMeta(title/shortdesc/icon/iconcolor) match the GoAppMetastruct and JSON tags, enabling richer manifest metadata on the frontend without type mismatches.
341-349: Draft-from-local command types are consistent
CommandMakeDraftFromLocalData.localappidandCommandMakeDraftFromLocalRtnData.draftappidcorrectly mirror the Go structs and JSON tags, so TS callers can rely on strong typing here.
524-534: WriteAppGoFile command/result typings look correct
CommandWriteAppGoFileDataandCommandWriteAppGoFileRtnDataline up with the Go definitions (appid,data64), matching the new RPC wrappers in both Go and TS.
915-920: New tsunami icon fields in ObjRTInfo are wired properlyAdding
"tsunami:icon"and"tsunami:iconcolor"toObjRTInfomatches how icon metadata is exposed elsewhere, and should support the updated Tsunami UI without breaking existing callers.pkg/waveapputil/waveapputil.go (1)
21-28: GetTsunamiScaffoldPath helper is straightforwardCentralizing scaffold path resolution here (settings override with app-path fallback) matches existing behavior and makes builder/tsunami callers consistent.
pkg/wshrpc/wshrpctypes.go (5)
159-176: New builder command constants are consistent with client/server usage
Command_WriteAppGoFile = "writeappgofile"andCommand_MakeDraftFromLocal = "makedraftfromlocal"match the strings used in both Go and TS client wrappers, keeping the command naming cohesive across layers.
333-350: WshRpcInterface additions match the new builder commandsThe new
WriteAppGoFileCommandandMakeDraftFromLocalCommandmethods follow existing builder interface conventions (ctx + data → rtn/error) and align with the new command types and frontend wrappers.
1015-1022: CommandWriteAppGoFile structs are correctly shaped*
AppId/Data64and the returnData64field reflect the intended API (send Go source, get formatted source back) and match the TS declarations and client usage.
1054-1070: AppMeta/AppManifest restructuring looks coherentIntroducing
AppMetaand embedding it inAppManifest(withappmetaJSON field) lines up with corresponding TS types and tsunami’s metadata model. This keeps manifest metadata cohesive while preserving existing config/data/secret fields.
1099-1105: Draft-from-local RPC structs are wired correctly
CommandMakeDraftFromLocalData.LocalAppIdandCommandMakeDraftFromLocalRtnData.DraftAppIduse the expected JSON field names and match frontend typings and client wrappers; no issues here.tsunami/engine/clientimpl.go (3)
46-47: LGTM! Icon metadata fields added correctly.The new
IconandIconColorfields properly extend the app metadata structure and align with the PR objectives. The field definitions are consistent with the corresponding structure inpkg/wshrpc/wshrpctypes.go(lines 1053-1058).
56-59: LGTM! AppManifest refactoring consolidates metadata properly.The restructuring from flat
AppTitleandAppShortDescfields to a nestedAppMetafield is a breaking API change, but it's clearly intentional and improves the structure by grouping all app metadata (title, description, icon, iconColor) into a single field. The structure matches exactly withpkg/wshrpc/wshrpctypes.go(lines 1065-1070), ensuring consistency across the codebase.
503-508: LGTM! GetAppManifest correctly implements the refactored structure.The method properly constructs and returns the new
AppManifeststructure with the nestedAppMetafield. The implementation correctly uses theappMetavariable (obtained fromGetAppMeta()on line 498) to populate theAppMetafield, ensuring all metadata including the new icon fields is properly included.
| ``` | ||
| import { getApi } from "@/store/global"; | ||
| getApi().getIsDev() | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
The code block should specify the language (TypeScript) for proper syntax highlighting and better documentation rendering.
Apply this diff:
-```
+```typescript
import { getApi } from "@/store/global";
getApi().getIsDev()
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
.roo/rules/rules.md around lines 65 to 69: the fenced code block is missing a
language identifier; update the opening fence from totypescript so the
block reads as a TypeScript fenced code block (i.e., replace the existing
opening triple backticks with ```typescript and leave the block content
unchanged).
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
