-
Notifications
You must be signed in to change notification settings - Fork 684
minor v0.13 fixes #2649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
minor v0.13 fixes #2649
Conversation
WalkthroughAdds centralized AI mode display-name generation ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/aiusechat/usechat.go (1)
83-93: Trim the secret before checking for emptiness to catch whitespace-only valuesRight now the
!exists || secret == ""check runs on the raw secret, and only afterwards youTrimSpace:secret, exists, err := secretstore.GetSecret(...) ... if !exists || secret == "" { ... } apiToken = strings.TrimSpace(secret)If the stored secret is all whitespace, this passes the check but becomes
""when assigned toapiToken, causing a confusing auth failure.Consider trimming first and validating the trimmed value:
- secret, exists, err := secretstore.GetSecret(config.APITokenSecretName) + rawSecret, exists, err := secretstore.GetSecret(config.APITokenSecretName) if err != nil { return nil, fmt.Errorf("failed to retrieve secret %s: %w", config.APITokenSecretName, err) } - if !exists || secret == "" { - return nil, fmt.Errorf("secret %s not found or empty", config.APITokenSecretName) - } - apiToken = strings.TrimSpace(secret) + secret := strings.TrimSpace(rawSecret) + if !exists || secret == "" { + return nil, fmt.Errorf("secret %s not found or empty", config.APITokenSecretName) + } + apiToken = secretThis keeps the desired whitespace normalization while failing fast on misconfigured secrets.
🧹 Nitpick comments (5)
pkg/secretstore/secretstore.go (1)
15-17: Trailing CR/LF trimming changes stored secret values
SetSecretnow strips all trailing\r/\ncharacters before persisting. That’s perfect for API keys copied with a newline, but it’s also a global behavior change for all secrets using this store (e.g., any multi‑line blobs that require a final newline).If this store is only intended for token‑style values, this is fine; otherwise, consider scoping the trim to the AI token call sites or documenting that trailing newlines are normalized so nobody relies on them later.
Also applies to: 232-235
frontend/app/aipanel/aipanelheader.tsx (1)
14-20: Header context menu hookup is straightforward and consistentWiring
onContextMenuon the header container to the same handler as the kebab click cleanly extends the existing menu affordance to right‑click without changing current click behavior. If you ever want to DRY this,handleContextMenucould just delegate tohandleKebabClick(e), but it’s fine as‑is.Also applies to: 23-26
frontend/app/aipanel/byokannouncement.tsx (1)
39-39: Check Tailwind!important syntax for the new link/button classesThe styling changes look good, but the new classes
!text-blue-400/hover:!text-blue-300use the old Tailwind v3 important‑modifier syntax. In Tailwind v4, the important flag must be at the end of the class name (e.g.text-blue-400!), per the v4 migration guide.If your build is already on Tailwind 4.x (as the repo context suggests), these prefixed
!…utilities may be ignored. It may be safer to either:
- Switch to the v4 style (
text-blue-400! hover:text-blue-300!), or- Drop the
!entirely if there’s no conflicting text color to override.Please confirm what Tailwind version is actually in use and adjust if needed.
Also applies to: 51-52, 60-61
frontend/app/aipanel/ai-utils.ts (1)
576-596: Centralized AI mode display-name helper is well-structured
getModeDisplayNamecleanly encapsulates the mode naming rules (explicit display name, azure‑legacy resource name, ormodel (provider)with fallbacks). This should keep dropdowns, menus, and context menus consistent and makes future tweaks to naming rules straightforward.frontend/app/aipanel/aimode.tsx (1)
10-11: AI mode UI now correctly reuses centralized display-name logicUsing
getModeDisplayNameinAIModeMenuItemand for the dropdown’sdisplayName/tooltip ensures all mode labels follow the same rules, and thedisplayIconfallback keeps the icon behavior predictable. Thepre-linedescription styling is a nice touch for multi-line descriptions.At some point it might be worth tightening
config’s type to something like{ mode: string } & AIModeConfigTypeinstead ofany, but that’s an existing pattern and not required for this change.Also applies to: 35-40, 43-47, 174-177, 203-207, 223-227
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
frontend/app/aipanel/ai-utils.ts(1 hunks)frontend/app/aipanel/aimode.tsx(9 hunks)frontend/app/aipanel/aipanel-contextmenu.ts(4 hunks)frontend/app/aipanel/aipanel.tsx(2 hunks)frontend/app/aipanel/aipanelheader.tsx(1 hunks)frontend/app/aipanel/byokannouncement.tsx(4 hunks)frontend/types/gotypes.d.ts(2 hunks)pkg/aiusechat/usechat.go(1 hunks)pkg/secretstore/secretstore.go(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/usechat.gofrontend/app/aipanel/aipanel.tsx
🧬 Code graph analysis (4)
frontend/app/aipanel/ai-utils.ts (1)
pkg/wconfig/settingsconfig.go (1)
AIModeConfigType(266-285)
frontend/app/aipanel/aimode.tsx (1)
frontend/app/aipanel/ai-utils.ts (1)
getModeDisplayName(582-596)
frontend/app/aipanel/aipanel.tsx (1)
frontend/app/aipanel/byokannouncement.tsx (1)
BYOKAnnouncement(73-73)
frontend/app/aipanel/aipanel-contextmenu.ts (3)
frontend/app/aipanel/ai-utils.ts (1)
getModeDisplayName(582-596)frontend/app/store/wshclientapi.ts (1)
RpcApi(682-682)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
⏰ 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 (6)
frontend/types/gotypes.d.ts (1)
1217-1218: Telemetry props extension is consistentThe additions of
"settings:customaimodes"and"settings:secretscount"to bothTEventPropsandTEventUserPropsmatch the existingsettings:*numeric counters and keep the shape of telemetry events consistent.Also applies to: 1301-1303
frontend/app/aipanel/aipanel.tsx (1)
86-88: Config-driven BYOK visibility and empty-state context menu wiring look solidUsing
fullConfig.waveaito detect non‑waveai@modes is a reasonable way to infer custom modes, and gating<BYOKAnnouncement />on!hasCustomModesshould avoid redundant BYOK prompts once users have configured their own modes. AttachingonContextMenuin the empty state tohandleWaveAIContextMenu(e, true)also keeps right‑click behavior consistent with the populated panel.Also applies to: 160-161, 503-505
frontend/app/aipanel/aipanel-contextmenu.ts (4)
4-4: ImportinggetModeDisplayNameto centralize labels is a good movePulling
getModeDisplayNamein here keeps AI mode display naming logic DRY and tied to a single helper. No issues with the import or usage pattern.
60-83: UsinggetModeDisplayNamefor Wave AI modes improves consistencySwapping the Wave AI mode label to
getModeDisplayName(config)ensures consistent naming and better fallbacks (model/provider or azure resource) without changing selection behavior.
86-113: Custom Modes labeling viagetModeDisplayNamematches Wave modesApplying the same
getModeDisplayName(config)helper for Custom Modes keeps their labels in sync with Wave AI modes and reuses the same fallback logic. The destructuring{ mode, ...config }still preserves all fields needed by the helper.
204-221: “Configure Modes” context menu action and telemetry wiring look correctThe separator placement keeps the new item visually grouped with AI settings, and the click handler both fires
RecordTEventCommand(withnoresponse: truefor non-blocking telemetry) and opens the Wave AI config viamodel.openWaveAIConfig(). This integrates cleanly with the existing RpcApi/TabRpcClient pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/app/aipanel/aipanelmessages.tsx (1)
11-15: LGTM!Tightening the
messagesprop type fromany[]toWaveUIMessage[]improves type safety and aligns with the typeduseChat<WaveUIMessage>inaipanel.tsx.Note: Line 78 still uses
as anyfor the placeholder streaming message. Consider defining a proper type-safe placeholder or a minimalWaveUIMessagefactory if this pattern is used elsewhere.pkg/aiusechat/usechat.go (1)
50-65: System prompt selection logic correctly gates on tools/widget accessRouting to
SystemPromptText_NoToolswhen!hasToolsCapability || !widgetAccessand skipping the strict tool add-on in that case is a solid way to avoid misleading the model about tools.Minor nit:
apiTypeis now unused ingetSystemPrompt. If you don’t plan to specialize prompts by API type again, consider dropping that parameter (and its argument at the call site) to reduce noise.Also applies to: 663-667
pkg/aiusechat/usechat-mode.go (1)
265-274: Mode-config broadcast wiring is reasonable; consider whether persistence is desiredPublishing
AIModeConfigUpdateonEvent_AIModeConfigviawps.Brokermatches the new event type. If you want late subscribers or reconnecting frontends to immediately see the last-known mode config without a separate RPC, it may be worth reviewing whether this event should be persisted in the broker (mirroring how other config-like events are treated).frontend/wave.ts (1)
204-212: Wave AI mode config initialization is correct; consider parallelizing RPCsStoring
waveaiModeConfig.configsintoatoms.waveaiModeConfigAtomin bothinitWaveandinitBuilderwires the new RPC into global state correctly.To reduce startup latency a bit, you could fetch full config and AI mode config in parallel instead of sequentially, e.g.:
- await loadMonaco(); - const fullConfig = await RpcApi.GetFullConfigCommand(TabRpcClient); - console.log("fullconfig", fullConfig); - globalStore.set(atoms.fullConfigAtom, fullConfig); - const waveaiModeConfig = await RpcApi.GetWaveAIModeConfigCommand(TabRpcClient); - globalStore.set(atoms.waveaiModeConfigAtom, waveaiModeConfig.configs); + await loadMonaco(); + const [fullConfig, waveaiModeConfig] = await Promise.all([ + RpcApi.GetFullConfigCommand(TabRpcClient), + RpcApi.GetWaveAIModeConfigCommand(TabRpcClient), + ]); + console.log("fullconfig", fullConfig); + globalStore.set(atoms.fullConfigAtom, fullConfig); + globalStore.set(atoms.waveaiModeConfigAtom, waveaiModeConfig.configs);(and similarly in
initBuilder).Also applies to: 284-290
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
cmd/server/main-server.go(2 hunks)frontend/app/aipanel/aimode.tsx(8 hunks)frontend/app/aipanel/aipanel.tsx(4 hunks)frontend/app/aipanel/aipanelmessages.tsx(1 hunks)frontend/app/aipanel/byokannouncement.tsx(4 hunks)frontend/app/store/global.ts(3 hunks)frontend/app/store/wshclientapi.ts(1 hunks)frontend/types/custom.d.ts(2 hunks)frontend/types/gotypes.d.ts(3 hunks)frontend/wave.ts(2 hunks)pkg/aiusechat/usechat-mode.go(2 hunks)pkg/aiusechat/usechat-prompts.go(1 hunks)pkg/aiusechat/usechat.go(3 hunks)pkg/tsgen/tsgen.go(1 hunks)pkg/wconfig/settingsconfig.go(1 hunks)pkg/wps/wpstypes.go(1 hunks)pkg/wshrpc/wshclient/wshclient.go(1 hunks)pkg/wshrpc/wshrpctypes.go(2 hunks)pkg/wshrpc/wshserver/wshserver.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/app/aipanel/aimode.tsx
- frontend/app/aipanel/byokannouncement.tsx
🧰 Additional context used
🧠 Learnings (5)
📚 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:
frontend/types/custom.d.tsfrontend/app/store/global.ts
📚 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/types/custom.d.tsfrontend/app/store/global.tsfrontend/wave.ts
📚 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/wshclient/wshclient.go
📚 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/usechat-prompts.gopkg/aiusechat/usechat.go
📚 Learning: 2025-10-14T06:30:54.763Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2430
File: frontend/app/aipanel/aimessage.tsx:137-144
Timestamp: 2025-10-14T06:30:54.763Z
Learning: In `frontend/app/aipanel/aimessage.tsx`, the `AIToolUseGroup` component splits file operation tool calls into separate batches (`fileOpsNeedApproval` and `fileOpsNoApproval`) based on their approval state before passing them to `AIToolUseBatch`. This ensures each batch has homogeneous approval states, making group-level approval handling valid.
Applied to files:
frontend/app/aipanel/aipanel.tsx
🧬 Code graph analysis (13)
frontend/types/custom.d.ts (1)
pkg/wconfig/settingsconfig.go (1)
AIModeConfigType(266-285)
pkg/wshrpc/wshclient/wshclient.go (3)
frontend/app/store/wshclientapi.ts (1)
GetWaveAIModeConfigCommand(356-358)pkg/wshutil/wshrpc.go (1)
WshRpc(47-61)pkg/wshrpc/wshrpctypes.go (1)
RpcOpts(366-372)
frontend/app/aipanel/aipanelmessages.tsx (1)
frontend/app/aipanel/aitypes.ts (1)
WaveUIMessage(35-35)
pkg/wshrpc/wshrpctypes.go (3)
frontend/app/store/wshclientapi.ts (1)
GetWaveAIModeConfigCommand(356-358)pkg/wshrpc/wshclient/wshclient.go (1)
GetWaveAIModeConfigCommand(432-435)pkg/wconfig/settingsconfig.go (1)
AIModeConfigUpdate(287-289)
pkg/tsgen/tsgen.go (1)
pkg/wconfig/settingsconfig.go (1)
AIModeConfigUpdate(287-289)
cmd/server/main-server.go (1)
pkg/aiusechat/usechat-mode.go (1)
InitAIModeConfigWatcher(242-246)
pkg/aiusechat/usechat-mode.go (4)
pkg/wconfig/filewatcher.go (1)
GetWatcher(36-60)pkg/wconfig/settingsconfig.go (2)
AIModeConfigType(266-285)AIModeConfigUpdate(287-289)pkg/wps/wps.go (1)
Broker(46-50)pkg/wps/wpstypes.go (2)
WaveEvent(27-33)Event_AIModeConfig(24-24)
frontend/app/store/global.ts (2)
pkg/wconfig/settingsconfig.go (2)
AIModeConfigType(266-285)AIModeConfigUpdate(287-289)frontend/app/store/jotaiStore.ts (1)
globalStore(3-3)
pkg/aiusechat/usechat.go (2)
pkg/aiusechat/usechat-prompts.go (2)
SystemPromptText_OpenAI(8-44)SystemPromptText_NoTools(46-81)pkg/aiusechat/uctypes/uctypes.go (1)
AICapabilityTools(172-172)
pkg/wshrpc/wshserver/wshserver.go (4)
frontend/app/store/wshclientapi.ts (1)
GetWaveAIModeConfigCommand(356-358)pkg/wshrpc/wshclient/wshclient.go (1)
GetWaveAIModeConfigCommand(432-435)pkg/wconfig/settingsconfig.go (1)
AIModeConfigUpdate(287-289)pkg/aiusechat/usechat-mode.go (1)
ComputeResolvedAIModeConfigs(253-263)
frontend/app/aipanel/aipanel.tsx (3)
frontend/app/store/global.ts (1)
atoms(825-825)frontend/app/aipanel/byokannouncement.tsx (1)
BYOKAnnouncement(73-73)frontend/app/aipanel/aitypes.ts (1)
WaveUIMessage(35-35)
frontend/types/gotypes.d.ts (1)
pkg/wconfig/settingsconfig.go (2)
AIModeConfigUpdate(287-289)AIModeConfigType(266-285)
frontend/wave.ts (3)
frontend/app/store/wshclientapi.ts (1)
RpcApi(687-687)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)frontend/app/store/global.ts (2)
globalStore(848-848)atoms(825-825)
⏰ 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 (23)
frontend/app/aipanel/aipanel.tsx (4)
24-24: LGTM!The
WaveUIMessagetype import aligns with the type used in theuseChatgeneric parameter on line 227.
87-90: LGTM!The logic correctly derives
hasCustomModesby checking for any key that doesn't start with the internal"waveai@"prefix. The optional chaining safely handles cases wherefullConfigorwaveaimay be undefined.
163-163: LGTM!Conditionally hiding the BYOK announcement when custom modes are configured makes sense—users with custom modes have already set up their own API keys.
227-227: LGTM!Adding the
WaveUIMessagegeneric type parameter touseChatimproves type safety and ensures consistency with the typing inAIPanelMessages.frontend/app/aipanel/aipanelmessages.tsx (1)
8-8: LGTM!The type-only import aligns with the new typing for the
messagesprop.pkg/wps/wpstypes.go (1)
8-25: NewEvent_AIModeConfigconstant is consistent with existing event taxonomyName and value fit the existing
waveai:*event pattern; no issues spotted.pkg/wconfig/settingsconfig.go (1)
265-302:AIModeConfigUpdatetype wiring looks correctThe new
AIModeConfigUpdatewrapper cleanly exposes aconfigsmap and aligns with how it’s used in RPC and event payloads; no structural issues found.pkg/wshrpc/wshclient/wshclient.go (1)
425-437:GetWaveAIModeConfigCommandmatches existing RPC helper patternsTyped response, nil data payload, and naming all align with
GetFullConfigCommandand friends; looks good.pkg/aiusechat/usechat-prompts.go (1)
46-81: No-tools system prompt is coherent and aligned with capabilitiesThe new
SystemPromptText_NoToolsclearly describes a tools-less environment while keeping formatting/safety guidance consistent; no issues from a behavior perspective.pkg/aiusechat/usechat.go (1)
88-98: Trimming secrets before validation is a safe robustness improvement
strings.TrimSpace(secret)before the!exists || secret == ""check ensures secrets that are only whitespace are rejected without changing semantics for valid tokens. This should help avoid subtle misconfigurations without impacting correct setups.pkg/aiusechat/usechat-mode.go (1)
253-263: Resolved AI mode configs computation is correct and side-effect free
ComputeResolvedAIModeConfigsiteratesfullConfig.WaveAIModes, copies each config, applies provider defaults on the copy, and stores it in a fresh map. That avoids mutating the watcher’s underlying config while ensuring RPCs/events see fully-resolved configs.cmd/server/main-server.go (1)
16-18: AI mode config watcher initialization is wired into startup correctlyHooking
aiusechat.InitAIModeConfigWatcher()right afterstartConfigWatcher()is a good place to ensure updates are broadcast as soon as config watching is active. With a nil-guard insideInitAIModeConfigWatcher, this path will also remain robust if the underlying file watcher cannot be created.Also applies to: 522-532
pkg/wshrpc/wshrpctypes.go (2)
102-102: LGTM!The new command constant follows the existing naming convention and is properly grouped with other config-related commands.
249-249: LGTM!The interface method signature is consistent with other config retrieval methods (e.g.,
GetFullConfigCommand). The return type correctly useswconfig.AIModeConfigUpdatewhich wraps the resolved mode configurations.frontend/types/gotypes.d.ts (2)
38-41: LGTM!The
AIModeConfigUpdatetype correctly mirrors the Go struct definition frompkg/wconfig/settingsconfig.go, with the map type properly represented as{[key: string]: AIModeConfigType}.
1222-1223: LGTM!The new telemetry properties are consistently added to both
TEventPropsandTEventUserProps, maintaining parity between the two types.Also applies to: 1306-1307
pkg/wshrpc/wshserver/wshserver.go (1)
595-599: LGTM!The implementation follows the established pattern of
GetFullConfigCommandand correctly usesaiusechat.ComputeResolvedAIModeConfigsto resolve provider defaults before returning the mode configurations.frontend/app/store/global.ts (2)
110-110: LGTM!The atom initialization follows the same pattern as
fullConfigAtomand the typeRecord<string, AIModeConfigType>correctly represents the resolved mode configurations map.
223-229: LGTM!The event handler correctly extracts the
configsfield from theAIModeConfigUpdatepayload and updates the atom. This follows the same pattern as the existingconfigevent handler forfullConfigAtom.frontend/app/store/wshclientapi.ts (1)
355-358: LGTM!The generated RPC method correctly matches the backend interface. It follows the same pattern as
GetFullConfigCommand(no input data, returns config type) and uses the correct command string"getwaveaimodeconfig".frontend/types/custom.d.ts (2)
19-19: LGTM!The atom declaration correctly stores AI mode configurations as a map keyed by mode name, with clear documentation indicating WebSocket updates.
501-501: LGTM!The intersection type correctly extends
AIModeConfigTypewith amodeproperty, providing a convenient combined type for UI components that need both the mode identifier and its configuration.pkg/tsgen/tsgen.go (1)
56-56: LGTM!The addition correctly includes
AIModeConfigUpdatein the TypeScript generation, following the established pattern and enabling frontend access to the AI mode configuration update type.
| func InitAIModeConfigWatcher() { | ||
| watcher := wconfig.GetWatcher() | ||
| watcher.RegisterUpdateHandler(handleConfigUpdate) | ||
| log.Printf("AI mode config watcher initialized\n") | ||
| } | ||
|
|
||
| func handleConfigUpdate(fullConfig wconfig.FullConfigType) { | ||
| resolvedConfigs := ComputeResolvedAIModeConfigs(fullConfig) | ||
| broadcastAIModeConfigs(resolvedConfigs) | ||
| } | ||
|
|
||
| func ComputeResolvedAIModeConfigs(fullConfig wconfig.FullConfigType) map[string]wconfig.AIModeConfigType { | ||
| resolvedConfigs := make(map[string]wconfig.AIModeConfigType) | ||
|
|
||
| for modeName, modeConfig := range fullConfig.WaveAIModes { | ||
| resolved := modeConfig | ||
| applyProviderDefaults(&resolved) | ||
| resolvedConfigs[modeName] = resolved | ||
| } | ||
|
|
||
| return resolvedConfigs | ||
| } | ||
|
|
||
| func broadcastAIModeConfigs(configs map[string]wconfig.AIModeConfigType) { | ||
| update := wconfig.AIModeConfigUpdate{ | ||
| Configs: configs, | ||
| } | ||
|
|
||
| wps.Broker.Publish(wps.WaveEvent{ | ||
| Event: wps.Event_AIModeConfig, | ||
| Data: update, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add nil-check for the config watcher to avoid startup panics
wconfig.GetWatcher() can return nil (e.g., if fsnotify.NewWatcher fails). Other call sites guard against this, but InitAIModeConfigWatcher calls RegisterUpdateHandler unconditionally, which would panic in that scenario and regress from “degrade gracefully” to “crash on startup”.
Recommend guarding the watcher before use:
func InitAIModeConfigWatcher() {
- watcher := wconfig.GetWatcher()
- watcher.RegisterUpdateHandler(handleConfigUpdate)
- log.Printf("AI mode config watcher initialized\n")
+ watcher := wconfig.GetWatcher()
+ if watcher == nil {
+ log.Printf("AI mode config watcher not initialized: config watcher unavailable\n")
+ return
+ }
+ watcher.RegisterUpdateHandler(handleConfigUpdate)
+ log.Printf("AI mode config watcher initialized\n")
}🤖 Prompt for AI Agents
In pkg/aiusechat/usechat-mode.go around lines 242 to 274,
InitAIModeConfigWatcher calls wconfig.GetWatcher() without checking for nil
which can cause a panic if watcher creation failed; change
InitAIModeConfigWatcher to check if watcher == nil and if so log a warning/error
and return early (do not call RegisterUpdateHandler), otherwise register the
handler and proceed as before so the service degrades gracefully instead of
crashing on startup.
No description provided.