Skip to content

Conversation

@sawka
Copy link
Member

@sawka sawka commented Dec 9, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

Walkthrough

Adds centralized AI mode display-name generation (getModeDisplayName) and updates UI to use it, introduces a "Configure Modes" context menu entry, exposes Wave AI mode configs to the frontend via a new atom and RPC, and adds an AIModeConfig watcher that resolves provider defaults and broadcasts mode configs. Adds types and RPC plumbing for GetWaveAIModeConfig, registers the watcher at server start, trims whitespace from stored secrets and API tokens, introduces a no-tools system prompt and toggling based on tool/widget capabilities, and tightens several TypeScript typings across the AI panel.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing careful review:
    • pkg/aiusechat: new InitAIModeConfigWatcher, ComputeResolvedAIModeConfigs, broadcast flow, and changed getSystemPrompt signature (ensure callers updated correctly).
    • RPC surface and server: new WshRpc command constant, WshServer.GetWaveAIModeConfigCommand, wshclient helper, frontend RpcApi additions (watch for duplicate method insertion noted).
    • Frontend state/typing: new waveaiModeConfigAtom, AIModeConfigWithMode type, atom exposure in global store, and places that consume the atom (wave.ts, UI components).
    • UI changes: ai-utils.getModeDisplayName logic and all places updated to use it; AIMode dropdown/menu/context menu additions and no-tools Tooltip UI.
    • Types and codegen: added wconfig.AIModeConfigUpdate into tsgen ExtraTypes and new public Go type in pkg/wconfig (verify generated artifacts and round-trip types).
    • Secret handling: trimming CRLF/whitespace in secretstore.SetSecret and getWaveAISettings token trimming — validate no unintended side effects.
    • Duplicates and consistency: check duplicate declarations reported (SystemPromptText_NoTools duplicate, duplicated RpcApi method insertion) and resolve potential build/type conflicts.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset. Add a detailed description explaining the purpose and scope of the changes, including AI mode configuration management, tool capability detection, and related UI updates.
Docstring Coverage ⚠️ Warning Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The PR title 'minor v0.13 fixes' is vague and generic, using non-descriptive terms that don't convey meaningful information about the substantial changeset. Consider a more descriptive title that highlights the main changes, such as 'Add AI mode config management and tool capability handling' or similar.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/minor-v013

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 values

Right now the !exists || secret == "" check runs on the raw secret, and only afterwards you TrimSpace:

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 to apiToken, 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 = secret

This 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

SetSecret now strips all trailing \r/\n characters 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 consistent

Wiring onContextMenu on 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, handleContextMenu could just delegate to handleKebabClick(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 classes

The styling changes look good, but the new classes !text-blue-400 / hover:!text-blue-300 use 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

getModeDisplayName cleanly encapsulates the mode naming rules (explicit display name, azure‑legacy resource name, or model (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 logic

Using getModeDisplayName in AIModeMenuItem and for the dropdown’s displayName/tooltip ensures all mode labels follow the same rules, and the displayIcon fallback keeps the icon behavior predictable. The pre-line description 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 } & AIModeConfigType instead of any, 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4ac5e0b and e39ec69.

⛔ Files ignored due to path filters (1)
  • package-lock.json is 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.go
  • frontend/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 consistent

The additions of "settings:customaimodes" and "settings:secretscount" to both TEventProps and TEventUserProps match the existing settings:* 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 solid

Using fullConfig.waveai to detect non‑waveai@ modes is a reasonable way to infer custom modes, and gating <BYOKAnnouncement /> on !hasCustomModes should avoid redundant BYOK prompts once users have configured their own modes. Attaching onContextMenu in the empty state to handleWaveAIContextMenu(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: Importing getModeDisplayName to centralize labels is a good move

Pulling getModeDisplayName in here keeps AI mode display naming logic DRY and tied to a single helper. No issues with the import or usage pattern.


60-83: Using getModeDisplayName for Wave AI modes improves consistency

Swapping 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 via getModeDisplayName matches Wave modes

Applying 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 correct

The separator placement keeps the new item visually grouped with AI settings, and the click handler both fires RecordTEventCommand (with noresponse: true for non-blocking telemetry) and opens the Wave AI config via model.openWaveAIConfig(). This integrates cleanly with the existing RpcApi/TabRpcClient pattern.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 messages prop type from any[] to WaveUIMessage[] improves type safety and aligns with the typed useChat<WaveUIMessage> in aipanel.tsx.

Note: Line 78 still uses as any for the placeholder streaming message. Consider defining a proper type-safe placeholder or a minimal WaveUIMessage factory if this pattern is used elsewhere.

pkg/aiusechat/usechat.go (1)

50-65: System prompt selection logic correctly gates on tools/widget access

Routing to SystemPromptText_NoTools when !hasToolsCapability || !widgetAccess and skipping the strict tool add-on in that case is a solid way to avoid misleading the model about tools.

Minor nit: apiType is now unused in getSystemPrompt. 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 desired

Publishing AIModeConfigUpdate on Event_AIModeConfig via wps.Broker matches 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 RPCs

Storing waveaiModeConfig.configs into atoms.waveaiModeConfigAtom in both initWave and initBuilder wires 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

📥 Commits

Reviewing files that changed from the base of the PR and between e39ec69 and 596e208.

📒 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.ts
  • frontend/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.ts
  • frontend/app/store/global.ts
  • frontend/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.go
  • pkg/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 WaveUIMessage type import aligns with the type used in the useChat generic parameter on line 227.


87-90: LGTM!

The logic correctly derives hasCustomModes by checking for any key that doesn't start with the internal "waveai@" prefix. The optional chaining safely handles cases where fullConfig or waveai may 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 WaveUIMessage generic type parameter to useChat improves type safety and ensures consistency with the typing in AIPanelMessages.

frontend/app/aipanel/aipanelmessages.tsx (1)

8-8: LGTM!

The type-only import aligns with the new typing for the messages prop.

pkg/wps/wpstypes.go (1)

8-25: New Event_AIModeConfig constant is consistent with existing event taxonomy

Name and value fit the existing waveai:* event pattern; no issues spotted.

pkg/wconfig/settingsconfig.go (1)

265-302: AIModeConfigUpdate type wiring looks correct

The new AIModeConfigUpdate wrapper cleanly exposes a configs map and aligns with how it’s used in RPC and event payloads; no structural issues found.

pkg/wshrpc/wshclient/wshclient.go (1)

425-437: GetWaveAIModeConfigCommand matches existing RPC helper patterns

Typed response, nil data payload, and naming all align with GetFullConfigCommand and friends; looks good.

pkg/aiusechat/usechat-prompts.go (1)

46-81: No-tools system prompt is coherent and aligned with capabilities

The new SystemPromptText_NoTools clearly 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

ComputeResolvedAIModeConfigs iterates fullConfig.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 correctly

Hooking aiusechat.InitAIModeConfigWatcher() right after startConfigWatcher() is a good place to ensure updates are broadcast as soon as config watching is active. With a nil-guard inside InitAIModeConfigWatcher, 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 uses wconfig.AIModeConfigUpdate which wraps the resolved mode configurations.

frontend/types/gotypes.d.ts (2)

38-41: LGTM!

The AIModeConfigUpdate type correctly mirrors the Go struct definition from pkg/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 TEventProps and TEventUserProps, 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 GetFullConfigCommand and correctly uses aiusechat.ComputeResolvedAIModeConfigs to 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 fullConfigAtom and the type Record<string, AIModeConfigType> correctly represents the resolved mode configurations map.


223-229: LGTM!

The event handler correctly extracts the configs field from the AIModeConfigUpdate payload and updates the atom. This follows the same pattern as the existing config event handler for fullConfigAtom.

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 AIModeConfigType with a mode property, 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 AIModeConfigUpdate in the TypeScript generation, following the established pattern and enabling frontend access to the AI mode configuration update type.

Comment on lines +242 to +274
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,
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

@sawka sawka merged commit c9c6192 into main Dec 9, 2025
7 checks passed
@sawka sawka deleted the sawka/minor-v013 branch December 9, 2025 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants